diff mbox

[6/8] tcg/tci: disable MTTCG if TCI is enabled

Message ID 20170629010300.2848-7-f4bug@amsat.org
State New
Headers show

Commit Message

Philippe Mathieu-Daudé June 29, 2017, 1:02 a.m. UTC
TCI + MTTCG cause strange errors...

  $ arm-softmmu/qemu-system-arm -machine raspi2 -cpu cortex-a7 -smp 4 -accel tcg,thread=multi -kernel kernel7.img
  qemu-system-arm: Guest expects a stronger memory ordering than the host provides
  This may cause strange/hard to debug errors
  Segmentation fault (core dumped)

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 configure | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Philippe Mathieu-Daudé July 26, 2017, 1:28 a.m. UTC | #1
Hi Peter,

I think this patch belongs to 2.10, as there is no time to fix TCI + 
MTTCG. Should I RESEND it alone with "for 2.10" subject?

One other option might be disable TCI if MTTCG enabled, but there is no 
./configure option for MTTCG while there is for TCI.

Regards,

Phil.

On 06/28/2017 10:02 PM, Philippe Mathieu-Daudé wrote:
> TCI + MTTCG cause strange errors...
> 
>    $ arm-softmmu/qemu-system-arm -machine raspi2 -cpu cortex-a7 -smp 4 -accel tcg,thread=multi -kernel kernel7.img
>    qemu-system-arm: Guest expects a stronger memory ordering than the host provides
>    This may cause strange/hard to debug errors
>    Segmentation fault (core dumped)
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>   configure | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index c571ad14e5..510f443e06 100755
> --- a/configure
> +++ b/configure
> @@ -6225,7 +6225,11 @@ fi
>   if test "$target_softmmu" = "yes" ; then
>     echo "CONFIG_SOFTMMU=y" >> $config_target_mak
>     if test "$mttcg" = "yes" ; then
> -    echo "TARGET_SUPPORTS_MTTCG=y" >> $config_target_mak
> +    if test "$tcg_interpreter" = "yes" ; then
> +        echo "TCI enabled, disabling MTTCG"
> +    else
> +        echo "TARGET_SUPPORTS_MTTCG=y" >> $config_target_mak
> +    fi
>     fi
>   fi
>   if test "$target_user_only" = "yes" ; then
>
Alex Bennée July 26, 2017, 8:02 a.m. UTC | #2
Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> Hi Peter,
>
> I think this patch belongs to 2.10, as there is no time to fix TCI +
> MTTCG. Should I RESEND it alone with "for 2.10" subject?
>
> One other option might be disable TCI if MTTCG enabled, but there is
> no ./configure option for MTTCG while there is for TCI.
>
> Regards,
>
> Phil.
>
> On 06/28/2017 10:02 PM, Philippe Mathieu-Daudé wrote:
>> TCI + MTTCG cause strange errors...
>>
>>    $ arm-softmmu/qemu-system-arm -machine raspi2 -cpu cortex-a7 -smp 4 -accel tcg,thread=multi -kernel kernel7.img
>>    qemu-system-arm: Guest expects a stronger memory ordering than the host provides
>>    This may cause strange/hard to debug errors
>>    Segmentation fault (core dumped)

So this isn't TCI enabling MTTCG by accident - this is the user forcing
it when it wouldn't otherwise be enabled. Hence the scary warning...

>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>   configure | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/configure b/configure
>> index c571ad14e5..510f443e06 100755
>> --- a/configure
>> +++ b/configure
>> @@ -6225,7 +6225,11 @@ fi
>>   if test "$target_softmmu" = "yes" ; then
>>     echo "CONFIG_SOFTMMU=y" >> $config_target_mak
>>     if test "$mttcg" = "yes" ; then
>> -    echo "TARGET_SUPPORTS_MTTCG=y" >> $config_target_mak
>> +    if test "$tcg_interpreter" = "yes" ; then
>> +        echo "TCI enabled, disabling MTTCG"
>> +    else
>> +        echo "TARGET_SUPPORTS_MTTCG=y" >> $config_target_mak
>> +    fi
>>     fi
>>   fi
>>   if test "$target_user_only" = "yes" ; then
>>


--
Alex Bennée
Philippe Mathieu-Daudé Aug. 3, 2017, 5:40 p.m. UTC | #3
On 07/26/2017 05:02 AM, Alex Bennée wrote:
> 
> Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
> 
>> Hi Peter,
>>
>> I think this patch belongs to 2.10, as there is no time to fix TCI +
>> MTTCG. Should I RESEND it alone with "for 2.10" subject?
>>
>> One other option might be disable TCI if MTTCG enabled, but there is
>> no ./configure option for MTTCG while there is for TCI.
>>
>> Regards,
>>
>> Phil.
>>
>> On 06/28/2017 10:02 PM, Philippe Mathieu-Daudé wrote:
>>> TCI + MTTCG cause strange errors...
>>>
>>>     $ arm-softmmu/qemu-system-arm -machine raspi2 -cpu cortex-a7 -smp 4 -accel tcg,thread=multi -kernel kernel7.img
>>>     qemu-system-arm: Guest expects a stronger memory ordering than the host provides
>>>     This may cause strange/hard to debug errors
>>>     Segmentation fault (core dumped)
> 
> So this isn't TCI enabling MTTCG by accident - this is the user forcing
> it when it wouldn't otherwise be enabled. Hence the scary warning...

Hmmm this is the user enabling TCI, I'm not sure about "forcing it".
Now MTTCG is enforced by the ./configure and there is no option to 
disable it.

I posted this patch after realizing there are more TCI users/testers 
than just Stefan Weil, Jaroslaw Pelczar and me. Peter commented about 
http://qira.me/ which doesn't look very active.
(see this thread: 
http://lists.nongnu.org/archive/html/qemu-devel/2017-06/threads.html#06528 )

Thinking about eventual downstream project using the TCI part of QEMU 
and trying to stay up-to-date with upstream, this patch might save 
headaches.

Since there is no formal yes/no to this patch I'm still wondering if I 
should track it or no.

Regards,

Phil.

>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>>    configure | 6 +++++-
>>>    1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/configure b/configure
>>> index c571ad14e5..510f443e06 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -6225,7 +6225,11 @@ fi
>>>    if test "$target_softmmu" = "yes" ; then
>>>      echo "CONFIG_SOFTMMU=y" >> $config_target_mak
>>>      if test "$mttcg" = "yes" ; then
>>> -    echo "TARGET_SUPPORTS_MTTCG=y" >> $config_target_mak
>>> +    if test "$tcg_interpreter" = "yes" ; then
>>> +        echo "TCI enabled, disabling MTTCG"
>>> +    else
>>> +        echo "TARGET_SUPPORTS_MTTCG=y" >> $config_target_mak
>>> +    fi
>>>      fi
>>>    fi
>>>    if test "$target_user_only" = "yes" ; then
>>>
> 
> 
> --
> Alex Bennée
>
Richard Henderson Aug. 3, 2017, 5:56 p.m. UTC | #4
On 08/03/2017 10:40 AM, Philippe Mathieu-Daudé wrote:
> On 07/26/2017 05:02 AM, Alex Bennée wrote:
>>
>> Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
>>
>>> Hi Peter,
>>>
>>> I think this patch belongs to 2.10, as there is no time to fix TCI +
>>> MTTCG. Should I RESEND it alone with "for 2.10" subject?
>>>
>>> One other option might be disable TCI if MTTCG enabled, but there is
>>> no ./configure option for MTTCG while there is for TCI.
>>>
>>> Regards,
>>>
>>> Phil.
>>>
>>> On 06/28/2017 10:02 PM, Philippe Mathieu-Daudé wrote:
>>>> TCI + MTTCG cause strange errors...
>>>>
>>>>     $ arm-softmmu/qemu-system-arm -machine raspi2 -cpu cortex-a7 -smp 4
>>>> -accel tcg,thread=multi -kernel kernel7.img
>>>>     qemu-system-arm: Guest expects a stronger memory ordering than the host
>>>> provides
>>>>     This may cause strange/hard to debug errors
>>>>     Segmentation fault (core dumped)
>>
>> So this isn't TCI enabling MTTCG by accident - this is the user forcing
>> it when it wouldn't otherwise be enabled. Hence the scary warning...
> 
> Hmmm this is the user enabling TCI, I'm not sure about "forcing it".
> Now MTTCG is enforced by the ./configure and there is no option to disable it.

Er, no.

If you write "-accel tcg" you are not forcing mttcg on.
If you write "-accel tcg,thread=multi" you are forcing it on.
If you write "-accel tcg,thread=single" you are forcing it off.

MTTCG is only enabled by default if TCG_TARGET_DEFAULT_MO is defined, and in
the case of TCI, it is not.


r~
Philippe Mathieu-Daudé Aug. 3, 2017, 6:04 p.m. UTC | #5
On 08/03/2017 02:56 PM, Richard Henderson wrote:
> On 08/03/2017 10:40 AM, Philippe Mathieu-Daudé wrote:
>> On 07/26/2017 05:02 AM, Alex Bennée wrote:
>>>
>>> Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
>>>
>>>> Hi Peter,
>>>>
>>>> I think this patch belongs to 2.10, as there is no time to fix TCI +
>>>> MTTCG. Should I RESEND it alone with "for 2.10" subject?
>>>>
>>>> One other option might be disable TCI if MTTCG enabled, but there is
>>>> no ./configure option for MTTCG while there is for TCI.
>>>>
>>>> Regards,
>>>>
>>>> Phil.
>>>>
>>>> On 06/28/2017 10:02 PM, Philippe Mathieu-Daudé wrote:
>>>>> TCI + MTTCG cause strange errors...
>>>>>
>>>>>      $ arm-softmmu/qemu-system-arm -machine raspi2 -cpu cortex-a7 -smp 4
>>>>> -accel tcg,thread=multi -kernel kernel7.img
>>>>>      qemu-system-arm: Guest expects a stronger memory ordering than the host
>>>>> provides
>>>>>      This may cause strange/hard to debug errors
>>>>>      Segmentation fault (core dumped)
>>>
>>> So this isn't TCI enabling MTTCG by accident - this is the user forcing
>>> it when it wouldn't otherwise be enabled. Hence the scary warning...
>>
>> Hmmm this is the user enabling TCI, I'm not sure about "forcing it".
>> Now MTTCG is enforced by the ./configure and there is no option to disable it.
> 
> Er, no.
> 
> If you write "-accel tcg" you are not forcing mttcg on.
> If you write "-accel tcg,thread=multi" you are forcing it on.

Ok! I obviously missed this part.

> If you write "-accel tcg,thread=single" you are forcing it off.
> 
> MTTCG is only enabled by default if TCG_TARGET_DEFAULT_MO is defined, and in
> the case of TCI, it is not.

Ok, so I misunderstood Alex :) I thought about ./configure enabled while 
he was describing -accel enabled.

Thanks for pointing that out :)

> 
> 
> r~
>
diff mbox

Patch

diff --git a/configure b/configure
index c571ad14e5..510f443e06 100755
--- a/configure
+++ b/configure
@@ -6225,7 +6225,11 @@  fi
 if test "$target_softmmu" = "yes" ; then
   echo "CONFIG_SOFTMMU=y" >> $config_target_mak
   if test "$mttcg" = "yes" ; then
-    echo "TARGET_SUPPORTS_MTTCG=y" >> $config_target_mak
+    if test "$tcg_interpreter" = "yes" ; then
+        echo "TCI enabled, disabling MTTCG"
+    else
+        echo "TARGET_SUPPORTS_MTTCG=y" >> $config_target_mak
+    fi
   fi
 fi
 if test "$target_user_only" = "yes" ; then