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

[Ben Cheatham <benjamin.cheatham@amd.com>]

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