Hi peter On 02/14/2017 03:51 PM, Peter Xu wrote: > 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. >
See if this suggestion could be incorporated into your patch:) http://lists.nongnu.org/archive/html/qemu-devel/2017-01/msg01418.html -- Sincerely, Cao jin > Signed-off-by: Peter Xu <pet...@redhat.com> > --- > hw/pci/pcie.c | 15 ++++----------- > 1 file changed, 4 insertions(+), 11 deletions(-) > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c > index cbd4bb4..e0e6f6a 100644 > --- a/hw/pci/pcie.c > +++ b/hw/pci/pcie.c > @@ -664,30 +664,23 @@ 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; > > /* 0 is reserved cap id. use internally to find the last capability > in the linked list */ > - next = pcie_find_capability_list(dev, 0, &prev); > - > + assert(pcie_find_capability_list(dev, 0, &prev) == 0); > 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); >