Re: [PATCH] hw/intc/arm_gicv3: Fix GICv3 redistributor security checking

[Tianrui Wei]

Hi Peter,

Many thanks for your detailed explanation. Upon further reflection, it
seems that I have misinterpreted some of the explanations in
the manual. Sorry for the confusion, the original implementation is
correct.

Thanks,
Tianrui

On Fri, Jul 16, 2021 at 4:32 PM Peter Maydell 
wrote:

> On Wed, 14 Jul 2021 at 20:46, Tianrui Wei  wrote:
> >
> > For redistributor to send sgi, we must test NSACR bits in secure mode.
> > However, current implementation inverts the security check, wrongly
> > skipping this it when the CPU is in secure state, and only carrying out
> > the check when the CPU is not secure or security is not implemented.
> > This patch corrects this problem by correcting the inversion of CPU
> > secure state checking. It has been tested to work with Linux version 5.11
> > in both aarch64 and arm version of qemu.
> >
> > According to “Arm Generic Interrupt Controller Architecture
> > Specification GIC architecture version 3 and version 4,” p. 930, 2008.
> > Chapter 12, page 530, when there is only one security state implemented,
> > GICD.CTLR.DS is always 0, thus checking NSACR in non-secure state. When
> > cpu is in secure state, ns = 0, thus the NSACR check is never performed.
> >
> > Signed-off-by: Tianrui Wei 
> > Signed-off-by: Jonathan Balkind 
> > Tested-by: Tianrui Wei 
> > ---
> >  hw/intc/arm_gicv3_redist.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/intc/arm_gicv3_redist.c b/hw/intc/arm_gicv3_redist.c
> > index 53da703ed8..84cfcfd18f 100644
> > --- a/hw/intc/arm_gicv3_redist.c
> > +++ b/hw/intc/arm_gicv3_redist.c
> > @@ -564,7 +564,7 @@ void gicv3_redist_send_sgi(GICv3CPUState *cs, int
> grp, int irq, bool ns)
> >  return;
> >  }
> >
> > -if (ns && !(cs->gic->gicd_ctlr & GICD_CTLR_DS)) {
> > +if (!ns && !(cs->gic->gicd_ctlr & GICD_CTLR_DS)) {
> >  /* If security is enabled we must test the NSACR bits */
> >  int nsaccess = gicr_ns_access(cs, irq);
>
> So, before this change:
>  * if the access to ICC_SGI[01]R_EL1 attempting to kick off this SGI
>is done in Secure state, we allow it
>  * if the GIC has security disabled (GICD_CTLR.DS is 1), we allow it
>  * if the access is from NonSecure and the GIC does not have security
>disabled, we check the NSACR bits to see if we should allow it
>
> With this change, we check the NSACR bits for accesses from Secure
> state, and we don't check them for accesses from NonSecure.
> This doesn't seem to me to match what the spec requires: in version
> IHI0069G of the GICv3 spec, section 12.1.10, the tables show that
> only accesses from NonSecure are subject to NSACR checks. (This makes
> intuitive sense: the GICR_NSACR is the NonSecure Access Control
> Register and it is controls NonSecure accesses, not Secure accesses.)
>
> What bug are you trying to fix with this patch ?
>
> thanks
> -- PMM
>

Re: [PATCH for-6.2 05/10] linux-user: Split mmap prototypes into user-mmap.h

[Peter Maydell]

On Sun, 18 Jul 2021 at 00:21, Peter Maydell  wrote:
>
> Split out the mmap prototypes into a new header user-mmap.h
> which we only include where required.
>
> Signed-off-by: Peter Maydell 
> ---
>  linux-user/qemu.h  | 20 
>  linux-user/user-mmap.h | 34 ++
>  linux-user/elfload.c   |  1 +
>  linux-user/flatload.c  |  1 +
>  linux-user/i386/cpu_loop.c |  1 +
>  linux-user/main.c  |  1 +
>  linux-user/mmap.c  |  1 +
>  linux-user/syscall.c   |  1 +
>  8 files changed, 40 insertions(+), 20 deletions(-)
>  create mode 100644 linux-user/user-mmap.h
>
> diff --git a/linux-user/qemu.h b/linux-user/qemu.h
> index 9e700d3af18..0cb79990579 100644
> --- a/linux-user/qemu.h
> +++ b/linux-user/qemu.h
> @@ -201,12 +201,6 @@ extern IOCTLEntry ioctl_entries[];
>   */
>  int info_is_fdpic(struct image_info *info);
>
> -uint32_t get_elf_eflags(int fd);
> -int load_elf_binary(struct linux_binprm *bprm, struct image_info *info);
> -int load_flt_binary(struct linux_binprm *bprm, struct image_info *info);
> -
> -abi_long memcpy_to_target(abi_ulong dest, const void *src,
> -  unsigned long len);
>  void target_set_brk(abi_ulong new_brk);
>  abi_long do_brk(abi_ulong new_brk);
>  void syscall_init(void);

Oops, this hunk should be in the previous patch (where these
were moved to loader.h).

> --- /dev/null
> +++ b/linux-user/user-mmap.h
> @@ -0,0 +1,34 @@
> +/*
> + * loader.h: prototypes for linux-user guest binary loader

...and this should say "user-mmap.h:".

> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, see .
> + */
> +

-- PMM

Re: [PATCH v3 10/13] target/riscv: Reduce riscv_tr_breakpoint_check pc advance to 2

[Peter Maydell]

On Sat, 17 Jul 2021 at 23:18, Richard Henderson
 wrote:
>
> The actual number of bytes advanced need not be 100% exact,
> but we should not cross a page when the insn would not.
>
> If rvc is enabled, the minimum insn size is 2.
>
> Signed-off-by: Richard Henderson 
> ---
>  target/riscv/translate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index deda0c8a44..5527f37ada 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -973,7 +973,7 @@ static bool riscv_tr_breakpoint_check(DisasContextBase 
> *dcbase, CPUState *cpu,
> [tb->pc, tb->pc + tb->size) in order to for it to be
> properly cleared -- thus we increment the PC here so that
> the logic setting tb->size below does the right thing.  */
> -ctx->base.pc_next += 4;
> +ctx->base.pc_next += 2;
>  return true;
>  }

Reviewed-by: Peter Maydell 

(What goes wrong if we just say "always use a TB size of 1 regardless
of target arch" rather than having the arch return the worst case
minimum insn length?)

thanks
-- PMM

Re: [PATCH v3 09/13] target/mips: Reduce mips_tr_breakpoint_check pc advance to 2

[Peter Maydell]

On Sat, 17 Jul 2021 at 23:18, Richard Henderson
 wrote:
>
> The actual number of bytes advanced need not be 100% exact,
> but we should not cross a page when the insn would not.
>
> If mips16 or mips32e are enabled, the minimum insn size is 2.
>
> Signed-off-by: Richard Henderson 
> ---
>  target/mips/tcg/translate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
> index fd980ea966..ef00fbd2ac 100644
> --- a/target/mips/tcg/translate.c
> +++ b/target/mips/tcg/translate.c
> @@ -16192,7 +16192,7 @@ static bool mips_tr_breakpoint_check(DisasContextBase 
> *dcbase, CPUState *cs,
>   * properly cleared -- thus we increment the PC here so that
>   * the logic setting tb->size below does the right thing.
>   */
> -ctx->base.pc_next += 4;
> +ctx->base.pc_next += 2;
>  return true;
>  }
>

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH v3 08/13] target/avr: Advance pc in avr_tr_breakpoint_check

[Peter Maydell]

On Sat, 17 Jul 2021 at 23:18, Richard Henderson
 wrote:
>
> Since 0b00b0c1e05b, tb->size must not be zero.
> Advance pc so that the breakpoint covers the insn at the bp.
>
> Signed-off-by: Richard Henderson 
> ---
>  target/avr/translate.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/target/avr/translate.c b/target/avr/translate.c
> index 8237a03c23..d768063d65 100644
> --- a/target/avr/translate.c
> +++ b/target/avr/translate.c
> @@ -2950,6 +2950,7 @@ static bool avr_tr_breakpoint_check(DisasContextBase 
> *dcbase, CPUState *cs,
>  DisasContext *ctx = container_of(dcbase, DisasContext, base);
>
>  gen_breakpoint(ctx);
> +ctx->base.pc_next += 2; /* advance by minimum insn len so tb->size != 0 
> */
>  return true;
>  }

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH v3 01/13] accel/tcg: Reduce CF_COUNT_MASK to match TCG_MAX_INSNS

[Peter Maydell]

On Sat, 17 Jul 2021 at 23:18, Richard Henderson
 wrote:
>
> The space reserved for CF_COUNT_MASK was overly large.
> Reduce to free up cflags bits and eliminate an extra test.
>
> Reviewed-by: Alex Bennée 
> Signed-off-by: Richard Henderson 
Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH for-6.2 10/10] linux-user: Move DEBUG_REMAP undef to uaccess.c

[Peter Maydell]

On Sun, 18 Jul 2021 at 00:21, Peter Maydell  wrote:
>
> Since commit 687ca797893ca1e853, the code that looks at the debug
> define DEBUG_REMAP is all in uaccess.c; move the #undef line to
> there from qemu.h (thus reducing significantly the amount of code
> that gets recompiled if you need to turn the debug on).
>
> Signed-off-by: Peter Maydell 
> ---
>  linux-user/qemu.h| 3 ---
>  linux-user/uaccess.c | 2 ++
>  2 files changed, 2 insertions(+), 3 deletions(-)

A minute after sending it out, I realized this patch is wrong:
there is still a use of DEBUG_REMAP in qemu.h. So this one should be
dropped.

thanks
-- PMM

[PATCH for-6.2 06/10] linux-user: Split safe-syscall macro into its own header

[Peter Maydell]

Split the safe-syscall macro from qemu.h into a new safe-syscall.h.

Signed-off-by: Peter Maydell 
---
 linux-user/qemu.h | 135 -
 linux-user/safe-syscall.h | 154 ++
 linux-user/syscall.c  |   1 +
 3 files changed, 155 insertions(+), 135 deletions(-)
 create mode 100644 linux-user/safe-syscall.h

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 0cb79990579..a82a46236e6 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -240,141 +240,6 @@ void probe_guest_base(const char *image_name,
 
 #include "qemu/log.h"
 
-/* safe_syscall.S */
-
-/**
- * safe_syscall:
- * @int number: number of system call to make
- * ...: arguments to the system call
- *
- * Call a system call if guest signal not pending.
- * This has the same API as the libc syscall() function, except that it
- * may return -1 with errno == TARGET_ERESTARTSYS if a signal was pending.
- *
- * Returns: the system call result, or -1 with an error code in errno
- * (Errnos are host errnos; we rely on TARGET_ERESTARTSYS not clashing
- * with any of the host errno values.)
- */
-
-/*
- * A guide to using safe_syscall() to handle interactions between guest
- * syscalls and guest signals:
- *
- * Guest syscalls come in two flavours:
- *
- * (1) Non-interruptible syscalls
- *
- * These are guest syscalls that never get interrupted by signals and
- * so never return EINTR. They can be implemented straightforwardly in
- * QEMU: just make sure that if the implementation code has to make any
- * blocking calls that those calls are retried if they return EINTR.
- * It's also OK to implement these with safe_syscall, though it will be
- * a little less efficient if a signal is delivered at the 'wrong' moment.
- *
- * Some non-interruptible syscalls need to be handled using block_signals()
- * to block signals for the duration of the syscall. This mainly applies
- * to code which needs to modify the data structures used by the
- * host_signal_handler() function and the functions it calls, including
- * all syscalls which change the thread's signal mask.
- *
- * (2) Interruptible syscalls
- *
- * These are guest syscalls that can be interrupted by signals and
- * for which we need to either return EINTR or arrange for the guest
- * syscall to be restarted. This category includes both syscalls which
- * always restart (and in the kernel return -ERESTARTNOINTR), ones
- * which only restart if there is no handler (kernel returns -ERESTARTNOHAND
- * or -ERESTART_RESTARTBLOCK), and the most common kind which restart
- * if the handler was registered with SA_RESTART (kernel returns
- * -ERESTARTSYS). System calls which are only interruptible in some
- * situations (like 'open') also need to be handled this way.
- *
- * Here it is important that the host syscall is made
- * via this safe_syscall() function, and *not* via the host libc.
- * If the host libc is used then the implementation will appear to work
- * most of the time, but there will be a race condition where a
- * signal could arrive just before we make the host syscall inside libc,
- * and then then guest syscall will not correctly be interrupted.
- * Instead the implementation of the guest syscall can use the safe_syscall
- * function but otherwise just return the result or errno in the usual
- * way; the main loop code will take care of restarting the syscall
- * if appropriate.
- *
- * (If the implementation needs to make multiple host syscalls this is
- * OK; any which might really block must be via safe_syscall(); for those
- * which are only technically blocking (ie which we know in practice won't
- * stay in the host kernel indefinitely) it's OK to use libc if necessary.
- * You must be able to cope with backing out correctly if some safe_syscall
- * you make in the implementation returns either -TARGET_ERESTARTSYS or
- * EINTR though.)
- *
- * block_signals() cannot be used for interruptible syscalls.
- *
- *
- * How and why the safe_syscall implementation works:
- *
- * The basic setup is that we make the host syscall via a known
- * section of host native assembly. If a signal occurs, our signal
- * handler checks the interrupted host PC against the addresse of that
- * known section. If the PC is before or at the address of the syscall
- * instruction then we change the PC to point at a "return
- * -TARGET_ERESTARTSYS" code path instead, and then exit the signal handler
- * (causing the safe_syscall() call to immediately return that value).
- * Then in the main.c loop if we see this magic return value we adjust
- * the guest PC to wind it back to before the system call, and invoke
- * the guest signal handler as usual.
- *
- * This winding-back will happen in two cases:
- * (1) signal came in just before we took the host syscall (a race);
- *   in this case we'll take the guest signal and have another go
- *   at the syscall afterwards, and this is indistinguishable for the
- *   guest from the timing having 

[PATCH for-6.2 10/10] linux-user: Move DEBUG_REMAP undef to uaccess.c

[Peter Maydell]

Since commit 687ca797893ca1e853, the code that looks at the debug
define DEBUG_REMAP is all in uaccess.c; move the #undef line to
there from qemu.h (thus reducing significantly the amount of code
that gets recompiled if you need to turn the debug on).

Signed-off-by: Peter Maydell 
---
 linux-user/qemu.h| 3 ---
 linux-user/uaccess.c | 2 ++
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 5c713fa8ab2..acc215b1a48 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -3,9 +3,6 @@
 
 #include "cpu.h"
 #include "exec/cpu_ldst.h"
-
-#undef DEBUG_REMAP
-
 #include "exec/user/abitypes.h"
 
 #include "syscall_defs.h"
diff --git a/linux-user/uaccess.c b/linux-user/uaccess.c
index 425cbf677f7..56fb4358d4f 100644
--- a/linux-user/uaccess.c
+++ b/linux-user/uaccess.c
@@ -5,6 +5,8 @@
 #include "qemu.h"
 #include "user-internals.h"
 
+#undef DEBUG_REMAP
+
 void *lock_user(int type, abi_ulong guest_addr, ssize_t len, bool copy)
 {
 void *host_addr;
-- 
2.20.1

[PATCH for-6.2 04/10] linux-user: Split loader-related prototypes into loader.h

[Peter Maydell]

Split guest-binary loader prototypes out into a new header
loader.h which we include only where required.

Signed-off-by: Peter Maydell 
---
 linux-user/loader.h| 59 ++
 linux-user/qemu.h  | 34 
 linux-user/elfload.c   |  1 +
 linux-user/flatload.c  |  1 +
 linux-user/linuxload.c |  1 +
 linux-user/main.c  |  1 +
 linux-user/signal.c|  1 +
 linux-user/syscall.c   |  1 +
 8 files changed, 65 insertions(+), 34 deletions(-)
 create mode 100644 linux-user/loader.h

diff --git a/linux-user/loader.h b/linux-user/loader.h
new file mode 100644
index 000..f375ee0679b
--- /dev/null
+++ b/linux-user/loader.h
@@ -0,0 +1,59 @@
+/*
+ * loader.h: prototypes for linux-user guest binary loader
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see .
+ */
+
+#ifndef LINUX_USER_LOADER_H
+#define LINUX_USER_LOADER_H
+
+/*
+ * Read a good amount of data initially, to hopefully get all the
+ * program headers loaded.
+ */
+#define BPRM_BUF_SIZE  1024
+
+/*
+ * This structure is used to hold the arguments that are
+ * used when loading binaries.
+ */
+struct linux_binprm {
+char buf[BPRM_BUF_SIZE] __attribute__((aligned));
+abi_ulong p;
+int fd;
+int e_uid, e_gid;
+int argc, envc;
+char **argv;
+char **envp;
+char *filename;/* Name of binary */
+int (*core_dump)(int, const CPUArchState *); /* coredump routine */
+};
+
+void do_init_thread(struct target_pt_regs *regs, struct image_info *infop);
+abi_ulong loader_build_argptr(int envc, int argc, abi_ulong sp,
+  abi_ulong stringp, int push_ptr);
+int loader_exec(int fdexec, const char *filename, char **argv, char **envp,
+ struct target_pt_regs *regs, struct image_info *infop,
+ struct linux_binprm *);
+
+uint32_t get_elf_eflags(int fd);
+int load_elf_binary(struct linux_binprm *bprm, struct image_info *info);
+int load_flt_binary(struct linux_binprm *bprm, struct image_info *info);
+
+abi_long memcpy_to_target(abi_ulong dest, const void *src,
+  unsigned long len);
+
+extern unsigned long guest_stack_size;
+
+#endif /* LINUX_USER_LOADER_H */
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 76d3f5e7eb9..9e700d3af18 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -173,30 +173,6 @@ void stop_all_tasks(void);
 extern const char *qemu_uname_release;
 extern unsigned long mmap_min_addr;
 
-/* ??? See if we can avoid exposing so much of the loader internals.  */
-
-/*
- * Read a good amount of data initially, to hopefully get all the
- * program headers loaded.
- */
-#define BPRM_BUF_SIZE  1024
-
-/*
- * This structure is used to hold the arguments that are
- * used when loading binaries.
- */
-struct linux_binprm {
-char buf[BPRM_BUF_SIZE] __attribute__((aligned));
-abi_ulong p;
-int fd;
-int e_uid, e_gid;
-int argc, envc;
-char **argv;
-char **envp;
-char *filename;/* Name of binary */
-int (*core_dump)(int, const CPUArchState *); /* coredump routine */
-};
-
 typedef struct IOCTLEntry IOCTLEntry;
 
 typedef abi_long do_ioctl_fn(const IOCTLEntry *ie, uint8_t *buf_temp,
@@ -217,13 +193,6 @@ extern IOCTLEntry ioctl_entries[];
 #define IOC_W 0x0002
 #define IOC_RW (IOC_R | IOC_W)
 
-void do_init_thread(struct target_pt_regs *regs, struct image_info *infop);
-abi_ulong loader_build_argptr(int envc, int argc, abi_ulong sp,
-  abi_ulong stringp, int push_ptr);
-int loader_exec(int fdexec, const char *filename, char **argv, char **envp,
- struct target_pt_regs *regs, struct image_info *infop,
- struct linux_binprm *);
-
 /*
  * Returns true if the image uses the FDPIC ABI. If this is the case,
  * we have to provide some information (loadmap, pt_dynamic_info) such
@@ -440,9 +409,6 @@ abi_ulong mmap_find_vma(abi_ulong, abi_ulong, abi_ulong);
 void mmap_fork_start(void);
 void mmap_fork_end(int child);
 
-/* main.c */
-extern unsigned long guest_stack_size;
-
 /* user access */
 
 #define VERIFY_READ  PAGE_READ
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 42ef2a11485..d56e00c18b4 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -6,6 +6,7 @@
 #include 
 
 #include "qemu.h"

[PATCH for-6.2 09/10] linux-user: Drop unneeded includes from qemu.h

[Peter Maydell]

Trim down the #includes in qemu.h where we can, either by
dropping unneeded headers or by moving them to user-internals.h.

This includes deleting a couple of #includes that appear at
weird points midway through the header file.

Signed-off-by: Peter Maydell 
---
 linux-user/qemu.h   | 4 
 linux-user/user-internals.h | 2 ++
 thunk.c | 1 +
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index fda90fc28d6..5c713fa8ab2 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -2,7 +2,6 @@
 #define QEMU_H
 
 #include "cpu.h"
-#include "exec/exec-all.h"
 #include "exec/cpu_ldst.h"
 
 #undef DEBUG_REMAP
@@ -163,8 +162,6 @@ typedef struct TaskState {
 struct target_sigaltstack sigaltstack_used;
 } __attribute__((aligned(16))) TaskState;
 
-#include "qemu/log.h"
-
 abi_long do_brk(abi_ulong new_brk);
 
 /* user access */
@@ -349,5 +346,4 @@ void *lock_user_string(abi_ulong guest_addr);
 #define unlock_user_struct(host_ptr, guest_addr, copy) \
 unlock_user(host_ptr, guest_addr, (copy) ? sizeof(*host_ptr) : 0)
 
-#include 
 #endif /* QEMU_H */
diff --git a/linux-user/user-internals.h b/linux-user/user-internals.h
index 1729a8b62e1..661612a088b 100644
--- a/linux-user/user-internals.h
+++ b/linux-user/user-internals.h
@@ -20,6 +20,8 @@
 
 #include "hostdep.h"
 #include "exec/user/thunk.h"
+#include "exec/exec-all.h"
+#include "qemu/log.h"
 
 extern char *exec_path;
 void init_task_state(TaskState *ts);
diff --git a/thunk.c b/thunk.c
index fc5be1a502e..dac4bf11c65 100644
--- a/thunk.c
+++ b/thunk.c
@@ -17,6 +17,7 @@
  * License along with this library; if not, see .
  */
 #include "qemu/osdep.h"
+#include "qemu/log.h"
 
 #include "qemu.h"
 #include "exec/user/thunk.h"
-- 
2.20.1

[PATCH for-6.2 08/10] linux-user: Don't include gdbstub.h in qemu.h

[Peter Maydell]

Currently the linux-user qemu.h pulls in gdbstub.h. There's no real reason
why it should do this; include it directly from the C files which require
it, and drop the include line in qemu.h.

(Note that several of the C files previously relying on this indirect
include were going out of their way to only include gdbstub.h conditionally
on not CONFIG_USER_ONLY!)

Signed-off-by: Peter Maydell 
---
 linux-user/qemu.h | 1 -
 gdbstub.c | 2 +-
 linux-user/exit.c | 1 +
 linux-user/main.c | 1 +
 linux-user/signal.c   | 2 ++
 semihosting/arm-compat-semi.c | 2 +-
 target/m68k/m68k-semi.c   | 2 +-
 target/nios2/nios2-semi.c | 2 +-
 8 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 92290a55c0d..fda90fc28d6 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -11,7 +11,6 @@
 
 #include "syscall_defs.h"
 #include "target_syscall.h"
-#include "exec/gdbstub.h"
 
 /*
  * This is the size of the host kernel's sigset_t, needed where we make
diff --git a/gdbstub.c b/gdbstub.c
index 52bde5bdc97..480745152b8 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -31,13 +31,13 @@
 #include "qemu/cutils.h"
 #include "qemu/module.h"
 #include "trace/trace-root.h"
+#include "exec/gdbstub.h"
 #ifdef CONFIG_USER_ONLY
 #include "qemu.h"
 #else
 #include "monitor/monitor.h"
 #include "chardev/char.h"
 #include "chardev/char-fe.h"
-#include "exec/gdbstub.h"
 #include "hw/cpu/cluster.h"
 #include "hw/boards.h"
 #endif
diff --git a/linux-user/exit.c b/linux-user/exit.c
index 371a0803ece..9f78be5feda 100644
--- a/linux-user/exit.c
+++ b/linux-user/exit.c
@@ -17,6 +17,7 @@
  *  along with this program; if not, see .
  */
 #include "qemu/osdep.h"
+#include "exec/gdbstub.h"
 #include "qemu.h"
 #include "user-internals.h"
 #ifdef CONFIG_GPROF
diff --git a/linux-user/main.c b/linux-user/main.c
index 21fe674de3d..8157a208efc 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -40,6 +40,7 @@
 #include "qemu/module.h"
 #include "qemu/plugin.h"
 #include "exec/exec-all.h"
+#include "exec/gdbstub.h"
 #include "tcg/tcg.h"
 #include "qemu/timer.h"
 #include "qemu/envlist.h"
diff --git a/linux-user/signal.c b/linux-user/signal.c
index 7c14f17fb2c..ca2f585b082 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -18,6 +18,8 @@
  */
 #include "qemu/osdep.h"
 #include "qemu/bitops.h"
+#include "exec/gdbstub.h"
+
 #include 
 #include 
 
diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c
index 1c29146dcfa..01badea99c8 100644
--- a/semihosting/arm-compat-semi.c
+++ b/semihosting/arm-compat-semi.c
@@ -37,12 +37,12 @@
 #include "semihosting/console.h"
 #include "semihosting/common-semi.h"
 #include "qemu/timer.h"
+#include "exec/gdbstub.h"
 #ifdef CONFIG_USER_ONLY
 #include "qemu.h"
 
 #define COMMON_SEMI_HEAP_SIZE (128 * 1024 * 1024)
 #else
-#include "exec/gdbstub.h"
 #include "qemu/cutils.h"
 #ifdef TARGET_ARM
 #include "hw/arm/boot.h"
diff --git a/target/m68k/m68k-semi.c b/target/m68k/m68k-semi.c
index d919245e4f8..44ec7e4612c 100644
--- a/target/m68k/m68k-semi.c
+++ b/target/m68k/m68k-semi.c
@@ -20,11 +20,11 @@
 #include "qemu/osdep.h"
 
 #include "cpu.h"
+#include "exec/gdbstub.h"
 #if defined(CONFIG_USER_ONLY)
 #include "qemu.h"
 #define SEMIHOSTING_HEAP_SIZE (128 * 1024 * 1024)
 #else
-#include "exec/gdbstub.h"
 #include "exec/softmmu-semi.h"
 #include "hw/boards.h"
 #endif
diff --git a/target/nios2/nios2-semi.c b/target/nios2/nios2-semi.c
index e508b2fafce..fe5598bae4d 100644
--- a/target/nios2/nios2-semi.c
+++ b/target/nios2/nios2-semi.c
@@ -24,11 +24,11 @@
 #include "qemu/osdep.h"
 
 #include "cpu.h"
+#include "exec/gdbstub.h"
 #if defined(CONFIG_USER_ONLY)
 #include "qemu.h"
 #else
 #include "qemu-common.h"
-#include "exec/gdbstub.h"
 #include "exec/softmmu-semi.h"
 #endif
 #include "qemu/log.h"
-- 
2.20.1

[PATCH for-6.2 01/10] linux-user: Fix coding style nits in qemu.h

[Peter Maydell]

We're about to move a lot of the code in qemu.h out into different
header files; fix the coding style nits first so that checkpatch
is happy with the pure code-movement patches. This is mostly
block-comment style but also a few whitespace issues.

Signed-off-by: Peter Maydell 
---
I didn't clean the whole file; some parts I know I don't need
to move (the user-access functions and macros) I left alone.
---
 linux-user/qemu.h | 47 ++-
 1 file changed, 30 insertions(+), 17 deletions(-)

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 3b0b6b75fe8..34b975ba502 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -15,12 +15,14 @@
 #include "target_syscall.h"
 #include "exec/gdbstub.h"
 
-/* This is the size of the host kernel's sigset_t, needed where we make
+/*
+ * This is the size of the host kernel's sigset_t, needed where we make
  * direct system calls that take a sigset_t pointer and a size.
  */
 #define SIGSET_T_SIZE (_NSIG / 8)
 
-/* This struct is used to hold certain information about the image.
+/*
+ * This struct is used to hold certain information about the image.
  * Basically, it replicates in user space what would be certain
  * task_struct fields in the kernel
  */
@@ -48,13 +50,13 @@ struct image_info {
 abi_ulong   env_strings;
 abi_ulong   file_string;
 uint32_telf_flags;
-intpersonality;
+int personality;
 abi_ulong   alignment;
 
 /* The fields below are used in FDPIC mode.  */
 abi_ulong   loadmap_addr;
 uint16_tnsegs;
-void   *loadsegs;
+void*loadsegs;
 abi_ulong   pt_dynamic_addr;
 abi_ulong   interpreter_loadmap_addr;
 abi_ulong   interpreter_pt_dynamic_addr;
@@ -98,8 +100,10 @@ struct emulated_sigtable {
 target_siginfo_t info;
 };
 
-/* NOTE: we force a big alignment so that the stack stored after is
-   aligned too */
+/*
+ * NOTE: we force a big alignment so that the stack stored after is
+ * aligned too
+ */
 typedef struct TaskState {
 pid_t ts_tid; /* tid (or pid) of this task */
 #ifdef TARGET_ARM
@@ -134,20 +138,23 @@ typedef struct TaskState {
 
 struct emulated_sigtable sync_signal;
 struct emulated_sigtable sigtab[TARGET_NSIG];
-/* This thread's signal mask, as requested by the guest program.
+/*
+ * This thread's signal mask, as requested by the guest program.
  * The actual signal mask of this thread may differ:
  *  + we don't let SIGSEGV and SIGBUS be blocked while running guest code
  *  + sometimes we block all signals to avoid races
  */
 sigset_t signal_mask;
-/* The signal mask imposed by a guest sigsuspend syscall, if we are
+/*
+ * The signal mask imposed by a guest sigsuspend syscall, if we are
  * currently in the middle of such a syscall
  */
 sigset_t sigsuspend_mask;
 /* Nonzero if we're leaving a sigsuspend and sigsuspend_mask is valid. */
 int in_sigsuspend;
 
-/* Nonzero if process_pending_signals() needs to do something (either
+/*
+ * Nonzero if process_pending_signals() needs to do something (either
  * handle a pending signal or unblock signals).
  * This flag is written from a signal handler so should be accessed via
  * the qatomic_read() and qatomic_set() functions. (It is not accessed
@@ -168,8 +175,10 @@ extern unsigned long mmap_min_addr;
 
 /* ??? See if we can avoid exposing so much of the loader internals.  */
 
-/* Read a good amount of data initially, to hopefully get all the
-   program headers loaded.  */
+/*
+ * Read a good amount of data initially, to hopefully get all the
+ * program headers loaded.
+ */
 #define BPRM_BUF_SIZE  1024
 
 /*
@@ -184,7 +193,7 @@ struct linux_binprm {
 int argc, envc;
 char **argv;
 char **envp;
-char * filename;/* Name of binary */
+char *filename;/* Name of binary */
 int (*core_dump)(int, const CPUArchState *); /* coredump routine */
 };
 
@@ -212,10 +221,11 @@ void do_init_thread(struct target_pt_regs *regs, struct 
image_info *infop);
 abi_ulong loader_build_argptr(int envc, int argc, abi_ulong sp,
   abi_ulong stringp, int push_ptr);
 int loader_exec(int fdexec, const char *filename, char **argv, char **envp,
- struct target_pt_regs * regs, struct image_info *infop,
+ struct target_pt_regs *regs, struct image_info *infop,
  struct linux_binprm *);
 
-/* Returns true if the image uses the FDPIC ABI. If this is the case,
+/*
+ * Returns true if the image uses the FDPIC ABI. If this is the case,
  * we have to provide some information (loadmap, pt_dynamic_info) such
  * that the program can be relocated adequately. This is also useful
  * when handling signals.
@@ -283,7 +293,8 @@ void probe_guest_base(const char 

[PATCH for-6.2 05/10] linux-user: Split mmap prototypes into user-mmap.h

[Peter Maydell]

Split out the mmap prototypes into a new header user-mmap.h
which we only include where required.

Signed-off-by: Peter Maydell 
---
 linux-user/qemu.h  | 20 
 linux-user/user-mmap.h | 34 ++
 linux-user/elfload.c   |  1 +
 linux-user/flatload.c  |  1 +
 linux-user/i386/cpu_loop.c |  1 +
 linux-user/main.c  |  1 +
 linux-user/mmap.c  |  1 +
 linux-user/syscall.c   |  1 +
 8 files changed, 40 insertions(+), 20 deletions(-)
 create mode 100644 linux-user/user-mmap.h

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 9e700d3af18..0cb79990579 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -201,12 +201,6 @@ extern IOCTLEntry ioctl_entries[];
  */
 int info_is_fdpic(struct image_info *info);
 
-uint32_t get_elf_eflags(int fd);
-int load_elf_binary(struct linux_binprm *bprm, struct image_info *info);
-int load_flt_binary(struct linux_binprm *bprm, struct image_info *info);
-
-abi_long memcpy_to_target(abi_ulong dest, const void *src,
-  unsigned long len);
 void target_set_brk(abi_ulong new_brk);
 abi_long do_brk(abi_ulong new_brk);
 void syscall_init(void);
@@ -395,20 +389,6 @@ void sparc64_set_context(CPUSPARCState *env);
 void sparc64_get_context(CPUSPARCState *env);
 #endif
 
-/* mmap.c */
-int target_mprotect(abi_ulong start, abi_ulong len, int prot);
-abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
- int flags, int fd, abi_ulong offset);
-int target_munmap(abi_ulong start, abi_ulong len);
-abi_long target_mremap(abi_ulong old_addr, abi_ulong old_size,
-   abi_ulong new_size, unsigned long flags,
-   abi_ulong new_addr);
-extern unsigned long last_brk;
-extern abi_ulong mmap_next_start;
-abi_ulong mmap_find_vma(abi_ulong, abi_ulong, abi_ulong);
-void mmap_fork_start(void);
-void mmap_fork_end(int child);
-
 /* user access */
 
 #define VERIFY_READ  PAGE_READ
diff --git a/linux-user/user-mmap.h b/linux-user/user-mmap.h
new file mode 100644
index 000..e94fbd10433
--- /dev/null
+++ b/linux-user/user-mmap.h
@@ -0,0 +1,34 @@
+/*
+ * loader.h: prototypes for linux-user guest binary loader
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see .
+ */
+
+#ifndef LINUX_USER_USER_MMAP_H
+#define LINUX_USER_USER_MMAP_H
+
+int target_mprotect(abi_ulong start, abi_ulong len, int prot);
+abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
+ int flags, int fd, abi_ulong offset);
+int target_munmap(abi_ulong start, abi_ulong len);
+abi_long target_mremap(abi_ulong old_addr, abi_ulong old_size,
+   abi_ulong new_size, unsigned long flags,
+   abi_ulong new_addr);
+extern unsigned long last_brk;
+extern abi_ulong mmap_next_start;
+abi_ulong mmap_find_vma(abi_ulong, abi_ulong, abi_ulong);
+void mmap_fork_start(void);
+void mmap_fork_end(int child);
+
+#endif /* LINUX_USER_USER_MMAP_H */
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index d56e00c18b4..f770de31672 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -7,6 +7,7 @@
 
 #include "qemu.h"
 #include "loader.h"
+#include "user-mmap.h"
 #include "disas/disas.h"
 #include "qemu/bitops.h"
 #include "qemu/path.h"
diff --git a/linux-user/flatload.c b/linux-user/flatload.c
index 7484a4a3543..99550061db8 100644
--- a/linux-user/flatload.c
+++ b/linux-user/flatload.c
@@ -37,6 +37,7 @@
 
 #include "qemu.h"
 #include "loader.h"
+#include "user-mmap.h"
 #include "flat.h"
 #include "target_flat.h"
 
diff --git a/linux-user/i386/cpu_loop.c b/linux-user/i386/cpu_loop.c
index db6dbf40cdc..fca0a75d8a1 100644
--- a/linux-user/i386/cpu_loop.c
+++ b/linux-user/i386/cpu_loop.c
@@ -22,6 +22,7 @@
 #include "qemu.h"
 #include "cpu_loop-common.h"
 #include "sighandling.h"
+#include "user-mmap.h"
 
 /***/
 /* CPUX86 core interface */
diff --git a/linux-user/main.c b/linux-user/main.c
index 4ec0e2ad55d..dfcdd80a563 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -51,6 +51,7 @@
 #include "fd-trans.h"
 #include "sighandling.h"
 #include "loader.h"
+#include "user-mmap.h"
 
 #ifndef AT_FLAGS_PRESERVE_ARGV0
 #define AT_FLAGS_PRESERVE_ARGV0_BIT 0
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 0e103859fed..4b182444bbd 

[PATCH for-6.2 07/10] linux-user: Split linux-user internals out of qemu.h

[Peter Maydell]

qemu.h is included in various non-linux-user files (which
mostly want the TaskState struct and the functions for
doing usermode access to guest addresses like lock_user(),
unlock_user(), get_user*(), etc).

Split out the parts that are only used in linux-user itself
into a new user-internals.h. This leaves qemu.h with basically
three things:
 * the definition of the TaskState struct
 * the user-access functions and macros
 * do_brk()
all of which are needed by code outside linux-user that
includes qemu.h.

The addition of all the extra #include lines was done with
  sed -i '/include.*qemu\.h/a #include "user-internals.h"' $(git grep -l 
'include.*qemu\.h' linux-user)
(and then undoing the change to fpa11.h).

Signed-off-by: Peter Maydell 
---
 linux-user/qemu.h| 164 +--
 linux-user/user-internals.h  | 184 +++
 linux-user/aarch64/cpu_loop.c|   1 +
 linux-user/aarch64/signal.c  |   1 +
 linux-user/alpha/cpu_loop.c  |   1 +
 linux-user/alpha/signal.c|   1 +
 linux-user/arm/cpu_loop.c|   1 +
 linux-user/arm/signal.c  |   1 +
 linux-user/cris/cpu_loop.c   |   1 +
 linux-user/cris/signal.c |   1 +
 linux-user/elfload.c |   1 +
 linux-user/exit.c|   1 +
 linux-user/fd-trans.c|   1 +
 linux-user/flatload.c|   1 +
 linux-user/hexagon/cpu_loop.c|   1 +
 linux-user/hexagon/signal.c  |   1 +
 linux-user/hppa/cpu_loop.c   |   1 +
 linux-user/hppa/signal.c |   1 +
 linux-user/i386/cpu_loop.c   |   1 +
 linux-user/i386/signal.c |   1 +
 linux-user/linuxload.c   |   1 +
 linux-user/m68k/cpu_loop.c   |   1 +
 linux-user/m68k/signal.c |   1 +
 linux-user/main.c|   1 +
 linux-user/microblaze/cpu_loop.c |   1 +
 linux-user/microblaze/signal.c   |   1 +
 linux-user/mips/cpu_loop.c   |   1 +
 linux-user/mips/signal.c |   1 +
 linux-user/mmap.c|   1 +
 linux-user/nios2/cpu_loop.c  |   1 +
 linux-user/nios2/signal.c|   1 +
 linux-user/openrisc/cpu_loop.c   |   1 +
 linux-user/openrisc/signal.c |   1 +
 linux-user/ppc/cpu_loop.c|   1 +
 linux-user/ppc/signal.c  |   1 +
 linux-user/riscv/cpu_loop.c  |   1 +
 linux-user/riscv/signal.c|   1 +
 linux-user/s390x/cpu_loop.c  |   1 +
 linux-user/s390x/signal.c|   1 +
 linux-user/semihost.c|   1 +
 linux-user/sh4/cpu_loop.c|   1 +
 linux-user/sh4/signal.c  |   1 +
 linux-user/signal.c  |   1 +
 linux-user/sparc/cpu_loop.c  |   1 +
 linux-user/sparc/signal.c|   1 +
 linux-user/strace.c  |   1 +
 linux-user/syscall.c |   1 +
 linux-user/uaccess.c |   1 +
 linux-user/uname.c   |   1 +
 linux-user/vm86.c|   1 +
 linux-user/xtensa/cpu_loop.c |   1 +
 linux-user/xtensa/signal.c   |   1 +
 52 files changed, 235 insertions(+), 163 deletions(-)
 create mode 100644 linux-user/user-internals.h

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index a82a46236e6..92290a55c0d 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -1,7 +1,6 @@
 #ifndef QEMU_H
 #define QEMU_H
 
-#include "hostdep.h"
 #include "cpu.h"
 #include "exec/exec-all.h"
 #include "exec/cpu_ldst.h"
@@ -10,7 +9,6 @@
 
 #include "exec/user/abitypes.h"
 
-#include "exec/user/thunk.h"
 #include "syscall_defs.h"
 #include "target_syscall.h"
 #include "exec/gdbstub.h"
@@ -166,93 +164,9 @@ typedef struct TaskState {
 struct target_sigaltstack sigaltstack_used;
 } __attribute__((aligned(16))) TaskState;
 
-extern char *exec_path;
-void init_task_state(TaskState *ts);
-void task_settid(TaskState *);
-void stop_all_tasks(void);
-extern const char *qemu_uname_release;
-extern unsigned long mmap_min_addr;
-
-typedef struct IOCTLEntry IOCTLEntry;
-
-typedef abi_long do_ioctl_fn(const IOCTLEntry *ie, uint8_t *buf_temp,
- int fd, int cmd, abi_long arg);
-
-struct IOCTLEntry {
-int target_cmd;
-unsigned int host_cmd;
-const char *name;
-int access;
-do_ioctl_fn *do_ioctl;
-const argtype arg_type[5];
-};
-
-extern IOCTLEntry ioctl_entries[];
-
-#define IOC_R 0x0001
-#define IOC_W 0x0002
-#define IOC_RW (IOC_R | IOC_W)
-
-/*
- * Returns true if the image uses the FDPIC ABI. If this is the case,
- * we have to provide some information (loadmap, pt_dynamic_info) such
- * that the program can be relocated adequately. This is also useful
- * when handling signals.
- */
-int info_is_fdpic(struct image_info *info);
-
-void target_set_brk(abi_ulong new_brk);
-abi_long do_brk(abi_ulong new_brk);
-void syscall_init(void);
-abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
-abi_long arg2, abi_long arg3, abi_long arg4,
-abi_long arg5, abi_long arg6, abi_long arg7,
-abi_long arg8);
-extern 

[PATCH for-6.2 03/10] linux-user: Split signal-related prototypes into sighandling.h

[Peter Maydell]

Split the signal related prototypes into a new header file
sighandling.h, and include it in those places that require it.

Signed-off-by: Peter Maydell 
---
 linux-user/qemu.h| 36 
 linux-user/sighandling.h | 56 
 linux-user/aarch64/cpu_loop.c|  1 +
 linux-user/aarch64/signal.c  |  1 +
 linux-user/alpha/cpu_loop.c  |  1 +
 linux-user/alpha/signal.c|  1 +
 linux-user/arm/cpu_loop.c|  1 +
 linux-user/arm/signal.c  |  1 +
 linux-user/cris/cpu_loop.c   |  1 +
 linux-user/cris/signal.c |  1 +
 linux-user/fd-trans.c|  1 +
 linux-user/hexagon/cpu_loop.c|  1 +
 linux-user/hexagon/signal.c  |  1 +
 linux-user/hppa/cpu_loop.c   |  1 +
 linux-user/hppa/signal.c |  1 +
 linux-user/i386/cpu_loop.c   |  1 +
 linux-user/i386/signal.c |  1 +
 linux-user/m68k/cpu_loop.c   |  1 +
 linux-user/m68k/signal.c |  1 +
 linux-user/main.c|  1 +
 linux-user/microblaze/cpu_loop.c |  1 +
 linux-user/microblaze/signal.c   |  1 +
 linux-user/mips/cpu_loop.c   |  1 +
 linux-user/mips/signal.c |  1 +
 linux-user/nios2/cpu_loop.c  |  1 +
 linux-user/nios2/signal.c|  1 +
 linux-user/openrisc/cpu_loop.c   |  1 +
 linux-user/openrisc/signal.c |  1 +
 linux-user/ppc/cpu_loop.c|  1 +
 linux-user/ppc/signal.c  |  1 +
 linux-user/riscv/cpu_loop.c  |  1 +
 linux-user/riscv/signal.c|  1 +
 linux-user/s390x/cpu_loop.c  |  1 +
 linux-user/s390x/signal.c|  1 +
 linux-user/sh4/cpu_loop.c|  1 +
 linux-user/sh4/signal.c  |  1 +
 linux-user/signal.c  |  1 +
 linux-user/sparc/cpu_loop.c  |  1 +
 linux-user/sparc/signal.c|  1 +
 linux-user/syscall.c |  1 +
 linux-user/xtensa/cpu_loop.c |  1 +
 linux-user/xtensa/signal.c   |  1 +
 42 files changed, 96 insertions(+), 36 deletions(-)
 create mode 100644 linux-user/sighandling.h

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index ad2d49fed9f..76d3f5e7eb9 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -415,42 +415,6 @@ extern long safe_syscall_base(int *pending, long number, 
...);
 /* syscall.c */
 int host_to_target_waitstatus(int status);
 
-/* signal.c */
-void process_pending_signals(CPUArchState *cpu_env);
-void signal_init(void);
-int queue_signal(CPUArchState *env, int sig, int si_type,
- target_siginfo_t *info);
-void host_to_target_siginfo(target_siginfo_t *tinfo, const siginfo_t *info);
-void target_to_host_siginfo(siginfo_t *info, const target_siginfo_t *tinfo);
-int target_to_host_signal(int sig);
-int host_to_target_signal(int sig);
-long do_sigreturn(CPUArchState *env);
-long do_rt_sigreturn(CPUArchState *env);
-abi_long do_sigaltstack(abi_ulong uss_addr, abi_ulong uoss_addr,
-CPUArchState *env);
-int do_sigprocmask(int how, const sigset_t *set, sigset_t *oldset);
-abi_long do_swapcontext(CPUArchState *env, abi_ulong uold_ctx,
-abi_ulong unew_ctx, abi_long ctx_size);
-/**
- * block_signals: block all signals while handling this guest syscall
- *
- * Block all signals, and arrange that the signal mask is returned to
- * its correct value for the guest before we resume execution of guest code.
- * If this function returns non-zero, then the caller should immediately
- * return -TARGET_ERESTARTSYS to the main loop, which will take the pending
- * signal and restart execution of the syscall.
- * If block_signals() returns zero, then the caller can continue with
- * emulation of the system call knowing that no signals can be taken
- * (and therefore that no race conditions will result).
- * This should only be called once, because if it is called a second time
- * it will always return non-zero. (Think of it like a mutex that can't
- * be recursively locked.)
- * Signals will be unblocked again by process_pending_signals().
- *
- * Return value: non-zero if there was a pending signal, zero if not.
- */
-int block_signals(void); /* Returns non zero if signal pending */
-
 #ifdef TARGET_I386
 /* vm86.c */
 void save_v86_state(CPUX86State *env);
diff --git a/linux-user/sighandling.h b/linux-user/sighandling.h
new file mode 100644
index 000..8da369b0381
--- /dev/null
+++ b/linux-user/sighandling.h
@@ -0,0 +1,56 @@
+/*
+ * sighandling.h: prototypes for linux-user signal handling
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  

[PATCH for-6.2 00/10] linux-user: split internals out of qemu.h

[Peter Maydell]

linux-user/qemu.h is an awkward header, for two reasons:
 (1) its name suggests it's a rather common generic header,
 but it's actually specific to the usermode emulators
 (2) it is a mix of:
   * lots of things internal to the implementation of linux-user
   * functions that a few files outside linux-user want
 (mostly the user-access functions like lock_user,
 get/put_user_*, etc, and also the TaskStruct definition)

This patchset tries to clean it up a bit by at least splitting
most of the "just internal to linux-user" parts out of qemu.h
and putting them in a handful of different .h files that are
then included by the linux-user files that need them.

I think the ideal would probably be to eventually junk
qemu.h entirely and have a few separate headers specifically
for the bits that non-linux-user code needs (eg a 'user-access.h'
for the get/put_user stuff), perhaps located somewhere that
means we don't need to put linux-user/ on the include path.
But that's awkward as it needs interaction with bsd-user too.
So this much cleanup seemed like a reasonable start...

Based-on: 20210717103017.20491-1-peter.mayd...@linaro.org
("target/hexagon: Drop include of qemu.h")

thanks
-- PMM

Peter Maydell (10):
  linux-user: Fix coding style nits in qemu.h
  linux-user: Split strace prototypes into strace.h
  linux-user: Split signal-related prototypes into sighandling.h
  linux-user: Split loader-related prototypes into loader.h
  linux-user: Split mmap prototypes into user-mmap.h
  linux-user: Split safe-syscall macro into its own header
  linux-user: Split linux-user internals out of qemu.h
  linux-user: Don't include gdbstub.h in qemu.h
  linux-user: Drop unneeded includes from qemu.h
  linux-user: Move DEBUG_REMAP undef to uaccess.c

 linux-user/loader.h  |  59 +
 linux-user/qemu.h| 432 ++-
 linux-user/safe-syscall.h| 154 +++
 linux-user/sighandling.h |  56 
 linux-user/strace.h  |  38 +++
 linux-user/user-internals.h  | 186 +
 linux-user/user-mmap.h   |  34 +++
 gdbstub.c|   2 +-
 linux-user/aarch64/cpu_loop.c|   2 +
 linux-user/aarch64/signal.c  |   2 +
 linux-user/alpha/cpu_loop.c  |   2 +
 linux-user/alpha/signal.c|   2 +
 linux-user/arm/cpu_loop.c|   2 +
 linux-user/arm/signal.c  |   2 +
 linux-user/cris/cpu_loop.c   |   2 +
 linux-user/cris/signal.c |   2 +
 linux-user/elfload.c |   3 +
 linux-user/exit.c|   2 +
 linux-user/fd-trans.c|   2 +
 linux-user/flatload.c|   3 +
 linux-user/hexagon/cpu_loop.c|   2 +
 linux-user/hexagon/signal.c  |   2 +
 linux-user/hppa/cpu_loop.c   |   2 +
 linux-user/hppa/signal.c |   2 +
 linux-user/i386/cpu_loop.c   |   3 +
 linux-user/i386/signal.c |   2 +
 linux-user/linuxload.c   |   2 +
 linux-user/m68k/cpu_loop.c   |   2 +
 linux-user/m68k/signal.c |   2 +
 linux-user/main.c|   5 +
 linux-user/microblaze/cpu_loop.c |   2 +
 linux-user/microblaze/signal.c   |   2 +
 linux-user/mips/cpu_loop.c   |   2 +
 linux-user/mips/signal.c |   2 +
 linux-user/mmap.c|   2 +
 linux-user/nios2/cpu_loop.c  |   2 +
 linux-user/nios2/signal.c|   2 +
 linux-user/openrisc/cpu_loop.c   |   2 +
 linux-user/openrisc/signal.c |   2 +
 linux-user/ppc/cpu_loop.c|   2 +
 linux-user/ppc/signal.c  |   2 +
 linux-user/riscv/cpu_loop.c  |   2 +
 linux-user/riscv/signal.c|   2 +
 linux-user/s390x/cpu_loop.c  |   2 +
 linux-user/s390x/signal.c|   2 +
 linux-user/semihost.c|   1 +
 linux-user/sh4/cpu_loop.c|   2 +
 linux-user/sh4/signal.c  |   2 +
 linux-user/signal.c  |   6 +
 linux-user/sparc/cpu_loop.c  |   2 +
 linux-user/sparc/signal.c|   2 +
 linux-user/strace.c  |   3 +
 linux-user/syscall.c |   6 +
 linux-user/uaccess.c |   3 +
 linux-user/uname.c   |   1 +
 linux-user/vm86.c|   1 +
 linux-user/xtensa/cpu_loop.c |   2 +
 linux-user/xtensa/signal.c   |   2 +
 semihosting/arm-compat-semi.c|   2 +-
 target/m68k/m68k-semi.c  |   2 +-
 target/nios2/nios2-semi.c|   2 +-
 thunk.c  |   1 +
 62 files changed, 661 insertions(+), 420 deletions(-)
 create mode 100644 linux-user/loader.h
 create mode 100644 linux-user/safe-syscall.h
 create mode 100644 linux-user/sighandling.h
 create mode 100644 linux-user/strace.h
 create mode 100644 linux-user/user-internals.h
 create mode 100644 linux-user/user-mmap.h

-- 
2.20.1

[PATCH for-6.2 02/10] linux-user: Split strace prototypes into strace.h

[Peter Maydell]

The functions implemented in strace.c are only used in a few files in
linux-user; split them out of qemu.h and into a new strace.h header
which we include in the places that need it.

Signed-off-by: Peter Maydell 
---
 linux-user/qemu.h| 18 --
 linux-user/strace.h  | 38 ++
 linux-user/signal.c  |  1 +
 linux-user/strace.c  |  2 ++
 linux-user/syscall.c |  1 +
 5 files changed, 42 insertions(+), 18 deletions(-)
 create mode 100644 linux-user/strace.h

diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index 34b975ba502..ad2d49fed9f 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -415,24 +415,6 @@ extern long safe_syscall_base(int *pending, long number, 
...);
 /* syscall.c */
 int host_to_target_waitstatus(int status);
 
-/* strace.c */
-void print_syscall(void *cpu_env, int num,
-   abi_long arg1, abi_long arg2, abi_long arg3,
-   abi_long arg4, abi_long arg5, abi_long arg6);
-void print_syscall_ret(void *cpu_env, int num, abi_long ret,
-   abi_long arg1, abi_long arg2, abi_long arg3,
-   abi_long arg4, abi_long arg5, abi_long arg6);
-/**
- * print_taken_signal:
- * @target_signum: target signal being taken
- * @tinfo: target_siginfo_t which will be passed to the guest for the signal
- *
- * Print strace output indicating that this signal is being taken by the guest,
- * in a format similar to:
- * --- SIGSEGV {si_signo=SIGSEGV, si_code=SI_KERNEL, si_addr=0} ---
- */
-void print_taken_signal(int target_signum, const target_siginfo_t *tinfo);
-
 /* signal.c */
 void process_pending_signals(CPUArchState *cpu_env);
 void signal_init(void);
diff --git a/linux-user/strace.h b/linux-user/strace.h
new file mode 100644
index 000..1e232d07fc8
--- /dev/null
+++ b/linux-user/strace.h
@@ -0,0 +1,38 @@
+/*
+ * strace.h: prototypes for linux-user builtin strace handling
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, see .
+ */
+
+#ifndef LINUX_USER_STRACE_H
+#define LINUX_USER_STRACE_H
+
+void print_syscall(void *cpu_env, int num,
+   abi_long arg1, abi_long arg2, abi_long arg3,
+   abi_long arg4, abi_long arg5, abi_long arg6);
+void print_syscall_ret(void *cpu_env, int num, abi_long ret,
+   abi_long arg1, abi_long arg2, abi_long arg3,
+   abi_long arg4, abi_long arg5, abi_long arg6);
+/**
+ * print_taken_signal:
+ * @target_signum: target signal being taken
+ * @tinfo: target_siginfo_t which will be passed to the guest for the signal
+ *
+ * Print strace output indicating that this signal is being taken by the guest,
+ * in a format similar to:
+ * --- SIGSEGV {si_signo=SIGSEGV, si_code=SI_KERNEL, si_addr=0} ---
+ */
+void print_taken_signal(int target_signum, const target_siginfo_t *tinfo);
+
+#endif /* LINUX_USER_STRACE_H */
diff --git a/linux-user/signal.c b/linux-user/signal.c
index a8faea6f090..ee1934947ac 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -22,6 +22,7 @@
 #include 
 
 #include "qemu.h"
+#include "strace.h"
 #include "trace.h"
 #include "signal-common.h"
 
diff --git a/linux-user/strace.c b/linux-user/strace.c
index cce0a5d1e35..ee3429fae82 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -1,4 +1,5 @@
 #include "qemu/osdep.h"
+
 #include 
 #include 
 #include 
@@ -14,6 +15,7 @@
 #include 
 #include 
 #include "qemu.h"
+#include "strace.h"
 
 struct syscallname {
 int nr;
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 376629c6891..4932eebd5e5 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -127,6 +127,7 @@
 #include "uname.h"
 
 #include "qemu.h"
+#include "strace.h"
 #include "qemu/guest-random.h"
 #include "qemu/selfmap.h"
 #include "user/syscall-trace.h"
-- 
2.20.1

RE: [PATCH] target/hexagon: Drop include of qemu.h

[Taylor Simpson]

> -Original Message-
> From: Peter Maydell 
> Sent: Saturday, July 17, 2021 4:30 AM
> To: qemu-devel@nongnu.org
> Cc: Taylor Simpson 
> Subject: [PATCH] target/hexagon: Drop include of qemu.h
> 
> The qemu.h file is a CONFIG_USER_ONLY header; it doesn't appear on the
> include path for softmmu builds.  Currently we include it unconditionally in
> target/hexagon/op_helper.c.  We used to need it for the put_user_*() and
> get_user_*() functions, but now that we have removed the uses of those
> from op_helper.c, the only reason it's still there is that we're implicitly 
> relying
> on it pulling in some other headers.
> 
> Explicitly include the headers we need for other functions, and drop the
> include of qemu.h.
> 
> Signed-off-by: Peter Maydell 
> ---
> Based-on: 1626384156-6248-1-git-send-email-tsimp...@quicinc.com
> ("[PATCH v3 0/2] SIGSEGV fixes")


Reviewed-by: Taylor Simpson 

Thanks Peter,
I'll add this to the series and send you a pull request.


Taylor

Re: [PULL 44/48] meson: Introduce target-specific Kconfig

[Peter Maydell]

On Thu, 8 Jul 2021 at 16:49, Paolo Bonzini  wrote:
>
> From: Philippe Mathieu-Daudé 
>
> Add a target-specific Kconfig. We need the definitions in Kconfig so
> the minikconf tool can verify they exits. However CONFIG_FOO is only
> enabled for target foo via the meson.build rules.
>
> Two architecture have a particularity, ARM and MIPS. As their
> translators have been split you can potentially build a plain 32 bit
> build along with a 64-bit version including the 32-bit subset.
>
> Signed-off-by: Philippe Mathieu-Daudé 
> Message-Id: <2021013316.232778-6-f4...@amsat.org>
> Signed-off-by: Alex Bennée 
> Reviewed-by: Thomas Huth 
> Message-Id: <20210707131744.26027-2-alex.ben...@linaro.org>
> Signed-off-by: Paolo Bonzini 

Hi; this change (commit f4063f9c319e392 in master) seems to have
resulted in qemu-system-tricore no longer building the
'tricore_testboard'. Before this commit:

$ ./build/tricore/qemu-system-tricore -M help
Supported machines are:
KIT_AURIX_TC277_TRB  Infineon AURIX TriBoard TC277 (D-Step)
none empty machine
tricore_testboarda minimal TriCore board

After this commit, 'tricore_testboard' no longer appears in the list.
The hw/tricore/meson.build uses "when: 'CONFIG_TRICORE'" to
compile the source files for the board, so presumably that
CONFIG switch is no longer being defined ?

This in turn breaks 'make check-tcg' for builds which build
tricore-softmmu, because some of the tests there want to run
code on the tricore_testboard:

[...]
  RUN tests for tricore
  TESTtest_abs.tst on tricore
qemu-system-tricore: unsupported machine type
Use -machine help to list supported machines
../Makefile.target:168: recipe for target 'run-test_abs.tst' failed


(I guess we're not running "check-tcg" in gitlab CI. I do run
it locally, but only on a linux-user-only build tree, which is
why I didn't see this earlier.)

thanks
-- PMM

[PATCH v3 08/13] target/avr: Advance pc in avr_tr_breakpoint_check

[Richard Henderson]

Since 0b00b0c1e05b, tb->size must not be zero.
Advance pc so that the breakpoint covers the insn at the bp.

Signed-off-by: Richard Henderson 
---
 target/avr/translate.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/avr/translate.c b/target/avr/translate.c
index 8237a03c23..d768063d65 100644
--- a/target/avr/translate.c
+++ b/target/avr/translate.c
@@ -2950,6 +2950,7 @@ static bool avr_tr_breakpoint_check(DisasContextBase 
*dcbase, CPUState *cs,
 DisasContext *ctx = container_of(dcbase, DisasContext, base);
 
 gen_breakpoint(ctx);
+ctx->base.pc_next += 2; /* advance by minimum insn len so tb->size != 0 */
 return true;
 }
 
-- 
2.25.1

[PATCH v3 10/13] target/riscv: Reduce riscv_tr_breakpoint_check pc advance to 2

[Richard Henderson]

The actual number of bytes advanced need not be 100% exact,
but we should not cross a page when the insn would not.

If rvc is enabled, the minimum insn size is 2.

Signed-off-by: Richard Henderson 
---
 target/riscv/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index deda0c8a44..5527f37ada 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -973,7 +973,7 @@ static bool riscv_tr_breakpoint_check(DisasContextBase 
*dcbase, CPUState *cpu,
[tb->pc, tb->pc + tb->size) in order to for it to be
properly cleared -- thus we increment the PC here so that
the logic setting tb->size below does the right thing.  */
-ctx->base.pc_next += 4;
+ctx->base.pc_next += 2;
 return true;
 }
 
-- 
2.25.1

[PATCH v3 09/13] target/mips: Reduce mips_tr_breakpoint_check pc advance to 2

[Richard Henderson]

The actual number of bytes advanced need not be 100% exact,
but we should not cross a page when the insn would not.

If mips16 or mips32e are enabled, the minimum insn size is 2.

Signed-off-by: Richard Henderson 
---
 target/mips/tcg/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index fd980ea966..ef00fbd2ac 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -16192,7 +16192,7 @@ static bool mips_tr_breakpoint_check(DisasContextBase 
*dcbase, CPUState *cs,
  * properly cleared -- thus we increment the PC here so that
  * the logic setting tb->size below does the right thing.
  */
-ctx->base.pc_next += 4;
+ctx->base.pc_next += 2;
 return true;
 }
 
-- 
2.25.1

[PATCH v3 13/13] accel/tcg: Encode breakpoint info into tb->cflags

[Richard Henderson]

Having this data in cflags means that hashing takes care
of selecting a TB with or without exceptions built in.
Which means that we no longer need to flush all TBs.

This does require that we single-step while we're within a page
that contains a breakpoint, so it's not yet ideal, but should be
an improvement over some corner-case slowdowns.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/404
Signed-off-by: Richard Henderson 
---
 include/exec/exec-all.h   |  7 
 accel/tcg/cpu-exec.c  | 68 ++-
 accel/tcg/translate-all.c |  4 --
 accel/tcg/translator.c| 85 +--
 cpu.c | 24 ---
 5 files changed, 119 insertions(+), 69 deletions(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 6873cce8df..7ab2578f71 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -502,9 +502,16 @@ struct TranslationBlock {
 #define CF_USE_ICOUNT0x0002
 #define CF_INVALID   0x0004 /* TB is stale. Set with @jmp_lock held */
 #define CF_PARALLEL  0x0008 /* Generate code for a parallel context */
+#define CF_BP_MASK   0x0030 /* See below */
+#define CF_BP_SHIFT  20
 #define CF_CLUSTER_MASK  0xff00 /* Top 8 bits are cluster ID */
 #define CF_CLUSTER_SHIFT 24
 
+#define CF_BP_NONE   (0 << CF_BP_SHIFT) /* TB does not interact with BPs */
+#define CF_BP_SSTEP  (1 << CF_BP_SHIFT) /* gdbstub single-step in effect */
+#define CF_BP_GDB(2 << CF_BP_SHIFT) /* gdbstub breakpoint at tb->pc */
+#define CF_BP_CPU(3 << CF_BP_SHIFT) /* arch breakpoint at tb->pc */
+
 /* Per-vCPU dynamic tracing state used to generate this TB */
 uint32_t trace_vcpu_dstate;
 
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 4d043a11aa..179a425ece 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -222,6 +222,65 @@ static inline void log_cpu_exec(target_ulong pc, CPUState 
*cpu,
 }
 }
 
+static uint32_t cflags_for_breakpoints(CPUState *cpu, target_ulong pc,
+   uint32_t cflags)
+{
+uint32_t bflags = 0;
+
+if (unlikely(cpu->singlestep_enabled)) {
+bflags = CF_BP_SSTEP;
+} else {
+bool match_page = false;
+CPUBreakpoint *bp;
+
+QTAILQ_FOREACH(bp, >breakpoints, entry) {
+/* Note exact pc matches */
+if (pc == bp->pc) {
+if (bp->flags & BP_GDB) {
+bflags = CF_BP_GDB;
+break;
+}
+if (bp->flags & BP_CPU) {
+bflags = CF_BP_CPU;
+break;
+}
+}
+/* Note page matches */
+if (((pc ^ bp->pc) & TARGET_PAGE_MASK) == 0) {
+match_page = true;
+}
+}
+
+if (likely(!bflags)) {
+if (likely(!match_page)) {
+return cflags;
+}
+
+/*
+ * Within the same page as a breakpoint, single-step,
+ * returning to helper_lookup_tb_ptr after each looking
+ * for the actual breakpoint.
+ *
+ * TODO: Perhaps better to record all of the TBs associated
+ * with a given virtual page that contains a breakpoint, and
+ * then invalidate them when a new overlapping breakpoint is
+ * set on the page.  Non-overlapping TBs would not be
+ * invalidated, nor would any TB need to be invalidated as
+ * breakpoints are removed.
+ */
+bflags = CF_NO_GOTO_TB;
+}
+}
+
+/*
+ * Reduce the TB to one insn.
+ * In the case of a BP hit, we will be raising an exception anyway.
+ * In the case of a page hit, return to helper_lookup_tb_ptr after
+ * every insn to look for the breakpoint.
+ */
+return (cflags & ~CF_COUNT_MASK) | 1 | bflags;
+}
+
 /**
  * helper_lookup_tb_ptr: quick check for next tb
  * @env: current cpu state
@@ -235,11 +294,13 @@ const void *HELPER(lookup_tb_ptr)(CPUArchState *env)
 CPUState *cpu = env_cpu(env);
 TranslationBlock *tb;
 target_ulong cs_base, pc;
-uint32_t flags;
+uint32_t flags, cflags;
 
 cpu_get_tb_cpu_state(env, , _base, );
 
-tb = tb_lookup(cpu, pc, cs_base, flags, curr_cflags(cpu));
+cflags = cflags_for_breakpoints(cpu, pc, curr_cflags(cpu));
+
+tb = tb_lookup(cpu, pc, cs_base, flags, cflags);
 if (tb == NULL) {
 return tcg_code_gen_epilogue;
 }
@@ -346,6 +407,8 @@ void cpu_exec_step_atomic(CPUState *cpu)
 cflags &= ~CF_PARALLEL;
 /* After 1 insn, return and release the exclusive lock. */
 cflags |= CF_NO_GOTO_TB | CF_NO_GOTO_PTR | 1;
+/* Raise any post-instruction debug exceptions. */
+cflags = cflags_for_breakpoints(cpu, pc, cflags);
 
 tb = tb_lookup(cpu, pc, cs_base, flags, cflags);
 if 

[PATCH v3 06/13] accel/tcg: Use CF_NO_GOTO_{TB, PTR} in cpu_exec_step_atomic

[Richard Henderson]

Request that the one TB returns immediately, so that
we release the exclusive lock as soon as possible.

Reviewed-by: Peter Maydell 
Reviewed-by: Alex Bennée 
Signed-off-by: Richard Henderson 
---
 accel/tcg/cpu-exec.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 2206c463f5..5bb099174f 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -330,8 +330,7 @@ void cpu_exec_step_atomic(CPUState *cpu)
 CPUArchState *env = (CPUArchState *)cpu->env_ptr;
 TranslationBlock *tb;
 target_ulong cs_base, pc;
-uint32_t flags;
-uint32_t cflags = (curr_cflags(cpu) & ~CF_PARALLEL) | 1;
+uint32_t flags, cflags;
 int tb_exit;
 
 if (sigsetjmp(cpu->jmp_env, 0) == 0) {
@@ -341,8 +340,14 @@ void cpu_exec_step_atomic(CPUState *cpu)
 cpu->running = true;
 
 cpu_get_tb_cpu_state(env, , _base, );
-tb = tb_lookup(cpu, pc, cs_base, flags, cflags);
 
+cflags = curr_cflags(cpu);
+/* Execute in a serial context. */
+cflags &= ~CF_PARALLEL;
+/* After 1 insn, return and release the exclusive lock. */
+cflags |= CF_NO_GOTO_TB | CF_NO_GOTO_PTR | 1;
+
+tb = tb_lookup(cpu, pc, cs_base, flags, cflags);
 if (tb == NULL) {
 mmap_lock();
 tb = tb_gen_code(cpu, pc, cs_base, flags, cflags);
-- 
2.25.1

R: R: [PULL 0/3] ppc-for-6.1 queue 20210713

[luigi burdo]

Hi Zoltan can be the gcc, in past on ppc we face the same kind of issue.
 i will ask some other guys in the debian ppc ML if can try to build qemu and 
see if they have the same issue. in positive case we will report to the 
mantainer.
Thanks
Luigi


Da: BALATON Zoltan 
Inviato: sabato 17 luglio 2021 21:55
A: luigi burdo 
Cc: David Gibson ; peter.mayd...@linaro.org 
; gr...@kaod.org ; 
qemu-...@nongnu.org ; qemu-devel@nongnu.org 

Oggetto: Re: R: [PULL 0/3] ppc-for-6.1 queue 20210713

On Sat, 17 Jul 2021, luigi burdo wrote:
> Hi
> i small report
> im try to build last relesase on a PowerMac G5 quad on debian sid PPC64 but 
> ld exit with an error:

Looks like it's gcc that crashes not ld. If you're sure it's not a
hardware problem this may be a gcc bug. Sometimes big compile jobs can
break if a machine is not stable but if it always fails at the same place
and does not fail when compiling something else like a Linux kernel then
it's more likely to be an actual bug. That function has some crazy use of
macros so maybe it's exposing some bug in gcc. You could restrict what
targets you build with --target-list configure option of QEMU if you don't
actually need mips (compiling will be faster if you only build the targets
you need) or you can try with clang if available in your distro to check
if that works better. If this can be consistently reproduced you could try
reporting it to gcc as the error message suggests. Probably not much QEMU
can do about it.

Regards,
BALATON Zoltan

> gigi@debian:~/src/tags/ppc-for-6.1-20210713/build$ ninja
> [3864/9215] Compiling C object 
> libqemu-mipsel-softmmu.fa.p/target_mips_tcg_msa_helper.c.o
> FAILED: libqemu-mipsel-softmmu.fa.p/target_mips_tcg_msa_helper.c.o
> cc -Ilibqemu-mipsel-softmmu.fa.p -I. -I.. -Itarget/mips -I../target/mips 
> -I../capstone/include/capstone -Iqapi -Itrace -Iui -Iui/shader 
> -I/usr/include/pixman-1 -I/usr/include/glib-2.0 
> -I/usr/lib/powerpc64-linux-gnu/glib-2.0/include -fdiagnostics-color=auto 
> -pipe -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem 
> /home/gigi/src/tags/ppc-for-6.1-20210713/linux-headers -isystem linux-headers 
> -iquote . -iquote /home/gigi/src/tags/ppc-for-6.1-20210713 -iquote 
> /home/gigi/src/tags/ppc-for-6.1-20210713/include -iquote 
> /home/gigi/src/tags/ppc-for-6.1-20210713/disas/libvixl -iquote 
> /home/gigi/src/tags/ppc-for-6.1-20210713/tcg/ppc -pthread -U_FORTIFY_SOURCE 
> -D_FORTIFY_SOURCE=2 -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 
> -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wundef 
> -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv 
> -Wold-style-declaration -Wold-style-definition -Wtype-limits 
> -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempt
 y-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined 
-Wimplicit-fallthrough=2 -Wno-missing-include-dirs -Wno-shift-negative-value 
-Wno-psabi -fstack-protector-strong -fPIE -isystem../linux-headers 
-isystemlinux-headers -DNEED_CPU_H 
'-DCONFIG_TARGET="mipsel-softmmu-config-target.h"' 
'-DCONFIG_DEVICES="mipsel-softmmu-config-devices.h"' -MD -MQ 
libqemu-mipsel-softmmu.fa.p/target_mips_tcg_msa_helper.c.o -MF 
libqemu-mipsel-softmmu.fa.p/target_mips_tcg_msa_helper.c.o.d -o 
libqemu-mipsel-softmmu.fa.p/target_mips_tcg_msa_helper.c.o -c 
../target/mips/tcg/msa_helper.c
> during RTL pass: sched1
> ../target/mips/tcg/msa_helper.c: In function ‘helper_msa_fmin_df’:
> ../target/mips/tcg/msa_helper.c:7536:1: internal compiler error: Errore di 
> segmentazione
> 7536 | }
>  | ^
> 0x3fffa7b8e1c3 generic_start_main
> ../csu/libc-start.c:308
> 0x3fffa7b8e3d3 __libc_start_main
> ../sysdeps/unix/sysv/linux/powerpc/libc-start.c:98
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See  for instructions.
> [3869/9215] Compiling C object 
> libqemu-mipsel-softmmu.fa.p/target_mips_tcg_translate.c.o
> ninja: build stopped: subcommand failed.
>
> Ciao
> Luigi
> 
> Da: Qemu-ppc  per conto 
> di David Gibson 
> Inviato: martedì 13 luglio 2021 04:07
> A: peter.mayd...@linaro.org ; gr...@kaod.org 
> 
> Cc: qemu-...@nongnu.org ; qemu-devel@nongnu.org 
> ; David Gibson 
> Oggetto: [PULL 0/3] ppc-for-6.1 queue 20210713
>
> The following changes since commit 57e28d34c0cb04abf7683ac6a12c87ede447c320:
>
>  Merge remote-tracking branch 'remotes/cohuck-gitlab/tags/s390x-20210708' 
> into staging (2021-07-12 19:15:11 +0100)
>
> are available in the Git repository at:
>
>  https://gitlab.com/dgibson/qemu.git tags/ppc-for-6.1-20210713
>
> for you to fetch changes up to c785a40179b10ce2d7a4afdb04f63c98d53a1756:
>
>  mv64361: Remove extra break from a switch case (2021-07-13 10:12:17 +1000)
>
> 
> ppc patch queue 2021-07-13
>
> I thought I'd sent the last PR before the 6.1 soft freeze, but
> 

[PATCH v3 05/13] accel/tcg: Handle -singlestep in curr_cflags

[Richard Henderson]

Exchange the test in translator_use_goto_tb for CF_NO_GOTO_TB,
and the test in tb_gen_code for setting CF_COUNT_MASK to 1.

Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 accel/tcg/cpu-exec.c  | 8 +++-
 accel/tcg/translate-all.c | 2 +-
 accel/tcg/translator.c| 2 +-
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 70ea3c7d68..2206c463f5 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -149,7 +149,13 @@ uint32_t curr_cflags(CPUState *cpu)
 {
 uint32_t cflags = cpu->tcg_cflags;
 
-if (qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
+/*
+ * For singlestep and -d nochain, suppress goto_tb so that
+ * we can log -d cpu,exec after every TB.
+ */
+if (singlestep) {
+cflags |= CF_NO_GOTO_TB | 1;
+} else if (qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
 cflags |= CF_NO_GOTO_TB;
 }
 
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 5cc01d693b..bf82c15aab 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1432,7 +1432,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
 }
 QEMU_BUILD_BUG_ON(CF_COUNT_MASK + 1 != TCG_MAX_INSNS);
 
-if (cpu->singlestep_enabled || singlestep) {
+if (cpu->singlestep_enabled) {
 max_insns = 1;
 }
 
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 2ea5a74f30..a59eb7c11b 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -39,7 +39,7 @@ bool translator_use_goto_tb(DisasContextBase *db, 
target_ulong dest)
 }
 
 /* Suppress goto_tb in the case of single-steping.  */
-if (db->singlestep_enabled || singlestep) {
+if (db->singlestep_enabled) {
 return false;
 }
 
-- 
2.25.1

[PATCH v3 02/13] accel/tcg: Move curr_cflags into cpu-exec.c

[Richard Henderson]

We will shortly have more than a simple member read here,
with stuff not necessarily exposed to exec/exec-all.h.

Reviewed-by: Peter Maydell 
Reviewed-by: Alex Bennée 
Signed-off-by: Richard Henderson 
---
 include/exec/exec-all.h | 5 +
 accel/tcg/cpu-exec.c| 5 +
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index dfe82ed19c..ae7603ca75 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -565,10 +565,7 @@ static inline uint32_t tb_cflags(const TranslationBlock 
*tb)
 }
 
 /* current cflags for hashing/comparison */
-static inline uint32_t curr_cflags(CPUState *cpu)
-{
-return cpu->tcg_cflags;
-}
+uint32_t curr_cflags(CPUState *cpu);
 
 /* TranslationBlock invalidate API */
 #if defined(CONFIG_USER_ONLY)
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index e22bcb99f7..ef4214d893 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -145,6 +145,11 @@ static void init_delay_params(SyncClocks *sc, const 
CPUState *cpu)
 }
 #endif /* CONFIG USER ONLY */
 
+uint32_t curr_cflags(CPUState *cpu)
+{
+return cpu->tcg_cflags;
+}
+
 /* Might cause an exception, so have a longjmp destination ready */
 static inline TranslationBlock *tb_lookup(CPUState *cpu, target_ulong pc,
   target_ulong cs_base,
-- 
2.25.1

[PATCH v3 11/13] accel/tcg: Adjust interface of TranslatorOps.breakpoint_check

[Richard Henderson]

We don't need the whole CPUBreakpoint structure in the check,
only the flags.  Return the instruction length to consolidate
the adjustment of db->pc_next.

Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 include/exec/translator.h | 17 +--
 accel/tcg/translator.c| 40 ---
 target/alpha/translate.c  | 12 +++
 target/arm/translate-a64.c| 14 
 target/arm/translate.c| 20 +++---
 target/avr/translate.c|  7 +++---
 target/cris/translate.c   | 14 
 target/hexagon/translate.c| 13 +++-
 target/hppa/translate.c   |  7 +++---
 target/i386/tcg/translate.c   | 15 -
 target/m68k/translate.c   | 14 +++-
 target/microblaze/translate.c | 14 +++-
 target/mips/tcg/translate.c   | 14 
 target/nios2/translate.c  | 13 +++-
 target/openrisc/translate.c   | 11 +++---
 target/ppc/translate.c| 13 +++-
 target/riscv/translate.c  | 11 +++---
 target/rx/translate.c |  8 +++
 target/s390x/tcg/translate.c  | 12 ---
 target/sh4/translate.c| 12 ---
 target/sparc/translate.c  |  9 
 target/tricore/translate.c| 13 +++-
 target/xtensa/translate.c | 12 ---
 23 files changed, 115 insertions(+), 200 deletions(-)

diff --git a/include/exec/translator.h b/include/exec/translator.h
index dd9c06d40d..433b753c5c 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -92,11 +92,15 @@ typedef struct DisasContextBase {
  * @breakpoint_check:
  *  When called, the breakpoint has already been checked to match the PC,
  *  but the target may decide the breakpoint missed the address
- *  (e.g., due to conditions encoded in their flags).  Return true to
- *  indicate that the breakpoint did hit, in which case no more breakpoints
- *  are checked.  If the breakpoint did hit, emit any code required to
- *  signal the exception, and set db->is_jmp as necessary to terminate
- *  the main loop.
+ *  (e.g., due to conditions encoded in their flags), in which case
+ *  db->is_jmp may be left as DISAS_NEXT or DISAS_TOO_MANY to indicate
+ *  that the insn should be translated.  Anything other than those two
+ *  will be taken to indicate an exception has been raised, but in most
+ *  cases db->is_jmp should be set to DISAS_NORETURN.
+ *
+ *  Return the minimum instruction size that should be applied to the TB.
+ *  The size of any TB cannot be zero, as that breaks the math used to
+ *  invalidate TBs.
  *
  * @translate_insn:
  *  Disassemble one instruction and set db->pc_next for the start
@@ -113,8 +117,7 @@ typedef struct TranslatorOps {
 void (*init_disas_context)(DisasContextBase *db, CPUState *cpu);
 void (*tb_start)(DisasContextBase *db, CPUState *cpu);
 void (*insn_start)(DisasContextBase *db, CPUState *cpu);
-bool (*breakpoint_check)(DisasContextBase *db, CPUState *cpu,
- const CPUBreakpoint *bp);
+int (*breakpoint_check)(DisasContextBase *db, CPUState *cpu, int flags);
 void (*translate_insn)(DisasContextBase *db, CPUState *cpu);
 void (*tb_stop)(DisasContextBase *db, CPUState *cpu);
 void (*disas_log)(const DisasContextBase *db, CPUState *cpu);
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index a59eb7c11b..1c44d096d8 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -50,7 +50,6 @@ bool translator_use_goto_tb(DisasContextBase *db, 
target_ulong dest)
 void translator_loop(const TranslatorOps *ops, DisasContextBase *db,
  CPUState *cpu, TranslationBlock *tb, int max_insns)
 {
-int bp_insn = 0;
 bool plugin_enabled;
 
 /* Initialize DisasContext */
@@ -91,19 +90,35 @@ void translator_loop(const TranslatorOps *ops, 
DisasContextBase *db,
 CPUBreakpoint *bp;
 QTAILQ_FOREACH(bp, >breakpoints, entry) {
 if (bp->pc == db->pc_next) {
-if (ops->breakpoint_check(db, cpu, bp)) {
-bp_insn = 1;
-break;
+int len = ops->breakpoint_check(db, cpu, bp->flags);
+
+/*
+ * The breakpoint_check hook may use DISAS_TOO_MANY
+ * to indicate that only one more instruction is to
+ * be executed.  Otherwise it should use DISAS_NORETURN
+ * when generating an exception, but may use a
+ * DISAS_TARGET_* value for Something Else.
+ */
+if (db->is_jmp > DISAS_TOO_MANY) {
+/*
+ * The address covered by the breakpoint must be
+ * included in [tb->pc, tb->pc + tb->size) in order
+ * to for it to 

[PATCH v3 12/13] accel/tcg: Hoist tb_cflags to a local in translator_loop

[Richard Henderson]

The access internal to tb_cflags() is atomic.
Avoid re-reading it as such for the multiple uses.

Reviewed-by: Alex Bennée 
Signed-off-by: Richard Henderson 
---
 accel/tcg/translator.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 1c44d096d8..449159a27c 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -50,6 +50,7 @@ bool translator_use_goto_tb(DisasContextBase *db, 
target_ulong dest)
 void translator_loop(const TranslatorOps *ops, DisasContextBase *db,
  CPUState *cpu, TranslationBlock *tb, int max_insns)
 {
+uint32_t cflags = tb_cflags(tb);
 bool plugin_enabled;
 
 /* Initialize DisasContext */
@@ -72,8 +73,7 @@ void translator_loop(const TranslatorOps *ops, 
DisasContextBase *db,
 ops->tb_start(db, cpu);
 tcg_debug_assert(db->is_jmp == DISAS_NEXT);  /* no early exit */
 
-plugin_enabled = plugin_gen_tb_start(cpu, tb,
- tb_cflags(db->tb) & CF_MEMI_ONLY);
+plugin_enabled = plugin_gen_tb_start(cpu, tb, cflags & CF_MEMI_ONLY);
 
 while (true) {
 db->num_insns++;
@@ -125,14 +125,13 @@ void translator_loop(const TranslatorOps *ops, 
DisasContextBase *db,
update db->pc_next and db->is_jmp to indicate what should be
done next -- either exiting this loop or locate the start of
the next instruction.  */
-if (db->num_insns == db->max_insns
-&& (tb_cflags(db->tb) & CF_LAST_IO)) {
+if (db->num_insns == db->max_insns && (cflags & CF_LAST_IO)) {
 /* Accept I/O on the last instruction.  */
 gen_io_start();
 ops->translate_insn(db, cpu);
 } else {
 /* we should only see CF_MEMI_ONLY for io_recompile */
-tcg_debug_assert(!(tb_cflags(db->tb) & CF_MEMI_ONLY));
+tcg_debug_assert(!(cflags & CF_MEMI_ONLY));
 ops->translate_insn(db, cpu);
 }
 
-- 
2.25.1

[PATCH v3 01/13] accel/tcg: Reduce CF_COUNT_MASK to match TCG_MAX_INSNS

[Richard Henderson]

The space reserved for CF_COUNT_MASK was overly large.
Reduce to free up cflags bits and eliminate an extra test.

Reviewed-by: Alex Bennée 
Signed-off-by: Richard Henderson 
---
 include/exec/exec-all.h   | 4 +++-
 accel/tcg/translate-all.c | 5 ++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 754f4130c9..dfe82ed19c 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -492,7 +492,9 @@ struct TranslationBlock {
 target_ulong cs_base; /* CS base for this block */
 uint32_t flags; /* flags defining in which context the code was generated 
*/
 uint32_t cflags;/* compile flags */
-#define CF_COUNT_MASK  0x7fff
+
+/* Note that TCG_MAX_INSNS is 512; we validate this match elsewhere. */
+#define CF_COUNT_MASK  0x01ff
 #define CF_LAST_IO 0x8000 /* Last insn may be an IO access.  */
 #define CF_MEMI_ONLY   0x0001 /* Only instrument memory ops */
 #define CF_USE_ICOUNT  0x0002
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 4df26de858..5cc01d693b 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1428,11 +1428,10 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
 
 max_insns = cflags & CF_COUNT_MASK;
 if (max_insns == 0) {
-max_insns = CF_COUNT_MASK;
-}
-if (max_insns > TCG_MAX_INSNS) {
 max_insns = TCG_MAX_INSNS;
 }
+QEMU_BUILD_BUG_ON(CF_COUNT_MASK + 1 != TCG_MAX_INSNS);
+
 if (cpu->singlestep_enabled || singlestep) {
 max_insns = 1;
 }
-- 
2.25.1

[PATCH v3 04/13] accel/tcg: Drop CF_NO_GOTO_PTR from -d nochain

[Richard Henderson]

The purpose of suppressing goto_ptr from -d nochain had been
to return to the main loop so that -d cpu would be recognized.
But we now include -d cpu logging in helper_lookup_tb_ptr so
there is no need to exclude goto_ptr.

Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 accel/tcg/cpu-exec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index d3232d5764..70ea3c7d68 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -150,7 +150,7 @@ uint32_t curr_cflags(CPUState *cpu)
 uint32_t cflags = cpu->tcg_cflags;
 
 if (qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
-cflags |= CF_NO_GOTO_TB | CF_NO_GOTO_PTR;
+cflags |= CF_NO_GOTO_TB;
 }
 
 return cflags;
-- 
2.25.1

[PATCH v3 07/13] accel/tcg: Move cflags lookup into tb_find

[Richard Henderson]

We will shortly require the guest pc for computing cflags,
so move the choice just after cpu_get_tb_cpu_state.

Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 accel/tcg/cpu-exec.c | 34 +-
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 5bb099174f..4d043a11aa 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -502,15 +502,29 @@ static inline void tb_add_jump(TranslationBlock *tb, int 
n,
 
 static inline TranslationBlock *tb_find(CPUState *cpu,
 TranslationBlock *last_tb,
-int tb_exit, uint32_t cflags)
+int tb_exit)
 {
 CPUArchState *env = (CPUArchState *)cpu->env_ptr;
 TranslationBlock *tb;
 target_ulong cs_base, pc;
-uint32_t flags;
+uint32_t flags, cflags;
 
 cpu_get_tb_cpu_state(env, , _base, );
 
+/*
+ * When requested, use an exact setting for cflags for the next
+ * execution.  This is used for icount, precise smc, and stop-
+ * after-access watchpoints.  Since this request should never
+ * have CF_INVALID set, -1 is a convenient invalid value that
+ * does not require tcg headers for cpu_common_reset.
+ */
+cflags = cpu->cflags_next_tb;
+if (cflags == -1) {
+cflags = curr_cflags(cpu);
+} else {
+cpu->cflags_next_tb = -1;
+}
+
 tb = tb_lookup(cpu, pc, cs_base, flags, cflags);
 if (tb == NULL) {
 mmap_lock();
@@ -868,21 +882,7 @@ int cpu_exec(CPUState *cpu)
 int tb_exit = 0;
 
 while (!cpu_handle_interrupt(cpu, _tb)) {
-uint32_t cflags = cpu->cflags_next_tb;
-TranslationBlock *tb;
-
-/* When requested, use an exact setting for cflags for the next
-   execution.  This is used for icount, precise smc, and stop-
-   after-access watchpoints.  Since this request should never
-   have CF_INVALID set, -1 is a convenient invalid value that
-   does not require tcg headers for cpu_common_reset.  */
-if (cflags == -1) {
-cflags = curr_cflags(cpu);
-} else {
-cpu->cflags_next_tb = -1;
-}
-
-tb = tb_find(cpu, last_tb, tb_exit, cflags);
+TranslationBlock *tb = tb_find(cpu, last_tb, tb_exit);
 cpu_loop_exec_tb(cpu, tb, _tb, _exit);
 /* Try to align the host and virtual clocks
if the guest is in advance */
-- 
2.25.1

[PATCH v3 00/13] tcg: breakpoint reorg

[Richard Henderson]

This is fixing #404 ("windows xp boot takes much longer...")
and several other similar reports.

Changes for v3:
  * Map CF_COUNT_MASK == 0 -> TCG_MAX_INSNS.
  * Split out *_breakpoint_check fixes for avr, mips, riscv.

Changes for v2:
  * All prerequisites and 7 of the patches from v1 with are merged.

Patches lacking review:
  08-target-avr-Advance-pc-in-avr_tr_breakpoint_check.patch
  09-target-mips-Reduce-mips_tr_breakpoint_check-pc-ad.patch
  10-target-riscv-Reduce-riscv_tr_breakpoint_check-pc-.patch
  13-accel-tcg-Encode-breakpoint-info-into-tb-cflags.patch


r~


Richard Henderson (13):
  accel/tcg: Reduce CF_COUNT_MASK to match TCG_MAX_INSNS
  accel/tcg: Move curr_cflags into cpu-exec.c
  accel/tcg: Add CF_NO_GOTO_TB and CF_NO_GOTO_PTR
  accel/tcg: Drop CF_NO_GOTO_PTR from -d nochain
  accel/tcg: Handle -singlestep in curr_cflags
  accel/tcg: Use CF_NO_GOTO_{TB,PTR} in cpu_exec_step_atomic
  accel/tcg: Move cflags lookup into tb_find
  target/avr: Advance pc in avr_tr_breakpoint_check
  target/mips: Reduce mips_tr_breakpoint_check pc advance to 2
  target/riscv: Reduce riscv_tr_breakpoint_check pc advance to 2
  accel/tcg: Adjust interface of TranslatorOps.breakpoint_check
  accel/tcg: Hoist tb_cflags to a local in translator_loop
  accel/tcg: Encode breakpoint info into tb->cflags

 include/exec/exec-all.h   |  30 +---
 include/exec/translator.h |  17 +++--
 accel/tcg/cpu-exec.c  | 130 --
 accel/tcg/translate-all.c |   7 +-
 accel/tcg/translator.c|  79 ++---
 cpu.c |  24 ---
 target/alpha/translate.c  |  12 +---
 target/arm/translate-a64.c|  14 ++--
 target/arm/translate.c|  20 +++---
 target/avr/translate.c|   6 +-
 target/cris/translate.c   |  14 ++--
 target/hexagon/translate.c|  13 +---
 target/hppa/translate.c   |   7 +-
 target/i386/tcg/translate.c   |  15 ++--
 target/m68k/translate.c   |  14 +---
 target/microblaze/translate.c |  14 +---
 target/mips/tcg/translate.c   |  14 ++--
 target/nios2/translate.c  |  13 +---
 target/openrisc/translate.c   |  11 +--
 target/ppc/translate.c|  13 +---
 target/riscv/translate.c  |  11 +--
 target/rx/translate.c |   8 +--
 target/s390x/tcg/translate.c  |  12 ++--
 target/sh4/translate.c|  12 ++--
 target/sparc/translate.c  |   9 ++-
 target/tricore/translate.c|  13 +---
 target/xtensa/translate.c |  12 ++--
 tcg/tcg-op.c  |  28 
 28 files changed, 280 insertions(+), 292 deletions(-)

-- 
2.25.1

[PATCH v3 03/13] accel/tcg: Add CF_NO_GOTO_TB and CF_NO_GOTO_PTR

[Richard Henderson]

Move the -d nochain check to bits on tb->cflags.
These will be used for more than -d nochain shortly.

Set bits during curr_cflags, test them in translator_use_goto_tb,
assert we're not doing anything odd in tcg_gen_goto_tb.  The test
in tcg_gen_exit_tb is redundant with the assert for goto_tb_issue_mask.

Reviewed-by: Alex Bennée 
Reviewed-by: Peter Maydell 
Signed-off-by: Richard Henderson 
---
 include/exec/exec-all.h | 16 +---
 accel/tcg/cpu-exec.c|  8 +++-
 accel/tcg/translator.c  |  5 +
 tcg/tcg-op.c| 28 
 4 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index ae7603ca75..6873cce8df 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -494,13 +494,15 @@ struct TranslationBlock {
 uint32_t cflags;/* compile flags */
 
 /* Note that TCG_MAX_INSNS is 512; we validate this match elsewhere. */
-#define CF_COUNT_MASK  0x01ff
-#define CF_LAST_IO 0x8000 /* Last insn may be an IO access.  */
-#define CF_MEMI_ONLY   0x0001 /* Only instrument memory ops */
-#define CF_USE_ICOUNT  0x0002
-#define CF_INVALID 0x0004 /* TB is stale. Set with @jmp_lock held */
-#define CF_PARALLEL0x0008 /* Generate code for a parallel context */
-#define CF_CLUSTER_MASK 0xff00 /* Top 8 bits are cluster ID */
+#define CF_COUNT_MASK0x01ff
+#define CF_NO_GOTO_TB0x0200 /* Do not chain with goto_tb */
+#define CF_NO_GOTO_PTR   0x0400 /* Do not chain with goto_ptr */
+#define CF_LAST_IO   0x8000 /* Last insn may be an IO access.  */
+#define CF_MEMI_ONLY 0x0001 /* Only instrument memory ops */
+#define CF_USE_ICOUNT0x0002
+#define CF_INVALID   0x0004 /* TB is stale. Set with @jmp_lock held */
+#define CF_PARALLEL  0x0008 /* Generate code for a parallel context */
+#define CF_CLUSTER_MASK  0xff00 /* Top 8 bits are cluster ID */
 #define CF_CLUSTER_SHIFT 24
 
 /* Per-vCPU dynamic tracing state used to generate this TB */
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index ef4214d893..d3232d5764 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -147,7 +147,13 @@ static void init_delay_params(SyncClocks *sc, const 
CPUState *cpu)
 
 uint32_t curr_cflags(CPUState *cpu)
 {
-return cpu->tcg_cflags;
+uint32_t cflags = cpu->tcg_cflags;
+
+if (qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
+cflags |= CF_NO_GOTO_TB | CF_NO_GOTO_PTR;
+}
+
+return cflags;
 }
 
 /* Might cause an exception, so have a longjmp destination ready */
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 59804af37b..2ea5a74f30 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -33,6 +33,11 @@ void translator_loop_temp_check(DisasContextBase *db)
 
 bool translator_use_goto_tb(DisasContextBase *db, target_ulong dest)
 {
+/* Suppress goto_tb if requested. */
+if (tb_cflags(db->tb) & CF_NO_GOTO_TB) {
+return false;
+}
+
 /* Suppress goto_tb in the case of single-steping.  */
 if (db->singlestep_enabled || singlestep) {
 return false;
diff --git a/tcg/tcg-op.c b/tcg/tcg-op.c
index 0c561fb253..e0d54d537f 100644
--- a/tcg/tcg-op.c
+++ b/tcg/tcg-op.c
@@ -2723,10 +2723,6 @@ void tcg_gen_exit_tb(const TranslationBlock *tb, 
unsigned idx)
seen this numbered exit before, via tcg_gen_goto_tb.  */
 tcg_debug_assert(tcg_ctx->goto_tb_issue_mask & (1 << idx));
 #endif
-/* When not chaining, exit without indicating a link.  */
-if (qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
-val = 0;
-}
 } else {
 /* This is an exit via the exitreq label.  */
 tcg_debug_assert(idx == TB_EXIT_REQUESTED);
@@ -2738,6 +2734,8 @@ void tcg_gen_exit_tb(const TranslationBlock *tb, unsigned 
idx)
 
 void tcg_gen_goto_tb(unsigned idx)
 {
+/* We tested CF_NO_GOTO_TB in translator_use_goto_tb. */
+tcg_debug_assert(!(tcg_ctx->tb_cflags & CF_NO_GOTO_TB));
 /* We only support two chained exits.  */
 tcg_debug_assert(idx <= TB_EXIT_IDXMAX);
 #ifdef CONFIG_DEBUG_TCG
@@ -2746,25 +2744,23 @@ void tcg_gen_goto_tb(unsigned idx)
 tcg_ctx->goto_tb_issue_mask |= 1 << idx;
 #endif
 plugin_gen_disable_mem_helpers();
-/* When not chaining, we simply fall through to the "fallback" exit.  */
-if (!qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
-tcg_gen_op1i(INDEX_op_goto_tb, idx);
-}
+tcg_gen_op1i(INDEX_op_goto_tb, idx);
 }
 
 void tcg_gen_lookup_and_goto_ptr(void)
 {
-if (!qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
-TCGv_ptr ptr;
+TCGv_ptr ptr;
 
-plugin_gen_disable_mem_helpers();
-ptr = tcg_temp_new_ptr();
-gen_helper_lookup_tb_ptr(ptr, cpu_env);
-tcg_gen_op1i(INDEX_op_goto_ptr, tcgv_ptr_arg(ptr));
-tcg_temp_free_ptr(ptr);
-} else {
+if (tcg_ctx->tb_cflags & CF_NO_GOTO_PTR) {
 

Re: [PATCH for-6.2 21/34] target/arm: Implement MVE VABAV

[Richard Henderson]

On 7/17/21 3:13 PM, Peter Maydell wrote:

On Sat, 17 Jul 2021 at 21:50, Richard Henderson
 wrote:


On 7/13/21 6:37 AM, Peter Maydell wrote:

+#define DO_VABAV(OP, ESIZE, TYPE)   \
+uint32_t HELPER(glue(mve_, OP))(CPUARMState *env, void *vn, \
+void *vm, uint32_t ra)  \
+{   \
+uint16_t mask = mve_element_mask(env);  \
+unsigned e; \
+TYPE *m = vm, *n = vn;  \
+for (e = 0; e < 16 / ESIZE; e++, mask >>= ESIZE) {  \
+if (mask & 1) { \
+int64_t n0 = n[H##ESIZE(e)];\
+int64_t m0 = m[H##ESIZE(e)];\
+uint32_t r = n0 >= m0 ? (n0 - m0) : (m0 - n0);  \


Any particular reason you're not using TYPE for n0 and m0?


I wanted to definitely be doing the subtraction with a signed
type wider than the inputs, because that's the way the
pseudocode effectively deals with it, and I didn't feel like
thinking through whether using the smaller and possibly
unsigned types would DTRT :-)


Fair enough.  Now that I think of it, I would expect the compiler to narrow the operation 
based on the result type.  So, unconditional r-b.


r~

Re: [PATCH for-6.2 21/34] target/arm: Implement MVE VABAV

[Peter Maydell]

On Sat, 17 Jul 2021 at 21:50, Richard Henderson
 wrote:
>
> On 7/13/21 6:37 AM, Peter Maydell wrote:
> > +#define DO_VABAV(OP, ESIZE, TYPE)   \
> > +uint32_t HELPER(glue(mve_, OP))(CPUARMState *env, void *vn, \
> > +void *vm, uint32_t ra)  \
> > +{   \
> > +uint16_t mask = mve_element_mask(env);  \
> > +unsigned e; \
> > +TYPE *m = vm, *n = vn;  \
> > +for (e = 0; e < 16 / ESIZE; e++, mask >>= ESIZE) {  \
> > +if (mask & 1) { \
> > +int64_t n0 = n[H##ESIZE(e)];\
> > +int64_t m0 = m[H##ESIZE(e)];\
> > +uint32_t r = n0 >= m0 ? (n0 - m0) : (m0 - n0);  \
>
> Any particular reason you're not using TYPE for n0 and m0?

I wanted to definitely be doing the subtraction with a signed
type wider than the inputs, because that's the way the
pseudocode effectively deals with it, and I didn't feel like
thinking through whether using the smaller and possibly
unsigned types would DTRT :-)

-- PMM

Re: [PATCH for-6.2 21/34] target/arm: Implement MVE VABAV

[Richard Henderson]

On 7/13/21 6:37 AM, Peter Maydell wrote:

+#define DO_VABAV(OP, ESIZE, TYPE)   \
+uint32_t HELPER(glue(mve_, OP))(CPUARMState *env, void *vn, \
+void *vm, uint32_t ra)  \
+{   \
+uint16_t mask = mve_element_mask(env);  \
+unsigned e; \
+TYPE *m = vm, *n = vn;  \
+for (e = 0; e < 16 / ESIZE; e++, mask >>= ESIZE) {  \
+if (mask & 1) { \
+int64_t n0 = n[H##ESIZE(e)];\
+int64_t m0 = m[H##ESIZE(e)];\
+uint32_t r = n0 >= m0 ? (n0 - m0) : (m0 - n0);  \


Any particular reason you're not using TYPE for n0 and m0?

Otherwise,
Reviewed-by: Richard Henderson 


r~

Re: [PATCH for-6.2 20/34] target/arm: Implement MVE integer min/max across vector

[Richard Henderson]

On 7/13/21 6:37 AM, Peter Maydell wrote:

+/* Max and min of absolute values */
+static int64_t do_maxa(int64_t n, int64_t m)
+{
+if (n < 0) {
+n = -n;
+}
+if (m < 0) {
+m = -m;
+}
+return MAX(n, m);
+}


This doesn't look quite right.  The n operand is extracted unsigned, and only the m 
operand is subjected to ABS.



r~

Re: [PATCH for-6.2 17/34] target/arm: Implement MVE VMLAS

[Richard Henderson]

On 7/17/21 3:06 AM, Peter Maydell wrote:

On Fri, 16 Jul 2021 at 23:12, Richard Henderson
 wrote:


On 7/13/21 6:37 AM, Peter Maydell wrote:

Implement the MVE VMLAS insn, which multiplies a vector by a vector
and adds a scalar.

Signed-off-by: Peter Maydell
---
   target/arm/helper-mve.h|  8 
   target/arm/mve.decode  |  3 +++
   target/arm/mve_helper.c| 31 +++
   target/arm/translate-mve.c |  2 ++
   4 files changed, 44 insertions(+)

...


+/* Vector by vector plus scalar */
+#define DO_VMLAS(D, N, M) ((N) * (D) + (M))
+
+DO_2OP_ACC_SCALAR_S(vmlass, DO_VMLAS)
+DO_2OP_ACC_SCALAR_U(vmlasu, DO_VMLAS)


This is confusing.  The ARM says

# Operations that do not perform
# widening are always unsigned (encoded with U=1),


I have noticed that that text often appears for insns where it doesn't
really apply. I mostly ignore the text in favour of looking at
the pseudocode for working out what is supposed to be done.


Yes, but in this case there's nothing about the pseudocode that suggests that sign matters 
at all.  Neither the multiply nor the addition are widening.  So is there really a signed 
VMLAS instruction?



r~

Re: [PATCH v3 0/4] virtio: Add vhost-user based RNG

[Michael S. Tsirkin]

On Fri, Jul 09, 2021 at 06:59:25PM -0600, Mathieu Poirier wrote:
> This sets adds a vhost-user based random number generator (RNG),
> similar to what has been done for i2c and virtiofsd, with the
> implementation following the patterns already set forth in those.
> 
> Applies cleanly to git://git.qemu.org/qemu.git master(05de778b5b8a).

There were several meson-related issues related to this patchset,
so I dropped it from the pull request for now.
Please work to resolve, and re-submit, preferably after the release.



> Thanks,
> Mathieu
> 
> Mathieu Poirier (4):
>   vhost-user-rng: Add vhost-user-rng implementation
>   vhost-user-rng-pci: Add vhost-user-rng-pci implementation
>   vhost-user-rng: backend: Add RNG vhost-user daemon implementation
>   docs: Add documentation for vhost based RNG implementation
> 
> 
> New for V3:
> - Rebased to latest master branch.
> - Fixed documentation warning.
> - Updated call to vhost_dev_init() to match new signature.
> - Dropped MAINTAINERS patch since it was already applied. 
> 
>  docs/tools/index.rst |   1 +
>  docs/tools/vhost-user-rng.rst|  74 +
>  hw/virtio/Kconfig|   5 +
>  hw/virtio/meson.build|   2 +
>  hw/virtio/vhost-user-rng-pci.c   |  79 +
>  hw/virtio/vhost-user-rng.c   | 294 +
>  include/hw/virtio/vhost-user-rng.h   |  33 ++
>  tools/meson.build|   8 +
>  tools/vhost-user-rng/50-qemu-rng.json.in |   5 +
>  tools/vhost-user-rng/main.c  | 403 +++
>  tools/vhost-user-rng/meson.build |  10 +
>  11 files changed, 914 insertions(+)
>  create mode 100644 docs/tools/vhost-user-rng.rst
>  create mode 100644 hw/virtio/vhost-user-rng-pci.c
>  create mode 100644 hw/virtio/vhost-user-rng.c
>  create mode 100644 include/hw/virtio/vhost-user-rng.h
>  create mode 100644 tools/vhost-user-rng/50-qemu-rng.json.in
>  create mode 100644 tools/vhost-user-rng/main.c
>  create mode 100644 tools/vhost-user-rng/meson.build
> 
> -- 
> 2.25.1

Re: R: [PULL 0/3] ppc-for-6.1 queue 20210713

[BALATON Zoltan]

On Sat, 17 Jul 2021, luigi burdo wrote:

Hi
i small report
im try to build last relesase on a PowerMac G5 quad on debian sid PPC64 but ld 
exit with an error:


Looks like it's gcc that crashes not ld. If you're sure it's not a 
hardware problem this may be a gcc bug. Sometimes big compile jobs can 
break if a machine is not stable but if it always fails at the same place 
and does not fail when compiling something else like a Linux kernel then 
it's more likely to be an actual bug. That function has some crazy use of 
macros so maybe it's exposing some bug in gcc. You could restrict what 
targets you build with --target-list configure option of QEMU if you don't 
actually need mips (compiling will be faster if you only build the targets 
you need) or you can try with clang if available in your distro to check 
if that works better. If this can be consistently reproduced you could try 
reporting it to gcc as the error message suggests. Probably not much QEMU 
can do about it.


Regards,
BALATON Zoltan


gigi@debian:~/src/tags/ppc-for-6.1-20210713/build$ ninja
[3864/9215] Compiling C object 
libqemu-mipsel-softmmu.fa.p/target_mips_tcg_msa_helper.c.o
FAILED: libqemu-mipsel-softmmu.fa.p/target_mips_tcg_msa_helper.c.o
cc -Ilibqemu-mipsel-softmmu.fa.p -I. -I.. -Itarget/mips -I../target/mips 
-I../capstone/include/capstone -Iqapi -Itrace -Iui -Iui/shader 
-I/usr/include/pixman-1 -I/usr/include/glib-2.0 
-I/usr/lib/powerpc64-linux-gnu/glib-2.0/include -fdiagnostics-color=auto -pipe 
-Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem 
/home/gigi/src/tags/ppc-for-6.1-20210713/linux-headers -isystem linux-headers 
-iquote . -iquote /home/gigi/src/tags/ppc-for-6.1-20210713 -iquote 
/home/gigi/src/tags/ppc-for-6.1-20210713/include -iquote 
/home/gigi/src/tags/ppc-for-6.1-20210713/disas/libvixl -iquote 
/home/gigi/src/tags/ppc-for-6.1-20210713/tcg/ppc -pthread -U_FORTIFY_SOURCE 
-D_FORTIFY_SOURCE=2 -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 
-D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wundef 
-Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv 
-Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security 
-Wformat-y2k -Winit-self -Wignored-qualifiers -Wempt

y-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 
-Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -fPIE 
-isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H 
'-DCONFIG_TARGET="mipsel-softmmu-config-target.h"' 
'-DCONFIG_DEVICES="mipsel-softmmu-config-devices.h"' -MD -MQ 
libqemu-mipsel-softmmu.fa.p/target_mips_tcg_msa_helper.c.o -MF 
libqemu-mipsel-softmmu.fa.p/target_mips_tcg_msa_helper.c.o.d -o 
libqemu-mipsel-softmmu.fa.p/target_mips_tcg_msa_helper.c.o -c ../target/mips/tcg/msa_helper.c

during RTL pass: sched1
../target/mips/tcg/msa_helper.c: In function ‘helper_msa_fmin_df’:
../target/mips/tcg/msa_helper.c:7536:1: internal compiler error: Errore di 
segmentazione
7536 | }
 | ^
0x3fffa7b8e1c3 generic_start_main
../csu/libc-start.c:308
0x3fffa7b8e3d3 __libc_start_main
../sysdeps/unix/sysv/linux/powerpc/libc-start.c:98
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See  for instructions.
[3869/9215] Compiling C object 
libqemu-mipsel-softmmu.fa.p/target_mips_tcg_translate.c.o
ninja: build stopped: subcommand failed.

Ciao
Luigi

Da: Qemu-ppc  per conto di 
David Gibson 
Inviato: martedì 13 luglio 2021 04:07
A: peter.mayd...@linaro.org ; gr...@kaod.org 

Cc: qemu-...@nongnu.org ; qemu-devel@nongnu.org 
; David Gibson 
Oggetto: [PULL 0/3] ppc-for-6.1 queue 20210713

The following changes since commit 57e28d34c0cb04abf7683ac6a12c87ede447c320:

 Merge remote-tracking branch 'remotes/cohuck-gitlab/tags/s390x-20210708' into 
staging (2021-07-12 19:15:11 +0100)

are available in the Git repository at:

 https://gitlab.com/dgibson/qemu.git tags/ppc-for-6.1-20210713

for you to fetch changes up to c785a40179b10ce2d7a4afdb04f63c98d53a1756:

 mv64361: Remove extra break from a switch case (2021-07-13 10:12:17 +1000)


ppc patch queue 2021-07-13

I thought I'd sent the last PR before the 6.1 soft freeze, but
unfortunately I need one more.  This last minute one puts in a SLOF
update, along with a couple of bugfixes.


Alexey Kardashevskiy (1):
 pseries: Update SLOF firmware image

BALATON Zoltan (2):
 ppc/pegasos2: Allow setprop in VOF
 mv64361: Remove extra break from a switch case

hw/pci-host/mv64361.c |   1 -
hw/ppc/pegasos2.c |  10 ++
pc-bios/README|   2 +-
pc-bios/slof.bin  | Bin 96 -> 991744 bytes
roms/SLOF |   2 +-
5 files changed, 12 insertions(+), 3 deletions(-)

Re: [PATCH v2 05/10] accel/tcg: Handle -singlestep in curr_cflags

[Peter Maydell]

On Sat, 17 Jul 2021 at 19:43, Richard Henderson
 wrote:
>
> On 7/17/21 10:42 AM, Alex Bennée wrote:
> > Hmm we are testing a magic global here and looking at
> > cpu->singlestep_enabled lower down. We have a transient singlestep which
> > is turned on an off via cpu->singlestep_enabled and a global as a debug
> > option. Can we rationalise it further?
>
> Not sure what you're asking.
>
> cpu->singlestep_enabled raises a debug exception after one insn, whereas 
> singlestep merely
> exits the tb after one insn.

We really should rename 'singlestep' to 'one_insn_per_tb' or something,
because it's continually confusing...

-- PMM

Re: [PATCH v2 04/10] accel/tcg: Drop CF_NO_GOTO_PTR from -d nochain

[Alex Bennée]

Richard Henderson  writes:

> On 7/17/21 10:39 AM, Alex Bennée wrote:
>> Richard Henderson  writes:
>> 
>>> The purpose of suppressing goto_ptr from -d nochain had been
>>> to return to the main loop so that -d cpu would be recognized.
>> Hmm is it though? I've always treated it as ensuring we always come
>> out
>> into the main loop (which is helpful for debugging).
>
> What's helpful for debugging wrt the main loop beyond logging?

Usually if I rr a bug I reverse continue to the top of the loop for my
re-run. I guess we can put breakpoints elsewhere it's just another place
to remember.

>
>
> r~
>
>> 
>>> But we now include -d cpu logging in helper_lookup_tb_ptr so
>>> there is no need to exclude goto_ptr.
>>>
>>> Signed-off-by: Richard Henderson 
>>> ---
>>>   accel/tcg/cpu-exec.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
>>> index d3232d5764..70ea3c7d68 100644
>>> --- a/accel/tcg/cpu-exec.c
>>> +++ b/accel/tcg/cpu-exec.c
>>> @@ -150,7 +150,7 @@ uint32_t curr_cflags(CPUState *cpu)
>>>   uint32_t cflags = cpu->tcg_cflags;
>>> if (qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
>>> -cflags |= CF_NO_GOTO_TB | CF_NO_GOTO_PTR;
>>> +cflags |= CF_NO_GOTO_TB;
>>>   }
>>> return cflags;
>> 


-- 
Alex Bennée

Re: [PATCH v2 08/10] accel/tcg: Adjust interface of TranslatorOps.breakpoint_check

[Richard Henderson]

On 7/17/21 10:52 AM, Peter Maydell wrote:

On Mon, 12 Jul 2021 at 16:48, Richard Henderson
 wrote:


We don't need the whole CPUBreakpoint structure in the check,
only the flags.  Return the instruction length to consolidate
the adjustment of db->pc_next.

Signed-off-by: Richard Henderson 




diff --git a/target/avr/translate.c b/target/avr/translate.c
index 8237a03c23..73ff467926 100644
--- a/target/avr/translate.c
+++ b/target/avr/translate.c
@@ -2944,13 +2944,13 @@ static void avr_tr_insn_start(DisasContextBase *dcbase, 
CPUState *cs)
  tcg_gen_insn_start(ctx->npc);
  }

-static bool avr_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cs,
-const CPUBreakpoint *bp)
+static int avr_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cs,
+   int bp_flags)
  {
  DisasContext *ctx = container_of(dcbase, DisasContext, base);

  gen_breakpoint(ctx);
-return true;
+return 2; /* minimum instruction length */


Here we weren't advancing pc_next at all, and now we do.


diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
index 47c967acbf..c7b9d813c2 100644
--- a/target/mips/tcg/translate.c
+++ b/target/mips/tcg/translate.c
@@ -16190,22 +16190,16 @@ static void mips_tr_insn_start(DisasContextBase 
*dcbase, CPUState *cs)
 ctx->btarget);
  }

-static bool mips_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cs,
- const CPUBreakpoint *bp)
+static int mips_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cs,
+int bp_flags)
  {
  DisasContext *ctx = container_of(dcbase, DisasContext, base);

  save_cpu_state(ctx, 1);
  ctx->base.is_jmp = DISAS_NORETURN;
  gen_helper_raise_exception_debug(cpu_env);
-/*
- * The address covered by the breakpoint must be included in
- * [tb->pc, tb->pc + tb->size) in order to for it to be
- * properly cleared -- thus we increment the PC here so that
- * the logic setting tb->size below does the right thing.
- */
-ctx->base.pc_next += 4;
-return true;
+
+return 2; /* minimum instruction length */
  }


Here we were advancing by 4 and now advance by 2.


diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index deda0c8a44..8a6bc58572 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -961,20 +961,15 @@ static void riscv_tr_insn_start(DisasContextBase *dcbase, 
CPUState *cpu)
  tcg_gen_insn_start(ctx->base.pc_next);
  }

-static bool riscv_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cpu,
-  const CPUBreakpoint *bp)
+static int riscv_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cpu,
+ int bp_flags)
  {
  DisasContext *ctx = container_of(dcbase, DisasContext, base);

  tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next);
  ctx->base.is_jmp = DISAS_NORETURN;
  gen_exception_debug();
-/* The address covered by the breakpoint must be included in
-   [tb->pc, tb->pc + tb->size) in order to for it to be
-   properly cleared -- thus we increment the PC here so that
-   the logic setting tb->size below does the right thing.  */
-ctx->base.pc_next += 4;
-return true;
+return 2; /* minimum instruction length */
  }


Ditto.

If these are intentional changes (are they bugfixes?) they should be in a
separate patch.


Yes, they are bug fixes.

r~



Otherwise
Reviewed-by: Peter Maydell 

thanks
-- PMM

[PATCH v3 09/10] trace: Fold mem-internal.h into mem.h

[Richard Henderson]

Since the last thing that mem.h does is include mem-internal.h,
the symbols are not actually private.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 trace/mem-internal.h | 50 
 trace/mem.h  | 50 ++--
 plugins/core.c   |  2 +-
 3 files changed, 40 insertions(+), 62 deletions(-)
 delete mode 100644 trace/mem-internal.h

diff --git a/trace/mem-internal.h b/trace/mem-internal.h
deleted file mode 100644
index 8b72b678fa..00
--- a/trace/mem-internal.h
+++ /dev/null
@@ -1,50 +0,0 @@
-/*
- * Helper functions for guest memory tracing
- *
- * Copyright (C) 2016 Lluís Vilanova 
- *
- * This work is licensed under the terms of the GNU GPL, version 2 or later.
- * See the COPYING file in the top-level directory.
- */
-
-#ifndef TRACE__MEM_INTERNAL_H
-#define TRACE__MEM_INTERNAL_H
-
-#define TRACE_MEM_SZ_SHIFT_MASK 0xf /* size shift mask */
-#define TRACE_MEM_SE (1ULL << 4)/* sign extended (y/n) */
-#define TRACE_MEM_BE (1ULL << 5)/* big endian (y/n) */
-#define TRACE_MEM_ST (1ULL << 6)/* store (y/n) */
-#define TRACE_MEM_MMU_SHIFT 8   /* mmu idx */
-
-static inline uint16_t trace_mem_build_info(
-int size_shift, bool sign_extend, MemOp endianness,
-bool store, unsigned int mmu_idx)
-{
-uint16_t res;
-
-res = size_shift & TRACE_MEM_SZ_SHIFT_MASK;
-if (sign_extend) {
-res |= TRACE_MEM_SE;
-}
-if (endianness == MO_BE) {
-res |= TRACE_MEM_BE;
-}
-if (store) {
-res |= TRACE_MEM_ST;
-}
-#ifdef CONFIG_SOFTMMU
-res |= mmu_idx << TRACE_MEM_MMU_SHIFT;
-#endif
-return res;
-}
-
-static inline uint16_t trace_mem_get_info(MemOp op,
-  unsigned int mmu_idx,
-  bool store)
-{
-return trace_mem_build_info(op & MO_SIZE, !!(op & MO_SIGN),
-op & MO_BSWAP, store,
-mmu_idx);
-}
-
-#endif /* TRACE__MEM_INTERNAL_H */
diff --git a/trace/mem.h b/trace/mem.h
index 9644f592b4..2f27e7bdf0 100644
--- a/trace/mem.h
+++ b/trace/mem.h
@@ -12,24 +12,52 @@
 
 #include "tcg/tcg.h"
 
-
-/**
- * trace_mem_get_info:
- *
- * Return a value for the 'info' argument in guest memory access traces.
- */
-static uint16_t trace_mem_get_info(MemOp op, unsigned int mmu_idx, bool store);
+#define TRACE_MEM_SZ_SHIFT_MASK 0xf /* size shift mask */
+#define TRACE_MEM_SE (1ULL << 4)/* sign extended (y/n) */
+#define TRACE_MEM_BE (1ULL << 5)/* big endian (y/n) */
+#define TRACE_MEM_ST (1ULL << 6)/* store (y/n) */
+#define TRACE_MEM_MMU_SHIFT 8   /* mmu idx */
 
 /**
  * trace_mem_build_info:
  *
  * Return a value for the 'info' argument in guest memory access traces.
  */
-static uint16_t trace_mem_build_info(int size_shift, bool sign_extend,
- MemOp endianness, bool store,
- unsigned int mmuidx);
+static inline uint16_t trace_mem_build_info(int size_shift, bool sign_extend,
+MemOp endianness, bool store,
+unsigned int mmu_idx)
+{
+uint16_t res;
+
+res = size_shift & TRACE_MEM_SZ_SHIFT_MASK;
+if (sign_extend) {
+res |= TRACE_MEM_SE;
+}
+if (endianness == MO_BE) {
+res |= TRACE_MEM_BE;
+}
+if (store) {
+res |= TRACE_MEM_ST;
+}
+#ifdef CONFIG_SOFTMMU
+res |= mmu_idx << TRACE_MEM_MMU_SHIFT;
+#endif
+return res;
+}
 
 
-#include "trace/mem-internal.h"
+/**
+ * trace_mem_get_info:
+ *
+ * Return a value for the 'info' argument in guest memory access traces.
+ */
+static inline uint16_t trace_mem_get_info(MemOp op,
+  unsigned int mmu_idx,
+  bool store)
+{
+return trace_mem_build_info(op & MO_SIZE, !!(op & MO_SIGN),
+op & MO_BSWAP, store,
+mmu_idx);
+}
 
 #endif /* TRACE__MEM_H */
diff --git a/plugins/core.c b/plugins/core.c
index e1bcdb570d..474db287cb 100644
--- a/plugins/core.c
+++ b/plugins/core.c
@@ -27,7 +27,7 @@
 #include "exec/helper-proto.h"
 #include "tcg/tcg.h"
 #include "tcg/tcg-op.h"
-#include "trace/mem-internal.h" /* mem_info macros */
+#include "trace/mem.h" /* mem_info macros */
 #include "plugin.h"
 #include "qemu/compiler.h"
 
-- 
2.25.1

[PATCH v3 05/10] accel/tcg: Standardize atomic helpers on softmmu api

[Richard Henderson]

Reduce the amount of code duplication by always passing
the TCGMemOpIdx argument to helper_atomic_*.  This is not
currently used for user-only, but it's easy to ignore.

Signed-off-by: Richard Henderson 
---
 accel/tcg/tcg-runtime.h   | 46 ---
 accel/tcg/cputlb.c| 32 
 accel/tcg/user-exec.c | 26 -
 tcg/tcg-op.c  | 47 ---
 accel/tcg/atomic_common.c.inc | 70 +++
 5 files changed, 78 insertions(+), 143 deletions(-)

diff --git a/accel/tcg/tcg-runtime.h b/accel/tcg/tcg-runtime.h
index 91a5b7e85f..37cbd722bf 100644
--- a/accel/tcg/tcg-runtime.h
+++ b/accel/tcg/tcg-runtime.h
@@ -39,8 +39,6 @@ DEF_HELPER_FLAGS_1(exit_atomic, TCG_CALL_NO_WG, noreturn, env)
 DEF_HELPER_FLAGS_3(memset, TCG_CALL_NO_RWG, ptr, ptr, int, ptr)
 #endif /* IN_HELPER_PROTO */
 
-#ifdef CONFIG_SOFTMMU
-
 DEF_HELPER_FLAGS_5(atomic_cmpxchgb, TCG_CALL_NO_WG,
i32, env, tl, i32, i32, i32)
 DEF_HELPER_FLAGS_5(atomic_cmpxchgw_be, TCG_CALL_NO_WG,
@@ -88,50 +86,6 @@ DEF_HELPER_FLAGS_5(atomic_cmpxchgq_le, TCG_CALL_NO_WG,
TCG_CALL_NO_WG, i32, env, tl, i32, i32)
 #endif /* CONFIG_ATOMIC64 */
 
-#else
-
-DEF_HELPER_FLAGS_4(atomic_cmpxchgb, TCG_CALL_NO_WG, i32, env, tl, i32, i32)
-DEF_HELPER_FLAGS_4(atomic_cmpxchgw_be, TCG_CALL_NO_WG, i32, env, tl, i32, i32)
-DEF_HELPER_FLAGS_4(atomic_cmpxchgw_le, TCG_CALL_NO_WG, i32, env, tl, i32, i32)
-DEF_HELPER_FLAGS_4(atomic_cmpxchgl_be, TCG_CALL_NO_WG, i32, env, tl, i32, i32)
-DEF_HELPER_FLAGS_4(atomic_cmpxchgl_le, TCG_CALL_NO_WG, i32, env, tl, i32, i32)
-#ifdef CONFIG_ATOMIC64
-DEF_HELPER_FLAGS_4(atomic_cmpxchgq_be, TCG_CALL_NO_WG, i64, env, tl, i64, i64)
-DEF_HELPER_FLAGS_4(atomic_cmpxchgq_le, TCG_CALL_NO_WG, i64, env, tl, i64, i64)
-#endif
-
-#ifdef CONFIG_ATOMIC64
-#define GEN_ATOMIC_HELPERS(NAME) \
-DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), b), \
-   TCG_CALL_NO_WG, i32, env, tl, i32)\
-DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), w_le),  \
-   TCG_CALL_NO_WG, i32, env, tl, i32)\
-DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), w_be),  \
-   TCG_CALL_NO_WG, i32, env, tl, i32)\
-DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), l_le),  \
-   TCG_CALL_NO_WG, i32, env, tl, i32)\
-DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), l_be),  \
-   TCG_CALL_NO_WG, i32, env, tl, i32)\
-DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), q_le),  \
-   TCG_CALL_NO_WG, i64, env, tl, i64)\
-DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), q_be),  \
-   TCG_CALL_NO_WG, i64, env, tl, i64)
-#else
-#define GEN_ATOMIC_HELPERS(NAME) \
-DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), b), \
-   TCG_CALL_NO_WG, i32, env, tl, i32)\
-DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), w_le),  \
-   TCG_CALL_NO_WG, i32, env, tl, i32)\
-DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), w_be),  \
-   TCG_CALL_NO_WG, i32, env, tl, i32)\
-DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), l_le),  \
-   TCG_CALL_NO_WG, i32, env, tl, i32)\
-DEF_HELPER_FLAGS_3(glue(glue(atomic_, NAME), l_be),  \
-   TCG_CALL_NO_WG, i32, env, tl, i32)
-#endif /* CONFIG_ATOMIC64 */
-
-#endif /* CONFIG_SOFTMMU */
-
 GEN_ATOMIC_HELPERS(fetch_add)
 GEN_ATOMIC_HELPERS(fetch_and)
 GEN_ATOMIC_HELPERS(fetch_or)
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index 63da1cc96f..842cf4b572 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -2725,38 +2725,6 @@ void cpu_stq_le_data(CPUArchState *env, target_ulong 
ptr, uint64_t val)
 #include "atomic_template.h"
 #endif
 
-/* Second set of helpers are directly callable from TCG as helpers.  */
-
-#undef EXTRA_ARGS
-#undef ATOMIC_NAME
-#undef ATOMIC_MMU_LOOKUP_RW
-#undef ATOMIC_MMU_LOOKUP_R
-#undef ATOMIC_MMU_LOOKUP_W
-
-#define EXTRA_ARGS , TCGMemOpIdx oi
-#define ATOMIC_NAME(X) HELPER(glue(glue(atomic_ ## X, SUFFIX), END))
-#define ATOMIC_MMU_LOOKUP_RW \
-atomic_mmu_lookup(env, addr, oi, DATA_SIZE, PAGE_READ | PAGE_WRITE, 
GETPC())
-#define ATOMIC_MMU_LOOKUP_R \
-atomic_mmu_lookup(env, addr, oi, DATA_SIZE, PAGE_READ, GETPC())
-#define ATOMIC_MMU_LOOKUP_W \
-atomic_mmu_lookup(env, addr, oi, DATA_SIZE, PAGE_WRITE, GETPC())
-
-#define DATA_SIZE 1
-#include "atomic_template.h"
-
-#define DATA_SIZE 2
-#include "atomic_template.h"
-
-#define DATA_SIZE 4
-#include "atomic_template.h"
-
-#ifdef CONFIG_ATOMIC64
-#define DATA_SIZE 8
-#include "atomic_template.h"
-#endif
-#undef ATOMIC_MMU_IDX
-
 /* Code access functions.  */
 
 static uint64_t full_ldub_code(CPUArchState *env, target_ulong addr,
diff --git 

[PATCH v3 10/10] accel/tcg: Push trace info building into atomic_common.c.inc

[Richard Henderson]

Use trace_mem_get_info instead of trace_mem_build_info,
using the TCGMemOpIdx that we already have.  Do this in
the atomic_trace_*_pre function as common subroutines.

Signed-off-by: Richard Henderson 
---
 accel/tcg/atomic_template.h   | 48 +--
 accel/tcg/atomic_common.c.inc | 37 ++-
 2 files changed, 37 insertions(+), 48 deletions(-)

diff --git a/accel/tcg/atomic_template.h b/accel/tcg/atomic_template.h
index 6ee0158c5f..d89af4cc1e 100644
--- a/accel/tcg/atomic_template.h
+++ b/accel/tcg/atomic_template.h
@@ -77,10 +77,8 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, 
target_ulong addr,
 DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
  PAGE_READ | PAGE_WRITE, retaddr);
 DATA_TYPE ret;
-uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
- ATOMIC_MMU_IDX);
+uint16_t info = atomic_trace_rmw_pre(env, addr, oi);
 
-atomic_trace_rmw_pre(env, addr, info);
 #if DATA_SIZE == 16
 ret = atomic16_cmpxchg(haddr, cmpv, newv);
 #else
@@ -99,10 +97,8 @@ ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong 
addr,
 DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
  PAGE_READ, retaddr);
 DATA_TYPE val;
-uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
- ATOMIC_MMU_IDX);
+uint16_t info = atomic_trace_ld_pre(env, addr, oi);
 
-atomic_trace_ld_pre(env, addr, info);
 val = atomic16_read(haddr);
 ATOMIC_MMU_CLEANUP;
 atomic_trace_ld_post(env, addr, info);
@@ -114,10 +110,8 @@ void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, 
ABI_TYPE val,
 {
 DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
  PAGE_WRITE, retaddr);
-uint16_t info = trace_mem_build_info(SHIFT, false, 0, true,
- ATOMIC_MMU_IDX);
+uint16_t info = atomic_trace_st_pre(env, addr, oi);
 
-atomic_trace_st_pre(env, addr, info);
 atomic16_set(haddr, val);
 ATOMIC_MMU_CLEANUP;
 atomic_trace_st_post(env, addr, info);
@@ -130,10 +124,8 @@ ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong 
addr, ABI_TYPE val,
 DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
  PAGE_READ | PAGE_WRITE, retaddr);
 DATA_TYPE ret;
-uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
- ATOMIC_MMU_IDX);
+uint16_t info = atomic_trace_rmw_pre(env, addr, oi);
 
-atomic_trace_rmw_pre(env, addr, info);
 ret = qatomic_xchg__nocheck(haddr, val);
 ATOMIC_MMU_CLEANUP;
 atomic_trace_rmw_post(env, addr, info);
@@ -147,9 +139,7 @@ ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong 
addr,   \
 DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,  \
  PAGE_READ | PAGE_WRITE, retaddr); \
 DATA_TYPE ret;  \
-uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,\
- ATOMIC_MMU_IDX);   \
-atomic_trace_rmw_pre(env, addr, info);  \
+uint16_t info = atomic_trace_rmw_pre(env, addr, oi);\
 ret = qatomic_##X(haddr, val);  \
 ATOMIC_MMU_CLEANUP; \
 atomic_trace_rmw_post(env, addr, info); \
@@ -182,9 +172,7 @@ ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong 
addr,   \
 XDATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE, \
   PAGE_READ | PAGE_WRITE, retaddr); \
 XDATA_TYPE cmp, old, new, val = xval;   \
-uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,\
- ATOMIC_MMU_IDX);   \
-atomic_trace_rmw_pre(env, addr, info);  \
+uint16_t info = atomic_trace_rmw_pre(env, addr, oi);\
 smp_mb();   \
 cmp = qatomic_read__nocheck(haddr); \
 do {\
@@ -228,10 +216,8 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, 
target_ulong addr,
 DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
  PAGE_READ | PAGE_WRITE, retaddr);
 DATA_TYPE ret;
-uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, false,
- ATOMIC_MMU_IDX);
+uint16_t info = atomic_trace_rmw_pre(env, addr, oi);
 
-atomic_trace_rmw_pre(env, addr, info);
 #if DATA_SIZE == 16
 ret = 

[PATCH v3 04/10] tcg: Rename helper_atomic_*_mmu and provide for user-only

[Richard Henderson]

Always provide the atomic interface using TCGMemOpIdx oi
and uintptr_t retaddr.  Rename from helper_* to cpu_* so
as to (mostly) match the exec/cpu_ldst.h functions, and
to emphasize that they are not callable from TCG directly.

Signed-off-by: Richard Henderson 
---
 include/tcg/tcg.h | 78 ---
 accel/tcg/cputlb.c|  8 ++--
 accel/tcg/user-exec.c | 59 --
 target/arm/helper-a64.c   |  8 ++--
 target/i386/tcg/mem_helper.c  | 15 +--
 target/m68k/op_helper.c   | 19 +++--
 target/ppc/mem_helper.c   | 16 +++
 target/s390x/tcg/mem_helper.c | 19 -
 8 files changed, 104 insertions(+), 118 deletions(-)

diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
index 25dd19d6e1..44ccd86f3e 100644
--- a/include/tcg/tcg.h
+++ b/include/tcg/tcg.h
@@ -1341,31 +1341,32 @@ void helper_be_stq_mmu(CPUArchState *env, target_ulong 
addr, uint64_t val,
 # define helper_ret_stl_mmu   helper_le_stl_mmu
 # define helper_ret_stq_mmu   helper_le_stq_mmu
 #endif
+#endif /* CONFIG_SOFTMMU */
 
-uint32_t helper_atomic_cmpxchgb_mmu(CPUArchState *env, target_ulong addr,
+uint32_t cpu_atomic_cmpxchgb_mmu(CPUArchState *env, target_ulong addr,
+ uint32_t cmpv, uint32_t newv,
+ TCGMemOpIdx oi, uintptr_t retaddr);
+uint32_t cpu_atomic_cmpxchgw_le_mmu(CPUArchState *env, target_ulong addr,
 uint32_t cmpv, uint32_t newv,
 TCGMemOpIdx oi, uintptr_t retaddr);
-uint32_t helper_atomic_cmpxchgw_le_mmu(CPUArchState *env, target_ulong addr,
-   uint32_t cmpv, uint32_t newv,
-   TCGMemOpIdx oi, uintptr_t retaddr);
-uint32_t helper_atomic_cmpxchgl_le_mmu(CPUArchState *env, target_ulong addr,
-   uint32_t cmpv, uint32_t newv,
-   TCGMemOpIdx oi, uintptr_t retaddr);
-uint64_t helper_atomic_cmpxchgq_le_mmu(CPUArchState *env, target_ulong addr,
-   uint64_t cmpv, uint64_t newv,
-   TCGMemOpIdx oi, uintptr_t retaddr);
-uint32_t helper_atomic_cmpxchgw_be_mmu(CPUArchState *env, target_ulong addr,
-   uint32_t cmpv, uint32_t newv,
-   TCGMemOpIdx oi, uintptr_t retaddr);
-uint32_t helper_atomic_cmpxchgl_be_mmu(CPUArchState *env, target_ulong addr,
-   uint32_t cmpv, uint32_t newv,
-   TCGMemOpIdx oi, uintptr_t retaddr);
-uint64_t helper_atomic_cmpxchgq_be_mmu(CPUArchState *env, target_ulong addr,
-   uint64_t cmpv, uint64_t newv,
-   TCGMemOpIdx oi, uintptr_t retaddr);
+uint32_t cpu_atomic_cmpxchgl_le_mmu(CPUArchState *env, target_ulong addr,
+uint32_t cmpv, uint32_t newv,
+TCGMemOpIdx oi, uintptr_t retaddr);
+uint64_t cpu_atomic_cmpxchgq_le_mmu(CPUArchState *env, target_ulong addr,
+uint64_t cmpv, uint64_t newv,
+TCGMemOpIdx oi, uintptr_t retaddr);
+uint32_t cpu_atomic_cmpxchgw_be_mmu(CPUArchState *env, target_ulong addr,
+uint32_t cmpv, uint32_t newv,
+TCGMemOpIdx oi, uintptr_t retaddr);
+uint32_t cpu_atomic_cmpxchgl_be_mmu(CPUArchState *env, target_ulong addr,
+uint32_t cmpv, uint32_t newv,
+TCGMemOpIdx oi, uintptr_t retaddr);
+uint64_t cpu_atomic_cmpxchgq_be_mmu(CPUArchState *env, target_ulong addr,
+uint64_t cmpv, uint64_t newv,
+TCGMemOpIdx oi, uintptr_t retaddr);
 
 #define GEN_ATOMIC_HELPER(NAME, TYPE, SUFFIX) \
-TYPE helper_atomic_ ## NAME ## SUFFIX ## _mmu \
+TYPE cpu_atomic_ ## NAME ## SUFFIX ## _mmu\
 (CPUArchState *env, target_ulong addr, TYPE val,  \
  TCGMemOpIdx oi, uintptr_t retaddr);
 
@@ -1411,31 +1412,22 @@ GEN_ATOMIC_HELPER_ALL(xchg)
 
 #undef GEN_ATOMIC_HELPER_ALL
 #undef GEN_ATOMIC_HELPER
-#endif /* CONFIG_SOFTMMU */
 
-/*
- * These aren't really a "proper" helpers because TCG cannot manage Int128.
- * However, use the same format as the others, for use by the backends.
- *
- * The cmpxchg functions are only defined if HAVE_CMPXCHG128;
- * the ld/st functions are only defined if HAVE_ATOMIC128,
- * as defined by .
- */
-Int128 helper_atomic_cmpxchgo_le_mmu(CPUArchState *env, target_ulong addr,
- Int128 cmpv, Int128 newv,
- TCGMemOpIdx oi, uintptr_t retaddr);
-Int128 helper_atomic_cmpxchgo_be_mmu(CPUArchState *env, 

[PATCH v3 07/10] accel/tcg: Remove ATOMIC_MMU_DECLS

[Richard Henderson]

All definitions are now empty.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 accel/tcg/atomic_template.h | 12 
 accel/tcg/cputlb.c  |  1 -
 accel/tcg/user-exec.c   |  1 -
 3 files changed, 14 deletions(-)

diff --git a/accel/tcg/atomic_template.h b/accel/tcg/atomic_template.h
index 52fb26a274..ae6b6a03be 100644
--- a/accel/tcg/atomic_template.h
+++ b/accel/tcg/atomic_template.h
@@ -74,7 +74,6 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong 
addr,
   ABI_TYPE cmpv, ABI_TYPE newv,
   TCGMemOpIdx oi, uintptr_t retaddr)
 {
-ATOMIC_MMU_DECLS;
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
 DATA_TYPE ret;
 uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
@@ -96,7 +95,6 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong 
addr,
 ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr,
  TCGMemOpIdx oi, uintptr_t retaddr)
 {
-ATOMIC_MMU_DECLS;
 DATA_TYPE val, *haddr = ATOMIC_MMU_LOOKUP_R;
 uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
  ATOMIC_MMU_IDX);
@@ -111,7 +109,6 @@ ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong 
addr,
 void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
  TCGMemOpIdx oi, uintptr_t retaddr)
 {
-ATOMIC_MMU_DECLS;
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_W;
 uint16_t info = trace_mem_build_info(SHIFT, false, 0, true,
  ATOMIC_MMU_IDX);
@@ -126,7 +123,6 @@ void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, 
ABI_TYPE val,
 ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
TCGMemOpIdx oi, uintptr_t retaddr)
 {
-ATOMIC_MMU_DECLS;
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
 DATA_TYPE ret;
 uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
@@ -143,7 +139,6 @@ ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong 
addr, ABI_TYPE val,
 ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr,   \
 ABI_TYPE val, TCGMemOpIdx oi, uintptr_t retaddr) \
 {   \
-ATOMIC_MMU_DECLS;   \
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;\
 DATA_TYPE ret;  \
 uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,\
@@ -177,7 +172,6 @@ GEN_ATOMIC_HELPER(xor_fetch)
 ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr,   \
 ABI_TYPE xval, TCGMemOpIdx oi, uintptr_t retaddr) \
 {   \
-ATOMIC_MMU_DECLS;   \
 XDATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;   \
 XDATA_TYPE cmp, old, new, val = xval;   \
 uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,\
@@ -223,7 +217,6 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, 
target_ulong addr,
   ABI_TYPE cmpv, ABI_TYPE newv,
   TCGMemOpIdx oi, uintptr_t retaddr)
 {
-ATOMIC_MMU_DECLS;
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
 DATA_TYPE ret;
 uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, false,
@@ -245,7 +238,6 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, 
target_ulong addr,
 ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr,
  TCGMemOpIdx oi, uintptr_t retaddr)
 {
-ATOMIC_MMU_DECLS;
 DATA_TYPE val, *haddr = ATOMIC_MMU_LOOKUP_R;
 uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, false,
  ATOMIC_MMU_IDX);
@@ -260,7 +252,6 @@ ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong 
addr,
 void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
  TCGMemOpIdx oi, uintptr_t retaddr)
 {
-ATOMIC_MMU_DECLS;
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_W;
 uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, true,
  ATOMIC_MMU_IDX);
@@ -277,7 +268,6 @@ void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, 
ABI_TYPE val,
 ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
TCGMemOpIdx oi, uintptr_t retaddr)
 {
-ATOMIC_MMU_DECLS;
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
 ABI_TYPE ret;
 uint16_t info = trace_mem_build_info(SHIFT, false, MO_BSWAP, false,
@@ -294,7 +284,6 @@ ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong 
addr, ABI_TYPE val,
 ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr,   \
 

[PATCH v3 08/10] accel/tcg: Expand ATOMIC_MMU_LOOKUP_*

[Richard Henderson]

Unify the parameters of atomic_mmu_lookup between cputlb.c and
user-exec.c.  Call the function directly, and remove the macros.

Signed-off-by: Richard Henderson 
---
 accel/tcg/atomic_template.h | 41 +
 accel/tcg/cputlb.c  |  7 +--
 accel/tcg/user-exec.c   | 12 ++-
 3 files changed, 36 insertions(+), 24 deletions(-)

diff --git a/accel/tcg/atomic_template.h b/accel/tcg/atomic_template.h
index ae6b6a03be..6ee0158c5f 100644
--- a/accel/tcg/atomic_template.h
+++ b/accel/tcg/atomic_template.h
@@ -74,7 +74,8 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong 
addr,
   ABI_TYPE cmpv, ABI_TYPE newv,
   TCGMemOpIdx oi, uintptr_t retaddr)
 {
-DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
+DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
+ PAGE_READ | PAGE_WRITE, retaddr);
 DATA_TYPE ret;
 uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
  ATOMIC_MMU_IDX);
@@ -95,7 +96,9 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong 
addr,
 ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr,
  TCGMemOpIdx oi, uintptr_t retaddr)
 {
-DATA_TYPE val, *haddr = ATOMIC_MMU_LOOKUP_R;
+DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
+ PAGE_READ, retaddr);
+DATA_TYPE val;
 uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
  ATOMIC_MMU_IDX);
 
@@ -109,7 +112,8 @@ ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong 
addr,
 void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
  TCGMemOpIdx oi, uintptr_t retaddr)
 {
-DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_W;
+DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
+ PAGE_WRITE, retaddr);
 uint16_t info = trace_mem_build_info(SHIFT, false, 0, true,
  ATOMIC_MMU_IDX);
 
@@ -123,7 +127,8 @@ void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, 
ABI_TYPE val,
 ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
TCGMemOpIdx oi, uintptr_t retaddr)
 {
-DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
+DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
+ PAGE_READ | PAGE_WRITE, retaddr);
 DATA_TYPE ret;
 uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,
  ATOMIC_MMU_IDX);
@@ -139,7 +144,8 @@ ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong 
addr, ABI_TYPE val,
 ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr,   \
 ABI_TYPE val, TCGMemOpIdx oi, uintptr_t retaddr) \
 {   \
-DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;\
+DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,  \
+ PAGE_READ | PAGE_WRITE, retaddr); \
 DATA_TYPE ret;  \
 uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,\
  ATOMIC_MMU_IDX);   \
@@ -161,7 +167,8 @@ GEN_ATOMIC_HELPER(xor_fetch)
 
 #undef GEN_ATOMIC_HELPER
 
-/* These helpers are, as a whole, full barriers.  Within the helper,
+/*
+ * These helpers are, as a whole, full barriers.  Within the helper,
  * the leading barrier is explicit and the trailing barrier is within
  * cmpxchg primitive.
  *
@@ -172,7 +179,8 @@ GEN_ATOMIC_HELPER(xor_fetch)
 ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr,   \
 ABI_TYPE xval, TCGMemOpIdx oi, uintptr_t retaddr) \
 {   \
-XDATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;   \
+XDATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE, \
+  PAGE_READ | PAGE_WRITE, retaddr); \
 XDATA_TYPE cmp, old, new, val = xval;   \
 uint16_t info = trace_mem_build_info(SHIFT, false, 0, false,\
  ATOMIC_MMU_IDX);   \
@@ -217,7 +225,8 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, 
target_ulong addr,
   ABI_TYPE cmpv, ABI_TYPE newv,
   TCGMemOpIdx oi, uintptr_t retaddr)
 {
-DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
+DATA_TYPE *haddr = atomic_mmu_lookup(env, addr, oi, DATA_SIZE,
+ PAGE_READ | PAGE_WRITE, retaddr);
 DATA_TYPE ret;
 uint16_t info = trace_mem_build_info(SHIFT, 

[PATCH v3 03/10] qemu/atomic: Add aligned_{int64,uint64}_t types

[Richard Henderson]

Use it to avoid some clang-12 -Watomic-alignment errors,
forcing some structures to be aligned and as a pointer when
we have ensured that the address is aligned.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 accel/tcg/atomic_template.h |  4 ++--
 include/qemu/atomic.h   | 14 +-
 include/qemu/stats64.h  |  2 +-
 softmmu/timers-state.h  |  2 +-
 linux-user/hppa/cpu_loop.c  |  2 +-
 util/qsp.c  |  4 ++--
 6 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/accel/tcg/atomic_template.h b/accel/tcg/atomic_template.h
index afa8a9daf3..d347462af5 100644
--- a/accel/tcg/atomic_template.h
+++ b/accel/tcg/atomic_template.h
@@ -28,8 +28,8 @@
 # define SHIFT  4
 #elif DATA_SIZE == 8
 # define SUFFIX q
-# define DATA_TYPE  uint64_t
-# define SDATA_TYPE int64_t
+# define DATA_TYPE  aligned_uint64_t
+# define SDATA_TYPE aligned_int64_t
 # define BSWAP  bswap64
 # define SHIFT  3
 #elif DATA_SIZE == 4
diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index bf89855209..f8f159052f 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -271,7 +271,19 @@
 _oldn;  \
 })
 
-/* Abstractions to access atomically (i.e. "once") i64/u64 variables */
+/*
+ * Abstractions to access atomically (i.e. "once") i64/u64 variables.
+ *
+ * The i386 abi is odd in that by default members are only aligned to
+ * 4 bytes, which means that 8-byte types can wind up mis-aligned.
+ * Clang will then warn about this, and emit a call into libatomic.
+ *
+ * Use of these types in structures when they will be used with atomic
+ * operations can avoid this.
+ */
+typedef int64_t aligned_int64_t __attribute__((aligned(8)));
+typedef uint64_t aligned_uint64_t __attribute__((aligned(8)));
+
 #ifdef CONFIG_ATOMIC64
 /* Use __nocheck because sizeof(void *) might be < sizeof(u64) */
 #define qatomic_read_i64  qatomic_read__nocheck
diff --git a/include/qemu/stats64.h b/include/qemu/stats64.h
index fdd3d1b8f9..802402254b 100644
--- a/include/qemu/stats64.h
+++ b/include/qemu/stats64.h
@@ -21,7 +21,7 @@
 
 typedef struct Stat64 {
 #ifdef CONFIG_ATOMIC64
-uint64_t value;
+aligned_uint64_t value;
 #else
 uint32_t low, high;
 uint32_t lock;
diff --git a/softmmu/timers-state.h b/softmmu/timers-state.h
index 8c262ce139..94bb7394c5 100644
--- a/softmmu/timers-state.h
+++ b/softmmu/timers-state.h
@@ -47,7 +47,7 @@ typedef struct TimersState {
 int64_t last_delta;
 
 /* Compensate for varying guest execution speed.  */
-int64_t qemu_icount_bias;
+aligned_int64_t qemu_icount_bias;
 
 int64_t vm_clock_warp_start;
 int64_t cpu_clock_offset;
diff --git a/linux-user/hppa/cpu_loop.c b/linux-user/hppa/cpu_loop.c
index 3aaaf3337c..82d8183821 100644
--- a/linux-user/hppa/cpu_loop.c
+++ b/linux-user/hppa/cpu_loop.c
@@ -82,7 +82,7 @@ static abi_ulong hppa_lws(CPUHPPAState *env)
 o64 = *(uint64_t *)g2h(cs, old);
 n64 = *(uint64_t *)g2h(cs, new);
 #ifdef CONFIG_ATOMIC64
-r64 = qatomic_cmpxchg__nocheck((uint64_t *)g2h(cs, addr),
+r64 = qatomic_cmpxchg__nocheck((aligned_uint64_t *)g2h(cs, 
addr),
o64, n64);
 ret = r64 != o64;
 #else
diff --git a/util/qsp.c b/util/qsp.c
index bacc5fa2f6..8562b14a87 100644
--- a/util/qsp.c
+++ b/util/qsp.c
@@ -83,8 +83,8 @@ typedef struct QSPCallSite QSPCallSite;
 struct QSPEntry {
 void *thread_ptr;
 const QSPCallSite *callsite;
-uint64_t n_acqs;
-uint64_t ns;
+aligned_uint64_t n_acqs;
+aligned_uint64_t ns;
 unsigned int n_objs; /* count of coalesced objs; only used for reporting */
 };
 typedef struct QSPEntry QSPEntry;
-- 
2.25.1

[PATCH v3 02/10] qemu/atomic: Remove pre-C11 atomic fallbacks

[Richard Henderson]

We now require c11, so the fallbacks are now dead code

Signed-off-by: Richard Henderson 
---
 configure |   7 --
 include/qemu/atomic.h | 204 +++---
 2 files changed, 10 insertions(+), 201 deletions(-)

diff --git a/configure b/configure
index 49b5481139..eb8515ccca 100755
--- a/configure
+++ b/configure
@@ -3981,18 +3981,11 @@ cat > $TMPC << EOF
 int main(void)
 {
   uint64_t x = 0, y = 0;
-#ifdef __ATOMIC_RELAXED
   y = __atomic_load_n(, __ATOMIC_RELAXED);
   __atomic_store_n(, y, __ATOMIC_RELAXED);
   __atomic_compare_exchange_n(, , x, 0, __ATOMIC_RELAXED, 
__ATOMIC_RELAXED);
   __atomic_exchange_n(, y, __ATOMIC_RELAXED);
   __atomic_fetch_add(, y, __ATOMIC_RELAXED);
-#else
-  typedef char is_host64[sizeof(void *) >= sizeof(uint64_t) ? 1 : -1];
-  __sync_lock_test_and_set(, y);
-  __sync_val_compare_and_swap(, y, 0);
-  __sync_fetch_and_add(, y);
-#endif
   return 0;
 }
 EOF
diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index 99d6030095..bf89855209 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -60,8 +60,9 @@
 (unsigned short)1, 
\
   (expr)+0))
 
-#ifdef __ATOMIC_RELAXED
-/* For C11 atomic ops */
+#ifndef __ATOMIC_RELAXED
+#error "Expecting C11 atomic ops"
+#endif
 
 /* Manual memory barriers
  *
@@ -239,193 +240,8 @@
 #define qatomic_xor(ptr, n) \
 ((void) __atomic_fetch_xor(ptr, n, __ATOMIC_SEQ_CST))
 
-#else /* __ATOMIC_RELAXED */
-
-#ifdef __alpha__
-#define smp_read_barrier_depends()   asm volatile("mb":::"memory")
-#endif
-
-#if defined(__i386__) || defined(__x86_64__) || defined(__s390x__)
-
-/*
- * Because of the strongly ordered storage model, wmb() and rmb() are nops
- * here (a compiler barrier only).  QEMU doesn't do accesses to write-combining
- * qemu memory or non-temporal load/stores from C code.
- */
-#define smp_mb_release()   barrier()
-#define smp_mb_acquire()   barrier()
-
-/*
- * __sync_lock_test_and_set() is documented to be an acquire barrier only,
- * but it is a full barrier at the hardware level.  Add a compiler barrier
- * to make it a full barrier also at the compiler level.
- */
-#define qatomic_xchg(ptr, i)(barrier(), __sync_lock_test_and_set(ptr, i))
-
-#elif defined(_ARCH_PPC)
-
-/*
- * We use an eieio() for wmb() on powerpc.  This assumes we don't
- * need to order cacheable and non-cacheable stores with respect to
- * each other.
- *
- * smp_mb has the same problem as on x86 for not-very-new GCC
- * (http://patchwork.ozlabs.org/patch/126184/, Nov 2011).
- */
-#define smp_wmb()  ({ asm volatile("eieio" ::: "memory"); (void)0; })
-#if defined(__powerpc64__)
-#define smp_mb_release()   ({ asm volatile("lwsync" ::: "memory"); (void)0; })
-#define smp_mb_acquire()   ({ asm volatile("lwsync" ::: "memory"); (void)0; })
-#else
-#define smp_mb_release()   ({ asm volatile("sync" ::: "memory"); (void)0; })
-#define smp_mb_acquire()   ({ asm volatile("sync" ::: "memory"); (void)0; })
-#endif
-#define smp_mb()   ({ asm volatile("sync" ::: "memory"); (void)0; })
-
-#endif /* _ARCH_PPC */
-
-/*
- * For (host) platforms we don't have explicit barrier definitions
- * for, we use the gcc __sync_synchronize() primitive to generate a
- * full barrier.  This should be safe on all platforms, though it may
- * be overkill for smp_mb_acquire() and smp_mb_release().
- */
-#ifndef smp_mb
-#define smp_mb()   __sync_synchronize()
-#endif
-
-#ifndef smp_mb_acquire
-#define smp_mb_acquire()   __sync_synchronize()
-#endif
-
-#ifndef smp_mb_release
-#define smp_mb_release()   __sync_synchronize()
-#endif
-
-#ifndef smp_read_barrier_depends
-#define smp_read_barrier_depends()   barrier()
-#endif
-
-#ifndef signal_barrier
-#define signal_barrier()barrier()
-#endif
-
-/* These will only be atomic if the processor does the fetch or store
- * in a single issue memory operation
- */
-#define qatomic_read__nocheck(p)   (*(__typeof__(*(p)) volatile*) (p))
-#define qatomic_set__nocheck(p, i) ((*(__typeof__(*(p)) volatile*) (p)) = (i))
-
-#define qatomic_read(ptr)   qatomic_read__nocheck(ptr)
-#define qatomic_set(ptr, i) qatomic_set__nocheck(ptr,i)
-
-/**
- * qatomic_rcu_read - reads a RCU-protected pointer to a local variable
- * into a RCU read-side critical section. The pointer can later be safely
- * dereferenced within the critical section.
- *
- * This ensures that the pointer copy is invariant thorough the whole critical
- * section.
- *
- * Inserts memory barriers on architectures that require them (currently only
- * Alpha) and documents which pointers are protected by RCU.
- *
- * qatomic_rcu_read also includes a compiler barrier to ensure that
- * value-speculative optimizations (e.g. VSS: Value Speculation
- * Scheduling) does not perform the data read before the pointer read
- * by speculating the value of the pointer.
- *
- * Should match qatomic_rcu_set(), qatomic_xchg(), qatomic_cmpxchg().
- */

[PATCH v3 06/10] accel/tcg: Fold EXTRA_ARGS into atomic_template.h

[Richard Henderson]

All instances of EXTRA_ARGS are now identical.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 accel/tcg/atomic_template.h | 36 
 accel/tcg/cputlb.c  |  1 -
 accel/tcg/user-exec.c   |  1 -
 3 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/accel/tcg/atomic_template.h b/accel/tcg/atomic_template.h
index d347462af5..52fb26a274 100644
--- a/accel/tcg/atomic_template.h
+++ b/accel/tcg/atomic_template.h
@@ -71,7 +71,8 @@
 #endif
 
 ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong addr,
-  ABI_TYPE cmpv, ABI_TYPE newv EXTRA_ARGS)
+  ABI_TYPE cmpv, ABI_TYPE newv,
+  TCGMemOpIdx oi, uintptr_t retaddr)
 {
 ATOMIC_MMU_DECLS;
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
@@ -92,7 +93,8 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong 
addr,
 
 #if DATA_SIZE >= 16
 #if HAVE_ATOMIC128
-ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr EXTRA_ARGS)
+ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr,
+ TCGMemOpIdx oi, uintptr_t retaddr)
 {
 ATOMIC_MMU_DECLS;
 DATA_TYPE val, *haddr = ATOMIC_MMU_LOOKUP_R;
@@ -106,8 +108,8 @@ ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong 
addr EXTRA_ARGS)
 return val;
 }
 
-void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr,
- ABI_TYPE val EXTRA_ARGS)
+void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
+ TCGMemOpIdx oi, uintptr_t retaddr)
 {
 ATOMIC_MMU_DECLS;
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_W;
@@ -121,8 +123,8 @@ void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr,
 }
 #endif
 #else
-ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr,
-   ABI_TYPE val EXTRA_ARGS)
+ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
+   TCGMemOpIdx oi, uintptr_t retaddr)
 {
 ATOMIC_MMU_DECLS;
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
@@ -139,7 +141,7 @@ ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong 
addr,
 
 #define GEN_ATOMIC_HELPER(X)\
 ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr,   \
-ABI_TYPE val EXTRA_ARGS)\
+ABI_TYPE val, TCGMemOpIdx oi, uintptr_t retaddr) \
 {   \
 ATOMIC_MMU_DECLS;   \
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;\
@@ -173,7 +175,7 @@ GEN_ATOMIC_HELPER(xor_fetch)
  */
 #define GEN_ATOMIC_HELPER_FN(X, FN, XDATA_TYPE, RET)\
 ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr,   \
-ABI_TYPE xval EXTRA_ARGS)   \
+ABI_TYPE xval, TCGMemOpIdx oi, uintptr_t retaddr) \
 {   \
 ATOMIC_MMU_DECLS;   \
 XDATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;   \
@@ -218,7 +220,8 @@ GEN_ATOMIC_HELPER_FN(umax_fetch, MAX,  DATA_TYPE, new)
 #endif
 
 ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, target_ulong addr,
-  ABI_TYPE cmpv, ABI_TYPE newv EXTRA_ARGS)
+  ABI_TYPE cmpv, ABI_TYPE newv,
+  TCGMemOpIdx oi, uintptr_t retaddr)
 {
 ATOMIC_MMU_DECLS;
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
@@ -239,7 +242,8 @@ ABI_TYPE ATOMIC_NAME(cmpxchg)(CPUArchState *env, 
target_ulong addr,
 
 #if DATA_SIZE >= 16
 #if HAVE_ATOMIC128
-ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr EXTRA_ARGS)
+ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong addr,
+ TCGMemOpIdx oi, uintptr_t retaddr)
 {
 ATOMIC_MMU_DECLS;
 DATA_TYPE val, *haddr = ATOMIC_MMU_LOOKUP_R;
@@ -253,8 +257,8 @@ ABI_TYPE ATOMIC_NAME(ld)(CPUArchState *env, target_ulong 
addr EXTRA_ARGS)
 return BSWAP(val);
 }
 
-void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr,
- ABI_TYPE val EXTRA_ARGS)
+void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
+ TCGMemOpIdx oi, uintptr_t retaddr)
 {
 ATOMIC_MMU_DECLS;
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_W;
@@ -270,8 +274,8 @@ void ATOMIC_NAME(st)(CPUArchState *env, target_ulong addr,
 }
 #endif
 #else
-ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr,
-   ABI_TYPE val EXTRA_ARGS)
+ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr, ABI_TYPE val,
+   TCGMemOpIdx oi, uintptr_t retaddr)
 {
 ATOMIC_MMU_DECLS;
 DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP_RW;
@@ -288,7 +292,7 @@ ABI_TYPE 

[PATCH v3 01/10] qemu/atomic: Use macros for CONFIG_ATOMIC64

[Richard Henderson]

Clang warnings about questionable atomic usage get localized
to the inline function in atomic.h.  By using a macro, we get
the full traceback to the original use that caused the warning.

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Richard Henderson 
---
 include/qemu/atomic.h | 25 +
 1 file changed, 5 insertions(+), 20 deletions(-)

diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index 3ccf84fd46..99d6030095 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -457,26 +457,11 @@
 
 /* Abstractions to access atomically (i.e. "once") i64/u64 variables */
 #ifdef CONFIG_ATOMIC64
-static inline int64_t qatomic_read_i64(const int64_t *ptr)
-{
-/* use __nocheck because sizeof(void *) might be < sizeof(u64) */
-return qatomic_read__nocheck(ptr);
-}
-
-static inline uint64_t qatomic_read_u64(const uint64_t *ptr)
-{
-return qatomic_read__nocheck(ptr);
-}
-
-static inline void qatomic_set_i64(int64_t *ptr, int64_t val)
-{
-qatomic_set__nocheck(ptr, val);
-}
-
-static inline void qatomic_set_u64(uint64_t *ptr, uint64_t val)
-{
-qatomic_set__nocheck(ptr, val);
-}
+/* Use __nocheck because sizeof(void *) might be < sizeof(u64) */
+#define qatomic_read_i64  qatomic_read__nocheck
+#define qatomic_read_u64  qatomic_read__nocheck
+#define qatomic_set_i64   qatomic_set__nocheck
+#define qatomic_set_u64   qatomic_set__nocheck
 
 static inline void qatomic64_init(void)
 {
-- 
2.25.1

[PATCH v3 00/10] Atomic cleanup + clang-12 build fix

[Richard Henderson]

This is intended to fix building with clang-12 on i386.
In the process, I found bugs wrt handling of guest memory in target/
with respect to atomics, fixed by unifying the api between softmmu
and user-only and removing some ifdefs under target/.

Unification of the api allowed some further cleanups.

I think that patches 1-5 fix all of the bugs, and that 6-10 are only
cleanup and could be left to next cycle.

Changes for v3:
  * Dropped the typeof_strip_qual patch with broader testing.
  * Squashed an include fix for trace/mem.h, with plugins enabled.
  * Applied Phil's R-b.


r~


Richard Henderson (10):
  qemu/atomic: Use macros for CONFIG_ATOMIC64
  qemu/atomic: Remove pre-C11 atomic fallbacks
  qemu/atomic: Add aligned_{int64,uint64}_t types
  tcg: Rename helper_atomic_*_mmu and provide for user-only
  accel/tcg: Standardize atomic helpers on softmmu api
  accel/tcg: Fold EXTRA_ARGS into atomic_template.h
  accel/tcg: Remove ATOMIC_MMU_DECLS
  accel/tcg: Expand ATOMIC_MMU_LOOKUP_*
  trace: Fold mem-internal.h into mem.h
  accel/tcg: Push trace info building into atomic_common.c.inc

 configure |   7 -
 accel/tcg/atomic_template.h   | 141 +---
 accel/tcg/tcg-runtime.h   |  46 ---
 include/qemu/atomic.h | 243 --
 include/qemu/stats64.h|   2 +-
 include/tcg/tcg.h |  78 +--
 softmmu/timers-state.h|   2 +-
 trace/mem-internal.h  |  50 ---
 trace/mem.h   |  50 +--
 accel/tcg/cputlb.c|  49 +--
 accel/tcg/user-exec.c |  41 +++---
 linux-user/hppa/cpu_loop.c|   2 +-
 plugins/core.c|   2 +-
 target/arm/helper-a64.c   |   8 +-
 target/i386/tcg/mem_helper.c  |  15 +--
 target/m68k/op_helper.c   |  19 +--
 target/ppc/mem_helper.c   |  16 +--
 target/s390x/tcg/mem_helper.c |  19 +--
 tcg/tcg-op.c  |  47 ++-
 util/qsp.c|   4 +-
 accel/tcg/atomic_common.c.inc | 107 +--
 21 files changed, 321 insertions(+), 627 deletions(-)
 delete mode 100644 trace/mem-internal.h

-- 
2.25.1

Re: [PATCH v2 10/10] accel/tcg: Encode breakpoint info into tb->cflags

[Richard Henderson]

On 7/17/21 10:58 AM, Peter Maydell wrote:

+static uint32_t cflags_for_breakpoints(CPUState *cpu, target_ulong pc,
+   uint32_t cflags)
+{
+uint32_t bflags = 0;
+
+if (unlikely(cpu->singlestep_enabled)) {
+bflags = CF_BP_SSTEP;
+} else {


Won't this ignore breakpoints when singlestepping ?


Single-step already has priority over other breakpoints:


 /* Pass breakpoint hits to target for further processing */
-if (!db->singlestep_enabled
-&& unlikely(!QTAILQ_EMPTY(>breakpoints))) {



r~

Re: [PATCH v2 06/10] accel/tcg: Use CF_NO_GOTO_{TB, PTR} in cpu_exec_step_atomic

[Richard Henderson]

On 7/17/21 10:43 AM, Peter Maydell wrote:

On Mon, 12 Jul 2021 at 16:46, Richard Henderson
 wrote:


Request that the one TB returns immediately, so that
we release the exclusive lock as soon as possible.

Signed-off-by: Richard Henderson 
---
  accel/tcg/cpu-exec.c | 11 ---
  1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 2206c463f5..5bb099174f 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -330,8 +330,7 @@ void cpu_exec_step_atomic(CPUState *cpu)
  CPUArchState *env = (CPUArchState *)cpu->env_ptr;
  TranslationBlock *tb;
  target_ulong cs_base, pc;
-uint32_t flags;
-uint32_t cflags = (curr_cflags(cpu) & ~CF_PARALLEL) | 1;
+uint32_t flags, cflags;
  int tb_exit;

  if (sigsetjmp(cpu->jmp_env, 0) == 0) {
@@ -341,8 +340,14 @@ void cpu_exec_step_atomic(CPUState *cpu)
  cpu->running = true;

  cpu_get_tb_cpu_state(env, , _base, );
-tb = tb_lookup(cpu, pc, cs_base, flags, cflags);

+cflags = curr_cflags(cpu);
+/* Execute in a serial context. */
+cflags &= ~CF_PARALLEL;
+/* After 1 insn, return and release the exclusive lock. */
+cflags |= CF_NO_GOTO_TB | CF_NO_GOTO_PTR | 1;
+
+tb = tb_lookup(cpu, pc, cs_base, flags, cflags);
  if (tb == NULL) {
  mmap_lock();
  tb = tb_gen_code(cpu, pc, cs_base, flags, cflags);


So previously we would have executed possibly a chain of TBs
before releasing the lock, and now we definitely execute just one?


Correct.


(I guess the execute-a-chain case is unlikely given the TB
only has one insn and we know it's an exclusive insn...)


I think it's actually likely.  While the tb would definitely end after one insn, we had 
passed nothing down that would lead to returning to the main loop.



r~



Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH v2 05/10] accel/tcg: Handle -singlestep in curr_cflags

[Richard Henderson]

On 7/17/21 10:42 AM, Alex Bennée wrote:


Richard Henderson  writes:


Exchange the test in translator_use_goto_tb for CF_NO_GOTO_TB,
and the test in tb_gen_code for setting CF_COUNT_MASK to 1.

Signed-off-by: Richard Henderson 
---
  accel/tcg/cpu-exec.c  | 8 +++-
  accel/tcg/translate-all.c | 2 +-
  accel/tcg/translator.c| 2 +-
  3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 70ea3c7d68..2206c463f5 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -149,7 +149,13 @@ uint32_t curr_cflags(CPUState *cpu)
  {
  uint32_t cflags = cpu->tcg_cflags;
  
-if (qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {

+/*
+ * For singlestep and -d nochain, suppress goto_tb so that
+ * we can log -d cpu,exec after every TB.
+ */
+if (singlestep) {


Hmm we are testing a magic global here and looking at
cpu->singlestep_enabled lower down. We have a transient singlestep which
is turned on an off via cpu->singlestep_enabled and a global as a debug
option. Can we rationalise it further?


Not sure what you're asking.

cpu->singlestep_enabled raises a debug exception after one insn, whereas singlestep merely 
exits the tb after one insn.


What is it that you want me to rationalize?


r~

Re: [PATCH v2 04/10] accel/tcg: Drop CF_NO_GOTO_PTR from -d nochain

[Richard Henderson]

On 7/17/21 10:39 AM, Alex Bennée wrote:


Richard Henderson  writes:


The purpose of suppressing goto_ptr from -d nochain had been
to return to the main loop so that -d cpu would be recognized.


Hmm is it though? I've always treated it as ensuring we always come out
into the main loop (which is helpful for debugging).


What's helpful for debugging wrt the main loop beyond logging?


r~




But we now include -d cpu logging in helper_lookup_tb_ptr so
there is no need to exclude goto_ptr.

Signed-off-by: Richard Henderson 
---
  accel/tcg/cpu-exec.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index d3232d5764..70ea3c7d68 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -150,7 +150,7 @@ uint32_t curr_cflags(CPUState *cpu)
  uint32_t cflags = cpu->tcg_cflags;
  
  if (qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {

-cflags |= CF_NO_GOTO_TB | CF_NO_GOTO_PTR;
+cflags |= CF_NO_GOTO_TB;
  }
  
  return cflags;

Re: [PATCH v2 01/10] accel/tcg: Reduce CF_COUNT_MASK to match TCG_MAX_INSNS

[Richard Henderson]

On 7/17/21 10:30 AM, Peter Maydell wrote:

On Mon, 12 Jul 2021 at 16:42, Richard Henderson
 wrote:


The space reserved for CF_COUNT_MASK was overly large.
Reduce to free up cflags bits and eliminate an extra test.

Signed-off-by: Richard Henderson 
---
  include/exec/exec-all.h   | 4 +++-
  accel/tcg/translate-all.c | 5 ++---
  2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 754f4130c9..dfe82ed19c 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -492,7 +492,9 @@ struct TranslationBlock {
  target_ulong cs_base; /* CS base for this block */
  uint32_t flags; /* flags defining in which context the code was generated 
*/
  uint32_t cflags;/* compile flags */
-#define CF_COUNT_MASK  0x7fff
+
+/* Note that TCG_MAX_INSNS is 512; we validate this match elsewhere. */
+#define CF_COUNT_MASK  0x01ff
  #define CF_LAST_IO 0x8000 /* Last insn may be an IO access.  */
  #define CF_MEMI_ONLY   0x0001 /* Only instrument memory ops */
  #define CF_USE_ICOUNT  0x0002
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 4df26de858..997e44c73b 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1430,9 +1430,8 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
  if (max_insns == 0) {
  max_insns = CF_COUNT_MASK;
  }
-if (max_insns > TCG_MAX_INSNS) {
-max_insns = TCG_MAX_INSNS;
-}
+QEMU_BUILD_BUG_ON(CF_COUNT_MASK + 1 != TCG_MAX_INSNS);
+


Previously we would allow max_insns = TCG_MAX_INSNS (512).
Now we only allow it to be 511...


Hmm.  Well, 0 is supposed to map to "max", currently written as

max_insns = cflags & CF_COUNT_MASK;
if (max_insns == 0) {
max_insns = CF_COUNT_MASK;
}

I could just as easily map 0 -> TCG_MAX_INSNS instead.


r~

R: [PULL 0/3] ppc-for-6.1 queue 20210713

[luigi burdo]

Hi
i small report
im try to build last relesase on a PowerMac G5 quad on debian sid PPC64 but ld 
exit with an error:


gigi@debian:~/src/tags/ppc-for-6.1-20210713/build$ ninja
[3864/9215] Compiling C object 
libqemu-mipsel-softmmu.fa.p/target_mips_tcg_msa_helper.c.o
FAILED: libqemu-mipsel-softmmu.fa.p/target_mips_tcg_msa_helper.c.o
cc -Ilibqemu-mipsel-softmmu.fa.p -I. -I.. -Itarget/mips -I../target/mips 
-I../capstone/include/capstone -Iqapi -Itrace -Iui -Iui/shader 
-I/usr/include/pixman-1 -I/usr/include/glib-2.0 
-I/usr/lib/powerpc64-linux-gnu/glib-2.0/include -fdiagnostics-color=auto -pipe 
-Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem 
/home/gigi/src/tags/ppc-for-6.1-20210713/linux-headers -isystem linux-headers 
-iquote . -iquote /home/gigi/src/tags/ppc-for-6.1-20210713 -iquote 
/home/gigi/src/tags/ppc-for-6.1-20210713/include -iquote 
/home/gigi/src/tags/ppc-for-6.1-20210713/disas/libvixl -iquote 
/home/gigi/src/tags/ppc-for-6.1-20210713/tcg/ppc -pthread -U_FORTIFY_SOURCE 
-D_FORTIFY_SOURCE=2 -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 
-D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wundef 
-Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv 
-Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security 
-Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs 
-Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 
-Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi 
-fstack-protector-strong -fPIE -isystem../linux-headers -isystemlinux-headers 
-DNEED_CPU_H '-DCONFIG_TARGET="mipsel-softmmu-config-target.h"' 
'-DCONFIG_DEVICES="mipsel-softmmu-config-devices.h"' -MD -MQ 
libqemu-mipsel-softmmu.fa.p/target_mips_tcg_msa_helper.c.o -MF 
libqemu-mipsel-softmmu.fa.p/target_mips_tcg_msa_helper.c.o.d -o 
libqemu-mipsel-softmmu.fa.p/target_mips_tcg_msa_helper.c.o -c 
../target/mips/tcg/msa_helper.c
during RTL pass: sched1
../target/mips/tcg/msa_helper.c: In function ‘helper_msa_fmin_df’:
../target/mips/tcg/msa_helper.c:7536:1: internal compiler error: Errore di 
segmentazione
 7536 | }
  | ^
0x3fffa7b8e1c3 generic_start_main
../csu/libc-start.c:308
0x3fffa7b8e3d3 __libc_start_main
../sysdeps/unix/sysv/linux/powerpc/libc-start.c:98
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See  for instructions.
[3869/9215] Compiling C object 
libqemu-mipsel-softmmu.fa.p/target_mips_tcg_translate.c.o
ninja: build stopped: subcommand failed.

Ciao
Luigi

Da: Qemu-ppc  per conto 
di David Gibson 
Inviato: martedì 13 luglio 2021 04:07
A: peter.mayd...@linaro.org ; gr...@kaod.org 

Cc: qemu-...@nongnu.org ; qemu-devel@nongnu.org 
; David Gibson 
Oggetto: [PULL 0/3] ppc-for-6.1 queue 20210713

The following changes since commit 57e28d34c0cb04abf7683ac6a12c87ede447c320:

  Merge remote-tracking branch 'remotes/cohuck-gitlab/tags/s390x-20210708' into 
staging (2021-07-12 19:15:11 +0100)

are available in the Git repository at:

  https://gitlab.com/dgibson/qemu.git tags/ppc-for-6.1-20210713

for you to fetch changes up to c785a40179b10ce2d7a4afdb04f63c98d53a1756:

  mv64361: Remove extra break from a switch case (2021-07-13 10:12:17 +1000)


ppc patch queue 2021-07-13

I thought I'd sent the last PR before the 6.1 soft freeze, but
unfortunately I need one more.  This last minute one puts in a SLOF
update, along with a couple of bugfixes.


Alexey Kardashevskiy (1):
  pseries: Update SLOF firmware image

BALATON Zoltan (2):
  ppc/pegasos2: Allow setprop in VOF
  mv64361: Remove extra break from a switch case

 hw/pci-host/mv64361.c |   1 -
 hw/ppc/pegasos2.c |  10 ++
 pc-bios/README|   2 +-
 pc-bios/slof.bin  | Bin 96 -> 991744 bytes
 roms/SLOF |   2 +-
 5 files changed, 12 insertions(+), 3 deletions(-)

Re: [PATCH v2 10/10] accel/tcg: Encode breakpoint info into tb->cflags

[Peter Maydell]

On Mon, 12 Jul 2021 at 16:49, Richard Henderson
 wrote:
>
> Having this data in cflags means that hashing takes care
> of selecting a TB with or without exceptions built in.
> Which means that we no longer need to flush all TBs.
>
> This does require that we single-step while we're within a page
> that contains a breakpoint, so it's not yet ideal, but should be
> an improvement over some corner-case slowdowns.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/404
> Signed-off-by: Richard Henderson 
> ---
>  include/exec/exec-all.h   |  7 
>  accel/tcg/cpu-exec.c  | 68 ++-
>  accel/tcg/translate-all.c |  4 --
>  accel/tcg/translator.c| 85 +--
>  cpu.c | 24 ---
>  5 files changed, 119 insertions(+), 69 deletions(-)
>
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 6873cce8df..7ab2578f71 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -502,9 +502,16 @@ struct TranslationBlock {
>  #define CF_USE_ICOUNT0x0002
>  #define CF_INVALID   0x0004 /* TB is stale. Set with @jmp_lock held 
> */
>  #define CF_PARALLEL  0x0008 /* Generate code for a parallel context 
> */
> +#define CF_BP_MASK   0x0030 /* See below */
> +#define CF_BP_SHIFT  20
>  #define CF_CLUSTER_MASK  0xff00 /* Top 8 bits are cluster ID */
>  #define CF_CLUSTER_SHIFT 24
>
> +#define CF_BP_NONE   (0 << CF_BP_SHIFT) /* TB does not interact with BPs 
> */
> +#define CF_BP_SSTEP  (1 << CF_BP_SHIFT) /* gdbstub single-step in effect 
> */
> +#define CF_BP_GDB(2 << CF_BP_SHIFT) /* gdbstub breakpoint at tb->pc 
> */
> +#define CF_BP_CPU(3 << CF_BP_SHIFT) /* arch breakpoint at tb->pc */
> +
>  /* Per-vCPU dynamic tracing state used to generate this TB */
>  uint32_t trace_vcpu_dstate;
>
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 4d043a11aa..179a425ece 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -222,6 +222,65 @@ static inline void log_cpu_exec(target_ulong pc, 
> CPUState *cpu,
>  }
>  }
>
> +static uint32_t cflags_for_breakpoints(CPUState *cpu, target_ulong pc,
> +   uint32_t cflags)
> +{
> +uint32_t bflags = 0;
> +
> +if (unlikely(cpu->singlestep_enabled)) {
> +bflags = CF_BP_SSTEP;
> +} else {

Won't this ignore breakpoints when singlestepping ?

-- PMM

Re: [PATCH v2 08/10] accel/tcg: Adjust interface of TranslatorOps.breakpoint_check

[Peter Maydell]

On Mon, 12 Jul 2021 at 16:48, Richard Henderson
 wrote:
>
> We don't need the whole CPUBreakpoint structure in the check,
> only the flags.  Return the instruction length to consolidate
> the adjustment of db->pc_next.
>
> Signed-off-by: Richard Henderson 


> diff --git a/target/avr/translate.c b/target/avr/translate.c
> index 8237a03c23..73ff467926 100644
> --- a/target/avr/translate.c
> +++ b/target/avr/translate.c
> @@ -2944,13 +2944,13 @@ static void avr_tr_insn_start(DisasContextBase 
> *dcbase, CPUState *cs)
>  tcg_gen_insn_start(ctx->npc);
>  }
>
> -static bool avr_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cs,
> -const CPUBreakpoint *bp)
> +static int avr_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cs,
> +   int bp_flags)
>  {
>  DisasContext *ctx = container_of(dcbase, DisasContext, base);
>
>  gen_breakpoint(ctx);
> -return true;
> +return 2; /* minimum instruction length */

Here we weren't advancing pc_next at all, and now we do.

> diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c
> index 47c967acbf..c7b9d813c2 100644
> --- a/target/mips/tcg/translate.c
> +++ b/target/mips/tcg/translate.c
> @@ -16190,22 +16190,16 @@ static void mips_tr_insn_start(DisasContextBase 
> *dcbase, CPUState *cs)
> ctx->btarget);
>  }
>
> -static bool mips_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cs,
> - const CPUBreakpoint *bp)
> +static int mips_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cs,
> +int bp_flags)
>  {
>  DisasContext *ctx = container_of(dcbase, DisasContext, base);
>
>  save_cpu_state(ctx, 1);
>  ctx->base.is_jmp = DISAS_NORETURN;
>  gen_helper_raise_exception_debug(cpu_env);
> -/*
> - * The address covered by the breakpoint must be included in
> - * [tb->pc, tb->pc + tb->size) in order to for it to be
> - * properly cleared -- thus we increment the PC here so that
> - * the logic setting tb->size below does the right thing.
> - */
> -ctx->base.pc_next += 4;
> -return true;
> +
> +return 2; /* minimum instruction length */
>  }

Here we were advancing by 4 and now advance by 2.

> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index deda0c8a44..8a6bc58572 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -961,20 +961,15 @@ static void riscv_tr_insn_start(DisasContextBase 
> *dcbase, CPUState *cpu)
>  tcg_gen_insn_start(ctx->base.pc_next);
>  }
>
> -static bool riscv_tr_breakpoint_check(DisasContextBase *dcbase, CPUState 
> *cpu,
> -  const CPUBreakpoint *bp)
> +static int riscv_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cpu,
> + int bp_flags)
>  {
>  DisasContext *ctx = container_of(dcbase, DisasContext, base);
>
>  tcg_gen_movi_tl(cpu_pc, ctx->base.pc_next);
>  ctx->base.is_jmp = DISAS_NORETURN;
>  gen_exception_debug();
> -/* The address covered by the breakpoint must be included in
> -   [tb->pc, tb->pc + tb->size) in order to for it to be
> -   properly cleared -- thus we increment the PC here so that
> -   the logic setting tb->size below does the right thing.  */
> -ctx->base.pc_next += 4;
> -return true;
> +return 2; /* minimum instruction length */
>  }

Ditto.

If these are intentional changes (are they bugfixes?) they should be in a
separate patch.

Otherwise
Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH v2 09/10] accel/tcg: Hoist tb_cflags to a local in translator_loop

[Alex Bennée]

Richard Henderson  writes:

> The access internal to tb_cflags() is atomic.
> Avoid re-reading it as such for the multiple uses.
>
> Signed-off-by: Richard Henderson 

Reviewed-by: Alex Bennée 

-- 
Alex Bennée

Re: [PATCH v2 06/10] accel/tcg: Use CF_NO_GOTO_{TB,PTR} in cpu_exec_step_atomic

[Alex Bennée]

Richard Henderson  writes:

> Request that the one TB returns immediately, so that
> we release the exclusive lock as soon as possible.
>
> Signed-off-by: Richard Henderson 

Reviewed-by: Alex Bennée 

-- 
Alex Bennée

Re: [PATCH v2 05/10] accel/tcg: Handle -singlestep in curr_cflags

[Alex Bennée]

Richard Henderson  writes:

> Exchange the test in translator_use_goto_tb for CF_NO_GOTO_TB,
> and the test in tb_gen_code for setting CF_COUNT_MASK to 1.
>
> Signed-off-by: Richard Henderson 
> ---
>  accel/tcg/cpu-exec.c  | 8 +++-
>  accel/tcg/translate-all.c | 2 +-
>  accel/tcg/translator.c| 2 +-
>  3 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 70ea3c7d68..2206c463f5 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -149,7 +149,13 @@ uint32_t curr_cflags(CPUState *cpu)
>  {
>  uint32_t cflags = cpu->tcg_cflags;
>  
> -if (qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
> +/*
> + * For singlestep and -d nochain, suppress goto_tb so that
> + * we can log -d cpu,exec after every TB.
> + */
> +if (singlestep) {

Hmm we are testing a magic global here and looking at
cpu->singlestep_enabled lower down. We have a transient singlestep which
is turned on an off via cpu->singlestep_enabled and a global as a debug
option. Can we rationalise it further?

> +cflags |= CF_NO_GOTO_TB | 1;
> +} else if (qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
>  cflags |= CF_NO_GOTO_TB;
>  }
>  
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index 997e44c73b..491c1a56b2 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -1432,7 +1432,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
>  }
>  QEMU_BUILD_BUG_ON(CF_COUNT_MASK + 1 != TCG_MAX_INSNS);
>  
> -if (cpu->singlestep_enabled || singlestep) {
> +if (cpu->singlestep_enabled) {
>  max_insns = 1;
>  }
>  
> diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
> index 2ea5a74f30..a59eb7c11b 100644
> --- a/accel/tcg/translator.c
> +++ b/accel/tcg/translator.c
> @@ -39,7 +39,7 @@ bool translator_use_goto_tb(DisasContextBase *db, 
> target_ulong dest)
>  }
>  
>  /* Suppress goto_tb in the case of single-steping.  */
> -if (db->singlestep_enabled || singlestep) {
> +if (db->singlestep_enabled) {
>  return false;
>  }


-- 
Alex Bennée

Re: [PATCH v2 07/10] accel/tcg: Move cflags lookup into tb_find

[Peter Maydell]

On Mon, 12 Jul 2021 at 16:42, Richard Henderson
 wrote:
>
> We will shortly require the guest pc for computing cflags,
> so move the choice just after cpu_get_tb_cpu_state.
>
> Signed-off-by: Richard Henderson 
> ---
>  accel/tcg/cpu-exec.c | 34 +-
>  1 file changed, 17 insertions(+), 17 deletions(-)

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH v2 06/10] accel/tcg: Use CF_NO_GOTO_{TB, PTR} in cpu_exec_step_atomic

[Peter Maydell]

On Mon, 12 Jul 2021 at 16:46, Richard Henderson
 wrote:
>
> Request that the one TB returns immediately, so that
> we release the exclusive lock as soon as possible.
>
> Signed-off-by: Richard Henderson 
> ---
>  accel/tcg/cpu-exec.c | 11 ---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 2206c463f5..5bb099174f 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -330,8 +330,7 @@ void cpu_exec_step_atomic(CPUState *cpu)
>  CPUArchState *env = (CPUArchState *)cpu->env_ptr;
>  TranslationBlock *tb;
>  target_ulong cs_base, pc;
> -uint32_t flags;
> -uint32_t cflags = (curr_cflags(cpu) & ~CF_PARALLEL) | 1;
> +uint32_t flags, cflags;
>  int tb_exit;
>
>  if (sigsetjmp(cpu->jmp_env, 0) == 0) {
> @@ -341,8 +340,14 @@ void cpu_exec_step_atomic(CPUState *cpu)
>  cpu->running = true;
>
>  cpu_get_tb_cpu_state(env, , _base, );
> -tb = tb_lookup(cpu, pc, cs_base, flags, cflags);
>
> +cflags = curr_cflags(cpu);
> +/* Execute in a serial context. */
> +cflags &= ~CF_PARALLEL;
> +/* After 1 insn, return and release the exclusive lock. */
> +cflags |= CF_NO_GOTO_TB | CF_NO_GOTO_PTR | 1;
> +
> +tb = tb_lookup(cpu, pc, cs_base, flags, cflags);
>  if (tb == NULL) {
>  mmap_lock();
>  tb = tb_gen_code(cpu, pc, cs_base, flags, cflags);

So previously we would have executed possibly a chain of TBs
before releasing the lock, and now we definitely execute just one?
(I guess the execute-a-chain case is unlikely given the TB
only has one insn and we know it's an exclusive insn...)

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH v2 05/10] accel/tcg: Handle -singlestep in curr_cflags

[Peter Maydell]

On Mon, 12 Jul 2021 at 16:42, Richard Henderson
 wrote:
>
> Exchange the test in translator_use_goto_tb for CF_NO_GOTO_TB,
> and the test in tb_gen_code for setting CF_COUNT_MASK to 1.
>
> Signed-off-by: Richard Henderson 
> ---
>  accel/tcg/cpu-exec.c  | 8 +++-
>  accel/tcg/translate-all.c | 2 +-
>  accel/tcg/translator.c| 2 +-
>  3 files changed, 9 insertions(+), 3 deletions(-)

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH v2 04/10] accel/tcg: Drop CF_NO_GOTO_PTR from -d nochain

[Alex Bennée]

Richard Henderson  writes:

> The purpose of suppressing goto_ptr from -d nochain had been
> to return to the main loop so that -d cpu would be recognized.

Hmm is it though? I've always treated it as ensuring we always come out
into the main loop (which is helpful for debugging).

> But we now include -d cpu logging in helper_lookup_tb_ptr so
> there is no need to exclude goto_ptr.
>
> Signed-off-by: Richard Henderson 
> ---
>  accel/tcg/cpu-exec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index d3232d5764..70ea3c7d68 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -150,7 +150,7 @@ uint32_t curr_cflags(CPUState *cpu)
>  uint32_t cflags = cpu->tcg_cflags;
>  
>  if (qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
> -cflags |= CF_NO_GOTO_TB | CF_NO_GOTO_PTR;
> +cflags |= CF_NO_GOTO_TB;
>  }
>  
>  return cflags;


-- 
Alex Bennée

Re: [PATCH v2 04/10] accel/tcg: Drop CF_NO_GOTO_PTR from -d nochain

[Peter Maydell]

On Mon, 12 Jul 2021 at 16:44, Richard Henderson
 wrote:
>
> The purpose of suppressing goto_ptr from -d nochain had been
> to return to the main loop so that -d cpu would be recognized.
> But we now include -d cpu logging in helper_lookup_tb_ptr so
> there is no need to exclude goto_ptr.
>
> Signed-off-by: Richard Henderson 
> ---
>  accel/tcg/cpu-exec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index d3232d5764..70ea3c7d68 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -150,7 +150,7 @@ uint32_t curr_cflags(CPUState *cpu)
>  uint32_t cflags = cpu->tcg_cflags;
>
>  if (qemu_loglevel_mask(CPU_LOG_TB_NOCHAIN)) {
> -cflags |= CF_NO_GOTO_TB | CF_NO_GOTO_PTR;
> +cflags |= CF_NO_GOTO_TB;
>  }

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH v2 03/10] accel/tcg: Add CF_NO_GOTO_TB and CF_NO_GOTO_PTR

[Peter Maydell]

On Mon, 12 Jul 2021 at 16:44, Richard Henderson
 wrote:
>
> Move the -d nochain check to bits on tb->cflags.
> These will be used for more than -d nochain shortly.
>
> Set bits during curr_cflags, test them in translator_use_goto_tb,
> assert we're not doing anything odd in tcg_gen_goto_tb.  The test
> in tcg_gen_exit_tb is redundant with the assert for goto_tb_issue_mask.
>
> Signed-off-by: Richard Henderson 
> ---
Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH v2 03/10] accel/tcg: Add CF_NO_GOTO_TB and CF_NO_GOTO_PTR

[Alex Bennée]

Richard Henderson  writes:

> Move the -d nochain check to bits on tb->cflags.
> These will be used for more than -d nochain shortly.
>
> Set bits during curr_cflags, test them in translator_use_goto_tb,
> assert we're not doing anything odd in tcg_gen_goto_tb.  The test
> in tcg_gen_exit_tb is redundant with the assert for goto_tb_issue_mask.
>
> Signed-off-by: Richard Henderson 

Reviewed-by: Alex Bennée 

-- 
Alex Bennée

Re: [PATCH v2 02/10] accel/tcg: Move curr_cflags into cpu-exec.c

[Alex Bennée]

Richard Henderson  writes:

> We will shortly have more than a simple member read here,
> with stuff not necessarily exposed to exec/exec-all.h.
>
> Signed-off-by: Richard Henderson 

Reviewed-by: Alex Bennée 

-- 
Alex Bennée

Re: [PATCH v2 01/10] accel/tcg: Reduce CF_COUNT_MASK to match TCG_MAX_INSNS

[Alex Bennée]

Richard Henderson  writes:

> The space reserved for CF_COUNT_MASK was overly large.
> Reduce to free up cflags bits and eliminate an extra test.
>
> Signed-off-by: Richard Henderson 

Reviewed-by: Alex Bennée 

-- 
Alex Bennée

Re: [PATCH v2 02/10] accel/tcg: Move curr_cflags into cpu-exec.c

[Peter Maydell]

On Mon, 12 Jul 2021 at 16:44, Richard Henderson
 wrote:
>
> We will shortly have more than a simple member read here,
> with stuff not necessarily exposed to exec/exec-all.h.
>
> Signed-off-by: Richard Henderson 
> ---

Reviewed-by: Peter Maydell 

(apologies if you saw a no-content email: gmail burp)

thanks
-- PMM

Re: [PATCH v2 01/10] accel/tcg: Reduce CF_COUNT_MASK to match TCG_MAX_INSNS

[Peter Maydell]

On Mon, 12 Jul 2021 at 16:42, Richard Henderson
 wrote:
>
> The space reserved for CF_COUNT_MASK was overly large.
> Reduce to free up cflags bits and eliminate an extra test.
>
> Signed-off-by: Richard Henderson 
> ---
>  include/exec/exec-all.h   | 4 +++-
>  accel/tcg/translate-all.c | 5 ++---
>  2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index 754f4130c9..dfe82ed19c 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -492,7 +492,9 @@ struct TranslationBlock {
>  target_ulong cs_base; /* CS base for this block */
>  uint32_t flags; /* flags defining in which context the code was 
> generated */
>  uint32_t cflags;/* compile flags */
> -#define CF_COUNT_MASK  0x7fff
> +
> +/* Note that TCG_MAX_INSNS is 512; we validate this match elsewhere. */
> +#define CF_COUNT_MASK  0x01ff
>  #define CF_LAST_IO 0x8000 /* Last insn may be an IO access.  */
>  #define CF_MEMI_ONLY   0x0001 /* Only instrument memory ops */
>  #define CF_USE_ICOUNT  0x0002
> diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
> index 4df26de858..997e44c73b 100644
> --- a/accel/tcg/translate-all.c
> +++ b/accel/tcg/translate-all.c
> @@ -1430,9 +1430,8 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
>  if (max_insns == 0) {
>  max_insns = CF_COUNT_MASK;
>  }
> -if (max_insns > TCG_MAX_INSNS) {
> -max_insns = TCG_MAX_INSNS;
> -}
> +QEMU_BUILD_BUG_ON(CF_COUNT_MASK + 1 != TCG_MAX_INSNS);
> +

Previously we would allow max_insns = TCG_MAX_INSNS (512).
Now we only allow it to be 511...

-- PMM

Re: [PATCH v2 00/10] tcg: breakpoint reorg

[Richard Henderson]

Ping.  A Tested-by is nice, but Reviewed-by is better,
and time is running out, even for bug fixes.

r~

On 7/12/21 8:39 AM, Richard Henderson wrote:

This is fixing #404 ("windows xp boot takes much longer...")
and several other similar reports.

For v2, all prerequisites and 7 of the patches from v1 with
reviews are now upstream.

Mark Cave-Ayland reported success with WinXP with v1, with
this patch set being even faster than b55f54bc~1.  Which was
a bit of a surprise, but I'll take it.  It means that it's
probably not worth making the breakpoint detection scheme
any more complicated.

I'd still like some more feedback.  Given this is fixing a
regression from qemu 5.2 I feel comfortable delaying this
past soft freeze, but not past hard freeze on the 20th.


r~


Richard Henderson (10):
   accel/tcg: Reduce CF_COUNT_MASK to match TCG_MAX_INSNS
   accel/tcg: Move curr_cflags into cpu-exec.c
   accel/tcg: Add CF_NO_GOTO_TB and CF_NO_GOTO_PTR
   accel/tcg: Drop CF_NO_GOTO_PTR from -d nochain
   accel/tcg: Handle -singlestep in curr_cflags
   accel/tcg: Use CF_NO_GOTO_{TB,PTR} in cpu_exec_step_atomic
   accel/tcg: Move cflags lookup into tb_find
   accel/tcg: Adjust interface of TranslatorOps.breakpoint_check
   accel/tcg: Hoist tb_cflags to a local in translator_loop
   accel/tcg: Encode breakpoint info into tb->cflags

  include/exec/exec-all.h   |  30 +---
  include/exec/translator.h |  17 +++--
  accel/tcg/cpu-exec.c  | 130 --
  accel/tcg/translate-all.c |   7 +-
  accel/tcg/translator.c|  79 ++---
  cpu.c |  24 ---
  target/alpha/translate.c  |  12 +---
  target/arm/translate-a64.c|  14 ++--
  target/arm/translate.c|  20 +++---
  target/avr/translate.c|   6 +-
  target/cris/translate.c   |  14 ++--
  target/hexagon/translate.c|  13 +---
  target/hppa/translate.c   |   7 +-
  target/i386/tcg/translate.c   |  15 ++--
  target/m68k/translate.c   |  14 +---
  target/microblaze/translate.c |  14 +---
  target/mips/tcg/translate.c   |  14 ++--
  target/nios2/translate.c  |  13 +---
  target/openrisc/translate.c   |  11 +--
  target/ppc/translate.c|  13 +---
  target/riscv/translate.c  |  11 +--
  target/rx/translate.c |   8 +--
  target/s390x/translate.c  |  12 ++--
  target/sh4/translate.c|  12 ++--
  target/sparc/translate.c  |   9 ++-
  target/tricore/translate.c|  13 +---
  target/xtensa/translate.c |  12 ++--
  tcg/tcg-op.c  |  28 
  28 files changed, 280 insertions(+), 292 deletions(-)

Re: [PATCH] target/hexagon: Drop include of qemu.h

[Richard Henderson]

On 7/17/21 3:30 AM, Peter Maydell wrote:

The qemu.h file is a CONFIG_USER_ONLY header; it doesn't appear on
the include path for softmmu builds.  Currently we include it
unconditionally in target/hexagon/op_helper.c.  We used to need it
for the put_user_*() and get_user_*() functions, but now that we have
removed the uses of those from op_helper.c, the only reason it's
still there is that we're implicitly relying on it pulling in some
other headers.

Explicitly include the headers we need for other functions, and drop
the include of qemu.h.

Signed-off-by: Peter Maydell
---
Based-on:1626384156-6248-1-git-send-email-tsimp...@quicinc.com
("[PATCH v3 0/2] SIGSEGV fixes")


Reviewed-by: Richard Henderson 

r~

Re: [PATCH 00/17] target/riscv: Use tcg_constant_*

[Richard Henderson]

On 7/16/21 8:59 PM, LIU Zhiwei wrote:

If we want to strictly obey the spec, we should
1) Ignore MSB 32bits for source register, and sign-extend the destination 
register.
2) Always use 32bit operation(TCG 32bit OP).

I want to still use TCG 64bit OP and just extend the source to 64bit by ext32s 
or ext32u.

Is is OK?


Yes, that sounds right.


r~

[Bug 1924738] Re: Failed to restore domain - error load load virtio-balloon:virtio

[Thomas Huth]

Ticket has been re-opened here :
https://gitlab.com/qemu-project/qemu/-/issues/485

** Bug watch added: gitlab.com/qemu-project/qemu/-/issues #485
   https://gitlab.com/qemu-project/qemu/-/issues/485

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1924738

Title:
  Failed to restore domain - error load load virtio-balloon:virtio

Status in QEMU:
  Expired

Bug description:
  I noticed a domain restore error on my virtual machines.
  I can't reproduce the error on a test virtual machine.

  sudo virsh save linux2020 /var/lib/libvirt/qemu/save/linux2020.save
  Domain 'linux2020' saved to /var/lib/libvirt/qemu/save/linux2020.save

  sudo virsh restore /var/lib/libvirt/qemu/save/linux2020.save
  error: Failed to restore domain from /var/lib/libvirt/qemu/save/linux2020.save
  error: внутренняя ошибка: QEMU неожиданно завершил работу монитора: 
qemu-system-x86_64: -chardev socket,id=charchannel0,fd=52,server,nowait: 
warning: short-form boolean option 'server' deprecated
  Please use server=on instead
  qemu-system-x86_64: -chardev socket,id=charchannel0,fd=52,server,nowait: 
warning: short-form boolean option 'nowait' deprecated
  Please use wait=off instead
  qemu-system-x86_64: -spice 
port=5900,addr=0.0.0.0,disable-ticketing,image-compression=off,seamless-migration=on:
 warning: short-form boolean option 'disable-ticketing' deprecated
  Please use disable-ticketing=on instead
  2021-04-16T09:47:15.037700Z qemu-system-x86_64: VQ 0 size 0x80 < 
last_avail_idx 0x0 - used_idx 0x
  2021-04-16T09:47:15.037737Z qemu-system-x86_64: Failed to load 
virtio-balloon:virtio
  2021-04-16T09:47:15.037744Z qemu-system-x86_64: error while loading state for 
instance 0x0 of device ':00:02.0/virtio-balloon'
  2021-04-16T09:47:15.037849Z qemu-system-x86_64: load of migration failed: 
Operation not permitted

  If in the machine configuration replace
  hvm
  to
  hvm
  the virtual machine is recovering normally

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1924738/+subscriptions

Re: [PATCH 00/13] new plugin argument passing scheme

[Mahmoud Mandour]

On Sat, Jul 17, 2021, 15:31 Alex Bennée  wrote:

>
> Mahmoud Mandour  writes:
>
> > Hello,
> >
> > This series removes passing arguments to plugins through "arg=" since
> > it's redundant and reduces readability especially when the argument
> > itself is composed of a name and a value.
>
> When you re-roll a series it's useful to add a version tag. You can use
> --subject-prefix in your git format-patch command to do this.
>
> I'll have a look at this on Monday.
>

Thank you so much for the notice. I usually do it but I missed it this
time, hopefully won't happen again.


> >
> > Also, passing arguments through "arg=" still works but is marked as
> > deprecated and will produce a deprecation warning.
> >
> > Right now, the code for parsing the argument before passing it to the
> > plugin is unfortunately not so clean but that's mainly because "arg=" is
> > still supported.
> >
> > At first, considering boolean parameters, those were not special to
> > plugins and QEMU did not complain about passing them in the form
> > "arg=bool_arg" even though that's considered a short-form boolean, which
> > is deprecated. As "arg" is removed, a deprecation warning is issued.
> >
> > This is mitigated by making plugins aware of boolean arguments and
> > parses them through a newly exposed API, namely the `qapi_bool_parse`
> > function through a plugin API function. Now plugins expect boolean
> > parameters to be passed in the form that other parts of QEMU expect,
> > i.e. "bool_arg=[on|true|yes|off|false|no]".
> >
> > Since we're still supporting "arg=arg_name", there are some assumptions
> > that I made that I think are suitable:
> >
> > 1. "arg=arg_name" will be passed to the plugin as "arg_name=on".
> > 2. "arg=on" and "arg" will not be assumed to be the old way of
> > passing args. Instead, it will assume that the argument name is
> > "arg" and it's a boolean parameter. (will be passed to plugin
> > as "arg=on")
> >
> > The docs are updated accordingly and a deprecation notice is put in the
> > deprecated.rst file.
> >
> > v1 -> v2:
> > 1. Added patches that handle test plugins as well
> > 2. Handled unsupported arguements in howvec
> >
> > Based-on: <20210714172151.8494-1-ma.mando...@gmail.com>
> >
> > However, the dependency is so light and it should only be in the patch
> >
> > docs/tcg-plugins: new passing parameters scheme for cache docs
> >
> > where it depends on
> >
> > docs/devel/tcg-plugins: added cores arg to cache plugin
> >
> > in the aforementioned series (conflict lies in the argument "cores=N"
> only.)
> >
> > Mahmoud Mandour (13):
> >   plugins: allow plugin arguments to be passed directly
> >   plugins/api: added a boolean parsing plugin api
> >   plugins/hotpages: introduce sortby arg and parsed bool args correctly
> >   plugins/hotblocks: Added correct boolean argument parsing
> >   plugins/lockstep: make socket path not positional & parse bool arg
> >   plugins/hwprofile: adapt to the new plugin arguments scheme
> >   plugins/howvec: Adapting to the new argument passing scheme.
> >   docs/tcg-plugins: new passing parameters scheme for cache docs
> >   tests/plugins/bb: adapt to the new arg passing scheme
> >   tests/plugins/insn: made arg inline not positional and parse it as
> > bool
> >   tests/plugins/mem: introduce "track" arg and make args not positional
> >   tests/plugins/syscalls: adhere to new arg-passing scheme
> >   docs/deprecated: deprecate passing plugin args through `arg=`
> >
> >  contrib/plugins/hotblocks.c | 14 +--
> >  contrib/plugins/hotpages.c  | 30 +++
> >  contrib/plugins/howvec.c| 27 ++---
> >  contrib/plugins/hwprofile.c | 39 --
> >  contrib/plugins/lockstep.c  | 31 +---
> >  docs/devel/tcg-plugins.rst  | 38 +++---
> >  docs/system/deprecated.rst  |  6 +
> >  include/qemu/qemu-plugin.h  | 13 ++
> >  linux-user/main.c   |  2 +-
> >  plugins/api.c   |  5 
> >  plugins/loader.c| 24 +++
> >  qemu-options.hx |  9 ---
> >  tests/plugin/bb.c   | 15 
> >  tests/plugin/insn.c | 14 +--
> >  tests/plugin/mem.c  | 47 +++--
> >  tests/plugin/syscall.c  | 23 --
> >  16 files changed, 236 insertions(+), 101 deletions(-)
>
>
> --
> Alex Bennée

Re: [PATCH 00/13] new plugin argument passing scheme

[Alex Bennée]

Mahmoud Mandour  writes:

> Hello,
>
> This series removes passing arguments to plugins through "arg=" since
> it's redundant and reduces readability especially when the argument
> itself is composed of a name and a value.

When you re-roll a series it's useful to add a version tag. You can use
--subject-prefix in your git format-patch command to do this.

I'll have a look at this on Monday.

>
> Also, passing arguments through "arg=" still works but is marked as
> deprecated and will produce a deprecation warning.
>
> Right now, the code for parsing the argument before passing it to the
> plugin is unfortunately not so clean but that's mainly because "arg=" is
> still supported.
>
> At first, considering boolean parameters, those were not special to
> plugins and QEMU did not complain about passing them in the form
> "arg=bool_arg" even though that's considered a short-form boolean, which
> is deprecated. As "arg" is removed, a deprecation warning is issued.
>
> This is mitigated by making plugins aware of boolean arguments and
> parses them through a newly exposed API, namely the `qapi_bool_parse`
> function through a plugin API function. Now plugins expect boolean
> parameters to be passed in the form that other parts of QEMU expect,
> i.e. "bool_arg=[on|true|yes|off|false|no]".
>
> Since we're still supporting "arg=arg_name", there are some assumptions
> that I made that I think are suitable:
>
> 1. "arg=arg_name" will be passed to the plugin as "arg_name=on".
> 2. "arg=on" and "arg" will not be assumed to be the old way of
> passing args. Instead, it will assume that the argument name is
> "arg" and it's a boolean parameter. (will be passed to plugin
> as "arg=on")
>
> The docs are updated accordingly and a deprecation notice is put in the
> deprecated.rst file.
>
> v1 -> v2:
> 1. Added patches that handle test plugins as well
> 2. Handled unsupported arguements in howvec
>
> Based-on: <20210714172151.8494-1-ma.mando...@gmail.com>
>
> However, the dependency is so light and it should only be in the patch
>
> docs/tcg-plugins: new passing parameters scheme for cache docs
>
> where it depends on
>
> docs/devel/tcg-plugins: added cores arg to cache plugin
>
> in the aforementioned series (conflict lies in the argument "cores=N" only.)
>
> Mahmoud Mandour (13):
>   plugins: allow plugin arguments to be passed directly
>   plugins/api: added a boolean parsing plugin api
>   plugins/hotpages: introduce sortby arg and parsed bool args correctly
>   plugins/hotblocks: Added correct boolean argument parsing
>   plugins/lockstep: make socket path not positional & parse bool arg
>   plugins/hwprofile: adapt to the new plugin arguments scheme
>   plugins/howvec: Adapting to the new argument passing scheme.
>   docs/tcg-plugins: new passing parameters scheme for cache docs
>   tests/plugins/bb: adapt to the new arg passing scheme
>   tests/plugins/insn: made arg inline not positional and parse it as
> bool
>   tests/plugins/mem: introduce "track" arg and make args not positional
>   tests/plugins/syscalls: adhere to new arg-passing scheme
>   docs/deprecated: deprecate passing plugin args through `arg=`
>
>  contrib/plugins/hotblocks.c | 14 +--
>  contrib/plugins/hotpages.c  | 30 +++
>  contrib/plugins/howvec.c| 27 ++---
>  contrib/plugins/hwprofile.c | 39 --
>  contrib/plugins/lockstep.c  | 31 +---
>  docs/devel/tcg-plugins.rst  | 38 +++---
>  docs/system/deprecated.rst  |  6 +
>  include/qemu/qemu-plugin.h  | 13 ++
>  linux-user/main.c   |  2 +-
>  plugins/api.c   |  5 
>  plugins/loader.c| 24 +++
>  qemu-options.hx |  9 ---
>  tests/plugin/bb.c   | 15 
>  tests/plugin/insn.c | 14 +--
>  tests/plugin/mem.c  | 47 +++--
>  tests/plugin/syscall.c  | 23 --
>  16 files changed, 236 insertions(+), 101 deletions(-)


-- 
Alex Bennée

Re: [PATCH] target/hexagon: Drop include of qemu.h

[Alex Bennée]

Peter Maydell  writes:

> The qemu.h file is a CONFIG_USER_ONLY header; it doesn't appear on
> the include path for softmmu builds.  Currently we include it
> unconditionally in target/hexagon/op_helper.c.  We used to need it
> for the put_user_*() and get_user_*() functions, but now that we have
> removed the uses of those from op_helper.c, the only reason it's
> still there is that we're implicitly relying on it pulling in some
> other headers.
>
> Explicitly include the headers we need for other functions, and drop
> the include of qemu.h.
>
> Signed-off-by: Peter Maydell 

Reviewed-by: Alex Bennée 

-- 
Alex Bennée

[PATCH] target/hexagon: Drop include of qemu.h

[Peter Maydell]

The qemu.h file is a CONFIG_USER_ONLY header; it doesn't appear on
the include path for softmmu builds.  Currently we include it
unconditionally in target/hexagon/op_helper.c.  We used to need it
for the put_user_*() and get_user_*() functions, but now that we have
removed the uses of those from op_helper.c, the only reason it's
still there is that we're implicitly relying on it pulling in some
other headers.

Explicitly include the headers we need for other functions, and drop
the include of qemu.h.

Signed-off-by: Peter Maydell 
---
Based-on: 1626384156-6248-1-git-send-email-tsimp...@quicinc.com
("[PATCH v3 0/2] SIGSEGV fixes")

I noticed this because it's the only place in the tree where we
include qemu.h that isn't either (a) a linux-user specific file
or (b) wrapping the #include line in an ifdef CONFIG_USER_ONLY.

 target/hexagon/op_helper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/hexagon/op_helper.c b/target/hexagon/op_helper.c
index a959dba56ee..61d5cde939a 100644
--- a/target/hexagon/op_helper.c
+++ b/target/hexagon/op_helper.c
@@ -16,7 +16,8 @@
  */
 
 #include "qemu/osdep.h"
-#include "qemu.h"
+#include "qemu/log.h"
+#include "exec/exec-all.h"
 #include "exec/cpu_ldst.h"
 #include "exec/helper-proto.h"
 #include "fpu/softfloat.h"
-- 
2.20.1

[PATCH 05/13] plugins/lockstep: make socket path not positional & parse bool arg

[Mahmoud Mandour]

Signed-off-by: Mahmoud Mandour 
---
 contrib/plugins/lockstep.c | 31 ++-
 docs/devel/tcg-plugins.rst |  2 +-
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/contrib/plugins/lockstep.c b/contrib/plugins/lockstep.c
index 7fd35eb669..a41ffe83fa 100644
--- a/contrib/plugins/lockstep.c
+++ b/contrib/plugins/lockstep.c
@@ -319,22 +319,35 @@ QEMU_PLUGIN_EXPORT int 
qemu_plugin_install(qemu_plugin_id_t id,
int argc, char **argv)
 {
 int i;
-
-if (!argc || !argv[0]) {
-qemu_plugin_outs("Need a socket path to talk to other instance.");
-return -1;
-}
+g_autofree char *sock_path = NULL;
 
 for (i = 0; i < argc; i++) {
 char *p = argv[i];
-if (strcmp(p, "verbose") == 0) {
-verbose = true;
-} else if (!setup_unix_socket(argv[0])) {
-qemu_plugin_outs("Failed to setup socket for communications.");
+g_autofree char **tokens = g_strsplit(p, "=", 2);
+
+if (g_strcmp0(tokens[0], "verbose") == 0) {
+if (!qemu_plugin_bool_parse(tokens[0], tokens[1], )) {
+fprintf(stderr, "boolean argument parsing failed: %s\n", p);
+return -1;
+}
+} else if (g_strcmp0(tokens[0], "sockpath") == 0) {
+sock_path = tokens[1];
+} else {
+fprintf(stderr, "option parsing failed: %s\n", p);
 return -1;
 }
 }
 
+if (sock_path == NULL) {
+fprintf(stderr, "Need a socket path to talk to other instance.\n");
+return -1;
+}
+
+if (!setup_unix_socket(sock_path)) {
+fprintf(stderr, "Failed to setup socket for communications.\n");
+return -1;
+}
+
 our_id = id;
 
 qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
index 370c11373f..6ddf9c28c0 100644
--- a/docs/devel/tcg-plugins.rst
+++ b/docs/devel/tcg-plugins.rst
@@ -270,7 +270,7 @@ communicate over::
 
   ./sparc-softmmu/qemu-system-sparc -monitor none -parallel none \
 -net none -M SS-20 -m 256 -kernel day11/zImage.elf \
--plugin ./contrib/plugins/liblockstep.so,arg=lockstep-sparc.sock \
+-plugin ./contrib/plugins/liblockstep.so,sockpath=lockstep-sparc.sock \
   -d plugin,nochain
 
 which will eventually report::
-- 
2.25.1

Re: [PATCH v2 10/11] trace: Fold mem-internal.h into mem.h

[Philippe Mathieu-Daudé]

On 7/17/21 3:41 AM, Richard Henderson wrote:
> Since the last thing that mem.h does is include mem-internal.h,
> the symbols are not actually private.
> 
> Signed-off-by: Richard Henderson 
> ---
>  trace/mem-internal.h | 50 
>  trace/mem.h  | 50 ++--
>  2 files changed, 39 insertions(+), 61 deletions(-)
>  delete mode 100644 trace/mem-internal.h

Reviewed-by: Philippe Mathieu-Daudé 

[PATCH 13/13] docs/deprecated: deprecate passing plugin args through `arg=`

[Mahmoud Mandour]

Signed-off-by: Mahmoud Mandour 
---
 docs/system/deprecated.rst | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index e2e0090878..aaf0ee5777 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -126,6 +126,12 @@ other options have been processed.  This will either have 
no effect (if
 if they were not given.  The property is therefore useless and should not be
 specified.
 
+Plugin argument passing through ``arg=`` (since 6.1)
+
+
+Passing arguments through ``arg=`` is redundant is makes the command-line less
+readable, especially when the argument itself consist of a name and a value,
+e.g. ``arg="arg_name=arg_value"``. Therefore, the usage of ``arg`` is 
redundant.
 
 QEMU Machine Protocol (QMP) commands
 
-- 
2.25.1

[PATCH 04/13] plugins/hotblocks: Added correct boolean argument parsing

[Mahmoud Mandour]

Signed-off-by: Mahmoud Mandour 
---
 contrib/plugins/hotblocks.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/contrib/plugins/hotblocks.c b/contrib/plugins/hotblocks.c
index 4b08340143..062200a7a4 100644
--- a/contrib/plugins/hotblocks.c
+++ b/contrib/plugins/hotblocks.c
@@ -133,8 +133,18 @@ QEMU_PLUGIN_EXPORT
 int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info,
 int argc, char **argv)
 {
-if (argc && strcmp(argv[0], "inline") == 0) {
-do_inline = true;
+for (int i = 0; i < argc; i++) {
+char *opt = argv[i];
+g_autofree char **tokens = g_strsplit(opt, "=", 2);
+if (g_strcmp0(tokens[0], "inline") == 0) {
+if (!qemu_plugin_bool_parse(tokens[0], tokens[1], _inline)) {
+fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
+return -1;
+}
+} else {
+fprintf(stderr, "option parsing failed: %s\n", opt);
+return -1;
+}
 }
 
 plugin_init();
-- 
2.25.1

[PATCH 09/13] tests/plugins/bb: adapt to the new arg passing scheme

[Mahmoud Mandour]

Signed-off-by: Mahmoud Mandour 
---
 tests/plugin/bb.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/tests/plugin/bb.c b/tests/plugin/bb.c
index de09bdde4e..7d470a1011 100644
--- a/tests/plugin/bb.c
+++ b/tests/plugin/bb.c
@@ -104,10 +104,17 @@ QEMU_PLUGIN_EXPORT int 
qemu_plugin_install(qemu_plugin_id_t id,
 
 for (i = 0; i < argc; i++) {
 char *opt = argv[i];
-if (g_strcmp0(opt, "inline") == 0) {
-do_inline = true;
-} else if (g_strcmp0(opt, "idle") == 0) {
-idle_report = true;
+g_autofree char **tokens = g_strsplit(opt, "=", 2);
+if (g_strcmp0(tokens[0], "inline") == 0) {
+if (!qemu_plugin_bool_parse(tokens[0], tokens[1], _inline)) {
+fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
+return -1;
+}
+} else if (g_strcmp0(tokens[0], "idle") == 0) {
+if (!qemu_plugin_bool_parse(tokens[0], tokens[1], _report)) {
+fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
+return -1;
+}
 } else {
 fprintf(stderr, "option parsing failed: %s\n", opt);
 return -1;
-- 
2.25.1

Re: [PATCH v2 08/11] accel/tcg: Remove ATOMIC_MMU_DECLS

[Philippe Mathieu-Daudé]

On 7/17/21 3:41 AM, Richard Henderson wrote:
> All definitions are now empty.
> 
> Signed-off-by: Richard Henderson 
> ---
>  accel/tcg/atomic_template.h | 12 
>  accel/tcg/cputlb.c  |  1 -
>  accel/tcg/user-exec.c   |  1 -
>  3 files changed, 14 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé 

[PATCH 08/13] docs/tcg-plugins: new passing parameters scheme for cache docs

[Mahmoud Mandour]

Signed-off-by: Mahmoud Mandour 
---
 docs/devel/tcg-plugins.rst | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
index 4ab9dc4bb1..be1256d50c 100644
--- a/docs/devel/tcg-plugins.rst
+++ b/docs/devel/tcg-plugins.rst
@@ -349,34 +349,34 @@ will report the following::
 
 The plugin has a number of arguments, all of them are optional:
 
-  * arg="limit=N"
+  * limit=N
 
   Print top N icache and dcache thrashing instructions along with their
   address, number of misses, and its disassembly. (default: 32)
 
-  * arg="icachesize=N"
-  * arg="iblksize=B"
-  * arg="iassoc=A"
+  * icachesize=N
+  * iblksize=B
+  * iassoc=A
 
   Instruction cache configuration arguments. They specify the cache size, block
   size, and associativity of the instruction cache, respectively.
   (default: N = 16384, B = 64, A = 8)
 
-  * arg="dcachesize=N"
-  * arg="dblksize=B"
-  * arg="dassoc=A"
+  * dcachesize=N
+  * dblksize=B
+  * dassoc=A
 
   Data cache configuration arguments. They specify the cache size, block size,
   and associativity of the data cache, respectively.
   (default: N = 16384, B = 64, A = 8)
 
-  * arg="evict=POLICY"
+  * evict=POLICY
 
   Sets the eviction policy to POLICY. Available policies are: :code:`lru`,
   :code:`fifo`, and :code:`rand`. The plugin will use the specified policy for
   both instruction and data caches. (default: POLICY = :code:`lru`)
 
-  * arg="cores=N"
+  * cores=N
 
   Sets the number of cores for which we maintain separate icache and dcache.
   (default: for linux-user, N = 1, for full system emulation: N = cores
-- 
2.25.1

[PATCH 07/13] plugins/howvec: Adapting to the new argument passing scheme.

[Mahmoud Mandour]

Correctly parsing plugin argument since they now must be provided as
full-form boolean parameters, e.g.:
-plugin ./contrib/plugins/libhowvec.so,verbose=on,inline=on

Also, introduced the argument "count" that accepts one opt to count
individually at a time.

Signed-off-by: Mahmoud Mandour 
---
 contrib/plugins/howvec.c   | 27 +++
 docs/devel/tcg-plugins.rst | 10 +-
 2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/contrib/plugins/howvec.c b/contrib/plugins/howvec.c
index 600f7facc1..4a5ec3d936 100644
--- a/contrib/plugins/howvec.c
+++ b/contrib/plugins/howvec.c
@@ -333,23 +333,34 @@ QEMU_PLUGIN_EXPORT int 
qemu_plugin_install(qemu_plugin_id_t id,
 
 for (i = 0; i < argc; i++) {
 char *p = argv[i];
-if (strcmp(p, "inline") == 0) {
-do_inline = true;
-} else if (strcmp(p, "verbose") == 0) {
-verbose = true;
-} else {
+g_autofree char **tokens = g_strsplit(p, "=", -1);
+if (g_strcmp0(tokens[0], "inline") == 0) {
+if (!qemu_plugin_bool_parse(tokens[0], tokens[1], _inline)) {
+fprintf(stderr, "boolean argument parsing failed: %s\n", p);
+return -1;
+}
+} else if (g_strcmp0(tokens[0], "verbose") == 0) {
+if (!qemu_plugin_bool_parse(tokens[0], tokens[1], )) {
+fprintf(stderr, "boolean argument parsing failed: %s\n", p);
+return -1;
+}
+} else if (g_strcmp0(tokens[0], "count") == 0) {
+char *value = tokens[1];
 int j;
 CountType type = COUNT_INDIVIDUAL;
-if (*p == '!') {
+if (*value == '!') {
 type = COUNT_NONE;
-p++;
+value++;
 }
 for (j = 0; j < class_table_sz; j++) {
-if (strcmp(p, class_table[j].opt) == 0) {
+if (strcmp(value, class_table[j].opt) == 0) {
 class_table[j].what = type;
 break;
 }
 }
+} else {
+fprintf(stderr, "option parsing failed: %s\n", p);
+return -1;
 }
 }
 
diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
index 753f56ac42..4ab9dc4bb1 100644
--- a/docs/devel/tcg-plugins.rst
+++ b/docs/devel/tcg-plugins.rst
@@ -79,7 +79,7 @@ Once built a program can be run with multiple plugins loaded 
each with
 their own arguments::
 
   $QEMU $OTHER_QEMU_ARGS \
-  -plugin tests/plugin/libhowvec.so,arg=inline,arg=hint \
+  -plugin tests/plugin/libhowvec.so,inline=on,count=hint \
   -plugin tests/plugin/libhotblocks.so
 
 Arguments are plugin specific and can be used to modify their
@@ -196,13 +196,13 @@ Similar to hotblocks but this time tracks memory 
accesses::
 
 This is an instruction classifier so can be used to count different
 types of instructions. It has a number of options to refine which get
-counted. You can give an argument for a class of instructions to break
-it down fully, so for example to see all the system registers
-accesses::
+counted. You can give a value to the `count` argument for a class of
+instructions to break it down fully, so for example to see all the system
+registers accesses::
 
   ./aarch64-softmmu/qemu-system-aarch64 $(QEMU_ARGS) \
 -append "root=/dev/sda2 systemd.unit=benchmark.service" \
--smp 4 -plugin ./contrib/plugins/libhowvec.so,arg=sreg -d plugin
+-smp 4 -plugin ./contrib/plugins/libhowvec.so,count=sreg -d plugin
 
 which will lead to a sorted list after the class breakdown::
 
-- 
2.25.1

[PATCH 11/13] tests/plugins/mem: introduce "track" arg and make args not positional

[Mahmoud Mandour]

This commit makes the plugin adhere to the new plugins arg-passing
scheme by expecting full-form boolean args instead of short-form
booleans. This necessitates that we introduce a new argument, here
"track", to accept "r", "w", or "rw".

Also, it makes arguments not positional and we only care about the last
value specified for a certain argument.

callback/inline args are now supplied separately as bool arguments so
that both can be enabled individually.

Signed-off-by: Mahmoud Mandour 
---
 tests/plugin/mem.c | 47 --
 1 file changed, 29 insertions(+), 18 deletions(-)

diff --git a/tests/plugin/mem.c b/tests/plugin/mem.c
index afd1d27e5c..3000f847b5 100644
--- a/tests/plugin/mem.c
+++ b/tests/plugin/mem.c
@@ -80,29 +80,40 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t 
id,
const qemu_info_t *info,
int argc, char **argv)
 {
-if (argc) {
-if (argc >= 3) {
-if (!strcmp(argv[2], "haddr")) {
-do_haddr = true;
-}
-}
-if (argc >= 2) {
-const char *str = argv[1];
 
-if (!strcmp(str, "r")) {
+for (int i = 0; i < argc; i++) {
+char *opt = argv[i];
+g_autofree char **tokens = g_strsplit(opt, "=", 2);
+
+if (g_strcmp0(tokens[0], "hadrr") == 0) {
+if (!qemu_plugin_bool_parse(tokens[0], tokens[1], _haddr)) {
+fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
+return -1;
+}
+} else if (g_strcmp0(tokens[0], "track") == 0) {
+if (g_strcmp0(tokens[1], "r") == 0) {
 rw = QEMU_PLUGIN_MEM_R;
-} else if (!strcmp(str, "w")) {
+} else if (g_strcmp0(tokens[1], "w") == 0) {
 rw = QEMU_PLUGIN_MEM_W;
+} else if (g_strcmp0(tokens[1], "rw") == 0) {
+rw = QEMU_PLUGIN_MEM_RW;
+} else {
+fprintf(stderr, "invaild value for argument track: %s\n", opt);
+return -1;
+}
+} else if (g_strcmp0(tokens[0], "inline") == 0) {
+if (!qemu_plugin_bool_parse(tokens[0], tokens[1], _inline)) {
+fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
+return -1;
+}
+} else if (g_strcmp0(tokens[0], "callback") == 0) {
+if (!qemu_plugin_bool_parse(tokens[0], tokens[1], _callback)) {
+fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
+return -1;
 }
-}
-if (!strcmp(argv[0], "inline")) {
-do_inline = true;
-do_callback = false;
-} else if (!strcmp(argv[0], "both")) {
-do_inline = true;
-do_callback = true;
 } else {
-do_callback = true;
+fprintf(stderr, "option parsing failed: %s\n", opt);
+return -1;
 }
 }
 
-- 
2.25.1

[PATCH 10/13] tests/plugins/insn: made arg inline not positional and parse it as bool

[Mahmoud Mandour]

Made argument "inline" not positional, this has two benefits. First is
that we adhere to how QEMU passes args generally, by taking the last
value of an argument and drop the others. And the second is that this
sets up a framework for potentially adding new args easily.

Signed-off-by: Mahmoud Mandour 
---
 tests/plugin/insn.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/tests/plugin/insn.c b/tests/plugin/insn.c
index c253980ec8..0f6a1938c1 100644
--- a/tests/plugin/insn.c
+++ b/tests/plugin/insn.c
@@ -62,8 +62,18 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t 
id,
const qemu_info_t *info,
int argc, char **argv)
 {
-if (argc && !strcmp(argv[0], "inline")) {
-do_inline = true;
+for (int i = 0; i < argc; i++) {
+char *opt = argv[i];
+g_autofree char **tokens = g_strsplit(opt, "=", 2);
+if (g_strcmp0(tokens[0], "inline") == 0) {
+if (!qemu_plugin_bool_parse(tokens[0], tokens[1], _inline)) {
+fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
+return -1;
+}
+} else {
+fprintf(stderr, "option parsing failed: %s\n", opt);
+return -1;
+}
 }
 
 qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
-- 
2.25.1

[PATCH 03/13] plugins/hotpages: introduce sortby arg and parsed bool args correctly

[Mahmoud Mandour]

Since plugin arguments now expect boolean arguments, a plugin argument
name "sortby" now expects a value of "read", "write", or "address".

"io" arg is now expected to be passed as a full-form boolean parameter,
i.e. "io=on|true|yes|off|false|no"

Signed-off-by: Mahmoud Mandour 
---
 contrib/plugins/hotpages.c | 30 --
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/contrib/plugins/hotpages.c b/contrib/plugins/hotpages.c
index bf53267532..0d12910af6 100644
--- a/contrib/plugins/hotpages.c
+++ b/contrib/plugins/hotpages.c
@@ -169,16 +169,26 @@ int qemu_plugin_install(qemu_plugin_id_t id, const 
qemu_info_t *info,
 
 for (i = 0; i < argc; i++) {
 char *opt = argv[i];
-if (g_strcmp0(opt, "reads") == 0) {
-sort_by = SORT_R;
-} else if (g_strcmp0(opt, "writes") == 0) {
-sort_by = SORT_W;
-} else if (g_strcmp0(opt, "address") == 0) {
-sort_by = SORT_A;
-} else if (g_strcmp0(opt, "io") == 0) {
-track_io = true;
-} else if (g_str_has_prefix(opt, "pagesize=")) {
-page_size = g_ascii_strtoull(opt + 9, NULL, 10);
+g_autofree char **tokens = g_strsplit(opt, "=", -1);
+
+if (g_strcmp0(tokens[0], "sortby") == 0) {
+if (g_strcmp0(tokens[1], "reads") == 0) {
+sort_by = SORT_R;
+} else if (g_strcmp0(tokens[1], "writes") == 0) {
+sort_by = SORT_W;
+} else if (g_strcmp0(tokens[1], "address") == 0) {
+sort_by = SORT_A;
+} else {
+fprintf(stderr, "invalid value to sortby: %s\n", tokens[1]);
+return -1;
+}
+} else if (g_strcmp0(tokens[0], "io") == 0) {
+if (!qemu_plugin_bool_parse(tokens[0], tokens[1], _io)) {
+fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
+return -1;
+}
+} else if (g_strcmp0(tokens[0], "pagesize") == 0) {
+page_size = g_ascii_strtoull(tokens[1], NULL, 10);
 } else {
 fprintf(stderr, "option parsing failed: %s\n", opt);
 return -1;
-- 
2.25.1

[PATCH 02/13] plugins/api: added a boolean parsing plugin api

[Mahmoud Mandour]

This call will help boolean argument parsing since arguments are now
passed to plugins as a name and value.

Signed-off-by: Mahmoud Mandour 
---
 include/qemu/qemu-plugin.h | 13 +
 plugins/api.c  |  5 +
 2 files changed, 18 insertions(+)

diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
index dc3496f36c..7d0b23c659 100644
--- a/include/qemu/qemu-plugin.h
+++ b/include/qemu/qemu-plugin.h
@@ -564,4 +564,17 @@ int qemu_plugin_n_max_vcpus(void);
  */
 void qemu_plugin_outs(const char *string);
 
+/**
+ * qemu_plugin_bool_parse() - parses a boolean argument in the form of
+ * "=[on|yes|true|off|no|false]"
+ *
+ * @name: argument name, the part before the equals sign
+ * @val: argument value, what's after the equals sign
+ * @ret: output return value
+ *
+ * returns true if the combination @name=@val parses correctly to a boolean
+ * argument, and false otherwise
+ */
+bool qemu_plugin_bool_parse(const char *name, const char *val, bool *ret);
+
 #endif /* QEMU_PLUGIN_API_H */
diff --git a/plugins/api.c b/plugins/api.c
index 332e2c60e2..43e239f377 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -383,3 +383,8 @@ void qemu_plugin_outs(const char *string)
 {
 qemu_log_mask(CPU_LOG_PLUGIN, "%s", string);
 }
+
+bool qemu_plugin_bool_parse(const char *name, const char *value, bool *ret)
+{
+return qapi_bool_parse(name, value, ret, NULL);
+}
-- 
2.25.1

[PATCH 12/13] tests/plugins/syscalls: adhere to new arg-passing scheme

[Mahmoud Mandour]

Signed-off-by: Mahmoud Mandour 
---
 tests/plugin/syscall.c | 23 ---
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/tests/plugin/syscall.c b/tests/plugin/syscall.c
index 6dd71092e1..484b48de49 100644
--- a/tests/plugin/syscall.c
+++ b/tests/plugin/syscall.c
@@ -119,17 +119,26 @@ QEMU_PLUGIN_EXPORT int 
qemu_plugin_install(qemu_plugin_id_t id,
const qemu_info_t *info,
int argc, char **argv)
 {
-if (argc == 0) {
-statistics = g_hash_table_new_full(NULL, g_direct_equal, NULL, g_free);
-} else {
-for (int i = 0; i < argc; i++) {
-if (g_strcmp0(argv[i], "print") != 0) {
-fprintf(stderr, "unsupported argument: %s\n", argv[i]);
-return -1;
+bool do_print = false;
+
+for (int i = 0; i < argc; i++) {
+char *opt = argv[i];
+g_autofree char **tokens = g_strsplit(opt, "=", 2);
+
+if (g_strcmp0(tokens[0], "print") == 0) {
+if (!qemu_plugin_bool_parse(tokens[0], tokens[1], _print)) {
+fprintf(stderr, "boolean argument parsing failed: %s\n", opt);
 }
+} else {
+fprintf(stderr, "unsupported argument: %s\n", argv[i]);
+return -1;
 }
 }
 
+if (!do_print) {
+statistics = g_hash_table_new_full(NULL, g_direct_equal, NULL, g_free);
+}
+
 qemu_plugin_register_vcpu_syscall_cb(id, vcpu_syscall);
 qemu_plugin_register_vcpu_syscall_ret_cb(id, vcpu_syscall_ret);
 qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
-- 
2.25.1

[PATCH 00/13] new plugin argument passing scheme

[Mahmoud Mandour]

Hello,

This series removes passing arguments to plugins through "arg=" since
it's redundant and reduces readability especially when the argument
itself is composed of a name and a value.

Also, passing arguments through "arg=" still works but is marked as
deprecated and will produce a deprecation warning.

Right now, the code for parsing the argument before passing it to the
plugin is unfortunately not so clean but that's mainly because "arg=" is
still supported.

At first, considering boolean parameters, those were not special to
plugins and QEMU did not complain about passing them in the form
"arg=bool_arg" even though that's considered a short-form boolean, which
is deprecated. As "arg" is removed, a deprecation warning is issued.

This is mitigated by making plugins aware of boolean arguments and
parses them through a newly exposed API, namely the `qapi_bool_parse`
function through a plugin API function. Now plugins expect boolean
parameters to be passed in the form that other parts of QEMU expect,
i.e. "bool_arg=[on|true|yes|off|false|no]".

Since we're still supporting "arg=arg_name", there are some assumptions
that I made that I think are suitable:

1. "arg=arg_name" will be passed to the plugin as "arg_name=on".
2. "arg=on" and "arg" will not be assumed to be the old way of
passing args. Instead, it will assume that the argument name is
"arg" and it's a boolean parameter. (will be passed to plugin
as "arg=on")

The docs are updated accordingly and a deprecation notice is put in the
deprecated.rst file.

v1 -> v2:
1. Added patches that handle test plugins as well
2. Handled unsupported arguements in howvec

Based-on: <20210714172151.8494-1-ma.mando...@gmail.com>

However, the dependency is so light and it should only be in the patch

docs/tcg-plugins: new passing parameters scheme for cache docs

where it depends on

docs/devel/tcg-plugins: added cores arg to cache plugin

in the aforementioned series (conflict lies in the argument "cores=N" only.)

Mahmoud Mandour (13):
  plugins: allow plugin arguments to be passed directly
  plugins/api: added a boolean parsing plugin api
  plugins/hotpages: introduce sortby arg and parsed bool args correctly
  plugins/hotblocks: Added correct boolean argument parsing
  plugins/lockstep: make socket path not positional & parse bool arg
  plugins/hwprofile: adapt to the new plugin arguments scheme
  plugins/howvec: Adapting to the new argument passing scheme.
  docs/tcg-plugins: new passing parameters scheme for cache docs
  tests/plugins/bb: adapt to the new arg passing scheme
  tests/plugins/insn: made arg inline not positional and parse it as
bool
  tests/plugins/mem: introduce "track" arg and make args not positional
  tests/plugins/syscalls: adhere to new arg-passing scheme
  docs/deprecated: deprecate passing plugin args through `arg=`

 contrib/plugins/hotblocks.c | 14 +--
 contrib/plugins/hotpages.c  | 30 +++
 contrib/plugins/howvec.c| 27 ++---
 contrib/plugins/hwprofile.c | 39 --
 contrib/plugins/lockstep.c  | 31 +---
 docs/devel/tcg-plugins.rst  | 38 +++---
 docs/system/deprecated.rst  |  6 +
 include/qemu/qemu-plugin.h  | 13 ++
 linux-user/main.c   |  2 +-
 plugins/api.c   |  5 
 plugins/loader.c| 24 +++
 qemu-options.hx |  9 ---
 tests/plugin/bb.c   | 15 
 tests/plugin/insn.c | 14 +--
 tests/plugin/mem.c  | 47 +++--
 tests/plugin/syscall.c  | 23 --
 16 files changed, 236 insertions(+), 101 deletions(-)

-- 
2.25.1