From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:53810) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TGvy4-0005WQ-3d for qemu-devel@nongnu.org; Wed, 26 Sep 2012 14:04:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TGvy1-00017K-SK for qemu-devel@nongnu.org; Wed, 26 Sep 2012 14:04:16 -0400 Received: from hub021-nj-3.exch021.serverdata.net ([206.225.164.218]:2385) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TGvy1-00017D-OO for qemu-devel@nongnu.org; Wed, 26 Sep 2012 14:04:13 -0400 Message-ID: <5063439C.8040804@CloudSwitch.Com> Date: Wed, 26 Sep 2012 14:04:12 -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> <5063432D.4040702@CloudSwitch.Com> In-Reply-To: <5063432D.4040702@CloudSwitch.Com> 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 14:02, Don Slutz wrote: > 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=20 >>>>> pcie_space) >>>>> { >>>>> 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=20 >>> 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=20 >>> configuration 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=20 >>> time. Here's another draft: >>> >>> >>> static int pci_find_space(PCIDevice *pdev, uint32_t start, uint32_t=20 >>> 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++) { Opps, I missed the dword_used++ Forget this comment... >>> 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,=20 >>> PCI_CONFIG_SPACE_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,=20 >>> PCIE_CONFIG_SPACE_SIZE); >>> } >>> >>> >> >> >> > -Don Slutz > -Don Slutz