diff mbox series

[v5,23/67] target/arm: Implement arm_cpu_record_sigsegv

Message ID 20211015041053.2769193-24-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 
Because of the complexity of setting ESR, continue to use
arm_deliver_fault.  This means we cannot remove the code
within cpu_loop that decodes EXCP_DATA_ABORT and
EXCP_PREFETCH_ABORT.

But using the new hook means that we don't have to do the
page_get_flags check manually, and we'll be able to restrict
the tlb_fill hook to sysemu later.

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

---
 target/arm/internals.h  |  6 ++++++
 target/arm/cpu.c        |  6 ++++--
 target/arm/cpu_tcg.c    |  6 ++++--
 target/arm/tlb_helper.c | 36 +++++++++++++++++++-----------------
 4 files changed, 33 insertions(+), 21 deletions(-)

-- 
2.25.1

Comments

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

> Because of the complexity of setting ESR, continue to use

> arm_deliver_fault.  This means we cannot remove the code

> within cpu_loop that decodes EXCP_DATA_ABORT and

> EXCP_PREFETCH_ABORT.

>

> But using the new hook means that we don't have to do the

> page_get_flags check manually, and we'll be able to restrict

> the tlb_fill hook to sysemu later.

>

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

> ---

>  target/arm/internals.h  |  6 ++++++

>  target/arm/cpu.c        |  6 ++++--

>  target/arm/cpu_tcg.c    |  6 ++++--

>  target/arm/tlb_helper.c | 36 +++++++++++++++++++-----------------

>  4 files changed, 33 insertions(+), 21 deletions(-)

>


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




> diff --git a/target/arm/internals.h b/target/arm/internals.h

> index 3612107ab2..5a7aaf0f51 100644

> --- a/target/arm/internals.h

> +++ b/target/arm/internals.h

> @@ -544,9 +544,15 @@ static inline bool arm_extabort_type(MemTxResult

> result)

>      return result != MEMTX_DECODE_ERROR;

>  }

>

> +#ifdef CONFIG_USER_ONLY

> +void arm_cpu_record_sigsegv(CPUState *cpu, vaddr addr,

> +                            MMUAccessType access_type,

> +                            bool maperr, uintptr_t ra);

> +#else

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

>                        MMUAccessType access_type, int mmu_idx,

>                        bool probe, uintptr_t retaddr);

> +#endif

>

>  static inline int arm_to_core_mmu_idx(ARMMMUIdx mmu_idx)

>  {

> diff --git a/target/arm/cpu.c b/target/arm/cpu.c

> index 641a8c2d3d..7a18a58ca0 100644

> --- a/target/arm/cpu.c

> +++ b/target/arm/cpu.c

> @@ -2031,10 +2031,12 @@ static const struct SysemuCPUOps arm_sysemu_ops = {

>  static const struct TCGCPUOps arm_tcg_ops = {

>      .initialize = arm_translate_init,

>      .synchronize_from_tb = arm_cpu_synchronize_from_tb,

> -    .tlb_fill = arm_cpu_tlb_fill,

>      .debug_excp_handler = arm_debug_excp_handler,

>

> -#if !defined(CONFIG_USER_ONLY)

> +#ifdef CONFIG_USER_ONLY

> +    .record_sigsegv = arm_cpu_record_sigsegv,

> +#else

> +    .tlb_fill = arm_cpu_tlb_fill,

>      .cpu_exec_interrupt = arm_cpu_exec_interrupt,

>      .do_interrupt = arm_cpu_do_interrupt,

>      .do_transaction_failed = arm_cpu_do_transaction_failed,

> diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c

> index 0d5adccf1a..7b3bea2fbb 100644

> --- a/target/arm/cpu_tcg.c

> +++ b/target/arm/cpu_tcg.c

> @@ -898,10 +898,12 @@ static void pxa270c5_initfn(Object *obj)

>  static const struct TCGCPUOps arm_v7m_tcg_ops = {

>      .initialize = arm_translate_init,

>      .synchronize_from_tb = arm_cpu_synchronize_from_tb,

> -    .tlb_fill = arm_cpu_tlb_fill,

>      .debug_excp_handler = arm_debug_excp_handler,

>

> -#if !defined(CONFIG_USER_ONLY)

> +#ifdef CONFIG_USER_ONLY

> +    .record_sigsegv = arm_cpu_record_sigsegv,

> +#else

> +    .tlb_fill = arm_cpu_tlb_fill,

>      .cpu_exec_interrupt = arm_v7m_cpu_exec_interrupt,

>      .do_interrupt = arm_v7m_cpu_do_interrupt,

>      .do_transaction_failed = arm_cpu_do_transaction_failed,

> diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c

> index 3107f9823e..dc5860180f 100644

> --- a/target/arm/tlb_helper.c

> +++ b/target/arm/tlb_helper.c

> @@ -147,28 +147,12 @@ void arm_cpu_do_transaction_failed(CPUState *cs,

> hwaddr physaddr,

>      arm_deliver_fault(cpu, addr, access_type, mmu_idx, &fi);

>  }

>

> -#endif /* !defined(CONFIG_USER_ONLY) */

> -

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

>                        MMUAccessType access_type, int mmu_idx,

>                        bool probe, uintptr_t retaddr)

>  {

>      ARMCPU *cpu = ARM_CPU(cs);

>      ARMMMUFaultInfo fi = {};

> -

> -#ifdef CONFIG_USER_ONLY

> -    int flags = page_get_flags(useronly_clean_ptr(address));

> -    if (flags & PAGE_VALID) {

> -        fi.type = ARMFault_Permission;

> -    } else {

> -        fi.type = ARMFault_Translation;

> -    }

> -    fi.level = 3;

> -

> -    /* now we have a real cpu fault */

> -    cpu_restore_state(cs, retaddr, true);

> -    arm_deliver_fault(cpu, address, access_type, mmu_idx, &fi);

> -#else

>      hwaddr phys_addr;

>      target_ulong page_size;

>      int prot, ret;

> @@ -210,5 +194,23 @@ bool arm_cpu_tlb_fill(CPUState *cs, vaddr address,

> int size,

>          cpu_restore_state(cs, retaddr, true);

>          arm_deliver_fault(cpu, address, access_type, mmu_idx, &fi);

>      }

> -#endif

>  }

> +#else

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

> +                            MMUAccessType access_type,

> +                            bool maperr, uintptr_t ra)

> +{

> +    ARMMMUFaultInfo fi = {

> +        .type = maperr ? ARMFault_Translation : ARMFault_Permission,

> +        .level = 3,

> +    };

> +    ARMCPU *cpu = ARM_CPU(cs);

> +

> +    /*

> +     * We report both ESR and FAR to signal handlers.

> +     * For now, it's easiest to deliver the fault normally.

> +     */

> +    cpu_restore_state(cs, ra, true);

> +    arm_deliver_fault(cpu, addr, access_type, MMU_USER_IDX, &fi);

> +}

> +#endif /* !defined(CONFIG_USER_ONLY) */

> --

> 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">Because of the complexity of setting ESR, continue to use<br>
arm_deliver_fault.  This means we cannot remove the code<br>
within cpu_loop that decodes EXCP_DATA_ABORT and<br>
EXCP_PREFETCH_ABORT.<br>
<br>
But using the new hook means that we don&#39;t have to do the<br>
page_get_flags check manually, and we&#39;ll be able to restrict<br>
the tlb_fill hook to sysemu later.<br>
<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/arm/internals.h  |  6 ++++++<br>
 target/arm/cpu.c        |  6 ++++--<br>
 target/arm/cpu_tcg.c    |  6 ++++--<br>
 target/arm/tlb_helper.c | 36 +++++++++++++++++++-----------------<br>
 4 files changed, 33 insertions(+), 21 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><br class="gmail-Apple-interchange-newline"></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/arm/internals.h b/target/arm/internals.h<br>
index 3612107ab2..5a7aaf0f51 100644<br>
--- a/target/arm/internals.h<br>
+++ b/target/arm/internals.h<br>
@@ -544,9 +544,15 @@ static inline bool arm_extabort_type(MemTxResult result)<br>
     return result != MEMTX_DECODE_ERROR;<br>
 }<br>
<br>
+#ifdef CONFIG_USER_ONLY<br>
+void arm_cpu_record_sigsegv(CPUState *cpu, vaddr addr,<br>
+                            MMUAccessType access_type,<br>
+                            bool maperr, uintptr_t ra);<br>
+#else<br>
 bool arm_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>
 static inline int arm_to_core_mmu_idx(ARMMMUIdx mmu_idx)<br>
 {<br>
diff --git a/target/arm/cpu.c b/target/arm/cpu.c<br>
index 641a8c2d3d..7a18a58ca0 100644<br>
--- a/target/arm/cpu.c<br>
+++ b/target/arm/cpu.c<br>
@@ -2031,10 +2031,12 @@ static const struct SysemuCPUOps arm_sysemu_ops = {<br>
 static const struct TCGCPUOps arm_tcg_ops = {<br>
     .initialize = arm_translate_init,<br>
     .synchronize_from_tb = arm_cpu_synchronize_from_tb,<br>
-    .tlb_fill = arm_cpu_tlb_fill,<br>
     .debug_excp_handler = arm_debug_excp_handler,<br>
<br>
-#if !defined(CONFIG_USER_ONLY)<br>
+#ifdef CONFIG_USER_ONLY<br>
+    .record_sigsegv = arm_cpu_record_sigsegv,<br>
+#else<br>
+    .tlb_fill = arm_cpu_tlb_fill,<br>
     .cpu_exec_interrupt = arm_cpu_exec_interrupt,<br>
     .do_interrupt = arm_cpu_do_interrupt,<br>
     .do_transaction_failed = arm_cpu_do_transaction_failed,<br>
diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c<br>
index 0d5adccf1a..7b3bea2fbb 100644<br>
--- a/target/arm/cpu_tcg.c<br>
+++ b/target/arm/cpu_tcg.c<br>
@@ -898,10 +898,12 @@ static void pxa270c5_initfn(Object *obj)<br>
 static const struct TCGCPUOps arm_v7m_tcg_ops = {<br>
     .initialize = arm_translate_init,<br>
     .synchronize_from_tb = arm_cpu_synchronize_from_tb,<br>
-    .tlb_fill = arm_cpu_tlb_fill,<br>
     .debug_excp_handler = arm_debug_excp_handler,<br>
<br>
-#if !defined(CONFIG_USER_ONLY)<br>
+#ifdef CONFIG_USER_ONLY<br>
+    .record_sigsegv = arm_cpu_record_sigsegv,<br>
+#else<br>
+    .tlb_fill = arm_cpu_tlb_fill,<br>
     .cpu_exec_interrupt = arm_v7m_cpu_exec_interrupt,<br>
     .do_interrupt = arm_v7m_cpu_do_interrupt,<br>
     .do_transaction_failed = arm_cpu_do_transaction_failed,<br>
diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c<br>
index 3107f9823e..dc5860180f 100644<br>
--- a/target/arm/tlb_helper.c<br>
+++ b/target/arm/tlb_helper.c<br>
@@ -147,28 +147,12 @@ void arm_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,<br>
     arm_deliver_fault(cpu, addr, access_type, mmu_idx, &amp;fi);<br>
 }<br>
<br>
-#endif /* !defined(CONFIG_USER_ONLY) */<br>
-<br>
 bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int size,<br>
                       MMUAccessType access_type, int mmu_idx,<br>
                       bool probe, uintptr_t retaddr)<br>
 {<br>
     ARMCPU *cpu = ARM_CPU(cs);<br>
     ARMMMUFaultInfo fi = {};<br>
-<br>
-#ifdef CONFIG_USER_ONLY<br>
-    int flags = page_get_flags(useronly_clean_ptr(address));<br>
-    if (flags &amp; PAGE_VALID) {<br>
-        fi.type = ARMFault_Permission;<br>
-    } else {<br>
-        fi.type = ARMFault_Translation;<br>
-    }<br>
-    fi.level = 3;<br>
-<br>
-    /* now we have a real cpu fault */<br>
-    cpu_restore_state(cs, retaddr, true);<br>
-    arm_deliver_fault(cpu, address, access_type, mmu_idx, &amp;fi);<br>
-#else<br>
     hwaddr phys_addr;<br>
     target_ulong page_size;<br>
     int prot, ret;<br>
@@ -210,5 +194,23 @@ bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int size,<br>
         cpu_restore_state(cs, retaddr, true);<br>
         arm_deliver_fault(cpu, address, access_type, mmu_idx, &amp;fi);<br>
     }<br>
-#endif<br>
 }<br>
+#else<br>
+void arm_cpu_record_sigsegv(CPUState *cs, vaddr addr,<br>
+                            MMUAccessType access_type,<br>
+                            bool maperr, uintptr_t ra)<br>
+{<br>
+    ARMMMUFaultInfo fi = {<br>
+        .type = maperr ? ARMFault_Translation : ARMFault_Permission,<br>
+        .level = 3,<br>
+    };<br>
+    ARMCPU *cpu = ARM_CPU(cs);<br>
+<br>
+    /*<br>
+     * We report both ESR and FAR to signal handlers.<br>
+     * For now, it&#39;s easiest to deliver the fault normally.<br>
+     */<br>
+    cpu_restore_state(cs, ra, true);<br>
+    arm_deliver_fault(cpu, addr, access_type, MMU_USER_IDX, &amp;fi);<br>
+}<br>
+#endif /* !defined(CONFIG_USER_ONLY) */<br>
-- <br>
2.25.1<br>
<br>
</blockquote></div></div>
diff mbox series

Patch

diff --git a/target/arm/internals.h b/target/arm/internals.h
index 3612107ab2..5a7aaf0f51 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -544,9 +544,15 @@  static inline bool arm_extabort_type(MemTxResult result)
     return result != MEMTX_DECODE_ERROR;
 }
 
+#ifdef CONFIG_USER_ONLY
+void arm_cpu_record_sigsegv(CPUState *cpu, vaddr addr,
+                            MMUAccessType access_type,
+                            bool maperr, uintptr_t ra);
+#else
 bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
                       MMUAccessType access_type, int mmu_idx,
                       bool probe, uintptr_t retaddr);
+#endif
 
 static inline int arm_to_core_mmu_idx(ARMMMUIdx mmu_idx)
 {
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 641a8c2d3d..7a18a58ca0 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2031,10 +2031,12 @@  static const struct SysemuCPUOps arm_sysemu_ops = {
 static const struct TCGCPUOps arm_tcg_ops = {
     .initialize = arm_translate_init,
     .synchronize_from_tb = arm_cpu_synchronize_from_tb,
-    .tlb_fill = arm_cpu_tlb_fill,
     .debug_excp_handler = arm_debug_excp_handler,
 
-#if !defined(CONFIG_USER_ONLY)
+#ifdef CONFIG_USER_ONLY
+    .record_sigsegv = arm_cpu_record_sigsegv,
+#else
+    .tlb_fill = arm_cpu_tlb_fill,
     .cpu_exec_interrupt = arm_cpu_exec_interrupt,
     .do_interrupt = arm_cpu_do_interrupt,
     .do_transaction_failed = arm_cpu_do_transaction_failed,
diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c
index 0d5adccf1a..7b3bea2fbb 100644
--- a/target/arm/cpu_tcg.c
+++ b/target/arm/cpu_tcg.c
@@ -898,10 +898,12 @@  static void pxa270c5_initfn(Object *obj)
 static const struct TCGCPUOps arm_v7m_tcg_ops = {
     .initialize = arm_translate_init,
     .synchronize_from_tb = arm_cpu_synchronize_from_tb,
-    .tlb_fill = arm_cpu_tlb_fill,
     .debug_excp_handler = arm_debug_excp_handler,
 
-#if !defined(CONFIG_USER_ONLY)
+#ifdef CONFIG_USER_ONLY
+    .record_sigsegv = arm_cpu_record_sigsegv,
+#else
+    .tlb_fill = arm_cpu_tlb_fill,
     .cpu_exec_interrupt = arm_v7m_cpu_exec_interrupt,
     .do_interrupt = arm_v7m_cpu_do_interrupt,
     .do_transaction_failed = arm_cpu_do_transaction_failed,
diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c
index 3107f9823e..dc5860180f 100644
--- a/target/arm/tlb_helper.c
+++ b/target/arm/tlb_helper.c
@@ -147,28 +147,12 @@  void arm_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
     arm_deliver_fault(cpu, addr, access_type, mmu_idx, &fi);
 }
 
-#endif /* !defined(CONFIG_USER_ONLY) */
-
 bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
                       MMUAccessType access_type, int mmu_idx,
                       bool probe, uintptr_t retaddr)
 {
     ARMCPU *cpu = ARM_CPU(cs);
     ARMMMUFaultInfo fi = {};
-
-#ifdef CONFIG_USER_ONLY
-    int flags = page_get_flags(useronly_clean_ptr(address));
-    if (flags & PAGE_VALID) {
-        fi.type = ARMFault_Permission;
-    } else {
-        fi.type = ARMFault_Translation;
-    }
-    fi.level = 3;
-
-    /* now we have a real cpu fault */
-    cpu_restore_state(cs, retaddr, true);
-    arm_deliver_fault(cpu, address, access_type, mmu_idx, &fi);
-#else
     hwaddr phys_addr;
     target_ulong page_size;
     int prot, ret;
@@ -210,5 +194,23 @@  bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
         cpu_restore_state(cs, retaddr, true);
         arm_deliver_fault(cpu, address, access_type, mmu_idx, &fi);
     }
-#endif
 }
+#else
+void arm_cpu_record_sigsegv(CPUState *cs, vaddr addr,
+                            MMUAccessType access_type,
+                            bool maperr, uintptr_t ra)
+{
+    ARMMMUFaultInfo fi = {
+        .type = maperr ? ARMFault_Translation : ARMFault_Permission,
+        .level = 3,
+    };
+    ARMCPU *cpu = ARM_CPU(cs);
+
+    /*
+     * We report both ESR and FAR to signal handlers.
+     * For now, it's easiest to deliver the fault normally.
+     */
+    cpu_restore_state(cs, ra, true);
+    arm_deliver_fault(cpu, addr, access_type, MMU_USER_IDX, &fi);
+}
+#endif /* !defined(CONFIG_USER_ONLY) */