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

[Edgar E. Iglesias]

On Fri, Apr 10, 2015 at 03:29:56PM +0100, Peter Maydell wrote:
> On 9 April 2015 at 04:47, Edgar E. Iglesias <address@hidden> 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 <address@hidden>
> >> Signed-off-by: Dirk Mueller <address@hidden>
> >> ---
> >>  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 <address@hidden>
Reviewed-by: Edgar E. Iglesias <address@hidden>

The patch has some kind of whitespace issue, but it applies
with --ignore-whitespace. I've pushed it now, thanks!

Cheers,
Edgar