| Message ID | 20210821094527.491232-2-florian.hauschild@fs.ei.tum.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | QEMU TCG plugin interface extensions | expand |
On Sat, 21 Aug 2021 at 10:48, Florian Hauschild <florian.hauschild@fs.ei.tum.de> wrote: > > This extension covers functions: > * to read and write guest memory > * to read and write guest registers > * to flush tb cache > * to control single stepping of qemu from plugin > > These changes allow the user to > * collect more information about the behaviour of the system > * change the guest state with a plugin during execution > * control cache of tcg > * allow for precise instrumentation in execution flow > + > +static int plugin_read_register(CPUState *cpu, GByteArray *buf, int reg) > +{ > + CPUClass *cc = CPU_GET_CLASS(cpu); > + if (reg < cc->gdb_num_core_regs) { > + return cc->gdb_read_register(cpu, buf, reg); > + } > + return 0; > +} At the point where these functions execute is the emulation definitely stopped (ie no register values currently held live in TCG locals) ? -- PMM
Am 21.08.21 um 15:18 schrieb Peter Maydell: > On Sat, 21 Aug 2021 at 10:48, Florian Hauschild > <florian.hauschild@fs.ei.tum.de> wrote: >> >> This extension covers functions: >> * to read and write guest memory >> * to read and write guest registers >> * to flush tb cache >> * to control single stepping of qemu from plugin >> >> These changes allow the user to >> * collect more information about the behaviour of the system >> * change the guest state with a plugin during execution >> * control cache of tcg >> * allow for precise instrumentation in execution flow > >> + >> +static int plugin_read_register(CPUState *cpu, GByteArray *buf, int reg) >> +{ >> + CPUClass *cc = CPU_GET_CLASS(cpu); >> + if (reg < cc->gdb_num_core_regs) { >> + return cc->gdb_read_register(cpu, buf, reg); >> + } >> + return 0; >> +} > > At the point where these functions execute is the emulation > definitely stopped (ie no register values currently held > live in TCG locals) ? > > -- PMM > I am not sure, if it is definitely stopped. I call them during tb_exec_cb and insn_exec_cb. I have used the extension on ARM and RISC-V single cpu guests and the data collected is the one i would expect during normal execution on real hardware. How this would behave on a multi cpu/core system i have not tested yet. Currently i am looking into this and as soon as i have found an answer i will write back again. Regards Florian
On Tue, 24 Aug 2021 at 15:34, Florian Hauschild <florian.hauschild@fs.ei.tum.de> wrote: > > > > Am 21.08.21 um 15:18 schrieb Peter Maydell: > > On Sat, 21 Aug 2021 at 10:48, Florian Hauschild > > <florian.hauschild@fs.ei.tum.de> wrote: > >> > >> This extension covers functions: > >> * to read and write guest memory > >> * to read and write guest registers > >> * to flush tb cache > >> * to control single stepping of qemu from plugin > >> > >> These changes allow the user to > >> * collect more information about the behaviour of the system > >> * change the guest state with a plugin during execution > >> * control cache of tcg > >> * allow for precise instrumentation in execution flow > > > >> + > >> +static int plugin_read_register(CPUState *cpu, GByteArray *buf, int reg) > >> +{ > >> + CPUClass *cc = CPU_GET_CLASS(cpu); > >> + if (reg < cc->gdb_num_core_regs) { > >> + return cc->gdb_read_register(cpu, buf, reg); > >> + } > >> + return 0; > >> +} > > > > At the point where these functions execute is the emulation > > definitely stopped (ie no register values currently held > > live in TCG locals) ? > I am not sure, if it is definitely stopped. > I call them during tb_exec_cb and insn_exec_cb. > I have used the extension on ARM and RISC-V single cpu guests and the > data collected is the one i would expect during normal execution on real > hardware. How this would behave on a multi cpu/core system i have not > tested yet. Multicore isn't relevant here. What you want to check for is what happens when the TB covers multiple guest instructions such that a later insn in the TB uses a register that is set by an earlier insn in the TB, eg: mov x0, 0 add x0, x0, 1 add x0, x0, 1 In this case TCG is likely to generate code which does not write back the intermediate 0 and 1 values of x0 to the CPUState struct, and so reading x0 via the gdb_read_register interface before the execution of the 3rd insn will continue to return whatever value x0 had before execution of the TB started. For the gdbstub's use of the gdb_read_register API, this can't happen, because we always completely stop the CPU (which means it is not inside a TB at all) before handling gdbstub packets requesting register information. I don't know whether the TCG plugin infrastructure takes steps with its various callbacks to ensure that intermediate values get written back to the CPU state before the callback is invoked: it's possible that this is safe, or can be made to be safe. thanks -- PMM
Am 24.08.21 um 16:47 schrieb Peter Maydell: > On Tue, 24 Aug 2021 at 15:34, Florian Hauschild > <florian.hauschild@fs.ei.tum.de> wrote: >> >> >> >> Am 21.08.21 um 15:18 schrieb Peter Maydell: >>> On Sat, 21 Aug 2021 at 10:48, Florian Hauschild >>> <florian.hauschild@fs.ei.tum.de> wrote: >>>> >>>> This extension covers functions: >>>> * to read and write guest memory >>>> * to read and write guest registers >>>> * to flush tb cache >>>> * to control single stepping of qemu from plugin >>>> >>>> These changes allow the user to >>>> * collect more information about the behaviour of the system >>>> * change the guest state with a plugin during execution >>>> * control cache of tcg >>>> * allow for precise instrumentation in execution flow >>> >>>> + >>>> +static int plugin_read_register(CPUState *cpu, GByteArray *buf, int reg) >>>> +{ >>>> + CPUClass *cc = CPU_GET_CLASS(cpu); >>>> + if (reg < cc->gdb_num_core_regs) { >>>> + return cc->gdb_read_register(cpu, buf, reg); >>>> + } >>>> + return 0; >>>> +} >>> >>> At the point where these functions execute is the emulation >>> definitely stopped (ie no register values currently held >>> live in TCG locals) ? > >> I am not sure, if it is definitely stopped. >> I call them during tb_exec_cb and insn_exec_cb. >> I have used the extension on ARM and RISC-V single cpu guests and the >> data collected is the one i would expect during normal execution on real >> hardware. How this would behave on a multi cpu/core system i have not >> tested yet. > > Multicore isn't relevant here. What you want to check for > is what happens when the TB covers multiple guest instructions > such that a later insn in the TB uses a register that is > set by an earlier insn in the TB, eg: > > mov x0, 0 > add x0, x0, 1 > add x0, x0, 1 > > In this case TCG is likely to generate code which does not > write back the intermediate 0 and 1 values of x0 to the CPUState > struct, and so reading x0 via the gdb_read_register interface > before the execution of the 3rd insn will continue to return > whatever value x0 had before execution of the TB started. > > For the gdbstub's use of the gdb_read_register API, this > can't happen, because we always completely stop the CPU > (which means it is not inside a TB at all) before handling > gdbstub packets requesting register information. > > I don't know whether the TCG plugin infrastructure takes steps > with its various callbacks to ensure that intermediate values > get written back to the CPU state before the callback is > invoked: it's possible that this is safe, or can be made to > be safe. > > thanks > -- PMM > Sorry, i misunderstood your question. Form my observation all three insn_cb would see x0 == 2. They are executed at the end of a tb execution. During my testing these changes were stable and i assume they are safe. But thats why i chose RFC. I am new to QEMU and might overlook something important. Please correct me if i am wrong: When the TB is executed, first the TB cb is executed, then the various instruction cb. If you would like to see x0 in between instructions (e.g mov and first add), QEMU need to be in single step mode. The plugin infrastructure does have some sort of infrastructure to tell the tcg if the registers are read or written to, but does apparently not use it. The register values seem to be written back before the various cbs are called. Regards Florian
On Thu, 26 Aug 2021 at 15:12, Florian Hauschild <florian.hauschild@fs.ei.tum.de> wrote: > Form my observation all three insn_cb would see x0 == 2. They are > executed at the end of a tb execution. The documentation for the insn_exec_cb says the cb is called every time an instruction is executed. That won't always be at the end of a TB, will it ? > Please correct me if i am wrong: > When the TB is executed, first the TB cb is executed, then the various > instruction cb. If you would like to see x0 in between instructions (e.g > mov and first add), QEMU need to be in single step mode. > The plugin infrastructure does have some sort of infrastructure to tell > the tcg if the registers are read or written to, but does apparently not > use it. The register values seem to be written back before the various > cbs are called. Any new plugin API for "read/write registers" needs to work correctly at any point where it is valid for it to be called, whether QEMU is in single-step mode or not. I guess we'll wait for Alex to get back from holiday and have a look at this... thanks -- PMM
Florian Hauschild <florian.hauschild@fs.ei.tum.de> writes: > This extension covers functions: > * to read and write guest memory > * to read and write guest registers > * to flush tb cache > * to control single stepping of qemu from plugin Next time please split the functionality into separate patches to aid review. > > These changes allow the user to > * collect more information about the behaviour of the system More introspection into guest state is a welcome improvement. > * change the guest state with a plugin during execution I have no in principle objection to this but the wider community may disagree. We are wary of plugins being used for GPL avoidance reasons. > * control cache of tcg Why? From the plugins point of view the internal state of the translator should be irrelevant. If it's not it's a bug. > * allow for precise instrumentation in execution flow More precise than every instruction and memory access? How exactly? > > Signed-off-by: Florian Hauschild <florian.hauschild@fs.ei.tum.de> > --- > include/qemu/qemu-plugin.h | 35 ++++++++++++ > plugins/meson.build | 1 + > plugins/readwriteextension.c | 106 +++++++++++++++++++++++++++++++++++ > 3 files changed, 142 insertions(+) > create mode 100644 plugins/readwriteextension.c > > diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h > index e6e815abc5..c7a0c5f379 100644 > --- a/include/qemu/qemu-plugin.h > +++ b/include/qemu/qemu-plugin.h > @@ -577,4 +577,39 @@ int qemu_plugin_n_max_vcpus(void); > */ > void qemu_plugin_outs(const char *string); > > + > +/** > + * read_reg() read a register > + * @reg: Number of the register > + * > + * Returns the value of the register > + */ > +uint64_t read_reg(int reg); > + > +/** > + * write_reg() - write to a register > + * @reg: number of the register > + * @val: value written to register > + */ > +void write_reg(int reg, uint64_t val); > + > +/** > + * plugin_flush_tb() - Flush the tb cache > + */ > +void plugin_flush_tb(void); > + > +/** > + * plugin_rw_memory_cpu() - Function to read from and write to a guest address. > + * @address: baseaddress of the memory section > + * @buffer: buffer managed by caller the value should be written to > + * @buf_size: size of the buffer and memory size read/written. > + * @write: 1 if write, 0 if read > + */ > +int plugin_rw_memory_cpu(uint64_t address, uint8_t buffer[], size_t buf_size, char write); > + > +/** > + * plugin_single_step() - Function to change single step behaviour from the plugin. > + */ > +void plugin_single_step(int enable); > + > #endif /* QEMU_PLUGIN_API_H */ > diff --git a/plugins/meson.build b/plugins/meson.build > index e77723010e..b95cbab0b1 100644 > --- a/plugins/meson.build > +++ b/plugins/meson.build > @@ -10,4 +10,5 @@ specific_ss.add(when: 'CONFIG_PLUGIN', if_true: [files( > 'loader.c', > 'core.c', > 'api.c', > + 'readwriteextension.c', > ), declare_dependency(link_args: plugin_ldflags)]) > diff --git a/plugins/readwriteextension.c b/plugins/readwriteextension.c > new file mode 100644 > index 0000000000..47460c396f > --- /dev/null > +++ b/plugins/readwriteextension.c > @@ -0,0 +1,106 @@ > +/** > + * QEMU Plugin read write extension code > + * > + * This is the code that allows the plugin to read and write > + * memory and registers and flush the tb cache. Also allows > + * to set QEMU into singlestep mode from Plugin. > + * > + * Based on plugin interface: > + * Copyright (C) 2017, Emilio G. Cota <cota@braap.org> > + * Copyright (C) 2019, Linaro > + * > + * Copyright (C) 2021 Florian Hauschild <florian.hauschild@tum.de> > + * > + * License: GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + * > + * SPDX-License-Identifier: GPL-2.0-or-later > + */ > + > + > + > +#include "qemu/osdep.h" > +#include "qemu/plugin.h" > +#include "hw/core/cpu.h" > +#include "cpu.h" > +#include "exec/exec-all.h" > + > +void plugin_async_flush_tb(CPUState *cpu, run_on_cpu_data arg); > +void plugin_async_flush_tb(CPUState *cpu, run_on_cpu_data arg) > +{ > + g_assert(cpu_in_exclusive_context(cpu)); > + tb_flush(cpu); > +} > + > + > + > +int plugin_rw_memory_cpu(uint64_t address, uint8_t buffer[], size_t buf_size, char write) > +{ > + return cpu_memory_rw_debug(current_cpu, address, buffer, buf_size, write); Accessing memory during a plugin event is tricky. There is no way to know if memory has changed underneath you. Would it instead be more useful to derive the access from instrumented instructions? That way you can know the exact value read or written at that exact moment in time. > +} > + > + > +void plugin_flush_tb(void) > +{ > + async_safe_run_on_cpu(current_cpu, plugin_async_flush_tb, RUN_ON_CPU_NULL); > +} > + > +static int plugin_read_register(CPUState *cpu, GByteArray *buf, int reg) > +{ > + CPUClass *cc = CPU_GET_CLASS(cpu); > + if (reg < cc->gdb_num_core_regs) { > + return cc->gdb_read_register(cpu, buf, reg); > + } > + return 0; > +} I'm not super keen on exposing gdb register id's to the plugins. For one thing they can be dynamic and there is no way for the plugins to know what the mapping is. I'd rather abstract the registers in a way that can be cleanly used by HMP, logging and plugins. > + > +uint64_t read_reg(int reg) > +{ > + GByteArray *val = g_byte_array_new(); > + uint64_t reg_ret = 0; > + int ret_bytes = plugin_read_register(current_cpu, val, reg); > + if (ret_bytes == 1) { > + reg_ret = val->data[0]; > + } > + if (ret_bytes == 2) { > + reg_ret = *(uint16_t *) &(val->data[0]); > + } > + if (ret_bytes == 4) { > + reg_ret = *(uint32_t *) &(val->data[0]); > + } > + if (ret_bytes == 8) { > + reg_ret = *(uint64_t *) &(val->data[0]); > + } > + return reg_ret; > +} There are larger registers than 8 bytes - for example the various vector implementations. They need to be accessible via this interface. > + > +void write_reg(int reg, uint64_t val) > +{ > + CPUState *cpu = current_cpu; > + CPUClass *cc = CPU_GET_CLASS(cpu); > + > + if (reg < cc->gdb_num_core_regs) { > + cc->gdb_write_register(cpu, (uint8_t *) &val, reg); > + } > +} > + > +void plugin_single_step(int enable) > +{ > + /* singlestep is set in softmmu/vl.c*/ > + static int orig_value; > + static int executed = 1; > + > + if (unlikely(executed == 1)) { > + orig_value = singlestep; > + executed = 2; > + } > + > + if (enable == 1) { > + singlestep = 1; > + } else { > + singlestep = orig_value; > + } > + > + tb_flush(current_cpu); > +} Again why? What does this gain over instrumenting instructions themselves?
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h index e6e815abc5..c7a0c5f379 100644 --- a/include/qemu/qemu-plugin.h +++ b/include/qemu/qemu-plugin.h @@ -577,4 +577,39 @@ int qemu_plugin_n_max_vcpus(void); */ void qemu_plugin_outs(const char *string); + +/** + * read_reg() read a register + * @reg: Number of the register + * + * Returns the value of the register + */ +uint64_t read_reg(int reg); + +/** + * write_reg() - write to a register + * @reg: number of the register + * @val: value written to register + */ +void write_reg(int reg, uint64_t val); + +/** + * plugin_flush_tb() - Flush the tb cache + */ +void plugin_flush_tb(void); + +/** + * plugin_rw_memory_cpu() - Function to read from and write to a guest address. + * @address: baseaddress of the memory section + * @buffer: buffer managed by caller the value should be written to + * @buf_size: size of the buffer and memory size read/written. + * @write: 1 if write, 0 if read + */ +int plugin_rw_memory_cpu(uint64_t address, uint8_t buffer[], size_t buf_size, char write); + +/** + * plugin_single_step() - Function to change single step behaviour from the plugin. + */ +void plugin_single_step(int enable); + #endif /* QEMU_PLUGIN_API_H */ diff --git a/plugins/meson.build b/plugins/meson.build index e77723010e..b95cbab0b1 100644 --- a/plugins/meson.build +++ b/plugins/meson.build @@ -10,4 +10,5 @@ specific_ss.add(when: 'CONFIG_PLUGIN', if_true: [files( 'loader.c', 'core.c', 'api.c', + 'readwriteextension.c', ), declare_dependency(link_args: plugin_ldflags)]) diff --git a/plugins/readwriteextension.c b/plugins/readwriteextension.c new file mode 100644 index 0000000000..47460c396f --- /dev/null +++ b/plugins/readwriteextension.c @@ -0,0 +1,106 @@ +/** + * QEMU Plugin read write extension code + * + * This is the code that allows the plugin to read and write + * memory and registers and flush the tb cache. Also allows + * to set QEMU into singlestep mode from Plugin. + * + * Based on plugin interface: + * Copyright (C) 2017, Emilio G. Cota <cota@braap.org> + * Copyright (C) 2019, Linaro + * + * Copyright (C) 2021 Florian Hauschild <florian.hauschild@tum.de> + * + * License: GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + + + +#include "qemu/osdep.h" +#include "qemu/plugin.h" +#include "hw/core/cpu.h" +#include "cpu.h" +#include "exec/exec-all.h" + +void plugin_async_flush_tb(CPUState *cpu, run_on_cpu_data arg); +void plugin_async_flush_tb(CPUState *cpu, run_on_cpu_data arg) +{ + g_assert(cpu_in_exclusive_context(cpu)); + tb_flush(cpu); +} + + + +int plugin_rw_memory_cpu(uint64_t address, uint8_t buffer[], size_t buf_size, char write) +{ + return cpu_memory_rw_debug(current_cpu, address, buffer, buf_size, write); + +} + + +void plugin_flush_tb(void) +{ + async_safe_run_on_cpu(current_cpu, plugin_async_flush_tb, RUN_ON_CPU_NULL); +} + +static int plugin_read_register(CPUState *cpu, GByteArray *buf, int reg) +{ + CPUClass *cc = CPU_GET_CLASS(cpu); + if (reg < cc->gdb_num_core_regs) { + return cc->gdb_read_register(cpu, buf, reg); + } + return 0; +} + +uint64_t read_reg(int reg) +{ + GByteArray *val = g_byte_array_new(); + uint64_t reg_ret = 0; + int ret_bytes = plugin_read_register(current_cpu, val, reg); + if (ret_bytes == 1) { + reg_ret = val->data[0]; + } + if (ret_bytes == 2) { + reg_ret = *(uint16_t *) &(val->data[0]); + } + if (ret_bytes == 4) { + reg_ret = *(uint32_t *) &(val->data[0]); + } + if (ret_bytes == 8) { + reg_ret = *(uint64_t *) &(val->data[0]); + } + return reg_ret; +} + +void write_reg(int reg, uint64_t val) +{ + CPUState *cpu = current_cpu; + CPUClass *cc = CPU_GET_CLASS(cpu); + + if (reg < cc->gdb_num_core_regs) { + cc->gdb_write_register(cpu, (uint8_t *) &val, reg); + } +} + +void plugin_single_step(int enable) +{ + /* singlestep is set in softmmu/vl.c*/ + static int orig_value; + static int executed = 1; + + if (unlikely(executed == 1)) { + orig_value = singlestep; + executed = 2; + } + + if (enable == 1) { + singlestep = 1; + } else { + singlestep = orig_value; + } + + tb_flush(current_cpu); +}
This extension covers functions: * to read and write guest memory * to read and write guest registers * to flush tb cache * to control single stepping of qemu from plugin These changes allow the user to * collect more information about the behaviour of the system * change the guest state with a plugin during execution * control cache of tcg * allow for precise instrumentation in execution flow Signed-off-by: Florian Hauschild <florian.hauschild@fs.ei.tum.de> --- include/qemu/qemu-plugin.h | 35 ++++++++++++ plugins/meson.build | 1 + plugins/readwriteextension.c | 106 +++++++++++++++++++++++++++++++++++ 3 files changed, 142 insertions(+) create mode 100644 plugins/readwriteextension.c