Re: [Qemu-devel] [PATCH] qemu: define a TOM register to report the base of PCI

[Stefano Stabellini]

On Fri, 22 Feb 2013, Xudong Hao wrote:
> Define a TOM(top of memory) register to report the base of PCI memory, update
> memory region dynamically.
> 
> The use case of this register defination is for Xen till now.
> 
> Signed-off-by: Xudong Hao <address@hidden>
> Signed-off-by: Xiantao Zhang <address@hidden>

Thanks for the patch!

Aside from Jan's comment on the pci config endianness, I would also like
to see an #define somewhere in QEMU to specific what's the default TOM.
In particular I wouldn't want 0xe0000000 (and 0xf0000000) to be
hardcoded in more than one place.


>  hw/pc.h       |  4 ---
>  hw/pc_piix.c  |  6 -----
>  hw/piix_pci.c | 79 
> ++++++++++++++++++++++++++++++++++++++++++++---------------
>  3 files changed, 59 insertions(+), 30 deletions(-)
> 
> diff --git a/hw/pc.h b/hw/pc.h
> index fbcf43d..2a60490 100644
> --- a/hw/pc.h
> +++ b/hw/pc.h
> @@ -134,10 +134,6 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int 
> *piix_devfn,
>                      MemoryRegion *address_space_mem,
>                      MemoryRegion *address_space_io,
>                      ram_addr_t ram_size,
> -                    hwaddr pci_hole_start,
> -                    hwaddr pci_hole_size,
> -                    hwaddr pci_hole64_start,
> -                    hwaddr pci_hole64_size,
>                      MemoryRegion *pci_memory,
>                      MemoryRegion *ram_memory);
>  
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index 0a6923d..98cf467 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -130,12 +130,6 @@ static void pc_init1(MemoryRegion *system_memory,
>      if (pci_enabled) {
>          pci_bus = i440fx_init(&i440fx_state, &piix3_devfn, &isa_bus, gsi,
>                                system_memory, system_io, ram_size,
> -                              below_4g_mem_size,
> -                              0x100000000ULL - below_4g_mem_size,
> -                              0x100000000ULL + above_4g_mem_size,
> -                              (sizeof(hwaddr) == 4
> -                               ? 0
> -                               : ((uint64_t)1 << 62)),
>                                pci_memory, ram_memory);
>      } else {
>          pci_bus = NULL;
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index 3d79c73..88bd688 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -86,6 +86,14 @@ struct PCII440FXState {
>  #define I440FX_PAM_SIZE 7
>  #define I440FX_SMRAM    0x72
>  
> +/* The maximum vaule of TOM(top of memory) register in I440FX
> + * is 1G, so it doesn't meet any popular virutal machines, so
> + * define another register to report the base of PCI memory.
> + * Use four bytes 0xb0-0xb3 for this purpose, they are originally
> + * resevered for host bridge.
> + * */
> +#define I440FX_PCI_HOLE_BASE 0xb0
> +
>  static void piix3_set_irq(void *opaque, int pirq, int level);
>  static PCIINTxRoute piix3_route_intx_pin_to_irq(void *opaque, int pci_intx);
>  static void piix3_write_config_xen(PCIDevice *dev,
> @@ -101,6 +109,43 @@ static int pci_slot_get_pirq(PCIDevice *pci_dev, int 
> pci_intx)
>      return (pci_intx + slot_addend) & 3;
>  }
>  
> +
> +static void i440fx_update_pci_mem_hole(PCII440FXState *f, bool del)
> +{
> +    ram_addr_t above_4g_mem_size;
> +    hwaddr pci_hole_start, pci_hole_size, pci_hole64_start, pci_hole64_size;
> +
> +    pci_hole_start = pci_default_read_config(&f->dev, I440FX_PCI_HOLE_BASE, 
> 4);
> +    pci_hole_size = 0x100000000ULL - pci_hole_start;
> +
> +    if (ram_size >= pci_hole_start) {
> +        above_4g_mem_size = ram_size - pci_hole_start;
> +    } else {
> +        above_4g_mem_size = 0;
> +    }
> +    pci_hole64_start = 0x100000000ULL + above_4g_mem_size;
> +    pci_hole64_size = sizeof(hwaddr) == 4 ? 0 : ((uint64_t)1 << 62);
> +
> +    if (del) {
> +        memory_region_del_subregion(f->system_memory, &f->pci_hole);
> +        if (pci_hole64_size) {
> +            memory_region_del_subregion(f->system_memory, 
> &f->pci_hole_64bit);
> +        }
> +    }
> +
> +    memory_region_init_alias(&f->pci_hole, "pci-hole", f->pci_address_space,
> +                             pci_hole_start, pci_hole_size);
> +    memory_region_add_subregion(f->system_memory, pci_hole_start, 
> &f->pci_hole);
> +    memory_region_init_alias(&f->pci_hole_64bit, "pci-hole64",
> +                             f->pci_address_space,
> +                             pci_hole64_start, pci_hole64_size);
> +    if (pci_hole64_size) {
> +        memory_region_add_subregion(f->system_memory, pci_hole64_start,
> +                                    &f->pci_hole_64bit);
> +    }
> +}
> +
> +
>  static void i440fx_update_memory_mappings(PCII440FXState *d)
>  {
>      int i;
> @@ -136,6 +181,9 @@ static void i440fx_write_config(PCIDevice *dev,
>          range_covers_byte(address, len, I440FX_SMRAM)) {
>          i440fx_update_memory_mappings(d);
>      }
> +    if (range_covers_byte(address, len, I440FX_PCI_HOLE_BASE)) {
> +        i440fx_update_pci_mem_hole(d, true);
> +    }
>  }
>  
>  static int i440fx_load_old(QEMUFile* f, void *opaque, int version_id)
> @@ -203,6 +251,16 @@ static int i440fx_initfn(PCIDevice *dev)
>  
>      d->dev.config[I440FX_SMRAM] = 0x02;
>  
> +    /* Emulate top of memory, here use 0xe0000000 as default val*/
> +    if (xen_enabled()) {
> +        d->dev.config[I440FX_PCI_HOLE_BASE] = 0xf0;
> +    } else {
> +        d->dev.config[I440FX_PCI_HOLE_BASE] = 0xe0;
> +    }
> +    d->dev.config[I440FX_PCI_HOLE_BASE + 1] = 0x00;
> +    d->dev.config[I440FX_PCI_HOLE_BASE + 2] = 0x00;
> +    d->dev.config[I440FX_PCI_HOLE_BASE + 3] = 0x00;
> +
>      cpu_smm_register(&i440fx_set_smm, d);
>      return 0;
>  }
> @@ -214,10 +272,6 @@ static PCIBus *i440fx_common_init(const char 
> *device_name,
>                                    MemoryRegion *address_space_mem,
>                                    MemoryRegion *address_space_io,
>                                    ram_addr_t ram_size,
> -                                  hwaddr pci_hole_start,
> -                                  hwaddr pci_hole_size,
> -                                  hwaddr pci_hole64_start,
> -                                  hwaddr pci_hole64_size,
>                                    MemoryRegion *pci_address_space,
>                                    MemoryRegion *ram_memory)
>  {
> @@ -244,16 +298,6 @@ static PCIBus *i440fx_common_init(const char 
> *device_name,
>      f->system_memory = address_space_mem;
>      f->pci_address_space = pci_address_space;
>      f->ram_memory = ram_memory;
> -    memory_region_init_alias(&f->pci_hole, "pci-hole", f->pci_address_space,
> -                             pci_hole_start, pci_hole_size);
> -    memory_region_add_subregion(f->system_memory, pci_hole_start, 
> &f->pci_hole);
> -    memory_region_init_alias(&f->pci_hole_64bit, "pci-hole64",
> -                             f->pci_address_space,
> -                             pci_hole64_start, pci_hole64_size);
> -    if (pci_hole64_size) {
> -        memory_region_add_subregion(f->system_memory, pci_hole64_start,
> -                                    &f->pci_hole_64bit);
> -    }
>      memory_region_init_alias(&f->smram_region, "smram-region",
>                               f->pci_address_space, 0xa0000, 0x20000);
>      memory_region_add_subregion_overlap(f->system_memory, 0xa0000,
> @@ -295,6 +339,7 @@ static PCIBus *i440fx_common_init(const char *device_name,
>      (*pi440fx_state)->dev.config[0x57]=ram_size;
>  
>      i440fx_update_memory_mappings(f);
> +    i440fx_update_pci_mem_hole(f, false);
>  
>      return b;
>  }
> @@ -304,10 +349,6 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int 
> *piix3_devfn,
>                      MemoryRegion *address_space_mem,
>                      MemoryRegion *address_space_io,
>                      ram_addr_t ram_size,
> -                    hwaddr pci_hole_start,
> -                    hwaddr pci_hole_size,
> -                    hwaddr pci_hole64_start,
> -                    hwaddr pci_hole64_size,
>                      MemoryRegion *pci_memory, MemoryRegion *ram_memory)
>  
>  {
> @@ -315,8 +356,6 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int 
> *piix3_devfn,
>  
>      b = i440fx_common_init("i440FX", pi440fx_state, piix3_devfn, isa_bus, 
> pic,
>                             address_space_mem, address_space_io, ram_size,
> -                           pci_hole_start, pci_hole_size,
> -                           pci_hole64_start, pci_hole64_size,
>                             pci_memory, ram_memory);
>      return b;
>  }
> -- 
> 1.7.12.1
>