On 11/4/24 19:31, Demin Han wrote:


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


It seems that you already have a wrapper script (qemu-risv64.sh). You can replace it and read command line from $@, which contains the command passed to it.

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.


If you pass a script instead, calling the same command, you can get access to command line by reading the content of $@.


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