Re: [Qemu-devel] [PATCH 0/4] pcie: Add support for Single Root I/O Virtualization

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Aug 29, 2014 at 09:17:05AM +0200, Knut Omang wrote:
> This patch set consists of two parts:
> 
>  - The two first patches implements SR/IOV building blocks in pcie/pci.
>    I have held this patch back for a while because I haven't had a good
>    example test case to accompany it, but in the light of the latest 
>    developments such as the discussion we have had around ARI and downstream
>    switches and root ports, I think it would be valuable to get this in,
>    to make it easier for people to experiment with creating devices with
>    many functions. Hopefully, I can also get some help to fix the hotplug
>    issues described below.
> 
>  - The two last patches contains an example to illustrate the usage
>    of the new SR/IOV support. The example leverages the e1000 
>    code and the fact that registers between E1000 and the PCIe based 
>    Intel 82576 Gigabit Ethernet Adapter (which supports SR/IOV) are quite similar, 
>    but so far without considering much of the differences beyond the 
>    bare minimum needed to trick the igb driver into loading to the point 
>    where VFs can be enabled.
> 
>    So you cannot yet use these PF or VF Ethernet devices to send ethernet packets,
>    and the implementation do not in any way attempt to model the internals
>    of igb such as the multiple queues or any multiplexing onto the same device, 
>    it only instantiates the VFs as well as the PFs mostly directly by inheritance
>    (and as separate devices) from E1000, but it should hopefully be relatively
>    easy to understand how to proceed to make "true" VFs.
>    It was also a nice exercise in using QOM.
> 
>    The changes to E1000 to accommodate igb (patch 3) should be fairly 
>    non-intrusive, nevertheless I suppose it should not be applied 
>    unless it will eventually lead to a new derived device which is enough
>    different from E1000 to qualify for a separate set of source files.
>    So if someone with more detailed knowledge of the internal differences
>    between igb and e1000 on the functional level might have input, that would be
>    great, an alternative of course be to only apply the two first 
>    patches, and leave any usage examples for the future.
> 
>    To test and see how it plays out, you can add something like this
>    to the command line:
> 
>      -device ioh3420,slot=0,id=pcie_port.0
>      -device igb,mac=DE:AD:BE:EE:04:18,vlan=1,bus=pcie_port.0
>      -net tap,vlan=1
>      
>    The Linux igb driver does not yet support changing the VF setup via sysfs
>    (the preferred way since kernel v.3.8) so to see some VFs on the guest, 
>    you need to set up a modprobe file with something like this:
> 
>       options igb max_vfs=4
> 
>    For some reason I dont yet understand, removal of the igb driver
>    does not cause a 0 write to sr/iov vf_enable to disable the VFs again, eg.
> 
>        # lspci | grep '^01:00'
>        01:00.0 Ethernet controller: Intel Corporation 82576 Gigabit Network Connection (rev 01)
>        01:00.1 Ethernet controller: Intel Corporation 82576 Virtual Function (rev 01)
>        01:00.2 Ethernet controller: Intel Corporation 82576 Virtual Function (rev 01)
>        01:00.3 Ethernet controller: Intel Corporation 82576 Virtual Function (rev 01)
>        01:00.4 Ethernet controller: Intel Corporation 82576 Virtual Function (rev 01)
> 
>    Writing directly to vf_enable using setpci works though, but after the later hotplug 
>    changes, a little too well, as after the VF removals have succeded, 
>    some way a slot power down gets triggered, also removing the PF:
> 
>       # setpci -s 01:00.0 168.b
>       0x19
>       # setpci -s 01:00.0 168.b=18
>       # lspci | grep '^01:00'
>       <nothing>
> 
>    For similar reasons attaching the igb device directly on the root complex (just remove
>    bus parameter above) attempts to enable VFs will fail with 
>       qdev.c:89: bus_add_child: Assertion `bus->allow_hotplug' failed
> 
>    I imagine this could for instance be a matter of defining a property "subfunction" 
>    or similar that allows qdev (QOM?) to discriminate between main devices and devices 
>    representing subfunctions of another main device. Suggestions on how to proceed on this
>    welcome.
> 
> This patch depends (by diff context only) on my patch:
> 
> [PATCH v2 1/4] pcie: Fix incorrect write to the ari capability next function field
> 
> and for stability on Paolo's 
> 
> [PATCH] pci_bridge: manually destroy memory regions within PCIBridgeWindows
> 
> both of which Michael has pulled, but which are not in master yet.
> 
> Thanks,

So practically you would like patches 1 and 2 applied,
and 3 and 4 are RFC?

> Knut Omang (4):
>   pci: Update pci_regs header
>   pcie: Add support for Single Root I/O Virtualization (SR/IOV)
>   e1000: Refactor to allow subclassing from other source file
>   igb: Example code to illustrate the SR/IOV support.
> 
>  hw/i386/kvm/pci-assign.c  |   4 +-
>  hw/misc/vfio.c            |   8 +-
>  hw/net/Makefile.objs      |   2 +-
>  hw/net/e1000.c            | 126 ++--------------
>  hw/net/e1000.h            | 139 +++++++++++++++++
>  hw/net/igb.c              | 293 ++++++++++++++++++++++++++++++++++++
>  hw/net/igb_regs.h         |  27 ++++
>  hw/pci/msi.c              |   4 -
>  hw/pci/msix.c             |   2 +-
>  hw/pci/pci.c              | 107 +++++++++----
>  hw/pci/pcie.c             | 205 ++++++++++++++++++++++++-
>  include/hw/pci/pci.h      |   6 +-
>  include/hw/pci/pci_regs.h | 371 ++++++++++++++++++++++++++++++++++------------
>  include/hw/pci/pcie.h     |  26 ++++
>  14 files changed, 1072 insertions(+), 248 deletions(-)
>  create mode 100644 hw/net/e1000.h
>  create mode 100644 hw/net/igb.c
>  create mode 100644 hw/net/igb_regs.h
> 
> -- 
> 1.9.0