All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v1 0/3] Raspberry Pi 3 support
@ 2018-02-08  5:50 Pekka Enberg
  2018-02-08  5:50 ` [Qemu-devel] [PATCH v1 2/3] raspi: " Pekka Enberg
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Pekka Enberg @ 2018-02-08  5:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, qemu-arm, Pekka Enberg

This patch series adds support for Raspberry Pi 3 as a new machine model
"raspi3", which is an extension of the "raspi2" model with the following
differences:

  - Default CPU type is "cortex-a53"
  - Firmware is at address 0x80000
  - Board ID is 0xc44 and board revision is 0xa02082

The patches were written by me but I used Zoltán Baldaszti's previous
work as a reference (with permission from the author):
    
  https://github.com/bztsrc/qemu-raspi3

Also available from:

  git@github.com:penberg/qemu.git raspi3/v1

Pekka Enberg (3):
  bcm2836: Make CPU type configurable
  raspi: Raspberry Pi 3 support
  raspi: Add "raspi3" machine type

 hw/arm/bcm2836.c         | 17 ++++++++-------
 hw/arm/raspi.c           | 55 +++++++++++++++++++++++++++++++++++++++---------
 include/hw/arm/bcm2836.h |  1 +
 3 files changed, 55 insertions(+), 18 deletions(-)

-- 
2.14.3

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH v1 2/3] raspi: Raspberry Pi 3 support
  2018-02-08  5:50 [Qemu-devel] [PATCH v1 0/3] Raspberry Pi 3 support Pekka Enberg
@ 2018-02-08  5:50 ` Pekka Enberg
  2018-02-08  5:50 ` [Qemu-devel] [PATCH v1 3/3] raspi: Add "raspi3" machine type Pekka Enberg
       [not found] ` <20180208055039.24666-2-penberg@iki.fi>
  2 siblings, 0 replies; 11+ messages in thread
From: Pekka Enberg @ 2018-02-08  5:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, qemu-arm, Pekka Enberg

This patch adds Raspberry Pi 3 support to hw/arm/raspi.c. The
differences to Pi 2 are:

 - Firmware address
 - Board ID
 - Board revision

The CPU is different too, but that's going to be configured as part of
the machine default CPU when we introduce a new machine type.

The patch was written from scratch by me but the logic is similar to
Zoltán Baldaszti's previous work, which I used as a reference (with
permission from the author):

  https://github.com/bztsrc/qemu-raspi3

Signed-off-by: Pekka Enberg <penberg@iki.fi>
---
 hw/arm/raspi.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index c24a4a1b14..66fe10e376 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -5,6 +5,9 @@
  * Rasperry Pi 2 emulation Copyright (c) 2015, Microsoft
  * Written by Andrew Baumann
  *
+ * Raspberry Pi 3 emulation Copyright (c) 2018 Zoltán Baldaszti
+ * Upstream code cleanup (c) 2018 Pekka Enberg 
+ *
  * This code is licensed under the GNU GPLv2 and later.
  */
 
@@ -22,10 +25,11 @@
 #define SMPBOOT_ADDR    0x300 /* this should leave enough space for ATAGS */
 #define MVBAR_ADDR      0x400 /* secure vectors */
 #define BOARDSETUP_ADDR (MVBAR_ADDR + 0x20) /* board setup code */
-#define FIRMWARE_ADDR   0x8000 /* Pi loads kernel.img here by default */
+#define FIRMWARE_ADDR_2 0x8000 /* Pi 2 loads kernel.img here by default */
+#define FIRMWARE_ADDR_3 0x80000 /* Pi 3 loads kernel.img here by default */
 
 /* Table of Linux board IDs for different Pi versions */
-static const int raspi_boardid[] = {[1] = 0xc42, [2] = 0xc43};
+static const int raspi_boardid[] = {[1] = 0xc42, [2] = 0xc43, [3] = 0xc44};
 
 typedef struct RasPiState {
     BCM2836State soc;
@@ -83,8 +87,8 @@ static void setup_boot(MachineState *machine, int version, size_t ram_size)
     binfo.secure_board_setup = true;
     binfo.secure_boot = true;
 
-    /* Pi2 requires SMP setup */
-    if (version == 2) {
+    /* Pi2 and Pi3 requires SMP setup */
+    if (version >= 2) {
         binfo.smp_loader_start = SMPBOOT_ADDR;
         binfo.write_secondary_boot = write_smpboot;
         binfo.secondary_cpu_reset_hook = reset_secondary;
@@ -94,15 +98,16 @@ static void setup_boot(MachineState *machine, int version, size_t ram_size)
      * the normal Linux boot process
      */
     if (machine->firmware) {
+        hwaddr firmware_addr = version == 3 ? FIRMWARE_ADDR_3 : FIRMWARE_ADDR_2;
         /* load the firmware image (typically kernel.img) */
-        r = load_image_targphys(machine->firmware, FIRMWARE_ADDR,
-                                ram_size - FIRMWARE_ADDR);
+        r = load_image_targphys(machine->firmware, firmware_addr,
+                                ram_size - firmware_addr);
         if (r < 0) {
             error_report("Failed to load firmware from %s", machine->firmware);
             exit(1);
         }
 
-        binfo.entry = FIRMWARE_ADDR;
+        binfo.entry = firmware_addr;
         binfo.firmware_loaded = true;
     } else {
         binfo.kernel_filename = machine->kernel_filename;
@@ -113,7 +118,7 @@ static void setup_boot(MachineState *machine, int version, size_t ram_size)
     arm_load_kernel(ARM_CPU(first_cpu), &binfo);
 }
 
-static void raspi2_init(MachineState *machine)
+static void raspi_init(MachineState *machine, int version)
 {
     RasPiState *s = g_new0(RasPiState, 1);
     uint32_t vcram_size;
@@ -139,7 +144,8 @@ static void raspi2_init(MachineState *machine)
                             &error_abort);
     object_property_set_int(OBJECT(&s->soc), smp_cpus, "enabled-cpus",
                             &error_abort);
-    object_property_set_int(OBJECT(&s->soc), 0xa21041, "board-rev",
+    int board_rev = version == 3 ? 0xa02082 : 0xa21041;
+    object_property_set_int(OBJECT(&s->soc), board_rev, "board-rev",
                             &error_abort);
     object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_abort);
 
@@ -157,7 +163,12 @@ static void raspi2_init(MachineState *machine)
 
     vcram_size = object_property_get_uint(OBJECT(&s->soc), "vcram-size",
                                           &error_abort);
-    setup_boot(machine, 2, machine->ram_size - vcram_size);
+    setup_boot(machine, version, machine->ram_size - vcram_size);
+}
+
+static void raspi2_init(MachineState *machine)
+{
+    raspi_init(machine, 2);
 }
 
 static void raspi2_machine_init(MachineClass *mc)
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH v1 3/3] raspi: Add "raspi3" machine type
  2018-02-08  5:50 [Qemu-devel] [PATCH v1 0/3] Raspberry Pi 3 support Pekka Enberg
  2018-02-08  5:50 ` [Qemu-devel] [PATCH v1 2/3] raspi: " Pekka Enberg
@ 2018-02-08  5:50 ` Pekka Enberg
  2018-02-15 12:39   ` Peter Maydell
       [not found] ` <20180208055039.24666-2-penberg@iki.fi>
  2 siblings, 1 reply; 11+ messages in thread
From: Pekka Enberg @ 2018-02-08  5:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, qemu-arm, Pekka Enberg

This patch adds a "raspi3" machine type, which can now be selected as
the machine to run on by users via the "-M" command line option to QEMU.

The machine type does *not* ignore memory transaction failures so we
likely need to add some dummy devices later when people run something
more complicated than what I'm using for testing.

Signed-off-by: Pekka Enberg <penberg@iki.fi>
---
 hw/arm/raspi.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index 66fe10e376..048ff23a51 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -187,3 +187,24 @@ static void raspi2_machine_init(MachineClass *mc)
     mc->ignore_memory_transaction_failures = true;
 };
 DEFINE_MACHINE("raspi2", raspi2_machine_init)
+
+static void raspi3_init(MachineState *machine)
+{
+    raspi_init(machine, 3);
+}
+
+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->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a53");
+    mc->max_cpus = BCM2836_NCPUS;
+    mc->min_cpus = BCM2836_NCPUS;
+    mc->default_cpus = BCM2836_NCPUS;
+    mc->default_ram_size = 1024 * 1024 * 1024;
+}
+DEFINE_MACHINE("raspi3", raspi3_machine_init)
-- 
2.14.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH v1 1/3] bcm2836: Make CPU type configurable
       [not found] ` <20180208055039.24666-2-penberg@iki.fi>
@ 2018-02-15 11:48   ` Peter Maydell
  2018-02-16  7:04     ` Pekka Enberg
  0 siblings, 1 reply; 11+ messages in thread
From: Peter Maydell @ 2018-02-15 11:48 UTC (permalink / raw)
  To: Pekka Enberg; +Cc: QEMU Developers, qemu-arm

On 8 February 2018 at 05:50, Pekka Enberg <penberg@iki.fi> wrote:
> This patch adds a "cpu-type" property to BCM2836 SoC in preparation for
> reusing the code for the Raspberry Pi 3, which has a different processor
> model.
>
> Signed-off-by: Pekka Enberg <penberg@iki.fi>

> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -135,6 +135,8 @@ static void raspi2_init(MachineState *machine)
>      /* Setup the SOC */
>      object_property_add_const_link(OBJECT(&s->soc), "ram", OBJECT(&s->ram),
>                                     &error_abort);
> +    object_property_set_str(OBJECT(&s->soc), machine->cpu_type, "cpu-type",
> +                            &error_abort);
>      object_property_set_int(OBJECT(&s->soc), smp_cpus, "enabled-cpus",
>                              &error_abort);
>      object_property_set_int(OBJECT(&s->soc), 0xa21041, "board-rev",
> @@ -166,6 +168,7 @@ static void raspi2_machine_init(MachineClass *mc)
>      mc->no_parallel = 1;
>      mc->no_floppy = 1;
>      mc->no_cdrom = 1;
> +    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15");
>      mc->max_cpus = BCM2836_NCPUS;
>      mc->min_cpus = BCM2836_NCPUS;
>      mc->default_cpus = BCM2836_NCPUS;

This change means that instead of ignoring the user's -cpu argument
we'll now unconditionally accept it even if it's nonsense for this
board. Neither behaviour is great. However, the patchset to allow
boards to easily specify the valid set of CPU types is still in
code review:
https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg00308.html

so I'm happy to take this as-is, and we'll add the validity check
when that patchset goes in.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH v1 3/3] raspi: Add "raspi3" machine type
  2018-02-08  5:50 ` [Qemu-devel] [PATCH v1 3/3] raspi: Add "raspi3" machine type Pekka Enberg
@ 2018-02-15 12:39   ` Peter Maydell
  2018-02-15 12:49     ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
  2018-02-16  7:08     ` [Qemu-devel] " Pekka Enberg
  0 siblings, 2 replies; 11+ messages in thread
From: Peter Maydell @ 2018-02-15 12:39 UTC (permalink / raw)
  To: Pekka Enberg; +Cc: QEMU Developers, qemu-arm

On 8 February 2018 at 05:50, Pekka Enberg <penberg@iki.fi> wrote:
> This patch adds a "raspi3" machine type, which can now be selected as
> the machine to run on by users via the "-M" command line option to QEMU.
>
> The machine type does *not* ignore memory transaction failures so we
> likely need to add some dummy devices later when people run something
> more complicated than what I'm using for testing.
>
> Signed-off-by: Pekka Enberg <penberg@iki.fi>
> ---
>  hw/arm/raspi.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
> index 66fe10e376..048ff23a51 100644
> --- a/hw/arm/raspi.c
> +++ b/hw/arm/raspi.c
> @@ -187,3 +187,24 @@ static void raspi2_machine_init(MachineClass *mc)
>      mc->ignore_memory_transaction_failures = true;
>  };
>  DEFINE_MACHINE("raspi2", raspi2_machine_init)
> +
> +static void raspi3_init(MachineState *machine)
> +{
> +    raspi_init(machine, 3);
> +}
> +
> +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->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a53");
> +    mc->max_cpus = BCM2836_NCPUS;
> +    mc->min_cpus = BCM2836_NCPUS;
> +    mc->default_cpus = BCM2836_NCPUS;
> +    mc->default_ram_size = 1024 * 1024 * 1024;
> +}
> +DEFINE_MACHINE("raspi3", raspi3_machine_init)

Hi. This patch breaks "make check", because it adds the raspi3
to the arm-softmmu (32-bit guest CPUs only) build, where the
cortex-a53 CPU doesn't exist:

e104462:xenial:qemu$ ./build/x86/arm-softmmu/qemu-system-arm -M raspi3
**
ERROR:/home/petmay01/linaro/qemu-from-laptop/qemu/qom/object.c:372:object_initialize_with_type:
assertion failed: (type != NULL)
Aborted (core dumped)

The usual way we avoid this is that 64-bit only boards are
in their own source file, which is only compiled if the right
CONFIG_FOO is set by default-configs/aarch64-softmmu.mak.
In this case splitting the 64-bit board into its own source
file would be weird and awkward, so the simple thing is to
guard the raspi3 bits with #ifdef TARGET_AARCH64.

(You might think we could define a CONFIG_RASPI3 in
aarch64-softmmu.mak and #ifdef on it, but for some reason
we don't expose those CONFIG_* to C code, possibly just because
we've never needed to in the past...)

Since this was the only code change needed, I'm just going to make
it and apply the patchset to target-arm.next, rather than ask
you to do a respin. (There was also a stray space-at-end-of-line
in patch 2 which checkpatch grumbles about; I'll fix that up too.)

thanks
-- PMM

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [Qemu-arm] [PATCH v1 3/3] raspi: Add "raspi3" machine type
  2018-02-15 12:39   ` Peter Maydell
@ 2018-02-15 12:49     ` Philippe Mathieu-Daudé
  2018-02-15 13:14       ` Philippe Mathieu-Daudé
  2018-02-16  7:08     ` [Qemu-devel] " Pekka Enberg
  1 sibling, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-02-15 12:49 UTC (permalink / raw)
  To: Peter Maydell, Pekka Enberg; +Cc: qemu-arm, QEMU Developers

On 02/15/2018 09:39 AM, Peter Maydell wrote:
> On 8 February 2018 at 05:50, Pekka Enberg <penberg@iki.fi> wrote:
>> This patch adds a "raspi3" machine type, which can now be selected as
>> the machine to run on by users via the "-M" command line option to QEMU.
>>
>> The machine type does *not* ignore memory transaction failures so we
>> likely need to add some dummy devices later when people run something
>> more complicated than what I'm using for testing.
>>
>> Signed-off-by: Pekka Enberg <penberg@iki.fi>
>> ---
>>  hw/arm/raspi.c | 21 +++++++++++++++++++++
>>  1 file changed, 21 insertions(+)
>>
>> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
>> index 66fe10e376..048ff23a51 100644
>> --- a/hw/arm/raspi.c
>> +++ b/hw/arm/raspi.c
>> @@ -187,3 +187,24 @@ static void raspi2_machine_init(MachineClass *mc)
>>      mc->ignore_memory_transaction_failures = true;
>>  };
>>  DEFINE_MACHINE("raspi2", raspi2_machine_init)
>> +
>> +static void raspi3_init(MachineState *machine)
>> +{
>> +    raspi_init(machine, 3);
>> +}
>> +
>> +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->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a53");
>> +    mc->max_cpus = BCM2836_NCPUS;
>> +    mc->min_cpus = BCM2836_NCPUS;
>> +    mc->default_cpus = BCM2836_NCPUS;
>> +    mc->default_ram_size = 1024 * 1024 * 1024;
>> +}
>> +DEFINE_MACHINE("raspi3", raspi3_machine_init)
> 
> Hi. This patch breaks "make check", because it adds the raspi3
> to the arm-softmmu (32-bit guest CPUs only) build, where the
> cortex-a53 CPU doesn't exist:
> 
> e104462:xenial:qemu$ ./build/x86/arm-softmmu/qemu-system-arm -M raspi3
> **
> ERROR:/home/petmay01/linaro/qemu-from-laptop/qemu/qom/object.c:372:object_initialize_with_type:
> assertion failed: (type != NULL)
> Aborted (core dumped)
> 
> The usual way we avoid this is that 64-bit only boards are
> in their own source file, which is only compiled if the right
> CONFIG_FOO is set by default-configs/aarch64-softmmu.mak.
> In this case splitting the 64-bit board into its own source
> file would be weird and awkward, so the simple thing is to
> guard the raspi3 bits with #ifdef TARGET_AARCH64.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> 
> (You might think we could define a CONFIG_RASPI3 in
> aarch64-softmmu.mak and #ifdef on it, but for some reason
> we don't expose those CONFIG_* to C code, possibly just because
> we've never needed to in the past...)
> 
> Since this was the only code change needed, I'm just going to make
> it and apply the patchset to target-arm.next, rather than ask
> you to do a respin. (There was also a stray space-at-end-of-line
> in patch 2 which checkpatch grumbles about; I'll fix that up too.)
> 
> thanks
> -- PMM
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [Qemu-arm] [PATCH v1 3/3] raspi: Add "raspi3" machine type
  2018-02-15 12:49     ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
@ 2018-02-15 13:14       ` Philippe Mathieu-Daudé
  2018-02-15 13:18         ` Peter Maydell
  0 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-02-15 13:14 UTC (permalink / raw)
  To: Peter Maydell, Pekka Enberg; +Cc: qemu-arm, QEMU Developers

On 02/15/2018 09:49 AM, Philippe Mathieu-Daudé wrote:
> On 02/15/2018 09:39 AM, Peter Maydell wrote:
>> On 8 February 2018 at 05:50, Pekka Enberg <penberg@iki.fi> wrote:
>>> This patch adds a "raspi3" machine type, which can now be selected as
>>> the machine to run on by users via the "-M" command line option to QEMU.
>>>
>>> The machine type does *not* ignore memory transaction failures so we
>>> likely need to add some dummy devices later when people run something
>>> more complicated than what I'm using for testing.
>>>
>>> Signed-off-by: Pekka Enberg <penberg@iki.fi>
>>> ---
>>>  hw/arm/raspi.c | 21 +++++++++++++++++++++
>>>  1 file changed, 21 insertions(+)
>>>
>>> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
>>> index 66fe10e376..048ff23a51 100644
>>> --- a/hw/arm/raspi.c
>>> +++ b/hw/arm/raspi.c
>>> @@ -187,3 +187,24 @@ static void raspi2_machine_init(MachineClass *mc)
>>>      mc->ignore_memory_transaction_failures = true;
>>>  };
>>>  DEFINE_MACHINE("raspi2", raspi2_machine_init)
>>> +
>>> +static void raspi3_init(MachineState *machine)
>>> +{
>>> +    raspi_init(machine, 3);
>>> +}
>>> +
>>> +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;

Now I remember why I hesitated with this patch,

This part {

>>> +    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a53");
>>> +    mc->max_cpus = BCM2836_NCPUS;
>>> +    mc->min_cpus = BCM2836_NCPUS;
>>> +    mc->default_cpus = BCM2836_NCPUS;

} is the BCM2837 SoC, very similar to the BCM2836.

>>> +    mc->default_ram_size = 1024 * 1024 * 1024;
>>> +}
>>> +DEFINE_MACHINE("raspi3", raspi3_machine_init)
>>
>> Hi. This patch breaks "make check", because it adds the raspi3
>> to the arm-softmmu (32-bit guest CPUs only) build, where the
>> cortex-a53 CPU doesn't exist:
>>
>> e104462:xenial:qemu$ ./build/x86/arm-softmmu/qemu-system-arm -M raspi3
>> **
>> ERROR:/home/petmay01/linaro/qemu-from-laptop/qemu/qom/object.c:372:object_initialize_with_type:
>> assertion failed: (type != NULL)
>> Aborted (core dumped)
>>
>> The usual way we avoid this is that 64-bit only boards are
>> in their own source file, which is only compiled if the right
>> CONFIG_FOO is set by default-configs/aarch64-softmmu.mak.
>> In this case splitting the 64-bit board into its own source
>> file would be weird and awkward, so the simple thing is to
>> guard the raspi3 bits with #ifdef TARGET_AARCH64.
> 
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
>>
>> (You might think we could define a CONFIG_RASPI3 in
>> aarch64-softmmu.mak and #ifdef on it, but for some reason
>> we don't expose those CONFIG_* to C code, possibly just because
>> we've never needed to in the past...)
>>
>> Since this was the only code change needed, I'm just going to make
>> it and apply the patchset to target-arm.next, rather than ask
>> you to do a respin. (There was also a stray space-at-end-of-line
>> in patch 2 which checkpatch grumbles about; I'll fix that up too.)
>>
>> thanks
>> -- PMM
>>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [Qemu-arm] [PATCH v1 3/3] raspi: Add "raspi3" machine type
  2018-02-15 13:14       ` Philippe Mathieu-Daudé
@ 2018-02-15 13:18         ` Peter Maydell
  2018-02-15 13:28           ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 11+ messages in thread
From: Peter Maydell @ 2018-02-15 13:18 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: Pekka Enberg, qemu-arm, QEMU Developers

On 15 February 2018 at 13:14, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> On 02/15/2018 09:49 AM, Philippe Mathieu-Daudé wrote:
>> On 02/15/2018 09:39 AM, Peter Maydell wrote:
>>> On 8 February 2018 at 05:50, Pekka Enberg <penberg@iki.fi> wrote:
>
> Now I remember why I hesitated with this patch,
>
> This part {
>
>>>> +    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a53");
>>>> +    mc->max_cpus = BCM2836_NCPUS;
>>>> +    mc->min_cpus = BCM2836_NCPUS;
>>>> +    mc->default_cpus = BCM2836_NCPUS;
>
> } is the BCM2837 SoC, very similar to the BCM2836.

Yeah, we had a whole go-around about whether we should have a
BCM2837 object or just make the BCM2836 object have a configurable
CPU type. You could argue either way...

thanks
-- PMM

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [Qemu-arm] [PATCH v1 3/3] raspi: Add "raspi3" machine type
  2018-02-15 13:18         ` Peter Maydell
@ 2018-02-15 13:28           ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-02-15 13:28 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Pekka Enberg, qemu-arm, QEMU Developers

On 02/15/2018 10:18 AM, Peter Maydell wrote:
> On 15 February 2018 at 13:14, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>> On 02/15/2018 09:49 AM, Philippe Mathieu-Daudé wrote:
>>> On 02/15/2018 09:39 AM, Peter Maydell wrote:
>>>> On 8 February 2018 at 05:50, Pekka Enberg <penberg@iki.fi> wrote:
>>
>> Now I remember why I hesitated with this patch,
>>
>> This part {
>>
>>>>> +    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a53");
>>>>> +    mc->max_cpus = BCM2836_NCPUS;
>>>>> +    mc->min_cpus = BCM2836_NCPUS;
>>>>> +    mc->default_cpus = BCM2836_NCPUS;
>>
>> } is the BCM2837 SoC, very similar to the BCM2836.
> 
> Yeah, we had a whole go-around about whether we should have a
> BCM2837 object or just make the BCM2836 object have a configurable
> CPU type. You could argue either way...

Since both SoCs are clocked at the same freq (and we don't model the L2
cache, the only diff) your suggestion (#ifdef TARGET_AARCH64) is the
easiest/cleaner way to go and I'm happy with it :)

A one-line comment would be worthful although.

Regards,

Phil.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH v1 1/3] bcm2836: Make CPU type configurable
  2018-02-15 11:48   ` [Qemu-devel] [PATCH v1 1/3] bcm2836: Make CPU type configurable Peter Maydell
@ 2018-02-16  7:04     ` Pekka Enberg
  0 siblings, 0 replies; 11+ messages in thread
From: Pekka Enberg @ 2018-02-16  7:04 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers, qemu-arm



On 02/15/2018 01:48 PM, Peter Maydell wrote:
> On 8 February 2018 at 05:50, Pekka Enberg <penberg@iki.fi> wrote:
>> This patch adds a "cpu-type" property to BCM2836 SoC in preparation for
>> reusing the code for the Raspberry Pi 3, which has a different processor
>> model.
>>
>> Signed-off-by: Pekka Enberg <penberg@iki.fi>
> 
>> --- a/hw/arm/raspi.c
>> +++ b/hw/arm/raspi.c
>> @@ -135,6 +135,8 @@ static void raspi2_init(MachineState *machine)
>>       /* Setup the SOC */
>>       object_property_add_const_link(OBJECT(&s->soc), "ram", OBJECT(&s->ram),
>>                                      &error_abort);
>> +    object_property_set_str(OBJECT(&s->soc), machine->cpu_type, "cpu-type",
>> +                            &error_abort);
>>       object_property_set_int(OBJECT(&s->soc), smp_cpus, "enabled-cpus",
>>                               &error_abort);
>>       object_property_set_int(OBJECT(&s->soc), 0xa21041, "board-rev",
>> @@ -166,6 +168,7 @@ static void raspi2_machine_init(MachineClass *mc)
>>       mc->no_parallel = 1;
>>       mc->no_floppy = 1;
>>       mc->no_cdrom = 1;
>> +    mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15");
>>       mc->max_cpus = BCM2836_NCPUS;
>>       mc->min_cpus = BCM2836_NCPUS;
>>       mc->default_cpus = BCM2836_NCPUS;
> 
> This change means that instead of ignoring the user's -cpu argument
> we'll now unconditionally accept it even if it's nonsense for this
> board. Neither behaviour is great. However, the patchset to allow
> boards to easily specify the valid set of CPU types is still in
> code review:
> https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg00308.html
> 
> so I'm happy to take this as-is, and we'll add the validity check
> when that patchset goes in.

I wondered about that too, but I applied the "monkey see, monkey do" 
approach to your review comments of the previous attempt. :-) But 
indeed, I also think we can fix this later in the tree.

- Pekka

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH v1 3/3] raspi: Add "raspi3" machine type
  2018-02-15 12:39   ` Peter Maydell
  2018-02-15 12:49     ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
@ 2018-02-16  7:08     ` Pekka Enberg
  1 sibling, 0 replies; 11+ messages in thread
From: Pekka Enberg @ 2018-02-16  7:08 UTC (permalink / raw)
  To: Peter Maydell; +Cc: QEMU Developers, qemu-arm

Hi,

On 02/15/2018 02:39 PM, Peter Maydell wrote:
> On 8 February 2018 at 05:50, Pekka Enberg <penberg@iki.fi> wrote:
>> This patch adds a "raspi3" machine type, which can now be selected as
>> the machine to run on by users via the "-M" command line option to QEMU.
>>
>> The machine type does *not* ignore memory transaction failures so we
>> likely need to add some dummy devices later when people run something
>> more complicated than what I'm using for testing.
>>
>> Signed-off-by: Pekka Enberg <penberg@iki.fi>
>> ---
>>   hw/arm/raspi.c | 21 +++++++++++++++++++++
>>   1 file changed, 21 insertions(+)
>>
>> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
>> index 66fe10e376..048ff23a51 100644
>> --- a/hw/arm/raspi.c
>> +++ b/hw/arm/raspi.c
>> @@ -187,3 +187,24 @@ static void raspi2_machine_init(MachineClass *mc)
>>       mc->ignore_memory_transaction_failures = true;
>>   };
>>   DEFINE_MACHINE("raspi2", raspi2_machine_init)
>> +
>> +static void raspi3_init(MachineState *machine)
>> +{
>> +    raspi_init(machine, 3);
>> +}
>> +
>> +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->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a53");
>> +    mc->max_cpus = BCM2836_NCPUS;
>> +    mc->min_cpus = BCM2836_NCPUS;
>> +    mc->default_cpus = BCM2836_NCPUS;
>> +    mc->default_ram_size = 1024 * 1024 * 1024;
>> +}
>> +DEFINE_MACHINE("raspi3", raspi3_machine_init)
> 
> Hi. This patch breaks "make check", because it adds the raspi3
> to the arm-softmmu (32-bit guest CPUs only) build, where the
> cortex-a53 CPU doesn't exist:
> 
> e104462:xenial:qemu$ ./build/x86/arm-softmmu/qemu-system-arm -M raspi3
> **
> ERROR:/home/petmay01/linaro/qemu-from-laptop/qemu/qom/object.c:372:object_initialize_with_type:
> assertion failed: (type != NULL)
> Aborted (core dumped)
> 
> The usual way we avoid this is that 64-bit only boards are
> in their own source file, which is only compiled if the right
> CONFIG_FOO is set by default-configs/aarch64-softmmu.mak.
> In this case splitting the 64-bit board into its own source
> file would be weird and awkward, so the simple thing is to
> guard the raspi3 bits with #ifdef TARGET_AARCH64.
> 
> (You might think we could define a CONFIG_RASPI3 in
> aarch64-softmmu.mak and #ifdef on it, but for some reason
> we don't expose those CONFIG_* to C code, possibly just because
> we've never needed to in the past...)
> 
> Since this was the only code change needed, I'm just going to make
> it and apply the patchset to target-arm.next, rather than ask
> you to do a respin. (There was also a stray space-at-end-of-line
> in patch 2 which checkpatch grumbles about; I'll fix that up too.)

Oh, it would have helped if I had actually read the whole thread before 
sending out v2. If I understood correctly, you only applied the first 
two patches (sorry about that trailing whitespace!). You therefore can 
just pick patch 3 from the v2 as the first two patches are unchanged.

Regards,

- Pekka

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-02-16  7:08 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-08  5:50 [Qemu-devel] [PATCH v1 0/3] Raspberry Pi 3 support Pekka Enberg
2018-02-08  5:50 ` [Qemu-devel] [PATCH v1 2/3] raspi: " Pekka Enberg
2018-02-08  5:50 ` [Qemu-devel] [PATCH v1 3/3] raspi: Add "raspi3" machine type Pekka Enberg
2018-02-15 12:39   ` Peter Maydell
2018-02-15 12:49     ` [Qemu-devel] [Qemu-arm] " Philippe Mathieu-Daudé
2018-02-15 13:14       ` Philippe Mathieu-Daudé
2018-02-15 13:18         ` Peter Maydell
2018-02-15 13:28           ` Philippe Mathieu-Daudé
2018-02-16  7:08     ` [Qemu-devel] " Pekka Enberg
     [not found] ` <20180208055039.24666-2-penberg@iki.fi>
2018-02-15 11:48   ` [Qemu-devel] [PATCH v1 1/3] bcm2836: Make CPU type configurable Peter Maydell
2018-02-16  7:04     ` Pekka Enberg

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.