Re: [PATCH 0/3] eliminate remaining places that abuse memory_region_allocate_system_memory()
On Sun, Oct 20, 2019 at 04:38:06PM +0200, Philippe Mathieu-Daudé wrote: > Ping? Sorry, missed this. queued on machine-next. Pull request will be submitted today or tomorrow. > > On Fri, Oct 11, 2019 at 5:23 PM Igor Mammedov wrote: > > On Thu, 10 Oct 2019 19:35:03 +0200 > > Igor Mammedov wrote: > > > > Forgot to actually CC Eduardo, > > > > > On Tue, 8 Oct 2019 07:33:15 -0400 > > > Igor Mammedov wrote: > > ... > > > Eduardo, > > > > > > This patches are fixing various machines across tree, so series does not > > > belong > > > to any particular arch specific tree, can you merge it via generic > > > machine tree? > > > > > > > > > > > > > > > > Igor Mammedov (3): > > > > sparc64: use memory_region_allocate_system_memory() only for '-m' > > > > specified RAM > > > > ppc: rs6000_mc: drop usage of memory_region_allocate_system_memory() > > > > hppa: drop usage of memory_region_allocate_system_memory() for ROM > > > > > > > > hw/hppa/machine.c| 5 ++--- > > > > hw/ppc/rs6000_mc.c | 15 ++- > > > > hw/sparc64/niagara.c | 25 + > > > > 3 files changed, 25 insertions(+), 20 deletions(-) > > > > -- Eduardo
Re: [PATCH 0/3] eliminate remaining places that abuse memory_region_allocate_system_memory()
On Mon, 21 Oct 2019 10:59:56 +0200 Philippe Mathieu-Daudé 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. > > > > > > Igor Mammedov (3): > >sparc64: use memory_region_allocate_system_memory() only for '-m' > > specified RAM > >ppc: rs6000_mc: drop usage of memory_region_allocate_system_memory() > >hppa: drop usage of memory_region_allocate_system_memory() for ROM > > > > hw/hppa/machine.c| 5 ++--- > > hw/ppc/rs6000_mc.c | 15 ++- > > hw/sparc64/niagara.c | 25 + > > What about the TYPE_SUN4M_MEMORY device in hw/sparc/sun4m.c? it has only one call site so it's not issue this series targets. I'll take care of it in follow up series, which will deal with converting memory_region_allocate_system_memory() to memdev only (probably I'll drop TYPE_SUN4M_MEMORY altogether) >
Re: [PATCH 0/3] eliminate remaining places that abuse memory_region_allocate_system_memory()
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. Igor Mammedov (3): sparc64: use memory_region_allocate_system_memory() only for '-m' specified RAM ppc: rs6000_mc: drop usage of memory_region_allocate_system_memory() hppa: drop usage of memory_region_allocate_system_memory() for ROM hw/hppa/machine.c| 5 ++--- hw/ppc/rs6000_mc.c | 15 ++- hw/sparc64/niagara.c | 25 + What about the TYPE_SUN4M_MEMORY device in hw/sparc/sun4m.c?
Re: [PATCH 0/3] eliminate remaining places that abuse memory_region_allocate_system_memory()
Ping? On Fri, Oct 11, 2019 at 5:23 PM Igor Mammedov wrote: > On Thu, 10 Oct 2019 19:35:03 +0200 > Igor Mammedov wrote: > > Forgot to actually CC Eduardo, > > > On Tue, 8 Oct 2019 07:33:15 -0400 > > Igor Mammedov wrote: > ... > > Eduardo, > > > > This patches are fixing various machines across tree, so series does not > > belong > > to any particular arch specific tree, can you merge it via generic machine > > tree? > > > > > > > > > > > Igor Mammedov (3): > > > sparc64: use memory_region_allocate_system_memory() only for '-m' > > > specified RAM > > > ppc: rs6000_mc: drop usage of memory_region_allocate_system_memory() > > > hppa: drop usage of memory_region_allocate_system_memory() for ROM > > > > > > hw/hppa/machine.c| 5 ++--- > > > hw/ppc/rs6000_mc.c | 15 ++- > > > hw/sparc64/niagara.c | 25 + > > > 3 files changed, 25 insertions(+), 20 deletions(-) > > >
Re: [PATCH 0/3] eliminate remaining places that abuse memory_region_allocate_system_memory()
On Thu, 10 Oct 2019 19:35:03 +0200 Igor Mammedov wrote: Forgot to actually CC Eduardo, > On Tue, 8 Oct 2019 07:33:15 -0400 > Igor Mammedov wrote: ... > Eduardo, > > This patches are fixing various machines across tree, so series does not > belong > to any particular arch specific tree, can you merge it via generic machine > tree? > > > > > > Igor Mammedov (3): > > sparc64: use memory_region_allocate_system_memory() only for '-m' > > specified RAM > > ppc: rs6000_mc: drop usage of memory_region_allocate_system_memory() > > hppa: drop usage of memory_region_allocate_system_memory() for ROM > > > > hw/hppa/machine.c| 5 ++--- > > hw/ppc/rs6000_mc.c | 15 ++- > > hw/sparc64/niagara.c | 25 + > > 3 files changed, 25 insertions(+), 20 deletions(-) > > > >
Re: [PATCH 0/3] eliminate remaining places that abuse memory_region_allocate_system_memory()
On Tue, 8 Oct 2019 07:33:15 -0400 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. Eduardo, This patches are fixing various machines across tree, so series does not belong to any particular arch specific tree, can you merge it via generic machine tree? > > > Igor Mammedov (3): > sparc64: use memory_region_allocate_system_memory() only for '-m' > specified RAM > ppc: rs6000_mc: drop usage of memory_region_allocate_system_memory() > hppa: drop usage of memory_region_allocate_system_memory() for ROM > > hw/hppa/machine.c| 5 ++--- > hw/ppc/rs6000_mc.c | 15 ++- > hw/sparc64/niagara.c | 25 + > 3 files changed, 25 insertions(+), 20 deletions(-) >
Re: [PATCH 0/3] eliminate remaining places that abuse memory_region_allocate_system_memory()
On Tue, 8 Oct 2019 14:41:25 +0200 Philippe Mathieu-Daudé 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)
Re: [PATCH 0/3] eliminate remaining places that abuse memory_region_allocate_system_memory()
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): -- >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)