Re: [PATCH for 6.2 34/49] bsd-user: Fix initializtion of task state

[Warner Losh]

On Tue, Aug 10, 2021 at 9:03 AM Richard Henderson <
richard.hender...@linaro.org> wrote:

> On 8/7/21 11:42 AM, Warner Losh wrote:
> > @@ -459,21 +435,11 @@ int main(int argc, char **argv)
> >   qemu_log("entry   0x" TARGET_ABI_FMT_lx "\n", info->entry);
> >   }
> >
> > -target_set_brk(info->brk);
> > -syscall_init();
> > -signal_init();
> > -
> > -/*
> > - * Now that we've loaded the binary, GUEST_BASE is fixed.  Delay
> > - * generating the prologue until now so that the prologue can take
> > - * the real value of GUEST_BASE into account.
> > - */
> > -tcg_prologue_init(tcg_ctx);
> > -
> >   /* build Task State */
> > -memset(ts, 0, sizeof(TaskState));
> > +ts = g_new0(TaskState, 1);
> >   init_task_state(ts);
> >   ts->info = info;
> > +ts->bprm = 
> >   cpu->opaque = ts;
> >
> >   target_set_brk(info->brk);
>
> It looks like some of this damage occurs in patch 22
> ("bsd-user: Move per-cpu code into target_arch_cpu.h")
> and could reasonably be squashed back.
>
> Otherwise,
> Reviewed-by: Richard Henderson 
>

I took the easy way and folded them together. Thanks for the tip.

Warner

Re: [PATCH for 6.2 34/49] bsd-user: Fix initializtion of task state

[Richard Henderson]

On 8/7/21 11:42 AM, Warner Losh wrote:

@@ -459,21 +435,11 @@ int main(int argc, char **argv)
  qemu_log("entry   0x" TARGET_ABI_FMT_lx "\n", info->entry);
  }
  
-target_set_brk(info->brk);

-syscall_init();
-signal_init();
-
-/*
- * Now that we've loaded the binary, GUEST_BASE is fixed.  Delay
- * generating the prologue until now so that the prologue can take
- * the real value of GUEST_BASE into account.
- */
-tcg_prologue_init(tcg_ctx);
-
  /* build Task State */
-memset(ts, 0, sizeof(TaskState));
+ts = g_new0(TaskState, 1);
  init_task_state(ts);
  ts->info = info;
+ts->bprm = 
  cpu->opaque = ts;
  
  target_set_brk(info->brk);


It looks like some of this damage occurs in patch 22
("bsd-user: Move per-cpu code into target_arch_cpu.h")
and could reasonably be squashed back.

Otherwise,
Reviewed-by: Richard Henderson 


r~

[PATCH for 6.2 34/49] bsd-user: Fix initializtion of task state

[Warner Losh]

From: Warner Losh 

Fix a number of mismerges in initializing the task state. Save a copy of
bprm in this structure and move it earlier before starting to setup
other state. Remove linux specific procfs access to find minimal vm
address that likely is here through a misguided merge. Remove duplicate
initialization as well.

Signed-off-by: Kyle Evans 
Signed-off-by: Warner Losh 
Signed-off-by: Stacey Son 
---
 bsd-user/main.c | 42 --
 bsd-user/qemu.h |  1 +
 2 files changed, 5 insertions(+), 38 deletions(-)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index 5ca1173f04..b5527537b4 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -205,7 +205,7 @@ int main(int argc, char **argv)
 struct target_pt_regs regs1, *regs = 
 struct image_info info1, *info = 
 struct bsd_binprm bprm;
-TaskState ts1, *ts = 
+TaskState *ts;
 CPUArchState *env;
 CPUState *cpu;
 int optind, rv;
@@ -407,31 +407,7 @@ int main(int argc, char **argv)
  */
 guest_base = HOST_PAGE_ALIGN(guest_base);
 
-/*
- * Read in mmap_min_addr kernel parameter.  This value is used
- * When loading the ELF image to determine whether guest_base
- * is needed.
- *
- * When user has explicitly set the quest base, we skip this
- * test.
- */
-if (!have_guest_base) {
-FILE *fp;
-
-fp = fopen("/proc/sys/vm/mmap_min_addr", "r");
-if (fp != NULL) {
-unsigned long tmp;
-if (fscanf(fp, "%lu", ) == 1) {
-mmap_min_addr = tmp;
-qemu_log_mask(CPU_LOG_PAGE, "host mmap_min_addr=0x%lx\n",
-  mmap_min_addr);
-}
-fclose(fp);
-}
-}
-
-if (loader_exec(filename, argv+optind, target_environ, regs, info,
-) != 0) {
+if (loader_exec(filename, argv+optind, target_environ, regs, info, )) 
{
 printf("Error loading %s\n", filename);
 _exit(1);
 }
@@ -459,21 +435,11 @@ int main(int argc, char **argv)
 qemu_log("entry   0x" TARGET_ABI_FMT_lx "\n", info->entry);
 }
 
-target_set_brk(info->brk);
-syscall_init();
-signal_init();
-
-/*
- * Now that we've loaded the binary, GUEST_BASE is fixed.  Delay
- * generating the prologue until now so that the prologue can take
- * the real value of GUEST_BASE into account.
- */
-tcg_prologue_init(tcg_ctx);
-
 /* build Task State */
-memset(ts, 0, sizeof(TaskState));
+ts = g_new0(TaskState, 1);
 init_task_state(ts);
 ts->info = info;
+ts->bprm = 
 cpu->opaque = ts;
 
 target_set_brk(info->brk);
diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index a22fc19cd6..bfd7b8eaa5 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -90,6 +90,7 @@ typedef struct TaskState {
 pid_t ts_tid; /* tid (or pid) of this task */
 
 struct TaskState *next;
+struct bsd_binprm *bprm;
 int used; /* non zero if used */
 struct image_info *info;
 
-- 
2.32.0