Re: [PATCH 3/3] meson: Disable CONFIG_NOTIFY1 on FreeBSD

[Kyle Evans]

On 2/5/24 09:31, Daniel P. Berrangé wrote:

On Mon, Feb 05, 2024 at 08:23:41AM -0700, Warner Losh wrote:

On Wed, Jan 31, 2024 at 9:42 AM Daniel P. Berrangé 
wrote:


On Wed, Jan 31, 2024 at 05:24:10PM +0100, Philippe Mathieu-Daudé wrote:

> [... snip ...]

On 25/1/24 20:48, Ilya Leoshkevich wrote:

make vm-build-freebsd fails with:

  ld: error: undefined symbol: inotify_init1
  >>> referenced by filemonitor-inotify.c:183

(../src/util/filemonitor-inotify.c:183)

  >>>

  util_filemonitor-inotify.c.o:(qemu_file_monitor_new) in archive
libqemuutil.a


On FreeBSD inotify functions are defined in libinotify.so, so it might
be tempting to add it to the dependencies. Doing so, however, reveals
that this library handles rename events differently from Linux:

  $ FILEMONITOR_DEBUG=1 build/tests/unit/test-util-filemonitor
  Rename /tmp/test-util-filemonitor-K13LI2/fish/one.txt ->

/tmp/test-util-filemonitor-K13LI2/two.txt

  Event id=2 event=2 file=one.txt
  Queue event id 2 event 2 file one.txt
  Queue event id 1 event 2 file two.txt
  Queue event id 10002 event 2 file two.txt
  Queue event id 1 event 0 file two.txt
  Queue event id 10002 event 0 file two.txt
  Event id=1 event=0 file=two.txt
  Expected event 0 but got 2


Interesting. So In the "Rename" test, the destination already exists.

BSD is thus reporting that 'two.txt' is deleted, before being (re)created
Linux is only reporting 'two.txt' is created.

I don't think we can easily paper over this difference. The easiest is
probably to conditionalize the test

  git diff
diff --git a/tests/unit/test-util-filemonitor.c
b/tests/unit/test-util-filemonitor.c
index a22de27595..c3b2006365 100644
--- a/tests/unit/test-util-filemonitor.c
+++ b/tests/unit/test-util-filemonitor.c
@@ -281,6 +281,14 @@ test_file_monitor_events(void)
  { .type = QFILE_MONITOR_TEST_OP_EVENT,
.filesrc = "one.txt", .watchid = ,
.eventid = QFILE_MONITOR_EVENT_DELETED },
+#ifdef __FreeBSD__
+{ .type = QFILE_MONITOR_TEST_OP_EVENT,
+  .filesrc = "two.txt", .watchid = ,
+  .eventid = QFILE_MONITOR_EVENT_DELETED },
+{ .type = QFILE_MONITOR_TEST_OP_EVENT,
+  .filesrc = "two.txt", .watchid = ,
+  .eventid = QFILE_MONITOR_EVENT_DELETED },
+#endif
  { .type = QFILE_MONITOR_TEST_OP_EVENT,
.filesrc = "two.txt", .watchid = ,
.eventid = QFILE_MONITOR_EVENT_CREATED },



I agree this is likely the best course of action. Has anybody filed a bug
at https://bugs.freebsd.org?


I've not, and I'm not even sure I would class it a FreeBSD bug. Other
than the fact that it differs from Linux behaviour, it feels like it
is reasonble semantics to emit a 'delete' event in this scenario so
that an event consumer can detect replacement of an existing file.



FWIW, +1... unless we miss the follow-up notification that it's been 
created, I'd personally put it into the WONTFIX bucket pretty quickly.


Barring some kind of NOTE_COVER (bad name) that can be emitted if a file 
is simply replaced by another, a directory being reported as shortened 
then extended is a valid and useful representation of the situation 
(even if not completely accurate) to avoid consumers missing the action 
entirely.


Thanks,

Kyle Evans

Re: [PATCH 1/7] bsd-user: spelling fixes

[Kyle Evans]

On 9/11/23 03:39, Michael Tokarev wrote:

Warner, Kyle, can you take a look please?

https://patchew.org/QEMU/20230909131258.354675-1-...@tls.msk.ru/20230909131258.354675-2-...@tls.msk.ru/



Hmm, the original for this doesn't seem to have landed in my inbox, but 
these all look OK to me.




09.09.2023 16:12, Michael Tokarev:

Signed-off-by: Michael Tokarev 
---
  bsd-user/errno_defs.h    | 2 +-
  bsd-user/freebsd/target_os_siginfo.h | 2 +-
  bsd-user/freebsd/target_os_stack.h   | 4 ++--
  bsd-user/freebsd/target_os_user.h    | 2 +-
  bsd-user/qemu.h  | 2 +-
  bsd-user/signal-common.h | 4 ++--
  bsd-user/signal.c    | 6 +++---
  7 files changed, 11 insertions(+), 11 deletions(-)




Reviewed-by: Kyle Evans 

Thanks,

Kyle Evans

Re: [PATCH 8/9] bsd-user: implement sysctlbyname(2)

[Kyle Evans]

On Sat, Feb 11, 2023 at 5:13 PM Richard Henderson
 wrote:
>
> On 2/10/23 13:18, Warner Losh wrote:
> > From: Kyle Evans 
> >
> > do_freebsd_sysctlbyname needs to translate the 'name' back down to a OID
> > so we can intercept the special ones. Do that and call the common wrapper
> > do_freebsd_sysctl_oid.
> >
> > Signed-off-by: Kyle Evans 
> > Signed-off-by: Warner Losh 
> > ---
> >   bsd-user/freebsd/os-sys.c | 58 +++
> >   bsd-user/freebsd/os-syscall.c |  4 +++
> >   2 files changed, 62 insertions(+)
> >
> > diff --git a/bsd-user/freebsd/os-sys.c b/bsd-user/freebsd/os-sys.c
> > index 13736936e5f..62c729dfe47 100644
> > --- a/bsd-user/freebsd/os-sys.c
> > +++ b/bsd-user/freebsd/os-sys.c
> > @@ -345,6 +345,64 @@ out:
> >   return ret;
> >   }
> >
> > +/*
> > + * This syscall was created to make sysctlbyname(3) more efficient.
> > + * Unfortunately, because we have to fake some sysctls, we can't do that.
>
> Can't do what?  Directly use sysctlbyname?
>

How about:

/*
 * This syscall was created to make sysctlbyname(3) more efficient, but
 * we can't really provide it in bsd-user.  Notably, we must always translate
 * the names independently since some sysctl values have to be faked
 * for the target environment, so it still has to break down to two syscalls
 * for the underlying implementation.
 */

> > +if (oldlenp) {
> > +if (get_user_ual(oldlen, oldlenp)) {
> > +return -TARGET_EFAULT;
> > +}
>
> Same comment about verifying write early.
>
> > +unlock_user(holdp, oldp, holdlen);
>
> And writeback vs error.
>
>
> r~

Re: [PATCH] linux-user,bsd-user: re-exec with G_SLICE=always-malloc

[Kyle Evans]

On Thu, Oct 6, 2022 at 11:29 AM Richard Henderson
 wrote:
>
> On 10/6/22 11:12, Daniel P. Berrangé wrote:
> > The only possible silver linining is that in static linked builds,
> > it appears that a QEMU constructor with priority 101, will pre-empt
> > the constructor from any library. This is kind of crazy, as it means
> > if any library or app code uses priorities, it'll get totally different
> > execution ordering depending on whether it is dynamic or statically
> > built.
>
> Plausible...
>

> > I guess we could rely on this hack if we declare that everyone using
> > binfmt is probably relying on static linked QEMU, and in non-binfmt
> > cases people can set the env var themselves.  It still feels pretty
> > dirty.
>
> ... but as you say, dirty.

FWIW, on FreeBSD at least, we don't support dynamically linked
bsd-user and I'd go as far as to say that we have no desire to change
that in the future, either -- the benefits are simply not there to
outweigh the pain for our use-cases.

Thanks,

Kyle Evans

Re: [PATCH] tests: Add sndio to the FreeBSD CI containers / VM

[Kyle Evans]

On Wed, Sep 28, 2022 at 10:07 PM Brad Smith  wrote:
>
> tests: Add sndio to the FreeBSD CI containers / VM
>
> Signed-off-by: Brad Smith 
> ---
>  .gitlab-ci.d/cirrus/freebsd-12.vars | 2 +-
>  .gitlab-ci.d/cirrus/freebsd-13.vars | 2 +-
>  tests/vm/freebsd| 3 +++
>  3 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/.gitlab-ci.d/cirrus/freebsd-12.vars 
> b/.gitlab-ci.d/cirrus/freebsd-12.vars
> index 1a5959810f..4a2b41a46c 100644
> --- a/.gitlab-ci.d/cirrus/freebsd-12.vars
> +++ b/.gitlab-ci.d/cirrus/freebsd-12.vars
> @@ -11,6 +11,6 @@ MAKE='/usr/local/bin/gmake'
>  NINJA='/usr/local/bin/ninja'
>  PACKAGING_COMMAND='pkg'
>  PIP3='/usr/local/bin/pip-3.8'
> -PKGS='alsa-lib bash bzip2 ca_root_nss capstone4 ccache cdrkit-genisoimage 
> cmocka ctags curl cyrus-sasl dbus diffutils dtc fusefs-libs3 gettext git glib 
> gmake gnutls gsed gtk3 json-c libepoxy libffi libgcrypt libjpeg-turbo libnfs 
> libslirp libspice-server libssh libtasn1 llvm lzo2 meson ncurses nettle ninja 
> opencv perl5 pixman pkgconf png py39-numpy py39-pillow py39-pip py39-sphinx 
> py39-sphinx_rtd_theme py39-yaml python3 rpm2cpio sdl2 sdl2_image snappy 
> spice-protocol tesseract texinfo usbredir virglrenderer vte3 zstd'
> +PKGS='alsa-lib bash bzip2 ca_root_nss capstone4 ccache cdrkit-genisoimage 
> cmocka ctags curl cyrus-sasl dbus diffutils dtc fusefs-libs3 gettext git glib 
> gmake gnutls gsed gtk3 json-c libepoxy libffi libgcrypt libjpeg-turbo libnfs 
> libslirp libspice-server libssh libtasn1 llvm lzo2 meson ncurses nettle ninja 
> opencv perl5 pixman pkgconf png py39-numpy py39-pillow py39-pip py39-sphinx 
> py39-sphinx_rtd_theme py39-yaml python3 rpm2cpio sdl2 sdl2_image snappy sndio 
> spice-protocol tesseract texinfo usbredir virglrenderer vte3 zstd'
>  PYPI_PKGS=''
>  PYTHON='/usr/local/bin/python3'
> diff --git a/.gitlab-ci.d/cirrus/freebsd-13.vars 
> b/.gitlab-ci.d/cirrus/freebsd-13.vars
> index 5e5aafd7e5..dc306aa858 100644
> --- a/.gitlab-ci.d/cirrus/freebsd-13.vars
> +++ b/.gitlab-ci.d/cirrus/freebsd-13.vars
> @@ -11,6 +11,6 @@ MAKE='/usr/local/bin/gmake'
>  NINJA='/usr/local/bin/ninja'
>  PACKAGING_COMMAND='pkg'
>  PIP3='/usr/local/bin/pip-3.8'
> -PKGS='alsa-lib bash bzip2 ca_root_nss capstone4 ccache cdrkit-genisoimage 
> cmocka ctags curl cyrus-sasl dbus diffutils dtc fusefs-libs3 gettext git glib 
> gmake gnutls gsed gtk3 json-c libepoxy libffi libgcrypt libjpeg-turbo libnfs 
> libslirp libspice-server libssh libtasn1 llvm lzo2 meson ncurses nettle ninja 
> opencv perl5 pixman pkgconf png py39-numpy py39-pillow py39-pip py39-sphinx 
> py39-sphinx_rtd_theme py39-yaml python3 rpm2cpio sdl2 sdl2_image snappy 
> spice-protocol tesseract texinfo usbredir virglrenderer vte3 zstd'
> +PKGS='alsa-lib bash bzip2 ca_root_nss capstone4 ccache cdrkit-genisoimage 
> cmocka ctags curl cyrus-sasl dbus diffutils dtc fusefs-libs3 gettext git glib 
> gmake gnutls gsed gtk3 json-c libepoxy libffi libgcrypt libjpeg-turbo libnfs 
> libslirp libspice-server libssh libtasn1 llvm lzo2 meson ncurses nettle ninja 
> opencv perl5 pixman pkgconf png py39-numpy py39-pillow py39-pip py39-sphinx 
> py39-sphinx_rtd_theme py39-yaml python3 rpm2cpio sdl2 sdl2_image snappy sndio 
> spice-protocol tesseract texinfo usbredir virglrenderer vte3 zstd'
>  PYPI_PKGS=''
>  PYTHON='/usr/local/bin/python3'
> diff --git a/tests/vm/freebsd b/tests/vm/freebsd
> index 3643fe325d..d6ff4461ba 100755
> --- a/tests/vm/freebsd
> +++ b/tests/vm/freebsd
> @@ -66,6 +66,9 @@ class FreeBSDVM(basevm.BaseVM):
>
>  # libs: networking
>  "libslirp",
> +
> +# libs: sndio
> +"sndio",
>  ]
>
>  BUILD_SCRIPT = """
> --

I'm afraid I'm not at all familiar with qemu's test setup, but sndio's
a valid pkg name and I can deduce well enough from the context that
these are indeed names that will be more or less passed to `pkg
install` and thus, installed in the respective environments. FWIW:

Reviewed-by: Kyle Evans 

Re: [PATCH 22/22] bsd-user/freebsd/os-syscall.c: Implement exit

[Kyle Evans]

On Tue, Feb 1, 2022 at 5:15 AM Warner Losh  wrote:
>
> Implement the exit system call. Bring in bsd-proc.h to contain all the
> process system call implementation and helper routines.
>
> Signed-off-by: Stacey Son 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/bsd-proc.h   | 43 +++
>  bsd-user/freebsd/os-syscall.c |  7 ++
>  2 files changed, 50 insertions(+)
>  create mode 100644 bsd-user/bsd-proc.h
>

Reviewed-by: Kyle Evans 

> diff --git a/bsd-user/bsd-proc.h b/bsd-user/bsd-proc.h
> new file mode 100644
> index 000..8f0b6990d14
> --- /dev/null
> +++ b/bsd-user/bsd-proc.h
> @@ -0,0 +1,43 @@
> +/*
> + *  process related system call shims and definitions
> + *
> + *  Copyright (c) 2013-2014 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 BSD_PROC_H_
> +#define BSD_PROC_H_
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* exit(2) */
> +static inline abi_long do_bsd_exit(void *cpu_env, abi_long arg1)
> +{
> +#ifdef TARGET_GPROF
> +_mcleanup();
> +#endif
> +gdb_exit(arg1);
> +qemu_plugin_user_exit();
> +/* XXX: should free thread stack and CPU env here  */
> +_exit(arg1);
> +
> +return 0;
> +}
> +
> +#endif /* !BSD_PROC_H_ */
> diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
> index f52c9e3c306..f66b6a1b1f5 100644
> --- a/bsd-user/freebsd/os-syscall.c
> +++ b/bsd-user/freebsd/os-syscall.c
> @@ -41,6 +41,7 @@
>  #include "user/syscall-trace.h"
>
>  #include "bsd-file.h"
> +#include "bsd-proc.h"
>
>  /* I/O */
>  safe_syscall3(ssize_t, read, int, fd, void *, buf, size_t, nbytes);
> @@ -227,6 +228,12 @@ abi_long do_freebsd_syscall(void *cpu_env, int num, 
> abi_long arg1,
>  }
>
>  switch (num) {
> +/*
> + * process system calls
> + */
> +case TARGET_FREEBSD_NR_exit: /* exit(2) */
> +ret = do_bsd_exit(cpu_env, arg1);
> +break;
>
>  /*
>   * File system calls.
> --
> 2.33.1
>

Re: [PATCH 13/22] bsd-user/bsd-file.h: Implementation details for the filesystem calls

[Kyle Evans]

On Tue, Feb 1, 2022 at 5:15 AM Warner Losh  wrote:
>
> An include file that pulls in all the definitions needed for the file
> related system calls. This also includes the host definitions to
> implement the system calls and some helper routines to lock/unlock
> different aspects of the system call arguments.
>
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/bsd-file.h   | 39 +++
>  bsd-user/freebsd/os-syscall.c |  2 ++
>  2 files changed, 41 insertions(+)
>  create mode 100644 bsd-user/bsd-file.h
>

Reviewed-by: Kyle Evans 

> diff --git a/bsd-user/bsd-file.h b/bsd-user/bsd-file.h
> new file mode 100644
> index 000..2f743db38e1
> --- /dev/null
> +++ b/bsd-user/bsd-file.h
> @@ -0,0 +1,39 @@
> +/*
> + *  file related system call shims 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 BSD_FILE_H_
> +#define BSD_FILE_H_
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "qemu/path.h"
> +
> +extern struct iovec *lock_iovec(int type, abi_ulong target_addr, int count,
> +int copy);
> +extern void unlock_iovec(struct iovec *vec, abi_ulong target_addr, int count,
> +int copy);
> +
> +#endif /* !BSD_FILE_H_ */
> diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
> index 2e84cf350b1..060134a9ecd 100644
> --- a/bsd-user/freebsd/os-syscall.c
> +++ b/bsd-user/freebsd/os-syscall.c
> @@ -40,6 +40,8 @@
>  #include "signal-common.h"
>  #include "user/syscall-trace.h"
>
> +#include "bsd-file.h"
> +
>  void target_set_brk(abi_ulong new_brk)
>  {
>  }
> --
> 2.33.1
>

Re: [PATCH 07/22] bsd-user/x86_64/target_arch_thread.h: Assume a FreeBSD target

[Kyle Evans]

On Tue, Feb 1, 2022 at 5:15 AM Warner Losh  wrote:
>
> Since we can't run on anything else, assume for the moment that this is
> a FreeBSD target. In the future, we'll need to handle this properly via
> some include file in bsd-user/*bsd/x86_64/mumble.h. There's a number of
> other diffs that would be needed to make things work on OtherBSD, so it
> doesn't make sense to preseve this one detail today.
>
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/x86_64/target_arch_thread.h | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>

Reviewed-by: Kyle Evans 

> diff --git a/bsd-user/x86_64/target_arch_thread.h 
> b/bsd-user/x86_64/target_arch_thread.h
> index d105e43fd35..b745d7ffeb7 100644
> --- a/bsd-user/x86_64/target_arch_thread.h
> +++ b/bsd-user/x86_64/target_arch_thread.h
> @@ -32,9 +32,7 @@ static inline void target_thread_init(struct target_pt_regs 
> *regs,
>  regs->rax = 0;
>  regs->rsp = infop->start_stack;
>  regs->rip = infop->entry;
> -if (bsd_type == target_freebsd) {
> -regs->rdi = infop->start_stack;
> -}
> +regs->rdi = infop->start_stack;
>  }
>
>  #endif /* !_TARGET_ARCH_THREAD_H_ */
> --
> 2.33.1
>

Re: [PATCH 15/22] bsd-user/freebsd/os-syscall.c: unlock_iovec

[Kyle Evans]

On Tue, Feb 1, 2022 at 5:15 AM Warner Losh  wrote:
>
> Releases the references to the iovec created by lock_iovec.
>
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/freebsd/os-syscall.c | 23 +++
>  1 file changed, 23 insertions(+)
>

Reviewed-by: Kyle Evans 

> diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
> index c21759ae7ce..d49945f0fcc 100644
> --- a/bsd-user/freebsd/os-syscall.c
> +++ b/bsd-user/freebsd/os-syscall.c
> @@ -167,6 +167,29 @@ struct iovec *lock_iovec(int type, abi_ulong target_addr,
>  return NULL;
>  }
>
> +void unlock_iovec(struct iovec *vec, abi_ulong target_addr,
> +int count, int copy)
> +{
> +struct target_iovec *target_vec;
> +int i;
> +
> +target_vec = lock_user(VERIFY_READ, target_addr,
> +   count * sizeof(struct target_iovec), 1);
> +if (target_vec) {
> +for (i = 0; i < count; i++) {
> +abi_ulong base = tswapal(target_vec[i].iov_base);
> +abi_long len = tswapal(target_vec[i].iov_len);
> +if (len < 0) {
> +break;
> +}
> +unlock_user(vec[i].iov_base, base, copy ? vec[i].iov_len : 0);
> +}
> +unlock_user(target_vec, target_addr, 0);
> +}
> +
> +free(vec);
> +}
> +
>  /*
>   * do_syscall() should always have a single exit point at the end so that
>   * actions, such as logging of syscall results, can be performed.  All errnos
> --
> 2.33.1
>

Re: [PATCH 14/22] bsd-user/freebsd/os-syscall.c: lock_iovec

[Kyle Evans]

On Tue, Feb 1, 2022 at 5:15 AM Warner Losh  wrote:
>
> lock_iovec will lock an I/O vec and the memory to which it referrs and
> create a iovec in the host space that referrs to it, with full error
> unwinding.
>

s/referrs/refers/ twice

> Signed-off-by: Warner Losh 
> ---
>  bsd-user/freebsd/os-syscall.c | 92 +++
>  1 file changed, 92 insertions(+)
>

Two typos, otherwise seems to LGTM:

Reviewed-by: Kyle Evans 

> diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
> index 060134a9ecd..c21759ae7ce 100644
> --- a/bsd-user/freebsd/os-syscall.c
> +++ b/bsd-user/freebsd/os-syscall.c
> @@ -75,6 +75,98 @@ bool is_error(abi_long ret)
>  return (abi_ulong)ret >= (abi_ulong)(-4096);
>  }
>
> +struct iovec *lock_iovec(int type, abi_ulong target_addr,
> +int count, int copy)
> +{
> +struct target_iovec *target_vec;
> +struct iovec *vec;
> +abi_ulong total_len, max_len;
> +int i;
> +int err = 0;
> +bool bad_address = false;
> +
> +if (count == 0) {
> +errno = 0;
> +return NULL;
> +}
> +if (count < 0 || count > IOV_MAX) {
> +errno = EINVAL;
> +return NULL;
> +}
> +
> +vec = calloc(count, sizeof(struct iovec));
> +if (vec == NULL) {
> +errno = ENOMEM;
> +return NULL;
> +}
> +
> +target_vec = lock_user(VERIFY_READ, target_addr,
> +   count * sizeof(struct target_iovec), 1);
> +if (target_vec == NULL) {
> +err = EFAULT;
> +goto fail2;
> +}
> +
> +/*
> + * ??? If host page size > target page size, this will result in a value
> + * larger than what we can actually support.
> + */
> +max_len = 0x7fff & TARGET_PAGE_MASK;
> +total_len = 0;
> +
> +for (i = 0; i < count; i++) {
> +abi_ulong base = tswapal(target_vec[i].iov_base);
> +abi_long len = tswapal(target_vec[i].iov_len);
> +
> +if (len < 0) {
> +err = EINVAL;
> +goto fail;
> +} else if (len == 0) {
> +/* Zero length pointer is ignored.  */
> +vec[i].iov_base = 0;
> +} else {
> +vec[i].iov_base = lock_user(type, base, len, copy);
> +/*
> + * If the first buffer pointer is bad, this is a fault.  But
> + * subsequent bad buffers will result in a partial write; this is
> + * realized by filling the vector with null pointers and zero
> + * lengths.
> + */
> +if (!vec[i].iov_base) {
> +if (i == 0) {
> +err = EFAULT;
> +goto fail;
> +} else {
> +bad_address = true;
> +}
> +}
> +if (bad_address) {
> +len = 0;
> +}
> +if (len > max_len - total_len) {
> +len = max_len - total_len;
> +}
> +}
> +vec[i].iov_len = len;
> +total_len += len;
> +}
> +
> +unlock_user(target_vec, target_addr, 0);
> +return vec;
> +
> + fail:
> +while (--i >= 0) {
> +if (tswapal(target_vec[i].iov_len) > 0) {
> +unlock_user(vec[i].iov_base, tswapal(target_vec[i].iov_base), 0);
> +}
> +}
> +unlock_user(target_vec, target_addr, 0);
> + fail2:
> +free(vec);
> +errno = err;
> +return NULL;
> +}
> +
>  /*
>   * do_syscall() should always have a single exit point at the end so that
>   * actions, such as logging of syscall results, can be performed.  All errnos
> --
> 2.33.1
>

Re: [PATCH 12/22] bsd-user/freebsd/os-syscall.c: Add get_errno and host_to_target_errno

[Kyle Evans]

On Tue, Feb 1, 2022 at 5:15 AM Warner Losh  wrote:
>
> Add the helper functions get_errno and host_to_target_errno. get_errno
> returns either the system call results, or the -errno when system call
> indicates failure by returning -1. Host_to_target_errno returns errno
> (since on FreeBSD they are the same on all architectures) along with a
> comment about why it's the identity.
>
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/freebsd/os-syscall.c | 23 +++
>  bsd-user/qemu.h   |  3 ++-
>  2 files changed, 25 insertions(+), 1 deletion(-)
>

Reviewed-by: Kyle Evans 

> diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
> index 7e2bedb918d..2e84cf350b1 100644
> --- a/bsd-user/freebsd/os-syscall.c
> +++ b/bsd-user/freebsd/os-syscall.c
> @@ -44,6 +44,29 @@ void target_set_brk(abi_ulong new_brk)
>  {
>  }
>
> +/*
> + * errno conversion.
> + */
> +abi_long get_errno(abi_long ret)
> +{
> +
> +if (ret == -1) {
> +return -host_to_target_errno(errno);
> +} else {
> +return ret;
> +}
> +}
> +
> +int host_to_target_errno(int err)
> +{
> +/*
> + * All the BSDs have the property that the error numbers are uniform 
> across
> + * all architectures for a given BSD, though they may vary between 
> different
> + * BSDs.
> + */
> +return err;
> +}
> +
>  bool is_error(abi_long ret)
>  {
>
> diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
> index e5742bd6c03..56042ddbc5d 100644
> --- a/bsd-user/qemu.h
> +++ b/bsd-user/qemu.h
> @@ -246,9 +246,10 @@ extern unsigned long target_dflssiz;
>  extern unsigned long target_maxssiz;
>  extern unsigned long target_sgrowsiz;
>
> -/* syscall.c */
> +/* os-syscall.c */
>  abi_long get_errno(abi_long ret);
>  bool is_error(abi_long ret);
> +int host_to_target_errno(int err);
>
>  /* os-sys.c */
>  abi_long do_freebsd_sysarch(void *cpu_env, abi_long arg1, abi_long arg2);
> --
> 2.33.1
>

Re: [PATCH 18/22] bsd-user: Define target_arg64

[Kyle Evans]

On Tue, Feb 1, 2022 at 5:15 AM Warner Losh  wrote:
>
> target_arg64 is a generic way to extract 64-bits from a pair of
> arguments. On 32-bit platforms, it returns them joined together as
> appropriate. On 64-bit platforms, it returns the first arg because it's
> already 64-bits.
>
> Signed-off-by: Stacey Son 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/qemu.h | 13 +
>  1 file changed, 13 insertions(+)
>

Reviewed-by: Kyle Evans 

> diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
> index a9efa807b78..af272c2a802 100644
> --- a/bsd-user/qemu.h
> +++ b/bsd-user/qemu.h
> @@ -462,6 +462,19 @@ static inline void *lock_user_string(abi_ulong 
> guest_addr)
>  #define unlock_user_struct(host_ptr, guest_addr, copy)  \
>  unlock_user(host_ptr, guest_addr, (copy) ? sizeof(*host_ptr) : 0)
>
> +static inline uint64_t target_arg64(uint32_t word0, uint32_t word1)
> +{
> +#if TARGET_ABI_BITS == 32
> +#ifdef TARGET_WORDS_BIGENDIAN
> +return ((uint64_t)word0 << 32) | word1;
> +#else
> +return ((uint64_t)word1 << 32) | word0;
> +#endif
> +#else /* TARGET_ABI_BITS != 32 */
> +return word0;
> +#endif /* TARGET_ABI_BITS != 32 */
> +}
> +
>  #include 
>
>  #include "user/safe-syscall.h"
> --
> 2.33.1
>

Re: [PATCH 08/22] bsd-user: Remove bsd_type

[Kyle Evans]

On Tue, Feb 1, 2022 at 5:14 AM Warner Losh  wrote:
>
> Remove keeping track of which type of bsd we're running on. It's no
> longer referenced in the code. Building bsd-user on NetBSD or OpenBSD
> isn't possible, let alone running that code. Stop pretending that we can
> do the cross BSD thing since there's been a large divergence since 2000
> that makes this nearly impossible between FreeBSD and {Net,Open}BSD and
> at least quite difficult between NetBSD and OpenBSD.
>
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/main.c | 2 --
>  bsd-user/qemu.h | 7 ---
>  2 files changed, 9 deletions(-)
>

Reviewed-by: Kyle Evans 

> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index bddb830e99b..88d347d05eb 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -96,7 +96,6 @@ unsigned long reserved_va;
>
>  static const char *interp_prefix = CONFIG_QEMU_INTERP_PREFIX;
>  const char *qemu_uname_release;
> -enum BSDType bsd_type;
>  char qemu_proc_pathname[PATH_MAX];  /* full path to exeutable */
>
>  unsigned long target_maxtsiz = TARGET_MAXTSIZ;   /* max text size */
> @@ -284,7 +283,6 @@ int main(int argc, char **argv)
>  const char *gdbstub = NULL;
>  char **target_environ, **wrk;
>  envlist_t *envlist = NULL;
> -bsd_type = HOST_DEFAULT_BSD_TYPE;
>  char *argv0 = NULL;
>
>  adjust_ssize();
> diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
> index 02921ac8b3b..e5742bd6c03 100644
> --- a/bsd-user/qemu.h
> +++ b/bsd-user/qemu.h
> @@ -29,13 +29,6 @@
>
>  extern char **environ;
>
> -enum BSDType {
> -target_freebsd,
> -target_netbsd,
> -target_openbsd,
> -};
> -extern enum BSDType bsd_type;
> -
>  #include "exec/user/thunk.h"
>  #include "target_arch.h"
>  #include "syscall_defs.h"
> --
> 2.33.1
>

Re: [PATCH 11/22] bsd-user/sycall.c: Now obsolete, remove

[Kyle Evans]

On Tue, Feb 1, 2022 at 5:15 AM Warner Losh  wrote:
>
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/syscall.c | 516 -
>  1 file changed, 516 deletions(-)
>  delete mode 100644 bsd-user/syscall.c
>

Reviewed-by: Kyle Evans 

> diff --git a/bsd-user/syscall.c b/bsd-user/syscall.c
> deleted file mode 100644
> index d3322760f43..000
> --- a/bsd-user/syscall.c
> +++ /dev/null
> @@ -1,516 +0,0 @@
> -/*
> - *  BSD syscalls
> - *
> - *  Copyright (c) 2003 - 2008 Fabrice Bellard
> - *
> - *  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 "qemu/osdep.h"
> -#include "qemu/cutils.h"
> -#include "qemu/path.h"
> -#include 
> -#include 
> -#include 
> -#include 
> -
> -#include "qemu.h"
> -#include "qemu-common.h"
> -#include "user/syscall-trace.h"
> -
> -//#define DEBUG
> -
> -static abi_ulong target_brk;
> -static abi_ulong target_original_brk;
> -
> -abi_long get_errno(abi_long ret)
> -{
> -if (ret == -1) {
> -/* XXX need to translate host -> target errnos here */
> -return -(errno);
> -}
> -return ret;
> -}
> -
> -#define target_to_host_bitmask(x, tbl) (x)
> -
> -bool is_error(abi_long ret)
> -{
> -return (abi_ulong)ret >= (abi_ulong)(-4096);
> -}
> -
> -void target_set_brk(abi_ulong new_brk)
> -{
> -target_original_brk = target_brk = HOST_PAGE_ALIGN(new_brk);
> -}
> -
> -/* do_obreak() must return target errnos. */
> -static abi_long do_obreak(abi_ulong new_brk)
> -{
> -abi_ulong brk_page;
> -abi_long mapped_addr;
> -int new_alloc_size;
> -
> -if (!new_brk)
> -return 0;
> -if (new_brk < target_original_brk)
> -return -TARGET_EINVAL;
> -
> -brk_page = HOST_PAGE_ALIGN(target_brk);
> -
> -/* If the new brk is less than this, set it and we're done... */
> -if (new_brk < brk_page) {
> -target_brk = new_brk;
> -return 0;
> -}
> -
> -/* We need to allocate more memory after the brk... */
> -new_alloc_size = HOST_PAGE_ALIGN(new_brk - brk_page + 1);
> -mapped_addr = get_errno(target_mmap(brk_page, new_alloc_size,
> -PROT_READ|PROT_WRITE,
> -MAP_ANON|MAP_FIXED|MAP_PRIVATE, -1, 
> 0));
> -
> -if (!is_error(mapped_addr))
> -target_brk = new_brk;
> -else
> -return mapped_addr;
> -
> -return 0;
> -}
> -
> -#ifdef __FreeBSD__
> -/*
> - * XXX this uses the undocumented oidfmt interface to find the kind of
> - * a requested sysctl, see /sys/kern/kern_sysctl.c:sysctl_sysctl_oidfmt()
> - * (this is mostly copied from src/sbin/sysctl/sysctl.c)
> - */
> -static int
> -oidfmt(int *oid, int len, char *fmt, uint32_t *kind)
> -{
> -int qoid[CTL_MAXNAME+2];
> -uint8_t buf[BUFSIZ];
> -int i;
> -size_t j;
> -
> -qoid[0] = 0;
> -qoid[1] = 4;
> -memcpy(qoid + 2, oid, len * sizeof(int));
> -
> -j = sizeof(buf);
> -i = sysctl(qoid, len + 2, buf, , 0, 0);
> -if (i)
> -return i;
> -
> -if (kind)
> -*kind = *(uint32_t *)buf;
> -
> -if (fmt)
> -strcpy(fmt, (char *)(buf + sizeof(uint32_t)));
> -return (0);
> -}
> -
> -/*
> - * try and convert sysctl return data for the target.
> - * XXX doesn't handle CTLTYPE_OPAQUE and CTLTYPE_STRUCT.
> - */
> -static int sysctl_oldcvt(void *holdp, size_t holdlen, uint32_t kind)
> -{
> -switch (kind & CTLTYPE) {
> -case CTLTYPE_INT:
> -case CTLTYPE_UINT:
> -*(uint32_t *)holdp = tswap32(*(uint32_t *)holdp);
> -break;
> -#ifdef TARGET_ABI32
> -case CTLTYPE_LONG:
> -case CTLTYPE_ULONG:
> -*(uint32_t *)holdp = tswap32(*(long *)holdp);
> -break;
> -#else
> -case CTLTYPE_LONG:
> -*(uint64_t *)holdp = tswap64(*(long *)holdp

Re: [PATCH 05/22] bsd-user/arm/target_arch_cpu.h: Only support FreeBSD sys calls

[Kyle Evans]

gs[0];
> +arg1 = env->regs[2];
> +arg2 = env->regs[3];
> +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);
> +arg7 = 0;
> +arg8 = 0;
> +} else {
> +arg1 = env->regs[0];
> +arg2 = env->regs[1];
> +arg3 = env->regs[2];
> +arg4 = env->regs[3];
> +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);
> +}
> +ret = do_freebsd_syscall(env, syscall_nr, arg1, arg2, arg3,
> + arg4, arg5, arg6, arg7, arg8);
> +/*
> + * Compare to arm/arm/vm_machdep.c
> + * cpu_set_syscall_retval()
> + */
> +if (-TARGET_EJUSTRETURN == ret) {
>  /*
> - * Compare to arm/arm/vm_machdep.c
> - * cpu_set_syscall_retval()
> + * Returning from a successful sigreturn syscall.
> + * Avoid clobbering register state.
>   */
> -if (-TARGET_EJUSTRETURN == ret) {
> -/*
> - * Returning from a successful sigreturn syscall.
> - * Avoid clobbering register state.
> - */
> -break;
> -}
> -if (-TARGET_ERESTART == ret) {
> -env->regs[15] -= env->thumb ? 2 : 4;
> -break;
> -}
> -if ((unsigned int)ret >= (unsigned int)(-515)) {
> -ret = -ret;
> -cpsr_write(env, CPSR_C, CPSR_C, CPSRWriteByInstr);
> -env->regs[0] = ret;
> -} else {
> -cpsr_write(env, 0, CPSR_C, CPSRWriteByInstr);
> -env->regs[0] = ret; /* XXX need to handle lseek()? */
> -/* env->regs[1] = 0; */
> -}
> +break;
> +}
> +if (-TARGET_ERESTART == ret) {
> +env->regs[15] -= env->thumb ? 2 : 4;
> +break;
> +}
> +if ((unsigned int)ret >= (unsigned int)(-515)) {
> +ret = -ret;
> +cpsr_write(env, CPSR_C, CPSR_C, CPSRWriteByInstr);
> +env->regs[0] = ret;
>  } else {
> -fprintf(stderr, "qemu: bsd_type (= %d) syscall "
> -"not supported\n", bsd_type);
> +cpsr_write(env, 0, CPSR_C, CPSRWriteByInstr);
> +env->regs[0] = ret; /* XXX need to handle lseek()? */
> +/* env->regs[1] = 0; */
>  }
>  }
>  break;
>

We should probably fix the lseek() situation sooner rather than later, but:

Reviewed-by: Kyle Evans 

Re: [PATCH 03/22] bsd-user/x86_64/target_arch_cpu.h: Remove openbsd syscall

[Kyle Evans]

On Tue, Feb 1, 2022 at 5:14 AM Warner Losh  wrote:
>
> This doesn't build on openbsd at the moment, and this could
> should arguably be in bsd-user/*bsd/x86_64 somewhere. Until
> we refactor to support OpenBSD/NetBSD again, drop it here.
>
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/x86_64/target_arch_cpu.h | 27 ---
>  1 file changed, 8 insertions(+), 19 deletions(-)
>

Reviewed-by: Kyle Evans 

As a general comment, I'd like to reach out to the others at some
point and gauge interest/ability to participate, but I definitely
agree that it would be better to drop !FreeBSD for now to simplify
upcoming improvements to the common core. I'm not aware of any other
forks that have tried to maintain bsd-user on their platforms.

> diff --git a/bsd-user/x86_64/target_arch_cpu.h 
> b/bsd-user/x86_64/target_arch_cpu.h
> index 9dc52d5afc4..5be2f02416e 100644
> --- a/bsd-user/x86_64/target_arch_cpu.h
> +++ b/bsd-user/x86_64/target_arch_cpu.h
> @@ -126,25 +126,14 @@ static inline void target_cpu_loop(CPUX86State *env)
>  switch (trapnr) {
>  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->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);
>  env->eip = env->exception_next_eip;
>  if (((abi_ulong)env->regs[R_EAX]) >= (abi_ulong)(-515)) {
>  env->regs[R_EAX] = -env->regs[R_EAX];
> --
> 2.33.1
>

Re: [PATCH 06/22] bsd-user/arm/target_arch_thread.h: Assume a FreeBSD target

[Kyle Evans]

On Tue, Feb 1, 2022 at 5:14 AM Warner Losh  wrote:
>
> Since we can't run on anything else, assume for the moment that this is
> a FreeBSD target. In the future, we'll need to handle this properly
> via some include file in bsd-user/*bsd/arm/mumble.h. There's a number
> of other diffs that would be needed to make things work on OtherBSD,
> so it doesn't make sense to preseve this one detail today.
>
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/arm/target_arch_thread.h | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>

Reviewed-by: Kyle Evans 

> diff --git a/bsd-user/arm/target_arch_thread.h 
> b/bsd-user/arm/target_arch_thread.h
> index 11c7f765838..fcafca2408c 100644
> --- a/bsd-user/arm/target_arch_thread.h
> +++ b/bsd-user/arm/target_arch_thread.h
> @@ -62,9 +62,7 @@ static inline void target_thread_init(struct target_pt_regs 
> *regs,
>  }
>  regs->ARM_pc = infop->entry & 0xfffe;
>  regs->ARM_sp = stack;
> -if (bsd_type == target_freebsd) {
> -regs->ARM_lr = infop->entry & 0xfffe;
> -}
> +regs->ARM_lr = infop->entry & 0xfffe;
>  /*
>   * FreeBSD kernel passes the ps_strings pointer in r0. This is used by 
> some
>   * programs to set status messages that we see in ps. bsd-user doesn't
> --
> 2.33.1
>

Re: [PATCH 04/22] bsd-user/i386/target_arch_cpu.h: Remove openbsd syscall

[Kyle Evans]

On Tue, Feb 1, 2022 at 5:14 AM Warner Losh  wrote:
>
> This doesn't build on openbsd at the moment, and this could
> should arguably be in bsd-user/*bsd/i386 somewhere. Until

could or should, let's pick one and drop the other. :-)

> we refactor to support OpenBSD/NetBSD again, drop it here.
>
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/i386/target_arch_cpu.h | 84 +++--
>  1 file changed, 37 insertions(+), 47 deletions(-)
>

Reviewed-by: Kyle Evans 

> diff --git a/bsd-user/i386/target_arch_cpu.h b/bsd-user/i386/target_arch_cpu.h
> index 3cbf69d8af2..9da22202d48 100644
> --- a/bsd-user/i386/target_arch_cpu.h
> +++ b/bsd-user/i386/target_arch_cpu.h
> @@ -116,55 +116,45 @@ static inline void target_cpu_loop(CPUX86State *env)
>  process_queued_cpu_work(cs);
>
>  switch (trapnr) {
> -case 0x80:
> +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);
> +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);
> -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]);
> +} 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);
> +   

Re: [PATCH 17/22] bsd-user: introduce target.h

[Kyle Evans]

On Tue, Feb 1, 2022 at 5:15 AM Warner Losh  wrote:
>
> Create target.h. This file is intended to be simple and describe basic
> things about the architecture. If something is a basic feature of the
> architecture, it belongs here. Should we need something that's per-BSD
> there will be a target-os.h that will live in the per-bsd directories.
>
> Define regpairs_aligned to reflect whether or not registers are 'paired'
> for 64-bit arguments or not. This will be false for all 64-bit targets,
> and will be true on those architectures that pair (currently just armv7
> and powerpc on FreeBSD 14.x).
>
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/arm/target.h| 21 +
>  bsd-user/i386/target.h   | 21 +
>  bsd-user/qemu.h  |  1 +
>  bsd-user/x86_64/target.h | 21 +
>  4 files changed, 64 insertions(+)
>  create mode 100644 bsd-user/arm/target.h
>  create mode 100644 bsd-user/i386/target.h
>  create mode 100644 bsd-user/x86_64/target.h
>

Reviewed-by: Kyle Evans 

> diff --git a/bsd-user/arm/target.h b/bsd-user/arm/target.h
> new file mode 100644
> index 000..1f7ee49bfb4
> --- /dev/null
> +++ b/bsd-user/arm/target.h
> @@ -0,0 +1,21 @@
> +/*
> + * Intel general target stuff that's common to all i386 details
> + *
> + * Copyright (c) 2022 M. Warner Losh 
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef TARGET_H
> +#define TARGET_H
> +
> +/*
> + * arm EABI 'lumps' the registers for 64-bit args.
> + */
> +static inline int regpairs_aligned(void *cpu_env)
> +{
> +return 1;
> +}
> +
> +#endif /* ! TARGET_H */
> +
> diff --git a/bsd-user/i386/target.h b/bsd-user/i386/target.h
> new file mode 100644
> index 000..b0ab477d683
> --- /dev/null
> +++ b/bsd-user/i386/target.h
> @@ -0,0 +1,21 @@
> +/*
> + * Intel general target stuff that's common to all i386 details
> + *
> + * Copyright (c) 2022 M. Warner Losh 
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef TARGET_ARCH_H
> +#define TARGET_ARCH_H
> +
> +/*
> + * i386 doesn't 'lump' the registers for 64-bit args.
> + */
> +static inline int regpairs_aligned(void *cpu_env)
> +{
> +return 0;
> +}
> +
> +#endif /* ! TARGET_ARCH_H */
> +
> diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
> index 56042ddbc5d..a9efa807b78 100644
> --- a/bsd-user/qemu.h
> +++ b/bsd-user/qemu.h
> @@ -35,6 +35,7 @@ extern char **environ;
>  #include "target_syscall.h"
>  #include "target_os_vmparam.h"
>  #include "target_os_signal.h"
> +#include "target.h"
>  #include "exec/gdbstub.h"
>
>  /*
> diff --git a/bsd-user/x86_64/target.h b/bsd-user/x86_64/target.h
> new file mode 100644
> index 000..6d3aef8fc49
> --- /dev/null
> +++ b/bsd-user/x86_64/target.h
> @@ -0,0 +1,21 @@
> +/*
> + * Intel general target stuff that's common to all x86_64 details
> + *
> + * Copyright (c) 2022 M. Warner Losh 
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef TARGET_H
> +#define TARGET_H
> +
> +/*
> + * x86 doesn't 'lump' the registers for 64-bit args, all args are 64 bits.
> + */
> +static inline int regpairs_aligned(void *cpu_env)
> +{
> +return 0;
> +}
> +
> +#endif /* ! TARGET_H */
> +
> --
> 2.33.1
>

Re: [PATCH 02/22] bsd-user/x86_64/target_arch_cpu.h: int $80 never was a BSD system call on amd64

[Kyle Evans]

On Tue, Feb 1, 2022 at 5:14 AM Warner Losh  wrote:
>
> Although initial versions of NetBSD did use int $80, it was replaced by
> syscall before any releases. OpenBSD and FreeBSD always did syscall.
>
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/x86_64/target_arch_cpu.h | 58 ---
>  1 file changed, 58 deletions(-)
>

Reviewed-by: Kyle Evans 

> diff --git a/bsd-user/x86_64/target_arch_cpu.h 
> b/bsd-user/x86_64/target_arch_cpu.h
> index 0a9c0f08946..9dc52d5afc4 100644
> --- a/bsd-user/x86_64/target_arch_cpu.h
> +++ b/bsd-user/x86_64/target_arch_cpu.h
> @@ -124,64 +124,6 @@ static inline void target_cpu_loop(CPUX86State *env)
>  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;
> -
>  case EXCP_SYSCALL:
>  /* syscall from syscall instruction */
>  if (bsd_type == target_freebsd) {
> --
> 2.33.1
>

Re: [PATCH 09/22] bsd-user/freebsd/os-syscall.c: Move syscall processing here

[Kyle Evans]

On Tue, Feb 1, 2022 at 5:15 AM Warner Losh  wrote:
>
> While there is some commonality between *BSD syscall processing, there's
> a number of differences and the system call numbers and ABIs have been
> independent since the late 90s. Move FreeBSD's proessing here and delete
> it.
>

"processing"

> The upstream implementation is somewhat different than the current
> implementation. It will be much easier to upstream these from scratch,
> justifying the final result, rather than working out the diffs and
> justifying the changes. Also tweak a comment to qemu standard form.
>
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/freebsd/os-syscall.c | 68 +++
>  1 file changed, 68 insertions(+)
>  create mode 100644 bsd-user/freebsd/os-syscall.c
>
> diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
> new file mode 100644
> index 000..7e2bedb918d
> --- /dev/null
> +++ b/bsd-user/freebsd/os-syscall.c
> @@ -0,0 +1,68 @@
> +/*
> + *  BSD syscalls
> + *
> + *  Copyright (c) 2003-2008 Fabrice Bellard
> + *  Copyright (c) 2013-2014 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/>.
> + */
> +
> +/*
> + * We need the FreeBSD "legacy" definitions. Rust needs the FreeBSD 11 system
> + * calls, so we have to emulate that despite FreeBSD being EOL'd.
> + */

... FreeBSD 11 being EOL'd.

> +#define _WANT_FREEBSD11_STAT
> +#define _WANT_FREEBSD11_STATFS
> +#define _WANT_FREEBSD11_DIRENT
> +#define _WANT_KERNEL_ERRNO
> +#define _WANT_SEMUN
> +#include "qemu/osdep.h"
> +#include "qemu/cutils.h"
> +#include "qemu/path.h"
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "qemu.h"
> +#include "qemu-common.h"
> +#include "signal-common.h"
> +#include "user/syscall-trace.h"
> +
> +void target_set_brk(abi_ulong new_brk)
> +{
> +}
> +
> +bool is_error(abi_long ret)
> +{
> +
> +return (abi_ulong)ret >= (abi_ulong)(-4096);
> +}
> +
> +/*
> + * do_syscall() should always have a single exit point at the end so that
> + * actions, such as logging of syscall results, can be performed.  All errnos
> + * that do_syscall() returns must be -TARGET_.
> + */
> +abi_long do_freebsd_syscall(void *cpu_env, int num, abi_long arg1,
> +abi_long arg2, abi_long arg3, abi_long arg4,
> +abi_long arg5, abi_long arg6, abi_long arg7,
> +abi_long arg8)
> +{
> +return 0;
> +}
> +
> +void syscall_init(void)
> +{
> +}
> --
> 2.33.1
>

Small typo + omission, otherwise:

Reviewed-by: Kyle Evans 

Re: [PATCH 23/30] bsd-user/signal.c: sigset manipulation routines.

[Kyle Evans]

On Sat, Jan 22, 2022 at 10:44 AM Warner Losh  wrote:
>
>
>
> On Fri, Jan 14, 2022 at 4:14 AM Peter Maydell  
> wrote:
>>
>> On Sun, 9 Jan 2022 at 16:53, Warner Losh  wrote:
>> >
>> > target_sigemptyset: resets a set to having no bits set
>> > qemu_sigorset:  computes the or of two sets
>> > target_sigaddset:   adds a signal to a set
>> > target_sigismember: returns true when signal is a member
>> > host_to_target_sigset_internal: convert host sigset to target
>> > host_to_target_sigset: convert host sigset to target
>> > target_to_host_sigset_internal: convert target sigset to host
>> > target_to_host_sigset: convert target sigset to host
>> >
>> > Signed-off-by: Stacey Son 
>> > Signed-off-by: Kyle Evans 
>> > Signed-off-by: Warner Losh 
>> > ---
>> >  bsd-user/qemu.h   |  3 ++
>> >  bsd-user/signal.c | 89 +++
>> >  2 files changed, 92 insertions(+)
>> >
>> > diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
>> > index e12617f5d69..e8c417c7c33 100644
>> > --- a/bsd-user/qemu.h
>> > +++ b/bsd-user/qemu.h
>> > @@ -223,7 +223,10 @@ void queue_signal(CPUArchState *env, int sig, 
>> > target_siginfo_t *info);
>> >  abi_long do_sigaltstack(abi_ulong uss_addr, abi_ulong uoss_addr, 
>> > abi_ulong sp);
>> >  int target_to_host_signal(int sig);
>> >  int host_to_target_signal(int sig);
>> > +void host_to_target_sigset(target_sigset_t *d, const sigset_t *s);
>> > +void target_to_host_sigset(sigset_t *d, const target_sigset_t *s);
>> >  void QEMU_NORETURN force_sig(int target_sig);
>> > +int qemu_sigorset(sigset_t *dest, const sigset_t *left, const sigset_t 
>> > *right);
>> >
>> >  /* mmap.c */
>> >  int target_mprotect(abi_ulong start, abi_ulong len, int prot);
>> > diff --git a/bsd-user/signal.c b/bsd-user/signal.c
>> > index 93c3b3c5033..8dadc9a39a7 100644
>> > --- a/bsd-user/signal.c
>> > +++ b/bsd-user/signal.c
>> > @@ -32,6 +32,9 @@
>> >
>> >  static struct target_sigaction sigact_table[TARGET_NSIG];
>> >  static void host_signal_handler(int host_sig, siginfo_t *info, void *puc);
>> > +static void target_to_host_sigset_internal(sigset_t *d,
>> > +const target_sigset_t *s);
>> > +
>> >
>> >  int host_to_target_signal(int sig)
>> >  {
>> > @@ -43,6 +46,44 @@ int target_to_host_signal(int sig)
>> >  return sig;
>> >  }
>> >
>> > +static inline void target_sigemptyset(target_sigset_t *set)
>> > +{
>> > +memset(set, 0, sizeof(*set));
>> > +}
>> > +
>> > +#include 
>>
>> Don't include system headers halfway through the file like this,
>> please : put the #include at the top of the file with the others.
>
>
> Yea, this isn't even needed, so I just removed it.
>
>>
>> > +
>> > +int
>> > +qemu_sigorset(sigset_t *dest, const sigset_t *left, const sigset_t *right)
>> > +{
>> > +sigset_t work;
>> > +int i;
>> > +
>> > +sigemptyset();
>> > +for (i = 1; i < NSIG; ++i) {
>> > +if (sigismember(left, i) || sigismember(right, i)) {
>> > +sigaddset(, i);
>> > +}
>> > +}
>> > +
>> > +*dest = work;
>> > +return 0;
>> > +}
>>
>> FreeBSD's manpage says it has a native sigorset() --
>> https://www.freebsd.org/cgi/man.cgi?query=sigemptyset=3=0=freebsd
>> can you just use that ?
>
>
> Yes.
>

For some added context, I added sigorset() to libc after 11.3/12.1 in
response to bsd-user using it, then forgot to remove the transition
aide after they went EoL.

Thanks,

Kyle Evans

Re: [PATCH 21/30] bsd-user/signal.c: force_sig

[Kyle Evans]

On Thu, Jan 13, 2022 at 2:53 PM Peter Maydell  wrote:
>
> On Thu, 13 Jan 2022 at 20:29, Peter Maydell  wrote:
> >
> > On Sun, 9 Jan 2022 at 16:44, Warner Losh  wrote:
> > >
> > > Force delivering a signal and generating a core file.
>
> > > +/* Abort execution with signal. */
> > > +void QEMU_NORETURN force_sig(int target_sig)
> >
> > In linux-user we call this dump_core_and_abort(), which is
> > a name that better describes what it's actually doing.
> >
> > (Today's linux-user's force_sig() does what the Linux kernel's
> > function of that name does -- it's a wrapper around
> > queue_signal() which delivers a signal to the guest with
> > .si_code = SI_KERNEL , si_pid = si_uid = 0.
> > Whether you want one of those or not depends on what BSD
> > kernels do in that kind of "we have to kill this process"
> > situation.)
>
> It looks like the FreeBSD kernel uses sigexit() as its equivalent
> function to Linux's force_sig(), incidentally. Not sure if
> you/we would prefer the bsd-user code to follow the naming that
> FreeBSD's kernel uses or the naming linux-user takes from
> the Linux kernel.
>

My $.02: let's go with linux-inherited linux-user names and drop in a
comment with the FreeBSD name, if they're functionally similar enough
(in general, not just for this specific case). My gut feeling is that
it'll be more useful in the long run if we can more quickly identify
parallels between the two, so changes affecting linux-user that may
benefit bsd-user are more easily identified and exchanged (and
vice-versa).

Thanks,

Kyle Evans

Re: [PATCH v4 01/36] bsd-user/mips*: Remove

[Kyle Evans]

On Fri, Nov 5, 2021 at 10:52 AM Richard Henderson
 wrote:
>
> On 11/4/21 11:18 PM, Warner Losh wrote:
> > FreeBSD has dropped support for mips starting with FreeBSD 14. mips
> > support has been removed from the bsd-user fork because updating it for
> > new signal requirements. Remove it here since it is a distraction.
> >
> > Signed-off-by: Warner Losh
> > ---
> >   bsd-user/mips/target_arch_sysarch.h   | 69 ---
> >   bsd-user/mips/target_syscall.h| 52 
> >   bsd-user/mips64/target_arch_sysarch.h | 69 ---
> >   bsd-user/mips64/target_syscall.h  | 53 
> >   4 files changed, 243 deletions(-)
> >   delete mode 100644 bsd-user/mips/target_arch_sysarch.h
> >   delete mode 100644 bsd-user/mips/target_syscall.h
> >   delete mode 100644 bsd-user/mips64/target_arch_sysarch.h
> >   delete mode 100644 bsd-user/mips64/target_syscall.h
> I'm somewhat surprised that sys/mips/mips still exists on the main branch?  
> But anyway,
>
> Acked-by: Richard Henderson 

It's slated for removal here within the coming month or two-

Re: [PATCH 20/24] bsd-user/arm/target_arch_signal.h: arm set_sigtramp_args

[Kyle Evans]

On Thu, Oct 28, 2021 at 12:25 PM Richard Henderson
 wrote:
>
> On 10/19/21 9:44 AM, Warner Losh wrote:
> > +regs->regs[TARGET_REG_PC] = ka->_sa_handler;
>
> Surely there should be some handling of thumb addresses here.
>

Honestly, this wouldn't surprise me- we're kind of a thumb landmine.
The other thumb-ish support is relatively recent as we hit something
that had an entry point on a thumb instruction; but we don't typically
build/run binaries including thumb instructions.

Re: [PATCH 13/24] bsd-user/arm/target_arch_thread.h: Routines to create and switch to a thread

[Kyle Evans]

On Wed, Oct 27, 2021 at 10:35 AM Warner Losh  wrote:
>
>
>
> On Tue, Oct 26, 2021 at 12:11 AM Kyle Evans  wrote:
>>
>> On Tue, Oct 26, 2021 at 1:01 AM Kyle Evans  wrote:
>> >
>> > On Tue, Oct 19, 2021 at 11:45 AM Warner Losh  wrote:
>> > >
>> > > Implement target_thread_init (to create a thread) and target_set_upcall
>> > > (to switch to a thread) for arm.
>> > >
>> > > Signed-off-by: Stacey Son 
>> > > Signed-off-by: Klye Evans 
>> > > Signed-off-by: Warner Losh 
>> > > ---
>> > >  bsd-user/arm/target_arch_thread.h | 71 +++
>> > >  1 file changed, 71 insertions(+)
>> > >  create mode 100644 bsd-user/arm/target_arch_thread.h
>> > >
>> > > diff --git a/bsd-user/arm/target_arch_thread.h 
>> > > b/bsd-user/arm/target_arch_thread.h
>> > > new file mode 100644
>> > > index 00..317364bb84
>> > > --- /dev/null
>> > > +++ b/bsd-user/arm/target_arch_thread.h
>> > > @@ -0,0 +1,71 @@
>> > > +/*
>> > > + *  arm thread support
>> > > + *
>> > > + *  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_THREAD_H_
>> > > +#define _TARGET_ARCH_THREAD_H_
>> > > +
>> > > +/* Compare to arm/arm/vm_machdep.c cpu_set_upcall_kse() */
>> > > +static inline void target_thread_set_upcall(CPUARMState *regs, 
>> > > abi_ulong entry,
>> > > +abi_ulong arg, abi_ulong stack_base, abi_ulong stack_size)
>> > > +{
>> > > +abi_ulong sp;
>> > > +
>> > > +/*
>> > > + * Make sure the stack is properly aligned.
>> > > + * arm/include/param.h (STACKLIGN() macro)
>> > > + */
>> > > +sp = (u_int)((stack_base + stack_size) -
>> > > +sizeof(struct target_trapframe)) & ~0x7;
>> > > +
>> > > +/* sp = stack base */
>> > > +regs->regs[13] = sp;
>> > > +/* pc = start function entry */
>> > > +regs->regs[15] = entry & 0xfffe;
>> > > +/* r0 = arg */
>> > > +regs->regs[0] = arg;
>> > > +regs->spsr = ARM_CPU_MODE_USR;
>> > > +if (entry & 0x1) {
>> > > +regs->spsr |= CPSR_T;
>> > > +}
>> > > +}
>> > > +
>> > > +static inline void target_thread_init(struct target_pt_regs *regs,
>> > > +struct image_info *infop)
>> > > +{
>> > > +abi_long stack = infop->start_stack;
>> > > +memset(regs, 0, sizeof(*regs));
>> > > +regs->ARM_cpsr = 0x10;
>> > > +if (infop->entry & 1) {
>> > > +regs->ARM_cpsr |= CPSR_T;
>> > > +}
>> > > +regs->ARM_pc = infop->entry & 0xfffe;
>> > > +regs->ARM_sp = infop->start_stack;
>> > > +if (bsd_type == target_freebsd) {
>> > > +regs->ARM_lr = infop->entry & 0xfffe;
>> > > +}
>> > > +/* FIXME - what to for failure of get_user()? */
>> > > +get_user_ual(regs->ARM_r2, stack + 8); /* envp */
>> > > +get_user_ual(regs->ARM_r1, stack + 4); /* envp */
>> > > +    /* XXX: it seems that r0 is zeroed after ! */
>> > > +regs->ARM_r0 = 0;
>> > > +/* For uClinux PIC binaries.  */
>> > > +/* XXX: Linux does this only on ARM with no MMU (do we care ?) */
>> > > +regs->ARM_r10 = infop->start_data;
>> > > +}
>> > > +
>> > > +#endif /* !_TARGET_ARCH_THREAD_H_ */
>> > > --
>> > > 2.32.0
>> > >
>> >
>> > I think it's obvious enough to folks already familiar with ARM, but I
>> > wonder if we shouldn't add in some basic commentary about the thumb
>> > bits above. Something like:
>> >
>> > /*
>> >  * The low bit in an entry point indicates a thumb instruction; the entry 
>> > point
>> >  * can't actually exist at this address because it must be 16- or 32-
>> > bit aligned.
>> >  * The low bit gets masked off and the T bit in CSPR is twiddled to
>> > indicate thumb.
>> >  */
>>
>> s/CSPR/CPSR/
>
>
> Does
>
> /*
>  * Thumb mode is encoded by the low bit in the entry point (since ARM 
> can't
>  * execute at odd addresses). When it's set, set the Thumb bit (T) in the
>  * CPSR.
>  */
>
>  Look good to you?
>

Yeah, that works for me!

Thanks,

Kyle Evans

Re: [PATCH 24/24] bsd-user: add arm target build

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh  wrote:
>
> Signed-off-by: Warner Losh 
> ---
>  configs/targets/arm-bsd-user.mak | 2 ++
>  1 file changed, 2 insertions(+)
>  create mode 100644 configs/targets/arm-bsd-user.mak
>
> diff --git a/configs/targets/arm-bsd-user.mak 
> b/configs/targets/arm-bsd-user.mak
> new file mode 100644
> index 00..deea21aaf5
> --- /dev/null
> +++ b/configs/targets/arm-bsd-user.mak
> @@ -0,0 +1,2 @@
> +TARGET_ARCH=arm
> +TARGET_XML_FILES= gdb-xml/arm-core.xml gdb-xml/arm-vfp.xml 
> gdb-xml/arm-vfp3.xml gdb-xml/arm-neon.xml gdb-xml/arm-m-profile.xml
> --
> 2.32.0
>

I'm not really qualified to review this one, but it looks basically
sane. I note that there's a gdb-xml/arm-vfp-sysregs.xml in the current
master that should probably be added to TARGET_XML_FILES.
Cross-referencing arm-linux-user and i386-bsd-user, this seems sane
and correct and I'm not aware of any other options that we would need
to consider setting, so let's call it:

Acked-by: Kyle Evans 

Re: [PATCH 20/24] bsd-user/arm/target_arch_signal.h: arm set_sigtramp_args

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh  wrote:
>
> Implement set_sigtramp_args to setup the arguments to the sigtramp
> calls.
>
> Signed-off-by: Stacey Son 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/arm/target_arch_signal.h | 35 +++
>  1 file changed, 35 insertions(+)
>
> diff --git a/bsd-user/arm/target_arch_signal.h 
> b/bsd-user/arm/target_arch_signal.h
> index 67355ff28f..b421c2522c 100644
> --- a/bsd-user/arm/target_arch_signal.h
> +++ b/bsd-user/arm/target_arch_signal.h
> @@ -128,4 +128,39 @@ struct target_trapframe {
>  abi_ulong tf_pc;
>  };
>
> +/*
> + * Compare to arm/arm/machdep.c sendsig()
> + * Assumes that target stack frame memory is locked.
> + */
> +static inline abi_long
> +set_sigtramp_args(CPUARMState *regs, int sig, struct target_sigframe *frame,
> +abi_ulong frame_addr, struct target_sigaction *ka)
> +{
> +/*
> + * Arguments to signal handler:
> + *  r0 = signal number
> + *  r1 = siginfo pointer
> + *  r2 = ucontext pointer
> + *  r5 = ucontext pointer
> + *  pc = signal handler pointer
> + *  sp = sigframe struct pointer
> + *  lr = sigtramp at base of user stack
> + */
> +
> +regs->regs[0] = sig;
> +regs->regs[1] = frame_addr +
> +offsetof(struct target_sigframe, sf_si);
> +regs->regs[2] = frame_addr +
> +offsetof(struct target_sigframe, sf_uc);
> +
> +/* the trampoline uses r5 as the uc address */
> +regs->regs[5] = frame_addr +
> +offsetof(struct target_sigframe, sf_uc);
> +regs->regs[TARGET_REG_PC] = ka->_sa_handler;
> +regs->regs[TARGET_REG_SP] = frame_addr;
> +    regs->regs[TARGET_REG_LR] = TARGET_PS_STRINGS - TARGET_SZSIGCODE;
> +
> +return 0;
> +}
> +
>  #endif /* !_TARGET_ARCH_SIGNAL_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH 18/24] bsd-user/arm/target_arch_signal.h: arm machine context for signals

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh  wrote:
>
> Signed-off-by: Stacey Son 
> Signed-off-by: Klye Evans 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/arm/target_arch_signal.h | 36 +++
>  1 file changed, 36 insertions(+)
>
> diff --git a/bsd-user/arm/target_arch_signal.h 
> b/bsd-user/arm/target_arch_signal.h
> index 973183d99c..9fee58ca9c 100644
> --- a/bsd-user/arm/target_arch_signal.h
> +++ b/bsd-user/arm/target_arch_signal.h
> @@ -54,4 +54,40 @@
>  #define TARGET_MINSIGSTKSZ  (1024 * 4)  /* min sig stack 
> size */
>  #define TARGET_SIGSTKSZ (TARGET_MINSIGSTKSZ + 32768)  /* recommended 
> size */
>
> +/* arm/arm/machdep.c */
> +struct target_sigcontext {
> +target_sigset_t sc_mask;/* signal mask to retstore */
> +int32_t sc_onstack; /* sigstack state to restore */
> +abi_longsc_pc;  /* pc at time of signal */
> +abi_longsc_reg[32]; /* processor regs 0 to 31 */
> +abi_longmullo, mulhi;   /* mullo and mulhi registers */
> +int32_t sc_fpused;  /* fp has been used */
> +abi_longsc_fpregs[33];  /* fp regs 0 to 31 & csr */
> +abi_longsc_fpc_eir; /* fp exception instr reg */
> +/* int32_t reserved[8]; */
> +};
> +
> +typedef struct {
> +uint32_t__fp_fpsr;
> +struct {
> +uint32_t__fp_exponent;
> +uint32_t__fp_mantissa_hi;
> +uint32_t__fp_mantissa_lo;
> +}   __fp_fr[8];
> +} target__fpregset_t;
> +
> +typedef struct {
> +uint32_t__vfp_fpscr;
> +uint32_t__vfp_fstmx[33];
> +uint32_t__vfp_fpsid;
> +} target__vfpregset_t;
> +
> +typedef struct target_mcontext {
> +uint32_t__gregs[TARGET__NGREG];
> +union {
> +target__fpregset_t  __fpregs;
> +    target__vfpregset_t __vfpregs;
> +} __fpu;
> +} target_mcontext_t;
> +
>  #endif /* !_TARGET_ARCH_SIGNAL_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH 21/24] bsd-user/arm/target_arch_signal.h: arm get_mcontext

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh  wrote:
>
> Get the machine context from the CPU state.
>
> Signed-off-by: Stacey Son 
> Signed-off-by: Klye Evans 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/arm/target_arch_signal.h | 38 +++
>  1 file changed, 38 insertions(+)
>
> diff --git a/bsd-user/arm/target_arch_signal.h 
> b/bsd-user/arm/target_arch_signal.h
> index b421c2522c..302fdc2846 100644
> --- a/bsd-user/arm/target_arch_signal.h
> +++ b/bsd-user/arm/target_arch_signal.h
> @@ -163,4 +163,42 @@ set_sigtramp_args(CPUARMState *regs, int sig, struct 
> target_sigframe *frame,
>  return 0;
>  }
>
> +/*
> + * Compare to arm/arm/machdep.c get_mcontext()
> + * Assumes that the memory is locked if mcp points to user memory.
> + */
> +static inline abi_long get_mcontext(CPUARMState *regs, target_mcontext_t 
> *mcp,
> +int flags)
> +{
> +int err = 0;
> +uint32_t *gr = mcp->__gregs;
> +
> +gr[TARGET_REG_CPSR] = tswap32(cpsr_read(regs));
> +if (flags & TARGET_MC_GET_CLEAR_RET) {
> +gr[TARGET_REG_R0] = 0;
> +gr[TARGET_REG_CPSR] &= ~CPSR_C;
> +} else {
> +gr[TARGET_REG_R0] = tswap32(regs->regs[0]);
> +}
> +
> +gr[TARGET_REG_R1] = tswap32(regs->regs[1]);
> +gr[TARGET_REG_R2] = tswap32(regs->regs[2]);
> +gr[TARGET_REG_R3] = tswap32(regs->regs[3]);
> +gr[TARGET_REG_R4] = tswap32(regs->regs[4]);
> +gr[TARGET_REG_R5] = tswap32(regs->regs[5]);
> +gr[TARGET_REG_R6] = tswap32(regs->regs[6]);
> +gr[TARGET_REG_R7] = tswap32(regs->regs[7]);
> +gr[TARGET_REG_R8] = tswap32(regs->regs[8]);
> +gr[TARGET_REG_R9] = tswap32(regs->regs[9]);
> +gr[TARGET_REG_R10] = tswap32(regs->regs[10]);
> +gr[TARGET_REG_R11] = tswap32(regs->regs[11]);
> +gr[TARGET_REG_R12] = tswap32(regs->regs[12]);
> +
> +gr[TARGET_REG_SP] = tswap32(regs->regs[13]);
> +gr[TARGET_REG_LR] = tswap32(regs->regs[14]);
> +gr[TARGET_REG_PC] = tswap32(regs->regs[15]);
> +
> +return err;
> +}
> +
>  #endif /* !_TARGET_ARCH_SIGNAL_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH 17/24] bsd-user/arm/target_arch_signal.h: arm specific signal registers and stack

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh  wrote:
>
> Defines for registers and stack layout related to signals.
>
> Signed-off-by: Stacey Son 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/arm/target_arch_signal.h | 57 +++
>  1 file changed, 57 insertions(+)
>  create mode 100644 bsd-user/arm/target_arch_signal.h
>
> diff --git a/bsd-user/arm/target_arch_signal.h 
> b/bsd-user/arm/target_arch_signal.h
> new file mode 100644
> index 00..973183d99c
> --- /dev/null
> +++ b/bsd-user/arm/target_arch_signal.h
> @@ -0,0 +1,57 @@
> +/*
> + *  arm signal 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 _TARGET_ARCH_SIGNAL_H_
> +#define _TARGET_ARCH_SIGNAL_H_
> +
> +#include "cpu.h"
> +
> +#define TARGET_REG_R0   0
> +#define TARGET_REG_R1   1
> +#define TARGET_REG_R2   2
> +#define TARGET_REG_R3   3
> +#define TARGET_REG_R4   4
> +#define TARGET_REG_R5   5
> +#define TARGET_REG_R6   6
> +#define TARGET_REG_R7   7
> +#define TARGET_REG_R8   8
> +#define TARGET_REG_R9   9
> +#define TARGET_REG_R10  10
> +#define TARGET_REG_R11  11
> +#define TARGET_REG_R12  12
> +#define TARGET_REG_R13  13
> +#define TARGET_REG_R14  14
> +#define TARGET_REG_R15  15
> +#define TARGET_REG_CPSR 16
> +#define TARGET__NGREG   17
> +/* Convenience synonyms */
> +#define TARGET_REG_FP   TARGET_REG_R11
> +#define TARGET_REG_SP   TARGET_REG_R13
> +#define TARGET_REG_LR   TARGET_REG_R14
> +#define TARGET_REG_PC   TARGET_REG_R15
> +
> +#define TARGET_INSN_SIZE4   /* arm instruction size */
> +
> +/* Size of the signal trampolin code. See _sigtramp(). */
> +#define TARGET_SZSIGCODE((abi_ulong)(9 * TARGET_INSN_SIZE))
> +
> +/* compare to arm/include/_limits.h */
> +#define TARGET_MINSIGSTKSZ  (1024 * 4)  /* min sig stack 
> size */
> +#define TARGET_SIGSTKSZ (TARGET_MINSIGSTKSZ + 32768)  /* recommended 
> size */
> +
> +#endif /* !_TARGET_ARCH_SIGNAL_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH 23/24] bsd-user/arm/target_arch_signal.h: arm get_ucontext_sigreturn

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh  wrote:
>
> Update ucontext to implement sigreturn.
>
> Signed-off-by: Stacey Son 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/arm/target_arch_signal.h | 18 ++
>  1 file changed, 18 insertions(+)
>
> diff --git a/bsd-user/arm/target_arch_signal.h 
> b/bsd-user/arm/target_arch_signal.h
> index 1d051af9ae..7da68c727c 100644
> --- a/bsd-user/arm/target_arch_signal.h
> +++ b/bsd-user/arm/target_arch_signal.h
> @@ -232,4 +232,22 @@ static inline abi_long set_mcontext(CPUARMState *regs, 
> target_mcontext_t *mcp,
>  return err;
>  }
>
> +/* Compare to arm/arm/machdep.c sys_sigreturn() */
> +static inline abi_long get_ucontext_sigreturn(CPUARMState *regs,
> +abi_ulong target_sf, abi_ulong *target_uc)
> +{
> +uint32_t cpsr = cpsr_read(regs);
> +
> +*target_uc = 0;
> +
> +if ((cpsr & CPSR_M) != ARM_CPU_MODE_USR ||
> +(cpsr & (CPSR_I | CPSR_F)) != 0) {
> +return -TARGET_EINVAL;
> +}
> +
> +*target_uc = target_sf;
> +
> +return 0;
> +}
> +
>  #endif /* !_TARGET_ARCH_SIGNAL_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH 15/24] bsd-user/arm/target_arch_elf.h: arm get hwcap

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh  wrote:
>
> Implement get_elf_hwcap to get the first word of hardware capabilities.
>
> Signed-off-by: Klye Evans 
> Signed-off-by: Stacey Son 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/arm/target_arch_elf.h | 72 +-
>  1 file changed, 71 insertions(+), 1 deletion(-)
>
> diff --git a/bsd-user/arm/target_arch_elf.h b/bsd-user/arm/target_arch_elf.h
> index 15b5c66511..02d25b8926 100644
> --- a/bsd-user/arm/target_arch_elf.h
> +++ b/bsd-user/arm/target_arch_elf.h
> @@ -31,6 +31,76 @@
>  #define USE_ELF_CORE_DUMP
>  #define ELF_EXEC_PAGESIZE   4096
>
> -#define ELF_HWCAP 0
> +#define ELF_HWCAP get_elf_hwcap()
> +
> +#define GET_FEATURE(feat, hwcap) \
> +do { if (arm_feature(>env, feat)) { hwcaps |= hwcap; } } while (0)
> +
> +#define GET_FEATURE_ID(feat, hwcap) \
> +do { if (cpu_isar_feature(feat, cpu)) { hwcaps |= hwcap; } } while (0)
> +
> +enum {
> +ARM_HWCAP_ARM_SWP   = 1 << 0,
> +ARM_HWCAP_ARM_HALF  = 1 << 1,
> +ARM_HWCAP_ARM_THUMB = 1 << 2,
> +ARM_HWCAP_ARM_26BIT = 1 << 3,
> +ARM_HWCAP_ARM_FAST_MULT = 1 << 4,
> +ARM_HWCAP_ARM_FPA   = 1 << 5,
> +ARM_HWCAP_ARM_VFP   = 1 << 6,
> +ARM_HWCAP_ARM_EDSP  = 1 << 7,
> +ARM_HWCAP_ARM_JAVA  = 1 << 8,
> +ARM_HWCAP_ARM_IWMMXT= 1 << 9,
> +ARM_HWCAP_ARM_CRUNCH= 1 << 10,
> +ARM_HWCAP_ARM_THUMBEE   = 1 << 11,
> +ARM_HWCAP_ARM_NEON  = 1 << 12,
> +ARM_HWCAP_ARM_VFPv3 = 1 << 13,
> +ARM_HWCAP_ARM_VFPv3D16  = 1 << 14,
> +ARM_HWCAP_ARM_TLS   = 1 << 15,
> +ARM_HWCAP_ARM_VFPv4 = 1 << 16,
> +ARM_HWCAP_ARM_IDIVA = 1 << 17,
> +ARM_HWCAP_ARM_IDIVT = 1 << 18,
> +ARM_HWCAP_ARM_VFPD32= 1 << 19,
> +ARM_HWCAP_ARM_LPAE  = 1 << 20,
> +ARM_HWCAP_ARM_EVTSTRM   = 1 << 21,
> +};
> +
> +static uint32_t get_elf_hwcap(void)
> +{
> +ARMCPU *cpu = ARM_CPU(thread_cpu);
> +uint32_t hwcaps = 0;
> +
> +hwcaps |= ARM_HWCAP_ARM_SWP;
> +hwcaps |= ARM_HWCAP_ARM_HALF;
> +hwcaps |= ARM_HWCAP_ARM_THUMB;
> +hwcaps |= ARM_HWCAP_ARM_FAST_MULT;
> +
> +/* probe for the extra features */
> +/* EDSP is in v5TE and above */
> +GET_FEATURE(ARM_FEATURE_V5, ARM_HWCAP_ARM_EDSP);
> +GET_FEATURE(ARM_FEATURE_IWMMXT, ARM_HWCAP_ARM_IWMMXT);
> +GET_FEATURE(ARM_FEATURE_THUMB2EE, ARM_HWCAP_ARM_THUMBEE);
> +GET_FEATURE(ARM_FEATURE_NEON, ARM_HWCAP_ARM_NEON);
> +GET_FEATURE(ARM_FEATURE_V6K, ARM_HWCAP_ARM_TLS);
> +GET_FEATURE(ARM_FEATURE_LPAE, ARM_HWCAP_ARM_LPAE);
> +GET_FEATURE_ID(aa32_arm_div, ARM_HWCAP_ARM_IDIVA);
> +GET_FEATURE_ID(aa32_thumb_div, ARM_HWCAP_ARM_IDIVT);
> +GET_FEATURE_ID(aa32_vfp, ARM_HWCAP_ARM_VFP);
> +
> +if (cpu_isar_feature(aa32_fpsp_v3, cpu) ||
> +cpu_isar_feature(aa32_fpdp_v3, cpu)) {
> +hwcaps |= ARM_HWCAP_ARM_VFPv3;
> +if (cpu_isar_feature(aa32_simd_r32, cpu)) {
> +hwcaps |= ARM_HWCAP_ARM_VFPD32;
> +} else {
> +hwcaps |= ARM_HWCAP_ARM_VFPv3D16;
> +}
> +}
> +GET_FEATURE_ID(aa32_simdfmac, ARM_HWCAP_ARM_VFPv4);
> +
> +return hwcaps;
> +}
> +
> +#undef GET_FEATURE
> +#undef GET_FEATURE_ID
>
>  #endif /* _TARGET_ARCH_ELF_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH 14/24] bsd-user/arm/target_arch_elf.h: arm defines for ELF

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh  wrote:
>
> Basic set of defines needed for arm ELF file activation.
>
> Signed-off-by: Stacey Son 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/arm/target_arch_elf.h | 36 ++
>  1 file changed, 36 insertions(+)
>  create mode 100644 bsd-user/arm/target_arch_elf.h
>
> diff --git a/bsd-user/arm/target_arch_elf.h b/bsd-user/arm/target_arch_elf.h
> new file mode 100644
> index 00..15b5c66511
> --- /dev/null
> +++ b/bsd-user/arm/target_arch_elf.h
> @@ -0,0 +1,36 @@
> +/*
> + *  arm ELF 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 _TARGET_ARCH_ELF_H_
> +#define _TARGET_ARCH_ELF_H_
> +
> +#define ELF_START_MMAP 0x8000
> +#define ELF_ET_DYN_LOAD_ADDR0x50
> +
> +#define elf_check_arch(x) ((x) == EM_ARM)
> +
> +#define ELF_CLASS   ELFCLASS32
> +#define ELF_DATAELFDATA2LSB
> +#define ELF_ARCHEM_ARM
> +
> +#define USE_ELF_CORE_DUMP
> +#define ELF_EXEC_PAGESIZE   4096
> +
> +#define ELF_HWCAP 0
> +
> +#endif /* _TARGET_ARCH_ELF_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH 22/24] bsd-user/arm/target_arch_signal.h: arm set_mcontext

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh  wrote:
>
> Move the machine context to the CPU state.
>
> Signed-off-by: Stacey Son 
> Signed-off-by: Klye Evans 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/arm/target_arch_signal.h | 31 +++
>  1 file changed, 31 insertions(+)
>
> diff --git a/bsd-user/arm/target_arch_signal.h 
> b/bsd-user/arm/target_arch_signal.h
> index 302fdc2846..1d051af9ae 100644
> --- a/bsd-user/arm/target_arch_signal.h
> +++ b/bsd-user/arm/target_arch_signal.h
> @@ -201,4 +201,35 @@ static inline abi_long get_mcontext(CPUARMState *regs, 
> target_mcontext_t *mcp,
>  return err;
>  }
>
> +/* Compare to arm/arm/machdep.c set_mcontext() */
> +static inline abi_long set_mcontext(CPUARMState *regs, target_mcontext_t 
> *mcp,
> +int srflag)
> +{
> +int err = 0;
> +const uint32_t *gr = mcp->__gregs;
> +uint32_t cpsr;
> +
> +regs->regs[0] = tswap32(gr[TARGET_REG_R0]);
> +regs->regs[1] = tswap32(gr[TARGET_REG_R1]);
> +regs->regs[2] = tswap32(gr[TARGET_REG_R2]);
> +regs->regs[3] = tswap32(gr[TARGET_REG_R3]);
> +regs->regs[4] = tswap32(gr[TARGET_REG_R4]);
> +regs->regs[5] = tswap32(gr[TARGET_REG_R5]);
> +regs->regs[6] = tswap32(gr[TARGET_REG_R6]);
> +regs->regs[7] = tswap32(gr[TARGET_REG_R7]);
> +regs->regs[8] = tswap32(gr[TARGET_REG_R8]);
> +regs->regs[9] = tswap32(gr[TARGET_REG_R9]);
> +regs->regs[10] = tswap32(gr[TARGET_REG_R10]);
> +regs->regs[11] = tswap32(gr[TARGET_REG_R11]);
> +regs->regs[12] = tswap32(gr[TARGET_REG_R12]);
> +
> +regs->regs[13] = tswap32(gr[TARGET_REG_SP]);
> +regs->regs[14] = tswap32(gr[TARGET_REG_LR]);
> +regs->regs[15] = tswap32(gr[TARGET_REG_PC]);
> +cpsr = tswap32(gr[TARGET_REG_CPSR]);
> +cpsr_write(regs, cpsr, CPSR_USER | CPSR_EXEC, CPSRWriteByInstr);
> +
> +return err;
> +}
> +
>  #endif /* !_TARGET_ARCH_SIGNAL_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH 07/24] bsd-user/arm/target_arch_cpu.h: Implment trivial EXCP exceptions

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh  wrote:
>
> Implent EXCP_UDEF, EXCP_DEBUG, EXCP_INTERRUPT, EXCP_ATOMIC and

s/Implent/Implement/

> EXCP_YIELD. The first two generate a signal to the emulated
> binary. EXCP_ATOMIC handles atomic operations. The remainder are fancy
> nops.
>
> Signed-off-by: Stacey Son 
> Signed-off-by: Mikaël Urankar 
> Signed-off-by: Klye Evans 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/arm/target_arch_cpu.h | 28 
>  1 file changed, 28 insertions(+)
>
> diff --git a/bsd-user/arm/target_arch_cpu.h b/bsd-user/arm/target_arch_cpu.h
> index 94c9109c3f..f22384676a 100644
> --- a/bsd-user/arm/target_arch_cpu.h
> +++ b/bsd-user/arm/target_arch_cpu.h
> @@ -47,6 +47,34 @@ static inline void target_cpu_loop(CPUARMState *env)
>  cpu_exec_end(cs);
>  process_queued_cpu_work(cs);
>  switch (trapnr) {
> +case EXCP_UDEF:
> +{
> +/* See arm/arm/undefined.c undefinedinstruction(); */
> +info.si_addr = env->regs[15];
> +info.si_signo = TARGET_SIGILL;
> +info.si_errno = 0;
> +info.si_code = TARGET_ILL_ILLADR;
> +queue_signal(env, info.si_signo, );
> +}
> +break;
> +case EXCP_INTERRUPT:
> +/* just indicate that signals should be handled asap */
> +break;
> +case EXCP_DEBUG:
> +{
> +
> +info.si_signo = TARGET_SIGTRAP;
> +info.si_errno = 0;
> +info.si_code = TARGET_TRAP_BRKPT;
> +queue_signal(env, info.si_signo, );
> +}
> +break;
> +case EXCP_ATOMIC:
> +cpu_exec_step_atomic(cs);
> +break;
> +case EXCP_YIELD:
> +/* nothing to do here for user-mode, just resume guest code */
> +break;
>  default:
>      fprintf(stderr, "qemu: unhandled CPU exception 0x%x - 
> aborting\n",
>  trapnr);
> --
> 2.32.0
>

Modulo typo:

Reviewed-by: Kyle Evans 

Re: [PATCH 19/24] bsd-user/arm/target_arch_signal.h: arm user context and trapframe for signals

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh  wrote:
>
> Arm specific user context structures for signal handling and the closely
> related trap frame.
>
> Signed-off-by: Stacey Son 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/arm/target_arch_signal.h | 38 +++
>  1 file changed, 38 insertions(+)
>
> diff --git a/bsd-user/arm/target_arch_signal.h 
> b/bsd-user/arm/target_arch_signal.h
> index 9fee58ca9c..67355ff28f 100644
> --- a/bsd-user/arm/target_arch_signal.h
> +++ b/bsd-user/arm/target_arch_signal.h
> @@ -90,4 +90,42 @@ typedef struct target_mcontext {
>  } __fpu;
>  } target_mcontext_t;
>
> +typedef struct target_ucontext {
> +target_sigset_t uc_sigmask;
> +target_mcontext_t   uc_mcontext;
> +abi_ulong   uc_link;
> +target_stack_t  uc_stack;
> +int32_t uc_flags;
> +int32_t __spare__[4];
> +} target_ucontext_t;
> +
> +struct target_sigframe {
> +target_siginfo_tsf_si;  /* saved siginfo */
> +target_ucontext_t   sf_uc;  /* saved ucontext */
> +};
> +
> +

We might be able to kill this extra newline? I'm not familiar enough
with qemu's style preferences here...

> +/* compare to sys/arm/include/frame.h */
> +struct target_trapframe {
> +abi_ulong tf_spsr; /* Zero on arm26 */
> +abi_ulong tf_r0;
> +abi_ulong tf_r1;
> +abi_ulong tf_r2;
> +abi_ulong tf_r3;
> +abi_ulong tf_r4;
> +abi_ulong tf_r5;
> +abi_ulong tf_r6;
> +abi_ulong tf_r7;
> +abi_ulong tf_r8;
> +abi_ulong tf_r9;
> +abi_ulong tf_r10;
> +abi_ulong tf_r11;
> +abi_ulong tf_r12;
> +abi_ulong tf_usr_sp;
> +abi_ulong tf_usr_lr;
> +abi_ulong tf_svc_sp; /* Not used on arm26 */
> +abi_ulong tf_svc_lr; /* Not used on arm26 */
> +abi_ulong tf_pc;
> +};
> +
>  #endif /* !_TARGET_ARCH_SIGNAL_H_ */
> --
> 2.32.0
>

I didn't think we actually supported arm26, but I see those comments
also exist verbatim in machine/frame.h; no objection to reflecting
them here, as well.

Reviewed-by: Kyle Evans 

Re: [PATCH 13/24] bsd-user/arm/target_arch_thread.h: Routines to create and switch to a thread

[Kyle Evans]

On Tue, Oct 26, 2021 at 1:01 AM Kyle Evans  wrote:
>
> On Tue, Oct 19, 2021 at 11:45 AM Warner Losh  wrote:
> >
> > Implement target_thread_init (to create a thread) and target_set_upcall
> > (to switch to a thread) for arm.
> >
> > Signed-off-by: Stacey Son 
> > Signed-off-by: Klye Evans 
> > Signed-off-by: Warner Losh 
> > ---
> >  bsd-user/arm/target_arch_thread.h | 71 +++
> >  1 file changed, 71 insertions(+)
> >  create mode 100644 bsd-user/arm/target_arch_thread.h
> >
> > diff --git a/bsd-user/arm/target_arch_thread.h 
> > b/bsd-user/arm/target_arch_thread.h
> > new file mode 100644
> > index 00..317364bb84
> > --- /dev/null
> > +++ b/bsd-user/arm/target_arch_thread.h
> > @@ -0,0 +1,71 @@
> > +/*
> > + *  arm thread support
> > + *
> > + *  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_THREAD_H_
> > +#define _TARGET_ARCH_THREAD_H_
> > +
> > +/* Compare to arm/arm/vm_machdep.c cpu_set_upcall_kse() */
> > +static inline void target_thread_set_upcall(CPUARMState *regs, abi_ulong 
> > entry,
> > +abi_ulong arg, abi_ulong stack_base, abi_ulong stack_size)
> > +{
> > +abi_ulong sp;
> > +
> > +/*
> > + * Make sure the stack is properly aligned.
> > + * arm/include/param.h (STACKLIGN() macro)
> > + */
> > +sp = (u_int)((stack_base + stack_size) -
> > +sizeof(struct target_trapframe)) & ~0x7;
> > +
> > +/* sp = stack base */
> > +regs->regs[13] = sp;
> > +/* pc = start function entry */
> > +regs->regs[15] = entry & 0xfffe;
> > +/* r0 = arg */
> > +regs->regs[0] = arg;
> > +regs->spsr = ARM_CPU_MODE_USR;
> > +if (entry & 0x1) {
> > +regs->spsr |= CPSR_T;
> > +}
> > +}
> > +
> > +static inline void target_thread_init(struct target_pt_regs *regs,
> > +struct image_info *infop)
> > +{
> > +abi_long stack = infop->start_stack;
> > +memset(regs, 0, sizeof(*regs));
> > +regs->ARM_cpsr = 0x10;
> > +if (infop->entry & 1) {
> > +regs->ARM_cpsr |= CPSR_T;
> > +}
> > +regs->ARM_pc = infop->entry & 0xfffe;
> > +regs->ARM_sp = infop->start_stack;
> > +if (bsd_type == target_freebsd) {
> > +regs->ARM_lr = infop->entry & 0xfffe;
> > +}
> > +/* FIXME - what to for failure of get_user()? */
> > +get_user_ual(regs->ARM_r2, stack + 8); /* envp */
> > +get_user_ual(regs->ARM_r1, stack + 4); /* envp */
> > +/* XXX: it seems that r0 is zeroed after ! */
> > +regs->ARM_r0 = 0;
> > +/* For uClinux PIC binaries.  */
> > +/* XXX: Linux does this only on ARM with no MMU (do we care ?) */
> > +regs->ARM_r10 = infop->start_data;
> > +}
> > +
> > +#endif /* !_TARGET_ARCH_THREAD_H_ */
> > --
> > 2.32.0
> >
>
> I think it's obvious enough to folks already familiar with ARM, but I
> wonder if we shouldn't add in some basic commentary about the thumb
> bits above. Something like:
>
> /*
>  * The low bit in an entry point indicates a thumb instruction; the entry 
> point
>  * can't actually exist at this address because it must be 16- or 32-
> bit aligned.
>  * The low bit gets masked off and the T bit in CSPR is twiddled to
> indicate thumb.
>  */

s/CSPR/CPSR/

Re: [PATCH 13/24] bsd-user/arm/target_arch_thread.h: Routines to create and switch to a thread

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh  wrote:
>
> Implement target_thread_init (to create a thread) and target_set_upcall
> (to switch to a thread) for arm.
>
> Signed-off-by: Stacey Son 
> Signed-off-by: Klye Evans 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/arm/target_arch_thread.h | 71 +++
>  1 file changed, 71 insertions(+)
>  create mode 100644 bsd-user/arm/target_arch_thread.h
>
> diff --git a/bsd-user/arm/target_arch_thread.h 
> b/bsd-user/arm/target_arch_thread.h
> new file mode 100644
> index 00..317364bb84
> --- /dev/null
> +++ b/bsd-user/arm/target_arch_thread.h
> @@ -0,0 +1,71 @@
> +/*
> + *  arm thread support
> + *
> + *  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 .
> + */
> +#ifndef _TARGET_ARCH_THREAD_H_
> +#define _TARGET_ARCH_THREAD_H_
> +
> +/* Compare to arm/arm/vm_machdep.c cpu_set_upcall_kse() */
> +static inline void target_thread_set_upcall(CPUARMState *regs, abi_ulong 
> entry,
> +abi_ulong arg, abi_ulong stack_base, abi_ulong stack_size)
> +{
> +abi_ulong sp;
> +
> +/*
> + * Make sure the stack is properly aligned.
> + * arm/include/param.h (STACKLIGN() macro)
> + */
> +sp = (u_int)((stack_base + stack_size) -
> +sizeof(struct target_trapframe)) & ~0x7;
> +
> +/* sp = stack base */
> +regs->regs[13] = sp;
> +/* pc = start function entry */
> +regs->regs[15] = entry & 0xfffe;
> +/* r0 = arg */
> +regs->regs[0] = arg;
> +regs->spsr = ARM_CPU_MODE_USR;
> +if (entry & 0x1) {
> +regs->spsr |= CPSR_T;
> +}
> +}
> +
> +static inline void target_thread_init(struct target_pt_regs *regs,
> +struct image_info *infop)
> +{
> +abi_long stack = infop->start_stack;
> +memset(regs, 0, sizeof(*regs));
> +regs->ARM_cpsr = 0x10;
> +if (infop->entry & 1) {
> +regs->ARM_cpsr |= CPSR_T;
> +}
> +regs->ARM_pc = infop->entry & 0xfffe;
> +regs->ARM_sp = infop->start_stack;
> +if (bsd_type == target_freebsd) {
> +regs->ARM_lr = infop->entry & 0xfffe;
> +}
> +/* FIXME - what to for failure of get_user()? */
> +get_user_ual(regs->ARM_r2, stack + 8); /* envp */
> +get_user_ual(regs->ARM_r1, stack + 4); /* envp */
> +/* XXX: it seems that r0 is zeroed after ! */
> +regs->ARM_r0 = 0;
> +/* For uClinux PIC binaries.  */
> +/* XXX: Linux does this only on ARM with no MMU (do we care ?) */
> +regs->ARM_r10 = infop->start_data;
> +}
> +
> +#endif /* !_TARGET_ARCH_THREAD_H_ */
> --
> 2.32.0
>

I think it's obvious enough to folks already familiar with ARM, but I
wonder if we shouldn't add in some basic commentary about the thumb
bits above. Something like:

/*
 * The low bit in an entry point indicates a thumb instruction; the entry point
 * can't actually exist at this address because it must be 16- or 32-
bit aligned.
 * The low bit gets masked off and the T bit in CSPR is twiddled to
indicate thumb.
 */

Re: [PATCH 11/24] bsd-user/arm/target_arch_vmparam.h: Parameters for arm address space

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh  wrote:
>
> Various parameters describing the layout of the ARM address space. In
> addition, define routines to get the stack pointer and to set the second
> return value.
>
> Signed-off-by: Stacey Son 
> Signed-off-by: Klye Evans 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/arm/target_arch_vmparam.h | 48 ++
>  1 file changed, 48 insertions(+)
>  create mode 100644 bsd-user/arm/target_arch_vmparam.h
>
> diff --git a/bsd-user/arm/target_arch_vmparam.h 
> b/bsd-user/arm/target_arch_vmparam.h
> new file mode 100644
> index 00..4bbc04ddf5
> --- /dev/null
> +++ b/bsd-user/arm/target_arch_vmparam.h
> @@ -0,0 +1,48 @@
> +/*
> + *  arm VM parameters 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 _TARGET_ARCH_VMPARAM_H_
> +#define _TARGET_ARCH_VMPARAM_H_
> +
> +#include "cpu.h"
> +
> +/* compare to sys/arm/include/vmparam.h */
> +#define TARGET_MAXTSIZ  (64 * MiB)   /* max text size */
> +#define TARGET_DFLDSIZ  (128 * MiB)  /* initial data size limit 
> */
> +#define TARGET_MAXDSIZ  (512 * MiB)  /* max data size */
> +#define TARGET_DFLSSIZ  (4 * MiB)/* initial stack size limit 
> */
> +#define TARGET_MAXSSIZ  (64 * MiB)   /* max stack size */
> +#define TARGET_SGROWSIZ (128 * KiB)  /* amount to grow stack */
> +
> +#define TARGET_RESERVED_VA  0xf700
> +
> +/* KERNBASE - 512 MB */
> +#define TARGET_VM_MAXUSER_ADDRESS   (0xc000 - (512 * MiB))
> +#define TARGET_USRSTACK TARGET_VM_MAXUSER_ADDRESS
> +
> +static inline abi_ulong get_sp_from_cpustate(CPUARMState *state)
> +{
> +return state->regs[13]; /* sp */
> +}
> +
> +static inline void set_second_rval(CPUARMState *state, abi_ulong retval2)
> +{
> +state->regs[1] = retval2;
> +}
> +
> +#endif  /* ! _TARGET_ARCH_VMPARAM_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH 16/24] bsd-user/arm/target_arch_elf.h: arm get_hwcap2 impl

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh  wrote:
>
> Implement the extended HW capabilities for HWCAP2.
>
> Signed-off-by: Klye Evans 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/arm/target_arch_elf.h | 22 ++
>  1 file changed, 22 insertions(+)
>
> diff --git a/bsd-user/arm/target_arch_elf.h b/bsd-user/arm/target_arch_elf.h
> index 02d25b8926..4a0215d02e 100644
> --- a/bsd-user/arm/target_arch_elf.h
> +++ b/bsd-user/arm/target_arch_elf.h
> @@ -32,6 +32,7 @@
>  #define ELF_EXEC_PAGESIZE   4096
>
>  #define ELF_HWCAP get_elf_hwcap()
> +#define ELF_HWCAP2 get_elf_hwcap2()
>
>  #define GET_FEATURE(feat, hwcap) \
>  do { if (arm_feature(>env, feat)) { hwcaps |= hwcap; } } while (0)
> @@ -64,6 +65,14 @@ enum {
>  ARM_HWCAP_ARM_EVTSTRM   = 1 << 21,
>  };
>
> +enum {
> +ARM_HWCAP2_ARM_AES  = 1 << 0,
> +ARM_HWCAP2_ARM_PMULL= 1 << 1,
> +ARM_HWCAP2_ARM_SHA1 = 1 << 2,
> +ARM_HWCAP2_ARM_SHA2 = 1 << 3,
> +ARM_HWCAP2_ARM_CRC32= 1 << 4,
> +};
> +
>  static uint32_t get_elf_hwcap(void)
>  {
>  ARMCPU *cpu = ARM_CPU(thread_cpu);
> @@ -100,6 +109,19 @@ static uint32_t get_elf_hwcap(void)
>  return hwcaps;
>  }
>
> +static uint32_t get_elf_hwcap2(void)
> +{
> +ARMCPU *cpu = ARM_CPU(thread_cpu);
> +uint32_t hwcaps = 0;
> +
> +GET_FEATURE_ID(aa32_aes, ARM_HWCAP2_ARM_AES);
> +GET_FEATURE_ID(aa32_pmull, ARM_HWCAP2_ARM_PMULL);
> +GET_FEATURE_ID(aa32_sha1, ARM_HWCAP2_ARM_SHA1);
> +GET_FEATURE_ID(aa32_sha2, ARM_HWCAP2_ARM_SHA2);
> +GET_FEATURE_ID(aa32_crc32, ARM_HWCAP2_ARM_CRC32);
> +return hwcaps;
> +}
> +
>  #undef GET_FEATURE
>  #undef GET_FEATURE_ID
>
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH 12/24] bsd-user/arm/target_arch_sigtramp.h: Signal Trampoline for arm

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh  wrote:
>
> Copy of the signal trampoline code for arm, as well as setup_sigtramp to
> write it to the stack.
>
> Signed-off-by: Stacey Son 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/arm/target_arch_sigtramp.h | 52 +
>  1 file changed, 52 insertions(+)
>  create mode 100644 bsd-user/arm/target_arch_sigtramp.h
>
> diff --git a/bsd-user/arm/target_arch_sigtramp.h 
> b/bsd-user/arm/target_arch_sigtramp.h
> new file mode 100644
> index 00..ed53d336ed
> --- /dev/null
> +++ b/bsd-user/arm/target_arch_sigtramp.h
> @@ -0,0 +1,52 @@
> +/*
> + *  arm sysarch() system call emulation
> + *
> + *  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_SIGTRAMP_H_
> +#define _TARGET_ARCH_SIGTRAMP_H_
> +
> +/* Compare to arm/arm/locore.S ENTRY_NP(sigcode) */
> +static inline abi_long setup_sigtramp(abi_ulong offset, unsigned sigf_uc,
> +unsigned sys_sigreturn)
> +{
> +int i;
> +uint32_t sys_exit = TARGET_FREEBSD_NR_exit;
> +/*
> + * The code has to load r7 manually rather than using
> + * "ldr r7, =SYS_return to make sure the size of the
> + * code is correct.
> + */
> +uint32_t sigtramp_code[] = {
> +/* 1 */ 0xE1AD,  /* mov r0, sp */
> +/* 2 */ 0xE280 + sigf_uc,/* add r0, r0, #SIGF_UC */
> +/* 3 */ 0xE59F700C,  /* ldr r7, [pc, #12] */
> +/* 4 */ 0xEF00 + sys_sigreturn,  /* swi (SYS_sigreturn) */
> +/* 5 */ 0xE59F7008,  /* ldr r7, [pc, #8] */
> +/* 6 */ 0xEF00 + sys_exit,   /* swi (SYS_exit)*/
> +/* 7 */ 0xEAFA,  /* b . -16 */
> +/* 8 */ sys_sigreturn,
> +/* 9 */ sys_exit
> +};
> +
> +for (i = 0; i < 9; i++) {
> +tswap32s(_code[i]);
> +}
> +
> +return memcpy_to_target(offset, sigtramp_code, TARGET_SZSIGCODE);
> +}
> +#endif /* _TARGET_ARCH_SIGTRAMP_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH 10/24] bsd-user/arm/target_arch_reg.h: Implement core dump register copying

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh  wrote:
>
> Implement the register copying routines to extract registers from the
> cpu for core dump generation.
>
> Signed-off-by: Stacey Son 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/arm/target_arch_reg.h | 60 ++
>  1 file changed, 60 insertions(+)
>  create mode 100644 bsd-user/arm/target_arch_reg.h
>
> diff --git a/bsd-user/arm/target_arch_reg.h b/bsd-user/arm/target_arch_reg.h
> new file mode 100644
> index 00..ef5ed5154f
> --- /dev/null
> +++ b/bsd-user/arm/target_arch_reg.h
> @@ -0,0 +1,60 @@
> +/*
> + *  FreeBSD arm register structures
> + *
> + *  Copyright (c) 2015 Stacey 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_REG_H_
> +#define _TARGET_ARCH_REG_H_
> +
> +/* See sys/arm/include/reg.h */
> +typedef struct target_reg {
> +uint32_tr[13];
> +uint32_tr_sp;
> +uint32_tr_lr;
> +uint32_tr_pc;
> +uint32_tr_cpsr;
> +} target_reg_t;
> +
> +typedef struct target_fp_reg {
> +uint32_tfp_exponent;
> +uint32_tfp_mantissa_hi;
> +u_int32_t   fp_mantissa_lo;
> +} target_fp_reg_t;
> +
> +typedef struct target_fpreg {
> +uint32_tfpr_fpsr;
> +target_fp_reg_t fpr[8];
> +} target_fpreg_t;
> +
> +#define tswapreg(ptr)   tswapal(ptr)
> +
> +static inline void target_copy_regs(target_reg_t *regs, const CPUARMState 
> *env)
> +{
> +int i;
> +
> +for (i = 0; i < 13; i++) {
> +regs->r[i] = tswapreg(env->regs[i + 1]);
> +}
> +regs->r_sp = tswapreg(env->regs[13]);
> +regs->r_lr = tswapreg(env->regs[14]);
> +regs->r_pc = tswapreg(env->regs[15]);
> +regs->r_cpsr = tswapreg(cpsr_read((CPUARMState *)env));
> +}
> +
> +#undef tswapreg
> +
> +#endif /* !_TARGET_ARCH_REG_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH 08/24] bsd-user/arm/target_arch_cpu.h: Implement data abort exceptions

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh  wrote:
>
> Implement EXCP_PREFETCH_ABORT AND EXCP_DATA_ABORT. Both of these data
> exceptions cause a SIGSEGV.
>
> Signed-off-by: Klye Evans 
> Signed-off-by: Olivier Houchard 
> Signed-off-by: Stacey Son 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/arm/target_arch_cpu.h | 11 +++
>  1 file changed, 11 insertions(+)
>
> diff --git a/bsd-user/arm/target_arch_cpu.h b/bsd-user/arm/target_arch_cpu.h
> index f22384676a..62d6ee89b6 100644
> --- a/bsd-user/arm/target_arch_cpu.h
> +++ b/bsd-user/arm/target_arch_cpu.h
> @@ -60,6 +60,17 @@ static inline void target_cpu_loop(CPUARMState *env)
>  case EXCP_INTERRUPT:
>  /* just indicate that signals should be handled asap */
>  break;
> +case EXCP_PREFETCH_ABORT:
> +/* See arm/arm/trap.c prefetch_abort_handler() */
> +case EXCP_DATA_ABORT:
> +/* See arm/arm/trap.c data_abort_handler() */
> +info.si_signo = TARGET_SIGSEGV;
> +info.si_errno = 0;
> +/* XXX: check env->error_code */
> +info.si_code = 0;
> +info.si_addr = env->exception.vaddress;
> +queue_signal(env, info.si_signo, );
> +    break;
>  case EXCP_DEBUG:
>  {
>
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH 06/24] bsd-user/arm/target_arch_cpu.h: Dummy target_cpu_loop implementation

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh  wrote:
>
> Add a boiler plate CPU loop that does nothing except return an error for
> all traps.
>
> Signed-off-by: Sean Bruno 
> Signed-off-by: Stacey Son 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/arm/target_arch_cpu.h | 22 ++
>  1 file changed, 22 insertions(+)
>
> diff --git a/bsd-user/arm/target_arch_cpu.h b/bsd-user/arm/target_arch_cpu.h
> index c71ec000b3..94c9109c3f 100644
> --- a/bsd-user/arm/target_arch_cpu.h
> +++ b/bsd-user/arm/target_arch_cpu.h
> @@ -35,6 +35,28 @@ static inline void target_cpu_init(CPUARMState *env,
>  }
>  }
>
> +static inline void target_cpu_loop(CPUARMState *env)
> +{
> +int trapnr;
> +target_siginfo_t info;
> +CPUState *cs = env_cpu(env);
> +
> +for (;;) {
> +cpu_exec_start(cs);
> +trapnr = cpu_exec(cs);
> +cpu_exec_end(cs);
> +process_queued_cpu_work(cs);
> +switch (trapnr) {
> +default:
> +fprintf(stderr, "qemu: unhandled CPU exception 0x%x - 
> aborting\n",
> +trapnr);
> +cpu_dump_state(cs, stderr, 0);
> +abort();
> +} /* switch() */
> +process_pending_signals(env);
> +} /* for (;;) */
> +}
> +
>  static inline void target_cpu_clone_regs(CPUARMState *env, target_ulong 
> newsp)
>  {
>  if (newsp) {
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH 03/24] bsd-user/arm/target_arch_cpu.c: Target specific TLS routines

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh  wrote:
>
> Target specific TLS routines to get and set the TLS values.
>
> Signed-off-by: Klye Evans 

s/Klye/Kyle/ :-)

> Signed-off-by: Stacey Son 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/arm/target_arch.h | 28 
>  bsd-user/arm/target_arch_cpu.c | 39 ++
>  2 files changed, 67 insertions(+)
>  create mode 100644 bsd-user/arm/target_arch.h
>  create mode 100644 bsd-user/arm/target_arch_cpu.c
>
> diff --git a/bsd-user/arm/target_arch.h b/bsd-user/arm/target_arch.h
> new file mode 100644
> index 00..93cfaea098
> --- /dev/null
> +++ b/bsd-user/arm/target_arch.h
> @@ -0,0 +1,28 @@
> +/*
> + * ARM 32-bit specific prototypes for bsd-user
> + *
> + *  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_H_
> +#define _TARGET_ARCH_H_
> +
> +#include "qemu.h"
> +
> +void target_cpu_set_tls(CPUARMState *env, target_ulong newtls);
> +target_ulong target_cpu_get_tls(CPUARMState *env);
> +
> +#endif /* !_TARGET_ARCH_H_ */
> diff --git a/bsd-user/arm/target_arch_cpu.c b/bsd-user/arm/target_arch_cpu.c
> new file mode 100644
> index 00..02bf9149d5
> --- /dev/null
> +++ b/bsd-user/arm/target_arch_cpu.c
> @@ -0,0 +1,39 @@
> +/*
> + *  arm cpu related 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/>.
> + */
> +#include "target_arch.h"
> +
> +void target_cpu_set_tls(CPUARMState *env, target_ulong newtls)
> +{
> +if (access_secure_reg(env)) {
> +env->cp15.tpidrurw_s = newtls;
> +env->cp15.tpidruro_s = newtls;
> +return;
> +}
> +
> +env->cp15.tpidr_el[0] = newtls;
> +env->cp15.tpidrro_el[0] = newtls;
> +}
> +
> +target_ulong target_cpu_get_tls(CPUARMState *env)
> +{
> +if (access_secure_reg(env)) {
> +return env->cp15.tpidruro_s;
> +}
> +return env->cp15.tpidrro_el[0];
> +}
> --
> 2.32.0
>

Modulo typo:

Reviewed-by: Kyle Evans 

Re: [PATCH 04/24] bsd-user/arm/target_arch_cpu.h: CPU Loop definitions

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh  wrote:
>
> target_arch_cpu.h is for CPU loop definitions. Create the file and
> define target_cpu_init and target_cpu_reset for arm.
>
> Signed-off-by: Olivier Houchard 
> Signed-off-by: Stacey Son 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/arm/target_arch_cpu.h | 42 ++
>  1 file changed, 42 insertions(+)
>  create mode 100644 bsd-user/arm/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 00..0f3538196d
> --- /dev/null
> +++ b/bsd-user/arm/target_arch_cpu.h
> @@ -0,0 +1,42 @@
> +/*
> + *  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_
> +
> +#include "target_arch.h"
> +
> +#define TARGET_DEFAULT_CPU_MODEL "any"
> +
> +static inline void target_cpu_init(CPUARMState *env,
> +struct target_pt_regs *regs)
> +{
> +int i;
> +
> +cpsr_write(env, regs->uregs[16], 0x, CPSRWriteRaw);
> +for (i = 0; i < 16; i++) {
> +    env->regs[i] = regs->uregs[i];
> +}
> +}
> +
> +static inline void target_cpu_reset(CPUArchState *cpu)
> +{
> +}
> +
> +#endif /* !_TARGET_ARCH_CPU_H */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH 09/24] bsd-user/arm/target_arch_cpu.h: Implement system call dispatch

[Kyle Evans]

RT == ret) {
> +env->regs[15] -= env->thumb ? 2 : 4;
> +break;
> +}
> +if ((unsigned int)ret >= (unsigned int)(-515)) {
> +ret = -ret;
> +cpsr_write(env, CPSR_C, CPSR_C, CPSRWriteByInstr);
> +env->regs[0] = ret;
> +} else {
> +cpsr_write(env, 0, CPSR_C, CPSRWriteByInstr);
> +env->regs[0] = ret; /* XXX need to handle lseek()? */
> +/* env->regs[1] = 0; */
> +    }
> +} else {
> +fprintf(stderr, "qemu: bsd_type (= %d) syscall "
> +"not supported\n", bsd_type);
> +}
> +}
> +break;
>  case EXCP_INTERRUPT:
>  /* just indicate that signals should be handled asap */
>  break;
> --
> 2.32.0
>

Modulo typo:

Reviewed-by: Kyle Evans 

Re: [PATCH 05/24] bsd-user/arm/target_arch_cpu.h: Implement target_cpu_clone_regs

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh  wrote:
>
> Implement target_cpu_clone_regs to clone the resister state on a fork.
>
> Signed-off-by: Stacey Son 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/arm/target_arch_cpu.h | 8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/bsd-user/arm/target_arch_cpu.h b/bsd-user/arm/target_arch_cpu.h
> index 0f3538196d..c71ec000b3 100644
> --- a/bsd-user/arm/target_arch_cpu.h
> +++ b/bsd-user/arm/target_arch_cpu.h
> @@ -35,6 +35,14 @@ static inline void target_cpu_init(CPUARMState *env,
>  }
>  }
>
> +static inline void target_cpu_clone_regs(CPUARMState *env, target_ulong 
> newsp)
> +{
> +if (newsp) {
> +env->regs[13] = newsp;
> +}
> +env->regs[0] = 0;
> +}
> +
>  static inline void target_cpu_reset(CPUArchState *cpu)
>  {
>  }
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH 02/24] bsd-user/arm/target_syscall.h: Add copyright and update name

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh  wrote:
>
> The preferred name for the 32-bit arm is now armv7. Update the name to
> reflect that. In addition, add Stacey's copyright to this file and
> update the include guards to the new convention.
>
> Signed-off-by: Stacey Son 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/arm/target_syscall.h | 27 +++
>  1 file changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/bsd-user/arm/target_syscall.h b/bsd-user/arm/target_syscall.h
> index ef4b37f017..a5f2bb4e01 100644
> --- a/bsd-user/arm/target_syscall.h
> +++ b/bsd-user/arm/target_syscall.h
> @@ -1,5 +1,24 @@
> -#ifndef BSD_USER_ARCH_SYSCALL_H_
> -#define BSD_USER_ARCH_SYSCALL_H_
> +/*
> + *  arm cpu system call stubs
> + *
> + *  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_SYSCALL_H_
> +#define _TARGET_ARCH_SYSCALL_H_
>
>  struct target_pt_regs {
>  abi_long uregs[17];
> @@ -31,6 +50,6 @@ struct target_pt_regs {
>  #define TARGET_FREEBSD_ARM_GET_TP   3
>
>  #define TARGET_HW_MACHINE   "arm"
> -#define TARGET_HW_MACHINE_ARCH  "armv6"
> +#define TARGET_HW_MACHINE_ARCH  "armv7"
>
> -#endif /* !BSD_USER_ARCH_SYSCALL_H_ */
> +#endif /* !_TARGET_ARCH_SYSCALL_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH 01/24] bsd-user/arm/target_arch_sysarch.h: Use consistent include guards

[Kyle Evans]

On Tue, Oct 19, 2021 at 11:45 AM Warner Losh  wrote:
>
> As part of upstreaming, the include guards have been made more
> consistent. Update this file to use the new guards.
>
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/arm/target_arch_sysarch.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/bsd-user/arm/target_arch_sysarch.h 
> b/bsd-user/arm/target_arch_sysarch.h
> index 632a5cd453..8cc6bff207 100644
> --- a/bsd-user/arm/target_arch_sysarch.h
> +++ b/bsd-user/arm/target_arch_sysarch.h
> @@ -17,8 +17,8 @@
>   *  along with this program; if not, see <http://www.gnu.org/licenses/>.
>   */
>
> -#ifndef BSD_USER_ARCH_SYSARCH_H_
> -#define BSD_USER_ARCH_SYSARCH_H_
> +#ifndef _TARGET_ARCH_SYSARCH_H_
> +#define _TARGET_ARCH_SYSARCH_H_
>
>  #include "target_syscall.h"
>  #include "target_arch.h"
> @@ -75,4 +75,4 @@ static inline void do_freebsd_arch_print_sysarch(
>  }
>  }
>
> -#endif /*!BSD_USER_ARCH_SYSARCH_H_ */
> +#endif /*!_TARGET_ARCH_SYSARCH_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH v2 01/15] meson: *-user: only descend into *-user when configured

[Kyle Evans]

On Mon, Oct 18, 2021 at 12:02 AM Warner Losh  wrote:
>
>
>
> On Sun, Oct 17, 2021 at 10:29 PM Warner Losh  wrote:
>>
>>
>>
>> On Sun, Oct 17, 2021 at 9:43 PM Kyle Evans  wrote:
>>>
>>> On Fri, Oct 8, 2021 at 6:15 PM Warner Losh  wrote:
>>> >
>>> > To increase flexibility, only descend into *-user when that is
>>> > configured. This allows *-user to selectively include directories based
>>> > on the host OS which may not exist on all hosts. Adopt Paolo's
>>> > suggestion of checking the configuration in the directories that know
>>> > about the configuration.
>>> >
>>> > Message-Id: <20210926220103.1721355-2-f4...@amsat.org>
>>> > Message-Id: <20210926220103.1721355-3-f4...@amsat.org>
>>> > Signed-off-by: Philippe Mathieu-Daudé 
>>> > Signed-off-by: Warner Losh 
>>> > Acked-by: Paolo Bonzini 
>>> >
>>> > Sponsored by:   Netflix
>>> > ---
>>> >  bsd-user/meson.build   | 4 
>>> >  linux-user/meson.build | 4 
>>> >  meson.build| 3 +--
>>> >  3 files changed, 9 insertions(+), 2 deletions(-)
>>> >
>>> > diff --git a/bsd-user/meson.build b/bsd-user/meson.build
>>> > index 0369549340..243fb78930 100644
>>> > --- a/bsd-user/meson.build
>>> > +++ b/bsd-user/meson.build
>>> > @@ -1,3 +1,7 @@
>>> > +if not config_target.has_key('CONFIG_BSD_USER')
>>> > +   subdir_done()
>>> > +endif
>>> > +
>>> >  bsd_user_ss.add(files(
>>> >'bsdload.c',
>>> >'elfload.c',
>>> > diff --git a/linux-user/meson.build b/linux-user/meson.build
>>> > index 9549f81682..602255a3d6 100644
>>> > --- a/linux-user/meson.build
>>> > +++ b/linux-user/meson.build
>>> > @@ -1,3 +1,7 @@
>>> > +if not config_target.has_key('CONFIG_LINUX_USER')
>>> > +   subdir_done()
>>> > +endif
>>> > +
>>> >  linux_user_ss.add(files(
>>> >'elfload.c',
>>> >'exit.c',
>>> > diff --git a/meson.build b/meson.build
>>> > index 99a0a3e689..1f2da5f7d9 100644
>>> > --- a/meson.build
>>> > +++ b/meson.build
>>> > @@ -2303,10 +2303,9 @@ subdir('ebpf')
>>> >
>>> >  common_ss.add(libbpf)
>>> >
>>> > -bsd_user_ss.add(files('gdbstub.c'))
>>> >  specific_ss.add_all(when: 'CONFIG_BSD_USER', if_true: bsd_user_ss)
>>> >
>>> > -linux_user_ss.add(files('gdbstub.c', 'thunk.c'))
>>> > +linux_user_ss.add(files('thunk.c'))
>>> >  specific_ss.add_all(when: 'CONFIG_LINUX_USER', if_true: linux_user_ss)
>>> >
>>> >  # needed for fuzzing binaries
>>> > --
>>> > 2.32.0
>>> >
>>>
>>> I don't understand the gdbstub.c removal  here; don't we still want to
>>> be compiling it in, just only if the appropriate
>>> CONFIG_{BSD,LINUX}_USER knob is set? I note that it doesn't appear to
>>> be added in individual *-user/meson.build, I assume it's uncommon to
>>> add in ../foo.c in meson-land...
>>
>>
>> It's added to specific_ss at line 2536
>> specific_ss.add(files('cpu.c', 'disas.c', 'gdbstub.c'), capstone)
>>
>> so we don't need to add it again here.
>
>
> I've also confirmed that it's built as both 
> libqemu-i386-bsd-user.fa.p/gdbstub.c.o
> and libqemu-x86_64-bsd-user.fa.p/gdbstub.c.o, which is what I'd expect given
> the current upstream supported architectures are only i386 and x86_64.
>
> Warner

Ah, ok, thanks! So that looks like a kind-of tangential cleanup, but
related enough that it makes sense.

Reviewed-by: Kyle Evans 

Re: [PATCH v2 15/15] bsd-user/signal: Create a dummy signal queueing function

[Kyle Evans]

On Fri, Oct 8, 2021 at 6:15 PM Warner Losh  wrote:
>
> Create dummy signal queueing function so we can start to integrate other
> architectures (at the cost of signals remaining broken) to tame the
> dependency graph a bit and to bring in signals in a more controlled
> fashion. Log unimplemented events to it in the mean time.
>
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/qemu.h   |  2 +-
>  bsd-user/signal.c | 11 ++-
>  2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
> index 5b815c3a23..62095eb975 100644
> --- a/bsd-user/qemu.h
> +++ b/bsd-user/qemu.h
> @@ -17,7 +17,6 @@
>  #ifndef QEMU_H
>  #define QEMU_H
>
> -
>  #include "qemu/osdep.h"
>  #include "cpu.h"
>  #include "qemu/units.h"
> @@ -209,6 +208,7 @@ void process_pending_signals(CPUArchState *cpu_env);
>  void signal_init(void);
>  long do_sigreturn(CPUArchState *env);
>  long do_rt_sigreturn(CPUArchState *env);
> +void queue_signal(CPUArchState *env, int sig, target_siginfo_t *info);
>  abi_long do_sigaltstack(abi_ulong uss_addr, abi_ulong uoss_addr, abi_ulong 
> sp);
>
>  /* mmap.c */
> diff --git a/bsd-user/signal.c b/bsd-user/signal.c
> index ad6d935569..0c1093deb1 100644
> --- a/bsd-user/signal.c
> +++ b/bsd-user/signal.c
> @@ -16,10 +16,19 @@
>   *  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 "qemu/osdep.h"
>
> +#include "qemu/osdep.h"
>  #include "qemu.h"
>
> +/*
> + * Queue a signal so that it will be send to the virtual CPU as soon as
> + * possible.
> + */
> +void queue_signal(CPUArchState *env, int sig, target_siginfo_t *info)
> +{
> +qemu_log_mask(LOG_UNIMP, "No signal queueing, dropping signal %d\n", 
> sig);
> +}
> +
>  void signal_init(void)
>  {
>  }
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH v3 7/9] bsd-user/mmap.c: Don't mmap fd == -1 independently from MAP_ANON flag

[Kyle Evans]

On Fri, Oct 8, 2021 at 4:29 PM Warner Losh  wrote:
>
> From: Guy Yur 
>
> Switch checks for !(flags & MAP_ANONYMOUS) with checks for fd != -1.
> MAP_STACK and MAP_GUARD both require fd == -1 and don't require mapping
> the fd either. Add analysis from Guy Yur detailing the different cases
> for MAP_GUARD and MAP_STACK.
>
> Signed-off-by: Guy Yur 
> [ partially merged before, finishing the job and documenting origin]
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/mmap.c | 30 +-
>  1 file changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
> index face98573f..4ecd949a10 100644
> --- a/bsd-user/mmap.c
> +++ b/bsd-user/mmap.c
> @@ -127,7 +127,27 @@ error:
>  return ret;
>  }
>
> -/* map an incomplete host page */
> +/*
> + * map an incomplete host page
> + *
> + * mmap_frag can be called with a valid fd, if flags doesn't contain one of
> + * MAP_ANON, MAP_STACK, MAP_GUARD. If we need to map a page in those cases, 
> we
> + * pass fd == -1. However, if flags contains MAP_GUARD then MAP_ANON cannot 
> be
> + * added.
> + *
> + * * If fd is valid (not -1) we want to map the pages with MAP_ANON.
> + * * If flags contains MAP_GUARD we don't want to add MAP_ANON because it
> + *   will be rejected.  See kern_mmap's enforcing of constraints for 
> MAP_GUARD
> + *   in sys/vm/vm_mmap.c.
> + * * If flags contains MAP_ANON it doesn't matter if we add it or not.
> + * * If flags contains MAP_STACK, mmap adds MAP_ANON when called so doesn't
> + *   matter if we add it or not either. See enforcing of constraints for
> + *   MAP_STACK in kern_mmap.
> + *
> + * Don't add MAP_ANON for the flags that use fd == -1 without specifying the
> + * flags directly, with the assumption that future flags that require fd == 
> -1
> + * will also not require MAP_ANON.
> + */
>  static int mmap_frag(abi_ulong real_start,
>   abi_ulong start, abi_ulong end,
>   int prot, int flags, int fd, abi_ulong offset)
> @@ -147,9 +167,9 @@ static int mmap_frag(abi_ulong real_start,
>  }
>
>  if (prot1 == 0) {
> -/* no page was there, so we allocate one */
> +/* no page was there, so we allocate one. See also above. */
>  void *p = mmap(host_start, qemu_host_page_size, prot,
> -   flags | MAP_ANON, -1, 0);
> +   flags | ((fd != -1) ? MAP_ANON : 0), -1, 0);
>  if (p == MAP_FAILED)
>  return -1;
>  prot1 = prot;
> @@ -157,7 +177,7 @@ static int mmap_frag(abi_ulong real_start,
>  prot1 &= PAGE_BITS;
>
>  prot_new = prot | prot1;
> -if (!(flags & MAP_ANON)) {
> +if (fd != -1) {
>  /* msync() won't work here, so we return an error if write is
> possible while it is a shared mapping */
>  if ((flags & TARGET_BSD_MAP_FLAGMASK) == MAP_SHARED &&
> @@ -565,7 +585,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int 
> prot,
>   * worst case: we cannot map the file because the offset is not
>   * aligned, so we read it
>   */
> -if (!(flags & MAP_ANON) &&
> +if (fd != -1 &&
>  (offset & ~qemu_host_page_mask) != (start & 
> ~qemu_host_page_mask)) {
>  /*
>   * msync() won't work here, so we return an error if write is
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH v2 01/15] meson: *-user: only descend into *-user when configured

[Kyle Evans]

On Fri, Oct 8, 2021 at 6:15 PM Warner Losh  wrote:
>
> To increase flexibility, only descend into *-user when that is
> configured. This allows *-user to selectively include directories based
> on the host OS which may not exist on all hosts. Adopt Paolo's
> suggestion of checking the configuration in the directories that know
> about the configuration.
>
> Message-Id: <20210926220103.1721355-2-f4...@amsat.org>
> Message-Id: <20210926220103.1721355-3-f4...@amsat.org>
> Signed-off-by: Philippe Mathieu-Daudé 
> Signed-off-by: Warner Losh 
> Acked-by: Paolo Bonzini 
>
> Sponsored by:   Netflix
> ---
>  bsd-user/meson.build   | 4 
>  linux-user/meson.build | 4 
>  meson.build| 3 +--
>  3 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/bsd-user/meson.build b/bsd-user/meson.build
> index 0369549340..243fb78930 100644
> --- a/bsd-user/meson.build
> +++ b/bsd-user/meson.build
> @@ -1,3 +1,7 @@
> +if not config_target.has_key('CONFIG_BSD_USER')
> +   subdir_done()
> +endif
> +
>  bsd_user_ss.add(files(
>'bsdload.c',
>'elfload.c',
> diff --git a/linux-user/meson.build b/linux-user/meson.build
> index 9549f81682..602255a3d6 100644
> --- a/linux-user/meson.build
> +++ b/linux-user/meson.build
> @@ -1,3 +1,7 @@
> +if not config_target.has_key('CONFIG_LINUX_USER')
> +   subdir_done()
> +endif
> +
>  linux_user_ss.add(files(
>'elfload.c',
>'exit.c',
> diff --git a/meson.build b/meson.build
> index 99a0a3e689..1f2da5f7d9 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2303,10 +2303,9 @@ subdir('ebpf')
>
>  common_ss.add(libbpf)
>
> -bsd_user_ss.add(files('gdbstub.c'))
>  specific_ss.add_all(when: 'CONFIG_BSD_USER', if_true: bsd_user_ss)
>
> -linux_user_ss.add(files('gdbstub.c', 'thunk.c'))
> +linux_user_ss.add(files('thunk.c'))
>  specific_ss.add_all(when: 'CONFIG_LINUX_USER', if_true: linux_user_ss)
>
>  # needed for fuzzing binaries
> --
> 2.32.0
>

I don't understand the gdbstub.c removal  here; don't we still want to
be compiling it in, just only if the appropriate
CONFIG_{BSD,LINUX}_USER knob is set? I note that it doesn't appear to
be added in individual *-user/meson.build, I assume it's uncommon to
add in ../foo.c in meson-land...

Thanks,

Kyle Evans

Re: [PATCH v3 6/9] bsd-user/mmap.c: Convert to qemu_log logging for mmap debugging

[Kyle Evans]

On Fri, Oct 8, 2021 at 4:25 PM Warner Losh  wrote:
>
> Convert DEBUG_MMAP to qemu_log CPU_LOG_PAGE.
>
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/mmap.c | 53 +
>  1 file changed, 23 insertions(+), 30 deletions(-)
>
> diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
> index 301108ed25..face98573f 100644
> --- a/bsd-user/mmap.c
> +++ b/bsd-user/mmap.c
> @@ -21,8 +21,6 @@
>  #include "qemu.h"
>  #include "qemu-common.h"
>
> -//#define DEBUG_MMAP
> -
>  static pthread_mutex_t mmap_mutex = PTHREAD_MUTEX_INITIALIZER;
>  static __thread int mmap_lock_count;
>
> @@ -67,14 +65,11 @@ int target_mprotect(abi_ulong start, abi_ulong len, int 
> prot)
>  abi_ulong end, host_start, host_end, addr;
>  int prot1, ret;
>
> -#ifdef DEBUG_MMAP
> -printf("mprotect: start=0x" TARGET_ABI_FMT_lx
> -   "len=0x" TARGET_ABI_FMT_lx " prot=%c%c%c\n", start, len,
> -   prot & PROT_READ ? 'r' : '-',
> -   prot & PROT_WRITE ? 'w' : '-',
> -   prot & PROT_EXEC ? 'x' : '-');
> -#endif
> -
> +qemu_log_mask(CPU_LOG_PAGE, "mprotect: start=0x" TARGET_ABI_FMT_lx
> +  " len=0x" TARGET_ABI_FMT_lx " prot=%c%c%c\n", start, len,
> +  prot & PROT_READ ? 'r' : '-',
> +  prot & PROT_WRITE ? 'w' : '-',
> +  prot & PROT_EXEC ? 'x' : '-');
>  if ((start & ~TARGET_PAGE_MASK) != 0)
>  return -EINVAL;
>  len = TARGET_PAGE_ALIGN(len);
> @@ -391,45 +386,43 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, 
> int prot,
>  abi_ulong ret, end, real_start, real_end, retaddr, host_offset, host_len;
>
>  mmap_lock();
> -#ifdef DEBUG_MMAP
> -{
> -printf("mmap: start=0x" TARGET_ABI_FMT_lx
> -   " len=0x" TARGET_ABI_FMT_lx " prot=%c%c%c flags=",
> -   start, len,
> -   prot & PROT_READ ? 'r' : '-',
> -   prot & PROT_WRITE ? 'w' : '-',
> -   prot & PROT_EXEC ? 'x' : '-');
> +if (qemu_loglevel_mask(CPU_LOG_PAGE)) {
> +qemu_log("mmap: start=0x" TARGET_ABI_FMT_lx
> + " len=0x" TARGET_ABI_FMT_lx " prot=%c%c%c flags=",
> + start, len,
> + prot & PROT_READ ? 'r' : '-',
> + prot & PROT_WRITE ? 'w' : '-',
> + prot & PROT_EXEC ? 'x' : '-');
>  if (flags & MAP_ALIGNMENT_MASK) {
> -printf("MAP_ALIGNED(%u) ", (flags & MAP_ALIGNMENT_MASK)
> ->> MAP_ALIGNMENT_SHIFT);
> +qemu_log("MAP_ALIGNED(%u) ",
> + (flags & MAP_ALIGNMENT_MASK) >> MAP_ALIGNMENT_SHIFT);
>  }
>  if (flags & MAP_GUARD) {
> -printf("MAP_GUARD ");
> +qemu_log("MAP_GUARD ");
>  }
>  if (flags & MAP_FIXED) {
> -printf("MAP_FIXED ");
> +qemu_log("MAP_FIXED ");
>  }
>  if (flags & MAP_ANON) {
> -printf("MAP_ANON ");
> +qemu_log("MAP_ANON ");
>  }
>  if (flags & MAP_EXCL) {
> -printf("MAP_EXCL ");
> +qemu_log("MAP_EXCL ");
>  }
>  if (flags & MAP_PRIVATE) {
> -printf("MAP_PRIVATE ");
> +qemu_log("MAP_PRIVATE ");
>  }
>  if (flags & MAP_SHARED) {
> -printf("MAP_SHARED ");
> +qemu_log("MAP_SHARED ");
>  }
>  if (flags & MAP_NOCORE) {
> -printf("MAP_NOCORE ");
> +qemu_log("MAP_NOCORE ");
>  }
>  if (flags & MAP_STACK) {
> -printf("MAP_STACK ");
> +qemu_log("MAP_STACK ");
>  }
> -printf("fd=%d offset=0x%llx\n", fd, offset);
> +qemu_log("fd=%d offset=0x%lx\n", fd, offset);
>  }
> -#endif
>
>  if ((flags & MAP_ANON) && fd != -1) {
>  errno = EINVAL;
> --
> 2.32.0
>
>

Reviewed-by: Kyle Evans 

Re: [PATCH v2 13/15] bsd-user/sysarch: Provide a per-arch framework for sysarch syscall

[Kyle Evans]

On Fri, Oct 8, 2021 at 6:15 PM Warner Losh  wrote:
>
> Add the missing glue to pull in do_freebsd_sysarch to call
> do_freebsd_arch_sysarch. Put it in os-sys.c, which will be used for
> sysctl and sysarch system calls because they are mostly arch specific.
>
> Signed-off-by: Stacey Son 
> Signed-off-by: Warner Losh 
> Reviewed-by: Richard Henderson 
> ---
>  bsd-user/freebsd/meson.build |  3 +++
>  bsd-user/freebsd/os-sys.c| 27 +++
>  bsd-user/meson.build |  3 +++
>  bsd-user/qemu.h  |  3 +++
>  bsd-user/syscall.c   |  7 ---
>  5 files changed, 36 insertions(+), 7 deletions(-)
>  create mode 100644 bsd-user/freebsd/meson.build
>  create mode 100644 bsd-user/freebsd/os-sys.c
>
> diff --git a/bsd-user/freebsd/meson.build b/bsd-user/freebsd/meson.build
> new file mode 100644
> index 00..4b69cca7b9
> --- /dev/null
> +++ b/bsd-user/freebsd/meson.build
> @@ -0,0 +1,3 @@
> +bsd_user_ss.add(files(
> +  'os-sys.c',
> +))
> diff --git a/bsd-user/freebsd/os-sys.c b/bsd-user/freebsd/os-sys.c
> new file mode 100644
> index 00..309e27b9d6
> --- /dev/null
> +++ b/bsd-user/freebsd/os-sys.c
> @@ -0,0 +1,27 @@
> +/*
> + *  FreeBSD sysctl() and sysarch() system call emulation
> + *
> + *  Copyright (c) 2013-15 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 "qemu.h"
> +#include "target_arch_sysarch.h"
> +
> +/* sysarch() is architecture dependent. */
> +abi_long do_freebsd_sysarch(void *cpu_env, abi_long arg1, abi_long arg2)
> +{
> +return do_freebsd_arch_sysarch(cpu_env, arg1, arg2);
> +}
> diff --git a/bsd-user/meson.build b/bsd-user/meson.build
> index 243fb78930..a4163c91ff 100644
> --- a/bsd-user/meson.build
> +++ b/bsd-user/meson.build
> @@ -12,3 +12,6 @@ bsd_user_ss.add(files(
>'syscall.c',
>'uaccess.c',
>  ))
> +
> +# Pull in the OS-specific build glue, if any
> +subdir(targetos)
> diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
> index cdb85140f4..e65e41d53d 100644
> --- a/bsd-user/qemu.h
> +++ b/bsd-user/qemu.h
> @@ -239,6 +239,9 @@ extern unsigned long target_sgrowsiz;
>  abi_long get_errno(abi_long ret);
>  bool is_error(abi_long ret);
>
> +/* os-sys.c */
> +abi_long do_freebsd_sysarch(void *cpu_env, abi_long arg1, abi_long arg2);
> +
>  /* user access */
>
>  #define VERIFY_READ  PAGE_READ
> diff --git a/bsd-user/syscall.c b/bsd-user/syscall.c
> index d3b9f431e2..d3322760f4 100644
> --- a/bsd-user/syscall.c
> +++ b/bsd-user/syscall.c
> @@ -88,13 +88,6 @@ static abi_long do_obreak(abi_ulong new_brk)
>  return 0;
>  }
>
> -#if defined(TARGET_I386)
> -static abi_long do_freebsd_sysarch(CPUX86State *env, int op, abi_ulong parms)
> -{
> -do_freebsd_arch_sysarch(env, op, parms);
> -}
> -#endif
> -
>  #ifdef __FreeBSD__
>  /*
>   * XXX this uses the undocumented oidfmt interface to find the kind of
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH v2 10/15] bsd-user: Remove used from TaskState

[Kyle Evans]

On Fri, Oct 8, 2021 at 6:15 PM Warner Losh  wrote:
>
> The 'used' field in TaskState is write only. Remove it from TaskState.
>
> Signed-off-by: Warner Losh 
> Reviewed-by: Richard Henderson 
> Reviewed-by: Philippe Mathieu-Daudé 
> ---
>  bsd-user/main.c | 1 -
>  bsd-user/qemu.h | 1 -
>  2 files changed, 2 deletions(-)
>
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index 48643eeabc..ee84554854 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -210,7 +210,6 @@ void init_task_state(TaskState *ts)
>  {
>  int i;
>
> -ts->used = 1;
>  ts->first_free = ts->sigqueue_table;
>  for (i = 0; i < MAX_SIGQUEUE_SIZE - 1; i++) {
>  ts->sigqueue_table[i].next = >sigqueue_table[i + 1];
> diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
> index 3b8475394c..c1170f14d9 100644
> --- a/bsd-user/qemu.h
> +++ b/bsd-user/qemu.h
> @@ -92,7 +92,6 @@ typedef struct TaskState {
>
>  struct TaskState *next;
>  struct bsd_binprm *bprm;
> -int used; /* non zero if used */
>      struct image_info *info;
>
>  struct emulated_sigtable sigtab[TARGET_NSIG];
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH v2 11/15] bsd-user: Add stop_all_tasks

[Kyle Evans]

On Fri, Oct 8, 2021 at 6:15 PM Warner Losh  wrote:
>
> Similar to the same function in linux-user: this stops all the current tasks.
>
> Signed-off-by: Stacey Son 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/main.c | 9 +
>  bsd-user/qemu.h | 1 +
>  2 files changed, 10 insertions(+)
>
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index ee84554854..cb5ea40236 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -195,6 +195,15 @@ static void usage(void)
>
>  __thread CPUState *thread_cpu;
>
> +void stop_all_tasks(void)
> +{
> +/*
> + * We trust when using NPTL (pthreads) start_exclusive() handles thread
> + * stopping correctly.
> + */
> +start_exclusive();
> +}
> +
>  bool qemu_cpu_is_self(CPUState *cpu)
>  {
>  return thread_cpu == cpu;
> diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
> index c1170f14d9..cdb85140f4 100644
> --- a/bsd-user/qemu.h
> +++ b/bsd-user/qemu.h
> @@ -103,6 +103,7 @@ typedef struct TaskState {
>  } __attribute__((aligned(16))) TaskState;
>
>  void init_task_state(TaskState *ts);
> +void stop_all_tasks(void);
>  extern const char *qemu_uname_release;
>
>  /*
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH v2 14/15] bsd-user: Rename sigqueue to qemu_sigqueue

[Kyle Evans]

On Fri, Oct 8, 2021 at 6:15 PM Warner Losh  wrote:
>
> To avoid a name clash with FreeBSD's sigqueue data structure in
> signalvar.h, rename sigqueue to qemu_sigqueue. This sturcture

s/sturcture/structure/

> is currently defined, but unused.
>
> Signed-off-by: Warner Losh 
> Reviewed-by: Richard Henderson 
> Reviewed-by: Philippe Mathieu-Daudé 
> ---
>  bsd-user/qemu.h | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
> index e65e41d53d..5b815c3a23 100644
> --- a/bsd-user/qemu.h
> +++ b/bsd-user/qemu.h
> @@ -73,15 +73,15 @@ struct image_info {
>
>  #define MAX_SIGQUEUE_SIZE 1024
>
> -struct sigqueue {
> -struct sigqueue *next;
> +struct qemu_sigqueue {
> +struct qemu_sigqueue *next;
> +target_siginfo_t info;
>  };
>
>  struct emulated_sigtable {
>  int pending; /* true if signal is pending */
> -struct sigqueue *first;
> -/* in order to always have memory for the first signal, we put it here */
> -struct sigqueue info;
> +struct qemu_sigqueue *first;
> +struct qemu_sigqueue info; /* Put first signal info here */
>  };
>
>  /*
> @@ -95,8 +95,8 @@ typedef struct TaskState {
>  struct image_info *info;
>
>  struct emulated_sigtable sigtab[TARGET_NSIG];
> -struct sigqueue sigqueue_table[MAX_SIGQUEUE_SIZE]; /* siginfo queue */
> -struct sigqueue *first_free; /* first free siginfo queue entry */
> +struct qemu_sigqueue sigqueue_table[MAX_SIGQUEUE_SIZE]; /* siginfo queue 
> */
> +struct qemu_sigqueue *first_free; /* first free siginfo queue entry */
>  int signal_pending; /* non zero if a signal may be pending */
>
>  uint8_t stack[];
> --
> 2.32.0
>

Typo noted above in the commit message; otherwise:

Reviewed-by: Kyle Evans 

Re: [PATCH v2 09/15] bsd-user/target_os_elf: If ELF_HWCAP2 is defined, publish it

[Kyle Evans]

On Fri, Oct 8, 2021 at 6:15 PM Warner Losh  wrote:
>
> Some architectures publish AT_HWCAP2 as well as AT_HWCAP. Those
> architectures will define ELF_HWCAP2 in their target_arch_elf.h files
> for the value for this process. If it is defined, then publish it.
>
> Signed-off-by: Warner Losh 
> Reviewed-by: Richard Henderson 
> Reviewed-by: Philippe Mathieu-Daudé 
> ---
>  bsd-user/freebsd/target_os_elf.h | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/bsd-user/freebsd/target_os_elf.h 
> b/bsd-user/freebsd/target_os_elf.h
> index adcffd1ddb..e5ac8e8e50 100644
> --- a/bsd-user/freebsd/target_os_elf.h
> +++ b/bsd-user/freebsd/target_os_elf.h
> @@ -112,6 +112,10 @@ static abi_ulong target_create_elf_tables(abi_ulong p, 
> int argc, int envc,
>  NEW_AUX_ENT(AT_ENTRY, load_bias + exec->e_entry);
>  features = ELF_HWCAP;
>  NEW_AUX_ENT(FREEBSD_AT_HWCAP, features);
> +#ifdef ELF_HWCAP2
> +features = ELF_HWCAP2;
> +NEW_AUX_ENT(FREEBSD_AT_HWCAP2, features);
> +#endif
>  NEW_AUX_ENT(AT_UID, (abi_ulong)getuid());
>  NEW_AUX_ENT(AT_EUID, (abi_ulong)geteuid());
>  NEW_AUX_ENT(AT_GID, (abi_ulong)getgid());
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH v2 12/15] bsd-user/sysarch: Move to using do_freebsd_arch_sysarch interface

[Kyle Evans]

On Fri, Oct 8, 2021 at 6:15 PM Warner Losh  wrote:
>
> do_freebsd_arch_sysarch() exists in $ARCH/target_arch_sysarch.h for x86.
> Call it from do_freebsd_sysarch() and remove the mostly duplicate
> version in syscall.c. Future changes will move it to os-sys.c and
> support other architectures.
>
> Signed-off-by: Warner Losh 
> Reviewed-by: Richard Henderson 
> ---
>  bsd-user/syscall.c | 45 +
>  1 file changed, 1 insertion(+), 44 deletions(-)
>
> diff --git a/bsd-user/syscall.c b/bsd-user/syscall.c
> index 2fd2ba8330..d3b9f431e2 100644
> --- a/bsd-user/syscall.c
> +++ b/bsd-user/syscall.c
> @@ -91,50 +91,7 @@ static abi_long do_obreak(abi_ulong new_brk)
>  #if defined(TARGET_I386)
>  static abi_long do_freebsd_sysarch(CPUX86State *env, int op, abi_ulong parms)
>  {
> -abi_long ret = 0;
> -abi_ulong val;
> -int idx;
> -
> -switch (op) {
> -#ifdef TARGET_ABI32
> -case TARGET_FREEBSD_I386_SET_GSBASE:
> -case TARGET_FREEBSD_I386_SET_FSBASE:
> -if (op == TARGET_FREEBSD_I386_SET_GSBASE)
> -#else
> -case TARGET_FREEBSD_AMD64_SET_GSBASE:
> -case TARGET_FREEBSD_AMD64_SET_FSBASE:
> -if (op == TARGET_FREEBSD_AMD64_SET_GSBASE)
> -#endif
> -idx = R_GS;
> -else
> -idx = R_FS;
> -if (get_user(val, parms, abi_ulong))
> -return -TARGET_EFAULT;
> -cpu_x86_load_seg(env, idx, 0);
> -env->segs[idx].base = val;
> -break;
> -#ifdef TARGET_ABI32
> -case TARGET_FREEBSD_I386_GET_GSBASE:
> -case TARGET_FREEBSD_I386_GET_FSBASE:
> -if (op == TARGET_FREEBSD_I386_GET_GSBASE)
> -#else
> -case TARGET_FREEBSD_AMD64_GET_GSBASE:
> -case TARGET_FREEBSD_AMD64_GET_FSBASE:
> -if (op == TARGET_FREEBSD_AMD64_GET_GSBASE)
> -#endif
> -idx = R_GS;
> -else
> -idx = R_FS;
> -val = env->segs[idx].base;
> -if (put_user(val, parms, abi_ulong))
> -return -TARGET_EFAULT;
> -break;
> -/* XXX handle the others... */
> -    default:
> -ret = -TARGET_EINVAL;
> -break;
> -}
> -return ret;
> +do_freebsd_arch_sysarch(env, op, parms);
>  }
>  #endif
>
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH v2 06/15] bsd-user/errno_defs.h: Add internal error numbers

[Kyle Evans]

On Fri, Oct 8, 2021 at 6:15 PM Warner Losh  wrote:
>
> From: Stacey Son 
>
> To emulate signals and interrupted system calls, we need to have the
> same mechanisms we have in the kernel, including these errno values.
>
> Signed-off-by: Stacey Son 
> Signed-off-by: Warner Losh 
> Reviewed-by: Richard Henderson 
> ---
>  bsd-user/errno_defs.h | 14 +++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/bsd-user/errno_defs.h b/bsd-user/errno_defs.h
> index 1efa502a12..b538dd93da 100644
> --- a/bsd-user/errno_defs.h
> +++ b/bsd-user/errno_defs.h
> @@ -1,6 +1,3 @@
> -/*  $OpenBSD: errno.h,v 1.20 2007/09/03 14:37:52 millert Exp $  */
> -/*  $NetBSD: errno.h,v 1.10 1996/01/20 01:33:53 jtc Exp $   */
> -
>  /*
>   * Copyright (c) 1982, 1986, 1989, 1993
>   *  The Regents of the University of California.  All rights reserved.
> @@ -37,6 +34,9 @@
>   *  @(#)errno.h 8.5 (Berkeley) 1/21/94
>   */
>
> +#ifndef _ERRNO_DEFS_H_
> +#define _ERRNO_DEFS_H_
> +
>  #define TARGET_EPERM1   /* Operation not permitted */
>  #define TARGET_ENOENT   2   /* No such file or directory 
> */
>  #define TARGET_ESRCH3   /* No such process */
> @@ -147,3 +147,11 @@
>  #define TARGET_EIDRM89  /* Identifier removed */
>  #define TARGET_ENOMSG   90  /* No message of desired 
> type */
>  #define TARGET_ELAST90  /* Must be equal largest 
> errno */
> +
> +/* Internal errors: */
> +#define TARGET_EJUSTRETURN  254 /* Just return without
> +   modifing regs */
> +#define TARGET_ERESTART 255 /* Restart syscall */
> +#define TARGET_ERESTARTSYS  TARGET_ERESTART /* Linux compat */
> +
> +#endif /* !  _ERRNO_DEFS_H_ */
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH v2 08/15] bsd-user/target_os_elf.h: Remove fallback ELF_HWCAP and reorder

[Kyle Evans]

On Fri, Oct 8, 2021 at 6:15 PM Warner Losh  wrote:
>
> All architectures have a ELF_HWCAP, so remove the fallback ifdef.
> Place ELF_HWCAP in the same order as on native FreeBSD.
>
> Signed-off-by: Warner Losh 
> Reviewed-by: Richard Henderson 
> Reviewed-by: Philippe Mathieu-Daudé 
> ---
>  bsd-user/freebsd/target_os_elf.h | 8 ++--
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/bsd-user/freebsd/target_os_elf.h 
> b/bsd-user/freebsd/target_os_elf.h
> index 2d03a883aa..adcffd1ddb 100644
> --- a/bsd-user/freebsd/target_os_elf.h
> +++ b/bsd-user/freebsd/target_os_elf.h
> @@ -38,10 +38,6 @@
>  #define ELF_PLATFORM (NULL)
>  #endif
>
> -#ifndef ELF_HWCAP
> -#define ELF_HWCAP 0
> -#endif
> -
>  /* XXX Look at the other conflicting AT_* values. */
>  #define FREEBSD_AT_NCPUS 19
>  #define FREEBSD_AT_HWCAP 25
> @@ -114,12 +110,12 @@ static abi_ulong target_create_elf_tables(abi_ulong p, 
> int argc, int envc,
>  NEW_AUX_ENT(AT_FLAGS, (abi_ulong)0);
>  NEW_AUX_ENT(FREEBSD_AT_NCPUS, (abi_ulong)bsd_get_ncpu());
>  NEW_AUX_ENT(AT_ENTRY, load_bias + exec->e_entry);
> +features = ELF_HWCAP;
> +NEW_AUX_ENT(FREEBSD_AT_HWCAP, features);
>  NEW_AUX_ENT(AT_UID, (abi_ulong)getuid());
>  NEW_AUX_ENT(AT_EUID, (abi_ulong)geteuid());
>  NEW_AUX_ENT(AT_GID, (abi_ulong)getgid());
>  NEW_AUX_ENT(AT_EGID, (abi_ulong)getegid());
> -features = ELF_HWCAP;
> -NEW_AUX_ENT(FREEBSD_AT_HWCAP, features);
>  target_auxents = sp; /* Note where the aux entries are in the target 
> */
>  #ifdef ARCH_DLINFO
>  /*
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH v2 07/15] bsd-user: move TARGET_MC_GET_CLEAR_RET to target_os_signal.h

[Kyle Evans]

On Fri, Oct 8, 2021 at 6:15 PM Warner Losh  wrote:
>
> Move TARGET_MC_GET_CLEAR_RET to freebsd/target_os_signal.h since it's
> architecture agnostic on FreeBSD.
>
> Signed-off-by: Warner Losh 
> Reviewed-by: Richard Henderson 
> Reviewed-by: Philippe Mathieu-Daudé 
> ---
>  bsd-user/freebsd/target_os_signal.h  | 3 +++
>  bsd-user/i386/target_arch_signal.h   | 2 --
>  bsd-user/x86_64/target_arch_signal.h | 2 --
>  3 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/bsd-user/freebsd/target_os_signal.h 
> b/bsd-user/freebsd/target_os_signal.h
> index 3ed454e086..1a4c5faf19 100644
> --- a/bsd-user/freebsd/target_os_signal.h
> +++ b/bsd-user/freebsd/target_os_signal.h
> @@ -1,6 +1,9 @@
>  #ifndef _TARGET_OS_SIGNAL_H_
>  #define _TARGET_OS_SIGNAL_H_
>
> +/* FreeBSD's sys/ucontext.h defines this */
> +#define TARGET_MC_GET_CLEAR_RET 0x0001
> +
>  #include "target_os_siginfo.h"
>  #include "target_arch_signal.h"
>
> diff --git a/bsd-user/i386/target_arch_signal.h 
> b/bsd-user/i386/target_arch_signal.h
> index 9812c6b034..a90750d602 100644
> --- a/bsd-user/i386/target_arch_signal.h
> +++ b/bsd-user/i386/target_arch_signal.h
> @@ -27,8 +27,6 @@
>  #define TARGET_MINSIGSTKSZ  (512 * 4)   /* min sig stack size */
>  #define TARGET_SIGSTKSZ (MINSIGSTKSZ + 32768)   /* recommended size */
>
> -#define TARGET_MC_GET_CLEAR_RET 0x0001
> -
>  struct target_sigcontext {
>  /* to be added */
>  };
> diff --git a/bsd-user/x86_64/target_arch_signal.h 
> b/bsd-user/x86_64/target_arch_signal.h
> index 4c1ff0e5ba..4bb753b08b 100644
> --- a/bsd-user/x86_64/target_arch_signal.h
> +++ b/bsd-user/x86_64/target_arch_signal.h
> @@ -27,8 +27,6 @@
>  #define TARGET_MINSIGSTKSZ  (512 * 4)   /* min sig stack size */
>  #define TARGET_SIGSTKSZ (MINSIGSTKSZ + 32768)   /* recommended size */
>
> -#define TARGET_MC_GET_CLEAR_RET 0x0001
> -
>  struct target_sigcontext {
>  /* to be added */
>  };
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH v2 05/15] bsd-user: export get_errno and is_error from syscall.c

[Kyle Evans]

On Fri, Oct 8, 2021 at 6:15 PM Warner Losh  wrote:
>
> Make get_errno and is_error global so files other than syscall.c can use
> them.
>
> Signed-off-by: Warner Losh 
> Reviewed-by: Richard Henderson 
> Reviewed-by: Philippe Mathieu-Daudé 
> ---
>  bsd-user/qemu.h|  4 
>  bsd-user/syscall.c | 10 +-
>  2 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
> index 522d6c4031..3b8475394c 100644
> --- a/bsd-user/qemu.h
> +++ b/bsd-user/qemu.h
> @@ -235,6 +235,10 @@ extern unsigned long target_dflssiz;
>  extern unsigned long target_maxssiz;
>  extern unsigned long target_sgrowsiz;
>
> +/* syscall.c */
> +abi_long get_errno(abi_long ret);
> +bool is_error(abi_long ret);
> +
>  /* user access */
>
>  #define VERIFY_READ  PAGE_READ
> diff --git a/bsd-user/syscall.c b/bsd-user/syscall.c
> index 372836d44d..2fd2ba8330 100644
> --- a/bsd-user/syscall.c
> +++ b/bsd-user/syscall.c
> @@ -33,18 +33,18 @@
>  static abi_ulong target_brk;
>  static abi_ulong target_original_brk;
>
> -static inline abi_long get_errno(abi_long ret)
> +abi_long get_errno(abi_long ret)
>  {
> -if (ret == -1)
> +if (ret == -1) {
>  /* XXX need to translate host -> target errnos here */
>  return -(errno);
> -else
> -return ret;
> +}
> +return ret;
>  }
>
>  #define target_to_host_bitmask(x, tbl) (x)
>
> -static inline int is_error(abi_long ret)
> +bool is_error(abi_long ret)
>  {
>  return (abi_ulong)ret >= (abi_ulong)(-4096);
>  }
> --
> 2.32.0
>


Reviewed-by: Kyle Evans 

Re: [PATCH v2 04/15] bsd-user: TARGET_RESET define is unused, remove it

[Kyle Evans]

On Fri, Oct 8, 2021 at 6:15 PM Warner Losh  wrote:
>
> Signed-off-by: Warner Losh 
> Reviewed-by: Richard Henderson 
> Reviewed-by: Philippe Mathieu-Daudé 
> ---
>  bsd-user/i386/target_arch_cpu.h   | 2 --
>  bsd-user/x86_64/target_arch_cpu.h | 2 --
>  2 files changed, 4 deletions(-)
>
> diff --git a/bsd-user/i386/target_arch_cpu.h b/bsd-user/i386/target_arch_cpu.h
> index 978e8066af..b28602adbb 100644
> --- a/bsd-user/i386/target_arch_cpu.h
> +++ b/bsd-user/i386/target_arch_cpu.h
> @@ -23,8 +23,6 @@
>
>  #define TARGET_DEFAULT_CPU_MODEL "qemu32"
>
> -#define TARGET_CPU_RESET(cpu)
> -
>  static inline void target_cpu_init(CPUX86State *env,
>  struct target_pt_regs *regs)
>  {
> diff --git a/bsd-user/x86_64/target_arch_cpu.h 
> b/bsd-user/x86_64/target_arch_cpu.h
> index 5f5ee602f9..5172b230f0 100644
> --- a/bsd-user/x86_64/target_arch_cpu.h
> +++ b/bsd-user/x86_64/target_arch_cpu.h
> @@ -23,8 +23,6 @@
>
>  #define TARGET_DEFAULT_CPU_MODEL "qemu64"
>
> -#define TARGET_CPU_RESET(cpu)
> -
>  static inline void target_cpu_init(CPUX86State *env,
>  struct target_pt_regs *regs)
>  {
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH v2 02/15] bsd-user/target_os-user.h: Remove support for FreeBSD older than 12.0

[Kyle Evans]

nd;   /* Finishing address. */
>  uint64_t kve_offset;   /* Mapping offset in object */
> -#if defined(__FreeBSD_version) && __FreeBSD_version >= 90
>  uint64_t kve_vn_fileid;   /* inode number if vnode */
> -#if defined(__FreeBSD_version) && __FreeBSD_version >= 1200031
>  uint32_t kve_vn_fsid_freebsd11;  /* dev_t of vnode location */
> -#else
> -uint32_t kve_vn_fsid;   /* dev_t of vnode location */
> -#endif
> -#else /* !  __FreeBSD_version >= 90 */
> -uint64_t kve_fileid;   /* inode number if vnode */
> -uint32_t kve_fsid;   /* dev_t of vnode location */
> -#endif /* !  __FreeBSD_version >= 90 */
>  int32_t  kve_flags;   /* Flags on map entry. */
>  int32_t  kve_resident;   /* Number of resident pages. */
>  int32_t  kve_private_resident;  /* Number of private pages. */
>  int32_t  kve_protection;  /* Protection bitmask. */
>  int32_t  kve_ref_count;   /* VM obj ref count. */
>  int32_t  kve_shadow_count;  /* VM obj shadow count. */
> -#if defined(__FreeBSD_version) && __FreeBSD_version >= 90
>  int32_t  kve_vn_type;   /* Vnode type. */
>  uint64_t kve_vn_size;   /* File size. */
> -#if defined(__FreeBSD_version) && __FreeBSD_version >= 1200031
>  uint32_t kve_vn_rdev_freebsd11;  /* Device id if device. */
> -#else
> -uint32_t kve_vn_rdev;   /* Device id if device. */
> -#endif
>  uint16_t kve_vn_mode;   /* File mode. */
>  uint16_t kve_status;   /* Status flags. */
> -#if defined(__FreeBSD_version) && __FreeBSD_version >= 1200031
>  #if (__FreeBSD_version >= 1300501 && __FreeBSD_version < 140) ||\
>  __FreeBSD_version >= 149
>  union {
> @@ -413,13 +322,6 @@ struct target_kinfo_vmentry {
>  #endif
>  uint64_t kve_vn_rdev;   /* Device id if device. */
>  int  _kve_ispare[8];  /* Space for more stuff. */
> -#else
> -int32_t  _kve_ispare[12];  /* Space for more stuff. */
> -#endif
> -#else /* !  __FreeBSD_version >= 90 */
> -int  _kve_pad0;
> -int32_t  _kve_ispare[16];  /* Space for more stuff. */
> -#endif /* !  __FreeBSD_version >= 90 */
>  /* Truncated before copyout in sysctl */
>  char  kve_path[PATH_MAX];  /* Path to VM obj, if any. */
>  };
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH v2 03/15] bsd-user/strace.list: Remove support for FreeBSD versions older than 12.0

[Kyle Evans]

On Fri, Oct 8, 2021 at 6:15 PM Warner Losh  wrote:
>
> Signed-off-by: Warner Losh 
> Reviewed-by: Richard Henderson 
> ---
>  bsd-user/freebsd/strace.list | 11 ---
>  1 file changed, 11 deletions(-)
>
> diff --git a/bsd-user/freebsd/strace.list b/bsd-user/freebsd/strace.list
> index b01b5f36e8..275d2dbe27 100644
> --- a/bsd-user/freebsd/strace.list
> +++ b/bsd-user/freebsd/strace.list
> @@ -33,10 +33,6 @@
>  { TARGET_FREEBSD_NR___syscall, "__syscall", NULL, NULL, NULL },
>  { TARGET_FREEBSD_NR___sysctl, "__sysctl", NULL, print_sysctl, NULL },
>  { TARGET_FREEBSD_NR__umtx_op, "_umtx_op", "%s(%#x, %d, %d, %#x, %#x)", NULL, 
> NULL },
> -#if defined(__FreeBSD_version) && __FreeBSD_version < 100
> -{ TARGET_FREEBSD_NR__umtx_lock, "__umtx_lock", NULL, NULL, NULL },
> -{ TARGET_FREEBSD_NR__umtx_unlock, "__umtx_unlock", NULL, NULL, NULL },
> -#endif
>  { TARGET_FREEBSD_NR_accept, "accept", "%s(%d,%#x,%#x)", NULL, NULL },
>  { TARGET_FREEBSD_NR_accept4, "accept4", "%s(%d,%d,%#x,%#x)", NULL, NULL },
>  { TARGET_FREEBSD_NR_access, "access", "%s(\"%s\",%#o)", NULL, NULL },
> @@ -49,10 +45,6 @@
>  { TARGET_FREEBSD_NR_cap_fcntls_get, "cap_fcntls_get", NULL, NULL, NULL },
>  { TARGET_FREEBSD_NR_cap_fcntls_limit, "cap_fcntls_limit", NULL, NULL, NULL },
>  { TARGET_FREEBSD_NR_cap_getmode, "cap_getmode", NULL, NULL, NULL },
> -#if defined(__FreeBSD_version) && __FreeBSD_version < 100
> -{ TARGET_FREEBSD_NR_cap_getrights, "cap_getrights", NULL, NULL, NULL },
> -{ TARGET_FREEBSD_NR_cap_new, "cap_new", NULL, NULL, NULL },
> -#endif
>  { TARGET_FREEBSD_NR_cap_ioctls_get, "cap_ioctls_get", NULL, NULL, NULL },
>  { TARGET_FREEBSD_NR_cap_ioctls_limit, "cap_ioctls_limit", NULL, NULL, NULL },
>  { TARGET_FREEBSD_NR_cap_rights_limit, "cap_rights_limit", NULL, NULL, NULL },
> @@ -146,9 +138,6 @@
>  { TARGET_FREEBSD_NR_freebsd11_kevent, "freebsd11_kevent", NULL, NULL, NULL },
>  { TARGET_FREEBSD_NR_kevent, "kevent", NULL, NULL, NULL },
>  { TARGET_FREEBSD_NR_kill, "kill", NULL, NULL, NULL },
> -#if defined(__FreeBSD_version) && __FreeBSD_version < 100
> -{ TARGET_FREEBSD_NR_killpg, "killpg", NULL, NULL, NULL },
> -#endif
>  { TARGET_FREEBSD_NR_kqueue, "kqueue", NULL, NULL, NULL },
>  { TARGET_FREEBSD_NR_ktrace, "ktrace", NULL, NULL, NULL },
>  { TARGET_FREEBSD_NR_lchown, "lchown", NULL, NULL, NULL },
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH v3 4/9] bsd-user/mmap.c: mmap return ENOMEM on overflow

[Kyle Evans]

On Fri, Oct 8, 2021 at 4:27 PM Warner Losh  wrote:
>
> mmap should return ENOMEM on len overflow rather than EINVAL. Return
> EINVAL when len == 0 and ENOMEM when the rounded to a page length is 0.
> Found by make check-tcg.
>
> Signed-off-by: Warner Losh 
> Reviewed-by: Richard Henderson 
> Reviewed-by: Philippe Mathieu-Daudé 
> ---
>  bsd-user/mmap.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
> index 6f33aec58b..f0be3b12cf 100644
> --- a/bsd-user/mmap.c
> +++ b/bsd-user/mmap.c
> @@ -455,11 +455,18 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, 
> int prot,
>  goto fail;
>  }
>
> -len = TARGET_PAGE_ALIGN(len);
>  if (len == 0) {
>  errno = EINVAL;
>  goto fail;
>  }
> +
> +/* Check for overflows */
> +len = TARGET_PAGE_ALIGN(len);
> +if (len == 0) {
> +errno = ENOMEM;
> +goto fail;
> +}
> +
>  real_start = start & qemu_host_page_mask;
>  host_offset = offset & qemu_host_page_mask;
>
> --
> 2.32.0
>
>

Reviewed-by: Kyle Evans 

Re: [PATCH v3 3/9] bsd-user/mmap.c: MAP_ symbols are defined, so no need for ifdefs

[Kyle Evans]

On Fri, Oct 8, 2021 at 4:24 PM Warner Losh  wrote:
>
> All these MAP_ symbols are always defined on supported FreeBSD versions
> (12.2 and newer), so remove the #ifdefs since they aren't needed.
>
> Signed-off-by: Warner Losh 
> Reviewed-by: Philippe Mathieu-Daudé 
> Acked-by: Richard Henderson 
> ---
>  bsd-user/mmap.c | 14 --
>  1 file changed, 14 deletions(-)
>
> diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
> index 4f4fa3ab46..6f33aec58b 100644
> --- a/bsd-user/mmap.c
> +++ b/bsd-user/mmap.c
> @@ -286,13 +286,9 @@ static abi_ulong mmap_find_vma_aligned(abi_ulong start, 
> abi_ulong size,
>  wrapped = repeat = 0;
>  prev = 0;
>  flags = MAP_ANONYMOUS | MAP_PRIVATE;
> -#ifdef MAP_ALIGNED
>  if (alignment != 0) {
>  flags |= MAP_ALIGNED(alignment);
>  }
> -#else
> -/* XXX TODO */
> -#endif
>
>  for (;; prev = ptr) {
>  /*
> @@ -407,22 +403,18 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, 
> int prot,
>  printf("MAP_ALIGNED(%u) ", (flags & MAP_ALIGNMENT_MASK)
>  >> MAP_ALIGNMENT_SHIFT);
>  }
> -#if MAP_GUARD
>  if (flags & MAP_GUARD) {
>  printf("MAP_GUARD ");
>  }
> -#endif
>  if (flags & MAP_FIXED) {
>  printf("MAP_FIXED ");
>  }
>  if (flags & MAP_ANONYMOUS) {
>  printf("MAP_ANON ");
>  }
> -#ifdef MAP_EXCL
>  if (flags & MAP_EXCL) {
>  printf("MAP_EXCL ");
>  }
> -#endif
>  if (flags & MAP_PRIVATE) {
>  printf("MAP_PRIVATE ");
>  }
> @@ -432,11 +424,9 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int 
> prot,
>  if (flags & MAP_NOCORE) {
>  printf("MAP_NOCORE ");
>  }
> -#ifdef MAP_STACK
>  if (flags & MAP_STACK) {
>  printf("MAP_STACK ");
>  }
> -#endif
>  printf("fd=%d offset=0x%llx\n", fd, offset);
>  }
>  #endif
> @@ -445,7 +435,6 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int 
> prot,
>  errno = EINVAL;
>  goto fail;
>  }
> -#ifdef MAP_STACK
>  if (flags & MAP_STACK) {
>  if ((fd != -1) || ((prot & (PROT_READ | PROT_WRITE)) !=
>  (PROT_READ | PROT_WRITE))) {
> @@ -453,8 +442,6 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int 
> prot,
>  goto fail;
>  }
>  }
> -#endif /* MAP_STACK */
> -#ifdef MAP_GUARD
>  if ((flags & MAP_GUARD) && (prot != PROT_NONE || fd != -1 ||
>  offset != 0 || (flags & (MAP_SHARED | MAP_PRIVATE |
>  /* MAP_PREFAULT | */ /* MAP_PREFAULT not in mman.h */
> @@ -462,7 +449,6 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int 
> prot,
>  errno = EINVAL;
>  goto fail;
>  }
> -#endif
>
>  if (offset & ~TARGET_PAGE_MASK) {
>  errno = EINVAL;
> --
> 2.32.0
>
>

Reviewed-by: Kyle Evans 

Re: [PATCH v3 9/9] bsd-user/mmap.c: assert that target_mprotect cannot fail

[Kyle Evans]

On Fri, Oct 8, 2021 at 4:31 PM Warner Losh  wrote:
>
> Similar to the equivalent linux-user change 86abac06c14. All error
> conditions that target_mprotect checks are also checked by target_mmap.
> EACCESS cannot happen because we are just removing PROT_WRITE.  ENOMEM
> should not happen because we are modifying a whole VMA (and we have
> bigger problems anyway if it happens).
>
> Fixes a Coverity false positive, where Coverity complains about
> target_mprotect's return value being passed to tb_invalidate_phys_range.
>
> Signed-off-by: Mikaël Urankar 
> Signed-off-by: Warner Losh 
> Reviewed-by: Richard Henderson 
> Reviewed-by: Philippe Mathieu-Daudé 
> ---
>  bsd-user/mmap.c | 5 +
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
> index 066d9c10ff..4586ad27d0 100644
> --- a/bsd-user/mmap.c
> +++ b/bsd-user/mmap.c
> @@ -604,10 +604,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int 
> prot,
>  }
>  if (!(prot & PROT_WRITE)) {
>  ret = target_mprotect(start, len, prot);
> -if (ret != 0) {
> -start = ret;
> -goto the_end;
> -}
> +assert(ret == 0);
>  }
>  goto the_end;
>  }
> --
> 2.32.0
>
>

Reviewed-by: Kyle Evans 

Re: [PATCH v3 5/9] bsd-user/mmap.c: mmap prefer MAP_ANON for BSD

[Kyle Evans]

On Fri, Oct 8, 2021 at 4:24 PM Warner Losh  wrote:
>
> MAP_ANON and MAP_ANONYMOUS are identical. Prefer MAP_ANON for BSD since
> the file is now a confusing mix of the two.
>
> Signed-off-by: Warner Losh 
> Reviewed-by: Philippe Mathieu-Daudé 
> Reviewed-by: Richard Henderson 
> ---
>  bsd-user/mmap.c | 11 +--
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
> index f0be3b12cf..301108ed25 100644
> --- a/bsd-user/mmap.c
> +++ b/bsd-user/mmap.c
> @@ -285,7 +285,7 @@ static abi_ulong mmap_find_vma_aligned(abi_ulong start, 
> abi_ulong size,
>  addr = start;
>  wrapped = repeat = 0;
>  prev = 0;
> -flags = MAP_ANONYMOUS | MAP_PRIVATE;
> +flags = MAP_ANON | MAP_PRIVATE;
>  if (alignment != 0) {
>  flags |= MAP_ALIGNED(alignment);
>  }
> @@ -409,7 +409,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int 
> prot,
>  if (flags & MAP_FIXED) {
>  printf("MAP_FIXED ");
>  }
> -if (flags & MAP_ANONYMOUS) {
> +if (flags & MAP_ANON) {
>  printf("MAP_ANON ");
>  }
>  if (flags & MAP_EXCL) {
> @@ -431,7 +431,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int 
> prot,
>  }
>  #endif
>
> -if ((flags & MAP_ANONYMOUS) && fd != -1) {
> +if ((flags & MAP_ANON) && fd != -1) {
>  errno = EINVAL;
>  goto fail;
>  }
> @@ -533,7 +533,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int 
> prot,
>   * qemu_real_host_page_size
>   */
>  p = mmap(g2h_untagged(start), host_len, prot,
> - flags | MAP_FIXED | ((fd != -1) ? MAP_ANONYMOUS : 0), -1, 
> 0);
> + flags | MAP_FIXED | ((fd != -1) ? MAP_ANON : 0), -1, 0);
>  if (p == MAP_FAILED)
>  goto fail;
>  /* update start so that it points to the file position at 'offset' */
> @@ -696,8 +696,7 @@ static void mmap_reserve(abi_ulong start, abi_ulong size)
>  }
>  if (real_start != real_end) {
>  mmap(g2h_untagged(real_start), real_end - real_start, PROT_NONE,
> - MAP_FIXED | MAP_ANONYMOUS | MAP_PRIVATE,
> - -1, 0);
> + MAP_FIXED | MAP_ANON | MAP_PRIVATE, -1, 0);
>  }
>  }
>
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH v3 2/9] bsd-user/mmap.c: check pread's return value to fix warnings with _FORTIFY_SOURCE

[Kyle Evans]

On Fri, Oct 8, 2021 at 4:24 PM Warner Losh  wrote:
>
> From: Mikaël Urankar 
>
> Simmilar to the equivalent linux-user: commit fb7e378cf9c, which added
> checking to pread's return value. Update to current qemu standards with
> {} around the if statement.
>
> Signed-off-by: Mikaël Urankar 
> Signed-off-by: Warner Losh 
> Reviewed-by: Richard Henderson 
> Reviewed-by: Philippe Mathieu-Daudé 
> ---
>  bsd-user/mmap.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
> index fc3c1480f5..4f4fa3ab46 100644
> --- a/bsd-user/mmap.c
> +++ b/bsd-user/mmap.c
> @@ -174,7 +174,9 @@ static int mmap_frag(abi_ulong real_start,
>  mprotect(host_start, qemu_host_page_size, prot1 | PROT_WRITE);
>
>  /* read the corresponding file data */
> -pread(fd, g2h_untagged(start), end - start, offset);
> +if (pread(fd, g2h_untagged(start), end - start, offset) == -1) {
> +return -1;
> +}
>
>  /* put final protection */
>  if (prot_new != (prot1 | PROT_WRITE))
> @@ -593,7 +595,9 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int 
> prot,
>-1, 0);
>  if (retaddr == -1)
>  goto fail;
> -pread(fd, g2h_untagged(start), len, offset);
> +if (pread(fd, g2h_untagged(start), len, offset) == -1) {
> +goto fail;
> +}
>  if (!(prot & PROT_WRITE)) {
>      ret = target_mprotect(start, len, prot);
>  if (ret != 0) {
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH v3 1/9] bsd-user/mmap.c: Always zero MAP_ANONYMOUS memory in mmap_frag()

[Kyle Evans]

On Fri, Oct 8, 2021 at 4:24 PM Warner Losh  wrote:
>
> From: Mikaël Urankar 
>
> Similar to the equivalent linux-user commit e6deac9cf99
>
> When mapping MAP_ANONYMOUS memory fragments, still need notice about to
> set it zero, or it will cause issues.
>
> Signed-off-by: Mikaël Urankar 
> Signed-off-by: Warner Losh 
> Reviewed-by: Richard Henderson 
> ---
>  bsd-user/mmap.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/bsd-user/mmap.c b/bsd-user/mmap.c
> index b40ab9045f..fc3c1480f5 100644
> --- a/bsd-user/mmap.c
> +++ b/bsd-user/mmap.c
> @@ -180,10 +180,12 @@ static int mmap_frag(abi_ulong real_start,
>  if (prot_new != (prot1 | PROT_WRITE))
>  mprotect(host_start, qemu_host_page_size, prot_new);
>  } else {
> -/* just update the protection */
>  if (prot_new != prot1) {
>  mprotect(host_start, qemu_host_page_size, prot_new);
>  }
> +if (prot_new & PROT_WRITE) {
> +memset(g2h_untagged(start), 0, end - start);
> +}
>  }
>  return 0;
>  }
> --
> 2.32.0
>

Reviewed-by: Kyle Evans 

Re: [PATCH 14/14] bsd-user/signal: Create a dummy signal queueing function

[Kyle Evans]

On Fri, Sep 24, 2021 at 3:11 PM Warner Losh  wrote:
>
>
>
> On Fri, Sep 24, 2021 at 6:00 AM Richard Henderson 
>  wrote:
>>
>> On 9/21/21 11:14 PM, Warner Losh wrote:
>> > Create dummy signal queueing function so we can start to integrate other
>> > architectures (at the cost of signals remaining broken) to tame the
>> > dependency graph a bit and to bring in signals in a more controlled
>> > fashion.
>> >
>> > Signed-off-by: Warner Losh 
>> > ---
>> >   bsd-user/qemu.h   | 1 +
>> >   bsd-user/signal.c | 8 
>> >   2 files changed, 9 insertions(+)
>> >
>> > diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
>> > index 5a2fd87e44..85d1f8fd2a 100644
>> > --- a/bsd-user/qemu.h
>> > +++ b/bsd-user/qemu.h
>> > @@ -209,6 +209,7 @@ void process_pending_signals(CPUArchState *cpu_env);
>> >   void signal_init(void);
>> >   long do_sigreturn(CPUArchState *env);
>> >   long do_rt_sigreturn(CPUArchState *env);
>> > +int queue_signal(CPUArchState *env, int sig, target_siginfo_t *info);
>> >   abi_long do_sigaltstack(abi_ulong uss_addr, abi_ulong uoss_addr, 
>> > abi_ulong sp);
>> >
>> >   /* mmap.c */
>> > diff --git a/bsd-user/signal.c b/bsd-user/signal.c
>> > index ad6d935569..4e7f618944 100644
>> > --- a/bsd-user/signal.c
>> > +++ b/bsd-user/signal.c
>> > @@ -19,6 +19,14 @@
>> >   #include "qemu/osdep.h"
>> >
>> >   #include "qemu.h"
>> > +/*
>>
>> Whacky whitespace.
>
>
> fixed.
>
>>
>> > + * Queue a signal so that it will be send to the virtual CPU as soon as
>> > + * possible.
>> > + */
>> > +int queue_signal(CPUArchState *env, int sig, target_siginfo_t *info)
>> > +{
>> > +return 1;
>> > +}
>>
>> Both here and in linux-user, there are no error conditions.  We should 
>> change the return
>> to void.
>
>
> I'll prep a patch to follow up for both linux and bsd user.
>
>>
>> Also, consider folding in the signal-common.h cleanup soon.
>> But don't let either hold you up too much with rebasing.
>
>
> It's on my list. This 'dummy' routine is just to get things linking to
> help simplify the rather tangled dependency tree to get things
> in, still have them compile and still have at least simple hello
> world continue to work. Behind these reviews are three streams
> of patches for 3 more architectures: arm, aarch64 and riscv64.
>
> I'll create a patch for both linux-user and fix in bsd-user as part of the
> signal.c upstreaming I'm working on.
>
> It brings to mind something else... There's times it might be easier
> to refactor between bsd-user and linux-user rather than upstream
> something that's largely copied from linux-user. Is there a good
> way to do that and talk about the design before I sink a ton of time
> into something that's the wrong direction?
>

I had a proposal on this list a long while back to refactor some stuff
into a top-level qemu-user that could be shared between the two,
starting with safe_syscall (which syscall can be substantially
shared), but it hadn't received any traction at that time.

Thanks,

Kyle Evans

Re: [PATCH v3 26/43] bsd-user: *BSD specific siginfo defintions

[Kyle Evans]

On Thu, Sep 2, 2021 at 6:55 PM  wrote:
>
> From: Warner Losh 
>
> Add FreeBSD, NetBSD and OpenBSD values for the various signal info types
> and defines to decode different signals to discover more information
> about the specific signal types.
>
> Signed-off-by: Stacey Son 
> Signed-off-by: Warner Losh 
> Acked-by: Richard Henderson 
> ---
>  bsd-user/freebsd/target_os_siginfo.h | 145 +++
>  bsd-user/freebsd/target_os_signal.h  |  78 ++
>  bsd-user/i386/target_arch_signal.h   |  94 +
>  bsd-user/netbsd/target_os_siginfo.h  |  82 +++
>  bsd-user/netbsd/target_os_signal.h   |  69 +
>  bsd-user/openbsd/target_os_siginfo.h |  82 +++
>  bsd-user/openbsd/target_os_signal.h  |  69 +
>  bsd-user/qemu.h  |   1 +
>  bsd-user/syscall_defs.h  |  10 --
>  bsd-user/x86_64/target_arch_signal.h |  94 +
>  10 files changed, 714 insertions(+), 10 deletions(-)
>  create mode 100644 bsd-user/freebsd/target_os_siginfo.h
>  create mode 100644 bsd-user/freebsd/target_os_signal.h
>  create mode 100644 bsd-user/i386/target_arch_signal.h
>  create mode 100644 bsd-user/netbsd/target_os_siginfo.h
>  create mode 100644 bsd-user/netbsd/target_os_signal.h
>  create mode 100644 bsd-user/openbsd/target_os_siginfo.h
>  create mode 100644 bsd-user/openbsd/target_os_signal.h
>  create mode 100644 bsd-user/x86_64/target_arch_signal.h
>

Reviewed-by: Kyle Evans 

Re: [PATCH v3 30/43] bsd-user: elf cleanup

[Kyle Evans]

On Thu, Sep 2, 2021 at 6:56 PM  wrote:
>
> From: Warner Losh 
>
> Move OS-dependent defines into target_os_elf.h. Move the architectural
> dependent stuff into target_arch_elf.h. Adjust elfload.c to use
> target_create_elf_tables instead of create_elf_tables.
>
> Signed-off-by: Warner Losh 
> Signed-off-by: Stacey Son 
> Signed-off-by: Kyle Evans 
> Signed-off-by: Justin Hibbits 
> Signed-off-by: Alexander Kabaev 
> Acked-by: Richard Henderson 
> ---
>  bsd-user/elfload.c   | 191 ---
>  bsd-user/freebsd/target_os_elf.h | 137 ++
>  bsd-user/netbsd/target_os_elf.h  | 146 +++
>  bsd-user/openbsd/target_os_elf.h | 146 +++
>  bsd-user/qemu.h  |   1 +
>  5 files changed, 454 insertions(+), 167 deletions(-)
>  create mode 100644 bsd-user/freebsd/target_os_elf.h
>  create mode 100644 bsd-user/netbsd/target_os_elf.h
>  create mode 100644 bsd-user/openbsd/target_os_elf.h

Reviewed-by: Kyle Evans 

Re: [PATCH v3 18/43] bsd-user: save the path to the qemu emulator

[Kyle Evans]

On Thu, Sep 2, 2021 at 6:53 PM  wrote:
>
> From: Warner Losh 
>
> Save the path to the qemu emulator. This will be used later when we have
> a more complete implementation of exec.
>
> Signed-off-by: Stacey Son 
> Signed-off-by: Warner Losh 
> Acked-by: Richard Henderson 
> ---
>  bsd-user/main.c | 21 +
>  bsd-user/qemu.h |  1 +
>  2 files changed, 22 insertions(+)
>

Reviewed-by: Kyle Evans 

Re: [PATCH v3 21/43] bsd-user: pull in target_arch_thread.h update target_arch_elf.h

[Kyle Evans]

On Thu, Sep 2, 2021 at 6:55 PM  wrote:
>
> From: Warner Losh 
>
> Update target_arch_elf.h to remove thread_init. Move its contents to
> target_arch_thread.h and rename to target_thread_init(). Update
> elfload.c to call it. Create thread_os_thread.h to hold the os specific
> parts of the thread and threat manipulation routines. Currently, it just

s/threat/thread/

> includes target_arch_thread.h. target_arch_thread.h contains the at the
> moment unused target_thread_set_upcall which will be used in the future
> when creating actual thread (i386 has this stubbed, but other
> architectures in the bsd-user tree have real ones). FreeBSD doesn't do
> AT_HWCAP, so remove that code. Linux does, and this code came from there.
>
> These changes are all interrelated and could be brokend own, but seem to

s/brokend own/broken  down/

> represent a reviewable changeset since most of the change is boiler
> plate.
>
> Signed-off-by: Stacey Son 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/elfload.c   |  4 ++-
>  bsd-user/freebsd/target_os_thread.h  | 25 +
>  bsd-user/i386/target_arch_elf.h  | 52 ++--
>  bsd-user/i386/target_arch_thread.h   | 47 +
>  bsd-user/netbsd/target_os_thread.h   | 25 +
>  bsd-user/openbsd/target_os_thread.h  | 25 +
>  bsd-user/x86_64/target_arch_elf.h| 38 ++--
>  bsd-user/x86_64/target_arch_thread.h | 40 +
>  8 files changed, 171 insertions(+), 85 deletions(-)
>  create mode 100644 bsd-user/freebsd/target_os_thread.h
>  create mode 100644 bsd-user/i386/target_arch_thread.h
>  create mode 100644 bsd-user/netbsd/target_os_thread.h
>  create mode 100644 bsd-user/openbsd/target_os_thread.h
>  create mode 100644 bsd-user/x86_64/target_arch_thread.h
>

Minor message nits, but otherwise:

Reviewed-by: Kyle Evans 

Re: [PATCH v3 33/43] bsd-user: Make cpu_model and cpu_type visible to all of main.c

[Kyle Evans]

On Thu, Sep 2, 2021 at 6:53 PM  wrote:
>
> From: Warner Losh 
>
> cpu_model and cpu_type will be used future commits, so move them from
> main() scoped to file scoped.
>
> Signed-off-by: Warner Losh 
> Acked-by: Richard Henderson 
> ---
>  bsd-user/main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

I think we should reduce this one to just moving cpu_type. cpu_model
is really only used in main() to derive the appropriate cpu_type,
which we do use later.

>
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index 71fd9d5aba..50c8fdc1e2 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -54,6 +54,8 @@
>  int singlestep;
>  unsigned long mmap_min_addr;
>  uintptr_t guest_base;
> +static const char *cpu_model;
> +static const char *cpu_type;
>  bool have_guest_base;
>  unsigned long reserved_va;
>
> @@ -201,8 +203,6 @@ static void save_proc_pathname(char *argv0)
>  int main(int argc, char **argv)
>  {
>  const char *filename;
> -const char *cpu_model;
> -const char *cpu_type;
>  const char *log_file = NULL;
>  const char *log_mask = NULL;
>  const char *seed_optarg = NULL;
> --
> 2.32.0
>

Re: [PATCH v3 34/43] bsd-user: update debugging in mmap.c

[Kyle Evans]

On Thu, Sep 2, 2021 at 6:54 PM  wrote:
>
> From: Warner Losh 
>
> Update the debugging code for new features and different targets.
>
> Signed-off-by: Mikaël Urankar 
> Signed-off-by: Sean Bruno 
> Signed-off-by: Kyle Evans 
> Signed-off-by: Warner Losh 
> Acked-by: Richard Henderson 
> ---
>  bsd-user/mmap.c | 55 ++---
>  1 file changed, 38 insertions(+), 17 deletions(-)
>

Reviewed-by: Kyle Evans 

Re: [PATCH v3 32/43] bsd-user: Rewrite target system call definintion glue

[Kyle Evans]

On Thu, Sep 2, 2021 at 6:54 PM  wrote:
>
> From: Warner Losh 
>
> Rewrite target definnitions to interface with the FreeBSD system calls.
> This covers basic types (time_t, iovec, umtx_time, timespec, timeval,
> rusage, rwusage) and basic defines (mmap, rusage). Also included are
> FreeBSD version-specific variations.
>
> Signed-off-by: Stacey Son 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/bsd-mman.h | 121 
>  bsd-user/mmap.c |   2 -
>  bsd-user/syscall_defs.h | 247 ++--
>  3 files changed, 162 insertions(+), 208 deletions(-)
>  delete mode 100644 bsd-user/bsd-mman.h
>

Reviewed-by: Kyle Evans 

Re: [PATCH v3 39/43] bsd-user: Refactor load_elf_sections and is_target_elf_binary

[Kyle Evans]

On Thu, Sep 2, 2021 at 6:56 PM  wrote:
>
> From: Warner Losh 
>
> Factor out load_elf_sections and is_target_elf_binary out of
> load_elf_interp.
>
> Signed-off-by: Mikaël Urankar 
> Signed-off-by: Stacey Son 
> Signed-off-by: Warner Losh 
> ---
>  bsd-user/elfload.c | 344 +
>  1 file changed, 158 insertions(+), 186 deletions(-)
>

Reviewed-by: Kyle Evans 

Re: [PATCH v3 36/43] bsd-user: Add target_os_user.h to capture the user/kernel structures

[Kyle Evans]

On Thu, Sep 2, 2021 at 6:55 PM  wrote:
>
> From: Warner Losh 
>
> This file evolved over the years to capture the user/kernel interfaces,
> including those that changed over time.
>
> Signed-off-by: Stacey Son 
> Signed-off-by: Michal Meloun 
> Signed-off-by: Warner Losh 
> Acked-by: Richard Henderson 
> ---
>  bsd-user/freebsd/target_os_user.h | 427 ++
>  1 file changed, 427 insertions(+)
>  create mode 100644 bsd-user/freebsd/target_os_user.h
>

Reviewed-by: Kyle Evans 

Re: [PATCH v3 40/43] bsd-user: move qemu_log to later in the file

[Kyle Evans]

On Thu, Sep 2, 2021 at 6:56 PM  wrote:
>
> From: Warner Losh 
>
> Signed-off-by: Warner Losh 
> Acked-by: Richard Henderson 
> ---
>  bsd-user/main.c | 17 +
>  1 file changed, 9 insertions(+), 8 deletions(-)
>

Subject nit: s/qemu_log/gemu_log/

Reviewed-by: Kyle Evans 

Re: [PATCH v3 43/43] bsd-user: Update mapping to handle reserved and starting conditions

[Kyle Evans]

On Thu, Sep 2, 2021 at 6:56 PM  wrote:
>
> From: Warner Losh 
>
> Update the reserved base based on what platform we're on, as well as the
> start of the mmap range. Update routines that find va ranges to interact
> with the reserved ranges as well as properly align the mapping (this is
> especially important for targets whose page size does not match the
> host's). Loop where appropriate when the initial address space offered
> by mmap does not meet the contraints.
>
> This has 18e80c55bb6 from linux-user folded in to the upstream
> bsd-user code as well.
>
> Signed-off-by: Mikaël Urankar 
> Signed-off-by: Stacey Son 
> Signed-off-by: Warner Losh 
> Acked-by: Richard Henderson 
> ---
>  bsd-user/main.c |  43 -
>  bsd-user/mmap.c | 415 
>  bsd-user/qemu.h |   5 +-
>  3 files changed, 392 insertions(+), 71 deletions(-)
>

Reviewed-by: Kyle Evans 

Re: [PATCH for 6.2 12/49] bsd-user: implement path searching

[Kyle Evans]

On Sat, Aug 7, 2021 at 10:11 PM Richard Henderson
 wrote:
>
> On 8/7/21 11:42 AM, Warner Losh wrote:
> > +path = g_strdup(p);
> > +if (path == NULL) {
>
> Only returns null when the input is null, which you've already eliminated.
>
> > +static bool find_in_path(char *path, const char *filename, char *retpath,
> > + size_t rpsize)
> > +{
> > +const char *d;
> > +
> > +while ((d = strsep(, ":")) != NULL) {
> > +if (*d == '\0') {
> > +d = ".";
> > +}
> > +if (snprintf(retpath, rpsize, "%s/%s", d, filename) >= 
> > (int)rpsize) {
> > +continue;
> > +}
> > +if (is_there((const char *)retpath)) {
> > +return true;
> > +}
> > +}
> > +return false;
>
> Hmm.  Fixed size retpath buffer isn't ideal.
> Any reason not to use g_find_program_in_path?
>

g_find_program_in_path may work well here, as well...

> I note that we don't search the path at all in linux-user/.
>

IIRC imgact_binmisc will have the resolved path but preserve argv as
it should have been were it not emulated, so we have to re-evaluate
the PATH search here because we try to be faithful to the context.

Thanks,

Kyle Evans

Re: qemu bsd-user plans

[Kyle Evans]

On Fri, 8 Jan 2021 at 19:43, Warner Losh  wrote:
>
> The FreeBSD project has rewritten bsd-user. We've been working on this for 
> quite some time (the earliest commits date from 2013). Maybe a dozen people 
> have worked on this over time, and there's 3 or 4 active developers focused 
> on FreeBSD changes at the moment.
>
> For a while, we'd merge in upstream changes from qemu. This worked great for 
> us, but left us with a big backlog that was hard to upstream. Each of the 
> updates took some time, so we got a little behind.
>
> So, a few years ago, I spent several weeks converting the tangled merge mess 
> into a set of linear patches and started moving that forward. This was around 
> the time 4.0 was released. I only managed to get the rebase forward to 3.1 
> release at the time before I hit problems related to poor testing environment 
> making it hard to verify newer versions were still working. Plus, we found a 
> few bugs that took a while to resolve for a number of reasons. Now that they 
> are resolved, we're able to use qemu-bsd-user to build ~30k packages for arm, 
> and ~20k for different types of mips in FreeBSD "ports" system. We now have 
> great confidence that it's working well again.
>
> Now that those bugs are resolved, I started trying to forward-port the 
> two-year-old base and immediately found myself hitting a number of problems. 
> A big problem was that I was re-doing a lot of work that was due to 
> innoculous changes upstream that I wouldn't have to do if the bsd-user 
> changes were upstream. These changes get in the way of dealing with the more 
> substantial structural changes in qemu that have happened.
>
> There had been talk of doing a remove and replace update of bsd-user. This 
> talk was before I managed to rebase things as far forward as 3.1 even. This 
> appealed to me because we've accumulated about 150 patches to date, many 
> quite large, and curating them into a set of maybe 400 or 500 changes to 
> match the size and scope of most patches I've seen posted to qemu-devel 
> seemed overwhelming.
>
> However, it's been another year since that plan was hatched, and it's become 
> clear to me that plan won't end in success. The closest I've been able to get 
> is 3.1 when 4.1 was current (about 6 months behind). It's time for a new plan.
>

As one of the developers on the FreeBSD side, I agree with this. In
addition to the reasons cited, reintroducing it would really take a
lot more time and effort and I'm not convinced it would ever be
completed because, IMO, going that route should really entail
redesigning it from the ground-up based on an abstraction of
linux-user. I find that right now I'm playing a lot of catch-up
because we seem to be largely copied from linux-user without later
improvement; there is a really healthy amount of platform-independent
stuff that really should end up ultimately shared in a qemu-user or
something to that effect to mitigate duplication of effort.

> So, my new plan is to rebase what changes I can to the tip of master and 
> submit those for review. I'll work with the developers on the FreeBSD side to 
> ensure they are included in reviews in addition to the normal qemu-devel 
> list. This will allow us to pare down the deltas between our code and 
> upstream to allow us to make progress. The changes will be held to the 
> standard 'makes things better'. Given how broken bsd-user is today in qemu 
> upstream, at first that will a very easy standard to make.
>
> The first patch I'll submit will be changing MAINTAINERS to point to me, 
> since I'm acting as the point person in this effort. I'll then re-submit some 
> other changes that I've submitted in the past, but CC the FreeBSD folks that 
> are currently active (they were only CC'd to former developers who lack the 
> time to review).
>

Thanks for taking this on!


Kyle Evans

Re: safe_syscall design guidance

[Kyle Evans]

On Mon, Oct 28, 2019 at 7:08 PM Kyle Evans  wrote:
>
> Hi,
>
> We're working on improving bsd-user in a local tree and rebasing forward
> to get our work suitable for upstreaming. I'm porting the safe_syscall stuff
> over to bsd-user, and would like to get some design guidance as it may best
> be implemented with some refactoring of linux-user.
>
> Below is an example of the refactoring my initial approach takes. I'm
> omitting !x86_64 in this e-mail because it's all along the same lines and
> only including the part relevant to linux-user. Effectively, linux-user/host
> is moved to qemu-user/host along with ^/linux-user/safe-syscall.S.
>
> Some bits specific to FreeBSD, also likely other *BSD but I've not yet
> verified, are sprinkled throughout host/*/* parts; this is the main point I
> suspect may be objectionable. FreeBSD indicates syscall error differently
> than Linux, and *context bits are also different. Other OS-specific bits
> for other arch are similar to the diff below.
>
> A full version of this can be found in my tree, currently only available on
> GitHub: https://github.com/kevans91/qemu-bsd-user/tree/safe_syscall -- this
> is applied to our version, currently based on qemu 3.1.
>
> Thoughts?
>

We've settled on duplicating the linux-user side over to bsd-user for
now to make progress, and make another attempt to solicit design
feedback later when we've rebased the rest of our work forward to
modern qemu.

Thanks,

Kyle Evans

safe_syscall design guidance

[Kyle Evans]

Hi,

We're working on improving bsd-user in a local tree and rebasing forward
to get our work suitable for upstreaming. I'm porting the safe_syscall stuff
over to bsd-user, and would like to get some design guidance as it may best
be implemented with some refactoring of linux-user.

Below is an example of the refactoring my initial approach takes. I'm
omitting !x86_64 in this e-mail because it's all along the same lines and
only including the part relevant to linux-user. Effectively, linux-user/host
is moved to qemu-user/host along with ^/linux-user/safe-syscall.S.

Some bits specific to FreeBSD, also likely other *BSD but I've not yet
verified, are sprinkled throughout host/*/* parts; this is the main point I
suspect may be objectionable. FreeBSD indicates syscall error differently
than Linux, and *context bits are also different. Other OS-specific bits
for other arch are similar to the diff below.

A full version of this can be found in my tree, currently only available on
GitHub: https://github.com/kevans91/qemu-bsd-user/tree/safe_syscall -- this
is applied to our version, currently based on qemu 3.1.

Thoughts?

Thanks,

Kyle Evans
diff --git a/Makefile.target b/Makefile.target
index 61f28d3722..13b3b13706 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -113,7 +113,7 @@ obj-$(call notempty,$(TARGET_XML_FILES)) += gdbstub-xml.o
 ifdef CONFIG_LINUX_USER
 
 QEMU_CFLAGS+=-I$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR) \
- -I$(SRC_PATH)/linux-user/host/$(ARCH) \
+ -I$(SRC_PATH)/qemu-user/host/$(ARCH) \
  -I$(SRC_PATH)/linux-user
 
 obj-y += linux-user/
@@ -128,6 +128,7 @@ ifdef CONFIG_BSD_USER
 
 QEMU_CFLAGS+=-I$(SRC_PATH)/bsd-user/$(TARGET_ABI_DIR) \
  -I$(SRC_PATH)/bsd-user/$(HOST_VARIANT_DIR) \
+ -I$(SRC_PATH)/qemu-user/host/$(ARCH) \
  -I$(SRC_PATH)/bsd-user \
  -D_WANT_SEMUN
 obj-y += bsd-user/
diff --git a/linux-user/Makefile.objs b/linux-user/Makefile.objs
index 769b8d8336..6089fc3a06 100644
--- a/linux-user/Makefile.objs
+++ b/linux-user/Makefile.objs
@@ -1,6 +1,6 @@
 obj-y = main.o syscall.o strace.o mmap.o signal.o \
elfload.o linuxload.o uaccess.o uname.o \
-   safe-syscall.o $(TARGET_ABI_DIR)/signal.o \
+   ../qemu-user/safe-syscall.o $(TARGET_ABI_DIR)/signal.o \
 $(TARGET_ABI_DIR)/cpu_loop.o exit.o fd-trans.o
 
 obj-$(TARGET_HAS_BFLT) += flatload.o
diff --git a/linux-user/host/x86_64/hostdep.h b/qemu-user/host/x86_64/hostdep.h
similarity index 78%
rename from linux-user/host/x86_64/hostdep.h
rename to qemu-user/host/x86_64/hostdep.h
index a4fefb5114..7aa233a116 100644
--- a/linux-user/host/x86_64/hostdep.h
+++ b/qemu-user/host/x86_64/hostdep.h
@@ -21,11 +21,17 @@
 extern char safe_syscall_start[];
 extern char safe_syscall_end[];
 
+#ifndef __FreeBSD__
+#define DEFINE_PCREG(puc)  &((ucontext_t 
*)(puc))->uc_mcontext.gregs[REG_RIP]
+#else
+typedef __register_t greg_t;
+#define DEFINE_PCREG(puc)   &((ucontext_t *)(puc))->uc_mcontext.mc_rip
+#endif
+
 /* Adjust the signal context to rewind out of safe-syscall if we're in it */
 static inline void rewind_if_in_safe_syscall(void *puc)
 {
-ucontext_t *uc = puc;
-greg_t *pcreg = >uc_mcontext.gregs[REG_RIP];
+greg_t *pcreg = DEFINE_PCREG(puc);
 
 if (*pcreg > (uintptr_t)safe_syscall_start
 && *pcreg < (uintptr_t)safe_syscall_end) {
diff --git a/linux-user/host/x86_64/safe-syscall.inc.S 
b/qemu-user/host/x86_64/safe-syscall.inc.S
similarity index 98%
rename from linux-user/host/x86_64/safe-syscall.inc.S
rename to qemu-user/host/x86_64/safe-syscall.inc.S
index f36992daa3..2b52e122d4 100644
--- a/linux-user/host/x86_64/safe-syscall.inc.S
+++ b/qemu-user/host/x86_64/safe-syscall.inc.S
@@ -72,6 +72,11 @@ safe_syscall_start:
 syscall
 safe_syscall_end:
 /* code path for having successfully executed the syscall */
+#ifdef __FreeBSD__
+jnb 2f
+neg%rax
+2:
+#endif
 pop %rbp
 .cfi_remember_state
 .cfi_def_cfa_offset 8
diff --git a/linux-user/safe-syscall.S b/qemu-user/safe-syscall.S
similarity index 100%
rename from linux-user/safe-syscall.S
rename to qemu-user/safe-syscall.S