Received-SPF: pass (google.com: domain of linux-kernel+bounces-50284-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Fri, 2 Feb 2024 10:01:40 -0800
From: Dan Williams <dan.j.williams@intel.com>
To: "Wang, Qingshun" <qingshun.wang@linux.intel.com>,
	<linux-pci@vger.kernel.org>, <linuxppc-dev@lists.ozlabs.org>,
	<linux-acpi@vger.kernel.org>
CC: <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>, Dan Williams
	<dan.j.williams@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>, "Wang, Qingshun"
	<qingshun.wang@linux.intel.com>
Subject: Re: [PATCH v2 3/4] PCI/AER: Fetch information for FTrace
Message-ID: <65bd2e04aed44_2d43c29463@dwillia2-mobl3.amr.corp.intel.com.notmuch>
References: <20240125062802.50819-1-qingshun.wang@linux.intel.com>
 <20240125062802.50819-4-qingshun.wang@linux.intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240125062802.50819-4-qingshun.wang@linux.intel.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?MmxC5IqBK7VpmiylOz8C5hYmORS6ZQCJDYAOS7p3lFhIUiQuRmVvAJ8oTx2W?=
 =?us-ascii?Q?7gC7RcFx/dNdKaJCMj1azUVcxB+dP1jGdI0+UTpVrsixJATXD1x5d+Q+4Evp?=
 =?us-ascii?Q?RmVkGcRKRt6A7uhenh1SKx3qFhwHfdIn+br36QVOEV+hetOGIl+rvGu/WAds?=
 =?us-ascii?Q?CWuzBqLcHK1g0A3LXklbooA3VNCAB4HePr/9oT65Vw+ksrxc1QPj7UyW1tPU?=
 =?us-ascii?Q?bBC7B0RknQbhTHS9rDzaAecBcs0PuxKKO9GHy0nCifiknlkxDPAD97IUUEaC?=
 =?us-ascii?Q?5fCkw3Lv+CX/r9pfA6aEzRtigOgNkSJS4M3FqaPiBJk4DpgTWbDKVlLGCHnn?=
 =?us-ascii?Q?pKz9Dg+O9sr6JTdPZc8BwsNUBVWKTCH0W1oQnjbwNnZI5iopx61b9HBiyY5D?=
 =?us-ascii?Q?w5naYfI047WeuJRm1dQJzWgra0EtjhA4uzk3aKaTb2V66GtnEOU6rgCbNSS8?=
 =?us-ascii?Q?0EZ4cJGaIG+VxxfSRhmVn4RATx0mK2/CaICsRwRmofd9cUIHijzdA5pAowZg?=
 =?us-ascii?Q?9h5MLG8jwgVXoa6g1KiNaPkFSOnoZVmuoSSCF/CYHX14k5WUE1Yx5V27deLt?=
 =?us-ascii?Q?2eo+A/e3nmxskPP21OMspT4VbLFeBmo1WUYTpgU9F5CBw+wUZSk3whvDOM5h?=
 =?us-ascii?Q?2NRBGNIOlxDHcMpDv5lGYXMfEtj9PMcnXbmaeVUVej8Ig8vCN62hUdCYL1Hk?=
 =?us-ascii?Q?VMP+rUCMvrZjg8sqSuJHiV9bZkz0btJ5XHIPd4czVD0XEDv0LxSWO0v+TLtY?=
 =?us-ascii?Q?Omm8uiFXtK9mrfSU9ScpXqyeTSvrG1OnHmJlk+nBq72BO9TZ5pB5ZzWHazN9?=
 =?us-ascii?Q?s50AoPUOGs/bIISEtdCeJT/jZ+U1qIovqpdsJiIkOudwDUX29ZuJnlRQ0gKB?=
 =?us-ascii?Q?lpyiPgR07kwoeKRapoWxvEYU4er0jlaVOgA9Yp03EY5nuU7ePNaDucJkm8o7?=
 =?us-ascii?Q?nEKvMw/CyDNqpi7KAlTe+RwZfH+PwPXE/GN1oqlU0Nmi2TZJna1cYHM/ec/y?=
 =?us-ascii?Q?WtODf2Z6j1jo83lKBIO7yGJZ7KH1BN7xpHOXav/4ITmngfwKRvFQWtBm9eKb?=
 =?us-ascii?Q?5GRoK2SJeN2vi/j6pg7DJ0iSFLWSuJrAfQP5Whrf83S000W7oRuA9r4nAzau?=
 =?us-ascii?Q?uczcAmOanQ/+pYqGHhIn9HPQdhr23L5m58i0k1luh5U/+begUDyjEaligoVD?=
 =?us-ascii?Q?DxrCHki274KTOFUdp8kzbq2CFCRZ9ng9mkXbkcc8TfdhRVLd65rMWGxBPAec?=
 =?us-ascii?Q?GthtX+vvfv4gr3HY+OOpdiOK6KDErHLjlH9HZULoBA9+wK8VlFNpZfi0qPrS?=
 =?us-ascii?Q?/GSI+fAN35xYqPBmdBisjBj28Nr2ygRTbmG+l9jFw50vfC+ATvzkrH94FUJD?=
 =?us-ascii?Q?ET0u+Kmk0PRERDy/UK7huva5KHesTxp+GXjA5qmNv97w1/fN1hfdYo5w2tSS?=
 =?us-ascii?Q?vyRFsCffWjKAnqFN8PgixccHoudsnW2xv6hQ7Ydb9Mmp6xZVeIHcwQ0MhX38?=
 =?us-ascii?Q?5+cLLXUBqR2NmNFDy7KNURiw2Tafdv4NTnLI+o64OrLm1CXWVfxbmRL1Ioma?=
 =?us-ascii?Q?4gh+KBuRu0aFy8qYpCW3ZdaDN+DK70lVzOA3wbIqAHIOajBRZlvrDivxORhh?=
 =?us-ascii?Q?qg=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: cf3eb480-5998-4d54-f111-08dc241904b5
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB8107.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Feb 2024 18:01:45.3268
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: k2Cd9c1VGv7LEosK26C1uvC9rlNP9JVwMX026iF/zzuUoV7qNN4Oyiyrl2GzhgscB4Ldb9938GcQBQZAuRVmui7XdeQ4dBGmwTnXp3jYdXM=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR11MB5506
X-OriginatorOrg: intel.com

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.