qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Mahmoud Mandour <ma.mandourr@gmail.com>
To: Alexandre Iooss <erdnaxe@crans.org>,
	"open list:All patches CC here" <qemu-devel@nongnu.org>
Cc: "Alex Bennée" <alex.bennee@linaro.org>
Subject: Re: [PATCH] contrib/plugins: add execlog to log instruction execution and memory access
Date: Mon, 14 Jun 2021 19:04:37 +0200	[thread overview]
Message-ID: <d2d0e9c1-872b-158d-fe74-42ef699c60a9@gmail.com> (raw)
In-Reply-To: <20210614090116.816833-1-erdnaxe@crans.org>


On 14/06/2021 11:01, Alexandre Iooss wrote:
> Log instruction execution and memory access to a file.
> This plugin can be used for reverse engineering or for side-channel analysis
> using QEMU.
>
> Signed-off-by: Alexandre Iooss <erdnaxe@crans.org>
> ---
>   MAINTAINERS               |   1 +
>   contrib/plugins/Makefile  |   1 +
>   contrib/plugins/execlog.c | 112 ++++++++++++++++++++++++++++++++++++++
>   3 files changed, 114 insertions(+)
>   create mode 100644 contrib/plugins/execlog.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7d9cd29042..65942d5802 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2974,6 +2974,7 @@ F: include/tcg/
>   
>   TCG Plugins
>   M: Alex Bennée <alex.bennee@linaro.org>
> +R: Alexandre Iooss <erdnaxe@crans.org>
>   S: Maintained
>   F: docs/devel/tcg-plugins.rst
>   F: plugins/
> diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile
> index b9d7935e5e..51093acd17 100644
> --- a/contrib/plugins/Makefile
> +++ b/contrib/plugins/Makefile
> @@ -13,6 +13,7 @@ include $(BUILD_DIR)/config-host.mak
>   VPATH += $(SRC_PATH)/contrib/plugins
>   
>   NAMES :=
> +NAMES += execlog
>   NAMES += hotblocks
>   NAMES += hotpages
>   NAMES += howvec
> diff --git a/contrib/plugins/execlog.c b/contrib/plugins/execlog.c
> new file mode 100644
> index 0000000000..80716e8eed
> --- /dev/null
> +++ b/contrib/plugins/execlog.c
> @@ -0,0 +1,112 @@
> +/*
> + * Copyright (C) 2021, Alexandre Iooss <erdnaxe@crans.org>
> + *
> + * Log instruction execution and memory access to a file.
> + * You may pass the output filename as argument.
> + *
> + * License: GNU GPL, version 2 or later.
> + *   See the COPYING file in the top-level directory.
> + */
> +#include <glib.h>
> +#include <inttypes.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +#include <qemu-plugin.h>
> +
> +QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
> +
> +/* Execution trace buffer */
> +FILE *output;

Is there a reason for using a regular FILE * instead of using 
`qemu_plugin_outs()`?

I don't see a benefit and since there's an API for logging I guess it's 
better to use it instead

> +
> +/**
> + * Log memory read or write
> + */
> +static void vcpu_mem(unsigned int vcpu_index, qemu_plugin_meminfo_t info,
> +                     uint64_t vaddr, void *udata)
> +{
> +    struct qemu_plugin_hwaddr *hwaddr = qemu_plugin_get_hwaddr(info, vaddr);
> +    if (!hwaddr) {
> +        return;
> +    }

So you just reject all memory accesses in user mode? I think that it 
equally useful

to log only the virtual address of a memory access in user-mode 
emulation? However, we better

wait for Alex's opinion on all this since he had remarks about 
introducing a new ad-hoc

tracing format...

> +
> +    /* Add data to execution log */
> +    const char *name = qemu_plugin_hwaddr_device_name(hwaddr);
> +    uint64_t addr = qemu_plugin_hwaddr_phys_addr(hwaddr);
> +    if (qemu_plugin_mem_is_store(info)) {
> +        fprintf(output, "mem: %s store at 0x%08lx\n", name, addr);
> +    } else {
> +        fprintf(output, "mem: %s load at 0x%08lx\n", name, addr);
> +    }
> +}
> +
> +/**
> + * Log instruction execution
> + */
> +static void vcpu_insn_exec(unsigned int cpu_index, void *udata)
> +{
> +    char *insn_disas = (char *)udata;
> +
> +    /* Add data to execution log */
> +    fprintf(output, "insn: %s\n", insn_disas);
> +}
> +
> +/**
> + * On translation block new translation
> + *
> + * QEMU convert code by translation block (TB). By hooking here we can then hook
> + * a callback on each instruction and memory access.
> + */
> +static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
> +{
> +    size_t n = qemu_plugin_tb_n_insns(tb);
> +    for (size_t i = 0; i < n; i++) {
> +        /* insn is shared between translations in QEMU, copy needed data here */
> +        struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i);
> +        char *insn_disas = qemu_plugin_insn_disas(insn);
> +
> +        /* Register callback on memory read or write */
> +        qemu_plugin_register_vcpu_mem_cb(insn, vcpu_mem,
> +                                         QEMU_PLUGIN_CB_NO_REGS,
> +                                         QEMU_PLUGIN_MEM_RW, NULL);
> +
> +        /* Register callback on instruction */
> +        qemu_plugin_register_vcpu_insn_exec_cb(
> +            insn, vcpu_insn_exec, QEMU_PLUGIN_CB_R_REGS, insn_disas);
> +    }
> +}
> +
> +/**
> + * On plugin exit, close output file
> + */
> +static void plugin_exit(qemu_plugin_id_t id, void *p)
> +{
> +    fclose(output);
> +}
> +
> +/**
> + * Install the plugin
> + */
> +QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
> +                                           const qemu_info_t *info, int argc,
> +                                           char **argv)
> +{
> +    /* Parse arguments to get output name and open for writing */
> +    char *filename = "execution.log";
> +    if (argc > 0) {
> +        filename = argv[0];
> +    }
> +    output = fopen(filename, "w");
> +    if (output == NULL) {
> +        qemu_plugin_outs("Cannot open output file for writing.\n");

Here, I think that it's more logical to output error messages to stderr 
since

logging can be redirected to a file so QEMU will error but the error message

would be written to the log file.

> +        return -1;
> +    }
> +
> +    /* Register translation block and exit callbacks */
> +    qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
> +    qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
> +
> +    return 0;
> +}


Thanks,

Mahmoud



  reply	other threads:[~2021-06-14 17:06 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-14  9:01 [PATCH] contrib/plugins: add execlog to log instruction execution and memory access Alexandre Iooss
2021-06-14 17:04 ` Mahmoud Mandour [this message]
2021-06-15  8:22   ` Alex Bennée
2021-06-15 16:47     ` Alexandre IOOSS
2021-06-16 15:15       ` Alexandre IOOSS
2021-06-17  9:55         ` Alex Bennée

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=d2d0e9c1-872b-158d-fe74-42ef699c60a9@gmail.com \
    --to=ma.mandourr@gmail.com \
    --cc=alex.bennee@linaro.org \
    --cc=erdnaxe@crans.org \
    --cc=qemu-devel@nongnu.org \
    /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).