* Re: [PATCH v18 12/13] PCI/CXL: Mask/Unmask CXL protocol errors
From: Dave Jiang @ 2026-07-20 22:52 UTC (permalink / raw)
To: Terry Bowman, Bjorn Helgaas, Dan Williams, Ira Weiny,
Jonathan Cameron, Len Brown, Rafael J . Wysocki, Robert Richter
Cc: linux-acpi, linux-cxl, linux-doc, linux-kernel, linux-pci,
linuxppc-dev, Alejandro Lucero, Alison Schofield, Ankit Agrawal,
Ard Biesheuvel, Ben Cheatham, Borislav Petkov, Breno Leitao,
Davidlohr Bueso, Fabio M . De Francesco, Gregory Price,
Hanjun Guo, Jonathan Corbet, Kees Cook,
Kuppuswamy Sathyanarayanan, Li Ming, Mahesh J Salgaonkar,
Mauro Carvalho Chehab, Oliver O'Halloran, Shiju Jose,
Shuah Khan, Shuai Xue, Smita Koralahalli, Tony Luck, Vishal Verma
In-Reply-To: <20260717222706.3540281-13-terry.bowman@amd.com>
On 7/17/26 3:27 PM, Terry Bowman wrote:
> CXL protocol errors are not enabled for all CXL devices after boot.
> They must be enabled in order to process CXL protocol errors. Provide
> matching teardown helpers so the masks are restored when a CXL Port
> or dport goes away.
>
> Add pci_aer_mask_internal_errors() as the symmetric counterpart to
> pci_aer_unmask_internal_errors() and export both for the cxl_core
> module.
>
> Introduce cxl_unmask_proto_interrupts() and cxl_mask_proto_interrupts()
> in cxl_core to wrap the PCI helpers with the dev_is_pci() and
> pcie_aer_is_native() gating CXL needs. Both helpers tolerate a NULL
> or non-PCI @dev so callers do not have to special-case it.
>
> Wire cxl_unmask_proto_interrupts() into the success path of
> cxl_dport_map_ras() and devm_cxl_port_ras_setup() so the unmask
> only runs when the RAS register block was actually mapped. Pair each
> unmask with a devm_add_action_or_reset() registration of
> cxl_mask_proto_irqs() scoped to the host device so the mask is
> restored when devres is released. This applies to dports, Endpoints,
> Upstream Switch Ports, Downstream Switch Ports, and Root Ports.
>
> Remove the dev_is_pci(dport->dport_dev) guard in
> devm_cxl_dport_rch_ras_setup(). On RCH systems dport->dport_dev is the
> pci_host_bridge device, which is not on pci_bus_type, so this guard
> caused the function to return early on real hardware without mapping
> dport RAS or AER registers. The caller already gates on dport->rch,
> which is sufficient to exclude cxl_test mock devices.
>
> Co-developed-by: Dan Williams <djbw@kernel.org>
> Signed-off-by: Dan Williams <djbw@kernel.org>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>
> ---
>
> Changes in v17->v18:
> - Make cxl_unmask_proto_interrupts() and cxl_mask_proto_interrupts() static
> - Remove dev_is_pci() guard from devm_cxl_dport_rch_ras_setup(); the guard
> blocked real RCH hardware because pci_host_bridge is not on pci_bus_type
>
> Changes in v16->v17:
> - Drop redundant cxl_mask_proto_interrupts() calls from unregister_port()
> and cxl_dport_remove(); the devres action registered alongside the unmask
> is the sole mask path.
> - Update title
> - Remove unnecessary check for aer_capabilities
> - Gate cxl_unmask_proto_interrupts() on pcie_aer_is_native()
> - Add pci_aer_mask_internal_errors() and cxl_mask_proto_interrupts()
> - Only unmask on successful cxl_map_component_regs()
> - NULL-check @dev in cxl_{un,}mask_proto_interrupts()
> - Drop static and declare in core/core.h
>
> Change in v15 -> v16:
> - None
>
> Change in v14 -> v15:
> - None
>
> Changes in v13->v14:
> - Update commit title's prefix (Bjorn)
>
> Changes in v12->v13:
> - Add dev and dev_is_pci() NULL checks in cxl_unmask_proto_interrupts() (Terry)
> - Add Dave Jiang's and Ben's review-by
>
> Changes in v11->v12:
> - None
> ---
> drivers/cxl/core/ras.c | 73 +++++++++++++++++++++++++++++++----
> drivers/pci/pcie/aer.c | 28 ++++++++++++--
> include/linux/aer.h | 2 +
> tools/testing/cxl/Kbuild | 1 +
> tools/testing/cxl/test/mock.c | 12 ++++++
> 5 files changed, 105 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c
> index 69b320c74469c..d77208af41e03 100644
> --- a/drivers/cxl/core/ras.c
> +++ b/drivers/cxl/core/ras.c
> @@ -117,16 +117,64 @@ static void cxl_cper_prot_err_work_fn(struct work_struct *work)
> }
> static DECLARE_WORK(cxl_cper_prot_err_work, cxl_cper_prot_err_work_fn);
>
> +static void cxl_unmask_proto_interrupts(struct device *dev)
> +{
> + struct pci_dev *pdev;
> +
> + if (!dev || !dev_is_pci(dev))
> + return;
> +
> + pdev = to_pci_dev(dev);
> + if (!pcie_aer_is_native(pdev))
> + return;
> +
> + pci_aer_unmask_internal_errors(pdev);
> +}
> +
> +static void cxl_mask_proto_interrupts(struct device *dev)
> +{
> + struct pci_dev *pdev;
> +
> + if (!dev || !dev_is_pci(dev))
> + return;
> +
> + pdev = to_pci_dev(dev);
> + if (!pcie_aer_is_native(pdev))
> + return;
> +
> + pci_aer_mask_internal_errors(pdev);
> +}
> +
> +static void cxl_mask_proto_irqs(void *dev)
> +{
> + cxl_mask_proto_interrupts(dev);
> +}
> +
> static void cxl_dport_map_ras(struct cxl_dport *dport)
> {
> struct cxl_register_map *map = &dport->reg_map;
> struct device *dev = dport->dport_dev;
>
> - if (!map->component_map.ras.valid)
> + if (!map->component_map.ras.valid) {
> dev_dbg(dev, "RAS registers not found\n");
> - else if (cxl_map_component_regs(map, &dport->regs.component,
> - BIT(CXL_CM_CAP_CAP_ID_RAS)))
> + return;
> + }
> +
> + if (cxl_map_component_regs(map, &dport->regs.component,
> + BIT(CXL_CM_CAP_CAP_ID_RAS))) {
> dev_dbg(dev, "Failed to map RAS capability.\n");
> + return;
> + }
> +
> + if (!dev_is_pci(dev))
> + return;
> +
> + cxl_unmask_proto_interrupts(dev);
> + if (devm_add_action_or_reset(dport_to_host(dport),
> + cxl_mask_proto_irqs, dev)) {
> + dev_warn(dev, "failed to defer CXL proto-irq mask; CXL protocol error reporting disabled\n");
> + dport->regs.component.ras = NULL;
> + }
> }
>
> /**
> @@ -143,9 +191,6 @@ void devm_cxl_dport_rch_ras_setup(struct cxl_dport *dport)
> {
> struct pci_host_bridge *host_bridge;
>
> - if (!dev_is_pci(dport->dport_dev))
> - return;
> -
> devm_cxl_dport_ras_setup(dport);
>
> host_bridge = to_pci_host_bridge(dport->dport_dev);
> @@ -160,6 +205,7 @@ EXPORT_SYMBOL_NS_GPL(devm_cxl_dport_rch_ras_setup, "CXL");
> void devm_cxl_port_ras_setup(struct cxl_port *port)
> {
> struct cxl_register_map *map = &port->reg_map;
> + struct device *dev;
>
> if (!map->component_map.ras.valid) {
> dev_dbg(&port->dev, "RAS registers not found\n");
> @@ -168,8 +214,21 @@ void devm_cxl_port_ras_setup(struct cxl_port *port)
>
> map->host = &port->dev;
> if (cxl_map_component_regs(map, &port->regs,
> - BIT(CXL_CM_CAP_CAP_ID_RAS)))
> + BIT(CXL_CM_CAP_CAP_ID_RAS))) {
> dev_dbg(&port->dev, "Failed to map RAS capability\n");
> + return;
> + }
> +
> + dev = is_cxl_endpoint(port) ? port->uport_dev->parent : port->uport_dev;
> + if (!dev_is_pci(dev))
> + return;
> +
> + cxl_unmask_proto_interrupts(dev);
> + if (devm_add_action_or_reset(&port->dev, cxl_mask_proto_irqs, dev)) {
> + dev_warn(&port->dev,
> + "failed to defer CXL proto-irq mask; CXL protocol error reporting disabled\n");
> + port->regs.ras = NULL;
> + }
> }
> EXPORT_SYMBOL_NS_GPL(devm_cxl_port_ras_setup, "CXL");
>
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 0bd23a65e7ebc..be6dc2cbd4491 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -1143,12 +1143,32 @@ void pci_aer_unmask_internal_errors(struct pci_dev *dev)
> mask &= ~PCI_ERR_COR_INTERNAL;
> pci_write_config_dword(dev, aer + PCI_ERR_COR_MASK, mask);
> }
> +EXPORT_SYMBOL_FOR_MODULES(pci_aer_unmask_internal_errors, "cxl_core");
>
> -/*
> - * Internal errors are too device-specific to enable generally, however for CXL
> - * their behavior is standardized for conveying CXL protocol errors.
> +/**
> + * pci_aer_mask_internal_errors - mask internal errors
> + * @dev: pointer to the pci_dev data structure
> + *
> + * Mask internal errors in the Uncorrectable and Correctable Error
> + * Mask registers.
> + *
> + * Note: AER must be enabled and supported by the device which must be
> + * checked in advance, e.g. with pcie_aer_is_native().
> */
> -EXPORT_SYMBOL_FOR_MODULES(pci_aer_unmask_internal_errors, "cxl_core");
> +void pci_aer_mask_internal_errors(struct pci_dev *dev)
> +{
> + int aer = dev->aer_cap;
> + u32 mask;
> +
> + pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_MASK, &mask);
> + mask |= PCI_ERR_UNC_INTN;
> + pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_MASK, mask);
> +
> + pci_read_config_dword(dev, aer + PCI_ERR_COR_MASK, &mask);
> + mask |= PCI_ERR_COR_INTERNAL;
> + pci_write_config_dword(dev, aer + PCI_ERR_COR_MASK, mask);
> +}
> +EXPORT_SYMBOL_FOR_MODULES(pci_aer_mask_internal_errors, "cxl_core");
>
> /**
> * pci_aer_handle_error - handle logging error into an event log
> diff --git a/include/linux/aer.h b/include/linux/aer.h
> index 8eba3192e2d15..b3657b80564b9 100644
> --- a/include/linux/aer.h
> +++ b/include/linux/aer.h
> @@ -58,6 +58,7 @@ struct aer_capability_regs {
> int pci_aer_clear_nonfatal_status(struct pci_dev *dev);
> int pcie_aer_is_native(struct pci_dev *dev);
> void pci_aer_unmask_internal_errors(struct pci_dev *dev);
> +void pci_aer_mask_internal_errors(struct pci_dev *dev);
> #else
> static inline int pci_aer_clear_nonfatal_status(struct pci_dev *dev)
> {
> @@ -65,6 +66,7 @@ static inline int pci_aer_clear_nonfatal_status(struct pci_dev *dev)
> }
> static inline int pcie_aer_is_native(struct pci_dev *dev) { return 0; }
> static inline void pci_aer_unmask_internal_errors(struct pci_dev *dev) { }
> +static inline void pci_aer_mask_internal_errors(struct pci_dev *dev) { }
> #endif
>
> #ifdef CONFIG_CXL_RAS
> diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild
> index 2be1df80fcc93..957945201f04d 100644
> --- a/tools/testing/cxl/Kbuild
> +++ b/tools/testing/cxl/Kbuild
> @@ -6,6 +6,7 @@ ldflags-y += --wrap=acpi_pci_find_root
> ldflags-y += --wrap=nvdimm_bus_register
> ldflags-y += --wrap=cxl_await_media_ready
> ldflags-y += --wrap=devm_cxl_add_rch_dport
> +ldflags-y += --wrap=devm_cxl_dport_rch_ras_setup
> ldflags-y += --wrap=cxl_endpoint_parse_cdat
> ldflags-y += --wrap=devm_cxl_endpoint_decoders_setup
> ldflags-y += --wrap=hmat_get_extended_linear_cache_size
> diff --git a/tools/testing/cxl/test/mock.c b/tools/testing/cxl/test/mock.c
> index 6454b868b122c..5ad3243da8d29 100644
> --- a/tools/testing/cxl/test/mock.c
> +++ b/tools/testing/cxl/test/mock.c
> @@ -220,6 +220,18 @@ struct cxl_dport *__wrap_devm_cxl_add_rch_dport(struct cxl_port *port,
> }
> EXPORT_SYMBOL_NS_GPL(__wrap_devm_cxl_add_rch_dport, "CXL");
>
> +void __wrap_devm_cxl_dport_rch_ras_setup(struct cxl_dport *dport)
> +{
> + int index;
> + struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
> +
> + if (!ops || !ops->is_mock_port(dport->dport_dev))
> + devm_cxl_dport_rch_ras_setup(dport);
> +
> + put_cxl_mock_ops(index);
> +}
> +EXPORT_SYMBOL_NS_GPL(__wrap_devm_cxl_dport_rch_ras_setup, "CXL");
> +
> void __wrap_cxl_endpoint_parse_cdat(struct cxl_port *port)
> {
> int index;
^ permalink raw reply
* Re: [PATCH v18 06/13] PCI: Establish common CXL Port protocol error flow
From: Jonathan Cameron @ 2026-07-20 23:05 UTC (permalink / raw)
To: Terry Bowman
Cc: Bjorn Helgaas, Dan Williams, Dave Jiang, Ira Weiny, Len Brown,
Rafael J . Wysocki, Robert Richter, linux-acpi, linux-cxl,
linux-doc, linux-kernel, linux-pci, linuxppc-dev,
Alejandro Lucero, Alison Schofield, Ankit Agrawal, Ard Biesheuvel,
Ben Cheatham, Borislav Petkov, Breno Leitao, Davidlohr Bueso,
Fabio M . De Francesco, Gregory Price, Hanjun Guo,
Jonathan Corbet, Kees Cook, Kuppuswamy Sathyanarayanan, Li Ming,
Mahesh J Salgaonkar, Mauro Carvalho Chehab, Oliver O'Halloran,
Shuah Khan, Shuai Xue, Smita Koralahalli, Tony Luck, Vishal Verma
In-Reply-To: <20260717222706.3540281-7-terry.bowman@amd.com>
On Fri, 17 Jul 2026 17:26:59 -0500
Terry Bowman <terry.bowman@amd.com> wrote:
> Add CXL protocol error dispatch in handle_error_source() using
> is_cxl_error() and cxl_forward_error() to route errors through the
> AER-CXL kfifo. Expand is_cxl_error() from Endpoint-only to include
> Root Port, Upstream Port, and Downstream Port device types. The
> producer and consumer go live in the same commit to avoid silently
> dropping CXL errors during bisect.
Take a stab at making this description a lot more concise. I'd like
to just be seeing a very brief summary of flow + clearly highlighting
of potentially controversial choices.
>
> For uncorrectable events, call cxl_proto_err_flush() to ensure CXL RAS
> registers are read, panic policy is applied, and CXL state is cleared
> before pci_aer_handle_error() drives PCIe recovery. Without the flush,
> AER recovery can tear down drivers and unmap the CXL RAS iomaps while
> the kfifo consumer is still reading them. Correctable events do not
> need the flush and run asynchronously. RCH kfifo support is added in
> the following patch ("PCI/CXL: Add RCH support to CXL handlers").
>
> Add cxl_handle_proto_error() to dispatch correctable and uncorrectable
> errors through the CXL RAS helpers. Add cxl_do_recovery() to coordinate
> uncorrectable recovery. Panic when a UCE is confirmed by a successful
> CXL RAS status register read. If the RAS registers cannot be read the
> UCE cannot be confirmed and panic is not triggered. Gate error handling
> on the port driver being bound to avoid processing errors on disabled
> devices.
>
> The kfifo consumer holds guard(device)(&port->dev) and checks
> port->dev.driver before accessing RAS registers, serializing against
> driver unbind and devm iomap teardown. For UCE, cxl_proto_err_flush()
> runs the worker synchronously before AER recovery, ensuring the device
> is present during RAS register access.
>
> Add to_ras_base() to centralize RAS base lookup: dport->regs.ras for
> Root/Downstream Ports, port->regs.ras for Upstream Ports and Endpoints.
> Use to_ras_base() to access the CXL devices' RAS registers as it will
> provide an avenue to inject status simulation during testing.
>
> Add CXL RAS logging in cxl_handle_cor_ras() and cxl_handle_ras(). The
> existing cxl_cor_error_detected() and cxl_error_detected() AER
> callbacks remain for all Endpoints and are reworked to use
> find_cxl_port_by_uport() and to_ras_base(), with UCE now triggering
> panic unconditionally. These callbacks are further updated in the
> following patch ("PCI/CXL: Add RCH support to CXL handlers").
>
> Fix a pre-existing race for cxlds between cxl_handle_rdport_errors() and
> cxl_memdev_shutdown() by holding a cxlmd device scoped_guard() around
> the rdport call. Release the lock before taking the Port lock to avoid
> the lock inversion.
>
> Co-developed-by: Dan Williams <djbw@kernel.org>
> Signed-off-by: Dan Williams <djbw@kernel.org>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
>
A few minor things about the code inline.
Thanks,
Jonathan
> diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c
> index 135f1997e6f4f..b190e69c2d415 100644
> --- a/drivers/cxl/core/ras.c
> +++ b/drivers/cxl/core/ras.c
> @@ -77,6 +77,36 @@ static int match_memdev_by_parent(struct device *dev, const void *uport)
> return 0;
> }
>
> +
> +/**
> + * find_cxl_port_by_dev - Use @dev as hint to do a _by_dport or _by_uport lookup
> + * @dev: generic device that may either be a companion of port or target dport
> + * @dport: output parameter; set to the matched dport for dport-class
I'd state this is optional.
> + * lookups (Root Port, Downstream Port), NULL otherwise.
> + *
> + * Return a 'struct cxl_port' with an elevated reference if found. Use
> + * __free(put_cxl_port) to release.
> + */
> +static struct cxl_port *find_cxl_port_by_dev(struct device *dev, struct cxl_dport **dport)
> +{
> + if (dport)
> + *dport = NULL;
> + if (!dev_is_pci(dev))
> + return NULL;
> +
> + switch (pci_pcie_type(to_pci_dev(dev))) {
> + case PCI_EXP_TYPE_ROOT_PORT:
> + case PCI_EXP_TYPE_DOWNSTREAM:
> + return find_cxl_port_by_dport(dev, dport);
> + case PCI_EXP_TYPE_UPSTREAM:
> + case PCI_EXP_TYPE_ENDPOINT:
> + case PCI_EXP_TYPE_RC_END:
> + return find_cxl_port_by_uport(dev);
> + }
> +
> + return NULL;
> +}
...
> @@ -270,22 +326,32 @@ bool cxl_handle_ras(struct device *dev, void __iomem *ras_base)
>
> void cxl_cor_error_detected(struct pci_dev *pdev)
> {
> - struct cxl_dev_state *cxlds = pci_get_drvdata(pdev);
> - struct cxl_memdev *cxlmd = cxlds->cxlmd;
> - struct device *dev = &cxlds->cxlmd->dev;
> + guard(device)(&pdev->dev);
> + if (!pdev->dev.driver)
> + return;
> +
> + struct cxl_port *port __free(put_cxl_port) = find_cxl_port_by_uport(&pdev->dev);
> + if (!port)
> + return;
> +
> + if (is_cxl_restricted(pdev)) {
> + struct cxl_dev_state *cxlds = pci_get_drvdata(pdev);
> + struct cxl_memdev *cxlmd = cxlds->cxlmd;
>
> - scoped_guard(device, dev) {
> - if (!dev->driver) {
> + scoped_guard(device, &cxlmd->dev) {
> + cxl_handle_rdport_errors(cxlds);
> + }
Unless this gets more complex later you can just use a guard() as exits
scope here anyway.
> + }
> +
> + scoped_guard(device, &port->dev) {
Similar here. I haven't read on though for this one so if it's needed
later just ignore me.
> + if (!port->dev.driver) {
> dev_warn(&pdev->dev,
> - "%s: memdev disabled, abort error handling\n",
> - dev_name(dev));
> + "%s: port disabled, abort error handling\n",
> + dev_name(&port->dev));
> return;
> }
>
> - if (cxlds->rcd)
> - cxl_handle_rdport_errors(cxlds);
> -
> - cxl_handle_cor_ras(&cxlds->cxlmd->dev, cxlmd->endpoint->regs.ras);
> + cxl_handle_cor_ras(port->uport_dev, to_ras_base(port, NULL));
> }
> }
> EXPORT_SYMBOL_NS_GPL(cxl_cor_error_detected, "CXL");
> @@ -293,42 +359,53 @@ EXPORT_SYMBOL_NS_GPL(cxl_cor_error_detected, "CXL");
> pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
> pci_channel_state_t state)
> {
> - struct cxl_dev_state *cxlds = pci_get_drvdata(pdev);
...
>
> + /*
> + * CXL.mem UCE means cache coherency is lost. Continuing risks
> + * silent data corruption across interleaved HDM regions.
It is a problem whether interleave or not. So maybe just stop
at corruption.
> + */
> + if (ue)
> + panic("CXL cachemem error");
...
> @@ -338,3 +415,67 @@ pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
...
> +static void __cxl_proto_err_work_fn(struct cxl_proto_err_work_data *wd)
> +{
> + struct cxl_port *port __free(put_cxl_port) = find_cxl_port_by_dev(&wd->pdev->dev, NULL);
> + if (!port) {
> + dev_err_ratelimited(&wd->pdev->dev,
> + "Failed to find parent port device in CXL topology\n");
> + return;
> + }
I'd put a blank line here.
> + guard(device)(&port->dev);
> + if (!port->dev.driver) {
> + dev_err_ratelimited(&port->dev,
> + "Port device is unbound, abort error handling\n");
> + return;
> + }
> +
> + struct cxl_dport *dport = cxl_find_dport_by_dev(port, &wd->pdev->dev);
> + if (!dport && (pci_pcie_type(wd->pdev) == PCI_EXP_TYPE_ROOT_PORT ||
> + pci_pcie_type(wd->pdev) == PCI_EXP_TYPE_DOWNSTREAM)) {
> + dev_err_ratelimited(&wd->pdev->dev,
> + "Failed to find dport device in CXL topology\n");
> + return;
> + }
> +
> + cxl_handle_proto_error(wd->pdev, port, dport, wd->severity);
> +}
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index c5bce25df51cb..2d9d40528e709 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -1185,7 +1185,20 @@ static void pci_aer_handle_error(struct pci_dev *dev, struct aer_err_info *info)
>
> static void handle_error_source(struct pci_dev *dev, struct aer_err_info *info)
> {
> + bool cxl_pending = false;
> +
> cxl_rch_handle_error(dev, info);
> +
> + if (is_cxl_error(dev, info))
> + cxl_pending |= cxl_forward_error(dev, info);
> +
> + /*
> + * Wait for UCE CXL work to complete before AER recovery
> + * tears down the device. CE can run asynchronously.
> + */
> + if (cxl_pending && info->severity != AER_CORRECTABLE)
> + cxl_proto_err_flush();
This kind of makes me wonder if the naming of _flush() is clear.
Normally I'd kind of expect that to be dumping any queued up errors.
Perhaps cxl_proto_err_wait_for_empty() or something like that?
I guess flush_work() set the precedent so I'm too later on this one!
Jonathan
^ permalink raw reply
* Re: [PATCH v18 07/13] PCI/CXL: Add RCH support to CXL handlers
From: Jonathan Cameron @ 2026-07-20 23:12 UTC (permalink / raw)
To: Terry Bowman
Cc: Bjorn Helgaas, Dan Williams, Dave Jiang, Ira Weiny, Len Brown,
Rafael J . Wysocki, Robert Richter, linux-acpi, linux-cxl,
linux-doc, linux-kernel, linux-pci, linuxppc-dev,
Alejandro Lucero, Alison Schofield, Ankit Agrawal, Ard Biesheuvel,
Ben Cheatham, Borislav Petkov, Breno Leitao, Davidlohr Bueso,
Fabio M . De Francesco, Gregory Price, Hanjun Guo,
Jonathan Corbet, Kees Cook, Kuppuswamy Sathyanarayanan, Li Ming,
Mahesh J Salgaonkar, Mauro Carvalho Chehab, Oliver O'Halloran,
Shiju Jose, Shuah Khan, Shuai Xue, Smita Koralahalli, Tony Luck,
Vishal Verma
In-Reply-To: <20260717222706.3540281-8-terry.bowman@amd.com>
On Fri, 17 Jul 2026 17:27:00 -0500
Terry Bowman <terry.bowman@amd.com> wrote:
> Restricted CXL Host (RCH) error handling is a separate path from the
> new CXL Port error handling flow. Fold RCH error handling into the
> Port flow so both share a common entry point.
>
> Update cxl_rch_handle_error_iter() to forward RCH protocol errors
> through the AER-CXL kfifo. Change cxl_rch_handle_error() return type
> from void to bool so handle_error_source() can determine whether work
> was enqueued and call cxl_proto_err_flush() before AER recovery
> proceeds.
>
> For RC_END devices, __cxl_proto_err_work_fn() calls
Can we refer to these as RCiEPs to match the spec? I have no idea
where RC_END naming came from but to me it isn't obviously that same
thing. Or talk about the narrow case of RCDs here as well.
> cxl_handle_rdport_errors() to process RCH Downstream Port errors,
> then falls through to the VH path for RC_END Endpoint handling.
>
> An RCD uncorrectable CXL RAS error now panics via cxl_do_recovery().
> Before this patch the RCH Downstream Port UCE path called
> cxl_handle_ras() but ignored its return value - no panic. After this
> patch the same condition calls cxl_do_recovery() which panics on
> confirmed UCE. The Endpoint UCE path already panicked at the parent
> commit. This matches the panic policy added in the common CXL Port
> protocol error flow.
>
> Remove cxl_cor_error_detected() and its .cor_error_detected
> registration in cxl_error_handlers. Correctable Endpoint errors are
> now routed through the AER-CXL kfifo like all other CXL protocol
> errors.
>
> Drop the cxlds->rcd / cxl_handle_rdport_errors(cxlds) branches from
> cxl_error_detected(). RCH downstream port error handling is now
> performed by __cxl_proto_err_work_fn() via the kfifo path, which
> calls cxl_handle_rdport_errors(pdev) before the common dispatch.
>
> Change cxl_handle_rdport_errors() to take a struct pci_dev * instead
> of a struct cxl_dev_state *, matching the new caller context. Re-fetch
> dport under guard() to close the TOCTOU window between
> cxl_pci_find_port()'s lockless xa_load() and the first dereference of
> the returned pointer.
>
> Change find_cxl_port_by_dev() RC_END lookup from
> find_cxl_port_by_dport(dev->parent) to find_cxl_port_by_uport(dev),
> matching the Endpoint lookup path. RC_END Endpoint port resolution
> uses the uport (the RC_END device itself), while the separate RCH
> Downstream Port lookup is handled by cxl_handle_rdport_errors().
>
> The RCH Downstream Port and the RCD Endpoint (RC_END) are separate
> devices with independent RAS register blocks. cxl_handle_rdport_errors()
> handles the RCH Downstream Port RAS. RCD Endpoint (RC_END) is handled in
> cxl_handle_proto_error().
>
> Use to_ras_base() in cxl_handle_rdport_errors() instead of referencing
> dport->regs.ras directly. Make to_ras_base() non-static in ras.c and
> declare it in core.h so ras_rch.c can access it. Route all RAS base address lookups
> through a single helper to prepare for CXL RAS error injection testing
> that follows this series.
>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
>
Other than taking another look at the patch description and seeing
if it can be more concise, this one looks fine to me.
Jonathan
^ permalink raw reply
* [PATCH] powerpc/ps3: Use cpu_relax() in ps3_create_spu()
From: Thorsten Blum @ 2026-07-20 23:11 UTC (permalink / raw)
To: Geoff Levand, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP)
Cc: Thorsten Blum, linuxppc-dev, linux-kernel
Use cpu_relax() to wait for the execution status SPE_EX_STATE_EXECUTED.
Drop the comments while at it.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
arch/powerpc/platforms/ps3/spu.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/platforms/ps3/spu.c b/arch/powerpc/platforms/ps3/spu.c
index e4e0b45e1b9d..8545c72385de 100644
--- a/arch/powerpc/platforms/ps3/spu.c
+++ b/arch/powerpc/platforms/ps3/spu.c
@@ -13,6 +13,7 @@
#include <linux/export.h>
#include <linux/io.h>
#include <linux/mm.h>
+#include <linux/processor.h>
#include <asm/spu.h>
#include <asm/spu_priv1.h>
@@ -363,12 +364,9 @@ static int __init ps3_create_spu(struct spu *spu, void *data)
if (result)
goto fail_enable;
- /* Make sure the spu is in SPE_EX_STATE_EXECUTED. */
-
- /* need something better here!!! */
- while (in_be64(&spu_pdata(spu)->shadow->spe_execution_status)
- != SPE_EX_STATE_EXECUTED)
- (void)0;
+ while (in_be64(&spu_pdata(spu)->shadow->spe_execution_status) !=
+ SPE_EX_STATE_EXECUTED)
+ cpu_relax();
return result;
^ permalink raw reply related
* [PATCH] powerpc/powermac: Simplify bootx_scan_dt_build_struct()
From: Thorsten Blum @ 2026-07-20 23:14 UTC (permalink / raw)
To: Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP)
Cc: Thorsten Blum, linuxppc-dev, linux-kernel
Assign the empty string directly instead of NULL checking namep again.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
arch/powerpc/platforms/powermac/bootx_init.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/powermac/bootx_init.c b/arch/powerpc/platforms/powermac/bootx_init.c
index 72eb99aba40f..abceae91dfde 100644
--- a/arch/powerpc/platforms/powermac/bootx_init.c
+++ b/arch/powerpc/platforms/powermac/bootx_init.c
@@ -284,9 +284,7 @@ static void __init bootx_scan_dt_build_struct(unsigned long base,
dt_push_token(OF_DT_BEGIN_NODE, mem_end);
/* get the node's full name */
- namep = np->full_name ? (char *)(base + np->full_name) : NULL;
- if (namep == NULL)
- namep = "";
+ namep = np->full_name ? (char *)(base + np->full_name) : "";
l = strlen(namep);
DBG("* struct: %s\n", namep);
^ permalink raw reply related
* Re: [PATCH v18 08/13] cxl/pci: Thread port and dport through RAS handling helpers
From: Jonathan Cameron @ 2026-07-20 23:17 UTC (permalink / raw)
To: Terry Bowman
Cc: Bjorn Helgaas, Dan Williams, Dave Jiang, Ira Weiny, Len Brown,
Rafael J . Wysocki, Robert Richter, linux-acpi, linux-cxl,
linux-doc, linux-kernel, linux-pci, linuxppc-dev,
Alejandro Lucero, Alison Schofield, Ankit Agrawal, Ard Biesheuvel,
Ben Cheatham, Borislav Petkov, Breno Leitao, Davidlohr Bueso,
Fabio M . De Francesco, Gregory Price, Hanjun Guo,
Jonathan Corbet, Kees Cook, Kuppuswamy Sathyanarayanan, Li Ming,
Mahesh J Salgaonkar, Mauro Carvalho Chehab, Oliver O'Halloran,
Shiju Jose, Shuah Khan, Shuai Xue, Smita Koralahalli, Tony Luck,
Vishal Verma
In-Reply-To: <20260717222706.3540281-9-terry.bowman@amd.com>
On Fri, 17 Jul 2026 17:27:01 -0500
Terry Bowman <terry.bowman@amd.com> wrote:
> From: Dan Williams <djbw@kernel.org>
>
> The callers of cxl_handle_ras() and cxl_handle_cor_ras() already hold
> a struct cxl_port * and struct cxl_dport * for the device being
> handled.
It seems unlikely that all have both of them given you have handling
in the code for dport == NULL. Perhaps reword.
> Passing a generic struct device * requires is_cxl_memdev()
> to distinguish Endpoints from ports at trace emission time. Threading
> port and dport directly enables is_cxl_endpoint(port) and explicit
> dport/port branching for cleaner trace dispatch.
>
> Refactor cxl_handle_ras() and cxl_handle_cor_ras() to accept struct
> cxl_port * and struct cxl_dport * directly. The CXL RAS trace event
> emission logic is split into three branches: Endpoint events are
> identified via is_cxl_endpoint(port) and emit with the memdev, dport
> events emit with dport->dport_dev, and Upstream Port events fall back
> to port->uport_dev.
>
> Update cxl_handle_rdport_errors() in ras_rch.c and
> cxl_handle_proto_error() in ras.c to pass port and dport to the
> refactored functions.
Definitely no need to say which files they are in. If we care we can
look at the patch.
>
> RCH Downstream Port correctable trace events now report the dport
> device (dport->dport_dev) as a consequence of threading port and dport
> through the RAS helpers. The following trace event rework ("cxl: Add
> port and dport identifiers to CXL AER trace events") adds explicit
> memdev, port, dport, and host fields that provide full context for
> all device types.
>
> Co-developed-by: Terry Bowman <terry.bowman@amd.com>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> Signed-off-by: Dan Williams <djbw@kernel.org>
>
> ---
>
> Changes in v17 -> v18:
> - New patch.
> ---
> drivers/cxl/core/core.h | 12 ++++++++----
> drivers/cxl/core/ras.c | 29 +++++++++++++++--------------
> drivers/cxl/core/ras_rch.c | 2 +-
> 3 files changed, 24 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> index 272634ff2615b..5ca1275fd8f35 100644
> --- a/drivers/cxl/core/core.h
> +++ b/drivers/cxl/core/core.h
> @@ -185,10 +185,12 @@ static inline struct device *dport_to_host(struct cxl_dport *dport)
> #ifdef CONFIG_CXL_RAS
> void cxl_ras_init(void);
> void cxl_ras_exit(void);
> -bool cxl_handle_ras(struct device *dev, void __iomem *ras_base);
> +bool cxl_handle_ras(struct cxl_port *port, struct cxl_dport *dport,
> + void __iomem *ras_base);
> void cxl_do_recovery(struct pci_dev *pdev, struct cxl_port *port,
> struct cxl_dport *dport);
> -void cxl_handle_cor_ras(struct device *dev, void __iomem *ras_base);
> +void cxl_handle_cor_ras(struct cxl_port *port, struct cxl_dport *dport,
> + void __iomem *ras_base);
> void cxl_dport_map_rch_aer(struct cxl_dport *dport);
> void cxl_disable_rch_root_ints(struct cxl_dport *dport);
> void cxl_handle_rdport_errors(struct pci_dev *pdev);
> @@ -197,13 +199,15 @@ void devm_cxl_dport_ras_setup(struct cxl_dport *dport);
> #else
> static inline void cxl_ras_init(void) { }
> static inline void cxl_ras_exit(void) { }
> -static inline bool cxl_handle_ras(struct device *dev, void __iomem *ras_base)
> +static inline bool cxl_handle_ras(struct cxl_port *port, struct cxl_dport *dport,
> + void __iomem *ras_base)
> {
> return false;
> }
> static inline void cxl_do_recovery(struct pci_dev *pdev, struct cxl_port *port,
> struct cxl_dport *dport) { }
> -static inline void cxl_handle_cor_ras(struct device *dev, void __iomem *ras_base) { }
> +static inline void cxl_handle_cor_ras(struct cxl_port *port, struct cxl_dport *dport,
> + void __iomem *ras_base) { }
> static inline void cxl_dport_map_rch_aer(struct cxl_dport *dport) { }
> static inline void cxl_disable_rch_root_ints(struct cxl_dport *dport) { }
> static inline void cxl_handle_rdport_errors(struct pci_dev *pdev) { }
> diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c
> index 9a142abcf4f8b..6f4a3c1b0bb85 100644
> --- a/drivers/cxl/core/ras.c
> +++ b/drivers/cxl/core/ras.c
> @@ -232,7 +232,6 @@ void __iomem *to_ras_base(struct cxl_port *port, struct cxl_dport *dport)
>
> void cxl_do_recovery(struct pci_dev *pdev, struct cxl_port *port, struct cxl_dport *dport)
> {
> - struct device *dev = dport ? dport->dport_dev : port->uport_dev;
> void __iomem *ras_base = to_ras_base(port, dport);
>
> if (!ras_base) {
> @@ -241,14 +240,14 @@ void cxl_do_recovery(struct pci_dev *pdev, struct cxl_port *port, struct cxl_dpo
> return;
> }
>
> - if (cxl_handle_ras(dev, ras_base))
> + if (cxl_handle_ras(port, dport, ras_base))
> panic("CXL cachemem error");
>
> dev_dbg(&pdev->dev,
> "CXL UCE signaled but no CXL RAS status bits set\n");
> }
>
> -void cxl_handle_cor_ras(struct device *dev, void __iomem *ras_base)
> +void cxl_handle_cor_ras(struct cxl_port *port, struct cxl_dport *dport, void __iomem *ras_base)
> {
> u32 status;
> void __iomem *addr;
> @@ -260,10 +259,12 @@ void cxl_handle_cor_ras(struct device *dev, void __iomem *ras_base)
> status = readl(addr);
> if (status & CXL_RAS_CORRECTABLE_STATUS_MASK) {
> writel(status & CXL_RAS_CORRECTABLE_STATUS_MASK, addr);
> - if (is_cxl_memdev(dev))
> - trace_cxl_aer_correctable_error(to_cxl_memdev(dev), status);
> + if (is_cxl_endpoint(port))
> + trace_cxl_aer_correctable_error(to_cxl_memdev(port->uport_dev), status);
> + else if (dport)
> + trace_cxl_port_aer_correctable_error(dport->dport_dev, status);
> else
> - trace_cxl_port_aer_correctable_error(dev, status);
> + trace_cxl_port_aer_correctable_error(port->uport_dev, status);
> }
> }
>
> @@ -288,7 +289,7 @@ static void header_log_copy(void __iomem *ras_base, u32 *log)
> * Log the state of the RAS status registers and prepare them to log the
> * next error status. Return 1 if reset needed.
> */
> -bool cxl_handle_ras(struct device *dev, void __iomem *ras_base)
> +bool cxl_handle_ras(struct cxl_port *port, struct cxl_dport *dport, void __iomem *ras_base)
> {
> u32 hl[CXL_HEADERLOG_TRACE_SIZE_U32] = {};
> void __iomem *addr;
> @@ -315,10 +316,12 @@ bool cxl_handle_ras(struct device *dev, void __iomem *ras_base)
> }
>
> header_log_copy(ras_base, hl);
> - if (is_cxl_memdev(dev))
> - trace_cxl_aer_uncorrectable_error(to_cxl_memdev(dev), status, fe, hl);
> + if (is_cxl_endpoint(port))
> + trace_cxl_aer_uncorrectable_error(to_cxl_memdev(port->uport_dev), status, fe, hl);
> + else if (dport)
> + trace_cxl_port_aer_uncorrectable_error(dport->dport_dev, status, fe, hl);
> else
> - trace_cxl_port_aer_uncorrectable_error(dev, status, fe, hl);
> + trace_cxl_port_aer_uncorrectable_error(port->uport_dev, status, fe, hl);
>
> writel(status & CXL_RAS_UNCORRECTABLE_STATUS_MASK, addr);
>
> @@ -351,7 +354,7 @@ pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
> * chance the situation is recoverable dump the status of the RAS
> * capability registers and bounce the active state of the memdev.
> */
> - ue = cxl_handle_ras(port->uport_dev, to_ras_base(port, NULL));
> + ue = cxl_handle_ras(port, NULL, to_ras_base(port, NULL));
> }
>
> /*
> @@ -382,10 +385,8 @@ EXPORT_SYMBOL_NS_GPL(cxl_error_detected, "CXL");
> static void cxl_handle_proto_error(struct pci_dev *pdev, struct cxl_port *port,
> struct cxl_dport *dport, int severity)
> {
> - struct device *dev = dport ? dport->dport_dev : port->uport_dev;
> -
> if (severity == AER_CORRECTABLE)
> - cxl_handle_cor_ras(dev, to_ras_base(port, dport));
> + cxl_handle_cor_ras(port, dport, to_ras_base(port, dport));
> else
> cxl_do_recovery(pdev, port, dport);
> }
> diff --git a/drivers/cxl/core/ras_rch.c b/drivers/cxl/core/ras_rch.c
> index f2d2fb83758b9..f4b98f2c11a1c 100644
> --- a/drivers/cxl/core/ras_rch.c
> +++ b/drivers/cxl/core/ras_rch.c
> @@ -118,7 +118,7 @@ void cxl_handle_rdport_errors(struct pci_dev *pdev)
>
> pci_print_aer(pdev, severity, &aer_regs);
> if (severity == AER_CORRECTABLE)
> - cxl_handle_cor_ras(&pdev->dev, to_ras_base(port, dport));
> + cxl_handle_cor_ras(dport->port, dport, to_ras_base(port, dport));
> else
> cxl_do_recovery(pdev, dport->port, dport);
> }
^ permalink raw reply
* Re: [PATCH v18 09/13] cxl: Update CXL Endpoint AER handler
From: Jonathan Cameron @ 2026-07-20 23:29 UTC (permalink / raw)
To: Terry Bowman
Cc: Bjorn Helgaas, Dan Williams, Dave Jiang, Ira Weiny, Len Brown,
Rafael J . Wysocki, Robert Richter, linux-acpi, linux-cxl,
linux-doc, linux-kernel, linux-pci, linuxppc-dev,
Alejandro Lucero, Alison Schofield, Ankit Agrawal, Ard Biesheuvel,
Ben Cheatham, Borislav Petkov, Breno Leitao, Davidlohr Bueso,
Fabio M . De Francesco, Gregory Price, Hanjun Guo,
Jonathan Corbet, Kees Cook, Kuppuswamy Sathyanarayanan, Li Ming,
Mahesh J Salgaonkar, Mauro Carvalho Chehab, Oliver O'Halloran,
Shiju Jose, Shuah Khan, Shuai Xue, Smita Koralahalli, Tony Luck,
Vishal Verma
In-Reply-To: <20260717222706.3540281-10-terry.bowman@amd.com>
On Fri, 17 Jul 2026 17:27:02 -0500
Terry Bowman <terry.bowman@amd.com> wrote:
> Rename cxl_error_detected() to cxl_pci_error_detected() and rename
> the struct pci_error_handlers instance to cxl_pci_error_handlers to
> avoid shadowing the struct type tag.
>
> Document the unconditional CXL RAS read policy: on a dead link,
> readl() returns 0xFFFFFFFF which is interpreted as UCE bits set and
> triggers a panic. If RAS registers are not mapped the read is
> skipped and the frozen/perm_failure switch cases defer to AER
> recovery for devices without active CXL.mem traffic.
>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
>
A couple of trivial things inline.
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
> drivers/cxl/core/ras.c | 24 +++++++++++++++---------
> drivers/cxl/cxlpci.h | 8 ++++----
> drivers/cxl/pci.c | 12 ++++++------
> 3 files changed, 25 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c
> index 6f4a3c1b0bb85..d5dc2c22565da 100644
> --- a/drivers/cxl/core/ras.c
> +++ b/drivers/cxl/core/ras.c
> @@ -328,10 +328,8 @@ bool cxl_handle_ras(struct cxl_port *port, struct cxl_dport *dport, void __iomem
> return true;
> }
>
> -
> -
This white space removal should be in patch 7 I think.
> -pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
> - pci_channel_state_t state)
> +pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
> + pci_channel_state_t state)
> {
> struct cxl_port *port __free(put_cxl_port) = find_cxl_port_by_uport(&pdev->dev);
> bool ue = false;
> @@ -349,10 +347,18 @@ pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
> }
>
> /*
> - * A frozen channel indicates an impending reset which is fatal to
> - * CXL.mem operation, and will likely crash the system. On the off
> - * chance the situation is recoverable dump the status of the RAS
> - * capability registers and bounce the active state of the memdev.
> + * The CXL RAS read is unconditional regardless of channel
Keep the 80 char wrap for comments. Also local style is single space after .
(I'm terrible at not following style on that stuff but do as I say not as
I do!)
> + * state. Any uncorrectable error bit set in the CXL RAS
> + * status register triggers a panic because CXL.mem cache
> + * coherency is already lost; continuing risks silent data
> + * corruption across interleaved HDM regions.
> + *
> + * On a dead link readl() returns 0xFFFFFFFF which sets all
> + * UCE bits and also triggers the panic - this is intentional.
> + * If RAS registers are not mapped the read is skipped, the
> + * panic is not reached, and the frozen/perm_failure switch
> + * cases below handle AER recovery for devices without active
> + * CXL.mem traffic.
Now this comment I like. Good and clear and explaining the non obvious decision.
> */
> ue = cxl_handle_ras(port, NULL, to_ras_base(port, NULL));
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 5c21db36073fe..6cf1db7b85020 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -1000,18 +1000,18 @@ static void cxl_reset_done(struct pci_dev *pdev)
> }
> }
>
> -static const struct pci_error_handlers cxl_error_handlers = {
> - .error_detected = cxl_error_detected,
> - .slot_reset = cxl_slot_reset,
> - .resume = cxl_error_resume,
> - .reset_done = cxl_reset_done,
> +static const struct pci_error_handlers cxl_pci_error_handlers = {
> + .error_detected = cxl_pci_error_detected,
> + .slot_reset = cxl_slot_reset,
> + .resume = cxl_error_resume,
> + .reset_done = cxl_reset_done,
Do we need the re-indent? Nice to make it more obvious what didn't change.
> };
^ permalink raw reply
* Re: [PATCH v18 13/13] Documentation: cxl: Document CXL protocol error handling
From: Dave Jiang @ 2026-07-20 23:40 UTC (permalink / raw)
To: Terry Bowman, Bjorn Helgaas, Dan Williams, Ira Weiny,
Jonathan Cameron, Len Brown, Rafael J . Wysocki, Robert Richter
Cc: linux-acpi, linux-cxl, linux-doc, linux-kernel, linux-pci,
linuxppc-dev, Alejandro Lucero, Alison Schofield, Ankit Agrawal,
Ard Biesheuvel, Ben Cheatham, Borislav Petkov, Breno Leitao,
Davidlohr Bueso, Fabio M . De Francesco, Gregory Price,
Hanjun Guo, Jonathan Corbet, Kees Cook,
Kuppuswamy Sathyanarayanan, Li Ming, Mahesh J Salgaonkar,
Mauro Carvalho Chehab, Oliver O'Halloran, Shiju Jose,
Shuah Khan, Shuai Xue, Smita Koralahalli, Tony Luck, Vishal Verma
In-Reply-To: <20260717222706.3540281-14-terry.bowman@amd.com>
On 7/17/26 3:27 PM, Terry Bowman wrote:
> Add Documentation/driver-api/cxl/linux/protocol-error-handling.rst
> describing the end-to-end CXL protocol error path: AER ingress, the
> AER-CXL kfifo handoff, the cxl_core consumer worker, RCD/RCH special
> cases, severity policy, trace events, and a source code map.
>
> This documents the architecture introduced by the preceding patches in
> this series.
>
> Assisted-by: Claude:claude-opus-4.7
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>
> ---
> Changes in v17->v18:
> - Simplify document for readability (Jonathan)
> - Drop historical context that goes stale (Jonathan)
> - Shorten ASCII flow diagram (Jonathan)
> - Drop manual backtick markup, use automarkup (Jonathan)
> - Clarify USP/DSP as single switch component (Dave)
> - Fix line wrapping to 80 chars (Jonathan)
> ---
> Documentation/driver-api/cxl/index.rst | 1 +
> .../cxl/linux/protocol-error-handling.rst | 222 ++++++++++++++++++
> 2 files changed, 223 insertions(+)
> create mode 100644 Documentation/driver-api/cxl/linux/protocol-error-handling.rst
>
> diff --git a/Documentation/driver-api/cxl/index.rst b/Documentation/driver-api/cxl/index.rst
> index 3dfae1d310ca5..6861b2e5726a3 100644
> --- a/Documentation/driver-api/cxl/index.rst
> +++ b/Documentation/driver-api/cxl/index.rst
> @@ -42,6 +42,7 @@ that have impacts on each other. The docs here break up configurations steps.
> linux/dax-driver
> linux/memory-hotplug
> linux/access-coordinates
> + linux/protocol-error-handling
>
> .. toctree::
> :maxdepth: 2
> diff --git a/Documentation/driver-api/cxl/linux/protocol-error-handling.rst b/Documentation/driver-api/cxl/linux/protocol-error-handling.rst
> new file mode 100644
> index 0000000000000..67f0492e56702
> --- /dev/null
> +++ b/Documentation/driver-api/cxl/linux/protocol-error-handling.rst
> @@ -0,0 +1,222 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +==============================
> +CXL Protocol Error Handling
> +==============================
> +
> +CXL devices report protocol-layer failures (CXL.cachemem RAS) as PCIe
> +AER Internal Errors: PCI_ERR_COR_INTERNAL for correctable events and
> +PCI_ERR_UNC_INTN for uncorrectable events. The actual fault
> +information lives in CXL RAS capability registers, not in the PCIe AER
> +status registers.
> +
> +The kernel routes every CXL Internal Error through a producer/consumer
> +pipeline shared by all CXL device types: Root Ports, Upstream/Downstream
> +Switch Ports, Endpoints, and Restricted CXL Devices (RCDs).
> +
> +
> +Architecture
> +============
> +
> +Two error planes run side by side:
> +
> +* The **PCIe AER plane** handles native PCIe errors (receiver
> + overflows, malformed TLPs, completion timeouts, etc.).
> +* The **CXL protocol error plane** handles CXL Internal Errors.
> + The AER core forwards them to cxl_core via a dedicated kfifo;
> + cxl_core reads the CXL RAS registers, emits trace events, and
> + applies recovery/panic policy.
> +
> +The boundary between the two planes is enforced by is_cxl_error() in
> +aer_cxl_vh.c. It checks info->is_cxl, the PCIe device type
> +(Endpoint, Root Port, Upstream, or Downstream), and whether the AER
> +status word indicates an internal error. RC_END devices are excluded
> +from is_cxl_error() because they reach the kfifo via the separate
> +cxl_rch_handle_error() path instead.
> +
> +The pipeline:
> +
> +1. **Producer** (aer_cxl_vh.c, aer_cxl_rch.c) - AER threaded
> + handler context. Classifies and enqueues a
> + struct cxl_proto_err_work_data into the kfifo.
> +2. **Queue** - the AER-CXL kfifo plus a backing work_struct.
> +3. **Consumer** (cxl_core/ras.c) - workqueue context. Resolves
> + the CXL port topology and dispatches to CE/UE handlers.
> +
> +
> +Topologies
> +==========
> +
> +Virtual Hierarchy (VH)
> +----------------------
> +
> +Standard PCIe topology: Root Port, optional switch (Upstream Port with
> +one or more Downstream Ports), and Endpoints. Each component raises
> +Internal Errors directly via the Root Port's AER interrupt.
> +
> +Producer: cxl_forward_error() in aer_cxl_vh.c.
> +
> +Restricted CXL Host (RCH)
> +--------------------------
> +
> +A Root Complex Event Collector (RCEC) aggregates errors from RCDs
> +attached as Root Complex Integrated Endpoints. The AER driver
> +iterates RCDs beneath the RCEC via pcie_walk_rcec() and forwards
> +each qualifying device through cxl_forward_error() into the same
> +kfifo.
> +
> +Producer: cxl_forward_error() in aer_cxl_vh.c, called from
> +cxl_rch_handle_error_iter() via pcie_walk_rcec().
> +
> +
> +Error flow
> +==========
> +
> +.. code-block:: text
> +
> + CXL device raises AER Internal Error
> + (PCI_ERR_COR_INTERNAL or PCI_ERR_UNC_INTN)
> + |
> + v
> + +--------------------------------------+
> + | AER core (aer.c) |
> + | aer_irq() -> aer_isr() |
> + | -> find_source_device() |
> + | -> handle_error_source(dev, info) |
> + +--------------------------------------+
> + |
> + v
> + +--------------------------------------+
> + | handle_error_source() dispatch |
> + | |
> + | 1. cxl_rch_handle_error() |
> + | [always; filters internally] |
> + | |
> + | 2. if is_cxl_error(): |
> + | cxl_forward_error() |
> + | [enqueue to kfifo] |
> + | |
> + | 3. if cxl_pending && non-CE: |
> + | cxl_proto_err_flush() |
> + | [sync drain before recovery] |
> + | |
> + | 4. pci_aer_handle_error() [always] |
> + +--------------------------------------+
> + |
> + (kfifo -> workqueue)
> + |
> + v
> + +--------------------------------------+
> + | __cxl_proto_err_work_fn() consumer |
> + | |
> + | if is_cxl_restricted(pdev): |
> + | cxl_handle_rdport_errors() |
> + | [RCH dport RAS first] |
> + | |
> + | port = find_cxl_port_by_dev( |
> + | &pdev->dev, NULL) |
> + | dport = cxl_find_dport_by_dev( |
> + | port, &pdev->dev) |
> + | [dport NULL for EP/USP; set RP/DSP] |
> + | |
> + | cxl_handle_proto_error() |
> + +--------------------------------------+
> + | |
> + v v
> + +-----------------+ +--------------------+
> + | CE | | UCE |
> + | cxl_handle_ | | cxl_do_recovery() |
> + | cor_ras() | | read RAS status |
> + | trace + clear | | trace + panic |
> + +-----------------+ +--------------------+
> +
> +cxl_do_recovery() reads the CXL RAS uncorrectable status register.
> +If UE bits are set, it emits the trace event and panics. If no bits
> +are set (e.g. RAS mapped but error already cleared), it logs a
> +diagnostic and defers to AER recovery.
> +
> +
> +Severity policy
> +===============
> +
> +**CE** - cxl_handle_cor_ras() reads the CXL RAS correctable status
> +register, clears set bits, and emits a cxl_aer_correctable_error
> +trace event. No recovery action.
> +
> +**UCE (non-fatal, and fatal on Root Port/Downstream Port)** - cxl_do_recovery() reads the CXL RAS
> +uncorrectable status register. If UE bits are set, the kernel panics.
> +CXL.cachemem traffic cannot be safely recovered once an uncorrectable
> +error is signaled; continuing risks silent data corruption across
> +interleaved HDM regions. This panic policy applies to the native AER
> +path. On firmware-first (CPER/GHES) platforms the CPER handler emits
> +trace events only and does not call cxl_do_recovery().
> +
> +**Fatal UCE on EP/USP** - The AER core driver does not read AER status
> +registers for Endpoint and Upstream Ports with fatal events because the
> +link is down. Without AER status, is_cxl_error() cannot classify
> +the event as a CXL protocol error and it falls through to standard
> +AER recovery.
> +
> +RCH special case
> +================
> +
> +When the consumer sees is_cxl_restricted(pdev), it calls
> +cxl_handle_rdport_errors() first to process the RCH Downstream
> +Port's RAS registers (accessed via RCRB, not standard config space).
> +It then continues to process the RCD Endpoint's own RAS registers
> +via the common path. Both register blocks are checked because
> +errors can appear in either independently.
> +
> +cxl_handle_rdport_errors() acquires the port lock internally.
> +Callers must not hold it.
> +
> +
> +Trace events
> +============
> +
> +Two trace events cover all device types and both the native AER and
> +CPER/GHES firmware-first paths:
> +
> +* cxl_aer_correctable_error
> +* cxl_aer_uncorrectable_error
> +
> +Fields:
> +
> +* ``memdev`` - memdev name for Endpoints; empty for non-Endpoints.
> +* ``port`` - CXL port device name.
> +* ``dport`` - Downstream Port device name; empty when not applicable.
> +* ``host`` - parent host bridge or uport device name.
> +* ``serial`` - PCI Device Serial Number from pdev->dsn (cached at
> + enumeration; no config-space read in the error path).
> +
> +
> +Interrupt masking
> +=================
> +
> +CXL Internal Error bits (PCI_ERR_UNC_INTN and PCI_ERR_COR_INTERNAL)
> +are unmasked in the AER capability only after the CXL RAS register
> +block is successfully mapped. A devm teardown action restores the
> +mask when the port or dport is removed, ensuring clean state after
> +driver removal.
> +
> +
> +Source files
> +============
> +
> +.. list-table::
> + :header-rows: 1
> +
> + * - File
> + - Role
> + * - drivers/pci/pcie/aer.c
> + - AER core; IRQ, dispatch
> + * - drivers/pci/pcie/aer_cxl_vh.c
> + - VH producer; kfifo
> + * - drivers/pci/pcie/aer_cxl_rch.c
> + - RCH dispatch; RCEC walk
> + * - drivers/cxl/core/ras.c
> + - Consumer; CE/UE handlers; CPER
> + * - drivers/cxl/core/ras_rch.c
> + - RCH dport RAS handling
> + * - drivers/acpi/apei/ghes.c
> + - CPER/GHES kfifo producer
^ permalink raw reply
* Re: [PATCH v18 10/13] cxl: Add port and dport identifiers to CXL AER trace events
From: Jonathan Cameron @ 2026-07-21 0:00 UTC (permalink / raw)
To: Terry Bowman
Cc: Bjorn Helgaas, Dan Williams, Dave Jiang, Ira Weiny, Len Brown,
Rafael J . Wysocki, Robert Richter, linux-acpi, linux-cxl,
linux-doc, linux-kernel, linux-pci, linuxppc-dev,
Alejandro Lucero, Alison Schofield, Ankit Agrawal, Ard Biesheuvel,
Ben Cheatham, Borislav Petkov, Breno Leitao, Davidlohr Bueso,
Fabio M . De Francesco, Gregory Price, Hanjun Guo,
Jonathan Corbet, Kees Cook, Kuppuswamy Sathyanarayanan, Li Ming,
Mahesh J Salgaonkar, Mauro Carvalho Chehab, Oliver O'Halloran,
Shiju Jose, Shuah Khan, Shuai Xue, Smita Koralahalli, Tony Luck,
Vishal Verma
In-Reply-To: <20260717222706.3540281-11-terry.bowman@amd.com>
On Fri, 17 Jul 2026 17:27:03 -0500
Terry Bowman <terry.bowman@amd.com> wrote:
> From: Dan Williams <djbw@kernel.org>
>
> Pass struct cxl_port * and struct cxl_dport * to the cxl_aer_*
> trace events instead of a plain struct device * derived at the
> caller. The trace event helpers then derive the right strings for
> endpoints, switch ports, root ports, and RCH downstream ports
> consistently across the CPER and native AER paths.
>
> The unified cxl_aer_* events keep "memdev" as the legacy field
> (endpoint events populate it with the memdev name; non-endpoint
> events emit memdev="") and add new "port" and "dport" string fields
> populated for all CXL device classes. Updated userspace can key
> off "port" and "dport" without a parallel set of events.
>
> Remove the separate cxl_port_aer_uncorrectable_error and
> cxl_port_aer_correctable_error trace events. All CXL AER events now
> use the unified cxl_aer_* events with port and dport fields.
>
> Rework cxl_cper_handle_prot_err() to use find_cxl_port_by_dev() and
> the unified trace helpers, replacing the per-port-type branching and
> bus_find_device() memdev lookup.
>
> The TP_printk format string places "port=%s dport=%s" between
> "memdev=%s" and "host=%s", changing the text-mode field order from
> the pre-patch output. This does not affect consumers such as
> rasdaemon that use libtraceevent to parse fields by name rather than
> by fixed text position.
>
> For non-endpoint events (switch port, root port, RCH dport),
> "memdev" is empty and "port"/"dport" carry the topology information.
>
> The serial number is retrieved via pci_get_dsn() which performs live
> PCI configuration space reads. A following patch ("PCI: Cache PCI
> DSN into pci_dev->dsn during probe") replaces these with a cached
> serial number to avoid config space access in error handlers and panic
> paths.
>
> Below are examples of the different CXL devices' error trace logs
> after this patch:
>
> ---------------------
> | CXL RP - 0C:00.0 |
> ---------------------
> |
> ---------------------
> | CXL USP - 0D:00.0 |
> ---------------------
> |
> --------------------
> | CXL DSP - 0E:00.0 |
> --------------------
> |
> ---------------------
> | CXL EP - 0F:00.0 |
> ---------------------
>
> Root Port:
> cxl_aer_correctable_error: memdev= port=port1 dport=0000:0c:00.0 \
> host=pci0000:0c serial=0: status: 'Memory Data ECC Error'
>
> cxl_aer_uncorrectable_error: memdev= port=port1 dport=0000:0c:00.0 \
> host=pci0000:0c serial=0: status: 'Cache Address Parity Error' \
> first_error: 'Cache Address Parity Error'
>
> Upstream Switch Port:
> cxl_aer_correctable_error: memdev= port=port2 dport= host=0000:0d:00.0 \
> serial=0: status: 'Memory Data ECC Error'
>
> UCE NA - Upstream Switch Port UCE's are handled in the portdrv driver's
> PCI AER callbacks that are not CXL aware.
>
> Downstream Switch Port:
> cxl_aer_correctable_error: memdev= port=port2 dport=0000:0e:00.0 \
> host=0000:0d:00.0 serial=0: status: 'Memory Data ECC Error'
>
> cxl_aer_uncorrectable_error: memdev= port=port2 dport=0000:0e:00.0 \
> host=0000:0d:00.0 serial=0: status: 'Cache Address Parity Error' \
> first_error: 'Cache Address Parity Error'
>
> Endpoint:
> cxl_aer_uncorrectable_error: memdev=mem1 port=endpoint4 dport= \
> host=0000:0f:00.0 serial=0: status: 'Cache Address Parity Error' \
> first_error: 'Cache Address Parity Error'
>
> cxl_aer_correctable_error: memdev=mem1 port=endpoint4 dport= host=0000:0f:00.0 \
> serial=0: status: 'Memory Data ECC Error'
>
> Co-developed-by: Terry Bowman <terry.bowman@amd.com>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> Signed-off-by: Dan Williams <djbw@kernel.org>
One question inline about reference counts for the port.
>
> diff --git a/drivers/cxl/core/ras.c b/drivers/cxl/core/ras.c
> index d5dc2c22565da..acf40b2396c3b 100644
> --- a/drivers/cxl/core/ras.c
> +++ b/drivers/cxl/core/ras.c
...
>
> @@ -109,47 +77,34 @@ static struct cxl_port *find_cxl_port_by_dev(struct device *dev, struct cxl_dpor
>
> void cxl_cper_handle_prot_err(struct cxl_cper_prot_err_work_data *data)
> {
> + struct cxl_dport *dport;
> unsigned int devfn = PCI_DEVFN(data->prot_err.agent_addr.device,
> data->prot_err.agent_addr.function);
> - struct pci_dev *pdev __free(pci_dev_put) =
> - pci_get_domain_bus_and_slot(data->prot_err.agent_addr.segment,
> - data->prot_err.agent_addr.bus,
> - devfn);
> - struct cxl_memdev *cxlmd;
> - int port_type;
> -
> - if (!pdev)
> - return;
> -
> - port_type = pci_pcie_type(pdev);
> - if (port_type == PCI_EXP_TYPE_ROOT_PORT ||
> - port_type == PCI_EXP_TYPE_DOWNSTREAM ||
> - port_type == PCI_EXP_TYPE_UPSTREAM) {
> - if (data->severity == AER_CORRECTABLE)
> - cxl_cper_trace_corr_port_prot_err(pdev, data->ras_cap);
> - else
> - cxl_cper_trace_uncorr_port_prot_err(pdev, data->ras_cap);
> -
> + struct pci_dev *pdev __free(pci_dev_put) = pci_get_domain_bus_and_slot(
> + data->prot_err.agent_addr.segment, data->prot_err.agent_addr.bus, devfn);
> + if (!pdev) {
> + pr_err_ratelimited("Failed to find CPER device in CXL topology\n");
> return;
> }
>
> - guard(device)(&pdev->dev);
> - if (!pdev->dev.driver) {
> - dev_warn_ratelimited(&pdev->dev,
> - "Device is unbound, abort CPER error handling\n");
> + struct cxl_port *port __free(put_cxl_port) = find_cxl_port_by_dev(&pdev->dev, NULL);
> + if (!port) {
> + dev_err_ratelimited(&pdev->dev,
> + "Failed to find parent port device in CXL topology\n");
> return;
> }
>
> - struct device *mem_dev __free(put_device) = bus_find_device(
> - &cxl_bus_type, NULL, pdev, match_memdev_by_parent);
> - if (!mem_dev)
> - return;
> + guard(device)(&port->dev);
Don't we have a reference for this from find_cxl_port_by_dev()?
Superficially scope looks the same.
> +
> + /* dport is NULL for Endpoint and Upstream Port devices */
> + dport = cxl_find_dport_by_dev(port, &pdev->dev);
>
> - cxlmd = to_cxl_memdev(mem_dev);
> if (data->severity == AER_CORRECTABLE)
> - cxl_cper_trace_corr_prot_err(cxlmd, data->ras_cap);
> + cxl_cper_trace_corr_prot_err(port, dport, pci_get_dsn(pdev),
> + &data->ras_cap);
> else
> - cxl_cper_trace_uncorr_prot_err(cxlmd, data->ras_cap);
> + cxl_cper_trace_uncorr_prot_err(port, dport, pci_get_dsn(pdev),
> + &data->ras_cap);
> }
> EXPORT_SYMBOL_GPL(cxl_cper_handle_prot_err);
>
> @@ -240,14 +195,14 @@ void cxl_do_recovery(struct pci_dev *pdev, struct cxl_port *port, struct cxl_dpo
> return;
> }
>
> - if (cxl_handle_ras(port, dport, ras_base))
> + if (cxl_handle_ras(port, dport, ras_base, pci_get_dsn(pdev)))
> panic("CXL cachemem error");
>
> dev_dbg(&pdev->dev,
> "CXL UCE signaled but no CXL RAS status bits set\n");
> }
>
> -void cxl_handle_cor_ras(struct cxl_port *port, struct cxl_dport *dport, void __iomem *ras_base)
> +void cxl_handle_cor_ras(struct cxl_port *port, struct cxl_dport *dport, void __iomem *ras_base, u64 serial)
That's over even the modern 100 char limit. Needs a line break.
> {
^ permalink raw reply
* Re: [PATCH v18 12/13] PCI/CXL: Mask/Unmask CXL protocol errors
From: Jonathan Cameron @ 2026-07-21 0:10 UTC (permalink / raw)
To: Terry Bowman
Cc: Bjorn Helgaas, Dan Williams, Dave Jiang, Ira Weiny, Len Brown,
Rafael J . Wysocki, Robert Richter, linux-acpi, linux-cxl,
linux-doc, linux-kernel, linux-pci, linuxppc-dev,
Alejandro Lucero, Alison Schofield, Ankit Agrawal, Ard Biesheuvel,
Ben Cheatham, Borislav Petkov, Breno Leitao, Davidlohr Bueso,
Fabio M . De Francesco, Gregory Price, Hanjun Guo,
Jonathan Corbet, Kees Cook, Kuppuswamy Sathyanarayanan, Li Ming,
Mahesh J Salgaonkar, Mauro Carvalho Chehab, Oliver O'Halloran,
Shiju Jose, Shuah Khan, Shuai Xue, Smita Koralahalli, Tony Luck,
Vishal Verma
In-Reply-To: <20260717222706.3540281-13-terry.bowman@amd.com>
On Fri, 17 Jul 2026 17:27:05 -0500
Terry Bowman <terry.bowman@amd.com> wrote:
> CXL protocol errors are not enabled for all CXL devices after boot.
> They must be enabled in order to process CXL protocol errors. Provide
> matching teardown helpers so the masks are restored when a CXL Port
> or dport goes away.
>
> Add pci_aer_mask_internal_errors() as the symmetric counterpart to
> pci_aer_unmask_internal_errors() and export both for the cxl_core
> module.
>
> Introduce cxl_unmask_proto_interrupts() and cxl_mask_proto_interrupts()
> in cxl_core to wrap the PCI helpers with the dev_is_pci() and
> pcie_aer_is_native() gating CXL needs. Both helpers tolerate a NULL
> or non-PCI @dev so callers do not have to special-case it.
>
> Wire cxl_unmask_proto_interrupts() into the success path of
> cxl_dport_map_ras() and devm_cxl_port_ras_setup() so the unmask
> only runs when the RAS register block was actually mapped. Pair each
> unmask with a devm_add_action_or_reset() registration of
> cxl_mask_proto_irqs() scoped to the host device so the mask is
> restored when devres is released. This applies to dports, Endpoints,
> Upstream Switch Ports, Downstream Switch Ports, and Root Ports.
>
> Remove the dev_is_pci(dport->dport_dev) guard in
> devm_cxl_dport_rch_ras_setup(). On RCH systems dport->dport_dev is the
> pci_host_bridge device, which is not on pci_bus_type, so this guard
> caused the function to return early on real hardware without mapping
> dport RAS or AER registers. The caller already gates on dport->rch,
> which is sufficient to exclude cxl_test mock devices.
>
> Co-developed-by: Dan Williams <djbw@kernel.org>
> Signed-off-by: Dan Williams <djbw@kernel.org>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
>
One minor thing inline and maybe take another spin at a more concise
patch description
Either way on both of them.
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
...
> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
> index 0bd23a65e7ebc..be6dc2cbd4491 100644
> --- a/drivers/pci/pcie/aer.c
> +++ b/drivers/pci/pcie/aer.c
> @@ -1143,12 +1143,32 @@ void pci_aer_unmask_internal_errors(struct pci_dev *dev)
> mask &= ~PCI_ERR_COR_INTERNAL;
> pci_write_config_dword(dev, aer + PCI_ERR_COR_MASK, mask);
> }
> +EXPORT_SYMBOL_FOR_MODULES(pci_aer_unmask_internal_errors, "cxl_core");
>
> -/*
> - * Internal errors are too device-specific to enable generally, however for CXL
> - * their behavior is standardized for conveying CXL protocol errors.
> +/**
> + * pci_aer_mask_internal_errors - mask internal errors
> + * @dev: pointer to the pci_dev data structure
> + *
> + * Mask internal errors in the Uncorrectable and Correctable Error
> + * Mask registers.
> + *
> + * Note: AER must be enabled and supported by the device which must be
> + * checked in advance, e.g. with pcie_aer_is_native().
> */
> -EXPORT_SYMBOL_FOR_MODULES(pci_aer_unmask_internal_errors, "cxl_core");
> +void pci_aer_mask_internal_errors(struct pci_dev *dev)
> +{
> + int aer = dev->aer_cap;
> + u32 mask;
> +
> + pci_read_config_dword(dev, aer + PCI_ERR_UNCOR_MASK, &mask);
> + mask |= PCI_ERR_UNC_INTN;
> + pci_write_config_dword(dev, aer + PCI_ERR_UNCOR_MASK, mask);
Could do
pci_clear_and_set_config_dword(dev, aer + PCI_ERR_UNCOR_MASK,
0, PCI_ERR_COR_INTERNAL);
or something along those lines.
Maybe it is worth wrapping that in some helpers to make it a bit
more regmap like in that it could have set and clear only variants.
> +
> + pci_read_config_dword(dev, aer + PCI_ERR_COR_MASK, &mask);
> + mask |= PCI_ERR_COR_INTERNAL;
> + pci_write_config_dword(dev, aer + PCI_ERR_COR_MASK, mask);
> +}
> +EXPORT_SYMBOL_FOR_MODULES(pci_aer_mask_internal_errors, "cxl_core");
^ permalink raw reply
* Re: [PATCH v18 13/13] Documentation: cxl: Document CXL protocol error handling
From: Jonathan Cameron @ 2026-07-21 0:19 UTC (permalink / raw)
To: Terry Bowman
Cc: Bjorn Helgaas, Dan Williams, Dave Jiang, Ira Weiny, Len Brown,
Rafael J . Wysocki, Robert Richter, linux-acpi, linux-cxl,
linux-doc, linux-kernel, linux-pci, linuxppc-dev,
Alejandro Lucero, Alison Schofield, Ankit Agrawal, Ard Biesheuvel,
Ben Cheatham, Borislav Petkov, Breno Leitao, Davidlohr Bueso,
Fabio M . De Francesco, Gregory Price, Hanjun Guo,
Jonathan Corbet, Kees Cook, Kuppuswamy Sathyanarayanan, Li Ming,
Mahesh J Salgaonkar, Mauro Carvalho Chehab, Oliver O'Halloran,
Shiju Jose, Shuah Khan, Shuai Xue, Smita Koralahalli, Tony Luck,
Vishal Verma
In-Reply-To: <20260717222706.3540281-14-terry.bowman@amd.com>
On Fri, 17 Jul 2026 17:27:06 -0500
Terry Bowman <terry.bowman@amd.com> wrote:
> Add Documentation/driver-api/cxl/linux/protocol-error-handling.rst
> describing the end-to-end CXL protocol error path: AER ingress, the
> AER-CXL kfifo handoff, the cxl_core consumer worker, RCD/RCH special
> cases, severity policy, trace events, and a source code map.
>
> This documents the architecture introduced by the preceding patches in
> this series.
>
> Assisted-by: Claude:claude-opus-4.7
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
A couple of trivial things inline to tidy up. Actual text seems good
to me.
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
>
> ---
> Changes in v17->v18:
> - Simplify document for readability (Jonathan)
> - Drop historical context that goes stale (Jonathan)
> - Shorten ASCII flow diagram (Jonathan)
> - Drop manual backtick markup, use automarkup (Jonathan)
> - Clarify USP/DSP as single switch component (Dave)
> - Fix line wrapping to 80 chars (Jonathan)
> ---
> Documentation/driver-api/cxl/index.rst | 1 +
> .../cxl/linux/protocol-error-handling.rst | 222 ++++++++++++++++++
> 2 files changed, 223 insertions(+)
> create mode 100644 Documentation/driver-api/cxl/linux/protocol-error-handling.rst
>
> diff --git a/Documentation/driver-api/cxl/index.rst b/Documentation/driver-api/cxl/index.rst
> index 3dfae1d310ca5..6861b2e5726a3 100644
> --- a/Documentation/driver-api/cxl/index.rst
> +++ b/Documentation/driver-api/cxl/index.rst
> @@ -42,6 +42,7 @@ that have impacts on each other. The docs here break up configurations steps.
> linux/dax-driver
> linux/memory-hotplug
> linux/access-coordinates
> + linux/protocol-error-handling
>
> .. toctree::
> :maxdepth: 2
> diff --git a/Documentation/driver-api/cxl/linux/protocol-error-handling.rst b/Documentation/driver-api/cxl/linux/protocol-error-handling.rst
> new file mode 100644
> index 0000000000000..67f0492e56702
> --- /dev/null
> +++ b/Documentation/driver-api/cxl/linux/protocol-error-handling.rst
> @@ -0,0 +1,222 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +==============================
> +CXL Protocol Error Handling
> +==============================
> +
> +CXL devices report protocol-layer failures (CXL.cachemem RAS) as PCIe
Why short wrap? Docs are 80 chars I think.
> +AER Internal Errors: PCI_ERR_COR_INTERNAL for correctable events and
> +PCI_ERR_UNC_INTN for uncorrectable events. The actual fault
> +information lives in CXL RAS capability registers, not in the PCIe AER
> +status registers.
> +Error flow
> +==========
> +
> +.. code-block:: text
> +
> + CXL device raises AER Internal Error
> + (PCI_ERR_COR_INTERNAL or PCI_ERR_UNC_INTN)
> + |
> + v
> + +--------------------------------------+
> + | AER core (aer.c) |
> + | aer_irq() -> aer_isr() |
> + | -> find_source_device() |
> + | -> handle_error_source(dev, info) |
> + +--------------------------------------+
> + |
> + v
> + +--------------------------------------+
> + | handle_error_source() dispatch |
> + | |
> + | 1. cxl_rch_handle_error() |
> + | [always; filters internally] |
> + | |
> + | 2. if is_cxl_error(): |
Smells like a missing space.
> + | cxl_forward_error() |
> + | [enqueue to kfifo] |
> + | |
> + | 3. if cxl_pending && non-CE: |
> + | cxl_proto_err_flush() |
> + | [sync drain before recovery] |
> + | |
> + | 4. pci_aer_handle_error() [always] |
> + +--------------------------------------+
> + |
> + (kfifo -> workqueue)
> + |
> + v
> + +--------------------------------------+
> + | __cxl_proto_err_work_fn() consumer |
> + | |
> + | if is_cxl_restricted(pdev): |
> + | cxl_handle_rdport_errors() |
> + | [RCH dport RAS first] |
> + | |
> + | port = find_cxl_port_by_dev( |
> + | &pdev->dev, NULL) |
> + | dport = cxl_find_dport_by_dev( |
> + | port, &pdev->dev) |
> + | [dport NULL for EP/USP; set RP/DSP] |
check the alignment here as well. A couple of extra spaces in the
lines above I think.
> + | |
> + | cxl_handle_proto_error() |
> + +--------------------------------------+
> + | |
> + v v
> + +-----------------+ +--------------------+
> + | CE | | UCE |
> + | cxl_handle_ | | cxl_do_recovery() |
> + | cor_ras() | | read RAS status |
> + | trace + clear | | trace + panic |
> + +-----------------+ +--------------------+
> +
> +cxl_do_recovery() reads the CXL RAS uncorrectable status register.
> +If UE bits are set, it emits the trace event and panics. If no bits
> +are set (e.g. RAS mapped but error already cleared), it logs a
> +diagnostic and defers to AER recovery.
> +
> +
> +Severity policy
> +===============
> +
> +**CE** - cxl_handle_cor_ras() reads the CXL RAS correctable status
> +register, clears set bits, and emits a cxl_aer_correctable_error
> +trace event. No recovery action.
> +
> +**UCE (non-fatal, and fatal on Root Port/Downstream Port)** - cxl_do_recovery() reads the CXL RAS
Wrap needs an update here.
> +uncorrectable status register. If UE bits are set, the kernel panics.
> +CXL.cachemem traffic cannot be safely recovered once an uncorrectable
> +error is signaled; continuing risks silent data corruption across
> +interleaved HDM regions. This panic policy applies to the native AER
> +path. On firmware-first (CPER/GHES) platforms the CPER handler emits
> +trace events only and does not call cxl_do_recovery().
^ permalink raw reply
* Re: [RFC] cxl: Device protocol AER injection
From: Jonathan Cameron @ 2026-07-21 0:46 UTC (permalink / raw)
To: Bowman, Terry
Cc: Ashok Raj, Bjorn Helgaas, Dan Williams, Dave Jiang, Ira Weiny,
Len Brown, Rafael J . Wysocki, Robert Richter, linux-acpi,
linux-cxl, linux-doc, linux-kernel, linux-pci, linuxppc-dev,
Alejandro Lucero, Alison Schofield, Ankit Agrawal, Ard Biesheuvel,
Ben Cheatham, Borislav Petkov, Breno Leitao, Davidlohr Bueso,
Fabio M . De Francesco, Gregory Price, Hanjun Guo,
Jonathan Corbet, Kees Cook, Kuppuswamy Sathyanarayanan, Li Ming,
Mahesh J Salgaonkar, Mauro Carvalho Chehab, Oliver O'Halloran,
Shiju Jose, Shuah Khan, Shuai Xue, Smita Koralahalli, Tony Luck,
Vishal Verma
In-Reply-To: <62c40932-0f9c-4a67-8ff6-4bc7ba24f40e@amd.com>
> >
> > Why is this an RFC rather than a final proposal? There should always
> > be something to give the reviewer that info in the patch description.
> > I'd actually be tempted to throw a cover letter in to have somewhere
> > out of the way to put that information.
> >
> > Is it simply because it only makes sense once the other seris lands.
> >
>
> The immediate priority was providing a testing procedure for the v18 series.
> I wasn't sure how the design would be received. For instance, the to_einj_ras_base()
> could be moved into cxl-test as a to_ras_base() mock implemented function or
> it could remain in ras.c (or maybe even ras_einj.c) outside of cxl-mock. I'm
> looking forward to Alison's review and comments.
>
> My preference is to introduce core/ras_einj,c and add these changes. This
> would be to isolate all the changes except for: cxl_ras_einj_init(), cxl_ras_einj_exit(),
> and to_einj_ras_base(). I've started moving forward with making these changes
> knowing the direction could change once this receives more reviews.
>
> Also worth discussing is the commandline takes multiple parameters for
> a single sysfs file which I know isn't acceptable by everyone. I personally
> like the interface as-is because its simpler to use in comparison to multiple
> files that must be set individually.
It is debugfs so rules are much more flexible than sysfs.
> >> ---
> >> drivers/cxl/Kconfig | 13 +++
> >> drivers/cxl/core/core.h | 21 ++++
> >> drivers/cxl/core/port.c | 2 +-
> >> drivers/cxl/core/ras.c | 208 ++++++++++++++++++++++++++++++++++
> >> drivers/cxl/core/ras_rch.c | 12 ++
> >> drivers/pci/pcie/aer_inject.c | 29 ++---
> >> include/linux/aer.h | 15 +++
> >> 7 files changed, 281 insertions(+), 19 deletions(-)
> >>
> >> diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
> >> index 80aeb0d556bd7..ef449228b2549 100644
> >> --- a/drivers/cxl/Kconfig
> >> +++ b/drivers/cxl/Kconfig
> >> @@ -238,6 +238,19 @@ config CXL_RAS
> >> def_bool y
> >> depends on ACPI_APEI_GHES && PCIEAER && CXL_BUS
> >>
> >> +config CXL_PROTO_AER_EINJ
> >> + bool "CXL: RAS Protocol Error Injection using AER EINJ"
> >> + depends on CXL_RAS
> >> + depends on PCIEAER_INJECT
> >
> > Do we think anyone who has CXL and PCIEAER_INJECT support will want
> > to carefully not build this? I'm just wondering if we can avoid asking
> > the question and base the built or not on the combination of those.
> >
>
> Good question: How do we incorporate this with the existing CXL EINJ
> functionality making them complementary and consistant? The existing
> EINJ is true ACPI injection but only supports RPs. The ACPI EINJ callouts
> are currrently in core/port.c. We should consider moving it into a common file
> such as core/ras_einj.c. With that move it will help force us merge the interfaces
> where/if possible and at least give central location for error RAS injection.
> I think folding the AER injection into PCIEAER_INJECT kernel config is
> reasonable but looking for others feedback.
>
I hadn't really thought about the injection method. Indeed raises
interesting questions of how it should be done. Add an ABI doc
in Documentation/ABI for next version.
> >
> >> +static DEFINE_MUTEX(cxl_aer_einj_mutex);
> >
> > Needs a comment for what data it is protecting.
> >
> >> +
> >> +struct cxl_aer_einj cxl_aer_einj = {
> >> + .lock = &cxl_aer_einj_mutex,
> >> +};
> >> +
> >> +static const char cxl_aer_einj_usage[] =
> >> + "ssss:bb:dd.f [UCE|CE] AER_STATUS RAS_STATUS [RCH]\n";
> >> +
> >> +static int cxl_aer_inject_error(struct pci_dev *pdev, bool correctable,
> >> + u32 aer_status, u32 ras_status)
> >> +{
> >> + /* RCD errors are signaled as internal errors on the associated RCEC */
> >> + if (pci_pcie_type(pdev) == PCI_EXP_TYPE_RC_END) {
> >> + if (!pdev->rcec)
> >> + return -ENODEV;
> >> + pdev = pdev->rcec;
> >> + }
> >> +
> >> + struct aer_error_inj einj = {
> >> + .bus = pdev->bus->number,
> >> + .dev = PCI_SLOT(pdev->devfn),
> >> + .fn = PCI_FUNC(pdev->devfn),
> >> + .domain = pci_domain_nr(pdev->bus),
> >> + };
> >> + int ret;
> >> + int aer_offset;
> >> + int ras_offset;
> >> +
> >> + if (correctable) {
> >> + einj.cor_status = aer_status | PCI_ERR_COR_INTERNAL;
> >> + aer_offset = PCI_ERR_COR_STATUS / sizeof(u32);
> >> + ras_offset = CXL_RAS_CORRECTABLE_STATUS_OFFSET / sizeof(u32);
> >
> > Given these are offsets into cxl_aer_einj.aer_registers / ras_registers
> > can we use sizeof(*cxl_aer_einj.aer_registers) etc
> >
> We could but that assumes the CEs are book ending the register blocks. And this would
> be inconsistent with UCE case below, right? Tell me if I misunderstaood the question.
I just meant to replace those sizeof(u32) with something that indicates
where the size is coming from. Applies equally below.
> >
> >> +
> >> +static void __iomem *to_einj_ras_base(struct cxl_port *port, struct cxl_dport *dport)
> >> +{
> >> + if (dport) {
> >> + if (cxl_aer_einj.is_rch) {
> >> + if (cxl_aer_einj.dev == dport->dport_dev) {
> >> + cxl_aer_einj.dev = NULL;
> >> + return (__force void __iomem *)cxl_aer_einj.ras_registers;
> >
> > Given the output of this is always force cast, maybe move that force up to the caller?
> >
>
> I think that works if it remains an internal helper and isn't made a mock function to to_ras_base().
> If to_einj_ras_base() is used as a mock than it would require changing the to_ras_base()
> as well.
>
> >> + }
> >> + } else {
> >> + if (cxl_aer_einj.dev == dport->dport_dev) {
> >> + pci_dev_put(to_pci_dev(cxl_aer_einj.dev));
> >
> > Not locally obvious why a thing called to_einj_ras_base should put anything it didn't
> > get. I think this needs a restructure to more obviously be tidying up references
> > that were held over the queue. At very leads needs a comment.
> > /* Reference held from X no longer needed so drop */
> >
>
> The ref was incremented in cxl_aer_einj_write() on invoking injection. The ref is
> decremented here after its usage. RCHs are excluded because they dont have a SBDF.
Just add minimal reference for that.
>
> >> + cxl_aer_einj.dev = NULL;
> >> + return (__force void __iomem *)cxl_aer_einj.ras_registers;
> >> + }
>
> >> + if (cxl_aer_einj.dev) {
> >> + void __iomem *einj = to_einj_ras_base(port, dport);
> >> + if (einj)
> >> + return einj;
> >> + }
> >> +#endif
> >> +
> >> if (dport)
> >> return dport->regs.ras;
> >>
> >> @@ -458,10 +656,20 @@ void cxl_ras_init(void)
> >> cxl_cper_register_prot_err_work(&cxl_cper_prot_err_work);
> >> cxl_register_proto_err_work(&cxl_proto_err_work,
> >> cxl_proto_err_do_flush);
> >> +#if IS_ENABLED(CONFIG_CXL_PROTO_AER_EINJ)
> >> + cxl_ras_create_debugfs(cxl_debugfs);
> > stub that in a header.
> >
>
> Actually, during rework today I moved it to cxl_ras_einj_init(),
> a new function. Thoughts?
Sounds good.
> >> diff --git a/drivers/pci/pcie/aer_inject.c b/drivers/pci/pcie/aer_inject.c
> >> index 09bfc7194ef31..b313adef680ae 100644
> >> --- a/drivers/pci/pcie/aer_inject.c
> >> +++ b/drivers/pci/pcie/aer_inject.c
> >> @@ -14,6 +14,7 @@
> >>
> >> #define dev_fmt(fmt) "aer_inject: " fmt
> >>
> >> +#include <linux/aer.h>
> >> #include <linux/module.h>
> >> #include <linux/init.h>
> >> #include <linux/interrupt.h>
> >> @@ -31,19 +32,6 @@
> >> static bool aer_mask_override;
> >> module_param(aer_mask_override, bool, 0);
> >>
> >> -struct aer_error_inj {
> >> - u8 bus;
> >> - u8 dev;
> >> - u8 fn;
> >> - u32 uncor_status;
> >> - u32 cor_status;
> >> - u32 header_log0;
> >> - u32 header_log1;
> >> - u32 header_log2;
> >> - u32 header_log3;
> >> - u32 domain;
> >> -};
> >> -
> >> struct aer_error {
> >> struct list_head list;
> >> u32 domain;
> >> @@ -316,7 +304,7 @@ static int pci_bus_set_aer_ops(struct pci_bus *bus)
> >> return 0;
> >> }
> >>
> >> -static int aer_inject(struct aer_error_inj *einj)
> >> +int aer_inject(struct aer_error_inj *einj)
> >> {
> >> struct aer_error *err, *rperr;
> >> struct aer_error *err_alloc = NULL, *rperr_alloc = NULL;
> >> @@ -332,10 +320,14 @@ static int aer_inject(struct aer_error_inj *einj)
> >> dev = pci_get_domain_bus_and_slot(einj->domain, einj->bus, devfn);
> >> if (!dev)
> >> return -ENODEV;
> >> - rpdev = pcie_find_root_port(dev);
> >> - /* If Root Port not found, try to find an RCEC */
> >> - if (!rpdev)
> >> - rpdev = dev->rcec;
> >> + if (pci_pcie_type(dev) == PCI_EXP_TYPE_RC_EC)
> >
> > { }
> > as the else is multiline (see coding standard)
> >
>
> Ok
>
> > Maybe need a comment for why it might be an RCEC for injection.
> > Is this an RCH specific path where there is nothing else to target?
> >
> >> + rpdev = dev;
> >> + else {
> >> + rpdev = pcie_find_root_port(dev);
> >> + /* If Root Port not found, try to find an RCEC */
> >> + if (!rpdev)
> >> + rpdev = dev->rcec;
> >> + }
> >> if (!rpdev) {
> >> pci_err(dev, "Neither Root Port nor RCEC found\n");
> >> ret = -ENODEV;
> >> @@ -482,6 +474,7 @@ static int aer_inject(struct aer_error_inj *einj)
> >> pci_dev_put(dev);
> >> return ret;
> >> }
> >> +EXPORT_SYMBOL_GPL(aer_inject);
> > I wonder if we want to restrict this to specific modules?
> >
> > One for Bjorn probably.
> >
> Because right now it injects AER to any PCI device.
>
> Also, worth discussing is the commandline currently uses multiple parameters for
> a single sysfs file. I mentioned this at the top.
>
>
> Thanks for reviewing Jonathan.
>
> -Terry
>
^ permalink raw reply
* RE: [PATCH RESEND v4 net-next 13/14] net: enetc: use alloc_etherdev_mqs() to create netdev for VF driver
From: Wei Fang (OSS) @ 2026-07-21 2:01 UTC (permalink / raw)
To: Joe Damato, Wei Fang (OSS)
Cc: Claudiu Manoil, Vladimir Oltean, Clark Wang,
andrew+netdev@lunn.ch, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, linux@armlinux.org.uk,
Wei Fang, chleroy@kernel.org, maxime.chevallier@bootlin.com,
imx@lists.linux.dev, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <al5cgs6CqVKsy/gh@devvm20253.cco0.facebook.com>
> On Mon, Jul 20, 2026 at 09:43:15AM +0800, wei.fang@oss.nxp.com wrote:
> > From: Wei Fang <wei.fang@nxp.com>
> >
> > The VF driver uses alloc_etherdev_mq() with ENETC_MAX_NUM_TXQS as the
> > queue count, which forces the TX and RX queue counts to be equal and
> > uses a compile-time constant rather than the actual hardware capability.
> >
> > After enetc_get_si_caps() is called, si->num_tx_rings and
> > si->num_rx_rings reflect the actual number of rings assigned to the VF
> > by the PF. For the ENETC VF on LS1028A and the upcoming i.MX95/94, their
> > SoCs have no more than 6 CPUs, and the number of TX/RX rings allocated
> > to the VF is less than 8.
> >
> > Therefore, switch to alloc_etherdev_mqs() so that the TX and RX queue
> > counts are set independently, each capped at ENETC_MAX_NUM_TXQS, based
> > on the actual number of rings assigned to the VF by the PF.
> >
> > Note that if future SoCs have more than 6 CPUs and more than 6 RX rings
> > allocated to VFs, the size of the int_vector array in struct
> > enetc_ndev_priv will need to be modified. Similarly, if more than 8 TX
> > rings are allocated to each int_vector, ENETC_MAX_NUM_TXQS will also
> > need to be modified.
> >
> > Signed-off-by: Wei Fang <wei.fang@nxp.com>
> > ---
> > drivers/net/ethernet/freescale/enetc/enetc_vf.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/enetc/enetc_vf.c
> b/drivers/net/ethernet/freescale/enetc/enetc_vf.c
> > index 9cdb0a4d6baf..7dcb4a0246f5 100644
> > --- a/drivers/net/ethernet/freescale/enetc/enetc_vf.c
> > +++ b/drivers/net/ethernet/freescale/enetc/enetc_vf.c
> > @@ -317,7 +317,14 @@ static int enetc_vf_probe(struct pci_dev *pdev,
> >
> > enetc_get_si_caps(si);
> >
> > - ndev = alloc_etherdev_mq(sizeof(*priv), ENETC_MAX_NUM_TXQS);
> > + /* Currently, the supported SoCs have a max of 6 CPUs and the VFs
> > + * have less than 6 RX/TX rings. So no issues for these supported
> > + * SoCs, but for future SoCs which have more CPUs or more TX/RX
> > + * rings, all the related logic needs to be improved.
> > + */
> > + ndev = alloc_etherdev_mqs(sizeof(*priv),
> > + min(si->num_tx_rings,
> ENETC_MAX_NUM_TXQS),
> > + min(si->num_rx_rings,
> ENETC_MAX_NUM_TXQS));
>
> Code looks right, but looks almost like a typo. I guess it would read nicer if
> ENETC_MAX_NUM_RXQS existed?
Yes, ENETC_MAX_NUM_RXQS is clearer. However, ENETC_MAX_NUM_RXQS
does not exist; in fact, their values are equal, and we plan to remove
ENETC_MAX_NUM_TXQS later, so a new macro ENETC_MAX_NUM_RXQS
was not added in this patch.
>
> That said:
>
> Reviewed-by: Joe Damato <joe@dama.to>
^ permalink raw reply
* Re: [PATCH RESEND v4 net-next 05/14] net: enetc: use PCI device name for debugfs directory
From: Joe Damato @ 2026-07-20 13:46 UTC (permalink / raw)
To: wei.fang
Cc: claudiu.manoil, vladimir.oltean, xiaoning.wang, andrew+netdev,
davem, edumazet, kuba, pabeni, linux, wei.fang, chleroy,
maxime.chevallier, imx, netdev, linux-kernel, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20260720014317.1059359-6-wei.fang@oss.nxp.com>
On Mon, Jul 20, 2026 at 09:43:07AM +0800, wei.fang@oss.nxp.com wrote:
> From: Wei Fang <wei.fang@nxp.com>
>
> enetc_create_debugfs() is called right after register_netdev(), at which
> point ndev->name still holds the format "eth%d" (e.g., eth0) rather than
> the final assigned name (e.g., via udev rules).
>
> Use pci_name() instead of netdev_name() to name the debugfs directory.
> The PCI device name is unique, stable, and available from the start,
> making it a more reliable identifier for the debugfs entry. Therefore,
> the observable debugfs path from something like
> /sys/kernel/debug/eth0/mac_filter to a PCI BDF-style path such as
> /sys/kernel/debug/0002:00:00.0/mac_filter.
>
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---
> drivers/net/ethernet/freescale/enetc/enetc4_debugfs.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_debugfs.c b/drivers/net/ethernet/freescale/enetc/enetc4_debugfs.c
> index 4a769d9e5679..be378bf8f74d 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc4_debugfs.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc4_debugfs.c
> @@ -81,10 +81,9 @@ DEFINE_SHOW_ATTRIBUTE(enetc_mac_filter);
>
> void enetc_create_debugfs(struct enetc_si *si)
> {
> - struct net_device *ndev = si->ndev;
> struct dentry *root;
>
> - root = debugfs_create_dir(netdev_name(ndev), NULL);
> + root = debugfs_create_dir(pci_name(si->pdev), NULL);
> if (IS_ERR(root))
> return;
>
Reviewed-by: Joe Damato <joe@dama.to>
^ permalink raw reply
* Re: [PATCH RESEND v4 net-next 14/14] net: enetc: use kzalloc_flex() for enetc_psfp_gate allocation
From: Joe Damato @ 2026-07-20 13:48 UTC (permalink / raw)
To: wei.fang
Cc: claudiu.manoil, vladimir.oltean, xiaoning.wang, andrew+netdev,
davem, edumazet, kuba, pabeni, linux, wei.fang, chleroy,
maxime.chevallier, imx, netdev, linux-kernel, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20260720014317.1059359-15-wei.fang@oss.nxp.com>
On Mon, Jul 20, 2026 at 09:43:16AM +0800, wei.fang@oss.nxp.com wrote:
> From: Wei Fang <wei.fang@nxp.com>
>
> Replace the open-coded struct_size() + kzalloc() pattern with the
> kzalloc_flex() helper when allocating struct enetc_psfp_gate. This
> removes the intermediate entries_size local variable and makes the
> allocation site more concise.
>
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---
> drivers/net/ethernet/freescale/enetc/enetc_qos.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_qos.c b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
> index 7b17bca24f26..2aa0fcaafcd2 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_qos.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_qos.c
> @@ -1135,7 +1135,6 @@ static int enetc_psfp_parse_clsflower(struct enetc_ndev_priv *priv,
> struct flow_action_entry *entry;
> struct action_gate_entry *e;
> u8 sfi_overwrite = 0;
> - int entries_size;
> int i, err;
>
> if (f->common.chain_index >= priv->psfp_cap.max_streamid) {
> @@ -1242,8 +1241,7 @@ static int enetc_psfp_parse_clsflower(struct enetc_ndev_priv *priv,
> goto free_filter;
> }
>
> - entries_size = struct_size(sgi, entries, entryg->gate.num_entries);
> - sgi = kzalloc(entries_size, GFP_KERNEL);
> + sgi = kzalloc_flex(*sgi, entries, entryg->gate.num_entries);
> if (!sgi) {
> err = -ENOMEM;
> goto free_filter;
Reviewed-by: Joe Damato <joe@dama.to>
^ permalink raw reply
* Re: [PATCH RESEND v4 net-next 01/14] net: enetc: extract common helpers for MAC promiscuous mode setting
From: Joe Damato @ 2026-07-20 14:02 UTC (permalink / raw)
To: wei.fang
Cc: claudiu.manoil, vladimir.oltean, xiaoning.wang, andrew+netdev,
davem, edumazet, kuba, pabeni, linux, wei.fang, chleroy,
maxime.chevallier, imx, netdev, linux-kernel, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20260720014317.1059359-2-wei.fang@oss.nxp.com>
On Mon, Jul 20, 2026 at 09:43:03AM +0800, wei.fang@oss.nxp.com wrote:
> From: Wei Fang <wei.fang@nxp.com>
>
> The PSIPMMR (Port Station Interface Promiscuous MAC Mode Register) in
> ENETC v4 has the same bit layout as the PSIPMR register in ENETC v1: bit
> n controls unicast promiscuous mode for SI n, and bit (n + 16) controls
> multicast promiscuous mode for SI n. The only difference between the two
> hardware generations is the register address offset.
>
> Since the register functionality is identical, the MAC promiscuous mode
> setting code can be shared between ENETC v1 and v4 drivers.
>
> Rename ENETC_PSIPMR to ENETC_PSIPMMR in enetc_hw.h to match the actual
> register name used in the reference manual, and extract two new common
> helper functions, enetc_set_si_uc_promisc() and
> enetc_set_si_mc_promisc(), into enetc_pf_common.c. These helpers select
> the correct register offset based on the hardware revision via
> is_enetc_rev1().
>
> Remove the v4-specific enetc4_pf_set_si_mac_promisc() function from
> enetc4_pf.c and the duplicate PSIPMMR_SI_MAC_UP/MP macro definitions
> from enetc4_hw.h, as they are now superseded by the shared code.
>
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---
> .../net/ethernet/freescale/enetc/enetc4_hw.h | 2 -
> .../net/ethernet/freescale/enetc/enetc4_pf.c | 21 +--------
> .../ethernet/freescale/enetc/enetc_ethtool.c | 2 +-
> .../net/ethernet/freescale/enetc/enetc_hw.h | 7 +--
> .../net/ethernet/freescale/enetc/enetc_pf.c | 11 ++---
> .../freescale/enetc/enetc_pf_common.c | 44 +++++++++++++++++++
> .../freescale/enetc/enetc_pf_common.h | 2 +
> 7 files changed, 56 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_hw.h b/drivers/net/ethernet/freescale/enetc/enetc4_hw.h
> index f18437556a0e..6a8f2ed56017 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc4_hw.h
> +++ b/drivers/net/ethernet/freescale/enetc/enetc4_hw.h
> @@ -69,8 +69,6 @@
>
> /* Port Station interface promiscuous MAC mode register */
> #define ENETC4_PSIPMMR 0x200
> -#define PSIPMMR_SI_MAC_UP(a) BIT(a) /* a = SI index */
> -#define PSIPMMR_SI_MAC_MP(a) BIT((a) + 16)
[...]
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_hw.h b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> index bf99b65d7598..66bfda60da9c 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
> @@ -180,9 +180,10 @@ enum enetc_bdr_type {TX, RX};
> #define ENETC_PMR_PSPEED_1000M BIT(9)
> #define ENETC_PMR_PSPEED_2500M BIT(10)
> #define ENETC_PSR 0x0004 /* RO */
> -#define ENETC_PSIPMR 0x0018
> -#define ENETC_PSIPMR_SET_UP(n) BIT(n) /* n = SI index */
> -#define ENETC_PSIPMR_SET_MP(n) BIT((n) + 16)
> +#define ENETC_PSIPMMR 0x0018
> +#define PSIPMMR_SI_MAC_UP(n) BIT(n) /* n = SI index */
> +#define PSIPMMR_SI_MAC_MP(n) BIT((n) + 16)
I probably would have fixed the leading spaces when copying/pasting the macro,
but that seems like a nit.
I read the rest of the code a few times and it looks right to me.
Reviewed-by: Joe Damato <joe@dama.to>
^ permalink raw reply
* Re: [PATCH RESEND v4 net-next 09/14] net: enetc: open-code enetc4_set_default_si_vlan_promisc()
From: Joe Damato @ 2026-07-20 15:26 UTC (permalink / raw)
To: wei.fang
Cc: claudiu.manoil, vladimir.oltean, xiaoning.wang, andrew+netdev,
davem, edumazet, kuba, pabeni, linux, wei.fang, chleroy,
maxime.chevallier, imx, netdev, linux-kernel, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20260720014317.1059359-10-wei.fang@oss.nxp.com>
On Mon, Jul 20, 2026 at 09:43:11AM +0800, wei.fang@oss.nxp.com wrote:
> From: Wei Fang <wei.fang@nxp.com>
>
> The function enetc4_set_default_si_vlan_promisc() is only called once,
> from enetc4_configure_port_si(). Open-code the loop at the call site
> and remove the single-use wrapper.
>
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---
> drivers/net/ethernet/freescale/enetc/enetc4_pf.c | 15 +++------------
> 1 file changed, 3 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> index 859b02f5170a..505e4abf6c37 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc4_pf.c
> @@ -307,17 +307,6 @@ static void enetc4_pf_set_si_vlan_promisc(struct enetc_hw *hw, int si, bool en)
> enetc_port_wr(hw, ENETC4_PSIPVMR, val);
> }
>
> -static void enetc4_set_default_si_vlan_promisc(struct enetc_pf *pf)
> -{
> - struct enetc_hw *hw = &pf->si->hw;
> - int num_si = pf->caps.num_vsi + 1;
> - int i;
> -
> - /* enforce VLAN promiscuous mode for all SIs */
> - for (i = 0; i < num_si; i++)
> - enetc4_pf_set_si_vlan_promisc(hw, i, true);
> -}
> -
> /* Allocate the number of MSI-X vectors for per SI. */
> static void enetc4_set_si_msix_num(struct enetc_pf *pf)
> {
> @@ -361,7 +350,9 @@ static void enetc4_configure_port_si(struct enetc_pf *pf)
> /* Outer VLAN tag will be used for VLAN filtering */
> enetc_port_wr(hw, ENETC4_PSIVLANFMR, PSIVLANFMR_VS);
>
> - enetc4_set_default_si_vlan_promisc(pf);
> + /* Enforce VLAN promiscuous mode for all SIs */
> + for (int i = 0; i < pf->caps.num_vsi + 1; i++)
> + enetc4_pf_set_si_vlan_promisc(hw, i, true);
>
> /* Disable SI MAC multicast & unicast promiscuous */
> enetc_port_wr(hw, ENETC4_PSIPMMR, 0);
Reviewed-by: Joe Damato <joe@dama.to>
^ permalink raw reply
* Re: [PATCH RESEND v4 net-next 11/14] net: enetc: move enetc_set_si_vlan_promisc() to enetc_pf_common.c
From: Joe Damato @ 2026-07-20 15:28 UTC (permalink / raw)
To: wei.fang
Cc: claudiu.manoil, vladimir.oltean, xiaoning.wang, andrew+netdev,
davem, edumazet, kuba, pabeni, linux, wei.fang, chleroy,
maxime.chevallier, imx, netdev, linux-kernel, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20260720014317.1059359-12-wei.fang@oss.nxp.com>
On Mon, Jul 20, 2026 at 09:43:13AM +0800, wei.fang@oss.nxp.com wrote:
> From: Wei Fang <wei.fang@nxp.com>
>
> The PSIPVMR in ENETC v4 has the same bit layout and functionality as the
> PSIPVMR register in ENETC v1: bit n (n <= 15) controls VLAN promiscuous
> mode for SI n. The only difference between the two hardware generations
> is the register address offset.
>
> Since the register functionality is identical, the VLAN promiscuous mode
> setting code can be shared between ENETC v1 and v4 drivers.
>
> Move enetc_set_si_vlan_promisc() from enetc_pf.c to enetc_pf_common.c
> and export it so that it can be shared between the two drivers. Add a
> revision check using is_enetc_rev1() to select the correct register
> offset (ENETC_PSIPVMR for v1 and ENETC4_PSIPVMR for v4) while keeping
> the same logic.
>
> Remove the v4-specific enetc4_pf_set_si_vlan_promisc() from enetc4_pf.c
> and replace its call site with the new common enetc_set_si_vlan_promisc()
> to eliminate code duplication.
>
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---
> .../net/ethernet/freescale/enetc/enetc4_pf.c | 17 ++------------
> .../net/ethernet/freescale/enetc/enetc_pf.c | 16 --------------
> .../freescale/enetc/enetc_pf_common.c | 22 +++++++++++++++++++
> .../freescale/enetc/enetc_pf_common.h | 1 +
> 4 files changed, 25 insertions(+), 31 deletions(-)
Reviewed-by: Joe Damato <joe@dama.to>
^ permalink raw reply
* Re: [PATCH RESEND v4 net-next 13/14] net: enetc: use alloc_etherdev_mqs() to create netdev for VF driver
From: Joe Damato @ 2026-07-20 17:36 UTC (permalink / raw)
To: wei.fang
Cc: claudiu.manoil, vladimir.oltean, xiaoning.wang, andrew+netdev,
davem, edumazet, kuba, pabeni, linux, wei.fang, chleroy,
maxime.chevallier, imx, netdev, linux-kernel, linuxppc-dev,
linux-arm-kernel
In-Reply-To: <20260720014317.1059359-14-wei.fang@oss.nxp.com>
On Mon, Jul 20, 2026 at 09:43:15AM +0800, wei.fang@oss.nxp.com wrote:
> From: Wei Fang <wei.fang@nxp.com>
>
> The VF driver uses alloc_etherdev_mq() with ENETC_MAX_NUM_TXQS as the
> queue count, which forces the TX and RX queue counts to be equal and
> uses a compile-time constant rather than the actual hardware capability.
>
> After enetc_get_si_caps() is called, si->num_tx_rings and
> si->num_rx_rings reflect the actual number of rings assigned to the VF
> by the PF. For the ENETC VF on LS1028A and the upcoming i.MX95/94, their
> SoCs have no more than 6 CPUs, and the number of TX/RX rings allocated
> to the VF is less than 8.
>
> Therefore, switch to alloc_etherdev_mqs() so that the TX and RX queue
> counts are set independently, each capped at ENETC_MAX_NUM_TXQS, based
> on the actual number of rings assigned to the VF by the PF.
>
> Note that if future SoCs have more than 6 CPUs and more than 6 RX rings
> allocated to VFs, the size of the int_vector array in struct
> enetc_ndev_priv will need to be modified. Similarly, if more than 8 TX
> rings are allocated to each int_vector, ENETC_MAX_NUM_TXQS will also
> need to be modified.
>
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---
> drivers/net/ethernet/freescale/enetc/enetc_vf.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc_vf.c b/drivers/net/ethernet/freescale/enetc/enetc_vf.c
> index 9cdb0a4d6baf..7dcb4a0246f5 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc_vf.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc_vf.c
> @@ -317,7 +317,14 @@ static int enetc_vf_probe(struct pci_dev *pdev,
>
> enetc_get_si_caps(si);
>
> - ndev = alloc_etherdev_mq(sizeof(*priv), ENETC_MAX_NUM_TXQS);
> + /* Currently, the supported SoCs have a max of 6 CPUs and the VFs
> + * have less than 6 RX/TX rings. So no issues for these supported
> + * SoCs, but for future SoCs which have more CPUs or more TX/RX
> + * rings, all the related logic needs to be improved.
> + */
> + ndev = alloc_etherdev_mqs(sizeof(*priv),
> + min(si->num_tx_rings, ENETC_MAX_NUM_TXQS),
> + min(si->num_rx_rings, ENETC_MAX_NUM_TXQS));
Code looks right, but looks almost like a typo. I guess it would read nicer if
ENETC_MAX_NUM_RXQS existed?
That said:
Reviewed-by: Joe Damato <joe@dama.to>
^ permalink raw reply
* [PATCH v3 1/5] powerpc/rtas: Handle ibm,open-errinjct return format
From: Narayana Murty N @ 2026-07-21 3:38 UTC (permalink / raw)
To: mahesh, maddy, mpe, christophe.leroy, gregkh, oohall, npiggin
Cc: linuxppc-dev, linux-kernel, tyreld, vaibhav, sbhat, ganeshgr,
sourabhjain, haren, nnmlinux, thuth
In-Reply-To: <20260721033815.5300-1-nnmlinux@linux.ibm.com>
PAPR specifies that ibm,open-errinjct has a unique return-cell layout:
rets[0] = injection session token (output parameter)
rets[1] = status code
This differs from every other RTAS call, where:
rets[0] = status code
rets[1..] = output parameters
As a result, the existing rtas_call() convention — return value is the
RTAS status, outputs[] receives the non-status output values — must be
preserved while correctly extracting status from rets[1] for this one
call.
Add rtas_token_is_open_errinjct() and rtas_status_from_args() helpers.
rtas_status_from_args() selects the correct status cell based on the
token, and the output-copy loop in rtas_call() is updated so that for
ibm,open-errinjct:
rtas_call() return = rets[1] (RTAS status)
outputs[0] = rets[0] (session token)
For all other calls the behaviour is unchanged: return value is rets[0]
and outputs[] receives rets[1..nret-1].
The sys_rtas userspace path is not modified: copy_to_user() still
copies raw RTAS return cells (rets[0..nret-1]) to userspace.
Callers passing a single output int (nret == 2) are safe because we
write at most nret-1 values into outputs[], never all nret cells.
Also move the '/* A -1 return code...*/' comment to immediately precede
the if (ret == -1) check it describes, and remove the redundant stale
else branch that re-assigned ret.
Reference: OpenPOWER PAPR documentation
https://files.openpower.foundation/s/XFgfMaqLMD5Bcm8
Signed-off-by: Narayana Murty N <nnmlinux@linux.ibm.com>
---
arch/powerpc/kernel/rtas.c | 51 ++++++++++++++++++++++++++++++++------
1 file changed, 44 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 8d81c1e7a8db..27d53f34494d 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -1117,6 +1117,29 @@ static bool token_is_restricted_errinjct(s32 token)
token == rtas_function_token(RTAS_FN_IBM_ERRINJCT);
}
+/**
+ * rtas_token_is_open_errinjct() - Test whether @token identifies ibm,open-errinjct.
+ */
+static bool rtas_token_is_open_errinjct(int token)
+{
+ return token == rtas_function_token(RTAS_FN_IBM_OPEN_ERRINJCT);
+}
+
+/**
+ * rtas_status_from_args() - Extract the RTAS status code from a completed
+ * call's return-cell array.
+ *
+ * For ibm,open-errinjct the status lives in rets[1]; for every other
+ * RTAS function it lives in rets[0].
+ */
+static int rtas_status_from_args(int token, struct rtas_args *args, int nret)
+{
+ if (rtas_token_is_open_errinjct(token) && nret > 1)
+ return be32_to_cpu(args->rets[1]);
+
+ return nret > 0 ? be32_to_cpu(args->rets[0]) : 0;
+}
+
/**
* rtas_call() - Invoke an RTAS firmware function.
* @token: Identifies the function being invoked.
@@ -1213,15 +1236,29 @@ int rtas_call(int token, int nargs, int nret, int *outputs, ...)
va_rtas_call_unlocked(args, token, nargs, nret, list);
va_end(list);
- /* A -1 return code indicates that the last command couldn't
- be completed due to a hardware error. */
- if (be32_to_cpu(args->rets[0]) == -1)
+ ret = rtas_status_from_args(token, args, nret);
+
+ /*
+ * A -1 return code indicates that the last command couldn't
+ * be completed due to a hardware error.
+ */
+ if (ret == -1)
buff_copy = __fetch_rtas_last_error(NULL);
- if (nret > 1 && outputs != NULL)
- for (i = 0; i < nret-1; ++i)
- outputs[i] = be32_to_cpu(args->rets[i + 1]);
- ret = (nret > 0) ? be32_to_cpu(args->rets[0]) : 0;
+ if (nret > 1 && outputs != NULL) {
+ if (rtas_token_is_open_errinjct(token)) {
+ /*
+ * ibm,open-errinjct: rets[0]=session token, rets[1]=status.
+ * Expose session token in outputs[0]; skip rets[1] (status).
+ */
+ outputs[0] = be32_to_cpu(args->rets[0]);
+ for (i = 1; i < nret - 1; ++i)
+ outputs[i] = be32_to_cpu(args->rets[i + 1]);
+ } else {
+ for (i = 0; i < nret - 1; ++i)
+ outputs[i] = be32_to_cpu(args->rets[i + 1]);
+ }
+ }
lockdep_unpin_lock(&rtas_lock, cookie);
raw_spin_unlock_irqrestore(&rtas_lock, flags);
--
2.54.0
^ permalink raw reply related
* [PATCH v3 0/5] powerpc/eeh: Add RTAS-based error injection support on pSeries
From: Narayana Murty N @ 2026-07-21 3:38 UTC (permalink / raw)
To: mahesh, maddy, mpe, christophe.leroy, gregkh, oohall, npiggin
Cc: linuxppc-dev, linux-kernel, tyreld, vaibhav, sbhat, ganeshgr,
sourabhjain, haren, nnmlinux, thuth
The pSeries EEH error-injection backend currently implements a limited
software-generated MMIO failure and does not use the error-injection
services provided by RTAS.
This series replaces that implementation with the PAPR-defined RTAS
workflow based on:
- ibm,open-errinjct
- ibm,errinjct
- ibm,close-errinjct
The implementation opens an RTAS error-injection session, prepares the
firmware work buffer, performs the requested injection, and closes the
session on both success and failure paths.
The existing EEH userspace ABI is preserved. EEH_ERR_TYPE_32 and
EEH_ERR_TYPE_64 continue to represent generic 32-bit and 64-bit IOA
bus-error injection requests. The pSeries backend maps these values to
the corresponding RTAS error types, while the PowerNV backend
explicitly maps them to the corresponding OPAL types.
Additional generic EEH error types (EEH_ERR_TYPE_RECOVERED_SPECIAL_EVENT,
EEH_ERR_TYPE_CORRUPTED_PAGE, and the cache/TLB corruption types) are now
defined in the UAPI header and mapped explicitly to RTAS firmware encodings
by the pSeries backend. Platform backends that do not support a valid
generic type return -EOPNOTSUPP.
No existing userspace ABI values are changed.
The current injection path can be exercised for VFIO-assigned devices
through VFIO_EEH_PE_INJECT_ERR. The guest or userspace VFIO application
continues to use the same generic EEH type and function values,
independent of whether the host platform uses RTAS or OPAL.
The series also handles the unusual return format of
ibm,open-errinjct:
rets[0] = error-injection session token
rets[1] = RTAS status
rtas_call() now returns rets[1] as the status and places the session
token in outputs[0], preserving the normal kernel rtas_call()
convention.
sys_rtas() is intentionally unchanged because it exposes the raw RTAS
return cells to userspace. Userspace therefore continues to receive
the session token and status in their PAPR-defined positions.
The RTAS work buffer is allocated during RTAS initialization below:
min(ppc64_rma_size, RTAS_INSTANTIATE_MAX)
using the same accessible-memory limit used for rtas_rmo_buf. The
kernel populates the buffer through its virtual mapping but passes its
physical address to firmware.
The complete open, inject and close sequence is serialized with a
mutex. RTAS busy and extended-delay return values are handled for all
three calls. A session token value of zero is accepted, and session
state is tracked independently from the token value.
The patches are organised as follows:
Handle the special ibm,open-errinjct return format in rtas_call().
Allocate an RTAS-accessible error-injection work buffer.
Add pSeries RTAS parameter validation and buffer encoding helpers.
Implement RTAS-based pSeries EEH error injection.
Explicitly map generic EEH error types to OPAL types on PowerNV.
Testing was performed on PowerVM with firmware providing the RTAS
error-injection services and with the corresponding QEMU support:
https://lore.kernel.org/qemu-devel/20260520095446.64206-1-nnmlinux@linux.ibm.com/
Signed-off-by: Narayana Murty N <nnmlinux@linux.ibm.com>
Narayana Murty N (5):
powerpc/rtas: Handle ibm,open-errinjct return format
powerpc/rtas: Allocate ibm,errinjct buffer below RTAS limit
powerpc/pseries/eeh: Add RTAS error validation helpers
powerpc/pseries/eeh: Implement RTAS-based EEH error injection
powerpc/powernv/eeh: Map VFIO EEH error injection to OPAL
arch/powerpc/include/asm/rtas.h | 26 ++
arch/powerpc/include/uapi/asm/eeh.h | 24 +-
arch/powerpc/kernel/rtas.c | 68 +++-
arch/powerpc/platforms/powernv/eeh-powernv.c | 36 +-
arch/powerpc/platforms/pseries/eeh_pseries.c | 367 +++++++++++++++++--
5 files changed, 483 insertions(+), 38 deletions(-)
Change Log:
V2 -> V3:
* Fixed ibm,open-errinjct return handling to correctly process firmware responses.
* Allocate the error-injection buffer in RTAS-accessible memory instead of general kernel memory.
* Pass the physical address of the error-injection buffer to firmware (previously incorrect address type).
* Accept session token zero as a valid token (previously rejected erroneously).
* Handle RTAS busy and extended-delay return codes for open-inject, and close calls.
* Simplified the validation helper — reduced complexity and removed redundant checks.
* Simplified the buffer-preparation helper for cleaner, more maintainable code.
* Validate that all required RTAS tokens are present before attempting to open a session.
* Added explicit generic EEH-to-OPAL error-type mapping for the PowerNV platform.
v1 -> v2: https://lore.kernel.org/all/20260527072433.94510-1-nnmlinux@linux.ibm.com/
* Addressed all review comments from Sourabh Jain
- Removed unnecessary empty line in rtas_call()
- Enhanced comment to explain PAPR specification requirements
- Corrected misleading comment about output handling
- Improved else block comment for better code clarity
* Fixed kernel test robot warnings
- Fixed kernel-doc warning for __maybe_unused parameter
- Confirmed sparse warnings are false positives (correct endianness handling)
* Added PowerNV platform abstraction layer (new Patch 5)
- Maps EEH error types to OPAL-specific types
- Simplifies type handling by direct variable update
* Improved code comments and documentation throughout
* Added Reported-by tags for kernel test robot findings
* Split into logical 5-patch series for better review
RFC -> v1: https://lore.kernel.org/all/20251205094510.4671-1-nnmlinux@linux.ibm.com/
* Initial 4-patch series
* Fixed PAPR ibm,open-errinjct output format (token,status order)
* Added pr_fmt handling for EEH subsystem compatibility
* Implemented comprehensive validation helpers
RFC: https://lore.kernel.org/all/20251107091009.43034-1-nnmlinux@linux.ibm.com/
* Initial RFC implementation
--
2.54.0
^ permalink raw reply
* [PATCH v3 2/5] powerpc/rtas: Allocate ibm,errinjct buffer below RTAS limit
From: Narayana Murty N @ 2026-07-21 3:38 UTC (permalink / raw)
To: mahesh, maddy, mpe, christophe.leroy, gregkh, oohall, npiggin
Cc: linuxppc-dev, linux-kernel, tyreld, vaibhav, sbhat, ganeshgr,
sourabhjain, haren, nnmlinux, thuth
In-Reply-To: <20260721033815.5300-1-nnmlinux@linux.ibm.com>
ibm,errinjct requires a caller-provided work buffer whose physical
address is passed to RTAS firmware.
A static C array such as:
char rtas_errinjct_buf[1024] __aligned(SZ_1K);
only guarantees alignment, not physical placement. If the array lands
above the RTAS-safe range (RTAS_INSTANTIATE_MAX, 1 GB) or above 4 GB,
RTAS receives a truncated or invalid address and the injection call
will fail silently or corrupt memory.
Instead, introduce rtas_errinjct_buf as a global unsigned long storing
the physical address allocated during rtas_initialize() using
memblock_phys_alloc_range() with the same rtas_region upper bound used
for rtas_rmo_buf. This matches the existing placement model for
RTAS-accessible buffers and guarantees the physical address fits in 32
bits.
Usage:
void *buf = __va(rtas_errinjct_buf); /* kernel VA to fill */
u32 buf_phys = lower_32_bits(rtas_errinjct_buf); /* PA for RTAS */
Always check upper_32_bits(rtas_errinjct_buf) == 0 before passing the
lower 32 bits to RTAS.
Add rtas_errinjct_mutex to serialise the complete open-session /
inject / close-session firmware call sequence. A mutex is required
because the sequence involves multiple rtas_call() invocations with
possible busy/extended-delay retries that may sleep.
Signed-off-by: Narayana Murty N <nnmlinux@linux.ibm.com>
---
arch/powerpc/include/asm/rtas.h | 26 ++++++++++++++++++++++++++
arch/powerpc/kernel/rtas.c | 17 +++++++++++++++++
2 files changed, 43 insertions(+)
diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
index d046bbd5017d..59e3c1296018 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -4,6 +4,7 @@
#ifdef __KERNEL__
#include <linux/mutex.h>
+#include <linux/sizes.h>
#include <linux/spinlock.h>
#include <asm/page.h>
#include <asm/rtas-types.h>
@@ -519,6 +520,31 @@ int rtas_get_error_log_max(void);
extern spinlock_t rtas_data_buf_lock;
extern char rtas_data_buf[RTAS_DATA_BUF_SIZE];
+/*
+ * RTAS error-injection work buffer.
+ *
+ * ibm,errinjct requires a caller-provided work buffer whose physical
+ * address is passed to firmware. A static C array only guarantees
+ * alignment, not physical placement; if it lands above the RTAS-safe
+ * range or above 4 GB, RTAS receives a truncated or bogus address.
+ *
+ * rtas_errinjct_buf stores the physical address allocated during
+ * rtas_initialize() using memblock_phys_alloc_range() with the same
+ * rtas_region upper bound used for rtas_rmo_buf, matching the existing
+ * placement model for RTAS-accessible buffers.
+ *
+ * Use __va(rtas_errinjct_buf) to obtain the kernel virtual address for
+ * filling the buffer, and lower_32_bits(rtas_errinjct_buf) to pass the
+ * physical address to RTAS (after checking upper_32_bits() == 0).
+ *
+ * rtas_errinjct_mutex must be held across the complete
+ * ibm,open-errinjct / ibm,errinjct / ibm,close-errinjct sequence.
+ */
+#define RTAS_ERRINJCT_BUF_SIZE SZ_1K
+
+extern unsigned long rtas_errinjct_buf;
+extern struct mutex rtas_errinjct_mutex;
+
/* RMO buffer reserved for user-space RTAS use */
extern unsigned long rtas_rmo_buf;
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 27d53f34494d..7883f973e8c9 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -769,6 +769,17 @@ EXPORT_SYMBOL_GPL(rtas_data_buf);
unsigned long rtas_rmo_buf;
+/*
+ * Physical address of the ibm,errinjct work buffer. Allocated during
+ * rtas_initialize() using memblock_phys_alloc_range() below rtas_region
+ * so the address fits in 32 bits and is safe to pass to RTAS firmware.
+ */
+unsigned long rtas_errinjct_buf;
+EXPORT_SYMBOL_GPL(rtas_errinjct_buf);
+
+DEFINE_MUTEX(rtas_errinjct_mutex);
+EXPORT_SYMBOL_GPL(rtas_errinjct_mutex);
+
/*
* If non-NULL, this gets called when the kernel terminates.
* This is done like this so rtas_flash can be a module.
@@ -2109,6 +2120,12 @@ void __init rtas_initialize(void)
panic("ERROR: RTAS: Failed to allocate %lx bytes below %pa\n",
PAGE_SIZE, &rtas_region);
+ rtas_errinjct_buf = memblock_phys_alloc_range(RTAS_ERRINJCT_BUF_SIZE,
+ SZ_1K, 0, rtas_region);
+ if (!rtas_errinjct_buf)
+ panic("ERROR: RTAS: Failed to allocate %lu bytes below %pa\n",
+ (unsigned long)RTAS_ERRINJCT_BUF_SIZE, &rtas_region);
+
rtas_work_area_reserve_arena(rtas_region);
}
--
2.54.0
^ permalink raw reply related
* [PATCH v3 3/5] powerpc/pseries/eeh: Add RTAS error validation helpers
From: Narayana Murty N @ 2026-07-21 3:38 UTC (permalink / raw)
To: mahesh, maddy, mpe, christophe.leroy, gregkh, oohall, npiggin
Cc: linuxppc-dev, linux-kernel, tyreld, vaibhav, sbhat, ganeshgr,
sourabhjain, haren, nnmlinux, thuth
In-Reply-To: <20260721033815.5300-1-nnmlinux@linux.ibm.com>
Add validation helpers and buffer-preparation infrastructure used by
the RTAS-based EEH error injection implementation.
Changes relative to the original submission:
- pr_fmt unconditionally defined at the top of the file, not wrapped
in an #ifndef guard.
- RTAS error-type constants (RTAS_ERR_TYPE_*) are defined as file-local
macros in eeh_pseries.c, not in the UAPI header. They are internal
PAPR firmware type codes, not generic EEH ABI values.
- validate_corrupted_page() drops the unused 'pe' parameter entirely
(no __maybe_unused). The mask value is silently accepted since it
is not meaningful for this error type.
- validate_ioa_bus_error() is removed; callers invoke
validate_addr_mask_in_pe() directly.
- validate_err_type() and validate_special_event() and
validate_corrupted_page() are marked static inline.
- prepare_errinjct_buffer() takes an explicit 'void *buf' pointer
(kernel virtual address) instead of accessing rtas_errinjct_buf
directly, making it easier to test and keeping the global out of
the helper.
- kernel-doc added for prepare_errinjct_buffer() including a Locking:
line stating the caller must hold rtas_errinjct_mutex.
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202512101130.EYUo0oZx-lkp@intel.com/
Signed-off-by: Narayana Murty N <nnmlinux@linux.ibm.com>
---
arch/powerpc/include/uapi/asm/eeh.h | 24 +-
arch/powerpc/platforms/pseries/eeh_pseries.c | 250 +++++++++++++++++++
2 files changed, 271 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/uapi/asm/eeh.h b/arch/powerpc/include/uapi/asm/eeh.h
index 3b5c47ff3fc4..2680b22f8917 100644
--- a/arch/powerpc/include/uapi/asm/eeh.h
+++ b/arch/powerpc/include/uapi/asm/eeh.h
@@ -15,9 +15,27 @@
#define EEH_PE_STATE_STOPPED_DMA 4 /* Stopped DMA only */
#define EEH_PE_STATE_UNAVAIL 5 /* Unavailable */
-/* EEH error types and functions */
-#define EEH_ERR_TYPE_32 0 /* 32-bits error */
-#define EEH_ERR_TYPE_64 1 /* 64-bits error */
+/*
+ * EEH error types.
+ *
+ * EEH_ERR_TYPE_32 and EEH_ERR_TYPE_64 are the original ABI values and must
+ * not be renumbered. The additional types below are generic identifiers for
+ * error-injection types supported by pSeries RTAS. Platform backends may
+ * return -EOPNOTSUPP for valid generic types that are not supported by their
+ * firmware.
+ */
+#define EEH_ERR_TYPE_32 0 /* 32-bits error */
+#define EEH_ERR_TYPE_64 1 /* 64-bits error */
+#define EEH_ERR_TYPE_RECOVERED_SPECIAL_EVENT 0x03
+#define EEH_ERR_TYPE_CORRUPTED_PAGE 0x04
+#define EEH_ERR_TYPE_CORRUPTED_DCACHE_START 0x09
+#define EEH_ERR_TYPE_CORRUPTED_DCACHE_END 0x0a
+#define EEH_ERR_TYPE_CORRUPTED_ICACHE_START 0x0b
+#define EEH_ERR_TYPE_CORRUPTED_ICACHE_END 0x0c
+#define EEH_ERR_TYPE_CORRUPTED_TLB_START 0x0d
+#define EEH_ERR_TYPE_CORRUPTED_TLB_END 0x0e
+
+/* EEH error functions */
#define EEH_ERR_FUNC_MIN 0
#define EEH_ERR_FUNC_LD_MEM_ADDR 0 /* Memory load */
#define EEH_ERR_FUNC_LD_MEM_DATA 1
diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index b12ef382fec7..25aad86c696d 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
@@ -1,4 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-or-later
+#define pr_fmt(fmt) "EEH: " fmt
/*
* The file intends to implement the platform dependent EEH operations on pseries.
* Actually, the pseries platform is built based on RTAS heavily. That means the
@@ -31,8 +32,27 @@
#include <asm/io.h>
#include <asm/machdep.h>
#include <asm/ppc-pci.h>
+#include <linux/mutex.h>
#include <asm/rtas.h>
+/*
+ * PAPR RTAS firmware error-injection type codes.
+ *
+ * These are internal firmware encodings used by ibm,errinjct. They are
+ * NOT generic EEH ABI values. Only the types actually handled by
+ * prepare_errinjct_buffer() are defined here.
+ */
+#define RTAS_ERR_TYPE_RECOVERED_SPECIAL_EVENT 0x03
+#define RTAS_ERR_TYPE_CORRUPTED_PAGE 0x04
+#define RTAS_ERR_TYPE_IOA_BUS_ERROR 0x07
+#define RTAS_ERR_TYPE_CORRUPTED_DCACHE_START 0x09
+#define RTAS_ERR_TYPE_CORRUPTED_DCACHE_END 0x0a
+#define RTAS_ERR_TYPE_CORRUPTED_ICACHE_START 0x0b
+#define RTAS_ERR_TYPE_CORRUPTED_ICACHE_END 0x0c
+#define RTAS_ERR_TYPE_CORRUPTED_TLB_START 0x0d
+#define RTAS_ERR_TYPE_CORRUPTED_TLB_END 0x0e
+#define RTAS_ERR_TYPE_IOA_BUS_ERROR_64 0x0f
+
/* RTAS tokens */
static int ibm_set_eeh_option;
static int ibm_set_slot_reset;
@@ -786,6 +806,236 @@ static int pseries_notify_resume(struct eeh_dev *edev)
}
#endif
+/**
+ * validate_addr_mask_in_pe() - Validate that addr+mask fall within PE BARs
+ * @pe: EEH PE containing one or more PCI devices
+ * @addr: Address to validate
+ * @mask: Address mask to validate
+ *
+ * Checks that @addr is mapped into a BAR/MMIO region of any device
+ * belonging to the PE. If @mask is non-zero, ensures it is consistent
+ * with @addr.
+ *
+ * Return: 0 if valid, RTAS_INVALID_PARAMETER on failure.
+ */
+static int validate_addr_mask_in_pe(struct eeh_pe *pe, unsigned long addr,
+ unsigned long mask)
+{
+ struct eeh_dev *edev, *tmp;
+ struct pci_dev *pdev;
+ int bar;
+ resource_size_t bar_start, bar_len;
+ bool valid = false;
+
+ /* nothing to validate */
+ if (addr == 0 && mask == 0)
+ return 0;
+
+ eeh_pe_for_each_dev(pe, edev, tmp) {
+ pdev = eeh_dev_to_pci_dev(edev);
+ if (!pdev)
+ continue;
+
+ for (bar = 0; bar < PCI_NUM_RESOURCES; bar++) {
+ bar_start = pci_resource_start(pdev, bar);
+ bar_len = pci_resource_len(pdev, bar);
+
+ if (!bar_len)
+ continue;
+
+ if (addr >= bar_start && addr < (bar_start + bar_len)) {
+ if ((addr & mask) != addr) {
+ pr_err("Mask 0x%lx invalid for addr 0x%lx in BAR[%d] range 0x%llx-0x%llx\n",
+ mask, addr, bar,
+ (unsigned long long)bar_start,
+ (unsigned long long)(bar_start + bar_len));
+ return RTAS_INVALID_PARAMETER;
+ }
+ pr_debug("addr=0x%lx mask=0x%lx validated in BAR[%d] of %s\n",
+ addr, mask, bar, pci_name(pdev));
+ valid = true;
+ }
+ }
+ }
+
+ if (!valid) {
+ pr_err("addr=0x%lx not within any BAR of any device in PE\n",
+ addr);
+ return RTAS_INVALID_PARAMETER;
+ }
+
+ return 0;
+}
+
+/**
+ * validate_special_event() - Validate parameters for special-event injection
+ * @addr: Address parameter (must be zero for this type)
+ * @mask: Mask parameter (must be zero for this type)
+ *
+ * Return: 0 if valid, RTAS_INVALID_PARAMETER otherwise.
+ */
+static inline int validate_special_event(unsigned long addr, unsigned long mask)
+{
+ if (addr || mask) {
+ pr_err("special-event injection must not specify addr/mask\n");
+ return RTAS_INVALID_PARAMETER;
+ }
+ return 0;
+}
+
+/**
+ * validate_corrupted_page() - Validate parameters for corrupted-page injection
+ * @addr: Physical page address (required, must be non-zero)
+ *
+ * The mask value is not meaningful for this error type and is ignored.
+ *
+ * Return: 0 if valid, RTAS_INVALID_PARAMETER otherwise.
+ */
+static inline int validate_corrupted_page(unsigned long addr)
+{
+ if (!addr) {
+ pr_err("corrupted-page injection requires non-zero addr\n");
+ return RTAS_INVALID_PARAMETER;
+ }
+ return 0;
+}
+
+/**
+ * pseries_eeh_type_to_rtas() - Map a generic EEH error type to its RTAS encoding.
+ * @type: Generic EEH error type (EEH_ERR_TYPE_*)
+ *
+ * Translates a userspace-visible generic EEH error type to the corresponding
+ * PAPR RTAS firmware type code. Every supported value appears as an explicit
+ * case; coincidental equality between generic EEH and RTAS values is not relied
+ * upon.
+ *
+ * Return: RTAS_ERR_TYPE_* value on success, -EINVAL for unsupported types.
+ */
+static int pseries_eeh_type_to_rtas(int type)
+{
+ switch (type) {
+ case EEH_ERR_TYPE_32:
+ return RTAS_ERR_TYPE_IOA_BUS_ERROR;
+ case EEH_ERR_TYPE_64:
+ return RTAS_ERR_TYPE_IOA_BUS_ERROR_64;
+ case EEH_ERR_TYPE_RECOVERED_SPECIAL_EVENT:
+ return RTAS_ERR_TYPE_RECOVERED_SPECIAL_EVENT;
+ case EEH_ERR_TYPE_CORRUPTED_PAGE:
+ return RTAS_ERR_TYPE_CORRUPTED_PAGE;
+ case EEH_ERR_TYPE_CORRUPTED_DCACHE_START:
+ return RTAS_ERR_TYPE_CORRUPTED_DCACHE_START;
+ case EEH_ERR_TYPE_CORRUPTED_DCACHE_END:
+ return RTAS_ERR_TYPE_CORRUPTED_DCACHE_END;
+ case EEH_ERR_TYPE_CORRUPTED_ICACHE_START:
+ return RTAS_ERR_TYPE_CORRUPTED_ICACHE_START;
+ case EEH_ERR_TYPE_CORRUPTED_ICACHE_END:
+ return RTAS_ERR_TYPE_CORRUPTED_ICACHE_END;
+ case EEH_ERR_TYPE_CORRUPTED_TLB_START:
+ return RTAS_ERR_TYPE_CORRUPTED_TLB_START;
+ case EEH_ERR_TYPE_CORRUPTED_TLB_END:
+ return RTAS_ERR_TYPE_CORRUPTED_TLB_END;
+ default:
+ return -EINVAL;
+ }
+}
+
+/**
+ * prepare_errinjct_buffer() - Build ibm,errinjct work buffer
+ * @buf: RTAS error-injection work buffer (kernel virtual address)
+ * @pe: EEH PE associated with the injection target
+ * @rtas_type: PAPR firmware error-injection type after generic EEH-to-RTAS
+ * translation (RTAS_ERR_TYPE_*)
+ * @func: Error function selector
+ * @addr: Target address, if applicable
+ * @mask: Address mask, if applicable
+ *
+ * Zeroes @buf and populates it according to the PAPR layout for @rtas_type.
+ * Performs inline parameter validation for each error type.
+ *
+ * Locking: Caller must hold rtas_errinjct_mutex.
+ *
+ * Return: 0 on success, RTAS_INVALID_PARAMETER on invalid input.
+ */
+static int prepare_errinjct_buffer(void *buf, struct eeh_pe *pe,
+ int rtas_type, int func,
+ unsigned long addr, unsigned long mask)
+{
+ __be64 *buf64 = (__be64 *)buf;
+ __be32 *buf32 = (__be32 *)buf;
+
+ memset(buf, 0, RTAS_ERRINJCT_BUF_SIZE);
+
+ switch (rtas_type) {
+ case RTAS_ERR_TYPE_RECOVERED_SPECIAL_EVENT:
+ /* func: 1 = non-persistent, 2 = persistent */
+ if (func < 1 || func > 2)
+ return RTAS_INVALID_PARAMETER;
+ if (validate_special_event(addr, mask))
+ return RTAS_INVALID_PARAMETER;
+ buf32[0] = cpu_to_be32(func);
+ break;
+
+ case RTAS_ERR_TYPE_CORRUPTED_PAGE:
+ if (validate_corrupted_page(addr))
+ return RTAS_INVALID_PARAMETER;
+ buf32[0] = cpu_to_be32(upper_32_bits(addr));
+ buf32[1] = cpu_to_be32(lower_32_bits(addr));
+ break;
+
+ case RTAS_ERR_TYPE_IOA_BUS_ERROR:
+ if (func < EEH_ERR_FUNC_LD_MEM_ADDR || func > EEH_ERR_FUNC_MAX)
+ return RTAS_INVALID_PARAMETER;
+ if (upper_32_bits(addr) || upper_32_bits(mask)) {
+ pr_err("32-bit IOA injection cannot encode addr=%#lx mask=%#lx\n",
+ addr, mask);
+ return RTAS_INVALID_PARAMETER;
+ }
+ if (validate_addr_mask_in_pe(pe, addr, mask))
+ return RTAS_INVALID_PARAMETER;
+ buf32[0] = cpu_to_be32((u32)addr);
+ buf32[1] = cpu_to_be32((u32)mask);
+ buf32[2] = cpu_to_be32(pe->addr);
+ buf32[3] = cpu_to_be32(BUID_HI(pe->phb->buid));
+ buf32[4] = cpu_to_be32(BUID_LO(pe->phb->buid));
+ buf32[5] = cpu_to_be32(func);
+ break;
+
+ case RTAS_ERR_TYPE_IOA_BUS_ERROR_64:
+ if (func < EEH_ERR_FUNC_MIN || func > EEH_ERR_FUNC_MAX)
+ return RTAS_INVALID_PARAMETER;
+ if (validate_addr_mask_in_pe(pe, addr, mask))
+ return RTAS_INVALID_PARAMETER;
+ buf64[0] = cpu_to_be64(addr);
+ buf64[1] = cpu_to_be64(mask);
+ buf32[4] = cpu_to_be32(pe->addr);
+ buf32[5] = cpu_to_be32(BUID_HI(pe->phb->buid));
+ buf32[6] = cpu_to_be32(BUID_LO(pe->phb->buid));
+ buf32[7] = cpu_to_be32(func);
+ break;
+
+ case RTAS_ERR_TYPE_CORRUPTED_DCACHE_START:
+ case RTAS_ERR_TYPE_CORRUPTED_DCACHE_END:
+ case RTAS_ERR_TYPE_CORRUPTED_ICACHE_START:
+ case RTAS_ERR_TYPE_CORRUPTED_ICACHE_END:
+ buf32[0] = cpu_to_be32(lower_32_bits(addr));
+ buf32[1] = cpu_to_be32(lower_32_bits(mask));
+ break;
+
+ case RTAS_ERR_TYPE_CORRUPTED_TLB_START:
+ case RTAS_ERR_TYPE_CORRUPTED_TLB_END:
+ buf32[0] = cpu_to_be32(lower_32_bits(addr));
+ break;
+
+ default:
+ pr_err("unsupported RTAS error injection type 0x%x\n", rtas_type);
+ return RTAS_INVALID_PARAMETER;
+ }
+
+ pr_debug("errinjct buffer ready: rtas_type=0x%x func=%d addr=0x%lx mask=0x%lx\n",
+ rtas_type, func, addr, mask);
+ return 0;
+}
+
/**
* pseries_eeh_err_inject - Inject specified error to the indicated PE
* @pe: the indicated PE
--
2.54.0
^ permalink raw reply related
* [PATCH v3 4/5] powerpc/pseries/eeh: Implement RTAS-based EEH error injection
From: Narayana Murty N @ 2026-07-21 3:38 UTC (permalink / raw)
To: mahesh, maddy, mpe, christophe.leroy, gregkh, oohall, npiggin
Cc: linuxppc-dev, linux-kernel, tyreld, vaibhav, sbhat, ganeshgr,
sourabhjain, haren, nnmlinux, thuth
In-Reply-To: <20260721033815.5300-1-nnmlinux@linux.ibm.com>
Replace legacy MMIO error injection with full PAPR-compliant RTAS error
injection supporting 14+ error types via
- ibm,open-errinjct
- ibm,errinjct
- ibm,close-errinjct.
Key features:
- Complete open-session-inject-close cycle management
- Special handling for ibm,open-errinjct output format (token,status)
- Comprehensive buffer preparation per PAPR layouts
- All pr_* logging uses pr_fmt("EEH: ") prefix
Tested with corresponding QEMU patches:
https://lore.kernel.org/all/20251029150618.186803-1-nnmlinux@linux.ibm.com/
Signed-off-by: Narayana Murty N <nnmlinux@linux.ibm.com>
---
arch/powerpc/platforms/pseries/eeh_pseries.c | 121 +++++++++++++++----
1 file changed, 95 insertions(+), 26 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
index 25aad86c696d..d32a84009fdc 100644
--- a/arch/powerpc/platforms/pseries/eeh_pseries.c
+++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
@@ -1037,40 +1037,109 @@ static int prepare_errinjct_buffer(void *buf, struct eeh_pe *pe,
}
/**
- * pseries_eeh_err_inject - Inject specified error to the indicated PE
- * @pe: the indicated PE
- * @type: error type
- * @func: specific error type
- * @addr: address
- * @mask: address mask
- * The routine is called to inject specified error, which is
- * determined by @type and @func, to the indicated PE
+ * pseries_eeh_err_inject - Inject specified error into the indicated PE
+ * @pe: the indicated PE
+ * @type: generic EEH error type (EEH_ERR_TYPE_*)
+ * @func: error function selector (EEH_ERR_FUNC_*)
+ * @addr: target address, if applicable
+ * @mask: address mask, if applicable
+ *
+ * Implements the EEH error-injection callback for pseries using the RTAS
+ * ibm,open-errinjct / ibm,errinjct / ibm,close-errinjct firmware services.
+ *
+ * Generic EEH error types are translated to RTAS firmware type codes via
+ * pseries_eeh_type_to_rtas() before the injection session is opened.
+ * Unsupported generic types are rejected with -EINVAL before any RTAS call
+ * is made. Existing userspace is unaffected.
+ *
+ * Return: 0 on success, negative errno or RTAS error code on failure.
*/
static int pseries_eeh_err_inject(struct eeh_pe *pe, int type, int func,
unsigned long addr, unsigned long mask)
{
- struct eeh_dev *pdev;
-
- /* Check on PCI error type */
- if (type != EEH_ERR_TYPE_32 && type != EEH_ERR_TYPE_64)
- return -EINVAL;
+ int open_token, errinjct_token, close_token;
+ int session_token = 0;
+ bool session_open = false;
+ void *buf;
+ u32 buf_phys;
+ int rc;
+ int rtas_type;
- switch (func) {
- case EEH_ERR_FUNC_LD_MEM_ADDR:
- case EEH_ERR_FUNC_LD_MEM_DATA:
- case EEH_ERR_FUNC_ST_MEM_ADDR:
- case EEH_ERR_FUNC_ST_MEM_DATA:
- /* injects a MMIO error for all pdev's belonging to PE */
- pci_lock_rescan_remove();
- list_for_each_entry(pdev, &pe->edevs, entry)
- eeh_pe_inject_mmio_error(pdev->pdev);
- pci_unlock_rescan_remove();
- break;
- default:
+ /* Guard: buffer must fit in 32 bits for RTAS */
+ if (WARN_ON_ONCE(upper_32_bits(rtas_errinjct_buf)))
return -ERANGE;
+
+ /* Map generic EEH ABI to RTAS-internal error type */
+ rtas_type = pseries_eeh_type_to_rtas(type);
+ if (rtas_type < 0) {
+ pr_err("unsupported EEH error type %#x\n", type);
+ return rtas_type;
}
- return 0;
+ /* Verify all three RTAS tokens are available before touching firmware */
+ open_token = rtas_function_token(RTAS_FN_IBM_OPEN_ERRINJCT);
+ errinjct_token = rtas_function_token(RTAS_FN_IBM_ERRINJCT);
+ close_token = rtas_function_token(RTAS_FN_IBM_CLOSE_ERRINJCT);
+
+ if (open_token == RTAS_UNKNOWN_SERVICE ||
+ errinjct_token == RTAS_UNKNOWN_SERVICE ||
+ close_token == RTAS_UNKNOWN_SERVICE) {
+ pr_err("ibm,open/errinjct/close-errinjct not available\n");
+ return -ENODEV;
+ }
+
+ buf = __va(rtas_errinjct_buf);
+ buf_phys = lower_32_bits(rtas_errinjct_buf);
+
+ mutex_lock(&rtas_errinjct_mutex);
+
+ /* Step 1: open injection session */
+ do {
+ rc = rtas_call(open_token, 0, 2, &session_token);
+ } while (rtas_busy_delay(rc));
+
+ if (rc) {
+ pr_err("ibm,open-errinjct failed: status=%d\n", rc);
+ goto out_unlock;
+ }
+ session_open = true;
+
+ /* Step 2: prepare the work buffer */
+ rc = prepare_errinjct_buffer(buf, pe, rtas_type, func, addr, mask);
+ if (rc) {
+ pr_err("failed to prepare errinjct buffer: rc=%d\n", rc);
+ goto out_close;
+ }
+
+ /* Step 3: inject the error */
+ do {
+ rc = rtas_call(errinjct_token, 3, 1, NULL,
+ rtas_type, session_token, buf_phys);
+ } while (rtas_busy_delay(rc));
+
+ if (rc)
+ pr_err("ibm,errinjct failed: status=%d\n", rc);
+
+out_close:
+ /* Step 4: always close the session */
+ {
+ int close_rc;
+
+ if (session_open) {
+ do {
+ close_rc = rtas_call(close_token, 1, 1, NULL,
+ session_token);
+ } while (rtas_busy_delay(close_rc));
+
+ if (close_rc)
+ pr_warn("ibm,close-errinjct failed: status=%d\n",
+ close_rc);
+ }
+ }
+
+out_unlock:
+ mutex_unlock(&rtas_errinjct_mutex);
+ return rc;
}
static struct eeh_ops pseries_eeh_ops = {
--
2.54.0
^ permalink raw reply related
* [PATCH v3 5/5] powerpc/powernv/eeh: Map VFIO EEH error injection to OPAL
From: Narayana Murty N @ 2026-07-21 3:38 UTC (permalink / raw)
To: mahesh, maddy, mpe, christophe.leroy, gregkh, oohall, npiggin
Cc: linuxppc-dev, linux-kernel, tyreld, vaibhav, sbhat, ganeshgr,
sourabhjain, haren, nnmlinux, thuth
In-Reply-To: <20260721033815.5300-1-nnmlinux@linux.ibm.com>
The EEH error-injection interface passes the generic userspace ABI
values EEH_ERR_TYPE_32 and EEH_ERR_TYPE_64 to the platform backend.
The PowerNV backend currently compares those generic values directly
with OPAL_ERR_INJECT_TYPE_IOA_BUS_ERR and
OPAL_ERR_INJECT_TYPE_IOA_BUS_ERR64. Although the corresponding values
are currently numerically identical, this implicitly couples the
generic EEH ABI to the OPAL firmware encoding.
Explicitly translate the generic EEH error types to their OPAL
equivalents in pnv_eeh_err_inject(). Keep the platform-specific
encoding within the PowerNV backend and reject unsupported generic
types with -EINVAL.
No userspace ABI values are changed.
Signed-off-by: Narayana Murty N <nnmlinux@linux.ibm.com>
---
arch/powerpc/platforms/powernv/eeh-powernv.c | 36 +++++++++++++++++---
1 file changed, 32 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index db3370d1673c..b0bcd014a133 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -1169,11 +1169,39 @@ static int pnv_eeh_err_inject(struct eeh_pe *pe, int type, int func,
struct pnv_phb *phb = hose->private_data;
s64 rc;
- if (type != OPAL_ERR_INJECT_TYPE_IOA_BUS_ERR &&
- type != OPAL_ERR_INJECT_TYPE_IOA_BUS_ERR64) {
- pr_warn("%s: Invalid error type %d\n",
+ /*
+ * Map generic EEH error-type ABI values to OPAL-specific type codes.
+ * EEH_ERR_TYPE_32 and EEH_ERR_TYPE_64 are the only types supported by
+ * OPAL. Additional generic types defined in the UAPI header are valid
+ * for pSeries RTAS but unsupported here; return -EOPNOTSUPP for those.
+ * Unknown or invalid values return -EINVAL.
+ *
+ * Note: currently OPAL_ERR_INJECT_TYPE_IOA_BUS_ERR == 0 ==
+ * EEH_ERR_TYPE_32 and OPAL_ERR_INJECT_TYPE_IOA_BUS_ERR64 == 1 ==
+ * EEH_ERR_TYPE_64, but the explicit switch makes the coupling
+ * visible and allows the values to diverge independently.
+ */
+ switch (type) {
+ case EEH_ERR_TYPE_32:
+ type = OPAL_ERR_INJECT_TYPE_IOA_BUS_ERR;
+ break;
+ case EEH_ERR_TYPE_64:
+ type = OPAL_ERR_INJECT_TYPE_IOA_BUS_ERR64;
+ break;
+ case EEH_ERR_TYPE_RECOVERED_SPECIAL_EVENT:
+ case EEH_ERR_TYPE_CORRUPTED_PAGE:
+ case EEH_ERR_TYPE_CORRUPTED_DCACHE_START:
+ case EEH_ERR_TYPE_CORRUPTED_DCACHE_END:
+ case EEH_ERR_TYPE_CORRUPTED_ICACHE_START:
+ case EEH_ERR_TYPE_CORRUPTED_ICACHE_END:
+ case EEH_ERR_TYPE_CORRUPTED_TLB_START:
+ case EEH_ERR_TYPE_CORRUPTED_TLB_END:
+ pr_warn("%s: EEH error type %d not supported by OPAL\n",
__func__, type);
- return -ERANGE;
+ return -EOPNOTSUPP;
+ default:
+ pr_warn("%s: unsupported EEH error type %d\n", __func__, type);
+ return -EINVAL;
}
if (func < OPAL_ERR_INJECT_FUNC_IOA_LD_MEM_ADDR ||
--
2.54.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox