[v2,01/17] hw/loongarch: Move boot fucntions to boot.c

Move some boot functions to boot.c and struct
loongarch_boot_info into struct LoongArchMachineState.

Signed-off-by: Song Gao <gaosong@loongson.cn>
---
 hw/loongarch/boot.c         | 127 ++++++++++++++++++++++++++++++++++++
 hw/loongarch/meson.build    |   1 +
 hw/loongarch/virt.c         | 118 ++-------------------------------
 include/hw/loongarch/boot.h |  21 ++++++
 include/hw/loongarch/virt.h |   2 +
 5 files changed, 155 insertions(+), 114 deletions(-)
 create mode 100644 hw/loongarch/boot.c
 create mode 100644 include/hw/loongarch/boot.h

On 2023/12/18 下午5:00, Song Gao wrote:
> Move some boot functions to boot.c and struct
> loongarch_boot_info into struct LoongArchMachineState.
> 
> Signed-off-by: Song Gao <gaosong@loongson.cn>
> ---
>   hw/loongarch/boot.c         | 127 ++++++++++++++++++++++++++++++++++++
>   hw/loongarch/meson.build    |   1 +
>   hw/loongarch/virt.c         | 118 ++-------------------------------
>   include/hw/loongarch/boot.h |  21 ++++++
>   include/hw/loongarch/virt.h |   2 +
>   5 files changed, 155 insertions(+), 114 deletions(-)
>   create mode 100644 hw/loongarch/boot.c
>   create mode 100644 include/hw/loongarch/boot.h
> 
> diff --git a/hw/loongarch/boot.c b/hw/loongarch/boot.c
> new file mode 100644
> index 0000000000..9f25ea5847
> --- /dev/null
> +++ b/hw/loongarch/boot.c
> @@ -0,0 +1,127 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * LoongArch boot helper functions.
> + *
> + * Copyright (c) 2023 Loongson Technology Corporation Limited
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/units.h"
> +#include "target/loongarch/cpu.h"
> +#include "hw/loongarch/virt.h"
> +#include "hw/loader.h"
> +#include "elf.h"
> +#include "qemu/error-report.h"
> +#include "sysemu/reset.h"
> +
> +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(struct loongarch_boot_info *info)
> +{
> +    uint64_t kernel_entry, kernel_low, kernel_high;
> +    ssize_t kernel_size;
> +
> +    kernel_size = load_elf(info->kernel_filename, NULL,
> +                           cpu_loongarch_virt_to_phys, NULL,
> +                           &kernel_entry, &kernel_low,
> +                           &kernel_high, NULL, 0,
> +                           EM_LOONGARCH, 1, 0);
> +
> +    if (kernel_size < 0) {
> +        error_report("could not load kernel '%s': %s",
> +                     info->kernel_filename,
> +                     load_elf_strerror(kernel_size));
> +        exit(1);
> +    }
> +    return kernel_entry;
> +}
> +
> +static void reset_load_elf(void *opaque)
> +{
> +    LoongArchCPU *cpu = opaque;
> +    CPULoongArchState *env = &cpu->env;
> +
> +    cpu_reset(CPU(cpu));
> +    if (env->load_elf) {
> +        cpu_set_pc(CPU(cpu), env->elf_address);
> +    }
> +}
> +
> +static void fw_cfg_add_kernel_info(struct loongarch_boot_info *info,
> +                                   FWCfgState *fw_cfg)
> +{
> +    /*
> +     * Expose the kernel, the command line, and the initrd in fw_cfg.
> +     * We don't process them here at all, it's all left to the
> +     * firmware.
> +     */
> +    load_image_to_fw_cfg(fw_cfg,
> +                         FW_CFG_KERNEL_SIZE, FW_CFG_KERNEL_DATA,
> +                         info->kernel_filename,
> +                         false);
> +
> +    if (info->initrd_filename) {
> +        load_image_to_fw_cfg(fw_cfg,
> +                             FW_CFG_INITRD_SIZE, FW_CFG_INITRD_DATA,
> +                             info->initrd_filename, false);
> +    }
> +
> +    if (info->kernel_cmdline) {
> +        fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE,
> +                       strlen(info->kernel_cmdline) + 1);
> +        fw_cfg_add_string(fw_cfg, FW_CFG_CMDLINE_DATA,
> +                          info->kernel_cmdline);
> +    }
> +}
> +
> +static void loongarch_firmware_boot(LoongArchMachineState *lams,
> +                                    struct loongarch_boot_info *info)
> +{
> +    fw_cfg_add_kernel_info(info, lams->fw_cfg);
> +}
> +
> +static void loongarch_direct_kernel_boot(LoongArchMachineState *lams,
> +                                         struct loongarch_boot_info *info)
> +{
> +    MachineState *machine = MACHINE(lams);
> +    int64_t kernel_addr = 0;
> +    LoongArchCPU *lacpu;
> +    int i;
> +
> +    if (info->kernel_filename) {
> +        kernel_addr = load_kernel_info(info);
> +    } else {
> +        error_report("Need kernel filename\n");
> +        exit(1);
> +    }
> +
> +    for (i = 0; i < machine->smp.cpus; i++) {
> +        lacpu = LOONGARCH_CPU(qemu_get_cpu(i));
> +        lacpu->env.load_elf = true;
> +        lacpu->env.elf_address = kernel_addr;
> +    }
> +}
> +
> +void loongarch_load_kernel(MachineState *ms, struct loongarch_boot_info *info)
> +{
> +    LoongArchMachineState *lams = LOONGARCH_MACHINE(ms);
> +    int i;
> +
> +    /* register reset function */
> +    for (i = 0; i < ms->smp.cpus; i++) {
> +        qemu_register_reset(reset_load_elf, LOONGARCH_CPU(qemu_get_cpu(i)));
> +    }
> +
> +    info->kernel_filename = ms->kernel_filename;
> +    info->kernel_cmdline = ms->kernel_cmdline;
> +    info->initrd_filename = ms->initrd_filename;
> +
> +    if (lams->bios_loaded) {
> +        loongarch_firmware_boot(lams, info);
> +    } else {
> +        loongarch_direct_kernel_boot(lams, info);
> +    }
> +}
> diff --git a/hw/loongarch/meson.build b/hw/loongarch/meson.build
> index c0421502ab..d306d82c2e 100644
> --- a/hw/loongarch/meson.build
> +++ b/hw/loongarch/meson.build
> @@ -1,6 +1,7 @@
>   loongarch_ss = ss.source_set()
>   loongarch_ss.add(files(
>       'fw_cfg.c',
> +    'boot.c',
>   ))
>   loongarch_ss.add(when: 'CONFIG_LOONGARCH_VIRT', if_true: [files('virt.c'), fdt])
>   loongarch_ss.add(when: 'CONFIG_ACPI', if_true: files('acpi-build.c'))
> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
> index 4b7dc67a2d..3e27d72f55 100644
> --- a/hw/loongarch/virt.c
> +++ b/hw/loongarch/virt.c
> @@ -46,14 +46,6 @@
>   #include "hw/block/flash.h"
>   #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);
> @@ -376,31 +368,6 @@ static void memmap_add_entry(uint64_t address, uint64_t length, uint32_t type)
>       memmap_entries++;
>   }
>   
> -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(const struct loaderparams *loaderparams)
> -{
> -    uint64_t kernel_entry, kernel_low, kernel_high;
> -    ssize_t kernel_size;
> -
> -    kernel_size = load_elf(loaderparams->kernel_filename, NULL,
> -                           cpu_loongarch_virt_to_phys, NULL,
> -                           &kernel_entry, &kernel_low,
> -                           &kernel_high, NULL, 0,
> -                           EM_LOONGARCH, 1, 0);
> -
> -    if (kernel_size < 0) {
> -        error_report("could not load kernel '%s': %s",
> -                     loaderparams->kernel_filename,
> -                     load_elf_strerror(kernel_size));
> -        exit(1);
> -    }
> -    return kernel_entry;
> -}
> -
>   static DeviceState *create_acpi_ged(DeviceState *pch_pic, LoongArchMachineState *lams)
>   {
>       DeviceState *dev;
> @@ -668,69 +635,6 @@ static void loongarch_firmware_init(LoongArchMachineState *lams)
>           memory_region_add_subregion(get_system_memory(), VIRT_BIOS_BASE, &lams->bios);
>           lams->bios_loaded = true;
>       }
> -
> -}
> -
> -static void reset_load_elf(void *opaque)
> -{
> -    LoongArchCPU *cpu = opaque;
> -    CPULoongArchState *env = &cpu->env;
> -
> -    cpu_reset(CPU(cpu));
> -    if (env->load_elf) {
> -        cpu_set_pc(CPU(cpu), env->elf_address);
> -    }
> -}
> -
> -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.
> -     * We don't process them here at all, it's all left to the
> -     * firmware.
> -     */
> -    load_image_to_fw_cfg(fw_cfg,
> -                         FW_CFG_KERNEL_SIZE, FW_CFG_KERNEL_DATA,
> -                         loaderparams->kernel_filename,
> -                         false);
> -
> -    if (loaderparams->initrd_filename) {
> -        load_image_to_fw_cfg(fw_cfg,
> -                             FW_CFG_INITRD_SIZE, FW_CFG_INITRD_DATA,
> -                             loaderparams->initrd_filename, false);
> -    }
> -
> -    if (loaderparams->kernel_cmdline) {
> -        fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE,
> -                       strlen(loaderparams->kernel_cmdline) + 1);
> -        fw_cfg_add_string(fw_cfg, FW_CFG_CMDLINE_DATA,
> -                          loaderparams->kernel_cmdline);
> -    }
> -}
> -
> -static void loongarch_firmware_boot(LoongArchMachineState *lams,
> -                                    const struct loaderparams *loaderparams)
> -{
> -    fw_cfg_add_kernel_info(loaderparams, lams->fw_cfg);
> -}
> -
> -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(loaderparams);
> -    if (!machine->firmware) {
> -        for (i = 0; i < machine->smp.cpus; i++) {
> -            lacpu = LOONGARCH_CPU(qemu_get_cpu(i));
> -            lacpu->env.load_elf = true;
> -            lacpu->env.elf_address = kernel_addr;
> -        }
> -    }
>   }
>   
>   static void loongarch_init(MachineState *machine)
> @@ -750,7 +654,6 @@ 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");
> @@ -844,24 +747,8 @@ static void loongarch_init(MachineState *machine)
>                           sizeof(struct memmap_entry) * (memmap_entries));
>       }
>       fdt_add_fw_cfg_node(lams);
> -    loaderparams.ram_size = ram_size;
> -    loaderparams.kernel_filename = machine->kernel_filename;
> -    loaderparams.kernel_cmdline = machine->kernel_cmdline;
> -    loaderparams.initrd_filename = machine->initrd_filename;
> -    /* load the kernel. */
> -    if (loaderparams.kernel_filename) {
> -        if (lams->bios_loaded) {
> -            loongarch_firmware_boot(lams, &loaderparams);
> -        } else {
> -            loongarch_direct_kernel_boot(lams, &loaderparams);
> -        }
> -    }
>       fdt_add_flash_node(lams);
> -    /* register reset function */
> -    for (i = 0; i < machine->smp.cpus; i++) {
> -        lacpu = LOONGARCH_CPU(qemu_get_cpu(i));
How about use CPU_FOREACH(cpu) here?

There is while-loop in function qemu_get_cpu, with for loop in previous 
line there will be double loop sentences.

Regards
Bibo Mao

> -        qemu_register_reset(reset_load_elf, lacpu);
> -    }
> +
>       /* Initialize the IO interrupt subsystem */
>       loongarch_irq_init(lams);
>       fdt_add_irqchip_node(lams);
> @@ -886,6 +773,9 @@ static void loongarch_init(MachineState *machine)
>       fdt_base = 1 * MiB;
>       qemu_fdt_dumpdtb(machine->fdt, lams->fdt_size);
>       rom_add_blob_fixed("fdt", machine->fdt, lams->fdt_size, fdt_base);
> +
> +    lams->bootinfo.ram_size = ram_size;
> +    loongarch_load_kernel(machine, &lams->bootinfo);
>   }
>   
>   bool loongarch_is_acpi_enabled(LoongArchMachineState *lams)
> diff --git a/include/hw/loongarch/boot.h b/include/hw/loongarch/boot.h
> new file mode 100644
> index 0000000000..3275c1e295
> --- /dev/null
> +++ b/include/hw/loongarch/boot.h
> @@ -0,0 +1,21 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Definitions for LoongArch boot.
> + *
> + * Copyright (C) 2023 Loongson Technology Corporation Limited
> + */
> +
> +#ifndef HW_LOONGARCH_BOOT_H
> +#define HW_LOONGARCH_BOOT_H
> +
> +struct loongarch_boot_info {
> +    uint64_t ram_size;
> +    const char *kernel_filename;
> +    const char *kernel_cmdline;
> +    const char *initrd_filename;
> +    uint64_t a0, a1, a2;
> +};
> +
> +void loongarch_load_kernel(MachineState *ms, struct loongarch_boot_info *info);
> +
> +#endif /* HW_LOONGARCH_BOOT_H */
> diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
> index 674f4655e0..e4126dd0e7 100644
> --- a/include/hw/loongarch/virt.h
> +++ b/include/hw/loongarch/virt.h
> @@ -13,6 +13,7 @@
>   #include "qemu/queue.h"
>   #include "hw/intc/loongarch_ipi.h"
>   #include "hw/block/flash.h"
> +#include "hw/loongarch/boot.h"
>   
>   #define LOONGARCH_MAX_CPUS      256
>   
> @@ -50,6 +51,7 @@ struct LoongArchMachineState {
>       DeviceState *platform_bus_dev;
>       PCIBus       *pci_bus;
>       PFlashCFI01  *flash;
> +    struct loongarch_boot_info bootinfo;
>   };
>   
>   #define TYPE_LOONGARCH_MACHINE  MACHINE_TYPE_NAME("virt")
>

在 2023/12/21 下午3:04, maobibo 写道:
>
>
> On 2023/12/18 下午5:00, Song Gao wrote:
>> Move some boot functions to boot.c and struct
>> loongarch_boot_info into struct LoongArchMachineState.
>>
>> Signed-off-by: Song Gao <gaosong@loongson.cn>
>> ---
>>   hw/loongarch/boot.c         | 127 ++++++++++++++++++++++++++++++++++++
>>   hw/loongarch/meson.build    |   1 +
>>   hw/loongarch/virt.c         | 118 ++-------------------------------
>>   include/hw/loongarch/boot.h |  21 ++++++
>>   include/hw/loongarch/virt.h |   2 +
>>   5 files changed, 155 insertions(+), 114 deletions(-)
>>   create mode 100644 hw/loongarch/boot.c
>>   create mode 100644 include/hw/loongarch/boot.h
>>
>> diff --git a/hw/loongarch/boot.c b/hw/loongarch/boot.c
>> new file mode 100644
>> index 0000000000..9f25ea5847
>> --- /dev/null
>> +++ b/hw/loongarch/boot.c
>> @@ -0,0 +1,127 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * LoongArch boot helper functions.
>> + * Yes,  #include <>
>> + * Copyright (c) 2023 Loongson Technology Corporation Limited
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qemu/units.h"
>> +#include "target/loongarch/cpu.h"
>> +#include "hw/loongarch/virt.h"
>> +#include "hw/loader.h"
>> +#include "elf.h"
>> +#include "qemu/error-report.h"
>> +#include "sysemu/reset.h"
>> +
>> +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(struct loongarch_boot_info *info)
>> +{
>> +    uint64_t kernel_entry, kernel_low, kernel_high;
>> +    ssize_t kernel_size;
>> +
>> +    kernel_size = load_elf(info->kernel_filename, NULL,
>> +                           cpu_loongarch_virt_to_phys, NULL,
>> +                           &kernel_entry, &kernel_low,
>> +                           &kernel_high, NULL, 0,
>> +                           EM_LOONGARCH, 1, 0);
>> +
>> +    if (kernel_size < 0) {
>> +        error_report("could not load kernel '%s': %s",
>> +                     info->kernel_filename,
>> +                     load_elf_strerror(kernel_size));
>> +        exit(1);
>> +    }
>> +    return kernel_entry;
>> +}
>> +
>> +static void reset_load_elf(void *opaque)
>> +{
>> +    LoongArchCPU *cpu = opaque;
>> +    CPULoongArchState *env = &cpu->env;
>> +
>> +    cpu_reset(CPU(cpu));
>> +    if (env->load_elf) {
>> +        cpu_set_pc(CPU(cpu), env->elf_address);
>> +    }
>> +}
>> +
>> +static void fw_cfg_add_kernel_info(struct loongarch_boot_info *info,
>> +                                   FWCfgState *fw_cfg)
>> +{
>> +    /*
>> +     * Expose the kernel, the command line, and the initrd in fw_cfg.
>> +     * We don't process them here at all, it's all left to the
>> +     * firmware.
>> +     */
>> +    load_image_to_fw_cfg(fw_cfg,
>> +                         FW_CFG_KERNEL_SIZE, FW_CFG_KERNEL_DATA,
>> +                         info->kernel_filename,
>> +                         false);
>> +
>> +    if (info->initrd_filename) {
>> +        load_image_to_fw_cfg(fw_cfg,
>> +                             FW_CFG_INITRD_SIZE, FW_CFG_INITRD_DATA,
>> +                             info->initrd_filename, false);
>> +    }
>> +
>> +    if (info->kernel_cmdline) {
>> +        fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE,
>> +                       strlen(info->kernel_cmdline) + 1);
>> +        fw_cfg_add_string(fw_cfg, FW_CFG_CMDLINE_DATA,
>> +                          info->kernel_cmdline);
>> +    }
>> +}
>> +
>> +static void loongarch_firmware_boot(LoongArchMachineState *lams,
>> +                                    struct loongarch_boot_info *info)
>> +{
>> +    fw_cfg_add_kernel_info(info, lams->fw_cfg);
>> +}
>> +
>> +static void loongarch_direct_kernel_boot(LoongArchMachineState *lams,
>> +                                         struct loongarch_boot_info 
>> *info)
>> +{
>> +    MachineState *machine = MACHINE(lams);
>> +    int64_t kernel_addr = 0;
>> +    LoongArchCPU *lacpu;
>> +    int i;
>> +
>> +    if (info->kernel_filename) {
>> +        kernel_addr = load_kernel_info(info);
>> +    } else {
>> +        error_report("Need kernel filename\n");
>> +        exit(1);
>> +    }
>> +
>> +    for (i = 0; i < machine->smp.cpus; i++) {
>> +        lacpu = LOONGARCH_CPU(qemu_get_cpu(i));
>> +        lacpu->env.load_elf = true;
>> +        lacpu->env.elf_address = kernel_addr;
>> +    }
>> +}
>> +
>> +void loongarch_load_kernel(MachineState *ms, struct 
>> loongarch_boot_info *info)
>> +{
>> +    LoongArchMachineState *lams = LOONGARCH_MACHINE(ms);
>> +    int i;
>> +
>> +    /* register reset function */
>> +    for (i = 0; i < ms->smp.cpus; i++) {
>> +        qemu_register_reset(reset_load_elf, 
>> LOONGARCH_CPU(qemu_get_cpu(i)));
>> +    }
>> +
>> +    info->kernel_filename = ms->kernel_filename;
>> +    info->kernel_cmdline = ms->kernel_cmdline;
>> +    info->initrd_filename = ms->initrd_filename;
>> +
>> +    if (lams->bios_loaded) {
>> +        loongarch_firmware_boot(lams, info);
>> +    } else {
>> +        loongarch_direct_kernel_boot(lams, info);
>> +    }
>> +}
>> diff --git a/hw/loongarch/meson.build b/hw/loongarch/meson.build
>> index c0421502ab..d306d82c2e 100644
>> --- a/hw/loongarch/meson.build
>> +++ b/hw/loongarch/meson.build
>> @@ -1,6 +1,7 @@
>>   loongarch_ss = ss.source_set()
>>   loongarch_ss.add(files(
>>       'fw_cfg.c',
>> +    'boot.c',
>>   ))
>>   loongarch_ss.add(when: 'CONFIG_LOONGARCH_VIRT', if_true: 
>> [files('virt.c'), fdt])
>>   loongarch_ss.add(when: 'CONFIG_ACPI', if_true: files('acpi-build.c'))
>> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
>> index 4b7dc67a2d..3e27d72f55 100644
>> --- a/hw/loongarch/virt.c
>> +++ b/hw/loongarch/virt.c
>> @@ -46,14 +46,6 @@
>>   #include "hw/block/flash.h"
>>   #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);
>> @@ -376,31 +368,6 @@ static void memmap_add_entry(uint64_t address, 
>> uint64_t length, uint32_t type)
>>       memmap_entries++;
>>   }
>>   -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(const struct loaderparams 
>> *loaderparams)
>> -{
>> -    uint64_t kernel_entry, kernel_low, kernel_high;
>> -    ssize_t kernel_size;
>> -
>> -    kernel_size = load_elf(loaderparams->kernel_filename, NULL,
>> -                           cpu_loongarch_virt_to_phys, NULL,
>> -                           &kernel_entry, &kernel_low,
>> -                           &kernel_high, NULL, 0,
>> -                           EM_LOONGARCH, 1, 0);
>> -
>> -    if (kernel_size < 0) {
>> -        error_report("could not load kernel '%s': %s",
>> -                     loaderparams->kernel_filename,
>> -                     load_elf_strerror(kernel_size));
>> -        exit(1);
>> -    }
>> -    return kernel_entry;
>> -}
>> -
>>   static DeviceState *create_acpi_ged(DeviceState *pch_pic, 
>> LoongArchMachineState *lams)
>>   {
>>       DeviceState *dev;
>> @@ -668,69 +635,6 @@ static void 
>> loongarch_firmware_init(LoongArchMachineState *lams)
>>           memory_region_add_subregion(get_system_memory(), 
>> VIRT_BIOS_BASE, &lams->bios);
>>           lams->bios_loaded = true;
>>       }
>> -
>> -}
>> -
>> -static void reset_load_elf(void *opaque)
>> -{
>> -    LoongArchCPU *cpu = opaque;
>> -    CPULoongArchState *env = &cpu->env;
>> -
>> -    cpu_reset(CPU(cpu));
>> -    if (env->load_elf) {
>> -        cpu_set_pc(CPU(cpu), env->elf_address);
>> -    }
>> -}
>> -
>> -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.
>> -     * We don't process them here at all, it's all left to the
>> -     * firmware.
>> -     */
>> -    load_image_to_fw_cfg(fw_cfg,
>> -                         FW_CFG_KERNEL_SIZE, FW_CFG_KERNEL_DATA,
>> -                         loaderparams->kernel_filename,
>> -                         false);
>> -
>> -    if (loaderparams->initrd_filename) {
>> -        load_image_to_fw_cfg(fw_cfg,
>> -                             FW_CFG_INITRD_SIZE, FW_CFG_INITRD_DATA,
>> -                             loaderparams->initrd_filename, false);
>> -    }
>> -
>> -    if (loaderparams->kernel_cmdline) {
>> -        fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE,
>> -                       strlen(loaderparams->kernel_cmdline) + 1);
>> -        fw_cfg_add_string(fw_cfg, FW_CFG_CMDLINE_DATA,
>> -                          loaderparams->kernel_cmdline);
>> -    }
>> -}
>> -
>> -static void loongarch_firmware_boot(LoongArchMachineState *lams,
>> -                                    const struct loaderparams 
>> *loaderparams)
>> -{
>> -    fw_cfg_add_kernel_info(loaderparams, lams->fw_cfg);
>> -}
>> -
>> -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(loaderparams);
>> -    if (!machine->firmware) {
>> -        for (i = 0; i < machine->smp.cpus; i++) {
>> -            lacpu = LOONGARCH_CPU(qemu_get_cpu(i));
>> -            lacpu->env.load_elf = true;
>> -            lacpu->env.elf_address = kernel_addr;
>> -        }
>> -    }
>>   }
>>     static void loongarch_init(MachineState *machine)
>> @@ -750,7 +654,6 @@ 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");
>> @@ -844,24 +747,8 @@ static void loongarch_init(MachineState *machine)
>>                           sizeof(struct memmap_entry) * 
>> (memmap_entries));
>>       }
>>       fdt_add_fw_cfg_node(lams);
>> -    loaderparams.ram_size = ram_size;
>> -    loaderparams.kernel_filename = machine->kernel_filename;
>> -    loaderparams.kernel_cmdline = machine->kernel_cmdline;
>> -    loaderparams.initrd_filename = machine->initrd_filename;
>> -    /* load the kernel. */
>> -    if (loaderparams.kernel_filename) {
>> -        if (lams->bios_loaded) {
>> -            loongarch_firmware_boot(lams, &loaderparams);
>> -        } else {
>> -            loongarch_direct_kernel_boot(lams, &loaderparams);
>> -        }
>> -    }
>>       fdt_add_flash_node(lams);
>> -    /* register reset function */
>> -    for (i = 0; i < machine->smp.cpus; i++) {
>> -        lacpu = LOONGARCH_CPU(qemu_get_cpu(i));
> How about use CPU_FOREACH(cpu) here?
>
> There is while-loop in function qemu_get_cpu, with for loop in 
> previous line there will be double loop sentences.
>
Got it, I will correct it on v3.

Thanks,
Song Gao
> Regards
> Bibo Mao
>
>> - qemu_register_reset(reset_load_elf, lacpu);
>> -    }
>> +
>>       /* Initialize the IO interrupt subsystem */
>>       loongarch_irq_init(lams);
>>       fdt_add_irqchip_node(lams);
>> @@ -886,6 +773,9 @@ static void loongarch_init(MachineState *machine)
>>       fdt_base = 1 * MiB;
>>       qemu_fdt_dumpdtb(machine->fdt, lams->fdt_size);
>>       rom_add_blob_fixed("fdt", machine->fdt, lams->fdt_size, fdt_base);
>> +
>> +    lams->bootinfo.ram_size = ram_size;
>> +    loongarch_load_kernel(machine, &lams->bootinfo);
>>   }
>>     bool loongarch_is_acpi_enabled(LoongArchMachineState *lams)
>> diff --git a/include/hw/loongarch/boot.h b/include/hw/loongarch/boot.h
>> new file mode 100644
>> index 0000000000..3275c1e295
>> --- /dev/null
>> +++ b/include/hw/loongarch/boot.h
>> @@ -0,0 +1,21 @@
>> +/* SPDX-License-Identifier: GPL-2.0-or-later */
>> +/*
>> + * Definitions for LoongArch boot.
>> + *
>> + * Copyright (C) 2023 Loongson Technology Corporation Limited
>> + */
>> +
>> +#ifndef HW_LOONGARCH_BOOT_H
>> +#define HW_LOONGARCH_BOOT_H
>> +
>> +struct loongarch_boot_info {
>> +    uint64_t ram_size;
>> +    const char *kernel_filename;
>> +    const char *kernel_cmdline;
>> +    const char *initrd_filename;
>> +    uint64_t a0, a1, a2;
>> +};
>> +
>> +void loongarch_load_kernel(MachineState *ms, struct 
>> loongarch_boot_info *info);
>> +
>> +#endif /* HW_LOONGARCH_BOOT_H */
>> diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
>> index 674f4655e0..e4126dd0e7 100644
>> --- a/include/hw/loongarch/virt.h
>> +++ b/include/hw/loongarch/virt.h
>> @@ -13,6 +13,7 @@
>>   #include "qemu/queue.h"
>>   #include "hw/intc/loongarch_ipi.h"
>>   #include "hw/block/flash.h"
>> +#include "hw/loongarch/boot.h"
>>     #define LOONGARCH_MAX_CPUS      256
>>   @@ -50,6 +51,7 @@ struct LoongArchMachineState {
>>       DeviceState *platform_bus_dev;
>>       PCIBus       *pci_bus;
>>       PFlashCFI01  *flash;
>> +    struct loongarch_boot_info bootinfo;
>>   };
>>     #define TYPE_LOONGARCH_MACHINE  MACHINE_TYPE_NAME("virt")
>>