RE: [PATCH v4 06/27] vfio/pci: Add CXL ops registration interface

[Manish Honap]

> -----Original Message-----
> From: Alex Williamson <alex@xxxxxxxxxxx>
> Sent: Thursday, August 27, 2026 2:42 AM
> To: Manish Honap <mhonap@xxxxxxxxxx>
> Cc: jgg@xxxxxxxx; Ankit Agrawal <ankita@xxxxxxxxxx>; jic23@xxxxxxxxxx;
> dave.jiang@xxxxxxxxx; alejandro.lucero-palau@xxxxxxx; Srirangan Madhavan
> <smadhavan@xxxxxxxxxx>; corbet@xxxxxxx; skhan@xxxxxxxxxxxxxxxxxxx;
> dave@xxxxxxxxxxxx; alison.schofield@xxxxxxxxx; vishal.l.verma@xxxxxxxxx;
> iweiny@xxxxxxxxxx; ming.li@xxxxxxxxxxxx; Yishai Hadas
> <yishaih@xxxxxxxxxx>; Shameer Kolothum Thodi
> <skolothumtho@xxxxxxxxxx>; kevin.tian@xxxxxxxxx; bhelgaas@xxxxxxxxxx;
> dmatlack@xxxxxxxxxx; kees@xxxxxxxxxx; gustavoars@xxxxxxxxxx; Neo Jia
> <cjia@xxxxxxxxxx>; Krishnakant Jaju <kjaju@xxxxxxxxxx>; Vikram Sethi
> <vsethi@xxxxxxxxxx>; Zhi Wang <zhiw@xxxxxxxxxx>; linux-
> doc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx;
> linux-cxl@xxxxxxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; linux-
> kselftest@xxxxxxxxxxxxxxx; linux-hardening@xxxxxxxxxxxxxxx; alex@xxxxxxxxxxx
> Subject: Re: [PATCH v4 06/27] vfio/pci: Add CXL ops registration interface
> 
> External email: Use caution opening links or attachments
> 
> 
> On Thu, 13 Aug 2026 15:06:10 +0530
> <mhonap@xxxxxxxxxx> wrote:
> 
> > From: Manish Honap <mhonap@xxxxxxxxxx>
> >
> > vfio-pci-core must stay free of any CXL header dependency, so CXL
> > Type-2 handling lives in a separate vfio-cxl module that plugs in a
> > set of callbacks. Add the registration interface: vfio-cxl registers a
> > single struct vfio_cxl_ops at module_init, and vfio-pci-core stores it
> > under a mutex.
> >
> > The owner field lets a later patch pin vfio-cxl for the lifetime of
> > each bound CXL device. No caller yet; the detection path is added next.
> >
> > Signed-off-by: Manish Honap <mhonap@xxxxxxxxxx>
> > ---
> >  drivers/vfio/pci/vfio_pci_core.c | 27 +++++++++++++++++++++++++++
> >  include/linux/vfio_pci_core.h    | 10 ++++++++++
> >  2 files changed, 37 insertions(+)
> >
> > diff --git a/drivers/vfio/pci/vfio_pci_core.c
> > b/drivers/vfio/pci/vfio_pci_core.c
> > index 3f11a9624b9c..88e68d43af9a 100644
> > --- a/drivers/vfio/pci/vfio_pci_core.c
> > +++ b/drivers/vfio/pci/vfio_pci_core.c
> > @@ -2670,6 +2670,33 @@ static void vfio_pci_dev_set_try_reset(struct
> vfio_device_set *dev_set)
> >       }
> >  }
> >
> > +static const struct vfio_cxl_ops *vfio_pci_cxl_ops; static
> > +DEFINE_MUTEX(vfio_pci_cxl_ops_lock);
> > +
> > +int vfio_pci_core_register_cxl_ops(const struct vfio_cxl_ops *ops) {
> > +     int ret = 0;
> > +
> > +     mutex_lock(&vfio_pci_cxl_ops_lock);
> > +     if (vfio_pci_cxl_ops)
> > +             ret = -EBUSY;
> > +     else
> > +             vfio_pci_cxl_ops = ops;
> > +     mutex_unlock(&vfio_pci_cxl_ops_lock);
> > +
> > +     return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(vfio_pci_core_register_cxl_ops);
> > +
> > +void vfio_pci_core_unregister_cxl_ops(const struct vfio_cxl_ops *ops)
> > +{
> > +     mutex_lock(&vfio_pci_cxl_ops_lock);
> > +     if (vfio_pci_cxl_ops == ops)
> > +             vfio_pci_cxl_ops = NULL;
> > +     mutex_unlock(&vfio_pci_cxl_ops_lock);
> > +}
> > +EXPORT_SYMBOL_GPL(vfio_pci_core_unregister_cxl_ops);
> > +
> 
> Use guards to simplify these, especially the registration path where we can then
> just return -EBUSY.
> 
> I see the get function stores a per-vdev cxl_ops pointer with module reference
> held for the life of the device, so the serialization on the mutex is only per-
> device at probe time, but it might still be worthwhile to preempt that with a
> rwsem here where both these paths are writers and the get is a reader.
> 

Okay, I will convert register/unregister/get to guards and the mutex to an rwsem.
(double-register returns -EBUSY)

> >  static void vfio_pci_core_cleanup(void)  {
> >       vfio_pci_uninit_perm_bits();
> > diff --git a/include/linux/vfio_pci_core.h
> > b/include/linux/vfio_pci_core.h index 9a1674c152aa..14753972e714
> > 100644
> > --- a/include/linux/vfio_pci_core.h
> > +++ b/include/linux/vfio_pci_core.h
> > @@ -66,6 +66,16 @@ struct vfio_pci_device_ops {
> >                              size_t nr_ranges);  };
> >
> > +struct vfio_cxl_ops {
> > +     int     (*init_device)(struct vfio_pci_core_device *vdev);
> > +     void    (*release_device)(struct vfio_pci_core_device *vdev);
> > +     /* Pinned per bound CXL device so vfio-cxl cannot unload under usage */
> > +     struct module *owner;
> > +};
> 
> To mirror vfio_device_ops, should these drop _device and just become .init
> and .release?  I think that better reflects their actual usage while the eventual
> .open_device and .close_device already reflect the mapping into
> vfio_device_ops sequencing.  Thanks,

Okay, I will rename the ops init_device -> .init and release_device -> .release.

Manish

> 
> Alex
> 
> > +
> > +int vfio_pci_core_register_cxl_ops(const struct vfio_cxl_ops *ops);
> > +void vfio_pci_core_unregister_cxl_ops(const struct vfio_cxl_ops
> > +*ops);
> > +
> >  #if IS_ENABLED(CONFIG_VFIO_PCI_DMABUF)
> >  int vfio_pci_core_fill_phys_vec(struct phys_vec *phys_vec,
> >                               struct vfio_region_dma_range
> > *dma_ranges,