| Message ID | 1507124979-8880-5-git-send-email-pmorel@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Refactoring of AIS support | expand |
On Wed, 4 Oct 2017 15:49:38 +0200 Pierre Morel <pmorel@linux.vnet.ibm.com> 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 <pmorel@linux.vnet.ibm.com> > --- > 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;
On 09/10/2017 11:06, Cornelia Huck wrote: > On Wed, 4 Oct 2017 15:49:38 +0200 > Pierre Morel <pmorel@linux.vnet.ibm.com> wrote: > > "not supported", surely? :) yes, "not supported" > >> 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 <pmorel@linux.vnet.ibm.com> >> --- >> 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? Yes in hw/vfio/vfio-common.h But I think I better do an undef / def instead of a ifndef #ifdef DPRINTF #undef DPRINTF #endif #define DPRINTF(fmt, ...) ... > >> >> 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 :) :) yes Historicaly, it used to be a no_kvm_intx but since AIS only works with AEN intx has no reason to be here. So the second no_kvm_msix must disappear. > >> + (!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? OK, I will find something better. > >> + return; >> + } > > Would it make sense to fail plugging before we try to create a zpci > device? clearly yes. I will put these test before the eventual zpci allocation. > >> pbdev->fh |= FH_SHM_VFIO; >> } else { >> pbdev->fh |= FH_SHM_EMUL; > > Thanks, Pierre
On Wed, 4 Oct 2017 15:49:38 +0200 Pierre Morel <pmorel@linux.vnet.ibm.com> wrote: > 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 <pmorel@linux.vnet.ibm.com> > --- > 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 Maybe a sign we shouldn't be including vfio/pci.h > 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) && > + (!fs || !fs->ais_supported)) { > + error_setg(errp, "VFIO PCI is not supported " > + "because kernel has no AIS capability."); > + return; > + } Hmm, you're basically looking at private data structure fields for experimental disable flags, which no user, or more importantly no management tool, could rightfully be expected to provide. Can we not take this into account to automatically do the right thing, ie. automatically set the necessary flag during the device realize? Thanks, Alex
On 09/10/2017 16:45, Alex Williamson wrote: > On Wed, 4 Oct 2017 15:49:38 +0200 > Pierre Morel <pmorel@linux.vnet.ibm.com> wrote: > >> 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 <pmorel@linux.vnet.ibm.com> >> --- >> 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 > > Maybe a sign we shouldn't be including vfio/pci.h > >> 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) && >> + (!fs || !fs->ais_supported)) { >> + error_setg(errp, "VFIO PCI is not supported " >> + "because kernel has no AIS capability."); >> + return; >> + } > > > Hmm, you're basically looking at private data structure fields for > experimental disable flags, which no user, or more importantly no > management tool, could rightfully be expected to provide. Can we not > take this into account to automatically do the right thing, ie. > automatically set the necessary flag during the device realize? Thanks, I am not sure to understand. Here we are in a s390x specific device and without vfio/pci.h, we have no access to the PCI VFIOdevice structure from the realize function. Do you mean to add an entry in the VFIOPCIdevice realize? A new quirk ? An architecture callback, because we also need access to S390FLICState ? We also have the possibility to not fall-back to AIS emulation for VFIO but we would loose the possibility to migrate to a host not supporting AIS. Thanks for your help. Pierre > > Alex >
On Mon, 9 Oct 2017 19:16:23 +0200 Pierre Morel <pmorel@linux.vnet.ibm.com> wrote: > On 09/10/2017 16:45, Alex Williamson wrote: > > On Wed, 4 Oct 2017 15:49:38 +0200 > > Pierre Morel <pmorel@linux.vnet.ibm.com> wrote: > > > >> 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 <pmorel@linux.vnet.ibm.com> > >> --- > >> 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 > > > > Maybe a sign we shouldn't be including vfio/pci.h > > > >> 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) && > >> + (!fs || !fs->ais_supported)) { > >> + error_setg(errp, "VFIO PCI is not supported " > >> + "because kernel has no AIS capability."); > >> + return; > >> + } > > > > > > Hmm, you're basically looking at private data structure fields for > > experimental disable flags, which no user, or more importantly no > > management tool, could rightfully be expected to provide. Can we not > > take this into account to automatically do the right thing, ie. > > automatically set the necessary flag during the device realize? Thanks, > > I am not sure to understand. > > Here we are in a s390x specific device and without vfio/pci.h, we have > no access to the PCI VFIOdevice structure from the realize function. > > Do you mean to add an entry in the VFIOPCIdevice realize? > A new quirk ? > An architecture callback, because we also need access to S390FLICState ? > > > We also have the possibility to not fall-back to AIS emulation for VFIO > but we would loose the possibility to migrate to a host not supporting AIS. Thinking about this some more: The problem is generally "if we have to fall back to ais emulation, we cannot support any pci device that bypasses qemu's interrupt injection", no? So this would imply vfio devices where the experimental switch is not used, but also virtio-pci if it is configured to use irqfd, I think. The ugly thing is that the platform-specific code depends on a property of the plugged device being set or not set... and that's likely to be different from device type to device type, and may not even be configurable for some device types.
On 10/10/2017 11:35, Cornelia Huck wrote: > On Mon, 9 Oct 2017 19:16:23 +0200 > Pierre Morel <pmorel@linux.vnet.ibm.com> wrote: > >> On 09/10/2017 16:45, Alex Williamson wrote: >>> On Wed, 4 Oct 2017 15:49:38 +0200 >>> Pierre Morel <pmorel@linux.vnet.ibm.com> wrote: >>> >>>> 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 <pmorel@linux.vnet.ibm.com> >>>> --- >>>> 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 >>> >>> Maybe a sign we shouldn't be including vfio/pci.h >>> >>>> 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) && >>>> + (!fs || !fs->ais_supported)) { >>>> + error_setg(errp, "VFIO PCI is not supported " >>>> + "because kernel has no AIS capability."); >>>> + return; >>>> + } >>> >>> >>> Hmm, you're basically looking at private data structure fields for >>> experimental disable flags, which no user, or more importantly no >>> management tool, could rightfully be expected to provide. Can we not >>> take this into account to automatically do the right thing, ie. >>> automatically set the necessary flag during the device realize? Thanks, >> >> I am not sure to understand. >> >> Here we are in a s390x specific device and without vfio/pci.h, we have >> no access to the PCI VFIOdevice structure from the realize function. >> >> Do you mean to add an entry in the VFIOPCIdevice realize? >> A new quirk ? >> An architecture callback, because we also need access to S390FLICState ? >> >> >> We also have the possibility to not fall-back to AIS emulation for VFIO >> but we would loose the possibility to migrate to a host not supporting AIS. > > Thinking about this some more: The problem is generally "if we have to > fall back to ais emulation, we cannot support any pci device that > bypasses qemu's interrupt injection", no? > > So this would imply vfio devices where the experimental switch is not > used, but also virtio-pci if it is configured to use irqfd, I think. > > The ugly thing is that the platform-specific code depends on a property > of the plugged device being set or not set... and that's likely to be > different from device type to device type, and may not even be > configurable for some device types. > Yes, right. Since in the case we have no AIS in KVM only PCI is impacted, do you think we can fence IRQFD for MSI with kvm_msi_via_irqfd_allowed = false; in kvm_arch_init_irq_routing() ? In fact, if we also verify the AIS availability in the same routine we can avoid the ugly include of vfio/pci.h Regards, Pierre
On Tue, 10 Oct 2017 18:01:53 +0200 Pierre Morel <pmorel@linux.vnet.ibm.com> wrote: > Since in the case we have no AIS in KVM only PCI is impacted, do you > think we can fence IRQFD for MSI with > > kvm_msi_via_irqfd_allowed = false; > > in kvm_arch_init_irq_routing() ? Hm, it seems we never set that for s390x... does that imply that we never support irqfd for pci anyway? > > In fact, if we also verify the AIS availability in the same routine we > can avoid the ugly include of vfio/pci.h If we can get the serialization right (and setting that value to false does indeed have the desired effect), that's probably the way to go.
On 11/10/2017 14:20, Cornelia Huck wrote: > On Tue, 10 Oct 2017 18:01:53 +0200 > Pierre Morel <pmorel@linux.vnet.ibm.com> wrote: > >> Since in the case we have no AIS in KVM only PCI is impacted, do you >> think we can fence IRQFD for MSI with >> >> kvm_msi_via_irqfd_allowed = false; >> >> in kvm_arch_init_irq_routing() ? > > Hm, it seems we never set that for s390x... does that imply that we > never support irqfd for pci anyway? > >> >> In fact, if we also verify the AIS availability in the same routine we >> can avoid the ugly include of vfio/pci.h > > If we can get the serialization right (and setting that value to false > does indeed have the desired effect), that's probably the way to go. > OK, thanks, I give it a try and propose a v2 on this basis.
On 11/10/2017 14:20, Cornelia Huck wrote: > On Tue, 10 Oct 2017 18:01:53 +0200 > Pierre Morel <pmorel@linux.vnet.ibm.com> wrote: > >> Since in the case we have no AIS in KVM only PCI is impacted, do you >> think we can fence IRQFD for MSI with >> >> kvm_msi_via_irqfd_allowed = false; >> >> in kvm_arch_init_irq_routing() ? > > Hm, it seems we never set that for s390x... does that imply that we > never support irqfd for pci anyway? ... yes, we never supported irqfd for virtio-pci anyway. This explains a lot of things to me. :) > >> >> In fact, if we also verify the AIS availability in the same routine we >> can avoid the ugly include of vfio/pci.h > > If we can get the serialization right (and setting that value to false > does indeed have the desired effect), that's probably the way to go. > So now, in v2, we will be able to **enable** irqfd if we detect that we handle AIS in kernel. :)
On Thu, 12 Oct 2017 16:48:26 +0200 Pierre Morel <pmorel@linux.vnet.ibm.com> wrote: > On 11/10/2017 14:20, Cornelia Huck wrote: > > On Tue, 10 Oct 2017 18:01:53 +0200 > > Pierre Morel <pmorel@linux.vnet.ibm.com> wrote: > > > >> Since in the case we have no AIS in KVM only PCI is impacted, do you > >> think we can fence IRQFD for MSI with > >> > >> kvm_msi_via_irqfd_allowed = false; > >> > >> in kvm_arch_init_irq_routing() ? > > > > Hm, it seems we never set that for s390x... does that imply that we > > never support irqfd for pci anyway? > > ... yes, we never supported irqfd for virtio-pci anyway. > This explains a lot of things to me. > :) :D > > > > >> > >> In fact, if we also verify the AIS availability in the same routine we > >> can avoid the ugly include of vfio/pci.h > > > > If we can get the serialization right (and setting that value to false > > does indeed have the desired effect), that's probably the way to go. > > > > So now, in v2, we will be able to **enable** irqfd if we detect that we > handle AIS in kernel. :) Let's see how that works out :)
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 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) && + (!fs || !fs->ais_supported)) { + error_setg(errp, "VFIO PCI is not supported " + "because kernel has no AIS capability."); + return; + } pbdev->fh |= FH_SHM_VFIO; } else { pbdev->fh |= FH_SHM_EMUL;
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 <pmorel@linux.vnet.ibm.com> --- hw/s390x/s390-pci-bus.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)