> -----Original Message-----
> From: Alex Williamson <[email protected]>
> Sent: Friday, August 28, 2026 4:33 AM
> 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]; Yishai Hadas
> <[email protected]>; Shameer Kolothum Thodi
> <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; Neo Jia
> <[email protected]>; Krishnakant Jaju <[email protected]>; Vikram Sethi
> <[email protected]>; Zhi Wang <[email protected]>; linux-
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected]
> Subject: Re: [PATCH v4 15/27] vfio/pci: Call CXL open and close hooks around
> device use
>
> External email: Use caution opening links or attachments
>
>
> On Thu, 13 Aug 2026 15:06:19 +0530
> <[email protected]> wrote:
>
> > From: Manish Honap <[email protected]>
> >
> > Volatile CXL register state must be sampled from live hardware when
> > the guest opens the device, not at bind, because a low-power
> > transition between bind and open can leave bind-time values stale.
>
> But patch 7 prevented such transitions, without really an explanation why.
> Accounting for being in full power state at open here further diminishes the
> requirement to restrict idle lower power states in that patch.
>
> > Add open_device and close_device to the CXL ops and call them from the
> > common enable and close paths. A failed open unwinds the enable like
> > any other error. The callbacks are stubs here and are filled in next.
> >
> > Signed-off-by: Manish Honap <[email protected]>
> > ---
> > drivers/vfio/pci/cxl/vfio_cxl_core.c | 11 +++++++
> > drivers/vfio/pci/vfio_pci_core.c | 44 ++++++++++++++++++++++------
> > include/linux/vfio_pci_core.h | 2 ++
> > 3 files changed, 48 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > b/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > index 966b7c22a6ac..d19fd638f538 100644
> > --- a/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > +++ b/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > @@ -100,9 +100,20 @@ static void vfio_cxl_release_device(struct
> vfio_pci_core_device *vdev)
> > vdev->cxl = NULL;
> > }
> >
> > +static int vfio_cxl_open_device(struct vfio_pci_core_device *vdev) {
> > + return 0;
> > +}
> > +
> > +static void vfio_cxl_close_device(struct vfio_pci_core_device *vdev)
> > +{ }
> > +
> > static const struct vfio_cxl_ops vfio_cxl_ops = {
> > .init_device = vfio_cxl_init_device,
> > .release_device = vfio_cxl_release_device,
> > + .open_device = vfio_cxl_open_device,
> > + .close_device = vfio_cxl_close_device,
> > .owner = THIS_MODULE,
> > };
> >
> > diff --git a/drivers/vfio/pci/vfio_pci_core.c
> > b/drivers/vfio/pci/vfio_pci_core.c
> > index 49dfbdaf3f05..470730cdc88b 100644
> > --- a/drivers/vfio/pci/vfio_pci_core.c
> > +++ b/drivers/vfio/pci/vfio_pci_core.c
> > @@ -584,6 +584,21 @@ static const struct dev_pm_ops
> vfio_pci_core_pm_ops = {
> > NULL)
> > };
> >
> > +static void vfio_pci_core_unmap_bars(struct vfio_pci_core_device
> > +*vdev) {
> > + struct pci_dev *pdev = vdev->pdev;
> > + int i, bar;
> > +
> > + for (i = 0; i < PCI_STD_NUM_BARS; i++) {
> > + bar = i + PCI_STD_RESOURCES;
> > + if (IS_ERR_OR_NULL(vdev->barmap[bar]))
> > + continue;
> > + pci_iounmap(pdev, vdev->barmap[bar]);
> > + pci_release_selected_regions(pdev, 1 << bar);
> > + vdev->barmap[bar] = NULL;
> > + }
> > +}
> > +
> > int vfio_pci_core_enable(struct vfio_pci_core_device *vdev) {
> > struct pci_dev *pdev = vdev->pdev; @@ -660,8 +675,23 @@ int
> > vfio_pci_core_enable(struct vfio_pci_core_device *vdev)
> >
> > vfio_pci_core_map_bars(vdev);
> >
> > + if (vdev->cxl_ops) {
> > + ret = vdev->cxl_ops->open_device(vdev);
> > + if (ret)
> > + goto out_free_config;
> > + }
> > +
> > return 0;
> >
> > +out_free_config:
> > + /*
> > + * open_device() runs after vfio_config_init() and map_bars() have
> > + * succeeded, but a failed first open never reaches
> vfio_pci_core_disable().
> > + * Unwind the common vconfig and BAR state here so the allocations and
> BAR
> > + * requests are not leaked for a later open to overwrite.
> > + */
> > + vfio_config_free(vdev);
> > + vfio_pci_core_unmap_bars(vdev);
>
> Unwind in the reverse order, this should start with unmap bars. The above
> comment is rather gratuitous and looks a bit like AI slop.
> Thanks,
>
> Alex
Okay, I will resolve this as suggested.
Manish
>
> > out_free_zdev:
> > vfio_pci_zdev_close_device(vdev);
> > out_free_state:
> > @@ -682,7 +712,7 @@ void vfio_pci_core_disable(struct
> vfio_pci_core_device *vdev)
> > struct pci_dev *pdev = vdev->pdev;
> > struct vfio_pci_dummy_resource *dummy_res, *tmp;
> > struct vfio_pci_ioeventfd *ioeventfd, *ioeventfd_tmp;
> > - int i, bar;
> > + int i;
> >
> > /* For needs_reset */
> > lockdep_assert_held(&vdev->vdev.dev_set->lock);
> > @@ -737,14 +767,7 @@ void vfio_pci_core_disable(struct
> > vfio_pci_core_device *vdev)
> >
> > vfio_config_free(vdev);
> >
> > - for (i = 0; i < PCI_STD_NUM_BARS; i++) {
> > - bar = i + PCI_STD_RESOURCES;
> > - if (IS_ERR_OR_NULL(vdev->barmap[bar]))
> > - continue;
> > - pci_iounmap(pdev, vdev->barmap[bar]);
> > - pci_release_selected_regions(pdev, 1 << bar);
> > - vdev->barmap[bar] = NULL;
> > - }
> > + vfio_pci_core_unmap_bars(vdev);
> >
> > list_for_each_entry_safe(dummy_res, tmp,
> > &vdev->dummy_resources_list, res_next)
> > { @@ -827,6 +850,9 @@ void vfio_pci_core_close_device(struct
> > vfio_device *core_vdev) #endif
> > vfio_pci_dma_buf_cleanup(vdev);
> >
> > + if (vdev->cxl_ops)
> > + vdev->cxl_ops->close_device(vdev);
> > +
> > vfio_pci_core_disable(vdev);
> >
> > mutex_lock(&vdev->igate);
> > diff --git a/include/linux/vfio_pci_core.h
> > b/include/linux/vfio_pci_core.h index 43755b91880f..7354dae1dd85
> > 100644
> > --- a/include/linux/vfio_pci_core.h
> > +++ b/include/linux/vfio_pci_core.h
> > @@ -70,6 +70,8 @@ struct vfio_pci_device_ops { struct vfio_cxl_ops {
> > int (*init_device)(struct vfio_pci_core_device *vdev);
> > void (*release_device)(struct vfio_pci_core_device *vdev);
> > + int (*open_device)(struct vfio_pci_core_device *vdev);
> > + void (*close_device)(struct vfio_pci_core_device *vdev);
> > /* Pinned per bound CXL device so vfio-cxl cannot unload under usage
> > */
> > struct module *owner;
> > };
