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
next prev parent 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).