mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] cxl/hdm: Enforce CFMWS memory type policy at decoder commit time
@ 2026-07-11  0:33 Mayank Rana
  2026-07-13 15:47 ` Dave Jiang
  2026-07-13 16:04 ` Davidlohr Bueso
  0 siblings, 2 replies; 7+ messages in thread
From: Mayank Rana @ 2026-07-11  0:33 UTC (permalink / raw)
  To: linux-cxl
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	linux-kernel, Mayank Rana

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.

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@oss.qualcomm.com>
---
  - 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) {
+		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);
 
-- 
2.34.1

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] cxl/hdm: Enforce CFMWS memory type policy at decoder commit time
  2026-07-11  0:33 [PATCH v2] cxl/hdm: Enforce CFMWS memory type policy at decoder commit time Mayank Rana
@ 2026-07-13 15:47 ` Dave Jiang
  2026-07-15 22:26   ` Mayank Rana
  2026-07-13 16:04 ` Davidlohr Bueso
  1 sibling, 1 reply; 7+ messages in thread
From: Dave Jiang @ 2026-07-13 15:47 UTC (permalink / raw)
  To: Mayank Rana, linux-cxl
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	linux-kernel



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.

> 
> 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@oss.qualcomm.com>
> ---
>   - 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.

DJ

> +		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);
>  


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] cxl/hdm: Enforce CFMWS memory type policy at decoder commit time
  2026-07-11  0:33 [PATCH v2] cxl/hdm: Enforce CFMWS memory type policy at decoder commit time Mayank Rana
  2026-07-13 15:47 ` Dave Jiang
@ 2026-07-13 16:04 ` Davidlohr Bueso
  2026-07-15 22:35   ` Mayank Rana
  1 sibling, 1 reply; 7+ messages in thread
From: Davidlohr Bueso @ 2026-07-13 16:04 UTC (permalink / raw)
  To: Mayank Rana
  Cc: linux-cxl, dan.j.williams, ira.weiny, vishal.l.verma,
	alison.schofield, linux-kernel

On Fri, 10 Jul 2026, 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:

Hi Mayank - please refer to the latest Back Invalidate patchset (v5)
for properly dealing with this:

https://lore.kernel.org/all/20260615145529.13848-1-dave@stgolabs.net/

Thanks,
Davidlohr

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] cxl/hdm: Enforce CFMWS memory type policy at decoder commit time
  2026-07-13 15:47 ` Dave Jiang
@ 2026-07-15 22:26   ` Mayank Rana
  0 siblings, 0 replies; 7+ messages in thread
From: Mayank Rana @ 2026-07-15 22:26 UTC (permalink / raw)
  To: Dave Jiang, linux-cxl
  Cc: dan.j.williams, ira.weiny, vishal.l.verma, alison.schofield,
	linux-kernel

Hi Dave

Thank you for review comments.

On 7/13/2026 8:47 AM, Dave Jiang wrote:
> 
> 
> 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.
ok. noted down.
>>
>> 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@oss.qualcomm.com>
>> ---
>>    - 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.
will refer suggested API, and rework upon this.
> DJ
> 
>> +		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);
>>   
> 
Regards,
Mayank


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] cxl/hdm: Enforce CFMWS memory type policy at decoder commit time
  2026-07-13 16:04 ` Davidlohr Bueso
@ 2026-07-15 22:35   ` Mayank Rana
  0 siblings, 0 replies; 7+ messages in thread
From: Mayank Rana @ 2026-07-15 22:35 UTC (permalink / raw)
  To: Davidlohr Bueso
  Cc: linux-cxl, dan.j.williams, ira.weiny, vishal.l.verma,
	alison.schofield, linux-kernel

Hi Davidlohr

Thank you for your review and suggestion.

On 7/13/2026 9:04 AM, Davidlohr Bueso wrote:
> On Fri, 10 Jul 2026, 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:
> 
> Hi Mayank - please refer to the latest Back Invalidate patchset (v5)
> for properly dealing with this:
> 
> https://lore.kernel.org/all/20260615145529.13848-1-dave@stgolabs.net/
Ok. I will review propose changes, and rework upon this fix.

Regards,
Mayank

> Thanks,
> Davidlohr


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] cxl/hdm: Enforce CFMWS memory type policy at decoder commit time
  2026-07-09 21:47 Mayank Rana
@ 2026-07-09 22:04 ` Mayank Rana
  0 siblings, 0 replies; 7+ messages in thread
From: Mayank Rana @ 2026-07-09 22:04 UTC (permalink / raw)
  To: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Alison Schofield,
	Vishal Verma, Dan Williams
  Cc: Ira Weiny, linux-cxl, linux-kernel

Sorry for sending this patch as it was diff to previous v1.
Please disregard this patch. I will review again and send update v2.

Regards,
Mayank

On 7/9/2026 2:47 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.
> 
> 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@oss.qualcomm.com>
> ---
> 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
>    - 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
> 
> Testing
> -------
> 
> The bug and fix were validated using QEMU with a modified CXL configuration
> on linux-next (v7.2-rc2).
> 
> Two test-only kernel changes were applied (not part of this patch):
> 
>    1. drivers/cxl/acpi.c: cfmws_to_decoder_flags() result overridden to
>       clear CXL_DECODER_F_TYPE2 and retain only CXL_DECODER_F_TYPE3,
>       simulating a CFMWS that restricts to HDM-H only (no HDM-DB).
> 
>    2. drivers/cxl/core/hdm.c: cxl_decoder_commit() forced to set
>       target_type = CXL_DECODER_DEVMEM for endpoint decoders before the
>       CFMWS check, simulating a dual-capable device committing HDM-DB.
> 
> Bug reproduction (without this patch):
> 
>    # cxl create-region -m mem0 -d decoder0.0 -t pmem
>    cxl region: cmd_create_region: 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: create_region: region0: failed to commit decode: Operation not supported
>    cxl region: cmd_create_region: created 0 regions
> 
>    dmesg:
>      cxl_port endpoint2: HDM-DB commit rejected on decoder2.0, \
>          CFMWS does not permit this memory type
> 
>    HDM-DB commit blocked with -EOPNOTSUPP as expected.
> 
> 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 | 25 ++++++++++++++++++++-----
>   1 file changed, 20 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index c26dab185dc8..127a187cdadb 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -836,14 +836,29 @@ static int cxl_decoder_commit(struct cxl_decoder *cxld)
>   	}
>   
>   	/*
> -	 * Enforce HDM-H: reject HDM-DB if CFMWS lacks CXL_DECODER_F_TYPE2.
> +	 * 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->target_type == CXL_DECODER_DEVMEM && cxld->region) {
> +	if (cxld->region) {
> +		unsigned long required_flag = 0;
> +		const char *type_name;
> +
>   		cxlrd = to_cxl_root_decoder(cxld->region->dev.parent);
> -		if (!(cxlrd->cxlsd.cxld.flags & CXL_DECODER_F_TYPE2)) {
> +		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,
> -				"HDM-DB commit rejected on %s, CFMWS restricts to HDM-H only\n",
> -				dev_name(&cxld->dev));
> +				"%s commit rejected on %s, CFMWS does not permit this memory type\n",
> +				type_name, dev_name(&cxld->dev));
>   			return -EOPNOTSUPP;
>   		}
>   	}


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2] cxl/hdm: Enforce CFMWS memory type policy at decoder commit time
@ 2026-07-09 21:47 Mayank Rana
  2026-07-09 22:04 ` Mayank Rana
  0 siblings, 1 reply; 7+ messages in thread
From: Mayank Rana @ 2026-07-09 21:47 UTC (permalink / raw)
  To: Davidlohr Bueso, Jonathan Cameron, Dave Jiang, Alison Schofield,
	Vishal Verma, Dan Williams
  Cc: Ira Weiny, linux-cxl, linux-kernel, Mayank Rana

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.

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@oss.qualcomm.com>
---
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
  - 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

Testing
-------

The bug and fix were validated using QEMU with a modified CXL configuration
on linux-next (v7.2-rc2).

Two test-only kernel changes were applied (not part of this patch):

  1. drivers/cxl/acpi.c: cfmws_to_decoder_flags() result overridden to
     clear CXL_DECODER_F_TYPE2 and retain only CXL_DECODER_F_TYPE3,
     simulating a CFMWS that restricts to HDM-H only (no HDM-DB).

  2. drivers/cxl/core/hdm.c: cxl_decoder_commit() forced to set
     target_type = CXL_DECODER_DEVMEM for endpoint decoders before the
     CFMWS check, simulating a dual-capable device committing HDM-DB.

Bug reproduction (without this patch):

  # cxl create-region -m mem0 -d decoder0.0 -t pmem
  cxl region: cmd_create_region: 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: create_region: region0: failed to commit decode: Operation not supported
  cxl region: cmd_create_region: created 0 regions

  dmesg:
    cxl_port endpoint2: HDM-DB commit rejected on decoder2.0, \
        CFMWS does not permit this memory type

  HDM-DB commit blocked with -EOPNOTSUPP as expected.

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 | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index c26dab185dc8..127a187cdadb 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -836,14 +836,29 @@ static int cxl_decoder_commit(struct cxl_decoder *cxld)
 	}
 
 	/*
-	 * Enforce HDM-H: reject HDM-DB if CFMWS lacks CXL_DECODER_F_TYPE2.
+	 * 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->target_type == CXL_DECODER_DEVMEM && cxld->region) {
+	if (cxld->region) {
+		unsigned long required_flag = 0;
+		const char *type_name;
+
 		cxlrd = to_cxl_root_decoder(cxld->region->dev.parent);
-		if (!(cxlrd->cxlsd.cxld.flags & CXL_DECODER_F_TYPE2)) {
+		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,
-				"HDM-DB commit rejected on %s, CFMWS restricts to HDM-H only\n",
-				dev_name(&cxld->dev));
+				"%s commit rejected on %s, CFMWS does not permit this memory type\n",
+				type_name, dev_name(&cxld->dev));
 			return -EOPNOTSUPP;
 		}
 	}
-- 
2.34.1

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-07-15 22:35 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-11  0:33 [PATCH v2] cxl/hdm: Enforce CFMWS memory type policy at decoder commit time Mayank Rana
2026-07-13 15:47 ` Dave Jiang
2026-07-15 22:26   ` Mayank Rana
2026-07-13 16:04 ` Davidlohr Bueso
2026-07-15 22:35   ` Mayank Rana
  -- strict thread matches above, loose matches on Subject: below --
2026-07-09 21:47 Mayank Rana
2026-07-09 22:04 ` Mayank Rana

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome