> -----Original Message-----
> From: Pierrick Bouvier <pierrick.bouv...@linaro.org>
> Sent: 2024年11月5日 10:50
> To: Demin Han <demin....@starfivetech.com>; qemu-devel@nongnu.org
> Cc: alex.ben...@linaro.org; erdn...@crans.org; ma.mando...@gmail.com
> Subject: Re: [PATCH] plugins: add plugin API to get args passed to binary
> 
> On 11/4/24 18:29, Demin Han wrote:
> >
> >> -----Original Message-----
> >> From: Pierrick Bouvier <pierrick.bouv...@linaro.org>
> >> Sent: 2024年11月5日 5:22
> >> To: Demin Han <demin....@starfivetech.com>; qemu-devel@nongnu.org
> >> Cc: alex.ben...@linaro.org; erdn...@crans.org;
> ma.mando...@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?
 qemu-riscv64.conf:
        
:qemu-riscv64:M::\x7fELF\x02\x01\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\xf3\x00:\xff\xff\xff\xff\xff\xff\xff\x00\xff\xff\xff\xff\xff\xff\xff\xff\xfe\xff\xff\xff:/opt/bin/qemu-riscv64.sh:
 qemu-riscv64.sh:
        #!/bin/bash
    /opt/bin/qemu-riscv64 --plugin /opt/bin/libxxx.so -L xxx $@
 
 We have no information about binary to pass.

> 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.

We cant set TEST_SUITE_RUN_UNDER="qemu-riscv64 --plugin /xxx/libxxx.so -L xxx".
We have no any information about binary under test even binary name.
We can get binary name in plugin using qemu_plugin_path_to_binary, but can't 
solve the issue mentioned in commit msg.


> >>> 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.bouv...@linaro.org>
> >>> *发送时间:* Saturday, November 2, 2024 2:18:20 AM
> >>> *收件人:* Demin Han <demin....@starfivetech.com>;
> >> qemu-devel@nongnu.org
> >>> <qemu-devel@nongnu.org>
> >>> *抄送:* alex.ben...@linaro.org <alex.ben...@linaro.org>;
> >>> erdn...@crans.org <erdn...@crans.org>; ma.mando...@gmail.com
> >>> <ma.mando...@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....@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

Reply via email to