Re: [Qemu-devel] [PATCH v6 00/16] KVM platform device passthrough

[Alex Williamson]

On Tue, 2014-09-16 at 14:51 -0600, Alex Williamson wrote:
> On Tue, 2014-09-16 at 00:01 +0200, Eric Auger wrote:
> > On 09/12/2014 01:05 AM, Christoffer Dall wrote:
> > > On Thu, Sep 11, 2014 at 04:51:14PM -0600, Alex Williamson wrote:
> > >> On Thu, 2014-09-11 at 15:23 -0700, Christoffer Dall wrote:
> > >>> On Thu, Sep 11, 2014 at 04:14:09PM -0600, Alex Williamson wrote:
> > >>>> On Tue, 2014-09-09 at 08:31 +0100, Eric Auger wrote:
> > >>>>> This RFC series aims at enabling KVM platform device passthrough.
> > >>>>> It implements a VFIO platform device, derived from VFIO PCI device.
> > >>>>>
> > >>>>> The VFIO platform device uses the host VFIO platform driver which must
> > >>>>> be bound to the assigned device prior to the QEMU system start.
> > >>>>>
> > >>>>> - the guest can directly access the device register space
> > >>>>> - assigned device IRQs are transparently routed to the guest by
> > >>>>>   QEMU/KVM (3 methods currently are supported: user-level eventfd
> > >>>>>   handling, irqfd, forwarded IRQs)
> > >>>>> - iommu is transparently programmed to prevent the device from
> > >>>>>   accessing physical pages outside of the guest address space
> > >>>>>
> > >>>>> This patch series is made of the following patch files:
> > >>>>>
> > >>>>> 1-7) Modifications to PCI code to prepare for VFIO platform device
> > >>>>> 8) split of PCI specific code and generic code (move)
> > >>>>> 9-11) creation of the VFIO calxeda xgmac platform device, without 
> > >>>>> irqfd
> > >>>>>       support (MMIO direct access and IRQ assignment).
> > >>>>> 12) fake injection test modality (to test multiple IRQ)
> > >>>>> 13) addition of irqfd/virqfd support
> > >>>>> 14-16) forwarded IRQ
> > >>>>>
> > >>>>> Dependency List:
> > >>>>>
> > >>>>> QEMU dependencies:
> > >>>>> [1] [PATCH v2 0/9] Dynamic sysbus device allocation support, Alex Graf
> > >>>>>     http://lists.gnu.org/archive/html/qemu-ppc/2014-07/msg00047.html
> > >>>>> [2] [RFC v3] machvirt dynamic sysbus device instantiation, Eric Auger
> > >>>>> [3] [PATCH v2 0/2] actual checks of KVM_CAP_IRQFD and 
> > >>>>> KVM_CAP_IRQFD_RESAMPLE,
> > >>>>>     Eric Auger
> > >>>>>     
> > >>>>> http://lists.nongnu.org/archive/html/qemu-devel/2014-09/msg00589.html
> > >>>>> [4] [RFC] vfio: migration to trace points, Eric Auger
> > >>>>>     
> > >>>>> http://lists.nongnu.org/archive/html/qemu-devel/2014-09/msg00569.html
> > >>>>>
> > >>>>> Kernel Dependencies:
> > >>>>> [5] [RFC Patch v6 0/20] VFIO support for platform devices, Antonios 
> > >>>>> Motakis
> > >>>>>     https://www.mail-archive.com/address@hidden/msg103247.html
> > >>>>> [6] [PATCH v3] ARM: KVM: add irqfd support, Eric Auger
> > >>>>>     https://lkml.org/lkml/2014/9/1/141
> > >>>>> [7] arm/arm64: KVM: Various VGIC cleanups and improvements, 
> > >>>>> Christoffer Dall
> > >>>>>     http://comments.gmane.org/gmane.linux.ports.arm.kernel/340430
> > >>>>> [8] [RFC v2 0/9] KVM-VFIO IRQ forward control, Eric Auger
> > >>>>>     https://lkml.org/lkml/2014/9/1/344
> > >>>>> [9] [RFC PATCH 0/9] ARM: Forwarding physical interrupts to a guest VM,
> > >>>>>     Marc Zyngier
> > >>>>>     http://lwn.net/Articles/603514/
> > >>>>>
> > >>>>> kernel pieces can be found at:
> > >>>>> http://git.linaro.org/people/eric.auger/linux.git
> > >>>>> (branch 3.17rc3_irqfd_forward_integ_v2)
> > >>>>> QEMU pieces can be found at:
> > >>>>> http://git.linaro.org/people/eric.auger/qemu.git (branch 
> > >>>>> vfio_integ_v6)
> > >>>>>
> > >>>>> The patch series was tested on Calxeda Midway (ARMv7) where one xgmac
> > >>>>> is assigned to KVM host while the second one is assigned to the guest.
> > >>>>> Reworked PCI device is not tested.
> > >>>>>
> > >>>>> Wiki for Calxeda Midway setup:
> > >>>>> https://wiki.linaro.org/LEG/Engineering/Virtualization/Platform_Device_Passthrough_on_Midway
> > >>>>>
> > >>>>> History:
> > >>>>>
> > >>>>> v5->v6:
> > >>>>> - rebase on 2.1rc5 PCI code
> > >>>>> - forwarded IRQ first integraton
> > >>>>
> > >>>> Why?  Are there acceleration paths that you're concerned cannot be
> > >>>> implemented or we do not already have a proof of concept for?  The base
> > >>>> kernel patch series you depend on is 3 months old yet this series
> > >>>> continues to grow and add new dependencies.  Please let's prioritize
> > >>>> getting something upstream instead of adding more blockers to prevent
> > >>>> that.  Thanks,
> > >>>>
> > >>> I'm not exactly sure what this changelog line was referring to
> > >>> (depending on Marc's forwarding IRQ patches?), but just want to add that
> > >>> there are a number of dependencies for the GIC that need to go in as
> > >>> well (should happen within a few weeks), but I think it's unlikely that
> > >>> the IRQ forwarding stuff goes in for v3.18 at this point.
> > >>>
> > >>> It may make sense as you suggest to keep that part out of this patch set
> > >>> and something merged sooner as opposed to later, but I'm too jet-lagged
> > >>> to completely understand if that's going to be a horrible mess.
> > >>
> > >> The point is that we're on v6 of a patch series and its first non-RFC
> > >> posting and we're rolling in a first pass at a QEMU implementation that
> > >> depends on a contested kernel RFC, which depends on another stagnant
> > >> kernel RFC.  I'm fine with working on it in parallel, but give me some
> > >> light at the end of the tunnel as a reviewer and maintainer that this
> > >> code isn't going to live indefinitely on the mailing list.  Do we really
> > >> need those GIC patches do be able to have non-KVM accelerated VFIO
> > >> platform device assignment?  We certainly don't need IRQ forwarding.
> > >> Thanks,
> > 
> > Hi Alex,
> > 
> > Sorry for the delay, I was travelling.
> > 
> > I understand your impatience. I personally would be happy if we could
> > envision upstreaming this patch in several steps. Let me know if it
> > makes sense.
> > 
> > STEP I:  integrate 1 - 11: leads to have a non-KVM accelerated VFIO QEMU
> > device. 12 can be part of it too but since it is a test feature this one
> > might be dropped. just let me know what you think.
> 
> I'd probably drop 12.  Is that really something that's useful in
> upstream code?  It's a good use of the vfio loopback interrupt and good
> testing, but do you really want to maintain it in the code?  Is it
> sufficient that it's been posted to the mailing list so you can find and
> re-apply it if you want to do similar testing again?
> 
> > depends on:
> > QEMU:
> > [1] [PATCH v2 0/9] Dynamic sysbus device allocation support, A. Graf
> > http://lists.gnu.org/archive/html/qemu-ppc/2014-07/msg00047.html
> > [2] [RFC v3] machvirt dynamic sysbus device instantiation, E. Auger
> > [4] [RFC] vfio: migration to trace points, E. Auger
> > http://lists.nongnu.org/archive/html/qemu-devel/2014-09/msg00569.html
> > KERNEL:
> > [5] [RFC Patch v6 0/20] VFIO support for platform devices, A. Motakis
> > https://www.mail-archive.com/address@hidden/msg103247.html
> 
> Ok, so let's start whittling down these dependencies.  Trace points
> shouldn't be any kind of blocker, you'll just need to teach me how to
> use them and post a non-RFC patch ;)  At this point I don't even
> remember the comments for the v6 VFIO kernel support for platform
> devices.  I hope we're close enough that the next version can be sent as
> non-RFC.  It might be a good idea to pick a target kernel version and
> start working towards it.  v3.18 is probably not a realistic goal at
> this point.  I don't know about the rest, but at least the remaining
> series is non-RFC and the other is only a single patch.
> 
> > Step II: integrate 13: kvm-accelerated QEMU VFIO device featuring
> > iqrfd/virqfd
> > 
> > depends on
> > [7] arm/arm64: KVM: Various VGIC cleanups and improvements, C. Dall
> > [6] [PATCH v3] ARM: KVM: add irqfd support, E. Auger
> > https://lkml.org/lkml/2014/9/1/141
> > 
> > Step III: integrate > 13:  kvm-accelerated QEMU VFIO device featuring
> > forwarded IRQs:
> > [8] [RFC v2 0/9] KVM-VFIO IRQ forward control, Eric Auger
> > https://lkml.org/lkml/2014/9/1/344
> > [9] [RFC PATCH 0/9] ARM: Forwarding physical interrupts to a guest VM,
> > Marc Zyngier, http://lwn.net/Articles/603514/
> > 
> > To me these 3 steps are quite independent from each other.
> 
> Yep, I agree.  Let's not get bogged down in letting these additional
> features interfere with progress on the base support.
> 
> > with respect to performance I think we have something reasonable now
> > with irqfd and forwarded IRQ so I do not expect any new features added
> > soon.
> > 
> > from now on, I do not plan to add any new patch file to this series but
> > just correct/modify according to comments & weaknesses.
> > 
> > I Hope it clarifies plans. Please let me know.
> 
> Thanks, it does.  We have several players in the VFIO platform space and
> I want to make sure we're aligned on a goal of getting code upstream,
> not just posting it to the list.  Thanks for the breakdown and your work
> towards getting those dependencies resolved.

Actually, should Step I from your perspective be patches 1-8 of this
series?  If we remove VFIO_DEVICE_TYPE_PLATFORM from patch 3 and the
resulting instances of it, the rest is simply moving and splitting PCI
support in preparation for, but independent of platform support.  That
can be done entirely in parallel to the platform kernel support and
leaves a lot less here to review when that comes around.  Thanks,

Alex