On 1/10/24 4:10 PM, Dan Williams wrote: > Ben Cheatham wrote: > [..] >>>> + >>>> + dir = cxl_debugfs_create_dir(dev_name(dport->dport_dev)); >>>> + >>>> + debugfs_create_file("einj_inject", 0200, dir, dport, >>>> + &cxl_einj_inject_fops); >>> >>> I will note that I am little bit uneasy about this ACPI'ism escaping >>> into the common core, but he mitigation for me is that if some other >>> platform firmware invented a platform-firmware method for error inject >>> it would simply need to reuse the einj_cxl_ namespace to make it common. >>> >> >> I'll be honest; I'm not sure I understand the concern here, but that's probably >> just inexperience on my part. That being said, I don't mind changing it if you >> have any suggestions! >> > > Notice how the CXL subsystem organizes all the ACPI interface concerns > into drivers/cxl/acpi.c. There are some generic callbacks like the > @calc_hb argument to cxl_root_decoder_alloc(), but there are no direct > ties of the CXL core code to ACPI details. This separation allows, in > principle, a non-ACPI platform (like PowerPC OpenFirmware) to support > CXL without needing to unwind a bunch of CXL-ACPI specific stuff. > > This "cxl_einj" work is leaking an ACPI specific term "EINJ" into the > core code. The reason I am ok with this is that the ABI is still gated > on CONFIG_ACPI_APEI_EINJ. Also, we still have the option to require that > OpenFirmware error injection just call their user facing ABI "EINJ" as > well, even if it is a different name in the OpenFirmware specification. > To be clear though no one has sent patches for CXL support on anything > other than ACPI based platforms. Ok that makes sense, thanks for the explanation!