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 = PCI_CONFIG_HEADER_SIZE;
> > > int i;
> > > uint32_t *dword_used = &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:
>
> ============
> 7.9. PCI Express Extended Capabilities
>
> PCI Express Extended Capability registers are located in device 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 – 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.
> ============
>
> 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 = start;
> int i;
> uint32_t *dword_used = &pdev->used[start];
>
> /* This approach ensures the capability is dword-aligned, as
>
> required by the PCI and PCI-E specifications */
> for (i = start; i < size; i += 4, dword_used++) {
> if (*dword_used)
> offset = i + 4;
> else if (i - offset + 4 >= 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_SPACE_SIZE);
> }
>
> static int pci_find_express_space(PCIDevice *pdev, uint16_t size) {
> assert (pci_config_size(pdev) >= PCIE_CONFIG_SPACE_SIZE);
> return pci_find_space(pdev, PCI_CONFIG_SPACE_SIZE,
> PCIE_CONFIG_SPACE_SIZE);
> }
>
>
