From 6362a59ea48660d300ab745a8b7263b90b26fd5c Mon Sep 17 00:00:00 2001 From: Jose Martins Date: Thu, 19 Mar 2026 10:17:07 +0000 Subject: [PATCH 1/9] fix(vgic): keep active interrupts in LRs when guest disables them When a guest disables an interrupt that is currently active in an LR, vgic_route() and vgic_add_lr() were returning early due to the !interrupt->enabled check, leaving the interrupt untracked. On re-enable, Bao would re-inject it as active causing incorrect state. Allow disabled-but-active interrupts through so the guest can EOI them directly via the LR, avoiding a spurious LRENP maintenance interrupt trap. Signed-off-by: Jose Martins --- src/arch/armv8/vgic.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/arch/armv8/vgic.c b/src/arch/armv8/vgic.c index 280464d96..3f0182362 100644 --- a/src/arch/armv8/vgic.c +++ b/src/arch/armv8/vgic.c @@ -149,7 +149,16 @@ void vgic_send_sgi_msg(struct vcpu* vcpu, cpumap_t pcpu_mask, irqid_t int_id) static void vgic_route(struct vcpu* vcpu, struct vgic_int* interrupt) { - if ((interrupt->state == INV) || !interrupt->enabled) { + if (interrupt->state == INV) { + return; + } + + /** + * A disabled interrupt that is still active must be placed in an LR so + * the guest can EOI it directly, avoiding an unnecessary LRENP maintenance + * interrupt trap. + */ + if (!interrupt->enabled && !(interrupt->state & ACT)) { return; } @@ -319,7 +328,7 @@ bool vgic_add_lr(struct vcpu* vcpu, struct vgic_int* interrupt) { bool ret = false; - if (!interrupt->enabled || interrupt->in_lr) { + if ((!interrupt->enabled && !(interrupt->state & ACT)) || interrupt->in_lr) { return ret; } From cc44d9bc24c08644660be7582c57dc09ea238b1d Mon Sep 17 00:00:00 2001 From: Jose Martins Date: Thu, 19 Mar 2026 10:18:05 +0000 Subject: [PATCH 2/9] fix(vgic): remove spilled interrupt from list on LRENP maintenance vgic_eoir_highest_spilled_active() was clearing the ACT state of the highest-priority spilled active interrupt but never removing it from the spilled list, causing it to be re-processed on subsequent maintenance interrupts or LR refills. Add list_rm() after gaining ownership. vgic_spilled_lock is held around the search and removal; it is released before vgic_add_lr() to avoid a deadlock with vgic_add_spilled() on the SW+PEND path. Signed-off-by: Jose Martins --- src/arch/armv8/vgic.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/arch/armv8/vgic.c b/src/arch/armv8/vgic.c index 3f0182362..8c14b2ee9 100644 --- a/src/arch/armv8/vgic.c +++ b/src/arch/armv8/vgic.c @@ -1161,18 +1161,28 @@ static void vgic_refill_lrs(struct vcpu* vcpu, bool npie) static void vgic_eoir_highest_spilled_active(struct vcpu* vcpu) { struct list* list = NULL; - struct vgic_int* interrupt = vgic_highest_prio_spilled(vcpu, ACT, &list); + struct vgic_int* interrupt; + spin_lock(&vcpu->vm->arch.vgic_spilled_lock); + interrupt = vgic_highest_prio_spilled(vcpu, ACT, &list); if (interrupt != NULL) { spin_lock(&interrupt->lock); if (vgic_get_ownership(vcpu, interrupt)) { - interrupt->state &= (uint8_t)~ACT; - if (vgic_int_is_hw(interrupt)) { - gic_set_act(interrupt->id, false); - } else { - if (interrupt->state & PEND) { - vgic_add_lr(vcpu, interrupt); - } + list_rm(list, &interrupt->node); + } else { + spin_unlock(&interrupt->lock); + interrupt = NULL; + } + } + spin_unlock(&vcpu->vm->arch.vgic_spilled_lock); + + if (interrupt != NULL) { + interrupt->state &= (uint8_t)~ACT; + if (vgic_int_is_hw(interrupt)) { + gic_set_act(interrupt->id, false); + } else { + if (interrupt->state & PEND) { + vgic_add_lr(vcpu, interrupt); } } spin_unlock(&interrupt->lock); From 69169497c93e8c33d86618ba7d9adeb39e0d64dd Mon Sep 17 00:00:00 2001 From: Jose Martins Date: Thu, 19 Mar 2026 11:09:05 +0000 Subject: [PATCH 3/9] fix(vgic): track unrouted interrupts in spilled list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When vgic_route() fails to place an interrupt in an LR or forward it to another CPU (e.g. no target configured), the interrupt was left untracked — not in any LR and not in the spilled list — causing it to be missed on future LR refills. Add in_spilled flag to vgic_int to prevent double-adds. vgic_add_spilled() guards on !in_lr && !in_spilled, and clears/sets the flag alongside list operations. vgic_route() unconditionally calls vgic_add_spilled() as a fallback after routing, which is a no-op if the interrupt was already placed in an LR or spilled by vgic_add_lr(). Signed-off-by: Jose Martins --- src/arch/armv8/inc/arch/vgic.h | 1 + src/arch/armv8/vgic.c | 42 ++++++++++++++++++++++------------ 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/src/arch/armv8/inc/arch/vgic.h b/src/arch/armv8/inc/arch/vgic.h index c9e49409d..1f113fccb 100644 --- a/src/arch/armv8/inc/arch/vgic.h +++ b/src/arch/armv8/inc/arch/vgic.h @@ -47,6 +47,7 @@ struct vgic_int { #endif bool hw; bool in_lr; + bool in_spilled; bool enabled; }; diff --git a/src/arch/armv8/vgic.c b/src/arch/armv8/vgic.c index 8c14b2ee9..b1dd1edc3 100644 --- a/src/arch/armv8/vgic.c +++ b/src/arch/armv8/vgic.c @@ -147,6 +147,23 @@ void vgic_send_sgi_msg(struct vcpu* vcpu, cpumap_t pcpu_mask, irqid_t int_id) } } +static void vgic_add_spilled(struct vcpu* vcpu, struct vgic_int* interrupt) +{ + spin_lock(&vcpu->vm->arch.vgic_spilled_lock); + if (!interrupt->in_lr && !interrupt->in_spilled) { + struct list* spilled_list = NULL; + if (gic_is_priv(interrupt->id)) { + spilled_list = &vcpu->arch.vgic_spilled; + } else { + spilled_list = &vcpu->vm->arch.vgic_spilled; + } + list_push(spilled_list, (node_t*)interrupt); + interrupt->in_spilled = true; + } + spin_unlock(&vcpu->vm->arch.vgic_spilled_lock); + gich_set_hcr(gich_get_hcr() | GICH_HCR_NPIE_BIT); +} + static void vgic_route(struct vcpu* vcpu, struct vgic_int* interrupt) { if (interrupt->state == INV) { @@ -181,6 +198,15 @@ static void vgic_route(struct vcpu* vcpu, struct vgic_int* interrupt) } } } + + /** + * If the interrupt was not placed in an LR or forwarded to another CPU, + * track it in the spilled list so it gets injected when an LR slot + * becomes available. + */ + if (!interrupt->in_lr && !interrupt->in_spilled) { + vgic_add_spilled(vcpu, interrupt); + } } static inline void vgic_write_lr(struct vcpu* vcpu, struct vgic_int* interrupt, size_t lr_ind) @@ -296,20 +322,6 @@ bool vgic_remove_lr(struct vcpu* vcpu, struct vgic_int* interrupt) return ret; } -static void vgic_add_spilled(struct vcpu* vcpu, struct vgic_int* interrupt) -{ - spin_lock(&vcpu->vm->arch.vgic_spilled_lock); - struct list* spilled_list = NULL; - if (gic_is_priv(interrupt->id)) { - spilled_list = &vcpu->arch.vgic_spilled; - } else { - spilled_list = &vcpu->vm->arch.vgic_spilled; - } - list_push(spilled_list, (node_t*)interrupt); - spin_unlock(&vcpu->vm->arch.vgic_spilled_lock); - gich_set_hcr(gich_get_hcr() | GICH_HCR_NPIE_BIT); -} - static void vgic_spill_lr(struct vcpu* vcpu, size_t lr_ind) { gic_lr_t lr = (gic_lr_t)gich_read_lr(lr_ind); @@ -1140,6 +1152,7 @@ static void vgic_refill_lrs(struct vcpu* vcpu, bool npie) bool got_ownership = vgic_get_ownership(vcpu, irq); if (got_ownership) { list_rm(list, &irq->node); + irq->in_spilled = false; vgic_write_lr(vcpu, irq, (size_t)lr_ind); } spin_unlock(&irq->lock); @@ -1169,6 +1182,7 @@ static void vgic_eoir_highest_spilled_active(struct vcpu* vcpu) spin_lock(&interrupt->lock); if (vgic_get_ownership(vcpu, interrupt)) { list_rm(list, &interrupt->node); + interrupt->in_spilled = false; } else { spin_unlock(&interrupt->lock); interrupt = NULL; From 89c9ae6ac80d83c761f6ec7f7139d7a2509bd747 Mon Sep 17 00:00:00 2001 From: Jose Martins Date: Thu, 19 Mar 2026 11:29:10 +0000 Subject: [PATCH 4/9] fix(vgic): remove interrupt from spilled list when placed in LR When vgic_add_lr() successfully placed an interrupt into an LR, it left a stale entry in the spilled list, causing vgic_refill_lrs() to find and attempt to re-process an already-tracked interrupt. Introduce vgic_remove_spilled() to wrap list_rm() and the in_spilled flag update in one place. Call it in vgic_add_lr() before vgic_write_lr(), and replace the open-coded equivalents in vgic_refill_lrs() and vgic_eoir_highest_spilled_active(). Drop the now-unused outlist parameter from vgic_highest_prio_spilled(). Signed-off-by: Jose Martins --- src/arch/armv8/vgic.c | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/src/arch/armv8/vgic.c b/src/arch/armv8/vgic.c index b1dd1edc3..58904e897 100644 --- a/src/arch/armv8/vgic.c +++ b/src/arch/armv8/vgic.c @@ -322,6 +322,16 @@ bool vgic_remove_lr(struct vcpu* vcpu, struct vgic_int* interrupt) return ret; } +/* Must be called holding vgic_spilled_lock */ +static void vgic_remove_spilled(struct vcpu* vcpu, struct vgic_int* interrupt) +{ + struct list* spilled_list = gic_is_priv(interrupt->id) + ? &vcpu->arch.vgic_spilled + : &vcpu->vm->arch.vgic_spilled; + list_rm(spilled_list, &interrupt->node); + interrupt->in_spilled = false; +} + static void vgic_spill_lr(struct vcpu* vcpu, size_t lr_ind) { gic_lr_t lr = (gic_lr_t)gich_read_lr(lr_ind); @@ -396,6 +406,11 @@ bool vgic_add_lr(struct vcpu* vcpu, struct vgic_int* interrupt) } if (lr_ind >= 0) { + if (interrupt->in_spilled) { + spin_lock(&vcpu->vm->arch.vgic_spilled_lock); + vgic_remove_spilled(vcpu, interrupt); + spin_unlock(&vcpu->vm->arch.vgic_spilled_lock); + } vgic_write_lr(vcpu, interrupt, (size_t)lr_ind); ret = true; } else { @@ -1108,8 +1123,7 @@ void vgic_ipi_handler(uint32_t event, uint64_t data) /** * Must be called holding the vgic_spilled_lock */ -static inline struct vgic_int* vgic_highest_prio_spilled(struct vcpu* vcpu, unsigned flags, - struct list** outlist) +static inline struct vgic_int* vgic_highest_prio_spilled(struct vcpu* vcpu, unsigned flags) { struct vgic_int* irq = NULL; struct list* spilled_lists[] = { @@ -1131,7 +1145,6 @@ static inline struct vgic_int* vgic_highest_prio_spilled(struct vcpu* vcpu, unsi bool is_lower_id = temp_irq->id < irq_id; if (is_higher_prio || (is_same_prio && is_lower_id)) { irq = temp_irq; - *outlist = list; } } } @@ -1145,14 +1158,12 @@ static void vgic_refill_lrs(struct vcpu* vcpu, bool npie) unsigned flags = npie ? PEND : ACT | PEND; spin_lock(&vcpu->vm->arch.vgic_spilled_lock); while (lr_ind >= 0) { - struct list* list = NULL; - struct vgic_int* irq = vgic_highest_prio_spilled(vcpu, flags, &list); + struct vgic_int* irq = vgic_highest_prio_spilled(vcpu, flags); if (irq != NULL) { spin_lock(&irq->lock); bool got_ownership = vgic_get_ownership(vcpu, irq); if (got_ownership) { - list_rm(list, &irq->node); - irq->in_spilled = false; + vgic_remove_spilled(vcpu, irq); vgic_write_lr(vcpu, irq, (size_t)lr_ind); } spin_unlock(&irq->lock); @@ -1173,16 +1184,14 @@ static void vgic_refill_lrs(struct vcpu* vcpu, bool npie) static void vgic_eoir_highest_spilled_active(struct vcpu* vcpu) { - struct list* list = NULL; struct vgic_int* interrupt; spin_lock(&vcpu->vm->arch.vgic_spilled_lock); - interrupt = vgic_highest_prio_spilled(vcpu, ACT, &list); + interrupt = vgic_highest_prio_spilled(vcpu, ACT); if (interrupt != NULL) { spin_lock(&interrupt->lock); if (vgic_get_ownership(vcpu, interrupt)) { - list_rm(list, &interrupt->node); - interrupt->in_spilled = false; + vgic_remove_spilled(vcpu, interrupt); } else { spin_unlock(&interrupt->lock); interrupt = NULL; From 696647cd8d626323c39d528f64560e61abc327b3 Mon Sep 17 00:00:00 2001 From: Jose Martins Date: Thu, 19 Mar 2026 11:35:15 +0000 Subject: [PATCH 5/9] fix(vgic): skip IPI forwarding for active interrupts in vgic_route() Sending an IPI for an active interrupt is pointless: vgic_yield_ownership() refuses to yield ownership while the interrupt is active, so the receiving CPU will fail to acquire ownership and do nothing. Avoid the unnecessary IPI by guarding the forwarding path with !(interrupt->state & ACT). Signed-off-by: Jose Martins --- src/arch/armv8/vgic.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/arch/armv8/vgic.c b/src/arch/armv8/vgic.c index 58904e897..bc9cc8a1a 100644 --- a/src/arch/armv8/vgic.c +++ b/src/arch/armv8/vgic.c @@ -183,7 +183,12 @@ static void vgic_route(struct vcpu* vcpu, struct vgic_int* interrupt) vgic_add_lr(vcpu, interrupt); } - if (!interrupt->in_lr && vgic_int_has_other_target(vcpu, interrupt)) { + /** + * An active interrupt cannot be forwarded to another CPU — ownership + * cannot be yielded while the interrupt is active, so any IPI would + * be silently ignored by the recipient. + */ + if (!interrupt->in_lr && !(interrupt->state & ACT) && vgic_int_has_other_target(vcpu, interrupt)) { union vgic_msg_data data = { .vm_id = (uint16_t)vcpu->vm->id, .vgicr_id = (uint16_t)vcpu->id, From 933af3264ea60a54dd460f7ddc8487ee9659bc2d Mon Sep 17 00:00:00 2001 From: Jose Martins Date: Thu, 19 Mar 2026 11:57:32 +0000 Subject: [PATCH 6/9] fix(vgic): do not write PEND to LRs of disabled interrupts When a disabled PENDACT interrupt is placed in an LR, the LR was written with both ACT and PEND set. On guest EOI the LR would transition from PENDACT to PEND and re-deliver the interrupt even though it is disabled. Strip PEND from the LR state written for disabled non-HW interrupts, and preserve it in interrupt->state so it is restored when the interrupt is re-enabled. HW interrupts need no equivalent handling: a PENDACT HW interrupt already has its LR written with only ACT, as an LR with the HW bit set cannot hold the pending and active state, and its pending state is instead tracked by the physical GIC, which cannot re-deliver it while the interrupt is disabled since the physical enable mirrors the virtual one. Also fix vgic_remove_lr() to OR in the preserved PEND when updating interrupt->state from the LR, so that an early removal (e.g. on re-enable before the guest EOIs) does not overwrite the preserved pending state. Signed-off-by: Jose Martins --- src/arch/armv8/vgic.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/arch/armv8/vgic.c b/src/arch/armv8/vgic.c index bc9cc8a1a..068766e00 100644 --- a/src/arch/armv8/vgic.c +++ b/src/arch/armv8/vgic.c @@ -278,10 +278,17 @@ static inline void vgic_write_lr(struct vcpu* vcpu, struct vgic_int* interrupt, lr |= GICH_LR_EOI_BIT; } - lr |= ((gic_lr_t)state << GICH_LR_STATE_OFF) & GICH_LR_STATE_MSK; + /** + * If the interrupt is disabled, strip PEND from the LR so the guest + * only EOIs the active portion and the interrupt is not re-delivered + * while disabled. The PEND bit is preserved in interrupt->state so + * it is restored when the interrupt is re-enabled. + */ + unsigned lr_state = !interrupt->enabled ? (state & (unsigned)~PEND) : state; + lr |= ((gic_lr_t)lr_state << GICH_LR_STATE_OFF) & GICH_LR_STATE_MSK; } - interrupt->state = (uint8_t)INV; + interrupt->state = !interrupt->enabled ? (uint8_t)(state & PEND) : (uint8_t)INV; interrupt->in_lr = true; interrupt->lr = (uint8_t)lr_ind; vcpu->arch.vgic_priv.curr_lrs[lr_ind] = interrupt->id; @@ -305,7 +312,7 @@ bool vgic_remove_lr(struct vcpu* vcpu, struct vgic_int* interrupt) interrupt->in_lr = false; if (GICH_LR_STATE(lr_val) != INV) { - interrupt->state = (uint8_t)GICH_LR_STATE(lr_val); + interrupt->state = (uint8_t)(GICH_LR_STATE(lr_val) | (interrupt->state & PEND)); #if (GIC_VERSION == GICV2) if (interrupt->id < GIC_MAX_SGIS) { if (interrupt->state & ACT) { From c96a91b246d0dc84b1c62817832175a3223b2b86 Mon Sep 17 00:00:00 2001 From: Jose Martins Date: Thu, 19 Mar 2026 11:59:47 +0000 Subject: [PATCH 7/9] fix(vgic): fall back to active LR when no pending can be spilled When all LRs are full and more than one of them holds a pending interrupt, but none of those is an eligible victim (none has lower priority than the new interrupt), the spill selection skipped eviction entirely, even if a lower priority active LR was available to spill. Add pend_ind >= 0 to the condition so the selection falls through to the active candidate when no eligible pending candidate exists. Signed-off-by: Jose Martins --- src/arch/armv8/vgic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/arch/armv8/vgic.c b/src/arch/armv8/vgic.c index 068766e00..70ff62d84 100644 --- a/src/arch/armv8/vgic.c +++ b/src/arch/armv8/vgic.c @@ -406,7 +406,7 @@ bool vgic_add_lr(struct vcpu* vcpu, struct vgic_int* interrupt) } } - if (pend_found > 1) { + if (pend_found > 1 && pend_ind >= 0) { lr_ind = pend_ind; } else { lr_ind = act_ind; From d1d37cd0b7f6661a5a649883e5c43cf9777c06e8 Mon Sep 17 00:00:00 2001 From: Jose Martins Date: Mon, 31 Aug 2026 11:07:21 +0100 Subject: [PATCH 8/9] fix(vgic): do not track forwarded interrupts in spilled list When vgic_route() forwards an interrupt to another CPU it also yields ownership, so adding it to the spilled list as well would let a refill on a non-target vcpu take ownership and inject it there. Skip the spilled list fallback when the interrupt was forwarded, and arm NPIE only when an interrupt is actually queued. Signed-off-by: Jose Martins --- src/arch/armv8/vgic.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/arch/armv8/vgic.c b/src/arch/armv8/vgic.c index 70ff62d84..e4fa9b3c7 100644 --- a/src/arch/armv8/vgic.c +++ b/src/arch/armv8/vgic.c @@ -147,6 +147,10 @@ void vgic_send_sgi_msg(struct vcpu* vcpu, cpumap_t pcpu_mask, irqid_t int_id) } } +/** + * Track an interrupt that could not be placed in an LR so it gets injected + * when an LR slot becomes available. + */ static void vgic_add_spilled(struct vcpu* vcpu, struct vgic_int* interrupt) { spin_lock(&vcpu->vm->arch.vgic_spilled_lock); @@ -159,9 +163,9 @@ static void vgic_add_spilled(struct vcpu* vcpu, struct vgic_int* interrupt) } list_push(spilled_list, (node_t*)interrupt); interrupt->in_spilled = true; + gich_set_hcr(gich_get_hcr() | GICH_HCR_NPIE_BIT); } spin_unlock(&vcpu->vm->arch.vgic_spilled_lock); - gich_set_hcr(gich_get_hcr() | GICH_HCR_NPIE_BIT); } static void vgic_route(struct vcpu* vcpu, struct vgic_int* interrupt) @@ -188,7 +192,9 @@ static void vgic_route(struct vcpu* vcpu, struct vgic_int* interrupt) * cannot be yielded while the interrupt is active, so any IPI would * be silently ignored by the recipient. */ - if (!interrupt->in_lr && !(interrupt->state & ACT) && vgic_int_has_other_target(vcpu, interrupt)) { + bool forwarded = false; + if (!interrupt->in_lr && !(interrupt->state & ACT) && + vgic_int_has_other_target(vcpu, interrupt)) { union vgic_msg_data data = { .vm_id = (uint16_t)vcpu->vm->id, .vgicr_id = (uint16_t)vcpu->id, @@ -200,16 +206,12 @@ static void vgic_route(struct vcpu* vcpu, struct vgic_int* interrupt) for (size_t i = 0; i < platform.cpu_num; i++) { if (trgtlist & (1ULL << i)) { cpu_send_msg(i, &msg); + forwarded = true; } } } - /** - * If the interrupt was not placed in an LR or forwarded to another CPU, - * track it in the spilled list so it gets injected when an LR slot - * becomes available. - */ - if (!interrupt->in_lr && !interrupt->in_spilled) { + if (!interrupt->in_lr && !forwarded) { vgic_add_spilled(vcpu, interrupt); } } @@ -337,9 +339,8 @@ bool vgic_remove_lr(struct vcpu* vcpu, struct vgic_int* interrupt) /* Must be called holding vgic_spilled_lock */ static void vgic_remove_spilled(struct vcpu* vcpu, struct vgic_int* interrupt) { - struct list* spilled_list = gic_is_priv(interrupt->id) - ? &vcpu->arch.vgic_spilled - : &vcpu->vm->arch.vgic_spilled; + struct list* spilled_list = + gic_is_priv(interrupt->id) ? &vcpu->arch.vgic_spilled : &vcpu->vm->arch.vgic_spilled; list_rm(spilled_list, &interrupt->node); interrupt->in_spilled = false; } From 9f48d2ec24f8b5409277884bc67a8d582fa2bddb Mon Sep 17 00:00:00 2001 From: Jose Martins Date: Mon, 31 Aug 2026 12:35:40 +0100 Subject: [PATCH 9/9] fix(vgic): make spilled list walkers lock in canonical order The spilled list walkers took the VM wide vgic_spilled_lock and then the candidate interrupt's lock, while every other path holds the interrupt lock and takes the spilled lock inside it (vgic_add_lr, vgic_add_spilled). Two cpus can deadlock: one holds the spilled lock in vgic_refill_lrs spinning on an interrupt lock, while the other holds that interrupt lock in vgic_int_set_field spinning on the spilled lock in vgic_add_lr. Restructure vgic_refill_lrs and vgic_eoir_highest_spilled_active to pick the candidate under the spilled lock, release it, take the interrupt lock, retake the spilled lock, and revalidate that the candidate is still spilled and eligible before acting, rescanning otherwise. The inverted order in vgic_refill_lrs predates this branch; vgic_eoir_highest_spilled_active gained the same pattern in an earlier commit of this series. Signed-off-by: Jose Martins --- src/arch/armv8/vgic.c | 100 ++++++++++++++++++++++++++---------------- 1 file changed, 63 insertions(+), 37 deletions(-) diff --git a/src/arch/armv8/vgic.c b/src/arch/armv8/vgic.c index e4fa9b3c7..ecda75eae 100644 --- a/src/arch/armv8/vgic.c +++ b/src/arch/armv8/vgic.c @@ -1169,59 +1169,85 @@ static void vgic_refill_lrs(struct vcpu* vcpu, bool npie) uint64_t elrsr = gich_get_elrsr(); ssize_t lr_ind = bit64_ffs(elrsr & BIT64_MASK(0, NUM_LRS)); unsigned flags = npie ? PEND : ACT | PEND; - spin_lock(&vcpu->vm->arch.vgic_spilled_lock); while (lr_ind >= 0) { + spin_lock(&vcpu->vm->arch.vgic_spilled_lock); struct vgic_int* irq = vgic_highest_prio_spilled(vcpu, flags); - if (irq != NULL) { - spin_lock(&irq->lock); - bool got_ownership = vgic_get_ownership(vcpu, irq); - if (got_ownership) { - vgic_remove_spilled(vcpu, irq); - vgic_write_lr(vcpu, irq, (size_t)lr_ind); - } - spin_unlock(&irq->lock); - if (!got_ownership) { - continue; - } - } else { + if (irq == NULL) { uint32_t hcr = gich_get_hcr(); gich_set_hcr(hcr & ~(GICH_HCR_NPIE_BIT | GICH_HCR_UIE_BIT)); + spin_unlock(&vcpu->vm->arch.vgic_spilled_lock); break; } - flags = ACT | PEND; - elrsr = gich_get_elrsr(); - lr_ind = bit64_ffs(elrsr & BIT64_MASK(0, NUM_LRS)); + spin_unlock(&vcpu->vm->arch.vgic_spilled_lock); + + /** + * The candidate was picked without holding its lock, so between the + * list walk and locking it here it may have left the list or changed + * state. Revalidate under both locks, taken in the canonical order + * (interrupt lock first, spilled lock inside), and rescan if it is + * no longer an eligible candidate. + */ + spin_lock(&irq->lock); + spin_lock(&vcpu->vm->arch.vgic_spilled_lock); + bool eligible = irq->in_spilled && ((vgic_get_state(irq) & flags) != 0) && + vgic_get_ownership(vcpu, irq); + if (eligible) { + vgic_remove_spilled(vcpu, irq); + } + spin_unlock(&vcpu->vm->arch.vgic_spilled_lock); + /** + * Writing the LR may take the previous occupant's interrupt lock, so + * it must happen after the spilled lock is released to preserve the + * canonical lock order. + */ + if (eligible) { + vgic_write_lr(vcpu, irq, (size_t)lr_ind); + flags = ACT | PEND; + elrsr = gich_get_elrsr(); + lr_ind = bit64_ffs(elrsr & BIT64_MASK(0, NUM_LRS)); + } + spin_unlock(&irq->lock); } - spin_unlock(&vcpu->vm->arch.vgic_spilled_lock); } static void vgic_eoir_highest_spilled_active(struct vcpu* vcpu) { - struct vgic_int* interrupt; + bool done = false; - spin_lock(&vcpu->vm->arch.vgic_spilled_lock); - interrupt = vgic_highest_prio_spilled(vcpu, ACT); - if (interrupt != NULL) { - spin_lock(&interrupt->lock); - if (vgic_get_ownership(vcpu, interrupt)) { - vgic_remove_spilled(vcpu, interrupt); - } else { - spin_unlock(&interrupt->lock); - interrupt = NULL; - } - } - spin_unlock(&vcpu->vm->arch.vgic_spilled_lock); + while (!done) { + spin_lock(&vcpu->vm->arch.vgic_spilled_lock); + struct vgic_int* interrupt = vgic_highest_prio_spilled(vcpu, ACT); + spin_unlock(&vcpu->vm->arch.vgic_spilled_lock); - if (interrupt != NULL) { - interrupt->state &= (uint8_t)~ACT; - if (vgic_int_is_hw(interrupt)) { - gic_set_act(interrupt->id, false); + if (interrupt == NULL) { + done = true; } else { - if (interrupt->state & PEND) { - vgic_add_lr(vcpu, interrupt); + /** + * As in vgic_refill_lrs, the candidate was picked without holding + * its lock; revalidate it under both locks in the canonical order + * and rescan if it raced off the list. + */ + spin_lock(&interrupt->lock); + spin_lock(&vcpu->vm->arch.vgic_spilled_lock); + bool listed = interrupt->in_spilled && ((vgic_get_state(interrupt) & ACT) != 0); + bool eligible = listed && vgic_get_ownership(vcpu, interrupt); + if (eligible) { + vgic_remove_spilled(vcpu, interrupt); + } + spin_unlock(&vcpu->vm->arch.vgic_spilled_lock); + if (eligible) { + interrupt->state &= (uint8_t)~ACT; + if (vgic_int_is_hw(interrupt)) { + gic_set_act(interrupt->id, false); + } else { + if (interrupt->state & PEND) { + vgic_add_lr(vcpu, interrupt); + } + } } + spin_unlock(&interrupt->lock); + done = listed; } - spin_unlock(&interrupt->lock); } }