* [PATCH] KVM: x86: don't notify userspace IOAPIC on edge-triggered interrupt EOI
@ 2020-02-14 2:32 linmiaohe
2020-02-17 16:12 ` Vitaly Kuznetsov
0 siblings, 1 reply; 5+ messages in thread
From: linmiaohe @ 2020-02-14 2:32 UTC (permalink / raw)
To: pbonzini, rkrcmar, sean.j.christopherson, vkuznets, wanpengli,
jmattson, joro, tglx, mingo, bp, hpa
Cc: linmiaohe, kvm, linux-kernel, x86
From: Miaohe Lin <linmiaohe@huawei.com>
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@huawei.com>
---
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);
--
2.19.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] KVM: x86: don't notify userspace IOAPIC on edge-triggered interrupt EOI
2020-02-14 2:32 [PATCH] KVM: x86: don't notify userspace IOAPIC on edge-triggered interrupt EOI linmiaohe
@ 2020-02-17 16:12 ` Vitaly Kuznetsov
0 siblings, 0 replies; 5+ messages in thread
From: Vitaly Kuznetsov @ 2020-02-17 16:12 UTC (permalink / raw)
To: linmiaohe
Cc: kvm, linux-kernel, x86, pbonzini, rkrcmar, sean.j.christopherson,
wanpengli, jmattson, joro, tglx, mingo, bp, hpa
linmiaohe <linmiaohe@huawei.com> writes:
> From: Miaohe Lin <linmiaohe@huawei.com>
>
> 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@huawei.com>
> ---
> 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@redhat.com>
(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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] KVM: x86: don't notify userspace IOAPIC on edge-triggered interrupt EOI
@ 2020-02-19 1:39 linmiaohe
0 siblings, 0 replies; 5+ messages in thread
From: linmiaohe @ 2020-02-19 1:39 UTC (permalink / raw)
To: Vitaly Kuznetsov
Cc: kvm, linux-kernel, x86, pbonzini, rkrcmar, sean.j.christopherson,
wanpengli, jmattson, joro, tglx, mingo, bp, hpa
Vitaly Kuznetsov <vkuznets@redhat.com> writes:
>linmiaohe <linmiaohe@huawei.com> writes:
>
>> Vitaly Kuznetsov <vkuznets@redhat.com> writes:
>>>linmiaohe <linmiaohe@huawei.com> writes:
>>
>> Yeh, there may be a buggy userspace hidden from this unexpected EOI
>> notifications. It may not be worth enough to fix it as we may spend many time to catch the bug.
>> Perhaps we should only remove the pointless checking of irq->level for
>> cleanup. :)
>
>I'm feeling brave so in case nobody expresses any particular concerns let's just fix it :-)
>
Sounds good. :)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] KVM: x86: don't notify userspace IOAPIC on edge-triggered interrupt EOI
2020-02-18 1:41 linmiaohe
@ 2020-02-18 9:51 ` Vitaly Kuznetsov
0 siblings, 0 replies; 5+ messages in thread
From: Vitaly Kuznetsov @ 2020-02-18 9:51 UTC (permalink / raw)
To: linmiaohe
Cc: kvm, linux-kernel, x86, pbonzini, rkrcmar, sean.j.christopherson,
wanpengli, jmattson, joro, tglx, mingo, bp, hpa
linmiaohe <linmiaohe@huawei.com> writes:
> Vitaly Kuznetsov <vkuznets@redhat.com> writes:
>>linmiaohe <linmiaohe@huawei.com> writes:
>>
>>> @@ -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@redhat.com>
>
> Thanks for review.
>
>>
>> (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).
>>
>
> Yeh, there may be a buggy userspace hidden from this unexpected EOI notifications. It may not be worth enough to fix it as we may spend many time
> to catch the bug.
> Perhaps we should only remove the pointless checking of irq->level for cleanup. :)
I'm feeling brave so in case nobody expresses any particular concerns
let's just fix it :-)
--
Vitaly
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] KVM: x86: don't notify userspace IOAPIC on edge-triggered interrupt EOI
@ 2020-02-18 1:41 linmiaohe
2020-02-18 9:51 ` Vitaly Kuznetsov
0 siblings, 1 reply; 5+ messages in thread
From: linmiaohe @ 2020-02-18 1:41 UTC (permalink / raw)
To: Vitaly Kuznetsov
Cc: kvm, linux-kernel, x86, pbonzini, rkrcmar, sean.j.christopherson,
wanpengli, jmattson, joro, tglx, mingo, bp, hpa
Vitaly Kuznetsov <vkuznets@redhat.com> writes:
>linmiaohe <linmiaohe@huawei.com> writes:
>
>> @@ -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@redhat.com>
Thanks for review.
>
> (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).
>
Yeh, there may be a buggy userspace hidden from this unexpected EOI notifications. It may not be worth enough to fix it as we may spend many time
to catch the bug.
Perhaps we should only remove the pointless checking of irq->level for cleanup. :)
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-02-19 1:39 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-14 2:32 [PATCH] KVM: x86: don't notify userspace IOAPIC on edge-triggered interrupt EOI linmiaohe
2020-02-17 16:12 ` Vitaly Kuznetsov
2020-02-18 1:41 linmiaohe
2020-02-18 9:51 ` Vitaly Kuznetsov
2020-02-19 1:39 linmiaohe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).