diff mbox series

[v1,07/16] hw/riscv: sifive_u: Remove compile time XLEN checks

Message ID 7ce5ff70057b2fad2d455887d048c419a7e42746.1603467169.git.alistair.francis@wdc.com (mailing list archive)
State New, archived
Headers show
Series RISC-V: Start to remove xlen preprocess | expand

Commit Message

Alistair Francis Oct. 23, 2020, 3:33 p.m. UTC
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 hw/riscv/sifive_u.c | 57 ++++++++++++++++++++++++---------------------
 1 file changed, 31 insertions(+), 26 deletions(-)

Comments

Bin Meng Oct. 26, 2020, 8:56 a.m. UTC | #1
On Fri, Oct 23, 2020 at 11:44 PM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  hw/riscv/sifive_u.c | 57 ++++++++++++++++++++++++---------------------
>  1 file changed, 31 insertions(+), 26 deletions(-)
>
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index 978cfedd24..3bc32c9225 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -60,12 +60,6 @@
>
>  #include <libfdt.h>
>
> -#if defined(TARGET_RISCV32)
> -# define BIOS_FILENAME "opensbi-riscv32-generic-fw_dynamic.bin"
> -#else
> -# define BIOS_FILENAME "opensbi-riscv64-generic-fw_dynamic.bin"
> -#endif
> -
>  static const struct MemmapEntry {
>      hwaddr base;
>      hwaddr size;
> @@ -93,7 +87,7 @@ static const struct MemmapEntry {
>  #define GEM_REVISION        0x10070109
>
>  static void create_fdt(SiFiveUState *s, const struct MemmapEntry *memmap,
> -    uint64_t mem_size, const char *cmdline)
> +                       uint64_t mem_size, const char *cmdline, bool is_32_bit)
>  {
>      MachineState *ms = MACHINE(qdev_get_machine());
>      void *fdt;
> @@ -167,11 +161,11 @@ static void create_fdt(SiFiveUState *s, const struct MemmapEntry *memmap,
>          qemu_fdt_add_subnode(fdt, nodename);
>          /* cpu 0 is the management hart that does not have mmu */
>          if (cpu != 0) {
> -#if defined(TARGET_RISCV32)
> -            qemu_fdt_setprop_string(fdt, nodename, "mmu-type", "riscv,sv32");
> -#else
> -            qemu_fdt_setprop_string(fdt, nodename, "mmu-type", "riscv,sv48");
> -#endif
> +            if (is_32_bit) {
> +                qemu_fdt_setprop_string(fdt, nodename, "mmu-type", "riscv,sv32");
> +            } else {
> +                qemu_fdt_setprop_string(fdt, nodename, "mmu-type", "riscv,sv48");
> +            }
>              isa = riscv_isa_string(&s->soc.u_cpus.harts[cpu - 1]);
>          } else {
>              isa = riscv_isa_string(&s->soc.e_cpus.harts[0]);
> @@ -414,7 +408,7 @@ static void sifive_u_machine_init(MachineState *machine)
>      MemoryRegion *system_memory = get_system_memory();
>      MemoryRegion *main_mem = g_new(MemoryRegion, 1);
>      MemoryRegion *flash0 = g_new(MemoryRegion, 1);
> -    target_ulong start_addr = memmap[SIFIVE_U_DEV_DRAM].base;
> +    uint64_t start_addr = memmap[SIFIVE_U_DEV_DRAM].base;

Why is this change necessary? If yes, should we do the same change to virt.c?

>      target_ulong firmware_end_addr, kernel_start_addr;
>      uint32_t start_addr_hi32 = 0x00000000;
>      int i;
> @@ -446,7 +440,8 @@ static void sifive_u_machine_init(MachineState *machine)
>                            qemu_allocate_irq(sifive_u_machine_reset, NULL, 0));
>
>      /* create device tree */
> -    create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline);
> +    create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline,
> +               riscv_is_32_bit(machine));
>
>      if (s->start_in_flash) {
>          /*
> @@ -475,8 +470,15 @@ static void sifive_u_machine_init(MachineState *machine)
>          break;
>      }
>
> -    firmware_end_addr = riscv_find_and_load_firmware(machine, BIOS_FILENAME,
> -                                                     start_addr, NULL);
> +    if (riscv_is_32_bit(machine)) {
> +        firmware_end_addr = riscv_find_and_load_firmware(machine,
> +                                    "opensbi-riscv32-generic-fw_dynamic.bin",
> +                                    start_addr, NULL);
> +    } else {
> +        firmware_end_addr = riscv_find_and_load_firmware(machine,
> +                                    "opensbi-riscv64-generic-fw_dynamic.bin",
> +                                    start_addr, NULL);
> +    }
>
>      if (machine->kernel_filename) {
>          kernel_start_addr = riscv_calc_kernel_start_addr(machine,
> @@ -506,9 +508,9 @@ static void sifive_u_machine_init(MachineState *machine)
>      /* Compute the fdt load address in dram */
>      fdt_load_addr = riscv_load_fdt(memmap[SIFIVE_U_DEV_DRAM].base,
>                                     machine->ram_size, s->fdt);
> -    #if defined(TARGET_RISCV64)
> -    start_addr_hi32 = start_addr >> 32;
> -    #endif
> +    if (!riscv_is_32_bit(machine)) {
> +        start_addr_hi32 = start_addr >> 32;
> +    }
>
>      /* reset vector */
>      uint32_t reset_vec[11] = {
> @@ -516,13 +518,8 @@ static void sifive_u_machine_init(MachineState *machine)
>          0x00000297,                    /* 1:  auipc  t0, %pcrel_hi(fw_dyn) */
>          0x02828613,                    /*     addi   a2, t0, %pcrel_lo(1b) */
>          0xf1402573,                    /*     csrr   a0, mhartid  */
> -#if defined(TARGET_RISCV32)
> -        0x0202a583,                    /*     lw     a1, 32(t0) */
> -        0x0182a283,                    /*     lw     t0, 24(t0) */
> -#elif defined(TARGET_RISCV64)
> -        0x0202b583,                    /*     ld     a1, 32(t0) */
> -        0x0182b283,                    /*     ld     t0, 24(t0) */
> -#endif
> +        0,
> +        0,
>          0x00028067,                    /*     jr     t0 */
>          start_addr,                    /* start: .dword */
>          start_addr_hi32,
> @@ -530,6 +527,14 @@ static void sifive_u_machine_init(MachineState *machine)
>          0x00000000,
>                                         /* fw_dyn: */
>      };
> +    if (riscv_is_32_bit(machine)) {
> +        reset_vec[4] = 0x0202a583;     /*     lw     a1, 32(t0) */
> +        reset_vec[5] = 0x0182a283;     /*     lw     t0, 24(t0) */
> +    } else {
> +        reset_vec[4] = 0x0202b583;     /*     ld     a1, 32(t0) */
> +        reset_vec[5] = 0x0182b283;     /*     ld     t0, 24(t0) */
> +    }
> +
>
>      /* copy in the reset vector in little_endian byte order */
>      for (i = 0; i < ARRAY_SIZE(reset_vec); i++) {

Regards,
Bin
Alistair Francis Oct. 26, 2020, 3:12 p.m. UTC | #2
On Mon, Oct 26, 2020 at 1:56 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Fri, Oct 23, 2020 at 11:44 PM Alistair Francis
> <alistair.francis@wdc.com> wrote:
> >
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  hw/riscv/sifive_u.c | 57 ++++++++++++++++++++++++---------------------
> >  1 file changed, 31 insertions(+), 26 deletions(-)
> >
> > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> > index 978cfedd24..3bc32c9225 100644
> > --- a/hw/riscv/sifive_u.c
> > +++ b/hw/riscv/sifive_u.c
> > @@ -60,12 +60,6 @@
> >
> >  #include <libfdt.h>
> >
> > -#if defined(TARGET_RISCV32)
> > -# define BIOS_FILENAME "opensbi-riscv32-generic-fw_dynamic.bin"
> > -#else
> > -# define BIOS_FILENAME "opensbi-riscv64-generic-fw_dynamic.bin"
> > -#endif
> > -
> >  static const struct MemmapEntry {
> >      hwaddr base;
> >      hwaddr size;
> > @@ -93,7 +87,7 @@ static const struct MemmapEntry {
> >  #define GEM_REVISION        0x10070109
> >
> >  static void create_fdt(SiFiveUState *s, const struct MemmapEntry *memmap,
> > -    uint64_t mem_size, const char *cmdline)
> > +                       uint64_t mem_size, const char *cmdline, bool is_32_bit)
> >  {
> >      MachineState *ms = MACHINE(qdev_get_machine());
> >      void *fdt;
> > @@ -167,11 +161,11 @@ static void create_fdt(SiFiveUState *s, const struct MemmapEntry *memmap,
> >          qemu_fdt_add_subnode(fdt, nodename);
> >          /* cpu 0 is the management hart that does not have mmu */
> >          if (cpu != 0) {
> > -#if defined(TARGET_RISCV32)
> > -            qemu_fdt_setprop_string(fdt, nodename, "mmu-type", "riscv,sv32");
> > -#else
> > -            qemu_fdt_setprop_string(fdt, nodename, "mmu-type", "riscv,sv48");
> > -#endif
> > +            if (is_32_bit) {
> > +                qemu_fdt_setprop_string(fdt, nodename, "mmu-type", "riscv,sv32");
> > +            } else {
> > +                qemu_fdt_setprop_string(fdt, nodename, "mmu-type", "riscv,sv48");
> > +            }
> >              isa = riscv_isa_string(&s->soc.u_cpus.harts[cpu - 1]);
> >          } else {
> >              isa = riscv_isa_string(&s->soc.e_cpus.harts[0]);
> > @@ -414,7 +408,7 @@ static void sifive_u_machine_init(MachineState *machine)
> >      MemoryRegion *system_memory = get_system_memory();
> >      MemoryRegion *main_mem = g_new(MemoryRegion, 1);
> >      MemoryRegion *flash0 = g_new(MemoryRegion, 1);
> > -    target_ulong start_addr = memmap[SIFIVE_U_DEV_DRAM].base;
> > +    uint64_t start_addr = memmap[SIFIVE_U_DEV_DRAM].base;
>
> Why is this change necessary? If yes, should we do the same change to virt.c?

Good catch, this isn't required.

I have removed this diff.

Alistair

>
> >      target_ulong firmware_end_addr, kernel_start_addr;
> >      uint32_t start_addr_hi32 = 0x00000000;
> >      int i;
> > @@ -446,7 +440,8 @@ static void sifive_u_machine_init(MachineState *machine)
> >                            qemu_allocate_irq(sifive_u_machine_reset, NULL, 0));
> >
> >      /* create device tree */
> > -    create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline);
> > +    create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline,
> > +               riscv_is_32_bit(machine));
> >
> >      if (s->start_in_flash) {
> >          /*
> > @@ -475,8 +470,15 @@ static void sifive_u_machine_init(MachineState *machine)
> >          break;
> >      }
> >
> > -    firmware_end_addr = riscv_find_and_load_firmware(machine, BIOS_FILENAME,
> > -                                                     start_addr, NULL);
> > +    if (riscv_is_32_bit(machine)) {
> > +        firmware_end_addr = riscv_find_and_load_firmware(machine,
> > +                                    "opensbi-riscv32-generic-fw_dynamic.bin",
> > +                                    start_addr, NULL);
> > +    } else {
> > +        firmware_end_addr = riscv_find_and_load_firmware(machine,
> > +                                    "opensbi-riscv64-generic-fw_dynamic.bin",
> > +                                    start_addr, NULL);
> > +    }
> >
> >      if (machine->kernel_filename) {
> >          kernel_start_addr = riscv_calc_kernel_start_addr(machine,
> > @@ -506,9 +508,9 @@ static void sifive_u_machine_init(MachineState *machine)
> >      /* Compute the fdt load address in dram */
> >      fdt_load_addr = riscv_load_fdt(memmap[SIFIVE_U_DEV_DRAM].base,
> >                                     machine->ram_size, s->fdt);
> > -    #if defined(TARGET_RISCV64)
> > -    start_addr_hi32 = start_addr >> 32;
> > -    #endif
> > +    if (!riscv_is_32_bit(machine)) {
> > +        start_addr_hi32 = start_addr >> 32;
> > +    }
> >
> >      /* reset vector */
> >      uint32_t reset_vec[11] = {
> > @@ -516,13 +518,8 @@ static void sifive_u_machine_init(MachineState *machine)
> >          0x00000297,                    /* 1:  auipc  t0, %pcrel_hi(fw_dyn) */
> >          0x02828613,                    /*     addi   a2, t0, %pcrel_lo(1b) */
> >          0xf1402573,                    /*     csrr   a0, mhartid  */
> > -#if defined(TARGET_RISCV32)
> > -        0x0202a583,                    /*     lw     a1, 32(t0) */
> > -        0x0182a283,                    /*     lw     t0, 24(t0) */
> > -#elif defined(TARGET_RISCV64)
> > -        0x0202b583,                    /*     ld     a1, 32(t0) */
> > -        0x0182b283,                    /*     ld     t0, 24(t0) */
> > -#endif
> > +        0,
> > +        0,
> >          0x00028067,                    /*     jr     t0 */
> >          start_addr,                    /* start: .dword */
> >          start_addr_hi32,
> > @@ -530,6 +527,14 @@ static void sifive_u_machine_init(MachineState *machine)
> >          0x00000000,
> >                                         /* fw_dyn: */
> >      };
> > +    if (riscv_is_32_bit(machine)) {
> > +        reset_vec[4] = 0x0202a583;     /*     lw     a1, 32(t0) */
> > +        reset_vec[5] = 0x0182a283;     /*     lw     t0, 24(t0) */
> > +    } else {
> > +        reset_vec[4] = 0x0202b583;     /*     ld     a1, 32(t0) */
> > +        reset_vec[5] = 0x0182b283;     /*     ld     t0, 24(t0) */
> > +    }
> > +
> >
> >      /* copy in the reset vector in little_endian byte order */
> >      for (i = 0; i < ARRAY_SIZE(reset_vec); i++) {
>
> Regards,
> Bin
diff mbox series

Patch

diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 978cfedd24..3bc32c9225 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -60,12 +60,6 @@ 
 
 #include <libfdt.h>
 
-#if defined(TARGET_RISCV32)
-# define BIOS_FILENAME "opensbi-riscv32-generic-fw_dynamic.bin"
-#else
-# define BIOS_FILENAME "opensbi-riscv64-generic-fw_dynamic.bin"
-#endif
-
 static const struct MemmapEntry {
     hwaddr base;
     hwaddr size;
@@ -93,7 +87,7 @@  static const struct MemmapEntry {
 #define GEM_REVISION        0x10070109
 
 static void create_fdt(SiFiveUState *s, const struct MemmapEntry *memmap,
-    uint64_t mem_size, const char *cmdline)
+                       uint64_t mem_size, const char *cmdline, bool is_32_bit)
 {
     MachineState *ms = MACHINE(qdev_get_machine());
     void *fdt;
@@ -167,11 +161,11 @@  static void create_fdt(SiFiveUState *s, const struct MemmapEntry *memmap,
         qemu_fdt_add_subnode(fdt, nodename);
         /* cpu 0 is the management hart that does not have mmu */
         if (cpu != 0) {
-#if defined(TARGET_RISCV32)
-            qemu_fdt_setprop_string(fdt, nodename, "mmu-type", "riscv,sv32");
-#else
-            qemu_fdt_setprop_string(fdt, nodename, "mmu-type", "riscv,sv48");
-#endif
+            if (is_32_bit) {
+                qemu_fdt_setprop_string(fdt, nodename, "mmu-type", "riscv,sv32");
+            } else {
+                qemu_fdt_setprop_string(fdt, nodename, "mmu-type", "riscv,sv48");
+            }
             isa = riscv_isa_string(&s->soc.u_cpus.harts[cpu - 1]);
         } else {
             isa = riscv_isa_string(&s->soc.e_cpus.harts[0]);
@@ -414,7 +408,7 @@  static void sifive_u_machine_init(MachineState *machine)
     MemoryRegion *system_memory = get_system_memory();
     MemoryRegion *main_mem = g_new(MemoryRegion, 1);
     MemoryRegion *flash0 = g_new(MemoryRegion, 1);
-    target_ulong start_addr = memmap[SIFIVE_U_DEV_DRAM].base;
+    uint64_t start_addr = memmap[SIFIVE_U_DEV_DRAM].base;
     target_ulong firmware_end_addr, kernel_start_addr;
     uint32_t start_addr_hi32 = 0x00000000;
     int i;
@@ -446,7 +440,8 @@  static void sifive_u_machine_init(MachineState *machine)
                           qemu_allocate_irq(sifive_u_machine_reset, NULL, 0));
 
     /* create device tree */
-    create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline);
+    create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline,
+               riscv_is_32_bit(machine));
 
     if (s->start_in_flash) {
         /*
@@ -475,8 +470,15 @@  static void sifive_u_machine_init(MachineState *machine)
         break;
     }
 
-    firmware_end_addr = riscv_find_and_load_firmware(machine, BIOS_FILENAME,
-                                                     start_addr, NULL);
+    if (riscv_is_32_bit(machine)) {
+        firmware_end_addr = riscv_find_and_load_firmware(machine,
+                                    "opensbi-riscv32-generic-fw_dynamic.bin",
+                                    start_addr, NULL);
+    } else {
+        firmware_end_addr = riscv_find_and_load_firmware(machine,
+                                    "opensbi-riscv64-generic-fw_dynamic.bin",
+                                    start_addr, NULL);
+    }
 
     if (machine->kernel_filename) {
         kernel_start_addr = riscv_calc_kernel_start_addr(machine,
@@ -506,9 +508,9 @@  static void sifive_u_machine_init(MachineState *machine)
     /* Compute the fdt load address in dram */
     fdt_load_addr = riscv_load_fdt(memmap[SIFIVE_U_DEV_DRAM].base,
                                    machine->ram_size, s->fdt);
-    #if defined(TARGET_RISCV64)
-    start_addr_hi32 = start_addr >> 32;
-    #endif
+    if (!riscv_is_32_bit(machine)) {
+        start_addr_hi32 = start_addr >> 32;
+    }
 
     /* reset vector */
     uint32_t reset_vec[11] = {
@@ -516,13 +518,8 @@  static void sifive_u_machine_init(MachineState *machine)
         0x00000297,                    /* 1:  auipc  t0, %pcrel_hi(fw_dyn) */
         0x02828613,                    /*     addi   a2, t0, %pcrel_lo(1b) */
         0xf1402573,                    /*     csrr   a0, mhartid  */
-#if defined(TARGET_RISCV32)
-        0x0202a583,                    /*     lw     a1, 32(t0) */
-        0x0182a283,                    /*     lw     t0, 24(t0) */
-#elif defined(TARGET_RISCV64)
-        0x0202b583,                    /*     ld     a1, 32(t0) */
-        0x0182b283,                    /*     ld     t0, 24(t0) */
-#endif
+        0,
+        0,
         0x00028067,                    /*     jr     t0 */
         start_addr,                    /* start: .dword */
         start_addr_hi32,
@@ -530,6 +527,14 @@  static void sifive_u_machine_init(MachineState *machine)
         0x00000000,
                                        /* fw_dyn: */
     };
+    if (riscv_is_32_bit(machine)) {
+        reset_vec[4] = 0x0202a583;     /*     lw     a1, 32(t0) */
+        reset_vec[5] = 0x0182a283;     /*     lw     t0, 24(t0) */
+    } else {
+        reset_vec[4] = 0x0202b583;     /*     ld     a1, 32(t0) */
+        reset_vec[5] = 0x0182b283;     /*     ld     t0, 24(t0) */
+    }
+
 
     /* copy in the reset vector in little_endian byte order */
     for (i = 0; i < ARRAY_SIZE(reset_vec); i++) {