diff mbox series

[v2,29/66] target/arm: Introduce arm_hcr_el2_eff_secstate

Message ID 20220822152741.1617527-30-richard.henderson@linaro.org
State Superseded
Headers show
Series target/arm: Implement FEAT_HAFDBS | expand

Commit Message

Richard Henderson Aug. 22, 2022, 3:27 p.m. UTC
For page walking, we may require HCR for a security state
that is not "current".

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/cpu.h    | 20 +++++++++++++-------
 target/arm/helper.c | 11 ++++++++---
 2 files changed, 21 insertions(+), 10 deletions(-)

Comments

Peter Maydell Sept. 20, 2022, 3:52 p.m. UTC | #1
On Mon, 22 Aug 2022 at 17:22, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> For page walking, we may require HCR for a security state
> that is not "current".
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/cpu.h    | 20 +++++++++++++-------
>  target/arm/helper.c | 11 ++++++++---
>  2 files changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index cea2121f67..a08e546de4 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -2401,15 +2401,15 @@ static inline bool arm_is_secure(CPUARMState *env)
>   * Return true if the current security state has AArch64 EL2 or AArch32 Hyp.
>   * This corresponds to the pseudocode EL2Enabled()
>   */
> +static inline bool arm_is_el2_enabled_secstate(CPUARMState *env, bool secure)
> +{
> +    return (arm_feature(env, ARM_FEATURE_EL2)
> +            && (!secure || (env->cp15.scr_el3 & SCR_EEL2)));

This doesn't need the outermost set of ().

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM
Richard Henderson Sept. 28, 2022, 7:38 p.m. UTC | #2
On 9/20/22 08:52, Peter Maydell wrote:
> On Mon, 22 Aug 2022 at 17:22, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> For page walking, we may require HCR for a security state
>> that is not "current".
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   target/arm/cpu.h    | 20 +++++++++++++-------
>>   target/arm/helper.c | 11 ++++++++---
>>   2 files changed, 21 insertions(+), 10 deletions(-)
>>
>> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
>> index cea2121f67..a08e546de4 100644
>> --- a/target/arm/cpu.h
>> +++ b/target/arm/cpu.h
>> @@ -2401,15 +2401,15 @@ static inline bool arm_is_secure(CPUARMState *env)
>>    * Return true if the current security state has AArch64 EL2 or AArch32 Hyp.
>>    * This corresponds to the pseudocode EL2Enabled()
>>    */
>> +static inline bool arm_is_el2_enabled_secstate(CPUARMState *env, bool secure)
>> +{
>> +    return (arm_feature(env, ARM_FEATURE_EL2)
>> +            && (!secure || (env->cp15.scr_el3 & SCR_EEL2)));
> 
> This doesn't need the outermost set of ().

Oh, that's an emacs alignment thing.

r~

> 
> Otherwise
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> 
> thanks
> -- PMM
diff mbox series

Patch

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index cea2121f67..a08e546de4 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2401,15 +2401,15 @@  static inline bool arm_is_secure(CPUARMState *env)
  * Return true if the current security state has AArch64 EL2 or AArch32 Hyp.
  * This corresponds to the pseudocode EL2Enabled()
  */
+static inline bool arm_is_el2_enabled_secstate(CPUARMState *env, bool secure)
+{
+    return (arm_feature(env, ARM_FEATURE_EL2)
+            && (!secure || (env->cp15.scr_el3 & SCR_EEL2)));
+}
+
 static inline bool arm_is_el2_enabled(CPUARMState *env)
 {
-    if (arm_feature(env, ARM_FEATURE_EL2)) {
-        if (arm_is_secure_below_el3(env)) {
-            return (env->cp15.scr_el3 & SCR_EEL2) != 0;
-        }
-        return true;
-    }
-    return false;
+    return arm_is_el2_enabled_secstate(env, arm_is_secure_below_el3(env));
 }
 
 #else
@@ -2423,6 +2423,11 @@  static inline bool arm_is_secure(CPUARMState *env)
     return false;
 }
 
+static inline bool arm_is_el2_enabled_secstate(CPUARMState *env, bool secure)
+{
+    return false;
+}
+
 static inline bool arm_is_el2_enabled(CPUARMState *env)
 {
     return false;
@@ -2435,6 +2440,7 @@  static inline bool arm_is_el2_enabled(CPUARMState *env)
  * "for all purposes other than a direct read or write access of HCR_EL2."
  * Not included here is HCR_RW.
  */
+uint64_t arm_hcr_el2_eff_secstate(CPUARMState *env, bool secure);
 uint64_t arm_hcr_el2_eff(CPUARMState *env);
 uint64_t arm_hcrx_el2_eff(CPUARMState *env);
 
diff --git a/target/arm/helper.c b/target/arm/helper.c
index b9f1a3d826..55355197b8 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -5101,15 +5101,15 @@  static void hcr_writelow(CPUARMState *env, const ARMCPRegInfo *ri,
 }
 
 /*
- * Return the effective value of HCR_EL2.
+ * Return the effective value of HCR_EL2, at the given security state.
  * Bits that are not included here:
  * RW       (read from SCR_EL3.RW as needed)
  */
-uint64_t arm_hcr_el2_eff(CPUARMState *env)
+uint64_t arm_hcr_el2_eff_secstate(CPUARMState *env, bool secure)
 {
     uint64_t ret = env->cp15.hcr_el2;
 
-    if (!arm_is_el2_enabled(env)) {
+    if (!arm_is_el2_enabled_secstate(env, secure)) {
         /*
          * "This register has no effect if EL2 is not enabled in the
          * current Security state".  This is ARMv8.4-SecEL2 speak for
@@ -5168,6 +5168,11 @@  uint64_t arm_hcr_el2_eff(CPUARMState *env)
     return ret;
 }
 
+uint64_t arm_hcr_el2_eff(CPUARMState *env)
+{
+    return arm_hcr_el2_eff_secstate(env, arm_is_secure_below_el3(env));
+}
+
 /*
  * Corresponds to ARM pseudocode function ELIsInHost().
  */