qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] bsd-user: add option to enable plugins
@ 2025-03-31 23:42 Pierrick Bouvier
  2025-03-31 23:50 ` Pierrick Bouvier
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Pierrick Bouvier @ 2025-03-31 23:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Warner Losh, richard.henderson, alex.bennee, Kyle Evans,
	Pierrick Bouvier

Nothing prevent plugins to be enabled on this platform for user
binaries, only the option in the driver is missing.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 bsd-user/main.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index fdb160bed0f..329bd1acc02 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -175,6 +175,9 @@ static void usage(void)
            "-strace           log system calls\n"
            "-trace            [[enable=]<pattern>][,events=<file>][,file=<file>]\n"
            "                  specify tracing options\n"
+#ifdef CONFIG_PLUGIN
+           "-plugin           [file=]<file>[,<argname>=<argvalue>]\n"
+#endif
            "\n"
            "Environment variables:\n"
            "QEMU_STRACE       Print system calls and arguments similar to the\n"
@@ -225,6 +228,8 @@ static void init_task_state(TaskState *ts)
     };
 }
 
+static QemuPluginList plugins = QTAILQ_HEAD_INITIALIZER(plugins);
+
 void gemu_log(const char *fmt, ...)
 {
     va_list ap;
@@ -307,6 +312,7 @@ int main(int argc, char **argv)
     cpu_model = NULL;
 
     qemu_add_opts(&qemu_trace_opts);
+    qemu_plugin_add_opts();
 
     optind = 1;
     for (;;) {
@@ -399,6 +405,11 @@ int main(int argc, char **argv)
             do_strace = 1;
         } else if (!strcmp(r, "trace")) {
             trace_opt_parse(optarg);
+#ifdef CONFIG_PLUGIN
+        } else if (!strcmp(r, "plugin")) {
+            r = argv[optind++];
+            qemu_plugin_opt_parse(r, &plugins);
+#endif
         } else if (!strcmp(r, "0")) {
             argv0 = argv[optind++];
         } else {
@@ -433,6 +444,7 @@ int main(int argc, char **argv)
         exit(1);
     }
     trace_init_file();
+    qemu_plugin_load_list(&plugins, &error_fatal);
 
     /* Zero out regs */
     memset(regs, 0, sizeof(struct target_pt_regs));
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] bsd-user: add option to enable plugins
  2025-03-31 23:42 [PATCH] bsd-user: add option to enable plugins Pierrick Bouvier
@ 2025-03-31 23:50 ` Pierrick Bouvier
  2025-04-01  6:15 ` Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 14+ messages in thread
From: Pierrick Bouvier @ 2025-03-31 23:50 UTC (permalink / raw)
  To: qemu-devel; +Cc: Warner Losh, richard.henderson, alex.bennee, Kyle Evans

On 3/31/25 16:42, Pierrick Bouvier wrote:
> Nothing prevent plugins to be enabled on this platform for user
> binaries, only the option in the driver is missing.
> 
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   bsd-user/main.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index fdb160bed0f..329bd1acc02 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -175,6 +175,9 @@ static void usage(void)
>              "-strace           log system calls\n"
>              "-trace            [[enable=]<pattern>][,events=<file>][,file=<file>]\n"
>              "                  specify tracing options\n"
> +#ifdef CONFIG_PLUGIN
> +           "-plugin           [file=]<file>[,<argname>=<argvalue>]\n"
> +#endif
>              "\n"
>              "Environment variables:\n"
>              "QEMU_STRACE       Print system calls and arguments similar to the\n"
> @@ -225,6 +228,8 @@ static void init_task_state(TaskState *ts)
>       };
>   }
>   
> +static QemuPluginList plugins = QTAILQ_HEAD_INITIALIZER(plugins);
> +
>   void gemu_log(const char *fmt, ...)
>   {
>       va_list ap;
> @@ -307,6 +312,7 @@ int main(int argc, char **argv)
>       cpu_model = NULL;
>   
>       qemu_add_opts(&qemu_trace_opts);
> +    qemu_plugin_add_opts();
>   
>       optind = 1;
>       for (;;) {
> @@ -399,6 +405,11 @@ int main(int argc, char **argv)
>               do_strace = 1;
>           } else if (!strcmp(r, "trace")) {
>               trace_opt_parse(optarg);
> +#ifdef CONFIG_PLUGIN
> +        } else if (!strcmp(r, "plugin")) {
> +            r = argv[optind++];
> +            qemu_plugin_opt_parse(r, &plugins);
> +#endif
>           } else if (!strcmp(r, "0")) {
>               argv0 = argv[optind++];
>           } else {
> @@ -433,6 +444,7 @@ int main(int argc, char **argv)
>           exit(1);
>       }
>       trace_init_file();
> +    qemu_plugin_load_list(&plugins, &error_fatal);
>   
>       /* Zero out regs */
>       memset(regs, 0, sizeof(struct target_pt_regs));

For BSD folks,

if you're not familiar with plugins, you can try this command:
$ ./configure && ninja -C build
# plugins are built by default
$ ./build/qemu-x86_64 \
  -plugin ./build/contrib/plugins/libstoptrigger,icount=1000000 \
  -plugin ./build/tests/tcg/plugins/libinsn \
  -d plugin \
  ./build/qemu-system-x86_64 --version

Output should be something similar to:
icount reached at 0x7f2933a6a3f8, exiting
cpu 0 insns: 1000001
total insns: 1000001

It stopped after executing 1000001 instructions (libstoptrigger), and 
report how many instructions (libinsn) were executed.

Regards,
Pierrick


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] bsd-user: add option to enable plugins
  2025-03-31 23:42 [PATCH] bsd-user: add option to enable plugins Pierrick Bouvier
  2025-03-31 23:50 ` Pierrick Bouvier
@ 2025-04-01  6:15 ` Philippe Mathieu-Daudé
  2025-04-01 14:33   ` Pierrick Bouvier
  2025-04-28 19:36 ` Pierrick Bouvier
  2025-05-08 12:18 ` Philippe Mathieu-Daudé
  3 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-04-01  6:15 UTC (permalink / raw)
  To: Pierrick Bouvier, qemu-devel
  Cc: Warner Losh, richard.henderson, alex.bennee, Kyle Evans

Hi Pierrick,

On 1/4/25 01:42, Pierrick Bouvier wrote:
> Nothing prevent plugins to be enabled on this platform for user
> binaries, only the option in the driver is missing.

Per commit 903e870f245 ("plugins/api: split out binary
path/start/end/entry code") this is deliberate:

     The BSD user-mode command line is still missing -plugin.
     This can be enabled once we have reliable check-tcg tests
     working for the BSDs.

Should we enable this without test harnessing?

> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   bsd-user/main.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)

Ideally we'd have helpers for common user code in common-user/...

Anyway, since this patch does what it says:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] bsd-user: add option to enable plugins
  2025-04-01  6:15 ` Philippe Mathieu-Daudé
@ 2025-04-01 14:33   ` Pierrick Bouvier
  2025-04-01 14:44     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 14+ messages in thread
From: Pierrick Bouvier @ 2025-04-01 14:33 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Warner Losh, richard.henderson, alex.bennee, Kyle Evans

On 3/31/25 23:15, Philippe Mathieu-Daudé wrote:
> Hi Pierrick,
> 
> On 1/4/25 01:42, Pierrick Bouvier wrote:
>> Nothing prevent plugins to be enabled on this platform for user
>> binaries, only the option in the driver is missing.
> 
> Per commit 903e870f245 ("plugins/api: split out binary
> path/start/end/entry code") this is deliberate:
> 
>       The BSD user-mode command line is still missing -plugin.
>       This can be enabled once we have reliable check-tcg tests
>       working for the BSDs.
> 
> Should we enable this without test harnessing?
> 

Thanks for pointing this.

However, I don't get the argument, as the same could be said about 
system mode, which runs on BSD also, and already has plugins enabled.
The coupling between user related code and plugins is very low (just 
options parsing and init code), so I don't see why we could have a bug 
related to a specific platform only for user binaries.

So either we deactivate plugins completely for bsd binaries, or we take 
a leap of faith that it works for them.

@Alex, any further insight on this?

>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>    bsd-user/main.c | 12 ++++++++++++
>>    1 file changed, 12 insertions(+)
> 
> Ideally we'd have helpers for common user code in common-user/...
> 

Everything is already common for plugins, except adding the call to 
plugin command line option parsing function.

> Anyway, since this patch does what it says:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 

Thanks,
Pierrick


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] bsd-user: add option to enable plugins
  2025-04-01 14:33   ` Pierrick Bouvier
@ 2025-04-01 14:44     ` Philippe Mathieu-Daudé
  2025-04-01 14:59       ` Pierrick Bouvier
  0 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-04-01 14:44 UTC (permalink / raw)
  To: Pierrick Bouvier, qemu-devel
  Cc: Warner Losh, richard.henderson, alex.bennee, Kyle Evans

On 1/4/25 16:33, Pierrick Bouvier wrote:
> On 3/31/25 23:15, Philippe Mathieu-Daudé wrote:
>> Hi Pierrick,
>>
>> On 1/4/25 01:42, Pierrick Bouvier wrote:
>>> Nothing prevent plugins to be enabled on this platform for user
>>> binaries, only the option in the driver is missing.
>>
>> Per commit 903e870f245 ("plugins/api: split out binary
>> path/start/end/entry code") this is deliberate:
>>
>>       The BSD user-mode command line is still missing -plugin.
>>       This can be enabled once we have reliable check-tcg tests
>>       working for the BSDs.
>>
>> Should we enable this without test harnessing?
>>
> 
> Thanks for pointing this.
> 
> However, I don't get the argument, as the same could be said about 
> system mode, which runs on BSD also, and already has plugins enabled.
> The coupling between user related code and plugins is very low (just 
> options parsing and init code), so I don't see why we could have a bug 
> related to a specific platform only for user binaries.
> 
> So either we deactivate plugins completely for bsd binaries, or we take 
> a leap of faith that it works for them.
> 
> @Alex, any further insight on this?
> 
>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>> ---
>>>    bsd-user/main.c | 12 ++++++++++++
>>>    1 file changed, 12 insertions(+)
>>
>> Ideally we'd have helpers for common user code in common-user/...
>>
> 
> Everything is already common for plugins, except adding the call to 
> plugin command line option parsing function.

Yeah, I mean the rest of main() ;)

>> Anyway, since this patch does what it says:
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>
> 
> Thanks,
> Pierrick
> 



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] bsd-user: add option to enable plugins
  2025-04-01 14:44     ` Philippe Mathieu-Daudé
@ 2025-04-01 14:59       ` Pierrick Bouvier
  2025-04-01 18:50         ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 14+ messages in thread
From: Pierrick Bouvier @ 2025-04-01 14:59 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Warner Losh, richard.henderson, alex.bennee, Kyle Evans

On 4/1/25 07:44, Philippe Mathieu-Daudé wrote:
> On 1/4/25 16:33, Pierrick Bouvier wrote:
>> On 3/31/25 23:15, Philippe Mathieu-Daudé wrote:
>>> Hi Pierrick,
>>>
>>> On 1/4/25 01:42, Pierrick Bouvier wrote:
>>>> Nothing prevent plugins to be enabled on this platform for user
>>>> binaries, only the option in the driver is missing.
>>>
>>> Per commit 903e870f245 ("plugins/api: split out binary
>>> path/start/end/entry code") this is deliberate:
>>>
>>>        The BSD user-mode command line is still missing -plugin.
>>>        This can be enabled once we have reliable check-tcg tests
>>>        working for the BSDs.
>>>
>>> Should we enable this without test harnessing?
>>>
>>
>> Thanks for pointing this.
>>
>> However, I don't get the argument, as the same could be said about
>> system mode, which runs on BSD also, and already has plugins enabled.
>> The coupling between user related code and plugins is very low (just
>> options parsing and init code), so I don't see why we could have a bug
>> related to a specific platform only for user binaries.
>>
>> So either we deactivate plugins completely for bsd binaries, or we take
>> a leap of faith that it works for them.
>>
>> @Alex, any further insight on this?
>>
>>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>> ---
>>>>     bsd-user/main.c | 12 ++++++++++++
>>>>     1 file changed, 12 insertions(+)
>>>
>>> Ideally we'd have helpers for common user code in common-user/...
>>>
>>
>> Everything is already common for plugins, except adding the call to
>> plugin command line option parsing function.
> 
> Yeah, I mean the rest of main() ;)
> 

It's not a priority at the moment, and not blocking anything on our 
path, but yes, it would be nice to share more ideally.

>>> Anyway, since this patch does what it says:
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>
>>
>> Thanks,
>> Pierrick
>>
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] bsd-user: add option to enable plugins
  2025-04-01 14:59       ` Pierrick Bouvier
@ 2025-04-01 18:50         ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-04-01 18:50 UTC (permalink / raw)
  To: Pierrick Bouvier, qemu-devel
  Cc: Warner Losh, richard.henderson, alex.bennee, Kyle Evans

On 1/4/25 16:59, Pierrick Bouvier wrote:
> On 4/1/25 07:44, Philippe Mathieu-Daudé wrote:
>> On 1/4/25 16:33, Pierrick Bouvier wrote:
>>> On 3/31/25 23:15, Philippe Mathieu-Daudé wrote:
>>>> Hi Pierrick,
>>>>
>>>> On 1/4/25 01:42, Pierrick Bouvier wrote:
>>>>> Nothing prevent plugins to be enabled on this platform for user
>>>>> binaries, only the option in the driver is missing.
>>>>
>>>> Per commit 903e870f245 ("plugins/api: split out binary
>>>> path/start/end/entry code") this is deliberate:
>>>>
>>>>        The BSD user-mode command line is still missing -plugin.
>>>>        This can be enabled once we have reliable check-tcg tests
>>>>        working for the BSDs.
>>>>
>>>> Should we enable this without test harnessing?
>>>>
>>>
>>> Thanks for pointing this.
>>>
>>> However, I don't get the argument, as the same could be said about
>>> system mode, which runs on BSD also, and already has plugins enabled.
>>> The coupling between user related code and plugins is very low (just
>>> options parsing and init code), so I don't see why we could have a bug
>>> related to a specific platform only for user binaries.
>>>
>>> So either we deactivate plugins completely for bsd binaries, or we take
>>> a leap of faith that it works for them.
>>>
>>> @Alex, any further insight on this?
>>>
>>>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>>> ---
>>>>>     bsd-user/main.c | 12 ++++++++++++
>>>>>     1 file changed, 12 insertions(+)
>>>>
>>>> Ideally we'd have helpers for common user code in common-user/...
>>>>
>>>
>>> Everything is already common for plugins, except adding the call to
>>> plugin command line option parsing function.
>>
>> Yeah, I mean the rest of main() ;)
>>
> 
> It's not a priority at the moment, and not blocking anything on our 
> path, but yes, it would be nice to share more ideally.

Just to be clear, I was not asking you to do that suggestion.

>>>> Anyway, since this patch does what it says:
>>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>>
>>>
>>> Thanks,
>>> Pierrick
>>>
>>
> 



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] bsd-user: add option to enable plugins
  2025-03-31 23:42 [PATCH] bsd-user: add option to enable plugins Pierrick Bouvier
  2025-03-31 23:50 ` Pierrick Bouvier
  2025-04-01  6:15 ` Philippe Mathieu-Daudé
@ 2025-04-28 19:36 ` Pierrick Bouvier
  2025-04-28 22:57   ` Kyle Evans
  2025-05-08 12:18 ` Philippe Mathieu-Daudé
  3 siblings, 1 reply; 14+ messages in thread
From: Pierrick Bouvier @ 2025-04-28 19:36 UTC (permalink / raw)
  To: qemu-devel; +Cc: Warner Losh, richard.henderson, alex.bennee, Kyle Evans

On 3/31/25 4:42 PM, Pierrick Bouvier wrote:
> Nothing prevent plugins to be enabled on this platform for user
> binaries, only the option in the driver is missing.
> 
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   bsd-user/main.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index fdb160bed0f..329bd1acc02 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -175,6 +175,9 @@ static void usage(void)
>              "-strace           log system calls\n"
>              "-trace            [[enable=]<pattern>][,events=<file>][,file=<file>]\n"
>              "                  specify tracing options\n"
> +#ifdef CONFIG_PLUGIN
> +           "-plugin           [file=]<file>[,<argname>=<argvalue>]\n"
> +#endif
>              "\n"
>              "Environment variables:\n"
>              "QEMU_STRACE       Print system calls and arguments similar to the\n"
> @@ -225,6 +228,8 @@ static void init_task_state(TaskState *ts)
>       };
>   }
>   
> +static QemuPluginList plugins = QTAILQ_HEAD_INITIALIZER(plugins);
> +
>   void gemu_log(const char *fmt, ...)
>   {
>       va_list ap;
> @@ -307,6 +312,7 @@ int main(int argc, char **argv)
>       cpu_model = NULL;
>   
>       qemu_add_opts(&qemu_trace_opts);
> +    qemu_plugin_add_opts();
>   
>       optind = 1;
>       for (;;) {
> @@ -399,6 +405,11 @@ int main(int argc, char **argv)
>               do_strace = 1;
>           } else if (!strcmp(r, "trace")) {
>               trace_opt_parse(optarg);
> +#ifdef CONFIG_PLUGIN
> +        } else if (!strcmp(r, "plugin")) {
> +            r = argv[optind++];
> +            qemu_plugin_opt_parse(r, &plugins);
> +#endif
>           } else if (!strcmp(r, "0")) {
>               argv0 = argv[optind++];
>           } else {
> @@ -433,6 +444,7 @@ int main(int argc, char **argv)
>           exit(1);
>       }
>       trace_init_file();
> +    qemu_plugin_load_list(&plugins, &error_fatal);
>   
>       /* Zero out regs */
>       memset(regs, 0, sizeof(struct target_pt_regs));

Gentle ping on this series.
As we didn't have any feedback from BSD side, could we consider to 
enable this upstream?

Regards,
Pierrick


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] bsd-user: add option to enable plugins
  2025-04-28 19:36 ` Pierrick Bouvier
@ 2025-04-28 22:57   ` Kyle Evans
  2025-04-28 23:03     ` Pierrick Bouvier
  2025-05-05 18:38     ` Pierrick Bouvier
  0 siblings, 2 replies; 14+ messages in thread
From: Kyle Evans @ 2025-04-28 22:57 UTC (permalink / raw)
  To: Pierrick Bouvier, qemu-devel; +Cc: Warner Losh, richard.henderson, alex.bennee

On 4/28/25 14:36, Pierrick Bouvier wrote:
> On 3/31/25 4:42 PM, Pierrick Bouvier wrote:
>> Nothing prevent plugins to be enabled on this platform for user
>> binaries, only the option in the driver is missing.
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>   bsd-user/main.c | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/bsd-user/main.c b/bsd-user/main.c
>> index fdb160bed0f..329bd1acc02 100644
>> --- a/bsd-user/main.c
>> +++ b/bsd-user/main.c
>> @@ -175,6 +175,9 @@ static void usage(void)
>>              "-strace           log system calls\n"
>>              "-trace            [[enable=]<pattern>][,events=<file>] 
>> [,file=<file>]\n"
>>              "                  specify tracing options\n"
>> +#ifdef CONFIG_PLUGIN
>> +           "-plugin           [file=]<file>[,<argname>=<argvalue>]\n"
>> +#endif
>>              "\n"
>>              "Environment variables:\n"
>>              "QEMU_STRACE       Print system calls and arguments 
>> similar to the\n"
>> @@ -225,6 +228,8 @@ static void init_task_state(TaskState *ts)
>>       };
>>   }
>> +static QemuPluginList plugins = QTAILQ_HEAD_INITIALIZER(plugins);
>> +
>>   void gemu_log(const char *fmt, ...)
>>   {
>>       va_list ap;
>> @@ -307,6 +312,7 @@ int main(int argc, char **argv)
>>       cpu_model = NULL;
>>       qemu_add_opts(&qemu_trace_opts);
>> +    qemu_plugin_add_opts();
>>       optind = 1;
>>       for (;;) {
>> @@ -399,6 +405,11 @@ int main(int argc, char **argv)
>>               do_strace = 1;
>>           } else if (!strcmp(r, "trace")) {
>>               trace_opt_parse(optarg);
>> +#ifdef CONFIG_PLUGIN
>> +        } else if (!strcmp(r, "plugin")) {
>> +            r = argv[optind++];
>> +            qemu_plugin_opt_parse(r, &plugins);
>> +#endif
>>           } else if (!strcmp(r, "0")) {
>>               argv0 = argv[optind++];
>>           } else {
>> @@ -433,6 +444,7 @@ int main(int argc, char **argv)
>>           exit(1);
>>       }
>>       trace_init_file();
>> +    qemu_plugin_load_list(&plugins, &error_fatal);
>>       /* Zero out regs */
>>       memset(regs, 0, sizeof(struct target_pt_regs));
> 
> Gentle ping on this series.
> As we didn't have any feedback from BSD side, could we consider to 
> enable this upstream?
> 

Sorry- I have no strong opinion on plugins, but the diff looks 
incredibly reasonable and non-invasive.  I'm not really seeing any 
reason we'd object, but I don't personally feel qualified to review this 
(except as a basic human C linter- I can't imagine the added calls 
breaking anything we rely on).

Thanks,

Kyle Evans


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] bsd-user: add option to enable plugins
  2025-04-28 22:57   ` Kyle Evans
@ 2025-04-28 23:03     ` Pierrick Bouvier
  2025-05-05 18:38     ` Pierrick Bouvier
  1 sibling, 0 replies; 14+ messages in thread
From: Pierrick Bouvier @ 2025-04-28 23:03 UTC (permalink / raw)
  To: Kyle Evans, qemu-devel; +Cc: Warner Losh, richard.henderson, alex.bennee

On 4/28/25 3:57 PM, Kyle Evans wrote:
>> Gentle ping on this series.
>> As we didn't have any feedback from BSD side, could we consider to
>> enable this upstream?
>>
> 
> Sorry- I have no strong opinion on plugins, but the diff looks
> incredibly reasonable and non-invasive.  I'm not really seeing any
> reason we'd object, but I don't personally feel qualified to review this
> (except as a basic human C linter- I can't imagine the added calls
> breaking anything we rely on).
> 

Thanks for your answer Kyle.
The current issue is that check-tcg is not tested on BSD hosts for now, 
so enabling plugins support is not tested.

That said, it works on the few cases I tried, and there is nothing 
specific to BSD in tcg or plugins code, so it would be reasonable to 
enable this I think.

> Thanks,
> 
> Kyle Evans



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] bsd-user: add option to enable plugins
  2025-04-28 22:57   ` Kyle Evans
  2025-04-28 23:03     ` Pierrick Bouvier
@ 2025-05-05 18:38     ` Pierrick Bouvier
  2025-05-06  0:25       ` Warner Losh
  1 sibling, 1 reply; 14+ messages in thread
From: Pierrick Bouvier @ 2025-05-05 18:38 UTC (permalink / raw)
  To: Kyle Evans, qemu-devel; +Cc: Warner Losh, richard.henderson, alex.bennee

On 4/28/25 3:57 PM, Kyle Evans wrote:
> On 4/28/25 14:36, Pierrick Bouvier wrote:
>> On 3/31/25 4:42 PM, Pierrick Bouvier wrote:
>>> Nothing prevent plugins to be enabled on this platform for user
>>> binaries, only the option in the driver is missing.
>>>
>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>> ---
>>>    bsd-user/main.c | 12 ++++++++++++
>>>    1 file changed, 12 insertions(+)
>>>
>>> diff --git a/bsd-user/main.c b/bsd-user/main.c
>>> index fdb160bed0f..329bd1acc02 100644
>>> --- a/bsd-user/main.c
>>> +++ b/bsd-user/main.c
>>> @@ -175,6 +175,9 @@ static void usage(void)
>>>               "-strace           log system calls\n"
>>>               "-trace            [[enable=]<pattern>][,events=<file>]
>>> [,file=<file>]\n"
>>>               "                  specify tracing options\n"
>>> +#ifdef CONFIG_PLUGIN
>>> +           "-plugin           [file=]<file>[,<argname>=<argvalue>]\n"
>>> +#endif
>>>               "\n"
>>>               "Environment variables:\n"
>>>               "QEMU_STRACE       Print system calls and arguments
>>> similar to the\n"
>>> @@ -225,6 +228,8 @@ static void init_task_state(TaskState *ts)
>>>        };
>>>    }
>>> +static QemuPluginList plugins = QTAILQ_HEAD_INITIALIZER(plugins);
>>> +
>>>    void gemu_log(const char *fmt, ...)
>>>    {
>>>        va_list ap;
>>> @@ -307,6 +312,7 @@ int main(int argc, char **argv)
>>>        cpu_model = NULL;
>>>        qemu_add_opts(&qemu_trace_opts);
>>> +    qemu_plugin_add_opts();
>>>        optind = 1;
>>>        for (;;) {
>>> @@ -399,6 +405,11 @@ int main(int argc, char **argv)
>>>                do_strace = 1;
>>>            } else if (!strcmp(r, "trace")) {
>>>                trace_opt_parse(optarg);
>>> +#ifdef CONFIG_PLUGIN
>>> +        } else if (!strcmp(r, "plugin")) {
>>> +            r = argv[optind++];
>>> +            qemu_plugin_opt_parse(r, &plugins);
>>> +#endif
>>>            } else if (!strcmp(r, "0")) {
>>>                argv0 = argv[optind++];
>>>            } else {
>>> @@ -433,6 +444,7 @@ int main(int argc, char **argv)
>>>            exit(1);
>>>        }
>>>        trace_init_file();
>>> +    qemu_plugin_load_list(&plugins, &error_fatal);
>>>        /* Zero out regs */
>>>        memset(regs, 0, sizeof(struct target_pt_regs));
>>
>> Gentle ping on this series.
>> As we didn't have any feedback from BSD side, could we consider to
>> enable this upstream?
>>
> 
> Sorry- I have no strong opinion on plugins, but the diff looks
> incredibly reasonable and non-invasive.  I'm not really seeing any
> reason we'd object, but I don't personally feel qualified to review this
> (except as a basic human C linter- I can't imagine the added calls
> breaking anything we rely on).
> 

@Alex, would you be open to enable this, as it concerns plugins?

> Thanks,
> 
> Kyle Evans



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] bsd-user: add option to enable plugins
  2025-05-05 18:38     ` Pierrick Bouvier
@ 2025-05-06  0:25       ` Warner Losh
  2025-05-06  8:24         ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 14+ messages in thread
From: Warner Losh @ 2025-05-06  0:25 UTC (permalink / raw)
  To: Pierrick Bouvier; +Cc: Kyle Evans, qemu-devel, richard.henderson, alex.bennee

I'm also ignorant of plugins, but (a) if not enabling plugins is a nop
and (b) plugins either work or fail completely, then I think we can
enable them. If they cause problems when not enabled by the user,
though, we'll likely have to revert.

I don't know enough about them, though, to review.

Warner

On Mon, May 5, 2025 at 12:38 PM Pierrick Bouvier
<pierrick.bouvier@linaro.org> wrote:
>
> On 4/28/25 3:57 PM, Kyle Evans wrote:
> > On 4/28/25 14:36, Pierrick Bouvier wrote:
> >> On 3/31/25 4:42 PM, Pierrick Bouvier wrote:
> >>> Nothing prevent plugins to be enabled on this platform for user
> >>> binaries, only the option in the driver is missing.
> >>>
> >>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> >>> ---
> >>>    bsd-user/main.c | 12 ++++++++++++
> >>>    1 file changed, 12 insertions(+)
> >>>
> >>> diff --git a/bsd-user/main.c b/bsd-user/main.c
> >>> index fdb160bed0f..329bd1acc02 100644
> >>> --- a/bsd-user/main.c
> >>> +++ b/bsd-user/main.c
> >>> @@ -175,6 +175,9 @@ static void usage(void)
> >>>               "-strace           log system calls\n"
> >>>               "-trace            [[enable=]<pattern>][,events=<file>]
> >>> [,file=<file>]\n"
> >>>               "                  specify tracing options\n"
> >>> +#ifdef CONFIG_PLUGIN
> >>> +           "-plugin           [file=]<file>[,<argname>=<argvalue>]\n"
> >>> +#endif
> >>>               "\n"
> >>>               "Environment variables:\n"
> >>>               "QEMU_STRACE       Print system calls and arguments
> >>> similar to the\n"
> >>> @@ -225,6 +228,8 @@ static void init_task_state(TaskState *ts)
> >>>        };
> >>>    }
> >>> +static QemuPluginList plugins = QTAILQ_HEAD_INITIALIZER(plugins);
> >>> +
> >>>    void gemu_log(const char *fmt, ...)
> >>>    {
> >>>        va_list ap;
> >>> @@ -307,6 +312,7 @@ int main(int argc, char **argv)
> >>>        cpu_model = NULL;
> >>>        qemu_add_opts(&qemu_trace_opts);
> >>> +    qemu_plugin_add_opts();
> >>>        optind = 1;
> >>>        for (;;) {
> >>> @@ -399,6 +405,11 @@ int main(int argc, char **argv)
> >>>                do_strace = 1;
> >>>            } else if (!strcmp(r, "trace")) {
> >>>                trace_opt_parse(optarg);
> >>> +#ifdef CONFIG_PLUGIN
> >>> +        } else if (!strcmp(r, "plugin")) {
> >>> +            r = argv[optind++];
> >>> +            qemu_plugin_opt_parse(r, &plugins);
> >>> +#endif
> >>>            } else if (!strcmp(r, "0")) {
> >>>                argv0 = argv[optind++];
> >>>            } else {
> >>> @@ -433,6 +444,7 @@ int main(int argc, char **argv)
> >>>            exit(1);
> >>>        }
> >>>        trace_init_file();
> >>> +    qemu_plugin_load_list(&plugins, &error_fatal);
> >>>        /* Zero out regs */
> >>>        memset(regs, 0, sizeof(struct target_pt_regs));
> >>
> >> Gentle ping on this series.
> >> As we didn't have any feedback from BSD side, could we consider to
> >> enable this upstream?
> >>
> >
> > Sorry- I have no strong opinion on plugins, but the diff looks
> > incredibly reasonable and non-invasive.  I'm not really seeing any
> > reason we'd object, but I don't personally feel qualified to review this
> > (except as a basic human C linter- I can't imagine the added calls
> > breaking anything we rely on).
> >
>
> @Alex, would you be open to enable this, as it concerns plugins?
>
> > Thanks,
> >
> > Kyle Evans
>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] bsd-user: add option to enable plugins
  2025-05-06  0:25       ` Warner Losh
@ 2025-05-06  8:24         ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-05-06  8:24 UTC (permalink / raw)
  To: Warner Losh, Pierrick Bouvier
  Cc: Kyle Evans, qemu-devel, richard.henderson, alex.bennee

On 6/5/25 02:25, Warner Losh wrote:
> I'm also ignorant of plugins, but (a) if not enabling plugins is a nop
> and (b) plugins either work or fail completely, then I think we can
> enable them. If they cause problems when not enabled by the user,
> though, we'll likely have to revert.

Elsewhere in this thread Pierrick mentioned:

"... system mode, which runs on BSD ... already has plugins enabled."

> 
> I don't know enough about them, though, to review.
> 
> Warner
> 
> On Mon, May 5, 2025 at 12:38 PM Pierrick Bouvier
> <pierrick.bouvier@linaro.org> wrote:
>>
>> On 4/28/25 3:57 PM, Kyle Evans wrote:
>>> On 4/28/25 14:36, Pierrick Bouvier wrote:
>>>> On 3/31/25 4:42 PM, Pierrick Bouvier wrote:
>>>>> Nothing prevent plugins to be enabled on this platform for user
>>>>> binaries, only the option in the driver is missing.
>>>>>
>>>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>>> ---
>>>>>     bsd-user/main.c | 12 ++++++++++++
>>>>>     1 file changed, 12 insertions(+)



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] bsd-user: add option to enable plugins
  2025-03-31 23:42 [PATCH] bsd-user: add option to enable plugins Pierrick Bouvier
                   ` (2 preceding siblings ...)
  2025-04-28 19:36 ` Pierrick Bouvier
@ 2025-05-08 12:18 ` Philippe Mathieu-Daudé
  3 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-05-08 12:18 UTC (permalink / raw)
  To: Pierrick Bouvier, qemu-devel
  Cc: Warner Losh, richard.henderson, alex.bennee, Kyle Evans

On 1/4/25 01:42, Pierrick Bouvier wrote:
> Nothing prevent plugins to be enabled on this platform for user
> binaries, only the option in the driver is missing.
> 
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>   bsd-user/main.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)

Patch queued, thanks.


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2025-05-08 12:19 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-31 23:42 [PATCH] bsd-user: add option to enable plugins Pierrick Bouvier
2025-03-31 23:50 ` Pierrick Bouvier
2025-04-01  6:15 ` Philippe Mathieu-Daudé
2025-04-01 14:33   ` Pierrick Bouvier
2025-04-01 14:44     ` Philippe Mathieu-Daudé
2025-04-01 14:59       ` Pierrick Bouvier
2025-04-01 18:50         ` Philippe Mathieu-Daudé
2025-04-28 19:36 ` Pierrick Bouvier
2025-04-28 22:57   ` Kyle Evans
2025-04-28 23:03     ` Pierrick Bouvier
2025-05-05 18:38     ` Pierrick Bouvier
2025-05-06  0:25       ` Warner Losh
2025-05-06  8:24         ` Philippe Mathieu-Daudé
2025-05-08 12:18 ` Philippe Mathieu-Daudé

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).