On Sat, Apr 16, 2022 at 1:25 AM Andrew Bresticker <abres...@rivosinc.com> wrote:
>
> Hi Alistair,
>
> On Wed, Apr 6, 2022 at 10:05 PM Alistair Francis
> <alistair.fran...@opensource.wdc.com> wrote:
> >
> > From: Alistair Francis <alistair.fran...@wdc.com>
> >
> > Move the binary and device tree loading code to the machine done
> > notifier. This allows us to prepare for editing the device tree as part
> > of the notifier.
> >
> > This is based on similar code in the ARM virt machine.
> >
> > Signed-off-by: Alistair Francis <alistair.fran...@wdc.com>
> > ---
> > include/hw/riscv/virt.h | 1 +
> > hw/riscv/virt.c | 180 +++++++++++++++++++++-------------------
> > 2 files changed, 97 insertions(+), 84 deletions(-)
> >
> > diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
> > index 78b058ec86..8b8db3fb7c 100644
> > --- a/include/hw/riscv/virt.h
> > +++ b/include/hw/riscv/virt.h
> > @@ -45,6 +45,7 @@ struct RISCVVirtState {
> > MachineState parent;
> >
> > /*< public >*/
> > + Notifier machine_done;
> > RISCVHartArrayState soc[VIRT_SOCKETS_MAX];
> > DeviceState *irqchip[VIRT_SOCKETS_MAX];
> > PFlashCFI01 *flash[2];
> > diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
> > index da50cbed43..3f065b540e 100644
> > --- a/hw/riscv/virt.c
> > +++ b/hw/riscv/virt.c
> > @@ -1156,6 +1156,99 @@ static DeviceState *virt_create_aia(RISCVVirtAIAType
> > aia_type, int aia_guests,
> > return aplic_m;
> > }
> >
> > +static
> > +void virt_machine_done(Notifier *notifier, void *data)
> > +{
> > + RISCVVirtState *s = container_of(notifier, RISCVVirtState,
> > + machine_done);
> > + const MemMapEntry *memmap = virt_memmap;
> > + MachineState *machine = MACHINE(s);
> > + target_ulong start_addr = memmap[VIRT_DRAM].base;
> > + target_ulong firmware_end_addr, kernel_start_addr;
> > + uint32_t fdt_load_addr;
> > + uint64_t kernel_entry;
> > +
> > + /* create device tree */
> > + create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline,
> > + riscv_is_32bit(&s->soc[0]));
>
> Creating the initial FDT in machine_init() is still useful for
> allowing guest-loader to add its FDT nodes. Yes it's currently broken
> because the FDT is finalized in machine_init(), but it'll remain
> broken after this patch :) How about splitting it up like the ARM virt
> machine and leaving FDT creation in machine_init() while doing the
> dynamic additions / finalization in the machine_done notifier? Happy
> to send a separate patch for that if you prefer.
Good point. I have left the create_fdt() call in machine_init(). Feel
free to add support for guest-loader editing FDT nodes
Alistair
>
> -Andrew
>
> > +
> > + /*
> > + * Only direct boot kernel is currently supported for KVM VM,
> > + * so the "-bios" parameter is ignored and treated like "-bios none"
> > + * when KVM is enabled.
> > + */
> > + if (kvm_enabled()) {
> > + g_free(machine->firmware);
> > + machine->firmware = g_strdup("none");
> > + }
> > +
> > + if (riscv_is_32bit(&s->soc[0])) {
> > + firmware_end_addr = riscv_find_and_load_firmware(machine,
> > + RISCV32_BIOS_BIN, start_addr, NULL);
> > + } else {
> > + firmware_end_addr = riscv_find_and_load_firmware(machine,
> > + RISCV64_BIOS_BIN, start_addr, NULL);
> > + }
> > +
> > + if (machine->kernel_filename) {
> > + kernel_start_addr = riscv_calc_kernel_start_addr(&s->soc[0],
> > +
> > firmware_end_addr);
> > +
> > + kernel_entry = riscv_load_kernel(machine->kernel_filename,
> > + kernel_start_addr, NULL);
> > +
> > + if (machine->initrd_filename) {
> > + hwaddr start;
> > + hwaddr end = riscv_load_initrd(machine->initrd_filename,
> > + machine->ram_size, kernel_entry,
> > + &start);
> > + qemu_fdt_setprop_cell(machine->fdt, "/chosen",
> > + "linux,initrd-start", start);
> > + qemu_fdt_setprop_cell(machine->fdt, "/chosen",
> > "linux,initrd-end",
> > + end);
> > + }
> > + } else {
> > + /*
> > + * If dynamic firmware is used, it doesn't know where is the next
> > mode
> > + * if kernel argument is not set.
> > + */
> > + kernel_entry = 0;
> > + }
> > +
> > + if (drive_get(IF_PFLASH, 0, 0)) {
> > + /*
> > + * Pflash was supplied, let's overwrite the address we jump to
> > after
> > + * reset to the base of the flash.
> > + */
> > + start_addr = virt_memmap[VIRT_FLASH].base;
> > + }
> > +
> > + /*
> > + * Init fw_cfg. Must be done before riscv_load_fdt, otherwise the
> > device
> > + * tree cannot be altered and we get FDT_ERR_NOSPACE.
> > + */
> > + s->fw_cfg = create_fw_cfg(machine);
> > + rom_set_fw(s->fw_cfg);
> > +
> > + /* Compute the fdt load address in dram */
> > + fdt_load_addr = riscv_load_fdt(memmap[VIRT_DRAM].base,
> > + machine->ram_size, machine->fdt);
> > + /* load the reset vector */
> > + riscv_setup_rom_reset_vec(machine, &s->soc[0], start_addr,
> > + virt_memmap[VIRT_MROM].base,
> > + virt_memmap[VIRT_MROM].size, kernel_entry,
> > + fdt_load_addr, machine->fdt);
> > +
> > + /*
> > + * Only direct boot kernel is currently supported for KVM VM,
> > + * So here setup kernel start address and fdt address.
> > + * TODO:Support firmware loading and integrate to TCG start
> > + */
> > + if (kvm_enabled()) {
> > + riscv_setup_direct_kernel(kernel_entry, fdt_load_addr);
> > + }
> > +}
> > +
> > static void virt_machine_init(MachineState *machine)
> > {
> > const MemMapEntry *memmap = virt_memmap;
> > @@ -1163,10 +1256,6 @@ static void virt_machine_init(MachineState *machine)
> > MemoryRegion *system_memory = get_system_memory();
> > MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
> > char *soc_name;
> > - target_ulong start_addr = memmap[VIRT_DRAM].base;
> > - target_ulong firmware_end_addr, kernel_start_addr;
> > - uint32_t fdt_load_addr;
> > - uint64_t kernel_entry;
> > DeviceState *mmio_irqchip, *virtio_irqchip, *pcie_irqchip;
> > int i, base_hartid, hart_count;
> >
> > @@ -1296,92 +1385,12 @@ static void virt_machine_init(MachineState *machine)
> > memory_region_add_subregion(system_memory, memmap[VIRT_DRAM].base,
> > machine->ram);
> >
> > - /* create device tree */
> > - create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline,
> > - riscv_is_32bit(&s->soc[0]));
> > -
> > /* boot rom */
> > memory_region_init_rom(mask_rom, NULL, "riscv_virt_board.mrom",
> > memmap[VIRT_MROM].size, &error_fatal);
> > memory_region_add_subregion(system_memory, memmap[VIRT_MROM].base,
> > mask_rom);
> >
> > - /*
> > - * Only direct boot kernel is currently supported for KVM VM,
> > - * so the "-bios" parameter is ignored and treated like "-bios none"
> > - * when KVM is enabled.
> > - */
> > - if (kvm_enabled()) {
> > - g_free(machine->firmware);
> > - machine->firmware = g_strdup("none");
> > - }
> > -
> > - if (riscv_is_32bit(&s->soc[0])) {
> > - firmware_end_addr = riscv_find_and_load_firmware(machine,
> > - RISCV32_BIOS_BIN, start_addr, NULL);
> > - } else {
> > - firmware_end_addr = riscv_find_and_load_firmware(machine,
> > - RISCV64_BIOS_BIN, start_addr, NULL);
> > - }
> > -
> > - if (machine->kernel_filename) {
> > - kernel_start_addr = riscv_calc_kernel_start_addr(&s->soc[0],
> > -
> > firmware_end_addr);
> > -
> > - kernel_entry = riscv_load_kernel(machine->kernel_filename,
> > - kernel_start_addr, NULL);
> > -
> > - if (machine->initrd_filename) {
> > - hwaddr start;
> > - hwaddr end = riscv_load_initrd(machine->initrd_filename,
> > - machine->ram_size, kernel_entry,
> > - &start);
> > - qemu_fdt_setprop_cell(machine->fdt, "/chosen",
> > - "linux,initrd-start", start);
> > - qemu_fdt_setprop_cell(machine->fdt, "/chosen",
> > "linux,initrd-end",
> > - end);
> > - }
> > - } else {
> > - /*
> > - * If dynamic firmware is used, it doesn't know where is the next
> > mode
> > - * if kernel argument is not set.
> > - */
> > - kernel_entry = 0;
> > - }
> > -
> > - if (drive_get(IF_PFLASH, 0, 0)) {
> > - /*
> > - * Pflash was supplied, let's overwrite the address we jump to
> > after
> > - * reset to the base of the flash.
> > - */
> > - start_addr = virt_memmap[VIRT_FLASH].base;
> > - }
> > -
> > - /*
> > - * Init fw_cfg. Must be done before riscv_load_fdt, otherwise the
> > device
> > - * tree cannot be altered and we get FDT_ERR_NOSPACE.
> > - */
> > - s->fw_cfg = create_fw_cfg(machine);
> > - rom_set_fw(s->fw_cfg);
> > -
> > - /* Compute the fdt load address in dram */
> > - fdt_load_addr = riscv_load_fdt(memmap[VIRT_DRAM].base,
> > - machine->ram_size, machine->fdt);
> > - /* load the reset vector */
> > - riscv_setup_rom_reset_vec(machine, &s->soc[0], start_addr,
> > - virt_memmap[VIRT_MROM].base,
> > - virt_memmap[VIRT_MROM].size, kernel_entry,
> > - fdt_load_addr, machine->fdt);
> > -
> > - /*
> > - * Only direct boot kernel is currently supported for KVM VM,
> > - * So here setup kernel start address and fdt address.
> > - * TODO:Support firmware loading and integrate to TCG start
> > - */
> > - if (kvm_enabled()) {
> > - riscv_setup_direct_kernel(kernel_entry, fdt_load_addr);
> > - }
> > -
> > /* SiFive Test MMIO device */
> > sifive_test_create(memmap[VIRT_TEST].base);
> >
> > @@ -1417,6 +1426,9 @@ static void virt_machine_init(MachineState *machine)
> > drive_get(IF_PFLASH, 0, i));
> > }
> > virt_flash_map(s, system_memory);
> > +
> > + s->machine_done.notify = virt_machine_done;
> > + qemu_add_machine_init_done_notifier(&s->machine_done);
> > }
> >
> > static void virt_machine_instance_init(Object *obj)
> > --
> > 2.35.1
> >
> >
