| Message ID | CAL5wTH4kgaxbbyw3zUoypk4vvFFnRWW=mke14ZSsfjhPTVgW9A@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Sat, Apr 04, 2015 at 02:15:10PM +0200, Dirk Müller wrote: > Commit 0b183fc871:"memory: move mem_path handling to > memory_region_allocate_system_memory" split memory_region_init_ram and > memory_region_init_ram_from_file. Also it moved mem-path handling a step > up from memory_region_init_ram to memory_region_allocate_system_memory. > > Therefore for any board that uses memory_region_init_ram directly, > -mem-path is not supported. > > Fix this by replacing memory_region_init_ram with > memory_region_allocate_system_memory. > > Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com> > Signed-off-by: Dirk Mueller <dmueller@suse.com> > --- > hw/cris/axis_dev88.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) Hi, A question, should this only be done for one of the memories? BTW, I'm having problems git am:ing this patch, not sure why... Cheers, Edgar > > diff --git a/hw/cris/axis_dev88.c b/hw/cris/axis_dev88.c > index 0479196..3cae480 100644 > --- a/hw/cris/axis_dev88.c > +++ b/hw/cris/axis_dev88.c > @@ -270,9 +270,8 @@ void axisdev88_init(MachineState *machine) > env = &cpu->env; > > /* allocate RAM */ > - memory_region_init_ram(phys_ram, NULL, "axisdev88.ram", ram_size, > - &error_abort); > - vmstate_register_ram_global(phys_ram); > + memory_region_allocate_system_memory(phys_ram, NULL, "axisdev88.ram", > + ram_size); > memory_region_add_subregion(address_space_mem, 0x40000000, phys_ram); > > /* The ETRAX-FS has 128Kb on chip ram, the docs refer to it as the > -- > 2.0.4
On 9 April 2015 at 04:47, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: > On Sat, Apr 04, 2015 at 02:15:10PM +0200, Dirk Müller wrote: >> Commit 0b183fc871:"memory: move mem_path handling to >> memory_region_allocate_system_memory" split memory_region_init_ram and >> memory_region_init_ram_from_file. Also it moved mem-path handling a step >> up from memory_region_init_ram to memory_region_allocate_system_memory. >> >> Therefore for any board that uses memory_region_init_ram directly, >> -mem-path is not supported. >> >> Fix this by replacing memory_region_init_ram with >> memory_region_allocate_system_memory. >> >> Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com> >> Signed-off-by: Dirk Mueller <dmueller@suse.com> >> --- >> hw/cris/axis_dev88.c | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) > > > Hi, > > A question, should this only be done for one of the memories? Yes; as I understand it every board should call memory_region_allocate_system_memory once and only once, to allocate the "big lump" of RAM which should be backed by the -mem-path memory (hugepages, etc). If a board's major RAM is actually in two parts, you can deal with this by calling memory_region_allocate_system_memory once to get an MR with enough RAM for both parts, and then creating alias MemoryRegions for each part which point into that. [cf: https://lists.gnu.org/archive/html/qemu-devel/2015-03/msg05073.html] In the case of this cris board it looks to me from a quick scan of the board init code like it has one big lump of RAM (the one converted by this code) and then a few smaller "internal ram" type things. So just doing this for the big lump is correct. (The only use of -mem-path here is to allow the user to cause us to back QEMU's guest RAM allocations with host huge pages, which is in turn only something you care about for KVM. So for the TCG-only CPUs and boards this is to some extent academic, except it would be nice to be able to enable an assert that the board init does call memory_region_allocate_system_memory exactly once so we can catch cases where we forgot for KVM-enabled archs.) thanks -- PMM
On Fri, Apr 10, 2015 at 03:29:56PM +0100, Peter Maydell wrote: > On 9 April 2015 at 04:47, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote: > > On Sat, Apr 04, 2015 at 02:15:10PM +0200, Dirk Müller wrote: > >> Commit 0b183fc871:"memory: move mem_path handling to > >> memory_region_allocate_system_memory" split memory_region_init_ram and > >> memory_region_init_ram_from_file. Also it moved mem-path handling a step > >> up from memory_region_init_ram to memory_region_allocate_system_memory. > >> > >> Therefore for any board that uses memory_region_init_ram directly, > >> -mem-path is not supported. > >> > >> Fix this by replacing memory_region_init_ram with > >> memory_region_allocate_system_memory. > >> > >> Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com> > >> Signed-off-by: Dirk Mueller <dmueller@suse.com> > >> --- > >> hw/cris/axis_dev88.c | 5 ++--- > >> 1 file changed, 2 insertions(+), 3 deletions(-) > > > > > > Hi, > > > > A question, should this only be done for one of the memories? > > Yes; as I understand it every board should call > memory_region_allocate_system_memory once and only once, > to allocate the "big lump" of RAM which should be backed by > the -mem-path memory (hugepages, etc). If a board's major > RAM is actually in two parts, you can deal with this by > calling memory_region_allocate_system_memory once to get > an MR with enough RAM for both parts, and then creating > alias MemoryRegions for each part which point into that. > [cf: https://lists.gnu.org/archive/html/qemu-devel/2015-03/msg05073.html] > > In the case of this cris board it looks to me from a > quick scan of the board init code like it has one big > lump of RAM (the one converted by this code) and then > a few smaller "internal ram" type things. So just doing > this for the big lump is correct. > > (The only use of -mem-path here is to allow the user to > cause us to back QEMU's guest RAM allocations with host > huge pages, which is in turn only something you care about > for KVM. So for the TCG-only CPUs and boards this is > to some extent academic, except it would be nice to be > able to enable an assert that the board init does call > memory_region_allocate_system_memory exactly once so we > can catch cases where we forgot for KVM-enabled archs.) > I see, thanks for explaining. I gave a quick try with a CRIS image and it works fine. Tested-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com> The patch has some kind of whitespace issue, but it applies with --ignore-whitespace. I've pushed it now, thanks! Cheers, Edgar
diff --git a/hw/cris/axis_dev88.c b/hw/cris/axis_dev88.c index 0479196..3cae480 100644 --- a/hw/cris/axis_dev88.c +++ b/hw/cris/axis_dev88.c @@ -270,9 +270,8 @@ void axisdev88_init(MachineState *machine) env = &cpu->env; /* allocate RAM */ - memory_region_init_ram(phys_ram, NULL, "axisdev88.ram", ram_size, - &error_abort); - vmstate_register_ram_global(phys_ram); + memory_region_allocate_system_memory(phys_ram, NULL, "axisdev88.ram", + ram_size); memory_region_add_subregion(address_space_mem, 0x40000000, phys_ram); /* The ETRAX-FS has 128Kb on chip ram, the docs refer to it as the
Commit 0b183fc871:"memory: move mem_path handling to memory_region_allocate_system_memory" split memory_region_init_ram and memory_region_init_ram_from_file. Also it moved mem-path handling a step up from memory_region_init_ram to memory_region_allocate_system_memory. Therefore for any board that uses memory_region_init_ram directly, -mem-path is not supported. Fix this by replacing memory_region_init_ram with memory_region_allocate_system_memory. Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com> Signed-off-by: Dirk Mueller <dmueller@suse.com> --- hw/cris/axis_dev88.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)