qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Don Slutz
Subject: Re: [Qemu-devel] [PATCH v3] Align PCI capabilities in pci_find_space
Date: Wed, 26 Sep 2012 14:02:21 -0400
User-agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:15.0) Gecko/20120907 Thunderbird/15.0.1

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 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)
I would think that dword_used needs to be based on i not start.
             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);
}





   -Don Slutz



reply via email to

[Prev in Thread] Current Thread [Next in Thread]