On Fri, Dec 30, 2022 at 8:04 PM Daniel Henrique Barboza <dbarb...@ventanamicro.com> wrote: > > > > On 12/30/22 06:05, Bin Meng wrote: > > On Fri, Dec 30, 2022 at 2:47 AM Daniel Henrique Barboza > > <dbarb...@ventanamicro.com> wrote: > >> The microchip_icicle_kit, sifive_u, spike and virt boards are now doing > >> the same steps when '-kernel' is used: > >> > >> - execute load_kernel() > >> - load init_rd() > >> - write kernel_cmdline in the fdt > >> > >> Let's fold everything inside riscv_load_kernel() to avoid code > >> repetition. Every other board that uses riscv_load_kernel() will have > >> this same behavior, including boards that doesn't have a valid FDT, so > >> we need to take care to not do FDT operations without checking it first. > >> > >> Since we're now doing way more than just loading the kernel, rename > >> riscv_load_kernel() to riscv_load_kernel_and_initrd(). > >> > >> Cc: Palmer Dabbelt <pal...@dabbelt.com> > >> Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org> > >> Signed-off-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com> > >> --- > >> hw/riscv/boot.c | 27 +++++++++++++++++++++------ > >> hw/riscv/microchip_pfsoc.c | 12 ++---------- > >> hw/riscv/opentitan.c | 2 +- > >> hw/riscv/sifive_e.c | 3 ++- > >> hw/riscv/sifive_u.c | 12 ++---------- > >> hw/riscv/spike.c | 14 +++----------- > >> hw/riscv/virt.c | 12 ++---------- > >> include/hw/riscv/boot.h | 6 +++--- > >> 8 files changed, 36 insertions(+), 52 deletions(-) > >> > >> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c > >> index 5606ea6f43..d18262c302 100644 > >> --- a/hw/riscv/boot.c > >> +++ b/hw/riscv/boot.c > >> @@ -171,12 +171,13 @@ target_ulong riscv_load_firmware(const char > >> *firmware_filename, > >> exit(1); > >> } > >> > >> -target_ulong riscv_load_kernel(MachineState *machine, > >> - target_ulong kernel_start_addr, > >> - symbol_fn_t sym_cb) > >> +target_ulong riscv_load_kernel_and_initrd(MachineState *machine, > >> + target_ulong kernel_start_addr, > >> + symbol_fn_t sym_cb) > > How about we keep the riscv_load_kernel() API, in case there is a need > > for machines that just want to load kernel only? > > > > Then introduce a new API riscv_load_kernel_and_initrd() to do both > > kernel and initrd loading? > > What about an extra flag to riscv_load_kernel(), e.g: > > riscv_load_kernel(MachineState *machine, target_ulong kernel_start_addr, > bool load_initrd, symbol_fn_t sym_cb) > > > And then machines that don't want to load initrd can opt out by using > load_initrd = false. The name of the flag would also make our intentions with > the API self explanatory (i.e. load_initrd makes load_kernel() loads initrd > as well). >
That sounds like a good idea! Regards, Bin
