[PATCH 0/9] maintainer updates (gdbstub, plugins, time control)

[PATCH 0/9] maintainer updates (gdbstub, plugins, time control)

[Alex Bennée]

Hi,

This is the current state of my maintainer trees. The gdbstub patches
are just minor clean-ups. The main feature this brings in is the
ability for plugins to control time. This has been discussed before
but represents the first time plugins can "control" the execution of
the core. The idea would be to eventually deprecate the icount auto
modes in favour of a plugin and just use icount for deterministic
execution and record/replay.

Alex.

Akihiko Odaki (1):
  plugins: Ensure register handles are not NULL

Alex Bennée (6):
  include/exec: add missing include guard comment
  gdbstub: move enums into separate header
  sysemu: add set_virtual_time to accel ops
  qtest: use cpu interface in qtest_clock_warp
  sysemu: generalise qtest_warp_clock as qemu_clock_advance_virtual_time
  plugins: add time control API

Pierrick Bouvier (2):
  qtest: move qtest_{get, set}_virtual_clock to accel/qtest/qtest.c
  contrib/plugins: add ips plugin example for cost modeling

 include/exec/gdbstub.h                        |  11 +-
 include/gdbstub/enums.h                       |  21 +++
 include/qemu/qemu-plugin.h                    |  25 +++
 include/qemu/timer.h                          |  15 ++
 include/sysemu/accel-ops.h                    |  18 +-
 include/sysemu/cpu-timers.h                   |   3 +-
 include/sysemu/qtest.h                        |   2 -
 accel/hvf/hvf-accel-ops.c                     |   2 +-
 accel/kvm/kvm-all.c                           |   2 +-
 accel/qtest/qtest.c                           |  13 ++
 accel/tcg/tcg-accel-ops.c                     |   2 +-
 contrib/plugins/ips.c                         | 164 ++++++++++++++++++
 gdbstub/user.c                                |   1 +
 monitor/hmp-cmds.c                            |   3 +-
 plugins/api.c                                 |  39 ++++-
 ...t-virtual-clock.c => cpus-virtual-clock.c} |   5 +
 system/cpus.c                                 |  11 ++
 system/qtest.c                                |  37 +---
 system/vl.c                                   |   1 +
 target/arm/hvf/hvf.c                          |   2 +-
 target/arm/hyp_gdbstub.c                      |   2 +-
 target/arm/kvm.c                              |   2 +-
 target/i386/kvm/kvm.c                         |   2 +-
 target/ppc/kvm.c                              |   2 +-
 target/s390x/kvm/kvm.c                        |   2 +-
 util/qemu-timer.c                             |  26 +++
 contrib/plugins/Makefile                      |   1 +
 plugins/qemu-plugins.symbols                  |   2 +
 stubs/meson.build                             |   2 +-
 29 files changed, 357 insertions(+), 61 deletions(-)
 create mode 100644 include/gdbstub/enums.h
 create mode 100644 contrib/plugins/ips.c
 rename stubs/{cpus-get-virtual-clock.c => cpus-virtual-clock.c} (68%)

-- 
2.39.2

[PATCH 1/9] include/exec: add missing include guard comment

[Alex Bennée]

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 include/exec/gdbstub.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index eb14b91139..008a92198a 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -144,4 +144,4 @@ void gdb_set_stop_cpu(CPUState *cpu);
 /* in gdbstub-xml.c, generated by scripts/feature_to_c.py */
 extern const GDBFeature gdb_static_features[];
 
-#endif
+#endif /* GDBSTUB_H */
-- 
2.39.2

Re: [PATCH 1/9] include/exec: add missing include guard comment

[Pierrick Bouvier]

On 6/12/24 08:35, Alex Bennée wrote:
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   include/exec/gdbstub.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
> index eb14b91139..008a92198a 100644
> --- a/include/exec/gdbstub.h
> +++ b/include/exec/gdbstub.h
> @@ -144,4 +144,4 @@ void gdb_set_stop_cpu(CPUState *cpu);
>   /* in gdbstub-xml.c, generated by scripts/feature_to_c.py */
>   extern const GDBFeature gdb_static_features[];
>   
> -#endif
> +#endif /* GDBSTUB_H */

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

Re: [PATCH 1/9] include/exec: add missing include guard comment

[Richard Henderson]

On 6/12/24 08:35, Alex Bennée wrote:
> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
> ---
>   include/exec/gdbstub.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 2/9] gdbstub: move enums into separate header

[Alex Bennée]

This is an experiment to further reduce the amount we throw into the
exec headers. It might not be as useful as I initially thought because
just under half of the users also need gdbserver_start().

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 include/exec/gdbstub.h    |  9 ---------
 include/gdbstub/enums.h   | 21 +++++++++++++++++++++
 accel/hvf/hvf-accel-ops.c |  2 +-
 accel/kvm/kvm-all.c       |  2 +-
 accel/tcg/tcg-accel-ops.c |  2 +-
 gdbstub/user.c            |  1 +
 monitor/hmp-cmds.c        |  3 ++-
 system/vl.c               |  1 +
 target/arm/hvf/hvf.c      |  2 +-
 target/arm/hyp_gdbstub.c  |  2 +-
 target/arm/kvm.c          |  2 +-
 target/i386/kvm/kvm.c     |  2 +-
 target/ppc/kvm.c          |  2 +-
 target/s390x/kvm/kvm.c    |  2 +-
 14 files changed, 34 insertions(+), 19 deletions(-)
 create mode 100644 include/gdbstub/enums.h

diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
index 008a92198a..1bd2c4ec2a 100644
--- a/include/exec/gdbstub.h
+++ b/include/exec/gdbstub.h
@@ -1,15 +1,6 @@
 #ifndef GDBSTUB_H
 #define GDBSTUB_H
 
-#define DEFAULT_GDBSTUB_PORT "1234"
-
-/* GDB breakpoint/watchpoint types */
-#define GDB_BREAKPOINT_SW        0
-#define GDB_BREAKPOINT_HW        1
-#define GDB_WATCHPOINT_WRITE     2
-#define GDB_WATCHPOINT_READ      3
-#define GDB_WATCHPOINT_ACCESS    4
-
 typedef struct GDBFeature {
     const char *xmlname;
     const char *xml;
diff --git a/include/gdbstub/enums.h b/include/gdbstub/enums.h
new file mode 100644
index 0000000000..c4d54a1d08
--- /dev/null
+++ b/include/gdbstub/enums.h
@@ -0,0 +1,21 @@
+/*
+ * gdbstub enums
+ *
+ * Copyright (c) 2024 Linaro Ltd
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef GDBSTUB_ENUMS_H
+#define GDBSTUB_ENUMS_H
+
+#define DEFAULT_GDBSTUB_PORT "1234"
+
+/* GDB breakpoint/watchpoint types */
+#define GDB_BREAKPOINT_SW        0
+#define GDB_BREAKPOINT_HW        1
+#define GDB_WATCHPOINT_WRITE     2
+#define GDB_WATCHPOINT_READ      3
+#define GDB_WATCHPOINT_ACCESS    4
+
+#endif /* GDBSTUB_ENUMS_H */
diff --git a/accel/hvf/hvf-accel-ops.c b/accel/hvf/hvf-accel-ops.c
index b2a37a2229..ac08cfb9f3 100644
--- a/accel/hvf/hvf-accel-ops.c
+++ b/accel/hvf/hvf-accel-ops.c
@@ -52,7 +52,7 @@
 #include "qemu/main-loop.h"
 #include "exec/address-spaces.h"
 #include "exec/exec-all.h"
-#include "exec/gdbstub.h"
+#include "gdbstub/enums.h"
 #include "sysemu/cpus.h"
 #include "sysemu/hvf.h"
 #include "sysemu/hvf_int.h"
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 009b49de44..5680cd157e 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -27,7 +27,7 @@
 #include "hw/pci/msi.h"
 #include "hw/pci/msix.h"
 #include "hw/s390x/adapter.h"
-#include "exec/gdbstub.h"
+#include "gdbstub/enums.h"
 #include "sysemu/kvm_int.h"
 #include "sysemu/runstate.h"
 #include "sysemu/cpus.h"
diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
index 1433e38f40..3c19e68a79 100644
--- a/accel/tcg/tcg-accel-ops.c
+++ b/accel/tcg/tcg-accel-ops.c
@@ -35,7 +35,7 @@
 #include "exec/exec-all.h"
 #include "exec/hwaddr.h"
 #include "exec/tb-flush.h"
-#include "exec/gdbstub.h"
+#include "gdbstub/enums.h"
 
 #include "hw/core/cpu.h"
 
diff --git a/gdbstub/user.c b/gdbstub/user.c
index edeb72efeb..e34b58b407 100644
--- a/gdbstub/user.c
+++ b/gdbstub/user.c
@@ -18,6 +18,7 @@
 #include "exec/gdbstub.h"
 #include "gdbstub/syscalls.h"
 #include "gdbstub/user.h"
+#include "gdbstub/enums.h"
 #include "hw/core/cpu.h"
 #include "trace.h"
 #include "internals.h"
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index ea79148ee8..067152589b 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -15,8 +15,9 @@
 
 #include "qemu/osdep.h"
 #include "exec/address-spaces.h"
-#include "exec/gdbstub.h"
 #include "exec/ioport.h"
+#include "exec/gdbstub.h"
+#include "gdbstub/enums.h"
 #include "monitor/hmp.h"
 #include "qemu/help_option.h"
 #include "monitor/monitor-internal.h"
diff --git a/system/vl.c b/system/vl.c
index a3eede5fa5..cfcb674425 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -68,6 +68,7 @@
 #include "sysemu/numa.h"
 #include "sysemu/hostmem.h"
 #include "exec/gdbstub.h"
+#include "gdbstub/enums.h"
 #include "qemu/timer.h"
 #include "chardev/char.h"
 #include "qemu/bitmap.h"
diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index 45e2218be5..ef9bc42738 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -33,7 +33,7 @@
 #include "trace/trace-target_arm_hvf.h"
 #include "migration/vmstate.h"
 
-#include "exec/gdbstub.h"
+#include "gdbstub/enums.h"
 
 #define MDSCR_EL1_SS_SHIFT  0
 #define MDSCR_EL1_MDE_SHIFT 15
diff --git a/target/arm/hyp_gdbstub.c b/target/arm/hyp_gdbstub.c
index ebde2899cd..f120d55caa 100644
--- a/target/arm/hyp_gdbstub.c
+++ b/target/arm/hyp_gdbstub.c
@@ -12,7 +12,7 @@
 #include "qemu/osdep.h"
 #include "cpu.h"
 #include "internals.h"
-#include "exec/gdbstub.h"
+#include "gdbstub/enums.h"
 
 /* Maximum and current break/watch point counts */
 int max_hw_bps, max_hw_wps;
diff --git a/target/arm/kvm.c b/target/arm/kvm.c
index 7cf5cf31de..70f79eda33 100644
--- a/target/arm/kvm.c
+++ b/target/arm/kvm.c
@@ -31,7 +31,7 @@
 #include "hw/pci/pci.h"
 #include "exec/memattrs.h"
 #include "exec/address-spaces.h"
-#include "exec/gdbstub.h"
+#include "gdbstub/enums.h"
 #include "hw/boards.h"
 #include "hw/irq.h"
 #include "qapi/visitor.h"
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 912f5d5a6b..a666129f41 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -38,7 +38,7 @@
 #include "hyperv.h"
 #include "hyperv-proto.h"
 
-#include "exec/gdbstub.h"
+#include "gdbstub/enums.h"
 #include "qemu/host-utils.h"
 #include "qemu/main-loop.h"
 #include "qemu/ratelimit.h"
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 005f2239f3..2c3932200b 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -39,7 +39,7 @@
 #include "migration/qemu-file-types.h"
 #include "sysemu/watchdog.h"
 #include "trace.h"
-#include "exec/gdbstub.h"
+#include "gdbstub/enums.h"
 #include "exec/memattrs.h"
 #include "exec/ram_addr.h"
 #include "sysemu/hostmem.h"
diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
index 1b494ecc20..94181d9281 100644
--- a/target/s390x/kvm/kvm.c
+++ b/target/s390x/kvm/kvm.c
@@ -40,7 +40,7 @@
 #include "sysemu/hw_accel.h"
 #include "sysemu/runstate.h"
 #include "sysemu/device_tree.h"
-#include "exec/gdbstub.h"
+#include "gdbstub/enums.h"
 #include "exec/ram_addr.h"
 #include "trace.h"
 #include "hw/s390x/s390-pci-inst.h"
-- 
2.39.2

Re: [PATCH 2/9] gdbstub: move enums into separate header

[Pierrick Bouvier]

On 6/12/24 08:35, Alex Bennée wrote:
> This is an experiment to further reduce the amount we throw into the
> exec headers. It might not be as useful as I initially thought because
> just under half of the users also need gdbserver_start().
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   include/exec/gdbstub.h    |  9 ---------
>   include/gdbstub/enums.h   | 21 +++++++++++++++++++++
>   accel/hvf/hvf-accel-ops.c |  2 +-
>   accel/kvm/kvm-all.c       |  2 +-
>   accel/tcg/tcg-accel-ops.c |  2 +-
>   gdbstub/user.c            |  1 +
>   monitor/hmp-cmds.c        |  3 ++-
>   system/vl.c               |  1 +
>   target/arm/hvf/hvf.c      |  2 +-
>   target/arm/hyp_gdbstub.c  |  2 +-
>   target/arm/kvm.c          |  2 +-
>   target/i386/kvm/kvm.c     |  2 +-
>   target/ppc/kvm.c          |  2 +-
>   target/s390x/kvm/kvm.c    |  2 +-
>   14 files changed, 34 insertions(+), 19 deletions(-)
>   create mode 100644 include/gdbstub/enums.h
> 
> diff --git a/include/exec/gdbstub.h b/include/exec/gdbstub.h
> index 008a92198a..1bd2c4ec2a 100644
> --- a/include/exec/gdbstub.h
> +++ b/include/exec/gdbstub.h
> @@ -1,15 +1,6 @@
>   #ifndef GDBSTUB_H
>   #define GDBSTUB_H
>   
> -#define DEFAULT_GDBSTUB_PORT "1234"
> -
> -/* GDB breakpoint/watchpoint types */
> -#define GDB_BREAKPOINT_SW        0
> -#define GDB_BREAKPOINT_HW        1
> -#define GDB_WATCHPOINT_WRITE     2
> -#define GDB_WATCHPOINT_READ      3
> -#define GDB_WATCHPOINT_ACCESS    4
> -
>   typedef struct GDBFeature {
>       const char *xmlname;
>       const char *xml;
> diff --git a/include/gdbstub/enums.h b/include/gdbstub/enums.h
> new file mode 100644
> index 0000000000..c4d54a1d08
> --- /dev/null
> +++ b/include/gdbstub/enums.h
> @@ -0,0 +1,21 @@
> +/*
> + * gdbstub enums
> + *
> + * Copyright (c) 2024 Linaro Ltd
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef GDBSTUB_ENUMS_H
> +#define GDBSTUB_ENUMS_H
> +
> +#define DEFAULT_GDBSTUB_PORT "1234"
> +
> +/* GDB breakpoint/watchpoint types */
> +#define GDB_BREAKPOINT_SW        0
> +#define GDB_BREAKPOINT_HW        1
> +#define GDB_WATCHPOINT_WRITE     2
> +#define GDB_WATCHPOINT_READ      3
> +#define GDB_WATCHPOINT_ACCESS    4
> +
> +#endif /* GDBSTUB_ENUMS_H */
> diff --git a/accel/hvf/hvf-accel-ops.c b/accel/hvf/hvf-accel-ops.c
> index b2a37a2229..ac08cfb9f3 100644
> --- a/accel/hvf/hvf-accel-ops.c
> +++ b/accel/hvf/hvf-accel-ops.c
> @@ -52,7 +52,7 @@
>   #include "qemu/main-loop.h"
>   #include "exec/address-spaces.h"
>   #include "exec/exec-all.h"
> -#include "exec/gdbstub.h"
> +#include "gdbstub/enums.h"
>   #include "sysemu/cpus.h"
>   #include "sysemu/hvf.h"
>   #include "sysemu/hvf_int.h"
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 009b49de44..5680cd157e 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -27,7 +27,7 @@
>   #include "hw/pci/msi.h"
>   #include "hw/pci/msix.h"
>   #include "hw/s390x/adapter.h"
> -#include "exec/gdbstub.h"
> +#include "gdbstub/enums.h"
>   #include "sysemu/kvm_int.h"
>   #include "sysemu/runstate.h"
>   #include "sysemu/cpus.h"
> diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
> index 1433e38f40..3c19e68a79 100644
> --- a/accel/tcg/tcg-accel-ops.c
> +++ b/accel/tcg/tcg-accel-ops.c
> @@ -35,7 +35,7 @@
>   #include "exec/exec-all.h"
>   #include "exec/hwaddr.h"
>   #include "exec/tb-flush.h"
> -#include "exec/gdbstub.h"
> +#include "gdbstub/enums.h"
>   
>   #include "hw/core/cpu.h"
>   
> diff --git a/gdbstub/user.c b/gdbstub/user.c
> index edeb72efeb..e34b58b407 100644
> --- a/gdbstub/user.c
> +++ b/gdbstub/user.c
> @@ -18,6 +18,7 @@
>   #include "exec/gdbstub.h"
>   #include "gdbstub/syscalls.h"
>   #include "gdbstub/user.h"
> +#include "gdbstub/enums.h"
>   #include "hw/core/cpu.h"
>   #include "trace.h"
>   #include "internals.h"
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index ea79148ee8..067152589b 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -15,8 +15,9 @@
>   
>   #include "qemu/osdep.h"
>   #include "exec/address-spaces.h"
> -#include "exec/gdbstub.h"
>   #include "exec/ioport.h"
> +#include "exec/gdbstub.h"
> +#include "gdbstub/enums.h"
>   #include "monitor/hmp.h"
>   #include "qemu/help_option.h"
>   #include "monitor/monitor-internal.h"
> diff --git a/system/vl.c b/system/vl.c
> index a3eede5fa5..cfcb674425 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -68,6 +68,7 @@
>   #include "sysemu/numa.h"
>   #include "sysemu/hostmem.h"
>   #include "exec/gdbstub.h"
> +#include "gdbstub/enums.h"
>   #include "qemu/timer.h"
>   #include "chardev/char.h"
>   #include "qemu/bitmap.h"
> diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
> index 45e2218be5..ef9bc42738 100644
> --- a/target/arm/hvf/hvf.c
> +++ b/target/arm/hvf/hvf.c
> @@ -33,7 +33,7 @@
>   #include "trace/trace-target_arm_hvf.h"
>   #include "migration/vmstate.h"
>   
> -#include "exec/gdbstub.h"
> +#include "gdbstub/enums.h"
>   
>   #define MDSCR_EL1_SS_SHIFT  0
>   #define MDSCR_EL1_MDE_SHIFT 15
> diff --git a/target/arm/hyp_gdbstub.c b/target/arm/hyp_gdbstub.c
> index ebde2899cd..f120d55caa 100644
> --- a/target/arm/hyp_gdbstub.c
> +++ b/target/arm/hyp_gdbstub.c
> @@ -12,7 +12,7 @@
>   #include "qemu/osdep.h"
>   #include "cpu.h"
>   #include "internals.h"
> -#include "exec/gdbstub.h"
> +#include "gdbstub/enums.h"
>   
>   /* Maximum and current break/watch point counts */
>   int max_hw_bps, max_hw_wps;
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 7cf5cf31de..70f79eda33 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -31,7 +31,7 @@
>   #include "hw/pci/pci.h"
>   #include "exec/memattrs.h"
>   #include "exec/address-spaces.h"
> -#include "exec/gdbstub.h"
> +#include "gdbstub/enums.h"
>   #include "hw/boards.h"
>   #include "hw/irq.h"
>   #include "qapi/visitor.h"
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index 912f5d5a6b..a666129f41 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -38,7 +38,7 @@
>   #include "hyperv.h"
>   #include "hyperv-proto.h"
>   
> -#include "exec/gdbstub.h"
> +#include "gdbstub/enums.h"
>   #include "qemu/host-utils.h"
>   #include "qemu/main-loop.h"
>   #include "qemu/ratelimit.h"
> diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> index 005f2239f3..2c3932200b 100644
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@ -39,7 +39,7 @@
>   #include "migration/qemu-file-types.h"
>   #include "sysemu/watchdog.h"
>   #include "trace.h"
> -#include "exec/gdbstub.h"
> +#include "gdbstub/enums.h"
>   #include "exec/memattrs.h"
>   #include "exec/ram_addr.h"
>   #include "sysemu/hostmem.h"
> diff --git a/target/s390x/kvm/kvm.c b/target/s390x/kvm/kvm.c
> index 1b494ecc20..94181d9281 100644
> --- a/target/s390x/kvm/kvm.c
> +++ b/target/s390x/kvm/kvm.c
> @@ -40,7 +40,7 @@
>   #include "sysemu/hw_accel.h"
>   #include "sysemu/runstate.h"
>   #include "sysemu/device_tree.h"
> -#include "exec/gdbstub.h"
> +#include "gdbstub/enums.h"
>   #include "exec/ram_addr.h"
>   #include "trace.h"
>   #include "hw/s390x/s390-pci-inst.h"

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

Re: [PATCH 2/9] gdbstub: move enums into separate header

[Richard Henderson]

On 6/12/24 08:35, Alex Bennée wrote:
> This is an experiment to further reduce the amount we throw into the
> exec headers. It might not be as useful as I initially thought because
> just under half of the users also need gdbserver_start().
> 
> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
> ---
>   include/exec/gdbstub.h    |  9 ---------
>   include/gdbstub/enums.h   | 21 +++++++++++++++++++++
>   accel/hvf/hvf-accel-ops.c |  2 +-
>   accel/kvm/kvm-all.c       |  2 +-
>   accel/tcg/tcg-accel-ops.c |  2 +-
>   gdbstub/user.c            |  1 +
>   monitor/hmp-cmds.c        |  3 ++-
>   system/vl.c               |  1 +
>   target/arm/hvf/hvf.c      |  2 +-
>   target/arm/hyp_gdbstub.c  |  2 +-
>   target/arm/kvm.c          |  2 +-
>   target/i386/kvm/kvm.c     |  2 +-
>   target/ppc/kvm.c          |  2 +-
>   target/s390x/kvm/kvm.c    |  2 +-
>   14 files changed, 34 insertions(+), 19 deletions(-)
>   create mode 100644 include/gdbstub/enums.h

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 3/9] plugins: Ensure register handles are not NULL

[Alex Bennée]

From: Akihiko Odaki <akihiko.odaki@daynix.com>

Ensure register handles are not NULL so that a plugin can assume NULL is
invalid as a register handle.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Message-Id: <20240229-null-v1-1-e716501d981e@daynix.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 plugins/api.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/plugins/api.c b/plugins/api.c
index 5a0a7f8c71..6bdb26bbe3 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -507,7 +507,7 @@ static GArray *create_register_handles(GArray *gdbstub_regs)
         }
 
         /* Create a record for the plugin */
-        desc.handle = GINT_TO_POINTER(grd->gdb_reg);
+        desc.handle = GINT_TO_POINTER(grd->gdb_reg + 1);
         desc.name = g_intern_string(grd->name);
         desc.feature = g_intern_string(grd->feature_name);
         g_array_append_val(find_data, desc);
@@ -528,7 +528,7 @@ int qemu_plugin_read_register(struct qemu_plugin_register *reg, GByteArray *buf)
 {
     g_assert(current_cpu);
 
-    return gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg));
+    return gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) - 1);
 }
 
 struct qemu_plugin_scoreboard *qemu_plugin_scoreboard_new(size_t element_size)
-- 
2.39.2

Re: [PATCH 3/9] plugins: Ensure register handles are not NULL

[Pierrick Bouvier]

On 6/12/24 08:35, Alex Bennée wrote:
> From: Akihiko Odaki <akihiko.odaki@daynix.com>
> 
> Ensure register handles are not NULL so that a plugin can assume NULL is
> invalid as a register handle.
> 
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> Message-Id: <20240229-null-v1-1-e716501d981e@daynix.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   plugins/api.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/plugins/api.c b/plugins/api.c
> index 5a0a7f8c71..6bdb26bbe3 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -507,7 +507,7 @@ static GArray *create_register_handles(GArray *gdbstub_regs)
>           }
>   
>           /* Create a record for the plugin */
> -        desc.handle = GINT_TO_POINTER(grd->gdb_reg);
> +        desc.handle = GINT_TO_POINTER(grd->gdb_reg + 1);
>           desc.name = g_intern_string(grd->name);
>           desc.feature = g_intern_string(grd->feature_name);
>           g_array_append_val(find_data, desc);
> @@ -528,7 +528,7 @@ int qemu_plugin_read_register(struct qemu_plugin_register *reg, GByteArray *buf)
>   {
>       g_assert(current_cpu);
>   
> -    return gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg));
> +    return gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) - 1);
>   }
>   
>   struct qemu_plugin_scoreboard *qemu_plugin_scoreboard_new(size_t element_size)

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

Re: [PATCH 3/9] plugins: Ensure register handles are not NULL

[Richard Henderson]

On 6/12/24 08:35, Alex Bennée wrote:
> From: Akihiko Odaki<akihiko.odaki@daynix.com>
> 
> Ensure register handles are not NULL so that a plugin can assume NULL is
> invalid as a register handle.
> 
> Signed-off-by: Akihiko Odaki<akihiko.odaki@daynix.com>
> Message-Id:<20240229-null-v1-1-e716501d981e@daynix.com>
> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
> ---
>   plugins/api.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 4/9] sysemu: add set_virtual_time to accel ops

[Alex Bennée]

We are about to remove direct calls to individual accelerators for
this information and will need a central point for plugins to hook
into time changes.

From: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20240530220610.1245424-2-pierrick.bouvier@linaro.org>
---
 include/sysemu/accel-ops.h                     | 18 +++++++++++++++++-
 include/sysemu/cpu-timers.h                    |  3 ++-
 ...et-virtual-clock.c => cpus-virtual-clock.c} |  5 +++++
 system/cpus.c                                  | 11 +++++++++++
 stubs/meson.build                              |  2 +-
 5 files changed, 36 insertions(+), 3 deletions(-)
 rename stubs/{cpus-get-virtual-clock.c => cpus-virtual-clock.c} (68%)

diff --git a/include/sysemu/accel-ops.h b/include/sysemu/accel-ops.h
index ef91fc28bb..a088672230 100644
--- a/include/sysemu/accel-ops.h
+++ b/include/sysemu/accel-ops.h
@@ -20,7 +20,12 @@
 typedef struct AccelOpsClass AccelOpsClass;
 DECLARE_CLASS_CHECKERS(AccelOpsClass, ACCEL_OPS, TYPE_ACCEL_OPS)
 
-/* cpus.c operations interface */
+/**
+ * struct AccelOpsClass - accelerator interfaces
+ *
+ * This structure is used to abstract accelerator differences from the
+ * core CPU code. Not all have to be implemented.
+ */
 struct AccelOpsClass {
     /*< private >*/
     ObjectClass parent_class;
@@ -44,7 +49,18 @@ struct AccelOpsClass {
 
     void (*handle_interrupt)(CPUState *cpu, int mask);
 
+    /**
+     * @get_virtual_clock: fetch virtual clock
+     * @set_virtual_clock: set virtual clock
+     *
+     * These allow the timer subsystem to defer to the accelerator to
+     * fetch time. The set function is needed if the accelerator wants
+     * to track the changes to time as the timer is warped through
+     * various timer events.
+     */
     int64_t (*get_virtual_clock)(void);
+    void (*set_virtual_clock)(int64_t time);
+
     int64_t (*get_elapsed_ticks)(void);
 
     /* gdbstub hooks */
diff --git a/include/sysemu/cpu-timers.h b/include/sysemu/cpu-timers.h
index d86738a378..7bfa960fbd 100644
--- a/include/sysemu/cpu-timers.h
+++ b/include/sysemu/cpu-timers.h
@@ -96,8 +96,9 @@ int64_t cpu_get_clock(void);
 
 void qemu_timer_notify_cb(void *opaque, QEMUClockType type);
 
-/* get the VIRTUAL clock and VM elapsed ticks via the cpus accel interface */
+/* get/set VIRTUAL clock and VM elapsed ticks via the cpus accel interface */
 int64_t cpus_get_virtual_clock(void);
+void cpus_set_virtual_clock(int64_t new_time);
 int64_t cpus_get_elapsed_ticks(void);
 
 #endif /* SYSEMU_CPU_TIMERS_H */
diff --git a/stubs/cpus-get-virtual-clock.c b/stubs/cpus-virtual-clock.c
similarity index 68%
rename from stubs/cpus-get-virtual-clock.c
rename to stubs/cpus-virtual-clock.c
index fd447d53f3..af7c1a1d40 100644
--- a/stubs/cpus-get-virtual-clock.c
+++ b/stubs/cpus-virtual-clock.c
@@ -6,3 +6,8 @@ int64_t cpus_get_virtual_clock(void)
 {
     return cpu_get_clock();
 }
+
+void cpus_set_virtual_clock(int64_t new_time)
+{
+    /* do nothing */
+}
diff --git a/system/cpus.c b/system/cpus.c
index f8fa78f33d..d3640c9503 100644
--- a/system/cpus.c
+++ b/system/cpus.c
@@ -229,6 +229,17 @@ int64_t cpus_get_virtual_clock(void)
     return cpu_get_clock();
 }
 
+/*
+ * Signal the new virtual time to the accelerator. This is only needed
+ * by accelerators that need to track the changes as we warp time.
+ */
+void cpus_set_virtual_clock(int64_t new_time)
+{
+    if (cpus_accel && cpus_accel->set_virtual_clock) {
+        cpus_accel->set_virtual_clock(new_time);
+    }
+}
+
 /*
  * return the time elapsed in VM between vm_start and vm_stop.  Unless
  * icount is active, cpus_get_elapsed_ticks() uses units of the host CPU cycle
diff --git a/stubs/meson.build b/stubs/meson.build
index f15b48d01f..772a3e817d 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -29,7 +29,7 @@ endif
 if have_block or have_ga
   stub_ss.add(files('replay-tools.c'))
   # stubs for hooks in util/main-loop.c, util/async.c etc.
-  stub_ss.add(files('cpus-get-virtual-clock.c'))
+  stub_ss.add(files('cpus-virtual-clock.c'))
   stub_ss.add(files('icount.c'))
   stub_ss.add(files('graph-lock.c'))
   if linux_io_uring.found()
-- 
2.39.2

Re: [PATCH 4/9] sysemu: add set_virtual_time to accel ops

[Richard Henderson]

On 6/12/24 08:35, Alex Bennée wrote:
> We are about to remove direct calls to individual accelerators for
> this information and will need a central point for plugins to hook
> into time changes.
> 
> From: Alex Bennée<alex.bennee@linaro.org>

You don't need a from in the middle.  :-)

> Reviewed-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> Signed-off-by: Pierrick Bouvier<pierrick.bouvier@linaro.org>
> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
> Message-Id:<20240530220610.1245424-2-pierrick.bouvier@linaro.org>
> ---
>   include/sysemu/accel-ops.h                     | 18 +++++++++++++++++-
>   include/sysemu/cpu-timers.h                    |  3 ++-
>   ...et-virtual-clock.c => cpus-virtual-clock.c} |  5 +++++
>   system/cpus.c                                  | 11 +++++++++++
>   stubs/meson.build                              |  2 +-
>   5 files changed, 36 insertions(+), 3 deletions(-)
>   rename stubs/{cpus-get-virtual-clock.c => cpus-virtual-clock.c} (68%)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

[PATCH 5/9] qtest: use cpu interface in qtest_clock_warp

[Alex Bennée]

This generalises the qtest_clock_warp code to use the AccelOps
handlers for updating its own sense of time. This will make the next
patch which moves the warp code closer to pure code motion.

From: Alex Bennée <alex.bennee@linaro.org>
Acked-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20240530220610.1245424-3-pierrick.bouvier@linaro.org>
---
 include/sysemu/qtest.h | 1 +
 accel/qtest/qtest.c    | 1 +
 system/qtest.c         | 6 +++---
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/sysemu/qtest.h b/include/sysemu/qtest.h
index b5d5fd3463..45f3b7e1df 100644
--- a/include/sysemu/qtest.h
+++ b/include/sysemu/qtest.h
@@ -36,6 +36,7 @@ void qtest_server_set_send_handler(void (*send)(void *, const char *),
 void qtest_server_inproc_recv(void *opaque, const char *buf);
 
 int64_t qtest_get_virtual_clock(void);
+void qtest_set_virtual_clock(int64_t count);
 #endif
 
 #endif
diff --git a/accel/qtest/qtest.c b/accel/qtest/qtest.c
index f6056ac836..53182e6c2a 100644
--- a/accel/qtest/qtest.c
+++ b/accel/qtest/qtest.c
@@ -52,6 +52,7 @@ static void qtest_accel_ops_class_init(ObjectClass *oc, void *data)
 
     ops->create_vcpu_thread = dummy_start_vcpu_thread;
     ops->get_virtual_clock = qtest_get_virtual_clock;
+    ops->set_virtual_clock = qtest_set_virtual_clock;
 };
 
 static const TypeInfo qtest_accel_ops_type = {
diff --git a/system/qtest.c b/system/qtest.c
index 507a358f3b..5be66b0140 100644
--- a/system/qtest.c
+++ b/system/qtest.c
@@ -332,14 +332,14 @@ int64_t qtest_get_virtual_clock(void)
     return qatomic_read_i64(&qtest_clock_counter);
 }
 
-static void qtest_set_virtual_clock(int64_t count)
+void qtest_set_virtual_clock(int64_t count)
 {
     qatomic_set_i64(&qtest_clock_counter, count);
 }
 
 static void qtest_clock_warp(int64_t dest)
 {
-    int64_t clock = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+    int64_t clock = cpus_get_virtual_clock();
     AioContext *aio_context;
     assert(qtest_enabled());
     aio_context = qemu_get_aio_context();
@@ -348,7 +348,7 @@ static void qtest_clock_warp(int64_t dest)
                                                       QEMU_TIMER_ATTR_ALL);
         int64_t warp = qemu_soonest_timeout(dest - clock, deadline);
 
-        qtest_set_virtual_clock(qtest_get_virtual_clock() + warp);
+        cpus_set_virtual_clock(cpus_get_virtual_clock() + warp);
 
         qemu_clock_run_timers(QEMU_CLOCK_VIRTUAL);
         timerlist_run_timers(aio_context->tlg.tl[QEMU_CLOCK_VIRTUAL]);
-- 
2.39.2

[PATCH 6/9] sysemu: generalise qtest_warp_clock as qemu_clock_advance_virtual_time

[Alex Bennée]

Move the key functionality of moving time forward into the clock
sub-system itself. This will allow us to plumb in time control into
plugins.

From: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20240530220610.1245424-4-pierrick.bouvier@linaro.org>
---
 include/qemu/timer.h | 15 +++++++++++++++
 system/qtest.c       | 25 +++----------------------
 util/qemu-timer.c    | 26 ++++++++++++++++++++++++++
 3 files changed, 44 insertions(+), 22 deletions(-)

diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 9a366e551f..910587d829 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -245,6 +245,21 @@ bool qemu_clock_run_timers(QEMUClockType type);
  */
 bool qemu_clock_run_all_timers(void);
 
+/**
+ * qemu_clock_advance_virtual_time(): advance the virtual time tick
+ * @target: target time in nanoseconds
+ *
+ * This function is used where the control of the flow of time has
+ * been delegated to outside the clock subsystem (be it qtest, icount
+ * or some other external source). You can ask the clock system to
+ * return @early at the first expired timer.
+ *
+ * Time can only move forward, attempts to reverse time would lead to
+ * an error.
+ *
+ * Returns: new virtual time.
+ */
+int64_t qemu_clock_advance_virtual_time(int64_t dest);
 
 /*
  * QEMUTimerList
diff --git a/system/qtest.c b/system/qtest.c
index 5be66b0140..8cb98966b4 100644
--- a/system/qtest.c
+++ b/system/qtest.c
@@ -337,26 +337,6 @@ void qtest_set_virtual_clock(int64_t count)
     qatomic_set_i64(&qtest_clock_counter, count);
 }
 
-static void qtest_clock_warp(int64_t dest)
-{
-    int64_t clock = cpus_get_virtual_clock();
-    AioContext *aio_context;
-    assert(qtest_enabled());
-    aio_context = qemu_get_aio_context();
-    while (clock < dest) {
-        int64_t deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL,
-                                                      QEMU_TIMER_ATTR_ALL);
-        int64_t warp = qemu_soonest_timeout(dest - clock, deadline);
-
-        cpus_set_virtual_clock(cpus_get_virtual_clock() + warp);
-
-        qemu_clock_run_timers(QEMU_CLOCK_VIRTUAL);
-        timerlist_run_timers(aio_context->tlg.tl[QEMU_CLOCK_VIRTUAL]);
-        clock = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-    }
-    qemu_clock_notify(QEMU_CLOCK_VIRTUAL);
-}
-
 static bool (*process_command_cb)(CharBackend *chr, gchar **words);
 
 void qtest_set_command_cb(bool (*pc_cb)(CharBackend *chr, gchar **words))
@@ -751,7 +731,8 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
             ns = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL,
                                             QEMU_TIMER_ATTR_ALL);
         }
-        qtest_clock_warp(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + ns);
+        qemu_clock_advance_virtual_time(
+            qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + ns);
         qtest_send_prefix(chr);
         qtest_sendf(chr, "OK %"PRIi64"\n",
                     (int64_t)qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
@@ -777,7 +758,7 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
         g_assert(words[1]);
         ret = qemu_strtoi64(words[1], NULL, 0, &ns);
         g_assert(ret == 0);
-        qtest_clock_warp(ns);
+        qemu_clock_advance_virtual_time(ns);
         qtest_send_prefix(chr);
         qtest_sendf(chr, "OK %"PRIi64"\n",
                     (int64_t)qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
diff --git a/util/qemu-timer.c b/util/qemu-timer.c
index 6a0de33dd2..213114be68 100644
--- a/util/qemu-timer.c
+++ b/util/qemu-timer.c
@@ -645,6 +645,11 @@ int64_t qemu_clock_get_ns(QEMUClockType type)
     }
 }
 
+static void qemu_virtual_clock_set_ns(int64_t time)
+{
+    return cpus_set_virtual_clock(time);
+}
+
 void init_clocks(QEMUTimerListNotifyCB *notify_cb)
 {
     QEMUClockType type;
@@ -675,3 +680,24 @@ bool qemu_clock_run_all_timers(void)
 
     return progress;
 }
+
+int64_t qemu_clock_advance_virtual_time(int64_t dest)
+{
+    int64_t clock = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+    AioContext *aio_context;
+    aio_context = qemu_get_aio_context();
+    while (clock < dest) {
+        int64_t deadline = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL,
+                                                      QEMU_TIMER_ATTR_ALL);
+        int64_t warp = qemu_soonest_timeout(dest - clock, deadline);
+
+        qemu_virtual_clock_set_ns(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + warp);
+
+        qemu_clock_run_timers(QEMU_CLOCK_VIRTUAL);
+        timerlist_run_timers(aio_context->tlg.tl[QEMU_CLOCK_VIRTUAL]);
+        clock = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+    }
+    qemu_clock_notify(QEMU_CLOCK_VIRTUAL);
+
+    return clock;
+}
-- 
2.39.2

[PATCH 7/9] qtest: move qtest_{get, set}_virtual_clock to accel/qtest/qtest.c

[Alex Bennée]

From: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20240530220610.1245424-5-pierrick.bouvier@linaro.org>
---
 include/sysemu/qtest.h |  3 ---
 accel/qtest/qtest.c    | 12 ++++++++++++
 system/qtest.c         | 12 ------------
 3 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/include/sysemu/qtest.h b/include/sysemu/qtest.h
index 45f3b7e1df..c161d75165 100644
--- a/include/sysemu/qtest.h
+++ b/include/sysemu/qtest.h
@@ -34,9 +34,6 @@ void qtest_server_init(const char *qtest_chrdev, const char *qtest_log, Error **
 void qtest_server_set_send_handler(void (*send)(void *, const char *),
                                  void *opaque);
 void qtest_server_inproc_recv(void *opaque, const char *buf);
-
-int64_t qtest_get_virtual_clock(void);
-void qtest_set_virtual_clock(int64_t count);
 #endif
 
 #endif
diff --git a/accel/qtest/qtest.c b/accel/qtest/qtest.c
index 53182e6c2a..bf14032d29 100644
--- a/accel/qtest/qtest.c
+++ b/accel/qtest/qtest.c
@@ -24,6 +24,18 @@
 #include "qemu/main-loop.h"
 #include "hw/core/cpu.h"
 
+static int64_t qtest_clock_counter;
+
+static int64_t qtest_get_virtual_clock(void)
+{
+    return qatomic_read_i64(&qtest_clock_counter);
+}
+
+static void qtest_set_virtual_clock(int64_t count)
+{
+    qatomic_set_i64(&qtest_clock_counter, count);
+}
+
 static int qtest_init_accel(MachineState *ms)
 {
     return 0;
diff --git a/system/qtest.c b/system/qtest.c
index 8cb98966b4..12703a2045 100644
--- a/system/qtest.c
+++ b/system/qtest.c
@@ -325,18 +325,6 @@ static void qtest_irq_handler(void *opaque, int n, int level)
     }
 }
 
-static int64_t qtest_clock_counter;
-
-int64_t qtest_get_virtual_clock(void)
-{
-    return qatomic_read_i64(&qtest_clock_counter);
-}
-
-void qtest_set_virtual_clock(int64_t count)
-{
-    qatomic_set_i64(&qtest_clock_counter, count);
-}
-
 static bool (*process_command_cb)(CharBackend *chr, gchar **words);
 
 void qtest_set_command_cb(bool (*pc_cb)(CharBackend *chr, gchar **words))
-- 
2.39.2

[PATCH 8/9] plugins: add time control API

[Alex Bennée]

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

Re: [PATCH 8/9] plugins: add time control API

[Pierrick Bouvier]

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

Re: [PATCH 8/9] plugins: add time control API

[Alex Bennée]

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.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 8/9] plugins: add time control API

[Pierrick Bouvier]

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

Re: [PATCH 8/9] plugins: add time control API

[Philippe Mathieu-Daudé]

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(+)

Re: [PATCH 8/9] plugins: add time control API

[Alex Bennée]

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(+)

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 8/9] plugins: add time control API

[Pierrick Bouvier]

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(+)
> 

[PATCH 9/9] contrib/plugins: add ips plugin example for cost modeling

[Alex Bennée]

From: Pierrick Bouvier <pierrick.bouvier@linaro.org>

This plugin uses the new time control interface to make decisions
about the state of time during the emulation. The algorithm is
currently very simple. The user specifies an ips rate which applies
per core. If the core runs ahead of its allocated execution time the
plugin sleeps for a bit to let real time catch up. Either way time is
updated for the emulation as a function of total executed instructions
with some adjustments for cores that idle.

Examples
--------

Slow down execution of /bin/true:
$ num_insn=$(./build/qemu-x86_64 -plugin ./build/tests/plugin/libinsn.so -d plugin /bin/true |& grep total | sed -e 's/.*: //')
$ time ./build/qemu-x86_64 -plugin ./build/contrib/plugins/libips.so,ips=$(($num_insn/4)) /bin/true
real 4.000s

Boot a Linux kernel simulating a 250MHz cpu:
$ /build/qemu-system-x86_64 -kernel /boot/vmlinuz-6.1.0-21-amd64 -append "console=ttyS0" -plugin ./build/contrib/plugins/libips.so,ips=$((250*1000*1000)) -smp 1 -m 512
check time until kernel panic on serial0

Tested in system mode by booting a full debian system, and using:
$ sysbench cpu run
Performance decrease linearly with the given number of ips.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Message-Id: <20240530220610.1245424-7-pierrick.bouvier@linaro.org>
---
 contrib/plugins/ips.c    | 164 +++++++++++++++++++++++++++++++++++++++
 contrib/plugins/Makefile |   1 +
 2 files changed, 165 insertions(+)
 create mode 100644 contrib/plugins/ips.c

diff --git a/contrib/plugins/ips.c b/contrib/plugins/ips.c
new file mode 100644
index 0000000000..db77729264
--- /dev/null
+++ b/contrib/plugins/ips.c
@@ -0,0 +1,164 @@
+/*
+ * ips rate limiting plugin.
+ *
+ * This plugin can be used to restrict the execution of a system to a
+ * particular number of Instructions Per Second (ips). This controls
+ * time as seen by the guest so while wall-clock time may be longer
+ * from the guests point of view time will pass at the normal rate.
+ *
+ * This uses the new plugin API which allows the plugin to control
+ * system time.
+ *
+ * Copyright (c) 2023 Linaro Ltd
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include <stdio.h>
+#include <glib.h>
+#include <qemu-plugin.h>
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
+/* how many times do we update time per sec */
+#define NUM_TIME_UPDATE_PER_SEC 10
+#define NSEC_IN_ONE_SEC (1000 * 1000 * 1000)
+
+static GMutex global_state_lock;
+
+static uint64_t max_insn_per_second = 1000 * 1000 * 1000; /* ips per core, per second */
+static uint64_t max_insn_per_quantum; /* trap every N instructions */
+static int64_t virtual_time_ns; /* last set virtual time */
+
+static const void *time_handle;
+
+typedef struct {
+    uint64_t total_insn;
+    uint64_t quantum_insn; /* insn in last quantum */
+    int64_t last_quantum_time; /* time when last quantum started */
+} vCPUTime;
+
+struct qemu_plugin_scoreboard *vcpus;
+
+/* return epoch time in ns */
+static int64_t now_ns(void)
+{
+    return g_get_real_time() * 1000;
+}
+
+static uint64_t num_insn_during(int64_t elapsed_ns)
+{
+    double num_secs = elapsed_ns / (double) NSEC_IN_ONE_SEC;
+    return num_secs * (double) max_insn_per_second;
+}
+
+static int64_t time_for_insn(uint64_t num_insn)
+{
+    double num_secs = (double) num_insn / (double) max_insn_per_second;
+    return num_secs * (double) NSEC_IN_ONE_SEC;
+}
+
+static void update_system_time(vCPUTime *vcpu)
+{
+    int64_t elapsed_ns = now_ns() - vcpu->last_quantum_time;
+    uint64_t max_insn = num_insn_during(elapsed_ns);
+
+    if (vcpu->quantum_insn >= max_insn) {
+        /* this vcpu ran faster than expected, so it has to sleep */
+        uint64_t insn_advance = vcpu->quantum_insn - max_insn;
+        uint64_t time_advance_ns = time_for_insn(insn_advance);
+        int64_t sleep_us = time_advance_ns / 1000;
+        g_usleep(sleep_us);
+    }
+
+    vcpu->total_insn += vcpu->quantum_insn;
+    vcpu->quantum_insn = 0;
+    vcpu->last_quantum_time = now_ns();
+
+    /* based on total number of instructions, what should be the new time? */
+    int64_t new_virtual_time = time_for_insn(vcpu->total_insn);
+
+    g_mutex_lock(&global_state_lock);
+
+    /* Time only moves forward. Another vcpu might have updated it already. */
+    if (new_virtual_time > virtual_time_ns) {
+        qemu_plugin_update_ns(time_handle, new_virtual_time);
+        virtual_time_ns = new_virtual_time;
+    }
+
+    g_mutex_unlock(&global_state_lock);
+}
+
+static void vcpu_init(qemu_plugin_id_t id, unsigned int cpu_index)
+{
+    vCPUTime *vcpu = qemu_plugin_scoreboard_find(vcpus, cpu_index);
+    vcpu->total_insn = 0;
+    vcpu->quantum_insn = 0;
+    vcpu->last_quantum_time = now_ns();
+}
+
+static void vcpu_exit(qemu_plugin_id_t id, unsigned int cpu_index)
+{
+    vCPUTime *vcpu = qemu_plugin_scoreboard_find(vcpus, cpu_index);
+    update_system_time(vcpu);
+}
+
+static void every_quantum_insn(unsigned int cpu_index, void *udata)
+{
+    vCPUTime *vcpu = qemu_plugin_scoreboard_find(vcpus, cpu_index);
+    g_assert(vcpu->quantum_insn >= max_insn_per_quantum);
+    update_system_time(vcpu);
+}
+
+static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
+{
+    size_t n_insns = qemu_plugin_tb_n_insns(tb);
+    qemu_plugin_u64 quantum_insn =
+        qemu_plugin_scoreboard_u64_in_struct(vcpus, vCPUTime, quantum_insn);
+    /* count (and eventually trap) once per tb */
+    qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
+        tb, QEMU_PLUGIN_INLINE_ADD_U64, quantum_insn, n_insns);
+    qemu_plugin_register_vcpu_tb_exec_cond_cb(
+        tb, every_quantum_insn,
+        QEMU_PLUGIN_CB_NO_REGS, QEMU_PLUGIN_COND_GE,
+        quantum_insn, max_insn_per_quantum, NULL);
+}
+
+static void plugin_exit(qemu_plugin_id_t id, void *udata)
+{
+    qemu_plugin_scoreboard_free(vcpus);
+}
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
+                                           const qemu_info_t *info, int argc,
+                                           char **argv)
+{
+    for (int i = 0; i < argc; i++) {
+        char *opt = argv[i];
+        g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
+        if (g_strcmp0(tokens[0], "ips") == 0) {
+            max_insn_per_second = g_ascii_strtoull(tokens[1], NULL, 10);
+            if (!max_insn_per_second && errno) {
+                fprintf(stderr, "%s: couldn't parse %s (%s)\n",
+                        __func__, tokens[1], g_strerror(errno));
+                return -1;
+            }
+        } else {
+            fprintf(stderr, "option parsing failed: %s\n", opt);
+            return -1;
+        }
+    }
+
+    vcpus = qemu_plugin_scoreboard_new(sizeof(vCPUTime));
+    max_insn_per_quantum = max_insn_per_second / NUM_TIME_UPDATE_PER_SEC;
+
+    time_handle = qemu_plugin_request_time_control();
+    g_assert(time_handle);
+
+    qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
+    qemu_plugin_register_vcpu_init_cb(id, vcpu_init);
+    qemu_plugin_register_vcpu_exit_cb(id, vcpu_exit);
+    qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
+
+    return 0;
+}
diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile
index 0b64d2c1e3..449ead1130 100644
--- a/contrib/plugins/Makefile
+++ b/contrib/plugins/Makefile
@@ -27,6 +27,7 @@ endif
 NAMES += hwprofile
 NAMES += cache
 NAMES += drcov
+NAMES += ips
 
 ifeq ($(CONFIG_WIN32),y)
 SO_SUFFIX := .dll
-- 
2.39.2

Re: [PATCH 9/9] contrib/plugins: add ips plugin example for cost modeling

[Dr. David Alan Gilbert]

* Alex Bennée (alex.bennee@linaro.org) wrote:
> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> 
> This plugin uses the new time control interface to make decisions
> about the state of time during the emulation. The algorithm is
> currently very simple. The user specifies an ips rate which applies
> per core. If the core runs ahead of its allocated execution time the
> plugin sleeps for a bit to let real time catch up. Either way time is
> updated for the emulation as a function of total executed instructions
> with some adjustments for cores that idle.

A few random thoughts:
  a) Are there any definitions of what a plugin that controls time
     should do with a live migration?
  b) The sleep in migration/dirtyrate.c points out g_usleep might
     sleep for longer, so reads the actual wall clock time to
     figure out a new 'now'.
  c) A fun thing to do with this would be to follow an external simulation
     or 2nd qemu, trying to keep the two from running too far past
     each other.

Dave

> Examples
> --------
> 
> Slow down execution of /bin/true:
> $ num_insn=$(./build/qemu-x86_64 -plugin ./build/tests/plugin/libinsn.so -d plugin /bin/true |& grep total | sed -e 's/.*: //')
> $ time ./build/qemu-x86_64 -plugin ./build/contrib/plugins/libips.so,ips=$(($num_insn/4)) /bin/true
> real 4.000s
> 
> Boot a Linux kernel simulating a 250MHz cpu:
> $ /build/qemu-system-x86_64 -kernel /boot/vmlinuz-6.1.0-21-amd64 -append "console=ttyS0" -plugin ./build/contrib/plugins/libips.so,ips=$((250*1000*1000)) -smp 1 -m 512
> check time until kernel panic on serial0
> 
> Tested in system mode by booting a full debian system, and using:
> $ sysbench cpu run
> Performance decrease linearly with the given number of ips.
> 
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Message-Id: <20240530220610.1245424-7-pierrick.bouvier@linaro.org>
> ---
>  contrib/plugins/ips.c    | 164 +++++++++++++++++++++++++++++++++++++++
>  contrib/plugins/Makefile |   1 +
>  2 files changed, 165 insertions(+)
>  create mode 100644 contrib/plugins/ips.c
> 
> diff --git a/contrib/plugins/ips.c b/contrib/plugins/ips.c
> new file mode 100644
> index 0000000000..db77729264
> --- /dev/null
> +++ b/contrib/plugins/ips.c
> @@ -0,0 +1,164 @@
> +/*
> + * ips rate limiting plugin.
> + *
> + * This plugin can be used to restrict the execution of a system to a
> + * particular number of Instructions Per Second (ips). This controls
> + * time as seen by the guest so while wall-clock time may be longer
> + * from the guests point of view time will pass at the normal rate.
> + *
> + * This uses the new plugin API which allows the plugin to control
> + * system time.
> + *
> + * Copyright (c) 2023 Linaro Ltd
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include <stdio.h>
> +#include <glib.h>
> +#include <qemu-plugin.h>
> +
> +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
> +
> +/* how many times do we update time per sec */
> +#define NUM_TIME_UPDATE_PER_SEC 10
> +#define NSEC_IN_ONE_SEC (1000 * 1000 * 1000)
> +
> +static GMutex global_state_lock;
> +
> +static uint64_t max_insn_per_second = 1000 * 1000 * 1000; /* ips per core, per second */
> +static uint64_t max_insn_per_quantum; /* trap every N instructions */
> +static int64_t virtual_time_ns; /* last set virtual time */
> +
> +static const void *time_handle;
> +
> +typedef struct {
> +    uint64_t total_insn;
> +    uint64_t quantum_insn; /* insn in last quantum */
> +    int64_t last_quantum_time; /* time when last quantum started */
> +} vCPUTime;
> +
> +struct qemu_plugin_scoreboard *vcpus;
> +
> +/* return epoch time in ns */
> +static int64_t now_ns(void)
> +{
> +    return g_get_real_time() * 1000;
> +}
> +
> +static uint64_t num_insn_during(int64_t elapsed_ns)
> +{
> +    double num_secs = elapsed_ns / (double) NSEC_IN_ONE_SEC;
> +    return num_secs * (double) max_insn_per_second;
> +}
> +
> +static int64_t time_for_insn(uint64_t num_insn)
> +{
> +    double num_secs = (double) num_insn / (double) max_insn_per_second;
> +    return num_secs * (double) NSEC_IN_ONE_SEC;
> +}
> +
> +static void update_system_time(vCPUTime *vcpu)
> +{
> +    int64_t elapsed_ns = now_ns() - vcpu->last_quantum_time;
> +    uint64_t max_insn = num_insn_during(elapsed_ns);
> +
> +    if (vcpu->quantum_insn >= max_insn) {
> +        /* this vcpu ran faster than expected, so it has to sleep */
> +        uint64_t insn_advance = vcpu->quantum_insn - max_insn;
> +        uint64_t time_advance_ns = time_for_insn(insn_advance);
> +        int64_t sleep_us = time_advance_ns / 1000;
> +        g_usleep(sleep_us);
> +    }
> +
> +    vcpu->total_insn += vcpu->quantum_insn;
> +    vcpu->quantum_insn = 0;
> +    vcpu->last_quantum_time = now_ns();
> +
> +    /* based on total number of instructions, what should be the new time? */
> +    int64_t new_virtual_time = time_for_insn(vcpu->total_insn);
> +
> +    g_mutex_lock(&global_state_lock);
> +
> +    /* Time only moves forward. Another vcpu might have updated it already. */
> +    if (new_virtual_time > virtual_time_ns) {
> +        qemu_plugin_update_ns(time_handle, new_virtual_time);
> +        virtual_time_ns = new_virtual_time;
> +    }
> +
> +    g_mutex_unlock(&global_state_lock);
> +}
> +
> +static void vcpu_init(qemu_plugin_id_t id, unsigned int cpu_index)
> +{
> +    vCPUTime *vcpu = qemu_plugin_scoreboard_find(vcpus, cpu_index);
> +    vcpu->total_insn = 0;
> +    vcpu->quantum_insn = 0;
> +    vcpu->last_quantum_time = now_ns();
> +}
> +
> +static void vcpu_exit(qemu_plugin_id_t id, unsigned int cpu_index)
> +{
> +    vCPUTime *vcpu = qemu_plugin_scoreboard_find(vcpus, cpu_index);
> +    update_system_time(vcpu);
> +}
> +
> +static void every_quantum_insn(unsigned int cpu_index, void *udata)
> +{
> +    vCPUTime *vcpu = qemu_plugin_scoreboard_find(vcpus, cpu_index);
> +    g_assert(vcpu->quantum_insn >= max_insn_per_quantum);
> +    update_system_time(vcpu);
> +}
> +
> +static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
> +{
> +    size_t n_insns = qemu_plugin_tb_n_insns(tb);
> +    qemu_plugin_u64 quantum_insn =
> +        qemu_plugin_scoreboard_u64_in_struct(vcpus, vCPUTime, quantum_insn);
> +    /* count (and eventually trap) once per tb */
> +    qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
> +        tb, QEMU_PLUGIN_INLINE_ADD_U64, quantum_insn, n_insns);
> +    qemu_plugin_register_vcpu_tb_exec_cond_cb(
> +        tb, every_quantum_insn,
> +        QEMU_PLUGIN_CB_NO_REGS, QEMU_PLUGIN_COND_GE,
> +        quantum_insn, max_insn_per_quantum, NULL);
> +}
> +
> +static void plugin_exit(qemu_plugin_id_t id, void *udata)
> +{
> +    qemu_plugin_scoreboard_free(vcpus);
> +}
> +
> +QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
> +                                           const qemu_info_t *info, int argc,
> +                                           char **argv)
> +{
> +    for (int i = 0; i < argc; i++) {
> +        char *opt = argv[i];
> +        g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
> +        if (g_strcmp0(tokens[0], "ips") == 0) {
> +            max_insn_per_second = g_ascii_strtoull(tokens[1], NULL, 10);
> +            if (!max_insn_per_second && errno) {
> +                fprintf(stderr, "%s: couldn't parse %s (%s)\n",
> +                        __func__, tokens[1], g_strerror(errno));
> +                return -1;
> +            }
> +        } else {
> +            fprintf(stderr, "option parsing failed: %s\n", opt);
> +            return -1;
> +        }
> +    }
> +
> +    vcpus = qemu_plugin_scoreboard_new(sizeof(vCPUTime));
> +    max_insn_per_quantum = max_insn_per_second / NUM_TIME_UPDATE_PER_SEC;
> +
> +    time_handle = qemu_plugin_request_time_control();
> +    g_assert(time_handle);
> +
> +    qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
> +    qemu_plugin_register_vcpu_init_cb(id, vcpu_init);
> +    qemu_plugin_register_vcpu_exit_cb(id, vcpu_exit);
> +    qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
> +
> +    return 0;
> +}
> diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile
> index 0b64d2c1e3..449ead1130 100644
> --- a/contrib/plugins/Makefile
> +++ b/contrib/plugins/Makefile
> @@ -27,6 +27,7 @@ endif
>  NAMES += hwprofile
>  NAMES += cache
>  NAMES += drcov
> +NAMES += ips
>  
>  ifeq ($(CONFIG_WIN32),y)
>  SO_SUFFIX := .dll
> -- 
> 2.39.2
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

Re: [PATCH 9/9] contrib/plugins: add ips plugin example for cost modeling

[Pierrick Bouvier]

Hi Dave,

On 6/12/24 14:02, Dr. David Alan Gilbert wrote:
> * Alex Bennée (alex.bennee@linaro.org) wrote:
>> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>
>> This plugin uses the new time control interface to make decisions
>> about the state of time during the emulation. The algorithm is
>> currently very simple. The user specifies an ips rate which applies
>> per core. If the core runs ahead of its allocated execution time the
>> plugin sleeps for a bit to let real time catch up. Either way time is
>> updated for the emulation as a function of total executed instructions
>> with some adjustments for cores that idle.
> 
> A few random thoughts:
>    a) Are there any definitions of what a plugin that controls time
>       should do with a live migration?

It's not something that was considered as part of this work.

>    b) The sleep in migration/dirtyrate.c points out g_usleep might
>       sleep for longer, so reads the actual wall clock time to
>       figure out a new 'now'.

The current API mentions time starts at 0 from qemu startup. Maybe we 
could consider in the future to change this behavior to retrieve time 
from an existing migrated machine.

>    c) A fun thing to do with this would be to follow an external simulation
>       or 2nd qemu, trying to keep the two from running too far past
>       each other.
> 

Basically, to slow the first one, waiting for the replicated one to 
catch up?

> Dave >
>> Examples
>> --------
>>
>> Slow down execution of /bin/true:
>> $ num_insn=$(./build/qemu-x86_64 -plugin ./build/tests/plugin/libinsn.so -d plugin /bin/true |& grep total | sed -e 's/.*: //')
>> $ time ./build/qemu-x86_64 -plugin ./build/contrib/plugins/libips.so,ips=$(($num_insn/4)) /bin/true
>> real 4.000s
>>
>> Boot a Linux kernel simulating a 250MHz cpu:
>> $ /build/qemu-system-x86_64 -kernel /boot/vmlinuz-6.1.0-21-amd64 -append "console=ttyS0" -plugin ./build/contrib/plugins/libips.so,ips=$((250*1000*1000)) -smp 1 -m 512
>> check time until kernel panic on serial0
>>
>> Tested in system mode by booting a full debian system, and using:
>> $ sysbench cpu run
>> Performance decrease linearly with the given number of ips.
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> Message-Id: <20240530220610.1245424-7-pierrick.bouvier@linaro.org>
>> ---
>>   contrib/plugins/ips.c    | 164 +++++++++++++++++++++++++++++++++++++++
>>   contrib/plugins/Makefile |   1 +
>>   2 files changed, 165 insertions(+)
>>   create mode 100644 contrib/plugins/ips.c
>>
>> diff --git a/contrib/plugins/ips.c b/contrib/plugins/ips.c
>> new file mode 100644
>> index 0000000000..db77729264
>> --- /dev/null
>> +++ b/contrib/plugins/ips.c
>> @@ -0,0 +1,164 @@
>> +/*
>> + * ips rate limiting plugin.
>> + *
>> + * This plugin can be used to restrict the execution of a system to a
>> + * particular number of Instructions Per Second (ips). This controls
>> + * time as seen by the guest so while wall-clock time may be longer
>> + * from the guests point of view time will pass at the normal rate.
>> + *
>> + * This uses the new plugin API which allows the plugin to control
>> + * system time.
>> + *
>> + * Copyright (c) 2023 Linaro Ltd
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +
>> +#include <stdio.h>
>> +#include <glib.h>
>> +#include <qemu-plugin.h>
>> +
>> +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
>> +
>> +/* how many times do we update time per sec */
>> +#define NUM_TIME_UPDATE_PER_SEC 10
>> +#define NSEC_IN_ONE_SEC (1000 * 1000 * 1000)
>> +
>> +static GMutex global_state_lock;
>> +
>> +static uint64_t max_insn_per_second = 1000 * 1000 * 1000; /* ips per core, per second */
>> +static uint64_t max_insn_per_quantum; /* trap every N instructions */
>> +static int64_t virtual_time_ns; /* last set virtual time */
>> +
>> +static const void *time_handle;
>> +
>> +typedef struct {
>> +    uint64_t total_insn;
>> +    uint64_t quantum_insn; /* insn in last quantum */
>> +    int64_t last_quantum_time; /* time when last quantum started */
>> +} vCPUTime;
>> +
>> +struct qemu_plugin_scoreboard *vcpus;
>> +
>> +/* return epoch time in ns */
>> +static int64_t now_ns(void)
>> +{
>> +    return g_get_real_time() * 1000;
>> +}
>> +
>> +static uint64_t num_insn_during(int64_t elapsed_ns)
>> +{
>> +    double num_secs = elapsed_ns / (double) NSEC_IN_ONE_SEC;
>> +    return num_secs * (double) max_insn_per_second;
>> +}
>> +
>> +static int64_t time_for_insn(uint64_t num_insn)
>> +{
>> +    double num_secs = (double) num_insn / (double) max_insn_per_second;
>> +    return num_secs * (double) NSEC_IN_ONE_SEC;
>> +}
>> +
>> +static void update_system_time(vCPUTime *vcpu)
>> +{
>> +    int64_t elapsed_ns = now_ns() - vcpu->last_quantum_time;
>> +    uint64_t max_insn = num_insn_during(elapsed_ns);
>> +
>> +    if (vcpu->quantum_insn >= max_insn) {
>> +        /* this vcpu ran faster than expected, so it has to sleep */
>> +        uint64_t insn_advance = vcpu->quantum_insn - max_insn;
>> +        uint64_t time_advance_ns = time_for_insn(insn_advance);
>> +        int64_t sleep_us = time_advance_ns / 1000;
>> +        g_usleep(sleep_us);
>> +    }
>> +
>> +    vcpu->total_insn += vcpu->quantum_insn;
>> +    vcpu->quantum_insn = 0;
>> +    vcpu->last_quantum_time = now_ns();
>> +
>> +    /* based on total number of instructions, what should be the new time? */
>> +    int64_t new_virtual_time = time_for_insn(vcpu->total_insn);
>> +
>> +    g_mutex_lock(&global_state_lock);
>> +
>> +    /* Time only moves forward. Another vcpu might have updated it already. */
>> +    if (new_virtual_time > virtual_time_ns) {
>> +        qemu_plugin_update_ns(time_handle, new_virtual_time);
>> +        virtual_time_ns = new_virtual_time;
>> +    }
>> +
>> +    g_mutex_unlock(&global_state_lock);
>> +}
>> +
>> +static void vcpu_init(qemu_plugin_id_t id, unsigned int cpu_index)
>> +{
>> +    vCPUTime *vcpu = qemu_plugin_scoreboard_find(vcpus, cpu_index);
>> +    vcpu->total_insn = 0;
>> +    vcpu->quantum_insn = 0;
>> +    vcpu->last_quantum_time = now_ns();
>> +}
>> +
>> +static void vcpu_exit(qemu_plugin_id_t id, unsigned int cpu_index)
>> +{
>> +    vCPUTime *vcpu = qemu_plugin_scoreboard_find(vcpus, cpu_index);
>> +    update_system_time(vcpu);
>> +}
>> +
>> +static void every_quantum_insn(unsigned int cpu_index, void *udata)
>> +{
>> +    vCPUTime *vcpu = qemu_plugin_scoreboard_find(vcpus, cpu_index);
>> +    g_assert(vcpu->quantum_insn >= max_insn_per_quantum);
>> +    update_system_time(vcpu);
>> +}
>> +
>> +static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
>> +{
>> +    size_t n_insns = qemu_plugin_tb_n_insns(tb);
>> +    qemu_plugin_u64 quantum_insn =
>> +        qemu_plugin_scoreboard_u64_in_struct(vcpus, vCPUTime, quantum_insn);
>> +    /* count (and eventually trap) once per tb */
>> +    qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
>> +        tb, QEMU_PLUGIN_INLINE_ADD_U64, quantum_insn, n_insns);
>> +    qemu_plugin_register_vcpu_tb_exec_cond_cb(
>> +        tb, every_quantum_insn,
>> +        QEMU_PLUGIN_CB_NO_REGS, QEMU_PLUGIN_COND_GE,
>> +        quantum_insn, max_insn_per_quantum, NULL);
>> +}
>> +
>> +static void plugin_exit(qemu_plugin_id_t id, void *udata)
>> +{
>> +    qemu_plugin_scoreboard_free(vcpus);
>> +}
>> +
>> +QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
>> +                                           const qemu_info_t *info, int argc,
>> +                                           char **argv)
>> +{
>> +    for (int i = 0; i < argc; i++) {
>> +        char *opt = argv[i];
>> +        g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
>> +        if (g_strcmp0(tokens[0], "ips") == 0) {
>> +            max_insn_per_second = g_ascii_strtoull(tokens[1], NULL, 10);
>> +            if (!max_insn_per_second && errno) {
>> +                fprintf(stderr, "%s: couldn't parse %s (%s)\n",
>> +                        __func__, tokens[1], g_strerror(errno));
>> +                return -1;
>> +            }
>> +        } else {
>> +            fprintf(stderr, "option parsing failed: %s\n", opt);
>> +            return -1;
>> +        }
>> +    }
>> +
>> +    vcpus = qemu_plugin_scoreboard_new(sizeof(vCPUTime));
>> +    max_insn_per_quantum = max_insn_per_second / NUM_TIME_UPDATE_PER_SEC;
>> +
>> +    time_handle = qemu_plugin_request_time_control();
>> +    g_assert(time_handle);
>> +
>> +    qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
>> +    qemu_plugin_register_vcpu_init_cb(id, vcpu_init);
>> +    qemu_plugin_register_vcpu_exit_cb(id, vcpu_exit);
>> +    qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
>> +
>> +    return 0;
>> +}
>> diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile
>> index 0b64d2c1e3..449ead1130 100644
>> --- a/contrib/plugins/Makefile
>> +++ b/contrib/plugins/Makefile
>> @@ -27,6 +27,7 @@ endif
>>   NAMES += hwprofile
>>   NAMES += cache
>>   NAMES += drcov
>> +NAMES += ips
>>   
>>   ifeq ($(CONFIG_WIN32),y)
>>   SO_SUFFIX := .dll
>> -- 
>> 2.39.2
>>

Re: [PATCH 9/9] contrib/plugins: add ips plugin example for cost modeling

[Dr. David Alan Gilbert]

* Pierrick Bouvier (pierrick.bouvier@linaro.org) wrote:
> Hi Dave,
> 
> On 6/12/24 14:02, Dr. David Alan Gilbert wrote:
> > * Alex Bennée (alex.bennee@linaro.org) wrote:
> > > From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> > > 
> > > This plugin uses the new time control interface to make decisions
> > > about the state of time during the emulation. The algorithm is
> > > currently very simple. The user specifies an ips rate which applies
> > > per core. If the core runs ahead of its allocated execution time the
> > > plugin sleeps for a bit to let real time catch up. Either way time is
> > > updated for the emulation as a function of total executed instructions
> > > with some adjustments for cores that idle.
> > 
> > A few random thoughts:
> >    a) Are there any definitions of what a plugin that controls time
> >       should do with a live migration?
> 
> It's not something that was considered as part of this work.

That's OK, the only thing is we need to stop anyone from hitting problems
when they don't realise it's not been addressed.
One way might be to add a migration blocker; see include/migration/blocker.h
then you might print something like 'Migration not available due to plugin ....'

> >    b) The sleep in migration/dirtyrate.c points out g_usleep might
> >       sleep for longer, so reads the actual wall clock time to
> >       figure out a new 'now'.
> 
> The current API mentions time starts at 0 from qemu startup. Maybe we could
> consider in the future to change this behavior to retrieve time from an
> existing migrated machine.

Ah, I meant for (b) to be independent of (a) - not related to migration; just
down to the fact you used g_usleep in the plugin and a g_usleep might sleep
for a different amount of time than you asked.

> >    c) A fun thing to do with this would be to follow an external simulation
> >       or 2nd qemu, trying to keep the two from running too far past
> >       each other.
> > 
> 
> Basically, to slow the first one, waiting for the replicated one to catch
> up?

Yes, something like that.

Dave

> > Dave >
> > > Examples
> > > --------
> > > 
> > > Slow down execution of /bin/true:
> > > $ num_insn=$(./build/qemu-x86_64 -plugin ./build/tests/plugin/libinsn.so -d plugin /bin/true |& grep total | sed -e 's/.*: //')
> > > $ time ./build/qemu-x86_64 -plugin ./build/contrib/plugins/libips.so,ips=$(($num_insn/4)) /bin/true
> > > real 4.000s
> > > 
> > > Boot a Linux kernel simulating a 250MHz cpu:
> > > $ /build/qemu-system-x86_64 -kernel /boot/vmlinuz-6.1.0-21-amd64 -append "console=ttyS0" -plugin ./build/contrib/plugins/libips.so,ips=$((250*1000*1000)) -smp 1 -m 512
> > > check time until kernel panic on serial0
> > > 
> > > Tested in system mode by booting a full debian system, and using:
> > > $ sysbench cpu run
> > > Performance decrease linearly with the given number of ips.
> > > 
> > > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> > > Message-Id: <20240530220610.1245424-7-pierrick.bouvier@linaro.org>
> > > ---
> > >   contrib/plugins/ips.c    | 164 +++++++++++++++++++++++++++++++++++++++
> > >   contrib/plugins/Makefile |   1 +
> > >   2 files changed, 165 insertions(+)
> > >   create mode 100644 contrib/plugins/ips.c
> > > 
> > > diff --git a/contrib/plugins/ips.c b/contrib/plugins/ips.c
> > > new file mode 100644
> > > index 0000000000..db77729264
> > > --- /dev/null
> > > +++ b/contrib/plugins/ips.c
> > > @@ -0,0 +1,164 @@
> > > +/*
> > > + * ips rate limiting plugin.
> > > + *
> > > + * This plugin can be used to restrict the execution of a system to a
> > > + * particular number of Instructions Per Second (ips). This controls
> > > + * time as seen by the guest so while wall-clock time may be longer
> > > + * from the guests point of view time will pass at the normal rate.
> > > + *
> > > + * This uses the new plugin API which allows the plugin to control
> > > + * system time.
> > > + *
> > > + * Copyright (c) 2023 Linaro Ltd
> > > + *
> > > + * SPDX-License-Identifier: GPL-2.0-or-later
> > > + */
> > > +
> > > +#include <stdio.h>
> > > +#include <glib.h>
> > > +#include <qemu-plugin.h>
> > > +
> > > +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
> > > +
> > > +/* how many times do we update time per sec */
> > > +#define NUM_TIME_UPDATE_PER_SEC 10
> > > +#define NSEC_IN_ONE_SEC (1000 * 1000 * 1000)
> > > +
> > > +static GMutex global_state_lock;
> > > +
> > > +static uint64_t max_insn_per_second = 1000 * 1000 * 1000; /* ips per core, per second */
> > > +static uint64_t max_insn_per_quantum; /* trap every N instructions */
> > > +static int64_t virtual_time_ns; /* last set virtual time */
> > > +
> > > +static const void *time_handle;
> > > +
> > > +typedef struct {
> > > +    uint64_t total_insn;
> > > +    uint64_t quantum_insn; /* insn in last quantum */
> > > +    int64_t last_quantum_time; /* time when last quantum started */
> > > +} vCPUTime;
> > > +
> > > +struct qemu_plugin_scoreboard *vcpus;
> > > +
> > > +/* return epoch time in ns */
> > > +static int64_t now_ns(void)
> > > +{
> > > +    return g_get_real_time() * 1000;
> > > +}
> > > +
> > > +static uint64_t num_insn_during(int64_t elapsed_ns)
> > > +{
> > > +    double num_secs = elapsed_ns / (double) NSEC_IN_ONE_SEC;
> > > +    return num_secs * (double) max_insn_per_second;
> > > +}
> > > +
> > > +static int64_t time_for_insn(uint64_t num_insn)
> > > +{
> > > +    double num_secs = (double) num_insn / (double) max_insn_per_second;
> > > +    return num_secs * (double) NSEC_IN_ONE_SEC;
> > > +}
> > > +
> > > +static void update_system_time(vCPUTime *vcpu)
> > > +{
> > > +    int64_t elapsed_ns = now_ns() - vcpu->last_quantum_time;
> > > +    uint64_t max_insn = num_insn_during(elapsed_ns);
> > > +
> > > +    if (vcpu->quantum_insn >= max_insn) {
> > > +        /* this vcpu ran faster than expected, so it has to sleep */
> > > +        uint64_t insn_advance = vcpu->quantum_insn - max_insn;
> > > +        uint64_t time_advance_ns = time_for_insn(insn_advance);
> > > +        int64_t sleep_us = time_advance_ns / 1000;
> > > +        g_usleep(sleep_us);
> > > +    }
> > > +
> > > +    vcpu->total_insn += vcpu->quantum_insn;
> > > +    vcpu->quantum_insn = 0;
> > > +    vcpu->last_quantum_time = now_ns();
> > > +
> > > +    /* based on total number of instructions, what should be the new time? */
> > > +    int64_t new_virtual_time = time_for_insn(vcpu->total_insn);
> > > +
> > > +    g_mutex_lock(&global_state_lock);
> > > +
> > > +    /* Time only moves forward. Another vcpu might have updated it already. */
> > > +    if (new_virtual_time > virtual_time_ns) {
> > > +        qemu_plugin_update_ns(time_handle, new_virtual_time);
> > > +        virtual_time_ns = new_virtual_time;
> > > +    }
> > > +
> > > +    g_mutex_unlock(&global_state_lock);
> > > +}
> > > +
> > > +static void vcpu_init(qemu_plugin_id_t id, unsigned int cpu_index)
> > > +{
> > > +    vCPUTime *vcpu = qemu_plugin_scoreboard_find(vcpus, cpu_index);
> > > +    vcpu->total_insn = 0;
> > > +    vcpu->quantum_insn = 0;
> > > +    vcpu->last_quantum_time = now_ns();
> > > +}
> > > +
> > > +static void vcpu_exit(qemu_plugin_id_t id, unsigned int cpu_index)
> > > +{
> > > +    vCPUTime *vcpu = qemu_plugin_scoreboard_find(vcpus, cpu_index);
> > > +    update_system_time(vcpu);
> > > +}
> > > +
> > > +static void every_quantum_insn(unsigned int cpu_index, void *udata)
> > > +{
> > > +    vCPUTime *vcpu = qemu_plugin_scoreboard_find(vcpus, cpu_index);
> > > +    g_assert(vcpu->quantum_insn >= max_insn_per_quantum);
> > > +    update_system_time(vcpu);
> > > +}
> > > +
> > > +static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
> > > +{
> > > +    size_t n_insns = qemu_plugin_tb_n_insns(tb);
> > > +    qemu_plugin_u64 quantum_insn =
> > > +        qemu_plugin_scoreboard_u64_in_struct(vcpus, vCPUTime, quantum_insn);
> > > +    /* count (and eventually trap) once per tb */
> > > +    qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
> > > +        tb, QEMU_PLUGIN_INLINE_ADD_U64, quantum_insn, n_insns);
> > > +    qemu_plugin_register_vcpu_tb_exec_cond_cb(
> > > +        tb, every_quantum_insn,
> > > +        QEMU_PLUGIN_CB_NO_REGS, QEMU_PLUGIN_COND_GE,
> > > +        quantum_insn, max_insn_per_quantum, NULL);
> > > +}
> > > +
> > > +static void plugin_exit(qemu_plugin_id_t id, void *udata)
> > > +{
> > > +    qemu_plugin_scoreboard_free(vcpus);
> > > +}
> > > +
> > > +QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
> > > +                                           const qemu_info_t *info, int argc,
> > > +                                           char **argv)
> > > +{
> > > +    for (int i = 0; i < argc; i++) {
> > > +        char *opt = argv[i];
> > > +        g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
> > > +        if (g_strcmp0(tokens[0], "ips") == 0) {
> > > +            max_insn_per_second = g_ascii_strtoull(tokens[1], NULL, 10);
> > > +            if (!max_insn_per_second && errno) {
> > > +                fprintf(stderr, "%s: couldn't parse %s (%s)\n",
> > > +                        __func__, tokens[1], g_strerror(errno));
> > > +                return -1;
> > > +            }
> > > +        } else {
> > > +            fprintf(stderr, "option parsing failed: %s\n", opt);
> > > +            return -1;
> > > +        }
> > > +    }
> > > +
> > > +    vcpus = qemu_plugin_scoreboard_new(sizeof(vCPUTime));
> > > +    max_insn_per_quantum = max_insn_per_second / NUM_TIME_UPDATE_PER_SEC;
> > > +
> > > +    time_handle = qemu_plugin_request_time_control();
> > > +    g_assert(time_handle);
> > > +
> > > +    qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
> > > +    qemu_plugin_register_vcpu_init_cb(id, vcpu_init);
> > > +    qemu_plugin_register_vcpu_exit_cb(id, vcpu_exit);
> > > +    qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
> > > +
> > > +    return 0;
> > > +}
> > > diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile
> > > index 0b64d2c1e3..449ead1130 100644
> > > --- a/contrib/plugins/Makefile
> > > +++ b/contrib/plugins/Makefile
> > > @@ -27,6 +27,7 @@ endif
> > >   NAMES += hwprofile
> > >   NAMES += cache
> > >   NAMES += drcov
> > > +NAMES += ips
> > >   ifeq ($(CONFIG_WIN32),y)
> > >   SO_SUFFIX := .dll
> > > -- 
> > > 2.39.2
> > > 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

Re: [PATCH 9/9] contrib/plugins: add ips plugin example for cost modeling

[Pierrick Bouvier]

On 6/14/24 15:00, Dr. David Alan Gilbert wrote:
> * Pierrick Bouvier (pierrick.bouvier@linaro.org) wrote:
>> Hi Dave,
>>
>> On 6/12/24 14:02, Dr. David Alan Gilbert wrote:
>>> * Alex Bennée (alex.bennee@linaro.org) wrote:
>>>> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>>
>>>> This plugin uses the new time control interface to make decisions
>>>> about the state of time during the emulation. The algorithm is
>>>> currently very simple. The user specifies an ips rate which applies
>>>> per core. If the core runs ahead of its allocated execution time the
>>>> plugin sleeps for a bit to let real time catch up. Either way time is
>>>> updated for the emulation as a function of total executed instructions
>>>> with some adjustments for cores that idle.
>>>
>>> A few random thoughts:
>>>     a) Are there any definitions of what a plugin that controls time
>>>        should do with a live migration?
>>
>> It's not something that was considered as part of this work.
> 
> That's OK, the only thing is we need to stop anyone from hitting problems
> when they don't realise it's not been addressed.
> One way might be to add a migration blocker; see include/migration/blocker.h
> then you might print something like 'Migration not available due to plugin ....'
> 

So basically, we could make a call to migrate_add_blocker(), when 
someone request time_control through plugin API?

IMHO, it's something that should be part of plugin API (if any plugin 
calls qemu_plugin_request_time_control()), instead of the plugin code 
itself. This way, any plugin getting time control automatically blocks 
any potential migration.

>>>     b) The sleep in migration/dirtyrate.c points out g_usleep might
>>>        sleep for longer, so reads the actual wall clock time to
>>>        figure out a new 'now'.
>>
>> The current API mentions time starts at 0 from qemu startup. Maybe we could
>> consider in the future to change this behavior to retrieve time from an
>> existing migrated machine.
> 
> Ah, I meant for (b) to be independent of (a) - not related to migration; just
> down to the fact you used g_usleep in the plugin and a g_usleep might sleep
> for a different amount of time than you asked.
> 

We know that, and the plugin is not meant to be "cycle accurate" in 
general, we just set a upper bound for number of instructions we can 
execute in a given amount of time (1/10 second for now).

We compute the new time based on how many instructions effectively ran 
on the most used cpu, so even if we slept a bit more than expected, it's 
correct.

>>>     c) A fun thing to do with this would be to follow an external simulation
>>>        or 2nd qemu, trying to keep the two from running too far past
>>>        each other.
>>>
>>
>> Basically, to slow the first one, waiting for the replicated one to catch
>> up?
> 
> Yes, something like that.
> 
> Dave
> 
>>> Dave >
>>>> Examples
>>>> --------
>>>>
>>>> Slow down execution of /bin/true:
>>>> $ num_insn=$(./build/qemu-x86_64 -plugin ./build/tests/plugin/libinsn.so -d plugin /bin/true |& grep total | sed -e 's/.*: //')
>>>> $ time ./build/qemu-x86_64 -plugin ./build/contrib/plugins/libips.so,ips=$(($num_insn/4)) /bin/true
>>>> real 4.000s
>>>>
>>>> Boot a Linux kernel simulating a 250MHz cpu:
>>>> $ /build/qemu-system-x86_64 -kernel /boot/vmlinuz-6.1.0-21-amd64 -append "console=ttyS0" -plugin ./build/contrib/plugins/libips.so,ips=$((250*1000*1000)) -smp 1 -m 512
>>>> check time until kernel panic on serial0
>>>>
>>>> Tested in system mode by booting a full debian system, and using:
>>>> $ sysbench cpu run
>>>> Performance decrease linearly with the given number of ips.
>>>>
>>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>> Message-Id: <20240530220610.1245424-7-pierrick.bouvier@linaro.org>
>>>> ---
>>>>    contrib/plugins/ips.c    | 164 +++++++++++++++++++++++++++++++++++++++
>>>>    contrib/plugins/Makefile |   1 +
>>>>    2 files changed, 165 insertions(+)
>>>>    create mode 100644 contrib/plugins/ips.c
>>>>
>>>> diff --git a/contrib/plugins/ips.c b/contrib/plugins/ips.c
>>>> new file mode 100644
>>>> index 0000000000..db77729264
>>>> --- /dev/null
>>>> +++ b/contrib/plugins/ips.c
>>>> @@ -0,0 +1,164 @@
>>>> +/*
>>>> + * ips rate limiting plugin.
>>>> + *
>>>> + * This plugin can be used to restrict the execution of a system to a
>>>> + * particular number of Instructions Per Second (ips). This controls
>>>> + * time as seen by the guest so while wall-clock time may be longer
>>>> + * from the guests point of view time will pass at the normal rate.
>>>> + *
>>>> + * This uses the new plugin API which allows the plugin to control
>>>> + * system time.
>>>> + *
>>>> + * Copyright (c) 2023 Linaro Ltd
>>>> + *
>>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>>> + */
>>>> +
>>>> +#include <stdio.h>
>>>> +#include <glib.h>
>>>> +#include <qemu-plugin.h>
>>>> +
>>>> +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
>>>> +
>>>> +/* how many times do we update time per sec */
>>>> +#define NUM_TIME_UPDATE_PER_SEC 10
>>>> +#define NSEC_IN_ONE_SEC (1000 * 1000 * 1000)
>>>> +
>>>> +static GMutex global_state_lock;
>>>> +
>>>> +static uint64_t max_insn_per_second = 1000 * 1000 * 1000; /* ips per core, per second */
>>>> +static uint64_t max_insn_per_quantum; /* trap every N instructions */
>>>> +static int64_t virtual_time_ns; /* last set virtual time */
>>>> +
>>>> +static const void *time_handle;
>>>> +
>>>> +typedef struct {
>>>> +    uint64_t total_insn;
>>>> +    uint64_t quantum_insn; /* insn in last quantum */
>>>> +    int64_t last_quantum_time; /* time when last quantum started */
>>>> +} vCPUTime;
>>>> +
>>>> +struct qemu_plugin_scoreboard *vcpus;
>>>> +
>>>> +/* return epoch time in ns */
>>>> +static int64_t now_ns(void)
>>>> +{
>>>> +    return g_get_real_time() * 1000;
>>>> +}
>>>> +
>>>> +static uint64_t num_insn_during(int64_t elapsed_ns)
>>>> +{
>>>> +    double num_secs = elapsed_ns / (double) NSEC_IN_ONE_SEC;
>>>> +    return num_secs * (double) max_insn_per_second;
>>>> +}
>>>> +
>>>> +static int64_t time_for_insn(uint64_t num_insn)
>>>> +{
>>>> +    double num_secs = (double) num_insn / (double) max_insn_per_second;
>>>> +    return num_secs * (double) NSEC_IN_ONE_SEC;
>>>> +}
>>>> +
>>>> +static void update_system_time(vCPUTime *vcpu)
>>>> +{
>>>> +    int64_t elapsed_ns = now_ns() - vcpu->last_quantum_time;
>>>> +    uint64_t max_insn = num_insn_during(elapsed_ns);
>>>> +
>>>> +    if (vcpu->quantum_insn >= max_insn) {
>>>> +        /* this vcpu ran faster than expected, so it has to sleep */
>>>> +        uint64_t insn_advance = vcpu->quantum_insn - max_insn;
>>>> +        uint64_t time_advance_ns = time_for_insn(insn_advance);
>>>> +        int64_t sleep_us = time_advance_ns / 1000;
>>>> +        g_usleep(sleep_us);
>>>> +    }
>>>> +
>>>> +    vcpu->total_insn += vcpu->quantum_insn;
>>>> +    vcpu->quantum_insn = 0;
>>>> +    vcpu->last_quantum_time = now_ns();
>>>> +
>>>> +    /* based on total number of instructions, what should be the new time? */
>>>> +    int64_t new_virtual_time = time_for_insn(vcpu->total_insn);
>>>> +
>>>> +    g_mutex_lock(&global_state_lock);
>>>> +
>>>> +    /* Time only moves forward. Another vcpu might have updated it already. */
>>>> +    if (new_virtual_time > virtual_time_ns) {
>>>> +        qemu_plugin_update_ns(time_handle, new_virtual_time);
>>>> +        virtual_time_ns = new_virtual_time;
>>>> +    }
>>>> +
>>>> +    g_mutex_unlock(&global_state_lock);
>>>> +}
>>>> +
>>>> +static void vcpu_init(qemu_plugin_id_t id, unsigned int cpu_index)
>>>> +{
>>>> +    vCPUTime *vcpu = qemu_plugin_scoreboard_find(vcpus, cpu_index);
>>>> +    vcpu->total_insn = 0;
>>>> +    vcpu->quantum_insn = 0;
>>>> +    vcpu->last_quantum_time = now_ns();
>>>> +}
>>>> +
>>>> +static void vcpu_exit(qemu_plugin_id_t id, unsigned int cpu_index)
>>>> +{
>>>> +    vCPUTime *vcpu = qemu_plugin_scoreboard_find(vcpus, cpu_index);
>>>> +    update_system_time(vcpu);
>>>> +}
>>>> +
>>>> +static void every_quantum_insn(unsigned int cpu_index, void *udata)
>>>> +{
>>>> +    vCPUTime *vcpu = qemu_plugin_scoreboard_find(vcpus, cpu_index);
>>>> +    g_assert(vcpu->quantum_insn >= max_insn_per_quantum);
>>>> +    update_system_time(vcpu);
>>>> +}
>>>> +
>>>> +static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
>>>> +{
>>>> +    size_t n_insns = qemu_plugin_tb_n_insns(tb);
>>>> +    qemu_plugin_u64 quantum_insn =
>>>> +        qemu_plugin_scoreboard_u64_in_struct(vcpus, vCPUTime, quantum_insn);
>>>> +    /* count (and eventually trap) once per tb */
>>>> +    qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
>>>> +        tb, QEMU_PLUGIN_INLINE_ADD_U64, quantum_insn, n_insns);
>>>> +    qemu_plugin_register_vcpu_tb_exec_cond_cb(
>>>> +        tb, every_quantum_insn,
>>>> +        QEMU_PLUGIN_CB_NO_REGS, QEMU_PLUGIN_COND_GE,
>>>> +        quantum_insn, max_insn_per_quantum, NULL);
>>>> +}
>>>> +
>>>> +static void plugin_exit(qemu_plugin_id_t id, void *udata)
>>>> +{
>>>> +    qemu_plugin_scoreboard_free(vcpus);
>>>> +}
>>>> +
>>>> +QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
>>>> +                                           const qemu_info_t *info, int argc,
>>>> +                                           char **argv)
>>>> +{
>>>> +    for (int i = 0; i < argc; i++) {
>>>> +        char *opt = argv[i];
>>>> +        g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
>>>> +        if (g_strcmp0(tokens[0], "ips") == 0) {
>>>> +            max_insn_per_second = g_ascii_strtoull(tokens[1], NULL, 10);
>>>> +            if (!max_insn_per_second && errno) {
>>>> +                fprintf(stderr, "%s: couldn't parse %s (%s)\n",
>>>> +                        __func__, tokens[1], g_strerror(errno));
>>>> +                return -1;
>>>> +            }
>>>> +        } else {
>>>> +            fprintf(stderr, "option parsing failed: %s\n", opt);
>>>> +            return -1;
>>>> +        }
>>>> +    }
>>>> +
>>>> +    vcpus = qemu_plugin_scoreboard_new(sizeof(vCPUTime));
>>>> +    max_insn_per_quantum = max_insn_per_second / NUM_TIME_UPDATE_PER_SEC;
>>>> +
>>>> +    time_handle = qemu_plugin_request_time_control();
>>>> +    g_assert(time_handle);
>>>> +
>>>> +    qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
>>>> +    qemu_plugin_register_vcpu_init_cb(id, vcpu_init);
>>>> +    qemu_plugin_register_vcpu_exit_cb(id, vcpu_exit);
>>>> +    qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
>>>> +
>>>> +    return 0;
>>>> +}
>>>> diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile
>>>> index 0b64d2c1e3..449ead1130 100644
>>>> --- a/contrib/plugins/Makefile
>>>> +++ b/contrib/plugins/Makefile
>>>> @@ -27,6 +27,7 @@ endif
>>>>    NAMES += hwprofile
>>>>    NAMES += cache
>>>>    NAMES += drcov
>>>> +NAMES += ips
>>>>    ifeq ($(CONFIG_WIN32),y)
>>>>    SO_SUFFIX := .dll
>>>> -- 
>>>> 2.39.2
>>>>

Re: [PATCH 9/9] contrib/plugins: add ips plugin example for cost modeling

[Dr. David Alan Gilbert]

* Pierrick Bouvier (pierrick.bouvier@linaro.org) wrote:
> On 6/14/24 15:00, Dr. David Alan Gilbert wrote:
> > * Pierrick Bouvier (pierrick.bouvier@linaro.org) wrote:
> > > Hi Dave,
> > > 
> > > On 6/12/24 14:02, Dr. David Alan Gilbert wrote:
> > > > * Alex Bennée (alex.bennee@linaro.org) wrote:
> > > > > From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> > > > > 
> > > > > This plugin uses the new time control interface to make decisions
> > > > > about the state of time during the emulation. The algorithm is
> > > > > currently very simple. The user specifies an ips rate which applies
> > > > > per core. If the core runs ahead of its allocated execution time the
> > > > > plugin sleeps for a bit to let real time catch up. Either way time is
> > > > > updated for the emulation as a function of total executed instructions
> > > > > with some adjustments for cores that idle.
> > > > 
> > > > A few random thoughts:
> > > >     a) Are there any definitions of what a plugin that controls time
> > > >        should do with a live migration?
> > > 
> > > It's not something that was considered as part of this work.
> > 
> > That's OK, the only thing is we need to stop anyone from hitting problems
> > when they don't realise it's not been addressed.
> > One way might be to add a migration blocker; see include/migration/blocker.h
> > then you might print something like 'Migration not available due to plugin ....'
> > 
> 
> So basically, we could make a call to migrate_add_blocker(), when someone
> request time_control through plugin API?
> 
> IMHO, it's something that should be part of plugin API (if any plugin calls
> qemu_plugin_request_time_control()), instead of the plugin code itself. This
> way, any plugin getting time control automatically blocks any potential
> migration.

Note my question asked for a 'any definitions of what a plugin ..' - so
you could define it that way, another one is to think that in the future
you may allow it and the plugin somehow interacts with migration not to
change time at certain migration phases.

> > > >     b) The sleep in migration/dirtyrate.c points out g_usleep might
> > > >        sleep for longer, so reads the actual wall clock time to
> > > >        figure out a new 'now'.
> > > 
> > > The current API mentions time starts at 0 from qemu startup. Maybe we could
> > > consider in the future to change this behavior to retrieve time from an
> > > existing migrated machine.
> > 
> > Ah, I meant for (b) to be independent of (a) - not related to migration; just
> > down to the fact you used g_usleep in the plugin and a g_usleep might sleep
> > for a different amount of time than you asked.
> > 
> 
> We know that, and the plugin is not meant to be "cycle accurate" in general,
> we just set a upper bound for number of instructions we can execute in a
> given amount of time (1/10 second for now).
> 
> We compute the new time based on how many instructions effectively ran on
> the most used cpu, so even if we slept a bit more than expected, it's
> correct.

Ah OK.

Dave

> > > >     c) A fun thing to do with this would be to follow an external simulation
> > > >        or 2nd qemu, trying to keep the two from running too far past
> > > >        each other.
> > > > 
> > > 
> > > Basically, to slow the first one, waiting for the replicated one to catch
> > > up?
> > 
> > Yes, something like that.
> > 
> > Dave
> > 
> > > > Dave >
> > > > > Examples
> > > > > --------
> > > > > 
> > > > > Slow down execution of /bin/true:
> > > > > $ num_insn=$(./build/qemu-x86_64 -plugin ./build/tests/plugin/libinsn.so -d plugin /bin/true |& grep total | sed -e 's/.*: //')
> > > > > $ time ./build/qemu-x86_64 -plugin ./build/contrib/plugins/libips.so,ips=$(($num_insn/4)) /bin/true
> > > > > real 4.000s
> > > > > 
> > > > > Boot a Linux kernel simulating a 250MHz cpu:
> > > > > $ /build/qemu-system-x86_64 -kernel /boot/vmlinuz-6.1.0-21-amd64 -append "console=ttyS0" -plugin ./build/contrib/plugins/libips.so,ips=$((250*1000*1000)) -smp 1 -m 512
> > > > > check time until kernel panic on serial0
> > > > > 
> > > > > Tested in system mode by booting a full debian system, and using:
> > > > > $ sysbench cpu run
> > > > > Performance decrease linearly with the given number of ips.
> > > > > 
> > > > > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> > > > > Message-Id: <20240530220610.1245424-7-pierrick.bouvier@linaro.org>
> > > > > ---
> > > > >    contrib/plugins/ips.c    | 164 +++++++++++++++++++++++++++++++++++++++
> > > > >    contrib/plugins/Makefile |   1 +
> > > > >    2 files changed, 165 insertions(+)
> > > > >    create mode 100644 contrib/plugins/ips.c
> > > > > 
> > > > > diff --git a/contrib/plugins/ips.c b/contrib/plugins/ips.c
> > > > > new file mode 100644
> > > > > index 0000000000..db77729264
> > > > > --- /dev/null
> > > > > +++ b/contrib/plugins/ips.c
> > > > > @@ -0,0 +1,164 @@
> > > > > +/*
> > > > > + * ips rate limiting plugin.
> > > > > + *
> > > > > + * This plugin can be used to restrict the execution of a system to a
> > > > > + * particular number of Instructions Per Second (ips). This controls
> > > > > + * time as seen by the guest so while wall-clock time may be longer
> > > > > + * from the guests point of view time will pass at the normal rate.
> > > > > + *
> > > > > + * This uses the new plugin API which allows the plugin to control
> > > > > + * system time.
> > > > > + *
> > > > > + * Copyright (c) 2023 Linaro Ltd
> > > > > + *
> > > > > + * SPDX-License-Identifier: GPL-2.0-or-later
> > > > > + */
> > > > > +
> > > > > +#include <stdio.h>
> > > > > +#include <glib.h>
> > > > > +#include <qemu-plugin.h>
> > > > > +
> > > > > +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
> > > > > +
> > > > > +/* how many times do we update time per sec */
> > > > > +#define NUM_TIME_UPDATE_PER_SEC 10
> > > > > +#define NSEC_IN_ONE_SEC (1000 * 1000 * 1000)
> > > > > +
> > > > > +static GMutex global_state_lock;
> > > > > +
> > > > > +static uint64_t max_insn_per_second = 1000 * 1000 * 1000; /* ips per core, per second */
> > > > > +static uint64_t max_insn_per_quantum; /* trap every N instructions */
> > > > > +static int64_t virtual_time_ns; /* last set virtual time */
> > > > > +
> > > > > +static const void *time_handle;
> > > > > +
> > > > > +typedef struct {
> > > > > +    uint64_t total_insn;
> > > > > +    uint64_t quantum_insn; /* insn in last quantum */
> > > > > +    int64_t last_quantum_time; /* time when last quantum started */
> > > > > +} vCPUTime;
> > > > > +
> > > > > +struct qemu_plugin_scoreboard *vcpus;
> > > > > +
> > > > > +/* return epoch time in ns */
> > > > > +static int64_t now_ns(void)
> > > > > +{
> > > > > +    return g_get_real_time() * 1000;
> > > > > +}
> > > > > +
> > > > > +static uint64_t num_insn_during(int64_t elapsed_ns)
> > > > > +{
> > > > > +    double num_secs = elapsed_ns / (double) NSEC_IN_ONE_SEC;
> > > > > +    return num_secs * (double) max_insn_per_second;
> > > > > +}
> > > > > +
> > > > > +static int64_t time_for_insn(uint64_t num_insn)
> > > > > +{
> > > > > +    double num_secs = (double) num_insn / (double) max_insn_per_second;
> > > > > +    return num_secs * (double) NSEC_IN_ONE_SEC;
> > > > > +}
> > > > > +
> > > > > +static void update_system_time(vCPUTime *vcpu)
> > > > > +{
> > > > > +    int64_t elapsed_ns = now_ns() - vcpu->last_quantum_time;
> > > > > +    uint64_t max_insn = num_insn_during(elapsed_ns);
> > > > > +
> > > > > +    if (vcpu->quantum_insn >= max_insn) {
> > > > > +        /* this vcpu ran faster than expected, so it has to sleep */
> > > > > +        uint64_t insn_advance = vcpu->quantum_insn - max_insn;
> > > > > +        uint64_t time_advance_ns = time_for_insn(insn_advance);
> > > > > +        int64_t sleep_us = time_advance_ns / 1000;
> > > > > +        g_usleep(sleep_us);
> > > > > +    }
> > > > > +
> > > > > +    vcpu->total_insn += vcpu->quantum_insn;
> > > > > +    vcpu->quantum_insn = 0;
> > > > > +    vcpu->last_quantum_time = now_ns();
> > > > > +
> > > > > +    /* based on total number of instructions, what should be the new time? */
> > > > > +    int64_t new_virtual_time = time_for_insn(vcpu->total_insn);
> > > > > +
> > > > > +    g_mutex_lock(&global_state_lock);
> > > > > +
> > > > > +    /* Time only moves forward. Another vcpu might have updated it already. */
> > > > > +    if (new_virtual_time > virtual_time_ns) {
> > > > > +        qemu_plugin_update_ns(time_handle, new_virtual_time);
> > > > > +        virtual_time_ns = new_virtual_time;
> > > > > +    }
> > > > > +
> > > > > +    g_mutex_unlock(&global_state_lock);
> > > > > +}
> > > > > +
> > > > > +static void vcpu_init(qemu_plugin_id_t id, unsigned int cpu_index)
> > > > > +{
> > > > > +    vCPUTime *vcpu = qemu_plugin_scoreboard_find(vcpus, cpu_index);
> > > > > +    vcpu->total_insn = 0;
> > > > > +    vcpu->quantum_insn = 0;
> > > > > +    vcpu->last_quantum_time = now_ns();
> > > > > +}
> > > > > +
> > > > > +static void vcpu_exit(qemu_plugin_id_t id, unsigned int cpu_index)
> > > > > +{
> > > > > +    vCPUTime *vcpu = qemu_plugin_scoreboard_find(vcpus, cpu_index);
> > > > > +    update_system_time(vcpu);
> > > > > +}
> > > > > +
> > > > > +static void every_quantum_insn(unsigned int cpu_index, void *udata)
> > > > > +{
> > > > > +    vCPUTime *vcpu = qemu_plugin_scoreboard_find(vcpus, cpu_index);
> > > > > +    g_assert(vcpu->quantum_insn >= max_insn_per_quantum);
> > > > > +    update_system_time(vcpu);
> > > > > +}
> > > > > +
> > > > > +static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
> > > > > +{
> > > > > +    size_t n_insns = qemu_plugin_tb_n_insns(tb);
> > > > > +    qemu_plugin_u64 quantum_insn =
> > > > > +        qemu_plugin_scoreboard_u64_in_struct(vcpus, vCPUTime, quantum_insn);
> > > > > +    /* count (and eventually trap) once per tb */
> > > > > +    qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
> > > > > +        tb, QEMU_PLUGIN_INLINE_ADD_U64, quantum_insn, n_insns);
> > > > > +    qemu_plugin_register_vcpu_tb_exec_cond_cb(
> > > > > +        tb, every_quantum_insn,
> > > > > +        QEMU_PLUGIN_CB_NO_REGS, QEMU_PLUGIN_COND_GE,
> > > > > +        quantum_insn, max_insn_per_quantum, NULL);
> > > > > +}
> > > > > +
> > > > > +static void plugin_exit(qemu_plugin_id_t id, void *udata)
> > > > > +{
> > > > > +    qemu_plugin_scoreboard_free(vcpus);
> > > > > +}
> > > > > +
> > > > > +QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
> > > > > +                                           const qemu_info_t *info, int argc,
> > > > > +                                           char **argv)
> > > > > +{
> > > > > +    for (int i = 0; i < argc; i++) {
> > > > > +        char *opt = argv[i];
> > > > > +        g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
> > > > > +        if (g_strcmp0(tokens[0], "ips") == 0) {
> > > > > +            max_insn_per_second = g_ascii_strtoull(tokens[1], NULL, 10);
> > > > > +            if (!max_insn_per_second && errno) {
> > > > > +                fprintf(stderr, "%s: couldn't parse %s (%s)\n",
> > > > > +                        __func__, tokens[1], g_strerror(errno));
> > > > > +                return -1;
> > > > > +            }
> > > > > +        } else {
> > > > > +            fprintf(stderr, "option parsing failed: %s\n", opt);
> > > > > +            return -1;
> > > > > +        }
> > > > > +    }
> > > > > +
> > > > > +    vcpus = qemu_plugin_scoreboard_new(sizeof(vCPUTime));
> > > > > +    max_insn_per_quantum = max_insn_per_second / NUM_TIME_UPDATE_PER_SEC;
> > > > > +
> > > > > +    time_handle = qemu_plugin_request_time_control();
> > > > > +    g_assert(time_handle);
> > > > > +
> > > > > +    qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
> > > > > +    qemu_plugin_register_vcpu_init_cb(id, vcpu_init);
> > > > > +    qemu_plugin_register_vcpu_exit_cb(id, vcpu_exit);
> > > > > +    qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
> > > > > +
> > > > > +    return 0;
> > > > > +}
> > > > > diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile
> > > > > index 0b64d2c1e3..449ead1130 100644
> > > > > --- a/contrib/plugins/Makefile
> > > > > +++ b/contrib/plugins/Makefile
> > > > > @@ -27,6 +27,7 @@ endif
> > > > >    NAMES += hwprofile
> > > > >    NAMES += cache
> > > > >    NAMES += drcov
> > > > > +NAMES += ips
> > > > >    ifeq ($(CONFIG_WIN32),y)
> > > > >    SO_SUFFIX := .dll
> > > > > -- 
> > > > > 2.39.2
> > > > > 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

Re: [PATCH 9/9] contrib/plugins: add ips plugin example for cost modeling

[Pierrick Bouvier]

On 6/17/24 13:56, Dr. David Alan Gilbert wrote:
> * Pierrick Bouvier (pierrick.bouvier@linaro.org) wrote:
>> On 6/14/24 15:00, Dr. David Alan Gilbert wrote:
>>> * Pierrick Bouvier (pierrick.bouvier@linaro.org) wrote:
>>>> Hi Dave,
>>>>
>>>> On 6/12/24 14:02, Dr. David Alan Gilbert wrote:
>>>>> * Alex Bennée (alex.bennee@linaro.org) wrote:
>>>>>> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>>>>
>>>>>> This plugin uses the new time control interface to make decisions
>>>>>> about the state of time during the emulation. The algorithm is
>>>>>> currently very simple. The user specifies an ips rate which applies
>>>>>> per core. If the core runs ahead of its allocated execution time the
>>>>>> plugin sleeps for a bit to let real time catch up. Either way time is
>>>>>> updated for the emulation as a function of total executed instructions
>>>>>> with some adjustments for cores that idle.
>>>>>
>>>>> A few random thoughts:
>>>>>      a) Are there any definitions of what a plugin that controls time
>>>>>         should do with a live migration?
>>>>
>>>> It's not something that was considered as part of this work.
>>>
>>> That's OK, the only thing is we need to stop anyone from hitting problems
>>> when they don't realise it's not been addressed.
>>> One way might be to add a migration blocker; see include/migration/blocker.h
>>> then you might print something like 'Migration not available due to plugin ....'
>>>
>>
>> So basically, we could make a call to migrate_add_blocker(), when someone
>> request time_control through plugin API?
>>
>> IMHO, it's something that should be part of plugin API (if any plugin calls
>> qemu_plugin_request_time_control()), instead of the plugin code itself. This
>> way, any plugin getting time control automatically blocks any potential
>> migration.
> 
> Note my question asked for a 'any definitions of what a plugin ..' - so
> you could define it that way, another one is to think that in the future
> you may allow it and the plugin somehow interacts with migration not to
> change time at certain migration phases.
> 

I would be in favor to forbid usage for now in this context. I'm not 
sure why people would play with migration and plugins generally at this 
time (there might be experiments or use cases I'm not aware of), so a 
simple barrier preventing that seems ok.

This plugin is part of an experiment where we implement a qemu feature 
(icount=auto in this case) by using plugins. If it turns into a 
successful usage and this plugin becomes popular, we can always lift the 
limitation later.

@Alex, would you like to add this now (icount=auto is still not removed 
from qemu), or wait for integration, and add this as another patch?

>>>>>      b) The sleep in migration/dirtyrate.c points out g_usleep might
>>>>>         sleep for longer, so reads the actual wall clock time to
>>>>>         figure out a new 'now'.
>>>>
>>>> The current API mentions time starts at 0 from qemu startup. Maybe we could
>>>> consider in the future to change this behavior to retrieve time from an
>>>> existing migrated machine.
>>>
>>> Ah, I meant for (b) to be independent of (a) - not related to migration; just
>>> down to the fact you used g_usleep in the plugin and a g_usleep might sleep
>>> for a different amount of time than you asked.
>>>
>>
>> We know that, and the plugin is not meant to be "cycle accurate" in general,
>> we just set a upper bound for number of instructions we can execute in a
>> given amount of time (1/10 second for now).
>>
>> We compute the new time based on how many instructions effectively ran on
>> the most used cpu, so even if we slept a bit more than expected, it's
>> correct.
> 
> Ah OK.
> 
> Dave
> 
>>>>>      c) A fun thing to do with this would be to follow an external simulation
>>>>>         or 2nd qemu, trying to keep the two from running too far past
>>>>>         each other.
>>>>>
>>>>
>>>> Basically, to slow the first one, waiting for the replicated one to catch
>>>> up?
>>>
>>> Yes, something like that.
>>>
>>> Dave
>>>
>>>>> Dave >
>>>>>> Examples
>>>>>> --------
>>>>>>
>>>>>> Slow down execution of /bin/true:
>>>>>> $ num_insn=$(./build/qemu-x86_64 -plugin ./build/tests/plugin/libinsn.so -d plugin /bin/true |& grep total | sed -e 's/.*: //')
>>>>>> $ time ./build/qemu-x86_64 -plugin ./build/contrib/plugins/libips.so,ips=$(($num_insn/4)) /bin/true
>>>>>> real 4.000s
>>>>>>
>>>>>> Boot a Linux kernel simulating a 250MHz cpu:
>>>>>> $ /build/qemu-system-x86_64 -kernel /boot/vmlinuz-6.1.0-21-amd64 -append "console=ttyS0" -plugin ./build/contrib/plugins/libips.so,ips=$((250*1000*1000)) -smp 1 -m 512
>>>>>> check time until kernel panic on serial0
>>>>>>
>>>>>> Tested in system mode by booting a full debian system, and using:
>>>>>> $ sysbench cpu run
>>>>>> Performance decrease linearly with the given number of ips.
>>>>>>
>>>>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>>>> Message-Id: <20240530220610.1245424-7-pierrick.bouvier@linaro.org>
>>>>>> ---
>>>>>>     contrib/plugins/ips.c    | 164 +++++++++++++++++++++++++++++++++++++++
>>>>>>     contrib/plugins/Makefile |   1 +
>>>>>>     2 files changed, 165 insertions(+)
>>>>>>     create mode 100644 contrib/plugins/ips.c
>>>>>>
>>>>>> diff --git a/contrib/plugins/ips.c b/contrib/plugins/ips.c
>>>>>> new file mode 100644
>>>>>> index 0000000000..db77729264
>>>>>> --- /dev/null
>>>>>> +++ b/contrib/plugins/ips.c
>>>>>> @@ -0,0 +1,164 @@
>>>>>> +/*
>>>>>> + * ips rate limiting plugin.
>>>>>> + *
>>>>>> + * This plugin can be used to restrict the execution of a system to a
>>>>>> + * particular number of Instructions Per Second (ips). This controls
>>>>>> + * time as seen by the guest so while wall-clock time may be longer
>>>>>> + * from the guests point of view time will pass at the normal rate.
>>>>>> + *
>>>>>> + * This uses the new plugin API which allows the plugin to control
>>>>>> + * system time.
>>>>>> + *
>>>>>> + * Copyright (c) 2023 Linaro Ltd
>>>>>> + *
>>>>>> + * SPDX-License-Identifier: GPL-2.0-or-later
>>>>>> + */
>>>>>> +
>>>>>> +#include <stdio.h>
>>>>>> +#include <glib.h>
>>>>>> +#include <qemu-plugin.h>
>>>>>> +
>>>>>> +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
>>>>>> +
>>>>>> +/* how many times do we update time per sec */
>>>>>> +#define NUM_TIME_UPDATE_PER_SEC 10
>>>>>> +#define NSEC_IN_ONE_SEC (1000 * 1000 * 1000)
>>>>>> +
>>>>>> +static GMutex global_state_lock;
>>>>>> +
>>>>>> +static uint64_t max_insn_per_second = 1000 * 1000 * 1000; /* ips per core, per second */
>>>>>> +static uint64_t max_insn_per_quantum; /* trap every N instructions */
>>>>>> +static int64_t virtual_time_ns; /* last set virtual time */
>>>>>> +
>>>>>> +static const void *time_handle;
>>>>>> +
>>>>>> +typedef struct {
>>>>>> +    uint64_t total_insn;
>>>>>> +    uint64_t quantum_insn; /* insn in last quantum */
>>>>>> +    int64_t last_quantum_time; /* time when last quantum started */
>>>>>> +} vCPUTime;
>>>>>> +
>>>>>> +struct qemu_plugin_scoreboard *vcpus;
>>>>>> +
>>>>>> +/* return epoch time in ns */
>>>>>> +static int64_t now_ns(void)
>>>>>> +{
>>>>>> +    return g_get_real_time() * 1000;
>>>>>> +}
>>>>>> +
>>>>>> +static uint64_t num_insn_during(int64_t elapsed_ns)
>>>>>> +{
>>>>>> +    double num_secs = elapsed_ns / (double) NSEC_IN_ONE_SEC;
>>>>>> +    return num_secs * (double) max_insn_per_second;
>>>>>> +}
>>>>>> +
>>>>>> +static int64_t time_for_insn(uint64_t num_insn)
>>>>>> +{
>>>>>> +    double num_secs = (double) num_insn / (double) max_insn_per_second;
>>>>>> +    return num_secs * (double) NSEC_IN_ONE_SEC;
>>>>>> +}
>>>>>> +
>>>>>> +static void update_system_time(vCPUTime *vcpu)
>>>>>> +{
>>>>>> +    int64_t elapsed_ns = now_ns() - vcpu->last_quantum_time;
>>>>>> +    uint64_t max_insn = num_insn_during(elapsed_ns);
>>>>>> +
>>>>>> +    if (vcpu->quantum_insn >= max_insn) {
>>>>>> +        /* this vcpu ran faster than expected, so it has to sleep */
>>>>>> +        uint64_t insn_advance = vcpu->quantum_insn - max_insn;
>>>>>> +        uint64_t time_advance_ns = time_for_insn(insn_advance);
>>>>>> +        int64_t sleep_us = time_advance_ns / 1000;
>>>>>> +        g_usleep(sleep_us);
>>>>>> +    }
>>>>>> +
>>>>>> +    vcpu->total_insn += vcpu->quantum_insn;
>>>>>> +    vcpu->quantum_insn = 0;
>>>>>> +    vcpu->last_quantum_time = now_ns();
>>>>>> +
>>>>>> +    /* based on total number of instructions, what should be the new time? */
>>>>>> +    int64_t new_virtual_time = time_for_insn(vcpu->total_insn);
>>>>>> +
>>>>>> +    g_mutex_lock(&global_state_lock);
>>>>>> +
>>>>>> +    /* Time only moves forward. Another vcpu might have updated it already. */
>>>>>> +    if (new_virtual_time > virtual_time_ns) {
>>>>>> +        qemu_plugin_update_ns(time_handle, new_virtual_time);
>>>>>> +        virtual_time_ns = new_virtual_time;
>>>>>> +    }
>>>>>> +
>>>>>> +    g_mutex_unlock(&global_state_lock);
>>>>>> +}
>>>>>> +
>>>>>> +static void vcpu_init(qemu_plugin_id_t id, unsigned int cpu_index)
>>>>>> +{
>>>>>> +    vCPUTime *vcpu = qemu_plugin_scoreboard_find(vcpus, cpu_index);
>>>>>> +    vcpu->total_insn = 0;
>>>>>> +    vcpu->quantum_insn = 0;
>>>>>> +    vcpu->last_quantum_time = now_ns();
>>>>>> +}
>>>>>> +
>>>>>> +static void vcpu_exit(qemu_plugin_id_t id, unsigned int cpu_index)
>>>>>> +{
>>>>>> +    vCPUTime *vcpu = qemu_plugin_scoreboard_find(vcpus, cpu_index);
>>>>>> +    update_system_time(vcpu);
>>>>>> +}
>>>>>> +
>>>>>> +static void every_quantum_insn(unsigned int cpu_index, void *udata)
>>>>>> +{
>>>>>> +    vCPUTime *vcpu = qemu_plugin_scoreboard_find(vcpus, cpu_index);
>>>>>> +    g_assert(vcpu->quantum_insn >= max_insn_per_quantum);
>>>>>> +    update_system_time(vcpu);
>>>>>> +}
>>>>>> +
>>>>>> +static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
>>>>>> +{
>>>>>> +    size_t n_insns = qemu_plugin_tb_n_insns(tb);
>>>>>> +    qemu_plugin_u64 quantum_insn =
>>>>>> +        qemu_plugin_scoreboard_u64_in_struct(vcpus, vCPUTime, quantum_insn);
>>>>>> +    /* count (and eventually trap) once per tb */
>>>>>> +    qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
>>>>>> +        tb, QEMU_PLUGIN_INLINE_ADD_U64, quantum_insn, n_insns);
>>>>>> +    qemu_plugin_register_vcpu_tb_exec_cond_cb(
>>>>>> +        tb, every_quantum_insn,
>>>>>> +        QEMU_PLUGIN_CB_NO_REGS, QEMU_PLUGIN_COND_GE,
>>>>>> +        quantum_insn, max_insn_per_quantum, NULL);
>>>>>> +}
>>>>>> +
>>>>>> +static void plugin_exit(qemu_plugin_id_t id, void *udata)
>>>>>> +{
>>>>>> +    qemu_plugin_scoreboard_free(vcpus);
>>>>>> +}
>>>>>> +
>>>>>> +QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
>>>>>> +                                           const qemu_info_t *info, int argc,
>>>>>> +                                           char **argv)
>>>>>> +{
>>>>>> +    for (int i = 0; i < argc; i++) {
>>>>>> +        char *opt = argv[i];
>>>>>> +        g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
>>>>>> +        if (g_strcmp0(tokens[0], "ips") == 0) {
>>>>>> +            max_insn_per_second = g_ascii_strtoull(tokens[1], NULL, 10);
>>>>>> +            if (!max_insn_per_second && errno) {
>>>>>> +                fprintf(stderr, "%s: couldn't parse %s (%s)\n",
>>>>>> +                        __func__, tokens[1], g_strerror(errno));
>>>>>> +                return -1;
>>>>>> +            }
>>>>>> +        } else {
>>>>>> +            fprintf(stderr, "option parsing failed: %s\n", opt);
>>>>>> +            return -1;
>>>>>> +        }
>>>>>> +    }
>>>>>> +
>>>>>> +    vcpus = qemu_plugin_scoreboard_new(sizeof(vCPUTime));
>>>>>> +    max_insn_per_quantum = max_insn_per_second / NUM_TIME_UPDATE_PER_SEC;
>>>>>> +
>>>>>> +    time_handle = qemu_plugin_request_time_control();
>>>>>> +    g_assert(time_handle);
>>>>>> +
>>>>>> +    qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
>>>>>> +    qemu_plugin_register_vcpu_init_cb(id, vcpu_init);
>>>>>> +    qemu_plugin_register_vcpu_exit_cb(id, vcpu_exit);
>>>>>> +    qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
>>>>>> +
>>>>>> +    return 0;
>>>>>> +}
>>>>>> diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile
>>>>>> index 0b64d2c1e3..449ead1130 100644
>>>>>> --- a/contrib/plugins/Makefile
>>>>>> +++ b/contrib/plugins/Makefile
>>>>>> @@ -27,6 +27,7 @@ endif
>>>>>>     NAMES += hwprofile
>>>>>>     NAMES += cache
>>>>>>     NAMES += drcov
>>>>>> +NAMES += ips
>>>>>>     ifeq ($(CONFIG_WIN32),y)
>>>>>>     SO_SUFFIX := .dll
>>>>>> -- 
>>>>>> 2.39.2
>>>>>>

Re: [PATCH 9/9] contrib/plugins: add ips plugin example for cost modeling

[Dr. David Alan Gilbert]

* Pierrick Bouvier (pierrick.bouvier@linaro.org) wrote:
> On 6/17/24 13:56, Dr. David Alan Gilbert wrote:
> > * Pierrick Bouvier (pierrick.bouvier@linaro.org) wrote:
> > > On 6/14/24 15:00, Dr. David Alan Gilbert wrote:
> > > > * Pierrick Bouvier (pierrick.bouvier@linaro.org) wrote:
> > > > > Hi Dave,
> > > > > 
> > > > > On 6/12/24 14:02, Dr. David Alan Gilbert wrote:
> > > > > > * Alex Bennée (alex.bennee@linaro.org) wrote:
> > > > > > > From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> > > > > > > 
> > > > > > > This plugin uses the new time control interface to make decisions
> > > > > > > about the state of time during the emulation. The algorithm is
> > > > > > > currently very simple. The user specifies an ips rate which applies
> > > > > > > per core. If the core runs ahead of its allocated execution time the
> > > > > > > plugin sleeps for a bit to let real time catch up. Either way time is
> > > > > > > updated for the emulation as a function of total executed instructions
> > > > > > > with some adjustments for cores that idle.
> > > > > > 
> > > > > > A few random thoughts:
> > > > > >      a) Are there any definitions of what a plugin that controls time
> > > > > >         should do with a live migration?
> > > > > 
> > > > > It's not something that was considered as part of this work.
> > > > 
> > > > That's OK, the only thing is we need to stop anyone from hitting problems
> > > > when they don't realise it's not been addressed.
> > > > One way might be to add a migration blocker; see include/migration/blocker.h
> > > > then you might print something like 'Migration not available due to plugin ....'
> > > > 
> > > 
> > > So basically, we could make a call to migrate_add_blocker(), when someone
> > > request time_control through plugin API?
> > > 
> > > IMHO, it's something that should be part of plugin API (if any plugin calls
> > > qemu_plugin_request_time_control()), instead of the plugin code itself. This
> > > way, any plugin getting time control automatically blocks any potential
> > > migration.
> > 
> > Note my question asked for a 'any definitions of what a plugin ..' - so
> > you could define it that way, another one is to think that in the future
> > you may allow it and the plugin somehow interacts with migration not to
> > change time at certain migration phases.
> > 
> 
> I would be in favor to forbid usage for now in this context. I'm not sure
> why people would play with migration and plugins generally at this time
> (there might be experiments or use cases I'm not aware of), so a simple
> barrier preventing that seems ok.
> 
> This plugin is part of an experiment where we implement a qemu feature
> (icount=auto in this case) by using plugins. If it turns into a successful
> usage and this plugin becomes popular, we can always lift the limitation
> later.

Sounds reasonable to me.

Dave

> @Alex, would you like to add this now (icount=auto is still not removed from
> qemu), or wait for integration, and add this as another patch?
> 
> > > > > >      b) The sleep in migration/dirtyrate.c points out g_usleep might
> > > > > >         sleep for longer, so reads the actual wall clock time to
> > > > > >         figure out a new 'now'.
> > > > > 
> > > > > The current API mentions time starts at 0 from qemu startup. Maybe we could
> > > > > consider in the future to change this behavior to retrieve time from an
> > > > > existing migrated machine.
> > > > 
> > > > Ah, I meant for (b) to be independent of (a) - not related to migration; just
> > > > down to the fact you used g_usleep in the plugin and a g_usleep might sleep
> > > > for a different amount of time than you asked.
> > > > 
> > > 
> > > We know that, and the plugin is not meant to be "cycle accurate" in general,
> > > we just set a upper bound for number of instructions we can execute in a
> > > given amount of time (1/10 second for now).
> > > 
> > > We compute the new time based on how many instructions effectively ran on
> > > the most used cpu, so even if we slept a bit more than expected, it's
> > > correct.
> > 
> > Ah OK.
> > 
> > Dave
> > 
> > > > > >      c) A fun thing to do with this would be to follow an external simulation
> > > > > >         or 2nd qemu, trying to keep the two from running too far past
> > > > > >         each other.
> > > > > > 
> > > > > 
> > > > > Basically, to slow the first one, waiting for the replicated one to catch
> > > > > up?
> > > > 
> > > > Yes, something like that.
> > > > 
> > > > Dave
> > > > 
> > > > > > Dave >
> > > > > > > Examples
> > > > > > > --------
> > > > > > > 
> > > > > > > Slow down execution of /bin/true:
> > > > > > > $ num_insn=$(./build/qemu-x86_64 -plugin ./build/tests/plugin/libinsn.so -d plugin /bin/true |& grep total | sed -e 's/.*: //')
> > > > > > > $ time ./build/qemu-x86_64 -plugin ./build/contrib/plugins/libips.so,ips=$(($num_insn/4)) /bin/true
> > > > > > > real 4.000s
> > > > > > > 
> > > > > > > Boot a Linux kernel simulating a 250MHz cpu:
> > > > > > > $ /build/qemu-system-x86_64 -kernel /boot/vmlinuz-6.1.0-21-amd64 -append "console=ttyS0" -plugin ./build/contrib/plugins/libips.so,ips=$((250*1000*1000)) -smp 1 -m 512
> > > > > > > check time until kernel panic on serial0
> > > > > > > 
> > > > > > > Tested in system mode by booting a full debian system, and using:
> > > > > > > $ sysbench cpu run
> > > > > > > Performance decrease linearly with the given number of ips.
> > > > > > > 
> > > > > > > Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> > > > > > > Message-Id: <20240530220610.1245424-7-pierrick.bouvier@linaro.org>
> > > > > > > ---
> > > > > > >     contrib/plugins/ips.c    | 164 +++++++++++++++++++++++++++++++++++++++
> > > > > > >     contrib/plugins/Makefile |   1 +
> > > > > > >     2 files changed, 165 insertions(+)
> > > > > > >     create mode 100644 contrib/plugins/ips.c
> > > > > > > 
> > > > > > > diff --git a/contrib/plugins/ips.c b/contrib/plugins/ips.c
> > > > > > > new file mode 100644
> > > > > > > index 0000000000..db77729264
> > > > > > > --- /dev/null
> > > > > > > +++ b/contrib/plugins/ips.c
> > > > > > > @@ -0,0 +1,164 @@
> > > > > > > +/*
> > > > > > > + * ips rate limiting plugin.
> > > > > > > + *
> > > > > > > + * This plugin can be used to restrict the execution of a system to a
> > > > > > > + * particular number of Instructions Per Second (ips). This controls
> > > > > > > + * time as seen by the guest so while wall-clock time may be longer
> > > > > > > + * from the guests point of view time will pass at the normal rate.
> > > > > > > + *
> > > > > > > + * This uses the new plugin API which allows the plugin to control
> > > > > > > + * system time.
> > > > > > > + *
> > > > > > > + * Copyright (c) 2023 Linaro Ltd
> > > > > > > + *
> > > > > > > + * SPDX-License-Identifier: GPL-2.0-or-later
> > > > > > > + */
> > > > > > > +
> > > > > > > +#include <stdio.h>
> > > > > > > +#include <glib.h>
> > > > > > > +#include <qemu-plugin.h>
> > > > > > > +
> > > > > > > +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
> > > > > > > +
> > > > > > > +/* how many times do we update time per sec */
> > > > > > > +#define NUM_TIME_UPDATE_PER_SEC 10
> > > > > > > +#define NSEC_IN_ONE_SEC (1000 * 1000 * 1000)
> > > > > > > +
> > > > > > > +static GMutex global_state_lock;
> > > > > > > +
> > > > > > > +static uint64_t max_insn_per_second = 1000 * 1000 * 1000; /* ips per core, per second */
> > > > > > > +static uint64_t max_insn_per_quantum; /* trap every N instructions */
> > > > > > > +static int64_t virtual_time_ns; /* last set virtual time */
> > > > > > > +
> > > > > > > +static const void *time_handle;
> > > > > > > +
> > > > > > > +typedef struct {
> > > > > > > +    uint64_t total_insn;
> > > > > > > +    uint64_t quantum_insn; /* insn in last quantum */
> > > > > > > +    int64_t last_quantum_time; /* time when last quantum started */
> > > > > > > +} vCPUTime;
> > > > > > > +
> > > > > > > +struct qemu_plugin_scoreboard *vcpus;
> > > > > > > +
> > > > > > > +/* return epoch time in ns */
> > > > > > > +static int64_t now_ns(void)
> > > > > > > +{
> > > > > > > +    return g_get_real_time() * 1000;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static uint64_t num_insn_during(int64_t elapsed_ns)
> > > > > > > +{
> > > > > > > +    double num_secs = elapsed_ns / (double) NSEC_IN_ONE_SEC;
> > > > > > > +    return num_secs * (double) max_insn_per_second;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static int64_t time_for_insn(uint64_t num_insn)
> > > > > > > +{
> > > > > > > +    double num_secs = (double) num_insn / (double) max_insn_per_second;
> > > > > > > +    return num_secs * (double) NSEC_IN_ONE_SEC;
> > > > > > > +}
> > > > > > > +
> > > > > > > +static void update_system_time(vCPUTime *vcpu)
> > > > > > > +{
> > > > > > > +    int64_t elapsed_ns = now_ns() - vcpu->last_quantum_time;
> > > > > > > +    uint64_t max_insn = num_insn_during(elapsed_ns);
> > > > > > > +
> > > > > > > +    if (vcpu->quantum_insn >= max_insn) {
> > > > > > > +        /* this vcpu ran faster than expected, so it has to sleep */
> > > > > > > +        uint64_t insn_advance = vcpu->quantum_insn - max_insn;
> > > > > > > +        uint64_t time_advance_ns = time_for_insn(insn_advance);
> > > > > > > +        int64_t sleep_us = time_advance_ns / 1000;
> > > > > > > +        g_usleep(sleep_us);
> > > > > > > +    }
> > > > > > > +
> > > > > > > +    vcpu->total_insn += vcpu->quantum_insn;
> > > > > > > +    vcpu->quantum_insn = 0;
> > > > > > > +    vcpu->last_quantum_time = now_ns();
> > > > > > > +
> > > > > > > +    /* based on total number of instructions, what should be the new time? */
> > > > > > > +    int64_t new_virtual_time = time_for_insn(vcpu->total_insn);
> > > > > > > +
> > > > > > > +    g_mutex_lock(&global_state_lock);
> > > > > > > +
> > > > > > > +    /* Time only moves forward. Another vcpu might have updated it already. */
> > > > > > > +    if (new_virtual_time > virtual_time_ns) {
> > > > > > > +        qemu_plugin_update_ns(time_handle, new_virtual_time);
> > > > > > > +        virtual_time_ns = new_virtual_time;
> > > > > > > +    }
> > > > > > > +
> > > > > > > +    g_mutex_unlock(&global_state_lock);
> > > > > > > +}
> > > > > > > +
> > > > > > > +static void vcpu_init(qemu_plugin_id_t id, unsigned int cpu_index)
> > > > > > > +{
> > > > > > > +    vCPUTime *vcpu = qemu_plugin_scoreboard_find(vcpus, cpu_index);
> > > > > > > +    vcpu->total_insn = 0;
> > > > > > > +    vcpu->quantum_insn = 0;
> > > > > > > +    vcpu->last_quantum_time = now_ns();
> > > > > > > +}
> > > > > > > +
> > > > > > > +static void vcpu_exit(qemu_plugin_id_t id, unsigned int cpu_index)
> > > > > > > +{
> > > > > > > +    vCPUTime *vcpu = qemu_plugin_scoreboard_find(vcpus, cpu_index);
> > > > > > > +    update_system_time(vcpu);
> > > > > > > +}
> > > > > > > +
> > > > > > > +static void every_quantum_insn(unsigned int cpu_index, void *udata)
> > > > > > > +{
> > > > > > > +    vCPUTime *vcpu = qemu_plugin_scoreboard_find(vcpus, cpu_index);
> > > > > > > +    g_assert(vcpu->quantum_insn >= max_insn_per_quantum);
> > > > > > > +    update_system_time(vcpu);
> > > > > > > +}
> > > > > > > +
> > > > > > > +static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
> > > > > > > +{
> > > > > > > +    size_t n_insns = qemu_plugin_tb_n_insns(tb);
> > > > > > > +    qemu_plugin_u64 quantum_insn =
> > > > > > > +        qemu_plugin_scoreboard_u64_in_struct(vcpus, vCPUTime, quantum_insn);
> > > > > > > +    /* count (and eventually trap) once per tb */
> > > > > > > +    qemu_plugin_register_vcpu_tb_exec_inline_per_vcpu(
> > > > > > > +        tb, QEMU_PLUGIN_INLINE_ADD_U64, quantum_insn, n_insns);
> > > > > > > +    qemu_plugin_register_vcpu_tb_exec_cond_cb(
> > > > > > > +        tb, every_quantum_insn,
> > > > > > > +        QEMU_PLUGIN_CB_NO_REGS, QEMU_PLUGIN_COND_GE,
> > > > > > > +        quantum_insn, max_insn_per_quantum, NULL);
> > > > > > > +}
> > > > > > > +
> > > > > > > +static void plugin_exit(qemu_plugin_id_t id, void *udata)
> > > > > > > +{
> > > > > > > +    qemu_plugin_scoreboard_free(vcpus);
> > > > > > > +}
> > > > > > > +
> > > > > > > +QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
> > > > > > > +                                           const qemu_info_t *info, int argc,
> > > > > > > +                                           char **argv)
> > > > > > > +{
> > > > > > > +    for (int i = 0; i < argc; i++) {
> > > > > > > +        char *opt = argv[i];
> > > > > > > +        g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
> > > > > > > +        if (g_strcmp0(tokens[0], "ips") == 0) {
> > > > > > > +            max_insn_per_second = g_ascii_strtoull(tokens[1], NULL, 10);
> > > > > > > +            if (!max_insn_per_second && errno) {
> > > > > > > +                fprintf(stderr, "%s: couldn't parse %s (%s)\n",
> > > > > > > +                        __func__, tokens[1], g_strerror(errno));
> > > > > > > +                return -1;
> > > > > > > +            }
> > > > > > > +        } else {
> > > > > > > +            fprintf(stderr, "option parsing failed: %s\n", opt);
> > > > > > > +            return -1;
> > > > > > > +        }
> > > > > > > +    }
> > > > > > > +
> > > > > > > +    vcpus = qemu_plugin_scoreboard_new(sizeof(vCPUTime));
> > > > > > > +    max_insn_per_quantum = max_insn_per_second / NUM_TIME_UPDATE_PER_SEC;
> > > > > > > +
> > > > > > > +    time_handle = qemu_plugin_request_time_control();
> > > > > > > +    g_assert(time_handle);
> > > > > > > +
> > > > > > > +    qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
> > > > > > > +    qemu_plugin_register_vcpu_init_cb(id, vcpu_init);
> > > > > > > +    qemu_plugin_register_vcpu_exit_cb(id, vcpu_exit);
> > > > > > > +    qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
> > > > > > > +
> > > > > > > +    return 0;
> > > > > > > +}
> > > > > > > diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile
> > > > > > > index 0b64d2c1e3..449ead1130 100644
> > > > > > > --- a/contrib/plugins/Makefile
> > > > > > > +++ b/contrib/plugins/Makefile
> > > > > > > @@ -27,6 +27,7 @@ endif
> > > > > > >     NAMES += hwprofile
> > > > > > >     NAMES += cache
> > > > > > >     NAMES += drcov
> > > > > > > +NAMES += ips
> > > > > > >     ifeq ($(CONFIG_WIN32),y)
> > > > > > >     SO_SUFFIX := .dll
> > > > > > > -- 
> > > > > > > 2.39.2
> > > > > > > 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

Re: [PATCH 9/9] contrib/plugins: add ips plugin example for cost modeling

[Alex Bennée]

Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:

> On 6/17/24 13:56, Dr. David Alan Gilbert wrote:
>> * Pierrick Bouvier (pierrick.bouvier@linaro.org) wrote:
>>> On 6/14/24 15:00, Dr. David Alan Gilbert wrote:
>>>> * Pierrick Bouvier (pierrick.bouvier@linaro.org) wrote:
>>>>> Hi Dave,
>>>>>
>>>>> On 6/12/24 14:02, Dr. David Alan Gilbert wrote:
>>>>>> * Alex Bennée (alex.bennee@linaro.org) wrote:
>>>>>>> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>>>>>
>>>>>>> This plugin uses the new time control interface to make decisions
>>>>>>> about the state of time during the emulation. The algorithm is
>>>>>>> currently very simple. The user specifies an ips rate which applies
>>>>>>> per core. If the core runs ahead of its allocated execution time the
>>>>>>> plugin sleeps for a bit to let real time catch up. Either way time is
>>>>>>> updated for the emulation as a function of total executed instructions
>>>>>>> with some adjustments for cores that idle.
>>>>>>
>>>>>> A few random thoughts:
>>>>>>      a) Are there any definitions of what a plugin that controls time
>>>>>>         should do with a live migration?
>>>>>
>>>>> It's not something that was considered as part of this work.
>>>>
>>>> That's OK, the only thing is we need to stop anyone from hitting problems
>>>> when they don't realise it's not been addressed.
>>>> One way might be to add a migration blocker; see include/migration/blocker.h
>>>> then you might print something like 'Migration not available due to plugin ....'
>>>>
>>>
>>> So basically, we could make a call to migrate_add_blocker(), when someone
>>> request time_control through plugin API?
>>>
>>> IMHO, it's something that should be part of plugin API (if any plugin calls
>>> qemu_plugin_request_time_control()), instead of the plugin code itself. This
>>> way, any plugin getting time control automatically blocks any potential
>>> migration.
>> Note my question asked for a 'any definitions of what a plugin ..' -
>> so
>> you could define it that way, another one is to think that in the future
>> you may allow it and the plugin somehow interacts with migration not to
>> change time at certain migration phases.
>> 
>
> I would be in favor to forbid usage for now in this context. I'm not
> sure why people would play with migration and plugins generally at
> this time (there might be experiments or use cases I'm not aware of),
> so a simple barrier preventing that seems ok.
>
> This plugin is part of an experiment where we implement a qemu feature
> (icount=auto in this case) by using plugins. If it turns into a
> successful usage and this plugin becomes popular, we can always lift
> the limitation later.
>
> @Alex, would you like to add this now (icount=auto is still not
> removed from qemu), or wait for integration, and add this as another
> patch?

I think we follow the deprecation process so once integrated we post a
deprecation notice in:

  https://qemu.readthedocs.io/en/master/about/deprecated.html

and then remove it after a couple of releases.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 9/9] contrib/plugins: add ips plugin example for cost modeling

[Pierrick Bouvier]

On 6/18/24 02:53, Alex Bennée wrote:
> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> 
>> On 6/17/24 13:56, Dr. David Alan Gilbert wrote:
>>> * Pierrick Bouvier (pierrick.bouvier@linaro.org) wrote:
>>>> On 6/14/24 15:00, Dr. David Alan Gilbert wrote:
>>>>> * Pierrick Bouvier (pierrick.bouvier@linaro.org) wrote:
>>>>>> Hi Dave,
>>>>>>
>>>>>> On 6/12/24 14:02, Dr. David Alan Gilbert wrote:
>>>>>>> * Alex Bennée (alex.bennee@linaro.org) wrote:
>>>>>>>> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>>>>>>
>>>>>>>> This plugin uses the new time control interface to make decisions
>>>>>>>> about the state of time during the emulation. The algorithm is
>>>>>>>> currently very simple. The user specifies an ips rate which applies
>>>>>>>> per core. If the core runs ahead of its allocated execution time the
>>>>>>>> plugin sleeps for a bit to let real time catch up. Either way time is
>>>>>>>> updated for the emulation as a function of total executed instructions
>>>>>>>> with some adjustments for cores that idle.
>>>>>>>
>>>>>>> A few random thoughts:
>>>>>>>       a) Are there any definitions of what a plugin that controls time
>>>>>>>          should do with a live migration?
>>>>>>
>>>>>> It's not something that was considered as part of this work.
>>>>>
>>>>> That's OK, the only thing is we need to stop anyone from hitting problems
>>>>> when they don't realise it's not been addressed.
>>>>> One way might be to add a migration blocker; see include/migration/blocker.h
>>>>> then you might print something like 'Migration not available due to plugin ....'
>>>>>
>>>>
>>>> So basically, we could make a call to migrate_add_blocker(), when someone
>>>> request time_control through plugin API?
>>>>
>>>> IMHO, it's something that should be part of plugin API (if any plugin calls
>>>> qemu_plugin_request_time_control()), instead of the plugin code itself. This
>>>> way, any plugin getting time control automatically blocks any potential
>>>> migration.
>>> Note my question asked for a 'any definitions of what a plugin ..' -
>>> so
>>> you could define it that way, another one is to think that in the future
>>> you may allow it and the plugin somehow interacts with migration not to
>>> change time at certain migration phases.
>>>
>>
>> I would be in favor to forbid usage for now in this context. I'm not
>> sure why people would play with migration and plugins generally at
>> this time (there might be experiments or use cases I'm not aware of),
>> so a simple barrier preventing that seems ok.
>>
>> This plugin is part of an experiment where we implement a qemu feature
>> (icount=auto in this case) by using plugins. If it turns into a
>> successful usage and this plugin becomes popular, we can always lift
>> the limitation later.
>>
>> @Alex, would you like to add this now (icount=auto is still not
>> removed from qemu), or wait for integration, and add this as another
>> patch?
> 
> I think we follow the deprecation process so once integrated we post a
> deprecation notice in:
> 
>    https://qemu.readthedocs.io/en/master/about/deprecated.html
> 
> and then remove it after a couple of releases.
> 

Sorry, I was not clear. I meant do we add a blocker in case someone 
tries to migrate a vm while this plugin is used?

Re: [PATCH 9/9] contrib/plugins: add ips plugin example for cost modeling

[Alex Bennée]

Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:

> On 6/18/24 02:53, Alex Bennée wrote:
>> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
>> 
>>> On 6/17/24 13:56, Dr. David Alan Gilbert wrote:
>>>> * Pierrick Bouvier (pierrick.bouvier@linaro.org) wrote:
>>>>> On 6/14/24 15:00, Dr. David Alan Gilbert wrote:
>>>>>> * Pierrick Bouvier (pierrick.bouvier@linaro.org) wrote:
>>>>>>> Hi Dave,
>>>>>>>
>>>>>>> On 6/12/24 14:02, Dr. David Alan Gilbert wrote:
>>>>>>>> * Alex Bennée (alex.bennee@linaro.org) wrote:
>>>>>>>>> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>>>>>>>
>>>>>>>>> This plugin uses the new time control interface to make decisions
>>>>>>>>> about the state of time during the emulation. The algorithm is
>>>>>>>>> currently very simple. The user specifies an ips rate which applies
>>>>>>>>> per core. If the core runs ahead of its allocated execution time the
>>>>>>>>> plugin sleeps for a bit to let real time catch up. Either way time is
>>>>>>>>> updated for the emulation as a function of total executed instructions
>>>>>>>>> with some adjustments for cores that idle.
>>>>>>>>
>>>>>>>> A few random thoughts:
>>>>>>>>       a) Are there any definitions of what a plugin that controls time
>>>>>>>>          should do with a live migration?
>>>>>>>
>>>>>>> It's not something that was considered as part of this work.
>>>>>>
>>>>>> That's OK, the only thing is we need to stop anyone from hitting problems
>>>>>> when they don't realise it's not been addressed.
>>>>>> One way might be to add a migration blocker; see include/migration/blocker.h
>>>>>> then you might print something like 'Migration not available due to plugin ....'
>>>>>>
>>>>>
>>>>> So basically, we could make a call to migrate_add_blocker(), when someone
>>>>> request time_control through plugin API?
>>>>>
>>>>> IMHO, it's something that should be part of plugin API (if any plugin calls
>>>>> qemu_plugin_request_time_control()), instead of the plugin code itself. This
>>>>> way, any plugin getting time control automatically blocks any potential
>>>>> migration.
>>>> Note my question asked for a 'any definitions of what a plugin ..' -
>>>> so
>>>> you could define it that way, another one is to think that in the future
>>>> you may allow it and the plugin somehow interacts with migration not to
>>>> change time at certain migration phases.
>>>>
>>>
>>> I would be in favor to forbid usage for now in this context. I'm not
>>> sure why people would play with migration and plugins generally at
>>> this time (there might be experiments or use cases I'm not aware of),
>>> so a simple barrier preventing that seems ok.
>>>
>>> This plugin is part of an experiment where we implement a qemu feature
>>> (icount=auto in this case) by using plugins. If it turns into a
>>> successful usage and this plugin becomes popular, we can always lift
>>> the limitation later.
>>>
>>> @Alex, would you like to add this now (icount=auto is still not
>>> removed from qemu), or wait for integration, and add this as another
>>> patch?
>> I think we follow the deprecation process so once integrated we post
>> a
>> deprecation notice in:
>>    https://qemu.readthedocs.io/en/master/about/deprecated.html
>> and then remove it after a couple of releases.
>> 
>
> Sorry, I was not clear. I meant do we add a blocker in case someone
> tries to migrate a vm while this plugin is used?

Oh yes - I can add that in the core plugin code.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 9/9] contrib/plugins: add ips plugin example for cost modeling

[Pierrick Bouvier]

On 6/19/24 02:49, Alex Bennée wrote:
> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> 
>> On 6/18/24 02:53, Alex Bennée wrote:
>>> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
>>>
>>>> On 6/17/24 13:56, Dr. David Alan Gilbert wrote:
>>>>> * Pierrick Bouvier (pierrick.bouvier@linaro.org) wrote:
>>>>>> On 6/14/24 15:00, Dr. David Alan Gilbert wrote:
>>>>>>> * Pierrick Bouvier (pierrick.bouvier@linaro.org) wrote:
>>>>>>>> Hi Dave,
>>>>>>>>
>>>>>>>> On 6/12/24 14:02, Dr. David Alan Gilbert wrote:
>>>>>>>>> * Alex Bennée (alex.bennee@linaro.org) wrote:
>>>>>>>>>> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>>>>>>>>
>>>>>>>>>> This plugin uses the new time control interface to make decisions
>>>>>>>>>> about the state of time during the emulation. The algorithm is
>>>>>>>>>> currently very simple. The user specifies an ips rate which applies
>>>>>>>>>> per core. If the core runs ahead of its allocated execution time the
>>>>>>>>>> plugin sleeps for a bit to let real time catch up. Either way time is
>>>>>>>>>> updated for the emulation as a function of total executed instructions
>>>>>>>>>> with some adjustments for cores that idle.
>>>>>>>>>
>>>>>>>>> A few random thoughts:
>>>>>>>>>        a) Are there any definitions of what a plugin that controls time
>>>>>>>>>           should do with a live migration?
>>>>>>>>
>>>>>>>> It's not something that was considered as part of this work.
>>>>>>>
>>>>>>> That's OK, the only thing is we need to stop anyone from hitting problems
>>>>>>> when they don't realise it's not been addressed.
>>>>>>> One way might be to add a migration blocker; see include/migration/blocker.h
>>>>>>> then you might print something like 'Migration not available due to plugin ....'
>>>>>>>
>>>>>>
>>>>>> So basically, we could make a call to migrate_add_blocker(), when someone
>>>>>> request time_control through plugin API?
>>>>>>
>>>>>> IMHO, it's something that should be part of plugin API (if any plugin calls
>>>>>> qemu_plugin_request_time_control()), instead of the plugin code itself. This
>>>>>> way, any plugin getting time control automatically blocks any potential
>>>>>> migration.
>>>>> Note my question asked for a 'any definitions of what a plugin ..' -
>>>>> so
>>>>> you could define it that way, another one is to think that in the future
>>>>> you may allow it and the plugin somehow interacts with migration not to
>>>>> change time at certain migration phases.
>>>>>
>>>>
>>>> I would be in favor to forbid usage for now in this context. I'm not
>>>> sure why people would play with migration and plugins generally at
>>>> this time (there might be experiments or use cases I'm not aware of),
>>>> so a simple barrier preventing that seems ok.
>>>>
>>>> This plugin is part of an experiment where we implement a qemu feature
>>>> (icount=auto in this case) by using plugins. If it turns into a
>>>> successful usage and this plugin becomes popular, we can always lift
>>>> the limitation later.
>>>>
>>>> @Alex, would you like to add this now (icount=auto is still not
>>>> removed from qemu), or wait for integration, and add this as another
>>>> patch?
>>> I think we follow the deprecation process so once integrated we post
>>> a
>>> deprecation notice in:
>>>     https://qemu.readthedocs.io/en/master/about/deprecated.html
>>> and then remove it after a couple of releases.
>>>
>>
>> Sorry, I was not clear. I meant do we add a blocker in case someone
>> tries to migrate a vm while this plugin is used?
> 
> Oh yes - I can add that in the core plugin code.
> 

Thanks!

Re: [PATCH 9/9] contrib/plugins: add ips plugin example for cost modeling

[Philippe Mathieu-Daudé]

On 12/6/24 17:35, Alex Bennée wrote:
> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> 
> This plugin uses the new time control interface to make decisions
> about the state of time during the emulation. The algorithm is
> currently very simple. The user specifies an ips rate which applies

... IPS rate (Instructions Per Second) which ...

> per core. If the core runs ahead of its allocated execution time the
> plugin sleeps for a bit to let real time catch up. Either way time is
> updated for the emulation as a function of total executed instructions
> with some adjustments for cores that idle.
> 
> Examples
> --------
> 
> Slow down execution of /bin/true:
> $ num_insn=$(./build/qemu-x86_64 -plugin ./build/tests/plugin/libinsn.so -d plugin /bin/true |& grep total | sed -e 's/.*: //')
> $ time ./build/qemu-x86_64 -plugin ./build/contrib/plugins/libips.so,ips=$(($num_insn/4)) /bin/true
> real 4.000s
> 
> Boot a Linux kernel simulating a 250MHz cpu:
> $ /build/qemu-system-x86_64 -kernel /boot/vmlinuz-6.1.0-21-amd64 -append "console=ttyS0" -plugin ./build/contrib/plugins/libips.so,ips=$((250*1000*1000)) -smp 1 -m 512
> check time until kernel panic on serial0
> 
> Tested in system mode by booting a full debian system, and using:
> $ sysbench cpu run
> Performance decrease linearly with the given number of ips.
> 
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Message-Id: <20240530220610.1245424-7-pierrick.bouvier@linaro.org>
> ---
>   contrib/plugins/ips.c    | 164 +++++++++++++++++++++++++++++++++++++++
>   contrib/plugins/Makefile |   1 +
>   2 files changed, 165 insertions(+)
>   create mode 100644 contrib/plugins/ips.c
> 
> diff --git a/contrib/plugins/ips.c b/contrib/plugins/ips.c
> new file mode 100644
> index 0000000000..db77729264
> --- /dev/null
> +++ b/contrib/plugins/ips.c
> @@ -0,0 +1,164 @@
> +/*
> + * ips rate limiting plugin.

The plugin names are really to packed to my taste (each time I look for
one I have to open most source files to figure out the correct one); so
please ease my life by using a more descriptive header at least:

      Instructions Per Second (IPS) rate limiting plugin.

Thanks.

> + * This plugin can be used to restrict the execution of a system to a
> + * particular number of Instructions Per Second (ips). This controls
> + * time as seen by the guest so while wall-clock time may be longer
> + * from the guests point of view time will pass at the normal rate.
> + *
> + * This uses the new plugin API which allows the plugin to control
> + * system time.
> + *
> + * Copyright (c) 2023 Linaro Ltd
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */

Re: [PATCH 9/9] contrib/plugins: add ips plugin example for cost modeling

[Pierrick Bouvier]

On 6/13/24 01:54, Philippe Mathieu-Daudé wrote:
> On 12/6/24 17:35, Alex Bennée wrote:
>> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>
>> This plugin uses the new time control interface to make decisions
>> about the state of time during the emulation. The algorithm is
>> currently very simple. The user specifies an ips rate which applies
> 
> ... IPS rate (Instructions Per Second) which ...
> 
>> per core. If the core runs ahead of its allocated execution time the
>> plugin sleeps for a bit to let real time catch up. Either way time is
>> updated for the emulation as a function of total executed instructions
>> with some adjustments for cores that idle.
>>
>> Examples
>> --------
>>
>> Slow down execution of /bin/true:
>> $ num_insn=$(./build/qemu-x86_64 -plugin ./build/tests/plugin/libinsn.so -d plugin /bin/true |& grep total | sed -e 's/.*: //')
>> $ time ./build/qemu-x86_64 -plugin ./build/contrib/plugins/libips.so,ips=$(($num_insn/4)) /bin/true
>> real 4.000s
>>
>> Boot a Linux kernel simulating a 250MHz cpu:
>> $ /build/qemu-system-x86_64 -kernel /boot/vmlinuz-6.1.0-21-amd64 -append "console=ttyS0" -plugin ./build/contrib/plugins/libips.so,ips=$((250*1000*1000)) -smp 1 -m 512
>> check time until kernel panic on serial0
>>
>> Tested in system mode by booting a full debian system, and using:
>> $ sysbench cpu run
>> Performance decrease linearly with the given number of ips.
>>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> Message-Id: <20240530220610.1245424-7-pierrick.bouvier@linaro.org>
>> ---
>>    contrib/plugins/ips.c    | 164 +++++++++++++++++++++++++++++++++++++++
>>    contrib/plugins/Makefile |   1 +
>>    2 files changed, 165 insertions(+)
>>    create mode 100644 contrib/plugins/ips.c
>>
>> diff --git a/contrib/plugins/ips.c b/contrib/plugins/ips.c
>> new file mode 100644
>> index 0000000000..db77729264
>> --- /dev/null
>> +++ b/contrib/plugins/ips.c
>> @@ -0,0 +1,164 @@
>> +/*
>> + * ips rate limiting plugin.
> 
> The plugin names are really to packed to my taste (each time I look for
> one I have to open most source files to figure out the correct one); so
> please ease my life by using a more descriptive header at least:
> 
>        Instructions Per Second (IPS) rate limiting plugin.
> 
> Thanks.
> 

I agree most of the plugin names are pretty cryptic, and they are 
lacking a common "help" system, to describe what they do, and which 
options are available for them. It's definitely something we could add 
in the future.

Regarding what you reported, I'm totally ok with the change.

However, since this is a new series, I'm not if I or Alex should change 
it. If it's ok for you to modify this Alex, it could be simpler than 
waiting for me to push a new patch with just this.

Let me know how you deal with this usually, and I'll do what is needed.

Thanks,
Pierrick

>> + * This plugin can be used to restrict the execution of a system to a
>> + * particular number of Instructions Per Second (ips). This controls
>> + * time as seen by the guest so while wall-clock time may be longer
>> + * from the guests point of view time will pass at the normal rate.
>> + *
>> + * This uses the new plugin API which allows the plugin to control
>> + * system time.
>> + *
>> + * Copyright (c) 2023 Linaro Ltd
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
> 

Re: [PATCH 9/9] contrib/plugins: add ips plugin example for cost modeling

[Alex Bennée]

Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:

> On 6/13/24 01:54, Philippe Mathieu-Daudé wrote:
>> On 12/6/24 17:35, Alex Bennée wrote:
>>> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>
>>> This plugin uses the new time control interface to make decisions
>>> about the state of time during the emulation. The algorithm is
>>> currently very simple. The user specifies an ips rate which applies
>> ... IPS rate (Instructions Per Second) which ...
>> 
>>> per core. If the core runs ahead of its allocated execution time the
>>> plugin sleeps for a bit to let real time catch up. Either way time is
>>> updated for the emulation as a function of total executed instructions
>>> with some adjustments for cores that idle.
>>>
>>> Examples
>>> --------
>>>
>>> Slow down execution of /bin/true:
>>> $ num_insn=$(./build/qemu-x86_64 -plugin ./build/tests/plugin/libinsn.so -d plugin /bin/true |& grep total | sed -e 's/.*: //')
>>> $ time ./build/qemu-x86_64 -plugin ./build/contrib/plugins/libips.so,ips=$(($num_insn/4)) /bin/true
>>> real 4.000s
>>>
>>> Boot a Linux kernel simulating a 250MHz cpu:
>>> $ /build/qemu-system-x86_64 -kernel /boot/vmlinuz-6.1.0-21-amd64 -append "console=ttyS0" -plugin ./build/contrib/plugins/libips.so,ips=$((250*1000*1000)) -smp 1 -m 512
>>> check time until kernel panic on serial0
>>>
>>> Tested in system mode by booting a full debian system, and using:
>>> $ sysbench cpu run
>>> Performance decrease linearly with the given number of ips.
>>>
>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>> Message-Id: <20240530220610.1245424-7-pierrick.bouvier@linaro.org>
>>> ---
>>>    contrib/plugins/ips.c    | 164 +++++++++++++++++++++++++++++++++++++++
>>>    contrib/plugins/Makefile |   1 +
>>>    2 files changed, 165 insertions(+)
>>>    create mode 100644 contrib/plugins/ips.c
>>>
>>> diff --git a/contrib/plugins/ips.c b/contrib/plugins/ips.c
>>> new file mode 100644
>>> index 0000000000..db77729264
>>> --- /dev/null
>>> +++ b/contrib/plugins/ips.c
>>> @@ -0,0 +1,164 @@
>>> +/*
>>> + * ips rate limiting plugin.
>> The plugin names are really to packed to my taste (each time I look
>> for
>> one I have to open most source files to figure out the correct one); so
>> please ease my life by using a more descriptive header at least:
>>        Instructions Per Second (IPS) rate limiting plugin.
>> Thanks.
>> 
>
> I agree most of the plugin names are pretty cryptic, and they are
> lacking a common "help" system, to describe what they do, and which
> options are available for them. It's definitely something we could add
> in the future.
>
> Regarding what you reported, I'm totally ok with the change.
>
> However, since this is a new series, I'm not if I or Alex should
> change it. If it's ok for you to modify this Alex, it could be simpler
> than waiting for me to push a new patch with just this.

Its my tree so I'll fix it up. I'll ask you if I want a respin ;-)

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 9/9] contrib/plugins: add ips plugin example for cost modeling

[Pierrick Bouvier]

On 6/16/24 11:43, Alex Bennée wrote:
> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> 
>> On 6/13/24 01:54, Philippe Mathieu-Daudé wrote:
>>> On 12/6/24 17:35, Alex Bennée wrote:
>>>> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>>
>>>> This plugin uses the new time control interface to make decisions
>>>> about the state of time during the emulation. The algorithm is
>>>> currently very simple. The user specifies an ips rate which applies
>>> ... IPS rate (Instructions Per Second) which ...
>>>
>>>> per core. If the core runs ahead of its allocated execution time the
>>>> plugin sleeps for a bit to let real time catch up. Either way time is
>>>> updated for the emulation as a function of total executed instructions
>>>> with some adjustments for cores that idle.
>>>>
>>>> Examples
>>>> --------
>>>>
>>>> Slow down execution of /bin/true:
>>>> $ num_insn=$(./build/qemu-x86_64 -plugin ./build/tests/plugin/libinsn.so -d plugin /bin/true |& grep total | sed -e 's/.*: //')
>>>> $ time ./build/qemu-x86_64 -plugin ./build/contrib/plugins/libips.so,ips=$(($num_insn/4)) /bin/true
>>>> real 4.000s
>>>>
>>>> Boot a Linux kernel simulating a 250MHz cpu:
>>>> $ /build/qemu-system-x86_64 -kernel /boot/vmlinuz-6.1.0-21-amd64 -append "console=ttyS0" -plugin ./build/contrib/plugins/libips.so,ips=$((250*1000*1000)) -smp 1 -m 512
>>>> check time until kernel panic on serial0
>>>>
>>>> Tested in system mode by booting a full debian system, and using:
>>>> $ sysbench cpu run
>>>> Performance decrease linearly with the given number of ips.
>>>>
>>>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>> Message-Id: <20240530220610.1245424-7-pierrick.bouvier@linaro.org>
>>>> ---
>>>>     contrib/plugins/ips.c    | 164 +++++++++++++++++++++++++++++++++++++++
>>>>     contrib/plugins/Makefile |   1 +
>>>>     2 files changed, 165 insertions(+)
>>>>     create mode 100644 contrib/plugins/ips.c
>>>>
>>>> diff --git a/contrib/plugins/ips.c b/contrib/plugins/ips.c
>>>> new file mode 100644
>>>> index 0000000000..db77729264
>>>> --- /dev/null
>>>> +++ b/contrib/plugins/ips.c
>>>> @@ -0,0 +1,164 @@
>>>> +/*
>>>> + * ips rate limiting plugin.
>>> The plugin names are really to packed to my taste (each time I look
>>> for
>>> one I have to open most source files to figure out the correct one); so
>>> please ease my life by using a more descriptive header at least:
>>>         Instructions Per Second (IPS) rate limiting plugin.
>>> Thanks.
>>>
>>
>> I agree most of the plugin names are pretty cryptic, and they are
>> lacking a common "help" system, to describe what they do, and which
>> options are available for them. It's definitely something we could add
>> in the future.
>>
>> Regarding what you reported, I'm totally ok with the change.
>>
>> However, since this is a new series, I'm not if I or Alex should
>> change it. If it's ok for you to modify this Alex, it could be simpler
>> than waiting for me to push a new patch with just this.
> 
> Its my tree so I'll fix it up. I'll ask you if I want a respin ;-)
> 

Thanks Alex.