On Tue, 2024-01-16 at 17:31 -0500, Matthew Rosato wrote: > ISM devices are sensitive to manipulation of the IOMMU, so the ISM > device > needs to be reset before the vfio-pci device is reset (triggering a > full > UNMAP). In order to ensure this occurs, trigger ISM device resets > from > subsystem_reset before triggering the PCI bus reset (which will also > trigger vfio-pci reset). This only needs to be done for ISM devices > which were enabled for use by the guest. > Further, ensure that AIF is disabled as part of the reset event. > > Fixes: ef1535901a ("s390x: do a subsystem reset before the unprotect > on reboot") > Fixes: 03451953c7 ("s390x/pci: reset ISM passthrough devices on > shutdown and system reset") > Reported-by: Cédric Le Goater <c...@redhat.com> > Signed-off-by: Matthew Rosato <mjros...@linux.ibm.com> > --- > hw/s390x/s390-pci-bus.c | 26 +++++++++++++++++--------- > hw/s390x/s390-virtio-ccw.c | 2 ++ > include/hw/s390x/s390-pci-bus.h | 1 + > 3 files changed, 20 insertions(+), 9 deletions(-) > > diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c > index 347580ebac..3e57d5faca 100644 > --- a/hw/s390x/s390-pci-bus.c > +++ b/hw/s390x/s390-pci-bus.c > @@ -151,20 +151,12 @@ static void s390_pci_shutdown_notifier(Notifier > *n, void *opaque) > pci_device_reset(pbdev->pdev); > } > > -static void s390_pci_reset_cb(void *opaque) > -{ > - S390PCIBusDevice *pbdev = opaque; > - > - pci_device_reset(pbdev->pdev); > -} > - > static void s390_pci_perform_unplug(S390PCIBusDevice *pbdev) > { > HotplugHandler *hotplug_ctrl; > > if (pbdev->pft == ZPCI_PFT_ISM) { > notifier_remove(&pbdev->shutdown_notifier); > - qemu_unregister_reset(s390_pci_reset_cb, pbdev); > } > > /* Unplug the PCI device */ > @@ -1132,7 +1124,6 @@ static void s390_pcihost_plug(HotplugHandler > *hotplug_dev, DeviceState *dev, > if (pbdev->pft == ZPCI_PFT_ISM) { > pbdev->shutdown_notifier.notify = > s390_pci_shutdown_notifier; > qemu_register_shutdown_notifier(&pbdev- > >shutdown_notifier); > - qemu_register_reset(s390_pci_reset_cb, pbdev); > } > } else { > pbdev->fh |= FH_SHM_EMUL; > @@ -1279,6 +1270,23 @@ static void s390_pci_enumerate_bridge(PCIBus > *bus, PCIDevice *pdev, > pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, s->bus_no, > 1); > } > > +void s390_pci_ism_reset(void) > +{ > + S390pciState *s = s390_get_phb(); > + > + S390PCIBusDevice *pbdev, *next; > + > + /* Trigger reset event for each passthrough ISM device currently > in-use */ > + QTAILQ_FOREACH_SAFE(pbdev, &s->zpci_devs, link, next) { > + if (pbdev->interp && pbdev->pft == ZPCI_PFT_ISM && > + pbdev->fh & FH_MASK_ENABLE) { > + s390_pci_kvm_aif_disable(pbdev); > + > + pci_device_reset(pbdev->pdev);
Do we care about the loss of a reset for ISM devices in a !interpretation case? (I seem to think such a configuration is not possible today, and so we don't care, but could use a reminder.) > + } > + } > +} > + > static void s390_pcihost_reset(DeviceState *dev) > { > S390pciState *s = S390_PCI_HOST_BRIDGE(dev); > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > index 1169e20b94..4de04f7e9f 100644 > --- a/hw/s390x/s390-virtio-ccw.c > +++ b/hw/s390x/s390-virtio-ccw.c > @@ -118,6 +118,8 @@ static void subsystem_reset(void) > DeviceState *dev; > int i; > > + s390_pci_ism_reset(); > + > for (i = 0; i < ARRAY_SIZE(reset_dev_types); i++) { > dev = DEVICE(object_resolve_path_type("", > reset_dev_types[i], NULL)); > if (dev) { > diff --git a/include/hw/s390x/s390-pci-bus.h b/include/hw/s390x/s390- > pci-bus.h > index 435e788867..2c43ea123f 100644 > --- a/include/hw/s390x/s390-pci-bus.h > +++ b/include/hw/s390x/s390-pci-bus.h > @@ -401,5 +401,6 @@ S390PCIBusDevice > *s390_pci_find_dev_by_target(S390pciState *s, > const char *target); > S390PCIBusDevice *s390_pci_find_next_avail_dev(S390pciState *s, > S390PCIBusDevice > *pbdev); > +void s390_pci_ism_reset(void); > > #endif