[Qemu-devel] [PATCH 0/6] configure: Try to fix --static linking

[Philippe Mathieu-Daudé]

Hi,

Apparently QEMU static linking is slowly bitroting. Obviously it
depends the libraries an user has installed, anyway it seems there
are not much testing done.

This series fixes few issues, enough to build QEMU on a Ubuntu
aarch64 host, but not yet on a x86_64 host:

LINKx86_64-softmmu/qemu-system-x86_64
  /usr/bin/ld: cannot find -lgtk-3
  /usr/bin/ld: cannot find -latk-bridge-2.0
  /usr/bin/ld: cannot find -latspi
  /usr/bin/ld: cannot find -lsystemd
  /usr/bin/ld: cannot find -lgdk-3
  /usr/bin/ld: cannot find -lwayland-egl
  /usr/bin/ld: cannot find -lmirclient
  /usr/bin/ld: cannot find -lmircore
  /usr/bin/ld: cannot find -lmircookie
  /usr/bin/ld: cannot find -lepoxy
  /usr/bin/ld: cannot find -latk-1.0
  /usr/bin/ld: cannot find -lgdk_pixbuf-2.0
  /usr/bin/ld: cannot find -lselinux
  /usr/bin/ld: cannot find -lgtk-3
  /usr/bin/ld: cannot find -latk-bridge-2.0
  /usr/bin/ld: cannot find -latspi
  /usr/bin/ld: cannot find -lsystemd
  /usr/bin/ld: cannot find -lgdk-3
  /usr/bin/ld: cannot find -lwayland-egl
  /usr/bin/ld: cannot find -lmirclient
  /usr/bin/ld: cannot find -lmircore
  /usr/bin/ld: cannot find -lmircookie
  /usr/bin/ld: cannot find -lepoxy
  /usr/bin/ld: cannot find -latk-1.0
  /usr/bin/ld: cannot find -lgdk_pixbuf-2.0
  /usr/bin/ld: cannot find -lselinux
  /usr/bin/ld: attempted static link of dynamic object 
`/usr/lib/x86_64-linux-gnu/libz.so'
  collect2: error: ld returned 1 exit status

Regards,

Phil.

Philippe Mathieu-Daudé (6):
  configure: Only generate GLUSTERFS variables if glusterfs is usable
  configure: Link test before auto-enabling glusterfs libraries
  configure: Link test before auto-enabling the libusb library
  configure: Link test before auto-enabling the libusbredir library
  configure: Link test before auto-enabling the pulseaudio library
  .travis.yml: Test static linking

 .travis.yml |   5 +++
 configure   | 113 +++-
 2 files changed, 90 insertions(+), 28 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH 1/6] configure: Only generate GLUSTERFS variables if glusterfs is usable

[Philippe Mathieu-Daudé]

It is pointless and confusing to have GLUSTERFS variables
in config-host.mak when glusterfs is not usable.

Signed-off-by: Philippe Mathieu-Daudé 
---
 configure | 36 ++--
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/configure b/configure
index b091b82cb3..13fd4a1166 100755
--- a/configure
+++ b/configure
@@ -7118,30 +7118,30 @@ if test "$glusterfs" = "yes" ; then
   echo "CONFIG_GLUSTERFS=m" >> $config_host_mak
   echo "GLUSTERFS_CFLAGS=$glusterfs_cflags" >> $config_host_mak
   echo "GLUSTERFS_LIBS=$glusterfs_libs" >> $config_host_mak
-fi
 
-if test "$glusterfs_xlator_opt" = "yes" ; then
-  echo "CONFIG_GLUSTERFS_XLATOR_OPT=y" >> $config_host_mak
-fi
+  if test "$glusterfs_xlator_opt" = "yes" ; then
+echo "CONFIG_GLUSTERFS_XLATOR_OPT=y" >> $config_host_mak
+  fi
 
-if test "$glusterfs_discard" = "yes" ; then
-  echo "CONFIG_GLUSTERFS_DISCARD=y" >> $config_host_mak
-fi
+  if test "$glusterfs_discard" = "yes" ; then
+echo "CONFIG_GLUSTERFS_DISCARD=y" >> $config_host_mak
+  fi
 
-if test "$glusterfs_fallocate" = "yes" ; then
-  echo "CONFIG_GLUSTERFS_FALLOCATE=y" >> $config_host_mak
-fi
+  if test "$glusterfs_fallocate" = "yes" ; then
+echo "CONFIG_GLUSTERFS_FALLOCATE=y" >> $config_host_mak
+  fi
 
-if test "$glusterfs_zerofill" = "yes" ; then
-  echo "CONFIG_GLUSTERFS_ZEROFILL=y" >> $config_host_mak
-fi
+  if test "$glusterfs_zerofill" = "yes" ; then
+echo "CONFIG_GLUSTERFS_ZEROFILL=y" >> $config_host_mak
+  fi
 
-if test "$glusterfs_ftruncate_has_stat" = "yes" ; then
-  echo "CONFIG_GLUSTERFS_FTRUNCATE_HAS_STAT=y" >> $config_host_mak
-fi
+  if test "$glusterfs_ftruncate_has_stat" = "yes" ; then
+echo "CONFIG_GLUSTERFS_FTRUNCATE_HAS_STAT=y" >> $config_host_mak
+  fi
 
-if test "$glusterfs_iocb_has_stat" = "yes" ; then
-  echo "CONFIG_GLUSTERFS_IOCB_HAS_STAT=y" >> $config_host_mak
+  if test "$glusterfs_iocb_has_stat" = "yes" ; then
+echo "CONFIG_GLUSTERFS_IOCB_HAS_STAT=y" >> $config_host_mak
+  fi
 fi
 
 if test "$libssh2" = "yes" ; then
-- 
2.20.1

[Qemu-devel] [PATCH 2/6] configure: Link test before auto-enabling glusterfs libraries

[Philippe Mathieu-Daudé]

Similarly to commit a73e82ef912, test the libraries link correctly
before considering them as usable.

This fixes using ./configure --static on Ubuntu 18.04:

  $ make subdir-aarch64-softmmu
  [...]
LINKaarch64-softmmu/qemu-system-aarch64
  /usr/bin/ld: cannot find -lgfapi
  /usr/bin/ld: cannot find -lglusterfs
  /usr/bin/ld: cannot find -lgfrpc
  /usr/bin/ld: cannot find -lgfxdr
  collect2: error: ld returned 1 exit status
  Makefile:204: recipe for target 'qemu-system-aarch64' failed
  make[1]: *** [qemu-system-aarch64] Error 1

  $ fgrep gf config-host.mak
  GLUSTERFS_LIBS=-lacl -lgfapi -lglusterfs -lgfrpc -lgfxdr -luuid

  $ lsb_release -cri
  Distributor ID: Ubuntu
  Release:18.04
  Codename:   bionic

Signed-off-by: Philippe Mathieu-Daudé 
---
 configure | 18 --
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 13fd4a1166..3428adb75b 100755
--- a/configure
+++ b/configure
@@ -4179,9 +4179,23 @@ fi
 # glusterfs probe
 if test "$glusterfs" != "no" ; then
   if $pkg_config --atleast-version=3 glusterfs-api; then
-glusterfs="yes"
 glusterfs_cflags=$($pkg_config --cflags glusterfs-api)
-glusterfs_libs=$($pkg_config --libs glusterfs-api)
+if test "$static" = "yes"; then
+glusterfs_libs=$($pkg_config --libs --static glusterfs-api)
+else
+glusterfs_libs=$($pkg_config --libs glusterfs-api)
+fi
+# Packaging for the static libraries is not always correct.
+# At least ubuntu 18.04 ships only shared libraries.
+write_c_skeleton
+if ! compile_prog "$glusterfs_cflags" "$glusterfs_libs" ; then
+if test "$glusterfs" = "yes" ; then
+  error_exit "glusterfs check failed."
+fi
+glusterfs="no"
+else
+glusterfs="yes"
+fi
 if $pkg_config --atleast-version=4 glusterfs-api; then
   glusterfs_xlator_opt="yes"
 fi
-- 
2.20.1

[Qemu-devel] [PATCH 4/6] configure: Link test before auto-enabling the libusbredir library

[Philippe Mathieu-Daudé]

Similarly to commit a73e82ef912, test the library links correctly
before considering it as usable.

This fixes using ./configure --static on Ubuntu 18.04:

  $ make subdir-aarch64-softmmu
  [...]
LINKaarch64-softmmu/qemu-system-aarch64
  /usr/bin/ld: cannot find -lusbredirparser
  collect2: error: ld returned 1 exit status
  Makefile:204: recipe for target 'qemu-system-aarch64' failed
  make[1]: *** [qemu-system-aarch64] Error 1

  $ fgrep redir config-host.mak
  USB_REDIR_LIBS=-lusbredirparser

  $ lsb_release -cri
  Distributor ID: Ubuntu
  Release:18.04
  Codename:   bionic

Signed-off-by: Philippe Mathieu-Daudé 
---
 configure | 18 --
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index e2511df1e3..4eed33e1b1 100755
--- a/configure
+++ b/configure
@@ -4926,9 +4926,23 @@ fi
 # check for usbredirparser for usb network redirection support
 if test "$usb_redir" != "no" ; then
 if $pkg_config --atleast-version=0.6 libusbredirparser-0.5; then
-usb_redir="yes"
 usb_redir_cflags=$($pkg_config --cflags libusbredirparser-0.5)
-usb_redir_libs=$($pkg_config --libs libusbredirparser-0.5)
+if test "$static" = "yes"; then
+usb_redir_libs=$($pkg_config --libs --static libusbredirparser-0.5)
+else
+usb_redir_libs=$($pkg_config --libs libusbredirparser-0.5)
+fi
+# Packaging for the static libraries is not always correct.
+# At least ubuntu 18.04 ships only shared libraries.
+write_c_skeleton
+if ! compile_prog "$usb_redir_cflags" "$usb_redir_libs" ; then
+if test "$usb_redir" = "yes" ; then
+  error_exit "usbredir check failed."
+fi
+usb_redir="no"
+else
+usb_redir="yes"
+fi
 else
 if test "$usb_redir" = "yes"; then
 feature_not_found "usb-redir" "Install usbredir devel"
-- 
2.20.1

[Qemu-devel] [PATCH 3/6] configure: Link test before auto-enabling the libusb library

[Philippe Mathieu-Daudé]

Similarly to commit a73e82ef912, test the library links correctly
before considering it as usable.

This fixes using ./configure --static on Ubuntu 18.04:

  $ make subdir-aarch64-softmmu
  [...]
LINKaarch64-softmmu/qemu-system-aarch64
  /usr/bin/ld: cannot find -ludev
  collect2: error: ld returned 1 exit status
  Makefile:204: recipe for target 'qemu-system-aarch64' failed
  make[1]: *** [qemu-system-aarch64] Error 1

  $ fgrep udev config-host.mak
  LIBUSB_LIBS=-lusb-1.0 -ludev -pthread

  $ lsb_release -cri
  Distributor ID: Ubuntu
  Release:18.04
  Codename:   bionic

Signed-off-by: Philippe Mathieu-Daudé 
---
 configure | 18 --
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index 3428adb75b..e2511df1e3 100755
--- a/configure
+++ b/configure
@@ -4898,9 +4898,23 @@ fi
 # check for libusb
 if test "$libusb" != "no" ; then
 if $pkg_config --atleast-version=1.0.13 libusb-1.0; then
-libusb="yes"
 libusb_cflags=$($pkg_config --cflags libusb-1.0)
-libusb_libs=$($pkg_config --libs libusb-1.0)
+if test "$static" = "yes"; then
+libusb_libs=$($pkg_config --libs --static libusb-1.0)
+else
+libusb_libs=$($pkg_config --libs libusb-1.0)
+fi
+# Packaging for the static libraries is not always correct.
+# At least ubuntu 18.04 ships only shared libraries.
+write_c_skeleton
+if ! compile_prog "$libusb_cflags" "$libusb_libs" ; then
+if test "$libusb" = "yes" ; then
+  error_exit "libusb check failed."
+fi
+libusb="no"
+else
+libusb="yes"
+fi
 else
 if test "$libusb" = "yes"; then
 feature_not_found "libusb" "Install libusb devel >= 1.0.13"
-- 
2.20.1

[Qemu-devel] [PATCH 5/6] configure: Link test before auto-enabling the pulseaudio library

[Philippe Mathieu-Daudé]

Similarly to commit a73e82ef912, test the library links correctly
before considering it as usable.

This fixes using ./configure --static on Ubuntu 18.04:

  $ make subdir-aarch64-softmmu
  [...]
LINKaarch64-softmmu/qemu-system-aarch64
  /usr/bin/ld: cannot find -lpulse
  /usr/bin/ld: cannot find -lpulsecommon-11.1
  collect2: error: ld returned 1 exit status
  Makefile:204: recipe for target 'qemu-system-aarch64' failed
  make[1]: *** [qemu-system-aarch64] Error 1

  $ fgrep pulse config-host.mak
  PULSE_LIBS=-L/usr/lib/aarch64-linux-gnu/pulseaudio -lpulse -lpulsecommon-11.1

  $ lsb_release -cri
  Distributor ID: Ubuntu
  Release:18.04
  Codename:   bionic

Signed-off-by: Philippe Mathieu-Daudé 
---
 configure | 23 +++
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index 4eed33e1b1..4d015496ae 100755
--- a/configure
+++ b/configure
@@ -3408,10 +3408,25 @@ for drv in $audio_drv_list; do
 
 pa | try-pa)
 if $pkg_config libpulse --exists; then
-pulse_libs=$($pkg_config libpulse --libs)
-audio_pt_int="yes"
-if test "$drv" = "try-pa"; then
-audio_drv_list=$(echo "$audio_drv_list" | sed -e 's/try-pa/pa/')
+pulse_cflags=$($pkg_config --cflags libpulse)
+if test "$static" = "yes"; then
+pulse_libs=$($pkg_config --libs --static libpulse)
+else
+pulse_libs=$($pkg_config --libs libpulse)
+fi
+# Packaging for the static libraries is not always correct.
+# At least ubuntu 18.04 ships only shared libraries.
+write_c_skeleton
+if ! compile_prog "$pulse_cflags" "$pulse_libs" ; then
+unset pulse_cflags pulse_libs
+if test "$drv" = "try-pa"; then
+audio_drv_list=$(echo "$audio_drv_list" | sed -e 's/try-pa//')
+fi
+else
+audio_pt_int="yes"
+if test "$drv" = "try-pa"; then
+audio_drv_list=$(echo "$audio_drv_list" | sed -e 
's/try-pa/pa/')
+fi
 fi
 else
 if test "$drv" = "try-pa"; then
-- 
2.20.1

[Qemu-devel] [PATCH 6/6] .travis.yml: Test static linking

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
This job currently fails:

  LINKlm32-softmmu/qemu-system-lm32
/usr/bin/ld: cannot find -lgtk-3
/usr/bin/ld: cannot find -latk-bridge-2.0
/usr/bin/ld: cannot find -latspi
/usr/bin/ld: cannot find -lsystemd
/usr/bin/ld: cannot find -lgdk-3
/usr/bin/ld: cannot find -lwayland-egl
/usr/bin/ld: cannot find -lmirclient
/usr/bin/ld: cannot find -lmircore
/usr/bin/ld: cannot find -lmircookie
/usr/bin/ld: cannot find -lepoxy
/usr/bin/ld: cannot find -latk-1.0
/usr/bin/ld: cannot find -lgdk_pixbuf-2.0
/usr/bin/ld: cannot find -lselinux
/usr/bin/ld: cannot find -lgtk-3
/usr/bin/ld: cannot find -latk-bridge-2.0
/usr/bin/ld: cannot find -latspi
/usr/bin/ld: cannot find -lsystemd
/usr/bin/ld: cannot find -lgdk-3
/usr/bin/ld: cannot find -lwayland-egl
/usr/bin/ld: cannot find -lmirclient
/usr/bin/ld: cannot find -lmircore
/usr/bin/ld: cannot find -lmircookie
/usr/bin/ld: cannot find -lepoxy
/usr/bin/ld: cannot find -latk-1.0
/usr/bin/ld: cannot find -lgdk_pixbuf-2.0
/usr/bin/ld: cannot find -lselinux
/usr/bin/ld: attempted static link of dynamic object 
`/usr/lib/x86_64-linux-gnu/libz.so'
collect2: error: ld returned 1 exit status
Makefile:204: recipe for target 'qemu-system-lm32' failed
make[1]: *** [qemu-system-lm32] Error 1
Makefile:472: recipe for target 'subdir-lm32-softmmu' failed
make: *** [subdir-lm32-softmmu] Error 2
---
 .travis.yml | 5 +
 1 file changed, 5 insertions(+)

diff --git a/.travis.yml b/.travis.yml
index 08502c0aa2..6962fff826 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -92,6 +92,11 @@ matrix:
 - CONFIG="--disable-user --target-list-exclude=${MAIN_SOFTMMU_TARGETS}"
 
 
+# Test static linking
+- env:
+- CONFIG="--static --target-list=lm32-softmmu"
+
+
 # Just build tools and run minimal unit and softfloat checks
 - env:
 - BASE_CONFIG="--enable-tools"
-- 
2.20.1

Re: [Qemu-devel] [PATCH 1/6] configure: Only generate GLUSTERFS variables if glusterfs is usable

[Niels de Vos]

On Fri, Jun 14, 2019 at 09:24:27AM +0200, Philippe Mathieu-Daudé wrote:
> It is pointless and confusing to have GLUSTERFS variables
> in config-host.mak when glusterfs is not usable.
> 
> Signed-off-by: Philippe Mathieu-Daudé 

Looks good to me, thanks.

Reviewed-by: Niels de Vos 

> ---
>  configure | 36 ++--
>  1 file changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/configure b/configure
> index b091b82cb3..13fd4a1166 100755
> --- a/configure
> +++ b/configure
> @@ -7118,30 +7118,30 @@ if test "$glusterfs" = "yes" ; then
>echo "CONFIG_GLUSTERFS=m" >> $config_host_mak
>echo "GLUSTERFS_CFLAGS=$glusterfs_cflags" >> $config_host_mak
>echo "GLUSTERFS_LIBS=$glusterfs_libs" >> $config_host_mak
> -fi
>  
> -if test "$glusterfs_xlator_opt" = "yes" ; then
> -  echo "CONFIG_GLUSTERFS_XLATOR_OPT=y" >> $config_host_mak
> -fi
> +  if test "$glusterfs_xlator_opt" = "yes" ; then
> +echo "CONFIG_GLUSTERFS_XLATOR_OPT=y" >> $config_host_mak
> +  fi
>  
> -if test "$glusterfs_discard" = "yes" ; then
> -  echo "CONFIG_GLUSTERFS_DISCARD=y" >> $config_host_mak
> -fi
> +  if test "$glusterfs_discard" = "yes" ; then
> +echo "CONFIG_GLUSTERFS_DISCARD=y" >> $config_host_mak
> +  fi
>  
> -if test "$glusterfs_fallocate" = "yes" ; then
> -  echo "CONFIG_GLUSTERFS_FALLOCATE=y" >> $config_host_mak
> -fi
> +  if test "$glusterfs_fallocate" = "yes" ; then
> +echo "CONFIG_GLUSTERFS_FALLOCATE=y" >> $config_host_mak
> +  fi
>  
> -if test "$glusterfs_zerofill" = "yes" ; then
> -  echo "CONFIG_GLUSTERFS_ZEROFILL=y" >> $config_host_mak
> -fi
> +  if test "$glusterfs_zerofill" = "yes" ; then
> +echo "CONFIG_GLUSTERFS_ZEROFILL=y" >> $config_host_mak
> +  fi
>  
> -if test "$glusterfs_ftruncate_has_stat" = "yes" ; then
> -  echo "CONFIG_GLUSTERFS_FTRUNCATE_HAS_STAT=y" >> $config_host_mak
> -fi
> +  if test "$glusterfs_ftruncate_has_stat" = "yes" ; then
> +echo "CONFIG_GLUSTERFS_FTRUNCATE_HAS_STAT=y" >> $config_host_mak
> +  fi
>  
> -if test "$glusterfs_iocb_has_stat" = "yes" ; then
> -  echo "CONFIG_GLUSTERFS_IOCB_HAS_STAT=y" >> $config_host_mak
> +  if test "$glusterfs_iocb_has_stat" = "yes" ; then
> +echo "CONFIG_GLUSTERFS_IOCB_HAS_STAT=y" >> $config_host_mak
> +  fi
>  fi
>  
>  if test "$libssh2" = "yes" ; then
> -- 
> 2.20.1
> 

Re: [Qemu-devel] [RFC] virtio-rng: add a watchdog

[Amit Shah]

On Thu, 2019-06-13 at 10:53 +0200, Laurent Vivier wrote:
> On 12/06/2019 09:03, Amit Shah wrote:
> > On Tue, 2019-06-11 at 19:20 +0200, Laurent Vivier wrote:
> > > The virtio-rng linux driver can be stuck in virtio_read() on a
> > > wait_for_completion_killable() call if the virtio-rng device in
> > > QEMU
> > > doesn't provide data.
> > > 
> > > It's a problem, because virtio_read() is called from
> > > rng_get_data()
> > > with
> > > reading_mutex() held.  The same mutex is taken by
> > > add_early_randomness()
> > > and hwrng_fillfn() and this brings to a hang during the boot
> > > sequence
> > > if
> > > the virtio-rng driver is builtin.
> > > Moreover, another lock is taken (rng_mutex) when the hwrng driver
> > > wants to switch the RNG device or the user tries to unplug the
> > > virtio-rng
> > > PCI card, and this can hang too because the virtio-rng driver is
> > > only
> > > able
> > > to release the card if the virtio-rng device sends back the
> > > virtqueue
> > > element.
> > > 
> > >   # echo -n virtio_rng.1 > /sys/class/misc/hw_random/rng_current
> > >   [  240.165234] INFO: task kworker/u2:1:34 blocked for more than
> > > 120
> > > seconds.
> > >   [  240.165961] "echo 0 >
> > > /proc/sys/kernel/hung_task_timeout_secs"
> > > disables this message.
> > >   [  240.166708] kworker/u2:1D
> > > b86b85a8 034  2 0x
> > >   [  240.166714] Workqueue: kacpi_hotplug acpi_hotplug_work_fn
> > >   [  240.166716]  a0e8f3c0b890 0046
> > > a0e8f3c0
> > > a0e8f3c0bfd8
> > >   [  240.166717]  a0e8f3c0bfd8 a0e8f3c0bfd8
> > > a0e8f3c0
> > > b86b85a0
> > >   [  240.166719]  b86b85a4 a0e8f3c0
> > > 
> > > b86b85a8
> > >   [  240.166720] Call Trace:
> > >   [  240.166725]  []
> > > schedule_preempt_disabled+0x29/0x70
> > >   [  240.166727]  []
> > > __mutex_lock_slowpath+0xc7/0x1d0
> > >   [  240.166728]  [] mutex_lock+0x1f/0x2f
> > >   [  240.166730]  [] hwrng_register+0x32/0x1d0
> > >   [  240.166733]  [] virtrng_scan+0x19/0x30
> > > [virtio_rng]
> > >   [  240.166744]  []
> > > virtio_dev_probe+0x1eb/0x290
> > > [virtio]
> > >   [  240.166746]  []
> > > driver_probe_device+0x145/0x3c0
> > >   ...
> > > 
> > > In some case, the QEMU RNG backend is not able to provide data,
> > > and
> > > the virtio-rng device is not aware of that:
> > > - with rng-random using /dev/random and no entropy is available,
> > > - with rng-egd started with a socket in "server,nowait" mode and
> > >   no daemon connected,
> > > - with rng-egd and an egd daemon that is not providing enough
> > > data,
> > > - ...
> > > 
> > > To release the locks regularly, this patch adds a watchdog in
> > > QEMU
> > > virtio-rng device that sends back to the guest the virtqueue
> > > buffer
> > > with a 0 byte payload. This case is expected and correctly
> > > managed by
> > > the hwrng core.
> > 
> > I'm wondering if it makes more sense to rework the way the kernel
> > driver requests for seeding entropy during probe.
> 
> The kernel side was my first angle of attack.
> I tried first to not block in add_early_randomness():
> 
>   "hwrng: core - don't block in add_early_randomness()"
>   https://patchwork.kernel.org/patch/10877571/
> 
> But I agree with the maintainer, the problem must be fixed at virtio-
> rng 
> level.

Yea.

How much of this is due to 'rng-egd was not set up properly; the
backend did not appear in time' -- ie a setup problem; vs a problem we
can expect to recur?

The current patch affects *all* requests from the guest.  The problem
being seen though is just during the driver probe.  Is the current
patch doing much more than is required?  Say the device runs out of
randomness to provide after setup - the guest's read calls will just
block and remain killable.  It's the probe currently that is not
killable.

Other options to explore are:

1. Ensure setup is ready (ie the device's backend is set up before
plugging in the device), so that the device is ready to serve requests
as soon as it's plugged in

2. Make the add_early_randomness optional for virtio-rng.  (This is a
big hammer, and working around bad setup problems.)




> 
> > The virtio_read call is killable, so it can take signals when
> > initiated
> > by userspace.  For the initial probe, specifying a timeout /
> > watchdog
> > in the driver is better.
> 
> Yes, I think also it's better, I tried to do something like that:
> 
> --- a/drivers/char/hw_random/virtio-rng.c
> +++ b/drivers/char/hw_random/virtio-rng.c
> @@ -77,10 +77,7 @@ static int virtio_read(struct hwrng *rng, void
> *buf, size_t size, bool wait)
> register_buffer(vi, buf, size);
> }
>  
> -   if (!wait)
> -   return 0;
> -
> -   ret = wait_for_completion_killable(&vi->have_data);
> +   ret = wait_for_completion_timeout(&vi->have_data, wait ?
> MAX_SCHEDULE_TIMEOUT : HZ);
> if (ret < 0)
> return ret;
> 
> But I have a pr

Re: [Qemu-devel] [PATCH] MAINTAINERS: Change maintership of Xen code under hw/9pfs

[Greg Kurz]

On Wed, 5 Jun 2019 10:12:05 +
Paul Durrant  wrote:

> > -Original Message-
> > From: Greg Kurz [mailto:gr...@kaod.org]
> > Sent: 05 June 2019 11:11
> > To: Anthony Perard 
> > Cc: qemu-devel@nongnu.org; Stefano Stabellini ; 
> > Paul Durrant
> > 
> > Subject: Re: [PATCH] MAINTAINERS: Change maintership of Xen code under 
> > hw/9pfs
> > 
> > On Wed, 29 May 2019 13:59:26 +0100
> > Anthony PERARD  wrote:
> >   
> > > On Wed, May 29, 2019 at 12:24:44PM +0200, Greg Kurz wrote:  
> > > > Xen folks are the actual maintainers for this.
> > > >
> > > > Signed-off-by: Greg Kurz 
> > > > ---
> > > >  MAINTAINERS |3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > index 1f5f8b7a2c37..d00380641796 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -411,7 +411,7 @@ M: Paul Durrant 
> > > >  L: xen-de...@lists.xenproject.org
> > > >  S: Supported
> > > >  F: */xen*
> > > > -F: hw/9pfs/xen-9p-backend.c
> > > > +F: hw/9pfs/xen-9p*
> > > >  F: hw/char/xen_console.c
> > > >  F: hw/display/xenfb.c
> > > >  F: hw/net/xen_nic.c
> > > > @@ -1505,6 +1505,7 @@ virtio-9p
> > > >  M: Greg Kurz 
> > > >  S: Supported
> > > >  F: hw/9pfs/
> > > > +X: hw/9pfs/xen-9p*
> > > >  F: fsdev/
> > > >  F: tests/virtio-9p-test.c
> > > >  T: git https://github.com/gkurz/qemu.git 9p-next
> > > >  
> > >
> > > Acked-by: Anthony PERARD 
> > >
> > > Thanks,
> > >  
> > 
> > Ping ?
> > 
> > I'd rather also get an ack from Stefano and Paul before merging this.
> >   
> 
> Fine by me...
> 
> Acked-by: Paul Durrant 
> 

Hi,

I'd prefer to also get an ack from Stefano but it seems that he hasn't posted
anything to qemu-devel@ since last January... Any suggestion how I should go
forward with this ?

Cheers,

--
Greg

> > Cheers,
> > 
> > --
> > Greg  

Re: [Qemu-devel] [PATCH] MAINTAINERS: Change maintership of Xen code under hw/9pfs

[Paul Durrant]

> -Original Message-
> From: Greg Kurz [mailto:gr...@kaod.org]
> Sent: 14 June 2019 09:16
> To: Paul Durrant 
> Cc: Anthony Perard ; qemu-devel@nongnu.org; 
> Stefano Stabellini
> 
> Subject: Re: [PATCH] MAINTAINERS: Change maintership of Xen code under hw/9pfs
> 
> On Wed, 5 Jun 2019 10:12:05 +
> Paul Durrant  wrote:
> 
> > > -Original Message-
> > > From: Greg Kurz [mailto:gr...@kaod.org]
> > > Sent: 05 June 2019 11:11
> > > To: Anthony Perard 
> > > Cc: qemu-devel@nongnu.org; Stefano Stabellini ; 
> > > Paul Durrant
> > > 
> > > Subject: Re: [PATCH] MAINTAINERS: Change maintership of Xen code under 
> > > hw/9pfs
> > >
> > > On Wed, 29 May 2019 13:59:26 +0100
> > > Anthony PERARD  wrote:
> > >
> > > > On Wed, May 29, 2019 at 12:24:44PM +0200, Greg Kurz wrote:
> > > > > Xen folks are the actual maintainers for this.
> > > > >
> > > > > Signed-off-by: Greg Kurz 
> > > > > ---
> > > > >  MAINTAINERS |3 ++-
> > > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > > index 1f5f8b7a2c37..d00380641796 100644
> > > > > --- a/MAINTAINERS
> > > > > +++ b/MAINTAINERS
> > > > > @@ -411,7 +411,7 @@ M: Paul Durrant 
> > > > >  L: xen-de...@lists.xenproject.org
> > > > >  S: Supported
> > > > >  F: */xen*
> > > > > -F: hw/9pfs/xen-9p-backend.c
> > > > > +F: hw/9pfs/xen-9p*
> > > > >  F: hw/char/xen_console.c
> > > > >  F: hw/display/xenfb.c
> > > > >  F: hw/net/xen_nic.c
> > > > > @@ -1505,6 +1505,7 @@ virtio-9p
> > > > >  M: Greg Kurz 
> > > > >  S: Supported
> > > > >  F: hw/9pfs/
> > > > > +X: hw/9pfs/xen-9p*
> > > > >  F: fsdev/
> > > > >  F: tests/virtio-9p-test.c
> > > > >  T: git https://github.com/gkurz/qemu.git 9p-next
> > > > >
> > > >
> > > > Acked-by: Anthony PERARD 
> > > >
> > > > Thanks,
> > > >
> > >
> > > Ping ?
> > >
> > > I'd rather also get an ack from Stefano and Paul before merging this.
> > >
> >
> > Fine by me...
> >
> > Acked-by: Paul Durrant 
> >
> 
> Hi,
> 
> I'd prefer to also get an ack from Stefano but it seems that he hasn't posted
> anything to qemu-devel@ since last January... Any suggestion how I should go
> forward with this ?
> 

Yes, I think it would be prudent to get an ack from Stefano. I'll ping him on 
IRC and see if I can get a response.

  Cheers,

Paul

> Cheers,
> 
> --
> Greg
> 
> > > Cheers,
> > >
> > > --
> > > Greg

Re: [Qemu-devel] [PATCH v3 11/15] monitor: Split out monitor/hmp.c

[Markus Armbruster]

Kevin Wolf  writes:

> Move HMP infrastructure from monitor/misc.c to monitor/hmp.c. This is
> code that can be shared for all targets, so compile it only once.
>
> The amount of function and particularly extern variables in
> monitor_int.h is probably a bit larger than it needs to be, but this way
> no non-trivial code modifications are needed. The interfaces between HMP
> and the monitor core can be cleaned up later.
>
> Signed-off-by: Kevin Wolf 
> ---
[...]
> diff --git a/monitor/hmp.c b/monitor/hmp.c
> new file mode 100644
> index 00..3621b195ed
> --- /dev/null
> +++ b/monitor/hmp.c
> @@ -0,0 +1,1415 @@
[...]
> +static int64_t expr_unary(Monitor *mon)
> +{
> +int64_t n;
> +char *p;
> +int ret;
> +
> +switch (*pch) {
> +case '+':
> +next();
> +n = expr_unary(mon);
> +break;
> +case '-':
> +next();
> +n = -expr_unary(mon);
> +break;
> +case '~':
> +next();
> +n = ~expr_unary(mon);
> +break;
> +case '(':
> +next();
> +n = expr_sum(mon);
> +if (*pch != ')') {
> +expr_error(mon, "')' expected");
> +}
> +next();
> +break;
> +case '\'':
> +pch++;
> +if (*pch == '\0') {
> +expr_error(mon, "character constant expected");
> +}
> +n = *pch;
> +pch++;
> +if (*pch != '\'') {
> +expr_error(mon, "missing terminating \' character");
> +}
> +next();
> +break;
> +case '$':
> +{
> +char buf[128], *q;
> +int64_t reg = 0;
> +
> +pch++;
> +q = buf;
> +while ((*pch >= 'a' && *pch <= 'z') ||
> +   (*pch >= 'A' && *pch <= 'Z') ||
> +   (*pch >= '0' && *pch <= '9') ||
> +   *pch == '_' || *pch == '.') {
> +if ((q - buf) < sizeof(buf) - 1) {
> +*q++ = *pch;
> +}
> +pch++;
> +}
> +while (qemu_isspace(*pch)) {
> +pch++;
> +}
> +*q = 0;
> +ret = get_monitor_def(®, buf);
> +if (ret < 0) {
> +expr_error(mon, "unknown register");
> +}
> +n = reg;
> +}
> +break;
> +case '\0':
> +expr_error(mon, "unexpected end of expression");
> +n = 0;
> +break;
> +default:
> +errno = 0;
> +n = strtoull(pch, &p, 0);

checkpatch.pl gripes:

ERROR: consider using qemu_strtoull in preference to strtoull


Let's add a TODO comment.  

> +if (errno == ERANGE) {
> +expr_error(mon, "number too large");
> +}
> +if (pch == p) {
> +expr_error(mon, "invalid char '%c' in expression", *p);
> +}
> +pch = p;
> +while (qemu_isspace(*pch)) {
> +pch++;
> +}
> +break;
> +}
> +return n;
> +}
[...]
> +static void monitor_find_completion(void *opaque,
> +const char *cmdline)
> +{
> +MonitorHMP *mon = opaque;
> +char *args[MAX_ARGS];
> +int nb_args, len;
> +
> +/* 1. parse the cmdline */
> +if (parse_cmdline(cmdline, &nb_args, args) < 0) {
> +return;
> +}
> +
> +/* if the line ends with a space, it means we want to complete the
> + * next arg */

checkpatch.pl again:

WARNING: Block comments use a leading /* on a separate line
WARNING: Block comments use a trailing */ on a separate line

Can touch up in my tree.

> +len = strlen(cmdline);
> +if (len > 0 && qemu_isspace(cmdline[len - 1])) {
> +if (nb_args >= MAX_ARGS) {
> +goto cleanup;
> +}
> +args[nb_args++] = g_strdup("");
> +}
> +
> +/* 2. auto complete according to args */
> +monitor_find_completion_by_table(mon, hmp_cmds, args, nb_args);
> +
> +cleanup:
> +free_cmdline_args(args, nb_args);
> +}
[...]
> diff --git a/monitor/misc.c b/monitor/misc.c
> index 368b8297d4..c8289959c0 100644
> --- a/monitor/misc.c
> +++ b/monitor/misc.c
[...]
> @@ -612,245 +580,27 @@ out:
>  return output;
>  }
>  
> -static int compare_cmd(const char *name, const char *list)
> +/**
> + * Is @name in the '|' separated list of names @list?
> + */
> +int hmp_compare_cmd(const char *name, const char *list)
>  {
>  const char *p, *pstart;
>  int len;
>  len = strlen(name);
>  p = list;
> -for(;;) {
> +for (;;) {
>  pstart = p;
>  p = qemu_strchrnul(p, '|');
> -if ((p - pstart) == len && !memcmp(pstart, name, len))
> +if ((p - pstart) == len && !memcmp(pstart, name, len)) {
>  return 1;

The diff gets confusing here.  The function remains unchanged.  Good.

> -if (*p == '\0')
> -break;
> -p++;
> -}
> -return 0;
> -}
> -
> -static int get_str(char *buf

Re: [Qemu-devel] [PATCH v3 12/15] monitor: Split out monitor/monitor.c

[Markus Armbruster]

Kevin Wolf  writes:

> Move the monitor core infrastructure from monitor/misc.c to
> monitor/monitor.c. This is code that can be shared for all targets, so
> compile it only once.
>
> What remains in monitor/misc.c after this patch is mostly monitor
> command implementations (which could move to hmp-cmds.c or qmp-cmds.c
> later) and code that requires a system emulator or is even
> target-dependent (including HMP command completion code).
>
> The amount of function and particularly extern variables in
> monitor_int.h is probably a bit larger than it needs to be, but this way
> no non-trivial code modifications are needed. The interfaces between all
> monitor parts can be cleaned up later.
>
> Signed-off-by: Kevin Wolf 
[...]
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> new file mode 100644
> index 00..6802b8e491
> --- /dev/null
> +++ b/monitor/monitor.c
> @@ -0,0 +1,637 @@
[...]
> +QemuOptsList qemu_mon_opts = {
> +.name = "mon",
> +.implied_opt_name = "chardev",
> +.head = QTAILQ_HEAD_INITIALIZER(qemu_mon_opts.head),
> +.desc = {
> +{
> +.name = "mode",
> +.type = QEMU_OPT_STRING,
> +},{
> +.name = "chardev",
> +.type = QEMU_OPT_STRING,
> +},{
> +.name = "pretty",
> +.type = QEMU_OPT_BOOL,
> +},
> +{ /* end of list */ }

checkpatch.pl gripes:

WARNING: Block comments use a leading /* on a separate line

I agree with ignoring this one, because it's the way it looks
everywhere.

> +},
> +};
[...]

Reviewed-by: Markus Armbruster 

Re: [Qemu-devel] [PATCH] [RFC] Add a eBPF-capable PCIe device

[Stefan Hajnoczi]

On Mon, Jun 03, 2019 at 02:58:26PM -0600, Martin Ichilevici de Oliveira wrote:

Thanks for sharing!

The bpf_ram accesses are unsafe.  The guest can modify bpf_ram while the
device is accessing it.  This is likely to cause security problems.

I think a model is required where the device copies in the program and
additional data before processing.  This way the guest cannot modify it
while the device is executing the program.

Also, please validate inputs.  The guest is untrusted.  Offsets, sizes,
etc cannot be trusted and must be bounds-checked.


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 1/1] tcg: Fix typos in helper_gvec_sar{8, 32, 64}v

[Peter Maydell]

On Fri, 14 Jun 2019 at 07:09, Aleksandar Markovic
 wrote:
> Peter, would you please explain to me what was going on with this patch?

The original patch was posted to the list on the 7th June; it got
reviewed and tested by Laurent. This email accidentally has "PATCH"
in the subject line instead of "PULL" but it's just part of the pull
request that got it into master -- that's just an accidental
error by Richard when he put the pull request together.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] docs/vhost-user.json: some firmware.json copy leftovers

[Stefan Hajnoczi]

On Wed, Jun 05, 2019 at 03:12:21PM +0200, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau 
> ---
>  docs/interop/vhost-user.json | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 2/6] configure: Link test before auto-enabling glusterfs libraries

[Philippe Mathieu-Daudé]

On 6/14/19 9:24 AM, Philippe Mathieu-Daudé wrote:
> Similarly to commit a73e82ef912, test the libraries link correctly
> before considering them as usable.
> 
> This fixes using ./configure --static on Ubuntu 18.04:
> 
>   $ make subdir-aarch64-softmmu
>   [...]
> LINKaarch64-softmmu/qemu-system-aarch64
>   /usr/bin/ld: cannot find -lgfapi
>   /usr/bin/ld: cannot find -lglusterfs
>   /usr/bin/ld: cannot find -lgfrpc
>   /usr/bin/ld: cannot find -lgfxdr
>   collect2: error: ld returned 1 exit status
>   Makefile:204: recipe for target 'qemu-system-aarch64' failed
>   make[1]: *** [qemu-system-aarch64] Error 1
> 
>   $ fgrep gf config-host.mak
>   GLUSTERFS_LIBS=-lacl -lgfapi -lglusterfs -lgfrpc -lgfxdr -luuid
> 
>   $ lsb_release -cri
>   Distributor ID: Ubuntu
>   Release:18.04
>   Codename:   bionic
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  configure | 18 --
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/configure b/configure
> index 13fd4a1166..3428adb75b 100755
> --- a/configure
> +++ b/configure
> @@ -4179,9 +4179,23 @@ fi
>  # glusterfs probe
>  if test "$glusterfs" != "no" ; then
>if $pkg_config --atleast-version=3 glusterfs-api; then
> -glusterfs="yes"
>  glusterfs_cflags=$($pkg_config --cflags glusterfs-api)
> -glusterfs_libs=$($pkg_config --libs glusterfs-api)
> +if test "$static" = "yes"; then
> +glusterfs_libs=$($pkg_config --libs --static glusterfs-api)
> +else
> +glusterfs_libs=$($pkg_config --libs glusterfs-api)
> +fi

I just noticed in ./configure:

  case "$opt" in
  --static)
static="yes"
LDFLAGS="-static $LDFLAGS"
QEMU_PKG_CONFIG_FLAGS="--static $QEMU_PKG_CONFIG_FLAGS"
  ;;

And

pkg_config_exe="${PKG_CONFIG-${cross_prefix}pkg-config}"
query_pkg_config() {
"${pkg_config_exe}" ${QEMU_PKG_CONFIG_FLAGS} "$@"
}
pkg_config=query_pkg_config

So I shouldn't need to test "$static" = "yes" and manually add --static.
(same apply to other patches in this series).

I'll see what's wrong...

> +# Packaging for the static libraries is not always correct.
> +# At least ubuntu 18.04 ships only shared libraries.
> +write_c_skeleton
> +if ! compile_prog "$glusterfs_cflags" "$glusterfs_libs" ; then
> +if test "$glusterfs" = "yes" ; then
> +  error_exit "glusterfs check failed."
> +fi
> +glusterfs="no"
> +else
> +glusterfs="yes"
> +fi
>  if $pkg_config --atleast-version=4 glusterfs-api; then
>glusterfs_xlator_opt="yes"
>  fi
> 

Re: [Qemu-devel] [PATCH v3 09/15] monitor: Create monitor-internal.h with common definitions

[Kevin Wolf]

Am 14.06.2019 um 08:37 hat Markus Armbruster geschrieben:
> Kevin Wolf  writes:
> 
> > Before we can split monitor/misc.c, we need to create a header file that
> > contains the common definitions that will be used by multiple source
> > files.
> >
> > For a start, add the type definitions for Monitor, MonitorHMP and
> > MonitorQMP and their dependencies. We'll add functions as needed when
> > splitting monitor/misc.c.
> >
> > Signed-off-by: Kevin Wolf 
> > Reviewed-by: Dr. David Alan Gilbert 
> > ---
> >  monitor/monitor-internal.h | 148 +
> >  monitor/misc.c | 110 +--
> >  MAINTAINERS|   2 +
> >  3 files changed, 151 insertions(+), 109 deletions(-)
> >  create mode 100644 monitor/monitor-internal.h
> >
> > diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
> > new file mode 100644
> > index 00..17a632b0ad
> > --- /dev/null
> > +++ b/monitor/monitor-internal.h
> > @@ -0,0 +1,148 @@
> > +/*
> > + * QEMU monitor
> > + *
> > + * Copyright (c) 2003-2004 Fabrice Bellard
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a 
> > copy
> > + * of this software and associated documentation files (the "Software"), 
> > to deal
> > + * in the Software without restriction, including without limitation the 
> > rights
> > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or 
> > sell
> > + * copies of the Software, and to permit persons to whom the Software is
> > + * furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice shall be included 
> > in
> > + * all copies or substantial portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS 
> > OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR 
> > OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> > FROM,
> > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS 
> > IN
> > + * THE SOFTWARE.
> > + */
> > +
> > +#ifndef MONITOR_INT_H
> > +#define MONITOR_INT_H
> 
> Rename to MONITOR_INTERNAL_H, so it again matches the file name.  Can
> touch up in my tree.

Oops, yes, please do.

> > +
> > +#include "monitor/monitor.h"
> > +#include "qapi/qmp/qdict.h"
> 
> These too are superfluous.  I'm willing to tolerate monitor.h anyway,
> since anything including monitor-internal.h is almost certainly going to
> need monitor.h, too.

I tried to drop them because you suggested so, but it results in compile
errors. On closer look, I think qdict.h can go because the typedef will
be present through qemu/osdep.h, which must be included before this one,
but MonitorHMP is only defined by monitor/monitor.h.

> > +#include "qapi/qmp/json-parser.h"
> > +#include "qapi/qmp/dispatch.h"
> > +#include "qapi/qapi-types-misc.h"
> > +
> > +#include "qemu/readline.h"
> > +#include "chardev/char-fe.h"
> > +#include "sysemu/iothread.h"
> 
> Another superfluous one.

IOThread is only defined by system/iothread.h, so I don't think you can
remove this one either.

> Happy to drop these two #include in my tree.

As long as the result still builds, feel free to drop includes from the
header (and probably add them to source files instead where they will be
missing then).

Kevin

Re: [Qemu-devel] [PATCH 0/6] configure: Try to fix --static linking

[Philippe Mathieu-Daudé]

On 6/14/19 9:24 AM, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> Apparently QEMU static linking is slowly bitroting. Obviously it
> depends the libraries an user has installed, anyway it seems there
> are not much testing done.
> 
> This series fixes few issues, enough to build QEMU on a Ubuntu
> aarch64 host, but not yet on a x86_64 host:
> 
> LINKx86_64-softmmu/qemu-system-x86_64
>   /usr/bin/ld: cannot find -lgtk-3
>   /usr/bin/ld: cannot find -latk-bridge-2.0
>   /usr/bin/ld: cannot find -latspi
>   /usr/bin/ld: cannot find -lsystemd
>   /usr/bin/ld: cannot find -lgdk-3
>   /usr/bin/ld: cannot find -lwayland-egl
>   /usr/bin/ld: cannot find -lmirclient
>   /usr/bin/ld: cannot find -lmircore
>   /usr/bin/ld: cannot find -lmircookie
>   /usr/bin/ld: cannot find -lepoxy
>   /usr/bin/ld: cannot find -latk-1.0
>   /usr/bin/ld: cannot find -lgdk_pixbuf-2.0
>   /usr/bin/ld: cannot find -lselinux
>   /usr/bin/ld: cannot find -lgtk-3
>   /usr/bin/ld: cannot find -latk-bridge-2.0
>   /usr/bin/ld: cannot find -latspi
>   /usr/bin/ld: cannot find -lsystemd
>   /usr/bin/ld: cannot find -lgdk-3
>   /usr/bin/ld: cannot find -lwayland-egl
>   /usr/bin/ld: cannot find -lmirclient
>   /usr/bin/ld: cannot find -lmircore
>   /usr/bin/ld: cannot find -lmircookie
>   /usr/bin/ld: cannot find -lepoxy
>   /usr/bin/ld: cannot find -latk-1.0
>   /usr/bin/ld: cannot find -lgdk_pixbuf-2.0
>   /usr/bin/ld: cannot find -lselinux
>   /usr/bin/ld: attempted static link of dynamic object 
> `/usr/lib/x86_64-linux-gnu/libz.so'
>   collect2: error: ld returned 1 exit status

This one is funny, when installing libvte on Ubuntu 18.04:

LINKx86_64-softmmu/qemu-system-x86_64
  c++: error: /usr/lib/x86_64-linux-gnu/libunistring.so: No such file or
directory
  c++: error: /usr/lib/x86_64-linux-gnu/libunistring.so: No such file or
directory
  c++: error: /usr/lib/x86_64-linux-gnu/libunistring.so: No such file or
directory
  c++: error: /usr/lib/x86_64-linux-gnu/libunistring.so: No such file or
directory

$ pkg-config --libs --static vte-2.91
-lvte-2.91 -lgtk-3 -latk-bridge-2.0 -latspi -ldbus-1 -lpthread -lsystemd
-lgdk-3 -lXinerama -lXi -lXrandr -lXcursor -lXcomposite -lXdamage
-lXfixes -lxkbcommon -lwayland-cursor -lwayland-egl -lwayland-client
-lepoxy -ldl -lpangocairo-1.0 -lpangoft2-1.0 -lharfbuzz -lm -lgraphite2
-lpango-1.0 -lm -latk-1.0 -lcairo-gobject -lcairo -lz -lpixman-1
-lfontconfig -lexpat -lfreetype -lexpat -lfreetype -lpng16 -lm -lz -lm
-lxcb-shm -lxcb-render -lXrender -lXext -lX11 -lpthread -lxcb -lXau
-lXdmcp -lgdk_pixbuf-2.0 -lm -lpng16 -lm -lz -lm -lgio-2.0 -lz -lresolv
-lselinux -lmount -lgmodule-2.0 -pthread -ldl -lgobject-2.0 -lffi
-lglib-2.0 -pthread -lpcre -pthread -lgnutls -lgmp
/usr/lib/x86_64-linux-gnu/libunistring.so -lidn2 -lhogweed -lgmp
-lnettle -ltasn1 -lp11-kit -lz

$ ls -ld /usr/lib/x86_64-linux-gnu/libunistring.so
ls: cannot access '/usr/lib/x86_64-linux-gnu/libunistring.so': No such
file or directory

$ ls -ld /usr/lib/x86_64-linux-gnu/libunistring.so*
lrwxrwxrwx. 1 root root  21 Mar  3  2018
/usr/lib/x86_64-linux-gnu/libunistring.so.2 -> libunistring.so.2.1.0
-rw-r--r--. 1 root root 1562664 Mar  3  2018
/usr/lib/x86_64-linux-gnu/libunistring.so.2.1.0

The fix is probably "sudo ln -s libunistring.so.2
/usr/lib/x86_64-linux-gnu/libunistring.so".

Re: [Qemu-devel] [PATCH v3 13/15] monitor: Split Monitor.flags into separate bools

[Markus Armbruster]

Kevin Wolf  writes:

> Monitor.flags contains three different flags: One to distinguish HMP
> from QMP; one specific to HMP (MONITOR_USE_READLINE) that is ignored
> with QMP; and another one specific to QMP (MONITOR_USE_PRETTY) that is
> ignored with HMP.
>
> Split the flags field into three bools and move them to the right
> subclass. Flags are still in use for the monitor_init() interface.
>
> Signed-off-by: Kevin Wolf 
> ---
>  monitor/monitor-internal.h |  8 +---
>  monitor/hmp.c  |  6 +++---
>  monitor/misc.c |  2 +-
>  monitor/monitor.c  | 14 +-
>  monitor/qmp.c  |  7 ---
>  5 files changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
> index 30679d522e..260725e51b 100644
> --- a/monitor/monitor-internal.h
> +++ b/monitor/monitor-internal.h
> @@ -90,8 +90,8 @@ typedef struct HMPCommand {
>  struct Monitor {
>  CharBackend chr;
>  int reset_seen;
> -int flags;
>  int suspend_cnt;/* Needs to be accessed atomically */
> +bool is_qmp;
>  bool skip_flush;
>  bool use_io_thread;
>  
> @@ -116,6 +116,7 @@ struct Monitor {
>  
>  struct MonitorHMP {
>  Monitor common;
> +bool use_readline;
>  /*
>   * State used only in the thread "owning" the monitor.
>   * If @use_io_thread, this is @mon_iothread. (This does not actually 
> happen
> @@ -129,6 +130,7 @@ struct MonitorHMP {
>  typedef struct {
>  Monitor common;
>  JSONMessageParser parser;
> +bool pretty;
>  /*
>   * When a client connects, we're in capabilities negotiation mode.
>   * @commands is &qmp_cap_negotiation_commands then.  When command
> @@ -152,7 +154,7 @@ typedef struct {
>   */
>  static inline bool monitor_is_qmp(const Monitor *mon)
>  {
> -return (mon->flags & MONITOR_USE_CONTROL);
> +return mon->is_qmp;
>  }

The function was marginal before, and it's pointless now.  Let's kill it
in a follow-up patch.  No need to do it in this series.

>  
>  typedef QTAILQ_HEAD(MonitorList, Monitor) MonitorList;
> @@ -169,7 +171,7 @@ void monitor_init_qmp(Chardev *chr, int flags);
>  void monitor_init_hmp(Chardev *chr, int flags);
>  
>  int monitor_puts(Monitor *mon, const char *str);
> -void monitor_data_init(Monitor *mon, int flags, bool skip_flush,
> +void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
> bool use_io_thread);
>  void monitor_data_destroy(Monitor *mon);
>  int monitor_can_read(void *opaque);
> diff --git a/monitor/hmp.c b/monitor/hmp.c
> index 3621b195ed..5ba8b6e8d5 100644
> --- a/monitor/hmp.c
> +++ b/monitor/hmp.c
> @@ -1396,12 +1396,12 @@ static void monitor_readline_flush(void *opaque)
>  void monitor_init_hmp(Chardev *chr, int flags)
>  {
>  MonitorHMP *mon = g_malloc0(sizeof(*mon));
> -bool use_readline = flags & MONITOR_USE_READLINE;
>  
> -monitor_data_init(&mon->common, flags, false, false);
> +monitor_data_init(&mon->common, false, false, false);
>  qemu_chr_fe_init(&mon->common.chr, chr, &error_abort);
>  
> -if (use_readline) {
> +mon->use_readline = flags & MONITOR_USE_READLINE;
> +if (mon->use_readline) {
>  mon->rs = readline_init(monitor_readline_printf,
>  monitor_readline_flush,
>  mon,
> diff --git a/monitor/misc.c b/monitor/misc.c
> index dddbddb21f..10c056394e 100644
> --- a/monitor/misc.c
> +++ b/monitor/misc.c
> @@ -134,7 +134,7 @@ char *qmp_human_monitor_command(const char *command_line, 
> bool has_cpu_index,
>  Monitor *old_mon;
>  MonitorHMP hmp = {};
>  
> -monitor_data_init(&hmp.common, 0, true, false);
> +monitor_data_init(&hmp.common, false, true, false);
>  
>  old_mon = cur_mon;
>  cur_mon = &hmp.common;
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index 6802b8e491..7325e4362b 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -80,14 +80,18 @@ bool monitor_cur_is_qmp(void)
>   * Note: not all HMP monitors use readline, e.g., gdbserver has a
>   * non-interactive HMP monitor, so readline is not used there.
>   */
> -static inline bool monitor_uses_readline(const Monitor *mon)
> +static inline bool monitor_uses_readline(const MonitorHMP *mon)
>  {
> -return mon->flags & MONITOR_USE_READLINE;
> +return mon->use_readline;
>  }

Another one to kill.

>  
>  static inline bool monitor_is_hmp_non_interactive(const Monitor *mon)
>  {
> -return !monitor_is_qmp(mon) && !monitor_uses_readline(mon);
> +if (monitor_is_qmp(mon)) {
> +return false;
> +}
> +
> +return !monitor_uses_readline(container_of(mon, MonitorHMP, common));
>  }
>  

Not sure about this one.  We'll see.

>  static void monitor_flush_locked(Monitor *mon);
> @@ -523,17 +527,17 @@ static void monitor_iothread_init(void)
>  mon_iothread = iothread_create("mon_iothread", &error_abort);
>  }
>  
> -void monitor_data

Re: [Qemu-devel] [PATCH v3 14/15] monitor: Replace monitor_init() with monitor_init_{hmp, qmp}()

[Markus Armbruster]

Kevin Wolf  writes:

> Most callers know which monitor type they want to have. Instead of
> calling monitor_init() with flags that can describe both types of
> monitors, make monitor_init_{hmp,qmp}() public interfaces that take
> specific bools instead of flags and call these functions directly.
>
> Signed-off-by: Kevin Wolf 

Neater now.  Thanks!

Reviewed-by: Markus Armbruster 

Re: [Qemu-devel] [PATCH v3 02/15] monitor: Split monitor_init in HMP and QMP function

[Markus Armbruster]

Kevin Wolf  writes:

> Instead of mixing HMP and QMP monitors in the same function, separate
> the monitor creation function for both.
>
> While in theory, one could pass both MONITOR_USE_CONTROL and
> MONITOR_USE_READLINE before this patch and both flags would do
> something, readline support is tightly coupled with HMP: QMP never feeds
> its input to readline, and the tab completion function treats the input
> as an HMP command. Therefore, this configuration is useless.
>
> After this patch, the QMP path asserts that MONITOR_USE_READLINE is not
> set. The HMP path can be used with or without MONITOR_USE_READLINE, like
> before.
>
> Signed-off-by: Kevin Wolf 
> Reviewed-by: Dr. David Alan Gilbert 
> Reviewed-by: Markus Armbruster 
> ---
>  monitor.c | 89 ---
>  1 file changed, 52 insertions(+), 37 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 9846a5623b..a70c1283b1 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -704,7 +704,7 @@ static void handle_hmp_command(Monitor *mon, const char 
> *cmdline);
>  
>  static void monitor_iothread_init(void);
>  
> -static void monitor_data_init(Monitor *mon, bool skip_flush,
> +static void monitor_data_init(Monitor *mon, int flags, bool skip_flush,
>bool use_io_thread)
>  {
>  if (use_io_thread && !mon_iothread) {
   monitor_iothread_init();
   }
   memset(mon, 0, sizeof(Monitor));

I'd like to replace this memset() by ...

   qemu_mutex_init(&mon->mon_lock);
   qemu_mutex_init(&mon->qmp.qmp_queue_lock);
   mon->outbuf = qstring_new();
   /* Use *mon_cmds by default. */
   mon->cmd_table = mon_cmds;
> @@ -719,6 +719,7 @@ static void monitor_data_init(Monitor *mon, bool 
> skip_flush,
>  mon->skip_flush = skip_flush;
>  mon->use_io_thread = use_io_thread;
>  mon->qmp.qmp_requests = g_queue_new();
> +mon->flags = flags;
>  }
>  
>  static void monitor_data_destroy(Monitor *mon)
> @@ -742,7 +743,7 @@ char *qmp_human_monitor_command(const char *command_line, 
> bool has_cpu_index,
>  char *output = NULL;
>  Monitor *old_mon, hmp;

... hmp = {} here (moved from PATCH 04), and ...

>  
> -monitor_data_init(&hmp, true, false);
> +monitor_data_init(&hmp, 0, true, false);
>  
>  old_mon = cur_mon;
>  cur_mon = &hmp;
> @@ -4605,19 +4606,51 @@ static void monitor_qmp_setup_handlers_bh(void 
> *opaque)
>  monitor_list_append(mon);
>  }
>  
> -void monitor_init(Chardev *chr, int flags)
> +static void monitor_init_qmp(Chardev *chr, int flags)
>  {
>  Monitor *mon = g_malloc(sizeof(*mon));

... g_malloc0() here (moved from PATCH 03), and ...

> -bool use_readline = flags & MONITOR_USE_READLINE;
> +
> +/* Only HMP supports readline */
> +assert(!(flags & MONITOR_USE_READLINE));
>  
>  /* Note: we run QMP monitor in I/O thread when @chr supports that */
> -monitor_data_init(mon, false,
> -  (flags & MONITOR_USE_CONTROL)
> -  && qemu_chr_has_feature(chr,
> -  QEMU_CHAR_FEATURE_GCONTEXT));
> +monitor_data_init(mon, flags, false,
> +  qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT));
>  
>  qemu_chr_fe_init(&mon->chr, chr, &error_abort);
> -mon->flags = flags;
> +qemu_chr_fe_set_echo(&mon->chr, true);
> +
> +json_message_parser_init(&mon->qmp.parser, handle_qmp_command, mon, 
> NULL);
> +if (mon->use_io_thread) {
> +/*
> + * Make sure the old iowatch is gone.  It's possible when
> + * e.g. the chardev is in client mode, with wait=on.
> + */
> +remove_fd_in_watch(chr);
> +/*
> + * We can't call qemu_chr_fe_set_handlers() directly here
> + * since chardev might be running in the monitor I/O
> + * thread.  Schedule a bottom half.
> + */
> +aio_bh_schedule_oneshot(iothread_get_aio_context(mon_iothread),
> +monitor_qmp_setup_handlers_bh, mon);
> +/* The bottom half will add @mon to @mon_list */
> +} else {
> +qemu_chr_fe_set_handlers(&mon->chr, monitor_can_read,
> + monitor_qmp_read, monitor_qmp_event,
> + NULL, mon, NULL, true);
> +monitor_list_append(mon);
> +}
> +}
> +
> +static void monitor_init_hmp(Chardev *chr, int flags)
> +{
> +Monitor *mon = g_malloc(sizeof(*mon));

... and g_malloc0() here (moved from PATCH 04).

This way, the responsibility for zeroing moves just once, right when its
new owners get created.  Saves us explaining in PATCH 03+04 why we make
these changes.  They were confusing enough for me to ask for an
explanation in my review of v2.

Happy do to it in my tree.

I'd be tempted to assert(!(flags & MONITOR_USE_CONTROL)) here, and the
opposite in monitor_init_qmp(), if your PATCH 14 didn't get rid of
@flags.  Okay as it i

Re: [Qemu-devel] [PATCH 0/6] configure: Try to fix --static linking

[Peter Maydell]

On Fri, 14 Jun 2019 at 08:27, Philippe Mathieu-Daudé  wrote:
> Apparently QEMU static linking is slowly bitroting. Obviously it
> depends the libraries an user has installed, anyway it seems there
> are not much testing done.

The main reason for supporting static linking is so we can build
the user-mode emulators. Almost always the problems with
static linking the softmmu binaries and the tools are
issues with the distro's packaging of the static libraries
(pkg-config files which specify things that don't work for
static is a common one).

So we could put in a lot of checking of "is what pkg-config
tells us broken". Or we could just say "we don't support static
linking for anything except the usermode binaries". We
should probably phase in deprecation of that because it's
possible somebody's using it seriously, but it seems like
a fairly weird thing to do to me.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3 03/15] monitor: Make MonitorQMP a child class of Monitor

[Markus Armbruster]

Kevin Wolf  writes:

> Currently, struct Monitor mixes state that is only relevant for HMP,
> state that is only relevant for QMP, and some actually shared state.
> In particular, a MonitorQMP field is present in the state of any
> monitor, even if it's not a QMP monitor and therefore doesn't use the
> state.
>
> As a first step towards a clean separation between QMP and HMP, let
> MonitorQMP extend Monitor and create a MonitorQMP object only when the
> monitor is actually a QMP monitor.
>
> Some places accessed Monitor.qmp unconditionally, even for HMP monitors.
> They can't keep doing this now, so during the conversion, they are
> either changed to become conditional on monitor_is_qmp() or to assert()
> that they always get a QMP monitor.
>
> Signed-off-by: Kevin Wolf 
> Reviewed-by: Dr. David Alan Gilbert 
> ---
>  monitor.c | 220 ++
>  1 file changed, 124 insertions(+), 96 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index a70c1283b1..855a147723 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -168,26 +168,6 @@ struct MonFdset {
[...]
> @@ -712,29 +717,33 @@ static void monitor_data_init(Monitor *mon, int flags, 
> bool skip_flush,
>  }
>  memset(mon, 0, sizeof(Monitor));
>  qemu_mutex_init(&mon->mon_lock);
> -qemu_mutex_init(&mon->qmp.qmp_queue_lock);
>  mon->outbuf = qstring_new();
>  /* Use *mon_cmds by default. */
>  mon->cmd_table = mon_cmds;
>  mon->skip_flush = skip_flush;
>  mon->use_io_thread = use_io_thread;
> -mon->qmp.qmp_requests = g_queue_new();
>  mon->flags = flags;
>  }
>  
> +static void monitor_data_destroy_qmp(MonitorQMP *mon)
> +{
> +json_message_parser_destroy(&mon->parser);
> +qemu_mutex_destroy(&mon->qmp_queue_lock);
> +monitor_qmp_cleanup_req_queue_locked(mon);
> +g_queue_free(mon->qmp_requests);
> +}
> +
>  static void monitor_data_destroy(Monitor *mon)
>  {
>  g_free(mon->mon_cpu_path);
>  qemu_chr_fe_deinit(&mon->chr, false);
>  if (monitor_is_qmp(mon)) {
> -json_message_parser_destroy(&mon->qmp.parser);
> +MonitorQMP *qmp_mon = container_of(mon, MonitorQMP, common);
> +monitor_data_destroy_qmp(qmp_mon);

I dislike declarations hiding among statements, and variables used just
once.  If the variable was used more than once, or its use was in a
complicated expression, or a lengthy line, I'd ask for a blank line to
separate declaration from statement.  But here, I'd prefer

   monitor_data_destroy_qmp(container_of(mon, MonitorQMP, common));

Can touch up in my tree.

>  }
>  readline_free(mon->rs);
>  qobject_unref(mon->outbuf);
>  qemu_mutex_destroy(&mon->mon_lock);
> -qemu_mutex_destroy(&mon->qmp.qmp_queue_lock);
> -monitor_qmp_cleanup_req_queue_locked(mon);
> -g_queue_free(mon->qmp.qmp_requests);
>  }
>  
>  char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
> @@ -1087,8 +1096,12 @@ static void query_commands_cb(QmpCommand *cmd, void 
> *opaque)
>  CommandInfoList *qmp_query_commands(Error **errp)
>  {
>  CommandInfoList *list = NULL;
> +MonitorQMP *mon;
> +
> +assert(monitor_is_qmp(cur_mon));

Could use monitor_cur_is_qmp().  If I mess with it in my tree anyway,
I'll consider changing to it.

> +mon = container_of(cur_mon, MonitorQMP, common);
>  
> -qmp_for_each_command(cur_mon->qmp.commands, query_commands_cb, &list);
> +qmp_for_each_command(mon->commands, query_commands_cb, &list);
>  
>  return list;
>  }
> @@ -1155,16 +1168,16 @@ static void monitor_init_qmp_commands(void)
>   qmp_marshal_qmp_capabilities, QCO_ALLOW_PRECONFIG);
>  }
>  
> -static bool qmp_oob_enabled(Monitor *mon)
> +static bool qmp_oob_enabled(MonitorQMP *mon)
>  {
> -return mon->qmp.capab[QMP_CAPABILITY_OOB];
> +return mon->capab[QMP_CAPABILITY_OOB];
>  }
>  
> -static void monitor_qmp_caps_reset(Monitor *mon)
> +static void monitor_qmp_caps_reset(MonitorQMP *mon)
>  {
> -memset(mon->qmp.capab_offered, 0, sizeof(mon->qmp.capab_offered));
> -memset(mon->qmp.capab, 0, sizeof(mon->qmp.capab));
> -mon->qmp.capab_offered[QMP_CAPABILITY_OOB] = mon->use_io_thread;
> +memset(mon->capab_offered, 0, sizeof(mon->capab_offered));
> +memset(mon->capab, 0, sizeof(mon->capab));
> +mon->capab_offered[QMP_CAPABILITY_OOB] = mon->common.use_io_thread;
>  }
>  
>  /*
> @@ -1172,7 +1185,7 @@ static void monitor_qmp_caps_reset(Monitor *mon)
>   * On success, set mon->qmp.capab[], and return true.
>   * On error, set @errp, and return false.
>   */
> -static bool qmp_caps_accept(Monitor *mon, QMPCapabilityList *list,
> +static bool qmp_caps_accept(MonitorQMP *mon, QMPCapabilityList *list,
>  Error **errp)
>  {
>  GString *unavailable = NULL;
> @@ -1181,7 +1194,7 @@ static bool qmp_caps_accept(Monitor *mon, 
> QMPCapabilityList *list,
>  memset(capab, 0, sizeof(capab));
>  
>  for (; lis

Re: [Qemu-devel] [PATCH v3 04/15] monitor: Create MonitorHMP with readline state

[Markus Armbruster]

Kevin Wolf  writes:

> The ReadLineState in Monitor is only used for HMP monitors. Create
> MonitorHMP and move it there.
>
> Signed-off-by: Kevin Wolf 
> Reviewed-by: Dr. David Alan Gilbert 
> ---
>  include/monitor/monitor.h |   5 +-
>  hmp.c |   4 +-
>  monitor.c | 129 +-
>  3 files changed, 79 insertions(+), 59 deletions(-)
>
> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> index 06cfcd8f36..efdea83bb3 100644
> --- a/include/monitor/monitor.h
> +++ b/include/monitor/monitor.h
> @@ -6,6 +6,7 @@
>  #include "qemu/readline.h"
>  
>  extern __thread Monitor *cur_mon;
> +typedef struct MonitorHMP MonitorHMP;
>  
>  /* flags for monitor_init */
>  /* 0x01 unused */
> @@ -34,8 +35,8 @@ void monitor_flush(Monitor *mon);
>  int monitor_set_cpu(int cpu_index);
>  int monitor_get_cpu_index(void);
>  
> -void monitor_read_command(Monitor *mon, int show_prompt);
> -int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
> +void monitor_read_command(MonitorHMP *mon, int show_prompt);
> +int monitor_read_password(MonitorHMP *mon, ReadLineFunc *readline_func,
>void *opaque);
>  
>  AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
> diff --git a/hmp.c b/hmp.c
> index be5e345c6f..99414cd39c 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1943,6 +1943,8 @@ static void hmp_change_read_arg(void *opaque, const 
> char *password,
>  
>  void hmp_change(Monitor *mon, const QDict *qdict)
>  {
> +/* FIXME Make MonitorHMP public and use container_of */

I think this FIXME should be resolved in PATCH 09.  Recommend to mention
in the commit message.  Can give it a try in my tree.

> +MonitorHMP *hmp_mon = (MonitorHMP *) mon;

Drop the space before mon.  Can touch up in my tree.

>  const char *device = qdict_get_str(qdict, "device");
>  const char *target = qdict_get_str(qdict, "target");
>  const char *arg = qdict_get_try_str(qdict, "arg");
> @@ -1960,7 +1962,7 @@ void hmp_change(Monitor *mon, const QDict *qdict)
>  if (strcmp(target, "passwd") == 0 ||
>  strcmp(target, "password") == 0) {
>  if (!arg) {
> -monitor_read_password(mon, hmp_change_read_arg, NULL);
> +monitor_read_password(hmp_mon, hmp_change_read_arg, NULL);
>  return;
>  }
>  }
> diff --git a/monitor.c b/monitor.c
> index 855a147723..572449f6db 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -192,14 +192,6 @@ struct Monitor {
>  bool skip_flush;
>  bool use_io_thread;
>  
> -/*
> - * State used only in the thread "owning" the monitor.
> - * If @use_io_thread, this is @mon_iothread.
> - * Else, it's the main thread.
> - * These members can be safely accessed without locks.
> - */
> -ReadLineState *rs;
> -
>  gchar *mon_cpu_path;
>  mon_cmd_t *cmd_table;
>  QTAILQ_ENTRY(Monitor) entry;
> @@ -220,6 +212,18 @@ struct Monitor {
>  int mux_out;
>  };
>  
> +struct MonitorHMP {
> +Monitor common;
> +/*
> + * State used only in the thread "owning" the monitor.
> + * If @use_io_thread, this is @mon_iothread. (This does not actually 
> happen
> + * in the current state of the code.)
> + * Else, it's the main thread.
> + * These members can be safely accessed without locks.
> + */
> +ReadLineState *rs;
> +};
> +
>  typedef struct {
>  Monitor common;
>  JSONMessageParser parser;
> @@ -326,7 +330,7 @@ bool monitor_cur_is_qmp(void)
>  return cur_mon && monitor_is_qmp(cur_mon);
>  }
>  
> -void monitor_read_command(Monitor *mon, int show_prompt)
> +void monitor_read_command(MonitorHMP *mon, int show_prompt)
>  {
>  if (!mon->rs)
>  return;
> @@ -336,7 +340,7 @@ void monitor_read_command(Monitor *mon, int show_prompt)
>  readline_show_prompt(mon->rs);
>  }
>  
> -int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
> +int monitor_read_password(MonitorHMP *mon, ReadLineFunc *readline_func,
>void *opaque)
>  {
>  if (mon->rs) {
> @@ -344,7 +348,8 @@ int monitor_read_password(Monitor *mon, ReadLineFunc 
> *readline_func,
>  /* prompt is printed on return from the command handler */
>  return 0;
>  } else {
> -monitor_printf(mon, "terminal does not support password 
> prompting\n");
> +monitor_printf(&mon->common,
> +   "terminal does not support password prompting\n");
>  return -ENOTTY;
>  }
>  }
> @@ -705,7 +710,7 @@ static void monitor_qapi_event_init(void)
>  qapi_event_throttle_equal);
>  }
>  
> -static void handle_hmp_command(Monitor *mon, const char *cmdline);
> +static void handle_hmp_command(MonitorHMP *mon, const char *cmdline);
>  
>  static void monitor_iothread_init(void);
>  
> @@ -715,7 +720,6 @@ static void monitor_d

Re: [Qemu-devel] [PATCH v3 15/15] vl: Deprecate -mon pretty=... for HMP monitors

[Markus Armbruster]

Kevin Wolf  writes:

> The -mon pretty=on|off switch of the -mon option applies only the QMP
> monitors. It used to be silently ignored for HMP. Deprecate this
> combination so that we can make it an error in future versions.
>
> Signed-off-by: Kevin Wolf 
> ---
>  vl.c | 10 +-
>  qemu-deprecated.texi |  6 ++
>  2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/vl.c b/vl.c
> index 32daa434eb..99a56b5556 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2317,6 +2317,10 @@ static int mon_init_func(void *opaque, QemuOpts *opts, 
> Error **errp)
>  return -1;
>  }
>  
> +if (!qmp && qemu_opt_get(opts, "pretty")) {
> +warn_report("'pretty' is deprecated for HMP monitors, it has no 
> effect "
> +"and will be removed in future versions");
> +}
>  if (qemu_opt_get_bool(opts, "pretty", 0)) {
>  pretty = true;
>  }
> @@ -2362,7 +2366,11 @@ static void monitor_parse(const char *optarg, const 
> char *mode, bool pretty)
>  opts = qemu_opts_create(qemu_find_opts("mon"), label, 1, &error_fatal);
>  qemu_opt_set(opts, "mode", mode, &error_abort);
>  qemu_opt_set(opts, "chardev", label, &error_abort);
> -qemu_opt_set_bool(opts, "pretty", pretty, &error_abort);
> +if (!strcmp(mode, "control")) {
> +qemu_opt_set_bool(opts, "pretty", pretty, &error_abort);
> +} else {
> +assert(pretty == false);
> +}
>  monitor_device_index++;
>  }
>  
> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> index 50292d820b..df04f2840b 100644
> --- a/qemu-deprecated.texi
> +++ b/qemu-deprecated.texi
> @@ -72,6 +72,12 @@ backend settings instead of environment variables.  To 
> ease migration to
>  the new format, the ``-audiodev-help'' option can be used to convert
>  the current values of the environment variables to ``-audiodev'' options.
>  
> +@subsection -mon ...,control=readline,pretty=on|off (since 4.1)
> +
> +The @code{pretty=on|off} switch has no effect for HMP monitors, but is
> +silently ignored. Using the switch with HMP monitors will become an
> +error in the future.
> +
>  @subsection -realtime (since 4.1)
>  
>  The @code{-realtime mlock=on|off} argument has been replaced by the

Good move.

Reviewed-by: Markus Armbruster 

Re: [Qemu-devel] [PATCH v8 4/7] block: introduce backup-top filter driver

[Vladimir Sementsov-Ogievskiy]

13.06.2019 18:57, Max Reitz wrote:
> On 29.05.19 17:46, Vladimir Sementsov-Ogievskiy wrote:
>> Backup-top filter does copy-before-write operation. It should be
>> inserted above active disk and has a target node for CBW, like the
>> following:
>>
>>  +---+
>>  | Guest |
>>  +---+
>>  |r,w
>>  v
>>  ++  target   +---+
>>  | backup_top |-->| target(qcow2) |
>>  ++   CBW +---+
>>  |
>> backing |r,w
>>  v
>>  +-+
>>  | Active disk |
>>  +-+
>>
>> The driver will be used in backup instead of write-notifiers.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy 
>> ---
>>   block/backup-top.h  |  64 +
>>   block/backup-top.c  | 322 
>>   block/Makefile.objs |   2 +
>>   3 files changed, 388 insertions(+)
>>   create mode 100644 block/backup-top.h
>>   create mode 100644 block/backup-top.c
>>
>> diff --git a/block/backup-top.h b/block/backup-top.h
>> new file mode 100644
>> index 00..788e18c358
>> --- /dev/null
>> +++ b/block/backup-top.h
>> @@ -0,0 +1,64 @@
>> +/*
>> + * backup-top filter driver
>> + *
>> + * The driver performs Copy-Before-Write (CBW) operation: it is injected 
>> above
>> + * some node, and before each write it copies _old_ data to the target node.
>> + *
>> + * Copyright (c) 2018 Virtuozzo International GmbH. All rights reserved.
>> + *
>> + * Author:
>> + *  Sementsov-Ogievskiy Vladimir 
>> + *
>> + * 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 BACKUP_TOP_H
>> +#define BACKUP_TOP_H
>> +
>> +#include "qemu/osdep.h"
>> +
>> +#include "block/block_int.h"
>> +
>> +typedef void (*BackupTopProgressCallback)(uint64_t done, void *opaque);
>> +typedef struct BDRVBackupTopState {
>> +HBitmap *copy_bitmap; /* what should be copied to @target on guest 
>> write. */
>> +BdrvChild *target;
>> +
>> +BackupTopProgressCallback progress_cb;
>> +void *progress_opaque;
>> +} BDRVBackupTopState;
>> +
>> +/*
>> + * bdrv_backup_top_append
>> + *
>> + * Append backup_top filter node above @source node. @target node will 
>> receive
>> + * the data backed up during CBE operations. New filter together with 
>> @target
>> + * node are attached to @source aio context.
>> + *
>> + * The resulting filter node is implicit.
> 
> Why?  It’s just as easy for the caller to just make it implicit if it
> should be.  (And usually the caller should decide that.)

Personally, I don't know what are the reasons for filters to bi implicit or not,
I just made it like other job-filters.. I can move making-implicit to the caller
or drop it at all (if it will work).

> 
>> + *
>> + * @copy_bitmap selects regions which needs CBW. Furthermore, backup_top 
>> will
>> + * use exactly this bitmap, so it may be used to control backup_top behavior
>> + * dynamically. Caller should not release @copy_bitmap during life-time of
>> + * backup_top. Progress is tracked by calling @progress_cb function.
>> + */
>> +BlockDriverState *bdrv_backup_top_append(
>> +BlockDriverState *source, BlockDriverState *target,
>> +HBitmap *copy_bitmap, Error **errp);
>> +void bdrv_backup_top_set_progress_callback(
>> +BlockDriverState *bs, BackupTopProgressCallback progress_cb,
>> +void *progress_opaque);
>> +void bdrv_backup_top_drop(BlockDriverState *bs);
>> +
>> +#endif /* BACKUP_TOP_H */
>> diff --git a/block/backup-top.c b/block/backup-top.c
>> new file mode 100644
>> index 00..1daa02f539
>> --- /dev/null
>> +++ b/block/backup-top.c
>> @@ -0,0 +1,322 @@
>> +/*
>> + * backup-top filter driver
>> + *
>> + * The driver performs Copy-Before-Write (CBW) operation: it is injected 
>> above
>> + * some node, and before each write it copies _old_ data to the target node.
>> + *
>> + * Copyright (c) 2018 Virtuozzo International GmbH. All rights reserved.
>> + *
>> + * Author:
>> + *  Sementsov-Ogievskiy Vladimir 
>> + *
>> + * 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 

Re: [Qemu-devel] [PULL 0/3] vfio updates 2019-06-13

[Peter Maydell]

On Thu, 13 Jun 2019 at 22:41, Alex Williamson
 wrote:
>
> The following changes since commit 650a379d505bf558bcb41124bc6c951a76cbc113:
>
>   Merge remote-tracking branch 
> 'remotes/pmaydell/tags/pull-target-arm-20190613-1' into staging (2019-06-13 
> 15:16:39 +0100)
>
> are available in the Git repository at:
>
>   git://github.com/awilliam/qemu-vfio.git tags/vfio-updates-20190613.0
>
> for you to fetch changes up to 201a733145751aa691e7e3b9c0f263f0c92db0c5:
>
>   vfio/common: Introduce vfio_set_irq_signaling helper (2019-06-13 09:57:37 
> -0600)
>
> 
> VFIO updates 2019-06-13
>
>  - Hide resizable BAR capability to prevent false guest resizing
>(Alex Williamson)
>
>  - Allow relocation to fix bogus MSI-X hardware (Alex Williamson)
>
>  - Condense IRQ setup into a common helper (Eric Auger)


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.1
for any user-visible changes.

-- PMM

Re: [Qemu-devel] [PATCH v3 00/15] monitor: Split monitor.c in core/HMP/QMP/misc

[Markus Armbruster]

Kevin Wolf  writes:

> monitor.c mixes a lot of different things in a single file: The core
> monitor infrastructure, HMP infrastrcture, QMP infrastructure, and the
> implementation of several HMP and QMP commands. Almost worse, struct
> Monitor mixes state for HMP, for QMP, and state actually shared between
> all monitors. monitor.c must be linked with a system emulator and even
> requires per-target compilation because some of the commands it
> implements access system emulator state.
>
> The reason why I care about this is that I'm working on a protoype for a
> storage daemon, which wants to use QMP (but probably not HMP) and
> obviously doesn't have any system emulator state. So I'm interested in
> some core monitor parts that can be linked to non-system-emulator tools.
>
> This series first creates separate structs MonitorQMP and MonitorHMP
> which inherit from Monitor, and then moves the associated infrastructure
> code into separate source files.
>
> While the split is probably not perfect, I think it's an improvement of
> the current state even for QEMU proper, and it's good enough so I can
> link my storage daemon against just monitor/core.o and monitor/qmp.o and
> get a useless QMP monitor that parses the JSON input and rejects
> everything as an unknown command.
>
> Next I'll try to teach it a subset of QMP commands that can actually be
> supported in a tool, but while there will be a few follow-up patches to
> achieve this, I don't expect that this work will bring up much that
> needs to be changed in the splitting process done in this series.

I think I can address the remaining rather minor issues without a
respin.  Please let me know if you disagree with any of my remarks.

Thanks for helping out with the monitor code!  I know it's rather crusty
in places.

Dave, I'll take this through my tree, if you don't mind.

Re: [Qemu-devel] [PATCH 0/2] qemu-tech: move part to docs/devel, delete part

[Stefan Hajnoczi]

On Fri, Jun 07, 2019 at 04:28:25PM +0100, Peter Maydell wrote:
> This patchset makes some of the cleanups to qemu-tech.texi which
> I suggested in my Sphinx transition plan:
> https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg04932.html
> 
> (1) the "translator internals" docs move to the devel/ manual
> (and are given a simple by-hand texi-to-rst conversion)
> (2) the "compared to other emulators" section is simply deleted:
> since we haven't updated it since 2015 we obviously don't
> care enough about it to keep it useful for users, and the wiki
> or website would be a better place for that kind of "you might
> want to know this before you install" information if we did
> have anybody interested in keeping it current.
> 
> I suspect the TCG docs may have some stale info in them too,
> but the devel/ manual isn't user-facing so not a very big deal.
> Followup patches correcting any inaccuracies welcome :-)
> 
> thanks
> -- PMM
> 
> Peter Maydell (2):
>   Convert "translator internals" docs to RST, move to devel manual
>   qemu-tech.texi: Remove "QEMU compared to other emulators" section
> 
>  docs/devel/index.rst |   1 +
>  docs/devel/tcg.rst   | 111 +++
>  qemu-tech.texi   | 210 ---
>  3 files changed, 112 insertions(+), 210 deletions(-)
>  create mode 100644 docs/devel/tcg.rst
> 
> -- 
> 2.20.1
> 
> 

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v3 15/15] vl: Deprecate -mon pretty=... for HMP monitors

[Kevin Wolf]

Am 14.06.2019 um 11:01 hat Markus Armbruster geschrieben:
> Kevin Wolf  writes:
> 
> > The -mon pretty=on|off switch of the -mon option applies only the QMP

s/the QMP/to QMP/

Can you fix up this one as well while you're at it?

> > monitors. It used to be silently ignored for HMP. Deprecate this
> > combination so that we can make it an error in future versions.
> >
> > Signed-off-by: Kevin Wolf 

Kevin

Re: [Qemu-devel] [PATCH v3 11/15] monitor: Split out monitor/hmp.c

[Dr. David Alan Gilbert]

* Markus Armbruster (arm...@redhat.com) wrote:
> Kevin Wolf  writes:
> 
> > Move HMP infrastructure from monitor/misc.c to monitor/hmp.c. This is
> > code that can be shared for all targets, so compile it only once.
> >
> > The amount of function and particularly extern variables in
> > monitor_int.h is probably a bit larger than it needs to be, but this way
> > no non-trivial code modifications are needed. The interfaces between HMP
> > and the monitor core can be cleaned up later.
> >
> > Signed-off-by: Kevin Wolf 
> > ---
> [...]
> > diff --git a/monitor/hmp.c b/monitor/hmp.c
> > new file mode 100644
> > index 00..3621b195ed
> > --- /dev/null
> > +++ b/monitor/hmp.c
> > @@ -0,0 +1,1415 @@
> [...]
> > +static int64_t expr_unary(Monitor *mon)
> > +{
> > +int64_t n;
> > +char *p;
> > +int ret;
> > +
> > +switch (*pch) {
> > +case '+':
> > +next();
> > +n = expr_unary(mon);
> > +break;
> > +case '-':
> > +next();
> > +n = -expr_unary(mon);
> > +break;
> > +case '~':
> > +next();
> > +n = ~expr_unary(mon);
> > +break;
> > +case '(':
> > +next();
> > +n = expr_sum(mon);
> > +if (*pch != ')') {
> > +expr_error(mon, "')' expected");
> > +}
> > +next();
> > +break;
> > +case '\'':
> > +pch++;
> > +if (*pch == '\0') {
> > +expr_error(mon, "character constant expected");
> > +}
> > +n = *pch;
> > +pch++;
> > +if (*pch != '\'') {
> > +expr_error(mon, "missing terminating \' character");
> > +}
> > +next();
> > +break;
> > +case '$':
> > +{
> > +char buf[128], *q;
> > +int64_t reg = 0;
> > +
> > +pch++;
> > +q = buf;
> > +while ((*pch >= 'a' && *pch <= 'z') ||
> > +   (*pch >= 'A' && *pch <= 'Z') ||
> > +   (*pch >= '0' && *pch <= '9') ||
> > +   *pch == '_' || *pch == '.') {
> > +if ((q - buf) < sizeof(buf) - 1) {
> > +*q++ = *pch;
> > +}
> > +pch++;
> > +}
> > +while (qemu_isspace(*pch)) {
> > +pch++;
> > +}
> > +*q = 0;
> > +ret = get_monitor_def(®, buf);
> > +if (ret < 0) {
> > +expr_error(mon, "unknown register");
> > +}
> > +n = reg;
> > +}
> > +break;
> > +case '\0':
> > +expr_error(mon, "unexpected end of expression");
> > +n = 0;
> > +break;
> > +default:
> > +errno = 0;
> > +n = strtoull(pch, &p, 0);
> 
> checkpatch.pl gripes:
> 
> ERROR: consider using qemu_strtoull in preference to strtoull
> 
> 
> Let's add a TODO comment.  
> 
> > +if (errno == ERANGE) {
> > +expr_error(mon, "number too large");
> > +}
> > +if (pch == p) {
> > +expr_error(mon, "invalid char '%c' in expression", *p);
> > +}
> > +pch = p;
> > +while (qemu_isspace(*pch)) {
> > +pch++;
> > +}
> > +break;
> > +}
> > +return n;
> > +}
> [...]
> > +static void monitor_find_completion(void *opaque,
> > +const char *cmdline)
> > +{
> > +MonitorHMP *mon = opaque;
> > +char *args[MAX_ARGS];
> > +int nb_args, len;
> > +
> > +/* 1. parse the cmdline */
> > +if (parse_cmdline(cmdline, &nb_args, args) < 0) {
> > +return;
> > +}
> > +
> > +/* if the line ends with a space, it means we want to complete the
> > + * next arg */
> 
> checkpatch.pl again:
> 
> WARNING: Block comments use a leading /* on a separate line
> WARNING: Block comments use a trailing */ on a separate line
> 
> Can touch up in my tree.

I wouldn't worry too much about fixing the existing problems here -
let's get the reorg done through kwolf's patches and then it's easier
to clean up later.

Dave

> > +len = strlen(cmdline);
> > +if (len > 0 && qemu_isspace(cmdline[len - 1])) {
> > +if (nb_args >= MAX_ARGS) {
> > +goto cleanup;
> > +}
> > +args[nb_args++] = g_strdup("");
> > +}
> > +
> > +/* 2. auto complete according to args */
> > +monitor_find_completion_by_table(mon, hmp_cmds, args, nb_args);
> > +
> > +cleanup:
> > +free_cmdline_args(args, nb_args);
> > +}
> [...]
> > diff --git a/monitor/misc.c b/monitor/misc.c
> > index 368b8297d4..c8289959c0 100644
> > --- a/monitor/misc.c
> > +++ b/monitor/misc.c
> [...]
> > @@ -612,245 +580,27 @@ out:
> >  return output;
> >  }
> >  
> > -static int compare_cmd(const char *name, const char *list)
> > +/**
> > + * Is @name in the '|' separated list of names @list?
> > + */
> > +int hmp_compare_cmd(const char *name, const char *list)
> >  {
> >  

Re: [Qemu-devel] [PATCH v1 2/4] disas/riscv: Disassemble reserved compressed encodings as illegal

[Palmer Dabbelt]

On Fri, 17 May 2019 15:11:01 PDT (-0700), Alistair Francis wrote:

From: Michael Clark 

Due to the design of the disassembler, the immediate is not
known during decoding of the opcode; so to handle compressed
encodings with reserved immediate values (non-zero), we need
to add an additional check during decompression to match
reserved encodings with zero immediates and translate them
into the illegal instruction.

The following compressed opcodes have reserved encodings with
zero immediates: c.addi4spn, c.addi, c.lui, c.addi16sp, c.srli,
c.srai, c.andi and c.slli

Signed-off-by: Michael Clark 
Signed-off-by: Alistair Francis 
---
 disas/riscv.c | 51 ++-
 1 file changed, 34 insertions(+), 17 deletions(-)

diff --git a/disas/riscv.c b/disas/riscv.c
index 59a9b0437a..3ab4586f0a 100644
--- a/disas/riscv.c
+++ b/disas/riscv.c
@@ -504,14 +504,19 @@ typedef struct {
 const rvc_constraint *constraints;
 } rv_comp_data;

+enum {
+rvcd_imm_nz = 0x1
+};
+
 typedef struct {
 const char * const name;
 const rv_codec codec;
 const char * const format;
 const rv_comp_data *pseudo;
-const int decomp_rv32;
-const int decomp_rv64;
-const int decomp_rv128;
+const short decomp_rv32;
+const short decomp_rv64;
+const short decomp_rv128;
+const short decomp_data;
 } rv_opcode_data;

 /* register names */
@@ -1011,7 +1016,7 @@ const rv_opcode_data opcode_data[] = {
 { "fcvt.q.lu", rv_codec_r_m, rv_fmt_rm_frd_rs1, NULL, 0, 0, 0 },
 { "fmv.x.q", rv_codec_r, rv_fmt_rd_frs1, NULL, 0, 0, 0 },
 { "fmv.q.x", rv_codec_r, rv_fmt_frd_rs1, NULL, 0, 0, 0 },
-{ "c.addi4spn", rv_codec_ciw_4spn, rv_fmt_rd_rs1_imm, NULL, rv_op_addi, 
rv_op_addi, rv_op_addi },
+{ "c.addi4spn", rv_codec_ciw_4spn, rv_fmt_rd_rs1_imm, NULL, rv_op_addi, 
rv_op_addi, rv_op_addi, rvcd_imm_nz },
 { "c.fld", rv_codec_cl_ld, rv_fmt_frd_offset_rs1, NULL, rv_op_fld, 
rv_op_fld, 0 },
 { "c.lw", rv_codec_cl_lw, rv_fmt_rd_offset_rs1, NULL, rv_op_lw, rv_op_lw, 
rv_op_lw },
 { "c.flw", rv_codec_cl_lw, rv_fmt_frd_offset_rs1, NULL, rv_op_flw, 0, 0 },
@@ -1019,14 +1024,14 @@ const rv_opcode_data opcode_data[] = {
 { "c.sw", rv_codec_cs_sw, rv_fmt_rs2_offset_rs1, NULL, rv_op_sw, rv_op_sw, 
rv_op_sw },
 { "c.fsw", rv_codec_cs_sw, rv_fmt_frs2_offset_rs1, NULL, rv_op_fsw, 0, 0 },
 { "c.nop", rv_codec_ci_none, rv_fmt_none, NULL, rv_op_addi, rv_op_addi, 
rv_op_addi },
-{ "c.addi", rv_codec_ci, rv_fmt_rd_rs1_imm, NULL, rv_op_addi, rv_op_addi, 
rv_op_addi },
+{ "c.addi", rv_codec_ci, rv_fmt_rd_rs1_imm, NULL, rv_op_addi, rv_op_addi, 
rv_op_addi, rvcd_imm_nz },
 { "c.jal", rv_codec_cj_jal, rv_fmt_rd_offset, NULL, rv_op_jal, 0, 0 },
 { "c.li", rv_codec_ci_li, rv_fmt_rd_rs1_imm, NULL, rv_op_addi, rv_op_addi, 
rv_op_addi },
-{ "c.addi16sp", rv_codec_ci_16sp, rv_fmt_rd_rs1_imm, NULL, rv_op_addi, 
rv_op_addi, rv_op_addi },
-{ "c.lui", rv_codec_ci_lui, rv_fmt_rd_imm, NULL, rv_op_lui, rv_op_lui, 
rv_op_lui },
-{ "c.srli", rv_codec_cb_sh6, rv_fmt_rd_rs1_imm, NULL, rv_op_srli, 
rv_op_srli, rv_op_srli },
-{ "c.srai", rv_codec_cb_sh6, rv_fmt_rd_rs1_imm, NULL, rv_op_srai, 
rv_op_srai, rv_op_srai },
-{ "c.andi", rv_codec_cb_imm, rv_fmt_rd_rs1_imm, NULL, rv_op_andi, 
rv_op_andi, rv_op_andi },
+{ "c.addi16sp", rv_codec_ci_16sp, rv_fmt_rd_rs1_imm, NULL, rv_op_addi, 
rv_op_addi, rv_op_addi, rvcd_imm_nz },
+{ "c.lui", rv_codec_ci_lui, rv_fmt_rd_imm, NULL, rv_op_lui, rv_op_lui, 
rv_op_lui, rvcd_imm_nz },
+{ "c.srli", rv_codec_cb_sh6, rv_fmt_rd_rs1_imm, NULL, rv_op_srli, 
rv_op_srli, rv_op_srli, rvcd_imm_nz },
+{ "c.srai", rv_codec_cb_sh6, rv_fmt_rd_rs1_imm, NULL, rv_op_srai, 
rv_op_srai, rv_op_srai, rvcd_imm_nz },
+{ "c.andi", rv_codec_cb_imm, rv_fmt_rd_rs1_imm, NULL, rv_op_andi, 
rv_op_andi, rv_op_andi, rvcd_imm_nz },


Unless I'm missing something, c.andi can have a zero immediate.


 { "c.sub", rv_codec_cs, rv_fmt_rd_rs1_rs2, NULL, rv_op_sub, rv_op_sub, 
rv_op_sub },
 { "c.xor", rv_codec_cs, rv_fmt_rd_rs1_rs2, NULL, rv_op_xor, rv_op_xor, 
rv_op_xor },
 { "c.or", rv_codec_cs, rv_fmt_rd_rs1_rs2, NULL, rv_op_or, rv_op_or, 
rv_op_or },
@@ -1036,7 +1041,7 @@ const rv_opcode_data opcode_data[] = {
 { "c.j", rv_codec_cj, rv_fmt_rd_offset, NULL, rv_op_jal, rv_op_jal, 
rv_op_jal },
 { "c.beqz", rv_codec_cb, rv_fmt_rs1_rs2_offset, NULL, rv_op_beq, 
rv_op_beq, rv_op_beq },
 { "c.bnez", rv_codec_cb, rv_fmt_rs1_rs2_offset, NULL, rv_op_bne, 
rv_op_bne, rv_op_bne },
-{ "c.slli", rv_codec_ci_sh6, rv_fmt_rd_rs1_imm, NULL, rv_op_slli, 
rv_op_slli, rv_op_slli },
+{ "c.slli", rv_codec_ci_sh6, rv_fmt_rd_rs1_imm, NULL, rv_op_slli, 
rv_op_slli, rv_op_slli, rvcd_imm_nz },
 { "c.fldsp", rv_codec_ci_ldsp, rv_fmt_frd_offset_rs1, NULL, rv_op_fld, 
rv_op_fld, rv_op_fld },
 { "c.lwsp", rv_codec_ci_lwsp, rv_fmt_rd_offset_rs1, NULL, rv_op_lw, 
rv_op_lw, rv_op_lw },
 { "c.flwsp", rv_codec_ci_lwsp, rv

Re: [Qemu-devel] [PATCH v8 7/7] block/backup: use backup-top instead of write notifiers

[Vladimir Sementsov-Ogievskiy]

13.06.2019 21:02, Max Reitz wrote:
> On 29.05.19 17:46, Vladimir Sementsov-Ogievskiy wrote:
>> Drop write notifiers and use filter node instead. Changes:
>>
>> 1. copy-before-writes now handled by filter node, so, drop all
>> is_write_notifier arguments.
>>
>> 2. we don't have intersecting requests, so their handling is dropped.
>> Instead, synchronization works as follows:
>> when backup or backup-top starts copying of some area it firstly
>> clears copy-bitmap bits, and nobody touches areas, not marked with
>> dirty bits in copy-bitmap, so there is no intersection. Also, backup
>> job copy operations are surrounded by bdrv region lock, which is
>> actually serializing request, to not interfere with guest writes and
>> not read changed data from source (before reading we clear
>> corresponding bit in copy-bitmap, so, this area is not more handled by
>> backup-top).
>>
>> 3. To sync with in-flight requests at job finish we now have drained
>> removing of the filter, we don't need rw-lock.
>>
>> == RFC part ==
>>
>> iotests changed:
>> 56: op-blocker doesn't shot now, as we set it on source, but then check
>> on filter, when trying to start second backup... Should I workaround it
>> somehow?
> 
> Hm.  Where does that error message even come from?  The fact that the
> target image is in use already (Due to file locks)?
> 
> It appears that way indeed.
> 
> It seems reasonable to me that you can now run a backup on top of
> another backup.  Well, I mean, it is a stupid thing to do, but I don’t
> see why the block layer would forbid doing so.
> 
> So the test seems superfluous to me.  If we want to keep it (why not),
> it should test the opposite, namely that a backup to a different image
> (with a different job ID) works.  (It seems simple enough to modify the
> job that way, so why not.)
> 
>> 129: Hmm, now it is not busy at this moment.. But it's illegal to check
>> busy, as job has pause-points and set busy to false in these points.
>> Why we assert it in this test?
> 
> Nobody knows, it’s probably wrong.  All I know is that 129 is just
> broken anyway.
> 
>> 141: Obvious, as drv0 is not root node now, but backing of the filter,
>> when we try to remove it.
> 
> I get a failed assertion in 256.  That is probably because the
> bdrv_set_aio_context() calls weren’t as unnecessary as I deemed them to be.

hmm, will check.

> 
>> Signed-off-by: Vladimir Sementsov-Ogievskiy 
>> ---
>>   block/backup.c | 171 ++---
>>   tests/qemu-iotests/056 |   2 +-
>>   tests/qemu-iotests/129 |   1 -
>>   tests/qemu-iotests/141.out |   2 +-
>>   4 files changed, 68 insertions(+), 108 deletions(-)
> 
> For some reason, my gcc starts to complain that backup_loop() may not
> initialize error_is_read after this patch.  I don’t know why that is.
> Perhaps it inlines backup_do_cow() now?  (So before it just saw that a
> pointer to error_is_read was passed to backup_do_cow() and took it as an
> opaque function, so it surely would set this value somewhere.  Now it
> inlines it and it can’t find whether that will definitely happen, so it
> complains.)
> 
> I don’t think it is strictly necessary to initialize error_is_read, but,
> well, it won’t hurt.
> 
>> diff --git a/block/backup.c b/block/backup.c
>> index 00f4f8af53..a5b8e04c9c 100644
>> --- a/block/backup.c
>> +++ b/block/backup.c
> 
> [...]
> 
>> @@ -60,56 +53,17 @@ typedef struct BackupBlockJob {
>>   
>>   static const BlockJobDriver backup_job_driver;
>>   
>> -/* See if in-flight requests overlap and wait for them to complete */
>> -static void coroutine_fn wait_for_overlapping_requests(BackupBlockJob *job,
>> -   int64_t start,
>> -   int64_t end)
>> -{
>> -CowRequest *req;
>> -bool retry;
>> -
>> -do {
>> -retry = false;
>> -QLIST_FOREACH(req, &job->inflight_reqs, list) {
>> -if (end > req->start_byte && start < req->end_byte) {
>> -qemu_co_queue_wait(&req->wait_queue, NULL);
>> -retry = true;
>> -break;
>> -}
>> -}
>> -} while (retry);
>> -}
>> -
>> -/* Keep track of an in-flight request */
>> -static void cow_request_begin(CowRequest *req, BackupBlockJob *job,
>> -  int64_t start, int64_t end)
>> -{
>> -req->start_byte = start;
>> -req->end_byte = end;
>> -qemu_co_queue_init(&req->wait_queue);
>> -QLIST_INSERT_HEAD(&job->inflight_reqs, req, list);
>> -}
>> -
>> -/* Forget about a completed request */
>> -static void cow_request_end(CowRequest *req)
>> -{
>> -QLIST_REMOVE(req, list);
>> -qemu_co_queue_restart_all(&req->wait_queue);
>> -}
>> -
>>   /* Copy range to target with a bounce buffer and return the bytes copied. 
>> If
>>* error occurred, return a negative error number */
>>   static int coroutine_fn backup_cow_with_bounce_buffer(BackupBlockJob

Re: [Qemu-devel] [PATCH] vhost-user-scsi: prevent using uninitialized vqs

[Stefan Hajnoczi]

On Tue, Jun 11, 2019 at 05:35:17PM -0700, Raphael Norwitz wrote:
> Of the 3 virtqueues, seabios only sets cmd, leaving ctrl
> and event without a physical address. This can cause
> vhost_verify_ring_part_mapping to return ENOMEM, causing
> the following logs:
> 
> qemu-system-x86_64: Unable to map available ring for ring 0
> qemu-system-x86_64: Verify ring failure on region 0
> 
> The qemu commit e6cc11d64fc998c11a4dfcde8fda3fc33a74d844
> has already resolved the issue for vhost scsi devices but
> the fix was never applied to vhost-user scsi devices.
> 
> Signed-off-by: Raphael Norwitz 
> ---
>  hw/scsi/vhost-user-scsi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH qemu] loader: Trace loaded images

[Stefan Hajnoczi]

On Fri, Jun 14, 2019 at 10:13:04AM +1000, Alexey Kardashevskiy wrote:
> 
> 
> On 13/06/2019 23:08, Philippe Mathieu-Daudé wrote:
> > Hi Alexey,
> > 
> > On 6/13/19 7:09 AM, Alexey Kardashevskiy wrote:
> >> This adds a trace point which prints every loaded image. This includes
> >> bios/firmware/kernel/initradmdisk/pcirom.
> >>
> >> Signed-off-by: Alexey Kardashevskiy 
> >> ---
> >>
> >> The example for a pseries guest:
> >>
> >> loader_write_rom slof.bin: @0x0 size=0xe22e0 ROM=0
> >> loader_write_rom phdr #0: /home/aik/t/vml4120le: @0x40 size=0x13df000 
> >> ROM=0
> >> loader_write_rom /home/aik/t/le.cpio: @0x1ad size=0x9463a00 ROM=0
> > 
> > I find the "ROM=0" part confuse, maybe you can change to "ROM:false".
> 
> How? I mean I can do that in the code as rom->isrom?"true":"false" and
> make trace point accept "%s" but it is quite ugly and others seem to
> just use %d for bool.

Yes, %d is the convention for bool.  Perhaps you can name it "is_rom"
instead of "ROM".  That way the name communicates that this is a boolean
value.

Stefan


signature.asc
Description: PGP signature

[Qemu-devel] [PATCH] block/replication.c: Fix crash issue after failover

[Zhang Chen]

From: Zhang Chen 

No block job on active disk after failover.
In the replication_stop() function have canceled the block job,
we check it again here.

Signed-off-by: Zhang Chen 
---
 block/replication.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/block/replication.c b/block/replication.c
index 3d4dedddfc..bdf2bf4bbc 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -146,7 +146,9 @@ static void replication_close(BlockDriverState *bs)
 replication_stop(s->rs, false, NULL);
 }
 if (s->stage == BLOCK_REPLICATION_FAILOVER) {
-job_cancel_sync(&s->active_disk->bs->job->job);
+if (s->secondary_disk->bs->job) {
+job_cancel_sync(&s->secondary_disk->bs->job->job);
+}
 }
 
 if (s->mode == REPLICATION_MODE_SECONDARY) {
-- 
2.17.GIT

Re: [Qemu-devel] [PATCH v3 00/15] monitor: Split monitor.c in core/HMP/QMP/misc

[Kevin Wolf]

Am 14.06.2019 um 11:06 hat Markus Armbruster geschrieben:
> Kevin Wolf  writes:
> 
> > monitor.c mixes a lot of different things in a single file: The core
> > monitor infrastructure, HMP infrastrcture, QMP infrastructure, and the
> > implementation of several HMP and QMP commands. Almost worse, struct
> > Monitor mixes state for HMP, for QMP, and state actually shared between
> > all monitors. monitor.c must be linked with a system emulator and even
> > requires per-target compilation because some of the commands it
> > implements access system emulator state.
> >
> > The reason why I care about this is that I'm working on a protoype for a
> > storage daemon, which wants to use QMP (but probably not HMP) and
> > obviously doesn't have any system emulator state. So I'm interested in
> > some core monitor parts that can be linked to non-system-emulator tools.
> >
> > This series first creates separate structs MonitorQMP and MonitorHMP
> > which inherit from Monitor, and then moves the associated infrastructure
> > code into separate source files.
> >
> > While the split is probably not perfect, I think it's an improvement of
> > the current state even for QEMU proper, and it's good enough so I can
> > link my storage daemon against just monitor/core.o and monitor/qmp.o and
> > get a useless QMP monitor that parses the JSON input and rejects
> > everything as an unknown command.
> >
> > Next I'll try to teach it a subset of QMP commands that can actually be
> > supported in a tool, but while there will be a few follow-up patches to
> > achieve this, I don't expect that this work will bring up much that
> > needs to be changed in the splitting process done in this series.
> 
> I think I can address the remaining rather minor issues without a
> respin.  Please let me know if you disagree with any of my remarks.

Feel free to make the changes you suggested, possibly with the exception
of the #includes in monitor-internal.h where I think you're only
partially right (see my reply there).

Please also consider fixing the commit message typo I pointed out for
patch 15.

Kevin

[Qemu-devel] [PATCH v3 1/5] virtio: add "use-started" property

[elohimes]

From: Xie Yongji 

In order to avoid migration issues, we introduce a "use-started"
property to the base virtio device to indicate whether use
"started" flag or not. This property will be true by default and
set to false when machine type <= 4.0.1.

Suggested-by: Greg Kurz 
Signed-off-by: Xie Yongji 
---
 hw/block/vhost-user-blk.c  |  4 ++--
 hw/core/machine.c  |  8 ++--
 hw/virtio/virtio.c | 18 +++---
 include/hw/virtio/virtio.h | 21 +
 4 files changed, 36 insertions(+), 15 deletions(-)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 9cb61336a6..85bc4017e7 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -191,7 +191,7 @@ static void vhost_user_blk_stop(VirtIODevice *vdev)
 static void vhost_user_blk_set_status(VirtIODevice *vdev, uint8_t status)
 {
 VHostUserBlk *s = VHOST_USER_BLK(vdev);
-bool should_start = vdev->started;
+bool should_start = virtio_device_started(vdev, status);
 int ret;
 
 if (!vdev->vm_running) {
@@ -317,7 +317,7 @@ static int vhost_user_blk_connect(DeviceState *dev)
 }
 
 /* restore vhost state */
-if (vdev->started) {
+if (virtio_device_started(vdev, vdev->status)) {
 ret = vhost_user_blk_start(vdev);
 if (ret < 0) {
 error_report("vhost-user-blk: vhost start failed: %s",
diff --git a/hw/core/machine.c b/hw/core/machine.c
index f1a0f45f9c..60d1e27d2c 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -24,10 +24,14 @@
 #include "hw/pci/pci.h"
 #include "hw/mem/nvdimm.h"
 
-GlobalProperty hw_compat_4_0_1[] = {};
+GlobalProperty hw_compat_4_0_1[] = {
+{ "virtio-device", "use-started", "false" },
+};
 const size_t hw_compat_4_0_1_len = G_N_ELEMENTS(hw_compat_4_0_1);
 
-GlobalProperty hw_compat_4_0[] = {};
+GlobalProperty hw_compat_4_0[] = {
+{ "virtio-device", "use-started", "false" },
+};
 const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0);
 
 GlobalProperty hw_compat_3_1[] = {
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 07f4a64b48..19062fbb96 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1162,10 +1162,8 @@ int virtio_set_status(VirtIODevice *vdev, uint8_t val)
 }
 }
 }
-vdev->started = val & VIRTIO_CONFIG_S_DRIVER_OK;
-if (unlikely(vdev->start_on_kick && vdev->started)) {
-vdev->start_on_kick = false;
-}
+
+virtio_set_started(vdev, val & VIRTIO_CONFIG_S_DRIVER_OK);
 
 if (k->set_status) {
 k->set_status(vdev, val);
@@ -1536,8 +1534,7 @@ static bool virtio_queue_notify_aio_vq(VirtQueue *vq)
 ret = vq->handle_aio_output(vdev, vq);
 
 if (unlikely(vdev->start_on_kick)) {
-vdev->started = true;
-vdev->start_on_kick = false;
+virtio_set_started(vdev, true);
 }
 }
 
@@ -1557,8 +1554,7 @@ static void virtio_queue_notify_vq(VirtQueue *vq)
 vq->handle_output(vdev, vq);
 
 if (unlikely(vdev->start_on_kick)) {
-vdev->started = true;
-vdev->start_on_kick = false;
+virtio_set_started(vdev, true);
 }
 }
 }
@@ -1579,8 +1575,7 @@ void virtio_queue_notify(VirtIODevice *vdev, int n)
 }
 
 if (unlikely(vdev->start_on_kick)) {
-vdev->started = true;
-vdev->start_on_kick = false;
+virtio_set_started(vdev, true);
 }
 }
 
@@ -2291,7 +2286,7 @@ static void virtio_vmstate_change(void *opaque, int 
running, RunState state)
 VirtIODevice *vdev = opaque;
 BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
 VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
-bool backend_run = running && vdev->started;
+bool backend_run = running && virtio_device_started(vdev, vdev->status);
 vdev->vm_running = running;
 
 if (backend_run) {
@@ -2669,6 +2664,7 @@ static void virtio_device_instance_finalize(Object *obj)
 
 static Property virtio_properties[] = {
 DEFINE_VIRTIO_COMMON_FEATURES(VirtIODevice, host_features),
+DEFINE_PROP_BOOL("use-started", VirtIODevice, use_started, true),
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 27c0efc3d0..15d5366939 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -105,6 +105,7 @@ struct VirtIODevice
 uint16_t device_id;
 bool vm_running;
 bool broken; /* device in invalid state, needs reset */
+bool use_started;
 bool started;
 bool start_on_kick; /* virtio 1.0 transitional devices support that */
 VMChangeStateEntry *vmstate;
@@ -351,4 +352,24 @@ static inline bool virtio_is_big_endian(VirtIODevice *vdev)
 /* Devices conforming to VIRTIO 1.0 or later are always LE. */
 return false;
 }
+
+static inline bool virtio_device_started(VirtIODevice *vdev, uint8_t status)
+{
+if (vdev->use_started) {
+return vdev->started;
+}
+
+return status & VIRTIO_CONFIG_S_DRIVER_OK;
+}
+
+s

[Qemu-devel] [PATCH v3 3/5] virtio: Set "start_on_kick" on virtio_set_features()

[elohimes]

From: Xie Yongji 

The guest feature is not set correctly on virtio_reset() and
virtio_init(). So we should not use it to set "start_on_kick" at that
point. This patch set "start_on_kick" on virtio_set_features() instead.

Fixes: badaf79cfdbd3 ("virtio: Introduce started flag to VirtioDevice")
Signed-off-by: Xie Yongji 
Reviewed-by: Greg Kurz 
---
 hw/virtio/virtio.c | 28 
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 473881e9ec..034320d277 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1212,7 +1212,7 @@ void virtio_reset(void *opaque)
 k->reset(vdev);
 }
 
-vdev->start_on_kick = !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1);
+vdev->start_on_kick = false;
 vdev->started = false;
 vdev->broken = false;
 vdev->guest_features = 0;
@@ -2063,14 +2063,21 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t 
val)
 return -EINVAL;
 }
 ret = virtio_set_features_nocheck(vdev, val);
-if (!ret && virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
-/* VIRTIO_RING_F_EVENT_IDX changes the size of the caches.  */
-int i;
-for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
-if (vdev->vq[i].vring.num != 0) {
-virtio_init_region_cache(vdev, i);
+if (!ret) {
+if (virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
+/* VIRTIO_RING_F_EVENT_IDX changes the size of the caches.  */
+int i;
+for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
+if (vdev->vq[i].vring.num != 0) {
+virtio_init_region_cache(vdev, i);
+}
 }
 }
+
+if (!virtio_device_started(vdev, vdev->status) &&
+!virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
+vdev->start_on_kick = true;
+}
 }
 return ret;
 }
@@ -,6 +2229,11 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int 
version_id)
 }
 }
 
+if (!virtio_device_started(vdev, vdev->status) &&
+!virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
+vdev->start_on_kick = true;
+}
+
 rcu_read_lock();
 for (i = 0; i < num; i++) {
 if (vdev->vq[i].vring.desc) {
@@ -2324,7 +2336,7 @@ void virtio_init(VirtIODevice *vdev, const char *name,
 g_malloc0(sizeof(*vdev->vector_queues) * nvectors);
 }
 
-vdev->start_on_kick = !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1);
+vdev->start_on_kick = false;
 vdev->started = false;
 vdev->device_id = device_id;
 vdev->status = 0;
-- 
2.17.1

[Qemu-devel] [PATCH v3 4/5] virtio: Make sure we get correct state of device on handle_aio_output()

[elohimes]

From: Xie Yongji 

We should set the flags: "start_on_kick" and "started" after we call
the kick functions (handle_aio_output() and handle_output()).

Signed-off-by: Xie Yongji 
---
 hw/virtio/virtio.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 034320d277..b4301f9e02 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1571,10 +1571,10 @@ void virtio_queue_notify(VirtIODevice *vdev, int n)
 event_notifier_set(&vq->host_notifier);
 } else if (vq->handle_output) {
 vq->handle_output(vdev, vq);
-}
 
-if (unlikely(vdev->start_on_kick)) {
-virtio_set_started(vdev, true);
+if (unlikely(vdev->start_on_kick)) {
+virtio_set_started(vdev, true);
+}
 }
 }
 
-- 
2.17.1

Re: [Qemu-devel] Virtual I2C adapter and I2C external simulator

[Stefan Hajnoczi]

On Thu, Jun 13, 2019 at 10:53:16AM -0700, Rahul Govind wrote:
> Hi Everyone,
> 
> I'm working on a project that involves building on top of existing QEMU
> work, and I'm trying to virtualize I2C devices that are shared between
> multiple VMs. I've been reading QEMU documentation and source code to get a
> better idea of how to add something like this to QEMU (since it doesn't
> look like something similar is already present) but I'm not sure how to
> proceed. I do have a few ideas and I would be grateful if anyone could give
> me feedback on them or suggestions for alternative solutions.
> 
> More specifically, I wish to virtualize a couple of devices that sit on the
> I2C bus. However, directly virtualizing the devices doesn't seem feasible
> since there are many other devices like multiplexers on the way to these
> devices. Also, what we need is to handle is just reads/write/ioctls on
> /dev/i2c-N.
> 
> I was wondering if there is a way to create a device that
> - 1) Emulates an I2C adapter, say /dev/i2c-0.
> - 2) Handles any reads/writes/ioctls sent to /dev/i2c-0, reformats any
> ioctls as IO, and then sends this to a simulator on the host.
> 
> This approach is just based on the ipmi interface and ipmi-bmc-extern
> device already present in QEMU, where ipmi-bmc-extern can communicate
> through a chardev with a simulator running on the host. Having a simulator
> on the host would be nice to have since it lets us write the simulator in a
> higher level language, and as previously mentioned, we have a few cases
> where multiple VMs share the same I2C device. However, I'm not sure if this
> even makes sense in the case of I2C.
> 
> If this approach doesn't make sense, what method would you recommend to
> have virtual I2C devices which are shared between multiple VMs?
> 
> Any suggestions or guidance is much appreciated!

Hi Rahul,
A good rule of thumb is that if it's possible in real hardware then it
can be emulated.  So in this case the question is whether an I2C device
can be accessed by multiple bus masters.  The answer seems to be yes:
https://www.i2c-bus.org/multimaster/

Ernest recently posted patches to add a chardev to QEMU that
reads/writes to a host /dev/i2c-0 device.  What you are describing is a
little different but related.

In theory it is possible to define an I2C-over-socket protocol (for
example a UNIX Domain Socket) so that an external process can simulate
an I2C device.  QEMU will emulate the I2C controller and forward bus
transactions to the external process.

If you want multi-master I2C then the external process needs to accept
multiple connections so that more than one QEMU can connect at
simultaneously.

Stefan


signature.asc
Description: PGP signature

[Qemu-devel] [PATCH v3 0/5] virtio: fix some issues of "started" and "start_on_kick" flag

[elohimes]

From: Xie Yongji 

We introduced two flags "started" and "start_on_kick" to indicate virtio
device's state before. But there still are some problems with them. So
we try to fixup them in this patchset.

The patch 1 introduces a "use-started" property to avoid a migration
issue under Greg Kurz's suggestion [1].

The patch 2 set "start_on_kick" flag for legacy devices.

The patch 3 fixes a regression bug that old guest is not able to boot with
vhost-user-blk device.

The patch 4,5 fix some problems with "started" and "start_on_kick" flag.

[1] https://lists.gnu.org/archive/html/qemu-devel/2019-05/msg06247.html

v3:
- change the order of patches
- Also disable "use-started" property by hw_compat_4_0

v2:
- Recalculate "start_on_kick" flag after migration instead of migrating
  it
- Set "start_on_kick" flag for legacy devices
- Avoid setting "started" flag when "use_started" property is true
- Set "use_started" to false by hw_compat_4_0_1 instead of hw_compat_4_0

Xie Yongji (5):
  virtio: add "use-started" property
  virtio: Set "start_on_kick" for legacy devices
  virtio: Set "start_on_kick" on virtio_set_features()
  virtio: Make sure we get correct state of device on
handle_aio_output()
  virtio: Don't change "started" flag on virtio_vmstate_change()

 hw/block/vhost-user-blk.c  |  4 +--
 hw/core/machine.c  |  8 --
 hw/virtio/virtio.c | 53 ++
 include/hw/virtio/virtio.h | 23 -
 4 files changed, 61 insertions(+), 27 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH v3 5/5] virtio: Don't change "started" flag on virtio_vmstate_change()

[elohimes]

From: Xie Yongji 

We will call virtio_set_status() on virtio_vmstate_change().
The "started" flag should not be changed in this case. Otherwise,
we may get an incorrect value when we set "started" flag but
not set DRIVER_OK in source VM.

Signed-off-by: Xie Yongji 
---
 hw/virtio/virtio.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index b4301f9e02..9af2e339af 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1163,7 +1163,10 @@ int virtio_set_status(VirtIODevice *vdev, uint8_t val)
 }
 }
 
-virtio_set_started(vdev, val & VIRTIO_CONFIG_S_DRIVER_OK);
+if ((vdev->status & VIRTIO_CONFIG_S_DRIVER_OK) !=
+(val & VIRTIO_CONFIG_S_DRIVER_OK)) {
+virtio_set_started(vdev, val & VIRTIO_CONFIG_S_DRIVER_OK);
+}
 
 if (k->set_status) {
 k->set_status(vdev, val);
-- 
2.17.1

[Qemu-devel] [PATCH] vfio-ccw: use vfio_set_irq_signaling

[Cornelia Huck]

Use the new helper.

Signed-off-by: Cornelia Huck 
---
 hw/vfio/ccw.c | 68 +++
 1 file changed, 14 insertions(+), 54 deletions(-)

diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
index 03a2becb3ec9..3dc08721a3db 100644
--- a/hw/vfio/ccw.c
+++ b/hw/vfio/ccw.c
@@ -197,10 +197,7 @@ read_err:
 static void vfio_ccw_register_io_notifier(VFIOCCWDevice *vcdev, Error **errp)
 {
 VFIODevice *vdev = &vcdev->vdev;
-struct vfio_irq_info *irq_info;
-struct vfio_irq_set *irq_set;
-size_t argsz;
-int32_t *pfd;
+int fd;
 
 if (vdev->num_irqs < VFIO_CCW_IO_IRQ_INDEX + 1) {
 error_setg(errp, "vfio: unexpected number of io irqs %u",
@@ -208,72 +205,35 @@ static void vfio_ccw_register_io_notifier(VFIOCCWDevice 
*vcdev, Error **errp)
 return;
 }
 
-argsz = sizeof(*irq_info);
-irq_info = g_malloc0(argsz);
-irq_info->index = VFIO_CCW_IO_IRQ_INDEX;
-irq_info->argsz = argsz;
-if (ioctl(vdev->fd, VFIO_DEVICE_GET_IRQ_INFO,
-  irq_info) < 0 || irq_info->count < 1) {
-error_setg_errno(errp, errno, "vfio: Error getting irq info");
-goto out_free_info;
-}
-
 if (event_notifier_init(&vcdev->io_notifier, 0)) {
 error_setg_errno(errp, errno,
  "vfio: Unable to init event notifier for IO");
-goto out_free_info;
+return;
 }
 
-argsz = sizeof(*irq_set) + sizeof(*pfd);
-irq_set = g_malloc0(argsz);
-irq_set->argsz = argsz;
-irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD |
- VFIO_IRQ_SET_ACTION_TRIGGER;
-irq_set->index = VFIO_CCW_IO_IRQ_INDEX;
-irq_set->start = 0;
-irq_set->count = 1;
-pfd = (int32_t *) &irq_set->data;
-
-*pfd = event_notifier_get_fd(&vcdev->io_notifier);
-qemu_set_fd_handler(*pfd, vfio_ccw_io_notifier_handler, NULL, vcdev);
-if (ioctl(vdev->fd, VFIO_DEVICE_SET_IRQS, irq_set)) {
-error_setg(errp, "vfio: Failed to set up io notification");
-qemu_set_fd_handler(*pfd, NULL, NULL, vcdev);
+fd = event_notifier_get_fd(&vcdev->io_notifier);
+qemu_set_fd_handler(fd, vfio_ccw_io_notifier_handler, NULL, vcdev);
+
+if (vfio_set_irq_signaling(vdev, VFIO_CCW_IO_IRQ_INDEX, 0,
+   VFIO_IRQ_SET_ACTION_TRIGGER, fd, errp)) {
+qemu_set_fd_handler(fd, NULL, NULL, vcdev);
 event_notifier_cleanup(&vcdev->io_notifier);
 }
-
-g_free(irq_set);
-
-out_free_info:
-g_free(irq_info);
 }
 
 static void vfio_ccw_unregister_io_notifier(VFIOCCWDevice *vcdev)
 {
-struct vfio_irq_set *irq_set;
-size_t argsz;
-int32_t *pfd;
-
-argsz = sizeof(*irq_set) + sizeof(*pfd);
-irq_set = g_malloc0(argsz);
-irq_set->argsz = argsz;
-irq_set->flags = VFIO_IRQ_SET_DATA_EVENTFD |
- VFIO_IRQ_SET_ACTION_TRIGGER;
-irq_set->index = VFIO_CCW_IO_IRQ_INDEX;
-irq_set->start = 0;
-irq_set->count = 1;
-pfd = (int32_t *) &irq_set->data;
-*pfd = -1;
-
-if (ioctl(vcdev->vdev.fd, VFIO_DEVICE_SET_IRQS, irq_set)) {
-error_report("vfio: Failed to de-assign device io fd: %m");
+Error *err = NULL;
+
+vfio_set_irq_signaling(&vcdev->vdev, VFIO_CCW_IO_IRQ_INDEX, 0,
+   VFIO_IRQ_SET_ACTION_TRIGGER, -1, &err);
+if (err) {
+error_reportf_err(err, VFIO_MSG_PREFIX, vcdev->vdev.name);
 }
 
 qemu_set_fd_handler(event_notifier_get_fd(&vcdev->io_notifier),
 NULL, NULL, vcdev);
 event_notifier_cleanup(&vcdev->io_notifier);
-
-g_free(irq_set);
 }
 
 static void vfio_ccw_get_region(VFIOCCWDevice *vcdev, Error **errp)
-- 
2.20.1

Re: [Qemu-devel] [PATCH 1/2] vl: Drain before (block) job cancel when quitting

[Vladimir Sementsov-Ogievskiy]

13.06.2019 19:03, Max Reitz wrote:
> [re-adding the original CCs, why not]
> 
> On 13.06.19 16:30, Vladimir Sementsov-Ogievskiy wrote:
>> 13.06.2019 17:21, Max Reitz wrote:
>>> On 13.06.19 16:19, Vladimir Sementsov-Ogievskiy wrote:
 13.06.2019 1:08, Max Reitz wrote:
> If the main loop cancels all block jobs while the block layer is not
> drained, this cancelling may not happen instantaneously.  We can start a
> drained section before vm_shutdown(), which entails another
> bdrv_drain_all(); this nested bdrv_drain_all() will thus be a no-op,
> basically.
>
> We do not have to end the drained section, because we actually do not
> want any requests to happen from this point on.
>
> Signed-off-by: Max Reitz 
> ---
> I don't know whether it actually makes sense to never end this drained
> section.  It makes sense to me.  Please correct me if I'm wrong.
> ---
> vl.c | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/vl.c b/vl.c
> index cd1fbc4cdc..3f8b3f74f5 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4538,6 +4538,17 @@ int main(int argc, char **argv, char **envp)
>  */
> migration_shutdown();
> 
> +/*
> + * We must cancel all block jobs while the block layer is drained,
> + * or cancelling will be affected by throttling and thus may block
> + * for an extended period of time.
> + * vm_shutdown() will bdrv_drain_all(), so we may as well include
> + * it in the drained section.
> + * We do not need to end this section, because we do not want any
> + * requests happening from here on anyway.
> + */
> +bdrv_drain_all_begin();
> +
> /* No more vcpu or device emulation activity beyond this point */
> vm_shutdown();
> 
>

 So, actually, the problem is that we may wait for job requests twice:
 on drain and then on cancel.
>>>
>>> We don’t wait on drain.  When the throttle node is drained, it will
>>> ignore throttling (as noted in the cover letter).
>>>
>>> We do wait when cancelling a job while the throttle node isn’t drained,
>>> though.  That’s the problem.
>>
>> Ah, understand now.
>>
>> Is it safe to drain_begin before stopping cpus? We may finish up then with 
>> some queued
>> somewhere IO requests..
> 
> Hm...  Aren’t guest devices prohibited from issuing requests to the
> block layer while their respective block device is drained?

It's at least a buggy place, I remember Denis Plotnikov sent patch to fix it 
and had a huge
discussion with Kevin.
And here it is:
https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg00732.html

> 
> Otherwise, I suppose I’ll have to move the bdrv_drain_all_begin() below
> the vm_shutdown().  That wouldn’t be too big of a problem.
> 
> Max
> 


-- 
Best regards,
Vladimir

[Qemu-devel] [PATCH v3 2/5] virtio: Set "start_on_kick" for legacy devices

[elohimes]

From: Xie Yongji 

Besides virtio 1.0 transitional devices, we should also
set "start_on_kick" flag for legacy devices (virtio 0.9).

Signed-off-by: Xie Yongji 
---
 hw/virtio/virtio.c | 6 ++
 include/hw/virtio/virtio.h | 2 +-
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 19062fbb96..473881e9ec 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1212,8 +1212,7 @@ void virtio_reset(void *opaque)
 k->reset(vdev);
 }
 
-vdev->start_on_kick = (virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1) &&
-  !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1));
+vdev->start_on_kick = !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1);
 vdev->started = false;
 vdev->broken = false;
 vdev->guest_features = 0;
@@ -2325,8 +2324,7 @@ void virtio_init(VirtIODevice *vdev, const char *name,
 g_malloc0(sizeof(*vdev->vector_queues) * nvectors);
 }
 
-vdev->start_on_kick = (virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1) &&
-  !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1));
+vdev->start_on_kick = !virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1);
 vdev->started = false;
 vdev->device_id = device_id;
 vdev->status = 0;
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 15d5366939..b189788cb2 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -107,7 +107,7 @@ struct VirtIODevice
 bool broken; /* device in invalid state, needs reset */
 bool use_started;
 bool started;
-bool start_on_kick; /* virtio 1.0 transitional devices support that */
+bool start_on_kick; /* when virtio 1.0 feature has not been negotiated */
 VMChangeStateEntry *vmstate;
 char *bus_name;
 uint8_t device_endian;
-- 
2.17.1

Re: [Qemu-devel] [QEMU] [PATCH v2 0/8] Add Qemu to SeaBIOS LCHS interface

[Sam Eiderman]

> On 14 Jun 2019, at 7:43, Gerd Hoffmann  wrote:
> 
>  Hi,
> 
>> Can there be a guest that will fail the MBR in such a way? Yes.
>> Look at the following MBR partition table of a Windows XP guest in our 
>> production
>> environment:
>> 
>> Disk size in sectors: 16777216
>> 
>> Binary (only one partition 16 bytes): 80 01 01 00 07 fe ff ff 3f 00 00 00 d5 
>> ea ff 00
>> Start: (0, 1, 1, 63)
>> End: (1023, 254, 63, 16771859)
>> 
>> As can be easily seen, any MBR guessing algorithm should guess:
>> 
>>  255 heads (since a value of 254 appears), 63 spt (since a value of 63 
>> appears)
>> 
>> Turns out that this image does not work with 255, 63 but actually requires
>> 
>>  16 heads, 63 spt
>> 
>> to boot.
>> 
>> So relying on MBR partitions alone is not always enough and sometimes manual 
>> intervention
>> is required.
> 
> Ok, given that seabios has no setup any manual configuration needs to be done 
> via qemu.
> 
> But why do we need a new interface for that?  IDE can pass the geometry
> to the guest.  virtio-blk has support too (VIRTIO_BLK_F_GEOMETRY).
> Likewise scsi (MODE_PAGE_HD_GEOMETRY).  So this should be doable without
> any qemu changes.

This was indeed considered (all 3 methods) but it has the following issues:

Physical geometries of devices must now also be logical geometries with 
translation=none.
When the OS will query these devices - It will now see different physical 
geometries, adapted to be logical geometries.
I’m not sure even how to implement this without breaking existing compatibility 
- since we don’t want to affect logical geometries of currently used guests.
MODE_PAGE_HD_GEOMETRY does not contain the spts, only cylinders (as 3 byte 
number) and heads (as 1 byte number) and computes the spts using: 
number_of_total_sectors / (heads * cylinders), this means that qemu now must 
report number_of_total_sectors as heads * cylinders * spt for SeaBIOS to 
correctly receive the number of spts - this is not optimal since 
number_of_total_sectors can not reflect the real amount of sectors in the disk 
which should be reported from CDB_CMD_READ_CAPACITY.
Moving a scsi-hd/virtio-blk with 255 physical heads to ide-hd, we will still 
need to report 255 heads - this is possible since a whole byte can be used in 
the “ide identify” command, but goes against the spec of a maximum of 16 heads 
for IDE.

Overall this approach is much more complicated.

Sam

> 
> cheers,
>  Gerd
> 

Re: [Qemu-devel] [PATCH] iotests: Hide timestamps for skipped tests

[Kevin Wolf]

Am 13.06.2019 um 20:37 hat Max Reitz geschrieben:
> Currently, the "thistime" variable is not reinitialized on every loop
> iteration.  This leads to tests that do not yield a run time (because
> they failed or were skipped) printing the run time of the previous test
> that did.  Fix that by reinitializing "thistime" for every test.
> 
> Signed-off-by: Max Reitz 

Thanks, applied to the block branch.

Kevin

Re: [Qemu-devel] [PATCH v1 3/4] disas/riscv: Fix `rdinstreth` constraint

[Palmer Dabbelt]

On Fri, 17 May 2019 15:11:04 PDT (-0700), Alistair Francis wrote:

From: Michael Clark 

The constraint for `rdinstreth` was comparing the csr number to 0xc80,
which is `cycleh` instead. Fix this.

Author: Wladimir J. van der Laan 


I'm not sure what this tag is supposed to mean.  If this is the actual author
of the patch, then shouldn't it also have a SOB?


Signed-off-by: Michael Clark 
Signed-off-by: Alistair Francis 
---
 disas/riscv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/disas/riscv.c b/disas/riscv.c
index 3ab4586f0a..c2578a3c4b 100644
--- a/disas/riscv.c
+++ b/disas/riscv.c
@@ -614,7 +614,7 @@ static const rvc_constraint rvcc_rdtime[] = { 
rvc_rs1_eq_x0, rvc_csr_eq_0xc01, r
 static const rvc_constraint rvcc_rdinstret[] = { rvc_rs1_eq_x0, 
rvc_csr_eq_0xc02, rvc_end };
 static const rvc_constraint rvcc_rdcycleh[] = { rvc_rs1_eq_x0, 
rvc_csr_eq_0xc80, rvc_end };
 static const rvc_constraint rvcc_rdtimeh[] = { rvc_rs1_eq_x0, 
rvc_csr_eq_0xc81, rvc_end };
-static const rvc_constraint rvcc_rdinstreth[] = { rvc_rs1_eq_x0, 
rvc_csr_eq_0xc80, rvc_end };
+static const rvc_constraint rvcc_rdinstreth[] = { rvc_rs1_eq_x0, 
rvc_csr_eq_0xc82, rvc_end };
 static const rvc_constraint rvcc_frcsr[] = { rvc_rs1_eq_x0, rvc_csr_eq_0x003, 
rvc_end };
 static const rvc_constraint rvcc_frrm[] = { rvc_rs1_eq_x0, rvc_csr_eq_0x002, 
rvc_end };
 static const rvc_constraint rvcc_frflags[] = { rvc_rs1_eq_x0, 
rvc_csr_eq_0x001, rvc_end };

Re: [Qemu-devel] [PATCH v6] ssh: switch from libssh2 to libssh

[Kevin Wolf]

Am 13.06.2019 um 21:41 hat Stefan Weil geschrieben:
> On 12.06.19 15:27, Philippe Mathieu-Daudé wrote:
> > Cc'ing Alex (Docker, Travis) and Stefan (MinGW)
> [...]
> > Note, libssh is not available on MinGW.
> 
> Nor is it available for Mingw64:
> 
> https://cygwin.com/cgi-bin2/package-grep.cgi?grep=mingw64-x86_64-libssh&arch=x86_64
> 
> That makes building for Windows more difficult because there is an
> additional dependency which must be built from source.

How many people do actually use the ssh block driver on Windows, though?
Isn't just building QEMU without it a quite reasonable option, too?

I wouldn't consider this a strong argument why we should keep using an
obsolete library.

Kevin

Re: [Qemu-devel] [PATCH v1 4/4] target/riscv: Implement riscv_cpu_unassigned_access

[Palmer Dabbelt]

On Fri, 17 May 2019 15:11:06 PDT (-0700), Alistair Francis wrote:

From: Michael Clark 

This patch adds support for the riscv_cpu_unassigned_access call
and will raise a load or store access fault.

Signed-off-by: Michael Clark 
[Changes by AF:
 - Squash two patches and rewrite commit message
 - Set baddr to the access address
]
Signed-off-by: Alistair Francis 
---
 target/riscv/cpu.c|  1 +
 target/riscv/cpu.h|  2 ++
 target/riscv/cpu_helper.c | 16 
 3 files changed, 19 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index b7675707e0..bfe92235d3 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -356,6 +356,7 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data)
 cc->gdb_stop_before_watchpoint = true;
 cc->disas_set_info = riscv_cpu_disas_set_info;
 #ifndef CONFIG_USER_ONLY
+cc->do_unassigned_access = riscv_cpu_unassigned_access;
 cc->do_unaligned_access = riscv_cpu_do_unaligned_access;
 cc->get_phys_page_debug = riscv_cpu_get_phys_page_debug;
 #endif
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index c17184f4e4..8250175811 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -264,6 +264,8 @@ void  riscv_cpu_do_unaligned_access(CPUState *cs, vaddr 
addr,
 bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
 MMUAccessType access_type, int mmu_idx,
 bool probe, uintptr_t retaddr);
+void riscv_cpu_unassigned_access(CPUState *cpu, hwaddr addr, bool is_write,
+ bool is_exec, int unused, unsigned size);
 char *riscv_isa_string(RISCVCPU *cpu);
 void riscv_cpu_list(void);

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 41d6db41c3..202b6f021d 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -356,6 +356,22 @@ hwaddr riscv_cpu_get_phys_page_debug(CPUState *cs, vaddr 
addr)
 return phys_addr;
 }

+void riscv_cpu_unassigned_access(CPUState *cs, hwaddr addr, bool is_write,
+ bool is_exec, int unused, unsigned size)
+{
+RISCVCPU *cpu = RISCV_CPU(cs);
+CPURISCVState *env = &cpu->env;
+
+if (is_write) {
+cs->exception_index = RISCV_EXCP_STORE_AMO_ACCESS_FAULT;
+} else {
+cs->exception_index = RISCV_EXCP_LOAD_ACCESS_FAULT;
+}
+
+env->badaddr = addr;
+riscv_raise_exception(&cpu->env, cs->exception_index, GETPC());
+}
+
 void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
MMUAccessType access_type, int mmu_idx,
uintptr_t retaddr)


Reviewed-by: Palmer Dabbelt 

Re: [Qemu-devel] [PATCH v1 0/4] Miscellaneous patches from the RISC-V fork

[Palmer Dabbelt]

On Fri, 17 May 2019 15:10:56 PDT (-0700), Alistair Francis wrote:

This should be the last series bringing the patches from the RISC-V fork
into mainline QEMU.

Dayeol Lee (1):
  target/riscv: Fix PMP range boundary address bug

Michael Clark (3):
  disas/riscv: Disassemble reserved compressed encodings as illegal
  disas/riscv: Fix `rdinstreth` constraint
  target/riscv: Implement riscv_cpu_unassigned_access

 disas/riscv.c | 53 ++-
 target/riscv/cpu.c|  1 +
 target/riscv/cpu.h|  2 ++
 target/riscv/cpu_helper.c | 16 
 target/riscv/pmp.c|  2 +-
 5 files changed, 55 insertions(+), 19 deletions(-)


There's some minor issues with two of these, but since they're all independent
I'm going to take the other two right now.

Thanks!

[Qemu-devel] [PATCH v2 0/9] configure: Fix softmmu --static linking

[Philippe Mathieu-Daudé]

Hi,

Apparently QEMU static linking is slowly bitroting. Obviously it
depends the libraries an user has installed, anyway it seems there
are not much testing done.

This series fixes few issues, enough to build QEMU on a Ubuntu
18.04 host.

Peter commented on v1:

  The main reason for supporting static linking is so we can build
  the user-mode emulators. Almost always the problems with
  static linking the softmmu binaries and the tools are
  issues with the distro's packaging of the static libraries
  (pkg-config files which specify things that don't work for
  static is a common one).

  So we could put in a lot of checking of "is what pkg-config
  tells us broken". Or we could just say "we don't support static
  linking for anything except the usermode binaries". We
  should probably phase in deprecation of that because it's
  possible somebody's using it seriously, but it seems like
  a fairly weird thing to do to me.

I share his view on this (restricting static linking to qemu-user)
but since the work was already done when I read his comment, I still
send the v2.

Since v1:
- pkg-config already use the '--static' argument, do not add it twice
- Fixed x86_64 host builds (was missing GTK and OpenGL patches)
- Added Niels R-b tag on the first patch
- The Travis-CI job now succeeds:
  https://travis-ci.org/philmd/qemu/jobs/545653697 (6 min 7 sec)

Regards,

Phil.

Philippe Mathieu-Daudé (9):
  configure: Only generate GLUSTERFS variables if glusterfs is usable
  configure: Link test before auto-enabling GlusterFS libraries
  configure: Link test before auto-enabling libusb library
  configure: Link test before auto-enabling libusbredir library
  configure: Link test before auto-enabling PulseAudio library
  configure: Link test before auto-enabling OpenGL libraries
  configure: Link test before auto-enabling GTK libraries
  tests/docker: Kludge for missing libunistring.so symlink on Ubuntu
18.04
  .travis.yml: Test softmmu static linking

 .travis.yml|   5 +
 configure  | 121 -
 tests/docker/dockerfiles/ubuntu1804.docker |   4 +
 3 files changed, 100 insertions(+), 30 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH v2 4/9] configure: Link test before auto-enabling libusbredir library

[Philippe Mathieu-Daudé]

Similarly to commit a73e82ef912, test the library links correctly
before considering it as usable.

This fixes using ./configure --static on Ubuntu 18.04:

  $ make subdir-aarch64-softmmu
  [...]
LINKaarch64-softmmu/qemu-system-aarch64
  /usr/bin/ld: cannot find -lusbredirparser
  collect2: error: ld returned 1 exit status
  Makefile:204: recipe for target 'qemu-system-aarch64' failed
  make[1]: *** [qemu-system-aarch64] Error 1

  $ fgrep redir config-host.mak
  USB_REDIR_LIBS=-lusbredirparser

  $ lsb_release -cri
  Distributor ID: Ubuntu
  Release:18.04
  Codename:   bionic

Signed-off-by: Philippe Mathieu-Daudé 
---
 configure | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 2ebaa32746..0dd6e8bed3 100755
--- a/configure
+++ b/configure
@@ -4918,9 +4918,19 @@ fi
 # check for usbredirparser for usb network redirection support
 if test "$usb_redir" != "no" ; then
 if $pkg_config --atleast-version=0.6 libusbredirparser-0.5; then
-usb_redir="yes"
 usb_redir_cflags=$($pkg_config --cflags libusbredirparser-0.5)
 usb_redir_libs=$($pkg_config --libs libusbredirparser-0.5)
+# Packaging for the static libraries is not always correct.
+# At least ubuntu 18.04 ships only shared libraries.
+write_c_skeleton
+if ! compile_prog "$usb_redir_cflags" "$usb_redir_libs" ; then
+if test "$usb_redir" = "yes" ; then
+  error_exit "usbredir check failed."
+fi
+usb_redir="no"
+else
+usb_redir="yes"
+fi
 else
 if test "$usb_redir" = "yes"; then
 feature_not_found "usb-redir" "Install usbredir devel"
-- 
2.20.1

[Qemu-devel] [PATCH v2 1/9] configure: Only generate GLUSTERFS variables if glusterfs is usable

[Philippe Mathieu-Daudé]

It is pointless and confusing to have GLUSTERFS variables
in config-host.mak when glusterfs is not usable.

Reviewed-by: Niels de Vos 
Signed-off-by: Philippe Mathieu-Daudé 
---
 configure | 36 ++--
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/configure b/configure
index b091b82cb3..13fd4a1166 100755
--- a/configure
+++ b/configure
@@ -7118,30 +7118,30 @@ if test "$glusterfs" = "yes" ; then
   echo "CONFIG_GLUSTERFS=m" >> $config_host_mak
   echo "GLUSTERFS_CFLAGS=$glusterfs_cflags" >> $config_host_mak
   echo "GLUSTERFS_LIBS=$glusterfs_libs" >> $config_host_mak
-fi
 
-if test "$glusterfs_xlator_opt" = "yes" ; then
-  echo "CONFIG_GLUSTERFS_XLATOR_OPT=y" >> $config_host_mak
-fi
+  if test "$glusterfs_xlator_opt" = "yes" ; then
+echo "CONFIG_GLUSTERFS_XLATOR_OPT=y" >> $config_host_mak
+  fi
 
-if test "$glusterfs_discard" = "yes" ; then
-  echo "CONFIG_GLUSTERFS_DISCARD=y" >> $config_host_mak
-fi
+  if test "$glusterfs_discard" = "yes" ; then
+echo "CONFIG_GLUSTERFS_DISCARD=y" >> $config_host_mak
+  fi
 
-if test "$glusterfs_fallocate" = "yes" ; then
-  echo "CONFIG_GLUSTERFS_FALLOCATE=y" >> $config_host_mak
-fi
+  if test "$glusterfs_fallocate" = "yes" ; then
+echo "CONFIG_GLUSTERFS_FALLOCATE=y" >> $config_host_mak
+  fi
 
-if test "$glusterfs_zerofill" = "yes" ; then
-  echo "CONFIG_GLUSTERFS_ZEROFILL=y" >> $config_host_mak
-fi
+  if test "$glusterfs_zerofill" = "yes" ; then
+echo "CONFIG_GLUSTERFS_ZEROFILL=y" >> $config_host_mak
+  fi
 
-if test "$glusterfs_ftruncate_has_stat" = "yes" ; then
-  echo "CONFIG_GLUSTERFS_FTRUNCATE_HAS_STAT=y" >> $config_host_mak
-fi
+  if test "$glusterfs_ftruncate_has_stat" = "yes" ; then
+echo "CONFIG_GLUSTERFS_FTRUNCATE_HAS_STAT=y" >> $config_host_mak
+  fi
 
-if test "$glusterfs_iocb_has_stat" = "yes" ; then
-  echo "CONFIG_GLUSTERFS_IOCB_HAS_STAT=y" >> $config_host_mak
+  if test "$glusterfs_iocb_has_stat" = "yes" ; then
+echo "CONFIG_GLUSTERFS_IOCB_HAS_STAT=y" >> $config_host_mak
+  fi
 fi
 
 if test "$libssh2" = "yes" ; then
-- 
2.20.1

[Qemu-devel] [PATCH v2 5/9] configure: Link test before auto-enabling PulseAudio library

[Philippe Mathieu-Daudé]

Similarly to commit a73e82ef912, test the library links correctly
before considering it as usable.

This fixes using ./configure --static on Ubuntu 18.04:

  $ make subdir-aarch64-softmmu
  [...]
LINKaarch64-softmmu/qemu-system-aarch64
  /usr/bin/ld: cannot find -lpulse
  /usr/bin/ld: cannot find -lpulsecommon-11.1
  collect2: error: ld returned 1 exit status
  Makefile:204: recipe for target 'qemu-system-aarch64' failed
  make[1]: *** [qemu-system-aarch64] Error 1

  $ fgrep pulse config-host.mak
  PULSE_LIBS=-L/usr/lib/aarch64-linux-gnu/pulseaudio -lpulse -lpulsecommon-11.1

  $ lsb_release -cri
  Distributor ID: Ubuntu
  Release:18.04
  Codename:   bionic

Signed-off-by: Philippe Mathieu-Daudé 
---
 configure | 19 +++
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index 0dd6e8bed3..449dbd69ce 100755
--- a/configure
+++ b/configure
@@ -3408,10 +3408,21 @@ for drv in $audio_drv_list; do
 
 pa | try-pa)
 if $pkg_config libpulse --exists; then
-pulse_libs=$($pkg_config libpulse --libs)
-audio_pt_int="yes"
-if test "$drv" = "try-pa"; then
-audio_drv_list=$(echo "$audio_drv_list" | sed -e 's/try-pa/pa/')
+pulse_cflags=$($pkg_config --cflags libpulse)
+pulse_libs=$($pkg_config --libs libpulse)
+# Packaging for the static libraries is not always correct.
+# At least ubuntu 18.04 ships only shared libraries.
+write_c_skeleton
+if ! compile_prog "$pulse_cflags" "$pulse_libs" ; then
+unset pulse_cflags pulse_libs
+if test "$drv" = "try-pa"; then
+audio_drv_list=$(echo "$audio_drv_list" | sed -e 's/try-pa//')
+fi
+else
+audio_pt_int="yes"
+if test "$drv" = "try-pa"; then
+audio_drv_list=$(echo "$audio_drv_list" | sed -e 
's/try-pa/pa/')
+fi
 fi
 else
 if test "$drv" = "try-pa"; then
-- 
2.20.1

[Qemu-devel] [PATCH v2 2/9] configure: Link test before auto-enabling GlusterFS libraries

[Philippe Mathieu-Daudé]

Similarly to commit a73e82ef912, test the libraries link correctly
before considering them as usable.

This fixes using ./configure --static on Ubuntu 18.04:

  $ make subdir-aarch64-softmmu
  [...]
LINKaarch64-softmmu/qemu-system-aarch64
  /usr/bin/ld: cannot find -lgfapi
  /usr/bin/ld: cannot find -lglusterfs
  /usr/bin/ld: cannot find -lgfrpc
  /usr/bin/ld: cannot find -lgfxdr
  collect2: error: ld returned 1 exit status
  Makefile:204: recipe for target 'qemu-system-aarch64' failed
  make[1]: *** [qemu-system-aarch64] Error 1

  $ fgrep gf config-host.mak
  GLUSTERFS_LIBS=-lacl -lgfapi -lglusterfs -lgfrpc -lgfxdr -luuid

  $ lsb_release -cri
  Distributor ID: Ubuntu
  Release:18.04
  Codename:   bionic

Signed-off-by: Philippe Mathieu-Daudé 
---
 configure | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 13fd4a1166..fe0e2e1b75 100755
--- a/configure
+++ b/configure
@@ -4179,9 +4179,19 @@ fi
 # glusterfs probe
 if test "$glusterfs" != "no" ; then
   if $pkg_config --atleast-version=3 glusterfs-api; then
-glusterfs="yes"
 glusterfs_cflags=$($pkg_config --cflags glusterfs-api)
 glusterfs_libs=$($pkg_config --libs glusterfs-api)
+# Packaging for the static libraries is not always correct.
+# At least ubuntu 18.04 ships only shared libraries.
+write_c_skeleton
+if ! compile_prog "$glusterfs_cflags" "$glusterfs_libs" ; then
+if test "$glusterfs" = "yes" ; then
+  error_exit "glusterfs check failed."
+fi
+glusterfs="no"
+else
+glusterfs="yes"
+fi
 if $pkg_config --atleast-version=4 glusterfs-api; then
   glusterfs_xlator_opt="yes"
 fi
-- 
2.20.1

[Qemu-devel] [PATCH v2 3/9] configure: Link test before auto-enabling libusb library

[Philippe Mathieu-Daudé]

Similarly to commit a73e82ef912, test the library links correctly
before considering it as usable.

This fixes using ./configure --static on Ubuntu 18.04:

  $ make subdir-aarch64-softmmu
  [...]
LINKaarch64-softmmu/qemu-system-aarch64
  /usr/bin/ld: cannot find -ludev
  collect2: error: ld returned 1 exit status
  Makefile:204: recipe for target 'qemu-system-aarch64' failed
  make[1]: *** [qemu-system-aarch64] Error 1

  $ fgrep udev config-host.mak
  LIBUSB_LIBS=-lusb-1.0 -ludev -pthread

  $ lsb_release -cri
  Distributor ID: Ubuntu
  Release:18.04
  Codename:   bionic

Signed-off-by: Philippe Mathieu-Daudé 
---
 configure | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index fe0e2e1b75..2ebaa32746 100755
--- a/configure
+++ b/configure
@@ -4894,9 +4894,19 @@ fi
 # check for libusb
 if test "$libusb" != "no" ; then
 if $pkg_config --atleast-version=1.0.13 libusb-1.0; then
-libusb="yes"
 libusb_cflags=$($pkg_config --cflags libusb-1.0)
 libusb_libs=$($pkg_config --libs libusb-1.0)
+# Packaging for the static libraries is not always correct.
+# At least ubuntu 18.04 ships only shared libraries.
+write_c_skeleton
+if ! compile_prog "$libusb_cflags" "$libusb_libs" ; then
+if test "$libusb" = "yes" ; then
+  error_exit "libusb check failed."
+fi
+libusb="no"
+else
+libusb="yes"
+fi
 else
 if test "$libusb" = "yes"; then
 feature_not_found "libusb" "Install libusb devel >= 1.0.13"
-- 
2.20.1

[Qemu-devel] [PATCH v2 8/9] tests/docker: Kludge for missing libunistring.so symlink on Ubuntu 18.04

[Philippe Mathieu-Daudé]

When linking statically on Ubuntu 18.04 we get:

  $ make subdir-x86_64-softmmu
  [...]
 LINKx86_64-softmmu/qemu-system-x86_64
  c++: error: /usr/lib/x86_64-linux-gnu/libunistring.so: No such file or 
directory

This library is pulled in by GTK:

  $ pkg-config --libs --static gtk+-3.0
  -lgtk-3 -latk-bridge-2.0 -latspi -ldbus-1 -lpthread -lsystemd -lgdk-3 
-lgio-2.0 -lXinerama -lXi -lXrandr -lXcursor -lXcomposite -lXdamage -lXfixes 
-lxkbcommon -lwayland-cursor -lwayland-egl -lwayland-client -lepoxy -ldl 
-lpangocairo-1.0 -lpangoft2-1.0 -lharfbuzz -lm -lgraphite2 -lpango-1.0 -lm 
-latk-1.0 -lcairo-gobject -lcairo -lz -lpixman-1 -lfontconfig -lexpat 
-lfreetype -lexpat -lfreetype -lpng16 -lm -lz -lm -lxcb-shm -lxcb-render 
-lXrender -lXext -lX11 -lpthread -lxcb -lXau -lXdmcp -lgdk_pixbuf-2.0 -lm 
-lpng16 -lm -lz -lm -lz -lgio-2.0 -lz -lresolv -lselinux -lmount -lgmodule-2.0 
-pthread -ldl -lgobject-2.0 -lffi -lglib-2.0 -pthread -lpcre -pthread

However, while the library is presentm, its symlink is missing:

  $ ls -ld /usr/lib/x86_64-linux-gnu/libunistring.so
  ls: cannot access '/usr/lib/x86_64-linux-gnu/libunistring.so': No such file 
or directory

  $ ls -ld /usr/lib/x86_64-linux-gnu/libunistring.so*
  lrwxrwxrwx. 1 root root  21 Mar  3  2018 
/usr/lib/x86_64-linux-gnu/libunistring.so.2 -> libunistring.so.2.1.0
  -rw-r--r--. 1 root root 1562664 Mar  3  2018 
/usr/lib/x86_64-linux-gnu/libunistring.so.2.1.0

Fix the issue by creating the missing symlink manually.

Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/docker/dockerfiles/ubuntu1804.docker | 4 
 1 file changed, 4 insertions(+)

diff --git a/tests/docker/dockerfiles/ubuntu1804.docker 
b/tests/docker/dockerfiles/ubuntu1804.docker
index 2e2900150b..7e45c52166 100644
--- a/tests/docker/dockerfiles/ubuntu1804.docker
+++ b/tests/docker/dockerfiles/ubuntu1804.docker
@@ -54,4 +54,8 @@ ENV PACKAGES flex bison \
 RUN apt-get update && \
 apt-get -y install $PACKAGES
 RUN dpkg -l $PACKAGES | sort > /packages.txt
+# The libunistring2 package does not create a symlink to libunistring.so
+# Create it manually to fix:
+# error: /usr/lib/x86_64-linux-gnu/libunistring.so: No such file or directory
+RUN ln -s libunistring.so.2 /usr/lib/x86_64-linux-gnu/libunistring.so
 ENV FEATURES clang pyyaml sdl2
-- 
2.20.1

[Qemu-devel] [PATCH v2 6/9] configure: Link test before auto-enabling OpenGL libraries

[Philippe Mathieu-Daudé]

Similarly to commit a73e82ef912, test the libraries link correctly
before considering them as usable.

This fixes using ./configure --static on Ubuntu 18.04:

  $ make subdir-lm32-softmmu
  [...]
  LINKlm32-softmmu/qemu-system-lm32
  /usr/bin/ld: cannot find -lepoxy
  /usr/bin/ld: cannot find -lgbm
  collect2: error: ld returned 1 exit status
  Makefile:204: recipe for target 'qemu-system-lm32' failed
  make[1]: *** [qemu-system-lm32] Error 1

  $ fgrep epoxy config-host.mak
  OPENGL_LIBS=-lepoxy -ldl -lgbm -ldl

  $ lsb_release -cri
  Distributor ID: Ubuntu
  Release:18.04
  Codename:   bionic

Signed-off-by: Philippe Mathieu-Daudé 
---
 configure | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index 449dbd69ce..a3da5adf80 100755
--- a/configure
+++ b/configure
@@ -4133,11 +4133,21 @@ if test "$opengl" != "no" ; then
   if $pkg_config $opengl_pkgs; then
 opengl_cflags="$($pkg_config --cflags $opengl_pkgs)"
 opengl_libs="$($pkg_config --libs $opengl_pkgs)"
-opengl=yes
-if test "$gtk" = "yes" && $pkg_config --exists "$gtkpackage >= 3.16"; then
-gtk_gl="yes"
+# Packaging for the static libraries is not always correct.
+# At least ubuntu 18.04 ships only shared libraries.
+write_c_skeleton
+if ! compile_prog "$opengl_cflags" "$opengl_libs" ; then
+if test "$opengl" = "yes" ; then
+  error_exit "opengl check failed."
+fi
+opengl=no
+else
+opengl=yes
+if test "$gtk" = "yes" && $pkg_config --exists "$gtkpackage >= 3.16"; 
then
+gtk_gl="yes"
+fi
+QEMU_CFLAGS="$QEMU_CFLAGS $opengl_cflags"
 fi
-QEMU_CFLAGS="$QEMU_CFLAGS $opengl_cflags"
   else
 if test "$opengl" = "yes" ; then
   feature_not_found "opengl" "Please install opengl (mesa) devel pkgs: 
$opengl_pkgs"
-- 
2.20.1

[Qemu-devel] [PATCH v2 7/9] configure: Link test before auto-enabling GTK libraries

[Philippe Mathieu-Daudé]

Similarly to commit a73e82ef912, test the libraries link correctly
before considering them as usable.

This fixes using ./configure --static on Ubuntu 18.04:

  $ make subdir-lm32-softmmu
  [...]
  LINKlm32-softmmu/qemu-system-lm32
  /usr/bin/ld: cannot find -lgtk-3
  /usr/bin/ld: cannot find -latk-bridge-2.0
  /usr/bin/ld: cannot find -latspi
  /usr/bin/ld: cannot find -lsystemd
  /usr/bin/ld: cannot find -lgdk-3
  /usr/bin/ld: cannot find -lwayland-cursor
  /usr/bin/ld: cannot find -lwayland-egl
  /usr/bin/ld: cannot find -lwayland-client
  /usr/bin/ld: cannot find -lepoxy
  /usr/bin/ld: cannot find -lgraphite2
  collect2: error: ld returned 1 exit status
  Makefile:204: recipe for target 'qemu-system-lm32' failed
  make[1]: *** [qemu-system-lm32] Error 1

  $ fgrep gdk config-host.mak
  GTK_LIBS=-lgtk-3 -latk-bridge-2.0 -latspi -ldbus-1 -lpthread -lsystemd 
-lgdk-3 -lgio-2.0 -lXinerama -lXi -lXrandr -lXcursor -lXcomposite -lXdamage 
-lXfixes -lxkbcommon -lwayland-cursor -lwayland-egl -lwayland-client -lepoxy 
-ldl -lpangocairo-1.0 -lpangoft2-1.0 -lharfbuzz -lm -lgraphite2 -lpango-1.0 -lm 
-latk-1.0 -lcairo-gobject -lcairo -lz -lpixman-1 -lfontconfig -lexpat 
-lfreetype -lexpat -lfreetype -lpng16 -lm -lz -lm -lxcb-shm -lxcb-render 
-lXrender -lXext -lX11 -lpthread -lxcb -lXau -lXdmcp -lgdk_pixbuf-2.0 -lm 
-lpng16 -lm -lz -lm -lz -lgio-2.0 -lz -lresolv -lselinux -lmount -lgmodule-2.0 
-pthread -ldl -lgobject-2.0 -lffi -lglib-2.0 -pthread -lpcre -pthread -lX11 
-lpthread -lxcb -lXau -lXdmcp
VTE_CFLAGS=-pthread -I/usr/include/vte-2.91 -I/usr/include/gtk-3.0 
-I/usr/include/at-spi2-atk/2.0 -I/usr/include/at-spi-2.0 
-I/usr/include/dbus-1.0 -I/usr/lib/x86_64-linux-gnu/dbus-1.0/include 
-I/usr/include/gtk-3.0 -I/usr/include/cairo -I/usr/include/pango-1.0 
-I/usr/include/harfbuzz -I/usr/include/pango-1.0 -I/usr/include/atk-1.0 
-I/usr/include/cairo -I/usr/include/pixman-1 -I/usr/include/freetype2 
-I/usr/include/libpng16 -I/usr/include/freetype2 -I/usr/include/libpng16 
-I/usr/include/gdk-pixbuf-2.0 -I/usr/include/libpng16 
-I/usr/include/gio-unix-2.0/ -I/usr/include/glib-2.0 
-I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I/usr/include/p11-kit-1

  $ lsb_release -cri
  Distributor ID: Ubuntu
  Release:18.04
  Codename:   bionic

Signed-off-by: Philippe Mathieu-Daudé 
---
 configure | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index a3da5adf80..ffd269b34f 100755
--- a/configure
+++ b/configure
@@ -2782,7 +2782,17 @@ if test "$gtk" != "no"; then
 gtk_cflags="$gtk_cflags $x11_cflags"
 gtk_libs="$gtk_libs $x11_libs"
 fi
-gtk="yes"
+# Packaging for the static libraries is not always correct.
+# At least ubuntu 18.04 ships only shared libraries.
+write_c_skeleton
+if ! compile_prog "$gtk_cflags" "$gtk_libs" ; then
+if test "$gtk" = "yes" ; then
+  error_exit "gtk check failed."
+fi
+gtk="no"
+else
+gtk="yes"
+fi
 elif test "$gtk" = "yes"; then
 feature_not_found "gtk" "Install gtk3-devel"
 else
-- 
2.20.1

Re: [Qemu-devel] [PATCH] block/replication.c: Fix crash issue after failover

[no-reply]

Patchew URL: 
https://patchew.org/QEMU/20190614092853.26551-1-chen.zh...@intel.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

PASS 32 test-opts-visitor /visitor/opts/range/beyond
PASS 33 test-opts-visitor /visitor/opts/dict/unvisited
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-coroutine -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-coroutine" 
==7787==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
==7787==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 
0x7ffef4e8; bottom 0x7fbf709f8000; size: 0x003f84488000 (272802283520)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-coroutine /basic/no-dangling-access
---
PASS 1 fdc-test /x86_64/fdc/cmos
PASS 2 fdc-test /x86_64/fdc/no_media_on_start
PASS 3 fdc-test /x86_64/fdc/read_without_media
==7804==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 4 fdc-test /x86_64/fdc/media_change
PASS 5 fdc-test /x86_64/fdc/sense_interrupt
PASS 6 fdc-test /x86_64/fdc/relative_seek
---
PASS 11 test-aio /aio/event/wait
PASS 12 test-aio /aio/event/flush
PASS 13 test-aio /aio/event/wait/no-flush-cb
==7812==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 14 test-aio /aio/timer/schedule
PASS 15 test-aio /aio/coroutine/queue-chaining
PASS 16 test-aio /aio-gsource/flush
---
PASS 11 fdc-test /x86_64/fdc/read_no_dma_18
PASS 28 test-aio /aio-gsource/timer/schedule
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-aio-multithread -m=quick -k --tap < /dev/null | 
./scripts/tap-driver.pl --test-name="test-aio-multithread" 
==7818==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 test-aio-multithread /aio/multi/lifecycle
PASS 2 test-aio-multithread /aio/multi/schedule
PASS 3 test-aio-multithread /aio/multi/mutex/contended
---
PASS 4 test-aio-multithread /aio/multi/mutex/handoff
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img 
tests/ide-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="ide-test" 
PASS 5 test-aio-multithread /aio/multi/mutex/mcs
==7852==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 ide-test /x86_64/ide/identify
PASS 6 test-aio-multithread /aio/multi/mutex/pthread
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-throttle -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-throttle" 
==7867==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
==7863==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 1 test-throttle /throttle/leak_bucket
PASS 2 test-throttle /throttle/compute_wait
PASS 3 test-throttle /throttle/init
---
PASS 14 test-throttle /throttle/config/max
PASS 15 test-throttle /throttle/config/iops_size
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-thread-pool -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl 
--test-name="test-thread-pool" 
==7876==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 2 ide-test /x86_64/ide/flush
PASS 1 test-thread-pool /thread-pool/submit
PASS 2 test-thread-pool /thread-pool/submit-aio
PASS 3 test-thread-pool /thread-pool/submit-co
PASS 4 test-thread-pool /thread-pool/submit-many
==7943==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 3 ide-test /x86_64/ide/bmdma/simple_rw
==7949==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 5 test-thread-pool /thread-pool/cancel
PASS 4 ide-test /x86_64/ide/bmdma/trim
==7955==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
PASS 5 ide-test /x86_64/ide/bmdma/short_prdt
PASS 6 test-thread-pool /thread-pool/cancel-async
==7961==WARNING: ASan doesn't fully support makecontext/swapcontext functions 
and may produce false positives in some cases!
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  
tests/test-hbit

Re: [Qemu-devel] [PATCH] block/replication.c: Fix crash issue after failover

[Kevin Wolf]

Am 14.06.2019 um 11:28 hat Zhang Chen geschrieben:
> From: Zhang Chen 
> 
> No block job on active disk after failover.
> In the replication_stop() function have canceled the block job,
> we check it again here.
> 
> Signed-off-by: Zhang Chen 
> ---
>  block/replication.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/block/replication.c b/block/replication.c
> index 3d4dedddfc..bdf2bf4bbc 100644
> --- a/block/replication.c
> +++ b/block/replication.c
> @@ -146,7 +146,9 @@ static void replication_close(BlockDriverState *bs)
>  replication_stop(s->rs, false, NULL);
>  }
>  if (s->stage == BLOCK_REPLICATION_FAILOVER) {
> -job_cancel_sync(&s->active_disk->bs->job->job);
> +if (s->secondary_disk->bs->job) {
> +job_cancel_sync(&s->secondary_disk->bs->job->job);
> +}

Why are you changing the code from active_disk to secondary_disk?

Also, please rebase on top of Vladimir's '[PATCH 0/4] block: drop
bs->job'.

Kevin

Re: [Qemu-devel] [PATCH v6] ssh: switch from libssh2 to libssh

[Philippe Mathieu-Daudé]

On 6/14/19 11:42 AM, Kevin Wolf wrote:
> Am 13.06.2019 um 21:41 hat Stefan Weil geschrieben:
>> On 12.06.19 15:27, Philippe Mathieu-Daudé wrote:
>>> Cc'ing Alex (Docker, Travis) and Stefan (MinGW)
>> [...]
>>> Note, libssh is not available on MinGW.
>>
>> Nor is it available for Mingw64:

Yes, by "MinGW" I mean both 32/64 implementations.

>>
>> https://cygwin.com/cgi-bin2/package-grep.cgi?grep=mingw64-x86_64-libssh&arch=x86_64
>>
>> That makes building for Windows more difficult because there is an
>> additional dependency which must be built from source.
> 
> How many people do actually use the ssh block driver on Windows, though?
> Isn't just building QEMU without it a quite reasonable option, too?
> 
> I wouldn't consider this a strong argument why we should keep using an
> obsolete library.

I agree with Kevin. The only user of the 'ssh' block driver that I am
aware of is the virt-v2v tool:

http://libguestfs.org/virt-v2v.1.html#convert-from-esxi-hypervisor-over-ssh-to-local-libvirt

Stefan, do you think someone would use it on a Windows host?

Thanks,

Phil.

Re: [Qemu-devel] [PATCH 0/4] block: drop bs->job

[Kevin Wolf]

Am 06.06.2019 um 17:41 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Hi all!
> 
> These series follow Kevin's suggestions under 
> https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg00670.html
> [Qemu-devel] [PATCH v2 0/2] introduce pinned blk
> (hope you don't mind me using exactly your wording in 02)

Thanks, applied to the block branch.

Kevin

Re: [Qemu-devel] [PATCH] roms/edk2-build.sh: Allow to run edk2-build.sh from command line

[Philippe Mathieu-Daudé]

Cc'ing Eric :)

On 6/13/19 7:54 PM, Philippe Mathieu-Daudé wrote:
> The edk2-build.sh script set the 'nounset' option:
> 
>   BASH(1)
> 
>   set [arg ...]
> 
>   -u   Treat unset variables and parameters other than the
>special parameters "@" and "*" as an error when
>performing parameter expansion.  If expansion is
>attempted on an unset variable or parameter, the shell
>prints an error message, and, if not interactive,
>exits with a non-zero status.
> 
> When running this script out of 'make', we get:
> 
>   $ cd roms
>   $ ./edk2-build.sh aarch64 --arch=AARCH64 
> --platform=ArmVirtPkg/ArmVirtQemu.dsc > /dev/null
>   ./edk2-build.sh: line 46: MAKEFLAGS: unbound variable
> 
> Fix this by checking the variable is defined before using it,
> else use a default value.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  roms/edk2-build.sh | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/roms/edk2-build.sh b/roms/edk2-build.sh
> index 4f46f8a6a2..5390228b4e 100755
> --- a/roms/edk2-build.sh
> +++ b/roms/edk2-build.sh
> @@ -43,7 +43,13 @@ fi
>  # any), for the edk2 "build" utility.
>  source ../edk2-funcs.sh
>  edk2_toolchain=$(qemu_edk2_get_toolchain "$emulation_target")
> -edk2_thread_count=$(qemu_edk2_get_thread_count "$MAKEFLAGS")
> +if [ -v MAKEFLAGS ]; then
> +  edk2_thread_count=$(qemu_edk2_get_thread_count "$MAKEFLAGS")
> +else
> +  # We are not running within 'make', let the edk2 "build" utility to fetch
> +  # the logical CPU count with Python's multiprocessing.cpu_count() method.
> +  edk2_thread_count=0
> +fi
>  qemu_edk2_set_cross_env "$emulation_target"
>  
>  # Build the platform firmware.
> 

Re: [Qemu-devel] [PATCH qemu] loader: Trace loaded images

[Philippe Mathieu-Daudé]

On 6/14/19 11:33 AM, Stefan Hajnoczi wrote:
> On Fri, Jun 14, 2019 at 10:13:04AM +1000, Alexey Kardashevskiy wrote:
>>
>>
>> On 13/06/2019 23:08, Philippe Mathieu-Daudé wrote:
>>> Hi Alexey,
>>>
>>> On 6/13/19 7:09 AM, Alexey Kardashevskiy wrote:
 This adds a trace point which prints every loaded image. This includes
 bios/firmware/kernel/initradmdisk/pcirom.

 Signed-off-by: Alexey Kardashevskiy 
 ---

 The example for a pseries guest:

 loader_write_rom slof.bin: @0x0 size=0xe22e0 ROM=0
 loader_write_rom phdr #0: /home/aik/t/vml4120le: @0x40 size=0x13df000 
 ROM=0
 loader_write_rom /home/aik/t/le.cpio: @0x1ad size=0x9463a00 ROM=0
>>>
>>> I find the "ROM=0" part confuse, maybe you can change to "ROM:false".
>>
>> How? I mean I can do that in the code as rom->isrom?"true":"false" and
>> make trace point accept "%s" but it is quite ugly and others seem to
>> just use %d for bool.
> 
> Yes, %d is the convention for bool.  Perhaps you can name it "is_rom"
> instead of "ROM".  That way the name communicates that this is a boolean
> value.

I agree a boolean is easier to parse for trace analyzer tools than a
"true/false" string.

Stefan, is it possible to add a boolean format string to the backends?
For example the 'log' backend would log it as "true"/"false".

Just a thought ;)

Regards,

Phil.



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v2 9/9] .travis.yml: Test softmmu static linking

[Philippe Mathieu-Daudé]

Add a test to avoid the ./configure script to bitrot.

Signed-off-by: Philippe Mathieu-Daudé 
---
 .travis.yml | 5 +
 1 file changed, 5 insertions(+)

diff --git a/.travis.yml b/.travis.yml
index 08502c0aa2..6962fff826 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -92,6 +92,11 @@ matrix:
 - CONFIG="--disable-user --target-list-exclude=${MAIN_SOFTMMU_TARGETS}"
 
 
+# Test static linking
+- env:
+- CONFIG="--static --target-list=lm32-softmmu"
+
+
 # Just build tools and run minimal unit and softfloat checks
 - env:
 - BASE_CONFIG="--enable-tools"
-- 
2.20.1

[Qemu-devel] [PATCH 0/2] target/arm: Support single-precision only FPUs

[Peter Maydell]

The Arm architecture permits FPUs which have only single-precision
support, not double-precision; Cortex-M4 and Cortex-M33 are
both like that. Now that we've refactored the VFP code to use
decodetree it's fairly easy to add the necessary checks on the
MVFR0 FPDP field so that we UNDEF any double-precision instructions
on CPUs like this.

The first patch fixes some no-visible-effect typos in the
names of struct arguments to some functions (caused by
cut-n-paste errors); not really related but I noticed them
while I was working on this.

thanks
-- PMM

Peter Maydell (2):
  target/arm: Fix typos in trans function prototypes
  target/arm: Only implement doubles if the FPU supports them

 target/arm/cpu.h   |   6 ++
 target/arm/translate-vfp.inc.c | 112 -
 2 files changed, 104 insertions(+), 14 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH 1/2] target/arm: Fix typos in trans function prototypes

[Peter Maydell]

In several places cut and paste errors meant we were using the wrong
type for the 'arg' struct in trans_ functions called by the
decodetree decoder, because we were using the _sp version of the
struct in the _dp function.  These were harmless, because the two
structs were identical and so decodetree made them typedefs of the
same underlying structure (and we'd have had a compile error if they
were not harmless), but we should clean them up anyway.

Signed-off-by: Peter Maydell 
---
 target/arm/translate-vfp.inc.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/target/arm/translate-vfp.inc.c b/target/arm/translate-vfp.inc.c
index 709fc65374d..85187bcc9dc 100644
--- a/target/arm/translate-vfp.inc.c
+++ b/target/arm/translate-vfp.inc.c
@@ -835,7 +835,7 @@ static bool trans_VMOV_64_sp(DisasContext *s, 
arg_VMOV_64_sp *a)
 return true;
 }
 
-static bool trans_VMOV_64_dp(DisasContext *s, arg_VMOV_64_sp *a)
+static bool trans_VMOV_64_dp(DisasContext *s, arg_VMOV_64_dp *a)
 {
 TCGv_i32 tmp;
 
@@ -910,7 +910,7 @@ static bool trans_VLDR_VSTR_sp(DisasContext *s, 
arg_VLDR_VSTR_sp *a)
 return true;
 }
 
-static bool trans_VLDR_VSTR_dp(DisasContext *s, arg_VLDR_VSTR_sp *a)
+static bool trans_VLDR_VSTR_dp(DisasContext *s, arg_VLDR_VSTR_dp *a)
 {
 uint32_t offset;
 TCGv_i32 addr;
@@ -1500,7 +1500,7 @@ static void gen_VMLA_dp(TCGv_i64 vd, TCGv_i64 vn, 
TCGv_i64 vm, TCGv_ptr fpst)
 tcg_temp_free_i64(tmp);
 }
 
-static bool trans_VMLA_dp(DisasContext *s, arg_VMLA_sp *a)
+static bool trans_VMLA_dp(DisasContext *s, arg_VMLA_dp *a)
 {
 return do_vfp_3op_dp(s, gen_VMLA_dp, a->vd, a->vn, a->vm, true);
 }
@@ -1538,7 +1538,7 @@ static void gen_VMLS_dp(TCGv_i64 vd, TCGv_i64 vn, 
TCGv_i64 vm, TCGv_ptr fpst)
 tcg_temp_free_i64(tmp);
 }
 
-static bool trans_VMLS_dp(DisasContext *s, arg_VMLS_sp *a)
+static bool trans_VMLS_dp(DisasContext *s, arg_VMLS_dp *a)
 {
 return do_vfp_3op_dp(s, gen_VMLS_dp, a->vd, a->vn, a->vm, true);
 }
@@ -1580,7 +1580,7 @@ static void gen_VNMLS_dp(TCGv_i64 vd, TCGv_i64 vn, 
TCGv_i64 vm, TCGv_ptr fpst)
 tcg_temp_free_i64(tmp);
 }
 
-static bool trans_VNMLS_dp(DisasContext *s, arg_VNMLS_sp *a)
+static bool trans_VNMLS_dp(DisasContext *s, arg_VNMLS_dp *a)
 {
 return do_vfp_3op_dp(s, gen_VNMLS_dp, a->vd, a->vn, a->vm, true);
 }
@@ -1614,7 +1614,7 @@ static void gen_VNMLA_dp(TCGv_i64 vd, TCGv_i64 vn, 
TCGv_i64 vm, TCGv_ptr fpst)
 tcg_temp_free_i64(tmp);
 }
 
-static bool trans_VNMLA_dp(DisasContext *s, arg_VNMLA_sp *a)
+static bool trans_VNMLA_dp(DisasContext *s, arg_VNMLA_dp *a)
 {
 return do_vfp_3op_dp(s, gen_VNMLA_dp, a->vd, a->vn, a->vm, true);
 }
@@ -1624,7 +1624,7 @@ static bool trans_VMUL_sp(DisasContext *s, arg_VMUL_sp *a)
 return do_vfp_3op_sp(s, gen_helper_vfp_muls, a->vd, a->vn, a->vm, false);
 }
 
-static bool trans_VMUL_dp(DisasContext *s, arg_VMUL_sp *a)
+static bool trans_VMUL_dp(DisasContext *s, arg_VMUL_dp *a)
 {
 return do_vfp_3op_dp(s, gen_helper_vfp_muld, a->vd, a->vn, a->vm, false);
 }
@@ -1648,7 +1648,7 @@ static void gen_VNMUL_dp(TCGv_i64 vd, TCGv_i64 vn, 
TCGv_i64 vm, TCGv_ptr fpst)
 gen_helper_vfp_negd(vd, vd);
 }
 
-static bool trans_VNMUL_dp(DisasContext *s, arg_VNMUL_sp *a)
+static bool trans_VNMUL_dp(DisasContext *s, arg_VNMUL_dp *a)
 {
 return do_vfp_3op_dp(s, gen_VNMUL_dp, a->vd, a->vn, a->vm, false);
 }
@@ -1658,7 +1658,7 @@ static bool trans_VADD_sp(DisasContext *s, arg_VADD_sp *a)
 return do_vfp_3op_sp(s, gen_helper_vfp_adds, a->vd, a->vn, a->vm, false);
 }
 
-static bool trans_VADD_dp(DisasContext *s, arg_VADD_sp *a)
+static bool trans_VADD_dp(DisasContext *s, arg_VADD_dp *a)
 {
 return do_vfp_3op_dp(s, gen_helper_vfp_addd, a->vd, a->vn, a->vm, false);
 }
@@ -1668,7 +1668,7 @@ static bool trans_VSUB_sp(DisasContext *s, arg_VSUB_sp *a)
 return do_vfp_3op_sp(s, gen_helper_vfp_subs, a->vd, a->vn, a->vm, false);
 }
 
-static bool trans_VSUB_dp(DisasContext *s, arg_VSUB_sp *a)
+static bool trans_VSUB_dp(DisasContext *s, arg_VSUB_dp *a)
 {
 return do_vfp_3op_dp(s, gen_helper_vfp_subd, a->vd, a->vn, a->vm, false);
 }
@@ -1678,7 +1678,7 @@ static bool trans_VDIV_sp(DisasContext *s, arg_VDIV_sp *a)
 return do_vfp_3op_sp(s, gen_helper_vfp_divs, a->vd, a->vn, a->vm, false);
 }
 
-static bool trans_VDIV_dp(DisasContext *s, arg_VDIV_sp *a)
+static bool trans_VDIV_dp(DisasContext *s, arg_VDIV_dp *a)
 {
 return do_vfp_3op_dp(s, gen_helper_vfp_divd, a->vd, a->vn, a->vm, false);
 }
@@ -1741,7 +1741,7 @@ static bool trans_VFM_sp(DisasContext *s, arg_VFM_sp *a)
 return true;
 }
 
-static bool trans_VFM_dp(DisasContext *s, arg_VFM_sp *a)
+static bool trans_VFM_dp(DisasContext *s, arg_VFM_dp *a)
 {
 /*
  * VFNMA : fd = muladd(-fd,  fn, fm)
@@ -2201,7 +2201,7 @@ static bool trans_VRINTR_sp(DisasContext *s, 
arg_VRINTR_sp *a)
 return true;
 }
 
-static bool trans_VRINTR_dp(DisasContext *s, arg_VRINTR_sp *a)
+static bool trans_VRINTR_dp(DisasConte

[Qemu-devel] [PATCH 2/2] target/arm: Only implement doubles if the FPU supports them

[Peter Maydell]

The architecture permits FPUs which have only single-precision
support, not double-precision; Cortex-M4 and Cortex-M33 are
both like that. Add the necessary checks on the MVFR0 FPDP
field so that we UNDEF any double-precision instructions on
CPUs like this.

Note that even if FPDP==0 the insns like VMOV-to/from-gpreg,
VLDM/VSTM, VLDR/VSTR which take double precision registers
still exist.

Signed-off-by: Peter Maydell 
---
 target/arm/cpu.h   |  6 +++
 target/arm/translate-vfp.inc.c | 84 ++
 2 files changed, 90 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 92298624215..29be1f7ea97 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3382,6 +3382,12 @@ static inline bool isar_feature_aa32_fpshvec(const 
ARMISARegisters *id)
 return FIELD_EX64(id->mvfr0, MVFR0, FPSHVEC) > 0;
 }
 
+static inline bool isar_feature_aa32_fpdp(const ARMISARegisters *id)
+{
+/* Return true if CPU supports double precision floating point */
+return FIELD_EX64(id->mvfr0, MVFR0, FPDP) > 0;
+}
+
 /*
  * We always set the FP and SIMD FP16 fields to indicate identical
  * levels of support (assuming SIMD is implemented at all), so
diff --git a/target/arm/translate-vfp.inc.c b/target/arm/translate-vfp.inc.c
index 85187bcc9dc..a3df81d3b07 100644
--- a/target/arm/translate-vfp.inc.c
+++ b/target/arm/translate-vfp.inc.c
@@ -173,6 +173,11 @@ static bool trans_VSEL(DisasContext *s, arg_VSEL *a)
 ((a->vm | a->vn | a->vd) & 0x10)) {
 return false;
 }
+
+if (dp && !dc_isar_feature(aa32_fpdp, s)) {
+return false;
+}
+
 rd = a->vd;
 rn = a->vn;
 rm = a->vm;
@@ -301,6 +306,11 @@ static bool trans_VMINMAXNM(DisasContext *s, arg_VMINMAXNM 
*a)
 ((a->vm | a->vn | a->vd) & 0x10)) {
 return false;
 }
+
+if (dp && !dc_isar_feature(aa32_fpdp, s)) {
+return false;
+}
+
 rd = a->vd;
 rn = a->vn;
 rm = a->vm;
@@ -382,6 +392,11 @@ static bool trans_VRINT(DisasContext *s, arg_VRINT *a)
 ((a->vm | a->vd) & 0x10)) {
 return false;
 }
+
+if (dp && !dc_isar_feature(aa32_fpdp, s)) {
+return false;
+}
+
 rd = a->vd;
 rm = a->vm;
 
@@ -440,6 +455,11 @@ static bool trans_VCVT(DisasContext *s, arg_VCVT *a)
 if (dp && !dc_isar_feature(aa32_fp_d32, s) && (a->vm & 0x10)) {
 return false;
 }
+
+if (dp && !dc_isar_feature(aa32_fpdp, s)) {
+return false;
+}
+
 rd = a->vd;
 rm = a->vm;
 
@@ -1268,6 +1288,10 @@ static bool do_vfp_3op_dp(DisasContext *s, VFPGen3OpDPFn 
*fn,
 return false;
 }
 
+if (!dc_isar_feature(aa32_fpdp, s)) {
+return false;
+}
+
 if (!dc_isar_feature(aa32_fpshvec, s) &&
 (veclen != 0 || s->vec_stride != 0)) {
 return false;
@@ -1413,6 +1437,10 @@ static bool do_vfp_2op_dp(DisasContext *s, VFPGen2OpDPFn 
*fn, int vd, int vm)
 return false;
 }
 
+if (!dc_isar_feature(aa32_fpdp, s)) {
+return false;
+}
+
 if (!dc_isar_feature(aa32_fpshvec, s) &&
 (veclen != 0 || s->vec_stride != 0)) {
 return false;
@@ -1773,6 +1801,10 @@ static bool trans_VFM_dp(DisasContext *s, arg_VFM_dp *a)
 return false;
 }
 
+if (!dc_isar_feature(aa32_fpdp, s)) {
+return false;
+}
+
 if (!vfp_access_check(s)) {
 return true;
 }
@@ -1878,6 +1910,10 @@ static bool trans_VMOV_imm_dp(DisasContext *s, 
arg_VMOV_imm_dp *a)
 return false;
 }
 
+if (!dc_isar_feature(aa32_fpdp, s)) {
+return false;
+}
+
 if (!dc_isar_feature(aa32_fpshvec, s) &&
 (veclen != 0 || s->vec_stride != 0)) {
 return false;
@@ -2028,6 +2064,10 @@ static bool trans_VCMP_dp(DisasContext *s, arg_VCMP_dp 
*a)
 return false;
 }
 
+if (!dc_isar_feature(aa32_fpdp, s)) {
+return false;
+}
+
 if (!vfp_access_check(s)) {
 return true;
 }
@@ -2097,6 +2137,10 @@ static bool trans_VCVT_f64_f16(DisasContext *s, 
arg_VCVT_f64_f16 *a)
 return false;
 }
 
+if (!dc_isar_feature(aa32_fpdp, s)) {
+return false;
+}
+
 if (!vfp_access_check(s)) {
 return true;
 }
@@ -2159,6 +2203,10 @@ static bool trans_VCVT_f16_f64(DisasContext *s, 
arg_VCVT_f16_f64 *a)
 return false;
 }
 
+if (!dc_isar_feature(aa32_fpdp, s)) {
+return false;
+}
+
 if (!vfp_access_check(s)) {
 return true;
 }
@@ -2215,6 +2263,10 @@ static bool trans_VRINTR_dp(DisasContext *s, 
arg_VRINTR_dp *a)
 return false;
 }
 
+if (!dc_isar_feature(aa32_fpdp, s)) {
+return false;
+}
+
 if (!vfp_access_check(s)) {
 return true;
 }
@@ -2272,6 +2324,10 @@ static bool trans_VRINTZ_dp(DisasContext *s, 
arg_VRINTZ_dp *a)
 return false;
 }
 
+if (!dc_isar_feature(aa32_fpdp, s)) {
+return false;
+}
+
 if (!vfp_access_check(s)) {
 return tr

Re: [Qemu-devel] [PATCH v3 15/15] vl: Deprecate -mon pretty=... for HMP monitors

[Markus Armbruster]

Kevin Wolf  writes:

> Am 14.06.2019 um 11:01 hat Markus Armbruster geschrieben:
>> Kevin Wolf  writes:
>> 
>> > The -mon pretty=on|off switch of the -mon option applies only the QMP
>
> s/the QMP/to QMP/
>
> Can you fix up this one as well while you're at it?

Sure!

>> > monitors. It used to be silently ignored for HMP. Deprecate this
>> > combination so that we can make it an error in future versions.
>> >
>> > Signed-off-by: Kevin Wolf 

[Qemu-devel] [PATCH 7/7] target/ppc/machine: Add kvmppc_pvr_workaround_required() stub

[Greg Kurz]

This allows to drop the CONFIG_KVM guard from the code.

Signed-off-by: Greg Kurz 
---
 target/ppc/kvm_ppc.h |5 +
 target/ppc/machine.c |2 --
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
index e642aaaf9226..98bd7d5da6d6 100644
--- a/target/ppc/kvm_ppc.h
+++ b/target/ppc/kvm_ppc.h
@@ -399,6 +399,11 @@ static inline int kvmppc_resize_hpt_commit(PowerPCCPU *cpu,
 return -ENOSYS;
 }
 
+static inline bool kvmppc_pvr_workaround_required(PowerPCCPU *cpu)
+{
+return false;
+}
+
 #endif
 
 #ifndef CONFIG_KVM
diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index 5ad7b40f4533..e82f5de9db7c 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -378,11 +378,9 @@ static int cpu_post_load(void *opaque, int version_id)
  * receive the PVR it expects as a workaround.
  *
  */
-#if defined(CONFIG_KVM)
 if (kvmppc_pvr_workaround_required(cpu)) {
 env->spr[SPR_PVR] = env->spr_cb[SPR_PVR].default_value;
 }
-#endif
 
 env->lr = env->spr[SPR_LR];
 env->ctr = env->spr[SPR_CTR];

[Qemu-devel] [PATCH 1/7] spapr_pci: Drop useless CONFIG_KVM ifdefery

[Greg Kurz]

kvm_enabled() expands to (0) when CONFIG_KVM is not defined.

Signed-off-by: Greg Kurz 
---
 hw/ppc/spapr_pci.c |2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index fbeb1c90ee6c..00d9f2cfe464 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1944,11 +1944,9 @@ static void spapr_phb_realize(DeviceState *dev, Error 
**errp)
  * For KVM we want to ensure that this memory is a full page so that
  * our memory slot is of page size granularity.
  */
-#ifdef CONFIG_KVM
 if (kvm_enabled()) {
 msi_window_size = getpagesize();
 }
-#endif
 
 memory_region_init_io(&sphb->msiwindow, OBJECT(sphb), &spapr_msi_ops, 
spapr,
   "msi", msi_window_size);

[Qemu-devel] [PATCH 5/7] hw/ppc: Drop useless CONFIG_KVM ifdefery

[Greg Kurz]

kvmppc_set_interrupt() has a stub that does nothing when CONFIG_KVM is
not defined.

Signed-off-by: Greg Kurz 
---
 hw/ppc/ppc.c |2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 9d91e8481b32..288196dfa67a 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -80,9 +80,7 @@ void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)
 }
 
 if (old_pending != env->pending_interrupts) {
-#ifdef CONFIG_KVM
 kvmppc_set_interrupt(cpu, n_IRQ, level);
-#endif
 }
 
 

[Qemu-devel] [PATCH 4/7] hw/ppc/prep: Drop useless CONFIG_KVM ifdefery

[Greg Kurz]

kvm_enabled() expands to (0) when CONFIG_KVM is not defined. It is
likely that the compiler will optimize the code out. And even if
it doesn't, we have a stub for kvmppc_get_hypercall().

Signed-off-by: Greg Kurz 
---
 hw/ppc/prep.c |2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index 2a8009e20b46..a248ce480d57 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -780,7 +780,6 @@ static void ibm_40p_init(MachineState *machine)
 
 fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_IS_KVM, kvm_enabled());
 if (kvm_enabled()) {
-#ifdef CONFIG_KVM
 uint8_t *hypercall;
 
 fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, kvmppc_get_tbfreq());
@@ -788,7 +787,6 @@ static void ibm_40p_init(MachineState *machine)
 kvmppc_get_hypercall(env, hypercall, 16);
 fw_cfg_add_bytes(fw_cfg, FW_CFG_PPC_KVM_HC, hypercall, 16);
 fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_KVM_PID, getpid());
-#endif
 } else {
 fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, NANOSECONDS_PER_SECOND);
 }

[Qemu-devel] [PATCH 0/7] ppc: Get rid of some CONFIG_KVM guards

[Greg Kurz]

There are several places where CONFIG_KVM is used to guard code that
should only be built when KVM is supported. It is generally preferable
to avoid that and leave such guards in header files for improved
readability.

In many cases, the execution of the code is also conditionned by
kvm_enabled() which expands to (0) when CONFIG_KVM is not defined.
This is likely to cause the compiler to optimize the code out,
and if it doesn't, the right way to address compiling issues is
to add stubs.

Successfuly compile tested on x86_64 and ppc64le linux. Travis shows
this builds fine on OSX as well.

--
Greg

---

Greg Kurz (7):
  spapr_pci: Drop useless CONFIG_KVM ifdefery
  hw/ppc/mac_oldworld: Drop useless CONFIG_KVM ifdefery
  hw/ppc/mac_newworld: Drop useless CONFIG_KVM ifdefery
  hw/ppc/prep: Drop useless CONFIG_KVM ifdefery
  hw/ppc: Drop useless CONFIG_KVM ifdefery
  ppc: Introduce kvmppc_set_reg_tb_offset() helper
  target/ppc/machine: Add kvmppc_pvr_workaround_required() stub


 hw/ppc/mac_newworld.c |4 
 hw/ppc/mac_oldworld.c |2 --
 hw/ppc/ppc.c  |7 +--
 hw/ppc/prep.c |2 --
 hw/ppc/spapr_pci.c|2 --
 target/ppc/kvm.c  |9 +
 target/ppc/kvm_ppc.h  |   10 ++
 target/ppc/machine.c  |2 --
 8 files changed, 20 insertions(+), 18 deletions(-)

[Qemu-devel] [PATCH 6/7] ppc: Introduce kvmppc_set_reg_tb_offset() helper

[Greg Kurz]

Introduce a KVM helper and its stub instead of guarding the code with
CONFIG_KVM.

Signed-off-by: Greg Kurz 
---
 hw/ppc/ppc.c |5 +
 target/ppc/kvm.c |9 +
 target/ppc/kvm_ppc.h |5 +
 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 288196dfa67a..a9e508c496de 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -1034,10 +1034,7 @@ static void timebase_load(PPCTimebase *tb)
 CPU_FOREACH(cpu) {
 PowerPCCPU *pcpu = POWERPC_CPU(cpu);
 pcpu->env.tb_env->tb_offset = tb_off_adj;
-#if defined(CONFIG_KVM)
-kvm_set_one_reg(cpu, KVM_REG_PPC_TB_OFFSET,
-&pcpu->env.tb_env->tb_offset);
-#endif
+kvmppc_set_reg_tb_offset(pcpu, pcpu->env.tb_env->tb_offset);
 }
 }
 
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index d4107dd70d21..cde39904510a 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -2939,3 +2939,12 @@ void kvmppc_set_reg_ppc_online(PowerPCCPU *cpu, unsigned 
int online)
 kvm_set_one_reg(cs, KVM_REG_PPC_ONLINE, &online);
 }
 }
+
+void kvmppc_set_reg_tb_offset(PowerPCCPU *cpu, int64_t tb_offset)
+{
+CPUState *cs = CPU(cpu);
+
+if (kvm_enabled()) {
+kvm_set_one_reg(cs, KVM_REG_PPC_TB_OFFSET, &tb_offset);
+}
+}
diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
index 45776cad79d9..e642aaaf9226 100644
--- a/target/ppc/kvm_ppc.h
+++ b/target/ppc/kvm_ppc.h
@@ -80,6 +80,7 @@ bool kvmppc_pvr_workaround_required(PowerPCCPU *cpu);
 bool kvmppc_hpt_needs_host_contiguous_pages(void);
 void kvm_check_mmu(PowerPCCPU *cpu, Error **errp);
 void kvmppc_set_reg_ppc_online(PowerPCCPU *cpu, unsigned int online);
+void kvmppc_set_reg_tb_offset(PowerPCCPU *cpu, int64_t tb_offset);
 
 #else
 
@@ -206,6 +207,10 @@ static inline void kvmppc_set_reg_ppc_online(PowerPCCPU 
*cpu,
 return;
 }
 
+static inline void kvmppc_set_reg_tb_offset(PowerPCCPU *cpu, int64_t tb_offset)
+{
+}
+
 #ifndef CONFIG_USER_ONLY
 static inline bool kvmppc_spapr_use_multitce(void)
 {

[Qemu-devel] [PATCH 2/7] hw/ppc/mac_oldworld: Drop useless CONFIG_KVM ifdefery

[Greg Kurz]

kvm_enabled() expands to (0) when CONFIG_KVM is not defined. It is
likely that the compiler will optimize the code out. And even if
it doesn't, we have a stub for kvmppc_get_hypercall().

Signed-off-by: Greg Kurz 
---
 hw/ppc/mac_oldworld.c |2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/ppc/mac_oldworld.c b/hw/ppc/mac_oldworld.c
index eddd005a7ce4..da751addc495 100644
--- a/hw/ppc/mac_oldworld.c
+++ b/hw/ppc/mac_oldworld.c
@@ -345,14 +345,12 @@ static void ppc_heathrow_init(MachineState *machine)
 
 fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_IS_KVM, kvm_enabled());
 if (kvm_enabled()) {
-#ifdef CONFIG_KVM
 uint8_t *hypercall;
 
 hypercall = g_malloc(16);
 kvmppc_get_hypercall(env, hypercall, 16);
 fw_cfg_add_bytes(fw_cfg, FW_CFG_PPC_KVM_HC, hypercall, 16);
 fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_KVM_PID, getpid());
-#endif
 }
 fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, tbfreq);
 /* Mac OS X requires a "known good" clock-frequency value; pass it one. */

[Qemu-devel] [PATCH 3/7] hw/ppc/mac_newworld: Drop useless CONFIG_KVM ifdefery

[Greg Kurz]

kvm_enabled() expands to (0) when CONFIG_KVM is not defined. The first
CONFIG_KVM guard is thus useless and it is likely that the compiler
will optimize the code out in the case of the second guard. And even
if it doesn't, we have a stub for kvmppc_get_hypercall().

Signed-off-by: Greg Kurz 
---
 hw/ppc/mac_newworld.c |4 
 1 file changed, 4 deletions(-)

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 4d835f32b536..c8d324552470 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -437,13 +437,11 @@ static void ppc_core99_init(MachineState *machine)
 }
 
 /* The NewWorld NVRAM is not located in the MacIO device */
-#ifdef CONFIG_KVM
 if (kvm_enabled() && getpagesize() > 4096) {
 /* We can't combine read-write and read-only in a single page, so
move the NVRAM out of ROM again for KVM */
 nvram_addr = 0xFFE0;
 }
-#endif
 dev = qdev_create(NULL, TYPE_MACIO_NVRAM);
 qdev_prop_set_uint32(dev, "size", 0x2000);
 qdev_prop_set_uint32(dev, "it_shift", 1);
@@ -488,14 +486,12 @@ static void ppc_core99_init(MachineState *machine)
 
 fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_IS_KVM, kvm_enabled());
 if (kvm_enabled()) {
-#ifdef CONFIG_KVM
 uint8_t *hypercall;
 
 hypercall = g_malloc(16);
 kvmppc_get_hypercall(env, hypercall, 16);
 fw_cfg_add_bytes(fw_cfg, FW_CFG_PPC_KVM_HC, hypercall, 16);
 fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_KVM_PID, getpid());
-#endif
 }
 fw_cfg_add_i32(fw_cfg, FW_CFG_PPC_TBFREQ, tbfreq);
 /* Mac OS X requires a "known good" clock-frequency value; pass it one. */

Re: [Qemu-devel] [PATCH v4 5/6] migration: Make no compression operations into its own structure

[Dr. David Alan Gilbert]

* Juan Quintela (quint...@redhat.com) wrote:
> It will be used later.
> 
> Signed-off-by: Juan Quintela 
> 
> ---
> Move setup of ->ops helper to proper place (wei)
> Rename s/none/nocomp/ (dave)
> ---
>  migration/migration.c |   9 ++
>  migration/migration.h |   1 +
>  migration/ram.c   | 188 --
>  3 files changed, 190 insertions(+), 8 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 3f17d8f2f8..a3526d395b 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2174,6 +2174,15 @@ int migrate_multifd_channels(void)
>  return s->parameters.multifd_channels;
>  }
>  
> +int migrate_multifd_method(void)
> +{
> +MigrationState *s;
> +
> +s = migrate_get_current();
> +
> +return s->parameters.multifd_compress;
> +}
> +
>  int migrate_use_xbzrle(void)
>  {
>  MigrationState *s;
> diff --git a/migration/migration.h b/migration/migration.h
> index 71c03353c3..437abf3405 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -270,6 +270,7 @@ bool migrate_auto_converge(void);
>  bool migrate_use_multifd(void);
>  bool migrate_pause_before_switchover(void);
>  int migrate_multifd_channels(void);
> +int migrate_multifd_method(void);
>  
>  int migrate_use_xbzrle(void);
>  int64_t migrate_xbzrle_cache_size(void);
> diff --git a/migration/ram.c b/migration/ram.c
> index b0ca989160..3b0002ddba 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -45,6 +45,7 @@
>  #include "page_cache.h"
>  #include "qemu/error-report.h"
>  #include "qapi/error.h"
> +#include "qapi/qapi-types-migration.h"
>  #include "qapi/qapi-events-migration.h"
>  #include "qapi/qmp/qerror.h"
>  #include "trace.h"
> @@ -661,6 +662,8 @@ typedef struct {
>  uint64_t num_packets;
>  /* pages sent through this channel */
>  uint64_t num_pages;
> +/* used for compression methods */
> +void *data;
>  }  MultiFDSendParams;
>  
>  typedef struct {
> @@ -696,8 +699,152 @@ typedef struct {
>  uint64_t num_pages;
>  /* syncs main thread and channels */
>  QemuSemaphore sem_sync;
> +/* used for de-compression methods */
> +void *data;
>  } MultiFDRecvParams;
>  
> +typedef struct {
> +/* Setup for sending side */
> +int (*send_setup)(MultiFDSendParams *p, Error **errp);
> +/* Cleanup for sending side */
> +void (*send_cleanup)(MultiFDSendParams *p);
> +/* Prepare the send packet */
> +int (*send_prepare)(MultiFDSendParams *p, uint32_t used, Error **errp);
> +/* Write the send packet */
> +int (*send_write)(MultiFDSendParams *p, uint32_t used, Error **errp);
> +/* Setup for receiving side */
> +int (*recv_setup)(MultiFDRecvParams *p, Error **errp);
> +/* Cleanup for receiving side */
> +void (*recv_cleanup)(MultiFDRecvParams *p);
> +/* Read all pages */
> +int (*recv_pages)(MultiFDRecvParams *p, uint32_t used, Error **errp);
> +} MultiFDMethods;
> +
> +/* Multifd without compression */
> +
> +/**
> + * nocomp_send_setup: setup send side
> + *
> + * For no compression this function does nothing.
> + *
> + * Returns 0 for success or -1 for error
> + *
> + * @p: Params for the channel that we are using
> + * @errp: pointer to an error
> + */
> +static int nocomp_send_setup(MultiFDSendParams *p, Error **errp)
> +{
> +return 0;
> +}
> +
> +/**
> + * nocomp_send_cleanup: cleanup send side
> + *
> + * For no compression this function does nothing.
> + *
> + * @p: Params for the channel that we are using
> + */
> +static void nocomp_send_cleanup(MultiFDSendParams *p)
> +{
> +return;
> +}
> +
> +/**
> + * nocomp_send_prepare: prepare date to be able to send
> + *
> + * For no compression we just have to calculate the size of the
> + * packet.
> + *
> + * Returns 0 for success or -1 for error
> + *
> + * @p: Params for the channel that we are using
> + * @used: number of pages used
> + * @errp: pointer to an error
> + */
> +static int nocomp_send_prepare(MultiFDSendParams *p, uint32_t used,
> +   Error **errp)
> +{
> +p->next_packet_size = used * qemu_target_page_size();
> +return 0;
> +}
> +
> +/**
> + * nocomp_send_write: do the actual write of the data
> + *
> + * For no compression we just have to write the data.
> + *
> + * Returns 0 for success or -1 for error
> + *
> + * @p: Params for the channel that we are using
> + * @used: number of pages used
> + * @errp: pointer to an error
> + */
> +static int nocomp_send_write(MultiFDSendParams *p, uint32_t used, Error 
> **errp)
> +{
> +return qio_channel_writev_all(p->c, p->pages->iov, used, errp);
> +}
> +
> +/**
> + * nocomp_recv_setup: setup receive side
> + *
> + * For no compression this function does nothing.
> + *
> + * Returns 0 for success or -1 for error
> + *
> + * @p: Params for the channel that we are using
> + * @errp: pointer to an error
> + */
> +static int nocomp_recv_setup(MultiFDRecvParams *p, Error **errp)
> +{
> +retur

Re: [Qemu-devel] [PATCH v3 1/5] virtio: add "use-started" property

[Greg Kurz]

On Fri, 14 Jun 2019 17:31:17 +0800
elohi...@gmail.com wrote:

> From: Xie Yongji 
> 
> In order to avoid migration issues, we introduce a "use-started"
> property to the base virtio device to indicate whether use
> "started" flag or not. This property will be true by default and
> set to false when machine type <= 4.0.1.
> 
> Suggested-by: Greg Kurz 
> Signed-off-by: Xie Yongji 
> ---
>  hw/block/vhost-user-blk.c  |  4 ++--
>  hw/core/machine.c  |  8 ++--

This patch conflicts with latest upstream changes to hw_compat_4_0_1[].

It seems you need to rebase. Also, I'm still not sure how we're supposed
to handle hw_compat_4_0_1[] versus hw_compat_4_0[]... nobody commented
on:

https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg00637.html
https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg00641.html

Maybe worth to sort that out before re-posting.

>  hw/virtio/virtio.c | 18 +++---
>  include/hw/virtio/virtio.h | 21 +
>  4 files changed, 36 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
> index 9cb61336a6..85bc4017e7 100644
> --- a/hw/block/vhost-user-blk.c
> +++ b/hw/block/vhost-user-blk.c
> @@ -191,7 +191,7 @@ static void vhost_user_blk_stop(VirtIODevice *vdev)
>  static void vhost_user_blk_set_status(VirtIODevice *vdev, uint8_t status)
>  {
>  VHostUserBlk *s = VHOST_USER_BLK(vdev);
> -bool should_start = vdev->started;
> +bool should_start = virtio_device_started(vdev, status);
>  int ret;
>  
>  if (!vdev->vm_running) {
> @@ -317,7 +317,7 @@ static int vhost_user_blk_connect(DeviceState *dev)
>  }
>  
>  /* restore vhost state */
> -if (vdev->started) {
> +if (virtio_device_started(vdev, vdev->status)) {
>  ret = vhost_user_blk_start(vdev);
>  if (ret < 0) {
>  error_report("vhost-user-blk: vhost start failed: %s",
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index f1a0f45f9c..60d1e27d2c 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -24,10 +24,14 @@
>  #include "hw/pci/pci.h"
>  #include "hw/mem/nvdimm.h"
>  
> -GlobalProperty hw_compat_4_0_1[] = {};
> +GlobalProperty hw_compat_4_0_1[] = {
> +{ "virtio-device", "use-started", "false" },
> +};
>  const size_t hw_compat_4_0_1_len = G_N_ELEMENTS(hw_compat_4_0_1);
>  
> -GlobalProperty hw_compat_4_0[] = {};
> +GlobalProperty hw_compat_4_0[] = {
> +{ "virtio-device", "use-started", "false" },
> +};
>  const size_t hw_compat_4_0_len = G_N_ELEMENTS(hw_compat_4_0);
>  
>  GlobalProperty hw_compat_3_1[] = {
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 07f4a64b48..19062fbb96 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1162,10 +1162,8 @@ int virtio_set_status(VirtIODevice *vdev, uint8_t val)
>  }
>  }
>  }
> -vdev->started = val & VIRTIO_CONFIG_S_DRIVER_OK;
> -if (unlikely(vdev->start_on_kick && vdev->started)) {
> -vdev->start_on_kick = false;
> -}
> +
> +virtio_set_started(vdev, val & VIRTIO_CONFIG_S_DRIVER_OK);
>  
>  if (k->set_status) {
>  k->set_status(vdev, val);
> @@ -1536,8 +1534,7 @@ static bool virtio_queue_notify_aio_vq(VirtQueue *vq)
>  ret = vq->handle_aio_output(vdev, vq);
>  
>  if (unlikely(vdev->start_on_kick)) {
> -vdev->started = true;
> -vdev->start_on_kick = false;
> +virtio_set_started(vdev, true);
>  }
>  }
>  
> @@ -1557,8 +1554,7 @@ static void virtio_queue_notify_vq(VirtQueue *vq)
>  vq->handle_output(vdev, vq);
>  
>  if (unlikely(vdev->start_on_kick)) {
> -vdev->started = true;
> -vdev->start_on_kick = false;
> +virtio_set_started(vdev, true);
>  }
>  }
>  }
> @@ -1579,8 +1575,7 @@ void virtio_queue_notify(VirtIODevice *vdev, int n)
>  }
>  
>  if (unlikely(vdev->start_on_kick)) {
> -vdev->started = true;
> -vdev->start_on_kick = false;
> +virtio_set_started(vdev, true);
>  }
>  }
>  
> @@ -2291,7 +2286,7 @@ static void virtio_vmstate_change(void *opaque, int 
> running, RunState state)
>  VirtIODevice *vdev = opaque;
>  BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
>  VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> -bool backend_run = running && vdev->started;
> +bool backend_run = running && virtio_device_started(vdev, vdev->status);
>  vdev->vm_running = running;
>  
>  if (backend_run) {
> @@ -2669,6 +2664,7 @@ static void virtio_device_instance_finalize(Object *obj)
>  
>  static Property virtio_properties[] = {
>  DEFINE_VIRTIO_COMMON_FEATURES(VirtIODevice, host_features),
> +DEFINE_PROP_BOOL("use-started", VirtIODevice, use_started, true),
>  DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 27c0efc3d0..15d5366939 100644
> --- a/include/hw/vir

Re: [Qemu-devel] [PATCH v1 2/2] s390x/cpumodel: Prepend KDSA features with "KDSA"

[Cornelia Huck]

On Wed, 12 Jun 2019 15:25:26 +0200
David Hildenbrand  wrote:

> Let's handle it just like for other crypto features.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/cpu_features_def.inc.h | 30 ++---
>  target/s390x/gen-features.c | 30 ++---
>  2 files changed, 30 insertions(+), 30 deletions(-)

Acked-by: Cornelia Huck 

Re: [Qemu-devel] [PATCH for 4.1 v3] target/riscv: Expose time CSRs when allowed by [m|s]counteren

[Palmer Dabbelt]

On Tue, 28 May 2019 11:30:20 PDT (-0700), jonat...@fintelia.io wrote:

Currently mcounteren.TM acts as though it is hardwired to zero, even though 
QEMU allows it to be set. This change resolves the issue by allowing reads to 
the time and timeh control registers when running in a privileged mode where 
such accesses are allowed.

The frequency of the time register is stored in the time_freq field of each 
hart so that it is accessible during CSR reads, but must be the same across all 
harts. Each board can initialize it to a custom value, although all currently 
use a 10 MHz frequency.

Signed-off-by: Jonathan Behrens 
---
 hw/riscv/riscv_hart.c   |  4 
 hw/riscv/sifive_clint.c | 30 ++
 hw/riscv/sifive_e.c |  2 ++
 hw/riscv/sifive_u.c |  4 +++-
 hw/riscv/spike.c|  6 +-
 hw/riscv/virt.c |  4 +++-
 include/hw/riscv/riscv_hart.h   |  1 +
 include/hw/riscv/sifive_clint.h |  4 
 include/hw/riscv/sifive_e.h |  4 
 include/hw/riscv/sifive_u.h |  1 +
 include/hw/riscv/spike.h|  1 +
 include/hw/riscv/virt.h |  1 +
 target/riscv/cpu.h  |  2 ++
 target/riscv/csr.c  | 17 +++--
 14 files changed, 60 insertions(+), 21 deletions(-)

diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
index e34a26a0ef..c39cd55330 100644
--- a/hw/riscv/riscv_hart.c
+++ b/hw/riscv/riscv_hart.c
@@ -19,6 +19,7 @@
  */

 #include "qemu/osdep.h"
+#include "qemu/timer.h"
 #include "qapi/error.h"
 #include "hw/sysbus.h"
 #include "target/riscv/cpu.h"
@@ -27,6 +28,8 @@
 static Property riscv_harts_props[] = {
 DEFINE_PROP_UINT32("num-harts", RISCVHartArrayState, num_harts, 1),
 DEFINE_PROP_STRING("cpu-type", RISCVHartArrayState, cpu_type),
+DEFINE_PROP_UINT64("timebase-frequency", RISCVHartArrayState, time_freq,
+   NANOSECONDS_PER_SECOND),
 DEFINE_PROP_END_OF_LIST(),
 };

@@ -49,6 +52,7 @@ static void riscv_harts_realize(DeviceState *dev, Error 
**errp)
 sizeof(RISCVCPU), s->cpu_type,
 &error_abort, NULL);
 s->harts[n].env.mhartid = n;
+s->harts[n].env.time_freq = s->time_freq;
 qemu_register_reset(riscv_harts_cpu_reset, &s->harts[n]);
 object_property_set_bool(OBJECT(&s->harts[n]), true,
  "realized", &err);
diff --git a/hw/riscv/sifive_clint.c b/hw/riscv/sifive_clint.c
index d4c159e937..71edf4dcc6 100644
--- a/hw/riscv/sifive_clint.c
+++ b/hw/riscv/sifive_clint.c
@@ -26,10 +26,10 @@
 #include "hw/riscv/sifive_clint.h"
 #include "qemu/timer.h"

-static uint64_t cpu_riscv_read_rtc(void)
+static uint64_t cpu_riscv_read_rtc(CPURISCVState *env)
 {
 return muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
-SIFIVE_CLINT_TIMEBASE_FREQ, NANOSECONDS_PER_SECOND);
+env->time_freq, NANOSECONDS_PER_SECOND);
 }

 /*
@@ -41,7 +41,7 @@ static void sifive_clint_write_timecmp(RISCVCPU *cpu, 
uint64_t value)
 uint64_t next;
 uint64_t diff;

-uint64_t rtc_r = cpu_riscv_read_rtc();
+uint64_t rtc_r = cpu_riscv_read_rtc(&cpu->env);

 cpu->env.timecmp = value;
 if (cpu->env.timecmp <= rtc_r) {
@@ -56,7 +56,7 @@ static void sifive_clint_write_timecmp(RISCVCPU *cpu, 
uint64_t value)
 diff = cpu->env.timecmp - rtc_r;
 /* back to ns (note args switched in muldiv64) */
 next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
-muldiv64(diff, NANOSECONDS_PER_SECOND, SIFIVE_CLINT_TIMEBASE_FREQ);
+muldiv64(diff, NANOSECONDS_PER_SECOND, cpu->env.time_freq);
 timer_mod(cpu->env.timer, next);
 }

@@ -107,11 +107,25 @@ static uint64_t sifive_clint_read(void *opaque, hwaddr 
addr, unsigned size)
 return 0;
 }
 } else if (addr == clint->time_base) {
-/* time_lo */
-return cpu_riscv_read_rtc() & 0x;
+/* All harts must have the same time frequency, so just use hart 0 */
+CPUState *cpu = qemu_get_cpu(0);
+CPURISCVState *env = cpu ? cpu->env_ptr : NULL;
+if (!env) {
+error_report("clint: hart 0 not valid?!");
+} else {
+/* time_lo */
+return cpu_riscv_read_rtc(env) & 0x;
+}
 } else if (addr == clint->time_base + 4) {
-/* time_hi */
-return (cpu_riscv_read_rtc() >> 32) & 0x;
+/* All harts must have the same time frequency, so just use hart 0 */
+CPUState *cpu = qemu_get_cpu(0);
+CPURISCVState *env = cpu ? cpu->env_ptr : NULL;
+if (!env) {
+error_report("clint: hart 0 not valid?!");
+} else {
+/* time_hi */
+return (cpu_riscv_read_rtc(env) >> 32) & 0x;
+}
 }

 error_report("clint: invalid read: %08x", (uint32_t)addr);
diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
index 80ac56fa7d..2d6f768ff1 100644
--- a/hw/risc

Re: [Qemu-devel] [PATCH v1 0/2] s390x/cpumodel: Rework CPU feature definition

[Cornelia Huck]

On Wed, 12 Jun 2019 15:25:24 +0200
David Hildenbrand  wrote:

> Rework the feature initialization, making it harder to make mistakes.
> While at it, rename the enum names of the KDSA features.
> 
> David Hildenbrand (2):
>   s390x/cpumodel: Rework CPU feature definition
>   s390x/cpumodel: Prepend KDSA features with "KDSA"
> 
>  target/s390x/cpu_features.c | 352 +-
>  target/s390x/cpu_features_def.h | 352 +-
>  target/s390x/cpu_features_def.inc.h | 369 
>  target/s390x/gen-features.c |  30 +--
>  4 files changed, 401 insertions(+), 702 deletions(-)
>  create mode 100644 target/s390x/cpu_features_def.inc.h
> 

Looks good to me. Do you want to take it via your tree?

Re: [Qemu-devel] [PATCH v1 1/2] s390x/cpumodel: Rework CPU feature definition

[Cornelia Huck]

On Wed, 12 Jun 2019 15:25:25 +0200
David Hildenbrand  wrote:

> Let's define features at a single spot and make it less error prone to
> define new features.
> 
> Signed-off-by: David Hildenbrand 
> ---
>  target/s390x/cpu_features.c | 352 +-
>  target/s390x/cpu_features_def.h | 352 +-
>  target/s390x/cpu_features_def.inc.h | 369 
>  3 files changed, 386 insertions(+), 687 deletions(-)
>  create mode 100644 target/s390x/cpu_features_def.inc.h

Yes, let's get rid of the need to define this in two places.

Acked-by: Cornelia Huck 

Re: [Qemu-devel] [PATCH 13/19] aspeed/smc: add a 'sdram_base' propertie

[Cédric Le Goater]

On 13/06/2019 16:32, Philippe Mathieu-Daudé wrote:
> On 5/25/19 5:12 PM, Cédric Le Goater wrote:
>> The DRAM address of a DMA transaction depends on the DRAM base address
>> of the SoC. Inform the SMC controller model of this value.
> 
> I'd reorder this one previous patch #16 "aspeed/smc: add support for
> DMAs" where you start to use sdram_base.
> 
> Regardless,
> Reviewed-by: Philippe Mathieu-Daudé 


done.

Thanks,

C. 

> 
>>
>> Signed-off-by: Cédric Le Goater 
>> ---
>>  include/hw/ssi/aspeed_smc.h | 3 +++
>>  hw/arm/aspeed_soc.c | 6 ++
>>  hw/ssi/aspeed_smc.c | 1 +
>>  3 files changed, 10 insertions(+)
>>
>> diff --git a/include/hw/ssi/aspeed_smc.h b/include/hw/ssi/aspeed_smc.h
>> index 3b1e7fce6c86..591279ba1f43 100644
>> --- a/include/hw/ssi/aspeed_smc.h
>> +++ b/include/hw/ssi/aspeed_smc.h
>> @@ -97,6 +97,9 @@ typedef struct AspeedSMCState {
>>  uint8_t r_timings;
>>  uint8_t conf_enable_w0;
>>  
>> +/* for DMA support */
>> +uint64_t sdram_base;
>> +
>>  AspeedSMCFlash *flashes;
>>  
>>  uint8_t snoop_index;
>> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
>> index 8cfe9e9515ed..65fbac896c85 100644
>> --- a/hw/arm/aspeed_soc.c
>> +++ b/hw/arm/aspeed_soc.c
>> @@ -326,6 +326,12 @@ static void aspeed_soc_realize(DeviceState *dev, Error 
>> **errp)
>> aspeed_soc_get_irq(s, ASPEED_I2C));
>>  
>>  /* FMC, The number of CS is set at the board level */
>> +object_property_set_int(OBJECT(&s->fmc), sc->info->memmap[ASPEED_SDRAM],
>> +"sdram-base", &err);
>> +if (err) {
>> +error_propagate(errp, err);
>> +return;
>> +}
>>  object_property_set_bool(OBJECT(&s->fmc), true, "realized", &err);
>>  if (err) {
>>  error_propagate(errp, err);
>> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
>> index f1e66870d71f..4ff12f7b27fc 100644
>> --- a/hw/ssi/aspeed_smc.c
>> +++ b/hw/ssi/aspeed_smc.c
>> @@ -912,6 +912,7 @@ static const VMStateDescription vmstate_aspeed_smc = {
>>  
>>  static Property aspeed_smc_properties[] = {
>>  DEFINE_PROP_UINT32("num-cs", AspeedSMCState, num_cs, 1),
>> +DEFINE_PROP_UINT64("sdram-base", AspeedSMCState, sdram_base, 0),
>>  DEFINE_PROP_END_OF_LIST(),
>>  };
>>  
>>

Re: [Qemu-devel] [PATCH 18/19] aspeed/smc: inject errors in DMA checksum

[Cédric Le Goater]

On 13/06/2019 16:31, Philippe Mathieu-Daudé wrote:
> Hi Cédric,
> 
> On 5/25/19 5:12 PM, Cédric Le Goater wrote:
>> Emulate read errors in the DMA Checksum Register for high frequencies
>> and optimistic settings of the Read Timing Compensation Register. This
>> will help in tuning the SPI timing calibration algorithm.
>>
>> The values below are those to expect from the first flash device of
>> the FMC controller of a palmetto-bmc machine.
>>
>> Signed-off-by: Cédric Le Goater 
>> ---
>>  hw/ssi/aspeed_smc.c | 29 +
>>  1 file changed, 29 insertions(+)
>>
>> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
>> index 406c30c60b3f..4c162912cf62 100644
>> --- a/hw/ssi/aspeed_smc.c
>> +++ b/hw/ssi/aspeed_smc.c
>> @@ -866,6 +866,30 @@ static void aspeed_smc_dma_calibration(AspeedSMCState 
>> *s)
>>  s->regs[s->r_ctrl0 + cs] |= CE_CTRL_CLOCK_FREQ(hclk_div);
>>  }
>>  
> 
> Can you add a comment (like the patch description) here?

yes. done.
 
>> +static bool aspeed_smc_inject_read_failure(AspeedSMCState *s)
>> +{
>> +uint8_t delay =
>> +(s->regs[R_DMA_CTRL] >> DMA_CTRL_DELAY_SHIFT) & DMA_CTRL_DELAY_MASK;
>> +uint8_t hclk_mask =
>> +(s->regs[R_DMA_CTRL] >> DMA_CTRL_FREQ_SHIFT) & DMA_CTRL_FREQ_MASK;
>> +
>> +/*
>> + * Typical values of a palmetto-bmc machine.
>> + */
>> +switch (aspeed_smc_hclk_divisor(hclk_mask)) {
>> +case 4 ... 16:
>> +return false;
>> +case 3: /* at least one HCLK cycle delay */
>> +return (delay & 0x7) < 1;
>> +case 2: /* at least two HCLK cycle delay */
>> +return (delay & 0x7) < 2;
>> +case 1: /* (> 100MHz) is above the max freq of the controller */
>> +return true;
>> +default:
>> +g_assert_not_reached();
>> +}
>> +}
>> +
>>  /*
>>   * Accumulate the result of the reads to provide a checksum that will
>>   * be used to validate the read timing settings.
>> @@ -903,6 +927,11 @@ static void aspeed_smc_dma_checksum(AspeedSMCState *s)
>>  s->regs[R_DMA_FLASH_ADDR] += 4;
>>  s->regs[R_DMA_LEN] -= 4;
>>  }
>> +
>> +if (aspeed_smc_inject_read_failure(s)) {
> 
> So this model real world where noise eventually triggers errors. Don't
> we want this to be enable by the user (or a QMP command)?

I can add a property at the device model level to trigger this behavior.
Such as :

   -global driver=aspeed.smc,property=timing,value=true

timing if defined would provide the maximum clock and delay settings.

Are there any other examples in QEMU ? 

Thanks,

C.

> 
>> +s->regs[R_DMA_CHECKSUM] = 0xbadc0de;
>> +}
>> +
>>  }
>>  
>>  static void aspeed_smc_dma_rw(AspeedSMCState *s)
>>

Re: [Qemu-devel] [PULL 01/29] SiFive RISC-V GPIO Device

[Palmer Dabbelt]

On Thu, 30 May 2019 03:57:12 PDT (-0700), Peter Maydell wrote:

On Sun, 26 May 2019 at 02:10, Palmer Dabbelt  wrote:


From: Fabien Chouteau 

QEMU model of the GPIO device on the SiFive E300 series SOCs.

The pins are not used by a board definition yet, however this
implementation can already be used to trigger GPIO interrupts from the
software by configuring a pin as both output and input.

Signed-off-by: Fabien Chouteau 
Reviewed-by: Palmer Dabbelt 
Signed-off-by: Palmer Dabbelt 


Hi; this patch causes Coverity to complain about a memory
leak (CID 1401707):


 static void riscv_sifive_e_soc_realize(DeviceState *dev, Error **errp)
 {
 const struct MemmapEntry *memmap = sifive_e_memmap;
+Error *err = NULL;

 SiFiveESoCState *s = RISCV_E_SOC(dev);
 MemoryRegion *sys_mem = get_system_memory();
@@ -184,8 +188,28 @@ static void riscv_sifive_e_soc_realize(DeviceState *dev, 
Error **errp)
 sifive_mmio_emulate(sys_mem, "riscv.sifive.e.aon",
 memmap[SIFIVE_E_AON].base, memmap[SIFIVE_E_AON].size);
 sifive_prci_create(memmap[SIFIVE_E_PRCI].base);
-sifive_mmio_emulate(sys_mem, "riscv.sifive.e.gpio0",
-memmap[SIFIVE_E_GPIO0].base, memmap[SIFIVE_E_GPIO0].size);
+
+/* GPIO */
+
+object_property_set_bool(OBJECT(&s->gpio), true, "realized", &err);
+if (err) {
+error_propagate(errp, err);
+return;
+}


This function allocated xip_mem and mask_rom via g_new() but
then this error-exit doesn't free them.

The best way to fix this is to stop doing separate memory
allocations at all -- instead just have fields in the
SiFiveESoCState struct
   MemoryRegion xip_mem;
   Memory_Region mask_rom;

and pass &s->xip_mem etc where currently the code uses xip_mem.


Sorry this took a while to fix, I've just sent a patch to fix the memory leak.

Re: [Qemu-devel] [PATCH v1 0/2] s390x/cpumodel: Rework CPU feature definition

[David Hildenbrand]

On 14.06.19 13:49, Cornelia Huck wrote:
> On Wed, 12 Jun 2019 15:25:24 +0200
> David Hildenbrand  wrote:
> 
>> Rework the feature initialization, making it harder to make mistakes.
>> While at it, rename the enum names of the KDSA features.
>>
>> David Hildenbrand (2):
>>   s390x/cpumodel: Rework CPU feature definition
>>   s390x/cpumodel: Prepend KDSA features with "KDSA"
>>
>>  target/s390x/cpu_features.c | 352 +-
>>  target/s390x/cpu_features_def.h | 352 +-
>>  target/s390x/cpu_features_def.inc.h | 369 
>>  target/s390x/gen-features.c |  30 +--
>>  4 files changed, 401 insertions(+), 702 deletions(-)
>>  create mode 100644 target/s390x/cpu_features_def.inc.h
>>
> 
> Looks good to me. Do you want to take it via your tree?
> 

Yep, will do. Thanks!

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v5 13/42] block: Use CAFs in block status functions

[Vladimir Sementsov-Ogievskiy]

13.06.2019 1:09, Max Reitz wrote:
> Use the child access functions in the block status inquiry functions as
> appropriate.
> 
> Signed-off-by: Max Reitz


Reviewed-by: Vladimir Sementsov-Ogievskiy 

-- 
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH v1 0/2] s390x/cpumodel: Rework CPU feature definition

[David Hildenbrand]

On 12.06.19 15:25, David Hildenbrand wrote:
> Rework the feature initialization, making it harder to make mistakes.
> While at it, rename the enum names of the KDSA features.
> 
> David Hildenbrand (2):
>   s390x/cpumodel: Rework CPU feature definition
>   s390x/cpumodel: Prepend KDSA features with "KDSA"
> 
>  target/s390x/cpu_features.c | 352 +-
>  target/s390x/cpu_features_def.h | 352 +-
>  target/s390x/cpu_features_def.inc.h | 369 
>  target/s390x/gen-features.c |  30 +--
>  4 files changed, 401 insertions(+), 702 deletions(-)
>  create mode 100644 target/s390x/cpu_features_def.inc.h
> 

Queued to

https://github.com/davidhildenbrand/qemu.git s390-tcg-next

Thanks to Janosch and Conny.

-- 

Thanks,

David / dhildenb

[Qemu-devel] [PATCH] RISC-V: Fix a memory leak when realizing a sifive_e

[Palmer Dabbelt]

Coverity pointed out a memory leak in riscv_sifive_e_soc_realize(),
where a pair of recently added MemoryRegion instances would not be freed
if there were errors elsewhere in the function.  The fix here is to
simply not use dynamic allocation for these instances: there's always
one of each in SiFiveESoCState, so instead we just include them within
the struct.

Thanks to Peter for pointing out the bug and suggesting the fix!

Fixes: 30efbf330a45 ("SiFive RISC-V GPIO Device")
Signed-off-by: Palmer Dabbelt 
---
 hw/riscv/sifive_e.c | 12 +---
 include/hw/riscv/sifive_e.h |  2 ++
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
index 80ac56fa7d5e..83375afcd1d6 100644
--- a/hw/riscv/sifive_e.c
+++ b/hw/riscv/sifive_e.c
@@ -158,17 +158,15 @@ static void riscv_sifive_e_soc_realize(DeviceState *dev, 
Error **errp)
 
 SiFiveESoCState *s = RISCV_E_SOC(dev);
 MemoryRegion *sys_mem = get_system_memory();
-MemoryRegion *xip_mem = g_new(MemoryRegion, 1);
-MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
 
 object_property_set_bool(OBJECT(&s->cpus), true, "realized",
 &error_abort);
 
 /* Mask ROM */
-memory_region_init_rom(mask_rom, NULL, "riscv.sifive.e.mrom",
+memory_region_init_rom(&s->mask_rom, NULL, "riscv.sifive.e.mrom",
 memmap[SIFIVE_E_MROM].size, &error_fatal);
 memory_region_add_subregion(sys_mem,
-memmap[SIFIVE_E_MROM].base, mask_rom);
+memmap[SIFIVE_E_MROM].base, &s->mask_rom);
 
 /* MMIO */
 s->plic = sifive_plic_create(memmap[SIFIVE_E_PLIC].base,
@@ -228,10 +226,10 @@ static void riscv_sifive_e_soc_realize(DeviceState *dev, 
Error **errp)
 memmap[SIFIVE_E_PWM2].base, memmap[SIFIVE_E_PWM2].size);
 
 /* Flash memory */
-memory_region_init_ram(xip_mem, NULL, "riscv.sifive.e.xip",
+memory_region_init_ram(&s->xip_mem, NULL, "riscv.sifive.e.xip",
 memmap[SIFIVE_E_XIP].size, &error_fatal);
-memory_region_set_readonly(xip_mem, true);
-memory_region_add_subregion(sys_mem, memmap[SIFIVE_E_XIP].base, xip_mem);
+memory_region_set_readonly(&s->xip_mem, true);
+memory_region_add_subregion(sys_mem, memmap[SIFIVE_E_XIP].base, 
&s->xip_mem);
 }
 
 static void riscv_sifive_e_machine_init(MachineClass *mc)
diff --git a/include/hw/riscv/sifive_e.h b/include/hw/riscv/sifive_e.h
index 3b14eb74621f..d175b24cb209 100644
--- a/include/hw/riscv/sifive_e.h
+++ b/include/hw/riscv/sifive_e.h
@@ -33,6 +33,8 @@ typedef struct SiFiveESoCState {
 RISCVHartArrayState cpus;
 DeviceState *plic;
 SIFIVEGPIOState gpio;
+MemoryRegion xip_mem;
+MemoryRegion mask_rom;
 } SiFiveESoCState;
 
 typedef struct SiFiveEState {
-- 
2.21.0

Re: [Qemu-devel] [PATCH v2] hw: misc: Add Aspeed XDMA device

[Cédric Le Goater]

On 11/06/2019 22:54, Eddie James wrote:
> The XDMA engine embedded in the Aspeed SOCs performs PCI DMA operations
> between the SOC (acting as a BMC) and a host processor in a server.
> 
> The XDMA engine exists on the AST2400, AST2500, and AST2600 SOCs, so
> enable it for all of those. Add trace events on the important register
> writes in the XDMA engine.
> 
> Signed-off-by: Eddie James 
> ---
> Changes since v1:
>  - add trace events
>  - minor cleanup
> 
> This patch is based on Cedric's big Aspeed update:
> http://patchwork.ozlabs.org/cover/1105343/

Eddie, 

I have pushed the patch in my tree and I plan to resend 
as part of the patchset above.

Thanks,

C. 

>  hw/arm/aspeed_soc.c   |  19 +
>  hw/misc/Makefile.objs |   1 +
>  hw/misc/aspeed_xdma.c | 165 
> ++
>  hw/misc/trace-events  |   3 +
>  include/hw/arm/aspeed_soc.h   |   3 +
>  include/hw/misc/aspeed_xdma.h |  30 
>  6 files changed, 221 insertions(+)
>  create mode 100644 hw/misc/aspeed_xdma.c
>  create mode 100644 include/hw/misc/aspeed_xdma.h
> 
> diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
> index 0a0ab87..6901697 100644
> --- a/hw/arm/aspeed_soc.c
> +++ b/hw/arm/aspeed_soc.c
> @@ -31,6 +31,7 @@ static const hwaddr aspeed_soc_ast2400_memmap[] = {
>  [ASPEED_VIC]= 0x1E6C,
>  [ASPEED_SDMC]   = 0x1E6E,
>  [ASPEED_SCU]= 0x1E6E2000,
> +[ASPEED_XDMA]   = 0x1E6E7000,
>  [ASPEED_ADC]= 0x1E6E9000,
>  [ASPEED_SRAM]   = 0x1E72,
>  [ASPEED_GPIO]   = 0x1E78,
> @@ -57,6 +58,7 @@ static const hwaddr aspeed_soc_ast2500_memmap[] = {
>  [ASPEED_VIC]= 0x1E6C,
>  [ASPEED_SDMC]   = 0x1E6E,
>  [ASPEED_SCU]= 0x1E6E2000,
> +[ASPEED_XDMA]   = 0x1E6E7000,
>  [ASPEED_ADC]= 0x1E6E9000,
>  [ASPEED_SRAM]   = 0x1E72,
>  [ASPEED_GPIO]   = 0x1E78,
> @@ -90,6 +92,7 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
>  [ASPEED_VIC]= 0x1E6C,
>  [ASPEED_SDMC]   = 0x1E6E,
>  [ASPEED_SCU]= 0x1E6E2000,
> +[ASPEED_XDMA]   = 0x1E6E7000,
>  [ASPEED_ADC]= 0x1E6E9000,
>  [ASPEED_SRAM]   = 0x1E72,
>  [ASPEED_GPIO]   = 0x1E78,
> @@ -137,6 +140,7 @@ static const int aspeed_soc_ast2400_irqmap[] = {
>  [ASPEED_I2C]= 12,
>  [ASPEED_ETH1]   = 2,
>  [ASPEED_ETH2]   = 3,
> +[ASPEED_XDMA]   = 6,
>  };
>  
>  #define aspeed_soc_ast2500_irqmap aspeed_soc_ast2400_irqmap
> @@ -174,6 +178,7 @@ static const int aspeed_soc_ast2600_irqmap[] = {
>  [ASPEED_ETH2]   = 3,
>  [ASPEED_FSI1]   = 100,
>  [ASPEED_FSI2]   = 101,
> +[ASPEED_XDMA]   = 6,
>  };
>  
>  static const char *aspeed_soc_ast2400_typenames[] = { "aspeed.smc.spi" };
> @@ -359,6 +364,9 @@ static void aspeed_soc_init(Object *obj)
>  sysbus_init_child_obj(obj, "fsi[*]", OBJECT(&s->fsi[0]),
>sizeof(s->fsi[0]), TYPE_ASPEED_FSI);
>  }
> +
> +sysbus_init_child_obj(obj, "xdma", OBJECT(&s->xdma), sizeof(s->xdma),
> +  TYPE_ASPEED_XDMA);
>  }
>  
>  static void aspeed_soc_realize(DeviceState *dev, Error **errp)
> @@ -662,6 +670,17 @@ static void aspeed_soc_realize(DeviceState *dev, Error 
> **errp)
>  sysbus_connect_irq(SYS_BUS_DEVICE(&s->fsi[0]), 0,
> aspeed_soc_get_irq(s, ASPEED_FSI1));
>  }
> +
> +/* XDMA */
> +object_property_set_bool(OBJECT(&s->xdma), true, "realized", &err);
> +if (err) {
> +error_propagate(errp, err);
> +return;
> +}
> +sysbus_mmio_map(SYS_BUS_DEVICE(&s->xdma), 0,
> +sc->info->memmap[ASPEED_XDMA]);
> +sysbus_connect_irq(SYS_BUS_DEVICE(&s->xdma), 0,
> +   aspeed_soc_get_irq(s, ASPEED_XDMA));
>  }
>  
>  static void aspeed_soc_class_init(ObjectClass *oc, void *data)
> diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
> index d33c1c6..dc2b9c3 100644
> --- a/hw/misc/Makefile.objs
> +++ b/hw/misc/Makefile.objs
> @@ -78,6 +78,7 @@ obj-$(CONFIG_PVPANIC) += pvpanic.o
>  obj-$(CONFIG_AUX) += auxbus.o
>  obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o aspeed_ibt.o
>  obj-$(CONFIG_ASPEED_SOC) += aspeed_pwm.o aspeed_lpc.o aspeed_fsi.o
> +obj-$(CONFIG_ASPEED_SOC) += aspeed_xdma.o
>  obj-$(CONFIG_MSF2) += msf2-sysreg.o
>  obj-$(CONFIG_NRF51_SOC) += nrf51_rng.o
>  
> diff --git a/hw/misc/aspeed_xdma.c b/hw/misc/aspeed_xdma.c
> new file mode 100644
> index 000..eebd4ad
> --- /dev/null
> +++ b/hw/misc/aspeed_xdma.c
> @@ -0,0 +1,165 @@
> +/*
> + * ASPEED XDMA Controller
> + * Eddie James 
> + *
> + * Copyright (C) 2019 IBM Corp
> + * SPDX-License-Identifer: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "qemu/error-report.h"
> +#include "hw/misc/aspeed_xdma.h"
> +#include "qapi/error.h"
> +
> +#include "trace.h"
> +
> +#define XDMA_BMC_CMDQ_ADDR 0x10
> +#define XDMA_BMC_CMDQ_ENDP  

Re: [Qemu-devel] [PATCH] RISC-V: Fix a memory leak when realizing a sifive_e

[Philippe Mathieu-Daudé]

On 6/14/19 2:08 PM, Palmer Dabbelt wrote:
> Coverity pointed out a memory leak in riscv_sifive_e_soc_realize(),
> where a pair of recently added MemoryRegion instances would not be freed
> if there were errors elsewhere in the function.  The fix here is to
> simply not use dynamic allocation for these instances: there's always
> one of each in SiFiveESoCState, so instead we just include them within
> the struct.
> 
> Thanks to Peter for pointing out the bug and suggesting the fix!

a.k.a. Suggested-by: Peter Maydell 

Maybe the thanks can go below the '---' tag, so it doesn't stay in the
git history.

> 
> Fixes: 30efbf330a45 ("SiFive RISC-V GPIO Device")
> Signed-off-by: Palmer Dabbelt 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  hw/riscv/sifive_e.c | 12 +---
>  include/hw/riscv/sifive_e.h |  2 ++
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
> index 80ac56fa7d5e..83375afcd1d6 100644
> --- a/hw/riscv/sifive_e.c
> +++ b/hw/riscv/sifive_e.c
> @@ -158,17 +158,15 @@ static void riscv_sifive_e_soc_realize(DeviceState 
> *dev, Error **errp)
>  
>  SiFiveESoCState *s = RISCV_E_SOC(dev);
>  MemoryRegion *sys_mem = get_system_memory();
> -MemoryRegion *xip_mem = g_new(MemoryRegion, 1);
> -MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
>  
>  object_property_set_bool(OBJECT(&s->cpus), true, "realized",
>  &error_abort);
>  
>  /* Mask ROM */
> -memory_region_init_rom(mask_rom, NULL, "riscv.sifive.e.mrom",
> +memory_region_init_rom(&s->mask_rom, NULL, "riscv.sifive.e.mrom",
>  memmap[SIFIVE_E_MROM].size, &error_fatal);
>  memory_region_add_subregion(sys_mem,
> -memmap[SIFIVE_E_MROM].base, mask_rom);
> +memmap[SIFIVE_E_MROM].base, &s->mask_rom);
>  
>  /* MMIO */
>  s->plic = sifive_plic_create(memmap[SIFIVE_E_PLIC].base,
> @@ -228,10 +226,10 @@ static void riscv_sifive_e_soc_realize(DeviceState 
> *dev, Error **errp)
>  memmap[SIFIVE_E_PWM2].base, memmap[SIFIVE_E_PWM2].size);
>  
>  /* Flash memory */
> -memory_region_init_ram(xip_mem, NULL, "riscv.sifive.e.xip",
> +memory_region_init_ram(&s->xip_mem, NULL, "riscv.sifive.e.xip",
>  memmap[SIFIVE_E_XIP].size, &error_fatal);
> -memory_region_set_readonly(xip_mem, true);
> -memory_region_add_subregion(sys_mem, memmap[SIFIVE_E_XIP].base, xip_mem);
> +memory_region_set_readonly(&s->xip_mem, true);
> +memory_region_add_subregion(sys_mem, memmap[SIFIVE_E_XIP].base, 
> &s->xip_mem);
>  }
>  
>  static void riscv_sifive_e_machine_init(MachineClass *mc)
> diff --git a/include/hw/riscv/sifive_e.h b/include/hw/riscv/sifive_e.h
> index 3b14eb74621f..d175b24cb209 100644
> --- a/include/hw/riscv/sifive_e.h
> +++ b/include/hw/riscv/sifive_e.h
> @@ -33,6 +33,8 @@ typedef struct SiFiveESoCState {
>  RISCVHartArrayState cpus;
>  DeviceState *plic;
>  SIFIVEGPIOState gpio;
> +MemoryRegion xip_mem;
> +MemoryRegion mask_rom;
>  } SiFiveESoCState;
>  
>  typedef struct SiFiveEState {
> 

Re: [Qemu-devel] [PATCH v2] hw: misc: Add Aspeed XDMA device

[Philippe Mathieu-Daudé]

On 6/14/19 2:10 PM, Cédric Le Goater wrote:
> On 11/06/2019 22:54, Eddie James wrote:
>> The XDMA engine embedded in the Aspeed SOCs performs PCI DMA operations
>> between the SOC (acting as a BMC) and a host processor in a server.
>>
>> The XDMA engine exists on the AST2400, AST2500, and AST2600 SOCs, so
>> enable it for all of those. Add trace events on the important register
>> writes in the XDMA engine.
>>
>> Signed-off-by: Eddie James 
>> ---
>> Changes since v1:
>>  - add trace events
>>  - minor cleanup

Thank you for these changes.

>>
>> This patch is based on Cedric's big Aspeed update:
>> http://patchwork.ozlabs.org/cover/1105343/
> 
> Eddie, 
> 
> I have pushed the patch in my tree and I plan to resend 
> as part of the patchset above.
> 
> Thanks,
> 
> C. 

Reviewed-by: Philippe Mathieu-Daudé 

Re: [Qemu-devel] [PATCH v6] ssh: switch from libssh2 to libssh

[Philippe Mathieu-Daudé]

On 6/14/19 2:29 PM, Stefan Weil wrote:
> On 14.06.19 12:13, Philippe Mathieu-Daudé wrote:
>> I agree with Kevin. The only user of the 'ssh' block driver that I am
>> aware of is the virt-v2v tool:
>>
>> http://libguestfs.org/virt-v2v.1.html#convert-from-esxi-hypervisor-over-ssh-to-local-libvirt
>>
>> Stefan, do you think someone would use it on a Windows host?
> 
> 
> I simply don't know. Typically people contact me if something does not
> work. Rarely they send an e-mail to say what they do and that everything
> is fine.
> 
> If QEMU for Windows builds without libssl, I'll build binaries without

"libssh" ;)

> it, add that information to the release notes and wait what happens.
> 
> So no objection from my side.

Glad to hear, thanks!

Pino: Can you improve the commit message to explain that QEMU only uses
libssh for the 'ssh block driver'? Maybe you (or Kevin/Max) can also add
a 1 line description of what is a 'block driver', so reviewer are not
worried that a feature might be removed.

Thanks!

Phil.

Re: [Qemu-devel] [PATCH v6] ssh: switch from libssh2 to libssh

[Stefan Weil]

On 14.06.19 12:13, Philippe Mathieu-Daudé wrote:
> I agree with Kevin. The only user of the 'ssh' block driver that I am
> aware of is the virt-v2v tool:
> 
> http://libguestfs.org/virt-v2v.1.html#convert-from-esxi-hypervisor-over-ssh-to-local-libvirt
> 
> Stefan, do you think someone would use it on a Windows host?


I simply don't know. Typically people contact me if something does not
work. Rarely they send an e-mail to say what they do and that everything
is fine.

If QEMU for Windows builds without libssl, I'll build binaries without
it, add that information to the release notes and wait what happens.

So no objection from my side.

Cheers
Stefan

Re: [Qemu-devel] [PATCH 18/19] aspeed/smc: inject errors in DMA checksum

[Philippe Mathieu-Daudé]

Cc'ing Markus & Marc-André

On 6/14/19 2:02 PM, Cédric Le Goater wrote:
> On 13/06/2019 16:31, Philippe Mathieu-Daudé wrote:
>> Hi Cédric,
>>
>> On 5/25/19 5:12 PM, Cédric Le Goater wrote:
>>> Emulate read errors in the DMA Checksum Register for high frequencies
>>> and optimistic settings of the Read Timing Compensation Register. This
>>> will help in tuning the SPI timing calibration algorithm.
>>>
>>> The values below are those to expect from the first flash device of
>>> the FMC controller of a palmetto-bmc machine.
>>>
>>> Signed-off-by: Cédric Le Goater 
>>> ---
>>>  hw/ssi/aspeed_smc.c | 29 +
>>>  1 file changed, 29 insertions(+)
>>>
>>> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
>>> index 406c30c60b3f..4c162912cf62 100644
>>> --- a/hw/ssi/aspeed_smc.c
>>> +++ b/hw/ssi/aspeed_smc.c
>>> @@ -866,6 +866,30 @@ static void aspeed_smc_dma_calibration(AspeedSMCState 
>>> *s)
>>>  s->regs[s->r_ctrl0 + cs] |= CE_CTRL_CLOCK_FREQ(hclk_div);
>>>  }
>>>  
>>
>> Can you add a comment (like the patch description) here?
> 
> yes. done.
>  
>>> +static bool aspeed_smc_inject_read_failure(AspeedSMCState *s)
>>> +{
>>> +uint8_t delay =
>>> +(s->regs[R_DMA_CTRL] >> DMA_CTRL_DELAY_SHIFT) & 
>>> DMA_CTRL_DELAY_MASK;
>>> +uint8_t hclk_mask =
>>> +(s->regs[R_DMA_CTRL] >> DMA_CTRL_FREQ_SHIFT) & DMA_CTRL_FREQ_MASK;
>>> +
>>> +/*
>>> + * Typical values of a palmetto-bmc machine.
>>> + */
>>> +switch (aspeed_smc_hclk_divisor(hclk_mask)) {
>>> +case 4 ... 16:
>>> +return false;
>>> +case 3: /* at least one HCLK cycle delay */
>>> +return (delay & 0x7) < 1;
>>> +case 2: /* at least two HCLK cycle delay */
>>> +return (delay & 0x7) < 2;
>>> +case 1: /* (> 100MHz) is above the max freq of the controller */
>>> +return true;
>>> +default:
>>> +g_assert_not_reached();
>>> +}
>>> +}
>>> +
>>>  /*
>>>   * Accumulate the result of the reads to provide a checksum that will
>>>   * be used to validate the read timing settings.
>>> @@ -903,6 +927,11 @@ static void aspeed_smc_dma_checksum(AspeedSMCState *s)
>>>  s->regs[R_DMA_FLASH_ADDR] += 4;
>>>  s->regs[R_DMA_LEN] -= 4;
>>>  }
>>> +
>>> +if (aspeed_smc_inject_read_failure(s)) {
>>
>> So this model real world where noise eventually triggers errors. Don't
>> we want this to be enable by the user (or a QMP command)?
> 
> I can add a property at the device model level to trigger this behavior.
> Such as :
> 
>-global driver=aspeed.smc,property=timing,value=true
> 
> timing if defined would provide the maximum clock and delay settings.

I was thinking of a simpler:

-global driver=aspeed.smc,property=inject_failure,value=true

Then

if (s->inject_failure && aspeed_smc_inject_read_failure(s)) {
s->regs[R_DMA_CHECKSUM] = 0xbadc0de;
}

> 
> Are there any other examples in QEMU ?

I think most of them are hidden in the codebase...

> 
> Thanks,
> 
> C.
> 
>>
>>> +s->regs[R_DMA_CHECKSUM] = 0xbadc0de;
>>> +}
>>> +
>>>  }
>>>  
>>>  static void aspeed_smc_dma_rw(AspeedSMCState *s)
>>>
>