Re: [Qemu-devel] [Patch v1 4/8] target_arm: Remove memory region init from armv7m_init

[Peter Crosthwaite]

On Sun, Sep 14, 2014 at 6:18 PM, Alistair Francis <address@hidden> wrote:
> This patch moves the memory region init code from the
> armv7m_init function to the stellaris_init function
>
> Signed-off-by: Alistair Francis <address@hidden>
> ---
>  hw/arm/armv7m.c    | 15 +--------------
>  hw/arm/stellaris.c | 23 +++++++++++++++++++----
>  2 files changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index aedef13..5c1f7b3 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -163,7 +163,7 @@ static void armv7m_reset(void *opaque)
>  }
>
>  /* Init CPU and memory for a v7-M based board.
> -   flash_size and sram_size are in kb.
> +   flash_size and sram_size are in bytes.
>     Returns the NVIC array.  */
>
>  qemu_irq *armv7m_init(MemoryRegion *system_memory,
> @@ -180,13 +180,8 @@ qemu_irq *armv7m_init(MemoryRegion *system_memory,
>      uint64_t lowaddr;
>      int i;
>      int big_endian;
> -    MemoryRegion *sram = g_new(MemoryRegion, 1);
> -    MemoryRegion *flash = g_new(MemoryRegion, 1);
>      MemoryRegion *hack = g_new(MemoryRegion, 1);
>
> -    flash_size *= 1024;
> -    sram_size *= 1024;
> -
>      if (cpu_model == NULL) {
>         cpu_model = "cortex-m3";
>      }
> @@ -209,14 +204,6 @@ qemu_irq *armv7m_init(MemoryRegion *system_memory,
>      code_size = ram_size - sram_size;
>  #endif
>
> -    /* Flash programming is done via the SCU, so pretend it is ROM.  */
> -    memory_region_init_ram(flash, NULL, "armv7m.flash", flash_size);
> -    vmstate_register_ram_global(flash);
> -    memory_region_set_readonly(flash, true);
> -    memory_region_add_subregion(system_memory, 0, flash);
> -    memory_region_init_ram(sram, NULL, "armv7m.sram", sram_size);
> -    vmstate_register_ram_global(sram);
> -    memory_region_add_subregion(system_memory, 0x20000000, sram);
>      armv7m_bitband_init();
>
>      nvic = qdev_create(NULL, "armv7m_nvic");
> diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
> index 64bd4b4..3c8e9d1 100644
> --- a/hw/arm/stellaris.c
> +++ b/hw/arm/stellaris.c
> @@ -1220,10 +1220,25 @@ static void stellaris_init(const char 
> *kernel_filename, const char *cpu_model,
>      int i;
>      int j;
>
> -    flash_size = ((board->dc0 & 0xffff) + 1) << 1;
> -    sram_size = (board->dc0 >> 18) + 1;
> -    pic = armv7m_init(get_system_memory(),
> -                      flash_size, sram_size, kernel_filename, cpu_model);
> +    MemoryRegion *sram = g_new(MemoryRegion, 1);
> +    MemoryRegion *flash = g_new(MemoryRegion, 1);
> +    MemoryRegion *system_memory = get_system_memory();
> +
> +    flash_size = (((board->dc0 & 0xffff) + 1) << 1) * 1024;
> +    sram_size = ((board->dc0 >> 18) + 1) * 1024;
> +
> +    /* Flash programming is done via the SCU, so pretend it is ROM.  */
> +    memory_region_init_ram(flash, NULL, "armv7m.flash", flash_size);

You can give this a better name, such as "stellaris.flash" now that we
are not in armv7m anymore.

Paolo, do you have any advice as to whether we should attempt to
parent this MR to the machine or something or just stay NULL
(considering we are in non-qdevified code?).

> +    vmstate_register_ram_global(flash);
> +    memory_region_set_readonly(flash, true);
> +    memory_region_add_subregion(system_memory, 0, flash);
> +
> +    memory_region_init_ram(sram, NULL, "armv7m.sram", sram_size);

Ditto.

> +    vmstate_register_ram_global(sram);
> +    memory_region_add_subregion(system_memory, 0x20000000, sram);
> +
> +    pic = armv7m_init(system_memory, flash_size, sram_size,
> +                      kernel_filename, cpu_model);
>

Can you drop the flash_size and sram_size args to armv7m? They should
be dead now.

Regards,
Peter

>      if (board->dc1 & (1 << 16)) {
>          dev = sysbus_create_varargs(TYPE_STELLARIS_ADC, 0x40038000,
> --
> 1.9.1
>
>