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: <BL0PR11MB304290D4AACC3C1E2661C9668A659@BL0PR11MB3042.namprd11.prod.outlook.com>
Date:   Wed, 10 Aug 2022 17:17:48 +0000
From:   "Dong, Eddie" <eddie.dong@...el.com>
To:     Dmytro Maluka <dmy@...ihalf.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>,
        Marc Zyngier <maz@...nel.org>
Subject: RE: [PATCH v2 0/5] KVM: Fix oneshot interrupts forwarding

> >
> >
> >> 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."
> >>
> >
> > Emulation of level triggered IRQ is a pain point ☹ I read we need to
> > emulate the "level" of the IRQ pin (connecting from device to IRQchip, i.e.
> ioapic here).
> > Technically, the guest can change the polarity of vIOAPIC, which will
> > lead to a new  virtual IRQ even w/o host side interrupt.
> 
> Thanks, interesting point. Do you mean that this behavior (a new vIRQ as a
> result of polarity change) may already happen with the existing KVM code?
> 
> It doesn't seem so to me. AFAICT, KVM completely ignores the vIOAPIC polarity
> bit, in particular it doesn't handle change of the polarity by the guest (i.e.
> doesn't update the virtual IRR register, and so on), so it shouldn't result in a
> new interrupt.

Correct, KVM doesn't handle polarity now. Probably because unlikely the commercial OSes 
will change polarity.

> 
> Since commit 100943c54e09 ("kvm: x86: ignore ioapic polarity") there seems to
> be an assumption that KVM interpretes the IRQ level value as active (asserted)
> vs inactive (deasserted) rather than high vs low, i.e.

Asserted/deasserted vs. high/low is same to me, though asserted/deasserted hints more for event rather than state.

> the polarity doesn't matter to KVM.
> 
> So, since both sides (KVM emulating the IOAPIC, and vfio/whatever emulating
> an external interrupt source) seem to operate on a level of abstraction of
> "asserted" vs "de-asserted" interrupt state regardless of the polarity, and that
> seems not a bug but a feature, it seems that we don't need to emulate the IRQ
> level as such. Or am I missing something?

YES, I know current KVM doesn't handle it.  Whether we should support it is another story which I cannot speak for.
Paolo and Alex are the right person 😊
The reason I mention this is because the complexity to adding a pending event vs. supporting a interrupt pin state is same.
I am wondering if we need to revisit it or not.  Behavior closing to real hardware helps us to avoid potential issues IMO, but I am fine to either choice.

> 
> OTOH, I guess this means that the existing KVM's emulation of level-triggered
> interrupts is somewhat limited (a guest may legitimately expect an interrupt
> fired as a result of polarity change, and that case is not supported by KVM). But
> that is rather out of scope of the oneshot interrupts issue addressed by this
> patchset.

Agree.
I didn't know any commercial OSes change polarity either. But I know Xen hypervisor uses polarity under certain condition.
One day, we may see the issue when running Xen as a L1 hypervisor.  But this is not the current worry.


> 
> > "pending" field of kvm_kernel_irqfd_resampler in patch 3 means more an
> event rather than an interrupt level.

I know.  I am fine either.

Thanks Eddie

> >
> >