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

[Jonathan Behrens]

I was trying to avoid forcing everyone to understand the Hypervisor
extension to develop for RISC-V.

I agree a full understanding of the hypervisor extension shouldn't be required. But if code is going to be dealing with supervisor mode registers, then whether the machine is in HS or VS mode is going to matter in a lot of cases. The cases where it doesn't matter are particularly troubling: it wouldn't be obvious from reading the code whether the author forgot about the possibility of register swapping or if the code actually works then too. Plus, for custom forks we can always add a one line comment above the register declarations saying you can just sxxx[NOVIRT] to access the normal supervisor registers unless you want to make sure your code works with the H-extension.

For example we don't have to check
virtualization status to dump the registers, we can just dump them and
know they are in the correct state (we do dump more is we have the
extension though). Image if someone adds a new way to dump registers,
I would like them not to care about if we are in V=1 or V=0 and they
can just dump them and know that the CSRs are correct.

This is almost a best case for being orthogonal to other features because you don't have to understand anything about how the registers impact machine execution in order to dump them.

But even here it is somewhat debatable whether a naive implementation would dump the "correct" versions of these registers. One view is that what you really want would be the values that M-mode code would see since it has the most objective view of the system. Another is that they are the values that would be returned if the the current operating mode tried to access them, or undefined if that access would trap. Maybe you even want to query the value for a specific mode independent of the current privilege level. If the behavior you want happens to match up to the chosen design, then yes a naive implementation that is oblivious the hypervisor extension will work without having to understand anything. However, I'd attribute that more to luck than elegance of a particular option.

Jonathan

On Tue, Jan 21, 2020 at 7:01 PM Alistair Francis <address@hidden> wrote:

On Tue, Jan 21, 2020 at 10:56 PM Jonathan Behrens <address@hidden> wrote:
>
> When I looked through the relevant code a few months ago, I couldn't find anywhere that could actually be agnostic to whether the real or virtual registers were in effect (other than emulating the actual CSR modification instructions). For almost all state, the VS behavior is filtered by HS-mode code. For example, you can grab satp in either mode but to properly do address translation you also have to factor in hgatp so you need to know the virtualization state anyway. Similarly, floating point co-proccessor state is tracked in two places with V=1 so that both the host and the guest can independently track dirty bits, but of course only the one in the "real" mstatus applies in non-virtualized mode.

So the idea is that if you aren't interested in the Hypervisor
extension you can just access the CSRs as usual (inside QEMU's source
code). That is you don't need to know anything about the Hypervisor
extension to add support for other extensions or to work on the RISC-V
target in QEMU.

I was trying to avoid forcing everyone to understand the Hypervisor
extension to develop for RISC-V. For example we don't have to check
virtulasition status to dump the registers, we can just dump them and
know they are in the correct state (we do dump more is we have the
extension though). Image if someone adds a new way to dump registers,
I would like them not to care about if we are in V=1 or V=0 and they
can just dump them and know that the CSRs are correct.

>
> The idea of using an array to track the two versions of registers seemed really elegant to me. If you know you want the host version you access element zero. If you know you want the guest version you access element one. And if you want the version that the running code would see you index by the virtualization mode. In any case, the choice indicates that you thought though which was the right option to use in that instance.

mstatus really is the only one that has any complications. The others
have a vs* version, which could be converted to an array but it
doesn't really matter as we just swap them.

Alistair

>
> Jonathan
>
> On Tue, Jan 21, 2020 at 6:02 AM Alistair Francis <address@hidden> wrote:
>>
>> On Wed, Jan 8, 2020 at 11:30 AM Palmer Dabbelt <address@hidden> wrote:
>> >
>> > 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.
>>
>> I think their implementation is different so it doesn't apply the same here.
>>
>> My main concern is that due to the modularity of RISC-V I don't expect
>> all future developers to keep track of the Hypervisor extensions. This
>> way we always have the correct state in the registers.
>>
>> There is only one pointer variable left, so we could drop the pointer
>> swapping part, but for now it's still here.
>>
>> Alistair
>>
>> >
>> > > 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);
>>