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: <20251104184522.GA1864503@bhelgaas>
Date: Tue, 4 Nov 2025 12:45:22 -0600
From: Bjorn Helgaas <helgaas@...nel.org>
To: Terry Bowman <terry.bowman@....com>
Cc: dave@...olabs.net, jonathan.cameron@...wei.com, dave.jiang@...el.com,
	alison.schofield@...el.com, dan.j.williams@...el.com,
	bhelgaas@...gle.com, shiju.jose@...wei.com, ming.li@...omail.com,
	Smita.KoralahalliChannabasappa@....com, rrichter@....com,
	dan.carpenter@...aro.org, PradeepVineshReddy.Kodamati@....com,
	lukas@...ner.de, Benjamin.Cheatham@....com,
	sathyanarayanan.kuppuswamy@...ux.intel.com,
	linux-cxl@...r.kernel.org, alucerop@....com, ira.weiny@...el.com,
	linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org
Subject: Re: [RESEND v13 21/25] PCI/AER: Dequeue forwarded CXL error

On Tue, Nov 04, 2025 at 11:03:01AM -0600, Terry Bowman wrote:
> The AER driver now forwards CXL protocol errors to the CXL driver via a
> kfifo. The CXL driver must consume these work items, initiate protocol
> error handling, and ensure RAS mappings remain valid throughout processing.
> 
> Implement cxl_proto_err_work_fn() to dequeue work items forwarded by the
> AER service driver and begin protocol error processing by calling
> cxl_handle_proto_error().
> 
> Add a PCI device lock on &pdev->dev within cxl_proto_err_work_fn() to
> keep the PCI device structure valid during handling. Locking an Endpoint
> will also defer RAS unmapping until the device is unlocked.
> 
> For Endpoints, add a lock on CXL memory device cxlds->dev. The CXL memory
> device structure holds the RAS register reference needed during error
> handling.
> 
> Add lock for the parent CXL Port for Root Ports, Downstream Ports, and
> Upstream Ports to prevent destruction of structures holding mapped RAS
> addresses while they are in use.
> 
> Invoke cxl_do_recovery() for uncorrectable errors. Treat this as a stub for
> now; implement its functionality in a future patch.
> 
> Export pci_clean_device_status() to enable cleanup of AER status following
> error handling.

s/pci_clean_device_status/pcie_clear_device_status/

Acked-by: Bjorn Helgaas <bhelgaas@...gle.com>

>  drivers/cxl/core/ras.c | 153 ++++++++++++++++++++++++++++++++++++++---
>  drivers/pci/pci.c      |   1 +
>  drivers/pci/pci.h      |   1 -
>  include/linux/pci.h    |   2 +

Looks like this is primarily a CXL change, and the PCI part is
minimal, so I question the "PCI/AER:" prefix in the subject.

> +static struct cxl_port *get_cxl_port(struct pci_dev *pdev)
> +{
> +	switch (pci_pcie_type(pdev)) {
> +	case PCI_EXP_TYPE_ROOT_PORT:
> +	case PCI_EXP_TYPE_DOWNSTREAM:
> +	{
> +		struct cxl_dport *dport;
> +		struct cxl_port *port = find_cxl_port(&pdev->dev, &dport);
> +
> +		if (!port) {
> +			pci_err(pdev, "Failed to find the CXL device");
> +			return NULL;
> +		}
> +		return port;
> +	}
> +	case PCI_EXP_TYPE_UPSTREAM:
> +	{
> +		struct cxl_port *port = find_cxl_port_by_uport(&pdev->dev);
> +
> +		if (!port) {
> +			pci_err(pdev, "Failed to find the CXL device");
> +			return NULL;
> +		}
> +		return port;
> +	}
> +	case PCI_EXP_TYPE_ENDPOINT:
> +	{
> +		struct cxl_dev_state *cxlds = pci_get_drvdata(pdev);
> +		struct cxl_port *port = cxlds->cxlmd->endpoint;
> +
> +		get_device(&port->dev);
> +		return port;
> +	}
> +	}
> +	pci_warn_once(pdev, "Error: Unsupported device type (%X)", pci_pcie_type(pdev));

Maybe use "%#x" so it's clear that this is hex?  PCI typically uses
lower-case hex; maybe the CXL convention is different.

> +static void cxl_handle_proto_error(struct cxl_proto_err_work_data *err_info)
> +{
> +	struct pci_dev *pdev = err_info->pdev;
> +	struct cxl_dev_state *cxlds = pci_get_drvdata(pdev);
> +
> +	if (err_info->severity == AER_CORRECTABLE) {
> +
> +		if (pdev->aer_cap)
> +			pci_clear_and_set_config_dword(pdev,
> +						       pdev->aer_cap + PCI_ERR_COR_STATUS,
> +						       0, PCI_ERR_COR_INTERNAL);
> +
> +		if (is_pcie_endpoint(pdev))
> +			cxl_cor_error_detected(&cxlds->cxlmd->dev);
> +		else
> +			cxl_port_cor_error_detected(&pdev->dev);
> +
> +		pcie_clear_device_status(pdev);

The AER clear above and pcie_clear_device_status() require
ownership of the PCIe Capability and the AER Capability, typically
granted by _OSC.

I suppose it's obvious that the OS does own these Capabilities if we
get here, but I'm not familiar with this code.


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ