Re: [Qemu-devel] [PATCH 2/2] pc: add 'etc/reserved-memory-end' fw_cfg interface for SeaBIOS

[Paolo Bonzini]

Il 12/11/2013 14:58, Igor Mammedov ha scritto:
> 'etc/reserved-memory-end' will allow QEMU to tell BIOS where PCI
> BARs mapping could safely start in high memory.
> 
> Allowing BIOS to start mapping 64-bit PCI BARs at address where it
> wouldn't conflict with other mappings QEMU might place before it.
> 
> That permits QEMU to reserve extra address space before
> 64-bit PCI hole for memory hotplug.

I may be royally wrong, but I think the new file should only be added to
new machine types.  Otherwise, after migrating old machine types from
new QEMU to old QEMU, you may end up with PCI BARs mapped outside the
"PCI windows" that exist until before patch 1/2 of this series.

Does this make sense?

Also, would it make sense to use the e820 interface that Gerd has added,
instead of a new fw_cfg file?

Thanks,

Paolo


> Signed-off-by: Igor Mammedov <address@hidden>
> ---
>  hw/i386/pc.c         | 14 +++++++++++++-
>  hw/pci-host/piix.c   |  3 ++-
>  hw/pci-host/q35.c    |  3 ++-
>  include/hw/i386/pc.h |  3 ++-
>  4 files changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 6c82ada..b504047 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1095,11 +1095,23 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t 
> below_4g_mem_size,
>  
>  /* setup pci memory address space mapping into system address space */
>  void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
> -                            MemoryRegion *pci_address_space)
> +                            MemoryRegion *pci_address_space,
> +                            uint64_t reserved_memory_end)
>  {
> +    uint64_t *val;
> +    FWCfgState *fw_cfg = fw_cfg_find();
> +
>      /* Set to lower priority than RAM */
>      memory_region_add_subregion_overlap(system_memory, 0x0,
>                                          pci_address_space, -1);
> +    g_assert(fw_cfg);
> +    /*
> +     *  Align address at 1G, this makes sure it can be exactly covered
> +     *  with a PAT entry even when using huge pages.
> +     */
> +    val = g_malloc(sizeof(*val));
> +    *val = cpu_to_le64(ROUND_UP(reserved_memory_end, 0x1ULL << 30));
> +    fw_cfg_add_file(fw_cfg, "etc/reserved-memory-end", val, sizeof(*val));
>  }
>  
>  void pc_acpi_init(const char *default_dsdt)
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 5d4e290..16205e7 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -351,7 +351,8 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
>  
>      /* setup pci memory mapping */
>      pc_pci_as_mapping_init(OBJECT(f), f->system_memory,
> -                           f->pci_address_space);
> +                           f->pci_address_space,
> +                           0x100000000ULL + above_4g_mem_size);
>  
>      memory_region_init_alias(&f->smram_region, OBJECT(d), "smram-region",
>                               f->pci_address_space, 0xa0000, 0x20000);
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index d1792de..1293353 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -353,7 +353,8 @@ static int mch_init(PCIDevice *d)
>  
>      /* setup pci memory mapping */
>      pc_pci_as_mapping_init(OBJECT(mch), mch->system_memory,
> -                           mch->pci_address_space);
> +                           mch->pci_address_space,
> +                           0x100000000ULL + mch->above_4g_mem_size);
>  
>      /* smram */
>      cpu_smm_register(&mch_set_smm, mch);
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 8b3be3c..2663046 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -130,7 +130,8 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t 
> below_4g_mem_size,
>  
>  
>  void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
> -                            MemoryRegion *pci_address_space);
> +                            MemoryRegion *pci_address_space,
> +                            uint64_t reserved_memory_end);
>  
>  FWCfgState *pc_memory_init(MemoryRegion *system_memory,
>                             const char *kernel_filename,
>