On 10/26/26 1:13 PM, Farhan Ali wrote:
> On subsystem reset of the s390 PCI host bridge (TYPE_S390_PCI_HOST_BRIDGE),
> child buses S390PCIBus and PCIBus are reset in order. Because S390PCIBus
> devices are reset first via the legacy reset hook, IOMMU mappings are torn
> down before the PCI device itself is reset. For a PCI passthrough device
> this opens a window where the device may issue DMA to addresses that no
> longer have valid IOMMU mappings, causing an error (PEC 0x7) on the host.
>
> The QEMU reset documentation (docs/devel/reset.rst) specifies that IOMMUs
> should perform teardown in the resettable 'exit' phase, after all devices
> have completed their 'hold' phase. Move the S390PCIBusDevice reset logic
> from the legacy reset hook to the resettable exit phase to close this
> window.
>
> Fixes: 3e5cfba3ca0a ("s390x/pci: introduce S390PCIBusDevice qdev")
> Cc: [email protected]
> Signed-off-by: Farhan Ali <[email protected]>
Thanks Farhan - I think this looks good, but I believe there's still
something off here that could either be fixed in this patch or with a
2nd patch before this one.
My concern is the interplay between the unplug_requested flag and the
FMB -- I think in the unlikely event we pcihost_reset() when
unplug_requested is set we would now miss freeing the FMB timer because
we won't actually end up driving s390_pci_device_reset_exit(). That's
because pcihost_reset is still operating on the legacy interface (i.e.
it gets run during the hold phase so the device gets removed from the
bus before the exit phase)
I think the simple answer would be to make sure the FMB timer is freed
in s390_pcihost_unplug() e.g. by calling fmb_timer_free() during
TYPE_S390_PCI_DEVICE handling. If it was already called before there
will be no timer to cancel and the FMB will just get re-zero'd one last
time.
Could you take a closer look?
Thanks,
Matt
> ---
> v1: https://lore.kernel.org/all/[email protected]/
> v1 -> v2
> - Change to s390_pci_device_reset() to resettable exit function.
>
> hw/s390x/s390-pci-bus.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index 2724b27ec2..a24ec98c1e 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -1777,9 +1777,9 @@ static void s390_pci_device_realize(DeviceState *dev,
> Error **errp)
> zpci->fmb.format = ZPCI_FMB_FORMAT;
> }
>
> -static void s390_pci_device_reset(DeviceState *dev)
> +static void s390_pci_device_reset_exit(Object *obj, ResetType type)
> {
> - S390PCIBusDevice *pbdev = S390_PCI_DEVICE(dev);
> + S390PCIBusDevice *pbdev = S390_PCI_DEVICE(obj);
>
> switch (pbdev->state) {
> case ZPCI_FS_RESERVED:
> @@ -2010,10 +2010,11 @@ static const VMStateDescription
> s390_pci_device_vmstate = {
> static void s390_pci_device_class_init(ObjectClass *klass, const void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> + ResettableClass *rc = RESETTABLE_CLASS(klass);
>
> dc->desc = "zpci device";
> set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> - device_class_set_legacy_reset(dc, s390_pci_device_reset);
> + rc->phases.exit = s390_pci_device_reset_exit;
> dc->bus_type = TYPE_S390_PCI_BUS;
> dc->realize = s390_pci_device_realize;
> device_class_set_props(dc, s390_pci_device_properties);
