diff mbox series

[PATCH-for-10.0] system/qtest: Access memory using cpu_physical_memory_rw() API

Message ID 20241127191914.34146-1-philmd@linaro.org (mailing list archive)
State New
Headers show
Series [PATCH-for-10.0] system/qtest: Access memory using cpu_physical_memory_rw() API | expand

Commit Message

Philippe Mathieu-Daudé Nov. 27, 2024, 7:19 p.m. UTC 
There is no vCPU within the QTest accelerator (well, they
are stubs doing nothing, see dummy_cpu_thread_fn).
Directly use the cpu_physical_memory_rw() API -- which
amusingly prefixed 'cpu_' does not use vCPU -- to access
memory. This reduces accesses to the global 'first_cpu'.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 system/qtest.c | 42 ++++++++++++++----------------------------
 1 file changed, 14 insertions(+), 28 deletions(-)

Comments

Richard Henderson Nov. 27, 2024, 10:35 p.m. UTC | #1 
On 11/27/24 13:19, Philippe Mathieu-Daudé wrote:
> There is no vCPU within the QTest accelerator (well, they
> are stubs doing nothing, see dummy_cpu_thread_fn).
> Directly use the cpu_physical_memory_rw() API -- which
> amusingly prefixed 'cpu_' does not use vCPU -- to access
> memory. This reduces accesses to the global 'first_cpu'.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   system/qtest.c | 42 ++++++++++++++----------------------------
>   1 file changed, 14 insertions(+), 28 deletions(-)
> 
> diff --git a/system/qtest.c b/system/qtest.c
> index 12703a20455..a2de9a7d5a4 100644
> --- a/system/qtest.c
> +++ b/system/qtest.c
> @@ -18,6 +18,7 @@
>   #include "chardev/char-fe.h"
>   #include "exec/ioport.h"
>   #include "exec/memory.h"
> +#include "exec/cpu-common.h"
>   #include "exec/tswap.h"
>   #include "hw/qdev-core.h"
>   #include "hw/irq.h"
> @@ -514,23 +515,19 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
>   
>           if (words[0][5] == 'b') {
>               uint8_t data = value;
> -            address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
> -                                &data, 1);
> +            cpu_physical_memory_write(addr, &data, 1);

This just calls address_space_write with &address_space_memory.
I assume we'll get rid of address_space_memory too, at some point.
But I suppose that's good enough for qtest for now.

r~
Philippe Mathieu-Daudé Nov. 28, 2024, 5:56 a.m. UTC | #2 
On 27/11/24 23:35, Richard Henderson wrote:
> On 11/27/24 13:19, Philippe Mathieu-Daudé wrote:
>> There is no vCPU within the QTest accelerator (well, they
>> are stubs doing nothing, see dummy_cpu_thread_fn).
>> Directly use the cpu_physical_memory_rw() API -- which
>> amusingly prefixed 'cpu_' does not use vCPU -- to access
>> memory. This reduces accesses to the global 'first_cpu'.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   system/qtest.c | 42 ++++++++++++++----------------------------
>>   1 file changed, 14 insertions(+), 28 deletions(-)
>>
>> diff --git a/system/qtest.c b/system/qtest.c
>> index 12703a20455..a2de9a7d5a4 100644
>> --- a/system/qtest.c
>> +++ b/system/qtest.c
>> @@ -18,6 +18,7 @@
>>   #include "chardev/char-fe.h"
>>   #include "exec/ioport.h"
>>   #include "exec/memory.h"
>> +#include "exec/cpu-common.h"
>>   #include "exec/tswap.h"
>>   #include "hw/qdev-core.h"
>>   #include "hw/irq.h"
>> @@ -514,23 +515,19 @@ static void qtest_process_command(CharBackend 
>> *chr, gchar **words)
>>           if (words[0][5] == 'b') {
>>               uint8_t data = value;
>> -            address_space_write(first_cpu->as, addr, 
>> MEMTXATTRS_UNSPECIFIED,
>> -                                &data, 1);
>> +            cpu_physical_memory_write(addr, &data, 1);
> 
> This just calls address_space_write with &address_space_memory.
> I assume we'll get rid of address_space_memory too, at some point.

Certainly, but one thing at a time ;) For the first prototype
I'm focusing on being able to instantiate vCPUs from different
target architecture in "any machine we have now" (even if it
doesn't make much sense). I'll then move to HW modelling, likely
starting by vCPU Clusters and then address spaces.

> But I suppose that's good enough for qtest for now.

Little win: less first_cpu accesses. What really bother me in
this file are the endianness mentions (and tswap calls).

> 
> r~
>
diff mbox series

Patch

diff --git a/system/qtest.c b/system/qtest.c
index 12703a20455..a2de9a7d5a4 100644
--- a/system/qtest.c
+++ b/system/qtest.c
@@ -18,6 +18,7 @@ 
 #include "chardev/char-fe.h"
 #include "exec/ioport.h"
 #include "exec/memory.h"
+#include "exec/cpu-common.h"
 #include "exec/tswap.h"
 #include "hw/qdev-core.h"
 #include "hw/irq.h"
@@ -514,23 +515,19 @@  static void qtest_process_command(CharBackend *chr, gchar **words)
 
         if (words[0][5] == 'b') {
             uint8_t data = value;
-            address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
-                                &data, 1);
+            cpu_physical_memory_write(addr, &data, 1);
         } else if (words[0][5] == 'w') {
             uint16_t data = value;
             tswap16s(&data);
-            address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
-                                &data, 2);
+            cpu_physical_memory_write(addr, &data, 2);
         } else if (words[0][5] == 'l') {
             uint32_t data = value;
             tswap32s(&data);
-            address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
-                                &data, 4);
+            cpu_physical_memory_write(addr, &data, 4);
         } else if (words[0][5] == 'q') {
             uint64_t data = value;
             tswap64s(&data);
-            address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
-                                &data, 8);
+            cpu_physical_memory_write(addr, &data, 8);
         }
         qtest_send_prefix(chr);
         qtest_send(chr, "OK\n");
@@ -548,22 +545,18 @@  static void qtest_process_command(CharBackend *chr, gchar **words)
 
         if (words[0][4] == 'b') {
             uint8_t data;
-            address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
-                               &data, 1);
+            cpu_physical_memory_read(addr, &data, 1);
             value = data;
         } else if (words[0][4] == 'w') {
             uint16_t data;
-            address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
-                               &data, 2);
+            cpu_physical_memory_read(addr, &data, 2);
             value = tswap16(data);
         } else if (words[0][4] == 'l') {
             uint32_t data;
-            address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
-                               &data, 4);
+            cpu_physical_memory_read(addr, &data, 4);
             value = tswap32(data);
         } else if (words[0][4] == 'q') {
-            address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
-                               &value, 8);
+            cpu_physical_memory_read(addr, &value, 8);
             tswap64s(&value);
         }
         qtest_send_prefix(chr);
@@ -583,9 +576,7 @@  static void qtest_process_command(CharBackend *chr, gchar **words)
         g_assert(len);
 
         data = g_malloc(len);
-        address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data,
-                           len);
-
+        cpu_physical_memory_read(addr, data, len);
         enc = qemu_hexdump_line(NULL, data, len, 0, 0);
 
         qtest_send_prefix(chr);
@@ -605,8 +596,7 @@  static void qtest_process_command(CharBackend *chr, gchar **words)
         g_assert(ret == 0);
 
         data = g_malloc(len);
-        address_space_read(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data,
-                           len);
+        cpu_physical_memory_read(addr, data, len);
         b64_data = g_base64_encode(data, len);
         qtest_send_prefix(chr);
         qtest_sendf(chr, "OK %s\n", b64_data);
@@ -640,8 +630,7 @@  static void qtest_process_command(CharBackend *chr, gchar **words)
                 data[i] = 0;
             }
         }
-        address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data,
-                            len);
+        cpu_physical_memory_write(addr, data, len);
         g_free(data);
 
         qtest_send_prefix(chr);
@@ -663,8 +652,7 @@  static void qtest_process_command(CharBackend *chr, gchar **words)
         if (len) {
             data = g_malloc(len);
             memset(data, pattern, len);
-            address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED,
-                                data, len);
+            cpu_physical_memory_write(addr, data, len);
             g_free(data);
         }
 
@@ -696,9 +684,7 @@  static void qtest_process_command(CharBackend *chr, gchar **words)
                            len, out_len);
             out_len = MIN(out_len, len);
         }
-
-        address_space_write(first_cpu->as, addr, MEMTXATTRS_UNSPECIFIED, data,
-                            len);
+        cpu_physical_memory_write(addr, data, len);
 
         qtest_send_prefix(chr);
         qtest_send(chr, "OK\n");