diff mbox series

[RFC,18/21] kvm/ioapic: mark gsi-2 used in ioapic routing init

Message ID 20221205173137.607044-19-dwmw2@infradead.org
State New
Headers show
Series Xen HVM support under KVM | expand

Commit Message

David Woodhouse Dec. 5, 2022, 5:31 p.m. UTC 
From: Ankur Arora <ankur.a.arora@oracle.com>

GSI-2/IOAPIC pin-2 is treated specially while initing
IRQ routing: PIC does not use it at all while the IOAPIC
maps virq=0 to pin-2 and does not use GSI-2.
(all other GSIs are identity mapped to pins.)

This results in any later code which allocates a virq
to be assigned GSI-2. This virq is in-turn used to
remap interrupts to HYPERVISOR_CALLBACK_VECTOR (0xf3)
to deliver to the guest.

Ordinarily this would be okay, but if the event delivery is
via direct injection via KVM_REQ_EVENT (without going
through the LAPIC) we see vmentry failure.

This works fine for any other values of GSI.

As a workaround, mark GSI-2 used.

Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 accel/kvm/kvm-all.c  | 5 +++++
 hw/i386/kvm/ioapic.c | 1 +
 include/sysemu/kvm.h | 1 +
 3 files changed, 7 insertions(+)

Comments

Philippe Mathieu-Daudé Dec. 5, 2022, 10:25 p.m. UTC | #1 
On 5/12/22 18:31, David Woodhouse wrote:
> From: Ankur Arora <ankur.a.arora@oracle.com>
> 
> GSI-2/IOAPIC pin-2 is treated specially while initing
> IRQ routing: PIC does not use it at all while the IOAPIC
> maps virq=0 to pin-2 and does not use GSI-2.
> (all other GSIs are identity mapped to pins.)
> 
> This results in any later code which allocates a virq
> to be assigned GSI-2. This virq is in-turn used to
> remap interrupts to HYPERVISOR_CALLBACK_VECTOR (0xf3)
> to deliver to the guest.
> 
> Ordinarily this would be okay, but if the event delivery is
> via direct injection via KVM_REQ_EVENT (without going
> through the LAPIC) we see vmentry failure.
> 
> This works fine for any other values of GSI.
> 
> As a workaround, mark GSI-2 used.
> 
> Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
>   accel/kvm/kvm-all.c  | 5 +++++
>   hw/i386/kvm/ioapic.c | 1 +
>   include/sysemu/kvm.h | 1 +
>   3 files changed, 7 insertions(+)


> diff --git a/hw/i386/kvm/ioapic.c b/hw/i386/kvm/ioapic.c
> index ee7c8ef68b..5fab0d35c9 100644
> --- a/hw/i386/kvm/ioapic.c
> +++ b/hw/i386/kvm/ioapic.c
> @@ -43,6 +43,7 @@ void kvm_pc_setup_irq_routing(bool pci_enabled)
>               }
>           }
>       }

Workarounds usually deserve some comment in the code.

> +    kvm_irqchip_set_gsi(s, 2);
>       kvm_irqchip_commit_routes(s);
>   }
David Woodhouse Dec. 6, 2022, 1:21 a.m. UTC | #2 
On Mon, 2022-12-05 at 23:25 +0100, Philippe Mathieu-Daudé wrote:
> On 5/12/22 18:31, David Woodhouse wrote:
> > From: Ankur Arora <
> > ankur.a.arora@oracle.com
> > >
> > 
> > GSI-2/IOAPIC pin-2 is treated specially while initing
> > IRQ routing: PIC does not use it at all while the IOAPIC
> > maps virq=0 to pin-2 and does not use GSI-2.
> > (all other GSIs are identity mapped to pins.)
> > 
> > This results in any later code which allocates a virq
> > to be assigned GSI-2. This virq is in-turn used to
> > remap interrupts to HYPERVISOR_CALLBACK_VECTOR (0xf3)
> > to deliver to the guest.
> > 
> > Ordinarily this would be okay, but if the event delivery is
> > via direct injection via KVM_REQ_EVENT (without going
> > through the LAPIC) we see vmentry failure.
> > 
> > This works fine for any other values of GSI.
> > 
> > As a workaround, mark GSI-2 used.
> > 
> > Signed-off-by: Ankur Arora <ankur.a.arora@oracle.com>
> > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> > ---
> >   accel/kvm/kvm-all.c  | 5 +++++
> >   hw/i386/kvm/ioapic.c | 1 +
> >   include/sysemu/kvm.h | 1 +
> >   3 files changed, 7 insertions(+)
> 
> 
> > diff --git a/hw/i386/kvm/ioapic.c b/hw/i386/kvm/ioapic.c
> > index ee7c8ef68b..5fab0d35c9 100644
> > --- a/hw/i386/kvm/ioapic.c
> > +++ b/hw/i386/kvm/ioapic.c
> > @@ -43,6 +43,7 @@ void kvm_pc_setup_irq_routing(bool pci_enabled)
> >               }
> >           }
> >       }
> 
> Workarounds usually deserve some comment in the code.

Yes, good point. Although I actually think I can kill this off
completely since we no longer attempt the deliver the vector directly
with KVM_REQ_EVENT anyway; the kernel injects it *for* us when it sees
that vcpu_info->evtchn_upcall_pending is set on entry to the guest
vCPU.

> > +    kvm_irqchip_set_gsi(s, 2);
> >       kvm_irqchip_commit_routes(s);
> >   }
diff mbox series

Patch

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 8a227515b7..b40cfc4144 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -1677,6 +1677,11 @@  static void set_gsi(KVMState *s, unsigned int gsi)
     set_bit(gsi, s->used_gsi_bitmap);
 }
 
+void kvm_irqchip_set_gsi(KVMState *s, unsigned int gsi)
+{
+    set_gsi(s, gsi);
+}
+
 static void clear_gsi(KVMState *s, unsigned int gsi)
 {
     clear_bit(gsi, s->used_gsi_bitmap);
diff --git a/hw/i386/kvm/ioapic.c b/hw/i386/kvm/ioapic.c
index ee7c8ef68b..5fab0d35c9 100644
--- a/hw/i386/kvm/ioapic.c
+++ b/hw/i386/kvm/ioapic.c
@@ -43,6 +43,7 @@  void kvm_pc_setup_irq_routing(bool pci_enabled)
             }
         }
     }
+    kvm_irqchip_set_gsi(s, 2);
     kvm_irqchip_commit_routes(s);
 }
 
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 8e882fbe96..a249ea480f 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -512,6 +512,7 @@  static inline void kvm_irqchip_commit_route_changes(KVMRouteChange *c)
 }
 
 void kvm_irqchip_release_virq(KVMState *s, int virq);
+void kvm_irqchip_set_gsi(KVMState *s, unsigned int gsi);
 
 int kvm_irqchip_add_adapter_route(KVMState *s, AdapterInfo *adapter);
 int kvm_irqchip_add_hv_sint_route(KVMState *s, uint32_t vcpu, uint32_t sint);