* [RFC 1/4] linux-user: Add host_signal_set_pc to set pc in mcontext
2021-11-08 2:37 [RFC 0/4] linux-user: simplify safe signal handling Warner Losh
@ 2021-11-08 2:37 ` Warner Losh
2021-11-08 15:03 ` Richard Henderson
2021-11-10 15:43 ` Philippe Mathieu-Daudé
2021-11-08 2:37 ` [RFC 2/4] linux-user/signal.c: Create a common rewind_if_in_safe_syscall Warner Losh
` (2 subsequent siblings)
3 siblings, 2 replies; 16+ messages in thread
From: Warner Losh @ 2021-11-08 2:37 UTC (permalink / raw)
To: qemu-devel
Cc: Richard Henderson, Laurent Vivier, Warner Losh, Philippe Mathieu-Daude
Add a new function host_signal_set_pc to set the next pc in an
mcontext. The caller should ensure this is a valid PC for execution.
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
linux-user/host/aarch64/host-signal.h | 5 +++++
linux-user/host/alpha/host-signal.h | 5 +++++
linux-user/host/arm/host-signal.h | 5 +++++
linux-user/host/i386/host-signal.h | 5 +++++
linux-user/host/mips/host-signal.h | 5 +++++
linux-user/host/ppc/host-signal.h | 5 +++++
linux-user/host/riscv/host-signal.h | 5 +++++
linux-user/host/s390/host-signal.h | 5 +++++
linux-user/host/sparc/host-signal.h | 9 +++++++++
linux-user/host/x86_64/host-signal.h | 5 +++++
10 files changed, 54 insertions(+)
diff --git a/linux-user/host/aarch64/host-signal.h b/linux-user/host/aarch64/host-signal.h
index 0c0b08383a..9770b36dc1 100644
--- a/linux-user/host/aarch64/host-signal.h
+++ b/linux-user/host/aarch64/host-signal.h
@@ -35,6 +35,11 @@ static inline uintptr_t host_signal_pc(ucontext_t *uc)
return uc->uc_mcontext.pc;
}
+static inline void host_signal_set_pc(ucontext_t *uc, uintptr_t pc)
+{
+ uc->uc_mcontext.pc = pc;
+}
+
static inline bool host_signal_write(siginfo_t *info, ucontext_t *uc)
{
struct _aarch64_ctx *hdr;
diff --git a/linux-user/host/alpha/host-signal.h b/linux-user/host/alpha/host-signal.h
index e080be412f..f4c942948a 100644
--- a/linux-user/host/alpha/host-signal.h
+++ b/linux-user/host/alpha/host-signal.h
@@ -16,6 +16,11 @@ static inline uintptr_t host_signal_pc(ucontext_t *uc)
return uc->uc_mcontext.sc_pc;
}
+static inline void host_signal_set_pc(ucontext_t *uc, uintptr_t pc)
+{
+ uc->uc_mcontext.sc_pc = pc;
+}
+
static inline bool host_signal_write(siginfo_t *info, ucontext_t *uc)
{
uint32_t *pc = (uint32_t *)host_signal_pc(uc);
diff --git a/linux-user/host/arm/host-signal.h b/linux-user/host/arm/host-signal.h
index efb165c0c5..6c095773c0 100644
--- a/linux-user/host/arm/host-signal.h
+++ b/linux-user/host/arm/host-signal.h
@@ -16,6 +16,11 @@ static inline uintptr_t host_signal_pc(ucontext_t *uc)
return uc->uc_mcontext.arm_pc;
}
+static inline void host_signal_set_pc(ucontext_t *uc, uintptr_t pc)
+{
+ uc->uc_mcontext.arm_pc = pc;
+}
+
static inline bool host_signal_write(siginfo_t *info, ucontext_t *uc)
{
/*
diff --git a/linux-user/host/i386/host-signal.h b/linux-user/host/i386/host-signal.h
index 4c8eef99ce..abe1ece5c9 100644
--- a/linux-user/host/i386/host-signal.h
+++ b/linux-user/host/i386/host-signal.h
@@ -16,6 +16,11 @@ static inline uintptr_t host_signal_pc(ucontext_t *uc)
return uc->uc_mcontext.gregs[REG_EIP];
}
+static inline void host_signal_set_pc(ucontext_t *uc, uintptr_t pc)
+{
+ uc->uc_mcontext.gregs[REG_EIP] = pc;
+}
+
static inline bool host_signal_write(siginfo_t *info, ucontext_t *uc)
{
return uc->uc_mcontext.gregs[REG_TRAPNO] == 0xe
diff --git a/linux-user/host/mips/host-signal.h b/linux-user/host/mips/host-signal.h
index ef341f7c20..c666ed8c3f 100644
--- a/linux-user/host/mips/host-signal.h
+++ b/linux-user/host/mips/host-signal.h
@@ -16,6 +16,11 @@ static inline uintptr_t host_signal_pc(ucontext_t *uc)
return uc->uc_mcontext.pc;
}
+static inline void host_signal_set_pc(ucontext_t *uc, uintptr_t pc)
+{
+ uc->uc_mcontext.pc = pc;
+}
+
#if defined(__misp16) || defined(__mips_micromips)
#error "Unsupported encoding"
#endif
diff --git a/linux-user/host/ppc/host-signal.h b/linux-user/host/ppc/host-signal.h
index a491c413dc..1d8e658ff7 100644
--- a/linux-user/host/ppc/host-signal.h
+++ b/linux-user/host/ppc/host-signal.h
@@ -16,6 +16,11 @@ static inline uintptr_t host_signal_pc(ucontext_t *uc)
return uc->uc_mcontext.regs->nip;
}
+static inline void host_signal_set_pc(ucontext_t *uc, uintptr_t pc)
+{
+ uc->uc_mcontext.regs->nip = pc;
+}
+
static inline bool host_signal_write(siginfo_t *info, ucontext_t *uc)
{
return uc->uc_mcontext.regs->trap != 0x400
diff --git a/linux-user/host/riscv/host-signal.h b/linux-user/host/riscv/host-signal.h
index 3b168cb58b..a4f170efb0 100644
--- a/linux-user/host/riscv/host-signal.h
+++ b/linux-user/host/riscv/host-signal.h
@@ -16,6 +16,11 @@ static inline uintptr_t host_signal_pc(ucontext_t *uc)
return uc->uc_mcontext.__gregs[REG_PC];
}
+static inline void host_signal_set_pc(ucontext_t *uc, uintptr_t pc)
+{
+ uc->uc_mcontext.__gregs[REG_PC] = pc;
+}
+
static inline bool host_signal_write(siginfo_t *info, ucontext_t *uc)
{
/*
diff --git a/linux-user/host/s390/host-signal.h b/linux-user/host/s390/host-signal.h
index 26990e4893..a524f2ab00 100644
--- a/linux-user/host/s390/host-signal.h
+++ b/linux-user/host/s390/host-signal.h
@@ -16,6 +16,11 @@ static inline uintptr_t host_signal_pc(ucontext_t *uc)
return uc->uc_mcontext.psw.addr;
}
+static inline void host_signal_set_pc(ucontext_t *uc, uintptr_t pc)
+{
+ uc->uc_mcontext.psw.addr = pc;
+}
+
static inline bool host_signal_write(siginfo_t *info, ucontext_t *uc)
{
uint16_t *pinsn = (uint16_t *)host_signal_pc(uc);
diff --git a/linux-user/host/sparc/host-signal.h b/linux-user/host/sparc/host-signal.h
index 5e71d33f8e..e301c2c1ea 100644
--- a/linux-user/host/sparc/host-signal.h
+++ b/linux-user/host/sparc/host-signal.h
@@ -20,6 +20,15 @@ static inline uintptr_t host_signal_pc(ucontext_t *uc)
#endif
}
+static inline void host_signal_set_pc(ucontext_t *uc, uintptr_t pc)
+{
+#ifdef __arch64__
+ uc->uc_mcontext.mc_gregs[MC_PC] = pc;
+#else
+ &uc->uc_mcontext.gregs[REG_PC] = pc;
+#endif
+}
+
static inline bool host_signal_write(siginfo_t *info, ucontext_t *uc)
{
uint32_t insn = *(uint32_t *)host_signal_pc(uc);
diff --git a/linux-user/host/x86_64/host-signal.h b/linux-user/host/x86_64/host-signal.h
index 883d2fcf65..c71d597eb2 100644
--- a/linux-user/host/x86_64/host-signal.h
+++ b/linux-user/host/x86_64/host-signal.h
@@ -15,6 +15,11 @@ static inline uintptr_t host_signal_pc(ucontext_t *uc)
return uc->uc_mcontext.gregs[REG_RIP];
}
+static inline void host_signal_set_pc(ucontext_t *uc, uintptr_t pc)
+{
+ uc->uc_mcontext.gregs[REG_RIP] = pc;
+}
+
static inline bool host_signal_write(siginfo_t *info, ucontext_t *uc)
{
return uc->uc_mcontext.gregs[REG_TRAPNO] == 0xe
--
2.33.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [RFC 1/4] linux-user: Add host_signal_set_pc to set pc in mcontext
2021-11-08 2:37 ` [RFC 1/4] linux-user: Add host_signal_set_pc to set pc in mcontext Warner Losh
@ 2021-11-08 15:03 ` Richard Henderson
2021-11-10 15:43 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 16+ messages in thread
From: Richard Henderson @ 2021-11-08 15:03 UTC (permalink / raw)
To: Warner Losh, qemu-devel; +Cc: Laurent Vivier, Philippe Mathieu-Daude
On 11/8/21 3:37 AM, Warner Losh wrote:
> Add a new function host_signal_set_pc to set the next pc in an
> mcontext. The caller should ensure this is a valid PC for execution.
>
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
> linux-user/host/aarch64/host-signal.h | 5 +++++
> linux-user/host/alpha/host-signal.h | 5 +++++
> linux-user/host/arm/host-signal.h | 5 +++++
> linux-user/host/i386/host-signal.h | 5 +++++
> linux-user/host/mips/host-signal.h | 5 +++++
> linux-user/host/ppc/host-signal.h | 5 +++++
> linux-user/host/riscv/host-signal.h | 5 +++++
> linux-user/host/s390/host-signal.h | 5 +++++
> linux-user/host/sparc/host-signal.h | 9 +++++++++
> linux-user/host/x86_64/host-signal.h | 5 +++++
> 10 files changed, 54 insertions(+)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC 1/4] linux-user: Add host_signal_set_pc to set pc in mcontext
2021-11-08 2:37 ` [RFC 1/4] linux-user: Add host_signal_set_pc to set pc in mcontext Warner Losh
2021-11-08 15:03 ` Richard Henderson
@ 2021-11-10 15:43 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-10 15:43 UTC (permalink / raw)
To: Warner Losh, qemu-devel; +Cc: Richard Henderson, Laurent Vivier
On 11/8/21 03:37, Warner Losh wrote:
> Add a new function host_signal_set_pc to set the next pc in an
> mcontext. The caller should ensure this is a valid PC for execution.
>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
> linux-user/host/aarch64/host-signal.h | 5 +++++
> linux-user/host/alpha/host-signal.h | 5 +++++
> linux-user/host/arm/host-signal.h | 5 +++++
> linux-user/host/i386/host-signal.h | 5 +++++
> linux-user/host/mips/host-signal.h | 5 +++++
> linux-user/host/ppc/host-signal.h | 5 +++++
> linux-user/host/riscv/host-signal.h | 5 +++++
> linux-user/host/s390/host-signal.h | 5 +++++
> linux-user/host/sparc/host-signal.h | 9 +++++++++
> linux-user/host/x86_64/host-signal.h | 5 +++++
> 10 files changed, 54 insertions(+)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [RFC 2/4] linux-user/signal.c: Create a common rewind_if_in_safe_syscall
2021-11-08 2:37 [RFC 0/4] linux-user: simplify safe signal handling Warner Losh
2021-11-08 2:37 ` [RFC 1/4] linux-user: Add host_signal_set_pc to set pc in mcontext Warner Losh
@ 2021-11-08 2:37 ` Warner Losh
2021-11-08 15:07 ` Richard Henderson
2021-11-10 15:44 ` Philippe Mathieu-Daudé
2021-11-08 2:37 ` [RFC 3/4] linux-user/safe-syscall.inc.S: Move to common-user Warner Losh
2021-11-08 2:37 ` [RFC 4/4] common-user: Allow return codes to be adjusted after sytsem call Warner Losh
3 siblings, 2 replies; 16+ messages in thread
From: Warner Losh @ 2021-11-08 2:37 UTC (permalink / raw)
To: qemu-devel
Cc: Richard Henderson, Laurent Vivier, Warner Losh, Philippe Mathieu-Daude
All instances of rewind_if_in_safe_syscall are the same, differing only
in how the instruction point is fetched from the ucontext and the size
of the registers. Use host_signal_pc and new host_signal_set_pc
interfaces to fetch the pointer to the PC and adjust if needed. Delete
all the old copies of rewind_if_in_safe_syscall.
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
linux-user/host/aarch64/hostdep.h | 20 --------------------
linux-user/host/arm/hostdep.h | 20 --------------------
linux-user/host/i386/hostdep.h | 20 --------------------
linux-user/host/ppc64/hostdep.h | 20 --------------------
linux-user/host/riscv/hostdep.h | 20 --------------------
linux-user/host/s390x/hostdep.h | 20 --------------------
linux-user/host/x86_64/hostdep.h | 20 --------------------
linux-user/signal.c | 18 +++++++++++++++++-
8 files changed, 17 insertions(+), 141 deletions(-)
diff --git a/linux-user/host/aarch64/hostdep.h b/linux-user/host/aarch64/hostdep.h
index a8d41a21ad..39299d798a 100644
--- a/linux-user/host/aarch64/hostdep.h
+++ b/linux-user/host/aarch64/hostdep.h
@@ -15,24 +15,4 @@
/* We have a safe-syscall.inc.S */
#define HAVE_SAFE_SYSCALL
-#ifndef __ASSEMBLER__
-
-/* These are defined by the safe-syscall.inc.S file */
-extern char safe_syscall_start[];
-extern char safe_syscall_end[];
-
-/* 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;
- __u64 *pcreg = &uc->uc_mcontext.pc;
-
- if (*pcreg > (uintptr_t)safe_syscall_start
- && *pcreg < (uintptr_t)safe_syscall_end) {
- *pcreg = (uintptr_t)safe_syscall_start;
- }
-}
-
-#endif /* __ASSEMBLER__ */
-
#endif
diff --git a/linux-user/host/arm/hostdep.h b/linux-user/host/arm/hostdep.h
index 9276fe6ceb..86b137875a 100644
--- a/linux-user/host/arm/hostdep.h
+++ b/linux-user/host/arm/hostdep.h
@@ -15,24 +15,4 @@
/* We have a safe-syscall.inc.S */
#define HAVE_SAFE_SYSCALL
-#ifndef __ASSEMBLER__
-
-/* These are defined by the safe-syscall.inc.S file */
-extern char safe_syscall_start[];
-extern char safe_syscall_end[];
-
-/* 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;
- unsigned long *pcreg = &uc->uc_mcontext.arm_pc;
-
- if (*pcreg > (uintptr_t)safe_syscall_start
- && *pcreg < (uintptr_t)safe_syscall_end) {
- *pcreg = (uintptr_t)safe_syscall_start;
- }
-}
-
-#endif /* __ASSEMBLER__ */
-
#endif
diff --git a/linux-user/host/i386/hostdep.h b/linux-user/host/i386/hostdep.h
index 073be74d87..ce7136501f 100644
--- a/linux-user/host/i386/hostdep.h
+++ b/linux-user/host/i386/hostdep.h
@@ -15,24 +15,4 @@
/* We have a safe-syscall.inc.S */
#define HAVE_SAFE_SYSCALL
-#ifndef __ASSEMBLER__
-
-/* These are defined by the safe-syscall.inc.S file */
-extern char safe_syscall_start[];
-extern char safe_syscall_end[];
-
-/* 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_EIP];
-
- if (*pcreg > (uintptr_t)safe_syscall_start
- && *pcreg < (uintptr_t)safe_syscall_end) {
- *pcreg = (uintptr_t)safe_syscall_start;
- }
-}
-
-#endif /* __ASSEMBLER__ */
-
#endif
diff --git a/linux-user/host/ppc64/hostdep.h b/linux-user/host/ppc64/hostdep.h
index 98979ad917..0c290dd904 100644
--- a/linux-user/host/ppc64/hostdep.h
+++ b/linux-user/host/ppc64/hostdep.h
@@ -15,24 +15,4 @@
/* We have a safe-syscall.inc.S */
#define HAVE_SAFE_SYSCALL
-#ifndef __ASSEMBLER__
-
-/* These are defined by the safe-syscall.inc.S file */
-extern char safe_syscall_start[];
-extern char safe_syscall_end[];
-
-/* 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;
- unsigned long *pcreg = &uc->uc_mcontext.gp_regs[PT_NIP];
-
- if (*pcreg > (uintptr_t)safe_syscall_start
- && *pcreg < (uintptr_t)safe_syscall_end) {
- *pcreg = (uintptr_t)safe_syscall_start;
- }
-}
-
-#endif /* __ASSEMBLER__ */
-
#endif
diff --git a/linux-user/host/riscv/hostdep.h b/linux-user/host/riscv/hostdep.h
index 2ba07456ae..7f67c22868 100644
--- a/linux-user/host/riscv/hostdep.h
+++ b/linux-user/host/riscv/hostdep.h
@@ -11,24 +11,4 @@
/* We have a safe-syscall.inc.S */
#define HAVE_SAFE_SYSCALL
-#ifndef __ASSEMBLER__
-
-/* These are defined by the safe-syscall.inc.S file */
-extern char safe_syscall_start[];
-extern char safe_syscall_end[];
-
-/* 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;
- unsigned long *pcreg = &uc->uc_mcontext.__gregs[REG_PC];
-
- if (*pcreg > (uintptr_t)safe_syscall_start
- && *pcreg < (uintptr_t)safe_syscall_end) {
- *pcreg = (uintptr_t)safe_syscall_start;
- }
-}
-
-#endif /* __ASSEMBLER__ */
-
#endif
diff --git a/linux-user/host/s390x/hostdep.h b/linux-user/host/s390x/hostdep.h
index 4f0171f36f..d801145854 100644
--- a/linux-user/host/s390x/hostdep.h
+++ b/linux-user/host/s390x/hostdep.h
@@ -15,24 +15,4 @@
/* We have a safe-syscall.inc.S */
#define HAVE_SAFE_SYSCALL
-#ifndef __ASSEMBLER__
-
-/* These are defined by the safe-syscall.inc.S file */
-extern char safe_syscall_start[];
-extern char safe_syscall_end[];
-
-/* 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;
- unsigned long *pcreg = &uc->uc_mcontext.psw.addr;
-
- if (*pcreg > (uintptr_t)safe_syscall_start
- && *pcreg < (uintptr_t)safe_syscall_end) {
- *pcreg = (uintptr_t)safe_syscall_start;
- }
-}
-
-#endif /* __ASSEMBLER__ */
-
#endif
diff --git a/linux-user/host/x86_64/hostdep.h b/linux-user/host/x86_64/hostdep.h
index a4fefb5114..9c62bd26bd 100644
--- a/linux-user/host/x86_64/hostdep.h
+++ b/linux-user/host/x86_64/hostdep.h
@@ -15,24 +15,4 @@
/* We have a safe-syscall.inc.S */
#define HAVE_SAFE_SYSCALL
-#ifndef __ASSEMBLER__
-
-/* These are defined by the safe-syscall.inc.S file */
-extern char safe_syscall_start[];
-extern char safe_syscall_end[];
-
-/* 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];
-
- if (*pcreg > (uintptr_t)safe_syscall_start
- && *pcreg < (uintptr_t)safe_syscall_end) {
- *pcreg = (uintptr_t)safe_syscall_start;
- }
-}
-
-#endif /* __ASSEMBLER__ */
-
#endif
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 81c45bfce9..dafdf46b93 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -793,7 +793,23 @@ int queue_signal(CPUArchState *env, int sig, int si_type,
return 1; /* indicates that the signal was queued */
}
-#ifndef HAVE_SAFE_SYSCALL
+#ifdef HAVE_SAFE_SYSCALL
+/* These are defined by the safe-syscall.inc.S file */
+extern char safe_syscall_start[];
+extern char safe_syscall_end[];
+
+/* 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 = (ucontext_t *)puc;
+ uintptr_t pcreg = host_signal_pc(uc);
+
+ if (pcreg > (uintptr_t)safe_syscall_start
+ && pcreg < (uintptr_t)safe_syscall_end) {
+ host_signal_set_pc(uc, (uintptr_t)safe_syscall_start);
+ }
+}
+#else
static inline void rewind_if_in_safe_syscall(void *puc)
{
/* Default version: never rewind */
--
2.33.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [RFC 2/4] linux-user/signal.c: Create a common rewind_if_in_safe_syscall
2021-11-08 2:37 ` [RFC 2/4] linux-user/signal.c: Create a common rewind_if_in_safe_syscall Warner Losh
@ 2021-11-08 15:07 ` Richard Henderson
2021-11-08 16:39 ` Warner Losh
2021-11-10 15:44 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 16+ messages in thread
From: Richard Henderson @ 2021-11-08 15:07 UTC (permalink / raw)
To: Warner Losh, qemu-devel; +Cc: Laurent Vivier, Philippe Mathieu-Daude
On 11/8/21 3:37 AM, Warner Losh wrote:
> All instances of rewind_if_in_safe_syscall are the same, differing only
> in how the instruction point is fetched from the ucontext and the size
> of the registers. Use host_signal_pc and new host_signal_set_pc
> interfaces to fetch the pointer to the PC and adjust if needed. Delete
> all the old copies of rewind_if_in_safe_syscall.
>
> Signed-off-by: Warner Losh<imp@bsdimp.com>
> ---
> linux-user/host/aarch64/hostdep.h | 20 --------------------
> linux-user/host/arm/hostdep.h | 20 --------------------
> linux-user/host/i386/hostdep.h | 20 --------------------
> linux-user/host/ppc64/hostdep.h | 20 --------------------
> linux-user/host/riscv/hostdep.h | 20 --------------------
> linux-user/host/s390x/hostdep.h | 20 --------------------
> linux-user/host/x86_64/hostdep.h | 20 --------------------
> linux-user/signal.c | 18 +++++++++++++++++-
> 8 files changed, 17 insertions(+), 141 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Although I think we can fairly safely drop HAVE_SAFE_SYSCALL. It is required for proper
operation. As with host-signal.h, really.
r~
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC 2/4] linux-user/signal.c: Create a common rewind_if_in_safe_syscall
2021-11-08 15:07 ` Richard Henderson
@ 2021-11-08 16:39 ` Warner Losh
2021-11-10 16:20 ` Warner Losh
0 siblings, 1 reply; 16+ messages in thread
From: Warner Losh @ 2021-11-08 16:39 UTC (permalink / raw)
To: Richard Henderson; +Cc: Philippe Mathieu-Daude, QEMU Developers, Laurent Vivier
[-- Attachment #1: Type: text/plain, Size: 1516 bytes --]
On Mon, Nov 8, 2021 at 8:07 AM Richard Henderson <
richard.henderson@linaro.org> wrote:
> On 11/8/21 3:37 AM, Warner Losh wrote:
> > All instances of rewind_if_in_safe_syscall are the same, differing only
> > in how the instruction point is fetched from the ucontext and the size
> > of the registers. Use host_signal_pc and new host_signal_set_pc
> > interfaces to fetch the pointer to the PC and adjust if needed. Delete
> > all the old copies of rewind_if_in_safe_syscall.
> >
> > Signed-off-by: Warner Losh<imp@bsdimp.com>
> > ---
> > linux-user/host/aarch64/hostdep.h | 20 --------------------
> > linux-user/host/arm/hostdep.h | 20 --------------------
> > linux-user/host/i386/hostdep.h | 20 --------------------
> > linux-user/host/ppc64/hostdep.h | 20 --------------------
> > linux-user/host/riscv/hostdep.h | 20 --------------------
> > linux-user/host/s390x/hostdep.h | 20 --------------------
> > linux-user/host/x86_64/hostdep.h | 20 --------------------
> > linux-user/signal.c | 18 +++++++++++++++++-
> > 8 files changed, 17 insertions(+), 141 deletions(-)
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
> Although I think we can fairly safely drop HAVE_SAFE_SYSCALL. It is
> required for proper
> operation. As with host-signal.h, really.
>
Yes. The only possible use I can see for it is to allow people to bring up
new platforms more easily by deferring the signal-safe system call details
until later in that process.
Warner
[-- Attachment #2: Type: text/html, Size: 2166 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC 2/4] linux-user/signal.c: Create a common rewind_if_in_safe_syscall
2021-11-08 16:39 ` Warner Losh
@ 2021-11-10 16:20 ` Warner Losh
2021-11-10 16:31 ` Richard Henderson
0 siblings, 1 reply; 16+ messages in thread
From: Warner Losh @ 2021-11-10 16:20 UTC (permalink / raw)
To: Richard Henderson; +Cc: Philippe Mathieu-Daude, QEMU Developers, Laurent Vivier
[-- Attachment #1: Type: text/plain, Size: 1779 bytes --]
On Mon, Nov 8, 2021 at 9:39 AM Warner Losh <imp@bsdimp.com> wrote:
>
>
> On Mon, Nov 8, 2021 at 8:07 AM Richard Henderson <
> richard.henderson@linaro.org> wrote:
>
>> On 11/8/21 3:37 AM, Warner Losh wrote:
>> > All instances of rewind_if_in_safe_syscall are the same, differing only
>> > in how the instruction point is fetched from the ucontext and the size
>> > of the registers. Use host_signal_pc and new host_signal_set_pc
>> > interfaces to fetch the pointer to the PC and adjust if needed. Delete
>> > all the old copies of rewind_if_in_safe_syscall.
>> >
>> > Signed-off-by: Warner Losh<imp@bsdimp.com>
>> > ---
>> > linux-user/host/aarch64/hostdep.h | 20 --------------------
>> > linux-user/host/arm/hostdep.h | 20 --------------------
>> > linux-user/host/i386/hostdep.h | 20 --------------------
>> > linux-user/host/ppc64/hostdep.h | 20 --------------------
>> > linux-user/host/riscv/hostdep.h | 20 --------------------
>> > linux-user/host/s390x/hostdep.h | 20 --------------------
>> > linux-user/host/x86_64/hostdep.h | 20 --------------------
>> > linux-user/signal.c | 18 +++++++++++++++++-
>> > 8 files changed, 17 insertions(+), 141 deletions(-)
>>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>>
>> Although I think we can fairly safely drop HAVE_SAFE_SYSCALL. It is
>> required for proper
>> operation. As with host-signal.h, really.
>>
>
> Yes. The only possible use I can see for it is to allow people to bring up
> new platforms more easily by deferring the signal-safe system call details
> until later in that process.
>
If we do, we'd need to remove the linux-user on a mips host tests from the
CI pipeline. Those are the only ones left that don't use this that we test.
Warner
[-- Attachment #2: Type: text/html, Size: 2735 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC 2/4] linux-user/signal.c: Create a common rewind_if_in_safe_syscall
2021-11-10 16:20 ` Warner Losh
@ 2021-11-10 16:31 ` Richard Henderson
0 siblings, 0 replies; 16+ messages in thread
From: Richard Henderson @ 2021-11-10 16:31 UTC (permalink / raw)
To: Warner Losh; +Cc: Philippe Mathieu-Daude, QEMU Developers, Laurent Vivier
On 11/10/21 5:20 PM, Warner Losh wrote:
> Although I think we can fairly safely drop HAVE_SAFE_SYSCALL. It is required for
> proper
> operation. As with host-signal.h, really.
>
> Yes. The only possible use I can see for it is to allow people to bring up new
> platforms more easily by deferring the signal-safe system call details until later in
> that process.
>
> If we do, we'd need to remove the linux-user on a mips host tests from the CI pipeline.
> Those are the only ones left that don't use this that we test.
Ack, thanks. I'll take care of this next release.
r~
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC 2/4] linux-user/signal.c: Create a common rewind_if_in_safe_syscall
2021-11-08 2:37 ` [RFC 2/4] linux-user/signal.c: Create a common rewind_if_in_safe_syscall Warner Losh
2021-11-08 15:07 ` Richard Henderson
@ 2021-11-10 15:44 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-10 15:44 UTC (permalink / raw)
To: Warner Losh, qemu-devel; +Cc: Richard Henderson, Laurent Vivier
On 11/8/21 03:37, Warner Losh wrote:
> All instances of rewind_if_in_safe_syscall are the same, differing only
> in how the instruction point is fetched from the ucontext and the size
> of the registers. Use host_signal_pc and new host_signal_set_pc
> interfaces to fetch the pointer to the PC and adjust if needed. Delete
> all the old copies of rewind_if_in_safe_syscall.
>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
> linux-user/host/aarch64/hostdep.h | 20 --------------------
> linux-user/host/arm/hostdep.h | 20 --------------------
> linux-user/host/i386/hostdep.h | 20 --------------------
> linux-user/host/ppc64/hostdep.h | 20 --------------------
> linux-user/host/riscv/hostdep.h | 20 --------------------
> linux-user/host/s390x/hostdep.h | 20 --------------------
> linux-user/host/x86_64/hostdep.h | 20 --------------------
> linux-user/signal.c | 18 +++++++++++++++++-
> 8 files changed, 17 insertions(+), 141 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [RFC 3/4] linux-user/safe-syscall.inc.S: Move to common-user
2021-11-08 2:37 [RFC 0/4] linux-user: simplify safe signal handling Warner Losh
2021-11-08 2:37 ` [RFC 1/4] linux-user: Add host_signal_set_pc to set pc in mcontext Warner Losh
2021-11-08 2:37 ` [RFC 2/4] linux-user/signal.c: Create a common rewind_if_in_safe_syscall Warner Losh
@ 2021-11-08 2:37 ` Warner Losh
2021-11-08 2:37 ` [RFC 4/4] common-user: Allow return codes to be adjusted after sytsem call Warner Losh
3 siblings, 0 replies; 16+ messages in thread
From: Warner Losh @ 2021-11-08 2:37 UTC (permalink / raw)
To: qemu-devel
Cc: Richard Henderson, Laurent Vivier, Warner Losh, Philippe Mathieu-Daude
Move all the safe_syscall.inc.S files to common-user. They are almost
identical between linux-user and bsd-user to re-use.
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
{linux-user => common-user}/host/aarch64/safe-syscall.inc.S | 0
{linux-user => common-user}/host/arm/safe-syscall.inc.S | 0
{linux-user => common-user}/host/i386/safe-syscall.inc.S | 0
{linux-user => common-user}/host/ppc64/safe-syscall.inc.S | 0
{linux-user => common-user}/host/riscv/safe-syscall.inc.S | 0
{linux-user => common-user}/host/s390x/safe-syscall.inc.S | 0
{linux-user => common-user}/host/x86_64/safe-syscall.inc.S | 0
meson.build | 1 +
8 files changed, 1 insertion(+)
rename {linux-user => common-user}/host/aarch64/safe-syscall.inc.S (100%)
rename {linux-user => common-user}/host/arm/safe-syscall.inc.S (100%)
rename {linux-user => common-user}/host/i386/safe-syscall.inc.S (100%)
rename {linux-user => common-user}/host/ppc64/safe-syscall.inc.S (100%)
rename {linux-user => common-user}/host/riscv/safe-syscall.inc.S (100%)
rename {linux-user => common-user}/host/s390x/safe-syscall.inc.S (100%)
rename {linux-user => common-user}/host/x86_64/safe-syscall.inc.S (100%)
diff --git a/linux-user/host/aarch64/safe-syscall.inc.S b/common-user/host/aarch64/safe-syscall.inc.S
similarity index 100%
rename from linux-user/host/aarch64/safe-syscall.inc.S
rename to common-user/host/aarch64/safe-syscall.inc.S
diff --git a/linux-user/host/arm/safe-syscall.inc.S b/common-user/host/arm/safe-syscall.inc.S
similarity index 100%
rename from linux-user/host/arm/safe-syscall.inc.S
rename to common-user/host/arm/safe-syscall.inc.S
diff --git a/linux-user/host/i386/safe-syscall.inc.S b/common-user/host/i386/safe-syscall.inc.S
similarity index 100%
rename from linux-user/host/i386/safe-syscall.inc.S
rename to common-user/host/i386/safe-syscall.inc.S
diff --git a/linux-user/host/ppc64/safe-syscall.inc.S b/common-user/host/ppc64/safe-syscall.inc.S
similarity index 100%
rename from linux-user/host/ppc64/safe-syscall.inc.S
rename to common-user/host/ppc64/safe-syscall.inc.S
diff --git a/linux-user/host/riscv/safe-syscall.inc.S b/common-user/host/riscv/safe-syscall.inc.S
similarity index 100%
rename from linux-user/host/riscv/safe-syscall.inc.S
rename to common-user/host/riscv/safe-syscall.inc.S
diff --git a/linux-user/host/s390x/safe-syscall.inc.S b/common-user/host/s390x/safe-syscall.inc.S
similarity index 100%
rename from linux-user/host/s390x/safe-syscall.inc.S
rename to common-user/host/s390x/safe-syscall.inc.S
diff --git a/linux-user/host/x86_64/safe-syscall.inc.S b/common-user/host/x86_64/safe-syscall.inc.S
similarity index 100%
rename from linux-user/host/x86_64/safe-syscall.inc.S
rename to common-user/host/x86_64/safe-syscall.inc.S
diff --git a/meson.build b/meson.build
index 26c58123e9..6f7acc8936 100644
--- a/meson.build
+++ b/meson.build
@@ -2883,6 +2883,7 @@ foreach target : target_dirs
if 'CONFIG_LINUX_USER' in config_target
base_dir = 'linux-user'
target_inc += include_directories('linux-user/host/' / config_host['ARCH'])
+ target_inc += include_directories('common-user/host/' / config_host['ARCH'])
endif
if 'CONFIG_BSD_USER' in config_target
base_dir = 'bsd-user'
--
2.33.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [RFC 4/4] common-user: Allow return codes to be adjusted after sytsem call
2021-11-08 2:37 [RFC 0/4] linux-user: simplify safe signal handling Warner Losh
` (2 preceding siblings ...)
2021-11-08 2:37 ` [RFC 3/4] linux-user/safe-syscall.inc.S: Move to common-user Warner Losh
@ 2021-11-08 2:37 ` Warner Losh
2021-11-08 15:10 ` Richard Henderson
3 siblings, 1 reply; 16+ messages in thread
From: Warner Losh @ 2021-11-08 2:37 UTC (permalink / raw)
To: qemu-devel
Cc: Richard Henderson, Laurent Vivier, Warner Losh, Philippe Mathieu-Daude
All the *-users generally use the Linux style of negative return codes
for errno. However, other systems, like FreeBSD, have a different
convention. Allow those systems to insert code after the syscall that
adjusts the return value of the system call to match the native linux
format.
Signed-off-by: Warner Losh <imp@bsdimp.com>
---
common-user/host/aarch64/safe-syscall.inc.S | 1 +
common-user/host/arm/safe-syscall.inc.S | 1 +
common-user/host/i386/safe-syscall.inc.S | 1 +
common-user/host/ppc64/safe-syscall.inc.S | 1 +
common-user/host/riscv/safe-syscall.inc.S | 1 +
common-user/host/s390x/safe-syscall.inc.S | 1 +
common-user/host/x86_64/safe-syscall.inc.S | 1 +
linux-user/safe-syscall.S | 1 +
8 files changed, 8 insertions(+)
diff --git a/common-user/host/aarch64/safe-syscall.inc.S b/common-user/host/aarch64/safe-syscall.inc.S
index bc1f5a9792..81d83e8e79 100644
--- a/common-user/host/aarch64/safe-syscall.inc.S
+++ b/common-user/host/aarch64/safe-syscall.inc.S
@@ -64,6 +64,7 @@ safe_syscall_start:
svc 0x0
safe_syscall_end:
/* code path for having successfully executed the syscall */
+ ADJUST_SYSCALL_RETCODE
ret
0:
diff --git a/common-user/host/arm/safe-syscall.inc.S b/common-user/host/arm/safe-syscall.inc.S
index 88c4958504..40e9a5e28d 100644
--- a/common-user/host/arm/safe-syscall.inc.S
+++ b/common-user/host/arm/safe-syscall.inc.S
@@ -78,6 +78,7 @@ safe_syscall_start:
swi 0
safe_syscall_end:
/* code path for having successfully executed the syscall */
+ ADJUST_SYSCALL_RETCODE
pop { r4, r5, r6, r7, r8, pc }
1:
diff --git a/common-user/host/i386/safe-syscall.inc.S b/common-user/host/i386/safe-syscall.inc.S
index 9e58fc6504..eb6b43bd81 100644
--- a/common-user/host/i386/safe-syscall.inc.S
+++ b/common-user/host/i386/safe-syscall.inc.S
@@ -75,6 +75,7 @@ safe_syscall_start:
int $0x80
safe_syscall_end:
/* code path for having successfully executed the syscall */
+ ADJUST_SYSCALL_RETCODE
pop %ebx
.cfi_remember_state
.cfi_adjust_cfa_offset -4
diff --git a/common-user/host/ppc64/safe-syscall.inc.S b/common-user/host/ppc64/safe-syscall.inc.S
index 875133173b..974bd03f8d 100644
--- a/common-user/host/ppc64/safe-syscall.inc.S
+++ b/common-user/host/ppc64/safe-syscall.inc.S
@@ -75,6 +75,7 @@ safe_syscall_start:
sc
safe_syscall_end:
/* code path when we did execute the syscall */
+ ADJUST_SYSCALL_RETCODE
ld 14, 16(1) /* restore r14 to its original value */
bnslr+
diff --git a/common-user/host/riscv/safe-syscall.inc.S b/common-user/host/riscv/safe-syscall.inc.S
index 9ca3fbfd1e..a4bd5c5c72 100644
--- a/common-user/host/riscv/safe-syscall.inc.S
+++ b/common-user/host/riscv/safe-syscall.inc.S
@@ -66,6 +66,7 @@ safe_syscall_start:
scall
safe_syscall_end:
/* code path for having successfully executed the syscall */
+ ADJUST_SYSCALL_RETCODE
ret
0:
diff --git a/common-user/host/s390x/safe-syscall.inc.S b/common-user/host/s390x/safe-syscall.inc.S
index 414b44ad38..4ba60fbed0 100644
--- a/common-user/host/s390x/safe-syscall.inc.S
+++ b/common-user/host/s390x/safe-syscall.inc.S
@@ -76,6 +76,7 @@ safe_syscall_start:
jne 2f
svc 0
safe_syscall_end:
+ ADJUST_SYSCALL_RETCODE
1: lg %r15,0(%r15) /* load back chain */
.cfi_remember_state
diff --git a/common-user/host/x86_64/safe-syscall.inc.S b/common-user/host/x86_64/safe-syscall.inc.S
index f36992daa3..e1ae6f83e6 100644
--- a/common-user/host/x86_64/safe-syscall.inc.S
+++ b/common-user/host/x86_64/safe-syscall.inc.S
@@ -72,6 +72,7 @@ safe_syscall_start:
syscall
safe_syscall_end:
/* code path for having successfully executed the syscall */
+ ADJUST_SYSCALL_RETCODE
pop %rbp
.cfi_remember_state
.cfi_def_cfa_offset 8
diff --git a/linux-user/safe-syscall.S b/linux-user/safe-syscall.S
index 42ea7c40ba..0d6dd19398 100644
--- a/linux-user/safe-syscall.S
+++ b/linux-user/safe-syscall.S
@@ -17,6 +17,7 @@
* so that this will pull in the right fragment for the architecture.
*/
#ifdef HAVE_SAFE_SYSCALL
+#define ADJUST_SYSCALL_RETCODE /* No adjustment for linux */
#include "safe-syscall.inc.S"
#endif
--
2.33.0
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [RFC 4/4] common-user: Allow return codes to be adjusted after sytsem call
2021-11-08 2:37 ` [RFC 4/4] common-user: Allow return codes to be adjusted after sytsem call Warner Losh
@ 2021-11-08 15:10 ` Richard Henderson
2021-11-08 18:49 ` Warner Losh
0 siblings, 1 reply; 16+ messages in thread
From: Richard Henderson @ 2021-11-08 15:10 UTC (permalink / raw)
To: Warner Losh, qemu-devel; +Cc: Laurent Vivier, Philippe Mathieu-Daude
On 11/8/21 3:37 AM, Warner Losh wrote:
> All the *-users generally use the Linux style of negative return codes
> for errno. However, other systems, like FreeBSD, have a different
> convention. Allow those systems to insert code after the syscall that
> adjusts the return value of the system call to match the native linux
> format.
>
> Signed-off-by: Warner Losh <imp@bsdimp.com>
> ---
> common-user/host/aarch64/safe-syscall.inc.S | 1 +
> common-user/host/arm/safe-syscall.inc.S | 1 +
> common-user/host/i386/safe-syscall.inc.S | 1 +
> common-user/host/ppc64/safe-syscall.inc.S | 1 +
> common-user/host/riscv/safe-syscall.inc.S | 1 +
> common-user/host/s390x/safe-syscall.inc.S | 1 +
> common-user/host/x86_64/safe-syscall.inc.S | 1 +
> linux-user/safe-syscall.S | 1 +
> 8 files changed, 8 insertions(+)
>
> diff --git a/common-user/host/aarch64/safe-syscall.inc.S b/common-user/host/aarch64/safe-syscall.inc.S
> index bc1f5a9792..81d83e8e79 100644
> --- a/common-user/host/aarch64/safe-syscall.inc.S
> +++ b/common-user/host/aarch64/safe-syscall.inc.S
> @@ -64,6 +64,7 @@ safe_syscall_start:
> svc 0x0
> safe_syscall_end:
> /* code path for having successfully executed the syscall */
> + ADJUST_SYSCALL_RETCODE
> ret
>
> 0:
Not sure about this, really. Is it really that much cleaner to insert this than create
separate 10-line files, with the adjustment included?
r~
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC 4/4] common-user: Allow return codes to be adjusted after sytsem call
2021-11-08 15:10 ` Richard Henderson
@ 2021-11-08 18:49 ` Warner Losh
2021-11-09 8:11 ` Richard Henderson
0 siblings, 1 reply; 16+ messages in thread
From: Warner Losh @ 2021-11-08 18:49 UTC (permalink / raw)
To: Richard Henderson; +Cc: Philippe Mathieu-Daude, QEMU Developers, Laurent Vivier
[-- Attachment #1: Type: text/plain, Size: 3394 bytes --]
On Mon, Nov 8, 2021 at 8:10 AM Richard Henderson <
richard.henderson@linaro.org> wrote:
> On 11/8/21 3:37 AM, Warner Losh wrote:
> > All the *-users generally use the Linux style of negative return codes
> > for errno. However, other systems, like FreeBSD, have a different
> > convention. Allow those systems to insert code after the syscall that
> > adjusts the return value of the system call to match the native linux
> > format.
> >
> > Signed-off-by: Warner Losh <imp@bsdimp.com>
> > ---
> > common-user/host/aarch64/safe-syscall.inc.S | 1 +
> > common-user/host/arm/safe-syscall.inc.S | 1 +
> > common-user/host/i386/safe-syscall.inc.S | 1 +
> > common-user/host/ppc64/safe-syscall.inc.S | 1 +
> > common-user/host/riscv/safe-syscall.inc.S | 1 +
> > common-user/host/s390x/safe-syscall.inc.S | 1 +
> > common-user/host/x86_64/safe-syscall.inc.S | 1 +
> > linux-user/safe-syscall.S | 1 +
> > 8 files changed, 8 insertions(+)
> >
> > diff --git a/common-user/host/aarch64/safe-syscall.inc.S
> b/common-user/host/aarch64/safe-syscall.inc.S
> > index bc1f5a9792..81d83e8e79 100644
> > --- a/common-user/host/aarch64/safe-syscall.inc.S
> > +++ b/common-user/host/aarch64/safe-syscall.inc.S
> > @@ -64,6 +64,7 @@ safe_syscall_start:
> > svc 0x0
> > safe_syscall_end:
> > /* code path for having successfully executed the syscall */
> > + ADJUST_SYSCALL_RETCODE
> > ret
> >
> > 0:
>
> Not sure about this, really. Is it really that much cleaner to insert
> this than create
> separate 10-line files, with the adjustment included?
>
While the meat of these files is only around 10 lines, the files themselves
are more like 70 lines with a lot of extra marking to ensure proper
integration with toolchains. Such lines have a tendency, over time, to need
adjusting as new toolchains change the requirements slightly (clang
certainly has forced that in a number of places in FreeBSD's code base, and
every new version seems to require something). The adjustments have all
been 3 lines (gmail seems to hate my formatting):
+#define ADJUST_SYSCALL_RETCODE \
+ jnb 2f; \
+ neg %rax; \
+ 2:
which is significantly easier to maintain than having to monitor these
files for changes and copying over the changes that happen. Plus, as I'm
upstreaming the arch support, it's one more file I have to include in the
review, it's one more file that may get questions and advice I'll have to
answer with 'it's a verbatim copy of the linux version with these three
lines added, why do I need to make those stylistic changes'. All of which
takes extra time. The upstreaming is going much more slowly than I'd
anticipated (but on the up-side also finding more problems than I thought
were latent in the system), and we're still at about 30k lines (after
starting at about 36k lines, though some of that is due to deleting mips).
It's been running about a month per 1000 lines to get reviewed and
upstreamed. There's about 626 lines in these 6 files, so sharing them
seemed like it would save me a few weeks of upstreaming time as well
(though I'll be the first to admit that translating LoC metrics to time has
a dubious history).
The other alternative I considered was having a #ifdef __FreeBSD__ ..
#endif in all those files, but I thought that even more intrusive.
Warner
[-- Attachment #2: Type: text/html, Size: 4225 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC 4/4] common-user: Allow return codes to be adjusted after sytsem call
2021-11-08 18:49 ` Warner Losh
@ 2021-11-09 8:11 ` Richard Henderson
2021-11-10 2:10 ` Warner Losh
0 siblings, 1 reply; 16+ messages in thread
From: Richard Henderson @ 2021-11-09 8:11 UTC (permalink / raw)
To: Warner Losh
Cc: Philippe Mathieu-Daude, Alex Bennée, QEMU Developers,
Laurent Vivier
On 11/8/21 7:49 PM, Warner Losh wrote:
> > /* code path for having successfully executed the syscall */
> > + ADJUST_SYSCALL_RETCODE
> > ret
> >
> > 0:
>
> Not sure about this, really. Is it really that much cleaner to insert this than create
> separate 10-line files, with the adjustment included?
...
> The adjustments have all been 3 lines (gmail seems to hate my formatting):
>
> +#define ADJUST_SYSCALL_RETCODE \
> + jnb 2f; \
> + neg %rax; \
> + 2:
>
> which is significantly easier to maintain than having to monitor these files for changes
> and copying over the changes that happen.
...
> The other alternative I considered was having a #ifdef __FreeBSD__ .. #endif in all those
> files, but I thought that even more intrusive.
Actually, the ifdef sounds surprisingly attractive to me. Is it ENOCOFFEE?
What I find awkward about ADJUST_SYSCALL_RETCODE is that when you're looking at the
definition, you have no reference to the context, and vice versa. Not that it can't be
worked out, but it seems like the same amount of code either way, and clearer when it's
together.
We've already split the host cpu apart, which is the major point of ifdeffery, so it
doesn't seem like we'll wind up with a large amount of ifdefs here; we're not likely to
see mynewos-user wanting to share this code any time soon.
I feel sufficiently fuzzy on this to solicit other opinions though.
r~
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [RFC 4/4] common-user: Allow return codes to be adjusted after sytsem call
2021-11-09 8:11 ` Richard Henderson
@ 2021-11-10 2:10 ` Warner Losh
0 siblings, 0 replies; 16+ messages in thread
From: Warner Losh @ 2021-11-10 2:10 UTC (permalink / raw)
To: Richard Henderson
Cc: Philippe Mathieu-Daude, Alex Bennée, QEMU Developers,
Laurent Vivier
[-- Attachment #1: Type: text/plain, Size: 2009 bytes --]
On Tue, Nov 9, 2021 at 1:11 AM Richard Henderson <
richard.henderson@linaro.org> wrote:
> On 11/8/21 7:49 PM, Warner Losh wrote:
> > > /* code path for having successfully executed the syscall */
> > > + ADJUST_SYSCALL_RETCODE
> > > ret
> > >
> > > 0:
> >
> > Not sure about this, really. Is it really that much cleaner to
> insert this than create
> > separate 10-line files, with the adjustment included?
> ...
> > The adjustments have all been 3 lines (gmail seems to hate my
> formatting):
> >
> > +#define ADJUST_SYSCALL_RETCODE \
> > + jnb 2f; \
> > + neg %rax; \
> > + 2:
> >
> > which is significantly easier to maintain than having to monitor these
> files for changes
> > and copying over the changes that happen.
> ...
> > The other alternative I considered was having a #ifdef __FreeBSD__ ..
> #endif in all those
> > files, but I thought that even more intrusive.
>
> Actually, the ifdef sounds surprisingly attractive to me. Is it ENOCOFFEE?
>
> What I find awkward about ADJUST_SYSCALL_RETCODE is that when you're
> looking at the
> definition, you have no reference to the context, and vice versa. Not
> that it can't be
> worked out, but it seems like the same amount of code either way, and
> clearer when it's
> together.
>
> We've already split the host cpu apart, which is the major point of
> ifdeffery, so it
> doesn't seem like we'll wind up with a large amount of ifdefs here; we're
> not likely to
> see mynewos-user wanting to share this code any time soon.
>
> I feel sufficiently fuzzy on this to solicit other opinions though.
>
I'm OK with #ifdef. The macro hides the ifdef in a general way... but
there's
always a problem generalizing from one example...
If nobody has a better idea, I'll respin with the #ifdefs in a day or two.
That would also
give good sharing and be a couple of steps down the road towards getting a
series of
signals patches queued up...
Warner
[-- Attachment #2: Type: text/html, Size: 2704 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread