Re: [PATCH v2 3/3] s390x/pci: drive ISM reset from subsystem reset

[Halil Pasic]

On Thu, 18 Jan 2024 13:51:51 -0500
Matthew Rosato <mjrosato@linux.ibm.com> wrote:

> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index eaf61d3640..c99682b07d 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -118,6 +118,14 @@ static void subsystem_reset(void)
>      DeviceState *dev;
>      int i;
>  
> +    /*
> +     * ISM firmware is sensitive to unexpected changes to the IOMMU, which 
> can
> +     * occur during reset of the vfio-pci device (unmap of entire aperture).
> +     * Ensure any passthrough ISM devices are reset now, while CPUs are 
> paused
> +     * but before vfio-pci cleanup occurs.
> +     */
> +    s390_pci_ism_reset();

Hm I'm not sure about special casing ISM in here. In my opinion the loop
below shall take care of all the reset.

For TYPE_AP_BRIDGE and TYPE_VIRTUAL_CSS_BRIDGE AFAIU a
device_cold_reset() on all objects of those types results in the resets
of objects that hang below these buses.

I guess this also happens for the S390PCIBusDevices, but not for the
actual PCI devices.

My understanding is that the entire PCI subsystem is to be reset when
a subsystem reset is performed.

I'm not familiar enough with our PCI emulation to know if a reset
to the TYPE_S390_PCI_HOST_BRIDGE is supposed to be sufficient to
accomplish that.

I have the feeling, I am missing something... Can you help me understand
this please?


> +
>      for (i = 0; i < ARRAY_SIZE(reset_dev_types); i++) {
>          dev = DEVICE(object_resolve_path_type("", reset_dev_types[i], NULL));
>          if (dev)