[PATCH] hw/cxl: fix the CDAT DOE overlapping the Flex Bus DVSEC when sn= is set

Junjie Cao posted 1 patch 3 weeks, 5 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260831022302.406740-1-junjie.cao@intel.com
Maintainers: Jonathan Cameron <jic23@kernel.org>
hw/mem/cxl_type3.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH] hw/cxl: fix the CDAT DOE overlapping the Flex Bus DVSEC when sn= is set
Posted by Junjie Cao 3 weeks, 5 days ago
ct3_realize() adds the CDAT DOE at a fixed 0x190.  Since 8700ee15de the
four DVSECs take 0x90 bytes, which from 0x100 ends exactly at 0x190.
With sn= the Device Serial Number capability pushes the block to
0x10c..0x19c, and the DOE, added later, overwrites the last 12 bytes of
the Flex Bus Port DVSEC: Capability2, Control2 and Status2.  Nothing
catches this -- pcie_add_capability() checks bounds, not overlap, and
the chain still walks because the DVSEC's next pointer becomes 0x190,
inside its own body.  Most cxl-type3 examples in
docs/system/devices/cxl.rst set sn=.

Derive the offset from the DVSEC block instead, as cxl_upstream.c
already does.  Without sn= the layout is unchanged byte for byte; with
sn= the DOE moves to 0x19c, below the AER capability at 0x200.  The
type 3 device has no VMStateDescription, so its config space never
reaches the migration stream.

Fixes: 8700ee15de ("hw/cxl: Standardize all references on CXL r3.1 and minor updates")
Signed-off-by: Junjie Cao <junjie.cao@intel.com>
---
Found while walking the extended capability chains for the UIO/SVC RFC
V2 review; independent of that series.

Tested at bde2492aac on q35 (pxb-cxl / cxl-rp / cxl-type3), dumping the
capability chain and raw config bytes from the guest with and without
sn=: the Flex Bus DVSEC keeps its full 0x20 bytes and the DOE sits at
0x19c; without sn= the bytes are identical before and after.  cxl-test
12/12.

On cxl-2026-03-25-draft REG_LOC_DVSEC_LENGTH is 0x34, so the overlap is
there without sn= too; with this change the DOE lands at 0x1a0/0x1ac.
cxl-2026-01-09-draft also fixes doe_comp at 0x1b0, which would then
collide -- the two want chaining.

 hw/mem/cxl_type3.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
index 28f41fa623..e6eb2e0cba 100644
--- a/hw/mem/cxl_type3.c
+++ b/hw/mem/cxl_type3.c
@@ -936,8 +936,8 @@ static void ct3_realize(PCIDevice *pci_dev, Error **errp)
     }
 
     /* DOE Initialization */
-    pcie_doe_init(pci_dev, &ct3d->doe_cdat, 0x190, doe_cdat_prot, true,
-                  CXL_T3_MSIX_PCIE_DOE_TABLE_ACCESS);
+    pcie_doe_init(pci_dev, &ct3d->doe_cdat, cxl_cstate->dvsec_offset,
+                  doe_cdat_prot, true, CXL_T3_MSIX_PCIE_DOE_TABLE_ACCESS);
 
     cxl_cstate->cdat.build_cdat_table = ct3_build_cdat_table;
     cxl_cstate->cdat.free_cdat_table = ct3_free_cdat_table;

base-commit: d2e570cc0f97b936902a5b1b86b73c0f5998b475
-- 
2.43.0
Re: [PATCH] hw/cxl: fix the CDAT DOE overlapping the Flex Bus DVSEC when sn= is set
Posted by Michael Tokarev 2 weeks ago
On 8/31/26 05:23, Junjie Cao wrote:
> ct3_realize() adds the CDAT DOE at a fixed 0x190.  Since 8700ee15de the
> four DVSECs take 0x90 bytes, which from 0x100 ends exactly at 0x190.
> With sn= the Device Serial Number capability pushes the block to
> 0x10c..0x19c, and the DOE, added later, overwrites the last 12 bytes of
> the Flex Bus Port DVSEC: Capability2, Control2 and Status2.  Nothing
> catches this -- pcie_add_capability() checks bounds, not overlap, and
> the chain still walks because the DVSEC's next pointer becomes 0x190,
> inside its own body.  Most cxl-type3 examples in
> docs/system/devices/cxl.rst set sn=.
> 
> Derive the offset from the DVSEC block instead, as cxl_upstream.c
> already does.  Without sn= the layout is unchanged byte for byte; with
> sn= the DOE moves to 0x19c, below the AER capability at 0x200.  The
> type 3 device has no VMStateDescription, so its config space never
> reaches the migration stream.
> 
> Fixes: 8700ee15de ("hw/cxl: Standardize all references on CXL r3.1 and minor updates")
> Signed-off-by: Junjie Cao <junjie.cao@intel.com>

This feels like a qemu-stable material.  I'm picking it up, but please
have qemu-stable@ in mind when the change is worth picking up for the
older stable releases.  And please let me know if I should not pick it up.

Thanks,

/mjt

> ---
> Found while walking the extended capability chains for the UIO/SVC RFC
> V2 review; independent of that series.
> 
> Tested at bde2492aac on q35 (pxb-cxl / cxl-rp / cxl-type3), dumping the
> capability chain and raw config bytes from the guest with and without
> sn=: the Flex Bus DVSEC keeps its full 0x20 bytes and the DOE sits at
> 0x19c; without sn= the bytes are identical before and after.  cxl-test
> 12/12.
> 
> On cxl-2026-03-25-draft REG_LOC_DVSEC_LENGTH is 0x34, so the overlap is
> there without sn= too; with this change the DOE lands at 0x1a0/0x1ac.
> cxl-2026-01-09-draft also fixes doe_comp at 0x1b0, which would then
> collide -- the two want chaining.
> 
>   hw/mem/cxl_type3.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index 28f41fa623..e6eb2e0cba 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -936,8 +936,8 @@ static void ct3_realize(PCIDevice *pci_dev, Error **errp)
>       }
>   
>       /* DOE Initialization */
> -    pcie_doe_init(pci_dev, &ct3d->doe_cdat, 0x190, doe_cdat_prot, true,
> -                  CXL_T3_MSIX_PCIE_DOE_TABLE_ACCESS);
> +    pcie_doe_init(pci_dev, &ct3d->doe_cdat, cxl_cstate->dvsec_offset,
> +                  doe_cdat_prot, true, CXL_T3_MSIX_PCIE_DOE_TABLE_ACCESS);
>   
>       cxl_cstate->cdat.build_cdat_table = ct3_build_cdat_table;
>       cxl_cstate->cdat.free_cdat_table = ct3_free_cdat_table;
> 
> base-commit: d2e570cc0f97b936902a5b1b86b73c0f5998b475