On Wed, 30 Jul 2025 16:47:05 -0500
Ben Cheatham <Benjamin.Cheatham@xxxxxxx> wrote:
> Add the CXL isolation service, which will provide the necessary
> information to the PCIe portdrv and CXL drivers to map, setup, and
> handle CXL isolation interrupts.
>
> Add functions to get the CXL isolation MSI/-X interrupt vector
> from the PCIe portdrv.
>
> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@xxxxxxx>
Hmm. I feel a bit guilty I haven't gotten back to a rework of the
portdrv. To potentially make that easier (when I or someone else
get the time), would it work for you if this only did MSI-X?
That would let us handle this in a somewhat 'pluggable' way without
having to deal with the limitations of MSI wrt to dynamic interrupt
allocation.
> ---
> drivers/cxl/Kconfig | 14 +++++
> drivers/cxl/cxl.h | 4 ++
> drivers/pci/pcie/Makefile | 1 +
> drivers/pci/pcie/cxl_isolation.c | 87 ++++++++++++++++++++++++++++++++
> drivers/pci/pcie/portdrv.c | 1 +
> drivers/pci/pcie/portdrv.h | 18 ++++++-
> 6 files changed, 124 insertions(+), 1 deletion(-)
> create mode 100644 drivers/pci/pcie/cxl_isolation.c
>
> diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
> index 57274de54a45..537e1e8e13da 100644
> --- a/drivers/cxl/Kconfig
> +++ b/drivers/cxl/Kconfig
> @@ -247,4 +247,18 @@ config CXL_NATIVE_RAS
> If unsure, or if this kernel is meant for production environments,
> say Y.
>
> +config CXL_ISOLATION
> + bool "CXL.mem Isolation Support"
> + depends on PCIEPORTBUS
> + depends on CXL_BUS=PCIEPORTBUS
> + help
> + Enables the CXL.mem isolation PCIe port bus service driver. This
> + driver, in combination with the CXL driver core, is responsible
> + for managing CXL-capable PCIe root ports that undergo CXL.mem
> + error isolation due to either a CXL.mem transaction timeout or
> + link down condition. Without error isolation, either of these
> + conditions will trigger a system reset.
> +
> + If unsure say 'y'
I'd drop this last line or say something more about when it makes sense to enable.
> diff --git a/drivers/pci/pcie/cxl_isolation.c b/drivers/pci/pcie/cxl_isolation.c
> new file mode 100644
> index 000000000000..550f16271d1c
> --- /dev/null
> +++ b/drivers/pci/pcie/cxl_isolation.c
> @@ -0,0 +1,87 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * The CXL Isolation PCIe port service driver provides functions to allocate
> + * and set up CXL Timeout & Isolation interrupts (CXL 3.2 12.3). This driver
> + * does no actual interrupt handling, it only provides the information for
> + * the CXL driver to set up its own handling because the CXL driver is better
> + * equipped to handle isolation interrupts.
> + *
> + * Copyright (C) 2025, Advanced Micro Devices, Inc.
> + * All Rights Reserved.
> + *
> + * Author: Ben Cheatham <Benjamin.Cheatham@xxxxxxx>
> + */
> +
> +#include <linux/pci.h>
> +
> +#include "../../cxl/cxlpci.h"
> +#include "portdrv.h"
> +
> +static int get_isolation_intr_vec(u32 cap)
> +{
> + if (!FIELD_GET(CXL_ISOLATION_CAP_INTR_SUPP, cap) ||
> + !FIELD_GET(CXL_ISOLATION_CAP_MEM_ISO_SUPP, cap))
> + return -ENXIO;
> +
> + return FIELD_GET(CXL_ISOLATION_CAP_INTR_MASK, cap);
> +}
> +
> +int pcie_cxliso_get_intr_vec(struct pci_dev *dev, int *vec)
> +{
> + struct cxl_component_regs regs;
> + struct cxl_register_map map;
> + u32 cap;
> + int rc;
> +
> + rc = cxl_find_regblock(dev, CXL_REGLOC_RBI_COMPONENT, &map);
> + if (rc)
> + return rc;
> +
> + rc = cxl_setup_regs(&map);
> + if (rc)
> + return rc;
> +
> + if (!map.component_map.isolation.valid)
> + return -ENXIO;
> +
Add some comments here on why we go through this dance of temporary mapping.
(I have similar for the CPMU code I never finished upstreaming).
> + rc = cxl_map_component_regs(&map, ®s,
> + BIT(CXL_CM_CAP_CAP_ID_ISOLATION));
> + if (rc)
> + return rc;
> +
> + cap = readl(regs.isolation + CXL_ISOLATION_CAPABILITY_OFFSET);
> + rc = get_isolation_intr_vec(cap);
> + if (rc < 0) {
Probably use a goto given common shared stuff to do on exit.
> + cxl_unmap_component_regs(&map, ®s,
> + BIT(CXL_CM_CAP_CAP_ID_ISOLATION));
> + return rc;
> + }
> +
> + if (vec)
> + *vec = rc;
> +
> + cxl_unmap_component_regs(&map, ®s, BIT(CXL_CM_CAP_CAP_ID_ISOLATION));
> + return 0;
> +
> +}
> +
> +static int cxl_isolation_probe(struct pcie_device *dev)
> +{
> + if (!pcie_is_cxl(dev->port) || pcie_cxliso_get_intr_vec(dev->port, NULL))
The second call has rich error codes so better not to eat them.
if (!pcie_is_cxl(dev->port))
return -ENXIO;
rc = pcie_cxl_iso_get_intr_vec();
if (rc)
return rc;
> + return -ENXIO;
> +
> + pci_info(dev->port, "CXLISO: Signaling with IRQ %d\n", dev->irq);
I guess there is history of this for other portdrv services, but to me too noisy
for a normal boot and should be trivial to get from /proc/interrupts
> + return 0;
> +}
