* [PATCH 1/8] plugins: force slow path when plugins instrument memory ops
2023-05-19 17:04 [PATCH 0/8] plugins/next: bugfixs and iops based time control RFC Alex Bennée
@ 2023-05-19 17:04 ` Alex Bennée
2023-05-19 17:04 ` [PATCH 2/8] plugins: fix memory leak while parsing options Alex Bennée
` (6 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Alex Bennée @ 2023-05-19 17:04 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Bennée, Yanan Wang, Riku Voipio, Laurent Vivier,
Marcel Apfelbaum, Marco Liebel, Mark Burton, Thomas Huth,
Peter Maydell, Richard Henderson, Eduardo Habkost, Paolo Bonzini,
qemu-arm, Philippe Mathieu-Daudé, Alexandre Iooss,
Mahmoud Mandour, Robert Henry, Aaron Lindsay
The lack of SVE memory instrumentation has been an omission in plugin
handling since it was introduced. Fortunately we can utilise the
probe_* functions to force all all memory access to follow the slow
path. We do this by checking the access type and presence of plugin
memory callbacks and if set return the TLB_MMIO flag.
We have to jump through a few hoops in user mode to re-use the flag
but it was the desired effect:
./qemu-system-aarch64 -display none -serial mon:stdio \
-M virt -cpu max -semihosting-config enable=on \
-kernel ./tests/tcg/aarch64-softmmu/memory-sve \
-plugin ./contrib/plugins/libexeclog.so,ifilter=st1w,afilter=0x40001808 -d plugin
gives (disas doesn't currently understand st1w):
0, 0x40001808, 0xe54342a0, ".byte 0xa0, 0x42, 0x43, 0xe5", store, 0x40213010, RAM, store, 0x40213014, RAM, store, 0x40213018, RAM
And for user-mode:
./qemu-aarch64 \
-plugin contrib/plugins/libexeclog.so,afilter=0x4007c0 \
-d plugin \
./tests/tcg/aarch64-linux-user/sha512-sve
gives:
1..10
ok 1 - do_test(&tests[i])
0, 0x4007c0, 0xa4004b80, ".byte 0x80, 0x4b, 0x00, 0xa4", load, 0x5500800370, load, 0x5500800371, load, 0x5500800372, load, 0x5500800373, load, 0x5500800374, load, 0x5500800375, load, 0x5500800376, load, 0x5500800377, load, 0x5500800378, load, 0x5500800379, load, 0x550080037a, load, 0x550080037b, load, 0x550080037c, load, 0x550080037d, load, 0x550080037e, load, 0x550080037f, load, 0x5500800380, load, 0x5500800381, load, 0x5500800382, load, 0x5500800383, load, 0x5500800384, load, 0x5500800385, load, 0x5500800386, lo
ad, 0x5500800387, load, 0x5500800388, load, 0x5500800389, load, 0x550080038a, load, 0x550080038b, load, 0x550080038c, load, 0x550080038d, load, 0x550080038e, load, 0x550080038f, load, 0x5500800390, load, 0x5500800391, load, 0x5500800392, load, 0x5500800393, load, 0x5500800394, load, 0x5500800395, load, 0x5500800396, load, 0x5500800397, load, 0x5500800398, load, 0x5500800399, load, 0x550080039a, load, 0x550080039b, load, 0x550080039c, load, 0x550080039d, load, 0x550080039e, load, 0x550080039f, load, 0x55008003a0, load, 0x55008003a1, load, 0x55008003a2, load, 0x55008003a3, load, 0x55008003a4, load, 0x55008003a5, load, 0x55008003a6, load, 0x55008003a7, load, 0x55008003a8, load, 0x55008003a9, load, 0x55008003aa, load, 0x55008003ab, load, 0x55008003ac, load, 0x55008003ad, load, 0x55008003ae, load, 0x55008003af
(4007c0 is the ld1b in the sha512-sve)
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Cc: Robert Henry <robhenry@microsoft.com>
Cc: Aaron Lindsay <aaron@os.amperecomputing.com>
---
include/exec/cpu-all.h | 2 +-
include/hw/core/cpu.h | 17 +++++++++++++++++
accel/tcg/cputlb.c | 4 +++-
accel/tcg/user-exec.c | 6 +++++-
target/arm/tcg/sve_helper.c | 4 ----
tests/tcg/aarch64/Makefile.target | 8 ++++++++
6 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 78d258af44..9ab09cf7c2 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -301,7 +301,7 @@ CPUArchState *cpu_copy(CPUArchState *env);
* be signaled by probe_access_flags().
*/
#define TLB_INVALID_MASK (1 << (TARGET_PAGE_BITS_MIN - 1))
-#define TLB_MMIO 0
+#define TLB_MMIO (1 << (TARGET_PAGE_BITS_MIN - 2))
#define TLB_WATCHPOINT 0
#else
diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
index 39150cf8f8..26fadf7e62 100644
--- a/include/hw/core/cpu.h
+++ b/include/hw/core/cpu.h
@@ -983,6 +983,23 @@ void cpu_watchpoint_remove_by_ref(CPUState *cpu, CPUWatchpoint *watchpoint);
void cpu_watchpoint_remove_all(CPUState *cpu, int mask);
#endif
+/**
+ * cpu_plugin_mem_cbs_enabled() - are plugin memory callbacks enabled?
+ * @cs: CPUState pointer
+ *
+ * The memory callbacks are installed if a plugin has instrumented an
+ * instruction for memory. This can be useful to know if you want to
+ * force a slow path for a series of memory accesses.
+ */
+static inline bool cpu_plugin_mem_cbs_enabled(const CPUState *cpu)
+{
+#ifdef CONFIG_PLUGIN
+ return !!cpu->plugin_mem_cbs;
+#else
+ return false;
+#endif
+}
+
/**
* cpu_get_address_space:
* @cpu: CPU to get address space from
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index ae0fbcdee2..f117fd8f16 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1540,7 +1540,9 @@ static int probe_access_internal(CPUArchState *env, target_ulong addr,
*pfull = &env_tlb(env)->d[mmu_idx].fulltlb[index];
/* Fold all "mmio-like" bits into TLB_MMIO. This is not RAM. */
- if (unlikely(flags & ~(TLB_WATCHPOINT | TLB_NOTDIRTY))) {
+ if (unlikely(flags & ~(TLB_WATCHPOINT | TLB_NOTDIRTY))
+ ||
+ (access_type != MMU_INST_FETCH && cpu_plugin_mem_cbs_enabled(env_cpu(env)))) {
*phost = NULL;
return TLB_MMIO;
}
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 36ad8284a5..383263cd1d 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -745,6 +745,10 @@ static int probe_access_internal(CPUArchState *env, target_ulong addr,
if (guest_addr_valid_untagged(addr)) {
int page_flags = page_get_flags(addr);
if (page_flags & acc_flag) {
+ if ((acc_flag == PAGE_READ || acc_flag == PAGE_WRITE)
+ && cpu_plugin_mem_cbs_enabled(env_cpu(env))) {
+ return TLB_MMIO;
+ }
return 0; /* success */
}
maperr = !(page_flags & PAGE_VALID);
@@ -767,7 +771,7 @@ int probe_access_flags(CPUArchState *env, target_ulong addr, int size,
g_assert(-(addr | TARGET_PAGE_MASK) >= size);
flags = probe_access_internal(env, addr, size, access_type, nonfault, ra);
- *phost = flags ? NULL : g2h(env_cpu(env), addr);
+ *phost = (flags & TLB_INVALID_MASK) ? NULL : g2h(env_cpu(env), addr);
return flags;
}
diff --git a/target/arm/tcg/sve_helper.c b/target/arm/tcg/sve_helper.c
index 0097522470..7c103fc9f7 100644
--- a/target/arm/tcg/sve_helper.c
+++ b/target/arm/tcg/sve_helper.c
@@ -5688,9 +5688,6 @@ void sve_ldN_r(CPUARMState *env, uint64_t *vg, const target_ulong addr,
flags = info.page[0].flags | info.page[1].flags;
if (unlikely(flags != 0)) {
-#ifdef CONFIG_USER_ONLY
- g_assert_not_reached();
-#else
/*
* At least one page includes MMIO.
* Any bus operation can fail with cpu_transaction_failed,
@@ -5727,7 +5724,6 @@ void sve_ldN_r(CPUARMState *env, uint64_t *vg, const target_ulong addr,
memcpy(&env->vfp.zregs[(rd + i) & 31], &scratch[i], reg_max);
}
return;
-#endif
}
/* The entire operation is in RAM, on valid pages. */
diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target
index 0315795487..f83edd8544 100644
--- a/tests/tcg/aarch64/Makefile.target
+++ b/tests/tcg/aarch64/Makefile.target
@@ -80,6 +80,14 @@ sha512-vector: sha512.c
TESTS += sha512-vector
+ifneq ($(CROSS_CC_HAS_SVE),)
+sha512-sve: CFLAGS=-O3 -march=armv8.1-a+sve
+sha512-sve: sha512.c
+ $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
+
+TESTS += sha512-sve
+endif
+
ifeq ($(HOST_GDB_SUPPORTS_ARCH),y)
GDB_SCRIPT=$(SRC_PATH)/tests/guest-debug/run-test.py
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 2/8] plugins: fix memory leak while parsing options
2023-05-19 17:04 [PATCH 0/8] plugins/next: bugfixs and iops based time control RFC Alex Bennée
2023-05-19 17:04 ` [PATCH 1/8] plugins: force slow path when plugins instrument memory ops Alex Bennée
@ 2023-05-19 17:04 ` Alex Bennée
2023-05-20 4:21 ` Philippe Mathieu-Daudé
2023-05-19 17:04 ` [PATCH 3/8] plugins: update lockstep to use g_memdup2 Alex Bennée
` (5 subsequent siblings)
7 siblings, 1 reply; 13+ messages in thread
From: Alex Bennée @ 2023-05-19 17:04 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Bennée, Yanan Wang, Riku Voipio, Laurent Vivier,
Marcel Apfelbaum, Marco Liebel, Mark Burton, Thomas Huth,
Peter Maydell, Richard Henderson, Eduardo Habkost, Paolo Bonzini,
qemu-arm, Philippe Mathieu-Daudé, Alexandre Iooss,
Mahmoud Mandour
It was hard to track down this leak as it was an internal allocation
by glib and the backtraces did not give much away. The autofree was
freeing the allocation with g_free() but not taking care of the
individual strings. They should have been freed with g_strfreev()
instead.
Searching the glib source code for the correct string free function
led to:
G_DEFINE_AUTO_CLEANUP_FREE_FUNC(GStrv, g_strfreev, NULL)
and indeed if you read to the bottom of the documentation page you
will find:
typedef gchar** GStrv;
A typedef alias for gchar**. This is mostly useful when used together with g_auto().
So fix up all the g_autofree g_strsplit case that smugly thought they
had de-allocation covered.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
contrib/plugins/cache.c | 2 +-
contrib/plugins/drcov.c | 2 +-
contrib/plugins/execlog.c | 2 +-
contrib/plugins/hotblocks.c | 2 +-
contrib/plugins/hotpages.c | 2 +-
contrib/plugins/howvec.c | 2 +-
contrib/plugins/hwprofile.c | 2 +-
contrib/plugins/lockstep.c | 2 +-
tests/plugin/bb.c | 2 +-
tests/plugin/insn.c | 2 +-
tests/plugin/mem.c | 2 +-
tests/plugin/syscall.c | 2 +-
12 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/contrib/plugins/cache.c b/contrib/plugins/cache.c
index 2e25184a7f..5036213f1b 100644
--- a/contrib/plugins/cache.c
+++ b/contrib/plugins/cache.c
@@ -772,7 +772,7 @@ int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info,
for (i = 0; i < argc; i++) {
char *opt = argv[i];
- g_autofree char **tokens = g_strsplit(opt, "=", 2);
+ g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
if (g_strcmp0(tokens[0], "iblksize") == 0) {
l1_iblksize = STRTOLL(tokens[1]);
diff --git a/contrib/plugins/drcov.c b/contrib/plugins/drcov.c
index b4a855adaf..686ae0a537 100644
--- a/contrib/plugins/drcov.c
+++ b/contrib/plugins/drcov.c
@@ -148,7 +148,7 @@ 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++) {
- g_autofree char **tokens = g_strsplit(argv[i], "=", 2);
+ g_auto(GStrv) tokens = g_strsplit(argv[i], "=", 2);
if (g_strcmp0(tokens[0], "filename") == 0) {
file_name = g_strdup(tokens[1]);
}
diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
index e255bd21fd..7129d526f8 100644
--- a/contrib/plugins/execlog.c
+++ b/contrib/plugins/execlog.c
@@ -227,7 +227,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
for (int i = 0; i < argc; i++) {
char *opt = argv[i];
- g_autofree char **tokens = g_strsplit(opt, "=", 2);
+ g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
if (g_strcmp0(tokens[0], "ifilter") == 0) {
parse_insn_match(tokens[1]);
} else if (g_strcmp0(tokens[0], "afilter") == 0) {
diff --git a/contrib/plugins/hotblocks.c b/contrib/plugins/hotblocks.c
index 062200a7a4..6b74d25fea 100644
--- a/contrib/plugins/hotblocks.c
+++ b/contrib/plugins/hotblocks.c
@@ -135,7 +135,7 @@ int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info,
{
for (int i = 0; i < argc; i++) {
char *opt = argv[i];
- g_autofree char **tokens = g_strsplit(opt, "=", 2);
+ g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
if (g_strcmp0(tokens[0], "inline") == 0) {
if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_inline)) {
fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
diff --git a/contrib/plugins/hotpages.c b/contrib/plugins/hotpages.c
index 0d12910af6..8316ae50c7 100644
--- a/contrib/plugins/hotpages.c
+++ b/contrib/plugins/hotpages.c
@@ -169,7 +169,7 @@ int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info,
for (i = 0; i < argc; i++) {
char *opt = argv[i];
- g_autofree char **tokens = g_strsplit(opt, "=", -1);
+ g_auto(GStrv) tokens = g_strsplit(opt, "=", -1);
if (g_strcmp0(tokens[0], "sortby") == 0) {
if (g_strcmp0(tokens[1], "reads") == 0) {
diff --git a/contrib/plugins/howvec.c b/contrib/plugins/howvec.c
index 4a5ec3d936..0ed01ea931 100644
--- a/contrib/plugins/howvec.c
+++ b/contrib/plugins/howvec.c
@@ -333,7 +333,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
for (i = 0; i < argc; i++) {
char *p = argv[i];
- g_autofree char **tokens = g_strsplit(p, "=", -1);
+ g_auto(GStrv) tokens = g_strsplit(p, "=", -1);
if (g_strcmp0(tokens[0], "inline") == 0) {
if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_inline)) {
fprintf(stderr, "boolean argument parsing failed: %s\n", p);
diff --git a/contrib/plugins/hwprofile.c b/contrib/plugins/hwprofile.c
index 691d4edb0c..739ac0c66b 100644
--- a/contrib/plugins/hwprofile.c
+++ b/contrib/plugins/hwprofile.c
@@ -263,7 +263,7 @@ int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info,
for (i = 0; i < argc; i++) {
char *opt = argv[i];
- g_autofree char **tokens = g_strsplit(opt, "=", 2);
+ g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
if (g_strcmp0(tokens[0], "track") == 0) {
if (g_strcmp0(tokens[1], "read") == 0) {
diff --git a/contrib/plugins/lockstep.c b/contrib/plugins/lockstep.c
index a41ffe83fa..e36f0b9562 100644
--- a/contrib/plugins/lockstep.c
+++ b/contrib/plugins/lockstep.c
@@ -323,7 +323,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
for (i = 0; i < argc; i++) {
char *p = argv[i];
- g_autofree char **tokens = g_strsplit(p, "=", 2);
+ g_auto(GStrv) tokens = g_strsplit(p, "=", 2);
if (g_strcmp0(tokens[0], "verbose") == 0) {
if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &verbose)) {
diff --git a/tests/plugin/bb.c b/tests/plugin/bb.c
index 7d470a1011..df50d1fd3b 100644
--- a/tests/plugin/bb.c
+++ b/tests/plugin/bb.c
@@ -104,7 +104,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
for (i = 0; i < argc; i++) {
char *opt = argv[i];
- g_autofree char **tokens = g_strsplit(opt, "=", 2);
+ g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
if (g_strcmp0(tokens[0], "inline") == 0) {
if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_inline)) {
fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
diff --git a/tests/plugin/insn.c b/tests/plugin/insn.c
index cd5ea5d4ae..e251a84d86 100644
--- a/tests/plugin/insn.c
+++ b/tests/plugin/insn.c
@@ -196,7 +196,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
{
for (int i = 0; i < argc; i++) {
char *opt = argv[i];
- g_autofree char **tokens = g_strsplit(opt, "=", 2);
+ g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
if (g_strcmp0(tokens[0], "inline") == 0) {
if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_inline)) {
fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
diff --git a/tests/plugin/mem.c b/tests/plugin/mem.c
index 4570f7d815..f3b9f696a0 100644
--- a/tests/plugin/mem.c
+++ b/tests/plugin/mem.c
@@ -83,7 +83,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
for (int i = 0; i < argc; i++) {
char *opt = argv[i];
- g_autofree char **tokens = g_strsplit(opt, "=", 2);
+ g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
if (g_strcmp0(tokens[0], "haddr") == 0) {
if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_haddr)) {
diff --git a/tests/plugin/syscall.c b/tests/plugin/syscall.c
index 96040c578f..72e1a5bf90 100644
--- a/tests/plugin/syscall.c
+++ b/tests/plugin/syscall.c
@@ -121,7 +121,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
for (int i = 0; i < argc; i++) {
char *opt = argv[i];
- g_autofree char **tokens = g_strsplit(opt, "=", 2);
+ g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
if (g_strcmp0(tokens[0], "print") == 0) {
if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &do_print)) {
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 2/8] plugins: fix memory leak while parsing options
2023-05-19 17:04 ` [PATCH 2/8] plugins: fix memory leak while parsing options Alex Bennée
@ 2023-05-20 4:21 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-05-20 4:21 UTC (permalink / raw)
To: Alex Bennée, qemu-devel, Daniel P. Berrangé,
Marc-André Lureau
Cc: Yanan Wang, Riku Voipio, Laurent Vivier, Marcel Apfelbaum,
Marco Liebel, Mark Burton, Thomas Huth, Peter Maydell,
Richard Henderson, Eduardo Habkost, Paolo Bonzini, qemu-arm,
Alexandre Iooss, Mahmoud Mandour
On 19/5/23 19:04, Alex Bennée wrote:
> It was hard to track down this leak as it was an internal allocation
> by glib and the backtraces did not give much away. The autofree was
> freeing the allocation with g_free() but not taking care of the
> individual strings. They should have been freed with g_strfreev()
> instead.
>
> Searching the glib source code for the correct string free function
> led to:
>
> G_DEFINE_AUTO_CLEANUP_FREE_FUNC(GStrv, g_strfreev, NULL)
>
> and indeed if you read to the bottom of the documentation page you
> will find:
>
> typedef gchar** GStrv;
>
> A typedef alias for gchar**. This is mostly useful when used together with g_auto().
So possibly glib could improve by declaring g_strsplit()
(and co) returning a GStrv instead of a gchar** type?
> So fix up all the g_autofree g_strsplit case that smugly thought they
> had de-allocation covered.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> contrib/plugins/cache.c | 2 +-
> contrib/plugins/drcov.c | 2 +-
> contrib/plugins/execlog.c | 2 +-
> contrib/plugins/hotblocks.c | 2 +-
> contrib/plugins/hotpages.c | 2 +-
> contrib/plugins/howvec.c | 2 +-
> contrib/plugins/hwprofile.c | 2 +-
> contrib/plugins/lockstep.c | 2 +-
> tests/plugin/bb.c | 2 +-
> tests/plugin/insn.c | 2 +-
> tests/plugin/mem.c | 2 +-
> tests/plugin/syscall.c | 2 +-
> 12 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/contrib/plugins/cache.c b/contrib/plugins/cache.c
> index 2e25184a7f..5036213f1b 100644
> --- a/contrib/plugins/cache.c
> +++ b/contrib/plugins/cache.c
> @@ -772,7 +772,7 @@ int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info,
>
> for (i = 0; i < argc; i++) {
> char *opt = argv[i];
> - g_autofree char **tokens = g_strsplit(opt, "=", 2);
> + g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/8] plugins: update lockstep to use g_memdup2
2023-05-19 17:04 [PATCH 0/8] plugins/next: bugfixs and iops based time control RFC Alex Bennée
2023-05-19 17:04 ` [PATCH 1/8] plugins: force slow path when plugins instrument memory ops Alex Bennée
2023-05-19 17:04 ` [PATCH 2/8] plugins: fix memory leak while parsing options Alex Bennée
@ 2023-05-19 17:04 ` Alex Bennée
2023-05-19 17:04 ` [RFC PATCH 4/8] sysemu: add set_virtual_time to accel ops Alex Bennée
` (4 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Alex Bennée @ 2023-05-19 17:04 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Bennée, Yanan Wang, Riku Voipio, Laurent Vivier,
Marcel Apfelbaum, Marco Liebel, Mark Burton, Thomas Huth,
Peter Maydell, Richard Henderson, Eduardo Habkost, Paolo Bonzini,
qemu-arm, Philippe Mathieu-Daudé, Alexandre Iooss,
Mahmoud Mandour
The old g_memdup is deprecated, use the replacement.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
contrib/plugins/lockstep.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/plugins/lockstep.c b/contrib/plugins/lockstep.c
index e36f0b9562..3614c3564c 100644
--- a/contrib/plugins/lockstep.c
+++ b/contrib/plugins/lockstep.c
@@ -130,7 +130,7 @@ static void report_divergance(ExecState *us, ExecState *them)
}
}
divergence_log = g_slist_prepend(divergence_log,
- g_memdup(&divrec, sizeof(divrec)));
+ g_memdup2(&divrec, sizeof(divrec)));
/* Output short log entry of going out of sync... */
if (verbose || divrec.distance == 1 || diverged) {
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* [RFC PATCH 4/8] sysemu: add set_virtual_time to accel ops
2023-05-19 17:04 [PATCH 0/8] plugins/next: bugfixs and iops based time control RFC Alex Bennée
` (2 preceding siblings ...)
2023-05-19 17:04 ` [PATCH 3/8] plugins: update lockstep to use g_memdup2 Alex Bennée
@ 2023-05-19 17:04 ` Alex Bennée
2023-05-20 4:23 ` Philippe Mathieu-Daudé
2023-05-19 17:04 ` [RFC PATCH 5/8] qtest: use cpu interface in qtest_clock_warp Alex Bennée
` (3 subsequent siblings)
7 siblings, 1 reply; 13+ messages in thread
From: Alex Bennée @ 2023-05-19 17:04 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Bennée, Yanan Wang, Riku Voipio, Laurent Vivier,
Marcel Apfelbaum, Marco Liebel, Mark Burton, Thomas Huth,
Peter Maydell, Richard Henderson, Eduardo Habkost, Paolo Bonzini,
qemu-arm, Philippe Mathieu-Daudé, Alexandre Iooss,
Mahmoud Mandour
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.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
include/sysemu/accel-ops.h | 18 +++++++++++++++++-
include/sysemu/cpu-timers.h | 3 ++-
softmmu/cpus.c | 11 +++++++++++
...et-virtual-clock.c => cpus-virtual-clock.c} | 5 +++++
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 3c1fab4b1e..224e85a649 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;
@@ -43,7 +48,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 2e786fe7fb..c3f4c262f8 100644
--- a/include/sysemu/cpu-timers.h
+++ b/include/sysemu/cpu-timers.h
@@ -84,8 +84,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 the 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/softmmu/cpus.c b/softmmu/cpus.c
index 9cbc8172b5..1e4f09553a 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -221,6 +221,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/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/stubs/meson.build b/stubs/meson.build
index 8412cad15f..c32c66a5c7 100644
--- a/stubs/meson.build
+++ b/stubs/meson.build
@@ -5,7 +5,7 @@ stub_ss.add(files('blockdev-close-all-bdrv-states.c'))
stub_ss.add(files('change-state-handler.c'))
stub_ss.add(files('cmos.c'))
stub_ss.add(files('cpu-get-clock.c'))
-stub_ss.add(files('cpus-get-virtual-clock.c'))
+stub_ss.add(files('cpus-virtual-clock.c'))
stub_ss.add(files('qemu-timer-notify-cb.c'))
stub_ss.add(files('icount.c'))
stub_ss.add(files('dump.c'))
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [RFC PATCH 4/8] sysemu: add set_virtual_time to accel ops
2023-05-19 17:04 ` [RFC PATCH 4/8] sysemu: add set_virtual_time to accel ops Alex Bennée
@ 2023-05-20 4:23 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-05-20 4:23 UTC (permalink / raw)
To: Alex Bennée, qemu-devel
Cc: Yanan Wang, Riku Voipio, Laurent Vivier, Marcel Apfelbaum,
Marco Liebel, Mark Burton, Thomas Huth, Peter Maydell,
Richard Henderson, Eduardo Habkost, Paolo Bonzini, qemu-arm,
Alexandre Iooss, Mahmoud Mandour
On 19/5/23 19:04, 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.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> include/sysemu/accel-ops.h | 18 +++++++++++++++++-
> include/sysemu/cpu-timers.h | 3 ++-
> softmmu/cpus.c | 11 +++++++++++
> ...et-virtual-clock.c => cpus-virtual-clock.c} | 5 +++++
> 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: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [RFC PATCH 5/8] qtest: use cpu interface in qtest_clock_warp
2023-05-19 17:04 [PATCH 0/8] plugins/next: bugfixs and iops based time control RFC Alex Bennée
` (3 preceding siblings ...)
2023-05-19 17:04 ` [RFC PATCH 4/8] sysemu: add set_virtual_time to accel ops Alex Bennée
@ 2023-05-19 17:04 ` Alex Bennée
2023-05-22 7:31 ` Thomas Huth
2023-05-19 17:04 ` [RFC PATCH 6/8] sysemu: generalise qtest_warp_clock as qemu_clock_advance_virtual_time Alex Bennée
` (2 subsequent siblings)
7 siblings, 1 reply; 13+ messages in thread
From: Alex Bennée @ 2023-05-19 17:04 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Bennée, Yanan Wang, Riku Voipio, Laurent Vivier,
Marcel Apfelbaum, Marco Liebel, Mark Burton, Thomas Huth,
Peter Maydell, Richard Henderson, Eduardo Habkost, Paolo Bonzini,
qemu-arm, Philippe Mathieu-Daudé, Alexandre Iooss,
Mahmoud Mandour
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.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
include/sysemu/qtest.h | 1 +
accel/qtest/qtest.c | 1 +
softmmu/qtest.c | 6 +++---
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/include/sysemu/qtest.h b/include/sysemu/qtest.h
index 85f05b0e46..e1f69783d6 100644
--- a/include/sysemu/qtest.h
+++ b/include/sysemu/qtest.h
@@ -35,5 +35,6 @@ 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
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/softmmu/qtest.c b/softmmu/qtest.c
index f8d764b719..34bc9e1f49 100644
--- a/softmmu/qtest.c
+++ b/softmmu/qtest.c
@@ -331,14 +331,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();
@@ -347,7 +347,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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [RFC PATCH 5/8] qtest: use cpu interface in qtest_clock_warp
2023-05-19 17:04 ` [RFC PATCH 5/8] qtest: use cpu interface in qtest_clock_warp Alex Bennée
@ 2023-05-22 7:31 ` Thomas Huth
0 siblings, 0 replies; 13+ messages in thread
From: Thomas Huth @ 2023-05-22 7:31 UTC (permalink / raw)
To: Alex Bennée, qemu-devel
Cc: Yanan Wang, Riku Voipio, Laurent Vivier, Marcel Apfelbaum,
Marco Liebel, Mark Burton, Peter Maydell, Richard Henderson,
Eduardo Habkost, Paolo Bonzini, qemu-arm,
Philippe Mathieu-Daudé, Alexandre Iooss, Mahmoud Mandour
On 19/05/2023 19.04, Alex Bennée wrote:
> 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.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> include/sysemu/qtest.h | 1 +
> accel/qtest/qtest.c | 1 +
> softmmu/qtest.c | 6 +++---
> 3 files changed, 5 insertions(+), 3 deletions(-)
Acked-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [RFC PATCH 6/8] sysemu: generalise qtest_warp_clock as qemu_clock_advance_virtual_time
2023-05-19 17:04 [PATCH 0/8] plugins/next: bugfixs and iops based time control RFC Alex Bennée
` (4 preceding siblings ...)
2023-05-19 17:04 ` [RFC PATCH 5/8] qtest: use cpu interface in qtest_clock_warp Alex Bennée
@ 2023-05-19 17:04 ` Alex Bennée
2023-05-20 4:27 ` Philippe Mathieu-Daudé
2023-05-19 17:04 ` [RFC PATCH 7/8] plugins: add time control API Alex Bennée
2023-05-19 17:04 ` [RFC PATCH 8/8] contrib/plugins: add iops plugin example for cost modelling Alex Bennée
7 siblings, 1 reply; 13+ messages in thread
From: Alex Bennée @ 2023-05-19 17:04 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Bennée, Yanan Wang, Riku Voipio, Laurent Vivier,
Marcel Apfelbaum, Marco Liebel, Mark Burton, Thomas Huth,
Peter Maydell, Richard Henderson, Eduardo Habkost, Paolo Bonzini,
qemu-arm, Philippe Mathieu-Daudé, Alexandre Iooss,
Mahmoud Mandour
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.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
include/qemu/timer.h | 15 +++++++++++++++
softmmu/qtest.c | 24 ++----------------------
util/qemu-timer.c | 26 ++++++++++++++++++++++++++
3 files changed, 43 insertions(+), 22 deletions(-)
diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index ee071e07d1..9a1a42a400 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/softmmu/qtest.c b/softmmu/qtest.c
index 34bc9e1f49..3cd6c11f5a 100644
--- a/softmmu/qtest.c
+++ b/softmmu/qtest.c
@@ -336,26 +336,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))
@@ -732,7 +712,7 @@ 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));
@@ -758,7 +738,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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [RFC PATCH 6/8] sysemu: generalise qtest_warp_clock as qemu_clock_advance_virtual_time
2023-05-19 17:04 ` [RFC PATCH 6/8] sysemu: generalise qtest_warp_clock as qemu_clock_advance_virtual_time Alex Bennée
@ 2023-05-20 4:27 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-05-20 4:27 UTC (permalink / raw)
To: Alex Bennée, qemu-devel
Cc: Yanan Wang, Riku Voipio, Laurent Vivier, Marcel Apfelbaum,
Marco Liebel, Mark Burton, Thomas Huth, Peter Maydell,
Richard Henderson, Eduardo Habkost, Paolo Bonzini, qemu-arm,
Alexandre Iooss, Mahmoud Mandour
On 19/5/23 19:04, Alex Bennée wrote:
> 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.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> include/qemu/timer.h | 15 +++++++++++++++
> softmmu/qtest.c | 24 ++----------------------
> util/qemu-timer.c | 26 ++++++++++++++++++++++++++
> 3 files changed, 43 insertions(+), 22 deletions(-)
>
> diff --git a/include/qemu/timer.h b/include/qemu/timer.h
> index ee071e07d1..9a1a42a400 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
Maybe '@target_ns'?
> + *
> + * 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);
s/dest/target[_ns]/
^ permalink raw reply [flat|nested] 13+ messages in thread
* [RFC PATCH 7/8] plugins: add time control API
2023-05-19 17:04 [PATCH 0/8] plugins/next: bugfixs and iops based time control RFC Alex Bennée
` (5 preceding siblings ...)
2023-05-19 17:04 ` [RFC PATCH 6/8] sysemu: generalise qtest_warp_clock as qemu_clock_advance_virtual_time Alex Bennée
@ 2023-05-19 17:04 ` Alex Bennée
2023-05-19 17:04 ` [RFC PATCH 8/8] contrib/plugins: add iops plugin example for cost modelling Alex Bennée
7 siblings, 0 replies; 13+ messages in thread
From: Alex Bennée @ 2023-05-19 17:04 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Bennée, Yanan Wang, Riku Voipio, Laurent Vivier,
Marcel Apfelbaum, Marco Liebel, Mark Burton, Thomas Huth,
Peter Maydell, Richard Henderson, Eduardo Habkost, Paolo Bonzini,
qemu-arm, Philippe Mathieu-Daudé, Alexandre Iooss,
Mahmoud Mandour
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.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
include/qemu/qemu-plugin.h | 19 +++++++++++++++++++
plugins/api.c | 22 ++++++++++++++++++++++
plugins/qemu-plugins.symbols | 2 ++
3 files changed, 43 insertions(+)
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 50a9957279..8385670976 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -536,7 +536,26 @@ void qemu_plugin_register_vcpu_mem_inline(struct qemu_plugin_insn *insn,
enum qemu_plugin_op op, void *ptr,
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.
+ */
+void qemu_plugin_update_ns(const void *handle, int64_t new_time);
typedef void
(*qemu_plugin_vcpu_syscall_cb_t)(qemu_plugin_id_t id, unsigned int vcpu_index,
diff --git a/plugins/api.c b/plugins/api.c
index 2078b16edb..8402b3a5f6 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -37,6 +37,7 @@
#include "qemu/osdep.h"
#include "qemu/plugin.h"
#include "qemu/log.h"
+#include "qemu/timer.h"
#include "tcg/tcg.h"
#include "exec/exec-all.h"
#include "exec/ram_addr.h"
@@ -442,3 +443,24 @@ uint64_t qemu_plugin_entry_code(void)
#endif
return entry;
}
+
+/*
+ * 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;
+}
+
+void qemu_plugin_update_ns(const void *handle, int64_t new_time)
+{
+ if (handle == &has_control) {
+ qemu_clock_advance_virtual_time(new_time);
+ }
+}
diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols
index 71f6c90549..91b882fecc 100644
--- a/plugins/qemu-plugins.symbols
+++ b/plugins/qemu-plugins.symbols
@@ -35,11 +35,13 @@
qemu_plugin_register_vcpu_tb_exec_cb;
qemu_plugin_register_vcpu_tb_exec_inline;
qemu_plugin_register_vcpu_tb_trans_cb;
+ qemu_plugin_request_time_control;
qemu_plugin_reset;
qemu_plugin_start_code;
qemu_plugin_tb_get_insn;
qemu_plugin_tb_n_insns;
qemu_plugin_tb_vaddr;
qemu_plugin_uninstall;
+ qemu_plugin_update_ns;
qemu_plugin_vcpu_for_each;
};
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread* [RFC PATCH 8/8] contrib/plugins: add iops plugin example for cost modelling
2023-05-19 17:04 [PATCH 0/8] plugins/next: bugfixs and iops based time control RFC Alex Bennée
` (6 preceding siblings ...)
2023-05-19 17:04 ` [RFC PATCH 7/8] plugins: add time control API Alex Bennée
@ 2023-05-19 17:04 ` Alex Bennée
7 siblings, 0 replies; 13+ messages in thread
From: Alex Bennée @ 2023-05-19 17:04 UTC (permalink / raw)
To: qemu-devel
Cc: Alex Bennée, Yanan Wang, Riku Voipio, Laurent Vivier,
Marcel Apfelbaum, Marco Liebel, Mark Burton, Thomas Huth,
Peter Maydell, Richard Henderson, Eduardo Habkost, Paolo Bonzini,
qemu-arm, Philippe Mathieu-Daudé, Alexandre Iooss,
Mahmoud Mandour
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 iops 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 as
updated for the emulation as a function of total executed instructions
with some adjustments for cores that idle.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
contrib/plugins/iops.c | 260 +++++++++++++++++++++++++++++++++++++++
contrib/plugins/Makefile | 1 +
2 files changed, 261 insertions(+)
create mode 100644 contrib/plugins/iops.c
diff --git a/contrib/plugins/iops.c b/contrib/plugins/iops.c
new file mode 100644
index 0000000000..6eb8f97820
--- /dev/null
+++ b/contrib/plugins/iops.c
@@ -0,0 +1,260 @@
+/*
+ * iops rate limiting plugin.
+ *
+ * This plugin can be used to restrict the execution of a system to a
+ * particular number of Instructions Per Second (IOPS). 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;
+
+#define SLICES 10 /* the number of slices per second we compute delay */
+
+static GMutex global_state_lock;
+
+static uint64_t iops = 1000000; /* iops rate, per core, per second */
+static uint64_t current_ticks; /* current global ticks */
+static uint64_t next_check; /* the next checkpoint for time */
+static bool precise_execution; /* count every instruction */
+
+static int64_t systime_at_start; /* time we started the first vCPU */
+
+static const uint64_t nsec_per_sec = 1000000000;
+static const void * time_handle;
+
+/*
+ * We need to track the number of instructions each vCPU has executed
+ * as well as what its current state is. We need to account for time
+ * passing while a vCPU is idle.
+ */
+
+typedef enum {
+ UNKNOWN = 0,
+ CREATED,
+ EXECUTING,
+ IDLE,
+ FINISHED
+} vCPUState;
+
+typedef struct {
+ /* pointer to vcpu counter entry */
+ uint64_t *counter;
+ vCPUState state;
+ /* timestamp when vCPU entered state */
+ uint64_t state_time;
+ /* number of ns vCPU was idle */
+ uint64_t total_idle;
+} vCPUTime;
+
+GArray *vcpus;
+uint64_t *vcpu_counters;
+
+/*
+ * Get the vcpu structure for this vCPU. We don't do any locking here
+ * as only one vCPU will ever access its own structure.
+ */
+static vCPUTime *get_vcpu(int cpu_index)
+{
+ return &g_array_index(vcpus, vCPUTime, cpu_index);
+}
+
+/*
+ * When emulation is running faster than real time this is the point
+ * we can throttle the execution of a given vCPU. Either way we can
+ * now tell the system to move time forward.
+ */
+static void update_system_time(int64_t vcpu_ticks)
+{
+ int64_t now = g_get_real_time();
+ int64_t real_runtime_ns = now - systime_at_start;
+
+ g_mutex_lock(&global_state_lock);
+ /* now we have the lock double check we are fastest */
+ if (vcpu_ticks > next_check) {
+
+ int64_t tick_runtime_ns = (vcpu_ticks / iops) * nsec_per_sec;
+ if (tick_runtime_ns > real_runtime_ns) {
+ int64_t sleep_us = (tick_runtime_ns - real_runtime_ns) / 1000;
+ g_usleep(sleep_us);
+ }
+
+ /* Having slept we can now move the clocks forward */
+ qemu_plugin_update_ns(time_handle, vcpu_ticks);
+ current_ticks = vcpu_ticks;
+ next_check = iops/SLICES;
+ }
+ g_mutex_unlock(&global_state_lock);
+}
+
+/*
+ * State tracking
+ */
+static void vcpu_init(qemu_plugin_id_t id, unsigned int cpu_index)
+{
+ vCPUTime *vcpu = get_vcpu(cpu_index);
+ vcpu->state = CREATED;
+ vcpu->state_time = *vcpu->counter;
+
+ g_mutex_lock(&global_state_lock);
+ if (!systime_at_start) {
+ systime_at_start = g_get_real_time();
+ }
+ g_mutex_unlock(&global_state_lock);
+}
+
+static void vcpu_idle(qemu_plugin_id_t id, unsigned int cpu_index)
+{
+ vCPUTime *vcpu = get_vcpu(cpu_index);
+ vcpu->state = IDLE;
+ vcpu->state_time = *vcpu->counter;
+
+ /* handle when we are the last vcpu to sleep here */
+}
+
+static void vcpu_resume(qemu_plugin_id_t id, unsigned int cpu_index)
+{
+ vCPUTime *vcpu = get_vcpu(cpu_index);
+
+ /*
+ * Now we need to reset counter to something approximating the
+ * current time, however we only update current_ticks when a block
+ * exceeds next_check. If the vCPU has been asleep for awhile this
+ * will probably do, otherwise lets pick somewhere between
+ * current_ticks and the next_check value.
+ */
+ if (vcpu->state_time < current_ticks) {
+ *vcpu->counter = current_ticks;
+ } else {
+ int64_t window = next_check - vcpu->state_time;
+ *vcpu->counter = next_check - (window / 2);
+ }
+
+ vcpu->state = EXECUTING;
+ vcpu->state_time = *vcpu->counter;
+}
+
+static void vcpu_exit(qemu_plugin_id_t id, unsigned int cpu_index)
+{
+ vCPUTime *vcpu = get_vcpu(cpu_index);
+ vcpu->state = FINISHED;
+ vcpu->state_time = *vcpu->counter;
+}
+
+/*
+ * tb exec
+ */
+static void vcpu_tb_exec(unsigned int cpu_index, void *udata)
+{
+ vCPUTime *vcpu = get_vcpu(cpu_index);
+ uint64_t count = *vcpu->counter;
+
+ count += GPOINTER_TO_UINT(udata);
+
+ if (count >= next_check) {
+ update_system_time(count);
+ }
+}
+
+/*
+ * We have two choices at translation time. In imprecise mode we just
+ * install a tb execution callback with the total number of
+ * instructions in the block. This ignores any partial execution
+ * effects but it reasonably fast. In precise mode we increment a
+ * per-vCPU counter for every execution.
+ */
+
+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_register_vcpu_tb_exec_cb(tb, vcpu_tb_exec,
+ QEMU_PLUGIN_CB_NO_REGS,
+ GUINT_TO_POINTER(n_insns));
+}
+
+/**
+ * Install the plugin
+ */
+QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
+ const qemu_info_t *info, int argc,
+ char **argv)
+{
+ /* This plugin only makes sense for system emulation */
+ if (!info->system_emulation) {
+ fprintf(stderr, "iops plugin only works with system emulation\n");
+ return -1;
+ }
+
+ for (int i = 0; i < argc; i++) {
+ char *opt = argv[i];
+ g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
+ if (g_strcmp0(tokens[0], "iops") == 0) {
+ iops = g_ascii_strtoull(tokens[1], NULL, 10);
+ if (!iops && errno) {
+ fprintf(stderr, "%s: couldn't parse %s (%s)\n",
+ __func__, tokens[1], g_strerror(errno));
+ return -1;
+ }
+
+ } else if (g_strcmp0(tokens[0], "precise") == 0) {
+ if (!qemu_plugin_bool_parse(tokens[0], tokens[1], &precise_execution)) {
+ fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
+ return -1;
+ }
+ } else {
+ fprintf(stderr, "option parsing failed: %s\n", opt);
+ return -1;
+ }
+ }
+
+ /*
+ * Setup the tracking information we need to run.
+ */
+ vcpus = g_array_new(true, true, sizeof(vCPUTime));
+ g_array_set_size(vcpus, info->system.max_vcpus);
+ vcpu_counters = g_malloc0_n(info->system.max_vcpus, sizeof(uint64_t));
+ for (int i = 0; i < info->system.max_vcpus; i++) {
+ vCPUTime *vcpu = get_vcpu(i);
+ vcpu->counter = &vcpu_counters[i];
+ }
+
+ /*
+ * We are going to check the state of time every slice so set the
+ * first check at t0 + iops/SLICES
+ */
+ next_check = iops/SLICES;
+
+ /*
+ * Only one plugin can request time control, if we don't get the
+ * handle there isn't much we can do.
+ */
+ time_handle = qemu_plugin_request_time_control();
+ if (!time_handle) {
+ fprintf(stderr, "%s: not given permission to control time\n", __func__);
+ return -1;
+ }
+
+ /*
+ * To track time we need to measure how many instructions each
+ * core is executing as well as when each vcpu enters/leaves the
+ */
+ qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
+
+ qemu_plugin_register_vcpu_init_cb(id, vcpu_init);
+ qemu_plugin_register_vcpu_idle_cb(id, vcpu_idle);
+ qemu_plugin_register_vcpu_resume_cb(id, vcpu_resume);
+ qemu_plugin_register_vcpu_exit_cb(id, vcpu_exit);
+
+ return 0;
+}
diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile
index b2b9db9f51..f269c18d11 100644
--- a/contrib/plugins/Makefile
+++ b/contrib/plugins/Makefile
@@ -21,6 +21,7 @@ NAMES += lockstep
NAMES += hwprofile
NAMES += cache
NAMES += drcov
+NAMES += iops
SONAMES := $(addsuffix .so,$(addprefix lib,$(NAMES)))
--
2.39.2
^ permalink raw reply related [flat|nested] 13+ messages in thread