arm/arm64: KVM: Keep elrsr/aisr in sync with software model

commit ae705930fca6322600690df9dc1c7d0516145a93 upstream.

There is an interesting bug in the vgic code, which manifests itself
when the KVM run loop has a signal pending or needs a vmid generation
rollover after having disabled interrupts but before actually switching
to the guest.

In this case, we flush the vgic as usual, but we sync back the vgic
state and exit to userspace before entering the guest.  The consequence
is that we will be syncing the list registers back to the software model
using the GICH_ELRSR and GICH_EISR from the last execution of the guest,
potentially overwriting a list register containing an interrupt.

This showed up during migration testing where we would capture a state
where the VM has masked the arch timer but there were no interrupts,
resulting in a hung test.

Cc: Marc Zyngier <marc.zyngier@arm.com>
Reported-by: Alex Bennee <alex.bennee@linaro.org>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index ac4888dc86bcb51d802cb08d1795c60d9c4180d3..36e0f219e5070d8ce45d36a5e5bd77e706b8443e 100644 (file)
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -113,6 +113,7 @@ struct vgic_ops {
        void    (*sync_lr_elrsr)(struct kvm_vcpu *, int, struct vgic_lr);
        u64     (*get_elrsr)(const struct kvm_vcpu *vcpu);
        u64     (*get_eisr)(const struct kvm_vcpu *vcpu);
+       void    (*clear_eisr)(struct kvm_vcpu *vcpu);
        u32     (*get_interrupt_status)(const struct kvm_vcpu *vcpu);
        void    (*enable_underflow)(struct kvm_vcpu *vcpu);
        void    (*disable_underflow)(struct kvm_vcpu *vcpu);

diff --git a/virt/kvm/arm/vgic-v2.c b/virt/kvm/arm/vgic-v2.c
index 2935405ad22f6ce7f1aaa1239653c3c37224a065..b9d48e8e1eb4fb2db999ef4dbeb5db7b1a273f6e 100644 (file)
--- a/virt/kvm/arm/vgic-v2.c
+++ b/virt/kvm/arm/vgic-v2.c
@@ -72,6 +72,8 @@ static void vgic_v2_sync_lr_elrsr(struct kvm_vcpu *vcpu, int lr,
 {
        if (!(lr_desc.state & LR_STATE_MASK))
                vcpu->arch.vgic_cpu.vgic_v2.vgic_elrsr |= (1ULL << lr);
+       else
+               vcpu->arch.vgic_cpu.vgic_v2.vgic_elrsr &= ~(1ULL << lr);
 }
 
 static u64 vgic_v2_get_elrsr(const struct kvm_vcpu *vcpu)
@@ -84,6 +86,11 @@ static u64 vgic_v2_get_eisr(const struct kvm_vcpu *vcpu)
        return vcpu->arch.vgic_cpu.vgic_v2.vgic_eisr;
 }
 
+static void vgic_v2_clear_eisr(struct kvm_vcpu *vcpu)
+{
+       vcpu->arch.vgic_cpu.vgic_v2.vgic_eisr = 0;
+}
+
 static u32 vgic_v2_get_interrupt_status(const struct kvm_vcpu *vcpu)
 {
        u32 misr = vcpu->arch.vgic_cpu.vgic_v2.vgic_misr;
@@ -148,6 +155,7 @@ static const struct vgic_ops vgic_v2_ops = {
        .sync_lr_elrsr          = vgic_v2_sync_lr_elrsr,
        .get_elrsr              = vgic_v2_get_elrsr,
        .get_eisr               = vgic_v2_get_eisr,
+       .clear_eisr             = vgic_v2_clear_eisr,
        .get_interrupt_status   = vgic_v2_get_interrupt_status,
        .enable_underflow       = vgic_v2_enable_underflow,
        .disable_underflow      = vgic_v2_disable_underflow,

diff --git a/virt/kvm/arm/vgic-v3.c b/virt/kvm/arm/vgic-v3.c
index 1c2c8eef05993cac943cb90db058dda87d37434e..58b8af00ee4c9fe12ef808f3854f4606a95c7270 100644 (file)
--- a/virt/kvm/arm/vgic-v3.c
+++ b/virt/kvm/arm/vgic-v3.c
@@ -86,6 +86,8 @@ static void vgic_v3_sync_lr_elrsr(struct kvm_vcpu *vcpu, int lr,
 {
        if (!(lr_desc.state & LR_STATE_MASK))
                vcpu->arch.vgic_cpu.vgic_v3.vgic_elrsr |= (1U << lr);
+       else
+               vcpu->arch.vgic_cpu.vgic_v3.vgic_elrsr &= ~(1U << lr);
 }
 
 static u64 vgic_v3_get_elrsr(const struct kvm_vcpu *vcpu)
@@ -98,6 +100,11 @@ static u64 vgic_v3_get_eisr(const struct kvm_vcpu *vcpu)
        return vcpu->arch.vgic_cpu.vgic_v3.vgic_eisr;
 }
 
+static void vgic_v3_clear_eisr(struct kvm_vcpu *vcpu)
+{
+       vcpu->arch.vgic_cpu.vgic_v3.vgic_eisr = 0;
+}
+
 static u32 vgic_v3_get_interrupt_status(const struct kvm_vcpu *vcpu)
 {
        u32 misr = vcpu->arch.vgic_cpu.vgic_v3.vgic_misr;
@@ -162,6 +169,7 @@ static const struct vgic_ops vgic_v3_ops = {
        .sync_lr_elrsr          = vgic_v3_sync_lr_elrsr,
        .get_elrsr              = vgic_v3_get_elrsr,
        .get_eisr               = vgic_v3_get_eisr,
+       .clear_eisr             = vgic_v3_clear_eisr,
        .get_interrupt_status   = vgic_v3_get_interrupt_status,
        .enable_underflow       = vgic_v3_enable_underflow,
        .disable_underflow      = vgic_v3_disable_underflow,

diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 680e4107352a6fe2876db4404f857f9994315ba1..57a16f4836d0697afcf560c0ff2550c4f4d3301c 100644 (file)
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -1219,6 +1219,11 @@ static inline u64 vgic_get_eisr(struct kvm_vcpu *vcpu)
        return vgic_ops->get_eisr(vcpu);
 }
 
+static inline void vgic_clear_eisr(struct kvm_vcpu *vcpu)
+{
+       vgic_ops->clear_eisr(vcpu);
+}
+
 static inline u32 vgic_get_interrupt_status(struct kvm_vcpu *vcpu)
 {
        return vgic_ops->get_interrupt_status(vcpu);
@@ -1258,6 +1263,7 @@ static void vgic_retire_lr(int lr_nr, int irq, struct kvm_vcpu *vcpu)
        vgic_set_lr(vcpu, lr_nr, vlr);
        clear_bit(lr_nr, vgic_cpu->lr_used);
        vgic_cpu->vgic_irq_lr_map[irq] = LR_EMPTY;
+       vgic_sync_lr_elrsr(vcpu, lr_nr, vlr);
 }
 
 /*
@@ -1313,6 +1319,7 @@ static bool vgic_queue_irq(struct kvm_vcpu *vcpu, u8 sgi_source_id, int irq)
                        BUG_ON(!test_bit(lr, vgic_cpu->lr_used));
                        vlr.state |= LR_STATE_PENDING;
                        vgic_set_lr(vcpu, lr, vlr);
+                       vgic_sync_lr_elrsr(vcpu, lr, vlr);
                        return true;
                }
        }
@@ -1334,6 +1341,7 @@ static bool vgic_queue_irq(struct kvm_vcpu *vcpu, u8 sgi_source_id, int irq)
                vlr.state |= LR_EOI_INT;
 
        vgic_set_lr(vcpu, lr, vlr);
+       vgic_sync_lr_elrsr(vcpu, lr, vlr);
 
        return true;
 }
@@ -1502,6 +1510,14 @@ static bool vgic_process_maintenance(struct kvm_vcpu *vcpu)
        if (status & INT_STATUS_UNDERFLOW)
                vgic_disable_underflow(vcpu);
 
+       /*
+        * In the next iterations of the vcpu loop, if we sync the vgic state
+        * after flushing it, but before entering the guest (this happens for
+        * pending signals and vmid rollovers), then make sure we don't pick
+        * up any old maintenance interrupts here.
+        */
+       vgic_clear_eisr(vcpu);
+
        return level_pending;
 }