Re: [PATCH] plugins: add plugin API to get args passed to binary

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 11/4/24 18:29, Demin Han wrote:
> 
>> -----Original Message-----
>> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> Sent: 2024年11月5日 5:22
>> To: Demin Han <demin.han@starfivetech.com>; qemu-devel@nongnu.org
>> Cc: alex.bennee@linaro.org; erdnaxe@crans.org; ma.mandourr@gmail.com
>> Subject: Re: [PATCH] plugins: add plugin API to get args passed to binary
>>
>> On 11/1/24 22:10, Demin Han wrote:
>>> Hi,
>>>
>>> Many benchmarks have their own build and run system, such as specint,
>>> we don’t want to change their code.
>>>
>>
>> I don't think those benchmarks (such as specint) integrate calling qemu with a
>> specific plugin on command line, so I guess you have a wrapper or something
>> where you could pass necessary information, or tweak output file, without
>> changing the benchmark itself. In case I'm wrong, feel free to correct me.
> 
> I have two methods without change test code, but they can't get args passed to binary:
> 1. for those without hook, such as specint, we can utilize binfmt_misc.
>    We may need a simple wrapper just to load plugin or set some common options and register this wrapper to binfmt_misc
> 2. for those with hook, such as llvm-test-suite, we can set TEST_SUITE_RUN_UNDER or utilize binfmt_misc
> 
> I have no idea to write a wrapper which can get args passed to binary without change code.
> If have, please give a example or some hint.
> 

If you use binfmt_misc, how are you passing the argument to use a plugin?

In the case of llvm-test-suite, you can set TEST_SUITE_RUN_UNDER to a 
wrapper adding a specific plugin, its options, and generating the log 
filename as expected.

>>> Actually the log maybe structural data such as in json format and may
>>> be output multiple log files with different statistics dimention for one run.
>>>
>>> -D can’t satisfy this.
>>
>> Indeed, it can output only a single file. If your plugin needs something more
>> advanced, you can try to output something yourself. However, a better and
>> simpler way would be to prefix lines output with a specific marker, and post
>> process your plugin trace with a custom script.
>>
>> Adding command line access to plugins does not solve any of those problems.
>>
>> I see value in what this series offer, but I don't see how it's related to the
>> current need you express.
> 
> Yes, this is not important and not most concerned.
> But if we can directly output json or yaml, it would be convenient for post-processing.
> This is a bonus for this added api.
> 

It's something you can do directly in the plugin, by outputing 
json/yaml, or any format wanted. It's probably not a feature we'll 
provide in the public API as it's a very specific need.

> Regards,
> Demin
> 
>> Regards,
>> Pierrick
>>
>>>
>>> Regard,
>>> Denin
>>>
>>> 获取 Outlook for iOS <https://aka.ms/o0ukef>
>>> ----------------------------------------------------------------------
>>> --
>>> *发件人:* Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>> *发送时间:* Saturday, November 2, 2024 2:18:20 AM
>>> *收件人:* Demin Han <demin.han@starfivetech.com>;
>> qemu-devel@nongnu.org
>>> <qemu-devel@nongnu.org>
>>> *抄送:* alex.bennee@linaro.org <alex.bennee@linaro.org>;
>>> erdnaxe@crans.org <erdnaxe@crans.org>; ma.mandourr@gmail.com
>>> <ma.mandourr@gmail.com>
>>> *主题:* Re: [PATCH] plugins: add plugin API to get args passed to binary
>>> Hi Demin,
>>>
>>> thanks for your contribution.
>>>
>>> On 11/1/24 02:00, demin.han wrote:
>>>> Why we need args?
>>>> When plugin outputs log files, only binary path can't distinguish
>>>> multiple runs if the binary passed with different args.
>>>> This is bad for CI using plugin.
>>>>
>>>
>>> Can it be solved simply by encoding this in name of log file from the
>>> CI run script?
>>> $ cmd="/usr/bin/echo Hello world"
>>> $ out_file="$(echo "$cmd" | sed -e 's/\s/_/').log"
>>> $ qemu -plugin... -d plugin -D "$out_file" $cmd
>>>
>>> I can see some good points to add this new API, but for the use case
>>> presented in commit message, I'm not sure to see what it solves.
>>>
>>>> Signed-off-by: demin.han <demin.han@starfivetech.com>
>>>> ---
>>>>     include/qemu/qemu-plugin.h   | 11 +++++++++++
>>>>     plugins/api.c                | 16 ++++++++++++++++
>>>>     plugins/qemu-plugins.symbols |  1 +
>>>>     3 files changed, 28 insertions(+)
>>>>
>>>> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
>>>> index 622c9a0232..daf75c9f5a 100644
>>>> --- a/include/qemu/qemu-plugin.h
>>>> +++ b/include/qemu/qemu-plugin.h
>>>> @@ -837,6 +837,17 @@ bool qemu_plugin_bool_parse(const char *name,
>>>> const char *val, bool *ret);
>>>>     QEMU_PLUGIN_API
>>>>     const char *qemu_plugin_path_to_binary(void);
>>>>
>>>> +/**
>>>> + * qemu_plugin_argv_to_binary() - argv to binary file being executed
>>>> + *
>>>> + * Return a string array representing the argv to the binary. For
>>>> +user-mode
>>>> + * this is the main executable's argv. For system emulation we
>>>> +currently
>>>> + * return NULL. The user should g_free() the string array once no
>>>> +longer
>>>> + * needed.
>>>> + */
>>>> +QEMU_PLUGIN_API
>>>> +const char **qemu_plugin_argv_to_binary(void);
>>>> +
>>>>     /**
>>>>      * qemu_plugin_start_code() - returns start of text segment
>>>>      *
>>>> diff --git a/plugins/api.c b/plugins/api.c  index
>>>> 24ea64e2de..fa2735db03 100644
>>>> --- a/plugins/api.c
>>>> +++ b/plugins/api.c
>>>> @@ -485,6 +485,22 @@ const char *qemu_plugin_path_to_binary(void)
>>>>         return path;
>>>>     }
>>>>
>>>> +const char **qemu_plugin_argv_to_binary(void)
>>>> +{
>>>> +    const char **argv = NULL;
>>>> +#ifdef CONFIG_USER_ONLY
>>>> +    int i, argc;
>>>> +    TaskState *ts = get_task_state(current_cpu);
>>>> +    argc = ts->bprm->argc;
>>>> +    argv = g_malloc(sizeof(char *) * (argc + 1));
>>>> +    for (i = 0; i < argc; ++i) {
>>>> +        argv[i] = g_strdup(ts->bprm->argv[i]);
>>>> +    }
>>>> +    argv[argc] = NULL;
>>>> +#endif
>>>> +    return argv;
>>>> +}
>>>> +
>>>>     uint64_t qemu_plugin_start_code(void)
>>>>     {
>>>>         uint64_t start = 0;
>>>> diff --git a/plugins/qemu-plugins.symbols
>>>> b/plugins/qemu-plugins.symbols  index 032661f9ea..532582effe 100644
>>>> --- a/plugins/qemu-plugins.symbols
>>>> +++ b/plugins/qemu-plugins.symbols
>>>> @@ -1,4 +1,5 @@
>>>>     {
>>>> +  qemu_plugin_argv_to_binary;
>>>>       qemu_plugin_bool_parse;
>>>>       qemu_plugin_end_code;
>>>>       qemu_plugin_entry_code;
>>>
>>> Regards,
>>> Pierrick