Re: [PATCH 14/14] cxl/port: Enable HDM Capability after validating DVSEC Ranges

[Dan Williams <dan.j.williams@intel.com>]

On Mon, May 16, 2022 at 12:32 PM <Ariel.Sibley@microchip.com> wrote:
>
> > > > Previously, the cxl_mem driver was relying on platform-firmware to set
> > > > "mem_enable". That is an invalid assumption as there is no requirement
> > > > that platform-firmware sets the bit before the driver sees a device,
> > > > especially in hot-plug scenarios. Additionally, ACPI-platforms that
> > > > support CXL 2.0 devices also support the ACPI CEDT (CXL Early Discovery
> > > > Table). That table outlines the platform permissible address ranges for
> > > > CXL operation. So, there is a need for the driver to set "mem_enable",
> > > > and there is information available to determine the validity of the CXL
> > > > DVSEC Ranges. Note that the DVSEC Ranges can not be shut off completely.
> > > > They always decode at least 256MB if "mem_enable" is set and the HDM
> > > > Decoder capability is disabled.
> > >
> > > Regarding "can not be shut off completely", CXL 2.0 Section 8.1.3.8.4
> > > has this statement:
> > > ---
> > > A CXL.mem capable device that does not implement CXL HDM Decoder
> > > Capability registers directs host accesses to an address A within its
> > > local HDM memory if the following two equations are satisfied -
> > > Memory_Base[63:28] <= (A >> 28) < Memory_Base[63:28]+Memory_Size[63:28]
> > > Memory_Active AND Mem_Enable=1
> > > ---
> > >
> > > Per the above, if a device advertises Memory_Size = 0, then the first
> > > equation (second part) can never be true. So, a device advertising
> > > Memory_Size = 0 will effectively shut off this range, even if
> > > Mem_Enable = 1 and the HDM Decoder capability is disabled.
> >
> > Hmm, so now I am confused as to why the spec goes on to say:
> >
> > "If the address A is not backed by real memory (e.g. a device with
> > less than 256 MB of
> > memory), a device that does not implement CXL HDM Decoder Capability registers
> > must handle those accesses gracefully i.e. return all 1’s on reads and
> > drop writes."
> >
> > This is what led me to conclude that even though Memory_Size is 0, the
> > device is still actively decoding [0, 256MB).
>
> My interpretation of that statement is that such a device would need to
> advertise a capacity of 256M, and behave per the text for the portion of
> the 256M range not backed by physical memory. Such a device would be
> problematic for the host to handle, as I'm not sure how the host would be
> able to determine the portion of the range that is usable. As such, I
> don't think building such a device would be practical.

Right, I don't expect it to happen, but it's easy enough for the
kernel to exclude the possibility.

It just so happens that one of the early revs of the QEMU patch set
introduced this case, so it may not be feasible for hardware, but it's
already happened for emulated devices.

> There are several other locations in the CXL 2.0 spec that indicate that
> devices must manage capacity in units of 256M. E.g. Table 175. Identify
> Memory Device Output Payload.

Yes, for this patch it is just trying to find a reasonable heuristic
for when "Mem_enable = 1" and "HDM Decoder Capabilty Enable = 0" to
determine that the kernel can go ahead and set "HDM Decoder Capability
Enable = 1"