Re: [Qemu-devel] [RFC PATCH 00/10] pseries: Allow VFIO devices on spapr-pci-host-bridge

[Laurent Vivier <lvivier@redhat.com>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1



On 24/09/2015 03:02, David Gibson wrote:
> On Wed, Sep 23, 2015 at 06:46:17PM +0200, Laurent Vivier wrote:
>> 
>> 
>> On 17/09/2015 15:09, David Gibson wrote:
>>> Currently the pseries machine type uses two types of PCI Host
>>> Bridge (PHB) devices: "spapr-pci-host-bridge" the 'normal'
>>> variant intended for emulated PCI devices, and
>>> "spapr-pci-vfio-host-bridge" intended for VFIO devices.
>>> 
>>> When using VFIO with pseries, a separate
>>> spapr-pci-vfio-host-bridge device is needed for every host
>>> IOMMU group from which you're using VFIO devices.  This is
>>> quite awkward for the user and/or management tools.  It's
>>> especially awkward since the current code makes essentially no
>>> attempt to detect and warn the user if the wrong sorts of
>>> devices are connected to the wrong PHB.
>>> 
>>> It turns out that the VFIO core code is actually general enough
>>> that VFIO devices almost work on the normal
>>> spapr-pci-host-bridge device. In fact with the right
>>> combination of circumstances they *can* work right now.
>>> 
>>> spapr-pci-vfio-host-bridge does 3 additional things:
>>> 
>>> 1) It disables KVM acceleration of the guest IOMMU.  That 
>>> acceleration breaks VFIO because it means guest IOMMU updates 
>>> bypass the VFIO infrastructure which keeps the host IOMMU in 
>>> sync.
>>> 
>>> 2) It automatically configures the guest PHB's DMA window to
>>> match the capabilities of the host IOMMU, and advertises that
>>> to the guest.
>>> 
>>> 3) It provides additional handling of EEH (Enhanced Error 
>>> Handling) functions.
>>> 
>>> This patch series: * Allows VFIO devices to be used on the
>>> spapr-pci-host-bridge by auto-switching the KVM TCE
>>> acceleration
>>> 
>>> * Adds verification that the host IOMMU can handle the DMA
>>> windows used by guest PHBs
>>> 
>>> * Allows the DMA window on the guest PHB to be configured with 
>>> device properties.  This can be used to make sure it matches a 
>>> host window, but in practice the default setting will already 
>>> work with the host IOMMU on all current systems.
>>> 
>>> * Adds support to the VFIO core to allow a VFIO device to be 
>>> hotplugged onto a bus which doesn't yet have VFIO devices.
>>> This already worked for systems without a guest-visible IOMMU 
>>> (i.e. x86), this series makes it work even with a guest
>>> visible IOMMU.
>>> 
>>> * Makes a few related cleanups along the way
>>> 
>>> This series does NOT allow EEH operations on VFIO devices on
>>> the spapr-pci-host-bridge device, so the
>>> spapr-pci-vfio-host-bridge device is left in for now.  It turns
>>> out there are some serious existing problems in both the qemu
>>> EEH implementation and (worse) in the EEH/VFIO kernel
>>> interface.  Fixing those is a problem for another day. Maybe
>>> tomorrow.
>>> 
>>> 
>>> I've tested basic assignment of an xHCI to a pseries guest,
>>> both at startup and with hotplug.  I haven't (yet) tested VFIO
>>> on x86 with this series.
>>> 
>>> This series probably needs to be merged via several different
>>> trees. I'm intending to split up as necessary once it's had
>>> some review.
>>> 
>>> David Gibson (10): vfio: Remove unneeded union from
>>> VFIOContainer vfio: Generalize vfio_listener_region_add failure
>>> path vfio: Check guest IOVA ranges against host IOMMU
>>> capabilities vfio: Record host IOMMU's available IO page sizes 
>>> memory: Allow replay of IOMMU mapping notifications vfio: Allow
>>> hotplug of containers onto existing guest IOMMU mappings 
>>> spapr_pci: Allow PCI host bridge DMA window to be configured 
>>> spapr_iommu: Rename vfio_accel parameter spapr_iommu: Provide a
>>> function to switch a TCE table to allowing VFIO spapr_pci:
>>> Allow VFIO devices to work on the normal PCI host bridge
>>> 
>>> hw/ppc/spapr_iommu.c          |  25 ++++++- hw/ppc/spapr_pci.c
>>> |  13 +++- hw/vfio/common.c              | 152
>>> +++++++++++++++++++++++++++--------------- 
>>> include/exec/memory.h         |  16 +++++ 
>>> include/hw/pci-host/spapr.h   |   3 +- include/hw/ppc/spapr.h
>>> |   6 +- include/hw/vfio/vfio-common.h |  21 +++--- memory.c
>>> |  18 +++++ target-ppc/kvm.c              |   4 +- 
>>> target-ppc/kvm_ppc.h          |   2 +- 10 files changed, 184
>>> insertions(+), 76 deletions(-)
>>> 
>> 
>> Just a comment on the form: checkpatch.pl vociferates because of
>> "DOS line endings", "trailing whitespace" and "line over 80
>> characters".
> 
> Hmm.. I see the trailing whitespace and over 80 characters
> warnings along with a few others.  Nothing about DOS line endings
> though. Wondering if that could be something that's come from your
> mailer.

Possible, but very strange: it's the first time it happens (thunderbird)
.

>> 
>> <troll> Please, use "vi" with "set ff=unix" </troll>
>> 
>> Stefan has a good tip to check automatically commits:
>> 
>> http://blog.vmsplice.net/2011/03/how-to-automatically-run-checkpatchp
l.html
>
>> 
> Nice, thanks.
> 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iEYEARECAAYFAlYDoA0ACgkQNKT2yavzbFMaDwCgx/b5gq4zARrEqot574NNIKTV
xGUAnjRPykSMtSMCvvwPKtTqotpYN2Jz
=4Chr
-----END PGP SIGNATURE-----