Merge init_thread and target_cpu_copy_regs.
There's no point going through a target_pt_regs intermediate.
Temporarily introduce HAVE_INIT_MAIN_THREAD during conversion.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/qemu.h          |  1 +
 linux-user/elfload.c       | 29 ++++++-----------------------
 linux-user/i386/cpu_loop.c | 31 ++++++-------------------------
 3 files changed, 13 insertions(+), 48 deletions(-)

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index dff7767bc8..475b11e4c4 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -367,5 +367,6 @@ CPUArchState *cpu_copy(CPUArchState *env);
 
 typedef struct target_pt_regs target_pt_regs;
 void target_cpu_copy_regs(CPUArchState *env, target_pt_regs *regs);
+void init_main_thread(CPUState *cs, struct image_info *info);
 
 #endif /* QEMU_H */
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 1d8cc7f6b5..553d632d46 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -155,17 +155,12 @@ typedef abi_int         target_pid_t;
 
 #ifdef TARGET_I386
 
+#define HAVE_INIT_MAIN_THREAD
+
 #ifdef TARGET_X86_64
 #define ELF_CLASS      ELFCLASS64
 #define ELF_ARCH       EM_X86_64
 
-static inline void init_thread(struct target_pt_regs *regs, struct image_info *infop)
-{
-    regs->rax = 0;
-    regs->rsp = infop->start_stack;
-    regs->rip = infop->entry;
-}
-
 #define ELF_NREG    27
 typedef target_elf_greg_t  target_elf_gregset_t[ELF_NREG];
 
@@ -243,22 +238,6 @@ static bool init_guest_commpage(void)
 
 #define EXSTACK_DEFAULT true
 
-static inline void init_thread(struct target_pt_regs *regs,
-                               struct image_info *infop)
-{
-    regs->esp = infop->start_stack;
-    regs->eip = infop->entry;
-
-    /* SVR4/i386 ABI (pages 3-31, 3-32) says that when the program
-       starts %edx contains a pointer to a function which might be
-       registered using `atexit'.  This provides a mean for the
-       dynamic linker to call DT_FINI functions for shared libraries
-       that have been loaded before the code runs.
-
-       A value of 0 tells we have no such handler.  */
-    regs->edx = 0;
-}
-
 #define ELF_NREG    17
 typedef target_elf_greg_t  target_elf_gregset_t[ELF_NREG];
 
@@ -3637,8 +3616,12 @@ static int elf_core_dump(int signr, const CPUArchState *env)
 
 void do_init_main_thread(CPUState *cs, struct image_info *infop)
 {
+#ifdef HAVE_INIT_MAIN_THREAD
+    init_main_thread(cs, infop);
+#else
     target_pt_regs regs = { };
 
     init_thread(&regs, infop);
     target_cpu_copy_regs(cpu_env(cs), &regs);
+#endif
 }
diff --git a/linux-user/i386/cpu_loop.c b/linux-user/i386/cpu_loop.c
index d96d5553fa..d60462172e 100644
--- a/linux-user/i386/cpu_loop.c
+++ b/linux-user/i386/cpu_loop.c
@@ -331,11 +331,10 @@ static void target_cpu_free(void *obj)
     g_free(obj);
 }
 
-void target_cpu_copy_regs(CPUArchState *env, target_pt_regs *regs)
+void init_main_thread(CPUState *cpu, struct image_info *info)
 {
-    CPUState *cpu = env_cpu(env);
+    CPUArchState *env = cpu_env(cpu);
     bool is64 = (env->features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM) != 0;
-    int i;
 
     OBJECT(cpu)->free = target_cpu_free;
     env->cr[0] = CR0_PG_MASK | CR0_WP_MASK | CR0_PE_MASK;
@@ -362,27 +361,9 @@ void target_cpu_copy_regs(CPUArchState *env, target_pt_regs *regs)
     env->eflags |= IF_MASK;
 
     /* linux register setup */
-#ifndef TARGET_ABI32
-    env->regs[R_EAX] = regs->rax;
-    env->regs[R_EBX] = regs->rbx;
-    env->regs[R_ECX] = regs->rcx;
-    env->regs[R_EDX] = regs->rdx;
-    env->regs[R_ESI] = regs->rsi;
-    env->regs[R_EDI] = regs->rdi;
-    env->regs[R_EBP] = regs->rbp;
-    env->regs[R_ESP] = regs->rsp;
-    env->eip = regs->rip;
-#else
-    env->regs[R_EAX] = regs->eax;
-    env->regs[R_EBX] = regs->ebx;
-    env->regs[R_ECX] = regs->ecx;
-    env->regs[R_EDX] = regs->edx;
-    env->regs[R_ESI] = regs->esi;
-    env->regs[R_EDI] = regs->edi;
-    env->regs[R_EBP] = regs->ebp;
-    env->regs[R_ESP] = regs->esp;
-    env->eip = regs->eip;
-#endif
+    memset(env->regs, 0, sizeof(env->regs));
+    env->regs[R_ESP] = info->start_stack;
+    env->eip = info->entry;
 
     /* linux interrupt setup */
 #ifndef TARGET_ABI32
@@ -394,7 +375,7 @@ void target_cpu_copy_regs(CPUArchState *env, target_pt_regs *regs)
                                 PROT_READ|PROT_WRITE,
                                 MAP_ANONYMOUS|MAP_PRIVATE, -1, 0);
     idt_table = g2h_untagged(env->idt.base);
-    for (i = 0; i < 20; i++) {
+    for (int i = 0; i < 20; i++) {
         set_idt(i, 0, is64);
     }
     set_idt(3, 3, is64);
-- 
2.43.0

On Wed, 30 Jul 2025 at 01:19, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Merge init_thread and target_cpu_copy_regs.
> There's no point going through a target_pt_regs intermediate.
> Temporarily introduce HAVE_INIT_MAIN_THREAD during conversion.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/qemu.h          |  1 +
>  linux-user/elfload.c       | 29 ++++++-----------------------
>  linux-user/i386/cpu_loop.c | 31 ++++++-------------------------
>  3 files changed, 13 insertions(+), 48 deletions(-)

> -static inline void init_thread(struct target_pt_regs *regs,
> -                               struct image_info *infop)
> -{
> -    regs->esp = infop->start_stack;
> -    regs->eip = infop->entry;
> -
> -    /* SVR4/i386 ABI (pages 3-31, 3-32) says that when the program
> -       starts %edx contains a pointer to a function which might be
> -       registered using `atexit'.  This provides a mean for the
> -       dynamic linker to call DT_FINI functions for shared libraries
> -       that have been loaded before the code runs.
> -
> -       A value of 0 tells we have no such handler.  */

This seems like a useful comment to retain -- it's nice
to know whether we're zeroing a register as an ABI requirement
versus just being tidy.

> -    regs->edx = 0;
> -}

> +    memset(env->regs, 0, sizeof(env->regs));

Are we not allowed to assume the regs are zero out of reset ?

> +    env->regs[R_ESP] = info->start_stack;
> +    env->eip = info->entry;

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

thanks
-- PMM

On 8/2/25 02:17, Peter Maydell wrote:
>> -static inline void init_thread(struct target_pt_regs *regs,
>> -                               struct image_info *infop)
>> -{
>> -    regs->esp = infop->start_stack;
>> -    regs->eip = infop->entry;
>> -
>> -    /* SVR4/i386 ABI (pages 3-31, 3-32) says that when the program
>> -       starts %edx contains a pointer to a function which might be
>> -       registered using `atexit'.  This provides a mean for the
>> -       dynamic linker to call DT_FINI functions for shared libraries
>> -       that have been loaded before the code runs.
>> -
>> -       A value of 0 tells we have no such handler.  */
> 
> This seems like a useful comment to retain -- it's nice
> to know whether we're zeroing a register as an ABI requirement
> versus just being tidy.

Fair.

>> +    memset(env->regs, 0, sizeof(env->regs));
> 
> Are we not allowed to assume the regs are zero out of reset ?

For whatever reason, x86 doesn't do that.
Eliding this line causes failures.

I think the SVR4 comment applies to x86_64 as well.  Yep, glibc/sysdeps/x86_64/start.S 
contains the same comment, although the linux kernel source does not.  The kernel simply 
zeros all registers regardless of normal (x64) vs compat (x32).

I'll move the one comment and expand it.


r~