Re: [PATCH 04/10] target/arm: convert 64 bit gdbstub to new helper

[Alex Bennée]

Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:

> On 3/19/25 11:22, Alex Bennée wrote:
>> For some of the helpers we need a temporary variable to copy from
>> although we could add some helpers to return pointers into env in
>> those cases if we wanted to.
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>   target/arm/gdbstub64.c | 53 ++++++++++++++++++++++++++----------------
>>   1 file changed, 33 insertions(+), 20 deletions(-)
>> diff --git a/target/arm/gdbstub64.c b/target/arm/gdbstub64.c
>> index 1a4dbec567..793332af31 100644
>> --- a/target/arm/gdbstub64.c
>> +++ b/target/arm/gdbstub64.c
>> @@ -20,7 +20,7 @@
>>   #include "qemu/log.h"
>>   #include "cpu.h"
>>   #include "internals.h"
>> -#include "gdbstub/helpers.h"
>> +#include "gdbstub/registers.h"
>>   #include "gdbstub/commands.h"
>>   #include "tcg/mte_helper.h"
>>   #if defined(CONFIG_USER_ONLY) && defined(CONFIG_LINUX)
>> @@ -35,15 +35,16 @@ int aarch64_cpu_gdb_read_register(CPUState *cs, 
>> GByteArray *mem_buf, int n)
>>         if (n < 31) {
>>           /* Core integer register.  */
>> -        return gdb_get_reg64(mem_buf, env->xregs[n]);
>> +        return gdb_get_register_value(MO_TEUO, mem_buf, (uint8_t *) 
>> &env->xregs[n]);
>>       }
>>       switch (n) {
>>       case 31:
>> -        return gdb_get_reg64(mem_buf, env->xregs[31]);
>> +        return gdb_get_register_value(MO_TEUO, mem_buf, (uint8_t *) 
>> &env->xregs[31]);
>>       case 32:
>> -        return gdb_get_reg64(mem_buf, env->pc);
>> +        return gdb_get_register_value(MO_TEUO, mem_buf, (uint8_t *) 
>> &env->pc);
>
> For the 3 occurrences, should it be MO_TEUQ instead?

>> -        int vq = sve_vqm1_for_el(env, arm_current_el(env)) + 1;
>> -        return gdb_get_reg64(buf, vq * 2);
>> +        uint64_t vq = (sve_vqm1_for_el(env, arm_current_el(env)) + 1) * 2;
>> +        return gdb_get_register_value(MO_TEUL, buf, (uint8_t *) &vq);
>>       }
>
> Should it be MO_TEUQ instead?

I've taken Philippe's suggestion and used explicit MO_32/64 which is
easier to follow.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro