Linux PCI subsystem development
 help / color / mirror / Atom feed
* [PATCH v2 0/2] PCI/CXL: Distinguish CXL capabilities in MCAP
@ 2026-08-17  7:58 penn
  2026-08-17  7:58 ` [PATCH v2 1/2] PCI: Add MMIO Capabilities Register Block definitions penn
  2026-08-17  7:58 ` [PATCH v2 2/2] cxl/core: Skip non-CXL capabilities in MCAP penn
  0 siblings, 2 replies; 11+ messages in thread
From: penn @ 2026-08-17  7:58 UTC (permalink / raw)
  To: linux-cxl
  Cc: linux-pci, linux-kernel, bhelgaas, dave, jic23, dave.jiang,
	alison.schofield, vishal.l.verma, ira.weiny, djbw, johnny.li,
	peng.guo, jingzhong.yang, Penn

From: Penn <engguopeng@buaa.edu.cn>

When MMPT is enabled, a device may expose PCI-SIG-defined MMPT
capabilities alongside CXL-defined capabilities in the same MMIO
Capabilities Register Block (MCAP).

MCAP capability IDs are scoped by Vendor ID, but the CXL capability
parser currently matches entries using only their capability IDs. This
can cause MMPT entries to be interpreted as CXL register blocks.

Add the generic PCI MCAP register definitions, then update the CXL
parser to reject capabilities carrying a non-zero Vendor ID other than
PCI_VENDOR_ID_CXL, while preserving compatibility with legacy CXL
capability headers.

Patch 1 adds the PCI MCAP array and capability header definitions.
Patch 2 filters non-CXL capabilities before interpreting their IDs
while continuing to accept legacy capability headers whose Vendor ID
field reads as zero.

The fix has been tested on CXL 1.1 and CXL 3.0 devices. On the
MMPT-enabled device, it prevents both Mailbox and Status capabilities
from being misidentified.

Changes in v2:
- Update the MCAP specification citation to PCIe r7.0 sec 6.35.
- Accept a zero Vendor ID for compatibility with legacy CXL devices.
- Move the u16 declaration above the u32 declaration.
- Document testing on CXL 1.1 and CXL 3.0 devices.

Penn (2):
  PCI: Add MMIO Capabilities Register Block definitions
  cxl/core: Skip non-CXL capabilities in MCAP

 drivers/cxl/core/regs.c       | 12 +++++++++++-
 include/uapi/linux/pci_regs.h | 22 ++++++++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletion(-)

-- 
2.43.0


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

* [PATCH v2 1/2] PCI: Add MMIO Capabilities Register Block definitions
  2026-08-17  7:58 [PATCH v2 0/2] PCI/CXL: Distinguish CXL capabilities in MCAP penn
@ 2026-08-17  7:58 ` penn
  2026-08-17  8:07   ` sashiko-bot
  2026-08-18 22:37   ` Dave Jiang
  2026-08-17  7:58 ` [PATCH v2 2/2] cxl/core: Skip non-CXL capabilities in MCAP penn
  1 sibling, 2 replies; 11+ messages in thread
From: penn @ 2026-08-17  7:58 UTC (permalink / raw)
  To: linux-cxl
  Cc: linux-pci, linux-kernel, bhelgaas, dave, jic23, dave.jiang,
	alison.schofield, vishal.l.verma, ira.weiny, djbw, johnny.li,
	peng.guo, jingzhong.yang, Penn

From: Penn <engguopeng@buaa.edu.cn>

PCIe r7.0 sec 6.35 defines the MMIO Capabilities Register Block
(MCAP). It consists of an array header followed by capability headers
that identify the version, location, size, and vendor of each MMIO
capability.

Define the MCAP array-header and capability-header registers and their
fields. These definitions allow subsystem drivers to use the Vendor ID
along with the Capability ID when capabilities from multiple vendors
share the same MCAP.

Signed-off-by: Penn <engguopeng@buaa.edu.cn>
---
Changes in v2:
- Update the MCAP specification citation to PCIe r7.0 sec 6.35.
- Document that capability header indices are one-based.

 include/uapi/linux/pci_regs.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index 14f634ab9350..4dd22cde9e71 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -1338,6 +1338,28 @@
 #define  PCI_IDE_SEL_ADDR_3(x)		(28 + (x) * PCI_IDE_SEL_ADDR_BLOCK_SIZE)
 #define PCI_IDE_SEL_BLOCK_SIZE(nr_assoc)  (20 + PCI_IDE_SEL_ADDR_BLOCK_SIZE * (nr_assoc))
 
+/* PCIe r7.0, sec 6.35: MMIO Capabilities Register Block (MCAP) */
+#define PCI_MCAP_ARRAY_1		0x00
+#define  PCI_MCAP_ARRAY_ID		__GENMASK(15, 0)
+#define  PCI_MCAP_ARRAY_VERSION		__GENMASK(23, 16)
+#define  PCI_MCAP_ARRAY_TYPE		__GENMASK(27, 24)
+#define  PCI_MCAP_ARRAY_TYPE_CLASS_CODE	0x0
+#define PCI_MCAP_ARRAY_2		0x04
+#define  PCI_MCAP_ARRAY_COUNT		__GENMASK(15, 0)
+
+/* Capability header indices are one-based. */
+#define PCI_MCAP_HDR_SIZEOF		0x10
+#define PCI_MCAP_HDR_BASE(n)		((n) * PCI_MCAP_HDR_SIZEOF)
+#define PCI_MCAP_HDR_REG_1		0x00
+#define  PCI_MCAP_CAP_ID		__GENMASK(15, 0)
+#define  PCI_MCAP_CAP_VERSION		__GENMASK(23, 16)
+#define PCI_MCAP_HDR_REG_2		0x04
+#define  PCI_MCAP_HDR_OFFSET		__GENMASK(31, 0)
+#define PCI_MCAP_HDR_REG_3		0x08
+#define  PCI_MCAP_HDR_LENGTH		__GENMASK(31, 0)
+#define PCI_MCAP_HDR_REG_4		0x0c
+#define  PCI_MCAP_HDR_VENDOR_ID		__GENMASK(15, 0)
+
 /*
  * Compute Express Link (CXL r4.0, sec 8.1)
  *
-- 
2.43.0


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

* [PATCH v2 2/2] cxl/core: Skip non-CXL capabilities in MCAP
  2026-08-17  7:58 [PATCH v2 0/2] PCI/CXL: Distinguish CXL capabilities in MCAP penn
  2026-08-17  7:58 ` [PATCH v2 1/2] PCI: Add MMIO Capabilities Register Block definitions penn
@ 2026-08-17  7:58 ` penn
  2026-08-17  8:10   ` sashiko-bot
                     ` (2 more replies)
  1 sibling, 3 replies; 11+ messages in thread
From: penn @ 2026-08-17  7:58 UTC (permalink / raw)
  To: linux-cxl
  Cc: linux-pci, linux-kernel, bhelgaas, dave, jic23, dave.jiang,
	alison.schofield, vishal.l.verma, ira.weiny, djbw, johnny.li,
	peng.guo, jingzhong.yang, Penn

From: Penn <engguopeng@buaa.edu.cn>

When MMPT is enabled, a CXL device may expose CXL-defined capabilities
and PCIe Management Message Passthrough (MMPT) capabilities in the same
MMIO Capabilities Register Block (MCAP).

The CXL capability parser currently identifies entries using only the
capability ID. Since capability IDs are scoped by Vendor ID, a
PCI-SIG-defined capability may have the same ID as a CXL-defined
capability. This causes non-CXL entries, including MMPT and MMIO Mailbox,
to be interpreted as CXL register blocks.

The MMPT register block may be interpreted as a CXL mailbox. This
causes mailbox initialization to fail with:

    cxl_pci 0000:3b:00.0: Mailbox is too small (64b)

Check the MCAP Vendor ID before interpreting an entry as a CXL-defined
capability. Skip capabilities with a non-zero Vendor ID other than
PCI_VENDOR_ID_CXL.

In legacy CXL capability headers, the field now used for the MCAP
Vendor ID was reserved and reads as zero. Continue to accept zero to
preserve compatibility with those devices.

The fix has been tested on CXL 1.1 and CXL 3.0 devices. On the
MMPT-enabled device, it prevents both Mailbox and Status capabilities
from being misidentified.

Fixes: 8adaf747c9f0 ("cxl/mem: Find device capabilities")
Suggested-by: Johnny <johnny.li@montage-tech.com>
Signed-off-by: Penn <engguopeng@buaa.edu.cn>
---
Changes in v2:
- Accept a zero Vendor ID for compatibility with legacy CXL capability
  headers.
- Move the u16 declaration above the u32 declaration to follow the
  reverse Christmas tree convention.
- Document testing on CXL 1.1 and CXL 3.0 devices.

 drivers/cxl/core/regs.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
index 93710cf4f0a6..c7c14089f2c6 100644
--- a/drivers/cxl/core/regs.c
+++ b/drivers/cxl/core/regs.c
@@ -133,8 +133,18 @@ void cxl_probe_device_regs(struct device *dev, void __iomem *base,
 
 	for (cap = 1; cap <= cap_count; cap++) {
 		struct cxl_reg_map *rmap;
+		u16 cap_id, vendor_id;
 		u32 offset, length;
-		u16 cap_id;
+
+		vendor_id = FIELD_GET(PCI_MCAP_HDR_VENDOR_ID,
+				      readl(base + PCI_MCAP_HDR_BASE(cap) +
+					    PCI_MCAP_HDR_REG_4));
+		/*
+		 * The Vendor ID field is reserved and reads as zero in legacy
+		 * CXL capability headers. See CXL r3.2, Table 8-44.
+		 */
+		if (vendor_id && vendor_id != PCI_VENDOR_ID_CXL)
+			continue;
 
 		cap_id = FIELD_GET(CXLDEV_CAP_HDR_CAP_ID_MASK,
 				   readl(base + cap * 0x10));
-- 
2.43.0


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

* Re: [PATCH v2 1/2] PCI: Add MMIO Capabilities Register Block definitions
  2026-08-17  7:58 ` [PATCH v2 1/2] PCI: Add MMIO Capabilities Register Block definitions penn
@ 2026-08-17  8:07   ` sashiko-bot
  2026-08-18 22:37   ` Dave Jiang
  1 sibling, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-08-17  8:07 UTC (permalink / raw)
  To: penn; +Cc: linux-cxl, linux-pci

> From: Penn <engguopeng@buaa.edu.cn>
> 
> PCIe r7.0 sec 6.35 defines the MMIO Capabilities Register Block
> (MCAP). It consists of an array header followed by capability headers
> that identify the version, location, size, and vendor of each MMIO
> capability.
> 
> Define the MCAP array-header and capability-header registers and their
> fields. These definitions allow subsystem drivers to use the Vendor ID
> along with the Capability ID when capabilities from multiple vendors
> share the same MCAP.
> 
> Signed-off-by: Penn <engguopeng@buaa.edu.cn>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817075854.17207-1-engguopeng@buaa.edu.cn?part=1


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

* Re: [PATCH v2 2/2] cxl/core: Skip non-CXL capabilities in MCAP
  2026-08-17  7:58 ` [PATCH v2 2/2] cxl/core: Skip non-CXL capabilities in MCAP penn
@ 2026-08-17  8:10   ` sashiko-bot
  2026-08-18 22:43   ` Dave Jiang
  2026-08-21 18:01   ` Jonathan Cameron
  2 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-08-17  8:10 UTC (permalink / raw)
  To: penn; +Cc: linux-cxl, linux-pci

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The function blindly trusts the hardware-provided `cap_count` without validating it against the mapped register block size, leading to out-of-bounds MMIO reads.
- [High] Hardware-provided capability `offset` and `length` are not validated against the mapped register block bounds, allowing arbitrary physical memory mapping.
--

--- Patch [2]: [PATCH v2 2/2] cxl/core: Skip non-CXL capabilities in MCAP ---
commit f423958d8928ff9878ca7f5a9766da9989945b54
Author: Penn <engguopeng@buaa.edu.cn>

cxl/core: Skip non-CXL capabilities in MCAP

This commit adds a check for the MCAP Vendor ID before interpreting an entry
as a CXL-defined capability. This prevents non-CXL entries from being
misinterpreted as CXL register blocks when MMPT is enabled.

> diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
> index 20c2d9fbcfe7d..ff1e1cbb9ca8d 100644
> --- a/drivers/cxl/core/regs.c
> +++ b/drivers/cxl/core/regs.c
> @@ -134,8 +134,18 @@ void cxl_probe_device_regs(struct device *dev, void __iomem *base,
>  
>  	for (cap = 1; cap <= cap_count; cap++) {

[Severity: High]
This isn't a bug introduced by this patch, but is it safe to trust the
hardware-provided cap_count without validating it against the mapped register
block size?

A malicious or faulty CXL device could present a large cap_count (up to
65535) but advertise a small PCI BAR. Since cxl_probe_device_regs() iterates
up to cap_count without bounding the loop by the mapped size, the capability
header reads could fall outside the ioremapped memory region and cause a
kernel oops/panic due to a page fault.

>  		struct cxl_reg_map *rmap;
> +		u16 cap_id, vendor_id;
>  		u32 offset, length;
> -		u16 cap_id;
> +
> +		vendor_id = FIELD_GET(PCI_MCAP_HDR_VENDOR_ID,
> +				      readl(base + PCI_MCAP_HDR_BASE(cap) +
> +					    PCI_MCAP_HDR_REG_4));
> +		/*
> +		 * The Vendor ID field is reserved and reads as zero in legacy
> +		 * CXL capability headers. See CXL r3.2, Table 8-44.
> +		 */
> +		if (vendor_id && vendor_id != PCI_VENDOR_ID_CXL)
> +			continue;
>  
>  		cap_id = FIELD_GET(CXLDEV_CAP_HDR_CAP_ID_MASK,
>  				   readl(base + cap * 0x10));
>  		offset = readl(base + cap * 0x10 + 0x4);
>  		length = readl(base + cap * 0x10 + 0x8);

[Severity: High]
This is a pre-existing issue, but does this code properly validate the
untrusted, hardware-provided offset and length?

In cxl_map_device_regs(), these values are used to compute the physical
address for mapping:

cxl_map_device_regs() {
...
    addr = phys_addr + mi->rmap->offset;
    length = mi->rmap->size;
    *(mi->addr) = devm_cxl_iomap_block(host, addr, length);
...
}

Because they are never checked against the CXL register block's actual size
(map->max_size), a device providing arbitrarily large offsets could cause
the kernel to map unrequested physical address space, potentially leading to
unintended MMIO interactions and system instability.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817075854.17207-1-engguopeng@buaa.edu.cn?part=2

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

* Re: [PATCH v2 1/2] PCI: Add MMIO Capabilities Register Block definitions
  2026-08-17  7:58 ` [PATCH v2 1/2] PCI: Add MMIO Capabilities Register Block definitions penn
  2026-08-17  8:07   ` sashiko-bot
@ 2026-08-18 22:37   ` Dave Jiang
  1 sibling, 0 replies; 11+ messages in thread
From: Dave Jiang @ 2026-08-18 22:37 UTC (permalink / raw)
  To: penn, linux-cxl
  Cc: linux-pci, linux-kernel, bhelgaas, dave, jic23, alison.schofield,
	vishal.l.verma, ira.weiny, djbw, johnny.li, peng.guo,
	jingzhong.yang



On 8/17/26 12:58 AM, penn wrote:
> From: Penn <engguopeng@buaa.edu.cn>
> 
> PCIe r7.0 sec 6.35 defines the MMIO Capabilities Register Block
> (MCAP). It consists of an array header followed by capability headers
> that identify the version, location, size, and vendor of each MMIO
> capability.
> 
> Define the MCAP array-header and capability-header registers and their
> fields. These definitions allow subsystem drivers to use the Vendor ID
> along with the Capability ID when capabilities from multiple vendors
> share the same MCAP.
> 
> Signed-off-by: Penn <engguopeng@buaa.edu.cn>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>


> ---
> Changes in v2:
> - Update the MCAP specification citation to PCIe r7.0 sec 6.35.
> - Document that capability header indices are one-based.
> 
>  include/uapi/linux/pci_regs.h | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index 14f634ab9350..4dd22cde9e71 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -1338,6 +1338,28 @@
>  #define  PCI_IDE_SEL_ADDR_3(x)		(28 + (x) * PCI_IDE_SEL_ADDR_BLOCK_SIZE)
>  #define PCI_IDE_SEL_BLOCK_SIZE(nr_assoc)  (20 + PCI_IDE_SEL_ADDR_BLOCK_SIZE * (nr_assoc))
>  
> +/* PCIe r7.0, sec 6.35: MMIO Capabilities Register Block (MCAP) */
> +#define PCI_MCAP_ARRAY_1		0x00
> +#define  PCI_MCAP_ARRAY_ID		__GENMASK(15, 0)
> +#define  PCI_MCAP_ARRAY_VERSION		__GENMASK(23, 16)
> +#define  PCI_MCAP_ARRAY_TYPE		__GENMASK(27, 24)
> +#define  PCI_MCAP_ARRAY_TYPE_CLASS_CODE	0x0
> +#define PCI_MCAP_ARRAY_2		0x04
> +#define  PCI_MCAP_ARRAY_COUNT		__GENMASK(15, 0)
> +
> +/* Capability header indices are one-based. */
> +#define PCI_MCAP_HDR_SIZEOF		0x10
> +#define PCI_MCAP_HDR_BASE(n)		((n) * PCI_MCAP_HDR_SIZEOF)
> +#define PCI_MCAP_HDR_REG_1		0x00
> +#define  PCI_MCAP_CAP_ID		__GENMASK(15, 0)
> +#define  PCI_MCAP_CAP_VERSION		__GENMASK(23, 16)
> +#define PCI_MCAP_HDR_REG_2		0x04
> +#define  PCI_MCAP_HDR_OFFSET		__GENMASK(31, 0)
> +#define PCI_MCAP_HDR_REG_3		0x08
> +#define  PCI_MCAP_HDR_LENGTH		__GENMASK(31, 0)
> +#define PCI_MCAP_HDR_REG_4		0x0c
> +#define  PCI_MCAP_HDR_VENDOR_ID		__GENMASK(15, 0)
> +
>  /*
>   * Compute Express Link (CXL r4.0, sec 8.1)
>   *


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

* Re: [PATCH v2 2/2] cxl/core: Skip non-CXL capabilities in MCAP
  2026-08-17  7:58 ` [PATCH v2 2/2] cxl/core: Skip non-CXL capabilities in MCAP penn
  2026-08-17  8:10   ` sashiko-bot
@ 2026-08-18 22:43   ` Dave Jiang
  2026-08-19  1:28     ` engguopeng
  2026-08-21 18:01   ` Jonathan Cameron
  2 siblings, 1 reply; 11+ messages in thread
From: Dave Jiang @ 2026-08-18 22:43 UTC (permalink / raw)
  To: penn, linux-cxl
  Cc: linux-pci, linux-kernel, bhelgaas, dave, jic23, alison.schofield,
	vishal.l.verma, ira.weiny, djbw, johnny.li, peng.guo,
	jingzhong.yang



On 8/17/26 12:58 AM, penn wrote:
> From: Penn <engguopeng@buaa.edu.cn>
> 
> When MMPT is enabled, a CXL device may expose CXL-defined capabilities
> and PCIe Management Message Passthrough (MMPT) capabilities in the same
> MMIO Capabilities Register Block (MCAP).
> 
> The CXL capability parser currently identifies entries using only the
> capability ID. Since capability IDs are scoped by Vendor ID, a
> PCI-SIG-defined capability may have the same ID as a CXL-defined
> capability. This causes non-CXL entries, including MMPT and MMIO Mailbox,
> to be interpreted as CXL register blocks.
> 
> The MMPT register block may be interpreted as a CXL mailbox. This
> causes mailbox initialization to fail with:
> 
>     cxl_pci 0000:3b:00.0: Mailbox is too small (64b)
> 
> Check the MCAP Vendor ID before interpreting an entry as a CXL-defined
> capability. Skip capabilities with a non-zero Vendor ID other than
> PCI_VENDOR_ID_CXL.
> 
> In legacy CXL capability headers, the field now used for the MCAP
> Vendor ID was reserved and reads as zero. Continue to accept zero to
> preserve compatibility with those devices.
> 
> The fix has been tested on CXL 1.1 and CXL 3.0 devices. On the
> MMPT-enabled device, it prevents both Mailbox and Status capabilities
> from being misidentified.
> 
> Fixes: 8adaf747c9f0 ("cxl/mem: Find device capabilities")
> Suggested-by: Johnny <johnny.li@montage-tech.com>
> Signed-off-by: Penn <engguopeng@buaa.edu.cn>

After addressing Lukas's comment,
Reviewed-by: Dave Jiang <dave.jiang@intel.com>

Given that you are utilizing the new PCIe MMIO MBOX feature, have you considered migrate the CXL MBOX block parsing code to a PCI lib and shared between PCI and CXL? I did attempted something [1] like that a while back but never upstreamed the code due to no hardware to test on.
 
[1]: https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/log/?h=pci-mbox

DJ

> ---
> Changes in v2:
> - Accept a zero Vendor ID for compatibility with legacy CXL capability
>   headers.
> - Move the u16 declaration above the u32 declaration to follow the
>   reverse Christmas tree convention.
> - Document testing on CXL 1.1 and CXL 3.0 devices.
> 
>  drivers/cxl/core/regs.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
> index 93710cf4f0a6..c7c14089f2c6 100644
> --- a/drivers/cxl/core/regs.c
> +++ b/drivers/cxl/core/regs.c
> @@ -133,8 +133,18 @@ void cxl_probe_device_regs(struct device *dev, void __iomem *base,
>  
>  	for (cap = 1; cap <= cap_count; cap++) {
>  		struct cxl_reg_map *rmap;
> +		u16 cap_id, vendor_id;
>  		u32 offset, length;
> -		u16 cap_id;
> +
> +		vendor_id = FIELD_GET(PCI_MCAP_HDR_VENDOR_ID,
> +				      readl(base + PCI_MCAP_HDR_BASE(cap) +
> +					    PCI_MCAP_HDR_REG_4));
> +		/*
> +		 * The Vendor ID field is reserved and reads as zero in legacy
> +		 * CXL capability headers. See CXL r3.2, Table 8-44.
> +		 */
> +		if (vendor_id && vendor_id != PCI_VENDOR_ID_CXL)
> +			continue;
>  
>  		cap_id = FIELD_GET(CXLDEV_CAP_HDR_CAP_ID_MASK,
>  				   readl(base + cap * 0x10));


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

* Re: Re: [PATCH v2 2/2] cxl/core: Skip non-CXL capabilities in MCAP
  2026-08-18 22:43   ` Dave Jiang
@ 2026-08-19  1:28     ` engguopeng
  2026-08-19 15:09       ` Dave Jiang
  0 siblings, 1 reply; 11+ messages in thread
From: engguopeng @ 2026-08-19  1:28 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-cxl, linux-pci, linux-kernel, bhelgaas, dave, jic23,
	alison.schofield, vishal.l.verma, ira.weiny, djbw, johnny.li,
	peng.guo, jingzhong.yang


> On 8/17/26 12:58 AM, penn wrote:
> > From: Penn <engguopeng@buaa.edu.cn>
> > 
> > When MMPT is enabled, a CXL device may expose CXL-defined capabilities
> > and PCIe Management Message Passthrough (MMPT) capabilities in the same
> > MMIO Capabilities Register Block (MCAP).
> > 
> > The CXL capability parser currently identifies entries using only the
> > capability ID. Since capability IDs are scoped by Vendor ID, a
> > PCI-SIG-defined capability may have the same ID as a CXL-defined
> > capability. This causes non-CXL entries, including MMPT and MMIO Mailbox,
> > to be interpreted as CXL register blocks.
> > 
> > The MMPT register block may be interpreted as a CXL mailbox. This
> > causes mailbox initialization to fail with:
> > 
> >     cxl_pci 0000:3b:00.0: Mailbox is too small (64b)
> > 
> > Check the MCAP Vendor ID before interpreting an entry as a CXL-defined
> > capability. Skip capabilities with a non-zero Vendor ID other than
> > PCI_VENDOR_ID_CXL.
> > 
> > In legacy CXL capability headers, the field now used for the MCAP
> > Vendor ID was reserved and reads as zero. Continue to accept zero to
> > preserve compatibility with those devices.
> > 
> > The fix has been tested on CXL 1.1 and CXL 3.0 devices. On the
> > MMPT-enabled device, it prevents both Mailbox and Status capabilities
> > from being misidentified.
> > 
> > Fixes: 8adaf747c9f0 ("cxl/mem: Find device capabilities")
> > Suggested-by: Johnny <johnny.li@montage-tech.com>
> > Signed-off-by: Penn <engguopeng@buaa.edu.cn>
> 
> After addressing Lukas's comment,
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> 
> Given that you are utilizing the new PCIe MMIO MBOX feature, have you considered migrate the CXL MBOX block parsing code to a PCI lib and shared between PCI and CXL? I did attempted something [1] like that a while back but never upstreamed the code due to no hardware to test on.
>  
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/log/?h=pci-mbox
> 
> DJ

Thanks for the review. Lukas's comment has already been addressed in v2,
but I missed his Reviewed-by tag. When applying, please also include:

Reviewed-by: Lukas Wunner <lukas@wunner.de>

I will add it if another revision is needed.

In addition, I will look into moving the CXL mailbox block parsing code to a common
PCI library so that it can be shared with the PCIe MMIO mailbox support.

Since that would be a broader refactoring, I would prefer to keep this
patch focused on fixing the Vendor ID validation issue and handle the
common parsing code in a separate follow-up patch or series. I will also
test the follow-up changes on real CXL hardware with MMPT support.

> > ---
> > Changes in v2:
> > - Accept a zero Vendor ID for compatibility with legacy CXL capability
> >   headers.
> > - Move the u16 declaration above the u32 declaration to follow the
> >   reverse Christmas tree convention.
> > - Document testing on CXL 1.1 and CXL 3.0 devices.
> > 
> >  drivers/cxl/core/regs.c | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
> > index 93710cf4f0a6..c7c14089f2c6 100644
> > --- a/drivers/cxl/core/regs.c
> > +++ b/drivers/cxl/core/regs.c
> > @@ -133,8 +133,18 @@ void cxl_probe_device_regs(struct device *dev, void __iomem *base,
> >  
> >  	for (cap = 1; cap <= cap_count; cap++) {
> >  		struct cxl_reg_map *rmap;
> > +		u16 cap_id, vendor_id;
> >  		u32 offset, length;
> > -		u16 cap_id;
> > +
> > +		vendor_id = FIELD_GET(PCI_MCAP_HDR_VENDOR_ID,
> > +				      readl(base + PCI_MCAP_HDR_BASE(cap) +
> > +					    PCI_MCAP_HDR_REG_4));
> > +		/*
> > +		 * The Vendor ID field is reserved and reads as zero in legacy
> > +		 * CXL capability headers. See CXL r3.2, Table 8-44.
> > +		 */
> > +		if (vendor_id && vendor_id != PCI_VENDOR_ID_CXL)
> > +			continue;
> >  
> >  		cap_id = FIELD_GET(CXLDEV_CAP_HDR_CAP_ID_MASK,
> >  				   readl(base + cap * 0x10));

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

* Re: [PATCH v2 2/2] cxl/core: Skip non-CXL capabilities in MCAP
  2026-08-19  1:28     ` engguopeng
@ 2026-08-19 15:09       ` Dave Jiang
  2026-08-20  2:36         ` engguopeng
  0 siblings, 1 reply; 11+ messages in thread
From: Dave Jiang @ 2026-08-19 15:09 UTC (permalink / raw)
  To: engguopeng
  Cc: linux-cxl, linux-pci, linux-kernel, bhelgaas, dave, jic23,
	alison.schofield, vishal.l.verma, ira.weiny, djbw, johnny.li,
	peng.guo, jingzhong.yang



On 8/18/26 6:28 PM, engguopeng@buaa.edu.cn wrote:
> 
>> On 8/17/26 12:58 AM, penn wrote:
>>> From: Penn <engguopeng@buaa.edu.cn>
>>>
>>> When MMPT is enabled, a CXL device may expose CXL-defined capabilities
>>> and PCIe Management Message Passthrough (MMPT) capabilities in the same
>>> MMIO Capabilities Register Block (MCAP).
>>>
>>> The CXL capability parser currently identifies entries using only the
>>> capability ID. Since capability IDs are scoped by Vendor ID, a
>>> PCI-SIG-defined capability may have the same ID as a CXL-defined
>>> capability. This causes non-CXL entries, including MMPT and MMIO Mailbox,
>>> to be interpreted as CXL register blocks.
>>>
>>> The MMPT register block may be interpreted as a CXL mailbox. This
>>> causes mailbox initialization to fail with:
>>>
>>>     cxl_pci 0000:3b:00.0: Mailbox is too small (64b)
>>>
>>> Check the MCAP Vendor ID before interpreting an entry as a CXL-defined
>>> capability. Skip capabilities with a non-zero Vendor ID other than
>>> PCI_VENDOR_ID_CXL.
>>>
>>> In legacy CXL capability headers, the field now used for the MCAP
>>> Vendor ID was reserved and reads as zero. Continue to accept zero to
>>> preserve compatibility with those devices.
>>>
>>> The fix has been tested on CXL 1.1 and CXL 3.0 devices. On the
>>> MMPT-enabled device, it prevents both Mailbox and Status capabilities
>>> from being misidentified.
>>>
>>> Fixes: 8adaf747c9f0 ("cxl/mem: Find device capabilities")
>>> Suggested-by: Johnny <johnny.li@montage-tech.com>
>>> Signed-off-by: Penn <engguopeng@buaa.edu.cn>
>>
>> After addressing Lukas's comment,
>> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>>
>> Given that you are utilizing the new PCIe MMIO MBOX feature, have you considered migrate the CXL MBOX block parsing code to a PCI lib and shared between PCI and CXL? I did attempted something [1] like that a while back but never upstreamed the code due to no hardware to test on.
>>  
>> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/log/?h=pci-mbox
>>
>> DJ
> 
> Thanks for the review. Lukas's comment has already been addressed in v2,
> but I missed his Reviewed-by tag. When applying, please also include:
> 
> Reviewed-by: Lukas Wunner <lukas@wunner.de>
> 
> I will add it if another revision is needed.
> 
> In addition, I will look into moving the CXL mailbox block parsing code to a common
> PCI library so that it can be shared with the PCIe MMIO mailbox support.
> 
> Since that would be a broader refactoring, I would prefer to keep this
> patch focused on fixing the Vendor ID validation issue and handle the
> common parsing code in a separate follow-up patch or series. I will also
> test the follow-up changes on real CXL hardware with MMPT support.

Sounds reasonable. Do you have MMPT support submitted to upstream for the PCI subsystem yet? I'm curious because there are certainly security concerns of an opaque pipe from device that the kernel has no visibility to. There will certainly be security pushback on the enabling from the kernel community without some protocol in place to ensure the data being tunneled will not result in malicious actions.

DJ 

> 
>>> ---
>>> Changes in v2:
>>> - Accept a zero Vendor ID for compatibility with legacy CXL capability
>>>   headers.
>>> - Move the u16 declaration above the u32 declaration to follow the
>>>   reverse Christmas tree convention.
>>> - Document testing on CXL 1.1 and CXL 3.0 devices.
>>>
>>>  drivers/cxl/core/regs.c | 12 +++++++++++-
>>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
>>> index 93710cf4f0a6..c7c14089f2c6 100644
>>> --- a/drivers/cxl/core/regs.c
>>> +++ b/drivers/cxl/core/regs.c
>>> @@ -133,8 +133,18 @@ void cxl_probe_device_regs(struct device *dev, void __iomem *base,
>>>  
>>>  	for (cap = 1; cap <= cap_count; cap++) {
>>>  		struct cxl_reg_map *rmap;
>>> +		u16 cap_id, vendor_id;
>>>  		u32 offset, length;
>>> -		u16 cap_id;
>>> +
>>> +		vendor_id = FIELD_GET(PCI_MCAP_HDR_VENDOR_ID,
>>> +				      readl(base + PCI_MCAP_HDR_BASE(cap) +
>>> +					    PCI_MCAP_HDR_REG_4));
>>> +		/*
>>> +		 * The Vendor ID field is reserved and reads as zero in legacy
>>> +		 * CXL capability headers. See CXL r3.2, Table 8-44.
>>> +		 */
>>> +		if (vendor_id && vendor_id != PCI_VENDOR_ID_CXL)
>>> +			continue;
>>>  
>>>  		cap_id = FIELD_GET(CXLDEV_CAP_HDR_CAP_ID_MASK,
>>>  				   readl(base + cap * 0x10));


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

* Re: [PATCH v2 2/2] cxl/core: Skip non-CXL capabilities in MCAP
  2026-08-19 15:09       ` Dave Jiang
@ 2026-08-20  2:36         ` engguopeng
  0 siblings, 0 replies; 11+ messages in thread
From: engguopeng @ 2026-08-20  2:36 UTC (permalink / raw)
  To: Dave Jiang
  Cc: linux-cxl, linux-pci, linux-kernel, bhelgaas, dave, jic23,
	alison.schofield, vishal.l.verma, ira.weiny, djbw, johnny.li,
	peng.guo, jingzhong.yang



On Wed, Aug 19, 2026 at 11:09 PM Dave Jiang <dave.jiang@intel.com> wrote:
> Sounds reasonable. Do you have MMPT support submitted to upstream for the PCI subsystem yet? I'm curious because there are certainly security concerns of an opaque pipe from device that the kernel has no visibility to. There will certainly be security pushback on the enabling from the kernel community without some protocol in place to ensure the data being tunneled will not result in malicious actions.
> 
> DJ 

No, MMPT support has not been submitted upstream yet, and I have not
looked into the security implications in detail. Thank you for pointing
this out.

I will try to investigate MMPT support, including the security concerns
you mentioned, before considering an upstream submission.

Thanks

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

* Re: [PATCH v2 2/2] cxl/core: Skip non-CXL capabilities in MCAP
  2026-08-17  7:58 ` [PATCH v2 2/2] cxl/core: Skip non-CXL capabilities in MCAP penn
  2026-08-17  8:10   ` sashiko-bot
  2026-08-18 22:43   ` Dave Jiang
@ 2026-08-21 18:01   ` Jonathan Cameron
  2 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2026-08-21 18:01 UTC (permalink / raw)
  To: penn
  Cc: linux-cxl, linux-pci, linux-kernel, bhelgaas, dave, dave.jiang,
	alison.schofield, vishal.l.verma, ira.weiny, djbw, johnny.li,
	peng.guo, jingzhong.yang

On Mon, 17 Aug 2026 15:58:54 +0800
penn <engguopeng@buaa.edu.cn> wrote:

> From: Penn <engguopeng@buaa.edu.cn>
> 
> When MMPT is enabled, a CXL device may expose CXL-defined capabilities
> and PCIe Management Message Passthrough (MMPT) capabilities in the same
> MMIO Capabilities Register Block (MCAP).

> 
> The CXL capability parser currently identifies entries using only the
> capability ID. Since capability IDs are scoped by Vendor ID, a
> PCI-SIG-defined capability may have the same ID as a CXL-defined
> capability. This causes non-CXL entries, including MMPT and MMIO Mailbox,
> to be interpreted as CXL register blocks.

Hi Penn,

I'm a bit lost to what is going on here. So bare with me!
I think the upshot of the following is we need a little more info
in this patch description.

If the problem you describe was 'real' then we'd have a backwards
compatibility bug in the CXL spec and those folk are normally
pretty good at catching those!

To someone who doesn't know how MCAP and CXL were made compatible
(or like me forgot!) it isn't obvious that the non CXL capabilities
can turn up via the CXL capabilities structures - having reread
the spec sections I'm not sure they can.

A reference + some text from the implementation note in CXL v4.0 8.2.9
CXL Device Register Interface would be helpful.  The picture in
Figure 8-12 for instance implies you should never see non CXL caps
headers via the the CXL register block path.

You are allowed to alias the destinations of the (sort of) pointers
in the capabilities array, but not the header with the pointers
in it.


> 
> The MMPT register block may be interpreted as a CXL mailbox. This
> causes mailbox initialization to fail with:
> 
>     cxl_pci 0000:3b:00.0: Mailbox is too small (64b)
> 
> Check the MCAP Vendor ID before interpreting an entry as a CXL-defined
> capability. Skip capabilities with a non-zero Vendor ID other than
> PCI_VENDOR_ID_CXL.
> 
> In legacy CXL capability headers, the field now used for the MCAP
> Vendor ID was reserved and reads as zero. Continue to accept zero to
> preserve compatibility with those devices.
> 
> The fix has been tested on CXL 1.1 and CXL 3.0 devices. On the
> MMPT-enabled device, it prevents both Mailbox and Status capabilities
> from being misidentified.
> 
> Fixes: 8adaf747c9f0 ("cxl/mem: Find device capabilities")
> Suggested-by: Johnny <johnny.li@montage-tech.com>
> Signed-off-by: Penn <engguopeng@buaa.edu.cn>
> ---
> Changes in v2:
> - Accept a zero Vendor ID for compatibility with legacy CXL capability
>   headers.
> - Move the u16 declaration above the u32 declaration to follow the
>   reverse Christmas tree convention.
> - Document testing on CXL 1.1 and CXL 3.0 devices.
> 
>  drivers/cxl/core/regs.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
> index 93710cf4f0a6..c7c14089f2c6 100644
> --- a/drivers/cxl/core/regs.c
> +++ b/drivers/cxl/core/regs.c
> @@ -133,8 +133,18 @@ void cxl_probe_device_regs(struct device *dev, void __iomem *base,
>  
>  	for (cap = 1; cap <= cap_count; cap++) {
>  		struct cxl_reg_map *rmap;
> +		u16 cap_id, vendor_id;
>  		u32 offset, length;
> -		u16 cap_id;
> +
> +		vendor_id = FIELD_GET(PCI_MCAP_HDR_VENDOR_ID,
> +				      readl(base + PCI_MCAP_HDR_BASE(cap) +
> +					    PCI_MCAP_HDR_REG_4));
This is in the headers, so I think any device that hits thew condition is
not spec compliant.  I'm not against papering over such a device, but if that
is the case we should identify which device it is.

Now, if were unified code to handle MCAP fully - then I'd be fine with this
check without such additional info.

Need another coffee, so may well be missing something here.

Jonathan
 
> +		/*
> +		 * The Vendor ID field is reserved and reads as zero in legacy
> +		 * CXL capability headers. See CXL r3.2, Table 8-44.
> +		 */
> +		if (vendor_id && vendor_id != PCI_VENDOR_ID_CXL)
> +			continue;
>  
>  		cap_id = FIELD_GET(CXLDEV_CAP_HDR_CAP_ID_MASK,
>  				   readl(base + cap * 0x10));


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

end of thread, other threads:[~2026-08-21 18:01 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-17  7:58 [PATCH v2 0/2] PCI/CXL: Distinguish CXL capabilities in MCAP penn
2026-08-17  7:58 ` [PATCH v2 1/2] PCI: Add MMIO Capabilities Register Block definitions penn
2026-08-17  8:07   ` sashiko-bot
2026-08-18 22:37   ` Dave Jiang
2026-08-17  7:58 ` [PATCH v2 2/2] cxl/core: Skip non-CXL capabilities in MCAP penn
2026-08-17  8:10   ` sashiko-bot
2026-08-18 22:43   ` Dave Jiang
2026-08-19  1:28     ` engguopeng
2026-08-19 15:09       ` Dave Jiang
2026-08-20  2:36         ` engguopeng
2026-08-21 18:01   ` Jonathan Cameron

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