Re: [PULL 2/7] s390x: do a subsystem reset before the unprotect on reboot

["Cédric Le Goater" <clg@redhat.com>]

On 9/12/23 13:41, Thomas Huth wrote:
> From: Janosch Frank <frankja@linux.ibm.com>
> 
> Bound APQNs have to be reset before tearing down the secure config via
> s390_machine_unprotect(). Otherwise the Ultravisor will return a error
> code.
> 
> So let's do a subsystem_reset() which includes a AP reset before the
> unprotect call. We'll do a full device_reset() afterwards which will
> reset some devices twice. That's ok since we can't move the
> device_reset() before the unprotect as it includes a CPU clear reset
> which the Ultravisor does not expect at that point in time.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> Message-ID: <20230901114851.154357-1-frankja@linux.ibm.com>
> Tested-by: Viktor Mihajlovski <mihajlov@linux.ibm.com>
> Acked-by: Christian Borntraeger <borntraeger@linux.ibm.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   hw/s390x/s390-virtio-ccw.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 3dd0b2372d..2d75f2131f 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -438,10 +438,20 @@ static void s390_machine_reset(MachineState *machine, ShutdownCause reason)
>       switch (reset_type) {
>       case S390_RESET_EXTERNAL:
>       case S390_RESET_REIPL:
> +        /*
> +         * Reset the subsystem which includes a AP reset. If a PV
> +         * guest had APQNs attached the AP reset is a prerequisite to
> +         * unprotecting since the UV checks if all APQNs are reset.
> +         */
> +        subsystem_reset();


This commit introduced a regression with pass-though ISM devices.

After startup, a reboot will generate extra device resets (vfio-pci in
this case) which break the pass-though ISM device in a subtle way,
probably related to IOMMU mapping according to 03451953c79e
("s390x/pci: reset ISM passthrough devices on shutdown and system
reset"). After poweroff, the device is left in a sort-of-a-use state
on the host and the LPAR has to be rebooted to clear the invalid state
of the device. To be noted, that standard PCI devices are immune to
this change.

The extra resets should avoided in some ways, (a shutdown notifier and
a reset callback are already registered for ISM devices by 03451953c79e)
and, most important, once the VM terminates, the device resources
should be cleared in the host kernel. So there seem to be two issues
to address in mainline QEMU and in Linux AFAICT.

Thanks,

C.



>           if (s390_is_pv()) {
>               s390_machine_unprotect(ms);
>           }
>   
> +        /*
> +         * Device reset includes CPU clear resets so this has to be
> +         * done AFTER the unprotect call above.
> +         */
>           qemu_devices_reset(reason);
>           s390_crypto_reset();
>