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

[Marcel Apfelbaum <marcel.apfelbaum@gmail.com>]

On 10/14/2015 06:51 PM, Knut Omang wrote:
> This patch set implements generic support for SR/IOV as an extension to the
> core PCIe functionality, similar to the way other capabilities such as AER
> is implemented.
>
> There is no implementation of any device that provides
> SR/IOV support included, but I have implemented a test
> example which can be found together with this patch set here:
>
>    git://github.com/knuto/qemu.git sriov_patches_v5
>
> Testing with the example device was documented here:
>
>    http://lists.nongnu.org/archive/html/qemu-devel/2014-08/msg05110.html
>
> Changes since v4:
>    - Mostly based on feeback in Marcel Apfelbaum's review:
>    - The patch with changes to pci_regs.h got eliminated by rebase
>    - Added some documentation as an additional patch
>    - Some trivial fixes moved to separate patch
>    - Modified code to use error and trace functions instead of printfs


Hi, thanks for posting another version!

Regarding the submission:
Please run ./scripts/checkpatch.pl on your patches, there are some
minor issues there (ms-dos line end...).
This would not prevent the maintainer to take the code, I think
he has some scripts to take care of it, however it may discover
some issues.

Otherwise, I only have some questions on patch 2/4.
I don't think it justifies a re-submission, I just wanted to point to some places
it may be worth to look again.

Thanks,
Marcel

>
> Changes since v3:
>    - Reworked 'pci: Update pci_regs header' to merge kernel version improvements
>      with the current qemu version instead of copying from the kernel version.
>
> Changes since v2:
>    - Rebased onto 090d0bfd
>    - Un-qdev'ified - avoids issues when resetting NUM_VFS
>    - Fixed handling of vf_offset/vf_stride
>
> Changes since v1:
>    - Rebased on top of latest master, eliminating prereqs.
>    - Implement proper support for VF_STRIDE, VF_OFFSET and SUP_PGSIZE
>      Time better spent fixing it than explaining what the previous
>      limitations were.
>      - Added new first patch to fix pci bug related to this
>    - Split out patch to pci_default_config_write to a separate patch 2
>      to highlight bug fix.
>    - Refactored out logic into new source files
>      hw/pci/pcie_sriov.c include/hw/pci/pcie_sriov.h
>      similar to pcie_aer.c/h.
>    - Rename functions and introduce structs to better separate
>      pf and vf functionality.
>    - Replaced is_vf member with pci_is_vf() function abstraction
>    - Fix numerous syntax, whitespace and comment issues
>      according to Michael's review.
>    - Fix memory leaks.
>    - Removed igb example device - a rebased version available
>      on github instead.
>
> Knut Omang (4):
>    pci: Make use of the devfn property when registering new devices
>    pcie: Add support for Single Root I/O Virtualization (SR/IOV)
>    pcie: Add some SR/IOV API documentation in docs/pcie_sriov.txt
>    pcie: A few minor fixes (type+code simplify)
>
>   docs/pcie_sriov.txt         | 115 +++++++++++++++++++
>   hw/pci/Makefile.objs        |   2 +-
>   hw/pci/pci.c                | 104 +++++++++++++-----
>   hw/pci/pcie.c               |   9 +-
>   hw/pci/pcie_sriov.c         | 263 ++++++++++++++++++++++++++++++++++++++++++++
>   include/hw/pci/pci.h        |  11 +-
>   include/hw/pci/pcie.h       |   6 +
>   include/hw/pci/pcie_sriov.h |  58 ++++++++++
>   include/qemu/typedefs.h     |   2 +
>   trace-events                |   5 +
>   10 files changed, 539 insertions(+), 36 deletions(-)
>   create mode 100644 docs/pcie_sriov.txt
>   create mode 100644 hw/pci/pcie_sriov.c
>   create mode 100644 include/hw/pci/pcie_sriov.h
>
> --
> 2.4.3
>