Re: [Qemu-devel] [PATCH 02/22] acpi-build: remove dependency from ram_addr.h

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

On Thu, Mar 26, 2015 at 06:38:20PM +0100, Paolo Bonzini wrote:
> ram_addr_t is an internal interface, everyone should go through
> MemoryRegion.  Clean it up by making rom_add_blob return a
> MemoryRegion* and using the new qemu_ram_resize infrastructure.
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  hw/core/loader.c     |  8 ++++----
>  hw/i386/acpi-build.c | 36 ++++++++++++++++++------------------
>  include/hw/loader.h  |  8 +++++---
>  3 files changed, 27 insertions(+), 25 deletions(-)
> 
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index d4c441f..7ee675c 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -835,12 +835,12 @@ err:
>      return -1;
>  }
>  
> -ram_addr_t rom_add_blob(const char *name, const void *blob, size_t len,
> +MemoryRegion *rom_add_blob(const char *name, const void *blob, size_t len,
>                     size_t max_len, hwaddr addr, const char *fw_file_name,
>                     FWCfgReadCallback fw_callback, void *callback_opaque)
>  {
>      Rom *rom;
> -    ram_addr_t ret = RAM_ADDR_MAX;
> +    MemoryRegion *mr = NULL;
>  
>      rom           = g_malloc0(sizeof(*rom));
>      rom->name     = g_strdup(name);
> @@ -858,7 +858,7 @@ ram_addr_t rom_add_blob(const char *name, const void *blob, size_t len,
>  
>          if (rom_file_has_mr) {
>              data = rom_set_mr(rom, OBJECT(fw_cfg), devpath);
> -            ret = memory_region_get_ram_addr(rom->mr);
> +            mr = rom->mr;
>          } else {
>              data = rom->data;
>          }
> @@ -867,7 +867,7 @@ ram_addr_t rom_add_blob(const char *name, const void *blob, size_t len,
>                                   fw_callback, callback_opaque,
>                                   data, rom->datasize);
>      }
> -    return ret;
> +    return mr;
>  }
>  
>  /* This function is specific for elf program because we don't need to allocate
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index d0a5c85..ec4d1e8 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -58,7 +58,6 @@
>  
>  #include "qapi/qmp/qint.h"
>  #include "qom/qom-qobject.h"
> -#include "exec/ram_addr.h"
>  
>  /* These are used to size the ACPI tables for -M pc-i440fx-1.7 and
>   * -M pc-i440fx-2.0.  Even if the actual amount of AML generated grows
> @@ -1323,13 +1322,13 @@ static inline void acpi_build_tables_cleanup(AcpiBuildTables *tables, bool mfre)
>  typedef
>  struct AcpiBuildState {
>      /* Copy of table in RAM (for patching). */
> -    ram_addr_t table_ram;
> +    MemoryRegion *table_mr;
>      /* Is table patched? */
>      uint8_t patched;
>      PcGuestInfo *guest_info;
>      void *rsdp;
> -    ram_addr_t rsdp_ram;
> -    ram_addr_t linker_ram;
> +    MemoryRegion *rsdp_mr;
> +    MemoryRegion *linker_mr;
>  } AcpiBuildState;
>  
>  static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg)
> @@ -1513,15 +1512,15 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables *tables)
>      g_array_free(table_offsets, true);
>  }
>  
> -static void acpi_ram_update(ram_addr_t ram, GArray *data)
> +static void acpi_ram_update(MemoryRegion *mr, GArray *data)
>  {
>      uint32_t size = acpi_data_len(data);
>  
>      /* Make sure RAM size is correct - in case it got changed e.g. by migration */
> -    qemu_ram_resize(ram, size, &error_abort);
> +    memory_region_ram_resize(mr, size, &error_abort);
>  
> -    memcpy(qemu_get_ram_ptr(ram), data->data, size);
> -    cpu_physical_memory_set_dirty_range_nocode(ram, size);
> +    memcpy(memory_region_get_ram_ptr(mr), data->data, size);
> +    memory_region_set_dirty(mr, 0, size);
>  }
>  
>  static void acpi_build_update(void *build_opaque, uint32_t offset)
> @@ -1539,15 +1538,15 @@ static void acpi_build_update(void *build_opaque, uint32_t offset)
>  
>      acpi_build(build_state->guest_info, &tables);
>  
> -    acpi_ram_update(build_state->table_ram, tables.table_data);
> +    acpi_ram_update(build_state->table_mr, tables.table_data);
>  
>      if (build_state->rsdp) {
>          memcpy(build_state->rsdp, tables.rsdp->data, acpi_data_len(tables.rsdp));
>      } else {
> -        acpi_ram_update(build_state->rsdp_ram, tables.rsdp);
> +        acpi_ram_update(build_state->rsdp_mr, tables.rsdp);
>      }
>  
> -    acpi_ram_update(build_state->linker_ram, tables.linker);
> +    acpi_ram_update(build_state->linker_mr, tables.linker);
>      acpi_build_tables_cleanup(&tables, true);
>  }
>  
> @@ -1557,8 +1556,9 @@ static void acpi_build_reset(void *build_opaque)
>      build_state->patched = 0;
>  }
>  
> -static ram_addr_t acpi_add_rom_blob(AcpiBuildState *build_state, GArray *blob,
> -                               const char *name, uint64_t max_size)
> +static MemoryRegion *acpi_add_rom_blob(AcpiBuildState *build_state,
> +                                       GArray *blob, const char *name,
> +                                       uint64_t max_size)
>  {
>      return rom_add_blob(name, blob->data, acpi_data_len(blob), max_size, -1,
>                          name, acpi_build_update, build_state);
> @@ -1604,12 +1604,12 @@ void acpi_setup(PcGuestInfo *guest_info)
>      acpi_build(build_state->guest_info, &tables);
>  
>      /* Now expose it all to Guest */
> -    build_state->table_ram = acpi_add_rom_blob(build_state, tables.table_data,
> +    build_state->table_mr = acpi_add_rom_blob(build_state, tables.table_data,
>                                                 ACPI_BUILD_TABLE_FILE,
>                                                 ACPI_BUILD_TABLE_MAX_SIZE);
> -    assert(build_state->table_ram != RAM_ADDR_MAX);
> +    assert(build_state->table_mr != NULL);
>  
> -    build_state->linker_ram =
> +    build_state->linker_mr =
>          acpi_add_rom_blob(build_state, tables.linker, "etc/table-loader", 0);
>  
>      fw_cfg_add_file(guest_info->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
> @@ -1627,10 +1627,10 @@ void acpi_setup(PcGuestInfo *guest_info)
>          fw_cfg_add_file_callback(guest_info->fw_cfg, ACPI_BUILD_RSDP_FILE,
>                                   acpi_build_update, build_state,
>                                   build_state->rsdp, rsdp_size);
> -        build_state->rsdp_ram = (ram_addr_t)-1;
> +        build_state->rsdp_mr = NULL;
>      } else {
>          build_state->rsdp = NULL;
> -        build_state->rsdp_ram = acpi_add_rom_blob(build_state, tables.rsdp,
> +        build_state->rsdp_mr = acpi_add_rom_blob(build_state, tables.rsdp,
>                                                    ACPI_BUILD_RSDP_FILE, 0);
>      }
>  
> diff --git a/include/hw/loader.h b/include/hw/loader.h
> index 4f0681b..485ff8f 100644
> --- a/include/hw/loader.h
> +++ b/include/hw/loader.h
> @@ -68,9 +68,11 @@ extern bool rom_file_has_mr;
>  int rom_add_file(const char *file, const char *fw_dir,
>                   hwaddr addr, int32_t bootindex,
>                   bool option_rom);
> -ram_addr_t rom_add_blob(const char *name, const void *blob, size_t len,
> -                   size_t max_len, hwaddr addr, const char *fw_file_name,
> -                   FWCfgReadCallback fw_callback, void *callback_opaque);
> +MemoryRegion *rom_add_blob(const char *name, const void *blob, size_t len,
> +                           size_t max_len, hwaddr addr,
> +                           const char *fw_file_name,
> +                           FWCfgReadCallback fw_callback,
> +                           void *callback_opaque);
>  int rom_add_elf_program(const char *name, void *data, size_t datasize,
>                          size_t romsize, hwaddr addr);
>  int rom_load_all(void);
> -- 
> 2.3.3
>