Re: [RFC PATCH v3] contrib/plugins: control flow plugin
Pierrick Bouvier writes:
> On 7/18/24 07:59, Alex Bennée wrote:
>> This is a simple control flow tracking plugin that uses the latest
>> inline and conditional operations to detect and track control flow
>> changes. It is currently an exercise at seeing how useful the changes
>> are.
>> Based-on: <20240312075428.244210-1-pierrick.bouv...@linaro.org>
>> Cc: Gustavo Romero
>> Cc: Pierrick Bouvier
>> Signed-off-by: Alex Bennée
>> Message-Id: <20240311153432.1395190-1-alex.ben...@linaro.org>
>> +/*
>> + * Called when we detect a non-linear execution (pc !=
>> + * pc_after_block). This could be due to a fault causing some sort of
>> + * exit exception (if last_pc != block_end) or just a taken branch.
>> + */
>> +static void vcpu_tb_branched_exec(unsigned int cpu_index, void *udata)
>> +{
>> +uint64_t lpc = qemu_plugin_u64_get(last_pc, cpu_index);
>> +uint64_t ebpc = qemu_plugin_u64_get(end_block, cpu_index);
>> +uint64_t npc = qemu_plugin_u64_get(pc_after_block, cpu_index);
>> +uint64_t pc = GPOINTER_TO_UINT(udata);
>> +
>> +/* return early for address 0 */
>> +if (!lpc) {
>> +return;
>> +}
>> +
>> +NodeData *node = fetch_node(lpc, true);
>
> I would suggest a different approach here.
>
> This plugin keeps data as a graph between instructions.
> Another possibility would be to use a *per vcpu* hashtable, which
> simply associates the key (source_addr, dest_addr), to a number of
> hits.
> (uint64, uint64) -> uint64. This is all we really need at exec time,
> the rest can be reconstructed for data gathered at translation time.
Hmm I'm not sure how to deal with 128 bit keys with glib's hash table
implementation. I think the gpointer can be an opaque pointer though
with GEqualFunc to compare - but adding multiple records to a hash table
seems wrong.
> This way, you can do all the work in vcpu_tb_branched_exec without
> needing a single lock. (here, we lock twice, once globally to fetch
> all the nodes, and once for the node itself).
>
> Then, at exit, you can merge hashtables from all vcpu, and do the work
> to rebuild the full graph from all transitions collected.
Well a lot of transitions are just continuations (although maybe not I
guess I need to check that hunch).
> As a bonus, you can get the true list of hottest branches, when now,
> it's the hottest insn only you have.
I'm not sure I follow. Are you saying there are control flow changes I
don't detect? The fall-through cases?
> The Node structure would simply becomes Insn, as you want to keep the
> pc, symbols and disassembly of every instruction.
> And you need to keep track of all tb too, with length and pointing to
> the list of instructions.
What would I do with the TB information that I couldn't encode in Insn
at translation time?
>
> It's a different paradigm from what is doing here, but I think it
> would scale much better, especially with multithreaded programs.
>
>> +DestData *data = NULL;
>> +bool early_exit = (lpc != ebpc);
>> +GArray *dests;
>> +
>> +/* the condition should never hit */
>> +g_assert(pc != npc);
>> +
>> +g_mutex_lock(>lock);
>> +
>> +if (early_exit) {
>> +fprintf(stderr, "%s: pc=%"PRIx64", epbc=%"PRIx64
>> +" npc=%"PRIx64", lpc=%"PRIx64", \n",
>> +__func__, pc, ebpc, npc, lpc);
>> +node->early_exit++;
>> +if (!node->mid_count) {
>> +/* count now as we've only just allocated */
>> +node->mid_count++;
>> +}
>> +}
>> +
>> +dests = node->dests;
>> +for (int i = 0; i < dests->len; i++) {
>> +if (g_array_index(dests, DestData, i).daddr == pc) {
>> +data = _array_index(dests, DestData, i);
>> +}
>> +}
>> +
>> +/* we've never seen this before, allocate a new entry */
>> +if (!data) {
>> +DestData new_entry = { .daddr = pc };
>> +g_array_append_val(dests, new_entry);
>> +data = _array_index(dests, DestData, dests->len - 1);
>> +g_assert(data->daddr == pc);
>> +}
>> +
>> +data->dcount++;
>> +node->dest_count++;
>> +
>> +g_mutex_unlock(>lock);
>> +}
>> +
>> +/*
>> + * At the start of each block we need to resolve two things:
>> + *
>> + * - is last_pc == block_end, if not we had an early exit
>> + * - is start of block last_pc + insn width, if not we jumped
>&
Re: [PATCH] tests/tcg/aarch64: Fix test-mte.py
Richard Henderson writes:
> Python 3.12 warns:
>
> TESTgdbstub MTE support on aarch64
> /home/rth/qemu/src/tests/tcg/aarch64/gdbstub/test-mte.py:21: SyntaxWarning:
> invalid escape sequence '\('
> PATTERN_0 = "Memory tags for address 0x[0-9a-f]+ match \(0x[0-9a-f]+\)."
>
> Double up the \ to pass one through to the pattern.
>
> Signed-off-by: Richard Henderson
Queued to maintainer/for-9.1-rc0, thanks.
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
[RFC PATCH v3] contrib/plugins: control flow plugin
This is a simple control flow tracking plugin that uses the latest
inline and conditional operations to detect and track control flow
changes. It is currently an exercise at seeing how useful the changes
are.
Based-on: <20240312075428.244210-1-pierrick.bouv...@linaro.org>
Cc: Gustavo Romero
Cc: Pierrick Bouvier
Signed-off-by: Alex Bennée
Message-Id: <20240311153432.1395190-1-alex.ben...@linaro.org>
---
v2
- only need a single call back
- drop need for INSN_WIDTH
- still don't understand the early exits
v3
- move initial STORE ops to first instruction to avoid confusion
with the conditional callback on the start
- filter out non-branches before processing
- fix off-by-one with accounting
- display "sync fault" or "branch" instead of raw numbers
---
contrib/plugins/cflow.c | 364 +++
contrib/plugins/Makefile | 1 +
2 files changed, 365 insertions(+)
create mode 100644 contrib/plugins/cflow.c
diff --git a/contrib/plugins/cflow.c b/contrib/plugins/cflow.c
new file mode 100644
index 00..e5b0d4bbbd
--- /dev/null
+++ b/contrib/plugins/cflow.c
@@ -0,0 +1,364 @@
+/*
+ * Control Flow plugin
+ *
+ * This plugin will track changes to control flow and detect where
+ * instructions fault.
+ *
+ * Copyright (c) 2024 Linaro Ltd
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
+typedef enum {
+SORT_HOTDEST, /* hottest branch */
+SORT_EARLY,/* most early exits */
+SORT_POPDEST, /* most destinations */
+} ReportType;
+
+ReportType report = SORT_HOTDEST;
+int topn = 10;
+
+typedef struct {
+uint64_t daddr;
+uint64_t dcount;
+} DestData;
+
+/* A node is an address where we can go to multiple places */
+typedef struct {
+GMutex lock;
+/* address of the branch point */
+uint64_t addr;
+/* array of DestData */
+GArray *dests;
+/* early exit/fault count */
+uint64_t early_exit;
+/* jump destination count */
+uint64_t dest_count;
+/* instruction data */
+char *insn_disas;
+/* symbol? */
+const char *symbol;
+/* times translated as last in block? */
+int last_count;
+/* times translated in the middle of block? */
+int mid_count;
+} NodeData;
+
+/* We use this to track the current execution state */
+typedef struct {
+/* address of end of block */
+uint64_t end_block;
+/* next pc after end of block */
+uint64_t pc_after_block;
+/* address of last executed PC */
+uint64_t last_pc;
+} VCPUScoreBoard;
+
+/* descriptors for accessing the above scoreboard */
+static qemu_plugin_u64 end_block;
+static qemu_plugin_u64 pc_after_block;
+static qemu_plugin_u64 last_pc;
+
+
+static GMutex node_lock;
+static GHashTable *nodes;
+struct qemu_plugin_scoreboard *state;
+
+/* SORT_HOTDEST */
+static gint hottest(gconstpointer a, gconstpointer b)
+{
+NodeData *na = (NodeData *) a;
+NodeData *nb = (NodeData *) b;
+
+return na->dest_count > nb->dest_count ? -1 :
+na->dest_count == nb->dest_count ? 0 : 1;
+}
+
+static gint early(gconstpointer a, gconstpointer b)
+{
+NodeData *na = (NodeData *) a;
+NodeData *nb = (NodeData *) b;
+
+return na->early_exit > nb->early_exit ? -1 :
+na->early_exit == nb->early_exit ? 0 : 1;
+}
+
+static gint popular(gconstpointer a, gconstpointer b)
+{
+NodeData *na = (NodeData *) a;
+NodeData *nb = (NodeData *) b;
+
+return na->dests->len > nb->dests->len ? -1 :
+na->dests->len == nb->dests->len ? 0 : 1;
+}
+
+/* Filter out non-branches - returns true to remove entry */
+static gboolean filter_non_branches(gpointer key, gpointer value, gpointer
user_data)
+{
+NodeData *node = (NodeData *) value;
+
+return node->dest_count == 0;
+}
+
+static void plugin_exit(qemu_plugin_id_t id, void *p)
+{
+g_autoptr(GString) result = g_string_new("collected ");
+GList *data;
+GCompareFunc sort =
+int n = 0;
+
+g_mutex_lock(_lock);
+g_string_append_printf(result, "%d control flow nodes in the hash table\n",
+ g_hash_table_size(nodes));
+
+/* remove all nodes that didn't branch */
+g_hash_table_foreach_remove(nodes, filter_non_branches, NULL);
+
+data = g_hash_table_get_values(nodes);
+
+switch (report) {
+case SORT_HOTDEST:
+sort =
+break;
+case SORT_EARLY:
+sort =
+break;
+case SORT_POPDEST:
+sort =
+break;
+}
+
+data = g_list_sort(data, sort);
+
+for (GList *l = data;
+ l != NULL && n < topn;
+ l = l->next, n++) {
+NodeData *n = l->data;
+const char *type = n->mid_count ? "sync fault" : "branch";
+g_str
[PATCH 13/15] target/riscv: Restrict semihosting to TCG
From: Philippe Mathieu-Daudé Semihosting currently uses the TCG probe_access API. To prepare for encoding the TCG dependency in Kconfig, do not enable it unless TCG is available. Suggested-by: Paolo Bonzini Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Anton Johansson Message-Id: <20240717105723.58965-7-phi...@linaro.org> Signed-off-by: Alex Bennée --- target/riscv/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/riscv/Kconfig b/target/riscv/Kconfig index 5f30df22f2..c332616d36 100644 --- a/target/riscv/Kconfig +++ b/target/riscv/Kconfig @@ -1,9 +1,9 @@ config RISCV32 bool -select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting() +imply ARM_COMPATIBLE_SEMIHOSTING if TCG select DEVICE_TREE # needed by boot.c config RISCV64 bool -select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting() +imply ARM_COMPATIBLE_SEMIHOSTING if TCG select DEVICE_TREE # needed by boot.c -- 2.39.2
[PATCH 12/15] target/mips: Restrict semihosting to TCG
From: Philippe Mathieu-Daudé Semihosting currently uses the TCG probe_access API. To prepare for encoding the TCG dependency in Kconfig, do not enable it unless TCG is available. Suggested-by: Paolo Bonzini Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Anton Johansson Message-Id: <20240717105723.58965-6-phi...@linaro.org> Signed-off-by: Alex Bennée --- target/mips/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/mips/Kconfig b/target/mips/Kconfig index eb19c94c7d..876048b150 100644 --- a/target/mips/Kconfig +++ b/target/mips/Kconfig @@ -1,6 +1,6 @@ config MIPS bool -select SEMIHOSTING +imply SEMIHOSTING if TCG config MIPS64 bool -- 2.39.2
[PATCH 08/15] semihosting: Include missing 'gdbstub/syscalls.h' header
From: Philippe Mathieu-Daudé "semihosting/syscalls.h" requires definitions from "gdbstub/syscalls.h", include it in order to avoid: include/semihosting/syscalls.h:23:38: error: unknown type name 'gdb_syscall_complete_cb' void semihost_sys_open(CPUState *cs, gdb_syscall_complete_cb complete, ^ Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20240717105723.58965-2-phi...@linaro.org> Signed-off-by: Alex Bennée --- include/semihosting/syscalls.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/semihosting/syscalls.h b/include/semihosting/syscalls.h index 3a5ec229eb..b5937c619a 100644 --- a/include/semihosting/syscalls.h +++ b/include/semihosting/syscalls.h @@ -9,6 +9,8 @@ #ifndef SEMIHOSTING_SYSCALLS_H #define SEMIHOSTING_SYSCALLS_H +#include "gdbstub/syscalls.h" + /* * Argument loading from the guest is performed by the caller; * results are returned via the 'complete' callback. -- 2.39.2
[PATCH 15/15] semihosting: Restrict to TCG
From: Philippe Mathieu-Daudé Semihosting currently uses the TCG probe_access API. It is pointless to have it in the binary when TCG isn't. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Message-Id: <20240717105723.58965-9-phi...@linaro.org> Signed-off-by: Alex Bennée --- semihosting/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/semihosting/Kconfig b/semihosting/Kconfig index eaf3a20ef5..fbe6ac87f9 100644 --- a/semihosting/Kconfig +++ b/semihosting/Kconfig @@ -1,6 +1,7 @@ config SEMIHOSTING bool + depends on TCG config ARM_COMPATIBLE_SEMIHOSTING bool -- 2.39.2
[PATCH 06/15] tests/plugins: use qemu_plugin_outs for inline stats
Using bare printf's in plugins is perfectly acceptable but they do
rather mess up the output of "make check-tcg". Convert the printfs to
use g_string and then output with the plugin output helper which will
already be captured to .pout files by the test harness.
Signed-off-by: Alex Bennée
---
tests/plugin/inline.c | 58 ---
1 file changed, 33 insertions(+), 25 deletions(-)
diff --git a/tests/plugin/inline.c b/tests/plugin/inline.c
index cd63827b7d..73dde99578 100644
--- a/tests/plugin/inline.c
+++ b/tests/plugin/inline.c
@@ -71,10 +71,12 @@ static void stats_insn(void)
const uint64_t cond_track_left =
qemu_plugin_u64_sum(insn_cond_track_count);
const uint64_t conditional =
cond_num_trigger * cond_trigger_limit + cond_track_left;
-printf("insn: %" PRIu64 "\n", expected);
-printf("insn: %" PRIu64 " (per vcpu)\n", per_vcpu);
-printf("insn: %" PRIu64 " (per vcpu inline)\n", inl_per_vcpu);
-printf("insn: %" PRIu64 " (cond cb)\n", conditional);
+g_autoptr(GString) stats = g_string_new("");
+g_string_append_printf(stats, "insn: %" PRIu64 "\n", expected);
+g_string_append_printf(stats, "insn: %" PRIu64 " (per vcpu)\n", per_vcpu);
+g_string_append_printf(stats, "insn: %" PRIu64 " (per vcpu inline)\n",
inl_per_vcpu);
+g_string_append_printf(stats, "insn: %" PRIu64 " (cond cb)\n",
conditional);
+qemu_plugin_outs(stats->str);
g_assert(expected > 0);
g_assert(per_vcpu == expected);
g_assert(inl_per_vcpu == expected);
@@ -91,10 +93,12 @@ static void stats_tb(void)
const uint64_t cond_track_left = qemu_plugin_u64_sum(tb_cond_track_count);
const uint64_t conditional =
cond_num_trigger * cond_trigger_limit + cond_track_left;
-printf("tb: %" PRIu64 "\n", expected);
-printf("tb: %" PRIu64 " (per vcpu)\n", per_vcpu);
-printf("tb: %" PRIu64 " (per vcpu inline)\n", inl_per_vcpu);
-printf("tb: %" PRIu64 " (conditional cb)\n", conditional);
+g_autoptr(GString) stats = g_string_new("");
+g_string_append_printf(stats, "tb: %" PRIu64 "\n", expected);
+g_string_append_printf(stats, "tb: %" PRIu64 " (per vcpu)\n", per_vcpu);
+g_string_append_printf(stats, "tb: %" PRIu64 " (per vcpu inline)\n",
inl_per_vcpu);
+g_string_append_printf(stats, "tb: %" PRIu64 " (conditional cb)\n",
conditional);
+qemu_plugin_outs(stats->str);
g_assert(expected > 0);
g_assert(per_vcpu == expected);
g_assert(inl_per_vcpu == expected);
@@ -107,9 +111,11 @@ static void stats_mem(void)
const uint64_t per_vcpu = qemu_plugin_u64_sum(count_mem);
const uint64_t inl_per_vcpu =
qemu_plugin_u64_sum(count_mem_inline);
-printf("mem: %" PRIu64 "\n", expected);
-printf("mem: %" PRIu64 " (per vcpu)\n", per_vcpu);
-printf("mem: %" PRIu64 " (per vcpu inline)\n", inl_per_vcpu);
+g_autoptr(GString) stats = g_string_new("");
+g_string_append_printf(stats, "mem: %" PRIu64 "\n", expected);
+g_string_append_printf(stats, "mem: %" PRIu64 " (per vcpu)\n", per_vcpu);
+g_string_append_printf(stats, "mem: %" PRIu64 " (per vcpu inline)\n",
inl_per_vcpu);
+qemu_plugin_outs(stats->str);
g_assert(expected > 0);
g_assert(per_vcpu == expected);
g_assert(inl_per_vcpu == expected);
@@ -118,6 +124,7 @@ static void stats_mem(void)
static void plugin_exit(qemu_plugin_id_t id, void *udata)
{
const unsigned int num_cpus = qemu_plugin_num_vcpus();
+g_autoptr(GString) stats = g_string_new("");
g_assert(num_cpus == max_cpu_index + 1);
for (int i = 0; i < num_cpus ; ++i) {
@@ -135,20 +142,21 @@ static void plugin_exit(qemu_plugin_id_t id, void *udata)
qemu_plugin_u64_get(insn_cond_num_trigger, i);
const uint64_t insn_cond_left =
qemu_plugin_u64_get(insn_cond_track_count, i);
-printf("cpu %d: tb (%" PRIu64 ", %" PRIu64
- ", %" PRIu64 " * %" PRIu64 " + %" PRIu64
- ") | "
- "insn (%" PRIu64 ", %" PRIu64
- ", %" PRIu64 " * %" PRIu64 " + %" PRIu64
- ") | "
- "mem (%" PRIu64 ", %" PRIu64 ")"
- "\n",
- i,
- tb, tb_inline,
- tb_cond_trigger, cond_trigger_limit, tb_cond_left,
-
[PATCH 11/15] target/m68k: Restrict semihosting to TCG
From: Philippe Mathieu-Daudé The semihosting feature depends on TCG (due to the probe_access API access). Although TCG is the single accelerator currently available for the m68k target, use the Kconfig "imply" directive which is more correct (if we were to support a different accel). Reported-by: Anton Johansson Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20240717105723.58965-5-phi...@linaro.org> Signed-off-by: Alex Bennée --- target/m68k/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/m68k/Kconfig b/target/m68k/Kconfig index 9eae71486f..23aae24ebe 100644 --- a/target/m68k/Kconfig +++ b/target/m68k/Kconfig @@ -1,3 +1,3 @@ config M68K bool -select SEMIHOSTING +imply SEMIHOSTING if TCG -- 2.39.2
[PATCH 04/15] plugins/stoptrigger: TCG plugin to stop execution under conditions
From: Simon Hamelin
This new plugin allows to stop emulation using conditions on the
emulation state. By setting this plugin arguments, it is possible
to set an instruction count limit and/or trigger address(es) to stop at.
The code returned at emulation exit can be customized.
This plugin demonstrates how someone could stop QEMU execution.
It could be used for research purposes to launch some code and
deterministically stop it and understand where its execution flow went.
Co-authored-by: Alexandre Iooss
Signed-off-by: Simon Hamelin
Signed-off-by: Alexandre Iooss
Reviewed-by: Pierrick Bouvier
Message-Id: <20240715081521.19122-2-simon.hame...@grenoble-inp.org>
Signed-off-by: Alex Bennée
---
docs/devel/tcg-plugins.rst| 22 +
contrib/plugins/stoptrigger.c | 151 ++
contrib/plugins/Makefile | 1 +
3 files changed, 174 insertions(+)
create mode 100644 contrib/plugins/stoptrigger.c
diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
index f7d7b9e3a4..954623f9bf 100644
--- a/docs/devel/tcg-plugins.rst
+++ b/docs/devel/tcg-plugins.rst
@@ -642,6 +642,28 @@ The plugin has a number of arguments, all of them are
optional:
configuration arguments implies ``l2=on``.
(default: N = 2097152 (2MB), B = 64, A = 16)
+- contrib/plugins/stoptrigger.c
+
+The stoptrigger plugin allows to setup triggers to stop emulation.
+It can be used for research purposes to launch some code and precisely stop it
+and understand where its execution flow went.
+
+Two types of triggers can be configured: a count of instructions to stop at,
+or an address to stop at. Multiple triggers can be set at once.
+
+By default, QEMU will exit with return code 0. A custom return code can be
+configured for each trigger using ``:CODE`` syntax.
+
+For example, to stop at the 20-th instruction with return code 41, at address
+0xd4 with return code 0 or at address 0xd8 with return code 42::
+
+ $ qemu-system-aarch64 $(QEMU_ARGS) \
+-plugin
./contrib/plugins/libstoptrigger.so,icount=20:41,addr=0xd4,addr=0xd8:42 -d
plugin
+
+The plugin will log the reason of exit, for example::
+
+ 0xd4 reached, exiting
+
Plugin API
==
diff --git a/contrib/plugins/stoptrigger.c b/contrib/plugins/stoptrigger.c
new file mode 100644
index 00..03ee22f4c6
--- /dev/null
+++ b/contrib/plugins/stoptrigger.c
@@ -0,0 +1,151 @@
+/*
+ * Copyright (C) 2024, Simon Hamelin
+ *
+ * Stop execution once a given address is reached or if the
+ * count of executed instructions reached a specified limit
+ *
+ * License: GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
+/* Scoreboard to track executed instructions count */
+typedef struct {
+uint64_t insn_count;
+} InstructionsCount;
+static struct qemu_plugin_scoreboard *insn_count_sb;
+static qemu_plugin_u64 insn_count;
+
+static uint64_t icount;
+static int icount_exit_code;
+
+static bool exit_on_icount;
+static bool exit_on_address;
+
+/* Map trigger addresses to exit code */
+static GHashTable *addrs_ht;
+
+static void exit_emulation(int return_code, char *message)
+{
+qemu_plugin_outs(message);
+g_free(message);
+exit(return_code);
+}
+
+static void exit_icount_reached(unsigned int cpu_index, void *udata)
+{
+uint64_t insn_vaddr = GPOINTER_TO_UINT(udata);
+char *msg = g_strdup_printf("icount reached at 0x%" PRIx64 ", exiting\n",
+insn_vaddr);
+
+exit_emulation(icount_exit_code, msg);
+}
+
+static void exit_address_reached(unsigned int cpu_index, void *udata)
+{
+uint64_t insn_vaddr = GPOINTER_TO_UINT(udata);
+char *msg = g_strdup_printf("0x%" PRIx64 " reached, exiting\n",
insn_vaddr);
+int exit_code;
+
+exit_code = GPOINTER_TO_INT(
+g_hash_table_lookup(addrs_ht, GUINT_TO_POINTER(insn_vaddr)));
+
+exit_emulation(exit_code, msg);
+}
+
+static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
+{
+size_t tb_n = qemu_plugin_tb_n_insns(tb);
+for (size_t i = 0; i < tb_n; i++) {
+struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i);
+gpointer insn_vaddr = GUINT_TO_POINTER(qemu_plugin_insn_vaddr(insn));
+
+if (exit_on_icount) {
+/* Increment and check scoreboard for each instruction */
+qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
+insn, QEMU_PLUGIN_INLINE_ADD_U64, insn_count, 1);
+qemu_plugin_register_vcpu_insn_exec_cond_cb(
+insn, exit_icount_reached, QEMU_PLUGIN_CB_NO_REGS,
+QEMU_PLUGIN_COND_EQ, insn_count, icount + 1, insn_vaddr);
+}
+
+if (exit_on_address) {
+if (g_hash_table_contains(addrs_ht, insn_vaddr)
[PATCH 09/15] target/m68k: Add semihosting stub
From: Philippe Mathieu-Daudé
Since the SEMIHOSTING feature is optional, we need
a stub to link when it is disabled.
Suggested-by: Paolo Bonzini
Signed-off-by: Philippe Mathieu-Daudé
Message-Id: <20240717105723.58965-3-phi...@linaro.org>
Signed-off-by: Alex Bennée
---
target/m68k/semihosting-stub.c | 15 +++
target/m68k/meson.build| 5 -
2 files changed, 19 insertions(+), 1 deletion(-)
create mode 100644 target/m68k/semihosting-stub.c
diff --git a/target/m68k/semihosting-stub.c b/target/m68k/semihosting-stub.c
new file mode 100644
index 00..d6a5965e29
--- /dev/null
+++ b/target/m68k/semihosting-stub.c
@@ -0,0 +1,15 @@
+/*
+ * m68k/ColdFire semihosting stub
+ *
+ * SPDX-FileContributor: Philippe Mathieu-Daudé
+ * SPDX-FileCopyrightText: 2024 Linaro Ltd.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+
+void do_m68k_semihosting(CPUM68KState *env, int nr)
+{
+g_assert_not_reached();
+}
diff --git a/target/m68k/meson.build b/target/m68k/meson.build
index 8d3f9ce288..4d213daaf6 100644
--- a/target/m68k/meson.build
+++ b/target/m68k/meson.build
@@ -11,9 +11,12 @@ m68k_ss.add(files(
m68k_system_ss = ss.source_set()
m68k_system_ss.add(files(
- 'm68k-semi.c',
'monitor.c'
))
+m68k_system_ss.add(when: ['CONFIG_SEMIHOSTING'],
+ if_true: files('m68k-semi.c'),
+ if_false: files('semihosting-stub.c')
+)
target_arch += {'m68k': m68k_ss}
target_system_arch += {'m68k': m68k_system_ss}
--
2.39.2
[PATCH 07/15] plugins/execlog.c: correct dump of registers values
From: Frédéric Pétrot
Register values are dumped as 'sz' chunks of two nibbles in the execlog
plugin, sz was 1 too big.
Signed-off-by: Frédéric Pétrot
Reviewed-by: Pierrick Bouvier
Message-Id: <20240620083805.73603-1-frederic.pet...@univ-grenoble-alpes.fr>
Signed-off-by: Alex Bennée
---
contrib/plugins/execlog.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
index 371db97eb1..1c1601cc0b 100644
--- a/contrib/plugins/execlog.c
+++ b/contrib/plugins/execlog.c
@@ -101,7 +101,7 @@ static void insn_check_regs(CPU *cpu)
GByteArray *temp = reg->last;
g_string_append_printf(cpu->last_exec, ", %s -> 0x", reg->name);
/* TODO: handle BE properly */
-for (int i = sz; i >= 0; i--) {
+for (int i = sz - 1; i >= 0; i--) {
g_string_append_printf(cpu->last_exec, "%02x",
reg->new->data[i]);
}
--
2.39.2
[PATCH 14/15] target/xtensa: Restrict semihosting to TCG
From: Philippe Mathieu-Daudé The semihosting feature depends on TCG (due to the probe_access API access). Although TCG is the single accelerator currently available for the xtensa target, use the Kconfig "imply" directive which is more correct (if we were to support a different accel). Reported-by: Anton Johansson Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20240717105723.58965-8-phi...@linaro.org> Signed-off-by: Alex Bennée --- target/xtensa/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/xtensa/Kconfig b/target/xtensa/Kconfig index 5e46049262..e8c2598c4d 100644 --- a/target/xtensa/Kconfig +++ b/target/xtensa/Kconfig @@ -1,3 +1,3 @@ config XTENSA bool -select SEMIHOSTING +imply SEMIHOSTING if TCG -- 2.39.2
[PATCH 00/15] Final bits for 9.1-rc0 (docker, plugins, gdbstub, semihosting)
I'm just flushing my various maintainer queues for the up-coming 9.1 soft freeze. Mostly this is a collection of fixes and tweaks although there is a new plugin in contrib. We've also bumped the libvirt-ci for the BSD python updates. The following still need review: tests/plugins: use qemu_plugin_outs for inline stats testing: bump to latest libvirt-ci Alex Bennée (3): testing: bump to latest libvirt-ci gdbstub: Re-factor gdb command extensions tests/plugins: use qemu_plugin_outs for inline stats Frédéric Pétrot (1): plugins/execlog.c: correct dump of registers values Philippe Mathieu-Daudé (8): semihosting: Include missing 'gdbstub/syscalls.h' header target/m68k: Add semihosting stub target/mips: Add semihosting stub target/m68k: Restrict semihosting to TCG target/mips: Restrict semihosting to TCG target/riscv: Restrict semihosting to TCG target/xtensa: Restrict semihosting to TCG semihosting: Restrict to TCG Pierrick Bouvier (1): plugins: fix mem callback array size Simon Hamelin (1): plugins/stoptrigger: TCG plugin to stop execution under conditions Thomas Huth (1): tests/avocado: Remove non-working sparc leon3 test MAINTAINERS | 1 - docs/devel/tcg-plugins.rst| 22 include/gdbstub/commands.h| 19 ++- include/semihosting/syscalls.h| 2 + target/arm/internals.h| 4 +- accel/tcg/plugin-gen.c| 3 +- contrib/plugins/execlog.c | 2 +- contrib/plugins/stoptrigger.c | 151 ++ gdbstub/gdbstub.c | 142 +++- target/arm/gdbstub.c | 16 +-- target/arm/gdbstub64.c| 11 +- target/m68k/semihosting-stub.c| 15 +++ target/mips/tcg/sysemu/semihosting-stub.c | 15 +++ tests/plugin/inline.c | 58 + .gitlab-ci.d/cirrus/freebsd-13.vars | 2 +- contrib/plugins/Makefile | 1 + semihosting/Kconfig | 1 + target/m68k/Kconfig | 2 +- target/m68k/meson.build | 5 +- target/mips/Kconfig | 2 +- target/mips/tcg/sysemu/meson.build| 6 +- target/riscv/Kconfig | 4 +- target/xtensa/Kconfig | 2 +- tests/avocado/machine_sparc_leon3.py | 37 -- tests/lcitool/libvirt-ci | 2 +- tests/vm/generated/freebsd.json | 14 +- 26 files changed, 370 insertions(+), 169 deletions(-) create mode 100644 contrib/plugins/stoptrigger.c create mode 100644 target/m68k/semihosting-stub.c create mode 100644 target/mips/tcg/sysemu/semihosting-stub.c delete mode 100644 tests/avocado/machine_sparc_leon3.py -- 2.39.2
[PATCH 05/15] plugins: fix mem callback array size
From: Pierrick Bouvier data was correctly copied, but size of array was not set (g_array_sized_new only reserves memory, but does not set size). As a result, callbacks were not called for code path relying on plugin_register_vcpu_mem_cb(). Found when trying to trigger mem access callbacks for atomic instructions. Reviewed-by: Xingtao Yao Reviewed-by: Richard Henderson Signed-off-by: Pierrick Bouvier Message-Id: <20240706191335.878142-2-pierrick.bouv...@linaro.org> --- accel/tcg/plugin-gen.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c index b6bae32b99..ec89a085b4 100644 --- a/accel/tcg/plugin-gen.c +++ b/accel/tcg/plugin-gen.c @@ -85,8 +85,7 @@ static void gen_enable_mem_helper(struct qemu_plugin_tb *ptb, len = insn->mem_cbs->len; arr = g_array_sized_new(false, false, sizeof(struct qemu_plugin_dyn_cb), len); -memcpy(arr->data, insn->mem_cbs->data, - len * sizeof(struct qemu_plugin_dyn_cb)); +g_array_append_vals(arr, insn->mem_cbs->data, len); qemu_plugin_add_dyn_cb_arr(arr); tcg_gen_st_ptr(tcg_constant_ptr((intptr_t)arr), tcg_env, -- 2.39.2
[PATCH 10/15] target/mips: Add semihosting stub
From: Philippe Mathieu-Daudé
Since the SEMIHOSTING feature is optional, we need
a stub to link when it is disabled.
Signed-off-by: Philippe Mathieu-Daudé
Message-Id: <20240717105723.58965-4-phi...@linaro.org>
Signed-off-by: Alex Bennée
---
target/mips/tcg/sysemu/semihosting-stub.c | 15 +++
target/mips/tcg/sysemu/meson.build| 6 --
2 files changed, 19 insertions(+), 2 deletions(-)
create mode 100644 target/mips/tcg/sysemu/semihosting-stub.c
diff --git a/target/mips/tcg/sysemu/semihosting-stub.c
b/target/mips/tcg/sysemu/semihosting-stub.c
new file mode 100644
index 00..7ae27d746f
--- /dev/null
+++ b/target/mips/tcg/sysemu/semihosting-stub.c
@@ -0,0 +1,15 @@
+/*
+ * MIPS semihosting stub
+ *
+ * SPDX-FileContributor: Philippe Mathieu-Daudé
+ * SPDX-FileCopyrightText: 2024 Linaro Ltd.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "internal.h"
+
+void mips_semihosting(CPUMIPSState *env)
+{
+g_assert_not_reached();
+}
diff --git a/target/mips/tcg/sysemu/meson.build
b/target/mips/tcg/sysemu/meson.build
index ec665a4b1e..911341ac37 100644
--- a/target/mips/tcg/sysemu/meson.build
+++ b/target/mips/tcg/sysemu/meson.build
@@ -1,10 +1,12 @@
mips_system_ss.add(files(
'cp0_helper.c',
- 'mips-semi.c',
'special_helper.c',
'tlb_helper.c',
))
-
+mips_system_ss.add(when: ['CONFIG_SEMIHOSTING'],
+ if_true: files('mips-semi.c'),
+ if_false: files('semihosting-stub.c')
+)
mips_system_ss.add(when: 'TARGET_MIPS64', if_true: files(
'lcsr_helper.c',
))
--
2.39.2
[PATCH 01/15] testing: bump to latest libvirt-ci
This brings in the latest python mappings for the BSD updates. Signed-off-by: Alex Bennée --- .gitlab-ci.d/cirrus/freebsd-13.vars | 2 +- tests/lcitool/libvirt-ci| 2 +- tests/vm/generated/freebsd.json | 14 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/.gitlab-ci.d/cirrus/freebsd-13.vars b/.gitlab-ci.d/cirrus/freebsd-13.vars index 3785afca36..29ab9645f9 100644 --- a/.gitlab-ci.d/cirrus/freebsd-13.vars +++ b/.gitlab-ci.d/cirrus/freebsd-13.vars @@ -11,6 +11,6 @@ MAKE='/usr/local/bin/gmake' NINJA='/usr/local/bin/ninja' PACKAGING_COMMAND='pkg' PIP3='/usr/local/bin/pip-3.8' -PKGS='alsa-lib bash bison bzip2 ca_root_nss capstone4 ccache cmocka ctags curl cyrus-sasl dbus diffutils dtc flex fusefs-libs3 gettext git glib gmake gnutls gsed gtk3 json-c libepoxy libffi libgcrypt libjpeg-turbo libnfs libslirp libspice-server libssh libtasn1 llvm lzo2 meson mtools ncurses nettle ninja opencv pixman pkgconf png py39-numpy py39-pillow py39-pip py39-sphinx py39-sphinx_rtd_theme py39-tomli py39-yaml python3 rpm2cpio sdl2 sdl2_image snappy sndio socat spice-protocol tesseract usbredir virglrenderer vte3 xorriso zstd' +PKGS='alsa-lib bash bison bzip2 ca_root_nss capstone4 ccache cmocka ctags curl cyrus-sasl dbus diffutils dtc flex fusefs-libs3 gettext git glib gmake gnutls gsed gtk3 json-c libepoxy libffi libgcrypt libjpeg-turbo libnfs libslirp libspice-server libssh libtasn1 llvm lzo2 meson mtools ncurses nettle ninja opencv pixman pkgconf png py311-numpy py311-pillow py311-pip py311-sphinx py311-sphinx_rtd_theme py311-tomli py311-yaml python3 rpm2cpio sdl2 sdl2_image snappy sndio socat spice-protocol tesseract usbredir virglrenderer vte3 xorriso zstd' PYPI_PKGS='' PYTHON='/usr/local/bin/python3' diff --git a/tests/lcitool/libvirt-ci b/tests/lcitool/libvirt-ci index 0e9490cebc..789b4601bc 16 --- a/tests/lcitool/libvirt-ci +++ b/tests/lcitool/libvirt-ci @@ -1 +1 @@ -Subproject commit 0e9490cebc726ef772b6c9e27dac32e7ae99f9b2 +Subproject commit 789b4601bce4e01f43fdb6ad4ce5ab4e46674440 diff --git a/tests/vm/generated/freebsd.json b/tests/vm/generated/freebsd.json index 2d5895ebed..2a361cecd0 100644 --- a/tests/vm/generated/freebsd.json +++ b/tests/vm/generated/freebsd.json @@ -51,13 +51,13 @@ "pixman", "pkgconf", "png", -"py39-numpy", -"py39-pillow", -"py39-pip", -"py39-sphinx", -"py39-sphinx_rtd_theme", -"py39-tomli", -"py39-yaml", +"py311-numpy", +"py311-pillow", +"py311-pip", +"py311-sphinx", +"py311-sphinx_rtd_theme", +"py311-tomli", +"py311-yaml", "python3", "rpm2cpio", "sdl2", -- 2.39.2
[PATCH 03/15] gdbstub: Re-factor gdb command extensions
Coverity reported a memory leak (CID 1549757) in this code and its
admittedly rather clumsy handling of extending the command table.
Instead of handing over a full array of the commands lets use the
lighter weight GPtrArray and simply test for the presence of each
entry as we go. This avoids complications of transferring ownership of
arrays and keeps the final command entries as static entries in the
target code.
Cc: Akihiko Odaki
Cc: Gustavo Bueno Romero
Cc: Peter Maydell
Signed-off-by: Alex Bennée
Reviewed-by: Gustavo Romero
---
v2
- don't use strrstr, instead maintain a strv array
- process the array later when building the query response
- shorten names to hit line length limit
---
include/gdbstub/commands.h | 19 +++--
target/arm/internals.h | 4 +-
gdbstub/gdbstub.c | 142 +
target/arm/gdbstub.c | 16 ++---
target/arm/gdbstub64.c | 11 ++-
5 files changed, 106 insertions(+), 86 deletions(-)
diff --git a/include/gdbstub/commands.h b/include/gdbstub/commands.h
index f3058f9dda..40f0514fe9 100644
--- a/include/gdbstub/commands.h
+++ b/include/gdbstub/commands.h
@@ -74,23 +74,28 @@ int gdb_put_packet(const char *buf);
/**
* gdb_extend_query_table() - Extend query table.
- * @table: The table with the additional query packet handlers.
- * @size: The number of handlers to be added.
+ * @table: GPtrArray of GdbCmdParseEntry entries.
+ *
+ * The caller should free @table afterwards
*/
-void gdb_extend_query_table(GdbCmdParseEntry *table, int size);
+void gdb_extend_query_table(GPtrArray *table);
/**
* gdb_extend_set_table() - Extend set table.
- * @table: The table with the additional set packet handlers.
- * @size: The number of handlers to be added.
+ * @table: GPtrArray of GdbCmdParseEntry entries.
+ *
+ * The caller should free @table afterwards
*/
-void gdb_extend_set_table(GdbCmdParseEntry *table, int size);
+void gdb_extend_set_table(GPtrArray *table);
/**
* gdb_extend_qsupported_features() - Extend the qSupported features string.
* @qsupported_features: The additional qSupported feature(s) string. The
string
* should start with a semicolon and, if there are more than one feature, the
- * features should be separate by a semiocolon.
+ * features should be separate by a semicolon.
+ *
+ * The caller should free @qsupported_features afterwards if
+ * dynamically allocated.
*/
void gdb_extend_qsupported_features(char *qsupported_features);
diff --git a/target/arm/internals.h b/target/arm/internals.h
index da22d04121..757b1fae92 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -359,8 +359,8 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu);
void arm_translate_init(void);
void arm_cpu_register_gdb_commands(ARMCPU *cpu);
-void aarch64_cpu_register_gdb_commands(ARMCPU *cpu, GString *, GArray *,
- GArray *);
+void aarch64_cpu_register_gdb_commands(ARMCPU *cpu, GString *,
+ GPtrArray *, GPtrArray *);
void arm_restore_state_to_opc(CPUState *cs,
const TranslationBlock *tb,
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index b9ad0a063e..104398f922 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -1614,18 +1614,21 @@ static void handle_query_thread_extra(GArray *params,
void *user_ctx)
gdb_put_strbuf();
}
-static char *extended_qsupported_features;
-void gdb_extend_qsupported_features(char *qsupported_features)
-{
-/*
- * We don't support different sets of CPU gdb features on different CPUs
yet
- * so assert the feature strings are the same on all CPUs, or is set only
- * once (1 CPU).
- */
-g_assert(extended_qsupported_features == NULL ||
- g_strcmp0(extended_qsupported_features, qsupported_features) ==
0);
-extended_qsupported_features = qsupported_features;
+static char **extra_query_flags;
+
+void gdb_extend_qsupported_features(char *qflags)
+{
+if (!extra_query_flags) {
+extra_query_flags = g_new0(char *, 2);
+extra_query_flags[0] = g_strdup(qflags);
+} else if (!g_strv_contains((const gchar * const *) extra_query_flags,
+qflags)) {
+int len = g_strv_length(extra_query_flags);
+extra_query_flags = g_realloc_n(extra_query_flags, len + 2,
+sizeof(char *));
+extra_query_flags[len] = g_strdup(qflags);
+}
}
static void handle_query_supported(GArray *params, void *user_ctx)
@@ -1668,8 +1671,11 @@ static void handle_query_supported(GArray *params, void
*user_ctx)
g_string_append(gdbserver_state.str_buf, ";vContSupported+;multiprocess+");
-if (extended_qsupported_features) {
-g_string_append(gdbserver_state.str_buf, extended_qsupported_features);
+if (extra_query_flags) {
+int extras = g_strv_length(extra_query_flags);
+for (in
[PATCH 02/15] tests/avocado: Remove non-working sparc leon3 test
From: Thomas Huth
The test has been marked as broken more than 4 years ago, and
so far nobody ever cared to fix it. Thus let's simply remove it
now ... if somebody ever needs it again, they can restore the
file from an older version of QEMU.
Signed-off-by: Thomas Huth
Reviewed-by: Clément Chigot
Acked-by: Alex Bennée
Message-Id: <20240710111755.60584-1-th...@redhat.com>
[AJB: fix MAINTAINERS]
Signed-off-by: Alex Bennée
---
MAINTAINERS | 1 -
tests/avocado/machine_sparc_leon3.py | 37
2 files changed, 38 deletions(-)
delete mode 100644 tests/avocado/machine_sparc_leon3.py
diff --git a/MAINTAINERS b/MAINTAINERS
index 7d9811458c..d5ff6c2498 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1727,7 +1727,6 @@ S: Maintained
F: hw/sparc/leon3.c
F: hw/*/grlib*
F: include/hw/*/grlib*
-F: tests/avocado/machine_sparc_leon3.py
S390 Machines
-
diff --git a/tests/avocado/machine_sparc_leon3.py
b/tests/avocado/machine_sparc_leon3.py
deleted file mode 100644
index e61b223185..00
--- a/tests/avocado/machine_sparc_leon3.py
+++ /dev/null
@@ -1,37 +0,0 @@
-# Functional test that boots a Leon3 machine and checks its serial console.
-#
-# Copyright (c) Philippe Mathieu-Daudé
-#
-# This work is licensed under the terms of the GNU GPL, version 2 or
-# later. See the COPYING file in the top-level directory.
-
-from avocado_qemu import QemuSystemTest
-from avocado_qemu import wait_for_console_pattern
-from avocado import skip
-
-
-class Leon3Machine(QemuSystemTest):
-
-timeout = 60
-
-@skip("Test currently broken")
-# A Window Underflow exception occurs before booting the kernel,
-# and QEMU exit calling cpu_abort(), which makes this test to fail.
-def test_leon3_helenos_uimage(self):
-"""
-:avocado: tags=arch:sparc
-:avocado: tags=machine:leon3_generic
-:avocado: tags=binfmt:uimage
-"""
-kernel_url = ('http://www.helenos.org/releases/'
- 'HelenOS-0.6.0-sparc32-leon3.bin')
-kernel_hash = 'a88c9cfdb8430c66650e5290a08765f9bf049a30'
-kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
-
-self.vm.set_console()
-self.vm.add_args('-kernel', kernel_path)
-
-self.vm.launch()
-
-wait_for_console_pattern(self, 'Copyright (c) 2001-2014 HelenOS
project')
-wait_for_console_pattern(self, 'Booting the kernel ...')
--
2.39.2
Re: [RFC PATCH] hw/core/cpu.h: try and document CPU_FOREACH[_SAFE]
Alex Bennée writes: > There is some confusion about when you should use one over the other. > Lets try and address that by adding some kdoc comments. > > Suggested-by: Paolo Bonzini > Signed-off-by: Alex Bennée ping? > --- > include/hw/core/cpu.h | 19 ++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h > index a2c8536943..7122f742c1 100644 > --- a/include/hw/core/cpu.h > +++ b/include/hw/core/cpu.h > @@ -587,8 +587,25 @@ extern CPUTailQ cpus_queue; > > #define first_cpuQTAILQ_FIRST_RCU(_queue) > #define CPU_NEXT(cpu)QTAILQ_NEXT_RCU(cpu, node) > + > +/** > + * CPU_FOREACH - Helper to iterate over all CPUs > + * > + * This macro iterates over all CPUs in the system. It must be used > + * under an RCU read protection, e.g. WITH_RCU_READ_LOCK_GUARD(). If > + * you don't want the CPU list to change while iterating use > + * CPU_FOREACH_SAFE under the cpu_list_lock(). > + */ > #define CPU_FOREACH(cpu) QTAILQ_FOREACH_RCU(cpu, _queue, node) > -#define CPU_FOREACH_SAFE(cpu, next_cpu) \ > + > +/** > + * CPU_FOREACH_SAFE - Helper to iterate over all CPUs, safe against CPU > changes > + * > + * This macro iterates over all CPUs in the system, and is safe > + * against CPU list changes. The target data structure must be > + * protected by cpu_list_lock(), and does not need RCU. > + */ > +#define CPU_FOREACH_SAFE(cpu, next_cpu) \ > QTAILQ_FOREACH_SAFE_RCU(cpu, _queue, node, next_cpu) > > extern __thread CPUState *current_cpu; -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH v4 0/8] semihosting: Restrict to TCG
Philippe Mathieu-Daudé writes: > v4: Address Paolo's comment > v3: Address Anton's comment > v2: Address Paolo's comment > > Semihosting currently uses the TCG probe_access API, > so it is pointless to have it in the binary when TCG > isn't. > > It could be implemented for other accelerators, but > work need to be done. Meanwhile, do not enable it > unless TCG is available. Queued to semihosting/next, thanks. -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH] plugins/execlog.c: correct dump of registers values
Frédéric Pétrot writes: > Register values are dumped as 'sz' chunks of two nibbles in the execlog > plugin, sz was 1 too big. > > Signed-off-by: Frédéric Pétrot > Queued to plugins/next, thanks. -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [RFC PATCH] gdbstub: Re-factor gdb command extensions
Richard Henderson writes: > On 7/17/24 02:55, Alex Bennée wrote: >>> Are you expecting the same GdbCmdParseEntry object to be registered >>> multiple times? Can we fix that at a higher level? >> Its basically a hack to deal with the fact everything is tied to the >> CPUObject so we register everything multiple times. We could do a if >> (!registerd) register() dance but I guess I'm thinking forward to a >> hydrogenous future but I guess we'd need to do more work then anyway. > > Any chance we could move it all to the CPUClass? We would have to move quite a bunch of stuff, including the system register creation code. I don't think that's a re-factor I want to attempt quite so close to soft-freeze. > > > r~ -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [RFC PATCH] gdbstub: Re-factor gdb command extensions
Richard Henderson writes:
> On 7/16/24 21:42, Alex Bennée wrote:
>> void gdb_extend_qsupported_features(char *qsupported_features)
>> {
>> -/*
>> - * We don't support different sets of CPU gdb features on different
>> CPUs yet
>> - * so assert the feature strings are the same on all CPUs, or is set
>> only
>> - * once (1 CPU).
>> - */
>> -g_assert(extended_qsupported_features == NULL ||
>> - g_strcmp0(extended_qsupported_features, qsupported_features)
>> == 0);
>> -
>> -extended_qsupported_features = qsupported_features;
>> +if (!extended_qsupported_features) {
>> +extended_qsupported_features = g_strdup(qsupported_features);
>> +} else if (!g_strrstr(extended_qsupported_features,
>> qsupported_features)) {
>
> Did you really need the last instance of the substring?
Not really - I just want to check the string hasn't been added before.
>
> I'll note that g_strrstr is quite simplistic, whereas strstr has a
> much more scalable algorithm.
>
>
>> +char *old = extended_qsupported_features;
>> +extended_qsupported_features = g_strdup_printf("%s%s", old,
>> qsupported_features);
>
> Right tool for the right job, please: g_strconcat().
>
> That said, did you *really* want to concatenate now, and have to
> search through the middle, as opposed to storing N strings separately?
> You could defer the concat until the actual negotiation with gdb.
> That would reduce strstr above to a loop over strcmp.
>
>> +for (int i = 0; i < extensions->len; i++) {
>> +gpointer entry = g_ptr_array_index(extensions, i);
>> +if (!g_ptr_array_find(table, entry, NULL)) {
>> +g_ptr_array_add(table, entry);
>
> Are you expecting the same GdbCmdParseEntry object to be registered
> multiple times? Can we fix that at a higher level?
Its basically a hack to deal with the fact everything is tied to the
CPUObject so we register everything multiple times. We could do a if
(!registerd) register() dance but I guess I'm thinking forward to a
hydrogenous future but I guess we'd need to do more work then anyway.
>
>
> r~
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH] cpu: Free queued CPU work
Akihiko Odaki writes:
> Running qemu-system-aarch64 -M virt -nographic and terminating it will
> result in a LeakSanitizer error due to remaining queued CPU work so
> free it.
>
> Signed-off-by: Akihiko Odaki
FWIW this is likely the queued async task that
qemu_plugin_vcpu_init_hook sets up on the fake CPU -M virt creates at:
/*
* Instantiate a temporary CPU object to find out about what
* we are about to deal with. Once this is done, get rid of
* the object.
*/
cpuobj = object_new(possible_cpus->cpus[0].type);
armcpu = ARM_CPU(cpuobj);
pa_bits = arm_pamax(armcpu);
object_unref(cpuobj);
Anyway:
Tested-by: Alex Bennée
> ---
> include/hw/core/cpu.h | 6 ++
> cpu-common.c | 11 +++
> hw/core/cpu-common.c | 1 +
> 3 files changed, 18 insertions(+)
>
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index a2c8536943f7..8e6466c1ddab 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -1000,6 +1000,12 @@ void cpu_resume(CPUState *cpu);
> */
> void cpu_remove_sync(CPUState *cpu);
>
> +/**
> + * free_queued_cpu_work() - free all items on CPU work queue
> + * @cpu: The CPU which work queue to free.
> + */
> +void free_queued_cpu_work(CPUState *cpu);
> +
> /**
> * process_queued_cpu_work() - process all items on CPU work queue
> * @cpu: The CPU which work queue to process.
> diff --git a/cpu-common.c b/cpu-common.c
> index ce78273af597..7ae136f98ca7 100644
> --- a/cpu-common.c
> +++ b/cpu-common.c
> @@ -331,6 +331,17 @@ void async_safe_run_on_cpu(CPUState *cpu,
> run_on_cpu_func func,
> queue_work_on_cpu(cpu, wi);
> }
>
> +void free_queued_cpu_work(CPUState *cpu)
> +{
> +while (!QSIMPLEQ_EMPTY(>work_list)) {
> +struct qemu_work_item *wi = QSIMPLEQ_FIRST(>work_list);
> +QSIMPLEQ_REMOVE_HEAD(>work_list, node);
> +if (wi->free) {
> +g_free(wi);
> +}
> +}
> +}
> +
> void process_queued_cpu_work(CPUState *cpu)
> {
> struct qemu_work_item *wi;
> diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
> index b19e1fdacf22..d2e3e4570ab7 100644
> --- a/hw/core/cpu-common.c
> +++ b/hw/core/cpu-common.c
> @@ -281,6 +281,7 @@ static void cpu_common_finalize(Object *obj)
> g_free(cpu->plugin_state);
> }
> #endif
> +free_queued_cpu_work(cpu);
> g_array_free(cpu->gdb_regs, TRUE);
> qemu_lockcnt_destroy(>in_ioctl_lock);
> qemu_mutex_destroy(>work_mutex);
>
> ---
> base-commit: f2cb4026fccfe073f84a4b440e41d3ed0c3134f6
> change-id: 20240714-cpu-c4d28823b4c2
>
> Best regards,
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH v6 1/7] plugins: fix mem callback array size
Pierrick Bouvier writes: > data was correctly copied, but size of array was not set > (g_array_sized_new only reserves memory, but does not set size). > > As a result, callbacks were not called for code path relying on > plugin_register_vcpu_mem_cb(). > > Found when trying to trigger mem access callbacks for atomic > instructions. > > Reviewed-by: Xingtao Yao > Reviewed-by: Richard Henderson > Signed-off-by: Pierrick Bouvier I'm queuing this patch to plugins/next as it is a fix. -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [RFC PATCH] gdbstub: Re-factor gdb command extensions
Gustavo Romero writes: > Hi Alex, > > On 7/16/24 8:42 AM, Alex Bennée wrote: >> Coverity reported a memory leak (CID 1549757) in this code and its >> admittedly rather clumsy handling of extending the command table. >> Instead of handing over a full array of the commands lets use the >> lighter weight GPtrArray and simply test for the presence of each >> entry as we go. This avoids complications of transferring ownership of >> arrays and keeps the final command entries as static entries in the >> target code. > How did you run Coverity to find the leak? I'm wondering what's the > quickest way to check it next time. Coverity is only run in the cloud on the released build. There is a container somewhere but I don't know how its used. I did test on a build with --enable-sanitizers though. > > >> Cc: Akihiko Odaki >> Cc: Gustavo Bueno Romero >> Cc: Peter Maydell >> Signed-off-by: Alex Bennée > > Reviewed-by: Gustavo Romero > > > Cheers, > Gustavo -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH v4] plugins/stoptrigger: TCG plugin to stop execution under conditions
Simon Hamelin writes: > This new plugin allows to stop emulation using conditions on the > emulation state. By setting this plugin arguments, it is possible > to set an instruction count limit and/or trigger address(es) to stop at. > The code returned at emulation exit can be customized. > > This plugin demonstrates how someone could stop QEMU execution. > It could be used for research purposes to launch some code and > deterministically stop it and understand where its execution flow went. > > Co-authored-by: Alexandre Iooss > Signed-off-by: Simon Hamelin > Signed-off-by: Alexandre Iooss Queued to plugins/next, thanks. -- Alex Bennée Virtualisation Tech Lead @ Linaro
[RFC PATCH] gdbstub: Re-factor gdb command extensions
Coverity reported a memory leak (CID 1549757) in this code and its
admittedly rather clumsy handling of extending the command table.
Instead of handing over a full array of the commands lets use the
lighter weight GPtrArray and simply test for the presence of each
entry as we go. This avoids complications of transferring ownership of
arrays and keeps the final command entries as static entries in the
target code.
Cc: Akihiko Odaki
Cc: Gustavo Bueno Romero
Cc: Peter Maydell
Signed-off-by: Alex Bennée
---
include/gdbstub/commands.h | 19 --
target/arm/internals.h | 4 +-
gdbstub/gdbstub.c | 127 +
target/arm/gdbstub.c | 16 ++---
target/arm/gdbstub64.c | 11 ++--
5 files changed, 95 insertions(+), 82 deletions(-)
diff --git a/include/gdbstub/commands.h b/include/gdbstub/commands.h
index f3058f9dda..40f0514fe9 100644
--- a/include/gdbstub/commands.h
+++ b/include/gdbstub/commands.h
@@ -74,23 +74,28 @@ int gdb_put_packet(const char *buf);
/**
* gdb_extend_query_table() - Extend query table.
- * @table: The table with the additional query packet handlers.
- * @size: The number of handlers to be added.
+ * @table: GPtrArray of GdbCmdParseEntry entries.
+ *
+ * The caller should free @table afterwards
*/
-void gdb_extend_query_table(GdbCmdParseEntry *table, int size);
+void gdb_extend_query_table(GPtrArray *table);
/**
* gdb_extend_set_table() - Extend set table.
- * @table: The table with the additional set packet handlers.
- * @size: The number of handlers to be added.
+ * @table: GPtrArray of GdbCmdParseEntry entries.
+ *
+ * The caller should free @table afterwards
*/
-void gdb_extend_set_table(GdbCmdParseEntry *table, int size);
+void gdb_extend_set_table(GPtrArray *table);
/**
* gdb_extend_qsupported_features() - Extend the qSupported features string.
* @qsupported_features: The additional qSupported feature(s) string. The
string
* should start with a semicolon and, if there are more than one feature, the
- * features should be separate by a semiocolon.
+ * features should be separate by a semicolon.
+ *
+ * The caller should free @qsupported_features afterwards if
+ * dynamically allocated.
*/
void gdb_extend_qsupported_features(char *qsupported_features);
diff --git a/target/arm/internals.h b/target/arm/internals.h
index da22d04121..757b1fae92 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -359,8 +359,8 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu);
void arm_translate_init(void);
void arm_cpu_register_gdb_commands(ARMCPU *cpu);
-void aarch64_cpu_register_gdb_commands(ARMCPU *cpu, GString *, GArray *,
- GArray *);
+void aarch64_cpu_register_gdb_commands(ARMCPU *cpu, GString *,
+ GPtrArray *, GPtrArray *);
void arm_restore_state_to_opc(CPUState *cs,
const TranslationBlock *tb,
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index b9ad0a063e..fd7b4ecddb 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -1615,17 +1615,16 @@ static void handle_query_thread_extra(GArray *params,
void *user_ctx)
}
static char *extended_qsupported_features;
+
void gdb_extend_qsupported_features(char *qsupported_features)
{
-/*
- * We don't support different sets of CPU gdb features on different CPUs
yet
- * so assert the feature strings are the same on all CPUs, or is set only
- * once (1 CPU).
- */
-g_assert(extended_qsupported_features == NULL ||
- g_strcmp0(extended_qsupported_features, qsupported_features) ==
0);
-
-extended_qsupported_features = qsupported_features;
+if (!extended_qsupported_features) {
+extended_qsupported_features = g_strdup(qsupported_features);
+} else if (!g_strrstr(extended_qsupported_features, qsupported_features)) {
+char *old = extended_qsupported_features;
+extended_qsupported_features = g_strdup_printf("%s%s", old,
qsupported_features);
+g_free(old);
+}
}
static void handle_query_supported(GArray *params, void *user_ctx)
@@ -1753,39 +1752,59 @@ static const GdbCmdParseEntry
gdb_gen_query_set_common_table[] = {
},
};
-/* Compares if a set of command parsers is equal to another set of parsers. */
-static bool cmp_cmds(GdbCmdParseEntry *c, GdbCmdParseEntry *d, int size)
+/**
+ * extend_table() - extend one of the command tables
+ * @table: the command table to extend (or NULL)
+ * @extensions: a list of GdbCmdParseEntry pointers
+ *
+ * The entries themselves should be pointers to static const
+ * GdbCmdParseEntry entries. If the entry is already in the table we
+ * skip adding it again.
+ *
+ * Returns (a potentially freshly allocated) GPtrArray of GdbCmdParseEntry
+ */
+static GPtrArray * extend_table(GPtrArray *table, GPtrArray *extensions)
{
-for (int i = 0; i < size; i++) {
-if (!(c[i].handler ==
Re: [PATCH] target/arm: Free GDB command data
Peter Maydell writes:
> On Sun, 14 Jul 2024 at 11:43, Akihiko Odaki wrote:
>>
>> Signed-off-by: Akihiko Odaki
>> ---
>> target/arm/gdbstub.c | 9 ++---
>> 1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
>> index c3a9b5eb1ed2..03f77362efc1 100644
>> --- a/target/arm/gdbstub.c
>> +++ b/target/arm/gdbstub.c
>> @@ -477,11 +477,11 @@ static GDBFeature
>> *arm_gen_dynamic_m_secextreg_feature(CPUState *cs,
>>
>> void arm_cpu_register_gdb_commands(ARMCPU *cpu)
>> {
>> -GArray *query_table =
>> +g_autoptr(GArray) query_table =
>> g_array_new(FALSE, FALSE, sizeof(GdbCmdParseEntry));
>> -GArray *set_table =
>> +g_autoptr(GArray) set_table =
>> g_array_new(FALSE, FALSE, sizeof(GdbCmdParseEntry));
>> -GString *qsupported_features = g_string_new(NULL);
>> +g_autoptr(GString) qsupported_features = g_string_new(NULL);
>>
>> if (arm_feature(>env, ARM_FEATURE_AARCH64)) {
>> #ifdef TARGET_AARCH64
>> @@ -495,6 +495,7 @@ void arm_cpu_register_gdb_commands(ARMCPU *cpu)
>> gdb_extend_query_table(_array_index(query_table,
>>GdbCmdParseEntry, 0),
>>query_table->len);
>> +g_array_free(g_steal_pointer(_table), FALSE);
>> }
>>
>> /* Set arch-specific handlers for 'Q' commands. */
>> @@ -502,11 +503,13 @@ void arm_cpu_register_gdb_commands(ARMCPU *cpu)
>> gdb_extend_set_table(_array_index(set_table,
>> GdbCmdParseEntry, 0),
>> set_table->len);
>> +g_array_free(g_steal_pointer(_table), FALSE);
>> }
>>
>> /* Set arch-specific qSupported feature. */
>> if (qsupported_features->len) {
>> gdb_extend_qsupported_features(qsupported_features->str);
>> +g_string_free(g_steal_pointer(_features), FALSE);
>> }
>> }
>
> I don't think this is the right approach to this leak (which
> Coverity also complained about):
>
> https://lore.kernel.org/qemu-devel/CAFEAcA8YJwWtQxdOe2wmH7i0jvjU=uv92oeb6vuzt1grqhr...@mail.gmail.com/
>
> I think the underlying problem is that the gdb_extend_query_table
> and gdb_extend_set_table functions have a weird API where they
> retain pointers to a chunk of the contents of the GArray but
> they don't get passed the actual GArray. My take is that it
> would be better to make the API to those functions more natural
> (either "take the whole GArray and take ownership of it" or
> else "copy the info they need and the caller retains ownership
> of both the GArray and its contents").
>
> Also, there is a second leak here if you have more than one
> CPU -- when the second CPU calls gdb_extend_query_table() etc,
> the function will leak the first CPU's data. Having the function
> API be clearly either "always takes ownership" or "never takes
> ownership" would make it easier to fix this leak too.
I'm working on cleaning this API up to make it easier to use. I'll send
a patch once its tested.
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH 3/3] tests/tcg/aarch64: Add test cases for SME FMOPA (widening)
Richard Henderson writes: > From: Daniyal Khan > > Signed-off-by: Daniyal Khan > Message-Id: 172090222034.13953.1688870870882292209...@git.sr.ht > [rth: Split test cases to separate patch, tidy assembly.] > Signed-off-by: Richard Henderson Reviewed-by: Alex Bennée -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH 2/3] target/arm: Use FPST_F16 for SME FMOPA (widening)
Richard Henderson writes: > This operation has float16 inputs and thus must use > the FZ16 control not the FZ control. > > Cc: qemu-sta...@nongnu.org > Reported-by: Daniyal Khan > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2374 > Signed-off-by: Richard Henderson Reviewed-by: Alex Bennée -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH 1/3] target/arm: Use float_status copy in sme_fmopa_s
Richard Henderson writes:
> From: Daniyal Khan
>
> We made a copy above because the fp exception flags
> are not propagated back to the FPST register, but
> then failed to use the copy.
>
> Cc: qemu-sta...@nongnu.org
> Fixes: 558e956c719 ("target/arm: Implement FMOPA, FMOPS (non-widening)")
> Signed-off-by: Daniyal Khan
> [rth: Split from a larger patch]
> Signed-off-by: Richard Henderson
Reviewed-by: Alex Bennée
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH] meson: Use -fno-sanitize=function when available
Akihiko Odaki writes:
> Commit 23ef50ae2d0c (".gitlab-ci.d/buildtest.yml: Use
> -fno-sanitize=function in the clang-system job") adds
> -fno-sanitize=function for the CI but doesn't add the flag in the
> other context. Move it to meson.build.
>
> Signed-off-by: Akihiko Odaki
> ---
> meson.build| 1 +
> .gitlab-ci.d/buildtest.yml | 1 -
> 2 files changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/meson.build b/meson.build
> index 6a93da48e1b5..80447833f07a 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -609,6 +609,7 @@ if host_os != 'openbsd' and \
> endif
>
> qemu_common_flags += cc.get_supported_arguments(hardening_flags)
> +qemu_common_flags +=
> cc.get_supported_arguments('-fno-sanitize=function')
What about checking the other hardening flags?
>
> add_global_arguments(qemu_common_flags, native: false, language:
> all_languages)
> add_global_link_arguments(qemu_ldflags, native: false, language:
> all_languages)
> diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
> index e3a0758bd9e5..a57822d65182 100644
> --- a/.gitlab-ci.d/buildtest.yml
> +++ b/.gitlab-ci.d/buildtest.yml
> @@ -430,7 +430,6 @@ clang-system:
> IMAGE: fedora
> CONFIGURE_ARGS: --cc=clang --cxx=clang++
>--extra-cflags=-fsanitize=undefined
> --extra-cflags=-fno-sanitize-recover=undefined
> - --extra-cflags=-fno-sanitize=function
> TARGETS: alpha-softmmu arm-softmmu m68k-softmmu mips64-softmmu
> s390x-softmmu
> MAKE_CHECK_ARGS: check-qtest check-tcg
>
>
> ---
> base-commit: f2cb4026fccfe073f84a4b440e41d3ed0c3134f6
> change-id: 20240714-function-7d32c723abbc
>
> Best regards,
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH] accel/tcg: Make cpu_exec_interrupt hook mandatory
Peter Maydell writes: > The TCGCPUOps::cpu_exec_interrupt hook is currently not mandatory; if > it is left NULL then we treat it as if it had returned false. However > since pretty much every architecture needs to handle interrupts, > almost every target we have provides the hook. The one exception is > Tricore, which doesn't currently implement the architectural > interrupt handling. > > Add a "do nothing" implementation of cpu_exec_hook for Tricore, > assert on startup that the CPU does provide the hook, and remove > the runtime NULL check before calling it. > > Signed-off-by: Peter Maydell Reviewed-by: Alex Bennée -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH v6 7/7] tests/tcg/x86_64: add test for plugin memory access
Pierrick Bouvier writes:
> On 7/8/24 12:15, Alex Bennée wrote:
>> Pierrick Bouvier writes:
>>
>>> Add an explicit test to check expected memory values are read/written.
>>> For sizes 8, 16, 32, 64 and 128, we generate a load/store operation.
>>> For size 8 -> 64, we generate an atomic __sync_val_compare_and_swap too.
>>> For 128bits memory access, we rely on SSE2 instructions.
>>>
>>> By default, atomic accesses are non atomic if a single cpu is running,
>>> so we force creation of a second one by creating a new thread first.
>>>
>>> load/store helpers code path can't be triggered easily in user mode (no
>>> softmmu), so we can't test it here.
>>>
>>> Can be run with:
>>> make -C build/tests/tcg/x86_64-linux-user
>>> run-plugin-test-plugin-mem-access-with-libmem.so
>>>
>>> Tested-by: Xingtao Yao
>>> Signed-off-by: Pierrick Bouvier
>>> ---
>>> tests/tcg/x86_64/test-plugin-mem-access.c | 89 +
>>> tests/tcg/x86_64/Makefile.target| 7 ++
>>> tests/tcg/x86_64/check-plugin-mem-access.sh | 48 +++
>>> 3 files changed, 144 insertions(+)
>>> create mode 100644 tests/tcg/x86_64/test-plugin-mem-access.c
>>> create mode 100755 tests/tcg/x86_64/check-plugin-mem-access.sh
>>>
>>> diff --git a/tests/tcg/x86_64/test-plugin-mem-access.c
>>> b/tests/tcg/x86_64/test-plugin-mem-access.c
>>> new file mode 100644
>>> index 000..7fdd6a55829
>>> --- /dev/null
>>> +++ b/tests/tcg/x86_64/test-plugin-mem-access.c
>>> @@ -0,0 +1,89 @@
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +
>>> +static void *data;
>>> +
>>> +#define DEFINE_STORE(name, type, value) \
>>> +static void store_##name(void) \
>>> +{ \
>>> +*((type *)data) = value;\
>>> +}
>>> +
>>> +#define DEFINE_ATOMIC_OP(name, type, value) \
>>> +static void atomic_op_##name(void) \
>>> +{ \
>>> +*((type *)data) = 0x42; \
>>> +__sync_val_compare_and_swap((type *)data, 0x42, value); \
>> Should we exercise the other compare and swap ops? Do they all come
>> through the same rwm path?
>>
>
> There are definitely several paths depending on the generated atomic op.
> However, the code is pretty straightforward (and implemented in a
> single function), so my thought was that one way to trigger this was
> enough.
If they all come through the same path I guess that's OK.
>>> +}
>>> +
>>> +#define DEFINE_LOAD(name, type) \
>>> +static void load_##name(void) \
>>> +{ \
>>> +register type var asm("eax") = *((type *) data);\
>>> +(void)var; \
>> This is a bit weird. It's the only inline asm needed that makes this
>> a
>> non-multiarch test. Why?
>>
>
> I'll answer here about why this test is arch specific, and not a multi arch.
>
> The problem I met is that all target architecture do not have native
> 64/128 bits types, and depending how code is compiled with gcc, you
> may or not generated expected vector instructions for 128bits
> operation. Same for atomic operations.
So we do handle this with the sha512 test, building variants of it with
various compiler flags to trigger the use of vectors. So the code is
multiarch but we have arch specific variants as dictated by the
Makefiles, i.e.:
sha512-sve: CFLAGS=-O3 -march=armv8.1-a+sve
sha512-sve: sha512.c
$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
TESTS += sha512-sve
> Thus, I chose to specialize this test for x86_64, and use sse2
> directly for 128 bits integers.
>
> You might say "How about adding ifdef for this". Yes sure, but the
> check script would become complicated too, and I just wanted to keep
> it simple.
We can keep the check-script per arch if we have to.
> Our interest here is not to check that memory accesses are
> correctly implemented in QEMU, but to check that a specific behavior
> on one arch is the one expected.
So the problem with not being multiarch is we don't build all targets in
all combinations. To limit CI time we often build a subset and now this
particular subset won't test the plugin paths.
>
> Does it make more sense for you?
>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [RFC PATCH 0/8] Convert avocado tests to normal Python unittests
Daniel P. Berrangé writes: > On Thu, Jul 11, 2024 at 07:44:39PM +0200, Thomas Huth wrote: >> On 11/07/2024 16.39, Fabiano Rosas wrote: >> > Thomas Huth writes: >> ... >> > > Things that need further attention though: >> > > >> > > - All tests that use the LinuxTest / LinuxDistro classes (e.g. based >> > >on cloud-init images) really depend on the Avocado framework, >> > >thus we'd need a solution for those if we want to continue with >> > >this approach >> > > >> > > - Same for all tests that require the LinuxSSHMixIn class - we'd >> > >need to provide a solution for ssh-based tests, too. >> > >> > These two seem to be dependent mostly avocado/utils only. Those could >> > still be used without the whole framework, no? Say we keep importing >> > avocado.utils, but run everything from meson, would that make sense? >> >> Yes ... maybe ... I can give it a try to see whether that works. > > We only import about 8 modules from avocado.utils. There are probably a > few more indirectly used, but worst case we just clone the bits we need > into the QEMU tree. Is Avocado still actively developed? I thought you guys used it quite widely within RedHat? > > With regards, > Daniel -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [RFC PATCH] build deps: update lcitool to include rust bits
Manos Pitsidianakis writes: > Hi Daniel, Alex, > > I will pick this patch up with all the reviewed-by trailers for my > next Rust RFC series if that's alright with you, Absolutely - I made it for you ;-) -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [RFC PATCH 5/8] tests_pytest: Implement fetch_asset() method for downloading assets
Richard Henderson writes:
> On 7/11/24 09:45, Richard Henderson wrote:
>> On 7/11/24 04:55, Thomas Huth wrote:
>>> + def fetch_asset(self, url, asset_hash):
>>> + cache_dir = os.path.expanduser("~/.cache/qemu/download")
>>> + if not os.path.exists(cache_dir):
>>> + os.makedirs(cache_dir)
>>> + fname = os.path.join(cache_dir,
>>> + hashlib.sha1(url.encode("utf-8")).hexdigest())
>>> + if os.path.exists(fname) and self.check_hash(fname, asset_hash):
>>> + return fname
>>> + logging.debug("Downloading %s to %s...", url, fname)
>>> + subprocess.check_call(["wget", "-c", url, "-O", fname +
>>> ".download"])
>>> + os.rename(fname + ".download", fname)
>>> + return fname
>> Download failure via exception?
>> Check hash on downloaded asset?
>
> I would prefer to see assets, particularly downloading, handled in a
> separate pass from tests.
And I assume cachable?
>
> (1) Asset download should not count against test timeout.
> (2) Running tests while disconnected should skip unavailable assets.
>
> Avocado kinda does this, but still generates errors instead of skips.
>
>
> r~
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH] tests/avocado: Remove non-working sparc leon3 test
Thomas Huth writes: > The test has been marked as broken more than 4 years ago, and > so far nobody ever cared to fix it. Thus let's simply remove it > now ... if somebody ever needs it again, they can restore the > file from an older version of QEMU. > > Signed-off-by: Thomas Huth Queued to testing/next, thanks. -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH] MAINTAINERS: add Edgar as Xen maintainer
Stefano Stabellini writes: > Add Edgar as Xen subsystem maintainer in QEMU. Edgar has been a QEMU > maintainer for years, and has already made key changes to one of the > most difficult areas of the Xen subsystem (the mapcache). > > Edgar volunteered helping us maintain the Xen subsystem in QEMU and we > are very happy to welcome him to the team. His knowledge and expertise > with QEMU internals will be of great help. > > Signed-off-by: Stefano Stabellini Reviewed-by: Alex Bennée -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH v2] plugins/stoptrigger: TCG plugin to stop execution under conditions
sn_vaddr = qemu_plugin_insn_vaddr(insn);
might as well just cast it once:
gpointer insn_vaddr = GUINT_TO_POINTER(qemu_plugin_insn_vaddr(insn));
> +
> +if (exit_on_icount) {
> +/* Increment and check scoreboard for each instruction */
> +qemu_plugin_register_vcpu_insn_exec_inline_per_vcpu(
> +insn, QEMU_PLUGIN_INLINE_ADD_U64, insn_count, 1);
> +qemu_plugin_register_vcpu_insn_exec_cond_cb(
> +insn, exit_icount_reached, QEMU_PLUGIN_CB_NO_REGS,
> +QEMU_PLUGIN_COND_EQ, insn_count, icount + 1, NULL);
Might be useful to report the PC at which you reach the icount boundary
> +}
> +
> +if (exit_on_address) {
> +g_mutex_lock(_ht_lock);
> +if (g_hash_table_contains(addrs_ht,
> GUINT_TO_POINTER(insn_vaddr))) {
> +/* Exit triggered by address */
> +qemu_plugin_register_vcpu_insn_exec_cb(
> +insn, exit_address_reached, QEMU_PLUGIN_CB_NO_REGS,
> +GUINT_TO_POINTER(insn_vaddr));
> +}
> +g_mutex_unlock(_ht_lock);
> +}
> +}
> +}
> +
> +static void plugin_exit(qemu_plugin_id_t id, void *p)
> +{
> +g_hash_table_destroy(addrs_ht);
> +qemu_plugin_scoreboard_free(insn_count_sb);
> +}
> +
> +QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
> + const qemu_info_t *info, int argc,
> + char **argv)
> +{
> +addrs_ht = g_hash_table_new(NULL, g_direct_equal);
> +
> +insn_count_sb = qemu_plugin_scoreboard_new(sizeof(InstructionsCount));
> +insn_count = qemu_plugin_scoreboard_u64_in_struct(
> +insn_count_sb, InstructionsCount, insn_count);
> +
> +for (int i = 0; i < argc; i++) {
> +char *opt = argv[i];
> +g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
> +if (g_strcmp0(tokens[0], "icount") == 0) {
> +g_auto(GStrv) icount_tokens = g_strsplit(tokens[1], ":", 2);
> +icount = g_ascii_strtoull(icount_tokens[0], NULL, 0);
> +if (icount < 1 || g_strrstr(icount_tokens[0], "-") !=
> NULL) {
I don't think strstoull would even parse something with - in it so I
would just do:
if (icount == 0) {
/* fail */
}
> +fprintf(stderr,
> +"icount parsing failed: '%s' must be a positive "
> +"integer\n",
> +icount_tokens[0]);
> +return -1;
> +}
> +if (icount_tokens[1]) {
> +icount_exit_code = g_ascii_strtoull(icount_tokens[1], NULL,
> 0);
> +}
> +exit_on_icount = true;
> +} else if (g_strcmp0(tokens[0], "addr") == 0) {
> +g_auto(GStrv) addr_tokens = g_strsplit(tokens[1], ":", 2);
> +uint64_t exit_addr = g_ascii_strtoull(addr_tokens[0], NULL, 0);
> +int exit_code = 0;
> +if (addr_tokens[1]) {
> +exit_code = g_ascii_strtoull(addr_tokens[1], NULL, 0);
> +}
> +g_mutex_lock(_ht_lock);
> +g_hash_table_insert(addrs_ht, GUINT_TO_POINTER(exit_addr),
> +GINT_TO_POINTER(exit_code));
> +g_mutex_unlock(_ht_lock);
> +exit_on_address = true;
> +} else {
> +fprintf(stderr, "option parsing failed: %s\n", opt);
> +return -1;
> +}
> +}
> +
> +if (!exit_on_icount && !exit_on_address) {
> +fprintf(stderr, "'icount' or 'addr' argument missing\n");
> +return -1;
> +}
> +
> +/* Register translation block and exit callbacks */
> +qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
> +qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
> +
> +return 0;
> +}
> diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
> index f7d7b9e3a4..954623f9bf 100644
> --- a/docs/devel/tcg-plugins.rst
> +++ b/docs/devel/tcg-plugins.rst
> @@ -642,6 +642,28 @@ The plugin has a number of arguments, all of them are
> optional:
>configuration arguments implies ``l2=on``.
>(default: N = 2097152 (2MB), B = 64, A = 16)
>
> +- contrib/plugins/stoptrigger.c
> +
> +The stoptrigger plugin allows to setup triggers to stop emulation.
> +It can be used for research purposes to launch some code and precisely stop
> it
> +and understand where its execution flow went.
> +
> +Two types of triggers can be configured: a count of instructions to stop at,
> +or an address to stop at. Multiple triggers can be set at once.
> +
> +By default, QEMU will exit with return code 0. A custom return code can be
> +configured for each trigger using ``:CODE`` syntax.
> +
> +For example, to stop at the 20-th instruction with return code 41, at address
> +0xd4 with return code 0 or at address 0xd8 with return code 42::
> +
> + $ qemu-system-aarch64 $(QEMU_ARGS) \
> +-plugin
> ./contrib/plugins/libstoptrigger.so,icount=20:41,addr=0xd4,addr=0xd8:42 -d
> plugin
> +
> +The plugin will log the reason of exit, for example::
> +
> + 0xd4 reached, exiting
> +
> Plugin API
> ==
Otherwise it looks good to me. Unless you want to tackle additional exit
modes?
What is your current use case for this?
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
[RFC PATCH] build deps: update lcitool to include rust bits
For rust development we need cargo, rustc and bindgen in our various development environments. Update the libvirt-ci project to (!495) and regenerate the containers and other dependency lists. Signed-off-by: Alex Bennée --- NB: - this is currently waiting on the upstream MR, but if you manually add the remote https://gitlab.com/stsquad/libvirt-ci/-/tree/more-rust-mappings the submodule update will work. --- .gitlab-ci.d/cirrus/freebsd-13.vars | 2 +- .gitlab-ci.d/cirrus/macos-13.vars | 2 +- .gitlab-ci.d/cirrus/macos-14.vars | 2 +- scripts/ci/setup/ubuntu/ubuntu-2204-aarch64.yaml | 3 +++ scripts/ci/setup/ubuntu/ubuntu-2204-s390x.yaml| 3 +++ tests/docker/dockerfiles/alpine.docker| 3 +++ tests/docker/dockerfiles/centos9.docker | 3 +++ tests/docker/dockerfiles/debian-amd64-cross.docker| 4 tests/docker/dockerfiles/debian-arm64-cross.docker| 4 tests/docker/dockerfiles/debian-armel-cross.docker| 4 tests/docker/dockerfiles/debian-armhf-cross.docker| 4 tests/docker/dockerfiles/debian-i686-cross.docker | 4 tests/docker/dockerfiles/debian-mips64el-cross.docker | 4 tests/docker/dockerfiles/debian-mipsel-cross.docker | 4 tests/docker/dockerfiles/debian-ppc64el-cross.docker | 4 tests/docker/dockerfiles/debian-s390x-cross.docker| 4 tests/docker/dockerfiles/debian.docker| 3 +++ tests/docker/dockerfiles/fedora-win64-cross.docker| 3 +++ tests/docker/dockerfiles/fedora.docker| 3 +++ tests/docker/dockerfiles/opensuse-leap.docker | 2 ++ tests/docker/dockerfiles/ubuntu2204.docker| 3 +++ tests/lcitool/libvirt-ci | 2 +- tests/lcitool/projects/qemu.yml | 3 +++ tests/vm/generated/freebsd.json | 2 ++ 24 files changed, 71 insertions(+), 4 deletions(-) diff --git a/.gitlab-ci.d/cirrus/freebsd-13.vars b/.gitlab-ci.d/cirrus/freebsd-13.vars index 3785afca36..8c3b02d089 100644 --- a/.gitlab-ci.d/cirrus/freebsd-13.vars +++ b/.gitlab-ci.d/cirrus/freebsd-13.vars @@ -11,6 +11,6 @@ MAKE='/usr/local/bin/gmake' NINJA='/usr/local/bin/ninja' PACKAGING_COMMAND='pkg' PIP3='/usr/local/bin/pip-3.8' -PKGS='alsa-lib bash bison bzip2 ca_root_nss capstone4 ccache cmocka ctags curl cyrus-sasl dbus diffutils dtc flex fusefs-libs3 gettext git glib gmake gnutls gsed gtk3 json-c libepoxy libffi libgcrypt libjpeg-turbo libnfs libslirp libspice-server libssh libtasn1 llvm lzo2 meson mtools ncurses nettle ninja opencv pixman pkgconf png py39-numpy py39-pillow py39-pip py39-sphinx py39-sphinx_rtd_theme py39-tomli py39-yaml python3 rpm2cpio sdl2 sdl2_image snappy sndio socat spice-protocol tesseract usbredir virglrenderer vte3 xorriso zstd' +PKGS='alsa-lib bash bison bzip2 ca_root_nss capstone4 ccache cmocka ctags curl cyrus-sasl dbus diffutils dtc flex fusefs-libs3 gettext git glib gmake gnutls gsed gtk3 json-c libepoxy libffi libgcrypt libjpeg-turbo libnfs libslirp libspice-server libssh libtasn1 llvm lzo2 meson mtools ncurses nettle ninja opencv pixman pkgconf png py39-numpy py39-pillow py39-pip py39-sphinx py39-sphinx_rtd_theme py39-tomli py39-yaml python3 rpm2cpio rust rust-bindgen-cli sdl2 sdl2_image snappy sndio socat spice-protocol tesseract usbredir virglrenderer vte3 xorriso zstd' PYPI_PKGS='' PYTHON='/usr/local/bin/python3' diff --git a/.gitlab-ci.d/cirrus/macos-13.vars b/.gitlab-ci.d/cirrus/macos-13.vars index 534f029956..3c8ba1a277 100644 --- a/.gitlab-ci.d/cirrus/macos-13.vars +++ b/.gitlab-ci.d/cirrus/macos-13.vars @@ -11,6 +11,6 @@ MAKE='/opt/homebrew/bin/gmake' NINJA='/opt/homebrew/bin/ninja' PACKAGING_COMMAND='brew' PIP3='/opt/homebrew/bin/pip3' -PKGS='bash bc bison bzip2 capstone ccache cmocka ctags curl dbus diffutils dtc flex gcovr gettext git glib gnu-sed gnutls gtk+3 jemalloc jpeg-turbo json-c libepoxy libffi libgcrypt libiscsi libnfs libpng libslirp libssh libtasn1 libusb llvm lzo make meson mtools ncurses nettle ninja pixman pkg-config python3 rpm2cpio sdl2 sdl2_image snappy socat sparse spice-protocol swtpm tesseract usbredir vde vte3 xorriso zlib zstd' +PKGS='bash bc bindgen bison bzip2 capstone ccache cmocka ctags curl dbus diffutils dtc flex gcovr gettext git glib gnu-sed gnutls gtk+3 jemalloc jpeg-turbo json-c libepoxy libffi libgcrypt libiscsi libnfs libpng libslirp libssh libtasn1 libusb llvm lzo make meson mtools ncurses nettle ninja pixman pkg-config python3 rpm2cpio rust sdl2 sdl2_image snappy socat sparse spice-protocol swtpm tesseract usbredir vde vte3 xorriso zlib zstd' PYPI_PKGS='PyYAML numpy pillow sphinx sphinx-rtd-theme tomli' PYTHON='/opt/homebrew/bin/python3' diff --git a/.gitlab-ci.d/cirrus/macos-14.vars b/.gitlab-ci.d/cirrus/macos-14.vars index 43070f4a26..d227c5deca 100644 --- a/.gitlab-ci.d/cirrus/macos-14.vars +++ b/.gitlab-ci.d/cirrus/macos-14
Re: [PATCH] tests/avocado: Remove non-working sparc leon3 test
Thomas Huth writes: > The test has been marked as broken more than 4 years ago, and > so far nobody ever cared to fix it. Thus let's simply remove it > now ... if somebody ever needs it again, they can restore the > file from an older version of QEMU. > > Signed-off-by: Thomas Huth Acked-by: Alex Bennée -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH] plugins/stoptrigger: TCG plugin to stop execution under conditions
> +
> +for (int i = 0; i < argc; i++) {
> +char *opt = argv[i];
> +g_auto(GStrv) tokens = g_strsplit(opt, "=", 2);
> +if (g_strcmp0(tokens[0], "icount") == 0) {
> +g_auto(GStrv) icount_tokens = g_strsplit(tokens[1], ":", 2);
> +icount = g_ascii_strtoull(icount_tokens[0], NULL, 0);
> +if (icount < 1 || g_strrstr(icount_tokens[0], "-") != NULL) {
> +fprintf(stderr, "icount parsing failed: '%s' must be a "
> +"positive integer\n", icount_tokens[0]);
> +return -1;
> +}
> +if (icount_tokens[1]) {
> +icount_exit_code = g_ascii_strtoull(icount_tokens[1], NULL,
> 0);
> +}
> +exit_on_icount = true;
> +} else if (g_strcmp0(tokens[0], "addr") == 0) {
> +g_auto(GStrv) addr_tokens = g_strsplit(tokens[1], ":", 2);
> +uint64_t exit_addr = g_ascii_strtoull(addr_tokens[0], NULL, 0);
> +int exit_code = 0;
> +if (addr_tokens[1]) {
> +exit_code = g_ascii_strtoull(addr_tokens[1], NULL, 0);
> +}
> +g_mutex_lock(_ht_lock);
> +g_hash_table_insert(addrs_ht, GUINT_TO_POINTER(exit_addr),
> +GINT_TO_POINTER(exit_code));
> +g_mutex_unlock(_ht_lock);
> +exit_on_address = true;
> +} else {
> +fprintf(stderr, "option parsing failed: %s\n", opt);
> +return -1;
> +}
> +}
> +
> +if (!exit_on_icount && !exit_on_address) {
> +fprintf(stderr, "'icount' or 'addr' argument missing\n");
> +return -1;
> +}
> +
> +/* Register translation block and exit callbacks */
> +qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
> +qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
> +
> +return 0;
> +}
> diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
> index f7d7b9e3a4..954623f9bf 100644
> --- a/docs/devel/tcg-plugins.rst
> +++ b/docs/devel/tcg-plugins.rst
> @@ -642,6 +642,28 @@ The plugin has a number of arguments, all of them are
> optional:
>configuration arguments implies ``l2=on``.
>(default: N = 2097152 (2MB), B = 64, A = 16)
>
> +- contrib/plugins/stoptrigger.c
> +
> +The stoptrigger plugin allows to setup triggers to stop emulation.
> +It can be used for research purposes to launch some code and precisely stop
> it
> +and understand where its execution flow went.
> +
> +Two types of triggers can be configured: a count of instructions to stop at,
> +or an address to stop at. Multiple triggers can be set at once.
> +
> +By default, QEMU will exit with return code 0. A custom return code can be
> +configured for each trigger using ``:CODE`` syntax.
So I can see this being useful for general testing of "did the code get
to this point". However would it be worth considering other cases like
simply stopping the VM or triggering a gdbstub exception?
> +
> +For example, to stop at the 20-th instruction with return code 41, at address
> +0xd4 with return code 0 or at address 0xd8 with return code 42::
> +
> + $ qemu-system-aarch64 $(QEMU_ARGS) \
> +-plugin
> ./contrib/plugins/libstoptrigger.so,icount=20:41,addr=0xd4,addr=0xd8:42 -d
> plugin
> +
> +The plugin will log the reason of exit, for example::
> +
> + 0xd4 reached, exiting
> +
> Plugin API
> ==
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [RFC PATCH v4 2/7] rust: add bindgen step as a meson dependency
Manos Pitsidianakis writes:
> Add mechanism to generate rust hw targets that depend on a custom
> bindgen target for rust bindings to C.
>
> + # We need one bindings_rs build target per arch target, so give them
> + # arch-specific names.
> + copy = fs.copyfile('rust/wrapper.h',
> + target + '_wrapper.h')
> + bindings_rs = rust_bindgen.bindgen(
> +input: copy,
> +dependencies: arch_deps + lib_deps,
> +output: 'bindings-' + target + '.rs',
> +include_directories: include_directories('.', 'include'),
> +args: [
> + '--ctypes-prefix', 'core::ffi',
> + '--formatter', 'rustfmt',
I guess we also need to detect rustfmt, although it seems to be
non-fatal:
15:59:27 [alex@debian-arm64:~/l/q/b/rust] review/rust-pl011-v4 + ninja
[831/3041] Generating bindings for Rust
rustmod-bindgen-aarch64-softmmu_wrapper.h
Failed to run rustfmt: cannot find binary path (non-fatal, continuing)
[3041/3041] Linking target tests/qtest/qos-test
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [RFC PATCH v4 5/7] .gitattributes: add Rust diff and merge attributes
Manos Pitsidianakis writes: > Set rust source code to diff=rust (built-in with new git versions) > and merge=binary for Cargo.lock files (they should not be merged but > auto-generated by cargo) > > Signed-off-by: Manos Pitsidianakis Reviewed-by: Alex Bennée -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [RFC PATCH v4 2/7] rust: add bindgen step as a meson dependency
Alex Bennée writes:
> Manos Pitsidianakis writes:
>
>> Add mechanism to generate rust hw targets that depend on a custom
>> bindgen target for rust bindings to C.
>>
>> This way bindings will be created before the rust crate is compiled.
>>
>> The bindings will end up in BUILDDIR/{target}-generated.rs and have the same
>> name
>> as a target:
>>
>> ninja aarch64-softmmu-generated.rs
>>
>> The way the bindings are generated is:
>>
>> 1. All required C headers are included in a single file, in our case
>>rust/wrapper.h for convenience. Otherwise we'd have to provide a list
>>of headers every time to the bindgen tool.
>>
>> 2. Meson creates a generated_rs target that runs bindgen making sure
>>the architecture etc header dependencies are present.
>>
>> 3. The generated_rs target takes a list of files, type symbols,
>>function symbols to block from being generated. This is not necessary
>>for the bindings to work, but saves us time and space.
>>
>> 4. Meson creates rust hardware target dependencies from the rust_targets
>>dictionary defined in rust/meson.build.
>>
>>Since we cannot declare a dependency on generated_rs before it is
>>declared in meson.build, the rust crate targets must be defined after
>>the generated_rs target for each target architecture is defined. This
>>way meson sets up the dependency tree properly.
>>
>> 5. After compiling each rust crate with the cargo_wrapper.py script,
>>its static library artifact is linked as a `whole-archive` with the
>>final binary.
>>
>> Signed-off-by: Manos Pitsidianakis
>
>> +
>> +msrv = {
>> + 'rustc': '1.77.2',
>> + 'cargo': '1.77.2',
>> + 'bindgen': '0.69.4',
>> +}
>
> This is still pretty bleeding edge (it even tripped up on the
> .cargo/bin/cargo I have installed). This needs to be set to the
> baseline which from:
>
> https://wiki.qemu.org/RustInQemu/2022
>
> Looks to be 1.24.0 for rustc and I guess even lower for cargo (Debian
> says 0.66.0). While it might make sense to delay merging if we are
> waiting for one distro to produce a new LTS we shouldn't be needing
> rustup by default.
Also bindgen, do we need such a new one? On Trixie:
Message:
../../rust/meson.build:41:4: ERROR: Problem encountered: bindgen version
0.66.1 is unsupported: Please upgrade to at least 0.69.4
A full log can be found at
/home/alex/lsrc/qemu.git/builds/rust/meson-logs/meson-log.txt
ERROR: meson setup failed
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [RFC PATCH v4 2/7] rust: add bindgen step as a meson dependency
Paolo Bonzini writes: > On Tue, Jul 9, 2024 at 2:09 PM Peter Maydell wrote: >> * what is the actual baseline requirement? We definitely want >>to support "using rustup on an older system" (should be no >>problem) and "current distro building QEMU using the distro's >>rust", I assume. It would certainly be nice to have "building >>QEMU on the older-but-still-in-our-support-list distro releases >>with that distro's rust", but this probably implies not just >>a minimum rust version but also a limited set of crates. > > I don't think limiting ourselves to the set of crates in the distro is > feasible. It's not the way the language works, for example I tried > checking if the "cstr" crate exists and I didn't find it in Debian. > > We can define a known-good set of versions: > * either via Cargo.lock (i.e. recording the versions and downloading > them) or by including sources in the tree > * at any time in qemu.git, at branching time (three times a year), or > on every release > > (That's six possibilities overall). > >>I think (but forget the details) that some of what we've done >>with Python where we accept that the older-but-still-supported >>distro will end up taking the "download at build time" path >>in the build system might apply also for Rust. > > Yes, and --without-download may be changed to the "--frozen" flag of cargo. > >> * what, on the Rust side, is the version requirement? Presumably >>there's some features that are pretty much "we really need >>this and can't do without it" and some which are "this would >>be pretty awkward not to have but if we have to we can implement >>some kind of fallback/alternative with a TODO note to come >>back and clean up when our baseline moves forwards". > > Here are the stopping points that I found over the last couple weeks: > > 1.56.0: 2021 edition > 1.59.0: const CStr::from_bytes_with_nul_unchecked (needed by cstr > crate, see below) > 1.64.0: std::ffi::c_char > 1.65.0: Generic Associated Types > 1.74.0: Clippy can be configured in Cargo.toml > 1.77.0: C string literals, offset_of! > > I think 1.59.0 is pretty much the lower bound. Not having offset_of! > will be a bit painful, but it can be worked around (and pretty much > has to be, because 1.77.0 is really new). > > As far as I understand, Debian bullseye has 1.63.0 these days and it's > the oldest platform we have. I was going to say Bookworm has now superseded Bullseye which will reach its release + 3 year support point in August. However the version you mention in the Bookworm one! > > Paolo > >> At that point we have more information to figure out what >> if any tradeoff we want to make. >> >> thanks >> -- PMM >> -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [RFC PATCH v4 2/7] rust: add bindgen step as a meson dependency
Manos Pitsidianakis writes:
> Add mechanism to generate rust hw targets that depend on a custom
> bindgen target for rust bindings to C.
>
> This way bindings will be created before the rust crate is compiled.
>
> The bindings will end up in BUILDDIR/{target}-generated.rs and have the same
> name
> as a target:
>
> ninja aarch64-softmmu-generated.rs
>
> The way the bindings are generated is:
>
> 1. All required C headers are included in a single file, in our case
>rust/wrapper.h for convenience. Otherwise we'd have to provide a list
>of headers every time to the bindgen tool.
>
> 2. Meson creates a generated_rs target that runs bindgen making sure
>the architecture etc header dependencies are present.
>
> 3. The generated_rs target takes a list of files, type symbols,
>function symbols to block from being generated. This is not necessary
>for the bindings to work, but saves us time and space.
>
> 4. Meson creates rust hardware target dependencies from the rust_targets
>dictionary defined in rust/meson.build.
>
>Since we cannot declare a dependency on generated_rs before it is
>declared in meson.build, the rust crate targets must be defined after
>the generated_rs target for each target architecture is defined. This
>way meson sets up the dependency tree properly.
>
> 5. After compiling each rust crate with the cargo_wrapper.py script,
>its static library artifact is linked as a `whole-archive` with the
>final binary.
>
> Signed-off-by: Manos Pitsidianakis
> +
> +msrv = {
> + 'rustc': '1.77.2',
> + 'cargo': '1.77.2',
> + 'bindgen': '0.69.4',
> +}
This is still pretty bleeding edge (it even tripped up on the
.cargo/bin/cargo I have installed). This needs to be set to the
baseline which from:
https://wiki.qemu.org/RustInQemu/2022
Looks to be 1.24.0 for rustc and I guess even lower for cargo (Debian
says 0.66.0). While it might make sense to delay merging if we are
waiting for one distro to produce a new LTS we shouldn't be needing
rustup by default.
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH v6 7/7] tests/tcg/x86_64: add test for plugin memory access
+run-plugin-test-plugin-mem-access-with-libmem.so: \
> + PLUGIN_ARGS=$(COMMA)print-accesses=true
> +run-plugin-test-plugin-mem-access-with-libmem.so: \
> + CHECK_PLUGIN_OUTPUT_COMMAND= \
> + $(SRC_PATH)/tests/tcg/x86_64/check-plugin-mem-access.sh
> +
> test-x86_64: LDFLAGS+=-lm -lc
> test-x86_64: test-i386.c test-i386.h test-i386-shift.h test-i386-muldiv.h
> $(CC) $(CFLAGS) $< -o $@ $(LDFLAGS)
> diff --git a/tests/tcg/x86_64/check-plugin-mem-access.sh
> b/tests/tcg/x86_64/check-plugin-mem-access.sh
> new file mode 100755
> index 000..163f1cfad34
> --- /dev/null
> +++ b/tests/tcg/x86_64/check-plugin-mem-access.sh
> @@ -0,0 +1,48 @@
> +#!/usr/bin/env bash
> +
> +set -euo pipefail
> +
> +die()
> +{
> +echo "$@" 1>&2
> +exit 1
> +}
> +
> +check()
> +{
> +file=$1
> +pattern=$2
> +grep "$pattern" "$file" > /dev/null || die "\"$pattern\" not found in
> $file"
> +}
> +
> +[ $# -eq 1 ] || die "usage: plugin_out_file"
> +
> +plugin_out=$1
> +
> +expected()
> +{
> +cat << EOF
> +,store_u8,.*,8,store,0xf1
> +,atomic_op_u8,.*,8,load,0x42
> +,atomic_op_u8,.*,8,store,0xf1
> +,load_u8,.*,8,load,0xf1
> +,store_u16,.*,16,store,0xf123
> +,atomic_op_u16,.*,16,load,0x0042
> +,atomic_op_u16,.*,16,store,0xf123
> +,load_u16,.*,16,load,0xf123
> +,store_u32,.*,32,store,0xff112233
> +,atomic_op_u32,.*,32,load,0x0042
> +,atomic_op_u32,.*,32,store,0xff112233
> +,load_u32,.*,32,load,0xff112233
> +,store_u64,.*,64,store,0xf123456789abcdef
> +,atomic_op_u64,.*,64,load,0x0042
> +,atomic_op_u64,.*,64,store,0xf123456789abcdef
> +,load_u64,.*,64,load,0xf123456789abcdef
> +,store_u128,.*,128,store,0xf122334455667788f123456789abcdef
> +,load_u128,.*,128,load,0xf122334455667788f123456789abcdef
> +EOF
> +}
> +
> +expected | while read line; do
> +check "$plugin_out" "$line"
> +done
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: QEMU Community Call Agenda Items (*July 9th*, 2024)
Manos Pitsidianakis writes: > Hello Alex, I thought It was tomorrow? QEMU Project Calendar says " > Tuesday, July 9⋅4:00 – 5:00pm > Every 2 weeks on Tuesday > " Sorry yes - I really should script my invite rather than copy/paste edit each time. > > On Mon, 8 Jul 2024 at 17:58, Alex Bennée wrote: >> >> >> Hi, >> >> The KVM/QEMU community call is at: >> >> https://meet.jit.si/kvmcallmeeting >> @ >> 8/7/2024 14:00 UTC >> >> Are there any agenda items for the sync-up? >> >> -- >> Alex Bennée >> Virtualisation Tech Lead @ Linaro -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH v6 5/7] tests/tcg: allow to check output of plugins
Pierrick Bouvier writes: > A specific plugin test can now read and check a plugin output, to ensure > it contains expected values. > > Tested-by: Xingtao Yao > Reviewed-by: Richard Henderson > Signed-off-by: Pierrick Bouvier > --- > tests/tcg/Makefile.target | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target > index 52616544d52..6f50b0176ea 100644 > --- a/tests/tcg/Makefile.target > +++ b/tests/tcg/Makefile.target > @@ -90,6 +90,7 @@ CFLAGS= > LDFLAGS= > > QEMU_OPTS= > +CHECK_PLUGIN_OUTPUT_COMMAND=true > > > # If TCG debugging, or TCI is enabled things are a lot slower > @@ -180,6 +181,9 @@ run-plugin-%: > -plugin $(PLUGIN_LIB)/$(call extract-plugin,$@)$(PLUGIN_ARGS) \ > -d plugin -D $*.pout \ >$(call strip-plugin,$<)) > + $(call quiet-command, $(CHECK_PLUGIN_OUTPUT_COMMAND) $*.pout, \ > +TEST, check plugin $(call extract-plugin,$@) output \ > +with $(call strip-plugin,$<)) > else > run-%: % > $(call run-test, $<, \ > @@ -194,6 +198,9 @@ run-plugin-%: > -plugin $(PLUGIN_LIB)/$(call extract-plugin,$@)$(PLUGIN_ARGS) > \ > -d plugin -D $*.pout \ > $(QEMU_OPTS) $(call strip-plugin,$<)) > + $(call quiet-command, $(CHECK_PLUGIN_OUTPUT_COMMAND) $*.pout, \ > +TEST, check plugin $(call extract-plugin,$@) output \ > +with $(call strip-plugin,$<)) No need to have a null test, we can wrap the call in an if: --8<---cut here---start->8--- modified tests/tcg/Makefile.target @@ -90,7 +90,7 @@ CFLAGS= LDFLAGS= QEMU_OPTS= -CHECK_PLUGIN_OUTPUT_COMMAND=true +CHECK_PLUGIN_OUTPUT_COMMAND= # If TCG debugging, or TCI is enabled things are a lot slower @@ -181,9 +181,10 @@ run-plugin-%: -plugin $(PLUGIN_LIB)/$(call extract-plugin,$@)$(PLUGIN_ARGS) \ -d plugin -D $*.pout \ $(call strip-plugin,$<)) - $(call quiet-command, $(CHECK_PLUGIN_OUTPUT_COMMAND) $*.pout, \ - TEST, check plugin $(call extract-plugin,$@) output \ - with $(call strip-plugin,$<)) + $(if $(CHECK_PLUGIN_OUTPUT_COMMAND),\ + $(call quiet-command, $(CHECK_PLUGIN_OUTPUT_COMMAND) $*.pout, \ + TEST, check plugin $(call extract-plugin,$@) output \ + with $(call strip-plugin,$<))) else run-%: % $(call run-test, $<, \ @@ -198,9 +199,10 @@ run-plugin-%: -plugin $(PLUGIN_LIB)/$(call extract-plugin,$@)$(PLUGIN_ARGS) \ -d plugin -D $*.pout \ $(QEMU_OPTS) $(call strip-plugin,$<)) - $(call quiet-command, $(CHECK_PLUGIN_OUTPUT_COMMAND) $*.pout, \ - TEST, check plugin $(call extract-plugin,$@) output \ - with $(call strip-plugin,$<)) + $(if $(CHECK_PLUGIN_OUTPUT_COMMAND),\ + $(call quiet-command, $(CHECK_PLUGIN_OUTPUT_COMMAND) $*.pout, \ + TEST, check plugin $(call extract-plugin,$@) output \ + with $(call strip-plugin,$<))) --8<---cut here---end--->8--- > endif > > gdb-%: % -- Alex Bennée Virtualisation Tech Lead @ Linaro
Updated invitation: QEMU/KVM developers conference call @ Every 2 weeks from 14:00 to 15:00 on Tuesday (BST) (qemu-devel@nongnu.org)
BEGIN:VCALENDAR PRODID:-//Google Inc//Google Calendar 70.9054//EN VERSION:2.0 CALSCALE:GREGORIAN METHOD:REQUEST BEGIN:VTIMEZONE TZID:America/New_York X-LIC-LOCATION:America/New_York BEGIN:DAYLIGHT TZOFFSETFROM:-0500 TZOFFSETTO:-0400 TZNAME:EDT DTSTART:19700308T02 RRULE:FREQ=YEARLY;BYMONTH=3;BYDAY=2SU END:DAYLIGHT BEGIN:STANDARD TZOFFSETFROM:-0400 TZOFFSETTO:-0500 TZNAME:EST DTSTART:19701101T02 RRULE:FREQ=YEARLY;BYMONTH=11;BYDAY=1SU END:STANDARD END:VTIMEZONE BEGIN:VEVENT DTSTART;TZID=America/New_York:20240709T09 DTEND;TZID=America/New_York:20240709T10 RRULE:FREQ=WEEKLY;WKST=MO;INTERVAL=2;BYDAY=TU DTSTAMP:20240708T145943Z ORGANIZER;CN=QEMU Project Calendar:mailto:c_k5p2lpgvbptdirku5si01blmnk@grou p.calendar.google.com UID:1gvub9435o7hrrem0a0ralnl5i_r20240709t130...@google.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=Alessandro Di Federico;X-NUM-GUESTS=0:mailto:a...@rev.ng ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;X-NUM-GUESTS=0:mailto:alex.ben...@linaro.org ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=Alistair Francis;X-NUM-GUESTS=0:mailto:alistair.fran...@wdc.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=arm...@redhat.com;X-NUM-GUESTS=0:mailto:arm...@redhat.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=bc...@quicinc.com;X-NUM-GUESTS=0:mailto:bc...@quicinc.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=berra...@redhat.com;X-NUM-GUESTS=0:mailto:berra...@redhat.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=c...@nvidia.com;X-NUM-GUESTS=0:mailto:c...@nvidia.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;X-NUM-GUESTS=0:mailto:c...@kaod.org ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=c...@f00f.org;X-NUM-GUESTS=0:mailto:c...@f00f.org ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=ebl...@redhat.com;X-NUM-GUESTS=0:mailto:ebl...@redhat.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=edgar.igles...@gmail.com;X-NUM-GUESTS=0:mailto:edgar.iglesias@gmail .com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=TENTATIVE;RSVP=TRU E;CN=edua...@habkost.net;X-NUM-GUESTS=0:mailto:edua...@habkost.net ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=fel...@nutanix.com;X-NUM-GUESTS=0:mailto:fel...@nutanix.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=i...@theiggy.com;X-NUM-GUESTS=0:mailto:i...@theiggy.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=i...@bsdimp.com;X-NUM-GUESTS=0:mailto:i...@bsdimp.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=j...@nvidia.com;X-NUM-GUESTS=0:mailto:j...@nvidia.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=jidong.x...@gmail.com;X-NUM-GUESTS=0:mailto:jidong.x...@gmail.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=jim@sifive.com;X-NUM-GUESTS=0:mailto:jim@sifive.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=konrad.w...@oracle.com;X-NUM-GUESTS=0:mailto:konrad.w...@oracle.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=mbur...@qti.qualcomm.com;X-NUM-GUESTS=0:mailto:mburton@qti.qualcomm .com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=ACCEPTED;RSVP=TRUE ;CN=md...@redhat.com;X-NUM-GUESTS=0:mailto:md...@redhat.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=TENTATIVE;RSVP=TRU E;CN=paul.walms...@sifive.com;X-NUM-GUESTS=0:mailto:paul.walms...@sifive.co m ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=pbonz...@redhat.com;X-NUM-GUESTS=0:mailto:pbonz...@redhat.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=Peter Maydell;X-NUM-GUESTS=0:mailto:peter.mayd...@linaro.org ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=Richard Henderson;X-NUM-GUESTS=0:mailto:richard.hender...@linaro.or g ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=TENTATIVE;RSVP=TRU E;CN=shen...@gmail.com;X-NUM-GUESTS=0:mailto:shen...@gmail.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=TENTATIVE;RSVP=TRU E;CN=stefa...@gmail.com;X-NUM-GUESTS=0:mailto:stefa...@gmail.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=wei.w.w...@intel.com;X-NUM-GUESTS=0:mailto:wei.w.w...@intel.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=zwu.ker...@gmail.com;X-NUM-GUESTS=0:mailto:zwu.ker...@gmail.com
Updated invitation: QEMU/KVM developers conference call @ Every 2 weeks from 14:00 to 15:00 on Tuesday from Tue 11 Jun to Tue 9 Jul (BST) (qemu-devel@nongnu.org)
BEGIN:VCALENDAR PRODID:-//Google Inc//Google Calendar 70.9054//EN VERSION:2.0 CALSCALE:GREGORIAN METHOD:REQUEST BEGIN:VTIMEZONE TZID:America/New_York X-LIC-LOCATION:America/New_York BEGIN:DAYLIGHT TZOFFSETFROM:-0500 TZOFFSETTO:-0400 TZNAME:EDT DTSTART:19700308T02 RRULE:FREQ=YEARLY;BYMONTH=3;BYDAY=2SU END:DAYLIGHT BEGIN:STANDARD TZOFFSETFROM:-0400 TZOFFSETTO:-0500 TZNAME:EST DTSTART:19701101T02 RRULE:FREQ=YEARLY;BYMONTH=11;BYDAY=1SU END:STANDARD END:VTIMEZONE BEGIN:VEVENT DTSTART;TZID=America/New_York:20240611T09 DTEND;TZID=America/New_York:20240611T10 RRULE:FREQ=WEEKLY;WKST=MO;UNTIL=20240709T035959Z;INTERVAL=2;BYDAY=TU DTSTAMP:20240708T145942Z ORGANIZER;CN=QEMU Project Calendar:mailto:c_k5p2lpgvbptdirku5si01blmnk@grou p.calendar.google.com UID:1gvub9435o7hrrem0a0ralnl5i_r20240611t130...@google.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=Alessandro Di Federico;X-NUM-GUESTS=0:mailto:a...@rev.ng ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;X-NUM-GUESTS=0:mailto:alex.ben...@linaro.org ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=Alistair Francis;X-NUM-GUESTS=0:mailto:alistair.fran...@wdc.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=arm...@redhat.com;X-NUM-GUESTS=0:mailto:arm...@redhat.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=bc...@quicinc.com;X-NUM-GUESTS=0:mailto:bc...@quicinc.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=berra...@redhat.com;X-NUM-GUESTS=0:mailto:berra...@redhat.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=c...@nvidia.com;X-NUM-GUESTS=0:mailto:c...@nvidia.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;X-NUM-GUESTS=0:mailto:c...@kaod.org ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=c...@f00f.org;X-NUM-GUESTS=0:mailto:c...@f00f.org ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=ebl...@redhat.com;X-NUM-GUESTS=0:mailto:ebl...@redhat.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=edgar.igles...@gmail.com;X-NUM-GUESTS=0:mailto:edgar.iglesias@gmail .com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=TENTATIVE;RSVP=TRU E;CN=edua...@habkost.net;X-NUM-GUESTS=0:mailto:edua...@habkost.net ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=fel...@nutanix.com;X-NUM-GUESTS=0:mailto:fel...@nutanix.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=i...@theiggy.com;X-NUM-GUESTS=0:mailto:i...@theiggy.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=i...@bsdimp.com;X-NUM-GUESTS=0:mailto:i...@bsdimp.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=j...@nvidia.com;X-NUM-GUESTS=0:mailto:j...@nvidia.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=jidong.x...@gmail.com;X-NUM-GUESTS=0:mailto:jidong.x...@gmail.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=jim@sifive.com;X-NUM-GUESTS=0:mailto:jim@sifive.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=konrad.w...@oracle.com;X-NUM-GUESTS=0:mailto:konrad.w...@oracle.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=mbur...@qti.qualcomm.com;X-NUM-GUESTS=0:mailto:mburton@qti.qualcomm .com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=ACCEPTED;RSVP=TRUE ;CN=md...@redhat.com;X-NUM-GUESTS=0:mailto:md...@redhat.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=TENTATIVE;RSVP=TRU E;CN=paul.walms...@sifive.com;X-NUM-GUESTS=0:mailto:paul.walms...@sifive.co m ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=pbonz...@redhat.com;X-NUM-GUESTS=0:mailto:pbonz...@redhat.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=Peter Maydell;X-NUM-GUESTS=0:mailto:peter.mayd...@linaro.org ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=Richard Henderson;X-NUM-GUESTS=0:mailto:richard.hender...@linaro.or g ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=TENTATIVE;RSVP=TRU E;CN=shen...@gmail.com;X-NUM-GUESTS=0:mailto:shen...@gmail.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=TENTATIVE;RSVP=TRU E;CN=stefa...@gmail.com;X-NUM-GUESTS=0:mailto:stefa...@gmail.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP= TRUE;CN=wei.w.w...@intel.com;X-NUM-GUESTS=0:mailto:wei.w.w...@intel.com ATTENDEE;CUTYPE=INDIVIDUAL;ROLE=REQ-PARTICIPANT;PARTSTAT=NEEDS-ACTION;RSVP=
QEMU Community Call Agenda Items (July 8th, 2024)
Hi, The KVM/QEMU community call is at: https://meet.jit.si/kvmcallmeeting @ 8/7/2024 14:00 UTC Are there any agenda items for the sync-up? -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH v6 4/7] tests/tcg: add mechanism to run specific tests with plugins
Pierrick Bouvier writes: > Only multiarch tests are run with plugins, and we want to be able to run > per-arch test with plugins too. > > Tested-by: Xingtao Yao > Reviewed-by: Richard Henderson > Signed-off-by: Pierrick Bouvier > --- > tests/tcg/Makefile.target | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target > index cb8cfeb6dac..52616544d52 100644 > --- a/tests/tcg/Makefile.target > +++ b/tests/tcg/Makefile.target > @@ -152,10 +152,11 @@ PLUGINS=$(patsubst %.c, lib%.so, $(notdir $(wildcard > $(PLUGIN_SRC)/*.c))) > # only expand MULTIARCH_TESTS which are common on most of our targets > # to avoid an exponential explosion as new tests are added. We also > # add some special helpers the run-plugin- rules can use below. > +# In more, extra tests can be added using PLUGINS_TESTS variable. > > ifneq ($(MULTIARCH_TESTS),) > $(foreach p,$(PLUGINS), \ > - $(foreach t,$(MULTIARCH_TESTS),\ > + $(foreach t,$(MULTIARCH_TESTS) $(PLUGINS_TESTS),\ > $(eval run-plugin-$(t)-with-$(p): $t $p) \ > $(eval RUN_TESTS+=run-plugin-$(t)-with-$(p > endif # MULTIARCH_TESTS I have no particular objection to adding this (except a minor nit of maybe the name should be ADDITIONAL_PLUGIN_TESTS). However the use of this later is for the test: tests/tcg/x86_64/test-plugin-mem-access.c and aside from the inline asm I don't see why this couldn't be a multi-arch test. Could we not use the atomic primitives to make it multiarch? -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH v6 3/7] plugins: extend API to get latest memory value accessed
Pierrick Bouvier writes: > This value can be accessed only during a memory callback, using > new qemu_plugin_mem_get_value function. > > Returned value can be extended when QEMU will support accesses wider > than 128 bits. > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1719 > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2152 > Reviewed-by: Richard Henderson > Reviewed-by: Xingtao Yao > Signed-off-by: Pierrick Bouvier Reviewed-by: Alex Bennée -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH v6 2/7] plugins: save value during memory accesses
Pierrick Bouvier writes:
> Different code paths handle memory accesses:
> - tcg generated code
> - load/store helpers
> - atomic helpers
>
> This value is saved in cpu->neg.plugin_mem_value_{high,low}. Values are
> written only for accessed word size (upper bits are not set).
>
> Atomic operations are doing read/write at the same time, so we generate
> two memory callbacks instead of one, to allow plugins to access distinct
> values.
>
> For now, we can have access only up to 128 bits, thus split this in two
> 64 bits words. When QEMU will support wider operations, we'll be able to
> reconsider this.
>
> Reviewed-by: Richard Henderson
> Signed-off-by: Pierrick Bouvier
Reviewed-by: Alex Bennée
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH v6 1/7] plugins: fix mem callback array size
Pierrick Bouvier writes: > data was correctly copied, but size of array was not set > (g_array_sized_new only reserves memory, but does not set size). > > As a result, callbacks were not called for code path relying on > plugin_register_vcpu_mem_cb(). > > Found when trying to trigger mem access callbacks for atomic > instructions. > > Reviewed-by: Xingtao Yao > Reviewed-by: Richard Henderson > Signed-off-by: Pierrick Bouvier Reviewed-by: Alex Bennée -- Alex Bennée Virtualisation Tech Lead @ Linaro
Re: [PATCH v1 2/2] xen: mapcache: Fix unmapping of first entries in buckets
"Edgar E. Iglesias" writes:
> On Thu, Jul 4, 2024 at 9:48 PM Edgar E. Iglesias
> wrote:
>
> On Thu, Jul 04, 2024 at 05:44:52PM +0100, Alex Bennée wrote:
> > Anthony PERARD writes:
> >
> > > On Tue, Jul 02, 2024 at 12:44:21AM +0200, Edgar E. Iglesias wrote:
> > >> From: "Edgar E. Iglesias"
> > >>
> > >> This fixes the clobbering of the entry->next pointer when
> > >> unmapping the first entry in a bucket of a mapcache.
> > >>
> > >> Fixes: 123acd816d ("xen: mapcache: Unmap first entries in buckets")
> > >> Reported-by: Anthony PERARD
> > >> Signed-off-by: Edgar E. Iglesias
> > >> ---
> > >> hw/xen/xen-mapcache.c | 12 +++-
> > >> 1 file changed, 11 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/hw/xen/xen-mapcache.c b/hw/xen/xen-mapcache.c
> > >> index 5f23b0adbe..18ba7b1d8f 100644
> > >> --- a/hw/xen/xen-mapcache.c
> > >> +++ b/hw/xen/xen-mapcache.c
> > >> @@ -597,7 +597,17 @@ static void
> xen_invalidate_map_cache_entry_unlocked(MapCache *mc,
> > >> pentry->next = entry->next;
> > >> g_free(entry);
> > >> } else {
> > >> -memset(entry, 0, sizeof *entry);
> > >> +/*
> > >> + * Invalidate mapping but keep entry->next pointing to the rest
> > >> + * of the list.
> > >> + *
> > >> + * Note that lock is already zero here, otherwise we don't
> unmap.
> > >> + */
> > >> +entry->paddr_index = 0;
> > >> +entry->vaddr_base = NULL;
> > >> +entry->valid_mapping = NULL;
> > >> +entry->flags = 0;
> > >> +entry->size = 0;
> > >
> > > This kind of feels like mc->entry should be an array of pointer rather
> > > than an array of MapCacheEntry but that seems to work well enough and
> > > not the first time entries are been cleared like that.
> >
> > The use of a hand rolled list is a bit of a concern considering QEMU and
> > Glib both provide various abstractions used around the rest of the code
> > base. The original patch that introduces the mapcache doesn't tell me
> > much about access patterns for the cache, just that it is trying to
> > solve memory exhaustion issues with lots of dynamic small mappings.
> >
> > Maybe a simpler structure is desirable?
> >
> > We also have an interval tree implementation ("qemu/interval-tree.h") if
> > what we really want is a sorted tree of memory that can be iterated
> > locklessly.
> >
>
> Yes, it would be interesting to benchmark other options.
> I agree that we should at minimum reuse existing lists/hash tables.
>
> We've also had some discussions around removing it partially or alltogether
> but
> there are some concerns around that. We're going to need something to
> keep track of grants. For 32-bit hosts, it's a problem to exhaust virtual
> address-space if mapping all of the guest (are folks still using 32-bit
> hosts?).
> There may be other issues aswell.
>
> Some benefits are that we'll remove some of the complexity and latency for
> mapping
> and unmapping stuff continously.
>
> One more thing I forgot to add is that IMO, these larger longer term
> changes should not block this tiny bugfix...
Agreed.
>
> Cheers,
> Edgar
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
[PULL 20/40] gitlab: don't bother with KVM for TCI builds
In fact any other accelerator would be pointless as the point is to
exercise the TCI accelerator anyway.
Reviewed-by: Thomas Huth
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-21-alex.ben...@linaro.org>
diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 425fc6479b..e3a0758bd9 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -635,7 +635,7 @@ build-tci:
- TARGETS="aarch64 arm hppa m68k microblaze ppc64 s390x x86_64"
- mkdir build
- cd build
-- ../configure --enable-tcg-interpreter --disable-docs --disable-gtk
--disable-vnc
+- ../configure --enable-tcg-interpreter --disable-kvm --disable-docs
--disable-gtk --disable-vnc
--target-list="$(for tg in $TARGETS; do echo -n ${tg}'-softmmu ';
done)"
|| { cat config.log meson-logs/meson-log.txt && exit 1; }
- make -j"$JOBS"
diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
index 3de0341afe..cb499e4ee0 100644
--- a/.gitlab-ci.d/crossbuilds.yml
+++ b/.gitlab-ci.d/crossbuilds.yml
@@ -68,7 +68,7 @@ cross-i686-tci:
variables:
IMAGE: debian-i686-cross
ACCEL: tcg-interpreter
-EXTRA_CONFIGURE_OPTS:
--target-list=i386-softmmu,i386-linux-user,aarch64-softmmu,aarch64-linux-user,ppc-softmmu,ppc-linux-user
--disable-plugins
+EXTRA_CONFIGURE_OPTS:
--target-list=i386-softmmu,i386-linux-user,aarch64-softmmu,aarch64-linux-user,ppc-softmmu,ppc-linux-user
--disable-plugins --disable-kvm
MAKE_CHECK_ARGS: check check-tcg
cross-mipsel-system:
--
2.39.2
[PULL 31/40] gdbstub: Move GdbCmdParseEntry into a new header file
From: Gustavo Romero
Move GdbCmdParseEntry and its associated types into a separate header
file to allow the use of GdbCmdParseEntry and other gdbstub command
functions outside of gdbstub.c.
Since GdbCmdParseEntry and get_param are now public, kdoc
GdbCmdParseEntry and rename get_param to gdb_get_cmd_param.
This commit also makes gdb_put_packet public since is used in gdbstub
command handling.
Signed-off-by: Gustavo Romero
Reviewed-by: Alex Bennée
Message-Id: <20240628050850.536447-3-gustavo.rom...@linaro.org>
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-32-alex.ben...@linaro.org>
diff --git a/gdbstub/internals.h b/gdbstub/internals.h
index 32f9f63297..34121dc61a 100644
--- a/gdbstub/internals.h
+++ b/gdbstub/internals.h
@@ -106,7 +106,6 @@ static inline int tohex(int v)
*/
void gdb_put_strbuf(void);
-int gdb_put_packet(const char *buf);
int gdb_put_packet_binary(const char *buf, int len, bool dump);
void gdb_hextomem(GByteArray *mem, const char *buf, int len);
void gdb_memtohex(GString *buf, const uint8_t *mem, int len);
@@ -166,27 +165,6 @@ void gdb_put_buffer(const uint8_t *buf, int len);
*/
void gdb_init_gdbserver_state(void);
-typedef enum GDBThreadIdKind {
-GDB_ONE_THREAD = 0,
-GDB_ALL_THREADS, /* One process, all threads */
-GDB_ALL_PROCESSES,
-GDB_READ_THREAD_ERR
-} GDBThreadIdKind;
-
-typedef union GdbCmdVariant {
-const char *data;
-uint8_t opcode;
-unsigned long val_ul;
-unsigned long long val_ull;
-struct {
-GDBThreadIdKind kind;
-uint32_t pid;
-uint32_t tid;
-} thread_id;
-} GdbCmdVariant;
-
-#define get_param(p, i)(_array_index(p, GdbCmdVariant, i))
-
void gdb_handle_query_rcmd(GArray *params, void *ctx); /* system */
void gdb_handle_query_offsets(GArray *params, void *user_ctx); /* user */
void gdb_handle_query_xfer_auxv(GArray *params, void *user_ctx); /*user */
diff --git a/include/gdbstub/commands.h b/include/gdbstub/commands.h
new file mode 100644
index 00..639257493e
--- /dev/null
+++ b/include/gdbstub/commands.h
@@ -0,0 +1,72 @@
+#ifndef GDBSTUB_COMMANDS_H
+#define GDBSTUB
+
+typedef void (*GdbCmdHandler)(GArray *params, void *user_ctx);
+
+typedef enum GDBThreadIdKind {
+GDB_ONE_THREAD = 0,
+GDB_ALL_THREADS, /* One process, all threads */
+GDB_ALL_PROCESSES,
+GDB_READ_THREAD_ERR
+} GDBThreadIdKind;
+
+typedef union GdbCmdVariant {
+const char *data;
+uint8_t opcode;
+unsigned long val_ul;
+unsigned long long val_ull;
+struct {
+GDBThreadIdKind kind;
+uint32_t pid;
+uint32_t tid;
+} thread_id;
+} GdbCmdVariant;
+
+#define gdb_get_cmd_param(p, i)(_array_index(p, GdbCmdVariant, i))
+
+/**
+ * typedef GdbCmdParseEntry - gdb command parser
+ *
+ * This structure keeps the information necessary to match a gdb command,
+ * parse it (extract its parameters), and select the correct handler for it.
+ *
+ * @cmd: The command to be matched
+ * @cmd_startswith: If true, @cmd is compared using startswith
+ * @schema: Each schema for the command parameter entry consists of 2 chars,
+ * the first char represents the parameter type handling the second char
+ * represents the delimiter for the next parameter.
+ *
+ * Currently supported schema types:
+ * 'l' -> unsigned long (stored in .val_ul)
+ * 'L' -> unsigned long long (stored in .val_ull)
+ * 's' -> string (stored in .data)
+ * 'o' -> single char (stored in .opcode)
+ * 't' -> thread id (stored in .thread_id)
+ * '?' -> skip according to delimiter
+ *
+ * Currently supported delimiters:
+ * '?' -> Stop at any delimiter (",;:=\0")
+ * '0' -> Stop at "\0"
+ * '.' -> Skip 1 char unless reached "\0"
+ * Any other value is treated as the delimiter value itself
+ *
+ * @allow_stop_reply: True iff the gdbstub can respond to this command with a
+ * "stop reply" packet. The list of commands that accept such response is
+ * defined at the GDB Remote Serial Protocol documentation. See:
+ *
https://sourceware.org/gdb/onlinedocs/gdb/Stop-Reply-Packets.html#Stop-Reply-Packets.
+ */
+typedef struct GdbCmdParseEntry {
+GdbCmdHandler handler;
+const char *cmd;
+bool cmd_startswith;
+const char *schema;
+bool allow_stop_reply;
+} GdbCmdParseEntry;
+
+/**
+ * gdb_put_packet() - put string into gdb server's buffer so it is sent
+ * to the client
+ */
+int gdb_put_packet(const char *buf);
+
+#endif /* GDBSTUB_COMMANDS_H */
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 37314b92e5..9ff2f4177d 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -30,6 +30,7 @@
#include "qemu/error-report.h"
#include "trace.h"
#include "exec/gdbstub.h"
+#include "gdbstub/commands.h"
#include "gdbstub/syscalls.h"
#ifdef CONFIG_USER_ONLY
#include "accel/tcg/vcpu-state.h"
@@ -920,43 +921,6 @@ static in
[PULL 33/40] target/arm: Fix exception case in allocation_tag_mem_probe
From: Gustavo Romero
If page in 'ptr_access' is inaccessible and probe is 'true'
allocation_tag_mem_probe should not throw an exception, but currently it
does, so fix it.
Signed-off-by: Gustavo Romero
Reviewed-by: Alex Bennée
Reviewed-by: Richard Henderson
Message-Id: <20240628050850.536447-5-gustavo.rom...@linaro.org>
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-34-alex.ben...@linaro.org>
diff --git a/target/arm/tcg/mte_helper.c b/target/arm/tcg/mte_helper.c
index 037ac6dd60..a50d576294 100644
--- a/target/arm/tcg/mte_helper.c
+++ b/target/arm/tcg/mte_helper.c
@@ -96,6 +96,9 @@ static uint8_t *allocation_tag_mem_probe(CPUARMState *env,
int ptr_mmu_idx,
assert(!(probe && ra));
if (!(flags & (ptr_access == MMU_DATA_STORE ? PAGE_WRITE_ORG :
PAGE_READ))) {
+if (probe) {
+return NULL;
+}
cpu_loop_exit_sigsegv(env_cpu(env), ptr, ptr_access,
!(flags & PAGE_VALID), ra);
}
--
2.39.2
[PULL 21/40] test/plugin: make insn plugin less noisy by default
While the match functionality is useful lets make the verbosity
optional while we are actually running.
Reviewed-by: Manos Pitsidianakis
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-22-alex.ben...@linaro.org>
diff --git a/tests/plugin/insn.c b/tests/plugin/insn.c
index 5e0aa03223..524f9ddde8 100644
--- a/tests/plugin/insn.c
+++ b/tests/plugin/insn.c
@@ -20,6 +20,7 @@ static qemu_plugin_u64 insn_count;
static bool do_inline;
static bool do_size;
+static bool do_trace;
static GArray *sizes;
typedef struct {
@@ -73,30 +74,30 @@ static void vcpu_insn_matched_exec_before(unsigned int
cpu_index, void *udata)
MatchCount *match = qemu_plugin_scoreboard_find(insn_match->counts,
cpu_index);
-g_autoptr(GString) ts = g_string_new("");
-
insn->hits++;
-g_string_append_printf(ts, "0x%" PRIx64 ", '%s', %"PRId64 " hits",
- insn->vaddr, insn->disas, insn->hits);
uint64_t icount = qemu_plugin_u64_get(insn_count, cpu_index);
uint64_t delta = icount - match->last_hit;
match->hits++;
match->total_delta += delta;
-
-g_string_append_printf(ts,
- " , cpu %u,"
- " %"PRId64" match hits,"
- " Δ+%"PRId64 " since last match,"
- " %"PRId64 " avg insns/match\n",
- cpu_index,
- match->hits, delta,
- match->total_delta / match->hits);
-
match->last_hit = icount;
-qemu_plugin_outs(ts->str);
+if (do_trace) {
+g_autoptr(GString) ts = g_string_new("");
+g_string_append_printf(ts, "0x%" PRIx64 ", '%s', %"PRId64 " hits",
+ insn->vaddr, insn->disas, insn->hits);
+g_string_append_printf(ts,
+ " , cpu %u,"
+ " %"PRId64" match hits,"
+ " Δ+%"PRId64 " since last match,"
+ " %"PRId64 " avg insns/match\n",
+ cpu_index,
+ match->hits, delta,
+ match->total_delta / match->hits);
+
+qemu_plugin_outs(ts->str);
+}
}
static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
@@ -216,6 +217,11 @@ QEMU_PLUGIN_EXPORT int
qemu_plugin_install(qemu_plugin_id_t id,
}
} else if (g_strcmp0(tokens[0], "match") == 0) {
parse_match(tokens[1]);
+} else if (g_strcmp0(tokens[0], "trace") == 0) {
+if (!qemu_plugin_bool_parse(tokens[0], tokens[1], _trace)) {
+fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
+return -1;
+}
} else {
fprintf(stderr, "option parsing failed: %s\n", opt);
return -1;
--
2.39.2
[PULL 38/40] gdbstub: Use true to set cmd_startswith
From: Gustavo Romero
cmd_startswith is a boolean so use 'true' to set it instead of 1.
Signed-off-by: Gustavo Romero
Message-Id: <20240628050850.536447-10-gustavo.rom...@linaro.org>
Reviewed-by: Manos Pitsidianakis
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-39-alex.ben...@linaro.org>
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 5c1612ed2a..b9ad0a063e 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -1433,26 +1433,26 @@ static const GdbCmdParseEntry gdb_v_commands_table[] = {
{
.handler = handle_v_cont_query,
.cmd = "Cont?",
-.cmd_startswith = 1
+.cmd_startswith = true
},
{
.handler = handle_v_cont,
.cmd = "Cont",
-.cmd_startswith = 1,
+.cmd_startswith = true,
.allow_stop_reply = true,
.schema = "s0"
},
{
.handler = handle_v_attach,
.cmd = "Attach;",
-.cmd_startswith = 1,
+.cmd_startswith = true,
.allow_stop_reply = true,
.schema = "l0"
},
{
.handler = handle_v_kill,
.cmd = "Kill;",
-.cmd_startswith = 1
+.cmd_startswith = true
},
#ifdef CONFIG_USER_ONLY
/*
@@ -1462,25 +1462,25 @@ static const GdbCmdParseEntry gdb_v_commands_table[] = {
{
.handler = gdb_handle_v_file_open,
.cmd = "File:open:",
-.cmd_startswith = 1,
+.cmd_startswith = true,
.schema = "s,L,L0"
},
{
.handler = gdb_handle_v_file_close,
.cmd = "File:close:",
-.cmd_startswith = 1,
+.cmd_startswith = true,
.schema = "l0"
},
{
.handler = gdb_handle_v_file_pread,
.cmd = "File:pread:",
-.cmd_startswith = 1,
+.cmd_startswith = true,
.schema = "l,L,L0"
},
{
.handler = gdb_handle_v_file_readlink,
.cmd = "File:readlink:",
-.cmd_startswith = 1,
+.cmd_startswith = true,
.schema = "s0"
},
#endif
@@ -1748,7 +1748,7 @@ static const GdbCmdParseEntry
gdb_gen_query_set_common_table[] = {
{
.handler = handle_set_qemu_sstep,
.cmd = "qemu.sstep=",
-.cmd_startswith = 1,
+.cmd_startswith = true,
.schema = "l0"
},
};
@@ -1804,7 +1804,7 @@ static const GdbCmdParseEntry gdb_gen_query_table[] = {
{
.handler = handle_query_thread_extra,
.cmd = "ThreadExtraInfo,",
-.cmd_startswith = 1,
+.cmd_startswith = true,
.schema = "t0"
},
#ifdef CONFIG_USER_ONLY
@@ -1816,14 +1816,14 @@ static const GdbCmdParseEntry gdb_gen_query_table[] = {
{
.handler = gdb_handle_query_rcmd,
.cmd = "Rcmd,",
-.cmd_startswith = 1,
+.cmd_startswith = true,
.schema = "s0"
},
#endif
{
.handler = handle_query_supported,
.cmd = "Supported:",
-.cmd_startswith = 1,
+.cmd_startswith = true,
.schema = "s0"
},
{
@@ -1834,7 +1834,7 @@ static const GdbCmdParseEntry gdb_gen_query_table[] = {
{
.handler = handle_query_xfer_features,
.cmd = "Xfer:features:read:",
-.cmd_startswith = 1,
+.cmd_startswith = true,
.schema = "s:l,l0"
},
#if defined(CONFIG_USER_ONLY)
@@ -1842,27 +1842,27 @@ static const GdbCmdParseEntry gdb_gen_query_table[] = {
{
.handler = gdb_handle_query_xfer_auxv,
.cmd = "Xfer:auxv:read::",
-.cmd_startswith = 1,
+.cmd_startswith = true,
.schema = "l,l0"
},
{
.handler = gdb_handle_query_xfer_siginfo,
.cmd = "Xfer:siginfo:read::",
-.cmd_startswith = 1,
+.cmd_startswith = true,
.schema = "l,l0"
},
#endif
{
.handler = gdb_handle_query_xfer_exec_file,
.cmd = "Xfer:exec-file:read:",
-.cmd_startswith = 1,
+.cmd_startswith = true,
.schema = "l:l,l0"
},
#endif
{
.handler = gdb_handle_query_attached,
.cmd = "Attached:",
-.cmd_startswith = 1
+.cmd_startswith = true
},
{
.handler = gdb_handle_query_attached,
@@ -1901,14 +1901,14 @@ static const GdbCmdParseEntry gdb_gen_set_table[] = {
{
.handler = handle_set_qemu_sstep,
.cmd = "qemu.sstep:",
-.cmd_startswith = 1,
+.cmd_startswith = true,
.schema = "l0"
},
#ifndef CONFIG_USER_ONLY
{
.handler = gdb_handle_set_qemu_phy_mem_mode,
.cmd = "qemu.PhyMemMode:",
-.cmd_startswith = 1,
+
[PULL 26/40] plugins/lockstep: clean-up output
We were repeating information which wasn't super clear. As we already
will have dumped the last failing PC just note the divergence and dump
the previous instruction log.
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-27-alex.ben...@linaro.org>
diff --git a/contrib/plugins/lockstep.c b/contrib/plugins/lockstep.c
index 1765fd6c36..6a7e9bbb39 100644
--- a/contrib/plugins/lockstep.c
+++ b/contrib/plugins/lockstep.c
@@ -135,10 +135,13 @@ static void report_divergance(ExecState *us, ExecState
*them)
/* Output short log entry of going out of sync... */
if (verbose || divrec.distance == 1 || diverged) {
-g_string_printf(out,
-"@ 0x%016" PRIx64 " vs 0x%016" PRIx64
+g_string_printf(out, "@ "
+"0x%016" PRIx64 " (%" PRId64 ") vs "
+"0x%016" PRIx64 " (%" PRId64 ")"
" (%d/%d since last)\n",
-us->pc, them->pc, g_slist_length(divergence_log),
+us->pc, us->insn_count,
+them->pc, them->insn_count,
+g_slist_length(divergence_log),
divrec.distance);
qemu_plugin_outs(out->str);
}
@@ -147,10 +150,7 @@ static void report_divergance(ExecState *us, ExecState
*them)
int i;
GSList *entry;
-g_string_printf(out,
-"Δ insn_count @ 0x%016" PRIx64
-" (%"PRId64") vs 0x%016" PRIx64 " (%"PRId64")\n",
-us->pc, us->insn_count, them->pc, them->insn_count);
+g_string_printf(out, "Δ too high, we have diverged, previous insns\n");
for (entry = log, i = 0;
g_slist_next(entry) && i < 5;
@@ -163,7 +163,7 @@ static void report_divergance(ExecState *us, ExecState
*them)
prev->insn_count);
}
qemu_plugin_outs(out->str);
-qemu_plugin_outs("too much divergence... giving up.");
+qemu_plugin_outs("giving up\n");
qemu_plugin_uninstall(our_id, plugin_cleanup);
}
}
--
2.39.2
[PULL 22/40] test/plugins: preserve the instruction record over translations
We are interested in the particular instruction so we should use a
stable record for it. We could bring this down to physical address but
for now vaddr + disas seems to do the trick.
Reviewed-by: Manos Pitsidianakis
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-23-alex.ben...@linaro.org>
diff --git a/tests/plugin/insn.c b/tests/plugin/insn.c
index 524f9ddde8..baf2d07205 100644
--- a/tests/plugin/insn.c
+++ b/tests/plugin/insn.c
@@ -43,6 +43,44 @@ typedef struct {
char *disas;
} Instruction;
+/* A hash table to hold matched instructions */
+static GHashTable *match_insn_records;
+static GMutex match_hash_lock;
+
+
+static Instruction * get_insn_record(const char *disas, uint64_t vaddr, Match
*m)
+{
+g_autofree char *str_hash = g_strdup_printf("%"PRIx64" %s", vaddr, disas);
+Instruction *record;
+
+g_mutex_lock(_hash_lock);
+
+if (!match_insn_records) {
+match_insn_records = g_hash_table_new(g_str_hash, g_str_equal);
+}
+
+record = g_hash_table_lookup(match_insn_records, str_hash);
+
+if (!record) {
+g_autoptr(GString) ts = g_string_new(str_hash);
+
+record = g_new0(Instruction, 1);
+record->disas = g_strdup(disas);
+record->vaddr = vaddr;
+record->match = m;
+
+g_hash_table_insert(match_insn_records, str_hash, record);
+
+g_string_prepend(ts, "Created record for: ");
+g_string_append(ts, "\n");
+qemu_plugin_outs(ts->str);
+}
+
+g_mutex_unlock(_hash_lock);
+
+return record;
+}
+
/*
* Initialise a new vcpu with reading the register list
*/
@@ -131,16 +169,19 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct
qemu_plugin_tb *tb)
* If we are tracking certain instructions we will need more
* information about the instruction which we also need to
* save if there is a hit.
+ *
+ * We only want one record for each occurrence of the matched
+ * instruction.
*/
if (matches->len) {
char *insn_disas = qemu_plugin_insn_disas(insn);
for (int j = 0; j < matches->len; j++) {
Match *m = _array_index(matches, Match, j);
if (g_str_has_prefix(insn_disas, m->match_string)) {
-Instruction *rec = g_new0(Instruction, 1);
-rec->disas = g_strdup(insn_disas);
-rec->vaddr = qemu_plugin_insn_vaddr(insn);
-rec->match = m;
+Instruction *rec = get_insn_record(insn_disas,
+
qemu_plugin_insn_vaddr(insn),
+ m);
+
qemu_plugin_register_vcpu_insn_exec_cb(
insn, vcpu_insn_matched_exec_before,
QEMU_PLUGIN_CB_NO_REGS, rec);
@@ -173,13 +214,38 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
qemu_plugin_u64_sum(insn_count));
}
qemu_plugin_outs(out->str);
-
qemu_plugin_scoreboard_free(insn_count.score);
+
+g_mutex_lock(_hash_lock);
+
for (i = 0; i < matches->len; ++i) {
Match *m = _array_index(matches, Match, i);
+GHashTableIter iter;
+Instruction *record;
+qemu_plugin_u64 hit_e =
qemu_plugin_scoreboard_u64_in_struct(m->counts, MatchCount, hits);
+uint64_t hits = qemu_plugin_u64_sum(hit_e);
+
+g_string_printf(out, "Match: %s, hits %"PRId64"\n", m->match_string,
hits);
+qemu_plugin_outs(out->str);
+
+g_hash_table_iter_init(, match_insn_records);
+while (g_hash_table_iter_next(, NULL, (void **))) {
+if (record->match == m) {
+g_string_printf(out,
+" %"PRIx64": %s (hits %"PRId64")\n",
+record->vaddr,
+record->disas,
+record->hits);
+qemu_plugin_outs(out->str);
+}
+}
+
g_free(m->match_string);
qemu_plugin_scoreboard_free(m->counts);
}
+
+g_mutex_unlock(_hash_lock);
+
g_array_free(matches, TRUE);
g_array_free(sizes, TRUE);
}
--
2.39.2
[PULL 28/40] plugins: Free CPUPluginState before destroying vCPU state
From: Philippe Mathieu-Daudé
cpu::plugin_state is allocated in cpu_common_initfn() when
the vCPU state is created. Release it in cpu_common_finalize()
when we are done.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Pierrick Bouvier
Message-Id: <20240606124010.2460-3-phi...@linaro.org>
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-29-alex.ben...@linaro.org>
diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h
index bc5aef979e..af5f9db469 100644
--- a/include/qemu/plugin.h
+++ b/include/qemu/plugin.h
@@ -149,6 +149,9 @@ struct CPUPluginState {
/**
* qemu_plugin_create_vcpu_state: allocate plugin state
+ *
+ * The returned data must be released with g_free()
+ * when no longer required.
*/
CPUPluginState *qemu_plugin_create_vcpu_state(void);
diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
index f131cde2c0..8f6cb64da3 100644
--- a/hw/core/cpu-common.c
+++ b/hw/core/cpu-common.c
@@ -283,6 +283,11 @@ static void cpu_common_finalize(Object *obj)
{
CPUState *cpu = CPU(obj);
+#ifdef CONFIG_PLUGIN
+if (tcg_enabled()) {
+g_free(cpu->plugin_state);
+}
+#endif
g_array_free(cpu->gdb_regs, TRUE);
qemu_lockcnt_destroy(>in_ioctl_lock);
qemu_mutex_destroy(>work_mutex);
--
2.39.2
[PULL 19/40] linux-user/main: Suppress out-of-range comparison warning for clang
From: Richard Henderson
For arm32 host and arm64 guest we get
.../main.c:851:32: error: result of comparison of constant 70368744177664 with
expression of type 'unsigned long' is always false
[-Werror,-Wtautological-constant-out-of-range-compare]
if (TASK_UNMAPPED_BASE < reserved_va) {
~~ ^ ~~~
We already disable -Wtype-limits here, for this exact comparison, but
that is not enough for clang. Disable -Wtautological-compare as well,
which is a superset. GCC ignores the unknown warning flag.
Signed-off-by: Richard Henderson
Reviewed-by: Akihiko Odaki
Message-Id: <20240630190050.160642-15-richard.hender...@linaro.org>
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-20-alex.ben...@linaro.org>
diff --git a/linux-user/main.c b/linux-user/main.c
index 94c99a1366..7d3cf45fa9 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -843,6 +843,7 @@ int main(int argc, char **argv, char **envp)
*/
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wtype-limits"
+#pragma GCC diagnostic ignored "-Wtautological-compare"
/*
* Select an initial value for task_unmapped_base that is in range.
--
2.39.2
[PULL 32/40] gdbstub: Add support for target-specific stubs
From: Gustavo Romero
Currently, it's not possible to have stubs specific to a given target,
even though there are GDB features which are target-specific, like, for
instance, memory tagging.
This commit introduces gdb_extend_qsupported_features,
gdb_extend_query_table, and gdb_extend_set_table functions as interfaces
to extend the qSupported string, the query handler table, and the set
handler table, allowing target-specific stub implementations.
Signed-off-by: Gustavo Romero
Reviewed-by: Alex Bennée
Message-Id: <20240628050850.536447-4-gustavo.rom...@linaro.org>
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-33-alex.ben...@linaro.org>
diff --git a/include/gdbstub/commands.h b/include/gdbstub/commands.h
index 639257493e..306dfdef97 100644
--- a/include/gdbstub/commands.h
+++ b/include/gdbstub/commands.h
@@ -69,4 +69,26 @@ typedef struct GdbCmdParseEntry {
*/
int gdb_put_packet(const char *buf);
+/**
+ * gdb_extend_query_table() - Extend query table.
+ * @table: The table with the additional query packet handlers.
+ * @size: The number of handlers to be added.
+ */
+void gdb_extend_query_table(GdbCmdParseEntry *table, int size);
+
+/**
+ * gdb_extend_set_table() - Extend set table.
+ * @table: The table with the additional set packet handlers.
+ * @size: The number of handlers to be added.
+ */
+void gdb_extend_set_table(GdbCmdParseEntry *table, int size);
+
+/**
+ * gdb_extend_qsupported_features() - Extend the qSupported features string.
+ * @qsupported_features: The additional qSupported feature(s) string. The
string
+ * should start with a semicolon and, if there are more than one feature, the
+ * features should be separate by a semiocolon.
+ */
+void gdb_extend_qsupported_features(char *qsupported_features);
+
#endif /* GDBSTUB_COMMANDS_H */
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 9ff2f4177d..b1ca253f97 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -1609,6 +1609,20 @@ static void handle_query_thread_extra(GArray *params,
void *user_ctx)
gdb_put_strbuf();
}
+static char *extended_qsupported_features;
+void gdb_extend_qsupported_features(char *qsupported_features)
+{
+/*
+ * We don't support different sets of CPU gdb features on different CPUs
yet
+ * so assert the feature strings are the same on all CPUs, or is set only
+ * once (1 CPU).
+ */
+g_assert(extended_qsupported_features == NULL ||
+ g_strcmp0(extended_qsupported_features, qsupported_features) ==
0);
+
+extended_qsupported_features = qsupported_features;
+}
+
static void handle_query_supported(GArray *params, void *user_ctx)
{
CPUClass *cc;
@@ -1648,6 +1662,11 @@ static void handle_query_supported(GArray *params, void
*user_ctx)
}
g_string_append(gdbserver_state.str_buf, ";vContSupported+;multiprocess+");
+
+if (extended_qsupported_features) {
+g_string_append(gdbserver_state.str_buf, extended_qsupported_features);
+}
+
gdb_put_strbuf();
}
@@ -1729,6 +1748,41 @@ static const GdbCmdParseEntry
gdb_gen_query_set_common_table[] = {
},
};
+/* Compares if a set of command parsers is equal to another set of parsers. */
+static bool cmp_cmds(GdbCmdParseEntry *c, GdbCmdParseEntry *d, int size)
+{
+for (int i = 0; i < size; i++) {
+if (!(c[i].handler == d[i].handler &&
+g_strcmp0(c[i].cmd, d[i].cmd) == 0 &&
+c[i].cmd_startswith == d[i].cmd_startswith &&
+g_strcmp0(c[i].schema, d[i].schema) == 0)) {
+
+/* Sets are different. */
+return false;
+}
+}
+
+/* Sets are equal, i.e. contain the same command parsers. */
+return true;
+}
+
+static GdbCmdParseEntry *extended_query_table;
+static int extended_query_table_size;
+void gdb_extend_query_table(GdbCmdParseEntry *table, int size)
+{
+/*
+ * We don't support different sets of CPU gdb features on different CPUs
yet
+ * so assert query table is the same on all CPUs, or is set only once
+ * (1 CPU).
+ */
+g_assert(extended_query_table == NULL ||
+ (extended_query_table_size == size &&
+ cmp_cmds(extended_query_table, table, size)));
+
+extended_query_table = table;
+extended_query_table_size = size;
+}
+
static const GdbCmdParseEntry gdb_gen_query_table[] = {
{
.handler = handle_query_curr_tid,
@@ -1821,6 +1875,22 @@ static const GdbCmdParseEntry gdb_gen_query_table[] = {
#endif
};
+static GdbCmdParseEntry *extended_set_table;
+static int extended_set_table_size;
+void gdb_extend_set_table(GdbCmdParseEntry *table, int size)
+{
+/*
+ * We don't support different sets of CPU gdb features on different CPUs
yet
+ * so assert set table is the same on all CPUs, or is set only once (1
CPU).
+ */
+g_assert(extended_set_table == NULL ||
+ (extended_set_table_size == size &&
+ cmp_c
[PULL 30/40] gdbstub: Clean up process_string_cmd
From: Gustavo Romero
Change 'process_string_cmd' to return true on success and false on
failure, instead of 0 and -1.
Signed-off-by: Gustavo Romero
Reviewed-by: Alex Bennée
Reviewed-by: Richard Henderson
Reviewed-by: Philippe Mathieu-Daudé
Message-Id: <20240628050850.536447-2-gustavo.rom...@linaro.org>
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-31-alex.ben...@linaro.org>
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index b3574997ea..37314b92e5 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -962,14 +962,14 @@ static inline int startswith(const char *string, const
char *pattern)
return !strncmp(string, pattern, strlen(pattern));
}
-static int process_string_cmd(const char *data,
- const GdbCmdParseEntry *cmds, int num_cmds)
+static bool process_string_cmd(const char *data,
+ const GdbCmdParseEntry *cmds, int num_cmds)
{
int i;
g_autoptr(GArray) params = g_array_new(false, true, sizeof(GdbCmdVariant));
if (!cmds) {
-return -1;
+return false;
}
for (i = 0; i < num_cmds; i++) {
@@ -984,16 +984,16 @@ static int process_string_cmd(const char *data,
if (cmd->schema) {
if (cmd_parse_params([strlen(cmd->cmd)],
cmd->schema, params)) {
-return -1;
+return false;
}
}
gdbserver_state.allow_stop_reply = cmd->allow_stop_reply;
cmd->handler(params, NULL);
-return 0;
+return true;
}
-return -1;
+return false;
}
static void run_cmd_parser(const char *data, const GdbCmdParseEntry *cmd)
@@ -1007,7 +1007,7 @@ static void run_cmd_parser(const char *data, const
GdbCmdParseEntry *cmd)
/* In case there was an error during the command parsing we must
* send a NULL packet to indicate the command is not supported */
-if (process_string_cmd(data, cmd, 1)) {
+if (!process_string_cmd(data, cmd, 1)) {
gdb_put_packet("");
}
}
@@ -1523,9 +1523,9 @@ static void handle_v_commands(GArray *params, void
*user_ctx)
return;
}
-if (process_string_cmd(get_param(params, 0)->data,
- gdb_v_commands_table,
- ARRAY_SIZE(gdb_v_commands_table))) {
+if (!process_string_cmd(get_param(params, 0)->data,
+gdb_v_commands_table,
+ARRAY_SIZE(gdb_v_commands_table))) {
gdb_put_packet("");
}
}
@@ -1889,15 +1889,15 @@ static void handle_gen_query(GArray *params, void
*user_ctx)
return;
}
-if (!process_string_cmd(get_param(params, 0)->data,
-gdb_gen_query_set_common_table,
-ARRAY_SIZE(gdb_gen_query_set_common_table))) {
+if (process_string_cmd(get_param(params, 0)->data,
+ gdb_gen_query_set_common_table,
+ ARRAY_SIZE(gdb_gen_query_set_common_table))) {
return;
}
-if (process_string_cmd(get_param(params, 0)->data,
- gdb_gen_query_table,
- ARRAY_SIZE(gdb_gen_query_table))) {
+if (!process_string_cmd(get_param(params, 0)->data,
+gdb_gen_query_table,
+ARRAY_SIZE(gdb_gen_query_table))) {
gdb_put_packet("");
}
}
@@ -1908,13 +1908,13 @@ static void handle_gen_set(GArray *params, void
*user_ctx)
return;
}
-if (!process_string_cmd(get_param(params, 0)->data,
-gdb_gen_query_set_common_table,
-ARRAY_SIZE(gdb_gen_query_set_common_table))) {
+if (process_string_cmd(get_param(params, 0)->data,
+ gdb_gen_query_set_common_table,
+ ARRAY_SIZE(gdb_gen_query_set_common_table))) {
return;
}
-if (process_string_cmd(get_param(params, 0)->data,
+if (!process_string_cmd(get_param(params, 0)->data,
gdb_gen_set_table,
ARRAY_SIZE(gdb_gen_set_table))) {
gdb_put_packet("");
--
2.39.2
[PULL 24/40] plugins/lockstep: make mixed-mode safe
The ExecState is shared across the socket and if we want to compare
say 64 bit and 32 bit binaries we need the two to use the same sizes
for things.
Reviewed-by: Richard Henderson
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-25-alex.ben...@linaro.org>
diff --git a/contrib/plugins/lockstep.c b/contrib/plugins/lockstep.c
index 67a779ee9d..8b90b37f67 100644
--- a/contrib/plugins/lockstep.c
+++ b/contrib/plugins/lockstep.c
@@ -57,7 +57,7 @@ typedef struct {
/* The execution state we compare */
typedef struct {
uint64_t pc;
-unsigned long insn_count;
+uint64_t insn_count;
} ExecState;
typedef struct {
@@ -148,7 +148,7 @@ static void report_divergance(ExecState *us, ExecState
*them)
g_string_printf(out,
"Δ insn_count @ 0x%016" PRIx64
-" (%ld) vs 0x%016" PRIx64 " (%ld)\n",
+" (%"PRId64") vs 0x%016" PRIx64 " (%"PRId64")\n",
us->pc, us->insn_count, them->pc, them->insn_count);
for (entry = log, i = 0;
--
2.39.2
[PULL 23/40] plugins/lockstep: preserve sock_path
We can't assign sock_path directly from the autofree'd GStrv, take a
copy.
Reviewed-by: Manos Pitsidianakis
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-24-alex.ben...@linaro.org>
diff --git a/contrib/plugins/lockstep.c b/contrib/plugins/lockstep.c
index 237543b43a..67a779ee9d 100644
--- a/contrib/plugins/lockstep.c
+++ b/contrib/plugins/lockstep.c
@@ -347,7 +347,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t
id,
return -1;
}
} else if (g_strcmp0(tokens[0], "sockpath") == 0) {
-sock_path = tokens[1];
+sock_path = g_strdup(tokens[1]);
} else {
fprintf(stderr, "option parsing failed: %s\n", p);
return -1;
--
2.39.2
[PULL 36/40] gdbstub: Make hex conversion function non-internal
From: Gustavo Romero Make gdb_hextomem non-internal so it's not confined to use only in gdbstub.c. Signed-off-by: Gustavo Romero Reviewed-by: Richard Henderson Message-Id: <20240628050850.536447-8-gustavo.rom...@linaro.org> Signed-off-by: Alex Bennée Message-Id: <20240705084047.857176-37-alex.ben...@linaro.org> diff --git a/gdbstub/internals.h b/gdbstub/internals.h index 34121dc61a..bf5a5c6302 100644 --- a/gdbstub/internals.h +++ b/gdbstub/internals.h @@ -107,7 +107,6 @@ static inline int tohex(int v) void gdb_put_strbuf(void); int gdb_put_packet_binary(const char *buf, int len, bool dump); -void gdb_hextomem(GByteArray *mem, const char *buf, int len); void gdb_memtohex(GString *buf, const uint8_t *mem, int len); void gdb_memtox(GString *buf, const char *mem, int len); void gdb_read_byte(uint8_t ch); diff --git a/include/gdbstub/commands.h b/include/gdbstub/commands.h index 306dfdef97..e51f276b40 100644 --- a/include/gdbstub/commands.h +++ b/include/gdbstub/commands.h @@ -91,4 +91,10 @@ void gdb_extend_set_table(GdbCmdParseEntry *table, int size); */ void gdb_extend_qsupported_features(char *qsupported_features); +/** + * Convert a hex string to bytes. Conversion is done per byte, so 2 hex digits + * are converted to 1 byte. Invalid hex digits are treated as 0 digits. + */ +void gdb_hextomem(GByteArray *mem, const char *buf, int len); + #endif /* GDBSTUB_COMMANDS_H */ -- 2.39.2
[PULL 39/40] gdbstub: Add support for MTE in user mode
From: Gustavo Romero
This commit implements the stubs to handle the qIsAddressTagged,
qMemTag, and QMemTag GDB packets, allowing all GDB 'memory-tag'
subcommands to work with QEMU gdbstub on aarch64 user mode. It also
implements the get/set functions for the special GDB MTE register
'tag_ctl', used to control the MTE fault type at runtime.
Signed-off-by: Gustavo Romero
Message-Id: <20240628050850.536447-11-gustavo.rom...@linaro.org>
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-40-alex.ben...@linaro.org>
diff --git a/configs/targets/aarch64-linux-user.mak
b/configs/targets/aarch64-linux-user.mak
index ba8bc5fe3f..8f0ed21d76 100644
--- a/configs/targets/aarch64-linux-user.mak
+++ b/configs/targets/aarch64-linux-user.mak
@@ -1,6 +1,6 @@
TARGET_ARCH=aarch64
TARGET_BASE_ARCH=arm
-TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml
gdb-xml/aarch64-pauth.xml
+TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml
gdb-xml/aarch64-pauth.xml gdb-xml/aarch64-mte.xml
TARGET_HAS_BFLT=y
CONFIG_SEMIHOSTING=y
CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
diff --git a/target/arm/internals.h b/target/arm/internals.h
index 11b5da2562..e1aa1a63b9 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -358,6 +358,10 @@ void init_cpreg_list(ARMCPU *cpu);
void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu);
void arm_translate_init(void);
+void arm_cpu_register_gdb_commands(ARMCPU *cpu);
+void aarch64_cpu_register_gdb_commands(ARMCPU *cpu, GString *, GArray *,
+ GArray *);
+
void arm_restore_state_to_opc(CPUState *cs,
const TranslationBlock *tb,
const uint64_t *data);
@@ -1640,6 +1644,8 @@ int aarch64_gdb_get_fpu_reg(CPUState *cs, GByteArray
*buf, int reg);
int aarch64_gdb_set_fpu_reg(CPUState *cs, uint8_t *buf, int reg);
int aarch64_gdb_get_pauth_reg(CPUState *cs, GByteArray *buf, int reg);
int aarch64_gdb_set_pauth_reg(CPUState *cs, uint8_t *buf, int reg);
+int aarch64_gdb_get_tag_ctl_reg(CPUState *cs, GByteArray *buf, int reg);
+int aarch64_gdb_set_tag_ctl_reg(CPUState *cs, uint8_t *buf, int reg);
void arm_cpu_sve_finalize(ARMCPU *cpu, Error **errp);
void arm_cpu_sme_finalize(ARMCPU *cpu, Error **errp);
void arm_cpu_pauth_finalize(ARMCPU *cpu, Error **errp);
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 35fa281f1b..14d4eca127 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2518,6 +2518,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error
**errp)
register_cp_regs_for_features(cpu);
arm_cpu_register_gdb_regs_for_features(cpu);
+arm_cpu_register_gdb_commands(cpu);
init_cpreg_list(cpu);
diff --git a/target/arm/gdbstub.c b/target/arm/gdbstub.c
index a3bb73cfa7..c3a9b5eb1e 100644
--- a/target/arm/gdbstub.c
+++ b/target/arm/gdbstub.c
@@ -21,6 +21,7 @@
#include "cpu.h"
#include "exec/gdbstub.h"
#include "gdbstub/helpers.h"
+#include "gdbstub/commands.h"
#include "sysemu/tcg.h"
#include "internals.h"
#include "cpu-features.h"
@@ -474,6 +475,41 @@ static GDBFeature
*arm_gen_dynamic_m_secextreg_feature(CPUState *cs,
#endif
#endif /* CONFIG_TCG */
+void arm_cpu_register_gdb_commands(ARMCPU *cpu)
+{
+GArray *query_table =
+g_array_new(FALSE, FALSE, sizeof(GdbCmdParseEntry));
+GArray *set_table =
+g_array_new(FALSE, FALSE, sizeof(GdbCmdParseEntry));
+GString *qsupported_features = g_string_new(NULL);
+
+if (arm_feature(>env, ARM_FEATURE_AARCH64)) {
+#ifdef TARGET_AARCH64
+aarch64_cpu_register_gdb_commands(cpu, qsupported_features,
query_table,
+ set_table);
+#endif
+}
+
+/* Set arch-specific handlers for 'q' commands. */
+if (query_table->len) {
+gdb_extend_query_table(_array_index(query_table,
+ GdbCmdParseEntry, 0),
+ query_table->len);
+}
+
+/* Set arch-specific handlers for 'Q' commands. */
+if (set_table->len) {
+gdb_extend_set_table(_array_index(set_table,
+ GdbCmdParseEntry, 0),
+ set_table->len);
+}
+
+/* Set arch-specific qSupported feature. */
+if (qsupported_features->len) {
+gdb_extend_qsupported_features(qsupported_features->str);
+}
+}
+
void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
{
CPUState *cs = CPU(cpu);
@@ -507,6 +543,16 @@ void arm_cpu_register_gdb_regs_for_features(ARMCPU *cpu)
gdb_find_static_feature("aarch64-pauth.xml"),
0);
}
+
+#ifdef CONFIG_USER_ONLY
+/* Memory Tagging Extension (MTE) 'tag_ctl' pseudo-register. */
+
[PULL 18/40] tests/tcg/arm: Use vmrs/vmsr instead of mcr/mrc
From: Richard Henderson
Clang 14 generates
/home/rth/qemu/src/tests/tcg/arm/fcvt.c:431:9: error: invalid operand for
instruction
asm("mrc p10, 7, r1, cr1, cr0, 0\n\t"
^
:1:6: note: instantiated into assembly here
mrc p10, 7, r1, cr1, cr0, 0
^
/home/rth/qemu/src/tests/tcg/arm/fcvt.c:432:32: error: invalid operand for
instruction
"orr r1, r1, %[flags]\n\t"
^
:3:6: note: instantiated into assembly here
mcr p10, 7, r1, cr1, cr0, 0
^
This is perhaps a clang bug, but using the neon mnemonic is clearer.
Signed-off-by: Richard Henderson
Reviewed-by: Akihiko Odaki
Message-Id: <20240630190050.160642-14-richard.hender...@linaro.org>
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-19-alex.ben...@linaro.org>
diff --git a/tests/tcg/arm/fcvt.c b/tests/tcg/arm/fcvt.c
index d8c61cd29f..ecebbb0247 100644
--- a/tests/tcg/arm/fcvt.c
+++ b/tests/tcg/arm/fcvt.c
@@ -427,10 +427,9 @@ int main(int argc, char *argv[argc])
/* And now with ARM alternative FP16 */
#if defined(__arm__)
-/* See glibc sysdeps/arm/fpu_control.h */
-asm("mrc p10, 7, r1, cr1, cr0, 0\n\t"
+asm("vmrs r1, fpscr\n\t"
"orr r1, r1, %[flags]\n\t"
-"mcr p10, 7, r1, cr1, cr0, 0\n\t"
+"vmsr fpscr, r1"
: /* no output */ : [flags] "n" (1 << 26) : "r1" );
#else
asm("mrs x1, fpcr\n\t"
--
2.39.2
[PULL 25/40] plugins/lockstep: mention the one-insn-per-tb option
This really helps with lockstep although its super slow on big jobs. Reviewed-by: Manos Pitsidianakis Signed-off-by: Alex Bennée Message-Id: <20240705084047.857176-26-alex.ben...@linaro.org> diff --git a/contrib/plugins/lockstep.c b/contrib/plugins/lockstep.c index 8b90b37f67..1765fd6c36 100644 --- a/contrib/plugins/lockstep.c +++ b/contrib/plugins/lockstep.c @@ -14,7 +14,8 @@ * particular run may execute the exact same sequence of blocks. An * asynchronous event (for example X11 graphics update) may cause a * block to end early and a new partial block to start. This means - * serial only test cases are a better bet. -d nochain may also help. + * serial only test cases are a better bet. -d nochain may also help + * as well as -accel tcg,one-insn-per-tb=on * * This code is not thread safe! * -- 2.39.2
[PULL 34/40] target/arm: Make some MTE helpers widely available
From: Gustavo Romero Make the MTE helpers allocation_tag_mem_probe, load_tag1, and store_tag1 available to other subsystems. Signed-off-by: Gustavo Romero Reviewed-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20240628050850.536447-6-gustavo.rom...@linaro.org> Signed-off-by: Alex Bennée Message-Id: <20240705084047.857176-35-alex.ben...@linaro.org> diff --git a/target/arm/tcg/mte_helper.h b/target/arm/tcg/mte_helper.h new file mode 100644 index 00..1f471fb69b --- /dev/null +++ b/target/arm/tcg/mte_helper.h @@ -0,0 +1,66 @@ +/* + * ARM MemTag operation helpers. + * + * This code is licensed under the GNU GPL v2 or later. + * + * SPDX-License-Identifier: LGPL-2.1-or-later + */ + +#ifndef TARGET_ARM_MTE_H +#define TARGET_ARM_MTE_H + +#include "exec/mmu-access-type.h" + +/** + * allocation_tag_mem_probe: + * @env: the cpu environment + * @ptr_mmu_idx: the addressing regime to use for the virtual address + * @ptr: the virtual address for which to look up tag memory + * @ptr_access: the access to use for the virtual address + * @ptr_size: the number of bytes in the normal memory access + * @tag_access: the access to use for the tag memory + * @probe: true to merely probe, never taking an exception + * @ra: the return address for exception handling + * + * Our tag memory is formatted as a sequence of little-endian nibbles. + * That is, the byte at (addr >> (LOG2_TAG_GRANULE + 1)) contains two + * tags, with the tag at [3:0] for the lower addr and the tag at [7:4] + * for the higher addr. + * + * Here, resolve the physical address from the virtual address, and return + * a pointer to the corresponding tag byte. + * + * If there is no tag storage corresponding to @ptr, return NULL. + * + * If the page is inaccessible for @ptr_access, or has a watchpoint, there are + * three options: + * (1) probe = true, ra = 0 : pure probe -- we return NULL if the page is not + * accessible, and do not take watchpoint traps. The calling code must + * handle those cases in the right priority compared to MTE traps. + * (2) probe = false, ra = 0 : probe, no fault expected -- the caller guarantees + * that the page is going to be accessible. We will take watchpoint traps. + * (3) probe = false, ra != 0 : non-probe -- we will take both memory access + * traps and watchpoint traps. + * (probe = true, ra != 0 is invalid and will assert.) + */ +uint8_t *allocation_tag_mem_probe(CPUARMState *env, int ptr_mmu_idx, + uint64_t ptr, MMUAccessType ptr_access, + int ptr_size, MMUAccessType tag_access, + bool probe, uintptr_t ra); + +/** + * load_tag1 - Load 1 tag (nibble) from byte + * @ptr: The tagged address + * @mem: The tag address (packed, 2 tags in byte) + */ +int load_tag1(uint64_t ptr, uint8_t *mem); + +/** + * store_tag1 - Store 1 tag (nibble) into byte + * @ptr: The tagged address + * @mem: The tag address (packed, 2 tags in byte) + * @tag: The tag to be stored in the nibble + */ +void store_tag1(uint64_t ptr, uint8_t *mem, int tag); + +#endif /* TARGET_ARM_MTE_H */ diff --git a/target/arm/tcg/mte_helper.c b/target/arm/tcg/mte_helper.c index a50d576294..9d2ba287ee 100644 --- a/target/arm/tcg/mte_helper.c +++ b/target/arm/tcg/mte_helper.c @@ -29,6 +29,7 @@ #include "hw/core/tcg-cpu-ops.h" #include "qapi/error.h" #include "qemu/guest-random.h" +#include "mte_helper.h" static int choose_nonexcluded_tag(int tag, int offset, uint16_t exclude) @@ -50,42 +51,10 @@ static int choose_nonexcluded_tag(int tag, int offset, uint16_t exclude) return tag; } -/** - * allocation_tag_mem_probe: - * @env: the cpu environment - * @ptr_mmu_idx: the addressing regime to use for the virtual address - * @ptr: the virtual address for which to look up tag memory - * @ptr_access: the access to use for the virtual address - * @ptr_size: the number of bytes in the normal memory access - * @tag_access: the access to use for the tag memory - * @probe: true to merely probe, never taking an exception - * @ra: the return address for exception handling - * - * Our tag memory is formatted as a sequence of little-endian nibbles. - * That is, the byte at (addr >> (LOG2_TAG_GRANULE + 1)) contains two - * tags, with the tag at [3:0] for the lower addr and the tag at [7:4] - * for the higher addr. - * - * Here, resolve the physical address from the virtual address, and return - * a pointer to the corresponding tag byte. - * - * If there is no tag storage corresponding to @ptr, return NULL. - * - * If the page is inaccessible for @ptr_access, or has a watchpoint, there are - * three options: - * (1) probe = true, ra = 0 : pure probe -- we return NULL if the page is not - * accessible, and do not take watchpoint traps. The calling code must - * handle those cases in the right priority compared to MTE traps. - * (2)
[PULL 27/40] plugins: Ensure vCPU index is assigned in init/exit hooks
From: Philippe Mathieu-Daudé
Since vCPUs are hashed by their index, this index can't
be uninitialized (UNASSIGNED_CPU_INDEX).
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Pierrick Bouvier
Message-Id: <20240606124010.2460-2-phi...@linaro.org>
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-28-alex.ben...@linaro.org>
diff --git a/plugins/core.c b/plugins/core.c
index 9d737d8278..a864275ae7 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -245,6 +245,7 @@ void qemu_plugin_vcpu_init_hook(CPUState *cpu)
{
bool success;
+assert(cpu->cpu_index != UNASSIGNED_CPU_INDEX);
qemu_rec_mutex_lock();
plugin.num_vcpus = MAX(plugin.num_vcpus, cpu->cpu_index + 1);
plugin_cpu_update__locked(>cpu_index, NULL, NULL);
@@ -263,6 +264,7 @@ void qemu_plugin_vcpu_exit_hook(CPUState *cpu)
plugin_vcpu_cb__simple(cpu, QEMU_PLUGIN_EV_VCPU_EXIT);
+assert(cpu->cpu_index != UNASSIGNED_CPU_INDEX);
qemu_rec_mutex_lock();
success = g_hash_table_remove(plugin.cpu_ht, >cpu_index);
g_assert(success);
--
2.39.2
[PULL 29/40] accel/tcg: Move qemu_plugin_vcpu_init__async() to plugins/
From: Philippe Mathieu-Daudé
Calling qemu_plugin_vcpu_init__async() on the vCPU thread
is a detail of plugins, not relevant to TCG vCPU management.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Reviewed-by: Pierrick Bouvier
Message-Id: <20240606124010.2460-4-phi...@linaro.org>
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-30-alex.ben...@linaro.org>
diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c
index 8f6cb64da3..b19e1fdacf 100644
--- a/hw/core/cpu-common.c
+++ b/hw/core/cpu-common.c
@@ -192,13 +192,6 @@ static void cpu_common_parse_features(const char
*typename, char *features,
}
}
-#ifdef CONFIG_PLUGIN
-static void qemu_plugin_vcpu_init__async(CPUState *cpu, run_on_cpu_data unused)
-{
-qemu_plugin_vcpu_init_hook(cpu);
-}
-#endif
-
static void cpu_common_realizefn(DeviceState *dev, Error **errp)
{
CPUState *cpu = CPU(dev);
@@ -274,7 +267,7 @@ static void cpu_common_initfn(Object *obj)
#ifdef CONFIG_PLUGIN
if (tcg_enabled()) {
cpu->plugin_state = qemu_plugin_create_vcpu_state();
-async_run_on_cpu(cpu, qemu_plugin_vcpu_init__async, RUN_ON_CPU_NULL);
+qemu_plugin_vcpu_init_hook(cpu);
}
#endif
}
diff --git a/plugins/core.c b/plugins/core.c
index a864275ae7..12c67b4b4e 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -241,7 +241,7 @@ static void plugin_grow_scoreboards__locked(CPUState *cpu)
end_exclusive();
}
-void qemu_plugin_vcpu_init_hook(CPUState *cpu)
+static void qemu_plugin_vcpu_init__async(CPUState *cpu, run_on_cpu_data unused)
{
bool success;
@@ -258,6 +258,12 @@ void qemu_plugin_vcpu_init_hook(CPUState *cpu)
plugin_vcpu_cb__simple(cpu, QEMU_PLUGIN_EV_VCPU_INIT);
}
+void qemu_plugin_vcpu_init_hook(CPUState *cpu)
+{
+/* Plugin initialization must wait until the cpu start executing code */
+async_run_on_cpu(cpu, qemu_plugin_vcpu_init__async, RUN_ON_CPU_NULL);
+}
+
void qemu_plugin_vcpu_exit_hook(CPUState *cpu)
{
bool success;
--
2.39.2
[PULL 40/40] tests/tcg/aarch64: Add MTE gdbstub tests
From: Gustavo Romero
Add tests to exercise the MTE stubs. The tests will only run if a
version of GDB that supports MTE is available in the test environment.
Signed-off-by: Gustavo Romero
[AJB: re-base and checkpatch fixes]
Message-Id: <20240628050850.536447-12-gustavo.rom...@linaro.org>
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-41-alex.ben...@linaro.org>
diff --git a/configure b/configure
index 8b6a2f16ce..019fcbd0ef 100755
--- a/configure
+++ b/configure
@@ -1673,6 +1673,10 @@ for target in $target_list; do
echo "GDB=$gdb_bin" >> $config_target_mak
fi
+ if test "${arch}" = "aarch64" && version_ge ${gdb_version##* } 15.0; then
+ echo "GDB_HAS_MTE=y" >> $config_target_mak
+ fi
+
echo "run-tcg-tests-$target: $qemu\$(EXESUF)" >> Makefile.prereqs
tcg_tests_targets="$tcg_tests_targets $target"
fi
diff --git a/tests/tcg/aarch64/mte-8.c b/tests/tcg/aarch64/mte-8.c
new file mode 100644
index 00..808135ba43
--- /dev/null
+++ b/tests/tcg/aarch64/mte-8.c
@@ -0,0 +1,99 @@
+/*
+ * To be compiled with -march=armv8.5-a+memtag
+ *
+ * This test is adapted from a Linux test. Please see:
+ *
+ *
https://www.kernel.org/doc/html/next/arch/arm64/memory-tagging-extension.html#example-of-correct-usage
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+/*
+ * From arch/arm64/include/uapi/asm/hwcap.h
+ */
+#define HWCAP2_MTE (1 << 18)
+
+/*
+ * From arch/arm64/include/uapi/asm/mman.h
+ */
+#define PROT_MTE 0x20
+
+/*
+ * Insert a random logical tag into the given pointer.
+ */
+#define insert_random_tag(ptr) ({ \
+uint64_t __val; \
+asm("irg %0, %1" : "=r" (__val) : "r" (ptr)); \
+__val; \
+})
+
+/*
+ * Set the allocation tag on the destination address.
+ */
+#define set_tag(tagged_addr) do { \
+asm volatile("stg %0, [%0]" : : "r" (tagged_addr) : "memory"); \
+} while (0)
+
+
+int main(int argc, char *argv[])
+{
+unsigned char *a;
+unsigned long page_sz = sysconf(_SC_PAGESIZE);
+unsigned long hwcap2 = getauxval(AT_HWCAP2);
+
+/* check if MTE is present */
+if (!(hwcap2 & HWCAP2_MTE)) {
+return EXIT_FAILURE;
+}
+
+/*
+ * Enable the tagged address ABI, synchronous or asynchronous MTE
+ * tag check faults (based on per-CPU preference) and allow all
+ * non-zero tags in the randomly generated set.
+ */
+if (prctl(PR_SET_TAGGED_ADDR_CTRL,
+ PR_TAGGED_ADDR_ENABLE | PR_MTE_TCF_SYNC | PR_MTE_TCF_ASYNC |
+ (0xfffe << PR_MTE_TAG_SHIFT),
+ 0, 0, 0)) {
+perror("prctl() failed");
+return EXIT_FAILURE;
+}
+
+a = mmap(0, page_sz, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+if (a == MAP_FAILED) {
+perror("mmap() failed");
+return EXIT_FAILURE;
+}
+
+printf("a[] address is %p\n", a);
+
+/*
+ * Enable MTE on the above anonymous mmap. The flag could be passed
+ * directly to mmap() and skip this step.
+ */
+if (mprotect(a, page_sz, PROT_READ | PROT_WRITE | PROT_MTE)) {
+perror("mprotect() failed");
+return EXIT_FAILURE;
+}
+
+/* access with the default tag (0) */
+a[0] = 1;
+a[1] = 2;
+
+printf("a[0] = %hhu a[1] = %hhu\n", a[0], a[1]);
+
+/* set the logical and allocation tags */
+a = (unsigned char *)insert_random_tag(a);
+set_tag(a);
+
+printf("%p\n", a);
+
+return 0;
+}
diff --git a/tests/tcg/aarch64/Makefile.target
b/tests/tcg/aarch64/Makefile.target
index ad1774c2ce..b53218e115 100644
--- a/tests/tcg/aarch64/Makefile.target
+++ b/tests/tcg/aarch64/Makefile.target
@@ -64,7 +64,7 @@ AARCH64_TESTS += bti-2
# MTE Tests
ifneq ($(CROSS_CC_HAS_ARMV8_MTE),)
-AARCH64_TESTS += mte-1 mte-2 mte-3 mte-4 mte-5 mte-6 mte-7
+AARCH64_TESTS += mte-1 mte-2 mte-3 mte-4 mte-5 mte-6 mte-7 mte-8
mte-%: CFLAGS += $(CROSS_CC_HAS_ARMV8_MTE)
endif
@@ -131,6 +131,18 @@ run-gdbstub-sve-ioctls: sve-ioctls
basic gdbstub SVE ZLEN support)
EXTRA_RUNS += run-gdbstub-sysregs run-gdbstub-sve-ioctls
+
+ifeq ($(GDB_HAS_MTE),y)
+run-gdbstub-mte: mte-8
+ $(call run-test, $@, $(GDB_SCRIPT) \
+ --gdb $(GDB) \
+ --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
+ --bin $< --test $(AARCH64_SRC)/gdbstub/test-mte.py, \
+ gdbstub MTE support)
+
+EXTRA_RUNS += run-gdbstub-mte
+endif
+
endif
endif
diff --git a/tests/tcg/aarch64/gdbstub/test-mte.py
b/tests/tcg/aarch64/gdbstub/test-mte.py
new file mode 100644
index
[PULL 05/40] tests/docker: Specify --userns keep-id for Podman
From: Akihiko Odaki Previously we are always specifying -u $(UID) to match the UID in the container with one outside. This causes a problem with rootless Podman. Rootless Podman remaps user IDs in the container to ones controllable for the current user outside. The -u option instructs Podman to use a specified UID in the container but does not affect the UID remapping. Therefore, the UID in the container can be remapped to some other UID outside the container. This can make the access to bind-mounted volumes fail because the remapped UID mismatches with the owner of the directories. Replace -u $(UID) with --userns keep-id, which fixes the UID remapping. This change is limited to Podman because Docker does not support --userns keep-id. Signed-off-by: Akihiko Odaki Message-Id: <20240626-podman-v1-1-f8c8daf2b...@daynix.com> Signed-off-by: Alex Bennée Message-Id: <20240705084047.857176-6-alex.ben...@linaro.org> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include index 8df50a0ca0..708e3a72fb 100644 --- a/tests/docker/Makefile.include +++ b/tests/docker/Makefile.include @@ -207,7 +207,12 @@ docker-run: docker-qemu-src $(call quiet-command, \ $(RUNC) run \ --rm\ - $(if $(NOUSER),,-u $(UID)) \ + $(if $(NOUSER),,\ + $(if $(filter docker,$(RUNC)), \ + -u $(UID), \ + --userns keep-id\ + ) \ + ) \ --security-opt seccomp=unconfined \ $(if $(DEBUG),-ti,) \ $(if $(NETWORK),$(if $(subst $(NETWORK),,1),--net=$(NETWORK)),--net=none) \ -- 2.39.2
[PULL 17/40] tests/tcg/arm: Use -march and -mfpu for fcvt
From: Richard Henderson Clang requires the architecture to be set properly in order to assemble the half-precision instructions. Signed-off-by: Richard Henderson Reviewed-by: Akihiko Odaki Message-Id: <20240630190050.160642-13-richard.hender...@linaro.org> Signed-off-by: Alex Bennée Message-Id: <20240705084047.857176-18-alex.ben...@linaro.org> diff --git a/tests/tcg/arm/Makefile.target b/tests/tcg/arm/Makefile.target index 95f891bf8c..8e287191af 100644 --- a/tests/tcg/arm/Makefile.target +++ b/tests/tcg/arm/Makefile.target @@ -29,8 +29,8 @@ test-arm-iwmmxt: test-arm-iwmmxt.S # Float-convert Tests ARM_TESTS += fcvt -fcvt: LDFLAGS+=-lm -# fcvt: CFLAGS+=-march=armv8.2-a+fp16 -mfpu=neon-fp-armv8 +fcvt: LDFLAGS += -lm +fcvt: CFLAGS += -march=armv8.2-a+fp16 -mfpu=neon-fp-armv8 run-fcvt: fcvt $(call run-test,fcvt,$(QEMU) $<) $(call diff-out,fcvt,$(ARM_SRC)/fcvt.ref) -- 2.39.2
[PULL 01/40] tests/lcitool: fix debian-i686-cross toolchain prefix
I guess we never noticed and tried to build with this cross image. Fix
the toolchain prefix so we actually build 32 bit images.
Reviewed-by: Richard Henderson
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-2-alex.ben...@linaro.org>
diff --git a/tests/docker/dockerfiles/debian-i686-cross.docker
b/tests/docker/dockerfiles/debian-i686-cross.docker
index f1e5b0b877..f4ef054a2e 100644
--- a/tests/docker/dockerfiles/debian-i686-cross.docker
+++ b/tests/docker/dockerfiles/debian-i686-cross.docker
@@ -169,7 +169,7 @@ endian = 'little'\n" >
/usr/local/share/meson/cross/i686-linux-gnu && \
ENV ABI "i686-linux-gnu"
ENV MESON_OPTS "--cross-file=i686-linux-gnu"
-ENV QEMU_CONFIGURE_OPTS --cross-prefix=x86_64-linux-gnu-
+ENV QEMU_CONFIGURE_OPTS --cross-prefix=i686-linux-gnu-
ENV DEF_TARGET_LIST
x86_64-softmmu,x86_64-linux-user,i386-softmmu,i386-linux-user
# As a final step configure the user (if env is defined)
ARG USER
diff --git a/tests/lcitool/refresh b/tests/lcitool/refresh
index b25e3ac4dd..ac803e34f1 100755
--- a/tests/lcitool/refresh
+++ b/tests/lcitool/refresh
@@ -167,7 +167,7 @@ try:
generate_dockerfile("debian-i686-cross", "debian-11",
cross="i686",
-trailer=cross_build("x86_64-linux-gnu-",
+trailer=cross_build("i686-linux-gnu-",
"x86_64-softmmu,"
"x86_64-linux-user,"
"i386-softmmu,i386-linux-user"))
--
2.39.2
[PULL 09/40] tests/tcg/aarch64: Explicitly specify register width
From: Akihiko Odaki
clang version 18.1.6 assumes a register is 64-bit by default and
complains if a 32-bit value is given. Explicitly specify register width
when passing a 32-bit value.
Signed-off-by: Akihiko Odaki
Reviewed-by: Philippe Mathieu-Daudé
Message-Id: <20240627-tcg-v2-3-1690a8133...@daynix.com>
Reviewed-by: Akihiko Odaki
Message-Id: <20240630190050.160642-5-richard.hender...@linaro.org>
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-10-alex.ben...@linaro.org>
diff --git a/tests/tcg/aarch64/bti-1.c b/tests/tcg/aarch64/bti-1.c
index 99a879af23..1fada8108d 100644
--- a/tests/tcg/aarch64/bti-1.c
+++ b/tests/tcg/aarch64/bti-1.c
@@ -17,15 +17,15 @@ static void skip2_sigill(int sig, siginfo_t *info,
ucontext_t *uc)
#define BTI_JC"hint #38"
#define BTYPE_1(DEST) \
-asm("mov %0,#1; adr x16, 1f; br x16; 1: " DEST "; mov %0,#0" \
+asm("mov %w0,#1; adr x16, 1f; br x16; 1: " DEST "; mov %w0,#0" \
: "=r"(skipped) : : "x16")
#define BTYPE_2(DEST) \
-asm("mov %0,#1; adr x16, 1f; blr x16; 1: " DEST "; mov %0,#0" \
+asm("mov %w0,#1; adr x16, 1f; blr x16; 1: " DEST "; mov %w0,#0" \
: "=r"(skipped) : : "x16", "x30")
#define BTYPE_3(DEST) \
-asm("mov %0,#1; adr x15, 1f; br x15; 1: " DEST "; mov %0,#0" \
+asm("mov %w0,#1; adr x15, 1f; br x15; 1: " DEST "; mov %w0,#0" \
: "=r"(skipped) : : "x15")
#define TEST(WHICH, DEST, EXPECT) \
diff --git a/tests/tcg/aarch64/bti-3.c b/tests/tcg/aarch64/bti-3.c
index 8c534c09d7..6a3bd037bc 100644
--- a/tests/tcg/aarch64/bti-3.c
+++ b/tests/tcg/aarch64/bti-3.c
@@ -11,15 +11,15 @@ static void skip2_sigill(int sig, siginfo_t *info,
ucontext_t *uc)
}
#define BTYPE_1() \
-asm("mov %0,#1; adr x16, 1f; br x16; 1: hint #25; mov %0,#0" \
+asm("mov %w0,#1; adr x16, 1f; br x16; 1: hint #25; mov %w0,#0" \
: "=r"(skipped) : : "x16", "x30")
#define BTYPE_2() \
-asm("mov %0,#1; adr x16, 1f; blr x16; 1: hint #25; mov %0,#0" \
+asm("mov %w0,#1; adr x16, 1f; blr x16; 1: hint #25; mov %w0,#0" \
: "=r"(skipped) : : "x16", "x30")
#define BTYPE_3() \
-asm("mov %0,#1; adr x15, 1f; br x15; 1: hint #25; mov %0,#0" \
+asm("mov %w0,#1; adr x15, 1f; br x15; 1: hint #25; mov %w0,#0" \
: "=r"(skipped) : : "x15", "x30")
#define TEST(WHICH, EXPECT) \
--
2.39.2
[PULL 11/40] tests/tcg/aarch64: Do not use x constraint
From: Akihiko Odaki
clang version 18.1.6 does not support x constraint for AArch64.
Use w instead.
Signed-off-by: Akihiko Odaki
Message-Id: <20240627-tcg-v2-5-1690a8133...@daynix.com>
Reviewed-by: Akihiko Odaki
Message-Id: <20240630190050.160642-7-richard.hender...@linaro.org>
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-12-alex.ben...@linaro.org>
diff --git a/tests/tcg/arm/fcvt.c b/tests/tcg/arm/fcvt.c
index 7ac47b564e..f631197287 100644
--- a/tests/tcg/arm/fcvt.c
+++ b/tests/tcg/arm/fcvt.c
@@ -126,7 +126,7 @@ static void convert_single_to_half(void)
asm("vcvtb.f16.f32 %0, %1" : "=t" (output) : "x" (input));
#else
uint16_t output;
-asm("fcvt %h0, %s1" : "=w" (output) : "x" (input));
+asm("fcvt %h0, %s1" : "=w" (output) : "w" (input));
#endif
print_half_number(i, output);
}
@@ -149,7 +149,7 @@ static void convert_single_to_double(void)
#if defined(__arm__)
asm("vcvt.f64.f32 %P0, %1" : "=w" (output) : "t" (input));
#else
-asm("fcvt %d0, %s1" : "=w" (output) : "x" (input));
+asm("fcvt %d0, %s1" : "=w" (output) : "w" (input));
#endif
print_double_number(i, output);
}
@@ -244,7 +244,7 @@ static void convert_double_to_half(void)
/* asm("vcvtb.f16.f64 %0, %P1" : "=t" (output) : "x" (input)); */
output = input;
#else
-asm("fcvt %h0, %d1" : "=w" (output) : "x" (input));
+asm("fcvt %h0, %d1" : "=w" (output) : "w" (input));
#endif
print_half_number(i, output);
}
@@ -267,7 +267,7 @@ static void convert_double_to_single(void)
#if defined(__arm__)
asm("vcvt.f32.f64 %0, %P1" : "=w" (output) : "x" (input));
#else
-asm("fcvt %s0, %d1" : "=w" (output) : "x" (input));
+asm("fcvt %s0, %d1" : "=w" (output) : "w" (input));
#endif
print_single_number(i, output);
@@ -335,7 +335,7 @@ static void convert_half_to_double(void)
/* asm("vcvtb.f64.f16 %P0, %1" : "=w" (output) : "t" (input)); */
output = input;
#else
-asm("fcvt %d0, %h1" : "=w" (output) : "x" (input));
+asm("fcvt %d0, %h1" : "=w" (output) : "w" (input));
#endif
print_double_number(i, output);
}
@@ -357,7 +357,7 @@ static void convert_half_to_single(void)
#if defined(__arm__)
asm("vcvtb.f32.f16 %0, %1" : "=w" (output) : "x" ((uint32_t)input));
#else
-asm("fcvt %s0, %h1" : "=w" (output) : "x" (input));
+asm("fcvt %s0, %h1" : "=w" (output) : "w" (input));
#endif
print_single_number(i, output);
}
@@ -380,7 +380,7 @@ static void convert_half_to_integer(void)
/* asm("vcvt.s32.f16 %0, %1" : "=t" (output) : "t" (input)); v8.2*/
output = input;
#else
-asm("fcvt %s0, %h1" : "=w" (output) : "x" (input));
+asm("fcvt %s0, %h1" : "=w" (output) : "w" (input));
#endif
print_int64(i, output);
}
--
2.39.2
[PULL 10/40] tests/tcg/aarch64: Fix irg operand type
From: Akihiko Odaki
irg expects 64-bit integers. Passing a 32-bit integer results in
compilation failure with clang version 18.1.6.
Signed-off-by: Akihiko Odaki
Message-Id: <20240627-tcg-v2-4-1690a8133...@daynix.com>
Reviewed-by: Akihiko Odaki
Message-Id: <20240630190050.160642-6-richard.hender...@linaro.org>
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-11-alex.ben...@linaro.org>
diff --git a/tests/tcg/aarch64/mte-1.c b/tests/tcg/aarch64/mte-1.c
index 88dcd617ad..146cad4a04 100644
--- a/tests/tcg/aarch64/mte-1.c
+++ b/tests/tcg/aarch64/mte-1.c
@@ -15,7 +15,7 @@ int main(int ac, char **av)
enable_mte(PR_MTE_TCF_NONE);
p0 = alloc_mte_mem(sizeof(*p0));
-asm("irg %0,%1,%2" : "=r"(p1) : "r"(p0), "r"(1));
+asm("irg %0,%1,%2" : "=r"(p1) : "r"(p0), "r"(1l));
assert(p1 != p0);
asm("subp %0,%1,%2" : "=r"(c) : "r"(p0), "r"(p1));
assert(c == 0);
--
2.39.2
[PULL 13/40] tests/tcg/arm: Fix fcvt result messages
From: Akihiko Odaki
The test cases for "converting double-precision to single-precision"
emits float but the result variable was typed as uint32_t and corrupted
the printed values. Propertly type it as float.
Signed-off-by: Akihiko Odaki
Fixes: 8ec8a55e3fc9 ("tests/tcg/arm: add fcvt test cases for AArch32/64")
Message-Id: <20240627-tcg-v2-1-1690a8133...@daynix.com>
[rth: Update arm ref file as well]
Signed-off-by: Richard Henderson
Reviewed-by: Akihiko Odaki
Message-Id: <20240630190050.160642-9-richard.hender...@linaro.org>
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-14-alex.ben...@linaro.org>
diff --git a/tests/tcg/arm/fcvt.c b/tests/tcg/arm/fcvt.c
index f631197287..157790e679 100644
--- a/tests/tcg/arm/fcvt.c
+++ b/tests/tcg/arm/fcvt.c
@@ -258,7 +258,7 @@ static void convert_double_to_single(void)
for (i = 0; i < ARRAY_SIZE(double_numbers); ++i) {
double input = double_numbers[i].d;
-uint32_t output;
+float output;
feclearexcept(FE_ALL_EXCEPT);
diff --git a/tests/tcg/aarch64/fcvt.ref b/tests/tcg/aarch64/fcvt.ref
index e7af24dc58..2726b41063 100644
--- a/tests/tcg/aarch64/fcvt.ref
+++ b/tests/tcg/aarch64/fcvt.ref
@@ -211,45 +211,45 @@ Converting double-precision to half-precision
40 HALF: 0x7f00 (0x1 => INVALID)
Converting double-precision to single-precision
00 DOUBLE: nan / 0x007ff4 (0 => OK)
-00 SINGLE: 2.145386496000e+09 / 0x4effc000 (0x1 => INVALID)
+00 SINGLE: nan / 0x7fe0 (0x1 => INVALID)
01 DOUBLE: -nan / 0x00fff8 (0 => OK)
-01 SINGLE: 4.290772992000e+09 / 0x4f7fc000 (0 => OK)
+01 SINGLE: -nan / 0xffc0 (0 => OK)
02 DOUBLE: -inf / 0x00fff0 (0 => OK)
-02 SINGLE: 4.286578688000e+09 / 0x4f7f8000 (0 => OK)
+02 SINGLE: -inf / 0xff80 (0 => OK)
03 DOUBLE: -1.79769313486231570815e+308 / 0x00ffef (0 => OK)
-03 SINGLE: 4.286578688000e+09 / 0x4f7f8000 (0x14 => OVERFLOW
INEXACT )
+03 SINGLE: -inf / 0xff80 (0x14 => OVERFLOW INEXACT )
04 DOUBLE: -3.40282346638528859812e+38 / 0x00c7efe000 (0 => OK)
-04 SINGLE: 4.286578688000e+09 / 0x4f7f8000 (0x10 =>INEXACT )
+04 SINGLE: -3.40282346638528859812e+38 / 0xff7f (0 => OK)
05 DOUBLE: -3.40282346638528859812e+38 / 0x00c7efe000 (0 => OK)
-05 SINGLE: 4.286578688000e+09 / 0x4f7f8000 (0x10 =>INEXACT )
+05 SINGLE: -3.40282346638528859812e+38 / 0xff7f (0 => OK)
06 DOUBLE: -1.11107529e+31 / 0x00c661874b135ff654 (0 => OK)
-06 SINGLE: 4.077664768000e+09 / 0x4f730c3a (0x10 =>INEXACT )
+06 SINGLE: -1.1114769645909791e+31 / 0xf30c3a59 (0x10 =>INEXACT )
07 DOUBLE: -1.11099085e+30 / 0x00c62c0bab523323b9 (0 => OK)
-07 SINGLE: 4.04962432e+09 / 0x4f71605d (0x10 =>INEXACT )
+07 SINGLE: -1.1113258488635273e+30 / 0xf1605d5b (0x10 =>INEXACT )
08 DOUBLE: -2.e+00 / 0x00c000 (0 => OK)
-08 SINGLE: 3.221225472000e+09 / 0x4f40 (0 => OK)
+08 SINGLE: -2.e+00 / 0xc000 (0 => OK)
09 DOUBLE: -1.e+00 / 0x00bff0 (0 => OK)
-09 SINGLE: 3.212836864000e+09 / 0x4f3f8000 (0 => OK)
+09 SINGLE: -1.e+00 / 0xbf80 (0 => OK)
10 DOUBLE: -2.22507385850720138309e-308 / 0x008010 (0 => OK)
-10 SINGLE: 2.147483648000e+09 / 0x4f00 (0x18 => UNDERFLOW
INEXACT )
+10 SINGLE: -0.e+00 / 0x8000 (0x18 => UNDERFLOW
INEXACT )
11 DOUBLE: -1.17549435082228750797e-38 / 0x00b810 (0 => OK)
-11 SINGLE: 2.155872256000e+09 / 0x4f008000 (0 => OK)
+11 SINGLE: -1.17549435082228750797e-38 / 0x8080 (0 => OK)
12 DOUBLE: 0.e+00 / (0 => OK)
12 SINGLE: 0.e+00 / 00 (0 => OK)
13 DOUBLE: 1.17549435082228750797e-38 / 0x003810 (0 => OK)
-13 SINGLE: 8.38860800e+06 / 0x4b00 (0 => OK)
+13 SINGLE: 1.17549435082228750797e-38 / 0x0080 (0 => OK)
14 DOUBLE: 2.9802322400013061e-08 / 0x003e60001c5f68 (0 => OK)
-14 SINGLE: 8.55638016e+08 / 0x4e4c (0x10 =>INEXACT )
+14 SINGLE: 2.98023223876953125000e-08 / 0x3300 (0x10 =>INEXACT )
15 DOUBLE: 5.960460015661e-08 / 0x003e6e6cb2fa82 (0 => OK)
-15 SINGLE: 8.64026624e+08 / 0x4e4e (0x10 =>INEXACT )
+15 SINGLE: 5.96045985901128005935e-08 / 0x3373 (0x10 =>INEXACT )
16 DOUBLE: 6.097559994299e-05 / 0x003f0ff801a9af58a1 (0 => OK)
-16 SINGLE: 9.47896320e+08 / 0x4e61ff00 (0x10 =>INEXACT )
+16 SINGLE: 6.09755988989491015673e-05 / 0x387fc00d (0x10 =>INEXACT )
17 DOUBLE: 6.1035200
[PULL 02/40] testing: restore some testing for i686
The commit 4f9a8315e6 (gitlab-ci.d/crossbuilds: Drop the i386 system emulation job) was a little too aggressive dropping testing for 32 bit system builds. Partially revert but using the debian-i686 cross build images this time as fedora has deprecated the 32 bit stuff. As the SEV breakage gets in the way and its TCG issues we want to catch I've added --disable-kvm to the build. Reported-by: Richard Henderson Suggested-by: Thomas Huth Signed-off-by: Alex Bennée Message-Id: <20240705084047.857176-3-alex.ben...@linaro.org> diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml index 47bdb99b5b..3de0341afe 100644 --- a/.gitlab-ci.d/crossbuilds.yml +++ b/.gitlab-ci.d/crossbuilds.yml @@ -37,6 +37,17 @@ cross-arm64-kvm-only: IMAGE: debian-arm64-cross EXTRA_CONFIGURE_OPTS: --disable-tcg --without-default-features +cross-i686-system: + extends: +- .cross_system_build_job +- .cross_test_artifacts + needs: +job: i686-debian-cross-container + variables: +IMAGE: debian-i686-cross +EXTRA_CONFIGURE_OPTS: --disable-kvm +MAKE_CHECK_ARGS: check-qtest + cross-i686-user: extends: - .cross_user_build_job -- 2.39.2
[PULL 35/40] target/arm: Factor out code for setting MTE TCF0 field
From: Gustavo Romero
Factor out the code used for setting the MTE TCF0 field from the prctl
code into a convenient function. Other subsystems, like gdbstub, need to
set this field as well, so keep it as a separate function to avoid
duplication and ensure consistency in how this field is set across the
board.
Signed-off-by: Gustavo Romero
Message-Id: <20240628050850.536447-7-gustavo.rom...@linaro.org>
[AJB: clean-up includes, move MTE defines]
Reviewed-by: Manos Pitsidianakis
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-36-alex.ben...@linaro.org>
diff --git a/linux-user/aarch64/mte_user_helper.h
b/linux-user/aarch64/mte_user_helper.h
new file mode 100644
index 00..8685e5175a
--- /dev/null
+++ b/linux-user/aarch64/mte_user_helper.h
@@ -0,0 +1,32 @@
+/*
+ * ARM MemTag convenience functions.
+ *
+ * This code is licensed under the GNU GPL v2 or later.
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
+#ifndef AARCH64_MTE_USER_HELPER_H
+#define AARCH64_MTE USER_HELPER_H
+
+#ifndef PR_MTE_TCF_SHIFT
+# define PR_MTE_TCF_SHIFT 1
+# define PR_MTE_TCF_NONE(0UL << PR_MTE_TCF_SHIFT)
+# define PR_MTE_TCF_SYNC(1UL << PR_MTE_TCF_SHIFT)
+# define PR_MTE_TCF_ASYNC (2UL << PR_MTE_TCF_SHIFT)
+# define PR_MTE_TCF_MASK(3UL << PR_MTE_TCF_SHIFT)
+# define PR_MTE_TAG_SHIFT 3
+# define PR_MTE_TAG_MASK(0xUL << PR_MTE_TAG_SHIFT)
+#endif
+
+/**
+ * arm_set_mte_tcf0 - Set TCF0 field in SCTLR_EL1 register
+ * @env: The CPU environment
+ * @value: The value to be set for the Tag Check Fault in EL0 field.
+ *
+ * Only SYNC and ASYNC modes can be selected. If ASYMM mode is given, the SYNC
+ * mode is selected instead. So, there is no way to set the ASYMM mode.
+ */
+void arm_set_mte_tcf0(CPUArchState *env, abi_long value);
+
+#endif /* AARCH64_MTE_USER_HELPER_H */
diff --git a/linux-user/aarch64/target_prctl.h
b/linux-user/aarch64/target_prctl.h
index aa8e203c15..ed75b9e4b5 100644
--- a/linux-user/aarch64/target_prctl.h
+++ b/linux-user/aarch64/target_prctl.h
@@ -7,6 +7,7 @@
#define AARCH64_TARGET_PRCTL_H
#include "target/arm/cpu-features.h"
+#include "mte_user_helper.h"
static abi_long do_prctl_sve_get_vl(CPUArchState *env)
{
@@ -173,26 +174,7 @@ static abi_long do_prctl_set_tagged_addr_ctrl(CPUArchState
*env, abi_long arg2)
env->tagged_addr_enable = arg2 & PR_TAGGED_ADDR_ENABLE;
if (cpu_isar_feature(aa64_mte, cpu)) {
-/*
- * Write PR_MTE_TCF to SCTLR_EL1[TCF0].
- *
- * The kernel has a per-cpu configuration for the sysadmin,
- * /sys/devices/system/cpu/cpu/mte_tcf_preferred,
- * which qemu does not implement.
- *
- * Because there is no performance difference between the modes, and
- * because SYNC is most useful for debugging MTE errors, choose SYNC
- * as the preferred mode. With this preference, and the way the API
- * uses only two bits, there is no way for the program to select
- * ASYMM mode.
- */
-unsigned tcf = 0;
-if (arg2 & PR_MTE_TCF_SYNC) {
-tcf = 1;
-} else if (arg2 & PR_MTE_TCF_ASYNC) {
-tcf = 2;
-}
-env->cp15.sctlr_el[1] = deposit64(env->cp15.sctlr_el[1], 38, 2, tcf);
+arm_set_mte_tcf0(env, arg2);
/*
* Write PR_MTE_TAG to GCR_EL1[Exclude].
diff --git a/linux-user/aarch64/mte_user_helper.c
b/linux-user/aarch64/mte_user_helper.c
new file mode 100644
index 00..a5b1c8503b
--- /dev/null
+++ b/linux-user/aarch64/mte_user_helper.c
@@ -0,0 +1,35 @@
+/*
+ * ARM MemTag convenience functions.
+ *
+ * This code is licensed under the GNU GPL v2 or later.
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu.h"
+#include "mte_user_helper.h"
+
+void arm_set_mte_tcf0(CPUArchState *env, abi_long value)
+{
+/*
+ * Write PR_MTE_TCF to SCTLR_EL1[TCF0].
+ *
+ * The kernel has a per-cpu configuration for the sysadmin,
+ * /sys/devices/system/cpu/cpu/mte_tcf_preferred,
+ * which qemu does not implement.
+ *
+ * Because there is no performance difference between the modes, and
+ * because SYNC is most useful for debugging MTE errors, choose SYNC
+ * as the preferred mode. With this preference, and the way the API
+ * uses only two bits, there is no way for the program to select
+ * ASYMM mode.
+ */
+unsigned tcf = 0;
+if (value & PR_MTE_TCF_SYNC) {
+tcf = 1;
+} else if (value & PR_MTE_TCF_ASYNC) {
+tcf = 2;
+}
+env->cp15.sctlr_el[1] = deposit64(env->cp15.sctlr_el[1], 38, 2, tcf);
+}
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index e2804312fc..b8c278b91d 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -6281,15 +6281,6 @@ abi_long do_arch_prc
[PULL 15/40] tests/tcg/arm: Use -fno-integrated-as for test-arm-iwmmxt
From: Richard Henderson Clang does not support IWMXT instructions. Fall back to the external assembler. Signed-off-by: Richard Henderson Reviewed-by: Akihiko Odaki Message-Id: <20240630190050.160642-11-richard.hender...@linaro.org> Signed-off-by: Alex Bennée Message-Id: <20240705084047.857176-16-alex.ben...@linaro.org> diff --git a/tests/tcg/arm/Makefile.target b/tests/tcg/arm/Makefile.target index 0a1965fce7..95f891bf8c 100644 --- a/tests/tcg/arm/Makefile.target +++ b/tests/tcg/arm/Makefile.target @@ -8,6 +8,11 @@ ARM_SRC=$(SRC_PATH)/tests/tcg/arm # Set search path for all sources VPATH += $(ARM_SRC) +config-cc.mak: Makefile + $(quiet-@)( \ + $(call cc-option,-fno-integrated-as, CROSS_CC_HAS_FNIA)) 3> config-cc.mak +-include config-cc.mak + float_madds: CFLAGS+=-mfpu=neon-vfpv4 # Basic Hello World @@ -17,7 +22,8 @@ hello-arm: LDFLAGS+=-nostdlib # IWMXT floating point extensions ARM_TESTS += test-arm-iwmmxt -test-arm-iwmmxt: CFLAGS+=-marm -march=iwmmxt -mabi=aapcs -mfpu=fpv4-sp-d16 +# Clang assembler does not support IWMXT, so use the external assembler. +test-arm-iwmmxt: CFLAGS += -marm -march=iwmmxt -mabi=aapcs -mfpu=fpv4-sp-d16 $(CROSS_CC_HAS_FNIA) test-arm-iwmmxt: test-arm-iwmmxt.S $(CC) $(CFLAGS) $< -o $@ $(LDFLAGS) -- 2.39.2
[PULL 12/40] tests/tcg/aarch64: Add -fno-integrated-as for sme
From: Richard Henderson The only use of SME is inline assembly. Both gcc and clang only support SME with very recent releases; by deferring detection to the assembler we get better test coverage. Signed-off-by: Richard Henderson Reviewed-by: Akihiko Odaki Message-Id: <20240630190050.160642-8-richard.hender...@linaro.org> Signed-off-by: Alex Bennée Message-Id: <20240705084047.857176-13-alex.ben...@linaro.org> diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target index 11ccde5579..ad1774c2ce 100644 --- a/tests/tcg/aarch64/Makefile.target +++ b/tests/tcg/aarch64/Makefile.target @@ -20,6 +20,7 @@ run-fcvt: fcvt config-cc.mak: Makefile $(quiet-@)( \ + fnia=`$(call cc-test,-fno-integrated-as) && echo -fno-integrated-as`; \ $(call cc-option,-march=armv8.1-a+sve, CROSS_CC_HAS_SVE); \ $(call cc-option,-march=armv8.1-a+sve2, CROSS_CC_HAS_SVE2); \ $(call cc-option,-march=armv8.2-a, CROSS_CC_HAS_ARMV8_2); \ @@ -27,7 +28,7 @@ config-cc.mak: Makefile $(call cc-option,-march=armv8.5-a, CROSS_CC_HAS_ARMV8_5); \ $(call cc-option,-mbranch-protection=standard, CROSS_CC_HAS_ARMV8_BTI); \ $(call cc-option,-march=armv8.5-a+memtag, CROSS_CC_HAS_ARMV8_MTE); \ - $(call cc-option,-Wa$(COMMA)-march=armv9-a+sme, CROSS_AS_HAS_ARMV9_SME)) 3> config-cc.mak + $(call cc-option,-Wa$(COMMA)-march=armv9-a+sme $$fnia, CROSS_AS_HAS_ARMV9_SME)) 3> config-cc.mak -include config-cc.mak ifneq ($(CROSS_CC_HAS_ARMV8_2),) -- 2.39.2
[PULL 00/40] maintainer updates for testing, plugins and gdbstub
The following changes since commit 5915139aba1646220630596de30c673528e047c9: Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging (2024-07-04 09:16:07 -0700) are available in the Git repository at: https://gitlab.com/stsquad/qemu.git tags/pull-maintainer-july24-050724-1 for you to fetch changes up to 340ca46b681b1e9cac1643a7fd964947aeb68a56: tests/tcg/aarch64: Add MTE gdbstub tests (2024-07-05 12:35:36 +0100) Updates for testing, plugins, gdbstub - restore some 32 bit host builds and testing - move some physmem tracepoint definitions - use --userns keep-id for podman builds - cleanup check-tcg compiler flag checking for Arm - fix some casting in fcvt test - tweak check-tcg inline asm for clang - suppress some invalid clang warnings - disable KVM for the TCI builds - improve the insn tracking plugin - cleanups to the lockstep plugin - free plugin data on cpu finalise - assert cpu->index assigned - move qemu_plugin_vcpu_init__async into plugin code - add support for dynamic gdb command tables - allow targets to extend gdb capabilities - enable user-mode MTE support Akihiko Odaki (6): tests/docker: Specify --userns keep-id for Podman tests/tcg/aarch64: Explicitly specify register width tests/tcg/aarch64: Fix irg operand type tests/tcg/aarch64: Do not use x constraint tests/tcg/arm: Fix fcvt result messages tests/tcg/arm: Manually register allocate half-precision numbers Alex Bennée (11): tests/lcitool: fix debian-i686-cross toolchain prefix testing: restore some testing for i686 tracepoints: move physmem trace points hw/core: ensure kernel_end never gets used undefined gitlab: don't bother with KVM for TCI builds test/plugin: make insn plugin less noisy by default test/plugins: preserve the instruction record over translations plugins/lockstep: preserve sock_path plugins/lockstep: make mixed-mode safe plugins/lockstep: mention the one-insn-per-tb option plugins/lockstep: clean-up output Gustavo Romero (11): gdbstub: Clean up process_string_cmd gdbstub: Move GdbCmdParseEntry into a new header file gdbstub: Add support for target-specific stubs target/arm: Fix exception case in allocation_tag_mem_probe target/arm: Make some MTE helpers widely available target/arm: Factor out code for setting MTE TCF0 field gdbstub: Make hex conversion function non-internal gdbstub: Pass CPU context to command handler gdbstub: Use true to set cmd_startswith gdbstub: Add support for MTE in user mode tests/tcg/aarch64: Add MTE gdbstub tests Philippe Mathieu-Daudé (3): plugins: Ensure vCPU index is assigned in init/exit hooks plugins: Free CPUPluginState before destroying vCPU state accel/tcg: Move qemu_plugin_vcpu_init__async() to plugins/ Richard Henderson (9): tests/tcg/minilib: Constify digits in print_num tests/tcg: Adjust variable defintion from cc-option tests/tcg/aarch64: Drop -fno-tree-loop-distribute-patterns tests/tcg/aarch64: Add -fno-integrated-as for sme tests/tcg/arm: Drop -N from LDFLAGS tests/tcg/arm: Use -fno-integrated-as for test-arm-iwmmxt tests/tcg/arm: Use -march and -mfpu for fcvt tests/tcg/arm: Use vmrs/vmsr instead of mcr/mrc linux-user/main: Suppress out-of-range comparison warning for clang configure | 4 + configs/targets/aarch64-linux-user.mak| 2 +- gdbstub/internals.h | 23 - include/gdbstub/commands.h| 103 include/qemu/plugin.h | 3 + linux-user/aarch64/mte_user_helper.h | 32 ++ linux-user/aarch64/target_prctl.h | 22 +- target/arm/internals.h| 6 + target/arm/tcg/mte_helper.h | 66 +++ contrib/plugins/lockstep.c| 23 +- gdbstub/gdbstub.c | 341 +++- gdbstub/syscalls.c| 7 +- gdbstub/system.c | 7 +- gdbstub/user-target.c | 25 +- gdbstub/user.c| 7 +- hw/core/cpu-common.c | 14 +- hw/core/loader-fit.c | 2 +- linux-user/aarch64/mte_user_helper.c | 35 ++ linux-user/main.c | 1 + linux-user/syscall.c | 9 - plugins/core.c| 10 +- system/physmem.c | 4 +- target/arm/cpu.c | 1 + target/
[PULL 37/40] gdbstub: Pass CPU context to command handler
From: Gustavo Romero
Allow passing the current CPU context to command handlers via user_ctx
when the handler requires it.
Signed-off-by: Gustavo Romero
Message-Id: <20240628050850.536447-9-gustavo.rom...@linaro.org>
Reviewed-by: Manos Pitsidianakis
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-38-alex.ben...@linaro.org>
diff --git a/include/gdbstub/commands.h b/include/gdbstub/commands.h
index e51f276b40..f3058f9dda 100644
--- a/include/gdbstub/commands.h
+++ b/include/gdbstub/commands.h
@@ -54,6 +54,8 @@ typedef union GdbCmdVariant {
* "stop reply" packet. The list of commands that accept such response is
* defined at the GDB Remote Serial Protocol documentation. See:
*
https://sourceware.org/gdb/onlinedocs/gdb/Stop-Reply-Packets.html#Stop-Reply-Packets.
+ *
+ * @need_cpu_context: Pass current CPU context to command handler via user_ctx.
*/
typedef struct GdbCmdParseEntry {
GdbCmdHandler handler;
@@ -61,6 +63,7 @@ typedef struct GdbCmdParseEntry {
bool cmd_startswith;
const char *schema;
bool allow_stop_reply;
+bool need_cpu_context;
} GdbCmdParseEntry;
/**
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index b1ca253f97..5c1612ed2a 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -938,6 +938,7 @@ static bool process_string_cmd(const char *data,
for (i = 0; i < num_cmds; i++) {
const GdbCmdParseEntry *cmd = [i];
+void *user_ctx = NULL;
g_assert(cmd->handler && cmd->cmd);
if ((cmd->cmd_startswith && !startswith(data, cmd->cmd)) ||
@@ -952,8 +953,12 @@ static bool process_string_cmd(const char *data,
}
}
+if (cmd->need_cpu_context) {
+user_ctx = (void *)gdbserver_state.g_cpu;
+}
+
gdbserver_state.allow_stop_reply = cmd->allow_stop_reply;
-cmd->handler(params, NULL);
+cmd->handler(params, user_ctx);
return true;
}
--
2.39.2
[PULL 08/40] tests/tcg/aarch64: Drop -fno-tree-loop-distribute-patterns
From: Richard Henderson This option is not supported by clang, and is not required in order to get sve code generation with gcc 12. Signed-off-by: Richard Henderson Reviewed-by: Akihiko Odaki Message-Id: <20240630190050.160642-4-richard.hender...@linaro.org> Signed-off-by: Alex Bennée Message-Id: <20240705084047.857176-9-alex.ben...@linaro.org> diff --git a/tests/tcg/aarch64/Makefile.softmmu-target b/tests/tcg/aarch64/Makefile.softmmu-target index 39d3f961c5..dd6d595830 100644 --- a/tests/tcg/aarch64/Makefile.softmmu-target +++ b/tests/tcg/aarch64/Makefile.softmmu-target @@ -39,7 +39,7 @@ memory: CFLAGS+=-DCHECK_UNALIGNED=1 memory-sve: memory.c $(LINK_SCRIPT) $(CRT_OBJS) $(MINILIB_OBJS) $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS) -memory-sve: CFLAGS+=-DCHECK_UNALIGNED=1 -march=armv8.1-a+sve -O3 -fno-tree-loop-distribute-patterns +memory-sve: CFLAGS+=-DCHECK_UNALIGNED=1 -march=armv8.1-a+sve -O3 TESTS+=memory-sve -- 2.39.2
[PULL 03/40] tracepoints: move physmem trace points
They don't need to be in the global trace-events file and can have a
local trace header. Also add address_space_map tracepoint for tracking
mapping behaviour.
Reviewed-by: Richard Henderson
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-4-alex.ben...@linaro.org>
diff --git a/system/physmem.c b/system/physmem.c
index 261196cde0..14aa025d41 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -53,7 +53,7 @@
#include "sysemu/hostmem.h"
#include "sysemu/hw_accel.h"
#include "sysemu/xen-mapcache.h"
-#include "trace/trace-root.h"
+#include "trace.h"
#ifdef CONFIG_FALLOCATE_PUNCH_HOLE
#include
@@ -3193,6 +3193,8 @@ void *address_space_map(AddressSpace *as,
MemoryRegion *mr;
FlatView *fv;
+trace_address_space_map(as, addr, len, is_write, *(uint32_t *) );
+
if (len == 0) {
return NULL;
}
diff --git a/system/trace-events b/system/trace-events
index 69c9044151..2ed1d59b1f 100644
--- a/system/trace-events
+++ b/system/trace-events
@@ -21,6 +21,12 @@ flatview_destroy(void *view, void *root) "%p (root %p)"
flatview_destroy_rcu(void *view, void *root) "%p (root %p)"
global_dirty_changed(unsigned int bitmask) "bitmask 0x%"PRIx32
+# physmem.c
+address_space_map(void *as, uint64_t addr, uint64_t len, bool is_write,
uint32_t attrs) "as:%p addr 0x%"PRIx64":%"PRIx64" write:%d attrs:0x%x"
+find_ram_offset(uint64_t size, uint64_t offset) "size: 0x%" PRIx64 " @ 0x%"
PRIx64
+find_ram_offset_loop(uint64_t size, uint64_t candidate, uint64_t offset,
uint64_t next, uint64_t mingap) "trying size: 0x%" PRIx64 " @ 0x%" PRIx64 ",
offset: 0x%" PRIx64" next: 0x%" PRIx64 " mingap: 0x%" PRIx64
+ram_block_discard_range(const char *rbname, void *hva, size_t length, bool
need_madvise, bool need_fallocate, int ret) "%s@%p + 0x%zx: madvise: %d
fallocate: %d ret: %d"
+
# cpus.c
vm_stop_flush_all(int ret) "ret %d"
diff --git a/trace-events b/trace-events
index dd318ed1af..9cb96f64c4 100644
--- a/trace-events
+++ b/trace-events
@@ -37,11 +37,6 @@ dma_complete(void *dbs, int ret, void *cb) "dbs=%p ret=%d
cb=%p"
dma_blk_cb(void *dbs, int ret) "dbs=%p ret=%d"
dma_map_wait(void *dbs) "dbs=%p"
-# exec.c
-find_ram_offset(uint64_t size, uint64_t offset) "size: 0x%" PRIx64 " @ 0x%"
PRIx64
-find_ram_offset_loop(uint64_t size, uint64_t candidate, uint64_t offset,
uint64_t next, uint64_t mingap) "trying size: 0x%" PRIx64 " @ 0x%" PRIx64 ",
offset: 0x%" PRIx64" next: 0x%" PRIx64 " mingap: 0x%" PRIx64
-ram_block_discard_range(const char *rbname, void *hva, size_t length, bool
need_madvise, bool need_fallocate, int ret) "%s@%p + 0x%zx: madvise: %d
fallocate: %d ret: %d"
-
# job.c
job_state_transition(void *job, int ret, const char *legal, const char *s0,
const char *s1) "job %p (ret: %d) attempting %s transition (%s-->%s)"
job_apply_verb(void *job, const char *state, const char *verb, const char
*legal) "job %p in state %s; applying verb %s (%s)"
--
2.39.2
[PULL 04/40] hw/core: ensure kernel_end never gets used undefined
Really the problem here is the return values of fit_load_[kernel|fdt]() are a little all over the place. However we don't want to somehow get through not having set kernel_end and having it just be random unused data. The compiler complained on an --enable-gcov build: In file included from ../../hw/core/loader-fit.c:20: /home/alex/lsrc/qemu.git/include/qemu/osdep.h: In function ‘load_fit’: /home/alex/lsrc/qemu.git/include/qemu/osdep.h:486:45: error: ‘kernel_end’ may be used uninitialized [-Werror=maybe-uninitialized] 486 | #define ROUND_UP(n, d) ROUND_DOWN((n) + (d) - 1, (d)) | ^ ../../hw/core/loader-fit.c:270:12: note: ‘kernel_end’ was declared here 270 | hwaddr kernel_end; |^~ Reviewed-by: Manos Pitsidianakis Signed-off-by: Alex Bennée Reviewed-by: Aleksandar Rikalo Message-Id: <20240705084047.857176-5-alex.ben...@linaro.org> diff --git a/hw/core/loader-fit.c b/hw/core/loader-fit.c index 9f20007dbb..7ccc9d5fbc 100644 --- a/hw/core/loader-fit.c +++ b/hw/core/loader-fit.c @@ -267,7 +267,7 @@ int load_fit(const struct fit_loader *ldr, const char *filename, void *opaque) const char *def_cfg_name; char path[FIT_LOADER_MAX_PATH]; int itb_size, configs, cfg_off, off; -hwaddr kernel_end; +hwaddr kernel_end = 0; int ret; itb = load_device_tree(filename, _size); -- 2.39.2
[PULL 07/40] tests/tcg: Adjust variable defintion from cc-option
From: Richard Henderson Define the variable to the compiler flag used, not "y". This avoids replication of the compiler flag itself. Signed-off-by: Richard Henderson Reviewed-by: Akihiko Odaki Message-Id: <20240630190050.160642-3-richard.hender...@linaro.org> Signed-off-by: Alex Bennée Message-Id: <20240705084047.857176-8-alex.ben...@linaro.org> diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target index f21be50d3b..cb8cfeb6da 100644 --- a/tests/tcg/Makefile.target +++ b/tests/tcg/Makefile.target @@ -49,7 +49,7 @@ quiet-command = $(call quiet-@,$2,$3)$1 cc-test = $(CC) -Werror $1 -c -o /dev/null -xc /dev/null >/dev/null 2>&1 cc-option = if $(call cc-test, $1); then \ -echo "$(TARGET_PREFIX)$1 detected" && echo "$(strip $2)=y" >&3; else \ +echo "$(TARGET_PREFIX)$1 detected" && echo "$(strip $2)=$(strip $1)" >&3; else \ echo "$(TARGET_PREFIX)$1 not detected"; fi # $1 = test name, $2 = cmd, $3 = desc diff --git a/tests/tcg/aarch64/Makefile.softmmu-target b/tests/tcg/aarch64/Makefile.softmmu-target index 4b03ef602e..39d3f961c5 100644 --- a/tests/tcg/aarch64/Makefile.softmmu-target +++ b/tests/tcg/aarch64/Makefile.softmmu-target @@ -81,7 +81,7 @@ run-memory-replay: memory-replay run-memory-record EXTRA_RUNS+=run-memory-replay ifneq ($(CROSS_CC_HAS_ARMV8_3),) -pauth-3: CFLAGS += -march=armv8.3-a +pauth-3: CFLAGS += $(CROSS_CC_HAS_ARMV8_3) else pauth-3: $(call skip-test, "BUILD of $@", "missing compiler support") diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target index 4ecbca6a41..11ccde5579 100644 --- a/tests/tcg/aarch64/Makefile.target +++ b/tests/tcg/aarch64/Makefile.target @@ -32,17 +32,17 @@ config-cc.mak: Makefile ifneq ($(CROSS_CC_HAS_ARMV8_2),) AARCH64_TESTS += dcpop -dcpop: CFLAGS += -march=armv8.2-a +dcpop: CFLAGS += $(CROSS_CC_HAS_ARMV8_2) endif ifneq ($(CROSS_CC_HAS_ARMV8_5),) AARCH64_TESTS += dcpodp -dcpodp: CFLAGS += -march=armv8.5-a +dcpodp: CFLAGS += $(CROSS_CC_HAS_ARMV8_5) endif # Pauth Tests ifneq ($(CROSS_CC_HAS_ARMV8_3),) AARCH64_TESTS += pauth-1 pauth-2 pauth-4 pauth-5 test-2375 -pauth-%: CFLAGS += -march=armv8.3-a +pauth-%: CFLAGS += $(CROSS_CC_HAS_ARMV8_3) test-2375: CFLAGS += -march=armv8.3-a run-pauth-1: QEMU_OPTS += -cpu max run-pauth-2: QEMU_OPTS += -cpu max @@ -55,7 +55,7 @@ endif # bti-1 tests the elf notes, so we require special compiler support. ifneq ($(CROSS_CC_HAS_ARMV8_BTI),) AARCH64_TESTS += bti-1 bti-3 -bti-1 bti-3: CFLAGS += -fno-stack-protector -mbranch-protection=standard +bti-1 bti-3: CFLAGS += -fno-stack-protector $(CROSS_CC_HAS_ARMV8_BTI) bti-1 bti-3: LDFLAGS += -nostdlib endif # bti-2 tests PROT_BTI, so no special compiler support required. @@ -64,12 +64,13 @@ AARCH64_TESTS += bti-2 # MTE Tests ifneq ($(CROSS_CC_HAS_ARMV8_MTE),) AARCH64_TESTS += mte-1 mte-2 mte-3 mte-4 mte-5 mte-6 mte-7 -mte-%: CFLAGS += -march=armv8.5-a+memtag +mte-%: CFLAGS += $(CROSS_CC_HAS_ARMV8_MTE) endif # SME Tests ifneq ($(CROSS_AS_HAS_ARMV9_SME),) AARCH64_TESTS += sme-outprod1 sme-smopa-1 sme-smopa-2 +sme-outprod1 sme-smopa-1 sme-smopa-2: CFLAGS += $(CROSS_AS_HAS_ARMV9_SME) endif # System Registers Tests @@ -99,7 +100,7 @@ TESTS += sha512-vector ifneq ($(CROSS_CC_HAS_SVE),) # SVE ioctl test AARCH64_TESTS += sve-ioctls -sve-ioctls: CFLAGS+=-march=armv8.1-a+sve +sve-ioctls: CFLAGS += $(CROSS_CC_HAS_SVE) sha512-sve: CFLAGS=-O3 -march=armv8.1-a+sve sha512-sve: sha512.c @@ -134,7 +135,7 @@ endif ifneq ($(CROSS_CC_HAS_SVE2),) AARCH64_TESTS += test-826 -test-826: CFLAGS+=-march=armv8.1-a+sve2 +test-826: CFLAGS += $(CROSS_CC_HAS_SVE2) endif TESTS += $(AARCH64_TESTS) -- 2.39.2
[PULL 06/40] tests/tcg/minilib: Constify digits in print_num
From: Richard Henderson
This avoids a memcpy to the stack when compiled with clang.
Since we don't enable optimization, nor provide memcpy,
this results in an undefined symbol error at link time.
Signed-off-by: Richard Henderson
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Akihiko Odaki
Message-Id: <20240630190050.160642-2-richard.hender...@linaro.org>
Signed-off-by: Alex Bennée
Message-Id: <20240705084047.857176-7-alex.ben...@linaro.org>
diff --git a/tests/tcg/minilib/printf.c b/tests/tcg/minilib/printf.c
index 10472b4f58..fb0189c2bb 100644
--- a/tests/tcg/minilib/printf.c
+++ b/tests/tcg/minilib/printf.c
@@ -27,7 +27,7 @@ static void print_str(char *s)
static void print_num(unsigned long long value, int base)
{
-char digits[] = "0123456789abcdef";
+static const char digits[] = "0123456789abcdef";
char buf[32];
int i = sizeof(buf) - 2, j;
--
2.39.2
