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

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Tue, 17 May 2022 19:44:28 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> On Tue, May 17, 2022 at 7:08 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. While DVSEC Ranges are expected to be at least
> > > 256M in size, the specification (CXL 2.0 Section 8.1.3.8.4 DVSEC CXL
> > > Range 1 Base Low) allows for the possibilty of devices smaller than
> > > 256M. So the range [0, 256M) is considered active even if Memory_size
> > > is 0.  
> >
> > Regarding "So the range [0, 256M) is considered active even if
> > Memory_size is 0."
> >
> > Since Memory_Base is included in address A, this portion of the equation
> > from CXL 2.0 Section 8.1.3.8.4 mandates that for host access to address A
> > to be directed to local HDM memory, Memory_Size[63:28] must be > 0:
> >
> > (A >> 28) < Memory_Base[63:28] + Memory_Size[63:28]
> >
> > This means if a device advertises Memory_Size = 0, no host access will
> > result in access to the HDM memory.
> >
> > I would also note this text from CXL 2.0 Section 8.1.3.8:
> > "A CXL.mem capable device is permitted to report zero memory size."
> >
> > For a device with a non-zero capacity less than 256M to satisfy the
> > equation, it would need to advertise a Memory_Size of at least 256M.  
> 
> I think we need an errata to delete the "(e.g. a device with less than
> 256 MB of memory)" mention. I otherwise do not see how such a device
> can exist if Memory_size must be >= 256M.

My reading of that is it is permissible to implement a device that
has say 16MiB or actual memory, report it as 256MiB and follow this
behavior for the 16-256 MiB range.  It also covers a 300MiB device
where the size of the HDM decoder is set to 512MiB etc.

As such I don't think it's wrong, but rather just not relevant to us
here (0 is a valid setting for Memory_Size).
Would need impdef means to establish the actual size of the
memory to do anything useful with that corner case.

Jonathan