On Thu, 1 Feb 2024 13:56:09 -0500
Gregory Price <gregory.pr...@memverge.com> wrote:
> On Thu, Feb 01, 2024 at 06:04:26PM +0000, Peter Maydell wrote:
> > On Thu, 1 Feb 2024 at 17:25, Alex Bennée <alex.ben...@linaro.org> wrote:
> > >
> > > Jonathan Cameron <jonathan.came...@huawei.com> writes:
> > > >> > #21 0x0000555555ca3e5d in do_st8_mmu (cpu=0x5555578e0cb0,
> > > >> > addr=23937, val=18386491784638059520, oi=6, ra=140736029817822) at
> > > >> > ../../accel/tcg/cputlb.c:2853
> > > >> > #22 0x00007fffa9107c63 in code_gen_buffer ()
> > > >>
> > > >> No thats different - we are actually writing to the MMIO region here.
> > > >> But the fact we hit cpu_abort because we can't find the TB we are
> > > >> executing is a little problematic.
> > > >>
> > > >> Does ra properly point to the code buffer here?
> > > >
> > > > Err. How would I tell?
> > >
> > > (gdb) p/x 140736029817822
> > > $1 = 0x7fffa9107bde
> > >
> > > seems off because code_gen_buffer starts at 0x00007fffa9107c63
> >
> > The code_gen_buffer doesn't *start* at 0x00007fffa9107c63 --
> > that is our return address into it, which is to say it's just
> > after the call insn to the do_st8_mmu helper. The 'ra' argument
> > to the helper function is going to be a number slightly lower
> > than that, because it points within the main lump of generated
> > code for the TB, whereas the helper call is done as part of
> > an out-of-line lump of common code at the end of the TB.
> >
> > The 'ra' here is fine -- the problem is that we don't
> > pass it all the way down the callstack and instead end
> > up using 0 as a 'ra' within the ptw code.
> >
> > -- PMM
>
> Is there any particular reason not to, as below?
> ~Gregory
>
One patch blindly applied.
New exciting trace...
Thread 5 "qemu-system-x86" received signal SIGABRT, Aborted.
[Switching to Thread 0x7ffff4efe6c0 (LWP 16503)]
__pthread_kill_implementation (no_tid=0, signo=6, threadid=<optimized out>) at
./nptl/pthread_kill.c:44
Download failed: Invalid argument. Continuing without source file
./nptl/./nptl/pthread_kill.c.
44 ./nptl/pthread_kill.c: No such file or directory.
(gdb) bt
#0 __pthread_kill_implementation (no_tid=0, signo=6, threadid=<optimized out>)
at ./nptl/pthread_kill.c:44
#1 __pthread_kill_internal (signo=6, threadid=<optimized out>) at
./nptl/pthread_kill.c:78
#2 __GI___pthread_kill (threadid=<optimized out>, signo=signo@entry=6) at
./nptl/pthread_kill.c:89
#3 0x00007ffff77c43b6 in __GI_raise (sig=sig@entry=6) at
../sysdeps/posix/raise.c:26
#4 0x00007ffff77aa87c in __GI_abort () at ./stdlib/abort.c:79
#5 0x00007ffff7b2ed1e in () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#6 0x00007ffff7b9622e in g_assertion_message_expr () at
/lib/x86_64-linux-gnu/libglib-2.0.so.0
#7 0x0000555555ab1929 in bql_lock_impl (file=0x555556049122
"../../accel/tcg/cputlb.c", line=2033) at ../../system/cpus.c:524
#8 bql_lock_impl (file=file@entry=0x555556049122 "../../accel/tcg/cputlb.c",
line=line@entry=2033) at ../../system/cpus.c:520
#9 0x0000555555c9f7d6 in do_ld_mmio_beN (cpu=0x5555578e0cb0,
full=0x7ffe88012950, ret_be=ret_be@entry=0, addr=19595792376,
size=size@entry=8, mmu_idx=4, type=MMU_DATA_LOAD, ra=0) at
../../accel/tcg/cputlb.c:2033
#10 0x0000555555ca0fbd in do_ld_8 (cpu=cpu@entry=0x5555578e0cb0,
p=p@entry=0x7ffff4efd1d0, mmu_idx=<optimized out>,
type=type@entry=MMU_DATA_LOAD, memop=<optimized out>, ra=ra@entry=0) at
../../accel/tcg/cputlb.c:2356
#11 0x0000555555ca341f in do_ld8_mmu (cpu=cpu@entry=0x5555578e0cb0,
addr=addr@entry=19595792376, oi=oi@entry=52, ra=0, ra@entry=52,
access_type=access_type@entry=MMU_DATA_LOAD) at ../../accel/tcg/cputlb.c:2439
#12 0x0000555555ca5f59 in cpu_ldq_mmu (ra=52, oi=52, addr=19595792376,
env=0x5555578e3470) at ../../accel/tcg/ldst_common.c.inc:169
#13 cpu_ldq_le_mmuidx_ra (env=0x5555578e3470, addr=19595792376,
mmu_idx=<optimized out>, ra=ra@entry=0) at ../../accel/tcg/ldst_common.c.inc:301
#14 0x0000555555b4b5fc in ptw_ldq (ra=0, in=0x7ffff4efd320) at
../../target/i386/tcg/sysemu/excp_helper.c:98
#15 ptw_ldq (ra=0, in=0x7ffff4efd320) at
../../target/i386/tcg/sysemu/excp_helper.c:93
#16 mmu_translate (env=env@entry=0x5555578e3470, in=0x7ffff4efd3e0,
out=0x7ffff4efd3b0, err=err@entry=0x7ffff4efd3c0, ra=ra@entry=0) at
../../target/i386/tcg/sysemu/excp_helper.c:174
#17 0x0000555555b4c4b3 in get_physical_address (ra=0, err=0x7ffff4efd3c0,
out=0x7ffff4efd3b0, mmu_idx=0, access_type=MMU_DATA_LOAD,
addr=18446741874686299840, env=0x5555578e3470) at
../../target/i386/tcg/sysemu/excp_helper.c:580
#18 x86_cpu_tlb_fill (cs=0x5555578e0cb0, addr=18446741874686299840,
size=<optimized out>, access_type=MMU_DATA_LOAD, mmu_idx=0, probe=<optimized
out>, retaddr=0) at ../../target/i386/tcg/sysemu/excp_helper.c:606
#19 0x0000555555ca0ee9 in tlb_fill (retaddr=0, mmu_idx=0,
access_type=MMU_DATA_LOAD, size=<optimized out>, addr=18446741874686299840,
cpu=0x7ffff4efd540) at ../../accel/tcg/cputlb.c:1315
#20 mmu_lookup1 (cpu=cpu@entry=0x5555578e0cb0, data=data@entry=0x7ffff4efd540,
mmu_idx=0, access_type=access_type@entry=MMU_DATA_LOAD, ra=ra@entry=0) at
../../accel/tcg/cputlb.c:1713
#21 0x0000555555ca2c61 in mmu_lookup (cpu=cpu@entry=0x5555578e0cb0,
addr=addr@entry=18446741874686299840, oi=oi@entry=32, ra=ra@entry=0,
type=type@entry=MMU_DATA_LOAD, l=l@entry=0x7ffff4efd540) at
../../accel/tcg/cputlb.c:1803
#22 0x0000555555ca3165 in do_ld4_mmu (cpu=cpu@entry=0x5555578e0cb0,
addr=addr@entry=18446741874686299840, oi=oi@entry=32, ra=ra@entry=0,
access_type=access_type@entry=MMU_DATA_LOAD) at ../../accel/tcg/cputlb.c:2416
#23 0x0000555555ca5ef9 in cpu_ldl_mmu (ra=0, oi=32, addr=18446741874686299840,
env=0x5555578e3470) at ../../accel/tcg/ldst_common.c.inc:158
#24 cpu_ldl_le_mmuidx_ra (env=env@entry=0x5555578e3470,
addr=addr@entry=18446741874686299840, mmu_idx=<optimized out>, ra=ra@entry=0)
at ../../accel/tcg/ldst_common.c.inc:294
#25 0x0000555555bb6cdd in do_interrupt64 (is_hw=1,
next_eip=18446744072399775809, error_code=0, is_int=0, intno=236,
env=0x5555578e3470) at ../../target/i386/tcg/seg_helper.c:889
#26 do_interrupt_all (cpu=cpu@entry=0x5555578e0cb0, intno=236,
is_int=is_int@entry=0, error_code=error_code@entry=0,
next_eip=next_eip@entry=0, is_hw=is_hw@entry=1) at
../../target/i386/tcg/seg_helper.c:1130
#27 0x0000555555bb87da in do_interrupt_x86_hardirq
(env=env@entry=0x5555578e3470, intno=<optimized out>, is_hw=is_hw@entry=1) at
../../target/i386/tcg/seg_helper.c:1162
#28 0x0000555555b5039c in x86_cpu_exec_interrupt (cs=0x5555578e0cb0,
interrupt_request=<optimized out>) at
../../target/i386/tcg/sysemu/seg_helper.c:197
#29 0x0000555555c94480 in cpu_handle_interrupt (last_tb=<synthetic pointer>,
cpu=0x5555578e0cb0) at ../../accel/tcg/cpu-exec.c:844
#30 cpu_exec_loop (cpu=cpu@entry=0x5555578e0cb0, sc=sc@entry=0x7ffff4efd7b0) at
../../accel/tcg/cpu-exec.c:951
#31 0x0000555555c94791 in cpu_exec_setjmp (cpu=cpu@entry=0x5555578e0cb0,
sc=sc@entry=0x7ffff4efd7b0) at ../../accel/tcg/cpu-exec.c:1029
#32 0x0000555555c94f7c in cpu_exec (cpu=cpu@entry=0x5555578e0cb0) at
../../accel/tcg/cpu-exec.c:1055
#33 0x0000555555cb9043 in tcg_cpu_exec (cpu=cpu@entry=0x5555578e0cb0) at
../../accel/tcg/tcg-accel-ops.c:76
#34 0x0000555555cb91a0 in mttcg_cpu_thread_fn (arg=arg@entry=0x5555578e0cb0) at
../../accel/tcg/tcg-accel-ops-mttcg.c:95
#35 0x0000555555e57270 in qemu_thread_start (args=0x555557956000) at
../../util/qemu-thread-posix.c:541
#36 0x00007ffff78176ba in start_thread (arg=<optimized out>) at
./nptl/pthread_create.c:444
#37 0x00007ffff78a60d0 in clone3 () at
../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
>
> diff --git a/target/i386/tcg/sysemu/excp_helper.c
> b/target/i386/tcg/sysemu/excp_helper.c
> index 5b86f439ad..2f581b9bfb 100644
> --- a/target/i386/tcg/sysemu/excp_helper.c
> +++ b/target/i386/tcg/sysemu/excp_helper.c
> @@ -59,14 +59,14 @@ typedef struct PTETranslate {
> hwaddr gaddr;
> } PTETranslate;
>
> -static bool ptw_translate(PTETranslate *inout, hwaddr addr)
> +static bool ptw_translate(PTETranslate *inout, hwaddr addr, uint64_t ra)
> {
> CPUTLBEntryFull *full;
> int flags;
>
> inout->gaddr = addr;
> flags = probe_access_full(inout->env, addr, 0, MMU_DATA_STORE,
> - inout->ptw_idx, true, &inout->haddr, &full, 0);
> + inout->ptw_idx, true, &inout->haddr, &full,
> ra);
>
> if (unlikely(flags & TLB_INVALID_MASK)) {
> TranslateFault *err = inout->err;
> @@ -82,20 +82,20 @@ static bool ptw_translate(PTETranslate *inout, hwaddr
> addr)
> return true;
> }
>
> -static inline uint32_t ptw_ldl(const PTETranslate *in)
> +static inline uint32_t ptw_ldl(const PTETranslate *in, uint64_t ra)
> {
> if (likely(in->haddr)) {
> return ldl_p(in->haddr);
> }
> - return cpu_ldl_mmuidx_ra(in->env, in->gaddr, in->ptw_idx, 0);
> + return cpu_ldl_mmuidx_ra(in->env, in->gaddr, in->ptw_idx, ra);
> }
>
> -static inline uint64_t ptw_ldq(const PTETranslate *in)
> +static inline uint64_t ptw_ldq(const PTETranslate *in, uint64_t ra)
> {
> if (likely(in->haddr)) {
> return ldq_p(in->haddr);
> }
> - return cpu_ldq_mmuidx_ra(in->env, in->gaddr, in->ptw_idx, 0);
> + return cpu_ldq_mmuidx_ra(in->env, in->gaddr, in->ptw_idx, ra);
> }
>
> /*
> @@ -132,7 +132,8 @@ static inline bool ptw_setl(const PTETranslate *in,
> uint32_t old, uint32_t set)
> }
>
> static bool mmu_translate(CPUX86State *env, const TranslateParams *in,
> - TranslateResult *out, TranslateFault *err)
> + TranslateResult *out, TranslateFault *err,
> + uint64_t ra)
> {
> const int32_t a20_mask = x86_get_a20_mask(env);
> const target_ulong addr = in->addr;
> @@ -166,11 +167,11 @@ static bool mmu_translate(CPUX86State *env, const
> TranslateParams *in,
> */
> pte_addr = ((in->cr3 & ~0xfff) +
> (((addr >> 48) & 0x1ff) << 3)) & a20_mask;
> - if (!ptw_translate(&pte_trans, pte_addr)) {
> + if (!ptw_translate(&pte_trans, pte_addr, ra)) {
> return false;
> }
> restart_5:
> - pte = ptw_ldq(&pte_trans);
> + pte = ptw_ldq(&pte_trans, ra);
> if (!(pte & PG_PRESENT_MASK)) {
> goto do_fault;
> }
> @@ -191,11 +192,11 @@ static bool mmu_translate(CPUX86State *env, const
> TranslateParams *in,
> */
> pte_addr = ((pte & PG_ADDRESS_MASK) +
> (((addr >> 39) & 0x1ff) << 3)) & a20_mask;
> - if (!ptw_translate(&pte_trans, pte_addr)) {
> + if (!ptw_translate(&pte_trans, pte_addr, ra)) {
> return false;
> }
> restart_4:
> - pte = ptw_ldq(&pte_trans);
> + pte = ptw_ldq(&pte_trans, ra);
> if (!(pte & PG_PRESENT_MASK)) {
> goto do_fault;
> }
> @@ -212,11 +213,11 @@ static bool mmu_translate(CPUX86State *env, const
> TranslateParams *in,
> */
> pte_addr = ((pte & PG_ADDRESS_MASK) +
> (((addr >> 30) & 0x1ff) << 3)) & a20_mask;
> - if (!ptw_translate(&pte_trans, pte_addr)) {
> + if (!ptw_translate(&pte_trans, pte_addr, ra)) {
> return false;
> }
> restart_3_lma:
> - pte = ptw_ldq(&pte_trans);
> + pte = ptw_ldq(&pte_trans, ra);
> if (!(pte & PG_PRESENT_MASK)) {
> goto do_fault;
> }
> @@ -239,12 +240,12 @@ static bool mmu_translate(CPUX86State *env, const
> TranslateParams *in,
> * Page table level 3
> */
> pte_addr = ((in->cr3 & ~0x1f) + ((addr >> 27) & 0x18)) &
> a20_mask;
> - if (!ptw_translate(&pte_trans, pte_addr)) {
> + if (!ptw_translate(&pte_trans, pte_addr, ra)) {
> return false;
> }
> rsvd_mask |= PG_HI_USER_MASK;
> restart_3_nolma:
> - pte = ptw_ldq(&pte_trans);
> + pte = ptw_ldq(&pte_trans, ra);
> if (!(pte & PG_PRESENT_MASK)) {
> goto do_fault;
> }
> @@ -262,11 +263,11 @@ static bool mmu_translate(CPUX86State *env, const
> TranslateParams *in,
> */
> pte_addr = ((pte & PG_ADDRESS_MASK) +
> (((addr >> 21) & 0x1ff) << 3)) & a20_mask;
> - if (!ptw_translate(&pte_trans, pte_addr)) {
> + if (!ptw_translate(&pte_trans, pte_addr, ra)) {
> return false;
> }
> restart_2_pae:
> - pte = ptw_ldq(&pte_trans);
> + pte = ptw_ldq(&pte_trans, ra);
> if (!(pte & PG_PRESENT_MASK)) {
> goto do_fault;
> }
> @@ -289,10 +290,10 @@ static bool mmu_translate(CPUX86State *env, const
> TranslateParams *in,
> */
> pte_addr = ((pte & PG_ADDRESS_MASK) +
> (((addr >> 12) & 0x1ff) << 3)) & a20_mask;
> - if (!ptw_translate(&pte_trans, pte_addr)) {
> + if (!ptw_translate(&pte_trans, pte_addr, ra)) {
> return false;
> }
> - pte = ptw_ldq(&pte_trans);
> + pte = ptw_ldq(&pte_trans, ra);
> if (!(pte & PG_PRESENT_MASK)) {
> goto do_fault;
> }
> @@ -307,11 +308,11 @@ static bool mmu_translate(CPUX86State *env, const
> TranslateParams *in,
> * Page table level 2
> */
> pte_addr = ((in->cr3 & ~0xfff) + ((addr >> 20) & 0xffc)) & a20_mask;
> - if (!ptw_translate(&pte_trans, pte_addr)) {
> + if (!ptw_translate(&pte_trans, pte_addr, ra)) {
> return false;
> }
> restart_2_nopae:
> - pte = ptw_ldl(&pte_trans);
> + pte = ptw_ldl(&pte_trans, ra);
> if (!(pte & PG_PRESENT_MASK)) {
> goto do_fault;
> }
> @@ -336,10 +337,10 @@ static bool mmu_translate(CPUX86State *env, const
> TranslateParams *in,
> * Page table level 1
> */
> pte_addr = ((pte & ~0xfffu) + ((addr >> 10) & 0xffc)) & a20_mask;
> - if (!ptw_translate(&pte_trans, pte_addr)) {
> + if (!ptw_translate(&pte_trans, pte_addr, ra)) {
> return false;
> }
> - pte = ptw_ldl(&pte_trans);
> + pte = ptw_ldl(&pte_trans, ra);
> if (!(pte & PG_PRESENT_MASK)) {
> goto do_fault;
> }
> @@ -529,7 +530,8 @@ static G_NORETURN void raise_stage2(CPUX86State *env,
> TranslateFault *err,
>
> static bool get_physical_address(CPUX86State *env, vaddr addr,
> MMUAccessType access_type, int mmu_idx,
> - TranslateResult *out, TranslateFault *err)
> + TranslateResult *out, TranslateFault *err,
> + uint64_t ra)
> {
> TranslateParams in;
> bool use_stage2 = env->hflags2 & HF2_NPT_MASK;
> @@ -548,7 +550,7 @@ static bool get_physical_address(CPUX86State *env, vaddr
> addr,
> in.mmu_idx = MMU_USER_IDX;
> in.ptw_idx = MMU_PHYS_IDX;
>
> - if (!mmu_translate(env, &in, out, err)) {
> + if (!mmu_translate(env, &in, out, err, ra)) {
> err->stage2 = S2_GPA;
> return false;
> }
> @@ -575,7 +577,7 @@ static bool get_physical_address(CPUX86State *env, vaddr
> addr,
> return false;
> }
> }
> - return mmu_translate(env, &in, out, err);
> + return mmu_translate(env, &in, out, err, ra);
> }
> break;
> }
> @@ -601,7 +603,7 @@ bool x86_cpu_tlb_fill(CPUState *cs, vaddr addr, int size,
> TranslateResult out;
> TranslateFault err;
>
> - if (get_physical_address(env, addr, access_type, mmu_idx, &out, &err)) {
> + if (get_physical_address(env, addr, access_type, mmu_idx, &out, &err,
> retaddr)) {
> /*
> * Even if 4MB pages, we map only one 4KB page in the cache to
> * avoid filling it too fast.
