When validating decoder IW/IG when setting up regions, the granularity
is irrelevant when iw=1 - all accesses will always route to the only
target anyway - so all ig values are "correct". Loosen the requirement
that `ig = (parent_iw * parent_ig)` when iw=1.

Signed-off-by: Gregory Price <gourry@gourry.net>
---
 drivers/cxl/core/region.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 04bc6cad092c..dec262eadf9a 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -1553,7 +1553,7 @@ static int cxl_port_setup_targets(struct cxl_port *port,
 
 	if (test_bit(CXL_REGION_F_AUTO, &cxlr->flags)) {
 		if (cxld->interleave_ways != iw ||
-		    cxld->interleave_granularity != ig ||
+		    (iw > 1 && cxld->interleave_granularity != ig) ||
 		    cxled->spa_range.start != p->res->start ||
 		    cxled->spa_range.end != p->res->end ||
 		    ((cxld->flags & CXL_DECODER_F_ENABLE) == 0)) {
-- 
2.47.1

Gregory Price wrote:
> When validating decoder IW/IG when setting up regions, the granularity
> is irrelevant when iw=1 - all accesses will always route to the only
> target anyway - so all ig values are "correct". Loosen the requirement
> that `ig = (parent_iw * parent_ig)` when iw=1.

Can you say a bit more here about the real world impact like we chatted
about on Discord? Something like:

---
The platform BIOS on "platform-X" specifies a 512-byte
interleave-granularity CXL Window when 256-byte is expected. This leads
to Linux erroneously rejecting the region configuration of 2 devices
attached to an x1 host bridge.
---

That way distros and platform-X folks can flag the importance of this fix.

On Wed, Apr 02, 2025 at 02:59:17PM -0700, Dan Williams wrote:
> Gregory Price wrote:
> > When validating decoder IW/IG when setting up regions, the granularity
> > is irrelevant when iw=1 - all accesses will always route to the only
> > target anyway - so all ig values are "correct". Loosen the requirement
> > that `ig = (parent_iw * parent_ig)` when iw=1.
> 
> Can you say a bit more here about the real world impact like we chatted
> about on Discord? Something like:
> 
> ---
> The platform BIOS on "platform-X" specifies a 512-byte
> interleave-granularity CXL Window when 256-byte is expected. This leads
> to Linux erroneously rejecting the region configuration of 2 devices
> attached to an x1 host bridge.
> ---
> 
> That way distros and platform-X folks can flag the importance of this fix.

Would like this inline or just in the changelog?

~Gregory

Gregory Price wrote:
> On Wed, Apr 02, 2025 at 02:59:17PM -0700, Dan Williams wrote:
> > Gregory Price wrote:
> > > When validating decoder IW/IG when setting up regions, the granularity
> > > is irrelevant when iw=1 - all accesses will always route to the only
> > > target anyway - so all ig values are "correct". Loosen the requirement
> > > that `ig = (parent_iw * parent_ig)` when iw=1.
> > 
> > Can you say a bit more here about the real world impact like we chatted
> > about on Discord? Something like:
> > 
> > ---
> > The platform BIOS on "platform-X" specifies a 512-byte
> > interleave-granularity CXL Window when 256-byte is expected. This leads
> > to Linux erroneously rejecting the region configuration of 2 devices
> > attached to an x1 host bridge.
> > ---
> > 
> > That way distros and platform-X folks can flag the importance of this fix.
> 
> Would like this inline or just in the changelog?

Changelog is fine, because this more about routing the fix than
explaining the code.

On 4/2/25 12:34 PM, Gregory Price wrote:
> When validating decoder IW/IG when setting up regions, the granularity
> is irrelevant when iw=1 - all accesses will always route to the only
> target anyway - so all ig values are "correct". Loosen the requirement
> that `ig = (parent_iw * parent_ig)` when iw=1.
> 
> Signed-off-by: Gregory Price <gourry@gourry.net>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/cxl/core/region.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 04bc6cad092c..dec262eadf9a 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1553,7 +1553,7 @@ static int cxl_port_setup_targets(struct cxl_port *port,
>  
>  	if (test_bit(CXL_REGION_F_AUTO, &cxlr->flags)) {
>  		if (cxld->interleave_ways != iw ||
> -		    cxld->interleave_granularity != ig ||
> +		    (iw > 1 && cxld->interleave_granularity != ig) ||
>  		    cxled->spa_range.start != p->res->start ||
>  		    cxled->spa_range.end != p->res->end ||
>  		    ((cxld->flags & CXL_DECODER_F_ENABLE) == 0)) {