Re: [PATCH 2/3] s390x/pci: refresh fh before disabling aif

[Cédric Le Goater]

On 1/16/24 23:31, Matthew Rosato wrote:
> Typically we refresh the host fh during CLP enable, however it's possible
> that the device goes through multiple reset events before the guest
> performs another CLP enable.  Let's handle this for now by refreshing the
> host handle from vfio before disabling aif.
>
> Fixes: 03451953c7 ("s390x/pci: reset ISM passthrough devices on shutdown and system 
> reset")
> Reported-by: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
> ---
>   hw/s390x/s390-pci-kvm.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/hw/s390x/s390-pci-kvm.c b/hw/s390x/s390-pci-kvm.c
> index f7e10cfa72..9eef4fc3ec 100644
> --- a/hw/s390x/s390-pci-kvm.c
> +++ b/hw/s390x/s390-pci-kvm.c
> @@ -18,6 +18,7 @@
>   #include "hw/s390x/s390-pci-bus.h"
>   #include "hw/s390x/s390-pci-kvm.h"
>   #include "hw/s390x/s390-pci-inst.h"
> +#include "hw/s390x/s390-pci-vfio.h"
>   #include "cpu_models.h"
>   
>   bool s390_pci_kvm_interp_allowed(void)
> @@ -64,9 +65,17 @@ int s390_pci_kvm_aif_disable(S390PCIBusDevice *pbdev)
>           return -EINVAL;
>       }
>   
> +    /*
> +     * The device may have already been reset but we still want to relinquish
> +     * the guest ISC, so always be sure to use an up-to-date host fh.
> +     */
> +    if (!s390_pci_get_host_fh(pbdev, &args.fh)) {
> +        return -EPERM;
> +    }

The callers of s390_pci_kvm_aif_disable() all test the original host
function with :

   if (pbdev->interp && (pbdev->fh & FH_MASK_ENABLE))

This change possibly fetches a new one. Shouldn't we move the test
also in s390_pci_kvm_aif_disable() ?


Thanks,

C.



> +
>       rc = kvm_vm_ioctl(kvm_state, KVM_S390_ZPCI_OP, &args);
>       if (rc == 0) {
> -        pbev->aif = false;
> +        pbdev->aif = false;
>       }
>   
>       return rc;