Re: [Qemu-devel] [PATCH 4/4] pc: add 'etc/pcimem64-minimum-address' fw_cfg interface to SeaBIOS

[Michael S. Tsirkin]

On Wed, Oct 16, 2013 at 10:49:14AM +0200, Igor Mammedov wrote:
> 'etc/pcimem64-minimum-address' will allow QEMU to communicate to BIOS
> where PCI memory address space mapping starts in high memory.
> 
> Allowing BIOS start mapping 64-bit PCI BARs at address where QEMU
> placed this mapping vs. hardcoded value right after highmem RAM.
> 
> That will allow QEMU to reserve extra address space before
> 64-bit PCI hole for memory hotplug.
> 
> Signed-off-by: Igor Mammedov <address@hidden>
> ---
>   * SeaBIOS patch: http://patchwork.ozlabs.org/patch/283623/
> 
> ---
>  hw/i386/pc.c         |    9 ++++++++-
>  hw/i386/pc_piix.c    |    2 +-
>  hw/pci-host/piix.c   |    5 +++--
>  hw/pci-host/q35.c    |    4 +++-
>  include/hw/i386/pc.h |    5 +++--
>  5 files changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 7ecb028..ac99ae1 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1055,7 +1055,7 @@ PcGuestInfo *pc_guest_info_init(ram_addr_t 
> below_4g_mem_size,
>  
>  /* setup pci memory regions mappings into system address space */
>  void pc_pci_as_mapping_init(Object *owner, MemoryRegion *system_memory,
> -                            MemoryRegion *pci_address_space,
> +                            MemoryRegion *pci_address_space, FWCfgState 
> *fw_cfg,
>                              MemoryRegion *pci32_as, MemoryRegion *pci64_as,
>                              uint32_t pci32_as_start, uint32_t pci32_as_size,
>                              uint64_t pci64_as_start, uint64_t pci64_as_size)
> @@ -1083,6 +1083,13 @@ void pc_pci_as_mapping_init(Object *owner, 
> MemoryRegion *system_memory,
>          memory_region_add_subregion(system_memory,
>                                      ROUND_UP(pci64_as_start, 0x1ULL << 30),
>                                      pci64_as);
> +        if (fw_cfg) {
> +            uint64_t *pcimem64_start = g_malloc(sizeof(*pcimem64_start));
> +
> +            *pcimem64_start = cpu_to_le64(pci64_as_start);
> +            fw_cfg_add_file(fw_cfg, "etc/pcimem64-minimum-address",
> +                            pcimem64_start, sizeof(*pcimem64_start));
> +        }
>      }
>  }
>  
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 2fed5fd..966c48a 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -148,7 +148,7 @@ static void pc_init1(QEMUMachineInitArgs *args,
>                                system_memory, system_io,
>                                below_4g_mem_size,
>                                above_4g_mem_size,
> -                              pci_memory, ram_memory);
> +                              pci_memory, ram_memory, fw_cfg);
>      } else {
>          pci_bus = NULL;
>          i440fx_state = NULL;
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index d422b25..9347099 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -314,7 +314,8 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
>                      hwaddr below_4g_mem_size,
>                      ram_addr_t above_4g_mem_size,
>                      MemoryRegion *pci_address_space,
> -                    MemoryRegion *ram_memory)
> +                    MemoryRegion *ram_memory,
> +                    FWCfgState *fw_cfg)
>  {
>      DeviceState *dev;
>      PCIBus *b;
> @@ -353,7 +354,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state,
>      }
>  
>      pc_pci_as_mapping_init(OBJECT(d), f->system_memory,
> -                           f->pci_address_space,
> +                           f->pci_address_space, fw_cfg,
>                             &f->pci_hole, &f->pci_hole_64bit,
>                             below_4g_mem_size,
>                             0x100000000ULL - below_4g_mem_size,
> diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
> index cb8aea0..583585c 100644
> --- a/hw/pci-host/q35.c
> +++ b/hw/pci-host/q35.c
> @@ -30,6 +30,7 @@
>  #include "hw/hw.h"
>  #include "hw/pci-host/q35.h"
>  #include "qapi/visitor.h"
> +#include "hw/nvram/fw_cfg.h"
>  
>  /****************************************************************************
>   * Q35 host
> @@ -336,10 +337,11 @@ static int mch_init(PCIDevice *d)
>  {
>      int i;
>      MCHPCIState *mch = MCH_PCI_DEVICE(d);
> +    FWCfgState *fw_cfg = FW_CFG(object_resolve_path(FW_CFG_PATH, NULL));

Seems to duplicate code from fw_cfg_find().
Why not just use it?

>  
>      /* setup pci memory regions */
>      pc_pci_as_mapping_init(OBJECT(mch), mch->system_memory,
> -                           mch->pci_address_space,
> +                           mch->pci_address_space, fw_cfg,
>                             &mch->pci_hole, &mch->pci_hole_64bit,
>                             mch->below_4g_mem_size,
>                             0x100000000ULL - mch->below_4g_mem_size,

Or call fw_cfg_find internally in pc_pci_as_mapping_init?

> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index ed86642..6d0d8c9 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -110,7 +110,7 @@ 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, FWCfgState 
> *fw_cfg,
>                              MemoryRegion *pci32_as, MemoryRegion *pci64_as,
>                              uint32_t pci32_as_start, uint32_t pci32_as_size,
>                              uint64_t pci64_as_start, uint64_t pci64_as_size);
> @@ -164,7 +164,8 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int 
> *piix_devfn,
>                      hwaddr below_4g_mem_size,
>                      ram_addr_t above_4g_mem_size,
>                      MemoryRegion *pci_memory,
> -                    MemoryRegion *ram_memory);
> +                    MemoryRegion *ram_memory,
> +                    FWCfgState *fw_cfg);
>  
>  /* piix4.c */
>  extern PCIDevice *piix4_dev;
> -- 
> 1.7.1