Re: [PATCH v3 38/87] linux-user/arm: Expand target_elf_gregset_t

[Richard Henderson <richard.henderson@linaro.org>]

On 8/29/25 00:47, Peter Maydell wrote:
> On Thu, 28 Aug 2025 at 13:10, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> Make use of the fact that target_elf_gregset_t is a proper structure.
>> Drop ELF_NREG, target_elf_greg_t, and tswapreg.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   linux-user/arm/target_elf.h | 11 +++++++----
>>   linux-user/arm/elfload.c    |  8 +++-----
>>   2 files changed, 10 insertions(+), 9 deletions(-)
>>
>> diff --git a/linux-user/arm/target_elf.h b/linux-user/arm/target_elf.h
>> index 94db3738e8..fa8f8af2f3 100644
>> --- a/linux-user/arm/target_elf.h
>> +++ b/linux-user/arm/target_elf.h
>> @@ -8,16 +8,19 @@
>>   #ifndef ARM_TARGET_ELF_H
>>   #define ARM_TARGET_ELF_H
>>
>> +#include "target_ptrace.h"
>> +
>>   #define HAVE_ELF_HWCAP          1
>>   #define HAVE_ELF_HWCAP2         1
>>   #define HAVE_ELF_PLATFORM       1
>>   #define HAVE_ELF_CORE_DUMP      1
>>
>> -typedef abi_ulong target_elf_greg_t;
>> -
>> -#define ELF_NREG                18
>> +/*
>> + * See linux kernel: arch/arm/include/asm/elf.h, where
>> + * elf_gregset_t is mapped to struct pt_regs via sizeof.
>> + */
>>   typedef struct target_elf_gregset_t {
>> -    target_elf_greg_t regs[ELF_NREG];
>> +    struct target_pt_regs pt;
>>   } target_elf_gregset_t;
>>
>>   #endif
>> diff --git a/linux-user/arm/elfload.c b/linux-user/arm/elfload.c
>> index 47fe16a1a6..726d3ec25c 100644
>> --- a/linux-user/arm/elfload.c
>> +++ b/linux-user/arm/elfload.c
>> @@ -201,13 +201,11 @@ const char *get_elf_platform(CPUState *cs)
>>   #undef END
>>   }
>>
>> -#define tswapreg(ptr)   tswapal(ptr)
>> -
>>   void elf_core_copy_regs(target_elf_gregset_t *r, const CPUARMState *env)
>>   {
>>       for (int i = 0; i < 16; ++i) {
>> -        r->regs[i] = tswapreg(env->regs[i]);
>> +        r->pt.regs[i] = tswapal(env->regs[i]);
>>       }
>> -    r->regs[16] = tswapreg(cpsr_read((CPUARMState *)env));
>> -    r->regs[17] = tswapreg(env->regs[0]); /* XXX */
>> +    r->pt.cpsr = tswapal(cpsr_read((CPUARMState *)env));
>> +    r->pt.orig_r0 = tswapal(env->regs[0]);
> 
> Why is it OK to drop the "XXX" comment here ?

I assumed XXX meant "what is this", and the answer is orig_r0.
I'm not even sure the value is wrong as-is, due to the way we process syscalls.


r~