[Qemu-devel] [PATCH v1 5/8] RISC-V: Add priv_ver to DisasContext

Alistair Francis posted 8 patches 6 years, 9 months ago
Maintainers: Michael Clark <mjc@sifive.com>, Palmer Dabbelt <palmer@sifive.com>, Alistair Francis <Alistair.Francis@wdc.com>, Sagar Karandikar <sagark@eecs.berkeley.edu>, Riku Voipio <riku.voipio@iki.fi>, Laurent Vivier <laurent@vivier.eu>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
[Qemu-devel] [PATCH v1 5/8] RISC-V: Add priv_ver to DisasContext
Posted by Alistair Francis 6 years, 9 months ago
The gen methods should access state from DisasContext. Add priv_ver
field to the DisasContext struct.

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

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 0581b3c1f7..833adf1d6f 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -43,6 +43,7 @@ typedef struct DisasContext {
     DisasContextBase base;
     /* pc_succ_insn points to the instruction following base.pc_next */
     target_ulong pc_succ_insn;
+    target_ulong priv_ver;
     uint32_t opcode;
     uint32_t mstatus_fs;
     uint32_t mem_idx;
@@ -1330,7 +1331,7 @@ static void gen_system(CPURISCVState *env, DisasContext *ctx, uint32_t opc,
 #ifndef CONFIG_USER_ONLY
     /* Extract funct7 value and check whether it matches SFENCE.VMA */
     if ((opc == OPC_RISC_ECALL) && ((csr >> 5) == 9)) {
-        if (env->priv_ver == PRIV_VERSION_1_10_0) {
+        if (ctx->priv_ver == PRIV_VERSION_1_10_0) {
             /* sfence.vma */
             /* TODO: handle ASID specific fences */
             gen_helper_tlb_flush(cpu_env);
@@ -1384,7 +1385,7 @@ static void gen_system(CPURISCVState *env, DisasContext *ctx, uint32_t opc,
             gen_helper_wfi(cpu_env);
             break;
         case 0x104: /* SFENCE.VM */
-            if (env->priv_ver <= PRIV_VERSION_1_09_1) {
+            if (ctx->priv_ver <= PRIV_VERSION_1_09_1) {
                 gen_helper_tlb_flush(cpu_env);
             } else {
                 gen_exception_illegal(ctx);
@@ -1851,13 +1852,15 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx)
     }
 }
 
-static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
+static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cpu)
 {
     DisasContext *ctx = container_of(dcbase, DisasContext, base);
+    CPURISCVState *env = cpu->env_ptr;
 
     ctx->pc_succ_insn = ctx->base.pc_first;
     ctx->mem_idx = ctx->base.tb->flags & TB_FLAGS_MMU_MASK;
     ctx->mstatus_fs = ctx->base.tb->flags & TB_FLAGS_MSTATUS_FS;
+    ctx->priv_ver = env->priv_ver;
     ctx->frm = -1;  /* unknown rounding mode */
 }
 
-- 
2.19.1


Re: [Qemu-devel] [PATCH v1 5/8] RISC-V: Add priv_ver to DisasContext
Posted by Richard Henderson 6 years, 9 months ago
On 1/15/19 10:58 AM, Alistair Francis wrote:
> -static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
> +static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cpu)

Why change this?  I know there is variation in the naming, but my
preferred default mapping is CPUState *cs, RISCVCPU *cpu.

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


r~

Re: [Qemu-devel] [PATCH v1 5/8] RISC-V: Add priv_ver to DisasContext
Posted by Alistair Francis 6 years, 9 months ago
On Tue, Jan 15, 2019 at 2:24 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 1/15/19 10:58 AM, Alistair Francis wrote:
> > -static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
> > +static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cpu)
>
> Why change this?  I know there is variation in the naming, but my
> preferred default mapping is CPUState *cs, RISCVCPU *cpu.

Good point, I have changed it back to cs.

Alistair

>
> Otherwise,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
>
> r~

Re: [Qemu-devel] [PATCH v1 5/8] RISC-V: Add priv_ver to DisasContext
Posted by Palmer Dabbelt 6 years, 9 months ago
On Tue, 15 Jan 2019 14:25:44 PST (-0800), alistair23@gmail.com wrote:
> On Tue, Jan 15, 2019 at 2:24 PM Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> On 1/15/19 10:58 AM, Alistair Francis wrote:
>> > -static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
>> > +static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cpu)
>>
>> Why change this?  I know there is variation in the naming, but my
>> preferred default mapping is CPUState *cs, RISCVCPU *cpu.
>
> Good point, I have changed it back to cs.

I don't see a v2, so I'm just going to go ahead and squash in

    diff --git a/target/riscv/translate.c b/target/riscv/translate.c
    index 8593c2170af4..b7176cbf98e1 100644
    --- a/target/riscv/translate.c
    +++ b/target/riscv/translate.c
    @@ -2015,10 +2015,10 @@ static void decode_opc(DisasContext *ctx)
         }
     }
    
    -static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cpu)
    +static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
     {
         DisasContext *ctx = container_of(dcbase, DisasContext, base);
    -    CPURISCVState *env = cpu->env_ptr;
    +    CPURISCVState *env = cs->env_ptr;
    
         ctx->pc_succ_insn = ctx->base.pc_first;
         ctx->mem_idx = ctx->base.tb->flags & TB_FLAGS_MMU_MASK;

and add Richard's tag.

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

Re: [Qemu-devel] [PATCH v1 5/8] RISC-V: Add priv_ver to DisasContext
Posted by Alistair Francis 6 years, 9 months ago
On Thu, Jan 24, 2019 at 4:37 PM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Tue, 15 Jan 2019 14:25:44 PST (-0800), alistair23@gmail.com wrote:
> > On Tue, Jan 15, 2019 at 2:24 PM Richard Henderson
> > <richard.henderson@linaro.org> wrote:
> >>
> >> On 1/15/19 10:58 AM, Alistair Francis wrote:
> >> > -static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
> >> > +static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cpu)
> >>
> >> Why change this?  I know there is variation in the naming, but my
> >> preferred default mapping is CPUState *cs, RISCVCPU *cpu.
> >
> > Good point, I have changed it back to cs.
>
> I don't see a v2, so I'm just going to go ahead and squash in

Yeah, I was waiting for more comments before sending a v2 on such a
trivial change.

>
>     diff --git a/target/riscv/translate.c b/target/riscv/translate.c
>     index 8593c2170af4..b7176cbf98e1 100644
>     --- a/target/riscv/translate.c
>     +++ b/target/riscv/translate.c
>     @@ -2015,10 +2015,10 @@ static void decode_opc(DisasContext *ctx)
>          }
>      }
>
>     -static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cpu)
>     +static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
>      {
>          DisasContext *ctx = container_of(dcbase, DisasContext, base);
>     -    CPURISCVState *env = cpu->env_ptr;
>     +    CPURISCVState *env = cs->env_ptr;
>
>          ctx->pc_succ_insn = ctx->base.pc_first;
>          ctx->mem_idx = ctx->base.tb->flags & TB_FLAGS_MMU_MASK;
>
> and add Richard's tag.

Sounds good.

Alistair

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