Re: plugins: Missing Store Exclusive Memory Accesses

From: "Alex Bennée" <alex.bennee@linaro.org>
To: Aaron Lindsay <aaron@os.amperecomputing.com>
Cc: cota@braap.org, richard.henderson@linaro.org, qemu-devel@nongnu.org
Subject: Re: plugins: Missing Store Exclusive Memory Accesses
Date: Wed, 20 Oct 2021 18:54:20 +0100	[thread overview]
Message-ID: <87cznzvavq.fsf@linaro.org> (raw)
In-Reply-To: <YXBN4UZvY5ESdz6v@strawberry.localdomain>

Aaron Lindsay <aaron@os.amperecomputing.com> writes:

> On Sep 22 16:22, Aaron Lindsay wrote:
>> On Sep 21 16:28, Aaron Lindsay wrote:
>> > On Sep 17 12:05, Alex Bennée wrote:
>> > > Aaron Lindsay <aaron@os.amperecomputing.com> writes:
>> > > > I recently noticed that the plugin interface does not appear to be
>> > > > emitting callbacks to functions registered via
>> > > > `qemu_plugin_register_vcpu_mem_cb` for AArch64 store exclusives. This
>> > > > would include instructions like `stxp  w16, x2, x3, [x4]` (encoding:
>> > > > 0xc8300c82). Seeing as how I'm only running with a single CPU, I don't
>> > > > see how this could be due to losing exclusivity after the preceding
>> > > > `ldxp`.
>> > > 
>> > > The exclusive handling is a bit special due to the need to emulate it's
>> > > behaviour using cmpxchg primitives.
>> > > 
>> > > >
>> > > > In looking at QEMU's source, I *think* this is because the
>> > > > `gen_store_exclusive` function in translate-a64.c is not making the same
>> > > > calls to `plugin_gen_mem_callbacks` & company that are being made by
>> > > > "normal" stores handled by functions like `tcg_gen_qemu_st_i64` (at
>> > > > least in my case; I do see some code paths under `gen_store_exclusive`
>> > > > call down into `tcg_gen_qemu_st_i64` eventually, but it appears not all
>> > > > of them do?).
>> > > 
>> > > The key TCG operation is the cmpxchg which does the effective store. For
>> > > -smp 1 we should use normal ld and st tcg ops. For > 1 it eventually
>> > > falls to tcg_gen_atomic_cmpxchg_XX which is a helper. That eventually
>> > > ends up at:
>> > > 
>> > >   atomic_trace_rmw_post
>> > > 
>> > > which should be where things are hooked.
>> > 
>> > When I open this up in gdb, I see that I'm getting the following call
>> > graph for the `stxp` instruction in question (for -smp 1):
>> > 
>> > gen_store_exclusive -> gen_helper_paired_cmpxchg64_le
>> > 
>> > In other words, I'm taking the `s->be_data == MO_LE` else/if clause.
>> > 
>> > I do not see where the helper behind that (defined in helper-a64.c as
>> > `uint64_t HELPER(paired_cmpxchg64_le)...`) is calling in to generate
>> > plugin callbacks in this case. Am I missing something?
>> 
>> Richard, Alex,
>> 
>> The more I look at this, the more it feels like the following
>> AArch64-specific helpers may have been overlooked when adding
>> tracing/plugin hooks:
>> 	gen_helper_paired_cmpxchg64_le
>> 	gen_helper_paired_cmpxchg64_be
>> 
>> But... I'm still not sure I fully understand how everything I'm digging
>> into interacts; I am happy to keep investigating and work towards a fix,
>> but think I need a nudge in the right direction.
>
> Ping?
>
> I'm happy to spend some more time digging into this issue, and would
> love to be pointed in the right direction if someone is able!

These all end up in:

      ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong addr,
                                    ABI_TYPE cmpv, ABI_TYPE newv,
                                    MemOpIdx oi, uintptr_t retaddr)

Have you got a test case you are using so I can try and replicate the
failure you are seeing? So far by inspection everything looks OK to me.

>
> Thanks!
>
> -Aaron

-- 
Alex Bennée