On 10/22/26 2:58 PM, Farhan Ali wrote:
> In s390_pci_device_reset(), IOMMU mappings are torn down without first
> resetting the device. 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) for the device on the host.
>
> Fix this by calling pci_device_reset() before tearing down IOMMU mappings
> in the default case. The dedicated ERROR-state case existed to issue
> that same reset before falling through, so it can now be removed.
>
> Fixes: 3e5cfba3ca0a ("s390x/pci: introduce S390PCIBusDevice qdev")
> Cc: [email protected]
> Signed-off-by: Farhan Ali <[email protected]>
I spent some time discussing this with Farhan off-list and asked him to
explore a possible alternate solution. To summarize:
While I believe this would fix the issue (by forcing the vfio-pci device
to be reset before the IOMMU is wiped), I wonder if we can do this more
cleanly by moving s390_pci_device_reset() from the legacy reset
interface to the multi-phase interface described in
docs/devel/reset.rst, and basically move all of s390_pci_device_reset(),
especially the IOMMU teardown, to the exit() call.
When the host bridge (TYPE_S390_PCI_HOST_BRIDGE) is reset during
subsystem_reset(), both the S390PCIBus and the PCIBus are reset. All
hold() handlers will complete before any exit() handler runs, so moving
pci_dereg_ioat() to exit() should guarantee that it runs after
vfio_pci_reset() has run in hold(), independent of the order of the two
buses on the bridge.
This should satisfy the following from reset.rst:
DMA capable devices are expected to cancel all outstanding DMA
operations during either 'enter' or 'hold' phases. IOMMUs are expected
to reset during the 'exit' phase and this sequencing makes sure no
outstanding DMA request will fault.
Thanks,
Matt
> ---
> hw/s390x/s390-pci-bus.c | 16 +---------------
> 1 file changed, 1 insertion(+), 15 deletions(-)
>
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index 2724b27ec2..7182d97a33 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -1786,22 +1786,8 @@ static void s390_pci_device_reset(DeviceState *dev)
> return;
> case ZPCI_FS_STANDBY:
> break;
> - case ZPCI_FS_ERROR:
> - /*
> - * Attempt a reset to recover the device. A device in ERROR state
> - * can be transitioned to ENABLED or DISABLED state via guest
> - * driven actions. Since this code path is also executed on
> - * subsystem_reset() and the expected state of a configured device
> - * after subsystem_reset() is to be in a DISABLED state, we need to
> - * transition from ERROR to DISABLED.
> - *
> - * Linux guests drive the state transition from ERROR to DISABLED
> - * via CLP_SET_DISABLE_PCI_FN. So it should be safe to transition
> - * from ERROR to DISABLED state.
> - */
> - pci_device_reset(pbdev->pdev);
> - /* fallthrough */
> default:
> + pci_device_reset(pbdev->pdev);
> pbdev->fh &= ~FH_MASK_ENABLE;
> pbdev->state = ZPCI_FS_DISABLED;
> break;
