[Qemu-devel] [PATCH 0/2] tests: add RTAS protocol

[Qemu-devel] [PATCH 0/2] tests: add RTAS protocol

[Laurent Vivier]

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".

Laurent Vivier (2):
  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                     | 17 +++++++++++
 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           | 43 +++++++++++++++++++++++++++
 14 files changed, 302 insertions(+), 2 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

[Qemu-devel] [PATCH 1/2] tests: make pc_alloc_init/init_flags/uninit generic

[Laurent Vivier]

And add support for ppc64.

Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 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..33fd33f
--- /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;
+}
+
+inline 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

[Qemu-devel] [PATCH 2/2] tests: add RTAS command in the protocol

[Laurent Vivier]

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>
---
 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           | 43 +++++++++++++++++++++++++++
 9 files changed, 199 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 da4826c..9ef40f5 100644
--- a/qtest.c
+++ b/qtest.c
@@ -27,6 +27,9 @@
 #include "qemu/config-file.h"
 #include "qemu/option.h"
 #include "qemu/error-report.h"
+#ifdef TARGET_PPC64
+#include "hw/ppc/spapr_rtas.h"
+#endif
 
 #define MAX_IRQ 256
 
@@ -528,6 +531,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;
+        uint32_t nargs, nret;
+
+        nargs = strtol(words[2], NULL, 0);
+        args = strtoll(words[3], NULL, 0);
+        nret = strtol(words[4], NULL, 0);
+        ret = strtoll(words[5], NULL, 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..47a664e
--- /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..1ccf57f
--- /dev/null
+++ b/tests/rtas-test.c
@@ -0,0 +1,43 @@
+#include <time.h>
+
+#include "qemu/osdep.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 = timegm(&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

Re: [Qemu-devel] [PATCH 0/2] tests: add RTAS protocol

[no-reply]

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH 0/2] tests: add RTAS protocol
Type: series
Message-id: 1473086312-4154-1-git-send-email-lvivier@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git show --no-patch --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/1473086312-4154-1-git-send-email-lvivier@redhat.com -> patchew/1473086312-4154-1-git-send-email-lvivier@redhat.com
Switched to a new branch 'test'
7c38bd9 tests: add RTAS command in the protocol
cf872f9 tests: make pc_alloc_init/init_flags/uninit generic

=== OUTPUT BEGIN ===
Checking PATCH 1/2: tests: make pc_alloc_init/init_flags/uninit generic...
Checking PATCH 2/2: tests: add RTAS command in the protocol...
ERROR: consider using qemu_strtol in preference to strtol
#90: FILE: qtest.c:539:
+        nargs = strtol(words[2], NULL, 0);

ERROR: consider using qemu_strtoll in preference to strtoll
#91: FILE: qtest.c:540:
+        args = strtoll(words[3], NULL, 0);

ERROR: consider using qemu_strtol in preference to strtol
#92: FILE: qtest.c:541:
+        nret = strtol(words[4], NULL, 0);

ERROR: consider using qemu_strtoll in preference to strtoll
#93: FILE: qtest.c:542:
+        ret = strtoll(words[5], NULL, 0);

ERROR: space required after that ',' (ctx:VxV)
#164: FILE: tests/libqos/rtas.c:29:
+static uint64_t qrtas_call(QGuestAllocator *alloc,const char *name,
                                                  ^

total: 5 errors, 0 warnings, 253 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

Re: [Qemu-devel] [PATCH 1/2] tests: make pc_alloc_init/init_flags/uninit generic

[Thomas Huth]

On 05.09.2016 16:38, Laurent Vivier wrote:
> And add support for ppc64.
> 
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  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

Maybe this could rather be put into a libqos-malloc-obj-y variable which
is then used to link these only to the tests that really need them?

>  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..33fd33f
> --- /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)

No need for parenthesis here.

> +/* 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;
> +}
> +
> +inline QGuestAllocator *ppc64_alloc_init(void)
> +{
> +    return ppc64_alloc_init_flags(ALLOC_NO_FLAGS);
> +}

Why is this function marked as inline? ... it is the last function in
the source file...

Apart from that, the patch looks fine to me.

 Thomas

Re: [Qemu-devel] [PATCH 1/2] tests: make pc_alloc_init/init_flags/uninit generic

[Laurent Vivier]

On 05/09/2016 18:00, Thomas Huth wrote:
> On 05.09.2016 16:38, Laurent Vivier wrote:
>> And add support for ppc64.
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>>  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
> 
> Maybe this could rather be put into a libqos-malloc-obj-y variable which
> is then used to link these only to the tests that really need them?

Good idea.

> 
>>  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..33fd33f
>> --- /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)
> 
> No need for parenthesis here.

You really don't like parenthesis...

> 
>> +/* 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;
>> +}
>> +
>> +inline QGuestAllocator *ppc64_alloc_init(void)
>> +{
>> +    return ppc64_alloc_init_flags(ALLOC_NO_FLAGS);
>> +}
> 
> Why is this function marked as inline? ... it is the last function in
> the source file...

stupidly copied from tests/libqos/malloc-generic.c

> 
> Apart from that, the patch looks fine to me.


Thanks,
Laurent

Re: [Qemu-devel] [PATCH 1/2] tests: make pc_alloc_init/init_flags/uninit generic

[Thomas Huth]

On 05.09.2016 18:48, Laurent Vivier wrote:
> 
> 
> On 05/09/2016 18:00, Thomas Huth wrote:
>> On 05.09.2016 16:38, Laurent Vivier wrote:
>>> And add support for ppc64.
>>>
>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>> ---
>>>  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
>>
>> Maybe this could rather be put into a libqos-malloc-obj-y variable which
>> is then used to link these only to the tests that really need them?
> 
> Good idea.

Thinking about this again, this maybe makes the Makefile more complex
than it has to be ... so maybe it's better to keep it the way you did it
in this patch. ... your choice!

 Thomas