From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:53058) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TGvwN-0003vh-0r for qemu-devel@nongnu.org; Wed, 26 Sep 2012 14:02:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TGvwF-0000So-Bh for qemu-devel@nongnu.org; Wed, 26 Sep 2012 14:02:30 -0400 Received: from hub021-nj-8.exch021.serverdata.net ([206.225.164.233]:18989) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TGvwF-0000ST-7I for qemu-devel@nongnu.org; Wed, 26 Sep 2012 14:02:23 -0400 Message-ID: <5063432D.4040702@CloudSwitch.Com> Date: Wed, 26 Sep 2012 14:02:21 -0400 From: Don Slutz MIME-Version: 1.0 References: <1348673453-3248-1-git-send-email-mjr@cs.wisc.edu> <1348676771.28860.199.camel@bling.home> <004401cd9c07$0b36d760$21a48620$@cs.wisc.edu> <1348678672.28860.204.camel@bling.home> <005901cd9c0f$4d3f9ef0$e7bedcd0$@cs.wisc.edu> <1348682122.28860.210.camel@bling.home> In-Reply-To: <1348682122.28860.210.camel@bling.home> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3] Align PCI capabilities in pci_find_space List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: qemu-devel@nongnu.org, Matt Renzelmann On 09/26/12 13:55, Alex Williamson wrote: > On Wed, 2012-09-26 at 12:49 -0500, Matt Renzelmann wrote: >>>> static int pci_find_space(PCIDevice *pdev, uint8_t size, bool pcie_spa= ce) >>>> { >>>> int config_base; >>>> int config_size; >>>> int offset =3D PCI_CONFIG_HEADER_SIZE; >>>> int i; >>>> uint32_t *dword_used =3D &pdev->used[PCI_CONFIG_HEADER_SIZE]; >>> This needs to change too. >>> >>> Is there a different alignment requirement for pcie? Seems like you >>> might be better off creating some helper like: >> I found a copy of the PCI-E 1.0 specification -- I don't have any later = copy available -- and it contains this text: >> >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> 7.9. PCI Express Extended Capabilities >> >> PCI Express Extended Capability registers are located in device configur= ation space at offsets 256 or greater ... >> >> Each capability structure must be DWORD aligned. >> >> ... >> >> 7.9.3. PCI Express Enhanced Capability Header >> >> Next Capability Offset =E2=80=93 This field contains the >> offset to the next PCI Express capability structure or >> 000h if no other items exist in the linked list of >> capabilities. >> >> ... >> >> The bottom two bits of this offset are reserved and >> must be implemented as 00b although software >> must mask them to allow for future uses of these >> bits. >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> >> so, I think it's reasonable to align everything to 4-bytes all the time.= Here's another draft: >> >> >> static int pci_find_space(PCIDevice *pdev, uint32_t start, uint32_t size= ) >> { >> int offset =3D start; >> int i; >> uint32_t *dword_used =3D &pdev->used[start]; >> >> /* This approach ensures the capability is dword-aligned, as >> required by the PCI and PCI-E specifications */ >> for (i =3D start; i < size; i +=3D 4, dword_used++) { >> if (*dword_used) I would think that dword_used needs to be based on i not start. >> offset =3D i + 4; >> else if (i - offset + 4 >=3D size) >> return offset; >> } > Mismatched uses of "size" here. We need both the end of the range to > search and the size of the sub-range we're looking for. Maybe start, > end, and size. Thanks, > > Alex > >> return 0; >> } >> >> static int pci_find_legacy_space(PCIDevice *pdev, uint8_t size) { >> return pci_find_space(pdev, PCI_CONFIG_HEADER_SIZE, PCI_CONFIG_SPAC= E_SIZE); >> } >> >> static int pci_find_express_space(PCIDevice *pdev, uint16_t size) { >> assert (pci_config_size(pdev) >=3D PCIE_CONFIG_SPACE_SIZE); >> return pci_find_space(pdev, PCI_CONFIG_SPACE_SIZE, PCIE_CONFIG_SPAC= E_SIZE); >> } >> >> > > > -Don Slutz