| Message ID | 20250402193443.944654-1-gourry@gourry.net |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl: core/region - ignore interleave granularity when ways=1 | expand |
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)) {
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.
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)) {
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(-)