Re: [PATCH 0/3] eliminate remaining places that abuse memory_region_allocate_system_memory()

[Igor Mammedov <imammedo@redhat.com>]

On Tue, 8 Oct 2019 14:41:25 +0200
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> Hi Igor,
> 
> On 10/8/19 1:33 PM, Igor Mammedov wrote:
> > Series cleans up remaining boards that call memory_region_allocate_system_memory()
> > multiple times, violating interface contract (the function should be called only
> > once).
> > 
> > With that cleaned up, it should be possible to switch from adhoc RAM allocation
> > in memory_region_allocate_system_memory()->allocate_system_memory_nonnuma() to
> > memory-backend based allocation, remaining roadblock for doing it is deprecated
> > -mem-path fallback to RAM allocation, which is scheduled for removal at 4.3
> > merge window. So remaining patches to consolidate system RAM allocation around
> > memory-backends and aliasing -mem-path/mem-prealloc to it are postponed till
> > then.  
> 
> How do we protect the codebase for new boards to not make the same mistake?
> 
> What about some code like this snippet (or nicer, but since this is a 
> developer error, and assert is enough IMO):
probably it's not worth effort (it's not too long till 4.2 softfreeze).

Like cover letter say, I'm planing to finish refactoring of
memory_region_allocate_system_memory() and I hope this function
will be gone during 4.3 and most boards will only need to map
pre-created (by common code) memory-backend wherever they used to
map RAM memory region.

> -- >8 --  
> 
> diff --git a/hw/core/numa.c b/hw/core/numa.c
> index 4dfec5c95b..a487677672 100644
> --- a/hw/core/numa.c
> +++ b/hw/core/numa.c
> @@ -484,6 +484,11 @@ static void 
> allocate_system_memory_nonnuma(MemoryRegion *mr, Object *owner,
>                                              const char *name,
>                                              uint64_t ram_size)
>   {
> +    static bool nonnuma_system_memory_allocated;
> +
> +    g_assert(!nonnuma_system_memory_allocated);
> +    nonnuma_system_memory_allocated = true;
> +
>       if (mem_path) {
>   #ifdef __linux__
>           Error *err = NULL;
> ---
> 
> $ hppa-softmmu/qemu-system-hppa
> **
> ERROR:/home/phil/source/qemu/hw/core/numa.c:489:allocate_system_memory_nonnuma: 
> assertion failed: (!nonnuma_system_memory_allocated)
> Aborted (core dumped)