From 84771c64a5ae0f28d4bacc3f85a1f852a70c6edc Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 31 Mar 2026 10:23:05 +0100 Subject: [PATCH 1/3] target/arm: do_ats_write(): avoid assertion when ptw failed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In do_ats_write() we try to assert that the cacheattrs from get_phys_addr_for_at() are in the form we expect: /* * ATS operations only do S1 or S1+S2 translations, so we never * have to deal with the ARMCacheAttrs format for S2 only. */ assert(!res.cacheattrs.is_s2_format); However, the GetPhysAddrResult struct documents that its fields are only valid when the page table walk succeeded. For a two stage page table walk which fails during stage two, we will return early from get_phys_addr_twostage() and depending on the fault type the res.cacheattrs may have been initialized with the stage 2 cache attr information in stage 2 format. In this case we will incorrectly assert here. Fix the assertion to not look at the res fields if the lookup failed. Note for stable backports: the do_ats_write() function is in target/arm/helper.c in older QEMU versions, but the change to the assert line is the same. Cc: qemu-stable@nongnu.org Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3328 Fixes: 9f225e607f21 ("target/arm: Postpone interpretation of stage 2 descriptor attribute bits") Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-id: 20260331092305.2062580-1-peter.maydell@linaro.org --- target/arm/tcg/cpregs-at.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/target/arm/tcg/cpregs-at.c b/target/arm/tcg/cpregs-at.c index 0e8f229aa7..53dd67375d 100644 --- a/target/arm/tcg/cpregs-at.c +++ b/target/arm/tcg/cpregs-at.c @@ -37,8 +37,9 @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t value, /* * ATS operations only do S1 or S1+S2 translations, so we never * have to deal with the ARMCacheAttrs format for S2 only. + * (Note that res fields are only valid on ptw success.) */ - assert(!res.cacheattrs.is_s2_format); + assert(ret || !res.cacheattrs.is_s2_format); if (ret) { /* From 566594f10873723a179057a604d890bfaa1a9f0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Sun, 5 Apr 2026 12:24:10 +0100 Subject: [PATCH 2/3] target/arm: fix fault_s1ns for stage 2 faults MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The computation of s1ns was simply wrong. For Stage 2 faults, it should indicate whether the faulting IPA is in the Non-Secure IPA space. Correct the logic to check for ARMSS_NonSecure and drop the extraneous s2_mmu_idx test. This is effectively a change in the intended semantics of the ARMMMUFaultInfo::s1ns field, so that we no longer try to make it exactly match HPFAR_EL2.NS but instead set it for any stage 2 fault on an NS IPA, relying on users of the field to check whether the fault is to be taken to Secure EL2 before propagating the field to the HPFAR_EL2.NS bit. Since the actual writing of HPFAR_EL2.NS is already gated by arm_is_secure_below_el3(env), we only need to update the comments to document this change of semantics. Cc: qemu-stable@nongnu.org Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/2568 Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Message-id: 20260405112410.603223-1-alex.bennee@linaro.org [PMM: also update comments about the s1ns field] Reviewed-by: Peter Maydell Signed-off-by: Peter Maydell --- target/arm/internals.h | 5 ++++- target/arm/ptw.c | 12 +++++++----- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/target/arm/internals.h b/target/arm/internals.h index 8ec2750847..85980f0e69 100644 --- a/target/arm/internals.h +++ b/target/arm/internals.h @@ -740,7 +740,10 @@ typedef enum ARMGPCF { * @paddr_space: physical address space that caused a fault for gpc * @stage2: True if we faulted at stage 2 * @s1ptw: True if we faulted at stage 2 while doing a stage 1 page-table walk - * @s1ns: True if we faulted on a non-secure IPA while in secure state + * @s1ns: True if we faulted on a non-secure IPA. Note that (unlike the + * HPFAR_EL2.NS bit) this is set for any stage 2 fault for an NS IPA, so + * code must check that this is for a fault taken to Secure EL2 before + * propagating s1ns to HPFAR_EL2.NS. * @ea: True if we should set the EA (external abort type) bit in syndrome */ typedef struct ARMMMUFaultInfo ARMMMUFaultInfo; diff --git a/target/arm/ptw.c b/target/arm/ptw.c index 08a76bd3f1..7b993bb5b3 100644 --- a/target/arm/ptw.c +++ b/target/arm/ptw.c @@ -611,12 +611,14 @@ static ARMSecuritySpace S2_security_space(ARMSecuritySpace s1_space, 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. + * For stage 2 faults, S1NS indicates whether the faulting IPA is + * in the Non-Secure (true) or Secure (false) IPA space. For all + * other kinds of fault, it is false. Note that we do not + * distinguish "s2 fault on NS IPA taken to Secure EL2" from + * "s2 fault on NS IPA taken to NS EL2 or Realm EL2" here, but + * instead do that when setting HPFAR_EL2.NS. */ - 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. */ From 8a9fa9ec99be2437a0741b6c74930e6d65dbcb0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Mon, 6 Apr 2026 17:56:48 +0100 Subject: [PATCH 3/3] docs/system: remove extraneous sentence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lest we confuse the reader about something we are no longer showing in the example. Fixes: 132f8ec799c (target/arm: change default pauth algorithm to impdef) Signed-off-by: Alex Bennée Reviewed-by: Pierrick Bouvier Message-id: 20260406165648.997995-1-alex.bennee@linaro.org Signed-off-by: Peter Maydell --- docs/system/introduction.rst | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/docs/system/introduction.rst b/docs/system/introduction.rst index be387a6645..bec7291e47 100644 --- a/docs/system/introduction.rst +++ b/docs/system/introduction.rst @@ -165,10 +165,8 @@ names so we can override the defaults later. -m 4096 \ We then define the 4 vCPUs using the ``max`` option which gives us all -the Arm features QEMU is capable of emulating. We enable a more -emulation friendly implementation of Arm's pointer authentication -algorithm. We explicitly specify TCG acceleration even though QEMU -would default to it anyway. +the Arm features QEMU is capable of emulating. We explicitly specify +TCG acceleration even though QEMU would default to it anyway. .. code::