[PULL 10/24] bsd-user: style tweak: use C not C++ comments

[PULL 10/24] bsd-user: style tweak: use C not C++ comments

[imp]

From: Warner Losh <imp@bsdimp.com>

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/qemu.h | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index b836b603af..3480db890d 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -71,7 +71,7 @@ struct image_info {
 
 struct sigqueue {
     struct sigqueue *next;
-    //target_siginfo_t info;
+    /* target_siginfo_t info; */
 };
 
 struct emulated_sigtable {
@@ -193,9 +193,11 @@ extern int do_strace;
 /* signal.c */
 void process_pending_signals(CPUArchState *cpu_env);
 void signal_init(void);
-//int queue_signal(CPUArchState *env, int sig, target_siginfo_t *info);
-//void host_to_target_siginfo(target_siginfo_t *tinfo, const siginfo_t *info);
-//void target_to_host_siginfo(siginfo_t *info, const target_siginfo_t *tinfo);
+/*
+ * int queue_signal(CPUArchState *env, int sig, target_siginfo_t *info);
+ * void host_to_target_siginfo(target_siginfo_t *tinfo, const siginfo_t *info);
+ * void target_to_host_siginfo(siginfo_t *info, const target_siginfo_t *tinfo);
+ */
 long do_sigreturn(CPUArchState *env);
 long do_rt_sigreturn(CPUArchState *env);
 abi_long do_sigaltstack(abi_ulong uss_addr, abi_ulong uoss_addr, abi_ulong sp);
-- 
2.22.1

[PULL 11/24] bsd-user: style tweak: if 0 -> ifdef notyet for code needed in future

[imp]

From: Warner Losh <imp@bsdimp.com>

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/elfload.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
index 87154283ef..07a00ddbd5 100644
--- a/bsd-user/elfload.c
+++ b/bsd-user/elfload.c
@@ -1270,7 +1270,7 @@ int load_elf_binary(struct linux_binprm *bprm, struct target_pt_regs *regs,
               ibcs2_interpreter = 1;
             }
 
-#if 0
+#ifdef notyet
             printf("Using ELF interpreter %s\n", path(elf_interpreter));
 #endif
             if (retval >= 0) {
@@ -1529,7 +1529,7 @@ int load_elf_binary(struct linux_binprm *bprm, struct target_pt_regs *regs,
 
     padzero(elf_bss, elf_brk);
 
-#if 0
+#ifdef notyet
     printf("(start_brk) %x\n" , info->start_brk);
     printf("(end_code) %x\n" , info->end_code);
     printf("(start_code) %x\n" , info->start_code);
-- 
2.22.1

Re: [PULL 11/24] bsd-user: style tweak: if 0 -> ifdef notyet for code needed in future

[Philippe Mathieu-Daudé]

On 4/23/21 10:39 PM, imp@bsdimp.com wrote:
> From: Warner Losh <imp@bsdimp.com>
> 
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/elfload.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
> index 87154283ef..07a00ddbd5 100644
> --- a/bsd-user/elfload.c
> +++ b/bsd-user/elfload.c
> @@ -1270,7 +1270,7 @@ int load_elf_binary(struct linux_binprm *bprm, struct target_pt_regs *regs,
>                ibcs2_interpreter = 1;
>              }
>  
> -#if 0
> +#ifdef notyet

Better describe in the cover letter "ignored checkpatch errors" and keep
this unmodified rather than trying to bypass them by dubious code style
IMO. The checkpatch.pl script is here to help us ;)

>              printf("Using ELF interpreter %s\n", path(elf_interpreter));
>  #endif

Re: [PULL 11/24] bsd-user: style tweak: if 0 -> ifdef notyet for code needed in future

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 1310 bytes --]

On Fri, Apr 23, 2021 at 3:23 PM Philippe Mathieu-Daudé <f4bug@amsat.org>
wrote:

> On 4/23/21 10:39 PM, imp@bsdimp.com wrote:
> > From: Warner Losh <imp@bsdimp.com>
> >
> > Signed-off-by: Warner Losh <imp@bsdimp.com>
> > ---
> >  bsd-user/elfload.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
> > index 87154283ef..07a00ddbd5 100644
> > --- a/bsd-user/elfload.c
> > +++ b/bsd-user/elfload.c
> > @@ -1270,7 +1270,7 @@ int load_elf_binary(struct linux_binprm *bprm,
> struct target_pt_regs *regs,
> >                ibcs2_interpreter = 1;
> >              }
> >
> > -#if 0
> > +#ifdef notyet
>
> Better describe in the cover letter "ignored checkpatch errors" and keep
> this unmodified rather than trying to bypass them by dubious code style
> IMO. The checkpatch.pl script is here to help us ;)
>

This one I honestly was unsure about. To be honest, it's fear that kept
me keeping this code....  Maybe it would be even better to just delete
this code entirely. I have a working final state to pull from, now that I
think about it to forumlate a reply, so maybe that would be even
better?

Warner


> >              printf("Using ELF interpreter %s\n", path(elf_interpreter));
> >  #endif
>

[-- Attachment #2: Type: text/html, Size: 2222 bytes --]

Re: [PULL 11/24] bsd-user: style tweak: if 0 -> ifdef notyet for code needed in future

[Philippe Mathieu-Daudé]

On 4/23/21 11:38 PM, Warner Losh wrote:
> On Fri, Apr 23, 2021 at 3:23 PM Philippe Mathieu-Daudé <f4bug@amsat.org
> <mailto:f4bug@amsat.org>> wrote:
> 
>     On 4/23/21 10:39 PM, imp@bsdimp.com <mailto:imp@bsdimp.com> wrote:
>     > From: Warner Losh <imp@bsdimp.com <mailto:imp@bsdimp.com>>
>     >
>     > Signed-off-by: Warner Losh <imp@bsdimp.com <mailto:imp@bsdimp.com>>
>     > ---
>     >  bsd-user/elfload.c | 4 ++--
>     >  1 file changed, 2 insertions(+), 2 deletions(-)
>     >
>     > diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
>     > index 87154283ef..07a00ddbd5 100644
>     > --- a/bsd-user/elfload.c
>     > +++ b/bsd-user/elfload.c
>     > @@ -1270,7 +1270,7 @@ int load_elf_binary(struct linux_binprm
>     *bprm, struct target_pt_regs *regs,
>     >                ibcs2_interpreter = 1;
>     >              }
>     > 
>     > -#if 0
>     > +#ifdef notyet
> 
>     Better describe in the cover letter "ignored checkpatch errors" and keep
>     this unmodified rather than trying to bypass them by dubious code style
>     IMO. The checkpatch.pl <http://checkpatch.pl> script is here to help
>     us ;)
> 
> 
> This one I honestly was unsure about. To be honest, it's fear that kept
> me keeping this code....  Maybe it would be even better to just delete
> this code entirely. I have a working final state to pull from, now that I
> think about it to forumlate a reply, so maybe that would be even
> better?

Personally I find it simpler. We use git, so we have the history in
the repository, can look at it and restore it if needed. This code
is dead since years.

Let's see what others think about this.

Regards,

Phil.

Re: [PULL 11/24] bsd-user: style tweak: if 0 -> ifdef notyet for code needed in future

[Richard Henderson]

On 4/23/21 3:08 PM, Philippe Mathieu-Daudé wrote:
> On 4/23/21 11:38 PM, Warner Losh wrote:
>> On Fri, Apr 23, 2021 at 3:23 PM Philippe Mathieu-Daudé <f4bug@amsat.org
>> <mailto:f4bug@amsat.org>> wrote:
>>
>>      On 4/23/21 10:39 PM, imp@bsdimp.com <mailto:imp@bsdimp.com> wrote:
>>      > From: Warner Losh <imp@bsdimp.com <mailto:imp@bsdimp.com>>
>>      >
>>      > Signed-off-by: Warner Losh <imp@bsdimp.com <mailto:imp@bsdimp.com>>
>>      > ---
>>      >  bsd-user/elfload.c | 4 ++--
>>      >  1 file changed, 2 insertions(+), 2 deletions(-)
>>      >
>>      > diff --git a/bsd-user/elfload.c b/bsd-user/elfload.c
>>      > index 87154283ef..07a00ddbd5 100644
>>      > --- a/bsd-user/elfload.c
>>      > +++ b/bsd-user/elfload.c
>>      > @@ -1270,7 +1270,7 @@ int load_elf_binary(struct linux_binprm
>>      *bprm, struct target_pt_regs *regs,
>>      >                ibcs2_interpreter = 1;
>>      >              }
>>      >
>>      > -#if 0
>>      > +#ifdef notyet
>>
>>      Better describe in the cover letter "ignored checkpatch errors" and keep
>>      this unmodified rather than trying to bypass them by dubious code style
>>      IMO. The checkpatch.pl <http://checkpatch.pl> script is here to help
>>      us ;)
>>
>>
>> This one I honestly was unsure about. To be honest, it's fear that kept
>> me keeping this code....  Maybe it would be even better to just delete
>> this code entirely. I have a working final state to pull from, now that I
>> think about it to forumlate a reply, so maybe that would be even
>> better?
> 
> Personally I find it simpler. We use git, so we have the history in
> the repository, can look at it and restore it if needed. This code
> is dead since years.
> 
> Let's see what others think about this.

Definitely better to remove.  We can review the new code more easily that way.


r~

[PULL 12/24] bsd-user: style tweak: if 0 -> ifdef notyet for code needed in future

[imp]

From: Warner Losh <imp@bsdimp.com>

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/main.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index ff886de98e..91603f5ac2 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -245,7 +245,7 @@ void cpu_loop(CPUX86State *env)
             }
             break;
 #endif
-#if 0
+#ifdef notyet
         case EXCP0B_NOSEG:
         case EXCP0C_STACK:
             info.si_signo = SIGBUS;
@@ -340,7 +340,7 @@ void cpu_loop(CPUX86State *env)
         case EXCP_INTERRUPT:
             /* just indicate that signals should be handled asap */
             break;
-#if 0
+#ifdef notyet
         case EXCP_DEBUG:
             {
                 int sig;
@@ -589,7 +589,7 @@ void cpu_loop(CPUSPARCState *env)
             break;
         case TT_TFAULT:
         case TT_DFAULT:
-#if 0
+#ifdef notyet
             {
                 info.si_signo = SIGSEGV;
                 info.si_errno = 0;
@@ -609,7 +609,7 @@ void cpu_loop(CPUSPARCState *env)
             break;
         case TT_TFAULT:
         case TT_DFAULT:
-#if 0
+#ifdef notyet
             {
                 info.si_signo = SIGSEGV;
                 info.si_errno = 0;
@@ -629,11 +629,11 @@ void cpu_loop(CPUSPARCState *env)
             break;
         case EXCP_DEBUG:
             {
-#if 0
+#ifdef notyet
                 int sig =
 #endif
                 gdb_handlesig(cs, TARGET_SIGTRAP);
-#if 0
+#ifdef notyet
                 if (sig)
                   {
                     info.si_signo = sig;
-- 
2.22.1

[PULL 13/24] bsd-user: style tweak: if 0 -> ifdef notyet for code needed in future

[imp]

From: Warner Losh <imp@bsdimp.com>

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/strace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bsd-user/strace.c b/bsd-user/strace.c
index 2c3b59caf0..6a78dd7efa 100644
--- a/bsd-user/strace.c
+++ b/bsd-user/strace.c
@@ -128,7 +128,7 @@ static void print_syscall_ret_addr(const struct syscallname *name, abi_long ret)
     }
 }
 
-#if 0 /* currently unused */
+#ifdef notyet /* currently unused */
 static void
 print_syscall_ret_raw(struct syscallname *name, abi_long ret)
 {
-- 
2.22.1

[PULL 14/24] bsd-user: style tweak: if 0 -> ifdef notyet for code needed in future

[imp]

From: Warner Losh <imp@bsdimp.com>

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/x86_64/target_syscall.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bsd-user/x86_64/target_syscall.h b/bsd-user/x86_64/target_syscall.h
index a8e6274b76..9a6e072f87 100644
--- a/bsd-user/x86_64/target_syscall.h
+++ b/bsd-user/x86_64/target_syscall.h
@@ -59,7 +59,7 @@ struct target_pt_regs {
 #define TARGET_GDT_ENTRY_TLS_MIN 12
 #define TARGET_GDT_ENTRY_TLS_MAX 14
 
-#if 0 // Redefine this
+#ifdef notyet /* Redefine this */
 struct target_modify_ldt_ldt_s {
 	unsigned int  entry_number;
         abi_ulong     base_addr;
-- 
2.22.1

[PULL 15/24] bsd-user: Fix commentary issues

[imp]

From: Warner Losh <imp@bsdimp.com>

Lines > 80 or 90 characters
C++ comments
BSD /*- block comment convention removed.

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/bsd-mman.h | 42 ++++++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/bsd-user/bsd-mman.h b/bsd-user/bsd-mman.h
index 910e8c1921..5a64d0d425 100644
--- a/bsd-user/bsd-mman.h
+++ b/bsd-user/bsd-mman.h
@@ -1,4 +1,4 @@
-/*-
+/*
  * Copyright (c) 1982, 1986, 1993
  *      The Regents of the University of California.  All rights reserved.
  *
@@ -30,16 +30,20 @@
  * $FreeBSD: src/sys/sys/mman.h,v 1.42 2008/03/28 04:29:27 ps Exp $
  */
 
-#define TARGET_FREEBSD_MAP_RESERVED0080 0x0080  /* previously misimplemented MAP_INHERIT */
-#define TARGET_FREEBSD_MAP_RESERVED0100 0x0100  /* previously unimplemented MAP_NOEXTEND */
-#define TARGET_FREEBSD_MAP_STACK        0x0400  /* region grows down, like a stack */
-#define TARGET_FREEBSD_MAP_NOSYNC       0x0800  /* page to but do not sync underlying file */
+#define TARGET_FREEBSD_MAP_RESERVED0080 0x0080
+                                 /* previously misimplemented MAP_INHERIT */
+#define TARGET_FREEBSD_MAP_RESERVED0100 0x0100
+                                 /* previously unimplemented MAP_NOEXTEND */
+#define TARGET_FREEBSD_MAP_STACK        0x0400
+                                 /* region grows down, like a stack */
+#define TARGET_FREEBSD_MAP_NOSYNC       0x0800
+                                 /* page to but do not sync underlying file */
 
 #define TARGET_FREEBSD_MAP_FLAGMASK     0x1ff7
 
 /*      $NetBSD: mman.h,v 1.42 2008/11/18 22:13:49 ad Exp $     */
 
-/*-
+/*
  * Copyright (c) 1982, 1986, 1993
  *      The Regents of the University of California.  All rights reserved.
  *
@@ -69,18 +73,21 @@
  *
  *      @(#)mman.h      8.2 (Berkeley) 1/9/95
  */
-#define TARGET_NETBSD_MAP_INHERIT       0x0080  /* region is retained after exec */
-#define TARGET_NETBSD_MAP_TRYFIXED      0x0400 /* attempt hint address, even within break */
-#define TARGET_NETBSD_MAP_WIRED         0x0800  /* mlock() mapping when it is established */
-
-#define TARGET_NETBSD_MAP_STACK         0x2000  /* allocated from memory, swap space (stack) */
+#define TARGET_NETBSD_MAP_INHERIT       0x0080
+                                /* region is retained after exec */
+#define TARGET_NETBSD_MAP_TRYFIXED      0x0400
+                                /* attempt hint address, even within break */
+#define TARGET_NETBSD_MAP_WIRED         0x0800
+                                /* mlock() mapping when it is established */
+#define TARGET_NETBSD_MAP_STACK         0x2000
+                                /* allocated from memory, swap space (stack) */
 
 #define TARGET_NETBSD_MAP_FLAGMASK      0x3ff7
 
 /*      $OpenBSD: mman.h,v 1.18 2003/07/21 22:52:19 tedu Exp $  */
 /*      $NetBSD: mman.h,v 1.11 1995/03/26 20:24:23 jtc Exp $    */
 
-/*-
+/*
  * Copyright (c) 1982, 1986, 1993
  *      The Regents of the University of California.  All rights reserved.
  *
@@ -111,11 +118,14 @@
  *      @(#)mman.h      8.1 (Berkeley) 6/2/93
  */
 
-#define TARGET_OPENBSD_MAP_INHERIT      0x0080  /* region is retained after exec */
-#define TARGET_OPENBSD_MAP_NOEXTEND     0x0100  /* for MAP_FILE, don't change file size */
-#define TARGET_OPENBSD_MAP_TRYFIXED     0x0400  /* attempt hint address, even within heap */
+#define TARGET_OPENBSD_MAP_INHERIT      0x0080
+                                /* region is retained after exec */
+#define TARGET_OPENBSD_MAP_NOEXTEND     0x0100
+                                /* for MAP_FILE, don't change file size */
+#define TARGET_OPENBSD_MAP_TRYFIXED     0x0400
+                                /* attempt hint address, even within heap */
 
 #define TARGET_OPENBSD_MAP_FLAGMASK     0x17f7
 
-// XXX
+/* XXX what to do in the future? */
 #define TARGET_BSD_MAP_FLAGMASK         0x3ff7
-- 
2.22.1

Re: [PULL 15/24] bsd-user: Fix commentary issues

[Richard Henderson]

On 4/23/21 1:39 PM, imp@bsdimp.com wrote:
> From: Warner Losh<imp@bsdimp.com>
> 
> Lines > 80 or 90 characters
> C++ comments
> BSD /*- block comment convention removed.
> 
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/bsd-mman.h | 42 ++++++++++++++++++++++++++----------------
>   1 file changed, 26 insertions(+), 16 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PULL 15/24] bsd-user: Fix commentary issues

[Daniel P. Berrangé]

On Fri, Apr 23, 2021 at 02:39:50PM -0600, imp@bsdimp.com wrote:
> From: Warner Losh <imp@bsdimp.com>
> 
> Lines > 80 or 90 characters
> C++ comments
> BSD /*- block comment convention removed.
> 
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/bsd-mman.h | 42 ++++++++++++++++++++++++++----------------
>  1 file changed, 26 insertions(+), 16 deletions(-)
> 
> diff --git a/bsd-user/bsd-mman.h b/bsd-user/bsd-mman.h
> index 910e8c1921..5a64d0d425 100644
> --- a/bsd-user/bsd-mman.h
> +++ b/bsd-user/bsd-mman.h
> @@ -1,4 +1,4 @@
> -/*-
> +/*
>   * Copyright (c) 1982, 1986, 1993
>   *      The Regents of the University of California.  All rights reserved.
>   *
> @@ -30,16 +30,20 @@
>   * $FreeBSD: src/sys/sys/mman.h,v 1.42 2008/03/28 04:29:27 ps Exp $
>   */
>  
> -#define TARGET_FREEBSD_MAP_RESERVED0080 0x0080  /* previously misimplemented MAP_INHERIT */
> -#define TARGET_FREEBSD_MAP_RESERVED0100 0x0100  /* previously unimplemented MAP_NOEXTEND */
> -#define TARGET_FREEBSD_MAP_STACK        0x0400  /* region grows down, like a stack */
> -#define TARGET_FREEBSD_MAP_NOSYNC       0x0800  /* page to but do not sync underlying file */
> +#define TARGET_FREEBSD_MAP_RESERVED0080 0x0080
> +                                 /* previously misimplemented MAP_INHERIT */
> +#define TARGET_FREEBSD_MAP_RESERVED0100 0x0100
> +                                 /* previously unimplemented MAP_NOEXTEND */
> +#define TARGET_FREEBSD_MAP_STACK        0x0400
> +                                 /* region grows down, like a stack */
> +#define TARGET_FREEBSD_MAP_NOSYNC       0x0800
> +                                 /* page to but do not sync underlying file */

I find this indented following comment style more ambiguous as to
what constant the comment applies to. IMHO would be clearer as

 /* previously misimplemented MAP_INHERIT */
 #define TARGET_FREEBSD_MAP_RESERVED0080 0x0080

 /* previously unimplemented MAP_NOEXTEND */
 #define TARGET_FREEBSD_MAP_RESERVED0100 0x0100

 /* region grows down, like a stack */
 #define TARGET_FREEBSD_MAP_STACK        0x0400

 /* page to but do not sync underlying file */
 #define TARGET_FREEBSD_MAP_NOSYNC       0x0800

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PULL 15/24] bsd-user: Fix commentary issues

[Peter Maydell]

On Mon, 26 Apr 2021 at 10:01, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Fri, Apr 23, 2021 at 02:39:50PM -0600, imp@bsdimp.com wrote:
> > -#define TARGET_FREEBSD_MAP_RESERVED0080 0x0080  /* previously misimplemented MAP_INHERIT */
> > -#define TARGET_FREEBSD_MAP_RESERVED0100 0x0100  /* previously unimplemented MAP_NOEXTEND */
> > -#define TARGET_FREEBSD_MAP_STACK        0x0400  /* region grows down, like a stack */
> > -#define TARGET_FREEBSD_MAP_NOSYNC       0x0800  /* page to but do not sync underlying file */
> > +#define TARGET_FREEBSD_MAP_RESERVED0080 0x0080
> > +                                 /* previously misimplemented MAP_INHERIT */
> > +#define TARGET_FREEBSD_MAP_RESERVED0100 0x0100
> > +                                 /* previously unimplemented MAP_NOEXTEND */
> > +#define TARGET_FREEBSD_MAP_STACK        0x0400
> > +                                 /* region grows down, like a stack */
> > +#define TARGET_FREEBSD_MAP_NOSYNC       0x0800
> > +                                 /* page to but do not sync underlying file */
>
> I find this indented following comment style more ambiguous as to
> what constant the comment applies to. IMHO would be clearer as
>
>  /* previously misimplemented MAP_INHERIT */
>  #define TARGET_FREEBSD_MAP_RESERVED0080 0x0080
>
>  /* previously unimplemented MAP_NOEXTEND */
>  #define TARGET_FREEBSD_MAP_RESERVED0100 0x0100
>
>  /* region grows down, like a stack */
>  #define TARGET_FREEBSD_MAP_STACK        0x0400
>
>  /* page to but do not sync underlying file */
>  #define TARGET_FREEBSD_MAP_NOSYNC       0x0800

Or alternatively decide that this is one of those cases where the coding
style's "If wrapping the line at 80 columns is obviously less readable and
more awkward, prefer not to wrap it" advice applies. The lines as they stand
are only 95 characters or so long.

thanks
-- PMM

Re: [PULL 15/24] bsd-user: Fix commentary issues

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 2745 bytes --]

On Mon, Apr 26, 2021 at 3:13 AM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Mon, 26 Apr 2021 at 10:01, Daniel P. Berrangé <berrange@redhat.com>
> wrote:
> >
> > On Fri, Apr 23, 2021 at 02:39:50PM -0600, imp@bsdimp.com wrote:
> > > -#define TARGET_FREEBSD_MAP_RESERVED0080 0x0080  /* previously
> misimplemented MAP_INHERIT */
> > > -#define TARGET_FREEBSD_MAP_RESERVED0100 0x0100  /* previously
> unimplemented MAP_NOEXTEND */
> > > -#define TARGET_FREEBSD_MAP_STACK        0x0400  /* region grows down,
> like a stack */
> > > -#define TARGET_FREEBSD_MAP_NOSYNC       0x0800  /* page to but do not
> sync underlying file */
> > > +#define TARGET_FREEBSD_MAP_RESERVED0080 0x0080
> > > +                                 /* previously misimplemented
> MAP_INHERIT */
> > > +#define TARGET_FREEBSD_MAP_RESERVED0100 0x0100
> > > +                                 /* previously unimplemented
> MAP_NOEXTEND */
> > > +#define TARGET_FREEBSD_MAP_STACK        0x0400
> > > +                                 /* region grows down, like a stack */
> > > +#define TARGET_FREEBSD_MAP_NOSYNC       0x0800
> > > +                                 /* page to but do not sync
> underlying file */
> >
> > I find this indented following comment style more ambiguous as to
> > what constant the comment applies to. IMHO would be clearer as
> >
> >  /* previously misimplemented MAP_INHERIT */
> >  #define TARGET_FREEBSD_MAP_RESERVED0080 0x0080
> >
> >  /* previously unimplemented MAP_NOEXTEND */
> >  #define TARGET_FREEBSD_MAP_RESERVED0100 0x0100
> >
> >  /* region grows down, like a stack */
> >  #define TARGET_FREEBSD_MAP_STACK        0x0400
> >
> >  /* page to but do not sync underlying file */
> >  #define TARGET_FREEBSD_MAP_NOSYNC       0x0800
>
> Or alternatively decide that this is one of those cases where the coding
> style's "If wrapping the line at 80 columns is obviously less readable and
> more awkward, prefer not to wrap it" advice applies. The lines as they
> stand
> are only 95 characters or so long.
>

I'm cool either way. Projects differ about how rigid or flexible style can
be.
I noticed this when cleaning up another thing. If we can apply that advice,
I'd prefer that.

Maybe it's better to just drop this entirely. In the final state, this file
is
here, but none of these flags are actually used. Not sure why they aren't,
but there's commentary that's explicit about using the host constants
in many places. Rather than take the time to sort this all out right now
(it is a minor detail I think in the scheme of things compared to all the
other changes coming), I'd propose dropping this hunk entirely, and
revisiting when the merging is done...

Warner

[-- Attachment #2: Type: text/html, Size: 3628 bytes --]

[PULL 16/24] bsd-user: Use preferred block comments

[imp]

From: Warner Losh <imp@bsdimp.com>

Use the preferred block comment style.

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/main.c | 28 ++++++++++++++++++----------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index 91603f5ac2..5958b07e62 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -51,9 +51,11 @@ const char *qemu_uname_release;
 extern char **environ;
 enum BSDType bsd_type;
 
-/* XXX: on x86 MAP_GROWSDOWN only works if ESP <= address + 32, so
-   we allocate a bigger stack. Need a better solution, for example
-   by remapping the process stack directly at the right place */
+/*
+ * XXX: on x86 MAP_GROWSDOWN only works if ESP <= address + 32, so
+ * we allocate a bigger stack. Need a better solution, for example
+ * by remapping the process stack directly at the right place
+ */
 unsigned long x86_stack_size = 512 * 1024;
 
 void gemu_log(const char *fmt, ...)
@@ -371,13 +373,17 @@ void cpu_loop(CPUX86State *env)
 #define SPARC64_STACK_BIAS 2047
 
 /* #define DEBUG_WIN */
-/* WARNING: dealing with register windows _is_ complicated. More info
-   can be found at http://www.sics.se/~psm/sparcstack.html */
+/*
+ * WARNING: dealing with register windows _is_ complicated. More info
+ * can be found at http://www.sics.se/~psm/sparcstack.html
+ */
 static inline int get_reg_index(CPUSPARCState *env, int cwp, int index)
 {
     index = (index + cwp * 16) % (16 * env->nwindows);
-    /* wrap handling : if cwp is on the last window, then we use the
-       registers 'after' the end */
+    /*
+     * wrap handling : if cwp is on the last window, then we use the
+     * registers 'after' the end
+     */
     if (index < 8 && env->cwp == env->nwindows - 1)
         index += 16 * env->nwindows;
     return index;
@@ -990,9 +996,11 @@ int main(int argc, char **argv)
     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.  */
+    /*
+     * 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);
     tcg_region_init();
 
-- 
2.22.1

Re: [PULL 16/24] bsd-user: Use preferred block comments

[Richard Henderson]

On 4/23/21 1:39 PM, imp@bsdimp.com wrote:
> From: Warner Losh<imp@bsdimp.com>
> 
> Use the preferred block comment style.
> 
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/main.c | 28 ++++++++++++++++++----------
>   1 file changed, 18 insertions(+), 10 deletions(-)


Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PULL 17/24] bsd-user: move extern to header file

[imp]

From: Warner Losh <imp@bsdimp.com>

extern char **environ has no standard home, so move the declaration from the .c
file to a handy .h file. Since this is a standard, old-school UNIX interface
dating from the 5th edition, it's not quite the same issue that the rule is
supposed to protect against, though.

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/main.c | 1 -
 bsd-user/qemu.h | 2 ++
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index 5958b07e62..a94ab63065 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -48,7 +48,6 @@ unsigned long reserved_va;
 
 static const char *interp_prefix = CONFIG_QEMU_INTERP_PREFIX;
 const char *qemu_uname_release;
-extern char **environ;
 enum BSDType bsd_type;
 
 /*
diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index 3480db890d..4305bd1aed 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -27,6 +27,8 @@
 
 #include "exec/user/abitypes.h"
 
+extern char **environ;
+
 enum BSDType {
     target_freebsd,
     target_netbsd,
-- 
2.22.1

Re: [PULL 17/24] bsd-user: move extern to header file

[Richard Henderson]

On 4/23/21 1:39 PM, imp@bsdimp.com wrote:
> From: Warner Losh<imp@bsdimp.com>
> 
> extern char **environ has no standard home, so move the declaration from the .c
> file to a handy .h file. Since this is a standard, old-school UNIX interface
> dating from the 5th edition, it's not quite the same issue that the rule is
> supposed to protect against, though.
> 
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/main.c | 1 -
>   bsd-user/qemu.h | 2 ++
>   2 files changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PULL 18/24] bsd-user: style changes for {}

[imp]

From: Warner Losh <imp@bsdimp.com>

Fix various issues with {} not being present on if / for / case statements.
Tweak indentation where needed to appease checkpatch after this.
Minor line length tweaks

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/main.c | 100 +++++++++++++++++++++++++++++-------------------
 1 file changed, 60 insertions(+), 40 deletions(-)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index a94ab63065..69e5525607 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -218,7 +218,7 @@ void cpu_loop(CPUX86State *env)
 #ifndef TARGET_ABI32
         case EXCP_SYSCALL:
             /* syscall from syscall instruction */
-            if (bsd_type == target_freebsd)
+            if (bsd_type == target_freebsd) {
                 env->regs[R_EAX] = do_freebsd_syscall(env,
                                                       env->regs[R_EAX],
                                                       env->regs[R_EDI],
@@ -227,7 +227,7 @@ void cpu_loop(CPUX86State *env)
                                                       env->regs[R_ECX],
                                                       env->regs[8],
                                                       env->regs[9], 0, 0);
-            else { /* if (bsd_type == target_openbsd) */
+            } else { /* if (bsd_type == target_openbsd) */
                 env->regs[R_EAX] = do_openbsd_syscall(env,
                                                       env->regs[R_EAX],
                                                       env->regs[R_EDI],
@@ -273,10 +273,11 @@ void cpu_loop(CPUX86State *env)
         case EXCP0E_PAGE:
             info.si_signo = SIGSEGV;
             info.si_errno = 0;
-            if (!(env->error_code & 1))
+            if (!(env->error_code & 1)) {
                 info.si_code = TARGET_SEGV_MAPERR;
-            else
+            } else {
                 info.si_code = TARGET_SEGV_ACCERR;
+            }
             info._sifields._sigfault._addr = env->cr[2];
             queue_signal(env, info.si_signo, &info);
             break;
@@ -343,23 +344,23 @@ void cpu_loop(CPUX86State *env)
             break;
 #ifdef notyet
         case EXCP_DEBUG:
-            {
-                int sig;
+        {
+            int sig;
 
-                sig = gdb_handlesig(env, TARGET_SIGTRAP);
-                if (sig)
-                  {
-                    info.si_signo = sig;
-                    info.si_errno = 0;
-                    info.si_code = TARGET_TRAP_BRKPT;
-                    queue_signal(env, info.si_signo, &info);
-                  }
+            sig = gdb_handlesig(env, TARGET_SIGTRAP);
+            if (sig) {
+                info.si_signo = sig;
+                info.si_errno = 0;
+                info.si_code = TARGET_TRAP_BRKPT;
+                queue_signal(env, info.si_signo, &info);
             }
             break;
+        }
 #endif
         default:
             pc = env->segs[R_CS].base + env->eip;
-            fprintf(stderr, "qemu: 0x%08lx: unhandled CPU exception 0x%x - aborting\n",
+            fprintf(stderr,
+                    "qemu: 0x%08lx: unhandled CPU exception 0x%x - aborting\n",
                     (long)pc, trapnr);
             abort();
         }
@@ -383,8 +384,9 @@ static inline int get_reg_index(CPUSPARCState *env, int cwp, int index)
      * wrap handling : if cwp is on the last window, then we use the
      * registers 'after' the end
      */
-    if (index < 8 && env->cwp == env->nwindows - 1)
+    if (index < 8 && env->cwp == env->nwindows - 1) {
         index += 16 * env->nwindows;
+    }
     return index;
 }
 
@@ -396,8 +398,9 @@ static inline void save_window_offset(CPUSPARCState *env, int cwp1)
 
     sp_ptr = env->regbase[get_reg_index(env, cwp1, 6)];
 #ifdef TARGET_SPARC64
-    if (sp_ptr & 3)
+    if (sp_ptr & 3) {
         sp_ptr += SPARC64_STACK_BIAS;
+    }
 #endif
 #if defined(DEBUG_WIN)
     printf("win_overflow: sp_ptr=0x" TARGET_ABI_FMT_lx " save_cwp=%d\n",
@@ -446,8 +449,9 @@ static void restore_window(CPUSPARCState *env)
     cwp1 = cpu_cwp_inc(env, env->cwp + 1);
     sp_ptr = env->regbase[get_reg_index(env, cwp1, 6)];
 #ifdef TARGET_SPARC64
-    if (sp_ptr & 3)
+    if (sp_ptr & 3) {
         sp_ptr += SPARC64_STACK_BIAS;
+    }
 #endif
 #if defined(DEBUG_WIN)
     printf("win_underflow: sp_ptr=0x" TARGET_ABI_FMT_lx " load_cwp=%d\n",
@@ -460,8 +464,9 @@ static void restore_window(CPUSPARCState *env)
     }
 #ifdef TARGET_SPARC64
     env->canrestore++;
-    if (env->cleanwin < env->nwindows - 1)
+    if (env->cleanwin < env->nwindows - 1) {
         env->cleanwin++;
+    }
     env->cansave--;
 #else
     env->wim = new_wim;
@@ -477,11 +482,13 @@ static void flush_windows(CPUSPARCState *env)
         /* if restore would invoke restore_window(), then we can stop */
         cwp1 = cpu_cwp_inc(env, env->cwp + offset);
 #ifndef TARGET_SPARC64
-        if (env->wim & (1 << cwp1))
+        if (env->wim & (1 << cwp1)) {
             break;
+        }
 #else
-        if (env->canrestore == 0)
+        if (env->canrestore == 0) {
             break;
+        }
         env->cansave++;
         env->canrestore--;
 #endif
@@ -516,8 +523,9 @@ void cpu_loop(CPUSPARCState *env)
 #else
         /* FreeBSD uses 0x141 for syscalls too */
         case 0x141:
-            if (bsd_type != target_freebsd)
+            if (bsd_type != target_freebsd) {
                 goto badtrap;
+            }
             /* fallthrough */
         case 0x100:
 #endif
@@ -526,7 +534,8 @@ void cpu_loop(CPUSPARCState *env)
                 ret = do_freebsd_syscall(env, syscall_nr,
                                          env->regwptr[0], env->regwptr[1],
                                          env->regwptr[2], env->regwptr[3],
-                                         env->regwptr[4], env->regwptr[5], 0, 0);
+                                         env->regwptr[4], env->regwptr[5],
+                                         0, 0);
             else if (bsd_type == target_netbsd)
                 ret = do_netbsd_syscall(env, syscall_nr,
                                         env->regwptr[0], env->regwptr[1],
@@ -620,10 +629,11 @@ void cpu_loop(CPUSPARCState *env)
                 info.si_errno = 0;
                 /* XXX: check env->error_code */
                 info.si_code = TARGET_SEGV_MAPERR;
-                if (trapnr == TT_DFAULT)
+                if (trapnr == TT_DFAULT) {
                     info._sifields._sigfault._addr = env->dmmuregs[4];
-                else
+                } else {
                     info._sifields._sigfault._addr = env->tsptr->tpc;
+                }
                 /* queue_signal(env, info.si_signo, &info); */
             }
 #endif
@@ -639,13 +649,12 @@ void cpu_loop(CPUSPARCState *env)
 #endif
                 gdb_handlesig(cs, TARGET_SIGTRAP);
 #ifdef notyet
-                if (sig)
-                  {
+                if (sig) {
                     info.si_signo = sig;
                     info.si_errno = 0;
                     info.si_code = TARGET_TRAP_BRKPT;
                     /* queue_signal(env, info.si_signo, &info); */
-                  }
+                }
 #endif
             }
             break;
@@ -754,8 +763,9 @@ int main(int argc, char **argv)
     envlist_t *envlist = NULL;
     bsd_type = target_openbsd;
 
-    if (argc <= 1)
+    if (argc <= 1) {
         usage();
+    }
 
     error_init(argv[0]);
     module_call_init(MODULE_INIT_TRACE);
@@ -775,11 +785,13 @@ int main(int argc, char **argv)
 
     optind = 1;
     for (;;) {
-        if (optind >= argc)
+        if (optind >= argc) {
             break;
+        }
         r = argv[optind];
-        if (r[0] != '-')
+        if (r[0] != '-') {
             break;
+        }
         optind++;
         r++;
         if (!strcmp(r, "-")) {
@@ -796,24 +808,28 @@ int main(int argc, char **argv)
             log_file = argv[optind++];
         } else if (!strcmp(r, "E")) {
             r = argv[optind++];
-            if (envlist_setenv(envlist, r) != 0)
+            if (envlist_setenv(envlist, r) != 0) {
                 usage();
+            }
         } else if (!strcmp(r, "ignore-environment")) {
             envlist_free(envlist);
             envlist = envlist_create();
         } else if (!strcmp(r, "U")) {
             r = argv[optind++];
-            if (envlist_unsetenv(envlist, r) != 0)
+            if (envlist_unsetenv(envlist, r) != 0) {
                 usage();
+            }
         } else if (!strcmp(r, "s")) {
             r = argv[optind++];
             x86_stack_size = strtol(r, (char **)&r, 0);
-            if (x86_stack_size <= 0)
+            if (x86_stack_size <= 0) {
                 usage();
-            if (*r == 'M')
+            }
+            if (*r == 'M') {
                 x86_stack_size *= MiB;
-            else if (*r == 'k' || *r == 'K')
+            } else if (*r == 'k' || *r == 'K') {
                 x86_stack_size *= KiB;
+            }
         } else if (!strcmp(r, "L")) {
             interp_prefix = argv[optind++];
         } else if (!strcmp(r, "p")) {
@@ -953,11 +969,13 @@ int main(int argc, char **argv)
     if (!have_guest_base) {
         FILE *fp;
 
-        if ((fp = fopen("/proc/sys/vm/mmap_min_addr", "r")) != NULL) {
+        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);
+                qemu_log_mask(CPU_LOG_PAGE, "host mmap_min_addr=0x%lx\n",
+                              mmap_min_addr);
             }
             fclose(fp);
         }
@@ -1130,10 +1148,12 @@ int main(int argc, char **argv)
         env->pc = regs->pc;
         env->npc = regs->npc;
         env->y = regs->y;
-        for (i = 0; i < 8; i++)
+        for (i = 0; i < 8; i++) {
             env->gregs[i] = regs->u_regs[i];
-        for (i = 0; i < 8; i++)
+        }
+        for (i = 0; i < 8; i++) {
             env->regwptr[i] = regs->u_regs[i + 8];
+        }
     }
 #else
 #error unsupported target CPU
-- 
2.22.1

Re: [PULL 18/24] bsd-user: style changes for {}

[Richard Henderson]

On 4/23/21 1:39 PM, imp@bsdimp.com wrote:
> From: Warner Losh<imp@bsdimp.com>
> 
> Fix various issues with {} not being present on if / for / case statements.
> Tweak indentation where needed to appease checkpatch after this.
> Minor line length tweaks
> 
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/main.c | 100 +++++++++++++++++++++++++++++-------------------
>   1 file changed, 60 insertions(+), 40 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PULL 19/24] bsd-user: use qemu_strtol in preference to strtol

[imp]

From: Warner Losh <imp@bsdimp.com>

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index 69e5525607..95fda8c5ae 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -821,7 +821,7 @@ int main(int argc, char **argv)
             }
         } else if (!strcmp(r, "s")) {
             r = argv[optind++];
-            x86_stack_size = strtol(r, (char **)&r, 0);
+            x86_stack_size = qemu_strtol(r, (char **)&r, 0);
             if (x86_stack_size <= 0) {
                 usage();
             }
@@ -853,7 +853,7 @@ int main(int argc, char **argv)
                 exit(1);
             }
         } else if (!strcmp(r, "B")) {
-            guest_base = strtol(argv[optind++], NULL, 0);
+            guest_base = qemu_strtol(argv[optind++], NULL, 0);
             have_guest_base = true;
         } else if (!strcmp(r, "drop-ld-preload")) {
             (void) envlist_unsetenv(envlist, "LD_PRELOAD");
-- 
2.22.1

Re: [PULL 19/24] bsd-user: use qemu_strtol in preference to strtol

[Philippe Mathieu-Daudé]

On 4/23/21 10:39 PM, imp@bsdimp.com wrote:
> From: Warner Losh <imp@bsdimp.com>
> 
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>  bsd-user/main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index 69e5525607..95fda8c5ae 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -821,7 +821,7 @@ int main(int argc, char **argv)
>              }
>          } else if (!strcmp(r, "s")) {
>              r = argv[optind++];
> -            x86_stack_size = strtol(r, (char **)&r, 0);
> +            x86_stack_size = qemu_strtol(r, (char **)&r, 0);
>              if (x86_stack_size <= 0) {
>                  usage();
>              }
> @@ -853,7 +853,7 @@ int main(int argc, char **argv)
>                  exit(1);
>              }
>          } else if (!strcmp(r, "B")) {
> -            guest_base = strtol(argv[optind++], NULL, 0);
> +            guest_base = qemu_strtol(argv[optind++], NULL, 0);

Missing error check (-EINVAL & -ERANGE).

>              have_guest_base = true;
>          } else if (!strcmp(r, "drop-ld-preload")) {
>              (void) envlist_unsetenv(envlist, "LD_PRELOAD");
> 

Re: [PULL 19/24] bsd-user: use qemu_strtol in preference to strtol

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 1395 bytes --]

On Fri, Apr 23, 2021 at 3:16 PM Philippe Mathieu-Daudé <f4bug@amsat.org>
wrote:

> On 4/23/21 10:39 PM, imp@bsdimp.com wrote:
> > From: Warner Losh <imp@bsdimp.com>
> >
> > Signed-off-by: Warner Losh <imp@bsdimp.com>
> > ---
> >  bsd-user/main.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/bsd-user/main.c b/bsd-user/main.c
> > index 69e5525607..95fda8c5ae 100644
> > --- a/bsd-user/main.c
> > +++ b/bsd-user/main.c
> > @@ -821,7 +821,7 @@ int main(int argc, char **argv)
> >              }
> >          } else if (!strcmp(r, "s")) {
> >              r = argv[optind++];
> > -            x86_stack_size = strtol(r, (char **)&r, 0);
> > +            x86_stack_size = qemu_strtol(r, (char **)&r, 0);
> >              if (x86_stack_size <= 0) {
> >                  usage();
> >              }
> > @@ -853,7 +853,7 @@ int main(int argc, char **argv)
> >                  exit(1);
> >              }
> >          } else if (!strcmp(r, "B")) {
> > -            guest_base = strtol(argv[optind++], NULL, 0);
> > +            guest_base = qemu_strtol(argv[optind++], NULL, 0);
>
> Missing error check (-EINVAL & -ERANGE).
>

Thanks! Will update.


> >              have_guest_base = true;
> >          } else if (!strcmp(r, "drop-ld-preload")) {
> >              (void) envlist_unsetenv(envlist, "LD_PRELOAD");
> >
>
>

[-- Attachment #2: Type: text/html, Size: 2347 bytes --]

[PULL 20/24] bsd-user: introduce host_os.h for bsd-specific code and defaults

[imp]

From: Warner Losh <imp@bsdimp.com>

Introduce host_os.h for frebsd, netbsd and openbsd. This sets the default bsd
being implemented today. In the future it will have code that is per-BSD
specific. Abstracted from a larger c93465b6208c4c95cc0a394ffef4180ba6ccf27a in
the qemu-bsd-user repo.

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/freebsd/host_os.h | 25 +++++++++++++++++++++++++
 bsd-user/main.c            |  4 +++-
 bsd-user/netbsd/host_os.h  | 25 +++++++++++++++++++++++++
 bsd-user/openbsd/host_os.h | 25 +++++++++++++++++++++++++
 4 files changed, 78 insertions(+), 1 deletion(-)
 create mode 100644 bsd-user/freebsd/host_os.h
 create mode 100644 bsd-user/netbsd/host_os.h
 create mode 100644 bsd-user/openbsd/host_os.h

diff --git a/bsd-user/freebsd/host_os.h b/bsd-user/freebsd/host_os.h
new file mode 100644
index 0000000000..ceb1543d06
--- /dev/null
+++ b/bsd-user/freebsd/host_os.h
@@ -0,0 +1,25 @@
+/*
+ *  FreeBSD host dependent code and definitions
+ *
+ *  Copyright (c) 2013 Stacey D. Son
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __HOST_OS_H_
+#define __HOST_OS_H_
+
+#define HOST_DEFAULT_BSD_TYPE target_freebsd
+
+#endif /*!__HOST_OS_H_ */
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 95fda8c5ae..fbcc8cc28c 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -40,6 +40,8 @@
 #include "exec/log.h"
 #include "trace/control.h"
 
+#include "host_os.h"
+
 int singlestep;
 unsigned long mmap_min_addr;
 uintptr_t guest_base;
@@ -761,7 +763,7 @@ int main(int argc, char **argv)
     const char *gdbstub = NULL;
     char **target_environ, **wrk;
     envlist_t *envlist = NULL;
-    bsd_type = target_openbsd;
+    bsd_type = HOST_DEFAULT_BSD_TYPE;
 
     if (argc <= 1) {
         usage();
diff --git a/bsd-user/netbsd/host_os.h b/bsd-user/netbsd/host_os.h
new file mode 100644
index 0000000000..ccbea076e6
--- /dev/null
+++ b/bsd-user/netbsd/host_os.h
@@ -0,0 +1,25 @@
+/*
+ *  NetBSD host dependent code and definitions
+ *
+ *  Copyright (c) 2013 Stacey D. Son
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __HOST_OS_H_
+#define __HOST_OS_H_
+
+#define HOST_DEFAULT_BSD_TYPE target_netbsd
+
+#endif /*!__HOST_OS_H_ */
diff --git a/bsd-user/openbsd/host_os.h b/bsd-user/openbsd/host_os.h
new file mode 100644
index 0000000000..79468073e4
--- /dev/null
+++ b/bsd-user/openbsd/host_os.h
@@ -0,0 +1,25 @@
+/*
+ *  OpenBSD host dependent code and definitions
+ *
+ *  Copyright (c) 2013 Stacey D. Son
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef __HOST_OS_H_
+#define __HOST_OS_H_
+
+#define HOST_DEFAULT_BSD_TYPE target_openbsd
+
+#endif /*!__HOST_OS_H_ */
-- 
2.22.1

Re: [PULL 20/24] bsd-user: introduce host_os.h for bsd-specific code and defaults

[Richard Henderson]

On 4/23/21 1:39 PM, imp@bsdimp.com wrote:
> From: Warner Losh<imp@bsdimp.com>
> 
> Introduce host_os.h for frebsd, netbsd and openbsd. This sets the default bsd
> being implemented today. In the future it will have code that is per-BSD
> specific. Abstracted from a larger c93465b6208c4c95cc0a394ffef4180ba6ccf27a in
> the qemu-bsd-user repo.
> 
> Signed-off-by: Stacey Son<sson@FreeBSD.org>
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/freebsd/host_os.h | 25 +++++++++++++++++++++++++
>   bsd-user/main.c            |  4 +++-
>   bsd-user/netbsd/host_os.h  | 25 +++++++++++++++++++++++++
>   bsd-user/openbsd/host_os.h | 25 +++++++++++++++++++++++++
>   4 files changed, 78 insertions(+), 1 deletion(-)
>   create mode 100644 bsd-user/freebsd/host_os.h
>   create mode 100644 bsd-user/netbsd/host_os.h
>   create mode 100644 bsd-user/openbsd/host_os.h

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PULL 21/24] bsd-user: add arm target_signal.h

[imp]

From: Warner Losh <imp@bsdimp.com>

Add a arm target_signal.h to complete the files currently in the tree. The arm
directory isn't compiled, so it was missing target_signal.h. Update it to the
same level as x86 and sparc. This was abstracted from the target_arch_vmparam.h
file in the bsd-user branch.

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/arm/target_signal.h | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 bsd-user/arm/target_signal.h

diff --git a/bsd-user/arm/target_signal.h b/bsd-user/arm/target_signal.h
new file mode 100644
index 0000000000..02be90a2d1
--- /dev/null
+++ b/bsd-user/arm/target_signal.h
@@ -0,0 +1,31 @@
+/*
+ * ARM target specific signal handling code
+ *
+ *  Copyright (c) 2013 Stacey D. Son
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef TARGET_SIGNAL_H
+#define TARGET_SIGNAL_H
+
+#include "cpu.h"
+
+/* this struct defines a stack used during syscall handling */
+
+static inline abi_ulong get_sp_from_cpustate(CPUARMState *state)
+{
+    return state->regs[R_ESP];
+}
+
+#endif /* TARGET_SIGNAL_H */
-- 
2.22.1

Re: [PULL 21/24] bsd-user: add arm target_signal.h

[Richard Henderson]

On 4/23/21 1:39 PM, imp@bsdimp.com wrote:
> +static inline abi_ulong get_sp_from_cpustate(CPUARMState *state)
> +{
> +    return state->regs[R_ESP];
> +}

Shall we just wait until arm actually compiles?
This won't, being a copy from x86...


r~

Re: [PULL 21/24] bsd-user: add arm target_signal.h

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 459 bytes --]

On Fri, Apr 23, 2021 at 5:19 PM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 4/23/21 1:39 PM, imp@bsdimp.com wrote:
> > +static inline abi_ulong get_sp_from_cpustate(CPUARMState *state)
> > +{
> > +    return state->regs[R_ESP];
> > +}
>
> Shall we just wait until arm actually compiles?
> This won't, being a copy from x86...
>

OK. Fair point. I'll drop this for now and pick it up when more of the arm
is in place. Good catch.


>
> r~
>

[-- Attachment #2: Type: text/html, Size: 1047 bytes --]

[PULL 22/24] bsd-user: create target_arch_cpu.h

[imp]

From: Warner Losh <imp@bsdimp.com>

Create target_arch_cpu.h to house the target_cpu_loop and target_cpu_init
functions. These are the empty files that will be populated by moving the
appropriate cpu-specific functions out of main.c. This work pre-dates the
linux-user work that moved these to cpu-loop.c, so was done differently. As
there's a number of things linux-user did differently than bsd-user in their
time of divergence, and as the recertification of the code to redo it the same
way will take a fair amount of effort, a separate effort to address the
divergence once everything is in the tree and we can create a common qemu-user
directory for the munane common elements between the two.

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/arm/target_arch_cpu.h     | 22 ++++++++++++++++++++++
 bsd-user/i386/target_arch_cpu.h    | 22 ++++++++++++++++++++++
 bsd-user/main.c                    |  1 +
 bsd-user/sparc/target_arch_cpu.h   | 22 ++++++++++++++++++++++
 bsd-user/sparc64/target_arch_cpu.h | 19 +++++++++++++++++++
 bsd-user/x86_64/target_arch_cpu.h  | 19 +++++++++++++++++++
 6 files changed, 105 insertions(+)
 create mode 100644 bsd-user/arm/target_arch_cpu.h
 create mode 100644 bsd-user/i386/target_arch_cpu.h
 create mode 100644 bsd-user/sparc/target_arch_cpu.h
 create mode 100644 bsd-user/sparc64/target_arch_cpu.h
 create mode 100644 bsd-user/x86_64/target_arch_cpu.h

diff --git a/bsd-user/arm/target_arch_cpu.h b/bsd-user/arm/target_arch_cpu.h
new file mode 100644
index 0000000000..71c89174f2
--- /dev/null
+++ b/bsd-user/arm/target_arch_cpu.h
@@ -0,0 +1,22 @@
+/*
+ *  arm cpu init and loop
+ *
+ *  Copyright (c) 2013 Stacey D. Son
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef _TARGET_ARCH_CPU_H_
+#define _TARGET_ARCH_CPU_H_
+
+#endif /* ! _TARGET_ARCH_CPU_H_ */
diff --git a/bsd-user/i386/target_arch_cpu.h b/bsd-user/i386/target_arch_cpu.h
new file mode 100644
index 0000000000..c05e048b9b
--- /dev/null
+++ b/bsd-user/i386/target_arch_cpu.h
@@ -0,0 +1,22 @@
+/*
+ *  i386 cpu init and loop
+ *
+ *  Copyright (c) 2013 Stacey D. Son
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef _TARGET_ARCH_CPU_H_
+#define _TARGET_ARCH_CPU_H_
+
+#endif /* ! _TARGET_ARCH_CPU_H_ */
diff --git a/bsd-user/main.c b/bsd-user/main.c
index fbcc8cc28c..d12b77fbfe 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -41,6 +41,7 @@
 #include "trace/control.h"
 
 #include "host_os.h"
+#include "target_arch_cpu.h"
 
 int singlestep;
 unsigned long mmap_min_addr;
diff --git a/bsd-user/sparc/target_arch_cpu.h b/bsd-user/sparc/target_arch_cpu.h
new file mode 100644
index 0000000000..dcf7694cba
--- /dev/null
+++ b/bsd-user/sparc/target_arch_cpu.h
@@ -0,0 +1,22 @@
+/*
+ *  sparc cpu init and loop
+ *
+ *  Copyright (c) 2013 Stacey D. Son
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef _TARGET_ARCH_CPU_H_
+#define _TARGET_ARCH_CPU_H_
+
+#endif /* ! _TARGET_ARCH_CPU_H_ */
diff --git a/bsd-user/sparc64/target_arch_cpu.h b/bsd-user/sparc64/target_arch_cpu.h
new file mode 100644
index 0000000000..c3962a8e9a
--- /dev/null
+++ b/bsd-user/sparc64/target_arch_cpu.h
@@ -0,0 +1,19 @@
+/*
+ *  sparc64 cpu init and loop
+ *
+ *  Copyright (c) 2013 Stacey D. Son
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#include "../sparc/target_arch_cpu.h"
diff --git a/bsd-user/x86_64/target_arch_cpu.h b/bsd-user/x86_64/target_arch_cpu.h
new file mode 100644
index 0000000000..56cb59ae0b
--- /dev/null
+++ b/bsd-user/x86_64/target_arch_cpu.h
@@ -0,0 +1,19 @@
+/*
+ *  x86_64 cpu init and loop
+ *
+ *  Copyright (c) 2013 Stacey D. Son
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#include "../i386/target_arch_cpu.h"
-- 
2.22.1

Re: [PULL 22/24] bsd-user: create target_arch_cpu.h

[Richard Henderson]

On 4/23/21 1:39 PM, imp@bsdimp.com wrote:
> From: Warner Losh<imp@bsdimp.com>
> 
> Create target_arch_cpu.h to house the target_cpu_loop and target_cpu_init
> functions. These are the empty files that will be populated by moving the
> appropriate cpu-specific functions out of main.c. This work pre-dates the
> linux-user work that moved these to cpu-loop.c, so was done differently. As
> there's a number of things linux-user did differently than bsd-user in their
> time of divergence, and as the recertification of the code to redo it the same
> way will take a fair amount of effort, a separate effort to address the
> divergence once everything is in the tree and we can create a common qemu-user
> directory for the munane common elements between the two.
> 
> Signed-off-by: Stacey Son<sson@FreeBSD.org>
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/arm/target_arch_cpu.h     | 22 ++++++++++++++++++++++
>   bsd-user/i386/target_arch_cpu.h    | 22 ++++++++++++++++++++++
>   bsd-user/main.c                    |  1 +
>   bsd-user/sparc/target_arch_cpu.h   | 22 ++++++++++++++++++++++
>   bsd-user/sparc64/target_arch_cpu.h | 19 +++++++++++++++++++
>   bsd-user/x86_64/target_arch_cpu.h  | 19 +++++++++++++++++++
>   6 files changed, 105 insertions(+)
>   create mode 100644 bsd-user/arm/target_arch_cpu.h
>   create mode 100644 bsd-user/i386/target_arch_cpu.h
>   create mode 100644 bsd-user/sparc/target_arch_cpu.h
>   create mode 100644 bsd-user/sparc64/target_arch_cpu.h
>   create mode 100644 bsd-user/x86_64/target_arch_cpu.h

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PULL 23/24] bsd-user: move x86 (i386 and x86_64) cpu_loop to target_arch_cpu.h

[imp]

From: Warner Losh <imp@bsdimp.com>

Move the x86 version of the cpu_loop to target_arch_cpu.h as
target_cpu_loop. Create a cpu_loop that calls the target_cpu_loop function, but
only for x86 for now. This is code-movement only commit.

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/i386/target_arch_cpu.h | 283 +++++++++++++++++++++++++++++++
 bsd-user/main.c                 | 284 +-------------------------------
 2 files changed, 286 insertions(+), 281 deletions(-)

diff --git a/bsd-user/i386/target_arch_cpu.h b/bsd-user/i386/target_arch_cpu.h
index c05e048b9b..c71b2ac3ec 100644
--- a/bsd-user/i386/target_arch_cpu.h
+++ b/bsd-user/i386/target_arch_cpu.h
@@ -19,4 +19,287 @@
 #ifndef _TARGET_ARCH_CPU_H_
 #define _TARGET_ARCH_CPU_H_
 
+/***********************************************************/
+/* CPUX86 core interface */
+
+uint64_t cpu_get_tsc(CPUX86State *env)
+{
+    return cpu_get_host_ticks();
+}
+
+static void write_dt(void *ptr, unsigned long addr, unsigned long limit,
+                     int flags)
+{
+    unsigned int e1, e2;
+    uint32_t *p;
+    e1 = (addr << 16) | (limit & 0xffff);
+    e2 = ((addr >> 16) & 0xff) | (addr & 0xff000000) | (limit & 0x000f0000);
+    e2 |= flags;
+    p = ptr;
+    p[0] = tswap32(e1);
+    p[1] = tswap32(e2);
+}
+
+static uint64_t *idt_table;
+#ifdef TARGET_X86_64
+static void set_gate64(void *ptr, unsigned int type, unsigned int dpl,
+                       uint64_t addr, unsigned int sel)
+{
+    uint32_t *p, e1, e2;
+    e1 = (addr & 0xffff) | (sel << 16);
+    e2 = (addr & 0xffff0000) | 0x8000 | (dpl << 13) | (type << 8);
+    p = ptr;
+    p[0] = tswap32(e1);
+    p[1] = tswap32(e2);
+    p[2] = tswap32(addr >> 32);
+    p[3] = 0;
+}
+/* only dpl matters as we do only user space emulation */
+static void set_idt(int n, unsigned int dpl)
+{
+    set_gate64(idt_table + n * 2, 0, dpl, 0, 0);
+}
+#else
+static void set_gate(void *ptr, unsigned int type, unsigned int dpl,
+                     uint32_t addr, unsigned int sel)
+{
+    uint32_t *p, e1, e2;
+    e1 = (addr & 0xffff) | (sel << 16);
+    e2 = (addr & 0xffff0000) | 0x8000 | (dpl << 13) | (type << 8);
+    p = ptr;
+    p[0] = tswap32(e1);
+    p[1] = tswap32(e2);
+}
+
+/* only dpl matters as we do only user space emulation */
+static void set_idt(int n, unsigned int dpl)
+{
+    set_gate(idt_table + n, 0, dpl, 0, 0);
+}
+#endif
+
+void target_cpu_loop(CPUX86State *env)
+{
+    CPUState *cs = env_cpu(env);
+    int trapnr;
+    abi_ulong pc;
+    /* target_siginfo_t info; */
+
+    for (;;) {
+        cpu_exec_start(cs);
+        trapnr = cpu_exec(cs);
+        cpu_exec_end(cs);
+        process_queued_cpu_work(cs);
+
+        switch (trapnr) {
+        case 0x80:
+            /* syscall from int $0x80 */
+            if (bsd_type == target_freebsd) {
+                abi_ulong params = (abi_ulong) env->regs[R_ESP] +
+                    sizeof(int32_t);
+                int32_t syscall_nr = env->regs[R_EAX];
+                int32_t arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8;
+
+                if (syscall_nr == TARGET_FREEBSD_NR_syscall) {
+                    get_user_s32(syscall_nr, params);
+                    params += sizeof(int32_t);
+                } else if (syscall_nr == TARGET_FREEBSD_NR___syscall) {
+                    get_user_s32(syscall_nr, params);
+                    params += sizeof(int64_t);
+                }
+                get_user_s32(arg1, params);
+                params += sizeof(int32_t);
+                get_user_s32(arg2, params);
+                params += sizeof(int32_t);
+                get_user_s32(arg3, params);
+                params += sizeof(int32_t);
+                get_user_s32(arg4, params);
+                params += sizeof(int32_t);
+                get_user_s32(arg5, params);
+                params += sizeof(int32_t);
+                get_user_s32(arg6, params);
+                params += sizeof(int32_t);
+                get_user_s32(arg7, params);
+                params += sizeof(int32_t);
+                get_user_s32(arg8, params);
+                env->regs[R_EAX] = do_freebsd_syscall(env,
+                                                      syscall_nr,
+                                                      arg1,
+                                                      arg2,
+                                                      arg3,
+                                                      arg4,
+                                                      arg5,
+                                                      arg6,
+                                                      arg7,
+                                                      arg8);
+            } else { /* if (bsd_type == target_openbsd) */
+                env->regs[R_EAX] = do_openbsd_syscall(env,
+                                                      env->regs[R_EAX],
+                                                      env->regs[R_EBX],
+                                                      env->regs[R_ECX],
+                                                      env->regs[R_EDX],
+                                                      env->regs[R_ESI],
+                                                      env->regs[R_EDI],
+                                                      env->regs[R_EBP]);
+            }
+            if (((abi_ulong)env->regs[R_EAX]) >= (abi_ulong)(-515)) {
+                env->regs[R_EAX] = -env->regs[R_EAX];
+                env->eflags |= CC_C;
+            } else {
+                env->eflags &= ~CC_C;
+            }
+            break;
+#ifndef TARGET_ABI32
+        case EXCP_SYSCALL:
+            /* syscall from syscall instruction */
+            if (bsd_type == target_freebsd) {
+                env->regs[R_EAX] = do_freebsd_syscall(env,
+                                                      env->regs[R_EAX],
+                                                      env->regs[R_EDI],
+                                                      env->regs[R_ESI],
+                                                      env->regs[R_EDX],
+                                                      env->regs[R_ECX],
+                                                      env->regs[8],
+                                                      env->regs[9], 0, 0);
+            } else { /* if (bsd_type == target_openbsd) */
+                env->regs[R_EAX] = do_openbsd_syscall(env,
+                                                      env->regs[R_EAX],
+                                                      env->regs[R_EDI],
+                                                      env->regs[R_ESI],
+                                                      env->regs[R_EDX],
+                                                      env->regs[10],
+                                                      env->regs[8],
+                                                      env->regs[9]);
+            }
+            env->eip = env->exception_next_eip;
+            if (((abi_ulong)env->regs[R_EAX]) >= (abi_ulong)(-515)) {
+                env->regs[R_EAX] = -env->regs[R_EAX];
+                env->eflags |= CC_C;
+            } else {
+                env->eflags &= ~CC_C;
+            }
+            break;
+#endif
+#ifdef notyet
+        case EXCP0B_NOSEG:
+        case EXCP0C_STACK:
+            info.si_signo = SIGBUS;
+            info.si_errno = 0;
+            info.si_code = TARGET_SI_KERNEL;
+            info._sifields._sigfault._addr = 0;
+            queue_signal(env, info.si_signo, &info);
+            break;
+        case EXCP0D_GPF:
+            /* XXX: potential problem if ABI32 */
+#ifndef TARGET_X86_64
+            if (env->eflags & VM_MASK) {
+                handle_vm86_fault(env);
+            } else
+#endif
+            {
+                info.si_signo = SIGSEGV;
+                info.si_errno = 0;
+                info.si_code = TARGET_SI_KERNEL;
+                info._sifields._sigfault._addr = 0;
+                queue_signal(env, info.si_signo, &info);
+            }
+            break;
+        case EXCP0E_PAGE:
+            info.si_signo = SIGSEGV;
+            info.si_errno = 0;
+            if (!(env->error_code & 1)) {
+                info.si_code = TARGET_SEGV_MAPERR;
+            } else {
+                info.si_code = TARGET_SEGV_ACCERR;
+            }
+            info._sifields._sigfault._addr = env->cr[2];
+            queue_signal(env, info.si_signo, &info);
+            break;
+        case EXCP00_DIVZ:
+#ifndef TARGET_X86_64
+            if (env->eflags & VM_MASK) {
+                handle_vm86_trap(env, trapnr);
+            } else
+#endif
+            {
+                /* division by zero */
+                info.si_signo = SIGFPE;
+                info.si_errno = 0;
+                info.si_code = TARGET_FPE_INTDIV;
+                info._sifields._sigfault._addr = env->eip;
+                queue_signal(env, info.si_signo, &info);
+            }
+            break;
+        case EXCP01_DB:
+        case EXCP03_INT3:
+#ifndef TARGET_X86_64
+            if (env->eflags & VM_MASK) {
+                handle_vm86_trap(env, trapnr);
+            } else
+#endif
+            {
+                info.si_signo = SIGTRAP;
+                info.si_errno = 0;
+                if (trapnr == EXCP01_DB) {
+                    info.si_code = TARGET_TRAP_BRKPT;
+                    info._sifields._sigfault._addr = env->eip;
+                } else {
+                    info.si_code = TARGET_SI_KERNEL;
+                    info._sifields._sigfault._addr = 0;
+                }
+                queue_signal(env, info.si_signo, &info);
+            }
+            break;
+        case EXCP04_INTO:
+        case EXCP05_BOUND:
+#ifndef TARGET_X86_64
+            if (env->eflags & VM_MASK) {
+                handle_vm86_trap(env, trapnr);
+            } else
+#endif
+            {
+                info.si_signo = SIGSEGV;
+                info.si_errno = 0;
+                info.si_code = TARGET_SI_KERNEL;
+                info._sifields._sigfault._addr = 0;
+                queue_signal(env, info.si_signo, &info);
+            }
+            break;
+        case EXCP06_ILLOP:
+            info.si_signo = SIGILL;
+            info.si_errno = 0;
+            info.si_code = TARGET_ILL_ILLOPN;
+            info._sifields._sigfault._addr = env->eip;
+            queue_signal(env, info.si_signo, &info);
+            break;
+#endif
+        case EXCP_INTERRUPT:
+            /* just indicate that signals should be handled asap */
+            break;
+#ifdef notyet
+        case EXCP_DEBUG:
+        {
+            int sig;
+
+            sig = gdb_handlesig(env, TARGET_SIGTRAP);
+            if (sig) {
+                info.si_signo = sig;
+                info.si_errno = 0;
+                info.si_code = TARGET_TRAP_BRKPT;
+                queue_signal(env, info.si_signo, &info);
+            }
+            break;
+        }
+#endif
+        default:
+            pc = env->segs[R_CS].base + env->eip;
+            fprintf(stderr,
+                    "qemu: 0x%08lx: unhandled CPU exception 0x%x - aborting\n",
+                    (long)pc, trapnr);
+            abort();
+        }
+        process_pending_signals(env);
+    }
+}
+
 #endif /* ! _TARGET_ARCH_CPU_H_ */
diff --git a/bsd-user/main.c b/bsd-user/main.c
index d12b77fbfe..3ebfa9425d 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -87,288 +87,10 @@ void fork_end(int child)
     }
 }
 
-#ifdef TARGET_I386
-/***********************************************************/
-/* CPUX86 core interface */
-
-uint64_t cpu_get_tsc(CPUX86State *env)
-{
-    return cpu_get_host_ticks();
-}
-
-static void write_dt(void *ptr, unsigned long addr, unsigned long limit,
-                     int flags)
-{
-    unsigned int e1, e2;
-    uint32_t *p;
-    e1 = (addr << 16) | (limit & 0xffff);
-    e2 = ((addr >> 16) & 0xff) | (addr & 0xff000000) | (limit & 0x000f0000);
-    e2 |= flags;
-    p = ptr;
-    p[0] = tswap32(e1);
-    p[1] = tswap32(e2);
-}
-
-static uint64_t *idt_table;
-#ifdef TARGET_X86_64
-static void set_gate64(void *ptr, unsigned int type, unsigned int dpl,
-                       uint64_t addr, unsigned int sel)
-{
-    uint32_t *p, e1, e2;
-    e1 = (addr & 0xffff) | (sel << 16);
-    e2 = (addr & 0xffff0000) | 0x8000 | (dpl << 13) | (type << 8);
-    p = ptr;
-    p[0] = tswap32(e1);
-    p[1] = tswap32(e2);
-    p[2] = tswap32(addr >> 32);
-    p[3] = 0;
-}
-/* only dpl matters as we do only user space emulation */
-static void set_idt(int n, unsigned int dpl)
-{
-    set_gate64(idt_table + n * 2, 0, dpl, 0, 0);
-}
-#else
-static void set_gate(void *ptr, unsigned int type, unsigned int dpl,
-                     uint32_t addr, unsigned int sel)
-{
-    uint32_t *p, e1, e2;
-    e1 = (addr & 0xffff) | (sel << 16);
-    e2 = (addr & 0xffff0000) | 0x8000 | (dpl << 13) | (type << 8);
-    p = ptr;
-    p[0] = tswap32(e1);
-    p[1] = tswap32(e2);
-}
-
-/* only dpl matters as we do only user space emulation */
-static void set_idt(int n, unsigned int dpl)
-{
-    set_gate(idt_table + n, 0, dpl, 0, 0);
-}
-#endif
-
-void cpu_loop(CPUX86State *env)
+#ifdef TARGET_I386 /* stopgap ifdef */
+void cpu_loop(CPUArchState *env)
 {
-    CPUState *cs = env_cpu(env);
-    int trapnr;
-    abi_ulong pc;
-    /* target_siginfo_t info; */
-
-    for (;;) {
-        cpu_exec_start(cs);
-        trapnr = cpu_exec(cs);
-        cpu_exec_end(cs);
-        process_queued_cpu_work(cs);
-
-        switch (trapnr) {
-        case 0x80:
-            /* syscall from int $0x80 */
-            if (bsd_type == target_freebsd) {
-                abi_ulong params = (abi_ulong) env->regs[R_ESP] +
-                    sizeof(int32_t);
-                int32_t syscall_nr = env->regs[R_EAX];
-                int32_t arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8;
-
-                if (syscall_nr == TARGET_FREEBSD_NR_syscall) {
-                    get_user_s32(syscall_nr, params);
-                    params += sizeof(int32_t);
-                } else if (syscall_nr == TARGET_FREEBSD_NR___syscall) {
-                    get_user_s32(syscall_nr, params);
-                    params += sizeof(int64_t);
-                }
-                get_user_s32(arg1, params);
-                params += sizeof(int32_t);
-                get_user_s32(arg2, params);
-                params += sizeof(int32_t);
-                get_user_s32(arg3, params);
-                params += sizeof(int32_t);
-                get_user_s32(arg4, params);
-                params += sizeof(int32_t);
-                get_user_s32(arg5, params);
-                params += sizeof(int32_t);
-                get_user_s32(arg6, params);
-                params += sizeof(int32_t);
-                get_user_s32(arg7, params);
-                params += sizeof(int32_t);
-                get_user_s32(arg8, params);
-                env->regs[R_EAX] = do_freebsd_syscall(env,
-                                                      syscall_nr,
-                                                      arg1,
-                                                      arg2,
-                                                      arg3,
-                                                      arg4,
-                                                      arg5,
-                                                      arg6,
-                                                      arg7,
-                                                      arg8);
-            } else { /* if (bsd_type == target_openbsd) */
-                env->regs[R_EAX] = do_openbsd_syscall(env,
-                                                      env->regs[R_EAX],
-                                                      env->regs[R_EBX],
-                                                      env->regs[R_ECX],
-                                                      env->regs[R_EDX],
-                                                      env->regs[R_ESI],
-                                                      env->regs[R_EDI],
-                                                      env->regs[R_EBP]);
-            }
-            if (((abi_ulong)env->regs[R_EAX]) >= (abi_ulong)(-515)) {
-                env->regs[R_EAX] = -env->regs[R_EAX];
-                env->eflags |= CC_C;
-            } else {
-                env->eflags &= ~CC_C;
-            }
-            break;
-#ifndef TARGET_ABI32
-        case EXCP_SYSCALL:
-            /* syscall from syscall instruction */
-            if (bsd_type == target_freebsd) {
-                env->regs[R_EAX] = do_freebsd_syscall(env,
-                                                      env->regs[R_EAX],
-                                                      env->regs[R_EDI],
-                                                      env->regs[R_ESI],
-                                                      env->regs[R_EDX],
-                                                      env->regs[R_ECX],
-                                                      env->regs[8],
-                                                      env->regs[9], 0, 0);
-            } else { /* if (bsd_type == target_openbsd) */
-                env->regs[R_EAX] = do_openbsd_syscall(env,
-                                                      env->regs[R_EAX],
-                                                      env->regs[R_EDI],
-                                                      env->regs[R_ESI],
-                                                      env->regs[R_EDX],
-                                                      env->regs[10],
-                                                      env->regs[8],
-                                                      env->regs[9]);
-            }
-            env->eip = env->exception_next_eip;
-            if (((abi_ulong)env->regs[R_EAX]) >= (abi_ulong)(-515)) {
-                env->regs[R_EAX] = -env->regs[R_EAX];
-                env->eflags |= CC_C;
-            } else {
-                env->eflags &= ~CC_C;
-            }
-            break;
-#endif
-#ifdef notyet
-        case EXCP0B_NOSEG:
-        case EXCP0C_STACK:
-            info.si_signo = SIGBUS;
-            info.si_errno = 0;
-            info.si_code = TARGET_SI_KERNEL;
-            info._sifields._sigfault._addr = 0;
-            queue_signal(env, info.si_signo, &info);
-            break;
-        case EXCP0D_GPF:
-            /* XXX: potential problem if ABI32 */
-#ifndef TARGET_X86_64
-            if (env->eflags & VM_MASK) {
-                handle_vm86_fault(env);
-            } else
-#endif
-            {
-                info.si_signo = SIGSEGV;
-                info.si_errno = 0;
-                info.si_code = TARGET_SI_KERNEL;
-                info._sifields._sigfault._addr = 0;
-                queue_signal(env, info.si_signo, &info);
-            }
-            break;
-        case EXCP0E_PAGE:
-            info.si_signo = SIGSEGV;
-            info.si_errno = 0;
-            if (!(env->error_code & 1)) {
-                info.si_code = TARGET_SEGV_MAPERR;
-            } else {
-                info.si_code = TARGET_SEGV_ACCERR;
-            }
-            info._sifields._sigfault._addr = env->cr[2];
-            queue_signal(env, info.si_signo, &info);
-            break;
-        case EXCP00_DIVZ:
-#ifndef TARGET_X86_64
-            if (env->eflags & VM_MASK) {
-                handle_vm86_trap(env, trapnr);
-            } else
-#endif
-            {
-                /* division by zero */
-                info.si_signo = SIGFPE;
-                info.si_errno = 0;
-                info.si_code = TARGET_FPE_INTDIV;
-                info._sifields._sigfault._addr = env->eip;
-                queue_signal(env, info.si_signo, &info);
-            }
-            break;
-        case EXCP01_DB:
-        case EXCP03_INT3:
-#ifndef TARGET_X86_64
-            if (env->eflags & VM_MASK) {
-                handle_vm86_trap(env, trapnr);
-            } else
-#endif
-            {
-                info.si_signo = SIGTRAP;
-                info.si_errno = 0;
-                if (trapnr == EXCP01_DB) {
-                    info.si_code = TARGET_TRAP_BRKPT;
-                    info._sifields._sigfault._addr = env->eip;
-                } else {
-                    info.si_code = TARGET_SI_KERNEL;
-                    info._sifields._sigfault._addr = 0;
-                }
-                queue_signal(env, info.si_signo, &info);
-            }
-            break;
-        case EXCP04_INTO:
-        case EXCP05_BOUND:
-#ifndef TARGET_X86_64
-            if (env->eflags & VM_MASK) {
-                handle_vm86_trap(env, trapnr);
-            } else
-#endif
-            {
-                info.si_signo = SIGSEGV;
-                info.si_errno = 0;
-                info.si_code = TARGET_SI_KERNEL;
-                info._sifields._sigfault._addr = 0;
-                queue_signal(env, info.si_signo, &info);
-            }
-            break;
-        case EXCP06_ILLOP:
-            info.si_signo = SIGILL;
-            info.si_errno = 0;
-            info.si_code = TARGET_ILL_ILLOPN;
-            info._sifields._sigfault._addr = env->eip;
-            queue_signal(env, info.si_signo, &info);
-            break;
-#endif
-        case EXCP_INTERRUPT:
-            /* just indicate that signals should be handled asap */
-            break;
-#ifdef notyet
-        case EXCP_DEBUG:
-        {
-            int sig;
-
-            sig = gdb_handlesig(env, TARGET_SIGTRAP);
-            if (sig) {
-                info.si_signo = sig;
-                info.si_errno = 0;
-                info.si_code = TARGET_TRAP_BRKPT;
-                queue_signal(env, info.si_signo, &info);
-            }
-            break;
-        }
-#endif
-        default:
-            pc = env->segs[R_CS].base + env->eip;
-            fprintf(stderr,
-                    "qemu: 0x%08lx: unhandled CPU exception 0x%x - aborting\n",
-                    (long)pc, trapnr);
-            abort();
-        }
-        process_pending_signals(env);
-    }
+    target_cpu_loop(env);
 }
 #endif
 
-- 
2.22.1

Re: [PULL 23/24] bsd-user: move x86 (i386 and x86_64) cpu_loop to target_arch_cpu.h

[Richard Henderson]

On 4/23/21 1:39 PM, imp@bsdimp.com wrote:
> From: Warner Losh<imp@bsdimp.com>
> 
> Move the x86 version of the cpu_loop to target_arch_cpu.h as
> target_cpu_loop. Create a cpu_loop that calls the target_cpu_loop function, but
> only for x86 for now. This is code-movement only commit.
> 
> Signed-off-by: Stacey Son<sson@FreeBSD.org>
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/i386/target_arch_cpu.h | 283 +++++++++++++++++++++++++++++++
>   bsd-user/main.c                 | 284 +-------------------------------
>   2 files changed, 286 insertions(+), 281 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PULL 24/24] bsd-user: move sparc cpu_loop into target_arch_cpu.h as target_cpu_loop

[imp]

From: Warner Losh <imp@bsdimp.com>

Move the sparc cpu_loop out of main.c and into target_arch_cpu.h and
rename it from cpu_loop to target_cpu_loop. Remove the #ifdef around
the catch-all cpu_loop.

Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/main.c                  | 305 -------------------------------
 bsd-user/sparc/target_arch_cpu.h | 300 ++++++++++++++++++++++++++++++
 2 files changed, 300 insertions(+), 305 deletions(-)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index 3ebfa9425d..5791c3a641 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -87,315 +87,10 @@ void fork_end(int child)
     }
 }
 
-#ifdef TARGET_I386 /* stopgap ifdef */
 void cpu_loop(CPUArchState *env)
 {
     target_cpu_loop(env);
 }
-#endif
-
-#ifdef TARGET_SPARC
-#define SPARC64_STACK_BIAS 2047
-
-/* #define DEBUG_WIN */
-/*
- * WARNING: dealing with register windows _is_ complicated. More info
- * can be found at http://www.sics.se/~psm/sparcstack.html
- */
-static inline int get_reg_index(CPUSPARCState *env, int cwp, int index)
-{
-    index = (index + cwp * 16) % (16 * env->nwindows);
-    /*
-     * wrap handling : if cwp is on the last window, then we use the
-     * registers 'after' the end
-     */
-    if (index < 8 && env->cwp == env->nwindows - 1) {
-        index += 16 * env->nwindows;
-    }
-    return index;
-}
-
-/* save the register window 'cwp1' */
-static inline void save_window_offset(CPUSPARCState *env, int cwp1)
-{
-    unsigned int i;
-    abi_ulong sp_ptr;
-
-    sp_ptr = env->regbase[get_reg_index(env, cwp1, 6)];
-#ifdef TARGET_SPARC64
-    if (sp_ptr & 3) {
-        sp_ptr += SPARC64_STACK_BIAS;
-    }
-#endif
-#if defined(DEBUG_WIN)
-    printf("win_overflow: sp_ptr=0x" TARGET_ABI_FMT_lx " save_cwp=%d\n",
-           sp_ptr, cwp1);
-#endif
-    for (i = 0; i < 16; i++) {
-        /* FIXME - what to do if put_user() fails? */
-        put_user_ual(env->regbase[get_reg_index(env, cwp1, 8 + i)], sp_ptr);
-        sp_ptr += sizeof(abi_ulong);
-    }
-}
-
-static void save_window(CPUSPARCState *env)
-{
-#ifndef TARGET_SPARC64
-    unsigned int new_wim;
-    new_wim = ((env->wim >> 1) | (env->wim << (env->nwindows - 1))) &
-        ((1LL << env->nwindows) - 1);
-    save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2));
-    env->wim = new_wim;
-#else
-    /*
-     * cansave is zero if the spill trap handler is triggered by `save` and
-     * nonzero if triggered by a `flushw`
-     */
-    save_window_offset(env, cpu_cwp_dec(env, env->cwp - env->cansave - 2));
-    env->cansave++;
-    env->canrestore--;
-#endif
-}
-
-static void restore_window(CPUSPARCState *env)
-{
-#ifndef TARGET_SPARC64
-    unsigned int new_wim;
-#endif
-    unsigned int i, cwp1;
-    abi_ulong sp_ptr;
-
-#ifndef TARGET_SPARC64
-    new_wim = ((env->wim << 1) | (env->wim >> (env->nwindows - 1))) &
-        ((1LL << env->nwindows) - 1);
-#endif
-
-    /* restore the invalid window */
-    cwp1 = cpu_cwp_inc(env, env->cwp + 1);
-    sp_ptr = env->regbase[get_reg_index(env, cwp1, 6)];
-#ifdef TARGET_SPARC64
-    if (sp_ptr & 3) {
-        sp_ptr += SPARC64_STACK_BIAS;
-    }
-#endif
-#if defined(DEBUG_WIN)
-    printf("win_underflow: sp_ptr=0x" TARGET_ABI_FMT_lx " load_cwp=%d\n",
-           sp_ptr, cwp1);
-#endif
-    for (i = 0; i < 16; i++) {
-        /* FIXME - what to do if get_user() fails? */
-        get_user_ual(env->regbase[get_reg_index(env, cwp1, 8 + i)], sp_ptr);
-        sp_ptr += sizeof(abi_ulong);
-    }
-#ifdef TARGET_SPARC64
-    env->canrestore++;
-    if (env->cleanwin < env->nwindows - 1) {
-        env->cleanwin++;
-    }
-    env->cansave--;
-#else
-    env->wim = new_wim;
-#endif
-}
-
-static void flush_windows(CPUSPARCState *env)
-{
-    int offset, cwp1;
-
-    offset = 1;
-    for (;;) {
-        /* if restore would invoke restore_window(), then we can stop */
-        cwp1 = cpu_cwp_inc(env, env->cwp + offset);
-#ifndef TARGET_SPARC64
-        if (env->wim & (1 << cwp1)) {
-            break;
-        }
-#else
-        if (env->canrestore == 0) {
-            break;
-        }
-        env->cansave++;
-        env->canrestore--;
-#endif
-        save_window_offset(env, cwp1);
-        offset++;
-    }
-    cwp1 = cpu_cwp_inc(env, env->cwp + 1);
-#ifndef TARGET_SPARC64
-    /* set wim so that restore will reload the registers */
-    env->wim = 1 << cwp1;
-#endif
-#if defined(DEBUG_WIN)
-    printf("flush_windows: nb=%d\n", offset - 1);
-#endif
-}
-
-void cpu_loop(CPUSPARCState *env)
-{
-    CPUState *cs = env_cpu(env);
-    int trapnr, ret, syscall_nr;
-    /* target_siginfo_t info; */
-
-    while (1) {
-        cpu_exec_start(cs);
-        trapnr = cpu_exec(cs);
-        cpu_exec_end(cs);
-        process_queued_cpu_work(cs);
-
-        switch (trapnr) {
-#ifndef TARGET_SPARC64
-        case 0x80:
-#else
-        /* FreeBSD uses 0x141 for syscalls too */
-        case 0x141:
-            if (bsd_type != target_freebsd) {
-                goto badtrap;
-            }
-            /* fallthrough */
-        case 0x100:
-#endif
-            syscall_nr = env->gregs[1];
-            if (bsd_type == target_freebsd)
-                ret = do_freebsd_syscall(env, syscall_nr,
-                                         env->regwptr[0], env->regwptr[1],
-                                         env->regwptr[2], env->regwptr[3],
-                                         env->regwptr[4], env->regwptr[5],
-                                         0, 0);
-            else if (bsd_type == target_netbsd)
-                ret = do_netbsd_syscall(env, syscall_nr,
-                                        env->regwptr[0], env->regwptr[1],
-                                        env->regwptr[2], env->regwptr[3],
-                                        env->regwptr[4], env->regwptr[5]);
-            else { /* if (bsd_type == target_openbsd) */
-#if defined(TARGET_SPARC64)
-                syscall_nr &= ~(TARGET_OPENBSD_SYSCALL_G7RFLAG |
-                                TARGET_OPENBSD_SYSCALL_G2RFLAG);
-#endif
-                ret = do_openbsd_syscall(env, syscall_nr,
-                                         env->regwptr[0], env->regwptr[1],
-                                         env->regwptr[2], env->regwptr[3],
-                                         env->regwptr[4], env->regwptr[5]);
-            }
-            if ((unsigned int)ret >= (unsigned int)(-515)) {
-                ret = -ret;
-#if defined(TARGET_SPARC64) && !defined(TARGET_ABI32)
-                env->xcc |= PSR_CARRY;
-#else
-                env->psr |= PSR_CARRY;
-#endif
-            } else {
-#if defined(TARGET_SPARC64) && !defined(TARGET_ABI32)
-                env->xcc &= ~PSR_CARRY;
-#else
-                env->psr &= ~PSR_CARRY;
-#endif
-            }
-            env->regwptr[0] = ret;
-            /* next instruction */
-#if defined(TARGET_SPARC64)
-            if (bsd_type == target_openbsd &&
-                env->gregs[1] & TARGET_OPENBSD_SYSCALL_G2RFLAG) {
-                env->pc = env->gregs[2];
-                env->npc = env->pc + 4;
-            } else if (bsd_type == target_openbsd &&
-                       env->gregs[1] & TARGET_OPENBSD_SYSCALL_G7RFLAG) {
-                env->pc = env->gregs[7];
-                env->npc = env->pc + 4;
-            } else {
-                env->pc = env->npc;
-                env->npc = env->npc + 4;
-            }
-#else
-            env->pc = env->npc;
-            env->npc = env->npc + 4;
-#endif
-            break;
-        case 0x83: /* flush windows */
-#ifdef TARGET_ABI32
-        case 0x103:
-#endif
-            flush_windows(env);
-            /* next instruction */
-            env->pc = env->npc;
-            env->npc = env->npc + 4;
-            break;
-#ifndef TARGET_SPARC64
-        case TT_WIN_OVF: /* window overflow */
-            save_window(env);
-            break;
-        case TT_WIN_UNF: /* window underflow */
-            restore_window(env);
-            break;
-        case TT_TFAULT:
-        case TT_DFAULT:
-#ifdef notyet
-            {
-                info.si_signo = SIGSEGV;
-                info.si_errno = 0;
-                /* XXX: check env->error_code */
-                info.si_code = TARGET_SEGV_MAPERR;
-                info._sifields._sigfault._addr = env->mmuregs[4];
-                queue_signal(env, info.si_signo, &info);
-            }
-#endif
-            break;
-#else
-        case TT_SPILL: /* window overflow */
-            save_window(env);
-            break;
-        case TT_FILL: /* window underflow */
-            restore_window(env);
-            break;
-        case TT_TFAULT:
-        case TT_DFAULT:
-#ifdef notyet
-            {
-                info.si_signo = SIGSEGV;
-                info.si_errno = 0;
-                /* XXX: check env->error_code */
-                info.si_code = TARGET_SEGV_MAPERR;
-                if (trapnr == TT_DFAULT) {
-                    info._sifields._sigfault._addr = env->dmmuregs[4];
-                } else {
-                    info._sifields._sigfault._addr = env->tsptr->tpc;
-                }
-                /* queue_signal(env, info.si_signo, &info); */
-            }
-#endif
-            break;
-#endif
-        case EXCP_INTERRUPT:
-            /* just indicate that signals should be handled asap */
-            break;
-        case EXCP_DEBUG:
-            {
-#ifdef notyet
-                int sig =
-#endif
-                gdb_handlesig(cs, TARGET_SIGTRAP);
-#ifdef notyet
-                if (sig) {
-                    info.si_signo = sig;
-                    info.si_errno = 0;
-                    info.si_code = TARGET_TRAP_BRKPT;
-                    /* queue_signal(env, info.si_signo, &info); */
-                }
-#endif
-            }
-            break;
-        default:
-#ifdef TARGET_SPARC64
-        badtrap:
-#endif
-            printf("Unhandled trap: 0x%x\n", trapnr);
-            cpu_dump_state(cs, stderr, 0);
-            exit(1);
-        }
-        process_pending_signals(env);
-    }
-}
-
-#endif
 
 static void usage(void)
 {
diff --git a/bsd-user/sparc/target_arch_cpu.h b/bsd-user/sparc/target_arch_cpu.h
index dcf7694cba..c2b9c2480e 100644
--- a/bsd-user/sparc/target_arch_cpu.h
+++ b/bsd-user/sparc/target_arch_cpu.h
@@ -19,4 +19,304 @@
 #ifndef _TARGET_ARCH_CPU_H_
 #define _TARGET_ARCH_CPU_H_
 
+#define SPARC64_STACK_BIAS 2047
+
+/* #define DEBUG_WIN */
+/*
+ * WARNING: dealing with register windows _is_ complicated. More info
+ * can be found at http://www.sics.se/~psm/sparcstack.html
+ */
+static inline int get_reg_index(CPUSPARCState *env, int cwp, int index)
+{
+    index = (index + cwp * 16) % (16 * env->nwindows);
+    /*
+     * wrap handling : if cwp is on the last window, then we use the
+     * registers 'after' the end
+     */
+    if (index < 8 && env->cwp == env->nwindows - 1) {
+        index += 16 * env->nwindows;
+    }
+    return index;
+}
+
+/* save the register window 'cwp1' */
+static inline void save_window_offset(CPUSPARCState *env, int cwp1)
+{
+    unsigned int i;
+    abi_ulong sp_ptr;
+
+    sp_ptr = env->regbase[get_reg_index(env, cwp1, 6)];
+#ifdef TARGET_SPARC64
+    if (sp_ptr & 3) {
+        sp_ptr += SPARC64_STACK_BIAS;
+    }
+#endif
+#if defined(DEBUG_WIN)
+    printf("win_overflow: sp_ptr=0x" TARGET_ABI_FMT_lx " save_cwp=%d\n",
+           sp_ptr, cwp1);
+#endif
+    for (i = 0; i < 16; i++) {
+        /* FIXME - what to do if put_user() fails? */
+        put_user_ual(env->regbase[get_reg_index(env, cwp1, 8 + i)], sp_ptr);
+        sp_ptr += sizeof(abi_ulong);
+    }
+}
+
+static void save_window(CPUSPARCState *env)
+{
+#ifndef TARGET_SPARC64
+    unsigned int new_wim;
+    new_wim = ((env->wim >> 1) | (env->wim << (env->nwindows - 1))) &
+        ((1LL << env->nwindows) - 1);
+    save_window_offset(env, cpu_cwp_dec(env, env->cwp - 2));
+    env->wim = new_wim;
+#else
+    /*
+     * cansave is zero if the spill trap handler is triggered by `save` and
+     * nonzero if triggered by a `flushw`
+     */
+    save_window_offset(env, cpu_cwp_dec(env, env->cwp - env->cansave - 2));
+    env->cansave++;
+    env->canrestore--;
+#endif
+}
+
+static void restore_window(CPUSPARCState *env)
+{
+#ifndef TARGET_SPARC64
+    unsigned int new_wim;
+#endif
+    unsigned int i, cwp1;
+    abi_ulong sp_ptr;
+
+#ifndef TARGET_SPARC64
+    new_wim = ((env->wim << 1) | (env->wim >> (env->nwindows - 1))) &
+        ((1LL << env->nwindows) - 1);
+#endif
+
+    /* restore the invalid window */
+    cwp1 = cpu_cwp_inc(env, env->cwp + 1);
+    sp_ptr = env->regbase[get_reg_index(env, cwp1, 6)];
+#ifdef TARGET_SPARC64
+    if (sp_ptr & 3) {
+        sp_ptr += SPARC64_STACK_BIAS;
+    }
+#endif
+#if defined(DEBUG_WIN)
+    printf("win_underflow: sp_ptr=0x" TARGET_ABI_FMT_lx " load_cwp=%d\n",
+           sp_ptr, cwp1);
+#endif
+    for (i = 0; i < 16; i++) {
+        /* FIXME - what to do if get_user() fails? */
+        get_user_ual(env->regbase[get_reg_index(env, cwp1, 8 + i)], sp_ptr);
+        sp_ptr += sizeof(abi_ulong);
+    }
+#ifdef TARGET_SPARC64
+    env->canrestore++;
+    if (env->cleanwin < env->nwindows - 1) {
+        env->cleanwin++;
+    }
+    env->cansave--;
+#else
+    env->wim = new_wim;
+#endif
+}
+
+static void flush_windows(CPUSPARCState *env)
+{
+    int offset, cwp1;
+
+    offset = 1;
+    for (;;) {
+        /* if restore would invoke restore_window(), then we can stop */
+        cwp1 = cpu_cwp_inc(env, env->cwp + offset);
+#ifndef TARGET_SPARC64
+        if (env->wim & (1 << cwp1)) {
+            break;
+        }
+#else
+        if (env->canrestore == 0) {
+            break;
+        }
+        env->cansave++;
+        env->canrestore--;
+#endif
+        save_window_offset(env, cwp1);
+        offset++;
+    }
+    cwp1 = cpu_cwp_inc(env, env->cwp + 1);
+#ifndef TARGET_SPARC64
+    /* set wim so that restore will reload the registers */
+    env->wim = 1 << cwp1;
+#endif
+#if defined(DEBUG_WIN)
+    printf("flush_windows: nb=%d\n", offset - 1);
+#endif
+}
+
+void target_cpu_loop(CPUSPARCState *env)
+{
+    CPUState *cs = env_cpu(env);
+    int trapnr, ret, syscall_nr;
+    /* target_siginfo_t info; */
+
+    while (1) {
+        cpu_exec_start(cs);
+        trapnr = cpu_exec(cs);
+        cpu_exec_end(cs);
+        process_queued_cpu_work(cs);
+
+        switch (trapnr) {
+#ifndef TARGET_SPARC64
+        case 0x80:
+#else
+        /* FreeBSD uses 0x141 for syscalls too */
+        case 0x141:
+            if (bsd_type != target_freebsd) {
+                goto badtrap;
+            }
+            /* fallthrough */
+        case 0x100:
+#endif
+            syscall_nr = env->gregs[1];
+            if (bsd_type == target_freebsd)
+                ret = do_freebsd_syscall(env, syscall_nr,
+                                         env->regwptr[0], env->regwptr[1],
+                                         env->regwptr[2], env->regwptr[3],
+                                         env->regwptr[4], env->regwptr[5],
+                                         0, 0);
+            else if (bsd_type == target_netbsd)
+                ret = do_netbsd_syscall(env, syscall_nr,
+                                        env->regwptr[0], env->regwptr[1],
+                                        env->regwptr[2], env->regwptr[3],
+                                        env->regwptr[4], env->regwptr[5]);
+            else { /* if (bsd_type == target_openbsd) */
+#if defined(TARGET_SPARC64)
+                syscall_nr &= ~(TARGET_OPENBSD_SYSCALL_G7RFLAG |
+                                TARGET_OPENBSD_SYSCALL_G2RFLAG);
+#endif
+                ret = do_openbsd_syscall(env, syscall_nr,
+                                         env->regwptr[0], env->regwptr[1],
+                                         env->regwptr[2], env->regwptr[3],
+                                         env->regwptr[4], env->regwptr[5]);
+            }
+            if ((unsigned int)ret >= (unsigned int)(-515)) {
+                ret = -ret;
+#if defined(TARGET_SPARC64) && !defined(TARGET_ABI32)
+                env->xcc |= PSR_CARRY;
+#else
+                env->psr |= PSR_CARRY;
+#endif
+            } else {
+#if defined(TARGET_SPARC64) && !defined(TARGET_ABI32)
+                env->xcc &= ~PSR_CARRY;
+#else
+                env->psr &= ~PSR_CARRY;
+#endif
+            }
+            env->regwptr[0] = ret;
+            /* next instruction */
+#if defined(TARGET_SPARC64)
+            if (bsd_type == target_openbsd &&
+                env->gregs[1] & TARGET_OPENBSD_SYSCALL_G2RFLAG) {
+                env->pc = env->gregs[2];
+                env->npc = env->pc + 4;
+            } else if (bsd_type == target_openbsd &&
+                       env->gregs[1] & TARGET_OPENBSD_SYSCALL_G7RFLAG) {
+                env->pc = env->gregs[7];
+                env->npc = env->pc + 4;
+            } else {
+                env->pc = env->npc;
+                env->npc = env->npc + 4;
+            }
+#else
+            env->pc = env->npc;
+            env->npc = env->npc + 4;
+#endif
+            break;
+        case 0x83: /* flush windows */
+#ifdef TARGET_ABI32
+        case 0x103:
+#endif
+            flush_windows(env);
+            /* next instruction */
+            env->pc = env->npc;
+            env->npc = env->npc + 4;
+            break;
+#ifndef TARGET_SPARC64
+        case TT_WIN_OVF: /* window overflow */
+            save_window(env);
+            break;
+        case TT_WIN_UNF: /* window underflow */
+            restore_window(env);
+            break;
+        case TT_TFAULT:
+        case TT_DFAULT:
+#ifdef notyet
+            {
+                info.si_signo = SIGSEGV;
+                info.si_errno = 0;
+                /* XXX: check env->error_code */
+                info.si_code = TARGET_SEGV_MAPERR;
+                info._sifields._sigfault._addr = env->mmuregs[4];
+                queue_signal(env, info.si_signo, &info);
+            }
+#endif
+            break;
+#else
+        case TT_SPILL: /* window overflow */
+            save_window(env);
+            break;
+        case TT_FILL: /* window underflow */
+            restore_window(env);
+            break;
+        case TT_TFAULT:
+        case TT_DFAULT:
+#ifdef notyet
+            {
+                info.si_signo = SIGSEGV;
+                info.si_errno = 0;
+                /* XXX: check env->error_code */
+                info.si_code = TARGET_SEGV_MAPERR;
+                if (trapnr == TT_DFAULT) {
+                    info._sifields._sigfault._addr = env->dmmuregs[4];
+                } else {
+                    info._sifields._sigfault._addr = env->tsptr->tpc;
+                }
+                /* queue_signal(env, info.si_signo, &info); */
+            }
+#endif
+            break;
+#endif
+        case EXCP_INTERRUPT:
+            /* just indicate that signals should be handled asap */
+            break;
+        case EXCP_DEBUG:
+            {
+#ifdef notyet
+                int sig =
+#endif
+                gdb_handlesig(cs, TARGET_SIGTRAP);
+#ifdef notyet
+                if (sig) {
+                    info.si_signo = sig;
+                    info.si_errno = 0;
+                    info.si_code = TARGET_TRAP_BRKPT;
+                    /* queue_signal(env, info.si_signo, &info); */
+                }
+#endif
+            }
+            break;
+        default:
+#ifdef TARGET_SPARC64
+        badtrap:
+#endif
+            printf("Unhandled trap: 0x%x\n", trapnr);
+            cpu_dump_state(cs, stderr, 0);
+            exit(1);
+        }
+        process_pending_signals(env);
+    }
+}
+
 #endif /* ! _TARGET_ARCH_CPU_H_ */
-- 
2.22.1

Re: [PULL 24/24] bsd-user: move sparc cpu_loop into target_arch_cpu.h as target_cpu_loop

[Richard Henderson]

On 4/23/21 1:39 PM, imp@bsdimp.com wrote:
> From: Warner Losh<imp@bsdimp.com>
> 
> Move the sparc cpu_loop out of main.c and into target_arch_cpu.h and
> rename it from cpu_loop to target_cpu_loop. Remove the #ifdef around
> the catch-all cpu_loop.
> 
> Signed-off-by: Stacey Son<sson@FreeBSD.org>
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/main.c                  | 305 -------------------------------
>   bsd-user/sparc/target_arch_cpu.h | 300 ++++++++++++++++++++++++++++++
>   2 files changed, 300 insertions(+), 305 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PULL 10/24] bsd-user: style tweak: use C not C++ comments

[Richard Henderson]

On 4/23/21 1:39 PM, imp@bsdimp.com wrote:
> From: Warner Losh <imp@bsdimp.com>
> 
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>   bsd-user/qemu.h | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
> index b836b603af..3480db890d 100644
> --- a/bsd-user/qemu.h
> +++ b/bsd-user/qemu.h
> @@ -71,7 +71,7 @@ struct image_info {
>   
>   struct sigqueue {
>       struct sigqueue *next;
> -    //target_siginfo_t info;
> +    /* target_siginfo_t info; */
>   };

Ack.

> @@ -193,9 +193,11 @@ extern int do_strace;
>   /* signal.c */
>   void process_pending_signals(CPUArchState *cpu_env);
>   void signal_init(void);
> -//int queue_signal(CPUArchState *env, int sig, target_siginfo_t *info);
> -//void host_to_target_siginfo(target_siginfo_t *tinfo, const siginfo_t *info);
> -//void target_to_host_siginfo(siginfo_t *info, const target_siginfo_t *tinfo);
> +/*
> + * int queue_signal(CPUArchState *env, int sig, target_siginfo_t *info);
> + * void host_to_target_siginfo(target_siginfo_t *tinfo, const siginfo_t *info);
> + * void target_to_host_siginfo(siginfo_t *info, const target_siginfo_t *tinfo);
> + */

These should probably be deleted with the #if 0 code that references them.


r~

Re: [PULL 10/24] bsd-user: style tweak: use C not C++ comments

[Warner Losh]

[-- Attachment #1: Type: text/plain, Size: 1453 bytes --]

On Fri, Apr 23, 2021 at 5:06 PM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 4/23/21 1:39 PM, imp@bsdimp.com wrote:
> > From: Warner Losh <imp@bsdimp.com>
> >
> > Signed-off-by: Warner Losh <imp@bsdimp.com>
> > ---
> >   bsd-user/qemu.h | 10 ++++++----
> >   1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
> > index b836b603af..3480db890d 100644
> > --- a/bsd-user/qemu.h
> > +++ b/bsd-user/qemu.h
> > @@ -71,7 +71,7 @@ struct image_info {
> >
> >   struct sigqueue {
> >       struct sigqueue *next;
> > -    //target_siginfo_t info;
> > +    /* target_siginfo_t info; */
> >   };
>
> Ack.
>
> > @@ -193,9 +193,11 @@ extern int do_strace;
> >   /* signal.c */
> >   void process_pending_signals(CPUArchState *cpu_env);
> >   void signal_init(void);
> > -//int queue_signal(CPUArchState *env, int sig, target_siginfo_t *info);
> > -//void host_to_target_siginfo(target_siginfo_t *tinfo, const siginfo_t
> *info);
> > -//void target_to_host_siginfo(siginfo_t *info, const target_siginfo_t
> *tinfo);
> > +/*
> > + * int queue_signal(CPUArchState *env, int sig, target_siginfo_t *info);
> > + * void host_to_target_siginfo(target_siginfo_t *tinfo, const siginfo_t
> *info);
> > + * void target_to_host_siginfo(siginfo_t *info, const target_siginfo_t
> *tinfo);
> > + */
>
> These should probably be deleted with the #if 0 code that references them.
>

OK. Sold.

Warner


>
> r~
>

[-- Attachment #2: Type: text/html, Size: 2353 bytes --]