| Message ID | 20240612153508.1532940-9-alex.bennee@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | maintainer updates (gdbstub, plugins, time control) | expand |
Hi Alex, I noticed the new symbols lack QEMU_PLUGIN_API qualifier in include/qemu/qemu-plugin.h: - qemu_plugin_update_ns - qemu_plugin_request_time_control So it would be impossible to use those symbols on windows. I kept a reminder to send a new patch after you pulled this, but if we go to a new series, it could be as fast for you to just add this directly. Thanks, Pierrick On 6/12/24 08:35, Alex Bennée wrote: > Expose the ability to control time through the plugin API. Only one > plugin can control time so it has to request control when loaded. > There are probably more corner cases to catch here. > > From: Alex Bennée <alex.bennee@linaro.org> > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > [AJB: tweaked user-mode handling] > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Message-Id: <20240530220610.1245424-6-pierrick.bouvier@linaro.org> > > --- > plugins/next > - make qemu_plugin_update_ns a NOP in user-mode > --- > include/qemu/qemu-plugin.h | 25 +++++++++++++++++++++++++ > plugins/api.c | 35 +++++++++++++++++++++++++++++++++++ > plugins/qemu-plugins.symbols | 2 ++ > 3 files changed, 62 insertions(+) > > diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h > index 95703d8fec..db4d67529e 100644 > --- a/include/qemu/qemu-plugin.h > +++ b/include/qemu/qemu-plugin.h > @@ -661,6 +661,31 @@ void qemu_plugin_register_vcpu_mem_inline_per_vcpu( > qemu_plugin_u64 entry, > uint64_t imm); > > +/** > + * qemu_plugin_request_time_control() - request the ability to control time > + * > + * This grants the plugin the ability to control system time. Only one > + * plugin can control time so if multiple plugins request the ability > + * all but the first will fail. > + * > + * Returns an opaque handle or NULL if fails > + */ > +const void *qemu_plugin_request_time_control(void); > + > +/** > + * qemu_plugin_update_ns() - update system emulation time > + * @handle: opaque handle returned by qemu_plugin_request_time_control() > + * @time: time in nanoseconds > + * > + * This allows an appropriately authorised plugin (i.e. holding the > + * time control handle) to move system time forward to @time. For > + * user-mode emulation the time is not changed by this as all reported > + * time comes from the host kernel. > + * > + * Start time is 0. > + */ > +void qemu_plugin_update_ns(const void *handle, int64_t time); > + > typedef void > (*qemu_plugin_vcpu_syscall_cb_t)(qemu_plugin_id_t id, unsigned int vcpu_index, > int64_t num, uint64_t a1, uint64_t a2, > diff --git a/plugins/api.c b/plugins/api.c > index 6bdb26bbe3..4431a0ea7e 100644 > --- a/plugins/api.c > +++ b/plugins/api.c > @@ -39,6 +39,7 @@ > #include "qemu/main-loop.h" > #include "qemu/plugin.h" > #include "qemu/log.h" > +#include "qemu/timer.h" > #include "tcg/tcg.h" > #include "exec/exec-all.h" > #include "exec/gdbstub.h" > @@ -583,3 +584,37 @@ uint64_t qemu_plugin_u64_sum(qemu_plugin_u64 entry) > } > return total; > } > + > +/* > + * Time control > + */ > +static bool has_control; > + > +const void *qemu_plugin_request_time_control(void) > +{ > + if (!has_control) { > + has_control = true; > + return &has_control; > + } > + return NULL; > +} > + > +#ifdef CONFIG_SOFTMMU > +static void advance_virtual_time__async(CPUState *cpu, run_on_cpu_data data) > +{ > + int64_t new_time = data.host_ulong; > + qemu_clock_advance_virtual_time(new_time); > +} > +#endif > + > +void qemu_plugin_update_ns(const void *handle, int64_t new_time) > +{ > +#ifdef CONFIG_SOFTMMU > + if (handle == &has_control) { > + /* Need to execute out of cpu_exec, so bql can be locked. */ > + async_run_on_cpu(current_cpu, > + advance_virtual_time__async, > + RUN_ON_CPU_HOST_ULONG(new_time)); > + } > +#endif > +} > diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols > index aa0a77a319..ca773d8d9f 100644 > --- a/plugins/qemu-plugins.symbols > +++ b/plugins/qemu-plugins.symbols > @@ -38,6 +38,7 @@ > qemu_plugin_register_vcpu_tb_exec_cond_cb; > qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu; > qemu_plugin_register_vcpu_tb_trans_cb; > + qemu_plugin_request_time_control; > qemu_plugin_reset; > qemu_plugin_scoreboard_free; > qemu_plugin_scoreboard_find; > @@ -51,5 +52,6 @@ > qemu_plugin_u64_set; > qemu_plugin_u64_sum; > qemu_plugin_uninstall; > + qemu_plugin_update_ns; > qemu_plugin_vcpu_for_each; > };
Pierrick Bouvier <pierrick.bouvier@linaro.org> writes: > Hi Alex, > > I noticed the new symbols lack QEMU_PLUGIN_API qualifier in > include/qemu/qemu-plugin.h: > - qemu_plugin_update_ns > - qemu_plugin_request_time_control > > So it would be impossible to use those symbols on windows. > > I kept a reminder to send a new patch after you pulled this, but if we > go to a new series, it could be as fast for you to just add this > directly. Sure if you send the patch I'll just merge it into the series.
On 6/12/24 12:37, Alex Bennée wrote: > Pierrick Bouvier <pierrick.bouvier@linaro.org> writes: > >> Hi Alex, >> >> I noticed the new symbols lack QEMU_PLUGIN_API qualifier in >> include/qemu/qemu-plugin.h: >> - qemu_plugin_update_ns >> - qemu_plugin_request_time_control >> >> So it would be impossible to use those symbols on windows. >> >> I kept a reminder to send a new patch after you pulled this, but if we >> go to a new series, it could be as fast for you to just add this >> directly. > > Sure if you send the patch I'll just merge it into the series. > I posted the series <20240612195147.93121-1-pierrick.bouvier@linaro.org> with this fix, and a second for issue reported by Richard. If you could integrate those in current series, that would be perfect :) Thanks! Pierrick
On 12/6/24 17:35, Alex Bennée wrote: > Expose the ability to control time through the plugin API. Only one > plugin can control time so it has to request control when loaded. > There are probably more corner cases to catch here. > > From: Alex Bennée <alex.bennee@linaro.org> Some of your patches include this dubious From: header, maybe strip? > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> > [AJB: tweaked user-mode handling] > Signed-off-by: Alex Bennée <alex.bennee@linaro.org> > Message-Id: <20240530220610.1245424-6-pierrick.bouvier@linaro.org> > > --- > plugins/next > - make qemu_plugin_update_ns a NOP in user-mode > --- > include/qemu/qemu-plugin.h | 25 +++++++++++++++++++++++++ > plugins/api.c | 35 +++++++++++++++++++++++++++++++++++ > plugins/qemu-plugins.symbols | 2 ++ > 3 files changed, 62 insertions(+)
Philippe Mathieu-Daudé <philmd@linaro.org> writes: > On 12/6/24 17:35, Alex Bennée wrote: >> Expose the ability to control time through the plugin API. Only one >> plugin can control time so it has to request control when loaded. >> There are probably more corner cases to catch here. >> From: Alex Bennée <alex.bennee@linaro.org> > > Some of your patches include this dubious From: header, maybe strip? I think because my original RFC patches went via Pierrick before pulling back into my tree. I can clean those up. > >> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >> [AJB: tweaked user-mode handling] >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >> Message-Id: <20240530220610.1245424-6-pierrick.bouvier@linaro.org> >> --- >> plugins/next >> - make qemu_plugin_update_ns a NOP in user-mode >> --- >> include/qemu/qemu-plugin.h | 25 +++++++++++++++++++++++++ >> plugins/api.c | 35 +++++++++++++++++++++++++++++++++++ >> plugins/qemu-plugins.symbols | 2 ++ >> 3 files changed, 62 insertions(+)
On 6/13/24 08:56, Alex Bennée wrote: > Philippe Mathieu-Daudé <philmd@linaro.org> writes: > >> On 12/6/24 17:35, Alex Bennée wrote: >>> Expose the ability to control time through the plugin API. Only one >>> plugin can control time so it has to request control when loaded. >>> There are probably more corner cases to catch here. >>> From: Alex Bennée <alex.bennee@linaro.org> >> >> Some of your patches include this dubious From: header, maybe strip? > > I think because my original RFC patches went via Pierrick before pulling > back into my tree. I can clean those up. > To be honest, I don't remember why I added those. Either I saw that in another series, or it was asked explicitely, but you can remove this for sure. >> >>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> >>> [AJB: tweaked user-mode handling] >>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> >>> Message-Id: <20240530220610.1245424-6-pierrick.bouvier@linaro.org> >>> --- >>> plugins/next >>> - make qemu_plugin_update_ns a NOP in user-mode >>> --- >>> include/qemu/qemu-plugin.h | 25 +++++++++++++++++++++++++ >>> plugins/api.c | 35 +++++++++++++++++++++++++++++++++++ >>> plugins/qemu-plugins.symbols | 2 ++ >>> 3 files changed, 62 insertions(+) >
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h index 95703d8fec..db4d67529e 100644 --- a/include/qemu/qemu-plugin.h +++ b/include/qemu/qemu-plugin.h @@ -661,6 +661,31 @@ void qemu_plugin_register_vcpu_mem_inline_per_vcpu( qemu_plugin_u64 entry, uint64_t imm); +/** + * qemu_plugin_request_time_control() - request the ability to control time + * + * This grants the plugin the ability to control system time. Only one + * plugin can control time so if multiple plugins request the ability + * all but the first will fail. + * + * Returns an opaque handle or NULL if fails + */ +const void *qemu_plugin_request_time_control(void); + +/** + * qemu_plugin_update_ns() - update system emulation time + * @handle: opaque handle returned by qemu_plugin_request_time_control() + * @time: time in nanoseconds + * + * This allows an appropriately authorised plugin (i.e. holding the + * time control handle) to move system time forward to @time. For + * user-mode emulation the time is not changed by this as all reported + * time comes from the host kernel. + * + * Start time is 0. + */ +void qemu_plugin_update_ns(const void *handle, int64_t time); + typedef void (*qemu_plugin_vcpu_syscall_cb_t)(qemu_plugin_id_t id, unsigned int vcpu_index, int64_t num, uint64_t a1, uint64_t a2, diff --git a/plugins/api.c b/plugins/api.c index 6bdb26bbe3..4431a0ea7e 100644 --- a/plugins/api.c +++ b/plugins/api.c @@ -39,6 +39,7 @@ #include "qemu/main-loop.h" #include "qemu/plugin.h" #include "qemu/log.h" +#include "qemu/timer.h" #include "tcg/tcg.h" #include "exec/exec-all.h" #include "exec/gdbstub.h" @@ -583,3 +584,37 @@ uint64_t qemu_plugin_u64_sum(qemu_plugin_u64 entry) } return total; } + +/* + * Time control + */ +static bool has_control; + +const void *qemu_plugin_request_time_control(void) +{ + if (!has_control) { + has_control = true; + return &has_control; + } + return NULL; +} + +#ifdef CONFIG_SOFTMMU +static void advance_virtual_time__async(CPUState *cpu, run_on_cpu_data data) +{ + int64_t new_time = data.host_ulong; + qemu_clock_advance_virtual_time(new_time); +} +#endif + +void qemu_plugin_update_ns(const void *handle, int64_t new_time) +{ +#ifdef CONFIG_SOFTMMU + if (handle == &has_control) { + /* Need to execute out of cpu_exec, so bql can be locked. */ + async_run_on_cpu(current_cpu, + advance_virtual_time__async, + RUN_ON_CPU_HOST_ULONG(new_time)); + } +#endif +} diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols index aa0a77a319..ca773d8d9f 100644 --- a/plugins/qemu-plugins.symbols +++ b/plugins/qemu-plugins.symbols @@ -38,6 +38,7 @@ qemu_plugin_register_vcpu_tb_exec_cond_cb; qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu; qemu_plugin_register_vcpu_tb_trans_cb; + qemu_plugin_request_time_control; qemu_plugin_reset; qemu_plugin_scoreboard_free; qemu_plugin_scoreboard_find; @@ -51,5 +52,6 @@ qemu_plugin_u64_set; qemu_plugin_u64_sum; qemu_plugin_uninstall; + qemu_plugin_update_ns; qemu_plugin_vcpu_for_each; };