Re: [Qemu-devel] [RFC] memory: use memory_region_init_ram() instead of memory_region_allocate_system_memory()
On Fri, 15 Feb 2019 19:18:50 +0100 Paolo Bonzini wrote: > On 15/02/19 16:42, Igor Mammedov wrote: > >> > >> What about -m, too? Then you'd specify a memdev instead of the initial > >> memory size. > > that somewhat what I've planned, > > make -m X translate into -object > > memory-backend-ram,id=magically-get-what-board-uses-now,size=X -machine > > memdev=thatid > > > > one more reason for memdev vs device is that -numa now uses memdevs and so > > far it > > doesn't look that non-numa initial RAM would get immediate benefits from > > using -device > > on most boards (well, I couldn't come up with any modulo backend/frontend > > consistent usage) > > What I meant is what about making the non-NUMA option -m memdev=thatid > (and the automatic translation you mention too). Yes, bikeshedding. :) sure, I can add it, it would be just an alias to machine.memdev > > Paolo >
Re: [Qemu-devel] [RFC] memory: use memory_region_init_ram() instead of memory_region_allocate_system_memory()
On 15/02/19 16:42, Igor Mammedov wrote: >> >> What about -m, too? Then you'd specify a memdev instead of the initial >> memory size. > that somewhat what I've planned, > make -m X translate into -object > memory-backend-ram,id=magically-get-what-board-uses-now,size=X -machine > memdev=thatid > > one more reason for memdev vs device is that -numa now uses memdevs and so > far it > doesn't look that non-numa initial RAM would get immediate benefits from > using -device > on most boards (well, I couldn't come up with any modulo backend/frontend > consistent usage) What I meant is what about making the non-NUMA option -m memdev=thatid (and the automatic translation you mention too). Yes, bikeshedding. :) Paolo
Re: [Qemu-devel] [RFC] memory: use memory_region_init_ram() instead of memory_region_allocate_system_memory()
On Fri, 15 Feb 2019 15:48:43 +0100 Paolo Bonzini wrote: > On 15/02/19 12:33, Igor Mammedov wrote: > > On Thu, 14 Feb 2019 19:11:27 +0100 > > Paolo Bonzini wrote: > > > >> On 14/02/19 15:07, Igor Mammedov wrote: > >>> Also some boards (ab)use memory_region_allocate_system_memory(), calling > >>> it several > >>> times to allocate various fixed sized chunks of RAM and ROMs, which is > >>> problematic > >>> to map to a single initial RAM Machine::memdev backend and is currently > >>> broken if > >>> -mem-path points to a not hugepage pool. > >> > >> This is certainly a good idea. However, I'm not sure why you would need > >> a memdev property on the Machine instead of just allowing 1 -numa node, > >> which is what really is. > > > > using '-numa node' would be confusing to user when he/she is not interested > > in numa usecase > > it also would enable numa fdt/acpi parts generated automatically (fixable > > but then again > > it adds more to confusion) and in the end there are boards that do not > > support numa at all > > (s390x). > > Fair enough. > > What about -m, too? Then you'd specify a memdev instead of the initial > memory size. that somewhat what I've planned, make -m X translate into -object memory-backend-ram,id=magically-get-what-board-uses-now,size=X -machine memdev=thatid one more reason for memdev vs device is that -numa now uses memdevs and so far it doesn't look that non-numa initial RAM would get immediate benefits from using -device on most boards (well, I couldn't come up with any modulo backend/frontend consistent usage) > > Paolo >
Re: [Qemu-devel] [RFC] memory: use memory_region_init_ram() instead of memory_region_allocate_system_memory()
On 15/02/19 12:33, Igor Mammedov wrote: > On Thu, 14 Feb 2019 19:11:27 +0100 > Paolo Bonzini wrote: > >> On 14/02/19 15:07, Igor Mammedov wrote: >>> Also some boards (ab)use memory_region_allocate_system_memory(), calling it >>> several >>> times to allocate various fixed sized chunks of RAM and ROMs, which is >>> problematic >>> to map to a single initial RAM Machine::memdev backend and is currently >>> broken if >>> -mem-path points to a not hugepage pool. >> >> This is certainly a good idea. However, I'm not sure why you would need >> a memdev property on the Machine instead of just allowing 1 -numa node, >> which is what really is. > > using '-numa node' would be confusing to user when he/she is not interested > in numa usecase > it also would enable numa fdt/acpi parts generated automatically (fixable but > then again > it adds more to confusion) and in the end there are boards that do not > support numa at all > (s390x). Fair enough. What about -m, too? Then you'd specify a memdev instead of the initial memory size. Paolo
Re: [Qemu-devel] [RFC] memory: use memory_region_init_ram() instead of memory_region_allocate_system_memory()
On Thu, 14 Feb 2019 18:30:37 + Peter Maydell wrote: > On Thu, 14 Feb 2019 at 14:07, Igor Mammedov wrote: > > Also some boards (ab)use memory_region_allocate_system_memory(), calling it > > several > > times to allocate various fixed sized chunks of RAM and ROMs, which is > > problematic > > to map to a single initial RAM Machine::memdev backend and is currently > > broken if > > -mem-path points to a not hugepage pool. > > These boards are buggy and we could fix them, if we wanted to > keep the existing API. We should in that case add assertions > that memory_region_allocate_system_memory() is called once and > only once, which would let "make check" enforce the rule. we can do this, but I'd rather remove memory_region_allocate_system_memory() API altogether which looks like overkill for most boards and use simpler memory_region_init_ram(). Then boards that need numa/hugepages could be switched to newer memdev/device model instead of using memory_region_init_ram() directly. My end-goal in all this exercise is to switch initial RAM to frontend/backend model and replace adhoc legacy numa memory code (-numa node,mem=size & default numa splitting) handling with a generic device or memdev based one. > > thanks > - PMM
Re: [Qemu-devel] [RFC] memory: use memory_region_init_ram() instead of memory_region_allocate_system_memory()
On Thu, 14 Feb 2019 19:11:27 +0100 Paolo Bonzini wrote: > On 14/02/19 15:07, Igor Mammedov wrote: > > Also some boards (ab)use memory_region_allocate_system_memory(), calling it > > several > > times to allocate various fixed sized chunks of RAM and ROMs, which is > > problematic > > to map to a single initial RAM Machine::memdev backend and is currently > > broken if > > -mem-path points to a not hugepage pool. > > This is certainly a good idea. However, I'm not sure why you would need > a memdev property on the Machine instead of just allowing 1 -numa node, > which is what really is. using '-numa node' would be confusing to user when he/she is not interested in numa usecase it also would enable numa fdt/acpi parts generated automatically (fixable but then again it adds more to confusion) and in the end there are boards that do not support numa at all (s390x). Machine memdev (initial-ram-memdev) property looked generic enough to me that it could be used with every board, but could go on a bit further and instead of memdev an add initial-ram property that would reference something like -device builtin-ram (I think I've seen some board using a sysbus based ram device for initial ram). If you ask why it's machine property, then I'm following '-m' semantics which is basically a machine property and I don't have a better idea how to tell board on CLI to use some device/memdev as initial ram. > Thanks, > > Paolo >
Re: [Qemu-devel] [RFC] memory: use memory_region_init_ram() instead of memory_region_allocate_system_memory()
On Thu, 14 Feb 2019 at 14:07, Igor Mammedov wrote: > Also some boards (ab)use memory_region_allocate_system_memory(), calling it > several > times to allocate various fixed sized chunks of RAM and ROMs, which is > problematic > to map to a single initial RAM Machine::memdev backend and is currently > broken if > -mem-path points to a not hugepage pool. These boards are buggy and we could fix them, if we wanted to keep the existing API. We should in that case add assertions that memory_region_allocate_system_memory() is called once and only once, which would let "make check" enforce the rule. thanks - PMM
Re: [Qemu-devel] [RFC] memory: use memory_region_init_ram() instead of memory_region_allocate_system_memory()
On 14/02/19 15:07, Igor Mammedov wrote: > Also some boards (ab)use memory_region_allocate_system_memory(), calling it > several > times to allocate various fixed sized chunks of RAM and ROMs, which is > problematic > to map to a single initial RAM Machine::memdev backend and is currently > broken if > -mem-path points to a not hugepage pool. This is certainly a good idea. However, I'm not sure why you would need a memdev property on the Machine instead of just allowing 1 -numa node, which is what really is. Thanks, Paolo
[Qemu-devel] [RFC] memory: use memory_region_init_ram() instead of memory_region_allocate_system_memory()
I'm considering to deprecating -mem-path/prealloc CLI options and replacing
them with a single memdev Machine property to allow interested users to pick
used backend for initial RAM (fixes mixed -mem-path+hostmem backends issues)
and as a transition step to modeling initial as a Device instead of (ab)using
MemoryRegion APIs.
Currently most boards use memory_region_allocate_system_memory() to allocate
RAM and the interface tied up too much to MemoryRegion that makes changing API
to hostmem across tree is quite a bit of work sometimes requiring logic rewrite
to do it cleanly and I'm not sure it's worth the effort and that it really has
a merit to do in case of TCG only boards.
I suggest to get rid memory_region_allocate_system_memory() on TCG only boards
since most of them don't really need NUMA features provided by this API and/or
probably won't noticeably benefit from -mem-path/prealloc backed memory if at
all
and replacing memory allocation with plain memory_region_init_ram() API.
It won't require extensive changes in TCG only boards we have now, since they
would
continue to use MemoryRegion based API approach. And the boards, that really
need
to use hugepages or numa features, could be amended to use new machine memdev
property and/or initial RAM being allocated by implictly (-m) using hostmem-ram
backend.
Also some boards (ab)use memory_region_allocate_system_memory(), calling it
several
times to allocate various fixed sized chunks of RAM and ROMs, which is
problematic
to map to a single initial RAM Machine::memdev backend and is currently broken
if
-mem-path points to a not hugepage pool.
This RFC attempts to cleanup things a bit on TCG only boards (only several ones)
side and test waters if it's acceptable approach. If it looks acceptable, I'll
send
a proper series to make usage of memory_region_allocate_system_memory() minimal
across
the codebase and then convert boards that actually use numa/hugepages to
initial RAM
memdev model (arm/virt, spapr, s390x, pc/q35).
Signed-off-by: Igor Mammedov
---
hw/arm/musicpal.c| 4 ++--
hw/arm/vexpress.c| 4 ++--
hw/sparc64/niagara.c | 30 +++---
3 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index d22532a11c..197e7d1282 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -1592,8 +1592,8 @@ 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",
- MP_RAM_DEFAULT_SIZE);
+memory_region_init_ram(ram, NULL, "musicpal.ram", MP_RAM_DEFAULT_SIZE,
+ &error_fatal);
memory_region_add_subregion(address_space_mem, 0, ram);
memory_region_init_ram(sram, NULL, "musicpal.sram", MP_SRAM_SIZE,
diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index c02d18ee61..a2cf5c0af2 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -280,8 +280,8 @@ static void a9_daughterboard_init(const
VexpressMachineState *vms,
exit(1);
}
-memory_region_allocate_system_memory(ram, NULL, "vexpress.highmem",
- ram_size);
+memory_region_init_ram(ram, NULL, "vexpress.highmem", ram_size,
+ &error_fatal);
low_ram_size = ram_size;
if (low_ram_size > 0x400) {
low_ram_size = 0x400;
diff --git a/hw/sparc64/niagara.c b/hw/sparc64/niagara.c
index f8a856f611..62e0348d5f 100644
--- a/hw/sparc64/niagara.c
+++ b/hw/sparc64/niagara.c
@@ -37,6 +37,7 @@
#include "sysemu/block-backend.h"
#include "qemu/error-report.h"
#include "sysemu/qtest.h"
+#include "qapi/error.h"
typedef struct NiagaraBoardState {
@@ -108,27 +109,26 @@ static void niagara_init(MachineState *machine)
/* init CPUs */
sparc64_cpu_devinit(machine->cpu_type, NIAGARA_PROM_BASE);
/* set up devices */
-memory_region_allocate_system_memory(&s->hv_ram, NULL, "sun4v-hv.ram",
- NIAGARA_HV_RAM_SIZE);
+memory_region_init_ram(&s->hv_ram, NULL, "sun4v-hv.ram",
+ NIAGARA_HV_RAM_SIZE, &error_fatal);
memory_region_add_subregion(sysmem, NIAGARA_HV_RAM_BASE, &s->hv_ram);
-memory_region_allocate_system_memory(&s->partition_ram, NULL,
- "sun4v-partition.ram",
- machine->ram_size);
+memory_region_init_ram(&s->partition_ram, NULL, "sun4v-partition.ram",
+ machine->ram_size, &error_fatal);
memory_region_add_subregion(sysmem, NIAGARA_PARTITION_RAM_BASE,
&s->partition_ram);
-memory_region_allocate_system_memory(&s->nvram, NULL,
- "sun4v.nvram", NIAGARA_NVRAM_SIZE);
+
