RE: [PATCH v4 05/27] cxl: Add a function-scoped reset entry for vfio-pci

[Manish Honap]

> -----Original Message-----
> From: Alex Williamson <alex@xxxxxxxxxxx>
> Sent: Wednesday, August 26, 2026 4: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 05/27] cxl: Add a function-scoped reset entry for vfio-
> pci
> 
> External email: Use caution opening links or attachments
> 
> 
> On Thu, 13 Aug 2026 15:06:09 +0530
> <mhonap@xxxxxxxxxx> wrote:
> 
> > From: Manish Honap <mhonap@xxxxxxxxxx>
> >
> > vfio-pci needs to run a CXL reset on a passthroughed Type-2 device
> > without the host-memory handling in cxl_reset_function(): the memory
> > behind the decoder belongs to the guest.
> >
> > Add cxl_reset_dvsec_sequence(), which runs the DVSEC reset and HDM
> > decoder restore under pci_dev_lock and cxl_rwsem, and
> > cxl_reset_capable() to gate it on a function-scoped reset.
> >
> > Parameterize Memory Clear in cxl_reset_execute() so the guest owns the
> > choice; the host path keeps it disabled.
> >
> > Signed-off-by: Manish Honap <mhonap@xxxxxxxxxx>
> > ---
> >  drivers/cxl/core/resource.c | 69 ++++++++++++++++++++++++++++++++++-
> --
> >  include/cxl/cxl.h           | 12 +++++++
> >  2 files changed, 76 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/cxl/core/resource.c b/drivers/cxl/core/resource.c
> > index 492ffe8e3576..cd15bd9171e6 100644
> > --- a/drivers/cxl/core/resource.c
> > +++ b/drivers/cxl/core/resource.c
> > @@ -1164,7 +1164,7 @@ static int cxl_reset_enable_cache(struct pci_dev
> *pdev, int dvsec)
> >                                    PCI_DVSEC_CXL_DISABLE_CACHING);  }
> >
> > -static int cxl_reset_initiate(struct pci_dev *pdev, int dvsec)
> > +static int cxl_reset_initiate(struct pci_dev *pdev, int dvsec, bool
> > +mem_clr_en)
> >  {
> >       u16 ctrl2;
> >       int rc;
> > @@ -1173,7 +1173,10 @@ static int cxl_reset_initiate(struct pci_dev *pdev,
> int dvsec)
> >       if (rc)
> >               return rc;
> >
> > -     ctrl2 &= ~PCI_DVSEC_CXL_RST_MEM_CLR_EN;
> > +     if (mem_clr_en)
> > +             ctrl2 |= PCI_DVSEC_CXL_RST_MEM_CLR_EN;
> > +     else
> > +             ctrl2 &= ~PCI_DVSEC_CXL_RST_MEM_CLR_EN;
> >       ctrl2 |= PCI_DVSEC_CXL_INIT_CXL_RST;
> >       return cxl_reset_write_ctrl2(pdev, dvsec, ctrl2);  }
> 
> I'd again avoid a bool arg here, an enum type might be a better option.
> 
> I see in the spec though that CXL Reset Mem Clr Enable is only supported when
> the DVSEC CXL Capability register reports CXL Reset Mem Clr Capable though.
> Should this return -ENOTTY if memory clearing is requested when the device is
> not capable?

okay, agreed; I will return -ENOTTY when a clear is requested but
PCI_DVSEC_CXL_RST_MEM_CLR_CAPABLE is unset

> 
> > @@ -1273,7 +1276,7 @@ static int cxl_reset_wait_done(struct pci_dev
> > *pdev, int dvsec, u16 cap)  }
> >
> >  static int cxl_reset_execute(struct pci_dev *pdev, bool *target_prepared,
> > -                          int dvsec, u16 cap)
> > +                          int dvsec, u16 cap, bool mem_clr_en)
> >  {
> >       int rc, rc2;
> >
> > @@ -1283,7 +1286,7 @@ static int cxl_reset_execute(struct pci_dev
> > *pdev, bool *target_prepared,
> >
> >       rc = cxl_pci_target_reset_prepare(pdev, target_prepared);
> >       if (!rc)
> > -             rc = cxl_reset_initiate(pdev, dvsec);
> > +             rc = cxl_reset_initiate(pdev, dvsec, mem_clr_en);
> >       if (!rc)
> >               rc = cxl_reset_wait_done(pdev, dvsec, cap);
> >
> > @@ -1322,7 +1325,8 @@ int cxl_reset_function(struct pci_dev *pdev, bool
> probe)
> >       scoped_guard(rwsem_write, &cxl_rwsem.region) {
> >               rc = cxl_hdm_ranges_prepare(&range_ctx, pdev);
> >               if (!rc)
> > -                     rc = cxl_reset_execute(pdev, &target_prepared, dvsec, cap);
> > +                     rc = cxl_reset_execute(pdev, &target_prepared, dvsec,
> > +                                            cap, false);
> >               if (!rc) {
> >                       u16 command;
> >
> > @@ -1340,3 +1344,58 @@ int cxl_reset_function(struct pci_dev *pdev, bool
> probe)
> >       cxl_pci_target_reset_done(pdev, &target_prepared);
> >       return rc;
> >  }
> > +
> > +/* True when a function-scoped CXL reset is available for @pdev. */
> > +bool cxl_reset_capable(struct pci_dev *pdev) {
> > +     u16 cap;
> > +
> > +     if (cxl_reset_dvsec(pdev, &cap) < 0)
> > +             return false;
> > +
> > +     if (pdev->multifunction)
> > +             return false;
> > +
> > +     return cxl_reset_hdm_available(pdev); }
> > +EXPORT_SYMBOL_NS_GPL(cxl_reset_capable, "CXL");
> > +
> > +/*
> > + * Run the DVSEC reset sequence and restore HDM state for a caller
> > +that owns
> > + * device quiesce and PCI config save/restore, such as vfio-pci. The
> > +HDM range
> > + * collection and CPU cache flush that cxl_reset_function() does for
> > +host-owned
> > + * memory are skipped; that memory belongs to the guest here.
> 
> It's worth noting in the comment that a device memory clear without a
> preceding cache invalidation risks that the cache writes back some of the
> memory we just cleared.  The use case later in the series does that cache
> invalidation, but the requirement is a bit subtle here.  Thanks,
> 

I will rebase this onto cxl_reset v11 (cxl_reset_get_dvsec / cxl_reset_execute) and add
the cache-invalidation-before-clear details in the comment.

Manish

> Alex
> 
> 
> > + */
> > +int cxl_reset_dvsec_sequence(struct pci_dev *pdev, bool mem_clr_en) {
> > +     bool target_prepared = false;
> > +     int dvsec;
> > +     int rc;
> > +     u16 cap;
> > +
> > +     dvsec = cxl_reset_dvsec(pdev, &cap);
> > +     if (dvsec < 0)
> > +             return dvsec;
> > +
> > +     if (pdev->multifunction)
> > +             return -ENOTTY;
> > +
> > +     /*
> > +      * Trylock rather than block: This follows the trylock convention of
> > +      * pci_reset_bus().
> > +      */
> > +     if (!pci_dev_trylock(pdev))
> > +             return -EBUSY;
> > +
> > +     scoped_guard(rwsem_write, &cxl_rwsem.region) {
> > +             rc = cxl_reset_execute(pdev, &target_prepared, dvsec, cap,
> > +                                    mem_clr_en);
> > +             if (!rc)
> > +                     rc = cxl_restore_hdm_after_pci_reset(pdev);
> > +     }
> > +
> > +     cxl_pci_target_reset_done(pdev, &target_prepared);
> > +     pci_dev_unlock(pdev);
> > +     return rc;
> > +}
> > +EXPORT_SYMBOL_NS_GPL(cxl_reset_dvsec_sequence, "CXL");
> > diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h index
> > f8e8fddba152..541ed6de75a6 100644
> > --- a/include/cxl/cxl.h
> > +++ b/include/cxl/cxl.h
> > @@ -164,6 +164,8 @@ void pci_cxl_hdm_init(struct pci_dev *pdev);  void
> > pci_cxl_hdm_release(struct pci_dev *pdev);  int
> > cxl_restore_hdm_after_pci_reset(struct pci_dev *pdev);  int
> > cxl_reset_function(struct pci_dev *pdev, bool probe);
> > +bool cxl_reset_capable(struct pci_dev *pdev); int
> > +cxl_reset_dvsec_sequence(struct pci_dev *pdev, bool mem_clr_en);
> >  #else
> >  static inline void pci_cxl_hdm_init(struct pci_dev *pdev)  { @@
> > -182,6 +184,16 @@ static inline int cxl_reset_function(struct pci_dev
> > *pdev, bool probe)  {
> >       return -ENOTTY;
> >  }
> > +
> > +static inline bool cxl_reset_capable(struct pci_dev *pdev) {
> > +     return false;
> > +}
> > +
> > +static inline int cxl_reset_dvsec_sequence(struct pci_dev *pdev, bool
> > +mem_clr_en) {
> > +     return -ENOTTY;
> > +}
> >  #endif
> >
> >  struct cxl_reg_map {