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@xxxxxxxxxxxxxxx> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> > --- > 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.