| Message ID | 20220624003701.1363500-9-pdel@fb.com |
|---|---|
| State | New |
| Headers | show |
| Series | aspeed: Add multi-SoC machine | expand |
On 6/24/22 02:37, Peter Delevoryas wrote: > You can test booting the BMC with both '-device loader' and '-drive > file'. This is necessary because of how the fb-openbmc boot sequence > works (jump to 0x20000000 after U-Boot SPL). > > > wget https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd > qemu-system-arm -machine fby35 -nographic \ > -device loader,file=fby35.mtd,addr=0,cpu-num=0 -drive file=fby35.mtd,format=raw,if=mtd > > Signed-off-by: Peter Delevoryas <pdel@fb.com> > --- > hw/arm/fby35.c | 39 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > > diff --git a/hw/arm/fby35.c b/hw/arm/fby35.c > index dc1ae8e62a..1e24cbf3f8 100644 > --- a/hw/arm/fby35.c > +++ b/hw/arm/fby35.c > @@ -21,17 +21,53 @@ > */ > > #include "qemu/osdep.h" > +#include "qemu/units.h" > +#include "qapi/error.h" > #include "hw/boards.h" > +#include "hw/arm/aspeed_soc.h" > > #define TYPE_FBY35 MACHINE_TYPE_NAME("fby35") > OBJECT_DECLARE_SIMPLE_TYPE(Fby35State, FBY35); > > struct Fby35State { > MachineState parent_obj; > + > + MemoryRegion bmc_memory; > + MemoryRegion bmc_dram; > + MemoryRegion bmc_boot_rom; > + > + AspeedSoCState bmc; > }; > > +static void fby35_bmc_init(Fby35State *s) > +{ > + uint32_t boot_rom_size; > + > + memory_region_init(&s->bmc_memory, OBJECT(s), "bmc-memory", UINT64_MAX); > + memory_region_init_ram(&s->bmc_dram, OBJECT(s), "bmc-dram", 2 * GiB, &error_abort); > + > + object_initialize_child(OBJECT(s), "bmc", &s->bmc, "ast2600-a3"); > + object_property_set_int(OBJECT(&s->bmc), "ram-size", s->bmc_dram.size, &error_abort); This fails to compile on some platforms : ./hw/arm/fby35.c: In function ‘fby35_bmc_init’: ../hw/arm/fby35.c:50:69: error: incompatible type for argument 3 of ‘object_property_set_int’ 50 | object_property_set_int(OBJECT(&s->bmc), "ram-size", s->bmc_dram.size, &error_abort); | ~~~~~~~~~~~^~~~~ | | | Int128 In file included from /builds/legoater/qemu/include/exec/memory.h:28, from /builds/legoater/qemu/include/hw/boards.h:6, from ../hw/arm/fby35.c:26: /builds/legoater/qemu/include/qom/object.h:1342:38: note: expected ‘int64_t’ {aka ‘long long int’} but argument is of type ‘Int128’ 1342 | int64_t value, Error **errp); | ~~~~~~~~^~~~~ You don't need to resend the patches 1-7. I have pulled them in my branch for the next PR. > + object_property_set_link(OBJECT(&s->bmc), "memory", OBJECT(&s->bmc_memory), &error_abort); > + object_property_set_link(OBJECT(&s->bmc), "dram", OBJECT(&s->bmc_dram), &error_abort); > + object_property_set_int(OBJECT(&s->bmc), "hw-strap1", 0x000000C0, &error_abort); > + object_property_set_int(OBJECT(&s->bmc), "hw-strap2", 0x00000003, &error_abort); We could share common definitions with the BMC machine in .h file. > + object_property_set_int(OBJECT(&s->bmc), "uart-default", ASPEED_DEV_UART5, &error_abort); > + qdev_realize(DEVICE(&s->bmc), NULL, &error_abort); > + > + boot_rom_size = ASPEED_SMC_GET_CLASS(&s->bmc.fmc)->segments[0].size; > + > + memory_region_init_rom(&s->bmc_boot_rom, OBJECT(s), "bmc-boot-rom", boot_rom_size, &error_abort); > + memory_region_add_subregion(&s->bmc_memory, 0, &s->bmc_boot_rom); > + > + aspeed_board_init_flashes(&s->bmc.fmc, "n25q00", 2, 0); I am not totally convinced with the ROM because it complexifies how the machine is started but I haven't tried the other way using the loader either. It is my TODO list. Thanks, C. > +} > + > static void fby35_init(MachineState *machine) > { > + Fby35State *s = FBY35(machine); > + > + fby35_bmc_init(s); > } > > static void fby35_class_init(ObjectClass *oc, void *data) > @@ -40,6 +76,9 @@ static void fby35_class_init(ObjectClass *oc, void *data) > > mc->desc = "Meta Platforms fby35"; > mc->init = fby35_init; > + mc->no_floppy = 1; > + mc->no_cdrom = 1; > + mc->min_cpus = mc->max_cpus = mc->default_cpus = 2; > } > > static const TypeInfo fby35_types[] = {
> On Jun 27, 2022, at 10:01 PM, Cédric Le Goater <clg@kaod.org> wrote: > > On 6/24/22 02:37, Peter Delevoryas wrote: >> You can test booting the BMC with both '-device loader' and '-drive >> file'. This is necessary because of how the fb-openbmc boot sequence >> works (jump to 0x20000000 after U-Boot SPL). >> >> wget https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd >> qemu-system-arm -machine fby35 -nographic \ >> -device loader,file=fby35.mtd,addr=0,cpu-num=0 -drive file=fby35.mtd,format=raw,if=mtd >> Signed-off-by: Peter Delevoryas <pdel@fb.com> >> --- >> hw/arm/fby35.c | 39 +++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 39 insertions(+) >> diff --git a/hw/arm/fby35.c b/hw/arm/fby35.c >> index dc1ae8e62a..1e24cbf3f8 100644 >> --- a/hw/arm/fby35.c >> +++ b/hw/arm/fby35.c >> @@ -21,17 +21,53 @@ >> */ >> #include "qemu/osdep.h" >> +#include "qemu/units.h" >> +#include "qapi/error.h" >> #include "hw/boards.h" >> +#include "hw/arm/aspeed_soc.h" >> #define TYPE_FBY35 MACHINE_TYPE_NAME("fby35") >> OBJECT_DECLARE_SIMPLE_TYPE(Fby35State, FBY35); >> struct Fby35State { >> MachineState parent_obj; >> + >> + MemoryRegion bmc_memory; >> + MemoryRegion bmc_dram; >> + MemoryRegion bmc_boot_rom; >> + >> + AspeedSoCState bmc; >> }; >> +static void fby35_bmc_init(Fby35State *s) >> +{ >> + uint32_t boot_rom_size; >> + >> + memory_region_init(&s->bmc_memory, OBJECT(s), "bmc-memory", UINT64_MAX); >> + memory_region_init_ram(&s->bmc_dram, OBJECT(s), "bmc-dram", 2 * GiB, &error_abort); >> + >> + object_initialize_child(OBJECT(s), "bmc", &s->bmc, "ast2600-a3"); >> + object_property_set_int(OBJECT(&s->bmc), "ram-size", s->bmc_dram.size, &error_abort); > > This fails to compile on some platforms : > > ./hw/arm/fby35.c: In function ‘fby35_bmc_init’: > ../hw/arm/fby35.c:50:69: error: incompatible type for argument 3 of ‘object_property_set_int’ > 50 | object_property_set_int(OBJECT(&s->bmc), "ram-size", s->bmc_dram.size, &error_abort); > | ~~~~~~~~~~~^~~~~ > | | > | Int128 > In file included from /builds/legoater/qemu/include/exec/memory.h:28, > from /builds/legoater/qemu/include/hw/boards.h:6, > from ../hw/arm/fby35.c:26: > /builds/legoater/qemu/include/qom/object.h:1342:38: note: expected ‘int64_t’ {aka ‘long long int’} but argument is of type ‘Int128’ > 1342 | int64_t value, Error **errp); > | ~~~~~~~~^~~~~ > > Oh yikes, I’ll fix this. Hopefully without casting from Int128 to int64_t or defining a new constant. > You don't need to resend the patches 1-7. I have pulled them in my branch > for the next PR. Got it, I’ll just resend this one as v3 then. > >> + object_property_set_link(OBJECT(&s->bmc), "memory", OBJECT(&s->bmc_memory), &error_abort); >> + object_property_set_link(OBJECT(&s->bmc), "dram", OBJECT(&s->bmc_dram), &error_abort); >> + object_property_set_int(OBJECT(&s->bmc), "hw-strap1", 0x000000C0, &error_abort); >> + object_property_set_int(OBJECT(&s->bmc), "hw-strap2", 0x00000003, &error_abort); > > We could share common definitions with the BMC machine in .h file. Good point, I’ll put then in aspeed.h then. > >> + object_property_set_int(OBJECT(&s->bmc), "uart-default", ASPEED_DEV_UART5, &error_abort); >> + qdev_realize(DEVICE(&s->bmc), NULL, &error_abort); >> + >> + boot_rom_size = ASPEED_SMC_GET_CLASS(&s->bmc.fmc)->segments[0].size; >> + >> + memory_region_init_rom(&s->bmc_boot_rom, OBJECT(s), "bmc-boot-rom", boot_rom_size, &error_abort); >> + memory_region_add_subregion(&s->bmc_memory, 0, &s->bmc_boot_rom); >> + >> + aspeed_board_init_flashes(&s->bmc.fmc, "n25q00", 2, 0); > > I am not totally convinced with the ROM because it complexifies how the > machine is started but I haven't tried the other way using the loader > either. It is my TODO list. Yep, totally agree: if I can fix this soon I’ll send out a fix or something, otherwise feel free to submit a change of your own. Thanks for the review! > > Thanks, > > C. > >> +} >> + >> static void fby35_init(MachineState *machine) >> { >> + Fby35State *s = FBY35(machine); >> + >> + fby35_bmc_init(s); >> } >> static void fby35_class_init(ObjectClass *oc, void *data) >> @@ -40,6 +76,9 @@ static void fby35_class_init(ObjectClass *oc, void *data) >> mc->desc = "Meta Platforms fby35"; >> mc->init = fby35_init; >> + mc->no_floppy = 1; >> + mc->no_cdrom = 1; >> + mc->min_cpus = mc->max_cpus = mc->default_cpus = 2; >> } >> static const TypeInfo fby35_types[] = { >
diff --git a/hw/arm/fby35.c b/hw/arm/fby35.c index dc1ae8e62a..1e24cbf3f8 100644 --- a/hw/arm/fby35.c +++ b/hw/arm/fby35.c @@ -21,17 +21,53 @@ */ #include "qemu/osdep.h" +#include "qemu/units.h" +#include "qapi/error.h" #include "hw/boards.h" +#include "hw/arm/aspeed_soc.h" #define TYPE_FBY35 MACHINE_TYPE_NAME("fby35") OBJECT_DECLARE_SIMPLE_TYPE(Fby35State, FBY35); struct Fby35State { MachineState parent_obj; + + MemoryRegion bmc_memory; + MemoryRegion bmc_dram; + MemoryRegion bmc_boot_rom; + + AspeedSoCState bmc; }; +static void fby35_bmc_init(Fby35State *s) +{ + uint32_t boot_rom_size; + + memory_region_init(&s->bmc_memory, OBJECT(s), "bmc-memory", UINT64_MAX); + memory_region_init_ram(&s->bmc_dram, OBJECT(s), "bmc-dram", 2 * GiB, &error_abort); + + object_initialize_child(OBJECT(s), "bmc", &s->bmc, "ast2600-a3"); + object_property_set_int(OBJECT(&s->bmc), "ram-size", s->bmc_dram.size, &error_abort); + object_property_set_link(OBJECT(&s->bmc), "memory", OBJECT(&s->bmc_memory), &error_abort); + object_property_set_link(OBJECT(&s->bmc), "dram", OBJECT(&s->bmc_dram), &error_abort); + object_property_set_int(OBJECT(&s->bmc), "hw-strap1", 0x000000C0, &error_abort); + object_property_set_int(OBJECT(&s->bmc), "hw-strap2", 0x00000003, &error_abort); + object_property_set_int(OBJECT(&s->bmc), "uart-default", ASPEED_DEV_UART5, &error_abort); + qdev_realize(DEVICE(&s->bmc), NULL, &error_abort); + + boot_rom_size = ASPEED_SMC_GET_CLASS(&s->bmc.fmc)->segments[0].size; + + memory_region_init_rom(&s->bmc_boot_rom, OBJECT(s), "bmc-boot-rom", boot_rom_size, &error_abort); + memory_region_add_subregion(&s->bmc_memory, 0, &s->bmc_boot_rom); + + aspeed_board_init_flashes(&s->bmc.fmc, "n25q00", 2, 0); +} + static void fby35_init(MachineState *machine) { + Fby35State *s = FBY35(machine); + + fby35_bmc_init(s); } static void fby35_class_init(ObjectClass *oc, void *data) @@ -40,6 +76,9 @@ static void fby35_class_init(ObjectClass *oc, void *data) mc->desc = "Meta Platforms fby35"; mc->init = fby35_init; + mc->no_floppy = 1; + mc->no_cdrom = 1; + mc->min_cpus = mc->max_cpus = mc->default_cpus = 2; } static const TypeInfo fby35_types[] = {
You can test booting the BMC with both '-device loader' and '-drive file'. This is necessary because of how the fb-openbmc boot sequence works (jump to 0x20000000 after U-Boot SPL). wget https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd qemu-system-arm -machine fby35 -nographic \ -device loader,file=fby35.mtd,addr=0,cpu-num=0 -drive file=fby35.mtd,format=raw,if=mtd Signed-off-by: Peter Delevoryas <pdel@fb.com> --- hw/arm/fby35.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)