qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 1/6] hw/riscv: virt: Add a machine done notifier


From: Alistair Francis
Subject: Re: [PATCH v2 1/6] hw/riscv: virt: Add a machine done notifier
Date: Tue, 19 Apr 2022 14:50:07 +1000

On Sat, Apr 16, 2022 at 1:25 AM Andrew Bresticker <abrestic@rivosinc.com> wrote:
>
> Hi Alistair,
>
> On Wed, Apr 6, 2022 at 10:05 PM Alistair Francis
> <alistair.francis@opensource.wdc.com> wrote:
> >
> > From: Alistair Francis <alistair.francis@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.francis@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
> >
> >



reply via email to

[Prev in Thread] Current Thread [Next in Thread]