kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: avi@redhat.com, gleb@redhat.com, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v8 0/6] kvm: level irqfd support
Date: Wed, 15 Aug 2012 22:22:24 +0300	[thread overview]
Message-ID: <20120815192224.GB5670@redhat.com> (raw)
In-Reply-To: <1345052191.4683.435.camel@ul30vt.home>

On Wed, Aug 15, 2012 at 11:36:31AM -0600, Alex Williamson wrote:
> On Wed, 2012-08-15 at 17:28 +0300, Michael S. Tsirkin wrote:
> > On Fri, Aug 10, 2012 at 04:37:08PM -0600, Alex Williamson wrote:
> > > v8:
> > > 
> > > Trying a new approach.  Nobody seems to like the internal IRQ
> > > source ID object and the interactions it implies between irqfd
> > > and eoifd, so let's get rid of it.  Instead, simply expose
> > > IRQ source IDs to userspace.  This lets the user be in charge
> > > of freeing them or hanging onto a source ID for later use.
> > 
> > In the end it turns out source ID is an optimization for shared
> > interrupts, isn't it?  Can't we apply the optimization transparently to
> > the user?  E.g. if we have some spare source IDs, allocate them, if we
> > run out, use a shared source ID?
> 
> Let's think about shared source IDs a bit more.  I think it's wrong that
> irqfd uses KVM_USERSPACE_IRQ_SOURCE_ID, but I'm questioning whether all
> irqfd users can share a source ID.  We do not get the logical OR of all
> users by putting them on the same source ID, we get "last set wins".
> KVM_USERSPACE_IRQ_SOURCE_ID is used for multiple inputs because the
> logical OR happens in userspace.  How would we not starve a user if we
> define KVM_IRQFD_SOURCE_ID?  What am I missing?

That all irqfds are deasserted on EOI anyway.  So there's no point
to do a logical OR.


> 
> So I'm inclined to say source IDs are a requirement for shared
> interrupts.

Can yo show a specific example that breaks?
I don't think it can exist.

> That means the re-use scheme becomes complicated (ex. we
> run out of IRQ source IDs, so we start looking for sharing by re-using a
> source ID used by a different GSI).  Do we want to do that in kernel or
> userspace?  This series allows userspace to deal with that complexity.
> Please let me know if I'm thinking incorrectly about source ID re-use.
> Thanks,
> 
> Alex

I think there is a misunderstanding.
All deassert on ack irqfds can share a source ID.
This is why I am now thinking deassert on ack behaviour
should be set when irqfd is assigned.


> 
> > > They
> > > can also detach and re-attach components at will.  It also opens
> > > up the possibility that userspace might want to use each IRQ
> > > source ID for more than one GSI (and avoids the kernel needing
> > > to manage that).  Per suggestions, EOIFD is now IRQ_ACKFD.
> > > 
> > > I really wanted to add a de-assert-only option to irqfd so the
> > > irq_ackfd could be fed directly into an irqfd, but I'm dependent
> > > on the ordering of de-assert _then_ signal an eventfd.  Keeping
> > > that ordering doesn't seem to be possible, especially since irqfd
> > > uses a workqueue, if I attempt to make that connection.  Thanks,
> > > 
> > > Alex
> > > ---
> > > 
> > > Alex Williamson (6):
> > >       kvm: Add de-assert option to KVM_IRQ_ACKFD
> > >       kvm: KVM_IRQ_ACKFD
> > >       kvm: Add assert-only option to KVM_IRQFD
> > >       kvm: Add IRQ source ID option to KVM_IRQFD
> > >       kvm: Expose IRQ source IDs to userspace
> > >       kvm: Allow filtering of acked irqs
> > > 
> > > 
> > >  Documentation/virtual/kvm/api.txt |   53 ++++++
> > >  arch/x86/kvm/Kconfig              |    1 
> > >  arch/x86/kvm/i8254.c              |    1 
> > >  arch/x86/kvm/i8259.c              |    8 +
> > >  arch/x86/kvm/x86.c                |    8 +
> > >  include/linux/kvm.h               |   39 ++++-
> > >  include/linux/kvm_host.h          |   18 ++
> > >  virt/kvm/Kconfig                  |    3 
> > >  virt/kvm/assigned-dev.c           |    1 
> > >  virt/kvm/eventfd.c                |  315 +++++++++++++++++++++++++++++++++++++
> > >  virt/kvm/ioapic.c                 |    5 -
> > >  virt/kvm/irq_comm.c               |   28 +++
> > >  virt/kvm/kvm_main.c               |   26 +++
> > >  13 files changed, 496 insertions(+), 10 deletions(-)
> 
> 

  reply	other threads:[~2012-08-15 19:22 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-10 22:37 [PATCH v8 0/6] kvm: level irqfd support Alex Williamson
2012-08-10 22:37 ` [PATCH v8 1/6] kvm: Allow filtering of acked irqs Alex Williamson
2012-08-15 12:27   ` Michael S. Tsirkin
2012-08-15 16:47     ` Alex Williamson
2012-08-15 19:24       ` Michael S. Tsirkin
2012-08-10 22:37 ` [PATCH v8 2/6] kvm: Expose IRQ source IDs to userspace Alex Williamson
2012-08-15 12:59   ` Michael S. Tsirkin
2012-08-15 17:05     ` Alex Williamson
2012-08-10 22:37 ` [PATCH v8 3/6] kvm: Add IRQ source ID option to KVM_IRQFD Alex Williamson
2012-08-15 13:49   ` Michael S. Tsirkin
2012-08-15 17:08     ` Alex Williamson
2012-08-10 22:37 ` [PATCH v8 4/6] kvm: Add assert-only " Alex Williamson
2012-08-10 22:37 ` [PATCH v8 5/6] kvm: KVM_IRQ_ACKFD Alex Williamson
2012-08-15 14:05   ` Michael S. Tsirkin
2012-08-15 17:17     ` Alex Williamson
2012-08-10 22:37 ` [PATCH v8 6/6] kvm: Add de-assert option to KVM_IRQ_ACKFD Alex Williamson
2012-08-15 14:11   ` Michael S. Tsirkin
2012-08-15 17:24     ` Alex Williamson
2012-08-15 14:28 ` [PATCH v8 0/6] kvm: level irqfd support Michael S. Tsirkin
2012-08-15 17:36   ` Alex Williamson
2012-08-15 19:22     ` Michael S. Tsirkin [this message]
2012-08-15 19:59       ` Alex Williamson
2012-08-16 12:34         ` Alex Williamson
2012-08-16 12:53           ` Michael S. Tsirkin
2012-08-16 16:29       ` Avi Kivity
2012-08-16 16:36         ` Michael S. Tsirkin
2012-08-16 16:39           ` Avi Kivity
2012-08-16 16:54             ` Michael S. Tsirkin
2012-08-16 16:54               ` Avi Kivity
2012-08-16 17:01                 ` Michael S. Tsirkin
2012-08-16 16:37         ` Alex Williamson
2012-08-16 16:32 ` Avi Kivity
2012-08-16 16:45   ` Alex Williamson

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=20120815192224.GB5670@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=linux-kernel@vger.kernel.org \
    /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 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).