accel/tcg: Remove qemu_tcg_mttcg_enabled()

[PATCH 1/2] target/riscv: Check for CF_PARALLEL instead of qemu_tcg_mttcg_enabled

Posted by Philippe Mathieu-Daudé 2 years, 7 months ago

A CPU knows whether MTTCG is enabled or not because it is
reflected in its TCG flags via the CF_PARALLEL bit.

Suggested-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/riscv/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 4035fe0e62..4dfa64af6a 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -473,7 +473,7 @@ static void rv64_veyron_v1_cpu_init(Object *obj)
 
 static void rv128_base_cpu_init(Object *obj)
 {
-    if (qemu_tcg_mttcg_enabled()) {
+    if (CPU(obj)->tcg_cflags & CF_PARALLEL) {
         /* Missing 128-bit aligned atomics */
         error_report("128-bit RISC-V currently does not work with Multi "
                      "Threaded TCG. Please use: -accel tcg,thread=single");
-- 
2.38.1

Re: [PATCH 1/2] target/riscv: Check for CF_PARALLEL instead of qemu_tcg_mttcg_enabled

Posted by Alex Bennée 2 years, 7 months ago

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> A CPU knows whether MTTCG is enabled or not because it is
> reflected in its TCG flags via the CF_PARALLEL bit.
>
> Suggested-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/riscv/cpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 4035fe0e62..4dfa64af6a 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -473,7 +473,7 @@ static void rv64_veyron_v1_cpu_init(Object *obj)
>  
>  static void rv128_base_cpu_init(Object *obj)
>  {
> -    if (qemu_tcg_mttcg_enabled()) {
> +    if (CPU(obj)->tcg_cflags & CF_PARALLEL) {

Hmm have you checked that tcg_cpu_init_cflags() has executed by this point?

>          /* Missing 128-bit aligned atomics */
>          error_report("128-bit RISC-V currently does not work with Multi "
>                       "Threaded TCG. Please use: -accel tcg,thread=single");

Not that we can do anything about it but in linux-user we start with
CF_PARALLEL unset and only set it at the point we spawn a new thread.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 1/2] target/riscv: Check for CF_PARALLEL instead of qemu_tcg_mttcg_enabled

Posted by Philippe Mathieu-Daudé 2 years, 7 months ago

On 29/6/23 18:26, Alex Bennée wrote:
> 
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> A CPU knows whether MTTCG is enabled or not because it is
>> reflected in its TCG flags via the CF_PARALLEL bit.
>>
>> Suggested-by: Alex Bennée <alex.bennee@linaro.org>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   target/riscv/cpu.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 4035fe0e62..4dfa64af6a 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -473,7 +473,7 @@ static void rv64_veyron_v1_cpu_init(Object *obj)
>>   
>>   static void rv128_base_cpu_init(Object *obj)
>>   {
>> -    if (qemu_tcg_mttcg_enabled()) {
>> +    if (CPU(obj)->tcg_cflags & CF_PARALLEL) {
> 
> Hmm have you checked that tcg_cpu_init_cflags() has executed by this point?

$arch_cpu_realize
  -> qemu_init_vcpu
      -> mttcg_start_vcpu_thread
        -> tcg_cpu_init_cflags

I'll document in the commit description.

>>           /* Missing 128-bit aligned atomics */
>>           error_report("128-bit RISC-V currently does not work with Multi "
>>                        "Threaded TCG. Please use: -accel tcg,thread=single");
> 
> Not that we can do anything about it but in linux-user we start with
> CF_PARALLEL unset and only set it at the point we spawn a new thread.

Hmm I'll give it more thinking then.

Thanks,

Phil.