diff mbox series

[v2,27/41] target/i386: Implement x86_cpu_record_sigsegv

Message ID 20210918184527.408540-28-richard.henderson@linaro.org
State Superseded
Headers show
Series linux-user: Streamline handling of SIGSEGV | expand

Commit Message

Richard Henderson Sept. 18, 2021, 6:45 p.m. UTC 
Record cr2, error_code, and exception_index.  That last means
that we must exit to cpu_loop ourselves, instead of letting
exception_index being overwritten.

Use the maperr parameter to properly set PG_ERROR_P_MASK.

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

---
 target/i386/tcg/helper-tcg.h       |  6 ++++++
 target/i386/tcg/tcg-cpu.c          |  3 ++-
 target/i386/tcg/user/excp_helper.c | 23 +++++++++++++++++------
 3 files changed, 25 insertions(+), 7 deletions(-)

-- 
2.25.1

Comments

Philippe Mathieu-Daudé Sept. 19, 2021, 6:32 p.m. UTC | #1 
On 9/18/21 20:45, Richard Henderson wrote:
> Record cr2, error_code, and exception_index.  That last means

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

> exception_index being overwritten.

> 

> Use the maperr parameter to properly set PG_ERROR_P_MASK.

> 

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

> ---

>  target/i386/tcg/helper-tcg.h       |  6 ++++++

>  target/i386/tcg/tcg-cpu.c          |  3 ++-

>  target/i386/tcg/user/excp_helper.c | 23 +++++++++++++++++------

>  3 files changed, 25 insertions(+), 7 deletions(-)


> diff --git a/target/i386/tcg/user/excp_helper.c b/target/i386/tcg/user/excp_helper.c

> index a89b5228fd..cd507e2a1b 100644

> --- a/target/i386/tcg/user/excp_helper.c

> +++ b/target/i386/tcg/user/excp_helper.c

> @@ -22,18 +22,29 @@

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

>  #include "tcg/helper-tcg.h"

>  

> -bool x86_cpu_tlb_fill(CPUState *cs, vaddr addr, int size,

> -                      MMUAccessType access_type, int mmu_idx,

> -                      bool probe, uintptr_t retaddr)

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

> +                            MMUAccessType access_type,

> +                            bool maperr, uintptr_t ra)

>  {

>      X86CPU *cpu = X86_CPU(cs);

>      CPUX86State *env = &cpu->env;

>  

> +    /*

> +     * The error_code that hw reports as part of the exception frame

> +     * is copied to linux sigcontext.err.  The exception_index is

> +     * copied to linux sigcontext.trapno.  Short of inventing a new

> +     * place to store the trapno, we cannot let our caller raise the

> +     * signal and set exception_index to EXCP_INTERRUPT.

> +     */

>      env->cr[2] = addr;

> -    env->error_code = (access_type == MMU_DATA_STORE) << PG_ERROR_W_BIT;

> -    env->error_code |= PG_ERROR_U_MASK;

> +    env->error_code = ((access_type == MMU_DATA_STORE) << PG_ERROR_W_BIT)

> +                    | (maperr ? 0 : PG_ERROR_P_MASK)

> +                    | PG_ERROR_U_MASK;

>      cs->exception_index = EXCP0E_PAGE;

> +

> +    /* Disable do_interrupt_user. */

>      env->exception_is_int = 0;

>      env->exception_next_eip = -1;

> -    cpu_loop_exit_restore(cs, retaddr);

> +

> +    cpu_loop_exit_restore(cs, ra);

>  }

> 


Better have an x86 expert also review this, but to the best
of my knowledge:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>


And YAY! btw, thanks :>
Warner Losh Sept. 19, 2021, 6:59 p.m. UTC | #2 
> On Sep 18, 2021, at 12:45 PM, Richard Henderson <richard.henderson@linaro.org> wrote:

> 

> Record cr2, error_code, and exception_index.  That last means

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

> exception_index being overwritten.

> 

> Use the maperr parameter to properly set PG_ERROR_P_MASK.

> 

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

> ---

> target/i386/tcg/helper-tcg.h       |  6 ++++++

> target/i386/tcg/tcg-cpu.c          |  3 ++-

> target/i386/tcg/user/excp_helper.c | 23 +++++++++++++++++------

> 3 files changed, 25 insertions(+), 7 deletions(-)



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

I think this encoding is fine, but haven’t thought though how I’d implement
this in bsd-user yet…  I have the signal code queued up, but not ready to send
off yet.

> diff --git a/target/i386/tcg/helper-tcg.h b/target/i386/tcg/helper-tcg.h

> index 60ca09e95e..0a4401e917 100644

> --- a/target/i386/tcg/helper-tcg.h

> +++ b/target/i386/tcg/helper-tcg.h

> @@ -43,9 +43,15 @@ bool x86_cpu_exec_interrupt(CPUState *cpu, int int_req);

> #endif

> 

> /* helper.c */

> +#ifdef CONFIG_USER_ONLY

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

> +                            MMUAccessType access_type,

> +                            bool maperr, uintptr_t ra);

> +#else

> bool x86_cpu_tlb_fill(CPUState *cs, vaddr address, int size,

>                       MMUAccessType access_type, int mmu_idx,

>                       bool probe, uintptr_t retaddr);

> +#endif

> 

> void breakpoint_handler(CPUState *cs);

> 

> diff --git a/target/i386/tcg/tcg-cpu.c b/target/i386/tcg/tcg-cpu.c

> index aef050d089..3fab3676b1 100644

> --- a/target/i386/tcg/tcg-cpu.c

> +++ b/target/i386/tcg/tcg-cpu.c

> @@ -77,11 +77,12 @@ static const struct TCGCPUOps x86_tcg_ops = {

>     .synchronize_from_tb = x86_cpu_synchronize_from_tb,

>     .cpu_exec_enter = x86_cpu_exec_enter,

>     .cpu_exec_exit = x86_cpu_exec_exit,

> -    .tlb_fill = x86_cpu_tlb_fill,

> #ifdef CONFIG_USER_ONLY

>     .fake_user_interrupt = x86_cpu_do_interrupt,

> +    .record_sigsegv = x86_cpu_record_sigsegv,

> #else

>     .has_work = x86_cpu_has_work,

> +    .tlb_fill = x86_cpu_tlb_fill,

>     .do_interrupt = x86_cpu_do_interrupt,

>     .cpu_exec_interrupt = x86_cpu_exec_interrupt,

>     .debug_excp_handler = breakpoint_handler,

> diff --git a/target/i386/tcg/user/excp_helper.c b/target/i386/tcg/user/excp_helper.c

> index a89b5228fd..cd507e2a1b 100644

> --- a/target/i386/tcg/user/excp_helper.c

> +++ b/target/i386/tcg/user/excp_helper.c

> @@ -22,18 +22,29 @@

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

> #include "tcg/helper-tcg.h"

> 

> -bool x86_cpu_tlb_fill(CPUState *cs, vaddr addr, int size,

> -                      MMUAccessType access_type, int mmu_idx,

> -                      bool probe, uintptr_t retaddr)

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

> +                            MMUAccessType access_type,

> +                            bool maperr, uintptr_t ra)

> {

>     X86CPU *cpu = X86_CPU(cs);

>     CPUX86State *env = &cpu->env;

> 

> +    /*

> +     * The error_code that hw reports as part of the exception frame

> +     * is copied to linux sigcontext.err.  The exception_index is

> +     * copied to linux sigcontext.trapno.  Short of inventing a new

> +     * place to store the trapno, we cannot let our caller raise the

> +     * signal and set exception_index to EXCP_INTERRUPT.

> +     */

>     env->cr[2] = addr;

> -    env->error_code = (access_type == MMU_DATA_STORE) << PG_ERROR_W_BIT;

> -    env->error_code |= PG_ERROR_U_MASK;

> +    env->error_code = ((access_type == MMU_DATA_STORE) << PG_ERROR_W_BIT)

> +                    | (maperr ? 0 : PG_ERROR_P_MASK)

> +                    | PG_ERROR_U_MASK;

>     cs->exception_index = EXCP0E_PAGE;

> +

> +    /* Disable do_interrupt_user. */

>     env->exception_is_int = 0;

>     env->exception_next_eip = -1;

> -    cpu_loop_exit_restore(cs, retaddr);

> +

> +    cpu_loop_exit_restore(cs, ra);

> }

> -- 

> 2.25.1

> 

>
diff mbox series

Patch

diff --git a/target/i386/tcg/helper-tcg.h b/target/i386/tcg/helper-tcg.h
index 60ca09e95e..0a4401e917 100644
--- a/target/i386/tcg/helper-tcg.h
+++ b/target/i386/tcg/helper-tcg.h
@@ -43,9 +43,15 @@  bool x86_cpu_exec_interrupt(CPUState *cpu, int int_req);
 #endif
 
 /* helper.c */
+#ifdef CONFIG_USER_ONLY
+void x86_cpu_record_sigsegv(CPUState *cs, vaddr addr,
+                            MMUAccessType access_type,
+                            bool maperr, uintptr_t ra);
+#else
 bool x86_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
                       MMUAccessType access_type, int mmu_idx,
                       bool probe, uintptr_t retaddr);
+#endif
 
 void breakpoint_handler(CPUState *cs);
 
diff --git a/target/i386/tcg/tcg-cpu.c b/target/i386/tcg/tcg-cpu.c
index aef050d089..3fab3676b1 100644
--- a/target/i386/tcg/tcg-cpu.c
+++ b/target/i386/tcg/tcg-cpu.c
@@ -77,11 +77,12 @@  static const struct TCGCPUOps x86_tcg_ops = {
     .synchronize_from_tb = x86_cpu_synchronize_from_tb,
     .cpu_exec_enter = x86_cpu_exec_enter,
     .cpu_exec_exit = x86_cpu_exec_exit,
-    .tlb_fill = x86_cpu_tlb_fill,
 #ifdef CONFIG_USER_ONLY
     .fake_user_interrupt = x86_cpu_do_interrupt,
+    .record_sigsegv = x86_cpu_record_sigsegv,
 #else
     .has_work = x86_cpu_has_work,
+    .tlb_fill = x86_cpu_tlb_fill,
     .do_interrupt = x86_cpu_do_interrupt,
     .cpu_exec_interrupt = x86_cpu_exec_interrupt,
     .debug_excp_handler = breakpoint_handler,
diff --git a/target/i386/tcg/user/excp_helper.c b/target/i386/tcg/user/excp_helper.c
index a89b5228fd..cd507e2a1b 100644
--- a/target/i386/tcg/user/excp_helper.c
+++ b/target/i386/tcg/user/excp_helper.c
@@ -22,18 +22,29 @@ 
 #include "exec/exec-all.h"
 #include "tcg/helper-tcg.h"
 
-bool x86_cpu_tlb_fill(CPUState *cs, vaddr addr, int size,
-                      MMUAccessType access_type, int mmu_idx,
-                      bool probe, uintptr_t retaddr)
+void x86_cpu_record_sigsegv(CPUState *cs, vaddr addr,
+                            MMUAccessType access_type,
+                            bool maperr, uintptr_t ra)
 {
     X86CPU *cpu = X86_CPU(cs);
     CPUX86State *env = &cpu->env;
 
+    /*
+     * The error_code that hw reports as part of the exception frame
+     * is copied to linux sigcontext.err.  The exception_index is
+     * copied to linux sigcontext.trapno.  Short of inventing a new
+     * place to store the trapno, we cannot let our caller raise the
+     * signal and set exception_index to EXCP_INTERRUPT.
+     */
     env->cr[2] = addr;
-    env->error_code = (access_type == MMU_DATA_STORE) << PG_ERROR_W_BIT;
-    env->error_code |= PG_ERROR_U_MASK;
+    env->error_code = ((access_type == MMU_DATA_STORE) << PG_ERROR_W_BIT)
+                    | (maperr ? 0 : PG_ERROR_P_MASK)
+                    | PG_ERROR_U_MASK;
     cs->exception_index = EXCP0E_PAGE;
+
+    /* Disable do_interrupt_user. */
     env->exception_is_int = 0;
     env->exception_next_eip = -1;
-    cpu_loop_exit_restore(cs, retaddr);
+
+    cpu_loop_exit_restore(cs, ra);
 }