[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3636999.dWV9SEqChM@fdefranc-mobl3>
Date: Wed, 26 Jun 2024 12:00:47 +0200
From: "Fabio M. De Francesco" <fabio.m.de.francesco@...ux.intel.com>
To: Alison Schofield <alison.schofield@...el.com>
Cc: Davidlohr Bueso <dave@...olabs.net>,
Jonathan Cameron <jonathan.cameron@...wei.com>,
Dave Jiang <dave.jiang@...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 Friday, June 21, 2024 10:57:32 PM GMT+2 Alison Schofield wrote:
> On Wed, Jun 19, 2024 at 02:59:41PM +0200, Fabio M. De Francesco wrote:
>
Hi Alison,
>
> Hi Fabio,
> You've written such a detailed commit msg, that it pulls me in,
> and now I want to understand more....
>
>
> > 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.
>
> Is there a spec reference for this?
>
I'll add a spec reference with v2.
> While you're spelling things out, please expand RCRB and CHBCR
I'll do it.
>
> >
> > 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.
> >
>
> Were users seeing this and confused by this silent failure?
> What did it look like before?
>
Please read Dan on this topic.
I think I'll add a few lines to the commit message to summarize Dan's comment.
>
> > 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(-)
> >
> > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> > index 571069863c62..9e226a65a5ea 100644
> > --- a/drivers/cxl/acpi.c
> > +++ b/drivers/cxl/acpi.c
> > @@ -482,6 +482,7 @@ struct cxl_chbs_context {
> > unsigned long long uid;
> > resource_size_t base;
> > u32 cxl_version;
> > + int count;
>
> Maybe s/count/nr_versions to be more explicit of what it counts.
>
Yes, "nr_versions" is better.
Thank you.
Fabio
>
> -- Alison
> > };
> >
> snip
> >
>
Powered by blists - more mailing lists