[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 2/2] target/riscv/cpu_helper.c: Fix mxr bit behavior
From: |
Alistair Francis |
Subject: |
Re: [PATCH v2 2/2] target/riscv/cpu_helper.c: Fix mxr bit behavior |
Date: |
Wed, 22 Nov 2023 12:10:49 +1000 |
On Tue, Nov 21, 2023 at 6:53 PM Ivan Klokov <ivan.klokov@syntacore.com> wrote:
>
> According to RISCV Specification sect 9.5 on two stage translation when
> V=1 the vsstatus(mstatus in QEMU's terms) field MXR, which makes
> execute-only pages readable, only overrides VS-stage page protection.
> Setting MXR at HS-level(mstatus_hs), however, overrides both VS-stage
> and G-stage execute-only permissions.
>
> The hypervisor extension changes the behavior of MXR\MPV\MPRV bits.
> Due to RISCV Specification sect. 9.4.1 when MPRV=1, explicit memory
> accesses are translated and protected, and endianness is applied, as
> though the current virtualization mode were set to MPV and the current
> nominal privilege mode were set to MPP. vsstatus.MXR makes readable
> those pages marked executable at the VS translation stage.
>
> Fixes: 36a18664ba ("target/riscv: Implement second stage MMU")
>
> Signed-off-by: Ivan Klokov <ivan.klokov@syntacore.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/cpu_helper.c | 24 ++++++++++++++++++++----
> 1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 9ff0952e46..e7e23b34f4 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1032,13 +1032,29 @@ restart:
> prot |= PAGE_WRITE;
> }
> if (pte & PTE_X) {
> - bool mxr;
> + bool mxr = false;
>
> - if (first_stage == true) {
> + /*
> + * Use mstatus for first stage or for the second stage without
> + * virt_enabled (MPRV+MPV)
> + */
> + if (first_stage || !env->virt_enabled) {
> mxr = get_field(env->mstatus, MSTATUS_MXR);
> - } else {
> - mxr = get_field(env->vsstatus, MSTATUS_MXR);
> }
> +
> + /* MPRV+MPV case, check VSSTATUS */
> + if (first_stage && two_stage && !env->virt_enabled) {
> + mxr |= get_field(env->vsstatus, MSTATUS_MXR);
> + }
> +
> + /*
> + * Setting MXR at HS-level overrides both VS-stage and G-stage
> + * execute-only permissions
> + */
> + if (env->virt_enabled) {
> + mxr |= get_field(env->mstatus_hs, MSTATUS_MXR);
> + }
> +
> if (mxr) {
> prot |= PAGE_READ;
> }
> --
> 2.34.1
>
>