[PATCH v1 0/1] hw/intc/arm_gic: Fix deactivation of SPI lines

[PATCH v1 0/1] hw/intc/arm_gic: Fix deactivation of SPI lines

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>

Julien reported that he has seen strange behaviour when running
Xen on QEMU using GICv2. When Xen migrates a guest's vCPU to
another pCPU while the vCPU is handling an interrupt the guest
is unable to properly deactivate interrupts.

It sounds like something rare but in some setups it actually
happens all the time.

Looking at it a little closer, our GICv2 model treats
deactivation of SPI lines as if they were PPI's, i.e banked per
CPU core. The state for active interrupts should only be banked
for PPI lines, not for SPI lines.

When deactivating SPI lines, I think we need to handle the state
as unbanked, similar to how we handle writes to GICD_ICACTIVER.

This fixes the problem on my side.

Cheers,
Edgar


Edgar E. Iglesias (1):
  hw/intc/arm_gic: Fix deactivation of SPI lines

 hw/intc/gic_internal.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)


base-commit: d16cab541ab9217977e2a39abf3d79f914146741
-- 
2.40.1

[PATCH v1 1/1] hw/intc/arm_gic: Fix deactivation of SPI lines

[Edgar E. Iglesias]

From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>

Julien reported that he has seen strange behaviour when running
Xen on QEMU using GICv2. When Xen migrates a guest's vCPU from
one pCPU to another while the vCPU is handling an interrupt, the
guest is unable to properly deactivate interrupts.

Looking at it a little closer, our GICv2 model treats
deactivation of SPI lines as if they were PPI's, i.e banked per
CPU core. The state for active interrupts should only be banked
for PPI lines, not for SPI lines.

Make deactivation of SPI lines unbanked, similar to how we
handle writes to GICD_ICACTIVER.

Reported-by: Julien Grall <julien@xen.org>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
---
 hw/intc/gic_internal.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/intc/gic_internal.h b/hw/intc/gic_internal.h
index 8d29b40ca1..8ddbf554c6 100644
--- a/hw/intc/gic_internal.h
+++ b/hw/intc/gic_internal.h
@@ -280,6 +280,8 @@ static inline void gic_set_active(GICState *s, int irq, int cpu)
 
 static inline void gic_clear_active(GICState *s, int irq, int cpu)
 {
+    unsigned int cm;
+
     if (gic_is_vcpu(cpu)) {
         uint32_t *entry = gic_get_lr_entry(s, irq, cpu);
         GICH_LR_CLEAR_ACTIVE(*entry);
@@ -301,11 +303,13 @@ static inline void gic_clear_active(GICState *s, int irq, int cpu)
              * the GIC is secure.
              */
             if (!s->security_extn || GIC_DIST_TEST_GROUP(phys_irq, 1 << rcpu)) {
-                GIC_DIST_CLEAR_ACTIVE(phys_irq, 1 << rcpu);
+                cm = phys_irq < GIC_INTERNAL ? 1 << rcpu : ALL_CPU_MASK;
+                GIC_DIST_CLEAR_ACTIVE(phys_irq, cm);
             }
         }
     } else {
-        GIC_DIST_CLEAR_ACTIVE(irq, 1 << cpu);
+        cm = irq < GIC_INTERNAL ? 1 << cpu : ALL_CPU_MASK;
+        GIC_DIST_CLEAR_ACTIVE(irq, cm);
     }
 }
 
-- 
2.40.1

Re: [PATCH v1 0/1] hw/intc/arm_gic: Fix deactivation of SPI lines

[Peter Maydell]

On Wed, 5 Jun 2024 at 15:43, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
>
> From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
>
> Julien reported that he has seen strange behaviour when running
> Xen on QEMU using GICv2. When Xen migrates a guest's vCPU to
> another pCPU while the vCPU is handling an interrupt the guest
> is unable to properly deactivate interrupts.
>
> It sounds like something rare but in some setups it actually
> happens all the time.
>
> Looking at it a little closer, our GICv2 model treats
> deactivation of SPI lines as if they were PPI's, i.e banked per
> CPU core. The state for active interrupts should only be banked
> for PPI lines, not for SPI lines.
>
> When deactivating SPI lines, I think we need to handle the state
> as unbanked, similar to how we handle writes to GICD_ICACTIVER.
>
> This fixes the problem on my side.

Applied to target-arm.next, thanks.

(I'm surprised anybody's still using GICv2 seriously at
this point...)

-- PMM