* [PATCH v4 0/2] cxl: Allow passthrough decoders with >16K granularity @ 2026-09-04 3:21 Alison Schofield 2026-09-04 3:21 ` [PATCH v4 1/2] cxl/region: Allow regions with an unencodable passthrough granularity Alison Schofield 2026-09-04 3:21 ` [PATCH v4 2/2] cxl/test: Add a 2-way 16K root decoder for passthrough testing Alison Schofield 0 siblings, 2 replies; 5+ messages in thread From: Alison Schofield @ 2026-09-04 3:21 UTC (permalink / raw) To: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Alison Schofield, Vishal Verma, Ira Weiny, Li Ming Cc: linux-cxl I kept RichardC and DaveJ tags as the helper is a mechanical refactor, not a behavior change. More/re-review is welcome. Changes in v4: - Rebase on Linux 7.3-rc1 - Rename Patch 1 to focus on the region config failure that user sees - Use Reported-by and Closes tags instead of Suggested-by in Patch 1 - Add and use a new granularity_is_valid() helper (RichardC) - Update the Patch2 commit log to say computes 8K, not 16K (RichardC) Link to v3: https://lore.kernel.org/linux-cxl/cover.1784940306.git.alison.schofield@intel.com/ Changes in v3: - Program the real IG encoding for an encodable non-interleaving granularity instead of always writing EIG=0 (RichardC, Sashiko) - Update Patch 1 commit msg text and this cover letter text per above change Link to v2: https://lore.kernel.org/linux-cxl/cover.1783795720.git.alison.schofield@intel.com/ Changes in v2: - Rebased on Linux 7.2-rc1 - Only v1 feedback was from Sashiko and that was not an issue Link to v1: https://lore.kernel.org/linux-cxl/cover.1781136281.git.alison.schofield@intel.com/ Begin Cover Letter: Sashiko reported [1] that region creation fails when the interleave granularity accumulated below the root decoder exceeds the 16K maximum encodable in the interleave granularity (IG) field. The decoder that computes that granularity is a passthrough decoder, which does not consume the IG field, so no decoder in the path interleaves at the granularity being rejected. The first patch avoids validating the interleave granularity of non-interleaving decoders, encodes the IG field as a don't-care only when the computed granularity is not encodable, and updates the sysfs ABI documentation accordingly. The second patch extends cxl-test with a topology that allows a CXL unit test to cover this case. A unit test update for this case is posted for review [2]. [1] https://lore.kernel.org/all/20260530002613.682E71F00893@smtp.kernel.org/ [2] https://lore.kernel.org/nvdimm/cover.1788489571.git.alison.schofield@intel.com/ Alison Schofield (2): cxl/region: Allow regions with an unencodable passthrough granularity cxl/test: Add a 2-way 16K root decoder for passthrough testing Documentation/ABI/testing/sysfs-bus-cxl | 5 ++- drivers/cxl/core/hdm.c | 8 ++++- drivers/cxl/core/region.c | 34 ++++++++++-------- tools/testing/cxl/test/cxl.c | 48 +++++++++++++++++++++++-- 4 files changed, 76 insertions(+), 19 deletions(-) base-commit: cee9395acd8043be0644b25c34bfa86623f2b935 -- 2.37.3 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v4 1/2] cxl/region: Allow regions with an unencodable passthrough granularity 2026-09-04 3:21 [PATCH v4 0/2] cxl: Allow passthrough decoders with >16K granularity Alison Schofield @ 2026-09-04 3:21 ` Alison Schofield 2026-09-04 3:36 ` sashiko-bot 2026-09-04 3:21 ` [PATCH v4 2/2] cxl/test: Add a 2-way 16K root decoder for passthrough testing Alison Schofield 1 sibling, 1 reply; 5+ messages in thread From: Alison Schofield @ 2026-09-04 3:21 UTC (permalink / raw) To: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Alison Schofield, Vishal Verma, Ira Weiny, Li Ming Cc: linux-cxl, Sashiko AI Review, Richard Cheng Region creation fails when the interleave granularity accumulated below the root decoder exceeds 16K, the maximum encodable in the HDM Decoder Control IG field, even when the decoder that computes it does not interleave. For example, a 2-way root decoder at 16K granularity with a single endpoint below each host bridge. Neither host bridge decoder interleaves, yet the 32K granularity computed for them is rejected and the region fails to attach an endpoint: cxl region3: root0: invalid granularity calculation (16384 * 2) cxl_port endpoint16: failed to attach decoder16.0 to region3: -22 A non-interleaving decoder does not consume the IG field, so region setup is rejecting a value hardware never reads. Only require the granularity to be encodable for interleaving decoders, both where it is inherited from the parent and where it is computed for the current decoder. Keep the computed value for passthrough decoders so it can seed descendant decoder setup. Accumulating the granularity below a passthrough decoder is not being undone: a descendant that does interleave must see the accumulated value or it would interleave on an address bit an ancestor already used. That descendant inherits a granularity above 16K, so it is still rejected on its own turn and a real overflow cannot be masked. When committing a non-interleaving decoder, still program the granularity if it is encodable; use a don't-care IG encoding only when it is not. This preserves an encodable passthrough granularity across re-enumeration, so descendants do not inherit a stale value. As a consequence, the interleave_granularity attribute of a decoder whose interleave_ways is 1 may report a value above 16K. Document that the reported granularity for non-interleaving decoders is a don't-care value that may exceed the maximum encodable in hardware. Fixes: 18f35dc9314d ("cxl/region: Refactor granularity select in cxl_port_setup_targets()") Reported-by: Sashiko AI Review <sashiko-bot@kernel.org> Closes: https://lore.kernel.org/all/20260530002613.682E71F00893@smtp.kernel.org/ Assisted-by: Claude:Opus-4-8 Reviewed-by: Richard Cheng <icheng@nvidia.com> Reviewed-by: Dave Jiang <dave.jiang@intel.com> Signed-off-by: Alison Schofield <alison.schofield@intel.com> --- Documentation/ABI/testing/sysfs-bus-cxl | 5 +++- drivers/cxl/core/hdm.c | 8 +++++- drivers/cxl/core/region.c | 34 ++++++++++++++----------- 3 files changed, 30 insertions(+), 17 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl index 7352dbd70bc7..6abfd792f07e 100644 --- a/Documentation/ABI/testing/sysfs-bus-cxl +++ b/Documentation/ABI/testing/sysfs-bus-cxl @@ -412,7 +412,10 @@ Description: space this decoder claims at address N before the decode rotates to the next target in the interleave at address N + interleave_granularity (assuming N is aligned to - interleave_granularity). + interleave_granularity). When 'interleave_ways' is 1, the + decoder does not interleave and the reported granularity is + a don't-care value that may exceed the maximum encodable in + hardware. What: /sys/bus/cxl/devices/decoderX.Y/create_pmem_region diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c index 0c80b76a5f9b..5ff1a75124cc 100644 --- a/drivers/cxl/core/hdm.c +++ b/drivers/cxl/core/hdm.c @@ -691,7 +691,13 @@ static void cxld_set_interleave(struct cxl_decoder *cxld, u32 *ctrl) if (WARN_ONCE(ways_to_eiw(cxld->interleave_ways, &eiw), "invalid interleave_ways: %d\n", cxld->interleave_ways)) return; - if (WARN_ONCE(granularity_to_eig(cxld->interleave_granularity, &eig), + + /* + * A non-interleaving decoder ignores the IG field, so an unencodable + * granularity is a don't-care rather than a failure. + */ + if (granularity_to_eig(cxld->interleave_granularity, &eig) && + WARN_ONCE(cxld->interleave_ways > 1, "invalid interleave_granularity: %d\n", cxld->interleave_granularity)) return; diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c index 27e63e6dab7c..89693f0181dd 100644 --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -1435,6 +1435,20 @@ static int check_interleave_cap(struct cxl_decoder *cxld, int iw, int ig) return 0; } +/* + * Hardware only consults the IG field of a decoder that interleaves, so a + * granularity the field cannot encode only matters when @ways exceeds one. + */ +static bool granularity_is_valid(int ways, int granularity) +{ + u16 eig; + + if (ways <= 1) + return true; + + return granularity_to_eig(granularity, &eig) == 0; +} + static int cxl_port_setup_targets(struct cxl_port *port, struct cxl_region *cxlr, struct cxl_endpoint_decoder *cxled) @@ -1449,7 +1463,6 @@ static int cxl_port_setup_targets(struct cxl_port *port, struct cxl_decoder *cxld = cxl_rr->decoder; struct cxl_switch_decoder *cxlsd; struct cxl_port *iter = port; - u16 eig, peig; u8 eiw, peiw; /* @@ -1522,12 +1535,11 @@ static int cxl_port_setup_targets(struct cxl_port *port, parent_iw = parent_cxld->interleave_ways; } - rc = granularity_to_eig(parent_ig, &peig); - if (rc) { + if (!granularity_is_valid(parent_iw, parent_ig)) { dev_dbg(&cxlr->dev, "%s:%s: invalid parent granularity: %d\n", dev_name(parent_port->uport_dev), dev_name(&parent_port->dev), parent_ig); - return rc; + return -EINVAL; } rc = ways_to_eiw(parent_iw, &peiw); @@ -1550,20 +1562,12 @@ static int cxl_port_setup_targets(struct cxl_port *port, * Interleave granularity is a multiple of @parent_port granularity. * Multiplier is the parent port interleave ways. */ - rc = granularity_to_eig(parent_ig * parent_iw, &eig); - if (rc) { + ig = parent_ig * parent_iw; + if (!granularity_is_valid(iw, ig)) { dev_dbg(&cxlr->dev, "%s: invalid granularity calculation (%d * %d)\n", dev_name(&parent_port->dev), parent_ig, parent_iw); - return rc; - } - - rc = eig_to_granularity(eig, &ig); - if (rc) { - dev_dbg(&cxlr->dev, "%s:%s: invalid interleave: %d\n", - dev_name(port->uport_dev), dev_name(&port->dev), - 256 << eig); - return rc; + return -EINVAL; } if (iw > 8 || iw > cxlsd->nr_targets) { -- 2.37.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v4 1/2] cxl/region: Allow regions with an unencodable passthrough granularity 2026-09-04 3:21 ` [PATCH v4 1/2] cxl/region: Allow regions with an unencodable passthrough granularity Alison Schofield @ 2026-09-04 3:36 ` sashiko-bot 2026-09-04 3:53 ` Alison Schofield 0 siblings, 1 reply; 5+ messages in thread From: sashiko-bot @ 2026-09-04 3:36 UTC (permalink / raw) To: Alison Schofield; +Cc: linux-cxl Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] During region auto-discovery, the driver relaxes the interleave granularity validation for non-interleaving passthrough decoders, but fails to update the decoder's software struct (`cxld->interleave_granularity`) with the computed logical value. As a result, descendant decoders inherit a stale hardware don't-care value. -- commit 581b6f11f4646ac877c9eaf9fb81a155a40889d5 Author: Alison Schofield <alison.schofield@intel.com> cxl/region: Allow regions with an unencodable passthrough granularity Region creation fails when the interleave granularity accumulated below the root decoder exceeds 16K, the maximum encodable in the HDM Decoder Control IG field, even when the decoder that computes it does not interleave. This patch keeps the computed value for passthrough decoders so it can seed descendant decoder setup. Closes: https://lore.kernel.org/all/20260530002613.682E71F00893@smtp.kernel.org/ > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index 27e63e6dab7c8..89693f0181ddb 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c [ ... ] > @@ -1550,20 +1562,12 @@ static int cxl_port_setup_targets(struct cxl_port *port, > * Interleave granularity is a multiple of @parent_port granularity. > * Multiplier is the parent port interleave ways. > */ > - rc = granularity_to_eig(parent_ig * parent_iw, &eig); > - if (rc) { > + ig = parent_ig * parent_iw; > + if (!granularity_is_valid(iw, ig)) { > dev_dbg(&cxlr->dev, > "%s: invalid granularity calculation (%d * %d)\n", > dev_name(&parent_port->dev), parent_ig, parent_iw); > - return rc; > - } > - > - rc = eig_to_granularity(eig, &ig); > - if (rc) { > - dev_dbg(&cxlr->dev, "%s:%s: invalid interleave: %d\n", > - dev_name(port->uport_dev), dev_name(&port->dev), > - 256 << eig); > - return rc; > + return -EINVAL; > } > > if (iw > 8 || iw > cxlsd->nr_targets) { [Severity: High] During region auto-discovery, the driver relaxes the interleave granularity validation for non-interleaving passthrough decoders. However, looking at the CXL_REGION_F_AUTO block immediately following this diff in drivers/cxl/core/region.c:cxl_port_setup_targets(): if (test_bit(CXL_REGION_F_AUTO, &cxlr->flags)) { if (cxld->interleave_ways != iw || (iw > 1 && cxld->interleave_granularity != ig) || !spa_maps_hpa(p, &cxld->hpa_range) || ((cxld->flags & CXL_DECODER_F_ENABLE) == 0)) { dev_err(&cxlr->dev, When iw == 1, does this code fail to update cxld->interleave_granularity with the computed logical ig? If cxld->interleave_granularity is not updated, it seems descendant decoders will inherit a stale hardware don't-care value instead of the true accumulated granularity. Could this cause the kernel to accept physically broken topologies during auto-discovery, potentially leading to silent memory corruption or Machine Check Exceptions (MCEs) when the region is accessed? -- Sashiko AI review · https://sashiko.dev/#/patchset/cover.1788487020.git.alison.schofield@intel.com?part=1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4 1/2] cxl/region: Allow regions with an unencodable passthrough granularity 2026-09-04 3:36 ` sashiko-bot @ 2026-09-04 3:53 ` Alison Schofield 0 siblings, 0 replies; 5+ messages in thread From: Alison Schofield @ 2026-09-04 3:53 UTC (permalink / raw) To: sashiko-reviews; +Cc: linux-cxl On Fri, Sep 04, 2026 at 03:36:24AM +0000, sashiko-bot@kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [High] During region auto-discovery, the driver relaxes the interleave granularity validation for non-interleaving passthrough decoders, but fails to update the decoder's software struct (`cxld->interleave_granularity`) with the computed logical value. As a result, descendant decoders inherit a stale hardware don't-care value. Same NO as in v1[1] (and in v2, v3) Gratuitously updating the field is not the right thing to do. [1] https://lore.kernel.org/linux-cxl/alKP8OP4DNT2nJnu@aschofie-mobl2.lan/#r No. A downstream decoder can't inherit an unecodable value into a failing check. A >16K granularity only appears below a wide interleave, and everything below that inherits an equal-or-larger value that an interleaving decoder couldn't encode. So every descendant is necessarily passthrough and never validates granularity. There is no path where updating the F_AUTO value would prevent a failure because the failure cannot happen. > -- > > commit 581b6f11f4646ac877c9eaf9fb81a155a40889d5 > Author: Alison Schofield <alison.schofield@intel.com> > > cxl/region: Allow regions with an unencodable passthrough granularity > > Region creation fails when the interleave granularity accumulated below the > root decoder exceeds 16K, the maximum encodable in the HDM Decoder Control > IG field, even when the decoder that computes it does not interleave. > > This patch keeps the computed value for passthrough decoders so it can > seed descendant decoder setup. > > Closes: https://lore.kernel.org/all/20260530002613.682E71F00893@smtp.kernel.org/ > > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > > index 27e63e6dab7c8..89693f0181ddb 100644 > > --- a/drivers/cxl/core/region.c > > +++ b/drivers/cxl/core/region.c > > [ ... ] > > > @@ -1550,20 +1562,12 @@ static int cxl_port_setup_targets(struct cxl_port *port, > > * Interleave granularity is a multiple of @parent_port granularity. > > * Multiplier is the parent port interleave ways. > > */ > > - rc = granularity_to_eig(parent_ig * parent_iw, &eig); > > - if (rc) { > > + ig = parent_ig * parent_iw; > > + if (!granularity_is_valid(iw, ig)) { > > dev_dbg(&cxlr->dev, > > "%s: invalid granularity calculation (%d * %d)\n", > > dev_name(&parent_port->dev), parent_ig, parent_iw); > > - return rc; > > - } > > - > > - rc = eig_to_granularity(eig, &ig); > > - if (rc) { > > - dev_dbg(&cxlr->dev, "%s:%s: invalid interleave: %d\n", > > - dev_name(port->uport_dev), dev_name(&port->dev), > > - 256 << eig); > > - return rc; > > + return -EINVAL; > > } > > > > if (iw > 8 || iw > cxlsd->nr_targets) { > > [Severity: High] > During region auto-discovery, the driver relaxes the interleave granularity > validation for non-interleaving passthrough decoders. However, looking at the > CXL_REGION_F_AUTO block immediately following this diff in > drivers/cxl/core/region.c:cxl_port_setup_targets(): > > if (test_bit(CXL_REGION_F_AUTO, &cxlr->flags)) { > if (cxld->interleave_ways != iw || > (iw > 1 && cxld->interleave_granularity != ig) || > !spa_maps_hpa(p, &cxld->hpa_range) || > ((cxld->flags & CXL_DECODER_F_ENABLE) == 0)) { > dev_err(&cxlr->dev, > > When iw == 1, does this code fail to update cxld->interleave_granularity > with the computed logical ig? > > If cxld->interleave_granularity is not updated, it seems descendant decoders > will inherit a stale hardware don't-care value instead of the true accumulated > granularity. Could this cause the kernel to accept physically broken topologies > during auto-discovery, potentially leading to silent memory corruption or > Machine Check Exceptions (MCEs) when the region is accessed? > > -- > Sashiko AI review · https://sashiko.dev/#/patchset/cover.1788487020.git.alison.schofield@intel.com?part=1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v4 2/2] cxl/test: Add a 2-way 16K root decoder for passthrough testing 2026-09-04 3:21 [PATCH v4 0/2] cxl: Allow passthrough decoders with >16K granularity Alison Schofield 2026-09-04 3:21 ` [PATCH v4 1/2] cxl/region: Allow regions with an unencodable passthrough granularity Alison Schofield @ 2026-09-04 3:21 ` Alison Schofield 1 sibling, 0 replies; 5+ messages in thread From: Alison Schofield @ 2026-09-04 3:21 UTC (permalink / raw) To: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Alison Schofield, Vishal Verma, Ira Weiny, Li Ming Cc: linux-cxl, Richard Cheng The cxl-test topologies do not provide a configuration where passthrough decoders compute a granularity above the 16K max encodable in hardware. The widest power-of-2 root decoder currently available is 2-way at 4K granularity, so a passthrough decoder below it computes 8K and the granularity accumulated at the endpoints tops out at 16K. Add a CFMWS entry for a 2-way root decoder at 16K granularity. This topology supports a region configuration spanning the two multi-port host bridges and placing one endpoint under each bridge. The intermediate switch decoders are then passthrough decoders with a computed granularity of 32K. This allows a CXL unit test to be added for this case. Place the new window in the XOR topology set so the default modulo topology used by most cxl-test coverage is unchanged. Assisted-by: Claude:Opus-4-8 Reviewed-by: Richard Cheng <icheng@nvidia.com> Reviewed-by: Dave Jiang <dave.jiang@intel.com> Signed-off-by: Alison Schofield <alison.schofield@intel.com> --- tools/testing/cxl/test/cxl.c | 48 ++++++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-) diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c index 62bd92b3be45..08ecb9b74372 100644 --- a/tools/testing/cxl/test/cxl.c +++ b/tools/testing/cxl/test/cxl.c @@ -190,10 +190,18 @@ static struct { struct acpi_cedt_cfmws cfmws; u32 target[3]; } cfmws8; + struct { + struct acpi_cedt_cfmws cfmws; + u32 target[2]; + } cfmws9; struct { struct acpi_cedt_cxims cxims; u64 xormap_list[2]; } cxims0; + struct { + struct acpi_cedt_cxims cxims; + u64 xormap_list[1]; + } cxims1; } __packed mock_cedt = { .cedt = { .header = { @@ -373,6 +381,28 @@ static struct { }, .target = { 0, 1, 2, }, }, + /* + * A 2-way root at 16K granularity. A passthrough decoder below this + * root computes a 32K granularity (16K * 2), which exceeds the maximum + * encodable in hardware. It exercises the passthrough granularity path + * in cxl_port_setup_targets(). + */ + .cfmws9 = { + .cfmws = { + .header = { + .type = ACPI_CEDT_TYPE_CFMWS, + .length = sizeof(mock_cedt.cfmws9), + }, + .interleave_arithmetic = ACPI_CEDT_CFMWS_ARITHMETIC_XOR, + .interleave_ways = 1, + .granularity = 6, + .restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM | + ACPI_CEDT_CFMWS_RESTRICT_PMEM, + .qtg_id = FAKE_QTG_ID, + .window_size = SZ_256M * 8UL, + }, + .target = { 0, 1, }, + }, .cxims0 = { .cxims = { .header = { @@ -384,6 +414,18 @@ static struct { }, .xormap_list = { 0x404100, 0x808200, }, }, + /* CXIMS for the 16K (hbig = 6) 2-way root, cfmws9 */ + .cxims1 = { + .cxims = { + .header = { + .type = ACPI_CEDT_TYPE_CXIMS, + .length = sizeof(mock_cedt.cxims1), + }, + .hbig = 6, + .nr_xormaps = 1, + }, + .xormap_list = { 0x0, }, + }, }; static struct acpi_cedt_cfmws type2_cfmws0 = { @@ -410,6 +452,7 @@ struct acpi_cedt_cfmws *mock_cfmws[] = { [6] = &mock_cedt.cfmws6.cfmws, [7] = &mock_cedt.cfmws7.cfmws, [8] = &mock_cedt.cfmws8.cfmws, + [9] = &mock_cedt.cfmws9.cfmws, }; static int cfmws_start; @@ -417,10 +460,11 @@ static int cfmws_end; #define CFMWS_MOD_ARRAY_START 0 #define CFMWS_MOD_ARRAY_END 5 #define CFMWS_XOR_ARRAY_START 6 -#define CFMWS_XOR_ARRAY_END 8 +#define CFMWS_XOR_ARRAY_END 9 -struct acpi_cedt_cxims *mock_cxims[1] = { +struct acpi_cedt_cxims *mock_cxims[2] = { [0] = &mock_cedt.cxims0.cxims, + [1] = &mock_cedt.cxims1.cxims, }; struct cxl_mock_res { -- 2.37.3 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-09-04 3:53 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-09-04 3:21 [PATCH v4 0/2] cxl: Allow passthrough decoders with >16K granularity Alison Schofield 2026-09-04 3:21 ` [PATCH v4 1/2] cxl/region: Allow regions with an unencodable passthrough granularity Alison Schofield 2026-09-04 3:36 ` sashiko-bot 2026-09-04 3:53 ` Alison Schofield 2026-09-04 3:21 ` [PATCH v4 2/2] cxl/test: Add a 2-way 16K root decoder for passthrough testing Alison Schofield
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox