Re: [PATCH] hw/loongarch: remove global loaderparams variable

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

Hi Thomas,

On 9/10/23 23:00, Thomas Weißschuh wrote:
> Passing the struct around explicitly makes the control-flow more
> obvious.
> 
> Signed-off-by: Thomas Weißschuh <thomas@t-8ch.de>
> ---
>   hw/loongarch/virt.c | 50 ++++++++++++++++++++++++---------------------
>   1 file changed, 27 insertions(+), 23 deletions(-)


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

Please zero-initialize with '= { }', otherwise great!

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

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