On Mon, May 10, 2021 at 7:58 AM Jonathan Cameron
[off-list ref] wrote:

On Fri, 7 May 2021 15:51:47 -0700
Dan Williams [off-list ref] wrote:

quoted

While CXL builds upon the PCI software model for dynamic enumeration and
control, a static platform component is required to bootstrap the CXL
memory layout. In addition to identifying the host bridges ACPI is
responsible for enumerating the CXL memory space that can be addressed
by decoders. This is similar to the requirement for ACPI to publish
resources reported by _CRS for PCI host bridges.

Introduce the cxl_root object as an abstract "port" into the CXL.mem
address space described by HDM decoders identified by the ACPI
CEDT.CHBS.

For now just establish the initial boilerplate and sysfs attributes, to
be followed by enumeration of the ports within the host bridge.

Note the allocation of CXL core device objects is split into separate
alloc and add steps in order to separate the alloc error path (kfree())
from the device add error path (put_device()).

Signed-off-by: Dan Williams <redacted>

Hi Dan

Just one bit in here that confused me (assuming I'm reading the code correctly).
You have is_visible for the dev_attr_supports_pmem etc to only show them if
the particular space supports that memory type. That's fine. You also have
the actual sysfs function checking the same flag to decide to return "1" or "0"
which would also be fine, but in combination it's rather odd as the sysfs
read function can never return "0" (sysfs attribute isn't visible in that
condition). Probably deserves at least a comment.

Ok.

That was deliberate since it's trivial to code and allows the
visibility policy to change without needing to go audit the attributes
that assumed invisibility. However, yes, it deserves a comment to save
brain cycles with that "hmm, that's odd" in the future.

This also needs some documentation for the new sysfs ABI
(Documentation/ABI/...) but that can be in a separate patch.

True.

Otherwise looks good to me.

Thanks.