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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 10/12/24 11:03, Peter Maydell wrote:
> On Wed, 27 Nov 2024 at 19:20, Philippe Mathieu-Daudé <philmd@linaro.org> 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);
> 
> I'm not a huge fan of this, because cpu_physical_memory_write()
> is one of those old APIs that it would be nice to see less
> use of, not more. Ideally anything issuing memory transactions
> should know what it's issuing them to, i.e. should be using
> address_space_* functions and passing an AddressSpace.

I totally agree with you. I'm chasing one problem at a time
starting by first_cpu, and you are already seeing ahead :)

Do you mind posting a documentation patch clarifying the
cpu_physical_memory_*() methods we want to deprecate?

> If you don't want to use first_cpu, then you could use
> address_space_write(address_space_memory, ...), which is
> what cpu_physical_memory_write() is doing under the hood.
> The qtest protocol assumes a single address space anyway.

Correct, good idea.

Next problem I have here is to understand what 'endianness'
means for QTest framework. Use case: heterogeneous ZynqMP
with ARM and MicroBlaze cores.

Thanks,

Phil.