diff mbox series

[8/9] plugins: add time control API

Message ID 20240612153508.1532940-9-alex.bennee@linaro.org (mailing list archive)
State New, archived
Headers show
Series maintainer updates (gdbstub, plugins, time control) | expand

Commit Message

Alex Bennée June 12, 2024, 3:35 p.m. UTC
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(+)

Comments

Pierrick Bouvier June 12, 2024, 3:56 p.m. UTC | #1
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;
>   };
Alex Bennée June 12, 2024, 7:37 p.m. UTC | #2
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.
Pierrick Bouvier June 12, 2024, 7:54 p.m. UTC | #3
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
Philippe Mathieu-Daudé June 13, 2024, 8:57 a.m. UTC | #4
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(+)
Alex Bennée June 13, 2024, 3:56 p.m. UTC | #5
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(+)
Pierrick Bouvier June 14, 2024, 5:36 p.m. UTC | #6
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 mbox series

Patch

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;
 };