On 9/12/23 9:00 AM, Jonathan Cameron wrote:
> 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
>
I don't think it matters in this case, but I agree.
I'll change it to use 0x2 instead.
>> $ 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.
>
Ok, I'll remove the config dependencies. It'll make my life a
lot easier :).
> 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.
>
I don't like it much either, I did it as a quick fix for the build errors
from the kernel test bot.
Thanks,
Ben
>> +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;
>
