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->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

On Mon, Oct 28, 2019 at 7:08 PM Kyle Evans <kevans@freebsd.org> 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