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

[Matt Renzelmann]

> > 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;
    }

    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);
}