[v2] target/arm: Use signed quantity to represent VMSAv8-64 translation level

[PATCH v2] target/arm: Use signed quantity to represent VMSAv8-64 translation level

Posted by Ard Biesheuvel 1 year, 5 months ago

The LPA2 extension implements 52-bit virtual addressing for 4k and 16k
translation granules, and for the former, this means an additional level
of translation is needed. This means we start counting at -1 instead of
0 when doing a walk, and so 'level' is now a signed quantity, and should
be typed as such. So turn it from uint32_t into int32_t.

This avoids a level of -1 getting misinterpreted as being >= 3, and
terminating a page table walk prematurely with a bogus output address.

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
Cc: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 target/arm/ptw.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 9a6277d862fac229..1d9bb4448761ddf4 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -1172,7 +1172,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
     ARMCPU *cpu = env_archcpu(env);
     ARMMMUIdx mmu_idx = ptw->in_mmu_idx;
     bool is_secure = ptw->in_secure;
-    uint32_t level;
+    int32_t level;
     ARMVAParameters param;
     uint64_t ttbr;
     hwaddr descaddr, indexmask, indexmask_grainsize;
@@ -1302,7 +1302,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, S1Translate *ptw,
          */
         uint32_t sl0 = extract32(tcr, 6, 2);
         uint32_t sl2 = extract64(tcr, 33, 1);
-        uint32_t startlevel;
+        int32_t startlevel;
         bool ok;
 
         /* SL2 is RES0 unless DS=1 & 4kb granule. */
-- 
2.35.1

Re: [PATCH v2] target/arm: Use signed quantity to represent VMSAv8-64 translation level

Posted by Peter Maydell 1 year, 5 months ago

On Tue, 22 Nov 2022 at 15:55, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> The LPA2 extension implements 52-bit virtual addressing for 4k and 16k
> translation granules, and for the former, this means an additional level
> of translation is needed. This means we start counting at -1 instead of
> 0 when doing a walk, and so 'level' is now a signed quantity, and should
> be typed as such. So turn it from uint32_t into int32_t.
>
> This avoids a level of -1 getting misinterpreted as being >= 3, and
> terminating a page table walk prematurely with a bogus output address.
>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Cc: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>



Applied to target-arm.next for 7.2, thanks.

-- PMM