>> diff --git a/plugins/api.c b/plugins/api.c
>> index eac04cc1f6..fc19bdb40b 100644
>> --- a/plugins/api.c
>> +++ b/plugins/api.c
>> @@ -41,6 +41,7 @@
>> #include "qemu/log.h"
>> #include "system/memory.h"
>> #include "tcg/tcg.h"
>> +#include "exec/cpu-common.h"
>> #include "exec/gdbstub.h"
>> #include "exec/target_page.h"
>> #include "exec/translation-block.h"
>> @@ -450,13 +451,27 @@ int qemu_plugin_write_register(struct
>> qemu_plugin_register *reg,
>> {
>> g_assert(current_cpu);
>>
>> - if (buf->len == 0 || qemu_plugin_get_cb_flags() !=
>> QEMU_PLUGIN_CB_RW_REGS) {
>> + if (buf->len == 0 || (
>> + qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS
>> + && qemu_plugin_get_cb_flags() !=
>> QEMU_PLUGIN_CB_RW_REGS_PC)) {
>> return -1;
>> }
>
> If we are exposing a specific qemu_plugin_set_pc we should probably
> forbid setting it via write_register. Can we filter out the PC from the
> register list?
The qemu_plugin_write_register API silently swallows writes to the PC
even though such a write doesn't actually have an effect, so excluding
the PC here might make sense and I'm happy to update the patch
accordingly.
Are there other registers that should be excluded as well?
General-purpose register writes are visible to the guest immediately,
but what about special registers? Do writes to those actually always
have the intended effect or should they also be excluded here?
>> return gdb_write_register(current_cpu, buf->data, GPOINTER_TO_INT(reg)
>> - 1);
>> }
>>
>> +void qemu_plugin_set_pc(uint64_t vaddr)
>> +{
>> + g_assert(current_cpu);
>> +
>> + if (qemu_plugin_get_cb_flags() != QEMU_PLUGIN_CB_RW_REGS_PC) {
>> + return;
>> + }
>
> Given we aggressively assert that some functions are called in the
> current_cpu context maybe we should do the same here? If the plugin
> tries to set the PC and it doesn't work what is going to happen?
Could you elaborate on that? I've added the g_assert(current_cpu) as in
other API functions, but I'm not sure what you mean beyond that.
As highlighted in the doc string (see below), the function already only
returns in case of errors, so the caller could just handle the error
case in code after the call to the API function.
I originally thought about adding a boolean return value to indicate
success or failure but given that the function only returns in case of
error, I considered that to be enough of a signal to the caller here.
>> +/**
>> + * qemu_plugin_set_pc() - set the program counter for the current vCPU
>> + *
>> + * @vaddr: the new virtual (guest) address for the program counter
>> + *
>> + * This function sets the program counter for the current vCPU to @vaddr and
>> + * resumes execution at that address. This function only returns in case of
>> + * errors.
>> + */
>> +QEMU_PLUGIN_API
>> +void qemu_plugin_set_pc(uint64_t vaddr);
Thanks,
Florian
