Initialize all 3 fields in main(), rather than in 4 different
target-specific functions.  Adjust the ifdef to be function
rather than target specific.  Include stack_base in the ifdef.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/qemu.h             |  4 ++--
 linux-user/aarch64/cpu_loop.c |  8 --------
 linux-user/arm/cpu_loop.c     | 37 +++++++++++++++--------------------
 linux-user/m68k/cpu_loop.c    |  9 ---------
 linux-user/main.c             |  5 +++++
 linux-user/riscv/cpu_loop.c   |  5 -----
 6 files changed, 23 insertions(+), 45 deletions(-)

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 6c97ab221f..dff7767bc8 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -122,12 +122,12 @@ struct TaskState {
 #ifdef TARGET_M68K
     abi_ulong tp_value;
 #endif
-#if defined(TARGET_ARM) || defined(TARGET_M68K) || defined(TARGET_RISCV)
+#ifdef CONFIG_SEMIHOSTING
     /* Extra fields for semihosted binaries.  */
     abi_ulong heap_base;
     abi_ulong heap_limit;
-#endif
     abi_ulong stack_base;
+#endif
     int used; /* non zero if used */
     struct image_info *info;
     struct linux_binprm *bprm;
diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
index fea43cefa6..030a630c93 100644
--- a/linux-user/aarch64/cpu_loop.c
+++ b/linux-user/aarch64/cpu_loop.c
@@ -140,9 +140,6 @@ void cpu_loop(CPUARMState *env)
 void target_cpu_copy_regs(CPUArchState *env, target_pt_regs *regs)
 {
     ARMCPU *cpu = env_archcpu(env);
-    CPUState *cs = env_cpu(env);
-    TaskState *ts = get_task_state(cs);
-    struct image_info *info = ts->info;
     int i;
 
     if (!(arm_feature(env, ARM_FEATURE_AARCH64))) {
@@ -167,9 +164,4 @@ void target_cpu_copy_regs(CPUArchState *env, target_pt_regs *regs)
     if (cpu_isar_feature(aa64_pauth, cpu)) {
         qemu_guest_getrandom_nofail(&env->keys, sizeof(env->keys));
     }
-
-    ts->stack_base = info->start_stack;
-    ts->heap_base = info->brk;
-    /* This will be filled in on the first SYS_HEAPINFO call.  */
-    ts->heap_limit = 0;
 }
diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
index 33f63951a9..1f3bb96484 100644
--- a/linux-user/arm/cpu_loop.c
+++ b/linux-user/arm/cpu_loop.c
@@ -482,30 +482,25 @@ void cpu_loop(CPUARMState *env)
 
 void target_cpu_copy_regs(CPUArchState *env, target_pt_regs *regs)
 {
-    CPUState *cpu = env_cpu(env);
-    TaskState *ts = get_task_state(cpu);
-    struct image_info *info = ts->info;
-    int i;
-
     cpsr_write(env, regs->uregs[16], CPSR_USER | CPSR_EXEC,
                CPSRWriteByInstr);
-    for(i = 0; i < 16; i++) {
+    for (int i = 0; i < 16; i++) {
         env->regs[i] = regs->uregs[i];
     }
-#if TARGET_BIG_ENDIAN
-    /* Enable BE8.  */
-    if (EF_ARM_EABI_VERSION(info->elf_flags) >= EF_ARM_EABI_VER4
-        && (info->elf_flags & EF_ARM_BE8)) {
-        env->uncached_cpsr |= CPSR_E;
-        env->cp15.sctlr_el[1] |= SCTLR_E0E;
-    } else {
-        env->cp15.sctlr_el[1] |= SCTLR_B;
-    }
-    arm_rebuild_hflags(env);
-#endif
 
-    ts->stack_base = info->start_stack;
-    ts->heap_base = info->brk;
-    /* This will be filled in on the first SYS_HEAPINFO call.  */
-    ts->heap_limit = 0;
+    if (TARGET_BIG_ENDIAN) {
+        CPUState *cpu = env_cpu(env);
+        TaskState *ts = get_task_state(cpu);
+        struct image_info *info = ts->info;
+
+        /* Enable BE8.  */
+        if (EF_ARM_EABI_VERSION(info->elf_flags) >= EF_ARM_EABI_VER4
+            && (info->elf_flags & EF_ARM_BE8)) {
+            env->uncached_cpsr |= CPSR_E;
+            env->cp15.sctlr_el[1] |= SCTLR_E0E;
+        } else {
+            env->cp15.sctlr_el[1] |= SCTLR_B;
+        }
+        arm_rebuild_hflags(env);
+    }
 }
diff --git a/linux-user/m68k/cpu_loop.c b/linux-user/m68k/cpu_loop.c
index 5da91b997a..23693f3358 100644
--- a/linux-user/m68k/cpu_loop.c
+++ b/linux-user/m68k/cpu_loop.c
@@ -94,10 +94,6 @@ void cpu_loop(CPUM68KState *env)
 
 void target_cpu_copy_regs(CPUArchState *env, target_pt_regs *regs)
 {
-    CPUState *cpu = env_cpu(env);
-    TaskState *ts = get_task_state(cpu);
-    struct image_info *info = ts->info;
-
     env->pc = regs->pc;
     env->dregs[0] = regs->d0;
     env->dregs[1] = regs->d1;
@@ -116,9 +112,4 @@ void target_cpu_copy_regs(CPUArchState *env, target_pt_regs *regs)
     env->aregs[6] = regs->a6;
     env->aregs[7] = regs->usp;
     env->sr = regs->sr;
-
-    ts->stack_base = info->start_stack;
-    ts->heap_base = info->brk;
-    /* This will be filled in on the first SYS_HEAPINFO call.  */
-    ts->heap_limit = 0;
 }
diff --git a/linux-user/main.c b/linux-user/main.c
index 68972f00a1..4def4be1c1 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -1050,6 +1050,11 @@ int main(int argc, char **argv, char **envp)
 
 #ifdef CONFIG_SEMIHOSTING
     qemu_semihosting_guestfd_init();
+
+    ts->stack_base = info->start_stack;
+    ts->heap_base = info->brk;
+    /* This will be filled in on the first SYS_HEAPINFO call.  */
+    ts->heap_limit = 0;
 #endif
 
     cpu_loop(env);
diff --git a/linux-user/riscv/cpu_loop.c b/linux-user/riscv/cpu_loop.c
index 3ac8bbfec1..2dd30c7b28 100644
--- a/linux-user/riscv/cpu_loop.c
+++ b/linux-user/riscv/cpu_loop.c
@@ -108,9 +108,4 @@ void target_cpu_copy_regs(CPUArchState *env, target_pt_regs *regs)
         error_report("Incompatible ELF: RVE cpu requires RVE ABI binary");
         exit(EXIT_FAILURE);
     }
-
-    ts->stack_base = info->start_stack;
-    ts->heap_base = info->brk;
-    /* This will be filled in on the first SYS_HEAPINFO call.  */
-    ts->heap_limit = 0;
 }
-- 
2.43.0

On Wed, 30 Jul 2025 at 01:48, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Initialize all 3 fields in main(), rather than in 4 different
> target-specific functions.  Adjust the ifdef to be function
> rather than target specific.  Include stack_base in the ifdef.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/qemu.h             |  4 ++--
>  linux-user/aarch64/cpu_loop.c |  8 --------
>  linux-user/arm/cpu_loop.c     | 37 +++++++++++++++--------------------
>  linux-user/m68k/cpu_loop.c    |  9 ---------
>  linux-user/main.c             |  5 +++++
>  linux-user/riscv/cpu_loop.c   |  5 -----
>  6 files changed, 23 insertions(+), 45 deletions(-)
>
> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
> index 6c97ab221f..dff7767bc8 100644
> --- a/linux-user/qemu.h
> +++ b/linux-user/qemu.h
> @@ -122,12 +122,12 @@ struct TaskState {
>  #ifdef TARGET_M68K
>      abi_ulong tp_value;
>  #endif
> -#if defined(TARGET_ARM) || defined(TARGET_M68K) || defined(TARGET_RISCV)
> +#ifdef CONFIG_SEMIHOSTING
>      /* Extra fields for semihosted binaries.  */
>      abi_ulong heap_base;
>      abi_ulong heap_limit;
> -#endif
>      abi_ulong stack_base;
> +#endif
>      int used; /* non zero if used */
>      struct image_info *info;
>      struct linux_binprm *bprm;

> diff --git a/linux-user/main.c b/linux-user/main.c
> index 68972f00a1..4def4be1c1 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -1050,6 +1050,11 @@ int main(int argc, char **argv, char **envp)
>
>  #ifdef CONFIG_SEMIHOSTING
>      qemu_semihosting_guestfd_init();
> +
> +    ts->stack_base = info->start_stack;
> +    ts->heap_base = info->brk;
> +    /* This will be filled in on the first SYS_HEAPINFO call.  */
> +    ts->heap_limit = 0;
>  #endif

Do we need to hide the struct fields and their initialization
behind an ifdef at all? We allocate our TaskState structs on
the heap so we don't care too much about their size, and
the init code here is trivial.

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

thanks
-- PMM

On 8/2/25 02:05, Peter Maydell wrote:
> On Wed, 30 Jul 2025 at 01:48, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> Initialize all 3 fields in main(), rather than in 4 different
>> target-specific functions.  Adjust the ifdef to be function
>> rather than target specific.  Include stack_base in the ifdef.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   linux-user/qemu.h             |  4 ++--
>>   linux-user/aarch64/cpu_loop.c |  8 --------
>>   linux-user/arm/cpu_loop.c     | 37 +++++++++++++++--------------------
>>   linux-user/m68k/cpu_loop.c    |  9 ---------
>>   linux-user/main.c             |  5 +++++
>>   linux-user/riscv/cpu_loop.c   |  5 -----
>>   6 files changed, 23 insertions(+), 45 deletions(-)
>>
>> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
>> index 6c97ab221f..dff7767bc8 100644
>> --- a/linux-user/qemu.h
>> +++ b/linux-user/qemu.h
>> @@ -122,12 +122,12 @@ struct TaskState {
>>   #ifdef TARGET_M68K
>>       abi_ulong tp_value;
>>   #endif
>> -#if defined(TARGET_ARM) || defined(TARGET_M68K) || defined(TARGET_RISCV)
>> +#ifdef CONFIG_SEMIHOSTING
>>       /* Extra fields for semihosted binaries.  */
>>       abi_ulong heap_base;
>>       abi_ulong heap_limit;
>> -#endif
>>       abi_ulong stack_base;
>> +#endif
>>       int used; /* non zero if used */
>>       struct image_info *info;
>>       struct linux_binprm *bprm;
> 
>> diff --git a/linux-user/main.c b/linux-user/main.c
>> index 68972f00a1..4def4be1c1 100644
>> --- a/linux-user/main.c
>> +++ b/linux-user/main.c
>> @@ -1050,6 +1050,11 @@ int main(int argc, char **argv, char **envp)
>>
>>   #ifdef CONFIG_SEMIHOSTING
>>       qemu_semihosting_guestfd_init();
>> +
>> +    ts->stack_base = info->start_stack;
>> +    ts->heap_base = info->brk;
>> +    /* This will be filled in on the first SYS_HEAPINFO call.  */
>> +    ts->heap_limit = 0;
>>   #endif
> 
> Do we need to hide the struct fields and their initialization
> behind an ifdef at all? We allocate our TaskState structs on
> the heap so we don't care too much about their size, and
> the init code here is trivial.

I guess not.  A comment about "this is to support semihosting" would do as well.

Looking again, these ts->heap_* fields look actively wrong, because the real values are in

   static abi_ulong target_brk, initial_target_brk;

in syscall.c.

The original thread's stack_base can always be had via ts->info->start_stack.  I guess the 
semihosting api has no concept of threads and per-thread stacks.


r~