Re: [PATCH v2] cxl/acpi: Warn on mixed CXL VH and RCH/RCD Hierarchy

[Alison Schofield <alison.schofield@intel.com>]

On Tue, Jul 02, 2024 at 09:14:51PM +0200, Fabio M. De Francesco wrote:
> On Tuesday, July 2, 2024 12:17:25 AM GMT+2 Alison Schofield wrote:
> > On Fri, Jun 28, 2024 at 07:48:07PM +0200, Fabio M. De Francesco wrote:
> > > Each Host Bridge instance has a corresponding CXL Host Bridge Structure
> > > (CHBS) ACPI table that identifies its capabilities. CHBS tables can be
> > > two types (CXL 3.1 Table 9-21): The PCIe Root Complex Register Block
> > > (RCRB) and CXL Host Bridge Component Registers (CHBCR).
> > > 
> > > If a Host Bridge is attached to a device that is operating in Restricted
> > > CXL Device Mode (RCD), BIOS publishes an RCRB with the base address of
> > > registers that describe its capabilities (CXL 3.1 sec. 9.11).
> > > 
> > > Instead, the new (CXL 2.0+) Component registers can only be accessed
> > > by means of a base address published with a CHBCR (CXL 3.1 sec. 9.12).
> > > 
> > > If an eRCD (a device that forces the host-bridge into CXL 1.1 Restricted
> > > CXL Host mode) is attached to a CXL 2.0+ Host-Bridge, the current CXL
> > > specification does not define a mechanism for finding CXL-2.0-only
> > > root-port component registers like HDM decoders and Extended Security
> > > capability.
> > > 
> > > An algorithm to locate a CHBCR associated with an RCRB, would be too
> > > invasive to land without some concrete motivation.
> > > 
> > > Therefore, just print a message to inform of unsupported config.
> > > 
> > > Count how many different CHBS "Version" types are detected by
> > > cxl_get_chbs_iter(). Then make cxl_get_chbs() print a warning if that sum
> > > is greater than 1.
> > > 
> > > Tested-by: Alison Schofield <alison.schofield@intel.com>
> > > Signed-off-by: Fabio M. De Francesco <fabio.m.de.francesco@linux.intel.com>
> > > ---
> > > 
> > > --- Changes for v2 ---
> > > 
> > > 	- Rewrite the Subject line (Alison)
> > > 	- Address a bug found by Alison while testing (thanks!)
> > 
> > 'Address' a bug doesn't descibe what changed from v1 especially for
> > folks who reviewed v1.
> > 
> > I'm not asking you to rev this patch, just explain the saved_version
> > change.
> > 
> > Thanks
> > -- Alison
> >
> 
> Alison,
> 
> Thanks again for testing, and also for noticing that I didn't give any 
> explanation of the new saved_version variable and the solution I introduced to 
> fix the bug you found. Then...
> 
> During tests, Alison noticed that the CHBS iterator was testing chbs-
> >cxl_version always against UINT_MAX.  
> 
> ctx->cxl_version must be assigned, along with ctx->base, before returning from 
> the CHBS iterator, only if all 'if' statement evaluate false.
> 
> Therefore, don't test anymore against ctx->cxl_version. Instead save every new 
> different chbs->cxl_version in ctx->saved_version and test against this new 
> field at every iteration. 
> 
> Add an inline comment that briefly (and indirectly) refers to this logic. 
Thanks!

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

>
snip

>