[PATCH REPOST v3 11/80] arm/digic_boards: use memdev for RAM

Igor Mammedov posted 80 patches 6 years ago
Maintainers: Andrzej Zaborowski <balrogg@gmail.com>, Eduardo Habkost <ehabkost@redhat.com>, Andrew Jeffery <andrew@aj.id.au>, Artyom Tarasenko <atar4qemu@gmail.com>, Paul Burton <pburton@wavecomp.com>, Thomas Huth <thuth@redhat.com>, Fabien Chouteau <chouteau@adacore.com>, Radoslaw Biernacki <radoslaw.biernacki@linaro.org>, Christian Borntraeger <borntraeger@de.ibm.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, KONRAD Frederic <frederic.konrad@adacore.com>, Cornelia Huck <cohuck@redhat.com>, "Philippe Mathieu-Daudé" <philmd@redhat.com>, Helge Deller <deller@gmx.de>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Cédric Le Goater" <clg@kaod.org>, Joel Stanley <joel@jms.id.au>, Andrew Baumann <Andrew.Baumann@microsoft.com>, BALATON Zoltan <balaton@eik.bme.hu>, Rob Herring <robh@kernel.org>, Paolo Bonzini <pbonzini@redhat.com>, Leif Lindholm <leif.lindholm@linaro.org>, Aleksandar Markovic <amarkovic@wavecomp.com>, Halil Pasic <pasic@linux.ibm.com>, Alistair Francis <alistair@alistair23.me>, Laurent Vivier <lvivier@redhat.com>, Sergio Lopez <slp@redhat.com>, David Hildenbrand <david@redhat.com>, Jean-Christophe Dubois <jcd@tribudubois.net>, Aurelien Jarno <aurelien@aurel32.net>, Antony Pavlov <antonynpavlov@gmail.com>, "Hervé Poussineau" <hpoussin@reactos.org>, Andrey Smirnov <andrew.smirnov@gmail.com>, Jan Kiszka <jan.kiszka@web.de>, Thomas Huth <huth@tuxfamily.org>, Michael Walle <michael@walle.cc>, Igor Mammedov <imammedo@redhat.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Richard Henderson <rth@twiddle.net>, Peter Chubb <peter.chubb@nicta.com.au>, Peter Maydell <peter.maydell@linaro.org>, "Michael S. Tsirkin" <mst@redhat.com>, Aleksandar Rikalo <aleksandar.rikalo@rt-rk.com>, Beniamino Galvani <b.galvani@gmail.com>, David Gibson <david@gibson.dropbear.id.au>
There is a newer version of this series
[PATCH REPOST v3 11/80] arm/digic_boards: use memdev for RAM
Posted by Igor Mammedov 6 years ago
memory_region_allocate_system_memory() API is going away, so
replace it with memdev allocated MemoryRegion. The later is
initialized by generic code, so board only needs to opt in
to memdev scheme by providing
  MachineClass::default_ram_id
and using MachineState::ram instead of manually initializing
RAM memory region.

PS:
 remove no longer needed DigicBoardState

PS2:
 while at it add check for user supplied RAM size and error
 out if it mismatches board expected value.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v3:
  * move 'DigicState *s' declaration to the top of digic4_board_init()
v2:
  * fix format string causing build failure on 32-bit host
    (Philippe Mathieu-Daudé <philmd@redhat.com>)

CC: drjones@redhat.com
CC: antonynpavlov@gmail.com
CC: peter.maydell@linaro.org
CC: qemu-arm@nongnu.org
---
 hw/arm/digic_boards.c | 40 +++++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/hw/arm/digic_boards.c b/hw/arm/digic_boards.c
index ef3fc2b..518a63e 100644
--- a/hw/arm/digic_boards.c
+++ b/hw/arm/digic_boards.c
@@ -35,39 +35,40 @@
 #include "hw/loader.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/qtest.h"
+#include "qemu/units.h"
+#include "qemu/cutils.h"
 
 #define DIGIC4_ROM0_BASE      0xf0000000
 #define DIGIC4_ROM1_BASE      0xf8000000
 #define DIGIC4_ROM_MAX_SIZE   0x08000000
 
-typedef struct DigicBoardState {
-    DigicState *digic;
-    MemoryRegion ram;
-} DigicBoardState;
-
 typedef struct DigicBoard {
-    hwaddr ram_size;
-    void (*add_rom0)(DigicBoardState *, hwaddr, const char *);
+    void (*add_rom0)(DigicState *, hwaddr, const char *);
     const char *rom0_def_filename;
-    void (*add_rom1)(DigicBoardState *, hwaddr, const char *);
+    void (*add_rom1)(DigicState *, hwaddr, const char *);
     const char *rom1_def_filename;
 } DigicBoard;
 
-static void digic4_board_init(DigicBoard *board)
+static void digic4_board_init(MachineState *machine, DigicBoard *board)
 {
     Error *err = NULL;
+    DigicState *s = DIGIC(object_new(TYPE_DIGIC));
+    MachineClass *mc = MACHINE_GET_CLASS(machine);
+
+    if (machine->ram_size != mc->default_ram_size) {
+        char *sz = size_to_str(mc->default_ram_size);
+        error_report("Invalid RAM size, should be %s", sz);
+        g_free(sz);
+        exit(EXIT_FAILURE);
+    }
 
-    DigicBoardState *s = g_new(DigicBoardState, 1);
-
-    s->digic = DIGIC(object_new(TYPE_DIGIC));
-    object_property_set_bool(OBJECT(s->digic), true, "realized", &err);
+    object_property_set_bool(OBJECT(s), true, "realized", &err);
     if (err != NULL) {
         error_reportf_err(err, "Couldn't realize DIGIC SoC: ");
         exit(1);
     }
 
-    memory_region_allocate_system_memory(&s->ram, NULL, "ram", board->ram_size);
-    memory_region_add_subregion(get_system_memory(), 0, &s->ram);
+    memory_region_add_subregion(get_system_memory(), 0, machine->ram);
 
     if (board->add_rom0) {
         board->add_rom0(s, DIGIC4_ROM0_BASE, board->rom0_def_filename);
@@ -78,7 +79,7 @@ static void digic4_board_init(DigicBoard *board)
     }
 }
 
-static void digic_load_rom(DigicBoardState *s, hwaddr addr,
+static void digic_load_rom(DigicState *s, hwaddr addr,
                            hwaddr max_size, const char *def_filename)
 {
     target_long rom_size;
@@ -118,7 +119,7 @@ static void digic_load_rom(DigicBoardState *s, hwaddr addr,
  * Samsung K8P3215UQB
  * 64M Bit (4Mx16) Page Mode / Multi-Bank NOR Flash Memory
  */
-static void digic4_add_k8p3215uqb_rom(DigicBoardState *s, hwaddr addr,
+static void digic4_add_k8p3215uqb_rom(DigicState *s, hwaddr addr,
                                       const char *def_filename)
 {
 #define FLASH_K8P3215UQB_SIZE (4 * 1024 * 1024)
@@ -135,14 +136,13 @@ static void digic4_add_k8p3215uqb_rom(DigicBoardState *s, hwaddr addr,
 }
 
 static DigicBoard digic4_board_canon_a1100 = {
-    .ram_size = 64 * 1024 * 1024,
     .add_rom1 = digic4_add_k8p3215uqb_rom,
     .rom1_def_filename = "canon-a1100-rom1.bin",
 };
 
 static void canon_a1100_init(MachineState *machine)
 {
-    digic4_board_init(&digic4_board_canon_a1100);
+    digic4_board_init(machine, &digic4_board_canon_a1100);
 }
 
 static void canon_a1100_machine_init(MachineClass *mc)
@@ -150,6 +150,8 @@ static void canon_a1100_machine_init(MachineClass *mc)
     mc->desc = "Canon PowerShot A1100 IS";
     mc->init = &canon_a1100_init;
     mc->ignore_memory_transaction_failures = true;
+    mc->default_ram_size = 64 * MiB;
+    mc->default_ram_id = "ram";
 }
 
 DEFINE_MACHINE("canon-a1100", canon_a1100_machine_init)
-- 
2.7.4


Re: [PATCH REPOST v3 11/80] arm/digic_boards: use memdev for RAM
Posted by Andrew Jones 6 years ago
On Thu, Jan 23, 2020 at 12:37:36PM +0100, Igor Mammedov wrote:
> memory_region_allocate_system_memory() API is going away, so
> replace it with memdev allocated MemoryRegion. The later is
> initialized by generic code, so board only needs to opt in
> to memdev scheme by providing
>   MachineClass::default_ram_id
> and using MachineState::ram instead of manually initializing
> RAM memory region.
> 
> PS:
>  remove no longer needed DigicBoardState
> 
> PS2:
>  while at it add check for user supplied RAM size and error
>  out if it mismatches board expected value.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v3:
>   * move 'DigicState *s' declaration to the top of digic4_board_init()
> v2:
>   * fix format string causing build failure on 32-bit host
>     (Philippe Mathieu-Daudé <philmd@redhat.com>)
> 
> CC: drjones@redhat.com
> CC: antonynpavlov@gmail.com
> CC: peter.maydell@linaro.org
> CC: qemu-arm@nongnu.org
> ---
>  hw/arm/digic_boards.c | 40 +++++++++++++++++++++-------------------
>  1 file changed, 21 insertions(+), 19 deletions(-)
>

I don't know anything about digic_boards, but I promised Igor to put
another pair of eyes on his changes. Looks fine to me.

Reviewed-by: Andrew Jones <drjones@redhat.com>