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.
-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
>
>
