target/riscv: Set the correct exception for implict G-stage translation fail

[PATCH] target/riscv: Set the correct exception for implict G-stage translation fail

Posted by Jason Chien 2 years, 7 months ago

The privileged spec states:
For a memory access made to support VS-stage address translation (such as
to read/write a VS-level page table), permissions are checked as though
for a load or store, not for the original access type. However, any
exception is always reported for the original access type (instruction,
load, or store/AMO).

The current implementation converts the access type to LOAD if implicit
G-stage translation fails which results in only reporting "Load guest-page
fault". This commit removes the convertion of access type, so the reported
exception conforms to the spec.

Signed-off-by: Jason Chien <jason.chien@sifive.com>
---
 target/riscv/cpu_helper.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index a944f25694..ff2a1469dc 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1277,7 +1277,6 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
         if (ret == TRANSLATE_G_STAGE_FAIL) {
             first_stage_error = false;
             two_stage_indirect_error = true;
-            access_type = MMU_DATA_LOAD;
         }
 
         qemu_log_mask(CPU_LOG_MMU,
-- 
2.17.1

Re: [PATCH] target/riscv: Set the correct exception for implict G-stage translation fail

Posted by Alistair Francis 2 years, 7 months ago

On Tue, Jun 27, 2023 at 11:32 PM Jason Chien <jason.chien@sifive.com> wrote:
>
> The privileged spec states:
> For a memory access made to support VS-stage address translation (such as
> to read/write a VS-level page table), permissions are checked as though
> for a load or store, not for the original access type. However, any
> exception is always reported for the original access type (instruction,
> load, or store/AMO).
>
> The current implementation converts the access type to LOAD if implicit
> G-stage translation fails which results in only reporting "Load guest-page
> fault". This commit removes the convertion of access type, so the reported
> exception conforms to the spec.
>
> Signed-off-by: Jason Chien <jason.chien@sifive.com>

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

Alistair

> ---
>  target/riscv/cpu_helper.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index a944f25694..ff2a1469dc 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1277,7 +1277,6 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>          if (ret == TRANSLATE_G_STAGE_FAIL) {
>              first_stage_error = false;
>              two_stage_indirect_error = true;
> -            access_type = MMU_DATA_LOAD;
>          }
>
>          qemu_log_mask(CPU_LOG_MMU,
> --
> 2.17.1
>
>

Re: [PATCH] target/riscv: Set the correct exception for implict G-stage translation fail

Posted by Daniel Henrique Barboza 2 years, 7 months ago


On 6/27/23 04:48, Jason Chien wrote:
> The privileged spec states:
> For a memory access made to support VS-stage address translation (such as
> to read/write a VS-level page table), permissions are checked as though
> for a load or store, not for the original access type. However, any
> exception is always reported for the original access type (instruction,
> load, or store/AMO).
> 
> The current implementation converts the access type to LOAD if implicit
> G-stage translation fails which results in only reporting "Load guest-page
> fault". This commit removes the convertion of access type, so the reported
> exception conforms to the spec.
> 
> Signed-off-by: Jason Chien <jason.chien@sifive.com>
> ---

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

>   target/riscv/cpu_helper.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index a944f25694..ff2a1469dc 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1277,7 +1277,6 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>           if (ret == TRANSLATE_G_STAGE_FAIL) {
>               first_stage_error = false;
>               two_stage_indirect_error = true;
> -            access_type = MMU_DATA_LOAD;
>           }
>   
>           qemu_log_mask(CPU_LOG_MMU,

Re: [PATCH] target/riscv: Set the correct exception for implict G-stage translation fail

Posted by Alistair Francis 2 years, 7 months ago

On Tue, Jun 27, 2023 at 11:32 PM Jason Chien <jason.chien@sifive.com> wrote:
>
> The privileged spec states:
> For a memory access made to support VS-stage address translation (such as
> to read/write a VS-level page table), permissions are checked as though
> for a load or store, not for the original access type. However, any
> exception is always reported for the original access type (instruction,
> load, or store/AMO).
>
> The current implementation converts the access type to LOAD if implicit
> G-stage translation fails which results in only reporting "Load guest-page
> fault". This commit removes the convertion of access type, so the reported
> exception conforms to the spec.
>
> Signed-off-by: Jason Chien <jason.chien@sifive.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  target/riscv/cpu_helper.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index a944f25694..ff2a1469dc 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1277,7 +1277,6 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>          if (ret == TRANSLATE_G_STAGE_FAIL) {
>              first_stage_error = false;
>              two_stage_indirect_error = true;
> -            access_type = MMU_DATA_LOAD;
>          }
>
>          qemu_log_mask(CPU_LOG_MMU,
> --
> 2.17.1
>
>