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

[Matthew Rosato <mjrosato@linux.ibm.com>]

On 12/10/20 5:28 AM, Cornelia Huck wrote:
> 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.
> 

Yes, but even so our current emulation support only sets up for MSI-X, 
it does not have an msi_init() equivalent.  I do believe that this 
requirement can be relaxed at some point for the emulation support as 
well, but the focus on this set was to at least stop fencing passthrough 
for no reason.