From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:38429) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TGvpd-0006Dm-0C for qemu-devel@nongnu.org; Wed, 26 Sep 2012 13:55:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TGvpW-0005gN-PZ for qemu-devel@nongnu.org; Wed, 26 Sep 2012 13:55:32 -0400 Received: from mx1.redhat.com ([209.132.183.28]:15718) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TGvpW-0005fh-I2 for qemu-devel@nongnu.org; Wed, 26 Sep 2012 13:55:26 -0400 Message-ID: <1348682122.28860.210.camel@bling.home> From: Alex Williamson Date: Wed, 26 Sep 2012 11:55:22 -0600 In-Reply-To: <005901cd9c0f$4d3f9ef0$e7bedcd0$@cs.wisc.edu> 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> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 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: Matt Renzelmann Cc: qemu-devel@nongnu.org On Wed, 2012-09-26 at 12:49 -0500, Matt Renzelmann wrote: > > > static int pci_find_space(PCIDevice *pdev, uint8_t size, bool 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]; > >=20 > > This needs to change too. > >=20 > > Is there a different alignment requirement for pcie? Seems like you > > might be better off creating some helper like: >=20 > I found a copy of the PCI-E 1.0 specification -- I don't have any later= copy available -- and it contains this text: >=20 > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > 7.9. PCI Express Extended Capabilities >=20 > PCI Express Extended Capability registers are located in device configu= ration space at offsets 256 or greater ... >=20 > Each capability structure must be DWORD aligned. >=20 > ... >=20 > 7.9.3. PCI Express Enhanced Capability Header >=20 > 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. >=20 > ... >=20 > 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 >=20 > so, I think it's reasonable to align everything to 4-bytes all the time= . Here's another draft: >=20 >=20 > static int pci_find_space(PCIDevice *pdev, uint32_t start, uint32_t siz= e) > { > int offset =3D start; > int i; > uint32_t *dword_used =3D &pdev->used[start]; >=20 > /* This approach ensures the capability is dword-aligned, as = =20 > required by the PCI and PCI-E specifications */ > for (i =3D start; i < size; i +=3D 4, dword_used++) { > if (*dword_used) > 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 >=20 > return 0; > } >=20 > 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); > } >=20 > 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); > } >=20 >=20