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

[Alex Bennée]

Sergey Fedorov <address@hidden> writes:

> On 15/04/16 17:23, Alex Bennée wrote:
>> This makes multi-threading the default for 32 bit ARM on x86. It has
>> been tested with Debian Jessie as well as my extended KVM unit tests
>> which stress the SMC and TB invalidation code. Those tests can be found
>> at:
>>
>>   https://github.com/stsquad/kvm-unit-tests/tree/mttcg/current-tests-v5
>>
>> Signed-off-by: Alex Bennée <address@hidden>
>> ---
>>  cpus.c | 20 ++++++++++++++++----
>>  1 file changed, 16 insertions(+), 4 deletions(-)
>>
>> 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.

>
> Kind regards,
> Sergey
>
>> +#else
>> +    /* Until memory ordering implemented things will likely break */
>>      return false;
>> +#endif
>>  }
>>
>>  void qemu_tcg_configure(QemuOpts *opts)


--
Alex Bennée