* [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; 9+ 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] 9+ 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-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, 1 reply; 9+ 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 [flat|nested] 9+ 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-18 22:37 ` Dave Jiang 0 siblings, 0 replies; 9+ 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] 9+ 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-18 22:43 ` Dave Jiang 2026-08-21 18:01 ` Jonathan Cameron 1 sibling, 2 replies; 9+ 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 [flat|nested] 9+ 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-18 22:43 ` Dave Jiang 2026-08-19 1:28 ` engguopeng 2026-08-21 18:01 ` Jonathan Cameron 1 sibling, 1 reply; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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-18 22:43 ` Dave Jiang @ 2026-08-21 18:01 ` Jonathan Cameron 1 sibling, 0 replies; 9+ 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] 9+ messages in thread
end of thread, other threads:[~2026-08-21 18:01 UTC | newest] Thread overview: 9+ 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-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-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
all inboxes | Powered by JetHome®