[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH v8 3/3] tests: add RTAS command in the protocol
|
From: |
Greg Kurz |
|
Subject: |
Re: [Qemu-ppc] [PATCH v8 3/3] tests: add RTAS command in the protocol |
|
Date: |
Wed, 14 Sep 2016 11:05:47 +0200 |
On Wed, 14 Sep 2016 13:22:40 +1000
David Gibson <address@hidden> wrote:
> On Tue, Sep 13, 2016 at 02:52:45PM +0200, Laurent Vivier 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 <address@hidden>
> > Reviewed-by: Greg Kurz <address@hidden>
>
> Reviewed-by: David Gibson <address@hidden>
>
> > ---
> > v8:
> > - add Greg's Rb
> > - exit if RTAS is not started with qemu-system-ppc64
> >
> > v7:
> > - don't allocate twice the memory for the RTAS call
> > - add "-machine pseries" in rtas-test
> >
> > v6:
> > - rebase
> >
> > v5:
> > - use qtest_spapr_boot() instead of machine_alloc_init()
> >
> > 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 | 41 ++++++++++++++++++++++++++
> > 9 files changed, 197 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 27b5ad4..b80c1db 100644
> > --- a/hw/ppc/spapr_rtas.c
> > +++ b/hw/ppc/spapr_rtas.c
> > @@ -37,6 +37,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"
> > @@ -692,6 +693,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());
>
> As a sanity check you could return an error if you're unable to
> resolve the machine as an sPAPR here. Since that should only happen
> on a broken testcase, it can be done as a later enhancement, though.
>
Hmm... if the machine isn't sPAPR, SPAPR_MACHINE() will abort, which makes
sense in the case of a broken testcase.
hw/ppc/spapr_rtas.c:703:qtest_rtas_call: Object 0x558f5de65590 is not an
instance of type spapr-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 649f7b2..22482cc 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
> >
> > @@ -534,6 +537,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 91df9f2..fd61f97 100644
> > --- a/tests/Makefile.include
> > +++ b/tests/Makefile.include
> > @@ -265,6 +265,7 @@ check-qtest-ppc64-y += tests/prom-env-test$(EXESUF)
> > check-qtest-ppc64-y += tests/drive_del-test$(EXESUF)
> > check-qtest-ppc64-y += tests/postcopy-test$(EXESUF)
> > check-qtest-ppc64-y += tests/boot-serial-test$(EXESUF)
> > +check-qtest-ppc64-y += tests/rtas-test$(EXESUF)
> >
> > check-qtest-sh4-y = tests/endianness-test$(EXESUF)
> >
> > @@ -578,6 +579,7 @@ 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-spapr-obj-y = $(libqos-obj-y) tests/libqos/malloc-spapr.o
> > libqos-spapr-obj-y += tests/libqos/libqos-spapr.o
> > +libqos-spapr-obj-y += tests/libqos/rtas.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/ahci.o
> > @@ -592,6 +594,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-spapr-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..820321a
> > --- /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 * 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;
> > +}
>
> This might be slightly more robust if the args were put into an actual
> rtas args buffer (with the token not populated), rather than separate
> args and rets array. Again, that can be a later improvement though.
>
> > +
> > +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 42ccb62..6f6bdf1 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 d2b4853..f7402e0 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..73c7803
> > --- /dev/null
> > +++ b/tests/rtas-test.c
> > @@ -0,0 +1,41 @@
> > +#include "qemu/osdep.h"
> > +#include "qemu/cutils.h"
> > +#include "libqtest.h"
> > +
> > +#include "libqos/libqos-spapr.h"
> > +#include "libqos/rtas.h"
> > +
> > +static void test_rtas_get_time_of_day(void)
> > +{
> > + QOSState *qs;
> > + struct tm tm;
> > + uint32_t ns;
> > + uint64_t ret;
> > + time_t t1, t2;
> > +
> > + qs = qtest_spapr_boot("-machine pseries");
> > + g_assert(qs != NULL);
> > +
> > + t1 = time(NULL);
> > + ret = qrtas_get_time_of_day(qs->alloc, &tm, &ns);
> > + g_assert_cmpint(ret, ==, 0);
> > + t2 = mktimegm(&tm);
> > + g_assert(t2 - t1 < 5); /* 5 sec max to run the test */
> > +
> > + qtest_spapr_shutdown(qs);
> > +}
> > +
> > +int main(int argc, char *argv[])
> > +{
> > + const char *arch = qtest_get_arch();
> > +
> > + g_test_init(&argc, &argv, NULL);
> > +
> > + if (strcmp(arch, "ppc64")) {
> > + g_printerr("RTAS requires ppc64-softmmu/qemu-system-ppc64\n");
> > + exit(EXIT_FAILURE);
> > + }
> > + qtest_add_func("rtas/get-time-of-day", test_rtas_get_time_of_day);
> > +
> > + return g_test_run();
> > +}
>
pgpbq0fR99w6e.pgp
Description: OpenPGP digital signature