hw: Let the machine be the owner of the system memory

[PATCH 21/21] hw/core: Assert memory_region_allocate_system_memory has machine owner

Philippe Mathieu-Daudé posted 21 patches 6 years, 3 months ago

Download mbox

Maintainers: KONRAD Frederic <frederic.konrad@adacore.com>, "Cédric Le Goater" <clg@kaod.org>, Artyom Tarasenko <atar4qemu@gmail.com>, Thomas Huth <huth@tuxfamily.org>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Beniamino Galvani <b.galvani@gmail.com>, "Hervé Poussineau" <hpoussin@reactos.org>, Aleksandar Rikalo <arikalo@wavecomp.com>, Alistair Francis <alistair@alistair23.me>, Rob Herring <robh@kernel.org>, Andrzej Zaborowski <balrogg@gmail.com>, Leif Lindholm <leif.lindholm@linaro.org>, Aleksandar Markovic <amarkovic@wavecomp.com>, Andrew Jeffery <andrew@aj.id.au>, Richard Henderson <rth@twiddle.net>, Aurelien Jarno <aurelien@aurel32.net>, Antony Pavlov <antonynpavlov@gmail.com>, Andrey Smirnov <andrew.smirnov@gmail.com>, David Gibson <david@gibson.dropbear.id.au>, Jean-Christophe Dubois <jcd@tribudubois.net>, Peter Chubb <peter.chubb@nicta.com.au>, Eduardo Habkost <ehabkost@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Philippe Mathieu-Daudé" <philmd@redhat.com>, Joel Stanley <joel@jms.id.au>, Jan Kiszka <jan.kiszka@web.de>, Fabien Chouteau <chouteau@adacore.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Andrew Baumann <Andrew.Baumann@microsoft.com>, Radoslaw Biernacki <radoslaw.biernacki@linaro.org>, Peter Maydell <peter.maydell@linaro.org>, Helge Deller <deller@gmx.de>, Michael Walle <michael@walle.cc>, Paul Burton <pburton@wavecomp.com>

Expand all Fold all

[PATCH 21/21] hw/core: Assert memory_region_allocate_system_memory has machine owner

Posted by Philippe Mathieu-Daudé 6 years, 3 months ago

All the memory_region_allocate_system_memory() pass a MachineState
argument. Add an assertion to ensure the new boards/machines added
set this argument, so all system memory object have the machine as
its QOM owner.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/core/numa.c      | 4 +---
 include/hw/boards.h | 2 +-
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/core/numa.c b/hw/core/numa.c
index 2e29e4bfe0..3e07e94d00 100644
--- a/hw/core/numa.c
+++ b/hw/core/numa.c
@@ -527,9 +527,7 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, MachineState *ms,
     uint64_t addr = 0;
     int i;
 
-    if (!ms) {
-        ms = MACHINE(qdev_get_machine());
-    }
+    g_assert(ms);
 
     if (ms->numa_state == NULL ||
         ms->numa_state->num_nodes == 0 || !have_memdevs) {
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 3b6cb82b6c..31ab6e7586 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -14,7 +14,7 @@
 /**
  * memory_region_allocate_system_memory - Allocate a board's main memory
  * @mr: the #MemoryRegion to be initialized
- * @ms: the #MachineState object that own the system memory
+ * @ms: the #MachineState object that own the system memory (must not be NULL)
  * @name: name of the memory region
  * @ram_size: size of the region in bytes
  *
-- 
2.21.0

Re: [PATCH 21/21] hw/core: Assert memory_region_allocate_system_memory has machine owner

Posted by Richard Henderson 6 years, 3 months ago

On 10/20/19 3:56 PM, Philippe Mathieu-Daudé wrote:
> All the memory_region_allocate_system_memory() pass a MachineState
> argument. Add an assertion to ensure the new boards/machines added
> set this argument, so all system memory object have the machine as
> its QOM owner.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/core/numa.c      | 4 +---
>  include/hw/boards.h | 2 +-
>  2 files changed, 2 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

It would be a good idea to add __attribute__((nonnull(x,y,z))) as well, but
since we don't currently have any markup for that in qemu, that can go in a
separate patch set.


r~

Re: [PATCH 21/21] hw/core: Assert memory_region_allocate_system_memory has machine owner

Posted by Alistair Francis 6 years, 3 months ago

On Sun, Oct 20, 2019 at 4:22 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> All the memory_region_allocate_system_memory() pass a MachineState
> argument. Add an assertion to ensure the new boards/machines added
> set this argument, so all system memory object have the machine as
> its QOM owner.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  hw/core/numa.c      | 4 +---
>  include/hw/boards.h | 2 +-
>  2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/hw/core/numa.c b/hw/core/numa.c
> index 2e29e4bfe0..3e07e94d00 100644
> --- a/hw/core/numa.c
> +++ b/hw/core/numa.c
> @@ -527,9 +527,7 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, MachineState *ms,
>      uint64_t addr = 0;
>      int i;
>
> -    if (!ms) {
> -        ms = MACHINE(qdev_get_machine());
> -    }
> +    g_assert(ms);
>
>      if (ms->numa_state == NULL ||
>          ms->numa_state->num_nodes == 0 || !have_memdevs) {
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 3b6cb82b6c..31ab6e7586 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -14,7 +14,7 @@
>  /**
>   * memory_region_allocate_system_memory - Allocate a board's main memory
>   * @mr: the #MemoryRegion to be initialized
> - * @ms: the #MachineState object that own the system memory
> + * @ms: the #MachineState object that own the system memory (must not be NULL)
>   * @name: name of the memory region
>   * @ram_size: size of the region in bytes
>   *
> --
> 2.21.0
>
>