Re: [Qemu-devel] [PATCH 2/3] Add a new PCI region type to supports 64 bit ranges

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Dec 28, 2011 at 06:26:05PM +1300, Alexey Korolev wrote:
> This patch adds PCI_REGION_TYPE_PREFMEM_64 region type and modifies types of
> variables to make it possible to work with 64 bit addresses.
> 
> Why I've added just one region type PCI_REGION_TYPE_PREFMEM_64 and haven't
> added PCI_REGION_TYPE_MEM_64? According to PCI architecture
> specification, the
> bridges can describe 64bit ranges for prefetchable type of memory
> only. So it's very
> unlikely that devices exporting 64bit non-prefetchable BARs.

Might happen for system devices I guess.

> Anyway
> this code will work
> with 64bit non-prefetchable BARs unless the PCI device is not behind
> the secondary bus.

So what happens if such a device is on root bus?

> 
> Signed-off-by: Alexey Korolev <alexey.korolev@endace.com>
> ---
>  src/pci.h     |    1 -
>  src/pciinit.c |   59
> +++++++++++++++++++++++++++++++++-----------------------
>  2 files changed, 35 insertions(+), 25 deletions(-)
> 
> diff --git a/src/pci.h b/src/pci.h
> index a2a5a4c..71f15fe 100644
> --- a/src/pci.h
> +++ b/src/pci.h
> @@ -54,7 +54,6 @@ struct pci_device {
>      struct {
>          u32 addr;
>          u32 size;
> -        int is64;
>      } bars[PCI_NUM_REGIONS];
> 
>      // Local information on device.
> diff --git a/src/pciinit.c b/src/pciinit.c
> index 7d83368..a574e38 100644
> --- a/src/pciinit.c
> +++ b/src/pciinit.c
> @@ -22,6 +22,7 @@ enum pci_region_type {
>      PCI_REGION_TYPE_IO,
>      PCI_REGION_TYPE_MEM,
>      PCI_REGION_TYPE_PREFMEM,
> +    PCI_REGION_TYPE_PREFMEM_64,
>      PCI_REGION_TYPE_COUNT,
>  };
> 
> @@ -29,18 +30,20 @@ static const char *region_type_name[] = {
>      [ PCI_REGION_TYPE_IO ]      = "io",
>      [ PCI_REGION_TYPE_MEM ]     = "mem",
>      [ PCI_REGION_TYPE_PREFMEM ] = "prefmem",
> +    [ PCI_REGION_TYPE_PREFMEM_64 ] = "prefmem64",
>  };
> 
>  struct pci_bus {
>      struct {
> -        /* pci region stats */
> -        u32 count[32 - PCI_MEM_INDEX_SHIFT];
> -        u32 sum, max;
>          /* seconday bus region sizes */
>          u32 size;
> -        /* pci region assignments */
> -        u32 bases[32 - PCI_MEM_INDEX_SHIFT];
> -        u32 base;
> +        /* pci region stats */
> +        u32 max;
> +        u32 count[32 - PCI_MEM_INDEX_SHIFT];
> +        s64 sum;
> +         /* pci region assignments */
> +        s64 base;
> +        s64 bases[32 - PCI_MEM_INDEX_SHIFT];
>      } r[PCI_REGION_TYPE_COUNT];
>      struct pci_device *bus_dev;
>  };
> @@ -69,6 +72,8 @@ static enum pci_region_type pci_addr_to_type(u32 addr)
>  {
>      if (addr & PCI_BASE_ADDRESS_SPACE_IO)
>          return PCI_REGION_TYPE_IO;
> +    if (addr & PCI_BASE_ADDRESS_MEM_TYPE_64)
> +        return PCI_REGION_TYPE_PREFMEM_64;
>      if (addr & PCI_BASE_ADDRESS_MEM_PREFETCH)
>          return PCI_REGION_TYPE_PREFMEM;
>      return PCI_REGION_TYPE_MEM;
> @@ -378,19 +383,16 @@ static void pci_bios_check_devices(struct
> pci_bus *busses)
>          struct pci_bus *bus = &busses[pci_bdf_to_bus(pci->bdf)];
>          int i;
>          for (i = 0; i < PCI_NUM_REGIONS; i++) {
> -            u32 val, size;
> +            u32 val, size, type;
>              pci_bios_get_bar(pci, i, &val, &size);
>              if (val == 0)
>                  continue;
> 
> -            pci_bios_bus_reserve(bus, pci_addr_to_type(val), size);
> +            type = pci_addr_to_type(val);
> +            pci_bios_bus_reserve(bus, type, size);
>              pci->bars[i].addr = val;
>              pci->bars[i].size = size;
> -            pci->bars[i].is64 = (!(val & PCI_BASE_ADDRESS_SPACE_IO) &&
> -                                 (val & PCI_BASE_ADDRESS_MEM_TYPE_MASK)
> -                                 == PCI_BASE_ADDRESS_MEM_TYPE_64);
> -
> -            if (pci->bars[i].is64)
> +            if (type == PCI_REGION_TYPE_PREFMEM_64)
>                  i++;
>          }
>      }
> @@ -426,6 +428,7 @@ static void pci_bios_check_devices(struct
> pci_bus *busses)
>  static int pci_bios_init_root_regions(struct pci_bus *bus, u32
> start, u32 end)
>  {
>      bus->r[PCI_REGION_TYPE_IO].base = 0xc000;
> +    bus->r[PCI_REGION_TYPE_PREFMEM_64].base = BUILD_PCIMEM_64_START;
> 
>      int reg1 = PCI_REGION_TYPE_PREFMEM, reg2 = PCI_REGION_TYPE_MEM;
>      if (bus->r[reg1].sum < bus->r[reg2].sum) {
> @@ -449,29 +452,34 @@ static int pci_bios_init_root_regions(struct
> pci_bus *bus, u32 start, u32 end)
> 
>  static void pci_bios_init_bus_bases(struct pci_bus *bus)
>  {
> -    u32 base, newbase, size;
> +    u32 size;
> +    s64 base, newbase;
>      int type, i;
> 
>      for (type = 0; type < PCI_REGION_TYPE_COUNT; type++) {
> -        dprintf(1, "  type %s max %x sum %x base %x\n",
> region_type_name[type],
> -                bus->r[type].max, bus->r[type].sum, bus->r[type].base);
> +        dprintf(1, "  type %s max %x sum 0x%08x%08x base %08x%08x\n",
> +                region_type_name[type], bus->r[type].max,
> +                (u32)(bus->r[type].sum>>32), (u32)bus->r[type].sum,
> +                (u32)(bus->r[type].base>>32), (u32)bus->r[type].base);
>          base = bus->r[type].base;
>          for (i = ARRAY_SIZE(bus->r[type].count)-1; i >= 0; i--) {
>              size = pci_index_to_size(i, type);
>              if (!bus->r[type].count[i])
>                  continue;
>              newbase = base + size * bus->r[type].count[i];
> -            dprintf(1, "    size %8x: %d bar(s), %8x -> %8x\n",
> -                    size, bus->r[type].count[i], base, newbase - 1);
> +            dprintf(1, "    size %8x: %d bar(s), %08x%08x -> %08x%08x\n",
> +                    size, bus->r[type].count[i], (u32)(base>>32),
> (u32)base,
> +                    (u32)((newbase - 1)>>32), (u32)newbase - 1);
>              bus->r[type].bases[i] = base;
>              base = newbase;
>          }
>      }
>  }
> 
> -static u32 pci_bios_bus_get_addr(struct pci_bus *bus, int type, u32 size)
> +static s64 pci_bios_bus_get_addr(struct pci_bus *bus, int type, u32 size)
>  {
> -    u32 index, addr;
> +    u32 index;
> +    s64 addr;
> 
>      index = pci_size_to_index(size, type);
>      addr = bus->r[type].bases[index];
> @@ -540,13 +548,16 @@ static void pci_bios_map_devices(struct
> pci_bus *busses)
>                  continue;
> 
>              int type = pci_addr_to_type(pci->bars[i].addr);
> -            u32 addr = pci_bios_bus_get_addr(bus, type, pci->bars[i].size);
> -            dprintf(1, "  bar %d, addr %x, size %x [%s]\n",
> -                    i, addr, pci->bars[i].size, region_type_name[type]);
> +            s64 addr = pci_bios_bus_get_addr(bus, type, pci->bars[i].size);
> +            dprintf(1, "  bar %d, addr %08x%08x, size %x [%s]\n",
> +                    i, (u32)(addr >> 32), (u32)addr, pci->bars[i].size,
> +                    region_type_name[type]);
>              pci_set_io_region_addr(pci, i, addr);
> 
> -            if (pci->bars[i].is64)
> +            if (type == PCI_REGION_TYPE_PREFMEM_64) {
>                  i++;
> +                pci_set_io_region_addr(pci, i, addr >> 32);
> +            }
>          }
>      }
>  }
> -- 
> 1.7.5.4