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

[Dave Jiang <dave.jiang@intel.com>]

On 2/24/25 7:27 PM, Alison Schofield wrote:
> 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;
> 

Ok the new code make sense to me now. :) 

> Thanks for that Dave!
>>
>> DJ
>>
> 
> snip