Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <9c54d736-1f4f-3c5b-d013-3812f5a0263d@intel.com>
Date:   Fri, 23 Jun 2023 15:07:00 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Betterbird/102.11.1
Subject: Re: [PATCH v7 23/27] cxl/pci: Add RCH downstream port error logging
To:     Terry Bowman <terry.bowman@amd.com>, <alison.schofield@intel.com>,
        <vishal.l.verma@intel.com>, <ira.weiny@intel.com>,
        <bwidawsk@kernel.org>, <dan.j.williams@intel.com>,
        <Jonathan.Cameron@huawei.com>, <linux-cxl@vger.kernel.org>
CC:     <rrichter@amd.com>, <linux-kernel@vger.kernel.org>,
        <bhelgaas@google.com>
References: <20230622205523.85375-1-terry.bowman@amd.com>
 <20230622205523.85375-24-terry.bowman@amd.com>
Content-Language: en-US
From:   Dave Jiang <dave.jiang@intel.com>
In-Reply-To: <20230622205523.85375-24-terry.bowman@amd.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?b1MydnFjUElvNFFuZEdDQWpFcGJSYnJMN0VGWHp0S1gxVWdsQ0lGRW42d0RV?=
 =?utf-8?B?WWx0L2hQeGEzdTN3eWRoR2J1NmdpRUthYWhrVVcrYTVsY2ZSdDM4WHQ1T09N?=
 =?utf-8?B?d3JWM1FiQStya2JVTjhIZlNkaUZWeWVMb2lWRW9oOXU5b1AvUlY4bTJPUk5r?=
 =?utf-8?B?TitFM3NhcWFQak15Nkl2NG4vcU1xV1c1blFlaHBRdE1jbTIyMGZOTDNYaVBT?=
 =?utf-8?B?TXRZRE02eHMzaFRqekFmTno3c3VCNHhmY1g4UllmSi9xMlZ4UlI2QnQwZlV4?=
 =?utf-8?B?MFNhT0xJYk1zY1JkYUE3M1RVR0U5RlVvaVJvekdOS2ZWcTYvZGRDN0F6TWZS?=
 =?utf-8?B?RkprZG53N2Z2RzVzdEU1bmFnRE9uQVlKR0dkVlVCL2hXYzVoWjlYQUVoK2Vz?=
 =?utf-8?B?bzVadllBOXNtUzRqTngwQWV3RFdEVW1XZnBlcU9LdFRxTWtqS25iZGNLcE4r?=
 =?utf-8?B?NGtuTFZkVXZaL2RQZE15OUxTZ0dSN1pocWVXM3dtZSt1TDR3ejFFd09ueDF4?=
 =?utf-8?B?c1pkWGsrQm9XempSaXdUcE91d0R0VE9UOGFvUHZvV3NYMHVZallyWnh3N1dT?=
 =?utf-8?B?bEZpSVZWcGkxTWRpNXJaZ3VsVUx5Q1NTeDNqZ2Qzb2hRVW51THdSWGtlbG4v?=
 =?utf-8?B?VGtUNE90eXJyTEtCMm43WGVoenJqWE96NytsdUh6VXJ3RHNObXFvYUFHaURk?=
 =?utf-8?B?WEJTODJZTHBNVDBndnBZcG1PRXJRVnllanBSTE1RSEVocUxuUER1ZVNIWDRQ?=
 =?utf-8?B?aUlmd2pDQ0RlcGNYN0ZTcVdnTEhoYW11ZDdDUlc5aHB0SHVJZS9PcWVGeWR3?=
 =?utf-8?B?Yy95eUFxOExPR2VkNjBGbFJmTWR0aVl0T0MzQStWTzI5eDdzOWhsYzJIclR6?=
 =?utf-8?B?WHY5MmVCczBZOElmY0lmbGhmWmRLUGxHZkNTNGdqSXc0d3pOeUowNEV2S0ov?=
 =?utf-8?B?V2hjSVFnWkpQYmxOMlZjYlZnV0JzajR3alNIamFnT1Z6bXgyeHF3SlpFdEdh?=
 =?utf-8?B?WDNlZWUxcjFsdmVuem1zaGRJa0l2bDVNWE5iU3VDSFFST2ZPbXFZNjE3VUlC?=
 =?utf-8?B?R3NhMUZYTWFoVnRuYXZBZzkwYk55enJiTlhWaHlKS3RyNkxUaTNTVzNkdkZK?=
 =?utf-8?B?WXowZXYzRjYyNk5ORHZucWdzc2doNmc2LzlxdU0weURPRGx4aEdycXVpQ2R0?=
 =?utf-8?B?VkJYa040c0RjWjFZeE9IaVB6ay9DV24vMUhLWWhxNTJ1RGVpQXZUSlVvOXF3?=
 =?utf-8?B?c0dXNVg0TzFyZ01NNFZWNndWQkRTeGh0dG1tSitpMVJLR2llRDdUMHdSRXhu?=
 =?utf-8?B?anRVYStFeDV4NWxoSDhqR2xuSGl2OTlWOHAvL2FsU2ptVllESVI3SXkzY29N?=
 =?utf-8?B?b3VQMHhjZmZTSmxRTUNVMFFMcnRxK1NTdWJxWUFrVFdveGY0Z29YQmRZSFF0?=
 =?utf-8?B?M0RLd25JZzBEek5Wdm9vWjZKcllzMXI2UTRSOFltdmRoK25nS2lZRWhnUDlZ?=
 =?utf-8?B?RGs1ZXR5andrQk83WEEwbnpYRnFLTWh2Q2FLemtzL3BtU3NhdnpLSUVHaFl4?=
 =?utf-8?B?VDF4OVZLa1cxOXkweXd2QnB1MDl1ancrai80K2Y5ak1uSEZxMFk4Z1Qwc1dT?=
 =?utf-8?B?SG9JVHZJQ2FUL2M1TmFZM3AvOFVTemRMdStFczlGOHBJNFdDdDFpeXNuU3dK?=
 =?utf-8?B?c0VtZFFHMGNPbDB2VFdROXlLRlI1Q2tkZUUxVFhJTjhiV1F0Y1J6anJLakJr?=
 =?utf-8?B?dytvcXZKMVdxemIzbXAxQ3pxckZKOGk3eG5lNVlUOGxSQ0I5ME9OTFZreXQ2?=
 =?utf-8?B?ZjBFV0lxZnphQm9QSFlMTGc5UXBFWC9pU1RZZHVHYkxYQy93QVBDcFd5U0wy?=
 =?utf-8?B?bXlIeHYvVDJhQ2ZUc0dLd0hrYVlvUXhhSE8wZWtkbDc2K0VySVZ2b0dGWUpL?=
 =?utf-8?B?UG9DaHlRNTIwY1dNVWVKOGNuWjN4T0xiRjVXMXd2M0haaTduOFpUeTNyb0FL?=
 =?utf-8?B?YS9iMHBiVVlNMFVCcFI2dHFzREQyaGlvR1FQYTdrSnZtNGJ3SXRwZlZscWxN?=
 =?utf-8?B?OURZdkxOS3RFbXJjOGR4eVhGa2djdXpxUVdBOEdPRmRCQngyWG1yYUZqRzJP?=
 =?utf-8?B?ejloOVRjZG1UTVA2Q3BIQ0xscmFGWHJCVnc1dGNFRG1nNUNzcEpIei92eUd1?=
 =?utf-8?B?d3c9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: b46bb980-3d63-4f7d-3470-08db74362df1
X-MS-Exchange-CrossTenant-AuthSource: PH7PR11MB5984.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Jun 2023 22:07:05.2512
 (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: IOV4Oazs+1h5mK9f1aZhTLb1ZrU6CQGK0tsmw6ApbwstxlD13bJPn4lfByzIGOLOjUrbI0PfVx+YAIOgVOGbTg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW3PR11MB4714
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,
        SPF_NONE,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 6/22/23 13:55, Terry Bowman wrote:
> RCH downstream port error logging is missing in the current CXL driver. The
> missing AER and RAS error logging is needed for communicating driver error
> details to userspace. Update the driver to include PCIe AER and CXL RAS
> error logging.
> 
> Add RCH downstream port error handling into the existing RCiEP handler.
> The downstream port error handler is added to the RCiEP error handler
> because the downstream port is implemented in a RCRB, is not PCI
> enumerable, and as a result is not directly accessible to the PCI AER
> root port driver. The AER root port driver calls the RCiEP handler for
> handling RCD errors and RCH downstream port protocol errors.
> 
> Update existing RCiEP correctable and uncorrectable handlers to also call
> the RCH handler. The RCH handler will read the RCH AER registers, check for
> error severity, and if an error exists will log using an existing kernel
> AER trace routine. The RCH handler will also log downstream port RAS errors
> if they exist.
> 
> Co-developed-by: Robert Richter <rrichter@amd.com>
> Signed-off-by: Robert Richter <rrichter@amd.com>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>   drivers/cxl/core/pci.c | 101 +++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 101 insertions(+)
> 
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index 9cb39835e154..9e0eba5ccfc4 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -5,6 +5,7 @@
>   #include <linux/delay.h>
>   #include <linux/pci.h>
>   #include <linux/pci-doe.h>
> +#include <linux/aer.h>
>   #include <cxlpci.h>
>   #include <cxlmem.h>
>   #include <cxl.h>
> @@ -747,10 +748,107 @@ static bool cxl_handle_endpoint_ras(struct cxl_dev_state *cxlds)
>   	return __cxl_handle_ras(cxlds, cxlds->regs.ras);
>   }
>   
> +#ifdef CONFIG_PCIEAER_CXL
> +
> +static void cxl_handle_rdport_cor_ras(struct cxl_dev_state *cxlds,
> +					  struct cxl_dport *dport)
> +{
> +	return __cxl_handle_cor_ras(cxlds, dport->regs.ras);
> +}
> +
> +static bool cxl_handle_rdport_ras(struct cxl_dev_state *cxlds,
> +				       struct cxl_dport *dport)
> +{
> +	return __cxl_handle_ras(cxlds, dport->regs.ras);
> +}
> +
> +/*
> + * Copy the AER capability registers using 32 bit read accesses.
> + * This is necessary because RCRB AER capability is MMIO mapped. Clear the
> + * status after copying.
> + *
> + * @aer_base: base address of AER capability block in RCRB
> + * @aer_regs: destination for copying AER capability
> + */
> +static bool cxl_rch_get_aer_info(void __iomem *aer_base,
> +				 struct aer_capability_regs *aer_regs)
> +{
> +	int read_cnt = sizeof(struct aer_capability_regs) / sizeof(u32);
> +	u32 *aer_regs_buf = (u32 *)aer_regs;
> +	int n;
> +
> +	if (!aer_base)
> +		return false;
> +
> +	/* Use readl() to guarantee 32-bit accesses */
> +	for (n = 0; n < read_cnt; n++)
> +		aer_regs_buf[n] = readl(aer_base + n * sizeof(u32));
> +
> +	writel(aer_regs->uncor_status, aer_base + PCI_ERR_UNCOR_STATUS);
> +	writel(aer_regs->cor_status, aer_base + PCI_ERR_COR_STATUS);
> +
> +	return true;
> +}
> +
> +/* Get AER severity. Return false if there is no error. */
> +static bool cxl_rch_get_aer_severity(struct aer_capability_regs *aer_regs,
> +				     int *severity)
> +{
> +	if (aer_regs->uncor_status & ~aer_regs->uncor_mask) {
> +		if (aer_regs->uncor_status & PCI_ERR_ROOT_FATAL_RCV)
> +			*severity = AER_FATAL;
> +		else
> +			*severity = AER_NONFATAL;
> +		return true;
> +	}
> +
> +	if (aer_regs->cor_status & ~aer_regs->cor_mask) {
> +		*severity = AER_CORRECTABLE;
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
> +static void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds)
> +{
> +	struct pci_dev *pdev = to_pci_dev(cxlds->dev);
> +	struct aer_capability_regs aer_regs;
> +	struct cxl_dport *dport;
> +	struct cxl_port *port;
> +	int severity;
> +
> +	port = cxl_pci_find_port(pdev, &dport);
> +	if (!port)
> +		return;
> +
> +	put_device(&port->dev);
> +
> +	if (!cxl_rch_get_aer_info(dport->regs.dport_aer, &aer_regs))
> +		return;
> +
> +	if (!cxl_rch_get_aer_severity(&aer_regs, &severity))
> +		return;
> +
> +	pci_print_aer(pdev, severity, &aer_regs);
> +
> +	if (severity == AER_CORRECTABLE)
> +		cxl_handle_rdport_cor_ras(cxlds, dport);
> +	else
> +		cxl_handle_rdport_ras(cxlds, dport);
> +}
> +
> +#else
> +static void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds) { }
> +#endif
> +
>   void cxl_cor_error_detected(struct pci_dev *pdev)
>   {
>   	struct cxl_dev_state *cxlds = pci_get_drvdata(pdev);
>   
> +	if (cxlds->rcd)
> +		cxl_handle_rdport_errors(cxlds);
> +
>   	cxl_handle_endpoint_cor_ras(cxlds);
>   }
>   EXPORT_SYMBOL_NS_GPL(cxl_cor_error_detected, CXL);
> @@ -763,6 +861,9 @@ pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
>   	struct device *dev = &cxlmd->dev;
>   	bool ue;
>   
> +	if (cxlds->rcd)
> +		cxl_handle_rdport_errors(cxlds);
> +
>   	/*
>   	 * A frozen channel indicates an impending reset which is fatal to
>   	 * CXL.mem operation, and will likely crash the system. On the off