[RFC PATCH] target/arm: always propagate s1ns state on faults

Alex Bennée posted 1 patch 1 week, 2 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260402154821.2340594-1-alex.bennee@linaro.org
Maintainers: Peter Maydell <peter.maydell@linaro.org>
target/arm/ptw.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
[RFC PATCH] target/arm: always propagate s1ns state on faults
Posted by Alex Bennée 1 week, 2 days ago
According to our mmuidx documentation:

  we fold together most secure and non-secure regimes for A-profile,
  because there are no banked system registers for aarch64

So we don't hold quite enough information during the table walk to
infer we are in secure mode. When we are trying to access a
none-secure page we get a stage 1 translation like:

  (rr) p ptw
  $1 = (S1Translate *) 0x7f07479f93a0
  (rr) p *ptw
  $2 = {in_mmu_idx = ARMMMUIdx_Stage2, in_ptw_idx = ARMMMUIdx_Phys_S, in_space = ARMSS_NonSecure, cur_space = ARMSS_NonSecure, in_debug = false, in_at = false,
    in_s1_is_el0 = false, in_prot_check = 1 '\001', in_nv1 = false, out_rw = true, out_be = false, out_space = ARMSS_Secure, out_virt = 1075929104, out_phys = 1075929104,
    out_host = 0x7f0750016010}

However as we only write HFPAR_EL2.NS when
arm_is_secure_below_el3(env) && fi.s1ns we can afford to relax the
check in fault_s1ns.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/2568
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 target/arm/ptw.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index d5b2ed6c99c..ccdc1ef9916 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -634,9 +634,11 @@ static bool fault_s1ns(ARMSecuritySpace space, ARMMMUIdx s2_mmu_idx)
      * For stage 2 faults in Secure EL22, S1NS indicates
      * whether the faulting IPA is in the Secure or NonSecure
      * IPA space. For all other kinds of fault, it is false.
+     *
+     * At this point we can't actually tell we are in SEL[012] but the
+     * actual setting of HPFAR_EL2.NS is gated by arm_is_secure_below_el3(env).
      */
-    return space == ARMSS_Secure && regime_is_stage2(s2_mmu_idx)
-        && s2_mmu_idx == ARMMMUIdx_Stage2_S;
+    return space == ARMSS_NonSecure && regime_is_stage2(s2_mmu_idx);
 }
 
 /* Translate a S1 pagetable walk through S2 if needed.  */
-- 
2.47.3


Re: [RFC PATCH] target/arm: always propagate s1ns state on faults
Posted by Richard Henderson 1 week ago
On 4/3/26 01:48, Alex Bennée wrote:
> According to our mmuidx documentation:
> 
>    we fold together most secure and non-secure regimes for A-profile,
>    because there are no banked system registers for aarch64
> 
> So we don't hold quite enough information during the table walk to
> infer we are in secure mode. When we are trying to access a
> none-secure page we get a stage 1 translation like:
> 
>    (rr) p ptw
>    $1 = (S1Translate *) 0x7f07479f93a0
>    (rr) p *ptw
>    $2 = {in_mmu_idx = ARMMMUIdx_Stage2, in_ptw_idx = ARMMMUIdx_Phys_S, in_space = ARMSS_NonSecure, cur_space = ARMSS_NonSecure, in_debug = false, in_at = false,
>      in_s1_is_el0 = false, in_prot_check = 1 '\001', in_nv1 = false, out_rw = true, out_be = false, out_space = ARMSS_Secure, out_virt = 1075929104, out_phys = 1075929104,
>      out_host = 0x7f0750016010}
> 
> However as we only write HFPAR_EL2.NS when
> arm_is_secure_below_el3(env) && fi.s1ns we can afford to relax the
> check in fault_s1ns.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/2568
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   target/arm/ptw.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> index d5b2ed6c99c..ccdc1ef9916 100644
> --- a/target/arm/ptw.c
> +++ b/target/arm/ptw.c
> @@ -634,9 +634,11 @@ static bool fault_s1ns(ARMSecuritySpace space, ARMMMUIdx s2_mmu_idx)
>        * For stage 2 faults in Secure EL22, S1NS indicates
>        * whether the faulting IPA is in the Secure or NonSecure
>        * IPA space. For all other kinds of fault, it is false.
> +     *
> +     * At this point we can't actually tell we are in SEL[012] but the
> +     * actual setting of HPFAR_EL2.NS is gated by arm_is_secure_below_el3(env).
>        */
> -    return space == ARMSS_Secure && regime_is_stage2(s2_mmu_idx)
> -        && s2_mmu_idx == ARMMMUIdx_Stage2_S;
> +    return space == ARMSS_NonSecure && regime_is_stage2(s2_mmu_idx);

The root problem is that the computation is simply wrong.  Nothing to do with the mmuidx 
documentation, so the patch description can be simplified.

Anyway, for the actual change,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~