Re: [PATCH 3/3] target/hppa: fix building gva for wide mode

[Richard Henderson <richard.henderson@linaro.org>]

On 3/23/24 22:09, Sven Schnelle wrote:
> 64 Bit hppa no longer has a fixed 32/32 bit split between space and
> offset. Instead it uses 42 bits for the offset. The lower 10 bits of
> the space are always zero, leaving 22 bits actually used. Simply or
> the values together to build the gva.
> 
> Signed-off-by: Sven Schnelle <svens@stackframe.org>
> ---
>   target/hppa/mem_helper.c | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
> index 84785b5a5c..6f895fced7 100644
> --- a/target/hppa/mem_helper.c
> +++ b/target/hppa/mem_helper.c
> @@ -523,13 +523,16 @@ void HELPER(itlbp_pa11)(CPUHPPAState *env, target_ulong addr, target_ulong reg)
>   }
>   
>   static void itlbt_pa20(CPUHPPAState *env, target_ulong r1,
> -                       target_ulong r2, vaddr va_b)
> +                       target_ulong r2, uint64_t spc, uint64_t off)
>   {
>       HPPATLBEntry *ent;
> -    vaddr va_e;
> +    vaddr va_b, va_e;
>       uint64_t va_size;
>       int mask_shift;
>   
> +    va_b = off & gva_offset_mask(env->psw);
> +    va_b |= spc << 32;

Actually, no, these instructions don't form a GVA in the normal fashion:

space ← ISR;
offset ← cat(ISR{32..63},IOR{32..63});
VIRTUAL_ADDR ← (space<<32) | (offset);

> -    vaddr va_b = deposit64(env->cr[CR_IOR], 32, 32, env->cr[CR_ISR]);

But this is wrong too.

Since the input here is from the result of a previous memory access, which populated 
ISR+IOR, I presume any masking would be done from the prior memory access.

We do need to fix the merge, per the docs though.


r~