Re: [PATCH v2] ACPI, APEI, EINJ: Remove memory range validation for CXL error types

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

On 5/31/23 10:57 PM, Dan Williams wrote:
> Dan Williams wrote:
>> Hi Ben,
>>
>> Ben Cheatham wrote:
>>> From: Yazen Ghannam <yazen.ghannam@amd.com>
>>>
>>> V2 Changes:
>>>  - Added Link tags for links
>>>  - removed stray unused variable
>>>
>>> This patch is a follow up to the discussion at [1], and builds on Tony's
>>> CXL error patch at [2].
>>>
>>> The new CXL error types will use the Memory Address field in the
>>> SET_ERROR_TYPE_WITH_ADDRESS structure in order to target a CXL 1.1
>>> compliant memory-mapped Downstream port. The value of the Memory Address
>>> will be in the port's MMIO range, and it will not represent physical
>>> (normal or persistent) memory.
>>>
>>> Allow error injection for CXL 1.1 systems by skipping memory range
>>> validation for CXL error injection types.
>>
>> This just feels a bit too loose especially when the kernel has
>> the cxl_acpi driver to perform the enumeration of CXL root ports.
>>
>> I know that Terry and Robert are teaching the PCI AER core how to
>> coordinate with RCRB information [1] (I still need to go dig in deeper
>> on that set). I would expect ACPI EINJ could benefit from similar
>> coordination and validate these addresses.
>>
I haven't kept up with that patch set either, I'll take a look.

>> Now, is it any address in the downstream-port RCRB range that is valid,
>> or only the base?
I took another look at the ACPI 6.5 spec since it's been a while and it doesn't specify,
but when I tested this patch I think I only used the base address. I'll try out other
addresses in the range and see if they work as well.

> 
> Now that I say the above, I wonder if the legacy EINJ interface is even
> the right interface for CXL protocol error injection. It feels like
> something that should be relative to the given 'struct cxl_port'
> instance where the error is to be injected. Userspace should not need to
> deal in MMIO addresses, especially in the RCH case where the RCRB is
> difficult for userspace to enumerate.
I think that's a good idea. My first idea is to have sysfs files under the port that 
are basically a shortcut for an EINJ injection through the legacy interface. I think
it might be possible to have just the sysfs files and no legacy interface support,
but since I'll most likely be reusing the EINJ code I imagine the legacy interface 
will have to be updated anyway.