From: "Michael S. Tsirkin" <mst@redhat.com>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: Gleb Natapov <gleb@redhat.com>, Avi Kivity <avi@redhat.com>,
Marcelo Tosatti <mtosatti@redhat.com>,
kvm@vger.kernel.org
Subject: Re: [PATCH RFC untested] kvm_set_irq: report coalesced for clear
Date: Thu, 19 Jul 2012 19:51:23 +0300 [thread overview]
Message-ID: <20120719165123.GA17213@redhat.com> (raw)
In-Reply-To: <1342715883.3142.17.camel@ul30vt>
On Thu, Jul 19, 2012 at 10:38:03AM -0600, Alex Williamson wrote:
> Yes, the problem isn't the state. The original patch works just fine to
> mask and assert the interrupt every time the device signals and
> de-assert and unmask on every EOI. KVM doesn't need to track this for
> migration (not that we support migration, of course), we can always just
> send an unmask to the device to retrigger an interrupt if needed.
I agree, just let's document this in case emulated devices use EOIFD.
> The thing Michael is trying to avoid is spurious assertions and
> de-assertions by tracking the state machine. Spurious assertions are
> not really a problem, at least for vfio where the interrupt is masked
> until kvm/qemu tells us to unmask it. So at any point in time we can
> reset the state machine with an unmask. Spurious unmasks are
> theoretically a problem if an IRQ is shared among multiple devices we
> can trigger unmasks for devices that haven't been asserted. vfio
> handles this pretty well though and recognizes the device isn't masked
> and does nothing.
>
> Something I note out of this discussion is that while the spinlock I use
> to maintain the state machine is ugly, the lock has no contention.
Good point. Overall I'm convinced now it was a good idea.
> I don't think that's necessarily the case with pic_lock. Anyway, I think
> we can do w/o the spinlock altogether. Lock contention and spurious
> eois over level triggered interrupts is probably not worth worrying
> about.
Thanks, good summary.
I think I can prove to you spurious wakeups are a problem though:
consider a setup where an IRQ is shared with a userspace device (e.g.
console). If said userspace uses EOIFD you wake up userspace on each
interrupt of an assigned device which is exactly what level IRQFD was
designed to avoid.
--
MST
prev parent reply other threads:[~2012-07-19 16:50 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-07-18 22:11 [PATCH RFC untested] kvm_set_irq: report coalesced for clear Michael S. Tsirkin
2012-07-18 22:40 ` Alex Williamson
2012-07-18 22:51 ` Michael S. Tsirkin
2012-07-19 7:53 ` Gleb Natapov
2012-07-19 9:17 ` Michael S. Tsirkin
2012-07-19 9:21 ` Gleb Natapov
2012-07-19 9:33 ` Michael S. Tsirkin
2012-07-19 9:41 ` Gleb Natapov
2012-07-19 10:26 ` Michael S. Tsirkin
2012-07-19 10:54 ` Gleb Natapov
2012-07-19 11:12 ` Michael S. Tsirkin
2012-07-19 11:18 ` Michael S. Tsirkin
2012-07-19 11:25 ` Gleb Natapov
2012-07-19 11:57 ` Michael S. Tsirkin
2012-07-19 16:38 ` Alex Williamson
2012-07-19 16:51 ` Michael S. Tsirkin [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20120719165123.GA17213@redhat.com \
--to=mst@redhat.com \
--cc=alex.williamson@redhat.com \
--cc=avi@redhat.com \
--cc=gleb@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=mtosatti@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.