Re: [RFC PATCH] tcg/plugins: implement a qemu_plugin_user_exit helper

["Alex Bennée" <alex.bennee@linaro.org>]

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

-- 
Alex Bennée