diff mbox series

[v5,34/67] target/ppc: Implement ppc_cpu_record_sigsegv

Message ID 20211015041053.2769193-35-richard.henderson@linaro.org
State Superseded
Headers show
Series user-only: Cleanup SIGSEGV and SIGBUS handling | expand

Commit Message

Richard Henderson Oct. 15, 2021, 4:10 a.m. UTC 
Record DAR, DSISR, and exception_index.  That last means
that we must exit to cpu_loop ourselves, instead of letting
exception_index being overwritten.

This is exactly what the user-mode ppc_cpu_tlb_fill does,
so simply rename it as ppc_cpu_record_sigsegv.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

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

---
 target/ppc/cpu.h              |  3 ---
 target/ppc/internal.h         |  9 +++++++++
 target/ppc/cpu_init.c         |  6 ++++--
 target/ppc/user_only_helper.c | 15 +++++++++++----
 4 files changed, 24 insertions(+), 9 deletions(-)

-- 
2.25.1

Comments

Warner Losh Oct. 15, 2021, 6:45 p.m. UTC | #1 
On Thu, Oct 14, 2021 at 10:11 PM Richard Henderson <
richard.henderson@linaro.org> wrote:

> Record DAR, DSISR, and exception_index.  That last means

> that we must exit to cpu_loop ourselves, instead of letting

> exception_index being overwritten.

>

> This is exactly what the user-mode ppc_cpu_tlb_fill does,

> so simply rename it as ppc_cpu_record_sigsegv.

>

> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

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

> ---

>  target/ppc/cpu.h              |  3 ---

>  target/ppc/internal.h         |  9 +++++++++

>  target/ppc/cpu_init.c         |  6 ++++--

>  target/ppc/user_only_helper.c | 15 +++++++++++----

>  4 files changed, 24 insertions(+), 9 deletions(-)

>


Reviewed-by: Warner Losh <imp@bsdimp.com>



> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h

> index baa4e7c34d..2242d57718 100644

> --- a/target/ppc/cpu.h

> +++ b/target/ppc/cpu.h

> @@ -1279,9 +1279,6 @@ extern const VMStateDescription vmstate_ppc_cpu;

>

>

>  /*****************************************************************************/

>  void ppc_translate_init(void);

> -bool ppc_cpu_tlb_fill(CPUState *cs, vaddr address, int size,

> -                      MMUAccessType access_type, int mmu_idx,

> -                      bool probe, uintptr_t retaddr);

>

>  #if !defined(CONFIG_USER_ONLY)

>  void ppc_store_sdr1(CPUPPCState *env, target_ulong value);

> diff --git a/target/ppc/internal.h b/target/ppc/internal.h

> index 55284369f5..339974b7d8 100644

> --- a/target/ppc/internal.h

> +++ b/target/ppc/internal.h

> @@ -283,5 +283,14 @@ static inline void pte_invalidate(target_ulong *pte0)

>  #define PTE_PTEM_MASK 0x7FFFFFBF

>  #define PTE_CHECK_MASK (TARGET_PAGE_MASK | 0x7B)

>

> +#ifdef CONFIG_USER_ONLY

> +void ppc_cpu_record_sigsegv(CPUState *cs, vaddr addr,

> +                            MMUAccessType access_type,

> +                            bool maperr, uintptr_t ra);

> +#else

> +bool ppc_cpu_tlb_fill(CPUState *cs, vaddr address, int size,

> +                      MMUAccessType access_type, int mmu_idx,

> +                      bool probe, uintptr_t retaddr);

> +#endif

>

>  #endif /* PPC_INTERNAL_H */

> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c

> index 6aad01d1d3..ec8da08f0b 100644

> --- a/target/ppc/cpu_init.c

> +++ b/target/ppc/cpu_init.c

> @@ -9014,9 +9014,11 @@ static const struct SysemuCPUOps ppc_sysemu_ops = {

>

>  static const struct TCGCPUOps ppc_tcg_ops = {

>    .initialize = ppc_translate_init,

> -  .tlb_fill = ppc_cpu_tlb_fill,

>

> -#ifndef CONFIG_USER_ONLY

> +#ifdef CONFIG_USER_ONLY

> +  .record_sigsegv = ppc_cpu_record_sigsegv,

> +#else

> +  .tlb_fill = ppc_cpu_tlb_fill,

>    .cpu_exec_interrupt = ppc_cpu_exec_interrupt,

>    .do_interrupt = ppc_cpu_do_interrupt,

>    .cpu_exec_enter = ppc_cpu_exec_enter,

> diff --git a/target/ppc/user_only_helper.c b/target/ppc/user_only_helper.c

> index aa3f867596..7ff76f7a06 100644

> --- a/target/ppc/user_only_helper.c

> +++ b/target/ppc/user_only_helper.c

> @@ -21,16 +21,23 @@

>  #include "qemu/osdep.h"

>  #include "cpu.h"

>  #include "exec/exec-all.h"

> +#include "internal.h"

>

> -

> -bool ppc_cpu_tlb_fill(CPUState *cs, vaddr address, int size,

> -                      MMUAccessType access_type, int mmu_idx,

> -                      bool probe, uintptr_t retaddr)

> +void ppc_cpu_record_sigsegv(CPUState *cs, vaddr address,

> +                            MMUAccessType access_type,

> +                            bool maperr, uintptr_t retaddr)

>  {

>      PowerPCCPU *cpu = POWERPC_CPU(cs);

>      CPUPPCState *env = &cpu->env;

>      int exception, error_code;

>

> +    /*

> +     * Both DSISR and the "trap number" (exception vector offset,

> +     * looked up from exception_index) are present in the linux-user

> +     * signal frame.

> +     * FIXME: we don't actually populate the trap number properly.

> +     * It would be easiest to fill in an env->trap value now.

> +     */

>


I think the same concerns apply to bsd-user, though
the details differ since trap frames only fill in information
relevant to the specific trap type. This may require some
refinement in the future when it's time to upstream bsd-user
ppc support. I'll revisit this, though, when that time comes.

Warner


>      if (access_type == MMU_INST_FETCH) {

>          exception = POWERPC_EXCP_ISI;

>          error_code = 0x40000000;

> --

> 2.25.1

>

>
<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Oct 14, 2021 at 10:11 PM Richard Henderson &lt;<a href="mailto:richard.henderson@linaro.org">richard.henderson@linaro.org</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Record DAR, DSISR, and exception_index.  That last means<br>
that we must exit to cpu_loop ourselves, instead of letting<br>
exception_index being overwritten.<br>
<br>
This is exactly what the user-mode ppc_cpu_tlb_fill does,<br>
so simply rename it as ppc_cpu_record_sigsegv.<br>
<br>
Reviewed-by: Philippe Mathieu-Daudé &lt;<a href="mailto:f4bug@amsat.org" target="_blank">f4bug@amsat.org</a>&gt;<br>

Signed-off-by: Richard Henderson &lt;<a href="mailto:richard.henderson@linaro.org" target="_blank">richard.henderson@linaro.org</a>&gt;<br>

---<br>
 target/ppc/cpu.h              |  3 ---<br>
 target/ppc/internal.h         |  9 +++++++++<br>
 target/ppc/cpu_init.c         |  6 ++++--<br>
 target/ppc/user_only_helper.c | 15 +++++++++++----<br>
 4 files changed, 24 insertions(+), 9 deletions(-)<br></blockquote><div><br></div><div><div>Reviewed-by: Warner Losh &lt;<a href="mailto:imp@bsdimp.com">imp@bsdimp.com</a>&gt;</div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h<br>
index baa4e7c34d..2242d57718 100644<br>
--- a/target/ppc/cpu.h<br>
+++ b/target/ppc/cpu.h<br>
@@ -1279,9 +1279,6 @@ extern const VMStateDescription vmstate_ppc_cpu;<br>
<br>
 /*****************************************************************************/<br>
 void ppc_translate_init(void);<br>
-bool ppc_cpu_tlb_fill(CPUState *cs, vaddr address, int size,<br>
-                      MMUAccessType access_type, int mmu_idx,<br>
-                      bool probe, uintptr_t retaddr);<br>
<br>
 #if !defined(CONFIG_USER_ONLY)<br>
 void ppc_store_sdr1(CPUPPCState *env, target_ulong value);<br>
diff --git a/target/ppc/internal.h b/target/ppc/internal.h<br>
index 55284369f5..339974b7d8 100644<br>
--- a/target/ppc/internal.h<br>
+++ b/target/ppc/internal.h<br>
@@ -283,5 +283,14 @@ static inline void pte_invalidate(target_ulong *pte0)<br>
 #define PTE_PTEM_MASK 0x7FFFFFBF<br>
 #define PTE_CHECK_MASK (TARGET_PAGE_MASK | 0x7B)<br>
<br>
+#ifdef CONFIG_USER_ONLY<br>
+void ppc_cpu_record_sigsegv(CPUState *cs, vaddr addr,<br>
+                            MMUAccessType access_type,<br>
+                            bool maperr, uintptr_t ra);<br>
+#else<br>
+bool ppc_cpu_tlb_fill(CPUState *cs, vaddr address, int size,<br>
+                      MMUAccessType access_type, int mmu_idx,<br>
+                      bool probe, uintptr_t retaddr);<br>
+#endif<br>
<br>
 #endif /* PPC_INTERNAL_H */<br>
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c<br>
index 6aad01d1d3..ec8da08f0b 100644<br>
--- a/target/ppc/cpu_init.c<br>
+++ b/target/ppc/cpu_init.c<br>
@@ -9014,9 +9014,11 @@ static const struct SysemuCPUOps ppc_sysemu_ops = {<br>
<br>
 static const struct TCGCPUOps ppc_tcg_ops = {<br>
   .initialize = ppc_translate_init,<br>
-  .tlb_fill = ppc_cpu_tlb_fill,<br>
<br>
-#ifndef CONFIG_USER_ONLY<br>
+#ifdef CONFIG_USER_ONLY<br>
+  .record_sigsegv = ppc_cpu_record_sigsegv,<br>
+#else<br>
+  .tlb_fill = ppc_cpu_tlb_fill,<br>
   .cpu_exec_interrupt = ppc_cpu_exec_interrupt,<br>
   .do_interrupt = ppc_cpu_do_interrupt,<br>
   .cpu_exec_enter = ppc_cpu_exec_enter,<br>
diff --git a/target/ppc/user_only_helper.c b/target/ppc/user_only_helper.c<br>
index aa3f867596..7ff76f7a06 100644<br>
--- a/target/ppc/user_only_helper.c<br>
+++ b/target/ppc/user_only_helper.c<br>
@@ -21,16 +21,23 @@<br>
 #include &quot;qemu/osdep.h&quot;<br>
 #include &quot;cpu.h&quot;<br>
 #include &quot;exec/exec-all.h&quot;<br>
+#include &quot;internal.h&quot;<br>
<br>
-<br>
-bool ppc_cpu_tlb_fill(CPUState *cs, vaddr address, int size,<br>
-                      MMUAccessType access_type, int mmu_idx,<br>
-                      bool probe, uintptr_t retaddr)<br>
+void ppc_cpu_record_sigsegv(CPUState *cs, vaddr address,<br>
+                            MMUAccessType access_type,<br>
+                            bool maperr, uintptr_t retaddr)<br>
 {<br>
     PowerPCCPU *cpu = POWERPC_CPU(cs);<br>
     CPUPPCState *env = &amp;cpu-&gt;env;<br>
     int exception, error_code;<br>
<br>
+    /*<br>
+     * Both DSISR and the &quot;trap number&quot; (exception vector offset,<br>
+     * looked up from exception_index) are present in the linux-user<br>
+     * signal frame.<br>
+     * FIXME: we don&#39;t actually populate the trap number properly.<br>
+     * It would be easiest to fill in an env-&gt;trap value now.<br>
+     */<br></blockquote><div><br></div><div>I think the same concerns apply to bsd-user, though</div><div>the details differ since trap frames only fill in information</div><div>relevant to the specific trap type. This may require some</div><div>refinement in the future when it&#39;s time to upstream bsd-user</div><div>ppc support. I&#39;ll revisit this, though, when that time comes.</div><div><br></div><div>Warner</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
     if (access_type == MMU_INST_FETCH) {<br>
         exception = POWERPC_EXCP_ISI;<br>
         error_code = 0x40000000;<br>
-- <br>
2.25.1<br>
<br>
</blockquote></div></div>
diff mbox series

Patch

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index baa4e7c34d..2242d57718 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1279,9 +1279,6 @@  extern const VMStateDescription vmstate_ppc_cpu;
 
 /*****************************************************************************/
 void ppc_translate_init(void);
-bool ppc_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
-                      MMUAccessType access_type, int mmu_idx,
-                      bool probe, uintptr_t retaddr);
 
 #if !defined(CONFIG_USER_ONLY)
 void ppc_store_sdr1(CPUPPCState *env, target_ulong value);
diff --git a/target/ppc/internal.h b/target/ppc/internal.h
index 55284369f5..339974b7d8 100644
--- a/target/ppc/internal.h
+++ b/target/ppc/internal.h
@@ -283,5 +283,14 @@  static inline void pte_invalidate(target_ulong *pte0)
 #define PTE_PTEM_MASK 0x7FFFFFBF
 #define PTE_CHECK_MASK (TARGET_PAGE_MASK | 0x7B)
 
+#ifdef CONFIG_USER_ONLY
+void ppc_cpu_record_sigsegv(CPUState *cs, vaddr addr,
+                            MMUAccessType access_type,
+                            bool maperr, uintptr_t ra);
+#else
+bool ppc_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
+                      MMUAccessType access_type, int mmu_idx,
+                      bool probe, uintptr_t retaddr);
+#endif
 
 #endif /* PPC_INTERNAL_H */
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 6aad01d1d3..ec8da08f0b 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -9014,9 +9014,11 @@  static const struct SysemuCPUOps ppc_sysemu_ops = {
 
 static const struct TCGCPUOps ppc_tcg_ops = {
   .initialize = ppc_translate_init,
-  .tlb_fill = ppc_cpu_tlb_fill,
 
-#ifndef CONFIG_USER_ONLY
+#ifdef CONFIG_USER_ONLY
+  .record_sigsegv = ppc_cpu_record_sigsegv,
+#else
+  .tlb_fill = ppc_cpu_tlb_fill,
   .cpu_exec_interrupt = ppc_cpu_exec_interrupt,
   .do_interrupt = ppc_cpu_do_interrupt,
   .cpu_exec_enter = ppc_cpu_exec_enter,
diff --git a/target/ppc/user_only_helper.c b/target/ppc/user_only_helper.c
index aa3f867596..7ff76f7a06 100644
--- a/target/ppc/user_only_helper.c
+++ b/target/ppc/user_only_helper.c
@@ -21,16 +21,23 @@ 
 #include "qemu/osdep.h"
 #include "cpu.h"
 #include "exec/exec-all.h"
+#include "internal.h"
 
-
-bool ppc_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
-                      MMUAccessType access_type, int mmu_idx,
-                      bool probe, uintptr_t retaddr)
+void ppc_cpu_record_sigsegv(CPUState *cs, vaddr address,
+                            MMUAccessType access_type,
+                            bool maperr, uintptr_t retaddr)
 {
     PowerPCCPU *cpu = POWERPC_CPU(cs);
     CPUPPCState *env = &cpu->env;
     int exception, error_code;
 
+    /*
+     * Both DSISR and the "trap number" (exception vector offset,
+     * looked up from exception_index) are present in the linux-user
+     * signal frame.
+     * FIXME: we don't actually populate the trap number properly.
+     * It would be easiest to fill in an env->trap value now.
+     */
     if (access_type == MMU_INST_FETCH) {
         exception = POWERPC_EXCP_ISI;
         error_code = 0x40000000;