[Qemu-devel] [PATCH for-1.3] vfio-pci: Fix KVM disabled path

[Qemu-devel] [PATCH for-1.3] vfio-pci: Fix KVM disabled path

[Alex Williamson]

kvm_check_extension() explodes when KVM isn't enabled so we need to
first test whether KVM is enabled.  Use kvm_irqchip_in_kernel() for
this since it matches the test we do before using this result.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/vfio_pci.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Anthony, please include this for 1.3.  Let me know if you prefer a
pull request.  Thanks,

Alex

diff --git a/hw/vfio_pci.c b/hw/vfio_pci.c
index 7c27834..2acc757 100644
--- a/hw/vfio_pci.c
+++ b/hw/vfio_pci.c
@@ -438,7 +438,8 @@ static int vfio_enable_intx(VFIODevice *vdev)
      * Only conditional to avoid generating error messages on platforms
      * where we won't actually use the result anyway.
      */
-    if (kvm_check_extension(kvm_state, KVM_CAP_IRQFD_RESAMPLE)) {
+    if (kvm_irqchip_in_kernel() &&
+        kvm_check_extension(kvm_state, KVM_CAP_IRQFD_RESAMPLE)) {
         vdev->intx.route = pci_device_route_intx_to_irq(&vdev->pdev,
                                                         vdev->intx.pin);
     }

Re: [Qemu-devel] [PATCH for-1.3] vfio-pci: Fix KVM disabled path

[Peter Maydell]

On 27 November 2012 19:45, Alex Williamson <alex.williamson@redhat.com> wrote:
> kvm_check_extension() explodes when KVM isn't enabled so we need to
> first test whether KVM is enabled.  Use kvm_irqchip_in_kernel() for
> this since it matches the test we do before using this result.

> --- a/hw/vfio_pci.c
> +++ b/hw/vfio_pci.c
> @@ -438,7 +438,8 @@ static int vfio_enable_intx(VFIODevice *vdev)
>       * Only conditional to avoid generating error messages on platforms
>       * where we won't actually use the result anyway.
>       */
> -    if (kvm_check_extension(kvm_state, KVM_CAP_IRQFD_RESAMPLE)) {
> +    if (kvm_irqchip_in_kernel() &&
> +        kvm_check_extension(kvm_state, KVM_CAP_IRQFD_RESAMPLE)) {
>          vdev->intx.route = pci_device_route_intx_to_irq(&vdev->pdev,
>                                                          vdev->intx.pin);


vfio_pci is supposed to be architecture-independent code, right?
kvm.h says you mustn't use kvm_irqchip_in_kernel() in such code
(its semantics vary from arch to arch). I think kvm_enabled()
is sufficient here since what we're actually guarding is just
the kvm_check_extension call.

-- PMM

Re: [Qemu-devel] [PATCH for-1.3] vfio-pci: Fix KVM disabled path

[Alex Williamson]

On Fri, 2012-11-30 at 18:57 +0000, Peter Maydell wrote:
> On 27 November 2012 19:45, Alex Williamson <alex.williamson@redhat.com> wrote:
> > kvm_check_extension() explodes when KVM isn't enabled so we need to
> > first test whether KVM is enabled.  Use kvm_irqchip_in_kernel() for
> > this since it matches the test we do before using this result.
> 
> > --- a/hw/vfio_pci.c
> > +++ b/hw/vfio_pci.c
> > @@ -438,7 +438,8 @@ static int vfio_enable_intx(VFIODevice *vdev)
> >       * Only conditional to avoid generating error messages on platforms
> >       * where we won't actually use the result anyway.
> >       */
> > -    if (kvm_check_extension(kvm_state, KVM_CAP_IRQFD_RESAMPLE)) {
> > +    if (kvm_irqchip_in_kernel() &&
> > +        kvm_check_extension(kvm_state, KVM_CAP_IRQFD_RESAMPLE)) {
> >          vdev->intx.route = pci_device_route_intx_to_irq(&vdev->pdev,
> >                                                          vdev->intx.pin);
> 
> 
> vfio_pci is supposed to be architecture-independent code, right?
> kvm.h says you mustn't use kvm_irqchip_in_kernel() in such code
> (its semantics vary from arch to arch). I think kvm_enabled()
> is sufficient here since what we're actually guarding is just
> the kvm_check_extension call.

Here, yes.  The other call spot we're matching is specifically looking
for whether kvm is capable of using IRQFD_RESAMPLE and whether it should
be used.  I think kvm_irqfds_enabled() is probably the right way to go.
Post 1.3 we might want to make kvm_intx_via_irqfd_enabled which
encapsulates the IRQFD_RESAMPLE test.  I'll do some testing and repost a
kvm_irqfds_enabled() version.  Thanks,

Alex