RE: [PATCH 06/10] hw/vfio/pci: Bind a CXL device to its fixed memory window

> -----Original Message-----
> From: Junjie Cao <[email protected]>
> Sent: 18 August 2026 15:22
> To: Manish Honap <[email protected]>
> Cc: [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];
> [email protected]; Krishnakant Jaju <[email protected]>; Vikram Sethi
> <[email protected]>; Zhi Wang <[email protected]>; [email protected];
> [email protected]
> Subject: Re: [PATCH 06/10] hw/vfio/pci: Bind a CXL device to its fixed
> memory window
> 
> External email: Use caution opening links or attachments
> 
> 
> Hi Manish,
> 
> On Thu, 13 Aug 2026 18:36:19 +0530, Manish Honap wrote:
> > +/*
> > + * Cold-plug path: the CFMWS windows are placed at machine init done,
> > +so the
> > + * binding can only be validated from this notifier. The VM has not
> > +run yet, so
> > + * a configuration error is fatal to startup. A hotplugged device is
> > +validated
> > + * in realize instead (see vfio_cxl_setup), where the failure fails
> > +device_add
> > + * without taking down the running VM.
> 
> At this point in the series that isn't true yet: the DEVICE(vdev)-
> >hotplugged branch only arrives in patch 7, while this patch registers
> the notifier unconditionally.
> qemu_add_machine_init_done_notifier() invokes the notifier immediately
> once the machine has reached PHASE_MACHINE_READY (hw/core/machine.c), so
> between patches 6 and 7 a device_add with a bad CFMWS config lands in
> vfio_cxl_bind_fmws() and exit(1)s the running VM. Move the hotplug
> branch from patch 7 into this patch; then every point of the series
> matches the comment.

okay, I will move the hot-plugged branch into this patch so every point
of the series matches the comment.

> 
> > +        for (slot = 0; slot < PCI_SLOT_MAX; slot++) {
> > +            for (fn = 0; fn < PCI_FUNC_MAX; fn++) {
> > +                if (ep_bus->devices[PCI_DEVFN(slot, fn)]) {
> > +                    nendpoints++;
> > +                    break;
> 
> This counts occupied slots, not functions: the inner loop breaks on the
> first function of a slot, so two vfio-cxl functions cold-plugged at slot
> 0 each pass with nendpoints == 1, bind the same single-target window,
> and map at the same base with equal priority -- the aliasing this check
> exists to reject. Count devfns instead.
> 

I agree this is a real bug. I will drop the break and count devfns as suggested.

> Many thanks,
> Junjie