Re: [PATCH v2] cxl/acpi: Limit XOR map application based on host bridge ways

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

On Wed, Aug 20, 2025 at 08:44:34AM -0700, Dave Jiang wrote:
> 
> 
> On 8/12/25 8:29 PM, alison.schofield@intel.com wrote:
> > From: Alison Schofield <alison.schofield@intel.com>
> > 
> > The CXL specification defines one set of XOR maps per Host Bridge
> > Interleave Granularity (HBIG), but the number of maps to apply
> > depends on the Host Bridge Interleave Ways (HBIW) for each region.
> > 
> > Currently, cxl_xor_hpa_to_spa() incorrectly applies all available
> > XOR maps regardless of the region's host bridge interleave ways.
> > This causes incorrect address translations when multiple CXL regions
> > with the same HBIG but different HBIW's coexist.
> > 
> > Example scenario with 3 XOR maps defined for 256-byte granularity:
> > - 4-way interleave region:  should apply 2 maps (was applying 3)
> > - 8-way interleave region:  should apply 3 maps (correct)
> > - 12-way interleave region: should apply 2 maps (was applying 3)
> > 
> > Per CXL 3.2 Section 9.18.1.4 and Table 9-22, fix this by using
> > a lookup table to determine the correct number of XOR maps based
> > on host bridge interleave ways.
> > 
> > Fixes: 3b2fedcd75e3 ("cxl: Restore XOR'd position bits during address translation")
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > Reviewed-by: Dan Williams <dan.j.williams@intel.com>
> > Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> > Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> 
> Applied to cxl/next
> c7ad33d50282168fbfed1c6662503b0d979a67c8

This patch can be dropped as there is nothing to fix.

The story above is true, but I missed the piece that came before. The
CXL driver actually stores a customized list of xormaps per root decoder
(see cxl_parse_cxims()) so there cannot be any extra maps in it's root
decoder platform data (struct cxl_cxims_data).

This came up during testing when I moved out of tree code for address
translation into the cxl-translate test module. In that case there was
only one struct cxl_cxmis_data, not multiples per region interleave (or
per root decoder) and so I was alarmed when all the xormaps were always
applied by the CXL driver. Now I know that is working as designed, and
it is the test module that needs to change.

My apologies for crying wolf!

> 
> > ---
> > 
> > Changes in v2:
> > - Use CXL_DECODER_MAX_INTERLEAVE in hbiw_to_nr_maps[] definition (Dan)
> > - Rebase onto latest cxl/next
> > 
> > 
> >  drivers/cxl/acpi.c | 18 +++++++++++++++---
> >  1 file changed, 15 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> > index f1625212b08b..26c494704437 100644
> > --- a/drivers/cxl/acpi.c
> > +++ b/drivers/cxl/acpi.c
> > @@ -16,19 +16,31 @@ struct cxl_cxims_data {
> >  	u64 xormaps[] __counted_by(nr_maps);
> >  };
> >  
> > +/*
> > + * There is one CXIMS, therefore one set of XOR maps, that all CXL Windows with
> > + * the same host bridge granularity share. The number of maps to apply at address
> > + * translation is based on the Host Bridge interleave ways of the CXL Window.
> > + * CXL Specification 3.2 Section 9.18.1.4 and Table 9-22.
> > + */
> > +#define HBIW_TO_NR_MAPS_SIZE (CXL_DECODER_MAX_INTERLEAVE + 1)
> > +
> > +static const int hbiw_to_nr_maps[HBIW_TO_NR_MAPS_SIZE] = {
> > +	[1] = 0, [2] = 1, [3] = 0, [4] = 2, [6] = 1, [8] = 3, [12] = 2, [16] = 4
> > +};
> > +
> >  static const guid_t acpi_cxl_qtg_id_guid =
> >  	GUID_INIT(0xF365F9A6, 0xA7DE, 0x4071,
> >  		  0xA6, 0x6A, 0xB4, 0x0C, 0x0B, 0x4F, 0x8E, 0x52);
> >  
> >  static u64 cxl_apply_xor_maps(struct cxl_root_decoder *cxlrd, u64 addr)
> >  {
> > +	int nr_maps_to_apply = hbiw_to_nr_maps[cxlrd->cxlsd.nr_targets];
> >  	struct cxl_cxims_data *cximsd = cxlrd->platform_data;
> > -	int hbiw = cxlrd->cxlsd.nr_targets;
> >  	u64 val;
> >  	int pos;
> >  
> >  	/* No xormaps for host bridge interleave ways of 1 or 3 */
> > -	if (hbiw == 1 || hbiw == 3)
> > +	if (!nr_maps_to_apply)
> >  		return addr;
> >  
> >  	/*
> > @@ -50,7 +62,7 @@ static u64 cxl_apply_xor_maps(struct cxl_root_decoder *cxlrd, u64 addr)
> >  	 * bits results in val==0, if odd the XOR result is val==1.
> >  	 */
> >  
> > -	for (int i = 0; i < cximsd->nr_maps; i++) {
> > +	for (int i = 0; i < nr_maps_to_apply; i++) {
> >  		if (!cximsd->xormaps[i])
> >  			continue;
> >  		pos = __ffs(cximsd->xormaps[i]);
> > 
> > base-commit: d9412f08e25a5b66f9021739c090cc9b8f1089b1
>