[Bug 1414466] Re: -net user, hostfwd=... is not working(qemu-system-aarch64)

2020-12-19 Thread Launchpad Bug Tracker
[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  -net user,hostfwd=... is not working(qemu-system-aarch64)

Status in QEMU:
  Expired

Bug description:
  QEMU version: git a46b3aaf6bb038d4f6f192a84df204f10929e75c

   /opt/qemu.git/bin/qemu-system-aarch64 --version
  QEMU emulator version 2.2.50, Copyright (c) 2003-2008 Fabrice Bellard

  Hosts:
  ovs - host machine (Ubuntu 14.04.1, x86_64)
  debian8-arm64 - guest 

  Guest start:
  user@ovs:~$ /opt/qemu.git/bin/qemu-system-aarch64 -machine virt -cpu 
cortex-a57 -nographic -smp 1 -m 512 -kernel vmlinuz-run -initrd initrd-run.img 
-append "root=/dev/sda2 console=ttyAMA0" -global virtio-blk-device.scsi=off 
-device virtio-scsi-device,id=scsi -drive 
file=debian8-arm64.img,id=rootimg,cache=unsafe,if=none -device 
scsi-hd,drive=rootimg -netdev user,id=unet -device 
virtio-net-device,netdev=unet -net user,hostfwd=tcp:127.0.0.1:1122-:22

  root@debian8-arm64:~# netstat -ntplu | grep ssh
  tcp0  0 0.0.0.0:22  0.0.0.0:*   LISTEN
  410/sshd
  tcp6   0  0 :::22   :::*LISTEN
  410/sshd   

  (no firewall in guest vm)

  user@ovs:~$ netstat -ntplu | grep 1122
  tcp0  0 127.0.0.1:1122  0.0.0.0:*   LISTEN
  18722/qemu-system-a

  user@ovs:~$ time ssh user@127.0.0.1 -p 1122
  ssh_exchange_identification: read: Connection reset by peer

  real  1m29.341s
  user  0m0.005s
  sys   0m0.000s

  Inside guest vm sshd works fine:
  root@debian8-arm64:~# ssh user@127.0.0.1 -p 22
  user@127.0.0.1's password: 
  
  user@debian8-arm64:~$ exit
  logout
  Connection to 127.0.0.1 closed.

  root@debian8-arm64:~# ssh user@10.0.2.15 -p 22
  user@10.0.2.15's password: 
  ...
  user@debian8-arm64:~$ exit
  logout
  Connection to 10.0.2.15 closed.

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



virtiofsd: sshfs as submount?

2020-12-19 Thread Laszlo Ersek
Hi Miklos,

the following 2019 presentation on Stefan's website:

  https://vmsplice.net/
  virtio-fs: A Shared File System for Virtual Machines at KVM Forum 2019
  
https://vmsplice.net/~stefan/virtio-fs_%20A%20Shared%20File%20System%20for%20Virtual%20Machines.pdf

has a slide called "Use case: File system-as-a-service" (slide#4). It
seems to confirm my "grand" idea to expose an sshfs submount to the
guest, via virtiofsd. (The guest need not / should not know it's a
submount, just see the files.) Beyond the pure utility of this, it feels
exciting to chain FUSE to FUSE. :)

I've tried it; the FUSE_READDIRPLUS request fails.

[2020-12-20 00:32:08.64+0100] [ID: 0006] unique: 83, opcode: READDIRPLUS 
(44), nodeid: 1, insize: 80, pid: 1
[2020-12-20 00:32:08.64+0100] [ID: 0006]unique: 83, error: -13 
(Permission denied), outsize: 16

More precisely, it fails on the directory entry in the containing
directory that is the sshfs mount point, when listing the containing
directory.

I've skimmed the following thread:

  [PATCH] virtiofsd: Show submounts
  https://www.redhat.com/archives/virtio-fs/2020-April/msg00023.html

(which is now QEMU commit ace0829c0d08), and I vaguely suspect it should
work -- the MS_REC flag is present, and the MS_REC flag seems to be so
old that I think my host kernel (latest RHEL7) must support it too.

So... does the sshfs filesystem present itself as unshareable? Is it
supposed to work? Does it break for others too?

Thanks!
Laszlo




[Bug 1906193] Re: riscv32 user mode emulation: fork return values broken

2020-12-19 Thread Andreas K . Hüttel
Thanks a lot! Will test and post the result on monday when I'm back
home.

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

Title:
  riscv32 user mode emulation: fork return values broken

Status in QEMU:
  New

Bug description:
  When running in a chroot with riscv32 (on x86_64; qemu git master as
  of today):

  The following short program forks; the child immediately returns with
  exit(42). The parent checks for the return value - and obtains 40!

  gcc-10.2

  ===
  #include 
  #include 
  #include 
  #include 

  main(c, v)
   int c;
   char **v;
  {
pid_t pid, p;
int s, i, n;

s = 0;
pid = fork();
if (pid == 0)
  exit(42);

/* wait for the process */
p = wait();
if (p != pid)
  exit (255);

if (WIFEXITED(s))
{
   int r=WEXITSTATUS(s);
   if (r!=42) {
printf("child wants to return %i (0x%X), parent received %i (0x%X), 
difference %i\n",42,42,r,r,r-42);
   }
}
  }
  ===

  (riscv-ilp32 chroot) farino /tmp # ./wait-test-short 
  child wants to return 42 (0x2A), parent received 40 (0x28), difference -2

  ===
  (riscv-ilp32 chroot) farino /tmp # gcc --version
  gcc (Gentoo 10.2.0-r1 p2) 10.2.0
  Copyright (C) 2020 Free Software Foundation, Inc.
  Dies ist freie Software; die Kopierbedingungen stehen in den Quellen. Es
  gibt KEINE Garantie; auch nicht für MARKTGÄNGIGKEIT oder FÜR SPEZIELLE ZWECKE.

  (riscv-ilp32 chroot) farino /tmp # ld --version
  GNU ld (Gentoo 2.34 p6) 2.34.0
  Copyright (C) 2020 Free Software Foundation, Inc.
  This program is free software; you may redistribute it under the terms of
  the GNU General Public License version 3 or (at your option) a later version.
  This program has absolutely no warranty.

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



Re: [PATCH 2/4] tcg: Additional Trap type for FreeBSD

2020-12-19 Thread Warner Losh
Again, this turns out to be bogus, for reasons I enumerated the first time
it sent...

Warner

On Sat, Dec 19, 2020 at 1:54 PM  wrote:

> From: Sean Bruno 
>
> FreeBSD can generate a trap 0xc as well as 0xe when writing to a
> read-only page.
>
> Signed-off-by: Juergen Lock 
> [imp rewored commit message for clarity]
> Signed-off-by: Warner Losh 
> ---
>  accel/tcg/user-exec.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
> index 4ebe25461a..1f5befa9f9 100644
> --- a/accel/tcg/user-exec.c
> +++ b/accel/tcg/user-exec.c
> @@ -343,7 +343,13 @@ int cpu_signal_handler(int host_signum, void *pinfo,
>
>  pc = PC_sig(uc);
>  return handle_cpu_signal(pc, info,
> - TRAP_sig(uc) == 0xe ? (ERROR_sig(uc) >> 1) &
> 1 : 0,
> +#if defined(__FreeBSD__) || defined(__DragonFly__)
> + (TRAP_sig(uc) == 0xe ||
> +  TRAP_sig(uc) == 0xc) ?
> +#else
> + TRAP_sig(uc) == 0xe ?
> +#endif
> + (ERROR_sig(uc) >> 1) & 1 : 0,
>   _sig(uc));
>  }
>
> --
> 2.22.1
>
>
>


Re: [PATCH 0/4] A few preliminary bsd-user patches

2020-12-19 Thread Warner Losh
I'm not at all sure why this was sent twice.  I thought I'd cleared out all
the 'failed' attempts on the first system I tried to git email-send from
after realizing sendmail hadn't been configured...

Sorry about that.

Warner

On Sat, Dec 19, 2020 at 1:54 PM  wrote:

> From: Warner Losh 
>
> Here's the first round of bsd-user patches. There's on the order of 280
> that
> we've done, but that's too much to review all at once. In addition, 3.1
> release
> was the last rebase point that we've been successful with for a number of
> reasons
> unrelated to qemu. Now that those have been resolved, we have a new push
> under way
> to push things forward, but wanted to upstream as many of the patches as
> we can
> directly to qemu's head to lighten the load of carrying all these.
>
> This first small series updates the system call lists, moves things around
> to
> make it easier to support divergence in the BSD world, and adjusts to the
> new
> meson build. It's also designed to help me learn how to land such a large
> set
> upstream.
>
> These patches have passed through several hands, with different tweaks
> over the
> years so have an unusually large number of signed-off-by lines that are the
> result of this refinement process where several hands have touched the
> patches
> in the last 7 years.
>
> Sean Bruno (1):
>   tcg: Additional Trap type for FreeBSD
>
> Stacey Son (1):
>   bsd-user: move strace OS/arch dependent code to host/arch dirs
>
> Warner Losh (2):
>   bsd-user: regenerate FreeBSD's system call numbers
>   bsd-user: Update strace.list for FreeBSD's latest syscalls
>
>  accel/tcg/user-exec.c  |   8 +-
>  bsd-user/arm/target_arch_sysarch.h |  78 +++
>  bsd-user/arm/target_syscall.h  |  36 ++
>  bsd-user/freebsd/os-strace.h   |  29 ++
>  bsd-user/freebsd/strace.list   |  65 ++-
>  bsd-user/freebsd/syscall_nr.h  | 695 ++---
>  bsd-user/i386/target_arch_sysarch.h|  77 +++
>  bsd-user/i386/target_syscall.h |  19 +
>  bsd-user/mips/target_arch_sysarch.h|  69 +++
>  bsd-user/mips/target_syscall.h |  52 ++
>  bsd-user/mips64/target_arch_sysarch.h  |  69 +++
>  bsd-user/mips64/target_syscall.h   |  53 ++
>  bsd-user/netbsd/os-strace.h|   1 +
>  bsd-user/openbsd/os-strace.h   |   1 +
>  bsd-user/sparc/target_arch_sysarch.h   |  52 ++
>  bsd-user/sparc/target_syscall.h|  24 +-
>  bsd-user/sparc64/target_arch_sysarch.h |  52 ++
>  bsd-user/sparc64/target_syscall.h  |  24 +-
>  bsd-user/strace.c  |  11 +
>  bsd-user/x86_64/target_arch_sysarch.h  |  76 +++
>  bsd-user/x86_64/target_syscall.h   |  21 +-
>  meson.build|   1 +
>  22 files changed, 1186 insertions(+), 327 deletions(-)
>  create mode 100644 bsd-user/arm/target_arch_sysarch.h
>  create mode 100644 bsd-user/arm/target_syscall.h
>  create mode 100644 bsd-user/freebsd/os-strace.h
>  create mode 100644 bsd-user/i386/target_arch_sysarch.h
>  create mode 100644 bsd-user/mips/target_arch_sysarch.h
>  create mode 100644 bsd-user/mips/target_syscall.h
>  create mode 100644 bsd-user/mips64/target_arch_sysarch.h
>  create mode 100644 bsd-user/mips64/target_syscall.h
>  create mode 100644 bsd-user/netbsd/os-strace.h
>  create mode 100644 bsd-user/openbsd/os-strace.h
>  create mode 100644 bsd-user/sparc/target_arch_sysarch.h
>  create mode 100644 bsd-user/sparc64/target_arch_sysarch.h
>  create mode 100644 bsd-user/x86_64/target_arch_sysarch.h
>
> --
> 2.22.1
>
>
>


Re: [PULL 02/45] vl: remove separate preconfig main_loop

2020-12-19 Thread Laurent Vivier
Le 15/12/2020 à 18:54, Paolo Bonzini a écrit :
> Move post-preconfig initialization to the x-exit-preconfig.  If preconfig
> is not requested, just exit preconfig mode immediately with the QMP
> command.
> 
> As a result, the preconfig loop will run with accel_setup_post
> and os_setup_post restrictions (xen_restrict, chroot, etc.)
> already done.
> 
> Reviewed-by: Igor Mammedov 
> Signed-off-by: Paolo Bonzini 
> ---
>  include/sysemu/runstate.h |  1 -
>  monitor/qmp-cmds.c|  9 
>  softmmu/vl.c  | 95 +--
>  3 files changed, 41 insertions(+), 64 deletions(-)

Moving the qemu_init_displays() before qemu_create_cli_devices() breaks the 
display when the graphic
card is provided by the cli.

Try:

qemu-system-x86_64 -device virtio-gpu-pci

-> we don't have any display, only the parallel port output...

Then:

diff --git a/softmmu/vl.c b/softmmu/vl.c
index 0ed5c5ba9348..2f198e0023e0 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2525,6 +2525,7 @@ void qmp_x_exit_preconfig(Error **errp)

 qemu_init_board();
 qemu_create_cli_devices();
+qemu_init_displays();
 qemu_machine_creation_done();

 if (loadvm) {
@@ -3529,7 +3530,6 @@ void qemu_init(int argc, char **argv, char **envp)
 exit(0);
 }

-qemu_init_displays();
 if (!preconfig_requested) {
 qmp_x_exit_preconfig(_fatal);
 }

and then re-try:

qemu-system-x86_64 -device virtio-gpu-pci

-> we have the BIOS boot screen.

Any idea?

Thanks,
Laurent


> diff --git a/include/sysemu/runstate.h b/include/sysemu/runstate.h
> index f760094858..e557f470d4 100644
> --- a/include/sysemu/runstate.h
> +++ b/include/sysemu/runstate.h
> @@ -41,7 +41,6 @@ typedef enum WakeupReason {
>  QEMU_WAKEUP_REASON_OTHER,
>  } WakeupReason;
>  
> -void qemu_exit_preconfig_request(void);
>  void qemu_system_reset_request(ShutdownCause reason);
>  void qemu_system_suspend_request(void);
>  void qemu_register_suspend_notifier(Notifier *notifier);
> diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
> index 639a219294..d141aaa132 100644
> --- a/monitor/qmp-cmds.c
> +++ b/monitor/qmp-cmds.c
> @@ -102,15 +102,6 @@ void qmp_system_powerdown(Error **errp)
>  qemu_system_powerdown_request();
>  }
>  
> -void qmp_x_exit_preconfig(Error **errp)
> -{
> -if (qdev_hotplug) {
> -error_setg(errp, "The command is permitted only before machine 
> initialization");
> -return;
> -}
> -qemu_exit_preconfig_request();
> -}
> -
>  void qmp_cont(Error **errp)
>  {
>  BlockBackend *blk;
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index ab2210bc79..a83e1a 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -1151,7 +1151,6 @@ static pid_t shutdown_pid;
>  static int powerdown_requested;
>  static int debug_requested;
>  static int suspend_requested;
> -static bool preconfig_exit_requested = true;
>  static WakeupReason wakeup_reason;
>  static NotifierList powerdown_notifiers =
>  NOTIFIER_LIST_INITIALIZER(powerdown_notifiers);
> @@ -1238,11 +1237,6 @@ static int qemu_debug_requested(void)
>  return r;
>  }
>  
> -void qemu_exit_preconfig_request(void)
> -{
> -preconfig_exit_requested = true;
> -}
> -
>  /*
>   * Reset the VM. Issue an event unless @reason is SHUTDOWN_CAUSE_NONE.
>   */
> @@ -1464,10 +1458,6 @@ static bool main_loop_should_exit(void)
>  RunState r;
>  ShutdownCause request;
>  
> -if (preconfig_exit_requested) {
> -preconfig_exit_requested = false;
> -return true;
> -}
>  if (qemu_debug_requested()) {
>  vm_stop(RUN_STATE_DEBUG);
>  }
> @@ -3283,6 +3273,43 @@ static void qemu_machine_creation_done(void)
>  register_global_state();
>  }
>  
> +void qmp_x_exit_preconfig(Error **errp)
> +{
> +if (qdev_hotplug) {
> +error_setg(errp, "The command is permitted only before machine 
> initialization");
> +return;
> +}
> +
> +qemu_init_board();
> +qemu_create_cli_devices();
> +qemu_machine_creation_done();
> +
> +if (loadvm) {
> +Error *local_err = NULL;
> +if (load_snapshot(loadvm, _err) < 0) {
> +error_report_err(local_err);
> +autostart = 0;
> +exit(1);
> +}
> +}
> +if (replay_mode != REPLAY_MODE_NONE) {
> +replay_vmstate_init();
> +}
> +
> +if (incoming) {
> +Error *local_err = NULL;
> +if (strcmp(incoming, "defer") != 0) {
> +qmp_migrate_incoming(incoming, _err);
> +if (local_err) {
> +error_reportf_err(local_err, "-incoming %s: ", incoming);
> +exit(1);
> +}
> +}
> +} else if (autostart) {
> +qmp_cont(NULL);
> +}
> +}
> +
>  void qemu_init(int argc, char **argv, char **envp)
>  {
>  QemuOpts *opts;
> @@ -3847,7 +3874,6 @@ void qemu_init(int argc, char **argv, char **envp)
>  }
>  break;
>  case 

[PATCH 3/4] bsd-user: move strace OS/arch dependent code to host/arch dirs

2020-12-19 Thread imp
From: Stacey Son 

This change moves host OS and arch dependent code for the sysarch
system call related to the -strace functionality into the
appropriate host OS and target arch directories.

Signed-off-by: Stacey Son 
Signed-off-by: Sean Bruno 
[ imp integrated minor build fixes from sbruno ]
Signed-off-by: Warner Losh 
---
 bsd-user/arm/target_arch_sysarch.h | 78 ++
 bsd-user/arm/target_syscall.h  | 36 
 bsd-user/freebsd/os-strace.h   | 29 ++
 bsd-user/freebsd/strace.list   |  3 +-
 bsd-user/i386/target_arch_sysarch.h| 77 +
 bsd-user/i386/target_syscall.h | 19 +++
 bsd-user/mips/target_arch_sysarch.h| 69 +++
 bsd-user/mips/target_syscall.h | 52 +
 bsd-user/mips64/target_arch_sysarch.h  | 69 +++
 bsd-user/mips64/target_syscall.h   | 53 +
 bsd-user/netbsd/os-strace.h|  1 +
 bsd-user/openbsd/os-strace.h   |  1 +
 bsd-user/sparc/target_arch_sysarch.h   | 52 +
 bsd-user/sparc/target_syscall.h| 24 +++-
 bsd-user/sparc64/target_arch_sysarch.h | 52 +
 bsd-user/sparc64/target_syscall.h  | 24 +++-
 bsd-user/strace.c  | 11 
 bsd-user/x86_64/target_arch_sysarch.h  | 76 +
 bsd-user/x86_64/target_syscall.h   | 21 ++-
 meson.build|  1 +
 20 files changed, 744 insertions(+), 4 deletions(-)
 create mode 100644 bsd-user/arm/target_arch_sysarch.h
 create mode 100644 bsd-user/arm/target_syscall.h
 create mode 100644 bsd-user/freebsd/os-strace.h
 create mode 100644 bsd-user/i386/target_arch_sysarch.h
 create mode 100644 bsd-user/mips/target_arch_sysarch.h
 create mode 100644 bsd-user/mips/target_syscall.h
 create mode 100644 bsd-user/mips64/target_arch_sysarch.h
 create mode 100644 bsd-user/mips64/target_syscall.h
 create mode 100644 bsd-user/netbsd/os-strace.h
 create mode 100644 bsd-user/openbsd/os-strace.h
 create mode 100644 bsd-user/sparc/target_arch_sysarch.h
 create mode 100644 bsd-user/sparc64/target_arch_sysarch.h
 create mode 100644 bsd-user/x86_64/target_arch_sysarch.h

diff --git a/bsd-user/arm/target_arch_sysarch.h 
b/bsd-user/arm/target_arch_sysarch.h
new file mode 100644
index 00..632a5cd453
--- /dev/null
+++ b/bsd-user/arm/target_arch_sysarch.h
@@ -0,0 +1,78 @@
+/*
+ *  arm sysarch() system call emulation
+ *
+ *  Copyright (c) 2013 Stacey D. Son
+ *
+ *  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 BSD_USER_ARCH_SYSARCH_H_
+#define BSD_USER_ARCH_SYSARCH_H_
+
+#include "target_syscall.h"
+#include "target_arch.h"
+
+static inline abi_long do_freebsd_arch_sysarch(CPUARMState *env, int op,
+abi_ulong parms)
+{
+int ret = 0;
+
+switch (op) {
+case TARGET_FREEBSD_ARM_SYNC_ICACHE:
+case TARGET_FREEBSD_ARM_DRAIN_WRITEBUF:
+break;
+
+case TARGET_FREEBSD_ARM_SET_TP:
+target_cpu_set_tls(env, parms);
+break;
+
+case TARGET_FREEBSD_ARM_GET_TP:
+ret = target_cpu_get_tls(env);
+break;
+
+default:
+ret = -TARGET_EINVAL;
+break;
+}
+return ret;
+}
+
+static inline void do_freebsd_arch_print_sysarch(
+const struct syscallname *name, abi_long arg1, abi_long arg2,
+abi_long arg3, abi_long arg4, abi_long arg5, abi_long arg6)
+{
+
+switch (arg1) {
+case TARGET_FREEBSD_ARM_SYNC_ICACHE:
+gemu_log("%s(ARM_SYNC_ICACHE, ...)", name->name);
+break;
+
+case TARGET_FREEBSD_ARM_DRAIN_WRITEBUF:
+gemu_log("%s(ARM_DRAIN_WRITEBUF, ...)", name->name);
+break;
+
+case TARGET_FREEBSD_ARM_SET_TP:
+gemu_log("%s(ARM_SET_TP, 0x" TARGET_ABI_FMT_lx ")", name->name, arg2);
+break;
+
+case TARGET_FREEBSD_ARM_GET_TP:
+gemu_log("%s(ARM_GET_TP, 0x" TARGET_ABI_FMT_lx ")", name->name, arg2);
+break;
+
+default:
+gemu_log("UNKNOWN OP: %d, " TARGET_ABI_FMT_lx ")", (int)arg1, arg2);
+}
+}
+
+#endif /*!BSD_USER_ARCH_SYSARCH_H_ */
diff --git a/bsd-user/arm/target_syscall.h b/bsd-user/arm/target_syscall.h
new file mode 100644
index 00..ef4b37f017
--- /dev/null
+++ b/bsd-user/arm/target_syscall.h
@@ -0,0 +1,36 @@
+#ifndef BSD_USER_ARCH_SYSCALL_H_
+#define 

[PATCH 4/4] bsd-user: Update strace.list for FreeBSD's latest syscalls

2020-12-19 Thread imp
From: Warner Losh 

Update strace.list to include all of FreeBSD's syscalls up through svn
r331280.

Signed-off-by: Stacey Son 
Signed-off-by: Sean Bruno 
Signed-off-by: Alexander Kabaev 
Signed-off-by: Jung-uk Kim 
Author: Michal Meloun 
Signed-off-by: Mikaël Urankar 
[imp moved this change to early in the sequence]
Signed-off-by: Warner Losh 
---
 bsd-user/freebsd/strace.list | 62 
 1 file changed, 55 insertions(+), 7 deletions(-)

diff --git a/bsd-user/freebsd/strace.list b/bsd-user/freebsd/strace.list
index d8f2eb66a6..b01b5f36e8 100644
--- a/bsd-user/freebsd/strace.list
+++ b/bsd-user/freebsd/strace.list
@@ -33,14 +33,32 @@
 { TARGET_FREEBSD_NR___syscall, "__syscall", NULL, NULL, NULL },
 { TARGET_FREEBSD_NR___sysctl, "__sysctl", NULL, print_sysctl, NULL },
 { TARGET_FREEBSD_NR__umtx_op, "_umtx_op", "%s(%#x, %d, %d, %#x, %#x)", NULL, 
NULL },
+#if defined(__FreeBSD_version) && __FreeBSD_version < 100
+{ TARGET_FREEBSD_NR__umtx_lock, "__umtx_lock", NULL, NULL, NULL },
+{ TARGET_FREEBSD_NR__umtx_unlock, "__umtx_unlock", NULL, NULL, NULL },
+#endif
 { TARGET_FREEBSD_NR_accept, "accept", "%s(%d,%#x,%#x)", NULL, NULL },
+{ TARGET_FREEBSD_NR_accept4, "accept4", "%s(%d,%d,%#x,%#x)", NULL, NULL },
 { TARGET_FREEBSD_NR_access, "access", "%s(\"%s\",%#o)", NULL, NULL },
 { TARGET_FREEBSD_NR_acct, "acct", NULL, NULL, NULL },
 { TARGET_FREEBSD_NR_adjtime, "adjtime", NULL, NULL, NULL },
 { TARGET_FREEBSD_NR_bind, "bind", NULL, NULL, NULL },
+{ TARGET_FREEBSD_NR_bindat, "bindat", NULL, NULL, NULL },
 { TARGET_FREEBSD_NR_break, "break", NULL, NULL, NULL },
+{ TARGET_FREEBSD_NR_cap_enter, "cap_enter", NULL, NULL, NULL },
+{ TARGET_FREEBSD_NR_cap_fcntls_get, "cap_fcntls_get", NULL, NULL, NULL },
+{ TARGET_FREEBSD_NR_cap_fcntls_limit, "cap_fcntls_limit", NULL, NULL, NULL },
+{ TARGET_FREEBSD_NR_cap_getmode, "cap_getmode", NULL, NULL, NULL },
+#if defined(__FreeBSD_version) && __FreeBSD_version < 100
+{ TARGET_FREEBSD_NR_cap_getrights, "cap_getrights", NULL, NULL, NULL },
+{ TARGET_FREEBSD_NR_cap_new, "cap_new", NULL, NULL, NULL },
+#endif
+{ TARGET_FREEBSD_NR_cap_ioctls_get, "cap_ioctls_get", NULL, NULL, NULL },
+{ TARGET_FREEBSD_NR_cap_ioctls_limit, "cap_ioctls_limit", NULL, NULL, NULL },
+{ TARGET_FREEBSD_NR_cap_rights_limit, "cap_rights_limit", NULL, NULL, NULL },
 { TARGET_FREEBSD_NR_chdir, "chdir", "%s(\"%s\")", NULL, NULL },
 { TARGET_FREEBSD_NR_chflags, "chflags", NULL, NULL, NULL },
+{ TARGET_FREEBSD_NR_chflagsat, "chflagsat", NULL, NULL, NULL },
 { TARGET_FREEBSD_NR_chmod, "chmod", "%s(\"%s\",%#o)", NULL, NULL },
 { TARGET_FREEBSD_NR_chown, "chown", NULL, NULL, NULL },
 { TARGET_FREEBSD_NR_chroot, "chroot", NULL, NULL, NULL },
@@ -49,6 +67,9 @@
 { TARGET_FREEBSD_NR_clock_settime, "clock_settime", NULL, NULL, NULL },
 { TARGET_FREEBSD_NR_close, "close", "%s(%d)", NULL, NULL },
 { TARGET_FREEBSD_NR_connect, "connect", "%s(%d,%#x,%d)", NULL, NULL },
+{ TARGET_FREEBSD_NR_connectat, "connectat", "%s(%d,%d,%#x,%d)", NULL, NULL },
+{ TARGET_FREEBSD_NR_cpuset_getdomain, "cpuset_getdomain", NULL, NULL, NULL },
+{ TARGET_FREEBSD_NR_cpuset_setdomain, "cpuset_setdomain", NULL, NULL, NULL },
 { TARGET_FREEBSD_NR_dup, "dup", NULL, NULL, NULL },
 { TARGET_FREEBSD_NR_dup2, "dup2", NULL, NULL, NULL },
 { TARGET_FREEBSD_NR_eaccess, "eaccess", "%s(\"%s\",%#x)", NULL, NULL },
@@ -62,7 +83,7 @@
 { TARGET_FREEBSD_NR_extattr_get_file, "extattr_get_file", "%s(\"%s\", %d, 
\"%s\", %#x, %d)", NULL, NULL },
 { TARGET_FREEBSD_NR_extattr_get_file, "extattr_get_link", "%s(\"%s\", %d, 
\"%s\", %#x, %d)", NULL, NULL },
 { TARGET_FREEBSD_NR_extattr_list_fd, "extattr_list_fd", "%s(%d, %d, %#x, %d)", 
NULL, NULL },
-{ TARGET_FREEBSD_NR_extattr_list_file, "extattr_list_file", "%s(\"%s\", %d, 
%#x, %d)", NULL, NULL },
+{ TARGET_FREEBSD_NR_extattr_list_file, "extattr_list_file", "%s(\"%s\", %#x, 
%d)", NULL, NULL },
 { TARGET_FREEBSD_NR_extattr_list_link, "extattr_list_link", "%s(\"%s\", %d, 
%#x, %d)", NULL, NULL },
 { TARGET_FREEBSD_NR_extattr_set_fd, "extattr_set_fd", "%s(%d, %d, \"%s\", %#x, 
%d)", NULL, NULL },
 { TARGET_FREEBSD_NR_extattr_set_file, "extattr_set_file", "%s(\"%s\", %d, 
\"%s\", %#x, %d)", NULL, NULL },
@@ -72,26 +93,34 @@
 { TARGET_FREEBSD_NR_fchmod, "fchmod", "%s(%d,%#o)", NULL, NULL },
 { TARGET_FREEBSD_NR_fchown, "fchown", "%s(%d,%d,%d)", NULL, NULL },
 { TARGET_FREEBSD_NR_fcntl, "fcntl", NULL, NULL, NULL },
+{ TARGET_FREEBSD_NR_fdatasync, "fdatasync", NULL, NULL, NULL },
 { TARGET_FREEBSD_NR_fexecve, "fexecve", NULL, print_execve, NULL },
 { TARGET_FREEBSD_NR_fhopen, "fhopen", NULL, NULL, NULL },
 { TARGET_FREEBSD_NR_fhstat, "fhstat", NULL, NULL, NULL },
 { TARGET_FREEBSD_NR_fhstatfs, "fhstatfs", NULL, NULL, NULL },
+{ TARGET_FREEBSD_NR_freebsd11_fhstat, "freebsd11_fhstat", NULL, NULL, NULL },
+{ TARGET_FREEBSD_NR_freebsd11_fhstatfs, "freebsd11_fhstatfs", NULL, NULL, NULL 
},
 { TARGET_FREEBSD_NR_flock, "flock", NULL, NULL, NULL },
 { TARGET_FREEBSD_NR_fork, "fork", "%s()", NULL, 

[PATCH 1/4] bsd-user: regenerate FreeBSD's system call numbers

2020-12-19 Thread imp
From: Warner Losh 

Recreate the FreeBSD system call numbers from current sys/syscall.h. Since this
isn't guaranteed to be on all systems, continue the practice of generating it
with some variation on:
sed -e s/SYS_/TARGET_NR_/ < $FREEBSD_SRC/sys/syscall.h > syscall_nr.h
until a more comprehensive system can be put in place.

Signed-off-by: Warner Losh 
---
 bsd-user/freebsd/syscall_nr.h | 695 +++---
 1 file changed, 380 insertions(+), 315 deletions(-)

diff --git a/bsd-user/freebsd/syscall_nr.h b/bsd-user/freebsd/syscall_nr.h
index d849024792..7f73a6d0f1 100644
--- a/bsd-user/freebsd/syscall_nr.h
+++ b/bsd-user/freebsd/syscall_nr.h
@@ -1,11 +1,11 @@
 /*
  * System call numbers.
  *
- * created from FreeBSD: releng/9.1/sys/kern/syscalls.master 229723
- * 2012-01-06 19:29:16Z jhb
+ * DO NOT EDIT-- this file is automatically @generated.
+ * $FreeBSD$
  */
 
-#define TARGET_FREEBSD_NR_syscall   0
+#define TARGET_FREEBSD_NR_syscall   0
 #define TARGET_FREEBSD_NR_exit  1
 #define TARGET_FREEBSD_NR_fork  2
 #define TARGET_FREEBSD_NR_read  3
@@ -13,438 +13,503 @@
 #define TARGET_FREEBSD_NR_open  5
 #define TARGET_FREEBSD_NR_close 6
 #define TARGET_FREEBSD_NR_wait4 7
-/* 8 is old creat */
+/* 8 is old creat */
 #define TARGET_FREEBSD_NR_link  9
-#define TARGET_FREEBSD_NR_unlink10
-/* 11 is obsolete execv */
+#define TARGET_FREEBSD_NR_unlink10
+/* 11 is obsolete execv */
 #define TARGET_FREEBSD_NR_chdir 12
-#define TARGET_FREEBSD_NR_fchdir13
-#define TARGET_FREEBSD_NR_mknod 14
+#define TARGET_FREEBSD_NR_fchdir13
+#define TARGET_FREEBSD_NR_freebsd11_mknod   14
 #define TARGET_FREEBSD_NR_chmod 15
 #define TARGET_FREEBSD_NR_chown 16
 #define TARGET_FREEBSD_NR_break 17
-#define TARGET_FREEBSD_NR_freebsd4_getfsstat18
-/* 19 is old lseek */
-#define TARGET_FREEBSD_NR_getpid20
+/* 18 is freebsd4 getfsstat */
+/* 19 is old lseek */
+#define TARGET_FREEBSD_NR_getpid20
 #define TARGET_FREEBSD_NR_mount 21
-#define TARGET_FREEBSD_NR_unmount   22
-#define TARGET_FREEBSD_NR_setuid23
-#define TARGET_FREEBSD_NR_getuid24
-#define TARGET_FREEBSD_NR_geteuid   25
-#define TARGET_FREEBSD_NR_ptrace26
-#define TARGET_FREEBSD_NR_recvmsg   27
-#define TARGET_FREEBSD_NR_sendmsg   28
-#define TARGET_FREEBSD_NR_recvfrom  29
-#define TARGET_FREEBSD_NR_accept30
+#define TARGET_FREEBSD_NR_unmount   22
+#define TARGET_FREEBSD_NR_setuid23
+#define TARGET_FREEBSD_NR_getuid24
+#define TARGET_FREEBSD_NR_geteuid   25
+#define TARGET_FREEBSD_NR_ptrace26
+#define TARGET_FREEBSD_NR_recvmsg   27
+#define TARGET_FREEBSD_NR_sendmsg   28
+#define TARGET_FREEBSD_NR_recvfrom  29
+#define TARGET_FREEBSD_NR_accept30
 #define TARGET_FREEBSD_NR_getpeername   31
 #define TARGET_FREEBSD_NR_getsockname   32
-#define TARGET_FREEBSD_NR_access33
-#define TARGET_FREEBSD_NR_chflags   34
-#define TARGET_FREEBSD_NR_fchflags  35
+#define TARGET_FREEBSD_NR_access33
+#define TARGET_FREEBSD_NR_chflags   34
+#define TARGET_FREEBSD_NR_fchflags  35
 #define TARGET_FREEBSD_NR_sync  36
 #define TARGET_FREEBSD_NR_kill  37
-/* 38 is old stat */
-#define TARGET_FREEBSD_NR_getppid   39
-/* 40 is old lstat */
+/* 38 is old stat */
+#define TARGET_FREEBSD_NR_getppid   39
+/* 40 is old lstat */
 #define TARGET_FREEBSD_NR_dup   41
-#define TARGET_FREEBSD_NR_pipe  42
-#define TARGET_FREEBSD_NR_getegid   43
-#define TARGET_FREEBSD_NR_profil44
-#define TARGET_FREEBSD_NR_ktrace45
-/* 46 is old sigaction */
-#define TARGET_FREEBSD_NR_getgid47
-/* 48 is old sigprocmask */
-#define TARGET_FREEBSD_NR_getlogin  49
-#define TARGET_FREEBSD_NR_setlogin  50
+#define TARGET_FREEBSD_NR_freebsd10_pipe42
+#define TARGET_FREEBSD_NR_getegid   43
+#define TARGET_FREEBSD_NR_profil44
+#define TARGET_FREEBSD_NR_ktrace45
+/* 46 is old sigaction */
+#define TARGET_FREEBSD_NR_getgid47
+/* 48 is old sigprocmask */
+#define TARGET_FREEBSD_NR_getlogin  49
+#define TARGET_FREEBSD_NR_setlogin  50
 #define TARGET_FREEBSD_NR_acct  51
-/* 52 is old sigpending */
+/* 52 is old sigpending */
 #define TARGET_FREEBSD_NR_sigaltstack   53
 #define TARGET_FREEBSD_NR_ioctl 54
-#define TARGET_FREEBSD_NR_reboot55
-#define TARGET_FREEBSD_NR_revoke56
-#define TARGET_FREEBSD_NR_symlink   57
-#define TARGET_FREEBSD_NR_readlink  58
-#define TARGET_FREEBSD_NR_execve59
+#define TARGET_FREEBSD_NR_reboot55
+#define TARGET_FREEBSD_NR_revoke56
+#define TARGET_FREEBSD_NR_symlink

[PATCH 2/4] tcg: Additional Trap type for FreeBSD

2020-12-19 Thread imp
From: Sean Bruno 

FreeBSD can generate a trap 0xc as well as 0xe when writing to a
read-only page.

Signed-off-by: Juergen Lock 
[imp rewored commit message for clarity]
Signed-off-by: Warner Losh 
---
 accel/tcg/user-exec.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 4ebe25461a..1f5befa9f9 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -343,7 +343,13 @@ int cpu_signal_handler(int host_signum, void *pinfo,
 
 pc = PC_sig(uc);
 return handle_cpu_signal(pc, info,
- TRAP_sig(uc) == 0xe ? (ERROR_sig(uc) >> 1) & 1 : 
0,
+#if defined(__FreeBSD__) || defined(__DragonFly__)
+ (TRAP_sig(uc) == 0xe ||
+  TRAP_sig(uc) == 0xc) ?
+#else
+ TRAP_sig(uc) == 0xe ?
+#endif
+ (ERROR_sig(uc) >> 1) & 1 : 0,
  _sig(uc));
 }
 
-- 
2.22.1




[PATCH 0/4] A few preliminary bsd-user patches

2020-12-19 Thread imp
From: Warner Losh 

Here's the first round of bsd-user patches. There's on the order of 280 that
we've done, but that's too much to review all at once. In addition, 3.1 release
was the last rebase point that we've been successful with for a number of 
reasons
unrelated to qemu. Now that those have been resolved, we have a new push under 
way
to push things forward, but wanted to upstream as many of the patches as we can
directly to qemu's head to lighten the load of carrying all these.

This first small series updates the system call lists, moves things around to
make it easier to support divergence in the BSD world, and adjusts to the new
meson build. It's also designed to help me learn how to land such a large set
upstream.

These patches have passed through several hands, with different tweaks over the
years so have an unusually large number of signed-off-by lines that are the
result of this refinement process where several hands have touched the patches
in the last 7 years.

Sean Bruno (1):
  tcg: Additional Trap type for FreeBSD

Stacey Son (1):
  bsd-user: move strace OS/arch dependent code to host/arch dirs

Warner Losh (2):
  bsd-user: regenerate FreeBSD's system call numbers
  bsd-user: Update strace.list for FreeBSD's latest syscalls

 accel/tcg/user-exec.c  |   8 +-
 bsd-user/arm/target_arch_sysarch.h |  78 +++
 bsd-user/arm/target_syscall.h  |  36 ++
 bsd-user/freebsd/os-strace.h   |  29 ++
 bsd-user/freebsd/strace.list   |  65 ++-
 bsd-user/freebsd/syscall_nr.h  | 695 ++---
 bsd-user/i386/target_arch_sysarch.h|  77 +++
 bsd-user/i386/target_syscall.h |  19 +
 bsd-user/mips/target_arch_sysarch.h|  69 +++
 bsd-user/mips/target_syscall.h |  52 ++
 bsd-user/mips64/target_arch_sysarch.h  |  69 +++
 bsd-user/mips64/target_syscall.h   |  53 ++
 bsd-user/netbsd/os-strace.h|   1 +
 bsd-user/openbsd/os-strace.h   |   1 +
 bsd-user/sparc/target_arch_sysarch.h   |  52 ++
 bsd-user/sparc/target_syscall.h|  24 +-
 bsd-user/sparc64/target_arch_sysarch.h |  52 ++
 bsd-user/sparc64/target_syscall.h  |  24 +-
 bsd-user/strace.c  |  11 +
 bsd-user/x86_64/target_arch_sysarch.h  |  76 +++
 bsd-user/x86_64/target_syscall.h   |  21 +-
 meson.build|   1 +
 22 files changed, 1186 insertions(+), 327 deletions(-)
 create mode 100644 bsd-user/arm/target_arch_sysarch.h
 create mode 100644 bsd-user/arm/target_syscall.h
 create mode 100644 bsd-user/freebsd/os-strace.h
 create mode 100644 bsd-user/i386/target_arch_sysarch.h
 create mode 100644 bsd-user/mips/target_arch_sysarch.h
 create mode 100644 bsd-user/mips/target_syscall.h
 create mode 100644 bsd-user/mips64/target_arch_sysarch.h
 create mode 100644 bsd-user/mips64/target_syscall.h
 create mode 100644 bsd-user/netbsd/os-strace.h
 create mode 100644 bsd-user/openbsd/os-strace.h
 create mode 100644 bsd-user/sparc/target_arch_sysarch.h
 create mode 100644 bsd-user/sparc64/target_arch_sysarch.h
 create mode 100644 bsd-user/x86_64/target_arch_sysarch.h

-- 
2.22.1




Re: [PATCH] acpi/gpex: Inform os to keep firmware resource map

2020-12-19 Thread Michael S. Tsirkin
On Fri, Dec 18, 2020 at 01:56:29PM +0800, Jiahui Cen wrote:
> Hi Michael,
> 
> On 2020/12/18 4:04, Michael S. Tsirkin wrote:
> > On Thu, Dec 17, 2020 at 09:29:26PM +0800, Jiahui Cen wrote:
> >> There may be some differences in pci resource assignment between guest os
> >> and firmware.
> >>
> >> Eg. A Bridge with Bus [d2]
> >> -+-[:d2]---01.0-[d3]01.0
> >>
> >> where [d2:01.00] is a pcie-pci-bridge with BAR0 (mem, 64-bit, 
> >> non-pref) [size=256]
> >>   [d3:01.00] is a PCI Device with BAR0 (mem, 64-bit, pref) 
> >> [size=128K]
> >>   BAR4 (mem, 64-bit, pref) 
> >> [size=64M]
> >>
> >> In EDK2, the Resource Map would be:
> >> PciBus: Resource Map for Bridge [D2|01|00]
> >> Type = PMem64; Base = 0x800400; Length = 0x410; 
> >> Alignment = 0x3FF
> >>Base = 0x800400; Length = 0x400; Alignment = 
> >> 0x3FF;  Owner = PCI [D3|01|00:20]
> >>Base = 0x800800; Length = 0x2;   Alignment = 
> >> 0x1;Owner = PCI [D3|01|00:10]
> >> Type =  Mem64; Base = 0x800810; Length = 0x100; Alignment 
> >> = 0xFFF
> >>
> >> While in Linux, kernel will use 0x2FF as the alignment to calculate
> >> the PMem64 size, which would be 0x600.
> >>
> >> The diffences could result in resource assignment failure.
> >>
> >> Using _DSM #5 method to inform guest os not to ignore the PCI configuration
> >> that firmware has done at boot time could handle the differences.
> >>
> >> Signed-off-by: Jiahui Cen 
> >> ---
> >>  hw/pci-host/gpex-acpi.c | 11 ++-
> >>  1 file changed, 10 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
> >> index 071aa11b5c..2b490f3379 100644
> >> --- a/hw/pci-host/gpex-acpi.c
> >> +++ b/hw/pci-host/gpex-acpi.c
> >> @@ -112,10 +112,19 @@ static void acpi_dsdt_add_pci_osc(Aml *dev)
> >>  UUID = aml_touuid("E5C937D0-3553-4D7A-9117-EA4D19C3434D");
> >>  ifctx = aml_if(aml_equal(aml_arg(0), UUID));
> >>  ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0)));
> >> -uint8_t byte_list[1] = {1};
> >> +uint8_t byte_list[1] = {0x21};
> >>  buf = aml_buffer(1, byte_list);
> > 
> > 
> > Hmm what is this change for?
> > 
> > I also noticed something weird.
> > Spec seems to say for _DSM for PCI Express Slot Information:
> > 
> > 
> > Arguments:
> > Arg0: UUID: E5C937D0-3553-4d7a-9117-EA4D19C3434D
> > Arg1: Revision ID: 2
> > Arg2: Function Index: 1
> > Arg3: Empty Package
> > 
> > 
> > how come we are comparing function index to 0 here?
> > 
> 
> PCI Firmware Spec says in 4.6.1. _DSM for PCI Express Slot Information
> 
> Note: Function 0 is a generic Query function that is supported by _DSMs with 
> any UUID and
> Revision ID. The definition of function 0 is generic to _DSM and specified in 
> the ACPI Specification,
> Version 3.0 (or later).
> 
> 
> And ACPI Spec says in 9.1.1 _DSM (Device Specific Method)
> 
> Return Value Information:
> If Function Index is zero, the return is a buffer containing one bit for each 
> function index, starting with zero. Bit 0
> indicates whether there is support for any functions other than function 0 
> for the specified UUID and Revision ID.
> If set to zero, no functions are supported (other than function zero) for the 
> specified UUID and Revision ID. If set
> to one, at least one additional function is supported. For all other bits in 
> the buffer, a bit is set to zero to indicate if
> that function index is not supported for the specific UUID and Revision ID. 
> (For example, bit 1 set to 0 indicates that
> function index 1 is not supported for the specific UUID and Revision ID.)
> 
> 
> I have no idea whether the original code does aim to use _DSM #0
> by setting function index 0 (The return value seems not to be suitable
> with _DSM #1). But if it does, I think it is necessary to set bit 5
> in return value to indicate _DSM #5 function is supported.



So let's make it self documenting:

{
0x1 << 0 /* support for support for any functions other than function 0 
*/ |
0x1 << 5 /* support for function 5 */
}



> > 
> > Also, as long as we are changing this probably shouldn't hard-code
> > 1 as array size ...
> > 
> 
> Is a macro enough? Like #define RET_BUF_SIZE 2

Better to use 

uint8_t byte_list[] = { ... };

And then ARRAY_SIZE(byte_list)


> > 
> >>  aml_append(ifctx1, aml_return(buf));
> >>  aml_append(ifctx, ifctx1);
> >> +
> >> +/* PCI Firmware Specification 3.2
> >> + * 4.6.5. _DSM for Ignoring PCI Boot Configurations
> > 
> > Note you must always quote the most recent spec that
> > your change refers to. This is so people can figure out
> > legacy guest compatibility.
> > 
> > In this case I think this first appeard in 3.1 not 3.2
> > 
> 
> OK, I'll fix this.
> 
> >> + * The UUID in _DSM in this context is
> >> + * {E5C937D0-3553-4D7A-9117-EA4D19C3434D}
> > 
> > This 

Re: [PATCH v2 8/8] tests/acceptance: Test boot_linux_console for fuloong2e

2020-12-19 Thread Philippe Mathieu-Daudé
On 12/19/20 8:23 AM, Jiaxun Yang wrote:
> The kernel comes from debian archive so it's trusted.
> 
> Signed-off-by: Jiaxun Yang 
> ---
>  tests/acceptance/boot_linux_console.py | 21 +
>  1 file changed, 21 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 



Re: [PATCH v2 0/8] MIPS Bootloader helper

2020-12-19 Thread Philippe Mathieu-Daudé
On 12/15/20 7:41 AM, Jiaxun Yang wrote:
> v2:
> A big reconstruction. rewrite helpers with CPU feature and sepreate
> changesets.
> 
> Jiaxun Yang (8):
>   hw/mips: Make bootloader addresses unsgined
>   hw/mips/malta: Use address translation helper to calculate
> bootloader_run_addr
>   hw/mips: Use address translation helper to handle ENVP_ADDR
>   hw/mips: Add a bootloader helper
>   hw/mips: Use bl_gen_kernel_jump to generate bootloaders
>   target/mips/addr: Add translation helpers for KSEG1
>   hw/mips/malta: Use bootloader helper to set BAR resgiters
>   hw/mips/boston: Use bootloader helper to set GCRs
> 
>  hw/mips/bootloader.c | 157 
>  hw/mips/boston.c |  62 +++--
>  hw/mips/fuloong2e.c  |  48 +++---
>  hw/mips/malta.c  | 171 ---
>  hw/mips/meson.build  |   2 +-
>  include/hw/mips/bootloader.h |  48 ++
>  target/mips/addr.c   |  10 ++
>  target/mips/cpu.h|   2 +
>  8 files changed, 306 insertions(+), 194 deletions(-)
>  create mode 100644 hw/mips/bootloader.c
>  create mode 100644 include/hw/mips/bootloader.h
> 

Tested-by: Philippe Mathieu-Daudé 



[PATCH 4/4] fuzz: delay IO until they can't trigger the crash

2020-12-19 Thread Qiuhao Li
Since programmers usually trigger an IO just before they need it. Try to
delay some IO instructions may help us better understanding the timing
context when debug.

Tested with Bug 1908062. Refined vs. Original result:

outl 0xcf8 0x881coutl 0xcf8 0x0
outb 0xcfc 0xc3| outl 0xcf8 0x881c
outl 0xcf8 0x8804  | outb 0xcfc 0xc3
outl 0xcfc 0x1006  | outl 0xcf8 0x8804
write 0xc31028 0x1 0x5a| outl 0xcfc 0x1006
write 0xc31024 0x2 0x10| write 0xc31028 0x1 0x5a
write 0xc3101c 0x1 0x01| writel 0xc3100c 0x2a6f6c63
write 0xc33002 0x1 0x0 v write 0xc31024 0x2 0x10
write 0x5c 0x1 0x10  write 0xc3101c 0x1 0x01
writel 0xc3100c 0x2a6f6c63   write 0xc31018 0x1 0x80
write 0xc31018 0x1 0x80  write 0x5c 0x1 0x10
outl 0xcf8 0x0   write 0xc33002 0x1 0x0

Signed-off-by: Qiuhao Li 
---
 scripts/oss-fuzz/minimize_qtest_trace.py | 21 +
 1 file changed, 21 insertions(+)

diff --git a/scripts/oss-fuzz/minimize_qtest_trace.py 
b/scripts/oss-fuzz/minimize_qtest_trace.py
index f3e88064c4..da7aa73b3c 100755
--- a/scripts/oss-fuzz/minimize_qtest_trace.py
+++ b/scripts/oss-fuzz/minimize_qtest_trace.py
@@ -214,6 +214,27 @@ def minimize_trace(inpath, outpath):
 
 assert(check_if_trace_crashes(newtrace, outpath))
 
+# delay IO instructions until they can't trigger the crash
+# Note: O(n^2) and many timeouts, kinda slow
+i = len(newtrace) - 1
+while i >= 0:
+tmp_i = newtrace[i]
+if len(tmp_i) < 2:
+i -= 1
+continue
+print("Delaying ", newtrace[i])
+for j in reversed(range(i+1, len(newtrace)+1)):
+newtrace.insert(j, tmp_i)
+del newtrace[i]
+if check_if_trace_crashes(newtrace, outpath):
+break
+newtrace.insert(i, tmp_i)
+del newtrace[j]
+i -= 1
+
+assert(check_if_trace_crashes(newtrace, outpath))
+# maybe another removing round
+
 
 if __name__ == '__main__':
 if len(sys.argv) < 3:
-- 
2.25.1




Re: [PATCH v2 7/8] hw/mips/fuloong2e: Add highmem support

2020-12-19 Thread Philippe Mathieu-Daudé
On 12/19/20 8:23 AM, Jiaxun Yang wrote:
> highmem started from 0x2000.
> Now we can have up to 2G RAM.
> 
> Signed-off-by: Jiaxun Yang 
> ---
> v2: Handle SPD for dual DIMM correctly.
> ---
>  hw/mips/fuloong2e.c | 61 -
>  1 file changed, 49 insertions(+), 12 deletions(-)

Tested-by: Philippe Mathieu-Daudé 



[PATCH 2/4] fuzz: split QTest writes from the rightmost byte

2020-12-19 Thread Qiuhao Li
Currently, we split the write commands' data from the middle. If it does not
work, try to move the pivot "left" and retry until there is no space left.
But, this is complete for ram writes but not for IO writes.

For example, there is an IO write command:

  write addr uuuu

u is the unnecessary byte for the crash. Unlike ram write commands, in most
case, a split IO write won't trigger the same crash, So if we split from the
middle, we will get:

  write addr uu (will be removed in next round)
  write addr uu

For uu, since split it from the middle and retry to the leftmost byte
won't get the same crash, we will be stopped from removing the last two
bytes.

Therefore, we should split QTest writes from the rightmost byte.

Tested with Bug 1908062. Refined vs. Original result:

outl 0xcf8 0x881coutl 0xcf8 0x881c
outb 0xcfc 0xc3  outb 0xcfc 0xc3
outl 0xcf8 0x882foutl 0xcf8 0x882f
outl 0xcf8 0x8804outl 0xcf8 0x8804
outl 0xcfc 0x9b2765beoutl 0xcfc 0x9b2765be
write 0xc31024 0x2 0x0055write 0xc31024 0x2 0x0055
write 0xc31028 0x1 0x5a  write 0xc31028 0x1 0x5a
write 0xc3101c 0x1 0x01  write 0xc3101c 0x1 0x01
writel 0xc3100c 0x2a6f6c63   writel 0xc3100c 0x2a6f6c63
write 0xc31018 0x1 0xa4  <-- write 0xc31016 0x3 0xa4a4a4
write 0x5c 0x1 0x19  write 0x5c 0x1 0x19
write 0xc33002 0x1 0x8a  write 0xc33002 0x1 0x8a

Signed-off-by: Qiuhao Li 
---
 scripts/oss-fuzz/minimize_qtest_trace.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/oss-fuzz/minimize_qtest_trace.py 
b/scripts/oss-fuzz/minimize_qtest_trace.py
index d3b09e6567..855c3bcb54 100755
--- a/scripts/oss-fuzz/minimize_qtest_trace.py
+++ b/scripts/oss-fuzz/minimize_qtest_trace.py
@@ -140,7 +140,7 @@ def minimize_trace(inpath, outpath):
 
 # 3.) If it is a qtest write command: write addr len data, try to split
 # it into two separate write commands. If splitting the write down the
-# middle does not work, try to move the pivot "left" and retry, until
+# rightmost does not work, try to move the pivot "left" and retry, 
until
 # there is no space left. The idea is to prune unneccessary bytes from
 # long writes, while accommodating arbitrary MemoryRegion access sizes
 # and alignments.
@@ -149,7 +149,7 @@ def minimize_trace(inpath, outpath):
 length = int(newtrace[i].split()[2], 16)
 data = newtrace[i].split()[3][2:]
 if length > 1:
-leftlength = int(length/2)
+leftlength = length - 1
 rightlength = length - leftlength
 newtrace.insert(i+1, "")
 while leftlength > 0:
-- 
2.25.1




[PATCH 3/4] fuzz: setting bits in operand of out/write to zero

2020-12-19 Thread Qiuhao Li
Simplifying the crash cases by opportunistically setting bits in
operands of
out/write to zero may help to debug, since usually bit one means turn on
or
trigger a function while zero is the default turn-off setting.

Tested Bug 1908062. Refined vs. Original result:

outl 0xcf8 0x881coutl 0xcf8 0x881c
outb 0xcfc 0xc3  outb 0xcfc 0xc3
outl 0xcf8 0x0   <-- outl 0xcf8 0x882f
outl 0xcf8 0x8804outl 0xcf8 0x8804
outl 0xcfc 0x1006<-- outl 0xcfc 0x9b2765be
write 0xc31024 0x2 0x10  <-- write 0xc31024 0x2 0x0055
write 0xc31028 0x1 0x5a  write 0xc31028 0x1 0x5a
write 0xc3101c 0x1 0x01  write 0xc3101c 0x1 0x01
writel 0xc3100c 0x2a6f6c63   writel 0xc3100c 0x2a6f6c63
write 0xc31018 0x1 0x80  <-- write 0xc31018 0x1 0xa4
write 0x5c 0x1 0x10  <-- write 0x5c 0x1 0x19
write 0xc33002 0x1 0x0   <-- write 0xc33002 0x1 0x8a

Signed-off-by: Qiuhao Li 
---
 scripts/oss-fuzz/minimize_qtest_trace.py | 42 +++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/scripts/oss-fuzz/minimize_qtest_trace.py 
b/scripts/oss-fuzz/minimize_qtest_trace.py
index 855c3bcb54..f3e88064c4 100755
--- a/scripts/oss-fuzz/minimize_qtest_trace.py
+++ b/scripts/oss-fuzz/minimize_qtest_trace.py
@@ -172,7 +172,47 @@ def minimize_trace(inpath, outpath):
 newtrace[i] = prior
 del newtrace[i+1]
 i += 1
-check_if_trace_crashes(newtrace, outpath)
+
+assert(check_if_trace_crashes(newtrace, outpath))
+
+TIMEOUT = (end-start)*2 # input is short now
+
+# try setting bits in operands of out/write to zero
+i = 0
+while i < len(newtrace):
+if (not newtrace[i].startswith("write ") and not
+   newtrace[i].startswith("out")):
+   i += 1
+   continue
+# write ADDR SIZE DATA
+# outx ADDR VALUE
+print("\nzero setting bits: {}".format(newtrace[i]))
+
+prefix = " ".join(newtrace[i].split()[:-1])
+data = newtrace[i].split()[-1]
+data_bin = bin(int(data, 16))
+data_bin_list = list(data_bin)
+
+for j in range(2, len(data_bin_list)):
+prior = newtrace[i]
+if (data_bin_list[j] == '1'):
+data_bin_list[j] = '0'
+data_try = hex(int("".join(data_bin_list), 2))
+# It seems qtest only accect hex with one byte zero padding
+if len(data_try) % 2 == 1:
+data_try = data_try[:2] + "0" + data_try[2:-1]
+
+newtrace[i] = "{prefix} {data_try}\n".format(
+prefix=prefix,
+data_try=data_try)
+
+if not check_if_trace_crashes(newtrace, outpath):
+data_bin_list[j] = '1'
+newtrace[i] = prior
+
+i += 1
+
+assert(check_if_trace_crashes(newtrace, outpath))
 
 
 if __name__ == '__main__':
-- 
2.25.1




[PATCH 1/4] fuzz: refine crash detection mechanism

2020-12-19 Thread Qiuhao Li
The original crash detection method is to fork a process to test our new
trace input. If the child process exits in time and the second-to-last line
is the same as the first crash, we think it is a crash triggered by the same
bug. However, in some situations, it doesn't work since it is a
hardcoded-offset string comparison.

For example, suppose an assertion failure makes the crash. In that case, the
second-to-last line will be 'timeout: the monitored command dumped core',
which doesn't contain any information about the assertion failure like where
it happened or the assertion statement. This may lead to a minimized input
triggers assertion failure but may indicate another bug. As for some
sanitizers' crashes, the direct string comparison may stop us from getting a
smaller input, since they may have a different leaf stack frame.

Perhaps we can detect crashes using both precise output string comparison
and rough pattern string match and info the user when the trace input
triggers different but a seminar output.

Tested:
Assertion failure, https://bugs.launchpad.net/qemu/+bug/1908062
AddressSanitizer, https://bugs.launchpad.net/qemu/+bug/1907497
Trace input that doesn't crash
Trace input that crashes Qtest

Signed-off-by: Qiuhao Li 
---
 scripts/oss-fuzz/minimize_qtest_trace.py | 59 ++--
 1 file changed, 46 insertions(+), 13 deletions(-)

diff --git a/scripts/oss-fuzz/minimize_qtest_trace.py 
b/scripts/oss-fuzz/minimize_qtest_trace.py
index 5e405a0d5f..d3b09e6567 100755
--- a/scripts/oss-fuzz/minimize_qtest_trace.py
+++ b/scripts/oss-fuzz/minimize_qtest_trace.py
@@ -10,11 +10,16 @@ import os
 import subprocess
 import time
 import struct
+import re
 
 QEMU_ARGS = None
 QEMU_PATH = None
 TIMEOUT = 5
-CRASH_TOKEN = None
+
+crash_patterns = ("Assertion.+failed",
+  "SUMMARY.+Sanitizer")
+crash_pattern = None
+crash_string = None
 
 write_suffix_lookup = {"b": (1, "B"),
"w": (2, "H"),
@@ -24,13 +29,12 @@ write_suffix_lookup = {"b": (1, "B"),
 def usage():
 sys.exit("""\
 Usage: QEMU_PATH="/path/to/qemu" QEMU_ARGS="args" {} input_trace output_trace
-By default, will try to use the second-to-last line in the output to identify
-whether the crash occred. Optionally, manually set a string that idenitifes the
-crash by setting CRASH_TOKEN=
+By default, we will try to search predefined crash patterns through the
+tracing output to see whether the crash occred. Optionally, manually set a
+string that idenitifes the crash by setting CRASH_PATTERN=
 """.format((sys.argv[0])))
 
 def check_if_trace_crashes(trace, path):
-global CRASH_TOKEN
 with open(path, "w") as tracefile:
 tracefile.write("".join(trace))
 
@@ -42,17 +46,47 @@ def check_if_trace_crashes(trace, path):
   shell=True,
   stdin=subprocess.PIPE,
   stdout=subprocess.PIPE)
+if rc.returncode == 137:# Timed Out
+return False
+
 stdo = rc.communicate()[0]
 output = stdo.decode('unicode_escape')
-if rc.returncode == 137:# Timed Out
-return False
-if len(output.splitlines()) < 2:
+output_lines = output.splitlines()
+# Usually we care about the summary info in the last few lines, reverse.
+output_lines.reverse()
+
+global crash_pattern, crash_patterns, crash_string
+if crash_pattern is None: # Initialization
+for line in output_lines:
+for c in crash_patterns:
+if re.search(c, line) is not None:
+crash_pattern = c
+crash_string = line
+print("Identifying crash pattern by this string: ",\
+  crash_string)
+print("Using regex pattern: ", crash_pattern)
+return True
+print("Failed to initialize crash pattern: no match.")
 return False
 
-if CRASH_TOKEN is None:
-CRASH_TOKEN = output.splitlines()[-2]
+# First, we search exactly the previous crash string.
+for line in output_lines:
+if crash_string == line:
+return True
+
+# Then we decide whether a similar (same pattern) crash happened.
+# Slower now :(
+for line in output_lines:
+if re.search(crash_pattern, line) is not None:
+print("\nINFO: The crash string changed during our minimization 
process.")
+print("Before: ", crash_string)
+print("After: ", line)
+print("The original regex pattern can still match, updated the 
crash string.")
+crash_string = line
+return True
 
-return CRASH_TOKEN in output
+# The input did not trigger (the same type) bug.
+return False
 
 
 def minimize_trace(inpath, outpath):
@@ -66,7 +100,6 @@ def minimize_trace(inpath, outpath):
 print("Crashed in {} seconds".format(end-start))
 TIMEOUT = (end-start)*5
 print("Setting the timeout for {} 

[PATCH 0/4] improve crash case minimization

2020-12-19 Thread Qiuhao Li
Extend and refine the crash case minimization process.

I forgot to cc some reviewers in the last patch, so I merge it as the
first on in this patch series.

Qiuhao Li (4):
  fuzz: refine crash detection mechanism
  fuzz: split QTest writes from the rightmost byte
  fuzz: setting bits in operand of out/write to zero
  fuzz: delay IO until they can't trigger the crash

 scripts/oss-fuzz/minimize_qtest_trace.py | 126 ---
 1 file changed, 110 insertions(+), 16 deletions(-)

-- 
2.25.1




[Bug 1906193] Re: riscv32 user mode emulation: fork return values broken

2020-12-19 Thread Alistair Francis
I have sent a patch, you can see it here:
https://patchwork.ozlabs.org/project/qemu-devel/list/?series=221381

It seems like QEMU's waitid implementation has a bug with handling the
status.

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

Title:
  riscv32 user mode emulation: fork return values broken

Status in QEMU:
  New

Bug description:
  When running in a chroot with riscv32 (on x86_64; qemu git master as
  of today):

  The following short program forks; the child immediately returns with
  exit(42). The parent checks for the return value - and obtains 40!

  gcc-10.2

  ===
  #include 
  #include 
  #include 
  #include 

  main(c, v)
   int c;
   char **v;
  {
pid_t pid, p;
int s, i, n;

s = 0;
pid = fork();
if (pid == 0)
  exit(42);

/* wait for the process */
p = wait();
if (p != pid)
  exit (255);

if (WIFEXITED(s))
{
   int r=WEXITSTATUS(s);
   if (r!=42) {
printf("child wants to return %i (0x%X), parent received %i (0x%X), 
difference %i\n",42,42,r,r,r-42);
   }
}
  }
  ===

  (riscv-ilp32 chroot) farino /tmp # ./wait-test-short 
  child wants to return 42 (0x2A), parent received 40 (0x28), difference -2

  ===
  (riscv-ilp32 chroot) farino /tmp # gcc --version
  gcc (Gentoo 10.2.0-r1 p2) 10.2.0
  Copyright (C) 2020 Free Software Foundation, Inc.
  Dies ist freie Software; die Kopierbedingungen stehen in den Quellen. Es
  gibt KEINE Garantie; auch nicht für MARKTGÄNGIGKEIT oder FÜR SPEZIELLE ZWECKE.

  (riscv-ilp32 chroot) farino /tmp # ld --version
  GNU ld (Gentoo 2.34 p6) 2.34.0
  Copyright (C) 2020 Free Software Foundation, Inc.
  This program is free software; you may redistribute it under the terms of
  the GNU General Public License version 3 or (at your option) a later version.
  This program has absolutely no warranty.

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



Re: [PATCH v2 6/8] hw/mips/fuloong2e: Correct cpuclock env

2020-12-19 Thread Philippe Mathieu-Daudé
On 12/19/20 8:21 AM, Jiaxun Yang wrote:
> It was missed in 3ca7639ff00 ("hw/mips/fuloong2e:
> Set CPU frequency to 533 MHz"), we need to tell kernel
> correct clocks.
> 
> Fixes: 3ca7639ff00 ("hw/mips/fuloong2e: Set CPU frequency to 533 MHz").
> Signed-off-by: Jiaxun Yang 
> ---
>  hw/mips/fuloong2e.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé 



[PATCH v1 1/1] linux-user/signal: Decode waitid si_code

2020-12-19 Thread Alistair Francis
When mapping the host waitid status to the target status we previously
just used decoding information in the status value. This doesn't follow
what the waitid documentation describes, which instead suggests using
the si_code value for the decoding. This results in the incorrect values
seen when calling waitid. This is especially apparent on RV32 where all
wait calls use waitid (see the bug case).

This patch uses the si_code value to map the waitid status.

Buglink: https://bugs.launchpad.net/qemu/+bug/1906193
Signed-off-by: Alistair Francis 
---
 linux-user/signal.c | 26 --
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index 73de934c65..b6c9326521 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -305,6 +305,7 @@ static inline void 
host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
 int sig = host_to_target_signal(info->si_signo);
 int si_code = info->si_code;
 int si_type;
+int status = info->si_status;
 tinfo->si_signo = sig;
 tinfo->si_errno = 0;
 tinfo->si_code = info->si_code;
@@ -349,8 +350,29 @@ static inline void 
host_to_target_siginfo_noswap(target_siginfo_t *tinfo,
 case TARGET_SIGCHLD:
 tinfo->_sifields._sigchld._pid = info->si_pid;
 tinfo->_sifields._sigchld._uid = info->si_uid;
-tinfo->_sifields._sigchld._status
-= host_to_target_waitstatus(info->si_status);
+
+/*
+ * Map host to target signal numbers for the waitid family of
+ * syscalls. This is similar to the functionality in
+ * host_to_target_waitstatus() except we use the si_code to
+ * determine the operation.
+ */
+switch (info->si_code) {
+case CLD_KILLED:
+case CLD_DUMPED:
+tinfo->_sifields._sigchld._status =
+host_to_target_signal(WTERMSIG(status)) |
+  (status & ~0x7f);
+break;
+case CLD_STOPPED:
+tinfo->_sifields._sigchld._status =
+(host_to_target_signal(WSTOPSIG(status)) << 8) |
+(status & 0xff);
+break;
+default:
+tinfo->_sifields._sigchld._status = status;
+}
+
 tinfo->_sifields._sigchld._utime = info->si_utime;
 tinfo->_sifields._sigchld._stime = info->si_stime;
 si_type = QEMU_SI_CHLD;
-- 
2.29.2




Re: [PATCH v2 2/8] hw/mips/fuloong2e: Relpace fault links

2020-12-19 Thread Philippe Mathieu-Daudé
On 12/19/20 8:12 AM, Jiaxun Yang wrote:
> Websites are downing, but GitHub may last forever.
> Loongson even doesn't recogonize 2E as their products nowadays..
> 
> Signed-off-by: Jiaxun Yang 
> ---
>  hw/mips/fuloong2e.c | 13 +++--
>  1 file changed, 3 insertions(+), 10 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 



Re: [PATCH] linux-user: Fix loading of BSS segments

2020-12-19 Thread Giuseppe Musacchio
On 19/12/20 11:55, Laurent Vivier wrote:
> Le 17/12/2020 à 11:17, Giuseppe Musacchio a écrit :
>> Some ELF binaries encode the .bss section as an extension of the data
>> ones by setting the segment p_memsz > p_filesz. Some other binaries take
>> a different route and encode it as a stand-alone PT_LOAD segment with
>> p_filesz = 0 and p_memsz > 0.
>>
>> Both the encodings are actually correct per ELF specification but the
>> ELF loader had some troubles in handling the former: with the old logic
>> it was very likely to get Qemu to crash in zero_bss when trying to
>> access unmapped memory.
>>
>> zero_bss isn't meant to allocate whole zero-filled segments but to
>> "complete" a previously mapped segment with the needed zero bits.
>>
>> The fix is pretty simple, if the segment is completely zero-filled we
>> simply allocate one or more pages (according to p_memsz) and avoid
>> calling zero_bss altogether.
> 
> So, the current code manages the bss segment when the memory page has already
> been allocated for the data segment by zeroing it:
> 
> +--+
>  PAGE  |
>  --++  |
>  DATA  |   BSS  |  |
> 
> So your patch fixes the case when there is no data segment and thus no page
> to complete:
> 
> +--+
>  PAGE  |
>  --+   |
>  BSS   |   |
> 
> 
> But could we have a case where the BSS starts in a page allocated for the
> data segment but needs more pages?
> 
> +--+--+
>  PAGE  | PAGE |
>  ++   |
>  DATA| BSS|   |
> 
> In this case we should also allocate the page, and the previous case is only a
> special case (data segment = 0) of this one.
> 

If I correctly understand your example here this case should be already
handled by zero_bss: the first chunk of .bss is mapped as part of the
vaddr_len mapping and is zeroed by the memset, while the other chunk/page
is mapped in the `host_map_start < host_end` clause.

> so something like (approxymately):
> 
> if (eppnt->p_filesz != 0) {
>target_mmap()
>if (vaddr_ef < vaddr_mem) {
>zero_bss(vaddr_ef, MIN(vaddr_mem, vaddr_ps + vaddr_len))
>}
> }
> if (vaddr_ps + vaddr_len < vaddr_mem) {
>   target_mmap(vaddr_ps + vaddr_len, vaddr_ps + vaddr_len - vaddr_mem - 1,
>   elf_prot, MAP_PRIVATE | MAP_FIXED | MAP_ANONYMOUS, -1, 0);
> }
> 
> I think your fix is correct, but I'm wondering if we can have something more
> generic, if we can cover an other possible case.
> 
> If you think we don't need to introduce more complexity for a case that can't
> happen I will queue your patch as is.
> 
> Thanks,
> Laurent
> 
>> Signed-off-by: Giuseppe Musacchio 
>> ---
>>  linux-user/elfload.c | 30 --
>>  1 file changed, 20 insertions(+), 10 deletions(-)
>>
>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
>> index 0b02a92602..a16c240e0f 100644
>> --- a/linux-user/elfload.c
>> +++ b/linux-user/elfload.c
>> @@ -2776,14 +2776,16 @@ static void load_elf_image(const char *image_name, 
>> int image_fd,
>>  vaddr = load_bias + eppnt->p_vaddr;
>>  vaddr_po = TARGET_ELF_PAGEOFFSET(vaddr);
>>  vaddr_ps = TARGET_ELF_PAGESTART(vaddr);
>> -vaddr_len = TARGET_ELF_PAGELENGTH(eppnt->p_filesz + vaddr_po);
>> +
>> +vaddr_ef = vaddr + eppnt->p_filesz;
>> +vaddr_em = vaddr + eppnt->p_memsz;
>>  
>>  /*
>> - * Some segments may be completely empty without any backing 
>> file
>> - * segment, in that case just let zero_bss allocate an empty 
>> buffer
>> - * for it.
>> + * Some segments may be completely empty, with a non-zero 
>> p_memsz
>> + * but no backing file segment.
>>   */
>>  if (eppnt->p_filesz != 0) {
>> +vaddr_len = TARGET_ELF_PAGELENGTH(eppnt->p_filesz + 
>> vaddr_po);
>>  error = target_mmap(vaddr_ps, vaddr_len, elf_prot,
>>  MAP_PRIVATE | MAP_FIXED,
>>  image_fd, eppnt->p_offset - vaddr_po);
>> @@ -2791,14 +2793,22 @@ static void load_elf_image(const char *image_name, 
>> int image_fd,
>>  if (error == -1) {
>>  goto exit_mmap;
>>  }
>> -}
>>  
>> -vaddr_ef = vaddr + eppnt->p_filesz;
>> -vaddr_em = vaddr + eppnt->p_memsz;
>> +/*
>> + * If the load segment requests extra zeros (e.g. bss), map 
>> it.
>> + */
>> +if (eppnt->p_filesz < eppnt->p_memsz) {
>> + 

Re: [PATCH v2 0/8] hm/mips/fuloong2e fixes

2020-12-19 Thread BALATON Zoltan via

On Sat, 19 Dec 2020, Jiaxun Yang wrote:

在2020年12月19日十二月 下午8:13,BALATON Zoltan写道:

On Sat, 19 Dec 2020, Jiaxun Yang wrote:

It can now boot Debian installer[1] as well as a custom PMON bootloader
distribution[2].

Note that it can't boot PMON shipped with actual machine as our ATI vgabios
is using some x86 hack that can't be handled by x86emu in original PMON.


This may be similar problem that I've seen with similar PPC firmwares:

https://osdn.net/projects/qmiga/wiki/SubprojectAti
https://osdn.net/projects/qmiga/wiki/SubprojectPegasos2#h3-Known.20issues

TLDR; vgabios-ati.bin is compiled for i386 real mode (bacause that's what
gcc can do, real x86 real mode would need something like bcc I think) that
some x86emu can't handle. You can either use Bochs vga bios via romfile
property of the vga emulation or try the option for x86emu when compiling
vgabios-ati.bin (which did not help the firmwares I've tried).


Hi,

Thinks for your reminder!

To be more specified, our x86emu in PMON can handle i386 real mode,
however vgabios-ati uses INT15h when INT10h ax=0x4f01 (Get VESA Mode)
is called. And x86emu won't process INT15h properly.

My workround[1] is to allow 0x4f01 to be failed in PMON, as ax=0x4f02
(Set VESA Mode) do work, it won't be a actual problem.


Adding Gerd who is the vgabios maintainer and added the VESA mode support 
so he knows about this even if no fix is needed but maybe he knows a 
simple way to work around it anyway. Maybe this could be related to my 
problems too but with the sam460ex firmware I did not get any output, it 
just stops (did not check u-boot-sam460ex source yet), but with the 
pegasos2 firmware I got this diagnostics:


INTERNAL ERROR: 000E=UNIMPLEMENTED EXTENDED OPCODE

EAX= EBX= ECX= EDX= ESP= EBP= ESI= EDI=
 AX=0055  BX=FFDA  CX=  DX=  SP=FF86  BP=FF9A  SI=  DI=FFA6
 DS=1000  ES=BAD0  SS=1000  CS=C000  IP=2E05   NV UP -- PL NZ NA PE NC
CS:IP = 0F
STACK:     FFDA   
  00: FE00 F000 FE01 F000 FE02 F000 FE03 F000
  10: FE04 F000 FE05 F000 FE06 F000 FE07 F000
  20: FE08 F000 FE09 F000 FE0A F000 FE0B F000
  30: FE0C F000 FE0D F000 FE0E F000 FE0F F000
  40: FE10 F000 FE11 F000 FE12 F000 FE13 F000
  50: FE14 F000 FE15 F000 FE16 F000 FE17 F000
  60: FE18 F000 FE19 F000 FE1A F000 FE1B F000
  70: FE1C F000 FE1D F000 FE1E F000 FE1F F000
  80: FE20 F000 FE21 F000 FE22 F000 FE23 F000
  90: FE24 F000 FE25 F000 FE26 F000 FE27 F000
  A0: FE28 F000 FE29 F000 FE2A F000 FE2B F000
  B0: FE2C F000 FE2D F000 FE2E F000 FE2F F000
  C0: FE30 F000 FE31 F000 FE32 F000 FE33 F000
  D0: FE34 F000 FE35 F000 FE36 F000 FE37 F000
  E0: FE38 F000 FE39 F000 FE3A F000 FE3B F000
  F0: FE3C F000 FE3D F000 FE3E F000 FE3F F000
  00: FE40 F000 FE41 F000 FE42 F000 FE43 F000
  10: FE44 F000 FE45 F000 FE46 F000 FE47 F000
  20: FE48 F000 FE49 F000 FE4A F000 FE4B F000
  30: FE4C F000 FE4D F000 FE4E F000 FE4F F000
MISC: UNHANDLED 32 BIT DATA PREFIX AT CS:IP = C000:2E04 0F
INTERNAL ERROR: 000A=UNHANDLED 32BIT PREFIX

EAX= EBX= ECX= EDX= ESP= EBP= ESI= EDI=
 AX=0055  BX=FFDA  CX=  DX=  SP=FF86  BP=FF9A  SI=  DI=FFA6
 DS=1000  ES=BAD0  SS=1000  CS=C000  IP=2E05   NV UP -- PL NZ NA PE NC
CS:IP = 0F
STACK:     FFDA   
  00: FE00 F000 FE01 F000 FE02 F000 FE03 F000
  10: FE04 F000 FE05 F000 FE06 F000 FE07 F000
  20: FE08 F000 FE09 F000 FE0A F000 FE0B F000
  30: FE0C F000 FE0D F000 FE0E F000 FE0F F000
  40: FE10 F000 FE11 F000 FE12 F000 FE13 F000
  50: FE14 F000 FE15 F000 FE16 F000 FE17 F000
  60: FE18 F000 FE19 F000 FE1A F000 FE1B F000
  70: FE1C F000 FE1D F000 FE1E F000 FE1F F000
  80: FE20 F000 FE21 F000 FE22 F000 FE23 F000
  90: FE24 F000 FE25 F000 FE26 F000 FE27 F000
  A0: FE28 F000 FE29 F000 FE2A F000 FE2B F000
  B0: FE2C F000 FE2D F000 FE2E F000 FE2F F000
  C0: FE30 F000 FE31 F000 FE32 F000 FE33 F000
  D0: FE34 F000 FE35 F000 FE36 F000 FE37 F000
  E0: FE38 F000 FE39 F000 FE3A F000 FE3B F000
  F0: FE3C F000 FE3D F000 FE3E F000 FE3F F000
  00: FE40 F000 FE41 F000 FE42 F000 FE43 F000
  10: FE44 F000 FE45 F000 FE46 F000 FE47 F000
  20: FE48 F000 FE49 F000 FE4A F000 FE4B F000
  30: FE4C F000 FE4D F000 FE4E F000 FE4F F000
Failed to emulate CS:IP [C000:2E04]=66,0F,BE,C0,E9,FB
UNHANDLED INT 10 FUNCTION 0100 WITHIN EMULATION
EA: BYTE READ FROM UNINITIALIZED LOW MEM 0040:0085
UNHANDLED INT 10 FUNCTION 0300 WITHIN EMULATION
UNHANDLED INT 10 FUNCTION 1301 WITHIN EMULATION
UNHANDLED INT 10 FUNCTION 0300 WITHIN EMULATION
UNHANDLED INT 10 FUNCTION 1301 WITHIN EMULATION
UNHANDLED INT 10 FUNCTION 0300 WITHIN EMULATION
UNHANDLED INT 10 FUNCTION 1301 WITHIN EMULATION
UNHANDLED INT 10 FUNCTION 0300 WITHIN EMULATION

which I assumed could be the same problem with sam460ex too but maybe it's 
different then.


Regards,
BALATON Zoltan



- Jiaxun



[PATCH] hvf: guard xgetbv call.

2020-12-19 Thread Hill Ma
This prevents illegal instruction on cpus do not support xgetbv.

Buglink: https://bugs.launchpad.net/qemu/+bug/1758819
Signed-off-by: Hill Ma 
---
 target/i386/hvf/x86_cpuid.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/target/i386/hvf/x86_cpuid.c b/target/i386/hvf/x86_cpuid.c
index a6842912f5..b4b7111fc3 100644
--- a/target/i386/hvf/x86_cpuid.c
+++ b/target/i386/hvf/x86_cpuid.c
@@ -100,11 +100,16 @@ uint32_t hvf_get_supported_cpuid(uint32_t func, uint32_t 
idx,
 break;
 case 0xD:
 if (idx == 0) {
-uint64_t host_xcr0 = xgetbv(0);
-uint64_t supp_xcr0 = host_xcr0 & (XSTATE_FP_MASK | XSTATE_SSE_MASK 
|
+uint64_t supp_xcr0 = XSTATE_FP_MASK | XSTATE_SSE_MASK |
   XSTATE_YMM_MASK | XSTATE_BNDREGS_MASK |
   XSTATE_BNDCSR_MASK | XSTATE_OPMASK_MASK |
-  XSTATE_ZMM_Hi256_MASK | 
XSTATE_Hi16_ZMM_MASK);
+  XSTATE_ZMM_Hi256_MASK | XSTATE_Hi16_ZMM_MASK;
+if ((ecx & CPUID_EXT_AVX) &&
+(ecx & CPUID_EXT_XSAVE) &&
+(ecx & CPUID_EXT_OSXSAVE)) {
+uint64_t host_xcr0 = xgetbv(0);
+supp_xcr0 &= host_xcr0;
+}
 eax &= supp_xcr0;
 } else if (idx == 1) {
 hv_vmx_read_capability(HV_VMX_CAP_PROCBASED2, );
-- 
2.20.1 (Apple Git-117)




Re: [PATCH] sun4m: don't connect two qemu_irqs directly to the same input

2020-12-19 Thread Artyom Tarasenko
сб, 19 дек. 2020 г., 12:19 Mark Cave-Ayland :

> The sun4m board code connects both of the IRQ outputs of each ESCC to the
> same slavio input qemu_irq. Connecting two qemu_irqs outputs directly to
> the
> same input is not valid as it produces subtly wrong behaviour (for instance
> if both the IRQ lines are high, and then one goes low, the PIC input will
> see
> this as a high-to-low transition even though the second IRQ line should
> still
> be holding it high).
>
> This kind of wiring needs an explicitly created OR gate; add one.
>
> Signed-off-by: Mark Cave-Ayland 
>

Reviewed-by: Artyom Tarasenko 

---
>  hw/sparc/Kconfig |  1 +
>  hw/sparc/sun4m.c | 23 ++-
>  2 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/hw/sparc/Kconfig b/hw/sparc/Kconfig
> index 91805afab6..8dcb10086f 100644
> --- a/hw/sparc/Kconfig
> +++ b/hw/sparc/Kconfig
> @@ -14,6 +14,7 @@ config SUN4M
>  select M48T59
>  select STP2000
>  select CHRP_NVRAM
> +select OR_IRQ
>
>  config LEON3
>  bool
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 8686371318..c06c43be18 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -50,6 +50,7 @@
>  #include "hw/misc/empty_slot.h"
>  #include "hw/misc/unimp.h"
>  #include "hw/irq.h"
> +#include "hw/or-irq.h"
>  #include "hw/loader.h"
>  #include "elf.h"
>  #include "trace.h"
> @@ -848,7 +849,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef
> *hwdef,
>  uint32_t initrd_size;
>  DriveInfo *fd[MAX_FD];
>  FWCfgState *fw_cfg;
> -DeviceState *dev;
> +DeviceState *dev, *ms_kb_orgate, *serial_orgate;
>  SysBusDevice *s;
>  unsigned int smp_cpus = machine->smp.cpus;
>  unsigned int max_cpus = machine->smp.max_cpus;
> @@ -994,10 +995,16 @@ static void sun4m_hw_init(const struct sun4m_hwdef
> *hwdef,
>  qdev_prop_set_uint32(dev, "chnAtype", escc_kbd);
>  s = SYS_BUS_DEVICE(dev);
>  sysbus_realize_and_unref(s, _fatal);
> -sysbus_connect_irq(s, 0, slavio_irq[14]);
> -sysbus_connect_irq(s, 1, slavio_irq[14]);
>  sysbus_mmio_map(s, 0, hwdef->ms_kb_base);
>
> +/* Logically OR both its IRQs together */
> +ms_kb_orgate = DEVICE(object_new(TYPE_OR_IRQ));
> +object_property_set_int(OBJECT(ms_kb_orgate), "num-lines", 2,
> _fatal);
> +qdev_realize_and_unref(ms_kb_orgate, NULL, _fatal);
> +sysbus_connect_irq(s, 0, qdev_get_gpio_in(ms_kb_orgate, 0));
> +sysbus_connect_irq(s, 1, qdev_get_gpio_in(ms_kb_orgate, 1));
> +qdev_connect_gpio_out(DEVICE(ms_kb_orgate), 0, slavio_irq[14]);
> +
>  dev = qdev_new(TYPE_ESCC);
>  qdev_prop_set_uint32(dev, "disabled", 0);
>  qdev_prop_set_uint32(dev, "frequency", ESCC_CLOCK);
> @@ -1009,10 +1016,16 @@ static void sun4m_hw_init(const struct sun4m_hwdef
> *hwdef,
>
>  s = SYS_BUS_DEVICE(dev);
>  sysbus_realize_and_unref(s, _fatal);
> -sysbus_connect_irq(s, 0, slavio_irq[15]);
> -sysbus_connect_irq(s, 1,  slavio_irq[15]);
>  sysbus_mmio_map(s, 0, hwdef->serial_base);
>
> +/* Logically OR both its IRQs together */
> +serial_orgate = DEVICE(object_new(TYPE_OR_IRQ));
> +object_property_set_int(OBJECT(serial_orgate), "num-lines", 2,
> _fatal);
> +qdev_realize_and_unref(serial_orgate, NULL, _fatal);
> +sysbus_connect_irq(s, 0, qdev_get_gpio_in(serial_orgate, 0));
> +sysbus_connect_irq(s, 1, qdev_get_gpio_in(serial_orgate, 1));
> +qdev_connect_gpio_out(DEVICE(serial_orgate), 0, slavio_irq[15]);
> +
>  if (hwdef->apc_base) {
>  apc_init(hwdef->apc_base, qemu_allocate_irq(cpu_halt_signal,
> NULL, 0));
>  }
> --
> 2.20.1
>
>


Re: [PATCH v3 2/2] hw/block: m25p80: Implement AAI-WP command support for SST flashes

2020-12-19 Thread Bin Meng
Hi Francisco,

On Fri, Dec 11, 2020 at 2:37 PM Bin Meng  wrote:
>
> From: Xuzhou Cheng 
>
> Auto Address Increment (AAI) Word-Program is a special command of
> SST flashes. AAI-WP allows multiple bytes of data to be programmed
> without re-issuing the next sequential address location.
>
> Signed-off-by: Xuzhou Cheng 
> Signed-off-by: Bin Meng 
>
> ---
>
> Changes in v3:
> - initialize aai_enable to false in reset_memory()
>
> Changes in v2:
> - add aai_enable into the vmstate
> - validate AAI command before decoding a new command
> - log guest errors during AAI_WP command handling
> - report AAI status in the status register
> - abort AAI programming when address is wrapped
> - make sure AAI programming starts from the even address
>
>  hw/block/m25p80.c | 61 
> +++
>  1 file changed, 61 insertions(+)
>

Any comments for this series?

Regards,
Bin



Re: [PATCH v2 0/8] hm/mips/fuloong2e fixes

2020-12-19 Thread Jiaxun Yang



在2020年12月19日十二月 下午8:13,BALATON Zoltan写道:
> On Sat, 19 Dec 2020, Jiaxun Yang wrote:
> > It can now boot Debian installer[1] as well as a custom PMON bootloader
> > distribution[2].
> >
> > Note that it can't boot PMON shipped with actual machine as our ATI vgabios
> > is using some x86 hack that can't be handled by x86emu in original PMON.
> 
> This may be similar problem that I've seen with similar PPC firmwares:
> 
> https://osdn.net/projects/qmiga/wiki/SubprojectAti
> https://osdn.net/projects/qmiga/wiki/SubprojectPegasos2#h3-Known.20issues
> 
> TLDR; vgabios-ati.bin is compiled for i386 real mode (bacause that's what 
> gcc can do, real x86 real mode would need something like bcc I think) that 
> some x86emu can't handle. You can either use Bochs vga bios via romfile 
> property of the vga emulation or try the option for x86emu when compiling 
> vgabios-ati.bin (which did not help the firmwares I've tried).

Hi,

Thinks for your reminder!

To be more specified, our x86emu in PMON can handle i386 real mode,
however vgabios-ati uses INT15h when INT10h ax=0x4f01 (Get VESA Mode)
is called. And x86emu won't process INT15h properly.

My workround[1] is to allow 0x4f01 to be failed in PMON, as ax=0x4f02
(Set VESA Mode) do work, it won't be a actual problem.


- Jiaxun



[Bug 1908781] [NEW] x86-64 not faulting when CS.L = 1 and CS.D = 1

2020-12-19 Thread Bruce Merry
Public bug reported:

In a UEFI application I accidentally created a code segment descriptor
where both the L and D bits were 1. This is supposed to generate a GP
fault (e.g. see page 2942 of
https://software.intel.com/sites/default/files/managed/39/c5/325462-sdm-
vol-1-2abcd-3abcd.pdf). When running with KVM a fault did indeed occur,
but when not specifying any acceleration, no fault occurred.

Let me know if you need me to develop a minimum example to debug from.
At the moment it's all part of a slightly more complicated bit of code.

Version: 5.2.0 (compiled from source)
Command line options: -smp cores=4 -m 8192 (plus whatever uefi-run adds to plug 
in OVMF and my UEFI application).
Environment: Ubuntu 20.04 on Ryzen 3700X

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  x86-64 not faulting when CS.L = 1 and CS.D = 1

Status in QEMU:
  New

Bug description:
  In a UEFI application I accidentally created a code segment descriptor
  where both the L and D bits were 1. This is supposed to generate a GP
  fault (e.g. see page 2942 of
  https://software.intel.com/sites/default/files/managed/39/c5/325462
  -sdm-vol-1-2abcd-3abcd.pdf). When running with KVM a fault did indeed
  occur, but when not specifying any acceleration, no fault occurred.

  Let me know if you need me to develop a minimum example to debug from.
  At the moment it's all part of a slightly more complicated bit of
  code.

  Version: 5.2.0 (compiled from source)
  Command line options: -smp cores=4 -m 8192 (plus whatever uefi-run adds to 
plug in OVMF and my UEFI application).
  Environment: Ubuntu 20.04 on Ryzen 3700X

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



Re: [PATCH 2/2] win32: make qemu_fd_register() specific to windows

2020-12-19 Thread Paolo Bonzini

On 18/12/20 14:57, marcandre.lur...@redhat.com wrote:

From: Marc-André Lureau 

Move the declaration of the function to a windows-specific header.

The only user left now is SLIRP, which needs special treatement since
it doesn't provide event objects itself for the socket/fds.

Signed-off-by: Marc-André Lureau 


This patch is not needed to fix the build, right?  (I don't disagree 
with it, but I wanted to understand why _you_ thought it was an 
improvement).


Paolo


---
  include/qemu/main-loop.h  | 2 --
  include/sysemu/os-win32.h | 2 ++
  net/slirp.c   | 2 ++
  util/main-loop.c  | 4 
  4 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index d6892fd208..bf93fd691d 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -310,8 +310,6 @@ void qemu_cond_timedwait_iothread(QemuCond *cond, int ms);
  
  /* internal interfaces */
  
-void qemu_fd_register(int fd);

-
  QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque);
  void qemu_bh_schedule_idle(QEMUBH *bh);
  
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h

index 5346d51e89..aa462e3ef6 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -117,6 +117,8 @@ static inline void qemu_funlockfile(FILE *f)
  {
  }
  
+void qemu_fd_register(int fd);

+
  /* We wrap all the sockets functions so that we can
   * set errno based on WSAGetLastError()
   */
diff --git a/net/slirp.c b/net/slirp.c
index 77042e6df7..b54c2882dc 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -196,7 +196,9 @@ static void net_slirp_timer_mod(void *timer, int64_t 
expire_timer,
  
  static void net_slirp_register_poll_fd(int fd, void *opaque)

  {
+#ifdef WIN32
  qemu_fd_register(fd);
+#endif
  }
  
  static void net_slirp_unregister_poll_fd(int fd, void *opaque)

diff --git a/util/main-loop.c b/util/main-loop.c
index 6470f8eae3..744b42fc54 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -179,10 +179,6 @@ static int max_priority;
  static int glib_pollfds_idx;
  static int glib_n_poll_fds;
  
-void qemu_fd_register(int fd)

-{
-}
-
  static void glib_pollfds_fill(int64_t *cur_timeout)
  {
  GMainContext *context = g_main_context_default();






Re: [PATCH v2 0/8] hm/mips/fuloong2e fixes

2020-12-19 Thread BALATON Zoltan via

On Sat, 19 Dec 2020, Jiaxun Yang wrote:

It can now boot Debian installer[1] as well as a custom PMON bootloader
distribution[2].

Note that it can't boot PMON shipped with actual machine as our ATI vgabios
is using some x86 hack that can't be handled by x86emu in original PMON.


This may be similar problem that I've seen with similar PPC firmwares:

https://osdn.net/projects/qmiga/wiki/SubprojectAti
https://osdn.net/projects/qmiga/wiki/SubprojectPegasos2#h3-Known.20issues

TLDR; vgabios-ati.bin is compiled for i386 real mode (bacause that's what 
gcc can do, real x86 real mode would need something like bcc I think) that 
some x86emu can't handle. You can either use Bochs vga bios via romfile 
property of the vga emulation or try the option for x86emu when compiling 
vgabios-ati.bin (which did not help the firmwares I've tried).


Regards,
BALATON Zoltan



Tree avilable at: https://gitlab.com/FlyGoat/qemu/-/tree/fuloong_fixes_v2

v2:
- Collect review tags.
- Get CPU clock via elegant method. (philmd)
- Add boot_linux_console scceptance test

[1]: 
http://archive.debian.org/debian/dists/jessie/main/installer-mipsel/current/images/loongson-2e/netboot/
[2]: 
https://github.com/loongson-community/pmon/releases/download/20201219/pmon-2edev.bin

Thanks.

- Jiaxun

Jiaxun Yang (8):
 hw/mips/fuloong2e: Remove define DEBUG_FULOONG2E_INIT
 hw/mips/fuloong2e: Relpace fault links
 hw/pci-host/bonito: Fixup IRQ mapping
 hw/pci-host/bonito: Fixup pci.lomem mapping
 hw/mips/fuloong2e: Remove unused env entry
 hw/mips/fuloong2e: Correct cpuclock env
 hw/mips/fuloong2e: Add highmem support
 tests/acceptance: Test boot_linux_console for fuloong2e

hw/mips/fuloong2e.c| 84 +-
hw/pci-host/bonito.c   | 45 --
tests/acceptance/boot_linux_console.py | 21 +++
3 files changed, 87 insertions(+), 63 deletions(-)






Re: [PATCH] configure: Fail when specified cross compiler cannot be found

2020-12-19 Thread Paolo Bonzini

On 18/12/20 12:57, Alex Bennée wrote:


Paolo Bonzini  writes:


On 17/12/20 18:56, Alex Bennée wrote:

To be honest at the moment the information is a little hidden at the top
of the output. It would be nice if we could teach meson to echo it in
it's nice coloured output.

Paolo,

Any ideas for the cleanest way to do that?


The code in configure is pretty small:

(for i in $cross_cc_vars; do
export $i
done
export target_list source_path use_containers
$source_path/tests/tcg/configure.sh)

configure would place the cross-cc variables (which are really just
command line options) in a file, something like config-cross-cc.mak, and
the Meson translation of the above would be

env = environment()
foreach k, v : keyval.load(meson.current_build_dir() /
'config-cross-cc.mak')
env.set(k, v)
endforeach
env.set('target_list', ','.join(target_dirs))
env.set('source_path', meson.source_root())
env.set('use_containers',
  'CROSS_CC_CONTAINERS' in config_host ? 'yes' : 'no')
message(run_command(files('tests/tcg/configure.sh'), env: env).stdout())

For a bit more polish, one could make tests/tcg/configure.sh print the
result in keyval format, parse it back from meson as a dictionary with
keyval.load(), and pass the result to summary().


Don't we already have this in the form of tests/tcg/config-$TARGET.mak?
Shouldn't we just injest that into meson after configure.sh has run?


Yes, that's also a possibility I guess!  We can also do both (use 
run_command, and then reread tests/tcg/config-$TARGET.mak).  Long term, 
I'd like to run tests/tcg/configure.sh separately for each target so 
that we can reuse it for pc-bios.


Paolo




Re: [PULL 00/33] QAPI patches patches for 2020-12-19

2020-12-19 Thread no-reply
Patchew URL: 
https://patchew.org/QEMU/20201219105532.1734134-1-arm...@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20201219105532.1734134-1-arm...@redhat.com
Subject: [PULL 00/33] QAPI patches patches for 2020-12-19

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag] patchew/20201219105532.1734134-1-arm...@redhat.com -> 
patchew/20201219105532.1734134-1-arm...@redhat.com
 * [new tag] 
patchew/20201219111934.5540-1-mark.cave-ayl...@ilande.co.uk -> 
patchew/20201219111934.5540-1-mark.cave-ayl...@ilande.co.uk
Switched to a new branch 'test'
50cf88f qobject: Make QString immutable
219adc3f block: Use GString instead of QString to build filenames
6b26586 keyval: Use GString to accumulate value strings
c201d84 json: Use GString instead of QString to accumulate strings
7f3b372 migration: Replace migration's JSON writer by the general one
528861f qobject: Factor JSON writer out of qobject_to_json()
2ec5364 qobject: Factor quoted_str() out of to_json()
3c6b4e4 qobject: Drop qstring_get_try_str()
e9a5862 qobject: Drop qobject_get_try_str()
5355c0b Revert "qobject: let object_property_get_str() use new API"
bb79759 block: Avoid qobject_get_try_str()
682d4a2 qmp: Fix tracing of non-string command IDs
94fb9f2 qobject: Move internals to qobject-internal.h
c35e746 hw/rdma: Replace QList by GQueue
ce6552d Revert "qstring: add qstring_free()"
92a214e qobject: Change qobject_to_json()'s value to GString
9300d4f qobject: Use GString instead of QString to accumulate JSON
83c58e9 qobject: Make qobject_to_json_pretty() take a pretty argument
cae34720 monitor: Use GString instead of QString for output buffer
5ce523f hmp: Simplify how qmp_human_monitor_command() gets output
d00c4a9 test-visitor-serialization: Clean up test_primitives()
1f20720 test-visitor-serialization: Drop insufficient precision workaround
08e5179 string-output-visitor: Fix to use sufficient precision
f6b278a test-string-output-visitor: Cover "unround" number
0c5456e qobject: Fix qnum_to_string() to use sufficient precision
99709b5 tests/check-qnum: Cover qnum_to_string() for "unround" argument
b9f996e tests/check-qjson: Replace redundant large_number()
5395cbb tests/check-qjson: Cover number 2^63
772a54f tests/check-qjson: Examine QNum more thoroughly
e4decd9 tests/check-qjson: Don't skip funny QNumber to JSON conversions
3b10bb6 qapi: Use QAPI_LIST_PREPEND() where possible
1a5e6e4 migration: Refactor migrate_cap_add
b197a61 rocker: Revamp fp_port_get_info

=== OUTPUT BEGIN ===
1/33 Checking commit b197a61caecb (rocker: Revamp fp_port_get_info)
2/33 Checking commit 1a5e6e4a2c9f (migration: Refactor migrate_cap_add)
3/33 Checking commit 3b10bb662508 (qapi: Use QAPI_LIST_PREPEND() where possible)
4/33 Checking commit e4decd945ac8 (tests/check-qjson: Don't skip funny QNumber 
to JSON conversions)
WARNING: Block comments use a leading /* on a separate line
#95: FILE: tests/check-qjson.c:883:
+{ "-32.20e-10", -32.20e-10, "-0" /* BUG */ },

ERROR: spaces required around that '-' (ctx:VxV)
#95: FILE: tests/check-qjson.c:883:
+{ "-32.20e-10", -32.20e-10, "-0" /* BUG */ },
^

total: 1 errors, 1 warnings, 97 lines checked

Patch 4/33 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/33 Checking commit 772a54f1cf5a (tests/check-qjson: Examine QNum more 
thoroughly)
6/33 Checking commit 5395cbbe7df9 (tests/check-qjson: Cover number 2^63)
7/33 Checking commit b9f996e8a2e1 (tests/check-qjson: Replace redundant 
large_number())
8/33 Checking commit 99709b586e03 (tests/check-qnum: Cover qnum_to_string() for 
"unround" argument)
9/33 Checking commit 0c5456ea8d00 (qobject: Fix qnum_to_string() to use 
sufficient precision)
WARNING: Block comments use a leading /* on a separate line
#105: FILE: qobject/qnum.c:170:
+/* FIXME: g_strdup_printf() is locale dependent; but JSON requires

ERROR: spaces required around that '-' (ctx:VxV)
#145: FILE: tests/check-qjson.c:886:
+{ "-32.20e-10", -32.20e-10, "-3.22e-09" },
^

total: 1 errors, 1 warnings, 69 lines checked

Patch 9/33 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

10/33 Checking commit f6b278afee74 (test-string-output-visitor: Cover "unround" 
number)
11/33 Checking commit 08e5179b5f93 (string-output-visitor: Fix to use 
sufficient precision)
12/33 Checking commit 1f20720448a4 (test-visitor-serialization: Drop 

[PATCH] sun4m: don't connect two qemu_irqs directly to the same input

2020-12-19 Thread Mark Cave-Ayland
The sun4m board code connects both of the IRQ outputs of each ESCC to the
same slavio input qemu_irq. Connecting two qemu_irqs outputs directly to the
same input is not valid as it produces subtly wrong behaviour (for instance
if both the IRQ lines are high, and then one goes low, the PIC input will see
this as a high-to-low transition even though the second IRQ line should still
be holding it high).

This kind of wiring needs an explicitly created OR gate; add one.

Signed-off-by: Mark Cave-Ayland 
---
 hw/sparc/Kconfig |  1 +
 hw/sparc/sun4m.c | 23 ++-
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/hw/sparc/Kconfig b/hw/sparc/Kconfig
index 91805afab6..8dcb10086f 100644
--- a/hw/sparc/Kconfig
+++ b/hw/sparc/Kconfig
@@ -14,6 +14,7 @@ config SUN4M
 select M48T59
 select STP2000
 select CHRP_NVRAM
+select OR_IRQ
 
 config LEON3
 bool
diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
index 8686371318..c06c43be18 100644
--- a/hw/sparc/sun4m.c
+++ b/hw/sparc/sun4m.c
@@ -50,6 +50,7 @@
 #include "hw/misc/empty_slot.h"
 #include "hw/misc/unimp.h"
 #include "hw/irq.h"
+#include "hw/or-irq.h"
 #include "hw/loader.h"
 #include "elf.h"
 #include "trace.h"
@@ -848,7 +849,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
 uint32_t initrd_size;
 DriveInfo *fd[MAX_FD];
 FWCfgState *fw_cfg;
-DeviceState *dev;
+DeviceState *dev, *ms_kb_orgate, *serial_orgate;
 SysBusDevice *s;
 unsigned int smp_cpus = machine->smp.cpus;
 unsigned int max_cpus = machine->smp.max_cpus;
@@ -994,10 +995,16 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef,
 qdev_prop_set_uint32(dev, "chnAtype", escc_kbd);
 s = SYS_BUS_DEVICE(dev);
 sysbus_realize_and_unref(s, _fatal);
-sysbus_connect_irq(s, 0, slavio_irq[14]);
-sysbus_connect_irq(s, 1, slavio_irq[14]);
 sysbus_mmio_map(s, 0, hwdef->ms_kb_base);
 
+/* Logically OR both its IRQs together */
+ms_kb_orgate = DEVICE(object_new(TYPE_OR_IRQ));
+object_property_set_int(OBJECT(ms_kb_orgate), "num-lines", 2, 
_fatal);
+qdev_realize_and_unref(ms_kb_orgate, NULL, _fatal);
+sysbus_connect_irq(s, 0, qdev_get_gpio_in(ms_kb_orgate, 0));
+sysbus_connect_irq(s, 1, qdev_get_gpio_in(ms_kb_orgate, 1));
+qdev_connect_gpio_out(DEVICE(ms_kb_orgate), 0, slavio_irq[14]);
+
 dev = qdev_new(TYPE_ESCC);
 qdev_prop_set_uint32(dev, "disabled", 0);
 qdev_prop_set_uint32(dev, "frequency", ESCC_CLOCK);
@@ -1009,10 +1016,16 @@ static void sun4m_hw_init(const struct sun4m_hwdef 
*hwdef,
 
 s = SYS_BUS_DEVICE(dev);
 sysbus_realize_and_unref(s, _fatal);
-sysbus_connect_irq(s, 0, slavio_irq[15]);
-sysbus_connect_irq(s, 1,  slavio_irq[15]);
 sysbus_mmio_map(s, 0, hwdef->serial_base);
 
+/* Logically OR both its IRQs together */
+serial_orgate = DEVICE(object_new(TYPE_OR_IRQ));
+object_property_set_int(OBJECT(serial_orgate), "num-lines", 2, 
_fatal);
+qdev_realize_and_unref(serial_orgate, NULL, _fatal);
+sysbus_connect_irq(s, 0, qdev_get_gpio_in(serial_orgate, 0));
+sysbus_connect_irq(s, 1, qdev_get_gpio_in(serial_orgate, 1));
+qdev_connect_gpio_out(DEVICE(serial_orgate), 0, slavio_irq[15]);
+
 if (hwdef->apc_base) {
 apc_init(hwdef->apc_base, qemu_allocate_irq(cpu_halt_signal, NULL, 0));
 }
-- 
2.20.1




[PULL 24/33] Revert "qobject: let object_property_get_str() use new API"

2020-12-19 Thread Markus Armbruster
Commit aafb21a0b9 "qobject: let object_property_get_str() use new API"
isn't much of a simplification.  Not worth having
object_property_get_str() differ from the other
object_property_get_FOO().  Revert.

This reverts commit aafb21a0b9cea5fa0fe52e68111bb6bd13837a02.

Cc: Paolo Bonzini 
Cc: Daniel P. Berrangé 
Cc: Eduardo Habkost 
Signed-off-by: Markus Armbruster 
Message-Id: <20201211171152.146877-12-arm...@redhat.com>
Reviewed-by: Eduardo Habkost 
---
 qom/object.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/qom/object.c b/qom/object.c
index f2ae6e6b2a..5cd43fe366 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1415,15 +1415,18 @@ char *object_property_get_str(Object *obj, const char 
*name,
   Error **errp)
 {
 QObject *ret = object_property_get_qobject(obj, name, errp);
+QString *qstring;
 char *retval;
 
 if (!ret) {
 return NULL;
 }
-
-retval = g_strdup(qobject_get_try_str(ret));
-if (!retval) {
+qstring = qobject_to(QString, ret);
+if (!qstring) {
 error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name, "string");
+retval = NULL;
+} else {
+retval = g_strdup(qstring_get_str(qstring));
 }
 
 qobject_unref(ret);
-- 
2.26.2




[PULL 29/33] migration: Replace migration's JSON writer by the general one

2020-12-19 Thread Markus Armbruster
Commit 8118f0950f "migration: Append JSON description of migration
stream" needs a JSON writer.  The existing qobject_to_json() wasn't a
good fit, because it requires building a QObject to convert.  Instead,
migration got its very own JSON writer, in commit 190c882ce2 "QJSON:
Add JSON writer".  It tacitly limits numbers to int64_t, and strings
contents to characters that don't need escaping, unlike
qobject_to_json().

The previous commit factored the JSON writer out of qobject_to_json().
Replace migration's JSON writer by it.

Cc: Juan Quintela 
Cc: Dr. David Alan Gilbert 
Signed-off-by: Markus Armbruster 
Message-Id: <20201211171152.146877-17-arm...@redhat.com>
Reviewed-by: Dr. David Alan Gilbert 
---
 include/migration/vmstate.h|   7 +-
 include/qapi/qmp/json-writer.h |   2 -
 include/qemu/typedefs.h|   4 +-
 migration/qjson.h  |  29 -
 hw/display/virtio-gpu.c|   2 +-
 hw/intc/s390_flic_kvm.c|   2 +-
 hw/nvram/eeprom93xx.c  |   2 +-
 hw/nvram/fw_cfg.c  |   2 +-
 hw/pci/msix.c  |   2 +-
 hw/pci/pci.c   |   4 +-
 hw/pci/shpc.c  |   2 +-
 hw/rtc/twl92230.c  |   2 +-
 hw/scsi/scsi-bus.c |   2 +-
 hw/usb/redirect.c  |   7 +-
 hw/virtio/virtio.c |   4 +-
 migration/qjson.c  | 114 -
 migration/savevm.c |  53 +++
 migration/vmstate-types.c  |  38 +--
 migration/vmstate.c|  52 +++
 target/alpha/machine.c |   2 +-
 target/arm/machine.c   |   6 +-
 target/avr/machine.c   |   4 +-
 target/hppa/machine.c  |   4 +-
 target/microblaze/machine.c|   2 +-
 target/mips/machine.c  |   4 +-
 target/openrisc/machine.c  |   2 +-
 target/ppc/machine.c   |  10 +--
 target/sparc/machine.c |   2 +-
 migration/meson.build  |   1 -
 29 files changed, 114 insertions(+), 253 deletions(-)
 delete mode 100644 migration/qjson.h
 delete mode 100644 migration/qjson.c

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 4d71dc8fba..075ee80096 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -43,7 +43,7 @@ struct VMStateInfo {
 const char *name;
 int (*get)(QEMUFile *f, void *pv, size_t size, const VMStateField *field);
 int (*put)(QEMUFile *f, void *pv, size_t size, const VMStateField *field,
-   QJSON *vmdesc);
+   JSONWriter *vmdesc);
 };
 
 enum VMStateFlags {
@@ -1169,9 +1169,10 @@ extern const VMStateInfo vmstate_info_qlist;
 int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
void *opaque, int version_id);
 int vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
-   void *opaque, QJSON *vmdesc);
+   void *opaque, JSONWriter *vmdesc);
 int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
- void *opaque, QJSON *vmdesc, int version_id);
+ void *opaque, JSONWriter *vmdesc,
+ int version_id);
 
 bool vmstate_save_needed(const VMStateDescription *vmsd, void *opaque);
 
diff --git a/include/qapi/qmp/json-writer.h b/include/qapi/qmp/json-writer.h
index 708d129018..b70ba64077 100644
--- a/include/qapi/qmp/json-writer.h
+++ b/include/qapi/qmp/json-writer.h
@@ -14,8 +14,6 @@
 #ifndef JSON_WRITER_H
 #define JSON_WRITER_H
 
-typedef struct JSONWriter JSONWriter;
-
 JSONWriter *json_writer_new(bool pretty);
 const char *json_writer_get(JSONWriter *);
 GString *json_writer_get_and_free(JSONWriter *);
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index 6281eae3b5..976b529dfb 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -57,8 +57,8 @@ typedef struct IOMMUMemoryRegion IOMMUMemoryRegion;
 typedef struct ISABus ISABus;
 typedef struct ISADevice ISADevice;
 typedef struct IsaDma IsaDma;
+typedef struct JSONWriter JSONWriter;
 typedef struct MACAddr MACAddr;
-typedef struct ReservedRegion ReservedRegion;
 typedef struct MachineClass MachineClass;
 typedef struct MachineState MachineState;
 typedef struct MemoryListener MemoryListener;
@@ -107,7 +107,6 @@ typedef struct QEMUSGList QEMUSGList;
 typedef struct QemuSpin QemuSpin;
 typedef struct QEMUTimer QEMUTimer;
 typedef struct QEMUTimerListGroup QEMUTimerListGroup;
-typedef struct QJSON QJSON;
 typedef struct QList QList;
 typedef struct QNull QNull;
 typedef struct QNum QNum;
@@ -115,6 +114,7 @@ typedef struct QObject QObject;
 typedef struct QString QString;
 typedef struct RAMBlock RAMBlock;
 typedef struct Range Range;
+typedef struct ReservedRegion ReservedRegion;
 typedef struct SavedIOTLB SavedIOTLB;
 typedef struct SHPCDevice SHPCDevice;
 typedef struct SSIBus SSIBus;
diff --git a/migration/qjson.h b/migration/qjson.h
deleted file mode 100644
index 

[PULL 03/33] qapi: Use QAPI_LIST_PREPEND() where possible

2020-12-19 Thread Markus Armbruster
From: Eric Blake 

Anywhere we create a list of just one item or by prepending items
(typically because order doesn't matter), we can use
QAPI_LIST_PREPEND().  But places where we must keep the list in order
by appending remain open-coded until later patches.

Note that as a side effect, this also performs a cleanup of two minor
issues in qga/commands-posix.c: the old code was performing
 new = g_malloc0(sizeof(*ret));
which 1) is confusing because you have to verify whether 'new' and
'ret' are variables with the same type, and 2) would conflict with C++
compilation (not an actual problem for this file, but makes
copy-and-paste harder).

Signed-off-by: Eric Blake 
Message-Id: <20201113011340.463563-5-ebl...@redhat.com>
Reviewed-by: Markus Armbruster 
Acked-by: Stefan Hajnoczi 
[Straightforward conflicts due to commit a8aa94b5f8 "qga: update
schema for guest-get-disks 'dependents' field" and commit a10b453a52
"target/mips: Move mips_cpu_add_definition() from helper.c to cpu.c"
resolved.  Commit message tweaked.]
Signed-off-by: Markus Armbruster 
---
 docs/devel/writing-qmp-commands.txt |  12 +--
 block/gluster.c |   4 +-
 block/qapi.c|   7 +-
 chardev/char.c  |  20 ++---
 hw/core/machine-qmp-cmds.c  |   6 +-
 hw/core/machine.c   |  11 +--
 hw/net/rocker/rocker_of_dpa.c   |  20 ++---
 hw/net/virtio-net.c |  21 ++
 migration/migration.c   |   7 +-
 migration/postcopy-ram.c|   7 +-
 monitor/hmp-cmds.c  |  13 ++--
 monitor/misc.c  |  25 +++---
 monitor/qmp-cmds-control.c  |  10 +--
 qemu-img.c  |   5 +-
 qga/commands-posix-ssh.c|   7 +-
 qga/commands-posix.c|  47 +++-
 qga/commands-win32.c|  32 ++--
 qga/commands.c  |   6 +-
 qom/qom-qmp-cmds.c  |  29 ++-
 target/arm/helper.c |   6 +-
 target/arm/monitor.c|  13 +---
 target/i386/cpu.c   |   6 +-
 target/mips/cpu.c   |   6 +-
 target/s390x/cpu_models.c   |  12 +--
 tests/test-clone-visitor.c  |   7 +-
 tests/test-qobject-output-visitor.c |  42 +--
 tests/test-visitor-serialization.c  | 113 
 trace/qmp.c |  22 +++---
 ui/input.c  |  16 ++--
 ui/vnc.c|  21 ++
 util/qemu-config.c  |  14 +---
 target/ppc/translate_init.c.inc |  12 +--
 32 files changed, 158 insertions(+), 421 deletions(-)

diff --git a/docs/devel/writing-qmp-commands.txt 
b/docs/devel/writing-qmp-commands.txt
index 28984686c9..258e63bff5 100644
--- a/docs/devel/writing-qmp-commands.txt
+++ b/docs/devel/writing-qmp-commands.txt
@@ -531,15 +531,11 @@ TimerAlarmMethodList *qmp_query_alarm_methods(Error 
**errp)
 bool current = true;
 
 for (p = alarm_timers; p->name; p++) {
-TimerAlarmMethodList *info = g_malloc0(sizeof(*info));
-info->value = g_malloc0(sizeof(*info->value));
-info->value->method_name = g_strdup(p->name);
-info->value->current = current;
-
+TimerAlarmMethod *value = g_malloc0(*value);
+value->method_name = g_strdup(p->name);
+value->current = current;
+QAPI_LIST_PREPEND(method_list, value);
 current = false;
-
-info->next = method_list;
-method_list = info;
 }
 
 return method_list;
diff --git a/block/gluster.c b/block/gluster.c
index 4f1448e2bc..1f8699b938 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -359,8 +359,8 @@ static int qemu_gluster_parse_uri(BlockdevOptionsGluster 
*gconf,
 return -EINVAL;
 }
 
-gconf->server = g_new0(SocketAddressList, 1);
-gconf->server->value = gsconf = g_new0(SocketAddress, 1);
+gsconf = g_new0(SocketAddress, 1);
+QAPI_LIST_PREPEND(gconf->server, gsconf);
 
 /* transport */
 if (!uri->scheme || !strcmp(uri->scheme, "gluster")) {
diff --git a/block/qapi.c b/block/qapi.c
index 036da085ee..0ca206f559 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -486,12 +486,7 @@ static void bdrv_query_blk_stats(BlockDeviceStats *ds, 
BlockBackend *blk)
 ds->account_failed = stats->account_failed;
 
 while ((ts = block_acct_interval_next(stats, ts))) {
-BlockDeviceTimedStatsList *timed_stats =
-g_malloc0(sizeof(*timed_stats));
 BlockDeviceTimedStats *dev_stats = g_malloc0(sizeof(*dev_stats));
-timed_stats->next = ds->timed_stats;
-timed_stats->value = dev_stats;
-ds->timed_stats = timed_stats;
 
 TimedAverage *rd = >latency[BLOCK_ACCT_READ];
 TimedAverage *wr = >latency[BLOCK_ACCT_WRITE];
@@ -515,6 +510,8 @@ static void bdrv_query_blk_stats(BlockDeviceStats *ds, 
BlockBackend *blk)
 block_acct_queue_depth(ts, BLOCK_ACCT_READ);
   

[PULL 21/33] qobject: Move internals to qobject-internal.h

2020-12-19 Thread Markus Armbruster
Signed-off-by: Markus Armbruster 
Message-Id: <20201211171152.146877-9-arm...@redhat.com>
---
 include/qapi/qmp/qbool.h   |  2 --
 include/qapi/qmp/qdict.h   |  2 --
 include/qapi/qmp/qlist.h   |  2 --
 include/qapi/qmp/qnull.h   |  2 --
 include/qapi/qmp/qnum.h|  3 ---
 include/qapi/qmp/qobject.h |  9 +
 include/qapi/qmp/qstring.h |  2 --
 qobject/qobject-internal.h | 39 ++
 qobject/qbool.c|  1 +
 qobject/qdict.c|  1 +
 qobject/qlist.c|  1 +
 qobject/qnull.c|  1 +
 qobject/qnum.c |  1 +
 qobject/qobject.c  |  1 +
 qobject/qstring.c  |  1 +
 15 files changed, 47 insertions(+), 21 deletions(-)
 create mode 100644 qobject/qobject-internal.h

diff --git a/include/qapi/qmp/qbool.h b/include/qapi/qmp/qbool.h
index 5f61e38e64..2f888d1057 100644
--- a/include/qapi/qmp/qbool.h
+++ b/include/qapi/qmp/qbool.h
@@ -23,7 +23,5 @@ struct QBool {
 
 QBool *qbool_from_bool(bool value);
 bool qbool_get_bool(const QBool *qb);
-bool qbool_is_equal(const QObject *x, const QObject *y);
-void qbool_destroy_obj(QObject *obj);
 
 #endif /* QBOOL_H */
diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index da942347a7..9934539c1b 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -39,10 +39,8 @@ void qdict_put_obj(QDict *qdict, const char *key, QObject 
*value);
 void qdict_del(QDict *qdict, const char *key);
 int qdict_haskey(const QDict *qdict, const char *key);
 QObject *qdict_get(const QDict *qdict, const char *key);
-bool qdict_is_equal(const QObject *x, const QObject *y);
 const QDictEntry *qdict_first(const QDict *qdict);
 const QDictEntry *qdict_next(const QDict *qdict, const QDictEntry *entry);
-void qdict_destroy_obj(QObject *obj);
 
 /* Helper to qdict_put_obj(), accepts any object */
 #define qdict_put(qdict, key, obj) \
diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h
index 595b7943e1..06e98ad5f4 100644
--- a/include/qapi/qmp/qlist.h
+++ b/include/qapi/qmp/qlist.h
@@ -51,8 +51,6 @@ QObject *qlist_pop(QList *qlist);
 QObject *qlist_peek(QList *qlist);
 int qlist_empty(const QList *qlist);
 size_t qlist_size(const QList *qlist);
-bool qlist_is_equal(const QObject *x, const QObject *y);
-void qlist_destroy_obj(QObject *obj);
 
 static inline const QListEntry *qlist_first(const QList *qlist)
 {
diff --git a/include/qapi/qmp/qnull.h b/include/qapi/qmp/qnull.h
index c1426882c5..e84ecceedb 100644
--- a/include/qapi/qmp/qnull.h
+++ b/include/qapi/qmp/qnull.h
@@ -26,6 +26,4 @@ static inline QNull *qnull(void)
 return qobject_ref(_);
 }
 
-bool qnull_is_equal(const QObject *x, const QObject *y);
-
 #endif /* QNULL_H */
diff --git a/include/qapi/qmp/qnum.h b/include/qapi/qmp/qnum.h
index bbae0a5ec8..7f84e20bfb 100644
--- a/include/qapi/qmp/qnum.h
+++ b/include/qapi/qmp/qnum.h
@@ -68,7 +68,4 @@ double qnum_get_double(QNum *qn);
 
 char *qnum_to_string(QNum *qn);
 
-bool qnum_is_equal(const QObject *x, const QObject *y);
-void qnum_destroy_obj(QObject *obj);
-
 #endif /* QNUM_H */
diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h
index fcfd549220..9003b71fd3 100644
--- a/include/qapi/qmp/qobject.h
+++ b/include/qapi/qmp/qobject.h
@@ -64,14 +64,6 @@ QEMU_BUILD_BUG_MSG(QTYPE__MAX != 7,
 #define qobject_to(type, obj)   \
 ((type *)qobject_check_type(obj, glue(QTYPE_CAST_TO_, type)))
 
-/* Initialize an object to default values */
-static inline void qobject_init(QObject *obj, QType type)
-{
-assert(QTYPE_NONE < type && type < QTYPE__MAX);
-obj->base.refcnt = 1;
-obj->base.type = type;
-}
-
 static inline void qobject_ref_impl(QObject *obj)
 {
 if (obj) {
@@ -90,6 +82,7 @@ bool qobject_is_equal(const QObject *x, const QObject *y);
 
 /**
  * qobject_destroy(): Free resources used by the object
+ * For use via qobject_unref() only!
  */
 void qobject_destroy(QObject *obj);
 
diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h
index ae5b4b44d2..e4ac761a22 100644
--- a/include/qapi/qmp/qstring.h
+++ b/include/qapi/qmp/qstring.h
@@ -33,7 +33,5 @@ const char *qobject_get_try_str(const QObject *qstring);
 void qstring_append_int(QString *qstring, int64_t value);
 void qstring_append(QString *qstring, const char *str);
 void qstring_append_chr(QString *qstring, int c);
-bool qstring_is_equal(const QObject *x, const QObject *y);
-void qstring_destroy_obj(QObject *obj);
 
 #endif /* QSTRING_H */
diff --git a/qobject/qobject-internal.h b/qobject/qobject-internal.h
new file mode 100644
index 00..b310c8e1b5
--- /dev/null
+++ b/qobject/qobject-internal.h
@@ -0,0 +1,39 @@
+/*
+ * QObject internals
+ *
+ * Copyright (C) 2015 Red Hat, Inc.
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1
+ * or later.  See the COPYING.LIB file in the top-level directory.
+ */
+
+#ifndef QOBJECT_INTERNAL_H
+#define QOBJECT_INTERNAL_H
+
+#include "qapi/qmp/qobject.h"
+

[PULL 17/33] qobject: Use GString instead of QString to accumulate JSON

2020-12-19 Thread Markus Armbruster
QString supports modifying its string, but it's quite limited: you can
only append.  The remaining callers use it for building an initial
string, never for modifying it later.

Use of GString for building the initial string is actually more
convenient here.  Change qobject_to_json() & friends to do that.

Once all such uses are replaced this way, QString can become immutable.

Signed-off-by: Markus Armbruster 
Message-Id: <20201211171152.146877-5-arm...@redhat.com>
---
 include/qapi/qmp/qstring.h |  1 +
 qobject/qjson.c| 85 +-
 qobject/qstring.c  | 19 +
 3 files changed, 58 insertions(+), 47 deletions(-)

diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h
index e2e356e5e7..ae7698d6c7 100644
--- a/include/qapi/qmp/qstring.h
+++ b/include/qapi/qmp/qstring.h
@@ -25,6 +25,7 @@ struct QString {
 QString *qstring_new(void);
 QString *qstring_from_str(const char *str);
 QString *qstring_from_substr(const char *str, size_t start, size_t end);
+QString *qstring_from_gstring(GString *gstr);
 size_t qstring_get_length(const QString *qstring);
 const char *qstring_get_str(const QString *qstring);
 const char *qstring_get_try_str(const QString *qstring);
diff --git a/qobject/qjson.c b/qobject/qjson.c
index 523a4ab8de..e7100a539c 100644
--- a/qobject/qjson.c
+++ b/qobject/qjson.c
@@ -149,28 +149,23 @@ QDict *qdict_from_jsonf_nofail(const char *string, ...)
 return qdict;
 }
 
-static void json_pretty_newline(QString *str, bool pretty, int indent)
+static void json_pretty_newline(GString *accu, bool pretty, int indent)
 {
-int i;
-
 if (pretty) {
-qstring_append(str, "\n");
-for (i = 0; i < indent; i++) {
-qstring_append(str, "");
-}
+g_string_append_printf(accu, "\n%*s", indent * 4, "");
 }
 }
 
-static void to_json(const QObject *obj, QString *str, bool pretty, int indent)
+static void to_json(const QObject *obj, GString *accu, bool pretty, int indent)
 {
 switch (qobject_type(obj)) {
 case QTYPE_QNULL:
-qstring_append(str, "null");
+g_string_append(accu, "null");
 break;
 case QTYPE_QNUM: {
 QNum *val = qobject_to(QNum, obj);
 char *buffer = qnum_to_string(val);
-qstring_append(str, buffer);
+g_string_append(accu, buffer);
 g_free(buffer);
 break;
 }
@@ -178,35 +173,34 @@ static void to_json(const QObject *obj, QString *str, 
bool pretty, int indent)
 QString *val = qobject_to(QString, obj);
 const char *ptr;
 int cp;
-char buf[16];
 char *end;
 
 ptr = qstring_get_str(val);
-qstring_append(str, "\"");
+g_string_append_c(accu, '"');
 
 for (; *ptr; ptr = end) {
 cp = mod_utf8_codepoint(ptr, 6, );
 switch (cp) {
 case '\"':
-qstring_append(str, "\\\"");
+g_string_append(accu, "\\\"");
 break;
 case '\\':
-qstring_append(str, "");
+g_string_append(accu, "");
 break;
 case '\b':
-qstring_append(str, "\\b");
+g_string_append(accu, "\\b");
 break;
 case '\f':
-qstring_append(str, "\\f");
+g_string_append(accu, "\\f");
 break;
 case '\n':
-qstring_append(str, "\\n");
+g_string_append(accu, "\\n");
 break;
 case '\r':
-qstring_append(str, "\\r");
+g_string_append(accu, "\\r");
 break;
 case '\t':
-qstring_append(str, "\\t");
+g_string_append(accu, "\\t");
 break;
 default:
 if (cp < 0) {
@@ -214,20 +208,18 @@ static void to_json(const QObject *obj, QString *str, 
bool pretty, int indent)
 }
 if (cp > 0x) {
 /* beyond BMP; need a surrogate pair */
-snprintf(buf, sizeof(buf), "\\u%04X\\u%04X",
- 0xD800 + ((cp - 0x1) >> 10),
- 0xDC00 + ((cp - 0x1) & 0x3FF));
+g_string_append_printf(accu, "\\u%04X\\u%04X",
+   0xD800 + ((cp - 0x1) >> 10),
+   0xDC00 + ((cp - 0x1) & 0x3FF));
 } else if (cp < 0x20 || cp >= 0x7F) {
-snprintf(buf, sizeof(buf), "\\u%04X", cp);
+g_string_append_printf(accu, "\\u%04X", cp);
 } else {
-buf[0] = cp;
-buf[1] = 0;
+g_string_append_c(accu, cp);
 }
-qstring_append(str, buf);
 }
 };
 
-qstring_append(str, 

[PULL 32/33] block: Use GString instead of QString to build filenames

2020-12-19 Thread Markus Armbruster
QString supports modifying its string, but it's quite limited: you can
only append.  Just one caller remains:
bdrv_parse_filename_strip_prefix() uses it just for building an
initial string.

Change it to do build the initial string with GString.  This is
another step towards making QString immutable.

Cc: Kevin Wolf 
Cc: Max Reitz 
Cc: qemu-bl...@nongnu.org
Signed-off-by: Markus Armbruster 
Message-Id: <20201211171152.146877-20-arm...@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 block.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/block.c b/block.c
index 934d1bcc8f..27b2c3ba15 100644
--- a/block.c
+++ b/block.c
@@ -217,7 +217,7 @@ void bdrv_parse_filename_strip_prefix(const char *filename, 
const char *prefix,
 /* Stripping the explicit protocol prefix may result in a protocol
  * prefix being (wrongly) detected (if the filename contains a colon) 
*/
 if (path_has_protocol(filename)) {
-QString *fat_filename;
+GString *fat_filename;
 
 /* This means there is some colon before the first slash; 
therefore,
  * this cannot be an absolute path */
@@ -225,12 +225,13 @@ void bdrv_parse_filename_strip_prefix(const char 
*filename, const char *prefix,
 
 /* And we can thus fix the protocol detection issue by prefixing it
  * by "./" */
-fat_filename = qstring_from_str("./");
-qstring_append(fat_filename, filename);
+fat_filename = g_string_new("./");
+g_string_append(fat_filename, filename);
 
-assert(!path_has_protocol(qstring_get_str(fat_filename)));
+assert(!path_has_protocol(fat_filename->str));
 
-qdict_put(options, "filename", fat_filename);
+qdict_put(options, "filename",
+  qstring_from_gstring(fat_filename));
 } else {
 /* If no protocol prefix was detected, we can use the shortened
  * filename as-is */
-- 
2.26.2




[PULL 25/33] qobject: Drop qobject_get_try_str()

2020-12-19 Thread Markus Armbruster
Signed-off-by: Markus Armbruster 
Message-Id: <20201211171152.146877-13-arm...@redhat.com>
---
 include/qapi/qmp/qstring.h |  1 -
 qobject/qstring.c  | 11 ---
 2 files changed, 12 deletions(-)

diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h
index e4ac761a22..56034dae54 100644
--- a/include/qapi/qmp/qstring.h
+++ b/include/qapi/qmp/qstring.h
@@ -29,7 +29,6 @@ QString *qstring_from_gstring(GString *gstr);
 size_t qstring_get_length(const QString *qstring);
 const char *qstring_get_str(const QString *qstring);
 const char *qstring_get_try_str(const QString *qstring);
-const char *qobject_get_try_str(const QObject *qstring);
 void qstring_append_int(QString *qstring, int64_t value);
 void qstring_append(QString *qstring, const char *str);
 void qstring_append_chr(QString *qstring, int c);
diff --git a/qobject/qstring.c b/qobject/qstring.c
index d6724bd4e5..cfe3f3bf00 100644
--- a/qobject/qstring.c
+++ b/qobject/qstring.c
@@ -149,17 +149,6 @@ const char *qstring_get_try_str(const QString *qstring)
 return qstring ? qstring_get_str(qstring) : NULL;
 }
 
-/**
- * qobject_get_try_str(): Return a pointer to the corresponding string
- *
- * NOTE: the string will only be returned if the object is valid, and
- * its type is QString, otherwise NULL is returned.
- */
-const char *qobject_get_try_str(const QObject *qstring)
-{
-return qstring_get_try_str(qobject_to(QString, qstring));
-}
-
 /**
  * qstring_is_equal(): Test whether the two QStrings are equal
  */
-- 
2.26.2




[PULL 18/33] qobject: Change qobject_to_json()'s value to GString

2020-12-19 Thread Markus Armbruster
qobject_to_json() and qobject_to_json_pretty() build a GString, then
covert it to QString.  Just one of the callers actually needs a
QString: qemu_rbd_parse_filename().  A few others need a string they
can modify: qmp_send_response(), qga's send_response(), to_json_str(),
and qmp_fd_vsend_fds().  The remainder just need a string.

Change qobject_to_json() and qobject_to_json_pretty() to return the
GString.

qemu_rbd_parse_filename() now has to convert to QString.  All others
save a QString temporary.  to_json_str() actually becomes a bit
simpler, because GString provides more convenient modification
functions.

Signed-off-by: Markus Armbruster 
Message-Id: <20201211171152.146877-6-arm...@redhat.com>
---
 include/qapi/qmp/qjson.h   |  4 +--
 block.c|  6 ++--
 block/rbd.c|  2 +-
 monitor/qmp.c  | 14 
 qemu-img.c | 25 +++--
 qga/main.c | 22 
 qobject/qjson.c|  6 ++--
 qom/object_interfaces.c|  4 +--
 qom/qom-hmp-cmds.c |  7 ++--
 tests/check-qjson.c| 56 ++
 tests/qtest/libqtest.c | 20 +--
 tests/test-visitor-serialization.c |  6 ++--
 12 files changed, 79 insertions(+), 93 deletions(-)

diff --git a/include/qapi/qmp/qjson.h b/include/qapi/qmp/qjson.h
index 82f4534f16..593b40b4e0 100644
--- a/include/qapi/qmp/qjson.h
+++ b/include/qapi/qmp/qjson.h
@@ -25,7 +25,7 @@ QDict *qdict_from_vjsonf_nofail(const char *string, va_list 
ap)
 QDict *qdict_from_jsonf_nofail(const char *string, ...)
 GCC_FMT_ATTR(1, 2);
 
-QString *qobject_to_json(const QObject *obj);
-QString *qobject_to_json_pretty(const QObject *obj, bool pretty);
+GString *qobject_to_json(const QObject *obj);
+GString *qobject_to_json_pretty(const QObject *obj, bool pretty);
 
 #endif /* QJSON_H */
diff --git a/block.c b/block.c
index 8f177504d4..09d4c6bd25 100644
--- a/block.c
+++ b/block.c
@@ -6995,13 +6995,13 @@ void bdrv_refresh_filename(BlockDriverState *bs)
 if (bs->exact_filename[0]) {
 pstrcpy(bs->filename, sizeof(bs->filename), bs->exact_filename);
 } else {
-QString *json = qobject_to_json(QOBJECT(bs->full_open_options));
+GString *json = qobject_to_json(QOBJECT(bs->full_open_options));
 if (snprintf(bs->filename, sizeof(bs->filename), "json:%s",
- qstring_get_str(json)) >= sizeof(bs->filename)) {
+ json->str) >= sizeof(bs->filename)) {
 /* Give user a hint if we truncated things. */
 strcpy(bs->filename + sizeof(bs->filename) - 4, "...");
 }
-qobject_unref(json);
+g_string_free(json, true);
 }
 }
 
diff --git a/block/rbd.c b/block/rbd.c
index 9bd2bce716..9071a00e3f 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -232,7 +232,7 @@ static void qemu_rbd_parse_filename(const char *filename, 
QDict *options,
 
 if (keypairs) {
 qdict_put(options, "=keyvalue-pairs",
-  qobject_to_json(QOBJECT(keypairs)));
+  qstring_from_gstring(qobject_to_json(QOBJECT(keypairs;
 }
 
 done:
diff --git a/monitor/qmp.c b/monitor/qmp.c
index 1197c50b20..374bb4b81c 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -110,15 +110,15 @@ static void 
monitor_qmp_cleanup_queue_and_resume(MonitorQMP *mon)
 void qmp_send_response(MonitorQMP *mon, const QDict *rsp)
 {
 const QObject *data = QOBJECT(rsp);
-QString *json;
+GString *json;
 
 json = qobject_to_json_pretty(data, mon->pretty);
 assert(json != NULL);
 
-qstring_append_chr(json, '\n');
-monitor_puts(>common, qstring_get_str(json));
+g_string_append_c(json, '\n');
+monitor_puts(>common, json->str);
 
-qobject_unref(json);
+g_string_free(json, true);
 }
 
 /*
@@ -320,9 +320,9 @@ static void handle_qmp_command(void *opaque, QObject *req, 
Error *err)
 } /* else will fail qmp_dispatch() */
 
 if (req && trace_event_get_state_backends(TRACE_HANDLE_QMP_COMMAND)) {
-QString *req_json = qobject_to_json(req);
-trace_handle_qmp_command(mon, qstring_get_str(req_json));
-qobject_unref(req_json);
+GString *req_json = qobject_to_json(req);
+trace_handle_qmp_command(mon, req_json->str);
+g_string_free(req_json, true);
 }
 
 if (qdict && qmp_is_oob(qdict)) {
diff --git a/qemu-img.c b/qemu-img.c
index 1a59dfd3f3..8597d069af 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -33,7 +33,6 @@
 #include "qapi/qobject-output-visitor.h"
 #include "qapi/qmp/qjson.h"
 #include "qapi/qmp/qdict.h"
-#include "qapi/qmp/qstring.h"
 #include "qemu/cutils.h"
 #include "qemu/config-file.h"
 #include "qemu/option.h"
@@ -627,7 +626,7 @@ fail:
 
 static void dump_json_image_check(ImageCheck *check, bool quiet)
 {
-QString *str;
+GString *str;
 QObject *obj;
 Visitor *v = 

[PULL 22/33] qmp: Fix tracing of non-string command IDs

2020-12-19 Thread Markus Armbruster
Tracepoints monitor_qmp_cmd_in_band and
monitor_qmp_cmd_out_of_band (commit cf869d5317 "qmp: support
out-of-band (oob) execution") treat non-string "id" like absent "id".
Fix that.

Signed-off-by: Markus Armbruster 
Message-Id: <20201211171152.146877-10-arm...@redhat.com>
---
 monitor/qmp.c | 30 ++
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/monitor/qmp.c b/monitor/qmp.c
index 374bb4b81c..8f91af32be 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -31,7 +31,6 @@
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qjson.h"
 #include "qapi/qmp/qlist.h"
-#include "qapi/qmp/qstring.h"
 #include "trace.h"
 
 struct QMPRequest {
@@ -276,9 +275,15 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data)
 mon->qmp_requests->length == QMP_REQ_QUEUE_LEN_MAX - 1;
 qemu_mutex_unlock(>qmp_queue_lock);
 if (req_obj->req) {
-QDict *qdict = qobject_to(QDict, req_obj->req);
-QObject *id = qdict ? qdict_get(qdict, "id") : NULL;
-trace_monitor_qmp_cmd_in_band(qobject_get_try_str(id) ?: "");
+if (trace_event_get_state(TRACE_MONITOR_QMP_CMD_IN_BAND)) {
+QDict *qdict = qobject_to(QDict, req_obj->req);
+QObject *id = qdict ? qdict_get(qdict, "id") : NULL;
+GString *id_json;
+
+id_json = id ? qobject_to_json(id) : g_string_new(NULL);
+trace_monitor_qmp_cmd_in_band(id_json->str);
+g_string_free(id_json, true);
+}
 monitor_qmp_dispatch(mon, req_obj->req);
 } else {
 assert(req_obj->err);
@@ -308,17 +313,11 @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data)
 static void handle_qmp_command(void *opaque, QObject *req, Error *err)
 {
 MonitorQMP *mon = opaque;
-QObject *id = NULL;
-QDict *qdict;
+QDict *qdict = qobject_to(QDict, req);
 QMPRequest *req_obj;
 
 assert(!req != !err);
 
-qdict = qobject_to(QDict, req);
-if (qdict) {
-id = qdict_get(qdict, "id");
-} /* else will fail qmp_dispatch() */
-
 if (req && trace_event_get_state_backends(TRACE_HANDLE_QMP_COMMAND)) {
 GString *req_json = qobject_to_json(req);
 trace_handle_qmp_command(mon, req_json->str);
@@ -327,7 +326,14 @@ static void handle_qmp_command(void *opaque, QObject *req, 
Error *err)
 
 if (qdict && qmp_is_oob(qdict)) {
 /* OOB commands are executed immediately */
-trace_monitor_qmp_cmd_out_of_band(qobject_get_try_str(id) ?: "");
+if (trace_event_get_state(TRACE_MONITOR_QMP_CMD_OUT_OF_BAND)) {
+QObject *id = qdict_get(qdict, "id");
+GString *id_json;
+
+id_json = id ? qobject_to_json(id) : g_string_new(NULL);
+trace_monitor_qmp_cmd_out_of_band(id_json->str);
+g_string_free(id_json, true);
+}
 monitor_qmp_dispatch(mon, req);
 qobject_unref(req);
 return;
-- 
2.26.2




[PULL 33/33] qobject: Make QString immutable

2020-12-19 Thread Markus Armbruster
The functions to modify a QString's string are all unused now.  Drop
them, and make the string immutable.  Saves 16 bytes per QString on my
system.

Signed-off-by: Markus Armbruster 
Message-Id: <20201211171152.146877-21-arm...@redhat.com>
---
 include/qapi/qmp/qstring.h |  8 +
 qobject/qstring.c  | 65 ++
 tests/check-qobject.c  |  3 +-
 tests/check-qstring.c  | 16 --
 4 files changed, 4 insertions(+), 88 deletions(-)

diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h
index 53567db6c0..1d8ba46936 100644
--- a/include/qapi/qmp/qstring.h
+++ b/include/qapi/qmp/qstring.h
@@ -17,19 +17,13 @@
 
 struct QString {
 struct QObjectBase_ base;
-char *string;
-size_t length;
-size_t capacity;
+const char *string;
 };
 
 QString *qstring_new(void);
 QString *qstring_from_str(const char *str);
 QString *qstring_from_substr(const char *str, size_t start, size_t end);
 QString *qstring_from_gstring(GString *gstr);
-size_t qstring_get_length(const QString *qstring);
 const char *qstring_get_str(const QString *qstring);
-void qstring_append_int(QString *qstring, int64_t value);
-void qstring_append(QString *qstring, const char *str);
-void qstring_append_chr(QString *qstring, int c);
 
 #endif /* QSTRING_H */
diff --git a/qobject/qstring.c b/qobject/qstring.c
index ea86d80cf0..b4613899b9 100644
--- a/qobject/qstring.c
+++ b/qobject/qstring.c
@@ -24,14 +24,6 @@ QString *qstring_new(void)
 return qstring_from_str("");
 }
 
-/**
- * qstring_get_length(): Get the length of a QString
- */
-size_t qstring_get_length(const QString *qstring)
-{
-return qstring->length;
-}
-
 /**
  * qstring_from_substr(): Create a new QString from a C string substring
  *
@@ -42,18 +34,9 @@ QString *qstring_from_substr(const char *str, size_t start, 
size_t end)
 QString *qstring;
 
 assert(start <= end);
-
 qstring = g_malloc(sizeof(*qstring));
 qobject_init(QOBJECT(qstring), QTYPE_QSTRING);
-
-qstring->length = end - start;
-qstring->capacity = qstring->length;
-
-assert(qstring->capacity < SIZE_MAX);
-qstring->string = g_malloc(qstring->capacity + 1);
-memcpy(qstring->string, str + start, qstring->length);
-qstring->string[qstring->length] = 0;
-
+qstring->string = g_strndup(str + start, end - start);
 return qstring;
 }
 
@@ -79,55 +62,11 @@ QString *qstring_from_gstring(GString *gstr)
 
 qstring = g_malloc(sizeof(*qstring));
 qobject_init(QOBJECT(qstring), QTYPE_QSTRING);
-qstring->length = gstr->len;
-qstring->capacity = gstr->allocated_len;
 qstring->string = g_string_free(gstr, false);
 return qstring;
 }
 
 
-static void capacity_increase(QString *qstring, size_t len)
-{
-if (qstring->capacity < (qstring->length + len)) {
-assert(len <= SIZE_MAX - qstring->capacity);
-qstring->capacity += len;
-assert(qstring->capacity <= SIZE_MAX / 2);
-qstring->capacity *= 2; /* use exponential growth */
-
-qstring->string = g_realloc(qstring->string, qstring->capacity + 1);
-}
-}
-
-/* qstring_append(): Append a C string to a QString
- */
-void qstring_append(QString *qstring, const char *str)
-{
-size_t len = strlen(str);
-
-capacity_increase(qstring, len);
-memcpy(qstring->string + qstring->length, str, len);
-qstring->length += len;
-qstring->string[qstring->length] = 0;
-}
-
-void qstring_append_int(QString *qstring, int64_t value)
-{
-char num[32];
-
-snprintf(num, sizeof(num), "%" PRId64, value);
-qstring_append(qstring, num);
-}
-
-/**
- * qstring_append_chr(): Append a C char to a QString
- */
-void qstring_append_chr(QString *qstring, int c)
-{
-capacity_increase(qstring, 1);
-qstring->string[qstring->length++] = c;
-qstring->string[qstring->length] = 0;
-}
-
 /**
  * qstring_get_str(): Return a pointer to the stored string
  *
@@ -158,6 +97,6 @@ void qstring_destroy_obj(QObject *obj)
 
 assert(obj != NULL);
 qs = qobject_to(QString, obj);
-g_free(qs->string);
+g_free((char *)qs->string);
 g_free(qs);
 }
diff --git a/tests/check-qobject.c b/tests/check-qobject.c
index 6b6deaeb8b..c1713d15af 100644
--- a/tests/check-qobject.c
+++ b/tests/check-qobject.c
@@ -155,8 +155,7 @@ static void qobject_is_equal_string_test(void)
 str_case = qstring_from_str("Foo");
 
 /* Should yield "foo" */
-str_built = qstring_from_substr("form", 0, 2);
-qstring_append_chr(str_built, 'o');
+str_built = qstring_from_substr("buffoon", 3, 6);
 
 check_unequal(str_base, str_whitespace_0, str_whitespace_1,
   str_whitespace_2, str_whitespace_3, str_case);
diff --git a/tests/check-qstring.c b/tests/check-qstring.c
index 2d079921e3..4bf9772093 100644
--- a/tests/check-qstring.c
+++ b/tests/check-qstring.c
@@ -47,21 +47,6 @@ static void qstring_get_str_test(void)
 qobject_unref(qstring);
 }
 
-static void qstring_append_chr_test(void)
-{
-

[PULL 30/33] json: Use GString instead of QString to accumulate strings

2020-12-19 Thread Markus Armbruster
QString supports modifying its string, but it's quite limited: you can
only append.  The remaining callers use it for building an initial
string, never for modifying it later.

Change parse_string() to do build the initial string with GString.
This is another step towards making QString immutable.

Signed-off-by: Markus Armbruster 
Message-Id: <20201211171152.146877-18-arm...@redhat.com>
---
 qobject/json-parser.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/qobject/json-parser.c b/qobject/json-parser.c
index c0f521b56b..d351039b10 100644
--- a/qobject/json-parser.c
+++ b/qobject/json-parser.c
@@ -130,7 +130,7 @@ static int cvt4hex(const char *s)
 static QString *parse_string(JSONParserContext *ctxt, JSONToken *token)
 {
 const char *ptr = token->str;
-QString *str;
+GString *str;
 char quote;
 const char *beg;
 int cp, trailing;
@@ -140,7 +140,7 @@ static QString *parse_string(JSONParserContext *ctxt, 
JSONToken *token)
 
 assert(*ptr == '"' || *ptr == '\'');
 quote = *ptr++;
-str = qstring_new();
+str = g_string_new(NULL);
 
 while (*ptr != quote) {
 assert(*ptr);
@@ -149,31 +149,31 @@ static QString *parse_string(JSONParserContext *ctxt, 
JSONToken *token)
 beg = ptr++;
 switch (*ptr++) {
 case '"':
-qstring_append_chr(str, '"');
+g_string_append_c(str, '"');
 break;
 case '\'':
-qstring_append_chr(str, '\'');
+g_string_append_c(str, '\'');
 break;
 case '\\':
-qstring_append_chr(str, '\\');
+g_string_append_c(str, '\\');
 break;
 case '/':
-qstring_append_chr(str, '/');
+g_string_append_c(str, '/');
 break;
 case 'b':
-qstring_append_chr(str, '\b');
+g_string_append_c(str, '\b');
 break;
 case 'f':
-qstring_append_chr(str, '\f');
+g_string_append_c(str, '\f');
 break;
 case 'n':
-qstring_append_chr(str, '\n');
+g_string_append_c(str, '\n');
 break;
 case 'r':
-qstring_append_chr(str, '\r');
+g_string_append_c(str, '\r');
 break;
 case 't':
-qstring_append_chr(str, '\t');
+g_string_append_c(str, '\t');
 break;
 case 'u':
 cp = cvt4hex(ptr);
@@ -200,7 +200,7 @@ static QString *parse_string(JSONParserContext *ctxt, 
JSONToken *token)
 (int)(ptr - beg), beg);
 goto out;
 }
-qstring_append(str, utf8_buf);
+g_string_append(str, utf8_buf);
 break;
 default:
 parse_error(ctxt, token, "invalid escape sequence in string");
@@ -225,14 +225,14 @@ static QString *parse_string(JSONParserContext *ctxt, 
JSONToken *token)
 ptr = end;
 len = mod_utf8_encode(utf8_buf, sizeof(utf8_buf), cp);
 assert(len >= 0);
-qstring_append(str, utf8_buf);
+g_string_append(str, utf8_buf);
 }
 }
 
-return str;
+return qstring_from_gstring(str);
 
 out:
-qobject_unref(str);
+g_string_free(str, true);
 return NULL;
 }
 
-- 
2.26.2




[PULL 14/33] hmp: Simplify how qmp_human_monitor_command() gets output

2020-12-19 Thread Markus Armbruster
Commit 48c043d0d1 "hmp: human-monitor-command: stop using the Memory
chardev driver" left us "if string is non-empty, duplicate it, else
duplicate the empty string".  Meh.  Duplicate it unconditionally.

Cc: Dr. David Alan Gilbert 
Signed-off-by: Markus Armbruster 
Message-Id: <20201211171152.146877-2-arm...@redhat.com>
Reviewed-by: Dr. David Alan Gilbert 
---
 monitor/misc.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/monitor/misc.c b/monitor/misc.c
index f2ee7cd77a..33d0dae2e8 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -136,11 +136,7 @@ char *qmp_human_monitor_command(const char *command_line, 
bool has_cpu_index,
 handle_hmp_command(, command_line);
 
 WITH_QEMU_LOCK_GUARD(_lock) {
-if (qstring_get_length(hmp.common.outbuf) > 0) {
-output = g_strdup(qstring_get_str(hmp.common.outbuf));
-} else {
-output = g_strdup("");
-}
+output = g_strdup(qstring_get_str(hmp.common.outbuf));
 }
 
 out:
-- 
2.26.2




[PULL 31/33] keyval: Use GString to accumulate value strings

2020-12-19 Thread Markus Armbruster
QString supports modifying its string, but it's quite limited: you can
only append.  The remaining callers use it for building an initial
string, never for modifying it later.

Change keyval_parse_one() to do build the initial string with GString.
This is another step towards making QString immutable.

Signed-off-by: Markus Armbruster 
Message-Id: <20201211171152.146877-19-arm...@redhat.com>
---
 util/keyval.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/util/keyval.c b/util/keyval.c
index 7f625ad33c..be34928813 100644
--- a/util/keyval.c
+++ b/util/keyval.c
@@ -189,7 +189,7 @@ static const char *keyval_parse_one(QDict *qdict, const 
char *params,
 QDict *cur;
 int ret;
 QObject *next;
-QString *val;
+GString *val;
 
 key = params;
 val_end = NULL;
@@ -263,7 +263,7 @@ static const char *keyval_parse_one(QDict *qdict, const 
char *params,
 
 if (key == implied_key) {
 assert(!*s);
-val = qstring_from_substr(params, 0, val_end - params);
+val = g_string_new_len(params, val_end - params);
 s = val_end;
 if (*s == ',') {
 s++;
@@ -276,7 +276,7 @@ static const char *keyval_parse_one(QDict *qdict, const 
char *params,
 }
 s++;
 
-val = qstring_new();
+val = g_string_new(NULL);
 for (;;) {
 if (!*s) {
 break;
@@ -286,11 +286,12 @@ static const char *keyval_parse_one(QDict *qdict, const 
char *params,
 break;
 }
 }
-qstring_append_chr(val, *s++);
+g_string_append_c(val, *s++);
 }
 }
 
-if (!keyval_parse_put(cur, key_in_cur, val, key, key_end, errp)) {
+if (!keyval_parse_put(cur, key_in_cur, qstring_from_gstring(val),
+  key, key_end, errp)) {
 return NULL;
 }
 return s;
-- 
2.26.2




[PULL 20/33] hw/rdma: Replace QList by GQueue

2020-12-19 Thread Markus Armbruster
RdmaProtectedQList provides a thread-safe queue of int64_t on top of a
QList.

rdma_protected_qlist_destroy() calls qlist_destroy_obj() directly.
qlist_destroy_obj() is actually for use by qobject_destroy() only.
The next commit will make that obvious.

The minimal fix would be calling qobject_unref() instead.  But QList
is actually a bad fit here.  It's designed for representing JSON
arrays.  We're better off with a GQueue here.  Replace.

Cc: Yuval Shaia 
Cc: Marcel Apfelbaum 
Signed-off-by: Markus Armbruster 
Message-Id: <20201211171152.146877-8-arm...@redhat.com>
---
 hw/rdma/rdma_backend_defs.h |  2 +-
 hw/rdma/rdma_utils.h| 15 ---
 hw/rdma/rdma_backend.c  | 10 +-
 hw/rdma/rdma_utils.c| 29 -
 4 files changed, 30 insertions(+), 26 deletions(-)

diff --git a/hw/rdma/rdma_backend_defs.h b/hw/rdma/rdma_backend_defs.h
index 0b55be3503..4e6c0ad695 100644
--- a/hw/rdma/rdma_backend_defs.h
+++ b/hw/rdma/rdma_backend_defs.h
@@ -43,7 +43,7 @@ typedef struct RdmaBackendDev {
 struct ibv_context *context;
 struct ibv_comp_channel *channel;
 uint8_t port_num;
-RdmaProtectedQList recv_mads_list;
+RdmaProtectedGQueue recv_mads_list;
 RdmaCmMux rdmacm_mux;
 } RdmaBackendDev;
 
diff --git a/hw/rdma/rdma_utils.h b/hw/rdma/rdma_utils.h
index e7babe96cb..9fd0efd940 100644
--- a/hw/rdma/rdma_utils.h
+++ b/hw/rdma/rdma_utils.h
@@ -28,10 +28,10 @@
 #define rdma_info_report(fmt, ...) \
 info_report("%s: " fmt, "rdma", ## __VA_ARGS__)
 
-typedef struct RdmaProtectedQList {
+typedef struct RdmaProtectedGQueue {
 QemuMutex lock;
-QList *list;
-} RdmaProtectedQList;
+GQueue *list;
+} RdmaProtectedGQueue;
 
 typedef struct RdmaProtectedGSList {
 QemuMutex lock;
@@ -40,10 +40,11 @@ typedef struct RdmaProtectedGSList {
 
 void *rdma_pci_dma_map(PCIDevice *dev, dma_addr_t addr, dma_addr_t plen);
 void rdma_pci_dma_unmap(PCIDevice *dev, void *buffer, dma_addr_t len);
-void rdma_protected_qlist_init(RdmaProtectedQList *list);
-void rdma_protected_qlist_destroy(RdmaProtectedQList *list);
-void rdma_protected_qlist_append_int64(RdmaProtectedQList *list, int64_t 
value);
-int64_t rdma_protected_qlist_pop_int64(RdmaProtectedQList *list);
+void rdma_protected_gqueue_init(RdmaProtectedGQueue *list);
+void rdma_protected_gqueue_destroy(RdmaProtectedGQueue *list);
+void rdma_protected_gqueue_append_int64(RdmaProtectedGQueue *list,
+int64_t value);
+int64_t rdma_protected_gqueue_pop_int64(RdmaProtectedGQueue *list);
 void rdma_protected_gslist_init(RdmaProtectedGSList *list);
 void rdma_protected_gslist_destroy(RdmaProtectedGSList *list);
 void rdma_protected_gslist_append_int32(RdmaProtectedGSList *list,
diff --git a/hw/rdma/rdma_backend.c b/hw/rdma/rdma_backend.c
index 5de010b1fa..6dcdfbbbe2 100644
--- a/hw/rdma/rdma_backend.c
+++ b/hw/rdma/rdma_backend.c
@@ -78,7 +78,7 @@ static void clean_recv_mads(RdmaBackendDev *backend_dev)
 unsigned long cqe_ctx_id;
 
 do {
-cqe_ctx_id = rdma_protected_qlist_pop_int64(_dev->
+cqe_ctx_id = rdma_protected_gqueue_pop_int64(_dev->
 recv_mads_list);
 if (cqe_ctx_id != -ENOENT) {
 qatomic_inc(_dev->rdma_dev_res->stats.missing_cqe);
@@ -597,7 +597,7 @@ static unsigned int save_mad_recv_buffer(RdmaBackendDev 
*backend_dev,
 bctx->up_ctx = ctx;
 bctx->sge = *sge;
 
-rdma_protected_qlist_append_int64(_dev->recv_mads_list, bctx_id);
+rdma_protected_gqueue_append_int64(_dev->recv_mads_list, bctx_id);
 
 return 0;
 }
@@ -,7 +,7 @@ static void process_incoming_mad_req(RdmaBackendDev 
*backend_dev,
 
 trace_mad_message("recv", msg->umad.mad, msg->umad_len);
 
-cqe_ctx_id = rdma_protected_qlist_pop_int64(_dev->recv_mads_list);
+cqe_ctx_id = rdma_protected_gqueue_pop_int64(_dev->recv_mads_list);
 if (cqe_ctx_id == -ENOENT) {
 rdma_warn_report("No more free MADs buffers, waiting for a while");
 sleep(THR_POLL_TO);
@@ -1185,7 +1185,7 @@ static int mad_init(RdmaBackendDev *backend_dev, 
CharBackend *mad_chr_be)
 return -EIO;
 }
 
-rdma_protected_qlist_init(_dev->recv_mads_list);
+rdma_protected_gqueue_init(_dev->recv_mads_list);
 
 enable_rdmacm_mux_async(backend_dev);
 
@@ -1205,7 +1205,7 @@ static void mad_fini(RdmaBackendDev *backend_dev)
 {
 disable_rdmacm_mux_async(backend_dev);
 qemu_chr_fe_disconnect(backend_dev->rdmacm_mux.chr_be);
-rdma_protected_qlist_destroy(_dev->recv_mads_list);
+rdma_protected_gqueue_destroy(_dev->recv_mads_list);
 }
 
 int rdma_backend_get_gid_index(RdmaBackendDev *backend_dev,
diff --git a/hw/rdma/rdma_utils.c b/hw/rdma/rdma_utils.c
index 698ed4716c..98df58f689 100644
--- a/hw/rdma/rdma_utils.c
+++ b/hw/rdma/rdma_utils.c
@@ -14,8 +14,6 @@
  */
 
 #include "qemu/osdep.h"
-#include "qapi/qmp/qlist.h"
-#include "qapi/qmp/qnum.h"
 #include "trace.h"
 

[PULL 28/33] qobject: Factor JSON writer out of qobject_to_json()

2020-12-19 Thread Markus Armbruster
We have two JSON writers written in C: qobject/qjson.c provides
qobject_to_json(), and migration/qjson.c provides a more low level
imperative interface.  They don't share code.  The latter tacitly
limits numbers to int64_t, and strings contents to characters that
don't need escaping.

Factor out qobject_to_json()'s JSON writer as qobject/json-writer.c.
Straightforward, except for numbers: since the writer is to be
independent of QObject, it can't use qnum_to_string().  Open-code it
instead.  This is actually an improvement of sorts, because it
liberates qnum_to_string() from JSON's needs: its JSON-related FIXMEs
move to the JSON writer, where they belong.

The next commit will replace migration/qjson.c.

Signed-off-by: Markus Armbruster 
Message-Id: <20201211171152.146877-16-arm...@redhat.com>
---
 include/qapi/qmp/json-writer.h |  37 +
 qobject/json-writer.c  | 247 +
 qobject/qjson.c| 125 +
 qobject/qnum.c |   5 -
 qobject/meson.build|   5 +-
 5 files changed, 318 insertions(+), 101 deletions(-)
 create mode 100644 include/qapi/qmp/json-writer.h
 create mode 100644 qobject/json-writer.c

diff --git a/include/qapi/qmp/json-writer.h b/include/qapi/qmp/json-writer.h
new file mode 100644
index 00..708d129018
--- /dev/null
+++ b/include/qapi/qmp/json-writer.h
@@ -0,0 +1,37 @@
+/*
+ * JSON Writer
+ *
+ * Copyright (c) 2020 Red Hat Inc.
+ *
+ * Authors:
+ *  Markus Armbruster 
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#ifndef JSON_WRITER_H
+#define JSON_WRITER_H
+
+typedef struct JSONWriter JSONWriter;
+
+JSONWriter *json_writer_new(bool pretty);
+const char *json_writer_get(JSONWriter *);
+GString *json_writer_get_and_free(JSONWriter *);
+void json_writer_free(JSONWriter *);
+
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(JSONWriter, json_writer_free)
+
+void json_writer_start_object(JSONWriter *, const char *name);
+void json_writer_end_object(JSONWriter *);
+void json_writer_start_array(JSONWriter *, const char *name);
+void json_writer_end_array(JSONWriter *);
+void json_writer_bool(JSONWriter *, const char *name, bool val);
+void json_writer_null(JSONWriter *, const char *name);
+void json_writer_int64(JSONWriter *, const char *name, int64_t val);
+void json_writer_uint64(JSONWriter *, const char *name, uint64_t val);
+void json_writer_double(JSONWriter *, const char *name, double val);
+void json_writer_str(JSONWriter *, const char *name, const char *str);
+
+#endif
diff --git a/qobject/json-writer.c b/qobject/json-writer.c
new file mode 100644
index 00..309a31d57a
--- /dev/null
+++ b/qobject/json-writer.c
@@ -0,0 +1,247 @@
+/*
+ * JSON Writer
+ *
+ * Copyright IBM, Corp. 2009
+ * Copyright (c) 2010-2020 Red Hat Inc.
+ *
+ * Authors:
+ *  Anthony Liguori   
+ *  Markus Armbruster 
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/qmp/json-writer.h"
+#include "qemu/unicode.h"
+
+struct JSONWriter {
+bool pretty;
+bool need_comma;
+GString *contents;
+GByteArray *container_is_array;
+};
+
+JSONWriter *json_writer_new(bool pretty)
+{
+JSONWriter *writer = g_new(JSONWriter, 1);
+
+writer->pretty = pretty;
+writer->need_comma = false;
+writer->contents = g_string_new(NULL);
+writer->container_is_array = g_byte_array_new();
+return writer;
+}
+
+const char *json_writer_get(JSONWriter *writer)
+{
+g_assert(!writer->container_is_array->len);
+return writer->contents->str;
+}
+
+GString *json_writer_get_and_free(JSONWriter *writer)
+{
+GString *contents = writer->contents;
+
+writer->contents = NULL;
+g_byte_array_free(writer->container_is_array, true);
+g_free(writer);
+return contents;
+}
+
+void json_writer_free(JSONWriter *writer)
+{
+if (writer) {
+g_string_free(json_writer_get_and_free(writer), true);
+}
+}
+
+static void enter_container(JSONWriter *writer, bool is_array)
+{
+unsigned depth = writer->container_is_array->len;
+
+g_byte_array_set_size(writer->container_is_array, depth + 1);
+writer->container_is_array->data[depth] = is_array;
+writer->need_comma = false;
+}
+
+static void leave_container(JSONWriter *writer, bool is_array)
+{
+unsigned depth = writer->container_is_array->len;
+
+assert(depth);
+assert(writer->container_is_array->data[depth - 1] == is_array);
+g_byte_array_set_size(writer->container_is_array, depth - 1);
+writer->need_comma = true;
+}
+
+static bool in_object(JSONWriter *writer)
+{
+unsigned depth = writer->container_is_array->len;
+
+return depth && !writer->container_is_array->data[depth - 1];
+}
+
+static void pretty_newline(JSONWriter *writer)
+{
+if (writer->pretty) {
+

[PULL 15/33] monitor: Use GString instead of QString for output buffer

2020-12-19 Thread Markus Armbruster
GString has a richer set of string operations than QString.  It should
be preferred to QString except where we need a QObject or reference
counting.  We don't here.  Switch to GString, and put its richer
interface to use.

Cc: Dr. David Alan Gilbert 
Signed-off-by: Markus Armbruster 
Message-Id: <20201211171152.146877-3-arm...@redhat.com>
Reviewed-by: Dr. David Alan Gilbert 
---
 monitor/monitor-internal.h |  2 +-
 monitor/misc.c |  2 +-
 monitor/monitor.c  | 20 
 3 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index a6131554da..40903d6386 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -105,7 +105,7 @@ struct Monitor {
  * Members that are protected by the per-monitor lock
  */
 QLIST_HEAD(, mon_fd_t) fds;
-QString *outbuf;
+GString *outbuf;
 guint out_watch;
 /* Read under either BQL or mon_lock, written with BQL+mon_lock.  */
 int mux_out;
diff --git a/monitor/misc.c b/monitor/misc.c
index 33d0dae2e8..10444b4e7a 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -136,7 +136,7 @@ char *qmp_human_monitor_command(const char *command_line, 
bool has_cpu_index,
 handle_hmp_command(, command_line);
 
 WITH_QEMU_LOCK_GUARD(_lock) {
-output = g_strdup(qstring_get_str(hmp.common.outbuf));
+output = g_strdup(hmp.common.outbuf->str);
 }
 
 out:
diff --git a/monitor/monitor.c b/monitor/monitor.c
index 84222cd130..1e4a6b3f20 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -29,7 +29,6 @@
 #include "qapi/qapi-emit-events.h"
 #include "qapi/qapi-visit-control.h"
 #include "qapi/qmp/qdict.h"
-#include "qapi/qmp/qstring.h"
 #include "qemu/error-report.h"
 #include "qemu/option.h"
 #include "sysemu/qtest.h"
@@ -181,22 +180,19 @@ static void monitor_flush_locked(Monitor *mon)
 return;
 }
 
-buf = qstring_get_str(mon->outbuf);
-len = qstring_get_length(mon->outbuf);
+buf = mon->outbuf->str;
+len = mon->outbuf->len;
 
 if (len && !mon->mux_out) {
 rc = qemu_chr_fe_write(>chr, (const uint8_t *) buf, len);
 if ((rc < 0 && errno != EAGAIN) || (rc == len)) {
 /* all flushed or error */
-qobject_unref(mon->outbuf);
-mon->outbuf = qstring_new();
+g_string_truncate(mon->outbuf, 0);
 return;
 }
 if (rc > 0) {
 /* partial write */
-QString *tmp = qstring_from_str(buf + rc);
-qobject_unref(mon->outbuf);
-mon->outbuf = tmp;
+g_string_erase(mon->outbuf, 0, rc);
 }
 if (mon->out_watch == 0) {
 mon->out_watch =
@@ -223,9 +219,9 @@ int monitor_puts(Monitor *mon, const char *str)
 for (i = 0; str[i]; i++) {
 c = str[i];
 if (c == '\n') {
-qstring_append_chr(mon->outbuf, '\r');
+g_string_append_c(mon->outbuf, '\r');
 }
-qstring_append_chr(mon->outbuf, c);
+g_string_append_c(mon->outbuf, c);
 if (c == '\n') {
 monitor_flush_locked(mon);
 }
@@ -602,7 +598,7 @@ void monitor_data_init(Monitor *mon, bool is_qmp, bool 
skip_flush,
 }
 qemu_mutex_init(>mon_lock);
 mon->is_qmp = is_qmp;
-mon->outbuf = qstring_new();
+mon->outbuf = g_string_new(NULL);
 mon->skip_flush = skip_flush;
 mon->use_io_thread = use_io_thread;
 }
@@ -616,7 +612,7 @@ void monitor_data_destroy(Monitor *mon)
 } else {
 readline_free(container_of(mon, MonitorHMP, common)->rs);
 }
-qobject_unref(mon->outbuf);
+g_string_free(mon->outbuf, true);
 qemu_mutex_destroy(>mon_lock);
 }
 
-- 
2.26.2




[PULL 19/33] Revert "qstring: add qstring_free()"

2020-12-19 Thread Markus Armbruster
This reverts commit 164c374b75f87c6765a705c4418ab7005a2d356f.

A free function for a reference-counted object is in bad taste.
Fortunately, this one is now also unused.  Drop it.

Signed-off-by: Markus Armbruster 
Message-Id: <20201211171152.146877-7-arm...@redhat.com>
---
 include/qapi/qmp/qstring.h |  1 -
 qobject/qstring.c  | 27 +--
 2 files changed, 5 insertions(+), 23 deletions(-)

diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h
index ae7698d6c7..ae5b4b44d2 100644
--- a/include/qapi/qmp/qstring.h
+++ b/include/qapi/qmp/qstring.h
@@ -34,7 +34,6 @@ void qstring_append_int(QString *qstring, int64_t value);
 void qstring_append(QString *qstring, const char *str);
 void qstring_append_chr(QString *qstring, int c);
 bool qstring_is_equal(const QObject *x, const QObject *y);
-char *qstring_free(QString *qstring, bool return_str);
 void qstring_destroy_obj(QObject *obj);
 
 #endif /* QSTRING_H */
diff --git a/qobject/qstring.c b/qobject/qstring.c
index af7c18ca73..c1891beda0 100644
--- a/qobject/qstring.c
+++ b/qobject/qstring.c
@@ -168,33 +168,16 @@ bool qstring_is_equal(const QObject *x, const QObject *y)
qobject_to(QString, y)->string);
 }
 
-/**
- * qstring_free(): Free the memory allocated by a QString object
- *
- * Return: if @return_str, return the underlying string, to be
- * g_free(), otherwise NULL is returned.
- */
-char *qstring_free(QString *qstring, bool return_str)
-{
-char *rv = NULL;
-
-if (return_str) {
-rv = qstring->string;
-} else {
-g_free(qstring->string);
-}
-
-g_free(qstring);
-
-return rv;
-}
-
 /**
  * qstring_destroy_obj(): Free all memory allocated by a QString
  * object
  */
 void qstring_destroy_obj(QObject *obj)
 {
+QString *qs;
+
 assert(obj != NULL);
-qstring_free(qobject_to(QString, obj), FALSE);
+qs = qobject_to(QString, obj);
+g_free(qs->string);
+g_free(qs);
 }
-- 
2.26.2




[PULL 23/33] block: Avoid qobject_get_try_str()

2020-12-19 Thread Markus Armbruster
I'm about to remove qobject_get_try_str().  Use qstring_get_str()
instead.  Safe because the argument is known to be a QString here.

Cc: Kevin Wolf 
Cc: Max Reitz 
Cc: qemu-bl...@nongnu.org
Signed-off-by: Markus Armbruster 
Message-Id: <20201211171152.146877-11-arm...@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
 block.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 09d4c6bd25..934d1bcc8f 100644
--- a/block.c
+++ b/block.c
@@ -4021,7 +4021,7 @@ static int bdrv_reopen_parse_backing(BDRVReopenState 
*reopen_state,
 new_backing_bs = NULL;
 break;
 case QTYPE_QSTRING:
-str = qobject_get_try_str(value);
+str = qstring_get_str(qobject_to(QString, value));
 new_backing_bs = bdrv_lookup_bs(NULL, str, errp);
 if (new_backing_bs == NULL) {
 return -EINVAL;
@@ -4284,8 +4284,8 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, 
BlockReopenQueue *queue,
 }
 
 if (child) {
-const char *str = qobject_get_try_str(new);
-if (!strcmp(child->bs->node_name, str)) {
+if (!strcmp(child->bs->node_name,
+qstring_get_str(qobject_to(QString, new {
 continue; /* Found child with this name, skip option */
 }
 }
-- 
2.26.2




[PULL 27/33] qobject: Factor quoted_str() out of to_json()

2020-12-19 Thread Markus Armbruster
Signed-off-by: Markus Armbruster 
Message-Id: <20201211171152.146877-15-arm...@redhat.com>
---
 qobject/qjson.c | 110 
 1 file changed, 54 insertions(+), 56 deletions(-)

diff --git a/qobject/qjson.c b/qobject/qjson.c
index 2f690c1816..962214f5a7 100644
--- a/qobject/qjson.c
+++ b/qobject/qjson.c
@@ -156,6 +156,58 @@ static void json_pretty_newline(GString *accu, bool 
pretty, int indent)
 }
 }
 
+static void quoted_str(const char *str, GString *accu)
+{
+const char *ptr;
+int cp;
+char *end;
+
+g_string_append_c(accu, '"');
+
+for (ptr = str; *ptr; ptr = end) {
+cp = mod_utf8_codepoint(ptr, 6, );
+switch (cp) {
+case '\"':
+g_string_append(accu, "\\\"");
+break;
+case '\\':
+g_string_append(accu, "");
+break;
+case '\b':
+g_string_append(accu, "\\b");
+break;
+case '\f':
+g_string_append(accu, "\\f");
+break;
+case '\n':
+g_string_append(accu, "\\n");
+break;
+case '\r':
+g_string_append(accu, "\\r");
+break;
+case '\t':
+g_string_append(accu, "\\t");
+break;
+default:
+if (cp < 0) {
+cp = 0xFFFD; /* replacement character */
+}
+if (cp > 0x) {
+/* beyond BMP; need a surrogate pair */
+g_string_append_printf(accu, "\\u%04X\\u%04X",
+   0xD800 + ((cp - 0x1) >> 10),
+   0xDC00 + ((cp - 0x1) & 0x3FF));
+} else if (cp < 0x20 || cp >= 0x7F) {
+g_string_append_printf(accu, "\\u%04X", cp);
+} else {
+g_string_append_c(accu, cp);
+}
+}
+};
+
+g_string_append_c(accu, '"');
+}
+
 static void to_json(const QObject *obj, GString *accu, bool pretty, int indent)
 {
 switch (qobject_type(obj)) {
@@ -170,56 +222,7 @@ static void to_json(const QObject *obj, GString *accu, 
bool pretty, int indent)
 break;
 }
 case QTYPE_QSTRING: {
-QString *val = qobject_to(QString, obj);
-const char *ptr;
-int cp;
-char *end;
-
-ptr = qstring_get_str(val);
-g_string_append_c(accu, '"');
-
-for (; *ptr; ptr = end) {
-cp = mod_utf8_codepoint(ptr, 6, );
-switch (cp) {
-case '\"':
-g_string_append(accu, "\\\"");
-break;
-case '\\':
-g_string_append(accu, "");
-break;
-case '\b':
-g_string_append(accu, "\\b");
-break;
-case '\f':
-g_string_append(accu, "\\f");
-break;
-case '\n':
-g_string_append(accu, "\\n");
-break;
-case '\r':
-g_string_append(accu, "\\r");
-break;
-case '\t':
-g_string_append(accu, "\\t");
-break;
-default:
-if (cp < 0) {
-cp = 0xFFFD; /* replacement character */
-}
-if (cp > 0x) {
-/* beyond BMP; need a surrogate pair */
-g_string_append_printf(accu, "\\u%04X\\u%04X",
-   0xD800 + ((cp - 0x1) >> 10),
-   0xDC00 + ((cp - 0x1) & 0x3FF));
-} else if (cp < 0x20 || cp >= 0x7F) {
-g_string_append_printf(accu, "\\u%04X", cp);
-} else {
-g_string_append_c(accu, cp);
-}
-}
-};
-
-g_string_append_c(accu, '"');
+quoted_str(qstring_get_str(qobject_to(QString, obj)), accu);
 break;
 }
 case QTYPE_QDICT: {
@@ -227,7 +230,6 @@ static void to_json(const QObject *obj, GString *accu, bool 
pretty, int indent)
 const char *comma = pretty ? "," : ", ";
 const char *sep = "";
 const QDictEntry *entry;
-QString *qkey;
 
 g_string_append_c(accu, '{');
 
@@ -236,11 +238,7 @@ static void to_json(const QObject *obj, GString *accu, 
bool pretty, int indent)
  entry = qdict_next(val, entry)) {
 g_string_append(accu, sep);
 json_pretty_newline(accu, pretty, indent + 1);
-
-qkey = qstring_from_str(qdict_entry_key(entry));
-to_json(QOBJECT(qkey), accu, pretty, indent + 1);
-qobject_unref(qkey);
-
+quoted_str(qdict_entry_key(entry), accu);
 g_string_append(accu, ": ");
 to_json(qdict_entry_value(entry), accu, pretty, indent + 1);
 sep = comma;
-- 
2.26.2




[PULL 13/33] test-visitor-serialization: Clean up test_primitives()

2020-12-19 Thread Markus Armbruster
test_primitives() uses union member intmax_t max to compare the
integer members.  Unspecified behavior.  Has worked fine for many
years, though.  Clean it up.

Signed-off-by: Markus Armbruster 
Message-Id: <20201210161452.2813491-11-arm...@redhat.com>
---
 tests/test-visitor-serialization.c | 44 +-
 1 file changed, 37 insertions(+), 7 deletions(-)

diff --git a/tests/test-visitor-serialization.c 
b/tests/test-visitor-serialization.c
index cf19924068..dfe120a50d 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -55,7 +55,6 @@ typedef struct PrimitiveType {
 int16_t s16;
 int32_t s32;
 int64_t s64;
-intmax_t max;
 } value;
 enum PrimitiveTypeKind type;
 const char *description;
@@ -307,15 +306,46 @@ static void test_primitives(gconstpointer opaque)
  _abort);
 
 g_assert(pt_copy != NULL);
-if (pt->type == PTYPE_STRING) {
+switch (pt->type) {
+case PTYPE_STRING:
 g_assert_cmpstr(pt->value.string, ==, pt_copy->value.string);
 g_free((char *)pt_copy->value.string);
-} else if (pt->type == PTYPE_NUMBER) {
+break;
+case PTYPE_BOOLEAN:
+g_assert_cmpint(pt->value.boolean, ==, pt->value.boolean);
+break;
+case PTYPE_NUMBER:
 g_assert_cmpfloat(pt->value.number, ==, pt_copy->value.number);
-} else if (pt->type == PTYPE_BOOLEAN) {
-g_assert_cmpint(!!pt->value.max, ==, !!pt->value.max);
-} else {
-g_assert_cmpint(pt->value.max, ==, pt_copy->value.max);
+break;
+case PTYPE_INTEGER:
+g_assert_cmpint(pt->value.integer, ==, pt_copy->value.integer);
+break;
+case PTYPE_U8:
+g_assert_cmpuint(pt->value.u8, ==, pt_copy->value.u8);
+break;
+case PTYPE_U16:
+g_assert_cmpuint(pt->value.u16, ==, pt_copy->value.u16);
+break;
+case PTYPE_U32:
+g_assert_cmpuint(pt->value.u32, ==, pt_copy->value.u32);
+break;
+case PTYPE_U64:
+g_assert_cmpuint(pt->value.u64, ==, pt_copy->value.u64);
+break;
+case PTYPE_S8:
+g_assert_cmpint(pt->value.s8, ==, pt_copy->value.s8);
+break;
+case PTYPE_S16:
+g_assert_cmpint(pt->value.s16, ==, pt_copy->value.s16);
+break;
+case PTYPE_S32:
+g_assert_cmpint(pt->value.s32, ==, pt_copy->value.s32);
+break;
+case PTYPE_S64:
+g_assert_cmpint(pt->value.s64, ==, pt_copy->value.s64);
+break;
+case PTYPE_EOL:
+g_assert_not_reached();
 }
 
 ops->cleanup(serialize_data);
-- 
2.26.2




[PULL 26/33] qobject: Drop qstring_get_try_str()

2020-12-19 Thread Markus Armbruster
No users left outside tests/, and the ones in tests/ can just as well
use qstring_get_str().  Do that, and drop the function.

Signed-off-by: Markus Armbruster 
Message-Id: <20201211171152.146877-14-arm...@redhat.com>
---
 include/qapi/qmp/qstring.h |  1 -
 qobject/qstring.c  | 10 --
 tests/check-qjson.c| 11 +--
 3 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h
index 56034dae54..53567db6c0 100644
--- a/include/qapi/qmp/qstring.h
+++ b/include/qapi/qmp/qstring.h
@@ -28,7 +28,6 @@ QString *qstring_from_substr(const char *str, size_t start, 
size_t end);
 QString *qstring_from_gstring(GString *gstr);
 size_t qstring_get_length(const QString *qstring);
 const char *qstring_get_str(const QString *qstring);
-const char *qstring_get_try_str(const QString *qstring);
 void qstring_append_int(QString *qstring, int64_t value);
 void qstring_append(QString *qstring, const char *str);
 void qstring_append_chr(QString *qstring, int c);
diff --git a/qobject/qstring.c b/qobject/qstring.c
index cfe3f3bf00..ea86d80cf0 100644
--- a/qobject/qstring.c
+++ b/qobject/qstring.c
@@ -139,16 +139,6 @@ const char *qstring_get_str(const QString *qstring)
 return qstring->string;
 }
 
-/**
- * qstring_get_try_str(): Return a pointer to the stored string
- *
- * NOTE: will return NULL if qstring is not provided.
- */
-const char *qstring_get_try_str(const QString *qstring)
-{
-return qstring ? qstring_get_str(qstring) : NULL;
-}
-
 /**
  * qstring_is_equal(): Test whether the two QStrings are equal
  */
diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index 337add0838..c845f91d43 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -89,7 +89,7 @@ static void escaped_string(void)
 for (j = 0; j < 2; j++) {
 if (test_cases[i].utf8_out) {
 cstr = from_json_str(test_cases[i].json_in, j, _abort);
-g_assert_cmpstr(qstring_get_try_str(cstr),
+g_assert_cmpstr(qstring_get_str(cstr),
 ==, test_cases[i].utf8_out);
 if (!test_cases[i].skip) {
 jstr = to_json_str(cstr);
@@ -751,7 +751,7 @@ static void utf8_string(void)
 /* Parse @json_in, expect @utf8_out */
 if (utf8_out) {
 str = from_json_str(json_in, j, _abort);
-g_assert_cmpstr(qstring_get_try_str(str), ==, utf8_out);
+g_assert_cmpstr(qstring_get_str(str), ==, utf8_out);
 qobject_unref(str);
 } else {
 str = from_json_str(json_in, j, NULL);
@@ -782,7 +782,7 @@ static void utf8_string(void)
 /* Parse @json_out right back, unless it has replacements */
 if (!strstr(json_out, "\\uFFFD")) {
 str = from_json_str(json_out, j, _abort);
-g_assert_cmpstr(qstring_get_try_str(str), ==, utf8_in);
+g_assert_cmpstr(qstring_get_str(str), ==, utf8_in);
 qobject_unref(str);
 }
 }
@@ -1021,9 +1021,8 @@ static void interpolation_valid(void)
 
 /* string */
 
-qstr = qobject_to(QString,
- qobject_from_jsonf_nofail("%s", value_s));
-g_assert_cmpstr(qstring_get_try_str(qstr), ==, value_s);
+qstr = qobject_to(QString, qobject_from_jsonf_nofail("%s", value_s));
+g_assert_cmpstr(qstring_get_str(qstr), ==, value_s);
 qobject_unref(qstr);
 
 /* object */
-- 
2.26.2




[PULL 07/33] tests/check-qjson: Replace redundant large_number()

2020-12-19 Thread Markus Armbruster
Move one of large_number()'s three checks to uint_number(), and the
other two to float_number().

Signed-off-by: Markus Armbruster 
Message-Id: <20201210161452.2813491-5-arm...@redhat.com>
---
 tests/check-qjson.c | 47 +++--
 1 file changed, 3 insertions(+), 44 deletions(-)

diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index 8cb8a40524..98515b1fd6 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -844,6 +844,7 @@ static void uint_number(void)
 const char *reencoded;
 } test_cases[] = {
 { "9223372036854775808", (uint64_t)1 << 63 },
+{ "18446744073709551615", UINT64_MAX },
 {},
 };
 int i;
@@ -872,49 +873,6 @@ static void uint_number(void)
 }
 }
 
-static void large_number(void)
-{
-const char *maxu64 = "18446744073709551615"; /* 2^64-1 */
-const char *gtu64 = "18446744073709551616"; /* 2^64 */
-const char *lti64 = "-9223372036854775809"; /* -2^63 - 1 */
-QNum *qnum;
-QString *str;
-uint64_t val;
-int64_t ival;
-
-qnum = qobject_to(QNum, qobject_from_json(maxu64, _abort));
-g_assert(qnum);
-g_assert_cmpuint(qnum_get_uint(qnum), ==, 18446744073709551615U);
-g_assert(!qnum_get_try_int(qnum, ));
-
-str = qobject_to_json(QOBJECT(qnum));
-g_assert_cmpstr(qstring_get_str(str), ==, maxu64);
-qobject_unref(str);
-qobject_unref(qnum);
-
-qnum = qobject_to(QNum, qobject_from_json(gtu64, _abort));
-g_assert(qnum);
-g_assert_cmpfloat(qnum_get_double(qnum), ==, 18446744073709552e3);
-g_assert(!qnum_get_try_uint(qnum, ));
-g_assert(!qnum_get_try_int(qnum, ));
-
-str = qobject_to_json(QOBJECT(qnum));
-g_assert_cmpstr(qstring_get_str(str), ==, gtu64);
-qobject_unref(str);
-qobject_unref(qnum);
-
-qnum = qobject_to(QNum, qobject_from_json(lti64, _abort));
-g_assert(qnum);
-g_assert_cmpfloat(qnum_get_double(qnum), ==, -92233720368547758e2);
-g_assert(!qnum_get_try_uint(qnum, ));
-g_assert(!qnum_get_try_int(qnum, ));
-
-str = qobject_to_json(QOBJECT(qnum));
-g_assert_cmpstr(qstring_get_str(str), ==, "-9223372036854775808");
-qobject_unref(str);
-qobject_unref(qnum);
-}
-
 static void float_number(void)
 {
 struct {
@@ -926,6 +884,8 @@ static void float_number(void)
 { "0.222", 0.222 },
 { "-32.12313", -32.12313 },
 { "-32.20e-10", -32.20e-10, "-0" /* BUG */ },
+{ "18446744073709551616", 0x1p64 },
+{ "-9223372036854775809", -0x1p63, "-9223372036854775808" },
 {},
 };
 int i;
@@ -1525,7 +1485,6 @@ int main(int argc, char **argv)
 
 g_test_add_func("/literals/number/int", int_number);
 g_test_add_func("/literals/number/uint", uint_number);
-g_test_add_func("/literals/number/large", large_number);
 g_test_add_func("/literals/number/float", float_number);
 
 g_test_add_func("/literals/keyword", keyword_literal);
-- 
2.26.2




[PULL 02/33] migration: Refactor migrate_cap_add

2020-12-19 Thread Markus Armbruster
From: Eric Blake 

Instead of taking a list parameter and returning a new head at a
distance, just return the new item for the caller to insert into a
list via QAPI_LIST_PREPEND.

Signed-off-by: Eric Blake 
Message-Id: <20201113011340.463563-4-ebl...@redhat.com>
Reviewed-by: Markus Armbruster 
Signed-off-by: Markus Armbruster 
---
 migration/migration.c | 22 +-
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index e0dbde4091..ccf969a62e 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1654,27 +1654,23 @@ void migrate_set_state(int *state, int old_state, int 
new_state)
 }
 }
 
-static MigrationCapabilityStatusList *migrate_cap_add(
-MigrationCapabilityStatusList *list,
-MigrationCapability index,
-bool state)
+static MigrationCapabilityStatus *migrate_cap_add(MigrationCapability index,
+  bool state)
 {
-MigrationCapabilityStatusList *cap;
+MigrationCapabilityStatus *cap;
 
-cap = g_new0(MigrationCapabilityStatusList, 1);
-cap->value = g_new0(MigrationCapabilityStatus, 1);
-cap->value->capability = index;
-cap->value->state = state;
-cap->next = list;
+cap = g_new0(MigrationCapabilityStatus, 1);
+cap->capability = index;
+cap->state = state;
 
 return cap;
 }
 
 void migrate_set_block_enabled(bool value, Error **errp)
 {
-MigrationCapabilityStatusList *cap;
+MigrationCapabilityStatusList *cap = NULL;
 
-cap = migrate_cap_add(NULL, MIGRATION_CAPABILITY_BLOCK, value);
+QAPI_LIST_PREPEND(cap, migrate_cap_add(MIGRATION_CAPABILITY_BLOCK, value));
 qmp_migrate_set_capabilities(cap, errp);
 qapi_free_MigrationCapabilityStatusList(cap);
 }
@@ -3863,7 +3859,7 @@ static bool migration_object_check(MigrationState *ms, 
Error **errp)
 
 for (i = 0; i < MIGRATION_CAPABILITY__MAX; i++) {
 if (ms->enabled_capabilities[i]) {
-head = migrate_cap_add(head, i, true);
+QAPI_LIST_PREPEND(head, migrate_cap_add(i, true));
 }
 }
 
-- 
2.26.2




[PULL 09/33] qobject: Fix qnum_to_string() to use sufficient precision

2020-12-19 Thread Markus Armbruster
We should serialize numbers to JSON so that they deserialize back to
the same number.  We fail to do so.

The culprit is qnum_to_string(): it uses format %f with trailing '0'
trimmed.  Results in pretty output for "nice" numbers, but is prone to
nasty rounding errors.  For instance, numbers between 0 and 0.005
get flushed to zero.

Where exactly the incorrect rounding can bite is tiresome to gauge.
Here's my take.

* In QMP output, type 'number':

  - query-blockstats value avg_rd_queue_depth

  - QMP query-migrate values mbps, cache-miss-rate, encoding-rate,
busy-rate, compression-rate.

  Relatively harmless, I guess.

* In tracing QMP input.  Harmless.

* In qemu-ga output, type 'number': guest-get-users value login-time.
  Harmless.

* In output of HMP qom-get.  Harmless.

Not affected, because double values don't actually occur there (I
think):

* QMP output, type 'any':

  * qom-get value

  * qom-list, qom-list-properties value default-value

  * query-cpu-model-comparison, query-cpu-model-baseline,
query-cpu-model-expansion value props.

* qemu-img --output json output.

* "json:" pseudo-filenames generated by bdrv_refresh_filename().

* The rbd block driver's "=keyvalue-pairs" hack.

* In -object help on property default values.  Aside: use of JSON
  feels inappropriate here.

* Output of HMP qom-get.

* Argument conversion to QemuOpts for qdev_device_add() and HMP with
  qemu_opts_from_qdict()

  QMP and HMP device_add, virtio-net failover primary creation,
  xen-usb "usb-host" creation, HMP netdev_add, object_add.

* The uses of qobject_input_visitor_new_flat_confused()

  As far as I can tell, none of the visited types contain double
  values.

* Dumping ImageInfoSpecific with dump_qobject()

Fix by formatting with %.17g.  17 decimal digits always suffice for
IEEE double.

The change to expected test output illustrates the effect: the
rounding errors are gone, but some seemingly "nice" numbers now get
converted to not so nice strings, e.g. 0.42 to "0.41998".
This is because 0.42 is not representable exactly in double.  It's
more accurate in this example than strictly necessary, though.

If ugly accuracy bothers us, we can we can try using the least number
of digits that still converts back to the same double.  In this
example, "0.42" would do.

Signed-off-by: Markus Armbruster 
Message-Id: <20201210161452.2813491-7-arm...@redhat.com>
---
 qobject/qnum.c  | 24 +++-
 tests/check-qjson.c |  8 
 tests/check-qnum.c  |  4 ++--
 3 files changed, 9 insertions(+), 27 deletions(-)

diff --git a/qobject/qnum.c b/qobject/qnum.c
index 7012fc57f2..bf1240ecec 100644
--- a/qobject/qnum.c
+++ b/qobject/qnum.c
@@ -161,37 +161,19 @@ double qnum_get_double(QNum *qn)
 
 char *qnum_to_string(QNum *qn)
 {
-char *buffer;
-int len;
-
 switch (qn->kind) {
 case QNUM_I64:
 return g_strdup_printf("%" PRId64, qn->u.i64);
 case QNUM_U64:
 return g_strdup_printf("%" PRIu64, qn->u.u64);
 case QNUM_DOUBLE:
-/* FIXME: snprintf() is locale dependent; but JSON requires
+/* FIXME: g_strdup_printf() is locale dependent; but JSON requires
  * numbers to be formatted as if in the C locale. Dependence
  * on C locale is a pervasive issue in QEMU. */
 /* FIXME: This risks printing Inf or NaN, which are not valid
  * JSON values. */
-/* FIXME: the default precision of 6 for %f often causes
- * rounding errors; we should be using DBL_DECIMAL_DIG (17),
- * and only rounding to a shorter number if the result would
- * still produce the same floating point value.  */
-buffer = g_strdup_printf("%f" , qn->u.dbl);
-len = strlen(buffer);
-while (len > 0 && buffer[len - 1] == '0') {
-len--;
-}
-
-if (len && buffer[len - 1] == '.') {
-buffer[len - 1] = 0;
-} else {
-buffer[len] = 0;
-}
-
-return buffer;
+/* 17 digits suffice for IEEE double */
+return g_strdup_printf("%.17g", qn->u.dbl);
 }
 
 assert(0);
diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index 98515b1fd6..ca8fb816e9 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -882,10 +882,10 @@ static void float_number(void)
 } test_cases[] = {
 { "32.43", 32.43 },
 { "0.222", 0.222 },
-{ "-32.12313", -32.12313 },
-{ "-32.20e-10", -32.20e-10, "-0" /* BUG */ },
-{ "18446744073709551616", 0x1p64 },
-{ "-9223372036854775809", -0x1p63, "-9223372036854775808" },
+{ "-32.12313", -32.12313, "-32.1231303" },
+{ "-32.20e-10", -32.20e-10, "-3.22e-09" },
+{ "18446744073709551616", 0x1p64, "1.8446744073709552e+19" },
+{ "-9223372036854775809", -0x1p63, "-9.2233720368547758e+18" },
 {},
 };
 int i;
diff --git a/tests/check-qnum.c b/tests/check-qnum.c
index a73809d021..b85fca2302 100644

[PULL 00/33] QAPI patches patches for 2020-12-19

2020-12-19 Thread Markus Armbruster
The following changes since commit a05f8ecd88f15273d033b6f044b850a8af84a5b8:

  Merge remote-tracking branch 
'remotes/alistair/tags/pull-riscv-to-apply-20201217-1' into staging (2020-12-18 
11:12:35 +)

are available in the Git repository at:

  git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2020-12-19

for you to fetch changes up to 4ac76ba414ecb94f086d73621775d8b38b6f0a43:

  qobject: Make QString immutable (2020-12-19 10:39:41 +0100)


QAPI patches patches for 2020-12-19


Eric Blake (3):
  rocker: Revamp fp_port_get_info
  migration: Refactor migrate_cap_add
  qapi: Use QAPI_LIST_PREPEND() where possible

Markus Armbruster (30):
  tests/check-qjson: Don't skip funny QNumber to JSON conversions
  tests/check-qjson: Examine QNum more thoroughly
  tests/check-qjson: Cover number 2^63
  tests/check-qjson: Replace redundant large_number()
  tests/check-qnum: Cover qnum_to_string() for "unround" argument
  qobject: Fix qnum_to_string() to use sufficient precision
  test-string-output-visitor: Cover "unround" number
  string-output-visitor: Fix to use sufficient precision
  test-visitor-serialization: Drop insufficient precision workaround
  test-visitor-serialization: Clean up test_primitives()
  hmp: Simplify how qmp_human_monitor_command() gets output
  monitor: Use GString instead of QString for output buffer
  qobject: Make qobject_to_json_pretty() take a pretty argument
  qobject: Use GString instead of QString to accumulate JSON
  qobject: Change qobject_to_json()'s value to GString
  Revert "qstring: add qstring_free()"
  hw/rdma: Replace QList by GQueue
  qobject: Move internals to qobject-internal.h
  qmp: Fix tracing of non-string command IDs
  block: Avoid qobject_get_try_str()
  Revert "qobject: let object_property_get_str() use new API"
  qobject: Drop qobject_get_try_str()
  qobject: Drop qstring_get_try_str()
  qobject: Factor quoted_str() out of to_json()
  qobject: Factor JSON writer out of qobject_to_json()
  migration: Replace migration's JSON writer by the general one
  json: Use GString instead of QString to accumulate strings
  keyval: Use GString to accumulate value strings
  block: Use GString instead of QString to build filenames
  qobject: Make QString immutable

 docs/devel/writing-qmp-commands.txt |  12 +-
 hw/net/rocker/rocker_fp.h   |   2 +-
 hw/rdma/rdma_backend_defs.h |   2 +-
 hw/rdma/rdma_utils.h|  15 ++-
 include/migration/vmstate.h |   7 +-
 include/qapi/qmp/json-writer.h  |  35 +
 include/qapi/qmp/qbool.h|   2 -
 include/qapi/qmp/qdict.h|   2 -
 include/qapi/qmp/qjson.h|   4 +-
 include/qapi/qmp/qlist.h|   2 -
 include/qapi/qmp/qnull.h|   2 -
 include/qapi/qmp/qnum.h |   3 -
 include/qapi/qmp/qobject.h  |   9 +-
 include/qapi/qmp/qstring.h  |  14 +-
 include/qemu/typedefs.h |   4 +-
 migration/qjson.h   |  29 -
 monitor/monitor-internal.h  |   2 +-
 qobject/qobject-internal.h  |  39 ++
 block.c |  23 ++--
 block/gluster.c |   4 +-
 block/qapi.c|   7 +-
 block/rbd.c |   2 +-
 chardev/char.c  |  20 ++-
 hw/core/machine-qmp-cmds.c  |   6 +-
 hw/core/machine.c   |  11 +-
 hw/display/virtio-gpu.c |   2 +-
 hw/intc/s390_flic_kvm.c |   2 +-
 hw/net/rocker/rocker.c  |   8 +-
 hw/net/rocker/rocker_fp.c   |  17 ++-
 hw/net/rocker/rocker_of_dpa.c   |  20 +--
 hw/net/virtio-net.c |  21 +--
 hw/nvram/eeprom93xx.c   |   2 +-
 hw/nvram/fw_cfg.c   |   2 +-
 hw/pci/msix.c   |   2 +-
 hw/pci/pci.c|   4 +-
 hw/pci/shpc.c   |   2 +-
 hw/rdma/rdma_backend.c  |  10 +-
 hw/rdma/rdma_utils.c|  29 +++--
 hw/rtc/twl92230.c   |   2 +-
 hw/scsi/scsi-bus.c  |   2 +-
 hw/usb/redirect.c   |   7 +-
 hw/virtio/virtio.c  |   4 +-
 migration/migration.c   |  29 ++---
 migration/postcopy-ram.c|   7 +-
 migration/qjson.c   | 114 -
 migration/savevm.c  |  53 
 migration/vmstate-types.c   |  38 +++---
 migration/vmstate.c |  52 
 monitor/hmp-cmds.c  |  13 +-
 monitor/misc.c  |  31 ++---
 monitor/monitor.c   |  20 ++-
 monitor/qmp-cmds-control.c  |  10 +-
 monitor/qmp.c   |  46 ---
 

[PULL 16/33] qobject: Make qobject_to_json_pretty() take a pretty argument

2020-12-19 Thread Markus Armbruster
Signed-off-by: Markus Armbruster 
Message-Id: <20201211171152.146877-4-arm...@redhat.com>
---
 include/qapi/qmp/qjson.h |  2 +-
 monitor/qmp.c|  2 +-
 qemu-img.c   |  8 
 qobject/qjson.c  | 28 +++-
 qom/qom-hmp-cmds.c   |  2 +-
 tests/qtest/libqtest.c   |  2 +-
 6 files changed, 19 insertions(+), 25 deletions(-)

diff --git a/include/qapi/qmp/qjson.h b/include/qapi/qmp/qjson.h
index 5ebbe5a118..82f4534f16 100644
--- a/include/qapi/qmp/qjson.h
+++ b/include/qapi/qmp/qjson.h
@@ -26,6 +26,6 @@ QDict *qdict_from_jsonf_nofail(const char *string, ...)
 GCC_FMT_ATTR(1, 2);
 
 QString *qobject_to_json(const QObject *obj);
-QString *qobject_to_json_pretty(const QObject *obj);
+QString *qobject_to_json_pretty(const QObject *obj, bool pretty);
 
 #endif /* QJSON_H */
diff --git a/monitor/qmp.c b/monitor/qmp.c
index b42f8c6af3..1197c50b20 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -112,7 +112,7 @@ void qmp_send_response(MonitorQMP *mon, const QDict *rsp)
 const QObject *data = QOBJECT(rsp);
 QString *json;
 
-json = mon->pretty ? qobject_to_json_pretty(data) : qobject_to_json(data);
+json = qobject_to_json_pretty(data, mon->pretty);
 assert(json != NULL);
 
 qstring_append_chr(json, '\n');
diff --git a/qemu-img.c b/qemu-img.c
index d599659c7f..1a59dfd3f3 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -633,7 +633,7 @@ static void dump_json_image_check(ImageCheck *check, bool 
quiet)
 
 visit_type_ImageCheck(v, NULL, , _abort);
 visit_complete(v, );
-str = qobject_to_json_pretty(obj);
+str = qobject_to_json_pretty(obj, true);
 assert(str != NULL);
 qprintf(quiet, "%s\n", qstring_get_str(str));
 qobject_unref(obj);
@@ -2795,7 +2795,7 @@ static void dump_json_image_info_list(ImageInfoList *list)
 
 visit_type_ImageInfoList(v, NULL, , _abort);
 visit_complete(v, );
-str = qobject_to_json_pretty(obj);
+str = qobject_to_json_pretty(obj, true);
 assert(str != NULL);
 printf("%s\n", qstring_get_str(str));
 qobject_unref(obj);
@@ -2811,7 +2811,7 @@ static void dump_json_image_info(ImageInfo *info)
 
 visit_type_ImageInfo(v, NULL, , _abort);
 visit_complete(v, );
-str = qobject_to_json_pretty(obj);
+str = qobject_to_json_pretty(obj, true);
 assert(str != NULL);
 printf("%s\n", qstring_get_str(str));
 qobject_unref(obj);
@@ -5242,7 +5242,7 @@ static void dump_json_block_measure_info(BlockMeasureInfo 
*info)
 
 visit_type_BlockMeasureInfo(v, NULL, , _abort);
 visit_complete(v, );
-str = qobject_to_json_pretty(obj);
+str = qobject_to_json_pretty(obj, true);
 assert(str != NULL);
 printf("%s\n", qstring_get_str(str));
 qobject_unref(obj);
diff --git a/qobject/qjson.c b/qobject/qjson.c
index f1f2c69704..523a4ab8de 100644
--- a/qobject/qjson.c
+++ b/qobject/qjson.c
@@ -149,8 +149,6 @@ QDict *qdict_from_jsonf_nofail(const char *string, ...)
 return qdict;
 }
 
-static void to_json(const QObject *obj, QString *str, int pretty, int indent);
-
 static void json_pretty_newline(QString *str, bool pretty, int indent)
 {
 int i;
@@ -163,7 +161,7 @@ static void json_pretty_newline(QString *str, bool pretty, 
int indent)
 }
 }
 
-static void to_json(const QObject *obj, QString *str, int pretty, int indent)
+static void to_json(const QObject *obj, QString *str, bool pretty, int indent)
 {
 switch (qobject_type(obj)) {
 case QTYPE_QNULL:
@@ -294,20 +292,16 @@ static void to_json(const QObject *obj, QString *str, int 
pretty, int indent)
 }
 }
 
+QString *qobject_to_json_pretty(const QObject *obj, bool pretty)
+{
+QString *str = qstring_new();
+
+to_json(obj, str, pretty, 0);
+
+return str;
+}
+
 QString *qobject_to_json(const QObject *obj)
 {
-QString *str = qstring_new();
-
-to_json(obj, str, 0, 0);
-
-return str;
-}
-
-QString *qobject_to_json_pretty(const QObject *obj)
-{
-QString *str = qstring_new();
-
-to_json(obj, str, 1, 0);
-
-return str;
+return qobject_to_json_pretty(obj, false);
 }
diff --git a/qom/qom-hmp-cmds.c b/qom/qom-hmp-cmds.c
index 8861a109d5..6b96dbe906 100644
--- a/qom/qom-hmp-cmds.c
+++ b/qom/qom-hmp-cmds.c
@@ -78,7 +78,7 @@ void hmp_qom_get(Monitor *mon, const QDict *qdict)
 QObject *obj = qmp_qom_get(path, property, );
 
 if (err == NULL) {
-QString *str = qobject_to_json_pretty(obj);
+QString *str = qobject_to_json_pretty(obj, true);
 monitor_printf(mon, "%s\n", qstring_get_str(str));
 qobject_unref(str);
 }
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index e49f3a1e45..213fa4f8fe 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1197,7 +1197,7 @@ void qtest_qmp_assert_success(QTestState *qts, const char 
*fmt, ...)
 
 g_assert(response);
 if (!qdict_haskey(response, "return")) {
-QString *s = qobject_to_json_pretty(QOBJECT(response));
+QString *s 

[PULL 11/33] string-output-visitor: Fix to use sufficient precision

2020-12-19 Thread Markus Armbruster
The string output visitor should serialize numbers so that the string
input visitor deserializes them back to the same number.  It fails to
do so.

print_type_number() uses format %f.  This is prone to nasty rounding
errors.  For instance, numbers between 0 and 0.005 get flushed to
zero.

We currently use this visitor only for HMP info migrate, info network,
info qtree, and info memdev.  No double values occur there as far as I
can tell.

Fix anyway by formatting with %.17g.  17 decimal digits always suffice
for IEEE double.

See also recent commit "qobject: Fix qnum_to_string() to use
sufficient precision".

Signed-off-by: Markus Armbruster 
Message-Id: <20201210161452.2813491-9-arm...@redhat.com>
---
 qapi/string-output-visitor.c   | 2 +-
 tests/test-string-output-visitor.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
index b74aa4d44c..5506c933de 100644
--- a/qapi/string-output-visitor.c
+++ b/qapi/string-output-visitor.c
@@ -258,7 +258,7 @@ static bool print_type_number(Visitor *v, const char *name, 
double *obj,
   Error **errp)
 {
 StringOutputVisitor *sov = to_sov(v);
-string_output_set(sov, g_strdup_printf("%f", *obj));
+string_output_set(sov, g_strdup_printf("%.17g", *obj));
 return true;
 }
 
diff --git a/tests/test-string-output-visitor.c 
b/tests/test-string-output-visitor.c
index cec20848ea..0dae04b960 100644
--- a/tests/test-string-output-visitor.c
+++ b/tests/test-string-output-visitor.c
@@ -136,7 +136,7 @@ static void test_visitor_out_number(TestOutputVisitorData 
*data,
 visit_type_number(data->ov, NULL, , _abort);
 
 str = visitor_get(data);
-g_assert_cmpstr(str, ==, "3.141593");
+g_assert_cmpstr(str, ==, "3.1415926535897931");
 }
 
 static void test_visitor_out_string(TestOutputVisitorData *data,
-- 
2.26.2




[PULL 05/33] tests/check-qjson: Examine QNum more thoroughly

2020-12-19 Thread Markus Armbruster
simple_number() checks only qnum_get_try_int().  Also check
qnum_get_try_uint() and qnum_get_double().

float_number() checks only qnum_get_double().  Also check
qnum_get_try_int() and qnum_get_try_uint().

Signed-off-by: Markus Armbruster 
Message-Id: <20201210161452.2813491-3-arm...@redhat.com>
---
 tests/check-qjson.c | 19 ---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index 2a5852904a..6ab6b111b0 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -807,7 +807,8 @@ static void simple_number(void)
 };
 int i;
 QNum *qnum;
-int64_t val;
+int64_t ival;
+uint64_t uval;
 QString *str;
 
 for (i = 0; test_cases[i].encoded; i++) {
@@ -815,8 +816,16 @@ static void simple_number(void)
   qobject_from_json(test_cases[i].encoded,
 _abort));
 g_assert(qnum);
-g_assert(qnum_get_try_int(qnum, ));
-g_assert_cmpint(val, ==, test_cases[i].decoded);
+g_assert(qnum_get_try_int(qnum, ));
+g_assert_cmpint(ival, ==, test_cases[i].decoded);
+if (test_cases[i].decoded >= 0) {
+g_assert(qnum_get_try_uint(qnum, ));
+g_assert_cmpuint(uval, ==, (uint64_t)test_cases[i].decoded);
+} else {
+g_assert(!qnum_get_try_uint(qnum, ));
+}
+g_assert_cmpfloat(qnum_get_double(qnum), ==,
+  (double)test_cases[i].decoded);
 
 str = qobject_to_json(QOBJECT(qnum));
 g_assert_cmpstr(qstring_get_str(str), ==,
@@ -885,6 +894,8 @@ static void float_number(void)
 };
 int i;
 QNum *qnum;
+int64_t ival;
+uint64_t uval;
 QString *str;
 
 for (i = 0; test_cases[i].encoded; i++) {
@@ -893,6 +904,8 @@ static void float_number(void)
 _abort));
 g_assert(qnum);
 g_assert_cmpfloat(qnum_get_double(qnum), ==, test_cases[i].decoded);
+g_assert(!qnum_get_try_int(qnum, ));
+g_assert(!qnum_get_try_uint(qnum, ));
 
 str = qobject_to_json(QOBJECT(qnum));
 g_assert_cmpstr(qstring_get_str(str), ==,
-- 
2.26.2




[PULL 12/33] test-visitor-serialization: Drop insufficient precision workaround

2020-12-19 Thread Markus Armbruster
Signed-off-by: Markus Armbruster 
Message-Id: <20201210161452.2813491-10-arm...@redhat.com>
---
 tests/test-visitor-serialization.c | 18 ++
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/tests/test-visitor-serialization.c 
b/tests/test-visitor-serialization.c
index 12275e56d8..cf19924068 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -311,17 +311,7 @@ static void test_primitives(gconstpointer opaque)
 g_assert_cmpstr(pt->value.string, ==, pt_copy->value.string);
 g_free((char *)pt_copy->value.string);
 } else if (pt->type == PTYPE_NUMBER) {
-GString *double_expected = g_string_new("");
-GString *double_actual = g_string_new("");
-/* we serialize with %f for our reference visitors, so rather than 
fuzzy
- * floating math to test "equality", just compare the formatted values
- */
-g_string_printf(double_expected, "%.6f", pt->value.number);
-g_string_printf(double_actual, "%.6f", pt_copy->value.number);
-g_assert_cmpstr(double_actual->str, ==, double_expected->str);
-
-g_string_free(double_expected, true);
-g_string_free(double_actual, true);
+g_assert_cmpfloat(pt->value.number, ==, pt_copy->value.number);
 } else if (pt->type == PTYPE_BOOLEAN) {
 g_assert_cmpint(!!pt->value.max, ==, !!pt->value.max);
 } else {
@@ -703,10 +693,6 @@ static PrimitiveType pt_values[] = {
 .value.boolean = 0,
 },
 /* number tests (double) */
-/* note: we format these to %.6f before comparing, since that's how
- * we serialize them and it doesn't make sense to check precision
- * beyond that.
- */
 {
 .description = "number_sanity1",
 .type = PTYPE_NUMBER,
@@ -715,7 +701,7 @@ static PrimitiveType pt_values[] = {
 {
 .description = "number_sanity2",
 .type = PTYPE_NUMBER,
-.value.number = 3.14159265,
+.value.number = 3.141593,
 },
 {
 .description = "number_min",
-- 
2.26.2




[PULL 06/33] tests/check-qjson: Cover number 2^63

2020-12-19 Thread Markus Armbruster
Signed-off-by: Markus Armbruster 
Message-Id: <20201210161452.2813491-4-arm...@redhat.com>
---
 tests/check-qjson.c | 41 +++--
 1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index 6ab6b111b0..8cb8a40524 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -791,7 +791,7 @@ static void utf8_string(void)
 }
 }
 
-static void simple_number(void)
+static void int_number(void)
 {
 struct {
 const char *encoded;
@@ -836,6 +836,42 @@ static void simple_number(void)
 }
 }
 
+static void uint_number(void)
+{
+struct {
+const char *encoded;
+uint64_t decoded;
+const char *reencoded;
+} test_cases[] = {
+{ "9223372036854775808", (uint64_t)1 << 63 },
+{},
+};
+int i;
+QNum *qnum;
+int64_t ival;
+uint64_t uval;
+QString *str;
+
+for (i = 0; test_cases[i].encoded; i++) {
+qnum = qobject_to(QNum,
+  qobject_from_json(test_cases[i].encoded,
+_abort));
+g_assert(qnum);
+g_assert(qnum_get_try_uint(qnum, ));
+g_assert_cmpuint(uval, ==, test_cases[i].decoded);
+g_assert(!qnum_get_try_int(qnum, ));
+g_assert_cmpfloat(qnum_get_double(qnum), ==,
+  (double)test_cases[i].decoded);
+
+str = qobject_to_json(QOBJECT(qnum));
+g_assert_cmpstr(qstring_get_str(str), ==,
+test_cases[i].reencoded ?: test_cases[i].encoded);
+qobject_unref(str);
+
+qobject_unref(qnum);
+}
+}
+
 static void large_number(void)
 {
 const char *maxu64 = "18446744073709551615"; /* 2^64-1 */
@@ -1487,7 +1523,8 @@ int main(int argc, char **argv)
 g_test_add_func("/literals/string/quotes", string_with_quotes);
 g_test_add_func("/literals/string/utf8", utf8_string);
 
-g_test_add_func("/literals/number/simple", simple_number);
+g_test_add_func("/literals/number/int", int_number);
+g_test_add_func("/literals/number/uint", uint_number);
 g_test_add_func("/literals/number/large", large_number);
 g_test_add_func("/literals/number/float", float_number);
 
-- 
2.26.2




[PULL 10/33] test-string-output-visitor: Cover "unround" number

2020-12-19 Thread Markus Armbruster
This demonstrates rounding error due to insufficient precision: double
3.1415926535897932 gets converted to JSON 3.141593.

Signed-off-by: Markus Armbruster 
Message-Id: <20201210161452.2813491-8-arm...@redhat.com>
---
 tests/test-string-output-visitor.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/test-string-output-visitor.c 
b/tests/test-string-output-visitor.c
index 9f6581439a..cec20848ea 100644
--- a/tests/test-string-output-visitor.c
+++ b/tests/test-string-output-visitor.c
@@ -130,13 +130,13 @@ static void test_visitor_out_bool(TestOutputVisitorData 
*data,
 static void test_visitor_out_number(TestOutputVisitorData *data,
 const void *unused)
 {
-double value = 3.14;
+double value = 3.1415926535897932;
 char *str;
 
 visit_type_number(data->ov, NULL, , _abort);
 
 str = visitor_get(data);
-g_assert_cmpstr(str, ==, "3.14");
+g_assert_cmpstr(str, ==, "3.141593");
 }
 
 static void test_visitor_out_string(TestOutputVisitorData *data,
-- 
2.26.2




[PULL 08/33] tests/check-qnum: Cover qnum_to_string() for "unround" argument

2020-12-19 Thread Markus Armbruster
qnum_to_string() has a FIXME comment about rounding errors due to
insufficient precision.  Cover it: 2.718281828459045 gets converted to
"2.718282".  The next commit will fix it.

Signed-off-by: Markus Armbruster 
Message-Id: <20201210161452.2813491-6-arm...@redhat.com>
---
 tests/check-qnum.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/tests/check-qnum.c b/tests/check-qnum.c
index 4105015872..a73809d021 100644
--- a/tests/check-qnum.c
+++ b/tests/check-qnum.c
@@ -150,6 +150,12 @@ static void qnum_to_string_test(void)
 g_assert_cmpstr(tmp, ==, "0.42");
 g_free(tmp);
 qobject_unref(qn);
+
+qn = qnum_from_double(2.718281828459045);
+tmp = qnum_to_string(qn);
+g_assert_cmpstr(tmp, ==, "2.718282"); /* BUG */
+g_free(tmp);
+qobject_unref(qn);
 }
 
 int main(int argc, char **argv)
-- 
2.26.2




[PULL 04/33] tests/check-qjson: Don't skip funny QNumber to JSON conversions

2020-12-19 Thread Markus Armbruster
simple_number() and float_number() convert from JSON to QNumber and
back.

simple_number() tests "-0", but skips the conversion back to JSON,
because it yields "0", not "-0".  Works as intended, so better cover
it: don't skip, but expect the funny result.

float_number() tests "-32.20e-10", but skips the conversion back to
JSON, because it yields "-0".  This is a known bug in
qnum_to_string(), marked FIXME there.  Cover the bug: don't skip, but
expect the funny result.

While there, switch from g_assert() to g_assert_cmpstr() & friends for
friendlier test failures.

Signed-off-by: Markus Armbruster 
Message-Id: <20201210161452.2813491-2-arm...@redhat.com>
---
 tests/check-qjson.c | 55 +
 1 file changed, 25 insertions(+), 30 deletions(-)

diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index 9a02079099..2a5852904a 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -793,37 +793,35 @@ static void utf8_string(void)
 
 static void simple_number(void)
 {
-int i;
 struct {
 const char *encoded;
 int64_t decoded;
-int skip;
+const char *reencoded;
 } test_cases[] = {
 { "0", 0 },
 { "1234", 1234 },
 { "1", 1 },
 { "-32", -32 },
-{ "-0", 0, .skip = 1 },
-{ },
+{ "-0", 0, "0" },
+{},
 };
+int i;
+QNum *qnum;
+int64_t val;
+QString *str;
 
 for (i = 0; test_cases[i].encoded; i++) {
-QNum *qnum;
-int64_t val;
-
 qnum = qobject_to(QNum,
   qobject_from_json(test_cases[i].encoded,
 _abort));
 g_assert(qnum);
 g_assert(qnum_get_try_int(qnum, ));
 g_assert_cmpint(val, ==, test_cases[i].decoded);
-if (test_cases[i].skip == 0) {
-QString *str;
 
-str = qobject_to_json(QOBJECT(qnum));
-g_assert(strcmp(qstring_get_str(str), test_cases[i].encoded) == 0);
-qobject_unref(str);
-}
+str = qobject_to_json(QOBJECT(qnum));
+g_assert_cmpstr(qstring_get_str(str), ==,
+test_cases[i].reencoded ?: test_cases[i].encoded);
+qobject_unref(str);
 
 qobject_unref(qnum);
 }
@@ -874,35 +872,32 @@ static void large_number(void)
 
 static void float_number(void)
 {
-int i;
 struct {
 const char *encoded;
 double decoded;
-int skip;
+const char *reencoded;
 } test_cases[] = {
 { "32.43", 32.43 },
 { "0.222", 0.222 },
 { "-32.12313", -32.12313 },
-{ "-32.20e-10", -32.20e-10, .skip = 1 },
-{ },
+{ "-32.20e-10", -32.20e-10, "-0" /* BUG */ },
+{},
 };
+int i;
+QNum *qnum;
+QString *str;
 
 for (i = 0; test_cases[i].encoded; i++) {
-QObject *obj;
-QNum *qnum;
-
-obj = qobject_from_json(test_cases[i].encoded, _abort);
-qnum = qobject_to(QNum, obj);
+qnum = qobject_to(QNum,
+  qobject_from_json(test_cases[i].encoded,
+_abort));
 g_assert(qnum);
-g_assert(qnum_get_double(qnum) == test_cases[i].decoded);
+g_assert_cmpfloat(qnum_get_double(qnum), ==, test_cases[i].decoded);
 
-if (test_cases[i].skip == 0) {
-QString *str;
-
-str = qobject_to_json(obj);
-g_assert(strcmp(qstring_get_str(str), test_cases[i].encoded) == 0);
-qobject_unref(str);
-}
+str = qobject_to_json(QOBJECT(qnum));
+g_assert_cmpstr(qstring_get_str(str), ==,
+test_cases[i].reencoded ?: test_cases[i].encoded);
+qobject_unref(str);
 
 qobject_unref(qnum);
 }
-- 
2.26.2




[PULL 01/33] rocker: Revamp fp_port_get_info

2020-12-19 Thread Markus Armbruster
From: Eric Blake 

Instead of modifying the value member of a list element passed as a
parameter, and open-coding the manipulation of that list, it's nicer
to just return a freshly allocated value to be prepended to a list
using QAPI_LIST_PREPEND.

Signed-off-by: Eric Blake 
Message-Id: <20201113011340.463563-3-ebl...@redhat.com>
Reviewed-by: Markus Armbruster 
Signed-off-by: Markus Armbruster 
---
 hw/net/rocker/rocker_fp.h |  2 +-
 hw/net/rocker/rocker.c|  8 +---
 hw/net/rocker/rocker_fp.c | 17 ++---
 3 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/hw/net/rocker/rocker_fp.h b/hw/net/rocker/rocker_fp.h
index dbe1dd329a..7ff57aac01 100644
--- a/hw/net/rocker/rocker_fp.h
+++ b/hw/net/rocker/rocker_fp.h
@@ -28,7 +28,7 @@ int fp_port_eg(FpPort *port, const struct iovec *iov, int 
iovcnt);
 
 char *fp_port_get_name(FpPort *port);
 bool fp_port_get_link_up(FpPort *port);
-void fp_port_get_info(FpPort *port, RockerPortList *info);
+RockerPort *fp_port_get_info(FpPort *port);
 void fp_port_get_macaddr(FpPort *port, MACAddr *macaddr);
 void fp_port_set_macaddr(FpPort *port, MACAddr *macaddr);
 uint8_t fp_port_get_learning(FpPort *port);
diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
index 1af1e6fa2f..c53a02315e 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -127,13 +127,7 @@ RockerPortList *qmp_query_rocker_ports(const char *name, 
Error **errp)
 }
 
 for (i = r->fp_ports - 1; i >= 0; i--) {
-RockerPortList *info = g_malloc0(sizeof(*info));
-info->value = g_malloc0(sizeof(*info->value));
-struct fp_port *port = r->fp_port[i];
-
-fp_port_get_info(port, info);
-info->next = list;
-list = info;
+QAPI_LIST_PREPEND(list, fp_port_get_info(r->fp_port[i]));
 }
 
 return list;
diff --git a/hw/net/rocker/rocker_fp.c b/hw/net/rocker/rocker_fp.c
index 4aa7da79b8..cbeed65bd5 100644
--- a/hw/net/rocker/rocker_fp.c
+++ b/hw/net/rocker/rocker_fp.c
@@ -51,14 +51,17 @@ bool fp_port_get_link_up(FpPort *port)
 return !qemu_get_queue(port->nic)->link_down;
 }
 
-void fp_port_get_info(FpPort *port, RockerPortList *info)
+RockerPort *fp_port_get_info(FpPort *port)
 {
-info->value->name = g_strdup(port->name);
-info->value->enabled = port->enabled;
-info->value->link_up = fp_port_get_link_up(port);
-info->value->speed = port->speed;
-info->value->duplex = port->duplex;
-info->value->autoneg = port->autoneg;
+RockerPort *value = g_malloc0(sizeof(*value));
+
+value->name = g_strdup(port->name);
+value->enabled = port->enabled;
+value->link_up = fp_port_get_link_up(port);
+value->speed = port->speed;
+value->duplex = port->duplex;
+value->autoneg = port->autoneg;
+return value;
 }
 
 void fp_port_get_macaddr(FpPort *port, MACAddr *macaddr)
-- 
2.26.2




Re: [PATCH] linux-user: Fix loading of BSS segments

2020-12-19 Thread Laurent Vivier
Le 17/12/2020 à 11:17, Giuseppe Musacchio a écrit :
> Some ELF binaries encode the .bss section as an extension of the data
> ones by setting the segment p_memsz > p_filesz. Some other binaries take
> a different route and encode it as a stand-alone PT_LOAD segment with
> p_filesz = 0 and p_memsz > 0.
> 
> Both the encodings are actually correct per ELF specification but the
> ELF loader had some troubles in handling the former: with the old logic
> it was very likely to get Qemu to crash in zero_bss when trying to
> access unmapped memory.
> 
> zero_bss isn't meant to allocate whole zero-filled segments but to
> "complete" a previously mapped segment with the needed zero bits.
> 
> The fix is pretty simple, if the segment is completely zero-filled we
> simply allocate one or more pages (according to p_memsz) and avoid
> calling zero_bss altogether.

So, the current code manages the bss segment when the memory page has already
been allocated for the data segment by zeroing it:

+--+
 PAGE  |
 --++  |
 DATA  |   BSS  |  |

So your patch fixes the case when there is no data segment and thus no page
to complete:

+--+
 PAGE  |
 --+   |
 BSS   |   |


But could we have a case where the BSS starts in a page allocated for the
data segment but needs more pages?

+--+--+
 PAGE  | PAGE |
 ++   |
 DATA| BSS|   |

In this case we should also allocate the page, and the previous case is only a
special case (data segment = 0) of this one.

so something like (approxymately):

if (eppnt->p_filesz != 0) {
   target_mmap()
   if (vaddr_ef < vaddr_mem) {
   zero_bss(vaddr_ef, MIN(vaddr_mem, vaddr_ps + vaddr_len))
   }
}
if (vaddr_ps + vaddr_len < vaddr_mem) {
  target_mmap(vaddr_ps + vaddr_len, vaddr_ps + vaddr_len - vaddr_mem - 1,
  elf_prot, MAP_PRIVATE | MAP_FIXED | MAP_ANONYMOUS, -1, 0);
}

I think your fix is correct, but I'm wondering if we can have something more
generic, if we can cover an other possible case.

If you think we don't need to introduce more complexity for a case that can't
happen I will queue your patch as is.

Thanks,
Laurent

> Signed-off-by: Giuseppe Musacchio 
> ---
>  linux-user/elfload.c | 30 --
>  1 file changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 0b02a92602..a16c240e0f 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -2776,14 +2776,16 @@ static void load_elf_image(const char *image_name, 
> int image_fd,
>  vaddr = load_bias + eppnt->p_vaddr;
>  vaddr_po = TARGET_ELF_PAGEOFFSET(vaddr);
>  vaddr_ps = TARGET_ELF_PAGESTART(vaddr);
> -vaddr_len = TARGET_ELF_PAGELENGTH(eppnt->p_filesz + vaddr_po);
> +
> +vaddr_ef = vaddr + eppnt->p_filesz;
> +vaddr_em = vaddr + eppnt->p_memsz;
>  
>  /*
> - * Some segments may be completely empty without any backing file
> - * segment, in that case just let zero_bss allocate an empty 
> buffer
> - * for it.
> + * Some segments may be completely empty, with a non-zero p_memsz
> + * but no backing file segment.
>   */
>  if (eppnt->p_filesz != 0) {
> +vaddr_len = TARGET_ELF_PAGELENGTH(eppnt->p_filesz + 
> vaddr_po);
>  error = target_mmap(vaddr_ps, vaddr_len, elf_prot,
>  MAP_PRIVATE | MAP_FIXED,
>  image_fd, eppnt->p_offset - vaddr_po);
> @@ -2791,14 +2793,22 @@ static void load_elf_image(const char *image_name, 
> int image_fd,
>  if (error == -1) {
>  goto exit_mmap;
>  }
> -}
>  
> -vaddr_ef = vaddr + eppnt->p_filesz;
> -vaddr_em = vaddr + eppnt->p_memsz;
> +/*
> + * If the load segment requests extra zeros (e.g. bss), map 
> it.
> + */
> +if (eppnt->p_filesz < eppnt->p_memsz) {
> +zero_bss(vaddr_ef, vaddr_em, elf_prot);
> +}
> +} else if (eppnt->p_memsz != 0) {
> +vaddr_len = TARGET_ELF_PAGELENGTH(eppnt->p_memsz + vaddr_po);
> +error = target_mmap(vaddr_ps, vaddr_len, elf_prot,
> +MAP_PRIVATE | MAP_FIXED | MAP_ANONYMOUS,
> +-1, 0);
>  
> -/* If the load segment requests extra zeros (e.g. bss), map it.  
> 

[PATCH 6/7] macio: wire macio GPIOs to OpenPIC using sysbus IRQs

2020-12-19 Thread Mark Cave-Ayland
This both allows the wiring to be done as Ben suggested in his original comment 
in
gpio.c and also enables the OpenPIC object property link to be removed.

Signed-off-by: Mark Cave-Ayland 
---
 hw/misc/macio/gpio.c | 24 +---
 hw/misc/macio/macio.c| 12 +++-
 include/hw/misc/macio/gpio.h |  2 --
 3 files changed, 12 insertions(+), 26 deletions(-)

diff --git a/hw/misc/macio/gpio.c b/hw/misc/macio/gpio.c
index 0fef8fb335..b1bcf830c3 100644
--- a/hw/misc/macio/gpio.c
+++ b/hw/misc/macio/gpio.c
@@ -57,10 +57,7 @@ void macio_set_gpio(MacIOGPIOState *s, uint32_t gpio, bool 
state)
 
 s->gpio_regs[gpio] = new_reg;
 
-/* This is will work until we fix the binding between MacIO and
- * the MPIC properly so we can route all GPIOs and avoid going
- * via the top level platform code.
- *
+/*
  * Note that we probably need to get access to the MPIC config to
  * decode polarity since qemu always use "raise" regardless.
  *
@@ -152,25 +149,15 @@ static const MemoryRegionOps macio_gpio_ops = {
 },
 };
 
-static void macio_gpio_realize(DeviceState *dev, Error **errp)
-{
-MacIOGPIOState *s = MACIO_GPIO(dev);
-
-s->gpio_extirqs[1] = qdev_get_gpio_in(DEVICE(s->pic),
-  NEWWORLD_EXTING_GPIO1);
-s->gpio_extirqs[9] = qdev_get_gpio_in(DEVICE(s->pic),
-  NEWWORLD_EXTING_GPIO9);
-}
-
 static void macio_gpio_init(Object *obj)
 {
 SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
 MacIOGPIOState *s = MACIO_GPIO(obj);
+int i;
 
-object_property_add_link(obj, "pic", TYPE_OPENPIC,
- (Object **) >pic,
- qdev_prop_allow_set_link_before_realize,
- 0);
+for (i = 0; i < 10; i++) {
+sysbus_init_irq(sbd, >gpio_extirqs[i]);
+}
 
 memory_region_init_io(>gpiomem, OBJECT(s), _gpio_ops, obj,
   "gpio", 0x30);
@@ -207,7 +194,6 @@ static void macio_gpio_class_init(ObjectClass *oc, void 
*data)
 DeviceClass *dc = DEVICE_CLASS(oc);
 NMIClass *nc = NMI_CLASS(oc);
 
-dc->realize = macio_gpio_realize;
 dc->reset = macio_gpio_reset;
 dc->vmsd = _macio_gpio;
 nc->nmi_monitor_handler = macio_gpio_nmi;
diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index 36be77cede..d17cffd868 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -324,14 +324,16 @@ static void macio_newworld_realize(PCIDevice *d, Error 
**errp)
 
 if (ns->has_pmu) {
 /* GPIOs */
-sysbus_dev = SYS_BUS_DEVICE(>gpio);
-object_property_set_link(OBJECT(>gpio), "pic", OBJECT(pic_dev),
- _abort);
-memory_region_add_subregion(>bar, 0x50,
-sysbus_mmio_get_region(sysbus_dev, 0));
 if (!qdev_realize(DEVICE(>gpio), BUS(>macio_bus), errp)) {
 return;
 }
+sysbus_dev = SYS_BUS_DEVICE(>gpio);
+sysbus_connect_irq(sysbus_dev, 1, qdev_get_gpio_in(pic_dev,
+   NEWWORLD_EXTING_GPIO1));
+sysbus_connect_irq(sysbus_dev, 9, qdev_get_gpio_in(pic_dev,
+   NEWWORLD_EXTING_GPIO9));
+memory_region_add_subregion(>bar, 0x50,
+sysbus_mmio_get_region(sysbus_dev, 0));
 
 /* PMU */
 object_initialize_child(OBJECT(s), "pmu", >pmu, TYPE_VIA_PMU);
diff --git a/include/hw/misc/macio/gpio.h b/include/hw/misc/macio/gpio.h
index 4dee09a9dd..7d2aa886c2 100644
--- a/include/hw/misc/macio/gpio.h
+++ b/include/hw/misc/macio/gpio.h
@@ -38,8 +38,6 @@ struct MacIOGPIOState {
 SysBusDevice parent;
 /*< public >*/
 
-OpenPICState *pic;
-
 MemoryRegion gpiomem;
 qemu_irq gpio_extirqs[10];
 uint8_t gpio_levels[8];
-- 
2.20.1




[PATCH 7/7] macio: don't set user_creatable to false

2020-12-19 Thread Mark Cave-Ayland
Now that all of the object property links to the heathrow PIC and OpenPIC have
been removed from the macio devices, it is safe to allow the macio-oldworld
and macio-neworld devices to be marked as user_creatable.

Signed-off-by: Mark Cave-Ayland 
---
 hw/misc/macio/macio.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index d17cffd868..e6eeb575d5 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -457,8 +457,6 @@ static void macio_class_init(ObjectClass *klass, void *data)
 k->class_id = PCI_CLASS_OTHERS << 8;
 device_class_set_props(dc, macio_properties);
 set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
-/* Reason: requires PIC property links to be set in macio_*_realize() */
-dc->user_creatable = false;
 }
 
 static const TypeInfo macio_bus_info = {
-- 
2.20.1




[PATCH 5/7] macio: move OpenPIC inside macio-newworld device

2020-12-19 Thread Mark Cave-Ayland
Signed-off-by: Mark Cave-Ayland 
---
 hw/misc/macio/macio.c | 19 +--
 hw/ppc/mac_newworld.c | 25 +++--
 include/hw/misc/macio/macio.h |  2 +-
 3 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index cfb87da6c9..36be77cede 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -273,7 +273,7 @@ static void macio_newworld_realize(PCIDevice *d, Error 
**errp)
 {
 MacIOState *s = MACIO(d);
 NewWorldMacIOState *ns = NEWWORLD_MACIO(d);
-DeviceState *pic_dev = DEVICE(ns->pic);
+DeviceState *pic_dev = DEVICE(>pic);
 Error *err = NULL;
 SysBusDevice *sysbus_dev;
 MemoryRegion *timer_memory = NULL;
@@ -284,17 +284,19 @@ static void macio_newworld_realize(PCIDevice *d, Error 
**errp)
 return;
 }
 
+/* OpenPIC */
+qdev_prop_set_uint32(pic_dev, "model", OPENPIC_MODEL_KEYLARGO);
+sysbus_dev = SYS_BUS_DEVICE(>pic);
+sysbus_realize_and_unref(sysbus_dev, _fatal);
+memory_region_add_subregion(>bar, 0x4,
+sysbus_mmio_get_region(sysbus_dev, 0));
+
 sysbus_dev = SYS_BUS_DEVICE(>escc);
 sysbus_connect_irq(sysbus_dev, 0, qdev_get_gpio_in(pic_dev,
NEWWORLD_ESCCB_IRQ));
 sysbus_connect_irq(sysbus_dev, 1, qdev_get_gpio_in(pic_dev,
NEWWORLD_ESCCA_IRQ));
 
-/* OpenPIC */
-sysbus_dev = SYS_BUS_DEVICE(ns->pic);
-memory_region_add_subregion(>bar, 0x4,
-sysbus_mmio_get_region(sysbus_dev, 0));
-
 /* IDE buses */
 macio_realize_ide(s, >ide[0],
   qdev_get_gpio_in(pic_dev, NEWWORLD_IDE0_IRQ),
@@ -369,10 +371,7 @@ static void macio_newworld_init(Object *obj)
 NewWorldMacIOState *ns = NEWWORLD_MACIO(obj);
 int i;
 
-object_property_add_link(obj, "pic", TYPE_OPENPIC,
- (Object **) >pic,
- qdev_prop_allow_set_link_before_realize,
- 0);
+object_initialize_child(OBJECT(s), "pic", >pic, TYPE_OPENPIC);
 
 object_initialize_child(OBJECT(s), "gpio", >gpio, TYPE_MACIO_GPIO);
 
diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 708bb2f1ab..e991db4add 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -293,18 +293,6 @@ static void ppc_core99_init(MachineState *machine)
 }
 }
 
-pic_dev = qdev_new(TYPE_OPENPIC);
-qdev_prop_set_uint32(pic_dev, "model", OPENPIC_MODEL_KEYLARGO);
-s = SYS_BUS_DEVICE(pic_dev);
-sysbus_realize_and_unref(s, _fatal);
-k = 0;
-for (i = 0; i < smp_cpus; i++) {
-for (j = 0; j < OPENPIC_OUTPUT_NB; j++) {
-sysbus_connect_irq(s, k++, openpic_irqs[i].irq[j]);
-}
-}
-g_free(openpic_irqs);
-
 if (PPC_INPUT(env) == PPC_FLAGS_INPUT_970) {
 /* 970 gets a U3 bus */
 /* Uninorth AGP bus */
@@ -378,8 +366,6 @@ static void ppc_core99_init(MachineState *machine)
 qdev_prop_set_uint64(dev, "frequency", tbfreq);
 qdev_prop_set_bit(dev, "has-pmu", has_pmu);
 qdev_prop_set_bit(dev, "has-adb", has_adb);
-object_property_set_link(OBJECT(macio), "pic", OBJECT(pic_dev),
- _abort);
 
 escc = ESCC(object_resolve_path_component(OBJECT(macio), "escc"));
 qdev_prop_set_chr(DEVICE(escc), "chrA", serial_hd(0));
@@ -387,6 +373,7 @@ static void ppc_core99_init(MachineState *machine)
 
 pci_realize_and_unref(macio, pci_bus, _fatal);
 
+pic_dev = DEVICE(object_resolve_path_component(OBJECT(macio), "pic"));
 for (i = 0; i < 4; i++) {
 qdev_connect_gpio_out(DEVICE(uninorth_pci), i,
   qdev_get_gpio_in(pic_dev, 0x1b + i));
@@ -407,6 +394,16 @@ static void ppc_core99_init(MachineState *machine)
 }
 }
 
+/* OpenPIC */
+s = SYS_BUS_DEVICE(pic_dev);
+k = 0;
+for (i = 0; i < smp_cpus; i++) {
+for (j = 0; j < OPENPIC_OUTPUT_NB; j++) {
+sysbus_connect_irq(s, k++, openpic_irqs[i].irq[j]);
+}
+}
+g_free(openpic_irqs);
+
 /* We only emulate 2 out of 3 IDE controllers for now */
 ide_drive_get(hd, ARRAY_SIZE(hd));
 
diff --git a/include/hw/misc/macio/macio.h b/include/hw/misc/macio/macio.h
index 707dfab50c..6c05f3bfd2 100644
--- a/include/hw/misc/macio/macio.h
+++ b/include/hw/misc/macio/macio.h
@@ -115,7 +115,7 @@ struct NewWorldMacIOState {
 
 bool has_pmu;
 bool has_adb;
-OpenPICState *pic;
+OpenPICState pic;
 MACIOIDEState ide[2];
 MacIOGPIOState gpio;
 };
-- 
2.20.1




[PATCH 4/7] mac_newworld: delay wiring of PCI IRQs in New World machine

2020-12-19 Thread Mark Cave-Ayland
This is to prepare for moving the OpenPIC device into the macio-newworld
device.

Signed-off-by: Mark Cave-Ayland 
---
 hw/ppc/mac_newworld.c | 46 ---
 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index c0accda592..708bb2f1ab 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -139,6 +139,7 @@ static void ppc_core99_init(MachineState *machine)
 int machine_arch;
 SysBusDevice *s;
 DeviceState *dev, *pic_dev;
+DeviceState *uninorth_internal_dev = NULL, *uninorth_agp_dev = NULL;
 hwaddr nvram_addr = 0xFFF04000;
 uint64_t tbfreq;
 unsigned int smp_cpus = machine->smp.cpus;
@@ -320,35 +321,24 @@ static void ppc_core99_init(MachineState *machine)
 sysbus_mmio_map(s, 0, 0xf080);
 sysbus_mmio_map(s, 1, 0xf0c0);
 
-for (i = 0; i < 4; i++) {
-qdev_connect_gpio_out(dev, i, qdev_get_gpio_in(pic_dev, 0x1b + i));
-}
-
 machine_arch = ARCH_MAC99_U3;
 } else {
 /* Use values found on a real PowerMac */
 /* Uninorth AGP bus */
-dev = qdev_new(TYPE_UNI_NORTH_AGP_HOST_BRIDGE);
-s = SYS_BUS_DEVICE(dev);
+uninorth_agp_dev = qdev_new(TYPE_UNI_NORTH_AGP_HOST_BRIDGE);
+s = SYS_BUS_DEVICE(uninorth_agp_dev);
 sysbus_realize_and_unref(s, _fatal);
 sysbus_mmio_map(s, 0, 0xf080);
 sysbus_mmio_map(s, 1, 0xf0c0);
 
-for (i = 0; i < 4; i++) {
-qdev_connect_gpio_out(dev, i, qdev_get_gpio_in(pic_dev, 0x1b + i));
-}
-
 /* Uninorth internal bus */
-dev = qdev_new(TYPE_UNI_NORTH_INTERNAL_PCI_HOST_BRIDGE);
-s = SYS_BUS_DEVICE(dev);
+uninorth_internal_dev = qdev_new(
+TYPE_UNI_NORTH_INTERNAL_PCI_HOST_BRIDGE);
+s = SYS_BUS_DEVICE(uninorth_internal_dev);
 sysbus_realize_and_unref(s, _fatal);
 sysbus_mmio_map(s, 0, 0xf480);
 sysbus_mmio_map(s, 1, 0xf4c0);
 
-for (i = 0; i < 4; i++) {
-qdev_connect_gpio_out(dev, i, qdev_get_gpio_in(pic_dev, 0x1b + i));
-}
-
 /* Uninorth main bus */
 dev = qdev_new(TYPE_UNI_NORTH_PCI_HOST_BRIDGE);
 qdev_prop_set_uint32(dev, "ofw-addr", 0xf200);
@@ -364,10 +354,6 @@ static void ppc_core99_init(MachineState *machine)
 sysbus_mmio_map(s, 0, 0xf280);
 sysbus_mmio_map(s, 1, 0xf2c0);
 
-for (i = 0; i < 4; i++) {
-qdev_connect_gpio_out(dev, i, qdev_get_gpio_in(pic_dev, 0x1b + i));
-}
-
 machine_arch = ARCH_MAC99;
 }
 
@@ -401,6 +387,26 @@ static void ppc_core99_init(MachineState *machine)
 
 pci_realize_and_unref(macio, pci_bus, _fatal);
 
+for (i = 0; i < 4; i++) {
+qdev_connect_gpio_out(DEVICE(uninorth_pci), i,
+  qdev_get_gpio_in(pic_dev, 0x1b + i));
+}
+
+/* TODO: additional PCI buses only wired up for 32-bit machines */
+if (PPC_INPUT(env) != PPC_FLAGS_INPUT_970) {
+/* Uninorth AGP bus */
+for (i = 0; i < 4; i++) {
+qdev_connect_gpio_out(uninorth_agp_dev, i,
+  qdev_get_gpio_in(pic_dev, 0x1b + i));
+}
+
+/* Uninorth internal bus */
+for (i = 0; i < 4; i++) {
+qdev_connect_gpio_out(uninorth_internal_dev, i,
+  qdev_get_gpio_in(pic_dev, 0x1b + i));
+}
+}
+
 /* We only emulate 2 out of 3 IDE controllers for now */
 ide_drive_get(hd, ARRAY_SIZE(hd));
 
-- 
2.20.1




[PATCH 3/7] macio: move heathrow PIC inside macio-oldworld device

2020-12-19 Thread Mark Cave-Ayland
Signed-off-by: Mark Cave-Ayland 
---
 hw/misc/macio/macio.c | 20 +--
 hw/ppc/mac_oldworld.c | 66 +--
 include/hw/misc/macio/macio.h |  2 +-
 3 files changed, 43 insertions(+), 45 deletions(-)

diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index bb601f782c..cfb87da6c9 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -140,7 +140,7 @@ static void macio_oldworld_realize(PCIDevice *d, Error 
**errp)
 {
 MacIOState *s = MACIO(d);
 OldWorldMacIOState *os = OLDWORLD_MACIO(d);
-DeviceState *pic_dev = DEVICE(os->pic);
+DeviceState *pic_dev = DEVICE(>pic);
 Error *err = NULL;
 SysBusDevice *sysbus_dev;
 
@@ -150,6 +150,14 @@ static void macio_oldworld_realize(PCIDevice *d, Error 
**errp)
 return;
 }
 
+/* Heathrow PIC */
+if (!qdev_realize(DEVICE(>pic), BUS(>macio_bus), errp)) {
+return;
+}
+sysbus_dev = SYS_BUS_DEVICE(>pic);
+memory_region_add_subregion(>bar, 0x0,
+sysbus_mmio_get_region(sysbus_dev, 0));
+
 qdev_prop_set_uint64(DEVICE(>cuda), "timebase-frequency",
  s->frequency);
 if (!qdev_realize(DEVICE(>cuda), BUS(>macio_bus), errp)) {
@@ -175,11 +183,6 @@ static void macio_oldworld_realize(PCIDevice *d, Error 
**errp)
 sysbus_mmio_get_region(sysbus_dev, 0));
 pmac_format_nvram_partition(>nvram, os->nvram.size);
 
-/* Heathrow PIC */
-sysbus_dev = SYS_BUS_DEVICE(os->pic);
-memory_region_add_subregion(>bar, 0x0,
-sysbus_mmio_get_region(sysbus_dev, 0));
-
 /* IDE buses */
 macio_realize_ide(s, >ide[0],
   qdev_get_gpio_in(pic_dev, OLDWORLD_IDE0_IRQ),
@@ -218,10 +221,7 @@ static void macio_oldworld_init(Object *obj)
 DeviceState *dev;
 int i;
 
-object_property_add_link(obj, "pic", TYPE_HEATHROW,
- (Object **) >pic,
- qdev_prop_allow_set_link_before_realize,
- 0);
+object_initialize_child(OBJECT(s), "pic", >pic, TYPE_HEATHROW);
 
 object_initialize_child(OBJECT(s), "cuda", >cuda, TYPE_CUDA);
 
diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index e58e0525fe..44ee99be88 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -98,7 +98,7 @@ static void ppc_heathrow_init(MachineState *machine)
 MACIOIDEState *macio_ide;
 ESCCState *escc;
 SysBusDevice *s;
-DeviceState *dev, *pic_dev;
+DeviceState *dev, *pic_dev, *grackle_dev;
 BusState *adb_bus;
 uint64_t bios_addr;
 int bios_size;
@@ -227,10 +227,17 @@ static void ppc_heathrow_init(MachineState *machine)
 }
 }
 
+/* Timebase Frequency */
+if (kvm_enabled()) {
+tbfreq = kvmppc_get_tbfreq();
+} else {
+tbfreq = TBFREQ;
+}
+
 /* Grackle PCI host bridge */
-dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
-qdev_prop_set_uint32(dev, "ofw-addr", 0x8000);
-s = SYS_BUS_DEVICE(dev);
+grackle_dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
+qdev_prop_set_uint32(grackle_dev, "ofw-addr", 0x8000);
+s = SYS_BUS_DEVICE(grackle_dev);
 sysbus_realize_and_unref(s, _fatal);
 
 sysbus_mmio_map(s, 0, GRACKLE_BASE);
@@ -242,14 +249,30 @@ static void ppc_heathrow_init(MachineState *machine)
 memory_region_add_subregion(get_system_memory(), 0xfe00,
 sysbus_mmio_get_region(s, 3));
 
-/* XXX: we register only 1 output pin for heathrow PIC */
-pic_dev = qdev_new(TYPE_HEATHROW);
-sysbus_realize_and_unref(SYS_BUS_DEVICE(pic_dev), _fatal);
+pci_bus = PCI_HOST_BRIDGE(grackle_dev)->bus;
+
+/* MacIO */
+macio = pci_new(PCI_DEVFN(16, 0), TYPE_OLDWORLD_MACIO);
+dev = DEVICE(macio);
+qdev_prop_set_uint64(dev, "frequency", tbfreq);
+
+escc = ESCC(object_resolve_path_component(OBJECT(macio), "escc"));
+qdev_prop_set_chr(DEVICE(escc), "chrA", serial_hd(0));
+qdev_prop_set_chr(DEVICE(escc), "chrB", serial_hd(1));
+
+pci_realize_and_unref(macio, pci_bus, _fatal);
+
+pic_dev = DEVICE(object_resolve_path_component(OBJECT(macio), "pic"));
+for (i = 0; i < 4; i++) {
+qdev_connect_gpio_out(grackle_dev, i,
+  qdev_get_gpio_in(pic_dev, 0x15 + i));
+}
 
 /* Connect the heathrow PIC outputs to the 6xx bus */
 for (i = 0; i < smp_cpus; i++) {
 switch (PPC_INPUT(env)) {
 case PPC_FLAGS_INPUT_6xx:
+/* XXX: we register only 1 output pin for heathrow PIC */
 qdev_connect_gpio_out(pic_dev, 0,
 ((qemu_irq *)env->irq_inputs)[PPC6xx_INPUT_INT]);
 break;
@@ -259,40 +282,14 @@ static void ppc_heathrow_init(MachineState *machine)
 }
 }
 
-/* Timebase Frequency */
-if (kvm_enabled()) {
-tbfreq = kvmppc_get_tbfreq();
-} else {
-

[PATCH 2/7] mac_oldworld: move initialisation of grackle before heathrow

2020-12-19 Thread Mark Cave-Ayland
Signed-off-by: Mark Cave-Ayland 
---
 hw/ppc/mac_oldworld.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 2ead34bdf1..e58e0525fe 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -227,6 +227,21 @@ static void ppc_heathrow_init(MachineState *machine)
 }
 }
 
+/* Grackle PCI host bridge */
+dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
+qdev_prop_set_uint32(dev, "ofw-addr", 0x8000);
+s = SYS_BUS_DEVICE(dev);
+sysbus_realize_and_unref(s, _fatal);
+
+sysbus_mmio_map(s, 0, GRACKLE_BASE);
+sysbus_mmio_map(s, 1, GRACKLE_BASE + 0x20);
+/* PCI hole */
+memory_region_add_subregion(get_system_memory(), 0x8000ULL,
+sysbus_mmio_get_region(s, 2));
+/* Register 2 MB of ISA IO space */
+memory_region_add_subregion(get_system_memory(), 0xfe00,
+sysbus_mmio_get_region(s, 3));
+
 /* XXX: we register only 1 output pin for heathrow PIC */
 pic_dev = qdev_new(TYPE_HEATHROW);
 sysbus_realize_and_unref(SYS_BUS_DEVICE(pic_dev), _fatal);
@@ -251,21 +266,6 @@ static void ppc_heathrow_init(MachineState *machine)
 tbfreq = TBFREQ;
 }
 
-/* Grackle PCI host bridge */
-dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
-qdev_prop_set_uint32(dev, "ofw-addr", 0x8000);
-s = SYS_BUS_DEVICE(dev);
-sysbus_realize_and_unref(s, _fatal);
-
-sysbus_mmio_map(s, 0, GRACKLE_BASE);
-sysbus_mmio_map(s, 1, GRACKLE_BASE + 0x20);
-/* PCI hole */
-memory_region_add_subregion(get_system_memory(), 0x8000ULL,
-sysbus_mmio_get_region(s, 2));
-/* Register 2 MB of ISA IO space */
-memory_region_add_subregion(get_system_memory(), 0xfe00,
-sysbus_mmio_get_region(s, 3));
-
 for (i = 0; i < 4; i++) {
 qdev_connect_gpio_out(dev, i, qdev_get_gpio_in(pic_dev, 0x15 + i));
 }
-- 
2.20.1




[PATCH 1/7] mac_oldworld: remove duplicate bus check for PPC_INPUT(env)

2020-12-19 Thread Mark Cave-Ayland
This condition will have already been caught when wiring the heathrow PIC
irqs to the CPU.

Signed-off-by: Mark Cave-Ayland 
---
 hw/ppc/mac_oldworld.c | 6 --
 1 file changed, 6 deletions(-)

diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index 04f98a4d81..2ead34bdf1 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -251,12 +251,6 @@ static void ppc_heathrow_init(MachineState *machine)
 tbfreq = TBFREQ;
 }
 
-/* init basic PC hardware */
-if (PPC_INPUT(env) != PPC_FLAGS_INPUT_6xx) {
-error_report("Only 6xx bus is supported on heathrow machine");
-exit(1);
-}
-
 /* Grackle PCI host bridge */
 dev = qdev_new(TYPE_GRACKLE_PCI_HOST_BRIDGE);
 qdev_prop_set_uint32(dev, "ofw-addr", 0x8000);
-- 
2.20.1




[PATCH 0/7] macio: remove PIC object property links

2020-12-19 Thread Mark Cave-Ayland
This patchset follows on from the dicussion at 
https://lists.gnu.org/archive/html/qemu-devel/2020-11/msg02630.html
where the user_creatable flag for the macio devices was set back to false just
before the 5.2 release.

The underlying cause was that the PIC object property links were not being set
before realise. Whilst this cannot happen when launching the g3beige and mac99
machines from qemu-system-ppc, it caused some automated tests to fail.

Here we fix the real problem which is to move the PIC for both machines into the
macio device, which not only matches real hardware but also enables the PIC 
object
property links to be completely removed.

Patch 6 rewires the macio gpios for the mac99 machine as per Ben's original 
comment
after the OpenPIC device has been moved into the macio-newworld device, and then
finally patch 7 removes setting the user_creatable flag to false on the macio 
devices
once again.

Signed-off-by: Mark Cave-Ayland 


Mark Cave-Ayland (7):
  mac_oldworld: remove duplicate bus check for PPC_INPUT(env)
  mac_oldworld: move initialisation of grackle before heathrow
  macio: move heathrow PIC inside macio-oldworld device
  mac_newworld: delay wiring of PCI IRQs in New World machine
  macio: move OpenPIC inside macio-newworld device
  macio: wire macio GPIOs to OpenPIC using sysbus IRQs
  macio: don't set user_creatable to false

 hw/misc/macio/gpio.c  | 24 +++
 hw/misc/macio/macio.c | 53 
 hw/ppc/mac_newworld.c | 71 
 hw/ppc/mac_oldworld.c | 76 ---
 include/hw/misc/macio/gpio.h  |  2 -
 include/hw/misc/macio/macio.h |  4 +-
 6 files changed, 104 insertions(+), 126 deletions(-)

-- 
2.20.1




Re: [PATCH v2 0/7] Common macros for QAPI list growth

2020-12-19 Thread Markus Armbruster
Markus Armbruster  writes:

> Eric Blake  writes:
>
>> v1, as such, was here:
>> https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg08003.html
>> (v6 11/11 qapi: Use QAPI_LIST_ADD() where possible)
>>
>> since then, I've rebased that patch (upstream went with PREPEND
>> instead of ADD), split things out for easier review, added
>> QAPI_LIST_APPEND, caught a lot more places that can use PREPEND, and
>> even fixed a years-old memory leak that might be worth having in 5.2.
>> But patches 2-7 are 6.0 material.
> [...]
>>  55 files changed, 431 insertions(+), 1007 deletions(-)
>
> Lovely.  These two macros are obviously overdue.

I'm queuing PATCH 2-4.  We concluced PATCH 1 isn't necessary.  PATCH 7
needs work, and I gather you'd like to improve PATCH 5 (parenthesis in
macro expansion) and 6 (use the opportunity to improve variable naming).

> Series needs a rebase.

The conflicts are due to

a8aa94b5f8 qga: update schema for guest-get-disks 'dependents' field
a10b453a52 target/mips: Move mips_cpu_add_definition() from helper.c to 
cpu.c

and easy enough to resolve for me.

Thanks!




Re: [PATCH 0/2] Cleanup internal WHPX headers

2020-12-19 Thread no-reply
Patchew URL: 
https://patchew.org/QEMU/20201219090637.1700900-1-pbonz...@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20201219090637.1700900-1-pbonz...@redhat.com
Subject: [PATCH 0/2] Cleanup internal WHPX headers

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag] patchew/20201219090637.1700900-1-pbonz...@redhat.com -> 
patchew/20201219090637.1700900-1-pbonz...@redhat.com
Switched to a new branch 'test'
aada1bf whpx: move internal definitions to whpx-internal.h
2b0771a whpx: rename whp-dispatch to whpx-internal.h

=== OUTPUT BEGIN ===
1/2 Checking commit 2b0771ae8ad0 (whpx: rename whp-dispatch to whpx-internal.h)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#64: 
rename from target/i386/whpx/whp-dispatch.h

total: 0 errors, 1 warnings, 46 lines checked

Patch 1/2 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/2 Checking commit aada1bf26255 (whpx: move internal definitions to 
whpx-internal.h)
ERROR: open brace '{' following function declarations go on the next line
#56: FILE: target/i386/whpx/whpx-all.c:1869:
+bool whpx_apic_in_platform(void) {

total: 1 errors, 0 warnings, 63 lines checked

Patch 2/2 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20201219090637.1700900-1-pbonz...@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

[PATCH 0/2] Cleanup internal WHPX headers

2020-12-19 Thread Paolo Bonzini
Remove Windows type usage from sysemu/whpx.h, because
the whp-dispatch.h header is internal to the WHPX accelerator
implementation.  Fixes WHPX build.

Paolo Bonzini (2):
  whpx: rename whp-dispatch to whpx-internal.h
  whpx: move internal definitions to whpx-internal.h

 include/sysemu/whpx.h | 22 +
 target/i386/whpx/whpx-all.c   |  9 +++
 target/i386/whpx/whpx-apic.c  |  2 +-
 target/i386/whpx/whpx-cpus.c  |  4 +---
 .../whpx/{whp-dispatch.h => whpx-internal.h}  | 24 ---
 5 files changed, 29 insertions(+), 32 deletions(-)
 rename target/i386/whpx/{whp-dispatch.h => whpx-internal.h} (91%)

-- 
2.26.2




[PATCH 1/2] whpx: rename whp-dispatch to whpx-internal.h

2020-12-19 Thread Paolo Bonzini
Rename the file in preparation for moving more implementation-internal
definitions to it.  The build is still broken though.

Signed-off-by: Paolo Bonzini 
---
 target/i386/whpx/whpx-all.c  | 5 +
 target/i386/whpx/whpx-apic.c | 2 +-
 target/i386/whpx/whpx-cpus.c | 4 +---
 target/i386/whpx/{whp-dispatch.h => whpx-internal.h} | 6 +++---
 4 files changed, 6 insertions(+), 11 deletions(-)
 rename target/i386/whpx/{whp-dispatch.h => whpx-internal.h} (98%)

diff --git a/target/i386/whpx/whpx-all.c b/target/i386/whpx/whpx-all.c
index 3b824fc9d7..12f79e2cd6 100644
--- a/target/i386/whpx/whpx-all.c
+++ b/target/i386/whpx/whpx-all.c
@@ -26,13 +26,10 @@
 #include "qapi/qapi-types-common.h"
 #include "qapi/qapi-visit-common.h"
 #include "migration/blocker.h"
-#include "whp-dispatch.h"
 #include 
 
 #include "whpx-cpus.h"
-
-#include 
-#include 
+#include "whpx-internal.h"
 
 #define HYPERV_APIC_BUS_FREQUENCY  (2ULL)
 
diff --git a/target/i386/whpx/whpx-apic.c b/target/i386/whpx/whpx-apic.c
index b127a3cb8a..1d330bf749 100644
--- a/target/i386/whpx/whpx-apic.c
+++ b/target/i386/whpx/whpx-apic.c
@@ -18,7 +18,7 @@
 #include "hw/pci/msi.h"
 #include "sysemu/hw_accel.h"
 #include "sysemu/whpx.h"
-#include "whp-dispatch.h"
+#include "whpx-internal.h"
 
 static void whpx_put_apic_state(APICCommonState *s,
 struct whpx_lapic_state *kapic)
diff --git a/target/i386/whpx/whpx-cpus.c b/target/i386/whpx/whpx-cpus.c
index d9bd5a2d36..f7e69881a3 100644
--- a/target/i386/whpx/whpx-cpus.c
+++ b/target/i386/whpx/whpx-cpus.c
@@ -15,11 +15,9 @@
 #include "qemu/guest-random.h"
 
 #include "sysemu/whpx.h"
+#include "whpx-internal.h"
 #include "whpx-cpus.h"
 
-#include 
-#include 
-
 static void *whpx_cpu_thread_fn(void *arg)
 {
 CPUState *cpu = arg;
diff --git a/target/i386/whpx/whp-dispatch.h b/target/i386/whpx/whpx-internal.h
similarity index 98%
rename from target/i386/whpx/whp-dispatch.h
rename to target/i386/whpx/whpx-internal.h
index cef5d848bd..e0a9ea1dce 100644
--- a/target/i386/whpx/whp-dispatch.h
+++ b/target/i386/whpx/whpx-internal.h
@@ -1,5 +1,5 @@
-#ifndef WHP_DISPATCH_H
-#define WHP_DISPATCH_H
+#ifndef WHP_INTERNAL_H
+#define WHP_INTERNAL_H
 
 #include 
 #include 
@@ -72,4 +72,4 @@ typedef enum WHPFunctionList {
 WINHV_PLATFORM_FNS_SUPPLEMENTAL
 } WHPFunctionList;
 
-#endif /* WHP_DISPATCH_H */
+#endif /* WHP_INTERNAL_H */
-- 
2.26.2





[PATCH 2/2] whpx: move internal definitions to whpx-internal.h

2020-12-19 Thread Paolo Bonzini
Only leave the external interface in sysemu/whpx.h.  whpx_apic_in_platform
is moved to a .c file because it needs whpx_state.

Reported-by: Marc-André Lureau 
Signed-off-by: Paolo Bonzini 
---
 include/sysemu/whpx.h| 22 +-
 target/i386/whpx/whpx-all.c  |  4 
 target/i386/whpx/whpx-internal.h | 18 ++
 3 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/include/sysemu/whpx.h b/include/sysemu/whpx.h
index 9346fd92e9..8ca1c1c4ac 100644
--- a/include/sysemu/whpx.h
+++ b/include/sysemu/whpx.h
@@ -15,28 +15,8 @@
 
 #ifdef CONFIG_WHPX
 
-#include "whp-dispatch.h"
-
-struct whpx_state {
-uint64_t mem_quota;
-WHV_PARTITION_HANDLE partition;
-bool kernel_irqchip_allowed;
-bool kernel_irqchip_required;
-bool apic_in_platform;
-};
-
-struct whpx_lapic_state {
-struct {
-uint32_t data;
-uint32_t padding[3];
-} fields[256];
-};
-
-extern struct whpx_state whpx_global;
 int whpx_enabled(void);
-
-void whpx_apic_get(DeviceState *s);
-#define whpx_apic_in_platform() (whpx_global.apic_in_platform)
+bool whpx_apic_in_platform(void);
 
 #else /* CONFIG_WHPX */
 
diff --git a/target/i386/whpx/whpx-all.c b/target/i386/whpx/whpx-all.c
index 12f79e2cd6..985ceba8f8 100644
--- a/target/i386/whpx/whpx-all.c
+++ b/target/i386/whpx/whpx-all.c
@@ -1866,6 +1866,10 @@ int whpx_enabled(void)
 return whpx_allowed;
 }
 
+bool whpx_apic_in_platform(void) {
+return whpx_global.apic_in_platform;
+}
+
 static void whpx_accel_class_init(ObjectClass *oc, void *data)
 {
 AccelClass *ac = ACCEL_CLASS(oc);
diff --git a/target/i386/whpx/whpx-internal.h b/target/i386/whpx/whpx-internal.h
index e0a9ea1dce..8cfaaef141 100644
--- a/target/i386/whpx/whpx-internal.h
+++ b/target/i386/whpx/whpx-internal.h
@@ -5,6 +5,24 @@
 #include 
 #include 
 
+struct whpx_state {
+uint64_t mem_quota;
+WHV_PARTITION_HANDLE partition;
+bool kernel_irqchip_allowed;
+bool kernel_irqchip_required;
+bool apic_in_platform;
+};
+
+struct whpx_lapic_state {
+struct {
+uint32_t data;
+uint32_t padding[3];
+} fields[256];
+};
+
+extern struct whpx_state whpx_global;
+void whpx_apic_get(DeviceState *s);
+
 #define WHV_E_UNKNOWN_CAPABILITY 0x80370300L
 
 #define LIST_WINHVPLATFORM_FUNCTIONS(X) \
-- 
2.26.2




Re: [PATCH v12 0/7] Introduce 'yank' oob qmp command to recover from hanging qemu

2020-12-19 Thread Markus Armbruster
Markus Armbruster  writes:

> Lukas Straub  writes:
>
>> Hello Everyone,
>> So here is v12.
>> @Marc-André Lureau, We still need an ACK for the chardev patch.
>
> Marc-André?  Would be good to get this wrapped before Christmas.

No go.

Unless someone objects, I'll merge this when I'm back (mid January) with
or without an ACK for chardev.