[PATCH 00/24] bsd-user: arm (32-bit) support

[PATCH 00/24] bsd-user: arm (32-bit) support

[Warner Losh]

This series of patches brings in 32-bit arm support for bsd-user.  It implements
all the bits needed to do image activation, signal handling, stack management
and threading. This allows us to get to the "Hello World" level and also to
satisfy all linking requirements. I've sliced up the current bsd-user fork
versions of all this code into what I hope are right-sized reviewable chunks.

make checkpatch has a couple of complaints about the comments for the signal
trampoline:
WARNING: Block comments use a leading /* on a separate line
+    /* 8 */ sys_sigreturn,

WARNING: Block comments use a leading /* on a separate line
+    /* 9 */ sys_exit

that I think should be ignored.

Warner Losh (24):
  bsd-user/arm/target_arch_sysarch.h: Use consistent include guards
  bsd-user/arm/target_syscall.h: Add copyright and update name
  bsd-user/arm/target_arch_cpu.c: Target specific TLS routines
  bsd-user/arm/target_arch_cpu.h: CPU Loop definitions
  bsd-user/arm/target_arch_cpu.h: Implement target_cpu_clone_regs
  bsd-user/arm/target_arch_cpu.h: Dummy target_cpu_loop implementation
  bsd-user/arm/target_arch_cpu.h: Implment trivial EXCP exceptions
  bsd-user/arm/target_arch_cpu.h: Implement data abort exceptions
  bsd-user/arm/target_arch_cpu.h: Implement system call dispatch
  bsd-user/arm/target_arch_reg.h: Implement core dump register copying
  bsd-user/arm/target_arch_vmparam.h: Parameters for arm address space
  bsd-user/arm/target_arch_sigtramp.h: Signal Trampoline for arm
  bsd-user/arm/target_arch_thread.h: Routines to create and switch to a
    thread
  bsd-user/arm/target_arch_elf.h: arm defines for ELF
  bsd-user/arm/target_arch_elf.h: arm get hwcap
  bsd-user/arm/target_arch_elf.h: arm get_hwcap2 impl
  bsd-user/arm/target_arch_signal.h: arm specific signal registers and
    stack
  bsd-user/arm/target_arch_signal.h: arm machine context for signals
  bsd-user/arm/target_arch_signal.h: arm user context and trapframe for
    signals
  bsd-user/arm/target_arch_signal.h: arm set_sigtramp_args
  bsd-user/arm/target_arch_signal.h: arm get_mcontext
  bsd-user/arm/target_arch_signal.h: arm set_mcontext
  bsd-user/arm/target_arch_signal.h: arm get_ucontext_sigreturn
  bsd-user: add arm target build

 bsd-user/arm/target_arch.h          |  28 +++
 bsd-user/arm/target_arch_cpu.c      |  39 +++++
 bsd-user/arm/target_arch_cpu.h      | 206 ++++++++++++++++++++++
 bsd-user/arm/target_arch_elf.h      | 128 ++++++++++++++
 bsd-user/arm/target_arch_reg.h      |  60 +++++++
 bsd-user/arm/target_arch_signal.h   | 253 ++++++++++++++++++++++++++++
 bsd-user/arm/target_arch_sigtramp.h |  52 ++++++
 bsd-user/arm/target_arch_sysarch.h  |   6 +-
 bsd-user/arm/target_arch_thread.h   |  71 ++++++++
 bsd-user/arm/target_arch_vmparam.h  |  48 ++++++
 bsd-user/arm/target_syscall.h       |  27 ++-
 configs/targets/arm-bsd-user.mak    |   2 +
 12 files changed, 913 insertions(+), 7 deletions(-)
 create mode 100644 bsd-user/arm/target_arch.h
 create mode 100644 bsd-user/arm/target_arch_cpu.c
 create mode 100644 bsd-user/arm/target_arch_cpu.h
 create mode 100644 bsd-user/arm/target_arch_elf.h
 create mode 100644 bsd-user/arm/target_arch_reg.h
 create mode 100644 bsd-user/arm/target_arch_signal.h
 create mode 100644 bsd-user/arm/target_arch_sigtramp.h
 create mode 100644 bsd-user/arm/target_arch_thread.h
 create mode 100644 bsd-user/arm/target_arch_vmparam.h
 create mode 100644 configs/targets/arm-bsd-user.mak

-- 
2.32.0

[PATCH 01/24] bsd-user/arm/target_arch_sysarch.h: Use consistent include guards

[Warner Losh]

As part of upstreaming, the include guards have been made more
consistent. Update this file to use the new guards.

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

diff --git a/bsd-user/arm/target_arch_sysarch.h b/bsd-user/arm/target_arch_sysarch.h
index 632a5cd453..8cc6bff207 100644
--- a/bsd-user/arm/target_arch_sysarch.h
+++ b/bsd-user/arm/target_arch_sysarch.h
@@ -17,8 +17,8 @@
  *  along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
-#ifndef BSD_USER_ARCH_SYSARCH_H_
-#define BSD_USER_ARCH_SYSARCH_H_
+#ifndef _TARGET_ARCH_SYSARCH_H_
+#define _TARGET_ARCH_SYSARCH_H_
 
 #include "target_syscall.h"
 #include "target_arch.h"
@@ -75,4 +75,4 @@ static inline void do_freebsd_arch_print_sysarch(
     }
 }
 
-#endif /*!BSD_USER_ARCH_SYSARCH_H_ */
+#endif /*!_TARGET_ARCH_SYSARCH_H_ */
-- 
2.32.0

Re: [PATCH 01/24] bsd-user/arm/target_arch_sysarch.h: Use consistent include guards

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
>
> As part of upstreaming, the include guards have been made more
> consistent. Update this file to use the new guards.
>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/arm/target_arch_sysarch.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/bsd-user/arm/target_arch_sysarch.h b/bsd-user/arm/target_arch_sysarch.h
> index 632a5cd453..8cc6bff207 100644
> --- a/bsd-user/arm/target_arch_sysarch.h
> +++ b/bsd-user/arm/target_arch_sysarch.h
> @@ -17,8 +17,8 @@
>   *  along with this program; if not, see <http://www.gnu.org/licenses/>.
>   */
>
> -#ifndef BSD_USER_ARCH_SYSARCH_H_
> -#define BSD_USER_ARCH_SYSARCH_H_
> +#ifndef _TARGET_ARCH_SYSARCH_H_
> +#define _TARGET_ARCH_SYSARCH_H_
>
>  #include "target_syscall.h"
>  #include "target_arch.h"
> @@ -75,4 +75,4 @@ static inline void do_freebsd_arch_print_sysarch(
>      }
>  }
>
> -#endif /*!BSD_USER_ARCH_SYSARCH_H_ */
> +#endif /*!_TARGET_ARCH_SYSARCH_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

Re: [PATCH 01/24] bsd-user/arm/target_arch_sysarch.h: Use consistent include guards

[Richard Henderson]

On 10/19/21 9:44 AM, Warner Losh wrote:
> As part of upstreaming, the include guards have been made more
> consistent. Update this file to use the new guards.
> 
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/arm/target_arch_sysarch.h | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)

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

r~

[PATCH 02/24] bsd-user/arm/target_syscall.h: Add copyright and update name

[Warner Losh]

The preferred name for the 32-bit arm is now armv7. Update the name to
reflect that. In addition, add Stacey's copyright to this file and
update the include guards to the new convention.

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/arm/target_syscall.h | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/bsd-user/arm/target_syscall.h b/bsd-user/arm/target_syscall.h
index ef4b37f017..a5f2bb4e01 100644
--- a/bsd-user/arm/target_syscall.h
+++ b/bsd-user/arm/target_syscall.h
@@ -1,5 +1,24 @@
-#ifndef BSD_USER_ARCH_SYSCALL_H_
-#define BSD_USER_ARCH_SYSCALL_H_
+/*
+ *  arm cpu system call stubs
+ *
+ *  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_SYSCALL_H_
+#define _TARGET_ARCH_SYSCALL_H_
 
 struct target_pt_regs {
     abi_long uregs[17];
@@ -31,6 +50,6 @@ struct target_pt_regs {
 #define TARGET_FREEBSD_ARM_GET_TP       3
 
 #define TARGET_HW_MACHINE       "arm"
-#define TARGET_HW_MACHINE_ARCH  "armv6"
+#define TARGET_HW_MACHINE_ARCH  "armv7"
 
-#endif /* !BSD_USER_ARCH_SYSCALL_H_ */
+#endif /* !_TARGET_ARCH_SYSCALL_H_ */
-- 
2.32.0

Re: [PATCH 02/24] bsd-user/arm/target_syscall.h: Add copyright and update name

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
>
> The preferred name for the 32-bit arm is now armv7. Update the name to
> reflect that. In addition, add Stacey's copyright to this file and
> update the include guards to the new convention.
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/arm/target_syscall.h | 27 +++++++++++++++++++++++----
>  1 file changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/bsd-user/arm/target_syscall.h b/bsd-user/arm/target_syscall.h
> index ef4b37f017..a5f2bb4e01 100644
> --- a/bsd-user/arm/target_syscall.h
> +++ b/bsd-user/arm/target_syscall.h
> @@ -1,5 +1,24 @@
> -#ifndef BSD_USER_ARCH_SYSCALL_H_
> -#define BSD_USER_ARCH_SYSCALL_H_
> +/*
> + *  arm cpu system call stubs
> + *
> + *  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_SYSCALL_H_
> +#define _TARGET_ARCH_SYSCALL_H_
>
>  struct target_pt_regs {
>      abi_long uregs[17];
> @@ -31,6 +50,6 @@ struct target_pt_regs {
>  #define TARGET_FREEBSD_ARM_GET_TP       3
>
>  #define TARGET_HW_MACHINE       "arm"
> -#define TARGET_HW_MACHINE_ARCH  "armv6"
> +#define TARGET_HW_MACHINE_ARCH  "armv7"
>
> -#endif /* !BSD_USER_ARCH_SYSCALL_H_ */
> +#endif /* !_TARGET_ARCH_SYSCALL_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

Re: [PATCH 02/24] bsd-user/arm/target_syscall.h: Add copyright and update name

[Richard Henderson]

On 10/19/21 9:44 AM, Warner Losh wrote:
> The preferred name for the 32-bit arm is now armv7. Update the name to
> reflect that. In addition, add Stacey's copyright to this file and
> update the include guards to the new convention.
> 
> Signed-off-by: Stacey Son<sson@FreeBSD.org>
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/arm/target_syscall.h | 27 +++++++++++++++++++++++----
>   1 file changed, 23 insertions(+), 4 deletions(-)

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

r~

[PATCH 03/24] bsd-user/arm/target_arch_cpu.c: Target specific TLS routines

[Warner Losh]

Target specific TLS routines to get and set the TLS values.

Signed-off-by: Klye Evans <kevans@FreeBSD.org>
Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/arm/target_arch.h     | 28 ++++++++++++++++++++++++
 bsd-user/arm/target_arch_cpu.c | 39 ++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+)
 create mode 100644 bsd-user/arm/target_arch.h
 create mode 100644 bsd-user/arm/target_arch_cpu.c

diff --git a/bsd-user/arm/target_arch.h b/bsd-user/arm/target_arch.h
new file mode 100644
index 0000000000..93cfaea098
--- /dev/null
+++ b/bsd-user/arm/target_arch.h
@@ -0,0 +1,28 @@
+/*
+ * ARM 32-bit specific prototypes for bsd-user
+ *
+ *  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_H_
+#define _TARGET_ARCH_H_
+
+#include "qemu.h"
+
+void target_cpu_set_tls(CPUARMState *env, target_ulong newtls);
+target_ulong target_cpu_get_tls(CPUARMState *env);
+
+#endif /* !_TARGET_ARCH_H_ */
diff --git a/bsd-user/arm/target_arch_cpu.c b/bsd-user/arm/target_arch_cpu.c
new file mode 100644
index 0000000000..02bf9149d5
--- /dev/null
+++ b/bsd-user/arm/target_arch_cpu.c
@@ -0,0 +1,39 @@
+/*
+ *  arm cpu related code
+ *
+ *  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/>.
+ */
+#include "target_arch.h"
+
+void target_cpu_set_tls(CPUARMState *env, target_ulong newtls)
+{
+    if (access_secure_reg(env)) {
+        env->cp15.tpidrurw_s = newtls;
+        env->cp15.tpidruro_s = newtls;
+        return;
+    }
+
+    env->cp15.tpidr_el[0] = newtls;
+    env->cp15.tpidrro_el[0] = newtls;
+}
+
+target_ulong target_cpu_get_tls(CPUARMState *env)
+{
+    if (access_secure_reg(env)) {
+        return env->cp15.tpidruro_s;
+    }
+    return env->cp15.tpidrro_el[0];
+}
-- 
2.32.0

Re: [PATCH 03/24] bsd-user/arm/target_arch_cpu.c: Target specific TLS routines

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
>
> Target specific TLS routines to get and set the TLS values.
>
> Signed-off-by: Klye Evans <kevans@FreeBSD.org>

s/Klye/Kyle/ :-)

> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/arm/target_arch.h     | 28 ++++++++++++++++++++++++
>  bsd-user/arm/target_arch_cpu.c | 39 ++++++++++++++++++++++++++++++++++
>  2 files changed, 67 insertions(+)
>  create mode 100644 bsd-user/arm/target_arch.h
>  create mode 100644 bsd-user/arm/target_arch_cpu.c
>
> diff --git a/bsd-user/arm/target_arch.h b/bsd-user/arm/target_arch.h
> new file mode 100644
> index 0000000000..93cfaea098
> --- /dev/null
> +++ b/bsd-user/arm/target_arch.h
> @@ -0,0 +1,28 @@
> +/*
> + * ARM 32-bit specific prototypes for bsd-user
> + *
> + *  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_H_
> +#define _TARGET_ARCH_H_
> +
> +#include "qemu.h"
> +
> +void target_cpu_set_tls(CPUARMState *env, target_ulong newtls);
> +target_ulong target_cpu_get_tls(CPUARMState *env);
> +
> +#endif /* !_TARGET_ARCH_H_ */
> diff --git a/bsd-user/arm/target_arch_cpu.c b/bsd-user/arm/target_arch_cpu.c
> new file mode 100644
> index 0000000000..02bf9149d5
> --- /dev/null
> +++ b/bsd-user/arm/target_arch_cpu.c
> @@ -0,0 +1,39 @@
> +/*
> + *  arm cpu related code
> + *
> + *  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/>.
> + */
> +#include "target_arch.h"
> +
> +void target_cpu_set_tls(CPUARMState *env, target_ulong newtls)
> +{
> +    if (access_secure_reg(env)) {
> +        env->cp15.tpidrurw_s = newtls;
> +        env->cp15.tpidruro_s = newtls;
> +        return;
> +    }
> +
> +    env->cp15.tpidr_el[0] = newtls;
> +    env->cp15.tpidrro_el[0] = newtls;
> +}
> +
> +target_ulong target_cpu_get_tls(CPUARMState *env)
> +{
> +    if (access_secure_reg(env)) {
> +        return env->cp15.tpidruro_s;
> +    }
> +    return env->cp15.tpidrro_el[0];
> +}
> --
> 2.32.0
>

Modulo typo:

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

Re: [PATCH 03/24] bsd-user/arm/target_arch_cpu.c: Target specific TLS routines

[Richard Henderson]

On 10/19/21 9:44 AM, Warner Losh wrote:
> Target specific TLS routines to get and set the TLS values.
> 
> Signed-off-by: Klye Evans<kevans@FreeBSD.org>
> Signed-off-by: Stacey Son<sson@FreeBSD.org>
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/arm/target_arch.h     | 28 ++++++++++++++++++++++++
>   bsd-user/arm/target_arch_cpu.c | 39 ++++++++++++++++++++++++++++++++++
>   2 files changed, 67 insertions(+)
>   create mode 100644 bsd-user/arm/target_arch.h
>   create mode 100644 bsd-user/arm/target_arch_cpu.c

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

r~

[PATCH 04/24] bsd-user/arm/target_arch_cpu.h: CPU Loop definitions

[Warner Losh]

target_arch_cpu.h is for CPU loop definitions. Create the file and
define target_cpu_init and target_cpu_reset for arm.

Signed-off-by: Olivier Houchard <cognet@ci0.org>
Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/arm/target_arch_cpu.h | 42 ++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 bsd-user/arm/target_arch_cpu.h

diff --git a/bsd-user/arm/target_arch_cpu.h b/bsd-user/arm/target_arch_cpu.h
new file mode 100644
index 0000000000..0f3538196d
--- /dev/null
+++ b/bsd-user/arm/target_arch_cpu.h
@@ -0,0 +1,42 @@
+/*
+ *  arm cpu init and loop
+ *
+ *  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_CPU_H_
+#define _TARGET_ARCH_CPU_H_
+
+#include "target_arch.h"
+
+#define TARGET_DEFAULT_CPU_MODEL "any"
+
+static inline void target_cpu_init(CPUARMState *env,
+        struct target_pt_regs *regs)
+{
+    int i;
+
+    cpsr_write(env, regs->uregs[16], 0xffffffff, CPSRWriteRaw);
+    for (i = 0; i < 16; i++) {
+        env->regs[i] = regs->uregs[i];
+    }
+}
+
+static inline void target_cpu_reset(CPUArchState *cpu)
+{
+}
+
+#endif /* !_TARGET_ARCH_CPU_H */
-- 
2.32.0

Re: [PATCH 04/24] bsd-user/arm/target_arch_cpu.h: CPU Loop definitions

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
>
> target_arch_cpu.h is for CPU loop definitions. Create the file and
> define target_cpu_init and target_cpu_reset for arm.
>
> Signed-off-by: Olivier Houchard <cognet@ci0.org>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/arm/target_arch_cpu.h | 42 ++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>  create mode 100644 bsd-user/arm/target_arch_cpu.h
>
> diff --git a/bsd-user/arm/target_arch_cpu.h b/bsd-user/arm/target_arch_cpu.h
> new file mode 100644
> index 0000000000..0f3538196d
> --- /dev/null
> +++ b/bsd-user/arm/target_arch_cpu.h
> @@ -0,0 +1,42 @@
> +/*
> + *  arm cpu init and loop
> + *
> + *  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_CPU_H_
> +#define _TARGET_ARCH_CPU_H_
> +
> +#include "target_arch.h"
> +
> +#define TARGET_DEFAULT_CPU_MODEL "any"
> +
> +static inline void target_cpu_init(CPUARMState *env,
> +        struct target_pt_regs *regs)
> +{
> +    int i;
> +
> +    cpsr_write(env, regs->uregs[16], 0xffffffff, CPSRWriteRaw);
> +    for (i = 0; i < 16; i++) {
> +        env->regs[i] = regs->uregs[i];
> +    }
> +}
> +
> +static inline void target_cpu_reset(CPUArchState *cpu)
> +{
> +}
> +
> +#endif /* !_TARGET_ARCH_CPU_H */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

Re: [PATCH 04/24] bsd-user/arm/target_arch_cpu.h: CPU Loop definitions

[Richard Henderson]

On 10/19/21 9:44 AM, Warner Losh wrote:
> +    cpsr_write(env, regs->uregs[16], 0xffffffff, CPSRWriteRaw);

This looks a bit suspicious.
Over in linux-user we use

     cpsr_write(env, regs->uregs[16], CPSR_USER | CPSR_EXEC,
                CPSRWriteByInstr);

Are you setting something special in pt_regs that would warrant writing supervisor bits of 
CPSR?  In addition, CPSRWriteRaw won't rebuild hflags, which means that changes to Thumb 
state won't be recognized properly.


r~

Re: [PATCH 04/24] bsd-user/arm/target_arch_cpu.h: CPU Loop definitions

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 1351 bytes --]

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

> On 10/19/21 9:44 AM, Warner Losh wrote:
> > +    cpsr_write(env, regs->uregs[16], 0xffffffff, CPSRWriteRaw);
>
> This looks a bit suspicious.
> Over in linux-user we use
>
>      cpsr_write(env, regs->uregs[16], CPSR_USER | CPSR_EXEC,
>                 CPSRWriteByInstr);
>
> Are you setting something special in pt_regs that would warrant writing
> supervisor bits of
> CPSR?  In addition, CPSRWriteRaw won't rebuild hflags, which means that
> changes to Thumb
> state won't be recognized properly.
>

Now that you highlight it, the code I posted looks wrong. The above code
makes better
sense to me. I'll make the change in our bsd-user fork and run it through
the FreeBSD
test harness that we have. I'll see if it introduces any regressions. Today
I have an
environment that I maintain by hand that runs ~5900 tests, of which
bsd-user's
qemu-arm passes like ~5500. My long term goal is to get that integrated
first into the
bsd-user fork's CI and later into the optional FreeBSD CI in qemu project.
In addition,
I'll be tagging 'testing qemu-user' in the testing environment so we can
exclude things
not emulated and/or mark known problems with a bug pointer. The test takes
about
3 hours to run in emulation is the only thing I'm worried about...

Warner

[-- Attachment #2: Type: text/html, Size: 1904 bytes --]

[PATCH 05/24] bsd-user/arm/target_arch_cpu.h: Implement target_cpu_clone_regs

[Warner Losh]

Implement target_cpu_clone_regs to clone the resister state on a fork.

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/arm/target_arch_cpu.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/bsd-user/arm/target_arch_cpu.h b/bsd-user/arm/target_arch_cpu.h
index 0f3538196d..c71ec000b3 100644
--- a/bsd-user/arm/target_arch_cpu.h
+++ b/bsd-user/arm/target_arch_cpu.h
@@ -35,6 +35,14 @@ static inline void target_cpu_init(CPUARMState *env,
     }
 }
 
+static inline void target_cpu_clone_regs(CPUARMState *env, target_ulong newsp)
+{
+    if (newsp) {
+        env->regs[13] = newsp;
+    }
+    env->regs[0] = 0;
+}
+
 static inline void target_cpu_reset(CPUArchState *cpu)
 {
 }
-- 
2.32.0

Re: [PATCH 05/24] bsd-user/arm/target_arch_cpu.h: Implement target_cpu_clone_regs

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
>
> Implement target_cpu_clone_regs to clone the resister state on a fork.
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/arm/target_arch_cpu.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/bsd-user/arm/target_arch_cpu.h b/bsd-user/arm/target_arch_cpu.h
> index 0f3538196d..c71ec000b3 100644
> --- a/bsd-user/arm/target_arch_cpu.h
> +++ b/bsd-user/arm/target_arch_cpu.h
> @@ -35,6 +35,14 @@ static inline void target_cpu_init(CPUARMState *env,
>      }
>  }
>
> +static inline void target_cpu_clone_regs(CPUARMState *env, target_ulong newsp)
> +{
> +    if (newsp) {
> +        env->regs[13] = newsp;
> +    }
> +    env->regs[0] = 0;
> +}
> +
>  static inline void target_cpu_reset(CPUArchState *cpu)
>  {
>  }
> --
> 2.32.0
>

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

Re: [PATCH 05/24] bsd-user/arm/target_arch_cpu.h: Implement target_cpu_clone_regs

[Richard Henderson]

On 10/19/21 9:44 AM, Warner Losh wrote:
> Implement target_cpu_clone_regs to clone the resister state on a fork.
> 
> Signed-off-by: Stacey Son<sson@FreeBSD.org>
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/arm/target_arch_cpu.h | 8 ++++++++
>   1 file changed, 8 insertions(+)

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

r~

[PATCH 06/24] bsd-user/arm/target_arch_cpu.h: Dummy target_cpu_loop implementation

[Warner Losh]

Add a boiler plate CPU loop that does nothing except return an error for
all traps.

Signed-off-by: Sean Bruno <sbruno@FreeBSD.org>
Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/arm/target_arch_cpu.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/bsd-user/arm/target_arch_cpu.h b/bsd-user/arm/target_arch_cpu.h
index c71ec000b3..94c9109c3f 100644
--- a/bsd-user/arm/target_arch_cpu.h
+++ b/bsd-user/arm/target_arch_cpu.h
@@ -35,6 +35,28 @@ static inline void target_cpu_init(CPUARMState *env,
     }
 }
 
+static inline void target_cpu_loop(CPUARMState *env)
+{
+    int trapnr;
+    target_siginfo_t info;
+    CPUState *cs = env_cpu(env);
+
+    for (;;) {
+        cpu_exec_start(cs);
+        trapnr = cpu_exec(cs);
+        cpu_exec_end(cs);
+        process_queued_cpu_work(cs);
+        switch (trapnr) {
+        default:
+            fprintf(stderr, "qemu: unhandled CPU exception 0x%x - aborting\n",
+                    trapnr);
+            cpu_dump_state(cs, stderr, 0);
+            abort();
+        } /* switch() */
+        process_pending_signals(env);
+    } /* for (;;) */
+}
+
 static inline void target_cpu_clone_regs(CPUARMState *env, target_ulong newsp)
 {
     if (newsp) {
-- 
2.32.0

Re: [PATCH 06/24] bsd-user/arm/target_arch_cpu.h: Dummy target_cpu_loop implementation

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
>
> Add a boiler plate CPU loop that does nothing except return an error for
> all traps.
>
> Signed-off-by: Sean Bruno <sbruno@FreeBSD.org>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/arm/target_arch_cpu.h | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/bsd-user/arm/target_arch_cpu.h b/bsd-user/arm/target_arch_cpu.h
> index c71ec000b3..94c9109c3f 100644
> --- a/bsd-user/arm/target_arch_cpu.h
> +++ b/bsd-user/arm/target_arch_cpu.h
> @@ -35,6 +35,28 @@ static inline void target_cpu_init(CPUARMState *env,
>      }
>  }
>
> +static inline void target_cpu_loop(CPUARMState *env)
> +{
> +    int trapnr;
> +    target_siginfo_t info;
> +    CPUState *cs = env_cpu(env);
> +
> +    for (;;) {
> +        cpu_exec_start(cs);
> +        trapnr = cpu_exec(cs);
> +        cpu_exec_end(cs);
> +        process_queued_cpu_work(cs);
> +        switch (trapnr) {
> +        default:
> +            fprintf(stderr, "qemu: unhandled CPU exception 0x%x - aborting\n",
> +                    trapnr);
> +            cpu_dump_state(cs, stderr, 0);
> +            abort();
> +        } /* switch() */
> +        process_pending_signals(env);
> +    } /* for (;;) */
> +}
> +
>  static inline void target_cpu_clone_regs(CPUARMState *env, target_ulong newsp)
>  {
>      if (newsp) {
> --
> 2.32.0
>

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

Re: [PATCH 06/24] bsd-user/arm/target_arch_cpu.h: Dummy target_cpu_loop implementation

[Richard Henderson]

On 10/19/21 9:44 AM, Warner Losh wrote:
> Add a boiler plate CPU loop that does nothing except return an error for
> all traps.
> 
> Signed-off-by: Sean Bruno<sbruno@FreeBSD.org>
> Signed-off-by: Stacey Son<sson@FreeBSD.org>
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/arm/target_arch_cpu.h | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)

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

r~

[PATCH 07/24] bsd-user/arm/target_arch_cpu.h: Implment trivial EXCP exceptions

[Warner Losh]

Implent EXCP_UDEF, EXCP_DEBUG, EXCP_INTERRUPT, EXCP_ATOMIC and
EXCP_YIELD. The first two generate a signal to the emulated
binary. EXCP_ATOMIC handles atomic operations. The remainder are fancy
nops.

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Mikaël Urankar <mikael.urankar@gmail.com>
Signed-off-by: Klye Evans <kevans@FreeBSD.org>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/arm/target_arch_cpu.h | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/bsd-user/arm/target_arch_cpu.h b/bsd-user/arm/target_arch_cpu.h
index 94c9109c3f..f22384676a 100644
--- a/bsd-user/arm/target_arch_cpu.h
+++ b/bsd-user/arm/target_arch_cpu.h
@@ -47,6 +47,34 @@ static inline void target_cpu_loop(CPUARMState *env)
         cpu_exec_end(cs);
         process_queued_cpu_work(cs);
         switch (trapnr) {
+        case EXCP_UDEF:
+            {
+                /* See arm/arm/undefined.c undefinedinstruction(); */
+                info.si_addr = env->regs[15];
+                info.si_signo = TARGET_SIGILL;
+                info.si_errno = 0;
+                info.si_code = TARGET_ILL_ILLADR;
+                queue_signal(env, info.si_signo, &info);
+            }
+            break;
+        case EXCP_INTERRUPT:
+            /* just indicate that signals should be handled asap */
+            break;
+        case EXCP_DEBUG:
+            {
+
+                info.si_signo = TARGET_SIGTRAP;
+                info.si_errno = 0;
+                info.si_code = TARGET_TRAP_BRKPT;
+                queue_signal(env, info.si_signo, &info);
+            }
+            break;
+        case EXCP_ATOMIC:
+            cpu_exec_step_atomic(cs);
+            break;
+        case EXCP_YIELD:
+            /* nothing to do here for user-mode, just resume guest code */
+            break;
         default:
             fprintf(stderr, "qemu: unhandled CPU exception 0x%x - aborting\n",
                     trapnr);
-- 
2.32.0

Re: [PATCH 07/24] bsd-user/arm/target_arch_cpu.h: Implment trivial EXCP exceptions

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
>
> Implent EXCP_UDEF, EXCP_DEBUG, EXCP_INTERRUPT, EXCP_ATOMIC and

s/Implent/Implement/

> EXCP_YIELD. The first two generate a signal to the emulated
> binary. EXCP_ATOMIC handles atomic operations. The remainder are fancy
> nops.
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Mikaël Urankar <mikael.urankar@gmail.com>
> Signed-off-by: Klye Evans <kevans@FreeBSD.org>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/arm/target_arch_cpu.h | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>
> diff --git a/bsd-user/arm/target_arch_cpu.h b/bsd-user/arm/target_arch_cpu.h
> index 94c9109c3f..f22384676a 100644
> --- a/bsd-user/arm/target_arch_cpu.h
> +++ b/bsd-user/arm/target_arch_cpu.h
> @@ -47,6 +47,34 @@ static inline void target_cpu_loop(CPUARMState *env)
>          cpu_exec_end(cs);
>          process_queued_cpu_work(cs);
>          switch (trapnr) {
> +        case EXCP_UDEF:
> +            {
> +                /* See arm/arm/undefined.c undefinedinstruction(); */
> +                info.si_addr = env->regs[15];
> +                info.si_signo = TARGET_SIGILL;
> +                info.si_errno = 0;
> +                info.si_code = TARGET_ILL_ILLADR;
> +                queue_signal(env, info.si_signo, &info);
> +            }
> +            break;
> +        case EXCP_INTERRUPT:
> +            /* just indicate that signals should be handled asap */
> +            break;
> +        case EXCP_DEBUG:
> +            {
> +
> +                info.si_signo = TARGET_SIGTRAP;
> +                info.si_errno = 0;
> +                info.si_code = TARGET_TRAP_BRKPT;
> +                queue_signal(env, info.si_signo, &info);
> +            }
> +            break;
> +        case EXCP_ATOMIC:
> +            cpu_exec_step_atomic(cs);
> +            break;
> +        case EXCP_YIELD:
> +            /* nothing to do here for user-mode, just resume guest code */
> +            break;
>          default:
>              fprintf(stderr, "qemu: unhandled CPU exception 0x%x - aborting\n",
>                      trapnr);
> --
> 2.32.0
>

Modulo typo:

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

Re: [PATCH 07/24] bsd-user/arm/target_arch_cpu.h: Implment trivial EXCP exceptions

[Richard Henderson]

On 10/19/21 9:44 AM, Warner Losh wrote:
> +        case EXCP_DEBUG:
> +            {
> +
> +                info.si_signo = TARGET_SIGTRAP;
> +                info.si_errno = 0;
> +                info.si_code = TARGET_TRAP_BRKPT;
> +                queue_signal(env, info.si_signo, &info);
> +            }

Usually this gets si_addr set too.

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

r~

[PATCH 08/24] bsd-user/arm/target_arch_cpu.h: Implement data abort exceptions

[Warner Losh]

Implement EXCP_PREFETCH_ABORT AND EXCP_DATA_ABORT. Both of these data
exceptions cause a SIGSEGV.

Signed-off-by: Klye Evans <kevans@FreeBSD.org>
Signed-off-by: Olivier Houchard <cognet@ci0.org>
Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/arm/target_arch_cpu.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/bsd-user/arm/target_arch_cpu.h b/bsd-user/arm/target_arch_cpu.h
index f22384676a..62d6ee89b6 100644
--- a/bsd-user/arm/target_arch_cpu.h
+++ b/bsd-user/arm/target_arch_cpu.h
@@ -60,6 +60,17 @@ static inline void target_cpu_loop(CPUARMState *env)
         case EXCP_INTERRUPT:
             /* 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);
+            break;
         case EXCP_DEBUG:
             {
 
-- 
2.32.0

Re: [PATCH 08/24] bsd-user/arm/target_arch_cpu.h: Implement data abort exceptions

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
>
> Implement EXCP_PREFETCH_ABORT AND EXCP_DATA_ABORT. Both of these data
> exceptions cause a SIGSEGV.
>
> Signed-off-by: Klye Evans <kevans@FreeBSD.org>
> Signed-off-by: Olivier Houchard <cognet@ci0.org>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/arm/target_arch_cpu.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/bsd-user/arm/target_arch_cpu.h b/bsd-user/arm/target_arch_cpu.h
> index f22384676a..62d6ee89b6 100644
> --- a/bsd-user/arm/target_arch_cpu.h
> +++ b/bsd-user/arm/target_arch_cpu.h
> @@ -60,6 +60,17 @@ static inline void target_cpu_loop(CPUARMState *env)
>          case EXCP_INTERRUPT:
>              /* 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);
> +            break;
>          case EXCP_DEBUG:
>              {
>
> --
> 2.32.0
>

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

Re: [PATCH 08/24] bsd-user/arm/target_arch_cpu.h: Implement data abort exceptions

[Richard Henderson]

On 10/19/21 9:44 AM, Warner Losh wrote:
> Implement EXCP_PREFETCH_ABORT AND EXCP_DATA_ABORT. Both of these data
> exceptions cause a SIGSEGV.
> 
> Signed-off-by: Klye Evans<kevans@FreeBSD.org>
> Signed-off-by: Olivier Houchard<cognet@ci0.org>
> Signed-off-by: Stacey Son<sson@FreeBSD.org>
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/arm/target_arch_cpu.h | 11 +++++++++++
>   1 file changed, 11 insertions(+)

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

I hope to have the patches that make this obsolete go in this week.


r~

Re: [PATCH 08/24] bsd-user/arm/target_arch_cpu.h: Implement data abort exceptions

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 829 bytes --]

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

> On 10/19/21 9:44 AM, Warner Losh wrote:
> > Implement EXCP_PREFETCH_ABORT AND EXCP_DATA_ABORT. Both of these data
> > exceptions cause a SIGSEGV.
> >
> > Signed-off-by: Klye Evans<kevans@FreeBSD.org>
> > Signed-off-by: Olivier Houchard<cognet@ci0.org>
> > Signed-off-by: Stacey Son<sson@FreeBSD.org>
> > Signed-off-by: Warner Losh<imp@bsdimp.com>
> > ---
> >   bsd-user/arm/target_arch_cpu.h | 11 +++++++++++
> >   1 file changed, 11 insertions(+)
>
> Short-term:
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
> I hope to have the patches that make this obsolete go in this week.
>

I both look forward to that and dread it a little. Should we try to order
our PULL requests
to make less work for both of us?

Warner

[-- Attachment #2: Type: text/html, Size: 1484 bytes --]

[PATCH 09/24] bsd-user/arm/target_arch_cpu.h: Implement system call dispatch

[Warner Losh]

Implement the system call dispatch. This implements all three kinds of
system call: direct and the two indirect variants. It handles all the
special cases for thumb as well.

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_cpu.h | 95 ++++++++++++++++++++++++++++++++++
 1 file changed, 95 insertions(+)

diff --git a/bsd-user/arm/target_arch_cpu.h b/bsd-user/arm/target_arch_cpu.h
index 62d6ee89b6..bc2eb05cfe 100644
--- a/bsd-user/arm/target_arch_cpu.h
+++ b/bsd-user/arm/target_arch_cpu.h
@@ -39,6 +39,7 @@ static inline void target_cpu_loop(CPUARMState *env)
 {
     int trapnr;
     target_siginfo_t info;
+    unsigned int n;
     CPUState *cs = env_cpu(env);
 
     for (;;) {
@@ -57,6 +58,100 @@ static inline void target_cpu_loop(CPUARMState *env)
                 queue_signal(env, info.si_signo, &info);
             }
             break;
+        case EXCP_SWI:
+        case EXCP_BKPT:
+            {
+                env->eabi = 1; /* FreeBSD is eabi only now */
+                /*
+                 * system call
+                 * See arm/arm/trap.c cpu_fetch_syscall_args()
+                 */
+                if (trapnr == EXCP_BKPT) {
+                    if (env->thumb) {
+                        env->regs[15] += 2;
+                    } else {
+                        env->regs[15] += 4;
+                    }
+                }
+                n = env->regs[7];
+                if (bsd_type == target_freebsd) {
+                    int ret;
+                    abi_ulong params = get_sp_from_cpustate(env);
+                    int32_t syscall_nr = n;
+                    int32_t arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8;
+
+                    /* See arm/arm/trap.c cpu_fetch_syscall_args() */
+                    if (syscall_nr == TARGET_FREEBSD_NR_syscall) {
+                        syscall_nr = env->regs[0];
+                        arg1 = env->regs[1];
+                        arg2 = env->regs[2];
+                        arg3 = env->regs[3];
+                        get_user_s32(arg4, params);
+                        params += sizeof(int32_t);
+                        get_user_s32(arg5, params);
+                        params += sizeof(int32_t);
+                        get_user_s32(arg6, params);
+                        params += sizeof(int32_t);
+                        get_user_s32(arg7, params);
+                        arg8 = 0;
+                    } else if (syscall_nr == TARGET_FREEBSD_NR___syscall) {
+                        syscall_nr = env->regs[0];
+                        arg1 = env->regs[2];
+                        arg2 = env->regs[3];
+                        get_user_s32(arg3, params);
+                        params += sizeof(int32_t);
+                        get_user_s32(arg4, params);
+                        params += sizeof(int32_t);
+                        get_user_s32(arg5, params);
+                        params += sizeof(int32_t);
+                        get_user_s32(arg6, params);
+                        arg7 = 0;
+                        arg8 = 0;
+                    } else {
+                        arg1 = env->regs[0];
+                        arg2 = env->regs[1];
+                        arg3 = env->regs[2];
+                        arg4 = env->regs[3];
+                        get_user_s32(arg5, params);
+                        params += sizeof(int32_t);
+                        get_user_s32(arg6, params);
+                        params += sizeof(int32_t);
+                        get_user_s32(arg7, params);
+                        params += sizeof(int32_t);
+                        get_user_s32(arg8, params);
+                    }
+                    ret = do_freebsd_syscall(env, syscall_nr, arg1, arg2, arg3,
+                            arg4, arg5, arg6, arg7, arg8);
+                    /*
+                     * Compare to arm/arm/vm_machdep.c
+                     * cpu_set_syscall_retval()
+                     */
+                    if (-TARGET_EJUSTRETURN == ret) {
+                        /*
+                         * Returning from a successful sigreturn syscall.
+                         * Avoid clobbering register state.
+                         */
+                        break;
+                    }
+                    if (-TARGET_ERESTART == ret) {
+                        env->regs[15] -= env->thumb ? 2 : 4;
+                        break;
+                    }
+                    if ((unsigned int)ret >= (unsigned int)(-515)) {
+                        ret = -ret;
+                        cpsr_write(env, CPSR_C, CPSR_C, CPSRWriteByInstr);
+                        env->regs[0] = ret;
+                    } else {
+                        cpsr_write(env, 0, CPSR_C, CPSRWriteByInstr);
+                        env->regs[0] = ret; /* XXX need to handle lseek()? */
+                        /* env->regs[1] = 0; */
+                    }
+                } else {
+                    fprintf(stderr, "qemu: bsd_type (= %d) syscall "
+                            "not supported\n", bsd_type);
+                }
+            }
+            break;
         case EXCP_INTERRUPT:
             /* just indicate that signals should be handled asap */
             break;
-- 
2.32.0

Re: [PATCH 09/24] bsd-user/arm/target_arch_cpu.h: Implement system call dispatch

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
>
> Implement the system call dispatch. This implements all three kinds of
> system call: direct and the two indirect variants. It handles all the
> special cases for thumb as well.
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Klye Evans <kevans@FreeBSD.org>
> Signed-off-by: Warner Losh <imp@bsdimp.com>

s/Klye/Kyle/

> ---
>  bsd-user/arm/target_arch_cpu.h | 95 ++++++++++++++++++++++++++++++++++
>  1 file changed, 95 insertions(+)
>
> diff --git a/bsd-user/arm/target_arch_cpu.h b/bsd-user/arm/target_arch_cpu.h
> index 62d6ee89b6..bc2eb05cfe 100644
> --- a/bsd-user/arm/target_arch_cpu.h
> +++ b/bsd-user/arm/target_arch_cpu.h
> @@ -39,6 +39,7 @@ static inline void target_cpu_loop(CPUARMState *env)
>  {
>      int trapnr;
>      target_siginfo_t info;
> +    unsigned int n;
>      CPUState *cs = env_cpu(env);
>
>      for (;;) {
> @@ -57,6 +58,100 @@ static inline void target_cpu_loop(CPUARMState *env)
>                  queue_signal(env, info.si_signo, &info);
>              }
>              break;
> +        case EXCP_SWI:
> +        case EXCP_BKPT:
> +            {
> +                env->eabi = 1; /* FreeBSD is eabi only now */
> +                /*
> +                 * system call
> +                 * See arm/arm/trap.c cpu_fetch_syscall_args()
> +                 */
> +                if (trapnr == EXCP_BKPT) {
> +                    if (env->thumb) {
> +                        env->regs[15] += 2;
> +                    } else {
> +                        env->regs[15] += 4;
> +                    }
> +                }
> +                n = env->regs[7];
> +                if (bsd_type == target_freebsd) {
> +                    int ret;
> +                    abi_ulong params = get_sp_from_cpustate(env);
> +                    int32_t syscall_nr = n;
> +                    int32_t arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8;
> +
> +                    /* See arm/arm/trap.c cpu_fetch_syscall_args() */
> +                    if (syscall_nr == TARGET_FREEBSD_NR_syscall) {
> +                        syscall_nr = env->regs[0];
> +                        arg1 = env->regs[1];
> +                        arg2 = env->regs[2];
> +                        arg3 = env->regs[3];
> +                        get_user_s32(arg4, params);
> +                        params += sizeof(int32_t);
> +                        get_user_s32(arg5, params);
> +                        params += sizeof(int32_t);
> +                        get_user_s32(arg6, params);
> +                        params += sizeof(int32_t);
> +                        get_user_s32(arg7, params);
> +                        arg8 = 0;
> +                    } else if (syscall_nr == TARGET_FREEBSD_NR___syscall) {
> +                        syscall_nr = env->regs[0];
> +                        arg1 = env->regs[2];
> +                        arg2 = env->regs[3];
> +                        get_user_s32(arg3, params);
> +                        params += sizeof(int32_t);
> +                        get_user_s32(arg4, params);
> +                        params += sizeof(int32_t);
> +                        get_user_s32(arg5, params);
> +                        params += sizeof(int32_t);
> +                        get_user_s32(arg6, params);
> +                        arg7 = 0;
> +                        arg8 = 0;
> +                    } else {
> +                        arg1 = env->regs[0];
> +                        arg2 = env->regs[1];
> +                        arg3 = env->regs[2];
> +                        arg4 = env->regs[3];
> +                        get_user_s32(arg5, params);
> +                        params += sizeof(int32_t);
> +                        get_user_s32(arg6, params);
> +                        params += sizeof(int32_t);
> +                        get_user_s32(arg7, params);
> +                        params += sizeof(int32_t);
> +                        get_user_s32(arg8, params);
> +                    }
> +                    ret = do_freebsd_syscall(env, syscall_nr, arg1, arg2, arg3,
> +                            arg4, arg5, arg6, arg7, arg8);
> +                    /*
> +                     * Compare to arm/arm/vm_machdep.c
> +                     * cpu_set_syscall_retval()
> +                     */
> +                    if (-TARGET_EJUSTRETURN == ret) {
> +                        /*
> +                         * Returning from a successful sigreturn syscall.
> +                         * Avoid clobbering register state.
> +                         */
> +                        break;
> +                    }
> +                    if (-TARGET_ERESTART == ret) {
> +                        env->regs[15] -= env->thumb ? 2 : 4;
> +                        break;
> +                    }
> +                    if ((unsigned int)ret >= (unsigned int)(-515)) {
> +                        ret = -ret;
> +                        cpsr_write(env, CPSR_C, CPSR_C, CPSRWriteByInstr);
> +                        env->regs[0] = ret;
> +                    } else {
> +                        cpsr_write(env, 0, CPSR_C, CPSRWriteByInstr);
> +                        env->regs[0] = ret; /* XXX need to handle lseek()? */
> +                        /* env->regs[1] = 0; */
> +                    }
> +                } else {
> +                    fprintf(stderr, "qemu: bsd_type (= %d) syscall "
> +                            "not supported\n", bsd_type);
> +                }
> +            }
> +            break;
>          case EXCP_INTERRUPT:
>              /* just indicate that signals should be handled asap */
>              break;
> --
> 2.32.0
>

Modulo typo:

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

Re: [PATCH 09/24] bsd-user/arm/target_arch_cpu.h: Implement system call dispatch

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 6185 bytes --]

On Sat, Oct 23, 2021 at 1:34 AM Kyle Evans <kevans@freebsd.org> wrote:

> On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
> >
> > Implement the system call dispatch. This implements all three kinds of
> > system call: direct and the two indirect variants. It handles all the
> > special cases for thumb as well.
> >
> > Signed-off-by: Stacey Son <sson@FreeBSD.org>
> > Signed-off-by: Klye Evans <kevans@FreeBSD.org>
> > Signed-off-by: Warner Losh <imp@bsdimp.com>
>
> s/Klye/Kyle/
>

I'll fix the typo in the QEMU.SOB file I have... <blush>

Thanks!


> > ---
> >  bsd-user/arm/target_arch_cpu.h | 95 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 95 insertions(+)
> >
> > diff --git a/bsd-user/arm/target_arch_cpu.h
> b/bsd-user/arm/target_arch_cpu.h
> > index 62d6ee89b6..bc2eb05cfe 100644
> > --- a/bsd-user/arm/target_arch_cpu.h
> > +++ b/bsd-user/arm/target_arch_cpu.h
> > @@ -39,6 +39,7 @@ static inline void target_cpu_loop(CPUARMState *env)
> >  {
> >      int trapnr;
> >      target_siginfo_t info;
> > +    unsigned int n;
> >      CPUState *cs = env_cpu(env);
> >
> >      for (;;) {
> > @@ -57,6 +58,100 @@ static inline void target_cpu_loop(CPUARMState *env)
> >                  queue_signal(env, info.si_signo, &info);
> >              }
> >              break;
> > +        case EXCP_SWI:
> > +        case EXCP_BKPT:
> > +            {
> > +                env->eabi = 1; /* FreeBSD is eabi only now */
> > +                /*
> > +                 * system call
> > +                 * See arm/arm/trap.c cpu_fetch_syscall_args()
> > +                 */
> > +                if (trapnr == EXCP_BKPT) {
> > +                    if (env->thumb) {
> > +                        env->regs[15] += 2;
> > +                    } else {
> > +                        env->regs[15] += 4;
> > +                    }
> > +                }
> > +                n = env->regs[7];
> > +                if (bsd_type == target_freebsd) {
> > +                    int ret;
> > +                    abi_ulong params = get_sp_from_cpustate(env);
> > +                    int32_t syscall_nr = n;
> > +                    int32_t arg1, arg2, arg3, arg4, arg5, arg6, arg7,
> arg8;
> > +
> > +                    /* See arm/arm/trap.c cpu_fetch_syscall_args() */
> > +                    if (syscall_nr == TARGET_FREEBSD_NR_syscall) {
> > +                        syscall_nr = env->regs[0];
> > +                        arg1 = env->regs[1];
> > +                        arg2 = env->regs[2];
> > +                        arg3 = env->regs[3];
> > +                        get_user_s32(arg4, params);
> > +                        params += sizeof(int32_t);
> > +                        get_user_s32(arg5, params);
> > +                        params += sizeof(int32_t);
> > +                        get_user_s32(arg6, params);
> > +                        params += sizeof(int32_t);
> > +                        get_user_s32(arg7, params);
> > +                        arg8 = 0;
> > +                    } else if (syscall_nr ==
> TARGET_FREEBSD_NR___syscall) {
> > +                        syscall_nr = env->regs[0];
> > +                        arg1 = env->regs[2];
> > +                        arg2 = env->regs[3];
> > +                        get_user_s32(arg3, params);
> > +                        params += sizeof(int32_t);
> > +                        get_user_s32(arg4, params);
> > +                        params += sizeof(int32_t);
> > +                        get_user_s32(arg5, params);
> > +                        params += sizeof(int32_t);
> > +                        get_user_s32(arg6, params);
> > +                        arg7 = 0;
> > +                        arg8 = 0;
> > +                    } else {
> > +                        arg1 = env->regs[0];
> > +                        arg2 = env->regs[1];
> > +                        arg3 = env->regs[2];
> > +                        arg4 = env->regs[3];
> > +                        get_user_s32(arg5, params);
> > +                        params += sizeof(int32_t);
> > +                        get_user_s32(arg6, params);
> > +                        params += sizeof(int32_t);
> > +                        get_user_s32(arg7, params);
> > +                        params += sizeof(int32_t);
> > +                        get_user_s32(arg8, params);
> > +                    }
> > +                    ret = do_freebsd_syscall(env, syscall_nr, arg1,
> arg2, arg3,
> > +                            arg4, arg5, arg6, arg7, arg8);
> > +                    /*
> > +                     * Compare to arm/arm/vm_machdep.c
> > +                     * cpu_set_syscall_retval()
> > +                     */
> > +                    if (-TARGET_EJUSTRETURN == ret) {
> > +                        /*
> > +                         * Returning from a successful sigreturn
> syscall.
> > +                         * Avoid clobbering register state.
> > +                         */
> > +                        break;
> > +                    }
> > +                    if (-TARGET_ERESTART == ret) {
> > +                        env->regs[15] -= env->thumb ? 2 : 4;
> > +                        break;
> > +                    }
> > +                    if ((unsigned int)ret >= (unsigned int)(-515)) {
> > +                        ret = -ret;
> > +                        cpsr_write(env, CPSR_C, CPSR_C,
> CPSRWriteByInstr);
> > +                        env->regs[0] = ret;
> > +                    } else {
> > +                        cpsr_write(env, 0, CPSR_C, CPSRWriteByInstr);
> > +                        env->regs[0] = ret; /* XXX need to handle
> lseek()? */
> > +                        /* env->regs[1] = 0; */
> > +                    }
> > +                } else {
> > +                    fprintf(stderr, "qemu: bsd_type (= %d) syscall "
> > +                            "not supported\n", bsd_type);
> > +                }
> > +            }
> > +            break;
> >          case EXCP_INTERRUPT:
> >              /* just indicate that signals should be handled asap */
> >              break;
> > --
> > 2.32.0
> >
>
> Modulo typo:
>
> Reviewed-by: Kyle Evans <kevans@FreeBSD.org>
>

[-- Attachment #2: Type: text/html, Size: 8751 bytes --]

Re: [PATCH 09/24] bsd-user/arm/target_arch_cpu.h: Implement system call dispatch

[Richard Henderson]

On 10/19/21 9:44 AM, Warner Losh wrote:
> +                env->eabi = 1; /* FreeBSD is eabi only now */

Surely this should be set target_cpu_init.  Even then it'd be just for completeness.  This 
is the kind of thing that shouldn't have escaped linux-user, as it's otherwise unused.

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


r~

Re: [PATCH 09/24] bsd-user/arm/target_arch_cpu.h: Implement system call dispatch

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 648 bytes --]

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

> On 10/19/21 9:44 AM, Warner Losh wrote:
> > +                env->eabi = 1; /* FreeBSD is eabi only now */
>
> Surely this should be set target_cpu_init.  Even then it'd be just for
> completeness.  This
> is the kind of thing that shouldn't have escaped linux-user, as it's
> otherwise unused.
>
> Otherwise,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>

I see no other uses outside of linux-user, so I'm just going to delete this
line. FreeBSD
used to support OABI in bsd-user, but always badly and so that code was
removed.

Warner

[-- Attachment #2: Type: text/html, Size: 1145 bytes --]

[PATCH 10/24] bsd-user/arm/target_arch_reg.h: Implement core dump register copying

[Warner Losh]

Implement the register copying routines to extract registers from the
cpu for core dump generation.

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

diff --git a/bsd-user/arm/target_arch_reg.h b/bsd-user/arm/target_arch_reg.h
new file mode 100644
index 0000000000..ef5ed5154f
--- /dev/null
+++ b/bsd-user/arm/target_arch_reg.h
@@ -0,0 +1,60 @@
+/*
+ *  FreeBSD arm register structures
+ *
+ *  Copyright (c) 2015 Stacey 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_REG_H_
+#define _TARGET_ARCH_REG_H_
+
+/* See sys/arm/include/reg.h */
+typedef struct target_reg {
+    uint32_t        r[13];
+    uint32_t        r_sp;
+    uint32_t        r_lr;
+    uint32_t        r_pc;
+    uint32_t        r_cpsr;
+} target_reg_t;
+
+typedef struct target_fp_reg {
+    uint32_t        fp_exponent;
+    uint32_t        fp_mantissa_hi;
+    u_int32_t       fp_mantissa_lo;
+} target_fp_reg_t;
+
+typedef struct target_fpreg {
+    uint32_t        fpr_fpsr;
+    target_fp_reg_t fpr[8];
+} target_fpreg_t;
+
+#define tswapreg(ptr)   tswapal(ptr)
+
+static inline void target_copy_regs(target_reg_t *regs, const CPUARMState *env)
+{
+    int i;
+
+    for (i = 0; i < 13; i++) {
+        regs->r[i] = tswapreg(env->regs[i + 1]);
+    }
+    regs->r_sp = tswapreg(env->regs[13]);
+    regs->r_lr = tswapreg(env->regs[14]);
+    regs->r_pc = tswapreg(env->regs[15]);
+    regs->r_cpsr = tswapreg(cpsr_read((CPUARMState *)env));
+}
+
+#undef tswapreg
+
+#endif /* !_TARGET_ARCH_REG_H_ */
-- 
2.32.0

Re: [PATCH 10/24] bsd-user/arm/target_arch_reg.h: Implement core dump register copying

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
>
> Implement the register copying routines to extract registers from the
> cpu for core dump generation.
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/arm/target_arch_reg.h | 60 ++++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
>  create mode 100644 bsd-user/arm/target_arch_reg.h
>
> diff --git a/bsd-user/arm/target_arch_reg.h b/bsd-user/arm/target_arch_reg.h
> new file mode 100644
> index 0000000000..ef5ed5154f
> --- /dev/null
> +++ b/bsd-user/arm/target_arch_reg.h
> @@ -0,0 +1,60 @@
> +/*
> + *  FreeBSD arm register structures
> + *
> + *  Copyright (c) 2015 Stacey 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_REG_H_
> +#define _TARGET_ARCH_REG_H_
> +
> +/* See sys/arm/include/reg.h */
> +typedef struct target_reg {
> +    uint32_t        r[13];
> +    uint32_t        r_sp;
> +    uint32_t        r_lr;
> +    uint32_t        r_pc;
> +    uint32_t        r_cpsr;
> +} target_reg_t;
> +
> +typedef struct target_fp_reg {
> +    uint32_t        fp_exponent;
> +    uint32_t        fp_mantissa_hi;
> +    u_int32_t       fp_mantissa_lo;
> +} target_fp_reg_t;
> +
> +typedef struct target_fpreg {
> +    uint32_t        fpr_fpsr;
> +    target_fp_reg_t fpr[8];
> +} target_fpreg_t;
> +
> +#define tswapreg(ptr)   tswapal(ptr)
> +
> +static inline void target_copy_regs(target_reg_t *regs, const CPUARMState *env)
> +{
> +    int i;
> +
> +    for (i = 0; i < 13; i++) {
> +        regs->r[i] = tswapreg(env->regs[i + 1]);
> +    }
> +    regs->r_sp = tswapreg(env->regs[13]);
> +    regs->r_lr = tswapreg(env->regs[14]);
> +    regs->r_pc = tswapreg(env->regs[15]);
> +    regs->r_cpsr = tswapreg(cpsr_read((CPUARMState *)env));
> +}
> +
> +#undef tswapreg
> +
> +#endif /* !_TARGET_ARCH_REG_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

Re: [PATCH 10/24] bsd-user/arm/target_arch_reg.h: Implement core dump register copying

[Richard Henderson]

On 10/19/21 9:44 AM, Warner Losh wrote:
> Implement the register copying routines to extract registers from the
> cpu for core dump generation.
> 
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Warner Losh <imp@bsdimp.com>

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

> +static inline void target_copy_regs(target_reg_t *regs, const CPUARMState *env)
> +{
> +    int i;
> +
> +    for (i = 0; i < 13; i++) {
> +        regs->r[i] = tswapreg(env->regs[i + 1]);
> +    }
> +    regs->r_sp = tswapreg(env->regs[13]);
> +    regs->r_lr = tswapreg(env->regs[14]);
> +    regs->r_pc = tswapreg(env->regs[15]);
> +    regs->r_cpsr = tswapreg(cpsr_read((CPUARMState *)env));

I guess we could do with a bit of const-ification in target/arm/...


r~

[PATCH 11/24] bsd-user/arm/target_arch_vmparam.h: Parameters for arm address space

[Warner Losh]

Various parameters describing the layout of the ARM address space. In
addition, define routines to get the stack pointer and to set the second
return value.

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_vmparam.h | 48 ++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 bsd-user/arm/target_arch_vmparam.h

diff --git a/bsd-user/arm/target_arch_vmparam.h b/bsd-user/arm/target_arch_vmparam.h
new file mode 100644
index 0000000000..4bbc04ddf5
--- /dev/null
+++ b/bsd-user/arm/target_arch_vmparam.h
@@ -0,0 +1,48 @@
+/*
+ *  arm VM parameters definitions
+ *
+ *  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_VMPARAM_H_
+#define _TARGET_ARCH_VMPARAM_H_
+
+#include "cpu.h"
+
+/* compare to sys/arm/include/vmparam.h */
+#define TARGET_MAXTSIZ      (64 * MiB)           /* max text size */
+#define TARGET_DFLDSIZ      (128 * MiB)          /* initial data size limit */
+#define TARGET_MAXDSIZ      (512 * MiB)          /* max data size */
+#define TARGET_DFLSSIZ      (4 * MiB)            /* initial stack size limit */
+#define TARGET_MAXSSIZ      (64 * MiB)           /* max stack size */
+#define TARGET_SGROWSIZ     (128 * KiB)          /* amount to grow stack */
+
+#define TARGET_RESERVED_VA  0xf7000000
+
+                /* KERNBASE - 512 MB */
+#define TARGET_VM_MAXUSER_ADDRESS   (0xc0000000 - (512 * MiB))
+#define TARGET_USRSTACK             TARGET_VM_MAXUSER_ADDRESS
+
+static inline abi_ulong get_sp_from_cpustate(CPUARMState *state)
+{
+    return state->regs[13]; /* sp */
+}
+
+static inline void set_second_rval(CPUARMState *state, abi_ulong retval2)
+{
+    state->regs[1] = retval2;
+}
+
+#endif  /* ! _TARGET_ARCH_VMPARAM_H_ */
-- 
2.32.0

Re: [PATCH 11/24] bsd-user/arm/target_arch_vmparam.h: Parameters for arm address space

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
>
> Various parameters describing the layout of the ARM address space. In
> addition, define routines to get the stack pointer and to set the second
> return value.
>
> 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_vmparam.h | 48 ++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>  create mode 100644 bsd-user/arm/target_arch_vmparam.h
>
> diff --git a/bsd-user/arm/target_arch_vmparam.h b/bsd-user/arm/target_arch_vmparam.h
> new file mode 100644
> index 0000000000..4bbc04ddf5
> --- /dev/null
> +++ b/bsd-user/arm/target_arch_vmparam.h
> @@ -0,0 +1,48 @@
> +/*
> + *  arm VM parameters definitions
> + *
> + *  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_VMPARAM_H_
> +#define _TARGET_ARCH_VMPARAM_H_
> +
> +#include "cpu.h"
> +
> +/* compare to sys/arm/include/vmparam.h */
> +#define TARGET_MAXTSIZ      (64 * MiB)           /* max text size */
> +#define TARGET_DFLDSIZ      (128 * MiB)          /* initial data size limit */
> +#define TARGET_MAXDSIZ      (512 * MiB)          /* max data size */
> +#define TARGET_DFLSSIZ      (4 * MiB)            /* initial stack size limit */
> +#define TARGET_MAXSSIZ      (64 * MiB)           /* max stack size */
> +#define TARGET_SGROWSIZ     (128 * KiB)          /* amount to grow stack */
> +
> +#define TARGET_RESERVED_VA  0xf7000000
> +
> +                /* KERNBASE - 512 MB */
> +#define TARGET_VM_MAXUSER_ADDRESS   (0xc0000000 - (512 * MiB))
> +#define TARGET_USRSTACK             TARGET_VM_MAXUSER_ADDRESS
> +
> +static inline abi_ulong get_sp_from_cpustate(CPUARMState *state)
> +{
> +    return state->regs[13]; /* sp */
> +}
> +
> +static inline void set_second_rval(CPUARMState *state, abi_ulong retval2)
> +{
> +    state->regs[1] = retval2;
> +}
> +
> +#endif  /* ! _TARGET_ARCH_VMPARAM_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

Re: [PATCH 11/24] bsd-user/arm/target_arch_vmparam.h: Parameters for arm address space

[Richard Henderson]

On 10/19/21 9:44 AM, Warner Losh wrote:
> Various parameters describing the layout of the ARM address space. In
> addition, define routines to get the stack pointer and to set the second
> return value.
> 
> 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_vmparam.h | 48 ++++++++++++++++++++++++++++++
>   1 file changed, 48 insertions(+)
>   create mode 100644 bsd-user/arm/target_arch_vmparam.h

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

r~

[PATCH 12/24] bsd-user/arm/target_arch_sigtramp.h: Signal Trampoline for arm

[Warner Losh]

Copy of the signal trampoline code for arm, as well as setup_sigtramp to
write it to the stack.

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

diff --git a/bsd-user/arm/target_arch_sigtramp.h b/bsd-user/arm/target_arch_sigtramp.h
new file mode 100644
index 0000000000..ed53d336ed
--- /dev/null
+++ b/bsd-user/arm/target_arch_sigtramp.h
@@ -0,0 +1,52 @@
+/*
+ *  arm sysarch() system call emulation
+ *
+ *  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_SIGTRAMP_H_
+#define _TARGET_ARCH_SIGTRAMP_H_
+
+/* Compare to arm/arm/locore.S ENTRY_NP(sigcode) */
+static inline abi_long setup_sigtramp(abi_ulong offset, unsigned sigf_uc,
+        unsigned sys_sigreturn)
+{
+    int i;
+    uint32_t sys_exit = TARGET_FREEBSD_NR_exit;
+    /*
+     * The code has to load r7 manually rather than using
+     * "ldr r7, =SYS_return to make sure the size of the
+     * code is correct.
+     */
+    uint32_t sigtramp_code[] = {
+    /* 1 */ 0xE1A0000D,                  /* mov r0, sp */
+    /* 2 */ 0xE2800000 + sigf_uc,        /* add r0, r0, #SIGF_UC */
+    /* 3 */ 0xE59F700C,                  /* ldr r7, [pc, #12] */
+    /* 4 */ 0xEF000000 + sys_sigreturn,  /* swi (SYS_sigreturn) */
+    /* 5 */ 0xE59F7008,                  /* ldr r7, [pc, #8] */
+    /* 6 */ 0xEF000000 + sys_exit,       /* swi (SYS_exit)*/
+    /* 7 */ 0xEAFFFFFA,                  /* b . -16 */
+    /* 8 */ sys_sigreturn,
+    /* 9 */ sys_exit
+    };
+
+    for (i = 0; i < 9; i++) {
+        tswap32s(&sigtramp_code[i]);
+    }
+
+    return memcpy_to_target(offset, sigtramp_code, TARGET_SZSIGCODE);
+}
+#endif /* _TARGET_ARCH_SIGTRAMP_H_ */
-- 
2.32.0

Re: [PATCH 12/24] bsd-user/arm/target_arch_sigtramp.h: Signal Trampoline for arm

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
>
> Copy of the signal trampoline code for arm, as well as setup_sigtramp to
> write it to the stack.
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/arm/target_arch_sigtramp.h | 52 +++++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
>  create mode 100644 bsd-user/arm/target_arch_sigtramp.h
>
> diff --git a/bsd-user/arm/target_arch_sigtramp.h b/bsd-user/arm/target_arch_sigtramp.h
> new file mode 100644
> index 0000000000..ed53d336ed
> --- /dev/null
> +++ b/bsd-user/arm/target_arch_sigtramp.h
> @@ -0,0 +1,52 @@
> +/*
> + *  arm sysarch() system call emulation
> + *
> + *  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_SIGTRAMP_H_
> +#define _TARGET_ARCH_SIGTRAMP_H_
> +
> +/* Compare to arm/arm/locore.S ENTRY_NP(sigcode) */
> +static inline abi_long setup_sigtramp(abi_ulong offset, unsigned sigf_uc,
> +        unsigned sys_sigreturn)
> +{
> +    int i;
> +    uint32_t sys_exit = TARGET_FREEBSD_NR_exit;
> +    /*
> +     * The code has to load r7 manually rather than using
> +     * "ldr r7, =SYS_return to make sure the size of the
> +     * code is correct.
> +     */
> +    uint32_t sigtramp_code[] = {
> +    /* 1 */ 0xE1A0000D,                  /* mov r0, sp */
> +    /* 2 */ 0xE2800000 + sigf_uc,        /* add r0, r0, #SIGF_UC */
> +    /* 3 */ 0xE59F700C,                  /* ldr r7, [pc, #12] */
> +    /* 4 */ 0xEF000000 + sys_sigreturn,  /* swi (SYS_sigreturn) */
> +    /* 5 */ 0xE59F7008,                  /* ldr r7, [pc, #8] */
> +    /* 6 */ 0xEF000000 + sys_exit,       /* swi (SYS_exit)*/
> +    /* 7 */ 0xEAFFFFFA,                  /* b . -16 */
> +    /* 8 */ sys_sigreturn,
> +    /* 9 */ sys_exit
> +    };
> +
> +    for (i = 0; i < 9; i++) {
> +        tswap32s(&sigtramp_code[i]);
> +    }
> +
> +    return memcpy_to_target(offset, sigtramp_code, TARGET_SZSIGCODE);
> +}
> +#endif /* _TARGET_ARCH_SIGTRAMP_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

Re: [PATCH 12/24] bsd-user/arm/target_arch_sigtramp.h: Signal Trampoline for arm

[Richard Henderson]

On 10/19/21 9:44 AM, Warner Losh wrote:
> +    /*
> +     * The code has to load r7 manually rather than using
> +     * "ldr r7, =SYS_return to make sure the size of the
> +     * code is correct.
> +     */

This comment was applicable to the original freebsd asm source; it has no bearing on the 
hex instructions below.

> +    uint32_t sigtramp_code[] = {
> +    /* 1 */ 0xE1A0000D,                  /* mov r0, sp */
> +    /* 2 */ 0xE2800000 + sigf_uc,        /* add r0, r0, #SIGF_UC */

I guess this was a bit of thumb1 source that accidentally became ABI?
Otherwise I can't think why you wouldn't squish the mov into the add.

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

r~

Re: [PATCH 12/24] bsd-user/arm/target_arch_sigtramp.h: Signal Trampoline for arm

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 1361 bytes --]

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

> On 10/19/21 9:44 AM, Warner Losh wrote:
> > +    /*
> > +     * The code has to load r7 manually rather than using
> > +     * "ldr r7, =SYS_return to make sure the size of the
> > +     * code is correct.
> > +     */
>
> This comment was applicable to the original freebsd asm source; it has no
> bearing on the
> hex instructions below


Yea, the code size will always be correct...


> > +    uint32_t sigtramp_code[] = {
> > +    /* 1 */ 0xE1A0000D,                  /* mov r0, sp */
> > +    /* 2 */ 0xE2800000 + sigf_uc,        /* add r0, r0, #SIGF_UC */
>
> I guess this was a bit of thumb1 source that accidentally became ABI?
> Otherwise I can't think why you wouldn't squish the mov into the add.
>

It's in our ABI. gdb and others know what the sigcode signature looks like
for sigreturn
so we can't really change it without a lot of pain... So we're kinda stuck
with very
old-school ARM code sequences for our modern armv7 port. There's likely no
reason to have the defensive call to sys_exit either, but like I said,
we're kinda
stuck with it w/o replacing it in a number of external sources...  You may
see similar
issues with the aarch64 code I'll post next.


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

thanks!

Warner


> r~
>

[-- Attachment #2: Type: text/html, Size: 2464 bytes --]

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

[Warner Losh]

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

[Kyle Evans]

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

[Kyle Evans]

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

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 4547 bytes --]

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

[-- Attachment #2: Type: text/html, Size: 6364 bytes --]

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

[Kyle Evans]

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

[Richard Henderson]

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

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 628 bytes --]

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~
>

[-- Attachment #2: Type: text/html, Size: 1442 bytes --]

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

[Richard Henderson]

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~

[PATCH 14/24] bsd-user/arm/target_arch_elf.h: arm defines for ELF

[Warner Losh]

Basic set of defines needed for arm ELF file activation.

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

diff --git a/bsd-user/arm/target_arch_elf.h b/bsd-user/arm/target_arch_elf.h
new file mode 100644
index 0000000000..15b5c66511
--- /dev/null
+++ b/bsd-user/arm/target_arch_elf.h
@@ -0,0 +1,36 @@
+/*
+ *  arm ELF definitions
+ *
+ *  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_ELF_H_
+#define _TARGET_ARCH_ELF_H_
+
+#define ELF_START_MMAP 0x80000000
+#define ELF_ET_DYN_LOAD_ADDR    0x500000
+
+#define elf_check_arch(x) ((x) == EM_ARM)
+
+#define ELF_CLASS       ELFCLASS32
+#define ELF_DATA        ELFDATA2LSB
+#define ELF_ARCH        EM_ARM
+
+#define USE_ELF_CORE_DUMP
+#define ELF_EXEC_PAGESIZE       4096
+
+#define ELF_HWCAP 0
+
+#endif /* _TARGET_ARCH_ELF_H_ */
-- 
2.32.0

Re: [PATCH 14/24] bsd-user/arm/target_arch_elf.h: arm defines for ELF

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
>
> Basic set of defines needed for arm ELF file activation.
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/arm/target_arch_elf.h | 36 ++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>  create mode 100644 bsd-user/arm/target_arch_elf.h
>
> diff --git a/bsd-user/arm/target_arch_elf.h b/bsd-user/arm/target_arch_elf.h
> new file mode 100644
> index 0000000000..15b5c66511
> --- /dev/null
> +++ b/bsd-user/arm/target_arch_elf.h
> @@ -0,0 +1,36 @@
> +/*
> + *  arm ELF definitions
> + *
> + *  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_ELF_H_
> +#define _TARGET_ARCH_ELF_H_
> +
> +#define ELF_START_MMAP 0x80000000
> +#define ELF_ET_DYN_LOAD_ADDR    0x500000
> +
> +#define elf_check_arch(x) ((x) == EM_ARM)
> +
> +#define ELF_CLASS       ELFCLASS32
> +#define ELF_DATA        ELFDATA2LSB
> +#define ELF_ARCH        EM_ARM
> +
> +#define USE_ELF_CORE_DUMP
> +#define ELF_EXEC_PAGESIZE       4096
> +
> +#define ELF_HWCAP 0
> +
> +#endif /* _TARGET_ARCH_ELF_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

Re: [PATCH 14/24] bsd-user/arm/target_arch_elf.h: arm defines for ELF

[Richard Henderson]

On 10/19/21 9:44 AM, Warner Losh wrote:
> Basic set of defines needed for arm ELF file activation.
> 
> Signed-off-by: Stacey Son<sson@FreeBSD.org>
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/arm/target_arch_elf.h | 36 ++++++++++++++++++++++++++++++++++
>   1 file changed, 36 insertions(+)
>   create mode 100644 bsd-user/arm/target_arch_elf.h

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

r~

[PATCH 15/24] bsd-user/arm/target_arch_elf.h: arm get hwcap

[Warner Losh]

Implement get_elf_hwcap to get the first word of hardware capabilities.

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

diff --git a/bsd-user/arm/target_arch_elf.h b/bsd-user/arm/target_arch_elf.h
index 15b5c66511..02d25b8926 100644
--- a/bsd-user/arm/target_arch_elf.h
+++ b/bsd-user/arm/target_arch_elf.h
@@ -31,6 +31,76 @@
 #define USE_ELF_CORE_DUMP
 #define ELF_EXEC_PAGESIZE       4096
 
-#define ELF_HWCAP 0
+#define ELF_HWCAP get_elf_hwcap()
+
+#define GET_FEATURE(feat, hwcap) \
+    do { if (arm_feature(&cpu->env, feat)) { hwcaps |= hwcap; } } while (0)
+
+#define GET_FEATURE_ID(feat, hwcap) \
+    do { if (cpu_isar_feature(feat, cpu)) { hwcaps |= hwcap; } } while (0)
+
+enum {
+    ARM_HWCAP_ARM_SWP       = 1 << 0,
+    ARM_HWCAP_ARM_HALF      = 1 << 1,
+    ARM_HWCAP_ARM_THUMB     = 1 << 2,
+    ARM_HWCAP_ARM_26BIT     = 1 << 3,
+    ARM_HWCAP_ARM_FAST_MULT = 1 << 4,
+    ARM_HWCAP_ARM_FPA       = 1 << 5,
+    ARM_HWCAP_ARM_VFP       = 1 << 6,
+    ARM_HWCAP_ARM_EDSP      = 1 << 7,
+    ARM_HWCAP_ARM_JAVA      = 1 << 8,
+    ARM_HWCAP_ARM_IWMMXT    = 1 << 9,
+    ARM_HWCAP_ARM_CRUNCH    = 1 << 10,
+    ARM_HWCAP_ARM_THUMBEE   = 1 << 11,
+    ARM_HWCAP_ARM_NEON      = 1 << 12,
+    ARM_HWCAP_ARM_VFPv3     = 1 << 13,
+    ARM_HWCAP_ARM_VFPv3D16  = 1 << 14,
+    ARM_HWCAP_ARM_TLS       = 1 << 15,
+    ARM_HWCAP_ARM_VFPv4     = 1 << 16,
+    ARM_HWCAP_ARM_IDIVA     = 1 << 17,
+    ARM_HWCAP_ARM_IDIVT     = 1 << 18,
+    ARM_HWCAP_ARM_VFPD32    = 1 << 19,
+    ARM_HWCAP_ARM_LPAE      = 1 << 20,
+    ARM_HWCAP_ARM_EVTSTRM   = 1 << 21,
+};
+
+static uint32_t get_elf_hwcap(void)
+{
+    ARMCPU *cpu = ARM_CPU(thread_cpu);
+    uint32_t hwcaps = 0;
+
+    hwcaps |= ARM_HWCAP_ARM_SWP;
+    hwcaps |= ARM_HWCAP_ARM_HALF;
+    hwcaps |= ARM_HWCAP_ARM_THUMB;
+    hwcaps |= ARM_HWCAP_ARM_FAST_MULT;
+
+    /* probe for the extra features */
+    /* EDSP is in v5TE and above */
+    GET_FEATURE(ARM_FEATURE_V5, ARM_HWCAP_ARM_EDSP);
+    GET_FEATURE(ARM_FEATURE_IWMMXT, ARM_HWCAP_ARM_IWMMXT);
+    GET_FEATURE(ARM_FEATURE_THUMB2EE, ARM_HWCAP_ARM_THUMBEE);
+    GET_FEATURE(ARM_FEATURE_NEON, ARM_HWCAP_ARM_NEON);
+    GET_FEATURE(ARM_FEATURE_V6K, ARM_HWCAP_ARM_TLS);
+    GET_FEATURE(ARM_FEATURE_LPAE, ARM_HWCAP_ARM_LPAE);
+    GET_FEATURE_ID(aa32_arm_div, ARM_HWCAP_ARM_IDIVA);
+    GET_FEATURE_ID(aa32_thumb_div, ARM_HWCAP_ARM_IDIVT);
+    GET_FEATURE_ID(aa32_vfp, ARM_HWCAP_ARM_VFP);
+
+    if (cpu_isar_feature(aa32_fpsp_v3, cpu) ||
+        cpu_isar_feature(aa32_fpdp_v3, cpu)) {
+        hwcaps |= ARM_HWCAP_ARM_VFPv3;
+        if (cpu_isar_feature(aa32_simd_r32, cpu)) {
+            hwcaps |= ARM_HWCAP_ARM_VFPD32;
+        } else {
+            hwcaps |= ARM_HWCAP_ARM_VFPv3D16;
+        }
+    }
+    GET_FEATURE_ID(aa32_simdfmac, ARM_HWCAP_ARM_VFPv4);
+
+    return hwcaps;
+}
+
+#undef GET_FEATURE
+#undef GET_FEATURE_ID
 
 #endif /* _TARGET_ARCH_ELF_H_ */
-- 
2.32.0

Re: [PATCH 15/24] bsd-user/arm/target_arch_elf.h: arm get hwcap

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
>
> Implement get_elf_hwcap to get the first word of hardware capabilities.
>
> Signed-off-by: Klye Evans <kevans@FreeBSD.org>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/arm/target_arch_elf.h | 72 +++++++++++++++++++++++++++++++++-
>  1 file changed, 71 insertions(+), 1 deletion(-)
>
> diff --git a/bsd-user/arm/target_arch_elf.h b/bsd-user/arm/target_arch_elf.h
> index 15b5c66511..02d25b8926 100644
> --- a/bsd-user/arm/target_arch_elf.h
> +++ b/bsd-user/arm/target_arch_elf.h
> @@ -31,6 +31,76 @@
>  #define USE_ELF_CORE_DUMP
>  #define ELF_EXEC_PAGESIZE       4096
>
> -#define ELF_HWCAP 0
> +#define ELF_HWCAP get_elf_hwcap()
> +
> +#define GET_FEATURE(feat, hwcap) \
> +    do { if (arm_feature(&cpu->env, feat)) { hwcaps |= hwcap; } } while (0)
> +
> +#define GET_FEATURE_ID(feat, hwcap) \
> +    do { if (cpu_isar_feature(feat, cpu)) { hwcaps |= hwcap; } } while (0)
> +
> +enum {
> +    ARM_HWCAP_ARM_SWP       = 1 << 0,
> +    ARM_HWCAP_ARM_HALF      = 1 << 1,
> +    ARM_HWCAP_ARM_THUMB     = 1 << 2,
> +    ARM_HWCAP_ARM_26BIT     = 1 << 3,
> +    ARM_HWCAP_ARM_FAST_MULT = 1 << 4,
> +    ARM_HWCAP_ARM_FPA       = 1 << 5,
> +    ARM_HWCAP_ARM_VFP       = 1 << 6,
> +    ARM_HWCAP_ARM_EDSP      = 1 << 7,
> +    ARM_HWCAP_ARM_JAVA      = 1 << 8,
> +    ARM_HWCAP_ARM_IWMMXT    = 1 << 9,
> +    ARM_HWCAP_ARM_CRUNCH    = 1 << 10,
> +    ARM_HWCAP_ARM_THUMBEE   = 1 << 11,
> +    ARM_HWCAP_ARM_NEON      = 1 << 12,
> +    ARM_HWCAP_ARM_VFPv3     = 1 << 13,
> +    ARM_HWCAP_ARM_VFPv3D16  = 1 << 14,
> +    ARM_HWCAP_ARM_TLS       = 1 << 15,
> +    ARM_HWCAP_ARM_VFPv4     = 1 << 16,
> +    ARM_HWCAP_ARM_IDIVA     = 1 << 17,
> +    ARM_HWCAP_ARM_IDIVT     = 1 << 18,
> +    ARM_HWCAP_ARM_VFPD32    = 1 << 19,
> +    ARM_HWCAP_ARM_LPAE      = 1 << 20,
> +    ARM_HWCAP_ARM_EVTSTRM   = 1 << 21,
> +};
> +
> +static uint32_t get_elf_hwcap(void)
> +{
> +    ARMCPU *cpu = ARM_CPU(thread_cpu);
> +    uint32_t hwcaps = 0;
> +
> +    hwcaps |= ARM_HWCAP_ARM_SWP;
> +    hwcaps |= ARM_HWCAP_ARM_HALF;
> +    hwcaps |= ARM_HWCAP_ARM_THUMB;
> +    hwcaps |= ARM_HWCAP_ARM_FAST_MULT;
> +
> +    /* probe for the extra features */
> +    /* EDSP is in v5TE and above */
> +    GET_FEATURE(ARM_FEATURE_V5, ARM_HWCAP_ARM_EDSP);
> +    GET_FEATURE(ARM_FEATURE_IWMMXT, ARM_HWCAP_ARM_IWMMXT);
> +    GET_FEATURE(ARM_FEATURE_THUMB2EE, ARM_HWCAP_ARM_THUMBEE);
> +    GET_FEATURE(ARM_FEATURE_NEON, ARM_HWCAP_ARM_NEON);
> +    GET_FEATURE(ARM_FEATURE_V6K, ARM_HWCAP_ARM_TLS);
> +    GET_FEATURE(ARM_FEATURE_LPAE, ARM_HWCAP_ARM_LPAE);
> +    GET_FEATURE_ID(aa32_arm_div, ARM_HWCAP_ARM_IDIVA);
> +    GET_FEATURE_ID(aa32_thumb_div, ARM_HWCAP_ARM_IDIVT);
> +    GET_FEATURE_ID(aa32_vfp, ARM_HWCAP_ARM_VFP);
> +
> +    if (cpu_isar_feature(aa32_fpsp_v3, cpu) ||
> +        cpu_isar_feature(aa32_fpdp_v3, cpu)) {
> +        hwcaps |= ARM_HWCAP_ARM_VFPv3;
> +        if (cpu_isar_feature(aa32_simd_r32, cpu)) {
> +            hwcaps |= ARM_HWCAP_ARM_VFPD32;
> +        } else {
> +            hwcaps |= ARM_HWCAP_ARM_VFPv3D16;
> +        }
> +    }
> +    GET_FEATURE_ID(aa32_simdfmac, ARM_HWCAP_ARM_VFPv4);
> +
> +    return hwcaps;
> +}
> +
> +#undef GET_FEATURE
> +#undef GET_FEATURE_ID
>
>  #endif /* _TARGET_ARCH_ELF_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

Re: [PATCH 15/24] bsd-user/arm/target_arch_elf.h: arm get hwcap

[Richard Henderson]

On 10/19/21 9:44 AM, Warner Losh wrote:
> Implement get_elf_hwcap to get the first word of hardware capabilities.
> 
> Signed-off-by: Klye Evans<kevans@FreeBSD.org>
> Signed-off-by: Stacey Son<sson@FreeBSD.org>
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/arm/target_arch_elf.h | 72 +++++++++++++++++++++++++++++++++-
>   1 file changed, 71 insertions(+), 1 deletion(-)

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

r~

[PATCH 16/24] bsd-user/arm/target_arch_elf.h: arm get_hwcap2 impl

[Warner Losh]

Implement the extended HW capabilities for HWCAP2.

Signed-off-by: Klye Evans <kevans@FreeBSD.org>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/arm/target_arch_elf.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/bsd-user/arm/target_arch_elf.h b/bsd-user/arm/target_arch_elf.h
index 02d25b8926..4a0215d02e 100644
--- a/bsd-user/arm/target_arch_elf.h
+++ b/bsd-user/arm/target_arch_elf.h
@@ -32,6 +32,7 @@
 #define ELF_EXEC_PAGESIZE       4096
 
 #define ELF_HWCAP get_elf_hwcap()
+#define ELF_HWCAP2 get_elf_hwcap2()
 
 #define GET_FEATURE(feat, hwcap) \
     do { if (arm_feature(&cpu->env, feat)) { hwcaps |= hwcap; } } while (0)
@@ -64,6 +65,14 @@ enum {
     ARM_HWCAP_ARM_EVTSTRM   = 1 << 21,
 };
 
+enum {
+    ARM_HWCAP2_ARM_AES      = 1 << 0,
+    ARM_HWCAP2_ARM_PMULL    = 1 << 1,
+    ARM_HWCAP2_ARM_SHA1     = 1 << 2,
+    ARM_HWCAP2_ARM_SHA2     = 1 << 3,
+    ARM_HWCAP2_ARM_CRC32    = 1 << 4,
+};
+
 static uint32_t get_elf_hwcap(void)
 {
     ARMCPU *cpu = ARM_CPU(thread_cpu);
@@ -100,6 +109,19 @@ static uint32_t get_elf_hwcap(void)
     return hwcaps;
 }
 
+static uint32_t get_elf_hwcap2(void)
+{
+    ARMCPU *cpu = ARM_CPU(thread_cpu);
+    uint32_t hwcaps = 0;
+
+    GET_FEATURE_ID(aa32_aes, ARM_HWCAP2_ARM_AES);
+    GET_FEATURE_ID(aa32_pmull, ARM_HWCAP2_ARM_PMULL);
+    GET_FEATURE_ID(aa32_sha1, ARM_HWCAP2_ARM_SHA1);
+    GET_FEATURE_ID(aa32_sha2, ARM_HWCAP2_ARM_SHA2);
+    GET_FEATURE_ID(aa32_crc32, ARM_HWCAP2_ARM_CRC32);
+    return hwcaps;
+}
+
 #undef GET_FEATURE
 #undef GET_FEATURE_ID
 
-- 
2.32.0

Re: [PATCH 16/24] bsd-user/arm/target_arch_elf.h: arm get_hwcap2 impl

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
>
> Implement the extended HW capabilities for HWCAP2.
>
> Signed-off-by: Klye Evans <kevans@FreeBSD.org>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/arm/target_arch_elf.h | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/bsd-user/arm/target_arch_elf.h b/bsd-user/arm/target_arch_elf.h
> index 02d25b8926..4a0215d02e 100644
> --- a/bsd-user/arm/target_arch_elf.h
> +++ b/bsd-user/arm/target_arch_elf.h
> @@ -32,6 +32,7 @@
>  #define ELF_EXEC_PAGESIZE       4096
>
>  #define ELF_HWCAP get_elf_hwcap()
> +#define ELF_HWCAP2 get_elf_hwcap2()
>
>  #define GET_FEATURE(feat, hwcap) \
>      do { if (arm_feature(&cpu->env, feat)) { hwcaps |= hwcap; } } while (0)
> @@ -64,6 +65,14 @@ enum {
>      ARM_HWCAP_ARM_EVTSTRM   = 1 << 21,
>  };
>
> +enum {
> +    ARM_HWCAP2_ARM_AES      = 1 << 0,
> +    ARM_HWCAP2_ARM_PMULL    = 1 << 1,
> +    ARM_HWCAP2_ARM_SHA1     = 1 << 2,
> +    ARM_HWCAP2_ARM_SHA2     = 1 << 3,
> +    ARM_HWCAP2_ARM_CRC32    = 1 << 4,
> +};
> +
>  static uint32_t get_elf_hwcap(void)
>  {
>      ARMCPU *cpu = ARM_CPU(thread_cpu);
> @@ -100,6 +109,19 @@ static uint32_t get_elf_hwcap(void)
>      return hwcaps;
>  }
>
> +static uint32_t get_elf_hwcap2(void)
> +{
> +    ARMCPU *cpu = ARM_CPU(thread_cpu);
> +    uint32_t hwcaps = 0;
> +
> +    GET_FEATURE_ID(aa32_aes, ARM_HWCAP2_ARM_AES);
> +    GET_FEATURE_ID(aa32_pmull, ARM_HWCAP2_ARM_PMULL);
> +    GET_FEATURE_ID(aa32_sha1, ARM_HWCAP2_ARM_SHA1);
> +    GET_FEATURE_ID(aa32_sha2, ARM_HWCAP2_ARM_SHA2);
> +    GET_FEATURE_ID(aa32_crc32, ARM_HWCAP2_ARM_CRC32);
> +    return hwcaps;
> +}
> +
>  #undef GET_FEATURE
>  #undef GET_FEATURE_ID
>
> --
> 2.32.0
>

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

Re: [PATCH 16/24] bsd-user/arm/target_arch_elf.h: arm get_hwcap2 impl

[Richard Henderson]

On 10/19/21 9:44 AM, Warner Losh wrote:
> Implement the extended HW capabilities for HWCAP2.
> 
> Signed-off-by: Klye Evans<kevans@FreeBSD.org>
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/arm/target_arch_elf.h | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)

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

r~

[PATCH 17/24] bsd-user/arm/target_arch_signal.h: arm specific signal registers and stack

[Warner Losh]

Defines for registers and stack layout related to signals.

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

diff --git a/bsd-user/arm/target_arch_signal.h b/bsd-user/arm/target_arch_signal.h
new file mode 100644
index 0000000000..973183d99c
--- /dev/null
+++ b/bsd-user/arm/target_arch_signal.h
@@ -0,0 +1,57 @@
+/*
+ *  arm signal definitions
+ *
+ *  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_SIGNAL_H_
+#define _TARGET_ARCH_SIGNAL_H_
+
+#include "cpu.h"
+
+#define TARGET_REG_R0   0
+#define TARGET_REG_R1   1
+#define TARGET_REG_R2   2
+#define TARGET_REG_R3   3
+#define TARGET_REG_R4   4
+#define TARGET_REG_R5   5
+#define TARGET_REG_R6   6
+#define TARGET_REG_R7   7
+#define TARGET_REG_R8   8
+#define TARGET_REG_R9   9
+#define TARGET_REG_R10  10
+#define TARGET_REG_R11  11
+#define TARGET_REG_R12  12
+#define TARGET_REG_R13  13
+#define TARGET_REG_R14  14
+#define TARGET_REG_R15  15
+#define TARGET_REG_CPSR 16
+#define TARGET__NGREG   17
+/* Convenience synonyms */
+#define TARGET_REG_FP   TARGET_REG_R11
+#define TARGET_REG_SP   TARGET_REG_R13
+#define TARGET_REG_LR   TARGET_REG_R14
+#define TARGET_REG_PC   TARGET_REG_R15
+
+#define TARGET_INSN_SIZE    4       /* arm instruction size */
+
+/* Size of the signal trampolin code. See _sigtramp(). */
+#define TARGET_SZSIGCODE    ((abi_ulong)(9 * TARGET_INSN_SIZE))
+
+/* compare to arm/include/_limits.h */
+#define TARGET_MINSIGSTKSZ  (1024 * 4)                  /* min sig stack size */
+#define TARGET_SIGSTKSZ     (TARGET_MINSIGSTKSZ + 32768)  /* recommended size */
+
+#endif /* !_TARGET_ARCH_SIGNAL_H_ */
-- 
2.32.0

Re: [PATCH 17/24] bsd-user/arm/target_arch_signal.h: arm specific signal registers and stack

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
>
> Defines for registers and stack layout related to signals.
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/arm/target_arch_signal.h | 57 +++++++++++++++++++++++++++++++
>  1 file changed, 57 insertions(+)
>  create mode 100644 bsd-user/arm/target_arch_signal.h
>
> diff --git a/bsd-user/arm/target_arch_signal.h b/bsd-user/arm/target_arch_signal.h
> new file mode 100644
> index 0000000000..973183d99c
> --- /dev/null
> +++ b/bsd-user/arm/target_arch_signal.h
> @@ -0,0 +1,57 @@
> +/*
> + *  arm signal definitions
> + *
> + *  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_SIGNAL_H_
> +#define _TARGET_ARCH_SIGNAL_H_
> +
> +#include "cpu.h"
> +
> +#define TARGET_REG_R0   0
> +#define TARGET_REG_R1   1
> +#define TARGET_REG_R2   2
> +#define TARGET_REG_R3   3
> +#define TARGET_REG_R4   4
> +#define TARGET_REG_R5   5
> +#define TARGET_REG_R6   6
> +#define TARGET_REG_R7   7
> +#define TARGET_REG_R8   8
> +#define TARGET_REG_R9   9
> +#define TARGET_REG_R10  10
> +#define TARGET_REG_R11  11
> +#define TARGET_REG_R12  12
> +#define TARGET_REG_R13  13
> +#define TARGET_REG_R14  14
> +#define TARGET_REG_R15  15
> +#define TARGET_REG_CPSR 16
> +#define TARGET__NGREG   17
> +/* Convenience synonyms */
> +#define TARGET_REG_FP   TARGET_REG_R11
> +#define TARGET_REG_SP   TARGET_REG_R13
> +#define TARGET_REG_LR   TARGET_REG_R14
> +#define TARGET_REG_PC   TARGET_REG_R15
> +
> +#define TARGET_INSN_SIZE    4       /* arm instruction size */
> +
> +/* Size of the signal trampolin code. See _sigtramp(). */
> +#define TARGET_SZSIGCODE    ((abi_ulong)(9 * TARGET_INSN_SIZE))
> +
> +/* compare to arm/include/_limits.h */
> +#define TARGET_MINSIGSTKSZ  (1024 * 4)                  /* min sig stack size */
> +#define TARGET_SIGSTKSZ     (TARGET_MINSIGSTKSZ + 32768)  /* recommended size */
> +
> +#endif /* !_TARGET_ARCH_SIGNAL_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

Re: [PATCH 17/24] bsd-user/arm/target_arch_signal.h: arm specific signal registers and stack

[Richard Henderson]

On 10/19/21 9:44 AM, Warner Losh wrote:
> +#define TARGET_INSN_SIZE    4       /* arm instruction size */

What is this for?  arm max insn size?

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

r~

Re: [PATCH 17/24] bsd-user/arm/target_arch_signal.h: arm specific signal registers and stack

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 756 bytes --]

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

> On 10/19/21 9:44 AM, Warner Losh wrote:
> > +#define TARGET_INSN_SIZE    4       /* arm instruction size */
>
> What is this for?  arm max insn size?
>

It's used a few lines below:

+#define TARGET_SZSIGCODE    ((abi_ulong)(9 * TARGET_INSN_SIZE))

which is a fancy way of getting the sizeof the sigtramp handler that's
copied onto the stack and used in the setup.
It's then used in bsd-user/arm/target_arch_signal.h because it doesn't have
visibility to target_arch_sigtramp.h's
namespace to get the size.

It's likely a 'todo' cleanup to make this less hard-coded.

Warner


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

[-- Attachment #2: Type: text/html, Size: 1498 bytes --]

[PATCH 18/24] bsd-user/arm/target_arch_signal.h: arm machine context for signals

[Warner Losh]

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_signal.h | 36 +++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/bsd-user/arm/target_arch_signal.h b/bsd-user/arm/target_arch_signal.h
index 973183d99c..9fee58ca9c 100644
--- a/bsd-user/arm/target_arch_signal.h
+++ b/bsd-user/arm/target_arch_signal.h
@@ -54,4 +54,40 @@
 #define TARGET_MINSIGSTKSZ  (1024 * 4)                  /* min sig stack size */
 #define TARGET_SIGSTKSZ     (TARGET_MINSIGSTKSZ + 32768)  /* recommended size */
 
+/* arm/arm/machdep.c */
+struct target_sigcontext {
+    target_sigset_t sc_mask;    /* signal mask to retstore */
+    int32_t     sc_onstack;     /* sigstack state to restore */
+    abi_long    sc_pc;          /* pc at time of signal */
+    abi_long    sc_reg[32];     /* processor regs 0 to 31 */
+    abi_long    mullo, mulhi;   /* mullo and mulhi registers */
+    int32_t     sc_fpused;      /* fp has been used */
+    abi_long    sc_fpregs[33];  /* fp regs 0 to 31 & csr */
+    abi_long    sc_fpc_eir;     /* fp exception instr reg */
+    /* int32_t reserved[8]; */
+};
+
+typedef struct {
+    uint32_t    __fp_fpsr;
+    struct {
+        uint32_t    __fp_exponent;
+        uint32_t    __fp_mantissa_hi;
+        uint32_t    __fp_mantissa_lo;
+    }       __fp_fr[8];
+} target__fpregset_t;
+
+typedef struct {
+    uint32_t    __vfp_fpscr;
+    uint32_t    __vfp_fstmx[33];
+    uint32_t    __vfp_fpsid;
+} target__vfpregset_t;
+
+typedef struct target_mcontext {
+    uint32_t        __gregs[TARGET__NGREG];
+    union {
+        target__fpregset_t  __fpregs;
+        target__vfpregset_t __vfpregs;
+    } __fpu;
+} target_mcontext_t;
+
 #endif /* !_TARGET_ARCH_SIGNAL_H_ */
-- 
2.32.0

Re: [PATCH 18/24] bsd-user/arm/target_arch_signal.h: arm machine context for signals

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
>
> 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_signal.h | 36 +++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
>
> diff --git a/bsd-user/arm/target_arch_signal.h b/bsd-user/arm/target_arch_signal.h
> index 973183d99c..9fee58ca9c 100644
> --- a/bsd-user/arm/target_arch_signal.h
> +++ b/bsd-user/arm/target_arch_signal.h
> @@ -54,4 +54,40 @@
>  #define TARGET_MINSIGSTKSZ  (1024 * 4)                  /* min sig stack size */
>  #define TARGET_SIGSTKSZ     (TARGET_MINSIGSTKSZ + 32768)  /* recommended size */
>
> +/* arm/arm/machdep.c */
> +struct target_sigcontext {
> +    target_sigset_t sc_mask;    /* signal mask to retstore */
> +    int32_t     sc_onstack;     /* sigstack state to restore */
> +    abi_long    sc_pc;          /* pc at time of signal */
> +    abi_long    sc_reg[32];     /* processor regs 0 to 31 */
> +    abi_long    mullo, mulhi;   /* mullo and mulhi registers */
> +    int32_t     sc_fpused;      /* fp has been used */
> +    abi_long    sc_fpregs[33];  /* fp regs 0 to 31 & csr */
> +    abi_long    sc_fpc_eir;     /* fp exception instr reg */
> +    /* int32_t reserved[8]; */
> +};
> +
> +typedef struct {
> +    uint32_t    __fp_fpsr;
> +    struct {
> +        uint32_t    __fp_exponent;
> +        uint32_t    __fp_mantissa_hi;
> +        uint32_t    __fp_mantissa_lo;
> +    }       __fp_fr[8];
> +} target__fpregset_t;
> +
> +typedef struct {
> +    uint32_t    __vfp_fpscr;
> +    uint32_t    __vfp_fstmx[33];
> +    uint32_t    __vfp_fpsid;
> +} target__vfpregset_t;
> +
> +typedef struct target_mcontext {
> +    uint32_t        __gregs[TARGET__NGREG];
> +    union {
> +        target__fpregset_t  __fpregs;
> +        target__vfpregset_t __vfpregs;
> +    } __fpu;
> +} target_mcontext_t;
> +
>  #endif /* !_TARGET_ARCH_SIGNAL_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

Re: [PATCH 18/24] bsd-user/arm/target_arch_signal.h: arm machine context for signals

[Richard Henderson]

On 10/19/21 9:44 AM, Warner Losh wrote:
> +/* arm/arm/machdep.c */
> +struct target_sigcontext {
> +    target_sigset_t sc_mask;    /* signal mask to retstore */
> +    int32_t     sc_onstack;     /* sigstack state to restore */
> +    abi_long    sc_pc;          /* pc at time of signal */
> +    abi_long    sc_reg[32];     /* processor regs 0 to 31 */
> +    abi_long    mullo, mulhi;   /* mullo and mulhi registers */
> +    int32_t     sc_fpused;      /* fp has been used */
> +    abi_long    sc_fpregs[33];  /* fp regs 0 to 31 & csr */
> +    abi_long    sc_fpc_eir;     /* fp exception instr reg */
> +    /* int32_t reserved[8]; */
> +};

 From the set of registers, this seems to be copied from mips.

The contents of sys/arm/include/signal.h are

struct sigcontext {
         int _dummy;
};

so I'm not sure what you're doing here.


r~

Re: [PATCH 18/24] bsd-user/arm/target_arch_signal.h: arm machine context for signals

[Richard Henderson]

On 10/19/21 9:44 AM, Warner Losh wrote:
> +typedef struct {
> +    uint32_t    __fp_fpsr;
> +    struct {
> +        uint32_t    __fp_exponent;
> +        uint32_t    __fp_mantissa_hi;
> +        uint32_t    __fp_mantissa_lo;
> +    }       __fp_fr[8];
> +} target__fpregset_t;
> +
> +typedef struct {
> +    uint32_t    __vfp_fpscr;
> +    uint32_t    __vfp_fstmx[33];
> +    uint32_t    __vfp_fpsid;
> +} target__vfpregset_t;
> +
> +typedef struct target_mcontext {
> +    uint32_t        __gregs[TARGET__NGREG];
> +    union {
> +        target__fpregset_t  __fpregs;
> +        target__vfpregset_t __vfpregs;
> +    } __fpu;
> +} target_mcontext_t;

This doesn't match what I see in sys/arm/include/ucontext.h at all.
I don't even see the string "fstmx" anywhere in the source tree.


r~

Re: [PATCH 18/24] bsd-user/arm/target_arch_signal.h: arm machine context for signals

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 1411 bytes --]

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

> On 10/19/21 9:44 AM, Warner Losh wrote:
> > +typedef struct {
> > +    uint32_t    __fp_fpsr;
> > +    struct {
> > +        uint32_t    __fp_exponent;
> > +        uint32_t    __fp_mantissa_hi;
> > +        uint32_t    __fp_mantissa_lo;
> > +    }       __fp_fr[8];
> > +} target__fpregset_t;
> > +
> > +typedef struct {
> > +    uint32_t    __vfp_fpscr;
> > +    uint32_t    __vfp_fstmx[33];
> > +    uint32_t    __vfp_fpsid;
> > +} target__vfpregset_t;
> > +
> > +typedef struct target_mcontext {
> > +    uint32_t        __gregs[TARGET__NGREG];
> > +    union {
> > +        target__fpregset_t  __fpregs;
> > +        target__vfpregset_t __vfpregs;
> > +    } __fpu;
> > +} target_mcontext_t;
>
> This doesn't match what I see in sys/arm/include/ucontext.h at all.
> I don't even see the string "fstmx" anywhere in the source tree.
>

Yea. This appears to be a copy mostly from mips and is totally
wrong for ARM. However, despite that, it works by accident because
the general registers all wind up in the right places and we don't
actually write to the full context....

I'll respin with the correct headers and any code tweaks that
are needed. As to 'how this happened' I'm at a loss: git blame
shows that it came in when the sbruno repo was created which
is the earliest history I have for the project.

Warner

[-- Attachment #2: Type: text/html, Size: 2097 bytes --]

[PATCH 19/24] bsd-user/arm/target_arch_signal.h: arm user context and trapframe for signals

[Warner Losh]

Arm specific user context structures for signal handling and the closely
related trap frame.

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/arm/target_arch_signal.h | 38 +++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/bsd-user/arm/target_arch_signal.h b/bsd-user/arm/target_arch_signal.h
index 9fee58ca9c..67355ff28f 100644
--- a/bsd-user/arm/target_arch_signal.h
+++ b/bsd-user/arm/target_arch_signal.h
@@ -90,4 +90,42 @@ typedef struct target_mcontext {
     } __fpu;
 } target_mcontext_t;
 
+typedef struct target_ucontext {
+    target_sigset_t     uc_sigmask;
+    target_mcontext_t   uc_mcontext;
+    abi_ulong           uc_link;
+    target_stack_t      uc_stack;
+    int32_t             uc_flags;
+    int32_t             __spare__[4];
+} target_ucontext_t;
+
+struct target_sigframe {
+    target_siginfo_t    sf_si;  /* saved siginfo */
+    target_ucontext_t   sf_uc;  /* saved ucontext */
+};
+
+
+/* compare to sys/arm/include/frame.h */
+struct target_trapframe {
+    abi_ulong tf_spsr; /* Zero on arm26 */
+    abi_ulong tf_r0;
+    abi_ulong tf_r1;
+    abi_ulong tf_r2;
+    abi_ulong tf_r3;
+    abi_ulong tf_r4;
+    abi_ulong tf_r5;
+    abi_ulong tf_r6;
+    abi_ulong tf_r7;
+    abi_ulong tf_r8;
+    abi_ulong tf_r9;
+    abi_ulong tf_r10;
+    abi_ulong tf_r11;
+    abi_ulong tf_r12;
+    abi_ulong tf_usr_sp;
+    abi_ulong tf_usr_lr;
+    abi_ulong tf_svc_sp; /* Not used on arm26 */
+    abi_ulong tf_svc_lr; /* Not used on arm26 */
+    abi_ulong tf_pc;
+};
+
 #endif /* !_TARGET_ARCH_SIGNAL_H_ */
-- 
2.32.0

Re: [PATCH 19/24] bsd-user/arm/target_arch_signal.h: arm user context and trapframe for signals

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
>
> Arm specific user context structures for signal handling and the closely
> related trap frame.
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/arm/target_arch_signal.h | 38 +++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>
> diff --git a/bsd-user/arm/target_arch_signal.h b/bsd-user/arm/target_arch_signal.h
> index 9fee58ca9c..67355ff28f 100644
> --- a/bsd-user/arm/target_arch_signal.h
> +++ b/bsd-user/arm/target_arch_signal.h
> @@ -90,4 +90,42 @@ typedef struct target_mcontext {
>      } __fpu;
>  } target_mcontext_t;
>
> +typedef struct target_ucontext {
> +    target_sigset_t     uc_sigmask;
> +    target_mcontext_t   uc_mcontext;
> +    abi_ulong           uc_link;
> +    target_stack_t      uc_stack;
> +    int32_t             uc_flags;
> +    int32_t             __spare__[4];
> +} target_ucontext_t;
> +
> +struct target_sigframe {
> +    target_siginfo_t    sf_si;  /* saved siginfo */
> +    target_ucontext_t   sf_uc;  /* saved ucontext */
> +};
> +
> +

We might be able to kill this extra newline? I'm not familiar enough
with qemu's style preferences here...

> +/* compare to sys/arm/include/frame.h */
> +struct target_trapframe {
> +    abi_ulong tf_spsr; /* Zero on arm26 */
> +    abi_ulong tf_r0;
> +    abi_ulong tf_r1;
> +    abi_ulong tf_r2;
> +    abi_ulong tf_r3;
> +    abi_ulong tf_r4;
> +    abi_ulong tf_r5;
> +    abi_ulong tf_r6;
> +    abi_ulong tf_r7;
> +    abi_ulong tf_r8;
> +    abi_ulong tf_r9;
> +    abi_ulong tf_r10;
> +    abi_ulong tf_r11;
> +    abi_ulong tf_r12;
> +    abi_ulong tf_usr_sp;
> +    abi_ulong tf_usr_lr;
> +    abi_ulong tf_svc_sp; /* Not used on arm26 */
> +    abi_ulong tf_svc_lr; /* Not used on arm26 */
> +    abi_ulong tf_pc;
> +};
> +
>  #endif /* !_TARGET_ARCH_SIGNAL_H_ */
> --
> 2.32.0
>

I didn't think we actually supported arm26, but I see those comments
also exist verbatim in machine/frame.h; no objection to reflecting
them here, as well.

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

Re: [PATCH 19/24] bsd-user/arm/target_arch_signal.h: arm user context and trapframe for signals

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 2474 bytes --]

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

> On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
> >
> > Arm specific user context structures for signal handling and the closely
> > related trap frame.
> >
> > Signed-off-by: Stacey Son <sson@FreeBSD.org>
> > Signed-off-by: Warner Losh <imp@bsdimp.com>
> > ---
> >  bsd-user/arm/target_arch_signal.h | 38 +++++++++++++++++++++++++++++++
> >  1 file changed, 38 insertions(+)
> >
> > diff --git a/bsd-user/arm/target_arch_signal.h
> b/bsd-user/arm/target_arch_signal.h
> > index 9fee58ca9c..67355ff28f 100644
> > --- a/bsd-user/arm/target_arch_signal.h
> > +++ b/bsd-user/arm/target_arch_signal.h
> > @@ -90,4 +90,42 @@ typedef struct target_mcontext {
> >      } __fpu;
> >  } target_mcontext_t;
> >
> > +typedef struct target_ucontext {
> > +    target_sigset_t     uc_sigmask;
> > +    target_mcontext_t   uc_mcontext;
> > +    abi_ulong           uc_link;
> > +    target_stack_t      uc_stack;
> > +    int32_t             uc_flags;
> > +    int32_t             __spare__[4];
> > +} target_ucontext_t;
> > +
> > +struct target_sigframe {
> > +    target_siginfo_t    sf_si;  /* saved siginfo */
> > +    target_ucontext_t   sf_uc;  /* saved ucontext */
> > +};
> > +
> > +
>
> We might be able to kill this extra newline? I'm not familiar enough
> with qemu's style preferences here...
>

done.


> > +/* compare to sys/arm/include/frame.h */
> > +struct target_trapframe {
> > +    abi_ulong tf_spsr; /* Zero on arm26 */
> > +    abi_ulong tf_r0;
> > +    abi_ulong tf_r1;
> > +    abi_ulong tf_r2;
> > +    abi_ulong tf_r3;
> > +    abi_ulong tf_r4;
> > +    abi_ulong tf_r5;
> > +    abi_ulong tf_r6;
> > +    abi_ulong tf_r7;
> > +    abi_ulong tf_r8;
> > +    abi_ulong tf_r9;
> > +    abi_ulong tf_r10;
> > +    abi_ulong tf_r11;
> > +    abi_ulong tf_r12;
> > +    abi_ulong tf_usr_sp;
> > +    abi_ulong tf_usr_lr;
> > +    abi_ulong tf_svc_sp; /* Not used on arm26 */
> > +    abi_ulong tf_svc_lr; /* Not used on arm26 */
> > +    abi_ulong tf_pc;
> > +};
> > +
> >  #endif /* !_TARGET_ARCH_SIGNAL_H_ */
> > --
> > 2.32.0
> >
>
> I didn't think we actually supported arm26, but I see those comments
> also exist verbatim in machine/frame.h; no objection to reflecting
> them here, as well.
>

Removed. I'll prep a commit for the fork too and remove them from FreeBSD
since they have never been relevant.


> Reviewed-by: Kyle Evans <kevans@FreeBSD.org>
>

Thanks!

[-- Attachment #2: Type: text/html, Size: 3771 bytes --]

Re: [PATCH 19/24] bsd-user/arm/target_arch_signal.h: arm user context and trapframe for signals

[Richard Henderson]

On 10/19/21 9:44 AM, Warner Losh wrote:
> Arm specific user context structures for signal handling and the closely
> related trap frame.
> 
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>   bsd-user/arm/target_arch_signal.h | 38 +++++++++++++++++++++++++++++++
>   1 file changed, 38 insertions(+)
> 
> diff --git a/bsd-user/arm/target_arch_signal.h b/bsd-user/arm/target_arch_signal.h
> index 9fee58ca9c..67355ff28f 100644
> --- a/bsd-user/arm/target_arch_signal.h
> +++ b/bsd-user/arm/target_arch_signal.h
> @@ -90,4 +90,42 @@ typedef struct target_mcontext {
>       } __fpu;
>   } target_mcontext_t;
>   
> +typedef struct target_ucontext {
> +    target_sigset_t     uc_sigmask;
> +    target_mcontext_t   uc_mcontext;
> +    abi_ulong           uc_link;
> +    target_stack_t      uc_stack;
> +    int32_t             uc_flags;
> +    int32_t             __spare__[4];
> +} target_ucontext_t;

This is from the common <sys/_ucontext.h>.  Is there any way we can share that here in 
qemu as well?

> +struct target_sigframe {
> +    target_siginfo_t    sf_si;  /* saved siginfo */
> +    target_ucontext_t   sf_uc;  /* saved ucontext */
> +};

You're missing the storage for

   mcontext_vfp_t  sf_vfp

the address of which goes in mcontext_t.mc_vfp_ptr (which isn't in the version of 
mcontext_t from patch 18).

> +/* compare to sys/arm/include/frame.h */
> +struct target_trapframe {
> +    abi_ulong tf_spsr; /* Zero on arm26 */
> +    abi_ulong tf_r0;
> +    abi_ulong tf_r1;
> +    abi_ulong tf_r2;
> +    abi_ulong tf_r3;
> +    abi_ulong tf_r4;
> +    abi_ulong tf_r5;
> +    abi_ulong tf_r6;
> +    abi_ulong tf_r7;
> +    abi_ulong tf_r8;
> +    abi_ulong tf_r9;
> +    abi_ulong tf_r10;
> +    abi_ulong tf_r11;
> +    abi_ulong tf_r12;
> +    abi_ulong tf_usr_sp;
> +    abi_ulong tf_usr_lr;
> +    abi_ulong tf_svc_sp; /* Not used on arm26 */
> +    abi_ulong tf_svc_lr; /* Not used on arm26 */
> +    abi_ulong tf_pc;
> +};

This is not used by the user abi for signals.  It is a kernel structure for its own 
internal trap handling.  Why are you including it?


r~

Re: [PATCH 19/24] bsd-user/arm/target_arch_signal.h: arm user context and trapframe for signals

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 2959 bytes --]

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

> On 10/19/21 9:44 AM, Warner Losh wrote:
> > Arm specific user context structures for signal handling and the closely
> > related trap frame.
> >
> > Signed-off-by: Stacey Son <sson@FreeBSD.org>
> > Signed-off-by: Warner Losh <imp@bsdimp.com>
> > ---
> >   bsd-user/arm/target_arch_signal.h | 38 +++++++++++++++++++++++++++++++
> >   1 file changed, 38 insertions(+)
> >
> > diff --git a/bsd-user/arm/target_arch_signal.h
> b/bsd-user/arm/target_arch_signal.h
> > index 9fee58ca9c..67355ff28f 100644
> > --- a/bsd-user/arm/target_arch_signal.h
> > +++ b/bsd-user/arm/target_arch_signal.h
> > @@ -90,4 +90,42 @@ typedef struct target_mcontext {
> >       } __fpu;
> >   } target_mcontext_t;
> >
> > +typedef struct target_ucontext {
> > +    target_sigset_t     uc_sigmask;
> > +    target_mcontext_t   uc_mcontext;
> > +    abi_ulong           uc_link;
> > +    target_stack_t      uc_stack;
> > +    int32_t             uc_flags;
> > +    int32_t             __spare__[4];
> > +} target_ucontext_t;
>
> This is from the common <sys/_ucontext.h>.  Is there any way we can share
> that here in
> qemu as well?
>

That's a good idea.


> > +struct target_sigframe {
> > +    target_siginfo_t    sf_si;  /* saved siginfo */
> > +    target_ucontext_t   sf_uc;  /* saved ucontext */
> > +};
>
> You're missing the storage for
>    mcontext_vfp_t  sf_vfp
>
the address of which goes in mcontext_t.mc_vfp_ptr


Added.


> (which isn't in the version of
> mcontext_t from patch 18).
>

Added to my revised patch 18.


> > +/* compare to sys/arm/include/frame.h */
> > +struct target_trapframe {
> > +    abi_ulong tf_spsr; /* Zero on arm26 */
> > +    abi_ulong tf_r0;
> > +    abi_ulong tf_r1;
> > +    abi_ulong tf_r2;
> > +    abi_ulong tf_r3;
> > +    abi_ulong tf_r4;
> > +    abi_ulong tf_r5;
> > +    abi_ulong tf_r6;
> > +    abi_ulong tf_r7;
> > +    abi_ulong tf_r8;
> > +    abi_ulong tf_r9;
> > +    abi_ulong tf_r10;
> > +    abi_ulong tf_r11;
> > +    abi_ulong tf_r12;
> > +    abi_ulong tf_usr_sp;
> > +    abi_ulong tf_usr_lr;
> > +    abi_ulong tf_svc_sp; /* Not used on arm26 */
> > +    abi_ulong tf_svc_lr; /* Not used on arm26 */
> > +    abi_ulong tf_pc;
> > +};
>
> This is not used by the user abi for signals.  It is a kernel structure
> for its own
> internal trap handling.  Why are you including it?
>

This code pre-dates my time... But I think it was included because the code
in target_arch_thread.h in patch 13 took its size.  But it turns out that's
bogus too. It
accidentally gets things aligned correctly, but should just be removed with
this struct as well. The kernel doesn't use it.

What's worse, ppc, aarch64 and riscv all copied this bogus trope :(. At
least I'll have a chance to fix it before you see it...

I've updated x86 that's upstream in my patch series as well... I'll send it
out Monday or maybe over the weekend.

Warner

[-- Attachment #2: Type: text/html, Size: 4633 bytes --]

[PATCH 20/24] bsd-user/arm/target_arch_signal.h: arm set_sigtramp_args

[Warner Losh]

Implement set_sigtramp_args to setup the arguments to the sigtramp
calls.

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/arm/target_arch_signal.h | 35 +++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/bsd-user/arm/target_arch_signal.h b/bsd-user/arm/target_arch_signal.h
index 67355ff28f..b421c2522c 100644
--- a/bsd-user/arm/target_arch_signal.h
+++ b/bsd-user/arm/target_arch_signal.h
@@ -128,4 +128,39 @@ struct target_trapframe {
     abi_ulong tf_pc;
 };
 
+/*
+ * Compare to arm/arm/machdep.c sendsig()
+ * Assumes that target stack frame memory is locked.
+ */
+static inline abi_long
+set_sigtramp_args(CPUARMState *regs, int sig, struct target_sigframe *frame,
+    abi_ulong frame_addr, struct target_sigaction *ka)
+{
+    /*
+     * Arguments to signal handler:
+     *  r0 = signal number
+     *  r1 = siginfo pointer
+     *  r2 = ucontext pointer
+     *  r5 = ucontext pointer
+     *  pc = signal handler pointer
+     *  sp = sigframe struct pointer
+     *  lr = sigtramp at base of user stack
+     */
+
+    regs->regs[0] = sig;
+    regs->regs[1] = frame_addr +
+        offsetof(struct target_sigframe, sf_si);
+    regs->regs[2] = frame_addr +
+        offsetof(struct target_sigframe, sf_uc);
+
+    /* the trampoline uses r5 as the uc address */
+    regs->regs[5] = frame_addr +
+        offsetof(struct target_sigframe, sf_uc);
+    regs->regs[TARGET_REG_PC] = ka->_sa_handler;
+    regs->regs[TARGET_REG_SP] = frame_addr;
+    regs->regs[TARGET_REG_LR] = TARGET_PS_STRINGS - TARGET_SZSIGCODE;
+
+    return 0;
+}
+
 #endif /* !_TARGET_ARCH_SIGNAL_H_ */
-- 
2.32.0

Re: [PATCH 20/24] bsd-user/arm/target_arch_signal.h: arm set_sigtramp_args

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
>
> Implement set_sigtramp_args to setup the arguments to the sigtramp
> calls.
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/arm/target_arch_signal.h | 35 +++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>
> diff --git a/bsd-user/arm/target_arch_signal.h b/bsd-user/arm/target_arch_signal.h
> index 67355ff28f..b421c2522c 100644
> --- a/bsd-user/arm/target_arch_signal.h
> +++ b/bsd-user/arm/target_arch_signal.h
> @@ -128,4 +128,39 @@ struct target_trapframe {
>      abi_ulong tf_pc;
>  };
>
> +/*
> + * Compare to arm/arm/machdep.c sendsig()
> + * Assumes that target stack frame memory is locked.
> + */
> +static inline abi_long
> +set_sigtramp_args(CPUARMState *regs, int sig, struct target_sigframe *frame,
> +    abi_ulong frame_addr, struct target_sigaction *ka)
> +{
> +    /*
> +     * Arguments to signal handler:
> +     *  r0 = signal number
> +     *  r1 = siginfo pointer
> +     *  r2 = ucontext pointer
> +     *  r5 = ucontext pointer
> +     *  pc = signal handler pointer
> +     *  sp = sigframe struct pointer
> +     *  lr = sigtramp at base of user stack
> +     */
> +
> +    regs->regs[0] = sig;
> +    regs->regs[1] = frame_addr +
> +        offsetof(struct target_sigframe, sf_si);
> +    regs->regs[2] = frame_addr +
> +        offsetof(struct target_sigframe, sf_uc);
> +
> +    /* the trampoline uses r5 as the uc address */
> +    regs->regs[5] = frame_addr +
> +        offsetof(struct target_sigframe, sf_uc);
> +    regs->regs[TARGET_REG_PC] = ka->_sa_handler;
> +    regs->regs[TARGET_REG_SP] = frame_addr;
> +    regs->regs[TARGET_REG_LR] = TARGET_PS_STRINGS - TARGET_SZSIGCODE;
> +
> +    return 0;
> +}
> +
>  #endif /* !_TARGET_ARCH_SIGNAL_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

Re: [PATCH 20/24] bsd-user/arm/target_arch_signal.h: arm set_sigtramp_args

[Richard Henderson]

On 10/19/21 9:44 AM, Warner Losh wrote:
> +    regs->regs[TARGET_REG_PC] = ka->_sa_handler;

Surely there should be some handling of thumb addresses here.


r~

Re: [PATCH 20/24] bsd-user/arm/target_arch_signal.h: arm set_sigtramp_args

[Kyle Evans]

On Thu, Oct 28, 2021 at 12:25 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 10/19/21 9:44 AM, Warner Losh wrote:
> > +    regs->regs[TARGET_REG_PC] = ka->_sa_handler;
>
> Surely there should be some handling of thumb addresses here.
>

Honestly, this wouldn't surprise me- we're kind of a thumb landmine.
The other thumb-ish support is relatively recent as we hit something
that had an entry point on a thumb instruction; but we don't typically
build/run binaries including thumb instructions.

Re: [PATCH 20/24] bsd-user/arm/target_arch_signal.h: arm set_sigtramp_args

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 697 bytes --]

On Thu, Oct 28, 2021 at 11:35 AM Kyle Evans <kevans@freebsd.org> wrote:

> On Thu, Oct 28, 2021 at 12:25 PM Richard Henderson
> <richard.henderson@linaro.org> wrote:
> >
> > On 10/19/21 9:44 AM, Warner Losh wrote:
> > > +    regs->regs[TARGET_REG_PC] = ka->_sa_handler;
> >
> > Surely there should be some handling of thumb addresses here.
> >
>
> Honestly, this wouldn't surprise me- we're kind of a thumb landmine.
> The other thumb-ish support is relatively recent as we hit something
> that had an entry point on a thumb instruction; but we don't typically
> build/run binaries including thumb instructions.
>

Yes. We don't have good, extensive test cases for this. We likely should.

Warner

[-- Attachment #2: Type: text/html, Size: 1203 bytes --]

Re: [PATCH 20/24] bsd-user/arm/target_arch_signal.h: arm set_sigtramp_args

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 502 bytes --]

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

> On 10/19/21 9:44 AM, Warner Losh wrote:
> > +    regs->regs[TARGET_REG_PC] = ka->_sa_handler;
>
> Surely there should be some handling of thumb addresses here.
>

Yes. I've added code to do this, but haven't run it through (a) our normal
test suite and (b) don't think I have any code that's normal mode with a
thumb mode signal handler to test it with... It will be in the next
iteration
I send out.

Warner

[-- Attachment #2: Type: text/html, Size: 932 bytes --]

Re: [PATCH 20/24] bsd-user/arm/target_arch_signal.h: arm set_sigtramp_args

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 1041 bytes --]

On Thu, Oct 28, 2021 at 4:41 PM Warner Losh <imp@bsdimp.com> wrote:

>
>
> On Thu, Oct 28, 2021 at 11:25 AM Richard Henderson <
> richard.henderson@linaro.org> wrote:
>
>> On 10/19/21 9:44 AM, Warner Losh wrote:
>> > +    regs->regs[TARGET_REG_PC] = ka->_sa_handler;
>>
>> Surely there should be some handling of thumb addresses here.
>>
>
> Yes. I've added code to do this, but haven't run it through (a) our normal
> test suite and (b) don't think I have any code that's normal mode with a
> thumb mode signal handler to test it with... It will be in the next
> iteration
> I send out.
>

FYI: I've fixed it in  the patch stream. I wrote 4 test programs: one in
a32 mode,
one in T32 mode, one starts in A32 and gets a signal in T32 mode and a final
one that starts in T32 and signals in A32 mode. They all work now. Before
all but the A32 one failed.

And the test suite runs about the same as before. I'll have to add these 4
programs
to the test suite :)... So good call. Thanks on this and the other
difficult ones you
flagged.

Warner

[-- Attachment #2: Type: text/html, Size: 1872 bytes --]

[PATCH 21/24] bsd-user/arm/target_arch_signal.h: arm get_mcontext

[Warner Losh]

Get the machine context from the CPU state.

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_signal.h | 38 +++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/bsd-user/arm/target_arch_signal.h b/bsd-user/arm/target_arch_signal.h
index b421c2522c..302fdc2846 100644
--- a/bsd-user/arm/target_arch_signal.h
+++ b/bsd-user/arm/target_arch_signal.h
@@ -163,4 +163,42 @@ set_sigtramp_args(CPUARMState *regs, int sig, struct target_sigframe *frame,
     return 0;
 }
 
+/*
+ * Compare to arm/arm/machdep.c get_mcontext()
+ * Assumes that the memory is locked if mcp points to user memory.
+ */
+static inline abi_long get_mcontext(CPUARMState *regs, target_mcontext_t *mcp,
+        int flags)
+{
+    int err = 0;
+    uint32_t *gr = mcp->__gregs;
+
+    gr[TARGET_REG_CPSR] = tswap32(cpsr_read(regs));
+    if (flags & TARGET_MC_GET_CLEAR_RET) {
+        gr[TARGET_REG_R0] = 0;
+        gr[TARGET_REG_CPSR] &= ~CPSR_C;
+    } else {
+        gr[TARGET_REG_R0] = tswap32(regs->regs[0]);
+    }
+
+    gr[TARGET_REG_R1] = tswap32(regs->regs[1]);
+    gr[TARGET_REG_R2] = tswap32(regs->regs[2]);
+    gr[TARGET_REG_R3] = tswap32(regs->regs[3]);
+    gr[TARGET_REG_R4] = tswap32(regs->regs[4]);
+    gr[TARGET_REG_R5] = tswap32(regs->regs[5]);
+    gr[TARGET_REG_R6] = tswap32(regs->regs[6]);
+    gr[TARGET_REG_R7] = tswap32(regs->regs[7]);
+    gr[TARGET_REG_R8] = tswap32(regs->regs[8]);
+    gr[TARGET_REG_R9] = tswap32(regs->regs[9]);
+    gr[TARGET_REG_R10] = tswap32(regs->regs[10]);
+    gr[TARGET_REG_R11] = tswap32(regs->regs[11]);
+    gr[TARGET_REG_R12] = tswap32(regs->regs[12]);
+
+    gr[TARGET_REG_SP] = tswap32(regs->regs[13]);
+    gr[TARGET_REG_LR] = tswap32(regs->regs[14]);
+    gr[TARGET_REG_PC] = tswap32(regs->regs[15]);
+
+    return err;
+}
+
 #endif /* !_TARGET_ARCH_SIGNAL_H_ */
-- 
2.32.0

Re: [PATCH 21/24] bsd-user/arm/target_arch_signal.h: arm get_mcontext

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
>
> Get the machine context from the CPU state.
>
> 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_signal.h | 38 +++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
>
> diff --git a/bsd-user/arm/target_arch_signal.h b/bsd-user/arm/target_arch_signal.h
> index b421c2522c..302fdc2846 100644
> --- a/bsd-user/arm/target_arch_signal.h
> +++ b/bsd-user/arm/target_arch_signal.h
> @@ -163,4 +163,42 @@ set_sigtramp_args(CPUARMState *regs, int sig, struct target_sigframe *frame,
>      return 0;
>  }
>
> +/*
> + * Compare to arm/arm/machdep.c get_mcontext()
> + * Assumes that the memory is locked if mcp points to user memory.
> + */
> +static inline abi_long get_mcontext(CPUARMState *regs, target_mcontext_t *mcp,
> +        int flags)
> +{
> +    int err = 0;
> +    uint32_t *gr = mcp->__gregs;
> +
> +    gr[TARGET_REG_CPSR] = tswap32(cpsr_read(regs));
> +    if (flags & TARGET_MC_GET_CLEAR_RET) {
> +        gr[TARGET_REG_R0] = 0;
> +        gr[TARGET_REG_CPSR] &= ~CPSR_C;
> +    } else {
> +        gr[TARGET_REG_R0] = tswap32(regs->regs[0]);
> +    }
> +
> +    gr[TARGET_REG_R1] = tswap32(regs->regs[1]);
> +    gr[TARGET_REG_R2] = tswap32(regs->regs[2]);
> +    gr[TARGET_REG_R3] = tswap32(regs->regs[3]);
> +    gr[TARGET_REG_R4] = tswap32(regs->regs[4]);
> +    gr[TARGET_REG_R5] = tswap32(regs->regs[5]);
> +    gr[TARGET_REG_R6] = tswap32(regs->regs[6]);
> +    gr[TARGET_REG_R7] = tswap32(regs->regs[7]);
> +    gr[TARGET_REG_R8] = tswap32(regs->regs[8]);
> +    gr[TARGET_REG_R9] = tswap32(regs->regs[9]);
> +    gr[TARGET_REG_R10] = tswap32(regs->regs[10]);
> +    gr[TARGET_REG_R11] = tswap32(regs->regs[11]);
> +    gr[TARGET_REG_R12] = tswap32(regs->regs[12]);
> +
> +    gr[TARGET_REG_SP] = tswap32(regs->regs[13]);
> +    gr[TARGET_REG_LR] = tswap32(regs->regs[14]);
> +    gr[TARGET_REG_PC] = tswap32(regs->regs[15]);
> +
> +    return err;
> +}
> +
>  #endif /* !_TARGET_ARCH_SIGNAL_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

Re: [PATCH 21/24] bsd-user/arm/target_arch_signal.h: arm get_mcontext

[Richard Henderson]

On 10/19/21 9:44 AM, Warner Losh wrote:
> Get the machine context from the CPU state.
> 
> 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_signal.h | 38 +++++++++++++++++++++++++++++++
>   1 file changed, 38 insertions(+)

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

r~

[PATCH 22/24] bsd-user/arm/target_arch_signal.h: arm set_mcontext

[Warner Losh]

Move the machine context to the CPU state.

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_signal.h | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/bsd-user/arm/target_arch_signal.h b/bsd-user/arm/target_arch_signal.h
index 302fdc2846..1d051af9ae 100644
--- a/bsd-user/arm/target_arch_signal.h
+++ b/bsd-user/arm/target_arch_signal.h
@@ -201,4 +201,35 @@ static inline abi_long get_mcontext(CPUARMState *regs, target_mcontext_t *mcp,
     return err;
 }
 
+/* Compare to arm/arm/machdep.c set_mcontext() */
+static inline abi_long set_mcontext(CPUARMState *regs, target_mcontext_t *mcp,
+        int srflag)
+{
+    int err = 0;
+    const uint32_t *gr = mcp->__gregs;
+    uint32_t cpsr;
+
+    regs->regs[0] = tswap32(gr[TARGET_REG_R0]);
+    regs->regs[1] = tswap32(gr[TARGET_REG_R1]);
+    regs->regs[2] = tswap32(gr[TARGET_REG_R2]);
+    regs->regs[3] = tswap32(gr[TARGET_REG_R3]);
+    regs->regs[4] = tswap32(gr[TARGET_REG_R4]);
+    regs->regs[5] = tswap32(gr[TARGET_REG_R5]);
+    regs->regs[6] = tswap32(gr[TARGET_REG_R6]);
+    regs->regs[7] = tswap32(gr[TARGET_REG_R7]);
+    regs->regs[8] = tswap32(gr[TARGET_REG_R8]);
+    regs->regs[9] = tswap32(gr[TARGET_REG_R9]);
+    regs->regs[10] = tswap32(gr[TARGET_REG_R10]);
+    regs->regs[11] = tswap32(gr[TARGET_REG_R11]);
+    regs->regs[12] = tswap32(gr[TARGET_REG_R12]);
+
+    regs->regs[13] = tswap32(gr[TARGET_REG_SP]);
+    regs->regs[14] = tswap32(gr[TARGET_REG_LR]);
+    regs->regs[15] = tswap32(gr[TARGET_REG_PC]);
+    cpsr = tswap32(gr[TARGET_REG_CPSR]);
+    cpsr_write(regs, cpsr, CPSR_USER | CPSR_EXEC, CPSRWriteByInstr);
+
+    return err;
+}
+
 #endif /* !_TARGET_ARCH_SIGNAL_H_ */
-- 
2.32.0

Re: [PATCH 22/24] bsd-user/arm/target_arch_signal.h: arm set_mcontext

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
>
> Move the machine context to the CPU state.
>
> 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_signal.h | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>
> diff --git a/bsd-user/arm/target_arch_signal.h b/bsd-user/arm/target_arch_signal.h
> index 302fdc2846..1d051af9ae 100644
> --- a/bsd-user/arm/target_arch_signal.h
> +++ b/bsd-user/arm/target_arch_signal.h
> @@ -201,4 +201,35 @@ static inline abi_long get_mcontext(CPUARMState *regs, target_mcontext_t *mcp,
>      return err;
>  }
>
> +/* Compare to arm/arm/machdep.c set_mcontext() */
> +static inline abi_long set_mcontext(CPUARMState *regs, target_mcontext_t *mcp,
> +        int srflag)
> +{
> +    int err = 0;
> +    const uint32_t *gr = mcp->__gregs;
> +    uint32_t cpsr;
> +
> +    regs->regs[0] = tswap32(gr[TARGET_REG_R0]);
> +    regs->regs[1] = tswap32(gr[TARGET_REG_R1]);
> +    regs->regs[2] = tswap32(gr[TARGET_REG_R2]);
> +    regs->regs[3] = tswap32(gr[TARGET_REG_R3]);
> +    regs->regs[4] = tswap32(gr[TARGET_REG_R4]);
> +    regs->regs[5] = tswap32(gr[TARGET_REG_R5]);
> +    regs->regs[6] = tswap32(gr[TARGET_REG_R6]);
> +    regs->regs[7] = tswap32(gr[TARGET_REG_R7]);
> +    regs->regs[8] = tswap32(gr[TARGET_REG_R8]);
> +    regs->regs[9] = tswap32(gr[TARGET_REG_R9]);
> +    regs->regs[10] = tswap32(gr[TARGET_REG_R10]);
> +    regs->regs[11] = tswap32(gr[TARGET_REG_R11]);
> +    regs->regs[12] = tswap32(gr[TARGET_REG_R12]);
> +
> +    regs->regs[13] = tswap32(gr[TARGET_REG_SP]);
> +    regs->regs[14] = tswap32(gr[TARGET_REG_LR]);
> +    regs->regs[15] = tswap32(gr[TARGET_REG_PC]);
> +    cpsr = tswap32(gr[TARGET_REG_CPSR]);
> +    cpsr_write(regs, cpsr, CPSR_USER | CPSR_EXEC, CPSRWriteByInstr);
> +
> +    return err;
> +}
> +
>  #endif /* !_TARGET_ARCH_SIGNAL_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

Re: [PATCH 22/24] bsd-user/arm/target_arch_signal.h: arm set_mcontext

[Richard Henderson]

On 10/19/21 9:44 AM, Warner Losh wrote:
> +    regs->regs[15] = tswap32(gr[TARGET_REG_PC]);
> +    cpsr = tswap32(gr[TARGET_REG_CPSR]);
> +    cpsr_write(regs, cpsr, CPSR_USER | CPSR_EXEC, CPSRWriteByInstr);

Hmm.  What's the expected behaviour if the saved CPSR state does not match the PC state 
wrt thumb?

I'm ok if this should fail in spectacular ways, I just wanna know.

I *think* what will happen at the moment is that qemu will go into a whacky state in which 
the translator will read and interpret unaligned data.

I have a pending patch set for arm to raise unaligned exceptions for mis-aligned pc.  For 
arm32 mode, this is fine, and we'll raise the exception.  But for thumb mode, this is 
architecturally impossible, and the translator will assert.

The assert is going to be a problem.  There are a couple of options:

(1) TARGET_REG_PC wins: unset PC[0] and adjust CPSR[T] to match.

(2) CPSR_T wins: unset pc[0] if CPSR[T] is set, unchanged otherwise.  (In the Arm ARM 
psueodcode, pc[0] is hardwired to 0 in thumb mode.)

(3) Don't worry about matching PC[0] and CPSR[T], but do prevent an impossible situation 
and unset PC[0] always.  If PC[1] is set, and CPSR[T] is unset, then we'll raise unaligned 
when the cpu restarts.

And, finally, you're missing the mc_vfp_* handling.


r~

Re: [PATCH 22/24] bsd-user/arm/target_arch_signal.h: arm set_mcontext

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 2160 bytes --]

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

> On 10/19/21 9:44 AM, Warner Losh wrote:
> > +    regs->regs[15] = tswap32(gr[TARGET_REG_PC]);
> > +    cpsr = tswap32(gr[TARGET_REG_CPSR]);
> > +    cpsr_write(regs, cpsr, CPSR_USER | CPSR_EXEC, CPSRWriteByInstr);
>
> Hmm.  What's the expected behaviour if the saved CPSR state does not match
> the PC state
> wrt thumb?
>
> I'm ok if this should fail in spectacular ways, I just wanna know.
>
> I *think* what will happen at the moment is that qemu will go into a
> whacky state in which
> the translator will read and interpret unaligned data.
>
> I have a pending patch set for arm to raise unaligned exceptions for
> mis-aligned pc.  For
> arm32 mode, this is fine, and we'll raise the exception.  But for thumb
> mode, this is
> architecturally impossible, and the translator will assert.
>
> The assert is going to be a problem.  There are a couple of options:
>
> (1) TARGET_REG_PC wins: unset PC[0] and adjust CPSR[T] to match.
>
> (2) CPSR_T wins: unset pc[0] if CPSR[T] is set, unchanged otherwise.  (In
> the Arm ARM
> psueodcode, pc[0] is hardwired to 0 in thumb mode.)
>
> (3) Don't worry about matching PC[0] and CPSR[T], but do prevent an
> impossible situation
> and unset PC[0] always.  If PC[1] is set, and CPSR[T] is unset, then we'll
> raise unaligned
> when the cpu restarts.
>

Consider this program:
#include <signal.h>
#include <stdio.h>
#include <unistd.h>
int g;
void fortytwo(int arg) { g = 42; }
int main(int argc, char **argv) {
        g = 123;
        signal(SIGALRM, fortytwo); alarm(1); pause();
        printf("G is %d\n", g);
}

Built for 'arm' I get
G is 42
Build -mthumb I get
qemu: uncaught target signal 11 (Segmentation fault) - core dumped
Segmentation fault

So even without your new assert, there are some issues I need to look into
before I can
get to this very interesting case :(. These are all good questions, though.
I clearly have
some work to do here...

And, finally, you're missing the mc_vfp_* handling.
>

Yes. We don't really support that at the moment, but I'll look into how
hard that might be
to add.

Warner

[-- Attachment #2: Type: text/html, Size: 3041 bytes --]

Re: [PATCH 22/24] bsd-user/arm/target_arch_signal.h: arm set_mcontext

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 3951 bytes --]

On Thu, Oct 28, 2021 at 6:07 PM Warner Losh <imp@bsdimp.com> wrote:

>
>
> On Thu, Oct 28, 2021 at 11:53 AM Richard Henderson <
> richard.henderson@linaro.org> wrote:
>
>> On 10/19/21 9:44 AM, Warner Losh wrote:
>> > +    regs->regs[15] = tswap32(gr[TARGET_REG_PC]);
>> > +    cpsr = tswap32(gr[TARGET_REG_CPSR]);
>> > +    cpsr_write(regs, cpsr, CPSR_USER | CPSR_EXEC, CPSRWriteByInstr);
>>
>> Hmm.  What's the expected behaviour if the saved CPSR state does not
>> match the PC state
>> wrt thumb?
>>
>> I'm ok if this should fail in spectacular ways, I just wanna know.
>>
>> I *think* what will happen at the moment is that qemu will go into a
>> whacky state in which
>> the translator will read and interpret unaligned data.
>>
>> I have a pending patch set for arm to raise unaligned exceptions for
>> mis-aligned pc.  For
>> arm32 mode, this is fine, and we'll raise the exception.  But for thumb
>> mode, this is
>> architecturally impossible, and the translator will assert.
>>
>> The assert is going to be a problem.  There are a couple of options:
>>
>> (1) TARGET_REG_PC wins: unset PC[0] and adjust CPSR[T] to match.
>>
>> (2) CPSR_T wins: unset pc[0] if CPSR[T] is set, unchanged otherwise.  (In
>> the Arm ARM
>> psueodcode, pc[0] is hardwired to 0 in thumb mode.)
>>
>> (3) Don't worry about matching PC[0] and CPSR[T], but do prevent an
>> impossible situation
>> and unset PC[0] always.  If PC[1] is set, and CPSR[T] is unset, then
>> we'll raise unaligned
>> when the cpu restarts.
>>
>
> Consider this program:
> #include <signal.h>
> #include <stdio.h>
> #include <unistd.h>
> int g;
> void fortytwo(int arg) { g = 42; }
> int main(int argc, char **argv) {
>         g = 123;
>         signal(SIGALRM, fortytwo); alarm(1); pause();
>         printf("G is %d\n", g);
> }
>
> Built for 'arm' I get
> G is 42
> Build -mthumb I get
> qemu: uncaught target signal 11 (Segmentation fault) - core dumped
> Segmentation fault
>
> So even without your new assert, there are some issues I need to look into
> before I can
> get to this very interesting case :(. These are all good questions,
> though. I clearly have
> some work to do here...
>

Turns out I just needed to filter things correctly, and the changes to
bsd-user/arm/target_arch_thread.h
made the thumb signals work. I've not yet written tests that run T32
instructions and get a A32
signal (or vice versa). I've also not tried to do the same with T32 and A32
threads (well, threads
executing in those two modes and switching between them). That is beyond
the scope of this
set of patches, though.

Real FreeBSD blindly sets these values and hopes the processor generates a
fault for invalid states.
With the filtering I added for the next round, we'll at least ensure that
PC[0] == CPSR[T]. This ensures
consistency, but I don't know how well it will fare in the real world.
FreeBSD's thumb support wrt
mcontext and thumb has only recently become more robust, but it doesn't
check in the kernel.


> And, finally, you're missing the mc_vfp_* handling.
>>
>
> Yes. We don't really support that at the moment, but I'll look into how
> hard that might be
> to add.
>

I've added it here and in get_mcontext too.

I'll also include an up-to-date copy of a pointer to the tip of the
bsd-user fork so you can
see the current state of thigns like signal.c, which I have penciled in for
after the aarch
and riscv64 patches that I have lined up (but haven't done the common
errors between the
archs yet). Since I'd either need to seen a super-large review or delay
things somewhat
for signal.c, I'd like to get the other architectures in since they are
almost ready unless there's
a compelling reason to do signal.c and all its dependencies next. But
that's getting a bit far
afield from this one patch....

And thank you for finding this and the other hard issues with this series!
It's been instructive
and gives me a few things to double check on the other, unsent series.

Warner

[-- Attachment #2: Type: text/html, Size: 5559 bytes --]

Re: [PATCH 22/24] bsd-user/arm/target_arch_signal.h: arm set_mcontext

[Richard Henderson]

On 10/19/21 9:44 AM, Warner Losh wrote:
> +    cpsr = tswap32(gr[TARGET_REG_CPSR]);
> +    cpsr_write(regs, cpsr, CPSR_USER | CPSR_EXEC, CPSRWriteByInstr);

The kernel's set_mcontext validates CPSR_{M,I,F}.


r~

[PATCH 23/24] bsd-user/arm/target_arch_signal.h: arm get_ucontext_sigreturn

[Warner Losh]

Update ucontext to implement sigreturn.

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/arm/target_arch_signal.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/bsd-user/arm/target_arch_signal.h b/bsd-user/arm/target_arch_signal.h
index 1d051af9ae..7da68c727c 100644
--- a/bsd-user/arm/target_arch_signal.h
+++ b/bsd-user/arm/target_arch_signal.h
@@ -232,4 +232,22 @@ static inline abi_long set_mcontext(CPUARMState *regs, target_mcontext_t *mcp,
     return err;
 }
 
+/* Compare to arm/arm/machdep.c sys_sigreturn() */
+static inline abi_long get_ucontext_sigreturn(CPUARMState *regs,
+        abi_ulong target_sf, abi_ulong *target_uc)
+{
+    uint32_t cpsr = cpsr_read(regs);
+
+    *target_uc = 0;
+
+    if ((cpsr & CPSR_M) != ARM_CPU_MODE_USR ||
+            (cpsr & (CPSR_I | CPSR_F)) != 0) {
+        return -TARGET_EINVAL;
+    }
+
+    *target_uc = target_sf;
+
+    return 0;
+}
+
 #endif /* !_TARGET_ARCH_SIGNAL_H_ */
-- 
2.32.0

Re: [PATCH 23/24] bsd-user/arm/target_arch_signal.h: arm get_ucontext_sigreturn

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
>
> Update ucontext to implement sigreturn.
>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/arm/target_arch_signal.h | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/bsd-user/arm/target_arch_signal.h b/bsd-user/arm/target_arch_signal.h
> index 1d051af9ae..7da68c727c 100644
> --- a/bsd-user/arm/target_arch_signal.h
> +++ b/bsd-user/arm/target_arch_signal.h
> @@ -232,4 +232,22 @@ static inline abi_long set_mcontext(CPUARMState *regs, target_mcontext_t *mcp,
>      return err;
>  }
>
> +/* Compare to arm/arm/machdep.c sys_sigreturn() */
> +static inline abi_long get_ucontext_sigreturn(CPUARMState *regs,
> +        abi_ulong target_sf, abi_ulong *target_uc)
> +{
> +    uint32_t cpsr = cpsr_read(regs);
> +
> +    *target_uc = 0;
> +
> +    if ((cpsr & CPSR_M) != ARM_CPU_MODE_USR ||
> +            (cpsr & (CPSR_I | CPSR_F)) != 0) {
> +        return -TARGET_EINVAL;
> +    }
> +
> +    *target_uc = target_sf;
> +
> +    return 0;
> +}
> +
>  #endif /* !_TARGET_ARCH_SIGNAL_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans <kevans@FreeBSD.org>

Re: [PATCH 23/24] bsd-user/arm/target_arch_signal.h: arm get_ucontext_sigreturn

[Richard Henderson]

On 10/19/21 9:44 AM, Warner Losh wrote:
> +/* Compare to arm/arm/machdep.c sys_sigreturn() */
> +static inline abi_long get_ucontext_sigreturn(CPUARMState *regs,
> +        abi_ulong target_sf, abi_ulong *target_uc)
> +{
> +    uint32_t cpsr = cpsr_read(regs);
> +
> +    *target_uc = 0;
> +
> +    if ((cpsr & CPSR_M) != ARM_CPU_MODE_USR ||
> +            (cpsr & (CPSR_I | CPSR_F)) != 0) {
> +        return -TARGET_EINVAL;
> +    }
> +
> +    *target_uc = target_sf;
> +
> +    return 0;
> +}

Since you've not yet filled in signal.c, I can't opine too much, but I'll note that the 
kernel has this check in set_mcontext, and sys_sigreturn defers to that for the validation.

So I think this can just be the final assignment.


r~

[PATCH 24/24] bsd-user: add arm target build

[Warner Losh]

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 configs/targets/arm-bsd-user.mak | 2 ++
 1 file changed, 2 insertions(+)
 create mode 100644 configs/targets/arm-bsd-user.mak

diff --git a/configs/targets/arm-bsd-user.mak b/configs/targets/arm-bsd-user.mak
new file mode 100644
index 0000000000..deea21aaf5
--- /dev/null
+++ b/configs/targets/arm-bsd-user.mak
@@ -0,0 +1,2 @@
+TARGET_ARCH=arm
+TARGET_XML_FILES= gdb-xml/arm-core.xml gdb-xml/arm-vfp.xml gdb-xml/arm-vfp3.xml gdb-xml/arm-neon.xml gdb-xml/arm-m-profile.xml
-- 
2.32.0

Re: [PATCH 24/24] bsd-user: add arm target build

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  configs/targets/arm-bsd-user.mak | 2 ++
>  1 file changed, 2 insertions(+)
>  create mode 100644 configs/targets/arm-bsd-user.mak
>
> diff --git a/configs/targets/arm-bsd-user.mak b/configs/targets/arm-bsd-user.mak
> new file mode 100644
> index 0000000000..deea21aaf5
> --- /dev/null
> +++ b/configs/targets/arm-bsd-user.mak
> @@ -0,0 +1,2 @@
> +TARGET_ARCH=arm
> +TARGET_XML_FILES= gdb-xml/arm-core.xml gdb-xml/arm-vfp.xml gdb-xml/arm-vfp3.xml gdb-xml/arm-neon.xml gdb-xml/arm-m-profile.xml
> --
> 2.32.0
>

I'm not really qualified to review this one, but it looks basically
sane. I note that there's a gdb-xml/arm-vfp-sysregs.xml in the current
master that should probably be added to TARGET_XML_FILES.
Cross-referencing arm-linux-user and i386-bsd-user, this seems sane
and correct and I'm not aware of any other options that we would need
to consider setting, so let's call it:

Acked-by: Kyle Evans <kevans@FreeBSD.org>

Re: [PATCH 24/24] bsd-user: add arm target build

[Richard Henderson]

On 10/25/21 11:21 PM, Kyle Evans wrote:
> On Tue, Oct 19, 2021 at 11:45 AM Warner Losh <imp@bsdimp.com> wrote:
>>
>> Signed-off-by: Warner Losh <imp@bsdimp.com>
>> ---
>>   configs/targets/arm-bsd-user.mak | 2 ++
>>   1 file changed, 2 insertions(+)
>>   create mode 100644 configs/targets/arm-bsd-user.mak
>>
>> diff --git a/configs/targets/arm-bsd-user.mak b/configs/targets/arm-bsd-user.mak
>> new file mode 100644
>> index 0000000000..deea21aaf5
>> --- /dev/null
>> +++ b/configs/targets/arm-bsd-user.mak
>> @@ -0,0 +1,2 @@
>> +TARGET_ARCH=arm
>> +TARGET_XML_FILES= gdb-xml/arm-core.xml gdb-xml/arm-vfp.xml gdb-xml/arm-vfp3.xml gdb-xml/arm-neon.xml gdb-xml/arm-m-profile.xml
>> --
>> 2.32.0
>>
> 
> I'm not really qualified to review this one, but it looks basically
> sane. I note that there's a gdb-xml/arm-vfp-sysregs.xml in the current
> master that should probably be added to TARGET_XML_FILES.
> Cross-referencing arm-linux-user and i386-bsd-user, this seems sane
> and correct and I'm not aware of any other options that we would need
> to consider setting, so let's call it:
> 
> Acked-by: Kyle Evans <kevans@FreeBSD.org>

Yeah, add sysregs.xml and give it
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~