[PATCH] target/hppa: Fix IIAOQ, IIASQ for pa2.0

[PATCH] target/hppa: Fix IIAOQ, IIASQ for pa2.0

[Richard Henderson]

The contents of IIAOQ depend on PSW_W.
Follow the text in "Interruption Instruction Address Queues",
pages 2-13 through 2-15.

Reported-by: Sven Schnelle <svens@stackframe.org>
Fixes: b10700d826c ("target/hppa: Update IIAOQ, IIASQ for pa2.0")
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---

Sven, I looked again through IIAOQ documentation and it does seem
like some of the bits are wrong, both on interrupt delivery and RFI.


r~

---
 target/hppa/int_helper.c | 20 +++++++++++---------
 target/hppa/sys_helper.c | 18 +++++++++---------
 2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/target/hppa/int_helper.c b/target/hppa/int_helper.c
index 90437a92cd..a667ee380d 100644
--- a/target/hppa/int_helper.c
+++ b/target/hppa/int_helper.c
@@ -107,14 +107,10 @@ void hppa_cpu_do_interrupt(CPUState *cs)
 
     /* step 3 */
     /*
-     * For pa1.x, IIASQ is simply a copy of IASQ.
-     * For pa2.0, IIASQ is the top bits of the virtual address,
-     *            or zero if translation is disabled.
+     * IIASQ is the top bits of the virtual address, or zero if translation
+     * is disabled -- with PSW_W == 0, this will reduce to the space.
      */
-    if (!hppa_is_pa20(env)) {
-        env->cr[CR_IIASQ] = env->iasq_f >> 32;
-        env->cr_back[0] = env->iasq_b >> 32;
-    } else if (old_psw & PSW_C) {
+    if (old_psw & PSW_C) {
         env->cr[CR_IIASQ] =
             hppa_form_gva_psw(old_psw, env->iasq_f, env->iaoq_f) >> 32;
         env->cr_back[0] =
@@ -123,8 +119,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;
+    /* IIAOQ is the full offset for wide mode, or 32 bits for narrow mode. */
+    if (old_psw & PSW_W) {
+        env->cr[CR_IIAOQ] = env->iaoq_f;
+        env->cr_back[1] = env->iaoq_b;
+    } else {
+        env->cr[CR_IIAOQ] = (uint32_t)env->iaoq_f;
+        env->cr_back[1] = (uint32_t)env->iaoq_b;
+    }
 
     if (old_psw & PSW_Q) {
         /* step 5 */
diff --git a/target/hppa/sys_helper.c b/target/hppa/sys_helper.c
index 208e51c086..22d6c89964 100644
--- a/target/hppa/sys_helper.c
+++ b/target/hppa/sys_helper.c
@@ -78,21 +78,21 @@ target_ulong HELPER(swap_system_mask)(CPUHPPAState *env, target_ulong nsm)
 
 void HELPER(rfi)(CPUHPPAState *env)
 {
-    env->iasq_f = (uint64_t)env->cr[CR_IIASQ] << 32;
-    env->iasq_b = (uint64_t)env->cr_back[0] << 32;
-    env->iaoq_f = env->cr[CR_IIAOQ];
-    env->iaoq_b = env->cr_back[1];
+    uint64_t mask;
+
+    cpu_hppa_put_psw(env, env->cr[CR_IPSW]);
 
     /*
      * For pa2.0, IIASQ is the top bits of the virtual address.
      * To recreate the space identifier, remove the offset bits.
+     * For pa1.x, the mask reduces to no change to space.
      */
-    if (hppa_is_pa20(env)) {
-        env->iasq_f &= ~env->iaoq_f;
-        env->iasq_b &= ~env->iaoq_b;
-    }
+    mask = gva_offset_mask(env->psw);
 
-    cpu_hppa_put_psw(env, env->cr[CR_IPSW]);
+    env->iaoq_f = env->cr[CR_IIAOQ];
+    env->iaoq_b = env->cr_back[1];
+    env->iasq_f = (env->cr[CR_IIASQ] << 32) & ~(env->iaoq_f & mask);
+    env->iasq_b = (env->cr_back[0] << 32) & ~(env->iaoq_b & mask);
 }
 
 static void getshadowregs(CPUHPPAState *env)
-- 
2.34.1

Re: [PATCH] target/hppa: Fix IIAOQ, IIASQ for pa2.0

[Sven Schnelle]

Richard Henderson <richard.henderson@linaro.org> writes:

> The contents of IIAOQ depend on PSW_W.
> Follow the text in "Interruption Instruction Address Queues",
> pages 2-13 through 2-15.
>
> Reported-by: Sven Schnelle <svens@stackframe.org>
> Fixes: b10700d826c ("target/hppa: Update IIAOQ, IIASQ for pa2.0")
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>
> Sven, I looked again through IIAOQ documentation and it does seem
> like some of the bits are wrong, both on interrupt delivery and RFI.

Yes, this fixes my issues, thanks!

Tested-by: Sven Schnelle <svens@stackframe.org>

Re: [PATCH] target/hppa: Fix IIAOQ, IIASQ for pa2.0

[Helge Deller]

On 4/2/24 03:25, Richard Henderson wrote:
> The contents of IIAOQ depend on PSW_W.
> Follow the text in "Interruption Instruction Address Queues",
> pages 2-13 through 2-15.
>
> Reported-by: Sven Schnelle <svens@stackframe.org>
> Fixes: b10700d826c ("target/hppa: Update IIAOQ, IIASQ for pa2.0")
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Tested-by: Helge Deller <deller@gmx.de>

Helge

> ---
>
> Sven, I looked again through IIAOQ documentation and it does seem
> like some of the bits are wrong, both on interrupt delivery and RFI.
>
>
> r~
>
> ---
>   target/hppa/int_helper.c | 20 +++++++++++---------
>   target/hppa/sys_helper.c | 18 +++++++++---------
>   2 files changed, 20 insertions(+), 18 deletions(-)
>
> diff --git a/target/hppa/int_helper.c b/target/hppa/int_helper.c
> index 90437a92cd..a667ee380d 100644
> --- a/target/hppa/int_helper.c
> +++ b/target/hppa/int_helper.c
> @@ -107,14 +107,10 @@ void hppa_cpu_do_interrupt(CPUState *cs)
>
>       /* step 3 */
>       /*
> -     * For pa1.x, IIASQ is simply a copy of IASQ.
> -     * For pa2.0, IIASQ is the top bits of the virtual address,
> -     *            or zero if translation is disabled.
> +     * IIASQ is the top bits of the virtual address, or zero if translation
> +     * is disabled -- with PSW_W == 0, this will reduce to the space.
>        */
> -    if (!hppa_is_pa20(env)) {
> -        env->cr[CR_IIASQ] = env->iasq_f >> 32;
> -        env->cr_back[0] = env->iasq_b >> 32;
> -    } else if (old_psw & PSW_C) {
> +    if (old_psw & PSW_C) {
>           env->cr[CR_IIASQ] =
>               hppa_form_gva_psw(old_psw, env->iasq_f, env->iaoq_f) >> 32;
>           env->cr_back[0] =
> @@ -123,8 +119,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;
> +    /* IIAOQ is the full offset for wide mode, or 32 bits for narrow mode. */
> +    if (old_psw & PSW_W) {
> +        env->cr[CR_IIAOQ] = env->iaoq_f;
> +        env->cr_back[1] = env->iaoq_b;
> +    } else {
> +        env->cr[CR_IIAOQ] = (uint32_t)env->iaoq_f;
> +        env->cr_back[1] = (uint32_t)env->iaoq_b;
> +    }
>
>       if (old_psw & PSW_Q) {
>           /* step 5 */
> diff --git a/target/hppa/sys_helper.c b/target/hppa/sys_helper.c
> index 208e51c086..22d6c89964 100644
> --- a/target/hppa/sys_helper.c
> +++ b/target/hppa/sys_helper.c
> @@ -78,21 +78,21 @@ target_ulong HELPER(swap_system_mask)(CPUHPPAState *env, target_ulong nsm)
>
>   void HELPER(rfi)(CPUHPPAState *env)
>   {
> -    env->iasq_f = (uint64_t)env->cr[CR_IIASQ] << 32;
> -    env->iasq_b = (uint64_t)env->cr_back[0] << 32;
> -    env->iaoq_f = env->cr[CR_IIAOQ];
> -    env->iaoq_b = env->cr_back[1];
> +    uint64_t mask;
> +
> +    cpu_hppa_put_psw(env, env->cr[CR_IPSW]);
>
>       /*
>        * For pa2.0, IIASQ is the top bits of the virtual address.
>        * To recreate the space identifier, remove the offset bits.
> +     * For pa1.x, the mask reduces to no change to space.
>        */
> -    if (hppa_is_pa20(env)) {
> -        env->iasq_f &= ~env->iaoq_f;
> -        env->iasq_b &= ~env->iaoq_b;
> -    }
> +    mask = gva_offset_mask(env->psw);
>
> -    cpu_hppa_put_psw(env, env->cr[CR_IPSW]);
> +    env->iaoq_f = env->cr[CR_IIAOQ];
> +    env->iaoq_b = env->cr_back[1];
> +    env->iasq_f = (env->cr[CR_IIASQ] << 32) & ~(env->iaoq_f & mask);
> +    env->iasq_b = (env->cr_back[0] << 32) & ~(env->iaoq_b & mask);
>   }
>
>   static void getshadowregs(CPUHPPAState *env)