On Wed, Dec 28, 2011 at 06:35:55PM +1300, Alexey Korolev wrote:
> All devices behind a bridge need to have all their regions consecutive and
> not overlapping with all the normal memory ranges.
> Since prefetchable memory is described by one record, we must avoid the
> situations
> when 32bit and 64bit prefetchable regions are present within one secondary
> bus.
How do we avoid this? Assume we have two devices:
a 32 bit and a 64 bit one, behind a bridge.
There are two main things we can do:
1. Make the 64 bit device only use the low 32 bit
2. Put the 32 bit one in the non-prefetcheable range
1 probably makes more sense for small BARs
2 probably makes more sense for large ones
Try also looking at e.g. linux bus scanning code for more ideas.
Another thing I don't see addressed here is that support for 64 bit
ranges is I think optional in the bridge.
>
> Signed-off-by: Alexey Korolev<alexey.koro...@endace.com>
Whitespace is corrupted: checkyour mail setup?
There should be spaces around operators:
a < b, I see a< b. Sometimes a< b (two spaces after).
> ---
> src/pciinit.c | 69 +++++++++++++++++++++++++++++++++++++++-----------------
> 1 files changed, 48 insertions(+), 21 deletions(-)
>
> diff --git a/src/pciinit.c b/src/pciinit.c
> index a574e38..92942d5 100644
> --- a/src/pciinit.c
> +++ b/src/pciinit.c
> @@ -17,6 +17,7 @@
>
> #define PCI_BRIDGE_IO_MIN 0x1000
> #define PCI_BRIDGE_MEM_MIN 0x100000
> +#define PCI_BRIDGE_MEM_MAX 0x80000000
>
> enum pci_region_type {
> PCI_REGION_TYPE_IO,
> @@ -45,6 +46,7 @@ struct pci_bus {
> s64 base;
> s64 bases[32 - PCI_MEM_INDEX_SHIFT];
> } r[PCI_REGION_TYPE_COUNT];
> + int is64;
> struct pci_device *bus_dev;
> };
>
> @@ -369,6 +371,26 @@ static void pci_bios_bus_reserve(struct pci_bus *bus,
> int type, u32 size)
> bus->r[type].max = size;
> }
>
> +static void pci_bios_secondary_bus_reserve(struct pci_bus *parent,
> + struct pci_bus *s, int type)
> +{
> + u32 limit = (type == PCI_REGION_TYPE_IO) ?
> + PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
> +
> + if (s->r[type].sum> PCI_BRIDGE_MEM_MAX) {
> + panic("Size: %08x%08x is too big\n",
> + (u32)s->r[type].sum, (u32)(s->r[type].sum>>32));
> + }
> + s->r[type].size = (u32)s->r[type].sum;
> + if (s->r[type].size< limit)
> + s->r[type].size = limit;
> + s->r[type].size = pci_size_roundup(s->r[type].size);
> +
> + pci_bios_bus_reserve(parent, type, s->r[type].size);
> + dprintf(1, " size: %x, type %s\n",
> + s->r[type].size, region_type_name[type]);
> +}
> +
> static void pci_bios_check_devices(struct pci_bus *busses)
> {
> dprintf(1, "PCI: check devices\n");
> @@ -392,8 +414,10 @@ static void pci_bios_check_devices(struct pci_bus
> *busses)
> pci_bios_bus_reserve(bus, type, size);
> pci->bars[i].addr = val;
> pci->bars[i].size = size;
> - if (type == PCI_REGION_TYPE_PREFMEM_64)
> + if (type == PCI_REGION_TYPE_PREFMEM_64) {
> + bus->is64 = 1;
> i++;
> + }
> }
> }
>
> @@ -404,22 +428,21 @@ static void pci_bios_check_devices(struct pci_bus
> *busses)
> if (!s->bus_dev)
> continue;
> struct pci_bus *parent =&busses[pci_bdf_to_bus(s->bus_dev->bdf)];
> +
> + if (s->r[PCI_REGION_TYPE_PREFMEM_64].sum&&
Space before && here and elsewhere.
> + s->r[PCI_REGION_TYPE_PREFMEM].sum) {
> + panic("Sparse PCI prefmem regions on the bus %d\n",
> secondary_bus);
> + }
> +
> + dprintf(1, "PCI: secondary bus %d\n", secondary_bus);
> int type;
> for (type = 0; type< PCI_REGION_TYPE_COUNT; type++) {
> - u32 limit = (type == PCI_REGION_TYPE_IO) ?
> - PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN;
> - s->r[type].size = s->r[type].sum;
> - if (s->r[type].size< limit)
> - s->r[type].size = limit;
> - s->r[type].size = pci_size_roundup(s->r[type].size);
> - pci_bios_bus_reserve(parent, type, s->r[type].size);
> - }
> - dprintf(1, "PCI: secondary bus %d sizes: io %x, mem %x, prefmem
> %x\n",
> - secondary_bus,
> - s->r[PCI_REGION_TYPE_IO].size,
> - s->r[PCI_REGION_TYPE_MEM].size,
> - s->r[PCI_REGION_TYPE_PREFMEM].size);
> - }
> + if ((type == PCI_REGION_TYPE_PREFMEM_64&& !s->is64) ||
> + (type == PCI_REGION_TYPE_PREFMEM&& s->is64))
> + continue;
Can't figure this out. What does this do?
> + pci_bios_secondary_bus_reserve(parent, s, type);
> + }
> + }
> }
>
> #define ROOT_BASE(top, sum, max) ALIGN_DOWN((top)-(sum),(max) ?: 1)
> @@ -507,14 +530,17 @@ static void pci_bios_map_devices(struct pci_bus *busses)
> struct pci_bus *parent =&busses[pci_bdf_to_bus(bdf)];
> int type;
> for (type = 0; type< PCI_REGION_TYPE_COUNT; type++) {
> + if ((type == PCI_REGION_TYPE_PREFMEM_64&& !s->is64) ||
> + (type == PCI_REGION_TYPE_PREFMEM&& s->is64))
> + continue;
> s->r[type].base = pci_bios_bus_get_addr(
> parent, type, s->r[type].size);
> }
> dprintf(1, "PCI: init bases bus %d (secondary)\n", secondary_bus);
> pci_bios_init_bus_bases(s);
>
> - u32 base = s->r[PCI_REGION_TYPE_IO].base;
> - u32 limit = base + s->r[PCI_REGION_TYPE_IO].size - 1;
> + s64 base = s->r[PCI_REGION_TYPE_IO].base;
> + s64 limit = base + s->r[PCI_REGION_TYPE_IO].size - 1;
> pci_config_writeb(bdf, PCI_IO_BASE, base>> PCI_IO_SHIFT);
> pci_config_writew(bdf, PCI_IO_BASE_UPPER16, 0);
> pci_config_writeb(bdf, PCI_IO_LIMIT, limit>> PCI_IO_SHIFT);
> @@ -525,12 +551,13 @@ static void pci_bios_map_devices(struct pci_bus *busses)
> pci_config_writew(bdf, PCI_MEMORY_BASE, base>> PCI_MEMORY_SHIFT);
> pci_config_writew(bdf, PCI_MEMORY_LIMIT, limit>> PCI_MEMORY_SHIFT);
>
> - base = s->r[PCI_REGION_TYPE_PREFMEM].base;
> - limit = base + s->r[PCI_REGION_TYPE_PREFMEM].size - 1;
> + type = s->is64 ? PCI_REGION_TYPE_PREFMEM_64 :
> PCI_REGION_TYPE_PREFMEM;
> + base = s->r[type].base;
> + limit = base + s->r[type].size - 1;
> pci_config_writew(bdf, PCI_PREF_MEMORY_BASE, base>>
> PCI_PREF_MEMORY_SHIFT);
> pci_config_writew(bdf, PCI_PREF_MEMORY_LIMIT, limit>>
> PCI_PREF_MEMORY_SHIFT);
> - pci_config_writel(bdf, PCI_PREF_BASE_UPPER32, 0);
> - pci_config_writel(bdf, PCI_PREF_LIMIT_UPPER32, 0);
> + pci_config_writel(bdf, PCI_PREF_BASE_UPPER32, base>> 32);
> + pci_config_writel(bdf, PCI_PREF_LIMIT_UPPER32, limit>> 32);
> }
>
> // Map regions on each device.
> --
> 1.7.5.4
>
