qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
To: Rowan Hart <rowanbhart@gmail.com>, qemu-devel@nongnu.org
Cc: "Richard Henderson" <richard.henderson@linaro.org>,
	"Eduardo Habkost" <eduardo@habkost.net>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"Alexandre Iooss" <erdnaxe@crans.org>,
	"Mahmoud Mandour" <ma.mandourr@gmail.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>
Subject: Re: [PATCH v2 3/3] Add inject plugin and x86_64 target for the inject plugin
Date: Fri, 6 Dec 2024 11:57:50 -0800	[thread overview]
Message-ID: <5ec40cb8-8a9d-4d13-b78e-79ea30317dbf@linaro.org> (raw)
In-Reply-To: <20241206102605.961658-4-rowanbhart@gmail.com>

On 12/6/24 02:26, Rowan Hart wrote:
> From: novafacing <rowanbhart@gmail.com>
> 
> ---
>   tests/tcg/plugins/inject.c       | 206 +++++++++++++++++++++++++++++++
>   tests/tcg/plugins/meson.build    |   2 +-
>   tests/tcg/x86_64/Makefile.target |   1 +
>   tests/tcg/x86_64/inject-target.c |  27 ++++
>   4 files changed, 235 insertions(+), 1 deletion(-)
>   create mode 100644 tests/tcg/plugins/inject.c
>   create mode 100644 tests/tcg/x86_64/inject-target.c
> 
> diff --git a/tests/tcg/plugins/inject.c b/tests/tcg/plugins/inject.c
> new file mode 100644
> index 0000000000..9edc2cd34e
> --- /dev/null
> +++ b/tests/tcg/plugins/inject.c

Could we find a better name?

> @@ -0,0 +1,206 @@
> +/*
> + * Copyright (C) 2024, Rowan Hart <rowanbhart@gmail.com>
> + *
> + * License: GNU GPL, version 2 or later.
> + *   See the COPYING file in the top-level directory.
> + */

We can add a comment here about what the plugin is doing.

> +#include "glib.h"
> +#include <assert.h>
> +#include <inttypes.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +#include <qemu-plugin.h>
> +
> +/*
> + * Specifies a Hypercall for an architecture:
> + *
> + * - Architecture name
> + * - Whether it is enabled
> + * - The hypercall instruction
> + * - The register names to pass the hypercall # and args
> + */
> +struct HypercallSpec {
> +    const char *name;
> +    const bool enabled;
> +    const char *hypercall;
> +    const bool little_endian;
> +    const char *num_reg;
> +    const char *arg0_reg;
> +    const char *arg1_reg;
> +};
> +
> +static const struct HypercallSpec *hypercall_spec;
> +
> +static const struct HypercallSpec hypercall_specs[] = {
> +    { "aarch64", false, NULL, true, 0, 0, 0 },
> +    { "aarch64_be", false, NULL, false, 0, 0, 0 },
> +    { "alpha", false, NULL, true, 0, 0, 0 },
> +    { "arm", false, NULL, true, 0, 0, 0 },
> +    { "armeb", false, NULL, false, 0, 0, 0 },
> +    { "avr", false, NULL, true, 0, 0, 0 },
> +    { "hexagon", false, NULL, true, 0, 0, 0 },
> +    { "hppa", false, NULL, false, 0, 0, 0 },
> +    { "i386", false, NULL, true, 0, 0, 0 },
> +    { "loongarch64", false, NULL, true, 0, 0, 0 },
> +    { "m68k", false, NULL, false, 0, 0, 0 },
> +    { "microblaze", false, NULL, false, 0, 0, 0 },
> +    { "microblazeel", false, NULL, true, 0, 0, 0 },
> +    { "mips", false, NULL, false, 0, 0, 0 },
> +    { "mips64", false, NULL, false, 0, 0, 0 },
> +    { "mips64el", false, NULL, true, 0, 0, 0 },
> +    { "mipsel", false, NULL, true, 0, 0, 0 },
> +    { "mipsn32", false, NULL, false, 0, 0, 0 },
> +    { "mipsn32el", false, NULL, true, 0, 0, 0 },
> +    { "or1k", false, NULL, false, 0, 0, 0 },
> +    { "ppc", false, NULL, false, 0, 0, 0 },
> +    { "ppc64", false, NULL, false, 0, 0, 0 },
> +    { "ppc64le", false, NULL, true, 0, 0, 0 },
> +    { "riscv32", false, NULL, true, 0, 0, 0 },
> +    { "riscv64", false, NULL, true, 0, 0, 0 },
> +    { "rx", false, NULL, true, 0, 0, 0 },
> +    { "s390x", false, NULL, false, 0, 0, 0 },
> +    { "sh4", false, NULL, true, 0, 0, 0 },
> +    { "sh4eb", false, NULL, false, 0, 0, 0 },
> +    { "sparc", false, NULL, false, 0, 0, 0 },
> +    { "sparc32plus", false, NULL, false, 0, 0, 0 },
> +    { "sparc64", false, NULL, false, 0, 0, 0 },
> +    { "tricore", false, NULL, true, 0, 0, 0 },
> +    { "x86_64", true, "\x0f\xa2", true, "rax", "rdi", "rsi" },
> +    { "xtensa", false, NULL, true, 0, 0, 0 },
> +    { "xtensaeb", false, NULL, false, 0, 0, 0 },
> +    { NULL, false, NULL, false, 0, 0, 0 },
> +};
> +
> +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
> +
> +/*
> + * Returns a handle to a register with a given name, or NULL if there is no
> + * such register.
> + */
> +static struct qemu_plugin_register *get_register(const char *name)
> +{
> +    GArray *registers = qemu_plugin_get_registers();
> +
> +    struct qemu_plugin_register *handle = NULL;
> +
> +    qemu_plugin_reg_descriptor *reg_descriptors =
> +        (qemu_plugin_reg_descriptor *)registers->data;
> +
> +    for (size_t i = 0; i < registers->len; i++) {
> +        if (!strcmp(reg_descriptors[i].name, name)) {
> +            handle = reg_descriptors[i].handle;
> +        }
> +    }
> +
> +    g_array_free(registers, true);
> +
> +    return handle;
> +}
> +
> +/*
> + * Transforms a byte array with at most 8 entries into a uint64_t
> + * depending on the target machine's endianness.
> + */
> +static uint64_t byte_array_to_uint64(GByteArray *buf)
> +{
> +    uint64_t value = 0;
> +    if (hypercall_spec->little_endian) {
> +        for (int i = 0; i < buf->len && i < sizeof(uint64_t); i++) {
> +            value |= ((uint64_t)buf->data[i]) << (i * 8);
> +        }
> +    } else {
> +        for (int i = 0; i < buf->len && i < sizeof(uint64_t); i++) {
> +            value |= ((uint64_t)buf->data[i]) << ((buf->len - 1 - i) * 8);
> +        }
> +    }
> +    return value;
> +}
> +
> +/*
> + * Handle a "hyperacll" instruction, which has some special meaning for this
> + * plugin.
> + */
> +static void hypercall(unsigned int vcpu_index, void *userdata)
> +{
> +    uint64_t num = 0, arg0 = 0, arg1 = 0;
> +    GByteArray *buf = g_byte_array_new();
> +    qemu_plugin_read_register(get_register(hypercall_spec->num_reg), buf);
> +    num = byte_array_to_uint64(buf);
> +
> +    g_byte_array_set_size(buf, 0);
> +    qemu_plugin_read_register(get_register(hypercall_spec->arg0_reg), buf);
> +    arg0 = byte_array_to_uint64(buf);
> +
> +    g_byte_array_set_size(buf, 0);
> +    qemu_plugin_read_register(get_register(hypercall_spec->arg1_reg), buf);
> +    arg1 = byte_array_to_uint64(buf);
> +
> +    switch (num) {
> +    /*
> +     * The write hypercall (#0x13371337) tells the plugin to write random bytes
> +     * of a given size into the memory of the emulated system at a particular
> +     * vaddr
> +     */

One challenge with picking a random value, is how to ensure this pattern 
has no other meaning for all architectures? I'm not sure we can find a 
single pattern of bytes that works for all arch, even though that would 
be definitely stylish :).

In more, it seems that we are reinventing the syscall interface, while 
we already have it. But as the current instrumentation only works for 
user-mode, having a specific hypercall interface might be worth it for 
plugins, so system mode could benefit from it too.

The work done here could serve later to define a proper interface.

> +    case 0x13371337: {
> +        GByteArray *data = g_byte_array_new();
> +        g_byte_array_set_size(data, arg1);
> +        for (uint64_t i = 0; i < arg1; i++) {
> +            data->data[i] = (uint8_t)g_random_int();
> +        }
> +        qemu_plugin_write_memory_vaddr(arg0, data);
> +        break;
> +    }
> +    default:
> +        break;
> +    }
> +
> +    g_byte_array_free(buf, TRUE);
> +}
> +
> +/*
> + * Callback on translation of a translation block.
> + */
> +static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
> +{
> +    for (size_t i = 0; i < qemu_plugin_tb_n_insns(tb); i++) {
> +        struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i);
> +        GByteArray *insn_data = g_byte_array_new();
> +        size_t insn_len = qemu_plugin_insn_size(insn);
> +        g_byte_array_set_size(insn_data, insn_len);
> +        qemu_plugin_insn_data(insn, insn_data->data, insn_data->len);
> +        if (!memcmp(insn_data->data, hypercall_spec->hypercall, insn_data->len)) {
> +            qemu_plugin_register_vcpu_insn_exec_cb(insn, hypercall,
> +                                                   QEMU_PLUGIN_CB_R_REGS, NULL);
> +        }
> +        g_byte_array_free(insn_data, true);
> +    }
> +}
> +
> +
> +/*
> + * Called when the plugin is installed
> + */
> +QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
> +                                           const qemu_info_t *info, int argc,
> +                                           char **argv)
> +{
> +    hypercall_spec = &hypercall_specs[0];
> +    while (hypercall_spec->name != NULL) {
> +        if (!strcmp(hypercall_spec->name, info->target_name)) {
> +            break;
> +        }
> +        hypercall_spec++;
> +    }
> +
> +    if (hypercall_spec->name == NULL) {
> +        qemu_plugin_outs("Error: no hypercall spec.");
> +        return -1;
> +    }
> +
> +    qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
> +
> +    return 0;
> +}
> diff --git a/tests/tcg/plugins/meson.build b/tests/tcg/plugins/meson.build
> index f847849b1b..96782416d3 100644
> --- a/tests/tcg/plugins/meson.build
> +++ b/tests/tcg/plugins/meson.build
> @@ -1,6 +1,6 @@
>   t = []
>   if get_option('plugins')
> -  foreach i : ['bb', 'empty', 'inline', 'insn', 'mem', 'syscall']
> +  foreach i : ['bb', 'empty', 'inline', 'insn', 'mem', 'syscall', 'inject']
>       if host_os == 'windows'
>         t += shared_module(i, files(i + '.c') + '../../../contrib/plugins/win32_linker.c',
>                           include_directories: '../../../include/qemu',
> diff --git a/tests/tcg/x86_64/Makefile.target b/tests/tcg/x86_64/Makefile.target
> index d6dff559c7..7c8e21636d 100644
> --- a/tests/tcg/x86_64/Makefile.target
> +++ b/tests/tcg/x86_64/Makefile.target
> @@ -18,6 +18,7 @@ X86_64_TESTS += adox
>   X86_64_TESTS += test-1648
>   X86_64_TESTS += test-2175
>   X86_64_TESTS += cross-modifying-code
> +X86_64_TESTS += inject-target
>   TESTS=$(MULTIARCH_TESTS) $(X86_64_TESTS) test-x86_64
>   else
>   TESTS=$(MULTIARCH_TESTS)
> diff --git a/tests/tcg/x86_64/inject-target.c b/tests/tcg/x86_64/inject-target.c
> new file mode 100644
> index 0000000000..c886e5ab8b
> --- /dev/null
> +++ b/tests/tcg/x86_64/inject-target.c
> @@ -0,0 +1,27 @@
> +#include <stddef.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +
> +#define hypercall(num, arg0, arg1)                                \
> +    unsigned int _a __attribute__((unused)) = 0;                  \
> +    unsigned int _b __attribute__((unused)) = 0;                  \
> +    unsigned int _c __attribute__((unused)) = 0;                  \
> +    unsigned int _d __attribute__((unused)) = 0;                  \
> +    __asm__ __volatile__("cpuid\n\t"                              \
> +                         : "=a"(_a), "=b"(_b), "=c"(_c), "=d"(_d) \
> +                         : "a"(num), "D"(arg0), "S"(arg1));
> +
> +int main(void)
> +{
> +    uint16_t value;
> +
> +    for (size_t i = 0; i < 1000000; i++) {
> +        hypercall(0x13371337, &value, sizeof(value));
> +        if (value == 0x1337) {
> +            printf("Victory!\n");
> +            return 0;
> +        }
> +    }
> +    return 1;
> +}
> +



  reply	other threads:[~2024-12-06 19:58 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-06 10:26 [PATCH v2 0/3] Add additional plugin API functions to read and write memory and registers Rowan Hart
2024-12-06 10:26 ` [PATCH v2 1/3] Expose gdb_write_register function to consumers of gdbstub Rowan Hart
2025-01-09 12:03   ` Alex Bennée
2024-12-06 10:26 ` [PATCH v2 2/3] Add plugin API functions for register R/W, hwaddr R/W, vaddr W Rowan Hart
2025-01-09 12:22   ` Alex Bennée
2024-12-06 10:26 ` [PATCH v2 3/3] Add inject plugin and x86_64 target for the inject plugin Rowan Hart
2024-12-06 19:57   ` Pierrick Bouvier [this message]
2024-12-07  1:02     ` Rowan Hart
2024-12-09 18:38       ` Pierrick Bouvier
2024-12-06 19:43 ` [PATCH v2 0/3] Add additional plugin API functions to read and write memory and registers Pierrick Bouvier
2024-12-07  0:57   ` Rowan Hart
2024-12-09 18:45     ` Pierrick Bouvier
2024-12-10 11:38       ` Alex Bennée
2024-12-10 18:40         ` Pierrick Bouvier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5ec40cb8-8a9d-4d13-b78e-79ea30317dbf@linaro.org \
    --to=pierrick.bouvier@linaro.org \
    --cc=alex.bennee@linaro.org \
    --cc=eduardo@habkost.net \
    --cc=erdnaxe@crans.org \
    --cc=ma.mandourr@gmail.com \
    --cc=pbonzini@redhat.com \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    --cc=rowanbhart@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).