On Mon, Jul 19, 2021, 1:22 PM Alex Bennée <alex.ben...@linaro.org> wrote:
>
> Warner Losh <i...@bsdimp.com> writes:
>
> > On Mon, Jul 19, 2021, 7:57 AM Alex Bennée <alex.ben...@linaro.org>
> wrote:
> >
> > Alex Bennée <alex.ben...@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 <i...@bsdimp.com>
> --
> Alex Bennée
>
