qemu-devel
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v1 15/36] target/riscv: Convert mstatus to pointers

From: Palmer Dabbelt
Subject: Re: [PATCH v1 15/36] target/riscv: Convert mstatus to pointers
Date: Tue, 07 Jan 2020 17:30:52 -0800 (PST) 
On Mon, 09 Dec 2019 10:11:19 PST (-0800), Alistair Francis wrote:

To handle the new Hypervisor CSR register aliasing let's use pointers.


For some reason I thought we were making this explicit?  In other words,
requiring that all callers provide which privilege mode they're using when
accessing these CSRs, as opposed to swapping around pointers.  I don't actually
care that much, but IIRC when we were talking with the ARM guys at Plumbers
they were pretty adament that would end up being a much cleaner implementation
as they'd tried this way and later changed over.


Signed-off-by: Alistair Francis <address@hidden>
---
 target/riscv/cpu.c        | 11 +++++++++--
 target/riscv/cpu.h        |  9 ++++++++-
 target/riscv/cpu_helper.c | 30 +++++++++++++++---------------
 target/riscv/csr.c        | 20 ++++++++++----------
 target/riscv/op_helper.c  | 14 +++++++-------
 5 files changed, 49 insertions(+), 35 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index a07c5689b3..e61cf46a73 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -236,7 +236,7 @@ static void riscv_cpu_dump_state(CPUState *cs, FILE *f, int 
flags)
     qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "pc      ", env->pc);
 #ifndef CONFIG_USER_ONLY
     qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mhartid ", env->mhartid);
-    qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mstatus ", env->mstatus);
+    qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "mstatus ", *env->mstatus);
     if (riscv_has_ext(env, RVH)) {
         qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "hstatus ", env->hstatus);
         qemu_fprintf(f, " %s " TARGET_FMT_lx "\n", "vsstatus ", env->vsstatus);
@@ -336,7 +336,7 @@ static void riscv_cpu_reset(CPUState *cs)
     mcc->parent_reset(cs);
 #ifndef CONFIG_USER_ONLY
     env->priv = PRV_M;
-    env->mstatus &= ~(MSTATUS_MIE | MSTATUS_MPRV);
+    *env->mstatus &= ~(MSTATUS_MIE | MSTATUS_MPRV);
     env->mcause = 0;
     env->pc = env->resetvec;
 #endif
@@ -465,8 +465,15 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 static void riscv_cpu_init(Object *obj)
 {
     RISCVCPU *cpu = RISCV_CPU(obj);
+#ifndef CONFIG_USER_ONLY
+    CPURISCVState *env = &cpu->env;
+#endif

     cpu_set_cpustate_pointers(cpu);
+
+#ifndef CONFIG_USER_ONLY
+    env->mstatus = &env->mstatus_novirt;
+#endif
 }

 static const VMStateDescription vmstate_riscv_cpu = {
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 21ae5a8b19..9dc8303c62 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -122,7 +122,7 @@ struct CPURISCVState {
     target_ulong resetvec;

     target_ulong mhartid;
-    target_ulong mstatus;
+    target_ulong *mstatus;

     target_ulong mip;
     uint32_t miclaim;
@@ -145,6 +145,13 @@ struct CPURISCVState {
     target_ulong mcause;
     target_ulong mtval;  /* since: priv-1.10.0 */

+    /* The following registers are the "real" versions that the pointer
+     * versions point to. These should never be used unless you know what you
+     * are doing. To access these use the pointer versions instead. This is
+     * required to handle the Hypervisor register swapping.
+     */
+    target_ulong mstatus_novirt;
+
     /* Hypervisor CSRs */
     target_ulong hstatus;
     target_ulong hedeleg;
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index b00f66824a..9684da7f7d 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -37,8 +37,8 @@ int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
 #ifndef CONFIG_USER_ONLY
 static int riscv_cpu_local_irq_pending(CPURISCVState *env)
 {
-    target_ulong mstatus_mie = get_field(env->mstatus, MSTATUS_MIE);
-    target_ulong mstatus_sie = get_field(env->mstatus, MSTATUS_SIE);
+    target_ulong mstatus_mie = get_field(*env->mstatus, MSTATUS_MIE);
+    target_ulong mstatus_sie = get_field(*env->mstatus, MSTATUS_SIE);
     target_ulong pending = env->mip & env->mie;
     target_ulong mie = env->priv < PRV_M || (env->priv == PRV_M && 
mstatus_mie);
     target_ulong sie = env->priv < PRV_S || (env->priv == PRV_S && 
mstatus_sie);
@@ -75,7 +75,7 @@ bool riscv_cpu_exec_interrupt(CPUState *cs, int 
interrupt_request)
 /* Return true is floating point support is currently enabled */
 bool riscv_cpu_fp_enabled(CPURISCVState *env)
 {
-    if (env->mstatus & MSTATUS_FS) {
+    if (*env->mstatus & MSTATUS_FS) {
         return true;
     }

@@ -198,8 +198,8 @@ static int get_physical_address(CPURISCVState *env, hwaddr 
*physical,
     int mode = mmu_idx;

     if (mode == PRV_M && access_type != MMU_INST_FETCH) {
-        if (get_field(env->mstatus, MSTATUS_MPRV)) {
-            mode = get_field(env->mstatus, MSTATUS_MPP);
+        if (get_field(*env->mstatus, MSTATUS_MPRV)) {
+            mode = get_field(*env->mstatus, MSTATUS_MPP);
         }
     }

@@ -213,11 +213,11 @@ static int get_physical_address(CPURISCVState *env, 
hwaddr *physical,

     hwaddr base;
     int levels, ptidxbits, ptesize, vm, sum;
-    int mxr = get_field(env->mstatus, MSTATUS_MXR);
+    int mxr = get_field(*env->mstatus, MSTATUS_MXR);

     if (env->priv_ver >= PRIV_VERSION_1_10_0) {
         base = (hwaddr)get_field(env->satp, SATP_PPN) << PGSHIFT;
-        sum = get_field(env->mstatus, MSTATUS_SUM);
+        sum = get_field(*env->mstatus, MSTATUS_SUM);
         vm = get_field(env->satp, SATP_MODE);
         switch (vm) {
         case VM_1_10_SV32:
@@ -237,8 +237,8 @@ static int get_physical_address(CPURISCVState *env, hwaddr 
*physical,
         }
     } else {
         base = (hwaddr)(env->sptbr) << PGSHIFT;
-        sum = !get_field(env->mstatus, MSTATUS_PUM);
-        vm = get_field(env->mstatus, MSTATUS_VM);
+        sum = !get_field(*env->mstatus, MSTATUS_PUM);
+        vm = get_field(*env->mstatus, MSTATUS_VM);
         switch (vm) {
         case VM_1_09_SV32:
           levels = 2; ptidxbits = 10; ptesize = 4; break;
@@ -492,8 +492,8 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int 
size,
     ret = get_physical_address(env, &pa, &prot, address, access_type, mmu_idx);

     if (mode == PRV_M && access_type != MMU_INST_FETCH) {
-        if (get_field(env->mstatus, MSTATUS_MPRV)) {
-            mode = get_field(env->mstatus, MSTATUS_MPP);
+        if (get_field(*env->mstatus, MSTATUS_MPRV)) {
+            mode = get_field(*env->mstatus, MSTATUS_MPP);
         }
     }

@@ -599,12 +599,12 @@ void riscv_cpu_do_interrupt(CPUState *cs)
     if (env->priv <= PRV_S &&
             cause < TARGET_LONG_BITS && ((deleg >> cause) & 1)) {
         /* handle the trap in S-mode */
-        target_ulong s = env->mstatus;
+        target_ulong s = *env->mstatus;
         s = set_field(s, MSTATUS_SPIE, env->priv_ver >= PRIV_VERSION_1_10_0 ?
             get_field(s, MSTATUS_SIE) : get_field(s, MSTATUS_UIE << 
env->priv));
         s = set_field(s, MSTATUS_SPP, env->priv);
         s = set_field(s, MSTATUS_SIE, 0);
-        env->mstatus = s;
+        *env->mstatus = s;
         env->scause = cause | ((target_ulong)async << (TARGET_LONG_BITS - 1));
         env->sepc = env->pc;
         env->sbadaddr = tval;
@@ -613,12 +613,12 @@ void riscv_cpu_do_interrupt(CPUState *cs)
         riscv_cpu_set_mode(env, PRV_S);
     } else {
         /* handle the trap in M-mode */
-        target_ulong s = env->mstatus;
+        target_ulong s = *env->mstatus;
         s = set_field(s, MSTATUS_MPIE, env->priv_ver >= PRIV_VERSION_1_10_0 ?
             get_field(s, MSTATUS_MIE) : get_field(s, MSTATUS_UIE << 
env->priv));
         s = set_field(s, MSTATUS_MPP, env->priv);
         s = set_field(s, MSTATUS_MIE, 0);
-        env->mstatus = s;
+        *env->mstatus = s;
         env->mcause = cause | ~(((target_ulong)-1) >> async);
         env->mepc = env->pc;
         env->mbadaddr = tval;
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 74e911af08..a4b598d49a 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -136,7 +136,7 @@ static int write_fflags(CPURISCVState *env, int csrno, 
target_ulong val)
     if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
         return -1;
     }
-    env->mstatus |= MSTATUS_FS;
+    *env->mstatus |= MSTATUS_FS;
 #endif
     riscv_cpu_set_fflags(env, val & (FSR_AEXC >> FSR_AEXC_SHIFT));
     return 0;
@@ -159,7 +159,7 @@ static int write_frm(CPURISCVState *env, int csrno, 
target_ulong val)
     if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
         return -1;
     }
-    env->mstatus |= MSTATUS_FS;
+    *env->mstatus |= MSTATUS_FS;
 #endif
     env->frm = val & (FSR_RD >> FSR_RD_SHIFT);
     return 0;
@@ -183,7 +183,7 @@ static int write_fcsr(CPURISCVState *env, int csrno, 
target_ulong val)
     if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
         return -1;
     }
-    env->mstatus |= MSTATUS_FS;
+    *env->mstatus |= MSTATUS_FS;
 #endif
     env->frm = (val & FSR_RD) >> FSR_RD_SHIFT;
     riscv_cpu_set_fflags(env, (val & FSR_AEXC) >> FSR_AEXC_SHIFT);
@@ -313,7 +313,7 @@ static int read_mhartid(CPURISCVState *env, int csrno, 
target_ulong *val)
 /* Machine Trap Setup */
 static int read_mstatus(CPURISCVState *env, int csrno, target_ulong *val)
 {
-    *val = env->mstatus;
+    *val = *env->mstatus;
     return 0;
 }

@@ -325,7 +325,7 @@ static int validate_vm(CPURISCVState *env, target_ulong vm)

 static int write_mstatus(CPURISCVState *env, int csrno, target_ulong val)
 {
-    target_ulong mstatus = env->mstatus;
+    target_ulong mstatus = *env->mstatus;
     target_ulong mask = 0;
     int dirty;

@@ -365,7 +365,7 @@ static int write_mstatus(CPURISCVState *env, int csrno, 
target_ulong val)
              ((mstatus & MSTATUS_FS) == MSTATUS_FS)) |
             ((mstatus & MSTATUS_XS) == MSTATUS_XS);
     mstatus = set_field(mstatus, MSTATUS_SD, dirty);
-    env->mstatus = mstatus;
+    *env->mstatus = mstatus;

     return 0;
 }
@@ -614,7 +614,7 @@ static int read_sstatus(CPURISCVState *env, int csrno, 
target_ulong *val)
 {
     target_ulong mask = ((env->priv_ver >= PRIV_VERSION_1_10_0) ?
                          sstatus_v1_10_mask : sstatus_v1_9_mask);
-    *val = env->mstatus & mask;
+    *val = *env->mstatus & mask;
     return 0;
 }

@@ -622,7 +622,7 @@ static int write_sstatus(CPURISCVState *env, int csrno, 
target_ulong val)
 {
     target_ulong mask = ((env->priv_ver >= PRIV_VERSION_1_10_0) ?
                          sstatus_v1_10_mask : sstatus_v1_9_mask);
-    target_ulong newval = (env->mstatus & ~mask) | (val & mask);
+    target_ulong newval = (*env->mstatus & ~mask) | (val & mask);
     return write_mstatus(env, CSR_MSTATUS, newval);
 }

@@ -737,7 +737,7 @@ static int read_satp(CPURISCVState *env, int csrno, 
target_ulong *val)
     if (!riscv_feature(env, RISCV_FEATURE_MMU)) {
         *val = 0;
     } else if (env->priv_ver >= PRIV_VERSION_1_10_0) {
-        if (env->priv == PRV_S && get_field(env->mstatus, MSTATUS_TVM)) {
+        if (env->priv == PRV_S && get_field(*env->mstatus, MSTATUS_TVM)) {
             return -1;
         } else {
             *val = env->satp;
@@ -762,7 +762,7 @@ static int write_satp(CPURISCVState *env, int csrno, 
target_ulong val)
         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)) {
+        if (env->priv == PRV_S && get_field(*env->mstatus, MSTATUS_TVM)) {
             return -1;
         } else {
             if((val ^ env->satp) & SATP_ASID) {
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 331cc36232..d150551bc9 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -83,11 +83,11 @@ target_ulong helper_sret(CPURISCVState *env, target_ulong 
cpu_pc_deb)
     }

     if (env->priv_ver >= PRIV_VERSION_1_10_0 &&
-        get_field(env->mstatus, MSTATUS_TSR)) {
+        get_field(*env->mstatus, MSTATUS_TSR)) {
         riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
     }

-    target_ulong mstatus = env->mstatus;
+    target_ulong mstatus = *env->mstatus;
     target_ulong prev_priv = get_field(mstatus, MSTATUS_SPP);
     mstatus = set_field(mstatus,
         env->priv_ver >= PRIV_VERSION_1_10_0 ?
@@ -96,7 +96,7 @@ target_ulong helper_sret(CPURISCVState *env, target_ulong 
cpu_pc_deb)
     mstatus = set_field(mstatus, MSTATUS_SPIE, 0);
     mstatus = set_field(mstatus, MSTATUS_SPP, PRV_U);
     riscv_cpu_set_mode(env, prev_priv);
-    env->mstatus = mstatus;
+    *env->mstatus = mstatus;

     return retpc;
 }
@@ -112,7 +112,7 @@ target_ulong helper_mret(CPURISCVState *env, target_ulong 
cpu_pc_deb)
         riscv_raise_exception(env, RISCV_EXCP_INST_ADDR_MIS, GETPC());
     }

-    target_ulong mstatus = env->mstatus;
+    target_ulong mstatus = *env->mstatus;
     target_ulong prev_priv = get_field(mstatus, MSTATUS_MPP);
     mstatus = set_field(mstatus,
         env->priv_ver >= PRIV_VERSION_1_10_0 ?
@@ -121,7 +121,7 @@ target_ulong helper_mret(CPURISCVState *env, target_ulong 
cpu_pc_deb)
     mstatus = set_field(mstatus, MSTATUS_MPIE, 0);
     mstatus = set_field(mstatus, MSTATUS_MPP, PRV_U);
     riscv_cpu_set_mode(env, prev_priv);
-    env->mstatus = mstatus;
+    *env->mstatus = mstatus;

     return retpc;
 }
@@ -132,7 +132,7 @@ void helper_wfi(CPURISCVState *env)

     if (env->priv == PRV_S &&
         env->priv_ver >= PRIV_VERSION_1_10_0 &&
-        get_field(env->mstatus, MSTATUS_TW)) {
+        get_field(*env->mstatus, MSTATUS_TW)) {
         riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
     } else {
         cs->halted = 1;
@@ -147,7 +147,7 @@ void helper_tlb_flush(CPURISCVState *env)
     if (!(env->priv >= PRV_S) ||
         (env->priv == PRV_S &&
          env->priv_ver >= PRIV_VERSION_1_10_0 &&
-         get_field(env->mstatus, MSTATUS_TVM))) {
+         get_field(*env->mstatus, MSTATUS_TVM))) {
         riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, GETPC());
     } else {
         tlb_flush(cs);
reply via email to
[Prev in Thread] Current Thread [Next in Thread]