From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41164) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bjIhb-00083F-Hx for qemu-devel@nongnu.org; Mon, 12 Sep 2016 00:18:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bjIhY-000334-8p for qemu-devel@nongnu.org; Mon, 12 Sep 2016 00:18:39 -0400 Date: Mon, 12 Sep 2016 14:17:13 +1000 From: David Gibson Message-ID: <20160912041713.GC15077@voom.fritz.box> References: <1473361207-28498-1-git-send-email-lvivier@redhat.com> <1473361207-28498-4-git-send-email-lvivier@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="yVhtmJPUSI46BTXb" Content-Disposition: inline In-Reply-To: <1473361207-28498-4-git-send-email-lvivier@redhat.com> Subject: Re: [Qemu-devel] [PATCH v6 3/3] tests: add RTAS command in the protocol List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laurent Vivier Cc: thuth@redhat.com, qemu-ppc@nongnu.org, qemu-devel@nongnu.org, groug@kaod.org --yVhtmJPUSI46BTXb Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Sep 08, 2016 at 09:00:07PM +0200, Laurent Vivier wrote: > Add a first test to validate the protocol: >=20 > - rtas/get-time-of-day compares the time > from the guest with the time from the host. >=20 > Signed-off-by: Laurent Vivier > --- > v6: > - rebase >=20 > v5: > - use qtest_spapr_boot() instead of machine_alloc_init() >=20 > v4: > - use qemu_strtoXXX() instead strtoXX() >=20 > v3: > - use mktimegm() instead of timegm() >=20 > v2: > - add a missing space in qrtas_call() prototype >=20 > 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 | 40 +++++++++++++++++++++++++ > 9 files changed, 196 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 >=20 > 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 @@ > =20 > #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, sPAPRM= achineState *spapr, > return H_PARAMETER; > } > =20 > +uint64_t qtest_rtas_call(char *cmd, uint32_t nargs, uint64_t args, > + uint32_t nret, uint64_t rets) > +{ > + int token; > + > + for (token =3D 0; token < RTAS_TOKEN_MAX - RTAS_TOKEN_BASE; token++)= { > + if (strcmp(cmd, rtas_table[token].name) =3D=3D 0) { > + sPAPRMachineState *spapr =3D SPAPR_MACHINE(qdev_get_machine(= )); > + PowerPCCPU *cpu =3D POWERPC_CPU(first_cpu); > + > + rtas_table[token].fn(cpu, spapr, token + RTAS_TOKEN_BASE, > + nargs, args, nret, rets); > + return H_SUCCESS; > + } > + } > + return H_PARAMETER; > +} I may be misunderstanding how qtest works here. But wouldn't it test a bit more of the common code paths if rather than doing your own token lookup here, you actually generated a full guest style rtas parameter buffer (token + args + rets in one structure), then dispatch through h_rtas, or spapr_rtas_call? > + > void spapr_rtas_register(int token, const char *name, spapr_rtas_fn fn) > { > assert((token >=3D 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 la= ter. > + * 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 > =20 > #define MAX_IRQ 256 > =20 > @@ -531,6 +534,20 @@ static void qtest_process_command(CharDriverState *c= hr, gchar **words) > =20 > qtest_send_prefix(chr); > qtest_send(chr, "OK\n"); > +#ifdef TARGET_PPC64 > + } else if (strcmp(words[0], "rtas") =3D=3D 0) { > + uint64_t res, args, ret; > + unsigned long nargs, nret; > + > + g_assert(qemu_strtoul(words[2], NULL, 0, &nargs) =3D=3D 0); > + g_assert(qemu_strtoull(words[3], NULL, 0, &args) =3D=3D 0); > + g_assert(qemu_strtoul(words[4], NULL, 0, &nret) =3D=3D 0); > + g_assert(qemu_strtoull(words[5], NULL, 0, &ret) =3D=3D 0); > + res =3D 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") =3D=3D = 0) { > int64_t ns; > =20 > 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 +=3D tests/prom-env-test$(EXESUF) > check-qtest-ppc64-y +=3D tests/drive_del-test$(EXESUF) > check-qtest-ppc64-y +=3D tests/postcopy-test$(EXESUF) > check-qtest-ppc64-y +=3D tests/boot-serial-test$(EXESUF) > +check-qtest-ppc64-y +=3D tests/rtas-test$(EXESUF) > =20 > check-qtest-sh4-y =3D tests/endianness-test$(EXESUF) > =20 > @@ -578,6 +579,7 @@ libqos-obj-y =3D tests/libqos/pci.o tests/libqos/fw_c= fg.o tests/libqos/malloc.o > libqos-obj-y +=3D tests/libqos/i2c.o tests/libqos/libqos.o > libqos-spapr-obj-y =3D $(libqos-obj-y) tests/libqos/malloc-spapr.o > libqos-spapr-obj-y +=3D tests/libqos/libqos-spapr.o > +libqos-spapr-obj-y +=3D tests/libqos/rtas.o > libqos-pc-obj-y =3D $(libqos-obj-y) tests/libqos/pci-pc.o > libqos-pc-obj-y +=3D tests/libqos/malloc-pc.o tests/libqos/libqos-pc.o > libqos-pc-obj-y +=3D 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..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 la= ter. > + * 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 =3D 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 =3D 0; i < nret; i++) { > + ret[i] =3D 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 =3D guest_alloc(alloc, (nargs + nret) * sizeof(uint32_t)= ); > + target_ret =3D guest_alloc(alloc, nret * sizeof(uint32_t)); Again, possibly I'm misunderstanding something, but it looks like you're over-allocating here - target_args seems to get enough space for both args and rets, then target_ret gets additional space for the rets as well. > + qrtas_copy_args(target_args, nargs, args); > + res =3D 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 =3D qrtas_call(alloc, "get-time-of-day", 0, NULL, 8, ret); > + if (res !=3D 0) { > + return res; > + } > + > + res =3D ret[0]; > + memset(tm, 0, sizeof(*tm)); > + tm->tm_year =3D ret[1] - 1900; > + tm->tm_mon =3D ret[2] - 1; > + tm->tm_mday =3D ret[3]; > + tm->tm_hour =3D ret[4]; > + tm->tm_min =3D ret[5]; > + tm->tm_sec =3D ret[6]; > + *ns =3D 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 la= ter. > + * 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, voi= d *data, size_t size) > g_strfreev(args); > } > =20 > +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 =3D 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= ); > =20 > /** > + * 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..3bca36b > --- /dev/null > +++ b/tests/rtas-test.c > @@ -0,0 +1,40 @@ > +#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 =3D qtest_spapr_boot(""); > + > + t1 =3D time(NULL); > + ret =3D qrtas_get_time_of_day(qs->alloc, &tm, &ns); > + g_assert_cmpint(ret, =3D=3D, 0); > + t2 =3D 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 =3D qtest_get_arch(); > + > + g_test_init(&argc, &argv, NULL); > + > + if (strcmp(arch, "ppc64") =3D=3D 0) { > + qtest_add_func("rtas/get-time-of-day", test_rtas_get_time_of_day= ); > + } else { > + g_assert_not_reached(); > + } > + > + return g_test_run(); > +} --=20 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 --yVhtmJPUSI46BTXb Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIcBAEBAgAGBQJX1ixJAAoJEGw4ysog2bOSAZkP/0u5cXc/iMJOtVSC1UPXQ1HA 2a1ZstrW8A1g3xFyRAjeOHeiyPiGRXj9BjqVc/JqaONsvZisF36I68Xhzgca8T8T YeY9wYvUXouK//RTEFDFL/gwTc6zVKO/PAjZayC3khqoGmcqoYnOXvaAU2JL6HuG 2+WEVPCa00/tmyPTFB0UjenVm1Za10lI4O3sK1siVqTe5oJIZ4OaaZNznR6mkNUo U+MGG9p4ZhBrDxyc1Ii3kmbu5UxaEcgk8j9s5MENO5RyQf9VvAuqO+8AWAMC8XEY IKviI9DH1uvhxgK3x1WgLllIYmW8VCqOZXbSxg4naokyvKGEQz3zh/G61QDIhSur GytkJ3HcijU0NuQU7c38rAzc8ceoeXiSptP+Yx0+mkHl+K8Ddp4ppAIBlzLx1AJV f1CDDfzNTv79tOdKBn5CSae7L3UrGrOnBWdqD3Uep7UDEDEQBFwziAg3TgicleT+ h97YLGPxEZ+7ZMgjdUibqwK5bV7n8N+YeKSkjyOMBuLvCjmfjRQ4U6pz9y/DxyTS 9naDmXhitYu9a80zyu2fa9EKrNTsPM7Ml0OxrCvKKW1gzzJcuNPucxRNayxdVCmv dkEfIqYAIscDb7nCf5mPVy7E6CgR/l1TNGbpZreuJi61PdpHYmHubW4tyheEgaU2 e6MdwpxzHoJCT0FAlvvR =IRHy -----END PGP SIGNATURE----- --yVhtmJPUSI46BTXb--