QEMU exposes gen1 PCI-express interconnect devices supporting only
2.5GT/s and x1 width.  It might not seem obvious that a virtual
bandwidth limitation can result in a real performance degradation, but
it's been reported that in some configurations assigned GPUs might not
scale their link speed up to the maximum supported value if the
downstream port above it only advertises limited link support.

As proposed[1] this series effectively implements virtual link
negotiation on downstream ports and enhances the generic PCIe root
port to allow user configurable speeds and widths.  The "negotiation"
simply mirrors the link status of the connected downstream device
providing the appearance of dynamic link speed scaling to match the
endpoint device.  Not yet implemented from the proposal is support
for globally updating defaults based on machine type, though the
foundation is provided here by allowing supporting PCIESlots to
implement an instance_init callback which can call into a common
helper for this.

I have not specifically tested migration with this, but we already
consider LNKSTA to be dynamic and the other changes implemented here
are static config space changes with no changes being implemented for
devices using default values, ie. they should be compatible by virtue
of existing config space migration support.

I think I've covered the required link related registers to support
PCIe 4.0, but please let me know if I've missed any.

Testing and feedback appreciated, patch 6/7 provides example qemu:arg
options and requirements to use with existing libvirt.  Native libvirt
support TBD.  Thanks,

Alex

[1] https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg03086.html

---

Alex Williamson (7):
      pcie: Create enums for link speed and width
      pci: Sync PCIe downstream port LNKSTA on read
      qapi: Define PCIe link speed and width properties
      pcie: Add link speed and width fields to PCIESlot
      pcie: Fill PCIESlot link fields to support higher speeds and widths
      pcie: Allow generic PCIe root port to specify link speed and width
      vfio/pci: Remove PCIe Link Status emulation


 hw/core/qdev-properties.c          |  178 ++++++++++++++++++++++++++++++++++++
 hw/pci-bridge/gen_pcie_root_port.c |    2 
 hw/pci-bridge/pcie_root_port.c     |   14 +++
 hw/pci/pci.c                       |    4 +
 hw/pci/pcie.c                      |  118 +++++++++++++++++++++++-
 hw/vfio/pci.c                      |    9 --
 include/hw/pci/pci.h               |   13 +++
 include/hw/pci/pcie.h              |    1 
 include/hw/pci/pcie_port.h         |    4 +
 include/hw/pci/pcie_regs.h         |   23 ++++-
 include/hw/qdev-properties.h       |    8 ++
 qapi/common.json                   |   42 ++++++++
 12 files changed, 404 insertions(+), 12 deletions(-)

Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 154222737752.9288.484557356059052047.stgit@gimli.home
Type: series
Subject: [Qemu-devel] [RFC for-3.2 PATCH 0/7] pcie: Enhanced link speed and width support

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
a592f52 vfio/pci: Remove PCIe Link Status emulation
ee466ea pcie: Allow generic PCIe root port to specify link speed and width
7d242bd pcie: Fill PCIESlot link fields to support higher speeds and widths
24a8568 pcie: Add link speed and width fields to PCIESlot
826c404 qapi: Define PCIe link speed and width properties
dd67d2e pci: Sync PCIe downstream port LNKSTA on read
9aea030 pcie: Create enums for link speed and width

=== OUTPUT BEGIN ===
Checking PATCH 1/7: pcie: Create enums for link speed and width...
Checking PATCH 2/7: pci: Sync PCIe downstream port LNKSTA on read...
ERROR: code indent should never use tabs
#41: FILE: hw/pci/pci.c:1358:
+^Ipcie_sync_bridge_lnk(d);$

total: 1 errors, 0 warnings, 81 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 3/7: qapi: Define PCIe link speed and width properties...
Checking PATCH 4/7: pcie: Add link speed and width fields to PCIESlot...
Checking PATCH 5/7: pcie: Fill PCIESlot link fields to support higher speeds and widths...
Checking PATCH 6/7: pcie: Allow generic PCIe root port to specify link speed and width...
Checking PATCH 7/7: vfio/pci: Remove PCIe Link Status emulation...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

I can confirm that these patches work as expected. Thank you kindly Alex 
for your hard work!

Tested-by: Geoffrey McRae <geoff@hostfission.com>

On 2018-11-15 07:50, Alex Williamson wrote:
> QEMU exposes gen1 PCI-express interconnect devices supporting only
> 2.5GT/s and x1 width.  It might not seem obvious that a virtual
> bandwidth limitation can result in a real performance degradation, but
> it's been reported that in some configurations assigned GPUs might not
> scale their link speed up to the maximum supported value if the
> downstream port above it only advertises limited link support.
> 
> As proposed[1] this series effectively implements virtual link
> negotiation on downstream ports and enhances the generic PCIe root
> port to allow user configurable speeds and widths.  The "negotiation"
> simply mirrors the link status of the connected downstream device
> providing the appearance of dynamic link speed scaling to match the
> endpoint device.  Not yet implemented from the proposal is support
> for globally updating defaults based on machine type, though the
> foundation is provided here by allowing supporting PCIESlots to
> implement an instance_init callback which can call into a common
> helper for this.
> 
> I have not specifically tested migration with this, but we already
> consider LNKSTA to be dynamic and the other changes implemented here
> are static config space changes with no changes being implemented for
> devices using default values, ie. they should be compatible by virtue
> of existing config space migration support.
> 
> I think I've covered the required link related registers to support
> PCIe 4.0, but please let me know if I've missed any.
> 
> Testing and feedback appreciated, patch 6/7 provides example qemu:arg
> options and requirements to use with existing libvirt.  Native libvirt
> support TBD.  Thanks,
> 
> Alex
> 
> [1] https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg03086.html
> 
> ---
> 
> Alex Williamson (7):
>       pcie: Create enums for link speed and width
>       pci: Sync PCIe downstream port LNKSTA on read
>       qapi: Define PCIe link speed and width properties
>       pcie: Add link speed and width fields to PCIESlot
>       pcie: Fill PCIESlot link fields to support higher speeds and 
> widths
>       pcie: Allow generic PCIe root port to specify link speed and 
> width
>       vfio/pci: Remove PCIe Link Status emulation
> 
> 
>  hw/core/qdev-properties.c          |  178 
> ++++++++++++++++++++++++++++++++++++
>  hw/pci-bridge/gen_pcie_root_port.c |    2
>  hw/pci-bridge/pcie_root_port.c     |   14 +++
>  hw/pci/pci.c                       |    4 +
>  hw/pci/pcie.c                      |  118 +++++++++++++++++++++++-
>  hw/vfio/pci.c                      |    9 --
>  include/hw/pci/pci.h               |   13 +++
>  include/hw/pci/pcie.h              |    1
>  include/hw/pci/pcie_port.h         |    4 +
>  include/hw/pci/pcie_regs.h         |   23 ++++-
>  include/hw/qdev-properties.h       |    8 ++
>  qapi/common.json                   |   42 ++++++++
>  12 files changed, 404 insertions(+), 12 deletions(-)