diff mbox series

[v2,04/40] bsd-user/arm/signal.c: get_mcontext should zero vfp data

Message ID 20220125012947.14974-5-imp@bsdimp.com
State New
Headers show
Series bsd-user: upstream our signal implementation | expand

Commit Message

Warner Losh Jan. 25, 2022, 1:29 a.m. UTC 
FreeBSD's get_mcontext doesn't return any vfp data. Instead, it zeros
out the vfp feilds (and all the spare fields). Impelement this
behavior. We're still missing the sysarch(ARM_GET_VFPCONTEXT) syscall,
though.

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/arm/signal.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Richard Henderson Jan. 27, 2022, 5:59 a.m. UTC | #1 
On 1/25/22 12:29, Warner Losh wrote:
> FreeBSD's get_mcontext doesn't return any vfp data. Instead, it zeros
> out the vfp feilds (and all the spare fields). Impelement this
> behavior. We're still missing the sysarch(ARM_GET_VFPCONTEXT) syscall,
> though.
> 
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>   bsd-user/arm/signal.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/bsd-user/arm/signal.c b/bsd-user/arm/signal.c
> index 9026343b478..6eadc6e3c56 100644
> --- a/bsd-user/arm/signal.c
> +++ b/bsd-user/arm/signal.c
> @@ -109,6 +109,14 @@ abi_long get_mcontext(CPUARMState *env, target_mcontext_t *mcp, int flags)
>       gr[TARGET_REG_LR] = tswap32(env->regs[14]);
>       gr[TARGET_REG_PC] = tswap32(env->regs[15]);
>   
> +    /*
> +     * FreeBSD's set_mcontext doesn't save VFP info, but blanks it out instead.
> +     * Instead, sysarch(ARM_GET_VFPSTATE) is used instead.
> +     */

Could be rewritten with fewer "instead".  You wanted get_mcontext.
Otherwise,

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


r~
Warner Losh Jan. 27, 2022, 3:44 p.m. UTC | #2 
On Wed, Jan 26, 2022 at 10:59 PM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 1/25/22 12:29, Warner Losh wrote:
> > FreeBSD's get_mcontext doesn't return any vfp data. Instead, it zeros
> > out the vfp feilds (and all the spare fields). Impelement this
> > behavior. We're still missing the sysarch(ARM_GET_VFPCONTEXT) syscall,
> > though.
> >
> > Signed-off-by: Warner Losh <imp@bsdimp.com>
> > ---
> >   bsd-user/arm/signal.c | 8 ++++++++
> >   1 file changed, 8 insertions(+)
> >
> > diff --git a/bsd-user/arm/signal.c b/bsd-user/arm/signal.c
> > index 9026343b478..6eadc6e3c56 100644
> > --- a/bsd-user/arm/signal.c
> > +++ b/bsd-user/arm/signal.c
> > @@ -109,6 +109,14 @@ abi_long get_mcontext(CPUARMState *env,
> target_mcontext_t *mcp, int flags)
> >       gr[TARGET_REG_LR] = tswap32(env->regs[14]);
> >       gr[TARGET_REG_PC] = tswap32(env->regs[15]);
> >
> > +    /*
> > +     * FreeBSD's set_mcontext doesn't save VFP info, but blanks it out
> instead.
> > +     * Instead, sysarch(ARM_GET_VFPSTATE) is used instead.
> > +     */
>
> Could be rewritten with fewer "instead".  You wanted get_mcontext.
>

Yes. Instead is indeed over used over and over redundantly :)

Thanks!

Warner


> Otherwise,
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
>
> r~
>
>
diff mbox series

Patch

diff --git a/bsd-user/arm/signal.c b/bsd-user/arm/signal.c
index 9026343b478..6eadc6e3c56 100644
--- a/bsd-user/arm/signal.c
+++ b/bsd-user/arm/signal.c
@@ -109,6 +109,14 @@  abi_long get_mcontext(CPUARMState *env, target_mcontext_t *mcp, int flags)
     gr[TARGET_REG_LR] = tswap32(env->regs[14]);
     gr[TARGET_REG_PC] = tswap32(env->regs[15]);
 
+    /*
+     * FreeBSD's set_mcontext doesn't save VFP info, but blanks it out instead.
+     * Instead, sysarch(ARM_GET_VFPSTATE) is used instead.
+     */
+    mcp->mc_vfp_size = 0;
+    mcp->mc_vfp_ptr = 0;
+    memset(&mcp->mc_spare, 0, sizeof(mcp->mc_spare));
+
     return 0;
 }