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

Tue, 10 Oct 2023 04:27:52 -0700 

在 2023/10/10 下午5:49, Thomas Weißschuh 写道:

Passing the struct around explicitly makes the control-flow more
obvious.

Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org>
Signed-off-by: Thomas Weißschuh <tho...@t-8ch.de>
---
Changes in v2:
- Drop initialize loaderparams variable
- Link to v1: 
https://lore.kernel.org/qemu-devel/20231009210018.249776-1-tho...@t-8ch.de/
---
  hw/loongarch/virt.c | 50 +++++++++++++++++++++++++++-----------------------
  1 file changed, 27 insertions(+), 23 deletions(-)


Reviewed-by: Song Gao <gaos...@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,

Reply via email to