linux-kernel - Re: [PATCH] kvm: don't clear level from irqfd

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: <1340808117.1207.215.camel@bling.home>
Date:	Wed, 27 Jun 2012 08:41:57 -0600
From:	Alex Williamson <alex.williamson@...hat.com>
To:	"Michael S. Tsirkin" <mst@...hat.com>
Cc:	Gleb Natapov <gleb@...hat.com>, Avi Kivity <avi@...hat.com>,
	Marcelo Tosatti <mtosatti@...hat.com>, kvm@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] kvm: don't clear level from irqfd

On Wed, 2012-06-27 at 14:38 +0300, Michael S. Tsirkin wrote:
> On Wed, Jun 27, 2012 at 01:22:54PM +0300, Gleb Natapov wrote:
> > On Wed, Jun 27, 2012 at 01:20:24PM +0300, Michael S. Tsirkin wrote:
> > > On Wed, Jun 27, 2012 at 01:03:46PM +0300, Gleb Natapov wrote:
> > > > On Wed, Jun 27, 2012 at 12:13:43PM +0300, Michael S. Tsirkin wrote:
> > > > > irqfd sets and clears the level immediately.
> > > > > This used to be necessary for MSI interrupts but isn't anymore.
> > > > > 
> > > > > Note! We never officially supported irqfd for level
> > > > > interrupts but it usually seemed to kind of work,
> > > > > and this change will break it. There are no known
> > > > > real users of that feature though - only old vfio prototypes.
> > > > > Removing this line will make it easier to add
> > > > > real support for level irqfds in the future.
> > > > > 
> > > > Did we officially supported irqfd for ioapic/pic edge interrupts?
> > > > Because those will break to.
> > > 
> > > Hard to say - as Alex points out it was never documented.
> > If we will start dropping everything that was never documented we will
> > hardly have working VMM at the end :)
> 
> Generally true.
> 
> However I just realized that this interface does not work
> reliably for ioapic/pic edge either. More or less for the same reason that
> it does not work for edge: we set bit in IRR, try to inject,
> then clear even if injection failed (e.g. because it is low
> priority). Guest will never get an interrupt in this case.
> 
> So what happened e.g. for Alex's vfio prototype is he never
> hit an interrupt window in light testing.
> 
> What we would need to do to fix it is:
> > > > > -	kvm_set_irq(kvm, KVM_USERSPACE_IRQ_SOURCE_ID, irqfd->gsi, 1);
> > > > > -	kvm_set_irq(kvm, KVM_USERSPACE_IRQ_SOURCE_ID, irqfd->gsi, 0);
> > > > > +	kvm_set_irq(kvm, KVM_USERSPACE_IRQ_SOURCE_ID, irqfd->gsi, 0);
> > > > > +	kvm_set_irq(kvm, KVM_USERSPACE_IRQ_SOURCE_ID, irqfd->gsi, 1);
> 
> Which will fix edge but break level same as this patch.
> But is it worth fixing? Maybe declare it broken and be
> done with it?

Ugh, so if I understand correctly, the case where kvm_set_irq() returns
<0, we're broken.  VFIO has masked the physical device and has no error
path to know that the interrupt inject was unsuccessful.  We almost need
an error eventfd that bounces the interrupt back to userspace for the
qemu interrupt model to handle, but that has it's own set of
complications.  Suggestions?  Declaring the interface broken isn't very
useful.  Thanks,

Alex

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/