Project

General

Profile

Actions

Bug #14438

closed

ppt gets width of PCIE_DEVCAP wrong

Added by Andy Fiddaman 7 months ago. Updated 7 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
-
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Bite-size
Tags:
Gerrit CR:

Description

While comparing assembly output from gcc7 and 10, I noticed that the ppt_flr() function was a lot shorter than the corresponding C code would suggest.
The compiler has optimised out everything after

    if ((PCI_CAP_GET16(hdl, 0, cap_ptr, PCIE_DEVCAP) & PCIE_DEVCAP_FLR) == 0)
        goto fail;

The width of PCIE_DEVCAP is 32-bits and PCIE_DEVCAP_FLR is bit 28 so this statement will always be true.

Actions #1

Updated by Electric Monk 7 months ago

  • Gerrit CR set to 1975
Actions #2

Updated by Andy Fiddaman 7 months ago

Since the compiler obviously knows that this statement will always be true, I tried to find a warning setting which would output that. However, even with every single warning enabled (via parsing the output of gcc -Wall -Wextra -Q --help=warning) this was not reported.

Actions #3

Updated by Andy Fiddaman 7 months ago

Jorge Schrauwen has tested this change on two of his physical hosts that run a number of VMs which use PCI passthrough. He has a number of different passthrough devices, some of which support FLR and some which do not. He has used dtrace to confirm that the ppt_flr() function is now actually performing the reset.

Of particular note is that this change appears to improve the behaviour of a part of his setup where a four-port igb device is split between VMs:

<sjorge> I stopped one of the 2 VM that have one of the igb nics 5 times and the other one kept working
<sjorge> Before it would sometimes break the nic until I either stopped both VMs and started them again, or rebooted (the physical host) if the former didn't work
# for i in 0 1 2 3 4 5; do /usr/lib/pci/pcieadm show-cfgspace -d ppt${i} pcie.devcap.flr; done
PCI Express Capability (0x10)
  Device Capabilities: 0x8fc0
    |--> Function Level Reset: unsupported (0x0)
PCI Express Capability (0x10)
  Device Capabilities: 0x8fc0
    |--> Function Level Reset: unsupported (0x0)
PCI Express Capability (0x10)
  Device Capabilities: 0x8fc0
    |--> Function Level Reset: unsupported (0x0)
PCI Express Capability (0x10)
  Device Capabilities: 0x8fc0
    |--> Function Level Reset: unsupported (0x0)
PCI Express Capability (0x10)
  Device Capabilities: 0x10008ce2
    |--> Function Level Reset: supported (0x10000000)
PCI Express Capability (0x10)
  Device Capabilities: 0x10008ce2
    |--> Function Level Reset: supported (0x10000000)
Actions #4

Updated by Electric Monk 7 months ago

  • Status changed from In Progress to Closed
  • % Done changed from 0 to 100

git commit 9ddf79f971e99ac9fc94da5126fe4c68c077ea6d

commit  9ddf79f971e99ac9fc94da5126fe4c68c077ea6d
Author: Andy Fiddaman <omnios@citrus-it.co.uk>
Date:   2022-01-24T14:23:14.000Z

    14438 ppt gets width of PCIE_DEVCAP wrong
    Reviewed by: Robert Mustacchi <rm+illumos@fingolfin.org>
    Reviewed by: Patrick Mooney <pmooney@pfmooney.com>
    Reviewed by: Toomas Soome <tsoome@me.com>
    Approved by: Gordon Ross <gordon.w.ross@gmail.com>

Actions

Also available in: Atom PDF