[v1] target/arm: Some pieces of support for 32-bit Hyp mode

[Qemu-devel] [PATCH 08/10] target/arm: Permit accesses to ELR_Hyp from Hyp mode via MSR/MRS (banked)

Posted by Peter Maydell 7 years, 2 months ago

The MSR (banked) and MRS (banked) instructions allow accesses to ELR_Hyp
from either Monitor or Hyp mode. Our translate time check
was overly strict and only permitted access from Monitor mode.

The runtime check wo do in msr_mrs_banked_exc_checks() had the
correct code in it, but never got there because of the earlier
"currmode == tgtmode" check. Special case ELR_Hyp.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/op_helper.c | 22 +++++++++++-----------
 target/arm/translate.c | 10 +++++++---
 2 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index d550978b5b9..952b8d122b7 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -611,6 +611,14 @@ static void msr_mrs_banked_exc_checks(CPUARMState *env, uint32_t tgtmode,
      */
     int curmode = env->uncached_cpsr & CPSR_M;
 
+    if (regno == 17) {
+        /* ELR_Hyp: a special case because access from tgtmode is OK */
+        if (curmode != ARM_CPU_MODE_HYP && curmode != ARM_CPU_MODE_MON) {
+            goto undef;
+        }
+        return;
+    }
+
     if (curmode == tgtmode) {
         goto undef;
     }
@@ -638,17 +646,9 @@ static void msr_mrs_banked_exc_checks(CPUARMState *env, uint32_t tgtmode,
     }
 
     if (tgtmode == ARM_CPU_MODE_HYP) {
-        switch (regno) {
-        case 17: /* ELR_Hyp */
-            if (curmode != ARM_CPU_MODE_HYP && curmode != ARM_CPU_MODE_MON) {
-                goto undef;
-            }
-            break;
-        default:
-            if (curmode != ARM_CPU_MODE_MON) {
-                goto undef;
-            }
-            break;
+        /* SPSR_Hyp, r13_hyp: accessible from Monitor mode only */
+        if (curmode != ARM_CPU_MODE_MON) {
+            goto undef;
         }
     }
 
diff --git a/target/arm/translate.c b/target/arm/translate.c
index f845da7c638..3f5751d4826 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -4506,10 +4506,14 @@ static bool msr_banked_access_decode(DisasContext *s, int r, int sysm, int rn,
         }
         break;
     case ARM_CPU_MODE_HYP:
-        /* Note that we can forbid accesses from EL2 here because they
-         * must be from Hyp mode itself
+        /*
+         * SPSR_hyp and r13_hyp can only be accessed from Monitor mode
+         * (and so we can forbid accesses from EL2 or below). elr_hyp
+         * can be accessed also from Hyp mode, so forbid accesses from
+         * EL0 or EL1.
          */
-        if (!arm_dc_feature(s, ARM_FEATURE_EL2) || s->current_el < 3) {
+        if (!arm_dc_feature(s, ARM_FEATURE_EL2) || s->current_el < 2 ||
+            (s->current_el < 3 && *regno != 17)) {
             goto undef;
         }
         break;
-- 
2.18.0

Re: [Qemu-devel] [PATCH 08/10] target/arm: Permit accesses to ELR_Hyp from Hyp mode via MSR/MRS (banked)

Posted by Edgar E. Iglesias 7 years, 2 months ago

On Tue, Aug 14, 2018 at 01:42:52PM +0100, Peter Maydell wrote:
> The MSR (banked) and MRS (banked) instructions allow accesses to ELR_Hyp
> from either Monitor or Hyp mode. Our translate time check
> was overly strict and only permitted access from Monitor mode.
> 
> The runtime check wo do in msr_mrs_banked_exc_checks() had the
> correct code in it, but never got there because of the earlier
> "currmode == tgtmode" check. Special case ELR_Hyp.

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target/arm/op_helper.c | 22 +++++++++++-----------
>  target/arm/translate.c | 10 +++++++---
>  2 files changed, 18 insertions(+), 14 deletions(-)
> 
> diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
> index d550978b5b9..952b8d122b7 100644
> --- a/target/arm/op_helper.c
> +++ b/target/arm/op_helper.c
> @@ -611,6 +611,14 @@ static void msr_mrs_banked_exc_checks(CPUARMState *env, uint32_t tgtmode,
>       */
>      int curmode = env->uncached_cpsr & CPSR_M;
>  
> +    if (regno == 17) {
> +        /* ELR_Hyp: a special case because access from tgtmode is OK */
> +        if (curmode != ARM_CPU_MODE_HYP && curmode != ARM_CPU_MODE_MON) {
> +            goto undef;
> +        }
> +        return;
> +    }
> +
>      if (curmode == tgtmode) {
>          goto undef;
>      }
> @@ -638,17 +646,9 @@ static void msr_mrs_banked_exc_checks(CPUARMState *env, uint32_t tgtmode,
>      }
>  
>      if (tgtmode == ARM_CPU_MODE_HYP) {
> -        switch (regno) {
> -        case 17: /* ELR_Hyp */
> -            if (curmode != ARM_CPU_MODE_HYP && curmode != ARM_CPU_MODE_MON) {
> -                goto undef;
> -            }
> -            break;
> -        default:
> -            if (curmode != ARM_CPU_MODE_MON) {
> -                goto undef;
> -            }
> -            break;
> +        /* SPSR_Hyp, r13_hyp: accessible from Monitor mode only */
> +        if (curmode != ARM_CPU_MODE_MON) {
> +            goto undef;
>          }
>      }
>  
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index f845da7c638..3f5751d4826 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -4506,10 +4506,14 @@ static bool msr_banked_access_decode(DisasContext *s, int r, int sysm, int rn,
>          }
>          break;
>      case ARM_CPU_MODE_HYP:
> -        /* Note that we can forbid accesses from EL2 here because they
> -         * must be from Hyp mode itself
> +        /*
> +         * SPSR_hyp and r13_hyp can only be accessed from Monitor mode
> +         * (and so we can forbid accesses from EL2 or below). elr_hyp
> +         * can be accessed also from Hyp mode, so forbid accesses from
> +         * EL0 or EL1.
>           */
> -        if (!arm_dc_feature(s, ARM_FEATURE_EL2) || s->current_el < 3) {
> +        if (!arm_dc_feature(s, ARM_FEATURE_EL2) || s->current_el < 2 ||
> +            (s->current_el < 3 && *regno != 17)) {
>              goto undef;
>          }
>          break;
> -- 
> 2.18.0
>

Re: [Qemu-devel] [PATCH 08/10] target/arm: Permit accesses to ELR_Hyp from Hyp mode via MSR/MRS (banked)

Posted by Luc Michel 7 years, 2 months ago

On 8/14/18 2:42 PM, Peter Maydell wrote:
> The MSR (banked) and MRS (banked) instructions allow accesses to ELR_Hyp
> from either Monitor or Hyp mode. Our translate time check
> was overly strict and only permitted access from Monitor mode.
> 
> The runtime check wo do in msr_mrs_banked_exc_checks() had the
"we"
> correct code in it, but never got there because of the earlier
> "currmode == tgtmode" check. Special case ELR_Hyp.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-By: Luc Michel <luc.michel@greensocs.com>

> ---
>  target/arm/op_helper.c | 22 +++++++++++-----------
>  target/arm/translate.c | 10 +++++++---
>  2 files changed, 18 insertions(+), 14 deletions(-)
> 
> diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
> index d550978b5b9..952b8d122b7 100644
> --- a/target/arm/op_helper.c
> +++ b/target/arm/op_helper.c
> @@ -611,6 +611,14 @@ static void msr_mrs_banked_exc_checks(CPUARMState *env, uint32_t tgtmode,
>       */
>      int curmode = env->uncached_cpsr & CPSR_M;
>  
> +    if (regno == 17) {
> +        /* ELR_Hyp: a special case because access from tgtmode is OK */
> +        if (curmode != ARM_CPU_MODE_HYP && curmode != ARM_CPU_MODE_MON) {
> +            goto undef;
> +        }
> +        return;
> +    }
> +
>      if (curmode == tgtmode) {
>          goto undef;
>      }
> @@ -638,17 +646,9 @@ static void msr_mrs_banked_exc_checks(CPUARMState *env, uint32_t tgtmode,
>      }
>  
>      if (tgtmode == ARM_CPU_MODE_HYP) {
> -        switch (regno) {
> -        case 17: /* ELR_Hyp */
> -            if (curmode != ARM_CPU_MODE_HYP && curmode != ARM_CPU_MODE_MON) {
> -                goto undef;
> -            }
> -            break;
> -        default:
> -            if (curmode != ARM_CPU_MODE_MON) {
> -                goto undef;
> -            }
> -            break;
> +        /* SPSR_Hyp, r13_hyp: accessible from Monitor mode only */
> +        if (curmode != ARM_CPU_MODE_MON) {
> +            goto undef;
>          }
>      }
>  
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index f845da7c638..3f5751d4826 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -4506,10 +4506,14 @@ static bool msr_banked_access_decode(DisasContext *s, int r, int sysm, int rn,
>          }
>          break;
>      case ARM_CPU_MODE_HYP:
> -        /* Note that we can forbid accesses from EL2 here because they
> -         * must be from Hyp mode itself
> +        /*
> +         * SPSR_hyp and r13_hyp can only be accessed from Monitor mode
> +         * (and so we can forbid accesses from EL2 or below). elr_hyp
> +         * can be accessed also from Hyp mode, so forbid accesses from
> +         * EL0 or EL1.
>           */
> -        if (!arm_dc_feature(s, ARM_FEATURE_EL2) || s->current_el < 3) {
> +        if (!arm_dc_feature(s, ARM_FEATURE_EL2) || s->current_el < 2 ||
> +            (s->current_el < 3 && *regno != 17)) {
>              goto undef;
>          }
>          break;
>

[Qemu-devel] [PATCH 01/10] target/arm: Correct typo in HAMAIR1 regdef name
[Qemu-devel] [PATCH 02/10] target/arm: Add missing .cp = 15 to HMAIR1 and HAMAIR1 regdefs
[Qemu-devel] [PATCH 03/10] target/arm: Implement RAZ/WI HACTLR2
[Qemu-devel] [PATCH 04/10] target/arm: Implement AArch32 HVBAR
[Qemu-devel] [PATCH 05/10] target/arm: Implement AArch32 HCR and HCR2
[Qemu-devel] [PATCH 06/10] target/arm: Implement AArch32 Hyp FARs
[Qemu-devel] [PATCH 07/10] target/arm: Implement ESR_EL2/HSR for AArch32 and no-EL2
[Qemu-devel] [PATCH 08/10] target/arm: Permit accesses to ELR_Hyp from Hyp mode via MSR/MRS (banked)
[Qemu-devel] [PATCH 09/10] target/arm: Implement AArch32 ERET instruction
[Qemu-devel] [PATCH 10/10] target/arm: Implement support for taking exceptions to Hyp mode