[RFC PATCH 0/3] KVM: arm64: Preserve level IRQ re-pending

[RFC PATCH 0/3] KVM: arm64: Preserve level IRQ re-pending

[Karl Mehltretter]

A guest can set a level interrupt pending again while handling it. The
GIC records Active and Pending and delivers the interrupt after
deactivation. KVM instead drops the second pending state for both the
GICv2 and GICv3 backends.

The VGIC keeps software pending state outside the LR because commit
67b5b673ad4d stopped placing Pending and Active together for level
interrupts. When the LR is folded, KVM cannot distinguish the pending
state used to populate the LR from a later ISPENDR write. This series
records the later write and preserves it across deactivation. The last
patch adds a two-vCPU GICv3 selftest.

The awkward case, and the reason for the RFC, is a write from another
vCPU while the interrupt remains in an LR. KVM cannot know whether the
write preceded or followed the guest ack. This series preserves the
write. A pre-ack write can therefore cause one extra delivery, while a
post-ack write is no longer lost.

The exact alternative is to stop and synchronize the target vCPU
before applying such a write, as the active-state accessors do. Is the
conservative replay acceptable here, or should ISPENDR pay that
synchronization cost?

The issue was reproduced with Zephyr, NuttX and RT-Thread guests. On a
Raspberry Pi 400 host with a GIC-400, each GICv2 guest handled the SPI
once before the fix and twice afterwards. A direct GIC-400 register
test also delivered the interrupt twice. With a QEMU TCG GICv3 host
running KVM, the new selftest failed before the fix and passed
afterwards. Existing vgic_irq and kvm-unit-tests GIC cases passed on
both versions. Those runtime tests used 8623551a424d.

The series applies without conflicts to kvmarm/next at dd0bc00e3e92.
The modified VGIC objects pass an arm64 GCC 14 W=1 build there, and the
selftest builds natively on arm64. GICv5 does not use the LR path
changed here. I have not tested a mapped physical interrupt or VFIO.

Karl Mehltretter (3):
  KVM: arm64: vgic-v2: Don't drop pending writes to active level IRQs
  KVM: arm64: vgic-v3: Don't drop pending writes to active level IRQs
  KVM: selftests: arm64: Test pending writes to active level IRQs

 arch/arm64/kvm/vgic/vgic-mmio.c              |   9 ++
 arch/arm64/kvm/vgic/vgic-v2.c                |  13 ++-
 arch/arm64/kvm/vgic/vgic-v3.c                |  13 ++-
 include/kvm/arm_vgic.h                       |   1 +
 tools/testing/selftests/kvm/arm64/vgic_irq.c | 101 +++++++++++++++++++
 5 files changed, 131 insertions(+), 6 deletions(-)


base-commit: dd0bc00e3e9201fd49b62ff9cc07890c89003afe
-- 
2.39.5 (Apple Git-154)

[RFC PATCH 1/3] KVM: arm64: vgic-v2: Don't drop pending writes to active level IRQs

[Karl Mehltretter]

A write to GICD_ISPENDR for an active level interrupt makes the
interrupt Active and Pending. It must be delivered again after it is
deactivated.

KVM loses the second pending state. vgic_v2_populate_lr() leaves
pending_latch set for a level interrupt, while vgic_v2_compute_lr()
keeps Pending and Active out of the same LR. vgic_v2_fold_lr() clears
the latch after the LR becomes empty. At that point it cannot tell the
original latch from one written while the interrupt was active, and
clears both.

Clear the original latch when the LR first reports no pending state.
Keep a latch which was not represented in the LR, either because the
interrupt was already active or because the guest wrote ISPENDR while
the interrupt was in an LR. Clear the marker together with the latch
on an ICPENDR write.

A remote ISPENDR write can race with the guest ack. KVM cannot tell
which happened first without synchronizing the target vCPU. Preserve
the write, which may cause one extra delivery if it preceded the ack
but avoids losing a write that followed it.

Fixes: 67b5b673ad4d ("KVM: arm/arm64: vgic: Disallow Active+Pending for level interrupts")
Assisted-by: LLM
---
 arch/arm64/kvm/vgic/vgic-mmio.c |  9 +++++++++
 arch/arm64/kvm/vgic/vgic-v2.c   | 13 ++++++++++---
 include/kvm/arm_vgic.h          |  1 +
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-mmio.c b/arch/arm64/kvm/vgic/vgic-mmio.c
index fddb9da0403d5..1728c78f99cfd 100644
--- a/arch/arm64/kvm/vgic/vgic-mmio.c
+++ b/arch/arm64/kvm/vgic/vgic-mmio.c
@@ -341,6 +341,13 @@ static void __set_pending(struct kvm_vcpu *vcpu, gpa_t addr, unsigned int len,
 			continue;
 		}
 
+		/*
+		 * The guest may already have acked what is in the LR,
+		 * in which case this is a new pending state.
+		 */
+		if (irq->on_lr)
+			irq->latch_written = true;
+
 		irq->pending_latch = true;
 		if (irq->hw && !is_user)
 			vgic_irq_set_phys_active(irq, true);
@@ -427,6 +434,8 @@ static void __clear_pending(struct kvm_vcpu *vcpu,
 			continue;
 		}
 
+		irq->latch_written = false;
+
 		if (irq->hw && !is_user)
 			vgic_hw_irq_cpending(vcpu, irq);
 		else
diff --git a/arch/arm64/kvm/vgic/vgic-v2.c b/arch/arm64/kvm/vgic/vgic-v2.c
index 7182f63fc9382..37474ea2431c5 100644
--- a/arch/arm64/kvm/vgic/vgic-v2.c
+++ b/arch/arm64/kvm/vgic/vgic-v2.c
@@ -57,7 +57,7 @@ static void vgic_v2_fold_lr(struct kvm_vcpu *vcpu, u32 val)
 {
 	u32 cpuid, intid = val & GICH_LR_VIRTUALID;
 	struct vgic_irq *irq;
-	bool deactivated;
+	bool deactivated, was_active;
 
 	/* Extract the source vCPU id from the LR */
 	cpuid = FIELD_GET(GICH_LR_PHYSID_CPUID, val) & 7;
@@ -71,7 +71,8 @@ static void vgic_v2_fold_lr(struct kvm_vcpu *vcpu, u32 val)
 
 	scoped_guard(raw_spinlock, &irq->irq_lock) {
 		/* Always preserve the active bit, note deactivation */
-		deactivated = irq->active && !(val & GICH_LR_ACTIVE_BIT);
+		was_active = irq->active;
+		deactivated = was_active && !(val & GICH_LR_ACTIVE_BIT);
 		irq->active = !!(val & GICH_LR_ACTIVE_BIT);
 
 		if (irq->active && vgic_irq_is_sgi(intid))
@@ -88,9 +89,15 @@ static void vgic_v2_fold_lr(struct kvm_vcpu *vcpu, u32 val)
 
 		/*
 		 * Clear soft pending state when level irqs have been acked.
+		 * A latch written while the irq was active is a new pending
+		 * state. A write while the irq was in the LR may also have
+		 * followed the ack, so preserve it too.
 		 */
-		if (irq->config == VGIC_CONFIG_LEVEL && !(val & GICH_LR_STATE))
+		if (irq->config == VGIC_CONFIG_LEVEL &&
+		    !(val & GICH_LR_PENDING_BIT) &&
+		    !was_active && !irq->latch_written)
 			irq->pending_latch = false;
+		irq->latch_written = false;
 
 		/* Handle resampling for mapped interrupts if required */
 		vgic_irq_handle_resampling(irq, deactivated, val & GICH_LR_PENDING_BIT);
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index 7be81b936bf10..28ed27d982dfc 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -292,6 +292,7 @@ struct vgic_irq {
 	bool active:1;
 	bool hw:1;			/* Tied to HW IRQ */
 	bool on_lr:1;			/* Present in a CPU LR */
+	bool latch_written:1;		/* Level only: latch set while on_lr */
 	refcount_t refcount;		/* Used for LPIs */
 	u32 hwintid;			/* HW INTID number */
 	unsigned int host_irq;		/* linux irq corresponding to hwintid */
-- 
2.39.5 (Apple Git-154)

Re: [RFC PATCH 1/3] KVM: arm64: vgic-v2: Don't drop pending writes to active level IRQs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] KVM: arm64: vgic-v2: unbacked GICH_LR_HW pending state after active ISPENDR
- [Medium] KVM: arm64: vgic-v2: preserved pending_latch on restore of active level IRQ
--

commit 559e1b17a5a169279b080f4ac07688f66063cf6a
Author: Karl Mehltretter <kmehltretter@gmail.com>

KVM: arm64: vgic-v2: Don't drop pending writes to active level IRQs

This commit preserves pending_latch in vgic_v2_fold_lr() when a
level-triggered interrupt was already active or had GICD_ISPENDR written while
present in a list register. It adds a latch_written flag to struct vgic_irq to
track pending writes that occur while irq->on_lr is set and clears it when
clearing pending state or folding list registers.

[ ... ]

> diff --git a/arch/arm64/kvm/vgic/vgic-v2.c b/arch/arm64/kvm/vgic/vgic-v2.c
> index 7182f63fc9382..37474ea2431c5 100644
> --- a/arch/arm64/kvm/vgic/vgic-v2.c
> +++ b/arch/arm64/kvm/vgic/vgic-v2.c
> @@ -57,7 +57,7 @@ static void vgic_v2_fold_lr(struct kvm_vcpu *vcpu, u32 val)
>  {
>  	u32 cpuid, intid = val & GICH_LR_VIRTUALID;
>  	struct vgic_irq *irq;
> -	bool deactivated;
> +	bool deactivated, was_active;
>  
>  	/* Extract the source vCPU id from the LR */
>  	cpuid = FIELD_GET(GICH_LR_PHYSID_CPUID, val) & 7;
> @@ -71,7 +71,8 @@ static void vgic_v2_fold_lr(struct kvm_vcpu *vcpu, u32 val)
>  
>  	scoped_guard(raw_spinlock, &irq->irq_lock) {
>  		/* Always preserve the active bit, note deactivation */
> -		deactivated = irq->active && !(val & GICH_LR_ACTIVE_BIT);
> +		was_active = irq->active;
> +		deactivated = was_active && !(val & GICH_LR_ACTIVE_BIT);
>  		irq->active = !!(val & GICH_LR_ACTIVE_BIT);
>  
>  		if (irq->active && vgic_irq_is_sgi(intid))
> @@ -88,9 +89,15 @@ static void vgic_v2_fold_lr(struct kvm_vcpu *vcpu, u32 val)
>  
>  		/*
>  		 * Clear soft pending state when level irqs have been acked.
> +		 * A latch written while the irq was active is a new pending
> +		 * state. A write while the irq was in the LR may also have
> +		 * followed the ack, so preserve it too.
>  		 */
> -		if (irq->config == VGIC_CONFIG_LEVEL && !(val & GICH_LR_STATE))
> +		if (irq->config == VGIC_CONFIG_LEVEL &&
> +		    !(val & GICH_LR_PENDING_BIT) &&
> +		    !was_active && !irq->latch_written)
>  			irq->pending_latch = false;

[Severity: Medium]
Could checking !was_active here cause a spurious second interrupt delivery
after saving and restoring a KVM_DEV_TYPE_ARM_VGIC_V2 guest with an active
level-triggered interrupt?

Unlike KVM_DEV_TYPE_ARM_VGIC_V3, which exposes KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO
and returns only irq->pending_latch to userspace in
arch/arm64/kvm/vgic/vgic-mmio.c:vgic_uaccess_read_pending()->__read_pending(),
KVM_DEV_TYPE_ARM_VGIC_V2 returns irq_is_pending(irq):

    switch (vcpu->kvm->arch.vgic.vgic_model) {
    case KVM_DEV_TYPE_ARM_VGIC_V3:
        if (is_user) {
            val = irq->pending_latch;
            break;
        }
        fallthrough;
    default:
        val = irq_is_pending(irq);
        break;
    }

When a GICv2 guest is saved while a level-triggered interrupt is active
(irq->active == true) and its device line is still asserted
(irq->line_level == true, irq->pending_latch == false), userspace reads 1 from
both GIC_DIST_ACTIVE_SET and GIC_DIST_PENDING_SET.

On restore, userspace writes 1 to GIC_DIST_ACTIVE_SET via
vgic_mmio_uaccess_write_sactive(), setting irq->active = true, and writes 1 to
GIC_DIST_PENDING_SET via vgic_uaccess_write_spending()->__set_pending(),
setting irq->pending_latch = true:

    int vgic_uaccess_write_spending(struct kvm_vcpu *vcpu,
                                    gpa_t addr, unsigned int len,
                                    unsigned long val)
    {
        __set_pending(vcpu, addr, len, val, true);
        return 0;
    }

Previously, once the guest cleared the device condition (lowering
irq->line_level) and deactivated the interrupt, vgic_v2_fold_lr() saw
!(val & GICH_LR_STATE) and cleared irq->pending_latch = false. With the
!was_active check in vgic_v2_fold_lr(), won't irq->pending_latch remain set
across the deactivation of the restored active interrupt and cause the
interrupt to be injected a second time?

> +		irq->latch_written = false;
>  
>  		/* Handle resampling for mapped interrupts if required */
>  		vgic_irq_handle_resampling(irq, deactivated, val & GICH_LR_PENDING_BIT);

[Severity: High]
What happens for a mapped level-triggered interrupt (irq->hw == true,
irq->config == VGIC_CONFIG_LEVEL, and !vgic_irq_needs_resampling(irq), such as
the architected timer interrupt) when the guest writes GICD_ISPENDR0 while the
interrupt is active?

In arch/arm64/kvm/vgic/vgic-mmio.c:vgic_mmio_write_spending()->__set_pending():

    if (irq->on_lr)
        irq->latch_written = true;

    irq->pending_latch = true;
    if (irq->hw && !is_user)
        vgic_irq_set_phys_active(irq, true);

Because the physical interrupt is already active in the host distributor for
the first in-flight delivery, vgic_irq_set_phys_active(irq, true) coalesces
with the existing physical active state. When the guest then deactivates the
first delivery (via GICC_EOIR / GICC_DIR in hardware, or via the EOIcount path
in vgic_v2_fold_lr_state()->vgic_v2_deactivate() writing GIC_CPU_DEACTIVATE),
the physical interrupt is deactivated in the host distributor.

When vgic_v2_fold_lr() runs on exit, was_active or irq->latch_written is true,
so irq->pending_latch remains true while irq->active becomes false. However,
neither vgic_v2_fold_lr() nor
arch/arm64/kvm/vgic/vgic.c:vgic_irq_handle_resampling() re-activates the
physical interrupt in the host distributor (and if lr_deactivated &&
irq->line_level holds with the physical line low, vgic_irq_handle_resampling()
explicitly clears the physical active state):

    if (unlikely(vgic_irq_needs_resampling(irq))) {
        resample = !(irq->active || irq->pending_latch);
    } else if (lr_pending || (lr_deactivated && irq->line_level)) {
        irq->line_level = vgic_get_phys_line_level(irq);
        resample = !irq->line_level;
    }

    if (resample)
        vgic_irq_set_phys_active(irq, false);

On the next guest entry,
arch/arm64/kvm/vgic/vgic-v2.c:vgic_v2_populate_lr()->vgic_v2_compute_lr() sees
irq->active == false and irq_is_pending(irq) == true from irq->pending_latch:

    if (irq->hw && !vgic_irq_needs_resampling(irq)) {
        val |= GICH_LR_HW;
        val |= irq->hwintid << GICH_LR_PHYSID_CPUID_SHIFT;
        if (irq->active)
            allow_pending = false;
    }

    if (allow_pending && irq_is_pending(irq)) {
        val |= GICH_LR_PENDING_BIT;

Can this populate the list register with GICH_LR_HW | GICH_LR_PENDING_BIT
while the corresponding physical interrupt is inactive in the host
distributor, violating the hardware-bit consistency requirement and triggering
a second physical deactivation when the guest completes the second delivery?

>  
>  		irq->on_lr = false;
>  	}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022064628.4206-1-kmehltretter@gmail.com?part=1

[RFC PATCH 2/3] KVM: arm64: vgic-v3: Don't drop pending writes to active level IRQs

[Karl Mehltretter]

The GICv3 backend also loses a level interrupt set pending through
GICD_ISPENDR or GICR_ISPENDR0 while it is active. Its LR handling has
the same split between pending_latch and an Active-only LR as GICv2,
and clears the new pending state after deactivation.

Apply the GICv2 bookkeeping to vgic_v3_fold_lr(). Clear the old latch
when the LR reports that the interrupt has been acknowledged, and
preserve a latch which was not represented in the LR.

GICv3 permits an implementation to clear register-set pending state on
ack. Doing so matches the GICv2 backend and makes a later ISPENDR write
a distinct pending state.

Fixes: 67b5b673ad4d ("KVM: arm/arm64: vgic: Disallow Active+Pending for level interrupts")
Assisted-by: LLM
---
 arch/arm64/kvm/vgic/vgic-v3.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index 726e20a1da6e7..8009dc6df00db 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -75,7 +75,7 @@ static void vgic_v3_fold_lr(struct kvm_vcpu *vcpu, u64 val)
 {
 	struct vgic_irq *irq;
 	bool is_v2_sgi = false;
-	bool deactivated;
+	bool deactivated, was_active;
 	u32 intid;
 
 	if (vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) {
@@ -93,7 +93,8 @@ static void vgic_v3_fold_lr(struct kvm_vcpu *vcpu, u64 val)
 		/* Always preserve the active bit for !LPIs, note deactivation */
 		if (irq->intid >= VGIC_MIN_LPI)
 			val &= ~ICH_LR_ACTIVE_BIT;
-		deactivated = irq->active && !(val & ICH_LR_ACTIVE_BIT);
+		was_active = irq->active;
+		deactivated = was_active && !(val & ICH_LR_ACTIVE_BIT);
 		irq->active = !!(val & ICH_LR_ACTIVE_BIT);
 
 		/* Edge is the only case where we preserve the pending bit */
@@ -103,9 +104,15 @@ static void vgic_v3_fold_lr(struct kvm_vcpu *vcpu, u64 val)
 
 		/*
 		 * Clear soft pending state when level irqs have been acked.
+		 * A latch written while the irq was active is a new pending
+		 * state. A write while the irq was in the LR may also have
+		 * followed the ack, so preserve it too.
 		 */
-		if (irq->config == VGIC_CONFIG_LEVEL && !(val & ICH_LR_STATE))
+		if (irq->config == VGIC_CONFIG_LEVEL &&
+		    !(val & ICH_LR_PENDING_BIT) &&
+		    !was_active && !irq->latch_written)
 			irq->pending_latch = false;
+		irq->latch_written = false;
 
 		if (is_v2_sgi) {
 			u8 cpuid = FIELD_GET(GICH_LR_PHYSID_CPUID, val);
-- 
2.39.5 (Apple Git-154)

Re: [RFC PATCH 2/3] KVM: arm64: vgic-v3: Don't drop pending writes to active level IRQs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] KVM: arm64: vgic-v3: vgic_v3_fold_lr() preserves latch on deactivated HW IRQ
--

commit 0c743aae5548587f04c81ee3dc502841f4713aff
Author: Karl Mehltretter <kmehltretter@gmail.com>

KVM: arm64: vgic-v3: Don't drop pending writes to active level IRQs

This commit updates vgic_v3_fold_lr() so that a soft-pending latch written
while a level-triggered interrupt is active or present in a list register is
preserved across deactivation.

> diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
> index 726e20a1da6e7..8009dc6df00db 100644
> --- a/arch/arm64/kvm/vgic/vgic-v3.c
> +++ b/arch/arm64/kvm/vgic/vgic-v3.c
> @@ -75,7 +75,7 @@ static void vgic_v3_fold_lr(struct kvm_vcpu *vcpu, u64 val)
>  {
>  	struct vgic_irq *irq;
>  	bool is_v2_sgi = false;
> -	bool deactivated;
> +	bool deactivated, was_active;
>  	u32 intid;
>  
>  	if (vcpu->kvm->arch.vgic.vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) {
> @@ -93,7 +93,8 @@ static void vgic_v3_fold_lr(struct kvm_vcpu *vcpu, u64 val)
>  		/* Always preserve the active bit for !LPIs, note deactivation */
>  		if (irq->intid >= VGIC_MIN_LPI)
>  			val &= ~ICH_LR_ACTIVE_BIT;
> -		deactivated = irq->active && !(val & ICH_LR_ACTIVE_BIT);
> +		was_active = irq->active;
> +		deactivated = was_active && !(val & ICH_LR_ACTIVE_BIT);
>  		irq->active = !!(val & ICH_LR_ACTIVE_BIT);
>  
>  		/* Edge is the only case where we preserve the pending bit */
> @@ -103,9 +104,15 @@ static void vgic_v3_fold_lr(struct kvm_vcpu *vcpu, u64 val)
>  
>  		/*
>  		 * Clear soft pending state when level irqs have been acked.
> +		 * A latch written while the irq was active is a new pending
> +		 * state. A write while the irq was in the LR may also have
> +		 * followed the ack, so preserve it too.
>  		 */
> -		if (irq->config == VGIC_CONFIG_LEVEL && !(val & ICH_LR_STATE))
> +		if (irq->config == VGIC_CONFIG_LEVEL &&
> +		    !(val & ICH_LR_PENDING_BIT) &&
> +		    !was_active && !irq->latch_written)
>  			irq->pending_latch = false;
> +		irq->latch_written = false;

[Severity: High]
What happens here when a mapped level-triggered interrupt (irq->hw is true,
irq->config == VGIC_CONFIG_LEVEL, and !vgic_irq_needs_resampling(irq), such
as the architected timer) has its pending latch set via
vgic_mmio_write_spending() while the interrupt is already active or in a list
register?

In arch/arm64/kvm/vgic/vgic-mmio.c:__set_pending(), writing the set-pending
register sets irq->pending_latch = true (and irq->latch_written = true if
irq->on_lr) and calls vgic_irq_set_phys_active(irq, true):

arch/arm64/kvm/vgic/vgic-mmio.c:__set_pending() {
    ...
		if (irq->on_lr)
			irq->latch_written = true;

		irq->pending_latch = true;
		if (irq->hw && !is_user)
			vgic_irq_set_phys_active(irq, true);
    ...
}

Because the physical interrupt is already active in the host distributor or
redistributor for the in-flight virtual interrupt, there is only a single
physical active state bit. When the guest then deactivates the virtual
interrupt, hardware deactivation (or vgic_v3_deactivate_phys() when
deactivated outside the list registers) clears the physical active state in
the host.

When vgic_v3_fold_lr() runs on exit, was_active or irq->latch_written is true,
so irq->pending_latch remains true while irq->active becomes false. Neither
vgic_v3_fold_lr() nor vgic_irq_handle_resampling() re-activates the physical
interrupt, and if irq->line_level was true and the physical line is now low,
vgic_irq_handle_resampling() explicitly clears the physical active state:

arch/arm64/kvm/vgic/vgic.c:vgic_irq_handle_resampling() {
	if (vgic_irq_is_mapped_level(irq)) {
		bool resample = false;

		if (unlikely(vgic_irq_needs_resampling(irq))) {
			resample = !(irq->active || irq->pending_latch);
		} else if (lr_pending || (lr_deactivated && irq->line_level)) {
			irq->line_level = vgic_get_phys_line_level(irq);
			resample = !irq->line_level;
		}

		if (resample)
			vgic_irq_set_phys_active(irq, false);
	}
}

On the next guest entry, irq->active is false and irq_is_pending(irq) is true
via the preserved irq->pending_latch, so vgic_v3_compute_lr() populates the
list register with ICH_LR_HW | ICH_LR_PENDING_BIT while the physical interrupt
is inactive in the host:

arch/arm64/kvm/vgic/vgic-v3.c:vgic_v3_compute_lr() {
    ...
	if (irq->hw && !vgic_irq_needs_resampling(irq)) {
		val |= ICH_LR_HW;
		val |= ((u64)irq->hwintid) << ICH_LR_PHYS_ID_SHIFT;
		if (irq->active)
			allow_pending = false;
	}
    ...
	if (allow_pending && irq_is_pending(irq)) {
		val |= ICH_LR_PENDING_BIT;
    ...
}

Doesn't placing a list register entry with ICH_LR_HW set in the pending state
while the corresponding physical interrupt is not active in the distributor
or redistributor violate the hardware-bit consistency requirement (resulting
in unpredictable cpu interface behavior and a duplicate physical deactivation
of an inactive physical interrupt)?

[ ... ]

>
> 
>  		if (is_v2_sgi) {
>  			u8 cpuid = FIELD_GET(GICH_LR_PHYSID_CPUID, val);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022064628.4206-1-kmehltretter@gmail.com?part=2

[RFC PATCH 3/3] KVM: selftests: arm64: Test pending writes to active level IRQs

[Karl Mehltretter]

Add a two-vCPU GICv3 test for a level SPI set pending through
GICD_ISPENDR. Re-pend the interrupt while it is active, first from the
handling vCPU and then from the other vCPU. Require a second delivery
after deactivation in both cases.

Also check that the other vCPU observes the initial pending state
before the interrupt is acknowledged.

Without the VGIC fix, the pending state is cleared on deactivation and
the test fails before the second delivery.

Assisted-by: LLM
---
 tools/testing/selftests/kvm/arm64/vgic_irq.c | 101 +++++++++++++++++++
 1 file changed, 101 insertions(+)

diff --git a/tools/testing/selftests/kvm/arm64/vgic_irq.c b/tools/testing/selftests/kvm/arm64/vgic_irq.c
index 3cb85932ee85c..81f38e523d3c0 100644
--- a/tools/testing/selftests/kvm/arm64/vgic_irq.c
+++ b/tools/testing/selftests/kvm/arm64/vgic_irq.c
@@ -959,6 +959,106 @@ static void guest_code_timer_spi(struct test_args *args, int cpuid)
 	GUEST_DONE();
 }
 
+#define REPEND_READ	1
+#define REPEND_READ_DONE	2
+#define REPEND_WRITE	3
+
+/*
+ * A level-sensitive SPI that is set pending through GICD_ISPENDR stays
+ * pending for every vCPU until it is acked. If it is set pending again while
+ * it is active, it becomes active and pending and must be taken a second time
+ * once it has been deactivated. The second pending state comes from the vCPU
+ * that handles the interrupt in the first round, and from the other vCPU in
+ * the second.
+ */
+static void guest_code_level_repend(struct test_args *args, int cpuid)
+{
+	int i;
+
+	gic_init(GIC_V3, 2);
+
+	gic_set_eoi_split(1);
+	gic_set_priority_mask(CPU_PRIO_MASK);
+
+	if (cpuid == 0) {
+		u32 intid;
+
+		local_irq_disable();
+
+		gic_irq_set_config(MIN_SPI, false);
+		gic_set_priority(MIN_SPI, IRQ_DEFAULT_PRIO);
+		gic_irq_enable(MIN_SPI);
+
+		for (i = 0; i < 2; i++) {
+			gic_irq_set_pending(MIN_SPI);
+
+			if (i == 1) {
+				/* Not acked yet: the other vCPU sees it pending */
+				WRITE_ONCE(args->shared_data, REPEND_READ);
+				dsb(ishst);
+
+				do {
+					dsb(ishld);
+				} while (READ_ONCE(args->shared_data) != REPEND_READ_DONE);
+			}
+
+			GUEST_ASSERT(gic_irq_get_pending(MIN_SPI));
+			intid = wait_for_and_activate_irq();
+			GUEST_ASSERT_EQ(intid, MIN_SPI);
+
+			if (i == 0) {
+				gic_irq_set_pending(MIN_SPI);
+			} else {
+				WRITE_ONCE(args->shared_data, REPEND_WRITE);
+				dsb(ishst);
+
+				do {
+					dsb(ishld);
+				} while (READ_ONCE(args->shared_data) == REPEND_WRITE);
+			}
+
+			GUEST_ASSERT(gic_irq_get_active(MIN_SPI));
+			gic_set_eoi(intid);
+			gic_set_dir(intid);
+			isb();
+
+			/* The second pending state must not have been lost */
+			GUEST_ASSERT(gic_irq_get_pending(MIN_SPI));
+			intid = wait_for_and_activate_irq();
+			GUEST_ASSERT_EQ(intid, MIN_SPI);
+
+			gic_set_eoi(intid);
+			gic_set_dir(intid);
+			isb();
+
+			GUEST_ASSERT(!gic_irq_get_active(MIN_SPI));
+			GUEST_ASSERT(!gic_irq_get_pending(MIN_SPI));
+			GUEST_ASSERT_EQ(gic_get_and_ack_irq(), IAR_SPURIOUS);
+		}
+	} else {
+		do {
+			dsb(ishld);
+		} while (READ_ONCE(args->shared_data) != REPEND_READ);
+
+		for (i = 0; i < 64; i++)
+			GUEST_ASSERT(gic_irq_get_pending(MIN_SPI));
+
+		WRITE_ONCE(args->shared_data, REPEND_READ_DONE);
+		dsb(ishst);
+
+		do {
+			dsb(ishld);
+		} while (READ_ONCE(args->shared_data) != REPEND_WRITE);
+
+		gic_irq_set_pending(MIN_SPI);
+
+		WRITE_ONCE(args->shared_data, 0);
+		dsb(ishst);
+	}
+
+	GUEST_DONE();
+}
+
 static void *test_vcpu_run(void *arg)
 {
 	struct kvm_vcpu *vcpu = arg;
@@ -1073,6 +1173,7 @@ int main(int argc, char **argv)
 		test_vgic_two_cpus(guest_code_asym_dir);
 		test_vgic_two_cpus(guest_code_group_en);
 		test_vgic_two_cpus(guest_code_timer_spi);
+		test_vgic_two_cpus(guest_code_level_repend);
 	} else {
 		test_vgic(nr_irqs, level_sensitive, eoi_split);
 	}
-- 
2.39.5 (Apple Git-154)

Re: [RFC PATCH 0/3] KVM: arm64: Preserve level IRQ re-pending

[Oliver Upton]

On Thu, Oct 22, 2026 at 08:46:25AM +0200, Karl Mehltretter wrote:
> A guest can set a level interrupt pending again while handling it. The
> GIC records Active and Pending and delivers the interrupt after
> deactivation. KVM instead drops the second pending state for both the
> GICv2 and GICv3 backends.
> 
> The VGIC keeps software pending state outside the LR because commit
> 67b5b673ad4d stopped placing Pending and Active together for level
> interrupts. When the LR is folded, KVM cannot distinguish the pending
> state used to populate the LR from a later ISPENDR write. This series
> records the later write and preserves it across deactivation. The last
> patch adds a two-vCPU GICv3 selftest.
> 
> The awkward case, and the reason for the RFC, is a write from another
> vCPU while the interrupt remains in an LR. KVM cannot know whether the
> write preceded or followed the guest ack. This series preserves the
> write. A pre-ack write can therefore cause one extra delivery, while a
> post-ack write is no longer lost.
> 
> The exact alternative is to stop and synchronize the target vCPU
> before applying such a write, as the active-state accessors do. Is the
> conservative replay acceptable here, or should ISPENDR pay that
> synchronization cost?

Yes, just kick the target vCPU out and sync the LRs back into the VGIC
state. That's consistent with the approach we're taking everywhere else
where this sort of issue arises.

If we're lucky the vCPU is just poking at its own redistributor and the
synchronization is free.

> The issue was reproduced with Zephyr, NuttX and RT-Thread guests. On a
> Raspberry Pi 400 host with a GIC-400, each GICv2 guest handled the SPI
> once before the fix and twice afterwards.

Have you configured these guests in such a way to reproduce the issue?
What exactly are they doing that is susceptible to this issue?

Thanks,
Oliver

Re: [RFC PATCH 0/3] KVM: arm64: Preserve level IRQ re-pending

[Karl Mehltretter]

On Fri, Oct 23, 2026 at 11:55:57PM +0100, Oliver Upton wrote:
> > The exact alternative is to stop and synchronize the target vCPU
> > before applying such a write, as the active-state accessors do. Is the
> > conservative replay acceptable here, or should ISPENDR pay that
> > synchronization cost?
> 
> Yes, just kick the target vCPU out and sync the LRs back into the VGIC
> state. That's consistent with the approach we're taking everywhere else
> where this sort of issue arises.
> 
> If we're lucky the vCPU is just poking at its own redistributor and the
> synchronization is free.

That makes sense. In my current v2, vgic_mmio_write_spending() uses
vgic_access_active_prepare() and vgic_access_active_finish() under
config_lock, and drops the extra struct vgic_irq flag.

> 
> > The issue was reproduced with Zephyr, NuttX and RT-Thread guests. On a
> > Raspberry Pi 400 host with a GIC-400, each GICv2 guest handled the SPI
> > once before the fix and twice afterwards.
> 
> Have you configured these guests in such a way to reproduce the issue?
> What exactly are they doing that is susceptible to this issue?
> 

Yes, the runs used a small test harness in each guest. Each test
configures unused SPI 150 as level-sensitive, pends it from thread
context, and pends it once more from the ISR before deactivation:

- Zephyr (qemu_cortex_a53): k_irq_set_pending() -> GICD_ISPENDR.
- NuttX (qemu-armv8a): up_set_irq_type(IRQ_HIGH_LEVEL) and
  up_trigger_irq() -> GICD_ISPENDR.
- RT-Thread (qemu-virt64-aarch64, pic.gicv2_eoimode=1):
  rt_pic_irq_set_state_raw() -> GICD_ISPENDR.

A driver that hits the same pattern is RT-Thread's
components/drivers/mailbox/mailbox-pic.c, which amp.dtsi configures with
level-high SPIs and rings via GICD_ISPENDR. 

pic_mbox_isr() reads MAILBOX_ISTATE once at entry and clears only those
bits at exit.

Sending on another channel while pic_mbox_isr() is active sets the new
MAILBOX_ISTATE bit and writes GICD_ISPENDR. Without this RFC fix,
deactivation drops the second pending state on both GICv2 and GICv3 and
leaves the new channel bit stuck.

Thanks,
Karl