Re: [PATCH 2/2] cxl/acpi: Support CXL XOR Interleave Math (CXIMS)

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

On Tue, Aug 09, 2022 at 04:36:14PM -0700, Dan Williams wrote:
> alison.schofield@ wrote:
> > From: Alison Schofield <alison.schofield@intel.com>
> > 
> > When the CFMWS is using XOR math, parse the corresponding
> > CXIMS structure and store the xormaps in the root decoder.
> > Use the xormaps in a new lookup, cxl_hb_xor(), to discover
> > a targets entry in a host bridge interleave target list.
> > 
> > Defined in CXL Spec 3.0 Section: 9.17.1
> > 
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > ---
> >  drivers/cxl/cxl.h  |  2 +
> >  drivers/cxl/acpi.c | 96 +++++++++++++++++++++++++++++++++++++++++++---
> >  2 files changed, 93 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> > index f680450f0b16..0a17a7007bff 100644
> > --- a/drivers/cxl/cxl.h
> > +++ b/drivers/cxl/cxl.h
> > @@ -330,12 +330,14 @@ struct cxl_switch_decoder {
> >   * @res: host / parent resource for region allocations
> >   * @region_id: region id for next region provisioning event
> >   * @calc_hb: which host bridge covers the n'th position by granularity
> > + * @platform_data: platform specific configuration data
> >   * @cxlsd: base cxl switch decoder
> >   */
> >  struct cxl_root_decoder {
> >  	struct resource *res;
> >  	atomic_t region_id;
> >  	struct cxl_dport *(*calc_hb)(struct cxl_root_decoder *cxlrd, int pos);
> > +	void *platform_data;
> >  	struct cxl_switch_decoder cxlsd;
> >  };
> >  
> > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> > index fb649683dd3a..6ac6751c7f4e 100644
> > --- a/drivers/cxl/acpi.c
> > +++ b/drivers/cxl/acpi.c
> > @@ -9,6 +9,79 @@
> >  #include "cxlpci.h"
> >  #include "cxl.h"
> >  
> > +struct cxims_data {
> > +	int nr_maps;
> > +	u64 xormaps[];
> > +};
> > +
> > +static struct cxl_dport *cxl_hb_xor(struct cxl_root_decoder *cxlrd, int pos)
> > +{
> > +	struct cxl_switch_decoder *cxlsd = &cxlrd->cxlsd;
> > +	struct cxims_data *cximsd = cxlrd->platform_data;
> > +	struct cxl_decoder *cxld = &cxlsd->cxld;
> > +	int ig = cxld->interleave_granularity;
> > +	int i, n = 0;
> > +	u64 hpa;
> > +
> > +	if (dev_WARN_ONCE(&cxld->dev,
> > +			  cxld->interleave_ways != cxlsd->nr_targets,
> > +			  "misconfigured root decoder\n"))
> > +		return NULL;
> > +	/*
> > +	 * Find this targets entry (n) in the host bridge interleave
> > +	 * list. Defined in CXL Spec 3.0 Section 9.17.1.3 Table 9-22
> > +	 */
> > +	hpa = cxlrd->res->start + pos * ig;
> > +	for (i = 0; i < cximsd->nr_maps; i++)
> > +		n |= (hweight64(hpa & cximsd->xormaps[i]) & 1) << i;
> > +
> > +	return cxlrd->cxlsd.target[n];
> > +}
> > +
> > +struct cxl_cxims_context {
> > +	struct device *dev;
> > +	struct cxl_root_decoder *cxlrd;
> > +};
> > +
> > +static int cxl_parse_cxims(union acpi_subtable_headers *header, void *arg,
> > +			   const unsigned long end)
> > +{
> > +	struct acpi_cedt_cxims *cxims = (struct acpi_cedt_cxims *)header;
> > +	struct cxl_cxims_context *ctx = arg;
> > +	struct cxl_root_decoder *cxlrd = ctx->cxlrd;
> > +	struct cxl_decoder *cxld = &cxlrd->cxlsd.cxld;
> > +	struct device *dev = ctx->dev;
> > +	struct cxims_data *cximsd;
> > +	unsigned int hbig;
> > +	u8 eiw;
> 
> Per your comment on Dave's patch lets keep eiw as a shorthand for
> "encoded interleave ways" i.e. the ilog2()'ish value of interleave-ways.
> So just: s/eiw/iw/.

Then maybe it's intentionally wrong. The eiw tells the number of
xormaps to store for use in cxl_hb_xor(). The 'iw', as stored in
cxld->interleave_ways would be too many, so I translate back to the
encoded version here.

> 
> Otherwise, looks good to me. Just need the ACPICA side to land.