Re: [PATCH v4 02/12] {linux,bsd}-user: Introduce get_task_state()

2024-02-19 Thread Richard Henderson

On 2/19/24 04:15, Ilya Leoshkevich wrote:

A CPU's TaskState is stored in the CPUState's void *opaque field,
accessing which is somewhat awkward due to having to use a cast.
Introduce a wrapper and use it everywhere.

Suggested-by: Alex Bennée
Signed-off-by: Ilya Leoshkevich
---
  bsd-user/bsd-file.h   |  2 +-
  bsd-user/qemu.h   |  5 +
  bsd-user/signal.c | 20 ++--
  gdbstub/user-target.c |  4 ++--
  include/user/safe-syscall.h   |  2 +-
  linux-user/aarch64/cpu_loop.c |  2 +-
  linux-user/arm/cpu_loop.c |  4 ++--
  linux-user/arm/signal.c   |  2 +-
  linux-user/cris/cpu_loop.c|  2 +-
  linux-user/elfload.c  |  6 +++---
  linux-user/hppa/signal.c  |  2 +-
  linux-user/linuxload.c|  2 +-
  linux-user/m68k/cpu_loop.c|  2 +-
  linux-user/m68k/target_cpu.h  |  2 +-
  linux-user/mips/cpu_loop.c|  2 +-
  linux-user/ppc/signal.c   |  4 ++--
  linux-user/qemu.h |  5 +
  linux-user/riscv/cpu_loop.c   |  2 +-
  linux-user/signal-common.h|  2 +-
  linux-user/signal.c   | 30 +++---
  linux-user/syscall.c  | 26 +-
  linux-user/vm86.c | 18 +-
  linux-user/xtensa/signal.c|  2 +-
  plugins/api.c |  8 
  semihosting/arm-compat-semi.c |  8 
  25 files changed, 87 insertions(+), 77 deletions(-)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH v4 02/12] {linux,bsd}-user: Introduce get_task_state()

2024-02-19 Thread Warner Losh
On Mon, Feb 19, 2024 at 7:21 AM Ilya Leoshkevich  wrote:

> A CPU's TaskState is stored in the CPUState's void *opaque field,
> accessing which is somewhat awkward due to having to use a cast.
> Introduce a wrapper and use it everywhere.
>
> Suggested-by: Alex Bennée 
> Signed-off-by: Ilya Leoshkevich 
>

Reviewed-by: Warner Losh 

The bsd-user stuff is definitely good. The linux-user seems good, but I
didn't look
at it as closely.

Warner

>  bsd-user/bsd-file.h   |  2 +-
>  bsd-user/qemu.h   |  5 +
>  bsd-user/signal.c | 20 ++--
>  gdbstub/user-target.c |  4 ++--
>  include/user/safe-syscall.h   |  2 +-
>  linux-user/aarch64/cpu_loop.c |  2 +-
>  linux-user/arm/cpu_loop.c |  4 ++--
>  linux-user/arm/signal.c   |  2 +-
>  linux-user/cris/cpu_loop.c|  2 +-
>  linux-user/elfload.c  |  6 +++---
>  linux-user/hppa/signal.c  |  2 +-
>  linux-user/linuxload.c|  2 +-
>  linux-user/m68k/cpu_loop.c|  2 +-
>  linux-user/m68k/target_cpu.h  |  2 +-
>  linux-user/mips/cpu_loop.c|  2 +-
>  linux-user/ppc/signal.c   |  4 ++--
>  linux-user/qemu.h |  5 +
>  linux-user/riscv/cpu_loop.c   |  2 +-
>  linux-user/signal-common.h|  2 +-
>  linux-user/signal.c   | 30 +++---
>  linux-user/syscall.c  | 26 +-
>  linux-user/vm86.c | 18 +-
>  linux-user/xtensa/signal.c|  2 +-
>  plugins/api.c |  8 
>  semihosting/arm-compat-semi.c |  8 
>  25 files changed, 87 insertions(+), 77 deletions(-)
>
> diff --git a/bsd-user/bsd-file.h b/bsd-user/bsd-file.h
> index 3c00dc00567..6fa2c30b4de 100644
> --- a/bsd-user/bsd-file.h
> +++ b/bsd-user/bsd-file.h
> @@ -641,7 +641,7 @@ static abi_long do_bsd_readlink(CPUArchState *env,
> abi_long arg1,
>  }
>  if (strcmp(p1, "/proc/curproc/file") == 0) {
>  CPUState *cpu = env_cpu(env);
> -TaskState *ts = (TaskState *)cpu->opaque;
> +TaskState *ts = get_task_state(cpu);
>  strncpy(p2, ts->bprm->fullpath, arg3);
>  ret = MIN((abi_long)strlen(ts->bprm->fullpath), arg3);
>  } else {
> diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
> index dc842fffa7d..a2417b25156 100644
> --- a/bsd-user/qemu.h
> +++ b/bsd-user/qemu.h
> @@ -110,6 +110,11 @@ typedef struct TaskState {
>  struct target_sigaltstack sigaltstack_used;
>  } __attribute__((aligned(16))) TaskState;
>
> +static inline TaskState *get_task_state(CPUState *cs)
> +{
> +return cs->opaque;
> +}
> +
>  void stop_all_tasks(void);
>  extern const char *interp_prefix;
>  extern const char *qemu_uname_release;
> diff --git a/bsd-user/signal.c b/bsd-user/signal.c
> index f4352e4530f..e9f80a06d32 100644
> --- a/bsd-user/signal.c
> +++ b/bsd-user/signal.c
> @@ -319,7 +319,7 @@ void host_to_target_siginfo(target_siginfo_t *tinfo,
> const siginfo_t *info)
>
>  int block_signals(void)
>  {
> -TaskState *ts = (TaskState *)thread_cpu->opaque;
> +TaskState *ts = get_task_state(thread_cpu);
>  sigset_t set;
>
>  /*
> @@ -359,7 +359,7 @@ void dump_core_and_abort(int target_sig)
>  {
>  CPUState *cpu = thread_cpu;
>  CPUArchState *env = cpu_env(cpu);
> -TaskState *ts = cpu->opaque;
> +TaskState *ts = get_task_state(cpu);
>  int core_dumped = 0;
>  int host_sig;
>  struct sigaction act;
> @@ -421,7 +421,7 @@ void queue_signal(CPUArchState *env, int sig, int
> si_type,
>target_siginfo_t *info)
>  {
>  CPUState *cpu = env_cpu(env);
> -TaskState *ts = cpu->opaque;
> +TaskState *ts = get_task_state(cpu);
>
>  trace_user_queue_signal(env, sig);
>
> @@ -476,7 +476,7 @@ void force_sig_fault(int sig, int code, abi_ulong addr)
>  static void host_signal_handler(int host_sig, siginfo_t *info, void *puc)
>  {
>  CPUState *cpu = thread_cpu;
> -TaskState *ts = cpu->opaque;
> +TaskState *ts = get_task_state(cpu);
>  target_siginfo_t tinfo;
>  ucontext_t *uc = puc;
>  struct emulated_sigtable *k;
> @@ -585,7 +585,7 @@ static void host_signal_handler(int host_sig,
> siginfo_t *info, void *puc)
>  /* compare to kern/kern_sig.c sys_sigaltstack() and kern_sigaltstack() */
>  abi_long do_sigaltstack(abi_ulong uss_addr, abi_ulong uoss_addr,
> abi_ulong sp)
>  {
> -TaskState *ts = (TaskState *)thread_cpu->opaque;
> +TaskState *ts = get_task_state(thread_cpu);
>  int ret;
>  target_stack_t oss;
>
> @@ -714,7 +714,7 @@ int do_sigaction(int sig, const struct
> target_sigaction *act,
>  static inline abi_ulong get_sigframe(struct target_sigaction *ka,
>  CPUArchState *env, size_t frame_size)
>  {
> -TaskState *ts = (TaskState *)thread_cpu->opaque;
> +TaskState *ts = get_task_state(thread_cpu);
>  abi_ulong sp;
>
>  /* Use default user stack */
> @@ -789,7 +789,7 @@ static int reset_signal_mask(target_ucontext_t
> *ucontext)
>  int i;
>  

[PATCH v4 02/12] {linux,bsd}-user: Introduce get_task_state()

2024-02-19 Thread Ilya Leoshkevich
A CPU's TaskState is stored in the CPUState's void *opaque field,
accessing which is somewhat awkward due to having to use a cast.
Introduce a wrapper and use it everywhere.

Suggested-by: Alex Bennée 
Signed-off-by: Ilya Leoshkevich 
---
 bsd-user/bsd-file.h   |  2 +-
 bsd-user/qemu.h   |  5 +
 bsd-user/signal.c | 20 ++--
 gdbstub/user-target.c |  4 ++--
 include/user/safe-syscall.h   |  2 +-
 linux-user/aarch64/cpu_loop.c |  2 +-
 linux-user/arm/cpu_loop.c |  4 ++--
 linux-user/arm/signal.c   |  2 +-
 linux-user/cris/cpu_loop.c|  2 +-
 linux-user/elfload.c  |  6 +++---
 linux-user/hppa/signal.c  |  2 +-
 linux-user/linuxload.c|  2 +-
 linux-user/m68k/cpu_loop.c|  2 +-
 linux-user/m68k/target_cpu.h  |  2 +-
 linux-user/mips/cpu_loop.c|  2 +-
 linux-user/ppc/signal.c   |  4 ++--
 linux-user/qemu.h |  5 +
 linux-user/riscv/cpu_loop.c   |  2 +-
 linux-user/signal-common.h|  2 +-
 linux-user/signal.c   | 30 +++---
 linux-user/syscall.c  | 26 +-
 linux-user/vm86.c | 18 +-
 linux-user/xtensa/signal.c|  2 +-
 plugins/api.c |  8 
 semihosting/arm-compat-semi.c |  8 
 25 files changed, 87 insertions(+), 77 deletions(-)

diff --git a/bsd-user/bsd-file.h b/bsd-user/bsd-file.h
index 3c00dc00567..6fa2c30b4de 100644
--- a/bsd-user/bsd-file.h
+++ b/bsd-user/bsd-file.h
@@ -641,7 +641,7 @@ static abi_long do_bsd_readlink(CPUArchState *env, abi_long 
arg1,
 }
 if (strcmp(p1, "/proc/curproc/file") == 0) {
 CPUState *cpu = env_cpu(env);
-TaskState *ts = (TaskState *)cpu->opaque;
+TaskState *ts = get_task_state(cpu);
 strncpy(p2, ts->bprm->fullpath, arg3);
 ret = MIN((abi_long)strlen(ts->bprm->fullpath), arg3);
 } else {
diff --git a/bsd-user/qemu.h b/bsd-user/qemu.h
index dc842fffa7d..a2417b25156 100644
--- a/bsd-user/qemu.h
+++ b/bsd-user/qemu.h
@@ -110,6 +110,11 @@ typedef struct TaskState {
 struct target_sigaltstack sigaltstack_used;
 } __attribute__((aligned(16))) TaskState;
 
+static inline TaskState *get_task_state(CPUState *cs)
+{
+return cs->opaque;
+}
+
 void stop_all_tasks(void);
 extern const char *interp_prefix;
 extern const char *qemu_uname_release;
diff --git a/bsd-user/signal.c b/bsd-user/signal.c
index f4352e4530f..e9f80a06d32 100644
--- a/bsd-user/signal.c
+++ b/bsd-user/signal.c
@@ -319,7 +319,7 @@ void host_to_target_siginfo(target_siginfo_t *tinfo, const 
siginfo_t *info)
 
 int block_signals(void)
 {
-TaskState *ts = (TaskState *)thread_cpu->opaque;
+TaskState *ts = get_task_state(thread_cpu);
 sigset_t set;
 
 /*
@@ -359,7 +359,7 @@ void dump_core_and_abort(int target_sig)
 {
 CPUState *cpu = thread_cpu;
 CPUArchState *env = cpu_env(cpu);
-TaskState *ts = cpu->opaque;
+TaskState *ts = get_task_state(cpu);
 int core_dumped = 0;
 int host_sig;
 struct sigaction act;
@@ -421,7 +421,7 @@ void queue_signal(CPUArchState *env, int sig, int si_type,
   target_siginfo_t *info)
 {
 CPUState *cpu = env_cpu(env);
-TaskState *ts = cpu->opaque;
+TaskState *ts = get_task_state(cpu);
 
 trace_user_queue_signal(env, sig);
 
@@ -476,7 +476,7 @@ void force_sig_fault(int sig, int code, abi_ulong addr)
 static void host_signal_handler(int host_sig, siginfo_t *info, void *puc)
 {
 CPUState *cpu = thread_cpu;
-TaskState *ts = cpu->opaque;
+TaskState *ts = get_task_state(cpu);
 target_siginfo_t tinfo;
 ucontext_t *uc = puc;
 struct emulated_sigtable *k;
@@ -585,7 +585,7 @@ static void host_signal_handler(int host_sig, siginfo_t 
*info, void *puc)
 /* compare to kern/kern_sig.c sys_sigaltstack() and kern_sigaltstack() */
 abi_long do_sigaltstack(abi_ulong uss_addr, abi_ulong uoss_addr, abi_ulong sp)
 {
-TaskState *ts = (TaskState *)thread_cpu->opaque;
+TaskState *ts = get_task_state(thread_cpu);
 int ret;
 target_stack_t oss;
 
@@ -714,7 +714,7 @@ int do_sigaction(int sig, const struct target_sigaction 
*act,
 static inline abi_ulong get_sigframe(struct target_sigaction *ka,
 CPUArchState *env, size_t frame_size)
 {
-TaskState *ts = (TaskState *)thread_cpu->opaque;
+TaskState *ts = get_task_state(thread_cpu);
 abi_ulong sp;
 
 /* Use default user stack */
@@ -789,7 +789,7 @@ static int reset_signal_mask(target_ucontext_t *ucontext)
 int i;
 sigset_t blocked;
 target_sigset_t target_set;
-TaskState *ts = (TaskState *)thread_cpu->opaque;
+TaskState *ts = get_task_state(thread_cpu);
 
 for (i = 0; i < TARGET_NSIG_WORDS; i++) {
 __get_user(target_set.__bits[i], >uc_sigmask.__bits[i]);
@@ -839,7 +839,7 @@ badframe:
 
 void signal_init(void)
 {
-TaskState *ts = (TaskState *)thread_cpu->opaque;
+TaskState *ts = get_task_state(thread_cpu);