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