ShuriZma
/
xemu
mirror of https://github.com/xemu-project/xemu.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

vfio/pci: Improve extended capability comments, skip masked caps 

Since commit 4bb571d857 ("pci/pcie: don't assume cap id 0 is
reserved") removes the internal use of extended capability ID 0, the
comment here becomes invalid.  However, peeling back the onion, the
code is still correct and we still can't seed the capability chain
with ID 0, unless we want to muck with using the version number to
force the header to be non-zero, which is much uglier to deal with.
The comment also now covers some of the subtleties of using cap ID 0,
such as transparently indicating absence of capabilities if none are
added.  This doesn't detract from the correctness of the referenced
commit as vfio in the kernel also uses capability ID zero to mask
capabilties.  In fact, we should skip zero capabilities precisely
because the kernel might also expose such a capability at the head
position and re-introduce the problem.

Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Tested-by: Peter Xu <peterx@redhat.com>
Reported-by: Jintack Lim <jintack@cs.columbia.edu>
Tested-by: Jintack Lim <jintack@cs.columbia.edu>
This commit is contained in:
Alex Williamson 2017-02-22 13:19:58 -07:00
parent 35c7cb4caf
commit d0d1cd70d1
1 changed files with 21 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
hw/vfio/pci.c
View File

@ -1880,16 +1880,26 @@ static void vfio_add_ext_cap(VFIOPCIDevice *vdev)
/* /*
* Extended capabilities are chained with each pointing to the next, so we * Extended capabilities are chained with each pointing to the next, so we
* can drop anything other than the head of the chain simply by modifying * can drop anything other than the head of the chain simply by modifying
* the previous next pointer. For the head of the chain, we can modify the * the previous next pointer. Seed the head of the chain here such that
* capability ID to something that cannot match a valid capability. ID * we can simply skip any capabilities we want to drop below, regardless
* 0 is reserved for this since absence of capabilities is indicated by * of their position in the chain. If this stub capability still exists
* 0 for the ID, version, AND next pointer. However, pcie_add_capability() * after we add the capabilities we want to expose, update the capability
* uses ID 0 as reserved for list management and will incorrectly match and * ID to zero. Note that we cannot seed with the capability header being
* assert if we attempt to pre-load the head of the chain with this ID. * zero as this conflicts with definition of an absent capability chain
* Use ID 0xFFFF temporarily since it is also seems to be reserved in * and prevents capabilities beyond the head of the list from being added.
* part for identifying absence of capabilities in a root complex register * By replacing the dummy capability ID with zero after walking the device
* block. If the ID still exists after adding capabilities, switch back to * chain, we also transparently mark extended capabilities as absent if
* zero. We'll mark this entire first dword as emulated for this purpose. * no capabilities were added. Note that the PCIe spec defines an absence
* of extended capabilities to be determined by a value of zero for the
* capability ID, version, AND next pointer. A non-zero next pointer
* should be sufficient to indicate additional capabilities are present,
* which will occur if we call pcie_add_capability() below. The entire
* first dword is emulated to support this.
*
* NB. The kernel side does similar masking, so be prepared that our
* view of the device may also contain a capability ID zero in the head
* of the chain. Skip it for the same reason that we cannot seed the
* chain with a zero capability.
*/ */
pci_set_long(pdev->config + PCI_CONFIG_SPACE_SIZE, pci_set_long(pdev->config + PCI_CONFIG_SPACE_SIZE,
PCI_EXT_CAP(0xFFFF, 0, 0)); PCI_EXT_CAP(0xFFFF, 0, 0));
@ -1915,6 +1925,7 @@ static void vfio_add_ext_cap(VFIOPCIDevice *vdev)
PCI_EXT_CAP_NEXT_MASK); PCI_EXT_CAP_NEXT_MASK);
switch (cap_id) { switch (cap_id) {
case 0: /* kernel masked capability */
case PCI_EXT_CAP_ID_SRIOV: /* Read-only VF BARs confuse OVMF */ case PCI_EXT_CAP_ID_SRIOV: /* Read-only VF BARs confuse OVMF */
case PCI_EXT_CAP_ID_ARI: /* XXX Needs next function virtualization */ case PCI_EXT_CAP_ID_ARI: /* XXX Needs next function virtualization */
trace_vfio_add_ext_cap_dropped(vdev->vbasedev.name, cap_id, next); trace_vfio_add_ext_cap_dropped(vdev->vbasedev.name, cap_id, next);