On Mon, Jul 19, 2021, 1:22 PM Alex Bennée <alex.bennee@linaro.org> wrote:

>
> 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:?
>

Sorry I wasn't clear.

Acked-by: Warner Losh <imp@bsdimp.com>



> --
> Alex Bennée
>