[v1,4/8] target_arm: Remove memory region init from armv7m_init

This patch moves the memory region init code from the
armv7m_init function to the stellaris_init function

Signed-off-by: Alistair Francis <alistair23@gmail.com>
---
 hw/arm/armv7m.c    | 15 +--------------
 hw/arm/stellaris.c | 23 +++++++++++++++++++----
 2 files changed, 20 insertions(+), 18 deletions(-)

On Sun, Sep 14, 2014 at 6:18 PM, Alistair Francis <alistair23@gmail.com> wrote:
> This patch moves the memory region init code from the
> armv7m_init function to the stellaris_init function
>
> Signed-off-by: Alistair Francis <alistair23@gmail.com>
> ---
>  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
>
>

Il 15/09/2014 17:20, Peter Crosthwaite ha scritto:
>> > +    /* 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?).

The machine is the default owner anyway.

Paolo

On Tue, Sep 16, 2014 at 1:20 AM, Peter Crosthwaite
<peter.crosthwaite@xilinx.com> wrote:
> On Sun, Sep 14, 2014 at 6:18 PM, Alistair Francis <alistair23@gmail.com> wrote:
>> This patch moves the memory region init code from the
>> armv7m_init function to the stellaris_init function
>>
>> Signed-off-by: Alistair Francis <alistair23@gmail.com>
>> ---
>>  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.

Will fix

>
> 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.

Both are still required. The load_image_targphys() requires the flash size and
there is some code that uses sram_size, although it is constantly #if disabled.
I'm not sure why it's there, but it would have to be removed if the
sram_size is.
It's something to do with greater then 32MB of SRAM overwriting the
bitband area.

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