qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Alex Williamson <alex.williamson@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Xu <peterx@redhat.com>,
	qemu-devel@nongnu.org, Igor Mammedov <imammedo@redhat.com>,
	Eduardo Habkost <ehabkost@redhat.com>,
	"Michael S . Tsirkin" <mst@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v2 1/3] q35: set split kernel irqchip as default
Date: Tue, 30 Apr 2019 18:28:37 -0600	[thread overview]
Message-ID: <20190430182837.5af22e13@x1.home> (raw)
In-Reply-To: <9de36ff8-78ec-8cb0-6c23-4c8ecc1efad9@redhat.com>

On Wed, 1 May 2019 01:09:48 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 01/05/19 01:01, Alex Williamson wrote:
> > Poking at this a bit, we can add kvm_irqchip_is_split() to the set of
> > things we test for in hw/vfio/pci.c:vfio_intx_enable_kvm() to avoid the
> > KVM INTx bypass when using split IRQ chip.  
> 
> Yes, this should be enough.
> 
> > The only way I can get the GPU/Windows configuration usable is to
> > assert the IRQ, immediately de-assert, and unmask the device all from
> > vfio_intx_interrupt().  An interrupt intensive graphics benchmark runs
> > at ~80% of KVM irqchip with about 10% more CPU load with this
> > experiment (but it actually runs!).  
> 
> Nice.  If you can do it directly from hw/vfio there may be no need to do
> more changes to the IOAPIC, and least not immediately.  But that is not
> a good emulation of INTX, isn't it?  IIUC, it relies on the
> level-triggered interrupt never being masked in the IOAPIC.

Yeah, that's the problem, well one of the problems in addition to the
lower performance and increased overhead, is that it's a rather poor
emulation of INTx.  It matches pci_irq_pulse(), which has been
discouraged from use.  Anything but kernel irqchip makes me nervous for
INTx, but emulating it with a different physical IRQ type definitely
more so.

> > Any other insights appreciated, and I really would like to understand
> > what we've gained with split irqchip and whether it's worth this.  
> 
> We've gained guest interrupt remapping support, we are not relying on
> newer kernel versions, and the attack surface from the guest to the
> hypervisor is smaller.

Interrupt remapping is really only necessary with high vCPU counts or
maybe nested device assignment, seems doubtful that has a larger user
base.  I did re-watch the video of Steve Rutherford's talk from a
couple years ago, but he does re-iterate that pulling the ioapic from
KVM does have drawbacks for general purpose VMs that I thought would
have precluded it from becoming the default for the base QEMU machine
type.  Getting a GeForce card to run with MSI requires (repeated)
regedit'ing on Windows or module options on Linux.  The audio device
can require the same, otherwise we're probably mostly looking at old
devices assigned for compatibility using INTx, NICs or USB devices
would of course more likely use MSI/X for anything reasonably modern.
Thanks,

Alex

WARNING: multiple messages have this Message-ID (diff)
From: Alex Williamson <alex.williamson@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>,
	"Michael S . Tsirkin" <mst@redhat.com>,
	qemu-devel@nongnu.org, Peter Xu <peterx@redhat.com>,
	Eduardo Habkost <ehabkost@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v2 1/3] q35: set split kernel irqchip as default
Date: Tue, 30 Apr 2019 18:28:37 -0600	[thread overview]
Message-ID: <20190430182837.5af22e13@x1.home> (raw)
Message-ID: <20190501002837.7HiuE92BKWt7MtqLUyrE04m343WG6WRzS_-5dt7RYDc@z> (raw)
In-Reply-To: <9de36ff8-78ec-8cb0-6c23-4c8ecc1efad9@redhat.com>

On Wed, 1 May 2019 01:09:48 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 01/05/19 01:01, Alex Williamson wrote:
> > Poking at this a bit, we can add kvm_irqchip_is_split() to the set of
> > things we test for in hw/vfio/pci.c:vfio_intx_enable_kvm() to avoid the
> > KVM INTx bypass when using split IRQ chip.  
> 
> Yes, this should be enough.
> 
> > The only way I can get the GPU/Windows configuration usable is to
> > assert the IRQ, immediately de-assert, and unmask the device all from
> > vfio_intx_interrupt().  An interrupt intensive graphics benchmark runs
> > at ~80% of KVM irqchip with about 10% more CPU load with this
> > experiment (but it actually runs!).  
> 
> Nice.  If you can do it directly from hw/vfio there may be no need to do
> more changes to the IOAPIC, and least not immediately.  But that is not
> a good emulation of INTX, isn't it?  IIUC, it relies on the
> level-triggered interrupt never being masked in the IOAPIC.

Yeah, that's the problem, well one of the problems in addition to the
lower performance and increased overhead, is that it's a rather poor
emulation of INTx.  It matches pci_irq_pulse(), which has been
discouraged from use.  Anything but kernel irqchip makes me nervous for
INTx, but emulating it with a different physical IRQ type definitely
more so.

> > Any other insights appreciated, and I really would like to understand
> > what we've gained with split irqchip and whether it's worth this.  
> 
> We've gained guest interrupt remapping support, we are not relying on
> newer kernel versions, and the attack surface from the guest to the
> hypervisor is smaller.

Interrupt remapping is really only necessary with high vCPU counts or
maybe nested device assignment, seems doubtful that has a larger user
base.  I did re-watch the video of Steve Rutherford's talk from a
couple years ago, but he does re-iterate that pulling the ioapic from
KVM does have drawbacks for general purpose VMs that I thought would
have precluded it from becoming the default for the base QEMU machine
type.  Getting a GeForce card to run with MSI requires (repeated)
regedit'ing on Windows or module options on Linux.  The audio device
can require the same, otherwise we're probably mostly looking at old
devices assigned for compatibility using INTx, NICs or USB devices
would of course more likely use MSI/X for anything reasonably modern.
Thanks,

Alex


  parent reply	other threads:[~2019-05-01  0:28 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-20  5:40 [Qemu-devel] [PATCH v2 0/3] q35: change defaults for kernel irqchip and IR Peter Xu
2018-12-20  5:40 ` [Qemu-devel] [PATCH v2 1/3] q35: set split kernel irqchip as default Peter Xu
2018-12-20 12:13   ` Eduardo Habkost
2019-04-26 19:27   ` Alex Williamson
2019-04-26 19:27     ` Alex Williamson
2019-04-26 21:02     ` Alex Williamson
2019-04-26 21:02       ` Alex Williamson
2019-04-27  5:29       ` Paolo Bonzini
2019-04-27  5:29         ` Paolo Bonzini
2019-04-27  8:09         ` Paolo Bonzini
2019-04-27  8:09           ` Paolo Bonzini
2019-04-29 14:21           ` Alex Williamson
2019-04-29 14:21             ` Alex Williamson
2019-04-29 14:55             ` Eduardo Habkost
2019-04-29 14:55               ` Eduardo Habkost
2019-04-29 15:22               ` Alex Williamson
2019-04-29 15:22                 ` Alex Williamson
2019-05-03 18:42                 ` Eduardo Habkost
2019-05-03 18:42                   ` Eduardo Habkost
2019-05-05  9:06                   ` Peter Xu
2019-05-05  9:06                     ` Peter Xu
2019-05-06 16:13                     ` Paolo Bonzini
2019-05-06 21:16                       ` Eduardo Habkost
2019-05-03 20:00               ` Michael S. Tsirkin
2019-05-03 20:00                 ` Michael S. Tsirkin
2019-05-03 20:23                 ` Eduardo Habkost
2019-05-03 20:23                   ` Eduardo Habkost
2019-05-06 16:17                 ` Paolo Bonzini
2019-04-30 23:01             ` Alex Williamson
2019-04-30 23:01               ` Alex Williamson
2019-04-30 23:09               ` Paolo Bonzini
2019-04-30 23:09                 ` Paolo Bonzini
2019-05-01  0:28                 ` Alex Williamson [this message]
2019-05-01  0:28                   ` Alex Williamson
2018-12-20  5:40 ` [Qemu-devel] [PATCH v2 2/3] x86-iommu: switch intr_supported to OnOffAuto type Peter Xu
2018-12-20  5:40 ` [Qemu-devel] [PATCH v2 3/3] x86-iommu: turn on IR by default if proper Peter Xu
2018-12-20 10:00 ` [Qemu-devel] [PATCH v2 0/3] q35: change defaults for kernel irqchip and IR Paolo Bonzini
2018-12-20 10:16   ` Peter Xu

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=20190430182837.5af22e13@x1.home \
    --to=alex.williamson@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peterx@redhat.com \
    --cc=qemu-devel@nongnu.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).