All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v4 0/3] tests: add RTAS protocol
@ 2016-09-06 13:17 Laurent Vivier
  2016-09-06 13:17 ` [Qemu-devel] [PATCH v4 1/3] qtest: replace strtoXX() by qemu_strtoXX() Laurent Vivier
                   ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: Laurent Vivier @ 2016-09-06 13:17 UTC (permalink / raw)
  To: david; +Cc: thuth, lvivier, qemu-ppc, qemu-devel, groug

This series allows to call RTAS commands from the qtest framework,
and defines a first test to call RTAS command "get-time-of-day"
to validate the protocol and test RTAS.

RTAS command parameters are passed to the guest via the
guest memory, so we also need to implement the guest memory
management functions for PPC64 target.

RTAS commands will be needed later to test PCI from the qtest framework
with ppc64 virtual machines: PCI configuration is read/written with
RTAS commands "ibm,read-pci-config", "ibm,write-pci-config".

v4:
- use qemu_strtoXXX() instead strtoXX(),
  add a patch in the series to change all strtoXX() in qtest.c

v3:
- use mktimegm() instead of timegm()

v2:
- remove useless parenthesis, inline
- add a missing space in qrtas_call() prototype

Laurent Vivier (3):
  qtest: replace strtoXX() by qemu_strtoXX()
  tests: make pc_alloc_init/init_flags/uninit generic
  tests: add RTAS command in the protocol

 hw/ppc/spapr_rtas.c         | 19 ++++++++++++
 include/hw/ppc/spapr_rtas.h | 10 +++++++
 qtest.c                     | 66 ++++++++++++++++++++++++++---------------
 tests/Makefile.include      |  6 +++-
 tests/libqos/libqos.h       |  2 +-
 tests/libqos/malloc-ppc64.c | 38 ++++++++++++++++++++++++
 tests/libqos/malloc-ppc64.h | 17 +++++++++++
 tests/libqos/malloc.c       | 42 +++++++++++++++++++++++++++
 tests/libqos/malloc.h       |  3 ++
 tests/libqos/rtas.c         | 71 +++++++++++++++++++++++++++++++++++++++++++++
 tests/libqos/rtas.h         | 11 +++++++
 tests/libqtest.c            | 10 +++++++
 tests/libqtest.h            | 15 ++++++++++
 tests/rtas-test.c           | 42 +++++++++++++++++++++++++++
 14 files changed, 327 insertions(+), 25 deletions(-)
 create mode 100644 include/hw/ppc/spapr_rtas.h
 create mode 100644 tests/libqos/malloc-ppc64.c
 create mode 100644 tests/libqos/malloc-ppc64.h
 create mode 100644 tests/libqos/rtas.c
 create mode 100644 tests/libqos/rtas.h
 create mode 100644 tests/rtas-test.c

-- 
2.5.5

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH v4 1/3] qtest: replace strtoXX() by qemu_strtoXX()
  2016-09-06 13:17 [Qemu-devel] [PATCH v4 0/3] tests: add RTAS protocol Laurent Vivier
@ 2016-09-06 13:17 ` Laurent Vivier
  2016-09-06 15:55   ` Greg Kurz
                     ` (2 more replies)
  2016-09-06 13:17 ` [Qemu-devel] [PATCH v4 2/3] tests: make pc_alloc_init/init_flags/uninit generic Laurent Vivier
  2016-09-06 13:17 ` [Qemu-devel] [PATCH v4 3/3] tests: add RTAS command in the protocol Laurent Vivier
  2 siblings, 3 replies; 25+ messages in thread
From: Laurent Vivier @ 2016-09-06 13:17 UTC (permalink / raw)
  To: david; +Cc: thuth, lvivier, qemu-ppc, qemu-devel, groug

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
v4:
- add this patch in the series to change all strtoXX() in qtest.c

 qtest.c | 49 ++++++++++++++++++++++++++-----------------------
 1 file changed, 26 insertions(+), 23 deletions(-)

diff --git a/qtest.c b/qtest.c
index da4826c..4c94708 100644
--- a/qtest.c
+++ b/qtest.c
@@ -27,6 +27,7 @@
 #include "qemu/config-file.h"
 #include "qemu/option.h"
 #include "qemu/error-report.h"
+#include "qemu/cutils.h"
 
 #define MAX_IRQ 256
 
@@ -324,12 +325,13 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
     } else if (strcmp(words[0], "outb") == 0 ||
                strcmp(words[0], "outw") == 0 ||
                strcmp(words[0], "outl") == 0) {
-        uint16_t addr;
-        uint32_t value;
+        unsigned long addr;
+        unsigned long value;
 
         g_assert(words[1] && words[2]);
-        addr = strtoul(words[1], NULL, 0);
-        value = strtoul(words[2], NULL, 0);
+        g_assert(qemu_strtoul(words[1], NULL, 0, &addr) == 0);
+        g_assert(qemu_strtoul(words[2], NULL, 0, &value) == 0);
+        g_assert(addr <= 0xffff);
 
         if (words[0][3] == 'b') {
             cpu_outb(addr, value);
@@ -343,11 +345,12 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
     } else if (strcmp(words[0], "inb") == 0 ||
         strcmp(words[0], "inw") == 0 ||
         strcmp(words[0], "inl") == 0) {
-        uint16_t addr;
+        unsigned long addr;
         uint32_t value = -1U;
 
         g_assert(words[1]);
-        addr = strtoul(words[1], NULL, 0);
+        g_assert(qemu_strtoul(words[1], NULL, 0, &addr) == 0);
+        g_assert(addr <= 0xffff);
 
         if (words[0][2] == 'b') {
             value = cpu_inb(addr);
@@ -366,8 +369,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
         uint64_t value;
 
         g_assert(words[1] && words[2]);
-        addr = strtoull(words[1], NULL, 0);
-        value = strtoull(words[2], NULL, 0);
+        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
+        g_assert(qemu_strtoull(words[2], NULL, 0, &value) == 0);
 
         if (words[0][5] == 'b') {
             uint8_t data = value;
@@ -395,7 +398,7 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
         uint64_t value = UINT64_C(-1);
 
         g_assert(words[1]);
-        addr = strtoull(words[1], NULL, 0);
+        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
 
         if (words[0][4] == 'b') {
             uint8_t data;
@@ -421,8 +424,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
         char *enc;
 
         g_assert(words[1] && words[2]);
-        addr = strtoull(words[1], NULL, 0);
-        len = strtoull(words[2], NULL, 0);
+        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
+        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
 
         data = g_malloc(len);
         cpu_physical_memory_read(addr, data, len);
@@ -443,8 +446,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
         gchar *b64_data;
 
         g_assert(words[1] && words[2]);
-        addr = strtoull(words[1], NULL, 0);
-        len = strtoull(words[2], NULL, 0);
+        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
+        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
 
         data = g_malloc(len);
         cpu_physical_memory_read(addr, data, len);
@@ -460,8 +463,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
         size_t data_len;
 
         g_assert(words[1] && words[2] && words[3]);
-        addr = strtoull(words[1], NULL, 0);
-        len = strtoull(words[2], NULL, 0);
+        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
+        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
 
         data_len = strlen(words[3]);
         if (data_len < 3) {
@@ -486,12 +489,12 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
     } else if (strcmp(words[0], "memset") == 0) {
         uint64_t addr, len;
         uint8_t *data;
-        uint8_t pattern;
+        unsigned long pattern;
 
         g_assert(words[1] && words[2] && words[3]);
-        addr = strtoull(words[1], NULL, 0);
-        len = strtoull(words[2], NULL, 0);
-        pattern = strtoull(words[3], NULL, 0);
+        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
+        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
+        g_assert(qemu_strtoul(words[3], NULL, 0, &pattern) == 0);
 
         data = g_malloc(len);
         memset(data, pattern, len);
@@ -507,8 +510,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
         gsize out_len;
 
         g_assert(words[1] && words[2] && words[3]);
-        addr = strtoull(words[1], NULL, 0);
-        len = strtoull(words[2], NULL, 0);
+        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
+        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
 
         data_len = strlen(words[3]);
         if (data_len < 3) {
@@ -532,7 +535,7 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
         int64_t ns;
 
         if (words[1]) {
-            ns = strtoll(words[1], NULL, 0);
+            g_assert(qemu_strtoll(words[1], NULL, 0, &ns) == 0);
         } else {
             ns = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL);
         }
@@ -544,7 +547,7 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
         int64_t ns;
 
         g_assert(words[1]);
-        ns = strtoll(words[1], NULL, 0);
+        g_assert(qemu_strtoll(words[1], NULL, 0, &ns) == 0);
         qtest_clock_warp(ns);
         qtest_send_prefix(chr);
         qtest_sendf(chr, "OK %"PRIi64"\n",
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH v4 2/3] tests: make pc_alloc_init/init_flags/uninit generic
  2016-09-06 13:17 [Qemu-devel] [PATCH v4 0/3] tests: add RTAS protocol Laurent Vivier
  2016-09-06 13:17 ` [Qemu-devel] [PATCH v4 1/3] qtest: replace strtoXX() by qemu_strtoXX() Laurent Vivier
@ 2016-09-06 13:17 ` Laurent Vivier
  2016-09-06 19:46   ` Thomas Huth
                     ` (2 more replies)
  2016-09-06 13:17 ` [Qemu-devel] [PATCH v4 3/3] tests: add RTAS command in the protocol Laurent Vivier
  2 siblings, 3 replies; 25+ messages in thread
From: Laurent Vivier @ 2016-09-06 13:17 UTC (permalink / raw)
  To: david; +Cc: thuth, lvivier, qemu-ppc, qemu-devel, groug

And add support for ppc64.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
v2:
- remove useless parenthesis, inline

 tests/Makefile.include      |  3 ++-
 tests/libqos/libqos.h       |  2 +-
 tests/libqos/malloc-ppc64.c | 38 ++++++++++++++++++++++++++++++++++++++
 tests/libqos/malloc-ppc64.h | 17 +++++++++++++++++
 tests/libqos/malloc.c       | 42 ++++++++++++++++++++++++++++++++++++++++++
 tests/libqos/malloc.h       |  3 +++
 6 files changed, 103 insertions(+), 2 deletions(-)
 create mode 100644 tests/libqos/malloc-ppc64.c
 create mode 100644 tests/libqos/malloc-ppc64.h

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 14be491..a286848 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -557,8 +557,9 @@ tests/test-crypto-block$(EXESUF): tests/test-crypto-block.o $(test-crypto-obj-y)
 
 libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o tests/libqos/malloc.o
 libqos-obj-y += tests/libqos/i2c.o tests/libqos/libqos.o
+libqos-obj-y += tests/libqos/malloc-ppc64.o tests/libqos/malloc-pc.o
 libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
-libqos-pc-obj-y += tests/libqos/malloc-pc.o tests/libqos/libqos-pc.o
+libqos-pc-obj-y += tests/libqos/libqos-pc.o
 libqos-pc-obj-y += tests/libqos/ahci.o
 libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o
 libqos-imx-obj-y = $(libqos-obj-y) tests/libqos/i2c-imx.o
diff --git a/tests/libqos/libqos.h b/tests/libqos/libqos.h
index 604980d..7b71607 100644
--- a/tests/libqos/libqos.h
+++ b/tests/libqos/libqos.h
@@ -3,7 +3,7 @@
 
 #include "libqtest.h"
 #include "libqos/pci.h"
-#include "libqos/malloc-pc.h"
+#include "libqos/malloc.h"
 
 typedef struct QOSOps {
     QGuestAllocator *(*init_allocator)(QAllocOpts);
diff --git a/tests/libqos/malloc-ppc64.c b/tests/libqos/malloc-ppc64.c
new file mode 100644
index 0000000..1b31e33
--- /dev/null
+++ b/tests/libqos/malloc-ppc64.c
@@ -0,0 +1,38 @@
+/*
+ * libqos malloc support for PPC64
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqos/malloc-ppc64.h"
+
+#include "qemu-common.h"
+
+#define PAGE_SIZE 4096
+
+/* Memory must be a multiple of 256 MB,
+ * so we have at least 256MB
+ */
+#define PPC64_MIN_SIZE 0x10000000
+
+void ppc64_alloc_uninit(QGuestAllocator *allocator)
+{
+    alloc_uninit(allocator);
+}
+
+QGuestAllocator *ppc64_alloc_init_flags(QAllocOpts flags)
+{
+    QGuestAllocator *s;
+
+    s = alloc_init_flags(flags, 1 << 20, PPC64_MIN_SIZE);
+    alloc_set_page_size(s, PAGE_SIZE);
+
+    return s;
+}
+
+QGuestAllocator *ppc64_alloc_init(void)
+{
+    return ppc64_alloc_init_flags(ALLOC_NO_FLAGS);
+}
diff --git a/tests/libqos/malloc-ppc64.h b/tests/libqos/malloc-ppc64.h
new file mode 100644
index 0000000..c2b2dff
--- /dev/null
+++ b/tests/libqos/malloc-ppc64.h
@@ -0,0 +1,17 @@
+/*
+ * libqos malloc support for PPC64
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef LIBQOS_MALLOC_PPC64_H
+#define LIBQOS_MALLOC_PPC64_H
+
+#include "libqos/malloc.h"
+
+QGuestAllocator *ppc64_alloc_init(void);
+QGuestAllocator *ppc64_alloc_init_flags(QAllocOpts flags);
+void ppc64_alloc_uninit(QGuestAllocator *allocator);
+
+#endif
diff --git a/tests/libqos/malloc.c b/tests/libqos/malloc.c
index b8eff5f..6a02345 100644
--- a/tests/libqos/malloc.c
+++ b/tests/libqos/malloc.c
@@ -12,6 +12,9 @@
 
 #include "qemu/osdep.h"
 #include "libqos/malloc.h"
+#include "libqos/malloc-pc.h"
+#include "libqos/malloc-ppc64.h"
+#include "libqtest.h"
 #include "qemu-common.h"
 #include "qemu/host-utils.h"
 
@@ -375,3 +378,42 @@ void migrate_allocator(QGuestAllocator *src,
     QTAILQ_INSERT_HEAD(src->free, node, MLIST_ENTNAME);
     return;
 }
+
+QGuestAllocator *machine_alloc_init(void)
+{
+    const char *arch = qtest_get_arch();
+
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        return pc_alloc_init();
+    }
+    if (strcmp(arch, "ppc64") == 0) {
+        return ppc64_alloc_init();
+    }
+    return NULL;
+}
+
+QGuestAllocator *machine_alloc_init_flags(QAllocOpts flags)
+{
+    const char *arch = qtest_get_arch();
+
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        return pc_alloc_init_flags(flags);
+    }
+    if (strcmp(arch, "ppc64") == 0) {
+        return ppc64_alloc_init_flags(flags);
+    }
+    return NULL;
+}
+
+void machine_alloc_uninit(QGuestAllocator *allocator)
+{
+    const char *arch = qtest_get_arch();
+
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        pc_alloc_uninit(allocator);
+        return;
+    }
+    if (strcmp(arch, "ppc64") == 0) {
+        ppc64_alloc_uninit(allocator);
+    }
+}
diff --git a/tests/libqos/malloc.h b/tests/libqos/malloc.h
index ae9dac8..a5f4c63 100644
--- a/tests/libqos/malloc.h
+++ b/tests/libqos/malloc.h
@@ -37,4 +37,7 @@ QGuestAllocator *alloc_init_flags(QAllocOpts flags,
 void alloc_set_page_size(QGuestAllocator *allocator, size_t page_size);
 void alloc_set_flags(QGuestAllocator *allocator, QAllocOpts opts);
 
+QGuestAllocator *machine_alloc_init(void);
+QGuestAllocator *machine_alloc_init_flags(QAllocOpts flags);
+void machine_alloc_uninit(QGuestAllocator *allocator);
 #endif
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Qemu-devel] [PATCH v4 3/3] tests: add RTAS command in the protocol
  2016-09-06 13:17 [Qemu-devel] [PATCH v4 0/3] tests: add RTAS protocol Laurent Vivier
  2016-09-06 13:17 ` [Qemu-devel] [PATCH v4 1/3] qtest: replace strtoXX() by qemu_strtoXX() Laurent Vivier
  2016-09-06 13:17 ` [Qemu-devel] [PATCH v4 2/3] tests: make pc_alloc_init/init_flags/uninit generic Laurent Vivier
@ 2016-09-06 13:17 ` Laurent Vivier
  2016-09-06 22:07   ` Greg Kurz
  2 siblings, 1 reply; 25+ messages in thread
From: Laurent Vivier @ 2016-09-06 13:17 UTC (permalink / raw)
  To: david; +Cc: thuth, lvivier, qemu-ppc, qemu-devel, groug

Add a first test to validate the protocol:

- rtas/get-time-of-day compares the time
  from the guest with the time from the host.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
v4:
- use qemu_strtoXXX() instead strtoXX()

v3:
- use mktimegm() instead of timegm()
 
v2:
- add a missing space in qrtas_call() prototype
 
 hw/ppc/spapr_rtas.c         | 19 ++++++++++++
 include/hw/ppc/spapr_rtas.h | 10 +++++++
 qtest.c                     | 17 +++++++++++
 tests/Makefile.include      |  3 ++
 tests/libqos/rtas.c         | 71 +++++++++++++++++++++++++++++++++++++++++++++
 tests/libqos/rtas.h         | 11 +++++++
 tests/libqtest.c            | 10 +++++++
 tests/libqtest.h            | 15 ++++++++++
 tests/rtas-test.c           | 42 +++++++++++++++++++++++++++
 9 files changed, 198 insertions(+)
 create mode 100644 include/hw/ppc/spapr_rtas.h
 create mode 100644 tests/libqos/rtas.c
 create mode 100644 tests/libqos/rtas.h
 create mode 100644 tests/rtas-test.c

diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
index dc058e5..8aed56f 100644
--- a/hw/ppc/spapr_rtas.c
+++ b/hw/ppc/spapr_rtas.c
@@ -36,6 +36,7 @@
 
 #include "hw/ppc/spapr.h"
 #include "hw/ppc/spapr_vio.h"
+#include "hw/ppc/spapr_rtas.h"
 #include "hw/ppc/ppc.h"
 #include "qapi-event.h"
 #include "hw/boards.h"
@@ -691,6 +692,24 @@ target_ulong spapr_rtas_call(PowerPCCPU *cpu, sPAPRMachineState *spapr,
     return H_PARAMETER;
 }
 
+uint64_t qtest_rtas_call(char *cmd, uint32_t nargs, uint64_t args,
+                         uint32_t nret, uint64_t rets)
+{
+    int token;
+
+    for (token = 0; token < RTAS_TOKEN_MAX - RTAS_TOKEN_BASE; token++) {
+        if (strcmp(cmd, rtas_table[token].name) == 0) {
+            sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
+            PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
+
+            rtas_table[token].fn(cpu, spapr, token + RTAS_TOKEN_BASE,
+                                 nargs, args, nret, rets);
+            return H_SUCCESS;
+        }
+    }
+    return H_PARAMETER;
+}
+
 void spapr_rtas_register(int token, const char *name, spapr_rtas_fn fn)
 {
     assert((token >= RTAS_TOKEN_BASE) && (token < RTAS_TOKEN_MAX));
diff --git a/include/hw/ppc/spapr_rtas.h b/include/hw/ppc/spapr_rtas.h
new file mode 100644
index 0000000..383611f
--- /dev/null
+++ b/include/hw/ppc/spapr_rtas.h
@@ -0,0 +1,10 @@
+#ifndef HW_SPAPR_RTAS_H
+#define HW_SPAPR_RTAS_H
+/*
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+uint64_t qtest_rtas_call(char *cmd, uint32_t nargs, uint64_t args,
+                         uint32_t nret, uint64_t rets);
+#endif /* HW_SPAPR_RTAS_H */
diff --git a/qtest.c b/qtest.c
index 4c94708..beb62b4 100644
--- a/qtest.c
+++ b/qtest.c
@@ -28,6 +28,9 @@
 #include "qemu/option.h"
 #include "qemu/error-report.h"
 #include "qemu/cutils.h"
+#ifdef TARGET_PPC64
+#include "hw/ppc/spapr_rtas.h"
+#endif
 
 #define MAX_IRQ 256
 
@@ -531,6 +534,20 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
 
         qtest_send_prefix(chr);
         qtest_send(chr, "OK\n");
+#ifdef TARGET_PPC64
+    } else if (strcmp(words[0], "rtas") == 0) {
+        uint64_t res, args, ret;
+        unsigned long nargs, nret;
+
+        g_assert(qemu_strtoul(words[2], NULL, 0, &nargs) == 0);
+        g_assert(qemu_strtoull(words[3], NULL, 0, &args) == 0);
+        g_assert(qemu_strtoul(words[4], NULL, 0, &nret) == 0);
+        g_assert(qemu_strtoull(words[5], NULL, 0, &ret) == 0);
+        res = qtest_rtas_call(words[1], nargs, args, nret, ret);
+
+        qtest_send_prefix(chr);
+        qtest_sendf(chr, "OK %"PRIu64"\n", res);
+#endif
     } else if (qtest_enabled() && strcmp(words[0], "clock_step") == 0) {
         int64_t ns;
 
diff --git a/tests/Makefile.include b/tests/Makefile.include
index a286848..c456b8b 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -272,6 +272,7 @@ check-qtest-sparc-y += tests/prom-env-test$(EXESUF)
 check-qtest-microblazeel-y = $(check-qtest-microblaze-y)
 check-qtest-xtensaeb-y = $(check-qtest-xtensa-y)
 check-qtest-ppc64-y += tests/postcopy-test$(EXESUF)
+check-qtest-ppc64-y += tests/rtas-test$(EXESUF)
 
 check-qtest-generic-y += tests/qom-test$(EXESUF)
 
@@ -558,6 +559,7 @@ tests/test-crypto-block$(EXESUF): tests/test-crypto-block.o $(test-crypto-obj-y)
 libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o tests/libqos/malloc.o
 libqos-obj-y += tests/libqos/i2c.o tests/libqos/libqos.o
 libqos-obj-y += tests/libqos/malloc-ppc64.o tests/libqos/malloc-pc.o
+libqos-obj-y += tests/libqos/rtas.o
 libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
 libqos-pc-obj-y += tests/libqos/libqos-pc.o
 libqos-pc-obj-y += tests/libqos/ahci.o
@@ -572,6 +574,7 @@ tests/m48t59-test$(EXESUF): tests/m48t59-test.o
 tests/endianness-test$(EXESUF): tests/endianness-test.o
 tests/spapr-phb-test$(EXESUF): tests/spapr-phb-test.o $(libqos-obj-y)
 tests/prom-env-test$(EXESUF): tests/prom-env-test.o $(libqos-obj-y)
+tests/rtas-test$(EXESUF): tests/rtas-test.o $(libqos-obj-y)
 tests/fdc-test$(EXESUF): tests/fdc-test.o
 tests/ide-test$(EXESUF): tests/ide-test.o $(libqos-pc-obj-y)
 tests/ahci-test$(EXESUF): tests/ahci-test.o $(libqos-pc-obj-y)
diff --git a/tests/libqos/rtas.c b/tests/libqos/rtas.c
new file mode 100644
index 0000000..d5f4ced
--- /dev/null
+++ b/tests/libqos/rtas.c
@@ -0,0 +1,71 @@
+/*
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "libqos/rtas.h"
+
+static void qrtas_copy_args(uint64_t target_args, uint32_t nargs,
+                            uint32_t *args)
+{
+    int i;
+
+    for (i = 0; i < nargs; i++) {
+        writel(target_args + i * sizeof(uint32_t), args[i]);
+    }
+}
+
+static void qrtas_copy_ret(uint64_t target_ret, uint32_t nret, uint32_t *ret)
+{
+    int i;
+
+    for (i = 0; i < nret; i++) {
+        ret[i] = readl(target_ret + i * sizeof(uint32_t));
+    }
+}
+
+static uint64_t qrtas_call(QGuestAllocator *alloc, const char *name,
+                           uint32_t nargs, uint32_t *args,
+                           uint32_t nret, uint32_t *ret)
+{
+    uint64_t res;
+    uint64_t target_args, target_ret;
+
+    target_args = guest_alloc(alloc, (nargs + nret) * sizeof(uint32_t));
+    target_ret = guest_alloc(alloc, nret * sizeof(uint32_t));
+
+    qrtas_copy_args(target_args, nargs, args);
+    res = qtest_rtas_call(global_qtest, name,
+                          nargs, target_args, nret, target_ret);
+    qrtas_copy_ret(target_ret, nret, ret);
+
+    guest_free(alloc, target_ret);
+    guest_free(alloc, target_args);
+
+    return res;
+}
+
+int qrtas_get_time_of_day(QGuestAllocator *alloc, struct tm *tm, uint32_t *ns)
+{
+    int res;
+    uint32_t ret[8];
+
+    res = qrtas_call(alloc, "get-time-of-day", 0, NULL, 8, ret);
+    if (res != 0) {
+        return res;
+    }
+
+    res = ret[0];
+    memset(tm, 0, sizeof(*tm));
+    tm->tm_year = ret[1] - 1900;
+    tm->tm_mon = ret[2] - 1;
+    tm->tm_mday = ret[3];
+    tm->tm_hour = ret[4];
+    tm->tm_min = ret[5];
+    tm->tm_sec = ret[6];
+    *ns = ret[7];
+
+    return res;
+}
diff --git a/tests/libqos/rtas.h b/tests/libqos/rtas.h
new file mode 100644
index 0000000..a1b60a8
--- /dev/null
+++ b/tests/libqos/rtas.h
@@ -0,0 +1,11 @@
+/*
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef LIBQOS_RTAS_H
+#define LIBQOS_RTAS_H
+#include "libqos/malloc.h"
+
+int qrtas_get_time_of_day(QGuestAllocator *alloc, struct tm *tm, uint32_t *ns);
+#endif /* LIBQOS_RTAS_H */
diff --git a/tests/libqtest.c b/tests/libqtest.c
index eb00f13..c9dd57b 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -751,6 +751,16 @@ void qtest_memread(QTestState *s, uint64_t addr, void *data, size_t size)
     g_strfreev(args);
 }
 
+uint64_t qtest_rtas_call(QTestState *s, const char *name,
+                         uint32_t nargs, uint64_t args,
+                         uint32_t nret, uint64_t ret)
+{
+    qtest_sendf(s, "rtas %s %u 0x%"PRIx64" %u 0x%"PRIx64"\n",
+                name, nargs, args, nret, ret);
+    qtest_rsp(s, 0);
+    return 0;
+}
+
 void qtest_add_func(const char *str, void (*fn)(void))
 {
     gchar *path = g_strdup_printf("/%s/%s", qtest_get_arch(), str);
diff --git a/tests/libqtest.h b/tests/libqtest.h
index 37f37ad..1badb76 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -318,6 +318,21 @@ uint64_t qtest_readq(QTestState *s, uint64_t addr);
 void qtest_memread(QTestState *s, uint64_t addr, void *data, size_t size);
 
 /**
+ * qtest_rtas_call:
+ * @s: #QTestState instance to operate on.
+ * @name: name of the command to call.
+ * @nargs: Number of args.
+ * @args: Guest address to read args from.
+ * @nret: Number of return value.
+ * @ret: Guest address to write return values to.
+ *
+ * Call an RTAS function
+ */
+uint64_t qtest_rtas_call(QTestState *s, const char *name,
+                         uint32_t nargs, uint64_t args,
+                         uint32_t nret, uint64_t ret);
+
+/**
  * qtest_bufread:
  * @s: #QTestState instance to operate on.
  * @addr: Guest address to read from.
diff --git a/tests/rtas-test.c b/tests/rtas-test.c
new file mode 100644
index 0000000..1be625c
--- /dev/null
+++ b/tests/rtas-test.c
@@ -0,0 +1,42 @@
+#include "qemu/osdep.h"
+#include "qemu/cutils.h"
+#include "libqtest.h"
+
+#include "libqos/rtas.h"
+
+static void test_rtas_get_time_of_day(void)
+{
+    QGuestAllocator *alloc;
+    struct tm tm;
+    uint32_t ns;
+    uint64_t ret;
+    time_t t1, t2;
+
+    qtest_start("");
+
+    alloc = machine_alloc_init();
+
+    t1 = time(NULL);
+    ret = qrtas_get_time_of_day(alloc, &tm, &ns);
+    g_assert_cmpint(ret, ==, 0);
+    t2 = mktimegm(&tm);
+    g_assert(t2 - t1 < 5); /* 5 sec max to run the test */
+
+    machine_alloc_uninit(alloc);
+    qtest_quit(global_qtest);
+}
+
+int main(int argc, char *argv[])
+{
+    const char *arch = qtest_get_arch();
+
+    g_test_init(&argc, &argv, NULL);
+
+    if (strcmp(arch, "ppc64") == 0) {
+        qtest_add_func("rtas/get-time-of-day", test_rtas_get_time_of_day);
+    } else {
+        g_assert_not_reached();
+    }
+
+    return g_test_run();
+}
-- 
2.5.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH v4 1/3] qtest: replace strtoXX() by qemu_strtoXX()
  2016-09-06 13:17 ` [Qemu-devel] [PATCH v4 1/3] qtest: replace strtoXX() by qemu_strtoXX() Laurent Vivier
@ 2016-09-06 15:55   ` Greg Kurz
  2016-09-06 18:17     ` Laurent Vivier
  2016-09-08  1:56   ` David Gibson
  2016-09-08  7:55   ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
  2 siblings, 1 reply; 25+ messages in thread
From: Greg Kurz @ 2016-09-06 15:55 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: david, thuth, qemu-ppc, qemu-devel

On Tue,  6 Sep 2016 15:17:55 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---

The patch also adds error checking and assertions. Maybe worth to be mentioned
in the changelog...

> v4:
> - add this patch in the series to change all strtoXX() in qtest.c
> 
>  qtest.c | 49 ++++++++++++++++++++++++++-----------------------
>  1 file changed, 26 insertions(+), 23 deletions(-)
> 
> diff --git a/qtest.c b/qtest.c
> index da4826c..4c94708 100644
> --- a/qtest.c
> +++ b/qtest.c
> @@ -27,6 +27,7 @@
>  #include "qemu/config-file.h"
>  #include "qemu/option.h"
>  #include "qemu/error-report.h"
> +#include "qemu/cutils.h"
>  
>  #define MAX_IRQ 256
>  
> @@ -324,12 +325,13 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>      } else if (strcmp(words[0], "outb") == 0 ||
>                 strcmp(words[0], "outw") == 0 ||
>                 strcmp(words[0], "outl") == 0) {
> -        uint16_t addr;
> -        uint32_t value;
> +        unsigned long addr;
> +        unsigned long value;
>  
>          g_assert(words[1] && words[2]);
> -        addr = strtoul(words[1], NULL, 0);
> -        value = strtoul(words[2], NULL, 0);
> +        g_assert(qemu_strtoul(words[1], NULL, 0, &addr) == 0);
> +        g_assert(qemu_strtoul(words[2], NULL, 0, &value) == 0);
> +        g_assert(addr <= 0xffff);
>  
>          if (words[0][3] == 'b') {
>              cpu_outb(addr, value);
> @@ -343,11 +345,12 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>      } else if (strcmp(words[0], "inb") == 0 ||
>          strcmp(words[0], "inw") == 0 ||
>          strcmp(words[0], "inl") == 0) {
> -        uint16_t addr;
> +        unsigned long addr;
>          uint32_t value = -1U;
>  
>          g_assert(words[1]);
> -        addr = strtoul(words[1], NULL, 0);
> +        g_assert(qemu_strtoul(words[1], NULL, 0, &addr) == 0);
> +        g_assert(addr <= 0xffff);
>  
>          if (words[0][2] == 'b') {
>              value = cpu_inb(addr);
> @@ -366,8 +369,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>          uint64_t value;
>  
>          g_assert(words[1] && words[2]);
> -        addr = strtoull(words[1], NULL, 0);
> -        value = strtoull(words[2], NULL, 0);
> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
> +        g_assert(qemu_strtoull(words[2], NULL, 0, &value) == 0);
>  
>          if (words[0][5] == 'b') {
>              uint8_t data = value;
> @@ -395,7 +398,7 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>          uint64_t value = UINT64_C(-1);
>  
>          g_assert(words[1]);
> -        addr = strtoull(words[1], NULL, 0);
> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
>  
>          if (words[0][4] == 'b') {
>              uint8_t data;
> @@ -421,8 +424,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>          char *enc;
>  
>          g_assert(words[1] && words[2]);
> -        addr = strtoull(words[1], NULL, 0);
> -        len = strtoull(words[2], NULL, 0);
> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
> +        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
>  
>          data = g_malloc(len);
>          cpu_physical_memory_read(addr, data, len);
> @@ -443,8 +446,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>          gchar *b64_data;
>  
>          g_assert(words[1] && words[2]);
> -        addr = strtoull(words[1], NULL, 0);
> -        len = strtoull(words[2], NULL, 0);
> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
> +        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
>  
>          data = g_malloc(len);
>          cpu_physical_memory_read(addr, data, len);
> @@ -460,8 +463,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>          size_t data_len;
>  
>          g_assert(words[1] && words[2] && words[3]);
> -        addr = strtoull(words[1], NULL, 0);
> -        len = strtoull(words[2], NULL, 0);
> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
> +        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
>  
>          data_len = strlen(words[3]);
>          if (data_len < 3) {
> @@ -486,12 +489,12 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>      } else if (strcmp(words[0], "memset") == 0) {
>          uint64_t addr, len;
>          uint8_t *data;
> -        uint8_t pattern;
> +        unsigned long pattern;
>  
>          g_assert(words[1] && words[2] && words[3]);
> -        addr = strtoull(words[1], NULL, 0);
> -        len = strtoull(words[2], NULL, 0);
> -        pattern = strtoull(words[3], NULL, 0);
> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
> +        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
> +        g_assert(qemu_strtoul(words[3], NULL, 0, &pattern) == 0);

And:

g_assert(pattern <= 0xff)

>  
>          data = g_malloc(len);
>          memset(data, pattern, len);
> @@ -507,8 +510,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>          gsize out_len;
>  
>          g_assert(words[1] && words[2] && words[3]);
> -        addr = strtoull(words[1], NULL, 0);
> -        len = strtoull(words[2], NULL, 0);
> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
> +        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
>  
>          data_len = strlen(words[3]);
>          if (data_len < 3) {
> @@ -532,7 +535,7 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>          int64_t ns;
>  
>          if (words[1]) {
> -            ns = strtoll(words[1], NULL, 0);
> +            g_assert(qemu_strtoll(words[1], NULL, 0, &ns) == 0);
>          } else {
>              ns = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL);
>          }
> @@ -544,7 +547,7 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>          int64_t ns;
>  
>          g_assert(words[1]);
> -        ns = strtoll(words[1], NULL, 0);
> +        g_assert(qemu_strtoll(words[1], NULL, 0, &ns) == 0);
>          qtest_clock_warp(ns);
>          qtest_send_prefix(chr);
>          qtest_sendf(chr, "OK %"PRIi64"\n",

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH v4 1/3] qtest: replace strtoXX() by qemu_strtoXX()
  2016-09-06 15:55   ` Greg Kurz
@ 2016-09-06 18:17     ` Laurent Vivier
  2016-09-06 21:13       ` Greg Kurz
  0 siblings, 1 reply; 25+ messages in thread
From: Laurent Vivier @ 2016-09-06 18:17 UTC (permalink / raw)
  To: Greg Kurz; +Cc: david, thuth, qemu-ppc, qemu-devel



On 06/09/2016 17:55, Greg Kurz wrote:
> On Tue,  6 Sep 2016 15:17:55 +0200
> Laurent Vivier <lvivier@redhat.com> wrote:
> 
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
> 
> The patch also adds error checking and assertions. Maybe worth to be mentioned
> in the changelog...

In case of a new version of the patch, I will...

> 
>> v4:
>> - add this patch in the series to change all strtoXX() in qtest.c
>>
>>  qtest.c | 49 ++++++++++++++++++++++++++-----------------------
>>  1 file changed, 26 insertions(+), 23 deletions(-)
>>
>> diff --git a/qtest.c b/qtest.c
>> index da4826c..4c94708 100644
>> --- a/qtest.c
>> +++ b/qtest.c
>> @@ -27,6 +27,7 @@
>>  #include "qemu/config-file.h"
>>  #include "qemu/option.h"
>>  #include "qemu/error-report.h"
>> +#include "qemu/cutils.h"
>>  
>>  #define MAX_IRQ 256
>>  
>> @@ -324,12 +325,13 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>>      } else if (strcmp(words[0], "outb") == 0 ||
>>                 strcmp(words[0], "outw") == 0 ||
>>                 strcmp(words[0], "outl") == 0) {
>> -        uint16_t addr;
>> -        uint32_t value;
>> +        unsigned long addr;
>> +        unsigned long value;
>>  
>>          g_assert(words[1] && words[2]);
>> -        addr = strtoul(words[1], NULL, 0);
>> -        value = strtoul(words[2], NULL, 0);
>> +        g_assert(qemu_strtoul(words[1], NULL, 0, &addr) == 0);
>> +        g_assert(qemu_strtoul(words[2], NULL, 0, &value) == 0);
>> +        g_assert(addr <= 0xffff);
>>  
>>          if (words[0][3] == 'b') {
>>              cpu_outb(addr, value);
>> @@ -343,11 +345,12 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>>      } else if (strcmp(words[0], "inb") == 0 ||
>>          strcmp(words[0], "inw") == 0 ||
>>          strcmp(words[0], "inl") == 0) {
>> -        uint16_t addr;
>> +        unsigned long addr;
>>          uint32_t value = -1U;
>>  
>>          g_assert(words[1]);
>> -        addr = strtoul(words[1], NULL, 0);
>> +        g_assert(qemu_strtoul(words[1], NULL, 0, &addr) == 0);
>> +        g_assert(addr <= 0xffff);
>>  
>>          if (words[0][2] == 'b') {
>>              value = cpu_inb(addr);
>> @@ -366,8 +369,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>>          uint64_t value;
>>  
>>          g_assert(words[1] && words[2]);
>> -        addr = strtoull(words[1], NULL, 0);
>> -        value = strtoull(words[2], NULL, 0);
>> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
>> +        g_assert(qemu_strtoull(words[2], NULL, 0, &value) == 0);
>>  
>>          if (words[0][5] == 'b') {
>>              uint8_t data = value;
>> @@ -395,7 +398,7 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>>          uint64_t value = UINT64_C(-1);
>>  
>>          g_assert(words[1]);
>> -        addr = strtoull(words[1], NULL, 0);
>> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
>>  
>>          if (words[0][4] == 'b') {
>>              uint8_t data;
>> @@ -421,8 +424,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>>          char *enc;
>>  
>>          g_assert(words[1] && words[2]);
>> -        addr = strtoull(words[1], NULL, 0);
>> -        len = strtoull(words[2], NULL, 0);
>> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
>> +        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
>>  
>>          data = g_malloc(len);
>>          cpu_physical_memory_read(addr, data, len);
>> @@ -443,8 +446,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>>          gchar *b64_data;
>>  
>>          g_assert(words[1] && words[2]);
>> -        addr = strtoull(words[1], NULL, 0);
>> -        len = strtoull(words[2], NULL, 0);
>> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
>> +        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
>>  
>>          data = g_malloc(len);
>>          cpu_physical_memory_read(addr, data, len);
>> @@ -460,8 +463,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>>          size_t data_len;
>>  
>>          g_assert(words[1] && words[2] && words[3]);
>> -        addr = strtoull(words[1], NULL, 0);
>> -        len = strtoull(words[2], NULL, 0);
>> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
>> +        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
>>  
>>          data_len = strlen(words[3]);
>>          if (data_len < 3) {
>> @@ -486,12 +489,12 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>>      } else if (strcmp(words[0], "memset") == 0) {
>>          uint64_t addr, len;
>>          uint8_t *data;
>> -        uint8_t pattern;
>> +        unsigned long pattern;
>>  
>>          g_assert(words[1] && words[2] && words[3]);
>> -        addr = strtoull(words[1], NULL, 0);
>> -        len = strtoull(words[2], NULL, 0);
>> -        pattern = strtoull(words[3], NULL, 0);
>> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
>> +        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
>> +        g_assert(qemu_strtoul(words[3], NULL, 0, &pattern) == 0);
> 
> And:
> 
> g_assert(pattern <= 0xff)

I think pattern > 0xff is valid as memset() takes an "int" and only uses
the byte value (for instance to use -1 to fill memory with 0xff). It
can't do bad things...

In the previous case ("g_assert(addr <= 0xffff)"), if addr > 0xffff,
cpu_out/in can write at a bad address. We could just ignore the upper
part of the word, but to debug test case I think it's good to have an
assert in this case.

Thanks,
Laurent

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/3] tests: make pc_alloc_init/init_flags/uninit generic
  2016-09-06 13:17 ` [Qemu-devel] [PATCH v4 2/3] tests: make pc_alloc_init/init_flags/uninit generic Laurent Vivier
@ 2016-09-06 19:46   ` Thomas Huth
  2016-09-06 21:41   ` Greg Kurz
  2016-09-08  2:04   ` David Gibson
  2 siblings, 0 replies; 25+ messages in thread
From: Thomas Huth @ 2016-09-06 19:46 UTC (permalink / raw)
  To: Laurent Vivier, david; +Cc: qemu-ppc, qemu-devel, groug

On 06.09.2016 15:17, Laurent Vivier wrote:
> And add support for ppc64.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> v2:
> - remove useless parenthesis, inline
> 
>  tests/Makefile.include      |  3 ++-
>  tests/libqos/libqos.h       |  2 +-
>  tests/libqos/malloc-ppc64.c | 38 ++++++++++++++++++++++++++++++++++++++
>  tests/libqos/malloc-ppc64.h | 17 +++++++++++++++++
>  tests/libqos/malloc.c       | 42 ++++++++++++++++++++++++++++++++++++++++++
>  tests/libqos/malloc.h       |  3 +++
>  6 files changed, 103 insertions(+), 2 deletions(-)
>  create mode 100644 tests/libqos/malloc-ppc64.c
>  create mode 100644 tests/libqos/malloc-ppc64.h
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 14be491..a286848 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -557,8 +557,9 @@ tests/test-crypto-block$(EXESUF): tests/test-crypto-block.o $(test-crypto-obj-y)
>  
>  libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o tests/libqos/malloc.o
>  libqos-obj-y += tests/libqos/i2c.o tests/libqos/libqos.o
> +libqos-obj-y += tests/libqos/malloc-ppc64.o tests/libqos/malloc-pc.o
>  libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
> -libqos-pc-obj-y += tests/libqos/malloc-pc.o tests/libqos/libqos-pc.o
> +libqos-pc-obj-y += tests/libqos/libqos-pc.o
>  libqos-pc-obj-y += tests/libqos/ahci.o
>  libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o
>  libqos-imx-obj-y = $(libqos-obj-y) tests/libqos/i2c-imx.o
> diff --git a/tests/libqos/libqos.h b/tests/libqos/libqos.h
> index 604980d..7b71607 100644
> --- a/tests/libqos/libqos.h
> +++ b/tests/libqos/libqos.h
> @@ -3,7 +3,7 @@
>  
>  #include "libqtest.h"
>  #include "libqos/pci.h"
> -#include "libqos/malloc-pc.h"
> +#include "libqos/malloc.h"
>  
>  typedef struct QOSOps {
>      QGuestAllocator *(*init_allocator)(QAllocOpts);
> diff --git a/tests/libqos/malloc-ppc64.c b/tests/libqos/malloc-ppc64.c
> new file mode 100644
> index 0000000..1b31e33
> --- /dev/null
> +++ b/tests/libqos/malloc-ppc64.c
> @@ -0,0 +1,38 @@
> +/*
> + * libqos malloc support for PPC64
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "libqos/malloc-ppc64.h"
> +
> +#include "qemu-common.h"
> +
> +#define PAGE_SIZE 4096
> +
> +/* Memory must be a multiple of 256 MB,
> + * so we have at least 256MB
> + */
> +#define PPC64_MIN_SIZE 0x10000000
> +
> +void ppc64_alloc_uninit(QGuestAllocator *allocator)
> +{
> +    alloc_uninit(allocator);
> +}
> +
> +QGuestAllocator *ppc64_alloc_init_flags(QAllocOpts flags)
> +{
> +    QGuestAllocator *s;
> +
> +    s = alloc_init_flags(flags, 1 << 20, PPC64_MIN_SIZE);
> +    alloc_set_page_size(s, PAGE_SIZE);
> +
> +    return s;
> +}
> +
> +QGuestAllocator *ppc64_alloc_init(void)
> +{
> +    return ppc64_alloc_init_flags(ALLOC_NO_FLAGS);
> +}
> diff --git a/tests/libqos/malloc-ppc64.h b/tests/libqos/malloc-ppc64.h
> new file mode 100644
> index 0000000..c2b2dff
> --- /dev/null
> +++ b/tests/libqos/malloc-ppc64.h
> @@ -0,0 +1,17 @@
> +/*
> + * libqos malloc support for PPC64
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef LIBQOS_MALLOC_PPC64_H
> +#define LIBQOS_MALLOC_PPC64_H
> +
> +#include "libqos/malloc.h"
> +
> +QGuestAllocator *ppc64_alloc_init(void);
> +QGuestAllocator *ppc64_alloc_init_flags(QAllocOpts flags);
> +void ppc64_alloc_uninit(QGuestAllocator *allocator);
> +
> +#endif
> diff --git a/tests/libqos/malloc.c b/tests/libqos/malloc.c
> index b8eff5f..6a02345 100644
> --- a/tests/libqos/malloc.c
> +++ b/tests/libqos/malloc.c
> @@ -12,6 +12,9 @@
>  
>  #include "qemu/osdep.h"
>  #include "libqos/malloc.h"
> +#include "libqos/malloc-pc.h"
> +#include "libqos/malloc-ppc64.h"
> +#include "libqtest.h"
>  #include "qemu-common.h"
>  #include "qemu/host-utils.h"
>  
> @@ -375,3 +378,42 @@ void migrate_allocator(QGuestAllocator *src,
>      QTAILQ_INSERT_HEAD(src->free, node, MLIST_ENTNAME);
>      return;
>  }
> +
> +QGuestAllocator *machine_alloc_init(void)
> +{
> +    const char *arch = qtest_get_arch();
> +
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        return pc_alloc_init();
> +    }
> +    if (strcmp(arch, "ppc64") == 0) {
> +        return ppc64_alloc_init();
> +    }
> +    return NULL;
> +}
> +
> +QGuestAllocator *machine_alloc_init_flags(QAllocOpts flags)
> +{
> +    const char *arch = qtest_get_arch();
> +
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        return pc_alloc_init_flags(flags);
> +    }
> +    if (strcmp(arch, "ppc64") == 0) {
> +        return ppc64_alloc_init_flags(flags);
> +    }
> +    return NULL;
> +}
> +
> +void machine_alloc_uninit(QGuestAllocator *allocator)
> +{
> +    const char *arch = qtest_get_arch();
> +
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        pc_alloc_uninit(allocator);
> +        return;
> +    }
> +    if (strcmp(arch, "ppc64") == 0) {
> +        ppc64_alloc_uninit(allocator);
> +    }
> +}
> diff --git a/tests/libqos/malloc.h b/tests/libqos/malloc.h
> index ae9dac8..a5f4c63 100644
> --- a/tests/libqos/malloc.h
> +++ b/tests/libqos/malloc.h
> @@ -37,4 +37,7 @@ QGuestAllocator *alloc_init_flags(QAllocOpts flags,
>  void alloc_set_page_size(QGuestAllocator *allocator, size_t page_size);
>  void alloc_set_flags(QGuestAllocator *allocator, QAllocOpts opts);
>  
> +QGuestAllocator *machine_alloc_init(void);
> +QGuestAllocator *machine_alloc_init_flags(QAllocOpts flags);
> +void machine_alloc_uninit(QGuestAllocator *allocator);
>  #endif
> 

Looks fine to me now!

Reviewed-by: Thomas Huth <thuth@redhat.com>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH v4 1/3] qtest: replace strtoXX() by qemu_strtoXX()
  2016-09-06 18:17     ` Laurent Vivier
@ 2016-09-06 21:13       ` Greg Kurz
  0 siblings, 0 replies; 25+ messages in thread
From: Greg Kurz @ 2016-09-06 21:13 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: david, thuth, qemu-ppc, qemu-devel

On Tue, 6 Sep 2016 20:17:00 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> On 06/09/2016 17:55, Greg Kurz wrote:
> > On Tue,  6 Sep 2016 15:17:55 +0200
> > Laurent Vivier <lvivier@redhat.com> wrote:
> >   
> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >> ---  
> > 
> > The patch also adds error checking and assertions. Maybe worth to be mentioned
> > in the changelog...  
> 
> In case of a new version of the patch, I will...
> 
> >   
> >> v4:
> >> - add this patch in the series to change all strtoXX() in qtest.c
> >>
> >>  qtest.c | 49 ++++++++++++++++++++++++++-----------------------
> >>  1 file changed, 26 insertions(+), 23 deletions(-)
> >>
> >> diff --git a/qtest.c b/qtest.c
> >> index da4826c..4c94708 100644
> >> --- a/qtest.c
> >> +++ b/qtest.c
> >> @@ -27,6 +27,7 @@
> >>  #include "qemu/config-file.h"
> >>  #include "qemu/option.h"
> >>  #include "qemu/error-report.h"
> >> +#include "qemu/cutils.h"
> >>  
> >>  #define MAX_IRQ 256
> >>  
> >> @@ -324,12 +325,13 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
> >>      } else if (strcmp(words[0], "outb") == 0 ||
> >>                 strcmp(words[0], "outw") == 0 ||
> >>                 strcmp(words[0], "outl") == 0) {
> >> -        uint16_t addr;
> >> -        uint32_t value;
> >> +        unsigned long addr;
> >> +        unsigned long value;
> >>  
> >>          g_assert(words[1] && words[2]);
> >> -        addr = strtoul(words[1], NULL, 0);
> >> -        value = strtoul(words[2], NULL, 0);
> >> +        g_assert(qemu_strtoul(words[1], NULL, 0, &addr) == 0);
> >> +        g_assert(qemu_strtoul(words[2], NULL, 0, &value) == 0);
> >> +        g_assert(addr <= 0xffff);
> >>  
> >>          if (words[0][3] == 'b') {
> >>              cpu_outb(addr, value);
> >> @@ -343,11 +345,12 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
> >>      } else if (strcmp(words[0], "inb") == 0 ||
> >>          strcmp(words[0], "inw") == 0 ||
> >>          strcmp(words[0], "inl") == 0) {
> >> -        uint16_t addr;
> >> +        unsigned long addr;
> >>          uint32_t value = -1U;
> >>  
> >>          g_assert(words[1]);
> >> -        addr = strtoul(words[1], NULL, 0);
> >> +        g_assert(qemu_strtoul(words[1], NULL, 0, &addr) == 0);
> >> +        g_assert(addr <= 0xffff);
> >>  
> >>          if (words[0][2] == 'b') {
> >>              value = cpu_inb(addr);
> >> @@ -366,8 +369,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
> >>          uint64_t value;
> >>  
> >>          g_assert(words[1] && words[2]);
> >> -        addr = strtoull(words[1], NULL, 0);
> >> -        value = strtoull(words[2], NULL, 0);
> >> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
> >> +        g_assert(qemu_strtoull(words[2], NULL, 0, &value) == 0);
> >>  
> >>          if (words[0][5] == 'b') {
> >>              uint8_t data = value;
> >> @@ -395,7 +398,7 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
> >>          uint64_t value = UINT64_C(-1);
> >>  
> >>          g_assert(words[1]);
> >> -        addr = strtoull(words[1], NULL, 0);
> >> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
> >>  
> >>          if (words[0][4] == 'b') {
> >>              uint8_t data;
> >> @@ -421,8 +424,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
> >>          char *enc;
> >>  
> >>          g_assert(words[1] && words[2]);
> >> -        addr = strtoull(words[1], NULL, 0);
> >> -        len = strtoull(words[2], NULL, 0);
> >> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
> >> +        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
> >>  
> >>          data = g_malloc(len);
> >>          cpu_physical_memory_read(addr, data, len);
> >> @@ -443,8 +446,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
> >>          gchar *b64_data;
> >>  
> >>          g_assert(words[1] && words[2]);
> >> -        addr = strtoull(words[1], NULL, 0);
> >> -        len = strtoull(words[2], NULL, 0);
> >> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
> >> +        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
> >>  
> >>          data = g_malloc(len);
> >>          cpu_physical_memory_read(addr, data, len);
> >> @@ -460,8 +463,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
> >>          size_t data_len;
> >>  
> >>          g_assert(words[1] && words[2] && words[3]);
> >> -        addr = strtoull(words[1], NULL, 0);
> >> -        len = strtoull(words[2], NULL, 0);
> >> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
> >> +        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
> >>  
> >>          data_len = strlen(words[3]);
> >>          if (data_len < 3) {
> >> @@ -486,12 +489,12 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
> >>      } else if (strcmp(words[0], "memset") == 0) {
> >>          uint64_t addr, len;
> >>          uint8_t *data;
> >> -        uint8_t pattern;
> >> +        unsigned long pattern;
> >>  
> >>          g_assert(words[1] && words[2] && words[3]);
> >> -        addr = strtoull(words[1], NULL, 0);
> >> -        len = strtoull(words[2], NULL, 0);
> >> -        pattern = strtoull(words[3], NULL, 0);
> >> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
> >> +        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
> >> +        g_assert(qemu_strtoul(words[3], NULL, 0, &pattern) == 0);  
> > 
> > And:
> > 
> > g_assert(pattern <= 0xff)  
> 
> I think pattern > 0xff is valid as memset() takes an "int" and only uses
> the byte value (for instance to use -1 to fill memory with 0xff). It
> can't do bad things...
> 

Of course... sorry for the noise :)

> In the previous case ("g_assert(addr <= 0xffff)"), if addr > 0xffff,
> cpu_out/in can write at a bad address. We could just ignore the upper
> part of the word, but to debug test case I think it's good to have an
> assert in this case.
> 

It makes sense indeed.

> Thanks,
> Laurent

Cheers.

--
Greg

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/3] tests: make pc_alloc_init/init_flags/uninit generic
  2016-09-06 13:17 ` [Qemu-devel] [PATCH v4 2/3] tests: make pc_alloc_init/init_flags/uninit generic Laurent Vivier
  2016-09-06 19:46   ` Thomas Huth
@ 2016-09-06 21:41   ` Greg Kurz
  2016-09-07  9:36     ` Laurent Vivier
  2016-09-08  2:04   ` David Gibson
  2 siblings, 1 reply; 25+ messages in thread
From: Greg Kurz @ 2016-09-06 21:41 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: david, thuth, qemu-ppc, qemu-devel

On Tue,  6 Sep 2016 15:17:56 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> And add support for ppc64.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> v2:
> - remove useless parenthesis, inline
> 

This works indeed but I'm just feeling curious about the QOSOps type introduced
by the following commit:

commit 90e5add6f2fa0b0bd9a4c1d5a4de2304b5f3e466
Author: John Snow <jsnow@redhat.com>
Date:   Mon Jan 19 15:15:55 2015 -0500

    libqos: add pc specific interface

Wouldn't this be better to implement something similar for ppc64 instead of
relying on strcmp() ?

Cheers.

--
Greg

>  tests/Makefile.include      |  3 ++-
>  tests/libqos/libqos.h       |  2 +-
>  tests/libqos/malloc-ppc64.c | 38 ++++++++++++++++++++++++++++++++++++++
>  tests/libqos/malloc-ppc64.h | 17 +++++++++++++++++
>  tests/libqos/malloc.c       | 42 ++++++++++++++++++++++++++++++++++++++++++
>  tests/libqos/malloc.h       |  3 +++
>  6 files changed, 103 insertions(+), 2 deletions(-)
>  create mode 100644 tests/libqos/malloc-ppc64.c
>  create mode 100644 tests/libqos/malloc-ppc64.h
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 14be491..a286848 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -557,8 +557,9 @@ tests/test-crypto-block$(EXESUF): tests/test-crypto-block.o $(test-crypto-obj-y)
>  
>  libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o tests/libqos/malloc.o
>  libqos-obj-y += tests/libqos/i2c.o tests/libqos/libqos.o
> +libqos-obj-y += tests/libqos/malloc-ppc64.o tests/libqos/malloc-pc.o
>  libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
> -libqos-pc-obj-y += tests/libqos/malloc-pc.o tests/libqos/libqos-pc.o
> +libqos-pc-obj-y += tests/libqos/libqos-pc.o
>  libqos-pc-obj-y += tests/libqos/ahci.o
>  libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o
>  libqos-imx-obj-y = $(libqos-obj-y) tests/libqos/i2c-imx.o
> diff --git a/tests/libqos/libqos.h b/tests/libqos/libqos.h
> index 604980d..7b71607 100644
> --- a/tests/libqos/libqos.h
> +++ b/tests/libqos/libqos.h
> @@ -3,7 +3,7 @@
>  
>  #include "libqtest.h"
>  #include "libqos/pci.h"
> -#include "libqos/malloc-pc.h"
> +#include "libqos/malloc.h"
>  
>  typedef struct QOSOps {
>      QGuestAllocator *(*init_allocator)(QAllocOpts);
> diff --git a/tests/libqos/malloc-ppc64.c b/tests/libqos/malloc-ppc64.c
> new file mode 100644
> index 0000000..1b31e33
> --- /dev/null
> +++ b/tests/libqos/malloc-ppc64.c
> @@ -0,0 +1,38 @@
> +/*
> + * libqos malloc support for PPC64
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "libqos/malloc-ppc64.h"
> +
> +#include "qemu-common.h"
> +
> +#define PAGE_SIZE 4096
> +
> +/* Memory must be a multiple of 256 MB,
> + * so we have at least 256MB
> + */
> +#define PPC64_MIN_SIZE 0x10000000
> +
> +void ppc64_alloc_uninit(QGuestAllocator *allocator)
> +{
> +    alloc_uninit(allocator);
> +}
> +
> +QGuestAllocator *ppc64_alloc_init_flags(QAllocOpts flags)
> +{
> +    QGuestAllocator *s;
> +
> +    s = alloc_init_flags(flags, 1 << 20, PPC64_MIN_SIZE);
> +    alloc_set_page_size(s, PAGE_SIZE);
> +
> +    return s;
> +}
> +
> +QGuestAllocator *ppc64_alloc_init(void)
> +{
> +    return ppc64_alloc_init_flags(ALLOC_NO_FLAGS);
> +}
> diff --git a/tests/libqos/malloc-ppc64.h b/tests/libqos/malloc-ppc64.h
> new file mode 100644
> index 0000000..c2b2dff
> --- /dev/null
> +++ b/tests/libqos/malloc-ppc64.h
> @@ -0,0 +1,17 @@
> +/*
> + * libqos malloc support for PPC64
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef LIBQOS_MALLOC_PPC64_H
> +#define LIBQOS_MALLOC_PPC64_H
> +
> +#include "libqos/malloc.h"
> +
> +QGuestAllocator *ppc64_alloc_init(void);
> +QGuestAllocator *ppc64_alloc_init_flags(QAllocOpts flags);
> +void ppc64_alloc_uninit(QGuestAllocator *allocator);
> +
> +#endif
> diff --git a/tests/libqos/malloc.c b/tests/libqos/malloc.c
> index b8eff5f..6a02345 100644
> --- a/tests/libqos/malloc.c
> +++ b/tests/libqos/malloc.c
> @@ -12,6 +12,9 @@
>  
>  #include "qemu/osdep.h"
>  #include "libqos/malloc.h"
> +#include "libqos/malloc-pc.h"
> +#include "libqos/malloc-ppc64.h"
> +#include "libqtest.h"
>  #include "qemu-common.h"
>  #include "qemu/host-utils.h"
>  
> @@ -375,3 +378,42 @@ void migrate_allocator(QGuestAllocator *src,
>      QTAILQ_INSERT_HEAD(src->free, node, MLIST_ENTNAME);
>      return;
>  }
> +
> +QGuestAllocator *machine_alloc_init(void)
> +{
> +    const char *arch = qtest_get_arch();
> +
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        return pc_alloc_init();
> +    }
> +    if (strcmp(arch, "ppc64") == 0) {
> +        return ppc64_alloc_init();
> +    }
> +    return NULL;
> +}
> +
> +QGuestAllocator *machine_alloc_init_flags(QAllocOpts flags)
> +{
> +    const char *arch = qtest_get_arch();
> +
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        return pc_alloc_init_flags(flags);
> +    }
> +    if (strcmp(arch, "ppc64") == 0) {
> +        return ppc64_alloc_init_flags(flags);
> +    }
> +    return NULL;
> +}
> +
> +void machine_alloc_uninit(QGuestAllocator *allocator)
> +{
> +    const char *arch = qtest_get_arch();
> +
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        pc_alloc_uninit(allocator);
> +        return;
> +    }
> +    if (strcmp(arch, "ppc64") == 0) {
> +        ppc64_alloc_uninit(allocator);
> +    }
> +}
> diff --git a/tests/libqos/malloc.h b/tests/libqos/malloc.h
> index ae9dac8..a5f4c63 100644
> --- a/tests/libqos/malloc.h
> +++ b/tests/libqos/malloc.h
> @@ -37,4 +37,7 @@ QGuestAllocator *alloc_init_flags(QAllocOpts flags,
>  void alloc_set_page_size(QGuestAllocator *allocator, size_t page_size);
>  void alloc_set_flags(QGuestAllocator *allocator, QAllocOpts opts);
>  
> +QGuestAllocator *machine_alloc_init(void);
> +QGuestAllocator *machine_alloc_init_flags(QAllocOpts flags);
> +void machine_alloc_uninit(QGuestAllocator *allocator);
>  #endif

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH v4 3/3] tests: add RTAS command in the protocol
  2016-09-06 13:17 ` [Qemu-devel] [PATCH v4 3/3] tests: add RTAS command in the protocol Laurent Vivier
@ 2016-09-06 22:07   ` Greg Kurz
  2016-09-07  9:25     ` Laurent Vivier
  0 siblings, 1 reply; 25+ messages in thread
From: Greg Kurz @ 2016-09-06 22:07 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: david, thuth, qemu-ppc, qemu-devel

On Tue,  6 Sep 2016 15:17:57 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> Add a first test to validate the protocol:
> 
> - rtas/get-time-of-day compares the time
>   from the guest with the time from the host.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> v4:
> - use qemu_strtoXXX() instead strtoXX()
> 
> v3:
> - use mktimegm() instead of timegm()
>  
> v2:
> - add a missing space in qrtas_call() prototype
>  
>  hw/ppc/spapr_rtas.c         | 19 ++++++++++++
>  include/hw/ppc/spapr_rtas.h | 10 +++++++
>  qtest.c                     | 17 +++++++++++
>  tests/Makefile.include      |  3 ++
>  tests/libqos/rtas.c         | 71 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/libqos/rtas.h         | 11 +++++++
>  tests/libqtest.c            | 10 +++++++
>  tests/libqtest.h            | 15 ++++++++++
>  tests/rtas-test.c           | 42 +++++++++++++++++++++++++++
>  9 files changed, 198 insertions(+)
>  create mode 100644 include/hw/ppc/spapr_rtas.h
>  create mode 100644 tests/libqos/rtas.c
>  create mode 100644 tests/libqos/rtas.h
>  create mode 100644 tests/rtas-test.c
> 
> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> index dc058e5..8aed56f 100644
> --- a/hw/ppc/spapr_rtas.c
> +++ b/hw/ppc/spapr_rtas.c
> @@ -36,6 +36,7 @@
>  
>  #include "hw/ppc/spapr.h"
>  #include "hw/ppc/spapr_vio.h"
> +#include "hw/ppc/spapr_rtas.h"
>  #include "hw/ppc/ppc.h"
>  #include "qapi-event.h"
>  #include "hw/boards.h"
> @@ -691,6 +692,24 @@ target_ulong spapr_rtas_call(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>      return H_PARAMETER;
>  }
>  
> +uint64_t qtest_rtas_call(char *cmd, uint32_t nargs, uint64_t args,
> +                         uint32_t nret, uint64_t rets)
> +{
> +    int token;
> +
> +    for (token = 0; token < RTAS_TOKEN_MAX - RTAS_TOKEN_BASE; token++) {
> +        if (strcmp(cmd, rtas_table[token].name) == 0) {
> +            sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> +            PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
> +
> +            rtas_table[token].fn(cpu, spapr, token + RTAS_TOKEN_BASE,
> +                                 nargs, args, nret, rets);
> +            return H_SUCCESS;
> +        }
> +    }
> +    return H_PARAMETER;
> +}
> +
>  void spapr_rtas_register(int token, const char *name, spapr_rtas_fn fn)
>  {
>      assert((token >= RTAS_TOKEN_BASE) && (token < RTAS_TOKEN_MAX));
> diff --git a/include/hw/ppc/spapr_rtas.h b/include/hw/ppc/spapr_rtas.h
> new file mode 100644
> index 0000000..383611f
> --- /dev/null
> +++ b/include/hw/ppc/spapr_rtas.h
> @@ -0,0 +1,10 @@
> +#ifndef HW_SPAPR_RTAS_H
> +#define HW_SPAPR_RTAS_H
> +/*
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +uint64_t qtest_rtas_call(char *cmd, uint32_t nargs, uint64_t args,
> +                         uint32_t nret, uint64_t rets);
> +#endif /* HW_SPAPR_RTAS_H */
> diff --git a/qtest.c b/qtest.c
> index 4c94708..beb62b4 100644
> --- a/qtest.c
> +++ b/qtest.c
> @@ -28,6 +28,9 @@
>  #include "qemu/option.h"
>  #include "qemu/error-report.h"
>  #include "qemu/cutils.h"
> +#ifdef TARGET_PPC64
> +#include "hw/ppc/spapr_rtas.h"
> +#endif
>  
>  #define MAX_IRQ 256
>  
> @@ -531,6 +534,20 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>  
>          qtest_send_prefix(chr);
>          qtest_send(chr, "OK\n");
> +#ifdef TARGET_PPC64
> +    } else if (strcmp(words[0], "rtas") == 0) {
> +        uint64_t res, args, ret;
> +        unsigned long nargs, nret;
> +
> +        g_assert(qemu_strtoul(words[2], NULL, 0, &nargs) == 0);
> +        g_assert(qemu_strtoull(words[3], NULL, 0, &args) == 0);
> +        g_assert(qemu_strtoul(words[4], NULL, 0, &nret) == 0);
> +        g_assert(qemu_strtoull(words[5], NULL, 0, &ret) == 0);
> +        res = qtest_rtas_call(words[1], nargs, args, nret, ret);
> +
> +        qtest_send_prefix(chr);
> +        qtest_sendf(chr, "OK %"PRIu64"\n", res);
> +#endif
>      } else if (qtest_enabled() && strcmp(words[0], "clock_step") == 0) {
>          int64_t ns;
>  
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index a286848..c456b8b 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -272,6 +272,7 @@ check-qtest-sparc-y += tests/prom-env-test$(EXESUF)
>  check-qtest-microblazeel-y = $(check-qtest-microblaze-y)
>  check-qtest-xtensaeb-y = $(check-qtest-xtensa-y)
>  check-qtest-ppc64-y += tests/postcopy-test$(EXESUF)
> +check-qtest-ppc64-y += tests/rtas-test$(EXESUF)
>  
>  check-qtest-generic-y += tests/qom-test$(EXESUF)
>  
> @@ -558,6 +559,7 @@ tests/test-crypto-block$(EXESUF): tests/test-crypto-block.o $(test-crypto-obj-y)
>  libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o tests/libqos/malloc.o
>  libqos-obj-y += tests/libqos/i2c.o tests/libqos/libqos.o
>  libqos-obj-y += tests/libqos/malloc-ppc64.o tests/libqos/malloc-pc.o
> +libqos-obj-y += tests/libqos/rtas.o
>  libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
>  libqos-pc-obj-y += tests/libqos/libqos-pc.o
>  libqos-pc-obj-y += tests/libqos/ahci.o
> @@ -572,6 +574,7 @@ tests/m48t59-test$(EXESUF): tests/m48t59-test.o
>  tests/endianness-test$(EXESUF): tests/endianness-test.o
>  tests/spapr-phb-test$(EXESUF): tests/spapr-phb-test.o $(libqos-obj-y)
>  tests/prom-env-test$(EXESUF): tests/prom-env-test.o $(libqos-obj-y)
> +tests/rtas-test$(EXESUF): tests/rtas-test.o $(libqos-obj-y)
>  tests/fdc-test$(EXESUF): tests/fdc-test.o
>  tests/ide-test$(EXESUF): tests/ide-test.o $(libqos-pc-obj-y)
>  tests/ahci-test$(EXESUF): tests/ahci-test.o $(libqos-pc-obj-y)
> diff --git a/tests/libqos/rtas.c b/tests/libqos/rtas.c
> new file mode 100644
> index 0000000..d5f4ced
> --- /dev/null
> +++ b/tests/libqos/rtas.c
> @@ -0,0 +1,71 @@
> +/*
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "libqtest.h"
> +#include "libqos/rtas.h"
> +
> +static void qrtas_copy_args(uint64_t target_args, uint32_t nargs,
> +                            uint32_t *args)
> +{
> +    int i;
> +
> +    for (i = 0; i < nargs; i++) {
> +        writel(target_args + i * sizeof(uint32_t), args[i]);
> +    }
> +}
> +
> +static void qrtas_copy_ret(uint64_t target_ret, uint32_t nret, uint32_t *ret)
> +{
> +    int i;
> +
> +    for (i = 0; i < nret; i++) {
> +        ret[i] = readl(target_ret + i * sizeof(uint32_t));
> +    }
> +}
> +
> +static uint64_t qrtas_call(QGuestAllocator *alloc, const char *name,
> +                           uint32_t nargs, uint32_t *args,
> +                           uint32_t nret, uint32_t *ret)
> +{
> +    uint64_t res;
> +    uint64_t target_args, target_ret;
> +
> +    target_args = guest_alloc(alloc, (nargs + nret) * sizeof(uint32_t));
> +    target_ret = guest_alloc(alloc, nret * sizeof(uint32_t));
> +
> +    qrtas_copy_args(target_args, nargs, args);
> +    res = qtest_rtas_call(global_qtest, name,
> +                          nargs, target_args, nret, target_ret);
> +    qrtas_copy_ret(target_ret, nret, ret);
> +
> +    guest_free(alloc, target_ret);
> +    guest_free(alloc, target_args);
> +
> +    return res;
> +}
> +
> +int qrtas_get_time_of_day(QGuestAllocator *alloc, struct tm *tm, uint32_t *ns)
> +{
> +    int res;
> +    uint32_t ret[8];
> +
> +    res = qrtas_call(alloc, "get-time-of-day", 0, NULL, 8, ret);
> +    if (res != 0) {
> +        return res;
> +    }
> +
> +    res = ret[0];
> +    memset(tm, 0, sizeof(*tm));
> +    tm->tm_year = ret[1] - 1900;
> +    tm->tm_mon = ret[2] - 1;
> +    tm->tm_mday = ret[3];
> +    tm->tm_hour = ret[4];
> +    tm->tm_min = ret[5];
> +    tm->tm_sec = ret[6];
> +    *ns = ret[7];
> +
> +    return res;
> +}
> diff --git a/tests/libqos/rtas.h b/tests/libqos/rtas.h
> new file mode 100644
> index 0000000..a1b60a8
> --- /dev/null
> +++ b/tests/libqos/rtas.h
> @@ -0,0 +1,11 @@
> +/*
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef LIBQOS_RTAS_H
> +#define LIBQOS_RTAS_H
> +#include "libqos/malloc.h"
> +
> +int qrtas_get_time_of_day(QGuestAllocator *alloc, struct tm *tm, uint32_t *ns);
> +#endif /* LIBQOS_RTAS_H */
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index eb00f13..c9dd57b 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -751,6 +751,16 @@ void qtest_memread(QTestState *s, uint64_t addr, void *data, size_t size)
>      g_strfreev(args);
>  }
>  
> +uint64_t qtest_rtas_call(QTestState *s, const char *name,
> +                         uint32_t nargs, uint64_t args,
> +                         uint32_t nret, uint64_t ret)
> +{
> +    qtest_sendf(s, "rtas %s %u 0x%"PRIx64" %u 0x%"PRIx64"\n",
> +                name, nargs, args, nret, ret);
> +    qtest_rsp(s, 0);
> +    return 0;
> +}
> +
>  void qtest_add_func(const char *str, void (*fn)(void))
>  {
>      gchar *path = g_strdup_printf("/%s/%s", qtest_get_arch(), str);
> diff --git a/tests/libqtest.h b/tests/libqtest.h
> index 37f37ad..1badb76 100644
> --- a/tests/libqtest.h
> +++ b/tests/libqtest.h
> @@ -318,6 +318,21 @@ uint64_t qtest_readq(QTestState *s, uint64_t addr);
>  void qtest_memread(QTestState *s, uint64_t addr, void *data, size_t size);
>  
>  /**
> + * qtest_rtas_call:
> + * @s: #QTestState instance to operate on.
> + * @name: name of the command to call.
> + * @nargs: Number of args.
> + * @args: Guest address to read args from.
> + * @nret: Number of return value.
> + * @ret: Guest address to write return values to.
> + *
> + * Call an RTAS function
> + */
> +uint64_t qtest_rtas_call(QTestState *s, const char *name,
> +                         uint32_t nargs, uint64_t args,
> +                         uint32_t nret, uint64_t ret);
> +
> +/**
>   * qtest_bufread:
>   * @s: #QTestState instance to operate on.
>   * @addr: Guest address to read from.
> diff --git a/tests/rtas-test.c b/tests/rtas-test.c
> new file mode 100644
> index 0000000..1be625c
> --- /dev/null
> +++ b/tests/rtas-test.c
> @@ -0,0 +1,42 @@
> +#include "qemu/osdep.h"
> +#include "qemu/cutils.h"
> +#include "libqtest.h"
> +
> +#include "libqos/rtas.h"
> +
> +static void test_rtas_get_time_of_day(void)
> +{
> +    QGuestAllocator *alloc;
> +    struct tm tm;
> +    uint32_t ns;
> +    uint64_t ret;
> +    time_t t1, t2;
> +
> +    qtest_start("");
> +
> +    alloc = machine_alloc_init();
> +
> +    t1 = time(NULL);
> +    ret = qrtas_get_time_of_day(alloc, &tm, &ns);
> +    g_assert_cmpint(ret, ==, 0);
> +    t2 = mktimegm(&tm);
> +    g_assert(t2 - t1 < 5); /* 5 sec max to run the test */
> +
> +    machine_alloc_uninit(alloc);
> +    qtest_quit(global_qtest);

Shouldn't this be qtest_end() since qtest_quit() passes its argument to
g_free() ?

> +}
> +
> +int main(int argc, char *argv[])
> +{
> +    const char *arch = qtest_get_arch();
> +
> +    g_test_init(&argc, &argv, NULL);
> +
> +    if (strcmp(arch, "ppc64") == 0) {
> +        qtest_add_func("rtas/get-time-of-day", test_rtas_get_time_of_day);
> +    } else {
> +        g_assert_not_reached();
> +    }
> +
> +    return g_test_run();
> +}

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH v4 3/3] tests: add RTAS command in the protocol
  2016-09-06 22:07   ` Greg Kurz
@ 2016-09-07  9:25     ` Laurent Vivier
  0 siblings, 0 replies; 25+ messages in thread
From: Laurent Vivier @ 2016-09-07  9:25 UTC (permalink / raw)
  To: Greg Kurz; +Cc: david, thuth, qemu-ppc, qemu-devel



On 07/09/2016 00:07, Greg Kurz wrote:
> On Tue,  6 Sep 2016 15:17:57 +0200
> Laurent Vivier <lvivier@redhat.com> wrote:
> 
>> Add a first test to validate the protocol:
>>
>> - rtas/get-time-of-day compares the time
>>   from the guest with the time from the host.
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>> v4:
>> - use qemu_strtoXXX() instead strtoXX()
>>
>> v3:
>> - use mktimegm() instead of timegm()
>>  
>> v2:
>> - add a missing space in qrtas_call() prototype
>>  
>>  hw/ppc/spapr_rtas.c         | 19 ++++++++++++
>>  include/hw/ppc/spapr_rtas.h | 10 +++++++
>>  qtest.c                     | 17 +++++++++++
>>  tests/Makefile.include      |  3 ++
>>  tests/libqos/rtas.c         | 71 +++++++++++++++++++++++++++++++++++++++++++++
>>  tests/libqos/rtas.h         | 11 +++++++
>>  tests/libqtest.c            | 10 +++++++
>>  tests/libqtest.h            | 15 ++++++++++
>>  tests/rtas-test.c           | 42 +++++++++++++++++++++++++++
>>  9 files changed, 198 insertions(+)
>>  create mode 100644 include/hw/ppc/spapr_rtas.h
>>  create mode 100644 tests/libqos/rtas.c
>>  create mode 100644 tests/libqos/rtas.h
>>  create mode 100644 tests/rtas-test.c
>>
>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>> index dc058e5..8aed56f 100644
>> --- a/hw/ppc/spapr_rtas.c
>> +++ b/hw/ppc/spapr_rtas.c
>> @@ -36,6 +36,7 @@
>>  
>>  #include "hw/ppc/spapr.h"
>>  #include "hw/ppc/spapr_vio.h"
>> +#include "hw/ppc/spapr_rtas.h"
>>  #include "hw/ppc/ppc.h"
>>  #include "qapi-event.h"
>>  #include "hw/boards.h"
>> @@ -691,6 +692,24 @@ target_ulong spapr_rtas_call(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>>      return H_PARAMETER;
>>  }
>>  
>> +uint64_t qtest_rtas_call(char *cmd, uint32_t nargs, uint64_t args,
>> +                         uint32_t nret, uint64_t rets)
>> +{
>> +    int token;
>> +
>> +    for (token = 0; token < RTAS_TOKEN_MAX - RTAS_TOKEN_BASE; token++) {
>> +        if (strcmp(cmd, rtas_table[token].name) == 0) {
>> +            sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>> +            PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
>> +
>> +            rtas_table[token].fn(cpu, spapr, token + RTAS_TOKEN_BASE,
>> +                                 nargs, args, nret, rets);
>> +            return H_SUCCESS;
>> +        }
>> +    }
>> +    return H_PARAMETER;
>> +}
>> +
>>  void spapr_rtas_register(int token, const char *name, spapr_rtas_fn fn)
>>  {
>>      assert((token >= RTAS_TOKEN_BASE) && (token < RTAS_TOKEN_MAX));
>> diff --git a/include/hw/ppc/spapr_rtas.h b/include/hw/ppc/spapr_rtas.h
>> new file mode 100644
>> index 0000000..383611f
>> --- /dev/null
>> +++ b/include/hw/ppc/spapr_rtas.h
>> @@ -0,0 +1,10 @@
>> +#ifndef HW_SPAPR_RTAS_H
>> +#define HW_SPAPR_RTAS_H
>> +/*
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +uint64_t qtest_rtas_call(char *cmd, uint32_t nargs, uint64_t args,
>> +                         uint32_t nret, uint64_t rets);
>> +#endif /* HW_SPAPR_RTAS_H */
>> diff --git a/qtest.c b/qtest.c
>> index 4c94708..beb62b4 100644
>> --- a/qtest.c
>> +++ b/qtest.c
>> @@ -28,6 +28,9 @@
>>  #include "qemu/option.h"
>>  #include "qemu/error-report.h"
>>  #include "qemu/cutils.h"
>> +#ifdef TARGET_PPC64
>> +#include "hw/ppc/spapr_rtas.h"
>> +#endif
>>  
>>  #define MAX_IRQ 256
>>  
>> @@ -531,6 +534,20 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>>  
>>          qtest_send_prefix(chr);
>>          qtest_send(chr, "OK\n");
>> +#ifdef TARGET_PPC64
>> +    } else if (strcmp(words[0], "rtas") == 0) {
>> +        uint64_t res, args, ret;
>> +        unsigned long nargs, nret;
>> +
>> +        g_assert(qemu_strtoul(words[2], NULL, 0, &nargs) == 0);
>> +        g_assert(qemu_strtoull(words[3], NULL, 0, &args) == 0);
>> +        g_assert(qemu_strtoul(words[4], NULL, 0, &nret) == 0);
>> +        g_assert(qemu_strtoull(words[5], NULL, 0, &ret) == 0);
>> +        res = qtest_rtas_call(words[1], nargs, args, nret, ret);
>> +
>> +        qtest_send_prefix(chr);
>> +        qtest_sendf(chr, "OK %"PRIu64"\n", res);
>> +#endif
>>      } else if (qtest_enabled() && strcmp(words[0], "clock_step") == 0) {
>>          int64_t ns;
>>  
>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index a286848..c456b8b 100644
>> --- a/tests/Makefile.include
>> +++ b/tests/Makefile.include
>> @@ -272,6 +272,7 @@ check-qtest-sparc-y += tests/prom-env-test$(EXESUF)
>>  check-qtest-microblazeel-y = $(check-qtest-microblaze-y)
>>  check-qtest-xtensaeb-y = $(check-qtest-xtensa-y)
>>  check-qtest-ppc64-y += tests/postcopy-test$(EXESUF)
>> +check-qtest-ppc64-y += tests/rtas-test$(EXESUF)
>>  
>>  check-qtest-generic-y += tests/qom-test$(EXESUF)
>>  
>> @@ -558,6 +559,7 @@ tests/test-crypto-block$(EXESUF): tests/test-crypto-block.o $(test-crypto-obj-y)
>>  libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o tests/libqos/malloc.o
>>  libqos-obj-y += tests/libqos/i2c.o tests/libqos/libqos.o
>>  libqos-obj-y += tests/libqos/malloc-ppc64.o tests/libqos/malloc-pc.o
>> +libqos-obj-y += tests/libqos/rtas.o
>>  libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
>>  libqos-pc-obj-y += tests/libqos/libqos-pc.o
>>  libqos-pc-obj-y += tests/libqos/ahci.o
>> @@ -572,6 +574,7 @@ tests/m48t59-test$(EXESUF): tests/m48t59-test.o
>>  tests/endianness-test$(EXESUF): tests/endianness-test.o
>>  tests/spapr-phb-test$(EXESUF): tests/spapr-phb-test.o $(libqos-obj-y)
>>  tests/prom-env-test$(EXESUF): tests/prom-env-test.o $(libqos-obj-y)
>> +tests/rtas-test$(EXESUF): tests/rtas-test.o $(libqos-obj-y)
>>  tests/fdc-test$(EXESUF): tests/fdc-test.o
>>  tests/ide-test$(EXESUF): tests/ide-test.o $(libqos-pc-obj-y)
>>  tests/ahci-test$(EXESUF): tests/ahci-test.o $(libqos-pc-obj-y)
>> diff --git a/tests/libqos/rtas.c b/tests/libqos/rtas.c
>> new file mode 100644
>> index 0000000..d5f4ced
>> --- /dev/null
>> +++ b/tests/libqos/rtas.c
>> @@ -0,0 +1,71 @@
>> +/*
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "libqtest.h"
>> +#include "libqos/rtas.h"
>> +
>> +static void qrtas_copy_args(uint64_t target_args, uint32_t nargs,
>> +                            uint32_t *args)
>> +{
>> +    int i;
>> +
>> +    for (i = 0; i < nargs; i++) {
>> +        writel(target_args + i * sizeof(uint32_t), args[i]);
>> +    }
>> +}
>> +
>> +static void qrtas_copy_ret(uint64_t target_ret, uint32_t nret, uint32_t *ret)
>> +{
>> +    int i;
>> +
>> +    for (i = 0; i < nret; i++) {
>> +        ret[i] = readl(target_ret + i * sizeof(uint32_t));
>> +    }
>> +}
>> +
>> +static uint64_t qrtas_call(QGuestAllocator *alloc, const char *name,
>> +                           uint32_t nargs, uint32_t *args,
>> +                           uint32_t nret, uint32_t *ret)
>> +{
>> +    uint64_t res;
>> +    uint64_t target_args, target_ret;
>> +
>> +    target_args = guest_alloc(alloc, (nargs + nret) * sizeof(uint32_t));
>> +    target_ret = guest_alloc(alloc, nret * sizeof(uint32_t));
>> +
>> +    qrtas_copy_args(target_args, nargs, args);
>> +    res = qtest_rtas_call(global_qtest, name,
>> +                          nargs, target_args, nret, target_ret);
>> +    qrtas_copy_ret(target_ret, nret, ret);
>> +
>> +    guest_free(alloc, target_ret);
>> +    guest_free(alloc, target_args);
>> +
>> +    return res;
>> +}
>> +
>> +int qrtas_get_time_of_day(QGuestAllocator *alloc, struct tm *tm, uint32_t *ns)
>> +{
>> +    int res;
>> +    uint32_t ret[8];
>> +
>> +    res = qrtas_call(alloc, "get-time-of-day", 0, NULL, 8, ret);
>> +    if (res != 0) {
>> +        return res;
>> +    }
>> +
>> +    res = ret[0];
>> +    memset(tm, 0, sizeof(*tm));
>> +    tm->tm_year = ret[1] - 1900;
>> +    tm->tm_mon = ret[2] - 1;
>> +    tm->tm_mday = ret[3];
>> +    tm->tm_hour = ret[4];
>> +    tm->tm_min = ret[5];
>> +    tm->tm_sec = ret[6];
>> +    *ns = ret[7];
>> +
>> +    return res;
>> +}
>> diff --git a/tests/libqos/rtas.h b/tests/libqos/rtas.h
>> new file mode 100644
>> index 0000000..a1b60a8
>> --- /dev/null
>> +++ b/tests/libqos/rtas.h
>> @@ -0,0 +1,11 @@
>> +/*
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +#ifndef LIBQOS_RTAS_H
>> +#define LIBQOS_RTAS_H
>> +#include "libqos/malloc.h"
>> +
>> +int qrtas_get_time_of_day(QGuestAllocator *alloc, struct tm *tm, uint32_t *ns);
>> +#endif /* LIBQOS_RTAS_H */
>> diff --git a/tests/libqtest.c b/tests/libqtest.c
>> index eb00f13..c9dd57b 100644
>> --- a/tests/libqtest.c
>> +++ b/tests/libqtest.c
>> @@ -751,6 +751,16 @@ void qtest_memread(QTestState *s, uint64_t addr, void *data, size_t size)
>>      g_strfreev(args);
>>  }
>>  
>> +uint64_t qtest_rtas_call(QTestState *s, const char *name,
>> +                         uint32_t nargs, uint64_t args,
>> +                         uint32_t nret, uint64_t ret)
>> +{
>> +    qtest_sendf(s, "rtas %s %u 0x%"PRIx64" %u 0x%"PRIx64"\n",
>> +                name, nargs, args, nret, ret);
>> +    qtest_rsp(s, 0);
>> +    return 0;
>> +}
>> +
>>  void qtest_add_func(const char *str, void (*fn)(void))
>>  {
>>      gchar *path = g_strdup_printf("/%s/%s", qtest_get_arch(), str);
>> diff --git a/tests/libqtest.h b/tests/libqtest.h
>> index 37f37ad..1badb76 100644
>> --- a/tests/libqtest.h
>> +++ b/tests/libqtest.h
>> @@ -318,6 +318,21 @@ uint64_t qtest_readq(QTestState *s, uint64_t addr);
>>  void qtest_memread(QTestState *s, uint64_t addr, void *data, size_t size);
>>  
>>  /**
>> + * qtest_rtas_call:
>> + * @s: #QTestState instance to operate on.
>> + * @name: name of the command to call.
>> + * @nargs: Number of args.
>> + * @args: Guest address to read args from.
>> + * @nret: Number of return value.
>> + * @ret: Guest address to write return values to.
>> + *
>> + * Call an RTAS function
>> + */
>> +uint64_t qtest_rtas_call(QTestState *s, const char *name,
>> +                         uint32_t nargs, uint64_t args,
>> +                         uint32_t nret, uint64_t ret);
>> +
>> +/**
>>   * qtest_bufread:
>>   * @s: #QTestState instance to operate on.
>>   * @addr: Guest address to read from.
>> diff --git a/tests/rtas-test.c b/tests/rtas-test.c
>> new file mode 100644
>> index 0000000..1be625c
>> --- /dev/null
>> +++ b/tests/rtas-test.c
>> @@ -0,0 +1,42 @@
>> +#include "qemu/osdep.h"
>> +#include "qemu/cutils.h"
>> +#include "libqtest.h"
>> +
>> +#include "libqos/rtas.h"
>> +
>> +static void test_rtas_get_time_of_day(void)
>> +{
>> +    QGuestAllocator *alloc;
>> +    struct tm tm;
>> +    uint32_t ns;
>> +    uint64_t ret;
>> +    time_t t1, t2;
>> +
>> +    qtest_start("");
>> +
>> +    alloc = machine_alloc_init();
>> +
>> +    t1 = time(NULL);
>> +    ret = qrtas_get_time_of_day(alloc, &tm, &ns);
>> +    g_assert_cmpint(ret, ==, 0);
>> +    t2 = mktimegm(&tm);
>> +    g_assert(t2 - t1 < 5); /* 5 sec max to run the test */
>> +
>> +    machine_alloc_uninit(alloc);
>> +    qtest_quit(global_qtest);
> 
> Shouldn't this be qtest_end() since qtest_quit() passes its argument to
> g_free() ?

Yes, I think it would be better.

Thanks,
Laurent

>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> +    const char *arch = qtest_get_arch();
>> +
>> +    g_test_init(&argc, &argv, NULL);
>> +
>> +    if (strcmp(arch, "ppc64") == 0) {
>> +        qtest_add_func("rtas/get-time-of-day", test_rtas_get_time_of_day);
>> +    } else {
>> +        g_assert_not_reached();
>> +    }
>> +
>> +    return g_test_run();
>> +}
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/3] tests: make pc_alloc_init/init_flags/uninit generic
  2016-09-06 21:41   ` Greg Kurz
@ 2016-09-07  9:36     ` Laurent Vivier
  2016-09-07 12:44       ` Greg Kurz
  0 siblings, 1 reply; 25+ messages in thread
From: Laurent Vivier @ 2016-09-07  9:36 UTC (permalink / raw)
  To: Greg Kurz; +Cc: david, thuth, qemu-ppc, qemu-devel



On 06/09/2016 23:41, Greg Kurz wrote:
> On Tue,  6 Sep 2016 15:17:56 +0200
> Laurent Vivier <lvivier@redhat.com> wrote:
> 
>> And add support for ppc64.
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>> v2:
>> - remove useless parenthesis, inline
>>
> 
> This works indeed but I'm just feeling curious about the QOSOps type introduced
> by the following commit:
> 
> commit 90e5add6f2fa0b0bd9a4c1d5a4de2304b5f3e466
> Author: John Snow <jsnow@redhat.com>
> Date:   Mon Jan 19 15:15:55 2015 -0500
> 
>     libqos: add pc specific interface
> 
> Wouldn't this be better to implement something similar for ppc64 instead of
> relying on strcmp() ?

Tests can be generic and to be run on several archs: we need the
strcmp() to check the guest arch [1], it can't be hardcoded in the test.

Thanks,
Laurent

[1]
const char *qtest_get_arch(void)
{
    const char *qemu = getenv("QTEST_QEMU_BINARY");
    g_assert(qemu != NULL);
    const char *end = strrchr(qemu, '/');

    return end + strlen("/qemu-system-");
}

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/3] tests: make pc_alloc_init/init_flags/uninit generic
  2016-09-07  9:36     ` Laurent Vivier
@ 2016-09-07 12:44       ` Greg Kurz
  2016-09-07 13:10         ` Laurent Vivier
  0 siblings, 1 reply; 25+ messages in thread
From: Greg Kurz @ 2016-09-07 12:44 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: david, thuth, qemu-ppc, qemu-devel

On Wed, 7 Sep 2016 11:36:21 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> On 06/09/2016 23:41, Greg Kurz wrote:
> > On Tue,  6 Sep 2016 15:17:56 +0200
> > Laurent Vivier <lvivier@redhat.com> wrote:
> >   
> >> And add support for ppc64.
> >>
> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >> ---
> >> v2:
> >> - remove useless parenthesis, inline
> >>  
> > 
> > This works indeed but I'm just feeling curious about the QOSOps type introduced
> > by the following commit:
> > 
> > commit 90e5add6f2fa0b0bd9a4c1d5a4de2304b5f3e466
> > Author: John Snow <jsnow@redhat.com>
> > Date:   Mon Jan 19 15:15:55 2015 -0500
> > 
> >     libqos: add pc specific interface
> > 
> > Wouldn't this be better to implement something similar for ppc64 instead of
> > relying on strcmp() ?  
> 
> Tests can be generic and to be run on several archs: we need the
> strcmp() to check the guest arch [1], it can't be hardcoded in the test.
> 

I agree for truely platform agnostic tests, but this is obviously not the case
for RTAS, which is the goal of this series.

My suggestion is basically to:
- keep malloc-ppc64.[ch] from your series
- introduce libqos-ppc64.[ch] like the existing libqos-pc.[ch]
- add qtest_ppc64_[start|end]() wrappers to pass global_qtest to
  qtest_ppc64_[boot|shutdown]()
- adapt the final RTAS test patch to use these wrappers and q[malloc|free]()

BTW, maybe s/ppc64/ppc to match hw/ppc, since libqos is about HW platforms,
not target archs.

This is more work, but I guess in the end it maybe useful in the long term.

And, of course, I'm volunteering to participate, with patches/reviewing/testing.

Makes sense ?

Cheers.

--
Greg

> Thanks,
> Laurent
> 
> [1]
> const char *qtest_get_arch(void)
> {
>     const char *qemu = getenv("QTEST_QEMU_BINARY");
>     g_assert(qemu != NULL);
>     const char *end = strrchr(qemu, '/');
> 
>     return end + strlen("/qemu-system-");
> }

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/3] tests: make pc_alloc_init/init_flags/uninit generic
  2016-09-07 12:44       ` Greg Kurz
@ 2016-09-07 13:10         ` Laurent Vivier
  2016-09-07 13:42           ` Greg Kurz
  0 siblings, 1 reply; 25+ messages in thread
From: Laurent Vivier @ 2016-09-07 13:10 UTC (permalink / raw)
  To: Greg Kurz; +Cc: david, thuth, qemu-ppc, qemu-devel



On 07/09/2016 14:44, Greg Kurz wrote:
> On Wed, 7 Sep 2016 11:36:21 +0200
> Laurent Vivier <lvivier@redhat.com> wrote:
> 
>> On 06/09/2016 23:41, Greg Kurz wrote:
>>> On Tue,  6 Sep 2016 15:17:56 +0200
>>> Laurent Vivier <lvivier@redhat.com> wrote:
>>>   
>>>> And add support for ppc64.
>>>>
>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>>> ---
>>>> v2:
>>>> - remove useless parenthesis, inline
>>>>  
>>>
>>> This works indeed but I'm just feeling curious about the QOSOps type introduced
>>> by the following commit:
>>>
>>> commit 90e5add6f2fa0b0bd9a4c1d5a4de2304b5f3e466
>>> Author: John Snow <jsnow@redhat.com>
>>> Date:   Mon Jan 19 15:15:55 2015 -0500
>>>
>>>     libqos: add pc specific interface
>>>
>>> Wouldn't this be better to implement something similar for ppc64 instead of
>>> relying on strcmp() ?  
>>
>> Tests can be generic and to be run on several archs: we need the
>> strcmp() to check the guest arch [1], it can't be hardcoded in the test.
>>
> 
> I agree for truely platform agnostic tests, but this is obviously not the case
> for RTAS, which is the goal of this series.
> 
> My suggestion is basically to:
> - keep malloc-ppc64.[ch] from your series
> - introduce libqos-ppc64.[ch] like the existing libqos-pc.[ch]
> - add qtest_ppc64_[start|end]() wrappers to pass global_qtest to
>   qtest_ppc64_[boot|shutdown]()
> - adapt the final RTAS test patch to use these wrappers and q[malloc|free]()

You're right it's the good way to implement guest memory allocation...
I'm going to add this part in the series.

> BTW, maybe s/ppc64/ppc to match hw/ppc, since libqos is about HW platforms,
> not target archs.

I use ppc64 because we guess guest memory is at least 256MB, and this is
true only with ppc64/pseries. With ppc, I think we should use qfw_cfg()
as for PC.

Thanks,
Laurent
> This is more work, but I guess in the end it maybe useful in the long term.
> 
> And, of course, I'm volunteering to participate, with patches/reviewing/testing.
> 
> Makes sense ?
> 
> Cheers.
> 
> --
> Greg
> 
>> Thanks,
>> Laurent
>>
>> [1]
>> const char *qtest_get_arch(void)
>> {
>>     const char *qemu = getenv("QTEST_QEMU_BINARY");
>>     g_assert(qemu != NULL);
>>     const char *end = strrchr(qemu, '/');
>>
>>     return end + strlen("/qemu-system-");
>> }
> 

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/3] tests: make pc_alloc_init/init_flags/uninit generic
  2016-09-07 13:10         ` Laurent Vivier
@ 2016-09-07 13:42           ` Greg Kurz
  0 siblings, 0 replies; 25+ messages in thread
From: Greg Kurz @ 2016-09-07 13:42 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: david, thuth, qemu-ppc, qemu-devel

On Wed, 7 Sep 2016 15:10:59 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> On 07/09/2016 14:44, Greg Kurz wrote:
> > On Wed, 7 Sep 2016 11:36:21 +0200
> > Laurent Vivier <lvivier@redhat.com> wrote:
> >   
> >> On 06/09/2016 23:41, Greg Kurz wrote:  
> >>> On Tue,  6 Sep 2016 15:17:56 +0200
> >>> Laurent Vivier <lvivier@redhat.com> wrote:
> >>>     
> >>>> And add support for ppc64.
> >>>>
> >>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >>>> ---
> >>>> v2:
> >>>> - remove useless parenthesis, inline
> >>>>    
> >>>
> >>> This works indeed but I'm just feeling curious about the QOSOps type introduced
> >>> by the following commit:
> >>>
> >>> commit 90e5add6f2fa0b0bd9a4c1d5a4de2304b5f3e466
> >>> Author: John Snow <jsnow@redhat.com>
> >>> Date:   Mon Jan 19 15:15:55 2015 -0500
> >>>
> >>>     libqos: add pc specific interface
> >>>
> >>> Wouldn't this be better to implement something similar for ppc64 instead of
> >>> relying on strcmp() ?    
> >>
> >> Tests can be generic and to be run on several archs: we need the
> >> strcmp() to check the guest arch [1], it can't be hardcoded in the test.
> >>  
> > 
> > I agree for truely platform agnostic tests, but this is obviously not the case
> > for RTAS, which is the goal of this series.
> > 
> > My suggestion is basically to:
> > - keep malloc-ppc64.[ch] from your series
> > - introduce libqos-ppc64.[ch] like the existing libqos-pc.[ch]
> > - add qtest_ppc64_[start|end]() wrappers to pass global_qtest to
> >   qtest_ppc64_[boot|shutdown]()
> > - adapt the final RTAS test patch to use these wrappers and q[malloc|free]()  
> 
> You're right it's the good way to implement guest memory allocation...
> I'm going to add this part in the series.
> 
> > BTW, maybe s/ppc64/ppc to match hw/ppc, since libqos is about HW platforms,
> > not target archs.  
> 
> I use ppc64 because we guess guest memory is at least 256MB, and this is
> true only with ppc64/pseries. With ppc, I think we should use qfw_cfg()
> as for PC.
> 

True. In this case, maybe you should even use spapr, since RTAS is for pseries
only, and so will be the PCI bits you mention in the cover letter.

Cheers.

--
Greg

> Thanks,
> Laurent
> > This is more work, but I guess in the end it maybe useful in the long term.
> > 
> > And, of course, I'm volunteering to participate, with patches/reviewing/testing.
> > 
> > Makes sense ?
> > 
> > Cheers.
> > 
> > --
> > Greg
> >   
> >> Thanks,
> >> Laurent
> >>
> >> [1]
> >> const char *qtest_get_arch(void)
> >> {
> >>     const char *qemu = getenv("QTEST_QEMU_BINARY");
> >>     g_assert(qemu != NULL);
> >>     const char *end = strrchr(qemu, '/');
> >>
> >>     return end + strlen("/qemu-system-");
> >> }  
> >   

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH v4 1/3] qtest: replace strtoXX() by qemu_strtoXX()
  2016-09-06 13:17 ` [Qemu-devel] [PATCH v4 1/3] qtest: replace strtoXX() by qemu_strtoXX() Laurent Vivier
  2016-09-06 15:55   ` Greg Kurz
@ 2016-09-08  1:56   ` David Gibson
  2016-09-08  7:55   ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
  2 siblings, 0 replies; 25+ messages in thread
From: David Gibson @ 2016-09-08  1:56 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: thuth, qemu-ppc, qemu-devel, groug

[-- Attachment #1: Type: text/plain, Size: 6577 bytes --]

On Tue, Sep 06, 2016 at 03:17:55PM +0200, Laurent Vivier wrote:
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> v4:
> - add this patch in the series to change all strtoXX() in qtest.c
> 
>  qtest.c | 49 ++++++++++++++++++++++++++-----------------------
>  1 file changed, 26 insertions(+), 23 deletions(-)
> 
> diff --git a/qtest.c b/qtest.c
> index da4826c..4c94708 100644
> --- a/qtest.c
> +++ b/qtest.c
> @@ -27,6 +27,7 @@
>  #include "qemu/config-file.h"
>  #include "qemu/option.h"
>  #include "qemu/error-report.h"
> +#include "qemu/cutils.h"
>  
>  #define MAX_IRQ 256
>  
> @@ -324,12 +325,13 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>      } else if (strcmp(words[0], "outb") == 0 ||
>                 strcmp(words[0], "outw") == 0 ||
>                 strcmp(words[0], "outl") == 0) {
> -        uint16_t addr;
> -        uint32_t value;
> +        unsigned long addr;
> +        unsigned long value;
>  
>          g_assert(words[1] && words[2]);
> -        addr = strtoul(words[1], NULL, 0);
> -        value = strtoul(words[2], NULL, 0);
> +        g_assert(qemu_strtoul(words[1], NULL, 0, &addr) == 0);
> +        g_assert(qemu_strtoul(words[2], NULL, 0, &value) == 0);
> +        g_assert(addr <= 0xffff);
>  
>          if (words[0][3] == 'b') {
>              cpu_outb(addr, value);
> @@ -343,11 +345,12 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>      } else if (strcmp(words[0], "inb") == 0 ||
>          strcmp(words[0], "inw") == 0 ||
>          strcmp(words[0], "inl") == 0) {
> -        uint16_t addr;
> +        unsigned long addr;
>          uint32_t value = -1U;
>  
>          g_assert(words[1]);
> -        addr = strtoul(words[1], NULL, 0);
> +        g_assert(qemu_strtoul(words[1], NULL, 0, &addr) == 0);
> +        g_assert(addr <= 0xffff);
>  
>          if (words[0][2] == 'b') {
>              value = cpu_inb(addr);
> @@ -366,8 +369,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>          uint64_t value;
>  
>          g_assert(words[1] && words[2]);
> -        addr = strtoull(words[1], NULL, 0);
> -        value = strtoull(words[2], NULL, 0);
> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
> +        g_assert(qemu_strtoull(words[2], NULL, 0, &value) == 0);
>  
>          if (words[0][5] == 'b') {
>              uint8_t data = value;
> @@ -395,7 +398,7 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>          uint64_t value = UINT64_C(-1);
>  
>          g_assert(words[1]);
> -        addr = strtoull(words[1], NULL, 0);
> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
>  
>          if (words[0][4] == 'b') {
>              uint8_t data;
> @@ -421,8 +424,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>          char *enc;
>  
>          g_assert(words[1] && words[2]);
> -        addr = strtoull(words[1], NULL, 0);
> -        len = strtoull(words[2], NULL, 0);
> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
> +        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
>  
>          data = g_malloc(len);
>          cpu_physical_memory_read(addr, data, len);
> @@ -443,8 +446,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>          gchar *b64_data;
>  
>          g_assert(words[1] && words[2]);
> -        addr = strtoull(words[1], NULL, 0);
> -        len = strtoull(words[2], NULL, 0);
> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
> +        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
>  
>          data = g_malloc(len);
>          cpu_physical_memory_read(addr, data, len);
> @@ -460,8 +463,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>          size_t data_len;
>  
>          g_assert(words[1] && words[2] && words[3]);
> -        addr = strtoull(words[1], NULL, 0);
> -        len = strtoull(words[2], NULL, 0);
> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
> +        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
>  
>          data_len = strlen(words[3]);
>          if (data_len < 3) {
> @@ -486,12 +489,12 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>      } else if (strcmp(words[0], "memset") == 0) {
>          uint64_t addr, len;
>          uint8_t *data;
> -        uint8_t pattern;
> +        unsigned long pattern;
>  
>          g_assert(words[1] && words[2] && words[3]);
> -        addr = strtoull(words[1], NULL, 0);
> -        len = strtoull(words[2], NULL, 0);
> -        pattern = strtoull(words[3], NULL, 0);
> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
> +        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
> +        g_assert(qemu_strtoul(words[3], NULL, 0, &pattern) == 0);
>  
>          data = g_malloc(len);
>          memset(data, pattern, len);
> @@ -507,8 +510,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>          gsize out_len;
>  
>          g_assert(words[1] && words[2] && words[3]);
> -        addr = strtoull(words[1], NULL, 0);
> -        len = strtoull(words[2], NULL, 0);
> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
> +        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
>  
>          data_len = strlen(words[3]);
>          if (data_len < 3) {
> @@ -532,7 +535,7 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>          int64_t ns;
>  
>          if (words[1]) {
> -            ns = strtoll(words[1], NULL, 0);
> +            g_assert(qemu_strtoll(words[1], NULL, 0, &ns) == 0);
>          } else {
>              ns = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL);
>          }
> @@ -544,7 +547,7 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>          int64_t ns;
>  
>          g_assert(words[1]);
> -        ns = strtoll(words[1], NULL, 0);
> +        g_assert(qemu_strtoll(words[1], NULL, 0, &ns) == 0);
>          qtest_clock_warp(ns);
>          qtest_send_prefix(chr);
>          qtest_sendf(chr, "OK %"PRIi64"\n",

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/3] tests: make pc_alloc_init/init_flags/uninit generic
  2016-09-06 13:17 ` [Qemu-devel] [PATCH v4 2/3] tests: make pc_alloc_init/init_flags/uninit generic Laurent Vivier
  2016-09-06 19:46   ` Thomas Huth
  2016-09-06 21:41   ` Greg Kurz
@ 2016-09-08  2:04   ` David Gibson
  2016-09-08  7:50     ` Laurent Vivier
  2 siblings, 1 reply; 25+ messages in thread
From: David Gibson @ 2016-09-08  2:04 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: thuth, qemu-ppc, qemu-devel, groug

[-- Attachment #1: Type: text/plain, Size: 6575 bytes --]

On Tue, Sep 06, 2016 at 03:17:56PM +0200, Laurent Vivier wrote:
> And add support for ppc64.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Some of my coments may be obsoleted by the discussion with Greg.

> ---
> v2:
> - remove useless parenthesis, inline
> 
>  tests/Makefile.include      |  3 ++-
>  tests/libqos/libqos.h       |  2 +-
>  tests/libqos/malloc-ppc64.c | 38 ++++++++++++++++++++++++++++++++++++++
>  tests/libqos/malloc-ppc64.h | 17 +++++++++++++++++
>  tests/libqos/malloc.c       | 42 ++++++++++++++++++++++++++++++++++++++++++
>  tests/libqos/malloc.h       |  3 +++
>  6 files changed, 103 insertions(+), 2 deletions(-)
>  create mode 100644 tests/libqos/malloc-ppc64.c
>  create mode 100644 tests/libqos/malloc-ppc64.h
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 14be491..a286848 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -557,8 +557,9 @@ tests/test-crypto-block$(EXESUF): tests/test-crypto-block.o $(test-crypto-obj-y)
>  
>  libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o tests/libqos/malloc.o
>  libqos-obj-y += tests/libqos/i2c.o tests/libqos/libqos.o
> +libqos-obj-y += tests/libqos/malloc-ppc64.o tests/libqos/malloc-pc.o
>  libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
> -libqos-pc-obj-y += tests/libqos/malloc-pc.o tests/libqos/libqos-pc.o
> +libqos-pc-obj-y += tests/libqos/libqos-pc.o
>  libqos-pc-obj-y += tests/libqos/ahci.o
>  libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o
>  libqos-imx-obj-y = $(libqos-obj-y) tests/libqos/i2c-imx.o
> diff --git a/tests/libqos/libqos.h b/tests/libqos/libqos.h
> index 604980d..7b71607 100644
> --- a/tests/libqos/libqos.h
> +++ b/tests/libqos/libqos.h
> @@ -3,7 +3,7 @@
>  
>  #include "libqtest.h"
>  #include "libqos/pci.h"
> -#include "libqos/malloc-pc.h"
> +#include "libqos/malloc.h"
>  
>  typedef struct QOSOps {
>      QGuestAllocator *(*init_allocator)(QAllocOpts);
> diff --git a/tests/libqos/malloc-ppc64.c b/tests/libqos/malloc-ppc64.c
> new file mode 100644
> index 0000000..1b31e33
> --- /dev/null
> +++ b/tests/libqos/malloc-ppc64.c
> @@ -0,0 +1,38 @@
> +/*
> + * libqos malloc support for PPC64
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "libqos/malloc-ppc64.h"
> +
> +#include "qemu-common.h"
> +
> +#define PAGE_SIZE 4096
> +
> +/* Memory must be a multiple of 256 MB,
> + * so we have at least 256MB
> + */
> +#define PPC64_MIN_SIZE 0x10000000

So, this is really a "pseries" machine type fact rather than a ppc64
fact.  Is there a way to make this dependent on machine type rather
than target architecture?  Naming this based on machine type would
seem to better match the "pc" things these are based on ("pc" rather
than "i386" or "x86_64").

> +
> +void ppc64_alloc_uninit(QGuestAllocator *allocator)
> +{
> +    alloc_uninit(allocator);
> +}
> +
> +QGuestAllocator *ppc64_alloc_init_flags(QAllocOpts flags)
> +{
> +    QGuestAllocator *s;
> +
> +    s = alloc_init_flags(flags, 1 << 20, PPC64_MIN_SIZE);
> +    alloc_set_page_size(s, PAGE_SIZE);
> +
> +    return s;
> +}
> +
> +QGuestAllocator *ppc64_alloc_init(void)
> +{
> +    return ppc64_alloc_init_flags(ALLOC_NO_FLAGS);
> +}
> diff --git a/tests/libqos/malloc-ppc64.h b/tests/libqos/malloc-ppc64.h
> new file mode 100644
> index 0000000..c2b2dff
> --- /dev/null
> +++ b/tests/libqos/malloc-ppc64.h
> @@ -0,0 +1,17 @@
> +/*
> + * libqos malloc support for PPC64
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#ifndef LIBQOS_MALLOC_PPC64_H
> +#define LIBQOS_MALLOC_PPC64_H
> +
> +#include "libqos/malloc.h"
> +
> +QGuestAllocator *ppc64_alloc_init(void);
> +QGuestAllocator *ppc64_alloc_init_flags(QAllocOpts flags);
> +void ppc64_alloc_uninit(QGuestAllocator *allocator);
> +
> +#endif
> diff --git a/tests/libqos/malloc.c b/tests/libqos/malloc.c
> index b8eff5f..6a02345 100644
> --- a/tests/libqos/malloc.c
> +++ b/tests/libqos/malloc.c
> @@ -12,6 +12,9 @@
>  
>  #include "qemu/osdep.h"
>  #include "libqos/malloc.h"
> +#include "libqos/malloc-pc.h"
> +#include "libqos/malloc-ppc64.h"
> +#include "libqtest.h"
>  #include "qemu-common.h"
>  #include "qemu/host-utils.h"
>  
> @@ -375,3 +378,42 @@ void migrate_allocator(QGuestAllocator *src,
>      QTAILQ_INSERT_HEAD(src->free, node, MLIST_ENTNAME);
>      return;
>  }
> +
> +QGuestAllocator *machine_alloc_init(void)
> +{
> +    const char *arch = qtest_get_arch();

Maybe we need to add a qtest_get_machine_type().

> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        return pc_alloc_init();
> +    }
> +    if (strcmp(arch, "ppc64") == 0) {
> +        return ppc64_alloc_init();
> +    }
> +    return NULL;
> +}
> +
> +QGuestAllocator *machine_alloc_init_flags(QAllocOpts flags)
> +{
> +    const char *arch = qtest_get_arch();
> +
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        return pc_alloc_init_flags(flags);
> +    }
> +    if (strcmp(arch, "ppc64") == 0) {
> +        return ppc64_alloc_init_flags(flags);
> +    }
> +    return NULL;
> +}
> +
> +void machine_alloc_uninit(QGuestAllocator *allocator)
> +{
> +    const char *arch = qtest_get_arch();
> +
> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
> +        pc_alloc_uninit(allocator);
> +        return;
> +    }
> +    if (strcmp(arch, "ppc64") == 0) {
> +        ppc64_alloc_uninit(allocator);
> +    }
> +}
> diff --git a/tests/libqos/malloc.h b/tests/libqos/malloc.h
> index ae9dac8..a5f4c63 100644
> --- a/tests/libqos/malloc.h
> +++ b/tests/libqos/malloc.h
> @@ -37,4 +37,7 @@ QGuestAllocator *alloc_init_flags(QAllocOpts flags,
>  void alloc_set_page_size(QGuestAllocator *allocator, size_t page_size);
>  void alloc_set_flags(QGuestAllocator *allocator, QAllocOpts opts);
>  
> +QGuestAllocator *machine_alloc_init(void);
> +QGuestAllocator *machine_alloc_init_flags(QAllocOpts flags);
> +void machine_alloc_uninit(QGuestAllocator *allocator);
>  #endif

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/3] tests: make pc_alloc_init/init_flags/uninit generic
  2016-09-08  2:04   ` David Gibson
@ 2016-09-08  7:50     ` Laurent Vivier
  2016-09-09 12:25       ` Greg Kurz
  0 siblings, 1 reply; 25+ messages in thread
From: Laurent Vivier @ 2016-09-08  7:50 UTC (permalink / raw)
  To: David Gibson; +Cc: thuth, qemu-ppc, qemu-devel, groug



On 08/09/2016 04:04, David Gibson wrote:
> On Tue, Sep 06, 2016 at 03:17:56PM +0200, Laurent Vivier wrote:
>> And add support for ppc64.
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> 
> Some of my coments may be obsoleted by the discussion with Greg.
> 
>> ---
>> v2:
>> - remove useless parenthesis, inline
>>
>>  tests/Makefile.include      |  3 ++-
>>  tests/libqos/libqos.h       |  2 +-
>>  tests/libqos/malloc-ppc64.c | 38 ++++++++++++++++++++++++++++++++++++++
>>  tests/libqos/malloc-ppc64.h | 17 +++++++++++++++++
>>  tests/libqos/malloc.c       | 42 ++++++++++++++++++++++++++++++++++++++++++
>>  tests/libqos/malloc.h       |  3 +++
>>  6 files changed, 103 insertions(+), 2 deletions(-)
>>  create mode 100644 tests/libqos/malloc-ppc64.c
>>  create mode 100644 tests/libqos/malloc-ppc64.h
>>
>> diff --git a/tests/Makefile.include b/tests/Makefile.include
>> index 14be491..a286848 100644
>> --- a/tests/Makefile.include
>> +++ b/tests/Makefile.include
>> @@ -557,8 +557,9 @@ tests/test-crypto-block$(EXESUF): tests/test-crypto-block.o $(test-crypto-obj-y)
>>  
>>  libqos-obj-y = tests/libqos/pci.o tests/libqos/fw_cfg.o tests/libqos/malloc.o
>>  libqos-obj-y += tests/libqos/i2c.o tests/libqos/libqos.o
>> +libqos-obj-y += tests/libqos/malloc-ppc64.o tests/libqos/malloc-pc.o
>>  libqos-pc-obj-y = $(libqos-obj-y) tests/libqos/pci-pc.o
>> -libqos-pc-obj-y += tests/libqos/malloc-pc.o tests/libqos/libqos-pc.o
>> +libqos-pc-obj-y += tests/libqos/libqos-pc.o
>>  libqos-pc-obj-y += tests/libqos/ahci.o
>>  libqos-omap-obj-y = $(libqos-obj-y) tests/libqos/i2c-omap.o
>>  libqos-imx-obj-y = $(libqos-obj-y) tests/libqos/i2c-imx.o
>> diff --git a/tests/libqos/libqos.h b/tests/libqos/libqos.h
>> index 604980d..7b71607 100644
>> --- a/tests/libqos/libqos.h
>> +++ b/tests/libqos/libqos.h
>> @@ -3,7 +3,7 @@
>>  
>>  #include "libqtest.h"
>>  #include "libqos/pci.h"
>> -#include "libqos/malloc-pc.h"
>> +#include "libqos/malloc.h"
>>  
>>  typedef struct QOSOps {
>>      QGuestAllocator *(*init_allocator)(QAllocOpts);
>> diff --git a/tests/libqos/malloc-ppc64.c b/tests/libqos/malloc-ppc64.c
>> new file mode 100644
>> index 0000000..1b31e33
>> --- /dev/null
>> +++ b/tests/libqos/malloc-ppc64.c
>> @@ -0,0 +1,38 @@
>> +/*
>> + * libqos malloc support for PPC64
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "libqos/malloc-ppc64.h"
>> +
>> +#include "qemu-common.h"
>> +
>> +#define PAGE_SIZE 4096
>> +
>> +/* Memory must be a multiple of 256 MB,
>> + * so we have at least 256MB
>> + */
>> +#define PPC64_MIN_SIZE 0x10000000
> 
> So, this is really a "pseries" machine type fact rather than a ppc64
> fact.  Is there a way to make this dependent on machine type rather
> than target architecture?  Naming this based on machine type would
> seem to better match the "pc" things these are based on ("pc" rather
> than "i386" or "x86_64").

I've changed all my "ppc64" by "spapr". Is "pseries" better?

>> +
>> +void ppc64_alloc_uninit(QGuestAllocator *allocator)
>> +{
>> +    alloc_uninit(allocator);
>> +}
>> +
>> +QGuestAllocator *ppc64_alloc_init_flags(QAllocOpts flags)
>> +{
>> +    QGuestAllocator *s;
>> +
>> +    s = alloc_init_flags(flags, 1 << 20, PPC64_MIN_SIZE);
>> +    alloc_set_page_size(s, PAGE_SIZE);
>> +
>> +    return s;
>> +}
>> +
>> +QGuestAllocator *ppc64_alloc_init(void)
>> +{
>> +    return ppc64_alloc_init_flags(ALLOC_NO_FLAGS);
>> +}
>> diff --git a/tests/libqos/malloc-ppc64.h b/tests/libqos/malloc-ppc64.h
>> new file mode 100644
>> index 0000000..c2b2dff
>> --- /dev/null
>> +++ b/tests/libqos/malloc-ppc64.h
>> @@ -0,0 +1,17 @@
>> +/*
>> + * libqos malloc support for PPC64
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + */
>> +
>> +#ifndef LIBQOS_MALLOC_PPC64_H
>> +#define LIBQOS_MALLOC_PPC64_H
>> +
>> +#include "libqos/malloc.h"
>> +
>> +QGuestAllocator *ppc64_alloc_init(void);
>> +QGuestAllocator *ppc64_alloc_init_flags(QAllocOpts flags);
>> +void ppc64_alloc_uninit(QGuestAllocator *allocator);
>> +
>> +#endif
>> diff --git a/tests/libqos/malloc.c b/tests/libqos/malloc.c
>> index b8eff5f..6a02345 100644
>> --- a/tests/libqos/malloc.c
>> +++ b/tests/libqos/malloc.c
>> @@ -12,6 +12,9 @@
>>  
>>  #include "qemu/osdep.h"
>>  #include "libqos/malloc.h"
>> +#include "libqos/malloc-pc.h"
>> +#include "libqos/malloc-ppc64.h"
>> +#include "libqtest.h"
>>  #include "qemu-common.h"
>>  #include "qemu/host-utils.h"
>>  
>> @@ -375,3 +378,42 @@ void migrate_allocator(QGuestAllocator *src,
>>      QTAILQ_INSERT_HEAD(src->free, node, MLIST_ENTNAME);
>>      return;
>>  }
>> +
>> +QGuestAllocator *machine_alloc_init(void)
>> +{
>> +    const char *arch = qtest_get_arch();
> 
> Maybe we need to add a qtest_get_machine_type().

I'm working on that...

>> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>> +        return pc_alloc_init();
>> +    }
>> +    if (strcmp(arch, "ppc64") == 0) {
>> +        return ppc64_alloc_init();
>> +    }
>> +    return NULL;
>> +}
>> +
>> +QGuestAllocator *machine_alloc_init_flags(QAllocOpts flags)
>> +{
>> +    const char *arch = qtest_get_arch();
>> +
>> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>> +        return pc_alloc_init_flags(flags);
>> +    }
>> +    if (strcmp(arch, "ppc64") == 0) {
>> +        return ppc64_alloc_init_flags(flags);
>> +    }
>> +    return NULL;
>> +}
>> +
>> +void machine_alloc_uninit(QGuestAllocator *allocator)
>> +{
>> +    const char *arch = qtest_get_arch();
>> +
>> +    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>> +        pc_alloc_uninit(allocator);
>> +        return;
>> +    }
>> +    if (strcmp(arch, "ppc64") == 0) {
>> +        ppc64_alloc_uninit(allocator);
>> +    }
>> +}
>> diff --git a/tests/libqos/malloc.h b/tests/libqos/malloc.h
>> index ae9dac8..a5f4c63 100644
>> --- a/tests/libqos/malloc.h
>> +++ b/tests/libqos/malloc.h
>> @@ -37,4 +37,7 @@ QGuestAllocator *alloc_init_flags(QAllocOpts flags,
>>  void alloc_set_page_size(QGuestAllocator *allocator, size_t page_size);
>>  void alloc_set_flags(QGuestAllocator *allocator, QAllocOpts opts);
>>  
>> +QGuestAllocator *machine_alloc_init(void);
>> +QGuestAllocator *machine_alloc_init_flags(QAllocOpts flags);
>> +void machine_alloc_uninit(QGuestAllocator *allocator);
>>  #endif
> 
Thanks,
Laurent

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 1/3] qtest: replace strtoXX() by qemu_strtoXX()
  2016-09-06 13:17 ` [Qemu-devel] [PATCH v4 1/3] qtest: replace strtoXX() by qemu_strtoXX() Laurent Vivier
  2016-09-06 15:55   ` Greg Kurz
  2016-09-08  1:56   ` David Gibson
@ 2016-09-08  7:55   ` Greg Kurz
  2 siblings, 0 replies; 25+ messages in thread
From: Greg Kurz @ 2016-09-08  7:55 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: david, thuth, qemu-ppc, qemu-devel

On Tue,  6 Sep 2016 15:17:55 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---

FWIW,

Reviewed-by: Greg Kurz <groug@kaod.org>

> v4:
> - add this patch in the series to change all strtoXX() in qtest.c
> 
>  qtest.c | 49 ++++++++++++++++++++++++++-----------------------
>  1 file changed, 26 insertions(+), 23 deletions(-)
> 
> diff --git a/qtest.c b/qtest.c
> index da4826c..4c94708 100644
> --- a/qtest.c
> +++ b/qtest.c
> @@ -27,6 +27,7 @@
>  #include "qemu/config-file.h"
>  #include "qemu/option.h"
>  #include "qemu/error-report.h"
> +#include "qemu/cutils.h"
>  
>  #define MAX_IRQ 256
>  
> @@ -324,12 +325,13 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>      } else if (strcmp(words[0], "outb") == 0 ||
>                 strcmp(words[0], "outw") == 0 ||
>                 strcmp(words[0], "outl") == 0) {
> -        uint16_t addr;
> -        uint32_t value;
> +        unsigned long addr;
> +        unsigned long value;
>  
>          g_assert(words[1] && words[2]);
> -        addr = strtoul(words[1], NULL, 0);
> -        value = strtoul(words[2], NULL, 0);
> +        g_assert(qemu_strtoul(words[1], NULL, 0, &addr) == 0);
> +        g_assert(qemu_strtoul(words[2], NULL, 0, &value) == 0);
> +        g_assert(addr <= 0xffff);
>  
>          if (words[0][3] == 'b') {
>              cpu_outb(addr, value);
> @@ -343,11 +345,12 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>      } else if (strcmp(words[0], "inb") == 0 ||
>          strcmp(words[0], "inw") == 0 ||
>          strcmp(words[0], "inl") == 0) {
> -        uint16_t addr;
> +        unsigned long addr;
>          uint32_t value = -1U;
>  
>          g_assert(words[1]);
> -        addr = strtoul(words[1], NULL, 0);
> +        g_assert(qemu_strtoul(words[1], NULL, 0, &addr) == 0);
> +        g_assert(addr <= 0xffff);
>  
>          if (words[0][2] == 'b') {
>              value = cpu_inb(addr);
> @@ -366,8 +369,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>          uint64_t value;
>  
>          g_assert(words[1] && words[2]);
> -        addr = strtoull(words[1], NULL, 0);
> -        value = strtoull(words[2], NULL, 0);
> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
> +        g_assert(qemu_strtoull(words[2], NULL, 0, &value) == 0);
>  
>          if (words[0][5] == 'b') {
>              uint8_t data = value;
> @@ -395,7 +398,7 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>          uint64_t value = UINT64_C(-1);
>  
>          g_assert(words[1]);
> -        addr = strtoull(words[1], NULL, 0);
> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
>  
>          if (words[0][4] == 'b') {
>              uint8_t data;
> @@ -421,8 +424,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>          char *enc;
>  
>          g_assert(words[1] && words[2]);
> -        addr = strtoull(words[1], NULL, 0);
> -        len = strtoull(words[2], NULL, 0);
> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
> +        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
>  
>          data = g_malloc(len);
>          cpu_physical_memory_read(addr, data, len);
> @@ -443,8 +446,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>          gchar *b64_data;
>  
>          g_assert(words[1] && words[2]);
> -        addr = strtoull(words[1], NULL, 0);
> -        len = strtoull(words[2], NULL, 0);
> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
> +        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
>  
>          data = g_malloc(len);
>          cpu_physical_memory_read(addr, data, len);
> @@ -460,8 +463,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>          size_t data_len;
>  
>          g_assert(words[1] && words[2] && words[3]);
> -        addr = strtoull(words[1], NULL, 0);
> -        len = strtoull(words[2], NULL, 0);
> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
> +        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
>  
>          data_len = strlen(words[3]);
>          if (data_len < 3) {
> @@ -486,12 +489,12 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>      } else if (strcmp(words[0], "memset") == 0) {
>          uint64_t addr, len;
>          uint8_t *data;
> -        uint8_t pattern;
> +        unsigned long pattern;
>  
>          g_assert(words[1] && words[2] && words[3]);
> -        addr = strtoull(words[1], NULL, 0);
> -        len = strtoull(words[2], NULL, 0);
> -        pattern = strtoull(words[3], NULL, 0);
> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
> +        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
> +        g_assert(qemu_strtoul(words[3], NULL, 0, &pattern) == 0);
>  
>          data = g_malloc(len);
>          memset(data, pattern, len);
> @@ -507,8 +510,8 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>          gsize out_len;
>  
>          g_assert(words[1] && words[2] && words[3]);
> -        addr = strtoull(words[1], NULL, 0);
> -        len = strtoull(words[2], NULL, 0);
> +        g_assert(qemu_strtoull(words[1], NULL, 0, &addr) == 0);
> +        g_assert(qemu_strtoull(words[2], NULL, 0, &len) == 0);
>  
>          data_len = strlen(words[3]);
>          if (data_len < 3) {
> @@ -532,7 +535,7 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>          int64_t ns;
>  
>          if (words[1]) {
> -            ns = strtoll(words[1], NULL, 0);
> +            g_assert(qemu_strtoll(words[1], NULL, 0, &ns) == 0);
>          } else {
>              ns = qemu_clock_deadline_ns_all(QEMU_CLOCK_VIRTUAL);
>          }
> @@ -544,7 +547,7 @@ static void qtest_process_command(CharDriverState *chr, gchar **words)
>          int64_t ns;
>  
>          g_assert(words[1]);
> -        ns = strtoll(words[1], NULL, 0);
> +        g_assert(qemu_strtoll(words[1], NULL, 0, &ns) == 0);
>          qtest_clock_warp(ns);
>          qtest_send_prefix(chr);
>          qtest_sendf(chr, "OK %"PRIi64"\n",

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/3] tests: make pc_alloc_init/init_flags/uninit generic
  2016-09-08  7:50     ` Laurent Vivier
@ 2016-09-09 12:25       ` Greg Kurz
  2016-09-09 12:31         ` Laurent Vivier
  0 siblings, 1 reply; 25+ messages in thread
From: Greg Kurz @ 2016-09-09 12:25 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: David Gibson, thuth, qemu-ppc, qemu-devel

On Thu, 8 Sep 2016 09:50:31 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> On 08/09/2016 04:04, David Gibson wrote:
> > On Tue, Sep 06, 2016 at 03:17:56PM +0200, Laurent Vivier wrote:  
> >> And add support for ppc64.
> >>
> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com>  
> > 
> > Some of my coments may be obsoleted by the discussion with Greg.
> >   
> >> ---
> >> v2:
> >> - remove useless parenthesis, inline
> [...]
> >> +
> >> +QGuestAllocator *machine_alloc_init(void)
> >> +{
> >> +    const char *arch = qtest_get_arch();  
> > 
> > Maybe we need to add a qtest_get_machine_type().  
> 
> I'm working on that...
> 

The problem is that qtest only knows about archs, based on $(TARGETS).
Maybe the machine type could be the default one for a given arch ?

Cheers.

--
Greg

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/3] tests: make pc_alloc_init/init_flags/uninit generic
  2016-09-09 12:25       ` Greg Kurz
@ 2016-09-09 12:31         ` Laurent Vivier
  2016-09-12  1:27           ` David Gibson
  0 siblings, 1 reply; 25+ messages in thread
From: Laurent Vivier @ 2016-09-09 12:31 UTC (permalink / raw)
  To: Greg Kurz; +Cc: David Gibson, thuth, qemu-ppc, qemu-devel



On 09/09/2016 14:25, Greg Kurz wrote:
> On Thu, 8 Sep 2016 09:50:31 +0200
> Laurent Vivier <lvivier@redhat.com> wrote:
> 
>> On 08/09/2016 04:04, David Gibson wrote:
>>> On Tue, Sep 06, 2016 at 03:17:56PM +0200, Laurent Vivier wrote:  
>>>> And add support for ppc64.
>>>>
>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>  
>>>
>>> Some of my coments may be obsoleted by the discussion with Greg.
>>>   
>>>> ---
>>>> v2:
>>>> - remove useless parenthesis, inline
>> [...]
>>>> +
>>>> +QGuestAllocator *machine_alloc_init(void)
>>>> +{
>>>> +    const char *arch = qtest_get_arch();  
>>>
>>> Maybe we need to add a qtest_get_machine_type().  
>>
>> I'm working on that...
>>
> 
> The problem is that qtest only knows about archs, based on $(TARGETS).
> Maybe the machine type could be the default one for a given arch ?

Once the machine is started we can use QMP[1] to ask the machine type
(for instance, "pseries-2.7-machine").

So what we could do is a generic qtest_machine_vboot() which ask the
machine type and configure the qtest framework accordingly.

Laurent
[1] { 'execute': 'qom-get', 'arguments': { 'path': '/machine',
'property': 'type' } }

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/3] tests: make pc_alloc_init/init_flags/uninit generic
  2016-09-09 12:31         ` Laurent Vivier
@ 2016-09-12  1:27           ` David Gibson
  2016-09-12  8:27             ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
  2016-09-12 15:46             ` [Qemu-devel] " Laurent Vivier
  0 siblings, 2 replies; 25+ messages in thread
From: David Gibson @ 2016-09-12  1:27 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: Greg Kurz, thuth, qemu-ppc, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1668 bytes --]

On Fri, Sep 09, 2016 at 02:31:55PM +0200, Laurent Vivier wrote:
> 
> 
> On 09/09/2016 14:25, Greg Kurz wrote:
> > On Thu, 8 Sep 2016 09:50:31 +0200
> > Laurent Vivier <lvivier@redhat.com> wrote:
> > 
> >> On 08/09/2016 04:04, David Gibson wrote:
> >>> On Tue, Sep 06, 2016 at 03:17:56PM +0200, Laurent Vivier wrote:  
> >>>> And add support for ppc64.
> >>>>
> >>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>  
> >>>
> >>> Some of my coments may be obsoleted by the discussion with Greg.
> >>>   
> >>>> ---
> >>>> v2:
> >>>> - remove useless parenthesis, inline
> >> [...]
> >>>> +
> >>>> +QGuestAllocator *machine_alloc_init(void)
> >>>> +{
> >>>> +    const char *arch = qtest_get_arch();  
> >>>
> >>> Maybe we need to add a qtest_get_machine_type().  
> >>
> >> I'm working on that...
> >>
> > 
> > The problem is that qtest only knows about archs, based on $(TARGETS).
> > Maybe the machine type could be the default one for a given arch ?
> 
> Once the machine is started we can use QMP[1] to ask the machine type
> (for instance, "pseries-2.7-machine").
> 
> So what we could do is a generic qtest_machine_vboot() which ask the
> machine type and configure the qtest framework accordingly.
> 
> Laurent
> [1] { 'execute': 'qom-get', 'arguments': { 'path': '/machine',
> 'property': 'type' } }

Ok.. doesn't the qtest framework start the machine though?  So it
should already know the machine type, shouldn't it?

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [Qemu-ppc] [PATCH v4 2/3] tests: make pc_alloc_init/init_flags/uninit generic
  2016-09-12  1:27           ` David Gibson
@ 2016-09-12  8:27             ` Greg Kurz
  2016-09-12 15:46             ` [Qemu-devel] " Laurent Vivier
  1 sibling, 0 replies; 25+ messages in thread
From: Greg Kurz @ 2016-09-12  8:27 UTC (permalink / raw)
  To: David Gibson; +Cc: Laurent Vivier, thuth, qemu-ppc, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 2050 bytes --]

On Mon, 12 Sep 2016 11:27:57 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Fri, Sep 09, 2016 at 02:31:55PM +0200, Laurent Vivier wrote:
> > 
> > 
> > On 09/09/2016 14:25, Greg Kurz wrote:  
> > > On Thu, 8 Sep 2016 09:50:31 +0200
> > > Laurent Vivier <lvivier@redhat.com> wrote:
> > >   
> > >> On 08/09/2016 04:04, David Gibson wrote:  
> > >>> On Tue, Sep 06, 2016 at 03:17:56PM +0200, Laurent Vivier wrote:    
> > >>>> And add support for ppc64.
> > >>>>
> > >>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>    
> > >>>
> > >>> Some of my coments may be obsoleted by the discussion with Greg.
> > >>>     
> > >>>> ---
> > >>>> v2:
> > >>>> - remove useless parenthesis, inline  
> > >> [...]  
> > >>>> +
> > >>>> +QGuestAllocator *machine_alloc_init(void)
> > >>>> +{
> > >>>> +    const char *arch = qtest_get_arch();    
> > >>>
> > >>> Maybe we need to add a qtest_get_machine_type().    
> > >>
> > >> I'm working on that...
> > >>  
> > > 
> > > The problem is that qtest only knows about archs, based on $(TARGETS).
> > > Maybe the machine type could be the default one for a given arch ?  
> > 
> > Once the machine is started we can use QMP[1] to ask the machine type
> > (for instance, "pseries-2.7-machine").
> > 
> > So what we could do is a generic qtest_machine_vboot() which ask the
> > machine type and configure the qtest framework accordingly.
> > 
> > Laurent
> > [1] { 'execute': 'qom-get', 'arguments': { 'path': '/machine',
> > 'property': 'type' } }  
> 
> Ok.. doesn't the qtest framework start the machine though?  So it
> should already know the machine type, shouldn't it?
> 

As already mentioned , qtest only knows about archs... the machine type is
either explicitly passed to qtest_start(), either the default one for a
given arch, if any.

For some generic code that would have to choose the right memory allocation
implementation based on the machine type, Laurent's suggestion seems to be
appropriate indeed.

Cheers.

--
Greg

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/3] tests: make pc_alloc_init/init_flags/uninit generic
  2016-09-12  1:27           ` David Gibson
  2016-09-12  8:27             ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
@ 2016-09-12 15:46             ` Laurent Vivier
  2016-09-12 17:37               ` Greg Kurz
  1 sibling, 1 reply; 25+ messages in thread
From: Laurent Vivier @ 2016-09-12 15:46 UTC (permalink / raw)
  To: David Gibson; +Cc: Greg Kurz, thuth, qemu-ppc, qemu-devel



On 12/09/2016 03:27, David Gibson wrote:
> On Fri, Sep 09, 2016 at 02:31:55PM +0200, Laurent Vivier wrote:
>>
>>
>> On 09/09/2016 14:25, Greg Kurz wrote:
>>> On Thu, 8 Sep 2016 09:50:31 +0200
>>> Laurent Vivier <lvivier@redhat.com> wrote:
>>>
>>>> On 08/09/2016 04:04, David Gibson wrote:
>>>>> On Tue, Sep 06, 2016 at 03:17:56PM +0200, Laurent Vivier wrote:  
>>>>>> And add support for ppc64.
>>>>>>
>>>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>  
>>>>>
>>>>> Some of my coments may be obsoleted by the discussion with Greg.
>>>>>   
>>>>>> ---
>>>>>> v2:
>>>>>> - remove useless parenthesis, inline
>>>> [...]
>>>>>> +
>>>>>> +QGuestAllocator *machine_alloc_init(void)
>>>>>> +{
>>>>>> +    const char *arch = qtest_get_arch();  
>>>>>
>>>>> Maybe we need to add a qtest_get_machine_type().  
>>>>
>>>> I'm working on that...
>>>>
>>>
>>> The problem is that qtest only knows about archs, based on $(TARGETS).
>>> Maybe the machine type could be the default one for a given arch ?
>>
>> Once the machine is started we can use QMP[1] to ask the machine type
>> (for instance, "pseries-2.7-machine").
>>
>> So what we could do is a generic qtest_machine_vboot() which ask the
>> machine type and configure the qtest framework accordingly.
>>
>> Laurent
>> [1] { 'execute': 'qom-get', 'arguments': { 'path': '/machine',
>> 'property': 'type' } }
> 
> Ok.. doesn't the qtest framework start the machine though?  So it
> should already know the machine type, shouldn't it?

In fact qtest starts the machine with the content of QTEST_QEMU_BINARY
and we don't provide "-machine" parameter, so it doesn't know the
machine type. It can guess it according to the machine arch, and by
default, ppc64 (arch) is pseries (machine).

Perhaps we can add "-machine pseries" in qtest_spapr_vboot().

Laurent
Laurent

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/3] tests: make pc_alloc_init/init_flags/uninit generic
  2016-09-12 15:46             ` [Qemu-devel] " Laurent Vivier
@ 2016-09-12 17:37               ` Greg Kurz
  0 siblings, 0 replies; 25+ messages in thread
From: Greg Kurz @ 2016-09-12 17:37 UTC (permalink / raw)
  To: Laurent Vivier; +Cc: David Gibson, thuth, qemu-ppc, qemu-devel

On Mon, 12 Sep 2016 17:46:35 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> On 12/09/2016 03:27, David Gibson wrote:
> > On Fri, Sep 09, 2016 at 02:31:55PM +0200, Laurent Vivier wrote:  
> >>
> >>
> >> On 09/09/2016 14:25, Greg Kurz wrote:  
> >>> On Thu, 8 Sep 2016 09:50:31 +0200
> >>> Laurent Vivier <lvivier@redhat.com> wrote:
> >>>  
> >>>> On 08/09/2016 04:04, David Gibson wrote:  
> >>>>> On Tue, Sep 06, 2016 at 03:17:56PM +0200, Laurent Vivier wrote:    
> >>>>>> And add support for ppc64.
> >>>>>>
> >>>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>    
> >>>>>
> >>>>> Some of my coments may be obsoleted by the discussion with Greg.
> >>>>>     
> >>>>>> ---
> >>>>>> v2:
> >>>>>> - remove useless parenthesis, inline  
> >>>> [...]  
> >>>>>> +
> >>>>>> +QGuestAllocator *machine_alloc_init(void)
> >>>>>> +{
> >>>>>> +    const char *arch = qtest_get_arch();    
> >>>>>
> >>>>> Maybe we need to add a qtest_get_machine_type().    
> >>>>
> >>>> I'm working on that...
> >>>>  
> >>>
> >>> The problem is that qtest only knows about archs, based on $(TARGETS).
> >>> Maybe the machine type could be the default one for a given arch ?  
> >>
> >> Once the machine is started we can use QMP[1] to ask the machine type
> >> (for instance, "pseries-2.7-machine").
> >>
> >> So what we could do is a generic qtest_machine_vboot() which ask the
> >> machine type and configure the qtest framework accordingly.
> >>
> >> Laurent
> >> [1] { 'execute': 'qom-get', 'arguments': { 'path': '/machine',
> >> 'property': 'type' } }  
> > 
> > Ok.. doesn't the qtest framework start the machine though?  So it
> > should already know the machine type, shouldn't it?  
> 
> In fact qtest starts the machine with the content of QTEST_QEMU_BINARY
> and we don't provide "-machine" parameter, so it doesn't know the
> machine type. It can guess it according to the machine arch, and by
> default, ppc64 (arch) is pseries (machine).
> 
> Perhaps we can add "-machine pseries" in qtest_spapr_vboot().
> 

We may want to test something that is only available for a given
version (like CPU hotplug for pseries >= 2.7 for example)... it
could *work* if we can pass "-machine pseries -machine pseries-2.7"
and the latter prevails, but that looks a bit like a hack.

Cheers.

--
Greg

> Laurent
> Laurent

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2016-09-12 17:37 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-06 13:17 [Qemu-devel] [PATCH v4 0/3] tests: add RTAS protocol Laurent Vivier
2016-09-06 13:17 ` [Qemu-devel] [PATCH v4 1/3] qtest: replace strtoXX() by qemu_strtoXX() Laurent Vivier
2016-09-06 15:55   ` Greg Kurz
2016-09-06 18:17     ` Laurent Vivier
2016-09-06 21:13       ` Greg Kurz
2016-09-08  1:56   ` David Gibson
2016-09-08  7:55   ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
2016-09-06 13:17 ` [Qemu-devel] [PATCH v4 2/3] tests: make pc_alloc_init/init_flags/uninit generic Laurent Vivier
2016-09-06 19:46   ` Thomas Huth
2016-09-06 21:41   ` Greg Kurz
2016-09-07  9:36     ` Laurent Vivier
2016-09-07 12:44       ` Greg Kurz
2016-09-07 13:10         ` Laurent Vivier
2016-09-07 13:42           ` Greg Kurz
2016-09-08  2:04   ` David Gibson
2016-09-08  7:50     ` Laurent Vivier
2016-09-09 12:25       ` Greg Kurz
2016-09-09 12:31         ` Laurent Vivier
2016-09-12  1:27           ` David Gibson
2016-09-12  8:27             ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
2016-09-12 15:46             ` [Qemu-devel] " Laurent Vivier
2016-09-12 17:37               ` Greg Kurz
2016-09-06 13:17 ` [Qemu-devel] [PATCH v4 3/3] tests: add RTAS command in the protocol Laurent Vivier
2016-09-06 22:07   ` Greg Kurz
2016-09-07  9:25     ` Laurent Vivier

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.