diff mbox series

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

Message ID 20210807214242.82385-35-imp@bsdimp.com (mailing list archive)
State New, archived
Headers show
Series bsd-user updates to run hello world | expand

Commit Message

Warner Losh Aug. 7, 2021, 9:42 p.m. UTC 
From: Warner Losh <imp@FreeBSD.org>

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 <kevans@FreeBSD.org>
Signed-off-by: Warner Losh <imp@bsdimp.com>
Signed-off-by: Stacey Son <sson@FreeBSD.org>
---
 bsd-user/main.c | 42 ++++--------------------------------------
 bsd-user/qemu.h |  1 +
 2 files changed, 5 insertions(+), 38 deletions(-)

Comments

Richard Henderson Aug. 10, 2021, 3:02 p.m. UTC | #1 
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 = &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 <richard.henderson@linaro.org>


r~
Warner Losh Aug. 10, 2021, 10:28 p.m. UTC | #2 
On Tue, Aug 10, 2021 at 9:03 AM Richard Henderson <
richard.henderson@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 = &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 <richard.henderson@linaro.org>
>

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

Warner
diff mbox series

Patch

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 = &regs1;
     struct image_info info1, *info = &info1;
     struct bsd_binprm bprm;
-    TaskState ts1, *ts = &ts1;
+    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", &tmp) == 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,
-                    &bprm) != 0) {
+    if (loader_exec(filename, argv+optind, target_environ, regs, info, &bprm)) {
         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 = &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;