[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9fa49c5d-4939-db48-c815-ccb496c82d0b@linux.intel.com>
Date: Tue, 17 Dec 2024 17:36:51 +0200 (EET)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Bjorn Helgaas <bhelgaas@...gle.com>
cc: linux-pci@...r.kernel.org,
Krzysztof Wilczyński <kw@...ux.com>,
Lukas Wunner <lukas@...ner.de>,
Jonathan Cameron <Jonathan.Cameron@...wei.com>,
Mahesh J Salgaonkar <mahesh@...ux.ibm.com>,
Oliver O'Halloran <oohall@...il.com>, LKML <linux-kernel@...r.kernel.org>,
linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH v7 6/8] PCI: Add TLP Prefix reading into
pcie_read_tlp_log()
On Tue, 17 Dec 2024, Ilpo Järvinen wrote:
> pcie_read_tlp_log() handles only 4 Header Log DWORDs but TLP Prefix Log
> (PCIe r6.1 secs 7.8.4.12 & 7.9.14.13) may also be present.
>
> Generalize pcie_read_tlp_log() and struct pcie_tlp_log to handle also
> TLP Prefix Log. The relevant registers are formatted identically in AER
> and DPC Capability, but has these variations:
>
> a) The offsets of TLP Prefix Log registers vary.
> b) DPC RP PIO TLP Prefix Log register can be < 4 DWORDs.
> c) AER TLP Prefix Log Present (PCIe r6.1 sec 7.8.4.7) can indicate
> Prefix Log is not present.
>
> Therefore callers must pass the offset of the TLP Prefix Log register
> and the entire length to pcie_read_tlp_log() to be able to read the
> correct number of TLP Prefix DWORDs from the correct offset.
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>
> ---
> drivers/pci/pci.h | 5 +++-
> drivers/pci/pcie/aer.c | 5 +++-
> drivers/pci/pcie/dpc.c | 13 +++++----
> drivers/pci/pcie/tlp.c | 50 +++++++++++++++++++++++++++++++----
> include/linux/aer.h | 1 +
> include/uapi/linux/pci_regs.h | 10 ++++---
> 6 files changed, 66 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index 55fcf3bac4f7..7797b2544d00 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -550,7 +550,9 @@ struct aer_err_info {
> int aer_get_device_error_info(struct pci_dev *dev, struct aer_err_info *info);
> void aer_print_error(struct pci_dev *dev, struct aer_err_info *info);
>
> -int pcie_read_tlp_log(struct pci_dev *dev, int where, struct pcie_tlp_log *log);
> +int pcie_read_tlp_log(struct pci_dev *dev, int where, int where2,
> + unsigned int tlp_len, struct pcie_tlp_log *log);
> +unsigned int aer_tlp_log_len(struct pci_dev *dev, u32 aercc);
> #endif /* CONFIG_PCIEAER */
>
> #ifdef CONFIG_PCIEPORTBUS
> @@ -569,6 +571,7 @@ void pci_dpc_init(struct pci_dev *pdev);
> void dpc_process_error(struct pci_dev *pdev);
> pci_ers_result_t dpc_reset_link(struct pci_dev *pdev);
> bool pci_dpc_recovered(struct pci_dev *pdev);
> +unsigned int dpc_tlp_log_len(struct pci_dev *dev);
> #else
> static inline void pci_save_dpc_state(struct pci_dev *dev) { }
> static inline void pci_restore_dpc_state(struct pci_dev *dev) { }
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 80c5ba8d8296..656dbf1ac45b 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -1248,7 +1248,10 @@ int aer_get_device_error_info(struct pci_dev *dev, struct aer_err_info *info)
>
> if (info->status & AER_LOG_TLP_MASKS) {
> info->tlp_header_valid = 1;
> - pcie_read_tlp_log(dev, aer + PCI_ERR_HEADER_LOG, &info->tlp);
> + pcie_read_tlp_log(dev, aer + PCI_ERR_HEADER_LOG,
> + aer + PCI_ERR_PREFIX_LOG,
> + aer_tlp_log_len(dev, aercc),
> + &info->tlp);
> }
> }
>
> diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
> index 2b6ef7efa3c1..7933b3cedb59 100644
> --- a/drivers/pci/pcie/dpc.c
> +++ b/drivers/pci/pcie/dpc.c
> @@ -190,7 +190,7 @@ pci_ers_result_t dpc_reset_link(struct pci_dev *pdev)
> static void dpc_process_rp_pio_error(struct pci_dev *pdev)
> {
> u16 cap = pdev->dpc_cap, dpc_status, first_error;
> - u32 status, mask, sev, syserr, exc, log, prefix;
> + u32 status, mask, sev, syserr, exc, log;
> struct pcie_tlp_log tlp_log;
> int i;
>
> @@ -217,20 +217,19 @@ static void dpc_process_rp_pio_error(struct pci_dev *pdev)
>
> if (pdev->dpc_rp_log_size < 4)
> goto clear_status;
> - pcie_read_tlp_log(pdev, cap + PCI_EXP_DPC_RP_PIO_HEADER_LOG, &tlp_log);
> + pcie_read_tlp_log(pdev, cap + PCI_EXP_DPC_RP_PIO_HEADER_LOG,
> + cap + PCI_EXP_DPC_RP_PIO_TLPPREFIX_LOG,
> + dpc_tlp_log_len(pdev), &tlp_log);
> pci_err(pdev, "TLP Header: %#010x %#010x %#010x %#010x\n",
> tlp_log.dw[0], tlp_log.dw[1], tlp_log.dw[2], tlp_log.dw[3]);
> + for (i = 0; i < pdev->dpc_rp_log_size - 5; i++)
> + pci_err(pdev, "TLP Prefix Header: dw%d, %#010x\n", i, tlp_log.prefix[i]);
>
> if (pdev->dpc_rp_log_size < 5)
> goto clear_status;
> pci_read_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_IMPSPEC_LOG, &log);
> pci_err(pdev, "RP PIO ImpSpec Log %#010x\n", log);
>
> - for (i = 0; i < pdev->dpc_rp_log_size - 5; i++) {
> - pci_read_config_dword(pdev,
> - cap + PCI_EXP_DPC_RP_PIO_TLPPREFIX_LOG + i * 4, &prefix);
> - pci_err(pdev, "TLP Prefix Header: dw%d, %#010x\n", i, prefix);
> - }
> clear_status:
> pci_write_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_STATUS, status);
> }
> diff --git a/drivers/pci/pcie/tlp.c b/drivers/pci/pcie/tlp.c
> index 65ac7b5d8a87..0c4bf6a50d7e 100644
> --- a/drivers/pci/pcie/tlp.c
> +++ b/drivers/pci/pcie/tlp.c
> @@ -11,26 +11,66 @@
>
> #include "../pci.h"
>
> +/**
> + * aer_tlp_log_len - Calculates AER Capability TLP Header/Prefix Log length
> + * @dev: PCIe device
> + *
> + * Return: TLP Header/Prefix Log length
> + */
> +unsigned int aer_tlp_log_len(struct pci_dev *dev, u32 aercc)
Hmpf, it seems I forgot to move the @aercc kerneldoc from Flit mode series
patch were it was originally added to here when I needed aercc to the
PCI_ERR_CAP_PREFIX_LOG_PRESENT check.
I'll send an update in a day or two.
> +{
> + return 4 + (aercc & PCI_ERR_CAP_PREFIX_LOG_PRESENT) ?
> + dev->eetlp_prefix_max : 0;
> +}
--
i.
Powered by blists - more mailing lists