[PATCH v1 0/8] RISC-V: Steps towards running 32-bit guests on

qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed

* [PATCH v1 0/8]  RISC-V: Steps towards running 32-bit guests on
@ 2021-04-02 20:02 Alistair Francis
  2021-04-02 20:02 ` [PATCH v1 1/8] target/riscv: Remove the hardcoded RVXLEN macro Alistair Francis
                   ` (7 more replies)
  0 siblings, 8 replies; 24+ messages in thread
From: Alistair Francis @ 2021-04-02 20:02 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: alistair.francis, bmeng.cn, palmer, alistair23

This is another step towards running 32-bit CPU code on the 64-bit
softmmu builds for RISC-V.

I have tested this and am able to run some 32-bit code, but eventually
hit some issue.  This series doesn't allow users to use 32-bit CPUs with
64-bit softmmu builds as it doesn't work yet. This series instead just
gets us a little closer to being able to and removes more hardcoded
macros so hopefully others also stop using them for new code.

Alistair Francis (8):
  target/riscv: Remove the hardcoded RVXLEN macro
  target/riscv: Remove the hardcoded SSTATUS_SD macro
  target/riscv: Remove the hardcoded HGATP_MODE macro
  target/riscv: Remove the hardcoded MSTATUS_SD macro
  target/riscv: Remove the hardcoded SATP_MODE macro
  target/riscv: Remove the unused HSTATUS_WPRI macro
  target/riscv: Remove an unused CASE_OP_32_64 macro
  target/riscv: Include RV32 instructions in RV64 build

 target/riscv/cpu.h            |  6 ----
 target/riscv/cpu_bits.h       | 44 ----------------------------
 target/riscv/insn16-32.decode | 24 ++++++++++++++++
 target/riscv/insn16-64.decode | 31 ++++++++++++++++++++
 target/riscv/cpu.c            |  6 +++-
 target/riscv/cpu_helper.c     | 51 +++++++++++++++++++++++++--------
 target/riscv/csr.c            | 54 +++++++++++++++++++++++++++--------
 target/riscv/monitor.c        | 22 ++++++++++----
 target/riscv/translate.c      | 43 ++++++++++++++++++++++------
 target/riscv/meson.build      |  7 +++--
 10 files changed, 197 insertions(+), 91 deletions(-)

-- 
2.31.0



^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v1 1/8] target/riscv: Remove the hardcoded RVXLEN macro
  2021-04-02 20:02 [PATCH v1 0/8] RISC-V: Steps towards running 32-bit guests on Alistair Francis
@ 2021-04-02 20:02 ` Alistair Francis
  2021-04-05 14:48   ` Richard Henderson
  2021-04-12  9:10   ` Bin Meng
  2021-04-02 20:02 ` [PATCH v1 2/8] target/riscv: Remove the hardcoded SSTATUS_SD macro Alistair Francis
                   ` (6 subsequent siblings)
  7 siblings, 2 replies; 24+ messages in thread
From: Alistair Francis @ 2021-04-02 20:02 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: alistair.francis, bmeng.cn, palmer, alistair23

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

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 0a33d387ba..ef838f5fbf 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -53,12 +53,6 @@
 #define RV32 ((target_ulong)1 << (TARGET_LONG_BITS - 2))
 #define RV64 ((target_ulong)2 << (TARGET_LONG_BITS - 2))
 
-#if defined(TARGET_RISCV32)
-#define RVXLEN RV32
-#elif defined(TARGET_RISCV64)
-#define RVXLEN RV64
-#endif
-
 #define RV(x) ((target_ulong)1 << (x - 'A'))
 
 #define RVI RV('I')
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 7d6ed80f6b..92c3195531 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -147,7 +147,11 @@ static void set_resetvec(CPURISCVState *env, int resetvec)
 static void riscv_any_cpu_init(Object *obj)
 {
     CPURISCVState *env = &RISCV_CPU(obj)->env;
-    set_misa(env, RVXLEN | RVI | RVM | RVA | RVF | RVD | RVC | RVU);
+#if defined(TARGET_RISCV32)
+    set_misa(env, RV32 | RVI | RVM | RVA | RVF | RVD | RVC | RVU);
+#elif defined(TARGET_RISCV64)
+    set_misa(env, RV64 | RVI | RVM | RVA | RVF | RVD | RVC | RVU);
+#endif
     set_priv_version(env, PRIV_VERSION_1_11_0);
 }
 
-- 
2.31.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v1 1/8] target/riscv: Remove the hardcoded RVXLEN macro
  2021-04-02 20:02 ` [PATCH v1 1/8] target/riscv: Remove the hardcoded RVXLEN macro Alistair Francis
@ 2021-04-05 14:48   ` Richard Henderson
  2021-04-12  9:10   ` Bin Meng
  1 sibling, 0 replies; 24+ messages in thread
From: Richard Henderson @ 2021-04-05 14:48 UTC (permalink / raw)
  To: Alistair Francis, qemu-devel, qemu-riscv; +Cc: alistair23, bmeng.cn, palmer

On 4/2/21 1:02 PM, Alistair Francis wrote:
> Signed-off-by: Alistair Francis<alistair.francis@wdc.com>
> ---
>   target/riscv/cpu.h | 6 ------
>   target/riscv/cpu.c | 6 +++++-
>   2 files changed, 5 insertions(+), 7 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v1 1/8] target/riscv: Remove the hardcoded RVXLEN macro
  2021-04-02 20:02 ` [PATCH v1 1/8] target/riscv: Remove the hardcoded RVXLEN macro Alistair Francis
  2021-04-05 14:48   ` Richard Henderson
@ 2021-04-12  9:10   ` Bin Meng
  1 sibling, 0 replies; 24+ messages in thread
From: Bin Meng @ 2021-04-12  9:10 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Palmer Dabbelt, open list:RISC-V,
	qemu-devel@nongnu.org Developers, Alistair Francis

On Sat, Apr 3, 2021 at 4:04 AM 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 | 6 +++++-
>  2 files changed, 5 insertions(+), 7 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v1 2/8] target/riscv: Remove the hardcoded SSTATUS_SD macro
  2021-04-02 20:02 [PATCH v1 0/8] RISC-V: Steps towards running 32-bit guests on Alistair Francis
  2021-04-02 20:02 ` [PATCH v1 1/8] target/riscv: Remove the hardcoded RVXLEN macro Alistair Francis
@ 2021-04-02 20:02 ` Alistair Francis
  2021-04-05 14:49   ` Richard Henderson
  2021-04-12  9:10   ` Bin Meng
  2021-04-02 20:02 ` [PATCH v1 3/8] target/riscv: Remove the hardcoded HGATP_MODE macro Alistair Francis
                   ` (5 subsequent siblings)
  7 siblings, 2 replies; 24+ messages in thread
From: Alistair Francis @ 2021-04-02 20:02 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: alistair.francis, bmeng.cn, palmer, alistair23

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

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index caf4599207..969dd05eae 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -423,12 +423,6 @@
 #define SSTATUS32_SD        0x80000000
 #define SSTATUS64_SD        0x8000000000000000ULL
 
-#if defined(TARGET_RISCV32)
-#define SSTATUS_SD SSTATUS32_SD
-#elif defined(TARGET_RISCV64)
-#define SSTATUS_SD SSTATUS64_SD
-#endif
-
 /* hstatus CSR bits */
 #define HSTATUS_VSBE         0x00000020
 #define HSTATUS_GVA          0x00000040
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d2585395bf..832c3bf7fd 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -418,7 +418,7 @@ static const target_ulong delegable_excps =
     (1ULL << (RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT));
 static const target_ulong sstatus_v1_10_mask = SSTATUS_SIE | SSTATUS_SPIE |
     SSTATUS_UIE | SSTATUS_UPIE | SSTATUS_SPP | SSTATUS_FS | SSTATUS_XS |
-    SSTATUS_SUM | SSTATUS_MXR | SSTATUS_SD;
+    SSTATUS_SUM | SSTATUS_MXR;
 static const target_ulong sip_writable_mask = SIP_SSIP | MIP_USIP | MIP_UEIP;
 static const target_ulong hip_writable_mask = MIP_VSSIP;
 static const target_ulong hvip_writable_mask = MIP_VSSIP | MIP_VSTIP | MIP_VSEIP;
@@ -738,6 +738,13 @@ static int rmw_mip(CPURISCVState *env, int csrno, target_ulong *ret_value,
 static int read_sstatus(CPURISCVState *env, int csrno, target_ulong *val)
 {
     target_ulong mask = (sstatus_v1_10_mask);
+
+    if (riscv_cpu_is_32bit(env)) {
+        mask |= SSTATUS32_SD;
+    } else {
+        mask |= SSTATUS64_SD;
+    }
+
     *val = env->mstatus & mask;
     return 0;
 }
-- 
2.31.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v1 2/8] target/riscv: Remove the hardcoded SSTATUS_SD macro
  2021-04-02 20:02 ` [PATCH v1 2/8] target/riscv: Remove the hardcoded SSTATUS_SD macro Alistair Francis
@ 2021-04-05 14:49   ` Richard Henderson
  2021-04-12  9:10   ` Bin Meng
  1 sibling, 0 replies; 24+ messages in thread
From: Richard Henderson @ 2021-04-05 14:49 UTC (permalink / raw)
  To: Alistair Francis, qemu-devel, qemu-riscv; +Cc: alistair23, bmeng.cn, palmer

On 4/2/21 1:02 PM, Alistair Francis wrote:
> Signed-off-by: Alistair Francis<alistair.francis@wdc.com>
> ---
>   target/riscv/cpu_bits.h | 6 ------
>   target/riscv/csr.c      | 9 ++++++++-
>   2 files changed, 8 insertions(+), 7 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v1 2/8] target/riscv: Remove the hardcoded SSTATUS_SD macro
  2021-04-02 20:02 ` [PATCH v1 2/8] target/riscv: Remove the hardcoded SSTATUS_SD macro Alistair Francis
  2021-04-05 14:49   ` Richard Henderson
@ 2021-04-12  9:10   ` Bin Meng
  1 sibling, 0 replies; 24+ messages in thread
From: Bin Meng @ 2021-04-12  9:10 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Palmer Dabbelt, open list:RISC-V,
	qemu-devel@nongnu.org Developers, Alistair Francis

On Sat, Apr 3, 2021 at 4:04 AM Alistair Francis
<alistair.francis@wdc.com> wrote:

Worth mentioning that this also fixed the issue of a writable SD bit

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

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v1 3/8] target/riscv: Remove the hardcoded HGATP_MODE macro
  2021-04-02 20:02 [PATCH v1 0/8] RISC-V: Steps towards running 32-bit guests on Alistair Francis
  2021-04-02 20:02 ` [PATCH v1 1/8] target/riscv: Remove the hardcoded RVXLEN macro Alistair Francis
  2021-04-02 20:02 ` [PATCH v1 2/8] target/riscv: Remove the hardcoded SSTATUS_SD macro Alistair Francis
@ 2021-04-02 20:02 ` Alistair Francis
  2021-04-05 14:54   ` Richard Henderson
  2021-04-02 20:02 ` [PATCH v1 4/8] target/riscv: Remove the hardcoded MSTATUS_SD macro Alistair Francis
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Alistair Francis @ 2021-04-02 20:02 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: alistair.francis, bmeng.cn, palmer, alistair23

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

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 969dd05eae..8caab23b62 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -207,17 +207,6 @@
 #define CSR_HTIMEDELTA      0x605
 #define CSR_HTIMEDELTAH     0x615
 
-#if defined(TARGET_RISCV32)
-#define HGATP_MODE           SATP32_MODE
-#define HGATP_VMID           SATP32_ASID
-#define HGATP_PPN            SATP32_PPN
-#endif
-#if defined(TARGET_RISCV64)
-#define HGATP_MODE           SATP64_MODE
-#define HGATP_VMID           SATP64_ASID
-#define HGATP_PPN            SATP64_PPN
-#endif
-
 /* Virtual CSRs */
 #define CSR_VSSTATUS        0x200
 #define CSR_VSIE            0x204
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 21c54ef561..6446af5de0 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -411,8 +411,13 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
         }
         widened = 0;
     } else {
-        base = (hwaddr)get_field(env->hgatp, HGATP_PPN) << PGSHIFT;
-        vm = get_field(env->hgatp, HGATP_MODE);
+        if (riscv_cpu_is_32bit(env)) {
+            base = (hwaddr)get_field(env->hgatp, SATP32_PPN) << PGSHIFT;
+            vm = get_field(env->hgatp, SATP32_MODE);
+        } else {
+            base = (hwaddr)get_field(env->hgatp, SATP64_PPN) << PGSHIFT;
+            vm = get_field(env->hgatp, SATP64_MODE);
+        }
         widened = 2;
     }
     /* status.SUM will be ignored if execute on background */
@@ -621,9 +626,15 @@ static void raise_mmu_exception(CPURISCVState *env, target_ulong address,
             get_field(env->satp, SATP_MODE) != VM_1_10_MBARE &&
             !pmp_violation;
     } else {
-        page_fault_exceptions =
-            get_field(env->hgatp, HGATP_MODE) != VM_1_10_MBARE &&
-            !pmp_violation;
+        if (riscv_cpu_is_32bit(env)) {
+            page_fault_exceptions =
+                get_field(env->hgatp, SATP32_MODE) != VM_1_10_MBARE &&
+                !pmp_violation;
+        } else {
+            page_fault_exceptions =
+                get_field(env->hgatp, SATP64_MODE) != VM_1_10_MBARE &&
+                !pmp_violation;
+        }
     }
     switch (access_type) {
     case MMU_INST_FETCH:
-- 
2.31.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v1 3/8] target/riscv: Remove the hardcoded HGATP_MODE macro
  2021-04-02 20:02 ` [PATCH v1 3/8] target/riscv: Remove the hardcoded HGATP_MODE macro Alistair Francis
@ 2021-04-05 14:54   ` Richard Henderson
  0 siblings, 0 replies; 24+ messages in thread
From: Richard Henderson @ 2021-04-05 14:54 UTC (permalink / raw)
  To: Alistair Francis, qemu-devel, qemu-riscv; +Cc: alistair23, bmeng.cn, palmer

On 4/2/21 1:02 PM, Alistair Francis wrote:
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>   target/riscv/cpu_bits.h   | 11 -----------
>   target/riscv/cpu_helper.c | 21 ++++++++++++++++-----
>   2 files changed, 16 insertions(+), 16 deletions(-)


Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

> @@ -621,9 +626,15 @@ static void raise_mmu_exception(CPURISCVState *env, target_ulong address,
>               get_field(env->satp, SATP_MODE) != VM_1_10_MBARE &&
>               !pmp_violation;
>       } else {
> -        page_fault_exceptions =
> -            get_field(env->hgatp, HGATP_MODE) != VM_1_10_MBARE &&
> -            !pmp_violation;
> +        if (riscv_cpu_is_32bit(env)) {
> +            page_fault_exceptions =
> +                get_field(env->hgatp, SATP32_MODE) != VM_1_10_MBARE &&
> +                !pmp_violation;
> +        } else {
> +            page_fault_exceptions =
> +                get_field(env->hgatp, SATP64_MODE) != VM_1_10_MBARE &&
> +                !pmp_violation;
> +        }

Looks like you could simplify this to extract the vm in each if branch, then do 
the comparison afterward.

   if (first) {
     vm = ...
   } else if (32bit) {
     vm = ...
   } else {
     vm = ...
   }
   page_fault_exceptions = vm != VM_1_10_MBARE && !pmp_violation;


r~



^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v1 4/8] target/riscv: Remove the hardcoded MSTATUS_SD macro
  2021-04-02 20:02 [PATCH v1 0/8] RISC-V: Steps towards running 32-bit guests on Alistair Francis
                   ` (2 preceding siblings ...)
  2021-04-02 20:02 ` [PATCH v1 3/8] target/riscv: Remove the hardcoded HGATP_MODE macro Alistair Francis
@ 2021-04-02 20:02 ` Alistair Francis
  2021-04-05 15:10   ` Richard Henderson
  2021-04-02 20:02 ` [PATCH v1 5/8] target/riscv: Remove the hardcoded SATP_MODE macro Alistair Francis
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Alistair Francis @ 2021-04-02 20:02 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: alistair.francis, bmeng.cn, palmer, alistair23

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu_bits.h  | 10 ----------
 target/riscv/csr.c       | 12 ++++++++++--
 target/riscv/translate.c | 19 +++++++++++++++++--
 3 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 8caab23b62..dd643d0f63 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -387,16 +387,6 @@
 #define MXL_RV64            2
 #define MXL_RV128           3
 
-#if defined(TARGET_RISCV32)
-#define MSTATUS_SD MSTATUS32_SD
-#define MISA_MXL MISA32_MXL
-#define MXL_VAL MXL_RV32
-#elif defined(TARGET_RISCV64)
-#define MSTATUS_SD MSTATUS64_SD
-#define MISA_MXL MISA64_MXL
-#define MXL_VAL MXL_RV64
-#endif
-
 /* sstatus CSR bits */
 #define SSTATUS_UIE         0x00000001
 #define SSTATUS_SIE         0x00000002
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 832c3bf7fd..6052b2d6e9 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -492,7 +492,11 @@ static int write_mstatus(CPURISCVState *env, int csrno, target_ulong val)
 
     dirty = ((mstatus & MSTATUS_FS) == MSTATUS_FS) |
             ((mstatus & MSTATUS_XS) == MSTATUS_XS);
-    mstatus = set_field(mstatus, MSTATUS_SD, dirty);
+    if (riscv_cpu_is_32bit(env)) {
+        mstatus = set_field(mstatus, MSTATUS32_SD, dirty);
+    } else {
+        mstatus = set_field(mstatus, MSTATUS64_SD, dirty);
+    }
     env->mstatus = mstatus;
 
     return 0;
@@ -564,7 +568,11 @@ static int write_misa(CPURISCVState *env, int csrno, target_ulong val)
     }
 
     /* misa.MXL writes are not supported by QEMU */
-    val = (env->misa & MISA_MXL) | (val & ~MISA_MXL);
+    if (riscv_cpu_is_32bit(env)) {
+        val = (env->misa & MISA32_MXL) | (val & ~MISA32_MXL);
+    } else {
+        val = (env->misa & MISA64_MXL) | (val & ~MISA64_MXL);
+    }
 
     /* flush translation cache */
     if (val != env->misa) {
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 2f9f5ccc62..9c6d998efa 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -369,6 +369,9 @@ static void gen_jal(DisasContext *ctx, int rd, target_ulong imm)
 static void mark_fs_dirty(DisasContext *ctx)
 {
     TCGv tmp;
+    CPUState *cpu = ctx->cs;
+    CPURISCVState *env = cpu->env_ptr;
+
     if (ctx->mstatus_fs == MSTATUS_FS) {
         return;
     }
@@ -377,12 +380,24 @@ static void mark_fs_dirty(DisasContext *ctx)
 
     tmp = tcg_temp_new();
     tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
-    tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | MSTATUS_SD);
+    if (riscv_cpu_is_32bit(env)) {
+        tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | MSTATUS32_SD);
+    } else {
+#if defined(TARGET_RISCV64)
+        tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | MSTATUS64_SD);
+#endif
+    }
     tcg_gen_st_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
 
     if (ctx->virt_enabled) {
         tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus_hs));
-        tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | MSTATUS_SD);
+        if (riscv_cpu_is_32bit(env)) {
+            tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | MSTATUS32_SD);
+        } else {
+#if defined(TARGET_RISCV64)
+            tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | MSTATUS64_SD);
+#endif
+        }
         tcg_gen_st_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus_hs));
     }
     tcg_temp_free(tmp);
-- 
2.31.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v1 4/8] target/riscv: Remove the hardcoded MSTATUS_SD macro
  2021-04-02 20:02 ` [PATCH v1 4/8] target/riscv: Remove the hardcoded MSTATUS_SD macro Alistair Francis
@ 2021-04-05 15:10   ` Richard Henderson
  2021-04-07 17:11     ` Alistair Francis
  2021-04-08 15:20     ` Alistair Francis
  0 siblings, 2 replies; 24+ messages in thread
From: Richard Henderson @ 2021-04-05 15:10 UTC (permalink / raw)
  To: Alistair Francis, qemu-devel, qemu-riscv; +Cc: alistair23, bmeng.cn, palmer

On 4/2/21 1:02 PM, Alistair Francis wrote:
> @@ -369,6 +369,9 @@ static void gen_jal(DisasContext *ctx, int rd, target_ulong imm)
>   static void mark_fs_dirty(DisasContext *ctx)
>   {
>       TCGv tmp;
> +    CPUState *cpu = ctx->cs;
> +    CPURISCVState *env = cpu->env_ptr;
> +
>       if (ctx->mstatus_fs == MSTATUS_FS) {
>           return;
>       }
> @@ -377,12 +380,24 @@ static void mark_fs_dirty(DisasContext *ctx)
>   
>       tmp = tcg_temp_new();
>       tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
> -    tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | MSTATUS_SD);
> +    if (riscv_cpu_is_32bit(env)) {

This is less than ideal, and will be incorrect long term.
You should check ctx->misa instead.

Eventually you'll need to change riscv_tr_init_disas_context to not just copy 
ctx->misa from env.  At present we flush all translation blocks when misa 
changes, which works.  But you won't want to do that when the hypervisor is 
64-bit and the guest is 32-bit.

Anyway, I think it would be a good idea to create a helper local to translate, 
akin to has_ext().

> +        tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | MSTATUS32_SD);
> +    } else {
> +#if defined(TARGET_RISCV64)
> +        tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | MSTATUS64_SD);
> +#endif

The ifdefs are ugly.  I presume there's some sort of compiler warning here? 
Does it go away if you cast to target_ulong?

How about

     target_ulong sd = is_32bit(ctx) ? MSTATUS32_SD : MSTATUS64_SD;
     tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | sd);


r~


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v1 4/8] target/riscv: Remove the hardcoded MSTATUS_SD macro
  2021-04-05 15:10   ` Richard Henderson
@ 2021-04-07 17:11     ` Alistair Francis
  2021-04-08 15:20     ` Alistair Francis
  1 sibling, 0 replies; 24+ messages in thread
From: Alistair Francis @ 2021-04-07 17:11 UTC (permalink / raw)
  To: Richard Henderson
  Cc: open list:RISC-V, Palmer Dabbelt, Bin Meng, Alistair Francis,
	qemu-devel@nongnu.org Developers

On Mon, Apr 5, 2021 at 11:10 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 4/2/21 1:02 PM, Alistair Francis wrote:
> > @@ -369,6 +369,9 @@ static void gen_jal(DisasContext *ctx, int rd, target_ulong imm)
> >   static void mark_fs_dirty(DisasContext *ctx)
> >   {
> >       TCGv tmp;
> > +    CPUState *cpu = ctx->cs;
> > +    CPURISCVState *env = cpu->env_ptr;
> > +
> >       if (ctx->mstatus_fs == MSTATUS_FS) {
> >           return;
> >       }
> > @@ -377,12 +380,24 @@ static void mark_fs_dirty(DisasContext *ctx)
> >
> >       tmp = tcg_temp_new();
> >       tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
> > -    tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | MSTATUS_SD);
> > +    if (riscv_cpu_is_32bit(env)) {
>
> This is less than ideal, and will be incorrect long term.
> You should check ctx->misa instead.
>
> Eventually you'll need to change riscv_tr_init_disas_context to not just copy
> ctx->misa from env.  At present we flush all translation blocks when misa
> changes, which works.  But you won't want to do that when the hypervisor is
> 64-bit and the guest is 32-bit.
>
> Anyway, I think it would be a good idea to create a helper local to translate,
> akin to has_ext().
>
> > +        tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | MSTATUS32_SD);
> > +    } else {
> > +#if defined(TARGET_RISCV64)
> > +        tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | MSTATUS64_SD);
> > +#endif
>
> The ifdefs are ugly.  I presume there's some sort of compiler warning here?
> Does it go away if you cast to target_ulong?
>
> How about
>
>      target_ulong sd = is_32bit(ctx) ? MSTATUS32_SD : MSTATUS64_SD;
>      tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | sd);
>

That works, thanks!

Alistair

>
> r~


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v1 4/8] target/riscv: Remove the hardcoded MSTATUS_SD macro
  2021-04-05 15:10   ` Richard Henderson
  2021-04-07 17:11     ` Alistair Francis
@ 2021-04-08 15:20     ` Alistair Francis
  2021-04-08 18:51       ` Richard Henderson
  1 sibling, 1 reply; 24+ messages in thread
From: Alistair Francis @ 2021-04-08 15:20 UTC (permalink / raw)
  To: Richard Henderson
  Cc: open list:RISC-V, Palmer Dabbelt, Bin Meng, Alistair Francis,
	qemu-devel@nongnu.org Developers

On Mon, Apr 5, 2021 at 11:10 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 4/2/21 1:02 PM, Alistair Francis wrote:
> > @@ -369,6 +369,9 @@ static void gen_jal(DisasContext *ctx, int rd, target_ulong imm)
> >   static void mark_fs_dirty(DisasContext *ctx)
> >   {
> >       TCGv tmp;
> > +    CPUState *cpu = ctx->cs;
> > +    CPURISCVState *env = cpu->env_ptr;
> > +
> >       if (ctx->mstatus_fs == MSTATUS_FS) {
> >           return;
> >       }
> > @@ -377,12 +380,24 @@ static void mark_fs_dirty(DisasContext *ctx)
> >
> >       tmp = tcg_temp_new();
> >       tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
> > -    tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | MSTATUS_SD);
> > +    if (riscv_cpu_is_32bit(env)) {
>
> This is less than ideal, and will be incorrect long term.
> You should check ctx->misa instead.
>
> Eventually you'll need to change riscv_tr_init_disas_context to not just copy
> ctx->misa from env.  At present we flush all translation blocks when misa
> changes, which works.  But you won't want to do that when the hypervisor is
> 64-bit and the guest is 32-bit.
>
> Anyway, I think it would be a good idea to create a helper local to translate,
> akin to has_ext().
>
> > +        tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | MSTATUS32_SD);
> > +    } else {
> > +#if defined(TARGET_RISCV64)
> > +        tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | MSTATUS64_SD);
> > +#endif
>
> The ifdefs are ugly.  I presume there's some sort of compiler warning here?
> Does it go away if you cast to target_ulong?
>
> How about
>
>      target_ulong sd = is_32bit(ctx) ? MSTATUS32_SD : MSTATUS64_SD;

It turns out clang doesn't like this, so I might still be stuck with ifdefs.

Alistair

>      tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS | sd);
>
>
> r~


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v1 4/8] target/riscv: Remove the hardcoded MSTATUS_SD macro
  2021-04-08 15:20     ` Alistair Francis
@ 2021-04-08 18:51       ` Richard Henderson
  0 siblings, 0 replies; 24+ messages in thread
From: Richard Henderson @ 2021-04-08 18:51 UTC (permalink / raw)
  To: Alistair Francis
  Cc: open list:RISC-V, Palmer Dabbelt, Bin Meng, Alistair Francis,
	qemu-devel@nongnu.org Developers

On 4/8/21 8:20 AM, Alistair Francis wrote:
>>       target_ulong sd = is_32bit(ctx) ? MSTATUS32_SD : MSTATUS64_SD;
> 
> It turns out clang doesn't like this, so I might still be stuck with ifdefs.

I think we need

#ifdef TARGET_RISCV32
#define is_32bit(ctx)  true
#else
...
#endif

based on

$ cat z.c
int foo(int x) { return x ? 1 : 1ul << 32; }
int bar(void) { return 1 ? 1 : 1ul << 32; }
rth@cloudburst:~$ clang-11 -S -Wall z.c
z.c:1:37: warning: implicit conversion from 'unsigned long' to 'int' changes 
value from 4294967296 to 0 [-Wconstant-conversion]
int foo(int x) { return x ? 1 : 1ul << 32; }
                  ~~~~~~         ~~~~^~~~~

r~


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v1 5/8] target/riscv: Remove the hardcoded SATP_MODE macro
  2021-04-02 20:02 [PATCH v1 0/8] RISC-V: Steps towards running 32-bit guests on Alistair Francis
                   ` (3 preceding siblings ...)
  2021-04-02 20:02 ` [PATCH v1 4/8] target/riscv: Remove the hardcoded MSTATUS_SD macro Alistair Francis
@ 2021-04-02 20:02 ` Alistair Francis
  2021-04-05 15:14   ` Richard Henderson
  2021-04-02 20:02 ` [PATCH v1 6/8] target/riscv: Remove the unused HSTATUS_WPRI macro Alistair Francis
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Alistair Francis @ 2021-04-02 20:02 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: alistair.francis, bmeng.cn, palmer, alistair23

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu_bits.h   | 11 -----------
 target/riscv/cpu_helper.c | 30 +++++++++++++++++++++++-------
 target/riscv/csr.c        | 33 ++++++++++++++++++++++++---------
 target/riscv/monitor.c    | 22 +++++++++++++++++-----
 4 files changed, 64 insertions(+), 32 deletions(-)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index dd643d0f63..6a816ce9c2 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -452,17 +452,6 @@
 #define SATP64_ASID         0x0FFFF00000000000ULL
 #define SATP64_PPN          0x00000FFFFFFFFFFFULL
 
-#if defined(TARGET_RISCV32)
-#define SATP_MODE           SATP32_MODE
-#define SATP_ASID           SATP32_ASID
-#define SATP_PPN            SATP32_PPN
-#endif
-#if defined(TARGET_RISCV64)
-#define SATP_MODE           SATP64_MODE
-#define SATP_ASID           SATP64_ASID
-#define SATP_PPN            SATP64_PPN
-#endif
-
 /* VM modes (mstatus.vm) privileged ISA 1.9.1 */
 #define VM_1_09_MBARE       0
 #define VM_1_09_MBB         1
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 6446af5de0..7ae9352d80 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -403,11 +403,21 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
 
     if (first_stage == true) {
         if (use_background) {
-            base = (hwaddr)get_field(env->vsatp, SATP_PPN) << PGSHIFT;
-            vm = get_field(env->vsatp, SATP_MODE);
+            if (riscv_cpu_is_32bit(env)) {
+                base = (hwaddr)get_field(env->vsatp, SATP32_PPN) << PGSHIFT;
+                vm = get_field(env->vsatp, SATP32_MODE);
+            } else {
+                base = (hwaddr)get_field(env->vsatp, SATP64_PPN) << PGSHIFT;
+                vm = get_field(env->vsatp, SATP64_MODE);
+            }
         } else {
-            base = (hwaddr)get_field(env->satp, SATP_PPN) << PGSHIFT;
-            vm = get_field(env->satp, SATP_MODE);
+            if (riscv_cpu_is_32bit(env)) {
+                base = (hwaddr)get_field(env->satp, SATP32_PPN) << PGSHIFT;
+                vm = get_field(env->satp, SATP32_MODE);
+            } else {
+                base = (hwaddr)get_field(env->satp, SATP64_PPN) << PGSHIFT;
+                vm = get_field(env->satp, SATP64_MODE);
+            }
         }
         widened = 0;
     } else {
@@ -622,9 +632,15 @@ static void raise_mmu_exception(CPURISCVState *env, target_ulong address,
     CPUState *cs = env_cpu(env);
     int page_fault_exceptions;
     if (first_stage) {
-        page_fault_exceptions =
-            get_field(env->satp, SATP_MODE) != VM_1_10_MBARE &&
-            !pmp_violation;
+        if (riscv_cpu_is_32bit(env)) {
+            page_fault_exceptions =
+                get_field(env->satp, SATP32_MODE) != VM_1_10_MBARE &&
+                !pmp_violation;
+        } else {
+            page_fault_exceptions =
+                get_field(env->satp, SATP64_MODE) != VM_1_10_MBARE &&
+                !pmp_violation;
+        }
     } else {
         if (riscv_cpu_is_32bit(env)) {
             page_fault_exceptions =
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 6052b2d6e9..b0ebaa029e 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -930,16 +930,31 @@ static int write_satp(CPURISCVState *env, int csrno, target_ulong val)
     if (!riscv_feature(env, RISCV_FEATURE_MMU)) {
         return 0;
     }
-    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)) {
-            return -RISCV_EXCP_ILLEGAL_INST;
-        } else {
-            if ((val ^ env->satp) & SATP_ASID) {
-                tlb_flush(env_cpu(env));
+    if (riscv_cpu_is_32bit(env)) {
+        if (validate_vm(env, get_field(val, SATP32_MODE)) &&
+            ((val ^ env->satp) & (SATP32_MODE | SATP32_ASID | SATP32_PPN)))
+        {
+            if (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)) {
+                return -RISCV_EXCP_ILLEGAL_INST;
+            } else {
+                if ((val ^ env->satp) & SATP32_ASID) {
+                    tlb_flush(env_cpu(env));
+                }
+                env->satp = val;
+            }
+        }
+    } else {
+        if (validate_vm(env, get_field(val, SATP64_MODE)) &&
+            ((val ^ env->satp) & (SATP64_MODE | SATP64_ASID | SATP64_PPN)))
+        {
+            if (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)) {
+                return -RISCV_EXCP_ILLEGAL_INST;
+            } else {
+                if ((val ^ env->satp) & SATP64_ASID) {
+                    tlb_flush(env_cpu(env));
+                }
+                env->satp = val;
             }
-            env->satp = val;
         }
     }
     return 0;
diff --git a/target/riscv/monitor.c b/target/riscv/monitor.c
index e51188f919..f7e6ea72b3 100644
--- a/target/riscv/monitor.c
+++ b/target/riscv/monitor.c
@@ -150,9 +150,14 @@ static void mem_info_svxx(Monitor *mon, CPUArchState *env)
     target_ulong last_size;
     int last_attr;
 
-    base = (hwaddr)get_field(env->satp, SATP_PPN) << PGSHIFT;
+    if (riscv_cpu_is_32bit(env)) {
+        base = (hwaddr)get_field(env->satp, SATP32_PPN) << PGSHIFT;
+        vm = get_field(env->satp, SATP32_MODE);
+    } else {
+        base = (hwaddr)get_field(env->satp, SATP64_PPN) << PGSHIFT;
+        vm = get_field(env->satp, SATP64_MODE);
+    }
 
-    vm = get_field(env->satp, SATP_MODE);
     switch (vm) {
     case VM_1_10_SV32:
         levels = 2;
@@ -215,9 +220,16 @@ void hmp_info_mem(Monitor *mon, const QDict *qdict)
         return;
     }
 
-    if (!(env->satp & SATP_MODE)) {
-        monitor_printf(mon, "No translation or protection\n");
-        return;
+    if (riscv_cpu_is_32bit(env)) {
+        if (!(env->satp & SATP32_MODE)) {
+            monitor_printf(mon, "No translation or protection\n");
+            return;
+        }
+    } else {
+        if (!(env->satp & SATP64_MODE)) {
+            monitor_printf(mon, "No translation or protection\n");
+            return;
+        }
     }
 
     mem_info_svxx(mon, env);
-- 
2.31.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v1 5/8] target/riscv: Remove the hardcoded SATP_MODE macro
  2021-04-02 20:02 ` [PATCH v1 5/8] target/riscv: Remove the hardcoded SATP_MODE macro Alistair Francis
@ 2021-04-05 15:14   ` Richard Henderson
  0 siblings, 0 replies; 24+ messages in thread
From: Richard Henderson @ 2021-04-05 15:14 UTC (permalink / raw)
  To: Alistair Francis, qemu-devel, qemu-riscv; +Cc: alistair23, bmeng.cn, palmer

On 4/2/21 1:02 PM, Alistair Francis wrote:
> @@ -622,9 +632,15 @@ static void raise_mmu_exception(CPURISCVState *env, target_ulong address,
>       CPUState *cs = env_cpu(env);
>       int page_fault_exceptions;
>       if (first_stage) {
> -        page_fault_exceptions =
> -            get_field(env->satp, SATP_MODE) != VM_1_10_MBARE &&
> -            !pmp_violation;
> +        if (riscv_cpu_is_32bit(env)) {
> +            page_fault_exceptions =
> +                get_field(env->satp, SATP32_MODE) != VM_1_10_MBARE &&
> +                !pmp_violation;
> +        } else {
> +            page_fault_exceptions =
> +                get_field(env->satp, SATP64_MODE) != VM_1_10_MBARE &&
> +                !pmp_violation;
> +        }

Similar commentary wrt HGATP_MODE.

>       } else {
>           if (riscv_cpu_is_32bit(env)) {
>               page_fault_exceptions =
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 6052b2d6e9..b0ebaa029e 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -930,16 +930,31 @@ static int write_satp(CPURISCVState *env, int csrno, target_ulong val)
>       if (!riscv_feature(env, RISCV_FEATURE_MMU)) {
>           return 0;
>       }
> -    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)) {
> -            return -RISCV_EXCP_ILLEGAL_INST;
> -        } else {
> -            if ((val ^ env->satp) & SATP_ASID) {
> -                tlb_flush(env_cpu(env));
> +    if (riscv_cpu_is_32bit(env)) {
> +        if (validate_vm(env, get_field(val, SATP32_MODE)) &&
> +            ((val ^ env->satp) & (SATP32_MODE | SATP32_ASID | SATP32_PPN)))
> +        {
> +            if (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)) {
> +                return -RISCV_EXCP_ILLEGAL_INST;
> +            } else {
> +                if ((val ^ env->satp) & SATP32_ASID) {
> +                    tlb_flush(env_cpu(env));
> +                }
> +                env->satp = val;
> +            }
> +        }

I think you really don't want to be duplicating this code.
Just select the constants into local variables.


r~


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v1 6/8] target/riscv: Remove the unused HSTATUS_WPRI macro
  2021-04-02 20:02 [PATCH v1 0/8] RISC-V: Steps towards running 32-bit guests on Alistair Francis
                   ` (4 preceding siblings ...)
  2021-04-02 20:02 ` [PATCH v1 5/8] target/riscv: Remove the hardcoded SATP_MODE macro Alistair Francis
@ 2021-04-02 20:02 ` Alistair Francis
  2021-04-05 15:15   ` Richard Henderson
  2021-04-12  9:10   ` Bin Meng
  2021-04-02 20:02 ` [PATCH v1 7/8] target/riscv: Remove an unused CASE_OP_32_64 macro Alistair Francis
  2021-04-02 20:03 ` [PATCH v1 8/8] target/riscv: Include RV32 instructions in RV64 build Alistair Francis
  7 siblings, 2 replies; 24+ messages in thread
From: Alistair Francis @ 2021-04-02 20:02 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: alistair.francis, bmeng.cn, palmer, alistair23

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

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 6a816ce9c2..9f6fbe3dc5 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -416,12 +416,6 @@
 #define HSTATUS32_WPRI       0xFF8FF87E
 #define HSTATUS64_WPRI       0xFFFFFFFFFF8FF87EULL
 
-#if defined(TARGET_RISCV32)
-#define HSTATUS_WPRI HSTATUS32_WPRI
-#elif defined(TARGET_RISCV64)
-#define HSTATUS_WPRI HSTATUS64_WPRI
-#endif
-
 #define HCOUNTEREN_CY        (1 << 0)
 #define HCOUNTEREN_TM        (1 << 1)
 #define HCOUNTEREN_IR        (1 << 2)
-- 
2.31.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v1 6/8] target/riscv: Remove the unused HSTATUS_WPRI macro
  2021-04-02 20:02 ` [PATCH v1 6/8] target/riscv: Remove the unused HSTATUS_WPRI macro Alistair Francis
@ 2021-04-05 15:15   ` Richard Henderson
  2021-04-12  9:10   ` Bin Meng
  1 sibling, 0 replies; 24+ messages in thread
From: Richard Henderson @ 2021-04-05 15:15 UTC (permalink / raw)
  To: Alistair Francis, qemu-devel, qemu-riscv; +Cc: alistair23, bmeng.cn, palmer

On 4/2/21 1:02 PM, Alistair Francis wrote:
> Signed-off-by: Alistair Francis<alistair.francis@wdc.com>
> ---
>   target/riscv/cpu_bits.h | 6 ------
>   1 file changed, 6 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v1 6/8] target/riscv: Remove the unused HSTATUS_WPRI macro
  2021-04-02 20:02 ` [PATCH v1 6/8] target/riscv: Remove the unused HSTATUS_WPRI macro Alistair Francis
  2021-04-05 15:15   ` Richard Henderson
@ 2021-04-12  9:10   ` Bin Meng
  1 sibling, 0 replies; 24+ messages in thread
From: Bin Meng @ 2021-04-12  9:10 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Palmer Dabbelt, open list:RISC-V,
	qemu-devel@nongnu.org Developers, Alistair Francis

On Sat, Apr 3, 2021 at 4:05 AM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/cpu_bits.h | 6 ------
>  1 file changed, 6 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v1 7/8] target/riscv: Remove an unused CASE_OP_32_64 macro
  2021-04-02 20:02 [PATCH v1 0/8] RISC-V: Steps towards running 32-bit guests on Alistair Francis
                   ` (5 preceding siblings ...)
  2021-04-02 20:02 ` [PATCH v1 6/8] target/riscv: Remove the unused HSTATUS_WPRI macro Alistair Francis
@ 2021-04-02 20:02 ` Alistair Francis
  2021-04-05 15:15   ` Richard Henderson
  2021-04-12  9:10   ` Bin Meng
  2021-04-02 20:03 ` [PATCH v1 8/8] target/riscv: Include RV32 instructions in RV64 build Alistair Francis
  7 siblings, 2 replies; 24+ messages in thread
From: Alistair Francis @ 2021-04-02 20:02 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: alistair.francis, bmeng.cn, palmer, alistair23

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/translate.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 9c6d998efa..4af55deaea 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -67,12 +67,6 @@ typedef struct DisasContext {
     CPUState *cs;
 } DisasContext;
 
-#ifdef TARGET_RISCV64
-#define CASE_OP_32_64(X) case X: case glue(X, W)
-#else
-#define CASE_OP_32_64(X) case X
-#endif
-
 static inline bool has_ext(DisasContext *ctx, uint32_t ext)
 {
     return ctx->misa & ext;
-- 
2.31.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v1 7/8] target/riscv: Remove an unused CASE_OP_32_64 macro
  2021-04-02 20:02 ` [PATCH v1 7/8] target/riscv: Remove an unused CASE_OP_32_64 macro Alistair Francis
@ 2021-04-05 15:15   ` Richard Henderson
  2021-04-12  9:10   ` Bin Meng
  1 sibling, 0 replies; 24+ messages in thread
From: Richard Henderson @ 2021-04-05 15:15 UTC (permalink / raw)
  To: Alistair Francis, qemu-devel, qemu-riscv; +Cc: alistair23, bmeng.cn, palmer

On 4/2/21 1:02 PM, Alistair Francis wrote:
> Signed-off-by: Alistair Francis<alistair.francis@wdc.com>
> ---
>   target/riscv/translate.c | 6 ------
>   1 file changed, 6 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v1 7/8] target/riscv: Remove an unused CASE_OP_32_64 macro
  2021-04-02 20:02 ` [PATCH v1 7/8] target/riscv: Remove an unused CASE_OP_32_64 macro Alistair Francis
  2021-04-05 15:15   ` Richard Henderson
@ 2021-04-12  9:10   ` Bin Meng
  1 sibling, 0 replies; 24+ messages in thread
From: Bin Meng @ 2021-04-12  9:10 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Palmer Dabbelt, open list:RISC-V,
	qemu-devel@nongnu.org Developers, Alistair Francis

On Sat, Apr 3, 2021 at 4:04 AM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/translate.c | 6 ------
>  1 file changed, 6 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v1 8/8] target/riscv: Include RV32 instructions in RV64 build
  2021-04-02 20:02 [PATCH v1 0/8] RISC-V: Steps towards running 32-bit guests on Alistair Francis
                   ` (6 preceding siblings ...)
  2021-04-02 20:02 ` [PATCH v1 7/8] target/riscv: Remove an unused CASE_OP_32_64 macro Alistair Francis
@ 2021-04-02 20:03 ` Alistair Francis
  2021-04-06 14:57   ` Richard Henderson
  7 siblings, 1 reply; 24+ messages in thread
From: Alistair Francis @ 2021-04-02 20:03 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: alistair.francis, bmeng.cn, palmer, alistair23

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/insn16-32.decode | 24 ++++++++++++++++++++++++
 target/riscv/insn16-64.decode | 31 +++++++++++++++++++++++++++++++
 target/riscv/translate.c      | 18 +++++++++++++++++-
 target/riscv/meson.build      |  7 +++++--
 4 files changed, 77 insertions(+), 3 deletions(-)

diff --git a/target/riscv/insn16-32.decode b/target/riscv/insn16-32.decode
index 0819b17028..f83f43e955 100644
--- a/target/riscv/insn16-32.decode
+++ b/target/riscv/insn16-32.decode
@@ -16,6 +16,30 @@
 # You should have received a copy of the GNU General Public License along with
 # this program.  If not, see <http://www.gnu.org/licenses/>.
 
+# Fields imported from insn16.decode:
+%rd        7:5
+%rs1_3     7:3                !function=ex_rvc_register
+%rs2_3     2:3                !function=ex_rvc_register
+%rs2_5     2:5
+
+# Immediates imported from insn16.decode:
+%uimm_cl_w     5:1 10:3 6:1       !function=ex_shift_2
+%uimm_6bit_lw 2:2 12:1 4:3           !function=ex_shift_2
+%uimm_6bit_sw 7:2 9:4                !function=ex_shift_2
+%imm_cj        12:s1 8:1 9:2 6:1 7:1 2:1 11:1 3:3 !function=ex_shift_1
+
+# Argument sets imported from insn16.decode:
+&j         imm rd       !extern
+&i         imm rs1 rd   !extern
+&s         imm rs1 rs2  !extern
+
+# Formats 16 imported from insn16.decode:
+@cl_w      ... ... ... .. ... .. &i      imm=%uimm_cl_w   rs1=%rs1_3  rd=%rs2_3
+@cj        ...    ........... .. &j      imm=%imm_cj
+@c_lwsp    ... . .....  ..... .. &i      imm=%uimm_6bit_lw rs1=2 %rd
+@c_swsp    ... . .....  ..... .. &s      imm=%uimm_6bit_sw rs1=2 rs2=%rs2_5
+@cs_w      ... ... ... .. ... .. &s      imm=%uimm_cl_w   rs1=%rs1_3  rs2=%rs2_3
+
 # *** RV32C Standard Extension (Quadrant 0) ***
 flw               011  ... ... .. ... 00 @cl_w
 fsw               111  ... ... .. ... 00 @cs_w
diff --git a/target/riscv/insn16-64.decode b/target/riscv/insn16-64.decode
index 672e1e916f..dbef1e5365 100644
--- a/target/riscv/insn16-64.decode
+++ b/target/riscv/insn16-64.decode
@@ -16,6 +16,37 @@
 # You should have received a copy of the GNU General Public License along with
 # this program.  If not, see <http://www.gnu.org/licenses/>.
 
+# Fields imported from insn16.decode:
+%rd        7:5
+%rs1_3     7:3                !function=ex_rvc_register
+%rs2_3     2:3                !function=ex_rvc_register
+%rs2_5     2:5
+
+# Immediates imported from insn16.decode:
+%imm_ci        12:s1 2:5
+%uimm_cl_d     5:2 10:3           !function=ex_shift_3
+%uimm_6bit_ld 2:3 12:1 5:2           !function=ex_shift_3
+%uimm_6bit_lw 2:2 12:1 4:3           !function=ex_shift_2
+%uimm_6bit_sd 7:3 10:3               !function=ex_shift_3
+
+# Argument sets imported from insn16.decode:
+&empty                  !extern
+&r         rd rs1 rs2   !extern
+&i         imm rs1 rd   !extern
+&s         imm rs1 rs2  !extern
+&j         imm rd       !extern
+&b         imm rs2 rs1  !extern
+&u         imm rd       !extern
+&shift     shamt rs1 rd !extern
+
+# Formats 16 imported from insn16.decode:
+@ci        ... . ..... .....  .. &i      imm=%imm_ci      rs1=%rd     %rd
+@cl_d      ... ... ... .. ... .. &i      imm=%uimm_cl_d   rs1=%rs1_3  rd=%rs2_3
+@cs_d      ... ... ... .. ... .. &s      imm=%uimm_cl_d   rs1=%rs1_3  rs2=%rs2_3
+@cs_2      ... ... ... .. ... .. &r      rs2=%rs2_3       rs1=%rs1_3  rd=%rs1_3
+@c_ldsp    ... . .....  ..... .. &i      imm=%uimm_6bit_ld rs1=2 %rd
+@c_sdsp    ... . .....  ..... .. &s      imm=%uimm_6bit_sd rs1=2 rs2=%rs2_5
+
 # *** RV64C Standard Extension (Quadrant 0) ***
 ld                011  ... ... .. ... 00 @cl_d
 sd                111  ... ... .. ... 00 @cs_d
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 4af55deaea..9a93c77fd6 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -602,6 +602,10 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
 
 /* Include the auto-generated decoder for 16 bit insn */
 #include "decode-insn16.c.inc"
+#include "decode-insn16-32.c.inc"
+#ifdef TARGET_RISCV64
+# include "decode-insn16-64.c.inc"
+#endif
 
 static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
 {
@@ -612,7 +616,19 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
         } else {
             ctx->pc_succ_insn = ctx->base.pc_next + 2;
             if (!decode_insn16(ctx, opcode)) {
-                gen_exception_illegal(ctx);
+                if (riscv_cpu_is_32bit(env)) {
+                    if (!decode_insn16_32(ctx, opcode)) {
+                        gen_exception_illegal(ctx);
+                    }
+                } else {
+#ifdef TARGET_RISCV64
+                    if (!decode_insn16_64(ctx, opcode)) {
+                        gen_exception_illegal(ctx);
+                    }
+#else
+                    gen_exception_illegal(ctx);
+#endif
+                }
             }
         }
     } else {
diff --git a/target/riscv/meson.build b/target/riscv/meson.build
index 88ab850682..d17b478120 100644
--- a/target/riscv/meson.build
+++ b/target/riscv/meson.build
@@ -1,12 +1,15 @@
 # FIXME extra_args should accept files()
 dir = meson.current_source_dir()
 gen32 = [
-  decodetree.process('insn16.decode', extra_args: [dir / 'insn16-32.decode', '--static-decode=decode_insn16', '--insnwidth=16']),
+  decodetree.process('insn16.decode', extra_args: ['--static-decode=decode_insn16', '--insnwidth=16']),
+  decodetree.process('insn16-32.decode', extra_args: ['--static-decode=decode_insn16_32', '--insnwidth=16']),
   decodetree.process('insn32.decode', extra_args: '--static-decode=decode_insn32'),
 ]
 
 gen64 = [
-  decodetree.process('insn16.decode', extra_args: [dir / 'insn16-64.decode', '--static-decode=decode_insn16', '--insnwidth=16']),
+  decodetree.process('insn16.decode', extra_args: ['--static-decode=decode_insn16', '--insnwidth=16']),
+  decodetree.process('insn16-32.decode', extra_args: ['--static-decode=decode_insn16_32', '--insnwidth=16']),
+  decodetree.process('insn16-64.decode', extra_args: ['--static-decode=decode_insn16_64', '--insnwidth=16']),
   decodetree.process('insn32.decode', extra_args: [dir / 'insn32-64.decode', '--static-decode=decode_insn32']),
 ]
 
-- 
2.31.0



^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v1 8/8] target/riscv: Include RV32 instructions in RV64 build
  2021-04-02 20:03 ` [PATCH v1 8/8] target/riscv: Include RV32 instructions in RV64 build Alistair Francis
@ 2021-04-06 14:57   ` Richard Henderson
  0 siblings, 0 replies; 24+ messages in thread
From: Richard Henderson @ 2021-04-06 14:57 UTC (permalink / raw)
  To: Alistair Francis, qemu-devel, qemu-riscv; +Cc: alistair23, bmeng.cn, palmer

On 4/2/21 1:03 PM, Alistair Francis wrote:
> Signed-off-by: Alistair Francis<alistair.francis@wdc.com>
> ---
>   target/riscv/insn16-32.decode | 24 ++++++++++++++++++++++++
>   target/riscv/insn16-64.decode | 31 +++++++++++++++++++++++++++++++
>   target/riscv/translate.c      | 18 +++++++++++++++++-
>   target/riscv/meson.build      |  7 +++++--
>   4 files changed, 77 insertions(+), 3 deletions(-)

Having these split out from the main decode file was a cool trick when we were 
sure that we never needed to support both.  Now, I think it would be better to 
merge the patterns back in to the main insn16.decode file.

You'd begin by adding a REQUIRE_64BIT(ctx) macro, much like the other REQUIRE 
macros in translate.c, using the translate-local is_32bit() function as 
discussed earlier.

You add this to all of the trans_* functions that are currently #ifdef 
TARGET_RISCV64, and merge the insn32-64.decode bits in first.

There may well be an issue of missing helper functions, particularly when it 
comes to RVF/RVD/RVV.  There's a suggestion that I made for Claudio for i386 
that may help here:

https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg08595.html

where you add stubs that allow the gen_helper_foo() function to be declared, 
insist that it be optimized away, and reduce to an abort with -O0.

Only after insn32.decode is complete, do insn16.decode.  That's because many of 
the insns here will need the REQUIRE_64BIT in place for correctness.

E.g.

{
   ld          011  ... ... .. ... 00 @cl_d
   flw         011  ... ... .. ... 00 @cl_w
}

Since ld already has REQUIRE_64BIT, for is_32bit it returns false, and we fall 
into the flw code.

There will be one extra helper required, when the 64-bit RVC insn has a set of 
illegal operands:

static bool trans_c64_illegal(DisasContext *ctx, arg_empty *a)
{
     REQUIRE_64BIT(ctx);
     return trans_illegal(ctx, a);
}

for use with quadrant 1:

{
   c64_illegal 001 -  00000  ----- 01 # c.addiw, RES rd=0
   addiw       001 .  .....  ..... 01 @ci
   jal         001     ........... 01 @cj    rd=1  # C.JAL

}

and quadrant 2:

{
   c64_illegal 011 -  00000  ----- 10 # c.ldsp, RES rd=0
   ld          011 .  .....  ..... 10 @c_ldsp
   flw         011 .  .....  ..... 10 @c_lwsp
}

r~

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2021-04-12  9:18 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-02 20:02 [PATCH v1 0/8] RISC-V: Steps towards running 32-bit guests on Alistair Francis
2021-04-02 20:02 ` [PATCH v1 1/8] target/riscv: Remove the hardcoded RVXLEN macro Alistair Francis
2021-04-05 14:48   ` Richard Henderson
2021-04-12  9:10   ` Bin Meng
2021-04-02 20:02 ` [PATCH v1 2/8] target/riscv: Remove the hardcoded SSTATUS_SD macro Alistair Francis
2021-04-05 14:49   ` Richard Henderson
2021-04-12  9:10   ` Bin Meng
2021-04-02 20:02 ` [PATCH v1 3/8] target/riscv: Remove the hardcoded HGATP_MODE macro Alistair Francis
2021-04-05 14:54   ` Richard Henderson
2021-04-02 20:02 ` [PATCH v1 4/8] target/riscv: Remove the hardcoded MSTATUS_SD macro Alistair Francis
2021-04-05 15:10   ` Richard Henderson
2021-04-07 17:11     ` Alistair Francis
2021-04-08 15:20     ` Alistair Francis
2021-04-08 18:51       ` Richard Henderson
2021-04-02 20:02 ` [PATCH v1 5/8] target/riscv: Remove the hardcoded SATP_MODE macro Alistair Francis
2021-04-05 15:14   ` Richard Henderson
2021-04-02 20:02 ` [PATCH v1 6/8] target/riscv: Remove the unused HSTATUS_WPRI macro Alistair Francis
2021-04-05 15:15   ` Richard Henderson
2021-04-12  9:10   ` Bin Meng
2021-04-02 20:02 ` [PATCH v1 7/8] target/riscv: Remove an unused CASE_OP_32_64 macro Alistair Francis
2021-04-05 15:15   ` Richard Henderson
2021-04-12  9:10   ` Bin Meng
2021-04-02 20:03 ` [PATCH v1 8/8] target/riscv: Include RV32 instructions in RV64 build Alistair Francis
2021-04-06 14:57   ` Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).