Re: [PATCH v2 2/2] cxl/core: Skip non-CXL capabilities in MCAP

[Jonathan Cameron]

On Mon, 17 Aug 2026 15:58:54 +0800
penn <engguopeng@xxxxxxxxxxx> wrote:

> From: Penn <engguopeng@xxxxxxxxxxx>
> 
> 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@xxxxxxxxxxxxxxxx>
> Signed-off-by: Penn <engguopeng@xxxxxxxxxxx>
> ---
> 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));