[PATCH 08/30] bsd-user/arm/target_arch_cpu.h: Implement data faults

Warner Losh posted 30 patches 4 years ago
There is a newer version of this series
[PATCH 08/30] bsd-user/arm/target_arch_cpu.h: Implement data faults
Posted by Warner Losh 4 years ago
Update for the richer set of data faults that are now possible. Copied
largely from linux-user/arm/cpu_loop.c

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/arm/target_arch_cpu.h | 44 ++++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/bsd-user/arm/target_arch_cpu.h b/bsd-user/arm/target_arch_cpu.h
index 996a361e3fe..51e592bcfe7 100644
--- a/bsd-user/arm/target_arch_cpu.h
+++ b/bsd-user/arm/target_arch_cpu.h
@@ -39,8 +39,7 @@ static inline void target_cpu_init(CPUARMState *env,
 
 static inline void target_cpu_loop(CPUARMState *env)
 {
-    int trapnr;
-    target_siginfo_t info;
+    int trapnr, si_signo, si_code;
     unsigned int n;
     CPUState *cs = env_cpu(env);
 
@@ -143,15 +142,40 @@ static inline void target_cpu_loop(CPUARMState *env)
             /* just indicate that signals should be handled asap */
             break;
         case EXCP_PREFETCH_ABORT:
-            /* See arm/arm/trap.c prefetch_abort_handler() */
         case EXCP_DATA_ABORT:
-            /* See arm/arm/trap.c data_abort_handler() */
-            info.si_signo = TARGET_SIGSEGV;
-            info.si_errno = 0;
-            /* XXX: check env->error_code */
-            info.si_code = 0;
-            info.si_addr = env->exception.vaddress;
-            queue_signal(env, info.si_signo, &info);
+            /*
+             * See arm/arm/trap-v6.c prefetch_abort_handler() and data_abort_handler()
+             *
+             * However, FreeBSD maps these to a generic value and then uses that
+             * to maybe fault in pages in vm/vm_fault.c:vm_fault_trap(). I
+             * believe that the indirection maps the same as Linux, but haven't
+             * chased down every single possible indirection.
+             */
+
+            /* For user-only we don't set TTBCR_EAE, so look at the FSR. */
+            switch (env->exception.fsr & 0x1f) {
+            case 0x1: /* Alignment */
+                si_signo = TARGET_SIGBUS;
+                si_code = TARGET_BUS_ADRALN;
+                break;
+            case 0x3: /* Access flag fault, level 1 */
+            case 0x6: /* Access flag fault, level 2 */
+            case 0x9: /* Domain fault, level 1 */
+            case 0xb: /* Domain fault, level 2 */
+            case 0xd: /* Permision fault, level 1 */
+            case 0xf: /* Permision fault, level 2 */
+                si_signo = TARGET_SIGSEGV;
+                si_code = TARGET_SEGV_ACCERR;
+                break;
+            case 0x5: /* Translation fault, level 1 */
+            case 0x7: /* Translation fault, level 2 */
+                si_signo = TARGET_SIGSEGV;
+                si_code = TARGET_SEGV_MAPERR;
+                break;
+            default:
+                g_assert_not_reached();
+            }
+            force_sig_fault(si_signo, si_code, env->exception.vaddress);
             break;
         case EXCP_DEBUG:
         case EXCP_BKPT:
-- 
2.33.1


Re: [PATCH 08/30] bsd-user/arm/target_arch_cpu.h: Implement data faults
Posted by Richard Henderson 4 years ago
On 1/10/22 3:19 AM, Warner Losh wrote:
> Update for the richer set of data faults that are now possible. Copied
> largely from linux-user/arm/cpu_loop.c
> 
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/arm/target_arch_cpu.h | 44 ++++++++++++++++++++++++++--------
>   1 file changed, 34 insertions(+), 10 deletions(-)

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

r~

Re: [PATCH 08/30] bsd-user/arm/target_arch_cpu.h: Implement data faults
Posted by Peter Maydell 4 years ago
On Sun, 9 Jan 2022 at 16:29, Warner Losh <imp@bsdimp.com> wrote:
>
> Update for the richer set of data faults that are now possible. Copied
> largely from linux-user/arm/cpu_loop.c
>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/arm/target_arch_cpu.h | 44 ++++++++++++++++++++++++++--------
>  1 file changed, 34 insertions(+), 10 deletions(-)
>
> diff --git a/bsd-user/arm/target_arch_cpu.h b/bsd-user/arm/target_arch_cpu.h
> index 996a361e3fe..51e592bcfe7 100644
> --- a/bsd-user/arm/target_arch_cpu.h
> +++ b/bsd-user/arm/target_arch_cpu.h
> @@ -39,8 +39,7 @@ static inline void target_cpu_init(CPUARMState *env,
>
>  static inline void target_cpu_loop(CPUARMState *env)
>  {
> -    int trapnr;
> -    target_siginfo_t info;
> +    int trapnr, si_signo, si_code;
>      unsigned int n;
>      CPUState *cs = env_cpu(env);
>
> @@ -143,15 +142,40 @@ static inline void target_cpu_loop(CPUARMState *env)
>              /* just indicate that signals should be handled asap */
>              break;
>          case EXCP_PREFETCH_ABORT:
> -            /* See arm/arm/trap.c prefetch_abort_handler() */
>          case EXCP_DATA_ABORT:
> -            /* See arm/arm/trap.c data_abort_handler() */
> -            info.si_signo = TARGET_SIGSEGV;
> -            info.si_errno = 0;
> -            /* XXX: check env->error_code */
> -            info.si_code = 0;
> -            info.si_addr = env->exception.vaddress;
> -            queue_signal(env, info.si_signo, &info);
> +            /*
> +             * See arm/arm/trap-v6.c prefetch_abort_handler() and data_abort_handler()
> +             *
> +             * However, FreeBSD maps these to a generic value and then uses that
> +             * to maybe fault in pages in vm/vm_fault.c:vm_fault_trap(). I
> +             * believe that the indirection maps the same as Linux, but haven't
> +             * chased down every single possible indirection.
> +             */
> +
> +            /* For user-only we don't set TTBCR_EAE, so look at the FSR. */
> +            switch (env->exception.fsr & 0x1f) {
> +            case 0x1: /* Alignment */
> +                si_signo = TARGET_SIGBUS;
> +                si_code = TARGET_BUS_ADRALN;
> +                break;
> +            case 0x3: /* Access flag fault, level 1 */
> +            case 0x6: /* Access flag fault, level 2 */
> +            case 0x9: /* Domain fault, level 1 */
> +            case 0xb: /* Domain fault, level 2 */
> +            case 0xd: /* Permision fault, level 1 */
> +            case 0xf: /* Permision fault, level 2 */

"Permission" (I see we have this typo in linux-user).

> +                si_signo = TARGET_SIGSEGV;
> +                si_code = TARGET_SEGV_ACCERR;
> +                break;
> +            case 0x5: /* Translation fault, level 1 */
> +            case 0x7: /* Translation fault, level 2 */
> +                si_signo = TARGET_SIGSEGV;
> +                si_code = TARGET_SEGV_MAPERR;
> +                break;
> +            default:
> +                g_assert_not_reached();
> +            }

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

Re: [PATCH 08/30] bsd-user/arm/target_arch_cpu.h: Implement data faults
Posted by Warner Losh 4 years ago
On Thu, Jan 13, 2022 at 10:40 AM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Sun, 9 Jan 2022 at 16:29, Warner Losh <imp@bsdimp.com> wrote:
> >
> > Update for the richer set of data faults that are now possible. Copied
> > largely from linux-user/arm/cpu_loop.c
> >
> > Signed-off-by: Warner Losh <imp@bsdimp.com>
> > ---
> >  bsd-user/arm/target_arch_cpu.h | 44 ++++++++++++++++++++++++++--------
> >  1 file changed, 34 insertions(+), 10 deletions(-)
> >
> > diff --git a/bsd-user/arm/target_arch_cpu.h
> b/bsd-user/arm/target_arch_cpu.h
> > index 996a361e3fe..51e592bcfe7 100644
> > --- a/bsd-user/arm/target_arch_cpu.h
> > +++ b/bsd-user/arm/target_arch_cpu.h
> > @@ -39,8 +39,7 @@ static inline void target_cpu_init(CPUARMState *env,
> >
> >  static inline void target_cpu_loop(CPUARMState *env)
> >  {
> > -    int trapnr;
> > -    target_siginfo_t info;
> > +    int trapnr, si_signo, si_code;
> >      unsigned int n;
> >      CPUState *cs = env_cpu(env);
> >
> > @@ -143,15 +142,40 @@ static inline void target_cpu_loop(CPUARMState
> *env)
> >              /* just indicate that signals should be handled asap */
> >              break;
> >          case EXCP_PREFETCH_ABORT:
> > -            /* See arm/arm/trap.c prefetch_abort_handler() */
> >          case EXCP_DATA_ABORT:
> > -            /* See arm/arm/trap.c data_abort_handler() */
> > -            info.si_signo = TARGET_SIGSEGV;
> > -            info.si_errno = 0;
> > -            /* XXX: check env->error_code */
> > -            info.si_code = 0;
> > -            info.si_addr = env->exception.vaddress;
> > -            queue_signal(env, info.si_signo, &info);
> > +            /*
> > +             * See arm/arm/trap-v6.c prefetch_abort_handler() and
> data_abort_handler()
> > +             *
> > +             * However, FreeBSD maps these to a generic value and then
> uses that
> > +             * to maybe fault in pages in
> vm/vm_fault.c:vm_fault_trap(). I
> > +             * believe that the indirection maps the same as Linux, but
> haven't
> > +             * chased down every single possible indirection.
> > +             */
> > +
> > +            /* For user-only we don't set TTBCR_EAE, so look at the
> FSR. */
> > +            switch (env->exception.fsr & 0x1f) {
> > +            case 0x1: /* Alignment */
> > +                si_signo = TARGET_SIGBUS;
> > +                si_code = TARGET_BUS_ADRALN;
> > +                break;
> > +            case 0x3: /* Access flag fault, level 1 */
> > +            case 0x6: /* Access flag fault, level 2 */
> > +            case 0x9: /* Domain fault, level 1 */
> > +            case 0xb: /* Domain fault, level 2 */
> > +            case 0xd: /* Permision fault, level 1 */
> > +            case 0xf: /* Permision fault, level 2 */
>
> "Permission" (I see we have this typo in linux-user).
>

Fixed. Also, if you can, please cc me if you'd like on 'back ported' fixes
into linux-user when you post them
for review that arise from this. It helps me keep track and not miss them
in this rather high volume mailing
list.


> > +                si_signo = TARGET_SIGSEGV;
> > +                si_code = TARGET_SEGV_ACCERR;
> > +                break;
> > +            case 0x5: /* Translation fault, level 1 */
> > +            case 0x7: /* Translation fault, level 2 */
> > +                si_signo = TARGET_SIGSEGV;
> > +                si_code = TARGET_SEGV_MAPERR;
> > +                break;
> > +            default:
> > +                g_assert_not_reached();
> > +            }
>
> Otherwise
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>

Thanks!

Warner
Re: [PATCH 08/30] bsd-user/arm/target_arch_cpu.h: Implement data faults
Posted by Peter Maydell 4 years ago
On Fri, 14 Jan 2022 at 18:14, Warner Losh <imp@bsdimp.com> wrote:
>
>
>
> On Thu, Jan 13, 2022 at 10:40 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>> On Sun, 9 Jan 2022 at 16:29, Warner Losh <imp@bsdimp.com> wrote:
>> >
>> > Update for the richer set of data faults that are now possible. Copied
>> > largely from linux-user/arm/cpu_loop.c
>> >
>> > Signed-off-by: Warner Losh <imp@bsdimp.com>

>> "Permission" (I see we have this typo in linux-user).
>
>
> Fixed. Also, if you can, please cc me if you'd like on 'back ported' fixes into linux-user when you post them
> for review that arise from this. It helps me keep track and not miss them in this rather high volume mailing
> list.

Sure, I can do that. Already posted this afternoon:
https://patchew.org/QEMU/20220114153732.3767229-1-peter.maydell@linaro.org/
https://patchew.org/QEMU/20220114155032.3767771-1-peter.maydell@linaro.org/

and I forgot about the 'permision' typo or I'd have folded it into
that 'nits' series, so I'll post that in a moment...

-- PMM