linux-kernel - Re: [PATCH] cxl/acpi: Warn on unsupported platform config detection

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <21841700.4csPzL39Zc@fdefranc-mobl3>
Date: Wed, 26 Jun 2024 12:28:19 +0200
From: "Fabio M. De Francesco" <fabio.m.de.francesco@...ux.intel.com>
To: Dan Williams <dan.j.williams@...el.com>
Cc: Davidlohr Bueso <dave@...olabs.net>,
 Jonathan Cameron <jonathan.cameron@...wei.com>,
 Dave Jiang <dave.jiang@...el.com>,
 Alison Schofield <alison.schofield@...el.com>,
 Vishal Verma <vishal.l.verma@...el.com>, Ira Weiny <ira.weiny@...el.com>,
 Dan Williams <dan.j.williams@...el.com>, linux-cxl@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] cxl/acpi: Warn on unsupported platform config detection

On Wednesday, June 26, 2024 1:14:18 AM GMT+2 Dan Williams wrote:

Hi Dan,

> 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: RCRB and 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.
> > 
> > However, the new (CXL 2.0+) Component registers (e.g., Extended Security
> > Capability), can only be accessed by means of a base address published
> > with a CHBCR.
> > 
> > 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.
> > 
> > Signed-off-by: Fabio M. De Francesco <fabio.m.de.francesco@...ux.intel.com>
> > ---
> >  drivers/cxl/acpi.c | 20 ++++++++++++++------
> >  1 file changed, 14 insertions(+), 6 deletions(-)
> > 

snip

> > @@ -533,6 +537,10 @@ static int cxl_get_chbs(struct device *dev, struct 
acpi_device *hb,
> >  
> >  	acpi_table_parse_cedt(ACPI_CEDT_TYPE_CHBS, cxl_get_chbs_iter, 
ctx);
> >  
> > +	if (ctx->count > 1)
> > +		/* Disclaim eRCD support given some component register 
may only be found via CHBCR */
> > +		dev_info(dev, "Unsupported platform config, mixed 
Virtual Host and Restricted CXL Host hierarchy.");
> 
> I believe this is already queued, but my personal preference is that
> multiline statements include brackets, or move the comment above the "if
> ()", so either:
> 
> 	/* Disclaim eRCD support given some component register may only be 
found via CHBCR */
> 	if (ctx->count > 1)
> 		dev_info(dev, "Unsupported platform config, mixed Virtual 
Host and Restricted CXL Host hierarchy.");
> 
> ...or:
> 
> 	if (ctx->count > 1) {
> 		/* Disclaim eRCD support given some component register 
may only be found via CHBCR */
> 		dev_info(dev, "Unsupported platform config, mixed Virtual 
Host and Restricted CXL Host hierarchy.");
> 	}
> 
> ...but don't spin the patch just for that fixup.
> 
I'll send v2 mainly for Alison's comments and so I will also add brackets here 
(the second solution meets my preferences).

Thank you,

Fabio