> -----Original Message-----
> From: Cédric Le Goater <[email protected]>
> Sent: Friday, September 18, 2026 9:39 PM
> To: Manish Honap <[email protected]>; [email protected]; Ankit Agrawal
> <[email protected]>; [email protected]; [email protected];
> [email protected]; Srirangan Madhavan
> <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Cc: Krishnakant Jaju <[email protected]>; Vikram Sethi <[email protected]>;
> Zhi Wang <[email protected]>; [email protected]; qemu-
> [email protected]; [email protected]
> Subject: Re: [PATCH v2 07/10] hw/vfio/pci: Map the CXL memory on the
> guest decoder commit
> 
> External email: Use caution opening links or attachments
> 
> 
> On 9/16/26 20:44, [email protected] wrote:
> > From: Manish Honap <[email protected]>
> >
> > Overlay the trapped HDM decoder block over the component BAR and
> > forward its accesses to the kernel. QEMU maps the HDM memory at the
> > base of the device's CFMWS window and presents that same base back to
> > the guest through the trapped decoder read
> >
> > Overlap the CFMWS window MemoryRegion the CXL host bridge maps at
> that
> > base, at a higher priority, so precedence is defined by priority
> > rather than subregion insertion order.
> >
> > QEMU always maps at the CFMWS base, so a decoder the guest commits at
> > any other base would leave the guest's view and the mapping diverged.
> > Track the base the guest writes to the trapped block and refuse to map
> > a commit at a different base; a firmware-committed decoder the guest
> > never reprograms keeps the window base.
> >
> > AI-used-for: code (prototype)
> > Signed-off-by: Manish Honap <[email protected]>
> > ---
> >   hw/cxl/cxl-host-stubs.c |   5 +
> >   hw/vfio/pci.c           | 386
> +++++++++++++++++++++++++++++++++++++++-
> >   hw/vfio/pci.h           |   8 +
> >   3 files changed, 393 insertions(+), 6 deletions(-)
> 
> 
> This patch introduces too many concepts at once, a reader familiar with VFIO
> can't separate the plumbing from the CXL details. This needs more commits.
> 
> So, something like
> 
> 1. framework
>     Set up the comp_regs_region
>     fd pass-through (pread/pwrite), no virtualization.
>     teardown
> 
> 2. Virtualization of decoder base registers
> 
> 3. Map/unmap of HDM memory
> 
>     if fmws_base never changes, there's no reason to remove and
>     re-add the subregion. Install it once and toggle with
>     memory_region_set_enabled(). this should simplify the decoder
>     handling.
> 
> 4. Config write hooks
>     PCI_COMMAND, CXL reset, PM transition, FLR x 2
>     each could be a separate patch I think
> 
> I don't think we need the machine notifier as said in the previous patch.
> 
> More below,
> 
> >
> > diff --git a/hw/cxl/cxl-host-stubs.c b/hw/cxl/cxl-host-stubs.c index
> > 9b515913ea..1067c944e2 100644
> > --- a/hw/cxl/cxl-host-stubs.c
> > +++ b/hw/cxl/cxl-host-stubs.c
> > @@ -23,3 +23,8 @@ GSList *cxl_fmws_get_all_sorted(void)
> >   {
> >       g_assert_not_reached();
> >   }
> > +
> > +int cxl_decoder_count_dec(int enc_cnt) {
> > +    g_assert_not_reached();
> > +}
> > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index
> > 670e0d1da4..fb3c39d4c6 100644
> > --- a/hw/vfio/pci.c
> > +++ b/hw/vfio/pci.c
> > @@ -1453,6 +1453,39 @@ uint32_t vfio_pci_read_config(PCIDevice *pdev,
> uint32_t addr, int len)
> >       return val;
> >   }
> >
> > +static void vfio_cxl_decoder_changed(VFIOPCIDevice *vdev); static
> > +void vfio_cxl_unmap_mem(VFIOPCIDevice *vdev);
> > +
> > +/*
> > + * Return true if this config write sets a Function Level Reset bit
> > +the kernel
> > + * acts on: the PCIe Device Control FLR bit or the Advanced Features FLR 
> > bit.
> > + * FLR bits are write-1-to-trigger and self-clearing, so inspect the
> > +written
> > + * value rather than the post-write config.
> > + */
> > +static bool vfio_cxl_flr_write(VFIOPCIDevice *vdev, uint32_t addr,
> > +                               uint32_t val, int len) {
> > +    PCIDevice *pdev = &vdev->parent_obj;
> > +    uint32_t off;
> > +
> > +    if (pdev->exp.exp_cap) {
> > +        /* PCI_EXP_DEVCTL_BCR_FLR (bit 15) is the high byte of Device 
> > Ctrl. */
> > +        off = pdev->exp.exp_cap + PCI_EXP_DEVCTL + 1;
> > +        if (off >= addr && off < addr + len &&
> > +            ((val >> (8 * (off - addr))) & (PCI_EXP_DEVCTL_BCR_FLR >> 8))) 
> > {
> > +            return true;
> > +        }
> > +    }
> > +    if (vdev->cxl.af_offset) {
> > +        off = vdev->cxl.af_offset + PCI_AF_CTRL;
> > +        if (off >= addr && off < addr + len &&
> > +            ((val >> (8 * (off - addr))) & PCI_AF_CTRL_FLR)) {
> > +            return true;
> > +        }
> > +    }
> > +    return false;
> > +}
> > +
> >   void vfio_pci_write_config(PCIDevice *pdev,
> >                              uint32_t addr, uint32_t val, int len)
> >   {
> > @@ -1522,9 +1555,74 @@ void vfio_pci_write_config(PCIDevice *pdev,
> >                   vfio_sub_page_bar_update_mapping(pdev, bar);
> >               }
> >           }
> > +
> > +        /*
> > +         * A firmware-committed CXL decoder is already committed at boot, 
> > so
> no
> > +         * guest control write triggers the HDM mapping. Map it once the 
> > guest
> > +         * enables memory decoding, so the region enters the guest address
> space
> > +         * and the IOAS while the device is live. On a clear of memory 
> > decoding,
> > +         * withdraw the overlay: the kernel revokes the HDM PTEs on the 
> > same
> > +         * write, so a retained memslot would fault a guest access during 
> > the
> > +         * disabled interval onto a zapped VMA and stop the VM; the enable
> path
> > +         * re-maps it.
> > +         */
> > +        if (vdev->cxl.enabled &&
> > +            range_covers_byte(addr, len, PCI_COMMAND)) {
> > +            if (pci_get_word(pdev->config + PCI_COMMAND) &
> PCI_COMMAND_MEMORY) {
> > +                vfio_cxl_decoder_changed(vdev);
> > +            } else {
> > +                vfio_cxl_unmap_mem(vdev);
> > +            }
> > +        }
> >       } else {
> >           /* Write everything to QEMU to keep emulated bits correct */
> >           pci_default_write_config(pdev, addr, val, len);
> > +
> > +        /*
> > +         * A guest CXL reset is a write to the CXL Device DVSEC ctrl2 
> > register,
> > +         * forwarded to the kernel above, which re-commits this 
> > firmware-fixed
> > +         * decoder. If the guest decommitted the decoder before the reset,
> which
> > +         * unmapped the HDM memory, that re-commit is not otherwise visible
> to
> > +         * QEMU, so rescan the decoder here to restore the mapping. Gated 
> > on
> > +         * memory decoding still being enabled, like the enable path above.
> > +         */
> > +        if (vdev->cxl.enabled && vdev->cxl.dvsec_offset &&
> > +            ranges_overlap(addr, len,
> > +                           vdev->cxl.dvsec_offset +
> > +                               offsetof(CXLDVSECDevice, ctrl2),
> > +                           sizeof_field(CXLDVSECDevice, ctrl2)) &&
> > +            (pci_get_word(pdev->config + PCI_COMMAND) &
> PCI_COMMAND_MEMORY)) {
> > +            vfio_cxl_decoder_changed(vdev);
> > +        }
> > +
> > +        /*
> > +         * A NoSoftRst- device the guest cycles D3hot->D0 has its physical
> > +         * decoder restored by the kernel on resume, which re-commits this
> > +         * firmware-fixed decoder just like a reset. That re-commit is not
> > +         * otherwise visible to QEMU, so on the transition back to D0 
> > rescan
> > +         * the decoder to restore a mapping the guest dropped before it
> > +         * suspended. Gated on memory decoding, like the paths above.
> > +         */
> > +        if (vdev->cxl.enabled && pdev->pm_cap &&
> > +            range_covers_byte(addr, len, pdev->pm_cap + PCI_PM_CTRL) &&
> > +            (pci_get_word(pdev->config + pdev->pm_cap + PCI_PM_CTRL) &
> > +             PCI_PM_CTRL_STATE_MASK) == 0 &&
> > +            (pci_get_word(pdev->config + PCI_COMMAND) &
> PCI_COMMAND_MEMORY)) {
> > +            vfio_cxl_decoder_changed(vdev);
> > +        }
> > +
> > +        /*
> > +         * A Function Level Reset the guest triggers through the PCIe 
> > Device
> > +         * Control or Advanced Features FLR bit is forwarded to the kernel,
> > +         * which re-commits this firmware-fixed decoder just like a CXL 
> > reset.
> > +         * That re-commit is not otherwise visible to QEMU, so rescan the
> > +         * decoder to restore a mapping the guest dropped before the FLR.
> Gated
> > +         * on memory decoding, like the paths above.
> > +         */
> > +        if (vdev->cxl.enabled && vfio_cxl_flr_write(vdev, addr, val, len) 
> > &&
> > +            (pci_get_word(pdev->config + PCI_COMMAND) &
> PCI_COMMAND_MEMORY)) {
> > +            vfio_cxl_decoder_changed(vdev);
> > +        }
> >       }
> >   }
> 
> 
> That's a lot of CXL-specific code injected into vfio_pci_write_config.
> Don't do that. Please add :
> 
>      if (vdev->cxl.enabled) {
>          vfio_cxl_config_written(vdev, addr, val, len);
>      }
> 
> I think that we should have a vfio/cxl.c subcomponent to isolate the CXL-
> specific code :
> 
>    vfio_cxl_setup, or better vfio_cxl_realize
>    vfio_cxl_config_written,
>    vfio_cxl_exit,
>    vfio_cxl_finalize,
>    etc.
> 
> 
> 
> > @@ -3811,6 +3909,239 @@ static void vfio_cxl_bind_fmws(Notifier *n,
> void *data)
> >       }
> >   }
> >
> > +/*
> > + * HDM decoder registers, relative to the trapped decoder block. The
> > +block holds
> > + * the HDM Decoder Capability register followed by one register set
> > +per decoder,
> > + * each VFIO_CXL_HDM_DECODER_STRIDE apart, indexed by decoder.
> > + */
> > +#define VFIO_CXL_HDM_CAP                  0x00
> > +#define VFIO_CXL_HDM_DECODER_STRIDE       0x20
> > +#define VFIO_CXL_HDM_DECODER_BASE_LOW(n) \
> > +    (0x10 + (n) * VFIO_CXL_HDM_DECODER_STRIDE) #define
> > +VFIO_CXL_HDM_DECODER_BASE_HIGH(n) \
> > +    (0x14 + (n) * VFIO_CXL_HDM_DECODER_STRIDE) #define
> > +VFIO_CXL_HDM_DECODER_CTRL(n) \
> > +    (0x20 + (n) * VFIO_CXL_HDM_DECODER_STRIDE)
> > +#define VFIO_CXL_HDM_CTRL_COMMITTED       (1 << 10)
> > +#define VFIO_CXL_HDM_BASE_LOW_MASK        0xf0000000U
> > +
> > +static void vfio_cxl_unmap_mem(VFIOPCIDevice *vdev) {
> > +    VFIOCXL *cxl = &vdev->cxl;
> > +
> > +    if (!cxl->dpa_mapped) {
> > +        return;
> > +    }
> > +    memory_region_del_subregion(get_system_memory(), cxl-
> >mem_region.mem);
> > +    cxl->dpa_mapped = false;
> > +    cxl->mapped_base = 0;
> > +}
> > +
> > +/*
> > + * The HDM Decoder Capability register encodes the decoder count in
> > +its low
> > + * nibble (CXL r3.1 8.2.4.20.1, encodings 0h..Ch for 1..32). Decode
> > +it with the
> > + * shared cxl_decoder_count_dec() helper so the commit handler walks
> > +every
> > + * decoder rather than assuming decoder 0. An unreadable register or
> > +a reserved
> > + * encoding (which the helper decodes to 0) is treated as a single decoder.
> > + */
> > +static unsigned vfio_cxl_decoder_count(VFIOPCIDevice *vdev) {
> > +    off_t off = vdev->cxl.comp_regs_region.fd_offset;
> > +    uint32_t cap = 0;
> > +    int count;
> > +
> > +    if (pread(vdev->vbasedev.fd, &cap, 4, off + VFIO_CXL_HDM_CAP) != 4) {
> > +        return 1;
> > +    }
> > +    count = cxl_decoder_count_dec(le32_to_cpu(cap) & 0xf);
> > +    return count > 0 ? count : 1;
> > +}
> > +
> > +/*
> > + * The guest committed or tore down an endpoint decoder. Walk each
> > +decoder in
> > + * the trapped HDM block and (un)map the HDM memory at the CFMWS
> > +window base,
> > + * not the base the guest programmed (see vfio_cxl_comp_regs_read).
> > +This
> > + * generation commits one non-interleaved decoder, so the walk stops
> > +at the
> > + * first committed decoder.
> 
> if only one decoder is supported and the code tracks the guest base per-device
> rather than per-decoder, why do the read/write handlers use
> VFIO_CXL_HDM_DECODER_BASE_LOW(0) to match all decoders ? This is
> inconsistent.
> 
> I think we should record the active decoder index 'active_decoder'
> instead. If a second committed decoder is found,  decoder_changed should
> bail out.
> 
> 
> > + */
> > +static void vfio_cxl_decoder_changed(VFIOPCIDevice *vdev) {
> > +    VFIOCXL *cxl = &vdev->cxl;
> > +    VFIODevice *vbasedev = &vdev->vbasedev;
> > +    off_t off = cxl->comp_regs_region.fd_offset;
> > +    unsigned n, count = vfio_cxl_decoder_count(vdev);
> > +
> > +    for (n = 0; n < count; n++) {
> > +        uint32_t ctrl = 0;
> > +
> > +        if (pread(vbasedev->fd, &ctrl, 4,
> > +                  off + VFIO_CXL_HDM_DECODER_CTRL(n)) != 4) {
> > +            return;
> > +        }
> > +        if (!(le32_to_cpu(ctrl) & VFIO_CXL_HDM_CTRL_COMMITTED)) {
> > +            continue;
> > +        }
> > +
> > +        /*
> > +         * QEMU always maps at the device's CFMWS base and presents that
> base
> > +         * back to the guest, so a decoder the guest committed at any other
> base
> > +         * would leave the guest's view and the actual mapping diverged. 
> > Reject
> > +         * it rather than silently relocate. A firmware-committed decoder 
> > the
> > +         * guest never reprogrammed (guest_base_written == false) keeps the
> > +         * CFMWS base.
> > +         */
> > +        if (cxl->guest_base_written) {
> > +            hwaddr guest_base = ((hwaddr)cxl->guest_base_hi << 32) |
> > +                                cxl->guest_base_lo;
> > +
> > +            if (guest_base != cxl->fmws_base) {
> > +                warn_report("vfio-cxl: %s: guest committed decoder %u at 
> > 0x%"
> > +                            HWADDR_PRIx ", not the CFMWS base 0x%" 
> > HWADDR_PRIx
> > +                            "; not mapping", vbasedev->name, n, guest_base,
> > +                            cxl->fmws_base);
> > +                vfio_cxl_unmap_mem(vdev);
> > +                return;
> > +            }
> > +        }
> > +
> > +        if (cxl->dpa_mapped && cxl->mapped_base == cxl->fmws_base) {
> > +            return;
> > +        }
> > +        /*
> > +         * Overlap the CFMWS window MemoryRegion the CXL host bridge
> already
> > +         * maps at this base, at a higher priority, so precedence is 
> > defined by
> > +         * priority rather than subregion insertion order.
> > +         */
> > +        memory_region_transaction_begin();
> > +        vfio_cxl_unmap_mem(vdev);
> > +        memory_region_add_subregion_overlap(get_system_memory(),
> > +                                            cxl->fmws_base,
> > +                                            cxl->mem_region.mem, 1);
> > +        memory_region_transaction_commit();
> > +        cxl->mapped_base = cxl->fmws_base;
> > +        cxl->dpa_mapped = true;
> > +        return;
> > +    }
> > +
> > +    /* No committed decoder in the block: tear down any existing mapping.
> */
> > +    vfio_cxl_unmap_mem(vdev);
> > +}
> > +
> > +static uint64_t vfio_cxl_comp_regs_read(void *opaque, hwaddr addr,
> > +                                        unsigned size) {
> > +    VFIORegion *region = opaque;
> > +    VFIODevice *vbasedev = region->vbasedev;
> > +    VFIOPCIDevice *vdev =
> > +        container_of(region, VFIOPCIDevice, cxl.comp_regs_region);
> > +    VFIOCXL *cxl = &vdev->cxl;
> > +    uint32_t val = 0xffffffff;
> > +
> > +    if (pread(vbasedev->fd, &val, size, region->fd_offset + addr) != size) 
> > {
> > +        error_report("vfio-cxl: %s: HDM decoder read at 0x%" HWADDR_PRIx
> > +                     " failed", vbasedev->name, addr);
> > +    }
> > +    val = le32_to_cpu(val);
> > +
> > +    /*
> > +     * The kernel shadow holds the host physical base for a firmware-
> committed
> > +     * decoder. Never expose that to the guest: present the guest physical
> base
> > +     * QEMU maps the HDM memory at (the device's CFMWS window). Any
> decoder's
> > +     * base registers are virtualized; ctrl, size and the capability 
> > header pass
> > +     * through. The base registers repeat every
> VFIO_CXL_HDM_DECODER_STRIDE.
> > +     */
> > +    if (addr >= VFIO_CXL_HDM_DECODER_BASE_LOW(0) &&
> > +        (addr - VFIO_CXL_HDM_DECODER_BASE_LOW(0)) %
> > +                VFIO_CXL_HDM_DECODER_STRIDE == 0) {
> > +        val = (val & ~VFIO_CXL_HDM_BASE_LOW_MASK) |
> > +              ((uint32_t)cxl->fmws_base & VFIO_CXL_HDM_BASE_LOW_MASK);
> > +    } else if (addr >= VFIO_CXL_HDM_DECODER_BASE_HIGH(0) &&
> > +               (addr - VFIO_CXL_HDM_DECODER_BASE_HIGH(0)) %
> > +                       VFIO_CXL_HDM_DECODER_STRIDE == 0) {
> > +        val = (uint32_t)(cxl->fmws_base >> 32);
> > +    }
> > +
> > +    return val;
> > +}
> > +
> > +static void vfio_cxl_comp_regs_write(void *opaque, hwaddr addr, uint64_t
> data,
> > +                                     unsigned size) {
> > +    VFIORegion *region = opaque;
> > +    VFIODevice *vbasedev = region->vbasedev;
> > +    VFIOPCIDevice *vdev =
> > +        container_of(region, VFIOPCIDevice, cxl.comp_regs_region);
> > +    uint32_t val = cpu_to_le32((uint32_t)data);
> > +
> > +    if (pwrite(vbasedev->fd, &val, size, region->fd_offset + addr) != 
> > size) {
> > +        error_report("vfio-cxl: %s: HDM decoder write at 0x%" HWADDR_PRIx
> > +                     " failed", vbasedev->name, addr);
> > +        return;
> > +    }
> > +
> > +    /*
> > +     * Record the base the guest programs into a decoder so the commit
> handler
> > +     * can reject a base other than the device's CFMWS window. The base low
> > +     * register carries HPA bits [31:28]; the high register carries 
> > [63:32].
> > +     */
> > +    if (addr >= VFIO_CXL_HDM_DECODER_BASE_LOW(0) &&
> > +        (addr - VFIO_CXL_HDM_DECODER_BASE_LOW(0)) %
> > +                VFIO_CXL_HDM_DECODER_STRIDE == 0) {
> > +        vdev->cxl.guest_base_lo = (uint32_t)data &
> VFIO_CXL_HDM_BASE_LOW_MASK;
> > +        vdev->cxl.guest_base_written = true;
> > +    } else if (addr >= VFIO_CXL_HDM_DECODER_BASE_HIGH(0) &&
> > +               (addr - VFIO_CXL_HDM_DECODER_BASE_HIGH(0)) %
> > +                       VFIO_CXL_HDM_DECODER_STRIDE == 0) {
> > +        vdev->cxl.guest_base_hi = (uint32_t)data;
> > +        vdev->cxl.guest_base_written = true;
> > +    }
> > +
> > +    /*
> > +     * The kernel runs the lock-on-commit FSM in the write above, so the
> > +     * committed state is settled by now; a control write on any decoder 
> > can
> > +     * change the mapping.
> > +     */
> > +    if (addr >= VFIO_CXL_HDM_DECODER_CTRL(0) &&
> > +        (addr - VFIO_CXL_HDM_DECODER_CTRL(0)) %
> > +        VFIO_CXL_HDM_DECODER_STRIDE == 0) {
> > +        vfio_cxl_decoder_changed(vdev);
> > +    }
> > +}
> > +
> > +static const MemoryRegionOps vfio_cxl_comp_regs_ops = {
> > +    .read = vfio_cxl_comp_regs_read,
> > +    .write = vfio_cxl_comp_regs_write,
> > +    .endianness = DEVICE_LITTLE_ENDIAN,
> > +    .valid = { .min_access_size = 4, .max_access_size = 4 },
> > +    .impl = { .min_access_size = 4, .max_access_size = 4 }, };
> > +
> > +/*
> > + * Locate the CXL Device DVSEC (CXL r3.1 8.1.3) in config space. The
> > +guest
> > + * triggers a CXL reset by writing its ctrl2 register; QEMU rescans
> > +the decoder
> > + * after that write so the HDM mapping is restored (see
> vfio_pci_write_config).
> > + * Returns the DVSEC config offset, or 0 if the device does not expose it.
> > + */
> > +static uint16_t vfio_cxl_find_device_dvsec(PCIDevice *pdev) {
> > +    uint16_t offset;
> > +
> > +    for (offset = PCI_CONFIG_SPACE_SIZE; offset;
> > +         offset = PCI_EXT_CAP_NEXT(pci_get_long(pdev->config + offset))) {
> > +        uint32_t hdr = pci_get_long(pdev->config + offset);
> > +
> > +        if (PCI_EXT_CAP_ID(hdr) == PCI_EXT_CAP_ID_DVSEC &&
> > +            (pci_get_long(pdev->config + offset + PCI_DVSEC_HEADER1) & 
> > 0xffff)
> > +                == CXL_VENDOR_ID &&
> > +            pci_get_word(pdev->config + offset + PCI_DVSEC_HEADER2)
> > +                == PCIE_CXL_DEVICE_DVSEC) {
> > +            return offset;
> > +        }
> > +    }
> > +
> > +    return 0;
> > +}
> 
> This should be a PCIe helper :
> 
>    uint16_t pcie_find_dvsec(PCIDevice *dev, uint16_t vendor_id, uint16_t
> dvsec_id);
> 

Okay, I will refactor this patch along the lines you suggested:

- framework (comp_regs_region setup, fd pass-through, teardown),
- base register virtualization,
- map and unmap of HDM memory,
- config write hooks for PCI_COMMAND, CXL reset, PM transition, and FLR.

map and unmap: I will install the subregion once and toggle it with
memory_region_set_enabled() instead of add and delete on every change.

Decoder handling: I will record the active decoder index and virtualize and
map for that decoder, and have decoder_changed bail out if it finds a second
committed decoder, since this patch series supports only one.

Rename vfio_cxl_find_device_dvsec() as pcie_find_dvsec(dev, vendor_id, dvsec_id)
in the PCIe code, so it is reusable.

Remove the machine_done notifier as suggested.

> Thanks,
> 
> C.
> 
> 
> >   /*
> >    * Learn the CXL geometry the kernel reports: the HPA-backed HDM memory
> region
> >    * and the trapped HDM decoder block (which BAR carries it and at what
> offset).
> > @@ -3864,6 +4195,8 @@ static bool vfio_cxl_setup(VFIOPCIDevice *vdev,
> Error **errp)
> >       cxl->dpa_size = mem_info->size;
> >       cxl->comp_bar = cap->bar;
> >       cxl->hdm_offset = cap->offset;
> > +    cxl->dvsec_offset = vfio_cxl_find_device_dvsec(&vdev->parent_obj);
> > +    cxl->af_offset = pci_find_capability(&vdev->parent_obj,
> > + PCI_CAP_ID_AF);
> >
> >       if (!vfio_cxl_check_topology(vdev, errp)) {
> >           return false;
> > @@ -3887,6 +4220,26 @@ static bool vfio_cxl_setup(VFIOPCIDevice *vdev,
> Error **errp)
> >                       "performance may be slow", vbasedev->name);
> >       }
> >
> > +    /*
> > +     * Trap the HDM decoder block: overlay it, priority 1, over the 
> > directly
> > +     * mapped component BAR, so guest decoder accesses reach the kernel
> FSM and
> > +     * the commit becomes visible to QEMU.
> > +     */
> > +    if (!vdev->bars[cxl->comp_bar].mr) {
> > +        error_setg(errp, "vfio-cxl: %s: component BAR %u is not present",
> > +                   vbasedev->name, cxl->comp_bar);
> > +        goto err;
> > +    }
> > +    if (vfio_region_setup_with_ops(OBJECT(vdev), vbasedev,
> > +                                   &cxl->comp_regs_region,
> > +                                   cxl->comp_regs_region_index, 
> > "cxl-comp-regs",
> > +                                   &vfio_cxl_comp_regs_ops, errp)) {
> > +        goto err;
> > +    }
> > +    memory_region_add_subregion_overlap(vdev->bars[cxl->comp_bar].mr,
> > +                                        cxl->hdm_offset,
> > +                                        cxl->comp_regs_region.mem,
> > + 1);
> > +
> >       if (DEVICE(vdev)->hotplugged) {
> >           /*
> >            * The machine is already up, so the CFMWS windows are
> > placed and the @@ -3894,9 +4247,7 @@ static bool
> vfio_cxl_setup(VFIOPCIDevice *vdev, Error **errp)
> >            * device_add fails cleanly instead of aborting the running VM.
> >            */
> >           if (!vfio_cxl_do_bind_fmws(vdev, errp)) {
> > -            vfio_region_exit(&cxl->mem_region);
> > -            vfio_region_finalize(&cxl->mem_region);
> > -            return false;
> > +            goto err;
> >           }
> >       } else {
> >           cxl->machine_done.notify = vfio_cxl_bind_fmws; @@ -3906,6
> > +4257,10 @@ static bool vfio_cxl_setup(VFIOPCIDevice *vdev, Error **errp)
> >       cxl->enabled = true;
> >
> >       return true;
> > +
> > +err:
> > +    vfio_cxl_teardown(vdev);
> > +    return false;
> >   }
> >
> >   static void vfio_cxl_teardown(VFIOPCIDevice *vdev) @@ -3916,15
> > +4271,26 @@ static void vfio_cxl_teardown(VFIOPCIDevice *vdev)
> >           return;
> >       }
> >
> > -    if (cxl->machine_done.notify) {
> > -        qemu_remove_machine_init_done_notifier(&cxl->machine_done);
> > -        cxl->machine_done.notify = NULL;
> > +    if (cxl->comp_regs_region.mem) {
> > +        if (vdev->bars[cxl->comp_bar].mr) {
> > +            memory_region_del_subregion(vdev->bars[cxl->comp_bar].mr,
> > +                                        cxl->comp_regs_region.mem);
> > +        }
> > +        vfio_region_exit(&cxl->comp_regs_region);
> > +        vfio_region_finalize(&cxl->comp_regs_region);
> >       }
> >
> > +    vfio_cxl_unmap_mem(vdev);
> > +
> >       if (cxl->mem_region.mem) {
> >           vfio_region_exit(&cxl->mem_region);
> >           vfio_region_finalize(&cxl->mem_region);
> >       }
> > +
> > +    if (cxl->machine_done.notify) {
> > +        qemu_remove_machine_init_done_notifier(&cxl->machine_done);
> > +        cxl->machine_done.notify = NULL;
> > +    }
> >   }
> >
> >   static void vfio_pci_realize(PCIDevice *pdev, Error **errp) @@
> > -4127,6 +4493,14 @@ static void vfio_exitfn(PCIDevice *pdev)
> >       vfio_pci_teardown_msi(vdev);
> >       vfio_pci_disable_rp_atomics(vdev);
> >       vfio_pci_bars_exit(vdev);
> > +    /*
> > +     * The committed HDM overlay is a subregion of system memory owned
> by this
> > +     * device, so it holds a reference that would keep the object alive 
> > past
> > +     * unrealize and block instance_finalize (where vfio_cxl_teardown
> otherwise
> > +     * runs). Drop it here; the call is idempotent for a device that never
> > +     * mapped or is not CXL.
> > +     */
> > +    vfio_cxl_unmap_mem(vdev);
> >       vfio_migration_exit(vbasedev);
> >       if (!vbasedev->mdev) {
> >           pci_device_unset_iommu_device(pdev);
> > diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h index
> > 22fe8d7ff7..5bfa976112 100644
> > --- a/hw/vfio/pci.h
> > +++ b/hw/vfio/pci.h
> > @@ -133,11 +133,19 @@ typedef struct VFIOCXL {
> >       uint32_t comp_regs_region_index; /* trapped HDM decoder register
> block */
> >       uint32_t comp_bar;               /* component BAR carrying that block 
> > */
> >       uint64_t hdm_offset;             /* block offset within the component 
> > BAR */
> > +    uint16_t dvsec_offset;           /* CXL Device DVSEC offset, 0 if none 
> > */
> > +    uint16_t af_offset;              /* PCI Advanced Features cap, 0 if 
> > none */
> >       uint64_t dpa_size;               /* size of the HDM memory region */
> >       VFIORegion mem_region;           /* HDM memory, mapped at committed
> GPA */
> >       Notifier machine_done;           /* CFMWS validated at machine_done */
> >       hwaddr fmws_base;                /* base of the memory window */
> >       uint64_t fmws_size;              /* size of the memory window */
> > +    VFIORegion comp_regs_region;     /* trapped HDM decoder block */
> > +    hwaddr mapped_base;              /* GPA the HDM memory is mapped at */
> > +    bool dpa_mapped;                 /* HDM memory currently in system 
> > memory
> */
> > +    uint32_t guest_base_lo;          /* decoder base low the guest wrote */
> > +    uint32_t guest_base_hi;          /* decoder base high the guest wrote 
> > */
> > +    bool guest_base_written;         /* the guest wrote a decoder base */
> >   } VFIOCXL;
> >
> >   struct VFIOPCIDevice {

Reply via email to