On Thu, 6 Oct 2016 20:56:58 +0200
Laurent Vivier <lviv...@redhat.com> wrote:
> 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.
>
> Signed-off-by: Laurent Vivier <lviv...@redhat.com>
> CC: Greg Kurz <gr...@kaod.org>
> CC: David Gibson <da...@gibson.dropbear.id.au>
> CC: Peter Maydell <peter.mayd...@linaro.org>
> ---
> 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.
>
This is definitely an improvement indeed.
Reviewed-by: Greg Kurz <gr...@kaod.org>
> 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(-)
>
> 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(CharDriverState *chr,
> gchar **words)
>
> qtest_send_prefix(chr);
> qtest_send(chr, "OK\n");
> + } else if (strcmp(words[0], "endianness") == 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") == 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(QVirtioDevice *d,
> uint64_t addr)
> int i;
> uint64_t u64 = 0;
>
> - if (qtest_big_endian()) {
> + if (target_big_endian()) {
> for (i = 0; i < 8; ++i) {
> u64 |= (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;
> };
>
> static GHookList abrt_hooks;
> @@ -146,89 +147,6 @@ void qtest_add_abrt_handler(GHookFunc fn, const void
> *data)
> g_hook_prepend(&abrt_hooks, hook);
> }
>
> -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 = getenv("QTEST_QEMU_BINARY");
> - g_assert(qemu_binary != NULL);
> -
> - s = g_malloc(sizeof(*s));
> -
> - socket_path = g_strdup_printf("/tmp/qtest-%d.sock", getpid());
> - qmp_socket_path = g_strdup_printf("/tmp/qtest-%d.qmp", getpid());
> -
> - sock = init_socket(socket_path);
> - qmpsock = init_socket(qmp_socket_path);
> -
> - qtest_add_abrt_handler(kill_qemu_hook_func, s);
> -
> - s->qemu_pid = fork();
> - if (s->qemu_pid == 0) {
> - setenv("QEMU_AUDIO_DRV", "none", true);
> - command = g_strdup_printf("exec %s "
> - "-qtest unix:%s,nowait "
> - "-qtest-log %s "
> - "-qmp unix:%s,nowait "
> - "-machine accel=qtest "
> - "-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 = socket_accept(sock);
> - if (s->fd >= 0) {
> - s->qmp_fd = socket_accept(qmpsock);
> - }
> - unlink(socket_path);
> - unlink(qmp_socket_path);
> - g_free(socket_path);
> - g_free(qmp_socket_path);
> -
> - g_assert(s->fd >= 0 && s->qmp_fd >= 0);
> -
> - s->rx = g_string_new("");
> - for (i = 0; i < MAX_IRQ; i++) {
> - s->irq_level[i] = 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 = g_list_remove(qtest_instances, s);
> - g_hook_destroy_link(&abrt_hooks, g_hook_find_data(&abrt_hooks, TRUE, 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;
>
> +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 = getenv("QTEST_QEMU_BINARY");
> + g_assert(qemu_binary != NULL);
> +
> + s = g_malloc(sizeof(*s));
> +
> + socket_path = g_strdup_printf("/tmp/qtest-%d.sock", getpid());
> + qmp_socket_path = g_strdup_printf("/tmp/qtest-%d.qmp", getpid());
> +
> + sock = init_socket(socket_path);
> + qmpsock = init_socket(qmp_socket_path);
> +
> + qtest_add_abrt_handler(kill_qemu_hook_func, s);
> +
> + s->qemu_pid = fork();
> + if (s->qemu_pid == 0) {
> + setenv("QEMU_AUDIO_DRV", "none", true);
> + command = g_strdup_printf("exec %s "
> + "-qtest unix:%s,nowait "
> + "-qtest-log %s "
> + "-qmp unix:%s,nowait "
> + "-machine accel=qtest "
> + "-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 = socket_accept(sock);
> + if (s->fd >= 0) {
> + s->qmp_fd = socket_accept(qmpsock);
> + }
> + unlink(socket_path);
> + unlink(qmp_socket_path);
> + g_free(socket_path);
> + g_free(qmp_socket_path);
> +
> + g_assert(s->fd >= 0 && s->qmp_fd >= 0);
> +
> + s->rx = g_string_new("");
> + for (i = 0; i < MAX_IRQ; i++) {
> + s->irq_level[i] = 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 = qtest_rsp(s, 1);
> + g_assert(strcmp(args[1], "big") == 0 || strcmp(args[1], "little") == 0);
> + s->big_endian = strcmp(args[1], "big") == 0;
> + g_strfreev(args);
> +
> + return s;
> +}
> +
> +void qtest_quit(QTestState *s)
> +{
> + qtest_instances = g_list_remove(qtest_instances, s);
> + g_hook_destroy_link(&abrt_hooks, g_hook_find_data(&abrt_hooks, TRUE, 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 = container_of(parser, QMPResponseParser, parser);
> @@ -886,50 +897,7 @@ char *hmp(const char *fmt, ...)
> return ret;
> }
>
> -bool qtest_big_endian(void)
> +bool qtest_big_endian(QTestState *s)
> {
> - const char *arch = qtest_get_arch();
> - int i;
> -
> - static const struct {
> - const char *arch;
> - bool big_endian;
> - } endianness[] = {
> - { "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 = 0; endianness[i].arch; i++) {
> - if (strcmp(endianness[i].arch, arch) == 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);
>
> /**
> + * qtest_big_endian:
> + * @s: QTestState instance to operate on.
> + *
> + * Returns: True if the architecture under test has a big endian
> configuration.
> + */
> +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)
> }
>
> /**
> - * qtest_big_endian:
> + * target_big_endian:
> *
> * Returns: True if the architecture under test has a big endian
> configuration.
> */
> -bool qtest_big_endian(void);
> -
> +static inline bool target_big_endian(void)
> +{
> + return qtest_big_endian(global_qtest);
> +}
>
> 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(QVirtioBlkReq
> *req)
> bool host_endian = false;
> #endif
>
> - if (qtest_big_endian() != host_endian) {
> + if (target_big_endian() != host_endian) {
> req->type = bswap32(req->type);
> req->ioprio = bswap32(req->ioprio);
> req->sector = bswap64(req->sector);
