Re: [PATCH v3 52/87] linux-user/s390x: Expand target_elf_gregset_t

[Peter Maydell <peter.maydell@linaro.org>]

On Thu, 28 Aug 2025 at 13:14, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Make use of the fact that target_elf_gregset_t is a proper structure.
> This lets us drop the ugly cast to uint32_t* in the middle.
>
> Drop ELF_NREG, target_elf_greg_t, and tswapreg.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/s390x/target_elf.h | 12 +++++++-----
>  linux-user/s390x/elfload.c    | 28 +++++++---------------------
>  2 files changed, 14 insertions(+), 26 deletions(-)
>
> diff --git a/linux-user/s390x/target_elf.h b/linux-user/s390x/target_elf.h
> index b7d863ee66..670c7b3eed 100644
> --- a/linux-user/s390x/target_elf.h
> +++ b/linux-user/s390x/target_elf.h
> @@ -8,15 +8,17 @@
>  #ifndef S390X_TARGET_ELF_H
>  #define S390X_TARGET_ELF_H
>
> +#include "target_ptrace.h"
> +
>  #define HAVE_ELF_HWCAP          1
>  #define HAVE_ELF_CORE_DUMP      1
>
> -typedef abi_ulong target_elf_greg_t;
> -
> -/* See linux kernel: arch/s390/include/uapi/asm/ptrace.h (s390_regs).  */
> -#define ELF_NREG                27
> +/*
> + * See linux kernel: arch/s390/include/asm/elf.h, where
> + * elf_gregset_t is typedef'd to struct s390_regs.
> + */
>  typedef struct target_elf_gregset_t {
> -    target_elf_greg_t regs[ELF_NREG];
> +    struct target_s390_regs pt;
>  } target_elf_gregset_t;
>
>  #endif
> diff --git a/linux-user/s390x/elfload.c b/linux-user/s390x/elfload.c
> index 4113273b72..27109279e2 100644
> --- a/linux-user/s390x/elfload.c
> +++ b/linux-user/s390x/elfload.c
> @@ -68,29 +68,15 @@ const char *elf_hwcap_str(uint32_t bit)
>      return bit < ARRAY_SIZE(hwcap_str) ? hwcap_str[bit] : NULL;
>  }
>
> -#define tswapreg(ptr)   tswapal(ptr)
> -
> -enum {
> -    TARGET_REG_PSWM = 0,
> -    TARGET_REG_PSWA = 1,
> -    TARGET_REG_GPRS = 2,
> -    TARGET_REG_ARS = 18,
> -    TARGET_REG_ORIG_R2 = 26,
> -};
> -
>  void elf_core_copy_regs(target_elf_gregset_t *r, const CPUS390XState *env)
>  {
> -    int i;
> -    uint32_t *aregs;
> -
> -    r->regs[TARGET_REG_PSWM] = tswapreg(env->psw.mask);
> -    r->regs[TARGET_REG_PSWA] = tswapreg(env->psw.addr);
> -    for (i = 0; i < 16; i++) {
> -        r->regs[TARGET_REG_GPRS + i] = tswapreg(env->regs[i]);
> +    r->pt.psw.mask = tswapal(env->psw.mask);
> +    r->pt.psw.addr = tswapal(env->psw.addr);
> +    for (int i = 0; i < 16; i++) {
> +        r->pt.gprs[i] = tswapal(env->regs[i]);
>      }
> -    aregs = (uint32_t *)&(r->regs[TARGET_REG_ARS]);
> -    for (i = 0; i < 16; i++) {
> -        aregs[i] = tswap32(env->aregs[i]);
> +    for (int i = 0; i < 16; i++) {
> +        r->pt.acrs[i] = tswap32(env->aregs[i]);
>      }

This cleans up that suspicious cast, which is nice.

> -    r->regs[TARGET_REG_ORIG_R2] = 0;
> +    r->pt.orig_gpr2 = 0;
>  }
> -

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM