[PULL v2 00/15] misc fixes (strtoz, plugins, guest-loader)

[PULL v2 00/15] misc fixes (strtoz, plugins, guest-loader)

[Alex Bennée]

Apologies for the delay, it took several swings at the CI to get a
clean pass. I've dropped the SYS_HEAPINFO and final kernel-doc
integration patch for now. I've also nabbed rth's strtoz fix although
that is also in another 1 change PR he submitted earlier.

The following changes since commit 5b7f5586d182b0cafb1f8d558992a14763e2953e:

  Merge remote-tracking branch 'remotes/kraxel/tags/usb-20210315-pull-request' into staging (2021-03-16 13:17:54 +0000)

are available in the Git repository at:

  https://github.com/stsquad/qemu.git tags/pull-misc-6.0-updates-170321-2

for you to fetch changes up to 9e7118023fda7c29016038e2292d4d14129b63da:

  hw/core: Only build guest-loader if libfdt is available (2021-03-17 07:17:46 +0000)

----------------------------------------------------------------
Final fixes for 6.0

  - plugins physical address changes
  - syscall tracking plugin
  - plugin kernel-doc comments (without integration)
  - libfdt build fix for guest-loader

----------------------------------------------------------------
Aaron Lindsay (1):
      plugins: Expose physical addresses instead of device offsets

Alex Bennée (9):
      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

Philippe Mathieu-Daudé (1):
      hw/core: Only build guest-loader if libfdt is available

Richard Henderson (1):
      utils: Use fixed-point arithmetic in qemu_strtosz

Yonggang Luo (2):
      plugins: getting qemu_plugin_get_hwaddr only expose one function prototype
      plugins: Fixes typo in qemu-plugin.h

 include/qemu/qemu-plugin.h  | 232 +++++++++++++++++++++++++++++++++++---------
 contrib/plugins/hotpages.c  |   2 +-
 contrib/plugins/hwprofile.c |   2 +-
 plugins/api.c               |  25 +++--
 tests/plugin/syscall.c      |  49 ++++++++++
 tests/unit/test-cutils.c    |   2 +-
 util/cutils.c               |  50 +++++++---
 hw/core/Kconfig             |   5 +
 hw/core/meson.build         |   3 +-
 tests/plugin/meson.build    |   2 +-
 10 files changed, 297 insertions(+), 75 deletions(-)
 create mode 100644 tests/plugin/syscall.c

-- 
2.20.1

[PULL v2 01/15] utils: Use fixed-point arithmetic in qemu_strtosz

[Alex Bennée]

From: Richard Henderson <richard.henderson@linaro.org>

Once we've parsed the fractional value, extract it into an integral
64-bit fraction.  Perform the scaling with integer arithmetic, and
simplify the overflow detection.

Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20210315185117.1986240-2-richard.henderson@linaro.org>

diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
index bad3a60993..e025b54c05 100644
--- a/tests/unit/test-cutils.c
+++ b/tests/unit/test-cutils.c
@@ -2128,7 +2128,7 @@ static void test_qemu_strtosz_float(void)
     str = "12.345M";
     err = qemu_strtosz(str, &endptr, &res);
     g_assert_cmpint(err, ==, 0);
-    g_assert_cmpint(res, ==, (uint64_t) (12.345 * MiB));
+    g_assert_cmpint(res, ==, (uint64_t) (12.345 * MiB + 0.5));
     g_assert(endptr == str + 7);
 }
 
diff --git a/util/cutils.c b/util/cutils.c
index d89a40a8c3..c442882b88 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -275,10 +275,9 @@ static int do_strtosz(const char *nptr, const char **end,
     int retval;
     const char *endptr, *f;
     unsigned char c;
-    bool mul_required = false, hex = false;
-    uint64_t val;
+    bool hex = false;
+    uint64_t val, valf = 0;
     int64_t mul;
-    double fraction = 0.0;
 
     /* Parse integral portion as decimal. */
     retval = qemu_strtou64(nptr, &endptr, 10, &val);
@@ -308,17 +307,19 @@ static int do_strtosz(const char *nptr, const char **end,
          * without fractional digits.  If we see an exponent, treat
          * the entire input as invalid instead.
          */
+        double fraction;
+
         f = endptr;
         retval = qemu_strtod_finite(f, &endptr, &fraction);
         if (retval) {
-            fraction = 0.0;
             endptr++;
         } else if (memchr(f, 'e', endptr - f) || memchr(f, 'E', endptr - f)) {
             endptr = nptr;
             retval = -EINVAL;
             goto out;
-        } else if (fraction != 0) {
-            mul_required = true;
+        } else {
+            /* Extract into a 64-bit fixed-point fraction. */
+            valf = (uint64_t)(fraction * 0x1p64);
         }
     }
     c = *endptr;
@@ -333,16 +334,35 @@ static int do_strtosz(const char *nptr, const char **end,
         mul = suffix_mul(default_suffix, unit);
         assert(mul > 0);
     }
-    if (mul == 1 && mul_required) {
-        endptr = nptr;
-        retval = -EINVAL;
-        goto out;
-    }
-    if (val > (UINT64_MAX - ((uint64_t) (fraction * mul))) / mul) {
-        retval = -ERANGE;
-        goto out;
+    if (mul == 1) {
+        /* When a fraction is present, a scale is required. */
+        if (valf != 0) {
+            endptr = nptr;
+            retval = -EINVAL;
+            goto out;
+        }
+    } else {
+        uint64_t valh, tmp;
+
+        /* Compute exact result: 64.64 x 64.0 -> 128.64 fixed point */
+        mulu64(&val, &valh, val, mul);
+        mulu64(&valf, &tmp, valf, mul);
+        val += tmp;
+        valh += val < tmp;
+
+        /* Round 0.5 upward. */
+        tmp = valf >> 63;
+        val += tmp;
+        valh += val < tmp;
+
+        /* Report overflow. */
+        if (valh != 0) {
+            retval = -ERANGE;
+            goto out;
+        }
     }
-    *result = val * mul + (uint64_t) (fraction * mul);
+
+    *result = val;
     retval = 0;
 
 out:
-- 
2.20.1

[PULL v2 02/15] plugins: new syscalls plugin

[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>
Message-Id: <20210312172821.31647-2-alex.bennee@linaro.org>

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

[PULL v2 03/15] plugins: Expose physical addresses instead of device offsets

[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>
Message-Id: <20210312172821.31647-3-alex.bennee@linaro.org>

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

[PULL v2 04/15] plugins: expand kernel-doc for qemu_info_t

[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>
Message-Id: <20210312172821.31647-5-alex.bennee@linaro.org>

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

[PULL v2 05/15] plugins: cleanup kernel-doc for qemu_plugin_install

[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>
Message-Id: <20210312172821.31647-6-alex.bennee@linaro.org>

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

[PULL v2 06/15] plugins: expand the callback typedef kernel-docs

[Alex Bennée]

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20210312172821.31647-7-alex.bennee@linaro.org>

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

[PULL v2 07/15] plugins: expand the typedef kernel-docs for translation

[Alex Bennée]

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20210312172821.31647-8-alex.bennee@linaro.org>

diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 09b235f0b4..529fe3e16b 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -103,14 +103,14 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
  * 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.
+ * This callback 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
+ * @userdata: a pointer to some user data supplied when the callback
  * was registered.
  */
 typedef void (*qemu_plugin_udata_cb_t)(qemu_plugin_id_t id, void *userdata);
@@ -126,7 +126,7 @@ typedef void (*qemu_plugin_vcpu_simple_cb_t)(qemu_plugin_id_t id,
 /**
  * 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
+ * @userdata: a pointer to some user data supplied when the callback
  * was registered.
  */
 typedef void (*qemu_plugin_vcpu_udata_cb_t)(unsigned int vcpu_index,
@@ -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

[PULL v2 08/15] plugins: add qemu_plugin_cb_flags to kernel-doc

[Alex Bennée]

Also add a note to explain currently they are unused.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20210312172821.31647-9-alex.bennee@linaro.org>

diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 529fe3e16b..e4d782b628 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

[PULL v2 09/15] plugins: add qemu_plugin_id_t to kernel-doc

[Alex Bennée]

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20210312172821.31647-10-alex.bennee@linaro.org>

diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index e4d782b628..272d240a8f 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

[PULL v2 10/15] plugins: expand inline exec kernel-doc documentation.

[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>
Message-Id: <20210312172821.31647-11-alex.bennee@linaro.org>

diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 272d240a8f..a3805bb299 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

[PULL v2 11/15] plugins: expand kernel-doc for instruction query and instrumentation

[Alex Bennée]

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20210312172821.31647-12-alex.bennee@linaro.org>

diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index a3805bb299..ad9dc4b69d 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -327,21 +327,70 @@ 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 containing the value of this
+ * instructions 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 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 hardware addr of instruction
+ * @insn: opaque instruction handle from qemu_plugin_tb_get_insn()
+ *
+ * Returns: hardware (physical) target address of instruction
+ */
 void *qemu_plugin_insn_haddr(const struct qemu_plugin_insn *insn);
 
 /*
-- 
2.20.1

[PULL v2 12/15] plugins: expand kernel-doc for memory query and instrumentation

[Alex Bennée]

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20210312172821.31647-13-alex.bennee@linaro.org>

diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index ad9dc4b69d..9e67ab1aa2 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -393,24 +393,48 @@ 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
+/**
+ * typedef qemu_plugin_meminfo_t - opaque memory transaction handle
  *
- * 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.
+ * This can be further queried using the qemu_plugin_mem_* query
+ * functions.
  */
 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

[PULL v2 13/15] plugins: getting qemu_plugin_get_hwaddr only expose one function prototype

[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>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20201013002806.1447-2-luoyonggang@gmail.com>
Message-Id: <20210312172821.31647-14-alex.bennee@linaro.org>

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

[PULL v2 14/15] plugins: Fixes typo in qemu-plugin.h

[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>
Message-Id: <20210312172821.31647-15-alex.bennee@linaro.org>

diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index 9e67ab1aa2..97cdfd7761 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

[PULL v2 15/15] hw/core: Only build guest-loader if libfdt is available

[Alex Bennée]

From: Philippe Mathieu-Daudé <philmd@redhat.com>

Add a Kconfig entry for guest-loader so we can optionally deselect
it (default is built in), and add a Meson dependency on libfdt.

This fixes when building with --disable-fdt:

  /usr/bin/ld: libcommon.fa.p/hw_core_guest-loader.c.o: in function `loader_insert_platform_data':
  hw/core/guest-loader.c:56: undefined reference to `qemu_fdt_add_subnode'
  /usr/bin/ld: hw/core/guest-loader.c:57: undefined reference to `qemu_fdt_setprop'
  /usr/bin/ld: hw/core/guest-loader.c:61: undefined reference to `qemu_fdt_setprop_string_array'
  /usr/bin/ld: hw/core/guest-loader.c:68: undefined reference to `qemu_fdt_setprop_string'
  /usr/bin/ld: hw/core/guest-loader.c:74: undefined reference to `qemu_fdt_setprop_string_array'
  collect2: error: ld returned 1 exit status

Fixes: a33ff6d2c6b ("hw/core: implement a guest-loader to support static hypervisor guests")
Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <20210315170439.2868903-1-philmd@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

diff --git a/hw/core/Kconfig b/hw/core/Kconfig
index fdf03514d7..9397503656 100644
--- a/hw/core/Kconfig
+++ b/hw/core/Kconfig
@@ -11,6 +11,11 @@ config GENERIC_LOADER
     bool
     default y
 
+config GUEST_LOADER
+    bool
+    default y
+    depends on TCG
+
 config OR_IRQ
     bool
 
diff --git a/hw/core/meson.build b/hw/core/meson.build
index 9cd72edf51..59f1605bb0 100644
--- a/hw/core/meson.build
+++ b/hw/core/meson.build
@@ -16,6 +16,7 @@ hwcore_files = files(
 common_ss.add(files('cpu.c'))
 common_ss.add(when: 'CONFIG_FITLOADER', if_true: files('loader-fit.c'))
 common_ss.add(when: 'CONFIG_GENERIC_LOADER', if_true: files('generic-loader.c'))
+common_ss.add(when: ['CONFIG_GUEST_LOADER', fdt], if_true: files('guest-loader.c'))
 common_ss.add(when: 'CONFIG_OR_IRQ', if_true: files('or-irq.c'))
 common_ss.add(when: 'CONFIG_PLATFORM_BUS', if_true: files('platform-bus.c'))
 common_ss.add(when: 'CONFIG_PTIMER', if_true: files('ptimer.c'))
@@ -37,8 +38,6 @@ softmmu_ss.add(files(
   'clock-vmstate.c',
 ))
 
-softmmu_ss.add(when: 'CONFIG_TCG', if_true: files('guest-loader.c'))
-
 specific_ss.add(when: 'CONFIG_SOFTMMU', if_true: files(
   'machine-qmp-cmds.c',
   'numa.c',
-- 
2.20.1

Re: [PULL v2 01/15] utils: Use fixed-point arithmetic in qemu_strtosz

[Philippe Mathieu-Daudé]

Hi Alex,

On 3/17/21 8:22 AM, Alex Bennée wrote:
> From: Richard Henderson <richard.henderson@linaro.org>
> 
> Once we've parsed the fractional value, extract it into an integral
> 64-bit fraction.  Perform the scaling with integer arithmetic, and
> simplify the overflow detection.
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20210315185117.1986240-2-richard.henderson@linaro.org>

Something is odd with your tooling, the '---' separator is missing.

The covers' tag is OK although.

> 
> diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
> index bad3a60993..e025b54c05 100644
> --- a/tests/unit/test-cutils.c
> +++ b/tests/unit/test-cutils.c
> @@ -2128,7 +2128,7 @@ static void test_qemu_strtosz_float(void)
>      str = "12.345M";
>      err = qemu_strtosz(str, &endptr, &res);
>      g_assert_cmpint(err, ==, 0);
> -    g_assert_cmpint(res, ==, (uint64_t) (12.345 * MiB));
> +    g_assert_cmpint(res, ==, (uint64_t) (12.345 * MiB + 0.5));
>      g_assert(endptr == str + 7);
>  }

Re: [PULL v2 15/15] hw/core: Only build guest-loader if libfdt is available

[Philippe Mathieu-Daudé]

Hi Alex,

On 3/17/21 8:22 AM, Alex Bennée wrote:
> From: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> Add a Kconfig entry for guest-loader so we can optionally deselect
> it (default is built in), and add a Meson dependency on libfdt.
> 
> This fixes when building with --disable-fdt:
> 
>   /usr/bin/ld: libcommon.fa.p/hw_core_guest-loader.c.o: in function `loader_insert_platform_data':
>   hw/core/guest-loader.c:56: undefined reference to `qemu_fdt_add_subnode'
>   /usr/bin/ld: hw/core/guest-loader.c:57: undefined reference to `qemu_fdt_setprop'
>   /usr/bin/ld: hw/core/guest-loader.c:61: undefined reference to `qemu_fdt_setprop_string_array'
>   /usr/bin/ld: hw/core/guest-loader.c:68: undefined reference to `qemu_fdt_setprop_string'
>   /usr/bin/ld: hw/core/guest-loader.c:74: undefined reference to `qemu_fdt_setprop_string_array'
>   collect2: error: ld returned 1 exit status
> 
> Fixes: a33ff6d2c6b ("hw/core: implement a guest-loader to support static hypervisor guests")
> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Message-Id: <20210315170439.2868903-1-philmd@redhat.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>

On Mon, Mar 15, 2021 at 06:04:39PM +0100, Philippe Mathieu-Daudé wrote:
>Add a Kconfig entry for guest-loader so we can optionally deselect
>it (default is built in), and add a Meson dependency on libfdt.
>
>This fixes when building with --disable-fdt:
>
>  /usr/bin/ld: libcommon.fa.p/hw_core_guest-loader.c.o: in function
`loader_insert_platform_data':
>  hw/core/guest-loader.c:56: undefined reference to `qemu_fdt_add_subnode'
>  /usr/bin/ld: hw/core/guest-loader.c:57: undefined reference to
`qemu_fdt_setprop'
>  /usr/bin/ld: hw/core/guest-loader.c:61: undefined reference to
`qemu_fdt_setprop_string_array'
>  /usr/bin/ld: hw/core/guest-loader.c:68: undefined reference to
`qemu_fdt_setprop_string'
>  /usr/bin/ld: hw/core/guest-loader.c:74: undefined reference to
`qemu_fdt_setprop_string_array'
>  collect2: error: ld returned 1 exit status
>
>Fixes: a33ff6d2c6b ("hw/core: implement a guest-loader to support
static hypervisor guests")
>Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
>Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

If you have to post a v3 due to merge conflict, this patch
also had:

Reviewed-by: Anthoine Bourgeois <anthoine.bourgeois@gmail.com>
Tested-by: Anthoine Bourgeois <anthoine.bourgeois@gmail.com>

(but it was sent off-list).

Thanks,

Phil.

Re: [PULL v2 01/15] utils: Use fixed-point arithmetic in qemu_strtosz

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> Hi Alex,
>
> On 3/17/21 8:22 AM, Alex Bennée wrote:
>> From: Richard Henderson <richard.henderson@linaro.org>
>> 
>> Once we've parsed the fractional value, extract it into an integral
>> 64-bit fraction.  Perform the scaling with integer arithmetic, and
>> simplify the overflow detection.
>> 
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Message-Id: <20210315185117.1986240-2-richard.henderson@linaro.org>
>
> Something is odd with your tooling, the '---' separator is missing.

Surely that's only when you have bellow the line comments? b4 strips
then when applying series.

>
> The covers' tag is OK although.
>
>> 
>> diff --git a/tests/unit/test-cutils.c b/tests/unit/test-cutils.c
>> index bad3a60993..e025b54c05 100644
>> --- a/tests/unit/test-cutils.c
>> +++ b/tests/unit/test-cutils.c
>> @@ -2128,7 +2128,7 @@ static void test_qemu_strtosz_float(void)
>>      str = "12.345M";
>>      err = qemu_strtosz(str, &endptr, &res);
>>      g_assert_cmpint(err, ==, 0);
>> -    g_assert_cmpint(res, ==, (uint64_t) (12.345 * MiB));
>> +    g_assert_cmpint(res, ==, (uint64_t) (12.345 * MiB + 0.5));
>>      g_assert(endptr == str + 7);
>>  }


-- 
Alex Bennée

Re: [PULL v2 01/15] utils: Use fixed-point arithmetic in qemu_strtosz

[Philippe Mathieu-Daudé]

On 3/17/21 1:13 PM, Alex Bennée wrote:
> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> 
>> Hi Alex,
>>
>> On 3/17/21 8:22 AM, Alex Bennée wrote:
>>> From: Richard Henderson <richard.henderson@linaro.org>
>>>
>>> Once we've parsed the fractional value, extract it into an integral
>>> 64-bit fraction.  Perform the scaling with integer arithmetic, and
>>> simplify the overflow detection.
>>>
>>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> Message-Id: <20210315185117.1986240-2-richard.henderson@linaro.org>
>>
>> Something is odd with your tooling, the '---' separator is missing.
> 
> Surely that's only when you have bellow the line comments? b4 strips
> then when applying series.

Yes, the problem is your series doesn't apply on top of 7625a1ed013
("utils: Use fixed-point arithmetic in qemu_strtosz")

$ git am v2_20210317_alex_bennee_misc_fixes_strtoz_plugins_guest_loader.mbx
Applying: utils: Use fixed-point arithmetic in qemu_strtosz
error: patch failed: tests/unit/test-cutils.c:2128
error: tests/unit/test-cutils.c: patch does not apply
error: patch failed: util/cutils.c:275
error: util/cutils.c: patch does not apply
Patch failed at 0001 utils: Use fixed-point arithmetic in qemu_strtosz
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

But skipping this patch, the rest can be applied properly by git-am.

Re: [PULL v2 01/15] utils: Use fixed-point arithmetic in qemu_strtosz

[Eric Blake]

On 3/17/21 8:16 AM, Philippe Mathieu-Daudé wrote:

> Yes, the problem is your series doesn't apply on top of 7625a1ed013
> ("utils: Use fixed-point arithmetic in qemu_strtosz")
> 
> $ git am v2_20210317_alex_bennee_misc_fixes_strtoz_plugins_guest_loader.mbx
> Applying: utils: Use fixed-point arithmetic in qemu_strtosz
> error: patch failed: tests/unit/test-cutils.c:2128
> error: tests/unit/test-cutils.c: patch does not apply
> error: patch failed: util/cutils.c:275
> error: util/cutils.c: patch does not apply
> Patch failed at 0001 utils: Use fixed-point arithmetic in qemu_strtosz
> hint: Use 'git am --show-current-patch=diff' to see the failed patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> 
> But skipping this patch, the rest can be applied properly by git-am.

Merging the same patch twice through two independent pull requests is
not a problem with git.  Applying the patches of a pull request is
different than applying a merge request directly (where you failed
trying to reapply a patch that is already present).  There's no need to
respin this pull request just to drop patch 1, but if there is another
reason to respin, rebasing the series will automatically drop patch 1
because it is already upstream through rth's pull request (as you noted,
commit 7625a1ed).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PULL v2 01/15] utils: Use fixed-point arithmetic in qemu_strtosz

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 3/17/21 1:13 PM, Alex Bennée wrote:
>> Philippe Mathieu-Daudé <philmd@redhat.com> writes:
>> 
>>> Hi Alex,
>>>
>>> On 3/17/21 8:22 AM, Alex Bennée wrote:
>>>> From: Richard Henderson <richard.henderson@linaro.org>
>>>>
>>>> Once we've parsed the fractional value, extract it into an integral
>>>> 64-bit fraction.  Perform the scaling with integer arithmetic, and
>>>> simplify the overflow detection.
>>>>
>>>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>>> Message-Id: <20210315185117.1986240-2-richard.henderson@linaro.org>
>>>
>>> Something is odd with your tooling, the '---' separator is missing.
>> 
>> Surely that's only when you have bellow the line comments? b4 strips
>> then when applying series.
>
> Yes, the problem is your series doesn't apply on top of 7625a1ed013
> ("utils: Use fixed-point arithmetic in qemu_strtosz")
>
> $ git am v2_20210317_alex_bennee_misc_fixes_strtoz_plugins_guest_loader.mbx
> Applying: utils: Use fixed-point arithmetic in qemu_strtosz
> error: patch failed: tests/unit/test-cutils.c:2128
> error: tests/unit/test-cutils.c: patch does not apply
> error: patch failed: util/cutils.c:275
> error: util/cutils.c: patch does not apply
> Patch failed at 0001 utils: Use fixed-point arithmetic in qemu_strtosz
> hint: Use 'git am --show-current-patch=diff' to see the failed patch
> When you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
>
> But skipping this patch, the rest can be applied properly by git-am.

I can imagine git am might get confused, out of interest what about git
merge (as this is a PR and previously git is pretty smart about this)?

-- 
Alex Bennée

Re: [PULL v2 00/15] misc fixes (strtoz, plugins, guest-loader)

[Peter Maydell]

On Wed, 17 Mar 2021 at 07:22, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Apologies for the delay, it took several swings at the CI to get a
> clean pass. I've dropped the SYS_HEAPINFO and final kernel-doc
> integration patch for now. I've also nabbed rth's strtoz fix although
> that is also in another 1 change PR he submitted earlier.
>
> The following changes since commit 5b7f5586d182b0cafb1f8d558992a14763e2953e:
>
>   Merge remote-tracking branch 'remotes/kraxel/tags/usb-20210315-pull-request' into staging (2021-03-16 13:17:54 +0000)
>
> are available in the Git repository at:
>
>   https://github.com/stsquad/qemu.git tags/pull-misc-6.0-updates-170321-2
>
> for you to fetch changes up to 9e7118023fda7c29016038e2292d4d14129b63da:
>
>   hw/core: Only build guest-loader if libfdt is available (2021-03-17 07:17:46 +0000)
>
> ----------------------------------------------------------------
> Final fixes for 6.0
>
>   - plugins physical address changes
>   - syscall tracking plugin
>   - plugin kernel-doc comments (without integration)
>   - libfdt build fix for guest-loader
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/6.0
for any user-visible changes.

-- PMM