On Wed, 2012-09-26 at 11:50 -0500, Matt Renzelmann wrote:
> > >
> > > hw/pci.c | 28 +++++++++++++++++++++-------
> > > 1 files changed, 21 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/hw/pci.c b/hw/pci.c
> > > index f855cf3..2217dda 100644
> > > --- a/hw/pci.c
> > > +++ b/hw/pci.c
> > > @@ -1626,16 +1626,30 @@ PCIDevice *pci_create_simple(PCIBus *bus, int
> > > devfn,
> > const char *name)
> > > return pci_create_simple_multifunction(bus, devfn, false, name);
> > > }
> > >
> > > -static int pci_find_space(PCIDevice *pdev, uint8_t size)
> > > +static int pci_find_space(PCIDevice *pdev, uint8_t size, bool
> > > include_pcie)
> > > {
> > > - int config_size = pci_config_size(pdev);
> > > + int config_size;
> > > int offset = PCI_CONFIG_HEADER_SIZE;
> > > int i;
> > > - for (i = PCI_CONFIG_HEADER_SIZE; i < config_size; ++i)
> > > - if (pdev->used[i])
> > > - offset = i + 1;
> > > - else if (i - offset + 1 == size)
> > > + uint32_t *dword_used = &pdev->used[PCI_CONFIG_HEADER_SIZE];
> > > +
> > > + if (include_pcie) {
> > > + assert (pci_config_size(pdev) >= PCIE_CONFIG_SPACE_SIZE);
> > > + config_size = PCIE_CONFIG_SPACE_SIZE;
> > > + } else {
> > > + config_size = PCI_CONFIG_SPACE_SIZE;
> > > + }
> > > +
> > > + /* This approach ensures the capability is dword-aligned, as
> > > + required by the PCI specification */
> > > + for (i = PCI_CONFIG_HEADER_SIZE; i < config_size; i += 4,
> > > dword_used++)
> > {
> >
> > I don't believe there's ever a case where a driver would want space and
> > not care if it's in standard or extended config space. They'll want one
> > or the other. So we'd be searching two distinct ranges. Thanks,
> >
> > Alex
>
> Ah, that makes sense, so would something like this work? I can re-send as a
> patch once we've got it.
>
> 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:
pci_find_space(PCIDevice *pdev, unsigned start, unsigned size, unsigned
align)
Then a pci_find_legacy_space and pci_find_express_space could both call
into it. Thanks,
Alex
>
> if (pcie_space) {
> assert (pci_config_size(pdev) >= PCIE_CONFIG_SPACE_SIZE);
> config_base = PCI_CONFIG_SPACE_SIZE;
> config_size = PCIE_CONFIG_SPACE_SIZE;
> } else {
> config_base = PCI_CONFIG_HEADER_SIZE;
> config_size = PCI_CONFIG_SPACE_SIZE;
> }
>
> /* This approach ensures the capability is dword-aligned, as
>
> required by the PCI specification */
> for (i = config_base; i < config_size; i += 4, dword_used++) {
>
> if (*dword_used)
> offset = i + 4;
> else if (i - offset + 4 >= size)
> return offset;
> }
>
> return 0;
> }
>
> Thanks for all your help with this,
> Matt
>
> >
> > > + if (*dword_used) {
> > > + offset = i + 4;
> > > + } else if (i - offset + 4 >= size) {
> > > return offset;
> > > + }
> > > + }
> > > +
> > > return 0;
> > > }
> > >
> > > @@ -1826,7 +1840,7 @@ int pci_add_capability(PCIDevice *pdev, uint8_t
> > cap_id,
> > > int i, overlapping_cap;
> > >
> > > if (!offset) {
> > > - offset = pci_find_space(pdev, size);
> > > + offset = pci_find_space(pdev, size, false);
> > > if (!offset) {
> > > return -ENOSPC;
> > > }
> >
> >
>
>
