Re: [Qemu-devel] [RFC v3 PATCH 07/14] tcg/ppc: Add support for fence

[Sergey Fedorov <serge.fdrv@gmail.com>]

On 22/06/16 23:21, Richard Henderson wrote:
> On 06/22/2016 12:50 PM, Sergey Fedorov wrote:
>> On 18/06/16 07:03, Pranith Kumar wrote:
>>> Signed-off-by: Richard Henderson <rth@twiddle.net>
>>> Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
>>> ---
>>>  tcg/ppc/tcg-target.inc.c | 24 ++++++++++++++++++++++++
>>>  1 file changed, 24 insertions(+)
>>>
>>> diff --git a/tcg/ppc/tcg-target.inc.c b/tcg/ppc/tcg-target.inc.c
>>> index da10052..766848e 100644
>>> --- a/tcg/ppc/tcg-target.inc.c
>>> +++ b/tcg/ppc/tcg-target.inc.c
>>> @@ -469,6 +469,10 @@ static int tcg_target_const_match(tcg_target_long val, TCGType type,
>>>  #define STHX   XO31(407)
>>>  #define STWX   XO31(151)
>>>  
>>> +#define EIEIO  XO31(854)
>>> +#define HWSYNC XO31(598)
>>> +#define LWSYNC (HWSYNC | (1u << 21))
>>> +
>>>  #define SPR(a, b) ((((a)<<5)|(b))<<11)
>>>  #define LR     SPR(8, 0)
>>>  #define CTR    SPR(9, 0)
>>> @@ -1237,6 +1241,21 @@ static void tcg_out_brcond2 (TCGContext *s, const TCGArg *args,
>>>      tcg_out_bc(s, BC | BI(7, CR_EQ) | BO_COND_TRUE, arg_label(args[5]));
>>>  }
>>>  
>>> +static void tcg_out_mb(TCGContext *s, TCGArg a0)
>>> +{
>>> +    switch (a0 & TCG_MO_ALL) {
>>> +    case TCG_MO_LD_LD:
>>> +        tcg_out32(s, LWSYNC);
>> lwsync can be used for all cases except store-load which requires
>> hwsync. eieio is for synchronizing memory-mapped IO which is not used by
>> TCG at all. 
> Have a look through linux/arch/powerpc/include/asm/barrier.h wherein we find
>
>   # The eieio instruction is a barrier providing an ordering ...
>   # for (a) cacheable stores ....
>
>   # However, on CPUs that don't support lwsync, lwsync actually maps to a
>   # heavy-weight sync, so smp_wmb() can be a lighter-weight eieio.
>
> And elsewhere,
>
> #if defined(__powerpc64__) || defined(CONFIG_PPC_E500MC)
> #define __SUBARCH_HAS_LWSYNC
> #endif
>
> Which suggests that ppc64 should use lwsync and ppc32 should use eieio for ST_ST.

Hmm, it's always useful to look at Linux kernel :) Looks like we should
also make this check and use eieio in place of lwsync if it's not supported.

Thanks,
Sergey