On 5/30/2023 5:37 PM, Markus Armbruster wrote:
> Fei Wu <fei2...@intel.com> writes:
>
>> This collects all the statistics for TBStatistics, not only for the
>> whole emulation but for each TB.
>>
>> Signed-off-by: Vanderson M. do Rosario <vanderson...@gmail.com>
>> Signed-off-by: Alex Bennée <alex.ben...@linaro.org>
>> Signed-off-by: Fei Wu <fei2...@intel.com>
>> ---
>> accel/tcg/monitor.c | 20 ++++-
>> accel/tcg/tb-stats.c | 146 ++++++++++++++++++++++++++++++++++
>> accel/tcg/tcg-accel-ops.c | 7 ++
>> accel/tcg/translate-all.c | 70 +++++++++++++++-
>> accel/tcg/translator.c | 7 +-
>> include/exec/tb-stats-flags.h | 2 +
>> include/exec/tb-stats.h | 46 +++++++++++
>> include/qemu/timer.h | 6 ++
>> include/tcg/tcg.h | 28 ++++++-
>> softmmu/runstate.c | 9 +++
>> tcg/tcg.c | 88 ++++++++++++++++++--
>> tests/qtest/qmp-cmd-test.c | 3 +
>> 12 files changed, 417 insertions(+), 15 deletions(-)
>>
>> diff --git a/accel/tcg/monitor.c b/accel/tcg/monitor.c
>> index e903dd1d2e..2bc87f2642 100644
>> --- a/accel/tcg/monitor.c
>> +++ b/accel/tcg/monitor.c
>> @@ -15,6 +15,7 @@
>> #include "sysemu/cpus.h"
>> #include "sysemu/cpu-timers.h"
>> #include "sysemu/tcg.h"
>> +#include "exec/tb-stats.h"
>> #include "internal.h"
>>
>>
>> @@ -69,6 +70,11 @@ HumanReadableText *qmp_x_query_opcount(Error **errp)
>> {
>> g_autoptr(GString) buf = g_string_new("");
>>
>> + if (!tb_stats_collection_enabled()) {
>> + error_setg(errp, "TB information not being recorded.");
>
> From error_setg()'s contract in include/qapi/error.h:
>
> * The resulting message should be a single phrase, with no newline or
> * trailing punctuation.
>
> Please drop the period. Same elsewhere, not flagging it again there.
>
Got it, will do.
>> + return NULL;
>> + }
>> +
>> if (!tcg_enabled()) {
>> error_setg(errp,
>> "Opcode count information is only available with
>> accel=tcg");
>> @@ -80,11 +86,23 @@ HumanReadableText *qmp_x_query_opcount(Error **errp)
>> return human_readable_text_from_str(buf);
>> }
>>
>> +#ifdef CONFIG_TCG
>> +HumanReadableText *qmp_x_query_profile(Error **errp)
>> +{
>> + g_autoptr(GString) buf = g_string_new("");
>> +
>> + dump_jit_exec_time_info(dev_time, buf);
>> + dev_time = 0;
>> +
>> + return human_readable_text_from_str(buf);
>> +}
>> +#else
>> HumanReadableText *qmp_x_query_profile(Error **errp)
>> {
>> - error_setg(errp, "Internal profiler not compiled");
>> + error_setg(errp, "TCG should be enabled!");
>> return NULL;
>> }
>> +#endif
>
> machine.json has
>
> ##
> # @x-query-profile:
> #
> # Query TCG profiling information
> #
> # Features:
> #
> # @unstable: This command is meant for debugging.
> #
> # Returns: profile information
> #
> # Since: 6.2
> ##
> { 'command': 'x-query-profile',
> 'returns': 'HumanReadableText',
> 'if': 'CONFIG_TCG',
> 'features': [ 'unstable' ] }
>
> Not changed in this series.
>
> Note the command is conditional on CONFIG_TCG, i.e. code generated for
> it is #if defined(CONFIG_TCG).
>
> The only other use is in hmp-commands-info.hx, and it is also guarded by
> CONFIG_TCG.
>
> Therefore, your #else is unreachable. You can delete it along with...
>
OK, so qmp_x_query_profile won't get called #ifndef CONFIG_TCG, I will
remove it.
Thanks,
Fei.
>>
>> static void hmp_tcg_register(void)
>> {
>
> [...]
>
>> diff --git a/tests/qtest/qmp-cmd-test.c b/tests/qtest/qmp-cmd-test.c
>> index 73a670e8fa..749aafe4da 100644
>> --- a/tests/qtest/qmp-cmd-test.c
>> +++ b/tests/qtest/qmp-cmd-test.c
>> @@ -46,6 +46,9 @@ static int query_error_class(const char *cmd)
>> { "query-balloon", ERROR_CLASS_DEVICE_NOT_ACTIVE },
>> { "query-hotpluggable-cpus", ERROR_CLASS_GENERIC_ERROR },
>> { "query-vm-generation-id", ERROR_CLASS_GENERIC_ERROR },
>> +#ifndef CONFIG_TCG
>> + { "x-query-profile", ERROR_CLASS_GENERIC_ERROR },
>> +#endif
>
> ... this entry.
>
>> /* Only valid with a USB bus added */
>> { "x-query-usb", ERROR_CLASS_GENERIC_ERROR },
>> /* Only valid with accel=tcg */
>
