Re: [PATCH v5 2/3] ACPI, APEI, EINJ: Add CXL 1.1 EINJ error type support

Ben Cheatham
2023-09-27T15:31:23Z

On 9/26/23 3:22 PM, Bjorn Helgaas wrote:
> On Mon, Sep 25, 2023 at 03:01:26PM -0500, Ben Cheatham 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 0x2 > flags
>> $ cat /sys/bus/cxl/devices/portX/dportY/cxl_rcrb_addr
>> 0xb2f00000
>> $ echo 0xb2f00000 > param1
>
> It seems sort of unpleasant to expose the physical base address of the
> RCRB. Is there any way to use a device address or other logical
> identifier instead and keep the actual MMIO address internal? E.g., a
> PCI device has a <domain>:<bus>:<device>.<function> address. I assume
> CXL addresses would look similar?
>

I have the MMIO address of the dport exposed here because the ACPI spec
says that the address is expected for CXL 1.1 errors and I wanted to match
the spec. For CXL 2.0 errors the SBDF is expected as you described.

>> $ echo 1 > error_inject
>> ...
>
>> +static int is_valid_cxl_addr(u64 addr)
>
> Maybe the function name should include a hint that this should be an
> RCRB address? But I don't claim to know the CXL architecture or
> nomenclature.
>

Good point, if I keep this function in v6 I'll make the name more
descriptive.

> Bjorn