On Thu, 7 Sep 2023 14:19:55 -0500
Ben Cheatham <Benjamin.Cheatham@xxxxxxx> wrote:
> Add support for CXL EINJ error types for CXL 1.1 hosts added in ACPI
> v6.5. Because these error types target memory-mapped CXL 1.1 compliant
> downstream ports and not physical (normal/persistent) memory, these
> error types are not currently allowed through the memory range
> validation done by the EINJ driver.
>
> The MMIO address of a CXL 1.1 downstream port can be found in the
> cxl_rcrb_addr file in the corresponding dport directory under
> /sys/bus/cxl/devices/portX. CXL 1.1 error types follow the same
> procedure as a memory error type, but with param1 set to the
> downstream port MMIO address.
>
> Example usage:
> $ cd /sys/kernel/debug/apei/einj
> $ cat available_error_type
> 0x00000008 Memory Correctable
> 0x00000010 Memory Uncorrectable non-fatal
> 0x00000020 Memory Uncorrectable fatal
> 0x00000040 PCI Express Correctable
> 0x00000080 PCI Express Uncorrectable non-fatal
> 0x00000100 PCI Express Uncorrectable fatal
> 0x00008000 CXL.mem Protocol Correctable
> 0x00020000 CXL.mem Protocol Uncorrectable fatal
> $ echo 0x8000 > error_type
> $ echo 0xfffffffffffff000 > param2
> $ echo 0x3 > flags
>From einj.rst (and the ACPI spec) bit 0 here is
Processor APIC field valid. Why is that relevant here?
If it were you'd be writing param3 I think. So probably
harmless, but I think this should be 0x2 > flags
> $ cat /sys/bus/cxl/devices/portX/dportY/cxl_rcrb_addr
> 0xb2f00000
> $ echo 0xb2f00000 > param1
> $ echo 1 > error_inject
>
> Signed-off-by: Ben Cheatham <Benjamin.Cheatham@xxxxxxx>
Hi Ben,
Why bother with the config dependent build?
It doesn't save that much in code built and right now there
are no non ACPI CXL systems so in reality it's always built
anyway.
Otherwise this LGTM
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 001ab8742e21..f8f300496140 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -1122,6 +1122,25 @@ struct cxl_dport *devm_cxl_add_rch_dport(struct cxl_port *port,
> }
> EXPORT_SYMBOL_NS_GPL(devm_cxl_add_rch_dport, CXL);
>
> +#if IS_ENABLED(CONFIG_CXL_ACPI)
I'm not a particular fan processor magic down in c files.
Do we really care about the saving of not having this in builds where
CONFIG_CXL_ACPI isn't set? I'm thinking those don't really exist.
> +struct cxl_dport *cxl_find_rch_dport_by_rcrb(resource_size_t rcrb_base)
> +{
> + struct cxl_dport *dport;
> + unsigned long index;
> +
> + if (!cxl_root)
> + return ERR_PTR(-ENODEV);
> +
> + xa_for_each(&cxl_root->dports, index, dport)
> + if ((dport->rch && dport->rcrb.base != CXL_RESOURCE_NONE)
> + && dport->rcrb.base == rcrb_base)
> + return dport;
> +
> + return NULL;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_find_rch_dport_by_rcrb, CXL);
> +#endif
> +
> static int add_ep(struct cxl_ep *new)
> {
> struct cxl_port *port = new->dport->port;
