From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45113) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bsPN2-0007RB-9d for qemu-devel@nongnu.org; Fri, 07 Oct 2016 03:15:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bsPMy-0007Gq-G1 for qemu-devel@nongnu.org; Fri, 07 Oct 2016 03:15:03 -0400 Received: from 15.mo6.mail-out.ovh.net ([188.165.39.161]:34904) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bsPMy-0007Ge-5C for qemu-devel@nongnu.org; Fri, 07 Oct 2016 03:15:00 -0400 Received: from player738.ha.ovh.net (b7.ovh.net [213.186.33.57]) by mo6.mail-out.ovh.net (Postfix) with ESMTP id 296EC35091 for ; Fri, 7 Oct 2016 09:14:59 +0200 (CEST) Date: Fri, 7 Oct 2016 09:14:51 +0200 From: Greg Kurz Message-ID: <20161007091451.396127cc@bahia> In-Reply-To: <20161006235529.GI18490@umbus.fritz.box> References: <1475780218-26393-1-git-send-email-lvivier@redhat.com> <20161006224622.516cee20@bahia> <20161006235529.GI18490@umbus.fritz.box> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; boundary="Sig_/vlhKPNm1PGP9PJb1go6mw5."; protocol="application/pgp-signature" Subject: Re: [Qemu-devel] [PATCH] qtest: ask endianness of the target in qtest_init() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: Laurent Vivier , qemu-devel@nongnu.org, Peter Maydell --Sig_/vlhKPNm1PGP9PJb1go6mw5. Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable On Fri, 7 Oct 2016 10:55:29 +1100 David Gibson wrote: > On Thu, Oct 06, 2016 at 10:46:22PM +0200, Greg Kurz wrote: > > On Thu, 6 Oct 2016 20:56:58 +0200 > > Laurent Vivier wrote: > > =20 > > > The target endianness is not deduced anymore from > > > the architecture name but asked directly to the guest, > > > using a new qtest command: "endianness". As it can't > > > change (this is the value of TARGET_WORDS_BIGENDIAN), > > > we store it to not have to ask every time we want to > > > know if we have to byte-swap a value. > > >=20 > > > Signed-off-by: Laurent Vivier > > > CC: Greg Kurz > > > CC: David Gibson > > > CC: Peter Maydell > > > --- > > > Note: this patch can be seen as a v2 of > > > "qtest: evaluate endianness of the target in qtest_init()" > > > from the patch series "tests: enable virtio tests on SPAPR" > > > in which I have added the idea from Peter to ask the endianness > > > directly to the guest using a new qtest command. > > > =20 > >=20 > > This is definitely an improvement indeed. > >=20 > > Reviewed-by: Greg Kurz =20 >=20 > It is an improvement. But I still think if we're relying on the > ill-defined "target endianness" we're already doing something wrong. >=20 Just to be sure, you're talking about the qtest case only or about the use of TARGET_WORDS_BIGENDIAN in QEMU itself ? > > =20 > > > qtest.c | 7 ++ > > > tests/libqos/virtio-pci.c | 2 +- > > > tests/libqtest.c | 224 ++++++++++++++++++++----------------= ---------- > > > tests/libqtest.h | 16 +++- > > > tests/virtio-blk-test.c | 2 +- > > > 5 files changed, 118 insertions(+), 133 deletions(-) > > >=20 > > > diff --git a/qtest.c b/qtest.c > > > index 22482cc..b53b39c 100644 > > > --- a/qtest.c > > > +++ b/qtest.c > > > @@ -537,6 +537,13 @@ static void qtest_process_command(CharDriverStat= e *chr, gchar **words) > > > =20 > > > qtest_send_prefix(chr); > > > qtest_send(chr, "OK\n"); > > > + } else if (strcmp(words[0], "endianness") =3D=3D 0) { > > > + qtest_send_prefix(chr); > > > +#if defined(TARGET_WORDS_BIGENDIAN) > > > + qtest_sendf(chr, "OK big\n"); > > > +#else > > > + qtest_sendf(chr, "OK little\n"); > > > +#endif > > > #ifdef TARGET_PPC64 > > > } else if (strcmp(words[0], "rtas") =3D=3D 0) { > > > uint64_t res, args, ret; > > > diff --git a/tests/libqos/virtio-pci.c b/tests/libqos/virtio-pci.c > > > index 18b92b9..6e005c1 100644 > > > --- a/tests/libqos/virtio-pci.c > > > +++ b/tests/libqos/virtio-pci.c > > > @@ -86,7 +86,7 @@ static uint64_t qvirtio_pci_config_readq(QVirtioDev= ice *d, uint64_t addr) > > > int i; > > > uint64_t u64 =3D 0; > > > =20 > > > - if (qtest_big_endian()) { > > > + if (target_big_endian()) { > > > for (i =3D 0; i < 8; ++i) { > > > u64 |=3D (uint64_t)qpci_io_readb(dev->pdev, > > > (void *)(uintptr_t)addr + i) << (7 -= i) * 8; > > > diff --git a/tests/libqtest.c b/tests/libqtest.c > > > index 6f6bdf1..27cf6b1 100644 > > > --- a/tests/libqtest.c > > > +++ b/tests/libqtest.c > > > @@ -37,6 +37,7 @@ struct QTestState > > > bool irq_level[MAX_IRQ]; > > > GString *rx; > > > pid_t qemu_pid; /* our child QEMU process */ > > > + bool big_endian; > > > }; > > > =20 > > > static GHookList abrt_hooks; > > > @@ -146,89 +147,6 @@ void qtest_add_abrt_handler(GHookFunc fn, const = void *data) > > > g_hook_prepend(&abrt_hooks, hook); > > > } > > > =20 > > > -QTestState *qtest_init(const char *extra_args) > > > -{ > > > - QTestState *s; > > > - int sock, qmpsock, i; > > > - gchar *socket_path; > > > - gchar *qmp_socket_path; > > > - gchar *command; > > > - const char *qemu_binary; > > > - > > > - qemu_binary =3D getenv("QTEST_QEMU_BINARY"); > > > - g_assert(qemu_binary !=3D NULL); > > > - > > > - s =3D g_malloc(sizeof(*s)); > > > - > > > - socket_path =3D g_strdup_printf("/tmp/qtest-%d.sock", getpid()); > > > - qmp_socket_path =3D g_strdup_printf("/tmp/qtest-%d.qmp", getpid(= )); > > > - > > > - sock =3D init_socket(socket_path); > > > - qmpsock =3D init_socket(qmp_socket_path); > > > - > > > - qtest_add_abrt_handler(kill_qemu_hook_func, s); > > > - > > > - s->qemu_pid =3D fork(); > > > - if (s->qemu_pid =3D=3D 0) { > > > - setenv("QEMU_AUDIO_DRV", "none", true); > > > - command =3D g_strdup_printf("exec %s " > > > - "-qtest unix:%s,nowait " > > > - "-qtest-log %s " > > > - "-qmp unix:%s,nowait " > > > - "-machine accel=3Dqtest " > > > - "-display none " > > > - "%s", qemu_binary, socket_path, > > > - getenv("QTEST_LOG") ? "/dev/fd/2" = : "/dev/null", > > > - qmp_socket_path, > > > - extra_args ?: ""); > > > - execlp("/bin/sh", "sh", "-c", command, NULL); > > > - exit(1); > > > - } > > > - > > > - s->fd =3D socket_accept(sock); > > > - if (s->fd >=3D 0) { > > > - s->qmp_fd =3D socket_accept(qmpsock); > > > - } > > > - unlink(socket_path); > > > - unlink(qmp_socket_path); > > > - g_free(socket_path); > > > - g_free(qmp_socket_path); > > > - > > > - g_assert(s->fd >=3D 0 && s->qmp_fd >=3D 0); > > > - > > > - s->rx =3D g_string_new(""); > > > - for (i =3D 0; i < MAX_IRQ; i++) { > > > - s->irq_level[i] =3D false; > > > - } > > > - > > > - /* Read the QMP greeting and then do the handshake */ > > > - qtest_qmp_discard_response(s, ""); > > > - qtest_qmp_discard_response(s, "{ 'execute': 'qmp_capabilities' }= "); > > > - > > > - if (getenv("QTEST_STOP")) { > > > - kill(s->qemu_pid, SIGSTOP); > > > - } > > > - > > > - return s; > > > -} > > > - > > > -void qtest_quit(QTestState *s) > > > -{ > > > - qtest_instances =3D g_list_remove(qtest_instances, s); > > > - g_hook_destroy_link(&abrt_hooks, g_hook_find_data(&abrt_hooks, T= RUE, s)); > > > - > > > - /* Uninstall SIGABRT handler on last instance */ > > > - if (!qtest_instances) { > > > - cleanup_sigabrt_handler(); > > > - } > > > - > > > - kill_qemu(s); > > > - close(s->fd); > > > - close(s->qmp_fd); > > > - g_string_free(s->rx, true); > > > - g_free(s); > > > -} > > > - > > > static void socket_send(int fd, const char *buf, size_t size) > > > { > > > size_t offset; > > > @@ -347,6 +265,99 @@ typedef struct { > > > QDict *response; > > > } QMPResponseParser; > > > =20 > > > +QTestState *qtest_init(const char *extra_args) > > > +{ > > > + QTestState *s; > > > + int sock, qmpsock, i; > > > + gchar *socket_path; > > > + gchar *qmp_socket_path; > > > + gchar *command; > > > + const char *qemu_binary; > > > + gchar **args; > > > + > > > + qemu_binary =3D getenv("QTEST_QEMU_BINARY"); > > > + g_assert(qemu_binary !=3D NULL); > > > + > > > + s =3D g_malloc(sizeof(*s)); > > > + > > > + socket_path =3D g_strdup_printf("/tmp/qtest-%d.sock", getpid()); > > > + qmp_socket_path =3D g_strdup_printf("/tmp/qtest-%d.qmp", getpid(= )); > > > + > > > + sock =3D init_socket(socket_path); > > > + qmpsock =3D init_socket(qmp_socket_path); > > > + > > > + qtest_add_abrt_handler(kill_qemu_hook_func, s); > > > + > > > + s->qemu_pid =3D fork(); > > > + if (s->qemu_pid =3D=3D 0) { > > > + setenv("QEMU_AUDIO_DRV", "none", true); > > > + command =3D g_strdup_printf("exec %s " > > > + "-qtest unix:%s,nowait " > > > + "-qtest-log %s " > > > + "-qmp unix:%s,nowait " > > > + "-machine accel=3Dqtest " > > > + "-display none " > > > + "%s", qemu_binary, socket_path, > > > + getenv("QTEST_LOG") ? "/dev/fd/2" > > > + : "/dev/null", > > > + qmp_socket_path, > > > + extra_args ?: ""); > > > + execlp("/bin/sh", "sh", "-c", command, NULL); > > > + exit(1); > > > + } > > > + > > > + s->fd =3D socket_accept(sock); > > > + if (s->fd >=3D 0) { > > > + s->qmp_fd =3D socket_accept(qmpsock); > > > + } > > > + unlink(socket_path); > > > + unlink(qmp_socket_path); > > > + g_free(socket_path); > > > + g_free(qmp_socket_path); > > > + > > > + g_assert(s->fd >=3D 0 && s->qmp_fd >=3D 0); > > > + > > > + s->rx =3D g_string_new(""); > > > + for (i =3D 0; i < MAX_IRQ; i++) { > > > + s->irq_level[i] =3D false; > > > + } > > > + > > > + /* Read the QMP greeting and then do the handshake */ > > > + qtest_qmp_discard_response(s, ""); > > > + qtest_qmp_discard_response(s, "{ 'execute': 'qmp_capabilities' }= "); > > > + > > > + if (getenv("QTEST_STOP")) { > > > + kill(s->qemu_pid, SIGSTOP); > > > + } > > > + > > > + /* ask endianness of the target */ > > > + > > > + qtest_sendf(s, "endianness\n"); > > > + args =3D qtest_rsp(s, 1); > > > + g_assert(strcmp(args[1], "big") =3D=3D 0 || strcmp(args[1], "lit= tle") =3D=3D 0); > > > + s->big_endian =3D strcmp(args[1], "big") =3D=3D 0; > > > + g_strfreev(args); > > > + > > > + return s; > > > +} > > > + > > > +void qtest_quit(QTestState *s) > > > +{ > > > + qtest_instances =3D g_list_remove(qtest_instances, s); > > > + g_hook_destroy_link(&abrt_hooks, g_hook_find_data(&abrt_hooks, T= RUE, s)); > > > + > > > + /* Uninstall SIGABRT handler on last instance */ > > > + if (!qtest_instances) { > > > + cleanup_sigabrt_handler(); > > > + } > > > + > > > + kill_qemu(s); > > > + close(s->fd); > > > + close(s->qmp_fd); > > > + g_string_free(s->rx, true); > > > + g_free(s); > > > +} > > > + > > > static void qmp_response(JSONMessageParser *parser, GQueue *tokens) > > > { > > > QMPResponseParser *qmp =3D container_of(parser, QMPResponseParse= r, parser); > > > @@ -886,50 +897,7 @@ char *hmp(const char *fmt, ...) > > > return ret; > > > } > > > =20 > > > -bool qtest_big_endian(void) > > > +bool qtest_big_endian(QTestState *s) > > > { > > > - const char *arch =3D qtest_get_arch(); > > > - int i; > > > - > > > - static const struct { > > > - const char *arch; > > > - bool big_endian; > > > - } endianness[] =3D { > > > - { "aarch64", false }, > > > - { "alpha", false }, > > > - { "arm", false }, > > > - { "cris", false }, > > > - { "i386", false }, > > > - { "lm32", true }, > > > - { "m68k", true }, > > > - { "microblaze", true }, > > > - { "microblazeel", false }, > > > - { "mips", true }, > > > - { "mips64", true }, > > > - { "mips64el", false }, > > > - { "mipsel", false }, > > > - { "moxie", true }, > > > - { "or32", true }, > > > - { "ppc", true }, > > > - { "ppc64", true }, > > > - { "ppcemb", true }, > > > - { "s390x", true }, > > > - { "sh4", false }, > > > - { "sh4eb", true }, > > > - { "sparc", true }, > > > - { "sparc64", true }, > > > - { "unicore32", false }, > > > - { "x86_64", false }, > > > - { "xtensa", false }, > > > - { "xtensaeb", true }, > > > - {}, > > > - }; > > > - > > > - for (i =3D 0; endianness[i].arch; i++) { > > > - if (strcmp(endianness[i].arch, arch) =3D=3D 0) { > > > - return endianness[i].big_endian; > > > - } > > > - } > > > - > > > - return false; > > > + return s->big_endian; > > > } > > > diff --git a/tests/libqtest.h b/tests/libqtest.h > > > index f7402e0..4be1f77 100644 > > > --- a/tests/libqtest.h > > > +++ b/tests/libqtest.h > > > @@ -410,6 +410,14 @@ int64_t qtest_clock_step(QTestState *s, int64_t = step); > > > int64_t qtest_clock_set(QTestState *s, int64_t val); > > > =20 > > > /** > > > + * qtest_big_endian: > > > + * @s: QTestState instance to operate on. > > > + * > > > + * Returns: True if the architecture under test has a big endian con= figuration. > > > + */ > > > +bool qtest_big_endian(QTestState *s); > > > + > > > +/** > > > * qtest_get_arch: > > > * > > > * Returns: The architecture for the QEMU executable under test. > > > @@ -874,12 +882,14 @@ static inline int64_t clock_set(int64_t val) > > > } > > > =20 > > > /** > > > - * qtest_big_endian: > > > + * target_big_endian: > > > * > > > * Returns: True if the architecture under test has a big endian con= figuration. > > > */ > > > -bool qtest_big_endian(void); > > > - > > > +static inline bool target_big_endian(void) > > > +{ > > > + return qtest_big_endian(global_qtest); > > > +} > > > =20 > > > QDict *qmp_fd_receive(int fd); > > > void qmp_fd_sendv(int fd, const char *fmt, va_list ap); > > > diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c > > > index 3c4fecc..0506917 100644 > > > --- a/tests/virtio-blk-test.c > > > +++ b/tests/virtio-blk-test.c > > > @@ -125,7 +125,7 @@ static inline void virtio_blk_fix_request(QVirtio= BlkReq *req) > > > bool host_endian =3D false; > > > #endif > > > =20 > > > - if (qtest_big_endian() !=3D host_endian) { > > > + if (target_big_endian() !=3D host_endian) { > > > req->type =3D bswap32(req->type); > > > req->ioprio =3D bswap32(req->ioprio); > > > req->sector =3D bswap64(req->sector); =20 > > =20 >=20 --Sig_/vlhKPNm1PGP9PJb1go6mw5. Content-Type: application/pgp-signature Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iEYEARECAAYFAlf3S2sACgkQAvw66wEB28K8XwCeMCG+yV+Gn07+YXDnAUJCdnNJ RJ4An0hhDlyyoL+CWTWxhShz72UvFn6r =wusX -----END PGP SIGNATURE----- --Sig_/vlhKPNm1PGP9PJb1go6mw5.--