[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7ajtd5skxgfkkwolsbxrd7y2yzfjsikffkg4shxvpumconc2rz@i7hk4gvirrx2>
Date: Sat, 3 Feb 2024 12:59:44 +0800
From: "Wang, Qingshun" <qingshun.wang@...ux.intel.com>
To: Dan Williams <dan.j.williams@...el.com>
Cc: linux-pci@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org,
linux-acpi@...r.kernel.org, chao.p.peng@...ux.intel.com, erwin.tsaur@...el.com,
feiting.wanyan@...el.com, qingshun.wang@...el.com, "Rafael J. Wysocki" <rafael@...nel.org>,
Len Brown <lenb@...nel.org>, James Morse <james.morse@....com>,
Tony Luck <tony.luck@...el.com>, Borislav Petkov <bp@...en8.de>,
Davidlohr Bueso <dave@...olabs.net>, Jonathan Cameron <jonathan.cameron@...wei.com>,
Dave Jiang <dave.jiang@...el.com>, Alison Schofield <alison.schofield@...el.com>,
Vishal Verma <vishal.l.verma@...el.com>, Ira Weiny <ira.weiny@...el.com>,
Bjorn Helgaas <bhelgaas@...gle.com>, Bjorn Helgaas <helgaas@...nel.org>,
Mahesh J Salgaonkar <mahesh@...ux.ibm.com>, Oliver O'Halloran <oohall@...il.com>,
Miaohe Lin <linmiaohe@...wei.com>, Shiju Jose <shiju.jose@...wei.com>,
Adam Preble <adam.c.preble@...el.com>, Li Yang <leoyang.li@....com>, Lukas Wunner <lukas@...ner.de>,
Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@...ux.intel.com>, Smita Koralahalli <Smita.KoralahalliChannabasappa@....com>,
Robert Richter <rrichter@....com>, linux-kernel@...r.kernel.org, linux-cxl@...r.kernel.org,
linux-edac@...r.kernel.org
Subject: Re: [PATCH v2 3/4] PCI/AER: Fetch information for FTrace
On Fri, Feb 02, 2024 at 10:01:40AM -0800, Dan Williams wrote:
> Wang, Qingshun wrote:
> > Fetch and store the data of 3 more registers: "Link Status", "Device
> > Control 2", and "Advanced Error Capabilities and Control". This data is
> > needed for external observation to better understand ANFE.
> >
> > Signed-off-by: "Wang, Qingshun" <qingshun.wang@...ux.intel.com>
> > ---
> > drivers/acpi/apei/ghes.c | 8 +++++++-
> > drivers/cxl/core/pci.c | 11 ++++++++++-
> > drivers/pci/pci.h | 4 ++++
> > drivers/pci/pcie/aer.c | 26 ++++++++++++++++++++------
> > include/linux/aer.h | 6 ++++--
> > 5 files changed, 45 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
> > index 6034039d5cff..047cc01be68c 100644
> > --- a/drivers/acpi/apei/ghes.c
> > +++ b/drivers/acpi/apei/ghes.c
> > @@ -594,7 +594,9 @@ static void ghes_handle_aer(struct acpi_hest_generic_data *gdata)
> > if (pcie_err->validation_bits & CPER_PCIE_VALID_DEVICE_ID &&
> > pcie_err->validation_bits & CPER_PCIE_VALID_AER_INFO) {
> > struct pcie_capability_regs *pcie_caps;
> > + u16 device_control_2 = 0;
> > u16 device_status = 0;
> > + u16 link_status = 0;
> > unsigned int devfn;
> > int aer_severity;
> > u8 *aer_info;
> > @@ -619,7 +621,9 @@ static void ghes_handle_aer(struct acpi_hest_generic_data *gdata)
> >
> > if (pcie_err->validation_bits & CPER_PCIE_VALID_CAPABILITY) {
> > pcie_caps = (struct pcie_capability_regs *)pcie_err->capability;
> > + device_control_2 = pcie_caps->device_control_2;
> > device_status = pcie_caps->device_status;
> > + link_status = pcie_caps->link_status;
> > }
> >
> > aer_recover_queue(pcie_err->device_id.segment,
> > @@ -627,7 +631,9 @@ static void ghes_handle_aer(struct acpi_hest_generic_data *gdata)
> > devfn, aer_severity,
> > (struct aer_capability_regs *)
> > aer_info,
> > - device_status);
> > + device_status,
> > + link_status,
> > + device_control_2);
> > }
> > #endif
> > }
> > diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> > index 9111a4415a63..3aa57fe8db42 100644
> > --- a/drivers/cxl/core/pci.c
> > +++ b/drivers/cxl/core/pci.c
> > @@ -903,7 +903,9 @@ static void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds)
> > struct aer_capability_regs aer_regs;
> > struct cxl_dport *dport;
> > struct cxl_port *port;
> > + u16 device_control_2;
> > u16 device_status;
> > + u16 link_status;
> > int severity;
> >
> > port = cxl_pci_find_port(pdev, &dport);
> > @@ -918,10 +920,17 @@ static void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds)
> > if (!cxl_rch_get_aer_severity(&aer_regs, &severity))
> > return;
> >
> > + if (pcie_capability_read_word(pdev, PCI_EXP_DEVCTL2, &device_control_2))
> > + return;
> > +
> > if (pcie_capability_read_word(pdev, PCI_EXP_DEVSTA, &device_status))
> > return;
> >
> > - pci_print_aer(pdev, severity, &aer_regs, device_status);
> > + if (pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &link_status))
> > + return;
> > +
> > + pci_print_aer(pdev, severity, &aer_regs, device_status,
> > + link_status, device_control_2);
>
> Rather than complicate the calling convention of pci_print_aer(), update
> the internals of pci_print_aer() to get these extra registers, or
> provide a new wrapper interface that satisfies the dependencies and
> switch users over to that. Otherwise multiple touches of the same code
> path in one patch set is indicative of the need for a higher level
> helper.
Thanks for the advice, it does make sense. Will reconsider the
implementation.
--
Best regards,
Wang, Qingshun
Powered by blists - more mailing lists