* [PATCH v1 00/14] plugins/next (phys addr, syscalls, lots of docs)
@ 2021-03-12 17:28 Alex Bennée
2021-03-12 17:28 ` [PATCH v1 01/14] plugins: new syscalls plugin Alex Bennée
` (13 more replies)
0 siblings, 14 replies; 31+ messages in thread
From: Alex Bennée @ 2021-03-12 17:28 UTC (permalink / raw
To: qemu-devel
Cc: robhenry, mahmoudabdalghany, aaron, cota, kuhn.chenqun,
Alex Bennée
Hi,
This is rolling up the plugins changes for 6.0. A couple of fixes and
I've also spent some time cleaning up the kernel-doc comments for the
developer manual.
The following need review:
- plugins: expand kernel-doc for memory query and instrumentation
- plugins: expand kernel-doc for instruction query and instrumentation
- plugins: expand inline exec kernel-doc documentation.
- plugins: add qemu_plugin_id_t to kernel-doc
- plugins: add qemu_plugin_cb_flags to kernel-doc
- plugins: expand the typedef kernel-docs for translation
- plugins: expand the callback typedef kernel-docs
- plugins: cleanup kernel-doc for qemu_plugin_install
- plugins: expand kernel-doc for qemu_info_t
- docs/devel: include the plugin API information from the headers
Aaron Lindsay (1):
plugins: Expose physical addresses instead of device offsets
Alex Bennée (10):
docs/devel: include the plugin API information from the headers
plugins: expand kernel-doc for qemu_info_t
plugins: cleanup kernel-doc for qemu_plugin_install
plugins: expand the callback typedef kernel-docs
plugins: expand the typedef kernel-docs for translation
plugins: add qemu_plugin_cb_flags to kernel-doc
plugins: add qemu_plugin_id_t to kernel-doc
plugins: expand inline exec kernel-doc documentation.
plugins: expand kernel-doc for instruction query and instrumentation
plugins: expand kernel-doc for memory query and instrumentation
Matthias Weckbecker (1):
plugins: new syscalls plugin
Yonggang Luo (2):
plugins: getting qemu_plugin_get_hwaddr only expose one function
prototype
plugins: Fixes typo in qemu-plugin.h
docs/devel/tcg-plugins.rst | 5 +
include/qemu/qemu-plugin.h | 230 +++++++++++++++++++++++++++++-------
contrib/plugins/hotpages.c | 2 +-
contrib/plugins/hwprofile.c | 2 +-
plugins/api.c | 25 ++--
tests/plugin/syscall.c | 49 ++++++++
tests/plugin/meson.build | 2 +-
7 files changed, 257 insertions(+), 58 deletions(-)
create mode 100644 tests/plugin/syscall.c
--
2.20.1
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v1 01/14] plugins: new syscalls plugin
2021-03-12 17:28 [PATCH v1 00/14] plugins/next (phys addr, syscalls, lots of docs) Alex Bennée
@ 2021-03-12 17:28 ` Alex Bennée
2021-03-12 17:28 ` [PATCH v1 02/14] plugins: Expose physical addresses instead of device offsets Alex Bennée
` (12 subsequent siblings)
13 siblings, 0 replies; 31+ messages in thread
From: Alex Bennée @ 2021-03-12 17:28 UTC (permalink / raw
To: qemu-devel
Cc: Matthias Weckbecker, robhenry, mahmoudabdalghany, aaron, cota,
kuhn.chenqun, Alex Bennée
From: Matthias Weckbecker <matthias@weckbecker.name>
This commit adds a new syscalls plugin that displays the syscalls
as they are executed and returned. This plugin outputs the number
of the syscall as well as the syscall return value.
Works in *-user only.
Essentially, this commit restores:
https://lists.gnu.org/archive/html/qemu-devel/2018-06/msg00846.html
by using the new QEMU plugin API.
Signed-off-by: Matthias Weckbecker <matthias@weckbecker.name>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20200812115816.4454-1-matthias@weckbecker.name>
---
tests/plugin/syscall.c | 49 ++++++++++++++++++++++++++++++++++++++++
tests/plugin/meson.build | 2 +-
2 files changed, 50 insertions(+), 1 deletion(-)
create mode 100644 tests/plugin/syscall.c
diff --git a/tests/plugin/syscall.c b/tests/plugin/syscall.c
new file mode 100644
index 0000000000..53ee2ab6c4
--- /dev/null
+++ b/tests/plugin/syscall.c
@@ -0,0 +1,49 @@
+/*
+ * Copyright (C) 2020, Matthias Weckbecker <matthias@weckbecker.name>
+ *
+ * License: GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#include <inttypes.h>
+#include <assert.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <glib.h>
+
+#include <qemu-plugin.h>
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
+
+static void vcpu_syscall(qemu_plugin_id_t id, unsigned int vcpu_index,
+ int64_t num, uint64_t a1, uint64_t a2,
+ uint64_t a3, uint64_t a4, uint64_t a5,
+ uint64_t a6, uint64_t a7, uint64_t a8)
+{
+ g_autofree gchar *out = g_strdup_printf("syscall #%" PRIi64 "\n", num);
+ qemu_plugin_outs(out);
+}
+
+static void vcpu_syscall_ret(qemu_plugin_id_t id, unsigned int vcpu_idx,
+ int64_t num, int64_t ret)
+{
+ g_autofree gchar *out;
+ out = g_strdup_printf("syscall #%" PRIi64 " returned -> %" PRIi64 "\n",
+ num, ret);
+ qemu_plugin_outs(out);
+}
+
+/* ************************************************************************* */
+
+static void plugin_exit(qemu_plugin_id_t id, void *p) {}
+
+QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
+ const qemu_info_t *info,
+ int argc, char **argv)
+{
+ qemu_plugin_register_vcpu_syscall_cb(id, vcpu_syscall);
+ qemu_plugin_register_vcpu_syscall_ret_cb(id, vcpu_syscall_ret);
+ qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
+ return 0;
+}
diff --git a/tests/plugin/meson.build b/tests/plugin/meson.build
index 1eacfa6e35..2bbfc4b19e 100644
--- a/tests/plugin/meson.build
+++ b/tests/plugin/meson.build
@@ -1,5 +1,5 @@
t = []
-foreach i : ['bb', 'empty', 'insn', 'mem']
+foreach i : ['bb', 'empty', 'insn', 'mem', 'syscall']
t += shared_module(i, files(i + '.c'),
include_directories: '../../include/qemu',
dependencies: glib)
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v1 02/14] plugins: Expose physical addresses instead of device offsets
2021-03-12 17:28 [PATCH v1 00/14] plugins/next (phys addr, syscalls, lots of docs) Alex Bennée
2021-03-12 17:28 ` [PATCH v1 01/14] plugins: new syscalls plugin Alex Bennée
@ 2021-03-12 17:28 ` Alex Bennée
2021-03-12 17:28 ` [PATCH v1 03/14] docs/devel: include the plugin API information from the headers Alex Bennée
` (11 subsequent siblings)
13 siblings, 0 replies; 31+ messages in thread
From: Alex Bennée @ 2021-03-12 17:28 UTC (permalink / raw
To: qemu-devel
Cc: robhenry, mahmoudabdalghany, aaron, cota, kuhn.chenqun,
Alex Bennée
From: Aaron Lindsay <aaron@os.amperecomputing.com>
This allows plugins to query for full virtual-to-physical address
translation for a given `qemu_plugin_hwaddr` and stops exposing the
offset within the device itself. As this change breaks the API,
QEMU_PLUGIN_VERSION is incremented.
Signed-off-by: Aaron Lindsay <aaron@os.amperecomputing.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20210309202802.211756-1-aaron@os.amperecomputing.com>
---
include/qemu/qemu-plugin.h | 32 +++++++++++++++++++++++++-------
contrib/plugins/hotpages.c | 2 +-
contrib/plugins/hwprofile.c | 2 +-
plugins/api.c | 17 ++++++++++++-----
4 files changed, 39 insertions(+), 14 deletions(-)
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index c66507fe8f..3303dce862 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -47,7 +47,7 @@ typedef uint64_t qemu_plugin_id_t;
extern QEMU_PLUGIN_EXPORT int qemu_plugin_version;
-#define QEMU_PLUGIN_VERSION 0
+#define QEMU_PLUGIN_VERSION 1
typedef struct {
/* string describing architecture */
@@ -307,8 +307,8 @@ bool qemu_plugin_mem_is_sign_extended(qemu_plugin_meminfo_t info);
bool qemu_plugin_mem_is_big_endian(qemu_plugin_meminfo_t info);
bool qemu_plugin_mem_is_store(qemu_plugin_meminfo_t info);
-/*
- * qemu_plugin_get_hwaddr():
+/**
+ * qemu_plugin_get_hwaddr() - return handle for memory operation
* @vaddr: the virtual address of the memory operation
*
* For system emulation returns a qemu_plugin_hwaddr handle to query
@@ -323,12 +323,30 @@ struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
uint64_t vaddr);
/*
- * The following additional queries can be run on the hwaddr structure
- * to return information about it. For non-IO accesses the device
- * offset will be into the appropriate block of RAM.
+ * The following additional queries can be run on the hwaddr structure to
+ * return information about it - namely whether it is for an IO access and the
+ * physical address associated with the access.
+ */
+
+/**
+ * qemu_plugin_hwaddr_is_io() - query whether memory operation is IO
+ * @haddr: address handle from qemu_plugin_get_hwaddr()
+ *
+ * Returns true if the handle's memory operation is to memory-mapped IO, or
+ * false if it is to RAM
*/
bool qemu_plugin_hwaddr_is_io(const struct qemu_plugin_hwaddr *haddr);
-uint64_t qemu_plugin_hwaddr_device_offset(const struct qemu_plugin_hwaddr *haddr);
+
+/**
+ * qemu_plugin_hwaddr_phys_addr() - query physical address for memory operation
+ * @haddr: address handle from qemu_plugin_get_hwaddr()
+ *
+ * Returns the physical address associated with the memory operation
+ *
+ * Note that the returned physical address may not be unique if you are dealing
+ * with multiple address spaces.
+ */
+uint64_t qemu_plugin_hwaddr_phys_addr(const struct qemu_plugin_hwaddr *haddr);
/*
* Returns a string representing the device. The string is valid for
diff --git a/contrib/plugins/hotpages.c b/contrib/plugins/hotpages.c
index eacc678eac..bf53267532 100644
--- a/contrib/plugins/hotpages.c
+++ b/contrib/plugins/hotpages.c
@@ -122,7 +122,7 @@ static void vcpu_haddr(unsigned int cpu_index, qemu_plugin_meminfo_t meminfo,
}
} else {
if (hwaddr && !qemu_plugin_hwaddr_is_io(hwaddr)) {
- page = (uint64_t) qemu_plugin_hwaddr_device_offset(hwaddr);
+ page = (uint64_t) qemu_plugin_hwaddr_phys_addr(hwaddr);
} else {
page = vaddr;
}
diff --git a/contrib/plugins/hwprofile.c b/contrib/plugins/hwprofile.c
index 6dac1d5f85..faf216ac00 100644
--- a/contrib/plugins/hwprofile.c
+++ b/contrib/plugins/hwprofile.c
@@ -201,7 +201,7 @@ static void vcpu_haddr(unsigned int cpu_index, qemu_plugin_meminfo_t meminfo,
return;
} else {
const char *name = qemu_plugin_hwaddr_device_name(hwaddr);
- uint64_t off = qemu_plugin_hwaddr_device_offset(hwaddr);
+ uint64_t off = qemu_plugin_hwaddr_phys_addr(hwaddr);
bool is_write = qemu_plugin_mem_is_store(meminfo);
DeviceCounts *counts;
diff --git a/plugins/api.c b/plugins/api.c
index 0b04380d57..3c7dc406e3 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -40,6 +40,7 @@
#include "sysemu/sysemu.h"
#include "tcg/tcg.h"
#include "exec/exec-all.h"
+#include "exec/ram_addr.h"
#include "disas/disas.h"
#include "plugin.h"
#ifndef CONFIG_USER_ONLY
@@ -298,19 +299,25 @@ bool qemu_plugin_hwaddr_is_io(const struct qemu_plugin_hwaddr *haddr)
#endif
}
-uint64_t qemu_plugin_hwaddr_device_offset(const struct qemu_plugin_hwaddr *haddr)
+uint64_t qemu_plugin_hwaddr_phys_addr(const struct qemu_plugin_hwaddr *haddr)
{
#ifdef CONFIG_SOFTMMU
if (haddr) {
if (!haddr->is_io) {
- ram_addr_t ram_addr = qemu_ram_addr_from_host((void *) haddr->v.ram.hostaddr);
- if (ram_addr == RAM_ADDR_INVALID) {
+ RAMBlock *block;
+ ram_addr_t offset;
+ void *hostaddr = (void *) haddr->v.ram.hostaddr;
+
+ block = qemu_ram_block_from_host(hostaddr, false, &offset);
+ if (!block) {
error_report("Bad ram pointer %"PRIx64"", haddr->v.ram.hostaddr);
abort();
}
- return ram_addr;
+
+ return block->offset + offset + block->mr->addr;
} else {
- return haddr->v.io.offset;
+ MemoryRegionSection *mrs = haddr->v.io.section;
+ return haddr->v.io.offset + mrs->mr->addr;
}
}
#endif
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v1 03/14] docs/devel: include the plugin API information from the headers
2021-03-12 17:28 [PATCH v1 00/14] plugins/next (phys addr, syscalls, lots of docs) Alex Bennée
2021-03-12 17:28 ` [PATCH v1 01/14] plugins: new syscalls plugin Alex Bennée
2021-03-12 17:28 ` [PATCH v1 02/14] plugins: Expose physical addresses instead of device offsets Alex Bennée
@ 2021-03-12 17:28 ` Alex Bennée
2021-03-12 18:19 ` Aaron Lindsay via
2021-03-12 17:28 ` [PATCH v1 04/14] plugins: expand kernel-doc for qemu_info_t Alex Bennée
` (10 subsequent siblings)
13 siblings, 1 reply; 31+ messages in thread
From: Alex Bennée @ 2021-03-12 17:28 UTC (permalink / raw
To: qemu-devel
Cc: robhenry, mahmoudabdalghany, aaron, cota, kuhn.chenqun,
Alex Bennée
We have kerneldoc tags for the headers so we might as well extract
them into our developer documentation whilst we are at it.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
docs/devel/tcg-plugins.rst | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
index 39ce86ed96..18c6581d85 100644
--- a/docs/devel/tcg-plugins.rst
+++ b/docs/devel/tcg-plugins.rst
@@ -63,6 +63,11 @@ valid during the lifetime of the callback so it is important that any
information that is needed is extracted during the callback and saved
by the plugin.
+API
+===
+
+.. kernel-doc:: include/qemu/qemu-plugin.h
+
Usage
=====
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v1 04/14] plugins: expand kernel-doc for qemu_info_t
2021-03-12 17:28 [PATCH v1 00/14] plugins/next (phys addr, syscalls, lots of docs) Alex Bennée
` (2 preceding siblings ...)
2021-03-12 17:28 ` [PATCH v1 03/14] docs/devel: include the plugin API information from the headers Alex Bennée
@ 2021-03-12 17:28 ` Alex Bennée
2021-03-12 18:20 ` Aaron Lindsay via
2021-03-12 17:28 ` [PATCH v1 05/14] plugins: cleanup kernel-doc for qemu_plugin_install Alex Bennée
` (9 subsequent siblings)
13 siblings, 1 reply; 31+ messages in thread
From: Alex Bennée @ 2021-03-12 17:28 UTC (permalink / raw
To: qemu-devel
Cc: robhenry, mahmoudabdalghany, aaron, cota, kuhn.chenqun,
Alex Bennée
It seems kernel-doc struggles a bit with typedef structs but with
enough encouragement we can get something out of it.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
include/qemu/qemu-plugin.h | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 3303dce862..4b84c6c293 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -49,22 +49,30 @@ extern QEMU_PLUGIN_EXPORT int qemu_plugin_version;
#define QEMU_PLUGIN_VERSION 1
-typedef struct {
- /* string describing architecture */
+/**
+ * struct qemu_info_t - system information for plugins
+ *
+ * This structure provides for some limited information about the
+ * system to allow the plugin to make decisions on how to proceed. For
+ * example it might only be suitable for running on some guest
+ * architectures or when under full system emulation.
+ */
+typedef struct qemu_info_t {
+ /** @target_name: string describing architecture */
const char *target_name;
+ /** @version: minimum and current plugin API level */
struct {
int min;
int cur;
} version;
- /* is this a full system emulation? */
+ /** @system_emulation: is this a full system emulation? */
bool system_emulation;
union {
- /*
- * smp_vcpus may change if vCPUs can be hot-plugged, max_vcpus
- * is the system-wide limit.
- */
+ /** @system: information relevant to system emulation */
struct {
+ /** @system.smp_vcpus: initial number of vCPUs */
int smp_vcpus;
+ /** @system.max_vcpus: maximum possible number of vCPUs */
int max_vcpus;
} system;
};
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v1 05/14] plugins: cleanup kernel-doc for qemu_plugin_install
2021-03-12 17:28 [PATCH v1 00/14] plugins/next (phys addr, syscalls, lots of docs) Alex Bennée
` (3 preceding siblings ...)
2021-03-12 17:28 ` [PATCH v1 04/14] plugins: expand kernel-doc for qemu_info_t Alex Bennée
@ 2021-03-12 17:28 ` Alex Bennée
2021-03-12 18:21 ` Aaron Lindsay via
2021-03-12 17:28 ` [PATCH v1 06/14] plugins: expand the callback typedef kernel-docs Alex Bennée
` (8 subsequent siblings)
13 siblings, 1 reply; 31+ messages in thread
From: Alex Bennée @ 2021-03-12 17:28 UTC (permalink / raw
To: qemu-devel
Cc: robhenry, mahmoudabdalghany, aaron, cota, kuhn.chenqun,
Alex Bennée
kernel-doc doesn't like multiple Note sections. Also add an explicit
Return.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
include/qemu/qemu-plugin.h | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 4b84c6c293..ac1bb318da 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -85,15 +85,15 @@ typedef struct qemu_info_t {
* @argc: number of arguments
* @argv: array of arguments (@argc elements)
*
- * All plugins must export this symbol.
- *
- * Note: Calling qemu_plugin_uninstall() from this function is a bug. To raise
- * an error during install, return !0.
+ * All plugins must export this symbol which is called when the plugin
+ * is first loaded. Calling qemu_plugin_uninstall() from this function
+ * is a bug.
*
* Note: @info is only live during the call. Copy any information we
- * want to keep.
+ * want to keep. @argv remains valid throughout the lifetime of the
+ * loaded plugin.
*
- * Note: @argv remains valid throughout the lifetime of the loaded plugin.
+ * Return: 0 on successful loading, !0 for an error.
*/
QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
const qemu_info_t *info,
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v1 06/14] plugins: expand the callback typedef kernel-docs
2021-03-12 17:28 [PATCH v1 00/14] plugins/next (phys addr, syscalls, lots of docs) Alex Bennée
` (4 preceding siblings ...)
2021-03-12 17:28 ` [PATCH v1 05/14] plugins: cleanup kernel-doc for qemu_plugin_install Alex Bennée
@ 2021-03-12 17:28 ` Alex Bennée
2021-03-12 18:25 ` Aaron Lindsay via
2021-03-12 17:28 ` [PATCH v1 07/14] plugins: expand the typedef kernel-docs for translation Alex Bennée
` (7 subsequent siblings)
13 siblings, 1 reply; 31+ messages in thread
From: Alex Bennée @ 2021-03-12 17:28 UTC (permalink / raw
To: qemu-devel
Cc: robhenry, mahmoudabdalghany, aaron, cota, kuhn.chenqun,
Alex Bennée
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
include/qemu/qemu-plugin.h | 25 ++++++++++++++++++++++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index ac1bb318da..09b235f0b4 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -99,17 +99,36 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
const qemu_info_t *info,
int argc, char **argv);
-/*
- * Prototypes for the various callback styles we will be registering
- * in the following functions.
+/**
+ * typedef qemu_plugin_simple_cb_t - simple callback
+ * @id: the unique qemu_plugin_id_t
+ *
+ * This call-back passes no information aside from the unique @id.
*/
typedef void (*qemu_plugin_simple_cb_t)(qemu_plugin_id_t id);
+/**
+ * typedef qemu_plugin_udata_cb_t - callback with user data
+ * @id: the unique qemu_plugin_id_t
+ * @userdata: a pointer to some user data supplied when the call-back
+ * was registered.
+ */
typedef void (*qemu_plugin_udata_cb_t)(qemu_plugin_id_t id, void *userdata);
+/**
+ * typedef qemu_plugin_vcpu_simple_cb_t - vcpu callback
+ * @id: the unique qemu_plugin_id_t
+ * @vcpu_index: the current vcpu context
+ */
typedef void (*qemu_plugin_vcpu_simple_cb_t)(qemu_plugin_id_t id,
unsigned int vcpu_index);
+/**
+ * typedef qemu_plugin_vcpu_udata_cb_t - vcpu callback
+ * @vcpu_index: the current vcpu context
+ * @userdata: a pointer to some user data supplied when the call-back
+ * was registered.
+ */
typedef void (*qemu_plugin_vcpu_udata_cb_t)(unsigned int vcpu_index,
void *userdata);
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v1 07/14] plugins: expand the typedef kernel-docs for translation
2021-03-12 17:28 [PATCH v1 00/14] plugins/next (phys addr, syscalls, lots of docs) Alex Bennée
` (5 preceding siblings ...)
2021-03-12 17:28 ` [PATCH v1 06/14] plugins: expand the callback typedef kernel-docs Alex Bennée
@ 2021-03-12 17:28 ` Alex Bennée
2021-03-12 18:27 ` Aaron Lindsay via
2021-03-12 17:28 ` [PATCH v1 08/14] plugins: add qemu_plugin_cb_flags to kernel-doc Alex Bennée
` (6 subsequent siblings)
13 siblings, 1 reply; 31+ messages in thread
From: Alex Bennée @ 2021-03-12 17:28 UTC (permalink / raw
To: qemu-devel
Cc: robhenry, mahmoudabdalghany, aaron, cota, kuhn.chenqun,
Alex Bennée
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
include/qemu/qemu-plugin.h | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 09b235f0b4..9ae3940d89 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -202,11 +202,9 @@ void qemu_plugin_register_vcpu_idle_cb(qemu_plugin_id_t id,
void qemu_plugin_register_vcpu_resume_cb(qemu_plugin_id_t id,
qemu_plugin_vcpu_simple_cb_t cb);
-/*
- * Opaque types that the plugin is given during the translation and
- * instrumentation phase.
- */
+/** struct qemu_plugin_tb - Opaque handle for a translation block */
struct qemu_plugin_tb;
+/** struct qemu_plugin_insn - Opaque handle for a translated instruction */
struct qemu_plugin_insn;
enum qemu_plugin_cb_flags {
@@ -221,6 +219,14 @@ enum qemu_plugin_mem_rw {
QEMU_PLUGIN_MEM_RW,
};
+/**
+ * typedef qemu_plugin_vcpu_tb_trans_cb_t - translation callback
+ * @id: unique plugin id
+ * @tb: opaque handle used for querying and instrumenting a block.
+ */
+typedef void (*qemu_plugin_vcpu_tb_trans_cb_t)(qemu_plugin_id_t id,
+ struct qemu_plugin_tb *tb);
+
/**
* qemu_plugin_register_vcpu_tb_trans_cb() - register a translate cb
* @id: plugin ID
@@ -233,9 +239,6 @@ enum qemu_plugin_mem_rw {
* callbacks to be triggered when the block or individual instruction
* executes.
*/
-typedef void (*qemu_plugin_vcpu_tb_trans_cb_t)(qemu_plugin_id_t id,
- struct qemu_plugin_tb *tb);
-
void qemu_plugin_register_vcpu_tb_trans_cb(qemu_plugin_id_t id,
qemu_plugin_vcpu_tb_trans_cb_t cb);
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v1 08/14] plugins: add qemu_plugin_cb_flags to kernel-doc
2021-03-12 17:28 [PATCH v1 00/14] plugins/next (phys addr, syscalls, lots of docs) Alex Bennée
` (6 preceding siblings ...)
2021-03-12 17:28 ` [PATCH v1 07/14] plugins: expand the typedef kernel-docs for translation Alex Bennée
@ 2021-03-12 17:28 ` Alex Bennée
2021-03-12 18:29 ` Aaron Lindsay via
2021-03-12 17:28 ` [PATCH v1 09/14] plugins: add qemu_plugin_id_t " Alex Bennée
` (5 subsequent siblings)
13 siblings, 1 reply; 31+ messages in thread
From: Alex Bennée @ 2021-03-12 17:28 UTC (permalink / raw
To: qemu-devel
Cc: robhenry, mahmoudabdalghany, aaron, cota, kuhn.chenqun,
Alex Bennée
Also add a note to explain currently they are unused.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
include/qemu/qemu-plugin.h | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 9ae3940d89..c98866a637 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -207,10 +207,20 @@ struct qemu_plugin_tb;
/** struct qemu_plugin_insn - Opaque handle for a translated instruction */
struct qemu_plugin_insn;
+/**
+ * enum qemu_plugin_cb_flags - type of callback
+ *
+ * @QEMU_PLUGIN_CB_NO_REGS: callback does not access the CPU's regs
+ * @QEMU_PLUGIN_CB_R_REGS: callback reads the CPU's regs
+ * @QEMU_PLUGIN_CB_RW_REGS: callback reads and writes the CPU's regs
+ *
+ * Note: currently unused, plugins cannot read or change system
+ * register state.
+ */
enum qemu_plugin_cb_flags {
- QEMU_PLUGIN_CB_NO_REGS, /* callback does not access the CPU's regs */
- QEMU_PLUGIN_CB_R_REGS, /* callback reads the CPU's regs */
- QEMU_PLUGIN_CB_RW_REGS, /* callback reads and writes the CPU's regs */
+ QEMU_PLUGIN_CB_NO_REGS,
+ QEMU_PLUGIN_CB_R_REGS,
+ QEMU_PLUGIN_CB_RW_REGS,
};
enum qemu_plugin_mem_rw {
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v1 09/14] plugins: add qemu_plugin_id_t to kernel-doc
2021-03-12 17:28 [PATCH v1 00/14] plugins/next (phys addr, syscalls, lots of docs) Alex Bennée
` (7 preceding siblings ...)
2021-03-12 17:28 ` [PATCH v1 08/14] plugins: add qemu_plugin_cb_flags to kernel-doc Alex Bennée
@ 2021-03-12 17:28 ` Alex Bennée
2021-03-12 18:29 ` Aaron Lindsay via
2021-03-12 17:28 ` [PATCH v1 10/14] plugins: expand inline exec kernel-doc documentation Alex Bennée
` (4 subsequent siblings)
13 siblings, 1 reply; 31+ messages in thread
From: Alex Bennée @ 2021-03-12 17:28 UTC (permalink / raw
To: qemu-devel
Cc: robhenry, mahmoudabdalghany, aaron, cota, kuhn.chenqun,
Alex Bennée
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
include/qemu/qemu-plugin.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index c98866a637..5ac6fe5f02 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -32,6 +32,9 @@
#define QEMU_PLUGIN_LOCAL __attribute__((visibility("hidden")))
#endif
+/**
+ * typedef qemu_plugin_id_t - Unique plugin ID
+ */
typedef uint64_t qemu_plugin_id_t;
/*
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v1 10/14] plugins: expand inline exec kernel-doc documentation.
2021-03-12 17:28 [PATCH v1 00/14] plugins/next (phys addr, syscalls, lots of docs) Alex Bennée
` (8 preceding siblings ...)
2021-03-12 17:28 ` [PATCH v1 09/14] plugins: add qemu_plugin_id_t " Alex Bennée
@ 2021-03-12 17:28 ` Alex Bennée
2021-03-12 18:30 ` Aaron Lindsay via
2021-03-12 17:28 ` [PATCH v1 11/14] plugins: expand kernel-doc for instruction query and instrumentation Alex Bennée
` (3 subsequent siblings)
13 siblings, 1 reply; 31+ messages in thread
From: Alex Bennée @ 2021-03-12 17:28 UTC (permalink / raw
To: qemu-devel
Cc: robhenry, mahmoudabdalghany, aaron, cota, kuhn.chenqun,
Alex Bennée
Remove the extraneous @cb parameter and document the non-atomic nature
of the INLINE_ADD_U64 operation.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
include/qemu/qemu-plugin.h | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 5ac6fe5f02..dc05fc1932 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -269,6 +269,14 @@ void qemu_plugin_register_vcpu_tb_exec_cb(struct qemu_plugin_tb *tb,
enum qemu_plugin_cb_flags flags,
void *userdata);
+/**
+ * enum qemu_plugin_op - describes an inline op
+ *
+ * @QEMU_PLUGIN_INLINE_ADD_U64: add an immediate value uint64_t
+ *
+ * Note: currently only a single inline op is supported.
+ */
+
enum qemu_plugin_op {
QEMU_PLUGIN_INLINE_ADD_U64,
};
@@ -283,6 +291,9 @@ enum qemu_plugin_op {
* Insert an inline op to every time a translated unit executes.
* Useful if you just want to increment a single counter somewhere in
* memory.
+ *
+ * Note: ops are not atomic so in multi-threaded/multi-smp situations
+ * you will get inexact results.
*/
void qemu_plugin_register_vcpu_tb_exec_inline(struct qemu_plugin_tb *tb,
enum qemu_plugin_op op,
@@ -305,7 +316,6 @@ void qemu_plugin_register_vcpu_insn_exec_cb(struct qemu_plugin_insn *insn,
/**
* qemu_plugin_register_vcpu_insn_exec_inline() - insn execution inline op
* @insn: the opaque qemu_plugin_insn handle for an instruction
- * @cb: callback function
* @op: the type of qemu_plugin_op (e.g. ADD_U64)
* @ptr: the target memory location for the op
* @imm: the op data (e.g. 1)
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v1 11/14] plugins: expand kernel-doc for instruction query and instrumentation
2021-03-12 17:28 [PATCH v1 00/14] plugins/next (phys addr, syscalls, lots of docs) Alex Bennée
` (9 preceding siblings ...)
2021-03-12 17:28 ` [PATCH v1 10/14] plugins: expand inline exec kernel-doc documentation Alex Bennée
@ 2021-03-12 17:28 ` Alex Bennée
2021-03-12 18:36 ` Aaron Lindsay via
2021-03-12 17:28 ` [PATCH v1 12/14] plugins: expand kernel-doc for memory " Alex Bennée
` (2 subsequent siblings)
13 siblings, 1 reply; 31+ messages in thread
From: Alex Bennée @ 2021-03-12 17:28 UTC (permalink / raw
To: qemu-devel
Cc: robhenry, mahmoudabdalghany, aaron, cota, kuhn.chenqun,
Alex Bennée
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
include/qemu/qemu-plugin.h | 52 ++++++++++++++++++++++++++++++++++++--
1 file changed, 50 insertions(+), 2 deletions(-)
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index dc05fc1932..d4adce730a 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -327,21 +327,69 @@ void qemu_plugin_register_vcpu_insn_exec_inline(struct qemu_plugin_insn *insn,
enum qemu_plugin_op op,
void *ptr, uint64_t imm);
-/*
- * Helpers to query information about the instructions in a block
+/**
+ * qemu_plugin_tb_n_insns() - query helper for number of insns in TB
+ * @tb: opaque handle to TB passed to callback
+ *
+ * Returns: number of instructions in this block
*/
size_t qemu_plugin_tb_n_insns(const struct qemu_plugin_tb *tb);
+/**
+ * qemu_plugin_tb_vaddr() - query helper for vaddr of TB start
+ * @tb: opaque handle to TB passed to callback
+ *
+ * Returns: virtual address of block start
+ */
uint64_t qemu_plugin_tb_vaddr(const struct qemu_plugin_tb *tb);
+/**
+ * qemu_plugin_tb_get_insn() - retrieve handle for instruction
+ * @tb: opaque handle to TB passed to callback
+ * @idx: instruction number, 0 indexed
+ *
+ * The returned handle can be used in follow up helper queries as well
+ * as when instrumenting an instruction. It is only valid for the
+ * lifetime of the callback.
+ *
+ * Returns: opaque handle to instruction
+ */
struct qemu_plugin_insn *
qemu_plugin_tb_get_insn(const struct qemu_plugin_tb *tb, size_t idx);
+/**
+ * qemu_plugin_insn_data() - return ptr to instruction data
+ * @insn: opaque instruction handle from qemu_plugin_tb_get_insn()
+ *
+ * Note: data is only valid for duration of callback. See
+ * qemu_plugin_insn_size() to calculate size of stream.
+ *
+ * Returns: pointer to a stream of bytes
+ */
const void *qemu_plugin_insn_data(const struct qemu_plugin_insn *insn);
+/**
+ * qemu_plugin_insn_size() - return size of instruction
+ * @insn: opaque instruction handle from qemu_plugin_tb_get_insn()
+ *
+ * Returns: size of instruction
+ */
size_t qemu_plugin_insn_size(const struct qemu_plugin_insn *insn);
+/**
+ * qemu_plugin_insn_vaddr() - return vaddr of instruction
+ * @insn: opaque instruction handle from qemu_plugin_tb_get_insn()
+ *
+ * Returns: virtual address of instruction
+ */
uint64_t qemu_plugin_insn_vaddr(const struct qemu_plugin_insn *insn);
+
+/**
+ * qemu_plugin_insn_haddr() - return vaddr of instruction
+ * @insn: opaque instruction handle from qemu_plugin_tb_get_insn()
+ *
+ * Returns: hardware (physical) address of instruction
+ */
void *qemu_plugin_insn_haddr(const struct qemu_plugin_insn *insn);
/*
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v1 12/14] plugins: expand kernel-doc for memory query and instrumentation
2021-03-12 17:28 [PATCH v1 00/14] plugins/next (phys addr, syscalls, lots of docs) Alex Bennée
` (10 preceding siblings ...)
2021-03-12 17:28 ` [PATCH v1 11/14] plugins: expand kernel-doc for instruction query and instrumentation Alex Bennée
@ 2021-03-12 17:28 ` Alex Bennée
2021-03-12 18:40 ` Aaron Lindsay via
2021-03-12 17:28 ` [PATCH v1 13/14] plugins: getting qemu_plugin_get_hwaddr only expose one function prototype Alex Bennée
2021-03-12 17:28 ` [PATCH v1 14/14] plugins: Fixes typo in qemu-plugin.h Alex Bennée
13 siblings, 1 reply; 31+ messages in thread
From: Alex Bennée @ 2021-03-12 17:28 UTC (permalink / raw
To: qemu-devel
Cc: robhenry, mahmoudabdalghany, aaron, cota, kuhn.chenqun,
Alex Bennée
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
include/qemu/qemu-plugin.h | 35 ++++++++++++++++++++++++++++-------
1 file changed, 28 insertions(+), 7 deletions(-)
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index d4adce730a..aed868d42a 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -392,24 +392,45 @@ uint64_t qemu_plugin_insn_vaddr(const struct qemu_plugin_insn *insn);
*/
void *qemu_plugin_insn_haddr(const struct qemu_plugin_insn *insn);
-/*
- * Memory Instrumentation
- *
- * The anonymous qemu_plugin_meminfo_t and qemu_plugin_hwaddr types
- * can be used in queries to QEMU to get more information about a
- * given memory access.
+/**
+ * typedef qemu_plugin_meminfo_t - opaque memory transaction handle
*/
typedef uint32_t qemu_plugin_meminfo_t;
+/** struct qemu_plugin_hwaddr - opaque hw address handle */
struct qemu_plugin_hwaddr;
-/* meminfo queries */
+/**
+ * qemu_plugin_mem_size_shift() - get size of access
+ * @info: opaque memory transaction handle
+ *
+ * Returns: size of access in ^2 (0=byte, 1=16bit, 2=32bit etc...)
+ */
unsigned int qemu_plugin_mem_size_shift(qemu_plugin_meminfo_t info);
+/**
+ * qemu_plugin_mem_is_sign_extended() - was the access sign extended
+ * @info: opaque memory transaction handle
+ *
+ * Returns: true if it was, otherwise false
+ */
bool qemu_plugin_mem_is_sign_extended(qemu_plugin_meminfo_t info);
+/**
+ * qemu_plugin_mem_is_big_endian() - was the access big endian
+ * @info: opaque memory transaction handle
+ *
+ * Returns: true if it was, otherwise false
+ */
bool qemu_plugin_mem_is_big_endian(qemu_plugin_meminfo_t info);
+/**
+ * qemu_plugin_mem_is_store() - was the access a store
+ * @info: opaque memory transaction handle
+ *
+ * Returns: true if it was, otherwise false
+ */
bool qemu_plugin_mem_is_store(qemu_plugin_meminfo_t info);
/**
* qemu_plugin_get_hwaddr() - return handle for memory operation
+ * @info: opaque memory info structure
* @vaddr: the virtual address of the memory operation
*
* For system emulation returns a qemu_plugin_hwaddr handle to query
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v1 13/14] plugins: getting qemu_plugin_get_hwaddr only expose one function prototype
2021-03-12 17:28 [PATCH v1 00/14] plugins/next (phys addr, syscalls, lots of docs) Alex Bennée
` (11 preceding siblings ...)
2021-03-12 17:28 ` [PATCH v1 12/14] plugins: expand kernel-doc for memory " Alex Bennée
@ 2021-03-12 17:28 ` Alex Bennée
2021-03-12 17:28 ` [PATCH v1 14/14] plugins: Fixes typo in qemu-plugin.h Alex Bennée
13 siblings, 0 replies; 31+ messages in thread
From: Alex Bennée @ 2021-03-12 17:28 UTC (permalink / raw
To: qemu-devel
Cc: Yonggang Luo, robhenry, mahmoudabdalghany, aaron, cota,
kuhn.chenqun, Alex Bennée
From: Yonggang Luo <luoyonggang@gmail.com>
This is used for counting how much function are export to qemu plugin.
Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20201013002806.1447-2-luoyonggang@gmail.com>
---
plugins/api.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/plugins/api.c b/plugins/api.c
index 3c7dc406e3..b22998cd7c 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -266,10 +266,12 @@ bool qemu_plugin_mem_is_store(qemu_plugin_meminfo_t info)
#ifdef CONFIG_SOFTMMU
static __thread struct qemu_plugin_hwaddr hwaddr_info;
+#endif
struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
uint64_t vaddr)
{
+#ifdef CONFIG_SOFTMMU
CPUState *cpu = current_cpu;
unsigned int mmu_idx = info >> TRACE_MEM_MMU_SHIFT;
hwaddr_info.is_store = info & TRACE_MEM_ST;
@@ -281,14 +283,10 @@ struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
}
return &hwaddr_info;
-}
#else
-struct qemu_plugin_hwaddr *qemu_plugin_get_hwaddr(qemu_plugin_meminfo_t info,
- uint64_t vaddr)
-{
return NULL;
-}
#endif
+}
bool qemu_plugin_hwaddr_is_io(const struct qemu_plugin_hwaddr *haddr)
{
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v1 14/14] plugins: Fixes typo in qemu-plugin.h
2021-03-12 17:28 [PATCH v1 00/14] plugins/next (phys addr, syscalls, lots of docs) Alex Bennée
` (12 preceding siblings ...)
2021-03-12 17:28 ` [PATCH v1 13/14] plugins: getting qemu_plugin_get_hwaddr only expose one function prototype Alex Bennée
@ 2021-03-12 17:28 ` Alex Bennée
13 siblings, 0 replies; 31+ messages in thread
From: Alex Bennée @ 2021-03-12 17:28 UTC (permalink / raw
To: qemu-devel
Cc: Yonggang Luo, robhenry, mahmoudabdalghany, aaron, cota,
kuhn.chenqun, Alex Bennée
From: Yonggang Luo <luoyonggang@gmail.com>
Getting the comment consistence with the function name
Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20201013002806.1447-3-luoyonggang@gmail.com>
---
include/qemu/qemu-plugin.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index aed868d42a..19e77e345b 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -256,7 +256,7 @@ void qemu_plugin_register_vcpu_tb_trans_cb(qemu_plugin_id_t id,
qemu_plugin_vcpu_tb_trans_cb_t cb);
/**
- * qemu_plugin_register_vcpu_tb_trans_exec_cb() - register execution callback
+ * qemu_plugin_register_vcpu_tb_exec_cb() - register execution callback
* @tb: the opaque qemu_plugin_tb handle for the translation
* @cb: callback function
* @flags: does the plugin read or write the CPU's registers?
@@ -282,7 +282,7 @@ enum qemu_plugin_op {
};
/**
- * qemu_plugin_register_vcpu_tb_trans_exec_inline() - execution inline op
+ * qemu_plugin_register_vcpu_tb_exec_inline() - execution inline op
* @tb: the opaque qemu_plugin_tb handle for the translation
* @op: the type of qemu_plugin_op (e.g. ADD_U64)
* @ptr: the target memory location for the op
--
2.20.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v1 03/14] docs/devel: include the plugin API information from the headers
2021-03-12 17:28 ` [PATCH v1 03/14] docs/devel: include the plugin API information from the headers Alex Bennée
@ 2021-03-12 18:19 ` Aaron Lindsay via
0 siblings, 0 replies; 31+ messages in thread
From: Aaron Lindsay via @ 2021-03-12 18:19 UTC (permalink / raw
To: Alex Bennée
Cc: qemu-devel, cota, kuhn.chenqun, robhenry, mahmoudabdalghany
On Mar 12 17:28, Alex Bennée wrote:
> We have kerneldoc tags for the headers so we might as well extract
> them into our developer documentation whilst we are at it.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Aaron Lindsay <aaron@os.amperecomputing.com>
> ---
> docs/devel/tcg-plugins.rst | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
> index 39ce86ed96..18c6581d85 100644
> --- a/docs/devel/tcg-plugins.rst
> +++ b/docs/devel/tcg-plugins.rst
> @@ -63,6 +63,11 @@ valid during the lifetime of the callback so it is important that any
> information that is needed is extracted during the callback and saved
> by the plugin.
>
> +API
> +===
> +
> +.. kernel-doc:: include/qemu/qemu-plugin.h
> +
> Usage
> =====
>
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v1 04/14] plugins: expand kernel-doc for qemu_info_t
2021-03-12 17:28 ` [PATCH v1 04/14] plugins: expand kernel-doc for qemu_info_t Alex Bennée
@ 2021-03-12 18:20 ` Aaron Lindsay via
0 siblings, 0 replies; 31+ messages in thread
From: Aaron Lindsay via @ 2021-03-12 18:20 UTC (permalink / raw
To: Alex Bennée
Cc: qemu-devel, cota, kuhn.chenqun, robhenry, mahmoudabdalghany
On Mar 12 17:28, Alex Bennée wrote:
> It seems kernel-doc struggles a bit with typedef structs but with
> enough encouragement we can get something out of it.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Aaron Lindsay <aaron@os.amperecomputing.com>
> ---
> include/qemu/qemu-plugin.h | 22 +++++++++++++++-------
> 1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index 3303dce862..4b84c6c293 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -49,22 +49,30 @@ extern QEMU_PLUGIN_EXPORT int qemu_plugin_version;
>
> #define QEMU_PLUGIN_VERSION 1
>
> -typedef struct {
> - /* string describing architecture */
> +/**
> + * struct qemu_info_t - system information for plugins
> + *
> + * This structure provides for some limited information about the
> + * system to allow the plugin to make decisions on how to proceed. For
> + * example it might only be suitable for running on some guest
> + * architectures or when under full system emulation.
> + */
> +typedef struct qemu_info_t {
> + /** @target_name: string describing architecture */
> const char *target_name;
> + /** @version: minimum and current plugin API level */
> struct {
> int min;
> int cur;
> } version;
> - /* is this a full system emulation? */
> + /** @system_emulation: is this a full system emulation? */
> bool system_emulation;
> union {
> - /*
> - * smp_vcpus may change if vCPUs can be hot-plugged, max_vcpus
> - * is the system-wide limit.
> - */
> + /** @system: information relevant to system emulation */
> struct {
> + /** @system.smp_vcpus: initial number of vCPUs */
> int smp_vcpus;
> + /** @system.max_vcpus: maximum possible number of vCPUs */
> int max_vcpus;
> } system;
> };
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v1 05/14] plugins: cleanup kernel-doc for qemu_plugin_install
2021-03-12 17:28 ` [PATCH v1 05/14] plugins: cleanup kernel-doc for qemu_plugin_install Alex Bennée
@ 2021-03-12 18:21 ` Aaron Lindsay via
0 siblings, 0 replies; 31+ messages in thread
From: Aaron Lindsay via @ 2021-03-12 18:21 UTC (permalink / raw
To: Alex Bennée
Cc: qemu-devel, cota, kuhn.chenqun, robhenry, mahmoudabdalghany
On Mar 12 17:28, Alex Bennée wrote:
> kernel-doc doesn't like multiple Note sections. Also add an explicit
> Return.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Aaron Lindsay <aaron@os.amperecomputing.com>
> ---
> include/qemu/qemu-plugin.h | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index 4b84c6c293..ac1bb318da 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -85,15 +85,15 @@ typedef struct qemu_info_t {
> * @argc: number of arguments
> * @argv: array of arguments (@argc elements)
> *
> - * All plugins must export this symbol.
> - *
> - * Note: Calling qemu_plugin_uninstall() from this function is a bug. To raise
> - * an error during install, return !0.
> + * All plugins must export this symbol which is called when the plugin
> + * is first loaded. Calling qemu_plugin_uninstall() from this function
> + * is a bug.
> *
> * Note: @info is only live during the call. Copy any information we
> - * want to keep.
> + * want to keep. @argv remains valid throughout the lifetime of the
> + * loaded plugin.
> *
> - * Note: @argv remains valid throughout the lifetime of the loaded plugin.
> + * Return: 0 on successful loading, !0 for an error.
> */
> QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
> const qemu_info_t *info,
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v1 06/14] plugins: expand the callback typedef kernel-docs
2021-03-12 17:28 ` [PATCH v1 06/14] plugins: expand the callback typedef kernel-docs Alex Bennée
@ 2021-03-12 18:25 ` Aaron Lindsay via
2021-03-15 18:04 ` Alex Bennée
0 siblings, 1 reply; 31+ messages in thread
From: Aaron Lindsay via @ 2021-03-12 18:25 UTC (permalink / raw
To: Alex Bennée
Cc: qemu-devel, cota, kuhn.chenqun, robhenry, mahmoudabdalghany
On Mar 12 17:28, Alex Bennée wrote:
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
One nit below, but otherwise:
Reviewed-by: Aaron Lindsay <aaron@os.amperecomputing.com>
> ---
> include/qemu/qemu-plugin.h | 25 ++++++++++++++++++++++---
> 1 file changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index ac1bb318da..09b235f0b4 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -99,17 +99,36 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
> const qemu_info_t *info,
> int argc, char **argv);
>
> -/*
> - * Prototypes for the various callback styles we will be registering
> - * in the following functions.
> +/**
> + * typedef qemu_plugin_simple_cb_t - simple callback
> + * @id: the unique qemu_plugin_id_t
> + *
> + * This call-back passes no information aside from the unique @id.
Should we be consistent about always using 'callback' or 'call-back'
instead of alternating? I tend to use 'callback', but I'm not sure I
have a solid reason to prefer it.
-Aaron
> */
> typedef void (*qemu_plugin_simple_cb_t)(qemu_plugin_id_t id);
>
> +/**
> + * typedef qemu_plugin_udata_cb_t - callback with user data
> + * @id: the unique qemu_plugin_id_t
> + * @userdata: a pointer to some user data supplied when the call-back
> + * was registered.
> + */
> typedef void (*qemu_plugin_udata_cb_t)(qemu_plugin_id_t id, void *userdata);
>
> +/**
> + * typedef qemu_plugin_vcpu_simple_cb_t - vcpu callback
> + * @id: the unique qemu_plugin_id_t
> + * @vcpu_index: the current vcpu context
> + */
> typedef void (*qemu_plugin_vcpu_simple_cb_t)(qemu_plugin_id_t id,
> unsigned int vcpu_index);
>
> +/**
> + * typedef qemu_plugin_vcpu_udata_cb_t - vcpu callback
> + * @vcpu_index: the current vcpu context
> + * @userdata: a pointer to some user data supplied when the call-back
> + * was registered.
> + */
> typedef void (*qemu_plugin_vcpu_udata_cb_t)(unsigned int vcpu_index,
> void *userdata);
>
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v1 07/14] plugins: expand the typedef kernel-docs for translation
2021-03-12 17:28 ` [PATCH v1 07/14] plugins: expand the typedef kernel-docs for translation Alex Bennée
@ 2021-03-12 18:27 ` Aaron Lindsay via
0 siblings, 0 replies; 31+ messages in thread
From: Aaron Lindsay via @ 2021-03-12 18:27 UTC (permalink / raw
To: Alex Bennée
Cc: qemu-devel, cota, kuhn.chenqun, robhenry, mahmoudabdalghany
On Mar 12 17:28, Alex Bennée wrote:
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Aaron Lindsay <aaron@os.amperecomputing.com>
> ---
> include/qemu/qemu-plugin.h | 17 ++++++++++-------
> 1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index 09b235f0b4..9ae3940d89 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -202,11 +202,9 @@ void qemu_plugin_register_vcpu_idle_cb(qemu_plugin_id_t id,
> void qemu_plugin_register_vcpu_resume_cb(qemu_plugin_id_t id,
> qemu_plugin_vcpu_simple_cb_t cb);
>
> -/*
> - * Opaque types that the plugin is given during the translation and
> - * instrumentation phase.
> - */
> +/** struct qemu_plugin_tb - Opaque handle for a translation block */
> struct qemu_plugin_tb;
> +/** struct qemu_plugin_insn - Opaque handle for a translated instruction */
> struct qemu_plugin_insn;
>
> enum qemu_plugin_cb_flags {
> @@ -221,6 +219,14 @@ enum qemu_plugin_mem_rw {
> QEMU_PLUGIN_MEM_RW,
> };
>
> +/**
> + * typedef qemu_plugin_vcpu_tb_trans_cb_t - translation callback
> + * @id: unique plugin id
> + * @tb: opaque handle used for querying and instrumenting a block.
> + */
> +typedef void (*qemu_plugin_vcpu_tb_trans_cb_t)(qemu_plugin_id_t id,
> + struct qemu_plugin_tb *tb);
> +
> /**
> * qemu_plugin_register_vcpu_tb_trans_cb() - register a translate cb
> * @id: plugin ID
> @@ -233,9 +239,6 @@ enum qemu_plugin_mem_rw {
> * callbacks to be triggered when the block or individual instruction
> * executes.
> */
> -typedef void (*qemu_plugin_vcpu_tb_trans_cb_t)(qemu_plugin_id_t id,
> - struct qemu_plugin_tb *tb);
> -
> void qemu_plugin_register_vcpu_tb_trans_cb(qemu_plugin_id_t id,
> qemu_plugin_vcpu_tb_trans_cb_t cb);
>
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v1 08/14] plugins: add qemu_plugin_cb_flags to kernel-doc
2021-03-12 17:28 ` [PATCH v1 08/14] plugins: add qemu_plugin_cb_flags to kernel-doc Alex Bennée
@ 2021-03-12 18:29 ` Aaron Lindsay via
2021-03-16 13:40 ` Alex Bennée
0 siblings, 1 reply; 31+ messages in thread
From: Aaron Lindsay via @ 2021-03-12 18:29 UTC (permalink / raw
To: Alex Bennée
Cc: qemu-devel, cota, kuhn.chenqun, robhenry, mahmoudabdalghany
On Mar 12 17:28, Alex Bennée wrote:
> Also add a note to explain currently they are unused.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
I'm personally interested in one clarification below, but don't think
that affects my:
Reviewed-by: Aaron Lindsay <aaron@os.amperecomputing.com>
> ---
> include/qemu/qemu-plugin.h | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index 9ae3940d89..c98866a637 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -207,10 +207,20 @@ struct qemu_plugin_tb;
> /** struct qemu_plugin_insn - Opaque handle for a translated instruction */
> struct qemu_plugin_insn;
>
> +/**
> + * enum qemu_plugin_cb_flags - type of callback
> + *
> + * @QEMU_PLUGIN_CB_NO_REGS: callback does not access the CPU's regs
> + * @QEMU_PLUGIN_CB_R_REGS: callback reads the CPU's regs
> + * @QEMU_PLUGIN_CB_RW_REGS: callback reads and writes the CPU's regs
> + *
> + * Note: currently unused, plugins cannot read or change system
> + * register state.
They are unused in the sense that the current plugin interface does not
provide a way to make use of them. But are they completely free from
side effects?
-Aaron
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v1 09/14] plugins: add qemu_plugin_id_t to kernel-doc
2021-03-12 17:28 ` [PATCH v1 09/14] plugins: add qemu_plugin_id_t " Alex Bennée
@ 2021-03-12 18:29 ` Aaron Lindsay via
0 siblings, 0 replies; 31+ messages in thread
From: Aaron Lindsay via @ 2021-03-12 18:29 UTC (permalink / raw
To: Alex Bennée
Cc: qemu-devel, cota, kuhn.chenqun, robhenry, mahmoudabdalghany
On Mar 12 17:28, Alex Bennée wrote:
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Aaron Lindsay <aaron@os.amperecomputing.com>
> ---
> include/qemu/qemu-plugin.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index c98866a637..5ac6fe5f02 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -32,6 +32,9 @@
> #define QEMU_PLUGIN_LOCAL __attribute__((visibility("hidden")))
> #endif
>
> +/**
> + * typedef qemu_plugin_id_t - Unique plugin ID
> + */
> typedef uint64_t qemu_plugin_id_t;
>
> /*
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v1 10/14] plugins: expand inline exec kernel-doc documentation.
2021-03-12 17:28 ` [PATCH v1 10/14] plugins: expand inline exec kernel-doc documentation Alex Bennée
@ 2021-03-12 18:30 ` Aaron Lindsay via
0 siblings, 0 replies; 31+ messages in thread
From: Aaron Lindsay via @ 2021-03-12 18:30 UTC (permalink / raw
To: Alex Bennée
Cc: qemu-devel, cota, kuhn.chenqun, robhenry, mahmoudabdalghany
On Mar 12 17:28, Alex Bennée wrote:
> Remove the extraneous @cb parameter and document the non-atomic nature
> of the INLINE_ADD_U64 operation.
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Aaron Lindsay <aaron@os.amperecomputing.com>
> ---
> include/qemu/qemu-plugin.h | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index 5ac6fe5f02..dc05fc1932 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -269,6 +269,14 @@ void qemu_plugin_register_vcpu_tb_exec_cb(struct qemu_plugin_tb *tb,
> enum qemu_plugin_cb_flags flags,
> void *userdata);
>
> +/**
> + * enum qemu_plugin_op - describes an inline op
> + *
> + * @QEMU_PLUGIN_INLINE_ADD_U64: add an immediate value uint64_t
> + *
> + * Note: currently only a single inline op is supported.
> + */
> +
> enum qemu_plugin_op {
> QEMU_PLUGIN_INLINE_ADD_U64,
> };
> @@ -283,6 +291,9 @@ enum qemu_plugin_op {
> * Insert an inline op to every time a translated unit executes.
> * Useful if you just want to increment a single counter somewhere in
> * memory.
> + *
> + * Note: ops are not atomic so in multi-threaded/multi-smp situations
> + * you will get inexact results.
> */
> void qemu_plugin_register_vcpu_tb_exec_inline(struct qemu_plugin_tb *tb,
> enum qemu_plugin_op op,
> @@ -305,7 +316,6 @@ void qemu_plugin_register_vcpu_insn_exec_cb(struct qemu_plugin_insn *insn,
> /**
> * qemu_plugin_register_vcpu_insn_exec_inline() - insn execution inline op
> * @insn: the opaque qemu_plugin_insn handle for an instruction
> - * @cb: callback function
> * @op: the type of qemu_plugin_op (e.g. ADD_U64)
> * @ptr: the target memory location for the op
> * @imm: the op data (e.g. 1)
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v1 11/14] plugins: expand kernel-doc for instruction query and instrumentation
2021-03-12 17:28 ` [PATCH v1 11/14] plugins: expand kernel-doc for instruction query and instrumentation Alex Bennée
@ 2021-03-12 18:36 ` Aaron Lindsay via
2021-03-16 13:48 ` Alex Bennée
0 siblings, 1 reply; 31+ messages in thread
From: Aaron Lindsay via @ 2021-03-12 18:36 UTC (permalink / raw
To: Alex Bennée
Cc: qemu-devel, cota, kuhn.chenqun, robhenry, mahmoudabdalghany
A few clarifications inline:
On Mar 12 17:28, Alex Bennée wrote:
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
> include/qemu/qemu-plugin.h | 52 ++++++++++++++++++++++++++++++++++++--
> 1 file changed, 50 insertions(+), 2 deletions(-)
>
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index dc05fc1932..d4adce730a 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -327,21 +327,69 @@ void qemu_plugin_register_vcpu_insn_exec_inline(struct qemu_plugin_insn *insn,
> enum qemu_plugin_op op,
> void *ptr, uint64_t imm);
>
> -/*
> - * Helpers to query information about the instructions in a block
> +/**
> + * qemu_plugin_tb_n_insns() - query helper for number of insns in TB
> + * @tb: opaque handle to TB passed to callback
> + *
> + * Returns: number of instructions in this block
> */
> size_t qemu_plugin_tb_n_insns(const struct qemu_plugin_tb *tb);
>
> +/**
> + * qemu_plugin_tb_vaddr() - query helper for vaddr of TB start
> + * @tb: opaque handle to TB passed to callback
> + *
> + * Returns: virtual address of block start
> + */
> uint64_t qemu_plugin_tb_vaddr(const struct qemu_plugin_tb *tb);
>
> +/**
> + * qemu_plugin_tb_get_insn() - retrieve handle for instruction
> + * @tb: opaque handle to TB passed to callback
> + * @idx: instruction number, 0 indexed
> + *
> + * The returned handle can be used in follow up helper queries as well
> + * as when instrumenting an instruction. It is only valid for the
> + * lifetime of the callback.
> + *
> + * Returns: opaque handle to instruction
> + */
> struct qemu_plugin_insn *
> qemu_plugin_tb_get_insn(const struct qemu_plugin_tb *tb, size_t idx);
>
> +/**
> + * qemu_plugin_insn_data() - return ptr to instruction data
> + * @insn: opaque instruction handle from qemu_plugin_tb_get_insn()
> + *
> + * Note: data is only valid for duration of callback. See
> + * qemu_plugin_insn_size() to calculate size of stream.
> + *
> + * Returns: pointer to a stream of bytes
Maybe this could be a little more explicit, something like "Returns:
pointer to a stream of bytes containing the value of this instruction's
opcode"?
> + */
> const void *qemu_plugin_insn_data(const struct qemu_plugin_insn *insn);
>
> +/**
> + * qemu_plugin_insn_size() - return size of instruction
> + * @insn: opaque instruction handle from qemu_plugin_tb_get_insn()
> + *
> + * Returns: size of instruction
size in bytes?
> + */
> size_t qemu_plugin_insn_size(const struct qemu_plugin_insn *insn);
>
> +/**
> + * qemu_plugin_insn_vaddr() - return vaddr of instruction
> + * @insn: opaque instruction handle from qemu_plugin_tb_get_insn()
> + *
> + * Returns: virtual address of instruction
> + */
> uint64_t qemu_plugin_insn_vaddr(const struct qemu_plugin_insn *insn);
> +
> +/**
> + * qemu_plugin_insn_haddr() - return vaddr of instruction
Copypasta: s/vaddr/haddr/ ?
> + * @insn: opaque instruction handle from qemu_plugin_tb_get_insn()
> + *
> + * Returns: hardware (physical) address of instruction
> + */
> void *qemu_plugin_insn_haddr(const struct qemu_plugin_insn *insn);
Is this the physical address of the instruction on the host or target?
-Aaron
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v1 12/14] plugins: expand kernel-doc for memory query and instrumentation
2021-03-12 17:28 ` [PATCH v1 12/14] plugins: expand kernel-doc for memory " Alex Bennée
@ 2021-03-12 18:40 ` Aaron Lindsay via
2021-03-16 13:55 ` Alex Bennée
0 siblings, 1 reply; 31+ messages in thread
From: Aaron Lindsay via @ 2021-03-12 18:40 UTC (permalink / raw
To: Alex Bennée
Cc: qemu-devel, cota, kuhn.chenqun, robhenry, mahmoudabdalghany
On Mar 12 17:28, Alex Bennée wrote:
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Small comment below, but otherwise:
Reviewed-by: Aaron Lindsay <aaron@os.amperecomputing.com>
> ---
> include/qemu/qemu-plugin.h | 35 ++++++++++++++++++++++++++++-------
> 1 file changed, 28 insertions(+), 7 deletions(-)
>
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index d4adce730a..aed868d42a 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -392,24 +392,45 @@ uint64_t qemu_plugin_insn_vaddr(const struct qemu_plugin_insn *insn);
> */
> void *qemu_plugin_insn_haddr(const struct qemu_plugin_insn *insn);
>
> -/*
> - * Memory Instrumentation
> - *
> - * The anonymous qemu_plugin_meminfo_t and qemu_plugin_hwaddr types
> - * can be used in queries to QEMU to get more information about a
> - * given memory access.
> +/**
> + * typedef qemu_plugin_meminfo_t - opaque memory transaction handle
Would it still be useful to include the types of things you can do
with a qemu_plugin_meminfo_t here?
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v1 06/14] plugins: expand the callback typedef kernel-docs
2021-03-12 18:25 ` Aaron Lindsay via
@ 2021-03-15 18:04 ` Alex Bennée
0 siblings, 0 replies; 31+ messages in thread
From: Alex Bennée @ 2021-03-15 18:04 UTC (permalink / raw
To: Aaron Lindsay; +Cc: kuhn.chenqun, cota, qemu-devel, robhenry, mahmoudabdalghany
Aaron Lindsay <aaron@os.amperecomputing.com> writes:
> On Mar 12 17:28, Alex Bennée wrote:
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> One nit below, but otherwise:
>
> Reviewed-by: Aaron Lindsay <aaron@os.amperecomputing.com>
>
>> ---
>> include/qemu/qemu-plugin.h | 25 ++++++++++++++++++++++---
>> 1 file changed, 22 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
>> index ac1bb318da..09b235f0b4 100644
>> --- a/include/qemu/qemu-plugin.h
>> +++ b/include/qemu/qemu-plugin.h
>> @@ -99,17 +99,36 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
>> const qemu_info_t *info,
>> int argc, char **argv);
>>
>> -/*
>> - * Prototypes for the various callback styles we will be registering
>> - * in the following functions.
>> +/**
>> + * typedef qemu_plugin_simple_cb_t - simple callback
>> + * @id: the unique qemu_plugin_id_t
>> + *
>> + * This call-back passes no information aside from the unique @id.
>
> Should we be consistent about always using 'callback' or 'call-back'
> instead of alternating? I tend to use 'callback', but I'm not sure I
> have a solid reason to prefer it.
No you are right we should stick to callback.
>
> -Aaron
>
>> */
>> typedef void (*qemu_plugin_simple_cb_t)(qemu_plugin_id_t id);
>>
>> +/**
>> + * typedef qemu_plugin_udata_cb_t - callback with user data
>> + * @id: the unique qemu_plugin_id_t
>> + * @userdata: a pointer to some user data supplied when the call-back
>> + * was registered.
>> + */
>> typedef void (*qemu_plugin_udata_cb_t)(qemu_plugin_id_t id, void *userdata);
>>
>> +/**
>> + * typedef qemu_plugin_vcpu_simple_cb_t - vcpu callback
>> + * @id: the unique qemu_plugin_id_t
>> + * @vcpu_index: the current vcpu context
>> + */
>> typedef void (*qemu_plugin_vcpu_simple_cb_t)(qemu_plugin_id_t id,
>> unsigned int vcpu_index);
>>
>> +/**
>> + * typedef qemu_plugin_vcpu_udata_cb_t - vcpu callback
>> + * @vcpu_index: the current vcpu context
>> + * @userdata: a pointer to some user data supplied when the call-back
>> + * was registered.
>> + */
>> typedef void (*qemu_plugin_vcpu_udata_cb_t)(unsigned int vcpu_index,
>> void *userdata);
>>
>> --
>> 2.20.1
>>
--
Alex Bennée
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v1 08/14] plugins: add qemu_plugin_cb_flags to kernel-doc
2021-03-12 18:29 ` Aaron Lindsay via
@ 2021-03-16 13:40 ` Alex Bennée
2021-03-16 14:05 ` Aaron Lindsay via
0 siblings, 1 reply; 31+ messages in thread
From: Alex Bennée @ 2021-03-16 13:40 UTC (permalink / raw
To: Aaron Lindsay; +Cc: kuhn.chenqun, cota, qemu-devel, robhenry, mahmoudabdalghany
Aaron Lindsay <aaron@os.amperecomputing.com> writes:
> On Mar 12 17:28, Alex Bennée wrote:
>> Also add a note to explain currently they are unused.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> I'm personally interested in one clarification below, but don't think
> that affects my:
>
> Reviewed-by: Aaron Lindsay <aaron@os.amperecomputing.com>
>
>> ---
>> include/qemu/qemu-plugin.h | 16 +++++++++++++---
>> 1 file changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
>> index 9ae3940d89..c98866a637 100644
>> --- a/include/qemu/qemu-plugin.h
>> +++ b/include/qemu/qemu-plugin.h
>> @@ -207,10 +207,20 @@ struct qemu_plugin_tb;
>> /** struct qemu_plugin_insn - Opaque handle for a translated instruction */
>> struct qemu_plugin_insn;
>>
>> +/**
>> + * enum qemu_plugin_cb_flags - type of callback
>> + *
>> + * @QEMU_PLUGIN_CB_NO_REGS: callback does not access the CPU's regs
>> + * @QEMU_PLUGIN_CB_R_REGS: callback reads the CPU's regs
>> + * @QEMU_PLUGIN_CB_RW_REGS: callback reads and writes the CPU's regs
>> + *
>> + * Note: currently unused, plugins cannot read or change system
>> + * register state.
>
> They are unused in the sense that the current plugin interface does not
> provide a way to make use of them. But are they completely free from
> side effects?
They are free of side effects visible to the plugin. Under the covers it
uses the existing TCG_CALL_NO_* mechanics to ensure that register state
is synced to/from TCG temporaries before the callback.
>
> -Aaron
--
Alex Bennée
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v1 11/14] plugins: expand kernel-doc for instruction query and instrumentation
2021-03-12 18:36 ` Aaron Lindsay via
@ 2021-03-16 13:48 ` Alex Bennée
2021-03-16 14:00 ` Aaron Lindsay via
0 siblings, 1 reply; 31+ messages in thread
From: Alex Bennée @ 2021-03-16 13:48 UTC (permalink / raw
To: Aaron Lindsay; +Cc: kuhn.chenqun, cota, qemu-devel, robhenry, mahmoudabdalghany
Aaron Lindsay <aaron@os.amperecomputing.com> writes:
> A few clarifications inline:
>
> On Mar 12 17:28, Alex Bennée wrote:
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>> include/qemu/qemu-plugin.h | 52 ++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 50 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
>> index dc05fc1932..d4adce730a 100644
>> --- a/include/qemu/qemu-plugin.h
>> +++ b/include/qemu/qemu-plugin.h
>> @@ -327,21 +327,69 @@ void qemu_plugin_register_vcpu_insn_exec_inline(struct qemu_plugin_insn *insn,
>> enum qemu_plugin_op op,
>> void *ptr, uint64_t imm);
>>
>> -/*
>> - * Helpers to query information about the instructions in a block
>> +/**
>> + * qemu_plugin_tb_n_insns() - query helper for number of insns in TB
>> + * @tb: opaque handle to TB passed to callback
>> + *
>> + * Returns: number of instructions in this block
>> */
>> size_t qemu_plugin_tb_n_insns(const struct qemu_plugin_tb *tb);
>>
>> +/**
>> + * qemu_plugin_tb_vaddr() - query helper for vaddr of TB start
>> + * @tb: opaque handle to TB passed to callback
>> + *
>> + * Returns: virtual address of block start
>> + */
>> uint64_t qemu_plugin_tb_vaddr(const struct qemu_plugin_tb *tb);
>>
>> +/**
>> + * qemu_plugin_tb_get_insn() - retrieve handle for instruction
>> + * @tb: opaque handle to TB passed to callback
>> + * @idx: instruction number, 0 indexed
>> + *
>> + * The returned handle can be used in follow up helper queries as well
>> + * as when instrumenting an instruction. It is only valid for the
>> + * lifetime of the callback.
>> + *
>> + * Returns: opaque handle to instruction
>> + */
>> struct qemu_plugin_insn *
>> qemu_plugin_tb_get_insn(const struct qemu_plugin_tb *tb, size_t idx);
>>
>> +/**
>> + * qemu_plugin_insn_data() - return ptr to instruction data
>> + * @insn: opaque instruction handle from qemu_plugin_tb_get_insn()
>> + *
>> + * Note: data is only valid for duration of callback. See
>> + * qemu_plugin_insn_size() to calculate size of stream.
>> + *
>> + * Returns: pointer to a stream of bytes
>
> Maybe this could be a little more explicit, something like "Returns:
> pointer to a stream of bytes containing the value of this instruction's
> opcode"?
>
>> + */
>> const void *qemu_plugin_insn_data(const struct qemu_plugin_insn *insn);
>>
>> +/**
>> + * qemu_plugin_insn_size() - return size of instruction
>> + * @insn: opaque instruction handle from qemu_plugin_tb_get_insn()
>> + *
>> + * Returns: size of instruction
>
> size in bytes?
>
>> + */
>> size_t qemu_plugin_insn_size(const struct qemu_plugin_insn *insn);
>>
>> +/**
>> + * qemu_plugin_insn_vaddr() - return vaddr of instruction
>> + * @insn: opaque instruction handle from qemu_plugin_tb_get_insn()
>> + *
>> + * Returns: virtual address of instruction
>> + */
>> uint64_t qemu_plugin_insn_vaddr(const struct qemu_plugin_insn *insn);
>> +
>> +/**
>> + * qemu_plugin_insn_haddr() - return vaddr of instruction
>
> Copypasta: s/vaddr/haddr/ ?
>
>> + * @insn: opaque instruction handle from qemu_plugin_tb_get_insn()
>> + *
>> + * Returns: hardware (physical) address of instruction
>> + */
>> void *qemu_plugin_insn_haddr(const struct qemu_plugin_insn *insn);
>
> Is this the physical address of the instruction on the host or target?
target.
>
> -Aaron
--
Alex Bennée
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v1 12/14] plugins: expand kernel-doc for memory query and instrumentation
2021-03-12 18:40 ` Aaron Lindsay via
@ 2021-03-16 13:55 ` Alex Bennée
0 siblings, 0 replies; 31+ messages in thread
From: Alex Bennée @ 2021-03-16 13:55 UTC (permalink / raw
To: Aaron Lindsay; +Cc: kuhn.chenqun, cota, qemu-devel, robhenry, mahmoudabdalghany
Aaron Lindsay <aaron@os.amperecomputing.com> writes:
> On Mar 12 17:28, Alex Bennée wrote:
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> Small comment below, but otherwise:
>
> Reviewed-by: Aaron Lindsay <aaron@os.amperecomputing.com>
>
>> ---
>> include/qemu/qemu-plugin.h | 35 ++++++++++++++++++++++++++++-------
>> 1 file changed, 28 insertions(+), 7 deletions(-)
>>
>> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
>> index d4adce730a..aed868d42a 100644
>> --- a/include/qemu/qemu-plugin.h
>> +++ b/include/qemu/qemu-plugin.h
>> @@ -392,24 +392,45 @@ uint64_t qemu_plugin_insn_vaddr(const struct qemu_plugin_insn *insn);
>> */
>> void *qemu_plugin_insn_haddr(const struct qemu_plugin_insn *insn);
>>
>> -/*
>> - * Memory Instrumentation
>> - *
>> - * The anonymous qemu_plugin_meminfo_t and qemu_plugin_hwaddr types
>> - * can be used in queries to QEMU to get more information about a
>> - * given memory access.
>> +/**
>> + * typedef qemu_plugin_meminfo_t - opaque memory transaction handle
>
> Would it still be useful to include the types of things you can do
> with a qemu_plugin_meminfo_t here?
I've referenced it can be "..further queried using the qemu_plugin_mem_*
query functions."
--
Alex Bennée
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v1 11/14] plugins: expand kernel-doc for instruction query and instrumentation
2021-03-16 13:48 ` Alex Bennée
@ 2021-03-16 14:00 ` Aaron Lindsay via
0 siblings, 0 replies; 31+ messages in thread
From: Aaron Lindsay via @ 2021-03-16 14:00 UTC (permalink / raw
To: Alex Bennée
Cc: qemu-devel, cota, kuhn.chenqun, robhenry, mahmoudabdalghany
On Mar 16 13:48, Alex Bennée wrote:
> Aaron Lindsay <aaron@os.amperecomputing.com> writes:
> > On Mar 12 17:28, Alex Bennée wrote:
> >> + * @insn: opaque instruction handle from qemu_plugin_tb_get_insn()
> >> + *
> >> + * Returns: hardware (physical) address of instruction
> >> + */
> >> void *qemu_plugin_insn_haddr(const struct qemu_plugin_insn *insn);
> >
> > Is this the physical address of the instruction on the host or target?
>
> target.
An observation: We're exposing a target physical address here as a void
* and for memory accesses (qemu_plugin_hwaddr_phys_addr) as a uint64_t.
-Aaron
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v1 08/14] plugins: add qemu_plugin_cb_flags to kernel-doc
2021-03-16 13:40 ` Alex Bennée
@ 2021-03-16 14:05 ` Aaron Lindsay via
0 siblings, 0 replies; 31+ messages in thread
From: Aaron Lindsay via @ 2021-03-16 14:05 UTC (permalink / raw
To: Alex Bennée
Cc: qemu-devel, cota, kuhn.chenqun, robhenry, mahmoudabdalghany
On Mar 16 13:40, Alex Bennée wrote:
>
> Aaron Lindsay <aaron@os.amperecomputing.com> writes:
>
> > On Mar 12 17:28, Alex Bennée wrote:
> >> Also add a note to explain currently they are unused.
> >>
> >> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> >
> > I'm personally interested in one clarification below, but don't think
> > that affects my:
> >
> > Reviewed-by: Aaron Lindsay <aaron@os.amperecomputing.com>
> >
> >> ---
> >> include/qemu/qemu-plugin.h | 16 +++++++++++++---
> >> 1 file changed, 13 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> >> index 9ae3940d89..c98866a637 100644
> >> --- a/include/qemu/qemu-plugin.h
> >> +++ b/include/qemu/qemu-plugin.h
> >> @@ -207,10 +207,20 @@ struct qemu_plugin_tb;
> >> /** struct qemu_plugin_insn - Opaque handle for a translated instruction */
> >> struct qemu_plugin_insn;
> >>
> >> +/**
> >> + * enum qemu_plugin_cb_flags - type of callback
> >> + *
> >> + * @QEMU_PLUGIN_CB_NO_REGS: callback does not access the CPU's regs
> >> + * @QEMU_PLUGIN_CB_R_REGS: callback reads the CPU's regs
> >> + * @QEMU_PLUGIN_CB_RW_REGS: callback reads and writes the CPU's regs
> >> + *
> >> + * Note: currently unused, plugins cannot read or change system
> >> + * register state.
> >
> > They are unused in the sense that the current plugin interface does not
> > provide a way to make use of them. But are they completely free from
> > side effects?
>
> They are free of side effects visible to the plugin. Under the covers it
> uses the existing TCG_CALL_NO_* mechanics to ensure that register state
> is synced to/from TCG temporaries before the callback.
I would currently find it useful to have that information included in
the documentation since there is no register state exposed and I am
basically hacking something together for my own use in the meantime...
but I understand that is in tension with the general philosophy of the
plugins to not expose implementation details.
-Aaron
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2021-03-16 14:17 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-03-12 17:28 [PATCH v1 00/14] plugins/next (phys addr, syscalls, lots of docs) Alex Bennée
2021-03-12 17:28 ` [PATCH v1 01/14] plugins: new syscalls plugin Alex Bennée
2021-03-12 17:28 ` [PATCH v1 02/14] plugins: Expose physical addresses instead of device offsets Alex Bennée
2021-03-12 17:28 ` [PATCH v1 03/14] docs/devel: include the plugin API information from the headers Alex Bennée
2021-03-12 18:19 ` Aaron Lindsay via
2021-03-12 17:28 ` [PATCH v1 04/14] plugins: expand kernel-doc for qemu_info_t Alex Bennée
2021-03-12 18:20 ` Aaron Lindsay via
2021-03-12 17:28 ` [PATCH v1 05/14] plugins: cleanup kernel-doc for qemu_plugin_install Alex Bennée
2021-03-12 18:21 ` Aaron Lindsay via
2021-03-12 17:28 ` [PATCH v1 06/14] plugins: expand the callback typedef kernel-docs Alex Bennée
2021-03-12 18:25 ` Aaron Lindsay via
2021-03-15 18:04 ` Alex Bennée
2021-03-12 17:28 ` [PATCH v1 07/14] plugins: expand the typedef kernel-docs for translation Alex Bennée
2021-03-12 18:27 ` Aaron Lindsay via
2021-03-12 17:28 ` [PATCH v1 08/14] plugins: add qemu_plugin_cb_flags to kernel-doc Alex Bennée
2021-03-12 18:29 ` Aaron Lindsay via
2021-03-16 13:40 ` Alex Bennée
2021-03-16 14:05 ` Aaron Lindsay via
2021-03-12 17:28 ` [PATCH v1 09/14] plugins: add qemu_plugin_id_t " Alex Bennée
2021-03-12 18:29 ` Aaron Lindsay via
2021-03-12 17:28 ` [PATCH v1 10/14] plugins: expand inline exec kernel-doc documentation Alex Bennée
2021-03-12 18:30 ` Aaron Lindsay via
2021-03-12 17:28 ` [PATCH v1 11/14] plugins: expand kernel-doc for instruction query and instrumentation Alex Bennée
2021-03-12 18:36 ` Aaron Lindsay via
2021-03-16 13:48 ` Alex Bennée
2021-03-16 14:00 ` Aaron Lindsay via
2021-03-12 17:28 ` [PATCH v1 12/14] plugins: expand kernel-doc for memory " Alex Bennée
2021-03-12 18:40 ` Aaron Lindsay via
2021-03-16 13:55 ` Alex Bennée
2021-03-12 17:28 ` [PATCH v1 13/14] plugins: getting qemu_plugin_get_hwaddr only expose one function prototype Alex Bennée
2021-03-12 17:28 ` [PATCH v1 14/14] plugins: Fixes typo in qemu-plugin.h Alex Bennée
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.