[Qemu-devel] [PATCH 3/4] target/arm: Assert hflags is correct in cpu_get_tb_cpu_state

Richard Henderson posted 4 patches 6 years, 8 months ago
There is a newer version of this series
[Qemu-devel] [PATCH 3/4] target/arm: Assert hflags is correct in cpu_get_tb_cpu_state
Posted by Richard Henderson 6 years, 8 months ago
Make sure that we are updating env->hflags everywhere required.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/helper.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index d8249f0eae..3c8724883d 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -13902,11 +13902,13 @@ void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
     if (is_a64(env)) {
         *pc = env->pc;
         flags = rebuild_hflags_a64(env, current_el);
+        assert(flags == env->hflags);
         flags = FIELD_DP32(flags, TBFLAG_A64, BTYPE, env->btype);
         pstate_for_ss = env->pstate;
     } else {
         *pc = env->regs[15];
         flags = rebuild_hflags_a32(env, current_el);
+        assert(flags == env->hflags);
         flags = FIELD_DP32(flags, TBFLAG_A32, THUMB, env->thumb);
         flags = FIELD_DP32(flags, TBFLAG_A32, CONDEXEC, env->condexec_bits);
         pstate_for_ss = env->uncached_cpsr;
-- 
2.17.1


Re: [Qemu-devel] [PATCH 3/4] target/arm: Assert hflags is correct in cpu_get_tb_cpu_state
Posted by Alex Bennée 6 years, 8 months ago
Richard Henderson <richard.henderson@linaro.org> writes:

> Make sure that we are updating env->hflags everywhere required.

It's a good job you added it thought because on this commit with:

  -machine virt,graphics=on,gic-version=3,virtualization=on

We get:

  qemu-system-aarch64: /home/alex.bennee/lsrc/qemu.git/target/arm/helper.c:14045: cpu_get_tb_cpu_state: Assertion `flags == env->hflags' failed.

  Thread 3 "qemu-system-aar" received signal SIGABRT, Aborted.
  [Switching to Thread 0x7fffde05e700 (LWP 20621)]
  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
  51      ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
  (gdb) bt
  #0  0x00007ffff278fe97 in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
  #1  0x00007ffff2791801 in __GI_abort () at abort.c:79
  #2  0x00007ffff278139a in __assert_fail_base (fmt=0x7ffff29087d8 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=assertion@entry=0x555555dbb012 "flags == env->hflags", file=file@entry=0x555555db96e8 "/home/alex.bennee/lsrc/qemu.git/target/arm/helper.c", line=line@entry=14045, function=function@entry=0x555555dbd810 <__PRETTY_FUNCTION__.43682> "cpu_get_tb_cpu_state") at assert.c:92
  #3  0x00007ffff2781412 in __GI___assert_fail (assertion=assertion@entry=0x555555dbb012 "flags == env->hflags", file=file@entry=0x555555db96e8 "/home/alex.bennee/lsrc/qemu.git/target/arm/helper.c", line=line@entry=14045, function=function@entry=0x555555dbd810 <__PRETTY_FUNCTION__.43682> "cpu_get_tb_cpu_state") at assert.c:101
  #4  0x00005555559bf437 in cpu_get_tb_cpu_state (env=0x5555567150e0, pc=pc@entry=0x7fffde05b0b8, cs_base=cs_base@entry=0x7fffde05b0b0, pflags=pflags@entry=0x7fffde05b0a8) at
  /home/alex.bennee/lsrc/qemu.git/target/arm/helper.c:14045
  #5  0x00005555558ab09b in tb_lookup__cpu_state (cf_mask=524288, flags=0x7fffde05b0a8, cs_base=0x7fffde05b0b0, pc=0x7fffde05b0b8, cpu=0x0) at /home/alex.bennee/lsrc/qemu.git/include/exec/tb-lookup.h:28
  #6  0x00005555558ab09b in tb_find (cf_mask=524288, tb_exit=0, last_tb=0x0, cpu=0x0) at /home/alex.bennee/lsrc/qemu.git/accel/tcg/cpu-exec.c:404
  #7  0x00005555558ab09b in cpu_exec (cpu=cpu@entry=0x55555670ce30) at /home/alex.bennee/lsrc/qemu.git/accel/tcg/cpu-exec.c:728
  #8  0x000055555586963f in tcg_cpu_exec (cpu=0x55555670ce30) at /home/alex.bennee/lsrc/qemu.git/cpus.c:1429
  #9  0x000055555586b7c3 in qemu_tcg_cpu_thread_fn (arg=arg@entry=0x55555670ce30) at /home/alex.bennee/lsrc/qemu.git/cpus.c:1733
  #10 0x0000555555d44e06 in qemu_thread_start (args=<optimized out>) at util/qemu-thread-posix.c:502
  #11 0x00007ffff2b496db in start_thread (arg=0x7fffde05e700) at pthread_create.c:463
  #12 0x00007ffff287288f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95


>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/helper.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index d8249f0eae..3c8724883d 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -13902,11 +13902,13 @@ void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
>      if (is_a64(env)) {
>          *pc = env->pc;
>          flags = rebuild_hflags_a64(env, current_el);
> +        assert(flags == env->hflags);
>          flags = FIELD_DP32(flags, TBFLAG_A64, BTYPE, env->btype);
>          pstate_for_ss = env->pstate;
>      } else {
>          *pc = env->regs[15];
>          flags = rebuild_hflags_a32(env, current_el);
> +        assert(flags == env->hflags);
>          flags = FIELD_DP32(flags, TBFLAG_A32, THUMB, env->thumb);
>          flags = FIELD_DP32(flags, TBFLAG_A32, CONDEXEC, env->condexec_bits);
>          pstate_for_ss = env->uncached_cpsr;


--
Alex Bennée

Re: [Qemu-devel] [PATCH 3/4] target/arm: Assert hflags is correct in cpu_get_tb_cpu_state
Posted by Alex Bennée 6 years, 8 months ago
Richard Henderson <richard.henderson@linaro.org> writes:

> Make sure that we are updating env->hflags everywhere required.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Arguably this is a debugging aid and we don't need it the git history,
nevertheless:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  target/arm/helper.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index d8249f0eae..3c8724883d 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -13902,11 +13902,13 @@ void cpu_get_tb_cpu_state(CPUARMState *env, target_ulong *pc,
>      if (is_a64(env)) {
>          *pc = env->pc;
>          flags = rebuild_hflags_a64(env, current_el);
> +        assert(flags == env->hflags);
>          flags = FIELD_DP32(flags, TBFLAG_A64, BTYPE, env->btype);
>          pstate_for_ss = env->pstate;
>      } else {
>          *pc = env->regs[15];
>          flags = rebuild_hflags_a32(env, current_el);
> +        assert(flags == env->hflags);
>          flags = FIELD_DP32(flags, TBFLAG_A32, THUMB, env->thumb);
>          flags = FIELD_DP32(flags, TBFLAG_A32, CONDEXEC, env->condexec_bits);
>          pstate_for_ss = env->uncached_cpsr;


--
Alex Bennée