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

Warner Losh posted 24 patches 4 years, 3 months ago
Maintainers: Warner Losh <imp@bsdimp.com>, Kyle Evans <kevans@freebsd.org>, Laurent Vivier <laurent@vivier.eu>, Michael Tokarev <mjt@tls.msk.ru>
There is a newer version of this series
[PATCH 04/24] bsd-user/arm/target_arch_cpu.h: CPU Loop definitions
Posted by Warner Losh 4 years, 3 months ago
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
Posted by Kyle Evans 4 years, 3 months ago
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
Posted by Richard Henderson 4 years, 3 months ago
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
Posted by Warner Losh 4 years, 3 months ago
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