diff mbox series

[1/5] target/arm/ptw: Load stage-2 tables from realm physical space

Message ID 20230719153018.1456180-3-jean-philippe@linaro.org
State New
Headers show
Series target/arm: Fixes for RME | expand

Commit Message

Jean-Philippe Brucker July 19, 2023, 3:30 p.m. UTC 
In realm state, stage-2 translation tables are fetched from the realm
physical address space (R_PGRQD).

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 target/arm/ptw.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Peter Maydell July 20, 2023, 4:28 p.m. UTC | #1 
On Wed, 19 Jul 2023 at 16:56, Jean-Philippe Brucker
<jean-philippe@linaro.org> wrote:
>
> In realm state, stage-2 translation tables are fetched from the realm
> physical address space (R_PGRQD).
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> ---
>  target/arm/ptw.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> index d1de934702..6318e13b98 100644
> --- a/target/arm/ptw.c
> +++ b/target/arm/ptw.c
> @@ -164,7 +164,11 @@ static ARMMMUIdx ptw_idx_for_stage_2(CPUARMState *env, ARMMMUIdx stage2idx)
>       * an NS stage 1+2 lookup while the NS bit is 0.)
>       */
>      if (!arm_is_secure_below_el3(env) || !arm_el_is_aa64(env, 3)) {
> -        return ARMMMUIdx_Phys_NS;
> +        if (arm_security_space_below_el3(env) == ARMSS_Realm) {
> +            return ARMMMUIdx_Phys_Realm;
> +        } else {
> +            return ARMMMUIdx_Phys_NS;
> +        }
>      }
>      if (stage2idx == ARMMMUIdx_Stage2_S) {
>          s2walk_secure = !(env->cp15.vstcr_el2 & VSTCR_SW);

This isn't wrong, but arm_is_secure_below_el3()
calls arm_security_space_below_el3(), so we kinda
duplicate work there. I think we should instead have:

    if (!arm_el_is_aa64(env, 3)) {
        return ARMMMUIdx_Phys_NS;
    }

    switch (arm_security_space_below_el3(env)) {
    case ARMSS_NonSecure:
        return ARMMUIdx_Phys_NS;
    case ARMSS_Realm:
        return ARMMMUIdx_Phys_Realm;
    case ARMSS_Secure:
        [existing code to look at the SW/NSW bits]
        return s2walk_secure ? ...;
    default:
        g_assert_not_reached();
    }

The comment above the function also needs tweaking
to say "SCR_EL3.NS or SCR_EL3.NSE bits" (we do already
do the TLB flush in scr_write).

thanks
-- PMM
diff mbox series

Patch

diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index d1de934702..6318e13b98 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -164,7 +164,11 @@  static ARMMMUIdx ptw_idx_for_stage_2(CPUARMState *env, ARMMMUIdx stage2idx)
      * an NS stage 1+2 lookup while the NS bit is 0.)
      */
     if (!arm_is_secure_below_el3(env) || !arm_el_is_aa64(env, 3)) {
-        return ARMMMUIdx_Phys_NS;
+        if (arm_security_space_below_el3(env) == ARMSS_Realm) {
+            return ARMMMUIdx_Phys_Realm;
+        } else {
+            return ARMMMUIdx_Phys_NS;
+        }
     }
     if (stage2idx == ARMMMUIdx_Stage2_S) {
         s2walk_secure = !(env->cp15.vstcr_el2 & VSTCR_SW);