Andrew Jones <drjo...@redhat.com> writes:
> Hello TCG developers, > > We have new debug test cases in kvm-unit-tests thanks to Ricardo > Koller. Yay tests ;-) > The singlestep test cases are failing with TCG. Enabling TCG debug outputs > the error > > TCG hflags mismatch (current:(0x000004a1,0x0000000000004000) > rebuilt:(0x000004a3,0x0000000000004000) This shows that: FIELD(TBFLAG_ANY, SS_ACTIVE, 1, 1) should be set but wasn't cached. > I noticed that the test passed on an older QEMU, so I bisected it and > found commit e979972a6a17 ("target/arm: Rely on hflags correct in > cpu_get_tb_cpu_state"), which unfortunately doesn't tell us anything > that the above error message didn't say already (apparently we can't > currently depend on hflags being correct wrt singlestep at this > point). Fortunately this is intended - the enable-debug always recalculates the hflags (and expensive operation) and makes it pretty easy to spot where we failed to call arm_rebuild_hflags(). You can do this with the normal debug tools or my new favourite tool (for short programs) using the execlog plugin. 0, 0x40080a24, 0x52840020, "movz w0, #0x2001" 0, 0x40080a28, 0x2a010000, "orr w0, w0, w1" 0, 0x40080a2c, 0xd5100240, "msr mdscr_el1, x0" 0, 0x40080a30, 0xd5033fdf, "isb " 0, 0x40080a34, 0x350001f4, "cbnz w20, #0x40080a70" TCG hflags mismatch (current:(0x000004a1,0x0000000000004000) rebuilt:(0x000004a3,0x0000000000004000) This is a touch weird though because any msr write does trigger a rebuild of the flags. See handle_sys(): if (!isread && !(ri->type & ARM_CP_SUPPRESS_TB_END)) { /* * A write to any coprocessor regiser that ends a TB * must rebuild the hflags for the next TB. */ TCGv_i32 tcg_el = tcg_const_i32(s->current_el); gen_helper_rebuild_hflags_a64(cpu_env, tcg_el); tcg_temp_free_i32(tcg_el); /* * We default to ending the TB on a coprocessor register write, * but allow this to be suppressed by the register definition * (usually only necessary to work around guest bugs). */ s->base.is_jmp = DISAS_UPDATE_EXIT; } And indeed in rr I can see it working though the tail end of helper_rebuild_flags_a64() but it seems arm_singlestep_active() returns false at this point. This ultimately fails at aa64_generate_debug_exceptions(): /* * Same EL to same EL debug exceptions need MDSCR_KDE enabled * while not masking the (D)ebug bit in DAIF. */ debug_el = arm_debug_target_el(env); if (cur_el == debug_el) { return extract32(env->cp15.mdscr_el1, 13, 1) && !(env->daif & PSTATE_D); } And if I look at the objdump it is indeed the instruction we never completed: a34: 350001f4 cbnz w20, a70 <ss_start+0x34> a38: d50348ff msr daifclr, #0x8 So if I force the flag generation on manipulating daif: --8<---------------cut here---------------start------------->8--- modified target/arm/helper-a64.c @@ -83,12 +83,14 @@ void HELPER(msr_i_daifset)(CPUARMState *env, uint32_t imm) { daif_check(env, 0x1e, imm, GETPC()); env->daif |= (imm << 6) & PSTATE_DAIF; + arm_rebuild_hflags(env); } void HELPER(msr_i_daifclear)(CPUARMState *env, uint32_t imm) { daif_check(env, 0x1f, imm, GETPC()); env->daif &= ~((imm << 6) & PSTATE_DAIF); + arm_rebuild_hflags(env); } --8<---------------cut here---------------end--------------->8--- I now get a working test: env QEMU=$HOME/lsrc/qemu.git/builds/all.debug/qemu-system-aarch64 ./run_tests.sh -g debug PASS debug-bp (6 tests) PASS debug-bp-migration (7 tests) PASS debug-wp (8 tests) PASS debug-wp-migration (9 tests) PASS debug-sstep (1 tests) PASS debug-sstep-migration (1 tests) (I was momentarily confused when debug-sstep failed, but that was I'd forgotten to point to my build, the system 5.2 qemu is broken in this regard). I'll spin up a proper patch. Side note: ad5fb8830150071487025b3594a7b1bf218d12d8 is the first bad commit commit ad5fb8830150071487025b3594a7b1bf218d12d8 Author: Zixuan Wang <zixuanw...@google.com> Date: Mon Oct 4 13:49:19 2021 -0700 breaks the running on kvm-unit-test for me, I needed to patch: --8<---------------cut here---------------start------------->8--- modified run_tests.sh @@ -31,7 +31,8 @@ specify the appropriate qemu binary for ARCH-run. EOF } -RUNTIME_arch_run="./$TEST_SUBDIR/run" +RUNTIME_arch_run="./$TEST_DIR/run" +#RUNTIME_arch_run="./$TEST_SUBDIR/run" source scripts/runtime.bash # require enhanced getopt --8<---------------cut here---------------end--------------->8--- -- Alex Bennée