Re: [Qemu-devel] [RFC v5 4/6] target-arm: Create new runtime helpers for excl accesses

[alvise rigo <a.rigo@virtualopensystems.com>]

On Wed, Sep 30, 2015 at 6:03 AM, Richard Henderson <rth@twiddle.net> wrote:
> On 09/24/2015 06:32 PM, Alvise Rigo wrote:
>>
>> Introduce a set of new runtime helpers do handle exclusive instructions.
>> This helpers are used as hooks to call the respective LL/SC helpers in
>> softmmu_llsc_template.h from TCG code.
>>
>> Suggested-by: Jani Kokkonen <jani.kokkonen@huawei.com>
>> Suggested-by: Claudio Fontana <claudio.fontana@huawei.com>
>> Signed-off-by: Alvise Rigo <a.rigo@virtualopensystems.com>
>> ---
>>   target-arm/helper.h    | 10 ++++++
>>   target-arm/op_helper.c | 94
>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 104 insertions(+)
>>
>> diff --git a/target-arm/helper.h b/target-arm/helper.h
>> index 827b33d..8e7a7c2 100644
>> --- a/target-arm/helper.h
>> +++ b/target-arm/helper.h
>> @@ -530,6 +530,16 @@ DEF_HELPER_2(dc_zva, void, env, i64)
>>   DEF_HELPER_FLAGS_2(neon_pmull_64_lo, TCG_CALL_NO_RWG_SE, i64, i64, i64)
>>   DEF_HELPER_FLAGS_2(neon_pmull_64_hi, TCG_CALL_NO_RWG_SE, i64, i64, i64)
>>
>> +DEF_HELPER_3(ldlink_aa32_i8, i32, env, i32, i32)
>> +DEF_HELPER_3(ldlink_aa32_i16, i32, env, i32, i32)
>> +DEF_HELPER_3(ldlink_aa32_i32, i32, env, i32, i32)
>> +DEF_HELPER_3(ldlink_aa32_i64, i64, env, i32, i32)
>> +
>> +DEF_HELPER_4(stcond_aa32_i8, i32, env, i32, i32, i32)
>> +DEF_HELPER_4(stcond_aa32_i16, i32, env, i32, i32, i32)
>> +DEF_HELPER_4(stcond_aa32_i32, i32, env, i32, i32, i32)
>> +DEF_HELPER_4(stcond_aa32_i64, i32, env, i32, i64, i32)
>> +
>>   #ifdef TARGET_AARCH64
>>   #include "helper-a64.h"
>>   #endif
>> diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
>> index 663c05d..d832ba8 100644
>> --- a/target-arm/op_helper.c
>> +++ b/target-arm/op_helper.c
>> @@ -969,3 +969,97 @@ uint32_t HELPER(ror_cc)(CPUARMState *env, uint32_t x,
>> uint32_t i)
>>           return ((uint32_t)x >> shift) | (x << (32 - shift));
>>       }
>>   }
>> +
>> +/* LoadLink helpers, only unsigned. */
>> +static void * const qemu_ldex_helpers[16] = {
>> +    [MO_UB]   = helper_ret_ldlinkub_mmu,
>> +
>> +    [MO_LEUW] = helper_le_ldlinkuw_mmu,
>> +    [MO_LEUL] = helper_le_ldlinkul_mmu,
>> +    [MO_LEQ]  = helper_le_ldlinkq_mmu,
>> +
>> +    [MO_BEUW] = helper_be_ldlinkuw_mmu,
>> +    [MO_BEUL] = helper_be_ldlinkul_mmu,
>> +    [MO_BEQ]  = helper_be_ldlinkq_mmu,
>> +};
>> +
>> +#define LDEX_HELPER(SUFF, OPC)                                          \
>> +uint32_t HELPER(ldlink_aa32_i##SUFF)(CPUARMState *env, uint32_t addr,   \
>> +                                                       uint32_t index)  \
>> +{                                                                       \
>> +    CPUArchState *state = env;                                          \
>> +    TCGMemOpIdx op;                                                     \
>> +                                                                        \
>> +    op = make_memop_idx(OPC, index);                                    \
>> +                                                                        \
>> +    tcg_target_ulong (*func)(CPUArchState *env, target_ulong addr,      \
>> +                             TCGMemOpIdx oi, uintptr_t retaddr);        \
>> +    func = qemu_ldex_helpers[OPC];                                      \
>> +                                                                        \
>> +    return (uint32_t)func(state, addr, op, GETRA());                    \
>> +}
>> +
>> +LDEX_HELPER(8, MO_UB)
>> +LDEX_HELPER(16, MO_TEUW)
>> +LDEX_HELPER(32, MO_TEUL)
>
>
> This is not what Aurelien meant.  I cannot see any reason at present why
> generic wrappers, available for all targets, shouldn't be sufficient.

I thought we could create ad-hoc helpers for each architecture - for
instance cmpxchg-like helpers for x86.
But now that I think about it, I can image just two types of helpers
(for the two atomic approaches - ll/sc and cmpxchg), that of course
can be generic.

>
> See tcg/tcg-runtime.h and tcg-runtime.c.

I will move them there.

>
> You shouldn't need to look up a function in a table like this.  The decision
> about whether to call a BE or LE helper should have been made in the
> translator.

In the next version I will:

- extend the macro to generate both versions of the helper and not just one
- make the decision in translate.c, always using MO_TE as 'toggle'

Thank you,
alvise

>
>
> r~