[v1] Fix mmu translation with H extension

[PATCH 1/2] target/riscv/cpu_helper.c: Invalid exception on MMU translation stage

Posted by Ivan Klokov 1 year ago

According to RISCV priveleged spec sect. 5.3.2 Virtual Address Translation Process
access-fault exceptions may raise only after PMA/PMP check. Current implementation
generates an acces-fault for mbare mode even if there were no PMA/PMP errors.
This patch removes the erroneous MMU mode check and generates an access-fault
exception based on the pmp_violation flag only.

Signed-off-by: Ivan Klokov <ivan.klokov@syntacore.com>
---
 target/riscv/cpu_helper.c | 30 +++++++-----------------------
 1 file changed, 7 insertions(+), 23 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index b7af69de53..9ff0952e46 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1143,47 +1143,31 @@ static void raise_mmu_exception(CPURISCVState *env, target_ulong address,
                                 bool two_stage_indirect)
 {
     CPUState *cs = env_cpu(env);
-    int page_fault_exceptions, vm;
-    uint64_t stap_mode;
-
-    if (riscv_cpu_mxl(env) == MXL_RV32) {
-        stap_mode = SATP32_MODE;
-    } else {
-        stap_mode = SATP64_MODE;
-    }
-
-    if (first_stage) {
-        vm = get_field(env->satp, stap_mode);
-    } else {
-        vm = get_field(env->hgatp, stap_mode);
-    }
-
-    page_fault_exceptions = vm != VM_1_10_MBARE && !pmp_violation;
 
     switch (access_type) {
     case MMU_INST_FETCH:
         if (env->virt_enabled && !first_stage) {
             cs->exception_index = RISCV_EXCP_INST_GUEST_PAGE_FAULT;
         } else {
-            cs->exception_index = page_fault_exceptions ?
-                RISCV_EXCP_INST_PAGE_FAULT : RISCV_EXCP_INST_ACCESS_FAULT;
+            cs->exception_index = pmp_violation ?
+                RISCV_EXCP_INST_ACCESS_FAULT : RISCV_EXCP_INST_PAGE_FAULT;
         }
         break;
     case MMU_DATA_LOAD:
         if (two_stage && !first_stage) {
             cs->exception_index = RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT;
         } else {
-            cs->exception_index = page_fault_exceptions ?
-                RISCV_EXCP_LOAD_PAGE_FAULT : RISCV_EXCP_LOAD_ACCESS_FAULT;
+            cs->exception_index = pmp_violation ?
+                RISCV_EXCP_LOAD_ACCESS_FAULT : RISCV_EXCP_LOAD_PAGE_FAULT;
         }
         break;
     case MMU_DATA_STORE:
         if (two_stage && !first_stage) {
             cs->exception_index = RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT;
         } else {
-            cs->exception_index = page_fault_exceptions ?
-                RISCV_EXCP_STORE_PAGE_FAULT :
-                RISCV_EXCP_STORE_AMO_ACCESS_FAULT;
+            cs->exception_index = pmp_violation ?
+                RISCV_EXCP_STORE_AMO_ACCESS_FAULT :
+                RISCV_EXCP_STORE_PAGE_FAULT;
         }
         break;
     default:
-- 
2.34.1

Re: [PATCH 1/2] target/riscv/cpu_helper.c: Invalid exception on MMU translation stage

Posted by Alistair Francis 1 year ago

On Mon, Nov 20, 2023 at 11:19 PM Ivan Klokov <ivan.klokov@syntacore.com> wrote:
>
> According to RISCV priveleged spec sect. 5.3.2 Virtual Address Translation Process
> access-fault exceptions may raise only after PMA/PMP check. Current implementation
> generates an acces-fault for mbare mode even if there were no PMA/PMP errors.
> This patch removes the erroneous MMU mode check and generates an access-fault
> exception based on the pmp_violation flag only.
>
> Signed-off-by: Ivan Klokov <ivan.klokov@syntacore.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/cpu_helper.c | 30 +++++++-----------------------
>  1 file changed, 7 insertions(+), 23 deletions(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index b7af69de53..9ff0952e46 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1143,47 +1143,31 @@ static void raise_mmu_exception(CPURISCVState *env, target_ulong address,
>                                  bool two_stage_indirect)
>  {
>      CPUState *cs = env_cpu(env);
> -    int page_fault_exceptions, vm;
> -    uint64_t stap_mode;
> -
> -    if (riscv_cpu_mxl(env) == MXL_RV32) {
> -        stap_mode = SATP32_MODE;
> -    } else {
> -        stap_mode = SATP64_MODE;
> -    }
> -
> -    if (first_stage) {
> -        vm = get_field(env->satp, stap_mode);
> -    } else {
> -        vm = get_field(env->hgatp, stap_mode);
> -    }
> -
> -    page_fault_exceptions = vm != VM_1_10_MBARE && !pmp_violation;
>
>      switch (access_type) {
>      case MMU_INST_FETCH:
>          if (env->virt_enabled && !first_stage) {
>              cs->exception_index = RISCV_EXCP_INST_GUEST_PAGE_FAULT;
>          } else {
> -            cs->exception_index = page_fault_exceptions ?
> -                RISCV_EXCP_INST_PAGE_FAULT : RISCV_EXCP_INST_ACCESS_FAULT;
> +            cs->exception_index = pmp_violation ?
> +                RISCV_EXCP_INST_ACCESS_FAULT : RISCV_EXCP_INST_PAGE_FAULT;
>          }
>          break;
>      case MMU_DATA_LOAD:
>          if (two_stage && !first_stage) {
>              cs->exception_index = RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT;
>          } else {
> -            cs->exception_index = page_fault_exceptions ?
> -                RISCV_EXCP_LOAD_PAGE_FAULT : RISCV_EXCP_LOAD_ACCESS_FAULT;
> +            cs->exception_index = pmp_violation ?
> +                RISCV_EXCP_LOAD_ACCESS_FAULT : RISCV_EXCP_LOAD_PAGE_FAULT;
>          }
>          break;
>      case MMU_DATA_STORE:
>          if (two_stage && !first_stage) {
>              cs->exception_index = RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT;
>          } else {
> -            cs->exception_index = page_fault_exceptions ?
> -                RISCV_EXCP_STORE_PAGE_FAULT :
> -                RISCV_EXCP_STORE_AMO_ACCESS_FAULT;
> +            cs->exception_index = pmp_violation ?
> +                RISCV_EXCP_STORE_AMO_ACCESS_FAULT :
> +                RISCV_EXCP_STORE_PAGE_FAULT;
>          }
>          break;
>      default:
> --
> 2.34.1
>
>

Re: [PATCH 1/2] target/riscv/cpu_helper.c: Invalid exception on MMU translation stage

Posted by Daniel Henrique Barboza 1 year ago


On 11/20/23 09:06, Ivan Klokov wrote:
> According to RISCV priveleged spec sect. 5.3.2 Virtual Address Translation Process

s/priveleged/privileged

> access-fault exceptions may raise only after PMA/PMP check. Current implementation
> generates an acces-fault for mbare mode even if there were no PMA/PMP errors.

s/acces-fault/access-fault

> This patch removes the erroneous MMU mode check and generates an access-fault
> exception based on the pmp_violation flag only.


Please add

Fixes: 1448689c7b ("target/riscv: Allow specifying MMU stage")

And we can push this fix for 8.2.

> 
> Signed-off-by: Ivan Klokov <ivan.klokov@syntacore.com>
> ---


Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

>   target/riscv/cpu_helper.c | 30 +++++++-----------------------
>   1 file changed, 7 insertions(+), 23 deletions(-)
> 
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index b7af69de53..9ff0952e46 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1143,47 +1143,31 @@ static void raise_mmu_exception(CPURISCVState *env, target_ulong address,
>                                   bool two_stage_indirect)
>   {
>       CPUState *cs = env_cpu(env);
> -    int page_fault_exceptions, vm;
> -    uint64_t stap_mode;
> -
> -    if (riscv_cpu_mxl(env) == MXL_RV32) {
> -        stap_mode = SATP32_MODE;
> -    } else {
> -        stap_mode = SATP64_MODE;
> -    }
> -
> -    if (first_stage) {
> -        vm = get_field(env->satp, stap_mode);
> -    } else {
> -        vm = get_field(env->hgatp, stap_mode);
> -    }
> -
> -    page_fault_exceptions = vm != VM_1_10_MBARE && !pmp_violation;
>   
>       switch (access_type) {
>       case MMU_INST_FETCH:
>           if (env->virt_enabled && !first_stage) {
>               cs->exception_index = RISCV_EXCP_INST_GUEST_PAGE_FAULT;
>           } else {
> -            cs->exception_index = page_fault_exceptions ?
> -                RISCV_EXCP_INST_PAGE_FAULT : RISCV_EXCP_INST_ACCESS_FAULT;
> +            cs->exception_index = pmp_violation ?
> +                RISCV_EXCP_INST_ACCESS_FAULT : RISCV_EXCP_INST_PAGE_FAULT;
>           }
>           break;
>       case MMU_DATA_LOAD:
>           if (two_stage && !first_stage) {
>               cs->exception_index = RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT;
>           } else {
> -            cs->exception_index = page_fault_exceptions ?
> -                RISCV_EXCP_LOAD_PAGE_FAULT : RISCV_EXCP_LOAD_ACCESS_FAULT;
> +            cs->exception_index = pmp_violation ?
> +                RISCV_EXCP_LOAD_ACCESS_FAULT : RISCV_EXCP_LOAD_PAGE_FAULT;
>           }
>           break;
>       case MMU_DATA_STORE:
>           if (two_stage && !first_stage) {
>               cs->exception_index = RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT;
>           } else {
> -            cs->exception_index = page_fault_exceptions ?
> -                RISCV_EXCP_STORE_PAGE_FAULT :
> -                RISCV_EXCP_STORE_AMO_ACCESS_FAULT;
> +            cs->exception_index = pmp_violation ?
> +                RISCV_EXCP_STORE_AMO_ACCESS_FAULT :
> +                RISCV_EXCP_STORE_PAGE_FAULT;
>           }
>           break;
>       default:

[PATCH 1/2] target/riscv/cpu_helper.c: Invalid exception on MMU translation stage
[PATCH 2/2] target/riscv/cpu_helper.c: Fix mxr bit behavior