Re: [Qemu-riscv] [Qemu-devel] [PATCH 0/2] target/riscv: Bugfixes found in decodetree conversion

[Palmer Dabbelt]

On Thu, 08 Nov 2018 09:29:26 PST (-0800), Bastian Koppelmann wrote:
> On 11/8/18 4:53 PM, Richard Henderson wrote:
> > On 11/8/18 1:06 PM, Bastian Koppelmann wrote:
> > > while going through the reviews of the riscv-decodetree patches, two bugs came
> > > up that I fix here. There is one more problem [1] mentioned by Richard but
> > > I don't have the time to investigate it further.
> > >
> > > [1] https://patchwork.kernel.org/patch/10650293/
> > That one's not a bug, but an optimization.
> >
> > The other bug mentioned is shrw and shaw not sign-extending the result.
>
>
> That was a bug I introduced during the conversion to decodetree.

My understand of that patch feedback is that there are two issues:

* We don't take advantage of the ordering bits on fences, which could allow us 
 to emit less conservative fences.  This would presumably increase 
 performance.
* We treat fences as NOPs under "#ifndef CONFIG_USER_ONLY", which is a bug.

Am I wrong about that second one?  I think the fix should look something like 
this, which I haven't even tried to compile

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 18d7b6d1471d..624d1c679a84 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -1766,7 +1766,6 @@ static void decode_RV32_64G(CPURISCVState *env, 
DisasContext *ctx)
                     GET_RM(ctx->opcode));
        break;
    case OPC_RISC_FENCE:
-#ifndef CONFIG_USER_ONLY
        if (ctx->opcode & 0x1000) {
            /* FENCE_I is a no-op in QEMU,
             * however we need to end the translation block */
@@ -1777,7 +1776,6 @@ static void decode_RV32_64G(CPURISCVState *env, 
DisasContext *ctx)
            /* FENCE is a full memory barrier. */
            tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC);
        }
-#endif
        break;
    case OPC_RISC_SYSTEM:
        gen_system(env, ctx, MASK_OP_SYSTEM(ctx->opcode), rd, rs1,