diff mbox

[v3.2,11/31] numa: introduce memory_region_allocate_system_memory

Message ID d0e020b4d66e8a8c92c5552dd208a69131f8d8ab.1400049817.git.hutao@cn.fujitsu.com
State New
Headers show

Commit Message

Hu Tao May 14, 2014, 9:43 a.m. UTC 
From: Paolo Bonzini <pbonzini@redhat.com>

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/i386/pc.c            | 4 +---
 include/hw/boards.h     | 6 +++++-
 include/sysemu/sysemu.h | 1 +
 numa.c                  | 9 +++++++++
 4 files changed, 16 insertions(+), 4 deletions(-)

Comments

Eduardo Habkost May 14, 2014, 8:30 p.m. UTC | #1 
On Wed, May 14, 2014 at 05:43:15PM +0800, Hu Tao wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/i386/pc.c            | 4 +---
>  include/hw/boards.h     | 6 +++++-
>  include/sysemu/sysemu.h | 1 +
>  numa.c                  | 9 +++++++++
>  4 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 3673da8..3778d41 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1210,9 +1210,7 @@ FWCfgState *pc_memory_init(QEMUMachineInitArgs *args,
>       * with older qemus that used qemu_ram_alloc().
>       */
>      ram = g_malloc(sizeof(*ram));
> -    memory_region_init_ram(ram, NULL, "pc.ram",
> -                           below_4g_mem_size + above_4g_mem_size);
> -    vmstate_register_ram_global(ram);
> +    memory_region_allocate_system_memory(ram, NULL, "pc.ram", args->ram_size);

I had to check if below_4g_mem_size+above_4g_mem_size can be always
safely replaced by args->ram_size. Personally, wouldn't change the
ram_size expression in the same patch that adds the new function, but:

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>


Maybe we could at least add:
  assert(below_4g_mem_size + above_4g_mem_size == args->ram_size);
to the code, later?


>      *ram_memory = ram;
>      ram_below_4g = g_malloc(sizeof(*ram_below_4g));
>      memory_region_init_alias(ram_below_4g, NULL, "ram-below-4g", ram,
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 4345bd0..3f1c17d 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -50,9 +50,13 @@ struct QEMUMachine {
>      const char *hw_version;
>  };
>  
> -#define TYPE_MACHINE_SUFFIX "-machine"
> +void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
> +                                          const char *name,
> +                                          uint64_t ram_size);
> +
>  int qemu_register_machine(QEMUMachine *m);
>  
> +#define TYPE_MACHINE_SUFFIX "-machine"
>  #define TYPE_MACHINE "machine"
>  #undef MACHINE  /* BSD defines it and QEMU does not use it */
>  #define MACHINE(obj) \
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 423d49e..caf88dd 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -10,6 +10,7 @@
>  #include "qemu/notify.h"
>  #include "qemu/main-loop.h"
>  #include "qemu/bitmap.h"
> +#include "qom/object.h"
>  
>  /* vl.c */
>  
> diff --git a/numa.c b/numa.c
> index 439df87..bcd7b04 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -33,6 +33,7 @@
>  #include "qapi/opts-visitor.h"
>  #include "qapi/dealloc-visitor.h"
>  #include "qapi/qmp/qerror.h"
> +#include "hw/boards.h"
>  
>  QemuOptsList qemu_numa_opts = {
>      .name = "numa",
> @@ -194,3 +195,11 @@ void set_numa_modes(void)
>          }
>      }
>  }
> +
> +void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
> +                                          const char *name,
> +                                          uint64_t ram_size)
> +{
> +    memory_region_init_ram(mr, owner, name, ram_size);
> +    vmstate_register_ram_global(mr);
> +}
> -- 
> 1.8.5.2.229.g4448466
> 
>
Michael S. Tsirkin June 8, 2014, 10:10 a.m. UTC | #2 
On Wed, May 14, 2014 at 05:43:15PM +0800, Hu Tao wrote:
> From: Paolo Bonzini <pbonzini@redhat.com>
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/i386/pc.c            | 4 +---
>  include/hw/boards.h     | 6 +++++-
>  include/sysemu/sysemu.h | 1 +
>  numa.c                  | 9 +++++++++
>  4 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 3673da8..3778d41 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1210,9 +1210,7 @@ FWCfgState *pc_memory_init(QEMUMachineInitArgs *args,
>       * with older qemus that used qemu_ram_alloc().
>       */
>      ram = g_malloc(sizeof(*ram));
> -    memory_region_init_ram(ram, NULL, "pc.ram",
> -                           below_4g_mem_size + above_4g_mem_size);
> -    vmstate_register_ram_global(ram);
> +    memory_region_allocate_system_memory(ram, NULL, "pc.ram", args->ram_size);
>      *ram_memory = ram;
>      ram_below_4g = g_malloc(sizeof(*ram_below_4g));
>      memory_region_init_alias(ram_below_4g, NULL, "ram-below-4g", ram,

Better keep below_4g_mem_size + above_4g_mem_size around, this way this
can be a stand-alone patch.

> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 4345bd0..3f1c17d 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -50,9 +50,13 @@ struct QEMUMachine {
>      const char *hw_version;
>  };
>  
> -#define TYPE_MACHINE_SUFFIX "-machine"
> +void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
> +                                          const char *name,
> +                                          uint64_t ram_size);
> +
>  int qemu_register_machine(QEMUMachine *m);
>  
> +#define TYPE_MACHINE_SUFFIX "-machine"
>  #define TYPE_MACHINE "machine"
>  #undef MACHINE  /* BSD defines it and QEMU does not use it */
>  #define MACHINE(obj) \
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 423d49e..caf88dd 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -10,6 +10,7 @@
>  #include "qemu/notify.h"
>  #include "qemu/main-loop.h"
>  #include "qemu/bitmap.h"
> +#include "qom/object.h"
>  
>  /* vl.c */
>  
> diff --git a/numa.c b/numa.c
> index 439df87..bcd7b04 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -33,6 +33,7 @@
>  #include "qapi/opts-visitor.h"
>  #include "qapi/dealloc-visitor.h"
>  #include "qapi/qmp/qerror.h"
> +#include "hw/boards.h"
>  
>  QemuOptsList qemu_numa_opts = {
>      .name = "numa",
> @@ -194,3 +195,11 @@ void set_numa_modes(void)
>          }
>      }
>  }
> +
> +void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
> +                                          const char *name,
> +                                          uint64_t ram_size)
> +{
> +    memory_region_init_ram(mr, owner, name, ram_size);
> +    vmstate_register_ram_global(mr);
> +}
> -- 
> 1.8.5.2.229.g4448466
>
Hu Tao June 9, 2014, 2:24 a.m. UTC | #3 
On Wed, May 14, 2014 at 05:30:23PM -0300, Eduardo Habkost wrote:
> On Wed, May 14, 2014 at 05:43:15PM +0800, Hu Tao wrote:
> > From: Paolo Bonzini <pbonzini@redhat.com>
> > 
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > ---
> >  hw/i386/pc.c            | 4 +---
> >  include/hw/boards.h     | 6 +++++-
> >  include/sysemu/sysemu.h | 1 +
> >  numa.c                  | 9 +++++++++
> >  4 files changed, 16 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 3673da8..3778d41 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -1210,9 +1210,7 @@ FWCfgState *pc_memory_init(QEMUMachineInitArgs *args,
> >       * with older qemus that used qemu_ram_alloc().
> >       */
> >      ram = g_malloc(sizeof(*ram));
> > -    memory_region_init_ram(ram, NULL, "pc.ram",
> > -                           below_4g_mem_size + above_4g_mem_size);
> > -    vmstate_register_ram_global(ram);
> > +    memory_region_allocate_system_memory(ram, NULL, "pc.ram", args->ram_size);
> 
> I had to check if below_4g_mem_size+above_4g_mem_size can be always
> safely replaced by args->ram_size. Personally, wouldn't change the
> ram_size expression in the same patch that adds the new function, but:
> 
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> 
> Maybe we could at least add:
>   assert(below_4g_mem_size + above_4g_mem_size == args->ram_size);
> to the code, later?

Thanks, Done.


Hu
Hu Tao June 9, 2014, 2:27 a.m. UTC | #4 
On Sun, Jun 08, 2014 at 01:10:25PM +0300, Michael S. Tsirkin wrote:
> On Wed, May 14, 2014 at 05:43:15PM +0800, Hu Tao wrote:
> > From: Paolo Bonzini <pbonzini@redhat.com>
> > 
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > ---
> >  hw/i386/pc.c            | 4 +---
> >  include/hw/boards.h     | 6 +++++-
> >  include/sysemu/sysemu.h | 1 +
> >  numa.c                  | 9 +++++++++
> >  4 files changed, 16 insertions(+), 4 deletions(-)
> > 
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 3673da8..3778d41 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -1210,9 +1210,7 @@ FWCfgState *pc_memory_init(QEMUMachineInitArgs *args,
> >       * with older qemus that used qemu_ram_alloc().
> >       */
> >      ram = g_malloc(sizeof(*ram));
> > -    memory_region_init_ram(ram, NULL, "pc.ram",
> > -                           below_4g_mem_size + above_4g_mem_size);
> > -    vmstate_register_ram_global(ram);
> > +    memory_region_allocate_system_memory(ram, NULL, "pc.ram", args->ram_size);
> >      *ram_memory = ram;
> >      ram_below_4g = g_malloc(sizeof(*ram_below_4g));
> >      memory_region_init_alias(ram_below_4g, NULL, "ram-below-4g", ram,
> 
> Better keep below_4g_mem_size + above_4g_mem_size around, this way this
> can be a stand-alone patch.

OK.

Hu
diff mbox

Patch

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 3673da8..3778d41 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1210,9 +1210,7 @@  FWCfgState *pc_memory_init(QEMUMachineInitArgs *args,
      * with older qemus that used qemu_ram_alloc().
      */
     ram = g_malloc(sizeof(*ram));
-    memory_region_init_ram(ram, NULL, "pc.ram",
-                           below_4g_mem_size + above_4g_mem_size);
-    vmstate_register_ram_global(ram);
+    memory_region_allocate_system_memory(ram, NULL, "pc.ram", args->ram_size);
     *ram_memory = ram;
     ram_below_4g = g_malloc(sizeof(*ram_below_4g));
     memory_region_init_alias(ram_below_4g, NULL, "ram-below-4g", ram,
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 4345bd0..3f1c17d 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -50,9 +50,13 @@  struct QEMUMachine {
     const char *hw_version;
 };
 
-#define TYPE_MACHINE_SUFFIX "-machine"
+void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
+                                          const char *name,
+                                          uint64_t ram_size);
+
 int qemu_register_machine(QEMUMachine *m);
 
+#define TYPE_MACHINE_SUFFIX "-machine"
 #define TYPE_MACHINE "machine"
 #undef MACHINE  /* BSD defines it and QEMU does not use it */
 #define MACHINE(obj) \
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 423d49e..caf88dd 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -10,6 +10,7 @@ 
 #include "qemu/notify.h"
 #include "qemu/main-loop.h"
 #include "qemu/bitmap.h"
+#include "qom/object.h"
 
 /* vl.c */
 
diff --git a/numa.c b/numa.c
index 439df87..bcd7b04 100644
--- a/numa.c
+++ b/numa.c
@@ -33,6 +33,7 @@ 
 #include "qapi/opts-visitor.h"
 #include "qapi/dealloc-visitor.h"
 #include "qapi/qmp/qerror.h"
+#include "hw/boards.h"
 
 QemuOptsList qemu_numa_opts = {
     .name = "numa",
@@ -194,3 +195,11 @@  void set_numa_modes(void)
         }
     }
 }
+
+void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
+                                          const char *name,
+                                          uint64_t ram_size)
+{
+    memory_region_init_ram(mr, owner, name, ram_size);
+    vmstate_register_ram_global(mr);
+}