Re: [PATCH v2] cxl/hdm: Enforce CFMWS memory type policy at decoder commit time
From: Mayank Rana
Date: Fri Jul 24 2026 - 19:09:59 EST
Hi Dave
On 7/23/2026 4:47 PM, Dave Jiang wrote:
I review proposed Davidlohr fixes, and seeing that cxl_region_attach() will prevent this situation. Agree, this patch next version is not needed.
On 7/23/26 4:18 PM, Mayank Rana wrote:
Hi Dave
On 7/15/2026 3:26 PM, Mayank Rana wrote:
Hi DaveThanks for the pointer -- I looked closely at cxl_region_decode_reset()
Thank you for review comments.
On 7/13/2026 8:47 AM, Dave Jiang wrote:
ok. noted down.
On 7/10/26 5:33 PM, Mayank Rana wrote:
A CXL Fixed Memory Window Structure (CFMWS) in the ACPI CEDT table
carries a restrictions field that describes the memory types a platform
window supports. cfmws_to_decoder_flags() translates this into
CXL_DECODER_F_TYPE2 (HDM-DB permitted) and CXL_DECODER_F_TYPE3 (HDM-H
permitted) flags on the root decoder.
However, nothing currently prevents a caller from committing an endpoint
or switch decoder whose target_type conflicts with the CFMWS policy:
- A decoder with target_type == CXL_DECODER_DEVMEM (HDM-DB) can be
committed on a window that lacks CXL_DECODER_F_TYPE2 (HDM-H only).
A dual-capable device will accept HOSTONLY=0 + COMMIT=1 without
asserting COMMIT_ERROR, silently violating platform policy.
- Symmetrically, a decoder with target_type == CXL_DECODER_HOSTONLYMEM
(HDM-H) can be committed on a window that lacks CXL_DECODER_F_TYPE3
(HDM-DB only), again violating ACPI CFMWS restrictions.
Add a check in cxl_decoder_commit() that walks from the decoder's
assigned region to the root decoder and rejects the commit with
-EOPNOTSUPP if the decoder's target_type requires a capability flag that
the CFMWS window does not advertise:
- CXL_DECODER_DEVMEM requires CXL_DECODER_F_TYPE2 on root decoder
- CXL_DECODER_HOSTONLYMEM requires CXL_DECODER_F_TYPE3 on the root
decoder
required_flag is initialized to zero so that any future target_type
values not covered by the if/else-if chain leave the enforcement check
a no-op via the leading required_flag && guard.
This makes the CFMWS restrictions field authoritative for memory type
enforcement in both directions, regardless of individual device capability.
The existing COMMIT_ERROR path in cxld_await_commit() remains as a
secondary safeguard for devices that cannot support the requested mode.
Can you please trim the commit log? The AI generated verbiage is excessively wordy. A simple and more to the point short log that conveys all the information would be appreciated.
will refer suggested API, and rework upon this.
Fixes: 3e23d17ce198 ("cxl/acpi: Use the ACPI CFMWS to create static decoder objects")
Assisted-by: Claude:claude-sonnet-4-6
Signed-off-by: Mayank Rana <mayank.rana@xxxxxxxxxxxxxxxx>
---
- Initialize required_flag = 0 and use explicit else-if for
CXL_DECODER_HOSTONLYMEM; guard enforcement with required_flag &&
so unknown target_type values skip the check cleanly
Changes in v2:
- Extend enforcement to cover both directions: HDM-H commits on
HDM-DB-only windows are now rejected in addition to HDM-DB commits
on HDM-H-only windows (reported by Sashiko AI review)
- Generalize condition from single DEVMEM check to type-dispatch
selecting required_flag based on target_type
- Update commit message and subject to reflect bidirectional policy
Testing
-------
The bug and fix were validated using QEMU with a modified CXL configuration.
Two test-only changes were applied (not part of this patch):
1. QEMU ACPI (hw/acpi/cxl.c): CFMWS restrictions field changed from
0x0f to 0x02 (ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM only, without
RESTRICT_DEVMEM). This causes cfmws_to_decoder_flags() to set
CXL_DECODER_F_TYPE3 only on the root decoder, simulating a platform
that does not permit HDM-DB.
2. Kernel (drivers/cxl/core/hdm.c): init_hdm_decoder() uncommitted
endpoint path changed to force CXL_DECODER_DEVMEM unconditionally,
simulating a dual-capable device (supports both HDM-H and HDM-DB).
QEMU's cxl-type3 device reports CXL_DEVTYPE_CLASSMEM and defaults
to HOSTONLYMEM; this override exercises the DEVMEM commit path that
a real Type-2 or dual-capable Type-3 device would trigger.
Bug reproduction (without this patch):
# cxl create-region -m mem0 -d decoder0.0 -t pmem
created 1 region
HDM-DB committed silently despite CFMWS advertising HDM-H only.
No error or warning in dmesg.
Fix validation (with this patch):
# cxl create-region -m mem0 -d decoder0.0 -t pmem
cxl region: cmd_create_region: created 0 regions
dmesg:
cxl_core: cxl region0: mem0:decoder2.0 type mismatch: 2 vs 3
cxl_port endpoint2: failed to attach decoder2.0 to region0: -6
The decoder's target_type (2=DEVMEM) mismatches the region's required
type (3=HOSTONLYMEM) enforced by the CFMWS restriction -- commit blocked.
QEMU invocation:
qemu-system-x86_64 \
-kernel bzImage \
-append "root=/dev/sda rw console=ttyS0" \
-drive file=rootfs.img,index=0,media=disk,format=raw \
-M q35,cxl=on -m 4G,maxmem=8G,slots=8 -smp 4 \
-object memory-backend-file,id=cxl-mem1,share=on,mem-path=/tmp/ cxltest.raw,size=256M \
-object memory-backend-file,id=cxl-lsa1,share=on,mem-path=/tmp/ lsa.raw,size=256M \
-device pxb-cxl,bus_nr=12,bus=pcie.0,id=cxl.1 \
-device cxl-rp,port=0,bus=cxl.1,id=root_port13,chassis=0,slot=2 \
-device cxl-type3,bus=root_port13,persistent-memdev=cxl- mem1,lsa=cxl-lsa1,id=cxl-pmem0,sn=0x1 \
-M cxl-fmw.0.targets.0=cxl.1,cxl-fmw.0.size=4G \
-nographic
drivers/cxl/core/hdm.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index 0c80b76a5f9b..127a187cdadb 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -802,6 +802,7 @@ static int cxl_decoder_commit(struct cxl_decoder *cxld)
struct cxl_port *port = to_cxl_port(cxld->dev.parent);
struct cxl_hdm *cxlhdm = dev_get_drvdata(&port->dev);
void __iomem *hdm = cxlhdm->regs.hdm_decoder;
+ struct cxl_root_decoder *cxlrd;
int id = cxld->id, rc;
if (cxld->flags & CXL_DECODER_F_ENABLE)
@@ -834,6 +835,34 @@ static int cxl_decoder_commit(struct cxl_decoder *cxld)
}
}
+ /*
+ * Enforce CFMWS memory type policy: reject commits where the decoder
+ * target_type conflicts with the root decoder's CFMWS restrictions.
+ * - HDM-DB (DEVMEM) requires CXL_DECODER_F_TYPE2 on the root decoder.
+ * - HDM-H (HOSTONLYMEM) requires CXL_DECODER_F_TYPE3 on the root decoder.
+ * - Unknown target_type values leave required_flag zero; skip enforcement.
+ */
+ if (cxld->region) {
I don't think we should gate the check based on whether cxld->region is valid or not. Can you please take a look at core/ region.c:271:cxl_region_decode_reset() and do something similar to acquire the root decoder by walking up the port hierachy? That should apply the policy check unconditionally. I would also suggest putting this entire block in a helper function.
and want to flag something before sending v3, since a literal port-walk
doesn't fully replace the region lookup here.
cxl_region_decode_reset() doesn't derive the decoder from the port walk
alone: it's called with cxlr already known, and at each port level uses cxl_rr_load(iter, cxlr) -- keyed by the region -- to find the decoder assigned to that region at that port. The walk finds the *port*; cxlr still finds the *decoder*.
The reason that matters for this check specifically: a single CXL root
port can host multiple root decoders, one per CFMWS window, each with
its own restrictions. A pure walk up to is_cxl_root(port) would tell us this decoder's SPA range originates somewhere under that root port, but not *which* of its root decoders' CFMWS restrictions apply -- only the region (or the port's region_ref tracking, which is also region-keyed) can disambiguate that.
So v3 keeps the region-based lookup (cxld->region->dev.parent ->
to_cxl_root_decoder()), but addresses what I take to be the actual concern -- silently skipping the policy check -- by making the check itself unconditional instead of gated:
if (dev_WARN_ONCE(&cxld->dev, !cxld->region,
"commit without region assignment\n"))
return -ENXIO;
ok sounds good
A decoder reaching commit() without a region assigned is now treated as
a bug (loud WARN + rejected commit) rather than something the check quietly steps around. By the time cxl_decoder_commit() runs, the region assignment has already happened during region assembly (cxl_rr_assign_decoder()), so this should not be reachable in practice. Also pulled the whole block into cxl_decoder_cfmws_check() as suggested.
v3 to follow shortly with this and the F_DEVMEM/F_HOSTONLY rename per
Davidlohr's BI series.
Davidlohr seems to indicate that with his code series, this patch may not be needed?
Thank you.
Regards,
Mayank>
DJ
Thanks,
Mayank
DJRegards,
+ unsigned long required_flag = 0;
+ const char *type_name;
+
+ cxlrd = to_cxl_root_decoder(cxld->region->dev.parent);
+ if (cxld->target_type == CXL_DECODER_DEVMEM) {
+ required_flag = CXL_DECODER_F_TYPE2;
+ type_name = "HDM-DB";
+ } else if (cxld->target_type == CXL_DECODER_HOSTONLYMEM) {
+ required_flag = CXL_DECODER_F_TYPE3;
+ type_name = "HDM-H";
+ }
+
+ if (required_flag && !(cxlrd->cxlsd.cxld.flags & required_flag)) {
+ dev_err(&port->dev,
+ "%s commit rejected on %s, CFMWS does not permit this memory type\n",
+ type_name, dev_name(&cxld->dev));
+ return -EOPNOTSUPP;
+ }
+ }
+
scoped_guard(rwsem_read, &cxl_rwsem.dpa)
setup_hw_decoder(cxld, hdm);
Mayank