[PATCH v1 00/16] RISC-V: Start to remove xlen preprocess

[PATCH v1 00/16] RISC-V: Start to remove xlen preprocess

[Alistair Francis]

The RISC-V QEMU port currently has lot of preprocessor directives that
check if we are targetting a 32-bit or 64-bit CPU. This means that the
64-bit RISC-V target can not run 32-bit CPUs. This is different to most
other QEMU architectures and doesn't allow us to mix xlens (such as when
running Hypervisors with different xlen guests).

This series is a step toward removing some of those to allow us to use
32-bit CPUs on 64-bit RISC-V targets.

Alistair Francis (16):
  target/riscv: Add a TYPE_RISCV_CPU_BASE CPU
  riscv: spike: Remove target macro conditionals
  riscv: virt: Remove target macro conditionals
  hw/riscv: boot: Remove compile time XLEN checks
  hw/riscv: virt: Remove compile time XLEN checks
  hw/riscv: spike: Remove compile time XLEN checks
  hw/riscv: sifive_u: Remove compile time XLEN checks
  target/riscv: fpu_helper: Match function defs in HELPER macros
  target/riscv: Add a riscv_cpu_is_32bit() helper function
  target/riscv: Specify the XLEN for CPUs
  target/riscv: cpu: Remove compile time XLEN checks
  target/riscv: cpu_helper: Remove compile time XLEN checks
  target/riscv: csr: Remove compile time XLEN checks
  target/riscv: cpu: Set XLEN independently from target
  target/riscv: Convert the get/set_field() to support 64-bit values
  target/riscv: Consolidate *statush registers

 include/hw/riscv/boot.h   |   8 +-
 include/hw/riscv/spike.h  |   6 --
 include/hw/riscv/virt.h   |   6 --
 target/riscv/cpu.h        |  36 ++++---
 target/riscv/cpu_bits.h   |  24 ++---
 hw/riscv/boot.c           |  55 ++++++-----
 hw/riscv/sifive_u.c       |  59 ++++++------
 hw/riscv/spike.c          |  50 +++++-----
 hw/riscv/virt.c           |  36 +++----
 target/riscv/cpu.c        |  83 ++++++++++------
 target/riscv/cpu_helper.c |  47 +++------
 target/riscv/csr.c        | 197 ++++++++++++++++++++------------------
 target/riscv/fpu_helper.c |  16 ++--
 target/riscv/op_helper.c  |  11 +--
 14 files changed, 328 insertions(+), 306 deletions(-)

-- 
2.28.0

[PATCH v1 01/16] target/riscv: Add a TYPE_RISCV_CPU_BASE CPU

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index de4705bb57..74a236d4bc 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -44,6 +44,12 @@
 #define TYPE_RISCV_CPU_SIFIVE_U34       RISCV_CPU_TYPE_NAME("sifive-u34")
 #define TYPE_RISCV_CPU_SIFIVE_U54       RISCV_CPU_TYPE_NAME("sifive-u54")
 
+#if defined(TARGET_RISCV32)
+# define TYPE_RISCV_CPU_BASE            TYPE_RISCV_CPU_BASE32
+#elif defined(TARGET_RISCV64)
+# define TYPE_RISCV_CPU_BASE            TYPE_RISCV_CPU_BASE64
+#endif
+
 #define RV32 ((target_ulong)1 << (TARGET_LONG_BITS - 2))
 #define RV64 ((target_ulong)2 << (TARGET_LONG_BITS - 2))
 
-- 
2.28.0

Re: [PATCH v1 01/16] target/riscv: Add a TYPE_RISCV_CPU_BASE CPU

[Bin Meng]

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>
> ---
>  target/riscv/cpu.h | 6 ++++++
>  1 file changed, 6 insertions(+)
>

Reviewed-by: Bin Meng <bin.meng@windriver.com>
Tested-by: Bin Meng <bin.meng@windriver.com>

[PATCH v1 02/16] riscv: spike: Remove target macro conditionals

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 include/hw/riscv/spike.h | 6 ------
 hw/riscv/spike.c         | 2 +-
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/include/hw/riscv/spike.h b/include/hw/riscv/spike.h
index cddeca2e77..cdd1a13011 100644
--- a/include/hw/riscv/spike.h
+++ b/include/hw/riscv/spike.h
@@ -47,10 +47,4 @@ enum {
     SPIKE_DRAM
 };
 
-#if defined(TARGET_RISCV32)
-#define SPIKE_V1_10_0_CPU TYPE_RISCV_CPU_BASE32
-#elif defined(TARGET_RISCV64)
-#define SPIKE_V1_10_0_CPU TYPE_RISCV_CPU_BASE64
-#endif
-
 #endif
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index facac6e7d2..29f07f47b1 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -317,7 +317,7 @@ static void spike_machine_class_init(ObjectClass *oc, void *data)
     mc->init = spike_board_init;
     mc->max_cpus = SPIKE_CPUS_MAX;
     mc->is_default = true;
-    mc->default_cpu_type = SPIKE_V1_10_0_CPU;
+    mc->default_cpu_type = TYPE_RISCV_CPU_BASE;
     mc->possible_cpu_arch_ids = riscv_numa_possible_cpu_arch_ids;
     mc->cpu_index_to_instance_props = riscv_numa_cpu_index_to_props;
     mc->get_default_cpu_node_id = riscv_numa_get_default_cpu_node_id;
-- 
2.28.0

Re: [PATCH v1 02/16] riscv: spike: Remove target macro conditionals

[Bin Meng]

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>
> ---
>  include/hw/riscv/spike.h | 6 ------
>  hw/riscv/spike.c         | 2 +-
>  2 files changed, 1 insertion(+), 7 deletions(-)
>

Reviewed-by: Bin Meng <bin.meng@windriver.com>

[PATCH v1 03/16] riscv: virt: Remove target macro conditionals

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 include/hw/riscv/virt.h | 6 ------
 hw/riscv/virt.c         | 2 +-
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
index b4ed9a32eb..84b7a3848f 100644
--- a/include/hw/riscv/virt.h
+++ b/include/hw/riscv/virt.h
@@ -89,10 +89,4 @@ enum {
 #define FDT_INT_MAP_WIDTH     (FDT_PCI_ADDR_CELLS + FDT_PCI_INT_CELLS + 1 + \
                                FDT_PLIC_ADDR_CELLS + FDT_PLIC_INT_CELLS)
 
-#if defined(TARGET_RISCV32)
-#define VIRT_CPU TYPE_RISCV_CPU_BASE32
-#elif defined(TARGET_RISCV64)
-#define VIRT_CPU TYPE_RISCV_CPU_BASE64
-#endif
-
 #endif
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 6bfd10dfc7..5f1fcebdc6 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -693,7 +693,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
     mc->desc = "RISC-V VirtIO board";
     mc->init = virt_machine_init;
     mc->max_cpus = VIRT_CPUS_MAX;
-    mc->default_cpu_type = VIRT_CPU;
+    mc->default_cpu_type = TYPE_RISCV_CPU_BASE;
     mc->pci_allow_0_address = true;
     mc->possible_cpu_arch_ids = riscv_numa_possible_cpu_arch_ids;
     mc->cpu_index_to_instance_props = riscv_numa_cpu_index_to_props;
-- 
2.28.0

Re: [PATCH v1 03/16] riscv: virt: Remove target macro conditionals

[Bin Meng]

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>
> ---
>  include/hw/riscv/virt.h | 6 ------
>  hw/riscv/virt.c         | 2 +-
>  2 files changed, 1 insertion(+), 7 deletions(-)
>

Reviewed-by: Bin Meng <bin.meng@windriver.com>
Tested-by: Bin Meng <bin.meng@windriver.com>

[PATCH v1 04/16] hw/riscv: boot: Remove compile time XLEN checks

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 include/hw/riscv/boot.h |  8 +++---
 hw/riscv/boot.c         | 55 ++++++++++++++++++++++-------------------
 hw/riscv/sifive_u.c     |  2 +-
 hw/riscv/spike.c        |  3 ++-
 hw/riscv/virt.c         |  2 +-
 5 files changed, 39 insertions(+), 31 deletions(-)

diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
index 0b01988727..b6d37a91d6 100644
--- a/include/hw/riscv/boot.h
+++ b/include/hw/riscv/boot.h
@@ -41,10 +41,12 @@ target_ulong riscv_load_kernel(const char *kernel_filename,
 hwaddr riscv_load_initrd(const char *filename, uint64_t mem_size,
                          uint64_t kernel_entry, hwaddr *start);
 uint32_t riscv_load_fdt(hwaddr dram_start, uint64_t dram_size, void *fdt);
-void riscv_setup_rom_reset_vec(hwaddr saddr, hwaddr rom_base,
-                               hwaddr rom_size, uint64_t kernel_entry,
+void riscv_setup_rom_reset_vec(MachineState *machine, hwaddr saddr,
+                               hwaddr rom_base, hwaddr rom_size,
+                               uint64_t kernel_entry,
                                uint32_t fdt_load_addr, void *fdt);
-void riscv_rom_copy_firmware_info(hwaddr rom_base, hwaddr rom_size,
+void riscv_rom_copy_firmware_info(MachineState *machine, hwaddr rom_base,
+                                  hwaddr rom_size,
                                   uint32_t reset_vec_size,
                                   uint64_t kernel_entry);
 
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index 9b3fe3fb1e..a6e4e52b32 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -32,12 +32,6 @@
 
 #include <libfdt.h>
 
-#if defined(TARGET_RISCV32)
-#define fw_dynamic_info_data(__val)     cpu_to_le32(__val)
-#else
-#define fw_dynamic_info_data(__val)     cpu_to_le64(__val)
-#endif
-
 bool riscv_is_32_bit(MachineState *machine)
 {
     if (!strncmp(machine->cpu_type, "rv32", 4)) {
@@ -216,16 +210,24 @@ uint32_t riscv_load_fdt(hwaddr dram_base, uint64_t mem_size, void *fdt)
     return fdt_addr;
 }
 
-void riscv_rom_copy_firmware_info(hwaddr rom_base, hwaddr rom_size,
-                              uint32_t reset_vec_size, uint64_t kernel_entry)
+void riscv_rom_copy_firmware_info(MachineState *machine, hwaddr rom_base,
+                                  hwaddr rom_size, uint32_t reset_vec_size,
+                                  uint64_t kernel_entry)
 {
     struct fw_dynamic_info dinfo;
     size_t dinfo_len;
 
-    dinfo.magic = fw_dynamic_info_data(FW_DYNAMIC_INFO_MAGIC_VALUE);
-    dinfo.version = fw_dynamic_info_data(FW_DYNAMIC_INFO_VERSION);
-    dinfo.next_mode = fw_dynamic_info_data(FW_DYNAMIC_INFO_NEXT_MODE_S);
-    dinfo.next_addr = fw_dynamic_info_data(kernel_entry);
+    if (riscv_is_32_bit(machine)) {
+        dinfo.magic = cpu_to_le32(FW_DYNAMIC_INFO_MAGIC_VALUE);
+        dinfo.version = cpu_to_le32(FW_DYNAMIC_INFO_VERSION);
+        dinfo.next_mode = cpu_to_le32(FW_DYNAMIC_INFO_NEXT_MODE_S);
+        dinfo.next_addr = cpu_to_le32(kernel_entry);
+    } else {
+        dinfo.magic = cpu_to_le64(FW_DYNAMIC_INFO_MAGIC_VALUE);
+        dinfo.version = cpu_to_le64(FW_DYNAMIC_INFO_VERSION);
+        dinfo.next_mode = cpu_to_le64(FW_DYNAMIC_INFO_NEXT_MODE_S);
+        dinfo.next_addr = cpu_to_le64(kernel_entry);
+    }
     dinfo.options = 0;
     dinfo.boot_hart = 0;
     dinfo_len = sizeof(dinfo);
@@ -245,28 +247,24 @@ void riscv_rom_copy_firmware_info(hwaddr rom_base, hwaddr rom_size,
                            &address_space_memory);
 }
 
-void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr rom_base,
-                               hwaddr rom_size, uint64_t kernel_entry,
+void riscv_setup_rom_reset_vec(MachineState *machine, hwaddr start_addr,
+                               hwaddr rom_base, hwaddr rom_size,
+                               uint64_t kernel_entry,
                                uint32_t fdt_load_addr, void *fdt)
 {
     int i;
     uint32_t start_addr_hi32 = 0x00000000;
 
-    #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[10] = {
         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,
@@ -274,6 +272,13 @@ void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr rom_base,
         0x00000000,
                                      /* fw_dyn: */
     };
+    if (riscv_is_32_bit(machine)) {
+        reset_vec[3] = 0x0202a583;   /*     lw     a1, 32(t0) */
+        reset_vec[4] = 0x0182a283;   /*     lw     t0, 24(t0) */
+    } else {
+        reset_vec[3] = 0x0202b583;   /*     ld     a1, 32(t0) */
+        reset_vec[4] = 0x0182b283;   /*     ld     t0, 24(t0) */
+    }
 
     /* copy in the reset vector in little_endian byte order */
     for (i = 0; i < ARRAY_SIZE(reset_vec); i++) {
@@ -281,7 +286,7 @@ void riscv_setup_rom_reset_vec(hwaddr start_addr, hwaddr rom_base,
     }
     rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
                           rom_base, &address_space_memory);
-    riscv_rom_copy_firmware_info(rom_base, rom_size, sizeof(reset_vec),
+    riscv_rom_copy_firmware_info(machine, rom_base, rom_size, sizeof(reset_vec),
                                  kernel_entry);
 
     return;
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index b2472c6627..978cfedd24 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -538,7 +538,7 @@ static void sifive_u_machine_init(MachineState *machine)
     rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
                           memmap[SIFIVE_U_DEV_MROM].base, &address_space_memory);
 
-    riscv_rom_copy_firmware_info(memmap[SIFIVE_U_DEV_MROM].base,
+    riscv_rom_copy_firmware_info(machine, memmap[SIFIVE_U_DEV_MROM].base,
                                  memmap[SIFIVE_U_DEV_MROM].size,
                                  sizeof(reset_vec), kernel_entry);
 }
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 29f07f47b1..875f371f0f 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -296,7 +296,8 @@ static void spike_board_init(MachineState *machine)
     fdt_load_addr = riscv_load_fdt(memmap[SPIKE_DRAM].base,
                                    machine->ram_size, s->fdt);
     /* load the reset vector */
-    riscv_setup_rom_reset_vec(memmap[SPIKE_DRAM].base, memmap[SPIKE_MROM].base,
+    riscv_setup_rom_reset_vec(machine, memmap[SPIKE_DRAM].base,
+                              memmap[SPIKE_MROM].base,
                               memmap[SPIKE_MROM].size, kernel_entry,
                               fdt_load_addr, s->fdt);
 
diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 5f1fcebdc6..192138cc76 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -643,7 +643,7 @@ static void virt_machine_init(MachineState *machine)
     fdt_load_addr = riscv_load_fdt(memmap[VIRT_DRAM].base,
                                    machine->ram_size, s->fdt);
     /* load the reset vector */
-    riscv_setup_rom_reset_vec(start_addr, virt_memmap[VIRT_MROM].base,
+    riscv_setup_rom_reset_vec(machine, start_addr, virt_memmap[VIRT_MROM].base,
                               virt_memmap[VIRT_MROM].size, kernel_entry,
                               fdt_load_addr, s->fdt);
 
-- 
2.28.0

Re: [PATCH v1 04/16] hw/riscv: boot: Remove compile time XLEN checks

[Bin Meng]

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>
> ---
>  include/hw/riscv/boot.h |  8 +++---
>  hw/riscv/boot.c         | 55 ++++++++++++++++++++++-------------------
>  hw/riscv/sifive_u.c     |  2 +-
>  hw/riscv/spike.c        |  3 ++-
>  hw/riscv/virt.c         |  2 +-
>  5 files changed, 39 insertions(+), 31 deletions(-)
>

Reviewed-by: Bin Meng <bin.meng@windriver.com>
Tested-by: Bin Meng <bin.meng@windriver.com>

[PATCH v1 05/16] hw/riscv: virt: Remove compile time XLEN checks

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 hw/riscv/virt.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
index 192138cc76..e76c0a32b9 100644
--- a/hw/riscv/virt.c
+++ b/hw/riscv/virt.c
@@ -43,12 +43,6 @@
 #include "hw/pci/pci.h"
 #include "hw/pci-host/gpex.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;
@@ -177,7 +171,7 @@ static void create_pcie_irq_map(void *fdt, char *nodename,
 }
 
 static void create_fdt(RISCVVirtState *s, const struct MemmapEntry *memmap,
-    uint64_t mem_size, const char *cmdline)
+                       uint64_t mem_size, const char *cmdline, bool is_32_bit)
 {
     void *fdt;
     int i, cpu, socket;
@@ -231,11 +225,11 @@ static void create_fdt(RISCVVirtState *s, const struct MemmapEntry *memmap,
             cpu_name = g_strdup_printf("/cpus/cpu@%d",
                 s->soc[socket].hartid_base + cpu);
             qemu_fdt_add_subnode(fdt, cpu_name);
-#if defined(TARGET_RISCV32)
-            qemu_fdt_setprop_string(fdt, cpu_name, "mmu-type", "riscv,sv32");
-#else
-            qemu_fdt_setprop_string(fdt, cpu_name, "mmu-type", "riscv,sv48");
-#endif
+            if (is_32_bit) {
+                qemu_fdt_setprop_string(fdt, cpu_name, "mmu-type", "riscv,sv32");
+            } else {
+                qemu_fdt_setprop_string(fdt, cpu_name, "mmu-type", "riscv,sv48");
+            }
             name = riscv_isa_string(&s->soc[socket].harts[cpu]);
             qemu_fdt_setprop_string(fdt, cpu_name, "riscv,isa", name);
             g_free(name);
@@ -595,7 +589,8 @@ static void virt_machine_init(MachineState *machine)
         main_mem);
 
     /* 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));
 
     /* boot rom */
     memory_region_init_rom(mask_rom, NULL, "riscv_virt_board.mrom",
@@ -603,8 +598,15 @@ static void virt_machine_init(MachineState *machine)
     memory_region_add_subregion(system_memory, memmap[VIRT_MROM].base,
                                 mask_rom);
 
-    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,
-- 
2.28.0

Re: [PATCH v1 05/16] hw/riscv: virt: Remove compile time XLEN checks

[Bin Meng]

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/virt.c | 32 +++++++++++++++++---------------
>  1 file changed, 17 insertions(+), 15 deletions(-)
>

Reviewed-by: Bin Meng <bin.meng@windriver.com>
Tested-by: Bin Meng <bin.meng@windriver.com>

[PATCH v1 06/16] hw/riscv: spike: Remove compile time XLEN checks

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 hw/riscv/spike.c | 45 ++++++++++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 21 deletions(-)

diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 875f371f0f..3e47e4579d 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -43,17 +43,6 @@
 #include "sysemu/qtest.h"
 #include "sysemu/sysemu.h"
 
-/*
- * Not like other RISC-V machines that use plain binary bios images,
- * keeping ELF files here was intentional because BIN files don't work
- * for the Spike machine as HTIF emulation depends on ELF parsing.
- */
-#if defined(TARGET_RISCV32)
-# define BIOS_FILENAME "opensbi-riscv32-generic-fw_dynamic.elf"
-#else
-# define BIOS_FILENAME "opensbi-riscv64-generic-fw_dynamic.elf"
-#endif
-
 static const struct MemmapEntry {
     hwaddr base;
     hwaddr size;
@@ -64,7 +53,7 @@ static const struct MemmapEntry {
 };
 
 static void create_fdt(SpikeState *s, const struct MemmapEntry *memmap,
-    uint64_t mem_size, const char *cmdline)
+                       uint64_t mem_size, const char *cmdline, bool is_32_bit)
 {
     void *fdt;
     uint64_t addr, size;
@@ -115,11 +104,11 @@ static void create_fdt(SpikeState *s, const struct MemmapEntry *memmap,
             cpu_name = g_strdup_printf("/cpus/cpu@%d",
                 s->soc[socket].hartid_base + cpu);
             qemu_fdt_add_subnode(fdt, cpu_name);
-#if defined(TARGET_RISCV32)
-            qemu_fdt_setprop_string(fdt, cpu_name, "mmu-type", "riscv,sv32");
-#else
-            qemu_fdt_setprop_string(fdt, cpu_name, "mmu-type", "riscv,sv48");
-#endif
+            if (is_32_bit) {
+                qemu_fdt_setprop_string(fdt, cpu_name, "mmu-type", "riscv,sv32");
+            } else {
+                qemu_fdt_setprop_string(fdt, cpu_name, "mmu-type", "riscv,sv48");
+            }
             name = riscv_isa_string(&s->soc[socket].harts[cpu]);
             qemu_fdt_setprop_string(fdt, cpu_name, "riscv,isa", name);
             g_free(name);
@@ -254,7 +243,8 @@ static void spike_board_init(MachineState *machine)
         main_mem);
 
     /* 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));
 
     /* boot rom */
     memory_region_init_rom(mask_rom, NULL, "riscv.spike.mrom",
@@ -262,9 +252,22 @@ static void spike_board_init(MachineState *machine)
     memory_region_add_subregion(system_memory, memmap[SPIKE_MROM].base,
                                 mask_rom);
 
-    firmware_end_addr = riscv_find_and_load_firmware(machine, BIOS_FILENAME,
-                                                     memmap[SPIKE_DRAM].base,
-                                                     htif_symbol_callback);
+    /*
+     * Not like other RISC-V machines that use plain binary bios images,
+     * keeping ELF files here was intentional because BIN files don't work
+     * for the Spike machine as HTIF emulation depends on ELF parsing.
+     */
+    if (riscv_is_32_bit(machine)) {
+        firmware_end_addr = riscv_find_and_load_firmware(machine,
+                                    "opensbi-riscv32-generic-fw_dynamic.elf",
+                                    memmap[SPIKE_DRAM].base,
+                                    htif_symbol_callback);
+    } else {
+        firmware_end_addr = riscv_find_and_load_firmware(machine,
+                                    "opensbi-riscv64-generic-fw_dynamic.elf",
+                                    memmap[SPIKE_DRAM].base,
+                                    htif_symbol_callback);
+    }
 
     if (machine->kernel_filename) {
         kernel_start_addr = riscv_calc_kernel_start_addr(machine,
-- 
2.28.0

Re: [PATCH v1 06/16] hw/riscv: spike: Remove compile time XLEN checks

[Bin Meng]

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/spike.c | 45 ++++++++++++++++++++++++---------------------
>  1 file changed, 24 insertions(+), 21 deletions(-)
>

Reviewed-by: Bin Meng <bin.meng@windriver.com>

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

[Alistair Francis]

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;
     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++) {
-- 
2.28.0

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

[Bin Meng]

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

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

[Alistair Francis]

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

[PATCH v1 08/16] target/riscv: fpu_helper: Match function defs in HELPER macros

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/fpu_helper.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c
index bb346a8249..507d7fe7fa 100644
--- a/target/riscv/fpu_helper.c
+++ b/target/riscv/fpu_helper.c
@@ -224,13 +224,13 @@ target_ulong helper_fcvt_wu_s(CPURISCVState *env, uint64_t rs1)
 }
 
 #if defined(TARGET_RISCV64)
-uint64_t helper_fcvt_l_s(CPURISCVState *env, uint64_t rs1)
+target_ulong helper_fcvt_l_s(CPURISCVState *env, uint64_t rs1)
 {
     float32 frs1 = check_nanbox_s(rs1);
     return float32_to_int64(frs1, &env->fp_status);
 }
 
-uint64_t helper_fcvt_lu_s(CPURISCVState *env, uint64_t rs1)
+target_ulong helper_fcvt_lu_s(CPURISCVState *env, uint64_t rs1)
 {
     float32 frs1 = check_nanbox_s(rs1);
     return float32_to_uint64(frs1, &env->fp_status);
@@ -248,12 +248,12 @@ uint64_t helper_fcvt_s_wu(CPURISCVState *env, target_ulong rs1)
 }
 
 #if defined(TARGET_RISCV64)
-uint64_t helper_fcvt_s_l(CPURISCVState *env, uint64_t rs1)
+uint64_t helper_fcvt_s_l(CPURISCVState *env, target_ulong rs1)
 {
     return nanbox_s(int64_to_float32(rs1, &env->fp_status));
 }
 
-uint64_t helper_fcvt_s_lu(CPURISCVState *env, uint64_t rs1)
+uint64_t helper_fcvt_s_lu(CPURISCVState *env, target_ulong rs1)
 {
     return nanbox_s(uint64_to_float32(rs1, &env->fp_status));
 }
@@ -337,12 +337,12 @@ target_ulong helper_fcvt_wu_d(CPURISCVState *env, uint64_t frs1)
 }
 
 #if defined(TARGET_RISCV64)
-uint64_t helper_fcvt_l_d(CPURISCVState *env, uint64_t frs1)
+target_ulong helper_fcvt_l_d(CPURISCVState *env, uint64_t frs1)
 {
     return float64_to_int64(frs1, &env->fp_status);
 }
 
-uint64_t helper_fcvt_lu_d(CPURISCVState *env, uint64_t frs1)
+target_ulong helper_fcvt_lu_d(CPURISCVState *env, uint64_t frs1)
 {
     return float64_to_uint64(frs1, &env->fp_status);
 }
@@ -359,12 +359,12 @@ uint64_t helper_fcvt_d_wu(CPURISCVState *env, target_ulong rs1)
 }
 
 #if defined(TARGET_RISCV64)
-uint64_t helper_fcvt_d_l(CPURISCVState *env, uint64_t rs1)
+uint64_t helper_fcvt_d_l(CPURISCVState *env, target_ulong rs1)
 {
     return int64_to_float64(rs1, &env->fp_status);
 }
 
-uint64_t helper_fcvt_d_lu(CPURISCVState *env, uint64_t rs1)
+uint64_t helper_fcvt_d_lu(CPURISCVState *env, target_ulong rs1)
 {
     return uint64_to_float64(rs1, &env->fp_status);
 }
-- 
2.28.0

Re: [PATCH v1 08/16] target/riscv: fpu_helper: Match function defs in HELPER macros

[Bin Meng]

On Fri, Oct 23, 2020 at 11:44 PM Alistair Francis
<alistair.francis@wdc.com> wrote:
>

Could you please put some details as to why changing uint64_t to
target_ulong (and vice versa) is needed?

> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/fpu_helper.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c
> index bb346a8249..507d7fe7fa 100644
> --- a/target/riscv/fpu_helper.c
> +++ b/target/riscv/fpu_helper.c
> @@ -224,13 +224,13 @@ target_ulong helper_fcvt_wu_s(CPURISCVState *env, uint64_t rs1)
>  }
>
>  #if defined(TARGET_RISCV64)
> -uint64_t helper_fcvt_l_s(CPURISCVState *env, uint64_t rs1)
> +target_ulong helper_fcvt_l_s(CPURISCVState *env, uint64_t rs1)
>  {
>      float32 frs1 = check_nanbox_s(rs1);
>      return float32_to_int64(frs1, &env->fp_status);
>  }
>
> -uint64_t helper_fcvt_lu_s(CPURISCVState *env, uint64_t rs1)
> +target_ulong helper_fcvt_lu_s(CPURISCVState *env, uint64_t rs1)
>  {
>      float32 frs1 = check_nanbox_s(rs1);
>      return float32_to_uint64(frs1, &env->fp_status);
> @@ -248,12 +248,12 @@ uint64_t helper_fcvt_s_wu(CPURISCVState *env, target_ulong rs1)
>  }
>
>  #if defined(TARGET_RISCV64)
> -uint64_t helper_fcvt_s_l(CPURISCVState *env, uint64_t rs1)
> +uint64_t helper_fcvt_s_l(CPURISCVState *env, target_ulong rs1)
>  {
>      return nanbox_s(int64_to_float32(rs1, &env->fp_status));
>  }
>
> -uint64_t helper_fcvt_s_lu(CPURISCVState *env, uint64_t rs1)
> +uint64_t helper_fcvt_s_lu(CPURISCVState *env, target_ulong rs1)
>  {
>      return nanbox_s(uint64_to_float32(rs1, &env->fp_status));
>  }
> @@ -337,12 +337,12 @@ target_ulong helper_fcvt_wu_d(CPURISCVState *env, uint64_t frs1)
>  }
>
>  #if defined(TARGET_RISCV64)
> -uint64_t helper_fcvt_l_d(CPURISCVState *env, uint64_t frs1)
> +target_ulong helper_fcvt_l_d(CPURISCVState *env, uint64_t frs1)
>  {
>      return float64_to_int64(frs1, &env->fp_status);
>  }
>
> -uint64_t helper_fcvt_lu_d(CPURISCVState *env, uint64_t frs1)
> +target_ulong helper_fcvt_lu_d(CPURISCVState *env, uint64_t frs1)
>  {
>      return float64_to_uint64(frs1, &env->fp_status);
>  }
> @@ -359,12 +359,12 @@ uint64_t helper_fcvt_d_wu(CPURISCVState *env, target_ulong rs1)
>  }
>
>  #if defined(TARGET_RISCV64)
> -uint64_t helper_fcvt_d_l(CPURISCVState *env, uint64_t rs1)
> +uint64_t helper_fcvt_d_l(CPURISCVState *env, target_ulong rs1)
>  {
>      return int64_to_float64(rs1, &env->fp_status);
>  }
>
> -uint64_t helper_fcvt_d_lu(CPURISCVState *env, uint64_t rs1)
> +uint64_t helper_fcvt_d_lu(CPURISCVState *env, target_ulong rs1)
>  {
>      return uint64_to_float64(rs1, &env->fp_status);
>  }
> --

Regards,
Bin

Re: [PATCH v1 08/16] target/riscv: fpu_helper: Match function defs in HELPER macros

[Alistair Francis]

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:
> >
>
> Could you please put some details as to why changing uint64_t to
> target_ulong (and vice versa) is needed?

Done!

Alistair

>
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  target/riscv/fpu_helper.c | 16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c
> > index bb346a8249..507d7fe7fa 100644
> > --- a/target/riscv/fpu_helper.c
> > +++ b/target/riscv/fpu_helper.c
> > @@ -224,13 +224,13 @@ target_ulong helper_fcvt_wu_s(CPURISCVState *env, uint64_t rs1)
> >  }
> >
> >  #if defined(TARGET_RISCV64)
> > -uint64_t helper_fcvt_l_s(CPURISCVState *env, uint64_t rs1)
> > +target_ulong helper_fcvt_l_s(CPURISCVState *env, uint64_t rs1)
> >  {
> >      float32 frs1 = check_nanbox_s(rs1);
> >      return float32_to_int64(frs1, &env->fp_status);
> >  }
> >
> > -uint64_t helper_fcvt_lu_s(CPURISCVState *env, uint64_t rs1)
> > +target_ulong helper_fcvt_lu_s(CPURISCVState *env, uint64_t rs1)
> >  {
> >      float32 frs1 = check_nanbox_s(rs1);
> >      return float32_to_uint64(frs1, &env->fp_status);
> > @@ -248,12 +248,12 @@ uint64_t helper_fcvt_s_wu(CPURISCVState *env, target_ulong rs1)
> >  }
> >
> >  #if defined(TARGET_RISCV64)
> > -uint64_t helper_fcvt_s_l(CPURISCVState *env, uint64_t rs1)
> > +uint64_t helper_fcvt_s_l(CPURISCVState *env, target_ulong rs1)
> >  {
> >      return nanbox_s(int64_to_float32(rs1, &env->fp_status));
> >  }
> >
> > -uint64_t helper_fcvt_s_lu(CPURISCVState *env, uint64_t rs1)
> > +uint64_t helper_fcvt_s_lu(CPURISCVState *env, target_ulong rs1)
> >  {
> >      return nanbox_s(uint64_to_float32(rs1, &env->fp_status));
> >  }
> > @@ -337,12 +337,12 @@ target_ulong helper_fcvt_wu_d(CPURISCVState *env, uint64_t frs1)
> >  }
> >
> >  #if defined(TARGET_RISCV64)
> > -uint64_t helper_fcvt_l_d(CPURISCVState *env, uint64_t frs1)
> > +target_ulong helper_fcvt_l_d(CPURISCVState *env, uint64_t frs1)
> >  {
> >      return float64_to_int64(frs1, &env->fp_status);
> >  }
> >
> > -uint64_t helper_fcvt_lu_d(CPURISCVState *env, uint64_t frs1)
> > +target_ulong helper_fcvt_lu_d(CPURISCVState *env, uint64_t frs1)
> >  {
> >      return float64_to_uint64(frs1, &env->fp_status);
> >  }
> > @@ -359,12 +359,12 @@ uint64_t helper_fcvt_d_wu(CPURISCVState *env, target_ulong rs1)
> >  }
> >
> >  #if defined(TARGET_RISCV64)
> > -uint64_t helper_fcvt_d_l(CPURISCVState *env, uint64_t rs1)
> > +uint64_t helper_fcvt_d_l(CPURISCVState *env, target_ulong rs1)
> >  {
> >      return int64_to_float64(rs1, &env->fp_status);
> >  }
> >
> > -uint64_t helper_fcvt_d_lu(CPURISCVState *env, uint64_t rs1)
> > +uint64_t helper_fcvt_d_lu(CPURISCVState *env, target_ulong rs1)
> >  {
> >      return uint64_to_float64(rs1, &env->fp_status);
> >  }
> > --
>
> Regards,
> Bin

[PATCH v1 09/16] target/riscv: Add a riscv_cpu_is_32bit() helper function

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu.h | 2 ++
 target/riscv/cpu.c | 9 +++++++++
 2 files changed, 11 insertions(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 74a236d4bc..86b063543d 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -383,6 +383,8 @@ FIELD(TB_FLAGS, LMUL, 3, 2)
 FIELD(TB_FLAGS, SEW, 5, 3)
 FIELD(TB_FLAGS, VILL, 8, 1)
 
+bool riscv_cpu_is_32bit(CPURISCVState *env);
+
 /*
  * A simplification for VLMAX
  * = (1 << LMUL) * VLEN / (8 * (1 << SEW))
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 0bbfd7f457..32234d4a70 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -107,6 +107,15 @@ const char *riscv_cpu_get_trap_name(target_ulong cause, bool async)
     }
 }
 
+bool riscv_cpu_is_32bit(CPURISCVState *env)
+{
+    if (env->misa & RV64) {
+        return false;
+    }
+
+    return true;
+}
+
 static void set_misa(CPURISCVState *env, target_ulong misa)
 {
     env->misa_mask = env->misa = misa;
-- 
2.28.0

Re: [PATCH v1 09/16] target/riscv: Add a riscv_cpu_is_32bit() helper function

[Bin Meng]

On Fri, Oct 23, 2020 at 11:45 PM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/cpu.h | 2 ++
>  target/riscv/cpu.c | 9 +++++++++
>  2 files changed, 11 insertions(+)
>

Reviewed-by: Bin Meng <bin.meng@windriver.com>
Tested-by: Bin Meng <bin.meng@windriver.com>

[PATCH v1 10/16] target/riscv: Specify the XLEN for CPUs

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu.c | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 32234d4a70..8f8c568974 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -157,22 +157,36 @@ static void riscv_base_cpu_init(Object *obj)
     set_misa(env, 0);
 }
 
-static void rvxx_sifive_u_cpu_init(Object *obj)
+#ifdef TARGET_RISCV64
+static void rv64_sifive_u_cpu_init(Object *obj)
 {
     CPURISCVState *env = &RISCV_CPU(obj)->env;
-    set_misa(env, RVXLEN | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
+    set_misa(env, RV64 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
     set_priv_version(env, PRIV_VERSION_1_10_0);
 }
 
-static void rvxx_sifive_e_cpu_init(Object *obj)
+static void rv64_sifive_e_cpu_init(Object *obj)
 {
     CPURISCVState *env = &RISCV_CPU(obj)->env;
-    set_misa(env, RVXLEN | RVI | RVM | RVA | RVC | RVU);
+    set_misa(env, RV64 | RVI | RVM | RVA | RVC | RVU);
     set_priv_version(env, PRIV_VERSION_1_10_0);
     qdev_prop_set_bit(DEVICE(obj), "mmu", false);
 }
+#else
+static void rv32_sifive_u_cpu_init(Object *obj)
+{
+    CPURISCVState *env = &RISCV_CPU(obj)->env;
+    set_misa(env, RV32 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
+    set_priv_version(env, PRIV_VERSION_1_10_0);
+}
 
-#if defined(TARGET_RISCV32)
+static void rv32_sifive_e_cpu_init(Object *obj)
+{
+    CPURISCVState *env = &RISCV_CPU(obj)->env;
+    set_misa(env, RV32 | RVI | RVM | RVA | RVC | RVU);
+    set_priv_version(env, PRIV_VERSION_1_10_0);
+    qdev_prop_set_bit(DEVICE(obj), "mmu", false);
+}
 
 static void rv32_ibex_cpu_init(Object *obj)
 {
@@ -190,7 +204,6 @@ static void rv32_imafcu_nommu_cpu_init(Object *obj)
     set_resetvec(env, DEFAULT_RSTVEC);
     qdev_prop_set_bit(DEVICE(obj), "mmu", false);
 }
-
 #endif
 
 static ObjectClass *riscv_cpu_class_by_name(const char *cpu_model)
@@ -647,13 +660,13 @@ static const TypeInfo riscv_cpu_type_infos[] = {
 #if defined(TARGET_RISCV32)
     DEFINE_CPU(TYPE_RISCV_CPU_BASE32,           riscv_base_cpu_init),
     DEFINE_CPU(TYPE_RISCV_CPU_IBEX,             rv32_ibex_cpu_init),
-    DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E31,       rvxx_sifive_e_cpu_init),
+    DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E31,       rv32_sifive_e_cpu_init),
     DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E34,       rv32_imafcu_nommu_cpu_init),
-    DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U34,       rvxx_sifive_u_cpu_init),
+    DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U34,       rv32_sifive_u_cpu_init),
 #elif defined(TARGET_RISCV64)
     DEFINE_CPU(TYPE_RISCV_CPU_BASE64,           riscv_base_cpu_init),
-    DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E51,       rvxx_sifive_e_cpu_init),
-    DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U54,       rvxx_sifive_u_cpu_init),
+    DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E51,       rv64_sifive_e_cpu_init),
+    DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U54,       rv64_sifive_u_cpu_init),
 #endif
 };
 
-- 
2.28.0

Re: [PATCH v1 10/16] target/riscv: Specify the XLEN for CPUs

[Bin Meng]

On Fri, Oct 23, 2020 at 11:45 PM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/cpu.c | 33 +++++++++++++++++++++++----------
>  1 file changed, 23 insertions(+), 10 deletions(-)
>

Reviewed-by: Bin Meng <bin.meng@windriver.com>
Tested-by: Bin Meng <bin.meng@windriver.com>

[PATCH v1 11/16] target/riscv: cpu: Remove compile time XLEN checks

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu.h |  6 ++----
 target/riscv/cpu.c | 17 +++++++++--------
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 86b063543d..6096243aed 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -151,13 +151,11 @@ struct CPURISCVState {
 
     target_ulong mhartid;
     target_ulong mstatus;
+    /* This is RV32 only */
+    target_ulong mstatush;
 
     target_ulong mip;
 
-#ifdef TARGET_RISCV32
-    target_ulong mstatush;
-#endif
-
     uint32_t miclaim;
 
     target_ulong mie;
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 8f8c568974..1b3efc1c81 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -239,9 +239,9 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int flags)
 #ifndef CONFIG_USER_ONLY
     qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mhartid ", env->mhartid);
     qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mstatus ", env->mstatus);
-#ifdef TARGET_RISCV32
-    qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mstatush ", env->mstatush);
-#endif
+    if (riscv_cpu_is_32bit(env)) {
+        qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mstatush ", env->mstatush);
+    }
     if (riscv_has_ext(env, RVH)) {
         qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "hstatus ", env->hstatus);
         qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "vsstatus ", env->vsstatus);
@@ -353,11 +353,12 @@ static void riscv_cpu_reset(DeviceState *dev)
 
 static void riscv_cpu_disas_set_info(CPUState *s, disassemble_info *info)
 {
-#if defined(TARGET_RISCV32)
-    info->print_insn = print_insn_riscv32;
-#elif defined(TARGET_RISCV64)
-    info->print_insn = print_insn_riscv64;
-#endif
+    RISCVCPU *cpu = RISCV_CPU(s);
+    if (riscv_cpu_is_32bit(&cpu->env)) {
+        info->print_insn = print_insn_riscv32;
+    } else {
+        info->print_insn = print_insn_riscv64;
+    }
 }
 
 static void riscv_cpu_realize(DeviceState *dev, Error **errp)
-- 
2.28.0

Re: [PATCH v1 11/16] target/riscv: cpu: Remove compile time XLEN checks

[Bin Meng]

On Fri, Oct 23, 2020 at 11:45 PM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/cpu.h |  6 ++----
>  target/riscv/cpu.c | 17 +++++++++--------
>  2 files changed, 11 insertions(+), 12 deletions(-)
>

Reviewed-by: Bin Meng <bin.meng@windriver.com>

[PATCH v1 12/16] target/riscv: cpu_helper: Remove compile time XLEN checks

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu.h        |  6 ++---
 target/riscv/cpu_helper.c | 52 ++++++++++++++++++++-------------------
 2 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 6096243aed..8bde15544d 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -194,9 +194,8 @@ struct CPURISCVState {
     target_ulong vscause;
     target_ulong vstval;
     target_ulong vsatp;
-#ifdef TARGET_RISCV32
+    /* This is RV32 only */
     target_ulong vsstatush;
-#endif
 
     target_ulong mtval2;
     target_ulong mtinst;
@@ -209,9 +208,8 @@ struct CPURISCVState {
     target_ulong stval_hs;
     target_ulong satp_hs;
     target_ulong mstatus_hs;
-#ifdef TARGET_RISCV32
+    /* This is RV32 only */
     target_ulong mstatush_hs;
-#endif
 
     target_ulong scounteren;
     target_ulong mcounteren;
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 4652082df1..62aed24feb 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -126,10 +126,10 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env)
         env->mstatus &= ~mstatus_mask;
         env->mstatus |= env->mstatus_hs;
 
-#if defined(TARGET_RISCV32)
-        env->vsstatush = env->mstatush;
-        env->mstatush |= env->mstatush_hs;
-#endif
+        if (riscv_cpu_is_32bit(env)) {
+            env->vsstatush = env->mstatush;
+            env->mstatush |= env->mstatush_hs;
+        }
 
         env->vstvec = env->stvec;
         env->stvec = env->stvec_hs;
@@ -154,10 +154,10 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env)
         env->mstatus &= ~mstatus_mask;
         env->mstatus |= env->vsstatus;
 
-#if defined(TARGET_RISCV32)
-        env->mstatush_hs = env->mstatush;
-        env->mstatush |= env->vsstatush;
-#endif
+        if (riscv_cpu_is_32bit(env)) {
+            env->mstatush_hs = env->mstatush;
+            env->mstatush |= env->vsstatush;
+        }
 
         env->stvec_hs = env->stvec;
         env->stvec = env->vstvec;
@@ -472,11 +472,13 @@ restart:
             return TRANSLATE_PMP_FAIL;
         }
 
-#if defined(TARGET_RISCV32)
-        target_ulong pte = address_space_ldl(cs->as, pte_addr, attrs, &res);
-#elif defined(TARGET_RISCV64)
-        target_ulong pte = address_space_ldq(cs->as, pte_addr, attrs, &res);
-#endif
+        target_ulong pte;
+        if (riscv_cpu_is_32bit(env)) {
+            pte = address_space_ldl(cs->as, pte_addr, attrs, &res);
+        } else {
+            pte = address_space_ldq(cs->as, pte_addr, attrs, &res);
+        }
+
         if (res != MEMTX_OK) {
             return TRANSLATE_FAIL;
         }
@@ -995,19 +997,19 @@ void riscv_cpu_do_interrupt(CPUState *cs)
             if (riscv_cpu_virt_enabled(env)) {
                 riscv_cpu_swap_hypervisor_regs(env);
             }
-#ifdef TARGET_RISCV32
-            env->mstatush = set_field(env->mstatush, MSTATUS_MPV,
-                                       riscv_cpu_virt_enabled(env));
-            if (riscv_cpu_virt_enabled(env) && tval) {
-                env->mstatush = set_field(env->mstatush, MSTATUS_GVA, 1);
-            }
-#else
-            env->mstatus = set_field(env->mstatus, MSTATUS_MPV,
-                                      riscv_cpu_virt_enabled(env));
-            if (riscv_cpu_virt_enabled(env) && tval) {
-                env->mstatus = set_field(env->mstatus, MSTATUS_GVA, 1);
+            if (riscv_cpu_is_32bit(env)) {
+                env->mstatush = set_field(env->mstatush, MSTATUS_MPV,
+                                           riscv_cpu_virt_enabled(env));
+                if (riscv_cpu_virt_enabled(env) && tval) {
+                    env->mstatush = set_field(env->mstatush, MSTATUS_GVA, 1);
+                }
+            } else {
+                env->mstatus = set_field(env->mstatus, MSTATUS_MPV,
+                                          riscv_cpu_virt_enabled(env));
+                if (riscv_cpu_virt_enabled(env) && tval) {
+                    env->mstatus = set_field(env->mstatus, MSTATUS_GVA, 1);
+                }
             }
-#endif
 
             mtval2 = env->guest_phys_fault_addr;
 
-- 
2.28.0

Re: [PATCH v1 12/16] target/riscv: cpu_helper: Remove compile time XLEN checks

[Bin Meng]

On Fri, Oct 23, 2020 at 11:45 PM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/cpu.h        |  6 ++---
>  target/riscv/cpu_helper.c | 52 ++++++++++++++++++++-------------------
>  2 files changed, 29 insertions(+), 29 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 6096243aed..8bde15544d 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -194,9 +194,8 @@ struct CPURISCVState {
>      target_ulong vscause;
>      target_ulong vstval;
>      target_ulong vsatp;
> -#ifdef TARGET_RISCV32
> +    /* This is RV32 only */
>      target_ulong vsstatush;

nits: could we move the definition to the line just below where
vsstatus is defined in this structure, like other similar *h members?

> -#endif
>
>      target_ulong mtval2;
>      target_ulong mtinst;
> @@ -209,9 +208,8 @@ struct CPURISCVState {
>      target_ulong stval_hs;
>      target_ulong satp_hs;
>      target_ulong mstatus_hs;
> -#ifdef TARGET_RISCV32
> +    /* This is RV32 only */
>      target_ulong mstatush_hs;
> -#endif
>
>      target_ulong scounteren;
>      target_ulong mcounteren;
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 4652082df1..62aed24feb 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -126,10 +126,10 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env)
>          env->mstatus &= ~mstatus_mask;
>          env->mstatus |= env->mstatus_hs;
>
> -#if defined(TARGET_RISCV32)
> -        env->vsstatush = env->mstatush;
> -        env->mstatush |= env->mstatush_hs;
> -#endif
> +        if (riscv_cpu_is_32bit(env)) {
> +            env->vsstatush = env->mstatush;
> +            env->mstatush |= env->mstatush_hs;
> +        }
>
>          env->vstvec = env->stvec;
>          env->stvec = env->stvec_hs;
> @@ -154,10 +154,10 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env)
>          env->mstatus &= ~mstatus_mask;
>          env->mstatus |= env->vsstatus;
>
> -#if defined(TARGET_RISCV32)
> -        env->mstatush_hs = env->mstatush;
> -        env->mstatush |= env->vsstatush;
> -#endif
> +        if (riscv_cpu_is_32bit(env)) {
> +            env->mstatush_hs = env->mstatush;
> +            env->mstatush |= env->vsstatush;
> +        }
>
>          env->stvec_hs = env->stvec;
>          env->stvec = env->vstvec;
> @@ -472,11 +472,13 @@ restart:
>              return TRANSLATE_PMP_FAIL;
>          }
>
> -#if defined(TARGET_RISCV32)
> -        target_ulong pte = address_space_ldl(cs->as, pte_addr, attrs, &res);
> -#elif defined(TARGET_RISCV64)
> -        target_ulong pte = address_space_ldq(cs->as, pte_addr, attrs, &res);
> -#endif
> +        target_ulong pte;
> +        if (riscv_cpu_is_32bit(env)) {
> +            pte = address_space_ldl(cs->as, pte_addr, attrs, &res);
> +        } else {
> +            pte = address_space_ldq(cs->as, pte_addr, attrs, &res);
> +        }
> +
>          if (res != MEMTX_OK) {
>              return TRANSLATE_FAIL;
>          }
> @@ -995,19 +997,19 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>              if (riscv_cpu_virt_enabled(env)) {
>                  riscv_cpu_swap_hypervisor_regs(env);
>              }
> -#ifdef TARGET_RISCV32
> -            env->mstatush = set_field(env->mstatush, MSTATUS_MPV,
> -                                       riscv_cpu_virt_enabled(env));
> -            if (riscv_cpu_virt_enabled(env) && tval) {
> -                env->mstatush = set_field(env->mstatush, MSTATUS_GVA, 1);
> -            }
> -#else
> -            env->mstatus = set_field(env->mstatus, MSTATUS_MPV,
> -                                      riscv_cpu_virt_enabled(env));
> -            if (riscv_cpu_virt_enabled(env) && tval) {
> -                env->mstatus = set_field(env->mstatus, MSTATUS_GVA, 1);
> +            if (riscv_cpu_is_32bit(env)) {
> +                env->mstatush = set_field(env->mstatush, MSTATUS_MPV,
> +                                           riscv_cpu_virt_enabled(env));

nits: looks the alignment is not on the left parenthesis

> +                if (riscv_cpu_virt_enabled(env) && tval) {
> +                    env->mstatush = set_field(env->mstatush, MSTATUS_GVA, 1);
> +                }
> +            } else {
> +                env->mstatus = set_field(env->mstatus, MSTATUS_MPV,
> +                                          riscv_cpu_virt_enabled(env));

ditto

> +                if (riscv_cpu_virt_enabled(env) && tval) {
> +                    env->mstatus = set_field(env->mstatus, MSTATUS_GVA, 1);
> +                }
>              }
> -#endif
>
>              mtval2 = env->guest_phys_fault_addr;
>

Regards,
Bin

Re: [PATCH v1 12/16] target/riscv: cpu_helper: Remove compile time XLEN checks

[Alistair Francis]

On Mon, Oct 26, 2020 at 1:56 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Fri, Oct 23, 2020 at 11:45 PM Alistair Francis
> <alistair.francis@wdc.com> wrote:
> >
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  target/riscv/cpu.h        |  6 ++---
> >  target/riscv/cpu_helper.c | 52 ++++++++++++++++++++-------------------
> >  2 files changed, 29 insertions(+), 29 deletions(-)
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 6096243aed..8bde15544d 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -194,9 +194,8 @@ struct CPURISCVState {
> >      target_ulong vscause;
> >      target_ulong vstval;
> >      target_ulong vsatp;
> > -#ifdef TARGET_RISCV32
> > +    /* This is RV32 only */
> >      target_ulong vsstatush;
>
> nits: could we move the definition to the line just below where
> vsstatus is defined in this structure, like other similar *h members?

This has been removed in the latest rebase.

Alistair

>
> > -#endif
> >
> >      target_ulong mtval2;
> >      target_ulong mtinst;
> > @@ -209,9 +208,8 @@ struct CPURISCVState {
> >      target_ulong stval_hs;
> >      target_ulong satp_hs;
> >      target_ulong mstatus_hs;
> > -#ifdef TARGET_RISCV32
> > +    /* This is RV32 only */
> >      target_ulong mstatush_hs;
> > -#endif
> >
> >      target_ulong scounteren;
> >      target_ulong mcounteren;
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 4652082df1..62aed24feb 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -126,10 +126,10 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env)
> >          env->mstatus &= ~mstatus_mask;
> >          env->mstatus |= env->mstatus_hs;
> >
> > -#if defined(TARGET_RISCV32)
> > -        env->vsstatush = env->mstatush;
> > -        env->mstatush |= env->mstatush_hs;
> > -#endif
> > +        if (riscv_cpu_is_32bit(env)) {
> > +            env->vsstatush = env->mstatush;
> > +            env->mstatush |= env->mstatush_hs;
> > +        }
> >
> >          env->vstvec = env->stvec;
> >          env->stvec = env->stvec_hs;
> > @@ -154,10 +154,10 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env)
> >          env->mstatus &= ~mstatus_mask;
> >          env->mstatus |= env->vsstatus;
> >
> > -#if defined(TARGET_RISCV32)
> > -        env->mstatush_hs = env->mstatush;
> > -        env->mstatush |= env->vsstatush;
> > -#endif
> > +        if (riscv_cpu_is_32bit(env)) {
> > +            env->mstatush_hs = env->mstatush;
> > +            env->mstatush |= env->vsstatush;
> > +        }
> >
> >          env->stvec_hs = env->stvec;
> >          env->stvec = env->vstvec;
> > @@ -472,11 +472,13 @@ restart:
> >              return TRANSLATE_PMP_FAIL;
> >          }
> >
> > -#if defined(TARGET_RISCV32)
> > -        target_ulong pte = address_space_ldl(cs->as, pte_addr, attrs, &res);
> > -#elif defined(TARGET_RISCV64)
> > -        target_ulong pte = address_space_ldq(cs->as, pte_addr, attrs, &res);
> > -#endif
> > +        target_ulong pte;
> > +        if (riscv_cpu_is_32bit(env)) {
> > +            pte = address_space_ldl(cs->as, pte_addr, attrs, &res);
> > +        } else {
> > +            pte = address_space_ldq(cs->as, pte_addr, attrs, &res);
> > +        }
> > +
> >          if (res != MEMTX_OK) {
> >              return TRANSLATE_FAIL;
> >          }
> > @@ -995,19 +997,19 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> >              if (riscv_cpu_virt_enabled(env)) {
> >                  riscv_cpu_swap_hypervisor_regs(env);
> >              }
> > -#ifdef TARGET_RISCV32
> > -            env->mstatush = set_field(env->mstatush, MSTATUS_MPV,
> > -                                       riscv_cpu_virt_enabled(env));
> > -            if (riscv_cpu_virt_enabled(env) && tval) {
> > -                env->mstatush = set_field(env->mstatush, MSTATUS_GVA, 1);
> > -            }
> > -#else
> > -            env->mstatus = set_field(env->mstatus, MSTATUS_MPV,
> > -                                      riscv_cpu_virt_enabled(env));
> > -            if (riscv_cpu_virt_enabled(env) && tval) {
> > -                env->mstatus = set_field(env->mstatus, MSTATUS_GVA, 1);
> > +            if (riscv_cpu_is_32bit(env)) {
> > +                env->mstatush = set_field(env->mstatush, MSTATUS_MPV,
> > +                                           riscv_cpu_virt_enabled(env));
>
> nits: looks the alignment is not on the left parenthesis
>
> > +                if (riscv_cpu_virt_enabled(env) && tval) {
> > +                    env->mstatush = set_field(env->mstatush, MSTATUS_GVA, 1);
> > +                }
> > +            } else {
> > +                env->mstatus = set_field(env->mstatus, MSTATUS_MPV,
> > +                                          riscv_cpu_virt_enabled(env));
>
> ditto
>
> > +                if (riscv_cpu_virt_enabled(env) && tval) {
> > +                    env->mstatus = set_field(env->mstatus, MSTATUS_GVA, 1);
> > +                }
> >              }
> > -#endif
> >
> >              mtval2 = env->guest_phys_fault_addr;
> >
>
> Regards,
> Bin

[PATCH v1 13/16] target/riscv: csr: Remove compile time XLEN checks

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu_bits.h |   2 -
 target/riscv/csr.c      | 182 +++++++++++++++++++++-------------------
 2 files changed, 96 insertions(+), 88 deletions(-)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index bd36062877..d9ad694b3f 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -448,9 +448,7 @@
 #define HSTATUS_VGEIN        0x0003F000
 #define HSTATUS_VTVM         0x00100000
 #define HSTATUS_VTSR         0x00400000
-#if defined(TARGET_RISCV64)
 #define HSTATUS_VSXL        0x300000000
-#endif
 
 #define HSTATUS32_WPRI       0xFF8FF87E
 #define HSTATUS64_WPRI       0xFFFFFFFFFF8FF87EULL
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index aaef6c6f20..b153cdfec3 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -102,44 +102,65 @@ static int ctr(CPURISCVState *env, int csrno)
                 return -RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
             }
             break;
-#if defined(TARGET_RISCV32)
-        case CSR_CYCLEH:
-            if (!get_field(env->hcounteren, HCOUNTEREN_CY) &&
-                get_field(env->mcounteren, HCOUNTEREN_CY)) {
-                return -RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
-            }
-            break;
-        case CSR_TIMEH:
-            if (!get_field(env->hcounteren, HCOUNTEREN_TM) &&
-                get_field(env->mcounteren, HCOUNTEREN_TM)) {
-                return -RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
-            }
-            break;
-        case CSR_INSTRETH:
-            if (!get_field(env->hcounteren, HCOUNTEREN_IR) &&
-                get_field(env->mcounteren, HCOUNTEREN_IR)) {
-                return -RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
-            }
-            break;
-        case CSR_HPMCOUNTER3H...CSR_HPMCOUNTER31H:
-            if (!get_field(env->hcounteren, 1 << (csrno - CSR_HPMCOUNTER3H)) &&
-                get_field(env->mcounteren, 1 << (csrno - CSR_HPMCOUNTER3H))) {
-                return -RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+        }
+        if (riscv_cpu_is_32bit(env)) {
+            switch (csrno) {
+            case CSR_CYCLEH:
+                if (!get_field(env->hcounteren, HCOUNTEREN_CY) &&
+                    get_field(env->mcounteren, HCOUNTEREN_CY)) {
+                    return -RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+                }
+                break;
+            case CSR_TIMEH:
+                if (!get_field(env->hcounteren, HCOUNTEREN_TM) &&
+                    get_field(env->mcounteren, HCOUNTEREN_TM)) {
+                    return -RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+                }
+                break;
+            case CSR_INSTRETH:
+                if (!get_field(env->hcounteren, HCOUNTEREN_IR) &&
+                    get_field(env->mcounteren, HCOUNTEREN_IR)) {
+                    return -RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+                }
+                break;
+            case CSR_HPMCOUNTER3H...CSR_HPMCOUNTER31H:
+                if (!get_field(env->hcounteren, 1 << (csrno - CSR_HPMCOUNTER3H)) &&
+                    get_field(env->mcounteren, 1 << (csrno - CSR_HPMCOUNTER3H))) {
+                    return -RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+                }
+                break;
             }
-            break;
-#endif
         }
     }
 #endif
     return 0;
 }
 
+static int ctr32(CPURISCVState *env, int csrno)
+{
+    if (!riscv_cpu_is_32bit(env)) {
+        return 0;
+    }
+
+    return ctr(env, csrno);
+}
+
 #if !defined(CONFIG_USER_ONLY)
 static int any(CPURISCVState *env, int csrno)
 {
     return 0;
 }
 
+static int any32(CPURISCVState *env, int csrno)
+{
+    if (!riscv_cpu_is_32bit(env)) {
+        return 0;
+    }
+
+    return any(env, csrno);
+
+}
+
 static int smode(CPURISCVState *env, int csrno)
 {
     return -!riscv_has_ext(env, RVS);
@@ -161,6 +182,16 @@ static int hmode(CPURISCVState *env, int csrno)
     return -RISCV_EXCP_ILLEGAL_INST;
 }
 
+static int hmode32(CPURISCVState *env, int csrno)
+{
+    if (!riscv_cpu_is_32bit(env)) {
+        return 0;
+    }
+
+    return hmode(env, csrno);
+
+}
+
 static int pmp(CPURISCVState *env, int csrno)
 {
     return -!riscv_feature(env, RISCV_FEATURE_PMP);
@@ -310,7 +341,6 @@ static int read_instret(CPURISCVState *env, int csrno, target_ulong *val)
     return 0;
 }
 
-#if defined(TARGET_RISCV32)
 static int read_instreth(CPURISCVState *env, int csrno, target_ulong *val)
 {
 #if !defined(CONFIG_USER_ONLY)
@@ -324,7 +354,6 @@ static int read_instreth(CPURISCVState *env, int csrno, target_ulong *val)
 #endif
     return 0;
 }
-#endif /* TARGET_RISCV32 */
 
 #if defined(CONFIG_USER_ONLY)
 static int read_time(CPURISCVState *env, int csrno, target_ulong *val)
@@ -333,13 +362,11 @@ static int read_time(CPURISCVState *env, int csrno, target_ulong *val)
     return 0;
 }
 
-#if defined(TARGET_RISCV32)
 static int read_timeh(CPURISCVState *env, int csrno, target_ulong *val)
 {
     *val = cpu_get_host_ticks() >> 32;
     return 0;
 }
-#endif
 
 #else /* CONFIG_USER_ONLY */
 
@@ -355,7 +382,6 @@ static int read_time(CPURISCVState *env, int csrno, target_ulong *val)
     return 0;
 }
 
-#if defined(TARGET_RISCV32)
 static int read_timeh(CPURISCVState *env, int csrno, target_ulong *val)
 {
     uint64_t delta = riscv_cpu_virt_enabled(env) ? env->htimedelta : 0;
@@ -367,7 +393,6 @@ static int read_timeh(CPURISCVState *env, int csrno, target_ulong *val)
     *val = (env->rdtime_fn(env->rdtime_fn_arg) + delta) >> 32;
     return 0;
 }
-#endif
 
 /* Machine constants */
 
@@ -406,19 +431,17 @@ static const target_ulong sip_writable_mask = SIP_SSIP | MIP_USIP | MIP_UEIP;
 static const target_ulong hip_writable_mask = MIP_VSSIP | MIP_VSTIP | MIP_VSEIP;
 static const target_ulong vsip_writable_mask = MIP_VSSIP;
 
-#if defined(TARGET_RISCV32)
-static const char valid_vm_1_10[16] = {
+static const char valid_vm_1_10_32[16] = {
     [VM_1_10_MBARE] = 1,
     [VM_1_10_SV32] = 1
 };
-#elif defined(TARGET_RISCV64)
-static const char valid_vm_1_10[16] = {
+
+static const char valid_vm_1_10_64[16] = {
     [VM_1_10_MBARE] = 1,
     [VM_1_10_SV39] = 1,
     [VM_1_10_SV48] = 1,
     [VM_1_10_SV57] = 1
 };
-#endif /* CONFIG_USER_ONLY */
 
 /* Machine Information Registers */
 static int read_zero(CPURISCVState *env, int csrno, target_ulong *val)
@@ -441,7 +464,11 @@ static int read_mstatus(CPURISCVState *env, int csrno, target_ulong *val)
 
 static int validate_vm(CPURISCVState *env, target_ulong vm)
 {
-    return valid_vm_1_10[vm & 0xf];
+    if (riscv_cpu_is_32bit(env)) {
+        return valid_vm_1_10_32[vm & 0xf];
+    } else {
+        return valid_vm_1_10_64[vm & 0xf];
+    }
 }
 
 static int write_mstatus(CPURISCVState *env, int csrno, target_ulong val)
@@ -459,13 +486,14 @@ static int write_mstatus(CPURISCVState *env, int csrno, target_ulong val)
         MSTATUS_SPP | MSTATUS_FS | MSTATUS_MPRV | MSTATUS_SUM |
         MSTATUS_MPP | MSTATUS_MXR | MSTATUS_TVM | MSTATUS_TSR |
         MSTATUS_TW;
-#if defined(TARGET_RISCV64)
-    /*
-     * RV32: MPV and GVA are not in mstatus. The current plan is to
-     * add them to mstatush. For now, we just don't support it.
-     */
-    mask |= MSTATUS_MPV | MSTATUS_GVA;
-#endif
+
+    if (!riscv_cpu_is_32bit(env)) {
+        /*
+         * RV32: MPV and GVA are not in mstatus. The current plan is to
+         * add them to mstatush. For now, we just don't support it.
+         */
+        mask |= MSTATUS_MPV | MSTATUS_GVA;
+    }
 
     mstatus = (mstatus & ~mask) | (val & mask);
 
@@ -477,7 +505,6 @@ static int write_mstatus(CPURISCVState *env, int csrno, target_ulong val)
     return 0;
 }
 
-#ifdef TARGET_RISCV32
 static int read_mstatush(CPURISCVState *env, int csrno, target_ulong *val)
 {
     *val = env->mstatush;
@@ -496,7 +523,6 @@ static int write_mstatush(CPURISCVState *env, int csrno, target_ulong val)
 
     return 0;
 }
-#endif
 
 static int read_misa(CPURISCVState *env, int csrno, target_ulong *val)
 {
@@ -894,10 +920,10 @@ static int write_satp(CPURISCVState *env, int csrno, target_ulong val)
 static int read_hstatus(CPURISCVState *env, int csrno, target_ulong *val)
 {
     *val = env->hstatus;
-#ifdef TARGET_RISCV64
-    /* We only support 64-bit VSXL */
-    *val = set_field(*val, HSTATUS_VSXL, 2);
-#endif
+    if (!riscv_cpu_is_32bit(env)) {
+        /* We only support 64-bit VSXL */
+        *val = set_field(*val, HSTATUS_VSXL, 2);
+    }
     /* We only support little endian */
     *val = set_field(*val, HSTATUS_VSBE, 0);
     return 0;
@@ -906,11 +932,9 @@ static int read_hstatus(CPURISCVState *env, int csrno, target_ulong *val)
 static int write_hstatus(CPURISCVState *env, int csrno, target_ulong val)
 {
     env->hstatus = val;
-#ifdef TARGET_RISCV64
-    if (get_field(val, HSTATUS_VSXL) != 2) {
+    if (!riscv_cpu_is_32bit(env) && get_field(val, HSTATUS_VSXL) != 2) {
         qemu_log_mask(LOG_UNIMP, "QEMU does not support mixed HSXLEN options.");
     }
-#endif
     if (get_field(val, HSTATUS_VSBE) != 0) {
         qemu_log_mask(LOG_UNIMP, "QEMU does not support big endian guests.");
     }
@@ -1052,11 +1076,11 @@ static int read_htimedelta(CPURISCVState *env, int csrno, target_ulong *val)
         return -RISCV_EXCP_ILLEGAL_INST;
     }
 
-#if defined(TARGET_RISCV32)
-    *val = env->htimedelta & 0xffffffff;
-#else
-    *val = env->htimedelta;
-#endif
+    if (riscv_cpu_is_32bit(env)) {
+        *val = env->htimedelta & 0xffffffff;
+    } else {
+       *val = env->htimedelta;
+    }
     return 0;
 }
 
@@ -1066,15 +1090,14 @@ static int write_htimedelta(CPURISCVState *env, int csrno, target_ulong val)
         return -RISCV_EXCP_ILLEGAL_INST;
     }
 
-#if defined(TARGET_RISCV32)
-    env->htimedelta = deposit64(env->htimedelta, 0, 32, (uint64_t)val);
-#else
-    env->htimedelta = val;
-#endif
+    if (riscv_cpu_is_32bit(env)) {
+        env->htimedelta = deposit64(env->htimedelta, 0, 32, (uint64_t)val);
+    } else {
+        env->htimedelta = val;
+    }
     return 0;
 }
 
-#if defined(TARGET_RISCV32)
 static int read_htimedeltah(CPURISCVState *env, int csrno, target_ulong *val)
 {
     if (!env->rdtime_fn) {
@@ -1094,7 +1117,6 @@ static int write_htimedeltah(CPURISCVState *env, int csrno, target_ulong val)
     env->htimedelta = deposit64(env->htimedelta, 32, 32, (uint64_t)val);
     return 0;
 }
-#endif
 
 /* Virtual CSR Registers */
 static int read_vsstatus(CPURISCVState *env, int csrno, target_ulong *val)
@@ -1372,26 +1394,20 @@ static riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     /* User Timers and Counters */
     [CSR_CYCLE] =               { ctr,  read_instret                        },
     [CSR_INSTRET] =             { ctr,  read_instret                        },
-#if defined(TARGET_RISCV32)
-    [CSR_CYCLEH] =              { ctr,  read_instreth                       },
-    [CSR_INSTRETH] =            { ctr,  read_instreth                       },
-#endif
+    [CSR_CYCLEH] =              { ctr32,  read_instreth                       },
+    [CSR_INSTRETH] =            { ctr32,  read_instreth                       },
 
     /* In privileged mode, the monitor will have to emulate TIME CSRs only if
      * rdtime callback is not provided by machine/platform emulation */
     [CSR_TIME] =                { ctr,  read_time                           },
-#if defined(TARGET_RISCV32)
-    [CSR_TIMEH] =               { ctr,  read_timeh                          },
-#endif
+    [CSR_TIMEH] =               { ctr32,  read_timeh                          },
 
 #if !defined(CONFIG_USER_ONLY)
     /* Machine Timers and Counters */
     [CSR_MCYCLE] =              { any,  read_instret                        },
     [CSR_MINSTRET] =            { any,  read_instret                        },
-#if defined(TARGET_RISCV32)
-    [CSR_MCYCLEH] =             { any,  read_instreth                       },
-    [CSR_MINSTRETH] =           { any,  read_instreth                       },
-#endif
+    [CSR_MCYCLEH] =             { any32,  read_instreth                       },
+    [CSR_MINSTRETH] =           { any32,  read_instreth                       },
 
     /* Machine Information Registers */
     [CSR_MVENDORID] =           { any,  read_zero                           },
@@ -1408,9 +1424,7 @@ static riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     [CSR_MTVEC] =               { any,  read_mtvec,       write_mtvec       },
     [CSR_MCOUNTEREN] =          { any,  read_mcounteren,  write_mcounteren  },
 
-#if defined(TARGET_RISCV32)
-    [CSR_MSTATUSH] =            { any,  read_mstatush,    write_mstatush    },
-#endif
+    [CSR_MSTATUSH] =            { any32,  read_mstatush,    write_mstatush    },
 
     [CSR_MSCOUNTEREN] =         { any,  read_mscounteren, write_mscounteren },
 
@@ -1450,9 +1464,7 @@ static riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     [CSR_HGEIP] =               { hmode,   read_hgeip,       write_hgeip      },
     [CSR_HGATP] =               { hmode,   read_hgatp,       write_hgatp      },
     [CSR_HTIMEDELTA] =          { hmode,   read_htimedelta,  write_htimedelta },
-#if defined(TARGET_RISCV32)
-    [CSR_HTIMEDELTAH] =         { hmode,   read_htimedeltah, write_htimedeltah},
-#endif
+    [CSR_HTIMEDELTAH] =         { hmode32,   read_htimedeltah, write_htimedeltah},
 
     [CSR_VSSTATUS] =            { hmode,   read_vsstatus,    write_vsstatus   },
     [CSR_VSIP] =                { hmode,   NULL,     NULL,     rmw_vsip       },
@@ -1475,9 +1487,7 @@ static riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     [CSR_HPMCOUNTER3   ... CSR_HPMCOUNTER31] =    { ctr,  read_zero          },
     [CSR_MHPMCOUNTER3  ... CSR_MHPMCOUNTER31] =   { any,  read_zero          },
     [CSR_MHPMEVENT3    ... CSR_MHPMEVENT31] =     { any,  read_zero          },
-#if defined(TARGET_RISCV32)
-    [CSR_HPMCOUNTER3H  ... CSR_HPMCOUNTER31H] =   { ctr,  read_zero          },
-    [CSR_MHPMCOUNTER3H ... CSR_MHPMCOUNTER31H] =  { any,  read_zero          },
-#endif
+    [CSR_HPMCOUNTER3H  ... CSR_HPMCOUNTER31H] =   { ctr32,  read_zero          },
+    [CSR_MHPMCOUNTER3H ... CSR_MHPMCOUNTER31H] =  { any32,  read_zero          },
 #endif /* !CONFIG_USER_ONLY */
 };
-- 
2.28.0

Re: [PATCH v1 13/16] target/riscv: csr: Remove compile time XLEN checks

[Bin Meng]

On Fri, Oct 23, 2020 at 11:45 PM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/cpu_bits.h |   2 -
>  target/riscv/csr.c      | 182 +++++++++++++++++++++-------------------
>  2 files changed, 96 insertions(+), 88 deletions(-)
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index bd36062877..d9ad694b3f 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -448,9 +448,7 @@
>  #define HSTATUS_VGEIN        0x0003F000
>  #define HSTATUS_VTVM         0x00100000
>  #define HSTATUS_VTSR         0x00400000
> -#if defined(TARGET_RISCV64)
>  #define HSTATUS_VSXL        0x300000000

nits: the indentation looks not aligned to the other lines in this code block

> -#endif
>
>  #define HSTATUS32_WPRI       0xFF8FF87E
>  #define HSTATUS64_WPRI       0xFFFFFFFFFF8FF87EULL
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index aaef6c6f20..b153cdfec3 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -102,44 +102,65 @@ static int ctr(CPURISCVState *env, int csrno)
>                  return -RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
>              }
>              break;
> -#if defined(TARGET_RISCV32)
> -        case CSR_CYCLEH:
> -            if (!get_field(env->hcounteren, HCOUNTEREN_CY) &&
> -                get_field(env->mcounteren, HCOUNTEREN_CY)) {
> -                return -RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> -            }
> -            break;
> -        case CSR_TIMEH:
> -            if (!get_field(env->hcounteren, HCOUNTEREN_TM) &&
> -                get_field(env->mcounteren, HCOUNTEREN_TM)) {
> -                return -RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> -            }
> -            break;
> -        case CSR_INSTRETH:
> -            if (!get_field(env->hcounteren, HCOUNTEREN_IR) &&
> -                get_field(env->mcounteren, HCOUNTEREN_IR)) {
> -                return -RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> -            }
> -            break;
> -        case CSR_HPMCOUNTER3H...CSR_HPMCOUNTER31H:
> -            if (!get_field(env->hcounteren, 1 << (csrno - CSR_HPMCOUNTER3H)) &&
> -                get_field(env->mcounteren, 1 << (csrno - CSR_HPMCOUNTER3H))) {
> -                return -RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> +        }
> +        if (riscv_cpu_is_32bit(env)) {
> +            switch (csrno) {
> +            case CSR_CYCLEH:
> +                if (!get_field(env->hcounteren, HCOUNTEREN_CY) &&
> +                    get_field(env->mcounteren, HCOUNTEREN_CY)) {
> +                    return -RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> +                }
> +                break;
> +            case CSR_TIMEH:
> +                if (!get_field(env->hcounteren, HCOUNTEREN_TM) &&
> +                    get_field(env->mcounteren, HCOUNTEREN_TM)) {
> +                    return -RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> +                }
> +                break;
> +            case CSR_INSTRETH:
> +                if (!get_field(env->hcounteren, HCOUNTEREN_IR) &&
> +                    get_field(env->mcounteren, HCOUNTEREN_IR)) {
> +                    return -RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> +                }
> +                break;
> +            case CSR_HPMCOUNTER3H...CSR_HPMCOUNTER31H:
> +                if (!get_field(env->hcounteren, 1 << (csrno - CSR_HPMCOUNTER3H)) &&
> +                    get_field(env->mcounteren, 1 << (csrno - CSR_HPMCOUNTER3H))) {
> +                    return -RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> +                }
> +                break;
>              }
> -            break;
> -#endif
>          }
>      }
>  #endif
>      return 0;
>  }
>
> +static int ctr32(CPURISCVState *env, int csrno)
> +{
> +    if (!riscv_cpu_is_32bit(env)) {
> +        return 0;

I believe this should return -RISCV_EXCP_ILLEGAL_INST

> +    }
> +
> +    return ctr(env, csrno);
> +}
> +
>  #if !defined(CONFIG_USER_ONLY)
>  static int any(CPURISCVState *env, int csrno)
>  {
>      return 0;
>  }
>
> +static int any32(CPURISCVState *env, int csrno)
> +{
> +    if (!riscv_cpu_is_32bit(env)) {
> +        return 0;

ditto

> +    }
> +
> +    return any(env, csrno);
> +
> +}
> +
>  static int smode(CPURISCVState *env, int csrno)
>  {
>      return -!riscv_has_ext(env, RVS);
> @@ -161,6 +182,16 @@ static int hmode(CPURISCVState *env, int csrno)
>      return -RISCV_EXCP_ILLEGAL_INST;
>  }
>
> +static int hmode32(CPURISCVState *env, int csrno)
> +{
> +    if (!riscv_cpu_is_32bit(env)) {
> +        return 0;
> +    }
> +
> +    return hmode(env, csrno);
> +
> +}
> +
>  static int pmp(CPURISCVState *env, int csrno)
>  {
>      return -!riscv_feature(env, RISCV_FEATURE_PMP);
> @@ -310,7 +341,6 @@ static int read_instret(CPURISCVState *env, int csrno, target_ulong *val)
>      return 0;
>  }
>
> -#if defined(TARGET_RISCV32)
>  static int read_instreth(CPURISCVState *env, int csrno, target_ulong *val)
>  {
>  #if !defined(CONFIG_USER_ONLY)
> @@ -324,7 +354,6 @@ static int read_instreth(CPURISCVState *env, int csrno, target_ulong *val)
>  #endif
>      return 0;
>  }
> -#endif /* TARGET_RISCV32 */
>
>  #if defined(CONFIG_USER_ONLY)
>  static int read_time(CPURISCVState *env, int csrno, target_ulong *val)
> @@ -333,13 +362,11 @@ static int read_time(CPURISCVState *env, int csrno, target_ulong *val)
>      return 0;
>  }
>
> -#if defined(TARGET_RISCV32)
>  static int read_timeh(CPURISCVState *env, int csrno, target_ulong *val)
>  {
>      *val = cpu_get_host_ticks() >> 32;
>      return 0;
>  }
> -#endif
>
>  #else /* CONFIG_USER_ONLY */
>
> @@ -355,7 +382,6 @@ static int read_time(CPURISCVState *env, int csrno, target_ulong *val)
>      return 0;
>  }
>
> -#if defined(TARGET_RISCV32)
>  static int read_timeh(CPURISCVState *env, int csrno, target_ulong *val)
>  {
>      uint64_t delta = riscv_cpu_virt_enabled(env) ? env->htimedelta : 0;
> @@ -367,7 +393,6 @@ static int read_timeh(CPURISCVState *env, int csrno, target_ulong *val)
>      *val = (env->rdtime_fn(env->rdtime_fn_arg) + delta) >> 32;
>      return 0;
>  }
> -#endif
>
>  /* Machine constants */
>
> @@ -406,19 +431,17 @@ static const target_ulong sip_writable_mask = SIP_SSIP | MIP_USIP | MIP_UEIP;
>  static const target_ulong hip_writable_mask = MIP_VSSIP | MIP_VSTIP | MIP_VSEIP;
>  static const target_ulong vsip_writable_mask = MIP_VSSIP;
>
> -#if defined(TARGET_RISCV32)
> -static const char valid_vm_1_10[16] = {
> +static const char valid_vm_1_10_32[16] = {
>      [VM_1_10_MBARE] = 1,
>      [VM_1_10_SV32] = 1
>  };
> -#elif defined(TARGET_RISCV64)
> -static const char valid_vm_1_10[16] = {
> +
> +static const char valid_vm_1_10_64[16] = {
>      [VM_1_10_MBARE] = 1,
>      [VM_1_10_SV39] = 1,
>      [VM_1_10_SV48] = 1,
>      [VM_1_10_SV57] = 1
>  };
> -#endif /* CONFIG_USER_ONLY */
>
>  /* Machine Information Registers */
>  static int read_zero(CPURISCVState *env, int csrno, target_ulong *val)
> @@ -441,7 +464,11 @@ static int read_mstatus(CPURISCVState *env, int csrno, target_ulong *val)
>
>  static int validate_vm(CPURISCVState *env, target_ulong vm)
>  {
> -    return valid_vm_1_10[vm & 0xf];
> +    if (riscv_cpu_is_32bit(env)) {
> +        return valid_vm_1_10_32[vm & 0xf];
> +    } else {
> +        return valid_vm_1_10_64[vm & 0xf];
> +    }
>  }
>
>  static int write_mstatus(CPURISCVState *env, int csrno, target_ulong val)
> @@ -459,13 +486,14 @@ static int write_mstatus(CPURISCVState *env, int csrno, target_ulong val)
>          MSTATUS_SPP | MSTATUS_FS | MSTATUS_MPRV | MSTATUS_SUM |
>          MSTATUS_MPP | MSTATUS_MXR | MSTATUS_TVM | MSTATUS_TSR |
>          MSTATUS_TW;
> -#if defined(TARGET_RISCV64)
> -    /*
> -     * RV32: MPV and GVA are not in mstatus. The current plan is to
> -     * add them to mstatush. For now, we just don't support it.
> -     */
> -    mask |= MSTATUS_MPV | MSTATUS_GVA;
> -#endif
> +
> +    if (!riscv_cpu_is_32bit(env)) {
> +        /*
> +         * RV32: MPV and GVA are not in mstatus. The current plan is to
> +         * add them to mstatush. For now, we just don't support it.
> +         */
> +        mask |= MSTATUS_MPV | MSTATUS_GVA;
> +    }
>
>      mstatus = (mstatus & ~mask) | (val & mask);
>
> @@ -477,7 +505,6 @@ static int write_mstatus(CPURISCVState *env, int csrno, target_ulong val)
>      return 0;
>  }
>
> -#ifdef TARGET_RISCV32
>  static int read_mstatush(CPURISCVState *env, int csrno, target_ulong *val)
>  {
>      *val = env->mstatush;
> @@ -496,7 +523,6 @@ static int write_mstatush(CPURISCVState *env, int csrno, target_ulong val)
>
>      return 0;
>  }
> -#endif
>
>  static int read_misa(CPURISCVState *env, int csrno, target_ulong *val)
>  {
> @@ -894,10 +920,10 @@ static int write_satp(CPURISCVState *env, int csrno, target_ulong val)
>  static int read_hstatus(CPURISCVState *env, int csrno, target_ulong *val)
>  {
>      *val = env->hstatus;
> -#ifdef TARGET_RISCV64
> -    /* We only support 64-bit VSXL */
> -    *val = set_field(*val, HSTATUS_VSXL, 2);
> -#endif
> +    if (!riscv_cpu_is_32bit(env)) {
> +        /* We only support 64-bit VSXL */
> +        *val = set_field(*val, HSTATUS_VSXL, 2);
> +    }
>      /* We only support little endian */
>      *val = set_field(*val, HSTATUS_VSBE, 0);
>      return 0;
> @@ -906,11 +932,9 @@ static int read_hstatus(CPURISCVState *env, int csrno, target_ulong *val)
>  static int write_hstatus(CPURISCVState *env, int csrno, target_ulong val)
>  {
>      env->hstatus = val;
> -#ifdef TARGET_RISCV64
> -    if (get_field(val, HSTATUS_VSXL) != 2) {
> +    if (!riscv_cpu_is_32bit(env) && get_field(val, HSTATUS_VSXL) != 2) {
>          qemu_log_mask(LOG_UNIMP, "QEMU does not support mixed HSXLEN options.");
>      }
> -#endif
>      if (get_field(val, HSTATUS_VSBE) != 0) {
>          qemu_log_mask(LOG_UNIMP, "QEMU does not support big endian guests.");
>      }
> @@ -1052,11 +1076,11 @@ static int read_htimedelta(CPURISCVState *env, int csrno, target_ulong *val)
>          return -RISCV_EXCP_ILLEGAL_INST;
>      }
>
> -#if defined(TARGET_RISCV32)
> -    *val = env->htimedelta & 0xffffffff;
> -#else
> -    *val = env->htimedelta;
> -#endif
> +    if (riscv_cpu_is_32bit(env)) {
> +        *val = env->htimedelta & 0xffffffff;
> +    } else {
> +       *val = env->htimedelta;

nits: *val indentation not aligned

> +    }
>      return 0;
>  }
>
> @@ -1066,15 +1090,14 @@ static int write_htimedelta(CPURISCVState *env, int csrno, target_ulong val)
>          return -RISCV_EXCP_ILLEGAL_INST;
>      }
>
> -#if defined(TARGET_RISCV32)
> -    env->htimedelta = deposit64(env->htimedelta, 0, 32, (uint64_t)val);
> -#else
> -    env->htimedelta = val;
> -#endif
> +    if (riscv_cpu_is_32bit(env)) {
> +        env->htimedelta = deposit64(env->htimedelta, 0, 32, (uint64_t)val);
> +    } else {
> +        env->htimedelta = val;
> +    }
>      return 0;
>  }
>
> -#if defined(TARGET_RISCV32)
>  static int read_htimedeltah(CPURISCVState *env, int csrno, target_ulong *val)
>  {
>      if (!env->rdtime_fn) {
> @@ -1094,7 +1117,6 @@ static int write_htimedeltah(CPURISCVState *env, int csrno, target_ulong val)
>      env->htimedelta = deposit64(env->htimedelta, 32, 32, (uint64_t)val);
>      return 0;
>  }
> -#endif
>
>  /* Virtual CSR Registers */
>  static int read_vsstatus(CPURISCVState *env, int csrno, target_ulong *val)
> @@ -1372,26 +1394,20 @@ static riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>      /* User Timers and Counters */
>      [CSR_CYCLE] =               { ctr,  read_instret                        },
>      [CSR_INSTRET] =             { ctr,  read_instret                        },
> -#if defined(TARGET_RISCV32)
> -    [CSR_CYCLEH] =              { ctr,  read_instreth                       },
> -    [CSR_INSTRETH] =            { ctr,  read_instreth                       },
> -#endif
> +    [CSR_CYCLEH] =              { ctr32,  read_instreth                       },
> +    [CSR_INSTRETH] =            { ctr32,  read_instreth                       },
>
>      /* In privileged mode, the monitor will have to emulate TIME CSRs only if
>       * rdtime callback is not provided by machine/platform emulation */
>      [CSR_TIME] =                { ctr,  read_time                           },
> -#if defined(TARGET_RISCV32)
> -    [CSR_TIMEH] =               { ctr,  read_timeh                          },
> -#endif
> +    [CSR_TIMEH] =               { ctr32,  read_timeh                          },
>
>  #if !defined(CONFIG_USER_ONLY)
>      /* Machine Timers and Counters */
>      [CSR_MCYCLE] =              { any,  read_instret                        },
>      [CSR_MINSTRET] =            { any,  read_instret                        },
> -#if defined(TARGET_RISCV32)
> -    [CSR_MCYCLEH] =             { any,  read_instreth                       },
> -    [CSR_MINSTRETH] =           { any,  read_instreth                       },
> -#endif
> +    [CSR_MCYCLEH] =             { any32,  read_instreth                       },
> +    [CSR_MINSTRETH] =           { any32,  read_instreth                       },
>
>      /* Machine Information Registers */
>      [CSR_MVENDORID] =           { any,  read_zero                           },
> @@ -1408,9 +1424,7 @@ static riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>      [CSR_MTVEC] =               { any,  read_mtvec,       write_mtvec       },
>      [CSR_MCOUNTEREN] =          { any,  read_mcounteren,  write_mcounteren  },
>
> -#if defined(TARGET_RISCV32)
> -    [CSR_MSTATUSH] =            { any,  read_mstatush,    write_mstatush    },
> -#endif
> +    [CSR_MSTATUSH] =            { any32,  read_mstatush,    write_mstatush    },
>
>      [CSR_MSCOUNTEREN] =         { any,  read_mscounteren, write_mscounteren },
>
> @@ -1450,9 +1464,7 @@ static riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>      [CSR_HGEIP] =               { hmode,   read_hgeip,       write_hgeip      },
>      [CSR_HGATP] =               { hmode,   read_hgatp,       write_hgatp      },
>      [CSR_HTIMEDELTA] =          { hmode,   read_htimedelta,  write_htimedelta },
> -#if defined(TARGET_RISCV32)
> -    [CSR_HTIMEDELTAH] =         { hmode,   read_htimedeltah, write_htimedeltah},
> -#endif
> +    [CSR_HTIMEDELTAH] =         { hmode32,   read_htimedeltah, write_htimedeltah},
>
>      [CSR_VSSTATUS] =            { hmode,   read_vsstatus,    write_vsstatus   },
>      [CSR_VSIP] =                { hmode,   NULL,     NULL,     rmw_vsip       },
> @@ -1475,9 +1487,7 @@ static riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>      [CSR_HPMCOUNTER3   ... CSR_HPMCOUNTER31] =    { ctr,  read_zero          },
>      [CSR_MHPMCOUNTER3  ... CSR_MHPMCOUNTER31] =   { any,  read_zero          },
>      [CSR_MHPMEVENT3    ... CSR_MHPMEVENT31] =     { any,  read_zero          },
> -#if defined(TARGET_RISCV32)
> -    [CSR_HPMCOUNTER3H  ... CSR_HPMCOUNTER31H] =   { ctr,  read_zero          },
> -    [CSR_MHPMCOUNTER3H ... CSR_MHPMCOUNTER31H] =  { any,  read_zero          },
> -#endif
> +    [CSR_HPMCOUNTER3H  ... CSR_HPMCOUNTER31H] =   { ctr32,  read_zero          },
> +    [CSR_MHPMCOUNTER3H ... CSR_MHPMCOUNTER31H] =  { any32,  read_zero          },
>  #endif /* !CONFIG_USER_ONLY */
>  };
> --

Regards,
Bin

[PATCH v1 14/16] target/riscv: cpu: Set XLEN independently from target

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 1b3efc1c81..c77687ccb2 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -150,14 +150,14 @@ static void riscv_any_cpu_init(Object *obj)
     set_priv_version(env, PRIV_VERSION_1_11_0);
 }
 
-static void riscv_base_cpu_init(Object *obj)
+#if defined(TARGET_RISCV64)
+static void rv64_base_cpu_init(Object *obj)
 {
     CPURISCVState *env = &RISCV_CPU(obj)->env;
     /* We set this in the realise function */
-    set_misa(env, 0);
+    set_misa(env, RV64);
 }
 
-#ifdef TARGET_RISCV64
 static void rv64_sifive_u_cpu_init(Object *obj)
 {
     CPURISCVState *env = &RISCV_CPU(obj)->env;
@@ -173,6 +173,13 @@ static void rv64_sifive_e_cpu_init(Object *obj)
     qdev_prop_set_bit(DEVICE(obj), "mmu", false);
 }
 #else
+static void rv32_base_cpu_init(Object *obj)
+{
+    CPURISCVState *env = &RISCV_CPU(obj)->env;
+    /* We set this in the realise function */
+    set_misa(env, RV32);
+}
+
 static void rv32_sifive_u_cpu_init(Object *obj)
 {
     CPURISCVState *env = &RISCV_CPU(obj)->env;
@@ -369,7 +376,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
     RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev);
     int priv_version = PRIV_VERSION_1_11_0;
     int vext_version = VEXT_VERSION_0_07_1;
-    target_ulong target_misa = 0;
+    target_ulong target_misa = env->misa;
     Error *local_err = NULL;
 
     cpu_exec_realizefn(cs, &local_err);
@@ -404,8 +411,8 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
 
     set_resetvec(env, cpu->cfg.resetvec);
 
-    /* If misa isn't set (rv32 and rv64 machines) set it here */
-    if (!env->misa) {
+    /* If only XLEN is set for misa, then set misa from properties */
+    if (env->misa == RV32 || env->misa == RV64) {
         /* Do some ISA extension error checking */
         if (cpu->cfg.ext_i && cpu->cfg.ext_e) {
             error_setg(errp,
@@ -501,7 +508,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
             set_vext_version(env, vext_version);
         }
 
-        set_misa(env, RVXLEN | target_misa);
+        set_misa(env, target_misa);
     }
 
     riscv_cpu_register_gdb_regs_for_features(cs);
@@ -659,13 +666,13 @@ static const TypeInfo riscv_cpu_type_infos[] = {
     },
     DEFINE_CPU(TYPE_RISCV_CPU_ANY,              riscv_any_cpu_init),
 #if defined(TARGET_RISCV32)
-    DEFINE_CPU(TYPE_RISCV_CPU_BASE32,           riscv_base_cpu_init),
+    DEFINE_CPU(TYPE_RISCV_CPU_BASE32,           rv32_base_cpu_init),
     DEFINE_CPU(TYPE_RISCV_CPU_IBEX,             rv32_ibex_cpu_init),
     DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E31,       rv32_sifive_e_cpu_init),
     DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E34,       rv32_imafcu_nommu_cpu_init),
     DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U34,       rv32_sifive_u_cpu_init),
 #elif defined(TARGET_RISCV64)
-    DEFINE_CPU(TYPE_RISCV_CPU_BASE64,           riscv_base_cpu_init),
+    DEFINE_CPU(TYPE_RISCV_CPU_BASE64,           rv64_base_cpu_init),
     DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E51,       rv64_sifive_e_cpu_init),
     DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U54,       rv64_sifive_u_cpu_init),
 #endif
-- 
2.28.0

Re: [PATCH v1 14/16] target/riscv: cpu: Set XLEN independently from target

[Bin Meng]

On Fri, Oct 23, 2020 at 11:45 PM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/cpu.c | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
>

Reviewed-by: Bin Meng <bin.meng@windriver.com>
Tested-by: Bin Meng <bin.meng@windriver.com>

[PATCH v1 15/16] target/riscv: Convert the get/set_field() to support 64-bit values

[Alistair Francis]

Allow the get_field() and set_field() macros to work on 64-bit values
even on 32-bit RISC-V.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu_bits.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index d9ad694b3f..db46739b1c 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -4,10 +4,10 @@
 #define TARGET_RISCV_CPU_BITS_H
 
 #define get_field(reg, mask) (((reg) & \
-                 (target_ulong)(mask)) / ((mask) & ~((mask) << 1)))
-#define set_field(reg, mask, val) (((reg) & ~(target_ulong)(mask)) | \
-                 (((target_ulong)(val) * ((mask) & ~((mask) << 1))) & \
-                 (target_ulong)(mask)))
+                 (uint64_t)(mask)) / ((mask) & ~((mask) << 1)))
+#define set_field(reg, mask, val) (((reg) & ~(uint64_t)(mask)) | \
+                 (((uint64_t)(val) * ((mask) & ~((mask) << 1))) & \
+                 (uint64_t)(mask)))
 
 /* Floating point round mode */
 #define FSR_RD_SHIFT        5
-- 
2.28.0

[PATCH v1 16/16] target/riscv: Consolidate *statush registers

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu.h        | 24 ++++++++++++++---------
 target/riscv/cpu_bits.h   | 14 ++-----------
 target/riscv/cpu.c        |  7 ++-----
 target/riscv/cpu_helper.c | 41 ++++++++++-----------------------------
 target/riscv/csr.c        | 15 ++++++++------
 target/riscv/op_helper.c  | 11 ++++-------
 6 files changed, 42 insertions(+), 70 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 8bde15544d..de864a947b 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -150,9 +150,11 @@ struct CPURISCVState {
     target_ulong resetvec;
 
     target_ulong mhartid;
-    target_ulong mstatus;
-    /* This is RV32 only */
-    target_ulong mstatush;
+    /*
+     * For RV32 this is 32-bit mstatus and 32-bit mstatush.
+     * For RV64 this is a 64-bit mstatus.
+     */
+    uint64_t mstatus;
 
     target_ulong mip;
 
@@ -187,15 +189,17 @@ struct CPURISCVState {
     uint64_t htimedelta;
 
     /* Virtual CSRs */
-    target_ulong vsstatus;
     target_ulong vstvec;
     target_ulong vsscratch;
     target_ulong vsepc;
     target_ulong vscause;
     target_ulong vstval;
     target_ulong vsatp;
-    /* This is RV32 only */
-    target_ulong vsstatush;
+    /*
+     * For RV32 this is 32-bit mstatus and 32-bit mstatush.
+     * For RV64 this is a 64-bit mstatus.
+     */
+    uint64_t vsstatus;
 
     target_ulong mtval2;
     target_ulong mtinst;
@@ -207,9 +211,11 @@ struct CPURISCVState {
     target_ulong scause_hs;
     target_ulong stval_hs;
     target_ulong satp_hs;
-    target_ulong mstatus_hs;
-    /* This is RV32 only */
-    target_ulong mstatush_hs;
+    /*
+     * For RV32 this is 32-bit mstatus and 32-bit mstatush.
+     * For RV64 this is a 64-bit mstatus.
+     */
+    uint64_t mstatus_hs;
 
     target_ulong scounteren;
     target_ulong mcounteren;
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index db46739b1c..4b75d2766e 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -381,19 +381,9 @@
 #define MSTATUS_TVM         0x00100000 /* since: priv-1.10 */
 #define MSTATUS_TW          0x20000000 /* since: priv-1.10 */
 #define MSTATUS_TSR         0x40000000 /* since: priv-1.10 */
-#if defined(TARGET_RISCV64)
-#define MSTATUS_GVA         0x4000000000ULL
-#define MSTATUS_MPV         0x8000000000ULL
-#elif defined(TARGET_RISCV32)
-#define MSTATUS_GVA         0x00000040
-#define MSTATUS_MPV         0x00000080
-#endif
 
-#ifdef TARGET_RISCV32
-# define MSTATUS_MPV_ISSET(env)  get_field(env->mstatush, MSTATUS_MPV)
-#else
-# define MSTATUS_MPV_ISSET(env)  get_field(env->mstatus, MSTATUS_MPV)
-#endif
+#define MSTATUS_GVA      0x4000000000ULL
+#define MSTATUS_MPV      0x8000000000ULL
 
 #define MSTATUS64_UXL       0x0000000300000000ULL
 #define MSTATUS64_SXL       0x0000000C00000000ULL
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index c77687ccb2..74c5ac5777 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -245,13 +245,10 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int flags)
     qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "pc      ", env->pc);
 #ifndef CONFIG_USER_ONLY
     qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mhartid ", env->mhartid);
-    qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mstatus ", env->mstatus);
-    if (riscv_cpu_is_32bit(env)) {
-        qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mstatush ", env->mstatush);
-    }
+    qemu_fprintf(f, " %s %016lx\n", "mstatus ", env->mstatus);
     if (riscv_has_ext(env, RVH)) {
         qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "hstatus ", env->hstatus);
-        qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "vsstatus ", env->vsstatus);
+        qemu_fprintf(f, " %s %016lx\n", "vsstatus ", env->vsstatus);
     }
     qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mip     ", env->mip);
     qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mie     ", env->mie);
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 62aed24feb..b109529ace 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -110,27 +110,19 @@ bool riscv_cpu_fp_enabled(CPURISCVState *env)
 
 void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env)
 {
-    target_ulong mstatus_mask = MSTATUS_MXR | MSTATUS_SUM | MSTATUS_FS |
-                                MSTATUS_SPP | MSTATUS_SPIE | MSTATUS_SIE;
+    uint64_t mstatus_mask = MSTATUS_MXR | MSTATUS_SUM | MSTATUS_FS |
+                            MSTATUS_SPP | MSTATUS_SPIE | MSTATUS_SIE |
+                            MSTATUS64_UXL;
     bool current_virt = riscv_cpu_virt_enabled(env);
 
     g_assert(riscv_has_ext(env, RVH));
 
-#if defined(TARGET_RISCV64)
-    mstatus_mask |= MSTATUS64_UXL;
-#endif
-
     if (current_virt) {
         /* Current V=1 and we are about to change to V=0 */
         env->vsstatus = env->mstatus & mstatus_mask;
         env->mstatus &= ~mstatus_mask;
         env->mstatus |= env->mstatus_hs;
 
-        if (riscv_cpu_is_32bit(env)) {
-            env->vsstatush = env->mstatush;
-            env->mstatush |= env->mstatush_hs;
-        }
-
         env->vstvec = env->stvec;
         env->stvec = env->stvec_hs;
 
@@ -154,11 +146,6 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env)
         env->mstatus &= ~mstatus_mask;
         env->mstatus |= env->vsstatus;
 
-        if (riscv_cpu_is_32bit(env)) {
-            env->mstatush_hs = env->mstatush;
-            env->mstatush |= env->vsstatush;
-        }
-
         env->stvec_hs = env->stvec;
         env->stvec = env->vstvec;
 
@@ -729,7 +716,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
     if (riscv_has_ext(env, RVH) && env->priv == PRV_M &&
         access_type != MMU_INST_FETCH &&
         get_field(env->mstatus, MSTATUS_MPRV) &&
-        MSTATUS_MPV_ISSET(env)) {
+        get_field(env->mstatus, MSTATUS_MPV)) {
         riscv_cpu_set_two_stage_lookup(env, true);
     }
 
@@ -801,7 +788,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
     if (riscv_has_ext(env, RVH) && env->priv == PRV_M &&
         access_type != MMU_INST_FETCH &&
         get_field(env->mstatus, MSTATUS_MPRV) &&
-        MSTATUS_MPV_ISSET(env)) {
+        get_field(env->mstatus, MSTATUS_MPV)) {
         riscv_cpu_set_two_stage_lookup(env, false);
     }
 
@@ -864,7 +851,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
     RISCVCPU *cpu = RISCV_CPU(cs);
     CPURISCVState *env = &cpu->env;
     bool force_hs_execp = riscv_cpu_force_hs_excep_enabled(env);
-    target_ulong s;
+    uint64_t s;
 
     /* cs->exception is 32-bits wide unlike mcause which is XLEN-bits wide
      * so we mask off the MSB and separate into trap type and cause.
@@ -997,18 +984,10 @@ void riscv_cpu_do_interrupt(CPUState *cs)
             if (riscv_cpu_virt_enabled(env)) {
                 riscv_cpu_swap_hypervisor_regs(env);
             }
-            if (riscv_cpu_is_32bit(env)) {
-                env->mstatush = set_field(env->mstatush, MSTATUS_MPV,
-                                           riscv_cpu_virt_enabled(env));
-                if (riscv_cpu_virt_enabled(env) && tval) {
-                    env->mstatush = set_field(env->mstatush, MSTATUS_GVA, 1);
-                }
-            } else {
-                env->mstatus = set_field(env->mstatus, MSTATUS_MPV,
-                                          riscv_cpu_virt_enabled(env));
-                if (riscv_cpu_virt_enabled(env) && tval) {
-                    env->mstatus = set_field(env->mstatus, MSTATUS_GVA, 1);
-                }
+            env->mstatus = set_field(env->mstatus, MSTATUS_MPV,
+                                     riscv_cpu_virt_enabled(env));
+            if (riscv_cpu_virt_enabled(env) && tval) {
+                env->mstatus = set_field(env->mstatus, MSTATUS_GVA, 1);
             }
 
             mtval2 = env->guest_phys_fault_addr;
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index b153cdfec3..7132a9bcd0 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -473,8 +473,8 @@ static int validate_vm(CPURISCVState *env, target_ulong vm)
 
 static int write_mstatus(CPURISCVState *env, int csrno, target_ulong val)
 {
-    target_ulong mstatus = env->mstatus;
-    target_ulong mask = 0;
+    uint64_t mstatus = env->mstatus;
+    uint64_t mask = 0;
     int dirty;
 
     /* flush tlb on mstatus fields that affect VM */
@@ -507,19 +507,22 @@ static int write_mstatus(CPURISCVState *env, int csrno, target_ulong val)
 
 static int read_mstatush(CPURISCVState *env, int csrno, target_ulong *val)
 {
-    *val = env->mstatush;
+    *val = env->mstatus >> 32;
     return 0;
 }
 
 static int write_mstatush(CPURISCVState *env, int csrno, target_ulong val)
 {
-    if ((val ^ env->mstatush) & (MSTATUS_MPV)) {
+    uint64_t val64 = (uint64_t) val << 32;
+
+    if ((val64 ^ env->mstatus) & (MSTATUS_MPV)) {
         tlb_flush(env_cpu(env));
     }
 
-    val &= MSTATUS_MPV | MSTATUS_GVA;
+    val64 &= MSTATUS_MPV | MSTATUS_GVA;
 
-    env->mstatush = val;
+    env->mstatus &= 0xFFFFFFFF;
+    env->mstatus |= val64;
 
     return 0;
 }
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 4ce73575a7..f9361e3af8 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -78,7 +78,8 @@ target_ulong helper_csrrc(CPURISCVState *env, target_ulong src,
 
 target_ulong helper_sret(CPURISCVState *env, target_ulong cpu_pc_deb)
 {
-    target_ulong prev_priv, prev_virt, mstatus;
+    target_ulong prev_priv, prev_virt;
+    uint64_t mstatus;
 
     if (!(env->priv >= PRV_S)) {
         riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
@@ -147,18 +148,14 @@ target_ulong helper_mret(CPURISCVState *env, target_ulong cpu_pc_deb)
         riscv_raise_exception(env, RISCV_EXCP_INST_ADDR_MIS, GETPC());
     }
 
-    target_ulong mstatus = env->mstatus;
+    uint64_t mstatus = env->mstatus;
     target_ulong prev_priv = get_field(mstatus, MSTATUS_MPP);
-    target_ulong prev_virt = MSTATUS_MPV_ISSET(env);
+    target_ulong prev_virt = !!get_field(env->mstatus, MSTATUS_MPV);
     mstatus = set_field(mstatus, MSTATUS_MIE,
                         get_field(mstatus, MSTATUS_MPIE));
     mstatus = set_field(mstatus, MSTATUS_MPIE, 1);
     mstatus = set_field(mstatus, MSTATUS_MPP, PRV_U);
-#ifdef TARGET_RISCV32
-    env->mstatush = set_field(env->mstatush, MSTATUS_MPV, 0);
-#else
     mstatus = set_field(mstatus, MSTATUS_MPV, 0);
-#endif
     env->mstatus = mstatus;
     riscv_cpu_set_mode(env, prev_priv);
 
-- 
2.28.0

Re: [PATCH v1 16/16] target/riscv: Consolidate *statush registers

[Richard Henderson]

On 10/23/20 8:33 AM, Alistair Francis wrote:
> +        qemu_fprintf(f, " %s %016lx\n", "vsstatus ", env->vsstatus);

You need PRIx64 here.


r~

Re: [PATCH v1 16/16] target/riscv: Consolidate *statush registers

[Bin Meng]

On Fri, Oct 23, 2020 at 11:45 PM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/cpu.h        | 24 ++++++++++++++---------
>  target/riscv/cpu_bits.h   | 14 ++-----------
>  target/riscv/cpu.c        |  7 ++-----
>  target/riscv/cpu_helper.c | 41 ++++++++++-----------------------------
>  target/riscv/csr.c        | 15 ++++++++------
>  target/riscv/op_helper.c  | 11 ++++-------
>  6 files changed, 42 insertions(+), 70 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 8bde15544d..de864a947b 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -150,9 +150,11 @@ struct CPURISCVState {
>      target_ulong resetvec;
>
>      target_ulong mhartid;
> -    target_ulong mstatus;
> -    /* This is RV32 only */
> -    target_ulong mstatush;
> +    /*
> +     * For RV32 this is 32-bit mstatus and 32-bit mstatush.
> +     * For RV64 this is a 64-bit mstatus.
> +     */
> +    uint64_t mstatus;
>
>      target_ulong mip;
>
> @@ -187,15 +189,17 @@ struct CPURISCVState {
>      uint64_t htimedelta;
>
>      /* Virtual CSRs */
> -    target_ulong vsstatus;
>      target_ulong vstvec;
>      target_ulong vsscratch;
>      target_ulong vsepc;
>      target_ulong vscause;
>      target_ulong vstval;
>      target_ulong vsatp;
> -    /* This is RV32 only */
> -    target_ulong vsstatush;
> +    /*
> +     * For RV32 this is 32-bit mstatus and 32-bit mstatush.
> +     * For RV64 this is a 64-bit mstatus.
> +     */
> +    uint64_t vsstatus;
>
>      target_ulong mtval2;
>      target_ulong mtinst;
> @@ -207,9 +211,11 @@ struct CPURISCVState {
>      target_ulong scause_hs;
>      target_ulong stval_hs;
>      target_ulong satp_hs;
> -    target_ulong mstatus_hs;
> -    /* This is RV32 only */
> -    target_ulong mstatush_hs;
> +    /*
> +     * For RV32 this is 32-bit mstatus and 32-bit mstatush.
> +     * For RV64 this is a 64-bit mstatus.
> +     */
> +    uint64_t mstatus_hs;
>
>      target_ulong scounteren;
>      target_ulong mcounteren;
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index db46739b1c..4b75d2766e 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -381,19 +381,9 @@
>  #define MSTATUS_TVM         0x00100000 /* since: priv-1.10 */
>  #define MSTATUS_TW          0x20000000 /* since: priv-1.10 */
>  #define MSTATUS_TSR         0x40000000 /* since: priv-1.10 */
> -#if defined(TARGET_RISCV64)
> -#define MSTATUS_GVA         0x4000000000ULL
> -#define MSTATUS_MPV         0x8000000000ULL
> -#elif defined(TARGET_RISCV32)
> -#define MSTATUS_GVA         0x00000040
> -#define MSTATUS_MPV         0x00000080
> -#endif
>
> -#ifdef TARGET_RISCV32
> -# define MSTATUS_MPV_ISSET(env)  get_field(env->mstatush, MSTATUS_MPV)
> -#else
> -# define MSTATUS_MPV_ISSET(env)  get_field(env->mstatus, MSTATUS_MPV)
> -#endif
> +#define MSTATUS_GVA      0x4000000000ULL
> +#define MSTATUS_MPV      0x8000000000ULL

nits: the indentation level is not aligned to other macros around

>
>  #define MSTATUS64_UXL       0x0000000300000000ULL
>  #define MSTATUS64_SXL       0x0000000C00000000ULL
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index c77687ccb2..74c5ac5777 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -245,13 +245,10 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int flags)
>      qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "pc      ", env->pc);
>  #ifndef CONFIG_USER_ONLY
>      qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mhartid ", env->mhartid);
> -    qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mstatus ", env->mstatus);
> -    if (riscv_cpu_is_32bit(env)) {
> -        qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mstatush ", env->mstatush);
> -    }
> +    qemu_fprintf(f, " %s %016lx\n", "mstatus ", env->mstatus);
>      if (riscv_has_ext(env, RVH)) {
>          qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "hstatus ", env->hstatus);
> -        qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "vsstatus ", env->vsstatus);
> +        qemu_fprintf(f, " %s %016lx\n", "vsstatus ", env->vsstatus);
>      }
>      qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mip     ", env->mip);
>      qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mie     ", env->mie);
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 62aed24feb..b109529ace 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -110,27 +110,19 @@ bool riscv_cpu_fp_enabled(CPURISCVState *env)
>
>  void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env)
>  {
> -    target_ulong mstatus_mask = MSTATUS_MXR | MSTATUS_SUM | MSTATUS_FS |
> -                                MSTATUS_SPP | MSTATUS_SPIE | MSTATUS_SIE;
> +    uint64_t mstatus_mask = MSTATUS_MXR | MSTATUS_SUM | MSTATUS_FS |
> +                            MSTATUS_SPP | MSTATUS_SPIE | MSTATUS_SIE |
> +                            MSTATUS64_UXL;
>      bool current_virt = riscv_cpu_virt_enabled(env);
>
>      g_assert(riscv_has_ext(env, RVH));
>
> -#if defined(TARGET_RISCV64)
> -    mstatus_mask |= MSTATUS64_UXL;
> -#endif
> -
>      if (current_virt) {
>          /* Current V=1 and we are about to change to V=0 */
>          env->vsstatus = env->mstatus & mstatus_mask;
>          env->mstatus &= ~mstatus_mask;
>          env->mstatus |= env->mstatus_hs;
>
> -        if (riscv_cpu_is_32bit(env)) {
> -            env->vsstatush = env->mstatush;
> -            env->mstatush |= env->mstatush_hs;
> -        }
> -
>          env->vstvec = env->stvec;
>          env->stvec = env->stvec_hs;
>
> @@ -154,11 +146,6 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env)
>          env->mstatus &= ~mstatus_mask;
>          env->mstatus |= env->vsstatus;
>
> -        if (riscv_cpu_is_32bit(env)) {
> -            env->mstatush_hs = env->mstatush;
> -            env->mstatush |= env->vsstatush;
> -        }
> -
>          env->stvec_hs = env->stvec;
>          env->stvec = env->vstvec;
>
> @@ -729,7 +716,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>      if (riscv_has_ext(env, RVH) && env->priv == PRV_M &&
>          access_type != MMU_INST_FETCH &&
>          get_field(env->mstatus, MSTATUS_MPRV) &&
> -        MSTATUS_MPV_ISSET(env)) {
> +        get_field(env->mstatus, MSTATUS_MPV)) {
>          riscv_cpu_set_two_stage_lookup(env, true);
>      }
>
> @@ -801,7 +788,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>      if (riscv_has_ext(env, RVH) && env->priv == PRV_M &&
>          access_type != MMU_INST_FETCH &&
>          get_field(env->mstatus, MSTATUS_MPRV) &&
> -        MSTATUS_MPV_ISSET(env)) {
> +        get_field(env->mstatus, MSTATUS_MPV)) {
>          riscv_cpu_set_two_stage_lookup(env, false);
>      }
>
> @@ -864,7 +851,7 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>      RISCVCPU *cpu = RISCV_CPU(cs);
>      CPURISCVState *env = &cpu->env;
>      bool force_hs_execp = riscv_cpu_force_hs_excep_enabled(env);
> -    target_ulong s;
> +    uint64_t s;
>
>      /* cs->exception is 32-bits wide unlike mcause which is XLEN-bits wide
>       * so we mask off the MSB and separate into trap type and cause.
> @@ -997,18 +984,10 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>              if (riscv_cpu_virt_enabled(env)) {
>                  riscv_cpu_swap_hypervisor_regs(env);
>              }
> -            if (riscv_cpu_is_32bit(env)) {
> -                env->mstatush = set_field(env->mstatush, MSTATUS_MPV,
> -                                           riscv_cpu_virt_enabled(env));
> -                if (riscv_cpu_virt_enabled(env) && tval) {
> -                    env->mstatush = set_field(env->mstatush, MSTATUS_GVA, 1);
> -                }
> -            } else {
> -                env->mstatus = set_field(env->mstatus, MSTATUS_MPV,
> -                                          riscv_cpu_virt_enabled(env));
> -                if (riscv_cpu_virt_enabled(env) && tval) {
> -                    env->mstatus = set_field(env->mstatus, MSTATUS_GVA, 1);
> -                }
> +            env->mstatus = set_field(env->mstatus, MSTATUS_MPV,
> +                                     riscv_cpu_virt_enabled(env));
> +            if (riscv_cpu_virt_enabled(env) && tval) {
> +                env->mstatus = set_field(env->mstatus, MSTATUS_GVA, 1);
>              }
>
>              mtval2 = env->guest_phys_fault_addr;
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index b153cdfec3..7132a9bcd0 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -473,8 +473,8 @@ static int validate_vm(CPURISCVState *env, target_ulong vm)
>
>  static int write_mstatus(CPURISCVState *env, int csrno, target_ulong val)
>  {
> -    target_ulong mstatus = env->mstatus;
> -    target_ulong mask = 0;
> +    uint64_t mstatus = env->mstatus;
> +    uint64_t mask = 0;
>      int dirty;
>
>      /* flush tlb on mstatus fields that affect VM */
> @@ -507,19 +507,22 @@ static int write_mstatus(CPURISCVState *env, int csrno, target_ulong val)
>
>  static int read_mstatush(CPURISCVState *env, int csrno, target_ulong *val)
>  {
> -    *val = env->mstatush;
> +    *val = env->mstatus >> 32;
>      return 0;
>  }
>
>  static int write_mstatush(CPURISCVState *env, int csrno, target_ulong val)
>  {
> -    if ((val ^ env->mstatush) & (MSTATUS_MPV)) {
> +    uint64_t val64 = (uint64_t) val << 32;
> +
> +    if ((val64 ^ env->mstatus) & (MSTATUS_MPV)) {
>          tlb_flush(env_cpu(env));
>      }
>
> -    val &= MSTATUS_MPV | MSTATUS_GVA;
> +    val64 &= MSTATUS_MPV | MSTATUS_GVA;
>
> -    env->mstatush = val;
> +    env->mstatus &= 0xFFFFFFFF;
> +    env->mstatus |= val64;
>
>      return 0;
>  }
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index 4ce73575a7..f9361e3af8 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -78,7 +78,8 @@ target_ulong helper_csrrc(CPURISCVState *env, target_ulong src,
>
>  target_ulong helper_sret(CPURISCVState *env, target_ulong cpu_pc_deb)
>  {
> -    target_ulong prev_priv, prev_virt, mstatus;
> +    target_ulong prev_priv, prev_virt;
> +    uint64_t mstatus;
>
>      if (!(env->priv >= PRV_S)) {
>          riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
> @@ -147,18 +148,14 @@ target_ulong helper_mret(CPURISCVState *env, target_ulong cpu_pc_deb)
>          riscv_raise_exception(env, RISCV_EXCP_INST_ADDR_MIS, GETPC());
>      }
>
> -    target_ulong mstatus = env->mstatus;
> +    uint64_t mstatus = env->mstatus;
>      target_ulong prev_priv = get_field(mstatus, MSTATUS_MPP);
> -    target_ulong prev_virt = MSTATUS_MPV_ISSET(env);
> +    target_ulong prev_virt = !!get_field(env->mstatus, MSTATUS_MPV);
>      mstatus = set_field(mstatus, MSTATUS_MIE,
>                          get_field(mstatus, MSTATUS_MPIE));
>      mstatus = set_field(mstatus, MSTATUS_MPIE, 1);
>      mstatus = set_field(mstatus, MSTATUS_MPP, PRV_U);
> -#ifdef TARGET_RISCV32
> -    env->mstatush = set_field(env->mstatush, MSTATUS_MPV, 0);
> -#else
>      mstatus = set_field(mstatus, MSTATUS_MPV, 0);
> -#endif
>      env->mstatus = mstatus;
>      riscv_cpu_set_mode(env, prev_priv);
>

Regards,
Bin

Re: [PATCH v1 00/16] RISC-V: Start to remove xlen preprocess

[Bin Meng]

Hi Alistair,

On Fri, Oct 23, 2020 at 11:44 PM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> The RISC-V QEMU port currently has lot of preprocessor directives that
> check if we are targetting a 32-bit or 64-bit CPU. This means that the
> 64-bit RISC-V target can not run 32-bit CPUs. This is different to most
> other QEMU architectures and doesn't allow us to mix xlens (such as when
> running Hypervisors with different xlen guests).
>
> This series is a step toward removing some of those to allow us to use
> 32-bit CPUs on 64-bit RISC-V targets.
>
> Alistair Francis (16):
>   target/riscv: Add a TYPE_RISCV_CPU_BASE CPU
>   riscv: spike: Remove target macro conditionals
>   riscv: virt: Remove target macro conditionals
>   hw/riscv: boot: Remove compile time XLEN checks
>   hw/riscv: virt: Remove compile time XLEN checks
>   hw/riscv: spike: Remove compile time XLEN checks
>   hw/riscv: sifive_u: Remove compile time XLEN checks
>   target/riscv: fpu_helper: Match function defs in HELPER macros
>   target/riscv: Add a riscv_cpu_is_32bit() helper function
>   target/riscv: Specify the XLEN for CPUs
>   target/riscv: cpu: Remove compile time XLEN checks
>   target/riscv: cpu_helper: Remove compile time XLEN checks
>   target/riscv: csr: Remove compile time XLEN checks
>   target/riscv: cpu: Set XLEN independently from target
>   target/riscv: Convert the get/set_field() to support 64-bit values
>   target/riscv: Consolidate *statush registers
>
>  include/hw/riscv/boot.h   |   8 +-
>  include/hw/riscv/spike.h  |   6 --
>  include/hw/riscv/virt.h   |   6 --
>  target/riscv/cpu.h        |  36 ++++---
>  target/riscv/cpu_bits.h   |  24 ++---
>  hw/riscv/boot.c           |  55 ++++++-----
>  hw/riscv/sifive_u.c       |  59 ++++++------
>  hw/riscv/spike.c          |  50 +++++-----
>  hw/riscv/virt.c           |  36 +++----
>  target/riscv/cpu.c        |  83 ++++++++++------
>  target/riscv/cpu_helper.c |  47 +++------
>  target/riscv/csr.c        | 197 ++++++++++++++++++++------------------
>  target/riscv/fpu_helper.c |  16 ++--
>  target/riscv/op_helper.c  |  11 +--
>  14 files changed, 328 insertions(+), 306 deletions(-)

Test result:

64-bit virt & sifive_u, boots OpenSBI and kernel.
32-bit virt boots OpenSBI and kernel. 32-bit sifive_u does not boot OpenSBI.

Regards,
Bin

Re: [PATCH v1 00/16] RISC-V: Start to remove xlen preprocess

[Alistair Francis]

On Mon, Oct 26, 2020 at 1:55 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Alistair,
>
> On Fri, Oct 23, 2020 at 11:44 PM Alistair Francis
> <alistair.francis@wdc.com> wrote:
> >
> > The RISC-V QEMU port currently has lot of preprocessor directives that
> > check if we are targetting a 32-bit or 64-bit CPU. This means that the
> > 64-bit RISC-V target can not run 32-bit CPUs. This is different to most
> > other QEMU architectures and doesn't allow us to mix xlens (such as when
> > running Hypervisors with different xlen guests).
> >
> > This series is a step toward removing some of those to allow us to use
> > 32-bit CPUs on 64-bit RISC-V targets.
> >
> > Alistair Francis (16):
> >   target/riscv: Add a TYPE_RISCV_CPU_BASE CPU
> >   riscv: spike: Remove target macro conditionals
> >   riscv: virt: Remove target macro conditionals
> >   hw/riscv: boot: Remove compile time XLEN checks
> >   hw/riscv: virt: Remove compile time XLEN checks
> >   hw/riscv: spike: Remove compile time XLEN checks
> >   hw/riscv: sifive_u: Remove compile time XLEN checks
> >   target/riscv: fpu_helper: Match function defs in HELPER macros
> >   target/riscv: Add a riscv_cpu_is_32bit() helper function
> >   target/riscv: Specify the XLEN for CPUs
> >   target/riscv: cpu: Remove compile time XLEN checks
> >   target/riscv: cpu_helper: Remove compile time XLEN checks
> >   target/riscv: csr: Remove compile time XLEN checks
> >   target/riscv: cpu: Set XLEN independently from target
> >   target/riscv: Convert the get/set_field() to support 64-bit values
> >   target/riscv: Consolidate *statush registers
> >
> >  include/hw/riscv/boot.h   |   8 +-
> >  include/hw/riscv/spike.h  |   6 --
> >  include/hw/riscv/virt.h   |   6 --
> >  target/riscv/cpu.h        |  36 ++++---
> >  target/riscv/cpu_bits.h   |  24 ++---
> >  hw/riscv/boot.c           |  55 ++++++-----
> >  hw/riscv/sifive_u.c       |  59 ++++++------
> >  hw/riscv/spike.c          |  50 +++++-----
> >  hw/riscv/virt.c           |  36 +++----
> >  target/riscv/cpu.c        |  83 ++++++++++------
> >  target/riscv/cpu_helper.c |  47 +++------
> >  target/riscv/csr.c        | 197 ++++++++++++++++++++------------------
> >  target/riscv/fpu_helper.c |  16 ++--
> >  target/riscv/op_helper.c  |  11 +--
> >  14 files changed, 328 insertions(+), 306 deletions(-)
>
> Test result:
>
> 64-bit virt & sifive_u, boots OpenSBI and kernel.
> 32-bit virt boots OpenSBI and kernel. 32-bit sifive_u does not boot OpenSBI.

Thanks, I have fixed the failure and added this to my automated test cases.

Alistair

>
> Regards,
> Bin