Re: [PATCH 2/2] plugins: add qemu_plugin_register_time_cb support

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 4/3/25 04:38, Alex Bennée wrote:
> This allows the a plugin which has control of time to supply a callback
> so it can control the reported virtual time instead of using the
> default cpu_get_clock().
> 
> Time control plugins still need to call qemu_plugin_update_ns() to
> ensure timers are moved forward.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   include/qemu/plugin-event.h |  1 +
>   include/qemu/plugin.h       |  9 +++++++++
>   include/qemu/qemu-plugin.h  | 18 ++++++++++++++++++
>   accel/tcg/tcg-accel-ops.c   |  5 +++++
>   plugins/api-system.c        |  8 ++++++++
>   plugins/core.c              | 22 ++++++++++++++++++++++
>   6 files changed, 63 insertions(+)
> 
> diff --git a/include/qemu/plugin-event.h b/include/qemu/plugin-event.h
> index 7056d8427b..ae9ec5ce85 100644
> --- a/include/qemu/plugin-event.h
> +++ b/include/qemu/plugin-event.h
> @@ -20,6 +20,7 @@ enum qemu_plugin_event {
>       QEMU_PLUGIN_EV_VCPU_SYSCALL_RET,
>       QEMU_PLUGIN_EV_FLUSH,
>       QEMU_PLUGIN_EV_ATEXIT,
> +    QEMU_PLUGIN_EV_GET_TIME,
>       QEMU_PLUGIN_EV_MAX, /* total number of plugin events we support */
>   };
>   
> diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
> index 9726a9ebf3..a9371a9a42 100644
> --- a/include/qemu/plugin.h
> +++ b/include/qemu/plugin.h
> @@ -63,6 +63,7 @@ union qemu_plugin_cb_sig {
>       qemu_plugin_vcpu_mem_cb_t        vcpu_mem;
>       qemu_plugin_vcpu_syscall_cb_t    vcpu_syscall;
>       qemu_plugin_vcpu_syscall_ret_cb_t vcpu_syscall_ret;
> +    qemu_plugin_time_cb_t            time;
>       void *generic;
>   };
>   
> @@ -175,6 +176,14 @@ void qemu_plugin_flush_cb(void);
>   
>   void qemu_plugin_atexit_cb(void);
>   
> +/**
> + * qemu_plugin_maybe_fetch_time() - fetch virtual time from plugin
> + * @tptr: pointer to int64_t for result
> + *
> + * Returns true if the plugin has set time, otherwise false
> + */
> +bool qemu_plugin_maybe_fetch_time(int64_t *tptr);
> +
>   void qemu_plugin_add_dyn_cb_arr(GArray *arr);
>   
>   static inline void qemu_plugin_disable_mem_helpers(CPUState *cpu)
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index 3a850aa216..c5f1cad8fb 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -713,6 +713,22 @@ void qemu_plugin_register_vcpu_mem_inline_per_vcpu(
>   QEMU_PLUGIN_API
>   const void *qemu_plugin_request_time_control(void);
>   
> +/**
> + * typedef qemu_plugin_vcpu_mem_cb_t - time callback function
> + * Returns time in ns (starting from 0)
> + */
> +typedef int64_t (*qemu_plugin_time_cb_t) (void);
> +
> +/**
> + * qemu_plugin_register_time_cb() - register a time callback
> + *
> + * This can only be called once a plugin has successfully called
> + * qemu_plugin_request_time_control(). The callback will get called
> + * whenever qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) is called.
> + */
> +QEMU_PLUGIN_API
> +void qemu_plugin_register_time_cb(qemu_plugin_id_t id, const void *handle, qemu_plugin_time_cb_t cb);
> +
>   /**
>    * qemu_plugin_update_ns() - update system emulation time
>    * @handle: opaque handle returned by qemu_plugin_request_time_control()
> @@ -723,6 +739,8 @@ const void *qemu_plugin_request_time_control(void);
>    * user-mode emulation the time is not changed by this as all reported
>    * time comes from the host kernel.
>    *
> + * This allows QEMU to execute any pending timers.
> + *
>    * Start time is 0.
>    */
>   QEMU_PLUGIN_API
> diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
> index 1432d1c5b1..5ed748f5cc 100644
> --- a/accel/tcg/tcg-accel-ops.c
> +++ b/accel/tcg/tcg-accel-ops.c
> @@ -33,6 +33,7 @@
>   #include "qemu/main-loop.h"
>   #include "qemu/guest-random.h"
>   #include "qemu/timer.h"
> +#include "qemu/plugin.h"
>   #include "exec/cputlb.h"
>   #include "exec/hwaddr.h"
>   #include "exec/tb-flush.h"
> @@ -199,6 +200,10 @@ static inline void tcg_remove_all_breakpoints(CPUState *cpu)
>   
>   static int64_t tcg_get_virtual_clock(void)
>   {
> +    int64_t from_plugin;
> +    if (qemu_plugin_maybe_fetch_time(&from_plugin)) {
> +        return from_plugin;
> +    }
>       return cpu_get_clock();
>   }
>   
> diff --git a/plugins/api-system.c b/plugins/api-system.c
> index cc190b167e..0f2a3eb5a6 100644
> --- a/plugins/api-system.c
> +++ b/plugins/api-system.c
> @@ -17,6 +17,7 @@
>   #include "hw/boards.h"
>   #include "qemu/plugin-memory.h"
>   #include "qemu/plugin.h"
> +#include "plugin.h"
>   
>   /*
>    * In system mode we cannot trace the binary being executed so the
> @@ -129,3 +130,10 @@ void qemu_plugin_update_ns(const void *handle, int64_t new_time)
>                            RUN_ON_CPU_HOST_ULONG(new_time));
>       }
>   }
> +
> +void qemu_plugin_register_time_cb(qemu_plugin_id_t id, const void *handle, qemu_plugin_time_cb_t cb)
> +{
> +    if (handle == &has_control) {
> +        plugin_register_cb(id, QEMU_PLUGIN_EV_GET_TIME, cb);
> +    }
> +}
> diff --git a/plugins/core.c b/plugins/core.c
> index eb9281fe54..d56b4c9d48 100644
> --- a/plugins/core.c
> +++ b/plugins/core.c
> @@ -571,6 +571,28 @@ void qemu_plugin_flush_cb(void)
>       plugin_cb__simple(QEMU_PLUGIN_EV_FLUSH);
>   }
>   
> +/*
> + * Disable CFI checks.
> + * The callback function has been loaded from an external library so we do not
> + * have type information
> + */
> +QEMU_DISABLE_CFI
> +bool qemu_plugin_maybe_fetch_time(int64_t *tptr)
> +{
> +    enum qemu_plugin_event ev = QEMU_PLUGIN_EV_GET_TIME;
> +
> +    /* there should only be one callback */
> +    if (!QLIST_EMPTY(&plugin.cb_lists[ev])) {
> +        struct qemu_plugin_cb *cb = QLIST_FIRST(&plugin.cb_lists[ev]);
> +        qemu_plugin_time_cb_t func = cb->f.generic;
> +        *tptr = func();
> +        return true;
> +    }
> +
> +    return false;
> +}
> +
> +
>   void exec_inline_op(enum plugin_dyn_cb_type type,
>                       struct qemu_plugin_inline_cb *cb,
>                       int cpu_index)

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>