[PATCH 0/3] target/hppa: Fix FP exception handling

[PATCH 0/3] target/hppa: Fix FP exception handling

[deller]

From: Helge Deller <deller@gmx.de>

This series fixes and improves the floating point exception
handling in the hppa system and user emulation.
A testcase is included in patch #3.

Please review.

Thanks!
Helge

Helge Deller (3):
  target/hppa: Copy instruction code into fr1 on FPU assist fault
  linux-user/hppa: Send proper si_code on SIGFPE exception
  target/hppa: Fix FPE exceptions

 linux-user/hppa/cpu_loop.c | 12 ++++++++++--
 target/hppa/fpu_helper.c   | 20 +++++++++++++++++---
 target/hppa/int_helper.c   |  4 ++++
 3 files changed, 31 insertions(+), 5 deletions(-)

-- 
2.47.0

[PATCH 1/3] target/hppa: Copy instruction code into fr1 on FPU assist fault

[deller]

From: Helge Deller <deller@gmx.de>

The hardware stores the instruction code in the lower bits of the FP
exception register #1 on FP assist traps.
This fixes the FP exception handler on Linux, as the Linux kernel uses
the value to decide on the correct signal which should be pushed into
userspace (see decode_fpu() in Linux kernel).

Signed-off-by: Helge Deller <deller@gmx.de>
---
 target/hppa/int_helper.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/target/hppa/int_helper.c b/target/hppa/int_helper.c
index 7d48643bb6..191ae19404 100644
--- a/target/hppa/int_helper.c
+++ b/target/hppa/int_helper.c
@@ -177,6 +177,10 @@ void hppa_cpu_do_interrupt(CPUState *cs)
                     }
                 }
                 env->cr[CR_IIR] = ldl_phys(cs->as, paddr);
+                if (i == EXCP_ASSIST) {
+                    /* stuff insn code into bits of FP exception register #1 */
+                    env->fr[0] |= (env->cr[CR_IIR] & 0x03ffffff);
+                }
             }
             break;
 
-- 
2.47.0

[PATCH 2/3] linux-user/hppa: Send proper si_code on SIGFPE exception

[deller]

From: Helge Deller <deller@gmx.de>

Improve the linux-user emulation to send the correct si_code depending
on overflow (TARGET_FPE_FLTOVF), underflow (TARGET_FPE_FLTUND), ...
Note that the hardware stores the relevant flags in FP exception
register #1, which is actually the lower 32-bits of the 64-bit fr[0]
register in qemu.

Signed-off-by: Helge Deller <deller@gmx.de>
---
 linux-user/hppa/cpu_loop.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/linux-user/hppa/cpu_loop.c b/linux-user/hppa/cpu_loop.c
index 890e758cd1..9abaad5ef8 100644
--- a/linux-user/hppa/cpu_loop.c
+++ b/linux-user/hppa/cpu_loop.c
@@ -112,7 +112,7 @@ static abi_ulong hppa_lws(CPUHPPAState *env)
 void cpu_loop(CPUHPPAState *env)
 {
     CPUState *cs = env_cpu(env);
-    abi_ulong ret;
+    abi_ulong ret, si_code = 0;
     int trapnr;
 
     while (1) {
@@ -169,7 +169,15 @@ void cpu_loop(CPUHPPAState *env)
             force_sig_fault(TARGET_SIGFPE, TARGET_FPE_CONDTRAP, env->iaoq_f);
             break;
         case EXCP_ASSIST:
-            force_sig_fault(TARGET_SIGFPE, 0, env->iaoq_f);
+            #define set_si_code(mask, val) \
+                if (env->fr[0] & mask) { si_code = val; }
+            set_si_code(R_FPSR_FLG_I_MASK, TARGET_FPE_FLTRES);
+            set_si_code(R_FPSR_FLG_U_MASK, TARGET_FPE_FLTUND);
+            set_si_code(R_FPSR_FLG_O_MASK, TARGET_FPE_FLTOVF);
+            set_si_code(R_FPSR_FLG_Z_MASK, TARGET_FPE_FLTDIV);
+            set_si_code(R_FPSR_FLG_V_MASK, TARGET_FPE_FLTINV);
+            #undef set_si_code
+            force_sig_fault(TARGET_SIGFPE, si_code, env->iaoq_f);
             break;
         case EXCP_BREAK:
             force_sig_fault(TARGET_SIGTRAP, TARGET_TRAP_BRKPT, env->iaoq_f);
-- 
2.47.0

[PATCH 3/3] target/hppa: Fix FPE exceptions

[deller]

From: Helge Deller <deller@gmx.de>

Implement FP exception register #1 (lower 32-bits of 64-bit fr[0]).
A proper implementation is necessary to allow the Linux kernel in
system mode and the qemu linux-user to send proper si_code values
on SIGFPE signal.

Always set the T-bit on taken exception, and merge over- and underflow
in system mode to just set overflow bit to mimic the behaviour I tested
on a physical machine.

The test program below can be used to verify correct behaviour. Note
that behaviour on SIGFPE may vary on different platforms. The program
should always detect the correct signal, but it may or may not be able
to sucessfully continue afterwards.

 #define _GNU_SOURCE
 #include <signal.h>
 #include <stdio.h>
 #include <fenv.h>
 #include <float.h>

 static void fpe_func(int sig, siginfo_t *i, void *v) {
    sigset_t set;
    sigemptyset(&set);
    sigaddset(&set, SIGFPE);
    sigprocmask(SIG_UNBLOCK, &set, NULL);
    printf("GOT signal %d with si_code %ld\n", sig, i->si_code);
 }

 int main(int argc, char *argv[]) {
    struct sigaction action = {
        .sa_sigaction = fpe_func,
        .sa_flags = SA_RESTART|SA_SIGINFO };
    sigaction(SIGFPE, &action, 0);
    feenableexcept(FE_OVERFLOW | FE_UNDERFLOW);
    double x = DBL_MIN;
    return printf("%lf\n", argc > 1
        ? 1.7976931348623158E308*1.7976931348623158E308
        : x / 10);
 }

Signed-off-by: Helge Deller <deller@gmx.de>
---
 target/hppa/fpu_helper.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/target/hppa/fpu_helper.c b/target/hppa/fpu_helper.c
index a62d9d3083..294ce0a970 100644
--- a/target/hppa/fpu_helper.c
+++ b/target/hppa/fpu_helper.c
@@ -95,7 +95,8 @@ static void update_fr0_op(CPUHPPAState *env, uintptr_t ra)
 {
     uint32_t soft_exp = get_float_exception_flags(&env->fp_status);
     uint32_t hard_exp = 0;
-    uint32_t shadow = env->fr0_shadow;
+    uint32_t shadow = env->fr0_shadow & 0x3ffffff;
+    uint32_t fr1 = 0;
 
     if (likely(soft_exp == 0)) {
         env->fr[0] = (uint64_t)shadow << 32;
@@ -108,9 +109,22 @@ static void update_fr0_op(CPUHPPAState *env, uintptr_t ra)
     hard_exp |= CONVERT_BIT(soft_exp, float_flag_overflow,  R_FPSR_ENA_O_MASK);
     hard_exp |= CONVERT_BIT(soft_exp, float_flag_divbyzero, R_FPSR_ENA_Z_MASK);
     hard_exp |= CONVERT_BIT(soft_exp, float_flag_invalid,   R_FPSR_ENA_V_MASK);
-    shadow |= hard_exp << (R_FPSR_FLAGS_SHIFT - R_FPSR_ENABLES_SHIFT);
+    if (hard_exp & shadow) {
+        shadow = FIELD_DP32(shadow, FPSR, T, 1);
+        /* fill exception register #1, which is lower 32-bits of fr[0] */
+#if !defined(CONFIG_USER_ONLY)
+        if (hard_exp & (R_FPSR_ENA_O_MASK | R_FPSR_ENA_U_MASK)) {
+            /* over- and underflow both set overflow flag only */
+            fr1 = FIELD_DP32(fr1, FPSR, C, 1);
+            fr1 = FIELD_DP32(fr1, FPSR, FLG_O, 1);
+        } else
+#endif
+        {
+            fr1 |= hard_exp << (R_FPSR_FLAGS_SHIFT - R_FPSR_ENABLES_SHIFT);
+        }
+    }
     env->fr0_shadow = shadow;
-    env->fr[0] = (uint64_t)shadow << 32;
+    env->fr[0] = (uint64_t)shadow << 32 | fr1;
 
     if (hard_exp & shadow) {
         hppa_dynamic_excp(env, EXCP_ASSIST, ra);
-- 
2.47.0

Re: [PATCH 3/3] target/hppa: Fix FPE exceptions

[Peter Maydell]

On Sat, 17 May 2025 at 13:02, <deller@kernel.org> wrote:
>
> From: Helge Deller <deller@gmx.de>
>
> Implement FP exception register #1 (lower 32-bits of 64-bit fr[0]).
> A proper implementation is necessary to allow the Linux kernel in
> system mode and the qemu linux-user to send proper si_code values
> on SIGFPE signal.
>
> Always set the T-bit on taken exception, and merge over- and underflow
> in system mode to just set overflow bit to mimic the behaviour I tested
> on a physical machine.

It looks like this patch broke the setting of the Flag
bits in the FPSR for non-trapped exceptions, which is
https://gitlab.com/qemu-project/qemu/-/issues/3158 ...

> diff --git a/target/hppa/fpu_helper.c b/target/hppa/fpu_helper.c
> index a62d9d3083..294ce0a970 100644
> --- a/target/hppa/fpu_helper.c
> +++ b/target/hppa/fpu_helper.c
> @@ -95,7 +95,8 @@ static void update_fr0_op(CPUHPPAState *env, uintptr_t ra)
>  {
>      uint32_t soft_exp = get_float_exception_flags(&env->fp_status);
>      uint32_t hard_exp = 0;
> -    uint32_t shadow = env->fr0_shadow;
> +    uint32_t shadow = env->fr0_shadow & 0x3ffffff;

We used to have 'shadow' contain all the existing Flag bits,
but now we mask them out. So whenever we call this function
we will zero out any previously set Flag bits.

> +    uint32_t fr1 = 0;
>
>      if (likely(soft_exp == 0)) {
>          env->fr[0] = (uint64_t)shadow << 32;
> @@ -108,9 +109,22 @@ static void update_fr0_op(CPUHPPAState *env, uintptr_t ra)
>      hard_exp |= CONVERT_BIT(soft_exp, float_flag_overflow,  R_FPSR_ENA_O_MASK);
>      hard_exp |= CONVERT_BIT(soft_exp, float_flag_divbyzero, R_FPSR_ENA_Z_MASK);
>      hard_exp |= CONVERT_BIT(soft_exp, float_flag_invalid,   R_FPSR_ENA_V_MASK);
> -    shadow |= hard_exp << (R_FPSR_FLAGS_SHIFT - R_FPSR_ENABLES_SHIFT);

...and we used to set the Flags bits for any exception that
had happened, and now we don't. So setting Flags for
non-trapped exceptions doesn't work.

> +    if (hard_exp & shadow) {
> +        shadow = FIELD_DP32(shadow, FPSR, T, 1);
> +        /* fill exception register #1, which is lower 32-bits of fr[0] */
> +#if !defined(CONFIG_USER_ONLY)
> +        if (hard_exp & (R_FPSR_ENA_O_MASK | R_FPSR_ENA_U_MASK)) {
> +            /* over- and underflow both set overflow flag only */
> +            fr1 = FIELD_DP32(fr1, FPSR, C, 1);
> +            fr1 = FIELD_DP32(fr1, FPSR, FLG_O, 1);
> +        } else
> +#endif
> +        {
> +            fr1 |= hard_exp << (R_FPSR_FLAGS_SHIFT - R_FPSR_ENABLES_SHIFT);
> +        }
> +    }
>      env->fr0_shadow = shadow;
> -    env->fr[0] = (uint64_t)shadow << 32;
> +    env->fr[0] = (uint64_t)shadow << 32 | fr1;
>
>      if (hard_exp & shadow) {
>          hppa_dynamic_excp(env, EXCP_ASSIST, ra);
> --

"Set all the Flag bits for non-trapping exceptions" seems
straightforward. I can't see anywhere in the function which
would care about the Flag bits in the 'shadow' variable,
though, so I don't understand why you were masking out the
Flag bits in this patch; so I'm not sure whether undoing that
will break anything. (I don't have a setup to try the test
program you give in this commit message.)

This patch fixes the "don't trap" case:

diff --git a/target/hppa/fpu_helper.c b/target/hppa/fpu_helper.c
index 45353202fae..2d272730f60 100644
--- a/target/hppa/fpu_helper.c
+++ b/target/hppa/fpu_helper.c
@@ -94,7 +94,8 @@ static void update_fr0_op(CPUHPPAState *env, uintptr_t ra)
 {
     uint32_t soft_exp = get_float_exception_flags(&env->fp_status);
     uint32_t hard_exp = 0;
-    uint32_t shadow = env->fr0_shadow & 0x3ffffff;
+    uint32_t shadow = env->fr0_shadow;
+    uint32_t to_flag = 0;
     uint32_t fr1 = 0;

     if (likely(soft_exp == 0)) {
@@ -122,6 +123,10 @@ static void update_fr0_op(CPUHPPAState *env, uintptr_t ra)
             fr1 |= hard_exp << (R_FPSR_FLAGS_SHIFT - R_FPSR_ENABLES_SHIFT);
         }
     }
+    /* Set the Flag bits for every exception that was not enabled */
+    to_flag = hard_exp & ~shadow;
+    shadow |= to_flag << (R_FPSR_FLAGS_SHIFT - R_FPSR_ENABLES_SHIFT);
+
     env->fr0_shadow = shadow;
     env->fr[0] = (uint64_t)shadow << 32 | fr1;

If it looks OK I'll send it as a proper patch.

-- PMM

Re: [PATCH 3/3] target/hppa: Fix FPE exceptions

[Helge Deller]

On 10/16/25 19:23, Peter Maydell wrote:
> On Sat, 17 May 2025 at 13:02, <deller@kernel.org> wrote:
>>
>> From: Helge Deller <deller@gmx.de>
>>
>> Implement FP exception register #1 (lower 32-bits of 64-bit fr[0]).
>> A proper implementation is necessary to allow the Linux kernel in
>> system mode and the qemu linux-user to send proper si_code values
>> on SIGFPE signal.
>>
>> Always set the T-bit on taken exception, and merge over- and underflow
>> in system mode to just set overflow bit to mimic the behaviour I tested
>> on a physical machine.
> 
> It looks like this patch broke the setting of the Flag
> bits in the FPSR for non-trapped exceptions, which is
> https://gitlab.com/qemu-project/qemu/-/issues/3158 ...
> 
>> diff --git a/target/hppa/fpu_helper.c b/target/hppa/fpu_helper.c
>> index a62d9d3083..294ce0a970 100644
>> --- a/target/hppa/fpu_helper.c
>> +++ b/target/hppa/fpu_helper.c
>> @@ -95,7 +95,8 @@ static void update_fr0_op(CPUHPPAState *env, uintptr_t ra)
>>   {
>>       uint32_t soft_exp = get_float_exception_flags(&env->fp_status);
>>       uint32_t hard_exp = 0;
>> -    uint32_t shadow = env->fr0_shadow;
>> +    uint32_t shadow = env->fr0_shadow & 0x3ffffff;
> 
> We used to have 'shadow' contain all the existing Flag bits,
> but now we mask them out. So whenever we call this function
> we will zero out any previously set Flag bits.
> 
>> +    uint32_t fr1 = 0;
>>
>>       if (likely(soft_exp == 0)) {
>>           env->fr[0] = (uint64_t)shadow << 32;
>> @@ -108,9 +109,22 @@ static void update_fr0_op(CPUHPPAState *env, uintptr_t ra)
>>       hard_exp |= CONVERT_BIT(soft_exp, float_flag_overflow,  R_FPSR_ENA_O_MASK);
>>       hard_exp |= CONVERT_BIT(soft_exp, float_flag_divbyzero, R_FPSR_ENA_Z_MASK);
>>       hard_exp |= CONVERT_BIT(soft_exp, float_flag_invalid,   R_FPSR_ENA_V_MASK);
>> -    shadow |= hard_exp << (R_FPSR_FLAGS_SHIFT - R_FPSR_ENABLES_SHIFT);
> 
> ...and we used to set the Flags bits for any exception that
> had happened, and now we don't. So setting Flags for
> non-trapped exceptions doesn't work.
> 
>> +    if (hard_exp & shadow) {
>> +        shadow = FIELD_DP32(shadow, FPSR, T, 1);
>> +        /* fill exception register #1, which is lower 32-bits of fr[0] */
>> +#if !defined(CONFIG_USER_ONLY)
>> +        if (hard_exp & (R_FPSR_ENA_O_MASK | R_FPSR_ENA_U_MASK)) {
>> +            /* over- and underflow both set overflow flag only */
>> +            fr1 = FIELD_DP32(fr1, FPSR, C, 1);
>> +            fr1 = FIELD_DP32(fr1, FPSR, FLG_O, 1);
>> +        } else
>> +#endif
>> +        {
>> +            fr1 |= hard_exp << (R_FPSR_FLAGS_SHIFT - R_FPSR_ENABLES_SHIFT);
>> +        }
>> +    }
>>       env->fr0_shadow = shadow;
>> -    env->fr[0] = (uint64_t)shadow << 32;
>> +    env->fr[0] = (uint64_t)shadow << 32 | fr1;
>>
>>       if (hard_exp & shadow) {
>>           hppa_dynamic_excp(env, EXCP_ASSIST, ra);
>> --
> 
> "Set all the Flag bits for non-trapping exceptions" seems
> straightforward. I can't see anywhere in the function which
> would care about the Flag bits in the 'shadow' variable,
> though, so I don't understand why you were masking out the
> Flag bits in this patch; so I'm not sure whether undoing that
> will break anything. (I don't have a setup to try the test
> program you give in this commit message.)
> 
> This patch fixes the "don't trap" case:
> 
> diff --git a/target/hppa/fpu_helper.c b/target/hppa/fpu_helper.c
> index 45353202fae..2d272730f60 100644
> --- a/target/hppa/fpu_helper.c
> +++ b/target/hppa/fpu_helper.c
> @@ -94,7 +94,8 @@ static void update_fr0_op(CPUHPPAState *env, uintptr_t ra)
>   {
>       uint32_t soft_exp = get_float_exception_flags(&env->fp_status);
>       uint32_t hard_exp = 0;
> -    uint32_t shadow = env->fr0_shadow & 0x3ffffff;
> +    uint32_t shadow = env->fr0_shadow;
> +    uint32_t to_flag = 0;
>       uint32_t fr1 = 0;
> 
>       if (likely(soft_exp == 0)) {
> @@ -122,6 +123,10 @@ static void update_fr0_op(CPUHPPAState *env, uintptr_t ra)
>               fr1 |= hard_exp << (R_FPSR_FLAGS_SHIFT - R_FPSR_ENABLES_SHIFT);
>           }
>       }
> +    /* Set the Flag bits for every exception that was not enabled */
> +    to_flag = hard_exp & ~shadow;
> +    shadow |= to_flag << (R_FPSR_FLAGS_SHIFT - R_FPSR_ENABLES_SHIFT);
> +
>       env->fr0_shadow = shadow;
>       env->fr[0] = (uint64_t)shadow << 32 | fr1;
> 
> If it looks OK I'll send it as a proper patch
Yes it looks good, so please send a patch!
I've tested your patch.
It indeed fixes the testcases as reported by Bruno:
https://gitlab.com/qemu-project/qemu/-/issues/3158
On a physical machine those testcases succeeded too.
Last, even my testcase still shows the same result (same signal)
as on real hardware.

So, thanks for fixing this issue!

Helge

Re: [PATCH 0/3] target/hppa: Fix FP exception handling

[Michael Tokarev]

On 17.05.2025 15:00, deller@kernel.org wrote:
> From: Helge Deller <deller@gmx.de>
> 
> This series fixes and improves the floating point exception
> handling in the hppa system and user emulation.
> A testcase is included in patch #3.
> 
> Please review.
> 
> Thanks!
> Helge
> 
> Helge Deller (3):
>    target/hppa: Copy instruction code into fr1 on FPU assist fault
>    linux-user/hppa: Send proper si_code on SIGFPE exception
>    target/hppa: Fix FPE exceptions

Is there anything relevant for qemu-stable?

At least patch #1 seems to be relevant, at least for the recent
qemu-stable series.

Thanks,

/mjt

Re: [PATCH 0/3] target/hppa: Fix FP exception handling

[Helge Deller]

On 5/24/25 12:26, Michael Tokarev wrote:
> On 17.05.2025 15:00, deller@kernel.org wrote:
>> From: Helge Deller <deller@gmx.de>
>>
>> This series fixes and improves the floating point exception
>> handling in the hppa system and user emulation.
>> A testcase is included in patch #3.
>>
>> Please review.
>>
>> Thanks!
>> Helge
>>
>> Helge Deller (3):
>>    target/hppa: Copy instruction code into fr1 on FPU assist fault
>>    linux-user/hppa: Send proper si_code on SIGFPE exception
>>    target/hppa: Fix FPE exceptions
> 
> Is there anything relevant for qemu-stable?

Basically all are relevant, but I think they don't apply cleanly.
  
> At least patch #1 seems to be relevant, at least for the recent
> qemu-stable series.

Patch #1 alone won't help, as the OS will only recognize it if
the bits are set as done in patch #3.
So, backporting all or none of the patches is the right way.

Helg

Re: [PATCH 0/3] target/hppa: Fix FP exception handling

[Michael Tokarev]

On 25.05.2025 03:42, Helge Deller wrote:
> On 5/24/25 12:26, Michael Tokarev wrote:
>> On 17.05.2025 15:00, deller@kernel.org wrote:
>>> From: Helge Deller <deller@gmx.de>
>>>
>>> This series fixes and improves the floating point exception
>>> handling in the hppa system and user emulation.
>>> A testcase is included in patch #3.
>>>
>>> Please review.
>>>
>>> Thanks!
>>> Helge
>>>
>>> Helge Deller (3):
>>>    target/hppa: Copy instruction code into fr1 on FPU assist fault
>>>    linux-user/hppa: Send proper si_code on SIGFPE exception
>>>    target/hppa: Fix FPE exceptions
>>
>> Is there anything relevant for qemu-stable?
> 
> Basically all are relevant, but I think they don't apply cleanly.

They do.

>> At least patch #1 seems to be relevant, at least for the recent
>> qemu-stable series.
> 
> Patch #1 alone won't help, as the OS will only recognize it if
> the bits are set as done in patch #3.
> So, backporting all or none of the patches is the right way.

Ok, this makes sense.  I've applied all 3 to 9.2 and 10.0 series.

Thanks,

/mjt