[v1] bsd-user: arm (32-bit) support

[PATCH 13/24] bsd-user/arm/target_arch_thread.h: Routines to create and switch to a thread

Posted by Warner Losh 4 years, 3 months ago

Implement target_thread_init (to create a thread) and target_set_upcall
(to switch to a thread) for arm.

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Klye Evans <kevans@FreeBSD.org>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/arm/target_arch_thread.h | 71 +++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)
 create mode 100644 bsd-user/arm/target_arch_thread.h

diff --git a/bsd-user/arm/target_arch_thread.h b/bsd-user/arm/target_arch_thread.h
new file mode 100644
index 0000000000..317364bb84
--- /dev/null
+++ b/bsd-user/arm/target_arch_thread.h
@@ -0,0 +1,71 @@
+/*
+ *  arm thread support
+ *
+ *  Copyright (c) 2013 Stacey D. Son
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef _TARGET_ARCH_THREAD_H_
+#define _TARGET_ARCH_THREAD_H_
+
+/* Compare to arm/arm/vm_machdep.c cpu_set_upcall_kse() */
+static inline void target_thread_set_upcall(CPUARMState *regs, abi_ulong entry,
+    abi_ulong arg, abi_ulong stack_base, abi_ulong stack_size)
+{
+    abi_ulong sp;
+
+    /*
+     * Make sure the stack is properly aligned.
+     * arm/include/param.h (STACKLIGN() macro)
+     */
+    sp = (u_int)((stack_base + stack_size) -
+        sizeof(struct target_trapframe)) & ~0x7;
+
+    /* sp = stack base */
+    regs->regs[13] = sp;
+    /* pc = start function entry */
+    regs->regs[15] = entry & 0xfffffffe;
+    /* r0 = arg */
+    regs->regs[0] = arg;
+    regs->spsr = ARM_CPU_MODE_USR;
+    if (entry & 0x1) {
+        regs->spsr |= CPSR_T;
+    }
+}
+
+static inline void target_thread_init(struct target_pt_regs *regs,
+        struct image_info *infop)
+{
+    abi_long stack = infop->start_stack;
+    memset(regs, 0, sizeof(*regs));
+    regs->ARM_cpsr = 0x10;
+    if (infop->entry & 1) {
+        regs->ARM_cpsr |= CPSR_T;
+    }
+    regs->ARM_pc = infop->entry & 0xfffffffe;
+    regs->ARM_sp = infop->start_stack;
+    if (bsd_type == target_freebsd) {
+        regs->ARM_lr = infop->entry & 0xfffffffe;
+    }
+    /* FIXME - what to for failure of get_user()? */
+    get_user_ual(regs->ARM_r2, stack + 8); /* envp */
+    get_user_ual(regs->ARM_r1, stack + 4); /* envp */
+    /* XXX: it seems that r0 is zeroed after ! */
+    regs->ARM_r0 = 0;
+    /* For uClinux PIC binaries.  */
+    /* XXX: Linux does this only on ARM with no MMU (do we care ?) */
+    regs->ARM_r10 = infop->start_data;
+}
+
+#endif /* !_TARGET_ARCH_THREAD_H_ */
-- 
2.32.0

Re: [PATCH 13/24] bsd-user/arm/target_arch_thread.h: Routines to create and switch to a thread

Posted by Kyle Evans 4 years, 3 months ago

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
>
> Implement target_thread_init (to create a thread) and target_set_upcall
> (to switch to a thread) for arm.
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Klye Evans <kevans@FreeBSD.org>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/arm/target_arch_thread.h | 71 +++++++++++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
>  create mode 100644 bsd-user/arm/target_arch_thread.h
>
> diff --git a/bsd-user/arm/target_arch_thread.h b/bsd-user/arm/target_arch_thread.h
> new file mode 100644
> index 0000000000..317364bb84
> --- /dev/null
> +++ b/bsd-user/arm/target_arch_thread.h
> @@ -0,0 +1,71 @@
> +/*
> + *  arm thread support
> + *
> + *  Copyright (c) 2013 Stacey D. Son
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +#ifndef _TARGET_ARCH_THREAD_H_
> +#define _TARGET_ARCH_THREAD_H_
> +
> +/* Compare to arm/arm/vm_machdep.c cpu_set_upcall_kse() */
> +static inline void target_thread_set_upcall(CPUARMState *regs, abi_ulong entry,
> +    abi_ulong arg, abi_ulong stack_base, abi_ulong stack_size)
> +{
> +    abi_ulong sp;
> +
> +    /*
> +     * Make sure the stack is properly aligned.
> +     * arm/include/param.h (STACKLIGN() macro)
> +     */
> +    sp = (u_int)((stack_base + stack_size) -
> +        sizeof(struct target_trapframe)) & ~0x7;
> +
> +    /* sp = stack base */
> +    regs->regs[13] = sp;
> +    /* pc = start function entry */
> +    regs->regs[15] = entry & 0xfffffffe;
> +    /* r0 = arg */
> +    regs->regs[0] = arg;
> +    regs->spsr = ARM_CPU_MODE_USR;
> +    if (entry & 0x1) {
> +        regs->spsr |= CPSR_T;
> +    }
> +}
> +
> +static inline void target_thread_init(struct target_pt_regs *regs,
> +        struct image_info *infop)
> +{
> +    abi_long stack = infop->start_stack;
> +    memset(regs, 0, sizeof(*regs));
> +    regs->ARM_cpsr = 0x10;
> +    if (infop->entry & 1) {
> +        regs->ARM_cpsr |= CPSR_T;
> +    }
> +    regs->ARM_pc = infop->entry & 0xfffffffe;
> +    regs->ARM_sp = infop->start_stack;
> +    if (bsd_type == target_freebsd) {
> +        regs->ARM_lr = infop->entry & 0xfffffffe;
> +    }
> +    /* FIXME - what to for failure of get_user()? */
> +    get_user_ual(regs->ARM_r2, stack + 8); /* envp */
> +    get_user_ual(regs->ARM_r1, stack + 4); /* envp */
> +    /* XXX: it seems that r0 is zeroed after ! */
> +    regs->ARM_r0 = 0;
> +    /* For uClinux PIC binaries.  */
> +    /* XXX: Linux does this only on ARM with no MMU (do we care ?) */
> +    regs->ARM_r10 = infop->start_data;
> +}
> +
> +#endif /* !_TARGET_ARCH_THREAD_H_ */
> --
> 2.32.0
>

I think it's obvious enough to folks already familiar with ARM, but I
wonder if we shouldn't add in some basic commentary about the thumb
bits above. Something like:

/*
 * The low bit in an entry point indicates a thumb instruction; the entry point
 * can't actually exist at this address because it must be 16- or 32-
bit aligned.
 * The low bit gets masked off and the T bit in CSPR is twiddled to
indicate thumb.
 */

Re: [PATCH 13/24] bsd-user/arm/target_arch_thread.h: Routines to create and switch to a thread

Posted by Kyle Evans 4 years, 3 months ago

On Tue, Oct 26, 2021 at 1:01 AM Kyle Evans <kevans@freebsd.org> wrote:
>
> On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
> >
> > Implement target_thread_init (to create a thread) and target_set_upcall
> > (to switch to a thread) for arm.
> >
> > Signed-off-by: Stacey Son <sson@FreeBSD.org>
> > Signed-off-by: Klye Evans <kevans@FreeBSD.org>
> > Signed-off-by: Warner Losh <imp@bsdimp.com>
> > ---
> >  bsd-user/arm/target_arch_thread.h | 71 +++++++++++++++++++++++++++++++
> >  1 file changed, 71 insertions(+)
> >  create mode 100644 bsd-user/arm/target_arch_thread.h
> >
> > diff --git a/bsd-user/arm/target_arch_thread.h b/bsd-user/arm/target_arch_thread.h
> > new file mode 100644
> > index 0000000000..317364bb84
> > --- /dev/null
> > +++ b/bsd-user/arm/target_arch_thread.h
> > @@ -0,0 +1,71 @@
> > +/*
> > + *  arm thread support
> > + *
> > + *  Copyright (c) 2013 Stacey D. Son
> > + *
> > + *  This program is free software; you can redistribute it and/or modify
> > + *  it under the terms of the GNU General Public License as published by
> > + *  the Free Software Foundation; either version 2 of the License, or
> > + *  (at your option) any later version.
> > + *
> > + *  This program is distributed in the hope that it will be useful,
> > + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + *  GNU General Public License for more details.
> > + *
> > + *  You should have received a copy of the GNU General Public License
> > + *  along with this program; if not, see <http://www.gnu.org/licenses/>.
> > + */
> > +#ifndef _TARGET_ARCH_THREAD_H_
> > +#define _TARGET_ARCH_THREAD_H_
> > +
> > +/* Compare to arm/arm/vm_machdep.c cpu_set_upcall_kse() */
> > +static inline void target_thread_set_upcall(CPUARMState *regs, abi_ulong entry,
> > +    abi_ulong arg, abi_ulong stack_base, abi_ulong stack_size)
> > +{
> > +    abi_ulong sp;
> > +
> > +    /*
> > +     * Make sure the stack is properly aligned.
> > +     * arm/include/param.h (STACKLIGN() macro)
> > +     */
> > +    sp = (u_int)((stack_base + stack_size) -
> > +        sizeof(struct target_trapframe)) & ~0x7;
> > +
> > +    /* sp = stack base */
> > +    regs->regs[13] = sp;
> > +    /* pc = start function entry */
> > +    regs->regs[15] = entry & 0xfffffffe;
> > +    /* r0 = arg */
> > +    regs->regs[0] = arg;
> > +    regs->spsr = ARM_CPU_MODE_USR;
> > +    if (entry & 0x1) {
> > +        regs->spsr |= CPSR_T;
> > +    }
> > +}
> > +
> > +static inline void target_thread_init(struct target_pt_regs *regs,
> > +        struct image_info *infop)
> > +{
> > +    abi_long stack = infop->start_stack;
> > +    memset(regs, 0, sizeof(*regs));
> > +    regs->ARM_cpsr = 0x10;
> > +    if (infop->entry & 1) {
> > +        regs->ARM_cpsr |= CPSR_T;
> > +    }
> > +    regs->ARM_pc = infop->entry & 0xfffffffe;
> > +    regs->ARM_sp = infop->start_stack;
> > +    if (bsd_type == target_freebsd) {
> > +        regs->ARM_lr = infop->entry & 0xfffffffe;
> > +    }
> > +    /* FIXME - what to for failure of get_user()? */
> > +    get_user_ual(regs->ARM_r2, stack + 8); /* envp */
> > +    get_user_ual(regs->ARM_r1, stack + 4); /* envp */
> > +    /* XXX: it seems that r0 is zeroed after ! */
> > +    regs->ARM_r0 = 0;
> > +    /* For uClinux PIC binaries.  */
> > +    /* XXX: Linux does this only on ARM with no MMU (do we care ?) */
> > +    regs->ARM_r10 = infop->start_data;
> > +}
> > +
> > +#endif /* !_TARGET_ARCH_THREAD_H_ */
> > --
> > 2.32.0
> >
>
> I think it's obvious enough to folks already familiar with ARM, but I
> wonder if we shouldn't add in some basic commentary about the thumb
> bits above. Something like:
>
> /*
>  * The low bit in an entry point indicates a thumb instruction; the entry point
>  * can't actually exist at this address because it must be 16- or 32-
> bit aligned.
>  * The low bit gets masked off and the T bit in CSPR is twiddled to
> indicate thumb.
>  */

s/CSPR/CPSR/

Re: [PATCH 13/24] bsd-user/arm/target_arch_thread.h: Routines to create and switch to a thread

Posted by Warner Losh 4 years, 3 months ago

On Tue, Oct 26, 2021 at 12:11 AM Kyle Evans <kevans@freebsd.org> wrote:

> On Tue, Oct 26, 2021 at 1:01 AM Kyle Evans <kevans@freebsd.org> wrote:
> >
> > On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
> > >
> > > Implement target_thread_init (to create a thread) and target_set_upcall
> > > (to switch to a thread) for arm.
> > >
> > > Signed-off-by: Stacey Son <sson@FreeBSD.org>
> > > Signed-off-by: Klye Evans <kevans@FreeBSD.org>
> > > Signed-off-by: Warner Losh <imp@bsdimp.com>
> > > ---
> > >  bsd-user/arm/target_arch_thread.h | 71 +++++++++++++++++++++++++++++++
> > >  1 file changed, 71 insertions(+)
> > >  create mode 100644 bsd-user/arm/target_arch_thread.h
> > >
> > > diff --git a/bsd-user/arm/target_arch_thread.h
> b/bsd-user/arm/target_arch_thread.h
> > > new file mode 100644
> > > index 0000000000..317364bb84
> > > --- /dev/null
> > > +++ b/bsd-user/arm/target_arch_thread.h
> > > @@ -0,0 +1,71 @@
> > > +/*
> > > + *  arm thread support
> > > + *
> > > + *  Copyright (c) 2013 Stacey D. Son
> > > + *
> > > + *  This program is free software; you can redistribute it and/or
> modify
> > > + *  it under the terms of the GNU General Public License as published
> by
> > > + *  the Free Software Foundation; either version 2 of the License, or
> > > + *  (at your option) any later version.
> > > + *
> > > + *  This program is distributed in the hope that it will be useful,
> > > + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > + *  GNU General Public License for more details.
> > > + *
> > > + *  You should have received a copy of the GNU General Public License
> > > + *  along with this program; if not, see <
> http://www.gnu.org/licenses/>.
> > > + */
> > > +#ifndef _TARGET_ARCH_THREAD_H_
> > > +#define _TARGET_ARCH_THREAD_H_
> > > +
> > > +/* Compare to arm/arm/vm_machdep.c cpu_set_upcall_kse() */
> > > +static inline void target_thread_set_upcall(CPUARMState *regs,
> abi_ulong entry,
> > > +    abi_ulong arg, abi_ulong stack_base, abi_ulong stack_size)
> > > +{
> > > +    abi_ulong sp;
> > > +
> > > +    /*
> > > +     * Make sure the stack is properly aligned.
> > > +     * arm/include/param.h (STACKLIGN() macro)
> > > +     */
> > > +    sp = (u_int)((stack_base + stack_size) -
> > > +        sizeof(struct target_trapframe)) & ~0x7;
> > > +
> > > +    /* sp = stack base */
> > > +    regs->regs[13] = sp;
> > > +    /* pc = start function entry */
> > > +    regs->regs[15] = entry & 0xfffffffe;
> > > +    /* r0 = arg */
> > > +    regs->regs[0] = arg;
> > > +    regs->spsr = ARM_CPU_MODE_USR;
> > > +    if (entry & 0x1) {
> > > +        regs->spsr |= CPSR_T;
> > > +    }
> > > +}
> > > +
> > > +static inline void target_thread_init(struct target_pt_regs *regs,
> > > +        struct image_info *infop)
> > > +{
> > > +    abi_long stack = infop->start_stack;
> > > +    memset(regs, 0, sizeof(*regs));
> > > +    regs->ARM_cpsr = 0x10;
> > > +    if (infop->entry & 1) {
> > > +        regs->ARM_cpsr |= CPSR_T;
> > > +    }
> > > +    regs->ARM_pc = infop->entry & 0xfffffffe;
> > > +    regs->ARM_sp = infop->start_stack;
> > > +    if (bsd_type == target_freebsd) {
> > > +        regs->ARM_lr = infop->entry & 0xfffffffe;
> > > +    }
> > > +    /* FIXME - what to for failure of get_user()? */
> > > +    get_user_ual(regs->ARM_r2, stack + 8); /* envp */
> > > +    get_user_ual(regs->ARM_r1, stack + 4); /* envp */
> > > +    /* XXX: it seems that r0 is zeroed after ! */
> > > +    regs->ARM_r0 = 0;
> > > +    /* For uClinux PIC binaries.  */
> > > +    /* XXX: Linux does this only on ARM with no MMU (do we care ?) */
> > > +    regs->ARM_r10 = infop->start_data;
> > > +}
> > > +
> > > +#endif /* !_TARGET_ARCH_THREAD_H_ */
> > > --
> > > 2.32.0
> > >
> >
> > I think it's obvious enough to folks already familiar with ARM, but I
> > wonder if we shouldn't add in some basic commentary about the thumb
> > bits above. Something like:
> >
> > /*
> >  * The low bit in an entry point indicates a thumb instruction; the
> entry point
> >  * can't actually exist at this address because it must be 16- or 32-
> > bit aligned.
> >  * The low bit gets masked off and the T bit in CSPR is twiddled to
> > indicate thumb.
> >  */
>
> s/CSPR/CPSR/
>

Does

    /*
     * Thumb mode is encoded by the low bit in the entry point (since ARM
can't
     * execute at odd addresses). When it's set, set the Thumb bit (T) in
the
     * CPSR.
     */

 Look good to you?

Warner

Re: [PATCH 13/24] bsd-user/arm/target_arch_thread.h: Routines to create and switch to a thread

Posted by Kyle Evans 4 years, 3 months ago

On Wed, Oct 27, 2021 at 10:35 AM Warner Losh <imp@bsdimp.com> wrote:
>
>
>
> On Tue, Oct 26, 2021 at 12:11 AM Kyle Evans <kevans@freebsd.org> wrote:
>>
>> On Tue, Oct 26, 2021 at 1:01 AM Kyle Evans <kevans@freebsd.org> wrote:
>> >
>> > On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
>> > >
>> > > Implement target_thread_init (to create a thread) and target_set_upcall
>> > > (to switch to a thread) for arm.
>> > >
>> > > Signed-off-by: Stacey Son <sson@FreeBSD.org>
>> > > Signed-off-by: Klye Evans <kevans@FreeBSD.org>
>> > > Signed-off-by: Warner Losh <imp@bsdimp.com>
>> > > ---
>> > >  bsd-user/arm/target_arch_thread.h | 71 +++++++++++++++++++++++++++++++
>> > >  1 file changed, 71 insertions(+)
>> > >  create mode 100644 bsd-user/arm/target_arch_thread.h
>> > >
>> > > diff --git a/bsd-user/arm/target_arch_thread.h b/bsd-user/arm/target_arch_thread.h
>> > > new file mode 100644
>> > > index 0000000000..317364bb84
>> > > --- /dev/null
>> > > +++ b/bsd-user/arm/target_arch_thread.h
>> > > @@ -0,0 +1,71 @@
>> > > +/*
>> > > + *  arm thread support
>> > > + *
>> > > + *  Copyright (c) 2013 Stacey D. Son
>> > > + *
>> > > + *  This program is free software; you can redistribute it and/or modify
>> > > + *  it under the terms of the GNU General Public License as published by
>> > > + *  the Free Software Foundation; either version 2 of the License, or
>> > > + *  (at your option) any later version.
>> > > + *
>> > > + *  This program is distributed in the hope that it will be useful,
>> > > + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> > > + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> > > + *  GNU General Public License for more details.
>> > > + *
>> > > + *  You should have received a copy of the GNU General Public License
>> > > + *  along with this program; if not, see <http://www.gnu.org/licenses/>.
>> > > + */
>> > > +#ifndef _TARGET_ARCH_THREAD_H_
>> > > +#define _TARGET_ARCH_THREAD_H_
>> > > +
>> > > +/* Compare to arm/arm/vm_machdep.c cpu_set_upcall_kse() */
>> > > +static inline void target_thread_set_upcall(CPUARMState *regs, abi_ulong entry,
>> > > +    abi_ulong arg, abi_ulong stack_base, abi_ulong stack_size)
>> > > +{
>> > > +    abi_ulong sp;
>> > > +
>> > > +    /*
>> > > +     * Make sure the stack is properly aligned.
>> > > +     * arm/include/param.h (STACKLIGN() macro)
>> > > +     */
>> > > +    sp = (u_int)((stack_base + stack_size) -
>> > > +        sizeof(struct target_trapframe)) & ~0x7;
>> > > +
>> > > +    /* sp = stack base */
>> > > +    regs->regs[13] = sp;
>> > > +    /* pc = start function entry */
>> > > +    regs->regs[15] = entry & 0xfffffffe;
>> > > +    /* r0 = arg */
>> > > +    regs->regs[0] = arg;
>> > > +    regs->spsr = ARM_CPU_MODE_USR;
>> > > +    if (entry & 0x1) {
>> > > +        regs->spsr |= CPSR_T;
>> > > +    }
>> > > +}
>> > > +
>> > > +static inline void target_thread_init(struct target_pt_regs *regs,
>> > > +        struct image_info *infop)
>> > > +{
>> > > +    abi_long stack = infop->start_stack;
>> > > +    memset(regs, 0, sizeof(*regs));
>> > > +    regs->ARM_cpsr = 0x10;
>> > > +    if (infop->entry & 1) {
>> > > +        regs->ARM_cpsr |= CPSR_T;
>> > > +    }
>> > > +    regs->ARM_pc = infop->entry & 0xfffffffe;
>> > > +    regs->ARM_sp = infop->start_stack;
>> > > +    if (bsd_type == target_freebsd) {
>> > > +        regs->ARM_lr = infop->entry & 0xfffffffe;
>> > > +    }
>> > > +    /* FIXME - what to for failure of get_user()? */
>> > > +    get_user_ual(regs->ARM_r2, stack + 8); /* envp */
>> > > +    get_user_ual(regs->ARM_r1, stack + 4); /* envp */
>> > > +    /* XXX: it seems that r0 is zeroed after ! */
>> > > +    regs->ARM_r0 = 0;
>> > > +    /* For uClinux PIC binaries.  */
>> > > +    /* XXX: Linux does this only on ARM with no MMU (do we care ?) */
>> > > +    regs->ARM_r10 = infop->start_data;
>> > > +}
>> > > +
>> > > +#endif /* !_TARGET_ARCH_THREAD_H_ */
>> > > --
>> > > 2.32.0
>> > >
>> >
>> > I think it's obvious enough to folks already familiar with ARM, but I
>> > wonder if we shouldn't add in some basic commentary about the thumb
>> > bits above. Something like:
>> >
>> > /*
>> >  * The low bit in an entry point indicates a thumb instruction; the entry point
>> >  * can't actually exist at this address because it must be 16- or 32-
>> > bit aligned.
>> >  * The low bit gets masked off and the T bit in CSPR is twiddled to
>> > indicate thumb.
>> >  */
>>
>> s/CSPR/CPSR/
>
>
> Does
>
>     /*
>      * Thumb mode is encoded by the low bit in the entry point (since ARM can't
>      * execute at odd addresses). When it's set, set the Thumb bit (T) in the
>      * CPSR.
>      */
>
>  Look good to you?
>

Yeah, that works for me!

Thanks,

Kyle Evans

Re: [PATCH 13/24] bsd-user/arm/target_arch_thread.h: Routines to create and switch to a thread

Posted by Richard Henderson 4 years, 3 months ago

On 10/19/21 9:44 AM, Warner Losh wrote:
> +    regs->spsr = ARM_CPU_MODE_USR;
> +    if (entry & 0x1) {
> +        regs->spsr |= CPSR_T;
> +    }

(1) Why are you setting SPSR not CPSR?
(2) Changes to CPSR cannot be made manually; you need cpsr_write.


r~

Re: [PATCH 13/24] bsd-user/arm/target_arch_thread.h: Routines to create and switch to a thread

Posted by Warner Losh 4 years, 3 months ago

On Thu, Oct 28, 2021 at 9:57 AM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 10/19/21 9:44 AM, Warner Losh wrote:
> > +    regs->spsr = ARM_CPU_MODE_USR;
> > +    if (entry & 0x1) {
> > +        regs->spsr |= CPSR_T;
> > +    }
>
> (1) Why are you setting SPSR not CPSR?
>

I think dyslexia is the best answer... I think I added that based on a
recent
FreeBSD kernel change and I'm not sure what happened....


> (2) Changes to CPSR cannot be made manually; you need cpsr_write.
>

So would

+        cpsr_write(env, cpsr_read(env) | CPSR_T, CPSR_T, CPSRWriteByInstr);

cover what's needed here?

Warner

r~
>

Re: [PATCH 13/24] bsd-user/arm/target_arch_thread.h: Routines to create and switch to a thread

Posted by Richard Henderson 4 years, 3 months ago

On 10/28/21 12:45 PM, Warner Losh wrote:
>     (2) Changes to CPSR cannot be made manually; you need cpsr_write.
> 
> 
> So would
> 
> +        cpsr_write(env, cpsr_read(env) | CPSR_T, CPSR_T, CPSRWriteByInstr);
> 
> cover what's needed here?

You can use

     cpsr_write(env, CPSR_T, CPSR_T, CPSRWriteByInstr);

because the mask takes care of not changing other bits.


r~