> On Jun 27, 2022, at 10:01 PM, Cédric Le Goater <c...@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 <p...@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[] = {
>
