Re: [PATCH] cxl/region: Allow 6 & 12 way regions on 3-way HB interleaves

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

On Mon, Feb 24, 2025 at 05:41:20PM -0700, Dave Jiang wrote:
> 
> 
> On 2/24/25 4:58 PM, alison.schofield@intel.com wrote:
> > From: Alison Schofield <alison.schofield@intel.com>
> > 
> > The CXL driver requires the granularity of a region and it's root
> 
> s/it's/its/

got it

> 
> > decoder to be the same. This is particularly restrictive for 3-way
> > host bridge interleaves where the only spec defined interleave
> > configurations for creating 6-way and 12-way regions on a 3-way HB
> configuration?
> > interleave require mixed granularities.
> requries?

considering that 

> 
> > 
> > CXL 3.1 Specification 9.13.1.1:
> 
> May as well go to 3.2
> 

got it


> > Legal Interleaving Configurations: 12-way, 6-way, and 3-way
> > 
> > Adding support for these new interleaves touches these areas:
> > 
> > 1) User created regions employing "cxl create-region" fail when the
> > ndctl tool gets a mixed granularity request. That is addressed in
> s/ndctl/CXL CLI/
> 

got it


> > a patch to the ndctl tool.
> > 

snip

> >  
> > +static bool interleave_granularity_allow(struct cxl_decoder *cxld, u16 ig)
> > +{
> > +	/*
> > +	 * When the host-bridge is interleaved, disallow region granularity
> > +	 * != root granularity with the exception of 3-way HB interleaves.
> > +	 * Allow the CXL Spec defined 3-way HB interleaves that can only be
> > +	 * configured when host-bridge interleave is greater that the
> > +	 * region interleave. (CXL 3.1 Specification 9.13.1.1)
> spec 3.2
> 
> > +	 * Allow 2+2+2 interleave where HB gran is 2 * region granularity
> > +	 *	 4+4+4 interleave where HB gran is 4 * region granularity
> > +	 *
> > +	 * Regions with a granularity greater than the root interleave result
> > +	 * in invalid DPA translations (invalid to support).
> > +	 */
> > +	if (cxld->interleave_ways > 1 && ig != cxld->interleave_granularity) {
> > +		if (cxld->interleave_ways != 3)
> > +			return false;
> > +
> > +		if (cxld->interleave_granularity % (2 * ig) &&
> > +		    cxld->interleave_granularity % (4 * ig))
> > +			return false;
> 
> Can you please explain how the math works here? I'm not understanding the code here vs the comments above.

I'm guessing you are talking about the last chunk only. We get there
after discovering a 3-way HB interleave. (HB interleave and root decoder
interleave are the same thing.) Once we know it's a 3-way, there are
only 2 possible values and those are defined in the spec as:
	2+2+2 interleave where HB gran is 2 * region granularity
	4+4+4 interleave where HB gran is 4 * region granularity

And now that I've gotten to this part of the explanation I think
the code is allowing multiples of 2*ig and 4*ig, when it should
be a stricter check for exactly 2*ig or 4*ig.

updating that to:
	if (cxld->interleave_granularity != 2 * ig &&
	    cxld->interleave_granularity != 4 * ig)
		return false;

Thanks for that Dave!
> 
> DJ
> 

snip