| Message ID | 20231010-loongarch-loader-params-v2-1-512cc7959683@t-8ch.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] hw/loongarch: remove global loaderparams variable | expand |
在 2023/10/10 下午5:49, Thomas Weißschuh 写道: > Passing the struct around explicitly makes the control-flow more > obvious. > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > Signed-off-by: Thomas Weißschuh <thomas@t-8ch.de> > --- > Changes in v2: > - Drop initialize loaderparams variable > - Link to v1: https://lore.kernel.org/qemu-devel/20231009210018.249776-1-thomas@t-8ch.de/ > --- > hw/loongarch/virt.c | 50 +++++++++++++++++++++++++++----------------------- > 1 file changed, 27 insertions(+), 23 deletions(-) > Reviewed-by: Song Gao <gaosong@loongson.cn> Thanks. SOng Gao > diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c > index 2629128aeda4..88bf0b0e97e1 100644 > --- a/hw/loongarch/virt.c > +++ b/hw/loongarch/virt.c > @@ -47,6 +47,13 @@ > #include "qemu/error-report.h" > > > +struct loaderparams { > + uint64_t ram_size; > + const char *kernel_filename; > + const char *kernel_cmdline; > + const char *initrd_filename; > +}; > + > static void virt_flash_create(LoongArchMachineState *lams) > { > DeviceState *dev = qdev_new(TYPE_PFLASH_CFI01); > @@ -411,24 +418,17 @@ static const MemoryRegionOps loongarch_virt_pm_ops = { > } > }; > > -static struct _loaderparams { > - uint64_t ram_size; > - const char *kernel_filename; > - const char *kernel_cmdline; > - const char *initrd_filename; > -} loaderparams; > - > static uint64_t cpu_loongarch_virt_to_phys(void *opaque, uint64_t addr) > { > return addr & MAKE_64BIT_MASK(0, TARGET_PHYS_ADDR_SPACE_BITS); > } > > -static int64_t load_kernel_info(void) > +static int64_t load_kernel_info(const struct loaderparams *loaderparams) > { > uint64_t kernel_entry, kernel_low, kernel_high; > ssize_t kernel_size; > > - kernel_size = load_elf(loaderparams.kernel_filename, NULL, > + kernel_size = load_elf(loaderparams->kernel_filename, NULL, > cpu_loongarch_virt_to_phys, NULL, > &kernel_entry, &kernel_low, > &kernel_high, NULL, 0, > @@ -436,7 +436,7 @@ static int64_t load_kernel_info(void) > > if (kernel_size < 0) { > error_report("could not load kernel '%s': %s", > - loaderparams.kernel_filename, > + loaderparams->kernel_filename, > load_elf_strerror(kernel_size)); > exit(1); > } > @@ -728,7 +728,8 @@ static void reset_load_elf(void *opaque) > } > } > > -static void fw_cfg_add_kernel_info(FWCfgState *fw_cfg) > +static void fw_cfg_add_kernel_info(const struct loaderparams *loaderparams, > + FWCfgState *fw_cfg) > { > /* > * Expose the kernel, the command line, and the initrd in fw_cfg. > @@ -737,36 +738,38 @@ static void fw_cfg_add_kernel_info(FWCfgState *fw_cfg) > */ > load_image_to_fw_cfg(fw_cfg, > FW_CFG_KERNEL_SIZE, FW_CFG_KERNEL_DATA, > - loaderparams.kernel_filename, > + loaderparams->kernel_filename, > false); > > - if (loaderparams.initrd_filename) { > + if (loaderparams->initrd_filename) { > load_image_to_fw_cfg(fw_cfg, > FW_CFG_INITRD_SIZE, FW_CFG_INITRD_DATA, > - loaderparams.initrd_filename, false); > + loaderparams->initrd_filename, false); > } > > - if (loaderparams.kernel_cmdline) { > + if (loaderparams->kernel_cmdline) { > fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE, > - strlen(loaderparams.kernel_cmdline) + 1); > + strlen(loaderparams->kernel_cmdline) + 1); > fw_cfg_add_string(fw_cfg, FW_CFG_CMDLINE_DATA, > - loaderparams.kernel_cmdline); > + loaderparams->kernel_cmdline); > } > } > > -static void loongarch_firmware_boot(LoongArchMachineState *lams) > +static void loongarch_firmware_boot(LoongArchMachineState *lams, > + const struct loaderparams *loaderparams) > { > - fw_cfg_add_kernel_info(lams->fw_cfg); > + fw_cfg_add_kernel_info(loaderparams, lams->fw_cfg); > } > > -static void loongarch_direct_kernel_boot(LoongArchMachineState *lams) > +static void loongarch_direct_kernel_boot(LoongArchMachineState *lams, > + const struct loaderparams *loaderparams) > { > MachineState *machine = MACHINE(lams); > int64_t kernel_addr = 0; > LoongArchCPU *lacpu; > int i; > > - kernel_addr = load_kernel_info(); > + kernel_addr = load_kernel_info(loaderparams); > if (!machine->firmware) { > for (i = 0; i < machine->smp.cpus; i++) { > lacpu = LOONGARCH_CPU(qemu_get_cpu(i)); > @@ -793,6 +796,7 @@ static void loongarch_init(MachineState *machine) > MachineClass *mc = MACHINE_GET_CLASS(machine); > CPUState *cpu; > char *ramName = NULL; > + struct loaderparams loaderparams = { }; > > if (!cpu_model) { > cpu_model = LOONGARCH_CPU_TYPE_NAME("la464"); > @@ -898,9 +902,9 @@ static void loongarch_init(MachineState *machine) > /* load the kernel. */ > if (loaderparams.kernel_filename) { > if (lams->bios_loaded) { > - loongarch_firmware_boot(lams); > + loongarch_firmware_boot(lams, &loaderparams); > } else { > - loongarch_direct_kernel_boot(lams); > + loongarch_direct_kernel_boot(lams, &loaderparams); > } > } > fdt_add_flash_node(lams); > > --- > base-commit: 2f3913f4b2ad74baeb5a6f1d36efbd9ecdf1057d > change-id: 20231010-loongarch-loader-params-f0bc0b2cb5ea > > Best regards, >
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c index 2629128aeda4..88bf0b0e97e1 100644 --- a/hw/loongarch/virt.c +++ b/hw/loongarch/virt.c @@ -47,6 +47,13 @@ #include "qemu/error-report.h" +struct loaderparams { + uint64_t ram_size; + const char *kernel_filename; + const char *kernel_cmdline; + const char *initrd_filename; +}; + static void virt_flash_create(LoongArchMachineState *lams) { DeviceState *dev = qdev_new(TYPE_PFLASH_CFI01); @@ -411,24 +418,17 @@ static const MemoryRegionOps loongarch_virt_pm_ops = { } }; -static struct _loaderparams { - uint64_t ram_size; - const char *kernel_filename; - const char *kernel_cmdline; - const char *initrd_filename; -} loaderparams; - static uint64_t cpu_loongarch_virt_to_phys(void *opaque, uint64_t addr) { return addr & MAKE_64BIT_MASK(0, TARGET_PHYS_ADDR_SPACE_BITS); } -static int64_t load_kernel_info(void) +static int64_t load_kernel_info(const struct loaderparams *loaderparams) { uint64_t kernel_entry, kernel_low, kernel_high; ssize_t kernel_size; - kernel_size = load_elf(loaderparams.kernel_filename, NULL, + kernel_size = load_elf(loaderparams->kernel_filename, NULL, cpu_loongarch_virt_to_phys, NULL, &kernel_entry, &kernel_low, &kernel_high, NULL, 0, @@ -436,7 +436,7 @@ static int64_t load_kernel_info(void) if (kernel_size < 0) { error_report("could not load kernel '%s': %s", - loaderparams.kernel_filename, + loaderparams->kernel_filename, load_elf_strerror(kernel_size)); exit(1); } @@ -728,7 +728,8 @@ static void reset_load_elf(void *opaque) } } -static void fw_cfg_add_kernel_info(FWCfgState *fw_cfg) +static void fw_cfg_add_kernel_info(const struct loaderparams *loaderparams, + FWCfgState *fw_cfg) { /* * Expose the kernel, the command line, and the initrd in fw_cfg. @@ -737,36 +738,38 @@ static void fw_cfg_add_kernel_info(FWCfgState *fw_cfg) */ load_image_to_fw_cfg(fw_cfg, FW_CFG_KERNEL_SIZE, FW_CFG_KERNEL_DATA, - loaderparams.kernel_filename, + loaderparams->kernel_filename, false); - if (loaderparams.initrd_filename) { + if (loaderparams->initrd_filename) { load_image_to_fw_cfg(fw_cfg, FW_CFG_INITRD_SIZE, FW_CFG_INITRD_DATA, - loaderparams.initrd_filename, false); + loaderparams->initrd_filename, false); } - if (loaderparams.kernel_cmdline) { + if (loaderparams->kernel_cmdline) { fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE, - strlen(loaderparams.kernel_cmdline) + 1); + strlen(loaderparams->kernel_cmdline) + 1); fw_cfg_add_string(fw_cfg, FW_CFG_CMDLINE_DATA, - loaderparams.kernel_cmdline); + loaderparams->kernel_cmdline); } } -static void loongarch_firmware_boot(LoongArchMachineState *lams) +static void loongarch_firmware_boot(LoongArchMachineState *lams, + const struct loaderparams *loaderparams) { - fw_cfg_add_kernel_info(lams->fw_cfg); + fw_cfg_add_kernel_info(loaderparams, lams->fw_cfg); } -static void loongarch_direct_kernel_boot(LoongArchMachineState *lams) +static void loongarch_direct_kernel_boot(LoongArchMachineState *lams, + const struct loaderparams *loaderparams) { MachineState *machine = MACHINE(lams); int64_t kernel_addr = 0; LoongArchCPU *lacpu; int i; - kernel_addr = load_kernel_info(); + kernel_addr = load_kernel_info(loaderparams); if (!machine->firmware) { for (i = 0; i < machine->smp.cpus; i++) { lacpu = LOONGARCH_CPU(qemu_get_cpu(i)); @@ -793,6 +796,7 @@ static void loongarch_init(MachineState *machine) MachineClass *mc = MACHINE_GET_CLASS(machine); CPUState *cpu; char *ramName = NULL; + struct loaderparams loaderparams = { }; if (!cpu_model) { cpu_model = LOONGARCH_CPU_TYPE_NAME("la464"); @@ -898,9 +902,9 @@ static void loongarch_init(MachineState *machine) /* load the kernel. */ if (loaderparams.kernel_filename) { if (lams->bios_loaded) { - loongarch_firmware_boot(lams); + loongarch_firmware_boot(lams, &loaderparams); } else { - loongarch_direct_kernel_boot(lams); + loongarch_direct_kernel_boot(lams, &loaderparams); } } fdt_add_flash_node(lams);