From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:38329) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TGx1l-0003Em-C8 for qemu-devel@nongnu.org; Wed, 26 Sep 2012 15:12:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TGx1k-0005EZ-7F for qemu-devel@nongnu.org; Wed, 26 Sep 2012 15:12:09 -0400 Received: from mx1.redhat.com ([209.132.183.28]:27770) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TGx1j-0005EQ-Si for qemu-devel@nongnu.org; Wed, 26 Sep 2012 15:12:08 -0400 Message-ID: <1348686724.28860.216.camel@bling.home> From: Alex Williamson Date: Wed, 26 Sep 2012 13:12:04 -0600 In-Reply-To: <1348685491-3931-1-git-send-email-mjr@cs.wisc.edu> References: <1348685491-3931-1-git-send-email-mjr@cs.wisc.edu> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4] Align PCI capabilities in pci_find_space List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Matt Renzelmann Cc: qemu-devel@nongnu.org On Wed, 2012-09-26 at 13:51 -0500, Matt Renzelmann wrote: > The current implementation of pci_find_space does not correctly align > PCI capabilities in the PCI configuration space. It also does not > support PCI-Express devices. This patch fixes these issues. > > Thanks to Alex Williamson for feedback. > > Signed-off-by: Matt Renzelmann > --- > > This version adds the assertions to pci_find_space and removes the > assertion from pci_find_express_space. > > hw/pci.c | 35 +++++++++++++++++++++++++++-------- > 1 files changed, 27 insertions(+), 8 deletions(-) Acked-by: Alex Williamson > diff --git a/hw/pci.c b/hw/pci.c > index f855cf3..7050596 100644 > --- a/hw/pci.c > +++ b/hw/pci.c > @@ -1626,19 +1626,38 @@ PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name) > return pci_create_simple_multifunction(bus, devfn, false, name); > } > > -static int pci_find_space(PCIDevice *pdev, uint8_t size) > +static int pci_find_space(PCIDevice *pdev, uint32_t start, > + uint32_t end, uint32_t size) > { > - int config_size = pci_config_size(pdev); > - int offset = PCI_CONFIG_HEADER_SIZE; > + int offset = start; > int i; > - for (i = PCI_CONFIG_HEADER_SIZE; i < config_size; ++i) > - if (pdev->used[i]) > - offset = i + 1; > - else if (i - offset + 1 == size) > + uint32_t *dword_used = &pdev->used[start]; > + > + assert(pci_config_size(pdev) >= end); > + assert(!(start & 0x3)); > + > + /* This approach ensures the capability is dword-aligned, as > + required by the PCI and PCI-E specifications */ > + for (i = start; i < end; i += 4, dword_used++) { > + if (*dword_used) > + offset = i + 4; > + else if (i - offset + 4 >= size) > return offset; > + } > + > return 0; > } > > +static int pci_find_legacy_space(PCIDevice *pdev, uint8_t size) { > + return pci_find_space(pdev, PCI_CONFIG_HEADER_SIZE, > + PCI_CONFIG_SPACE_SIZE, size); > +} > + > +static int pci_find_express_space(PCIDevice *pdev, uint16_t size) { > + return pci_find_space(pdev, PCI_CONFIG_SPACE_SIZE, > + PCIE_CONFIG_SPACE_SIZE, size); > +} > + > static uint8_t pci_find_capability_list(PCIDevice *pdev, uint8_t cap_id, > uint8_t *prev_p) > { > @@ -1826,7 +1845,7 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, > int i, overlapping_cap; > > if (!offset) { > - offset = pci_find_space(pdev, size); > + offset = pci_find_legacy_space(pdev, size); > if (!offset) { > return -ENOSPC; > }