Endpoints requiring address translation might not be aware of the
system's interleaving configuration. Instead, interleaving can be
configured on an upper memory domain (from an endpoint view) and thus
is not visible to the endpoint. For region creation this might cause
an invalid interleaving config that does not match the CFMWS entries.

Use the interleaving configuration of the root decoders to create a
region which bases on CFMWS entries. This always matches the system's
interleaving configuration and is independent of the underlying memory
topology.

Signed-off-by: Robert Richter <rrichter@amd.com>
---
 drivers/cxl/core/region.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index e218f0be2409..c3322bae05b9 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -3477,8 +3477,8 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
 	}
 
 	p->res = res;
-	p->interleave_ways = cxled->cxld.interleave_ways;
-	p->interleave_granularity = cxled->cxld.interleave_granularity;
+	p->interleave_ways = cxlrd->cxlsd.cxld.interleave_ways;
+	p->interleave_granularity = cxlrd->cxlsd.cxld.interleave_granularity;
 	p->state = CXL_CONFIG_INTERLEAVE_ACTIVE;
 
 	rc = sysfs_update_group(&cxlr->dev.kobj, get_cxl_region_target_group());
-- 
2.39.5

On Tue, Jan 07, 2025 at 03:10:09PM +0100, Robert Richter wrote:
> Endpoints requiring address translation might not be aware of the
> system's interleaving configuration. Instead, interleaving can be
> configured on an upper memory domain (from an endpoint view) and thus
> is not visible to the endpoint. For region creation this might cause
> an invalid interleaving config that does not match the CFMWS entries.
> 
> Use the interleaving configuration of the root decoders to create a
> region which bases on CFMWS entries. This always matches the system's
> interleaving configuration and is independent of the underlying memory
> topology.

This sounds like a restriction, more restrictive than present.

Won't it block all region interleave ways greater than root decoder
interleave ways? ie. disallows 2, 2+2, 2+2+2, 4, etc.


> 
> Signed-off-by: Robert Richter <rrichter@amd.com>
> ---
>  drivers/cxl/core/region.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index e218f0be2409..c3322bae05b9 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3477,8 +3477,8 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
>  	}
>  
>  	p->res = res;
> -	p->interleave_ways = cxled->cxld.interleave_ways;
> -	p->interleave_granularity = cxled->cxld.interleave_granularity;
> +	p->interleave_ways = cxlrd->cxlsd.cxld.interleave_ways;
> +	p->interleave_granularity = cxlrd->cxlsd.cxld.interleave_granularity;
>  	p->state = CXL_CONFIG_INTERLEAVE_ACTIVE;
>  
>  	rc = sysfs_update_group(&cxlr->dev.kobj, get_cxl_region_target_group());
> -- 
> 2.39.5
>

On 13.01.25 09:48:47, Alison Schofield wrote:
> On Tue, Jan 07, 2025 at 03:10:09PM +0100, Robert Richter wrote:
> > Endpoints requiring address translation might not be aware of the
> > system's interleaving configuration. Instead, interleaving can be
> > configured on an upper memory domain (from an endpoint view) and thus
> > is not visible to the endpoint. For region creation this might cause
> > an invalid interleaving config that does not match the CFMWS entries.
> > 
> > Use the interleaving configuration of the root decoders to create a
> > region which bases on CFMWS entries. This always matches the system's
> > interleaving configuration and is independent of the underlying memory
> > topology.
> 
> This sounds like a restriction, more restrictive than present.
> 
> Won't it block all region interleave ways greater than root decoder
> interleave ways? ie. disallows 2, 2+2, 2+2+2, 4, etc.

Yes, a combination of all decoders from the endpoint up to the root
result in the interleaving config. This will be fixed in the next
version.

Thanks for catching this,

-Robert