From: novafacing <rowanbhart@gmail.com>
Signed-off-by: novafacing <rowanbhart@gmail.com>
Signed-off-by: Rowan Hart <rowanbhart@gmail.com>
---
include/qemu/qemu-plugin.h | 96 +++++++++++++++++++++++++++++++++++
plugins/api.c | 100 +++++++++++++++++++++++++++++++++++++
2 files changed, 196 insertions(+)
diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index eff8430b4a..d4f229abd9 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -1014,6 +1014,102 @@ QEMU_PLUGIN_API
bool qemu_plugin_write_memory_vaddr(uint64_t addr,
GByteArray *data);
+/**
+ * enum qemu_plugin_hwaddr_operation_result - result of a memory operation
+ *
+ * @QEMU_PLUGIN_HWADDR_OPERATION_OK: hwaddr operation succeeded
+ * @QEMU_PLUGIN_HWADDR_OPERATION_ERROR: unexpected error occurred
+ * @QEMU_PLUGIN_HWADDR_OPERATION_DEVICE_ERROR: error in memory device
+ * @QEMU_PLUGIN_HWADDR_OPERATION_ACCESS_DENIED: permission error
+ * @QEMU_PLUGIN_HWADDR_OPERATION_INVALID_ADDRESS: address was invalid
+ * @QEMU_PLUGIN_HWADDR_OPERATION_INVALID_ADDRESS_SPACE: invalid address space
+ */
+enum qemu_plugin_hwaddr_operation_result {
+ QEMU_PLUGIN_HWADDR_OPERATION_OK,
+ QEMU_PLUGIN_HWADDR_OPERATION_ERROR,
+ QEMU_PLUGIN_HWADDR_OPERATION_DEVICE_ERROR,
+ QEMU_PLUGIN_HWADDR_OPERATION_ACCESS_DENIED,
+ QEMU_PLUGIN_HWADDR_OPERATION_INVALID_ADDRESS,
+ QEMU_PLUGIN_HWADDR_OPERATION_INVALID_ADDRESS_SPACE,
+};
+
+/**
+ * qemu_plugin_read_memory_hwaddr() - read from memory using a hardware address
+ *
+ * @as_idx: The index of the address space to read from
+ * @addr: A physical address to read from
+ * @data: A byte array to store data into
+ * @len: The number of bytes to read, starting from @addr
+ *
+ * @len bytes of data is read starting at @addr and stored into @data. If @data
+ * is not large enough to hold @len bytes, it will be expanded to the necessary
+ * size, reallocating if necessary. @len must be greater than 0.
+ *
+ * This function does not ensure writes are flushed prior to reading, so
+ * callers should take care when calling this function in plugin callbacks to
+ * avoid attempting to read data which may not yet be written and should use
+ * the memory callback API instead.
+ *
+ * This function is only valid for softmmu targets.
+ *
+ * Returns a qemu_plugin_hwaddr_operation_result indicating the result of the
+ * operation.
+ */
+QEMU_PLUGIN_API
+enum qemu_plugin_hwaddr_operation_result
+qemu_plugin_read_memory_hwaddr(unsigned int as_idx, uint64_t addr,
+ GByteArray *data, size_t len);
+
+/**
+ * qemu_plugin_write_memory_hwaddr() - write to memory using a hardware address
+ *
+ * @as_idx: The index of the address space to write to
+ * @addr: A physical address to write to
+ * @data: A byte array containing the data to write
+ *
+ * The contents of @data will be written to memory starting at the hardware
+ * address @addr.
+ *
+ * This function does not guarantee consistency of writes, nor does it ensure
+ * that pending writes are flushed either before or after the write takes place,
+ * so callers should take care when calling this function in plugin callbacks to
+ * avoid depending on the existence of data written using this function which
+ * may be overwritten afterward. In addition, this function requires that the
+ * pages containing the address are not locked. Practically, this means that you
+ * should not write instruction memory in a current translation block inside a
+ * callback registered with qemu_plugin_register_vcpu_tb_trans_cb.
+ *
+ * You can, for example, write instruction memory in a current translation block
+ * in a callback registered with qemu_plugin_register_vcpu_tb_exec_cb, although
+ * be aware that the write will not be flushed until after the translation block
+ * has finished executing. In general, this function should be used to write
+ * data memory or to patch code at a known address, not in a current translation
+ * block.
+ *
+ * This function is only valid for softmmu targets.
+ *
+ * Returns a qemu_plugin_hwaddr_operation_result indicating the result of the
+ * operation.
+ */
+QEMU_PLUGIN_API
+enum qemu_plugin_hwaddr_operation_result
+qemu_plugin_write_memory_hwaddr(unsigned int as_idx, uint64_t addr,
+ GByteArray *data);
+
+/**
+ * qemu_plugin_translate_vaddr() - translate a virtual address to a physical one
+ *
+ * @vaddr: virtual address to translate
+ * @hwaddr: pointer to store the physical address
+ *
+ * This function is only valid in vCPU context (i.e. in callbacks) and is only
+ * valid for softmmu targets.
+ *
+ * Returns true on success and false on failure.
+ */
+QEMU_PLUGIN_API
+bool qemu_plugin_translate_vaddr(uint64_t vaddr, uint64_t *hwaddr);
+
/**
* qemu_plugin_scoreboard_new() - alloc a new scoreboard
*
diff --git a/plugins/api.c b/plugins/api.c
index 19c10bb39e..5983768783 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -569,6 +569,106 @@ bool qemu_plugin_write_memory_vaddr(uint64_t addr, GByteArray *data)
return true;
}
+enum qemu_plugin_hwaddr_operation_result
+qemu_plugin_read_memory_hwaddr(unsigned int as_idx, hwaddr addr,
+ GByteArray *data, size_t len)
+{
+#ifdef CONFIG_SOFTMMU
+ CPUState *cpu = current_cpu;
+
+ if (len == 0 || as_idx >= cpu->cpu_ases_count) {
+ return QEMU_PLUGIN_HWADDR_OPERATION_INVALID_ADDRESS_SPACE;
+ }
+
+ g_byte_array_set_size(data, len);
+
+ AddressSpace *as = cpu_get_address_space(cpu, as_idx);
+
+ if (as == NULL) {
+ return QEMU_PLUGIN_HWADDR_OPERATION_INVALID_ADDRESS_SPACE;
+ }
+
+ MemTxResult res = address_space_rw(as, addr,
+ MEMTXATTRS_UNSPECIFIED, data->data,
+ data->len, false);
+
+ switch (res) {
+ case MEMTX_OK:
+ return QEMU_PLUGIN_HWADDR_OPERATION_OK;
+ case MEMTX_ERROR:
+ return QEMU_PLUGIN_HWADDR_OPERATION_DEVICE_ERROR;
+ case MEMTX_DECODE_ERROR:
+ return QEMU_PLUGIN_HWADDR_OPERATION_INVALID_ADDRESS;
+ case MEMTX_ACCESS_ERROR:
+ return QEMU_PLUGIN_HWADDR_OPERATION_ACCESS_DENIED;
+ default:
+ return QEMU_PLUGIN_HWADDR_OPERATION_ERROR;
+ }
+#else
+ return QEMU_PLUGIN_HWADDR_OPERATION_ERROR;
+#endif
+}
+
+enum qemu_plugin_hwaddr_operation_result
+qemu_plugin_write_memory_hwaddr(unsigned int as_idx, hwaddr addr,
+ GByteArray *data)
+{
+#ifdef CONFIG_SOFTMMU
+ CPUState *cpu = current_cpu;
+
+ if (data->len == 0 || as_idx >= cpu->cpu_ases_count) {
+ return QEMU_PLUGIN_HWADDR_OPERATION_INVALID_ADDRESS_SPACE;
+ }
+
+ AddressSpace *as = cpu_get_address_space(cpu, as_idx);
+
+ if (as == NULL) {
+ return QEMU_PLUGIN_HWADDR_OPERATION_INVALID_ADDRESS_SPACE;
+ }
+
+ qemu_plugin_outs("Got cpu address space...\n");
+
+ MemTxResult res = address_space_rw(as, addr,
+ MEMTXATTRS_UNSPECIFIED, data->data,
+ data->len, true);
+ switch (res) {
+ case MEMTX_OK:
+ return QEMU_PLUGIN_HWADDR_OPERATION_OK;
+ case MEMTX_ERROR:
+ return QEMU_PLUGIN_HWADDR_OPERATION_DEVICE_ERROR;
+ case MEMTX_DECODE_ERROR:
+ return QEMU_PLUGIN_HWADDR_OPERATION_INVALID_ADDRESS;
+ case MEMTX_ACCESS_ERROR:
+ return QEMU_PLUGIN_HWADDR_OPERATION_ACCESS_DENIED;
+ default:
+ return QEMU_PLUGIN_HWADDR_OPERATION_ERROR;
+ }
+#else
+ return QEMU_PLUGIN_HWADDR_OPERATION_ERROR;
+#endif
+}
+
+bool qemu_plugin_translate_vaddr(uint64_t vaddr, uint64_t *hwaddr)
+{
+#ifdef CONFIG_SOFTMMU
+ g_assert(current_cpu);
+
+ CPUState *cpu = current_cpu;
+
+ uint64_t res = cpu_get_phys_page_debug(cpu, vaddr);
+
+ if (res == (uint64_t)-1) {
+ return false;
+ }
+
+ *hwaddr = res | (vaddr & ~TARGET_PAGE_MASK);
+
+ return true;
+#else
+ return false;
+#endif
+}
+
struct qemu_plugin_scoreboard *qemu_plugin_scoreboard_new(size_t element_size)
{
return plugin_scoreboard_new(element_size);
--
2.49.0Hi Rowan,
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index eff8430b4a..d4f229abd9 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -1014,6 +1014,102 @@ QEMU_PLUGIN_API
> bool qemu_plugin_write_memory_vaddr(uint64_t addr,
> GByteArray *data);
>
> <snip>
> +/**
> + * qemu_plugin_translate_vaddr() - translate a virtual address to a physical one
> + *
> + * @vaddr: virtual address to translate
> + * @hwaddr: pointer to store the physical address
> + *
> + * This function is only valid in vCPU context (i.e. in callbacks) and is only
> + * valid for softmmu targets.
> + *
> + * Returns true on success and false on failure.
> + */
> +QEMU_PLUGIN_API
> +bool qemu_plugin_translate_vaddr(uint64_t vaddr, uint64_t *hwaddr);
As a (testing) plugin author, I'm quite interested in this function in
particular. For the next iteration, would you be willing to split this
one out into its own patch? This would enhance the chance of it getting
picked up even if other parts of the series should not make it.
> diff --git a/plugins/api.c b/plugins/api.c
> index 19c10bb39e..5983768783 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -569,6 +569,106 @@ bool qemu_plugin_write_memory_vaddr(uint64_t addr, GByteArray *data)
> <snip>
> +bool qemu_plugin_translate_vaddr(uint64_t vaddr, uint64_t *hwaddr)
> +{
> +#ifdef CONFIG_SOFTMMU
> + g_assert(current_cpu);
> +
> + CPUState *cpu = current_cpu;
> +
> + uint64_t res = cpu_get_phys_page_debug(cpu, vaddr);
> +
> + if (res == (uint64_t)-1) {
> + return false;
> + }
> +
> + *hwaddr = res | (vaddr & ~TARGET_PAGE_MASK);
> +
> + return true;
> +#else
> + return false;
> +#endif
> +}
This definition strikes me as odd. What was your reason to assert
`current_cpu` here, but not in the other two functions? Also a bit
surprising is the declaration of `cpu` if you use it in just one place
(rather than just use `current_cpu` directly as for the assertion).
And there is no reason in particular why the vCPU could not be a
function parameter of `qemu_plugin_translate_vaddr`, right? You don't
have the same restrictions as in `qemu_plugin_read_memory_hwaddr` or
`qemu_plugin_hwaddr_operation_result` where you actually touch memory?
Regards,
Julian
On 5/22/25 4:59 AM, Julian Ganz wrote: > This definition strikes me as odd. What was your reason to assert > `current_cpu` here, but not in the other two functions? Also a bit > surprising is the declaration of `cpu` if you use it in just one place > (rather than just use `current_cpu` directly as for the assertion). > > And there is no reason in particular why the vCPU could not be a > function parameter of `qemu_plugin_translate_vaddr`, right? You don't > have the same restrictions as in `qemu_plugin_read_memory_hwaddr` or > `qemu_plugin_hwaddr_operation_result` where you actually touch memory? > That's a good point, adding a "unsigned int vcpu_index" to the signature should be enough to query current or any other vcpu easily. > Regards, > Julian
> > > > This definition strikes me as odd. What was your reason to assert > > `current_cpu` here, but not in the other two functions? Also a bit > > surprising is the declaration of `cpu` if you use it in just one place > > (rather than just use `current_cpu` directly as for the assertion). > > > > And there is no reason in particular why the vCPU could not be a > > function parameter of `qemu_plugin_translate_vaddr`, right? You don't > > have the same restrictions as in `qemu_plugin_read_memory_hwaddr` or > > `qemu_plugin_hwaddr_operation_result` where you actually touch memory? > > > > That's a good point, adding a "unsigned int vcpu_index" to the signature > should be enough to query current or any other vcpu easily. > This is a really nice idea, it might be nice to make a vcpu version of read/write register too. For memory, I'd think going with the current memory is probably fine, I don't see any configs with different memory per vcpu? >
On 5/22/25 2:01 PM, Rowan Hart wrote: > > > This definition strikes me as odd. What was your reason to assert > > `current_cpu` here, but not in the other two functions? Also a bit > > surprising is the declaration of `cpu` if you use it in just one > place > > (rather than just use `current_cpu` directly as for the assertion). > > > > And there is no reason in particular why the vCPU could not be a > > function parameter of `qemu_plugin_translate_vaddr`, right? You don't > > have the same restrictions as in `qemu_plugin_read_memory_hwaddr` or > > `qemu_plugin_hwaddr_operation_result` where you actually touch > memory? > > > > That's a good point, adding a "unsigned int vcpu_index" to the > signature > should be enough to query current or any other vcpu easily. > > This is a really nice idea, it might be nice to make a vcpu version of > read/write register too. For memory, I'd think going with the current > memory is probably fine, I don't see any configs with different memory > per vcpu? > Thinking about it twice, and after reading Alex comments for writing registers, it's probably not a good idea to allow such side effects on other vcpus (on registers and memory). In case of registers, there is nothing ensuring they will be written correctly, so it only makes sense for current_cpu, as we are in a callback running on it. Safer to have the same semantic for memory read/write also, so please forget my idea.
On 5/21/25 2:43 AM, Rowan Hart wrote: > From: novafacing <rowanbhart@gmail.com> > > Signed-off-by: novafacing <rowanbhart@gmail.com> > Signed-off-by: Rowan Hart <rowanbhart@gmail.com> > --- > include/qemu/qemu-plugin.h | 96 +++++++++++++++++++++++++++++++++++ > plugins/api.c | 100 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 196 insertions(+) Reading this patch, and patch 3 (Add address space API), I am not sure AddressSpace is something we want to leak in plugins interface. It is a concept *very* internal to QEMU, and not reflecting directly something concerning the emulated architecture (it is related, but not officially described for it). The same way qemu_plugin_write_memory_vaddr is only valid in the current page table setup, we could assume the same for current address space, and return an error if memory is not mapped with current AS. Eventually, we could read/write a given hwaddr in all existing address spaces (starting with current mapped one), if it makes sense to do this, which I'm not sure about. What are your thoughts on this? qemu_plugin_translate_vaddr is fine for me.
Well, first I just noticed that I left a debug print in this function! So I'll fix that. > Reading this patch, and patch 3 (Add address space API), I am not sure > AddressSpace is something we want to leak in plugins interface. > It is a concept *very* internal to QEMU, and not reflecting directly > something concerning the emulated architecture (it is related, but not > officially described for it). > > The same way qemu_plugin_write_memory_vaddr is only valid in the > current page table setup, we could assume the same for current address > space, and return an error if memory is not mapped with current AS. > Eventually, we could read/write a given hwaddr in all existing address > spaces (starting with current mapped one), if it makes sense to do > this, which I'm not sure about. > > What are your thoughts on this? I definitely see the arguments for not exposing it even as an opaque struct, internality not withstanding it also adds some complexity for plugin authors. My thought with exposing it is kind of twofold. First, there are specific address spaces like the secure address space on ARM or I/O memory on x86 that plugins might like to access and I think it's easiest to facilitate that if we just let them choose which one they want to r/w to. Second, I wanted to lean towards being less restrictive now to avoid having to go back and remove restrictions later since even though it's very internal, it doesn't seem very likely to change. That said, if you think it's more trouble than it's worth I'm totally fine with just defaulting to writing to the current AS (or to cpu-memory, whichever's more reasonable). Your call, just let me know which way you think is best for v4 :) > qemu_plugin_translate_vaddr is fine for me. I did have a question about this -- one of the test plugins prints out vaddr, haddr from qemu_plugin_insn_haddr, and the translated haddr from qemu_plugin_translate_vaddr. When running with direct memory mappings in a system test, the vaddr = translated haddr, which is correct, but the haddr from qemu_plugin_insn_haddr was incorrect (it was 0x7f....f<actual address>). Is this expected behavior? Thanks for the feedback!
On 5/21/25 8:34 PM, Rowan Hart wrote:
> Well, first I just noticed that I left a debug print in this function!
> So I'll fix that.
>
>> Reading this patch, and patch 3 (Add address space API), I am not sure
>> AddressSpace is something we want to leak in plugins interface.
>> It is a concept *very* internal to QEMU, and not reflecting directly
>> something concerning the emulated architecture (it is related, but not
>> officially described for it).
>>
>> The same way qemu_plugin_write_memory_vaddr is only valid in the
>> current page table setup, we could assume the same for current address
>> space, and return an error if memory is not mapped with current AS.
>> Eventually, we could read/write a given hwaddr in all existing address
>> spaces (starting with current mapped one), if it makes sense to do
>> this, which I'm not sure about.
>>
>> What are your thoughts on this?
>
> I definitely see the arguments for not exposing it even as an opaque
> struct, internality not withstanding it also adds some complexity for
> plugin authors.
>
> My thought with exposing it is kind of twofold. First, there are
> specific address spaces like the secure address space on ARM or I/O
> memory on x86 that plugins might like to access and I think it's easiest
> to facilitate that if we just let them choose which one they want to r/w
> to. Second, I wanted to lean towards being less restrictive now to avoid
> having to go back and remove restrictions later since even though it's
> very internal, it doesn't seem very likely to change.
>
> That said, if you think it's more trouble than it's worth I'm totally
> fine with just defaulting to writing to the current AS (or to
> cpu-memory, whichever's more reasonable). Your call, just let me know
> which way you think is best for v4 :)
I understand your point, but to the opposite of registers, I think we
should refrain from exposing all this.
For now, we can just use the current AS.
Later, we could consider to add a new architecture specific parameter
for that need (being a union, with fields per base architecture). So
people writing architecture specific plugins can have a solution.
AddressSpace as = {.arm = ADDRESS_SPACE_ARM_SECURE};
qemu_plugin_read_memory_hwaddr_as(addr, data, as);
>> qemu_plugin_translate_vaddr is fine for me.
> I did have a question about this -- one of the test plugins prints out
> vaddr, haddr from qemu_plugin_insn_haddr, and the translated haddr from
> qemu_plugin_translate_vaddr. When running with direct memory mappings in
> a system test, the vaddr = translated haddr, which is correct, but the
> haddr from qemu_plugin_insn_haddr was incorrect (it was 0x7f....f<actual
> address>). Is this expected behavior?
>
qemu_plugin_insn_haddr returns directly a pointer to instruction in
(host) memory, which is different from hwaddr, thus the void* signature.
Name is pretty confusing though, qemu_plugin_insn_ptr could have been a
better name.
> Thanks for the feedback!
>
>
© 2016 - 2025 Red Hat, Inc.