Re: [RFC 2/8] s390x/pci: MSI-X isn't strictly required for passthrough

[Cornelia Huck <cohuck@redhat.com>]

On Wed,  9 Dec 2020 15:34:20 -0500
Matthew Rosato <mjrosato@linux.ibm.com> wrote:

> s390 PCI currently disallows PCI devices without the MSI-X capability.
> However, this fence doesn't make sense for passthrough devices.  Move
> the check to only fence emulated devices (e.g., virtio).
> 
> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
> Reviewed-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  hw/s390x/s390-pci-bus.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index 05f7460..afad048 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -1028,12 +1028,12 @@ static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>              s390_pci_get_clp_info(pbdev);
>          } else {
>              pbdev->fh |= FH_SHM_EMUL;
> -        }
>  
> -        if (s390_pci_msix_init(pbdev)) {
> -            error_setg(errp, "MSI-X support is mandatory "
> -                       "in the S390 architecture");
> -            return;
> +            if (s390_pci_msix_init(pbdev)) {
> +                error_setg(errp, "MSI-X support is mandatory "
> +                           "in the S390 architecture");
> +                return;
> +            }
>          }
>  
>          if (dev->hotplugged) {
> @@ -1073,7 +1073,9 @@ static void s390_pcihost_unplug(HotplugHandler *hotplug_dev, DeviceState *dev,
>          devfn = pci_dev->devfn;
>          qdev_unrealize(dev);
>  
> -        s390_pci_msix_free(pbdev);
> +        if (pbdev->fh & FH_SHM_EMUL) {
> +            s390_pci_msix_free(pbdev);
> +        }
>          s390_pci_iommu_free(s, bus, devfn);
>          pbdev->pdev = NULL;
>          pbdev->state = ZPCI_FS_RESERVED;

Remind me: Wasn't it only msi that was strictly required (i.e., not msi-x?)

Can we generally relax this requirement, possibly with some changes in
the adapter interrupt mapping? I might misremember, though.