Re: [Qemu-devel] [PATCH v1 4/5] s390x/pci: Refuse to realize VFIO-PCI if AIS needed but supported

[Cornelia Huck]

On Wed,  4 Oct 2017 15:49:38 +0200
Pierre Morel <address@hidden> wrote:

"not supported", surely?

> In S390x the Adapter Interrupt Suppression facility is used to mask
> interrupts of other PCI devices during interruption handling.
> 
> VFIO PCI allows the interrupts to be delivered rapidely through KVM via
> IRQfd or to be delivered through QEMU.
> The choice is made through the x-kvm-intx and x-kvo-misx properties of
> the VFIO PCI device.
> 
> If the VFIO PCI device is using the direct KVM access through IRQfd and
> we know that KVM does not implement AIS support we refuse to realize the
> VFIO PCI device.
> 
> In all other cases, emulation and VFIO PCI sending interrupts through
> QEMU, we intercept the propagated IRQ, and protect it with the QEMU AIS
> implementation before to send it to the guest through KVM.
> 
> Signed-off-by: Pierre Morel <address@hidden>
> ---
>  hw/s390x/s390-pci-bus.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index d9c294a..4afe49b 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -21,18 +21,21 @@
>  #include "hw/pci/pci_bus.h"
>  #include "hw/pci/pci_bridge.h"
>  #include "hw/pci/msi.h"
> +#include "hw/vfio/pci.h"
>  #include "qemu/error-report.h"
>  
>  #ifndef DEBUG_S390PCI_BUS
>  #define DEBUG_S390PCI_BUS  0
>  #endif
>  
> +#ifndef DPRINTF
>  #define DPRINTF(fmt, ...)                                         \
>      do {                                                          \
>          if (DEBUG_S390PCI_BUS) {                                  \
>              fprintf(stderr, "S390pci-bus: " fmt, ## __VA_ARGS__); \
>          }                                                         \
>      } while (0)
> +#endif

Ugh. Is there DPRINTF redefinition going on?

>  
>  S390pciState *s390_get_phb(void)
>  {
> @@ -751,6 +754,15 @@ static void s390_pcihost_hot_plug(HotplugHandler 
> *hotplug_dev,
>          }
>  
>          if (object_dynamic_cast(OBJECT(dev), "vfio-pci")) {
> +            S390FLICState *fs = s390_get_flic();
> +            VFIOPCIDevice *vdev = DO_UPCAST(VFIOPCIDevice, pdev, pdev);
> +
> +            if ((!vdev->no_kvm_msix || !vdev->no_kvm_msix) &&

One of the no_kvm_msix should probably be something else :)

> +                (!fs || !fs->ais_supported)) {
> +                error_setg(errp, "VFIO PCI is not supported "
> +                           "because kernel has no AIS capability.");

I think failure is per-device (i.e., you would be able to plug a
different vfio device if you turned off irqfd support for it); should
that be reflected in the error message?

> +                return;
> +            }

Would it make sense to fail plugging before we try to create a zpci
device?

>              pbdev->fh |= FH_SHM_VFIO;
>          } else {
>              pbdev->fh |= FH_SHM_EMUL;