[RFC PATCH] target/ppc: don't overflow SR lookup with 64 bit vaddr

[RFC PATCH] target/ppc: don't overflow SR lookup with 64 bit vaddr

[Alex Bennée]

One of the side effects of making vaddr always 64 bits is there are
places where we assume it is sized to the guest. As a result a simple
shift might bring in extra bits.

Using extract32 stops the crash in:

  ./pyvenv/bin/meson test qtest-ppc/prom-env-test

with TCI enabled but the test still hangs.

Over to you PPC maintainers ;-)

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 target/ppc/mmu-hash32.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/ppc/mmu-hash32.c b/target/ppc/mmu-hash32.c
index 8b980a5aa90..ce9c86ceacd 100644
--- a/target/ppc/mmu-hash32.c
+++ b/target/ppc/mmu-hash32.c
@@ -342,7 +342,7 @@ bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
     }
 
     /* 3. Look up the Segment Register */
-    sr = env->sr[eaddr >> 28];
+    sr = env->sr[extract32(eaddr, 28, 4)];
 
     /* 4. Handle direct store segments */
     if (sr & SR32_T) {
-- 
2.47.3

Re: [RFC PATCH] target/ppc: don't overflow SR lookup with 64 bit vaddr

[Peter Maydell]

On Thu, 27 Nov 2025 at 17:20, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> One of the side effects of making vaddr always 64 bits

vaddr has traditionally always been 64 bits, though.
(introduced in 2013 commit 577f42c0e11a5 as 'uint64_t').
Commit a70af12ad in February changed it the other way,
so that it is now uintptr_t instead of uint64_t, and might
be 32 bits on some hosts.

>  is there are
> places where we assume it is sized to the guest. As a result a simple
> shift might bring in extra bits.
>
> Using extract32 stops the crash in:
>
>   ./pyvenv/bin/meson test qtest-ppc/prom-env-test
>
> with TCI enabled but the test still hangs.
>
> Over to you PPC maintainers ;-)

But vaddr is an unsigned type -- why has something
sign-extended a 32-bit guest register value into it?

I think your problem is somewhere down in the callstack
where we are likely inadvertently sign-extending.

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  target/ppc/mmu-hash32.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/ppc/mmu-hash32.c b/target/ppc/mmu-hash32.c
> index 8b980a5aa90..ce9c86ceacd 100644
> --- a/target/ppc/mmu-hash32.c
> +++ b/target/ppc/mmu-hash32.c
> @@ -342,7 +342,7 @@ bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
>      }
>
>      /* 3. Look up the Segment Register */
> -    sr = env->sr[eaddr >> 28];
> +    sr = env->sr[extract32(eaddr, 28, 4)];
>
>      /* 4. Handle direct store segments */
>      if (sr & SR32_T) {
> --

thanks
-- PMM