All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/12] 2023 Q1 bsd-user upstreaming: bugfixes and sysctl
@ 2023-02-14  0:27 Warner Losh
  2023-02-14  0:27 ` [PATCH v2 01/12] bsd-user: Don't truncate the return value from freebsd_syscall Warner Losh
                   ` (11 more replies)
  0 siblings, 12 replies; 28+ messages in thread
From: Warner Losh @ 2023-02-14  0:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Warner Losh, richard.henderson, Paolo Bonzini,
	kevans, f4bug, Thomas Huth

This group of patches gets the basic framework for sysctl upstreamed. There's a
lot more to translate far too many binary blobs the kernel publishes via
sysctls, but I'm leaving those out in the name of simplicity.

There's also a bug fix from Doug Rabson that fixes a long int confusion leading
to a trunctation of addresses (oops)

I've added a new command line arg -strict for debugging when you want to stop
dead in the tracks when there's something unimplemented (like system calls)
rather than trying one's best to cope. It will also let whoever is working on
upstreaming to get something working to run it this way and find the missing
bits more easily. sysctl happens to be the first unimplemented system call for a
dynamically linked hello world.

There's a fix for the -static option, since clang hates -no-pie and needs only
-fno-pie.

Finally, I'm changing how I'm upstreaming a little. I'm doing a little deeper
dives into our rather chaotic repo to find a couple of authors I might have
missed. From here on out, I'll be using the original author's name as the git
author. I'll also tag the co-authors better as well when there's multiple people
that did something (other than reformat and/or move code around). I've
discovered more code moved about than I'd previously known. This seems more in
line with standard practice. Also, I've reviewed all these changes, but I don't
know if I need to add Reviewed-by: or not, so I've done it for one or two and
will make it consistent before the pull request. git log suggests maintainers
are inconsistent about this (or I've not discovered the rules they follow).

check-patch gives some line lenght warnings, but should otherwise be OK. There's
several static functions that aren't used until the end of the patch
series... Not sure the best way to suppress the build warnings there (but since
they are just warnings...).

v2:
o Created various helper functions to make the code a little better
o split a few patches that I thought would be approved together but
  that generated commentary. It's easier to manage 1 per patch for
  those.
o Add/delete G_GNU_UNUSED to ensure all patches compile w/o warnings
o Fix 64-bit running 32-bit binary to get a LONG or ULONG. Add a
  bounce buffer for these so we don't overflow anything on the target
  and return all the elements of arrays.
o Fixed a number of nits noticed in the review.
o Add or improve comments to explain things there were questions on
  during the review.
o fix noted typos
o fix host != target page size differences
o Add pointers to FreeBSD source code, as appropriate
o fix locking (mostly unlocking) on error paths
o Note: -strict feedback not yet applied due to large numbers of changes
  from the rest. Next round.

Doug Rabson (1):
  bsd-user: Don't truncate the return value from freebsd_syscall

Juergen Lock (3):
  bsd-user: sysctl helper funtions: sysctl_name2oid and sysctl_oidfmt
  bsd-user: common routine do_freebsd_sysctl_oid for all sysctl variants
  bsd-user: Start translation of arch-specific sysctls

Kyle Evans (2):
  bsd-user: do_freebsd_sysctl helper for sysctl(2)
  bsd-user: implement sysctlbyname(2)

Stacey Son (2):
  bsd-user: Add sysarch syscall
  bsd-user: Helper routines oidfmt

Warner Losh (4):
  build: Don't specify -no-pie for --static user-mode programs
  bsd-user: various helper routines for sysctl
  bsd-user: Helper routines h2t_old_sysctl
  bsd-user: Add -strict

 bsd-user/freebsd/os-sys.c     | 582 ++++++++++++++++++++++++++++++++++
 bsd-user/freebsd/os-syscall.c |  21 +-
 bsd-user/main.c               |   5 +-
 bsd-user/qemu.h               |   6 +
 configure                     |   2 +-
 5 files changed, 613 insertions(+), 3 deletions(-)

-- 
2.39.1



^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2 01/12] bsd-user: Don't truncate the return value from freebsd_syscall
  2023-02-14  0:27 [PATCH v2 00/12] 2023 Q1 bsd-user upstreaming: bugfixes and sysctl Warner Losh
@ 2023-02-14  0:27 ` Warner Losh
  2023-02-14  0:27 ` [PATCH v2 02/12] build: Don't specify -no-pie for --static user-mode programs Warner Losh
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 28+ messages in thread
From: Warner Losh @ 2023-02-14  0:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Warner Losh, richard.henderson, Paolo Bonzini,
	kevans, f4bug, Thomas Huth, Doug Rabson

From: Doug Rabson <dfr@rabson.org>

System call return values on FreeBSD are in a register (which is spelled
abi_long in qemu). This was being assigned into an int variable which
causes problems for 64bit targets.

Resolves: https://github.com/qemu-bsd-user/qemu-bsd-user/issues/40
Signed-off-by: Doug Rabson <dfr@rabson.org>
Reviewed-by: Warner Losh <imp@bsdimp.com>
[ Edited commit message for upstreaming into qemu-project ]
Signed-off-by: Warner Losh <imp@bsdimp.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 bsd-user/freebsd/os-syscall.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 57996cad8ae..b4a663fc021 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -512,7 +512,7 @@ abi_long do_freebsd_syscall(void *cpu_env, int num, abi_long arg1,
                             abi_long arg8)
 {
     CPUState *cpu = env_cpu(cpu_env);
-    int ret;
+    abi_long ret;
 
     trace_guest_user_syscall(cpu, num, arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8);
     if (do_strace) {
-- 
2.39.1



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 02/12] build: Don't specify -no-pie for --static user-mode programs
  2023-02-14  0:27 [PATCH v2 00/12] 2023 Q1 bsd-user upstreaming: bugfixes and sysctl Warner Losh
  2023-02-14  0:27 ` [PATCH v2 01/12] bsd-user: Don't truncate the return value from freebsd_syscall Warner Losh
@ 2023-02-14  0:27 ` Warner Losh
  2023-02-14  0:27 ` [PATCH v2 03/12] bsd-user: Add sysarch syscall Warner Losh
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 28+ messages in thread
From: Warner Losh @ 2023-02-14  0:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Warner Losh, richard.henderson, Paolo Bonzini,
	kevans, f4bug, Thomas Huth

When building with clang, -no-pie gives a warning on every single build,
so remove it.

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

diff --git a/configure b/configure
index 64960c6000f..eb284ccf308 100755
--- a/configure
+++ b/configure
@@ -1313,7 +1313,7 @@ if test "$static" = "yes"; then
     error_exit "-static-pie not available due to missing toolchain support"
   else
     pie="no"
-    QEMU_CFLAGS="-fno-pie -no-pie $QEMU_CFLAGS"
+    QEMU_CFLAGS="-fno-pie $QEMU_CFLAGS"
   fi
 elif test "$pie" = "no"; then
   if compile_prog "-Werror -fno-pie" "-no-pie"; then
-- 
2.39.1



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 03/12] bsd-user: Add sysarch syscall
  2023-02-14  0:27 [PATCH v2 00/12] 2023 Q1 bsd-user upstreaming: bugfixes and sysctl Warner Losh
  2023-02-14  0:27 ` [PATCH v2 01/12] bsd-user: Don't truncate the return value from freebsd_syscall Warner Losh
  2023-02-14  0:27 ` [PATCH v2 02/12] build: Don't specify -no-pie for --static user-mode programs Warner Losh
@ 2023-02-14  0:27 ` Warner Losh
  2023-02-14  0:27 ` [PATCH v2 04/12] bsd-user: various helper routines for sysctl Warner Losh
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 28+ messages in thread
From: Warner Losh @ 2023-02-14  0:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Warner Losh, richard.henderson, Paolo Bonzini,
	kevans, f4bug, Thomas Huth, Stacey Son, Juergen Lock

From: Stacey Son <sson@FreeBSD.org>

Connect up the sysarch system call.

Signed-off-by: Juergen Lock <nox@jelal.kn-bremen.de>
Co-authored-by: Juergen Lock <nox@jelal.kn-bremen.de>
Signed-off-by: Stacey Son <sson@FreeBSD.org>
Reviewed-by: Warner Losh <imp@bsdimp.com>
Signed-off-by: Warner Losh <imp@bsdimp.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 bsd-user/freebsd/os-syscall.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index b4a663fc021..e00997a818c 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -491,6 +491,13 @@ static abi_long freebsd_syscall(void *cpu_env, int num, abi_long arg1,
         ret = do_bsd_undelete(arg1);
         break;
 
+        /*
+         * sys{ctl, arch, call}
+         */
+    case TARGET_FREEBSD_NR_sysarch: /* sysarch(2) */
+        ret = do_freebsd_sysarch(cpu_env, arg1, arg2);
+        break;
+
     default:
         qemu_log_mask(LOG_UNIMP, "Unsupported syscall: %d\n", num);
         ret = -TARGET_ENOSYS;
-- 
2.39.1



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 04/12] bsd-user: various helper routines for sysctl
  2023-02-14  0:27 [PATCH v2 00/12] 2023 Q1 bsd-user upstreaming: bugfixes and sysctl Warner Losh
                   ` (2 preceding siblings ...)
  2023-02-14  0:27 ` [PATCH v2 03/12] bsd-user: Add sysarch syscall Warner Losh
@ 2023-02-14  0:27 ` Warner Losh
  2023-02-14 20:52   ` Richard Henderson
  2023-02-14  0:27 ` [PATCH v2 05/12] bsd-user: Helper routines oidfmt Warner Losh
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 28+ messages in thread
From: Warner Losh @ 2023-02-14  0:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Warner Losh, richard.henderson, Paolo Bonzini,
	kevans, f4bug, Thomas Huth

cap_memory - Caps the memory to just below MAXINT
scale_to_target_pages - Account for difference in host / targe page size
h2t_long_sat - converts a int64_t to a int32_t, saturating at max / min values
h2t_ulong_sat - converts a uint64_t to a uint32_t, saturating at max value

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

diff --git a/bsd-user/freebsd/os-sys.c b/bsd-user/freebsd/os-sys.c
index 1676ec10f83..cfbc4148a5c 100644
--- a/bsd-user/freebsd/os-sys.c
+++ b/bsd-user/freebsd/os-sys.c
@@ -21,6 +21,106 @@
 #include "qemu.h"
 #include "target_arch_sysarch.h"
 
+#include <sys/sysctl.h>
+
+/*
+ * Length for the fixed length types.
+ * 0 means variable length for strings and structures
+ * Compare with sys/kern_sysctl.c ctl_size
+ * Note: Not all types appear to be used in-tree.
+ */
+static const int G_GNUC_UNUSED target_ctl_size[CTLTYPE+1] = {
+	[CTLTYPE_INT] = sizeof(abi_int),
+	[CTLTYPE_UINT] = sizeof(abi_uint),
+	[CTLTYPE_LONG] = sizeof(abi_long),
+	[CTLTYPE_ULONG] = sizeof(abi_ulong),
+	[CTLTYPE_S8] = sizeof(int8_t),
+	[CTLTYPE_S16] = sizeof(int16_t),
+	[CTLTYPE_S32] = sizeof(int32_t),
+	[CTLTYPE_S64] = sizeof(int64_t),
+	[CTLTYPE_U8] = sizeof(uint8_t),
+	[CTLTYPE_U16] = sizeof(uint16_t),
+	[CTLTYPE_U32] = sizeof(uint32_t),
+	[CTLTYPE_U64] = sizeof(uint64_t),
+};
+
+static const int G_GNUC_UNUSED host_ctl_size[CTLTYPE+1] = {
+	[CTLTYPE_INT] = sizeof(int),
+	[CTLTYPE_UINT] = sizeof(u_int),
+	[CTLTYPE_LONG] = sizeof(long),
+	[CTLTYPE_ULONG] = sizeof(u_long),
+	[CTLTYPE_S8] = sizeof(int8_t),
+	[CTLTYPE_S16] = sizeof(int16_t),
+	[CTLTYPE_S32] = sizeof(int32_t),
+	[CTLTYPE_S64] = sizeof(int64_t),
+	[CTLTYPE_U8] = sizeof(uint8_t),
+	[CTLTYPE_U16] = sizeof(uint16_t),
+	[CTLTYPE_U32] = sizeof(uint32_t),
+	[CTLTYPE_U64] = sizeof(uint64_t),
+};
+
+#ifdef TARGET_ABI32
+/*
+ * Limit the amount of available memory to be most of the 32-bit address
+ * space. 0x100c000 was arrived at through trial and error as a good
+ * definition of 'most'.
+ */
+static const abi_ulong target_max_mem = UINT32_MAX - 0x100c000 + 1;
+
+static abi_ulong G_GNUC_UNUSED cap_memory(uint64_t mem)
+{
+    if (((unsigned long)target_max_mem) < mem) {
+        mem = target_max_mem;
+    }
+
+    return mem;
+}
+#endif
+
+static unsigned long host_page_size;
+
+static abi_ulong G_GNUC_UNUSED scale_to_target_pages(uint64_t pages)
+{
+    if (host_page_size == 0) {
+        host_page_size = getpagesize();
+    }
+
+    pages = muldiv64(pages, host_page_size, TARGET_PAGE_SIZE);
+#ifdef TARGET_ABI32
+    abi_ulong maxpages = target_max_mem / (abi_ulong)TARGET_PAGE_SIZE;
+
+    if (((unsigned long)maxpages) < pages) {
+        pages = maxpages;
+    }
+#endif
+    return pages;
+}
+
+#ifdef TARGET_ABI32
+static abi_long G_GNUC_UNUSED h2t_long_sat(long l)
+{
+    if (l > INT32_MAX) {
+        l = INT32_MAX;
+    } else if (l < INT32_MIN) {
+        l = INT32_MIN;
+    }
+    return l;
+}
+
+static abi_ulong G_GNUC_UNUSED h2t_ulong_sat(u_long ul)
+{
+    if (ul > UINT32_MAX) {
+        ul = UINT32_MAX;
+    }
+    return ul;
+}
+#endif
+
+/*
+ * placeholder until bsd-user downstream upstreams this with its thread support
+ */
+#define bsd_get_ncpu() 1
+
 /* sysarch() is architecture dependent. */
 abi_long do_freebsd_sysarch(void *cpu_env, abi_long arg1, abi_long arg2)
 {
-- 
2.39.1



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 05/12] bsd-user: Helper routines oidfmt
  2023-02-14  0:27 [PATCH v2 00/12] 2023 Q1 bsd-user upstreaming: bugfixes and sysctl Warner Losh
                   ` (3 preceding siblings ...)
  2023-02-14  0:27 ` [PATCH v2 04/12] bsd-user: various helper routines for sysctl Warner Losh
@ 2023-02-14  0:27 ` Warner Losh
  2023-02-14 20:53   ` Richard Henderson
  2023-02-14  0:27 ` [PATCH v2 06/12] bsd-user: Helper routines h2t_old_sysctl Warner Losh
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 28+ messages in thread
From: Warner Losh @ 2023-02-14  0:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Warner Losh, richard.henderson, Paolo Bonzini,
	kevans, f4bug, Thomas Huth, Stacey Son, Sean Bruno, Juergen Lock,
	Raphael Kubo da Costa

From: Stacey Son <sson@FreeBSD.org>

oidfmt uses undocumented system call to get the type of the sysctl.

Co-Authored-by: Sean Bruno <sbruno@FreeBSD.org>
Signed-off-by: Sean Bruno <sbruno@FreeBSD.org>
Co-Authored-by: Juergen Lock <nox@jelal.kn-bremen.de>
Signed-off-by: Juergen Lock <nox@jelal.kn-bremen.de>
Co-Authored-by: Raphael Kubo da Costa <rakuco@FreeBSD.org>
Signed-off-by: Raphael Kubo da Costa <rakuco@FreeBSD.org>
Signed-off-by: Stacey Son <sson@FreeBSD.org>
Reviewed-by: Warner Losh <imp@bsdimp.com>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/freebsd/os-sys.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/bsd-user/freebsd/os-sys.c b/bsd-user/freebsd/os-sys.c
index cfbc4148a5c..1df53a3e53b 100644
--- a/bsd-user/freebsd/os-sys.c
+++ b/bsd-user/freebsd/os-sys.c
@@ -121,6 +121,38 @@ static abi_ulong G_GNUC_UNUSED h2t_ulong_sat(u_long ul)
  */
 #define bsd_get_ncpu() 1
 
+/*
+ * This uses the undocumented oidfmt interface to find the kind of a requested
+ * sysctl, see /sys/kern/kern_sysctl.c:sysctl_sysctl_oidfmt() (compare to
+ * src/sbin/sysctl/sysctl.c)
+ */
+static int G_GNUC_UNUSED 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] = CTL_SYSCTL;
+    qoid[1] = CTL_SYSCTL_OIDFMT;
+    memcpy(qoid + 2, oid, len * sizeof(int));
+
+    j = sizeof(buf);
+    i = sysctl(qoid, len + 2, buf, &j, 0, 0);
+    if (i) {
+        return i;
+    }
+
+    if (kind) {
+        *kind = *(uint32_t *)buf;
+    }
+
+    if (fmt) {
+        strcpy(fmt, (char *)(buf + sizeof(uint32_t)));
+    }
+    return 0;
+}
+
 /* sysarch() is architecture dependent. */
 abi_long do_freebsd_sysarch(void *cpu_env, abi_long arg1, abi_long arg2)
 {
-- 
2.39.1



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 06/12] bsd-user: Helper routines h2t_old_sysctl
  2023-02-14  0:27 [PATCH v2 00/12] 2023 Q1 bsd-user upstreaming: bugfixes and sysctl Warner Losh
                   ` (4 preceding siblings ...)
  2023-02-14  0:27 ` [PATCH v2 05/12] bsd-user: Helper routines oidfmt Warner Losh
@ 2023-02-14  0:27 ` Warner Losh
  2023-02-14 21:16   ` Richard Henderson
  2023-02-14  0:27 ` [PATCH v2 07/12] bsd-user: sysctl helper funtions: sysctl_name2oid and sysctl_oidfmt Warner Losh
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 28+ messages in thread
From: Warner Losh @ 2023-02-14  0:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Warner Losh, richard.henderson, Paolo Bonzini,
	kevans, f4bug, Thomas Huth, Sean Bruno, Juergen Lock,
	Raphael Kubo da Costa, Stacey Son

h2t_old_sysctl does the byte swapping in the data to return it to the
target for the 'well known' types.

Co-Authored-by: Sean Bruno <sbruno@FreeBSD.org>
Signed-off-by: Sean Bruno <sbruno@FreeBSD.org>
Co-Authored-by: Juergen Lock <nox@jelal.kn-bremen.de>
Signed-off-by: Juergen Lock <nox@jelal.kn-bremen.de>
Co-Authored-by: Raphael Kubo da Costa <rakuco@FreeBSD.org>
Signed-off-by: Raphael Kubo da Costa <rakuco@FreeBSD.org>
Co-Authored-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/freebsd/os-sys.c | 95 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 91 insertions(+), 4 deletions(-)

diff --git a/bsd-user/freebsd/os-sys.c b/bsd-user/freebsd/os-sys.c
index 1df53a3e53b..457e61f5b36 100644
--- a/bsd-user/freebsd/os-sys.c
+++ b/bsd-user/freebsd/os-sys.c
@@ -29,7 +29,7 @@
  * Compare with sys/kern_sysctl.c ctl_size
  * Note: Not all types appear to be used in-tree.
  */
-static const int G_GNUC_UNUSED target_ctl_size[CTLTYPE+1] = {
+static const int target_ctl_size[CTLTYPE+1] = {
 	[CTLTYPE_INT] = sizeof(abi_int),
 	[CTLTYPE_UINT] = sizeof(abi_uint),
 	[CTLTYPE_LONG] = sizeof(abi_long),
@@ -44,7 +44,7 @@ static const int G_GNUC_UNUSED target_ctl_size[CTLTYPE+1] = {
 	[CTLTYPE_U64] = sizeof(uint64_t),
 };
 
-static const int G_GNUC_UNUSED host_ctl_size[CTLTYPE+1] = {
+static const int host_ctl_size[CTLTYPE+1] = {
 	[CTLTYPE_INT] = sizeof(int),
 	[CTLTYPE_UINT] = sizeof(u_int),
 	[CTLTYPE_LONG] = sizeof(long),
@@ -97,7 +97,7 @@ static abi_ulong G_GNUC_UNUSED scale_to_target_pages(uint64_t pages)
 }
 
 #ifdef TARGET_ABI32
-static abi_long G_GNUC_UNUSED h2t_long_sat(long l)
+static abi_long h2t_long_sat(long l)
 {
     if (l > INT32_MAX) {
         l = INT32_MAX;
@@ -107,7 +107,7 @@ static abi_long G_GNUC_UNUSED h2t_long_sat(long l)
     return l;
 }
 
-static abi_ulong G_GNUC_UNUSED h2t_ulong_sat(u_long ul)
+static abi_ulong h2t_ulong_sat(u_long ul)
 {
     if (ul > UINT32_MAX) {
         ul = UINT32_MAX;
@@ -153,6 +153,93 @@ static int G_GNUC_UNUSED oidfmt(int *oid, int len, char *fmt, uint32_t *kind)
     return 0;
 }
 
+/*
+ * Convert the old value from host to target.
+ *
+ * For LONG and ULONG on ABI32, we need to 'down convert' the 8 byte quantities
+ * to 4 bytes. The caller setup a buffer in host memory to get this data from
+ * the kernel and pass it to us. We do the down conversion and adjust the length
+ * so the caller knows what to write as the returned length into the target when
+ * it copies the down converted values into the target.
+ *
+ * For normal integral types, we just need to byte swap. No size changes.
+ *
+ * For strings and node data, there's no conversion needed.
+ *
+ * For opaque data, per sysctl OID converts take care of it.
+ */
+static void G_GNUC_UNUSED h2t_old_sysctl(void *holdp, size_t *holdlen, uint32_t kind)
+{
+    size_t len;
+    int hlen, tlen;
+    uint8_t *hp, *tp;
+
+    /*
+     * Although rare, we can have arrays of sysctl. Both sysctl_old_ddb in
+     * kern_sysctl.c and show_var in sbin/sysctl/sysctl.c have code that loops
+     * this way.  *holdlen has been set by the kernel to the host's length.
+     * Only LONG and ULONG on ABI32 have different sizes: see below.
+     */
+    hp = (uint8_t *)holdp;
+    tp = hp;
+    len = 0;
+    hlen = host_ctl_size[kind & CTLTYPE];
+    tlen = target_ctl_size[kind & CTLTYPE];
+
+    /*
+     * hlen == 0 for CTLTYPE_STRING and CTLTYPE_NODE, which need no conversion
+     * as well as CTLTYPE_OPAQUE, which needs special converters.
+     */
+    if (hlen == 0) {
+        return;
+    }
+
+    while (len < *holdlen) {
+        if (hlen == tlen) {
+            switch (hlen) {
+            case 1:
+                /* Nothing needed: no byteswapping and assigning in place */
+                break;
+            case 2:
+                *(uint16_t *)tp = tswap16(*(uint16_t *)hp);
+                break;
+            case 4:
+                *(uint32_t *)tp = tswap32(*(uint32_t *)hp);
+                break;
+            case 8:
+                *(uint64_t *)tp = tswap64(*(uint64_t *)hp);
+                break;
+            }
+        }
+#ifdef TARGET_ABI32
+        else {
+            /*
+             * Saturating assignment for the only two types that differ between
+             * 32-bit and 64-bit machines. All other integral types have the
+             * same, fixed size and will be converted w/o loss of precision
+             * in the above switch.
+             */
+            switch (kind & CTLTYPE) {
+            case CTLTYPE_LONG:
+                *(abi_long *)tp = tswap32(h2t_long_sat(*(long *)hp));
+                break;
+            case CTLTYPE_ULONG:
+                *(abi_ulong *)tp = tswap32(h2t_ulong_sat(*(u_long *)hp));
+                break;
+            }
+        }
+#endif
+        tp += tlen;
+        hp += hlen;
+        len += hlen;
+    }
+#ifdef TARGET_ABI32
+    if (hlen != tlen) {
+        *holdlen = (*holdlen / hlen) * tlen;
+    }
+#endif
+}
+
 /* sysarch() is architecture dependent. */
 abi_long do_freebsd_sysarch(void *cpu_env, abi_long arg1, abi_long arg2)
 {
-- 
2.39.1



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 07/12] bsd-user: sysctl helper funtions: sysctl_name2oid and sysctl_oidfmt
  2023-02-14  0:27 [PATCH v2 00/12] 2023 Q1 bsd-user upstreaming: bugfixes and sysctl Warner Losh
                   ` (5 preceding siblings ...)
  2023-02-14  0:27 ` [PATCH v2 06/12] bsd-user: Helper routines h2t_old_sysctl Warner Losh
@ 2023-02-14  0:27 ` Warner Losh
  2023-02-14 21:18   ` Richard Henderson
  2023-02-14  0:27 ` [PATCH v2 08/12] bsd-user: common routine do_freebsd_sysctl_oid for all sysctl variants Warner Losh
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 28+ messages in thread
From: Warner Losh @ 2023-02-14  0:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Warner Losh, richard.henderson, Paolo Bonzini,
	kevans, f4bug, Thomas Huth, Juergen Lock

From: Juergen Lock <nox@jelal.kn-bremen.de>

Helper functions for sysctl implementations. sysctl_name2oid and
sysctl_oidfmt convert oids between host and targets

Signed-off-by: Juergen Lock <nox@jelal.kn-bremen.de>
Reviewed-by: Warner Losh <imp@bsdimp.com>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/freebsd/os-sys.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/bsd-user/freebsd/os-sys.c b/bsd-user/freebsd/os-sys.c
index 457e61f5b36..084d53c16f6 100644
--- a/bsd-user/freebsd/os-sys.c
+++ b/bsd-user/freebsd/os-sys.c
@@ -240,6 +240,24 @@ static void G_GNUC_UNUSED h2t_old_sysctl(void *holdp, size_t *holdlen, uint32_t
 #endif
 }
 
+/*
+ * Convert the undocmented name2oid sysctl data for the target.
+ */
+static inline void G_GNUC_UNUSED sysctl_name2oid(uint32_t *holdp, size_t holdlen)
+{
+    size_t i, num = holdlen / sizeof(uint32_t);
+
+    for (i = 0; i < num; i++) {
+        holdp[i] = tswap32(holdp[i]);
+    }
+}
+
+static inline void G_GNUC_UNUSED sysctl_oidfmt(uint32_t *holdp)
+{
+    /* byte swap the kind */
+    holdp[0] = tswap32(holdp[0]);
+}
+
 /* sysarch() is architecture dependent. */
 abi_long do_freebsd_sysarch(void *cpu_env, abi_long arg1, abi_long arg2)
 {
-- 
2.39.1



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 08/12] bsd-user: common routine do_freebsd_sysctl_oid for all sysctl variants
  2023-02-14  0:27 [PATCH v2 00/12] 2023 Q1 bsd-user upstreaming: bugfixes and sysctl Warner Losh
                   ` (6 preceding siblings ...)
  2023-02-14  0:27 ` [PATCH v2 07/12] bsd-user: sysctl helper funtions: sysctl_name2oid and sysctl_oidfmt Warner Losh
@ 2023-02-14  0:27 ` Warner Losh
  2023-02-14 21:21   ` Richard Henderson
  2023-02-14  0:27 ` [PATCH v2 09/12] bsd-user: Start translation of arch-specific sysctls Warner Losh
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 28+ messages in thread
From: Warner Losh @ 2023-02-14  0:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Warner Losh, richard.henderson, Paolo Bonzini,
	kevans, f4bug, Thomas Huth, Juergen Lock, Stacey Son

From: Juergen Lock <nox@jelal.kn-bremen.de>

do_freebsd_sysctl_oid filters out some of the binary and special sysctls
where host != target. None of the sysctls that have to be translated from
host to target are handled here.

Signed-off-by: Juergen Lock <nox@jelal.kn-bremen.de>
Co-Authored-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Stacey Son <sson@FreeBSD.org>
Co-Authored-by: Warner Losh <imp@bsdimp.com>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/freebsd/os-sys.c | 90 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 86 insertions(+), 4 deletions(-)

diff --git a/bsd-user/freebsd/os-sys.c b/bsd-user/freebsd/os-sys.c
index 084d53c16f6..e847404af66 100644
--- a/bsd-user/freebsd/os-sys.c
+++ b/bsd-user/freebsd/os-sys.c
@@ -126,7 +126,7 @@ static abi_ulong h2t_ulong_sat(u_long ul)
  * sysctl, see /sys/kern/kern_sysctl.c:sysctl_sysctl_oidfmt() (compare to
  * src/sbin/sysctl/sysctl.c)
  */
-static int G_GNUC_UNUSED oidfmt(int *oid, int len, char *fmt, uint32_t *kind)
+static int oidfmt(int *oid, int len, char *fmt, uint32_t *kind)
 {
     int qoid[CTL_MAXNAME + 2];
     uint8_t buf[BUFSIZ];
@@ -168,7 +168,7 @@ static int G_GNUC_UNUSED oidfmt(int *oid, int len, char *fmt, uint32_t *kind)
  *
  * For opaque data, per sysctl OID converts take care of it.
  */
-static void G_GNUC_UNUSED h2t_old_sysctl(void *holdp, size_t *holdlen, uint32_t kind)
+static void h2t_old_sysctl(void *holdp, size_t *holdlen, uint32_t kind)
 {
     size_t len;
     int hlen, tlen;
@@ -243,7 +243,7 @@ static void G_GNUC_UNUSED h2t_old_sysctl(void *holdp, size_t *holdlen, uint32_t
 /*
  * Convert the undocmented name2oid sysctl data for the target.
  */
-static inline void G_GNUC_UNUSED sysctl_name2oid(uint32_t *holdp, size_t holdlen)
+static inline void sysctl_name2oid(uint32_t *holdp, size_t holdlen)
 {
     size_t i, num = holdlen / sizeof(uint32_t);
 
@@ -252,12 +252,94 @@ static inline void G_GNUC_UNUSED sysctl_name2oid(uint32_t *holdp, size_t holdlen
     }
 }
 
-static inline void G_GNUC_UNUSED sysctl_oidfmt(uint32_t *holdp)
+static inline void sysctl_oidfmt(uint32_t *holdp)
 {
     /* byte swap the kind */
     holdp[0] = tswap32(holdp[0]);
 }
 
+static abi_long G_GNUC_UNUSED do_freebsd_sysctl_oid(CPUArchState *env, int32_t *snamep,
+        int32_t namelen, void *holdp, size_t *holdlenp, void *hnewp,
+        size_t newlen)
+{
+    uint32_t kind = 0;
+    abi_long ret;
+    size_t holdlen, oldlen;
+#ifdef TARGET_ABI32
+    void *old_holdp;
+#endif
+
+    holdlen = oldlen = *holdlenp;
+    oidfmt(snamep, namelen, NULL, &kind);
+
+    /* Handle some arch/emulator dependent sysctl()'s here. */
+
+#ifdef TARGET_ABI32
+    /*
+     * For long and ulong with a 64-bit host and a 32-bit target we have to do
+     * special things. holdlen here is the length provided by the target to the
+     * system call. So we allocate a buffer twice as large because longs are twice
+     * as big on the host which will be writing them. In h2t_old_sysctl we'll adjust
+     * them and adjust the length.
+     */
+    if (kind == CTLTYPE_LONG || kind == CTLTYPE_ULONG) {
+        old_holdp = holdp;
+        holdlen = holdlen * 2;
+        holdp = g_malloc(holdlen);
+    }
+#endif
+
+    ret = get_errno(sysctl(snamep, namelen, holdp, &holdlen, hnewp, newlen));
+    if (!ret && (holdp != 0)) {
+
+        if (snamep[0] == CTL_SYSCTL) {
+            switch (snamep[1]) {
+            case CTL_SYSCTL_NEXT:
+            case CTL_SYSCTL_NAME2OID:
+            case CTL_SYSCTL_NEXTNOSKIP:
+                /*
+                 * All of these return an OID array, so we need to convert to
+                 * target.
+                 */
+                sysctl_name2oid(holdp, holdlen);
+                break;
+
+            case CTL_SYSCTL_OIDFMT:
+                /* Handle oidfmt */
+                sysctl_oidfmt(holdp);
+                break;
+            case CTL_SYSCTL_OIDDESCR:
+            case CTL_SYSCTL_OIDLABEL:
+            default:
+                /* Handle it based on the type */
+                h2t_old_sysctl(holdp, &holdlen, kind);
+                /* NB: None of these are LONG or ULONG */
+                break;
+            }
+        } else {
+            /*
+             * Need to convert from host to target. All the weird special cases
+             * are handled above.
+             */
+            h2t_old_sysctl(holdp, &holdlen, kind);
+#ifdef TARGET_ABI32
+            /*
+             * For the 32-bit on 64-bit case, for longs we need to copy the
+             * now-converted buffer to the target and free the buffer.
+             */
+            if (kind == CTLTYPE_LONG || kind == CTLTYPE_ULONG) {
+                memcpy(old_holdp, holdp, holdlen);
+                g_free(holdp);
+                holdp = old_holdp;
+            }
+#endif
+        }
+    }
+
+    *holdlenp = holdlen;
+    return ret;
+}
+
 /* sysarch() is architecture dependent. */
 abi_long do_freebsd_sysarch(void *cpu_env, abi_long arg1, abi_long arg2)
 {
-- 
2.39.1



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 09/12] bsd-user: Start translation of arch-specific sysctls
  2023-02-14  0:27 [PATCH v2 00/12] 2023 Q1 bsd-user upstreaming: bugfixes and sysctl Warner Losh
                   ` (7 preceding siblings ...)
  2023-02-14  0:27 ` [PATCH v2 08/12] bsd-user: common routine do_freebsd_sysctl_oid for all sysctl variants Warner Losh
@ 2023-02-14  0:27 ` Warner Losh
  2023-02-14 21:36   ` Richard Henderson
  2023-02-14  0:27 ` [PATCH v2 10/12] bsd-user: do_freebsd_sysctl helper for sysctl(2) Warner Losh
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 28+ messages in thread
From: Warner Losh @ 2023-02-14  0:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Warner Losh, richard.henderson, Paolo Bonzini,
	kevans, f4bug, Thomas Huth, Juergen Lock, Stacey Son

From: Juergen Lock <nox@jelal.kn-bremen.de>

Intercept some syscalls that we need to translate (like the archiecture
we're running on) and translate them. These are only the simplest ones
so far.

Signed-off-by: Juergen Lock <nox@jelal.kn-bremen.de>
Co-Authored-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Stacey Son <sson@FreeBSD.org>
Co-Authored-by: Warner Losh <imp@bsdimp.com>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/freebsd/os-sys.c | 145 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 143 insertions(+), 2 deletions(-)

diff --git a/bsd-user/freebsd/os-sys.c b/bsd-user/freebsd/os-sys.c
index e847404af66..cbaa70958b9 100644
--- a/bsd-user/freebsd/os-sys.c
+++ b/bsd-user/freebsd/os-sys.c
@@ -67,7 +67,7 @@ static const int host_ctl_size[CTLTYPE+1] = {
  */
 static const abi_ulong target_max_mem = UINT32_MAX - 0x100c000 + 1;
 
-static abi_ulong G_GNUC_UNUSED cap_memory(uint64_t mem)
+static abi_ulong cap_memory(uint64_t mem)
 {
     if (((unsigned long)target_max_mem) < mem) {
         mem = target_max_mem;
@@ -79,7 +79,7 @@ static abi_ulong G_GNUC_UNUSED cap_memory(uint64_t mem)
 
 static unsigned long host_page_size;
 
-static abi_ulong G_GNUC_UNUSED scale_to_target_pages(uint64_t pages)
+static abi_ulong scale_to_target_pages(uint64_t pages)
 {
     if (host_page_size == 0) {
         host_page_size = getpagesize();
@@ -273,6 +273,146 @@ static abi_long G_GNUC_UNUSED do_freebsd_sysctl_oid(CPUArchState *env, int32_t *
     oidfmt(snamep, namelen, NULL, &kind);
 
     /* Handle some arch/emulator dependent sysctl()'s here. */
+    switch (snamep[0]) {
+    case CTL_KERN:
+        switch (snamep[1]) {
+        case KERN_USRSTACK:
+            if (oldlen) {
+                (*(abi_ulong *)holdp) = tswapal(TARGET_USRSTACK);
+            }
+            holdlen = sizeof(abi_ulong);
+            ret = 0;
+            goto out;
+
+        case KERN_PS_STRINGS:
+            if (oldlen) {
+                (*(abi_ulong *)holdp) = tswapal(TARGET_PS_STRINGS);
+            }
+            holdlen = sizeof(abi_ulong);
+            ret = 0;
+            goto out;
+
+        default:
+            break;
+        }
+        break;
+
+    case CTL_HW:
+        switch (snamep[1]) {
+        case HW_MACHINE:
+            holdlen = sizeof(TARGET_HW_MACHINE);
+            if (holdp) {
+                strlcpy(holdp, TARGET_HW_MACHINE, oldlen);
+            }
+            ret = 0;
+            goto out;
+
+        case HW_MACHINE_ARCH:
+        {
+            holdlen = sizeof(TARGET_HW_MACHINE_ARCH);
+            if (holdp) {
+                strlcpy(holdp, TARGET_HW_MACHINE_ARCH, oldlen);
+            }
+            ret = 0;
+            goto out;
+        }
+        case HW_NCPU:
+            if (oldlen) {
+                (*(int32_t *)holdp) = tswap32(bsd_get_ncpu());
+            }
+            holdlen = sizeof(int32_t);
+            ret = 0;
+            goto out;
+#if defined(TARGET_ARM)
+        case HW_FLOATINGPT:
+            if (oldlen) {
+                ARMCPU *cpu = env_archcpu(env);
+                *(abi_int *)holdp = cpu_isar_feature(aa32_vfp, cpu);
+            }
+            holdlen = sizeof(int32_t);
+            ret = 0;
+            goto out;
+#endif
+
+
+#ifdef TARGET_ABI32
+        case HW_PHYSMEM:
+        case HW_USERMEM:
+        case HW_REALMEM:
+            holdlen = sizeof(abi_ulong);
+            ret = 0;
+
+            if (oldlen) {
+                int mib[2] = {snamep[0], snamep[1]};
+                unsigned long lvalue;
+                size_t len = sizeof(lvalue);
+
+                if (sysctl(mib, 2, &lvalue, &len, NULL, 0) == -1) {
+                    ret = -1;
+                } else {
+                    lvalue = cap_memory(lvalue);
+                    (*(abi_ulong *)holdp) = tswapal((abi_ulong)lvalue);
+                }
+            }
+            goto out;
+#endif
+
+        default:
+        {
+            static int oid_hw_availpages;
+            static int oid_hw_pagesizes;
+
+            if (!oid_hw_availpages) {
+                int real_oid[CTL_MAXNAME + 2];
+                size_t len = sizeof(real_oid) / sizeof(int);
+
+                if (sysctlnametomib("hw.availpages", real_oid, &len) >= 0) {
+                    oid_hw_availpages = real_oid[1];
+                }
+            }
+            if (!oid_hw_pagesizes) {
+                int real_oid[CTL_MAXNAME + 2];
+                size_t len = sizeof(real_oid) / sizeof(int);
+
+                if (sysctlnametomib("hw.pagesizes", real_oid, &len) >= 0) {
+                    oid_hw_pagesizes = real_oid[1];
+                }
+            }
+
+            if (oid_hw_availpages && snamep[1] == oid_hw_availpages) {
+                long lvalue;
+                size_t len = sizeof(lvalue);
+
+                if (sysctlbyname("hw.availpages", &lvalue, &len, NULL, 0) == -1) {
+                    ret = -1;
+                } else {
+                    if (oldlen) {
+                        lvalue = scale_to_target_pages(lvalue);
+                        (*(abi_ulong *)holdp) = tswapal((abi_ulong)lvalue);
+                    }
+                    holdlen = sizeof(abi_ulong);
+                    ret = 0;
+                }
+                goto out;
+            }
+
+            if (oid_hw_pagesizes && snamep[1] == oid_hw_pagesizes) {
+                if (oldlen) {
+                    (*(abi_ulong *)holdp) = tswapal((abi_ulong)getpagesize());
+                    ((abi_ulong *)holdp)[1] = 0;
+                }
+                holdlen = sizeof(abi_ulong) * 2;
+                ret = 0;
+                goto out;
+            }
+            break;
+        }
+        }
+        break;
+
+    default:
+        break;
+    }
 
 #ifdef TARGET_ABI32
     /*
@@ -336,6 +476,7 @@ static abi_long G_GNUC_UNUSED do_freebsd_sysctl_oid(CPUArchState *env, int32_t *
         }
     }
 
+out:
     *holdlenp = holdlen;
     return ret;
 }
-- 
2.39.1



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 10/12] bsd-user: do_freebsd_sysctl helper for sysctl(2)
  2023-02-14  0:27 [PATCH v2 00/12] 2023 Q1 bsd-user upstreaming: bugfixes and sysctl Warner Losh
                   ` (8 preceding siblings ...)
  2023-02-14  0:27 ` [PATCH v2 09/12] bsd-user: Start translation of arch-specific sysctls Warner Losh
@ 2023-02-14  0:27 ` Warner Losh
  2023-02-14 21:45   ` Richard Henderson
  2023-02-14  0:27 ` [PATCH v2 11/12] bsd-user: implement sysctlbyname(2) Warner Losh
  2023-02-14  0:27 ` [PATCH v2 12/12] bsd-user: Add -strict Warner Losh
  11 siblings, 1 reply; 28+ messages in thread
From: Warner Losh @ 2023-02-14  0:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Warner Losh, richard.henderson, Paolo Bonzini,
	kevans, f4bug, Thomas Huth, Kyle Evans, Juergen Lock, Stacey Son

From: Kyle Evans <kevans@FreeBSD.org>

Implement the wrapper function for sysctl(2). This puts the oid
arguments into a standard form and calls the common
do_freebsd_sysctl_oid.

Signed-off-by: Kyle Evans <kevans@FreeBSD.org>
Co-Authored-by: Juergen Lock <nox@jelal.kn-bremen.de>
Signed-off-by: Juergen Lock <nox@jelal.kn-bremen.de>
Co-Authored-by: Stacey Son <sson@FreeBSD.org>
Signed-off-by: Stacey Son <sson@FreeBSD.org>
Reviewed-by: Warner Losh <imp@bsdimp.com>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/freebsd/os-sys.c     | 57 ++++++++++++++++++++++++++++++++++-
 bsd-user/freebsd/os-syscall.c |  4 +++
 bsd-user/qemu.h               |  2 ++
 3 files changed, 62 insertions(+), 1 deletion(-)

diff --git a/bsd-user/freebsd/os-sys.c b/bsd-user/freebsd/os-sys.c
index cbaa70958b9..e70f8f21c52 100644
--- a/bsd-user/freebsd/os-sys.c
+++ b/bsd-user/freebsd/os-sys.c
@@ -258,7 +258,7 @@ static inline void sysctl_oidfmt(uint32_t *holdp)
     holdp[0] = tswap32(holdp[0]);
 }
 
-static abi_long G_GNUC_UNUSED do_freebsd_sysctl_oid(CPUArchState *env, int32_t *snamep,
+static abi_long do_freebsd_sysctl_oid(CPUArchState *env, int32_t *snamep,
         int32_t namelen, void *holdp, size_t *holdlenp, void *hnewp,
         size_t newlen)
 {
@@ -481,6 +481,61 @@ out:
     return ret;
 }
 
+abi_long do_freebsd_sysctl(CPUArchState *env, abi_ulong namep, int32_t namelen,
+        abi_ulong oldp, abi_ulong oldlenp, abi_ulong newp, abi_ulong newlen)
+{
+    abi_long ret = -TARGET_EFAULT;
+    void *hnamep, *holdp = NULL, *hnewp = NULL;
+    size_t holdlen;
+    abi_ulong oldlen = 0;
+    int32_t *snamep = g_malloc(sizeof(int32_t) * namelen), *p, *q, i;
+
+    /* oldlenp is read/write, pre-check here for write */
+    if (oldlenp) {
+        if (!access_ok(VERIFY_WRITE, oldlenp, sizeof(abi_ulong)) ||
+            get_user_ual(oldlen, oldlenp)) {
+            goto out;
+        }
+    }
+    hnamep = lock_user(VERIFY_READ, namep, namelen, 1);
+    if (hnamep == NULL) {
+        goto out;
+    }
+    if (newp) {
+        hnewp = lock_user(VERIFY_READ, newp, newlen, 1);
+        if (hnewp == NULL) {
+            goto out;
+        }
+    }
+    if (oldp) {
+        holdp = lock_user(VERIFY_WRITE, oldp, oldlen, 0);
+        if (holdp == NULL) {
+            goto out;
+        }
+    }
+    holdlen = oldlen;
+    for (p = hnamep, q = snamep, i = 0; i < namelen; p++, i++, q++) {
+        *q = tswap32(*p);
+    }
+
+    ret = do_freebsd_sysctl_oid(env, snamep, namelen, holdp, &holdlen, hnewp,
+        newlen);
+
+    /*
+     * writeability pre-checked above. __sysctl(2) returns ENOMEM and updates
+     * oldlenp for the proper size to use.
+     */
+    if (oldlenp && (ret == 0 || ret == -TARGET_ENOMEM)) {
+        put_user_ual(holdlen, oldlenp);
+    }
+    unlock_user(hnamep, namep, 0);
+    unlock_user(holdp, oldp, holdlen);
+    unlock_user(holdp, oldp, ret == 0 ? holdlen : 0);
+out:
+    g_free(snamep);
+    return ret;
+}
+
 /* sysarch() is architecture dependent. */
 abi_long do_freebsd_sysarch(void *cpu_env, abi_long arg1, abi_long arg2)
 {
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index e00997a818c..20ab3d4d9a1 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -494,6 +494,10 @@ static abi_long freebsd_syscall(void *cpu_env, int num, abi_long arg1,
         /*
          * sys{ctl, arch, call}
          */
+    case TARGET_FREEBSD_NR___sysctl: /* sysctl(3) */
+        ret = do_freebsd_sysctl(cpu_env, arg1, arg2, arg3, arg4, arg5, arg6);
+        break;
+
     case TARGET_FREEBSD_NR_sysarch: /* sysarch(2) */
         ret = do_freebsd_sysarch(cpu_env, arg1, arg2);
         break;
diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index 0ceecfb6dfa..c7248cfde6f 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -252,6 +252,8 @@ bool is_error(abi_long ret);
 int host_to_target_errno(int err);
 
 /* os-sys.c */
+abi_long do_freebsd_sysctl(CPUArchState *env, abi_ulong namep, int32_t namelen,
+        abi_ulong oldp, abi_ulong oldlenp, abi_ulong newp, abi_ulong newlen);
 abi_long do_freebsd_sysarch(void *cpu_env, abi_long arg1, abi_long arg2);
 
 /* user access */
-- 
2.39.1



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 11/12] bsd-user: implement sysctlbyname(2)
  2023-02-14  0:27 [PATCH v2 00/12] 2023 Q1 bsd-user upstreaming: bugfixes and sysctl Warner Losh
                   ` (9 preceding siblings ...)
  2023-02-14  0:27 ` [PATCH v2 10/12] bsd-user: do_freebsd_sysctl helper for sysctl(2) Warner Losh
@ 2023-02-14  0:27 ` Warner Losh
  2023-02-14 21:48   ` Richard Henderson
  2023-02-14  0:27 ` [PATCH v2 12/12] bsd-user: Add -strict Warner Losh
  11 siblings, 1 reply; 28+ messages in thread
From: Warner Losh @ 2023-02-14  0:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Warner Losh, richard.henderson, Paolo Bonzini,
	kevans, f4bug, Thomas Huth, Kyle Evans

From: Kyle Evans <kevans@FreeBSD.org>

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 <kevans@FreeBSD.org>
Reviewed-by: Warner Losh <imp@bsdimp.com>
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/freebsd/os-sys.c     | 67 +++++++++++++++++++++++++++++++++++
 bsd-user/freebsd/os-syscall.c |  4 +++
 bsd-user/qemu.h               |  3 ++
 3 files changed, 74 insertions(+)

diff --git a/bsd-user/freebsd/os-sys.c b/bsd-user/freebsd/os-sys.c
index e70f8f21c52..33720ddbb38 100644
--- a/bsd-user/freebsd/os-sys.c
+++ b/bsd-user/freebsd/os-sys.c
@@ -481,6 +481,73 @@ out:
     return ret;
 }
 
+/*
+ * 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.
+ */
+abi_long do_freebsd_sysctlbyname(CPUArchState *env, abi_ulong namep,
+        int32_t namelen, abi_ulong oldp, abi_ulong oldlenp, abi_ulong newp,
+        abi_ulong newlen)
+{
+    abi_long ret = -TARGET_EFAULT;
+    void *holdp = NULL, *hnewp = NULL;
+    char *snamep = NULL;
+    int oid[CTL_MAXNAME + 2];
+    size_t holdlen, oidplen;
+    abi_ulong oldlen = 0;
+
+    /* oldlenp is read/write, pre-check here for write */
+    if (oldlenp) {
+        if (!access_ok(VERIFY_WRITE, oldlenp, sizeof(abi_ulong)) ||
+            get_user_ual(oldlen, oldlenp)) {
+            goto out;
+        }
+    }
+    snamep = lock_user_string(namep);
+    if (snamep == NULL) {
+        goto out;
+    }
+    if (newp) {
+        hnewp = lock_user(VERIFY_READ, newp, newlen, 1);
+        if (hnewp == NULL) {
+            goto out;
+        }
+    }
+    if (oldp) {
+        holdp = lock_user(VERIFY_WRITE, oldp, oldlen, 0);
+        if (holdp == NULL) {
+            goto out;
+        }
+    }
+    holdlen = oldlen;
+
+    oidplen = sizeof(oid) / sizeof(int);
+    if (sysctlnametomib(snamep, oid, &oidplen) != 0) {
+        ret = -TARGET_EINVAL;
+        goto out;
+    }
+
+    ret = do_freebsd_sysctl_oid(env, oid, oidplen, holdp, &holdlen, hnewp,
+        newlen);
+
+    /*
+     * writeability pre-checked above. __sysctl(2) returns ENOMEM and updates
+     * oldlenp for the proper size to use.
+     */
+    if (oldlenp && (ret == 0 || ret == -TARGET_ENOMEM)) {
+        put_user_ual(holdlen, oldlenp);
+    }
+out:
+    unlock_user(snamep, namep, 0);
+    unlock_user(holdp, oldp, ret == 0 ? holdlen : 0);
+    unlock_user(hnewp, newp, 0);
+
+    return ret;
+}
+
 abi_long do_freebsd_sysctl(CPUArchState *env, abi_ulong namep, int32_t namelen,
         abi_ulong oldp, abi_ulong oldlenp, abi_ulong newp, abi_ulong newlen)
 {
diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 20ab3d4d9a1..179a20c304b 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -498,6 +498,10 @@ static abi_long freebsd_syscall(void *cpu_env, int num, abi_long arg1,
         ret = do_freebsd_sysctl(cpu_env, arg1, arg2, arg3, arg4, arg5, arg6);
         break;
 
+    case TARGET_FREEBSD_NR___sysctlbyname: /* sysctlbyname(2) */
+        ret = do_freebsd_sysctlbyname(cpu_env, arg1, arg2, arg3, arg4, arg5, arg6);
+        break;
+
     case TARGET_FREEBSD_NR_sysarch: /* sysarch(2) */
         ret = do_freebsd_sysarch(cpu_env, arg1, arg2);
         break;
diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index c7248cfde6f..e24a8cfcfb1 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -254,6 +254,9 @@ int host_to_target_errno(int err);
 /* os-sys.c */
 abi_long do_freebsd_sysctl(CPUArchState *env, abi_ulong namep, int32_t namelen,
         abi_ulong oldp, abi_ulong oldlenp, abi_ulong newp, abi_ulong newlen);
+abi_long do_freebsd_sysctlbyname(CPUArchState *env, abi_ulong namep,
+        int32_t namelen, abi_ulong oldp, abi_ulong oldlenp, abi_ulong newp,
+        abi_ulong newlen);
 abi_long do_freebsd_sysarch(void *cpu_env, abi_long arg1, abi_long arg2);
 
 /* user access */
-- 
2.39.1



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 12/12] bsd-user: Add -strict
  2023-02-14  0:27 [PATCH v2 00/12] 2023 Q1 bsd-user upstreaming: bugfixes and sysctl Warner Losh
                   ` (10 preceding siblings ...)
  2023-02-14  0:27 ` [PATCH v2 11/12] bsd-user: implement sysctlbyname(2) Warner Losh
@ 2023-02-14  0:27 ` Warner Losh
  2023-02-14  6:57   ` Philippe Mathieu-Daudé
  2023-02-14 21:49   ` Richard Henderson
  11 siblings, 2 replies; 28+ messages in thread
From: Warner Losh @ 2023-02-14  0:27 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Bennée, Warner Losh, richard.henderson, Paolo Bonzini,
	kevans, f4bug, Thomas Huth

Most of the time, it's useful to make our best effort, but sometimes we
want to know right away when we don't implement something. First place
we use it is for unknown syscalls.

Signed-off-by: Warner Losh <imp@bsdimp.com>
---
 bsd-user/freebsd/os-syscall.c | 4 ++++
 bsd-user/main.c               | 5 ++++-
 bsd-user/qemu.h               | 1 +
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
index 179a20c304b..e2b26ecb8dd 100644
--- a/bsd-user/freebsd/os-syscall.c
+++ b/bsd-user/freebsd/os-syscall.c
@@ -508,6 +508,10 @@ static abi_long freebsd_syscall(void *cpu_env, int num, abi_long arg1,
 
     default:
         qemu_log_mask(LOG_UNIMP, "Unsupported syscall: %d\n", num);
+        if (bsd_user_strict) {
+            printf("Unimplemented system call %d\n", num);
+            abort();
+        }
         ret = -TARGET_ENOSYS;
         break;
     }
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 41290e16f98..ba0ad86ad28 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -91,9 +91,10 @@ unsigned long reserved_va = MAX_RESERVED_VA;
 unsigned long reserved_va;
 #endif
 
-static const char *interp_prefix = CONFIG_QEMU_INTERP_PREFIX;
+const char *interp_prefix = CONFIG_QEMU_INTERP_PREFIX;
 const char *qemu_uname_release;
 char qemu_proc_pathname[PATH_MAX];  /* full path to exeutable */
+bool bsd_user_strict = false;	/* Abort for unimplemned things */
 
 unsigned long target_maxtsiz = TARGET_MAXTSIZ;   /* max text size */
 unsigned long target_dfldsiz = TARGET_DFLDSIZ;   /* initial data size limit */
@@ -396,6 +397,8 @@ int main(int argc, char **argv)
             trace_opt_parse(optarg);
         } else if (!strcmp(r, "0")) {
             argv0 = argv[optind++];
+        } else if (!strcmp(r, "strict")) {
+            bsd_user_strict = true;
         } else {
             usage();
         }
diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index e24a8cfcfb1..22bd5a3df42 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -113,6 +113,7 @@ typedef struct TaskState {
 
 void stop_all_tasks(void);
 extern const char *qemu_uname_release;
+extern bool bsd_user_strict;
 
 /*
  * TARGET_ARG_MAX defines the number of bytes allocated for arguments
-- 
2.39.1



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 12/12] bsd-user: Add -strict
  2023-02-14  0:27 ` [PATCH v2 12/12] bsd-user: Add -strict Warner Losh
@ 2023-02-14  6:57   ` Philippe Mathieu-Daudé
  2023-02-14 21:49   ` Richard Henderson
  1 sibling, 0 replies; 28+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-14  6:57 UTC (permalink / raw)
  To: Warner Losh, qemu-devel
  Cc: Alex Bennée, richard.henderson, Paolo Bonzini, kevans,
	f4bug, Thomas Huth

On 14/2/23 01:27, Warner Losh wrote:
> Most of the time, it's useful to make our best effort, but sometimes we
> want to know right away when we don't implement something. First place
> we use it is for unknown syscalls.
> 
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>   bsd-user/freebsd/os-syscall.c | 4 ++++
>   bsd-user/main.c               | 5 ++++-
>   bsd-user/qemu.h               | 1 +
>   3 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
> index 179a20c304b..e2b26ecb8dd 100644
> --- a/bsd-user/freebsd/os-syscall.c
> +++ b/bsd-user/freebsd/os-syscall.c
> @@ -508,6 +508,10 @@ static abi_long freebsd_syscall(void *cpu_env, int num, abi_long arg1,
>   
>       default:
>           qemu_log_mask(LOG_UNIMP, "Unsupported syscall: %d\n", num);
> +        if (bsd_user_strict) {
> +            printf("Unimplemented system call %d\n", num);
> +            abort();
> +        }
>           ret = -TARGET_ENOSYS;
>           break;
>       }
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index 41290e16f98..ba0ad86ad28 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -91,9 +91,10 @@ unsigned long reserved_va = MAX_RESERVED_VA;
>   unsigned long reserved_va;
>   #endif
>   
> -static const char *interp_prefix = CONFIG_QEMU_INTERP_PREFIX;
> +const char *interp_prefix = CONFIG_QEMU_INTERP_PREFIX;
>   const char *qemu_uname_release;
>   char qemu_proc_pathname[PATH_MAX];  /* full path to exeutable */
> +bool bsd_user_strict = false;	/* Abort for unimplemned things */

"executable" (pre-existing), "unimplemented".

>   unsigned long target_maxtsiz = TARGET_MAXTSIZ;   /* max text size */
>   unsigned long target_dfldsiz = TARGET_DFLDSIZ;   /* initial data size limit */
> @@ -396,6 +397,8 @@ int main(int argc, char **argv)
>               trace_opt_parse(optarg);
>           } else if (!strcmp(r, "0")) {
>               argv0 = argv[optind++];
> +        } else if (!strcmp(r, "strict")) {
> +            bsd_user_strict = true;

As a debug feature, can't we just use some getenv() var?

>           } else {
>               usage();
>           }
> diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
> index e24a8cfcfb1..22bd5a3df42 100644
> --- a/bsd-user/qemu.h
> +++ b/bsd-user/qemu.h
> @@ -113,6 +113,7 @@ typedef struct TaskState {
>   
>   void stop_all_tasks(void);
>   extern const char *qemu_uname_release;
> +extern bool bsd_user_strict;
>   
>   /*
>    * TARGET_ARG_MAX defines the number of bytes allocated for arguments



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 04/12] bsd-user: various helper routines for sysctl
  2023-02-14  0:27 ` [PATCH v2 04/12] bsd-user: various helper routines for sysctl Warner Losh
@ 2023-02-14 20:52   ` Richard Henderson
  2023-02-14 21:31     ` Warner Losh
  0 siblings, 1 reply; 28+ messages in thread
From: Richard Henderson @ 2023-02-14 20:52 UTC (permalink / raw)
  To: Warner Losh, qemu-devel
  Cc: Alex Bennée, Paolo Bonzini, kevans, f4bug, Thomas Huth

On 2/13/23 14:27, Warner Losh wrote:
> +#ifdef TARGET_ABI32
> +/*
> + * Limit the amount of available memory to be most of the 32-bit address
> + * space. 0x100c000 was arrived at through trial and error as a good
> + * definition of 'most'.
> + */
> +static const abi_ulong target_max_mem = UINT32_MAX - 0x100c000 + 1;
> +
> +static abi_ulong G_GNUC_UNUSED cap_memory(uint64_t mem)
> +{
> +    if (((unsigned long)target_max_mem) < mem) {
> +        mem = target_max_mem;
> +    }
> +
> +    return mem;
> +}
> +#endif

Identity function for ABI64?

> +static unsigned long host_page_size;
> +
> +static abi_ulong G_GNUC_UNUSED scale_to_target_pages(uint64_t pages)
> +{
> +    if (host_page_size == 0) {
> +        host_page_size = getpagesize();
> +    }

qemu_real_host_page_size()


> +
> +    pages = muldiv64(pages, host_page_size, TARGET_PAGE_SIZE);
> +#ifdef TARGET_ABI32
> +    abi_ulong maxpages = target_max_mem / (abi_ulong)TARGET_PAGE_SIZE;
> +
> +    if (((unsigned long)maxpages) < pages) {
> +        pages = maxpages;
> +    }
> +#endif

No need for either cast.  Just use MIN().

> +#ifdef TARGET_ABI32
> +static abi_long G_GNUC_UNUSED h2t_long_sat(long l)

h2g.

> +{
> +    if (l > INT32_MAX) {
> +        l = INT32_MAX;
> +    } else if (l < INT32_MIN) {
> +        l = INT32_MIN;
> +    }
> +    return l;
> +}
> +
> +static abi_ulong G_GNUC_UNUSED h2t_ulong_sat(u_long ul)
> +{
> +    if (ul > UINT32_MAX) {
> +        ul = UINT32_MAX;
> +    }
> +    return ul;
> +}
> +#endif

Anyway, identity functions for ABI64?


r~


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 05/12] bsd-user: Helper routines oidfmt
  2023-02-14  0:27 ` [PATCH v2 05/12] bsd-user: Helper routines oidfmt Warner Losh
@ 2023-02-14 20:53   ` Richard Henderson
  0 siblings, 0 replies; 28+ messages in thread
From: Richard Henderson @ 2023-02-14 20:53 UTC (permalink / raw)
  To: Warner Losh, qemu-devel
  Cc: Alex Bennée, Paolo Bonzini, kevans, f4bug, Thomas Huth,
	Stacey Son, Sean Bruno, Juergen Lock, Raphael Kubo da Costa

On 2/13/23 14:27, Warner Losh wrote:
> From: Stacey Son<sson@FreeBSD.org>
> 
> oidfmt uses undocumented system call to get the type of the sysctl.
> 
> Co-Authored-by: Sean Bruno<sbruno@FreeBSD.org>
> Signed-off-by: Sean Bruno<sbruno@FreeBSD.org>
> Co-Authored-by: Juergen Lock<nox@jelal.kn-bremen.de>
> Signed-off-by: Juergen Lock<nox@jelal.kn-bremen.de>
> Co-Authored-by: Raphael Kubo da Costa<rakuco@FreeBSD.org>
> Signed-off-by: Raphael Kubo da Costa<rakuco@FreeBSD.org>
> Signed-off-by: Stacey Son<sson@FreeBSD.org>
> Reviewed-by: Warner Losh<imp@bsdimp.com>
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/freebsd/os-sys.c | 32 ++++++++++++++++++++++++++++++++
>   1 file changed, 32 insertions(+)

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

r~


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 06/12] bsd-user: Helper routines h2t_old_sysctl
  2023-02-14  0:27 ` [PATCH v2 06/12] bsd-user: Helper routines h2t_old_sysctl Warner Losh
@ 2023-02-14 21:16   ` Richard Henderson
  2023-02-15  5:58     ` Warner Losh
  0 siblings, 1 reply; 28+ messages in thread
From: Richard Henderson @ 2023-02-14 21:16 UTC (permalink / raw)
  To: Warner Losh, qemu-devel
  Cc: Alex Bennée, Paolo Bonzini, kevans, f4bug, Thomas Huth,
	Sean Bruno, Juergen Lock, Raphael Kubo da Costa, Stacey Son

On 2/13/23 14:27, Warner Losh wrote:
> +/*
> + * Convert the old value from host to target.

host vs guest is clearer language; "target" gets overloaded, even though still present in 
the code base.

> + *
> + * For LONG and ULONG on ABI32, we need to 'down convert' the 8 byte quantities
> + * to 4 bytes. The caller setup a buffer in host memory to get this data from
> + * the kernel and pass it to us. We do the down conversion and adjust the length
> + * so the caller knows what to write as the returned length into the target when
> + * it copies the down converted values into the target.
> + *
> + * For normal integral types, we just need to byte swap. No size changes.
> + *
> + * For strings and node data, there's no conversion needed.
> + *
> + * For opaque data, per sysctl OID converts take care of it.
> + */
> +static void G_GNUC_UNUSED h2t_old_sysctl(void *holdp, size_t *holdlen, uint32_t kind)

h2g.

> +    /*
> +     * hlen == 0 for CTLTYPE_STRING and CTLTYPE_NODE, which need no conversion
> +     * as well as CTLTYPE_OPAQUE, which needs special converters.
> +     */
> +    if (hlen == 0) {
> +        return;
> +    }
> +
> +    while (len < *holdlen) {
> +        if (hlen == tlen) {
> +            switch (hlen) {
> +            case 1:
> +                /* Nothing needed: no byteswapping and assigning in place */
> +                break;
> +            case 2:
> +                *(uint16_t *)tp = tswap16(*(uint16_t *)hp);
> +                break;
> +            case 4:
> +                *(uint32_t *)tp = tswap32(*(uint32_t *)hp);
> +                break;
> +            case 8:
> +                *(uint64_t *)tp = tswap64(*(uint64_t *)hp);
> +                break;
> +            }

default: g_assert_not_reached().

> +        }
> +#ifdef TARGET_ABI32
> +        else {
> +            /*
> +             * Saturating assignment for the only two types that differ between
> +             * 32-bit and 64-bit machines. All other integral types have the
> +             * same, fixed size and will be converted w/o loss of precision
> +             * in the above switch.
> +             */
> +            switch (kind & CTLTYPE) {
> +            case CTLTYPE_LONG:
> +                *(abi_long *)tp = tswap32(h2t_long_sat(*(long *)hp));
> +                break;
> +            case CTLTYPE_ULONG:
> +                *(abi_ulong *)tp = tswap32(h2t_ulong_sat(*(u_long *)hp));
> +                break;
> +            }

default: g_assert_not_reached().

> +        }
> +#endif

#else
     g_assert_not_reached();


r~


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 07/12] bsd-user: sysctl helper funtions: sysctl_name2oid and sysctl_oidfmt
  2023-02-14  0:27 ` [PATCH v2 07/12] bsd-user: sysctl helper funtions: sysctl_name2oid and sysctl_oidfmt Warner Losh
@ 2023-02-14 21:18   ` Richard Henderson
  0 siblings, 0 replies; 28+ messages in thread
From: Richard Henderson @ 2023-02-14 21:18 UTC (permalink / raw)
  To: Warner Losh, qemu-devel
  Cc: Alex Bennée, Paolo Bonzini, kevans, f4bug, Thomas Huth,
	Juergen Lock

On 2/13/23 14:27, Warner Losh wrote:
> From: Juergen Lock<nox@jelal.kn-bremen.de>
> 
> Helper functions for sysctl implementations. sysctl_name2oid and
> sysctl_oidfmt convert oids between host and targets
> 
> Signed-off-by: Juergen Lock<nox@jelal.kn-bremen.de>
> Reviewed-by: Warner Losh<imp@bsdimp.com>
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
>   bsd-user/freebsd/os-sys.c | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
> 
> diff --git a/bs

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

r~


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 08/12] bsd-user: common routine do_freebsd_sysctl_oid for all sysctl variants
  2023-02-14  0:27 ` [PATCH v2 08/12] bsd-user: common routine do_freebsd_sysctl_oid for all sysctl variants Warner Losh
@ 2023-02-14 21:21   ` Richard Henderson
  0 siblings, 0 replies; 28+ messages in thread
From: Richard Henderson @ 2023-02-14 21:21 UTC (permalink / raw)
  To: Warner Losh, qemu-devel
  Cc: Alex Bennée, Paolo Bonzini, kevans, f4bug, Thomas Huth,
	Juergen Lock, Stacey Son

On 2/13/23 14:27, Warner Losh wrote:
> From: Juergen Lock <nox@jelal.kn-bremen.de>
> 
> do_freebsd_sysctl_oid filters out some of the binary and special sysctls
> where host != target. None of the sysctls that have to be translated from
> host to target are handled here.
> 
> Signed-off-by: Juergen Lock <nox@jelal.kn-bremen.de>
> Co-Authored-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Co-Authored-by: Warner Losh <imp@bsdimp.com>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>   bsd-user/freebsd/os-sys.c | 90 +++++++++++++++++++++++++++++++++++++--
>   1 file changed, 86 insertions(+), 4 deletions(-)

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


r~


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 04/12] bsd-user: various helper routines for sysctl
  2023-02-14 20:52   ` Richard Henderson
@ 2023-02-14 21:31     ` Warner Losh
  2023-02-14 21:39       ` Richard Henderson
  0 siblings, 1 reply; 28+ messages in thread
From: Warner Losh @ 2023-02-14 21:31 UTC (permalink / raw)
  To: Richard Henderson
  Cc: qemu-devel, Alex Bennée, Paolo Bonzini, kevans, f4bug, Thomas Huth

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

On Tue, Feb 14, 2023 at 1:52 PM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 2/13/23 14:27, Warner Losh wrote:
> > +#ifdef TARGET_ABI32
> > +/*
> > + * Limit the amount of available memory to be most of the 32-bit address
> > + * space. 0x100c000 was arrived at through trial and error as a good
> > + * definition of 'most'.
> > + */
> > +static const abi_ulong target_max_mem = UINT32_MAX - 0x100c000 + 1;
> > +
> > +static abi_ulong G_GNUC_UNUSED cap_memory(uint64_t mem)
> > +{
> > +    if (((unsigned long)target_max_mem) < mem) {
> > +        mem = target_max_mem;
> > +    }
> > +
> > +    return mem;
> > +}
> > +#endif
>
> Identity function for ABI64?
>

Indirectly, yes. For ABI64 we simply don't intercept these sysctl nodes.


> > +static unsigned long host_page_size;
> > +
> > +static abi_ulong G_GNUC_UNUSED scale_to_target_pages(uint64_t pages)
> > +{
> > +    if (host_page_size == 0) {
> > +        host_page_size = getpagesize();
> > +    }
>
> qemu_real_host_page_size()
>

OK. Easy enough. That was a warning from checkpatch anyway that had slipped
my mind.


> > +
> > +    pages = muldiv64(pages, host_page_size, TARGET_PAGE_SIZE);
> > +#ifdef TARGET_ABI32
> > +    abi_ulong maxpages = target_max_mem / (abi_ulong)TARGET_PAGE_SIZE;
> > +
> > +    if (((unsigned long)maxpages) < pages) {
> > +        pages = maxpages;
> > +    }
> > +#endif
>
> No need for either cast.  Just use MIN().
>

Gotcha.


> > +#ifdef TARGET_ABI32
> > +static abi_long G_GNUC_UNUSED h2t_long_sat(long l)
>
> h2g.
>

OK.


> > +{
> > +    if (l > INT32_MAX) {
> > +        l = INT32_MAX;
> > +    } else if (l < INT32_MIN) {
> > +        l = INT32_MIN;
> > +    }
> > +    return l;
> > +}
> > +
> > +static abi_ulong G_GNUC_UNUSED h2t_ulong_sat(u_long ul)
> > +{
> > +    if (ul > UINT32_MAX) {
> > +        ul = UINT32_MAX;
> > +    }
> > +    return ul;
> > +}
> > +#endif
>
> Anyway, identity functions for ABI64?
>

Right now they aren't used at all for ABI64...  But that's in later
patches...  We only do
special things for  LONG or ULONG on ABI32... Otherwise, the normal paths
wouldn't
call these at all.

Warner

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 09/12] bsd-user: Start translation of arch-specific sysctls
  2023-02-14  0:27 ` [PATCH v2 09/12] bsd-user: Start translation of arch-specific sysctls Warner Losh
@ 2023-02-14 21:36   ` Richard Henderson
  2023-02-16 22:57     ` Warner Losh
  0 siblings, 1 reply; 28+ messages in thread
From: Richard Henderson @ 2023-02-14 21:36 UTC (permalink / raw)
  To: Warner Losh, qemu-devel
  Cc: Alex Bennée, Paolo Bonzini, kevans, f4bug, Thomas Huth,
	Juergen Lock, Stacey Son

On 2/13/23 14:27, Warner Losh wrote:
> +        case HW_NCPU:
> +            if (oldlen) {
> +                (*(int32_t *)holdp) = tswap32(bsd_get_ncpu());
> +            }
> +            holdlen = sizeof(int32_t);
> +            ret = 0;
> +            goto out;

Anything using SYSCTL_INT should use abi_int.

> +#if defined(TARGET_ARM)
> +        case HW_FLOATINGPT:
> +            if (oldlen) {
> +                ARMCPU *cpu = env_archcpu(env);
> +                *(abi_int *)holdp = cpu_isar_feature(aa32_vfp, cpu);
> +            }
> +            holdlen = sizeof(int32_t);

abi_int for consistency.

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


r~


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 04/12] bsd-user: various helper routines for sysctl
  2023-02-14 21:31     ` Warner Losh
@ 2023-02-14 21:39       ` Richard Henderson
  0 siblings, 0 replies; 28+ messages in thread
From: Richard Henderson @ 2023-02-14 21:39 UTC (permalink / raw)
  To: Warner Losh
  Cc: qemu-devel, Alex Bennée, Paolo Bonzini, kevans, f4bug, Thomas Huth

On 2/14/23 11:31, Warner Losh wrote:
> Right now they aren't used at all for ABI64...  But that's in later patches...  We only do
> special things for  LONG or ULONG on ABI32... Otherwise, the normal paths wouldn't
> call these at all.

Yes, I've just seen patch 9, and agree they aren't needed for abi64.


r~


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 10/12] bsd-user: do_freebsd_sysctl helper for sysctl(2)
  2023-02-14  0:27 ` [PATCH v2 10/12] bsd-user: do_freebsd_sysctl helper for sysctl(2) Warner Losh
@ 2023-02-14 21:45   ` Richard Henderson
  0 siblings, 0 replies; 28+ messages in thread
From: Richard Henderson @ 2023-02-14 21:45 UTC (permalink / raw)
  To: Warner Losh, qemu-devel
  Cc: Alex Bennée, Paolo Bonzini, kevans, f4bug, Thomas Huth,
	Juergen Lock, Stacey Son

On 2/13/23 14:27, Warner Losh wrote:
> +    int32_t *snamep = g_malloc(sizeof(int32_t) * namelen), *p, *q, i;

g_new(int32_t, namelen)

> +    unlock_user(holdp, oldp, holdlen);
> +    unlock_user(holdp, oldp, ret == 0 ? holdlen : 0);

double-unlock.  clearly the first line is extra.

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


r~


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 11/12] bsd-user: implement sysctlbyname(2)
  2023-02-14  0:27 ` [PATCH v2 11/12] bsd-user: implement sysctlbyname(2) Warner Losh
@ 2023-02-14 21:48   ` Richard Henderson
  0 siblings, 0 replies; 28+ messages in thread
From: Richard Henderson @ 2023-02-14 21:48 UTC (permalink / raw)
  To: Warner Losh, qemu-devel
  Cc: Alex Bennée, Paolo Bonzini, kevans, f4bug, Thomas Huth

On 2/13/23 14:27, Warner Losh wrote:
> +    oidplen = sizeof(oid) / sizeof(int);

ARRAY_SIZE(oid)

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


r~


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 12/12] bsd-user: Add -strict
  2023-02-14  0:27 ` [PATCH v2 12/12] bsd-user: Add -strict Warner Losh
  2023-02-14  6:57   ` Philippe Mathieu-Daudé
@ 2023-02-14 21:49   ` Richard Henderson
  2023-02-16 22:37     ` Warner Losh
  1 sibling, 1 reply; 28+ messages in thread
From: Richard Henderson @ 2023-02-14 21:49 UTC (permalink / raw)
  To: Warner Losh, qemu-devel
  Cc: Alex Bennée, Paolo Bonzini, kevans, f4bug, Thomas Huth

On 2/13/23 14:27, Warner Losh wrote:
> Most of the time, it's useful to make our best effort, but sometimes we
> want to know right away when we don't implement something. First place
> we use it is for unknown syscalls.
> 
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
>   bsd-user/freebsd/os-syscall.c | 4 ++++
>   bsd-user/main.c               | 5 ++++-
>   bsd-user/qemu.h               | 1 +
>   3 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
> index 179a20c304b..e2b26ecb8dd 100644
> --- a/bsd-user/freebsd/os-syscall.c
> +++ b/bsd-user/freebsd/os-syscall.c
> @@ -508,6 +508,10 @@ static abi_long freebsd_syscall(void *cpu_env, int num, abi_long arg1,
>   
>       default:
>           qemu_log_mask(LOG_UNIMP, "Unsupported syscall: %d\n", num);
> +        if (bsd_user_strict) {
> +            printf("Unimplemented system call %d\n", num);
> +            abort();
> +        }

Still don't like the printf.  I suggested alternatives against v1.

r~


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 06/12] bsd-user: Helper routines h2t_old_sysctl
  2023-02-14 21:16   ` Richard Henderson
@ 2023-02-15  5:58     ` Warner Losh
  0 siblings, 0 replies; 28+ messages in thread
From: Warner Losh @ 2023-02-15  5:58 UTC (permalink / raw)
  To: Richard Henderson
  Cc: qemu-devel, Alex Bennée, Paolo Bonzini, kevans, f4bug,
	Thomas Huth, Sean Bruno, Juergen Lock, Raphael Kubo da Costa,
	Stacey Son

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

On Tue, Feb 14, 2023 at 2:16 PM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 2/13/23 14:27, Warner Losh wrote:
> > +/*
> > + * Convert the old value from host to target.
>
> host vs guest is clearer language; "target" gets overloaded, even though
> still present in
> the code base.
>

OK. Will do. We have that all over the place upstream... I'll start there
too...


>
> > + *
> > + * For LONG and ULONG on ABI32, we need to 'down convert' the 8 byte
> quantities
> > + * to 4 bytes. The caller setup a buffer in host memory to get this
> data from
> > + * the kernel and pass it to us. We do the down conversion and adjust
> the length
> > + * so the caller knows what to write as the returned length into the
> target when
> > + * it copies the down converted values into the target.
> > + *
> > + * For normal integral types, we just need to byte swap. No size
> changes.
> > + *
> > + * For strings and node data, there's no conversion needed.
> > + *
> > + * For opaque data, per sysctl OID converts take care of it.
> > + */
> > +static void G_GNUC_UNUSED h2t_old_sysctl(void *holdp, size_t *holdlen,
> uint32_t kind)
>
> h2g.
>

OK.


> > +    /*
> > +     * hlen == 0 for CTLTYPE_STRING and CTLTYPE_NODE, which need no
> conversion
> > +     * as well as CTLTYPE_OPAQUE, which needs special converters.
> > +     */
> > +    if (hlen == 0) {
> > +        return;
> > +    }
> > +
> > +    while (len < *holdlen) {
> > +        if (hlen == tlen) {
> > +            switch (hlen) {
> > +            case 1:
> > +                /* Nothing needed: no byteswapping and assigning in
> place */
> > +                break;
> > +            case 2:
> > +                *(uint16_t *)tp = tswap16(*(uint16_t *)hp);
> > +                break;
> > +            case 4:
> > +                *(uint32_t *)tp = tswap32(*(uint32_t *)hp);
> > +                break;
> > +            case 8:
> > +                *(uint64_t *)tp = tswap64(*(uint64_t *)hp);
> > +                break;
> > +            }
>
> default: g_assert_not_reached().
>

Ah!  I need that in several places... Thanks.


> > +        }
> > +#ifdef TARGET_ABI32
> > +        else {
> > +            /*
> > +             * Saturating assignment for the only two types that differ
> between
> > +             * 32-bit and 64-bit machines. All other integral types
> have the
> > +             * same, fixed size and will be converted w/o loss of
> precision
> > +             * in the above switch.
> > +             */
> > +            switch (kind & CTLTYPE) {
> > +            case CTLTYPE_LONG:
> > +                *(abi_long *)tp = tswap32(h2t_long_sat(*(long *)hp));
> > +                break;
> > +            case CTLTYPE_ULONG:
> > +                *(abi_ulong *)tp = tswap32(h2t_ulong_sat(*(u_long
> *)hp));
> > +                break;
> > +            }
>
> default: g_assert_not_reached().
>
> > +        }
> > +#endif
>
> #else
>      g_assert_not_reached();
>

Gotcha... Thanks!

Warner


>
> r~
>

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 12/12] bsd-user: Add -strict
  2023-02-14 21:49   ` Richard Henderson
@ 2023-02-16 22:37     ` Warner Losh
  0 siblings, 0 replies; 28+ messages in thread
From: Warner Losh @ 2023-02-16 22:37 UTC (permalink / raw)
  To: Richard Henderson
  Cc: qemu-devel, Alex Bennée, Paolo Bonzini, kevans, f4bug, Thomas Huth

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

On Tue, Feb 14, 2023 at 2:49 PM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 2/13/23 14:27, Warner Losh wrote:
> > Most of the time, it's useful to make our best effort, but sometimes we
> > want to know right away when we don't implement something. First place
> > we use it is for unknown syscalls.
> >
> > Signed-off-by: Warner Losh <imp@bsdimp.com>
> > ---
> >   bsd-user/freebsd/os-syscall.c | 4 ++++
> >   bsd-user/main.c               | 5 ++++-
> >   bsd-user/qemu.h               | 1 +
> >   3 files changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/bsd-user/freebsd/os-syscall.c
> b/bsd-user/freebsd/os-syscall.c
> > index 179a20c304b..e2b26ecb8dd 100644
> > --- a/bsd-user/freebsd/os-syscall.c
> > +++ b/bsd-user/freebsd/os-syscall.c
> > @@ -508,6 +508,10 @@ static abi_long freebsd_syscall(void *cpu_env, int
> num, abi_long arg1,
> >
> >       default:
> >           qemu_log_mask(LOG_UNIMP, "Unsupported syscall: %d\n", num);
> > +        if (bsd_user_strict) {
> > +            printf("Unimplemented system call %d\n", num);
> > +            abort();
> > +        }
>
> Still don't like the printf.  I suggested alternatives against v1.
>

Yea, this was unchanged in this version since there were too many other
things to do in the other bits that went into v2.

I'm thinking, though, that this isn't ready, so I'm going to drop it from
v3.

Warner

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 09/12] bsd-user: Start translation of arch-specific sysctls
  2023-02-14 21:36   ` Richard Henderson
@ 2023-02-16 22:57     ` Warner Losh
  0 siblings, 0 replies; 28+ messages in thread
From: Warner Losh @ 2023-02-16 22:57 UTC (permalink / raw)
  To: Richard Henderson
  Cc: qemu-devel, Alex Bennée, Paolo Bonzini, kevans, f4bug,
	Thomas Huth, Juergen Lock, Stacey Son

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

On Tue, Feb 14, 2023 at 2:36 PM Richard Henderson <
richard.henderson@linaro.org> wrote:

> On 2/13/23 14:27, Warner Losh wrote:
> > +        case HW_NCPU:
> > +            if (oldlen) {
> > +                (*(int32_t *)holdp) = tswap32(bsd_get_ncpu());
> > +            }
> > +            holdlen = sizeof(int32_t);
> > +            ret = 0;
> > +            goto out;
>
> Anything using SYSCTL_INT should use abi_int.
>
> > +#if defined(TARGET_ARM)
> > +        case HW_FLOATINGPT:
> > +            if (oldlen) {
> > +                ARMCPU *cpu = env_archcpu(env);
> > +                *(abi_int *)holdp = cpu_isar_feature(aa32_vfp, cpu);
> > +            }
> > +            holdlen = sizeof(int32_t);
>
> abi_int for consistency.
>
> Otherwise,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>

Makes sense.. Thanks!

Warner

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

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2023-02-16 22:58 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-14  0:27 [PATCH v2 00/12] 2023 Q1 bsd-user upstreaming: bugfixes and sysctl Warner Losh
2023-02-14  0:27 ` [PATCH v2 01/12] bsd-user: Don't truncate the return value from freebsd_syscall Warner Losh
2023-02-14  0:27 ` [PATCH v2 02/12] build: Don't specify -no-pie for --static user-mode programs Warner Losh
2023-02-14  0:27 ` [PATCH v2 03/12] bsd-user: Add sysarch syscall Warner Losh
2023-02-14  0:27 ` [PATCH v2 04/12] bsd-user: various helper routines for sysctl Warner Losh
2023-02-14 20:52   ` Richard Henderson
2023-02-14 21:31     ` Warner Losh
2023-02-14 21:39       ` Richard Henderson
2023-02-14  0:27 ` [PATCH v2 05/12] bsd-user: Helper routines oidfmt Warner Losh
2023-02-14 20:53   ` Richard Henderson
2023-02-14  0:27 ` [PATCH v2 06/12] bsd-user: Helper routines h2t_old_sysctl Warner Losh
2023-02-14 21:16   ` Richard Henderson
2023-02-15  5:58     ` Warner Losh
2023-02-14  0:27 ` [PATCH v2 07/12] bsd-user: sysctl helper funtions: sysctl_name2oid and sysctl_oidfmt Warner Losh
2023-02-14 21:18   ` Richard Henderson
2023-02-14  0:27 ` [PATCH v2 08/12] bsd-user: common routine do_freebsd_sysctl_oid for all sysctl variants Warner Losh
2023-02-14 21:21   ` Richard Henderson
2023-02-14  0:27 ` [PATCH v2 09/12] bsd-user: Start translation of arch-specific sysctls Warner Losh
2023-02-14 21:36   ` Richard Henderson
2023-02-16 22:57     ` Warner Losh
2023-02-14  0:27 ` [PATCH v2 10/12] bsd-user: do_freebsd_sysctl helper for sysctl(2) Warner Losh
2023-02-14 21:45   ` Richard Henderson
2023-02-14  0:27 ` [PATCH v2 11/12] bsd-user: implement sysctlbyname(2) Warner Losh
2023-02-14 21:48   ` Richard Henderson
2023-02-14  0:27 ` [PATCH v2 12/12] bsd-user: Add -strict Warner Losh
2023-02-14  6:57   ` Philippe Mathieu-Daudé
2023-02-14 21:49   ` Richard Henderson
2023-02-16 22:37     ` Warner Losh

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.