[PATCH v2 0/2] linux-user/ppc: Deliver SIGTRAP on tw[i]/td[i]

[PATCH v2 0/2] linux-user/ppc: Deliver SIGTRAP on tw[i]/td[i]

[matheus.ferst]

From: Matheus Ferst <matheus.ferst@eldorado.org.br>

In the review of 66c6b40aba1, Richard Henderson suggested[1] using
"trap" instead of ".long 0x0" to generate the signal to test XER
save/restore behavior. However, linux-user aborts when a trap
exception is raised, so we kept the patch with SIGILL.

This patch series is a follow-up to remove the cpu_abort call, deliver
SIGTRAP instead (using TRAP_BRKPT as si_code), and apply the suggestion
to the signal_save_restore_xer test.

This change removes the "qemu: fatal: Tried to call a TRAP" reported in
issue #588[2], but we still have the "Unknown privilege violation (03)"
error.

I had some problems building with
--static, so I couldn't test if it solves the problem.

Based-on: https://lists.gnu.org/archive/html/qemu-devel/2021-12/msg03454.html

[1] https://lists.gnu.org/archive/html/qemu-ppc/2021-10/msg00143.html

v2:
 - Based-on rth's patch to use force_sig_fault and avoid merge conflicts

Matheus Ferst (2):
  linux-user/ppc: deliver SIGTRAP on POWERPC_EXCP_TRAP
  tests/tcg/ppc64le: change signal_save_restore_xer to use SIGTRAP

 linux-user/ppc/cpu_loop.c                   | 3 ++-
 tests/tcg/ppc64le/signal_save_restore_xer.c | 8 ++++----
 2 files changed, 6 insertions(+), 5 deletions(-)

-- 
2.25.1

[PATCH v2 1/2] linux-user/ppc: deliver SIGTRAP on POWERPC_EXCP_TRAP

[matheus.ferst]

From: Matheus Ferst <matheus.ferst@eldorado.org.br>

Handle POWERPC_EXCP_TRAP in cpu_loop to deliver SIGTRAP on tw[i]/td[i].
The si_code comes from do_program_check in the kernel source file
arch/powerpc/kernel/traps.c

Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
 linux-user/ppc/cpu_loop.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/linux-user/ppc/cpu_loop.c b/linux-user/ppc/cpu_loop.c
index 46e6ffd6d3..6c99feb19b 100644
--- a/linux-user/ppc/cpu_loop.c
+++ b/linux-user/ppc/cpu_loop.c
@@ -188,7 +188,8 @@ void cpu_loop(CPUPPCState *env)
                 }
                 break;
             case POWERPC_EXCP_TRAP:
-                cpu_abort(cs, "Tried to call a TRAP\n");
+                si_signo = TARGET_SIGTRAP;
+                si_code = TARGET_TRAP_BRKPT;
                 break;
             default:
                 /* Should not happen ! */
-- 
2.25.1

Re: [PATCH v2 1/2] linux-user/ppc: deliver SIGTRAP on POWERPC_EXCP_TRAP

[Richard Henderson]

On 1/4/22 1:00 PM, matheus.ferst@eldorado.org.br wrote:
> From: Matheus Ferst<matheus.ferst@eldorado.org.br>
> 
> Handle POWERPC_EXCP_TRAP in cpu_loop to deliver SIGTRAP on tw[i]/td[i].
> The si_code comes from do_program_check in the kernel source file
> arch/powerpc/kernel/traps.c
> 
> Signed-off-by: Matheus Ferst<matheus.ferst@eldorado.org.br>
> ---
>   linux-user/ppc/cpu_loop.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v2 1/2] linux-user/ppc: deliver SIGTRAP on POWERPC_EXCP_TRAP

[Richard Henderson]

On 1/4/22 2:12 PM, Richard Henderson wrote:
> On 1/4/22 1:00 PM, matheus.ferst@eldorado.org.br wrote:
>> From: Matheus Ferst<matheus.ferst@eldorado.org.br>
>>
>> Handle POWERPC_EXCP_TRAP in cpu_loop to deliver SIGTRAP on tw[i]/td[i].
>> The si_code comes from do_program_check in the kernel source file
>> arch/powerpc/kernel/traps.c
>>
>> Signed-off-by: Matheus Ferst<matheus.ferst@eldorado.org.br>
>> ---
>>   linux-user/ppc/cpu_loop.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/588


r~

Re: [PATCH v2 1/2] linux-user/ppc: deliver SIGTRAP on POWERPC_EXCP_TRAP

[Cédric Le Goater]

On 1/4/22 22:00, matheus.ferst@eldorado.org.br wrote:
> From: Matheus Ferst <matheus.ferst@eldorado.org.br>
> 
> Handle POWERPC_EXCP_TRAP in cpu_loop to deliver SIGTRAP on tw[i]/td[i].
> The si_code comes from do_program_check in the kernel source file
> arch/powerpc/kernel/traps.c
> 
> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
> ---
>   linux-user/ppc/cpu_loop.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/ppc/cpu_loop.c b/linux-user/ppc/cpu_loop.c
> index 46e6ffd6d3..6c99feb19b 100644
> --- a/linux-user/ppc/cpu_loop.c
> +++ b/linux-user/ppc/cpu_loop.c
> @@ -188,7 +188,8 @@ void cpu_loop(CPUPPCState *env)
>                   }
>                   break;
>               case POWERPC_EXCP_TRAP:
> -                cpu_abort(cs, "Tried to call a TRAP\n");
> +                si_signo = TARGET_SIGTRAP;
> +                si_code = TARGET_TRAP_BRKPT;

These lack 'info.'

Thanks,

C.

>                   break;
>               default:
>                   /* Should not happen ! */
> 

Re: [PATCH v2 1/2] linux-user/ppc: deliver SIGTRAP on POWERPC_EXCP_TRAP

[Richard Henderson]

On 1/4/22 10:46 PM, Cédric Le Goater wrote:
> On 1/4/22 22:00, matheus.ferst@eldorado.org.br wrote:
>> From: Matheus Ferst <matheus.ferst@eldorado.org.br>
>>
>> Handle POWERPC_EXCP_TRAP in cpu_loop to deliver SIGTRAP on tw[i]/td[i].
>> The si_code comes from do_program_check in the kernel source file
>> arch/powerpc/kernel/traps.c
>>
>> Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
>> ---
>>   linux-user/ppc/cpu_loop.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/linux-user/ppc/cpu_loop.c b/linux-user/ppc/cpu_loop.c
>> index 46e6ffd6d3..6c99feb19b 100644
>> --- a/linux-user/ppc/cpu_loop.c
>> +++ b/linux-user/ppc/cpu_loop.c
>> @@ -188,7 +188,8 @@ void cpu_loop(CPUPPCState *env)
>>                   }
>>                   break;
>>               case POWERPC_EXCP_TRAP:
>> -                cpu_abort(cs, "Tried to call a TRAP\n");
>> +                si_signo = TARGET_SIGTRAP;
>> +                si_code = TARGET_TRAP_BRKPT;
> 
> These lack 'info.'

You missed the Based-on in the cover-letter.


r~

[PATCH v2 2/2] tests/tcg/ppc64le: change signal_save_restore_xer to use SIGTRAP

[matheus.ferst]

From: Matheus Ferst <matheus.ferst@eldorado.org.br>

Now that linux-user delivers the signal on tw, we can change
signal_save_restore_xer to use SIGTRAP instead of SIGILL.

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Matheus Ferst <matheus.ferst@eldorado.org.br>
---
 tests/tcg/ppc64le/signal_save_restore_xer.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/tcg/ppc64le/signal_save_restore_xer.c b/tests/tcg/ppc64le/signal_save_restore_xer.c
index e4f8a07dd7..9227f4f455 100644
--- a/tests/tcg/ppc64le/signal_save_restore_xer.c
+++ b/tests/tcg/ppc64le/signal_save_restore_xer.c
@@ -11,7 +11,7 @@
 
 uint64_t saved;
 
-void sigill_handler(int sig, siginfo_t *si, void *ucontext)
+void sigtrap_handler(int sig, siginfo_t *si, void *ucontext)
 {
     ucontext_t *uc = ucontext;
     uc->uc_mcontext.regs->nip += 4;
@@ -23,14 +23,14 @@ int main(void)
 {
     uint64_t initial = XER_CA | XER_CA32, restored;
     struct sigaction sa = {
-        .sa_sigaction = sigill_handler,
+        .sa_sigaction = sigtrap_handler,
         .sa_flags = SA_SIGINFO
     };
 
-    sigaction(SIGILL, &sa, NULL);
+    sigaction(SIGTRAP, &sa, NULL);
 
     asm("mtspr 1, %1\n\t"
-        ".long 0x0\n\t"
+        "trap\n\t"
         "mfspr %0, 1\n\t"
         : "=r" (restored)
         : "r" (initial));
-- 
2.25.1