[PATCH] KVM: arm/arm64: Do not rely on LR state to guess EOI MI status

[Marc Zyngier]

We so far rely on the LR state to decide whether the guest has
EOI'd a level interrupt or not. While this looks like a good
idea on the surface, it leads to a couple of annoying corner
cases:

Example 1: (P = Pending, A = Active, MI = Maintenance Interrupt)
P -> guest IAR -> A -> exit/entry -> P+A -> guest EOI -> P -> MI

The state is now pending, we've really EOI'd the interrupt, and
yet lr_signals_eoi_mi() returns false, since the state is not 0.
The result is that we won't signal anything on the corresponding
irqfd, which people complain about. Meh.

Example 2:
P+A -> guest EOI -> P -> delayed MI -> guest IAR -> A -> MI fires

Same issue: state isn't 0, and nothing happens.

The core of the problem is that we can't decide on whether an
interrupt has been EOId by just looking at the LR if we ever
want to support the P+A state, as things do change behind our back.

An alternative to dropping P+A is to bring back our friend EISR,
which indicates which LRs have generated a MI. Instead of dragging
the state around like we used to do, use it to clear the EOI bit
from the in-memory copy, and use that as a predicate to find out
if it fired or not.

Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
---
 virt/kvm/arm/hyp/vgic-v2-sr.c | 8 ++++++++
 virt/kvm/arm/hyp/vgic-v3-sr.c | 6 ++++++
 virt/kvm/arm/vgic/vgic-v2.c   | 3 +--
 virt/kvm/arm/vgic/vgic-v3.c   | 3 +--
 4 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/virt/kvm/arm/hyp/vgic-v2-sr.c b/virt/kvm/arm/hyp/vgic-v2-sr.c
index 4fe6e797e8b3..475cb2d7fd33 100644
--- a/virt/kvm/arm/hyp/vgic-v2-sr.c
+++ b/virt/kvm/arm/hyp/vgic-v2-sr.c
@@ -43,6 +43,11 @@ static void __hyp_text save_lrs(struct kvm_vcpu *vcpu, void __iomem *base)
 	struct vgic_v2_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v2;
 	int i;
 	u64 used_lrs = vcpu->arch.vgic_cpu.used_lrs;
+	u64 eisr;
+
+	eisr = readl_relaxed(base + GICH_EISR0);
+	if (unlikely(used_lrs > 32))
+		eisr |= (u64)readl_relaxed(base + GICH_EISR1) << 32;
 
 	for (i = 0; i < used_lrs; i++) {
 		if (cpu_if->vgic_elrsr & (1UL << i))
@@ -50,6 +55,9 @@ static void __hyp_text save_lrs(struct kvm_vcpu *vcpu, void __iomem *base)
 		else
 			cpu_if->vgic_lr[i] = readl_relaxed(base + GICH_LR0 + (i * 4));
 
+		if ((cpu_if->vgic_lr[i] & GICH_LR_EOI) && !(eisr & (1UL << i)))
+			cpu_if->vgic_lr[i] &= ~GICH_LR_EOI;
+
 		writel_relaxed(0, base + GICH_LR0 + (i * 4));
 	}
 }
diff --git a/virt/kvm/arm/hyp/vgic-v3-sr.c b/virt/kvm/arm/hyp/vgic-v3-sr.c
index b89ce5432214..2ce63d6740b0 100644
--- a/virt/kvm/arm/hyp/vgic-v3-sr.c
+++ b/virt/kvm/arm/hyp/vgic-v3-sr.c
@@ -223,8 +223,10 @@ void __hyp_text __vgic_v3_save_state(struct kvm_vcpu *vcpu)
 	if (used_lrs) {
 		int i;
 		u32 nr_pre_bits;
+		u32 eisr;
 
 		cpu_if->vgic_elrsr = read_gicreg(ICH_ELSR_EL2);
+		eisr = read_gicreg(ICH_EISR_EL2);
 
 		write_gicreg(0, ICH_HCR_EL2);
 		val = read_gicreg(ICH_VTR_EL2);
@@ -236,6 +238,10 @@ void __hyp_text __vgic_v3_save_state(struct kvm_vcpu *vcpu)
 			else
 				cpu_if->vgic_lr[i] = __gic_v3_get_lr(i);
 
+			if ((cpu_if->vgic_lr[i] & ICH_LR_EOI) &&
+			    !(eisr & (1 << i)))
+				cpu_if->vgic_lr[i] &= ~ICH_LR_EOI;
+
 			__gic_v3_set_lr(0, i);
 		}
 
diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c
index e9d840a75e7b..0be616e4ee29 100644
--- a/virt/kvm/arm/vgic/vgic-v2.c
+++ b/virt/kvm/arm/vgic/vgic-v2.c
@@ -46,8 +46,7 @@ void vgic_v2_set_underflow(struct kvm_vcpu *vcpu)
 
 static bool lr_signals_eoi_mi(u32 lr_val)
 {
-	return !(lr_val & GICH_LR_STATE) && (lr_val & GICH_LR_EOI) &&
-	       !(lr_val & GICH_LR_HW);
+	return (lr_val & GICH_LR_EOI) && !(lr_val & GICH_LR_HW);
 }
 
 /*
diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
index 6b329414e57a..c68352b8ed28 100644
--- a/virt/kvm/arm/vgic/vgic-v3.c
+++ b/virt/kvm/arm/vgic/vgic-v3.c
@@ -35,8 +35,7 @@ void vgic_v3_set_underflow(struct kvm_vcpu *vcpu)
 
 static bool lr_signals_eoi_mi(u64 lr_val)
 {
-	return !(lr_val & ICH_LR_STATE) && (lr_val & ICH_LR_EOI) &&
-	       !(lr_val & ICH_LR_HW);
+	return (lr_val & ICH_LR_EOI) && !(lr_val & ICH_LR_HW);
 }
 
 void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)
-- 
2.14.2
-- 
Jazz is not dead. It just smells funny...