linux-kernel - Re: [PATCH v2 0/5] KVM: Fix oneshot interrupts forwarding

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c5d8f537-5695-42f0-88a9-de80e21f5f4c@semihalf.com>
Date:   Tue, 9 Aug 2022 09:24:28 +0200
From:   Dmytro Maluka <dmy@...ihalf.com>
To:     "Dong, Eddie" <eddie.dong@...el.com>,
        "Christopherson,, Sean" <seanjc@...gle.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Eric Auger <eric.auger@...hat.com>,
        Alex Williamson <alex.williamson@...hat.com>,
        "Liu, Rong L" <rong.l.liu@...el.com>,
        Zhenyu Wang <zhenyuw@...ux.intel.com>,
        Tomasz Nowicki <tn@...ihalf.com>,
        Grzegorz Jaszczyk <jaz@...ihalf.com>,
        "upstream@...ihalf.com" <upstream@...ihalf.com>,
        Dmitry Torokhov <dtor@...gle.com>
Subject: Re: [PATCH v2 0/5] KVM: Fix oneshot interrupts forwarding

On 8/9/22 1:26 AM, Dong, Eddie wrote:
>>
>> The existing KVM mechanism for forwarding of level-triggered interrupts using
>> resample eventfd doesn't work quite correctly in the case of interrupts that are
>> handled in a Linux guest as oneshot interrupts (IRQF_ONESHOT). Such an
>> interrupt is acked to the device in its threaded irq handler, i.e. later than it is
>> acked to the interrupt controller (EOI at the end of hardirq), not earlier. The
>> existing KVM code doesn't take that into account, which results in erroneous
>> extra interrupts in the guest caused by premature re-assert of an
>> unacknowledged IRQ by the host.
> 
> Interesting...  How it behaviors in native side? 

In native it behaves correctly, since Linux masks such a oneshot
interrupt at the beginning of hardirq, so that the EOI at the end of
hardirq doesn't result in its immediate re-assert, and then unmasks it
later, after its threaded irq handler completes.

In handle_fasteoi_irq():

	if (desc->istate & IRQS_ONESHOT)
		mask_irq(desc);

	handle_irq_event(desc);

	cond_unmask_eoi_irq(desc, chip);


and later in unmask_threaded_irq():

	unmask_irq(desc);

I also mentioned that in patch #3 description:
"Linux keeps such interrupt masked until its threaded handler finishes,
to prevent the EOI from re-asserting an unacknowledged interrupt.
However, with KVM + vfio (or whatever is listening on the resamplefd)
we don't check that the interrupt is still masked in the guest at the
moment of EOI. Resamplefd is notified regardless, so vfio prematurely
unmasks the host physical IRQ, thus a new (unwanted) physical interrupt
is generated in the host and queued for injection to the guest."

> 
>>
>> This patch series fixes this issue (for now on x86 only) by checking if the
>> interrupt is unmasked when we receive irq ack (EOI) and, in case if it's masked,
>> postponing resamplefd notify until the guest unmasks it.
>>
>> Patches 1 and 2 extend the existing support for irq mask notifiers in KVM,
>> which is a prerequisite needed for KVM irqfd to use mask notifiers to know
>> when an interrupt is masked or unmasked.
>>
>> Patch 3 implements the actual fix: postponing resamplefd notify in irqfd until
>> the irq is unmasked.
>>
>> Patches 4 and 5 just do some optional renaming for consistency, as we are now
>> using irq mask notifiers in irqfd along with irq ack notifiers.
>>
>> Please see individual patches for more details.
>>
>> v2:
>>   - Fixed compilation failure on non-x86: mask_notifier_list moved from
>>     x86 "struct kvm_arch" to generic "struct kvm".
>>   - kvm_fire_mask_notifiers() also moved from x86 to generic code, even
>>     though it is not called on other architectures for now.
>>   - Instead of kvm_irq_is_masked() implemented
>>     kvm_register_and_fire_irq_mask_notifier() to fix potential race
>>     when reading the initial IRQ mask state.
>>   - Renamed for clarity:
>>       - irqfd_resampler_mask() -> irqfd_resampler_mask_notify()
>>       - kvm_irq_has_notifier() -> kvm_irq_has_ack_notifier()
>>       - resampler->notifier -> resampler->ack_notifier
>>   - Reorganized code in irqfd_resampler_ack() and
>>     irqfd_resampler_mask_notify() to make it easier to follow.
>>   - Don't follow unwanted "return type on separate line" style for
>>     irqfd_resampler_mask_notify().
>>
>> Dmytro Maluka (5):
>>   KVM: x86: Move irq mask notifiers from x86 to generic KVM
>>   KVM: x86: Add kvm_register_and_fire_irq_mask_notifier()
>>   KVM: irqfd: Postpone resamplefd notify for oneshot interrupts
>>   KVM: irqfd: Rename resampler->notifier
>>   KVM: Rename kvm_irq_has_notifier()
>>
>>  arch/x86/include/asm/kvm_host.h |  17 +---
>>  arch/x86/kvm/i8259.c            |   6 ++
>>  arch/x86/kvm/ioapic.c           |   8 +-
>>  arch/x86/kvm/ioapic.h           |   1 +
>>  arch/x86/kvm/irq_comm.c         |  74 +++++++++++------
>>  arch/x86/kvm/x86.c              |   1 -
>>  include/linux/kvm_host.h        |  21 ++++-
>>  include/linux/kvm_irqfd.h       |  16 +++-
>>  virt/kvm/eventfd.c              | 136 ++++++++++++++++++++++++++++----
>>  virt/kvm/kvm_main.c             |   1 +
>>  10 files changed, 221 insertions(+), 60 deletions(-)
>>
>> --
>> 2.37.1.559.g78731f0fdb-goog
>