[PATCH-for-10.1 v5 6/7] accel/tcg: Implement get_[vcpu]_stats()

Philippe Mathieu-Daudé posted 7 patches 4 months ago
There is a newer version of this series
[PATCH-for-10.1 v5 6/7] accel/tcg: Implement get_[vcpu]_stats()
Posted by Philippe Mathieu-Daudé 4 months ago
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 accel/tcg/tcg-all.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
index eaeb465dfd5..fc3f28e3532 100644
--- a/accel/tcg/tcg-all.c
+++ b/accel/tcg/tcg-all.c
@@ -236,6 +236,11 @@ static int tcg_gdbstub_supported_sstep_flags(AccelState *as)
     }
 }
 
+static void tcg_get_stats(AccelState *as, GString *buf)
+{
+    tcg_dump_stats(as, buf);
+}
+
 static void tcg_accel_class_init(ObjectClass *oc, const void *data)
 {
     AccelClass *ac = ACCEL_CLASS(oc);
@@ -243,6 +248,7 @@ static void tcg_accel_class_init(ObjectClass *oc, const void *data)
     ac->init_machine = tcg_init_machine;
     ac->cpu_common_realize = tcg_exec_realizefn;
     ac->cpu_common_unrealize = tcg_exec_unrealizefn;
+    ac->get_stats = tcg_get_stats;
     ac->allowed = &tcg_allowed;
     ac->gdbstub_supported_sstep_flags = tcg_gdbstub_supported_sstep_flags;
 
-- 
2.49.0


Re: [PATCH-for-10.1 v5 6/7] accel/tcg: Implement get_[vcpu]_stats()
Posted by Richard Henderson 4 months ago
On 7/15/25 04:40, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   accel/tcg/tcg-all.c | 6 ++++++
>   1 file changed, 6 insertions(+)

Oh, this is what causes tcg-stats to be used by user-only binaries, is it?

> 
> diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
> index eaeb465dfd5..fc3f28e3532 100644
> --- a/accel/tcg/tcg-all.c
> +++ b/accel/tcg/tcg-all.c
> @@ -236,6 +236,11 @@ static int tcg_gdbstub_supported_sstep_flags(AccelState *as)
>       }
>   }
>   
> +static void tcg_get_stats(AccelState *as, GString *buf)
> +{
> +    tcg_dump_stats(as, buf);
> +}

Is this shim preparatory for something else?
Otherwise...

> +
>   static void tcg_accel_class_init(ObjectClass *oc, const void *data)
>   {
>       AccelClass *ac = ACCEL_CLASS(oc);
> @@ -243,6 +248,7 @@ static void tcg_accel_class_init(ObjectClass *oc, const void *data)
>       ac->init_machine = tcg_init_machine;
>       ac->cpu_common_realize = tcg_exec_realizefn;
>       ac->cpu_common_unrealize = tcg_exec_unrealizefn;
> +    ac->get_stats = tcg_get_stats;

... assign tcg_dump_stats directly?


r~

>       ac->allowed = &tcg_allowed;
>       ac->gdbstub_supported_sstep_flags = tcg_gdbstub_supported_sstep_flags;
>   


Re: [PATCH-for-10.1 v5 6/7] accel/tcg: Implement get_[vcpu]_stats()
Posted by Philippe Mathieu-Daudé 4 months ago
On 15/7/25 14:48, Richard Henderson wrote:
> On 7/15/25 04:40, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   accel/tcg/tcg-all.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
> 
> Oh, this is what causes tcg-stats to be used by user-only binaries, is it?

Indeed, otherwise we'd have to use #ifdef'ry or stubs; and there is
no good reason to not dump TCG stats on user emulation (except indeed
this code path is currently unreachable there).

> 
>>
>> diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
>> index eaeb465dfd5..fc3f28e3532 100644
>> --- a/accel/tcg/tcg-all.c
>> +++ b/accel/tcg/tcg-all.c
>> @@ -236,6 +236,11 @@ static int 
>> tcg_gdbstub_supported_sstep_flags(AccelState *as)
>>       }
>>   }
>> +static void tcg_get_stats(AccelState *as, GString *buf)
>> +{
>> +    tcg_dump_stats(as, buf);
>> +}
> 
> Is this shim preparatory for something else?

No, I didn't realize during rebase this can be simplified.

> Otherwise...
> 
>> +
>>   static void tcg_accel_class_init(ObjectClass *oc, const void *data)
>>   {
>>       AccelClass *ac = ACCEL_CLASS(oc);
>> @@ -243,6 +248,7 @@ static void tcg_accel_class_init(ObjectClass *oc, 
>> const void *data)
>>       ac->init_machine = tcg_init_machine;
>>       ac->cpu_common_realize = tcg_exec_realizefn;
>>       ac->cpu_common_unrealize = tcg_exec_unrealizefn;
>> +    ac->get_stats = tcg_get_stats;
> 
> ... assign tcg_dump_stats directly?
> 
> 
> r~
> 
>>       ac->allowed = &tcg_allowed;
>>       ac->gdbstub_supported_sstep_flags = 
>> tcg_gdbstub_supported_sstep_flags;
> 


Re: [PATCH-for-10.1 v5 6/7] accel/tcg: Implement get_[vcpu]_stats()
Posted by Philippe Mathieu-Daudé 4 months ago
Maybe "accel/tcg: Implement get_stats()" is a better subject.

On 15/7/25 12:40, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   accel/tcg/tcg-all.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/accel/tcg/tcg-all.c b/accel/tcg/tcg-all.c
> index eaeb465dfd5..fc3f28e3532 100644
> --- a/accel/tcg/tcg-all.c
> +++ b/accel/tcg/tcg-all.c
> @@ -236,6 +236,11 @@ static int tcg_gdbstub_supported_sstep_flags(AccelState *as)
>       }
>   }
>   
> +static void tcg_get_stats(AccelState *as, GString *buf)
> +{
> +    tcg_dump_stats(as, buf);
> +}
> +
>   static void tcg_accel_class_init(ObjectClass *oc, const void *data)
>   {
>       AccelClass *ac = ACCEL_CLASS(oc);
> @@ -243,6 +248,7 @@ static void tcg_accel_class_init(ObjectClass *oc, const void *data)
>       ac->init_machine = tcg_init_machine;
>       ac->cpu_common_realize = tcg_exec_realizefn;
>       ac->cpu_common_unrealize = tcg_exec_unrealizefn;
> +    ac->get_stats = tcg_get_stats;
>       ac->allowed = &tcg_allowed;
>       ac->gdbstub_supported_sstep_flags = tcg_gdbstub_supported_sstep_flags;
>