linux-kernel - Re: [PATCH v8 06/16] CXL/PCI: Introduce CXL uncorrectable protocol error 'recovery'

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250423173540.000034b3@huawei.com>
Date: Wed, 23 Apr 2025 17:35:40 +0100
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Terry Bowman <terry.bowman@....com>
CC: <linux-cxl@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-pci@...r.kernel.org>, <nifan.cxl@...il.com>, <dave@...olabs.net>,
	<dave.jiang@...el.com>, <alison.schofield@...el.com>,
	<vishal.l.verma@...el.com>, <dan.j.williams@...el.com>,
	<bhelgaas@...gle.com>, <mahesh@...ux.ibm.com>, <ira.weiny@...el.com>,
	<oohall@...il.com>, <Benjamin.Cheatham@....com>, <rrichter@....com>,
	<nathan.fontenot@....com>, <Smita.KoralahalliChannabasappa@....com>,
	<lukas@...ner.de>, <ming.li@...omail.com>,
	<PradeepVineshReddy.Kodamati@....com>
Subject: Re: [PATCH v8 06/16] CXL/PCI: Introduce CXL uncorrectable protocol
 error 'recovery'

On Wed, 26 Mar 2025 20:47:07 -0500
Terry Bowman <terry.bowman@....com> wrote:

> Create cxl_do_recovery() to provide uncorrectable protocol error (UCE)
> handling. Follow similar design as found in PCIe error driver,
> pcie_do_recovery(). One difference is that cxl_do_recovery() will treat all
> UCEs as fatal with a kernel panic. This is to prevent corruption on CXL
> memory.
> 
> Copy the PCIe error handlers merge_result(). Introduce PCI_ERS_RESULT_PANIC
> and add support in the merge_result() routine.
> 
> Copy pci_walk_bridge() to cxl_walk_bridge(). Make a change to walk the
> first device in all cases.
> 
> Copy report_error_detected() to cxl_report_error_detected(). Update this
> function to populate the CXL error information structure, 'struct
> cxl_prot_error_info', before calling the device error handler.
> 
> Call panic() to halt the system in the case of uncorrectable errors (UCE)
> in cxl_do_recovery(). Export pci_aer_clear_fatal_status() for CXL to use
> if a UCE is not found. In this case the AER status must be cleared and
> uses pci_aer_clear_fatal_status().
> 
> Signed-off-by: Terry Bowman <terry.bowman@....com>
> ---
>  drivers/cxl/core/ras.c | 92 +++++++++++++++++++++++++++++++++++++++++-
>  drivers/pci/pci.h      |  2 -
>  include/linux/pci.h    |  5 +++
>  3 files changed, 96 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c
> index eca8f11a05d9..1f94fc08e72b 100644
> --- a/drivers/cxl/core/ras.c
> +++ b/drivers/cxl/core/ras.c
> @@ -141,7 +141,97 @@ int cxl_create_prot_err_info(struct pci_dev *_pdev, int severity,
>  }
>  EXPORT_SYMBOL_NS_GPL(cxl_create_prot_err_info, "CXL");
>  
> -static void cxl_do_recovery(struct pci_dev *pdev) { }
> +
> +static pci_ers_result_t merge_result(enum pci_ers_result orig,

Rename perhaps to avoid confusion / grep clashed...

> +				     enum pci_ers_result new)
> +{
> +	if (new == PCI_ERS_RESULT_PANIC)
> +		return PCI_ERS_RESULT_PANIC;
> +
> +	if (new == PCI_ERS_RESULT_NO_AER_DRIVER)
> +		return PCI_ERS_RESULT_NO_AER_DRIVER;
> +
> +	if (new == PCI_ERS_RESULT_NONE)
> +		return orig;
> +
> +	switch (orig) {
> +	case PCI_ERS_RESULT_CAN_RECOVER:
> +	case PCI_ERS_RESULT_RECOVERED:
> +		orig = new;
> +		break;
> +	case PCI_ERS_RESULT_DISCONNECT:
> +		if (new == PCI_ERS_RESULT_NEED_RESET)
> +			orig = PCI_ERS_RESULT_NEED_RESET;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	return orig;
> +}
> +
> +static void cxl_walk_bridge(struct pci_dev *bridge,
> +			    int (*cb)(struct pci_dev *, void *),
> +			    void *userdata)
> +{
> +	if (cb(bridge, userdata))
> +		return;
> +
> +	if (bridge->subordinate)
> +		pci_walk_bus(bridge->subordinate, cb, userdata);
> +}
> +

Trivial but seems there are two blank lines where one will do.

> +
> +static int cxl_report_error_detected(struct pci_dev *pdev, void *data)
> +{
> +	struct cxl_driver *pdrv;
> +	pci_ers_result_t vote, *result = data;
> +	struct cxl_prot_error_info err_info = { 0 };
> +	const struct cxl_error_handlers *cxl_err_handler;
> +
> +	if (cxl_create_prot_err_info(pdev, AER_FATAL, &err_info))
> +		return 0;
> +
> +	struct device *dev __free(put_device) = get_device(err_info.dev);
> +	if (!dev)
> +		return 0;
> +
> +	pdrv = to_cxl_drv(dev->driver);
> +	if (!pdrv || !pdrv->err_handler ||
> +	    !pdrv->err_handler->error_detected)
> +		return 0;
> +
> +	cxl_err_handler = pdrv->err_handler;
> +	vote = cxl_err_handler->error_detected(dev, &err_info);
> +
> +	*result = merge_result(*result, vote);
> +
> +	return 0;
> +}
> +
> +static void cxl_do_recovery(struct pci_dev *pdev)
> +{
> +	struct pci_host_bridge *host = pci_find_host_bridge(pdev->bus);
> +	pci_ers_result_t status = PCI_ERS_RESULT_CAN_RECOVER;
> +
> +	cxl_walk_bridge(pdev, cxl_report_error_detected, &status);
> +	if (status == PCI_ERS_RESULT_PANIC)
> +		panic("CXL cachemem error.");
> +
> +	/*
> +	 * If we have native control of AER, clear error status in the device
> +	 * that detected the error.  If the platform retained control of AER,
> +	 * it is responsible for clearing this status.  In that case, the
> +	 * signaling device may not even be visible to the OS.
> +	 */
> +	if (host->native_aer) {
> +		pcie_clear_device_status(pdev);
> +		pci_aer_clear_nonfatal_status(pdev);
> +		pci_aer_clear_fatal_status(pdev);
> +	}
> +
> +	pci_info(pdev, "CXL uncorrectable error.\n");
> +}
>  
>  static int cxl_rch_handle_error_iter(struct pci_dev *pdev, void *data)
>  {