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

[Eric Auger]

On 09/16/2014 11:23 PM, Alex Williamson wrote:
> 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,

Hi Alex,

yes sure, we can add another step consisting in preparing the PCI code
before introducing vfio platform device. I thought you would prefer to
have a "client" of those changes.

Best Regards

Eric
> 
> Alex
>