On 5/31/23 10:57 PM, Dan Williams wrote: > Dan Williams wrote: >> Hi Ben, >> >> Ben Cheatham wrote: >>> From: Yazen Ghannam <yazen.ghannam@xxxxxxx> >>> >>> 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.
