| Message ID | 20200208165645.15657-4-f4bug@amsat.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw/arm/raspi: Dynamically create machines based on the board revision | expand |
On Sat, 8 Feb 2020 at 16:57, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > > The board revision encode the board version. Add a helper > to extract the version, and use it. > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > hw/arm/raspi.c | 31 +++++++++++++++++++++++++++---- > 1 file changed, 27 insertions(+), 4 deletions(-) > > diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c > index 818146fdbb..f285e2988f 100644 > --- a/hw/arm/raspi.c > +++ b/hw/arm/raspi.c > @@ -16,6 +16,7 @@ > #include "qapi/error.h" > #include "cpu.h" > #include "hw/arm/bcm2836.h" > +#include "hw/registerfields.h" > #include "qemu/error-report.h" > #include "hw/boards.h" > #include "hw/loader.h" > @@ -37,6 +38,28 @@ typedef struct RasPiState { > MemoryRegion ram; > } RasPiState; > > +/* > + * Board revision codes: > + * www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/ > + */ > +FIELD(REV_CODE, REVISION, 0, 4); > +FIELD(REV_CODE, TYPE, 4, 8); > +FIELD(REV_CODE, PROCESSOR, 12, 4); > +FIELD(REV_CODE, MANUFACTURER, 16, 4); > +FIELD(REV_CODE, MEMORY_SIZE, 20, 3); > +FIELD(REV_CODE, STYLE, 23, 1); > + > +static int board_processor_id(uint32_t board_rev) > +{ > + assert(FIELD_EX32(board_rev, REV_CODE, STYLE)); /* Only new style */ > + return FIELD_EX32(board_rev, REV_CODE, PROCESSOR); > +} > + > +static int board_version(uint32_t board_rev) > +{ > + return board_processor_id(board_rev) + 1; This uses the 'processor' field, which basically means the SoC (0 for BCM2835, 1 for BCM2836, 2 for BCMM2837, 3 for BCM2711). We use 'version' for a wider range of things in our code here: * do we need SMP setup? * which address does the firmware image go? * do we need to set up SMC vectors so no-op SMC works? * as well as "which SoC do we instantiate"? We think of 'version' as basically "raspi 2 or 3?", but according to the table in your url you can get a version of the raspi 2b with a BCM2837 SoC, which confuses this idea. Anyway, since what we have in this patch works OK for the set of board models we support, I'm happy to leave the patch as-is, but maybe worth checking and considering what in our code we should really be making conditional on "actually the SoC type" and what on something else... thanks -- PMM
On 2/13/20 2:40 PM, Peter Maydell wrote: > On Sat, 8 Feb 2020 at 16:57, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >> >> The board revision encode the board version. Add a helper >> to extract the version, and use it. >> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> >> --- >> hw/arm/raspi.c | 31 +++++++++++++++++++++++++++---- >> 1 file changed, 27 insertions(+), 4 deletions(-) >> >> diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c >> index 818146fdbb..f285e2988f 100644 >> --- a/hw/arm/raspi.c >> +++ b/hw/arm/raspi.c >> @@ -16,6 +16,7 @@ >> #include "qapi/error.h" >> #include "cpu.h" >> #include "hw/arm/bcm2836.h" >> +#include "hw/registerfields.h" >> #include "qemu/error-report.h" >> #include "hw/boards.h" >> #include "hw/loader.h" >> @@ -37,6 +38,28 @@ typedef struct RasPiState { >> MemoryRegion ram; >> } RasPiState; >> >> +/* >> + * Board revision codes: >> + * www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/ >> + */ >> +FIELD(REV_CODE, REVISION, 0, 4); >> +FIELD(REV_CODE, TYPE, 4, 8); >> +FIELD(REV_CODE, PROCESSOR, 12, 4); >> +FIELD(REV_CODE, MANUFACTURER, 16, 4); >> +FIELD(REV_CODE, MEMORY_SIZE, 20, 3); >> +FIELD(REV_CODE, STYLE, 23, 1); >> + >> +static int board_processor_id(uint32_t board_rev) >> +{ >> + assert(FIELD_EX32(board_rev, REV_CODE, STYLE)); /* Only new style */ >> + return FIELD_EX32(board_rev, REV_CODE, PROCESSOR); >> +} >> + >> +static int board_version(uint32_t board_rev) >> +{ >> + return board_processor_id(board_rev) + 1; > > This uses the 'processor' field, which basically means the SoC > (0 for BCM2835, 1 for BCM2836, 2 for BCMM2837, 3 for BCM2711). > We use 'version' for a wider range of things in our code here: > * do we need SMP setup? > * which address does the firmware image go? > * do we need to set up SMC vectors so no-op SMC works? > * as well as "which SoC do we instantiate"? > > We think of 'version' as basically "raspi 2 or 3?", but > according to the table in your url you can get a version of > the raspi 2b with a BCM2837 SoC, which confuses this idea. > > Anyway, since what we have in this patch works OK for the set > of board models we support, I'm happy to leave the patch as-is, > but maybe worth checking and considering what in our code we > should really be making conditional on "actually the SoC type" > and what on something else... Yes you are right, version = (2, 3) was too simple, I replaced the 'version' check by 'processor_id' in the series introducing the raspi4 (with other cleanups). Eventually this file will only use the board_rev fields directly.
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c index 818146fdbb..f285e2988f 100644 --- a/hw/arm/raspi.c +++ b/hw/arm/raspi.c @@ -16,6 +16,7 @@ #include "qapi/error.h" #include "cpu.h" #include "hw/arm/bcm2836.h" +#include "hw/registerfields.h" #include "qemu/error-report.h" #include "hw/boards.h" #include "hw/loader.h" @@ -37,6 +38,28 @@ typedef struct RasPiState { MemoryRegion ram; } RasPiState; +/* + * Board revision codes: + * www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/ + */ +FIELD(REV_CODE, REVISION, 0, 4); +FIELD(REV_CODE, TYPE, 4, 8); +FIELD(REV_CODE, PROCESSOR, 12, 4); +FIELD(REV_CODE, MANUFACTURER, 16, 4); +FIELD(REV_CODE, MEMORY_SIZE, 20, 3); +FIELD(REV_CODE, STYLE, 23, 1); + +static int board_processor_id(uint32_t board_rev) +{ + assert(FIELD_EX32(board_rev, REV_CODE, STYLE)); /* Only new style */ + return FIELD_EX32(board_rev, REV_CODE, PROCESSOR); +} + +static int board_version(uint32_t board_rev) +{ + return board_processor_id(board_rev) + 1; +} + static void write_smpboot(ARMCPU *cpu, const struct arm_boot_info *info) { static const uint32_t smpboot[] = { @@ -164,9 +187,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, uint32_t board_rev) { RasPiState *s = g_new0(RasPiState, 1); + int version = board_version(board_rev); uint32_t vcram_size; DriveInfo *di; BlockBackend *blk; @@ -192,7 +216,6 @@ 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_bool(OBJECT(&s->soc), true, "realized", &error_abort); @@ -216,7 +239,7 @@ static void raspi_init(MachineState *machine, int version) static void raspi2_init(MachineState *machine) { - raspi_init(machine, 2); + raspi_init(machine, 0xa21041); } static void raspi2_machine_init(MachineClass *mc) @@ -238,7 +261,7 @@ DEFINE_MACHINE("raspi2", raspi2_machine_init) #ifdef TARGET_AARCH64 static void raspi3_init(MachineState *machine) { - raspi_init(machine, 3); + raspi_init(machine, 0xa02082); } static void raspi3_machine_init(MachineClass *mc)
The board revision encode the board version. Add a helper to extract the version, and use it. Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- hw/arm/raspi.c | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-)