qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/1] plugins: add API to read guest CPU memory from hwaddr
@ 2024-08-28  6:32 Rowan Hart
  2024-08-28  6:32 ` [PATCH 1/1] " Rowan Hart
  2024-09-05 15:26 ` [PATCH 0/1] " Alex Bennée
  0 siblings, 2 replies; 8+ messages in thread
From: Rowan Hart @ 2024-08-28  6:32 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alexandre Iooss, Alex Bennée, Pierrick Bouvier,
	Mahmoud Mandour, Rowan Hart

This patch adds a single API function which allows reading from a guest
CPU physical address.

I don't know of a good way to add a self-contained test for this feature
to tests/tcg/plugins, but I did come up with a small test case to
demonstrate the functionality using peiyuanix/riscv-os:

First, grab and build the firmware code:

curl -o firmware.S https://raw.githubusercontent.com/peiyuanix/riscv-os/main/03-Bare-Metal-Hello-RISC-V/firmware.s
curl -o firmware.x https://raw.githubusercontent.com/peiyuanix/riscv-os/main/03-Bare-Metal-Hello-RISC-V/firmware.ld
riscv64-linux-gnu-as firmware.S -o firmware.o
riscv64-linux-gnu-ld -T firmware.x -o firmare firmware.o
riscv64-linux-gnu-objcopy -O binary -S firmware firmware.bin

Next, grab and build the plugin (just dumps from phys address on first
instruction executed):

curl -o dump-riscv-firmware.c https://gist.githubusercontent.com/novafacing/5abc08052fab671a0fb26547810b4c55/raw/33772d614d6e36eae30e3405af34f149d7cc608b/dump-riscv-firmware.c
gcc -rdynamic -shared -fPIC -Iinclude/qemu $(pkg-config --cflags --libs glib-2.0) -o libdump-riscv-firmware.so dump-riscv-firmware.c

Finally, run the plugin:

qemu-system-riscv64 -display none -machine virt -serial stdio -bios firmware.bin -plugin $(pwd)libdump-riscv-firmware.so -d plugin

This outputs as expected -- the hexdump of the running firmware:

b7 01 00 10 a3 80 01 00 93 02 00 08 a3 81 51 00  | ..............Q.
93 02 50 00 23 80 51 00 93 02 00 00 a3 80 51 00  | ..P.#.Q.......Q.
93 02 30 00 a3 81 51 00 93 02 10 00 23 81 51 00  | ..0...Q.....#.Q.
23 82 01 00 83 82 51 00 83 82 01 00 a3 83 01 00  | #.....Q.........
93 02 80 04 23 80 51 00 93 02 50 06 23 80 51 00  | ....#.Q...P.#.Q.
93 02 c0 06 23 80 51 00 93 02 c0 06 23 80 51 00  | ....#.Q.....#.Q.
93 02 f0 06 23 80 51 00 93 02 c0 02 23 80 51 00  | ....#.Q.....#.Q.
93 02 00 02 23 80 51 00 93 02 20 05 23 80 51 00  | ....#.Q.....#.Q.
93 02 90 04 23 80 51 00 93 02 30 05 23 80 51 00  | ....#.Q...0.#.Q.
93 02 30 04 23 80 51 00 93 02 d0 02 23 80 51 00  | ..0.#.Q.....#.Q.
93 02 60 05 23 80 51 00 93 02 10 02 23 80 51 00  | ..`.#.Q.....#.Q.
93 02 a0 00 23 80 51 00 6f 00 00 00 00 00 00 00  | ....#.Q.o.......
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  | ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  | ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  | ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  | ................
Hello, RISC-V!

Rowan Hart (1):
  plugins: add API to read guest CPU memory from hwaddr

 include/qemu/qemu-plugin.h   | 22 ++++++++++++++++++++++
 plugins/api.c                | 17 +++++++++++++++++
 plugins/qemu-plugins.symbols |  2 ++
 3 files changed, 41 insertions(+)

-- 
2.46.0



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/1] plugins: add API to read guest CPU memory from hwaddr
  2024-08-28  6:32 [PATCH 0/1] plugins: add API to read guest CPU memory from hwaddr Rowan Hart
@ 2024-08-28  6:32 ` Rowan Hart
  2024-08-28 14:41   ` Rowan Hart
                     ` (2 more replies)
  2024-09-05 15:26 ` [PATCH 0/1] " Alex Bennée
  1 sibling, 3 replies; 8+ messages in thread
From: Rowan Hart @ 2024-08-28  6:32 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alexandre Iooss, Alex Bennée, Pierrick Bouvier,
	Mahmoud Mandour, Rowan Hart

Signed-off-by: Rowan Hart <rowanbhart@gmail.com>
---
 include/qemu/qemu-plugin.h   | 22 ++++++++++++++++++++++
 plugins/api.c                | 17 +++++++++++++++++
 plugins/qemu-plugins.symbols |  2 ++
 3 files changed, 41 insertions(+)

diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index c71c705b69..25f39c0960 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -868,6 +868,28 @@ QEMU_PLUGIN_API
 int qemu_plugin_read_register(struct qemu_plugin_register *handle,
                               GByteArray *buf);
 
+/**
+ * qemu_plugin_read_cpu_memory_hwaddr() - read CPU memory from hwaddr
+ *
+ * @addr: A virtual 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.
+ *
+ * Returns true on success and false on failure.
+ */
+QEMU_PLUGIN_API
+bool qemu_plugin_read_cpu_memory_hwaddr(uint64_t addr,
+                                          GByteArray *data, size_t len);
+
 /**
  * qemu_plugin_scoreboard_new() - alloc a new scoreboard
  *
diff --git a/plugins/api.c b/plugins/api.c
index 2ff13d09de..c87bed6641 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -527,6 +527,22 @@ GArray *qemu_plugin_get_registers(void)
     return create_register_handles(regs);
 }
 
+bool qemu_plugin_read_cpu_memory_hwaddr(uint64_t addr,
+                                        GByteArray *data, uint64_t len)
+{
+#ifndef CONFIG_USER_ONLY
+    if (len == 0) {
+        return false;
+    }
+
+    g_byte_array_set_size(data, len);
+    cpu_physical_memory_rw(addr, (void *)data->data, len, 0);
+    return true;
+#else
+    return false;
+#endif
+}
+
 int qemu_plugin_read_register(struct qemu_plugin_register *reg, GByteArray *buf)
 {
     g_assert(current_cpu);
@@ -534,6 +550,7 @@ int qemu_plugin_read_register(struct qemu_plugin_register *reg, GByteArray *buf)
     return gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) - 1);
 }
 
+
 struct qemu_plugin_scoreboard *qemu_plugin_scoreboard_new(size_t element_size)
 {
     return plugin_scoreboard_new(element_size);
diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols
index ca773d8d9f..5d9cfd71bb 100644
--- a/plugins/qemu-plugins.symbols
+++ b/plugins/qemu-plugins.symbols
@@ -20,6 +20,8 @@
   qemu_plugin_num_vcpus;
   qemu_plugin_outs;
   qemu_plugin_path_to_binary;
+  qemu_plugin_read_cpu_memory_hwaddr;
+  qemu_plugin_read_io_memory_hwaddr;
   qemu_plugin_read_register;
   qemu_plugin_register_atexit_cb;
   qemu_plugin_register_flush_cb;
-- 
2.46.0



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] plugins: add API to read guest CPU memory from hwaddr
  2024-08-28  6:32 ` [PATCH 1/1] " Rowan Hart
@ 2024-08-28 14:41   ` Rowan Hart
  2024-08-30 19:30   ` Pierrick Bouvier
  2025-01-09 11:38   ` Alex Bennée
  2 siblings, 0 replies; 8+ messages in thread
From: Rowan Hart @ 2024-08-28 14:41 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alexandre Iooss, Alex Bennée, Pierrick Bouvier,
	Mahmoud Mandour

[-- Attachment #1: Type: text/plain, Size: 265 bytes --]

> +  qemu_plugin_read_cpu_memory_hwaddr;
> +  qemu_plugin_read_io_memory_hwaddr;

This second symbol name should be removed, I initially wanted to implement
for IO as well but there is no good generic way I can see to access a list
of IO AddressSpace to read from.

[-- Attachment #2: Type: text/html, Size: 348 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] plugins: add API to read guest CPU memory from hwaddr
  2024-08-28  6:32 ` [PATCH 1/1] " Rowan Hart
  2024-08-28 14:41   ` Rowan Hart
@ 2024-08-30 19:30   ` Pierrick Bouvier
  2024-08-30 19:33     ` Pierrick Bouvier
  2025-01-09 11:38   ` Alex Bennée
  2 siblings, 1 reply; 8+ messages in thread
From: Pierrick Bouvier @ 2024-08-30 19:30 UTC (permalink / raw)
  To: Rowan Hart, qemu-devel; +Cc: Alexandre Iooss, Alex Bennée, Mahmoud Mandour

Hi Rowan,

thanks for this good complement on the virt address read function.

However, to be able to merge a new plugins API function, we must have a 
concrete usage of it, through one of the existing plugin.
What could be a good demonstration of value brought by being able to 
read a physical address?

Thanks,
Pierrick

On 8/27/24 23:32, Rowan Hart wrote:
> Signed-off-by: Rowan Hart <rowanbhart@gmail.com>
> ---
>   include/qemu/qemu-plugin.h   | 22 ++++++++++++++++++++++
>   plugins/api.c                | 17 +++++++++++++++++
>   plugins/qemu-plugins.symbols |  2 ++
>   3 files changed, 41 insertions(+)
> 
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index c71c705b69..25f39c0960 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -868,6 +868,28 @@ QEMU_PLUGIN_API
>   int qemu_plugin_read_register(struct qemu_plugin_register *handle,
>                                 GByteArray *buf);
>   
> +/**
> + * qemu_plugin_read_cpu_memory_hwaddr() - read CPU memory from hwaddr
> + *
> + * @addr: A virtual 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.
> + *
> + * Returns true on success and false on failure.
> + */
> +QEMU_PLUGIN_API
> +bool qemu_plugin_read_cpu_memory_hwaddr(uint64_t addr,
> +                                          GByteArray *data, size_t len);
> +
>   /**
>    * qemu_plugin_scoreboard_new() - alloc a new scoreboard
>    *
> diff --git a/plugins/api.c b/plugins/api.c
> index 2ff13d09de..c87bed6641 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -527,6 +527,22 @@ GArray *qemu_plugin_get_registers(void)
>       return create_register_handles(regs);
>   }
>   
> +bool qemu_plugin_read_cpu_memory_hwaddr(uint64_t addr,
> +                                        GByteArray *data, uint64_t len)
> +{
> +#ifndef CONFIG_USER_ONLY
> +    if (len == 0) {
> +        return false;
> +    }
> +
> +    g_byte_array_set_size(data, len);
> +    cpu_physical_memory_rw(addr, (void *)data->data, len, 0);
> +    return true;
> +#else
> +    return false;
> +#endif
> +}
> +
>   int qemu_plugin_read_register(struct qemu_plugin_register *reg, GByteArray *buf)
>   {
>       g_assert(current_cpu);
> @@ -534,6 +550,7 @@ int qemu_plugin_read_register(struct qemu_plugin_register *reg, GByteArray *buf)
>       return gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) - 1);
>   }
>   
> +
>   struct qemu_plugin_scoreboard *qemu_plugin_scoreboard_new(size_t element_size)
>   {
>       return plugin_scoreboard_new(element_size);
> diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols
> index ca773d8d9f..5d9cfd71bb 100644
> --- a/plugins/qemu-plugins.symbols
> +++ b/plugins/qemu-plugins.symbols
> @@ -20,6 +20,8 @@
>     qemu_plugin_num_vcpus;
>     qemu_plugin_outs;
>     qemu_plugin_path_to_binary;
> +  qemu_plugin_read_cpu_memory_hwaddr;
> +  qemu_plugin_read_io_memory_hwaddr;

As you mentioned, you can remove the second one for v2.

>     qemu_plugin_read_register;
>     qemu_plugin_register_atexit_cb;
>     qemu_plugin_register_flush_cb;


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] plugins: add API to read guest CPU memory from hwaddr
  2024-08-30 19:30   ` Pierrick Bouvier
@ 2024-08-30 19:33     ` Pierrick Bouvier
  0 siblings, 0 replies; 8+ messages in thread
From: Pierrick Bouvier @ 2024-08-30 19:33 UTC (permalink / raw)
  To: Rowan Hart, qemu-devel; +Cc: Alexandre Iooss, Alex Bennée, Mahmoud Mandour

And by the way, feel free to integrate this with your other series (as 
it's a very similar topic) in a v3, so we can review both at the same time.

Thanks,
Pierrick

On 8/30/24 12:30, Pierrick Bouvier wrote:
> Hi Rowan,
> 
> thanks for this good complement on the virt address read function.
> 
> However, to be able to merge a new plugins API function, we must have a
> concrete usage of it, through one of the existing plugin.
> What could be a good demonstration of value brought by being able to
> read a physical address?
> 
> Thanks,
> Pierrick
> 
> On 8/27/24 23:32, Rowan Hart wrote:
>> Signed-off-by: Rowan Hart <rowanbhart@gmail.com>
>> ---
>>    include/qemu/qemu-plugin.h   | 22 ++++++++++++++++++++++
>>    plugins/api.c                | 17 +++++++++++++++++
>>    plugins/qemu-plugins.symbols |  2 ++
>>    3 files changed, 41 insertions(+)
>>
>> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
>> index c71c705b69..25f39c0960 100644
>> --- a/include/qemu/qemu-plugin.h
>> +++ b/include/qemu/qemu-plugin.h
>> @@ -868,6 +868,28 @@ QEMU_PLUGIN_API
>>    int qemu_plugin_read_register(struct qemu_plugin_register *handle,
>>                                  GByteArray *buf);
>>    
>> +/**
>> + * qemu_plugin_read_cpu_memory_hwaddr() - read CPU memory from hwaddr
>> + *
>> + * @addr: A virtual 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.
>> + *
>> + * Returns true on success and false on failure.
>> + */
>> +QEMU_PLUGIN_API
>> +bool qemu_plugin_read_cpu_memory_hwaddr(uint64_t addr,
>> +                                          GByteArray *data, size_t len);
>> +
>>    /**
>>     * qemu_plugin_scoreboard_new() - alloc a new scoreboard
>>     *
>> diff --git a/plugins/api.c b/plugins/api.c
>> index 2ff13d09de..c87bed6641 100644
>> --- a/plugins/api.c
>> +++ b/plugins/api.c
>> @@ -527,6 +527,22 @@ GArray *qemu_plugin_get_registers(void)
>>        return create_register_handles(regs);
>>    }
>>    
>> +bool qemu_plugin_read_cpu_memory_hwaddr(uint64_t addr,
>> +                                        GByteArray *data, uint64_t len)
>> +{
>> +#ifndef CONFIG_USER_ONLY
>> +    if (len == 0) {
>> +        return false;
>> +    }
>> +
>> +    g_byte_array_set_size(data, len);
>> +    cpu_physical_memory_rw(addr, (void *)data->data, len, 0);
>> +    return true;
>> +#else
>> +    return false;
>> +#endif
>> +}
>> +
>>    int qemu_plugin_read_register(struct qemu_plugin_register *reg, GByteArray *buf)
>>    {
>>        g_assert(current_cpu);
>> @@ -534,6 +550,7 @@ int qemu_plugin_read_register(struct qemu_plugin_register *reg, GByteArray *buf)
>>        return gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) - 1);
>>    }
>>    
>> +
>>    struct qemu_plugin_scoreboard *qemu_plugin_scoreboard_new(size_t element_size)
>>    {
>>        return plugin_scoreboard_new(element_size);
>> diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols
>> index ca773d8d9f..5d9cfd71bb 100644
>> --- a/plugins/qemu-plugins.symbols
>> +++ b/plugins/qemu-plugins.symbols
>> @@ -20,6 +20,8 @@
>>      qemu_plugin_num_vcpus;
>>      qemu_plugin_outs;
>>      qemu_plugin_path_to_binary;
>> +  qemu_plugin_read_cpu_memory_hwaddr;
>> +  qemu_plugin_read_io_memory_hwaddr;
> 
> As you mentioned, you can remove the second one for v2.
> 
>>      qemu_plugin_read_register;
>>      qemu_plugin_register_atexit_cb;
>>      qemu_plugin_register_flush_cb;


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/1] plugins: add API to read guest CPU memory from hwaddr
  2024-08-28  6:32 [PATCH 0/1] plugins: add API to read guest CPU memory from hwaddr Rowan Hart
  2024-08-28  6:32 ` [PATCH 1/1] " Rowan Hart
@ 2024-09-05 15:26 ` Alex Bennée
  2024-09-18  5:23   ` Rowan Hart
  1 sibling, 1 reply; 8+ messages in thread
From: Alex Bennée @ 2024-09-05 15:26 UTC (permalink / raw)
  To: Rowan Hart; +Cc: qemu-devel, Alexandre Iooss, Pierrick Bouvier, Mahmoud Mandour

Rowan Hart <rowanbhart@gmail.com> writes:

> This patch adds a single API function which allows reading from a guest
> CPU physical address.
>
> I don't know of a good way to add a self-contained test for this feature
> to tests/tcg/plugins, but I did come up with a small test case to
> demonstrate the functionality using peiyuanix/riscv-os:

We have bare metal system tests (hello and memory) for i386, alpha,
loongarch64, aarch64 and arm. If you fancy having a go at implementing a
boot.S for riscv64 that would be super helpful for the check-tcg tests
as a whole.

See:

  tests/tcg/i386/system/boot.S
  tests/tcg/alpha/system/boot.S
  tests/tcg/loongarch64/system/boot.S
  tests/tcg/aarch64/system/boot.S
  tests/tcg/x86_64/system/boot.S
  tests/tcg/arm/system/boot.S

for what is needed (basically a MMU-enabled flat memory map and some
sort of emit char helper, probably using semihosting in this case)

>
> First, grab and build the firmware code:
>
> curl -o firmware.S https://raw.githubusercontent.com/peiyuanix/riscv-os/main/03-Bare-Metal-Hello-RISC-V/firmware.s
> curl -o firmware.x https://raw.githubusercontent.com/peiyuanix/riscv-os/main/03-Bare-Metal-Hello-RISC-V/firmware.ld
> riscv64-linux-gnu-as firmware.S -o firmware.o
> riscv64-linux-gnu-ld -T firmware.x -o firmare firmware.o
> riscv64-linux-gnu-objcopy -O binary -S firmware firmware.bin
>
> Next, grab and build the plugin (just dumps from phys address on first
> instruction executed):
>
> curl -o dump-riscv-firmware.c https://gist.githubusercontent.com/novafacing/5abc08052fab671a0fb26547810b4c55/raw/33772d614d6e36eae30e3405af34f149d7cc608b/dump-riscv-firmware.c
> gcc -rdynamic -shared -fPIC -Iinclude/qemu $(pkg-config --cflags --libs glib-2.0) -o libdump-riscv-firmware.so dump-riscv-firmware.c
>
<snip>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/1] plugins: add API to read guest CPU memory from hwaddr
  2024-09-05 15:26 ` [PATCH 0/1] " Alex Bennée
@ 2024-09-18  5:23   ` Rowan Hart
  0 siblings, 0 replies; 8+ messages in thread
From: Rowan Hart @ 2024-09-18  5:23 UTC (permalink / raw)
  To: Alex Bennée
  Cc: qemu-devel, Alexandre Iooss, Pierrick Bouvier, Mahmoud Mandour

> 
> See:
> 
>   tests/tcg/i386/system/boot.S
>   tests/tcg/alpha/system/boot.S
>   tests/tcg/loongarch64/system/boot.S
>   tests/tcg/aarch64/system/boot.S
>   tests/tcg/x86_64/system/boot.S
>   tests/tcg/arm/system/boot.S
> 
> for what is needed (basically a MMU-enabled flat memory map and some
> sort of emit char helper, probably using semihosting in this case)

Sounds good! Sorry for the long pause, had some stuff going on :)

Investigating this now! I've never messed with semihosting, should be fun.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/1] plugins: add API to read guest CPU memory from hwaddr
  2024-08-28  6:32 ` [PATCH 1/1] " Rowan Hart
  2024-08-28 14:41   ` Rowan Hart
  2024-08-30 19:30   ` Pierrick Bouvier
@ 2025-01-09 11:38   ` Alex Bennée
  2 siblings, 0 replies; 8+ messages in thread
From: Alex Bennée @ 2025-01-09 11:38 UTC (permalink / raw)
  To: Rowan Hart; +Cc: qemu-devel, Alexandre Iooss, Pierrick Bouvier, Mahmoud Mandour

Rowan Hart <rowanbhart@gmail.com> writes:

Apologies for the delay, I realise this has been sitting on my review
queue too long.

> Signed-off-by: Rowan Hart <rowanbhart@gmail.com>
> ---
>  include/qemu/qemu-plugin.h   | 22 ++++++++++++++++++++++
>  plugins/api.c                | 17 +++++++++++++++++
>  plugins/qemu-plugins.symbols |  2 ++
>  3 files changed, 41 insertions(+)
>
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index c71c705b69..25f39c0960 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -868,6 +868,28 @@ QEMU_PLUGIN_API
>  int qemu_plugin_read_register(struct qemu_plugin_register *handle,
>                                GByteArray *buf);
>  
> +/**
> + * qemu_plugin_read_cpu_memory_hwaddr() - read CPU memory from hwaddr
> + *
> + * @addr: A virtual address to read from

A physical address

> + * @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.

Maybe we should be clear this can only be called from a vCPU context?
See bellow.

> + *
> + * Returns true on success and false on failure.
> + */
> +QEMU_PLUGIN_API
> +bool qemu_plugin_read_cpu_memory_hwaddr(uint64_t addr,
> +                                          GByteArray *data, size_t len);
> +
>  /**
>   * qemu_plugin_scoreboard_new() - alloc a new scoreboard
>   *
> diff --git a/plugins/api.c b/plugins/api.c
> index 2ff13d09de..c87bed6641 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -527,6 +527,22 @@ GArray *qemu_plugin_get_registers(void)
>      return create_register_handles(regs);
>  }
>  
> +bool qemu_plugin_read_cpu_memory_hwaddr(uint64_t addr,
> +                                        GByteArray *data, uint64_t len)
> +{
> +#ifndef CONFIG_USER_ONLY
> +    if (len == 0) {
> +        return false;
> +    }
> +
> +    g_byte_array_set_size(data, len);
> +    cpu_physical_memory_rw(addr, (void *)data->data, len, 0);

One concern here is that cpu_physical_memory_* swallows any error
conditions so the user might not get what they are expecting even if we
return true here.

It would be safer I think to use address_space_read_full with
&address_space_memory and MEMTXATTRS_UNSPECIFIED and check the result so
we can signal to the user when it fails.

However that does gloss over some details because you can have multiple
address spaces. As each vCPU can potentially have its own maybe we want:

  g_assert(current_cpu);
  res = address_space_read_full(current_cpu->as, addr, attrs, buf, len);
  return res == MEMTX_OK ? true : false;

However even that elides a complexity because a CPU can have multiple
AddressSpaces depending on what mode it is in.

For example Arm can have normal, secure and potentially normal and
secure versions of tag memory. Currently we have no way of exposing that
information to plugins (and maybe we don't want to, currently
arm_addressspace() is only used for internal page table walking and some
stack stuff).

x86 has two potential address spaces with the extra one being used for
I think System Management Mode.


> +    return true;
> +#else
> +    return false;
> +#endif
> +}
> +
>  int qemu_plugin_read_register(struct qemu_plugin_register *reg, GByteArray *buf)
>  {
>      g_assert(current_cpu);
> @@ -534,6 +550,7 @@ int qemu_plugin_read_register(struct qemu_plugin_register *reg, GByteArray *buf)
>      return gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) - 1);
>  }
>  
> +
>  struct qemu_plugin_scoreboard *qemu_plugin_scoreboard_new(size_t element_size)
>  {
>      return plugin_scoreboard_new(element_size);
> diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols
> index ca773d8d9f..5d9cfd71bb 100644
> --- a/plugins/qemu-plugins.symbols
> +++ b/plugins/qemu-plugins.symbols
> @@ -20,6 +20,8 @@
>    qemu_plugin_num_vcpus;
>    qemu_plugin_outs;
>    qemu_plugin_path_to_binary;
> +  qemu_plugin_read_cpu_memory_hwaddr;
> +  qemu_plugin_read_io_memory_hwaddr;
>    qemu_plugin_read_register;
>    qemu_plugin_register_atexit_cb;
>    qemu_plugin_register_flush_cb;

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2025-01-09 11:39 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-28  6:32 [PATCH 0/1] plugins: add API to read guest CPU memory from hwaddr Rowan Hart
2024-08-28  6:32 ` [PATCH 1/1] " Rowan Hart
2024-08-28 14:41   ` Rowan Hart
2024-08-30 19:30   ` Pierrick Bouvier
2024-08-30 19:33     ` Pierrick Bouvier
2025-01-09 11:38   ` Alex Bennée
2024-09-05 15:26 ` [PATCH 0/1] " Alex Bennée
2024-09-18  5:23   ` Rowan Hart

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).