Re: [Qemu-devel] [PATCH v3] Align PCI capabilities in pci_find_space

[Alex Williamson]

On Wed, 2012-09-26 at 13:05 -0500, Matt Renzelmann wrote:
> > 
> > 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,
> > 
> 
> Ah of course, how's this:
> 
> static int pci_find_space(PCIDevice *pdev, uint32_t start,
>                           uint32_t end, 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 < end; i += 4, dword_used++) { 
>         if (*dword_used)
>             offset = i + 4;
>         else if (i - offset + 4 >= size)
>             return offset;
>     }
> 
>     return 0;
> }

Looks reasonable to me

> 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) {
>     assert (pci_config_size(pdev) >= PCIE_CONFIG_SPACE_SIZE);

This could be done generically in pci_find_space

assert(pci_config_size(pdev) >= end)

You could also add an alignment check

assert(!(start & 0x3))

Thanks,

Alex

>     return pci_find_space(pdev, PCI_CONFIG_SPACE_SIZE,
>                           PCIE_CONFIG_SPACE_SIZE, size);
> }
>