Warner Losh <imp@bsdimp.com> writes:
> On Mon, Jul 19, 2021, 7:57 AM Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Alex Bennée <alex.bennee@linaro.org> writes:
>
> > In user-mode emulation there is a small race between preexit_cleanup
> > and exit_group() which means we may end up calling instrumented
> > instructions before the kernel reaps child threads. To solve this we
> > implement a new helper which ensures the callbacks are flushed along
> > with any translations before we let the host do it's a thing.
> >
> > While we are at it make the documentation of
> > qemu_plugin_register_atexit_cb clearer as to what the user can expect.
> >
> <snip>
> >
> > +/*
> > + * Handle exit from linux-user. Unlike the normal atexit() mechanism
> > + * we need to handle the clean-up manually as it's possible threads
> > + * are still running. We need to remove all callbacks from code
> > + * generation, flush the current translations and then we can safely
> > + * trigger the exit callbacks.
> > + */
> > +
> > +void qemu_plugin_user_exit(void)
> > +{
> > + enum qemu_plugin_event ev;
> > +
> > + QEMU_LOCK_GUARD(&plugin.lock);
> > +
> > + start_exclusive();
> > +
> > + /* un-register all callbacks except the final AT_EXIT one */
> > + for (ev = 0; ev < QEMU_PLUGIN_EV_MAX; ev++) {
> > + if (ev != QEMU_PLUGIN_EV_ATEXIT) {
> > + struct qemu_plugin_ctx *ctx;
> > + QTAILQ_FOREACH(ctx, &plugin.ctxs, entry) {
> > + plugin_unregister_cb__locked(ctx, ev);
> > + }
> > + }
> > + }
> > +
> > + tb_flush(current_cpu);
>
> We also need to disable memory helpers during the exclusive period as
> that is another route into a callback:
> --8<---------------cut here---------------start------------->8---
> modified plugins/core.c
> @@ -498,6 +499,7 @@ void qemu_plugin_register_atexit_cb(qemu_plugin_id_t id,
> void qemu_plugin_user_exit(void)
> {
> enum qemu_plugin_event ev;
> + CPUState *cpu;
>
> QEMU_LOCK_GUARD(&plugin.lock);
>
> @@ -514,6 +516,11 @@ void qemu_plugin_user_exit(void)
> }
>
> tb_flush(current_cpu);
> +
> + CPU_FOREACH(cpu) {
> + qemu_plugin_disable_mem_helpers(cpu);
> + }
> +
> end_exclusive();
>
> /* now it's safe to handle the exit case */
> --8<---------------cut here---------------end--------------->8---
>
> I think both of these are find from a bsd-user point of view.
Acked-by: or Reviewed-by:?