From 4bb571d857d973d9308d9fdb1f48d983d6639bd4 Mon Sep 17 00:00:00 2001 From: Michael S. Tsirkin Date: Wed, 15 Feb 2017 22:37:45 +0200 Subject: pci/pcie: don't assume cap id 0 is reserved VFIO actually wants to create a capability with ID == 0. This is done to make guest drivers skip the given capability. pcie_add_capability then trips up on this capability when looking for end of capability list. To support this use-case, it's easy enough to switch to e.g. 0xffffffff for these comparisons - we can be sure it will never match a 16-bit capability ID. Signed-off-by: Michael S. Tsirkin Reviewed-by: Peter Xu Reviewed-by: Alex Williamson --- hw/pci/pcie.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'hw/pci/pcie.c') diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c index cbd4bb4f8c..f4dd177bc4 100644 --- a/hw/pci/pcie.c +++ b/hw/pci/pcie.c @@ -610,7 +610,8 @@ bool pcie_cap_is_arifwd_enabled(const PCIDevice *dev) * uint16_t ext_cap_size */ -static uint16_t pcie_find_capability_list(PCIDevice *dev, uint16_t cap_id, +/* Passing a cap_id value > 0xffff will return 0 and put end of list in prev */ +static uint16_t pcie_find_capability_list(PCIDevice *dev, uint32_t cap_id, uint16_t *prev_p) { uint16_t prev = 0; @@ -679,9 +680,11 @@ void pcie_add_capability(PCIDevice *dev, } else { uint16_t prev; - /* 0 is reserved cap id. use internally to find the last capability - in the linked list */ - next = pcie_find_capability_list(dev, 0, &prev); + /* + * 0xffffffff is not a valid cap id (it's a 16 bit field). use + * internally to find the last capability in the linked list. + */ + next = pcie_find_capability_list(dev, 0xffffffff, &prev); assert(prev >= PCI_CONFIG_SPACE_SIZE); assert(next == 0); -- cgit v1.2.3-55-g7522 From d4e9b75aa03d3f0e08fa431998764d7f72d78a48 Mon Sep 17 00:00:00 2001 From: Peter Xu Date: Thu, 16 Feb 2017 12:06:01 +0800 Subject: pcie: simplify pcie_add_capability() When we add PCIe extended capabilities, we should be following the rule that we add the head extended cap (at offset 0x100) first, then the rest of them. Meanwhile, we are always adding new capability bits at the end of the list. Here the "next" looks meaningless in all cases since it should always be zero (along with the "header"). Simplify the function a bit, and it looks more readable now. Signed-off-by: Peter Xu Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/pci/pcie.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) (limited to 'hw/pci/pcie.c') diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c index f4dd177bc4..fc54bfd53d 100644 --- a/hw/pci/pcie.c +++ b/hw/pci/pcie.c @@ -665,32 +665,24 @@ void pcie_add_capability(PCIDevice *dev, uint16_t cap_id, uint8_t cap_ver, uint16_t offset, uint16_t size) { - uint32_t header; - uint16_t next; - assert(offset >= PCI_CONFIG_SPACE_SIZE); assert(offset < offset + size); assert(offset + size <= PCIE_CONFIG_SPACE_SIZE); assert(size >= 8); assert(pci_is_express(dev)); - if (offset == PCI_CONFIG_SPACE_SIZE) { - header = pci_get_long(dev->config + offset); - next = PCI_EXT_CAP_NEXT(header); - } else { + if (offset != PCI_CONFIG_SPACE_SIZE) { uint16_t prev; /* * 0xffffffff is not a valid cap id (it's a 16 bit field). use * internally to find the last capability in the linked list. */ - next = pcie_find_capability_list(dev, 0xffffffff, &prev); - + pcie_find_capability_list(dev, 0xffffffff, &prev); assert(prev >= PCI_CONFIG_SPACE_SIZE); - assert(next == 0); pcie_ext_cap_set_next(dev, prev, offset); } - pci_set_long(dev->config + offset, PCI_EXT_CAP(cap_id, cap_ver, next)); + pci_set_long(dev->config + offset, PCI_EXT_CAP(cap_id, cap_ver, 0)); /* Make capability read-only by default */ memset(dev->wmask + offset, 0, size); -- cgit v1.2.3-55-g7522