2017-11-05 21:52 GMT+08:00 Nikita Leshenko <nikita.leshchenko@xxxxxxxxxx>:
> KVM uses ioapic_handled_vectors to track vectors that need to notify the
> IOAPIC on EOI. The problem is that IOAPIC can be reconfigured while an
> interrupt with old configuration is pending or running and
> ioapic_handled_vectors only remembers the newest configuration;
> thus EOI from the old interrupt is not delievered to the IOAPIC.
>
> A previous commit db2bdcbbbd32
> ("KVM: x86: fix edge EOI and IOAPIC reconfig race")
> addressed this issue by adding pending edge-triggered interrupts to
> ioapic_handled_vectors, fixing this race for edge-triggered interrupts.
> The commit explicitly ignored level-triggered interrupts,
> but this race applies to them as well:
>
> 1) IOAPIC sends a level triggered interrupt vector to VCPU0
> 2) VCPU0's handler deasserts the irq line and reconfigures the IOAPIC
> to route the vector to VCPU1. The reconfiguration rewrites only the
> upper 32 bits of the IOREDTBLn register. (Causes KVM to update
> ioapic_handled_vectors for VCPU0 and it no longer includes the vector.)
Refer to __ioapic_write_entry() in linux guest kernel codes, both
upper 32 bits and lower 32 bits are written to IOREDTBLx, so when the
scenario which you mentioned will occur?
Regards,
Wanpeng Li
> 3) VCPU0 sends EOI for the vector, but it's not delievered to the
> IOAPIC because the ioapic_handled_vectors doesn't include the vector.
> 4) New interrupts are not delievered to VCPU1 because remote_irr bit
> is set forever.
>
> Therefore, the correct behavior is to add all pending and running
> interrupts to ioapic_handled_vectors.
>
> This commit introduces a slight performance hit similar to
> commit db2bdcbbbd32 ("KVM: x86: fix edge EOI and IOAPIC reconfig race")
> for the rare case that the vector is reused by a non-IOAPIC source on
> VCPU0. We prefer to keep solution simple and not handle this case just
> as the original commit does.
>
> Fixes: db2bdcbbbd32 ("KVM: x86: fix edge EOI and IOAPIC reconfig race")
>
> Signed-off-by: Nikita Leshenko <nikita.leshchenko@xxxxxxxxxx>
> Reviewed-by: Liran Alon <liran.alon@xxxxxxxxxx>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> ---
> arch/x86/kvm/ioapic.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
> index bdff437acbcb..ae0a7dc318b2 100644
> --- a/arch/x86/kvm/ioapic.c
> +++ b/arch/x86/kvm/ioapic.c
> @@ -257,8 +257,7 @@ void kvm_ioapic_scan_entry(struct kvm_vcpu *vcpu, ulong *ioapic_handled_vectors)
> index == RTC_GSI) {
> if (kvm_apic_match_dest(vcpu, NULL, 0,
> e->fields.dest_id, e->fields.dest_mode) ||
> - (e->fields.trig_mode == IOAPIC_EDGE_TRIG &&
> - kvm_apic_pending_eoi(vcpu, e->fields.vector)))
> + kvm_apic_pending_eoi(vcpu, e->fields.vector))
> __set_bit(e->fields.vector,
> ioapic_handled_vectors);
> }
> --
> 2.13.3
>
