[PULL 1/3] target/arm: do_ats_write(): avoid assertion when ptw failed

Maintainers: Pierrick Bouvier <pierrick.bouvier@linaro.org>, Peter Maydell <peter.maydell@linaro.org>
There is a newer version of this series
[PULL 1/3] target/arm: do_ats_write(): avoid assertion when ptw failed
Posted by Peter Maydell 4 days ago
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 <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
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) {
         /*
-- 
2.43.0