Re: [PATCH v5 1/2] target/riscv: separate priv from mmu_idx

[Richard Henderson <richard.henderson@linaro.org>]

On 3/23/23 22:41, Fei Wu wrote:
> @@ -762,7 +764,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
>        * (riscv_cpu_do_interrupt) is correct */
>       MemTxResult res;
>       MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
> -    int mode = mmu_idx & TB_FLAGS_PRIV_MMU_MASK;
> +    int mode = env->priv;

This is not ok.

Or rather, it's ok as an intermediate step before fixing the other mis-uses of the 
interface in get_physical_address.

The interface to be provided by TCGCPUOps.tlb_fill is that (modulo tlb flushes) mmu_idx 
details the state we want the access to have.  We describe that state with the comment you 
move in patch 2:

+/*
+ * The current MMU Modes are:
+ *  - U                 0b000
+ *  - S                 0b001
+ *  - S+SUM             0b010
+ *  - M                 0b011
+ *  - HLV/HLVX/HSV adds 0b100
+ */

Anything that's in those bits shouldn't be re-examined from env.

So, in the short-term I'll give you

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

But for the long-term, let's see if we can untangle the ugly mess. I'll collect my 
thoughts and post some patches for discussion -- I think that will be clearer than the 
prose I began to write here.


r~