RE: [PATCH v4 23/27] vfio/cxl: Refresh the decoder snapshot after a device reset

[Manish Honap]

> -----Original Message-----
> From: Alex Williamson <alex@xxxxxxxxxxx>
> Sent: Saturday, August 29, 2026 4:03 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 23/27] vfio/cxl: Refresh the decoder snapshot after a
> device reset
> 
> External email: Use caution opening links or attachments
> 
> 
> On Thu, 13 Aug 2026 15:06:27 +0530
> <mhonap@xxxxxxxxxx> wrote:
> 
> > From: Manish Honap <mhonap@xxxxxxxxxx>
> >
> > A reset clears the HDM decoder registers, so the guest snapshot has to
> > be resampled once the reset settles, on every path that can reset the
> > function: the reset ioctl, an FLR driven through config space, and a
> > bus hot reset. Re-enable Memory Space first, since a config restore
> > can leave it off and the component-BAR read would then take an
> Unsupported Request.
> >
> > The bus hot reset zaps BARs directly rather than through
> > vfio_pci_zap_and_down_write_memory_lock(), so it also needs the HDM
> > window zapped by hand; route both zap sites through a common helper.
> >
> > Signed-off-by: Manish Honap <mhonap@xxxxxxxxxx>
> > ---
> >  drivers/vfio/pci/cxl/vfio_cxl_core.c | 82 ++++++++++++++++++++++++++++
> >  drivers/vfio/pci/vfio_pci_config.c   |  2 +
> >  drivers/vfio/pci/vfio_pci_core.c     | 25 ++++++++-
> >  drivers/vfio/pci/vfio_pci_priv.h     | 20 +++++++
> >  include/linux/vfio_pci_core.h        |  4 ++
> >  5 files changed, 131 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > b/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > index f1c6bf06c408..f45eaa60bad2 100644
> > --- a/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > +++ b/drivers/vfio/pci/cxl/vfio_cxl_core.c
> > @@ -555,6 +555,86 @@ static void vfio_cxl_zap(struct vfio_pci_core_device
> *vdev)
> >                           range_len(&cxl->hpa_range), true);  }
> >
> > +static void vfio_cxl_post_reset(struct vfio_pci_core_device *vdev) {
> > +     struct vfio_cxl_state *cxl = vdev->cxl;
> > +     struct pci_dev *pdev = vdev->pdev;
> > +     bool re_enabled = false;
> > +     int i, dwords;
> > +     u16 cmd;
> > +
> > +     lockdep_assert_held_write(&vdev->memory_lock);
> > +
> > +     if (!cxl || !cxl->hdm_shadow)
> > +             return;
> > +
> > +     /*
> > +      * The decoder registers are read through the component BAR. A config
> > +      * restore can leave Memory Space disabled, and the read would then
> > +      * return an Unsupported Request, so re-enable it before sampling.
> > +      */
> > +     pci_read_config_word(pdev, PCI_COMMAND, &cmd);
> > +     if (!(cmd & PCI_COMMAND_MEMORY)) {
> > +             pci_write_config_word(pdev, PCI_COMMAND,
> > +                                   cmd | PCI_COMMAND_MEMORY);
> > +             re_enabled = true;
> > +     }
> > +
> > +     dwords = cxl->hdm_len / sizeof(u32);
> > +     for (i = 0; i < dwords; i++)
> > +             cxl->hdm_shadow[i] = cpu_to_le32(readl(cxl->hdm_regs +
> > +                                                    i * sizeof(u32)));
> > +     /*
> > +      * Leave Memory Space as it was found. The guest owns Memory Space
> > +      * through vconfig, so a physical enable done only to sample must not
> > +      * outlive the sampling or the function would decode while vconfig
> > +      * reports it off.
> > +      */
> > +     if (re_enabled)
> > +             pci_write_config_word(pdev, PCI_COMMAND, cmd);
> 
> This is usually done by just saving the original and re-writing it, but this whole
> patch is really just ammunition for why it should be read live rather than
> shadowed.
> 
> See also the reset_done callback in pci_error_handlers, we shouldn't be open
> coding calls to this everywhere, but of course this goes away if we expose it live
> rather than shadow it.  Thanks,
> 
> Alex

I will drop this patch as per suggestion on patch 18.

Manish

> 
> > +}
> > +
> > +static int vfio_cxl_pm_restore(struct vfio_pci_core_device *vdev) {
> > +     struct vfio_cxl_state *cxl = vdev->cxl;
> > +     struct pci_dev *pdev = vdev->pdev;
> > +     int rc;
> > +
> > +     lockdep_assert_held_write(&vdev->memory_lock);
> > +
> > +     if (!cxl || !cxl->hdm_shadow) {
> > +             pci_dbg(pdev, "vfio-cxl: pm_restore: no shadow (device not open),
> skipping\n");
> > +             return 0;
> > +     }
> > +
> > +     /*
> > +      * A D3hot->D0 transition can soft-reset the function and clear the HDM
> > +      * decoder. Restore the physical decoder before the fault gate re-inserts
> > +      * the mapping. The restore needs the device lock, taken here after
> > +      * memory_lock to match the reset path ordering. On failure the decoder
> is
> > +      * left unrestored, so close the access gate (zap no longer clears it) and
> > +      * return the error so the caller keeps the HDM range inaccessible.
> > +      */
> > +     if (!pci_dev_trylock(pdev)) {
> > +             pci_warn(pdev, "vfio-cxl: pm_restore: could not lock device, HDM
> not restored\n");
> > +             cxl->hdm_valid = false;
> > +             return -EBUSY;
> > +     }
> > +
> > +     rc = cxl_restore_hdm_after_pci_reset(pdev);
> > +     pci_dev_unlock(pdev);
> > +     if (rc) {
> > +             pci_err(pdev, "vfio-cxl: pm_restore: HDM restore failed: %d\n", rc);
> > +             cxl->hdm_valid = false;
> > +             return rc;
> > +     }
> > +
> > +     vfio_cxl_post_reset(vdev);
> > +     /* The decoder is restored and re-sampled, so reopen the access gate. */
> > +     cxl->hdm_valid = true;
> > +     return 0;
> > +}
> > +
> >  static int vfio_cxl_open_device(struct vfio_pci_core_device *vdev)  {
> >       struct vfio_cxl_state *cxl = vdev->cxl; @@ -767,6 +847,8 @@
> > static const struct vfio_cxl_ops vfio_cxl_ops = {
> >       .config_read    = vfio_cxl_config_read,
> >       .config_write   = vfio_cxl_config_write,
> >       .zap            = vfio_cxl_zap,
> > +     .post_reset     = vfio_cxl_post_reset,
> > +     .pm_restore     = vfio_cxl_pm_restore,
> >       .owner          = THIS_MODULE,
> >  };
> >
> > diff --git a/drivers/vfio/pci/vfio_pci_config.c
> > b/drivers/vfio/pci/vfio_pci_config.c
> > index f088e4ce5e07..01d808546a4c 100644
> > --- a/drivers/vfio/pci/vfio_pci_config.c
> > +++ b/drivers/vfio/pci/vfio_pci_config.c
> > @@ -911,6 +911,7 @@ static int vfio_exp_config_write(struct
> vfio_pci_core_device *vdev, int pos,
> >                       vfio_pci_zap_and_down_write_memory_lock(vdev);
> >                       vfio_pci_dma_buf_move(vdev, true);
> >                       pci_try_reset_function(vdev->pdev);
> > +                     vfio_pci_cxl_post_reset(vdev);
> >                       if (__vfio_pci_memory_enabled(vdev))
> >                               vfio_pci_dma_buf_move(vdev, false);
> >                       up_write(&vdev->memory_lock); @@ -996,6 +997,7
> > @@ static int vfio_af_config_write(struct vfio_pci_core_device *vdev, int
> pos,
> >                       vfio_pci_zap_and_down_write_memory_lock(vdev);
> >                       vfio_pci_dma_buf_move(vdev, true);
> >                       pci_try_reset_function(vdev->pdev);
> > +                     vfio_pci_cxl_post_reset(vdev);
> >                       if (__vfio_pci_memory_enabled(vdev))
> >                               vfio_pci_dma_buf_move(vdev, false);
> >                       up_write(&vdev->memory_lock); diff --git
> > a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> > index 1a54f15d1c2c..fc8235c8b4fc 100644
> > --- a/drivers/vfio/pci/vfio_pci_core.c
> > +++ b/drivers/vfio/pci/vfio_pci_core.c
> > @@ -362,6 +362,14 @@ int vfio_pci_set_power_state(struct
> vfio_pci_core_device *vdev, pci_power_t stat
> >               } else if (needs_restore) {
> >                       pci_load_and_free_saved_state(pdev, &vdev->pm_save);
> >                       pci_restore_state(pdev);
> > +                     /*
> > +                      * A NoSoftRst- device soft-resets on D3hot->D0, which can
> > +                      * clear a CXL HDM decoder. Restore it before the fault
> > +                      * gate re-inserts the HDM mapping. memory_lock is held on
> > +                      * this path (the PM config write and runtime PM entry both
> > +                      * take it before the D0 transition).
> > +                      */
> > +                     vfio_pci_cxl_pm_restore(vdev);
> >               }
> >       }
> >
> > @@ -529,6 +537,13 @@ static int vfio_pci_core_runtime_resume(struct
> device *dev)
> >               eventfd_signal(vdev->pm_wake_eventfd_ctx);
> >               __vfio_pci_runtime_pm_exit(vdev);
> >       }
> > +     /*
> > +      * A NoSoftRst- function can soft-reset on the runtime D3hot->D0
> > +      * transition and clear a CXL HDM decoder. Restore it while memory_lock
> > +      * is held, before the fault gate can re-insert the HDM mapping. PCI
> > +      * config restore alone does not restore the component decoder registers.
> > +      */
> > +     vfio_pci_cxl_pm_restore(vdev);
> >       up_write(&vdev->memory_lock);
> >
> >       if (vdev->pm_intx_masked)
> > @@ -1449,6 +1464,7 @@ static int vfio_pci_ioctl_reset(struct
> > vfio_pci_core_device *vdev,
> >
> >       vfio_pci_dma_buf_move(vdev, true);
> >       ret = pci_try_reset_function(vdev->pdev);
> > +     vfio_pci_cxl_post_reset(vdev);
> >       if (__vfio_pci_memory_enabled(vdev))
> >               vfio_pci_dma_buf_move(vdev, false);
> >       up_write(&vdev->memory_lock);
> > @@ -1838,8 +1854,7 @@ void
> vfio_pci_zap_and_down_write_memory_lock(struct vfio_pci_core_device
> *vdev)
> >        * a runtime-PM entry, D3 transition, or reset would leave the guest
> >        * with live mappings into a quiesced device.
> >        */
> > -     if (vdev->cxl_ops && vdev->cxl_ops->zap)
> > -             vdev->cxl_ops->zap(vdev);
> > +     vfio_pci_cxl_zap(vdev);
> >  }
> >
> >  u16 vfio_pci_memory_lock_and_enable(struct vfio_pci_core_device
> > *vdev) @@ -2780,6 +2795,8 @@ static int
> > vfio_pci_dev_set_hot_reset(struct vfio_device_set *dev_set,
> >
> >               vfio_pci_dma_buf_move(vdev, true);
> >               vfio_pci_zap_bars(vdev);
> > +             /* zap_bars misses the HDM window; bus reset needs it too */
> > +             vfio_pci_cxl_zap(vdev);
> >       }
> >
> >       if (!list_entry_is_head(vdev,
> > @@ -2802,6 +2819,10 @@ static int vfio_pci_dev_set_hot_reset(struct
> > vfio_device_set *dev_set,
> >
> >       ret = pci_reset_bus(pdev);
> >
> > +     /* Re-sample decoder state for any CXL device the bus reset touched. */
> > +     list_for_each_entry(vdev, &dev_set->device_list, vdev.dev_set_list)
> > +             vfio_pci_cxl_post_reset(vdev);
> > +
> >       vdev = list_last_entry(&dev_set->device_list,
> >                              struct vfio_pci_core_device,
> > vdev.dev_set_list);
> >
> > diff --git a/drivers/vfio/pci/vfio_pci_priv.h
> > b/drivers/vfio/pci/vfio_pci_priv.h
> > index 902d17815ab6..46e67573d264 100644
> > --- a/drivers/vfio/pci/vfio_pci_priv.h
> > +++ b/drivers/vfio/pci/vfio_pci_priv.h
> > @@ -82,6 +82,26 @@ int vfio_pci_set_power_state(struct
> vfio_pci_core_device *vdev,
> >                            pci_power_t state);
> >
> >  void vfio_pci_zap_and_down_write_memory_lock(struct
> > vfio_pci_core_device *vdev);
> > +
> > +static inline void vfio_pci_cxl_zap(struct vfio_pci_core_device
> > +*vdev) {
> > +     if (vdev->cxl_ops && vdev->cxl_ops->zap)
> > +             vdev->cxl_ops->zap(vdev); }
> > +
> > +static inline void vfio_pci_cxl_post_reset(struct
> > +vfio_pci_core_device *vdev) {
> > +     if (vdev->cxl_ops && vdev->cxl_ops->post_reset)
> > +             vdev->cxl_ops->post_reset(vdev); }
> > +
> > +static inline int vfio_pci_cxl_pm_restore(struct vfio_pci_core_device
> > +*vdev) {
> > +     if (vdev->cxl_ops && vdev->cxl_ops->pm_restore)
> > +             return vdev->cxl_ops->pm_restore(vdev);
> > +     return 0;
> > +}
> > +
> >  u16 vfio_pci_memory_lock_and_enable(struct vfio_pci_core_device
> > *vdev);  void vfio_pci_memory_unlock_and_restore(struct
> vfio_pci_core_device *vdev,
> >                                       u16 cmd); diff --git
> > a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h index
> > 8b93949d4484..c438d968dc59 100644
> > --- a/include/linux/vfio_pci_core.h
> > +++ b/include/linux/vfio_pci_core.h
> > @@ -78,6 +78,10 @@ struct vfio_cxl_ops {
> >                               int count, __le32 val);
> >       /* Revoke the HDM mapping; paired with the BAR zap */
> >       void    (*zap)(struct vfio_pci_core_device *vdev);
> > +     /* Re-sample the decoder state once a reset has settled */
> > +     void    (*post_reset)(struct vfio_pci_core_device *vdev);
> > +     /* Restore the HDM decoder after a D3hot->D0 soft reset */
> > +     int     (*pm_restore)(struct vfio_pci_core_device *vdev);
> >
> >       /* Pinned per bound CXL device so vfio-cxl cannot unload under usage */
> >       struct module *owner;