[PATCH v2 0/3] RTISC-V: Remove deprecated ISA, CPUs and machines

[PATCH v2 0/3] RTISC-V: Remove deprecated ISA, CPUs and machines

[Alistair Francis]

v2:
 - Remove the CPUs and ISA seperatley

Alistair Francis (3):
  hw/riscv: spike: Remove deprecated ISA specific machines
  target/riscv: Remove the deprecated CPUs
  target/riscv: Drop support for ISA spec version 1.09.1

 hw/riscv/spike.c                              | 217 ------------------
 include/hw/riscv/spike.h                      |   6 +-
 target/riscv/cpu.c                            |  30 ---
 target/riscv/cpu.h                            |   8 -
 target/riscv/csr.c                            |  82 ++-----
 .../riscv/insn_trans/trans_privileged.inc.c   |   6 -
 tests/qtest/machine-none-test.c               |   4 +-
 7 files changed, 21 insertions(+), 332 deletions(-)

-- 
2.26.2

[PATCH v2 1/3] hw/riscv: spike: Remove deprecated ISA specific machines

[Alistair Francis]

The ISA specific Spike machines have  been deprecated in QEMU since 4.1,
let's finally remove them.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/riscv/spike.c         | 217 ---------------------------------------
 include/hw/riscv/spike.h |   6 +-
 2 files changed, 2 insertions(+), 221 deletions(-)

diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index d0c4843712..7bbbdb5036 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -257,221 +257,6 @@ static void spike_board_init(MachineState *machine)
         false);
 }
 
-static void spike_v1_10_0_board_init(MachineState *machine)
-{
-    const struct MemmapEntry *memmap = spike_memmap;
-
-    SpikeState *s = g_new0(SpikeState, 1);
-    MemoryRegion *system_memory = get_system_memory();
-    MemoryRegion *main_mem = g_new(MemoryRegion, 1);
-    MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
-    int i;
-    unsigned int smp_cpus = machine->smp.cpus;
-
-    if (!qtest_enabled()) {
-        info_report("The Spike v1.10.0 machine has been deprecated. "
-                    "Please use the generic spike machine and specify the ISA "
-                    "versions using -cpu.");
-    }
-
-    /* Initialize SOC */
-    object_initialize_child(OBJECT(machine), "soc", &s->soc, sizeof(s->soc),
-                            TYPE_RISCV_HART_ARRAY, &error_abort, NULL);
-    object_property_set_str(OBJECT(&s->soc), SPIKE_V1_10_0_CPU, "cpu-type",
-                            &error_abort);
-    object_property_set_int(OBJECT(&s->soc), smp_cpus, "num-harts",
-                            &error_abort);
-    object_property_set_bool(OBJECT(&s->soc), true, "realized",
-                            &error_abort);
-
-    /* register system main memory (actual RAM) */
-    memory_region_init_ram(main_mem, NULL, "riscv.spike.ram",
-                           machine->ram_size, &error_fatal);
-    memory_region_add_subregion(system_memory, memmap[SPIKE_DRAM].base,
-        main_mem);
-
-    /* create device tree */
-    create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline);
-
-    /* boot rom */
-    memory_region_init_rom(mask_rom, NULL, "riscv.spike.mrom",
-                           memmap[SPIKE_MROM].size, &error_fatal);
-    memory_region_add_subregion(system_memory, memmap[SPIKE_MROM].base,
-                                mask_rom);
-
-    if (machine->kernel_filename) {
-        riscv_load_kernel(machine->kernel_filename, htif_symbol_callback);
-    }
-
-    /* reset vector */
-    uint32_t reset_vec[8] = {
-        0x00000297,                  /* 1:  auipc  t0, %pcrel_hi(dtb) */
-        0x02028593,                  /*     addi   a1, t0, %pcrel_lo(1b) */
-        0xf1402573,                  /*     csrr   a0, mhartid  */
-#if defined(TARGET_RISCV32)
-        0x0182a283,                  /*     lw     t0, 24(t0) */
-#elif defined(TARGET_RISCV64)
-        0x0182b283,                  /*     ld     t0, 24(t0) */
-#endif
-        0x00028067,                  /*     jr     t0 */
-        0x00000000,
-        memmap[SPIKE_DRAM].base,     /* start: .dword DRAM_BASE */
-        0x00000000,
-                                     /* dtb: */
-    };
-
-    /* copy in the reset vector in little_endian byte order */
-    for (i = 0; i < sizeof(reset_vec) >> 2; i++) {
-        reset_vec[i] = cpu_to_le32(reset_vec[i]);
-    }
-    rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
-                          memmap[SPIKE_MROM].base, &address_space_memory);
-
-    /* copy in the device tree */
-    if (fdt_pack(s->fdt) || fdt_totalsize(s->fdt) >
-            memmap[SPIKE_MROM].size - sizeof(reset_vec)) {
-        error_report("not enough space to store device-tree");
-        exit(1);
-    }
-    qemu_fdt_dumpdtb(s->fdt, fdt_totalsize(s->fdt));
-    rom_add_blob_fixed_as("mrom.fdt", s->fdt, fdt_totalsize(s->fdt),
-                          memmap[SPIKE_MROM].base + sizeof(reset_vec),
-                          &address_space_memory);
-
-    /* initialize HTIF using symbols found in load_kernel */
-    htif_mm_init(system_memory, mask_rom, &s->soc.harts[0].env, serial_hd(0));
-
-    /* Core Local Interruptor (timer and IPI) */
-    sifive_clint_create(memmap[SPIKE_CLINT].base, memmap[SPIKE_CLINT].size,
-        smp_cpus, SIFIVE_SIP_BASE, SIFIVE_TIMECMP_BASE, SIFIVE_TIME_BASE,
-        false);
-}
-
-static void spike_v1_09_1_board_init(MachineState *machine)
-{
-    const struct MemmapEntry *memmap = spike_memmap;
-
-    SpikeState *s = g_new0(SpikeState, 1);
-    MemoryRegion *system_memory = get_system_memory();
-    MemoryRegion *main_mem = g_new(MemoryRegion, 1);
-    MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
-    int i;
-    unsigned int smp_cpus = machine->smp.cpus;
-
-    if (!qtest_enabled()) {
-        info_report("The Spike v1.09.1 machine has been deprecated. "
-                    "Please use the generic spike machine and specify the ISA "
-                    "versions using -cpu.");
-    }
-
-    /* Initialize SOC */
-    object_initialize_child(OBJECT(machine), "soc", &s->soc, sizeof(s->soc),
-                            TYPE_RISCV_HART_ARRAY, &error_abort, NULL);
-    object_property_set_str(OBJECT(&s->soc), SPIKE_V1_09_1_CPU, "cpu-type",
-                            &error_abort);
-    object_property_set_int(OBJECT(&s->soc), smp_cpus, "num-harts",
-                            &error_abort);
-    object_property_set_bool(OBJECT(&s->soc), true, "realized",
-                            &error_abort);
-
-    /* register system main memory (actual RAM) */
-    memory_region_init_ram(main_mem, NULL, "riscv.spike.ram",
-                           machine->ram_size, &error_fatal);
-    memory_region_add_subregion(system_memory, memmap[SPIKE_DRAM].base,
-        main_mem);
-
-    /* boot rom */
-    memory_region_init_rom(mask_rom, NULL, "riscv.spike.mrom",
-                           memmap[SPIKE_MROM].size, &error_fatal);
-    memory_region_add_subregion(system_memory, memmap[SPIKE_MROM].base,
-                                mask_rom);
-
-    if (machine->kernel_filename) {
-        riscv_load_kernel(machine->kernel_filename, htif_symbol_callback);
-    }
-
-    /* reset vector */
-    uint32_t reset_vec[8] = {
-        0x297 + memmap[SPIKE_DRAM].base - memmap[SPIKE_MROM].base, /* lui */
-        0x00028067,                   /* jump to DRAM_BASE */
-        0x00000000,                   /* reserved */
-        memmap[SPIKE_MROM].base + sizeof(reset_vec), /* config string pointer */
-        0, 0, 0, 0                    /* trap vector */
-    };
-
-    /* part one of config string - before memory size specified */
-    const char *config_string_tmpl =
-        "platform {\n"
-        "  vendor ucb;\n"
-        "  arch spike;\n"
-        "};\n"
-        "rtc {\n"
-        "  addr 0x%" PRIx64 "x;\n"
-        "};\n"
-        "ram {\n"
-        "  0 {\n"
-        "    addr 0x%" PRIx64 "x;\n"
-        "    size 0x%" PRIx64 "x;\n"
-        "  };\n"
-        "};\n"
-        "core {\n"
-        "  0" " {\n"
-        "    " "0 {\n"
-        "      isa %s;\n"
-        "      timecmp 0x%" PRIx64 "x;\n"
-        "      ipi 0x%" PRIx64 "x;\n"
-        "    };\n"
-        "  };\n"
-        "};\n";
-
-    /* build config string with supplied memory size */
-    char *isa = riscv_isa_string(&s->soc.harts[0]);
-    char *config_string = g_strdup_printf(config_string_tmpl,
-        (uint64_t)memmap[SPIKE_CLINT].base + SIFIVE_TIME_BASE,
-        (uint64_t)memmap[SPIKE_DRAM].base,
-        (uint64_t)ram_size, isa,
-        (uint64_t)memmap[SPIKE_CLINT].base + SIFIVE_TIMECMP_BASE,
-        (uint64_t)memmap[SPIKE_CLINT].base + SIFIVE_SIP_BASE);
-    g_free(isa);
-    size_t config_string_len = strlen(config_string);
-
-    /* copy in the reset vector in little_endian byte order */
-    for (i = 0; i < sizeof(reset_vec) >> 2; i++) {
-        reset_vec[i] = cpu_to_le32(reset_vec[i]);
-    }
-    rom_add_blob_fixed_as("mrom.reset", reset_vec, sizeof(reset_vec),
-                          memmap[SPIKE_MROM].base, &address_space_memory);
-
-    /* copy in the config string */
-    rom_add_blob_fixed_as("mrom.reset", config_string, config_string_len,
-                          memmap[SPIKE_MROM].base + sizeof(reset_vec),
-                          &address_space_memory);
-
-    /* initialize HTIF using symbols found in load_kernel */
-    htif_mm_init(system_memory, mask_rom, &s->soc.harts[0].env, serial_hd(0));
-
-    /* Core Local Interruptor (timer and IPI) */
-    sifive_clint_create(memmap[SPIKE_CLINT].base, memmap[SPIKE_CLINT].size,
-        smp_cpus, SIFIVE_SIP_BASE, SIFIVE_TIMECMP_BASE, SIFIVE_TIME_BASE,
-        false);
-
-    g_free(config_string);
-}
-
-static void spike_v1_09_1_machine_init(MachineClass *mc)
-{
-    mc->desc = "RISC-V Spike Board (Privileged ISA v1.9.1)";
-    mc->init = spike_v1_09_1_board_init;
-    mc->max_cpus = 1;
-}
-
-static void spike_v1_10_0_machine_init(MachineClass *mc)
-{
-    mc->desc = "RISC-V Spike Board (Privileged ISA v1.10)";
-    mc->init = spike_v1_10_0_board_init;
-    mc->max_cpus = 1;
-}
-
 static void spike_machine_init(MachineClass *mc)
 {
     mc->desc = "RISC-V Spike Board";
@@ -481,6 +266,4 @@ static void spike_machine_init(MachineClass *mc)
     mc->default_cpu_type = SPIKE_V1_10_0_CPU;
 }
 
-DEFINE_MACHINE("spike_v1.9.1", spike_v1_09_1_machine_init)
-DEFINE_MACHINE("spike_v1.10", spike_v1_10_0_machine_init)
 DEFINE_MACHINE("spike", spike_machine_init)
diff --git a/include/hw/riscv/spike.h b/include/hw/riscv/spike.h
index dc770421bc..b98cfea0e4 100644
--- a/include/hw/riscv/spike.h
+++ b/include/hw/riscv/spike.h
@@ -39,11 +39,9 @@ enum {
 };
 
 #if defined(TARGET_RISCV32)
-#define SPIKE_V1_09_1_CPU TYPE_RISCV_CPU_RV32GCSU_V1_09_1
-#define SPIKE_V1_10_0_CPU TYPE_RISCV_CPU_RV32GCSU_V1_10_0
+#define SPIKE_V1_10_0_CPU TYPE_RISCV_CPU_SIFIVE_U34
 #elif defined(TARGET_RISCV64)
-#define SPIKE_V1_09_1_CPU TYPE_RISCV_CPU_RV64GCSU_V1_09_1
-#define SPIKE_V1_10_0_CPU TYPE_RISCV_CPU_RV64GCSU_V1_10_0
+#define SPIKE_V1_10_0_CPU TYPE_RISCV_CPU_SIFIVE_U54
 #endif
 
 #endif
-- 
2.26.2

[PATCH v2 2/3] target/riscv: Remove the deprecated CPUs

[Alistair Francis]

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu.c              | 28 ----------------------------
 target/riscv/cpu.h              |  7 -------
 tests/qtest/machine-none-test.c |  4 ++--
 3 files changed, 2 insertions(+), 37 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 059d71f2c7..112f2e3a2f 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -135,16 +135,6 @@ static void riscv_base32_cpu_init(Object *obj)
     set_misa(env, 0);
 }
 
-static void rv32gcsu_priv1_09_1_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_09_1);
-    set_resetvec(env, DEFAULT_RSTVEC);
-    set_feature(env, RISCV_FEATURE_MMU);
-    set_feature(env, RISCV_FEATURE_PMP);
-}
-
 static void rv32gcsu_priv1_10_0_cpu_init(Object *obj)
 {
     CPURISCVState *env = &RISCV_CPU(obj)->env;
@@ -182,16 +172,6 @@ static void riscv_base64_cpu_init(Object *obj)
     set_misa(env, 0);
 }
 
-static void rv64gcsu_priv1_09_1_cpu_init(Object *obj)
-{
-    CPURISCVState *env = &RISCV_CPU(obj)->env;
-    set_misa(env, RV64 | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
-    set_priv_version(env, PRIV_VERSION_1_09_1);
-    set_resetvec(env, DEFAULT_RSTVEC);
-    set_feature(env, RISCV_FEATURE_MMU);
-    set_feature(env, RISCV_FEATURE_PMP);
-}
-
 static void rv64gcsu_priv1_10_0_cpu_init(Object *obj)
 {
     CPURISCVState *env = &RISCV_CPU(obj)->env;
@@ -621,18 +601,10 @@ static const TypeInfo riscv_cpu_type_infos[] = {
     DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E31,       rv32imacu_nommu_cpu_init),
     DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E34,       rv32imafcu_nommu_cpu_init),
     DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U34,       rv32gcsu_priv1_10_0_cpu_init),
-    /* Depreacted */
-    DEFINE_CPU(TYPE_RISCV_CPU_RV32IMACU_NOMMU,  rv32imacu_nommu_cpu_init),
-    DEFINE_CPU(TYPE_RISCV_CPU_RV32GCSU_V1_09_1, rv32gcsu_priv1_09_1_cpu_init),
-    DEFINE_CPU(TYPE_RISCV_CPU_RV32GCSU_V1_10_0, rv32gcsu_priv1_10_0_cpu_init)
 #elif defined(TARGET_RISCV64)
     DEFINE_CPU(TYPE_RISCV_CPU_BASE64,           riscv_base64_cpu_init),
     DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_E51,       rv64imacu_nommu_cpu_init),
     DEFINE_CPU(TYPE_RISCV_CPU_SIFIVE_U54,       rv64gcsu_priv1_10_0_cpu_init),
-    /* Deprecated */
-    DEFINE_CPU(TYPE_RISCV_CPU_RV64IMACU_NOMMU,  rv64imacu_nommu_cpu_init),
-    DEFINE_CPU(TYPE_RISCV_CPU_RV64GCSU_V1_09_1, rv64gcsu_priv1_09_1_cpu_init),
-    DEFINE_CPU(TYPE_RISCV_CPU_RV64GCSU_V1_10_0, rv64gcsu_priv1_10_0_cpu_init)
 #endif
 };
 
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index d0e7f5b9c5..76b98d7a33 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -40,13 +40,6 @@
 #define TYPE_RISCV_CPU_SIFIVE_E51       RISCV_CPU_TYPE_NAME("sifive-e51")
 #define TYPE_RISCV_CPU_SIFIVE_U34       RISCV_CPU_TYPE_NAME("sifive-u34")
 #define TYPE_RISCV_CPU_SIFIVE_U54       RISCV_CPU_TYPE_NAME("sifive-u54")
-/* Deprecated */
-#define TYPE_RISCV_CPU_RV32IMACU_NOMMU  RISCV_CPU_TYPE_NAME("rv32imacu-nommu")
-#define TYPE_RISCV_CPU_RV32GCSU_V1_09_1 RISCV_CPU_TYPE_NAME("rv32gcsu-v1.9.1")
-#define TYPE_RISCV_CPU_RV32GCSU_V1_10_0 RISCV_CPU_TYPE_NAME("rv32gcsu-v1.10.0")
-#define TYPE_RISCV_CPU_RV64IMACU_NOMMU  RISCV_CPU_TYPE_NAME("rv64imacu-nommu")
-#define TYPE_RISCV_CPU_RV64GCSU_V1_09_1 RISCV_CPU_TYPE_NAME("rv64gcsu-v1.9.1")
-#define TYPE_RISCV_CPU_RV64GCSU_V1_10_0 RISCV_CPU_TYPE_NAME("rv64gcsu-v1.10.0")
 
 #define RV32 ((target_ulong)1 << (TARGET_LONG_BITS - 2))
 #define RV64 ((target_ulong)2 << (TARGET_LONG_BITS - 2))
diff --git a/tests/qtest/machine-none-test.c b/tests/qtest/machine-none-test.c
index 8bb54a6360..b52311ec2e 100644
--- a/tests/qtest/machine-none-test.c
+++ b/tests/qtest/machine-none-test.c
@@ -54,8 +54,8 @@ static struct arch2cpu cpus_map[] = {
     { "xtensa", "dc233c" },
     { "xtensaeb", "fsf" },
     { "hppa", "hppa" },
-    { "riscv64", "rv64gcsu-v1.10.0" },
-    { "riscv32", "rv32gcsu-v1.9.1" },
+    { "riscv64", "sifive-u54" },
+    { "riscv32", "sifive-u34" },
     { "rx", "rx62n" },
 };
 
-- 
2.26.2

[PATCH v2 3/3] target/riscv: Drop support for ISA spec version 1.09.1

[Alistair Francis]

The RISC-V ISA spec version 1.09.1 has been deprecated in QEMU since
4.1. It's not commonly used so let's remove support for it.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu.c                            |  2 -
 target/riscv/cpu.h                            |  1 -
 target/riscv/csr.c                            | 82 ++++---------------
 .../riscv/insn_trans/trans_privileged.inc.c   |  6 --
 4 files changed, 17 insertions(+), 74 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 112f2e3a2f..eeb91f8513 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -368,8 +368,6 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
             priv_version = PRIV_VERSION_1_11_0;
         } else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.10.0")) {
             priv_version = PRIV_VERSION_1_10_0;
-        } else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.9.1")) {
-            priv_version = PRIV_VERSION_1_09_1;
         } else {
             error_setg(errp,
                        "Unsupported privilege spec version '%s'",
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 76b98d7a33..c022539012 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -73,7 +73,6 @@ enum {
     RISCV_FEATURE_MISA
 };
 
-#define PRIV_VERSION_1_09_1 0x00010901
 #define PRIV_VERSION_1_10_0 0x00011000
 #define PRIV_VERSION_1_11_0 0x00011100
 
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 11d184cd16..df3498b24f 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -58,31 +58,11 @@ static int ctr(CPURISCVState *env, int csrno)
 #if !defined(CONFIG_USER_ONLY)
     CPUState *cs = env_cpu(env);
     RISCVCPU *cpu = RISCV_CPU(cs);
-    uint32_t ctr_en = ~0u;
 
     if (!cpu->cfg.ext_counters) {
         /* The Counters extensions is not enabled */
         return -1;
     }
-
-    /*
-     * The counters are always enabled at run time on newer priv specs, as the
-     * CSR has changed from controlling that the counters can be read to
-     * controlling that the counters increment.
-     */
-    if (env->priv_ver > PRIV_VERSION_1_09_1) {
-        return 0;
-    }
-
-    if (env->priv < PRV_M) {
-        ctr_en &= env->mcounteren;
-    }
-    if (env->priv < PRV_S) {
-        ctr_en &= env->scounteren;
-    }
-    if (!(ctr_en & (1u << (csrno & 31)))) {
-        return -1;
-    }
 #endif
     return 0;
 }
@@ -358,34 +338,21 @@ static int write_mstatus(CPURISCVState *env, int csrno, target_ulong val)
     int dirty;
 
     /* flush tlb on mstatus fields that affect VM */
-    if (env->priv_ver <= PRIV_VERSION_1_09_1) {
-        if ((val ^ mstatus) & (MSTATUS_MXR | MSTATUS_MPP |
-                MSTATUS_MPRV | MSTATUS_SUM | MSTATUS_VM)) {
-            tlb_flush(env_cpu(env));
-        }
-        mask = MSTATUS_SIE | MSTATUS_SPIE | MSTATUS_MIE | MSTATUS_MPIE |
-            MSTATUS_SPP | MSTATUS_FS | MSTATUS_MPRV | MSTATUS_SUM |
-            MSTATUS_MPP | MSTATUS_MXR |
-            (validate_vm(env, get_field(val, MSTATUS_VM)) ?
-                MSTATUS_VM : 0);
+    if ((val ^ mstatus) & (MSTATUS_MXR | MSTATUS_MPP | MSTATUS_MPV |
+            MSTATUS_MPRV | MSTATUS_SUM)) {
+        tlb_flush(env_cpu(env));
     }
-    if (env->priv_ver >= PRIV_VERSION_1_10_0) {
-        if ((val ^ mstatus) & (MSTATUS_MXR | MSTATUS_MPP | MSTATUS_MPV |
-                MSTATUS_MPRV | MSTATUS_SUM)) {
-            tlb_flush(env_cpu(env));
-        }
-        mask = MSTATUS_SIE | MSTATUS_SPIE | MSTATUS_MIE | MSTATUS_MPIE |
-            MSTATUS_SPP | MSTATUS_FS | MSTATUS_MPRV | MSTATUS_SUM |
-            MSTATUS_MPP | MSTATUS_MXR | MSTATUS_TVM | MSTATUS_TSR |
-            MSTATUS_TW;
+    mask = MSTATUS_SIE | MSTATUS_SPIE | MSTATUS_MIE | MSTATUS_MPIE |
+        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 MTL are not in mstatus. The current plan is to
-             * add them to mstatush. For now, we just don't support it.
-             */
-            mask |= MSTATUS_MTL | MSTATUS_MPV;
+        /*
+         * RV32: MPV and MTL are not in mstatus. The current plan is to
+         * add them to mstatush. For now, we just don't support it.
+         */
+        mask |= MSTATUS_MTL | MSTATUS_MPV;
 #endif
-    }
 
     mstatus = (mstatus & ~mask) | (val & mask);
 
@@ -553,8 +520,7 @@ static int write_mcounteren(CPURISCVState *env, int csrno, target_ulong val)
 /* This regiser is replaced with CSR_MCOUNTINHIBIT in 1.11.0 */
 static int read_mscounteren(CPURISCVState *env, int csrno, target_ulong *val)
 {
-    if (env->priv_ver > PRIV_VERSION_1_09_1
-        && env->priv_ver < PRIV_VERSION_1_11_0) {
+    if (env->priv_ver < PRIV_VERSION_1_11_0) {
         return -1;
     }
     *val = env->mcounteren;
@@ -564,8 +530,7 @@ static int read_mscounteren(CPURISCVState *env, int csrno, target_ulong *val)
 /* This regiser is replaced with CSR_MCOUNTINHIBIT in 1.11.0 */
 static int write_mscounteren(CPURISCVState *env, int csrno, target_ulong val)
 {
-    if (env->priv_ver > PRIV_VERSION_1_09_1
-        && env->priv_ver < PRIV_VERSION_1_11_0) {
+    if (env->priv_ver < PRIV_VERSION_1_11_0) {
         return -1;
     }
     env->mcounteren = val;
@@ -574,20 +539,13 @@ static int write_mscounteren(CPURISCVState *env, int csrno, target_ulong val)
 
 static int read_mucounteren(CPURISCVState *env, int csrno, target_ulong *val)
 {
-    if (env->priv_ver > PRIV_VERSION_1_09_1) {
-        return -1;
-    }
-    *val = env->scounteren;
+    return -1;
     return 0;
 }
 
 static int write_mucounteren(CPURISCVState *env, int csrno, target_ulong val)
 {
-    if (env->priv_ver > PRIV_VERSION_1_09_1) {
-        return -1;
-    }
-    env->scounteren = val;
-    return 0;
+    return -1;
 }
 
 /* Machine Trap Handling */
@@ -829,13 +787,7 @@ static int write_satp(CPURISCVState *env, int csrno, target_ulong val)
     if (!riscv_feature(env, RISCV_FEATURE_MMU)) {
         return 0;
     }
-    if (env->priv_ver <= PRIV_VERSION_1_09_1 && (val ^ env->sptbr)) {
-        tlb_flush(env_cpu(env));
-        env->sptbr = val & (((target_ulong)
-            1 << (TARGET_PHYS_ADDR_SPACE_BITS - PGSHIFT)) - 1);
-    }
-    if (env->priv_ver >= PRIV_VERSION_1_10_0 &&
-        validate_vm(env, get_field(val, SATP_MODE)) &&
+    if (validate_vm(env, get_field(val, SATP_MODE)) &&
         ((val ^ env->satp) & (SATP_MODE | SATP_ASID | SATP_PPN)))
     {
         if (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)) {
diff --git a/target/riscv/insn_trans/trans_privileged.inc.c b/target/riscv/insn_trans/trans_privileged.inc.c
index 76c2fad71c..1af9fa7df8 100644
--- a/target/riscv/insn_trans/trans_privileged.inc.c
+++ b/target/riscv/insn_trans/trans_privileged.inc.c
@@ -95,12 +95,6 @@ static bool trans_sfence_vma(DisasContext *ctx, arg_sfence_vma *a)
 
 static bool trans_sfence_vm(DisasContext *ctx, arg_sfence_vm *a)
 {
-#ifndef CONFIG_USER_ONLY
-    if (ctx->priv_ver <= PRIV_VERSION_1_09_1) {
-        gen_helper_tlb_flush(cpu_env);
-        return true;
-    }
-#endif
     return false;
 }
 
-- 
2.26.2

Re: [PATCH v2 0/3] RTISC-V: Remove deprecated ISA, CPUs and machines

[Alistair Francis]

On Thu, May 7, 2020 at 12:19 PM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> v2:
>  - Remove the CPUs and ISA seperatley
>
> Alistair Francis (3):
>   hw/riscv: spike: Remove deprecated ISA specific machines
>   target/riscv: Remove the deprecated CPUs
>   target/riscv: Drop support for ISA spec version 1.09.1

Any more comments?

Alistair

>
>  hw/riscv/spike.c                              | 217 ------------------
>  include/hw/riscv/spike.h                      |   6 +-
>  target/riscv/cpu.c                            |  30 ---
>  target/riscv/cpu.h                            |   8 -
>  target/riscv/csr.c                            |  82 ++-----
>  .../riscv/insn_trans/trans_privileged.inc.c   |   6 -
>  tests/qtest/machine-none-test.c               |   4 +-
>  7 files changed, 21 insertions(+), 332 deletions(-)
>
> --
> 2.26.2
>

Re: [PATCH v2 1/3] hw/riscv: spike: Remove deprecated ISA specific machines

[Bin Meng]

On Fri, May 8, 2020 at 3:21 AM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> The ISA specific Spike machines have  been deprecated in QEMU since 4.1,

nits: there are 2 spaces between 'have' and 'been'

> let's finally remove them.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/riscv/spike.c         | 217 ---------------------------------------
>  include/hw/riscv/spike.h |   6 +-
>  2 files changed, 2 insertions(+), 221 deletions(-)
>

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

Re: [PATCH v2 2/3] target/riscv: Remove the deprecated CPUs

[Bin Meng]

On Fri, May 8, 2020 at 3:19 AM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/cpu.c              | 28 ----------------------------
>  target/riscv/cpu.h              |  7 -------
>  tests/qtest/machine-none-test.c |  4 ++--
>  3 files changed, 2 insertions(+), 37 deletions(-)
>

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

Re: [PATCH v2 3/3] target/riscv: Drop support for ISA spec version 1.09.1

[Bin Meng]

On Fri, May 8, 2020 at 3:22 AM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> The RISC-V ISA spec version 1.09.1 has been deprecated in QEMU since
> 4.1. It's not commonly used so let's remove support for it.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/cpu.c                            |  2 -
>  target/riscv/cpu.h                            |  1 -
>  target/riscv/csr.c                            | 82 ++++---------------
>  .../riscv/insn_trans/trans_privileged.inc.c   |  6 --
>  4 files changed, 17 insertions(+), 74 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 112f2e3a2f..eeb91f8513 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -368,8 +368,6 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>              priv_version = PRIV_VERSION_1_11_0;
>          } else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.10.0")) {
>              priv_version = PRIV_VERSION_1_10_0;
> -        } else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.9.1")) {
> -            priv_version = PRIV_VERSION_1_09_1;
>          } else {
>              error_setg(errp,
>                         "Unsupported privilege spec version '%s'",
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 76b98d7a33..c022539012 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -73,7 +73,6 @@ enum {
>      RISCV_FEATURE_MISA
>  };
>
> -#define PRIV_VERSION_1_09_1 0x00010901
>  #define PRIV_VERSION_1_10_0 0x00011000
>  #define PRIV_VERSION_1_11_0 0x00011100
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 11d184cd16..df3498b24f 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -58,31 +58,11 @@ static int ctr(CPURISCVState *env, int csrno)
>  #if !defined(CONFIG_USER_ONLY)
>      CPUState *cs = env_cpu(env);
>      RISCVCPU *cpu = RISCV_CPU(cs);
> -    uint32_t ctr_en = ~0u;
>
>      if (!cpu->cfg.ext_counters) {
>          /* The Counters extensions is not enabled */
>          return -1;
>      }
> -
> -    /*
> -     * The counters are always enabled at run time on newer priv specs, as the
> -     * CSR has changed from controlling that the counters can be read to
> -     * controlling that the counters increment.
> -     */
> -    if (env->priv_ver > PRIV_VERSION_1_09_1) {
> -        return 0;
> -    }
> -
> -    if (env->priv < PRV_M) {
> -        ctr_en &= env->mcounteren;
> -    }
> -    if (env->priv < PRV_S) {
> -        ctr_en &= env->scounteren;
> -    }
> -    if (!(ctr_en & (1u << (csrno & 31)))) {
> -        return -1;
> -    }
>  #endif
>      return 0;
>  }
> @@ -358,34 +338,21 @@ static int write_mstatus(CPURISCVState *env, int csrno, target_ulong val)
>      int dirty;
>
>      /* flush tlb on mstatus fields that affect VM */
> -    if (env->priv_ver <= PRIV_VERSION_1_09_1) {
> -        if ((val ^ mstatus) & (MSTATUS_MXR | MSTATUS_MPP |
> -                MSTATUS_MPRV | MSTATUS_SUM | MSTATUS_VM)) {
> -            tlb_flush(env_cpu(env));
> -        }
> -        mask = MSTATUS_SIE | MSTATUS_SPIE | MSTATUS_MIE | MSTATUS_MPIE |
> -            MSTATUS_SPP | MSTATUS_FS | MSTATUS_MPRV | MSTATUS_SUM |
> -            MSTATUS_MPP | MSTATUS_MXR |
> -            (validate_vm(env, get_field(val, MSTATUS_VM)) ?
> -                MSTATUS_VM : 0);
> +    if ((val ^ mstatus) & (MSTATUS_MXR | MSTATUS_MPP | MSTATUS_MPV |
> +            MSTATUS_MPRV | MSTATUS_SUM)) {
> +        tlb_flush(env_cpu(env));
>      }
> -    if (env->priv_ver >= PRIV_VERSION_1_10_0) {
> -        if ((val ^ mstatus) & (MSTATUS_MXR | MSTATUS_MPP | MSTATUS_MPV |
> -                MSTATUS_MPRV | MSTATUS_SUM)) {
> -            tlb_flush(env_cpu(env));
> -        }
> -        mask = MSTATUS_SIE | MSTATUS_SPIE | MSTATUS_MIE | MSTATUS_MPIE |
> -            MSTATUS_SPP | MSTATUS_FS | MSTATUS_MPRV | MSTATUS_SUM |
> -            MSTATUS_MPP | MSTATUS_MXR | MSTATUS_TVM | MSTATUS_TSR |
> -            MSTATUS_TW;
> +    mask = MSTATUS_SIE | MSTATUS_SPIE | MSTATUS_MIE | MSTATUS_MPIE |
> +        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 MTL are not in mstatus. The current plan is to
> -             * add them to mstatush. For now, we just don't support it.
> -             */
> -            mask |= MSTATUS_MTL | MSTATUS_MPV;
> +        /*
> +         * RV32: MPV and MTL are not in mstatus. The current plan is to
> +         * add them to mstatush. For now, we just don't support it.
> +         */
> +        mask |= MSTATUS_MTL | MSTATUS_MPV;

The indentation level is wrong

>  #endif
> -    }
>
>      mstatus = (mstatus & ~mask) | (val & mask);
>
> @@ -553,8 +520,7 @@ static int write_mcounteren(CPURISCVState *env, int csrno, target_ulong val)
>  /* This regiser is replaced with CSR_MCOUNTINHIBIT in 1.11.0 */
>  static int read_mscounteren(CPURISCVState *env, int csrno, target_ulong *val)
>  {
> -    if (env->priv_ver > PRIV_VERSION_1_09_1
> -        && env->priv_ver < PRIV_VERSION_1_11_0) {
> +    if (env->priv_ver < PRIV_VERSION_1_11_0) {
>          return -1;
>      }
>      *val = env->mcounteren;
> @@ -564,8 +530,7 @@ static int read_mscounteren(CPURISCVState *env, int csrno, target_ulong *val)
>  /* This regiser is replaced with CSR_MCOUNTINHIBIT in 1.11.0 */
>  static int write_mscounteren(CPURISCVState *env, int csrno, target_ulong val)
>  {
> -    if (env->priv_ver > PRIV_VERSION_1_09_1
> -        && env->priv_ver < PRIV_VERSION_1_11_0) {
> +    if (env->priv_ver < PRIV_VERSION_1_11_0) {
>          return -1;
>      }
>      env->mcounteren = val;
> @@ -574,20 +539,13 @@ static int write_mscounteren(CPURISCVState *env, int csrno, target_ulong val)
>
>  static int read_mucounteren(CPURISCVState *env, int csrno, target_ulong *val)
>  {
> -    if (env->priv_ver > PRIV_VERSION_1_09_1) {
> -        return -1;
> -    }
> -    *val = env->scounteren;
> +    return -1;
>      return 0;
>  }
>
>  static int write_mucounteren(CPURISCVState *env, int csrno, target_ulong val)
>  {
> -    if (env->priv_ver > PRIV_VERSION_1_09_1) {
> -        return -1;
> -    }
> -    env->scounteren = val;
> -    return 0;
> +    return -1;
>  }
>
>  /* Machine Trap Handling */
> @@ -829,13 +787,7 @@ static int write_satp(CPURISCVState *env, int csrno, target_ulong val)
>      if (!riscv_feature(env, RISCV_FEATURE_MMU)) {
>          return 0;
>      }
> -    if (env->priv_ver <= PRIV_VERSION_1_09_1 && (val ^ env->sptbr)) {
> -        tlb_flush(env_cpu(env));
> -        env->sptbr = val & (((target_ulong)
> -            1 << (TARGET_PHYS_ADDR_SPACE_BITS - PGSHIFT)) - 1);
> -    }
> -    if (env->priv_ver >= PRIV_VERSION_1_10_0 &&
> -        validate_vm(env, get_field(val, SATP_MODE)) &&
> +    if (validate_vm(env, get_field(val, SATP_MODE)) &&
>          ((val ^ env->satp) & (SATP_MODE | SATP_ASID | SATP_PPN)))
>      {
>          if (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)) {
> diff --git a/target/riscv/insn_trans/trans_privileged.inc.c b/target/riscv/insn_trans/trans_privileged.inc.c
> index 76c2fad71c..1af9fa7df8 100644
> --- a/target/riscv/insn_trans/trans_privileged.inc.c
> +++ b/target/riscv/insn_trans/trans_privileged.inc.c
> @@ -95,12 +95,6 @@ static bool trans_sfence_vma(DisasContext *ctx, arg_sfence_vma *a)
>
>  static bool trans_sfence_vm(DisasContext *ctx, arg_sfence_vm *a)
>  {
> -#ifndef CONFIG_USER_ONLY
> -    if (ctx->priv_ver <= PRIV_VERSION_1_09_1) {
> -        gen_helper_tlb_flush(cpu_env);
> -        return true;
> -    }
> -#endif
>      return false;
>  }

There are 3 more places in this file that should be cleaned up:

./target/riscv/insn_trans/trans_privileged.inc.c:88:    if
(ctx->priv_ver >= PRIV_VERSION_1_10_0) {
./target/riscv/insn_trans/trans_privileged.inc.c:110:    if
(ctx->priv_ver >= PRIV_VERSION_1_10_0 &&
./target/riscv/insn_trans/trans_privileged.inc.c:130:    if
(ctx->priv_ver >= PRIV_VERSION_1_10_0 &&

And the following place in monitor.c should be cleaned up too:

./target/riscv/monitor.c:218:    if (env->priv_ver < PRIV_VERSION_1_10_0) {

And several other places in

./target/riscv/op_helper.c:87:    if (env->priv_ver >= PRIV_VERSION_1_10_0 &&
./target/riscv/op_helper.c:123:            env->priv_ver >=
PRIV_VERSION_1_10_0 ?
./target/riscv/op_helper.c:151:        env->priv_ver >= PRIV_VERSION_1_10_0 ?
./target/riscv/op_helper.c:180:        env->priv_ver >= PRIV_VERSION_1_10_0 &&
./target/riscv/op_helper.c:196:         env->priv_ver >= PRIV_VERSION_1_10_0 &&

Regards,
Bin

Re: [PATCH v2 3/3] target/riscv: Drop support for ISA spec version 1.09.1

[Alistair Francis]

On Wed, May 20, 2020 at 6:18 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Fri, May 8, 2020 at 3:22 AM Alistair Francis
> <alistair.francis@wdc.com> wrote:
> >
> > The RISC-V ISA spec version 1.09.1 has been deprecated in QEMU since
> > 4.1. It's not commonly used so let's remove support for it.
> >
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  target/riscv/cpu.c                            |  2 -
> >  target/riscv/cpu.h                            |  1 -
> >  target/riscv/csr.c                            | 82 ++++---------------
> >  .../riscv/insn_trans/trans_privileged.inc.c   |  6 --
> >  4 files changed, 17 insertions(+), 74 deletions(-)
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 112f2e3a2f..eeb91f8513 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -368,8 +368,6 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
> >              priv_version = PRIV_VERSION_1_11_0;
> >          } else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.10.0")) {
> >              priv_version = PRIV_VERSION_1_10_0;
> > -        } else if (!g_strcmp0(cpu->cfg.priv_spec, "v1.9.1")) {
> > -            priv_version = PRIV_VERSION_1_09_1;
> >          } else {
> >              error_setg(errp,
> >                         "Unsupported privilege spec version '%s'",
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 76b98d7a33..c022539012 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -73,7 +73,6 @@ enum {
> >      RISCV_FEATURE_MISA
> >  };
> >
> > -#define PRIV_VERSION_1_09_1 0x00010901
> >  #define PRIV_VERSION_1_10_0 0x00011000
> >  #define PRIV_VERSION_1_11_0 0x00011100
> >
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index 11d184cd16..df3498b24f 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -58,31 +58,11 @@ static int ctr(CPURISCVState *env, int csrno)
> >  #if !defined(CONFIG_USER_ONLY)
> >      CPUState *cs = env_cpu(env);
> >      RISCVCPU *cpu = RISCV_CPU(cs);
> > -    uint32_t ctr_en = ~0u;
> >
> >      if (!cpu->cfg.ext_counters) {
> >          /* The Counters extensions is not enabled */
> >          return -1;
> >      }
> > -
> > -    /*
> > -     * The counters are always enabled at run time on newer priv specs, as the
> > -     * CSR has changed from controlling that the counters can be read to
> > -     * controlling that the counters increment.
> > -     */
> > -    if (env->priv_ver > PRIV_VERSION_1_09_1) {
> > -        return 0;
> > -    }
> > -
> > -    if (env->priv < PRV_M) {
> > -        ctr_en &= env->mcounteren;
> > -    }
> > -    if (env->priv < PRV_S) {
> > -        ctr_en &= env->scounteren;
> > -    }
> > -    if (!(ctr_en & (1u << (csrno & 31)))) {
> > -        return -1;
> > -    }
> >  #endif
> >      return 0;
> >  }
> > @@ -358,34 +338,21 @@ static int write_mstatus(CPURISCVState *env, int csrno, target_ulong val)
> >      int dirty;
> >
> >      /* flush tlb on mstatus fields that affect VM */
> > -    if (env->priv_ver <= PRIV_VERSION_1_09_1) {
> > -        if ((val ^ mstatus) & (MSTATUS_MXR | MSTATUS_MPP |
> > -                MSTATUS_MPRV | MSTATUS_SUM | MSTATUS_VM)) {
> > -            tlb_flush(env_cpu(env));
> > -        }
> > -        mask = MSTATUS_SIE | MSTATUS_SPIE | MSTATUS_MIE | MSTATUS_MPIE |
> > -            MSTATUS_SPP | MSTATUS_FS | MSTATUS_MPRV | MSTATUS_SUM |
> > -            MSTATUS_MPP | MSTATUS_MXR |
> > -            (validate_vm(env, get_field(val, MSTATUS_VM)) ?
> > -                MSTATUS_VM : 0);
> > +    if ((val ^ mstatus) & (MSTATUS_MXR | MSTATUS_MPP | MSTATUS_MPV |
> > +            MSTATUS_MPRV | MSTATUS_SUM)) {
> > +        tlb_flush(env_cpu(env));
> >      }
> > -    if (env->priv_ver >= PRIV_VERSION_1_10_0) {
> > -        if ((val ^ mstatus) & (MSTATUS_MXR | MSTATUS_MPP | MSTATUS_MPV |
> > -                MSTATUS_MPRV | MSTATUS_SUM)) {
> > -            tlb_flush(env_cpu(env));
> > -        }
> > -        mask = MSTATUS_SIE | MSTATUS_SPIE | MSTATUS_MIE | MSTATUS_MPIE |
> > -            MSTATUS_SPP | MSTATUS_FS | MSTATUS_MPRV | MSTATUS_SUM |
> > -            MSTATUS_MPP | MSTATUS_MXR | MSTATUS_TVM | MSTATUS_TSR |
> > -            MSTATUS_TW;
> > +    mask = MSTATUS_SIE | MSTATUS_SPIE | MSTATUS_MIE | MSTATUS_MPIE |
> > +        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 MTL are not in mstatus. The current plan is to
> > -             * add them to mstatush. For now, we just don't support it.
> > -             */
> > -            mask |= MSTATUS_MTL | MSTATUS_MPV;
> > +        /*
> > +         * RV32: MPV and MTL are not in mstatus. The current plan is to
> > +         * add them to mstatush. For now, we just don't support it.
> > +         */
> > +        mask |= MSTATUS_MTL | MSTATUS_MPV;
>
> The indentation level is wrong

Good catch.

>
> >  #endif
> > -    }
> >
> >      mstatus = (mstatus & ~mask) | (val & mask);
> >
> > @@ -553,8 +520,7 @@ static int write_mcounteren(CPURISCVState *env, int csrno, target_ulong val)
> >  /* This regiser is replaced with CSR_MCOUNTINHIBIT in 1.11.0 */
> >  static int read_mscounteren(CPURISCVState *env, int csrno, target_ulong *val)
> >  {
> > -    if (env->priv_ver > PRIV_VERSION_1_09_1
> > -        && env->priv_ver < PRIV_VERSION_1_11_0) {
> > +    if (env->priv_ver < PRIV_VERSION_1_11_0) {
> >          return -1;
> >      }
> >      *val = env->mcounteren;
> > @@ -564,8 +530,7 @@ static int read_mscounteren(CPURISCVState *env, int csrno, target_ulong *val)
> >  /* This regiser is replaced with CSR_MCOUNTINHIBIT in 1.11.0 */
> >  static int write_mscounteren(CPURISCVState *env, int csrno, target_ulong val)
> >  {
> > -    if (env->priv_ver > PRIV_VERSION_1_09_1
> > -        && env->priv_ver < PRIV_VERSION_1_11_0) {
> > +    if (env->priv_ver < PRIV_VERSION_1_11_0) {
> >          return -1;
> >      }
> >      env->mcounteren = val;
> > @@ -574,20 +539,13 @@ static int write_mscounteren(CPURISCVState *env, int csrno, target_ulong val)
> >
> >  static int read_mucounteren(CPURISCVState *env, int csrno, target_ulong *val)
> >  {
> > -    if (env->priv_ver > PRIV_VERSION_1_09_1) {
> > -        return -1;
> > -    }
> > -    *val = env->scounteren;
> > +    return -1;
> >      return 0;
> >  }
> >
> >  static int write_mucounteren(CPURISCVState *env, int csrno, target_ulong val)
> >  {
> > -    if (env->priv_ver > PRIV_VERSION_1_09_1) {
> > -        return -1;
> > -    }
> > -    env->scounteren = val;
> > -    return 0;
> > +    return -1;
> >  }
> >
> >  /* Machine Trap Handling */
> > @@ -829,13 +787,7 @@ static int write_satp(CPURISCVState *env, int csrno, target_ulong val)
> >      if (!riscv_feature(env, RISCV_FEATURE_MMU)) {
> >          return 0;
> >      }
> > -    if (env->priv_ver <= PRIV_VERSION_1_09_1 && (val ^ env->sptbr)) {
> > -        tlb_flush(env_cpu(env));
> > -        env->sptbr = val & (((target_ulong)
> > -            1 << (TARGET_PHYS_ADDR_SPACE_BITS - PGSHIFT)) - 1);
> > -    }
> > -    if (env->priv_ver >= PRIV_VERSION_1_10_0 &&
> > -        validate_vm(env, get_field(val, SATP_MODE)) &&
> > +    if (validate_vm(env, get_field(val, SATP_MODE)) &&
> >          ((val ^ env->satp) & (SATP_MODE | SATP_ASID | SATP_PPN)))
> >      {
> >          if (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)) {
> > diff --git a/target/riscv/insn_trans/trans_privileged.inc.c b/target/riscv/insn_trans/trans_privileged.inc.c
> > index 76c2fad71c..1af9fa7df8 100644
> > --- a/target/riscv/insn_trans/trans_privileged.inc.c
> > +++ b/target/riscv/insn_trans/trans_privileged.inc.c
> > @@ -95,12 +95,6 @@ static bool trans_sfence_vma(DisasContext *ctx, arg_sfence_vma *a)
> >
> >  static bool trans_sfence_vm(DisasContext *ctx, arg_sfence_vm *a)
> >  {
> > -#ifndef CONFIG_USER_ONLY
> > -    if (ctx->priv_ver <= PRIV_VERSION_1_09_1) {
> > -        gen_helper_tlb_flush(cpu_env);
> > -        return true;
> > -    }
> > -#endif
> >      return false;
> >  }
>
> There are 3 more places in this file that should be cleaned up:
>
> ./target/riscv/insn_trans/trans_privileged.inc.c:88:    if
> (ctx->priv_ver >= PRIV_VERSION_1_10_0) {
> ./target/riscv/insn_trans/trans_privileged.inc.c:110:    if
> (ctx->priv_ver >= PRIV_VERSION_1_10_0 &&
> ./target/riscv/insn_trans/trans_privileged.inc.c:130:    if
> (ctx->priv_ver >= PRIV_VERSION_1_10_0 &&
>
> And the following place in monitor.c should be cleaned up too:
>
> ./target/riscv/monitor.c:218:    if (env->priv_ver < PRIV_VERSION_1_10_0) {
>
> And several other places in
>
> ./target/riscv/op_helper.c:87:    if (env->priv_ver >= PRIV_VERSION_1_10_0 &&
> ./target/riscv/op_helper.c:123:            env->priv_ver >=
> PRIV_VERSION_1_10_0 ?
> ./target/riscv/op_helper.c:151:        env->priv_ver >= PRIV_VERSION_1_10_0 ?
> ./target/riscv/op_helper.c:180:        env->priv_ver >= PRIV_VERSION_1_10_0 &&
> ./target/riscv/op_helper.c:196:         env->priv_ver >= PRIV_VERSION_1_10_0 &&

Yeah you are right. I have fixed all of these.

Alistair

>
> Regards,
> Bin

Re: [PATCH v2 1/3] hw/riscv: spike: Remove deprecated ISA specific machines

[Bin Meng]

On Fri, May 8, 2020 at 3:21 AM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> The ISA specific Spike machines have  been deprecated in QEMU since 4.1,
> let's finally remove them.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/riscv/spike.c         | 217 ---------------------------------------
>  include/hw/riscv/spike.h |   6 +-
>  2 files changed, 2 insertions(+), 221 deletions(-)
>

[snip]

> diff --git a/include/hw/riscv/spike.h b/include/hw/riscv/spike.h
> index dc770421bc..b98cfea0e4 100644
> --- a/include/hw/riscv/spike.h
> +++ b/include/hw/riscv/spike.h
> @@ -39,11 +39,9 @@ enum {
>  };
>
>  #if defined(TARGET_RISCV32)
> -#define SPIKE_V1_09_1_CPU TYPE_RISCV_CPU_RV32GCSU_V1_09_1
> -#define SPIKE_V1_10_0_CPU TYPE_RISCV_CPU_RV32GCSU_V1_10_0
> +#define SPIKE_V1_10_0_CPU TYPE_RISCV_CPU_SIFIVE_U34
>  #elif defined(TARGET_RISCV64)
> -#define SPIKE_V1_09_1_CPU TYPE_RISCV_CPU_RV64GCSU_V1_09_1
> -#define SPIKE_V1_10_0_CPU TYPE_RISCV_CPU_RV64GCSU_V1_10_0
> +#define SPIKE_V1_10_0_CPU TYPE_RISCV_CPU_SIFIVE_U54
>  #endif

On a second thought, I think we should not use the SIFIVE CPU type
here for Spike.

It should use the one that is used by 'virt', eg: TYPE_RISCV_CPU_BASE{32,64}

Regards,
Bin