Skip to content

Commit 0e3dfda

Browse files
committed
KVM: arm/arm64: arch_timer: Preserve physical dist. active state on LR.active
We were incorrectly removing the active state from the physical distributor on the timer interrupt when the timer output level was deasserted. We shouldn't be doing this without considering the virtual interrupt's active state, because the architecture requires that when an LR has the HW bit set and the pending or active bits set, then the physical interrupt must also have the corresponding bits set. This addresses an issue where we have been observing an inconsistency between the LR state and the physical distributor state where the LR state was active and the physical distributor was not active, which shouldn't happen. Reviewed-by: Marc Zyngier <marc.zyngier@arm.com> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
1 parent 7e16aa8 commit 0e3dfda

3 files changed

Lines changed: 40 additions & 24 deletions

File tree

include/kvm/arm_vgic.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -342,10 +342,10 @@ int kvm_vgic_inject_mapped_irq(struct kvm *kvm, int cpuid,
342342
struct irq_phys_map *map, bool level);
343343
void vgic_v3_dispatch_sgi(struct kvm_vcpu *vcpu, u64 reg);
344344
int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vcpu);
345-
int kvm_vgic_vcpu_active_irq(struct kvm_vcpu *vcpu);
346345
struct irq_phys_map *kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu,
347346
int virt_irq, int irq);
348347
int kvm_vgic_unmap_phys_irq(struct kvm_vcpu *vcpu, struct irq_phys_map *map);
348+
bool kvm_vgic_map_is_active(struct kvm_vcpu *vcpu, struct irq_phys_map *map);
349349

350350
#define irqchip_in_kernel(k) (!!((k)->arch.vgic.in_kernel))
351351
#define vgic_initialized(k) (!!((k)->arch.vgic.nr_cpus))

virt/kvm/arm/arch_timer.c

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -221,17 +221,23 @@ void kvm_timer_flush_hwstate(struct kvm_vcpu *vcpu)
221221
kvm_timer_update_state(vcpu);
222222

223223
/*
224-
* If we enter the guest with the virtual input level to the VGIC
225-
* asserted, then we have already told the VGIC what we need to, and
226-
* we don't need to exit from the guest until the guest deactivates
227-
* the already injected interrupt, so therefore we should set the
228-
* hardware active state to prevent unnecessary exits from the guest.
229-
*
230-
* Conversely, if the virtual input level is deasserted, then always
231-
* clear the hardware active state to ensure that hardware interrupts
232-
* from the timer triggers a guest exit.
233-
*/
234-
if (timer->irq.level)
224+
* If we enter the guest with the virtual input level to the VGIC
225+
* asserted, then we have already told the VGIC what we need to, and
226+
* we don't need to exit from the guest until the guest deactivates
227+
* the already injected interrupt, so therefore we should set the
228+
* hardware active state to prevent unnecessary exits from the guest.
229+
*
230+
* Also, if we enter the guest with the virtual timer interrupt active,
231+
* then it must be active on the physical distributor, because we set
232+
* the HW bit and the guest must be able to deactivate the virtual and
233+
* physical interrupt at the same time.
234+
*
235+
* Conversely, if the virtual input level is deasserted and the virtual
236+
* interrupt is not active, then always clear the hardware active state
237+
* to ensure that hardware interrupts from the timer triggers a guest
238+
* exit.
239+
*/
240+
if (timer->irq.level || kvm_vgic_map_is_active(vcpu, timer->map))
235241
phys_active = true;
236242
else
237243
phys_active = false;

virt/kvm/arm/vgic.c

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1096,6 +1096,27 @@ static void vgic_retire_lr(int lr_nr, struct kvm_vcpu *vcpu)
10961096
vgic_set_lr(vcpu, lr_nr, vlr);
10971097
}
10981098

1099+
static bool dist_active_irq(struct kvm_vcpu *vcpu)
1100+
{
1101+
struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
1102+
1103+
return test_bit(vcpu->vcpu_id, dist->irq_active_on_cpu);
1104+
}
1105+
1106+
bool kvm_vgic_map_is_active(struct kvm_vcpu *vcpu, struct irq_phys_map *map)
1107+
{
1108+
int i;
1109+
1110+
for (i = 0; i < vcpu->arch.vgic_cpu.nr_lr; i++) {
1111+
struct vgic_lr vlr = vgic_get_lr(vcpu, i);
1112+
1113+
if (vlr.irq == map->virt_irq && vlr.state & LR_STATE_ACTIVE)
1114+
return true;
1115+
}
1116+
1117+
return dist_active_irq(vcpu);
1118+
}
1119+
10991120
/*
11001121
* An interrupt may have been disabled after being made pending on the
11011122
* CPU interface (the classic case is a timer running while we're
@@ -1248,7 +1269,7 @@ static void __kvm_vgic_flush_hwstate(struct kvm_vcpu *vcpu)
12481269
* may have been serviced from another vcpu. In all cases,
12491270
* move along.
12501271
*/
1251-
if (!kvm_vgic_vcpu_pending_irq(vcpu) && !kvm_vgic_vcpu_active_irq(vcpu))
1272+
if (!kvm_vgic_vcpu_pending_irq(vcpu) && !dist_active_irq(vcpu))
12521273
goto epilog;
12531274

12541275
/* SGIs */
@@ -1479,17 +1500,6 @@ int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vcpu)
14791500
return test_bit(vcpu->vcpu_id, dist->irq_pending_on_cpu);
14801501
}
14811502

1482-
int kvm_vgic_vcpu_active_irq(struct kvm_vcpu *vcpu)
1483-
{
1484-
struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
1485-
1486-
if (!irqchip_in_kernel(vcpu->kvm))
1487-
return 0;
1488-
1489-
return test_bit(vcpu->vcpu_id, dist->irq_active_on_cpu);
1490-
}
1491-
1492-
14931503
void vgic_kick_vcpus(struct kvm *kvm)
14941504
{
14951505
struct kvm_vcpu *vcpu;

0 commit comments

Comments
 (0)