Re: [PATCH] KVM: x86: don't notify userspace IOAPIC on edge-triggered interrupt EOI

[Vitaly Kuznetsov]

linmiaohe <linmiaohe@xxxxxxxxxx> writes:

> From: Miaohe Lin <linmiaohe@xxxxxxxxxx>
>
> Commit 13db77347db1 ("KVM: x86: don't notify userspace IOAPIC on edge
> EOI") said, edge-triggered interrupts don't set a bit in TMR, which means
> that IOAPIC isn't notified on EOI. And var level indicates level-triggered
> interrupt.
> But commit 3159d36ad799 ("KVM: x86: use generic function for MSI parsing")
> replace var level with irq.level by mistake. Fix it by changing irq.level
> to irq.trig_mode.
>
> Fixes: 3159d36ad799 ("KVM: x86: use generic function for MSI parsing")
> Signed-off-by: Miaohe Lin <linmiaohe@xxxxxxxxxx>
> ---
>  arch/x86/kvm/irq_comm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/irq_comm.c b/arch/x86/kvm/irq_comm.c
> index 79afa0bb5f41..c47d2acec529 100644
> --- a/arch/x86/kvm/irq_comm.c
> +++ b/arch/x86/kvm/irq_comm.c
> @@ -417,7 +417,7 @@ void kvm_scan_ioapic_routes(struct kvm_vcpu *vcpu,
>  
>  			kvm_set_msi_irq(vcpu->kvm, entry, &irq);
>  
> -			if (irq.level &&
> +			if (irq.trig_mode &&
>  			    kvm_apic_match_dest(vcpu, NULL, APIC_DEST_NOSHORT,
>  						irq.dest_id, irq.dest_mode))
>  				__set_bit(irq.vector, ioapic_handled_vectors);

Assuming Radim's comment (13db77347db1) is correct, the change in
3159d36ad799 looks wrong and your patch restores the status
quo. Actually, kvm_set_msi_irq() always sets irq->level = 1 so checking
it is pointless.

Reviewed-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>

(but it is actually possible that there's a buggy userspace out there
which expects EOI notifications; we won't find out unless we try to fix
the bug).

-- 
Vitaly