Re: [Qemu-devel] [RFC v1 12/12] cpus: default MTTCG to on for 32 bit ARM on x86

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

On 06/06/16 11:58, Alex Bennée wrote:
> Sergey Fedorov <serge.fdrv@gmail.com> writes:
>
>> On 15/04/16 17:23, Alex Bennée wrote:
>>> diff --git a/cpus.c b/cpus.c
>>> index 860e2a9..daa92c7 100644
>>> --- a/cpus.c
>>> +++ b/cpus.c
>>> @@ -171,12 +171,24 @@ opts_init(tcg_register_config);
>>>
>>>  static bool default_mttcg_enabled(void)
>>>  {
>>> -    /*
>>> -     * TODO: Check if we have a chance to have MTTCG working on this guest/host.
>>> -     *       Basically is the atomic instruction implemented? Is there any
>>> -     *       memory ordering issue?
>>> +    /* Checklist for enabling MTTCG on a given frontend/backend combination
>>> +     *
>>> +     *  - Are atomics correctly modelled for an MTTCG environment
>> - Are cross-CPU manipulations safe (e.g. TLB invalidation/flush from
>> target helper)
>> - Are TCG context manipulations safe (e.g. TB invalidation from target
>> helper)
> OK
>
>>> +     *  - If the backend is weakly ordered
>>> +     *    - has the front-end implemented explicit memory ordering ops
>>> +     *    - does the back-end generate code to ensure memory ordering
>>>       */
>>> +#if defined(__i386__) || defined(__x86_64__)
>>> +    /* x86 backend is strongly ordered which helps a lot */
>>> +    #if defined(TARGET_ARM)
>>> +    return true;
>>> +    #else
>>> +    return false;
>>> +    #endif
>> Is it okay to indent preprocessor lines this way? I think preprocessor
>> lines are better to stand out from regular code and could be indented
>> like this:
>>
>> #if defined(__foo__)
>> # if defined(BAR)
>>     /* ... */
>> # else
>>     /* ... */
>> # endif
>> #else
>>     /* ... */
>> #endif
> To be honest I was expecting more push-back on this because it is such
> an ugly way of solving the problem and expressing what a default on
> means.

I could be okay as long as there are only a few options. We could also
put here some generic tests like strong/weak ordering checks and
introduce target- and host-specific functions which can tell us if we
should ever try enabling MTTCG for them.

Kind regards,
Sergey