Re: [PATCH v2 1/5] target/riscv: Add a virtualised MMU Mode

From: Richard Henderson <richard.henderson@linaro.org>
To: Alistair Francis <alistair.francis@wdc.com>,
	qemu-devel@nongnu.org, qemu-riscv@nongnu.org
Cc: alistair23@gmail.com, bmeng.cn@gmail.com, palmer@dabbelt.com
Subject: Re: [PATCH v2 1/5] target/riscv: Add a virtualised MMU Mode
Date: Wed, 28 Oct 2020 08:13:17 -0700	[thread overview]
Message-ID: <f8c23bed-3b6c-e01d-cb04-ce945d509970@linaro.org> (raw)
In-Reply-To: <d0eeb9ea64462044a67f5b803d6cb62dd10e017a.1603896075.git.alistair.francis@wdc.com>

On 10/28/20 7:42 AM, Alistair Francis wrote:
> Add a new MMU mode that includes the current virt mode.
> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/cpu-param.h  | 10 +++++++++-
>  target/riscv/cpu.h        |  4 +++-
>  target/riscv/cpu_helper.c |  6 +++++-
>  3 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/target/riscv/cpu-param.h b/target/riscv/cpu-param.h
> index 664fc1d371..0db6e23140 100644
> --- a/target/riscv/cpu-param.h
> +++ b/target/riscv/cpu-param.h
> @@ -18,6 +18,14 @@
>  # define TARGET_VIRT_ADDR_SPACE_BITS 32 /* sv32 */
>  #endif
>  #define TARGET_PAGE_BITS 12 /* 4 KiB Pages */
> -#define NB_MMU_MODES 4
> +/*
> + * The current MMU Modes are:
> + *  - U  mode 0b000
> + *  - S  mode 0b001
> + *  - M  mode 0b011
> + *  - HU mode 0b100
> + *  - HS mode 0b101
> + */
> +#define NB_MMU_MODES 6
>  
>  #endif
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 87b68affa8..5d8e54c426 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -363,7 +363,9 @@ void QEMU_NORETURN riscv_raise_exception(CPURISCVState *env,
>  target_ulong riscv_cpu_get_fflags(CPURISCVState *env);
>  void riscv_cpu_set_fflags(CPURISCVState *env, target_ulong);
>  
> -#define TB_FLAGS_MMU_MASK   3
> +#define TB_FLAGS_MMU_MASK   7
> +#define TB_FLAGS_PRIV_MMU_MASK                3
> +#define TB_FLAGS_PRIV_HYP_ACCESS_MASK   (1 << 2)
>  #define TB_FLAGS_MSTATUS_FS MSTATUS_FS
>  
>  typedef CPURISCVState CPUArchState;
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 3eb3a034db..453e4c6d8a 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -30,6 +30,10 @@ int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
>  #ifdef CONFIG_USER_ONLY
>      return 0;
>  #else
> +    if (riscv_cpu_virt_enabled(env)) {
> +        return env->priv | TB_FLAGS_PRIV_HYP_ACCESS_MASK;
> +    }

This is wrong.  You only want to set this flag in response to one of the
hypervisor special instructions.  This is setting it any time virt is enabled.

r~