[PATCH 00/21] hw: Let the machine be the owner of the system memory

[PATCH 00/21] hw: Let the machine be the owner of the system memory

[Philippe Mathieu-Daudé]

Hi,

This series is based on Igor's "eliminate remaining places that
abuse memory_region_allocate_system_memory()":
https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg01601.html

It is quite simple, we enforce all machines to be the QOM owner
of the system memory.

This changes the memory tree from:

  (qemu) info mtree -o
  memory-region: pc.ram
    0000000000000000-0000000007ffffff (prio 0, ram): pc.ram parent:{obj path=/machine/unattached}

to:

  (qemu) info mtree -o
  memory-region: pc.ram
    0000000000000000-0000000007ffffff (prio 0, ram): pc.ram owner:{obj path=/machine}

Few patches are required to clean the codebase first, to unify the
creation of the system memory in the board/machine code. Mostly some
old ARM machines (pre-QOM) were affected.

Please review (as a generic codebase cleanup).

Regards,

Phil.

Based-on: <20191008113318.7012-1-imammedo@redhat.com>

Philippe Mathieu-Daudé (21):
  hw/arm/xilinx_zynq: Use the IEC binary prefix definitions
  hw/arm/mps2: Use the IEC binary prefix definitions
  hw/arm/collie: Create the RAM in the board
  hw/arm/omap2: Create the RAM in the board
  hw/arm/omap1: Create the RAM in the board
  hw/arm/digic4: Inline digic4_board_setup_ram() function
  hw: Drop QOM ownership on memory_region_allocate_system_memory() calls
  hw/alpha/dp264: Create the RAM in the board
  hw: Let memory_region_allocate_system_memory take MachineState
    argument
  hw/core: Let the machine be the owner of the system memory
  hw/alpha: Let the machine be the owner of the system memory
  hw/arm: Let the machine be the owner of the system memory
  hw/cris: Let the machine be the owner of the system memory
  hw/hppa: Let the machine be the owner of the system memory
  hw/i386: Let the machine be the owner of the system memory
  hw/lm32: Let the machine be the owner of the system memory
  hw/m68k: Let the machine be the owner of the system memory
  hw/mips: Let the machine be the owner of the system memory
  hw/ppc: Let the machine be the owner of the system memory
  hw/sparc: Let the machine be the owner of the system memory
  hw/core: Assert memory_region_allocate_system_memory has machine owner

 hw/alpha/alpha_sys.h      |  2 +-
 hw/alpha/dp264.c          | 11 ++++++++++-
 hw/alpha/typhoon.c        |  9 +--------
 hw/arm/aspeed.c           |  2 +-
 hw/arm/collie.c           |  8 ++++++--
 hw/arm/cubieboard.c       |  2 +-
 hw/arm/digic_boards.c     | 14 +++++---------
 hw/arm/highbank.c         |  3 ++-
 hw/arm/imx25_pdk.c        |  2 +-
 hw/arm/integratorcp.c     |  2 +-
 hw/arm/kzm.c              |  2 +-
 hw/arm/mcimx6ul-evk.c     |  2 +-
 hw/arm/mcimx7d-sabre.c    |  2 +-
 hw/arm/mps2-tz.c          |  5 +++--
 hw/arm/mps2.c             |  5 +++--
 hw/arm/musicpal.c         |  2 +-
 hw/arm/nseries.c          | 10 +++++++---
 hw/arm/omap1.c            | 12 +++++-------
 hw/arm/omap2.c            | 13 +++++--------
 hw/arm/omap_sx1.c         |  8 ++++++--
 hw/arm/palm.c             |  8 ++++++--
 hw/arm/raspi.c            |  2 +-
 hw/arm/sabrelite.c        |  2 +-
 hw/arm/sbsa-ref.c         |  2 +-
 hw/arm/strongarm.c        |  7 +------
 hw/arm/strongarm.h        |  4 +---
 hw/arm/versatilepb.c      |  2 +-
 hw/arm/vexpress.c         |  4 ++--
 hw/arm/virt.c             |  2 +-
 hw/arm/xilinx_zynq.c      |  5 +++--
 hw/arm/xlnx-versal-virt.c |  2 +-
 hw/arm/xlnx-zcu102.c      |  2 +-
 hw/core/null-machine.c    |  2 +-
 hw/core/numa.c            |  9 +++++----
 hw/cris/axis_dev88.c      |  2 +-
 hw/hppa/machine.c         |  2 +-
 hw/i386/pc.c              |  2 +-
 hw/lm32/lm32_boards.c     |  4 ++--
 hw/lm32/milkymist.c       |  2 +-
 hw/m68k/an5206.c          |  2 +-
 hw/m68k/mcf5208.c         |  2 +-
 hw/m68k/next-cube.c       |  2 +-
 hw/mips/boston.c          |  2 +-
 hw/mips/mips_fulong2e.c   |  3 ++-
 hw/mips/mips_jazz.c       |  2 +-
 hw/mips/mips_malta.c      |  2 +-
 hw/mips/mips_mipssim.c    |  2 +-
 hw/mips/mips_r4k.c        |  3 ++-
 hw/ppc/e500.c             |  3 ++-
 hw/ppc/mac_newworld.c     |  3 ++-
 hw/ppc/mac_oldworld.c     |  2 +-
 hw/ppc/pnv.c              |  2 +-
 hw/ppc/ppc405_boards.c    |  6 +++---
 hw/ppc/prep.c             |  3 ++-
 hw/ppc/spapr.c            |  2 +-
 hw/ppc/virtex_ml507.c     |  2 +-
 hw/sparc/leon3.c          |  2 +-
 hw/sparc/sun4m.c          |  2 +-
 include/hw/arm/omap.h     | 10 +++-------
 include/hw/boards.h       |  6 ++++--
 60 files changed, 127 insertions(+), 117 deletions(-)

-- 
2.21.0

[PATCH 01/21] hw/arm/xilinx_zynq: Use the IEC binary prefix definitions

[Philippe Mathieu-Daudé]

IEC binary prefixes ease code review: the unit is explicit.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/arm/xilinx_zynq.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index c14774e542..3a0fa5b23f 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -16,6 +16,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/units.h"
 #include "qapi/error.h"
 #include "cpu.h"
 #include "hw/sysbus.h"
@@ -194,7 +195,7 @@ static void zynq_init(MachineState *machine)
     memory_region_add_subregion(address_space_mem, 0, ext_ram);
 
     /* 256K of on-chip memory */
-    memory_region_init_ram(ocm_ram, NULL, "zynq.ocm_ram", 256 << 10,
+    memory_region_init_ram(ocm_ram, NULL, "zynq.ocm_ram", 256 * KiB,
                            &error_fatal);
     memory_region_add_subregion(address_space_mem, 0xFFFC0000, ocm_ram);
 
-- 
2.21.0

[PATCH 02/21] hw/arm/mps2: Use the IEC binary prefix definitions

[Philippe Mathieu-Daudé]

IEC binary prefixes ease code review: the unit is explicit.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/arm/mps2-tz.c | 3 ++-
 hw/arm/mps2.c    | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
index 6b24aaacde..f8b620bcc6 100644
--- a/hw/arm/mps2-tz.c
+++ b/hw/arm/mps2-tz.c
@@ -38,6 +38,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/units.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "hw/arm/boot.h"
@@ -458,7 +459,7 @@ static void mps2tz_common_init(MachineState *machine)
      * call the 16MB our "system memory", as it's the largest lump.
      */
     memory_region_allocate_system_memory(&mms->psram,
-                                         NULL, "mps.ram", 0x01000000);
+                                         NULL, "mps.ram", 16 * MiB);
     memory_region_add_subregion(system_memory, 0x80000000, &mms->psram);
 
     /* The overflow IRQs for all UARTs are ORed together.
diff --git a/hw/arm/mps2.c b/hw/arm/mps2.c
index 10efff36b2..d002b126d3 100644
--- a/hw/arm/mps2.c
+++ b/hw/arm/mps2.c
@@ -23,6 +23,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/units.h"
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "hw/arm/boot.h"
@@ -146,7 +147,7 @@ static void mps2_common_init(MachineState *machine)
      * zbt_boot_ctrl is always zero).
      */
     memory_region_allocate_system_memory(&mms->psram,
-                                         NULL, "mps.ram", 0x1000000);
+                                         NULL, "mps.ram", 16 * MiB);
     memory_region_add_subregion(system_memory, 0x21000000, &mms->psram);
 
     switch (mmc->fpga_type) {
-- 
2.21.0

[PATCH 03/21] hw/arm/collie: Create the RAM in the board

[Philippe Mathieu-Daudé]

The SDRAM is incorrectly created in the SA1110 SoC.
Move its creation in the board code, this will later allow the
board to have the QOM ownership of the RAM.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/arm/collie.c    | 8 ++++++--
 hw/arm/strongarm.c | 7 +------
 hw/arm/strongarm.h | 4 +---
 3 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/hw/arm/collie.c b/hw/arm/collie.c
index b1288ccea8..970a4405cc 100644
--- a/hw/arm/collie.c
+++ b/hw/arm/collie.c
@@ -27,9 +27,13 @@ static void collie_init(MachineState *machine)
 {
     StrongARMState *s;
     DriveInfo *dinfo;
-    MemoryRegion *sysmem = get_system_memory();
+    MemoryRegion *sdram = g_new(MemoryRegion, 1);
 
-    s = sa1110_init(sysmem, collie_binfo.ram_size, machine->cpu_type);
+    s = sa1110_init(machine->cpu_type);
+
+    memory_region_allocate_system_memory(sdram, NULL, "strongarm.sdram",
+                                         collie_binfo.ram_size);
+    memory_region_add_subregion(get_system_memory(), SA_SDCS0, sdram);
 
     dinfo = drive_get(IF_PFLASH, 0, 0);
     pflash_cfi01_register(SA_CS0, "collie.fl1", 0x02000000,
diff --git a/hw/arm/strongarm.c b/hw/arm/strongarm.c
index dc65d88a65..6bee034914 100644
--- a/hw/arm/strongarm.c
+++ b/hw/arm/strongarm.c
@@ -1586,8 +1586,7 @@ static const TypeInfo strongarm_ssp_info = {
 };
 
 /* Main CPU functions */
-StrongARMState *sa1110_init(MemoryRegion *sysmem,
-                            unsigned int sdram_size, const char *cpu_type)
+StrongARMState *sa1110_init(const char *cpu_type)
 {
     StrongARMState *s;
     int i;
@@ -1601,10 +1600,6 @@ StrongARMState *sa1110_init(MemoryRegion *sysmem,
 
     s->cpu = ARM_CPU(cpu_create(cpu_type));
 
-    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,
                     qdev_get_gpio_in(DEVICE(s->cpu), ARM_CPU_IRQ),
                     qdev_get_gpio_in(DEVICE(s->cpu), ARM_CPU_FIQ),
diff --git a/hw/arm/strongarm.h b/hw/arm/strongarm.h
index e98840b461..192821f6aa 100644
--- a/hw/arm/strongarm.h
+++ b/hw/arm/strongarm.h
@@ -55,7 +55,6 @@ enum {
 
 typedef struct {
     ARMCPU *cpu;
-    MemoryRegion sdram;
     DeviceState *pic;
     DeviceState *gpio;
     DeviceState *ppc;
@@ -63,7 +62,6 @@ typedef struct {
     SSIBus *ssp_bus;
 } StrongARMState;
 
-StrongARMState *sa1110_init(MemoryRegion *sysmem,
-                            unsigned int sdram_size, const char *rev);
+StrongARMState *sa1110_init(const char *cpu_type);
 
 #endif
-- 
2.21.0

[PATCH 04/21] hw/arm/omap2: Create the RAM in the board

[Philippe Mathieu-Daudé]

The SDRAM is incorrectly created in the OMAP2420 SoC.
Move its creation in the board code, this will later allow the
board to have the QOM ownership of the RAM.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/arm/nseries.c      | 10 +++++++---
 hw/arm/omap2.c        | 13 +++++--------
 include/hw/arm/omap.h |  4 +---
 3 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
index a36971d39a..11f2c193f3 100644
--- a/hw/arm/nseries.c
+++ b/hw/arm/nseries.c
@@ -1311,11 +1311,15 @@ static int n810_atag_setup(const struct arm_boot_info *info, void *p)
 static void n8x0_init(MachineState *machine,
                       struct arm_boot_info *binfo, int model)
 {
-    MemoryRegion *sysmem = get_system_memory();
+    MemoryRegion *sdram = g_new(MemoryRegion, 1);
     struct n800_s *s = (struct n800_s *) g_malloc0(sizeof(*s));
-    int sdram_size = binfo->ram_size;
+    uint64_t sdram_size = binfo->ram_size;
 
-    s->mpu = omap2420_mpu_init(sysmem, sdram_size, machine->cpu_type);
+    memory_region_allocate_system_memory(sdram, NULL, "omap2.dram",
+                                         sdram_size);
+    memory_region_add_subregion(get_system_memory(), OMAP2_Q2_BASE, sdram);
+
+    s->mpu = omap2420_mpu_init(sdram, machine->cpu_type);
 
     /* Setup peripherals
      *
diff --git a/hw/arm/omap2.c b/hw/arm/omap2.c
index bd7ddff983..457f152bac 100644
--- a/hw/arm/omap2.c
+++ b/hw/arm/omap2.c
@@ -22,6 +22,7 @@
 #include "qemu/error-report.h"
 #include "qapi/error.h"
 #include "cpu.h"
+#include "exec/address-spaces.h"
 #include "sysemu/blockdev.h"
 #include "sysemu/qtest.h"
 #include "sysemu/reset.h"
@@ -2276,8 +2277,7 @@ static const struct dma_irq_map omap2_dma_irq_map[] = {
     { 0, OMAP_INT_24XX_SDMA_IRQ3 },
 };
 
-struct omap_mpu_state_s *omap2420_mpu_init(MemoryRegion *sysmem,
-                unsigned long sdram_size,
+struct omap_mpu_state_s *omap2420_mpu_init(MemoryRegion *sdram,
                 const char *cpu_type)
 {
     struct omap_mpu_state_s *s = g_new0(struct omap_mpu_state_s, 1);
@@ -2286,11 +2286,11 @@ struct omap_mpu_state_s *omap2420_mpu_init(MemoryRegion *sysmem,
     int i;
     SysBusDevice *busdev;
     struct omap_target_agent_s *ta;
+    MemoryRegion *sysmem = get_system_memory();
 
     /* Core */
     s->mpu_model = omap2420;
     s->cpu = ARM_CPU(cpu_create(cpu_type));
-    s->sdram_size = sdram_size;
     s->sram_size = OMAP242X_SRAM_SIZE;
 
     s->wakeup = qemu_allocate_irq(omap_mpu_wakeup, s, 0);
@@ -2299,9 +2299,6 @@ struct omap_mpu_state_s *omap2420_mpu_init(MemoryRegion *sysmem,
     omap_clk_init(s);
 
     /* Memory-mapped stuff */
-    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_fatal);
     memory_region_add_subregion(sysmem, OMAP2_SRAM_BASE, &s->sram);
@@ -2338,8 +2335,8 @@ struct omap_mpu_state_s *omap2420_mpu_init(MemoryRegion *sysmem,
     s->port->addr_valid = omap2_validate_addr;
 
     /* Register SDRAM and SRAM ports for fast DMA transfers.  */
-    soc_dma_port_add_mem(s->dma, memory_region_get_ram_ptr(&s->sdram),
-                         OMAP2_Q2_BASE, s->sdram_size);
+    soc_dma_port_add_mem(s->dma, memory_region_get_ram_ptr(sdram),
+                         OMAP2_Q2_BASE, memory_region_size(sdram));
     soc_dma_port_add_mem(s->dma, memory_region_get_ram_ptr(&s->sram),
                          OMAP2_SRAM_BASE, s->sram_size);
 
diff --git a/include/hw/arm/omap.h b/include/hw/arm/omap.h
index 2fda996648..763d8eab4f 100644
--- a/include/hw/arm/omap.h
+++ b/include/hw/arm/omap.h
@@ -824,7 +824,6 @@ struct omap_mpu_state_s {
     MemoryRegion tap_iomem;
     MemoryRegion imif_ram;
     MemoryRegion emiff_ram;
-    MemoryRegion sdram;
     MemoryRegion sram;
 
     struct omap_dma_port_if_s {
@@ -938,8 +937,7 @@ struct omap_mpu_state_s *omap310_mpu_init(MemoryRegion *system_memory,
                 const char *core);
 
 /* omap2.c */
-struct omap_mpu_state_s *omap2420_mpu_init(MemoryRegion *sysmem,
-                unsigned long sdram_size,
+struct omap_mpu_state_s *omap2420_mpu_init(MemoryRegion *sdram,
                 const char *core);
 
 uint32_t omap_badwidth_read8(void *opaque, hwaddr addr);
-- 
2.21.0

[PATCH 05/21] hw/arm/omap1: Create the RAM in the board

[Philippe Mathieu-Daudé]

The SDRAM is incorrectly created in the OMAP310 SoC.
Move its creation in the board code, this will later allow the
board to have the QOM ownership of the RAM.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/arm/omap1.c        | 12 +++++-------
 hw/arm/omap_sx1.c     |  8 ++++++--
 hw/arm/palm.c         |  8 ++++++--
 include/hw/arm/omap.h |  6 ++----
 4 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/hw/arm/omap1.c b/hw/arm/omap1.c
index 0400593805..6ce038a453 100644
--- a/hw/arm/omap1.c
+++ b/hw/arm/omap1.c
@@ -23,6 +23,7 @@
 #include "qapi/error.h"
 #include "qemu-common.h"
 #include "cpu.h"
+#include "exec/address-spaces.h"
 #include "hw/boards.h"
 #include "hw/hw.h"
 #include "hw/irq.h"
@@ -3858,8 +3859,7 @@ static int omap_validate_tipb_mpui_addr(struct omap_mpu_state_s *s,
     return range_covers_byte(0xe1010000, 0xe1020004 - 0xe1010000, addr);
 }
 
-struct omap_mpu_state_s *omap310_mpu_init(MemoryRegion *system_memory,
-                unsigned long sdram_size,
+struct omap_mpu_state_s *omap310_mpu_init(MemoryRegion *dram,
                 const char *cpu_type)
 {
     int i;
@@ -3867,11 +3867,12 @@ struct omap_mpu_state_s *omap310_mpu_init(MemoryRegion *system_memory,
     qemu_irq dma_irqs[6];
     DriveInfo *dinfo;
     SysBusDevice *busdev;
+    MemoryRegion *system_memory = get_system_memory();
 
     /* Core */
     s->mpu_model = omap310;
     s->cpu = ARM_CPU(cpu_create(cpu_type));
-    s->sdram_size = sdram_size;
+    s->sdram_size = memory_region_size(dram);
     s->sram_size = OMAP15XX_SRAM_SIZE;
 
     s->wakeup = qemu_allocate_irq(omap_mpu_wakeup, s, 0);
@@ -3880,9 +3881,6 @@ struct omap_mpu_state_s *omap310_mpu_init(MemoryRegion *system_memory,
     omap_clk_init(s);
 
     /* Memory-mapped stuff */
-    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_fatal);
     memory_region_add_subregion(system_memory, OMAP_IMIF_BASE, &s->imif_ram);
@@ -3925,7 +3923,7 @@ struct omap_mpu_state_s *omap310_mpu_init(MemoryRegion *system_memory,
     s->port[tipb_mpui].addr_valid = omap_validate_tipb_mpui_addr;
 
     /* Register SDRAM and SRAM DMA ports for fast transfers.  */
-    soc_dma_port_add_mem(s->dma, memory_region_get_ram_ptr(&s->emiff_ram),
+    soc_dma_port_add_mem(s->dma, memory_region_get_ram_ptr(dram),
                          OMAP_EMIFF_BASE, s->sdram_size);
     soc_dma_port_add_mem(s->dma, memory_region_get_ram_ptr(&s->imif_ram),
                          OMAP_IMIF_BASE, s->sram_size);
diff --git a/hw/arm/omap_sx1.c b/hw/arm/omap_sx1.c
index c071197be7..be245714db 100644
--- a/hw/arm/omap_sx1.c
+++ b/hw/arm/omap_sx1.c
@@ -103,6 +103,7 @@ static void sx1_init(MachineState *machine, const int version)
 {
     struct omap_mpu_state_s *mpu;
     MemoryRegion *address_space = get_system_memory();
+    MemoryRegion *dram = g_new(MemoryRegion, 1);
     MemoryRegion *flash = g_new(MemoryRegion, 1);
     MemoryRegion *cs = g_new(MemoryRegion, 4);
     static uint32_t cs0val = 0x00213090;
@@ -118,8 +119,11 @@ static void sx1_init(MachineState *machine, const int version)
         flash_size = flash2_size;
     }
 
-    mpu = omap310_mpu_init(address_space, sx1_binfo.ram_size,
-                           machine->cpu_type);
+    memory_region_allocate_system_memory(dram, NULL, "omap1.dram",
+                                         sx1_binfo.ram_size);
+    memory_region_add_subregion(address_space, OMAP_EMIFF_BASE, dram);
+
+    mpu = omap310_mpu_init(dram, machine->cpu_type);
 
     /* External Flash (EMIFS) */
     memory_region_init_ram(flash, NULL, "omap_sx1.flash0-0", flash_size,
diff --git a/hw/arm/palm.c b/hw/arm/palm.c
index 02a3a82b9b..72eca8cc55 100644
--- a/hw/arm/palm.c
+++ b/hw/arm/palm.c
@@ -190,16 +190,20 @@ static void palmte_init(MachineState *machine)
     MemoryRegion *address_space_mem = get_system_memory();
     struct omap_mpu_state_s *mpu;
     int flash_size = 0x00800000;
-    int sdram_size = palmte_binfo.ram_size;
     static uint32_t cs0val = 0xffffffff;
     static uint32_t cs1val = 0x0000e1a0;
     static uint32_t cs2val = 0x0000e1a0;
     static uint32_t cs3val = 0xe1a0e1a0;
     int rom_size, rom_loaded = 0;
+    MemoryRegion *dram = g_new(MemoryRegion, 1);
     MemoryRegion *flash = g_new(MemoryRegion, 1);
     MemoryRegion *cs = g_new(MemoryRegion, 4);
 
-    mpu = omap310_mpu_init(address_space_mem, sdram_size, machine->cpu_type);
+    memory_region_allocate_system_memory(dram, NULL, "omap1.dram",
+                                         palmte_binfo.ram_size);
+    memory_region_add_subregion(address_space_mem, OMAP_EMIFF_BASE, dram);
+
+    mpu = omap310_mpu_init(dram, machine->cpu_type);
 
     /* External Flash (EMIFS) */
     memory_region_init_ram(flash, NULL, "palmte.flash", flash_size,
diff --git a/include/hw/arm/omap.h b/include/hw/arm/omap.h
index 763d8eab4f..f3aa670036 100644
--- a/include/hw/arm/omap.h
+++ b/include/hw/arm/omap.h
@@ -823,7 +823,6 @@ struct omap_mpu_state_s {
     MemoryRegion mpui_io_iomem;
     MemoryRegion tap_iomem;
     MemoryRegion imif_ram;
-    MemoryRegion emiff_ram;
     MemoryRegion sram;
 
     struct omap_dma_port_if_s {
@@ -835,7 +834,7 @@ struct omap_mpu_state_s {
                         hwaddr addr);
     } port[__omap_dma_port_last];
 
-    unsigned long sdram_size;
+    uint64_t sdram_size;
     unsigned long sram_size;
 
     /* MPUI-TIPB peripherals */
@@ -932,8 +931,7 @@ struct omap_mpu_state_s {
 };
 
 /* omap1.c */
-struct omap_mpu_state_s *omap310_mpu_init(MemoryRegion *system_memory,
-                unsigned long sdram_size,
+struct omap_mpu_state_s *omap310_mpu_init(MemoryRegion *sdram,
                 const char *core);
 
 /* omap2.c */
-- 
2.21.0

[PATCH 06/21] hw/arm/digic4: Inline digic4_board_setup_ram() function

[Philippe Mathieu-Daudé]

Having the RAM creation code in a separate function is not
very helpful. Move this code directly inside the board_init()
function, this will later allow the board to have the QOM
ownership of the RAM.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/arm/digic_boards.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/hw/arm/digic_boards.c b/hw/arm/digic_boards.c
index 304e4d1a29..ef3fc2b6a5 100644
--- a/hw/arm/digic_boards.c
+++ b/hw/arm/digic_boards.c
@@ -53,12 +53,6 @@ typedef struct DigicBoard {
     const char *rom1_def_filename;
 } DigicBoard;
 
-static void digic4_board_setup_ram(DigicBoardState *s, hwaddr ram_size)
-{
-    memory_region_allocate_system_memory(&s->ram, NULL, "ram", ram_size);
-    memory_region_add_subregion(get_system_memory(), 0, &s->ram);
-}
-
 static void digic4_board_init(DigicBoard *board)
 {
     Error *err = NULL;
@@ -72,7 +66,8 @@ static void digic4_board_init(DigicBoard *board)
         exit(1);
     }
 
-    digic4_board_setup_ram(s, board->ram_size);
+    memory_region_allocate_system_memory(&s->ram, NULL, "ram", board->ram_size);
+    memory_region_add_subregion(get_system_memory(), 0, &s->ram);
 
     if (board->add_rom0) {
         board->add_rom0(s, DIGIC4_ROM0_BASE, board->rom0_def_filename);
-- 
2.21.0

[PATCH 07/21] hw: Drop QOM ownership on memory_region_allocate_system_memory() calls

[Philippe Mathieu-Daudé]

All the memory_region_allocate_system_memory() calls are in the
board_init() code.  From the 58 calls in the repository, only
4 set the 'owner' parameter. It is obvious we want the Machine
to be the owner of the RAM, so we want to use OBJECT(machine)
as owner. We can simplify a bit by passing MachineState to
memory_region_allocate_system_memory(). In preparation of this
refactor, first drop the 'owner' argument from these 4 uses.

  $ git grep memory_region_allocate_system_memory hw/ | wc -l
  58

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/alpha/typhoon.c | 2 +-
 hw/arm/raspi.c     | 2 +-
 hw/hppa/machine.c  | 2 +-
 hw/sparc/sun4m.c   | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index 179e1f7658..8489ec335c 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -851,7 +851,7 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
 
     /* Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
        but the address space hole reserved at this point is 8TB.  */
-    memory_region_allocate_system_memory(&s->ram_region, OBJECT(s), "ram",
+    memory_region_allocate_system_memory(&s->ram_region, NULL, "ram",
                                          ram_size);
     memory_region_add_subregion(addr_space, 0, &s->ram_region);
 
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index 615d755879..f76b6eaad3 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -182,7 +182,7 @@ static void raspi_init(MachineState *machine, int version)
                             &error_abort, NULL);
 
     /* Allocate and map RAM */
-    memory_region_allocate_system_memory(&s->ram, OBJECT(machine), "ram",
+    memory_region_allocate_system_memory(&s->ram, NULL, "ram",
                                          machine->ram_size);
     /* FIXME: Remove when we have custom CPU address space support */
     memory_region_add_subregion_overlap(get_system_memory(), 0, &s->ram, 0);
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index 953d454f48..dbe1ff0fe5 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -96,7 +96,7 @@ static void machine_hppa_init(MachineState *machine)
 
     /* Main memory region. */
     ram_region = g_new(MemoryRegion, 1);
-    memory_region_allocate_system_memory(ram_region, OBJECT(machine),
+    memory_region_allocate_system_memory(ram_region, NULL,
                                          "ram", ram_size);
     memory_region_add_subregion(addr_space, 0, ram_region);
 
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 6c5a17a020..0bb7524655 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -788,7 +788,7 @@ static void ram_realize(DeviceState *dev, Error **errp)
     RamDevice *d = SUN4M_RAM(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
 
-    memory_region_allocate_system_memory(&d->ram, OBJECT(d), "sun4m.ram",
+    memory_region_allocate_system_memory(&d->ram, NULL, "sun4m.ram",
                                          d->size);
     sysbus_init_mmio(sbd, &d->ram);
 }
-- 
2.21.0

[PATCH 08/21] hw/alpha/dp264: Create the RAM in the board

[Philippe Mathieu-Daudé]

The SDRAM is incorrectly created in the Tyohoon northbridge ASIC.
Move its creation in the board code, this will later allow the
board to have the QOM ownership of the RAM.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
checkpatch complains:

 ERROR: spaces required around that '*' (ctx:WxV)
 #10: FILE: hw/alpha/alpha_sys.h:13:
 +PCIBus *typhoon_init(ISABus **, qemu_irq *, AlphaCPU *[4],
                                                       ^
---
 hw/alpha/alpha_sys.h |  2 +-
 hw/alpha/dp264.c     | 11 ++++++++++-
 hw/alpha/typhoon.c   |  9 +--------
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/hw/alpha/alpha_sys.h b/hw/alpha/alpha_sys.h
index 4e127a6de8..17f97e1bfe 100644
--- a/hw/alpha/alpha_sys.h
+++ b/hw/alpha/alpha_sys.h
@@ -10,7 +10,7 @@
 #include "hw/i386/pc.h"
 
 
-PCIBus *typhoon_init(ram_addr_t, ISABus **, qemu_irq *, AlphaCPU *[4],
+PCIBus *typhoon_init(ISABus **, qemu_irq *, AlphaCPU *[4],
                      pci_map_irq_fn);
 
 /* alpha_pci.c.  */
diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index 51feee8558..ddc249261c 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -62,6 +62,7 @@ static void clipper_init(MachineState *machine)
     uint64_t palcode_entry, palcode_low, palcode_high;
     uint64_t kernel_entry, kernel_low, kernel_high;
     unsigned int smp_cpus = machine->smp.cpus;
+    MemoryRegion ram_region;
 
     /* Create up to 4 cpus.  */
     memset(cpus, 0, sizeof(cpus));
@@ -73,8 +74,16 @@ static void clipper_init(MachineState *machine)
     cpus[0]->env.trap_arg1 = 0;
     cpus[0]->env.trap_arg2 = smp_cpus;
 
+    /*
+     * Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
+     * but the address space hole reserved at this point is 8TB.
+     */
+    memory_region_allocate_system_memory(&ram_region, NULL, "ram",
+                                         ram_size);
+    memory_region_add_subregion(get_system_memory(), 0, &ram_region);
+
     /* Init the chipset.  */
-    pci_bus = typhoon_init(ram_size, &isa_bus, &rtc_irq, cpus,
+    pci_bus = typhoon_init(&isa_bus, &rtc_irq, cpus,
                            clipper_pci_map_irq);
 
     /* Since we have an SRM-compatible PALcode, use the SRM epoch.  */
diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index 8489ec335c..12589ef331 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -58,7 +58,6 @@ typedef struct TyphoonState {
     TyphoonCchip cchip;
     TyphoonPchip pchip;
     MemoryRegion dchip_region;
-    MemoryRegion ram_region;
 } TyphoonState;
 
 /* Called when one of DRIR or DIM changes.  */
@@ -817,7 +816,7 @@ static void typhoon_alarm_timer(void *opaque)
     cpu_interrupt(CPU(s->cchip.cpu[cpu]), CPU_INTERRUPT_TIMER);
 }
 
-PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
+PCIBus *typhoon_init(ISABus **isa_bus,
                      qemu_irq *p_rtc_irq,
                      AlphaCPU *cpus[4], pci_map_irq_fn sys_map_irq)
 {
@@ -849,12 +848,6 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
 
     *p_rtc_irq = qemu_allocate_irq(typhoon_set_timer_irq, s, 0);
 
-    /* Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
-       but the address space hole reserved at this point is 8TB.  */
-    memory_region_allocate_system_memory(&s->ram_region, NULL, "ram",
-                                         ram_size);
-    memory_region_add_subregion(addr_space, 0, &s->ram_region);
-
     /* TIGbus, 0x801.0000.0000, 1GB.  */
     /* ??? The TIGbus is used for delivering interrupts, and access to
        the flash ROM.  I'm not sure that we need to implement it at all.  */
-- 
2.21.0

[PATCH 09/21] hw: Let memory_region_allocate_system_memory take MachineState argument

[Philippe Mathieu-Daudé]

All the codebase calls memory_region_allocate_system_memory() with
a NULL 'owner' from the board_init() function.
Let pass a MachineState argument, and enforce the QOM ownership of
the system memory.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/core/numa.c      | 11 +++++++----
 include/hw/boards.h |  6 ++++--
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/hw/core/numa.c b/hw/core/numa.c
index 038c96d4ab..2e29e4bfe0 100644
--- a/hw/core/numa.c
+++ b/hw/core/numa.c
@@ -520,21 +520,24 @@ static void allocate_system_memory_nonnuma(MemoryRegion *mr, Object *owner,
     vmstate_register_ram_global(mr);
 }
 
-void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
+void memory_region_allocate_system_memory(MemoryRegion *mr, MachineState *ms,
                                           const char *name,
                                           uint64_t ram_size)
 {
     uint64_t addr = 0;
     int i;
-    MachineState *ms = MACHINE(qdev_get_machine());
+
+    if (!ms) {
+        ms = MACHINE(qdev_get_machine());
+    }
 
     if (ms->numa_state == NULL ||
         ms->numa_state->num_nodes == 0 || !have_memdevs) {
-        allocate_system_memory_nonnuma(mr, owner, name, ram_size);
+        allocate_system_memory_nonnuma(mr, OBJECT(ms), name, ram_size);
         return;
     }
 
-    memory_region_init(mr, owner, name, ram_size);
+    memory_region_init(mr, OBJECT(ms), name, ram_size);
     for (i = 0; i < ms->numa_state->num_nodes; i++) {
         uint64_t size = ms->numa_state->nodes[i].node_mem;
         HostMemoryBackend *backend = ms->numa_state->nodes[i].node_memdev;
diff --git a/include/hw/boards.h b/include/hw/boards.h
index de45087f34..3b6cb82b6c 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -14,7 +14,7 @@
 /**
  * memory_region_allocate_system_memory - Allocate a board's main memory
  * @mr: the #MemoryRegion to be initialized
- * @owner: the object that tracks the region's reference count
+ * @ms: the #MachineState object that own the system memory
  * @name: name of the memory region
  * @ram_size: size of the region in bytes
  *
@@ -38,8 +38,10 @@
  * Smaller pieces of memory (display RAM, static RAMs, etc) don't need
  * to be backed via the -mem-path memory backend and can simply
  * be created via memory_region_init_ram().
+ *
+ * The #MachineState object will track the region's reference count.
  */
-void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
+void memory_region_allocate_system_memory(MemoryRegion *mr, MachineState *ms,
                                           const char *name,
                                           uint64_t ram_size);
 
-- 
2.21.0

[PATCH 10/21] hw/core: Let the machine be the owner of the system memory

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/core/null-machine.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/core/null-machine.c b/hw/core/null-machine.c
index 1aa0a9a01a..16546c8140 100644
--- a/hw/core/null-machine.c
+++ b/hw/core/null-machine.c
@@ -35,7 +35,7 @@ static void machine_none_init(MachineState *mch)
     if (mch->ram_size) {
         MemoryRegion *ram = g_new(MemoryRegion, 1);
 
-        memory_region_allocate_system_memory(ram, NULL, "ram", mch->ram_size);
+        memory_region_allocate_system_memory(ram, mch, "ram", mch->ram_size);
         memory_region_add_subregion(get_system_memory(), 0, ram);
     }
 
-- 
2.21.0

[PATCH 11/21] hw/alpha: Let the machine be the owner of the system memory

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/alpha/dp264.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index ddc249261c..5696b1e79b 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -78,7 +78,7 @@ static void clipper_init(MachineState *machine)
      * Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
      * but the address space hole reserved at this point is 8TB.
      */
-    memory_region_allocate_system_memory(&ram_region, NULL, "ram",
+    memory_region_allocate_system_memory(&ram_region, machine, "ram",
                                          ram_size);
     memory_region_add_subregion(get_system_memory(), 0, &ram_region);
 
-- 
2.21.0

[PATCH 12/21] hw/arm: Let the machine be the owner of the system memory

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/arm/aspeed.c           | 2 +-
 hw/arm/collie.c           | 2 +-
 hw/arm/cubieboard.c       | 2 +-
 hw/arm/digic_boards.c     | 7 ++++---
 hw/arm/highbank.c         | 3 ++-
 hw/arm/imx25_pdk.c        | 2 +-
 hw/arm/integratorcp.c     | 2 +-
 hw/arm/kzm.c              | 2 +-
 hw/arm/mcimx6ul-evk.c     | 2 +-
 hw/arm/mcimx7d-sabre.c    | 2 +-
 hw/arm/mps2-tz.c          | 4 ++--
 hw/arm/mps2.c             | 4 ++--
 hw/arm/musicpal.c         | 2 +-
 hw/arm/nseries.c          | 2 +-
 hw/arm/omap_sx1.c         | 2 +-
 hw/arm/palm.c             | 2 +-
 hw/arm/raspi.c            | 2 +-
 hw/arm/sabrelite.c        | 2 +-
 hw/arm/sbsa-ref.c         | 2 +-
 hw/arm/versatilepb.c      | 2 +-
 hw/arm/vexpress.c         | 4 ++--
 hw/arm/virt.c             | 2 +-
 hw/arm/xilinx_zynq.c      | 2 +-
 hw/arm/xlnx-versal-virt.c | 2 +-
 hw/arm/xlnx-zcu102.c      | 2 +-
 25 files changed, 32 insertions(+), 30 deletions(-)

diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index 52993f84b4..f2a52e1ade 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -212,7 +212,7 @@ static void aspeed_board_init(MachineState *machine,
     ram_size = object_property_get_uint(OBJECT(&bmc->soc), "ram-size",
                                         &error_abort);
 
-    memory_region_allocate_system_memory(&bmc->ram, NULL, "ram", ram_size);
+    memory_region_allocate_system_memory(&bmc->ram, machine, "ram", ram_size);
     memory_region_add_subregion(&bmc->ram_container, 0, &bmc->ram);
     memory_region_add_subregion(get_system_memory(),
                                 sc->memmap[ASPEED_SDRAM],
diff --git a/hw/arm/collie.c b/hw/arm/collie.c
index 970a4405cc..632531a88d 100644
--- a/hw/arm/collie.c
+++ b/hw/arm/collie.c
@@ -31,7 +31,7 @@ static void collie_init(MachineState *machine)
 
     s = sa1110_init(machine->cpu_type);
 
-    memory_region_allocate_system_memory(sdram, NULL, "strongarm.sdram",
+    memory_region_allocate_system_memory(sdram, machine, "strongarm.sdram",
                                          collie_binfo.ram_size);
     memory_region_add_subregion(get_system_memory(), SA_SDCS0, sdram);
 
diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
index 6dc2f1d6b6..b76ad7ef69 100644
--- a/hw/arm/cubieboard.c
+++ b/hw/arm/cubieboard.c
@@ -65,7 +65,7 @@ static void cubieboard_init(MachineState *machine)
         exit(1);
     }
 
-    memory_region_allocate_system_memory(&s->sdram, NULL, "cubieboard.ram",
+    memory_region_allocate_system_memory(&s->sdram, machine, "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 ef3fc2b6a5..f3d1febe87 100644
--- a/hw/arm/digic_boards.c
+++ b/hw/arm/digic_boards.c
@@ -53,7 +53,7 @@ typedef struct DigicBoard {
     const char *rom1_def_filename;
 } DigicBoard;
 
-static void digic4_board_init(DigicBoard *board)
+static void digic4_board_init(MachineState *machine, DigicBoard *board)
 {
     Error *err = NULL;
 
@@ -66,7 +66,8 @@ static void digic4_board_init(DigicBoard *board)
         exit(1);
     }
 
-    memory_region_allocate_system_memory(&s->ram, NULL, "ram", board->ram_size);
+    memory_region_allocate_system_memory(&s->ram, machine, "ram",
+                                         board->ram_size);
     memory_region_add_subregion(get_system_memory(), 0, &s->ram);
 
     if (board->add_rom0) {
@@ -142,7 +143,7 @@ static DigicBoard digic4_board_canon_a1100 = {
 
 static void canon_a1100_init(MachineState *machine)
 {
-    digic4_board_init(&digic4_board_canon_a1100);
+    digic4_board_init(machine, &digic4_board_canon_a1100);
 }
 
 static void canon_a1100_machine_init(MachineClass *mc)
diff --git a/hw/arm/highbank.c b/hw/arm/highbank.c
index f1724d6929..57e549ec00 100644
--- a/hw/arm/highbank.c
+++ b/hw/arm/highbank.c
@@ -290,7 +290,8 @@ static void calxeda_init(MachineState *machine, enum cxmachines machine_id)
 
     sysmem = get_system_memory();
     dram = g_new(MemoryRegion, 1);
-    memory_region_allocate_system_memory(dram, NULL, "highbank.dram", ram_size);
+    memory_region_allocate_system_memory(dram, machine, "highbank.dram",
+                                         ram_size);
     /* SDRAM at address zero.  */
     memory_region_add_subregion(sysmem, 0, dram);
 
diff --git a/hw/arm/imx25_pdk.c b/hw/arm/imx25_pdk.c
index c76fc2bd94..e88d325e10 100644
--- a/hw/arm/imx25_pdk.c
+++ b/hw/arm/imx25_pdk.c
@@ -84,7 +84,7 @@ static void imx25_pdk_init(MachineState *machine)
         machine->ram_size = FSL_IMX25_SDRAM0_SIZE + FSL_IMX25_SDRAM1_SIZE;
     }
 
-    memory_region_allocate_system_memory(&s->ram, NULL, "imx25.ram",
+    memory_region_allocate_system_memory(&s->ram, machine, "imx25.ram",
                                          machine->ram_size);
     memory_region_add_subregion(get_system_memory(), FSL_IMX25_SDRAM0_ADDR,
                                 &s->ram);
diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index 524970840d..a8053be396 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -605,7 +605,7 @@ static void integratorcp_init(MachineState *machine)
 
     cpu = ARM_CPU(cpuobj);
 
-    memory_region_allocate_system_memory(ram, NULL, "integrator.ram",
+    memory_region_allocate_system_memory(ram, machine, "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.  */
diff --git a/hw/arm/kzm.c b/hw/arm/kzm.c
index 1d5ef289d5..e81bd8e0dc 100644
--- a/hw/arm/kzm.c
+++ b/hw/arm/kzm.c
@@ -84,7 +84,7 @@ static void kzm_init(MachineState *machine)
         machine->ram_size = FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE;
     }
 
-    memory_region_allocate_system_memory(&s->ram, NULL, "kzm.ram",
+    memory_region_allocate_system_memory(&s->ram, machine, "kzm.ram",
                                          machine->ram_size);
     memory_region_add_subregion(get_system_memory(), FSL_IMX31_SDRAM0_ADDR,
                                 &s->ram);
diff --git a/hw/arm/mcimx6ul-evk.c b/hw/arm/mcimx6ul-evk.c
index e90b393a44..95653db599 100644
--- a/hw/arm/mcimx6ul-evk.c
+++ b/hw/arm/mcimx6ul-evk.c
@@ -48,7 +48,7 @@ static void mcimx6ul_evk_init(MachineState *machine)
 
     object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_fatal);
 
-    memory_region_allocate_system_memory(&s->ram, NULL, "mcimx6ul-evk.ram",
+    memory_region_allocate_system_memory(&s->ram, machine, "mcimx6ul-evk.ram",
                                          machine->ram_size);
     memory_region_add_subregion(get_system_memory(),
                                 FSL_IMX6UL_MMDC_ADDR, &s->ram);
diff --git a/hw/arm/mcimx7d-sabre.c b/hw/arm/mcimx7d-sabre.c
index 0d1f62d30a..d535a119f7 100644
--- a/hw/arm/mcimx7d-sabre.c
+++ b/hw/arm/mcimx7d-sabre.c
@@ -50,7 +50,7 @@ static void mcimx7d_sabre_init(MachineState *machine)
                             TYPE_FSL_IMX7, &error_fatal, NULL);
     object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_fatal);
 
-    memory_region_allocate_system_memory(&s->ram, NULL, "mcimx7d-sabre.ram",
+    memory_region_allocate_system_memory(&s->ram, machine, "mcimx7d-sabre.ram",
                                          machine->ram_size);
     memory_region_add_subregion(get_system_memory(),
                                 FSL_IMX7_MMDC_ADDR, &s->ram);
diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
index f8b620bcc6..96fe815361 100644
--- a/hw/arm/mps2-tz.c
+++ b/hw/arm/mps2-tz.c
@@ -458,8 +458,8 @@ static void mps2tz_common_init(MachineState *machine)
      * tradeoffs. For QEMU they're all just RAM, though. We arbitrarily
      * call the 16MB our "system memory", as it's the largest lump.
      */
-    memory_region_allocate_system_memory(&mms->psram,
-                                         NULL, "mps.ram", 16 * MiB);
+    memory_region_allocate_system_memory(&mms->psram, machine,
+                                         "mps.ram", 16 * MiB);
     memory_region_add_subregion(system_memory, 0x80000000, &mms->psram);
 
     /* The overflow IRQs for all UARTs are ORed together.
diff --git a/hw/arm/mps2.c b/hw/arm/mps2.c
index d002b126d3..4687bc7ed1 100644
--- a/hw/arm/mps2.c
+++ b/hw/arm/mps2.c
@@ -146,8 +146,8 @@ static void mps2_common_init(MachineState *machine)
      * This is of no use for QEMU so we don't implement it (as if
      * zbt_boot_ctrl is always zero).
      */
-    memory_region_allocate_system_memory(&mms->psram,
-                                         NULL, "mps.ram", 16 * MiB);
+    memory_region_allocate_system_memory(&mms->psram, machine,
+                                         "mps.ram", 16 * MiB);
     memory_region_add_subregion(system_memory, 0x21000000, &mms->psram);
 
     switch (mmc->fpga_type) {
diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index f68a399a98..2a2c7b3abf 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -1596,7 +1596,7 @@ static void musicpal_init(MachineState *machine)
     cpu = ARM_CPU(cpu_create(machine->cpu_type));
 
     /* For now we use a fixed - the original - RAM size */
-    memory_region_allocate_system_memory(ram, NULL, "musicpal.ram",
+    memory_region_allocate_system_memory(ram, machine, "musicpal.ram",
                                          MP_RAM_DEFAULT_SIZE);
     memory_region_add_subregion(address_space_mem, 0, ram);
 
diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
index 11f2c193f3..8ab30fde24 100644
--- a/hw/arm/nseries.c
+++ b/hw/arm/nseries.c
@@ -1315,7 +1315,7 @@ static void n8x0_init(MachineState *machine,
     struct n800_s *s = (struct n800_s *) g_malloc0(sizeof(*s));
     uint64_t sdram_size = binfo->ram_size;
 
-    memory_region_allocate_system_memory(sdram, NULL, "omap2.dram",
+    memory_region_allocate_system_memory(sdram, machine, "omap2.dram",
                                          sdram_size);
     memory_region_add_subregion(get_system_memory(), OMAP2_Q2_BASE, sdram);
 
diff --git a/hw/arm/omap_sx1.c b/hw/arm/omap_sx1.c
index be245714db..26f4e1834b 100644
--- a/hw/arm/omap_sx1.c
+++ b/hw/arm/omap_sx1.c
@@ -119,7 +119,7 @@ static void sx1_init(MachineState *machine, const int version)
         flash_size = flash2_size;
     }
 
-    memory_region_allocate_system_memory(dram, NULL, "omap1.dram",
+    memory_region_allocate_system_memory(dram, machine, "omap1.dram",
                                          sx1_binfo.ram_size);
     memory_region_add_subregion(address_space, OMAP_EMIFF_BASE, dram);
 
diff --git a/hw/arm/palm.c b/hw/arm/palm.c
index 72eca8cc55..dceb4ab6a8 100644
--- a/hw/arm/palm.c
+++ b/hw/arm/palm.c
@@ -199,7 +199,7 @@ static void palmte_init(MachineState *machine)
     MemoryRegion *flash = g_new(MemoryRegion, 1);
     MemoryRegion *cs = g_new(MemoryRegion, 4);
 
-    memory_region_allocate_system_memory(dram, NULL, "omap1.dram",
+    memory_region_allocate_system_memory(dram, machine, "omap1.dram",
                                          palmte_binfo.ram_size);
     memory_region_add_subregion(address_space_mem, OMAP_EMIFF_BASE, dram);
 
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index f76b6eaad3..6f1520bad6 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -182,7 +182,7 @@ static void raspi_init(MachineState *machine, int version)
                             &error_abort, NULL);
 
     /* Allocate and map RAM */
-    memory_region_allocate_system_memory(&s->ram, NULL, "ram",
+    memory_region_allocate_system_memory(&s->ram, machine, "ram",
                                          machine->ram_size);
     /* FIXME: Remove when we have custom CPU address space support */
     memory_region_add_subregion_overlap(get_system_memory(), 0, &s->ram, 0);
diff --git a/hw/arm/sabrelite.c b/hw/arm/sabrelite.c
index 96cc455c5c..dfeffcb2a7 100644
--- a/hw/arm/sabrelite.c
+++ b/hw/arm/sabrelite.c
@@ -64,7 +64,7 @@ static void sabrelite_init(MachineState *machine)
         exit(1);
     }
 
-    memory_region_allocate_system_memory(&s->ram, NULL, "sabrelite.ram",
+    memory_region_allocate_system_memory(&s->ram, machine, "sabrelite.ram",
                                          machine->ram_size);
     memory_region_add_subregion(get_system_memory(), FSL_IMX6_MMDC_ADDR,
                                 &s->ram);
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index 27046cc284..1d4db8d7fd 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -687,7 +687,7 @@ static void sbsa_ref_init(MachineState *machine)
         object_unref(cpuobj);
     }
 
-    memory_region_allocate_system_memory(ram, NULL, "sbsa-ref.ram",
+    memory_region_allocate_system_memory(ram, machine, "sbsa-ref.ram",
                                          machine->ram_size);
     memory_region_add_subregion(sysmem, sbsa_ref_memmap[SBSA_MEM].base, ram);
 
diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
index e86af01537..5cde8e3677 100644
--- a/hw/arm/versatilepb.c
+++ b/hw/arm/versatilepb.c
@@ -220,7 +220,7 @@ static void versatile_init(MachineState *machine, int board_id)
 
     cpu = ARM_CPU(cpuobj);
 
-    memory_region_allocate_system_memory(ram, NULL, "versatile.ram",
+    memory_region_allocate_system_memory(ram, machine, "versatile.ram",
                                          machine->ram_size);
     /* ??? RAM should repeat to fill physical memory space.  */
     /* SDRAM at address zero.  */
diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index 4673a88a8d..b97770cc68 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -283,7 +283,7 @@ static void a9_daughterboard_init(const VexpressMachineState *vms,
         exit(1);
     }
 
-    memory_region_allocate_system_memory(ram, NULL, "vexpress.highmem",
+    memory_region_allocate_system_memory(ram, machine, "vexpress.highmem",
                                          ram_size);
     low_ram_size = ram_size;
     if (low_ram_size > 0x4000000) {
@@ -375,7 +375,7 @@ static void a15_daughterboard_init(const VexpressMachineState *vms,
         }
     }
 
-    memory_region_allocate_system_memory(ram, NULL, "vexpress.highmem",
+    memory_region_allocate_system_memory(ram, machine, "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 d4bedc2607..917d5549c1 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1702,7 +1702,7 @@ static void machvirt_init(MachineState *machine)
         }
     }
 
-    memory_region_allocate_system_memory(ram, NULL, "mach-virt.ram",
+    memory_region_allocate_system_memory(ram, machine, "mach-virt.ram",
                                          machine->ram_size);
     memory_region_add_subregion(sysmem, vms->memmap[VIRT_MEM].base, ram);
     if (machine->device_memory) {
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 3a0fa5b23f..abb3162cfc 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -190,7 +190,7 @@ static void zynq_init(MachineState *machine)
     }
 
     /* DDR remapped to address zero.  */
-    memory_region_allocate_system_memory(ext_ram, NULL, "zynq.ext_ram",
+    memory_region_allocate_system_memory(ext_ram, machine, "zynq.ext_ram",
                                          ram_size);
     memory_region_add_subregion(address_space_mem, 0, ext_ram);
 
diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
index 462493c467..565a52bdab 100644
--- a/hw/arm/xlnx-versal-virt.c
+++ b/hw/arm/xlnx-versal-virt.c
@@ -414,7 +414,7 @@ static void versal_virt_init(MachineState *machine)
         psci_conduit = QEMU_PSCI_CONDUIT_SMC;
     }
 
-    memory_region_allocate_system_memory(&s->mr_ddr, NULL, "ddr",
+    memory_region_allocate_system_memory(&s->mr_ddr, machine, "ddr",
                                          machine->ram_size);
 
     sysbus_init_child_obj(OBJECT(machine), "xlnx-ve", &s->soc,
diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
index 53cfe7c1f1..0de41adb75 100644
--- a/hw/arm/xlnx-zcu102.c
+++ b/hw/arm/xlnx-zcu102.c
@@ -87,7 +87,7 @@ static void xlnx_zcu102_init(MachineState *machine)
                  ram_size);
     }
 
-    memory_region_allocate_system_memory(&s->ddr_ram, NULL, "ddr-ram",
+    memory_region_allocate_system_memory(&s->ddr_ram, machine, "ddr-ram",
                                          ram_size);
 
     object_initialize_child(OBJECT(machine), "soc", &s->soc, sizeof(s->soc),
-- 
2.21.0

[PATCH 13/21] hw/cris: Let the machine be the owner of the system memory

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/cris/axis_dev88.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/cris/axis_dev88.c b/hw/cris/axis_dev88.c
index 940c7dd122..faa7058733 100644
--- a/hw/cris/axis_dev88.c
+++ b/hw/cris/axis_dev88.c
@@ -270,7 +270,7 @@ void axisdev88_init(MachineState *machine)
     env = &cpu->env;
 
     /* allocate RAM */
-    memory_region_allocate_system_memory(phys_ram, NULL, "axisdev88.ram",
+    memory_region_allocate_system_memory(phys_ram, machine, "axisdev88.ram",
                                          ram_size);
     memory_region_add_subregion(address_space_mem, 0x40000000, phys_ram);
 
-- 
2.21.0

[PATCH 14/21] hw/hppa: Let the machine be the owner of the system memory

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/hppa/machine.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index dbe1ff0fe5..6703bfd351 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -96,7 +96,7 @@ static void machine_hppa_init(MachineState *machine)
 
     /* Main memory region. */
     ram_region = g_new(MemoryRegion, 1);
-    memory_region_allocate_system_memory(ram_region, NULL,
+    memory_region_allocate_system_memory(ram_region, machine,
                                          "ram", ram_size);
     memory_region_add_subregion(addr_space, 0, ram_region);
 
-- 
2.21.0

[PATCH 15/21] hw/i386: Let the machine be the owner of the system memory

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/i386/pc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 4b1904237e..3414dc423a 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1655,7 +1655,7 @@ void pc_memory_init(PCMachineState *pcms,
      * with older qemus that used qemu_ram_alloc().
      */
     ram = g_malloc(sizeof(*ram));
-    memory_region_allocate_system_memory(ram, NULL, "pc.ram",
+    memory_region_allocate_system_memory(ram, machine, "pc.ram",
                                          machine->ram_size);
     *ram_memory = ram;
     ram_below_4g = g_malloc(sizeof(*ram_below_4g));
-- 
2.21.0

[PATCH 16/21] hw/lm32: Let the machine be the owner of the system memory

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/lm32/lm32_boards.c | 4 ++--
 hw/lm32/milkymist.c   | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/lm32/lm32_boards.c b/hw/lm32/lm32_boards.c
index 5ae308bfcf..50b607e0cc 100644
--- a/hw/lm32/lm32_boards.c
+++ b/hw/lm32/lm32_boards.c
@@ -107,7 +107,7 @@ static void lm32_evr_init(MachineState *machine)
 
     reset_info->flash_base = flash_base;
 
-    memory_region_allocate_system_memory(phys_ram, NULL, "lm32_evr.sdram",
+    memory_region_allocate_system_memory(phys_ram, machine, "lm32_evr.sdram",
                                          ram_size);
     memory_region_add_subregion(address_space_mem, ram_base, phys_ram);
 
@@ -200,7 +200,7 @@ static void lm32_uclinux_init(MachineState *machine)
 
     reset_info->flash_base = flash_base;
 
-    memory_region_allocate_system_memory(phys_ram, NULL,
+    memory_region_allocate_system_memory(phys_ram, machine,
                                          "lm32_uclinux.sdram", ram_size);
     memory_region_add_subregion(address_space_mem, ram_base, phys_ram);
 
diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c
index 460d322de5..ee2eb1877e 100644
--- a/hw/lm32/milkymist.c
+++ b/hw/lm32/milkymist.c
@@ -116,7 +116,7 @@ milkymist_init(MachineState *machine)
 
     cpu_lm32_set_phys_msb_ignore(env, 1);
 
-    memory_region_allocate_system_memory(phys_sdram, NULL, "milkymist.sdram",
+    memory_region_allocate_system_memory(phys_sdram, machine, "milkymist.sdram",
                                          sdram_size);
     memory_region_add_subregion(address_space_mem, sdram_base, phys_sdram);
 
-- 
2.21.0

[PATCH 17/21] hw/m68k: Let the machine be the owner of the system memory

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/m68k/an5206.c    | 2 +-
 hw/m68k/mcf5208.c   | 2 +-
 hw/m68k/next-cube.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/m68k/an5206.c b/hw/m68k/an5206.c
index 54ccbe1a82..fb045c2436 100644
--- a/hw/m68k/an5206.c
+++ b/hw/m68k/an5206.c
@@ -46,7 +46,7 @@ static void an5206_init(MachineState *machine)
     env->rambar0 = AN5206_RAMBAR_ADDR | 1;
 
     /* DRAM at address zero */
-    memory_region_allocate_system_memory(ram, NULL, "an5206.ram", ram_size);
+    memory_region_allocate_system_memory(ram, machine, "an5206.ram", ram_size);
     memory_region_add_subregion(address_space_mem, 0, ram);
 
     /* Internal SRAM.  */
diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
index 34d34eba17..8e8c8ef349 100644
--- a/hw/m68k/mcf5208.c
+++ b/hw/m68k/mcf5208.c
@@ -248,7 +248,7 @@ static void mcf5208evb_init(MachineState *machine)
     memory_region_add_subregion(address_space_mem, 0x00000000, rom);
 
     /* DRAM at 0x40000000 */
-    memory_region_allocate_system_memory(ram, NULL, "mcf5208.ram", ram_size);
+    memory_region_allocate_system_memory(ram, machine, "mcf5208.ram", ram_size);
     memory_region_add_subregion(address_space_mem, 0x40000000, ram);
 
     /* Internal SRAM.  */
diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c
index e5343348d0..6aed9376f3 100644
--- a/hw/m68k/next-cube.c
+++ b/hw/m68k/next-cube.c
@@ -893,7 +893,7 @@ static void next_cube_init(MachineState *machine)
     memcpy(ns->rtc.ram, rtc_ram2, 32);
 
     /* 64MB RAM starting at 0x04000000  */
-    memory_region_allocate_system_memory(ram, NULL, "next.ram", ram_size);
+    memory_region_allocate_system_memory(ram, machine, "next.ram", ram_size);
     memory_region_add_subregion(sysmem, 0x04000000, ram);
 
     /* Framebuffer */
-- 
2.21.0

[PATCH 18/21] hw/mips: Let the machine be the owner of the system memory

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/mips/boston.c        | 2 +-
 hw/mips/mips_fulong2e.c | 3 ++-
 hw/mips/mips_jazz.c     | 2 +-
 hw/mips/mips_malta.c    | 2 +-
 hw/mips/mips_mipssim.c  | 2 +-
 hw/mips/mips_r4k.c      | 3 ++-
 6 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index ca7d813a52..8445fee0f1 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -474,7 +474,7 @@ static void boston_mach_init(MachineState *machine)
     memory_region_add_subregion_overlap(sys_mem, 0x18000000, flash, 0);
 
     ddr = g_new(MemoryRegion, 1);
-    memory_region_allocate_system_memory(ddr, NULL, "boston.ddr",
+    memory_region_allocate_system_memory(ddr, machine, "boston.ddr",
                                          machine->ram_size);
     memory_region_add_subregion_overlap(sys_mem, 0x80000000, ddr, 0);
 
diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index cf537dd7e6..d5a5cef619 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -318,7 +318,8 @@ static void mips_fulong2e_init(MachineState *machine)
     ram_size = 256 * MiB;
 
     /* allocate RAM */
-    memory_region_allocate_system_memory(ram, NULL, "fulong2e.ram", ram_size);
+    memory_region_allocate_system_memory(ram, machine,
+                                         "fulong2e.ram", ram_size);
     memory_region_init_ram(bios, NULL, "fulong2e.bios", BIOS_SIZE,
                            &error_fatal);
     memory_region_set_readonly(bios, true);
diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index 8d010a0b6e..88b125855f 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -188,7 +188,7 @@ static void mips_jazz_init(MachineState *machine,
     cc->do_transaction_failed = mips_jazz_do_transaction_failed;
 
     /* allocate RAM */
-    memory_region_allocate_system_memory(ram, NULL, "mips_jazz.ram",
+    memory_region_allocate_system_memory(ram, machine, "mips_jazz.ram",
                                          machine->ram_size);
     memory_region_add_subregion(address_space, 0, ram);
 
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 4d9c64b36a..af56a29ccb 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1267,7 +1267,7 @@ void mips_malta_init(MachineState *machine)
     }
 
     /* register RAM at high address where it is undisturbed by IO */
-    memory_region_allocate_system_memory(ram_high, NULL, "mips_malta.ram",
+    memory_region_allocate_system_memory(ram_high, machine, "mips_malta.ram",
                                          ram_size);
     memory_region_add_subregion(system_memory, 0x80000000, ram_high);
 
diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
index 282bbecb24..c1933231e2 100644
--- a/hw/mips/mips_mipssim.c
+++ b/hw/mips/mips_mipssim.c
@@ -166,7 +166,7 @@ mips_mipssim_init(MachineState *machine)
     qemu_register_reset(main_cpu_reset, reset_info);
 
     /* Allocate RAM. */
-    memory_region_allocate_system_memory(ram, NULL, "mips_mipssim.ram",
+    memory_region_allocate_system_memory(ram, machine, "mips_mipssim.ram",
                                          ram_size);
     memory_region_init_ram(bios, NULL, "mips_mipssim.bios", BIOS_SIZE,
                            &error_fatal);
diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
index bc0be26544..59f8cacfb6 100644
--- a/hw/mips/mips_r4k.c
+++ b/hw/mips/mips_r4k.c
@@ -203,7 +203,8 @@ void mips_r4k_init(MachineState *machine)
                      " maximum 256MB", ram_size / MiB);
         exit(1);
     }
-    memory_region_allocate_system_memory(ram, NULL, "mips_r4k.ram", ram_size);
+    memory_region_allocate_system_memory(ram, machine, "mips_r4k.ram",
+                                         ram_size);
 
     memory_region_add_subregion(address_space_mem, 0, ram);
 
-- 
2.21.0

[PATCH 19/21] hw/ppc: Let the machine be the owner of the system memory

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/ppc/e500.c          | 3 ++-
 hw/ppc/mac_newworld.c  | 3 ++-
 hw/ppc/mac_oldworld.c  | 2 +-
 hw/ppc/pnv.c           | 2 +-
 hw/ppc/ppc405_boards.c | 6 +++---
 hw/ppc/prep.c          | 3 ++-
 hw/ppc/spapr.c         | 2 +-
 hw/ppc/virtex_ml507.c  | 2 +-
 8 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 91cd4c26f9..9c7be26248 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -912,7 +912,8 @@ void ppce500_init(MachineState *machine)
     machine->ram_size = ram_size;
 
     /* Register Memory */
-    memory_region_allocate_system_memory(ram, NULL, "mpc8544ds.ram", ram_size);
+    memory_region_allocate_system_memory(ram, machine, "mpc8544ds.ram",
+                                         ram_size);
     memory_region_add_subregion(address_space_mem, 0, ram);
 
     dev = qdev_create(NULL, "e500-ccsr");
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index c5bbcc7433..8409114eed 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -152,7 +152,8 @@ static void ppc_core99_init(MachineState *machine)
     }
 
     /* allocate RAM */
-    memory_region_allocate_system_memory(ram, NULL, "ppc_core99.ram", ram_size);
+    memory_region_allocate_system_memory(ram, machine, "ppc_core99.ram",
+                                         ram_size);
     memory_region_add_subregion(get_system_memory(), 0, ram);
 
     /* allocate and load BIOS */
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 0fa680b749..9dd645476a 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -127,7 +127,7 @@ static void ppc_heathrow_init(MachineState *machine)
         exit(1);
     }
 
-    memory_region_allocate_system_memory(ram, NULL, "ppc_heathrow.ram",
+    memory_region_allocate_system_memory(ram, machine, "ppc_heathrow.ram",
                                          ram_size);
     memory_region_add_subregion(sysmem, 0, ram);
 
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 7cf64b6d25..ac731c4f88 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -640,7 +640,7 @@ static void pnv_init(MachineState *machine)
     }
 
     ram = g_new(MemoryRegion, 1);
-    memory_region_allocate_system_memory(ram, NULL, "pnv.ram",
+    memory_region_allocate_system_memory(ram, machine, "pnv.ram",
                                          machine->ram_size);
     memory_region_add_subregion(get_system_memory(), 0, ram);
 
diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
index 388cae0b43..8a2d2f4511 100644
--- a/hw/ppc/ppc405_boards.c
+++ b/hw/ppc/ppc405_boards.c
@@ -162,8 +162,8 @@ static void ref405ep_init(MachineState *machine)
     MemoryRegion *sysmem = get_system_memory();
 
     /* XXX: fix this */
-    memory_region_allocate_system_memory(&ram_memories[0], NULL, "ef405ep.ram",
-                                         0x08000000);
+    memory_region_allocate_system_memory(&ram_memories[0], machine,
+                                         "ef405ep.ram", 128 * MiB);
     ram_bases[0] = 0;
     ram_sizes[0] = 0x08000000;
     memory_region_init(&ram_memories[1], NULL, "ef405ep.ram1", 0);
@@ -427,7 +427,7 @@ static void taihu_405ep_init(MachineState *machine)
 
     /* RAM is soldered to the board so the size cannot be changed */
     ram_size = 0x08000000;
-    memory_region_allocate_system_memory(ram, NULL, "taihu_405ep.ram",
+    memory_region_allocate_system_memory(ram, machine, "taihu_405ep.ram",
                                          ram_size);
 
     ram_bases[0] = 0;
diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 4f3c6bf190..54b00805e0 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -445,7 +445,8 @@ static void ppc_prep_init(MachineState *machine)
     }
 
     /* allocate RAM */
-    memory_region_allocate_system_memory(ram, NULL, "ppc_prep.ram", ram_size);
+    memory_region_allocate_system_memory(ram, machine, "ppc_prep.ram",
+                                         ram_size);
     memory_region_add_subregion(sysmem, 0, ram);
 
     if (linux_boot) {
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 4eb97d3a9b..1f168edd43 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2840,7 +2840,7 @@ static void spapr_machine_init(MachineState *machine)
     }
 
     /* allocate RAM */
-    memory_region_allocate_system_memory(ram, NULL, "ppc_spapr.ram",
+    memory_region_allocate_system_memory(ram, machine, "ppc_spapr.ram",
                                          machine->ram_size);
     memory_region_add_subregion(sysmem, 0, ram);
 
diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
index 68625522d8..86c9273dc0 100644
--- a/hw/ppc/virtex_ml507.c
+++ b/hw/ppc/virtex_ml507.c
@@ -224,7 +224,7 @@ static void virtex_init(MachineState *machine)
 
     qemu_register_reset(main_cpu_reset, cpu);
 
-    memory_region_allocate_system_memory(phys_ram, NULL, "ram", ram_size);
+    memory_region_allocate_system_memory(phys_ram, machine, "ram", ram_size);
     memory_region_add_subregion(address_space_mem, ram_base, phys_ram);
 
     dinfo = drive_get(IF_PFLASH, 0, 0);
-- 
2.21.0

[PATCH 20/21] hw/sparc: Let the machine be the owner of the system memory

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/sparc/leon3.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/sparc/leon3.c b/hw/sparc/leon3.c
index c5f1b1ee72..f58f2782c6 100644
--- a/hw/sparc/leon3.c
+++ b/hw/sparc/leon3.c
@@ -244,7 +244,7 @@ static void leon3_generic_hw_init(MachineState *machine)
         exit(1);
     }
 
-    memory_region_allocate_system_memory(ram, NULL, "leon3.ram", ram_size);
+    memory_region_allocate_system_memory(ram, machine, "leon3.ram", ram_size);
     memory_region_add_subregion(address_space_mem, LEON3_RAM_OFFSET, ram);
 
     /* Allocate BIOS */
-- 
2.21.0

[PATCH 21/21] hw/core: Assert memory_region_allocate_system_memory has machine owner

[Philippe Mathieu-Daudé]

All the memory_region_allocate_system_memory() pass a MachineState
argument. Add an assertion to ensure the new boards/machines added
set this argument, so all system memory object have the machine as
its QOM owner.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/core/numa.c      | 4 +---
 include/hw/boards.h | 2 +-
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/core/numa.c b/hw/core/numa.c
index 2e29e4bfe0..3e07e94d00 100644
--- a/hw/core/numa.c
+++ b/hw/core/numa.c
@@ -527,9 +527,7 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, MachineState *ms,
     uint64_t addr = 0;
     int i;
 
-    if (!ms) {
-        ms = MACHINE(qdev_get_machine());
-    }
+    g_assert(ms);
 
     if (ms->numa_state == NULL ||
         ms->numa_state->num_nodes == 0 || !have_memdevs) {
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 3b6cb82b6c..31ab6e7586 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -14,7 +14,7 @@
 /**
  * memory_region_allocate_system_memory - Allocate a board's main memory
  * @mr: the #MemoryRegion to be initialized
- * @ms: the #MachineState object that own the system memory
+ * @ms: the #MachineState object that own the system memory (must not be NULL)
  * @name: name of the memory region
  * @ram_size: size of the region in bytes
  *
-- 
2.21.0

Re: [PATCH 00/21] hw: Let the machine be the owner of the system memory

[no-reply]

Patchew URL: https://patchew.org/QEMU/20191020225650.3671-1-philmd@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH 00/21] hw: Let the machine be the owner of the system memory
Type: series
Message-id: 20191020225650.3671-1-philmd@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Switched to a new branch 'test'
23b4e9a hw/core: Assert memory_region_allocate_system_memory has machine owner
67caaf0 hw/sparc: Let the machine be the owner of the system memory
9610d69 hw/ppc: Let the machine be the owner of the system memory
3cf0c55 hw/mips: Let the machine be the owner of the system memory
c6cd486 hw/m68k: Let the machine be the owner of the system memory
dd8e9a6 hw/lm32: Let the machine be the owner of the system memory
c7b9ccd hw/i386: Let the machine be the owner of the system memory
655182f hw/hppa: Let the machine be the owner of the system memory
20dc622 hw/cris: Let the machine be the owner of the system memory
fc3c6d2 hw/arm: Let the machine be the owner of the system memory
e9f1ebc hw/alpha: Let the machine be the owner of the system memory
3ef111b hw/core: Let the machine be the owner of the system memory
31bd838 hw: Let memory_region_allocate_system_memory take MachineState argument
dae9d11 hw/alpha/dp264: Create the RAM in the board
c31eba0 hw: Drop QOM ownership on memory_region_allocate_system_memory() calls
728aa65 hw/arm/digic4: Inline digic4_board_setup_ram() function
c2a9052 hw/arm/omap1: Create the RAM in the board
4f4977f hw/arm/omap2: Create the RAM in the board
d1959b2 hw/arm/collie: Create the RAM in the board
815f5c1 hw/arm/mps2: Use the IEC binary prefix definitions
103190f hw/arm/xilinx_zynq: Use the IEC binary prefix definitions

=== OUTPUT BEGIN ===
1/21 Checking commit 103190fd9b28 (hw/arm/xilinx_zynq: Use the IEC binary prefix definitions)
2/21 Checking commit 815f5c166924 (hw/arm/mps2: Use the IEC binary prefix definitions)
3/21 Checking commit d1959b2d3e49 (hw/arm/collie: Create the RAM in the board)
4/21 Checking commit 4f4977fa8205 (hw/arm/omap2: Create the RAM in the board)
5/21 Checking commit c2a90524bd33 (hw/arm/omap1: Create the RAM in the board)
6/21 Checking commit 728aa65672f4 (hw/arm/digic4: Inline digic4_board_setup_ram() function)
7/21 Checking commit c31eba0d025b (hw: Drop QOM ownership on memory_region_allocate_system_memory() calls)
8/21 Checking commit dae9d111acb1 (hw/alpha/dp264: Create the RAM in the board)
ERROR: spaces required around that '*' (ctx:WxV)
#24: FILE: hw/alpha/alpha_sys.h:13:
+PCIBus *typhoon_init(ISABus **, qemu_irq *, AlphaCPU *[4],
                                                      ^

total: 1 errors, 0 warnings, 59 lines checked

Patch 8/21 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

9/21 Checking commit 31bd838d2d79 (hw: Let memory_region_allocate_system_memory take MachineState argument)
10/21 Checking commit 3ef111b6a62a (hw/core: Let the machine be the owner of the system memory)
11/21 Checking commit e9f1ebcb1624 (hw/alpha: Let the machine be the owner of the system memory)
12/21 Checking commit fc3c6d23b63f (hw/arm: Let the machine be the owner of the system memory)
13/21 Checking commit 20dc622c1df3 (hw/cris: Let the machine be the owner of the system memory)
14/21 Checking commit 655182f6e74a (hw/hppa: Let the machine be the owner of the system memory)
15/21 Checking commit c7b9ccda7442 (hw/i386: Let the machine be the owner of the system memory)
16/21 Checking commit dd8e9a66c1f0 (hw/lm32: Let the machine be the owner of the system memory)
17/21 Checking commit c6cd4866d6a0 (hw/m68k: Let the machine be the owner of the system memory)
18/21 Checking commit 3cf0c55fcc1b (hw/mips: Let the machine be the owner of the system memory)
19/21 Checking commit 9610d6908cbc (hw/ppc: Let the machine be the owner of the system memory)
20/21 Checking commit 67caaf05019b (hw/sparc: Let the machine be the owner of the system memory)
21/21 Checking commit 23b4e9a52318 (hw/core: Assert memory_region_allocate_system_memory has machine owner)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20191020225650.3671-1-philmd@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [PATCH 19/21] hw/ppc: Let the machine be the owner of the system memory

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 5817 bytes --]

On Mon, Oct 21, 2019 at 12:56:48AM +0200, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/ppc/e500.c          | 3 ++-
>  hw/ppc/mac_newworld.c  | 3 ++-
>  hw/ppc/mac_oldworld.c  | 2 +-
>  hw/ppc/pnv.c           | 2 +-
>  hw/ppc/ppc405_boards.c | 6 +++---
>  hw/ppc/prep.c          | 3 ++-
>  hw/ppc/spapr.c         | 2 +-
>  hw/ppc/virtex_ml507.c  | 2 +-
>  8 files changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index 91cd4c26f9..9c7be26248 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -912,7 +912,8 @@ void ppce500_init(MachineState *machine)
>      machine->ram_size = ram_size;
>  
>      /* Register Memory */
> -    memory_region_allocate_system_memory(ram, NULL, "mpc8544ds.ram", ram_size);
> +    memory_region_allocate_system_memory(ram, machine, "mpc8544ds.ram",
> +                                         ram_size);
>      memory_region_add_subregion(address_space_mem, 0, ram);
>  
>      dev = qdev_create(NULL, "e500-ccsr");
> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
> index c5bbcc7433..8409114eed 100644
> --- a/hw/ppc/mac_newworld.c
> +++ b/hw/ppc/mac_newworld.c
> @@ -152,7 +152,8 @@ static void ppc_core99_init(MachineState *machine)
>      }
>  
>      /* allocate RAM */
> -    memory_region_allocate_system_memory(ram, NULL, "ppc_core99.ram", ram_size);
> +    memory_region_allocate_system_memory(ram, machine, "ppc_core99.ram",
> +                                         ram_size);
>      memory_region_add_subregion(get_system_memory(), 0, ram);
>  
>      /* allocate and load BIOS */
> diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
> index 0fa680b749..9dd645476a 100644
> --- a/hw/ppc/mac_oldworld.c
> +++ b/hw/ppc/mac_oldworld.c
> @@ -127,7 +127,7 @@ static void ppc_heathrow_init(MachineState *machine)
>          exit(1);
>      }
>  
> -    memory_region_allocate_system_memory(ram, NULL, "ppc_heathrow.ram",
> +    memory_region_allocate_system_memory(ram, machine, "ppc_heathrow.ram",
>                                           ram_size);
>      memory_region_add_subregion(sysmem, 0, ram);
>  
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 7cf64b6d25..ac731c4f88 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -640,7 +640,7 @@ static void pnv_init(MachineState *machine)
>      }
>  
>      ram = g_new(MemoryRegion, 1);
> -    memory_region_allocate_system_memory(ram, NULL, "pnv.ram",
> +    memory_region_allocate_system_memory(ram, machine, "pnv.ram",
>                                           machine->ram_size);
>      memory_region_add_subregion(get_system_memory(), 0, ram);
>  
> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
> index 388cae0b43..8a2d2f4511 100644
> --- a/hw/ppc/ppc405_boards.c
> +++ b/hw/ppc/ppc405_boards.c
> @@ -162,8 +162,8 @@ static void ref405ep_init(MachineState *machine)
>      MemoryRegion *sysmem = get_system_memory();
>  
>      /* XXX: fix this */
> -    memory_region_allocate_system_memory(&ram_memories[0], NULL, "ef405ep.ram",
> -                                         0x08000000);
> +    memory_region_allocate_system_memory(&ram_memories[0], machine,
> +                                         "ef405ep.ram", 128 * MiB);
>      ram_bases[0] = 0;
>      ram_sizes[0] = 0x08000000;
>      memory_region_init(&ram_memories[1], NULL, "ef405ep.ram1", 0);
> @@ -427,7 +427,7 @@ static void taihu_405ep_init(MachineState *machine)
>  
>      /* RAM is soldered to the board so the size cannot be changed */
>      ram_size = 0x08000000;
> -    memory_region_allocate_system_memory(ram, NULL, "taihu_405ep.ram",
> +    memory_region_allocate_system_memory(ram, machine, "taihu_405ep.ram",
>                                           ram_size);
>  
>      ram_bases[0] = 0;
> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
> index 4f3c6bf190..54b00805e0 100644
> --- a/hw/ppc/prep.c
> +++ b/hw/ppc/prep.c
> @@ -445,7 +445,8 @@ static void ppc_prep_init(MachineState *machine)
>      }
>  
>      /* allocate RAM */
> -    memory_region_allocate_system_memory(ram, NULL, "ppc_prep.ram", ram_size);
> +    memory_region_allocate_system_memory(ram, machine, "ppc_prep.ram",
> +                                         ram_size);
>      memory_region_add_subregion(sysmem, 0, ram);
>  
>      if (linux_boot) {
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 4eb97d3a9b..1f168edd43 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2840,7 +2840,7 @@ static void spapr_machine_init(MachineState *machine)
>      }
>  
>      /* allocate RAM */
> -    memory_region_allocate_system_memory(ram, NULL, "ppc_spapr.ram",
> +    memory_region_allocate_system_memory(ram, machine, "ppc_spapr.ram",
>                                           machine->ram_size);
>      memory_region_add_subregion(sysmem, 0, ram);
>  
> diff --git a/hw/ppc/virtex_ml507.c b/hw/ppc/virtex_ml507.c
> index 68625522d8..86c9273dc0 100644
> --- a/hw/ppc/virtex_ml507.c
> +++ b/hw/ppc/virtex_ml507.c
> @@ -224,7 +224,7 @@ static void virtex_init(MachineState *machine)
>  
>      qemu_register_reset(main_cpu_reset, cpu);
>  
> -    memory_region_allocate_system_memory(phys_ram, NULL, "ram", ram_size);
> +    memory_region_allocate_system_memory(phys_ram, machine, "ram", ram_size);
>      memory_region_add_subregion(address_space_mem, ram_base, phys_ram);
>  
>      dinfo = drive_get(IF_PFLASH, 0, 0);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 18/21] hw/mips: Let the machine be the owner of the system memory

[Aleksandar Markovic]

[-- Attachment #1: Type: text/plain, Size: 4335 bytes --]

On Monday, October 21, 2019, Philippe Mathieu-Daudé <philmd@redhat.com>
wrote:

> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/mips/boston.c        | 2 +-
>  hw/mips/mips_fulong2e.c | 3 ++-
>  hw/mips/mips_jazz.c     | 2 +-
>  hw/mips/mips_malta.c    | 2 +-
>  hw/mips/mips_mipssim.c  | 2 +-
>  hw/mips/mips_r4k.c      | 3 ++-
>  6 files changed, 8 insertions(+), 6 deletions(-)
>
>
Philippe, can this patch be applied independently (on other patches of this
series)?

Thanks,
A.




> diff --git a/hw/mips/boston.c b/hw/mips/boston.c
> index ca7d813a52..8445fee0f1 100644
> --- a/hw/mips/boston.c
> +++ b/hw/mips/boston.c
> @@ -474,7 +474,7 @@ static void boston_mach_init(MachineState *machine)
>      memory_region_add_subregion_overlap(sys_mem, 0x18000000, flash, 0);
>
>      ddr = g_new(MemoryRegion, 1);
> -    memory_region_allocate_system_memory(ddr, NULL, "boston.ddr",
> +    memory_region_allocate_system_memory(ddr, machine, "boston.ddr",
>                                           machine->ram_size);
>      memory_region_add_subregion_overlap(sys_mem, 0x80000000, ddr, 0);
>
> diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
> index cf537dd7e6..d5a5cef619 100644
> --- a/hw/mips/mips_fulong2e.c
> +++ b/hw/mips/mips_fulong2e.c
> @@ -318,7 +318,8 @@ static void mips_fulong2e_init(MachineState *machine)
>      ram_size = 256 * MiB;
>
>      /* allocate RAM */
> -    memory_region_allocate_system_memory(ram, NULL, "fulong2e.ram",
> ram_size);
> +    memory_region_allocate_system_memory(ram, machine,
> +                                         "fulong2e.ram", ram_size);
>      memory_region_init_ram(bios, NULL, "fulong2e.bios", BIOS_SIZE,
>                             &error_fatal);
>      memory_region_set_readonly(bios, true);
> diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
> index 8d010a0b6e..88b125855f 100644
> --- a/hw/mips/mips_jazz.c
> +++ b/hw/mips/mips_jazz.c
> @@ -188,7 +188,7 @@ static void mips_jazz_init(MachineState *machine,
>      cc->do_transaction_failed = mips_jazz_do_transaction_failed;
>
>      /* allocate RAM */
> -    memory_region_allocate_system_memory(ram, NULL, "mips_jazz.ram",
> +    memory_region_allocate_system_memory(ram, machine, "mips_jazz.ram",
>                                           machine->ram_size);
>      memory_region_add_subregion(address_space, 0, ram);
>
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index 4d9c64b36a..af56a29ccb 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -1267,7 +1267,7 @@ void mips_malta_init(MachineState *machine)
>      }
>
>      /* register RAM at high address where it is undisturbed by IO */
> -    memory_region_allocate_system_memory(ram_high, NULL,
> "mips_malta.ram",
> +    memory_region_allocate_system_memory(ram_high, machine,
> "mips_malta.ram",
>                                           ram_size);
>      memory_region_add_subregion(system_memory, 0x80000000, ram_high);
>
> diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
> index 282bbecb24..c1933231e2 100644
> --- a/hw/mips/mips_mipssim.c
> +++ b/hw/mips/mips_mipssim.c
> @@ -166,7 +166,7 @@ mips_mipssim_init(MachineState *machine)
>      qemu_register_reset(main_cpu_reset, reset_info);
>
>      /* Allocate RAM. */
> -    memory_region_allocate_system_memory(ram, NULL, "mips_mipssim.ram",
> +    memory_region_allocate_system_memory(ram, machine,
> "mips_mipssim.ram",
>                                           ram_size);
>      memory_region_init_ram(bios, NULL, "mips_mipssim.bios", BIOS_SIZE,
>                             &error_fatal);
> diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
> index bc0be26544..59f8cacfb6 100644
> --- a/hw/mips/mips_r4k.c
> +++ b/hw/mips/mips_r4k.c
> @@ -203,7 +203,8 @@ void mips_r4k_init(MachineState *machine)
>                       " maximum 256MB", ram_size / MiB);
>          exit(1);
>      }
> -    memory_region_allocate_system_memory(ram, NULL, "mips_r4k.ram",
> ram_size);
> +    memory_region_allocate_system_memory(ram, machine, "mips_r4k.ram",
> +                                         ram_size);
>
>      memory_region_add_subregion(address_space_mem, 0, ram);
>
> --
> 2.21.0
>
>
>

[-- Attachment #2: Type: text/html, Size: 5476 bytes --]

Re: [PATCH 09/21] hw: Let memory_region_allocate_system_memory take MachineState argument

[Thomas Huth]

On 21/10/2019 00.56, Philippe Mathieu-Daudé wrote:
> All the codebase calls memory_region_allocate_system_memory() with
> a NULL 'owner' from the board_init() function.

It's a little bit weird that you first changed the owner to NULL in
patch 7, just to change the type of the parameter here and then change
the parameter back to the machine afterwards. I think I'd rather squash
pash 7 (and the follow-up patches like 14) into this one here - it's
just four files that need to be adapted, so I think that's still fine,
and finally that's less churn to be reviewed.

> Let pass a MachineState argument, and enforce the QOM ownership of
> the system memory.

BTW, good idea!

> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/core/numa.c      | 11 +++++++----
>  include/hw/boards.h |  6 ++++--
>  2 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/core/numa.c b/hw/core/numa.c
> index 038c96d4ab..2e29e4bfe0 100644
> --- a/hw/core/numa.c
> +++ b/hw/core/numa.c
> @@ -520,21 +520,24 @@ static void allocate_system_memory_nonnuma(MemoryRegion *mr, Object *owner,
>      vmstate_register_ram_global(mr);
>  }
>  
> -void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
> +void memory_region_allocate_system_memory(MemoryRegion *mr, MachineState *ms,
>                                            const char *name,
>                                            uint64_t ram_size)
>  {
>      uint64_t addr = 0;
>      int i;
> -    MachineState *ms = MACHINE(qdev_get_machine());
> +
> +    if (!ms) {
> +        ms = MACHINE(qdev_get_machine());
> +    }
>  
>      if (ms->numa_state == NULL ||
>          ms->numa_state->num_nodes == 0 || !have_memdevs) {
> -        allocate_system_memory_nonnuma(mr, owner, name, ram_size);
> +        allocate_system_memory_nonnuma(mr, OBJECT(ms), name, ram_size);
>          return;
>      }
>  
> -    memory_region_init(mr, owner, name, ram_size);
> +    memory_region_init(mr, OBJECT(ms), name, ram_size);
>      for (i = 0; i < ms->numa_state->num_nodes; i++) {
>          uint64_t size = ms->numa_state->nodes[i].node_mem;
>          HostMemoryBackend *backend = ms->numa_state->nodes[i].node_memdev;
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index de45087f34..3b6cb82b6c 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -14,7 +14,7 @@
>  /**
>   * memory_region_allocate_system_memory - Allocate a board's main memory
>   * @mr: the #MemoryRegion to be initialized
> - * @owner: the object that tracks the region's reference count
> + * @ms: the #MachineState object that own the system memory

s/own/owns/

>   * @name: name of the memory region
>   * @ram_size: size of the region in bytes
>   *
> @@ -38,8 +38,10 @@
>   * Smaller pieces of memory (display RAM, static RAMs, etc) don't need
>   * to be backed via the -mem-path memory backend and can simply
>   * be created via memory_region_init_ram().
> + *
> + * The #MachineState object will track the region's reference count.
>   */
> -void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
> +void memory_region_allocate_system_memory(MemoryRegion *mr, MachineState *ms,
>                                            const char *name,
>                                            uint64_t ram_size);
>  
> 

 Thomas

Re: [PATCH 17/21] hw/m68k: Let the machine be the owner of the system memory

[Thomas Huth]

On 21/10/2019 00.56, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/m68k/an5206.c    | 2 +-
>  hw/m68k/mcf5208.c   | 2 +-
>  hw/m68k/next-cube.c | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/m68k/an5206.c b/hw/m68k/an5206.c
> index 54ccbe1a82..fb045c2436 100644
> --- a/hw/m68k/an5206.c
> +++ b/hw/m68k/an5206.c
> @@ -46,7 +46,7 @@ static void an5206_init(MachineState *machine)
>      env->rambar0 = AN5206_RAMBAR_ADDR | 1;
>  
>      /* DRAM at address zero */
> -    memory_region_allocate_system_memory(ram, NULL, "an5206.ram", ram_size);
> +    memory_region_allocate_system_memory(ram, machine, "an5206.ram", ram_size);
>      memory_region_add_subregion(address_space_mem, 0, ram);
>  
>      /* Internal SRAM.  */
> diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
> index 34d34eba17..8e8c8ef349 100644
> --- a/hw/m68k/mcf5208.c
> +++ b/hw/m68k/mcf5208.c
> @@ -248,7 +248,7 @@ static void mcf5208evb_init(MachineState *machine)
>      memory_region_add_subregion(address_space_mem, 0x00000000, rom);
>  
>      /* DRAM at 0x40000000 */
> -    memory_region_allocate_system_memory(ram, NULL, "mcf5208.ram", ram_size);
> +    memory_region_allocate_system_memory(ram, machine, "mcf5208.ram", ram_size);
>      memory_region_add_subregion(address_space_mem, 0x40000000, ram);
>  
>      /* Internal SRAM.  */
> diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c
> index e5343348d0..6aed9376f3 100644
> --- a/hw/m68k/next-cube.c
> +++ b/hw/m68k/next-cube.c
> @@ -893,7 +893,7 @@ static void next_cube_init(MachineState *machine)
>      memcpy(ns->rtc.ram, rtc_ram2, 32);
>  
>      /* 64MB RAM starting at 0x04000000  */
> -    memory_region_allocate_system_memory(ram, NULL, "next.ram", ram_size);
> +    memory_region_allocate_system_memory(ram, machine, "next.ram", ram_size);
>      memory_region_add_subregion(sysmem, 0x04000000, ram);
>  
>      /* Framebuffer */
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH 09/21] hw: Let memory_region_allocate_system_memory take MachineState argument

[Philippe Mathieu-Daudé]

On 10/21/19 9:27 AM, Thomas Huth wrote:
> On 21/10/2019 00.56, Philippe Mathieu-Daudé wrote:
>> All the codebase calls memory_region_allocate_system_memory() with
>> a NULL 'owner' from the board_init() function.
> 
> It's a little bit weird that you first changed the owner to NULL in
> patch 7, just to change the type of the parameter here and then change
> the parameter back to the machine afterwards. I think I'd rather squash
> pash 7 (and the follow-up patches like 14) into this one here - it's
> just four files that need to be adapted, so I think that's still fine,
> and finally that's less churn to be reviewed.

I haven't thought of it and like your suggestion :)

>> Let pass a MachineState argument, and enforce the QOM ownership of
>> the system memory.
> 
> BTW, good idea!

Thanks :)

> 
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>   hw/core/numa.c      | 11 +++++++----
>>   include/hw/boards.h |  6 ++++--
>>   2 files changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/core/numa.c b/hw/core/numa.c
>> index 038c96d4ab..2e29e4bfe0 100644
>> --- a/hw/core/numa.c
>> +++ b/hw/core/numa.c
>> @@ -520,21 +520,24 @@ static void allocate_system_memory_nonnuma(MemoryRegion *mr, Object *owner,
>>       vmstate_register_ram_global(mr);
>>   }
>>   
>> -void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
>> +void memory_region_allocate_system_memory(MemoryRegion *mr, MachineState *ms,
>>                                             const char *name,
>>                                             uint64_t ram_size)
>>   {
>>       uint64_t addr = 0;
>>       int i;
>> -    MachineState *ms = MACHINE(qdev_get_machine());
>> +
>> +    if (!ms) {
>> +        ms = MACHINE(qdev_get_machine());
>> +    }
>>   
>>       if (ms->numa_state == NULL ||
>>           ms->numa_state->num_nodes == 0 || !have_memdevs) {
>> -        allocate_system_memory_nonnuma(mr, owner, name, ram_size);
>> +        allocate_system_memory_nonnuma(mr, OBJECT(ms), name, ram_size);
>>           return;
>>       }
>>   
>> -    memory_region_init(mr, owner, name, ram_size);
>> +    memory_region_init(mr, OBJECT(ms), name, ram_size);
>>       for (i = 0; i < ms->numa_state->num_nodes; i++) {
>>           uint64_t size = ms->numa_state->nodes[i].node_mem;
>>           HostMemoryBackend *backend = ms->numa_state->nodes[i].node_memdev;
>> diff --git a/include/hw/boards.h b/include/hw/boards.h
>> index de45087f34..3b6cb82b6c 100644
>> --- a/include/hw/boards.h
>> +++ b/include/hw/boards.h
>> @@ -14,7 +14,7 @@
>>   /**
>>    * memory_region_allocate_system_memory - Allocate a board's main memory
>>    * @mr: the #MemoryRegion to be initialized
>> - * @owner: the object that tracks the region's reference count
>> + * @ms: the #MachineState object that own the system memory
> 
> s/own/owns/
> 
>>    * @name: name of the memory region
>>    * @ram_size: size of the region in bytes
>>    *
>> @@ -38,8 +38,10 @@
>>    * Smaller pieces of memory (display RAM, static RAMs, etc) don't need
>>    * to be backed via the -mem-path memory backend and can simply
>>    * be created via memory_region_init_ram().
>> + *
>> + * The #MachineState object will track the region's reference count.
>>    */
>> -void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
>> +void memory_region_allocate_system_memory(MemoryRegion *mr, MachineState *ms,
>>                                             const char *name,
>>                                             uint64_t ram_size);
>>   
>>
> 
>   Thomas
> 

Re: [PATCH 18/21] hw/mips: Let the machine be the owner of the system memory

[Philippe Mathieu-Daudé]

Hi Aleksandar,

On 10/21/19 8:25 AM, Aleksandar Markovic wrote:
> 
> 
> On Monday, October 21, 2019, Philippe Mathieu-Daudé <philmd@redhat.com 
> <mailto:philmd@redhat.com>> wrote:
> 
>     Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com
>     <mailto:philmd@redhat.com>>
>     ---
>       hw/mips/boston.c        | 2 +-
>       hw/mips/mips_fulong2e.c | 3 ++-
>       hw/mips/mips_jazz.c     | 2 +-
>       hw/mips/mips_malta.c    | 2 +-
>       hw/mips/mips_mipssim.c  | 2 +-
>       hw/mips/mips_r4k.c      | 3 ++-
>       6 files changed, 8 insertions(+), 6 deletions(-)
> 
> 
> Philippe, can this patch be applied independently (on other patches of 
> this series)?

Unfortunately not because patch #9 changed the prototype of
memory_region_allocate_system_memory() so this patch alone
won't build.

I rather expect this series goes as a whole via the machine-next
tree.

Do you mind giving your Acked-by tag to this patch?

Thanks,

Phil.

> 
> 
>     diff --git a/hw/mips/boston.c b/hw/mips/boston.c
>     index ca7d813a52..8445fee0f1 100644
>     --- a/hw/mips/boston.c
>     +++ b/hw/mips/boston.c
>     @@ -474,7 +474,7 @@ static void boston_mach_init(MachineState *machine)
>           memory_region_add_subregion_overlap(sys_mem, 0x18000000,
>     flash, 0);
> 
>           ddr = g_new(MemoryRegion, 1);
>     -    memory_region_allocate_system_memory(ddr, NULL, "boston.ddr",
>     +    memory_region_allocate_system_memory(ddr, machine, "boston.ddr",
>                                                machine->ram_size);
>           memory_region_add_subregion_overlap(sys_mem, 0x80000000, ddr, 0);
> 
>     diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
>     index cf537dd7e6..d5a5cef619 100644
>     --- a/hw/mips/mips_fulong2e.c
>     +++ b/hw/mips/mips_fulong2e.c
>     @@ -318,7 +318,8 @@ static void mips_fulong2e_init(MachineState
>     *machine)
>           ram_size = 256 * MiB;
> 
>           /* allocate RAM */
>     -    memory_region_allocate_system_memory(ram, NULL, "fulong2e.ram",
>     ram_size);
>     +    memory_region_allocate_system_memory(ram, machine,
>     +                                         "fulong2e.ram", ram_size);
>           memory_region_init_ram(bios, NULL, "fulong2e.bios", BIOS_SIZE,
>                                  &error_fatal);
>           memory_region_set_readonly(bios, true);
>     diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
>     index 8d010a0b6e..88b125855f 100644
>     --- a/hw/mips/mips_jazz.c
>     +++ b/hw/mips/mips_jazz.c
>     @@ -188,7 +188,7 @@ static void mips_jazz_init(MachineState *machine,
>           cc->do_transaction_failed = mips_jazz_do_transaction_failed;
> 
>           /* allocate RAM */
>     -    memory_region_allocate_system_memory(ram, NULL, "mips_jazz.ram",
>     +    memory_region_allocate_system_memory(ram, machine, "mips_jazz.ram",
>                                                machine->ram_size);
>           memory_region_add_subregion(address_space, 0, ram);
> 
>     diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
>     index 4d9c64b36a..af56a29ccb 100644
>     --- a/hw/mips/mips_malta.c
>     +++ b/hw/mips/mips_malta.c
>     @@ -1267,7 +1267,7 @@ void mips_malta_init(MachineState *machine)
>           }
> 
>           /* register RAM at high address where it is undisturbed by IO */
>     -    memory_region_allocate_system_memory(ram_high, NULL,
>     "mips_malta.ram",
>     +    memory_region_allocate_system_memory(ram_high, machine,
>     "mips_malta.ram",
>                                                ram_size);
>           memory_region_add_subregion(system_memory, 0x80000000, ram_high);
> 
>     diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
>     index 282bbecb24..c1933231e2 100644
>     --- a/hw/mips/mips_mipssim.c
>     +++ b/hw/mips/mips_mipssim.c
>     @@ -166,7 +166,7 @@ mips_mipssim_init(MachineState *machine)
>           qemu_register_reset(main_cpu_reset, reset_info);
> 
>           /* Allocate RAM. */
>     -    memory_region_allocate_system_memory(ram, NULL, "mips_mipssim.ram",
>     +    memory_region_allocate_system_memory(ram, machine,
>     "mips_mipssim.ram",
>                                                ram_size);
>           memory_region_init_ram(bios, NULL, "mips_mipssim.bios", BIOS_SIZE,
>                                  &error_fatal);
>     diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
>     index bc0be26544..59f8cacfb6 100644
>     --- a/hw/mips/mips_r4k.c
>     +++ b/hw/mips/mips_r4k.c
>     @@ -203,7 +203,8 @@ void mips_r4k_init(MachineState *machine)
>                            " maximum 256MB", ram_size / MiB);
>               exit(1);
>           }
>     -    memory_region_allocate_system_memory(ram, NULL, "mips_r4k.ram",
>     ram_size);
>     +    memory_region_allocate_system_memory(ram, machine, "mips_r4k.ram",
>     +                                         ram_size);
> 
>           memory_region_add_subregion(address_space_mem, 0, ram);
> 
>     -- 
>     2.21.0
> 
> 

Re: [PATCH 00/21] hw: Let the machine be the owner of the system memory

[Philippe Mathieu-Daudé]

On 10/21/19 12:56 AM, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> This series is based on Igor's "eliminate remaining places that
> abuse memory_region_allocate_system_memory()":
> https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg01601.html
> 
> It is quite simple, we enforce all machines to be the QOM owner
> of the system memory.
> 
> This changes the memory tree from:
> 
>    (qemu) info mtree -o
>    memory-region: pc.ram
>      0000000000000000-0000000007ffffff (prio 0, ram): pc.ram parent:{obj path=/machine/unattached}
> 
> to:
> 
>    (qemu) info mtree -o
>    memory-region: pc.ram
>      0000000000000000-0000000007ffffff (prio 0, ram): pc.ram owner:{obj path=/machine}
> 
> Few patches are required to clean the codebase first, to unify the
> creation of the system memory in the board/machine code. Mostly some
> old ARM machines (pre-QOM) were affected.
> 
> Please review (as a generic codebase cleanup).
> 
> Regards,
> 
> Phil.
> 
> Based-on: <20191008113318.7012-1-imammedo@redhat.com>
> 
> Philippe Mathieu-Daudé (21):
>    hw/arm/xilinx_zynq: Use the IEC binary prefix definitions
>    hw/arm/mps2: Use the IEC binary prefix definitions
>    hw/arm/collie: Create the RAM in the board
>    hw/arm/omap2: Create the RAM in the board
>    hw/arm/omap1: Create the RAM in the board
>    hw/arm/digic4: Inline digic4_board_setup_ram() function
>    hw: Drop QOM ownership on memory_region_allocate_system_memory() calls
>    hw/alpha/dp264: Create the RAM in the board
>    hw: Let memory_region_allocate_system_memory take MachineState
>      argument
>    hw/core: Let the machine be the owner of the system memory
>    hw/alpha: Let the machine be the owner of the system memory
>    hw/arm: Let the machine be the owner of the system memory
>    hw/cris: Let the machine be the owner of the system memory
>    hw/hppa: Let the machine be the owner of the system memory
>    hw/i386: Let the machine be the owner of the system memory
>    hw/lm32: Let the machine be the owner of the system memory
>    hw/m68k: Let the machine be the owner of the system memory
>    hw/mips: Let the machine be the owner of the system memory
>    hw/ppc: Let the machine be the owner of the system memory
>    hw/sparc: Let the machine be the owner of the system memory
>    hw/core: Assert memory_region_allocate_system_memory has machine owner

I forgot 4 other calls:

hw/ppc/ppc4xx_devs.c:708:    memory_region_allocate_system_memory(ram, 
NULL, "ppc4xx.sdram", ram_size);
hw/s390x/s390-virtio-ccw.c:164: 
memory_region_allocate_system_memory(ram, NULL, "s390.ram", mem_size);
hw/sparc/sun4m.c:791:    memory_region_allocate_system_memory(&d->ram, 
NULL, "sun4m.ram",
hw/sparc64/niagara.c:114: 
memory_region_allocate_system_memory(&s->partition_ram, NULL,

Re: [PATCH 08/21] hw/alpha/dp264: Create the RAM in the board

[Philippe Mathieu-Daudé]

On 10/21/19 12:56 AM, Philippe Mathieu-Daudé wrote:
> The SDRAM is incorrectly created in the Tyohoon northbridge ASIC.
> Move its creation in the board code, this will later allow the
> board to have the QOM ownership of the RAM.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> checkpatch complains:
> 
>   ERROR: spaces required around that '*' (ctx:WxV)
>   #10: FILE: hw/alpha/alpha_sys.h:13:
>   +PCIBus *typhoon_init(ISABus **, qemu_irq *, AlphaCPU *[4],
>                                                         ^
> ---
>   hw/alpha/alpha_sys.h |  2 +-
>   hw/alpha/dp264.c     | 11 ++++++++++-
>   hw/alpha/typhoon.c   |  9 +--------
>   3 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/alpha/alpha_sys.h b/hw/alpha/alpha_sys.h
> index 4e127a6de8..17f97e1bfe 100644
> --- a/hw/alpha/alpha_sys.h
> +++ b/hw/alpha/alpha_sys.h
> @@ -10,7 +10,7 @@
>   #include "hw/i386/pc.h"
>   
>   
> -PCIBus *typhoon_init(ram_addr_t, ISABus **, qemu_irq *, AlphaCPU *[4],
> +PCIBus *typhoon_init(ISABus **, qemu_irq *, AlphaCPU *[4],
>                        pci_map_irq_fn);
>   
>   /* alpha_pci.c.  */
> diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
> index 51feee8558..ddc249261c 100644
> --- a/hw/alpha/dp264.c
> +++ b/hw/alpha/dp264.c
> @@ -62,6 +62,7 @@ static void clipper_init(MachineState *machine)
>       uint64_t palcode_entry, palcode_low, palcode_high;
>       uint64_t kernel_entry, kernel_low, kernel_high;
>       unsigned int smp_cpus = machine->smp.cpus;
> +    MemoryRegion ram_region;

This is incorrect, this variable should be in .heap, not stack.

>   
>       /* Create up to 4 cpus.  */
>       memset(cpus, 0, sizeof(cpus));
> @@ -73,8 +74,16 @@ static void clipper_init(MachineState *machine)
>       cpus[0]->env.trap_arg1 = 0;
>       cpus[0]->env.trap_arg2 = smp_cpus;
>   
> +    /*
> +     * Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
> +     * but the address space hole reserved at this point is 8TB.
> +     */
> +    memory_region_allocate_system_memory(&ram_region, NULL, "ram",
> +                                         ram_size);
> +    memory_region_add_subregion(get_system_memory(), 0, &ram_region);
> +
>       /* Init the chipset.  */
> -    pci_bus = typhoon_init(ram_size, &isa_bus, &rtc_irq, cpus,
> +    pci_bus = typhoon_init(&isa_bus, &rtc_irq, cpus,
>                              clipper_pci_map_irq);
>   
>       /* Since we have an SRM-compatible PALcode, use the SRM epoch.  */
> diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
> index 8489ec335c..12589ef331 100644
> --- a/hw/alpha/typhoon.c
> +++ b/hw/alpha/typhoon.c
> @@ -58,7 +58,6 @@ typedef struct TyphoonState {
>       TyphoonCchip cchip;
>       TyphoonPchip pchip;
>       MemoryRegion dchip_region;
> -    MemoryRegion ram_region;
>   } TyphoonState;
>   
>   /* Called when one of DRIR or DIM changes.  */
> @@ -817,7 +816,7 @@ static void typhoon_alarm_timer(void *opaque)
>       cpu_interrupt(CPU(s->cchip.cpu[cpu]), CPU_INTERRUPT_TIMER);
>   }
>   
> -PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
> +PCIBus *typhoon_init(ISABus **isa_bus,
>                        qemu_irq *p_rtc_irq,
>                        AlphaCPU *cpus[4], pci_map_irq_fn sys_map_irq)
>   {
> @@ -849,12 +848,6 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
>   
>       *p_rtc_irq = qemu_allocate_irq(typhoon_set_timer_irq, s, 0);
>   
> -    /* Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
> -       but the address space hole reserved at this point is 8TB.  */
> -    memory_region_allocate_system_memory(&s->ram_region, NULL, "ram",
> -                                         ram_size);
> -    memory_region_add_subregion(addr_space, 0, &s->ram_region);
> -
>       /* TIGbus, 0x801.0000.0000, 1GB.  */
>       /* ??? The TIGbus is used for delivering interrupts, and access to
>          the flash ROM.  I'm not sure that we need to implement it at all.  */
> 

Re: [PATCH 12/21] hw/arm: Let the machine be the owner of the system memory

[Peter Maydell]

On Mon, 21 Oct 2019 at 00:01, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---

>  hw/arm/virt.c             | 2 +-

I think from a quick code scan that this is ok, but did
you test that migration compat from old to new still works?
I vaguely recall that there are some cases when adding an
owner to a memory region changes the name string used for
identifying the ramblock in migration.

thanks
-- PMM

Re: [PATCH 12/21] hw/arm: Let the machine be the owner of the system memory

[Philippe Mathieu-Daudé]

On 10/21/19 11:22 AM, Peter Maydell wrote:
> On Mon, 21 Oct 2019 at 00:01, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
> 
>>   hw/arm/virt.c             | 2 +-
> 
> I think from a quick code scan that this is ok, but did
> you test that migration compat from old to new still works?
> I vaguely recall that there are some cases when adding an
> owner to a memory region changes the name string used for
> identifying the ramblock in migration.

It seems to still works:

$ make check-qtest-aarch64 V=1
QTEST_QEMU_BINARY=aarch64-softmmu/qemu-system-aarch64 
QTEST_QEMU_IMG=qemu-img tests/migration-test -m=quick -k --tap < 
/dev/null | ./scripts/tap-driver.pl --test-name="migration-test"
PASS 1 migration-test /aarch64/migration/deprecated
PASS 2 migration-test /aarch64/migration/bad_dest
PASS 3 migration-test /aarch64/migration/fd_proto
PASS 4 migration-test /aarch64/migration/validate_uuid
PASS 5 migration-test /aarch64/migration/validate_uuid_error
PASS 6 migration-test /aarch64/migration/validate_uuid_src_not_set
PASS 7 migration-test /aarch64/migration/validate_uuid_dst_not_set
PASS 8 migration-test /aarch64/migration/auto_converge
PASS 9 migration-test /aarch64/migration/postcopy/unix
PASS 10 migration-test /aarch64/migration/postcopy/recovery
PASS 11 migration-test /aarch64/migration/precopy/unix
PASS 12 migration-test /aarch64/migration/precopy/tcp
PASS 13 migration-test /aarch64/migration/xbzrle/unix

This test migrate the virt machine.

Is this enough?

Regards,

Phil.

Re: [PATCH 12/21] hw/arm: Let the machine be the owner of the system memory

[Peter Maydell]

On Mon, 21 Oct 2019 at 10:34, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> On 10/21/19 11:22 AM, Peter Maydell wrote:
> > On Mon, 21 Oct 2019 at 00:01, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> >>
> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >> ---
> >
> >>   hw/arm/virt.c             | 2 +-
> >
> > I think from a quick code scan that this is ok, but did
> > you test that migration compat from old to new still works?
> > I vaguely recall that there are some cases when adding an
> > owner to a memory region changes the name string used for
> > identifying the ramblock in migration.
>
> It seems to still works:
>
> $ make check-qtest-aarch64 V=1

> This test migrate the virt machine.
>
> Is this enough?

No, you need to test migration from a QEMU binary without
this patchset to a QEMU binary that has it. Otherwise you're
only checking that the new version can migrate from itself
to itself. I find the easiest way to test this is just to
use the 'savevm' command to save a state snapshot to a
qcow2 disk image while running the old binary, and then run
'loadvm' with the new binary and check it restored OK.

thanks
-- PMM

Re: [PATCH 12/21] hw/arm: Let the machine be the owner of the system memory

[Philippe Mathieu-Daudé]

Cc'ing Paolo/David.

On 10/21/19 11:39 AM, Peter Maydell wrote:
> On Mon, 21 Oct 2019 at 10:34, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> On 10/21/19 11:22 AM, Peter Maydell wrote:
>>> On Mon, 21 Oct 2019 at 00:01, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>> ---
>>>
>>>>    hw/arm/virt.c             | 2 +-
>>>
>>> I think from a quick code scan that this is ok, but did
>>> you test that migration compat from old to new still works?
>>> I vaguely recall that there are some cases when adding an
>>> owner to a memory region changes the name string used for
>>> identifying the ramblock in migration.
>>
>> It seems to still works:
>>
>> $ make check-qtest-aarch64 V=1
> 
>> This test migrate the virt machine.
>>
>> Is this enough?
> 
> No, you need to test migration from a QEMU binary without
> this patchset to a QEMU binary that has it. Otherwise you're
> only checking that the new version can migrate from itself
> to itself. I find the easiest way to test this is just to
> use the 'savevm' command to save a state snapshot to a
> qcow2 disk image while running the old binary, and then run
> 'loadvm' with the new binary and check it restored OK.

I did not think if this case.

I followed your blog post [*] and tested the migration works OK.

Paolo, now thinking about regular testing, we should add this testing to 
patchew too. Something like:

- when mainstream/master is updated, patchew build QEMU (it should be 
already mostly ccached) and generate some vm dumps with 'savevm'.

- build/test the series

- if series succeeded testing, run 'loadvm' tests

[*] 
https://translatedcode.wordpress.com/2015/07/06/tricks-for-debugging-qemu-savevm-snapshots/

Re: [PATCH 18/21] hw/mips: Let the machine be the owner of the system memory

[Aleksandar Markovic]

[-- Attachment #1: Type: text/plain, Size: 5499 bytes --]

On Monday, October 21, 2019, Philippe Mathieu-Daudé <philmd@redhat.com>
wrote:

> Hi Aleksandar,
>
> On 10/21/19 8:25 AM, Aleksandar Markovic wrote:
>
>>
>>
>> On Monday, October 21, 2019, Philippe Mathieu-Daudé <philmd@redhat.com
>> <mailto:philmd@redhat.com>> wrote:
>>
>>     Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com
>>     <mailto:philmd@redhat.com>>
>>     ---
>>       hw/mips/boston.c        | 2 +-
>>       hw/mips/mips_fulong2e.c | 3 ++-
>>       hw/mips/mips_jazz.c     | 2 +-
>>       hw/mips/mips_malta.c    | 2 +-
>>       hw/mips/mips_mipssim.c  | 2 +-
>>       hw/mips/mips_r4k.c      | 3 ++-
>>       6 files changed, 8 insertions(+), 6 deletions(-)
>>
>>
>> Philippe, can this patch be applied independently (on other patches of
>> this series)?
>>
>
> Unfortunately not because patch #9 changed the prototype of
> memory_region_allocate_system_memory() so this patch alone
> won't build.
>
> I rather expect this series goes as a whole via the machine-next
> tree.
>
> Do you mind giving your Acked-by tag to this patch?
>
>
I can't, sorry, for MIPS target, we don't accept patches with empty commit
messages.

A.




> Thanks,
>
> Phil.
>
>
>>
>>     diff --git a/hw/mips/boston.c b/hw/mips/boston.c
>>     index ca7d813a52..8445fee0f1 100644
>>     --- a/hw/mips/boston.c
>>     +++ b/hw/mips/boston.c
>>     @@ -474,7 +474,7 @@ static void boston_mach_init(MachineState
>> *machine)
>>           memory_region_add_subregion_overlap(sys_mem, 0x18000000,
>>     flash, 0);
>>
>>           ddr = g_new(MemoryRegion, 1);
>>     -    memory_region_allocate_system_memory(ddr, NULL, "boston.ddr",
>>     +    memory_region_allocate_system_memory(ddr, machine, "boston.ddr",
>>                                                machine->ram_size);
>>           memory_region_add_subregion_overlap(sys_mem, 0x80000000, ddr,
>> 0);
>>
>>     diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
>>     index cf537dd7e6..d5a5cef619 100644
>>     --- a/hw/mips/mips_fulong2e.c
>>     +++ b/hw/mips/mips_fulong2e.c
>>     @@ -318,7 +318,8 @@ static void mips_fulong2e_init(MachineState
>>     *machine)
>>           ram_size = 256 * MiB;
>>
>>           /* allocate RAM */
>>     -    memory_region_allocate_system_memory(ram, NULL, "fulong2e.ram",
>>     ram_size);
>>     +    memory_region_allocate_system_memory(ram, machine,
>>     +                                         "fulong2e.ram", ram_size);
>>           memory_region_init_ram(bios, NULL, "fulong2e.bios", BIOS_SIZE,
>>                                  &error_fatal);
>>           memory_region_set_readonly(bios, true);
>>     diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
>>     index 8d010a0b6e..88b125855f 100644
>>     --- a/hw/mips/mips_jazz.c
>>     +++ b/hw/mips/mips_jazz.c
>>     @@ -188,7 +188,7 @@ static void mips_jazz_init(MachineState *machine,
>>           cc->do_transaction_failed = mips_jazz_do_transaction_failed;
>>
>>           /* allocate RAM */
>>     -    memory_region_allocate_system_memory(ram, NULL, "mips_jazz.ram",
>>     +    memory_region_allocate_system_memory(ram, machine,
>> "mips_jazz.ram",
>>                                                machine->ram_size);
>>           memory_region_add_subregion(address_space, 0, ram);
>>
>>     diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
>>     index 4d9c64b36a..af56a29ccb 100644
>>     --- a/hw/mips/mips_malta.c
>>     +++ b/hw/mips/mips_malta.c
>>     @@ -1267,7 +1267,7 @@ void mips_malta_init(MachineState *machine)
>>           }
>>
>>           /* register RAM at high address where it is undisturbed by IO */
>>     -    memory_region_allocate_system_memory(ram_high, NULL,
>>     "mips_malta.ram",
>>     +    memory_region_allocate_system_memory(ram_high, machine,
>>     "mips_malta.ram",
>>                                                ram_size);
>>           memory_region_add_subregion(system_memory, 0x80000000,
>> ram_high);
>>
>>     diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
>>     index 282bbecb24..c1933231e2 100644
>>     --- a/hw/mips/mips_mipssim.c
>>     +++ b/hw/mips/mips_mipssim.c
>>     @@ -166,7 +166,7 @@ mips_mipssim_init(MachineState *machine)
>>           qemu_register_reset(main_cpu_reset, reset_info);
>>
>>           /* Allocate RAM. */
>>     -    memory_region_allocate_system_memory(ram, NULL,
>> "mips_mipssim.ram",
>>     +    memory_region_allocate_system_memory(ram, machine,
>>     "mips_mipssim.ram",
>>                                                ram_size);
>>           memory_region_init_ram(bios, NULL, "mips_mipssim.bios",
>> BIOS_SIZE,
>>                                  &error_fatal);
>>     diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
>>     index bc0be26544..59f8cacfb6 100644
>>     --- a/hw/mips/mips_r4k.c
>>     +++ b/hw/mips/mips_r4k.c
>>     @@ -203,7 +203,8 @@ void mips_r4k_init(MachineState *machine)
>>                            " maximum 256MB", ram_size / MiB);
>>               exit(1);
>>           }
>>     -    memory_region_allocate_system_memory(ram, NULL, "mips_r4k.ram",
>>     ram_size);
>>     +    memory_region_allocate_system_memory(ram, machine,
>> "mips_r4k.ram",
>>     +                                         ram_size);
>>
>>           memory_region_add_subregion(address_space_mem, 0, ram);
>>
>>     --     2.21.0
>>
>>
>>

[-- Attachment #2: Type: text/html, Size: 7234 bytes --]

Re: [PATCH 18/21] hw/mips: Let the machine be the owner of the system memory

[Philippe Mathieu-Daudé]

On 10/21/19 12:56 PM, Aleksandar Markovic wrote:
> 
> 
> On Monday, October 21, 2019, Philippe Mathieu-Daudé <philmd@redhat.com 
> <mailto:philmd@redhat.com>> wrote:
> 
>     Hi Aleksandar,
> 
>     On 10/21/19 8:25 AM, Aleksandar Markovic wrote:
> 
> 
> 
>         On Monday, October 21, 2019, Philippe Mathieu-Daudé
>         <philmd@redhat.com <mailto:philmd@redhat.com>
>         <mailto:philmd@redhat.com <mailto:philmd@redhat.com>>> wrote:
> 
>              Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com
>         <mailto:philmd@redhat.com>
>              <mailto:philmd@redhat.com <mailto:philmd@redhat.com>>>
>              ---
>                hw/mips/boston.c        | 2 +-
>                hw/mips/mips_fulong2e.c | 3 ++-
>                hw/mips/mips_jazz.c     | 2 +-
>                hw/mips/mips_malta.c    | 2 +-
>                hw/mips/mips_mipssim.c  | 2 +-
>                hw/mips/mips_r4k.c      | 3 ++-
>                6 files changed, 8 insertions(+), 6 deletions(-)
> 
> 
>         Philippe, can this patch be applied independently (on other
>         patches of this series)?
> 
> 
>     Unfortunately not because patch #9 changed the prototype of
>     memory_region_allocate_system_memory() so this patch alone
>     won't build.
> 
>     I rather expect this series goes as a whole via the machine-next
>     tree.
> 
>     Do you mind giving your Acked-by tag to this patch?
> 
> 
> I can't, sorry, for MIPS target, we don't accept patches with empty 
> commit messages.

I see, I'v been wondering what to do, if simply repeating the patch 
subject or the cover, but since the series applies to all targets, this 
seems overkill. I think I'll simply repeat the subject then.

> 
> A.
> 
> 
>     Thanks,
> 
>     Phil.
> 
> 
> 
>              diff --git a/hw/mips/boston.c b/hw/mips/boston.c
>              index ca7d813a52..8445fee0f1 100644
>              --- a/hw/mips/boston.c
>              +++ b/hw/mips/boston.c
>              @@ -474,7 +474,7 @@ static void
>         boston_mach_init(MachineState *machine)
>                    memory_region_add_subregion_overlap(sys_mem, 0x18000000,
>              flash, 0);
> 
>                    ddr = g_new(MemoryRegion, 1);
>              -    memory_region_allocate_system_memory(ddr, NULL,
>         "boston.ddr",
>              +    memory_region_allocate_system_memory(ddr, machine,
>         "boston.ddr",
>                                                         machine->ram_size);
>                    memory_region_add_subregion_overlap(sys_mem,
>         0x80000000, ddr, 0);
> 
>              diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
>              index cf537dd7e6..d5a5cef619 100644
>              --- a/hw/mips/mips_fulong2e.c
>              +++ b/hw/mips/mips_fulong2e.c
>              @@ -318,7 +318,8 @@ static void mips_fulong2e_init(MachineState
>              *machine)
>                    ram_size = 256 * MiB;
> 
>                    /* allocate RAM */
>              -    memory_region_allocate_system_memory(ram, NULL,
>         "fulong2e.ram",
>              ram_size);
>              +    memory_region_allocate_system_memory(ram, machine,
>              +                                         "fulong2e.ram",
>         ram_size);
>                    memory_region_init_ram(bios, NULL, "fulong2e.bios",
>         BIOS_SIZE,
>                                           &error_fatal);
>                    memory_region_set_readonly(bios, true);
>              diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
>              index 8d010a0b6e..88b125855f 100644
>              --- a/hw/mips/mips_jazz.c
>              +++ b/hw/mips/mips_jazz.c
>              @@ -188,7 +188,7 @@ static void mips_jazz_init(MachineState
>         *machine,
>                    cc->do_transaction_failed =
>         mips_jazz_do_transaction_failed;
> 
>                    /* allocate RAM */
>              -    memory_region_allocate_system_memory(ram, NULL,
>         "mips_jazz.ram",
>              +    memory_region_allocate_system_memory(ram, machine,
>         "mips_jazz.ram",
>                                                         machine->ram_size);
>                    memory_region_add_subregion(address_space, 0, ram);
> 
>              diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
>              index 4d9c64b36a..af56a29ccb 100644
>              --- a/hw/mips/mips_malta.c
>              +++ b/hw/mips/mips_malta.c
>              @@ -1267,7 +1267,7 @@ void mips_malta_init(MachineState
>         *machine)
>                    }
> 
>                    /* register RAM at high address where it is
>         undisturbed by IO */
>              -    memory_region_allocate_system_memory(ram_high, NULL,
>              "mips_malta.ram",
>              +    memory_region_allocate_system_memory(ram_high, machine,
>              "mips_malta.ram",
>                                                         ram_size);
>                    memory_region_add_subregion(system_memory,
>         0x80000000, ram_high);
> 
>              diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
>              index 282bbecb24..c1933231e2 100644
>              --- a/hw/mips/mips_mipssim.c
>              +++ b/hw/mips/mips_mipssim.c
>              @@ -166,7 +166,7 @@ mips_mipssim_init(MachineState *machine)
>                    qemu_register_reset(main_cpu_reset, reset_info);
> 
>                    /* Allocate RAM. */
>              -    memory_region_allocate_system_memory(ram, NULL,
>         "mips_mipssim.ram",
>              +    memory_region_allocate_system_memory(ram, machine,
>              "mips_mipssim.ram",
>                                                         ram_size);
>                    memory_region_init_ram(bios, NULL,
>         "mips_mipssim.bios", BIOS_SIZE,
>                                           &error_fatal);
>              diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
>              index bc0be26544..59f8cacfb6 100644
>              --- a/hw/mips/mips_r4k.c
>              +++ b/hw/mips/mips_r4k.c
>              @@ -203,7 +203,8 @@ void mips_r4k_init(MachineState *machine)
>                                     " maximum 256MB", ram_size / MiB);
>                        exit(1);
>                    }
>              -    memory_region_allocate_system_memory(ram, NULL,
>         "mips_r4k.ram",
>              ram_size);
>              +    memory_region_allocate_system_memory(ram, machine,
>         "mips_r4k.ram",
>              +                                         ram_size);
> 
>                    memory_region_add_subregion(address_space_mem, 0, ram);
> 
>              --     2.21.0
> 
> 

Re: [PATCH 18/21] hw/mips: Let the machine be the owner of the system memory

[Aleksandar Markovic]

[-- Attachment #1: Type: text/plain, Size: 7133 bytes --]

On Monday, October 21, 2019, Philippe Mathieu-Daudé <philmd@redhat.com>
wrote:

> On 10/21/19 12:56 PM, Aleksandar Markovic wrote:
>
>>
>>
>> On Monday, October 21, 2019, Philippe Mathieu-Daudé <philmd@redhat.com
>> <mailto:philmd@redhat.com>> wrote:
>>
>>     Hi Aleksandar,
>>
>>     On 10/21/19 8:25 AM, Aleksandar Markovic wrote:
>>
>>
>>
>>         On Monday, October 21, 2019, Philippe Mathieu-Daudé
>>         <philmd@redhat.com <mailto:philmd@redhat.com>
>>         <mailto:philmd@redhat.com <mailto:philmd@redhat.com>>> wrote:
>>
>>              Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com
>>         <mailto:philmd@redhat.com>
>>              <mailto:philmd@redhat.com <mailto:philmd@redhat.com>>>
>>              ---
>>                hw/mips/boston.c        | 2 +-
>>                hw/mips/mips_fulong2e.c | 3 ++-
>>                hw/mips/mips_jazz.c     | 2 +-
>>                hw/mips/mips_malta.c    | 2 +-
>>                hw/mips/mips_mipssim.c  | 2 +-
>>                hw/mips/mips_r4k.c      | 3 ++-
>>                6 files changed, 8 insertions(+), 6 deletions(-)
>>
>>
>>         Philippe, can this patch be applied independently (on other
>>         patches of this series)?
>>
>>
>>     Unfortunately not because patch #9 changed the prototype of
>>     memory_region_allocate_system_memory() so this patch alone
>>     won't build.
>>
>>     I rather expect this series goes as a whole via the machine-next
>>     tree.
>>
>>     Do you mind giving your Acked-by tag to this patch?
>>
>>
>> I can't, sorry, for MIPS target, we don't accept patches with empty
>> commit messages.
>>
>
> I see, I'v been wondering what to do, if simply repeating the patch
> subject or the cover, but since the series applies to all targets, this
> seems overkill. I think I'll simply repeat the subject then.
>
>
Can't you at least add a sentence on why is this needed, or what is
achieved?

Thanks,

A.



>
>> A.
>>
>>
>>     Thanks,
>>
>>     Phil.
>>
>>
>>
>>              diff --git a/hw/mips/boston.c b/hw/mips/boston.c
>>              index ca7d813a52..8445fee0f1 100644
>>              --- a/hw/mips/boston.c
>>              +++ b/hw/mips/boston.c
>>              @@ -474,7 +474,7 @@ static void
>>         boston_mach_init(MachineState *machine)
>>                    memory_region_add_subregion_overlap(sys_mem,
>> 0x18000000,
>>              flash, 0);
>>
>>                    ddr = g_new(MemoryRegion, 1);
>>              -    memory_region_allocate_system_memory(ddr, NULL,
>>         "boston.ddr",
>>              +    memory_region_allocate_system_memory(ddr, machine,
>>         "boston.ddr",
>>
>> machine->ram_size);
>>                    memory_region_add_subregion_overlap(sys_mem,
>>         0x80000000, ddr, 0);
>>
>>              diff --git a/hw/mips/mips_fulong2e.c
>> b/hw/mips/mips_fulong2e.c
>>              index cf537dd7e6..d5a5cef619 100644
>>              --- a/hw/mips/mips_fulong2e.c
>>              +++ b/hw/mips/mips_fulong2e.c
>>              @@ -318,7 +318,8 @@ static void
>> mips_fulong2e_init(MachineState
>>              *machine)
>>                    ram_size = 256 * MiB;
>>
>>                    /* allocate RAM */
>>              -    memory_region_allocate_system_memory(ram, NULL,
>>         "fulong2e.ram",
>>              ram_size);
>>              +    memory_region_allocate_system_memory(ram, machine,
>>              +                                         "fulong2e.ram",
>>         ram_size);
>>                    memory_region_init_ram(bios, NULL, "fulong2e.bios",
>>         BIOS_SIZE,
>>                                           &error_fatal);
>>                    memory_region_set_readonly(bios, true);
>>              diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
>>              index 8d010a0b6e..88b125855f 100644
>>              --- a/hw/mips/mips_jazz.c
>>              +++ b/hw/mips/mips_jazz.c
>>              @@ -188,7 +188,7 @@ static void mips_jazz_init(MachineState
>>         *machine,
>>                    cc->do_transaction_failed =
>>         mips_jazz_do_transaction_failed;
>>
>>                    /* allocate RAM */
>>              -    memory_region_allocate_system_memory(ram, NULL,
>>         "mips_jazz.ram",
>>              +    memory_region_allocate_system_memory(ram, machine,
>>         "mips_jazz.ram",
>>
>> machine->ram_size);
>>                    memory_region_add_subregion(address_space, 0, ram);
>>
>>              diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
>>              index 4d9c64b36a..af56a29ccb 100644
>>              --- a/hw/mips/mips_malta.c
>>              +++ b/hw/mips/mips_malta.c
>>              @@ -1267,7 +1267,7 @@ void mips_malta_init(MachineState
>>         *machine)
>>                    }
>>
>>                    /* register RAM at high address where it is
>>         undisturbed by IO */
>>              -    memory_region_allocate_system_memory(ram_high, NULL,
>>              "mips_malta.ram",
>>              +    memory_region_allocate_system_memory(ram_high, machine,
>>              "mips_malta.ram",
>>                                                         ram_size);
>>                    memory_region_add_subregion(system_memory,
>>         0x80000000, ram_high);
>>
>>              diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
>>              index 282bbecb24..c1933231e2 100644
>>              --- a/hw/mips/mips_mipssim.c
>>              +++ b/hw/mips/mips_mipssim.c
>>              @@ -166,7 +166,7 @@ mips_mipssim_init(MachineState *machine)
>>                    qemu_register_reset(main_cpu_reset, reset_info);
>>
>>                    /* Allocate RAM. */
>>              -    memory_region_allocate_system_memory(ram, NULL,
>>         "mips_mipssim.ram",
>>              +    memory_region_allocate_system_memory(ram, machine,
>>              "mips_mipssim.ram",
>>                                                         ram_size);
>>                    memory_region_init_ram(bios, NULL,
>>         "mips_mipssim.bios", BIOS_SIZE,
>>                                           &error_fatal);
>>              diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
>>              index bc0be26544..59f8cacfb6 100644
>>              --- a/hw/mips/mips_r4k.c
>>              +++ b/hw/mips/mips_r4k.c
>>              @@ -203,7 +203,8 @@ void mips_r4k_init(MachineState *machine)
>>                                     " maximum 256MB", ram_size / MiB);
>>                        exit(1);
>>                    }
>>              -    memory_region_allocate_system_memory(ram, NULL,
>>         "mips_r4k.ram",
>>              ram_size);
>>              +    memory_region_allocate_system_memory(ram, machine,
>>         "mips_r4k.ram",
>>              +                                         ram_size);
>>
>>                    memory_region_add_subregion(address_space_mem, 0,
>> ram);
>>
>>              --     2.21.0
>>
>>
>>

[-- Attachment #2: Type: text/html, Size: 9957 bytes --]

Re: [PATCH 18/21] hw/mips: Let the machine be the owner of the system memory

[Philippe Mathieu-Daudé]

On 10/21/19 1:10 PM, Aleksandar Markovic wrote:
> On Monday, October 21, 2019, Philippe Mathieu-Daudé <philmd@redhat.com 
> <mailto:philmd@redhat.com>> wrote:
> 
>     On 10/21/19 12:56 PM, Aleksandar Markovic wrote:
> 
> 
> 
>         On Monday, October 21, 2019, Philippe Mathieu-Daudé
>         <philmd@redhat.com <mailto:philmd@redhat.com>
>         <mailto:philmd@redhat.com <mailto:philmd@redhat.com>>> wrote:
> 
>              Hi Aleksandar,
> 
>              On 10/21/19 8:25 AM, Aleksandar Markovic wrote:
> 
> 
> 
>                  On Monday, October 21, 2019, Philippe Mathieu-Daudé
>                  <philmd@redhat.com <mailto:philmd@redhat.com>
>         <mailto:philmd@redhat.com <mailto:philmd@redhat.com>>
>                  <mailto:philmd@redhat.com <mailto:philmd@redhat.com>
>         <mailto:philmd@redhat.com <mailto:philmd@redhat.com>>>> wrote:
> 
>                       Signed-off-by: Philippe Mathieu-Daudé
>         <philmd@redhat.com <mailto:philmd@redhat.com>
>                  <mailto:philmd@redhat.com <mailto:philmd@redhat.com>>
>                       <mailto:philmd@redhat.com
>         <mailto:philmd@redhat.com> <mailto:philmd@redhat.com
>         <mailto:philmd@redhat.com>>>>
>                       ---
>                         hw/mips/boston.c        | 2 +-
>                         hw/mips/mips_fulong2e.c | 3 ++-
>                         hw/mips/mips_jazz.c     | 2 +-
>                         hw/mips/mips_malta.c    | 2 +-
>                         hw/mips/mips_mipssim.c  | 2 +-
>                         hw/mips/mips_r4k.c      | 3 ++-
>                         6 files changed, 8 insertions(+), 6 deletions(-)
> 
> 
>                  Philippe, can this patch be applied independently (on other
>                  patches of this series)?
> 
> 
>              Unfortunately not because patch #9 changed the prototype of
>              memory_region_allocate_system_memory() so this patch alone
>              won't build.
> 
>              I rather expect this series goes as a whole via the
>         machine-next
>              tree.
> 
>              Do you mind giving your Acked-by tag to this patch?
> 
> 
>         I can't, sorry, for MIPS target, we don't accept patches with
>         empty commit messages.
> 
> 
>     I see, I'v been wondering what to do, if simply repeating the patch
>     subject or the cover, but since the series applies to all targets,
>     this seems overkill. I think I'll simply repeat the subject then.
> 
> 
> Can't you at least add a sentence on why is this needed, or what is 
> achieved?

You are alright, I'll do.

> 
> Thanks,
> 
> A.
> 
> 
>         A.
> 
> 
>              Thanks,
> 
>              Phil.
> 
> 
> 
>                       diff --git a/hw/mips/boston.c b/hw/mips/boston.c
>                       index ca7d813a52..8445fee0f1 100644
>                       --- a/hw/mips/boston.c
>                       +++ b/hw/mips/boston.c
>                       @@ -474,7 +474,7 @@ static void
>                  boston_mach_init(MachineState *machine)
>                             memory_region_add_subregion_overlap(sys_mem,
>         0x18000000,
>                       flash, 0);
> 
>                             ddr = g_new(MemoryRegion, 1);
>                       -    memory_region_allocate_system_memory(ddr, NULL,
>                  "boston.ddr",
>                       +    memory_region_allocate_system_memory(ddr,
>         machine,
>                  "boston.ddr",
>                                                                 
>         machine->ram_size);
>                             memory_region_add_subregion_overlap(sys_mem,
>                  0x80000000, ddr, 0);
> 
>                       diff --git a/hw/mips/mips_fulong2e.c
>         b/hw/mips/mips_fulong2e.c
>                       index cf537dd7e6..d5a5cef619 100644
>                       --- a/hw/mips/mips_fulong2e.c
>                       +++ b/hw/mips/mips_fulong2e.c
>                       @@ -318,7 +318,8 @@ static void
>         mips_fulong2e_init(MachineState
>                       *machine)
>                             ram_size = 256 * MiB;
> 
>                             /* allocate RAM */
>                       -    memory_region_allocate_system_memory(ram, NULL,
>                  "fulong2e.ram",
>                       ram_size);
>                       +    memory_region_allocate_system_memory(ram,
>         machine,
>                       +                                       
>           "fulong2e.ram",
>                  ram_size);
>                             memory_region_init_ram(bios, NULL,
>         "fulong2e.bios",
>                  BIOS_SIZE,
>                                                    &error_fatal);
>                             memory_region_set_readonly(bios, true);
>                       diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
>                       index 8d010a0b6e..88b125855f 100644
>                       --- a/hw/mips/mips_jazz.c
>                       +++ b/hw/mips/mips_jazz.c
>                       @@ -188,7 +188,7 @@ static void
>         mips_jazz_init(MachineState
>                  *machine,
>                             cc->do_transaction_failed =
>                  mips_jazz_do_transaction_failed;
> 
>                             /* allocate RAM */
>                       -    memory_region_allocate_system_memory(ram, NULL,
>                  "mips_jazz.ram",
>                       +    memory_region_allocate_system_memory(ram,
>         machine,
>                  "mips_jazz.ram",
>                                                                 
>         machine->ram_size);
>                             memory_region_add_subregion(address_space,
>         0, ram);
> 
>                       diff --git a/hw/mips/mips_malta.c
>         b/hw/mips/mips_malta.c
>                       index 4d9c64b36a..af56a29ccb 100644
>                       --- a/hw/mips/mips_malta.c
>                       +++ b/hw/mips/mips_malta.c
>                       @@ -1267,7 +1267,7 @@ void
>         mips_malta_init(MachineState
>                  *machine)
>                             }
> 
>                             /* register RAM at high address where it is
>                  undisturbed by IO */
>                       -   
>         memory_region_allocate_system_memory(ram_high, NULL,
>                       "mips_malta.ram",
>                       +   
>         memory_region_allocate_system_memory(ram_high, machine,
>                       "mips_malta.ram",
>                                                                  ram_size);
>                             memory_region_add_subregion(system_memory,
>                  0x80000000, ram_high);
> 
>                       diff --git a/hw/mips/mips_mipssim.c
>         b/hw/mips/mips_mipssim.c
>                       index 282bbecb24..c1933231e2 100644
>                       --- a/hw/mips/mips_mipssim.c
>                       +++ b/hw/mips/mips_mipssim.c
>                       @@ -166,7 +166,7 @@ mips_mipssim_init(MachineState
>         *machine)
>                             qemu_register_reset(main_cpu_reset, reset_info);
> 
>                             /* Allocate RAM. */
>                       -    memory_region_allocate_system_memory(ram, NULL,
>                  "mips_mipssim.ram",
>                       +    memory_region_allocate_system_memory(ram,
>         machine,
>                       "mips_mipssim.ram",
>                                                                  ram_size);
>                             memory_region_init_ram(bios, NULL,
>                  "mips_mipssim.bios", BIOS_SIZE,
>                                                    &error_fatal);
>                       diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
>                       index bc0be26544..59f8cacfb6 100644
>                       --- a/hw/mips/mips_r4k.c
>                       +++ b/hw/mips/mips_r4k.c
>                       @@ -203,7 +203,8 @@ void
>         mips_r4k_init(MachineState *machine)
>                                              " maximum 256MB", ram_size
>         / MiB);
>                                 exit(1);
>                             }
>                       -    memory_region_allocate_system_memory(ram, NULL,
>                  "mips_r4k.ram",
>                       ram_size);
>                       +    memory_region_allocate_system_memory(ram,
>         machine,
>                  "mips_r4k.ram",
>                       +                                         ram_size);
> 
>                           
>           memory_region_add_subregion(address_space_mem, 0, ram);
> 
>                       --     2.21.0
> 
> 

Re: [PATCH 18/21] hw/mips: Let the machine be the owner of the system memory

[Aleksandar Markovic]

[-- Attachment #1: Type: text/plain, Size: 9010 bytes --]

On Monday, October 21, 2019, Philippe Mathieu-Daudé <philmd@redhat.com>
wrote:

> On 10/21/19 1:10 PM, Aleksandar Markovic wrote:
>
>> On Monday, October 21, 2019, Philippe Mathieu-Daudé <philmd@redhat.com
>> <mailto:philmd@redhat.com>> wrote:
>>
>>     On 10/21/19 12:56 PM, Aleksandar Markovic wrote:
>>
>>
>>
>>         On Monday, October 21, 2019, Philippe Mathieu-Daudé
>>         <philmd@redhat.com <mailto:philmd@redhat.com>
>>         <mailto:philmd@redhat.com <mailto:philmd@redhat.com>>> wrote:
>>
>>              Hi Aleksandar,
>>
>>              On 10/21/19 8:25 AM, Aleksandar Markovic wrote:
>>
>>
>>
>>                  On Monday, October 21, 2019, Philippe Mathieu-Daudé
>>                  <philmd@redhat.com <mailto:philmd@redhat.com>
>>         <mailto:philmd@redhat.com <mailto:philmd@redhat.com>>
>>                  <mailto:philmd@redhat.com <mailto:philmd@redhat.com>
>>         <mailto:philmd@redhat.com <mailto:philmd@redhat.com>>>> wrote:
>>
>>                       Signed-off-by: Philippe Mathieu-Daudé
>>         <philmd@redhat.com <mailto:philmd@redhat.com>
>>                  <mailto:philmd@redhat.com <mailto:philmd@redhat.com>>
>>                       <mailto:philmd@redhat.com
>>         <mailto:philmd@redhat.com> <mailto:philmd@redhat.com
>>         <mailto:philmd@redhat.com>>>>
>>                       ---
>>                         hw/mips/boston.c        | 2 +-
>>                         hw/mips/mips_fulong2e.c | 3 ++-
>>                         hw/mips/mips_jazz.c     | 2 +-
>>                         hw/mips/mips_malta.c    | 2 +-
>>                         hw/mips/mips_mipssim.c  | 2 +-
>>                         hw/mips/mips_r4k.c      | 3 ++-
>>                         6 files changed, 8 insertions(+), 6 deletions(-)
>>
>>
>>                  Philippe, can this patch be applied independently (on
>> other
>>                  patches of this series)?
>>
>>
>>              Unfortunately not because patch #9 changed the prototype of
>>              memory_region_allocate_system_memory() so this patch alone
>>              won't build.
>>
>>              I rather expect this series goes as a whole via the
>>         machine-next
>>              tree.
>>
>>              Do you mind giving your Acked-by tag to this patch?
>>
>>
>>         I can't, sorry, for MIPS target, we don't accept patches with
>>         empty commit messages.
>>
>>
>>     I see, I'v been wondering what to do, if simply repeating the patch
>>     subject or the cover, but since the series applies to all targets,
>>     this seems overkill. I think I'll simply repeat the subject then.
>>
>>
>> Can't you at least add a sentence on why is this needed, or what is
>> achieved?
>>
>
> You are alright, I'll do.



Cool, I'd appreciate it very much.

A.




>
>
>> Thanks,
>>
>> A.
>>
>>
>>         A.
>>
>>
>>              Thanks,
>>
>>              Phil.
>>
>>
>>
>>                       diff --git a/hw/mips/boston.c b/hw/mips/boston.c
>>                       index ca7d813a52..8445fee0f1 100644
>>                       --- a/hw/mips/boston.c
>>                       +++ b/hw/mips/boston.c
>>                       @@ -474,7 +474,7 @@ static void
>>                  boston_mach_init(MachineState *machine)
>>                             memory_region_add_subregion_overlap(sys_mem,
>>         0x18000000,
>>                       flash, 0);
>>
>>                             ddr = g_new(MemoryRegion, 1);
>>                       -    memory_region_allocate_system_memory(ddr,
>> NULL,
>>                  "boston.ddr",
>>                       +    memory_region_allocate_system_memory(ddr,
>>         machine,
>>                  "boston.ddr",
>>
>> machine->ram_size);
>>                             memory_region_add_subregion_overlap(sys_mem,
>>                  0x80000000, ddr, 0);
>>
>>                       diff --git a/hw/mips/mips_fulong2e.c
>>         b/hw/mips/mips_fulong2e.c
>>                       index cf537dd7e6..d5a5cef619 100644
>>                       --- a/hw/mips/mips_fulong2e.c
>>                       +++ b/hw/mips/mips_fulong2e.c
>>                       @@ -318,7 +318,8 @@ static void
>>         mips_fulong2e_init(MachineState
>>                       *machine)
>>                             ram_size = 256 * MiB;
>>
>>                             /* allocate RAM */
>>                       -    memory_region_allocate_system_memory(ram,
>> NULL,
>>                  "fulong2e.ram",
>>                       ram_size);
>>                       +    memory_region_allocate_system_memory(ram,
>>         machine,
>>                       +
>>  "fulong2e.ram",
>>                  ram_size);
>>                             memory_region_init_ram(bios, NULL,
>>         "fulong2e.bios",
>>                  BIOS_SIZE,
>>                                                    &error_fatal);
>>                             memory_region_set_readonly(bios, true);
>>                       diff --git a/hw/mips/mips_jazz.c
>> b/hw/mips/mips_jazz.c
>>                       index 8d010a0b6e..88b125855f 100644
>>                       --- a/hw/mips/mips_jazz.c
>>                       +++ b/hw/mips/mips_jazz.c
>>                       @@ -188,7 +188,7 @@ static void
>>         mips_jazz_init(MachineState
>>                  *machine,
>>                             cc->do_transaction_failed =
>>                  mips_jazz_do_transaction_failed;
>>
>>                             /* allocate RAM */
>>                       -    memory_region_allocate_system_memory(ram,
>> NULL,
>>                  "mips_jazz.ram",
>>                       +    memory_region_allocate_system_memory(ram,
>>         machine,
>>                  "mips_jazz.ram",
>>
>> machine->ram_size);
>>                             memory_region_add_subregion(address_space,
>>         0, ram);
>>
>>                       diff --git a/hw/mips/mips_malta.c
>>         b/hw/mips/mips_malta.c
>>                       index 4d9c64b36a..af56a29ccb 100644
>>                       --- a/hw/mips/mips_malta.c
>>                       +++ b/hw/mips/mips_malta.c
>>                       @@ -1267,7 +1267,7 @@ void
>>         mips_malta_init(MachineState
>>                  *machine)
>>                             }
>>
>>                             /* register RAM at high address where it is
>>                  undisturbed by IO */
>>                       -           memory_region_allocate_system_memory(ram_high,
>> NULL,
>>                       "mips_malta.ram",
>>                       +           memory_region_allocate_system_memory(ram_high,
>> machine,
>>                       "mips_malta.ram",
>>
>> ram_size);
>>                             memory_region_add_subregion(system_memory,
>>                  0x80000000, ram_high);
>>
>>                       diff --git a/hw/mips/mips_mipssim.c
>>         b/hw/mips/mips_mipssim.c
>>                       index 282bbecb24..c1933231e2 100644
>>                       --- a/hw/mips/mips_mipssim.c
>>                       +++ b/hw/mips/mips_mipssim.c
>>                       @@ -166,7 +166,7 @@ mips_mipssim_init(MachineState
>>         *machine)
>>                             qemu_register_reset(main_cpu_reset,
>> reset_info);
>>
>>                             /* Allocate RAM. */
>>                       -    memory_region_allocate_system_memory(ram,
>> NULL,
>>                  "mips_mipssim.ram",
>>                       +    memory_region_allocate_system_memory(ram,
>>         machine,
>>                       "mips_mipssim.ram",
>>
>> ram_size);
>>                             memory_region_init_ram(bios, NULL,
>>                  "mips_mipssim.bios", BIOS_SIZE,
>>                                                    &error_fatal);
>>                       diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
>>                       index bc0be26544..59f8cacfb6 100644
>>                       --- a/hw/mips/mips_r4k.c
>>                       +++ b/hw/mips/mips_r4k.c
>>                       @@ -203,7 +203,8 @@ void
>>         mips_r4k_init(MachineState *machine)
>>                                              " maximum 256MB", ram_size
>>         / MiB);
>>                                 exit(1);
>>                             }
>>                       -    memory_region_allocate_system_memory(ram,
>> NULL,
>>                  "mips_r4k.ram",
>>                       ram_size);
>>                       +    memory_region_allocate_system_memory(ram,
>>         machine,
>>                  "mips_r4k.ram",
>>                       +                                         ram_size);
>>
>>                                     memory_region_add_subregion(address_space_mem,
>> 0, ram);
>>
>>                       --     2.21.0
>>
>>
>>

[-- Attachment #2: Type: text/html, Size: 13554 bytes --]

Re: [PATCH 00/21] hw: Let the machine be the owner of the system memory

[Philippe Mathieu-Daudé]

Hi Peter,

On 10/21/19 10:52 AM, Philippe Mathieu-Daudé wrote:
> On 10/21/19 12:56 AM, Philippe Mathieu-Daudé wrote:
>> Hi,
>>
>> This series is based on Igor's "eliminate remaining places that
>> abuse memory_region_allocate_system_memory()":
>> https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg01601.html
>>
>> It is quite simple, we enforce all machines to be the QOM owner
>> of the system memory.
>>
>> This changes the memory tree from:
>>
>>    (qemu) info mtree -o
>>    memory-region: pc.ram
>>      0000000000000000-0000000007ffffff (prio 0, ram): pc.ram 
>> parent:{obj path=/machine/unattached}
>>
>> to:
>>
>>    (qemu) info mtree -o
>>    memory-region: pc.ram
>>      0000000000000000-0000000007ffffff (prio 0, ram): pc.ram 
>> owner:{obj path=/machine}
>>
>> Few patches are required to clean the codebase first, to unify the
>> creation of the system memory in the board/machine code. Mostly some
>> old ARM machines (pre-QOM) were affected.
>>
>> Please review (as a generic codebase cleanup).
>>
>> Regards,
>>
>> Phil.
>>
>> Based-on: <20191008113318.7012-1-imammedo@redhat.com>
>>
>> Philippe Mathieu-Daudé (21):
>>    hw/arm/xilinx_zynq: Use the IEC binary prefix definitions
>>    hw/arm/mps2: Use the IEC binary prefix definitions
>>    hw/arm/collie: Create the RAM in the board
>>    hw/arm/omap2: Create the RAM in the board
>>    hw/arm/omap1: Create the RAM in the board
>>    hw/arm/digic4: Inline digic4_board_setup_ram() function
>>    hw: Drop QOM ownership on memory_region_allocate_system_memory() calls
>>    hw/alpha/dp264: Create the RAM in the board
>>    hw: Let memory_region_allocate_system_memory take MachineState
>>      argument
>>    hw/core: Let the machine be the owner of the system memory
>>    hw/alpha: Let the machine be the owner of the system memory
>>    hw/arm: Let the machine be the owner of the system memory
>>    hw/cris: Let the machine be the owner of the system memory
>>    hw/hppa: Let the machine be the owner of the system memory
>>    hw/i386: Let the machine be the owner of the system memory
>>    hw/lm32: Let the machine be the owner of the system memory
>>    hw/m68k: Let the machine be the owner of the system memory
>>    hw/mips: Let the machine be the owner of the system memory
>>    hw/ppc: Let the machine be the owner of the system memory
>>    hw/sparc: Let the machine be the owner of the system memory
>>    hw/core: Assert memory_region_allocate_system_memory has machine owner
> 
> I forgot 4 other calls:
> 
> hw/ppc/ppc4xx_devs.c:708:    memory_region_allocate_system_memory(ram, 
> NULL, "ppc4xx.sdram", ram_size);
> hw/s390x/s390-virtio-ccw.c:164: 
> memory_region_allocate_system_memory(ram, NULL, "s390.ram", mem_size);
> hw/sparc/sun4m.c:791:    memory_region_allocate_system_memory(&d->ram, 
> NULL, "sun4m.ram",
> hw/sparc64/niagara.c:114: 
> memory_region_allocate_system_memory(&s->partition_ram, NULL,

I'll need to respin this series because of this omission, but
- hw/ppc requires more cleanup patches as ARM has,
- hw/sparc is blocked until Igor sent his "convert
   memory_region_allocate_system_memory() to memdev" series.

I don't plan to rework on the ARM cleanups patches (1-6):

   hw/arm/xilinx_zynq: Use the IEC binary prefix definitions
   hw/arm/mps2: Use the IEC binary prefix definitions
   hw/arm/collie: Create the RAM in the board
   hw/arm/omap2: Create the RAM in the board
   hw/arm/omap1: Create the RAM in the board
   hw/arm/digic4: Inline digic4_board_setup_ram() function

Maybe you can take them directly in arm-next once they are reviewed,
then the rest can go via machine-next.

Thanks,

Phil.

Re: [PATCH 12/21] hw/arm: Let the machine be the owner of the system memory

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> Cc'ing Paolo/David.
> 
> On 10/21/19 11:39 AM, Peter Maydell wrote:
> > On Mon, 21 Oct 2019 at 10:34, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> > > 
> > > On 10/21/19 11:22 AM, Peter Maydell wrote:
> > > > On Mon, 21 Oct 2019 at 00:01, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> > > > > 
> > > > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > > > > ---
> > > > 
> > > > >    hw/arm/virt.c             | 2 +-
> > > > 
> > > > I think from a quick code scan that this is ok, but did
> > > > you test that migration compat from old to new still works?
> > > > I vaguely recall that there are some cases when adding an
> > > > owner to a memory region changes the name string used for
> > > > identifying the ramblock in migration.
> > > 
> > > It seems to still works:
> > > 
> > > $ make check-qtest-aarch64 V=1
> > 
> > > This test migrate the virt machine.
> > > 
> > > Is this enough?
> > 
> > No, you need to test migration from a QEMU binary without
> > this patchset to a QEMU binary that has it. Otherwise you're
> > only checking that the new version can migrate from itself
> > to itself. I find the easiest way to test this is just to
> > use the 'savevm' command to save a state snapshot to a
> > qcow2 disk image while running the old binary, and then run
> > 'loadvm' with the new binary and check it restored OK.
> 
> I did not think if this case.
> 
> I followed your blog post [*] and tested the migration works OK.
> 
> Paolo, now thinking about regular testing, we should add this testing to
> patchew too. Something like:
> 
> - when mainstream/master is updated, patchew build QEMU (it should be
> already mostly ccached) and generate some vm dumps with 'savevm'.
> 
> - build/test the series
> 
> - if series succeeded testing, run 'loadvm' tests
> 
> [*] https://translatedcode.wordpress.com/2015/07/06/tricks-for-debugging-qemu-savevm-snapshots/

Avocado certainly already has an option for specifying source and
destination qemu separately; I've used that for testing
cross version in the past.

The challenge is finding a command line/set of devices for each
architecture that's expected to be stable.
You want a command line with as big a set of devices as possible (for
coverage) yet really is tied to machine type.

Dave

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH 01/21] hw/arm/xilinx_zynq: Use the IEC binary prefix definitions

[Richard Henderson]

On 10/20/19 3:56 PM, Philippe Mathieu-Daudé wrote:
> IEC binary prefixes ease code review: the unit is explicit.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/arm/xilinx_zynq.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 02/21] hw/arm/mps2: Use the IEC binary prefix definitions

[Richard Henderson]

On 10/20/19 3:56 PM, Philippe Mathieu-Daudé wrote:
> IEC binary prefixes ease code review: the unit is explicit.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/arm/mps2-tz.c | 3 ++-
>  hw/arm/mps2.c    | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 03/21] hw/arm/collie: Create the RAM in the board

[Richard Henderson]

On 10/20/19 3:56 PM, Philippe Mathieu-Daudé wrote:
> The SDRAM is incorrectly created in the SA1110 SoC.
> Move its creation in the board code, this will later allow the
> board to have the QOM ownership of the RAM.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/arm/collie.c    | 8 ++++++--
>  hw/arm/strongarm.c | 7 +------
>  hw/arm/strongarm.h | 4 +---
>  3 files changed, 8 insertions(+), 11 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 04/21] hw/arm/omap2: Create the RAM in the board

[Richard Henderson]

On 10/20/19 3:56 PM, Philippe Mathieu-Daudé wrote:
>  static void n8x0_init(MachineState *machine,
>                        struct arm_boot_info *binfo, int model)
>  {
> -    MemoryRegion *sysmem = get_system_memory();
> +    MemoryRegion *sdram = g_new(MemoryRegion, 1);
>      struct n800_s *s = (struct n800_s *) g_malloc0(sizeof(*s));
> -    int sdram_size = binfo->ram_size;
> +    uint64_t sdram_size = binfo->ram_size;
>  
> -    s->mpu = omap2420_mpu_init(sysmem, sdram_size, machine->cpu_type);
> +    memory_region_allocate_system_memory(sdram, NULL, "omap2.dram",
> +                                         sdram_size);
> +    memory_region_add_subregion(get_system_memory(), OMAP2_Q2_BASE, sdram);
> +
> +    s->mpu = omap2420_mpu_init(sdram, machine->cpu_type);

Any reason not to put the MemoryRegion in to the n800_s structure?

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 05/21] hw/arm/omap1: Create the RAM in the board

[Richard Henderson]

On 10/20/19 3:56 PM, Philippe Mathieu-Daudé wrote:
> The SDRAM is incorrectly created in the OMAP310 SoC.
> Move its creation in the board code, this will later allow the
> board to have the QOM ownership of the RAM.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/arm/omap1.c        | 12 +++++-------
>  hw/arm/omap_sx1.c     |  8 ++++++--
>  hw/arm/palm.c         |  8 ++++++--
>  include/hw/arm/omap.h |  6 ++----
>  4 files changed, 19 insertions(+), 15 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 06/21] hw/arm/digic4: Inline digic4_board_setup_ram() function

[Richard Henderson]

On 10/20/19 3:56 PM, Philippe Mathieu-Daudé wrote:
> Having the RAM creation code in a separate function is not
> very helpful. Move this code directly inside the board_init()
> function, this will later allow the board to have the QOM
> ownership of the RAM.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/arm/digic_boards.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 11/21] hw/alpha: Let the machine be the owner of the system memory

[Richard Henderson]

On 10/20/19 3:56 PM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/alpha/dp264.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
> index ddc249261c..5696b1e79b 100644
> --- a/hw/alpha/dp264.c
> +++ b/hw/alpha/dp264.c
> @@ -78,7 +78,7 @@ static void clipper_init(MachineState *machine)
>       * Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
>       * but the address space hole reserved at this point is 8TB.
>       */
> -    memory_region_allocate_system_memory(&ram_region, NULL, "ram",
> +    memory_region_allocate_system_memory(&ram_region, machine, "ram",
>                                           ram_size);
>      memory_region_add_subregion(get_system_memory(), 0, &ram_region);
>  
> 

I take it this squishes into the squish of patches 7+9?


r~

Re: [PATCH 04/21] hw/arm/omap2: Create the RAM in the board

[Philippe Mathieu-Daudé]

On 10/21/19 6:59 PM, Richard Henderson wrote:
> On 10/20/19 3:56 PM, Philippe Mathieu-Daudé wrote:
>>   static void n8x0_init(MachineState *machine,
>>                         struct arm_boot_info *binfo, int model)
>>   {
>> -    MemoryRegion *sysmem = get_system_memory();
>> +    MemoryRegion *sdram = g_new(MemoryRegion, 1);
>>       struct n800_s *s = (struct n800_s *) g_malloc0(sizeof(*s));
>> -    int sdram_size = binfo->ram_size;
>> +    uint64_t sdram_size = binfo->ram_size;
>>   
>> -    s->mpu = omap2420_mpu_init(sysmem, sdram_size, machine->cpu_type);
>> +    memory_region_allocate_system_memory(sdram, NULL, "omap2.dram",
>> +                                         sdram_size);
>> +    memory_region_add_subregion(get_system_memory(), OMAP2_Q2_BASE, sdram);
>> +
>> +    s->mpu = omap2420_mpu_init(sdram, machine->cpu_type);
> 
> Any reason not to put the MemoryRegion in to the n800_s structure?

Ah yes, better to keep it in n800_s in case we ever QOM'ify this SoC.
I'll do.

> Otherwise,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Thanks :)

Re: [PATCH 11/21] hw/alpha: Let the machine be the owner of the system memory

[Richard Henderson]

On 10/21/19 10:14 AM, Richard Henderson wrote:
> On 10/20/19 3:56 PM, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  hw/alpha/dp264.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
>> index ddc249261c..5696b1e79b 100644
>> --- a/hw/alpha/dp264.c
>> +++ b/hw/alpha/dp264.c
>> @@ -78,7 +78,7 @@ static void clipper_init(MachineState *machine)
>>       * Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
>>       * but the address space hole reserved at this point is 8TB.
>>       */
>> -    memory_region_allocate_system_memory(&ram_region, NULL, "ram",
>> +    memory_region_allocate_system_memory(&ram_region, machine, "ram",
>>                                           ram_size);
>>      memory_region_add_subregion(get_system_memory(), 0, &ram_region);
>>  
>>
> 
> I take it this squishes into the squish of patches 7+9?

Ah, no, it was the TyphoonState that had owned the ram prior to patch 7.


r~

Re: [PATCH 11/21] hw/alpha: Let the machine be the owner of the system memory

[Philippe Mathieu-Daudé]

On 10/21/19 7:19 PM, Richard Henderson wrote:
> On 10/21/19 10:14 AM, Richard Henderson wrote:
>> On 10/20/19 3:56 PM, Philippe Mathieu-Daudé wrote:
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> ---
>>>   hw/alpha/dp264.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
>>> index ddc249261c..5696b1e79b 100644
>>> --- a/hw/alpha/dp264.c
>>> +++ b/hw/alpha/dp264.c
>>> @@ -78,7 +78,7 @@ static void clipper_init(MachineState *machine)
>>>        * Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
>>>        * but the address space hole reserved at this point is 8TB.
>>>        */
>>> -    memory_region_allocate_system_memory(&ram_region, NULL, "ram",
>>> +    memory_region_allocate_system_memory(&ram_region, machine, "ram",
>>>                                            ram_size);
>>>       memory_region_add_subregion(get_system_memory(), 0, &ram_region);
>>>   
>>>
>>
>> I take it this squishes into the squish of patches 7+9?
> 
> Ah, no, it was the TyphoonState that had owned the ram prior to patch 7.

Indeed, I'll squash them and document in the commit description that I 
dropped Typhoon ownership. I'm not sure what's best here, keep it 
unassigned until patch #11 (this), or reassign to OBJECT(machine).

New squashed patch is:

-- 8< --
diff --git a/hw/alpha/alpha_sys.h b/hw/alpha/alpha_sys.h
index 4e127a6de8..17f97e1bfe 100644
--- a/hw/alpha/alpha_sys.h
+++ b/hw/alpha/alpha_sys.h
@@ -10,7 +10,7 @@
  #include "hw/i386/pc.h"


-PCIBus *typhoon_init(ram_addr_t, ISABus **, qemu_irq *, AlphaCPU *[4],
+PCIBus *typhoon_init(ISABus **, qemu_irq *, AlphaCPU *[4],
                       pci_map_irq_fn);

  /* alpha_pci.c.  */
diff --git a/hw/alpha/dp264.c b/hw/alpha/dp264.c
index 51feee8558..a93a476531 100644
--- a/hw/alpha/dp264.c
+++ b/hw/alpha/dp264.c
@@ -62,6 +62,7 @@ static void clipper_init(MachineState *machine)
      uint64_t palcode_entry, palcode_low, palcode_high;
      uint64_t kernel_entry, kernel_low, kernel_high;
      unsigned int smp_cpus = machine->smp.cpus;
+    MemoryRegion *ram = g_new(MemoryRegion, 1);

      /* Create up to 4 cpus.  */
      memset(cpus, 0, sizeof(cpus));
@@ -73,8 +74,15 @@ static void clipper_init(MachineState *machine)
      cpus[0]->env.trap_arg1 = 0;
      cpus[0]->env.trap_arg2 = smp_cpus;

+    /*
+     * Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
+     * but the address space hole reserved at this point is 8TB.
+     */
+    memory_region_allocate_system_memory(ram, OBJECT(machine), "ram", 
ram_size);
+    memory_region_add_subregion(get_system_memory(), 0, ram);
+
      /* Init the chipset.  */
-    pci_bus = typhoon_init(ram_size, &isa_bus, &rtc_irq, cpus,
+    pci_bus = typhoon_init(&isa_bus, &rtc_irq, cpus,
                             clipper_pci_map_irq);

      /* Since we have an SRM-compatible PALcode, use the SRM epoch.  */
diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
index 179e1f7658..12589ef331 100644
--- a/hw/alpha/typhoon.c
+++ b/hw/alpha/typhoon.c
@@ -58,7 +58,6 @@ typedef struct TyphoonState {
      TyphoonCchip cchip;
      TyphoonPchip pchip;
      MemoryRegion dchip_region;
-    MemoryRegion ram_region;
  } TyphoonState;

  /* Called when one of DRIR or DIM changes.  */
@@ -817,7 +816,7 @@ static void typhoon_alarm_timer(void *opaque)
      cpu_interrupt(CPU(s->cchip.cpu[cpu]), CPU_INTERRUPT_TIMER);
  }

-PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
+PCIBus *typhoon_init(ISABus **isa_bus,
                       qemu_irq *p_rtc_irq,
                       AlphaCPU *cpus[4], pci_map_irq_fn sys_map_irq)
  {
@@ -849,12 +848,6 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus 
**isa_bus,

      *p_rtc_irq = qemu_allocate_irq(typhoon_set_timer_irq, s, 0);

-    /* Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
-       but the address space hole reserved at this point is 8TB.  */
-    memory_region_allocate_system_memory(&s->ram_region, OBJECT(s), "ram",
-                                         ram_size);
-    memory_region_add_subregion(addr_space, 0, &s->ram_region);
-
      /* TIGbus, 0x801.0000.0000, 1GB.  */
      /* ??? The TIGbus is used for delivering interrupts, and access to
         the flash ROM.  I'm not sure that we need to implement it at 
all.  */
---

Re: [PATCH 08/21] hw/alpha/dp264: Create the RAM in the board

[Richard Henderson]

On 10/20/19 3:56 PM, Philippe Mathieu-Daudé wrote:
> @@ -73,8 +74,16 @@ static void clipper_init(MachineState *machine)
>      cpus[0]->env.trap_arg1 = 0;
>      cpus[0]->env.trap_arg2 = smp_cpus;
>  
> +    /*
> +     * Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
> +     * but the address space hole reserved at this point is 8TB.
> +     */
> +    memory_region_allocate_system_memory(&ram_region, NULL, "ram",
> +                                         ram_size);
> +    memory_region_add_subregion(get_system_memory(), 0, &ram_region);

The thing that I don't like about this is that ...

> @@ -849,12 +848,6 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
>  
>      *p_rtc_irq = qemu_allocate_irq(typhoon_set_timer_irq, s, 0);
>  
> -    /* Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
> -       but the address space hole reserved at this point is 8TB.  */
> -    memory_region_allocate_system_memory(&s->ram_region, NULL, "ram",
> -                                         ram_size);
> -    memory_region_add_subregion(addr_space, 0, &s->ram_region);
> -
>      /* TIGbus, 0x801.0000.0000, 1GB.  */
>      /* ??? The TIGbus is used for delivering interrupts, and access to
>         the flash ROM.  I'm not sure that we need to implement it at all.  */

... previously, it was clear that Typhoon owns the entire address space layout.
 With this moved to dp264.c, the "address space hole" comment is out of context
and makes no sense.

Would it be too weird to pass in a MemoryRegion* to typhoon_init and leave the
memory_region_add_subregion() call where it is?


r~

Re: [PATCH 13/21] hw/cris: Let the machine be the owner of the system memory

[Richard Henderson]

On 10/20/19 3:56 PM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/cris/axis_dev88.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 14/21] hw/hppa: Let the machine be the owner of the system memory

[Richard Henderson]

On 10/20/19 3:56 PM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/hppa/machine.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 15/21] hw/i386: Let the machine be the owner of the system memory

[Richard Henderson]

On 10/20/19 3:56 PM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/i386/pc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 16/21] hw/lm32: Let the machine be the owner of the system memory

[Richard Henderson]

On 10/20/19 3:56 PM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/lm32/lm32_boards.c | 4 ++--
>  hw/lm32/milkymist.c   | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 17/21] hw/m68k: Let the machine be the owner of the system memory

[Richard Henderson]

On 10/20/19 3:56 PM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/m68k/an5206.c    | 2 +-
>  hw/m68k/mcf5208.c   | 2 +-
>  hw/m68k/next-cube.c | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 19/21] hw/ppc: Let the machine be the owner of the system memory

[Richard Henderson]

On 10/20/19 3:56 PM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/ppc/e500.c          | 3 ++-
>  hw/ppc/mac_newworld.c  | 3 ++-
>  hw/ppc/mac_oldworld.c  | 2 +-
>  hw/ppc/pnv.c           | 2 +-
>  hw/ppc/ppc405_boards.c | 6 +++---
>  hw/ppc/prep.c          | 3 ++-
>  hw/ppc/spapr.c         | 2 +-
>  hw/ppc/virtex_ml507.c  | 2 +-
>  8 files changed, 13 insertions(+), 10 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 20/21] hw/sparc: Let the machine be the owner of the system memory

[Richard Henderson]

On 10/20/19 3:56 PM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/sparc/leon3.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH 08/21] hw/alpha/dp264: Create the RAM in the board

[Philippe Mathieu-Daudé]

On 10/21/19 7:27 PM, Richard Henderson wrote:
> On 10/20/19 3:56 PM, Philippe Mathieu-Daudé wrote:
>> @@ -73,8 +74,16 @@ static void clipper_init(MachineState *machine)
>>       cpus[0]->env.trap_arg1 = 0;
>>       cpus[0]->env.trap_arg2 = smp_cpus;
>>   
>> +    /*
>> +     * Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
>> +     * but the address space hole reserved at this point is 8TB.
>> +     */
>> +    memory_region_allocate_system_memory(&ram_region, NULL, "ram",
>> +                                         ram_size);
>> +    memory_region_add_subregion(get_system_memory(), 0, &ram_region);
> 
> The thing that I don't like about this is that ...
> 
>> @@ -849,12 +848,6 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
>>   
>>       *p_rtc_irq = qemu_allocate_irq(typhoon_set_timer_irq, s, 0);
>>   
>> -    /* Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
>> -       but the address space hole reserved at this point is 8TB.  */
>> -    memory_region_allocate_system_memory(&s->ram_region, NULL, "ram",
>> -                                         ram_size);
>> -    memory_region_add_subregion(addr_space, 0, &s->ram_region);
>> -
>>       /* TIGbus, 0x801.0000.0000, 1GB.  */
>>       /* ??? The TIGbus is used for delivering interrupts, and access to
>>          the flash ROM.  I'm not sure that we need to implement it at all.  */
> 
> ... previously, it was clear that Typhoon owns the entire address space layout.
>   With this moved to dp264.c, the "address space hole" comment is out of context
> and makes no sense.
> 
> Would it be too weird to pass in a MemoryRegion* to typhoon_init and leave the
> memory_region_add_subregion() call where it is?

Yes, this will also better match the hardware.
I have a WiP series that first QOM'ify the CY82C693UB southbridge then 
the Typhoon northbridge and pass the DRAM MemoryRegion* as a link, so in 
the non-QOM form your suggestion is the same. I'll do your change.

Re: [PATCH 21/21] hw/core: Assert memory_region_allocate_system_memory has machine owner

[Richard Henderson]

On 10/20/19 3:56 PM, Philippe Mathieu-Daudé wrote:
> All the memory_region_allocate_system_memory() pass a MachineState
> argument. Add an assertion to ensure the new boards/machines added
> set this argument, so all system memory object have the machine as
> its QOM owner.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/core/numa.c      | 4 +---
>  include/hw/boards.h | 2 +-
>  2 files changed, 2 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

It would be a good idea to add __attribute__((nonnull(x,y,z))) as well, but
since we don't currently have any markup for that in qemu, that can go in a
separate patch set.


r~

Re: [EXTERNAL][PATCH 18/21] hw/mips: Let the machine be the owner of the system memory

[Aleksandar Markovic]

> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/mips/boston.c        | 2 +-
>  hw/mips/mips_fulong2e.c | 3 ++-
>  hw/mips/mips_jazz.c     | 2 +-
>  hw/mips/mips_malta.c    | 2 +-
>  hw/mips/mips_mipssim.c  | 2 +-
>  hw/mips/mips_r4k.c      | 3 ++-
>  6 files changed, 8 insertions(+), 6 deletions(-)

When the commit message is completed:

Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>

________________________________________
From: Philippe Mathieu-Daudé <philmd@redhat.com>
Sent: Monday, October 21, 2019 12:56 AM
To: Paolo Bonzini; Igor Mammedov; qemu-devel@nongnu.org; Markus Armbruster; Eduardo Habkost
Cc: Thomas Huth; Antony Pavlov; Michael Walle; David Gibson; qemu-arm@nongnu.org; KONRAD Frederic; Cédric Le Goater; Fabien Chouteau; Andrey Smirnov; Joel Stanley; Peter Chubb; Andrew Jeffery; Helge Deller; Aurelien Jarno; Jean-Christophe Dubois; Jan Kiszka; Paul Burton; qemu-ppc@nongnu.org; Hervé Poussineau; Andrew Baumann; Mark Cave-Ayland; Beniamino Galvani; Artyom Tarasenko; Edgar E. Iglesias; Leif Lindholm; Aleksandar Markovic; Michael S. Tsirkin; Radoslaw Biernacki; Rob Herring; Richard Henderson; Alistair Francis; Andrzej Zaborowski; Marcel Apfelbaum; Philippe Mathieu-Daudé; Aleksandar Rikalo; Peter Maydell
Subject: [EXTERNAL][PATCH 18/21] hw/mips: Let the machine be the owner of the system memory

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/mips/boston.c        | 2 +-
 hw/mips/mips_fulong2e.c | 3 ++-
 hw/mips/mips_jazz.c     | 2 +-
 hw/mips/mips_malta.c    | 2 +-
 hw/mips/mips_mipssim.c  | 2 +-
 hw/mips/mips_r4k.c      | 3 ++-
 6 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/hw/mips/boston.c b/hw/mips/boston.c
index ca7d813a52..8445fee0f1 100644
--- a/hw/mips/boston.c
+++ b/hw/mips/boston.c
@@ -474,7 +474,7 @@ static void boston_mach_init(MachineState *machine)
     memory_region_add_subregion_overlap(sys_mem, 0x18000000, flash, 0);

     ddr = g_new(MemoryRegion, 1);
-    memory_region_allocate_system_memory(ddr, NULL, "boston.ddr",
+    memory_region_allocate_system_memory(ddr, machine, "boston.ddr",
                                          machine->ram_size);
     memory_region_add_subregion_overlap(sys_mem, 0x80000000, ddr, 0);

diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/mips_fulong2e.c
index cf537dd7e6..d5a5cef619 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/mips_fulong2e.c
@@ -318,7 +318,8 @@ static void mips_fulong2e_init(MachineState *machine)
     ram_size = 256 * MiB;

     /* allocate RAM */
-    memory_region_allocate_system_memory(ram, NULL, "fulong2e.ram", ram_size);
+    memory_region_allocate_system_memory(ram, machine,
+                                         "fulong2e.ram", ram_size);
     memory_region_init_ram(bios, NULL, "fulong2e.bios", BIOS_SIZE,
                            &error_fatal);
     memory_region_set_readonly(bios, true);
diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index 8d010a0b6e..88b125855f 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -188,7 +188,7 @@ static void mips_jazz_init(MachineState *machine,
     cc->do_transaction_failed = mips_jazz_do_transaction_failed;

     /* allocate RAM */
-    memory_region_allocate_system_memory(ram, NULL, "mips_jazz.ram",
+    memory_region_allocate_system_memory(ram, machine, "mips_jazz.ram",
                                          machine->ram_size);
     memory_region_add_subregion(address_space, 0, ram);

diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 4d9c64b36a..af56a29ccb 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -1267,7 +1267,7 @@ void mips_malta_init(MachineState *machine)
     }

     /* register RAM at high address where it is undisturbed by IO */
-    memory_region_allocate_system_memory(ram_high, NULL, "mips_malta.ram",
+    memory_region_allocate_system_memory(ram_high, machine, "mips_malta.ram",
                                          ram_size);
     memory_region_add_subregion(system_memory, 0x80000000, ram_high);

diff --git a/hw/mips/mips_mipssim.c b/hw/mips/mips_mipssim.c
index 282bbecb24..c1933231e2 100644
--- a/hw/mips/mips_mipssim.c
+++ b/hw/mips/mips_mipssim.c
@@ -166,7 +166,7 @@ mips_mipssim_init(MachineState *machine)
     qemu_register_reset(main_cpu_reset, reset_info);

     /* Allocate RAM. */
-    memory_region_allocate_system_memory(ram, NULL, "mips_mipssim.ram",
+    memory_region_allocate_system_memory(ram, machine, "mips_mipssim.ram",
                                          ram_size);
     memory_region_init_ram(bios, NULL, "mips_mipssim.bios", BIOS_SIZE,
                            &error_fatal);
diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
index bc0be26544..59f8cacfb6 100644
--- a/hw/mips/mips_r4k.c
+++ b/hw/mips/mips_r4k.c
@@ -203,7 +203,8 @@ void mips_r4k_init(MachineState *machine)
                      " maximum 256MB", ram_size / MiB);
         exit(1);
     }
-    memory_region_allocate_system_memory(ram, NULL, "mips_r4k.ram", ram_size);
+    memory_region_allocate_system_memory(ram, machine, "mips_r4k.ram",
+                                         ram_size);

     memory_region_add_subregion(address_space_mem, 0, ram);

--
2.21.0

Re: [PATCH 01/21] hw/arm/xilinx_zynq: Use the IEC binary prefix definitions

[Alistair Francis]

On Sun, Oct 20, 2019 at 3:58 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> IEC binary prefixes ease code review: the unit is explicit.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/arm/xilinx_zynq.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> index c14774e542..3a0fa5b23f 100644
> --- a/hw/arm/xilinx_zynq.c
> +++ b/hw/arm/xilinx_zynq.c
> @@ -16,6 +16,7 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "qemu/units.h"
>  #include "qapi/error.h"
>  #include "cpu.h"
>  #include "hw/sysbus.h"
> @@ -194,7 +195,7 @@ static void zynq_init(MachineState *machine)
>      memory_region_add_subregion(address_space_mem, 0, ext_ram);
>
>      /* 256K of on-chip memory */
> -    memory_region_init_ram(ocm_ram, NULL, "zynq.ocm_ram", 256 << 10,
> +    memory_region_init_ram(ocm_ram, NULL, "zynq.ocm_ram", 256 * KiB,
>                             &error_fatal);
>      memory_region_add_subregion(address_space_mem, 0xFFFC0000, ocm_ram);
>
> --
> 2.21.0
>
>

Re: [PATCH 02/21] hw/arm/mps2: Use the IEC binary prefix definitions

[Alistair Francis]

On Sun, Oct 20, 2019 at 4:02 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> IEC binary prefixes ease code review: the unit is explicit.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/arm/mps2-tz.c | 3 ++-
>  hw/arm/mps2.c    | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
> index 6b24aaacde..f8b620bcc6 100644
> --- a/hw/arm/mps2-tz.c
> +++ b/hw/arm/mps2-tz.c
> @@ -38,6 +38,7 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "qemu/units.h"
>  #include "qapi/error.h"
>  #include "qemu/error-report.h"
>  #include "hw/arm/boot.h"
> @@ -458,7 +459,7 @@ static void mps2tz_common_init(MachineState *machine)
>       * call the 16MB our "system memory", as it's the largest lump.
>       */
>      memory_region_allocate_system_memory(&mms->psram,
> -                                         NULL, "mps.ram", 0x01000000);
> +                                         NULL, "mps.ram", 16 * MiB);
>      memory_region_add_subregion(system_memory, 0x80000000, &mms->psram);
>
>      /* The overflow IRQs for all UARTs are ORed together.
> diff --git a/hw/arm/mps2.c b/hw/arm/mps2.c
> index 10efff36b2..d002b126d3 100644
> --- a/hw/arm/mps2.c
> +++ b/hw/arm/mps2.c
> @@ -23,6 +23,7 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "qemu/units.h"
>  #include "qapi/error.h"
>  #include "qemu/error-report.h"
>  #include "hw/arm/boot.h"
> @@ -146,7 +147,7 @@ static void mps2_common_init(MachineState *machine)
>       * zbt_boot_ctrl is always zero).
>       */
>      memory_region_allocate_system_memory(&mms->psram,
> -                                         NULL, "mps.ram", 0x1000000);
> +                                         NULL, "mps.ram", 16 * MiB);
>      memory_region_add_subregion(system_memory, 0x21000000, &mms->psram);
>
>      switch (mmc->fpga_type) {
> --
> 2.21.0
>
>

Re: [PATCH 03/21] hw/arm/collie: Create the RAM in the board

[Alistair Francis]

On Sun, Oct 20, 2019 at 3:59 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> The SDRAM is incorrectly created in the SA1110 SoC.
> Move its creation in the board code, this will later allow the
> board to have the QOM ownership of the RAM.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/arm/collie.c    | 8 ++++++--
>  hw/arm/strongarm.c | 7 +------
>  hw/arm/strongarm.h | 4 +---
>  3 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/hw/arm/collie.c b/hw/arm/collie.c
> index b1288ccea8..970a4405cc 100644
> --- a/hw/arm/collie.c
> +++ b/hw/arm/collie.c
> @@ -27,9 +27,13 @@ static void collie_init(MachineState *machine)
>  {
>      StrongARMState *s;
>      DriveInfo *dinfo;
> -    MemoryRegion *sysmem = get_system_memory();
> +    MemoryRegion *sdram = g_new(MemoryRegion, 1);
>
> -    s = sa1110_init(sysmem, collie_binfo.ram_size, machine->cpu_type);
> +    s = sa1110_init(machine->cpu_type);
> +
> +    memory_region_allocate_system_memory(sdram, NULL, "strongarm.sdram",
> +                                         collie_binfo.ram_size);
> +    memory_region_add_subregion(get_system_memory(), SA_SDCS0, sdram);
>
>      dinfo = drive_get(IF_PFLASH, 0, 0);
>      pflash_cfi01_register(SA_CS0, "collie.fl1", 0x02000000,
> diff --git a/hw/arm/strongarm.c b/hw/arm/strongarm.c
> index dc65d88a65..6bee034914 100644
> --- a/hw/arm/strongarm.c
> +++ b/hw/arm/strongarm.c
> @@ -1586,8 +1586,7 @@ static const TypeInfo strongarm_ssp_info = {
>  };
>
>  /* Main CPU functions */
> -StrongARMState *sa1110_init(MemoryRegion *sysmem,
> -                            unsigned int sdram_size, const char *cpu_type)
> +StrongARMState *sa1110_init(const char *cpu_type)
>  {
>      StrongARMState *s;
>      int i;
> @@ -1601,10 +1600,6 @@ StrongARMState *sa1110_init(MemoryRegion *sysmem,
>
>      s->cpu = ARM_CPU(cpu_create(cpu_type));
>
> -    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,
>                      qdev_get_gpio_in(DEVICE(s->cpu), ARM_CPU_IRQ),
>                      qdev_get_gpio_in(DEVICE(s->cpu), ARM_CPU_FIQ),
> diff --git a/hw/arm/strongarm.h b/hw/arm/strongarm.h
> index e98840b461..192821f6aa 100644
> --- a/hw/arm/strongarm.h
> +++ b/hw/arm/strongarm.h
> @@ -55,7 +55,6 @@ enum {
>
>  typedef struct {
>      ARMCPU *cpu;
> -    MemoryRegion sdram;
>      DeviceState *pic;
>      DeviceState *gpio;
>      DeviceState *ppc;
> @@ -63,7 +62,6 @@ typedef struct {
>      SSIBus *ssp_bus;
>  } StrongARMState;
>
> -StrongARMState *sa1110_init(MemoryRegion *sysmem,
> -                            unsigned int sdram_size, const char *rev);
> +StrongARMState *sa1110_init(const char *cpu_type);
>
>  #endif
> --
> 2.21.0
>
>

Re: [PATCH 06/21] hw/arm/digic4: Inline digic4_board_setup_ram() function

[Alistair Francis]

On Sun, Oct 20, 2019 at 4:07 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> Having the RAM creation code in a separate function is not
> very helpful. Move this code directly inside the board_init()
> function, this will later allow the board to have the QOM
> ownership of the RAM.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/arm/digic_boards.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/hw/arm/digic_boards.c b/hw/arm/digic_boards.c
> index 304e4d1a29..ef3fc2b6a5 100644
> --- a/hw/arm/digic_boards.c
> +++ b/hw/arm/digic_boards.c
> @@ -53,12 +53,6 @@ typedef struct DigicBoard {
>      const char *rom1_def_filename;
>  } DigicBoard;
>
> -static void digic4_board_setup_ram(DigicBoardState *s, hwaddr ram_size)
> -{
> -    memory_region_allocate_system_memory(&s->ram, NULL, "ram", ram_size);
> -    memory_region_add_subregion(get_system_memory(), 0, &s->ram);
> -}
> -
>  static void digic4_board_init(DigicBoard *board)
>  {
>      Error *err = NULL;
> @@ -72,7 +66,8 @@ static void digic4_board_init(DigicBoard *board)
>          exit(1);
>      }
>
> -    digic4_board_setup_ram(s, board->ram_size);
> +    memory_region_allocate_system_memory(&s->ram, NULL, "ram", board->ram_size);
> +    memory_region_add_subregion(get_system_memory(), 0, &s->ram);
>
>      if (board->add_rom0) {
>          board->add_rom0(s, DIGIC4_ROM0_BASE, board->rom0_def_filename);
> --
> 2.21.0
>
>

Re: [PATCH 13/21] hw/cris: Let the machine be the owner of the system memory

[Alistair Francis]

On Sun, Oct 20, 2019 at 4:14 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/cris/axis_dev88.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/cris/axis_dev88.c b/hw/cris/axis_dev88.c
> index 940c7dd122..faa7058733 100644
> --- a/hw/cris/axis_dev88.c
> +++ b/hw/cris/axis_dev88.c
> @@ -270,7 +270,7 @@ void axisdev88_init(MachineState *machine)
>      env = &cpu->env;
>
>      /* allocate RAM */
> -    memory_region_allocate_system_memory(phys_ram, NULL, "axisdev88.ram",
> +    memory_region_allocate_system_memory(phys_ram, machine, "axisdev88.ram",
>                                           ram_size);
>      memory_region_add_subregion(address_space_mem, 0x40000000, phys_ram);
>
> --
> 2.21.0
>
>

Re: [PATCH 14/21] hw/hppa: Let the machine be the owner of the system memory

[Alistair Francis]

On Sun, Oct 20, 2019 at 4:17 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/hppa/machine.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
> index dbe1ff0fe5..6703bfd351 100644
> --- a/hw/hppa/machine.c
> +++ b/hw/hppa/machine.c
> @@ -96,7 +96,7 @@ static void machine_hppa_init(MachineState *machine)
>
>      /* Main memory region. */
>      ram_region = g_new(MemoryRegion, 1);
> -    memory_region_allocate_system_memory(ram_region, NULL,
> +    memory_region_allocate_system_memory(ram_region, machine,
>                                           "ram", ram_size);
>      memory_region_add_subregion(addr_space, 0, ram_region);
>
> --
> 2.21.0
>
>

Re: [PATCH 15/21] hw/i386: Let the machine be the owner of the system memory

[Alistair Francis]

On Sun, Oct 20, 2019 at 4:20 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/i386/pc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 4b1904237e..3414dc423a 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1655,7 +1655,7 @@ void pc_memory_init(PCMachineState *pcms,
>       * with older qemus that used qemu_ram_alloc().
>       */
>      ram = g_malloc(sizeof(*ram));
> -    memory_region_allocate_system_memory(ram, NULL, "pc.ram",
> +    memory_region_allocate_system_memory(ram, machine, "pc.ram",
>                                           machine->ram_size);
>      *ram_memory = ram;
>      ram_below_4g = g_malloc(sizeof(*ram_below_4g));
> --
> 2.21.0
>
>

Re: [PATCH 16/21] hw/lm32: Let the machine be the owner of the system memory

[Alistair Francis]

On Sun, Oct 20, 2019 at 4:10 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/lm32/lm32_boards.c | 4 ++--
>  hw/lm32/milkymist.c   | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/lm32/lm32_boards.c b/hw/lm32/lm32_boards.c
> index 5ae308bfcf..50b607e0cc 100644
> --- a/hw/lm32/lm32_boards.c
> +++ b/hw/lm32/lm32_boards.c
> @@ -107,7 +107,7 @@ static void lm32_evr_init(MachineState *machine)
>
>      reset_info->flash_base = flash_base;
>
> -    memory_region_allocate_system_memory(phys_ram, NULL, "lm32_evr.sdram",
> +    memory_region_allocate_system_memory(phys_ram, machine, "lm32_evr.sdram",
>                                           ram_size);
>      memory_region_add_subregion(address_space_mem, ram_base, phys_ram);
>
> @@ -200,7 +200,7 @@ static void lm32_uclinux_init(MachineState *machine)
>
>      reset_info->flash_base = flash_base;
>
> -    memory_region_allocate_system_memory(phys_ram, NULL,
> +    memory_region_allocate_system_memory(phys_ram, machine,
>                                           "lm32_uclinux.sdram", ram_size);
>      memory_region_add_subregion(address_space_mem, ram_base, phys_ram);
>
> diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c
> index 460d322de5..ee2eb1877e 100644
> --- a/hw/lm32/milkymist.c
> +++ b/hw/lm32/milkymist.c
> @@ -116,7 +116,7 @@ milkymist_init(MachineState *machine)
>
>      cpu_lm32_set_phys_msb_ignore(env, 1);
>
> -    memory_region_allocate_system_memory(phys_sdram, NULL, "milkymist.sdram",
> +    memory_region_allocate_system_memory(phys_sdram, machine, "milkymist.sdram",
>                                           sdram_size);
>      memory_region_add_subregion(address_space_mem, sdram_base, phys_sdram);
>
> --
> 2.21.0
>
>

Re: [PATCH 17/21] hw/m68k: Let the machine be the owner of the system memory

[Alistair Francis]

On Sun, Oct 20, 2019 at 4:20 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/m68k/an5206.c    | 2 +-
>  hw/m68k/mcf5208.c   | 2 +-
>  hw/m68k/next-cube.c | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/m68k/an5206.c b/hw/m68k/an5206.c
> index 54ccbe1a82..fb045c2436 100644
> --- a/hw/m68k/an5206.c
> +++ b/hw/m68k/an5206.c
> @@ -46,7 +46,7 @@ static void an5206_init(MachineState *machine)
>      env->rambar0 = AN5206_RAMBAR_ADDR | 1;
>
>      /* DRAM at address zero */
> -    memory_region_allocate_system_memory(ram, NULL, "an5206.ram", ram_size);
> +    memory_region_allocate_system_memory(ram, machine, "an5206.ram", ram_size);
>      memory_region_add_subregion(address_space_mem, 0, ram);
>
>      /* Internal SRAM.  */
> diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
> index 34d34eba17..8e8c8ef349 100644
> --- a/hw/m68k/mcf5208.c
> +++ b/hw/m68k/mcf5208.c
> @@ -248,7 +248,7 @@ static void mcf5208evb_init(MachineState *machine)
>      memory_region_add_subregion(address_space_mem, 0x00000000, rom);
>
>      /* DRAM at 0x40000000 */
> -    memory_region_allocate_system_memory(ram, NULL, "mcf5208.ram", ram_size);
> +    memory_region_allocate_system_memory(ram, machine, "mcf5208.ram", ram_size);
>      memory_region_add_subregion(address_space_mem, 0x40000000, ram);
>
>      /* Internal SRAM.  */
> diff --git a/hw/m68k/next-cube.c b/hw/m68k/next-cube.c
> index e5343348d0..6aed9376f3 100644
> --- a/hw/m68k/next-cube.c
> +++ b/hw/m68k/next-cube.c
> @@ -893,7 +893,7 @@ static void next_cube_init(MachineState *machine)
>      memcpy(ns->rtc.ram, rtc_ram2, 32);
>
>      /* 64MB RAM starting at 0x04000000  */
> -    memory_region_allocate_system_memory(ram, NULL, "next.ram", ram_size);
> +    memory_region_allocate_system_memory(ram, machine, "next.ram", ram_size);
>      memory_region_add_subregion(sysmem, 0x04000000, ram);
>
>      /* Framebuffer */
> --
> 2.21.0
>
>

Re: [PATCH 21/21] hw/core: Assert memory_region_allocate_system_memory has machine owner

[Alistair Francis]

On Sun, Oct 20, 2019 at 4:22 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> All the memory_region_allocate_system_memory() pass a MachineState
> argument. Add an assertion to ensure the new boards/machines added
> set this argument, so all system memory object have the machine as
> its QOM owner.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/core/numa.c      | 4 +---
>  include/hw/boards.h | 2 +-
>  2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/hw/core/numa.c b/hw/core/numa.c
> index 2e29e4bfe0..3e07e94d00 100644
> --- a/hw/core/numa.c
> +++ b/hw/core/numa.c
> @@ -527,9 +527,7 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, MachineState *ms,
>      uint64_t addr = 0;
>      int i;
>
> -    if (!ms) {
> -        ms = MACHINE(qdev_get_machine());
> -    }
> +    g_assert(ms);
>
>      if (ms->numa_state == NULL ||
>          ms->numa_state->num_nodes == 0 || !have_memdevs) {
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 3b6cb82b6c..31ab6e7586 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -14,7 +14,7 @@
>  /**
>   * memory_region_allocate_system_memory - Allocate a board's main memory
>   * @mr: the #MemoryRegion to be initialized
> - * @ms: the #MachineState object that own the system memory
> + * @ms: the #MachineState object that own the system memory (must not be NULL)
>   * @name: name of the memory region
>   * @ram_size: size of the region in bytes
>   *
> --
> 2.21.0
>
>

Re: [PATCH 07/21] hw: Drop QOM ownership on memory_region_allocate_system_memory() calls

[Alistair Francis]

On Sun, Oct 20, 2019 at 4:10 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> All the memory_region_allocate_system_memory() calls are in the
> board_init() code.  From the 58 calls in the repository, only
> 4 set the 'owner' parameter. It is obvious we want the Machine
> to be the owner of the RAM, so we want to use OBJECT(machine)
> as owner. We can simplify a bit by passing MachineState to
> memory_region_allocate_system_memory(). In preparation of this
> refactor, first drop the 'owner' argument from these 4 uses.
>
>   $ git grep memory_region_allocate_system_memory hw/ | wc -l
>   58
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/alpha/typhoon.c | 2 +-
>  hw/arm/raspi.c     | 2 +-
>  hw/hppa/machine.c  | 2 +-
>  hw/sparc/sun4m.c   | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
> index 179e1f7658..8489ec335c 100644
> --- a/hw/alpha/typhoon.c
> +++ b/hw/alpha/typhoon.c
> @@ -851,7 +851,7 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
>
>      /* Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
>         but the address space hole reserved at this point is 8TB.  */
> -    memory_region_allocate_system_memory(&s->ram_region, OBJECT(s), "ram",
> +    memory_region_allocate_system_memory(&s->ram_region, NULL, "ram",
>                                           ram_size);
>      memory_region_add_subregion(addr_space, 0, &s->ram_region);
>
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index 615d755879..f76b6eaad3 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -182,7 +182,7 @@ static void raspi_init(MachineState *machine, int version)
>                              &error_abort, NULL);
>
>      /* Allocate and map RAM */
> -    memory_region_allocate_system_memory(&s->ram, OBJECT(machine), "ram",
> +    memory_region_allocate_system_memory(&s->ram, NULL, "ram",
>                                           machine->ram_size);
>      /* FIXME: Remove when we have custom CPU address space support */
>      memory_region_add_subregion_overlap(get_system_memory(), 0, &s->ram, 0);
> diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
> index 953d454f48..dbe1ff0fe5 100644
> --- a/hw/hppa/machine.c
> +++ b/hw/hppa/machine.c
> @@ -96,7 +96,7 @@ static void machine_hppa_init(MachineState *machine)
>
>      /* Main memory region. */
>      ram_region = g_new(MemoryRegion, 1);
> -    memory_region_allocate_system_memory(ram_region, OBJECT(machine),
> +    memory_region_allocate_system_memory(ram_region, NULL,
>                                           "ram", ram_size);
>      memory_region_add_subregion(addr_space, 0, ram_region);
>
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 6c5a17a020..0bb7524655 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -788,7 +788,7 @@ static void ram_realize(DeviceState *dev, Error **errp)
>      RamDevice *d = SUN4M_RAM(dev);
>      SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
>
> -    memory_region_allocate_system_memory(&d->ram, OBJECT(d), "sun4m.ram",
> +    memory_region_allocate_system_memory(&d->ram, NULL, "sun4m.ram",
>                                           d->size);
>      sysbus_init_mmio(sbd, &d->ram);
>  }
> --
> 2.21.0
>
>

Re: [PATCH 09/21] hw: Let memory_region_allocate_system_memory take MachineState argument

[Alistair Francis]

On Sun, Oct 20, 2019 at 4:12 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> All the codebase calls memory_region_allocate_system_memory() with
> a NULL 'owner' from the board_init() function.
> Let pass a MachineState argument, and enforce the QOM ownership of
> the system memory.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/core/numa.c      | 11 +++++++----
>  include/hw/boards.h |  6 ++++--
>  2 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/hw/core/numa.c b/hw/core/numa.c
> index 038c96d4ab..2e29e4bfe0 100644
> --- a/hw/core/numa.c
> +++ b/hw/core/numa.c
> @@ -520,21 +520,24 @@ static void allocate_system_memory_nonnuma(MemoryRegion *mr, Object *owner,
>      vmstate_register_ram_global(mr);
>  }
>
> -void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
> +void memory_region_allocate_system_memory(MemoryRegion *mr, MachineState *ms,
>                                            const char *name,
>                                            uint64_t ram_size)
>  {
>      uint64_t addr = 0;
>      int i;
> -    MachineState *ms = MACHINE(qdev_get_machine());
> +
> +    if (!ms) {
> +        ms = MACHINE(qdev_get_machine());
> +    }
>
>      if (ms->numa_state == NULL ||
>          ms->numa_state->num_nodes == 0 || !have_memdevs) {
> -        allocate_system_memory_nonnuma(mr, owner, name, ram_size);
> +        allocate_system_memory_nonnuma(mr, OBJECT(ms), name, ram_size);
>          return;
>      }
>
> -    memory_region_init(mr, owner, name, ram_size);
> +    memory_region_init(mr, OBJECT(ms), name, ram_size);
>      for (i = 0; i < ms->numa_state->num_nodes; i++) {
>          uint64_t size = ms->numa_state->nodes[i].node_mem;
>          HostMemoryBackend *backend = ms->numa_state->nodes[i].node_memdev;
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index de45087f34..3b6cb82b6c 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -14,7 +14,7 @@
>  /**
>   * memory_region_allocate_system_memory - Allocate a board's main memory
>   * @mr: the #MemoryRegion to be initialized
> - * @owner: the object that tracks the region's reference count
> + * @ms: the #MachineState object that own the system memory
>   * @name: name of the memory region
>   * @ram_size: size of the region in bytes
>   *
> @@ -38,8 +38,10 @@
>   * Smaller pieces of memory (display RAM, static RAMs, etc) don't need
>   * to be backed via the -mem-path memory backend and can simply
>   * be created via memory_region_init_ram().
> + *
> + * The #MachineState object will track the region's reference count.
>   */
> -void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
> +void memory_region_allocate_system_memory(MemoryRegion *mr, MachineState *ms,
>                                            const char *name,
>                                            uint64_t ram_size);
>
> --
> 2.21.0
>
>

Re: [PATCH 10/21] hw/core: Let the machine be the owner of the system memory

[Alistair Francis]

On Sun, Oct 20, 2019 at 4:07 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/core/null-machine.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/core/null-machine.c b/hw/core/null-machine.c
> index 1aa0a9a01a..16546c8140 100644
> --- a/hw/core/null-machine.c
> +++ b/hw/core/null-machine.c
> @@ -35,7 +35,7 @@ static void machine_none_init(MachineState *mch)
>      if (mch->ram_size) {
>          MemoryRegion *ram = g_new(MemoryRegion, 1);
>
> -        memory_region_allocate_system_memory(ram, NULL, "ram", mch->ram_size);
> +        memory_region_allocate_system_memory(ram, mch, "ram", mch->ram_size);
>          memory_region_add_subregion(get_system_memory(), 0, ram);
>      }
>
> --
> 2.21.0
>
>

Re: [PATCH 07/21] hw: Drop QOM ownership on memory_region_allocate_system_memory() calls

[Artyom Tarasenko]

On Tue, Oct 22, 2019 at 1:23 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Sun, Oct 20, 2019 at 4:10 PM Philippe Mathieu-Daudé
> <philmd@redhat.com> wrote:
> >
> > All the memory_region_allocate_system_memory() calls are in the
> > board_init() code.  From the 58 calls in the repository, only
> > 4 set the 'owner' parameter. It is obvious we want the Machine
> > to be the owner of the RAM, so we want to use OBJECT(machine)
> > as owner. We can simplify a bit by passing MachineState to
> > memory_region_allocate_system_memory(). In preparation of this
> > refactor, first drop the 'owner' argument from these 4 uses.
> >
> >   $ git grep memory_region_allocate_system_memory hw/ | wc -l
> >   58
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Reviewed-by: Artyom Tarasenko <atar4qemu@gmail.com>

>
> Alistair
>
> > ---
> >  hw/alpha/typhoon.c | 2 +-
> >  hw/arm/raspi.c     | 2 +-
> >  hw/hppa/machine.c  | 2 +-
> >  hw/sparc/sun4m.c   | 2 +-
> >  4 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c
> > index 179e1f7658..8489ec335c 100644
> > --- a/hw/alpha/typhoon.c
> > +++ b/hw/alpha/typhoon.c
> > @@ -851,7 +851,7 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus,
> >
> >      /* Main memory region, 0x00.0000.0000.  Real hardware supports 32GB,
> >         but the address space hole reserved at this point is 8TB.  */
> > -    memory_region_allocate_system_memory(&s->ram_region, OBJECT(s), "ram",
> > +    memory_region_allocate_system_memory(&s->ram_region, NULL, "ram",
> >                                           ram_size);
> >      memory_region_add_subregion(addr_space, 0, &s->ram_region);
> >
> > diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> > index 615d755879..f76b6eaad3 100644
> > --- a/hw/arm/raspi.c
> > +++ b/hw/arm/raspi.c
> > @@ -182,7 +182,7 @@ static void raspi_init(MachineState *machine, int version)
> >                              &error_abort, NULL);
> >
> >      /* Allocate and map RAM */
> > -    memory_region_allocate_system_memory(&s->ram, OBJECT(machine), "ram",
> > +    memory_region_allocate_system_memory(&s->ram, NULL, "ram",
> >                                           machine->ram_size);
> >      /* FIXME: Remove when we have custom CPU address space support */
> >      memory_region_add_subregion_overlap(get_system_memory(), 0, &s->ram, 0);
> > diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
> > index 953d454f48..dbe1ff0fe5 100644
> > --- a/hw/hppa/machine.c
> > +++ b/hw/hppa/machine.c
> > @@ -96,7 +96,7 @@ static void machine_hppa_init(MachineState *machine)
> >
> >      /* Main memory region. */
> >      ram_region = g_new(MemoryRegion, 1);
> > -    memory_region_allocate_system_memory(ram_region, OBJECT(machine),
> > +    memory_region_allocate_system_memory(ram_region, NULL,
> >                                           "ram", ram_size);
> >      memory_region_add_subregion(addr_space, 0, ram_region);
> >
> > diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> > index 6c5a17a020..0bb7524655 100644
> > --- a/hw/sparc/sun4m.c
> > +++ b/hw/sparc/sun4m.c
> > @@ -788,7 +788,7 @@ static void ram_realize(DeviceState *dev, Error **errp)
> >      RamDevice *d = SUN4M_RAM(dev);
> >      SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> >
> > -    memory_region_allocate_system_memory(&d->ram, OBJECT(d), "sun4m.ram",
> > +    memory_region_allocate_system_memory(&d->ram, NULL, "sun4m.ram",
> >                                           d->size);
> >      sysbus_init_mmio(sbd, &d->ram);
> >  }
> > --
> > 2.21.0
> >
> >



-- 
Regards,
Artyom Tarasenko

SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu

Re: [PATCH 12/21] hw/arm: Let the machine be the owner of the system memory

[Philippe Mathieu-Daudé]

Cc'ing avocado-devel for test idea.

On 10/21/19 4:57 PM, Dr. David Alan Gilbert wrote:
> * Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
>> Cc'ing Paolo/David.
>>
>> On 10/21/19 11:39 AM, Peter Maydell wrote:
>>> On Mon, 21 Oct 2019 at 10:34, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>>>
>>>> On 10/21/19 11:22 AM, Peter Maydell wrote:
>>>>> On Mon, 21 Oct 2019 at 00:01, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>>>>>
>>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>>> ---
>>>>>
>>>>>>    hw/arm/virt.c             | 2 +-
>>>>>
>>>>> I think from a quick code scan that this is ok, but did
>>>>> you test that migration compat from old to new still works?
>>>>> I vaguely recall that there are some cases when adding an
>>>>> owner to a memory region changes the name string used for
>>>>> identifying the ramblock in migration.
>>>>
>>>> It seems to still works:
>>>>
>>>> $ make check-qtest-aarch64 V=1
>>>
>>>> This test migrate the virt machine.
>>>>
>>>> Is this enough?
>>>
>>> No, you need to test migration from a QEMU binary without
>>> this patchset to a QEMU binary that has it. Otherwise you're
>>> only checking that the new version can migrate from itself
>>> to itself. I find the easiest way to test this is just to
>>> use the 'savevm' command to save a state snapshot to a
>>> qcow2 disk image while running the old binary, and then run
>>> 'loadvm' with the new binary and check it restored OK.
>>
>> I did not think if this case.
>>
>> I followed your blog post [*] and tested the migration works OK.
>>
>> Paolo, now thinking about regular testing, we should add this testing to
>> patchew too. Something like:
>>
>> - when mainstream/master is updated, patchew build QEMU (it should be
>> already mostly ccached) and generate some vm dumps with 'savevm'.
>>
>> - build/test the series
>>
>> - if series succeeded testing, run 'loadvm' tests
>>
>> [*] https://translatedcode.wordpress.com/2015/07/06/tricks-for-debugging-qemu-savevm-snapshots/
> 
> Avocado certainly already has an option for specifying source and
> destination qemu separately; I've used that for testing
> cross version in the past.
> 
> The challenge is finding a command line/set of devices for each
> architecture that's expected to be stable.
> You want a command line with as big a set of devices as possible (for
> coverage) yet really is tied to machine type.
> 
> Dave
> 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>