[PATCH 0/6] hw/arm/raspi: Dynamically create machines based on the board revision

[PATCH 0/6] hw/arm/raspi: Dynamically create machines based on the board revision

[Philippe Mathieu-Daudé]

Hi,

These patches simplify adding the raspi0/raspi1/raspi4 boards.

I still need some time to figure out what is the best way to
organize the SoC code because since these socs use different
ARM cores, the interrupt controller are different too.

Igor has been working in his "refactor main RAM allocation to use
hostmem backend" series, and now v4 [1] is almost reviewed.
His raspi patch [2] clashes with my work, Since it is easier for
him to apply his on top of mine, I am sending these patches first.

Please review,

Phil.

[1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg675738.html
[2] https://www.mail-archive.com/qemu-devel@nongnu.org/msg675752.html

Philippe Mathieu-Daudé (6):
  hw/arm/raspi: Use BCM2708 machine type with pre Device Tree kernels
  hw/arm/raspi: Get board version from board revision code
  hw/arm/raspi: Get the SoC type name from the revision code
  hw/arm/raspi: Get board RAM size from board revision code
  hw/arm/raspi: Dynamically create machines based on the board revision
  hw/arm/raspi: Get the CPU core count from the revision code

 hw/arm/raspi.c | 178 +++++++++++++++++++++++++++++++++++++------------
 1 file changed, 135 insertions(+), 43 deletions(-)

-- 
2.21.1

[PATCH 1/6] hw/arm/raspi: Use BCM2708 machine type with pre Device Tree kernels

[Philippe Mathieu-Daudé]

When booting without device tree, the Linux kernels uses the $R1
register to determine the machine type. The list of values is
registered at [1].

There are two entries for the Raspberry Pi:

- https://www.arm.linux.org.uk/developer/machines/list.php?mid=3138
  name: MACH_TYPE_BCM2708
  value: 0xc42 (3138)
  status: Active, not mainlined
  date: 15 Oct 2010

- https://www.arm.linux.org.uk/developer/machines/list.php?mid=4828
  name: MACH_TYPE_BCM2835
  value: 4828
  status: Active, mainlined
  date: 6 Dec 2013

QEMU always used the non-mainlined type MACH_TYPE_BCM2708.
The value 0xc43 is registered to 'MX51_GGC' (processor i.MX51), and
0xc44 to 'Western Digital Sharespace NAS' (processor Marvell 88F5182).

The Raspberry Pi foundation bootloader only sets the BCM2708 machine
type, see [2] or [3]:

 133 9:
 134     mov r0, #0
 135     ldr r1, =3138       @ BCM2708 machine id
 136     ldr r2, atags       @ ATAGS
 137     bx  r4

U-Boot only uses MACH_TYPE_BCM2708 (see [4]):

 25 /*
 26  * 2835 is a SKU in a series for which the 2708 is the first or primary SoC,
 27  * so 2708 has historically been used rather than a dedicated 2835 ID.
 28  *
 29  * We don't define a machine type for bcm2709/bcm2836 since the RPi Foundation
 30  * chose to use someone else's previously registered machine ID (3139, MX51_GGC)
 31  * rather than obtaining a valid ID:-/
 32  *
 33  * For the bcm2837, hopefully a machine type is not needed, since everything
 34  * is DT.
 35  */

While the definition MACH_BCM2709 with value 0xc43 was introduced in
a commit described "Add 2709 platform for Raspberry Pi 2" out of the
mainline Linux kernel, it does not seem used, and the platform is
introduced with Device Tree support anyway (see [5] and [6]).

Remove the unused values (0xc43 introduced in commit 1df7d1f9303aef
"raspi: add raspberry pi 2 machine" and 0xc44 in commit bade58166f4
"raspi: Raspberry Pi 3 support"), keeping only MACH_TYPE_BCM2708.

[1] https://www.arm.linux.org.uk/developer/machines/
[2] https://github.com/raspberrypi/tools/blob/920c7ed2e/armstubs/armstub7.S#L135
[3] https://github.com/raspberrypi/tools/blob/49719d554/armstubs/armstub7.S#L64
[4] https://gitlab.denx.de/u-boot/u-boot/blob/v2015.04/include/configs/rpi-common.h#L18
[5] https://github.com/raspberrypi/linux/commit/d9fac63adac#diff-6722037d79570df5b392a49e0e006573R526
[6] http://lists.infradead.org/pipermail/linux-rpi-kernel/2015-February/001268.html

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Cc: Zoltán Baldaszti <bztemail@gmail.com>
Cc: Pekka Enberg <penberg@iki.fi>
Cc: Stephen Warren <swarren@nvidia.com>
Cc: Kshitij Soni <kshitij.soni@broadcom.com>
Cc: Michael Chan <michael.chan@broadcom.com>
Cc: Andrew Baumann <Andrew.Baumann@microsoft.com>
Cc: Jeremy Linton <lintonrjeremy@gmail.com>
Cc: Pete Batard <pete@akeo.ie>
---
 hw/arm/raspi.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index 3996f6c63a..ef76a27f33 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -29,8 +29,7 @@
 #define FIRMWARE_ADDR_3 0x80000 /* Pi 3 loads kernel.img here by default */
 #define SPINTABLE_ADDR  0xd8 /* Pi 3 bootloader spintable */
 
-/* Table of Linux board IDs for different Pi versions */
-static const int raspi_boardid[] = {[1] = 0xc42, [2] = 0xc43, [3] = 0xc44};
+#define MACH_TYPE_BCM2708   3138 /* Linux board IDs */
 
 typedef struct RasPiState {
     BCM283XState soc;
@@ -116,7 +115,7 @@ static void setup_boot(MachineState *machine, int version, size_t ram_size)
     static struct arm_boot_info binfo;
     int r;
 
-    binfo.board_id = raspi_boardid[version];
+    binfo.board_id = MACH_TYPE_BCM2708;
     binfo.ram_size = ram_size;
     binfo.nb_cpus = machine->smp.cpus;
 
-- 
2.21.1

Re: [PATCH 1/6] hw/arm/raspi: Use BCM2708 machine type with pre Device Tree kernels

[Alistair Francis]

On Mon, Feb 3, 2020 at 12:28 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> When booting without device tree, the Linux kernels uses the $R1
> register to determine the machine type. The list of values is
> registered at [1].
>
> There are two entries for the Raspberry Pi:
>
> - https://www.arm.linux.org.uk/developer/machines/list.php?mid=3138
>   name: MACH_TYPE_BCM2708
>   value: 0xc42 (3138)
>   status: Active, not mainlined
>   date: 15 Oct 2010
>
> - https://www.arm.linux.org.uk/developer/machines/list.php?mid=4828
>   name: MACH_TYPE_BCM2835
>   value: 4828
>   status: Active, mainlined
>   date: 6 Dec 2013
>
> QEMU always used the non-mainlined type MACH_TYPE_BCM2708.
> The value 0xc43 is registered to 'MX51_GGC' (processor i.MX51), and
> 0xc44 to 'Western Digital Sharespace NAS' (processor Marvell 88F5182).
>
> The Raspberry Pi foundation bootloader only sets the BCM2708 machine
> type, see [2] or [3]:
>
>  133 9:
>  134     mov r0, #0
>  135     ldr r1, =3138       @ BCM2708 machine id
>  136     ldr r2, atags       @ ATAGS
>  137     bx  r4
>
> U-Boot only uses MACH_TYPE_BCM2708 (see [4]):
>
>  25 /*
>  26  * 2835 is a SKU in a series for which the 2708 is the first or primary SoC,
>  27  * so 2708 has historically been used rather than a dedicated 2835 ID.
>  28  *
>  29  * We don't define a machine type for bcm2709/bcm2836 since the RPi Foundation
>  30  * chose to use someone else's previously registered machine ID (3139, MX51_GGC)
>  31  * rather than obtaining a valid ID:-/
>  32  *
>  33  * For the bcm2837, hopefully a machine type is not needed, since everything
>  34  * is DT.
>  35  */
>
> While the definition MACH_BCM2709 with value 0xc43 was introduced in
> a commit described "Add 2709 platform for Raspberry Pi 2" out of the
> mainline Linux kernel, it does not seem used, and the platform is
> introduced with Device Tree support anyway (see [5] and [6]).
>
> Remove the unused values (0xc43 introduced in commit 1df7d1f9303aef
> "raspi: add raspberry pi 2 machine" and 0xc44 in commit bade58166f4
> "raspi: Raspberry Pi 3 support"), keeping only MACH_TYPE_BCM2708.
>
> [1] https://www.arm.linux.org.uk/developer/machines/
> [2] https://github.com/raspberrypi/tools/blob/920c7ed2e/armstubs/armstub7.S#L135
> [3] https://github.com/raspberrypi/tools/blob/49719d554/armstubs/armstub7.S#L64
> [4] https://gitlab.denx.de/u-boot/u-boot/blob/v2015.04/include/configs/rpi-common.h#L18
> [5] https://github.com/raspberrypi/linux/commit/d9fac63adac#diff-6722037d79570df5b392a49e0e006573R526
> [6] http://lists.infradead.org/pipermail/linux-rpi-kernel/2015-February/001268.html
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

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

Alistair

> ---
> Cc: Zoltán Baldaszti <bztemail@gmail.com>
> Cc: Pekka Enberg <penberg@iki.fi>
> Cc: Stephen Warren <swarren@nvidia.com>
> Cc: Kshitij Soni <kshitij.soni@broadcom.com>
> Cc: Michael Chan <michael.chan@broadcom.com>
> Cc: Andrew Baumann <Andrew.Baumann@microsoft.com>
> Cc: Jeremy Linton <lintonrjeremy@gmail.com>
> Cc: Pete Batard <pete@akeo.ie>
> ---
>  hw/arm/raspi.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index 3996f6c63a..ef76a27f33 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -29,8 +29,7 @@
>  #define FIRMWARE_ADDR_3 0x80000 /* Pi 3 loads kernel.img here by default */
>  #define SPINTABLE_ADDR  0xd8 /* Pi 3 bootloader spintable */
>
> -/* Table of Linux board IDs for different Pi versions */
> -static const int raspi_boardid[] = {[1] = 0xc42, [2] = 0xc43, [3] = 0xc44};
> +#define MACH_TYPE_BCM2708   3138 /* Linux board IDs */
>
>  typedef struct RasPiState {
>      BCM283XState soc;
> @@ -116,7 +115,7 @@ static void setup_boot(MachineState *machine, int version, size_t ram_size)
>      static struct arm_boot_info binfo;
>      int r;
>
> -    binfo.board_id = raspi_boardid[version];
> +    binfo.board_id = MACH_TYPE_BCM2708;
>      binfo.ram_size = ram_size;
>      binfo.nb_cpus = machine->smp.cpus;
>
> --
> 2.21.1
>
>

[PATCH 2/6] hw/arm/raspi: Get board version from board revision code

[Philippe Mathieu-Daudé]

The chip ID is encoded in the board revision, and the board version
is simply 'chip_id + 1'.
We want to support more boards which follow the same scheme.
Introduce a new RaspiBoardInfo structure which we'll extend in the
following commits.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/arm/raspi.c | 45 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 39 insertions(+), 6 deletions(-)

diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index ef76a27f33..eaa8c49009 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -36,6 +36,39 @@ typedef struct RasPiState {
     MemoryRegion ram;
 } RasPiState;
 
+typedef struct RaspiBoardInfo {
+    /*
+     * Board revision codes:
+     * www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/
+     */
+    uint32_t board_rev;
+} RaspiBoardInfo;
+
+enum { BOARD_PI2, BOARD_PI3 };
+
+static const RaspiBoardInfo raspi_boards[] = {
+    [BOARD_PI2] =
+    {
+        .board_rev = 0xa21041,
+    },
+#ifdef TARGET_AARCH64
+    [BOARD_PI3] =
+    {
+        .board_rev = 0xa02082,
+    },
+#endif
+};
+
+static int board_chip_id(const RaspiBoardInfo *config)
+{
+    return extract32(config->board_rev, 12, 4);
+}
+
+static int board_version(const RaspiBoardInfo *config)
+{
+    return board_chip_id(config) + 1;
+}
+
 static void write_smpboot(ARMCPU *cpu, const struct arm_boot_info *info)
 {
     static const uint32_t smpboot[] = {
@@ -163,9 +196,10 @@ static void setup_boot(MachineState *machine, int version, size_t ram_size)
     arm_load_kernel(ARM_CPU(first_cpu), machine, &binfo);
 }
 
-static void raspi_init(MachineState *machine, int version)
+static void raspi_init(MachineState *machine, const RaspiBoardInfo *config)
 {
     RasPiState *s = g_new0(RasPiState, 1);
+    int version = board_version(config);
     uint32_t vcram_size;
     DriveInfo *di;
     BlockBackend *blk;
@@ -191,9 +225,8 @@ static void raspi_init(MachineState *machine, int version)
     /* Setup the SOC */
     object_property_add_const_link(OBJECT(&s->soc), "ram", OBJECT(&s->ram),
                                    &error_abort);
-    int board_rev = version == 3 ? 0xa02082 : 0xa21041;
-    object_property_set_int(OBJECT(&s->soc), board_rev, "board-rev",
-                            &error_abort);
+    object_property_set_int(OBJECT(&s->soc), config->board_rev,
+                            "board-rev", &error_abort);
     object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_abort);
 
     /* Create and plug in the SD cards */
@@ -215,7 +248,7 @@ static void raspi_init(MachineState *machine, int version)
 
 static void raspi2_init(MachineState *machine)
 {
-    raspi_init(machine, 2);
+    raspi_init(machine, &raspi_boards[BOARD_PI2]);
 }
 
 static void raspi2_machine_init(MachineClass *mc)
@@ -237,7 +270,7 @@ DEFINE_MACHINE("raspi2", raspi2_machine_init)
 #ifdef TARGET_AARCH64
 static void raspi3_init(MachineState *machine)
 {
-    raspi_init(machine, 3);
+    raspi_init(machine, &raspi_boards[BOARD_PI3]);
 }
 
 static void raspi3_machine_init(MachineClass *mc)
-- 
2.21.1

Re: [PATCH 2/6] hw/arm/raspi: Get board version from board revision code

[Igor Mammedov]

On Mon,  3 Feb 2020 09:26:15 +0100
Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:

> The chip ID is encoded in the board revision, and the board version
> is simply 'chip_id + 1'.
> We want to support more boards which follow the same scheme.
> Introduce a new RaspiBoardInfo structure which we'll extend in the
> following commits.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/arm/raspi.c | 45 +++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 39 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index ef76a27f33..eaa8c49009 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -36,6 +36,39 @@ typedef struct RasPiState {
>      MemoryRegion ram;
>  } RasPiState;
>  
> +typedef struct RaspiBoardInfo {
> +    /*
> +     * Board revision codes:
> +     * www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/
> +     */
> +    uint32_t board_rev;
> +} RaspiBoardInfo;
> +
> +enum { BOARD_PI2, BOARD_PI3 };
> +
> +static const RaspiBoardInfo raspi_boards[] = {
> +    [BOARD_PI2] =
> +    {
> +        .board_rev = 0xa21041,
> +    },
> +#ifdef TARGET_AARCH64
> +    [BOARD_PI3] =
> +    {
> +        .board_rev = 0xa02082,
> +    },
> +#endif
> +};

This patter was(is) used widely by embed boards before QOM
(I mean introducing various structures to keep boards configs
and then passing them around). Which I found were cumbersome
to deal with with working on ram refactoring.

But have you considered using machine classes which
perfectly feet the task, instead of repeating old pattern?



> +static int board_chip_id(const RaspiBoardInfo *config)
> +{
> +    return extract32(config->board_rev, 12, 4);
> +}
> +
> +static int board_version(const RaspiBoardInfo *config)
> +{
> +    return board_chip_id(config) + 1;
> +}
> +
>  static void write_smpboot(ARMCPU *cpu, const struct arm_boot_info *info)
>  {
>      static const uint32_t smpboot[] = {
> @@ -163,9 +196,10 @@ static void setup_boot(MachineState *machine, int version, size_t ram_size)
>      arm_load_kernel(ARM_CPU(first_cpu), machine, &binfo);
>  }
>  
> -static void raspi_init(MachineState *machine, int version)
> +static void raspi_init(MachineState *machine, const RaspiBoardInfo *config)
>  {
>      RasPiState *s = g_new0(RasPiState, 1);
> +    int version = board_version(config);
>      uint32_t vcram_size;
>      DriveInfo *di;
>      BlockBackend *blk;
> @@ -191,9 +225,8 @@ static void raspi_init(MachineState *machine, int version)
>      /* Setup the SOC */
>      object_property_add_const_link(OBJECT(&s->soc), "ram", OBJECT(&s->ram),
>                                     &error_abort);
> -    int board_rev = version == 3 ? 0xa02082 : 0xa21041;
> -    object_property_set_int(OBJECT(&s->soc), board_rev, "board-rev",
> -                            &error_abort);
> +    object_property_set_int(OBJECT(&s->soc), config->board_rev,
> +                            "board-rev", &error_abort);
>      object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_abort);
>  
>      /* Create and plug in the SD cards */
> @@ -215,7 +248,7 @@ static void raspi_init(MachineState *machine, int version)
>  
>  static void raspi2_init(MachineState *machine)
>  {
> -    raspi_init(machine, 2);
> +    raspi_init(machine, &raspi_boards[BOARD_PI2]);
>  }
>  
>  static void raspi2_machine_init(MachineClass *mc)
> @@ -237,7 +270,7 @@ DEFINE_MACHINE("raspi2", raspi2_machine_init)
>  #ifdef TARGET_AARCH64
>  static void raspi3_init(MachineState *machine)
>  {
> -    raspi_init(machine, 3);
> +    raspi_init(machine, &raspi_boards[BOARD_PI3]);
>  }
>  
>  static void raspi3_machine_init(MachineClass *mc)

[PATCH 3/6] hw/arm/raspi: Get the SoC type name from the revision code

[Philippe Mathieu-Daudé]

We already know the Chip ID, let's name it with the
board_soc_name() helper.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/arm/raspi.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index eaa8c49009..f5e54fe876 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -31,6 +31,18 @@
 
 #define MACH_TYPE_BCM2708   3138 /* Linux board IDs */
 
+enum BoardIdChip {
+    C_BCM2836 = 1,
+    C_BCM2837 = 2,
+};
+
+static const struct {
+    const char *soc_name;
+} soc_config[] = {
+    [C_BCM2836] = {TYPE_BCM2836},
+    [C_BCM2837] = {TYPE_BCM2837},
+};
+
 typedef struct RasPiState {
     BCM283XState soc;
     MemoryRegion ram;
@@ -69,6 +81,11 @@ static int board_version(const RaspiBoardInfo *config)
     return board_chip_id(config) + 1;
 }
 
+static const char *board_soc_name(const RaspiBoardInfo *config)
+{
+    return soc_config[board_chip_id(config)].soc_name;
+}
+
 static void write_smpboot(ARMCPU *cpu, const struct arm_boot_info *info)
 {
     static const uint32_t smpboot[] = {
@@ -213,8 +230,7 @@ static void raspi_init(MachineState *machine, const RaspiBoardInfo *config)
     }
 
     object_initialize_child(OBJECT(machine), "soc", &s->soc, sizeof(s->soc),
-                            version == 3 ? TYPE_BCM2837 : TYPE_BCM2836,
-                            &error_abort, NULL);
+                            board_soc_name(config), &error_abort, NULL);
 
     /* Allocate and map RAM */
     memory_region_allocate_system_memory(&s->ram, OBJECT(machine), "ram",
-- 
2.21.1

[PATCH 4/6] hw/arm/raspi: Get board RAM size from board revision code

[Philippe Mathieu-Daudé]

The amount of RAM is encoded in the board revision.
Add the board_ram_size() helper, it will allow us to quickly
support new boards.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/arm/raspi.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index f5e54fe876..656d834e07 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -13,6 +13,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/units.h"
+#include "qemu/cutils.h"
 #include "qapi/error.h"
 #include "cpu.h"
 #include "hw/arm/bcm2836.h"
@@ -71,6 +72,11 @@ static const RaspiBoardInfo raspi_boards[] = {
 #endif
 };
 
+static uint64_t board_ram_size(const RaspiBoardInfo *config)
+{
+    return 1 * MiB << extract32(config->board_rev, 20, 4);
+}
+
 static int board_chip_id(const RaspiBoardInfo *config)
 {
     return extract32(config->board_rev, 12, 4);
@@ -222,10 +228,13 @@ static void raspi_init(MachineState *machine, const RaspiBoardInfo *config)
     BlockBackend *blk;
     BusState *bus;
     DeviceState *carddev;
+    uint64_t ram_size;
 
-    if (machine->ram_size > 1 * GiB) {
-        error_report("Requested ram size is too large for this machine: "
-                     "maximum is 1GB");
+    ram_size = board_ram_size(config);
+    if (machine->ram_size != ram_size) {
+        char *size_str = size_to_str(ram_size);
+        error_report("This machine can only be used with %s", size_str);
+        g_free(size_str);
         exit(1);
     }
 
-- 
2.21.1

Re: [PATCH 4/6] hw/arm/raspi: Get board RAM size from board revision code

[Igor Mammedov]

On Mon,  3 Feb 2020 09:26:17 +0100
Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:

> The amount of RAM is encoded in the board revision.
> Add the board_ram_size() helper, it will allow us to quickly
> support new boards.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Acked-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/arm/raspi.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index f5e54fe876..656d834e07 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -13,6 +13,7 @@
>  
>  #include "qemu/osdep.h"
>  #include "qemu/units.h"
> +#include "qemu/cutils.h"
>  #include "qapi/error.h"
>  #include "cpu.h"
>  #include "hw/arm/bcm2836.h"
> @@ -71,6 +72,11 @@ static const RaspiBoardInfo raspi_boards[] = {
>  #endif
>  };
>  
> +static uint64_t board_ram_size(const RaspiBoardInfo *config)
> +{
> +    return 1 * MiB << extract32(config->board_rev, 20, 4);
> +}
> +
>  static int board_chip_id(const RaspiBoardInfo *config)
>  {
>      return extract32(config->board_rev, 12, 4);
> @@ -222,10 +228,13 @@ static void raspi_init(MachineState *machine, const RaspiBoardInfo *config)
>      BlockBackend *blk;
>      BusState *bus;
>      DeviceState *carddev;
> +    uint64_t ram_size;
>  
> -    if (machine->ram_size > 1 * GiB) {
> -        error_report("Requested ram size is too large for this machine: "
> -                     "maximum is 1GB");
> +    ram_size = board_ram_size(config);
> +    if (machine->ram_size != ram_size) {
> +        char *size_str = size_to_str(ram_size);
> +        error_report("This machine can only be used with %s", size_str);
> +        g_free(size_str);
>          exit(1);
>      }
>  

[PATCH 5/6] hw/arm/raspi: Dynamically create machines based on the board revision

[Philippe Mathieu-Daudé]

We added tiny helpers extracting different values from the board
revision, we can now create a full board based on its revision.
The 'raspi-common' type is common to all raspi boards.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/arm/raspi.c | 93 +++++++++++++++++++++++++++++++++-----------------
 1 file changed, 62 insertions(+), 31 deletions(-)

diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index 656d834e07..12dc4d1486 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -44,12 +44,17 @@ static const struct {
     [C_BCM2837] = {TYPE_BCM2837},
 };
 
-typedef struct RasPiState {
+typedef struct RaspiMachineState {
+    /*< private >*/
+    MachineState parent_obj;
+    /*< public >*/
     BCM283XState soc;
     MemoryRegion ram;
-} RasPiState;
+} RaspiMachineState;
 
 typedef struct RaspiBoardInfo {
+    const char *name;
+    const char *desc;
     /*
      * Board revision codes:
      * www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/
@@ -57,16 +62,32 @@ typedef struct RaspiBoardInfo {
     uint32_t board_rev;
 } RaspiBoardInfo;
 
-enum { BOARD_PI2, BOARD_PI3 };
+typedef struct RaspiMachineClass {
+    /*< private >*/
+    MachineClass parent_obj;
+    /*< public >*/
+    const RaspiBoardInfo *config;
+} RaspiMachineClass;
+
+#define TYPE_RASPI_MACHINE       MACHINE_TYPE_NAME("raspi-common")
+#define RASPI_MACHINE(obj) \
+    OBJECT_CHECK(RaspiMachineState, (obj), TYPE_RASPI_MACHINE)
+
+#define RASPI_MACHINE_CLASS(klass) \
+     OBJECT_CLASS_CHECK(RaspiMachineClass, (klass), TYPE_RASPI_MACHINE)
+#define RASPI_MACHINE_GET_CLASS(obj) \
+     OBJECT_GET_CLASS(RaspiMachineClass, (obj), TYPE_RASPI_MACHINE)
 
 static const RaspiBoardInfo raspi_boards[] = {
-    [BOARD_PI2] =
     {
+        .name = MACHINE_TYPE_NAME("raspi2"),
+        .desc = "Raspberry Pi 2B",
         .board_rev = 0xa21041,
     },
 #ifdef TARGET_AARCH64
-    [BOARD_PI3] =
     {
+        .name = MACHINE_TYPE_NAME("raspi3"),
+        .desc = "Raspberry Pi 3B",
         .board_rev = 0xa02082,
     },
 #endif
@@ -221,7 +242,7 @@ static void setup_boot(MachineState *machine, int version, size_t ram_size)
 
 static void raspi_init(MachineState *machine, const RaspiBoardInfo *config)
 {
-    RasPiState *s = g_new0(RasPiState, 1);
+    RaspiMachineState *s = RASPI_MACHINE(machine);
     int version = board_version(config);
     uint32_t vcram_size;
     DriveInfo *di;
@@ -271,15 +292,22 @@ static void raspi_init(MachineState *machine, const RaspiBoardInfo *config)
     setup_boot(machine, version, machine->ram_size - vcram_size);
 }
 
-static void raspi2_init(MachineState *machine)
+static void raspi_machine_init(MachineState *machine)
 {
-    raspi_init(machine, &raspi_boards[BOARD_PI2]);
+    RaspiMachineClass *rmc = RASPI_MACHINE_GET_CLASS(machine);
+
+    raspi_init(machine, rmc->config);
 }
 
-static void raspi2_machine_init(MachineClass *mc)
+static void raspi_machine_class_init(ObjectClass *oc, void *data)
 {
-    mc->desc = "Raspberry Pi 2";
-    mc->init = raspi2_init;
+    MachineClass *mc = MACHINE_CLASS(oc);
+    RaspiMachineClass *rmc = RASPI_MACHINE_CLASS(oc);
+    const RaspiBoardInfo *config = data;
+
+    rmc->config = config;
+    mc->desc = config->desc;
+    mc->init = raspi_machine_init;
     mc->block_default_type = IF_SD;
     mc->no_parallel = 1;
     mc->no_floppy = 1;
@@ -287,29 +315,32 @@ static void raspi2_machine_init(MachineClass *mc)
     mc->max_cpus = BCM283X_NCPUS;
     mc->min_cpus = BCM283X_NCPUS;
     mc->default_cpus = BCM283X_NCPUS;
-    mc->default_ram_size = 1 * GiB;
+    mc->default_ram_size = board_ram_size(config);
     mc->ignore_memory_transaction_failures = true;
+}
+
+static const TypeInfo raspi_machine_type = {
+    .name = TYPE_RASPI_MACHINE,
+    .parent = TYPE_MACHINE,
+    .instance_size = sizeof(RaspiMachineState),
+    .class_size = sizeof(RaspiMachineClass),
+    .abstract = true,
 };
-DEFINE_MACHINE("raspi2", raspi2_machine_init)
 
-#ifdef TARGET_AARCH64
-static void raspi3_init(MachineState *machine)
+static void raspi_machine_types(void)
 {
-    raspi_init(machine, &raspi_boards[BOARD_PI3]);
+    int i;
+
+    type_register_static(&raspi_machine_type);
+    for (i = 0; i < ARRAY_SIZE(raspi_boards); ++i) {
+        TypeInfo ti = {
+            .name       = raspi_boards[i].name,
+            .parent     = TYPE_RASPI_MACHINE,
+            .class_init = raspi_machine_class_init,
+            .class_data = (void *)&raspi_boards[i],
+        };
+        type_register(&ti);
+    }
 }
 
-static void raspi3_machine_init(MachineClass *mc)
-{
-    mc->desc = "Raspberry Pi 3";
-    mc->init = raspi3_init;
-    mc->block_default_type = IF_SD;
-    mc->no_parallel = 1;
-    mc->no_floppy = 1;
-    mc->no_cdrom = 1;
-    mc->max_cpus = BCM283X_NCPUS;
-    mc->min_cpus = BCM283X_NCPUS;
-    mc->default_cpus = BCM283X_NCPUS;
-    mc->default_ram_size = 1 * GiB;
-}
-DEFINE_MACHINE("raspi3", raspi3_machine_init)
-#endif
+type_init(raspi_machine_types)
-- 
2.21.1

Re: [PATCH 5/6] hw/arm/raspi: Dynamically create machines based on the board revision

[Igor Mammedov]

On Mon,  3 Feb 2020 09:26:18 +0100
Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:

> We added tiny helpers extracting different values from the board
> revision, we can now create a full board based on its revision.
> The 'raspi-common' type is common to all raspi boards.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/arm/raspi.c | 93 +++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 62 insertions(+), 31 deletions(-)
> 
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index 656d834e07..12dc4d1486 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -44,12 +44,17 @@ static const struct {
>      [C_BCM2837] = {TYPE_BCM2837},
>  };
>  
> -typedef struct RasPiState {
> +typedef struct RaspiMachineState {
> +    /*< private >*/
> +    MachineState parent_obj;
> +    /*< public >*/
>      BCM283XState soc;
>      MemoryRegion ram;
> -} RasPiState;
> +} RaspiMachineState;
>  
>  typedef struct RaspiBoardInfo {
> +    const char *name;
> +    const char *desc;
>      /*
>       * Board revision codes:
>       * www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/
> @@ -57,16 +62,32 @@ typedef struct RaspiBoardInfo {
>      uint32_t board_rev;
>  } RaspiBoardInfo;
>  
> -enum { BOARD_PI2, BOARD_PI3 };
> +typedef struct RaspiMachineClass {
> +    /*< private >*/
> +    MachineClass parent_obj;
> +    /*< public >*/
> +    const RaspiBoardInfo *config;
> +} RaspiMachineClass;
> +
> +#define TYPE_RASPI_MACHINE       MACHINE_TYPE_NAME("raspi-common")
> +#define RASPI_MACHINE(obj) \
> +    OBJECT_CHECK(RaspiMachineState, (obj), TYPE_RASPI_MACHINE)
> +
> +#define RASPI_MACHINE_CLASS(klass) \
> +     OBJECT_CLASS_CHECK(RaspiMachineClass, (klass), TYPE_RASPI_MACHINE)
> +#define RASPI_MACHINE_GET_CLASS(obj) \
> +     OBJECT_GET_CLASS(RaspiMachineClass, (obj), TYPE_RASPI_MACHINE)
>  
>  static const RaspiBoardInfo raspi_boards[] = {
> -    [BOARD_PI2] =
>      {
> +        .name = MACHINE_TYPE_NAME("raspi2"),
> +        .desc = "Raspberry Pi 2B",
>          .board_rev = 0xa21041,
>      },
>  #ifdef TARGET_AARCH64
> -    [BOARD_PI3] =
>      {
> +        .name = MACHINE_TYPE_NAME("raspi3"),
> +        .desc = "Raspberry Pi 3B",
>          .board_rev = 0xa02082,
>      },
>  #endif
> @@ -221,7 +242,7 @@ static void setup_boot(MachineState *machine, int version, size_t ram_size)
>  
>  static void raspi_init(MachineState *machine, const RaspiBoardInfo *config)
>  {
> -    RasPiState *s = g_new0(RasPiState, 1);
> +    RaspiMachineState *s = RASPI_MACHINE(machine);
>      int version = board_version(config);
>      uint32_t vcram_size;
>      DriveInfo *di;
> @@ -271,15 +292,22 @@ static void raspi_init(MachineState *machine, const RaspiBoardInfo *config)
>      setup_boot(machine, version, machine->ram_size - vcram_size);
>  }
>  
> -static void raspi2_init(MachineState *machine)
> +static void raspi_machine_init(MachineState *machine)
>  {
> -    raspi_init(machine, &raspi_boards[BOARD_PI2]);
> +    RaspiMachineClass *rmc = RASPI_MACHINE_GET_CLASS(machine);
> +
> +    raspi_init(machine, rmc->config);
>  }
>  
> -static void raspi2_machine_init(MachineClass *mc)
> +static void raspi_machine_class_init(ObjectClass *oc, void *data)
>  {
> -    mc->desc = "Raspberry Pi 2";
> -    mc->init = raspi2_init;
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +    RaspiMachineClass *rmc = RASPI_MACHINE_CLASS(oc);
> +    const RaspiBoardInfo *config = data;
> +
> +    rmc->config = config;
> +    mc->desc = config->desc;
> +    mc->init = raspi_machine_init;
>      mc->block_default_type = IF_SD;
>      mc->no_parallel = 1;
>      mc->no_floppy = 1;
> @@ -287,29 +315,32 @@ static void raspi2_machine_init(MachineClass *mc)
>      mc->max_cpus = BCM283X_NCPUS;
>      mc->min_cpus = BCM283X_NCPUS;
>      mc->default_cpus = BCM283X_NCPUS;
> -    mc->default_ram_size = 1 * GiB;
> +    mc->default_ram_size = board_ram_size(config);
>      mc->ignore_memory_transaction_failures = true;
> +}
> +
> +static const TypeInfo raspi_machine_type = {
> +    .name = TYPE_RASPI_MACHINE,
> +    .parent = TYPE_MACHINE,
> +    .instance_size = sizeof(RaspiMachineState),
> +    .class_size = sizeof(RaspiMachineClass),
> +    .abstract = true,
>  };
> -DEFINE_MACHINE("raspi2", raspi2_machine_init)
>  
> -#ifdef TARGET_AARCH64
> -static void raspi3_init(MachineState *machine)
> +static void raspi_machine_types(void)
>  {
> -    raspi_init(machine, &raspi_boards[BOARD_PI3]);
> +    int i;
> +
> +    type_register_static(&raspi_machine_type);
> +    for (i = 0; i < ARRAY_SIZE(raspi_boards); ++i) {
> +        TypeInfo ti = {
> +            .name       = raspi_boards[i].name,
> +            .parent     = TYPE_RASPI_MACHINE,
> +            .class_init = raspi_machine_class_init,
> +            .class_data = (void *)&raspi_boards[i],
> +        };
> +        type_register(&ti);
> +    }

from what I've seen we do something like this in cases where
we need to generate a lot of classes based on data.

The thing is that it's harder to deal with compared to typical
template where one defines machine classes and per machine
type foo_class_init() functions. Yes it's a bit more boiler
plate but it's what is typically used/consistent with machine types.
So if one would have touch touch that future, one wouldn't have
to wrap their brain around yet another way to define machine.

>  }
>  
> -static void raspi3_machine_init(MachineClass *mc)
> -{
> -    mc->desc = "Raspberry Pi 3";
> -    mc->init = raspi3_init;
> -    mc->block_default_type = IF_SD;
> -    mc->no_parallel = 1;
> -    mc->no_floppy = 1;
> -    mc->no_cdrom = 1;
> -    mc->max_cpus = BCM283X_NCPUS;
> -    mc->min_cpus = BCM283X_NCPUS;
> -    mc->default_cpus = BCM283X_NCPUS;
> -    mc->default_ram_size = 1 * GiB;
> -}
> -DEFINE_MACHINE("raspi3", raspi3_machine_init)
> -#endif
> +type_init(raspi_machine_types)

[PATCH 6/6] hw/arm/raspi: Get the CPU core count from the revision code

[Philippe Mathieu-Daudé]

The number of cores is tied to the SoC, and we already know how
to extract the Chip ID from the revision code. Use it to determine
the number of cores.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/arm/raspi.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index 12dc4d1486..aff5d57261 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -39,9 +39,10 @@ enum BoardIdChip {
 
 static const struct {
     const char *soc_name;
+    int cores_count;
 } soc_config[] = {
-    [C_BCM2836] = {TYPE_BCM2836},
-    [C_BCM2837] = {TYPE_BCM2837},
+    [C_BCM2836] = {TYPE_BCM2836, BCM283X_NCPUS},
+    [C_BCM2837] = {TYPE_BCM2837, BCM283X_NCPUS},
 };
 
 typedef struct RaspiMachineState {
@@ -113,6 +114,11 @@ static const char *board_soc_name(const RaspiBoardInfo *config)
     return soc_config[board_chip_id(config)].soc_name;
 }
 
+static int cpu_cores_count(const RaspiBoardInfo *config)
+{
+    return soc_config[board_chip_id(config)].cores_count;
+}
+
 static void write_smpboot(ARMCPU *cpu, const struct arm_boot_info *info)
 {
     static const uint32_t smpboot[] = {
@@ -312,9 +318,7 @@ static void raspi_machine_class_init(ObjectClass *oc, void *data)
     mc->no_parallel = 1;
     mc->no_floppy = 1;
     mc->no_cdrom = 1;
-    mc->max_cpus = BCM283X_NCPUS;
-    mc->min_cpus = BCM283X_NCPUS;
-    mc->default_cpus = BCM283X_NCPUS;
+    mc->default_cpus = mc->min_cpus = mc->max_cpus = cpu_cores_count(config);
     mc->default_ram_size = board_ram_size(config);
     mc->ignore_memory_transaction_failures = true;
 }
-- 
2.21.1

Re: [PATCH 0/6] hw/arm/raspi: Dynamically create machines based on the board revision

[no-reply]

Patchew URL: https://patchew.org/QEMU/20200203082619.7426-1-f4bug@amsat.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [PATCH 0/6] hw/arm/raspi: Dynamically create machines based on the board revision
Message-id: 20200203082619.7426-1-f4bug@amsat.org
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Switched to a new branch 'test'
ba9fce5 hw/arm/raspi: Get the CPU core count from the revision code
dd68e6d hw/arm/raspi: Dynamically create machines based on the board revision
d471baa hw/arm/raspi: Get board RAM size from board revision code
97ae39f hw/arm/raspi: Get the SoC type name from the revision code
cb64a6d hw/arm/raspi: Get board version from board revision code
a4b6fc3 hw/arm/raspi: Use BCM2708 machine type with pre Device Tree kernels

=== OUTPUT BEGIN ===
1/6 Checking commit a4b6fc3d4ca9 (hw/arm/raspi: Use BCM2708 machine type with pre Device Tree kernels)
2/6 Checking commit cb64a6de4571 (hw/arm/raspi: Get board version from board revision code)
ERROR: that open brace { should be on the previous line
#36: FILE: hw/arm/raspi.c:51:
+    [BOARD_PI2] =
+    {

ERROR: that open brace { should be on the previous line
#41: FILE: hw/arm/raspi.c:56:
+    [BOARD_PI3] =
+    {

total: 2 errors, 0 warnings, 77 lines checked

Patch 2/6 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/6 Checking commit 97ae39f1b2a1 (hw/arm/raspi: Get the SoC type name from the revision code)
4/6 Checking commit d471baa88380 (hw/arm/raspi: Get board RAM size from board revision code)
5/6 Checking commit dd68e6dcaa11 (hw/arm/raspi: Dynamically create machines based on the board revision)
6/6 Checking commit ba9fce566944 (hw/arm/raspi: Get the CPU core count from the revision code)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20200203082619.7426-1-f4bug@amsat.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com