Re: [PATCH] target/hppa: mask upper iaoq bits when returning to narrow mode

[Sven Schnelle <svens@stackframe.org>]

Sven Schnelle <svens@stackframe.org> writes:

> Sven Schnelle <svens@stackframe.org> writes:
>
>> Richard Henderson <richard.henderson@linaro.org> writes:
>>
>>> On 4/1/24 04:52, Sven Schnelle wrote:
>>>> For unknown reasons, Java 1.5 on 64-bit HP-UX 11.11 does signed
>>>> computation of the new IAOQ value in the signal handler. In the
>>>> current code these bits are not masked when returning to narrow
>>>> mode, causing java to crash.
>>>> Signed-off-by: Sven Schnelle <svens@stackframe.org>
>>>> ---
>>>>   target/hppa/sys_helper.c | 4 ++++
>>>>   1 file changed, 4 insertions(+)
>>>> diff --git a/target/hppa/sys_helper.c b/target/hppa/sys_helper.c
>>>> index 208e51c086..3bbc2da71b 100644
>>>> --- a/target/hppa/sys_helper.c
>>>> +++ b/target/hppa/sys_helper.c
>>>> @@ -83,6 +83,10 @@ void HELPER(rfi)(CPUHPPAState *env)
>>>>       env->iaoq_f = env->cr[CR_IIAOQ];
>>>>       env->iaoq_b = env->cr_back[1];
>>>>   +    if (!(env->cr[CR_IPSW] & PSW_W)) {
>>>> +        env->iaoq_f &= 0xffffffff;
>>>> +        env->iaoq_b &= 0xffffffff;
>>>> +    }
>>>
>>> This shouldn't be needed, because we are already masking these bits
>>> later, in cpu_get_tb_cpu_state.  But I do have some cleanups in this
>>> area, and perhaps one of them matters.
>> Any thoughts? Otherwise i need to investigate and make a wrong patch
>> again :-)
>
> This seems to be caused by IIAOQ's containing the upper bits. With the
> patch below i'm able to boot. Not sure whether it's correct though.
>
> diff --git a/target/hppa/int_helper.c b/target/hppa/int_helper.c
> index 58c13d3e61..f7c4cca8f1 100644
> --- a/target/hppa/int_helper.c
> +++ b/target/hppa/int_helper.c
> @@ -123,8 +123,14 @@ void hppa_cpu_do_interrupt(CPUState *cs)
>          env->cr[CR_IIASQ] = 0;
>          env->cr_back[0] = 0;
>      }
> -    env->cr[CR_IIAOQ] = env->iaoq_f;
> -    env->cr_back[1] = env->iaoq_b;
> +    if (old_psw & PSW_W) {
> +        env->cr[CR_IIAOQ] = env->iaoq_f;
> +        env->cr_back[1] = env->iaoq_b;
> +    } else {
> +        env->cr[CR_IIAOQ] = (env->iaoq_f & 0xffffffff);
> +        env->cr_back[1] = env->iaoq_b & 0xffffffff;
> +    }
> +

I guess the interesting question where should these bits get masked out
- i would assume that this place is to late, and it should happen
earlier in trans_be/when the iaoq value is copied. On the other hand
you had one commit that removed the masking in copy_iaoq_entry()...