Received-SPF: pass (google.com: domain of linux-kernel+bounces-50926-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Sat, 3 Feb 2024 12:59:44 +0800
From: "Wang, Qingshun" <qingshun.wang@linux.intel.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: linux-pci@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, 
	linux-acpi@vger.kernel.org, chao.p.peng@linux.intel.com, erwin.tsaur@intel.com, 
	feiting.wanyan@intel.com, qingshun.wang@intel.com, "Rafael J. Wysocki" <rafael@kernel.org>, 
	Len Brown <lenb@kernel.org>, James Morse <james.morse@arm.com>, 
	Tony Luck <tony.luck@intel.com>, Borislav Petkov <bp@alien8.de>, 
	Davidlohr Bueso <dave@stgolabs.net>, Jonathan Cameron <jonathan.cameron@huawei.com>, 
	Dave Jiang <dave.jiang@intel.com>, Alison Schofield <alison.schofield@intel.com>, 
	Vishal Verma <vishal.l.verma@intel.com>, Ira Weiny <ira.weiny@intel.com>, 
	Bjorn Helgaas <bhelgaas@google.com>, Bjorn Helgaas <helgaas@kernel.org>, 
	Mahesh J Salgaonkar <mahesh@linux.ibm.com>, Oliver O'Halloran <oohall@gmail.com>, 
	Miaohe Lin <linmiaohe@huawei.com>, Shiju Jose <shiju.jose@huawei.com>, 
	Adam Preble <adam.c.preble@intel.com>, Li Yang <leoyang.li@nxp.com>, Lukas Wunner <lukas@wunner.de>, 
	Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>, Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>, 
	Robert Richter <rrichter@amd.com>, linux-kernel@vger.kernel.org, linux-cxl@vger.kernel.org, 
	linux-edac@vger.kernel.org
Subject: Re: [PATCH v2 3/4] PCI/AER: Fetch information for FTrace
Message-ID: <7ajtd5skxgfkkwolsbxrd7y2yzfjsikffkg4shxvpumconc2rz@i7hk4gvirrx2>
References: <20240125062802.50819-1-qingshun.wang@linux.intel.com>
 <20240125062802.50819-4-qingshun.wang@linux.intel.com>
 <65bd2e04aed44_2d43c29463@dwillia2-mobl3.amr.corp.intel.com.notmuch>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <65bd2e04aed44_2d43c29463@dwillia2-mobl3.amr.corp.intel.com.notmuch>

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@linux.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