| Message ID | 20180208055039.24666-4-penberg@iki.fi |
|---|---|
| State | New |
| Headers | show |
| Series | Raspberry Pi 3 support | expand |
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
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 >
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 >>
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
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.
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
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)
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(+)