1. Patchew
  2. QEMU
  3. [RFC 0/8] s390x/pci: Fixing s390 vfio-pci ISM support
  4. View patch

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

Matthew Rosato posted 8 patches 5 years, 2 months ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Matthew Rosato <mjrosato@linux.ibm.com>, Christian Borntraeger <borntraeger@de.ibm.com>, Thomas Huth <thuth@redhat.com>, Halil Pasic <pasic@linux.ibm.com>, David Hildenbrand <david@redhat.com>, Cornelia Huck <cohuck@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>
There is a newer version of this series
[RFC 2/8] s390x/pci: MSI-X isn't strictly required for passthrough
Posted by Matthew Rosato 5 years, 2 months ago 
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;
-- 
1.8.3.1
Re: [RFC 2/8] s390x/pci: MSI-X isn't strictly required for passthrough
Posted by Cornelia Huck 5 years, 2 months ago 
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.
Re: [RFC 2/8] s390x/pci: MSI-X isn't strictly required for passthrough
Posted by Matthew Rosato 5 years, 2 months ago 
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.
Re: [RFC 2/8] s390x/pci: MSI-X isn't strictly required for passthrough
Posted by Cornelia Huck 5 years, 1 month ago 
On Thu, 10 Dec 2020 10:13:29 -0500
Matthew Rosato <mjrosato@linux.ibm.com> wrote:

> 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.

Looking back to when this was introduced, I see that 857cc71985dc
("s390x/pci: merge msix init functions") actually makes this mandatory
and states that nothing changes for passthrough. Has anything changed
regarding msi-x in the architecture in the meantime?
Re: [RFC 2/8] s390x/pci: MSI-X isn't strictly required for passthrough
Posted by Matthew Rosato 5 years, 1 month ago 
On 12/17/20 8:08 AM, Cornelia Huck wrote:
> On Thu, 10 Dec 2020 10:13:29 -0500
> Matthew Rosato <mjrosato@linux.ibm.com> wrote:
> 
>> 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.
> 
> Looking back to when this was introduced, I see that 857cc71985dc
> ("s390x/pci: merge msix init functions") actually makes this mandatory
> and states that nothing changes for passthrough. Has anything changed
> regarding msi-x in the architecture in the meantime?
> 

I don't believe anything has changed from the architecture or kernel 
support perspective (Maybe I'm wrong about the latter; Niklas feel free 
to correct me in that case) -- Otherwise, as far as I can, tell the 
statement from 857cc71985dc was just incorrect and the restriction was 
placed unnecessarily for passthrough.  vfio-pci sets up for both, and 
our base s390 kernel support allows for both (and I can drive ISM 
traffic and see interrupts being routed to the guest with these patches 
applied of course); it's just our qemu emulation support that only sets 
up for MSI-X.

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.