[Qemu-devel] [PATCH for-2.3] arm: memory: Replace memory_region_init_ram with memory_region_allocate_system_memory

[Qemu-devel] [PATCH for-2.3] arm: memory: Replace memory_region_init_ram with memory_region_allocate_system_memory

[Dirk Müller]

Commit 0b183fc871:"memory: move mem_path handling to
memory_region_allocate_system_memory" split memory_region_init_ram and
memory_region_init_ram_from_file. Also it moved mem-path handling a step
up from memory_region_init_ram to memory_region_allocate_system_memory.

Therefore for any board that uses memory_region_init_ram directly,
-mem-path is not supported.

Fix this by replacing memory_region_init_ram with
memory_region_allocate_system_memory.

Signed-off-by: Dirk Mueller <dmueller@suse.com>
---
 hw/arm/cubieboard.c   |  5 ++---
 hw/arm/digic_boards.c |  3 +--
 hw/arm/exynos4210.c   | 12 ++++++------
 hw/arm/highbank.c     |  2 +-
 hw/arm/integratorcp.c |  4 ++--
 hw/arm/kzm.c          |  3 +--
 hw/arm/musicpal.c     |  5 ++---
 hw/arm/omap1.c        |  7 ++++---
 hw/arm/omap2.c        |  6 +++---
 hw/arm/realview.c     | 10 ++++------
 hw/arm/strongarm.c    |  7 ++++---
 hw/arm/versatilepb.c  |  5 ++---
 hw/arm/vexpress.c     | 10 ++++------
 hw/arm/virt.c         |  5 ++---
 hw/arm/xilinx_zynq.c  |  5 ++---
 15 files changed, 40 insertions(+), 49 deletions(-)

diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
index d1e53be..1582250 100644
--- a/hw/arm/cubieboard.c
+++ b/hw/arm/cubieboard.c
@@ -63,9 +63,8 @@ static void cubieboard_init(MachineState *machine)
         exit(1);
     }

-    memory_region_init_ram(&s->sdram, NULL, "cubieboard.ram",
-                           machine->ram_size, &error_abort);
-    vmstate_register_ram_global(&s->sdram);
+    memory_region_allocate_system_memory(&s->sdram, NULL, "cubieboard.ram",
+                                         machine->ram_size);
     memory_region_add_subregion(get_system_memory(), AW_A10_SDRAM_BASE,
                                 &s->sdram);

diff --git a/hw/arm/digic_boards.c b/hw/arm/digic_boards.c
index e576646..f8ba9e5 100644
--- a/hw/arm/digic_boards.c
+++ b/hw/arm/digic_boards.c
@@ -51,9 +51,8 @@ typedef struct DigicBoard {

 static void digic4_board_setup_ram(DigicBoardState *s, hwaddr ram_size)
 {
-    memory_region_init_ram(&s->ram, NULL, "ram", ram_size, &error_abort);
+    memory_region_allocate_system_memory(&s->ram, NULL, "ram", ram_size);
     memory_region_add_subregion(get_system_memory(), 0, &s->ram);
-    vmstate_register_ram_global(&s->ram);
 }

 static void digic4_board_init(DigicBoard *board)
diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index c55fab8..63ea4a9 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -283,16 +283,16 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
     /* DRAM */
     mem_size = ram_size;
     if (mem_size > EXYNOS4210_DRAM_MAX_SIZE) {
-        memory_region_init_ram(&s->dram1_mem, NULL, "exynos4210.dram1",
-                mem_size - EXYNOS4210_DRAM_MAX_SIZE, &error_abort);
-        vmstate_register_ram_global(&s->dram1_mem);
+        unsigned long dram1_size = mem_size - EXYNOS4210_DRAM_MAX_SIZE;
+        memory_region_allocate_system_memory(&s->dram1_mem, NULL,
+                                             "exynos4210.dram1",
+                                             dram1_size);
         memory_region_add_subregion(system_mem, EXYNOS4210_DRAM1_BASE_ADDR,
                 &s->dram1_mem);
         mem_size = EXYNOS4210_DRAM_MAX_SIZE;
     }
-    memory_region_init_ram(&s->dram0_mem, NULL, "exynos4210.dram0", mem_size,
-                           &error_abort);
-    vmstate_register_ram_global(&s->dram0_mem);
+    memory_region_allocate_system_memory(&s->dram0_mem, NULL,
+                                         "exynos4210.dram0", mem_size);
     memory_region_add_subregion(system_mem, EXYNOS4210_DRAM0_BASE_ADDR,
             &s->dram0_mem);

diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index ddd10dc..4458315 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -267,7 +267,7 @@ static void calxeda_init(MachineState *machine,
enum cxmachines machine_id)

     sysmem = get_system_memory();
     dram = g_new(MemoryRegion, 1);
-    memory_region_init_ram(dram, NULL, "highbank.dram", ram_size,
&error_abort);
+    memory_region_allocate_system_memory(dram, NULL, "highbank.dram",
ram_size);
     /* SDRAM at address zero.  */
     memory_region_add_subregion(sysmem, 0, dram);

diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index cb609cd..0fbbf99 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -567,8 +567,8 @@ static void integratorcp_init(MachineState *machine)

     cpu = ARM_CPU(cpuobj);

-    memory_region_init_ram(ram, NULL, "integrator.ram", ram_size,
&error_abort);
-    vmstate_register_ram_global(ram);
+    memory_region_allocate_system_memory(ram, NULL, "integrator.ram",
+                                         ram_size);
     /* ??? On a real system the first 1Mb is mapped as SSRAM or boot flash.  */
     /* ??? RAM should repeat to fill physical memory space.  */
     /* SDRAM at address zero*/
diff --git a/hw/arm/kzm.c b/hw/arm/kzm.c
index 94ceab6..5be0369 100644
--- a/hw/arm/kzm.c
+++ b/hw/arm/kzm.c
@@ -97,8 +97,7 @@ static void kzm_init(MachineState *machine)

     /* On a real system, the first 16k is a `secure boot rom' */

-    memory_region_init_ram(ram, NULL, "kzm.ram", ram_size, &error_abort);
-    vmstate_register_ram_global(ram);
+    memory_region_allocate_system_memory(ram, NULL, "kzm.ram", ram_size);
     memory_region_add_subregion(address_space_mem, KZM_RAMADDRESS, ram);

     memory_region_init_alias(ram_alias, NULL, "ram.alias", ram, 0, ram_size);
diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index 3712de6..a3b1314 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -1600,9 +1600,8 @@ static void musicpal_init(MachineState *machine)
     }

     /* For now we use a fixed - the original - RAM size */
-    memory_region_init_ram(ram, NULL, "musicpal.ram", MP_RAM_DEFAULT_SIZE,
-                           &error_abort);
-    vmstate_register_ram_global(ram);
+    memory_region_allocate_system_memory(ram, NULL, "musicpal.ram",
+                                         MP_RAM_DEFAULT_SIZE);
     memory_region_add_subregion(address_space_mem, 0, ram);

     memory_region_init_ram(sram, NULL, "musicpal.sram", MP_SRAM_SIZE,
diff --git a/hw/arm/omap1.c b/hw/arm/omap1.c
index 0615044..91ffb58 100644
--- a/hw/arm/omap1.c
+++ b/hw/arm/omap1.c
@@ -16,6 +16,8 @@
  * You should have received a copy of the GNU General Public License along
  * with this program; if not, see <http://www.gnu.org/licenses/>.
  */
+
+#include "hw/boards.h"
 #include "hw/hw.h"
 #include "hw/arm/arm.h"
 #include "hw/arm/omap.h"
@@ -3878,9 +3880,8 @@ struct omap_mpu_state_s
*omap310_mpu_init(MemoryRegion *system_memory,
     omap_clk_init(s);

     /* Memory-mapped stuff */
-    memory_region_init_ram(&s->emiff_ram, NULL, "omap1.dram", s->sdram_size,
-                           &error_abort);
-    vmstate_register_ram_global(&s->emiff_ram);
+    memory_region_allocate_system_memory(&s->emiff_ram, NULL, "omap1.dram",
+                                         s->sdram_size);
     memory_region_add_subregion(system_memory, OMAP_EMIFF_BASE, &s->emiff_ram);
     memory_region_init_ram(&s->imif_ram, NULL, "omap1.sram", s->sram_size,
                            &error_abort);
diff --git a/hw/arm/omap2.c b/hw/arm/omap2.c
index 3239014..e39b317 100644
--- a/hw/arm/omap2.c
+++ b/hw/arm/omap2.c
@@ -20,6 +20,7 @@

 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
+#include "hw/boards.h"
 #include "hw/hw.h"
 #include "hw/arm/arm.h"
 #include "hw/arm/omap.h"
@@ -2271,9 +2272,8 @@ struct omap_mpu_state_s
*omap2420_mpu_init(MemoryRegion *sysmem,
     omap_clk_init(s);

     /* Memory-mapped stuff */
-    memory_region_init_ram(&s->sdram, NULL, "omap2.dram", s->sdram_size,
-                           &error_abort);
-    vmstate_register_ram_global(&s->sdram);
+    memory_region_allocate_system_memory(&s->sdram, NULL, "omap2.dram",
+                                         s->sdram_size);
     memory_region_add_subregion(sysmem, OMAP2_Q2_BASE, &s->sdram);
     memory_region_init_ram(&s->sram, NULL, "omap2.sram", s->sram_size,
                            &error_abort);
diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index ef2788d..4ed5ceb 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -150,15 +150,13 @@ static void realview_init(MachineState *machine,
         ram_lo = g_new(MemoryRegion, 1);
         low_ram_size = ram_size - 0x20000000;
         ram_size = 0x20000000;
-        memory_region_init_ram(ram_lo, NULL, "realview.lowmem", low_ram_size,
-                               &error_abort);
-        vmstate_register_ram_global(ram_lo);
+        memory_region_allocate_system_memory(ram_lo, NULL, "realview.lowmem",
+                                             low_ram_size);
         memory_region_add_subregion(sysmem, 0x20000000, ram_lo);
     }

-    memory_region_init_ram(ram_hi, NULL, "realview.highmem", ram_size,
-                           &error_abort);
-    vmstate_register_ram_global(ram_hi);
+    memory_region_allocate_system_memory(ram_hi, NULL, "realview.highmem",
+                                         ram_size);
     low_ram_size = ram_size;
     if (low_ram_size > 0x10000000)
       low_ram_size = 0x10000000;
diff --git a/hw/arm/strongarm.c b/hw/arm/strongarm.c
index 3206345..1ddea6d 100644
--- a/hw/arm/strongarm.c
+++ b/hw/arm/strongarm.c
@@ -26,6 +26,8 @@
  *  Contributions after 2012-01-13 are licensed under the terms of the
  *  GNU GPL, version 2 or (at your option) any later version.
  */
+
+#include "hw/boards.h"
 #include "hw/sysbus.h"
 #include "strongarm.h"
 #include "qemu/error-report.h"
@@ -1604,9 +1606,8 @@ StrongARMState *sa1110_init(MemoryRegion *sysmem,
         exit(1);
     }

-    memory_region_init_ram(&s->sdram, NULL, "strongarm.sdram", sdram_size,
-                           &error_abort);
-    vmstate_register_ram_global(&s->sdram);
+    memory_region_allocate_system_memory(&s->sdram, NULL, "strongarm.sdram",
+                                         sdram_size);
     memory_region_add_subregion(sysmem, SA_SDCS0, &s->sdram);

     s->pic = sysbus_create_varargs("strongarm_pic", 0x90050000,
diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
index 624fdb0..6c69f4e 100644
--- a/hw/arm/versatilepb.c
+++ b/hw/arm/versatilepb.c
@@ -226,9 +226,8 @@ static void versatile_init(MachineState *machine,
int board_id)

     cpu = ARM_CPU(cpuobj);

-    memory_region_init_ram(ram, NULL, "versatile.ram", machine->ram_size,
-                           &error_abort);
-    vmstate_register_ram_global(ram);
+    memory_region_allocate_system_memory(ram, NULL, "versatile.ram",
+                                         machine->ram_size);
     /* ??? RAM should repeat to fill physical memory space.  */
     /* SDRAM at address zero.  */
     memory_region_add_subregion(sysmem, 0, ram);
diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index e9a7ced..c99234c 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -276,9 +276,8 @@ static void a9_daughterboard_init(const
VexpressMachineState *vms,
         exit(1);
     }

-    memory_region_init_ram(ram, NULL, "vexpress.highmem", ram_size,
-                           &error_abort);
-    vmstate_register_ram_global(ram);
+    memory_region_allocate_system_memory(ram, NULL, "vexpress.highmem",
+                                         ram_size);
     low_ram_size = ram_size;
     if (low_ram_size > 0x4000000) {
         low_ram_size = 0x4000000;
@@ -371,9 +370,8 @@ static void a15_daughterboard_init(const
VexpressMachineState *vms,
         }
     }

-    memory_region_init_ram(ram, NULL, "vexpress.highmem", ram_size,
-                           &error_abort);
-    vmstate_register_ram_global(ram);
+    memory_region_allocate_system_memory(ram, NULL, "vexpress.highmem",
+                                         ram_size);
     /* RAM is from 0x80000000 upwards; there is no low-memory alias for it. */
     memory_region_add_subregion(sysmem, 0x80000000, ram);

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index b652b07..766bad7 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -799,9 +799,8 @@ static void machvirt_init(MachineState *machine)
     fdt_add_cpu_nodes(vbi);
     fdt_add_psci_node(vbi);

-    memory_region_init_ram(ram, NULL, "mach-virt.ram", machine->ram_size,
-                           &error_abort);
-    vmstate_register_ram_global(ram);
+    memory_region_allocate_system_memory(ram, NULL, "mach-virt.ram",
+                                         machine->ram_size);
     memory_region_add_subregion(sysmem, vbi->memmap[VIRT_MEM].base, ram);

     create_flash(vbi);
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 5c37521..a4e7b5c 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -161,9 +161,8 @@ static void zynq_init(MachineState *machine)
     }

     /* DDR remapped to address zero.  */
-    memory_region_init_ram(ext_ram, NULL, "zynq.ext_ram", ram_size,
-                           &error_abort);
-    vmstate_register_ram_global(ext_ram);
+    memory_region_allocate_system_memory(ext_ram, NULL, "zynq.ext_ram",
+                                         ram_size);
     memory_region_add_subregion(address_space_mem, 0, ext_ram);

     /* 256K of on-chip memory */
-- 
2.0.4

Re: [Qemu-devel] [PATCH for-2.3] arm: memory: Replace memory_region_init_ram with memory_region_allocate_system_memory

[Paolo Bonzini]

On 24/03/2015 22:24, Dirk Müller wrote:
> +++ b/hw/arm/exynos4210.c
> @@ -283,16 +283,16 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
>      /* DRAM */
>      mem_size = ram_size;
>      if (mem_size > EXYNOS4210_DRAM_MAX_SIZE) {
> -        memory_region_init_ram(&s->dram1_mem, NULL, "exynos4210.dram1",
> -                mem_size - EXYNOS4210_DRAM_MAX_SIZE, &error_abort);
> -        vmstate_register_ram_global(&s->dram1_mem);
> +        unsigned long dram1_size = mem_size - EXYNOS4210_DRAM_MAX_SIZE;
> +        memory_region_allocate_system_memory(&s->dram1_mem, NULL,
> +                                             "exynos4210.dram1",
> +                                             dram1_size);
>          memory_region_add_subregion(system_mem, EXYNOS4210_DRAM1_BASE_ADDR,
>                  &s->dram1_mem);
>          mem_size = EXYNOS4210_DRAM_MAX_SIZE;
>      }
> -    memory_region_init_ram(&s->dram0_mem, NULL, "exynos4210.dram0", mem_size,
> -                           &error_abort);
> -    vmstate_register_ram_global(&s->dram0_mem);
> +    memory_region_allocate_system_memory(&s->dram0_mem, NULL,
> +                                         "exynos4210.dram0", mem_size);
>      memory_region_add_subregion(system_mem, EXYNOS4210_DRAM0_BASE_ADDR,
>              &s->dram0_mem);

You cannot call memory_region_allocate_system_memory twice.  For cases
like this one, the right thing to do is to create a region with
memory_region_allocate_system_memory, and then replace
memory_region_init_ram with memory_region_init_alias.  This "slices" the
region created by memory_region_allocate_system_memory.

Paolo

Re: [Qemu-devel] [PATCH for-2.3] arm: memory: Replace memory_region_init_ram with memory_region_allocate_system_memory

[Dirk Müller]

Hi Paolo,

> You cannot call memory_region_allocate_system_memory twice.  For cases
> like this one, the right thing to do is to create a region with
> memory_region_allocate_system_memory, and then replace
> memory_region_init_ram with memory_region_init_alias.  This "slices" the
> region created by memory_region_allocate_system_memory.

Thanks for the review and the explanation. Like this?

--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -144,6 +144,7 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
     DeviceState *dev;
     SysBusDevice *busdev;
     ObjectClass *cpu_oc;
+    MemoryRegion *ram_all = g_new(MemoryRegion, 1);

     cpu_oc = cpu_class_by_name(TYPE_ARM_CPU, "cortex-a9");
     assert(cpu_oc);
@@ -281,18 +282,22 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
                                 &s->iram_mem);

     /* DRAM */
+    memory_region_allocate_system_memory(ram_all, NULL, "exynos4210.ram",
+                                         ram_size);
+
     mem_size = ram_size;
     if (mem_size > EXYNOS4210_DRAM_MAX_SIZE) {
-        memory_region_init_ram(&s->dram1_mem, NULL, "exynos4210.dram1",
-                mem_size - EXYNOS4210_DRAM_MAX_SIZE, &error_abort);
-        vmstate_register_ram_global(&s->dram1_mem);
+        memory_region_init_alias(&s->dram1_mem, NULL, "exynos4210.dram1",
+                                 ram_all,
+                                 EXYNOS4210_DRAM1_BASE_ADDR,
+                                 mem_size - EXYNOS4210_DRAM_MAX_SIZE);
         memory_region_add_subregion(system_mem, EXYNOS4210_DRAM1_BASE_ADDR,
                 &s->dram1_mem);
         mem_size = EXYNOS4210_DRAM_MAX_SIZE;
     }
-    memory_region_init_ram(&s->dram0_mem, NULL, "exynos4210.dram0", mem_size,
-                           &error_abort);
-    vmstate_register_ram_global(&s->dram0_mem);
+    memory_region_init_alias(&s->dram1_mem, NULL, "exynos4210.dram0",
+                                ram_all,
+                                EXYNOS4210_DRAM0_BASE_ADDR, mem_size);
     memory_region_add_subregion(system_mem, EXYNOS4210_DRAM0_BASE_ADDR,
             &s->dram0_mem);

Re: [Qemu-devel] [PATCH for-2.3] arm: memory: Replace memory_region_init_ram with memory_region_allocate_system_memory

[Peter Maydell]

On 24 March 2015 at 21:24, Dirk Müller <dirk@dmllr.de> wrote:
> Commit 0b183fc871:"memory: move mem_path handling to
> memory_region_allocate_system_memory" split memory_region_init_ram and
> memory_region_init_ram_from_file. Also it moved mem-path handling a step
> up from memory_region_init_ram to memory_region_allocate_system_memory.
>
> Therefore for any board that uses memory_region_init_ram directly,
> -mem-path is not supported.
>
> Fix this by replacing memory_region_init_ram with
> memory_region_allocate_system_memory.
>
> Signed-off-by: Dirk Mueller <dmueller@suse.com>
> ---
>  hw/arm/cubieboard.c   |  5 ++---
>  hw/arm/digic_boards.c |  3 +--
>  hw/arm/exynos4210.c   | 12 ++++++------
>  hw/arm/highbank.c     |  2 +-
>  hw/arm/integratorcp.c |  4 ++--
>  hw/arm/kzm.c          |  3 +--
>  hw/arm/musicpal.c     |  5 ++---
>  hw/arm/omap1.c        |  7 ++++---
>  hw/arm/omap2.c        |  6 +++---
>  hw/arm/realview.c     | 10 ++++------
>  hw/arm/strongarm.c    |  7 ++++---
>  hw/arm/versatilepb.c  |  5 ++---
>  hw/arm/vexpress.c     | 10 ++++------
>  hw/arm/virt.c         |  5 ++---
>  hw/arm/xilinx_zynq.c  |  5 ++---

So why do only the ARM boards get fixes here?
(Why did we allow ourselves to break half our boards
in commit 0b183fc871 in the first place?)

-- PMM

Re: [Qemu-devel] [PATCH for-2.3] arm: memory: Replace memory_region_init_ram with memory_region_allocate_system_memory

[Peter Maydell]

On 25 March 2015 at 00:10, Peter Maydell <peter.maydell@linaro.org> wrote:
> So why do only the ARM boards get fixes here?

...ah, I see you've submitted patches for other boards too,
you just didn't put them together into a single series. Sorry.

-- PMM

Re: [Qemu-devel] [PATCH for-2.3] arm: memory: Replace memory_region_init_ram with memory_region_allocate_system_memory

[Paolo Bonzini]

On 25/03/2015 01:10, Peter Maydell wrote:
> So why do only the ARM boards get fixes here?
> (Why did we allow ourselves to break half our boards
> in commit 0b183fc871 in the first place?)

Because the patches to convert them were in my stolen laptop. :/

Paolo

Re: [Qemu-devel] [PATCH for-2.3] arm: memory: Replace memory_region_init_ram with memory_region_allocate_system_memory

[Dirk Müller]

Hi Peter,

>> So why do only the ARM boards get fixes here?
> ...ah, I see you've submitted patches for other boards too,
> you just didn't put them together into a single series. Sorry.

Some other architectures were already fixed (x86 and ppc) and I'll be
working through the other arches shortly. I've tested this on vexpress
and virt.

Greetings,
Dirk