[PATCH] meson: skip SDL2 detection if --disable-system

[Paolo Bonzini]

SDL is only used for system emulation; avoid spurious warnings for
static --disable-system emulation by skipping the detection of
the library if there are no system emulation targets.

Reported-by: Peter Maydell 
Signed-off-by: Paolo Bonzini 
---
 meson.build | 27 ---
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/meson.build b/meson.build
index 57c2fe2b65..19d4d42512 100644
--- a/meson.build
+++ b/meson.build
@@ -21,6 +21,16 @@ qemu_datadir = get_option('datadir') + 
get_option('confsuffix')
 config_host_data = configuration_data()
 genh = []
 
+target_dirs = config_host['TARGET_DIRS'].split()
+have_user = false
+have_system = false
+foreach target : target_dirs
+  have_user = have_user or target.endswith('-user')
+  have_system = have_system or target.endswith('-softmmu')
+endforeach
+have_tools = 'CONFIG_TOOLS' in config_host
+have_block = have_system or have_tools
+
 add_project_arguments(config_host['QEMU_CFLAGS'].split(),
   native: false, language: ['c', 'objc'])
 add_project_arguments(config_host['QEMU_CXXFLAGS'].split(),
@@ -225,9 +235,12 @@ if 'CONFIG_BRLAPI' in config_host
   brlapi = declare_dependency(link_args: config_host['BRLAPI_LIBS'].split())
 endif
 
-sdl = dependency('sdl2', required: get_option('sdl'), static: enable_static,
- include_type: 'system')
-sdl_image = not_found
+sdl = not_found
+if have_system
+  sdl = dependency('sdl2', required: get_option('sdl'), static: enable_static,
+   include_type: 'system')
+  sdl_image = not_found
+endif
 if sdl.found()
   # work around 2.0.8 bug
   sdl = declare_dependency(compile_args: '-Wno-undef',
@@ -423,9 +436,6 @@ endforeach
 genh += configure_file(output: 'config-host.h', configuration: 
config_host_data)
 
 minikconf = find_program('scripts/minikconf.py')
-target_dirs = config_host['TARGET_DIRS'].split()
-have_user = false
-have_system = false
 config_devices_mak_list = []
 config_devices_h = {}
 config_target_h = {}
@@ -446,7 +456,6 @@ kconfig_external_symbols = [
 ]
 ignored = ['TARGET_XML_FILES', 'TARGET_ABI_DIR', 'TARGET_DIRS']
 foreach target : target_dirs
-  have_user = have_user or target.endswith('-user')
   config_target = keyval.load(meson.current_build_dir() / target / 
'config-target.mak')
 
   config_target_data = configuration_data()
@@ -469,8 +478,6 @@ foreach target : target_dirs
configuration: 
config_target_data)}
 
   if target.endswith('-softmmu')
-have_system = true
-
 base_kconfig = []
 foreach sym : kconfig_external_symbols
   if sym in config_target or sym in config_host
@@ -500,8 +507,6 @@ foreach target : target_dirs
   endif
   config_target_mak += {target: config_target}
 endforeach
-have_tools = 'CONFIG_TOOLS' in config_host
-have_block = have_system or have_tools
 
 grepy = find_program('scripts/grepy.sh')
 # This configuration is used to build files that are shared by
-- 
2.26.2

[PATCH] configure: add --ninja option

[Paolo Bonzini]

On Windows it is not possible to invoke a Python script as $NINJA.
If ninja is present use it directly, while if it is not we can
keep using ninjatool.

Reported-by: Mark Cave-Ayland 
Signed-off-by: Paolo Bonzini 
---
 configure | 16 +++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 51b6164f69..340d01e185 100755
--- a/configure
+++ b/configure
@@ -568,6 +568,7 @@ rng_none="no"
 secret_keyring=""
 libdaxctl=""
 meson=""
+ninja=""
 skip_meson=no
 gettext=""
 
@@ -1052,6 +1053,8 @@ for opt do
   ;;
   --meson=*) meson="$optarg"
   ;;
+  --ninja=*) ninja="$optarg"
+  ;;
   --smbd=*) smbd="$optarg"
   ;;
   --extra-cflags=*)
@@ -1820,6 +1823,7 @@ Advanced options (experts only):
   --python=PYTHON  use specified python [$python]
   --sphinx-build=SPHINXuse specified sphinx-build [$sphinx_build]
   --meson=MESONuse specified meson [$meson]
+  --ninja=NINJAuse specified ninja [$ninja]
   --smbd=SMBD  use specified smbd [$smbd]
   --with-git=GIT   use specified git [$git]
   --static enable static build [$static]
@@ -2058,6 +2062,16 @@ case "$meson" in
 *) meson=$(command -v meson) ;;
 esac
 
+# Probe for ninja (used for compdb)
+
+if test -z "$ninja"; then
+for c in ninja ninja-build samu; do
+if has $c; then
+ninja="$(command -v ninja)"
+break
+fi
+done
+fi
 
 # Check that the C compiler works. Doing this here before testing
 # the host CPU ensures that we had a valid CC to autodetect the
@@ -8194,7 +8208,7 @@ fi
 mv $cross config-meson.cross
 
 rm -rf meson-private meson-info meson-logs
-NINJA=$PWD/ninjatool $meson setup \
+NINJA=${ninja-$PWD/ninjatool} $meson setup \
 --prefix "${pre_prefix}$prefix" \
 --libdir "${pre_prefix}$libdir" \
 --libexecdir "${pre_prefix}$libexecdir" \
-- 
2.26.2

[PATCH] meson: move pixman detection to meson

[Paolo Bonzini]

When pixman is not installed (or too old), but virglrenderer is available
and "configure" has been run with "--disable-system", the build currently
aborts when trying to compile vhost-user-gpu (since it requires pixman).

Let's skip the build of vhost-user-gpu when pixman is not installed or
too old.  Instead of adding CONFIG_PIXMAN, it is simpler to move the
detection to pixman.

Based on a patch by Thomas Huth. 

Fixes: 9b52b17ba5 ("configure: Allow to build tools without pixman")
Reported-by: Rafael Kitover 
Reported-by: Philippe Mathieu-Daudé 
Signed-off-by: Paolo Bonzini 
---
 configure  | 21 ++---
 contrib/vhost-user-gpu/meson.build |  3 ++-
 meson.build| 12 +++-
 3 files changed, 11 insertions(+), 25 deletions(-)

diff --git a/configure b/configure
index a5fa472c64..51b6164f69 100755
--- a/configure
+++ b/configure
@@ -3923,20 +3923,6 @@ if test "$modules" = yes; then
 fi
 fi
 
-##
-# pixman support probe
-
-if test "$softmmu" = "no"; then
-  pixman_cflags=
-  pixman_libs=
-elif $pkg_config --atleast-version=0.21.8 pixman-1 > /dev/null 2>&1; then
-  pixman_cflags=$($pkg_config --cflags pixman-1)
-  pixman_libs=$($pkg_config --libs pixman-1)
-else
-  error_exit "pixman >= 0.21.8 not present." \
-  "Please install the pixman devel package."
-fi
-
 ##
 # libmpathpersist probe
 
@@ -6649,8 +6635,8 @@ echo_version() {
 fi
 }
 
-# prepend pixman and ftd flags after all config tests are done
-QEMU_CFLAGS="$pixman_cflags $fdt_cflags $QEMU_CFLAGS"
+# prepend ftd flags after all config tests are done
+QEMU_CFLAGS="$fdt_cflags $QEMU_CFLAGS"
 QEMU_LDFLAGS="$fdt_ldflags $QEMU_LDFLAGS"
 
 config_host_mak="config-host.mak"
@@ -8053,9 +8039,6 @@ fi
 
 done # for target in $targets
 
-echo "PIXMAN_CFLAGS=$pixman_cflags" >> $config_host_mak
-echo "PIXMAN_LIBS=$pixman_libs" >> $config_host_mak
-
 if [ "$fdt" = "git" ]; then
   subdirs="$subdirs dtc"
 fi
diff --git a/contrib/vhost-user-gpu/meson.build 
b/contrib/vhost-user-gpu/meson.build
index 8df4c13bc5..7d9b29da8b 100644
--- a/contrib/vhost-user-gpu/meson.build
+++ b/contrib/vhost-user-gpu/meson.build
@@ -1,5 +1,6 @@
 if 'CONFIG_TOOLS' in config_host and 'CONFIG_VIRGL' in config_host \
-and 'CONFIG_GBM' in config_host and 'CONFIG_LINUX' in config_host
+and 'CONFIG_GBM' in config_host and 'CONFIG_LINUX' in config_host \
+and pixman.found()
   executable('vhost-user-gpu', files('vhost-user-gpu.c', 'virgl.c', 'vugbm.c'),
  link_with: libvhost_user,
  dependencies: [qemuutil, pixman, gbm, virgl],
diff --git a/meson.build b/meson.build
index bcd39b39da..57c2fe2b65 100644
--- a/meson.build
+++ b/meson.build
@@ -114,8 +114,11 @@ if 'CONFIG_GNUTLS' in config_host
   gnutls = declare_dependency(compile_args: 
config_host['GNUTLS_CFLAGS'].split(),
   link_args: config_host['GNUTLS_LIBS'].split())
 endif
-pixman = declare_dependency(compile_args: config_host['PIXMAN_CFLAGS'].split(),
-link_args: config_host['PIXMAN_LIBS'].split())
+pixman = not_found
+if have_system or have_tools
+  pixman = dependency('pixman', required: have_system, version:'>=0.21.8',
+  static: enable_static)
+endif
 pam = not_found
 if 'CONFIG_AUTH_PAM' in config_host
   pam = cc.find_library('pam')
@@ -1094,9 +1097,7 @@ if have_tools
   if 'CONFIG_VHOST_USER' in config_host
 subdir('contrib/libvhost-user')
 subdir('contrib/vhost-user-blk')
-if 'CONFIG_LINUX' in config_host
-  subdir('contrib/vhost-user-gpu')
-endif
+subdir('contrib/vhost-user-gpu')
 subdir('contrib/vhost-user-input')
 subdir('contrib/vhost-user-scsi')
   endif
@@ -1303,6 +1304,7 @@ summary_info += {'SDL image support': sdl_image.found()}
 # TODO: add back version
 summary_info += {'GTK support':   config_host.has_key('CONFIG_GTK')}
 summary_info += {'GTK GL support':config_host.has_key('CONFIG_GTK_GL')}
+summary_info += {'pixman':pixman.found()}
 # TODO: add back version
 summary_info += {'VTE support':   config_host.has_key('CONFIG_VTE')}
 summary_info += {'TLS priority':  config_host['CONFIG_TLS_PRIORITY']}
-- 
2.26.2

Re: [PATCH v5 07/10] block: introduce preallocate filter

[Vladimir Sementsov-Ogievskiy]

24.08.2020 20:52, Vladimir Sementsov-Ogievskiy wrote:

21.08.2020 17:11, Vladimir Sementsov-Ogievskiy wrote:

It's intended to be inserted between format and protocol nodes to
preallocate additional space (expanding protocol file) on writes
crossing EOF. It improves performance for file-systems with slow
allocation.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  docs/system/qemu-block-drivers.rst.inc |  26 +++
  qapi/block-core.json   |  20 +-
  block/preallocate.c    | 291 +
  block/Makefile.objs    |   1 +
  4 files changed, 337 insertions(+), 1 deletion(-)
  create mode 100644 block/preallocate.c

diff --git a/docs/system/qemu-block-drivers.rst.inc 
b/docs/system/qemu-block-drivers.rst.inc
index b052a6d14e..5e8a35c571 100644
--- a/docs/system/qemu-block-drivers.rst.inc
+++ b/docs/system/qemu-block-drivers.rst.inc
@@ -952,3 +952,29 @@ on host and see if there are locks held by the QEMU 
process on the image file.
  More than one byte could be locked by the QEMU instance, each byte of which
  reflects a particular permission that is acquired or protected by the running
  block driver.
+


[..]


+
+static coroutine_fn int preallocate_co_preadv_part(
+    BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+    QEMUIOVector *qiov, size_t qiov_offset, int flags)
+{
+    return bdrv_co_preadv_part(bs->file, offset, bytes, qiov, qiov_offset,
+   flags);
+}
+
+static int coroutine_fn preallocate_co_pdiscard(BlockDriverState *bs,
+   int64_t offset, int bytes)
+{
+    return bdrv_co_pdiscard(bs->file, offset, bytes);
+}
+
+static bool coroutine_fn do_preallocate(BlockDriverState *bs, int64_t offset,
+   int64_t bytes, bool write_zero)
+{
+    BDRVPreallocateState *s = bs->opaque;
+    int64_t len, start, end;


int64_t old_data_end = s->data_end;


+
+    if (!s->active) {
+    return false;
+    }
+
+    if (s->data_end >= 0) {
+    s->data_end = MAX(s->data_end,
+  QEMU_ALIGN_UP(offset + bytes, BDRV_SECTOR_SIZE));
+    }
+
+    len = bdrv_getlength(bs->file->bs);
+    if (len < 0) {
+    return false;


return old_data_end >= 0 && offset >= old_data_end;


And with this small improvement we make the following test 20% faster (ssd, 
ext4):

./qemu-img create -f qcow2 $img 16G; ./qemu-img bench -c 15 -d 64 -f qcow2  
-s 16k -t none -n -w $img;

(assume additional patch which inserts the filter).

Great! So, preallocation is generally good idea, not only for vstorage.

What about inserting the filter by default?

I'll come tomorrow with more complete test results.



Some results:

the two commands to compare:
img=/ssd/x.qcow2; for i in {1..5}; do ./qemu-img create -f qcow2 $img 16G; 
./qemu-img bench -c 15 -d 64 -s 16k -t none -n -w --image-opts 
driver=qcow2,file.filename=$img; done
img=/ssd/x.qcow2; for i in {1..5}; do ./qemu-img create -f qcow2 $img 16G; 
./qemu-img bench -c 15 -d 64 -s 16k -t none -n -w --image-opts 
driver=qcow2,file.driver=preallocate,file.file.filename=$img; done


  no-filter  with-filter
ssd
  ext4: 6.94s  5.53s (-20%)
  xfs:  6.9s   25s (+262%)
hdd (with -c 45000)
  ext4: 8.23s  12.75s (+55%)
  xfs:  7.79s  25.4s (+226%)

vstorage (some custom distributed fs), with -c 4000 over ext4 over ssd: 42.9s 
~> 0.27s, more than 150 times faster with filter!
same, with -c 2000 over ext4 over hdd: 22.8s ~> 0.53s, ~43 times faster.


So, we do have large improvement for the vstorage (some custom distributed fs), 
which is our main target. And there is a significant improvement for ext4 over 
ssd. And (a lot more) significant degradation in other cases. For sure, we 
can't insert the filter by default, but it is useful.


--
Best regards,
Vladimir

Re: [PATCH v3 48/74] s390-virtio-ccw: Rename S390_MACHINE_CLASS macro

[Cornelia Huck]

On Tue, 25 Aug 2020 15:20:44 -0400
Eduardo Habkost  wrote:

> Rename it to be consistent with S390_CCW_MACHINE and
> TYPE_S390_CCW_MACHINE.
> 
> This will make future conversion to OBJECT_DECLARE* easier.
> 
> Signed-off-by: Eduardo Habkost 
> ---
> Changes series v2 -> v3: new patch added to series v3
> 
> ---
> Cc: Richard Henderson 
> Cc: David Hildenbrand 
> Cc: Cornelia Huck 
> Cc: Thomas Huth 
> Cc: Halil Pasic 
> Cc: Christian Borntraeger 
> Cc: qemu-s3...@nongnu.org
> Cc: qemu-devel@nongnu.org
> ---
>  include/hw/s390x/s390-virtio-ccw.h |  2 +-
>  hw/s390x/s390-virtio-ccw.c | 14 +++---
>  2 files changed, 8 insertions(+), 8 deletions(-)

Acked-by: Cornelia Huck 

Re: [PATCH 1/4] meson: Fixes the ninjatool issue that E$$: are generated in Makefile.ninja SIMPLE_PATH_RE should match the full path token. Or the $ and : contained in path would not matched if the pa

[Markus Armbruster]

Please do not put all of your commit message in the subject line.

https://wiki.qemu.org/Contribute/SubmitAPatch#Write_a_meaningful_commit_message

https://chris.beams.io/posts/git-commit/

[PATCH V2 01/10] qemu/: fix some comment spelling errors

[zhaolichang]

I found that there are many spelling errors in the comments of qemu,
so I used the spellcheck tool to check the spelling errors
and finally found some spelling errors in the folder.

Signed-off-by: zhaolichang 
Reviewed-by: Alex Bennée 
---
Changelog   | 2 +-
accel/tcg/user-exec.c   | 2 +-
audio/audio.c   | 2 +-
block.c | 2 +-
configure   | 2 +-
fsdev/virtfs-proxy-helper.c | 2 +-
hmp-commands.hx | 2 +-
libdecnumber/decNumber.c| 2 +-
qemu-img.c  | 2 +-
qobject/qdict.c | 2 +-
scsi/pr-manager-helper.c| 2 +-
11 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/Changelog b/Changelog
index 4a90bb9e8b..f7e178ccc0 100644
--- a/Changelog
+++ b/Changelog
@@ -241,7 +241,7 @@ version 0.8.0:
version 0.7.2:

   - x86_64 fixes (Win2000 and Linux 2.6 boot in 32 bit)
-  - merge self modifying code handling in dirty ram page mecanism.
+  - merge self modifying code handling in dirty ram page mechanism.
   - MIPS fixes (Ralf Baechle)
   - better user net performances

diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index bb039eb32d..5c96819ded 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -88,7 +88,7 @@ static inline int handle_cpu_signal(uintptr_t pc, siginfo_t 
*info,
  * use that value directly.  Within cpu_restore_state_from_tb, we
  * assume PC comes from GETPC(), as used by the helper functions,
  * so we adjust the address by -GETPC_ADJ to form an address that
- * is within the call insn, so that the address does not accidentially
+ * is within the call insn, so that the address does not accidentally
  * match the beginning of the next guest insn.  However, when the
  * pc comes from the signal frame it points to the actual faulting
  * host memory insn and not the return from a call insn.
diff --git a/audio/audio.c b/audio/audio.c
index ce8c6dec5f..1a68cfaafb 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1674,7 +1674,7 @@ static AudioState *audio_init(Audiodev *dev, const char 
*name)
 head = audio_handle_legacy_opts();
 /*
  * In case of legacy initialization, all Audiodevs in the list will 
have
- * the same configuration (except the driver), so it does't matter 
which
+ * the same configuration (except the driver), so it doesn't matter 
which
  * one we chose.  We need an Audiodev to set up AudioState before we 
can
  * init a driver.  Also note that dev at this point is still in the
  * list.
diff --git a/block.c b/block.c
index 2ba76b2c36..8f067c9b95 100644
--- a/block.c
+++ b/block.c
@@ -2604,7 +2604,7 @@ static void bdrv_replace_child_noperm(BdrvChild *child,

/*
  * Updates @child to change its reference to point to @new_bs, including
- * checking and applying the necessary permisson updates both to the old node
+ * checking and applying the necessary permission updates both to the old node
  * and to @new_bs.
  *
 * NULL is passed as @new_bs for removing the reference before freeing @child.
diff --git a/configure b/configure
index b1e11397a8..19360981f6 100755
--- a/configure
+++ b/configure
@@ -3462,7 +3462,7 @@ EOF
 xfs="yes"
   else
 if test "$xfs" = "yes" ; then
-  feature_not_found "xfs" "Instal xfsprogs/xfslibs devel"
+  feature_not_found "xfs" "Install xfsprogs/xfslibs devel"
 fi
 xfs=no
   fi
diff --git a/fsdev/virtfs-proxy-helper.c b/fsdev/virtfs-proxy-helper.c
index de061a8a0e..15c0e79b06 100644
--- a/fsdev/virtfs-proxy-helper.c
+++ b/fsdev/virtfs-proxy-helper.c
@@ -518,7 +518,7 @@ static void statfs_to_prstatfs(ProxyStatFS *pr_stfs, struct 
statfs *stfs)

/*
  * Gets stat/statfs information and packs in out_iovec structure
- * on success returns number of bytes packed in out_iovec struture
+ * on success returns number of bytes packed in out_iovec structure
  * otherwise returns -errno
  */
static int do_stat(int type, struct iovec *iovec, struct iovec *out_iovec)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 60f395c276..27c4bbe0f2 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1267,7 +1267,7 @@ ERST
 },
SRST
``drive_backup``
-  Start a point-in-time copy of a block device to a specificed target.
+  Start a point-in-time copy of a block device to a specified target.
ERST

 {
diff --git a/libdecnumber/decNumber.c b/libdecnumber/decNumber.c
index 8c197023f4..1ffe458ad8 100644
--- a/libdecnumber/decNumber.c
+++ b/libdecnumber/decNumber.c
@@ -5626,7 +5626,7 @@ static const uShort LNnn[90] = {
/*would certainly save at least one if it were made ten times */
/*bigger, too (for truncated fractions 0.100 through 0.999).  */
/*However, for most practical evaluations, at least four or five  */
-/*iterations will be neede -- so this would only speed up by  */
+/*iterations will be needed -- so this would only speed up by  */
/*20-25% a

[PATCH V2 00/10] fix some comment spelling errors

[zhaolichang]

I found that there are many spelling errors in the comments of qemu,
so I used the spellcheck tool to check the spelling errors, this series fixed 
this
spelling errors.

v1 -> v2:
  address Peter Maydell's comments
  address Alex Bennée's comments
  add reviewed-by for patch 1,2,3,4,5,6,9,10

zhaolichang (10):
  contrib/: fix some comment spelling errors
  disas/: fix some comment spelling errors
  block/: fix some comment spelling errors
  linux-user/: fix some comment spelling errors
  util/: fix some comment spelling errors
  scripts/: fix some comment spelling errors
  docs/: fix some comment spelling errors
  migration/: fix some comment spelling errors
  qemu/: fix some comment spelling errors

Changelog|  2 +-
accel/tcg/user-exec.c|  2 +-
audio/audio.c|  2 +-
block.c  |  2 +-
block/block-copy.c   |  2 +-
block/linux-aio.c|  2 +-
block/mirror.c   |  2 +-
block/vhdx.c |  2 +-
block/vhdx.h |  4 ++--
configure|  2 +-
contrib/gitdm/filetypes.txt  |  6 +++---
contrib/ivshmem-client/ivshmem-client.h  |  2 +-
contrib/libvhost-user/libvhost-user.c|  4 ++--
contrib/libvhost-user/libvhost-user.h|  2 +-
disas/hppa.c |  2 +-
disas/m68k.c |  8 
disas/ppc.c  |  2 +-
docs/COLO-FT.txt |  6 +++---
docs/devel/blkdebug.txt  |  2 +-
docs/devel/migration.rst |  2 +-
docs/devel/testing.rst   |  2 +-
docs/devel/tracing.txt   |  2 +-
docs/interop/bitmaps.rst |  2 +-
docs/interop/dbus.rst|  4 ++--
docs/interop/nbd.txt |  2 +-
docs/interop/vhost-user-gpu.rst  |  2 +-
docs/interop/vhost-user.rst  |  4 ++--
docs/rdma.txt|  2 +-
docs/specs/ppc-spapr-hotplug.txt |  4 ++--
docs/specs/ppc-spapr-xive.rst|  4 ++--
docs/system/arm/aspeed.rst   |  2 +-
docs/system/deprecated.rst   |  8 
docs/system/target-avr.rst   |  4 ++--
docs/tools/virtiofsd.rst |  2 +-
fsdev/virtfs-proxy-helper.c  |  2 +-
hmp-commands.hx  |  2 +-
libdecnumber/decNumber.c |  2 +-
linux-user/aarch64/signal.c  |  2 +-
linux-user/cris/target_syscall.h |  4 ++--
linux-user/flat.h|  2 +-
linux-user/flatload.c|  4 ++--
linux-user/host/ppc64/safe-syscall.inc.S |  2 +-
linux-user/syscall.c |  4 ++--
migration/colo-failover.c|  2 +-
migration/colo.c |  2 +-
migration/multifd.c  |  4 ++--
migration/postcopy-ram.c |  4 ++--
migration/postcopy-ram.h |  2 +-
migration/ram.c  | 10 +-
migration/rdma.c |  8 
migration/savevm.c   |  4 ++--
qapi/block-core.json |  4 ++--
qapi/crypto.json |  4 ++--
qemu-img.c   |  2 +-
qobject/qdict.c  |  2 +-
scripts/checkpatch.pl|  2 +-
scripts/clean-header-guards.pl   |  2 +-
scripts/decodetree.py|  6 +++---
scripts/oss-fuzz/build.sh|  2 +-
scripts/tracetool/__init__.py|  2 +-
scsi/pr-manager-helper.c |  2 +-
util/osdep.c |  2 +-
util/qemu-progress.c |  2 +-
util/qemu-sockets.c  |  2 +-
util/qemu-thread-win32.c |  2 +-
util/qht.c   |  2 +-
util/trace-events|  2 +-
67 files changed, 101 insertions(+), 101 deletions(-)

--
2.26.2.windows.1

[PATCH V2 04/10] scripts/: fix some comment spelling errors

[zhaolichang]

I found that there are many spelling errors in the comments of qemu,
so I used the spellcheck tool to check the spelling errors
and finally found some spelling errors in the scripts folder.

Signed-off-by: zhaolichang 
Reviewed-by: Peter Maydell 
---
scripts/checkpatch.pl  | 2 +-
scripts/clean-header-guards.pl | 2 +-
scripts/decodetree.py  | 6 +++---
scripts/oss-fuzz/build.sh  | 2 +-
scripts/tracetool/__init__.py  | 2 +-
5 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index bd3faa154c..50910899f2 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1870,7 +1870,7 @@ sub process {
substr($s, 0, length($c), '');

  # Make sure we remove the line prefixes as we have
-   # none on the first line, and are going to readd 
them
+  # none on the first line, and are going to re-add 
them
# where necessary.
$s =~ s/\n./\n/gs;

diff --git a/scripts/clean-header-guards.pl b/scripts/clean-header-guards.pl
index f47d673ad5..a6680253b1 100755
--- a/scripts/clean-header-guards.pl
+++ b/scripts/clean-header-guards.pl
@@ -19,7 +19,7 @@
# Does the following:
# - Header files without a recognizable header guard are skipped.
# - Clean up any untidy header guards in-place.  Warn if the cleanup
-#   renames guard symbols, and explain how to find occurences of these
+#   renames guard symbols, and explain how to find occurrences of these
#   symbols that may have to be updated manually.
# - Warn about duplicate header guard symbols.  To make full use of
#   this warning, you should clean up *all* headers in one run.
diff --git a/scripts/decodetree.py b/scripts/decodetree.py
index 4cd1e10904..c78f54ef0a 100644
--- a/scripts/decodetree.py
+++ b/scripts/decodetree.py
@@ -88,7 +88,7 @@ def str_indent(c):

 def str_fields(fields):
-"""Return a string uniquely identifing FIELDS"""
+"""Return a string uniquely identifying FIELDS"""
 r = ''
 for n in sorted(fields.keys()):
 r += '_' + n
@@ -806,7 +806,7 @@ def parse_generic(lineno, parent_pat, name, toks):
 arg = None
 fmt = None
 for t in toks:
-# '&Foo' gives a format an explcit argument set.
+# '&Foo' gives a format an explicit argument set.
 if t[0] == '&':
 tt = t[1:]
 if arg:
@@ -895,7 +895,7 @@ def parse_generic(lineno, parent_pat, name, toks):
 elif not (is_format and width == 0) and width != insnwidth:
 error(lineno, 'definition has {0} bits'.format(width))

-# Do not check for fields overlaping fields; one valid usage
+# Do not check for fields overlapping fields; one valid usage
 # is to be able to duplicate fields via import.
 fieldmask = 0
 for f in flds.values():
diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh
index f0b7442c96..41fb746731 100755
--- a/scripts/oss-fuzz/build.sh
+++ b/scripts/oss-fuzz/build.sh
@@ -69,7 +69,7 @@ mkdir -p "$DEST_DIR/lib/"  # Copy the shared libraries here

if ! make "-j$(nproc)" qemu-fuzz-i386; then
 fatal "Build failed. Please specify a compiler with fuzzing support"\
-  "using the \$CC and \$CXX environemnt variables"\
+  "using the \$CC and \$CXX environment variables"\
   "\nFor example: CC=clang CXX=clang++ $0"
fi

diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index 3ccfa1e116..3ee54be223 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -34,7 +34,7 @@ def error(*lines):
def out(*lines, **kwargs):
 """Write a set of output lines.

-You can use kwargs as a shorthand for mapping variables when formating all
+You can use kwargs as a shorthand for mapping variables when formatting all
 the strings in lines.
 """
 lines = [ l % kwargs for l in lines ]
--
2.26.2.windows.1

[PATCH V2 02/10] migration/: fix some comment spelling errors

[zhaolichang]

I found that there are many spelling errors in the comments of qemu,
so I used the spellcheck tool to check the spelling errors
and finally found some spelling errors in the migration folder.

Signed-off-by: zhaolichang 
Reviewed-by: Peter Maydell 
---
migration/colo-failover.c |  2 +-
migration/colo.c  |  2 +-
migration/multifd.c   |  4 ++--
migration/postcopy-ram.c  |  4 ++--
migration/postcopy-ram.h  |  2 +-
migration/ram.c   | 10 +-
migration/rdma.c  |  8 
migration/savevm.c|  4 ++--
8 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/migration/colo-failover.c b/migration/colo-failover.c
index e9ca0b4774..b717edc8e2 100644
--- a/migration/colo-failover.c
+++ b/migration/colo-failover.c
@@ -46,7 +46,7 @@ void failover_request_active(Error **errp)
{
if (failover_set_state(FAILOVER_STATUS_NONE,
 FAILOVER_STATUS_REQUIRE) != FAILOVER_STATUS_NONE) {
-error_setg(errp, "COLO failover is already actived");
+error_setg(errp, "COLO failover is already activated");
 return;
 }
 failover_bh = qemu_bh_new(colo_failover_bh, NULL);
diff --git a/migration/colo.c b/migration/colo.c
index ea7d1e9d4e..80788d46b5 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -632,7 +632,7 @@ out:
 /*
  * It is safe to unregister notifier after failover finished.
  * Besides, colo_delay_timer and colo_checkpoint_sem can't be
- * released befor unregister notifier, or there will be use-after-free
+ * released before unregister notifier, or there will be use-after-free
  * error.
  */
 colo_compare_unregister_notifier(&packets_compare_notifier);
diff --git a/migration/multifd.c b/migration/multifd.c
index d0441202aa..ac84a61797 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -731,7 +731,7 @@ static void multifd_new_send_channel_async(QIOTask *task, 
gpointer opaque)
 qemu_sem_post(&p->sem_sync);
 /*
  * Although multifd_send_thread is not created, but main migration
- * thread neet to judge whether it is running, so we need to mark
+ * thread needs to judge whether it is running, so we need to mark
  * its status.
  */
 p->quit = true;
@@ -1042,7 +1042,7 @@ bool multifd_recv_all_channels_created(void)

/*
  * Try to receive all multifd channels to get ready for the migration.
- * - Return true and do not set @errp when correctly receving all channels;
+ * - Return true and do not set @errp when correctly receiving all channels;
  * - Return false and do not set @errp when correctly receiving the current 
one;
  * - Return false and set @errp when failing to receive the current channel.
  */
diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 1bb22f2b6c..baf094ba3a 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -237,7 +237,7 @@ release_ufd:
  * request_ufd_features: this function should be called only once on a newly
  * opened ufd, subsequent calls will lead to error.
  *
- * Returns: true on succes
+ * Returns: true on success
  *
  * @ufd: fd obtained from userfaultfd syscall
  * @features: bit mask see UFFD_API_FEATURES
@@ -807,7 +807,7 @@ static void mark_postcopy_blocktime_end(uintptr_t addr)

 low_time_offset = get_low_time_offset(dc);
 /* lookup cpu, to clear it,
- * that algorithm looks straighforward, but it's not
+ * that algorithm looks straightforward, but it's not
  * optimal, more optimal algorithm is keeping tree or hash
  * where key is address value is a list of  */
 for (i = 0; i < smp_cpus; i++) {
diff --git a/migration/postcopy-ram.h b/migration/postcopy-ram.h
index 9941feb63a..6d2b3cf124 100644
--- a/migration/postcopy-ram.h
+++ b/migration/postcopy-ram.h
@@ -161,7 +161,7 @@ struct PostCopyFD {
  */
void postcopy_register_shared_ufd(struct PostCopyFD *pcfd);
void postcopy_unregister_shared_ufd(struct PostCopyFD *pcfd);
-/* Call each of the shared 'waker's registerd telling them of
+/* Call each of the shared 'waker's registered telling them of
  * availability of a block.
  */
int postcopy_notify_shared_wake(RAMBlock *rb, uint64_t offset);
diff --git a/migration/ram.c b/migration/ram.c
index 76d4fee5d5..c5f36aeae5 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -256,7 +256,7 @@ int64_t ramblock_recv_bitmap_send(QEMUFile *file,
 /*
  * Always use little endian when sending the bitmap. This is
  * required that when source and destination VMs are not using the
- * same endianess. (Note: big endian won't work.)
+ * same endianness. (Note: big endian won't work.)
  */
 bitmap_to_le(le_bitmap, block->receivedmap, nbits);

@@ -275,7 +275,7 @@ int64_t ramblock_recv_bitmap_send(QEMUFile *file,
 qemu_put_buffer(file, (const uint8_t *)le_bitmap, size);
 /*
  * Mark as an end, in case the middle part is screwed up due to
- * some "misterious" reason.
+ * some "mysterious" reason.

[PATCH V2 10/10] contrib/: fix some comment spelling errors

[zhaolichang]

I found that there are many spelling errors in the comments of qemu,
so I used the spellcheck tool to check the spelling errors
and finally found some spelling errors in the contrib folder.

Signed-off-by: zhaolichang 
Reviewed-by: Alex Bennée 
---
contrib/gitdm/filetypes.txt | 6 +++---
contrib/ivshmem-client/ivshmem-client.h | 2 +-
contrib/libvhost-user/libvhost-user.c   | 4 ++--
contrib/libvhost-user/libvhost-user.h   | 2 +-
4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/contrib/gitdm/filetypes.txt b/contrib/gitdm/filetypes.txt
index 9e9c505205..d2d6f6db8d 100644
--- a/contrib/gitdm/filetypes.txt
+++ b/contrib/gitdm/filetypes.txt
@@ -22,7 +22,7 @@
# in the gitdm sample-config directory.
#
# This file contains associations parameters regarding filetypes
-# (documentation, develompent, multimedia, images...)
+# (documentation, development, multimedia, images...)
#
# format:
# filetype   []
@@ -59,8 +59,8 @@ filetype code \.s$ # Assembly
filetype code \.S$ # Assembly
filetype code \.asm$# Assembly
filetype code \.awk$ # awk
-filetype code ^common$  # script fragements
-filetype code ^common.*$  # script fragements
+filetype code ^common$  # script fragments
+filetype code ^common.*$  # script fragments
filetype code (qom|qmp)-\w+$  # python script fragments

#
diff --git a/contrib/ivshmem-client/ivshmem-client.h 
b/contrib/ivshmem-client/ivshmem-client.h
index fe3cc4a03d..fc45a38060 100644
--- a/contrib/ivshmem-client/ivshmem-client.h
+++ b/contrib/ivshmem-client/ivshmem-client.h
@@ -174,7 +174,7 @@ int ivshmem_client_notify_all_vects(const IvshmemClient 
*client,
 const IvshmemClientPeer *peer);

/**
- * Broadcat a notification to all vectors of all peers
+ * Broadcast a notification to all vectors of all peers
  *
  * @client: The ivshmem client
  *
diff --git a/contrib/libvhost-user/libvhost-user.c 
b/contrib/libvhost-user/libvhost-user.c
index 53f16bdf08..9d30ff2283 100644
--- a/contrib/libvhost-user/libvhost-user.c
+++ b/contrib/libvhost-user/libvhost-user.c
@@ -684,7 +684,7 @@ vu_add_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {

 /*
  * If we are in postcopy mode and we receive a u64 payload with a 0 value
- * we know all the postcopy client bases have been recieved, and we
+ * we know all the postcopy client bases have been received, and we
  * should start generating faults.
  */
 if (track_ramblocks &&
@@ -973,7 +973,7 @@ vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg)
 for (i = 0; i < dev->max_queues; i++) {
 if (dev->vq[i].vring.desc) {
 if (map_ring(dev, &dev->vq[i])) {
-vu_panic(dev, "remaping queue %d during setmemtable", i);
+vu_panic(dev, "remapping queue %d during setmemtable", i);
 }
 }
 }
diff --git a/contrib/libvhost-user/libvhost-user.h 
b/contrib/libvhost-user/libvhost-user.h
index 844c37c648..287ac5fec7 100644
--- a/contrib/libvhost-user/libvhost-user.h
+++ b/contrib/libvhost-user/libvhost-user.h
@@ -424,7 +424,7 @@ typedef struct VuVirtqElement {
  * @remove_watch: a remove_watch callback
  * @iface: a VuDevIface structure with vhost-user device callbacks
  *
- * Intializes a VuDev vhost-user context.
+ * Initializes a VuDev vhost-user context.
  *
  * Returns: true on success, false on failure.
  **/
--
2.26.2.windows.1

[PATCH V2 03/10] docs/: fix some comment spelling errors

[zhaolichang]

I found that there are many spelling errors in the comments of qemu,
so I used the spellcheck tool to check the spelling errors
and finally found some spelling errors in the docs folder.

Signed-off-by: zhaolichang 
Reviewed-by: Peter Maydell 
---
docs/COLO-FT.txt | 6 +++---
docs/devel/blkdebug.txt  | 2 +-
docs/devel/migration.rst | 2 +-
docs/devel/testing.rst   | 2 +-
docs/devel/tracing.txt   | 2 +-
docs/interop/bitmaps.rst | 2 +-
docs/interop/dbus.rst| 4 ++--
docs/interop/nbd.txt | 2 +-
docs/interop/vhost-user-gpu.rst  | 2 +-
docs/interop/vhost-user.rst  | 4 ++--
docs/rdma.txt| 2 +-
docs/specs/ppc-spapr-hotplug.txt | 4 ++--
docs/specs/ppc-spapr-xive.rst| 4 ++--
docs/system/arm/aspeed.rst   | 2 +-
docs/system/deprecated.rst   | 8 
docs/system/target-avr.rst   | 4 ++--
docs/tools/virtiofsd.rst | 2 +-
17 files changed, 27 insertions(+), 27 deletions(-)

diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt
index c8e1740935..bc5fb2a1bb 100644
--- a/docs/COLO-FT.txt
+++ b/docs/COLO-FT.txt
@@ -91,7 +91,7 @@ the heartbeat stops responding, the secondary node will 
trigger a failover
as soon as it determines the absence.

COLO disk Manager:
-When primary VM writes data into image, the colo disk manger captures this data
+When primary VM writes data into image, the colo disk manager captures this 
data
and sends it to secondary VM's which makes sure the context of secondary VM's
image is consistent with the context of primary VM 's image.
For more details, please refer to docs/block-replication.txt.
@@ -146,12 +146,12 @@ in test procedure.

== Test procedure ==
Note: Here we are running both instances on the same host for testing,
-change the IP Addresses if you want to run it on two hosts. Initally
+change the IP Addresses if you want to run it on two hosts. Initially
127.0.0.1 is the Primary Host and 127.0.0.2 is the Secondary Host.

== Startup qemu ==
1. Primary:
-Note: Initally, $imagefolder/primary.qcow2 needs to be copied to all hosts.
+Note: Initially, $imagefolder/primary.qcow2 needs to be copied to all hosts.
You don't need to change any IP's here, because 0.0.0.0 listens on any
interface. The chardev's with 127.0.0.1 IP's loopback to the local qemu
instance.
diff --git a/docs/devel/blkdebug.txt b/docs/devel/blkdebug.txt
index 43d8e8f9c6..0b0c128d35 100644
--- a/docs/devel/blkdebug.txt
+++ b/docs/devel/blkdebug.txt
@@ -62,7 +62,7 @@ Rules support the following attributes:

   errno - the numeric errno value to return when a request matches this rule.
   The errno values depend on the host since the numeric values are not
-  standarized in the POSIX specification.
+  standardized in the POSIX specification.

   sector - (optional) a sector number that the request must overlap in order to
match this rule
diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst
index 2eb08624fc..49112bb27a 100644
--- a/docs/devel/migration.rst
+++ b/docs/devel/migration.rst
@@ -625,7 +625,7 @@ It can be issued immediately after migration is started or 
any
time later on.  Issuing it after the end of a migration is harmless.

Blocktime is a postcopy live migration metric, intended to show how
-long the vCPU was in state of interruptable sleep due to pagefault.
+long the vCPU was in state of interruptible sleep due to pagefault.
That metric is calculated both for all vCPUs as overlapped value, and
separately for each vCPU. These values are calculated on destination
side.  To enable postcopy blocktime calculation, enter following
diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index 196e3bc35e..bd64c1bdcd 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -471,7 +471,7 @@ the warning.
A few important files for suppressing warnings are:

tests/tsan/suppressions.tsan - Has TSan warnings we wish to suppress at runtime.
-The comment on each supression will typically indicate why we are
+The comment on each suppression will typically indicate why we are
suppressing it.  More information on the file format can be found here:

https://github.com/google/sanitizers/wiki/ThreadSanitizerSuppressions
diff --git a/docs/devel/tracing.txt b/docs/devel/tracing.txt
index 6144d9921b..d2160655b4 100644
--- a/docs/devel/tracing.txt
+++ b/docs/devel/tracing.txt
@@ -55,7 +55,7 @@ without any sub-directory path prefix. eg io/channel-buffer.c 
would do
   #include "trace.h"

To access the 'io/trace.h' file. While it is possible to include a trace.h
-file from outside a source files' own sub-directory, this is discouraged in
+file from outside a source file's own sub-directory, this is discouraged in
general. It is strongly preferred that all events be declared directly in
the sub-directory that uses them. The only exception is where there are some
shared trace events defined in the top level directory trace-events file.
diff --git a/docs/interop/bit

[PATCH V2 05/10] util/: fix some comment spelling errors

[zhaolichang]

I found that there are many spelling errors in the comments of qemu,
so I used the spellcheck tool to check the spelling errors
and finally found some spelling errors in the util folder.

Signed-off-by: zhaolichang 
Reviewed-by: Alex Bennée 
---
util/osdep.c | 2 +-
util/qemu-progress.c | 2 +-
util/qemu-sockets.c  | 2 +-
util/qemu-thread-win32.c | 2 +-
util/qht.c   | 2 +-
util/trace-events| 2 +-
6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/util/osdep.c b/util/osdep.c
index 4829c07ff6..e50dc2214e 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -392,7 +392,7 @@ int qemu_unlink(const char *name)
  * Set errno if fewer than `count' bytes are written.
  *
  * This function don't work with non-blocking fd's.
- * Any of the possibilities with non-bloking fd's is bad:
+ * Any of the possibilities with non-blocking fd's is bad:
  *   - return a short write (then name is wrong)
  *   - busy wait adding (errno == EAGAIN) to the loop
  */
diff --git a/util/qemu-progress.c b/util/qemu-progress.c
index 3c2223c1a2..20d51f8c12 100644
--- a/util/qemu-progress.c
+++ b/util/qemu-progress.c
@@ -131,7 +131,7 @@ void qemu_progress_end(void)
/*
  * Report progress.
  * @delta is how much progress we made.
- * If @max is zero, @delta is an absolut value of the total job done.
+ * If @max is zero, @delta is an absolute value of the total job done.
  * Else, @delta is a progress delta since the last call, as a fraction
  * of @max.  I.e. the delta is @delta * @max / 100. This allows
  * relative accounting of functions which may be a different fraction of
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index b37d288866..99ce2fd5e6 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -416,7 +416,7 @@ static struct addrinfo 
*inet_parse_connect_saddr(InetSocketAddress *saddr,

 /* At least FreeBSD and OS-X 10.6 declare AI_V4MAPPED but
  * then don't implement it in their getaddrinfo(). Detect
- * this and retry without the flag since that's preferrable
+ * this and retry without the flag since that's preferable
  * to a fatal error
  */
 if (rc == EAI_BADFLAGS &&
diff --git a/util/qemu-thread-win32.c b/util/qemu-thread-win32.c
index 56a8da..d207b0cb58 100644
--- a/util/qemu-thread-win32.c
+++ b/util/qemu-thread-win32.c
@@ -289,7 +289,7 @@ void qemu_event_wait(QemuEvent *ev)
 ResetEvent(ev->event);

 /* Tell qemu_event_set that there are waiters.  No need to retry
- * because there cannot be a concurent busy->free transition.
+ * because there cannot be a concurrent busy->free transition.
  * After the CAS, the event will be either set or busy.
  */
 if (atomic_cmpxchg(&ev->value, EV_FREE, EV_BUSY) == EV_SET) {
diff --git a/util/qht.c b/util/qht.c
index 67e5d5b916..b2e020c398 100644
--- a/util/qht.c
+++ b/util/qht.c
@@ -49,7 +49,7 @@
  * it anymore.
  *
  * Writers check for concurrent resizes by comparing ht->map before and after
- * acquiring their bucket lock. If they don't match, a resize has occured
+ * acquiring their bucket lock. If they don't match, a resize has occurred
  * while the bucket spinlock was being acquired.
  *
  * Related Work:
diff --git a/util/trace-events b/util/trace-events
index 0ce42822eb..4bc01808af 100644
--- a/util/trace-events
+++ b/util/trace-events
@@ -28,7 +28,7 @@ qemu_file_monitor_add_watch(void *mon, const char *dirpath, 
const char *filename
qemu_file_monitor_remove_watch(void *mon, const char *dirpath, int64_t id) 
"File monitor %p remove watch dir='%s' id=%" PRId64
qemu_file_monitor_new(void *mon, int fd) "File monitor %p created fd=%d"
qemu_file_monitor_enable_watch(void *mon, const char *dirpath, int id) "File 
monitor %p enable watch dir='%s' id=%u"
-qemu_file_monitor_disable_watch(void *mon, const char *dirpath, int id) "Fle 
monitor %p disable watch dir='%s' id=%u"
+qemu_file_monitor_disable_watch(void *mon, const char *dirpath, int id) "File 
monitor %p disable watch dir='%s' id=%u"
qemu_file_monitor_event(void *mon, const char *dirpath, const char *filename, 
int mask, unsigned int id) "File monitor %p event dir='%s' file='%s' mask=0x%x 
id=%u"
qemu_file_monitor_dispatch(void *mon, const char *dirpath, const char 
*filename, int ev, void *cb, void *opaque, int64_t id) "File monitor %p 
dispatch dir='%s' file='%s' ev=%d cb=%p opaque=%p id=%" PRId64

--
2.26.2.windows.1

[PATCH V2 07/10] block/: fix some comment spelling errors

[zhaolichang]

I found that there are many spelling errors in the comments of qemu,
so I used the spellcheck tool to check the spelling errors
and finally found some spelling errors in the block folder.

Signed-off-by: zhaolichang 
---
block/block-copy.c | 2 +-
block/linux-aio.c  | 2 +-
block/mirror.c | 2 +-
block/vhdx.c   | 2 +-
block/vhdx.h   | 4 ++--
5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/block/block-copy.c b/block/block-copy.c
index a30b9097ef..d648306ce5 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -321,7 +321,7 @@ static coroutine_fn int block_copy_task_run(AioTaskPool 
*pool,
  * Do copy of cluster-aligned chunk. Requested region is allowed to exceed
  * s->len only to cover last cluster when s->len is not aligned to clusters.
  *
- * No sync here: nor bitmap neighter intersecting requests handling, only copy.
+ * No sync here: neither bitmap nor intersecting requests handling, only copy.
  *
  * Returns 0 on success.
  */
diff --git a/block/linux-aio.c b/block/linux-aio.c
index 3c0527c2bf..772ff860ea 100644
--- a/block/linux-aio.c
+++ b/block/linux-aio.c
@@ -222,7 +222,7 @@ static void qemu_laio_process_completions(LinuxAioState *s)

 /* If we are nested we have to notify the level above that we are done
  * by setting event_max to zero, upper level will then jump out of it's
- * own `for` loop.  If we are the last all counters droped to zero. */
+ * own `for` loop.  If we are the last all counters dropped to zero. */
 s->event_max = 0;
 s->event_idx = 0;
}
diff --git a/block/mirror.c b/block/mirror.c
index e8e8844afc..22d574167e 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -478,7 +478,7 @@ static uint64_t coroutine_fn 
mirror_iteration(MirrorBlockJob *s)

 job_pause_point(&s->common.job);

-/* Find the number of consective dirty chunks following the first dirty
+/* Find the number of consecutive dirty chunks following the first dirty
  * one, and wait for in flight requests in them. */
 bdrv_dirty_bitmap_lock(s->dirty_bitmap);
 while (nb_chunks * s->granularity < s->buf_size) {
diff --git a/block/vhdx.c b/block/vhdx.c
index 791eb90263..84eedee83f 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1073,7 +1073,7 @@ static int vhdx_open(BlockDriverState *bs, QDict 
*options, int flags,
 goto fail;
 }

-/* endian convert populated BAT field entires */
+/* endian convert populated BAT field entries */
 for (i = 0; i < s->bat_entries; i++) {
 s->bat[i] = le64_to_cpu(s->bat[i]);
 }
diff --git a/block/vhdx.h b/block/vhdx.h
index 0b74924cee..e385e484b4 100644
--- a/block/vhdx.h
+++ b/block/vhdx.h
@@ -212,7 +212,7 @@ typedef struct QEMU_PACKED VHDXLogDataSector {
 uint32_tsequence_high;  /* 4 MSB of 8 byte sequence_number */
 uint8_t data[4084]; /* raw data, bytes 8-4091 (inclusive).
see the data descriptor field for 
the
-   other mising bytes */
+   other missing bytes */
 uint32_tsequence_low;   /* 4 LSB of 8 byte sequence_number */
} VHDXLogDataSector;

@@ -257,7 +257,7 @@ typedef struct QEMU_PACKED VHDXMetadataTableHeader {

#define VHDX_META_FLAGS_IS_USER 0x01/* max 1024 entries */
#define VHDX_META_FLAGS_IS_VIRTUAL_DISK 0x02/* virtual disk metadata if set,
-   otherwise file metdata */
+   otherwise file metadata */
#define VHDX_META_FLAGS_IS_REQUIRED 0x04/* parse must understand this
entry to open the file */
typedef struct QEMU_PACKED VHDXMetadataTableEntry {
--
2.26.2.windows.1

[PATCH V2 06/10] linux-user/: fix some comment spelling errors

[zhaolichang]

I found that there are many spelling errors in the comments of qemu,
so I used the spellcheck tool to check the spelling errors
and finally found some spelling errors in the linux-user folder.

Signed-off-by: zhaolichang 
Reviewed-by: Alex Bennée 
---
linux-user/aarch64/signal.c  | 2 +-
linux-user/cris/target_syscall.h | 4 ++--
linux-user/flat.h| 2 +-
linux-user/flatload.c| 4 ++--
linux-user/host/ppc64/safe-syscall.inc.S | 2 +-
linux-user/syscall.c | 4 ++--
6 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/linux-user/aarch64/signal.c b/linux-user/aarch64/signal.c
index cd521ee42d..d50c1ae583 100644
--- a/linux-user/aarch64/signal.c
+++ b/linux-user/aarch64/signal.c
@@ -78,7 +78,7 @@ struct target_sve_context {
 struct target_aarch64_ctx head;
 uint16_t vl;
 uint16_t reserved[3];
-/* The actual SVE data immediately follows.  It is layed out
+/* The actual SVE data immediately follows.  It is laid out
  * according to TARGET_SVE_SIG_{Z,P}REG_OFFSET, based off of
  * the original struct pointer.
  */
diff --git a/linux-user/cris/target_syscall.h b/linux-user/cris/target_syscall.h
index 29d69009ff..c7ae89d73a 100644
--- a/linux-user/cris/target_syscall.h
+++ b/linux-user/cris/target_syscall.h
@@ -4,7 +4,7 @@
#define UNAME_MACHINE "cris"
#define UNAME_MINIMUM_RELEASE "2.6.32"

-/* pt_regs not only specifices the format in the user-struct during
+/* pt_regs not only specifies the format in the user-struct during
  * ptrace but is also the frame format used in the kernel prologue/epilogues
  * themselves
  */
@@ -32,7 +32,7 @@ struct target_pt_regs {
 unsigned long spc;
 unsigned long ccs;
 unsigned long srp;
-unsigned long erp; /* This is actually the debugged process' PC */
+unsigned long erp; /* This is actually the debugged process's PC */
 /* For debugging purposes; saved only when needed. */
 unsigned long exs;
 unsigned long eda;
diff --git a/linux-user/flat.h b/linux-user/flat.h
index 1e44b33443..ed518e2013 100644
--- a/linux-user/flat.h
+++ b/linux-user/flat.h
@@ -43,7 +43,7 @@ struct flat_hdr {
 abi_ulong reloc_count;  /* Number of relocation records */
 abi_ulong flags;
 abi_ulong build_date;   /* When the program/library was built */
-abi_ulong filler[5];/* Reservered, set to zero */
+   abi_ulong filler[5];/* Reserved, set to zero */
};

#define FLAT_FLAG_RAM0x0001 /* load program entirely into RAM */
diff --git a/linux-user/flatload.c b/linux-user/flatload.c
index 8fb448f0bf..14d2999d15 100644
--- a/linux-user/flatload.c
+++ b/linux-user/flatload.c
@@ -442,7 +442,7 @@ static int load_flat_file(struct linux_binprm * bprm,
 indx_len = (indx_len + 15) & ~(abi_ulong)15;

 /*
- * Alloate the address space.
+ * Allocate the address space.
  */
 probe_guest_base(bprm->filename, 0,
  text_len + data_len + extra + indx_len);
@@ -794,7 +794,7 @@ int load_flt_binary(struct linux_binprm *bprm, struct 
image_info *info)
#error here
 for (i = MAX_SHARED_LIBS-1; i>0; i--) {
 if (libinfo[i].loaded) {
-/* Push previos first to call address */
+/* Push previous first to call address */
 --sp;
 if (put_user_ual(start_addr, sp))
 return -EFAULT;
diff --git a/linux-user/host/ppc64/safe-syscall.inc.S 
b/linux-user/host/ppc64/safe-syscall.inc.S
index 8ed73a5b86..875133173b 100644
--- a/linux-user/host/ppc64/safe-syscall.inc.S
+++ b/linux-user/host/ppc64/safe-syscall.inc.S
@@ -84,7 +84,7 @@ safe_syscall_end:

   /* code path when we didn't execute the syscall */
0:  addi 3, 0, -TARGET_ERESTARTSYS
-ld 14, 16(1) /* restore r14 to its orginal value */
+   ld 14, 16(1) /* restore r14 to its original value */
 blr
 .cfi_endproc

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index b4a7b605f3..bdfa51dde8 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -477,7 +477,7 @@ _syscall4(int, sys_prlimit64, pid_t, pid, int, resource,

 #if defined(TARGET_NR_timer_create)
-/* Maxiumum of 32 active POSIX timers allowed at any one time. */
+/* Maximum of 32 active POSIX timers allowed at any one time. */
static timer_t g_posix_timers[32] = { 0, } ;

static inline int next_free_host_timer(void)
@@ -7800,7 +7800,7 @@ static abi_long do_syscall1(void *cpu_env, int num, 
abi_long arg1,
 switch(num) {
 case TARGET_NR_exit:
 /* In old applications this may be used to implement _exit(2).
-   However in threaded applictions it is used for thread termination,
+   However in threaded applications it is used for thread termination,
and _exit_group is used for application termination.
Do thread termination if we have more then one thread.  

[PATCH V2 09/10] qapi/: fix some comment spelling errors

[zhaolichang]

I found that there are many spelling errors in the comments of qemu,
so I used the spellcheck tool to check the spelling errors
and finally found some spelling errors in the qapi folder.

Signed-off-by: zhaolichang 
Reviewed-by: Markus Armbruster 
---
qapi/block-core.json | 4 ++--
qapi/crypto.json | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 197bdc1c36..b0ef7c19d4 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1790,7 +1790,7 @@
#
# @block-backend: corresponds to BlockBackend
#
-# @block-job: corresonds to BlockJob
+# @block-job: corresponds to BlockJob
#
# @block-driver: corresponds to BlockDriverState
#
@@ -2061,7 +2061,7 @@
# @target: name of the destination dirty bitmap
#
# @bitmaps: name(s) of the source dirty bitmap(s) at @node and/or fully
-#   specifed BlockDirtyBitmap elements. The latter are supported
+#   specified BlockDirtyBitmap elements. The latter are supported
#   since 4.1.
#
# Since: 4.0
diff --git a/qapi/crypto.json b/qapi/crypto.json
index bb7930d332..2aebe6fa20 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -336,7 +336,7 @@
# written into added active keyslots
#
# @old-secret:Optional (for deactivation only)
-# If given will deactive all keyslots that
+# If given will deactivate all keyslots that
# match password located in QCryptoSecret with this ID
#
# @iter-time: Optional (for activation only)
@@ -354,7 +354,7 @@
# keyslot to deactivate
#
# @secret:Optional. The ID of a QCryptoSecret object providing the
-# password to use to retrive current master key.
+# password to use to retrieve current master key.
# Defaults to the same secret that was used to open the image
#
#
--
2.26.2.windows.1

[PATCH V2 08/10] disas/: fix some comment spelling errors

[zhaolichang]

I found that there are many spelling errors in the comments of qemu,
so I used the spellcheck tool to check the spelling errors
and finally found some spelling errors in the disas folder.

Signed-off-by: zhaolichang 
---
disas/hppa.c | 2 +-
disas/m68k.c | 8 
disas/ppc.c  | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/disas/hppa.c b/disas/hppa.c
index 2dbd1fc445..dcf9a47f34 100644
--- a/disas/hppa.c
+++ b/disas/hppa.c
@@ -2021,7 +2021,7 @@ print_insn_hppa (bfd_vma memaddr, disassemble_info *info)
fput_fp_reg (GET_FIELD (insn, 6, 10), info);
 break;

-   /* 'fA' will not generate a space before the regsiter
+  /* 'fA' will not generate a space before the register
name.  Normally that is fine.  Except that it
causes problems with xmpyu which has no FP format
completer.  */
diff --git a/disas/m68k.c b/disas/m68k.c
index 863409c67c..aefaecfbd6 100644
--- a/disas/m68k.c
+++ b/disas/m68k.c
@@ -70,7 +70,7 @@ struct floatformat
   unsigned int exp_start;
   unsigned int exp_len;
   /* Bias added to a "true" exponent to form the biased exponent.  It
- is intentionally signed as, otherwize, -exp_bias can turn into a
+ is intentionally signed as, otherwise, -exp_bias can turn into a
  very large number (e.g., given the exp_bias of 0x3fff and a 64
  bit long, the equation (long)(1 - exp_bias) evaluates to
  4294950914) instead of -16382).  */
@@ -479,7 +479,7 @@ struct m68k_opcode_alias
   and remaining 3 bits of register shifted 9 bits in first word.
   Indicate upper/lower in 1 bit shifted 7 bits in second word.
   Use with `R' or `u' format.
-   n  `m' withouth upper/lower indication. (For M[S]ACx; 4 bits split
+   n  `m' without upper/lower indication. (For M[S]ACx; 4 bits split
   with MSB shifted 6 bits in first word and remaining 3 bits of
   register shifted 9 bits in first word.  No upper/lower
   indication is done.)  Use with `R' or `u' format.
@@ -854,7 +854,7 @@ fetch_arg (unsigned char *buffer,

/* Check if an EA is valid for a particular code.  This is required
for the EMAC instructions since the type of source address determines
-   if it is a EMAC-load instruciton if the EA is mode 2-5, otherwise it
+   if it is a EMAC-load instruction if the EA is mode 2-5, otherwise it
   is a non-load EMAC instruction and the bits mean register Ry.
A similar case exists for the movem instructions where the register
mask is interpreted differently for different EAs.  */
@@ -1080,7 +1080,7 @@ print_indexed (int basereg,

/* Returns number of bytes "eaten" by the operand, or
return -1 if an invalid operand was found, or -2 if
-   an opcode tabe error was found.
+   an opcode table error was found.
ADDR is the pc for this arg to be relative to.  */

static int
diff --git a/disas/ppc.c b/disas/ppc.c
index 63e97cfe1d..02be878198 100644
--- a/disas/ppc.c
+++ b/disas/ppc.c
@@ -5226,7 +5226,7 @@ operand_value_powerpc (const struct powerpc_operand 
*operand,
   if ((operand->flags & PPC_OPERAND_SIGNED) != 0)
 {
   /* BITM is always some number of zeros followed by some
- number of ones, followed by some numer of zeros.  */
+number of ones, followed by some number of zeros.  */
   unsigned long top = operand->bitm;
   /* top & -top gives the rightmost 1 bit, so this
  fills in any trailing zeros.  */
--
2.26.2.windows.1

[PULL v2 00/34] Block patches

[Max Reitz]

The following changes since commit 30aa19446d82358a30eac3b556b4d6641e00b7c1:

  Merge remote-tracking branch 'remotes/cschoenebeck/tags/pull-9p-20200812' 
into staging (2020-08-24 16:39:53 +0100)

are available in the Git repository at:

  https://github.com/XanClic/qemu.git tags/pull-block-2020-08-26

for you to fetch changes up to a5d3cfa2dc775e5d99f013703b8508f1d989d588:

  iotests: Add tests for qcow2 images with extended L2 entries (2020-08-26 
08:49:51 +0200)


Block patches:
- qcow2 subclusters (extended L2 entries)


v2:
- Fixed the shebang line in iotest 271


Alberto Garcia (34):
  qcow2: Make Qcow2AioTask store the full host offset
  qcow2: Convert qcow2_get_cluster_offset() into qcow2_get_host_offset()
  qcow2: Add calculate_l2_meta()
  qcow2: Split cluster_needs_cow() out of count_cow_clusters()
  qcow2: Process QCOW2_CLUSTER_ZERO_ALLOC clusters in handle_copied()
  qcow2: Add get_l2_entry() and set_l2_entry()
  qcow2: Document the Extended L2 Entries feature
  qcow2: Add dummy has_subclusters() function
  qcow2: Add subcluster-related fields to BDRVQcow2State
  qcow2: Add offset_to_sc_index()
  qcow2: Add offset_into_subcluster() and size_to_subclusters()
  qcow2: Add l2_entry_size()
  qcow2: Update get/set_l2_entry() and add get/set_l2_bitmap()
  qcow2: Add QCow2SubclusterType and qcow2_get_subcluster_type()
  qcow2: Add qcow2_get_subcluster_range_type()
  qcow2: Add qcow2_cluster_is_allocated()
  qcow2: Add cluster type parameter to qcow2_get_host_offset()
  qcow2: Replace QCOW2_CLUSTER_* with QCOW2_SUBCLUSTER_*
  qcow2: Handle QCOW2_SUBCLUSTER_UNALLOCATED_ALLOC
  qcow2: Add subcluster support to calculate_l2_meta()
  qcow2: Add subcluster support to qcow2_get_host_offset()
  qcow2: Add subcluster support to zero_in_l2_slice()
  qcow2: Add subcluster support to discard_in_l2_slice()
  qcow2: Add subcluster support to check_refcounts_l2()
  qcow2: Update L2 bitmap in qcow2_alloc_cluster_link_l2()
  qcow2: Clear the L2 bitmap when allocating a compressed cluster
  qcow2: Add subcluster support to handle_alloc_space()
  qcow2: Add subcluster support to qcow2_co_pwrite_zeroes()
  qcow2: Add subcluster support to qcow2_measure()
  qcow2: Add prealloc field to QCowL2Meta
  qcow2: Add the 'extended_l2' option and the QCOW2_INCOMPAT_EXTL2 bit
  qcow2: Allow preallocation and backing files if extended_l2 is set
  qcow2: Assert that expand_zero_clusters_in_l1() does not support
subclusters
  iotests: Add tests for qcow2 images with extended L2 entries

 docs/interop/qcow2.txt   |  68 ++-
 docs/qcow2-cache.txt |  19 +-
 qapi/block-core.json |   7 +
 block/qcow2.h| 211 ++-
 include/block/block_int.h|   1 +
 block/qcow2-cluster.c| 906 +--
 block/qcow2-refcount.c   |  47 +-
 block/qcow2.c| 302 +++
 block/trace-events   |   2 +-
 tests/qemu-iotests/031.out   |   8 +-
 tests/qemu-iotests/036.out   |   4 +-
 tests/qemu-iotests/049.out   | 102 ++--
 tests/qemu-iotests/060.out   |   3 +-
 tests/qemu-iotests/061   |   6 +
 tests/qemu-iotests/061.out   |  25 +-
 tests/qemu-iotests/065   |  12 +-
 tests/qemu-iotests/082.out   |  39 +-
 tests/qemu-iotests/085.out   |  38 +-
 tests/qemu-iotests/144.out   |   4 +-
 tests/qemu-iotests/182.out   |   2 +-
 tests/qemu-iotests/185.out   |   8 +-
 tests/qemu-iotests/198   |   2 +
 tests/qemu-iotests/206.out   |   6 +-
 tests/qemu-iotests/242.out   |   5 +
 tests/qemu-iotests/255.out   |   8 +-
 tests/qemu-iotests/271   | 901 ++
 tests/qemu-iotests/271.out   | 726 +
 tests/qemu-iotests/274.out   |  49 +-
 tests/qemu-iotests/280.out   |   2 +-
 tests/qemu-iotests/291.out   |   2 +
 tests/qemu-iotests/302.out   |   1 +
 tests/qemu-iotests/303.out   |   4 +-
 tests/qemu-iotests/common.filter |   1 +
 tests/qemu-iotests/group |   1 +
 34 files changed, 2952 insertions(+), 570 deletions(-)
 create mode 100755 tests/qemu-iotests/271
 create mode 100644 tests/qemu-iotests/271.out

-- 
2.26.2

[Bug 1893003] [NEW] qemu-user doesn't translate host/target data for iovec I/O

[Mike Gelfand]

Public bug reported:

When using iovec I/O functions (like `readv`), no data translation
happens. I'm hitting this issue with libevent upon constructing a
bufferevent over an inotify descriptor, and then building for either
ppc64 or s390x (both big-endian) on x86_64 (little-endian) and running
resulting code with qemu-ppc64 or qemu-s390x on Gentoo using latest QEMU
version available (5.0.0-r2).

The code in question is in
https://github.com/transmission/transmission/blob/master/libtransmission
/watchdir-inotify.c (`tr_watchdir_inotify_new`,
`tr_watchdir_inotify_on_event`).

While `read` syscall is handled properly, `readv` (which libevent is
using in my case) doesn't have any logic to call
`host_to_target_data_inotify` or any other translation function, leaving
inotify data unchanged (with values in little-endian), which then leads
to unit test failures. Quoting `do_syscall1` implementation bits for the
reference:

---8<---begin---
case TARGET_NR_read:
if (arg2 == 0 && arg3 == 0) {
return get_errno(safe_read(arg1, 0, 0));
} else {
if (!(p = lock_user(VERIFY_WRITE, arg2, arg3, 0)))
return -TARGET_EFAULT;
ret = get_errno(safe_read(arg1, p, arg3));
if (ret >= 0 &&
fd_trans_host_to_target_data(arg1)) {
ret = fd_trans_host_to_target_data(arg1)(p, ret);
}
unlock_user(p, arg2, ret);
}
return ret;
...
case TARGET_NR_readv:
{
struct iovec *vec = lock_iovec(VERIFY_WRITE, arg2, arg3, 0);
if (vec != NULL) {
ret = get_errno(safe_readv(arg1, vec, arg3));
unlock_iovec(vec, arg2, arg3, 1);
} else {
ret = -host_to_target_errno(errno);
}
}
return ret;
---8<---end---

To reiterate, the issue is not only with `readv` but with other iovec
functions as well.

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  qemu-user doesn't translate host/target data for iovec I/O

Status in QEMU:
  New

Bug description:
  When using iovec I/O functions (like `readv`), no data translation
  happens. I'm hitting this issue with libevent upon constructing a
  bufferevent over an inotify descriptor, and then building for either
  ppc64 or s390x (both big-endian) on x86_64 (little-endian) and running
  resulting code with qemu-ppc64 or qemu-s390x on Gentoo using latest
  QEMU version available (5.0.0-r2).

  The code in question is in
  https://github.com/transmission/transmission/blob/master/libtransmission
  /watchdir-inotify.c (`tr_watchdir_inotify_new`,
  `tr_watchdir_inotify_on_event`).

  While `read` syscall is handled properly, `readv` (which libevent is
  using in my case) doesn't have any logic to call
  `host_to_target_data_inotify` or any other translation function,
  leaving inotify data unchanged (with values in little-endian), which
  then leads to unit test failures. Quoting `do_syscall1` implementation
  bits for the reference:

  ---8<---begin---
  case TARGET_NR_read:
  if (arg2 == 0 && arg3 == 0) {
  return get_errno(safe_read(arg1, 0, 0));
  } else {
  if (!(p = lock_user(VERIFY_WRITE, arg2, arg3, 0)))
  return -TARGET_EFAULT;
  ret = get_errno(safe_read(arg1, p, arg3));
  if (ret >= 0 &&
  fd_trans_host_to_target_data(arg1)) {
  ret = fd_trans_host_to_target_data(arg1)(p, ret);
  }
  unlock_user(p, arg2, ret);
  }
  return ret;
  ...
  case TARGET_NR_readv:
  {
  struct iovec *vec = lock_iovec(VERIFY_WRITE, arg2, arg3, 0);
  if (vec != NULL) {
  ret = get_errno(safe_readv(arg1, vec, arg3));
  unlock_iovec(vec, arg2, arg3, 1);
  } else {
  ret = -host_to_target_errno(errno);
  }
  }
  return ret;
  ---8<---end---

  To reiterate, the issue is not only with `readv` but with other iovec
  functions as well.

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

Re: [PATCH] meson: don't require CONFIG_VTE for the GTK UI

[Paolo Bonzini]

Queued, thanks.

Paolo

On Wed, Aug 26, 2020 at 12:23 AM Mark Cave-Ayland
 wrote:
>
> Prevously CONFIG_VTE was not required to build QEMU with GTK UI support as not
> all platforms have VTE available (in particular Windows).
>
> Remove this requirement from the meson build system to enable QEMU to be built
> with GTK UI support for Windows once again.
>
> Signed-off-by: Mark Cave-Ayland 
> ---
>  ui/meson.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ui/meson.build b/ui/meson.build
> index 018c5698bf..a81d5c259c 100644
> --- a/ui/meson.build
> +++ b/ui/meson.build
> @@ -42,7 +42,7 @@ if config_host.has_key('CONFIG_CURSES')
>ui_modules += {'curses' : curses_ss}
>  endif
>
> -if config_host.has_key('CONFIG_GTK') and config_host.has_key('CONFIG_VTE')
> +if config_host.has_key('CONFIG_GTK')
>softmmu_ss.add(when: 'CONFIG_WIN32', if_true: files('win32-kbd-hook.c'))
>
>gtk_ss = ss.source_set()
> --
> 2.20.1
>

Re: [PATCH v3 50/74] migration: Rename class type checking macros

[Juan Quintela]

Eduardo Habkost  wrote:
> Rename the macros to make them consistent with the MIGRATION_OBJ
> macro name.
>
> This will make future conversion to OBJECT_DECLARE* easier.
>
> Signed-off-by: Eduardo Habkost 
> ---
> Changes series v2 -> v3: new patch added to series v3
>
> ---
> Cc: Juan Quintela 
> Cc: "Dr. David Alan Gilbert" 
> Cc: qemu-devel@nongnu.org

not used anywhere, so if it makes your life easier.

Reviewed-by: Juan Quintela 

Re: [PATCH v3 62/74] [automated] Use TYPE_INFO macro

[Juan Quintela]

Eduardo Habkost  wrote:
> Generated using:
>   $ ./scripts/codeconverter/converter.py -i --passes=2 \
> --pattern=TypeRegisterCall,TypeInitMacro $(git grep -l TypeInfo -- 
> '*.[ch]')
>
> One notable difference is that files declaring multiple types
> will now have multiple separate __construtor__ functions
> declared, instead of one for all types.
>
> Reviewed-by: Daniel P. Berrangé 
> Signed-off-by: Eduardo Habkost 
> ---
> Changes v2 -> v3:
> * Removed hunks due to rebase conflicts:
>   hw/sd/milkymist-memcard.c hw/sd/pl181.c
> * Reviewed-by line from Daniel was kept, as no additional hunks
>   are introduced in this version
>
> Changes v1 -> v2:
> * Add note about multiple constructor functions to commit message
>   (suggested by Daniel)
> Signed-off-by: Eduardo Habkost 

[ I removed CC'd people, -ETOOMANYRECIPIENTS]



> diff --git a/migration/migration.c b/migration/migration.c
> index dbd4afa1e8..561e2ae697 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -3844,10 +3844,6 @@ static const TypeInfo migration_type = {
>  .instance_init = migration_instance_init,
>  .instance_finalize = migration_instance_finalize,
>  };
> +TYPE_INFO(migration_type)
>  
> -static void register_migration_types(void)
> -{
> -type_register_static(&migration_type);
> -}
>  
> -type_init(register_migration_types);
> diff --git a/migration/rdma.c b/migration/rdma.c
> index bea6532813..15ad985d26 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -3942,13 +3942,9 @@ static const TypeInfo qio_channel_rdma_info = {
>  .instance_finalize = qio_channel_rdma_finalize,
>  .class_init = qio_channel_rdma_class_init,
>  };
> +TYPE_INFO(qio_channel_rdma_info)
>  
> -static void qio_channel_rdma_register_types(void)
> -{
> -type_register_static(&qio_channel_rdma_info);
> -}
>  
> -type_init(qio_channel_rdma_register_types);
>  
>  static QEMUFile *qemu_fopen_rdma(RDMAContext *rdma, const char *mode)
>  {

For the migration bits.

Reviewed-by: Juan Quintela 

Re: [PATCH v3 65/74] [automated] Move QOM typedefs and add missing includes (pass 2)

[Juan Quintela]

Eduardo Habkost  wrote:
> Some typedefs and macros are defined after the type check macros.
> This makes it difficult to automatically replace their
> definitions with OBJECT_DECLARE_TYPE.
>
> Patch generated using:
>
>  $ ./scripts/codeconverter/converter.py -i \
>--pattern=QOMStructTypedefSplit $(git grep -l '' -- '*.[ch]')
>
> which will split "typdef struct { ... } TypedefName"
> declarations.
>
> Followed by:
>
>  $ ./scripts/codeconverter/converter.py -i --pattern=MoveSymbols \
> $(git grep -l '' -- '*.[ch]')
>
> which will:
> - move the typedefs and #defines above the type check macros
> - add missing #include "qom/object.h" lines if necessary
>
> Signed-off-by: Eduardo Habkost 
> ---
> Changes series v2 -> v3: this is a new patch added in series v3
>
> The script was re-run after rebase and after additional patches
> were added to this series.
>
> This is being submitted as a separate patch to make review
> easier, but it can be squashed into the previous patch once it
> gets reviewed.
>
> Signed-off-by: Eduardo Habkost 

> diff --git a/migration/migration.h b/migration/migration.h
> index ae497bd45a..4103e549bb 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -21,6 +21,7 @@
>  #include "qemu/coroutine_int.h"
>  #include "io/channel.h"
>  #include "net/announce.h"
> +#include "qom/object.h"
>  
>  struct PostcopyBlocktimeContext;
>  
> @@ -114,6 +115,7 @@ void 
> fill_destination_postcopy_migration_info(MigrationInfo *info);
>  
>  #define TYPE_MIGRATION "migration"
>  
> +typedef struct MigrationClass MigrationClass;
>  #define MIGRATION_OBJ_CLASS(klass) \
>  OBJECT_CLASS_CHECK(MigrationClass, (klass), TYPE_MIGRATION)
>  #define MIGRATION_OBJ(obj) \
> @@ -121,10 +123,10 @@ void 
> fill_destination_postcopy_migration_info(MigrationInfo *info);
>  #define MIGRATION_OBJ_GET_CLASS(obj) \
>  OBJECT_GET_CLASS(MigrationClass, (obj), TYPE_MIGRATION)
>  
> -typedef struct MigrationClass {
> +struct MigrationClass {
>  /*< private >*/
>  DeviceClass parent_class;
> -} MigrationClass;
> +};
>  
>  struct MigrationState
>  {

Reviewed-by: Juan Quintela 

for the migration bits.

Re: [PATCH v3 64/74] [automated] Move QOM typedefs and add missing includes

[Juan Quintela]

[dropped people from CC]

Eduardo Habkost  wrote:
> Some typedefs and macros are defined after the type check macros.
> This makes it difficult to automatically replace their
> definitions with OBJECT_DECLARE_TYPE.
>
> Patch generated using:
>
>  $ ./scripts/codeconverter/converter.py -i \
>--pattern=QOMStructTypedefSplit $(git grep -l '' -- '*.[ch]')
>
> which will split "typdef struct { ... } TypedefName"
> declarations.
>
> Followed by:
>
>  $ ./scripts/codeconverter/converter.py -i --pattern=MoveSymbols \
> $(git grep -l '' -- '*.[ch]')
>
> which will:
> - move the typedefs and #defines above the type check macros
> - add missing #include "qom/object.h" lines if necessary
>
> Reviewed-by: Daniel P. Berrangé 
> Signed-off-by: Eduardo Habkost 
> ---
> Changes v2 -> v3:
> * Removed hunks due to rebase conflicts: hw/arm/integratorcp.c
>   hw/arm/versatilepb.c hw/arm/vexpress.c hw/sd/pl181.c
>   include/hw/ppc/xive.h
> * Removed hunks due to conflicts with other patches in this
>   series: include/hw/block/swim.h include/hw/display/macfb.h
>   include/hw/rdma/rdma.h migration/migration.h
>   target/rx/cpu-qom.h
> * Reviewed-by line from Daniel was kept, as no additional hunks
>   are introduced in this version
>
> Changes v1 -> v2:
> * Re-ran script after moving a few macros and typedefs.  Now the
>   patch also changes:
>   - SysbusAHCIState at hw/ide/ahci.h
>   - VhostUserGPU at hw/virtio/virtio-gpu.h
>   - I8257State at hw/dma/i8257.h
>   - AllwinnerAHCIState at hw/ide/ahci.h
>   - ISAKBDState at hw/input/i8042.h
>   - PIIXState at hw/southbridge/piix.h
>   - VFIOPCIDevice at hw/vfio/pci.h
>   - missing include at hw/net/rocker/rocker.h
>   - missing include at hw/scsi/mptsas.h
>   - missing include at include/hw/arm/pxa.h
>   - missing include at include/sysemu/kvm.h
>
> Signed-off-by: Eduardo Habkost 

> diff --git a/migration/rdma.c b/migration/rdma.c
> index 15ad985d26..e3eac913bc 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -35,6 +35,7 @@
>  #include 
>  #include 
>  #include "trace.h"
> +#include "qom/object.h"
>  
>  /*
>   * Print and error on both the Monitor and the Log file.
> @@ -397,10 +398,10 @@ typedef struct RDMAContext {
>  } RDMAContext;
>  
>  #define TYPE_QIO_CHANNEL_RDMA "qio-channel-rdma"
> +typedef struct QIOChannelRDMA QIOChannelRDMA;
>  #define QIO_CHANNEL_RDMA(obj) \
>  OBJECT_CHECK(QIOChannelRDMA, (obj), TYPE_QIO_CHANNEL_RDMA)
>  
> -typedef struct QIOChannelRDMA QIOChannelRDMA;
>  
>  
>  struct QIOChannelRDMA {

Reviewed-by: Juan Quintela 

Re: [PATCH] meson: skip SDL2 detection if --disable-system

[Marc-André Lureau]

On Wed, Aug 26, 2020 at 11:03 AM Paolo Bonzini  wrote:

> SDL is only used for system emulation; avoid spurious warnings for
> static --disable-system emulation by skipping the detection of
> the library if there are no system emulation targets.
>
> Reported-by: Peter Maydell 
> Signed-off-by: Paolo Bonzini 
>

Reviewed-by: Marc-André Lureau 

---
>  meson.build | 27 ---
>  1 file changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index 57c2fe2b65..19d4d42512 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -21,6 +21,16 @@ qemu_datadir = get_option('datadir') +
> get_option('confsuffix')
>  config_host_data = configuration_data()
>  genh = []
>
> +target_dirs = config_host['TARGET_DIRS'].split()
> +have_user = false
> +have_system = false
> +foreach target : target_dirs
> +  have_user = have_user or target.endswith('-user')
> +  have_system = have_system or target.endswith('-softmmu')
> +endforeach
> +have_tools = 'CONFIG_TOOLS' in config_host
> +have_block = have_system or have_tools
> +
>  add_project_arguments(config_host['QEMU_CFLAGS'].split(),
>native: false, language: ['c', 'objc'])
>  add_project_arguments(config_host['QEMU_CXXFLAGS'].split(),
> @@ -225,9 +235,12 @@ if 'CONFIG_BRLAPI' in config_host
>brlapi = declare_dependency(link_args:
> config_host['BRLAPI_LIBS'].split())
>  endif
>
> -sdl = dependency('sdl2', required: get_option('sdl'), static:
> enable_static,
> - include_type: 'system')
> -sdl_image = not_found
> +sdl = not_found
> +if have_system
> +  sdl = dependency('sdl2', required: get_option('sdl'), static:
> enable_static,
> +   include_type: 'system')
> +  sdl_image = not_found
> +endif
>  if sdl.found()
># work around 2.0.8 bug
>sdl = declare_dependency(compile_args: '-Wno-undef',
> @@ -423,9 +436,6 @@ endforeach
>  genh += configure_file(output: 'config-host.h', configuration:
> config_host_data)
>
>  minikconf = find_program('scripts/minikconf.py')
> -target_dirs = config_host['TARGET_DIRS'].split()
> -have_user = false
> -have_system = false
>  config_devices_mak_list = []
>  config_devices_h = {}
>  config_target_h = {}
> @@ -446,7 +456,6 @@ kconfig_external_symbols = [
>  ]
>  ignored = ['TARGET_XML_FILES', 'TARGET_ABI_DIR', 'TARGET_DIRS']
>  foreach target : target_dirs
> -  have_user = have_user or target.endswith('-user')
>config_target = keyval.load(meson.current_build_dir() / target /
> 'config-target.mak')
>
>config_target_data = configuration_data()
> @@ -469,8 +478,6 @@ foreach target : target_dirs
> configuration:
> config_target_data)}
>
>if target.endswith('-softmmu')
> -have_system = true
> -
>  base_kconfig = []
>  foreach sym : kconfig_external_symbols
>if sym in config_target or sym in config_host
> @@ -500,8 +507,6 @@ foreach target : target_dirs
>endif
>config_target_mak += {target: config_target}
>  endforeach
> -have_tools = 'CONFIG_TOOLS' in config_host
> -have_block = have_system or have_tools
>
>  grepy = find_program('scripts/grepy.sh')
>  # This configuration is used to build files that are shared by
> --
> 2.26.2
>
>
>

-- 
Marc-André Lureau

Re: [PATCH v3 67/74] [automated] Use DECLARE_*CHECKER* macros (pass 2)

[Juan Quintela]

Eduardo Habkost  wrote:
>  $ ./scripts/codeconverter/converter.py -i \
>--pattern=TypeCheckMacro $(git grep -l '' -- '*.[ch]')
>
> Signed-off-by: Eduardo Habkost 
> ---
> Changes series v2 -> v3: this is a new patch added in series v3
>
> The script was re-run after rebase and after additional patches
> were added to this series.
>
> This is being submitted as a separate patch to make review
> easier, but it can be squashed into the previous patch once it
> gets reviewed.
>

...

> diff --git a/migration/migration.h b/migration/migration.h
> index 4103e549bb..bdc7450da3 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -116,12 +116,8 @@ void 
> fill_destination_postcopy_migration_info(MigrationInfo *info);
>  #define TYPE_MIGRATION "migration"
>  
>  typedef struct MigrationClass MigrationClass;
> -#define MIGRATION_OBJ_CLASS(klass) \
> -OBJECT_CLASS_CHECK(MigrationClass, (klass), TYPE_MIGRATION)
> -#define MIGRATION_OBJ(obj) \
> -OBJECT_CHECK(MigrationState, (obj), TYPE_MIGRATION)
> -#define MIGRATION_OBJ_GET_CLASS(obj) \
> -OBJECT_GET_CLASS(MigrationClass, (obj), TYPE_MIGRATION)
> +DECLARE_OBJ_CHECKERS(MigrationState, MigrationClass,
> + MIGRATION_OBJ, TYPE_MIGRATION)
>  
>  struct MigrationClass {
>  /*< private >*/

Reviewed-by: Juan Quintela 

Re: [PATCH v3 66/74] [automated] Use DECLARE_*CHECKER* macros

[Juan Quintela]

Eduardo Habkost  wrote:
>  $ ./scripts/codeconverter/converter.py -i \
>--pattern=TypeCheckMacro $(git grep -l '' -- '*.[ch]')
>
> Reviewed-by: Daniel P. Berrangé 
> Signed-off-by: Eduardo Habkost 
> ---
> Changes v2 -> v3:
> * Removed hunks due to rebase conflicts:
>   hw/arm/integratorcp.c hw/arm/versatilepb.c hw/sd/pl181.c
>   include/hw/ppc/xive.h
> * Reviewed-by line from Daniel was kept, as no additional hunks
>   are introduced in this version

[Dropeed CC'd]

...

> diff --git a/migration/rdma.c b/migration/rdma.c
> index e3eac913bc..87cb277d05 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -399,8 +399,8 @@ typedef struct RDMAContext {
>  
>  #define TYPE_QIO_CHANNEL_RDMA "qio-channel-rdma"
>  typedef struct QIOChannelRDMA QIOChannelRDMA;
> -#define QIO_CHANNEL_RDMA(obj) \
> -OBJECT_CHECK(QIOChannelRDMA, (obj), TYPE_QIO_CHANNEL_RDMA)
> +DECLARE_INSTANCE_CHECKER(QIOChannelRDMA, QIO_CHANNEL_RDMA,
> + TYPE_QIO_CHANNEL_RDMA)
>  
>  
>  

Reviewed-by: Juan Quintela 

Re: [PATCH] os-posix: Restore firmware location ../share/qemu

[Paolo Bonzini]

Hi, I will post next week a patch to make all searches relative to the
executable path. I will CC you so you can test it.

Thanks!

Paolo

Il mer 26 ago 2020, 10:01  ha scritto:

> Prior to commit 6dd2dacedd83d12328afa8559bffb2b9ec5c89ed (v5.0.0), the
> binary relative path ../share/qemu was searched for firmware, but in
> that commit, this path got lost.
>
> Consider the following use-case:
> * QEMU is built in a docker image on one system.
> * QEMU is supposed to be executed on a plethora of distributions/systems
> * QEMU is not installed system wide on the executors
> When building QEMU, the --prefix configure flag is used to generate a
> tree containing all the QEMU resources that needs to be transfered to
> the executors. The path to the root of the QEMU tree might be different
> for the different executors, therefore, the path stored in
> CONFIG_QEMU_DATADIR is likely not the right one.
> With this use-case, the only likely path is one that is expressed as
> relative to the root of the QEMU binary tree or the QEMU binary iself.
>
> Signed-off-by: Torbjörn Svensson 
> ---
>  os-posix.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/os-posix.c b/os-posix.c
> index bf98508b6d..f016ac374c 100644
> --- a/os-posix.c
> +++ b/os-posix.c
> @@ -96,6 +96,11 @@ char *os_find_datadir(void)
>  exec_dir = qemu_get_exec_dir();
>  g_return_val_if_fail(exec_dir != NULL, NULL);
>
> +dir = g_build_filename(exec_dir, "..", "share", "qemu", NULL);
> +if (g_file_test(dir, G_FILE_TEST_IS_DIR)) {
> +return g_steal_pointer(&dir);
> +}
> +
>  dir = g_build_filename(exec_dir, "pc-bios", NULL);
>  if (g_file_test(dir, G_FILE_TEST_IS_DIR)) {
>  return g_steal_pointer(&dir);
> --
> 2.26.2
>
>

Re: [PATCH 3/4] meson: Mingw64 gcc doesn't recognize system include_type for sdl2

[Daniel P . Berrangé]

On Wed, Aug 26, 2020 at 08:48:22AM +0200, Paolo Bonzini wrote:
> On Tue, Aug 25, 2020 at 11:38 PM Mark Cave-Ayland
>  wrote:
> > Marc-André had a query about why this is marked as a system include, 
> > however I can
> > confirm that it fixes the missing "SDL.h" issue during build.
> 
> It was marked as a system include in an attempt to work around the SDL
> 2.0.8 bug that requires -Wno-undef. In general we enable lots of
> warnings and sometimes they trip dependencies, so using include_type:
> 'system' in principle makes sense. But if it doesn't work with
> Windows, it's not a regression to remove it.

SDL code is isolated to just a couple of files, so if we nede to squelch
a -Wno-undef warning, we can just use pragma push/pop to disable the
warning selectively in the code.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [PATCH for-4.2 v2 3/3] block/file-posix: Let post-EOF fallocate serialize

[Vladimir Sementsov-Ogievskiy]

22.08.2020 20:04, Vladimir Sementsov-Ogievskiy wrote:

22.08.2020 20:03, Vladimir Sementsov-Ogievskiy wrote:

01.11.2019 18:25, Max Reitz wrote:

The XFS kernel driver has a bug that may cause data corruption for qcow2
images as of qemu commit c8bb23cbdbe32f.  We can work around it by
treating post-EOF fallocates as serializing up until infinity (INT64_MAX
in practice).


Hi! I'm doing some investigation, and here is an interesting result:

Consider the following test:

img=/ssd/x.qcow2; ./qemu-img create -f qcow2 $img 16G; ./qemu-img bench -c 
5 -d 64 -f qcow2 -o 1k -s 64k -t none -w $img


Bisecting results changes between 2.12 and 5.1, I found the following:

2.12: ~20s



c8bb23cbdbe32 "qcow2: skip writing zero buffers to empty COW areas"  -> becomes 
~12s  [1]




292d06b925b27 "block/file-posix: Let post-EOF fallocate serialize"   -> becomes 
~9s  [2]



v5.1 ~9s


And [1] is obvious, it is the main purpose of c8bb23cbdbe32. But [2] is a 
surprise for me.. Any ideas?

===

just to check: staying at c8bb23cbdbe32 I revert c8bb23cbdbe32 and get again 
~19.7s. So [2] doesn't substitute [1].



Note, it's all ext4.




Let's go further:

If I add -n, to use aio native:
./qemu-img create -f qcow2 $img 16G; ./qemu-img bench -c 5 -d 64 -f qcow2 
-o 1k -s 64k -t none -n -w $img;

Then this patch doesn't have such effect..

But if consider the test without an offset on hdd, the effect is very 
significant:

./qemu-img create -f qcow2 $img 16G; ./qemu-img bench -c 45000 -d 64 -f qcow2  
-s 16k -t none -n -w $img;

v2.12 ~10.4s
c8bb23cbdbe32^ ~10.6s
c8bb23cbdbe32 qcow2: skip writing zero buffers to empty COW areas: ~16.7s : 
degradation!!!
292d06b925b27^ ~16.4s
292d06b925 block/file-posix: Let post-EOF fallocate serialize: ~7.6s: great 
improvement !

--
Best regards,
Vladimir

Re: [PATCH v2 1/5] meson: pass confsuffix option

[Daniel P . Berrangé]

On Tue, Aug 25, 2020 at 09:58:23PM +0400, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> The following patches will make use of it to fix installation paths.
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  configure | 1 +
>  meson_options.txt | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/configure b/configure
> index b1e11397a8..e19e2de2f0 100755
> --- a/configure
> +++ b/configure
> @@ -8222,6 +8222,7 @@ NINJA=$PWD/ninjatool $meson setup \
>  --mandir "${pre_prefix}$mandir" \
>  --sysconfdir "${pre_prefix}$sysconfdir" \
>  --localstatedir "${pre_prefix}$local_statedir" \
> +-Dconfsuffix="$confsuffix" \
>  -Doptimization=$(if test "$debug" = yes; then echo 0; else echo 2; 
> fi) \
>  -Ddebug=$(if test "$debug_info" = yes; then echo true; else echo 
> false; fi) \
>  -Dwerror=$(if test "$werror" = yes; then echo true; else echo false; 
> fi) \
> diff --git a/meson_options.txt b/meson_options.txt
> index c55f9cd94c..7bb2c0fca9 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -1,3 +1,4 @@
> +option('confsuffix', type : 'string', value: 'qemu')

In "configure",  $confsuffix defaults to "/qemu", but here is misses the
"/".  Not having the "/" is better as meson will add the correct platform
specific dir separator, but it makes me think that configure needs updating
to strip a leading "/" when passing it into meson ?  

>  option('gettext', type : 'boolean', value : true)
>  option('sdl', type : 'feature', value : 'auto')
>  option('sdl_image', type : 'feature', value : 'auto')
> -- 
> 2.26.2
> 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [PATCH v2 2/5] meson: use meson datadir instead of qemu_datadir

[Daniel P . Berrangé]

On Tue, Aug 25, 2020 at 09:58:24PM +0400, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> When cross-compiling, by default qemu_datadir is 'c:\Program
> Files\QEMU', which is not recognized as being an absolute path, and
> meson will end up adding the prefix again.
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  contrib/vhost-user-gpu/meson.build | 2 +-
>  meson.build| 3 ++-
>  pc-bios/descriptors/meson.build| 2 +-
>  pc-bios/keymaps/meson.build| 6 +++---
>  pc-bios/meson.build| 2 +-
>  tools/virtiofsd/meson.build| 2 +-
>  trace/meson.build  | 2 +-
>  7 files changed, 10 insertions(+), 9 deletions(-)
> 

> diff --git a/meson.build b/meson.build
> index f0fe5f8799..20f20a7bfc 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -17,6 +17,7 @@ config_all_disas = keyval.load(meson.current_build_dir() / 
> 'config-all-disas.mak
>  enable_modules = 'CONFIG_MODULES' in config_host
>  enable_static = 'CONFIG_STATIC' in config_host
>  build_docs = 'BUILD_DOCS' in config_host
> +qemu_datadir = get_option('datadir') + get_option('confsuffix')

This needs to be

   get_option('datadir') / get_option('confsuffix')

to add the dir separator if we're using the default meson value
for "confsuffix" which lacks a leading "/".


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [PATCH v2 3/5] meson: add docdir option and pass pre-prefix qemu_docdir

[Daniel P . Berrangé]

On Tue, Aug 25, 2020 at 09:58:25PM +0400, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> When cross-compiling, by default qemu_docdir is 'c:\Program Files\QEMU\'
> which is not recognized as being an absolute path, and meson will end up
> adding the prefix again.
> 
> Add an option to pass docdir location to meson, pre-prefixed like we do
> with other directories and use that instead of config_host['qemu_docdir'].
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  configure | 1 +
>  docs/meson.build  | 4 ++--
>  meson.build   | 3 ++-
>  meson_options.txt | 1 +
>  4 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/configure b/configure
> index e19e2de2f0..e644841299 100755
> --- a/configure
> +++ b/configure
> @@ -8223,6 +8223,7 @@ NINJA=$PWD/ninjatool $meson setup \
>  --sysconfdir "${pre_prefix}$sysconfdir" \
>  --localstatedir "${pre_prefix}$local_statedir" \
>  -Dconfsuffix="$confsuffix" \
> +-Ddocdir="${pre_prefix}$qemu_docdir" \

This is passing an absolute path.


> diff --git a/meson_options.txt b/meson_options.txt
> index 7bb2c0fca9..fb9312fddd 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -1,4 +1,5 @@
>  option('confsuffix', type : 'string', value: 'qemu')
> +option('docdir', type : 'string', value : 'doc/qemu')

...but this default is a relative dir, presumably relative to
datadir.  The code expects an absolute dir.


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [PATCH v2 4/5] meson: use meson mandir instead of qemu_mandir

[Daniel P . Berrangé]

On Tue, Aug 25, 2020 at 09:58:26PM +0400, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> When cross-compiling, by default qemu_mandir is 'c:\Program
> Files\QEMU', which is not recognized as being an absolute path, and
> meson will end up adding the prefix again.
> 
> Use the pre-prefixed meson mandir option instead.
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  meson.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Daniel P. Berrangé 


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [PATCH v5 06/10] block: introduce BDRV_REQ_NO_WAIT flag

[Max Reitz]

On 26.08.20 08:26, Vladimir Sementsov-Ogievskiy wrote:
> 25.08.2020 16:10, Max Reitz wrote:
>> On 21.08.20 16:11, Vladimir Sementsov-Ogievskiy wrote:
>>> Add flag to make serialising request no wait: if there are conflicting
>>> requests, just return error immediately. It's will be used in upcoming
>>> preallocate filter.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy 
>>> ---
>>>   include/block/block.h |  9 -
>>>   block/io.c    | 11 ++-
>>>   2 files changed, 18 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/block/block.h b/include/block/block.h
>>> index b8f4e86e8d..877fda06a4 100644
>>> --- a/include/block/block.h
>>> +++ b/include/block/block.h
>>> @@ -67,8 +67,15 @@ typedef enum {
>>>    * written to qiov parameter which may be NULL.
>>>    */
>>>   BDRV_REQ_PREFETCH  = 0x200,
>>> +
>>> +    /*
>>> + * If we need to wait for other requests, just fail immediately.
>>> Used
>>> + * only together with BDRV_REQ_SERIALISING.
>>> + */
>>> +    BDRV_REQ_NO_WAIT = 0x400,
>>> +
>>>   /* Mask of valid flags */
>>> -    BDRV_REQ_MASK   = 0x3ff,
>>> +    BDRV_REQ_MASK   = 0x7ff,
>>>   } BdrvRequestFlags;
>>>     typedef struct BlockSizes {
>>> diff --git a/block/io.c b/block/io.c
>>> index dd28befb08..c93b1e98a3 100644
>>> --- a/block/io.c
>>> +++ b/block/io.c
>>> @@ -1912,9 +1912,18 @@ bdrv_co_write_req_prepare(BdrvChild *child,
>>> int64_t offset, uint64_t bytes,
>>>   assert(!(bs->open_flags & BDRV_O_INACTIVE));
>>>   assert((bs->open_flags & BDRV_O_NO_IO) == 0);
>>>   assert(!(flags & ~BDRV_REQ_MASK));
>>> +    assert(!((flags & BDRV_REQ_NO_WAIT) && !(flags &
>>> BDRV_REQ_SERIALISING)));
>>>     if (flags & BDRV_REQ_SERIALISING) {
>>> -    bdrv_make_request_serialising(req, bdrv_get_cluster_size(bs));
>>> +    QEMU_LOCK_GUARD(&bs->reqs_lock);
>>> +
>>> +    tracked_request_set_serialising(req,
>>> bdrv_get_cluster_size(bs));
>>> +
>>> +    if ((flags & BDRV_REQ_NO_WAIT) &&
>>> bdrv_find_conflicting_request(req)) {
>>
>> bdrv_find_conflicting_request() will return NULL even if there are
>> conflicting requests, but those have a non-NULL waiting_for.  Is that
>> something to consider?
>>
>> (I would like to think that will never have a real impact because then
>> we must find some other conflicting request; but isn’t is possible that
>> we find an overlapping request that waits for another request with which
>> it overlaps, but our request does not?)
>>
> 
> Actually check in bdrv_find_conflicting_request() is the same like in
> the following
> bdrv_wait_serialising_requests_locked(), so, if
> bdrv_find_conflicting_request() returns
> NULL, it means that in bdrv_wait_serialising_requests_locked() it will
> return NULL
> again (as there are no yield points) and we will not wait, so all is OK.

OK.  I thought that maybe we would want to avoid that other requests
might have to wait for the preallocation write.  (Of course, we can’t
avoid that altogether, but if we already know of such requests at the
beginning of the request...)

Well, if the only thing to look out for is that preallocation writes
themselves do not wait:

Reviewed-by: Max Reitz 

> And, why is it OK to ignore already waiting requests in
> bdrv_wait_serialising_requests_locked(): just because if we proceed now
> with our request,
> these waiting requests will have to wait for us, when they wake and go
> to the next iteration
> of waiting loop.

Sure.



signature.asc
Description: OpenPGP digital signature

Re: [PATCH v2 3/5] meson: add docdir option and pass pre-prefix qemu_docdir

[Marc-André Lureau]

Hi

On Wed, Aug 26, 2020 at 12:32 PM Daniel P. Berrangé 
wrote:

> On Tue, Aug 25, 2020 at 09:58:25PM +0400, marcandre.lur...@redhat.com
> wrote:
> > From: Marc-André Lureau 
> >
> > When cross-compiling, by default qemu_docdir is 'c:\Program Files\QEMU\'
> > which is not recognized as being an absolute path, and meson will end up
> > adding the prefix again.
> >
> > Add an option to pass docdir location to meson, pre-prefixed like we do
> > with other directories and use that instead of
> config_host['qemu_docdir'].
> >
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  configure | 1 +
> >  docs/meson.build  | 4 ++--
> >  meson.build   | 3 ++-
> >  meson_options.txt | 1 +
> >  4 files changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/configure b/configure
> > index e19e2de2f0..e644841299 100755
> > --- a/configure
> > +++ b/configure
> > @@ -8223,6 +8223,7 @@ NINJA=$PWD/ninjatool $meson setup \
> >  --sysconfdir "${pre_prefix}$sysconfdir" \
> >  --localstatedir "${pre_prefix}$local_statedir" \
> >  -Dconfsuffix="$confsuffix" \
> > +-Ddocdir="${pre_prefix}$qemu_docdir" \
>
> This is passing an absolute path.
>
>
> > diff --git a/meson_options.txt b/meson_options.txt
> > index 7bb2c0fca9..fb9312fddd 100644
> > --- a/meson_options.txt
> > +++ b/meson_options.txt
> > @@ -1,4 +1,5 @@
> >  option('confsuffix', type : 'string', value: 'qemu')
> > +option('docdir', type : 'string', value : 'doc/qemu')
>
> ...but this default is a relative dir, presumably relative to
> datadir.  The code expects an absolute dir.
>


Meson accepts both absolute and relative path for installation location. If
it's relative, it will be under the $prefix directory.


>
> Regards,
> Daniel
> --
> |: https://berrange.com  -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org -o-
> https://fstop138.berrange.com :|
> |: https://entangle-photo.org-o-
> https://www.instagram.com/dberrange :|
>
>
>

-- 
Marc-André Lureau

Re: [PATCH v2 5/5] meson: add NSIS building

[Daniel P . Berrangé]

On Tue, Aug 25, 2020 at 09:58:27PM +0400, marcandre.lur...@redhat.com wrote:
> From: Marc-André Lureau 
> 
> Signed-off-by: Marc-André Lureau 
> ---
>  Makefile| 56 ---
>  meson.build | 25 
>  scripts/nsis.py | 78 +
>  3 files changed, 103 insertions(+), 56 deletions(-)
>  create mode 100644 scripts/nsis.py

> diff --git a/scripts/nsis.py b/scripts/nsis.py
> new file mode 100644
> index 00..e1c409344e
> --- /dev/null
> +++ b/scripts/nsis.py
> @@ -0,0 +1,78 @@
> +#!/usr/bin/env python3
> +#
> +# Copyright (C) 2020 Red Hat, Inc.
> +#
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +
> +import argparse
> +import glob
> +import os
> +import shutil
> +import subprocess
> +import tempfile
> +
> +
> +def signcode(path):
> +cmd = os.environ.get("SIGNCODE")
> +if not cmd:
> +return
> +subprocess.run([cmd, path])

I know the existing makefile used  $SIGNCODE env variable, but I can't
help thinking it would be better to have it as a configure arg, and
in turn a meson arg.

None the less, it isn't worse than what already exists so

Reviewed-by: Daniel P. Berrangé 


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: linux-user static build broken

[Daniel P . Berrangé]

On Tue, Aug 25, 2020 at 10:36:13PM +0200, Laurent Vivier wrote:
> Hi,
> 
> since we have switched to meson, the statically linked binaries of qemu
> linux-user are broken:
> 
> cd $OBJ
> $SRC/configure --static --target-list=m68k-linux-user
> make
> ./qemu-m68k
> Segmentation fault (core dumped)
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0x77bd6833 in __dcigettext ()
> (gdb) bt
> #0  0x77bd6833 in __dcigettext ()
> #1  0x77bd5352 in __assert_fail ()
> #2  0x77c4d74c in _dl_relocate_static_pie ()
> #3  0x77bc713e in __libc_start_main ()
> #4  0x77a0029e in _start ()
> 
> If I build with --disable-pie it works again.
> 
> Any idea?

I'd suggest checking the compiler args used with v5.1.0 vs git master
and see if any flags related to PIE or similar changed. I already found
one bug wrt PIE on Windows builds this way.

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

[Bug 1893010] [NEW] qemu linux-user doesn't support OFD fcntl locks

[Mike Gelfand]

Public bug reported:

"Open file description locks (non-POSIX)", as they are described in
fcntl(2) man page, aren't supported by qemu-user  and attempting to use
those results in EINVAL. I'm on Gentoo with latest QEMU version
currently available (5.0.0-r2), and trying to emulate ppc64 and s390x on
x86_64.

Looking at linux-user/syscall.c, I'm guessing the issue is in (at least)
`target_to_host_fcntl_cmd` where switch reaches the default clause as
there're no cases for F_OFD_SETLK / F_OFD_SETLKW / F_OFD_GETLK.

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  qemu linux-user doesn't support OFD fcntl locks

Status in QEMU:
  New

Bug description:
  "Open file description locks (non-POSIX)", as they are described in
  fcntl(2) man page, aren't supported by qemu-user  and attempting to
  use those results in EINVAL. I'm on Gentoo with latest QEMU version
  currently available (5.0.0-r2), and trying to emulate ppc64 and s390x
  on x86_64.

  Looking at linux-user/syscall.c, I'm guessing the issue is in (at
  least) `target_to_host_fcntl_cmd` where switch reaches the default
  clause as there're no cases for F_OFD_SETLK / F_OFD_SETLKW /
  F_OFD_GETLK.

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

Re: linux-user static build broken

[Laurent Vivier]

Le 26/08/2020 à 10:44, Daniel P. Berrangé a écrit :
> On Tue, Aug 25, 2020 at 10:36:13PM +0200, Laurent Vivier wrote:
>> Hi,
>>
>> since we have switched to meson, the statically linked binaries of qemu
>> linux-user are broken:
>>
>> cd $OBJ
>> $SRC/configure --static --target-list=m68k-linux-user
>> make
>> ./qemu-m68k
>> Segmentation fault (core dumped)
>>
>> Program received signal SIGSEGV, Segmentation fault.
>> 0x77bd6833 in __dcigettext ()
>> (gdb) bt
>> #0  0x77bd6833 in __dcigettext ()
>> #1  0x77bd5352 in __assert_fail ()
>> #2  0x77c4d74c in _dl_relocate_static_pie ()
>> #3  0x77bc713e in __libc_start_main ()
>> #4  0x77a0029e in _start ()
>>
>> If I build with --disable-pie it works again.
>>
>> Any idea?
> 
> I'd suggest checking the compiler args used with v5.1.0 vs git master
> and see if any flags related to PIE or similar changed. I already found
> one bug wrt PIE on Windows builds this way.
> 
> Regards,
> Daniel
> 

It's what I'm doing.

There are both "--static-pie" and "--pie" on the new command line, but
keeping only the first doesn't fix the problem.

There is also a strange '-Wl,-rpath,RIGIN/' that would mean "make" is
not using $(ORIGIN) but $ORIGIN...

Thanks,
Laurent

Re: [PATCH v5 07/10] block: introduce preallocate filter

[Max Reitz]

On 26.08.20 08:49, Vladimir Sementsov-Ogievskiy wrote:
> 25.08.2020 18:11, Max Reitz wrote:
>> On 21.08.20 16:11, Vladimir Sementsov-Ogievskiy wrote:
>>> It's intended to be inserted between format and protocol nodes to
>>> preallocate additional space (expanding protocol file) on writes
>>> crossing EOF. It improves performance for file-systems with slow
>>> allocation.
>>>
>>> Signed-off-by: Vladimir Sementsov-Ogievskiy 
>>> ---
>>>   docs/system/qemu-block-drivers.rst.inc |  26 +++
>>>   qapi/block-core.json   |  20 +-
>>>   block/preallocate.c    | 291 +
>>>   block/Makefile.objs    |   1 +
>>>   4 files changed, 337 insertions(+), 1 deletion(-)
>>>   create mode 100644 block/preallocate.c

[...]

>>> diff --git a/block/preallocate.c b/block/preallocate.c
>>> new file mode 100644
>>> index 00..bdf54dbd2f
>>> --- /dev/null
>>> +++ b/block/preallocate.c
>>> @@ -0,0 +1,291 @@
>>> +/*
>>> + * preallocate filter driver
>>> + *
>>> + * The driver performs preallocate operation: it is injected above
>>> + * some node, and before each write over EOF it does additional
>>> preallocating
>>> + * write-zeroes request.
>>> + *
>>> + * Copyright (c) 2020 Virtuozzo International GmbH.
>>> + *
>>> + * 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 .
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +
>>> +#include "qapi/error.h"
>>> +#include "qemu/module.h"
>>> +#include "qemu/option.h"
>>> +#include "qemu/units.h"
>>> +#include "block/block_int.h"
>>> +
>>> +
>>> +typedef struct BDRVPreallocateState {
>>> +    int64_t prealloc_size;
>>> +    int64_t prealloc_align;
>>> +
>>> +    /*
>>> + * Filter is started as not-active, so it doesn't do any
>>> preallocations nor
>>> + * requires BLK_PERM_RESIZE on its child. This is needed to
>>> create filter
>>> + * above another node-child and then do bdrv_replace_node to
>>> insert the
>>> + * filter.
>>
>> A bit weird, but seems fair.  It’s basically just a cache for whether
>> this node has a writer on it or not.
>>
>> Apart from the weirdness, I don’t understand the “another node-child”.
>> Say you have “format” -> “proto”, and then you want to insert
>> “prealloc”.  Wouldn’t you blockdev-add prealloc,file=proto and then
>> blockdev-replace format.file=prealloc?
> 
> Yes something like this. Actually, I'm about inserting the filter
> automatically from block layer code, but your reasoning is about same
> thing and is better.
> 
>> So what is that “other node-child”?
> 
> Hmm, just my misleading wording. At least '-' in wrong place. Just
> "other node child", or "child of another node"..

OK.

>>> + *
>>> + * Filter becomes active the first time it detects that its
>>> parents have
>>> + * BLK_PERM_RESIZE on it.
>>> + * Filter becomes active forever: it doesn't lose active status
>>> if parents
>>> + * lose BLK_PERM_RESIZE, otherwise we'll not be able to shrink
>>> the file on
>>> + * filter close.
>>
>> Oh, the file is shrunk?  That wasn’t clear from the documentation.
>>
>> Hm.  Seems like useful behavior.  I just wonder if keeping the
>> permission around indefinitely makes sense.  Why not just truncate it
>> when the permission is revoked?
> 
> How? Parent is closed earlier, so on close we will not have the
> permission. So, we force-keep the permission up to our close().

I thought that preallocate_child_perm() would be invoked when the parent
is detached, so we could do the truncate there, before relinquishing
preallocate’s RESIZE permission.

[...]

>>> +static void preallocate_close(BlockDriverState *bs)
>>> +{
>>> +    BDRVPreallocateState *s = bs->opaque;
>>> +
>>> +    if (s->active && s->data_end >= 0 &&
>>> +    bdrv_getlength(bs->file->bs) > s->data_end)
>>> +    {
>>> +    bdrv_truncate(bs->file, s->data_end, true,
>>> PREALLOC_MODE_OFF, 0, NULL);
>>
>> Now that I think more about it...  What if there are other writers on
>> bs->file?  This may throw data away.
> 
> Good point. Actually, if bs->file has other writing parents, the logic
> of the filter
> around "data_end" is broken. So we must unshare WRITE and RESIZE
> permissions.

That’s certainly a heavy hammer, but it’d work.

>>  Maybe BDS.wr_highest_offset can
>> help to make a be

Re: [PATCH v2 2/5] meson: use meson datadir instead of qemu_datadir

[Marc-André Lureau]

Hi

On Wed, Aug 26, 2020 at 12:30 PM Daniel P. Berrangé 
wrote:

> On Tue, Aug 25, 2020 at 09:58:24PM +0400, marcandre.lur...@redhat.com
> wrote:
> > From: Marc-André Lureau 
> >
> > When cross-compiling, by default qemu_datadir is 'c:\Program
> > Files\QEMU', which is not recognized as being an absolute path, and
> > meson will end up adding the prefix again.
> >
> > Signed-off-by: Marc-André Lureau 
> > ---
> >  contrib/vhost-user-gpu/meson.build | 2 +-
> >  meson.build| 3 ++-
> >  pc-bios/descriptors/meson.build| 2 +-
> >  pc-bios/keymaps/meson.build| 6 +++---
> >  pc-bios/meson.build| 2 +-
> >  tools/virtiofsd/meson.build| 2 +-
> >  trace/meson.build  | 2 +-
> >  7 files changed, 10 insertions(+), 9 deletions(-)
> >
>
> > diff --git a/meson.build b/meson.build
> > index f0fe5f8799..20f20a7bfc 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -17,6 +17,7 @@ config_all_disas =
> keyval.load(meson.current_build_dir() / 'config-all-disas.mak
> >  enable_modules = 'CONFIG_MODULES' in config_host
> >  enable_static = 'CONFIG_STATIC' in config_host
> >  build_docs = 'BUILD_DOCS' in config_host
> > +qemu_datadir = get_option('datadir') + get_option('confsuffix')
>
> This needs to be
>
>get_option('datadir') / get_option('confsuffix')
>
> to add the dir separator if we're using the default meson value
> for "confsuffix" which lacks a leading "/".
>

right, fixed

-- 
Marc-André Lureau

[Bug 1893010] Re: qemu linux-user doesn't support OFD fcntl locks

[Mike Gelfand]

** Summary changed:

- qemu-user doesn't support OFD fcntl locks
+ qemu linux-user doesn't support OFD fcntl locks

** Tags added: linux-user

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

Title:
  qemu linux-user doesn't support OFD fcntl locks

Status in QEMU:
  New

Bug description:
  "Open file description locks (non-POSIX)", as they are described in
  fcntl(2) man page, aren't supported by qemu-user  and attempting to
  use those results in EINVAL. I'm on Gentoo with latest QEMU version
  currently available (5.0.0-r2), and trying to emulate ppc64 and s390x
  on x86_64.

  Looking at linux-user/syscall.c, I'm guessing the issue is in (at
  least) `target_to_host_fcntl_cmd` where switch reaches the default
  clause as there're no cases for F_OFD_SETLK / F_OFD_SETLKW /
  F_OFD_GETLK.

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

[Bug 1893003] Re: qemu linux-user doesn't translate host/target data for iovec I/O

[Mike Gelfand]

** Summary changed:

- qemu-user doesn't translate host/target data for iovec I/O
+ qemu linux-user doesn't translate host/target data for iovec I/O

** Tags added: linux-user

** Tags added: ppc s390x

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

Title:
  qemu linux-user doesn't translate host/target data for iovec I/O

Status in QEMU:
  New

Bug description:
  When using iovec I/O functions (like `readv`), no data translation
  happens. I'm hitting this issue with libevent upon constructing a
  bufferevent over an inotify descriptor, and then building for either
  ppc64 or s390x (both big-endian) on x86_64 (little-endian) and running
  resulting code with qemu-ppc64 or qemu-s390x on Gentoo using latest
  QEMU version available (5.0.0-r2).

  The code in question is in
  https://github.com/transmission/transmission/blob/master/libtransmission
  /watchdir-inotify.c (`tr_watchdir_inotify_new`,
  `tr_watchdir_inotify_on_event`).

  While `read` syscall is handled properly, `readv` (which libevent is
  using in my case) doesn't have any logic to call
  `host_to_target_data_inotify` or any other translation function,
  leaving inotify data unchanged (with values in little-endian), which
  then leads to unit test failures. Quoting `do_syscall1` implementation
  bits for the reference:

  ---8<---begin---
  case TARGET_NR_read:
  if (arg2 == 0 && arg3 == 0) {
  return get_errno(safe_read(arg1, 0, 0));
  } else {
  if (!(p = lock_user(VERIFY_WRITE, arg2, arg3, 0)))
  return -TARGET_EFAULT;
  ret = get_errno(safe_read(arg1, p, arg3));
  if (ret >= 0 &&
  fd_trans_host_to_target_data(arg1)) {
  ret = fd_trans_host_to_target_data(arg1)(p, ret);
  }
  unlock_user(p, arg2, ret);
  }
  return ret;
  ...
  case TARGET_NR_readv:
  {
  struct iovec *vec = lock_iovec(VERIFY_WRITE, arg2, arg3, 0);
  if (vec != NULL) {
  ret = get_errno(safe_readv(arg1, vec, arg3));
  unlock_iovec(vec, arg2, arg3, 1);
  } else {
  ret = -host_to_target_errno(errno);
  }
  }
  return ret;
  ---8<---end---

  To reiterate, the issue is not only with `readv` but with other iovec
  functions as well.

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

Re: [PATCH v5 06/10] block: introduce BDRV_REQ_NO_WAIT flag

[Vladimir Sementsov-Ogievskiy]

26.08.2020 11:36, Max Reitz wrote:

On 26.08.20 08:26, Vladimir Sementsov-Ogievskiy wrote:

25.08.2020 16:10, Max Reitz wrote:

On 21.08.20 16:11, Vladimir Sementsov-Ogievskiy wrote:

Add flag to make serialising request no wait: if there are conflicting
requests, just return error immediately. It's will be used in upcoming
preallocate filter.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
   include/block/block.h |  9 -
   block/io.c    | 11 ++-
   2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/include/block/block.h b/include/block/block.h
index b8f4e86e8d..877fda06a4 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -67,8 +67,15 @@ typedef enum {
    * written to qiov parameter which may be NULL.
    */
   BDRV_REQ_PREFETCH  = 0x200,
+
+    /*
+ * If we need to wait for other requests, just fail immediately.
Used
+ * only together with BDRV_REQ_SERIALISING.
+ */
+    BDRV_REQ_NO_WAIT = 0x400,
+
   /* Mask of valid flags */
-    BDRV_REQ_MASK   = 0x3ff,
+    BDRV_REQ_MASK   = 0x7ff,
   } BdrvRequestFlags;
     typedef struct BlockSizes {
diff --git a/block/io.c b/block/io.c
index dd28befb08..c93b1e98a3 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1912,9 +1912,18 @@ bdrv_co_write_req_prepare(BdrvChild *child,
int64_t offset, uint64_t bytes,
   assert(!(bs->open_flags & BDRV_O_INACTIVE));
   assert((bs->open_flags & BDRV_O_NO_IO) == 0);
   assert(!(flags & ~BDRV_REQ_MASK));
+    assert(!((flags & BDRV_REQ_NO_WAIT) && !(flags &
BDRV_REQ_SERIALISING)));
     if (flags & BDRV_REQ_SERIALISING) {
-    bdrv_make_request_serialising(req, bdrv_get_cluster_size(bs));
+    QEMU_LOCK_GUARD(&bs->reqs_lock);
+
+    tracked_request_set_serialising(req,
bdrv_get_cluster_size(bs));
+
+    if ((flags & BDRV_REQ_NO_WAIT) &&
bdrv_find_conflicting_request(req)) {


bdrv_find_conflicting_request() will return NULL even if there are
conflicting requests, but those have a non-NULL waiting_for.  Is that
something to consider?

(I would like to think that will never have a real impact because then
we must find some other conflicting request; but isn’t is possible that
we find an overlapping request that waits for another request with which
it overlaps, but our request does not?)



Actually check in bdrv_find_conflicting_request() is the same like in
the following
bdrv_wait_serialising_requests_locked(), so, if
bdrv_find_conflicting_request() returns
NULL, it means that in bdrv_wait_serialising_requests_locked() it will
return NULL
again (as there are no yield points) and we will not wait, so all is OK.


OK.  I thought that maybe we would want to avoid that other requests
might have to wait for the preallocation write.  (Of course, we can’t
avoid that altogether, but if we already know of such requests at the
beginning of the request...)



Hmm, I see your point now. Hmm actually, what we want:

1. make preallocation on write
2. serialize the request
3. if there are conflicting requests do nothing, as (most probably) the
conflicting request will do preallocation itself

So, what we can say about intersecting requests, which are waiting for something (and 
therefore are not "conflicting")?
Aha, for sure, they don't preallocate (otherwise they don't wait). So [3] is 
still satisfied..

There was not original aim to not make the other parallel request wait for 
preallocation. So, it may be done later.. Intuitively, I don't expect the 
benefit: if there is no concurrent preallocate request, who are these 
intersecting requests? For example, EOF-crossing READS, waiting for some 
serialized not preallocating (so fit into file-size) WRITE. But this shouldn't 
happen often :)


Well, if the only thing to look out for is that preallocation writes
themselves do not wait:


So, let's assume that yes, we care that they don't wait themselves (mostly to 
avoid concurrent preallocation requests) and don't care about other 
intersecting requests.



Reviewed-by: Max Reitz 


Thanks!




And, why is it OK to ignore already waiting requests in
bdrv_wait_serialising_requests_locked(): just because if we proceed now
with our request,
these waiting requests will have to wait for us, when they wake and go
to the next iteration
of waiting loop.


Sure.




--
Best regards,
Vladimir

Re: device compatibility interface for live migration with assigned devices

[Yan Zhao]

On Thu, Aug 20, 2020 at 02:24:26PM +0100, Sean Mooney wrote:
> On Thu, 2020-08-20 at 14:27 +0800, Yan Zhao wrote:
> > On Thu, Aug 20, 2020 at 06:16:28AM +0100, Sean Mooney wrote:
> > > On Thu, 2020-08-20 at 12:01 +0800, Yan Zhao wrote:
> > > > On Thu, Aug 20, 2020 at 02:29:07AM +0100, Sean Mooney wrote:
> > > > > On Thu, 2020-08-20 at 08:39 +0800, Yan Zhao wrote:
> > > > > > On Tue, Aug 18, 2020 at 11:36:52AM +0200, Cornelia Huck wrote:
> > > > > > > On Tue, 18 Aug 2020 10:16:28 +0100
> > > > > > > Daniel P. Berrangé  wrote:
> > > > > > > 
> > > > > > > > On Tue, Aug 18, 2020 at 05:01:51PM +0800, Jason Wang wrote:
> > > > > > > > >On 2020/8/18 下午4:55, Daniel P. Berrangé wrote:
> > > > > > > > > 
> > > > > > > > >  On Tue, Aug 18, 2020 at 11:24:30AM +0800, Jason Wang wrote:
> > > > > > > > > 
> > > > > > > > >  On 2020/8/14 下午1:16, Yan Zhao wrote:
> > > > > > > > > 
> > > > > > > > >  On Thu, Aug 13, 2020 at 12:24:50PM +0800, Jason Wang wrote:
> > > > > > > > > 
> > > > > > > > >  On 2020/8/10 下午3:46, Yan Zhao wrote:  
> > > > > > > > >  we actually can also retrieve the same information through 
> > > > > > > > > sysfs, .e.g
> > > > > > > > > 
> > > > > > > > >  |- [path to device]
> > > > > > > > > |--- migration
> > > > > > > > > | |--- self
> > > > > > > > > | |   |---device_api
> > > > > > > > > ||   |---mdev_type
> > > > > > > > > ||   |---software_version
> > > > > > > > > ||   |---device_id
> > > > > > > > > ||   |---aggregator
> > > > > > > > > | |--- compatible
> > > > > > > > > | |   |---device_api
> > > > > > > > > ||   |---mdev_type
> > > > > > > > > ||   |---software_version
> > > > > > > > > ||   |---device_id
> > > > > > > > > ||   |---aggregator
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > >  Yes but:
> > > > > > > > > 
> > > > > > > > >  - You need one file per attribute (one syscall for one 
> > > > > > > > > attribute)
> > > > > > > > >  - Attribute is coupled with kobject
> > > > > > > 
> > > > > > > Is that really that bad? You have the device with an embedded 
> > > > > > > kobject
> > > > > > > anyway, and you can just put things into an attribute group?
> > > > > > > 
> > > > > > > [Also, I think that self/compatible split in the example makes 
> > > > > > > things
> > > > > > > needlessly complex. Shouldn't semantic versioning and matching 
> > > > > > > already
> > > > > > > cover nearly everything? I would expect very few cases that are 
> > > > > > > more
> > > > > > > complex than that. Maybe the aggregation stuff, but I don't think 
> > > > > > > we
> > > > > > > need that self/compatible split for that, either.]
> > > > > > 
> > > > > > Hi Cornelia,
> > > > > > 
> > > > > > The reason I want to declare compatible list of attributes is that
> > > > > > sometimes it's not a simple 1:1 matching of source attributes and 
> > > > > > target attributes
> > > > > > as I demonstrated below,
> > > > > > source mdev of (mdev_type i915-GVTg_V5_2 + aggregator 1) is 
> > > > > > compatible to
> > > > > > target mdev of (mdev_type i915-GVTg_V5_4 + aggregator 2),
> > > > > >(mdev_type i915-GVTg_V5_8 + aggregator 4)
> > > > > 
> > > > > the way you are doing the nameing is till really confusing by the way
> > > > > if this has not already been merged in the kernel can you chagne the 
> > > > > mdev
> > > > > so that mdev_type i915-GVTg_V5_2 is 2 of mdev_type i915-GVTg_V5_1 
> > > > > instead of half the device
> > > > > 
> > > > > currently you need to deived the aggratod by the number at the end of 
> > > > > the mdev type to figure out
> > > > > how much of the phsicial device is being used with is a very unfridly 
> > > > > api convention
> > > > > 
> > > > > the way aggrator are being proposed in general is not really someting 
> > > > > i like but i thin this at least
> > > > > is something that should be able to correct.
> > > > > 
> > > > > with the complexity in the mdev type name + aggrator i suspect that 
> > > > > this will never be support
> > > > > in openstack nova directly requireing integration via cyborg unless 
> > > > > we can pre partion the
> > > > > device in to mdevs staicaly and just ignore this.
> > > > > 
> > > > > this is way to vendor sepecif to integrate into something like 
> > > > > openstack in nova unless we can guarentee
> > > > > taht how aggreator work will be portable across vendors genericly.
> > > > > 
> > > > > > 
> > > > > > and aggragator may be just one of such examples that 1:1 matching 
> > > > > > does not
> > > > > > fit.
> > > > > 
> > > > > for openstack nova i dont see us support anything beyond the 1:1 case 
> > > > > where the mdev type does not change.
> > > > > 
> > > > 
> > > > hi Sean,
> > > > I understand it's hard for openstack. but 1:N is always meaningful.
> > > > e.g.
> > > > if source device 1 has cap A, it is compatible to
> > > > device 2: cap A,
> > > > device 3: cap A+B,
> > > > device 4: cap A+B+C
>

Re: linux-user static build broken

[Paolo Bonzini]

$ORIGIN is a special literal used by ld.so. It's probably fixed by the same
patch that was posted for msys.

Paolo

Il mer 26 ago 2020, 10:51 Laurent Vivier  ha scritto:

> Le 26/08/2020 à 10:44, Daniel P. Berrangé a écrit :
> > On Tue, Aug 25, 2020 at 10:36:13PM +0200, Laurent Vivier wrote:
> >> Hi,
> >>
> >> since we have switched to meson, the statically linked binaries of qemu
> >> linux-user are broken:
> >>
> >> cd $OBJ
> >> $SRC/configure --static --target-list=m68k-linux-user
> >> make
> >> ./qemu-m68k
> >> Segmentation fault (core dumped)
> >>
> >> Program received signal SIGSEGV, Segmentation fault.
> >> 0x77bd6833 in __dcigettext ()
> >> (gdb) bt
> >> #0  0x77bd6833 in __dcigettext ()
> >> #1  0x77bd5352 in __assert_fail ()
> >> #2  0x77c4d74c in _dl_relocate_static_pie ()
> >> #3  0x77bc713e in __libc_start_main ()
> >> #4  0x77a0029e in _start ()
> >>
> >> If I build with --disable-pie it works again.
> >>
> >> Any idea?
> >
> > I'd suggest checking the compiler args used with v5.1.0 vs git master
> > and see if any flags related to PIE or similar changed. I already found
> > one bug wrt PIE on Windows builds this way.
> >
> > Regards,
> > Daniel
> >
>
> It's what I'm doing.
>
> There are both "--static-pie" and "--pie" on the new command line, but
> keeping only the first doesn't fix the problem.
>
> There is also a strange '-Wl,-rpath,RIGIN/' that would mean "make" is
> not using $(ORIGIN) but $ORIGIN...
>
> Thanks,
> Laurent
>
>

Re: [PATCH v5 07/10] block: introduce preallocate filter

[Vladimir Sementsov-Ogievskiy]

26.08.2020 11:52, Max Reitz wrote:

On 26.08.20 08:49, Vladimir Sementsov-Ogievskiy wrote:

25.08.2020 18:11, Max Reitz wrote:

On 21.08.20 16:11, Vladimir Sementsov-Ogievskiy wrote:

It's intended to be inserted between format and protocol nodes to
preallocate additional space (expanding protocol file) on writes
crossing EOF. It improves performance for file-systems with slow
allocation.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
   docs/system/qemu-block-drivers.rst.inc |  26 +++
   qapi/block-core.json   |  20 +-
   block/preallocate.c    | 291 +
   block/Makefile.objs    |   1 +
   4 files changed, 337 insertions(+), 1 deletion(-)
   create mode 100644 block/preallocate.c


[...]


diff --git a/block/preallocate.c b/block/preallocate.c
new file mode 100644
index 00..bdf54dbd2f
--- /dev/null
+++ b/block/preallocate.c
@@ -0,0 +1,291 @@
+/*
+ * preallocate filter driver
+ *
+ * The driver performs preallocate operation: it is injected above
+ * some node, and before each write over EOF it does additional
preallocating
+ * write-zeroes request.
+ *
+ * Copyright (c) 2020 Virtuozzo International GmbH.
+ *
+ * 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 .
+ */
+
+#include "qemu/osdep.h"
+
+#include "qapi/error.h"
+#include "qemu/module.h"
+#include "qemu/option.h"
+#include "qemu/units.h"
+#include "block/block_int.h"
+
+
+typedef struct BDRVPreallocateState {
+    int64_t prealloc_size;
+    int64_t prealloc_align;
+
+    /*
+ * Filter is started as not-active, so it doesn't do any
preallocations nor
+ * requires BLK_PERM_RESIZE on its child. This is needed to
create filter
+ * above another node-child and then do bdrv_replace_node to
insert the
+ * filter.


A bit weird, but seems fair.  It’s basically just a cache for whether
this node has a writer on it or not.

Apart from the weirdness, I don’t understand the “another node-child”.
Say you have “format” -> “proto”, and then you want to insert
“prealloc”.  Wouldn’t you blockdev-add prealloc,file=proto and then
blockdev-replace format.file=prealloc?


Yes something like this. Actually, I'm about inserting the filter
automatically from block layer code, but your reasoning is about same
thing and is better.


So what is that “other node-child”?


Hmm, just my misleading wording. At least '-' in wrong place. Just
"other node child", or "child of another node"..


OK.


+ *
+ * Filter becomes active the first time it detects that its
parents have
+ * BLK_PERM_RESIZE on it.
+ * Filter becomes active forever: it doesn't lose active status
if parents
+ * lose BLK_PERM_RESIZE, otherwise we'll not be able to shrink
the file on
+ * filter close.


Oh, the file is shrunk?  That wasn’t clear from the documentation.

Hm.  Seems like useful behavior.  I just wonder if keeping the
permission around indefinitely makes sense.  Why not just truncate it
when the permission is revoked?


How? Parent is closed earlier, so on close we will not have the
permission. So, we force-keep the permission up to our close().


I thought that preallocate_child_perm() would be invoked when the parent
is detached, so we could do the truncate there, before relinquishing
preallocate’s RESIZE permission.



Hmm, I can check it. I just a bit afraid of doing something serious like 
truncation in .bdrv_child_perm() handler, which doesn't seem to imply such 
usage.




+static void preallocate_close(BlockDriverState *bs)
+{
+    BDRVPreallocateState *s = bs->opaque;
+
+    if (s->active && s->data_end >= 0 &&
+    bdrv_getlength(bs->file->bs) > s->data_end)
+    {
+    bdrv_truncate(bs->file, s->data_end, true,
PREALLOC_MODE_OFF, 0, NULL);


Now that I think more about it...  What if there are other writers on
bs->file?  This may throw data away.


Good point. Actually, if bs->file has other writing parents, the logic
of the filter
around "data_end" is broken. So we must unshare WRITE and RESIZE
permissions.


That’s certainly a heavy hammer, but it’d work.


  Maybe BDS.wr_highest_offset can
help to make a better call?


Anyway, we'll have to use wr_highest_offset of the filter not the child
node
(in the child wr_highest_offset will track preallocations as well),


That’s true.


so we want to unshare WRITE/RESIZE per

[Bug 1892604] Re: qemu-system-arm: ../hw/usb/hcd-dwc2.c:666: dwc2_glbreg_read: Assertion `addr <= GINTSTS2' failed.

[Peter Maydell]

It is still a bug in QEMU -- we shouldn't allow guest behaviour to make
QEMU assert(). If there's unimplemented functionality in the USB
controller model that can be logged with qemu_log_mask(LOG_UNIMP, ...)

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

Title:
  qemu-system-arm: ../hw/usb/hcd-dwc2.c:666: dwc2_glbreg_read: Assertion
  `addr <= GINTSTS2' failed.

Status in QEMU:
  New

Bug description:
  When trying to run the 2016-05-27 Raspbian image on the emulated
  raspi2 platform, the system boots but shortly after the login prompt
  QEMU (master; commit ID ca489cd037e4d50dc6c40570a167504ad7e5a521) dies
  with:

  qemu-system-arm: ../hw/usb/hcd-dwc2.c:666: dwc2_glbreg_read: Assertion
  `addr <= GINTSTS2' failed.

  Steps to reproduce:

  1. Get the image: wget
  
http://downloads.raspberrypi.org/raspbian/images/raspbian-2016-05-31/2016-05-27
  -raspbian-jessie.zip

  2. Extract the kernel image and DTB:

  sudo losetup -f --show -P 2016-05-27-raspbian-jessie.img
  sudo mkdir /mnt/rpi
  sudo mount /dev/loop11p1 /mnt/rpi/
  cp /mnt/rpi/kernel7.img . 



  cp /mnt/rpi/bcm2709-rpi-2-b.dtb . 



  sudo umount /mnt/rpi 
  sudo losetup -d /dev/loop11 

  3. Run QEMU:
  qemu-system-arm -M raspi2 -m 1G -dtb bcm2709-rpi-2-b.dtb -kernel kernel7.img 
-append "rw earlyprintk loglevel=8 console=ttyAMA0,115200 dwc_otg.lpm_enable=0 
root=/dev/mmcblk0p2" -sd 2016-05-27-raspbian-jessie.img -smp 4 -serial stdio 
-display none

  A few seconds after the login prompt is displayed, QEMU will exit with
  the assertion failure.

  I also tried changing all of the asserts to if statements that (for
  MMIO reads) returned 0 and (for writes) just returned, but this
  resulted in a non-responsive system.

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

Re: [PATCH v2 6/7] x68: acpi: trigger SMI before sending hotplug Notify event to OSPM

[Igor Mammedov]

On Tue, 25 Aug 2020 19:25:48 +0200
Laszlo Ersek  wrote:

> On 08/18/20 14:22, Igor Mammedov wrote:
> > In case firmware has negotiated CPU hotplug SMI feature, generate
> > AML to describe SMI IO port region and send SMI to firmware
> > on each CPU hotplug SCI in case new CPUs were hotplugged.
> >
> > Since new CPUs can be hotplugged while CPU_SCAN_METHOD is running
> > we can't send SMI before new CPUs are fetched from QEMU as it
> > could cause sending Notify to a CPU that firmware hasn't seen yet.
> > So fetch new CPUs into local cache first and then send SMI and
> > after that sends Notify events to cached CPUs. This should ensure
> > that Notify is sent only to CPUs which were processed by firmware
> > first.
> > Any CPUs that were hotplugged after caching will be processed
> > by the next CPU_SCAN_METHOD, when pending SCI is handled.
> >
> > Signed-off-by: Igor Mammedov 
> > ---
> > v2:
> >  - clear insert event after firmware has returned
> >control from SMI. (Laszlo Ersek )

Laszlo,
Thanks a lot for such detailed review
I'll respin series with your feed back incorporated.

[...]

Re: [PATCH v5 02/12] migration/dirtyrate: add DirtyRateStatus to denote calculation status

[David Edmondson]

Minor wordsmithing...

On Monday, 2020-08-24 at 17:14:30 +08, Chuan Zheng wrote:

> add DirtyRateStatus to denote calculating status.
>
> Signed-off-by: Chuan Zheng 
> ---
>  migration/dirtyrate.c | 22 ++
>  qapi/migration.json   | 17 +
>  2 files changed, 39 insertions(+)
>
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index 366f4e9..91987c5 100644
> --- a/migration/dirtyrate.c
> +++ b/migration/dirtyrate.c
> @@ -23,6 +23,19 @@
>  #include "migration.h"
>  #include "dirtyrate.h"
>  
> +static int CalculatingState = DIRTY_RATE_STATUS_UNSTARTED;
> +
> +static int dirtyrate_set_state(int *state, int old_state, int new_state)
> +{
> +assert(new_state < DIRTY_RATE_STATUS__MAX);
> +if (atomic_cmpxchg(state, old_state, new_state) == old_state) {
> +return 0;
> +} else {
> +return -1;
> +}
> +}
> +
> +
>  static void calculate_dirtyrate(struct DirtyRateConfig config)
>  {
>  /* todo */
> @@ -32,8 +45,17 @@ static void calculate_dirtyrate(struct DirtyRateConfig 
> config)
>  void *get_dirtyrate_thread(void *arg)
>  {
>  struct DirtyRateConfig config = *(struct DirtyRateConfig *)arg;
> +int ret;
> +
> +ret = dirtyrate_set_state(&CalculatingState, DIRTY_RATE_STATUS_UNSTARTED,
> +  DIRTY_RATE_STATUS_MEASURING);
> +if (ret == -1) {
> +return NULL;
> +}
>  
>  calculate_dirtyrate(config);
>  
> +ret = dirtyrate_set_state(&CalculatingState, DIRTY_RATE_STATUS_MEASURING,
> +  DIRTY_RATE_STATUS_MEASURED);
>  return NULL;
>  }
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 5f6b061..d640165 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -1720,3 +1720,20 @@
>  ##
>  { 'event': 'UNPLUG_PRIMARY',
>'data': { 'device-id': 'str' } }
> +
> +##
> +# @DirtyRateStatus:
> +#
> +# An enumeration of dirtyrate status.
> +#
> +# @unstarted: query-dirtyrate thread is not initial.

@unstarted: the dirtyrate thread has not been started.

> +#
> +# @measuring: query-dirtyrate thread is created and start to measure.

@measuring: the dirtyrate thread is measuring.

> +#
> +# @measured:  query-dirtyrate thread is end, we can get result.

@measured: the dirtyrate thread has measured and results are available.

> +#
> +# Since: 5.2
> +#
> +##
> +{ 'enum': 'DirtyRateStatus',
> +  'data': [ 'unstarted', 'measuring', 'measured'] }
> -- 
> 1.8.3.1

dme.
-- 
Tonight I'm gonna bury that horse in the ground.

Re: [PATCH v2 6/7] x68: acpi: trigger SMI before sending hotplug Notify event to OSPM

[Laszlo Ersek]

Hi Igor,

On 08/25/20 19:25, Laszlo Ersek wrote:

> So I would suggest fetching the CNEW array element back into "uid"
> first, then using "uid" for both the NOTIFY call, and the (currently
> missing) restoration of CSEL. Then we can write 1 to CINS.
>
> Expressed as a patch on top of yours:
>
>> diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
>> index 4864c3b39694..2bea6144fd5e 100644
>> --- a/hw/acpi/cpu.c
>> +++ b/hw/acpi/cpu.c
>> @@ -564,8 +564,11 @@ void build_cpus_aml(Aml *table, MachineState *machine, 
>> CPUHotplugFeatures opts,
>>  aml_append(method, aml_store(zero, cpu_idx));
>>  while_ctx = aml_while(aml_lless(cpu_idx, num_added_cpus));
>>  {
>> -aml_append(while_ctx, aml_call2(CPU_NOTIFY_METHOD,
>> -aml_derefof(aml_index(new_cpus, cpu_idx)), dev_chk));
>> +aml_append(while_ctx,
>> +aml_store(aml_derefof(aml_index(new_cpus, cpu_idx)), 
>> uid));
>> +aml_append(while_ctx,
>> +aml_call2(CPU_NOTIFY_METHOD, uid, dev_chk));
>> +aml_append(while_ctx, aml_store(uid, cpu_selector));
>>  aml_append(while_ctx, aml_store(one, ins_evt));
>>  aml_append(while_ctx, aml_increment(cpu_idx));
>>  }
>
> This effects the following change, in the decompiled method:
>
>> @@ -37,15 +37,17 @@
>>  If ((Local_NumAddedCpus != Zero))
>>  {
>>  \_SB.PCI0.SMI0.SMIC = 0x04
>>  }
>>
>>  Local_CpuIdx = Zero
>>  While ((Local_CpuIdx < Local_NumAddedCpus))
>>  {
>> -CTFY (DerefOf (CNEW [Local_CpuIdx]), One)
>> +Local_Uid = DerefOf (CNEW [Local_CpuIdx])
>> +CTFY (Local_Uid, One)
>> +\_SB.PCI0.PRES.CSEL = Local_Uid
>>  \_SB.PCI0.PRES.CINS = One
>>  Local_CpuIdx++
>>  }
>>
>>  Release (\_SB.PCI0.PRES.CPLK)
>>  }
>
> With this change, the
>
>   virsh setvcpus DOMAIN 8 --live
>
> command works for me. The topology in my test domain has CPU#0 and
> CPU#2 cold-plugged, so the command adds 6 VCPUs. Viewed from the
> firmware side, the 6 "device_add" commands, issued in close succession
> by libvirtd, coalesce into 4 "batches". (And of course the firmware
> sees the 4 batches back-to-back.)

unfortunately, with more testing, I have run into two more races:

(1) When a "device_add" occurs after the ACPI loop collects the CPUS
from the register block, but before the SMI.

Here, the "stray CPU" is processed fine by the firmware. However,
the CTFY loop in ACPI does not know about the CPU, so it doesn't
clear the pending insert event for it. And when the firmware is
entered with an SMI for the *next* time, the firmware sees the same
CPU *again* as pending, and tries to relocate it again. Bad things
happen.

(2) When a "device_add" occurs after the SMI, but before the firmware
collects the pending CPUs from the register block.

Here, the firmware collects the "stray CPU". However, the "broadcast
SMI", with which we entered the firmware, did *not* cover the stray
CPU -- the CPU_FOREACH() loop in ich9_apm_ctrl_changed() could not
make the SMI pending for the new CPU, because at that time, the CPU
had not been added yet. As a result, when the firmware sends an
INIT-SIPI-SIPI to the new CPU, expecting it to boot right into SMM,
the new CPU instead boots straight into the post-RSM (normal mode)
"pen", skipping its initial SMI handler. Meaning that the CPU halts
nicely, but its SMBASE is never relocated, and the SMRAM message
exchange with the BSP falls apart.

Possible mitigations I can think of:

For problem (1):

  (1a) Change the firmware so it notices that it has relocated the
   "stray" CPU before -- such CPUs should be simply skipped in the
   firmware. The next time the CTFY loop runs in ACPI, it will clear
   the pending event.

  (1b) Alternatively, stop consuming the hotplug register block in the
   firmware altogether, and work out general message passing, from
   ACPI to firmware. See the outline here:

 
http://mid.mail-archive.com/cf887d74-f65d-602a-9629-3d25cef93a69@redhat.com

For problem (2):

  (2a) Change the firmware so that it sends a directed SMI as well to
   each CPU, just before sending an INIT-SIPI-SIPI. This should be
   idempotent -- if the broadcast SMI *has* covered the the CPU,
   then sending a directed SMI should make no difference.

  (2b) Alternatively, change the "device_add" command in QEMU so that,
   if "CPU hotplug with SMI" has been negotiated, the new CPU is
   added with the SMI made pending for it at once. (That is, no
   hot-plugged CPU would exist with the directed SMI *not* pending
   for it.)

  (2c) Alternatively, approach (1b) would fix problem (2) as well -- the
   firmware would only relocate such CPUs that ACPI collected before
   injecting the SMI. So all those CPUs would have the

Re: [PULL 00/18] riscv-to-apply queue

[Peter Maydell]

On Wed, 26 Aug 2020 at 04:21, Bin Meng  wrote:
> On Wed, Aug 26, 2020 at 6:41 AM Alistair Francis  wrote:
> > Richard and Philippe review patches and some of the RISC-V patches get
> > reviewed by the RISC-V community. The main problem (which is a common
> > problem in open source) is that large technical patch series just get
> > ignored.
>
> Yep, I am only comfortable reviewing patches which I have confidence
> in. Right now I am not working on any H- or V - extension for RISC-V
> so I cannot contribute to any review of these large numbers of H- or
> V- extension related patches. Sorry!

So, everybody has a ton of work they need to do and only a limited
amount of time they might have for code review, so it's important to
prioritise. But I would encourage you, and other people contributing
to RISC-V parts of QEMU, to at least sometimes review changes that are
a little bit out of your "comfort zone" if nobody else seems to be
doing so. Review can find bugs, areas that are confusing or need
comments, etc, even without a thorough knowledge of the relevant spec.
(In fact, not knowing the spec can help in identifying where
explanatory comments can help the reader!) And for the project it means
we have more people who at least have some idea of what that bit of code
is doing. Review that is limited to "this code seems to make sense but
I haven't checked it against the spec" is better than patches getting
no review at all, I think. And it's a good way to build your knowledge
of the codebase and the architecture over time.

thanks
-- PMM

Re: [PULL 00/18] riscv-to-apply queue

[Peter Maydell]

On Tue, 25 Aug 2020 at 20:01, Alistair Francis  wrote:
>
> The following changes since commit 7774e403f2ac58b3e87bfe8d2f77676501ba893e:
>
>   Merge remote-tracking branch 
> 'remotes/kraxel/tags/fixes-20200825-pull-request' into staging (2020-08-25 
> 10:54:51 +0100)
>
> are available in the Git repository at:
>
>   g...@github.com:alistair23/qemu.git tags/pull-riscv-to-apply-20200825
>
> for you to fetch changes up to e39a8320b088dd5efc9ebaafe387e52b3d962665:
>
>   target/riscv: Support the Virtual Instruction fault (2020-08-25 09:11:36 
> -0700)
>
> 
> This pull request first adds support for multi-socket NUMA RISC-V
> machines. The Spike and Virt machines both support NUMA sockets.
>
> This PR also updates the current experimental Hypervisor support to the
> v0.6.1 spec.
>


Applied, thanks.

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

-- PMM

Re: [PATCH v5 03/12] migration/dirtyrate: Add RamlockDirtyInfo to store sampled page info

[David Edmondson]

On Monday, 2020-08-24 at 17:14:31 +08, Chuan Zheng wrote:

> Add RamlockDirtyInfo to store sampled page info of each ramblock.
>
> Signed-off-by: Chuan Zheng 
> ---
>  migration/dirtyrate.h | 18 ++
>  1 file changed, 18 insertions(+)
>
> diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
> index 33669b7..7da 100644
> --- a/migration/dirtyrate.h
> +++ b/migration/dirtyrate.h
> @@ -19,6 +19,11 @@
>   */
>  #define DIRTYRATE_DEFAULT_SAMPLE_PAGES512
>  
> +/*
> + * Record ramblock idstr
> + */
> +#define RAMBLOCK_INFO_MAX_LEN 256
> +
>  /* Take 1s as default for calculation duration */
>  #define DEFAULT_FETCH_DIRTYRATE_TIME_SEC  1
>  
> @@ -27,6 +32,19 @@ struct DirtyRateConfig {
>  int64_t sample_period_seconds; /* time duration between two sampling */
>  };
>  
> +/*
> + * Store dirtypage info for each ramblock.
> + */
> +struct RamblockDirtyInfo {
> +char idstr[RAMBLOCK_INFO_MAX_LEN]; /* idstr for each ramblock */
> +uint8_t *ramblock_addr; /* base address of ramblock we measure */
> +uint64_t ramblock_pages; /* ramblock size in 4K-page */

It's probably a stupid question, but why not store a pointer to the
RAMBlock rather than copying some of the details?

> +uint64_t *sample_page_vfn; /* relative offset address for sampled page */
> +uint64_t sample_pages_count; /* count of sampled pages */
> +uint64_t sample_dirty_count; /* cout of dirty pages we measure */

"cout" -> "count"

> +uint32_t *hash_result; /* array of hash result for sampled pages */
> +};
> +
>  void *get_dirtyrate_thread(void *arg);
>  #endif
>  
> -- 
> 1.8.3.1

dme.
-- 
Please forgive me if I act a little strange, for I know not what I do.

Re: [PATCH] hw/timer/armv7m_systick: assert that board code set system_clock_scale

[Philippe Mathieu-Daudé]

Le mar. 25 août 2020 18:09, Peter Maydell  a
écrit :

> It is the responsibility of board code for an armv7m system to set
> system_clock_scale appropriately for the CPU speed of the core.
> If it forgets to do this, then QEMU will hang if the guest tries
> to use the systick timer in the "tick at the CPU clock frequency" mode.
>
> We forgot that in a couple of our boards (see commits ce4f70e81ed23c93f,
> e7e5a9595ab1136). Add an assertion in the systick reset method so
> we don't let any new boards in with the same bug.
>
> Signed-off-by: Peter Maydell 
>


Reviewed-by: Philippe Mathieu-Daudé 

---
> In the longer term we should make this a clocktree input and
> plumb it through the armv7m container and so on, but for the
> moment this assert() improves the current situation.
> ---
>  hw/timer/armv7m_systick.c | 8 
>  1 file changed, 8 insertions(+)
>
> diff --git a/hw/timer/armv7m_systick.c b/hw/timer/armv7m_systick.c
> index 74c58bcf245..a8cec7eb56b 100644
> --- a/hw/timer/armv7m_systick.c
> +++ b/hw/timer/armv7m_systick.c
> @@ -202,6 +202,14 @@ static void systick_reset(DeviceState *dev)
>  {
>  SysTickState *s = SYSTICK(dev);
>
> +/*
> + * Forgetting to set system_clock_scale is always a board code
> + * bug. We can't check this earlier because for some boards
> + * (like stellaris) it is not yet configured at the point where
> + * the systick device is realized.
> + */
> +assert(system_clock_scale != 0);
> +
>  s->control = 0;
>  s->reload = 0;
>  s->tick = 0;
> --
> 2.20.1
>
>
>

Re: [PATCH v5 03/12] migration/dirtyrate: Add RamlockDirtyInfo to store sampled page info

[Zheng Chuan]

On 2020/8/26 17:29, David Edmondson wrote:
> On Monday, 2020-08-24 at 17:14:31 +08, Chuan Zheng wrote:
> 
>> Add RamlockDirtyInfo to store sampled page info of each ramblock.
>>
>> Signed-off-by: Chuan Zheng 
>> ---
>>  migration/dirtyrate.h | 18 ++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
>> index 33669b7..7da 100644
>> --- a/migration/dirtyrate.h
>> +++ b/migration/dirtyrate.h
>> @@ -19,6 +19,11 @@
>>   */
>>  #define DIRTYRATE_DEFAULT_SAMPLE_PAGES512
>>  
>> +/*
>> + * Record ramblock idstr
>> + */
>> +#define RAMBLOCK_INFO_MAX_LEN 256
>> +
>>  /* Take 1s as default for calculation duration */
>>  #define DEFAULT_FETCH_DIRTYRATE_TIME_SEC  1
>>  
>> @@ -27,6 +32,19 @@ struct DirtyRateConfig {
>>  int64_t sample_period_seconds; /* time duration between two sampling */
>>  };
>>  
>> +/*
>> + * Store dirtypage info for each ramblock.
>> + */
>> +struct RamblockDirtyInfo {
>> +char idstr[RAMBLOCK_INFO_MAX_LEN]; /* idstr for each ramblock */
>> +uint8_t *ramblock_addr; /* base address of ramblock we measure */
>> +uint64_t ramblock_pages; /* ramblock size in 4K-page */
> 
> It's probably a stupid question, but why not store a pointer to the
> RAMBlock rather than copying some of the details?
> 
>> +uint64_t *sample_page_vfn; /* relative offset address for sampled page 
>> */
>> +uint64_t sample_pages_count; /* count of sampled pages */
>> +uint64_t sample_dirty_count; /* cout of dirty pages we measure */
> 
> "cout" -> "count"
> 
Hi, David.
Thank you for review.
Actually I have resent [PATCH V5], but it's my fault to forget to add resent 
tag which may lead to confusing.
In new patch series, this spell mistake is fixed.
Please refer to the newer patch series:)

>> +uint32_t *hash_result; /* array of hash result for sampled pages */
>> +};
>> +
>>  void *get_dirtyrate_thread(void *arg);
>>  #endif
>>  
>> -- 
>> 1.8.3.1
> 
> dme.
> 

Re: [PATCH v2 1/1] core/register: Specify instance_size in the TypeInfo

[Philippe Mathieu-Daudé]

Le mar. 25 août 2020 19:42, Alistair Francis  a
écrit :

> Reported-by: Eduardo Habkost 
> Signed-off-by: Alistair Francis 
> ---
>  hw/core/register.c | 31 +--
>  1 file changed, 13 insertions(+), 18 deletions(-)
>
> diff --git a/hw/core/register.c b/hw/core/register.c
> index ddf91eb445..31038bd7cc 100644
> --- a/hw/core/register.c
> +++ b/hw/core/register.c
> @@ -176,17 +176,6 @@ void register_reset(RegisterInfo *reg)
>  }
>  }
>
> -void register_init(RegisterInfo *reg)
> -{
> -assert(reg);
> -
> -if (!reg->data || !reg->access) {
> -return;
> -}
> -
> -object_initialize((void *)reg, sizeof(*reg), TYPE_REGISTER);
> -}
> -
>  void register_write_memory(void *opaque, hwaddr addr,
> uint64_t value, unsigned size)
>  {
> @@ -269,13 +258,18 @@ static RegisterInfoArray
> *register_init_block(DeviceState *owner,
>  int index = rae[i].addr / data_size;
>  RegisterInfo *r = &ri[index];
>
> -*r = (RegisterInfo) {
> -.data = data + data_size * index,
> -.data_size = data_size,
> -.access = &rae[i],
> -.opaque = owner,
> -};
> -register_init(r);
> +if (data + data_size * index == 0 || !&rae[i]) {
> +continue;
> +}
> +
> +/* Init the register, this will zero it. */
> +object_initialize((void *)r, sizeof(*r), TYPE_REGISTER);
>

Easier to review &ri[index] than that void* cast IMO.
Otherwise:
Reviewed-by: Philippe Mathieu-Daudé 

+
> +/* Set the properties of the register */
> +r->data = data + data_size * index;
> +r->data_size = data_size;
> +r->access = &rae[i];
> +r->opaque = owner;
>
>  r_array->r[i] = r;
>  }
> @@ -329,6 +323,7 @@ static const TypeInfo register_info = {
>  .name  = TYPE_REGISTER,
>  .parent = TYPE_DEVICE,
>  .class_init = register_class_init,
> +.instance_size = sizeof(RegisterInfo),
>  };
>
>  static void register_register_types(void)
> --
> 2.28.0
>
>
>

Re: [PATCH v2 2/5] meson: use meson datadir instead of qemu_datadir

[Paolo Bonzini]

You'd need to check first if it works correctly if confsuffix "looks like"
an absolute path ('/qemu'). So + looks correct to me.

(Sorry for top posting).

Paolo

Il mer 26 ago 2020, 10:53 Marc-André Lureau  ha
scritto:

> Hi
>
> On Wed, Aug 26, 2020 at 12:30 PM Daniel P. Berrangé 
> wrote:
>
>> On Tue, Aug 25, 2020 at 09:58:24PM +0400, marcandre.lur...@redhat.com
>> wrote:
>> > From: Marc-André Lureau 
>> >
>> > When cross-compiling, by default qemu_datadir is 'c:\Program
>> > Files\QEMU', which is not recognized as being an absolute path, and
>> > meson will end up adding the prefix again.
>> >
>> > Signed-off-by: Marc-André Lureau 
>> > ---
>> >  contrib/vhost-user-gpu/meson.build | 2 +-
>> >  meson.build| 3 ++-
>> >  pc-bios/descriptors/meson.build| 2 +-
>> >  pc-bios/keymaps/meson.build| 6 +++---
>> >  pc-bios/meson.build| 2 +-
>> >  tools/virtiofsd/meson.build| 2 +-
>> >  trace/meson.build  | 2 +-
>> >  7 files changed, 10 insertions(+), 9 deletions(-)
>> >
>>
>> > diff --git a/meson.build b/meson.build
>> > index f0fe5f8799..20f20a7bfc 100644
>> > --- a/meson.build
>> > +++ b/meson.build
>> > @@ -17,6 +17,7 @@ config_all_disas =
>> keyval.load(meson.current_build_dir() / 'config-all-disas.mak
>> >  enable_modules = 'CONFIG_MODULES' in config_host
>> >  enable_static = 'CONFIG_STATIC' in config_host
>> >  build_docs = 'BUILD_DOCS' in config_host
>> > +qemu_datadir = get_option('datadir') + get_option('confsuffix')
>>
>> This needs to be
>>
>>get_option('datadir') / get_option('confsuffix')
>>
>> to add the dir separator if we're using the default meson value
>> for "confsuffix" which lacks a leading "/".
>>
>
> right, fixed
>
> --
> Marc-André Lureau
>

Re: [PATCH-for-5.2 0/4] hw/char/serial: Use the Clock API to feed the UART reference clock

[Philippe Mathieu-Daudé]

Hi Peter,

Le lun. 24 août 2020 17:20, Peter Maydell  a
écrit :

> On Sat, 22 Aug 2020 at 21:00, Philippe Mathieu-Daudé 
> wrote:
> >
> > On 8/6/20 3:03 PM, Philippe Mathieu-Daudé wrote:
> > > This series improve tracing of multiple UART device in the same
> > > chipset, and allow to use the Clock API to feed each device with
> > > an (updatable) input clock.
> > >
> > > Based-on: <20200730165900.7030-1-phi...@redhat.com>
> > > "hw/char: Remove TYPE_SERIAL_IO"
> > >
> > > Philippe Mathieu-Daudé (4):
> > >   hw/char/serial: Replace commented DPRINTF() by trace event
> > >   hw/char/serial: Remove old DEBUG_SERIAL commented code
> > >   hw/char/serial: Let SerialState have an 'id' field
> > >   hw/char/serial: Use the Clock API to feed the UART reference clock
> > >
> > >  include/hw/char/serial.h |  4 +++
> > >  hw/char/serial.c | 55 +++-
> > >  hw/char/trace-events |  5 ++--
> > >  3 files changed, 39 insertions(+), 25 deletions(-)
> > >
> >
> > ping? Should I resend with the typo from patch 4 fixed?
>
> Which tree are you expecting the patches to go in via ?
>

I cc'ed you because having reviewed the Clock API series you are familiar
with it.
However I expect this series to be merged by the chardev maintainers.
In particular to verify the default values (when no input clock provided).


> thanks
> -- PMM
>
>

Re: [PATCH v2 0/3] hw/misc/unimp: Improve how offset/value are displayed

[Philippe Mathieu-Daudé]

Hi Peter,

Le lun. 24 août 2020 17:01, Peter Maydell  a
écrit :

> On Sat, 22 Aug 2020 at 21:02, Philippe Mathieu-Daudé 
> wrote:
> >
> > On 8/12/20 9:02 PM, Philippe Mathieu-Daudé wrote:
> > > This series aims to ease looking at the '-d unimp' output reported
> > > by the UnimplementedDevice.
> > >
> > > - read/write accesses are now aligned
> > > - the value format width uses the access size
> > > - the offset (address) uses the size of the memory region it belongs
> > >
> > > Series fully reviewed.
> >
> > ping?
>
> What tree were you expecting these to go in via? Probably worth
> being specific about who you're pinging in this kind of case ;-)
>

Ok understood for the next time. In this case you are listed as maintainer
:) Maybe you can Ack the series and then I'll ask to merge it via the
trivial or misc trees.


> -- PMM
>

Re: [PATCH v2 0/3] hw/misc/unimp: Improve how offset/value are displayed

[Philippe Mathieu-Daudé]

Le mer. 26 août 2020 11:54, Philippe Mathieu-Daudé  a
écrit :

> Hi Peter,
>
> Le lun. 24 août 2020 17:01, Peter Maydell  a
> écrit :
>
>> On Sat, 22 Aug 2020 at 21:02, Philippe Mathieu-Daudé 
>> wrote:
>> >
>> > On 8/12/20 9:02 PM, Philippe Mathieu-Daudé wrote:
>> > > This series aims to ease looking at the '-d unimp' output reported
>> > > by the UnimplementedDevice.
>> > >
>> > > - read/write accesses are now aligned
>> > > - the value format width uses the access size
>> > > - the offset (address) uses the size of the memory region it belongs
>> > >
>> > > Series fully reviewed.
>> >
>> > ping?
>>
>> What tree were you expecting these to go in via? Probably worth
>> being specific about who you're pinging in this kind of case ;-)
>>
>
> Ok understood for the next time. In this case you are listed as maintainer
> :) Maybe you can Ack the series and then I'll ask to merge it via the
> trivial or misc trees.
>

I just realized qemu-trivial was correctly cc'ed.


>
>> -- PMM
>>
>

Re: [PATCH v5 06/12] migration/dirtyrate: Record hash results for each sampled page

[David Edmondson]

On Monday, 2020-08-24 at 17:14:34 +08, Chuan Zheng wrote:

> Record hash results for each sampled page, crc32 is taken to calculate
> hash results for each sampled 4K-page.
>
> Signed-off-by: Chuan Zheng 
> Signed-off-by: YanYing Zhuang 
> ---
>  migration/dirtyrate.c | 136 
> ++
>  migration/dirtyrate.h |  15 ++
>  2 files changed, 151 insertions(+)
>
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index f6a94d8..66de426 100644
> --- a/migration/dirtyrate.c
> +++ b/migration/dirtyrate.c
> @@ -10,6 +10,7 @@
>   * See the COPYING file in the top-level directory.
>   */
>  
> +#include 
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
>  #include "crypto/hash.h"
> @@ -66,6 +67,141 @@ static void update_dirtyrate(uint64_t msec)
>  DirtyStat.dirty_rate = dirtyrate;
>  }
>  
> +/*
> + * get hash result for the sampled memory with length of 4K byte in ramblock,
> + * which starts from ramblock base address.
> + */
> +static uint32_t get_ramblock_vfn_hash(struct RamblockDirtyInfo *info,
> +  uint64_t vfn)
> +{
> +struct iovec iov_array;

There's no need for an iovec now that crc32() is being used.

> +uint32_t crc;
> +
> +iov_array.iov_base = info->ramblock_addr +
> + vfn * DIRTYRATE_SAMPLE_PAGE_SIZE;
> +iov_array.iov_len = DIRTYRATE_SAMPLE_PAGE_SIZE;
> +
> +crc = crc32(0, iov_array.iov_base, iov_array.iov_len);
> +
> +return crc;
> +}
> +
> +static int save_ramblock_hash(struct RamblockDirtyInfo *info)
> +{
> +unsigned int sample_pages_count;
> +int i;
> +int ret = -1;

There's no need to initialise "ret".

> +GRand *rand = g_rand_new();

Calling g_rand_new() when the result may not be used (because of the
various conditions immediately below) seems as though it might waste
entropy. Could this be pushed down just above the loop? It would even
get rid of the gotos ;-)

> +sample_pages_count = info->sample_pages_count;
> +
> +/* ramblock size less than one page, return success to skip this 
> ramblock */
> +if (unlikely(info->ramblock_pages == 0 || sample_pages_count == 0)) {
> +ret = 0;
> +goto out;
> +}
> +
> +info->hash_result = g_try_malloc0_n(sample_pages_count,
> +sizeof(uint32_t));
> +if (!info->hash_result) {
> +ret = -1;
> +goto out;
> +}
> +
> +info->sample_page_vfn = g_try_malloc0_n(sample_pages_count,
> +sizeof(uint64_t));
> +if (!info->sample_page_vfn) {
> +g_free(info->hash_result);
> +ret = -1;
> +goto out;
> +}
> +
> +for (i = 0; i < sample_pages_count; i++) {
> +info->sample_page_vfn[i] = g_rand_int_range(rand, 0,
> +info->ramblock_pages - 
> 1);
> +info->hash_result[i] = get_ramblock_vfn_hash(info,
> + 
> info->sample_page_vfn[i]);
> +}
> +ret = 0;
> +
> +out:
> +g_rand_free(rand);
> +return ret;
> +}
> +
> +static void get_ramblock_dirty_info(RAMBlock *block,
> +struct RamblockDirtyInfo *info,
> +struct DirtyRateConfig *config)
> +{
> +uint64_t sample_pages_per_gigabytes = config->sample_pages_per_gigabytes;
> +
> +/* Right shift 30 bits to calc block size in GB */
> +info->sample_pages_count = (qemu_ram_get_used_length(block) *
> +sample_pages_per_gigabytes) >>
> +DIRTYRATE_PAGE_SHIFT_GB;

Doing the calculation this way around seems odd. Why was it not:

info->sample_pages_count = (qemu_ram_get_used_length(block) >>
DIRTYRATE_PAGE_SHIFT_GB) *
sample_pages_per_gigabytes;

?

> +
> +/* Right shift 12 bits to calc page count in 4KB */
> +info->ramblock_pages = qemu_ram_get_used_length(block) >>
> +   DIRTYRATE_PAGE_SHIFT_KB;
> +info->ramblock_addr = qemu_ram_get_host_addr(block);
> +strcpy(info->idstr, qemu_ram_get_idstr(block));
> +}
> +
> +static struct RamblockDirtyInfo *
> +alloc_ramblock_dirty_info(int *block_index,
> +  struct RamblockDirtyInfo *block_dinfo)
> +{
> +struct RamblockDirtyInfo *info = NULL;
> +int index = *block_index;
> +
> +if (!block_dinfo) {
> +index = 0;
> +block_dinfo = g_try_new(struct RamblockDirtyInfo, 1);
> +} else {
> +index++;
> +block_dinfo = g_try_realloc(block_dinfo, (index + 1) *
> +sizeof(struct RamblockDirtyInfo));

g_try_renew() instead of g_try_realloc()?

> +}
> +if (!block_dinfo) {
> +return NULL;
> +}
> +
> +info = &block_dinfo[index];
> +*block_index = index;
> +memset(info, 0, s

Re: [PATCH v5 1/8] hw/i386: Remove node_id, nr_nodes and nodes_per_pkg from topology

[Igor Mammedov]

On Fri, 21 Aug 2020 17:12:25 -0500
Babu Moger  wrote:

> Remove node_id, nr_nodes and nodes_per_pkg from topology. Use
> die_id, nr_dies and dies_per_pkg which is already available.
> Removes the confusion over two variables.
> 
> With node_id removed in topology the uninitialized memory issue
> with -device and CPU hotplug will be fixed.
> 

it would be easier to review if you first put all reverts,
and then this patch on top.

well, to actually avoid that request from others,
I'd suggest just to do that to avoid issue with merging.

other than that patches 2-7/8 look good to me.

> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1828750
> Signed-off-by: Babu Moger 
> ---
>  hw/i386/pc.c   |1 -
>  hw/i386/x86.c  |1 -
>  include/hw/i386/topology.h |   40 +---
>  target/i386/cpu.c  |   24 ++--
>  target/i386/cpu.h  |1 -
>  tests/test-x86-cpuid.c |   40 
>  6 files changed, 39 insertions(+), 68 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 47c5ca3e34..0ae5cb3af4 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1498,7 +1498,6 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
>  init_topo_info(&topo_info, x86ms);
>  
>  env->nr_dies = x86ms->smp_dies;
> -env->nr_nodes = topo_info.nodes_per_pkg;
>  env->pkg_offset = x86ms->apicid_pkg_offset(&topo_info);
>  
>  /*
> diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> index 67bee1bcb8..f6eab947df 100644
> --- a/hw/i386/x86.c
> +++ b/hw/i386/x86.c
> @@ -62,7 +62,6 @@ inline void init_topo_info(X86CPUTopoInfo *topo_info,
>  {
>  MachineState *ms = MACHINE(x86ms);
>  
> -topo_info->nodes_per_pkg = ms->numa_state->num_nodes / ms->smp.sockets;
>  topo_info->dies_per_pkg = x86ms->smp_dies;
>  topo_info->cores_per_die = ms->smp.cores;
>  topo_info->threads_per_core = ms->smp.threads;
> diff --git a/include/hw/i386/topology.h b/include/hw/i386/topology.h
> index 07239f95f4..05ddde7ba0 100644
> --- a/include/hw/i386/topology.h
> +++ b/include/hw/i386/topology.h
> @@ -47,14 +47,12 @@ typedef uint32_t apic_id_t;
>  
>  typedef struct X86CPUTopoIDs {
>  unsigned pkg_id;
> -unsigned node_id;
>  unsigned die_id;
>  unsigned core_id;
>  unsigned smt_id;
>  } X86CPUTopoIDs;
>  
>  typedef struct X86CPUTopoInfo {
> -unsigned nodes_per_pkg;
>  unsigned dies_per_pkg;
>  unsigned cores_per_die;
>  unsigned threads_per_core;
> @@ -89,11 +87,6 @@ static inline unsigned apicid_die_width(X86CPUTopoInfo 
> *topo_info)
>  return apicid_bitwidth_for_count(topo_info->dies_per_pkg);
>  }
>  
> -/* Bit width of the node_id field per socket */
> -static inline unsigned apicid_node_width_epyc(X86CPUTopoInfo *topo_info)
> -{
> -return apicid_bitwidth_for_count(MAX(topo_info->nodes_per_pkg, 1));
> -}
>  /* Bit offset of the Core_ID field
>   */
>  static inline unsigned apicid_core_offset(X86CPUTopoInfo *topo_info)
> @@ -114,30 +107,23 @@ static inline unsigned apicid_pkg_offset(X86CPUTopoInfo 
> *topo_info)
>  return apicid_die_offset(topo_info) + apicid_die_width(topo_info);
>  }
>  
> -#define NODE_ID_OFFSET 3 /* Minimum node_id offset if numa configured */
> +#define EPYC_DIE_OFFSET 3 /* Minimum die_id offset if numa configured */
>  
>  /*
> - * Bit offset of the node_id field
> - *
> - * Make sure nodes_per_pkg >  0 if numa configured else zero.
> + * Bit offset of the die_id field
>   */
> -static inline unsigned apicid_node_offset_epyc(X86CPUTopoInfo *topo_info)
> +static inline unsigned apicid_die_offset_epyc(X86CPUTopoInfo *topo_info)
>  {
> -unsigned offset = apicid_die_offset(topo_info) +
> -  apicid_die_width(topo_info);
> +unsigned offset = apicid_core_offset(topo_info) +
> +  apicid_core_width(topo_info);
>  
> -if (topo_info->nodes_per_pkg) {
> -return MAX(NODE_ID_OFFSET, offset);
> -} else {
> -return offset;
> -}
> +return MAX(EPYC_DIE_OFFSET, offset);
>  }
>  
>  /* Bit offset of the Pkg_ID (socket ID) field */
>  static inline unsigned apicid_pkg_offset_epyc(X86CPUTopoInfo *topo_info)
>  {
> -return apicid_node_offset_epyc(topo_info) +
> -   apicid_node_width_epyc(topo_info);
> +return apicid_die_offset_epyc(topo_info) + apicid_die_width(topo_info);
>  }
>  
>  /*
> @@ -150,8 +136,7 @@ x86_apicid_from_topo_ids_epyc(X86CPUTopoInfo *topo_info,
>const X86CPUTopoIDs *topo_ids)
>  {
>  return (topo_ids->pkg_id  << apicid_pkg_offset_epyc(topo_info)) |
> -   (topo_ids->node_id << apicid_node_offset_epyc(topo_info)) |
> -   (topo_ids->die_id  << apicid_die_offset(topo_info)) |
> +   (topo_ids->die_id  << apicid_die_offset_epyc(topo_info)) |
> (topo_ids->core_id << apicid_core_offset(topo_info)) |
> topo_ids->smt_id;
>  }
> @@ -160,15 +145,11

Re: [PATCH v5 07/10] block: introduce preallocate filter

[Max Reitz]

On 26.08.20 11:15, Vladimir Sementsov-Ogievskiy wrote:
> 26.08.2020 11:52, Max Reitz wrote:
>> On 26.08.20 08:49, Vladimir Sementsov-Ogievskiy wrote:
>>> 25.08.2020 18:11, Max Reitz wrote:
 On 21.08.20 16:11, Vladimir Sementsov-Ogievskiy wrote:
> It's intended to be inserted between format and protocol nodes to
> preallocate additional space (expanding protocol file) on writes
> crossing EOF. It improves performance for file-systems with slow
> allocation.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>    docs/system/qemu-block-drivers.rst.inc |  26 +++
>    qapi/block-core.json   |  20 +-
>    block/preallocate.c    | 291
> +
>    block/Makefile.objs    |   1 +
>    4 files changed, 337 insertions(+), 1 deletion(-)
>    create mode 100644 block/preallocate.c
>>
>> [...]
>>
> diff --git a/block/preallocate.c b/block/preallocate.c
> new file mode 100644
> index 00..bdf54dbd2f
> --- /dev/null
> +++ b/block/preallocate.c
> @@ -0,0 +1,291 @@
> +/*
> + * preallocate filter driver
> + *
> + * The driver performs preallocate operation: it is injected above
> + * some node, and before each write over EOF it does additional
> preallocating
> + * write-zeroes request.
> + *
> + * Copyright (c) 2020 Virtuozzo International GmbH.
> + *
> + * 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
> .
> + */
> +
> +#include "qemu/osdep.h"
> +
> +#include "qapi/error.h"
> +#include "qemu/module.h"
> +#include "qemu/option.h"
> +#include "qemu/units.h"
> +#include "block/block_int.h"
> +
> +
> +typedef struct BDRVPreallocateState {
> +    int64_t prealloc_size;
> +    int64_t prealloc_align;
> +
> +    /*
> + * Filter is started as not-active, so it doesn't do any
> preallocations nor
> + * requires BLK_PERM_RESIZE on its child. This is needed to
> create filter
> + * above another node-child and then do bdrv_replace_node to
> insert the
> + * filter.

 A bit weird, but seems fair.  It’s basically just a cache for whether
 this node has a writer on it or not.

 Apart from the weirdness, I don’t understand the “another node-child”.
 Say you have “format” -> “proto”, and then you want to insert
 “prealloc”.  Wouldn’t you blockdev-add prealloc,file=proto and then
 blockdev-replace format.file=prealloc?
>>>
>>> Yes something like this. Actually, I'm about inserting the filter
>>> automatically from block layer code, but your reasoning is about same
>>> thing and is better.
>>>
 So what is that “other node-child”?
>>>
>>> Hmm, just my misleading wording. At least '-' in wrong place. Just
>>> "other node child", or "child of another node"..
>>
>> OK.
>>
> + *
> + * Filter becomes active the first time it detects that its
> parents have
> + * BLK_PERM_RESIZE on it.
> + * Filter becomes active forever: it doesn't lose active status
> if parents
> + * lose BLK_PERM_RESIZE, otherwise we'll not be able to shrink
> the file on
> + * filter close.

 Oh, the file is shrunk?  That wasn’t clear from the documentation.

 Hm.  Seems like useful behavior.  I just wonder if keeping the
 permission around indefinitely makes sense.  Why not just truncate it
 when the permission is revoked?
>>>
>>> How? Parent is closed earlier, so on close we will not have the
>>> permission. So, we force-keep the permission up to our close().
>>
>> I thought that preallocate_child_perm() would be invoked when the parent
>> is detached, so we could do the truncate there, before relinquishing
>> preallocate’s RESIZE permission.
>>
> 
> Hmm, I can check it. I just a bit afraid of doing something serious like
> truncation in .bdrv_child_perm() handler, which doesn't seem to imply
> such usage.

I’m a bit conflicted.  On one hand, I share your concern.  On the other,
I find it completely reasonable that drivers might want to do I/O when
permissions change.

Usually, this is done as part of reop

Re: [PATCH v5 03/12] migration/dirtyrate: Add RamlockDirtyInfo to store sampled page info

[Zheng Chuan]

On 2020/8/26 17:40, Zheng Chuan wrote:
> 
> 
> On 2020/8/26 17:29, David Edmondson wrote:
>> On Monday, 2020-08-24 at 17:14:31 +08, Chuan Zheng wrote:
>>
>>> Add RamlockDirtyInfo to store sampled page info of each ramblock.
>>>
>>> Signed-off-by: Chuan Zheng 
>>> ---
>>>  migration/dirtyrate.h | 18 ++
>>>  1 file changed, 18 insertions(+)
>>>
>>> diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
>>> index 33669b7..7da 100644
>>> --- a/migration/dirtyrate.h
>>> +++ b/migration/dirtyrate.h
>>> @@ -19,6 +19,11 @@
>>>   */
>>>  #define DIRTYRATE_DEFAULT_SAMPLE_PAGES512
>>>  
>>> +/*
>>> + * Record ramblock idstr
>>> + */
>>> +#define RAMBLOCK_INFO_MAX_LEN 256
>>> +
>>>  /* Take 1s as default for calculation duration */
>>>  #define DEFAULT_FETCH_DIRTYRATE_TIME_SEC  1
>>>  
>>> @@ -27,6 +32,19 @@ struct DirtyRateConfig {
>>>  int64_t sample_period_seconds; /* time duration between two sampling */
>>>  };
>>>  
>>> +/*
>>> + * Store dirtypage info for each ramblock.
>>> + */
>>> +struct RamblockDirtyInfo {
>>> +char idstr[RAMBLOCK_INFO_MAX_LEN]; /* idstr for each ramblock */
>>> +uint8_t *ramblock_addr; /* base address of ramblock we measure */
>>> +uint64_t ramblock_pages; /* ramblock size in 4K-page */
>>
>> It's probably a stupid question, but why not store a pointer to the
>> RAMBlock rather than copying some of the details?
>>
Missing this question:)
Since I only hold the RCU around each part separately, the RAMBlock
could disappear between the initial hash, and the later compare;  so
using the name makes it safe.
Again, Please refer to the newer patch series:)

>>> +uint64_t *sample_page_vfn; /* relative offset address for sampled page 
>>> */
>>> +uint64_t sample_pages_count; /* count of sampled pages */
>>> +uint64_t sample_dirty_count; /* cout of dirty pages we measure */
>>
>> "cout" -> "count"
>>
> Hi, David.
> Thank you for review.
> Actually I have resent [PATCH V5], but it's my fault to forget to add resent 
> tag which may lead to confusing.
> In new patch series, this spell mistake is fixed.
> Please refer to the newer patch series:)
> 
>>> +uint32_t *hash_result; /* array of hash result for sampled pages */
>>> +};
>>> +
>>>  void *get_dirtyrate_thread(void *arg);
>>>  #endif
>>>  
>>> -- 
>>> 1.8.3.1
>>
>> dme.
>>

Re: [PULL 00/18] riscv-to-apply queue

[Bin Meng]

Hi Peter,

On Wed, Aug 26, 2020 at 5:25 PM Peter Maydell  wrote:
>
> On Wed, 26 Aug 2020 at 04:21, Bin Meng  wrote:
> > On Wed, Aug 26, 2020 at 6:41 AM Alistair Francis  
> > wrote:
> > > Richard and Philippe review patches and some of the RISC-V patches get
> > > reviewed by the RISC-V community. The main problem (which is a common
> > > problem in open source) is that large technical patch series just get
> > > ignored.
> >
> > Yep, I am only comfortable reviewing patches which I have confidence
> > in. Right now I am not working on any H- or V - extension for RISC-V
> > so I cannot contribute to any review of these large numbers of H- or
> > V- extension related patches. Sorry!
>
> So, everybody has a ton of work they need to do and only a limited
> amount of time they might have for code review, so it's important to
> prioritise. But I would encourage you, and other people contributing
> to RISC-V parts of QEMU, to at least sometimes review changes that are
> a little bit out of your "comfort zone" if nobody else seems to be
> doing so. Review can find bugs, areas that are confusing or need
> comments, etc, even without a thorough knowledge of the relevant spec.
> (In fact, not knowing the spec can help in identifying where
> explanatory comments can help the reader!) And for the project it means
> we have more people who at least have some idea of what that bit of code
> is doing. Review that is limited to "this code seems to make sense but
> I haven't checked it against the spec" is better than patches getting
> no review at all, I think. And it's a good way to build your knowledge
> of the codebase and the architecture over time.

Agree. I really wanted to spend more time on this project but like you
said it's priorities.

One thing I do not understand is that according to MAINTAINTERS there
are 4 custodians for the RISC-V maintenance work but it looks to me so
far only Alistair is actively reviewing patches. I know Palmer used to
review patches but if it's only one person that might be some issues.
At least MAINTAINTERS can cross-review, and we have 4 there.

Regards,
Bin

Re: [PATCH v5 07/12] migration/dirtyrate: Compare page hash results for recorded sampled page

[David Edmondson]

On Monday, 2020-08-24 at 17:14:35 +08, Chuan Zheng wrote:

> Compare page hash results for recorded sampled page.
>
> Signed-off-by: Chuan Zheng 
> Signed-off-by: YanYing Zhuang 
> ---
>  migration/dirtyrate.c | 64 
> +++
>  1 file changed, 64 insertions(+)
>
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index 66de426..050270d 100644
> --- a/migration/dirtyrate.c
> +++ b/migration/dirtyrate.c
> @@ -202,6 +202,70 @@ static int record_ramblock_hash_info(struct 
> RamblockDirtyInfo **block_dinfo,
>  return 0;
>  }
>  
> +static int calc_page_dirty_rate(struct RamblockDirtyInfo *info)

This never fails - could be void?

> +{
> +uint32_t crc;
> +int i;
> +
> +for (i = 0; i < info->sample_pages_count; i++) {
> +crc = get_ramblock_vfn_hash(info, info->sample_page_vfn[i]);
> +if (crc != info->hash_result[i]) {
> +info->sample_dirty_count++;
> +}
> +}
> +
> +return 0;
> +}
> +
> +static bool find_page_matched(RAMBlock *block, struct RamblockDirtyInfo 
> *infos,
> +  int count, struct RamblockDirtyInfo **matched)

This might as well just return a struct RamblockDirtyInfo pointer (or NULL).

> +{
> +int i;
> +
> +for (i = 0; i < count; i++) {
> +if (!strcmp(infos[i].idstr, qemu_ram_get_idstr(block))) {
> +break;
> +}
> +}
> +
> +if (i == count) {
> +return false;
> +}
> +
> +if (infos[i].ramblock_addr != qemu_ram_get_host_addr(block) ||
> +infos[i].ramblock_pages !=
> +(qemu_ram_get_used_length(block) >> DIRTYRATE_PAGE_SHIFT_KB)) {
> +return false;
> +}
> +
> +*matched = &infos[i];
> +return true;
> +}
> +
> +static int compare_page_hash_info(struct RamblockDirtyInfo *info,
> +  int block_index)
> +{
> +struct RamblockDirtyInfo *block_dinfo = NULL;
> +RAMBlock *block = NULL;
> +
> +RAMBLOCK_FOREACH_MIGRATABLE(block) {
> +block_dinfo = NULL;
> +if (!find_page_matched(block, info, block_index + 1, &block_dinfo)) {
> +continue;
> +}
> +if (calc_page_dirty_rate(block_dinfo) < 0) {
> +return -1;
> +}
> +update_dirtyrate_stat(block_dinfo);
> +}
> +
> +if (!DirtyStat.total_sample_count) {
> +return -1;
> +}
> +
> +return 0;
> +}
> +
>  static void calculate_dirtyrate(struct DirtyRateConfig config)
>  {
>  /* todo */
> -- 
> 1.8.3.1

dme.
-- 
Sometimes these eyes, forget the face they're peering from.

Re: [PATCH v5 08/12] migration/dirtyrate: skip sampling ramblock with size below MIN_RAMBLOCK_SIZE

[David Edmondson]

On Monday, 2020-08-24 at 17:14:36 +08, Chuan Zheng wrote:

> In order to sample real RAM, skip ramblock with size below MIN_RAMBLOCK_SIZE
> which is set as 128M.
>
> Signed-off-by: Chuan Zheng 
> ---
>  migration/dirtyrate.c | 24 
>  migration/dirtyrate.h | 10 ++
>  2 files changed, 34 insertions(+)
>
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index 050270d..bd398b7 100644
> --- a/migration/dirtyrate.c
> +++ b/migration/dirtyrate.c
> @@ -173,6 +173,24 @@ alloc_ramblock_dirty_info(int *block_index,
>  return block_dinfo;
>  }
>  
> +static int skip_sample_ramblock(RAMBlock *block)
> +{
> +int64_t ramblock_size;
> +
> +/* ramblock size in MB */
> +ramblock_size = qemu_ram_get_used_length(block) >> 
> DIRTYRATE_PAGE_SHIFT_MB;
> +
> +/*
> + * Consider ramblock with size larger than 128M is what we
> + * want to sample.
> + */
> +if (ramblock_size < MIN_RAMBLOCK_SIZE) {
> +return -1;
> +}
> +
> +return 0;
> +}
> +
>  static int record_ramblock_hash_info(struct RamblockDirtyInfo **block_dinfo,
>   struct DirtyRateConfig config,
>   int *block_index)
> @@ -183,6 +201,9 @@ static int record_ramblock_hash_info(struct 
> RamblockDirtyInfo **block_dinfo,
>  int index = 0;
>  
>  RAMBLOCK_FOREACH_MIGRATABLE(block) {
> +if (skip_sample_ramblock(block) < 0) {
> +continue;
> +}
>  dinfo = alloc_ramblock_dirty_info(&index, dinfo);
>  if (dinfo == NULL) {
>  return -1;
> @@ -249,6 +270,9 @@ static int compare_page_hash_info(struct 
> RamblockDirtyInfo *info,
>  RAMBlock *block = NULL;
>  
>  RAMBLOCK_FOREACH_MIGRATABLE(block) {
> +if (skip_sample_ramblock(block) < 0) {
> +continue;
> +}
>  block_dinfo = NULL;
>  if (!find_page_matched(block, info, block_index + 1, &block_dinfo)) {
>  continue;
> diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
> index 5050add..41bc264 100644
> --- a/migration/dirtyrate.h
> +++ b/migration/dirtyrate.h
> @@ -35,10 +35,20 @@
>  #define DIRTYRATE_PAGE_SHIFT_KB   12
>  
>  /*
> + * Sample page size MB shift
> + */
> +#define DIRTYRATE_PAGE_SHIFT_MB   20
> +
> +/*
>   * Sample page size 1G shift
>   */
>  #define DIRTYRATE_PAGE_SHIFT_GB   30
>  
> +/*
> + * minimum ramblock size to sampled

"Minimum RAMBlock size to sample, in megabytes."

> + */
> +#define MIN_RAMBLOCK_SIZE 128
> +
>  /* Take 1s as default for calculation duration */
>  #define DEFAULT_FETCH_DIRTYRATE_TIME_SEC  1
>  
> -- 
> 1.8.3.1

dme.
-- 
I went starin' out of my window, been caught doin' it once or twice.

oslib-posix：Fix handle fd leak in qemu_write_pidfile()

[AlexChen]

From: alexchen 

The fd will leak when (a.st_ino == b.st_ino) is true, fix it.

Signed-off-by: AlexChen 
---
 util/oslib-posix.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index ad8001a4ad..74cf5e9c73 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -176,6 +176,7 @@ bool qemu_write_pidfile(const char *path, Error **errp)
 goto fail_unlink;
 }

+close(fd);
 return true;

 fail_unlink:
-- 
2.19.1

Re: [PATCH v2 07/10] vfio/platform: Remove dead assignment in vfio_intp_interrupt()

[Li Qiang]

Chenqun (kuhn)  于2020年8月26日周三 上午9:47写道：
>
>
> > > Clang static code analyzer show warning:
> > > hw/vfio/platform.c:239:9: warning: Value stored to 'ret' is never read
> > > ret = event_notifier_test_and_clear(intp->interrupt);
> > > ^ ~~
> > >
> > > Reported-by: Euler Robot 
> > > Signed-off-by: Chen Qun 
> > > Reviewed-by: Eric Auger 
> > > ---
> > > Cc: Alex Williamson 
> > > Cc: Eric Auger 
> > > Cc: Stefan Hajnoczi 
> > > ---
> > >  hw/vfio/platform.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c index
> > > ac2cefc9b1..869ed2c39d 100644
> > > --- a/hw/vfio/platform.c
> > > +++ b/hw/vfio/platform.c
> > > @@ -236,7 +236,7 @@ static void vfio_intp_interrupt(VFIOINTp *intp)
> > >  trace_vfio_intp_interrupt_set_pending(intp->pin);
> > >  QSIMPLEQ_INSERT_TAIL(&vdev->pending_intp_queue,
> > >   intp, pqnext);
> > > -ret = event_notifier_test_and_clear(intp->interrupt);
> >
> > Shouldn't we check the 'ret' like the other place in this function?
>
> Hi, Li Qiang,
>
> Eric、Alex、Stefan has already discussed this point in the V1 version.
> https://patchwork.kernel.org/patch/11711897/

Ok I see, then

Reviewed-by: Li Qiang 

Thanks,
Li Qiang

>
> Thanks.

elf2dmp: Fix memory leak on main() error paths

[AlexChen]

From: AlexChen 

The 'kdgb' is allocating memory in get_kdbg(), but it is not freed
in both fill_header() and fill_context() failed branches, fix it.

Signed-off-by: AlexChen 
---
 contrib/elf2dmp/main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/elf2dmp/main.c b/contrib/elf2dmp/main.c
index 9a2dbc2902..ac746e49e0 100644
--- a/contrib/elf2dmp/main.c
+++ b/contrib/elf2dmp/main.c
@@ -568,12 +568,12 @@ int main(int argc, char *argv[])
 if (fill_header(&header, &ps, &vs, KdDebuggerDataBlock, kdbg,
 KdVersionBlock, qemu_elf.state_nr)) {
 err = 1;
-goto out_pdb;
+goto out_kdbg;
 }

 if (fill_context(kdbg, &vs, &qemu_elf)) {
 err = 1;
-goto out_pdb;
+goto out_kdbg;
 }

 if (write_dump(&ps, &header, argv[2])) {
-- 
2.19.1

Re: [PATCH v5 09/12] migration/dirtyrate: Implement get_sample_page_period() and block_sample_page_period()

[David Edmondson]

On Monday, 2020-08-24 at 17:14:37 +08, Chuan Zheng wrote:

> Implement get_sample_page_period() and set_sample_page_period() to
> sleep specific time between sample actions.
>
> Signed-off-by: Chuan Zheng 
> ---
>  migration/dirtyrate.c | 24 
>  migration/dirtyrate.h |  2 ++
>  2 files changed, 26 insertions(+)
>
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index bd398b7..d1c0a78 100644
> --- a/migration/dirtyrate.c
> +++ b/migration/dirtyrate.c
> @@ -28,6 +28,30 @@
>  static int CalculatingState = DIRTY_RATE_STATUS_UNSTARTED;
>  static struct DirtyRateStat DirtyStat;
>  
> +static int64_t set_sample_page_period(int64_t msec, int64_t initial_time)
> +{
> +int64_t current_time;
> +
> +current_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> +if ((current_time - initial_time) >= msec) {
> +msec = current_time - initial_time;
> +} else {
> +g_usleep((msec + initial_time - current_time) * 1000);
> +}
> +
> +return msec;
> +}
> +
> +static int64_t get_sample_page_period(int64_t sec)
> +{
> +if (sec <= MIN_FETCH_DIRTYRATE_TIME_SEC ||

Shouldn't the minimum value be allowed?

That is, this test should be "sec < MIN_FETCH_DIRTYRATE_TIME_SEC" and
MIN_FETCH_DIRTYRATE_TIME_SEC should be 1.

> +sec > MAX_FETCH_DIRTYRATE_TIME_SEC) {
> +sec = DEFAULT_FETCH_DIRTYRATE_TIME_SEC;
> +}
> +
> +return sec;
> +}
> +
>  static int dirtyrate_set_state(int *state, int old_state, int new_state)
>  {
>  assert(new_state < DIRTY_RATE_STATUS__MAX);
> diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
> index 41bc264..50a5636 100644
> --- a/migration/dirtyrate.h
> +++ b/migration/dirtyrate.h
> @@ -51,6 +51,8 @@
>  
>  /* Take 1s as default for calculation duration */
>  #define DEFAULT_FETCH_DIRTYRATE_TIME_SEC  1
> +#define MIN_FETCH_DIRTYRATE_TIME_SEC  0
> +#define MAX_FETCH_DIRTYRATE_TIME_SEC  60
>  
>  struct DirtyRateConfig {
>  uint64_t sample_pages_per_gigabytes; /* sample pages per GB */
> -- 
> 1.8.3.1

dme.
-- 
Facts don't do what I want them to.

Re: [PATCH v3 43/74] nubus: Rename class type checking macros

[Laurent Vivier]

Le 25/08/2020 à 21:20, Eduardo Habkost a écrit :
> Rename the existing class type checking macros to be consistent
> with the type name and instance type checking macro.  Use a
> NUBUS_MACFB prefix instead of MACFB_NUBUS.
> 
> This will make future conversion to OBJECT_DECLARE* easier.
> 
> Signed-off-by: Eduardo Habkost 
> ---
> Changes series v2 -> v3: new patch added in series v3
> 
> ---
> Cc: Laurent Vivier 
> Cc: qemu-devel@nongnu.org
> ---
>  include/hw/display/macfb.h | 4 ++--
>  hw/display/macfb.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/hw/display/macfb.h b/include/hw/display/macfb.h
> index 26367ae2c4..347871b623 100644
> --- a/include/hw/display/macfb.h
> +++ b/include/hw/display/macfb.h
> @@ -40,9 +40,9 @@ typedef struct {
>  MacfbState macfb;
>  } MacfbSysBusState;
>  
> -#define MACFB_NUBUS_DEVICE_CLASS(class) \
> +#define NUBUS_MACFB_CLASS(class) \
>  OBJECT_CLASS_CHECK(MacfbNubusDeviceClass, (class), TYPE_NUBUS_MACFB)
> -#define MACFB_NUBUS_GET_CLASS(obj) \
> +#define NUBUS_MACFB_GET_CLASS(obj) \
>  OBJECT_GET_CLASS(MacfbNubusDeviceClass, (obj), TYPE_NUBUS_MACFB)
>  
>  typedef struct MacfbNubusDeviceClass {
> diff --git a/hw/display/macfb.c b/hw/display/macfb.c
> index b68faff4bb..ff8bdb846b 100644
> --- a/hw/display/macfb.c
> +++ b/hw/display/macfb.c
> @@ -391,7 +391,7 @@ static void macfb_nubus_realize(DeviceState *dev, Error 
> **errp)
>  {
>  NubusDevice *nd = NUBUS_DEVICE(dev);
>  MacfbNubusState *s = NUBUS_MACFB(dev);
> -MacfbNubusDeviceClass *ndc = MACFB_NUBUS_GET_CLASS(dev);
> +MacfbNubusDeviceClass *ndc = NUBUS_MACFB_GET_CLASS(dev);
>  MacfbState *ms = &s->macfb;
>  
>  ndc->parent_realize(dev, errp);
> @@ -443,7 +443,7 @@ static void macfb_sysbus_class_init(ObjectClass *klass, 
> void *data)
>  static void macfb_nubus_class_init(ObjectClass *klass, void *data)
>  {
>  DeviceClass *dc = DEVICE_CLASS(klass);
> -MacfbNubusDeviceClass *ndc = MACFB_NUBUS_DEVICE_CLASS(klass);
> +MacfbNubusDeviceClass *ndc = NUBUS_MACFB_CLASS(klass);
>  
>  device_class_set_parent_realize(dc, macfb_nubus_realize,
>  &ndc->parent_realize);
> 

Acked-by: Laurent Vivier 

Re: [PATCH v3 49/74] swim: Rename struct SWIM to Swim

[Laurent Vivier]

Le 25/08/2020 à 21:20, Eduardo Habkost a écrit :
> Currently we have a SWIM typedef and a SWIM type checking macro,
> but OBJECT_DECLARE* would transform the SWIM macro into a
> function, and the function name would conflict with the SWIM
> typedef name.
> 
> Rename the struct and typedef to "Swim". This will make future
> conversion to OBJECT_DECLARE* easier.
> 
> Signed-off-by: Eduardo Habkost 
> ---
> Changes series v2 -> v3: new patch added to series v3
> 
> ---
> Cc: Laurent Vivier 
> Cc: Kevin Wolf 
> Cc: Max Reitz 
> Cc: qemu-bl...@nongnu.org
> Cc: qemu-devel@nongnu.org
> 
> Signed-off-by: Eduardo Habkost 
> ---
>  include/hw/block/swim.h |  6 +++---
>  hw/block/swim.c | 10 +-
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/include/hw/block/swim.h b/include/hw/block/swim.h
> index 6add3499d0..9d8b65c561 100644
> --- a/include/hw/block/swim.h
> +++ b/include/hw/block/swim.h
> @@ -67,10 +67,10 @@ struct SWIMCtrl {
>  };
>  
>  #define TYPE_SWIM "swim"
> -#define SWIM(obj) OBJECT_CHECK(SWIM, (obj), TYPE_SWIM)
> +#define SWIM(obj) OBJECT_CHECK(Swim, (obj), TYPE_SWIM)
>  
> -typedef struct SWIM {
> +typedef struct Swim {
>  SysBusDevice parent_obj;
>  SWIMCtrl ctrl;
> -} SWIM;
> +} Swim;
>  #endif
> diff --git a/hw/block/swim.c b/hw/block/swim.c
> index 74f56e8f46..20133a814c 100644
> --- a/hw/block/swim.c
> +++ b/hw/block/swim.c
> @@ -387,7 +387,7 @@ static const MemoryRegionOps swimctrl_mem_ops = {
>  
>  static void sysbus_swim_reset(DeviceState *d)
>  {
> -SWIM *sys = SWIM(d);
> +Swim *sys = SWIM(d);
>  SWIMCtrl *ctrl = &sys->ctrl;
>  int i;
>  
> @@ -408,7 +408,7 @@ static void sysbus_swim_reset(DeviceState *d)
>  static void sysbus_swim_init(Object *obj)
>  {
>  SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
> -SWIM *sbs = SWIM(obj);
> +Swim *sbs = SWIM(obj);
>  SWIMCtrl *swimctrl = &sbs->ctrl;
>  
>  memory_region_init_io(&swimctrl->iomem, obj, &swimctrl_mem_ops, swimctrl,
> @@ -418,7 +418,7 @@ static void sysbus_swim_init(Object *obj)
>  
>  static void sysbus_swim_realize(DeviceState *dev, Error **errp)
>  {
> -SWIM *sys = SWIM(dev);
> +Swim *sys = SWIM(dev);
>  SWIMCtrl *swimctrl = &sys->ctrl;
>  
>  qbus_create_inplace(&swimctrl->bus, sizeof(SWIMBus), TYPE_SWIM_BUS, dev,
> @@ -460,7 +460,7 @@ static const VMStateDescription vmstate_sysbus_swim = {
>  .name = "SWIM",
>  .version_id = 1,
>  .fields = (VMStateField[]) {
> -VMSTATE_STRUCT(ctrl, SWIM, 0, vmstate_swim, SWIMCtrl),
> +VMSTATE_STRUCT(ctrl, Swim, 0, vmstate_swim, SWIMCtrl),
>  VMSTATE_END_OF_LIST()
>  }
>  };
> @@ -477,7 +477,7 @@ static void sysbus_swim_class_init(ObjectClass *oc, void 
> *data)
>  static const TypeInfo sysbus_swim_info = {
>  .name  = TYPE_SWIM,
>  .parent= TYPE_SYS_BUS_DEVICE,
> -.instance_size = sizeof(SWIM),
> +.instance_size = sizeof(Swim),
>  .instance_init = sysbus_swim_init,
>  .class_init= sysbus_swim_class_init,
>  };
> 

Acked-by: Laurent Vivier 

Re: oslib-posix：Fix handle fd leak in qemu_write_pidfile()

[Daniel P . Berrangé]

On Wed, Aug 26, 2020 at 06:14:48PM +0800, AlexChen wrote:
> From: alexchen 
> 
> The fd will leak when (a.st_ino == b.st_ino) is true, fix it.

That is *INTENTIONAL*.  We're holding a lock on the file and the
lock exists only while the FD is open.  When QEMU exists, the FD
is closed and the lock is released. There is no leak.

> Signed-off-by: AlexChen 
> ---
>  util/oslib-posix.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index ad8001a4ad..74cf5e9c73 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -176,6 +176,7 @@ bool qemu_write_pidfile(const char *path, Error **errp)
>  goto fail_unlink;
>  }
> 
> +close(fd);
>  return true;
> 
>  fail_unlink:
> -- 
> 2.19.1
> 
> 

Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [PATCH v5 10/12] migration/dirtyrate: Implement calculate_dirtyrate() function

[David Edmondson]

On Monday, 2020-08-24 at 17:14:38 +08, Chuan Zheng wrote:

> Implement calculate_dirtyrate() function.
>
> Signed-off-by: Chuan Zheng 
> Signed-off-by: YanYing Zhuang 
> ---
>  migration/dirtyrate.c | 45 +++--
>  1 file changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index d1c0a78..9f52f5f 100644
> --- a/migration/dirtyrate.c
> +++ b/migration/dirtyrate.c
> @@ -171,6 +171,21 @@ static void get_ramblock_dirty_info(RAMBlock *block,
>  strcpy(info->idstr, qemu_ram_get_idstr(block));
>  }
>  
> +static void free_ramblock_dirty_info(struct RamblockDirtyInfo *infos, int 
> count)
> +{
> +int i;
> +
> +if (!infos) {
> +return;
> +}
> +
> +for (i = 0; i < count; i++) {
> +g_free(infos[i].sample_page_vfn);
> +g_free(infos[i].hash_result);
> +}
> +g_free(infos);
> +}
> +
>  static struct RamblockDirtyInfo *
>  alloc_ramblock_dirty_info(int *block_index,
>struct RamblockDirtyInfo *block_dinfo)
> @@ -316,8 +331,34 @@ static int compare_page_hash_info(struct 
> RamblockDirtyInfo *info,
>  
>  static void calculate_dirtyrate(struct DirtyRateConfig config)
>  {
> -/* todo */
> -return;
> +struct RamblockDirtyInfo *block_dinfo = NULL;
> +int block_index = 0;
> +int64_t msec = 0;
> +int64_t initial_time;
> +
> +rcu_register_thread();
> +reset_dirtyrate_stat();
> +initial_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
> +rcu_read_lock();

Page dirtying that happens while acquiring the lock will not be
accounted for, but is within the time window. Could we store the time
after acquiring the lock?

> +if (record_ramblock_hash_info(&block_dinfo, config, &block_index) < 0) {
> +goto out;
> +}
> +rcu_read_unlock();
> +
> +msec = config.sample_period_seconds * 1000;
> +msec = set_sample_page_period(msec, initial_time);
> +
> +rcu_read_lock();
> +if (compare_page_hash_info(block_dinfo, block_index) < 0) {
> +goto out;
> +}
> +
> +update_dirtyrate(msec);
> +
> +out:
> +rcu_read_unlock();

Is it necessary to hold the lock across update_dirtyrate()?

> +free_ramblock_dirty_info(block_dinfo, block_index + 1);
> +rcu_unregister_thread();
>  }
>  
>  void *get_dirtyrate_thread(void *arg)
> -- 
> 1.8.3.1

dme.
-- 
There's someone in my head but it's not me.

Re: [PATCH v3 00/74] qom: Automated conversion of type checking boilerplate

[Roman Bolshakov]

On Tue, Aug 25, 2020 at 03:19:56PM -0400, Eduardo Habkost wrote:
> git tree for this series:
> https://github.com/ehabkost/qemu-hacks/tree/work/qom-macros-autoconvert
> 

Hi Eduardo,

another assert fires during QEMU start:

$ lldb -- $QEMU -cpu nahelem -M q35,accel=hvf -cdrom test.iso
(lldb) target create "[...]/qemu/build/qemu-system-x86_64"
Current executable set to '[...]/qemu/build/qemu-system-x86_64' (x86_64).
(lldb) settings set -- target.run-args  "-cpu" "nahelem" "-M" "q35,accel=hvf" 
"-cdrom" "test.iso"
(lldb) r
Process 92411 launched: '[...]/qemu/build/qemu-system-x86_64' (x86_64)
**
ERROR:../qom/object.c:505:object_initialize_with_type: assertion failed: (size 
>= type->instance_size)
Bail out! ERROR:../qom/object.c:505:object_initialize_with_type: assertion 
failed: (size >= type->instance_size)
Process 92411 stopped
* thread #3, stop reason = signal SIGABRT
frame #0: 0x7fff6a75e33a libsystem_kernel.dylib`__pthread_kill + 10
libsystem_kernel.dylib`__pthread_kill:
->  0x7fff6a75e33a <+10>: jae0x7fff6a75e344; <+20>
0x7fff6a75e33c <+12>: movq   %rax, %rdi
0x7fff6a75e33f <+15>: jmp0x7fff6a758629; cerror_nocancel
0x7fff6a75e344 <+20>: retq
Target 0: (qemu-system-x86_64) stopped.
(lldb) bt
* thread #3, stop reason = signal SIGABRT
  * frame #0: 0x7fff6a75e33a libsystem_kernel.dylib`__pthread_kill + 10
frame #1: 0x7fff6a81ae60 libsystem_pthread.dylib`pthread_kill + 430
frame #2: 0x7fff6a6e5808 libsystem_c.dylib`abort + 120
frame #3: 0x000101314c36 libglib-2.0.0.dylib`g_assertion_message + 406
frame #4: 0x000101314c9e libglib-2.0.0.dylib`g_assertion_message_expr + 
94
frame #5: 0x000100366f0c 
qemu-system-x86_64`object_initialize_with_type(obj=, 
size=, type=) at object.c:505:5 [opt]
frame #6: 0x000100400e48 
qemu-system-x86_64`qbus_create_inplace(bus=0x, 
size=, typename=, parent=0x, 
name="main-system-bus") at bus.c:153:5 [opt]
frame #7: 0x00010006800a qemu-system-x86_64`sysbus_get_default 
[inlined] main_system_bus_create at sysbus.c:346:5 [opt]
frame #8: 0x000100067fe2 qemu-system-x86_64`sysbus_get_default at 
sysbus.c:354 [opt]
frame #9: 0x0001002b774f 
qemu-system-x86_64`qemu_init(argc=, argv=, 
envp=) at vl.c:3890:41 [opt]
frame #10: 0x00018c99 
qemu-system-x86_64`qemu_main(argc=, argv=, 
envp=) at main.c:49:5 [opt]
frame #11: 0x00010007bbd6 
qemu-system-x86_64`call_qemu_main(opaque=) at cocoa.m:1710:14 [opt]
frame #12: 0x0001004631ee 
qemu-system-x86_64`qemu_thread_start(args=) at 
qemu-thread-posix.c:521:9 [opt]
frame #13: 0x7fff6a81b109 libsystem_pthread.dylib`_pthread_start + 148
frame #14: 0x7fff6a816b8b libsystem_pthread.dylib`thread_start + 15
(lldb) f 7
qemu-system-x86_64 was compiled with optimization - stepping may behave oddly; 
variables may not be available.
frame #7: 0x00010006800a qemu-system-x86_64`sysbus_get_default [inlined] 
main_system_bus_create at sysbus.c:346:5 [opt]
   343  /* assign main_system_bus before qbus_create_inplace()
   344   * in order to make "if (bus != sysbus_get_default())" work */
   345  main_system_bus = g_malloc0(system_bus_info.instance_size);
-> 346  qbus_create_inplace(main_system_bus, system_bus_info.instance_size,
   347  TYPE_SYSTEM_BUS, NULL, "main-system-bus");
   348  OBJECT(main_system_bus)->free = g_free;
   349  }
(lldb) f 6
frame #6: 0x000100400e48 
qemu-system-x86_64`qbus_create_inplace(bus=0x, 
size=, typename=, parent=0x, 
name="main-system-bus") at bus.c:153:5 [opt]
   150  void qbus_create_inplace(void *bus, size_t size, const char *typename,
   151   DeviceState *parent, const char *name)
   152  {
-> 153  object_initialize(bus, size, typename);
   154  qbus_init(bus, parent, name);
   155  }
   156
(lldb) f 5
frame #5: 0x000100366f0c 
qemu-system-x86_64`object_initialize_with_type(obj=, 
size=, type=) at object.c:505:5 [opt]
   502
   503  g_assert(type->instance_size >= sizeof(Object));
   504  g_assert(type->abstract == false);
-> 505  g_assert(size >= type->instance_size);
   506
   507  memset(obj, 0, type->instance_size);
   508  obj->class = type->class;

Regards,
Roman

Re: [PATCH v2 05/10] hw/virtio/vhost-user:Remove dead assignment in scrub_shadow_regions()

[Li Qiang]

Chen Qun  于2020年8月25日周二 下午7:28写道：
>
> Clang static code analyzer show warning:
> hw/virtio/vhost-user.c:606:9: warning: Value stored to 'mr' is never read
> mr = vhost_user_get_mr_data(reg->userspace_addr, &offset, &fd);
> ^~
>
> Reported-by: Euler Robot 
> Signed-off-by: Chen Qun 
> Reviewed-by: Raphael Norwitz 

Reviewed-by: Li Qiang 

> ---
> Cc: "Michael S. Tsirkin" 
> Cc: Raphael Norwitz 
> ---
>  hw/virtio/vhost-user.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index d7e2423762..9c5b4f7fbc 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -603,7 +603,7 @@ static void scrub_shadow_regions(struct vhost_dev *dev,
>   */
>  for (i = 0; i < dev->mem->nregions; i++) {
>  reg = &dev->mem->regions[i];
> -mr = vhost_user_get_mr_data(reg->userspace_addr, &offset, &fd);
> +vhost_user_get_mr_data(reg->userspace_addr, &offset, &fd);
>  if (fd > 0) {
>  ++fd_num;
>  }
> --
> 2.23.0
>
>

Re: [PATCH v5 11/12] migration/dirtyrate: Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function

[David Edmondson]

On Monday, 2020-08-24 at 17:14:39 +08, Chuan Zheng wrote:

> Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function which could be 
> called
>
> Signed-off-by: Chuan Zheng 
> ---
>  migration/dirtyrate.c | 45 +
>  qapi/migration.json   | 44 
>  2 files changed, 89 insertions(+)
>
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index 9f52f5f..08c46d3 100644
> --- a/migration/dirtyrate.c
> +++ b/migration/dirtyrate.c
> @@ -62,6 +62,28 @@ static int dirtyrate_set_state(int *state, int old_state, 
> int new_state)
>  }
>  }
>  
> +static struct DirtyRateInfo *query_dirty_rate_info(void)
> +{
> +int64_t dirty_rate = DirtyStat.dirty_rate;
> +struct DirtyRateInfo *info = g_malloc0(sizeof(DirtyRateInfo));
> +
> +if (CalculatingState == DIRTY_RATE_STATUS_MEASURED) {
> +info->dirty_rate = dirty_rate;
> +} else {
> +info->dirty_rate = -1;
> +}
> +
> +info->status = CalculatingState;
> +/*
> + * Only support query once for each calculation,
> + * reset as DIRTY_RATE_STATUS_UNSTARTED after query
> + */
> +(void)dirtyrate_set_state(&CalculatingState, CalculatingState,
> +  DIRTY_RATE_STATUS_UNSTARTED);

Is there a reason for this restriction? Removing it would require
clarifying the state model, I suppose.

> +
> +return info;
> +}
> +
>  static void reset_dirtyrate_stat(void)
>  {
>  DirtyStat.total_dirty_samples = 0;
> @@ -378,3 +400,26 @@ void *get_dirtyrate_thread(void *arg)
>DIRTY_RATE_STATUS_MEASURED);
>  return NULL;
>  }
> +
> +void qmp_calc_dirty_rate(int64_t calc_time, Error **errp)
> +{
> +static struct DirtyRateConfig config;
> +QemuThread thread;
> +
> +/*
> + * We don't begin calculating thread only when it's in calculating 
> status.

"If the dirty rate is already being measured, don't attempt to start."

> + */
> +if (CalculatingState == DIRTY_RATE_STATUS_MEASURING) {

Should this return an error to the caller?

> +return;
> +}
> +
> +config.sample_period_seconds = get_sample_page_period(calc_time);

If the specified sample period is outside the min/max, should an error
be returned to the caller?

> +config.sample_pages_per_gigabytes = DIRTYRATE_DEFAULT_SAMPLE_PAGES;
> +qemu_thread_create(&thread, "get_dirtyrate", get_dirtyrate_thread,
> +   (void *)&config, QEMU_THREAD_DETACHED);
> +}
> +
> +struct DirtyRateInfo *qmp_query_dirty_rate(Error **errp)
> +{
> +return query_dirty_rate_info();
> +}
> diff --git a/qapi/migration.json b/qapi/migration.json
> index d640165..826bfd7 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -1737,3 +1737,47 @@
>  ##
>  { 'enum': 'DirtyRateStatus',
>'data': [ 'unstarted', 'measuring', 'measured'] }
> +
> +##
> +# @DirtyRateInfo:
> +#
> +# Information about current dirty page rate of vm.
> +#
> +# @dirty-rate: @dirtyrate describing the dirty page rate of vm
> +#  in units of MB/s.
> +#  If this field return '-1', it means querying is not
> +#  start or not complete.
> +#
> +# @status: status containing dirtyrate query status includes
> +#  'unstarted' or 'measuring' or 'measured'
> +#
> +# Since: 5.2
> +#
> +##
> +{ 'struct': 'DirtyRateInfo',
> +  'data': {'dirty-rate': 'int64',
> +   'status': 'DirtyRateStatus'} }
> +
> +##
> +# @calc-dirty-rate:
> +#
> +# start calculating dirty page rate for vm
> +#
> +# @calc-time: time in units of second for sample dirty pages
> +#
> +# Since: 5.2
> +#
> +# Example:
> +#   {"command": "cal-dirty-rate", "data": {"calc-time": 1} }

"cal-dirty-rate" -> "calc-dirty-rate".

> +#
> +##
> +{ 'command': 'calc-dirty-rate', 'data': {'calc-time': 'int64'} }
> +
> +##
> +# @query-dirty-rate:
> +#
> +# query dirty page rate in units of MB/s for vm
> +#
> +# Since: 5.2
> +##
> +{ 'command': 'query-dirty-rate', 'returns': 'DirtyRateInfo' }
> -- 
> 1.8.3.1

dme.
-- 
I'm going in the out door, I'm doing all right.

Re: [PATCH v5 03/12] migration/dirtyrate: Add RamlockDirtyInfo to store sampled page info

[Dr. David Alan Gilbert]

* David Edmondson (d...@dme.org) wrote:
> On Monday, 2020-08-24 at 17:14:31 +08, Chuan Zheng wrote:
> 
> > Add RamlockDirtyInfo to store sampled page info of each ramblock.
> >
> > Signed-off-by: Chuan Zheng 
> > ---
> >  migration/dirtyrate.h | 18 ++
> >  1 file changed, 18 insertions(+)
> >
> > diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
> > index 33669b7..7da 100644
> > --- a/migration/dirtyrate.h
> > +++ b/migration/dirtyrate.h
> > @@ -19,6 +19,11 @@
> >   */
> >  #define DIRTYRATE_DEFAULT_SAMPLE_PAGES512
> >  
> > +/*
> > + * Record ramblock idstr
> > + */
> > +#define RAMBLOCK_INFO_MAX_LEN 256
> > +
> >  /* Take 1s as default for calculation duration */
> >  #define DEFAULT_FETCH_DIRTYRATE_TIME_SEC  1
> >  
> > @@ -27,6 +32,19 @@ struct DirtyRateConfig {
> >  int64_t sample_period_seconds; /* time duration between two sampling */
> >  };
> >  
> > +/*
> > + * Store dirtypage info for each ramblock.
> > + */
> > +struct RamblockDirtyInfo {
> > +char idstr[RAMBLOCK_INFO_MAX_LEN]; /* idstr for each ramblock */
> > +uint8_t *ramblock_addr; /* base address of ramblock we measure */
> > +uint64_t ramblock_pages; /* ramblock size in 4K-page */
> 
> It's probably a stupid question, but why not store a pointer to the
> RAMBlock rather than copying some of the details?

I think I figured that out in the last round;  this code runs as:

rcu lock {
   calculate initial CRCs
}


rcu lock {
   calculate new CRCs
}

A RAMBlock might get deleted between the two.

Dave

   
> > +uint64_t *sample_page_vfn; /* relative offset address for sampled page 
> > */
> > +uint64_t sample_pages_count; /* count of sampled pages */
> > +uint64_t sample_dirty_count; /* cout of dirty pages we measure */
> 
> "cout" -> "count"
> 
> > +uint32_t *hash_result; /* array of hash result for sampled pages */
> > +};
> > +
> >  void *get_dirtyrate_thread(void *arg);
> >  #endif
> >  
> > -- 
> > 1.8.3.1
> 
> dme.
> -- 
> Please forgive me if I act a little strange, for I know not what I do.
> 
-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [PATCH 11/12] vnc-auth-sasl: Plug memleak in vnc_socket_ip_addr_string

[Li Qiang]

Pan Nengyuan  于2020年8月14日周五 下午6:40写道：
>
> 'addr' forgot to free in vnc_socket_ip_addr_string error path. Fix that.

s/forgot/is forgot, I think the maintainer will do this minor adjustment.

.



>
> Reported-by: Euler Robot 
> Signed-off-by: Pan Nengyuan 

Reviewed-by: Li Qiang 

> ---
> Cc: Gerd Hoffmann 
> ---
>  ui/vnc-auth-sasl.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c
> index 7b2b09f242..0517b2ead9 100644
> --- a/ui/vnc-auth-sasl.c
> +++ b/ui/vnc-auth-sasl.c
> @@ -522,6 +522,7 @@ vnc_socket_ip_addr_string(QIOChannelSocket *ioc,
>
>  if (addr->type != SOCKET_ADDRESS_TYPE_INET) {
>  error_setg(errp, "Not an inet socket type");
> +qapi_free_SocketAddress(addr);
>  return NULL;
>  }
>  ret = g_strdup_printf("%s;%s", addr->u.inet.host, addr->u.inet.port);
> --
> 2.18.2
>
>

Re: [PATCH 10/12] block/file-posix: fix a possible undefined behavior

[Li Qiang]

Pan Nengyuan  于2020年8月14日周五 下午6:32写道：
>
> local_err is not initialized to NULL, it will cause a assert error as below:
> qemu/util/error.c:59: error_setv: Assertion `*errp == NULL' failed.
>
> Fixes: c6447510690
> Reported-by: Euler Robot 
> Signed-off-by: Pan Nengyuan 

Reviewed-by: Li Qiang 

> ---
> Cc: Kevin Wolf 
> Cc: Max Reitz 
> Cc: Aarushi Mehta 
> Cc: qemu-bl...@nongnu.org
> ---
>  block/file-posix.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 9a00d4190a..697a7d9eea 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -2113,7 +2113,7 @@ static void raw_aio_attach_aio_context(BlockDriverState 
> *bs,
>  #endif
>  #ifdef CONFIG_LINUX_IO_URING
>  if (s->use_linux_io_uring) {
> -Error *local_err;
> +Error *local_err = NULL;
>  if (!aio_setup_linux_io_uring(new_context, &local_err)) {
>  error_reportf_err(local_err, "Unable to use linux io_uring, "
>   "falling back to thread pool: ");
> --
> 2.18.2
>
>

Re: [PATCH 09/12] blockdev: Fix a memleak in drive_backup_prepare()

[Li Qiang]

Pan Nengyuan  于2020年8月14日周五 下午6:54写道：
>
> 'local_err' seems forgot to propagate in error path, it'll cause
> a memleak. Fix it.
>
> Reported-by: Euler Robot 
> Signed-off-by: Pan Nengyuan 

Reviewed-by: Li Qiang 

> ---
> Cc: Kevin Wolf 
> Cc: Max Reitz 
> Cc: Markus Armbruster 
> Cc: qemu-bl...@nongnu.org
> ---
>  blockdev.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/blockdev.c b/blockdev.c
> index 3848a9c8ab..842ac289c1 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -1801,6 +1801,7 @@ static void drive_backup_prepare(BlkActionState 
> *common, Error **errp)
>  if (set_backing_hd) {
>  bdrv_set_backing_hd(target_bs, source, &local_err);
>  if (local_err) {
> +error_propagate(errp, local_err);
>  goto unref;
>  }
>  }
> --
> 2.18.2
>
>

QEMU | Pipeline #182124352 has failed for master | 78dca230

[GitLab via]

Your pipeline has failed.

Project: QEMU ( https://gitlab.com/qemu-project/qemu )
Branch: master ( https://gitlab.com/qemu-project/qemu/-/commits/master )

Commit: 78dca230 ( 
https://gitlab.com/qemu-project/qemu/-/commit/78dca230c97ed0d6e16ae0c96d5407644d991994
 )
Commit Message: Merge remote-tracking branch 'remotes/alistair/...
Commit Author: Peter Maydell ( https://gitlab.com/pm215 )

Pipeline #182124352 ( 
https://gitlab.com/qemu-project/qemu/-/pipelines/182124352 ) triggered by Alex 
Bennée ( https://gitlab.com/stsquad )
had 1 failed build.

Job #704293460 ( https://gitlab.com/qemu-project/qemu/-/jobs/704293460/raw )

Stage: test
Name: acceptance-system-centos
Trace: 10:35:39 ERROR|   File 
"/builds/qemu-project/qemu/python/qemu/machine.py", line 342, in launch
self._launch()

10:35:39 ERROR|   File "/builds/qemu-project/qemu/python/qemu/machine.py", line 
369, in _launch
self._post_launch()

10:35:39 ERROR|   File "/builds/qemu-project/qemu/python/qemu/machine.py", line 
288, in _post_launch
self._qmp.accept()

10:35:39 ERROR|   File "/builds/qemu-project/qemu/python/qemu/qmp.py", line 
236, in accept
return self.__negotiate_capabilities()

10:35:39 ERROR|   File "/builds/qemu-project/qemu/python/qemu/qmp.py", line 
129, in __negotiate_capabilities
resp = self.cmd('qmp_capabilities')

10:35:39 ERROR|   File "/builds/qemu-project/qemu/python/qemu/qmp.py", line 
266, in cmd
return self.cmd_obj(qmp_cmd)

10:35:39 ERROR|   File "/builds/qemu-project/qemu/python/qemu/qmp.py", line 
249, in cmd_obj
raise QMPConnectError("Unexpected empty reply from server")

10:35:39 ERROR| qemu.qmp.QMPConnectError: Unexpected empty reply from server

10:35:39 ERROR| ERROR 34-tests/acceptance/vnc.py:Vnc.test_change_password -> 
QMPConnectError: Unexpected empty reply from server
10:35:39 INFO | 
$ du -chs ${CI_PROJECT_DIR}/avocado-cache
1.2G/builds/qemu-project/qemu/avocado-cache
1.2Gtotal
section_end:1598438144:after_script
ERROR: Job failed: exit code 1



-- 
You're receiving this email because of your account on gitlab.com.

[PATCH v6 04/20] acpi: ged: add control regs

[Gerd Hoffmann]

Add control regs (sleep, reset) for hw-reduced acpi.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Igor Mammedov 
---
 include/hw/acpi/generic_event_device.h | 12 +++
 hw/acpi/generic_event_device.c | 44 ++
 2 files changed, 56 insertions(+)

diff --git a/include/hw/acpi/generic_event_device.h 
b/include/hw/acpi/generic_event_device.h
index 90a9180db572..4533cddb486c 100644
--- a/include/hw/acpi/generic_event_device.h
+++ b/include/hw/acpi/generic_event_device.h
@@ -72,6 +72,17 @@
 #define ACPI_GED_EVT_SEL_OFFSET0x0
 #define ACPI_GED_EVT_SEL_LEN   0x4
 
+#define ACPI_GED_REG_SLEEP_CTL 0x00
+#define ACPI_GED_REG_SLEEP_STS 0x01
+#define ACPI_GED_REG_RESET 0x02
+#define ACPI_GED_REG_COUNT 0x03
+
+/* ACPI_GED_REG_RESET value for reset*/
+#define ACPI_GED_RESET_VALUE   0x42
+
+/* ACPI_GED_REG_SLEEP_CTL.SLP_TYP value for S5 (aka poweroff) */
+#define ACPI_GED_SLP_TYP_S50x05
+
 #define GED_DEVICE  "GED"
 #define AML_GED_EVT_REG "EREG"
 #define AML_GED_EVT_SEL "ESEL"
@@ -87,6 +98,7 @@
 
 typedef struct GEDState {
 MemoryRegion evt;
+MemoryRegion regs;
 uint32_t sel;
 } GEDState;
 
diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
index b8abdefa1c77..491df80a5cc7 100644
--- a/hw/acpi/generic_event_device.c
+++ b/hw/acpi/generic_event_device.c
@@ -20,6 +20,7 @@
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
 #include "qemu/error-report.h"
+#include "sysemu/runstate.h"
 
 static const uint32_t ged_supported_events[] = {
 ACPI_GED_MEM_HOTPLUG_EVT,
@@ -176,6 +177,45 @@ static const MemoryRegionOps ged_evt_ops = {
 },
 };
 
+static uint64_t ged_regs_read(void *opaque, hwaddr addr, unsigned size)
+{
+return 0;
+}
+
+static void ged_regs_write(void *opaque, hwaddr addr, uint64_t data,
+   unsigned int size)
+{
+bool slp_en;
+int slp_typ;
+
+switch (addr) {
+case ACPI_GED_REG_SLEEP_CTL:
+slp_typ = (data >> 2) & 0x07;
+slp_en  = (data >> 5) & 0x01;
+if (slp_en && slp_typ == 5) {
+qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
+}
+return;
+case ACPI_GED_REG_SLEEP_STS:
+return;
+case ACPI_GED_REG_RESET:
+if (data == ACPI_GED_RESET_VALUE) {
+qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
+}
+return;
+}
+}
+
+static const MemoryRegionOps ged_regs_ops = {
+.read = ged_regs_read,
+.write = ged_regs_write,
+.endianness = DEVICE_LITTLE_ENDIAN,
+.valid = {
+.min_access_size = 1,
+.max_access_size = 1,
+},
+};
+
 static void acpi_ged_device_plug_cb(HotplugHandler *hotplug_dev,
 DeviceState *dev, Error **errp)
 {
@@ -332,6 +372,10 @@ static void acpi_ged_initfn(Object *obj)
  sysbus_init_mmio(sbd, &s->container_memhp);
  acpi_memory_hotplug_init(&s->container_memhp, OBJECT(dev),
   &s->memhp_state, 0);
+
+memory_region_init_io(&ged_st->regs, obj, &ged_regs_ops, ged_st,
+  TYPE_ACPI_GED "-regs", ACPI_GED_REG_COUNT);
+sysbus_init_mmio(sbd, &ged_st->regs);
 }
 
 static void acpi_ged_class_init(ObjectClass *class, void *data)
-- 
2.27.0

[PATCH v6 05/20] acpi: ged: add x86 device variant.

[Gerd Hoffmann]

Set AcpiDeviceIfClass->madt_cpu,
otherwise identical to TYPE_ACPI_GED.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Igor Mammedov 
---
 include/hw/acpi/generic_event_device.h |  4 +++
 hw/i386/generic_event_device_x86.c | 36 ++
 hw/i386/meson.build|  1 +
 3 files changed, 41 insertions(+)
 create mode 100644 hw/i386/generic_event_device_x86.c

diff --git a/include/hw/acpi/generic_event_device.h 
b/include/hw/acpi/generic_event_device.h
index 4533cddb486c..bd79cfff2b6a 100644
--- a/include/hw/acpi/generic_event_device.h
+++ b/include/hw/acpi/generic_event_device.h
@@ -69,6 +69,10 @@
 #define ACPI_GED(obj) \
 OBJECT_CHECK(AcpiGedState, (obj), TYPE_ACPI_GED)
 
+#define TYPE_ACPI_GED_X86 "acpi-ged-x86"
+#define ACPI_GED_X86(obj) \
+OBJECT_CHECK(AcpiGedX86State, (obj), TYPE_ACPI_GED_X86)
+
 #define ACPI_GED_EVT_SEL_OFFSET0x0
 #define ACPI_GED_EVT_SEL_LEN   0x4
 
diff --git a/hw/i386/generic_event_device_x86.c 
b/hw/i386/generic_event_device_x86.c
new file mode 100644
index ..e26fb02a2ef6
--- /dev/null
+++ b/hw/i386/generic_event_device_x86.c
@@ -0,0 +1,36 @@
+/*
+ * x86 variant of the generic event device for hw reduced acpi
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/acpi/generic_event_device.h"
+#include "hw/i386/pc.h"
+
+static void acpi_ged_x86_class_init(ObjectClass *class, void *data)
+{
+AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_CLASS(class);
+
+adevc->madt_cpu = pc_madt_cpu_entry;
+}
+
+static const TypeInfo acpi_ged_x86_info = {
+.name  = TYPE_ACPI_GED_X86,
+.parent= TYPE_ACPI_GED,
+.class_init= acpi_ged_x86_class_init,
+.interfaces = (InterfaceInfo[]) {
+{ TYPE_HOTPLUG_HANDLER },
+{ TYPE_ACPI_DEVICE_IF },
+{ }
+}
+};
+
+static void acpi_ged_x86_register_types(void)
+{
+type_register_static(&acpi_ged_x86_info);
+}
+
+type_init(acpi_ged_x86_register_types)
diff --git a/hw/i386/meson.build b/hw/i386/meson.build
index 63918fbe22f9..1a7d1a685d77 100644
--- a/hw/i386/meson.build
+++ b/hw/i386/meson.build
@@ -18,6 +18,7 @@ i386_ss.add(when: 'CONFIG_VMPORT', if_true: files('vmport.c'))
 i386_ss.add(when: 'CONFIG_VTD', if_true: files('intel_iommu.c'))
 
 i386_ss.add(when: 'CONFIG_ACPI', if_true: files('acpi-common.c'))
+i386_ss.add(when: 'CONFIG_ACPI_HW_REDUCED', if_true: 
files('generic_event_device_x86.c'))
 i386_ss.add(when: 'CONFIG_PC', if_true: files(
   'pc.c',
   'pc_sysfw.c',
-- 
2.27.0

[PATCH v6 02/20] seabios: add microvm config, update build rules

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 roms/Makefile   |  5 -
 roms/config.seabios-microvm | 26 ++
 2 files changed, 30 insertions(+), 1 deletion(-)
 create mode 100644 roms/config.seabios-microvm

diff --git a/roms/Makefile b/roms/Makefile
index 2673a39f9dc2..f8890c7e39de 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -70,9 +70,12 @@ default help:
@echo "  clean  -- delete the files generated by the 
previous" \
  "build targets"
 
-bios: build-seabios-config-seabios-128k build-seabios-config-seabios-256k
+bios: build-seabios-config-seabios-128k \
+   build-seabios-config-seabios-256k \
+   build-seabios-config-seabios-microvm
cp seabios/builds/seabios-128k/bios.bin ../pc-bios/bios.bin
cp seabios/builds/seabios-256k/bios.bin ../pc-bios/bios-256k.bin
+   cp seabios/builds/seabios-microvm/bios.bin ../pc-bios/bios-microvm.bin
 
 vgabios seavgabios: $(patsubst %,seavgabios-%,$(vgabios_variants))
 
diff --git a/roms/config.seabios-microvm b/roms/config.seabios-microvm
new file mode 100644
index ..a253e2edc6ec
--- /dev/null
+++ b/roms/config.seabios-microvm
@@ -0,0 +1,26 @@
+CONFIG_QEMU=y
+CONFIG_QEMU_HARDWARE=y
+CONFIG_PERMIT_UNALIGNED_PCIROM=y
+CONFIG_ROM_SIZE=128
+CONFIG_XEN=n
+CONFIG_BOOTSPLASH=n
+CONFIG_ATA=n
+CONFIG_AHCI=n
+CONFIG_SDCARD=n
+CONFIG_PVSCSI=n
+CONFIG_ESP_SCSI=n
+CONFIG_LSI_SCSI=n
+CONFIG_MEGASAS=n
+CONFIG_MPT_SCSI=n
+CONFIG_FLOPPY=n
+CONFIG_FLASH_FLOPPY=n
+CONFIG_NVME=n
+CONFIG_PS2PORT=n
+CONFIG_USB=n
+CONFIG_LPT=n
+CONFIG_RTC_TIMER=n
+CONFIG_USE_SMM=n
+CONFIG_PMTIMER=n
+CONFIG_TCGBIOS=n
+CONFIG_HARDWARE_IRQ=n
+CONFIG_ACPI_PARSE=y
-- 
2.27.0

[PATCH v6 01/20] microvm: name qboot binary qboot.rom

[Gerd Hoffmann]

qboot isn't a bios and shouldnt be named that way.

Signed-off-by: Gerd Hoffmann 
---
 hw/i386/microvm.c   |   4 ++--
 pc-bios/{bios-microvm.bin => qboot.rom} | Bin
 roms/Makefile   |   6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)
 rename pc-bios/{bios-microvm.bin => qboot.rom} (100%)

diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 81d0888930d1..b1dc7e49c159 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -47,7 +47,7 @@
 #include "kvm_i386.h"
 #include "hw/xen/start_info.h"
 
-#define MICROVM_BIOS_FILENAME "bios-microvm.bin"
+#define MICROVM_QBOOT_FILENAME "qboot.rom"
 
 static void microvm_set_rtc(MicrovmMachineState *mms, ISADevice *s)
 {
@@ -158,7 +158,7 @@ static void microvm_devices_init(MicrovmMachineState *mms)
 }
 
 if (bios_name == NULL) {
-bios_name = MICROVM_BIOS_FILENAME;
+bios_name = MICROVM_QBOOT_FILENAME;
 }
 x86_bios_rom_init(get_system_memory(), true);
 }
diff --git a/pc-bios/bios-microvm.bin b/pc-bios/qboot.rom
similarity index 100%
rename from pc-bios/bios-microvm.bin
rename to pc-bios/qboot.rom
diff --git a/roms/Makefile b/roms/Makefile
index 5d9f15b6777f..2673a39f9dc2 100644
--- a/roms/Makefile
+++ b/roms/Makefile
@@ -66,7 +66,7 @@ default help:
@echo "  efi-- update UEFI (edk2) platform firmware"
@echo "  opensbi32-generic  -- update OpenSBI for 32-bit generic 
machine"
@echo "  opensbi64-generic  -- update OpenSBI for 64-bit generic 
machine"
-   @echo "  bios-microvm   -- update bios-microvm.bin (qboot)"
+   @echo "  qboot  -- update qboot"
@echo "  clean  -- delete the files generated by the 
previous" \
  "build targets"
 
@@ -182,9 +182,9 @@ opensbi64-generic:
cp opensbi/build/platform/generic/firmware/fw_dynamic.bin 
../pc-bios/opensbi-riscv64-generic-fw_dynamic.bin
cp opensbi/build/platform/generic/firmware/fw_dynamic.elf 
../pc-bios/opensbi-riscv64-generic-fw_dynamic.elf
 
-bios-microvm:
+qboot:
$(MAKE) -C qboot
-   cp qboot/bios.bin ../pc-bios/bios-microvm.bin
+   cp qboot/bios.bin ../pc-bios/qboot.rom
 
 clean:
rm -rf seabios/.config seabios/out seabios/builds
-- 
2.27.0

[PATCH v6 12/20] microvm/acpi: disable virtio-mmio cmdline hack

[Gerd Hoffmann]

... in case we are using ACPI.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Igor Mammedov 
---
 hw/i386/microvm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 54510a03f754..04209eb38fbe 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -343,7 +343,8 @@ static void microvm_machine_reset(MachineState *machine)
 CPUState *cs;
 X86CPU *cpu;
 
-if (machine->kernel_filename != NULL &&
+if (!x86_machine_is_acpi_enabled(X86_MACHINE(machine)) &&
+machine->kernel_filename != NULL &&
 mms->auto_kernel_cmdline && !mms->kernel_cmdline_fixed) {
 microvm_fix_kernel_cmdline(machine);
 mms->kernel_cmdline_fixed = true;
-- 
2.27.0

[PATCH v6 03/20] seabios: add bios-microvm.bin binary

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 pc-bios/bios-microvm.bin | Bin 0 -> 131072 bytes
 1 file changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 pc-bios/bios-microvm.bin

diff --git a/pc-bios/bios-microvm.bin b/pc-bios/bios-microvm.bin
new file mode 100644
index 
..eefd32197e8d637be0266e3410551f711728c6d0
GIT binary patch
literal 131072
zcmeFadwf*Y_5Xb)nIw}ia0W79z#yZJHdIt%L5Y9{hJ>pc2q+f3)Y?=FZ7IwUEGUVS
zXb#7twqDz>ZKYnSR{OQqLKrI~1Sa5>i%O8HXse!KP~r{3Mdo?0eI^l6f6w#$^}Jrs
zYxMQWoU`{nYp=cb+H0@9_C5zq%lwy-Kt=)?31lRYkw8WQ83|-0kdZ(}0vQQpB#@Cn
zMgkcLWF(N0Kt=)?31lRYkw8WQ83|-0kdZ(}0vQQpB#@CnMgkcLWF(N0Kt=)?31lRY
zkw8WQ83|-0kdZ(}0vQQpB#@CnMgkcLWF(N0Kt=)?31lRYkw8WQ83|-0kdZ(}0vQQp
zB#@CnMgkcLWF(N0Kt=)?31lRYkw8WQ83|-0kdZ(}0vQQpB#@CnMgkcLWF(N0Kt=)?
z31lRYkw8WQ83|-0kdZ(}0vQQpB#@CnMgkcLWF(N0Kt=)?31lRYkw8WQ83|-0kdZ(}
z0vQQpB#@CnMgkcLWF(N0Kt=)?31lRYkw8WQ83|-0kdZ(}0vQQpB#@CnMgkcLWF(N0
z!2fp=D0FGsQqTY%0Ow|D+UwwL&^KGt27w?r)vamO;78y(5CKi#x8MQrAUMyfY14h0
zHV3%-YTAv3n)W!@04Di0trk2F2K3joWhZFbec%x=zDU!q0*gQcxDPxIJ_Oc4P1^(d
z4brrM;9Bq~NP4qMtzgVhO)L36<-t2=P#0JXUnRq#4Uf=|F5Q#EbXG);3|
zq-pnqgBNSsnoBfoca^3+Hbc`+tERue!{8slHIqIDV`kBppcd>0zDuba)PMvSc^N!`
zU(QBHKm%9-iZ0i*lRya=1Ezsr0Rub$o&#_EP}6=kN7L3`p=q~WiTtn9v_n@@PmQJ>
z1@50BpX)U374RBxU$1EezyLP*7)<*aGM}eu9pL@>@b`1t2mS;;0q6fh)AoVNU(#<2
zxCeg%o4~6e=U2!G{BaTGzNeu8%J4){yGrmY4?qnh^fTQqIKt(x|16W0d%0M>wgV3?(8rC>U^
z3>*L_-p)0+2K)@<{YKMzgF-M6`~)lnOTllz6Cm(gO*<2u3#Nddg7@y?{$BbJJb54E
z^M3dR7yMq+cKwg0eeVHu3)}|2{|Dp;(J3DUZ;u$29FDkOKLSV{5<(;Ag-DmplRgU=4T|><4%Mm9{;JE`cE+2wn#>pF+<-
z6SxiB3GN2>g9pHa;9>9>coIAfo(0X|U!VnS0Gq%|U<-H+bbx<@+0Sa)eDKmn#s~1k
zHLVy72NS?-Fdx)`yTN1NUtkOPFX#dfY(huDUeF0X2VLNAF7ziCoy$Y_JkSK=!6jfm
zxEE{(`+%OWX%*ln;1=)*Xa=8wQwq>$Fa!J;EC#;^tHDotqi)@{tmW+&p=@*_8!av5pWCm8`uv5=RgZQ2{wa&
zgMuG0&PQrm=sf20QP^FORmQjgcQW@q2fhUHG4NZC4IQUxM?hpe?V3P;fEF;kf;kBM
z1l$4s0XBiRz^A~f)U=IY@I?9ttN=0aD)=wxGYQ_nqm$7~(CZ@j2IpRk-39t3%tgRk
zh0cNtrqd4Kox$7$wpYXdOwz%xKnv){TsQ;NfWLyRpp5x&BDfPAVNMLrhCh&fIeG@>
zfP27e;Pf9dw}JOT{v7lg{1dzchRkI?0;gQToDO~tZU#?+1aM!8Os{5qfX$%jN9f-*
zjE`%Xx91__`RF2e5bOiz{}Q{k0GkZL;AXHB=)Zyo@F;i&oV*C#1C5{q6owfW;61;p
zmulMOUZ-8@_BMb0*@EX_;vg05u>9dfkTX
zK`9suZU%n_^DOKQSOrRN*R-K`GG~H^euJM1uKF!wA9(NLIj{h{1Wvyj`GFz#z&n@-
zW`NJZ^n2k2+ztK$wt)YDL!kG4=rouN{s+7Pa_(pT2bCZOM*fcR1?~qQfCFI2@6lB-
z3jFzh@Y^21_XQt-S{*8464EN;J@Gp55YHxfp@@O@Cood
zjGllnxE**OLFXUE&j1&Ki@{uQ7kC`J0KNb@e}N{L2yO(ofCs^5@NZD~7~=%|&*SvL
zU$I$WGx&a%OS@u*ORKrF=FSxVX+8hdApNiZm*?D){>?neD*SqJx%&oy0gKmsT5ZlfidMw|T@9*{L5J5|=P
zuy5U{z;f@Tc)3eUb~dzx^Fk}K#{_Qr!swXOHtVuWtIcIWzgcu(XXCE$NHgR$;zQp!
z`IWMGZP1+O%bw>c`*`6Xqt;_g^ct7>j746v+HG{%L!obUCUeablkLLQRadl`(%wGi
zBv08CZzR{83@
z#+LAjWu4&@qDy>QxUc=Vv?8?EUdeB1OENpUDJAVQeWkmgRA^4|o-xI9#uQ)Ki;J?1
z7tMaj#Mr&m6V5l@9NWo7q@Z+L^ziV-*;e%yW6K{J*3sC2F-nLw%g;ldJVrA__D0;;VITRD
zrf)O{Z-KMMRCs_nB-zhG$_b;>-bWrnDKJlL+!fh_46^7pk#_Y8Ps8R&?-hgU4xb%9
zy;AedDWn+tKNSjo=qjWQL2kUE#
ztVLUlPwNil*A~p%TwqNbxTQRqs5|V6cBBlQs@kffU0IRy%-&~oS*?Uy?vvQGKKc~&?{WPtOCfw13N5Xw;
z%EtL3xiw|e`-MH`WOwpIw8GdVim}~z!5rSWKRh~mG!^#ftEc+PUWp7f7r49c`*A-|
zU%klxg9V<(1>Q(+bAgZY1B{N;8%bZZ#h+XkUEHV9Yg;%@Up3xkP0PwE+qr0D-3=+mvL^&ZThNw((Bq9eV3=}ou^k0N1CXXR#)q7&v^`i%5}1@B(>D~
zdy-n(bDnfEwf3CbxZ3+bAFXYc@jin!nh2D(FiH;Y4)@6(=P8`-H75s1On!8Q=!vz^
zW0rfOb|BidJA9_OBoJLvq=ko=zMA@9`?I~_{?_b((P8&Gj0%6sMPIpi+nTsL`HJXH
z)B2V@Ir^$?O|8*)hnVhrWc)V2rOij=1mJh(~wp(XRYjceG`ATeRgoXCSxN
zEjdq%oMyaXejnqVyzxYH!rPQ^8;9(w4MbO3BDoeR$(&H~-L%1^1CVfZug91WbQ}I)
z=`QJs(Z#_VPsb*^^;IK=l%_22TL?zeA33o7gZ-_6u-iCLx<9`&|4=CT`sH(!DL{i~
zm<7x()2d}04q8#vG_Y=o>#DHVEN~KLyCfy+*pw_u$v!5fJiEzdX2l7TxT$YWoBCl0
zwr5HG(A?RQ$gMk=6~@eDhkG5}9JyevWF?2_t0rZ)zUwy%)@I9nj!GsmtMy%PenBNy
zuGV+my$Xzi6-Cki==m=igD|ND(f?d%3~F}qNZJ^dtzYOd+Ko3pdpKEwiM56mx!yEh
zG+sIIw(HZW(Zi{5A8Xl2ZN-E^uBHh?T*>2fZ1PJV6p
z3FjsA*Gfs6?95+SRDE^P8m*3)Nx5Wn*iTT@Nf@sZ<|OVXMPIEw^ccgyzI=n+s3Czy
zJ@gt)X_L%{$xJ*-W?GHfj5kCJ!=)%wJ30j|ZH@em1^HwDjgrQQ<8lJzBx~!IoU4T+
z#;fgaDXD$^VGk+xufFYJFFCM3i#(o`uKebc
zJYq&hA2CXjgXRWOI&}>8Vg(ogKRXF6ZNmbyc_
z-tZ2k65IGQe)PbMS<#$G|ajDy=(2QAb(;aNr
zvLTYVUn&V@#l%e6w;y6=l2TU4?W8P|6z7qf+~&DuJM_jHo+yj!jaTxkuL|WcF}9?l
z+fuI1zK5Kmn5J%uW4bBotERe3cST!M(d{W$Yv1k0MVfJi!v}Pg|JYClkZWpeN0@$I011PBuA(9$=HzRz)H%y75HB!;~TIN?|iy(XKV=710
zaN{~Waqj>rL@zi?wvGbs2
zh5V|e42rX(Z6)7YSXjg|bgZRv^|VB-)0N*Wl`~|w7%v#v##DDaQfzP5BbK=t_m3;e|xA%o^B5nOG0}nAa~H+^e^@oL-}frXzVav)>n739=W2GD88+)
z-eepyI$4J#_E8KAVYC`Mtl6%ovy3-C-dnnVOjhI!YkJn+=s{uZoN)pd%MhYB-TY!vm|eKAGx(0Q(SC_l^iK|J)mtopb{QDf0b@HUTC
zWlZ!=7DI_H)<|h9Wi2SOmUzu7zbrJ(DV^qw&y#s+Nt)#WJb$k_yVETH9AD#vWZrX9
zIgb7MT6fvbg}FwTaex(Bm;LVNsg!w%EZ9Bf<)53AJG+VOIMYd%Mojyv;yI%R>BDUzPpo=yo%nPO7rBwAFGP;W?6
z{~IZy1#$cAeJt6RxYDcHU17hhWDnBz)5t*n9wG*I&pK8XA!>f{7;hM_+G`jd5(J3k
z8lA?Wnov#L_jLwU7(8`nqd<41y+9OFF}dv`T))!4Emo)&r9H+DRS9Yd!Di{hGp
z-rQi3H`bF1Nk)
zvs9{S;$fQKuC9(m+r05m>iWv$--Pr5=CdKEaxt&P>#`k}qkim@kL#KsoM%77Fi3vH
zx-vTUVN$ekz<95oX-C&;U2WZIdA2FVI+ap5KC2_RvQ4!US&%$;r-&wH-X&gcq!&ZA
z(=NeRNS>mvULW)2unV>irGJ1VQ+7!;1`qt(|ADF*m4qJ8RrOVgeMc)&Z-4I4e{|Gm
zSL}3(SQCO-_8ih{Z~;#*&x%h74sjW8s^mpb-Y=<1fm0p>+6gu->^csJ!BipENkiWla;%~@MPo!6F(iptM

Re: [PATCH v5 03/12] migration/dirtyrate: Add RamlockDirtyInfo to store sampled page info

[David Edmondson]

On Wednesday, 2020-08-26 at 11:33:30 +01, Dr. David Alan Gilbert wrote:

> * David Edmondson (d...@dme.org) wrote:
>> On Monday, 2020-08-24 at 17:14:31 +08, Chuan Zheng wrote:
>> 
>> > Add RamlockDirtyInfo to store sampled page info of each ramblock.
>> >
>> > Signed-off-by: Chuan Zheng 
>> > ---
>> >  migration/dirtyrate.h | 18 ++
>> >  1 file changed, 18 insertions(+)
>> >
>> > diff --git a/migration/dirtyrate.h b/migration/dirtyrate.h
>> > index 33669b7..7da 100644
>> > --- a/migration/dirtyrate.h
>> > +++ b/migration/dirtyrate.h
>> > @@ -19,6 +19,11 @@
>> >   */
>> >  #define DIRTYRATE_DEFAULT_SAMPLE_PAGES512
>> >  
>> > +/*
>> > + * Record ramblock idstr
>> > + */
>> > +#define RAMBLOCK_INFO_MAX_LEN 256
>> > +
>> >  /* Take 1s as default for calculation duration */
>> >  #define DEFAULT_FETCH_DIRTYRATE_TIME_SEC  1
>> >  
>> > @@ -27,6 +32,19 @@ struct DirtyRateConfig {
>> >  int64_t sample_period_seconds; /* time duration between two sampling 
>> > */
>> >  };
>> >  
>> > +/*
>> > + * Store dirtypage info for each ramblock.
>> > + */
>> > +struct RamblockDirtyInfo {
>> > +char idstr[RAMBLOCK_INFO_MAX_LEN]; /* idstr for each ramblock */
>> > +uint8_t *ramblock_addr; /* base address of ramblock we measure */
>> > +uint64_t ramblock_pages; /* ramblock size in 4K-page */
>> 
>> It's probably a stupid question, but why not store a pointer to the
>> RAMBlock rather than copying some of the details?
>
> I think I figured that out in the last round;  this code runs as:
>
> rcu lock {
>calculate initial CRCs
> }
>
> 
> rcu lock {
>calculate new CRCs
> }
>
> A RAMBlock might get deleted between the two.

Makes sense, thanks.

dme.
-- 
Why does it have to be like this? I can never tell.

[PATCH v6 15/20] x86: move cpu hotplug from pc to x86

[Gerd Hoffmann]

The cpu hotplug code handles the initialization of coldplugged cpus
too, so it is needed even in case cpu hotplug is not supported.

Move the code from pc to x86, so microvm can use it.

Move both plug and unplug to keep everything in one place, even
though microvm needs plug only.

Signed-off-by: Gerd Hoffmann 
---
 include/hw/i386/x86.h |  10 ++
 hw/i386/pc.c  | 283 +-
 hw/i386/x86.c | 268 +++
 3 files changed, 283 insertions(+), 278 deletions(-)

diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
index de74c831c3ab..5c3803e74e31 100644
--- a/include/hw/i386/x86.h
+++ b/include/hw/i386/x86.h
@@ -102,6 +102,16 @@ CpuInstanceProperties x86_cpu_index_to_props(MachineState 
*ms,
  unsigned cpu_index);
 int64_t x86_get_default_cpu_node_id(const MachineState *ms, int idx);
 const CPUArchIdList *x86_possible_cpu_arch_ids(MachineState *ms);
+CPUArchId *x86_find_cpu_slot(MachineState *ms, uint32_t id, int *idx);
+void x86_rtc_set_cpus_count(ISADevice *rtc, uint16_t cpus_count);
+void x86_cpu_pre_plug(HotplugHandler *hotplug_dev,
+  DeviceState *dev, Error **errp);
+void x86_cpu_plug(HotplugHandler *hotplug_dev,
+  DeviceState *dev, Error **errp);
+void x86_cpu_unplug_request_cb(HotplugHandler *hotplug_dev,
+   DeviceState *dev, Error **errp);
+void x86_cpu_unplug_cb(HotplugHandler *hotplug_dev,
+   DeviceState *dev, Error **errp);
 
 void x86_bios_rom_init(MemoryRegion *rom_memory, bool isapc_ram_fw);
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 0bd6dbbd7bf6..b55369357e5d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -803,19 +803,6 @@ void pc_hot_add_cpu(MachineState *ms, const int64_t id, 
Error **errp)
 }
 }
 
-static void rtc_set_cpus_count(ISADevice *rtc, uint16_t cpus_count)
-{
-if (cpus_count > 0xff) {
-/* If the number of CPUs can't be represented in 8 bits, the
- * BIOS must use "FW_CFG_NB_CPUS". Set RTC field to 0 just
- * to make old BIOSes fail more predictably.
- */
-rtc_set_memory(rtc, 0x5f, 0);
-} else {
-rtc_set_memory(rtc, 0x5f, cpus_count - 1);
-}
-}
-
 static
 void pc_machine_done(Notifier *notifier, void *data)
 {
@@ -825,7 +812,7 @@ void pc_machine_done(Notifier *notifier, void *data)
 PCIBus *bus = pcms->bus;
 
 /* set the number of CPUs */
-rtc_set_cpus_count(x86ms->rtc, x86ms->boot_cpus);
+x86_rtc_set_cpus_count(x86ms->rtc, x86ms->boot_cpus);
 
 if (bus) {
 int extra_hosts = 0;
@@ -1373,266 +1360,6 @@ static void pc_memory_unplug(HotplugHandler 
*hotplug_dev,
 error_propagate(errp, local_err);
 }
 
-static int pc_apic_cmp(const void *a, const void *b)
-{
-   CPUArchId *apic_a = (CPUArchId *)a;
-   CPUArchId *apic_b = (CPUArchId *)b;
-
-   return apic_a->arch_id - apic_b->arch_id;
-}
-
-/* returns pointer to CPUArchId descriptor that matches CPU's apic_id
- * in ms->possible_cpus->cpus, if ms->possible_cpus->cpus has no
- * entry corresponding to CPU's apic_id returns NULL.
- */
-static CPUArchId *pc_find_cpu_slot(MachineState *ms, uint32_t id, int *idx)
-{
-CPUArchId apic_id, *found_cpu;
-
-apic_id.arch_id = id;
-found_cpu = bsearch(&apic_id, ms->possible_cpus->cpus,
-ms->possible_cpus->len, sizeof(*ms->possible_cpus->cpus),
-pc_apic_cmp);
-if (found_cpu && idx) {
-*idx = found_cpu - ms->possible_cpus->cpus;
-}
-return found_cpu;
-}
-
-static void pc_cpu_plug(HotplugHandler *hotplug_dev,
-DeviceState *dev, Error **errp)
-{
-CPUArchId *found_cpu;
-Error *local_err = NULL;
-X86CPU *cpu = X86_CPU(dev);
-PCMachineState *pcms = PC_MACHINE(hotplug_dev);
-X86MachineState *x86ms = X86_MACHINE(hotplug_dev);
-
-if (x86ms->acpi_dev) {
-hotplug_handler_plug(x86ms->acpi_dev, dev, &local_err);
-if (local_err) {
-goto out;
-}
-}
-
-/* increment the number of CPUs */
-x86ms->boot_cpus++;
-if (x86ms->rtc) {
-rtc_set_cpus_count(x86ms->rtc, x86ms->boot_cpus);
-}
-if (x86ms->fw_cfg) {
-fw_cfg_modify_i16(x86ms->fw_cfg, FW_CFG_NB_CPUS, x86ms->boot_cpus);
-}
-
-found_cpu = pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, NULL);
-found_cpu->cpu = OBJECT(dev);
-out:
-error_propagate(errp, local_err);
-}
-static void pc_cpu_unplug_request_cb(HotplugHandler *hotplug_dev,
- DeviceState *dev, Error **errp)
-{
-int idx = -1;
-X86CPU *cpu = X86_CPU(dev);
-PCMachineState *pcms = PC_MACHINE(hotplug_dev);
-X86MachineState *x86ms = X86_MACHINE(hotplug_dev);
-
-if (!x86ms->acpi_dev) {
-error_setg(errp, "CPU hot unplug not supported without ACPI");
-return;
-}
-
-pc_find_cpu_slot(MACHINE(pcms), cpu->apic_id, &idx);
-assert(idx != -1);
-   

[PATCH v6 19/20] tests/acpi: add microvm test

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
---
 tests/qtest/bios-tables-test.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 058ba3886659..883532e531d6 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1008,6 +1008,20 @@ static void test_acpi_virt_tcg_memhp(void)
 
 }
 
+static void test_acpi_microvm_tcg(void)
+{
+test_data data;
+
+memset(&data, 0, sizeof(data));
+data.machine = "microvm";
+data.required_struct_types = NULL; /* no smbios */
+data.required_struct_types_len = 0;
+data.blkdev = "virtio-blk-device";
+test_acpi_one(" -machine microvm,acpi=on,rtc=off",
+  &data);
+free_test_data(&data);
+}
+
 static void test_acpi_virt_tcg_numamem(void)
 {
 test_data data = {
@@ -1119,6 +1133,7 @@ int main(int argc, char *argv[])
 qtest_add_func("acpi/q35/dimmpxm", test_acpi_q35_tcg_dimm_pxm);
 qtest_add_func("acpi/piix4/acpihmat", test_acpi_piix4_tcg_acpi_hmat);
 qtest_add_func("acpi/q35/acpihmat", test_acpi_q35_tcg_acpi_hmat);
+qtest_add_func("acpi/microvm", test_acpi_microvm_tcg);
 } else if (strcmp(arch, "aarch64") == 0) {
 qtest_add_func("acpi/virt", test_acpi_virt_tcg);
 qtest_add_func("acpi/virt/numamem", test_acpi_virt_tcg_numamem);
-- 
2.27.0

[PATCH v6 14/20] x86: move acpi_dev from pc/microvm

[Gerd Hoffmann]

Both pc and microvm machine types have a acpi_dev field.
Move it to the common base type.

Signed-off-by: Gerd Hoffmann 
---
 include/hw/i386/microvm.h |  1 -
 include/hw/i386/pc.h  |  1 -
 include/hw/i386/x86.h |  1 +
 hw/i386/acpi-build.c  |  2 +-
 hw/i386/acpi-microvm.c|  5 +++--
 hw/i386/microvm.c | 10 ++
 hw/i386/pc.c  | 34 +++---
 hw/i386/pc_piix.c |  2 +-
 hw/i386/pc_q35.c  |  2 +-
 9 files changed, 32 insertions(+), 26 deletions(-)

diff --git a/include/hw/i386/microvm.h b/include/hw/i386/microvm.h
index b6e0d4395af7..b8ec99aeb051 100644
--- a/include/hw/i386/microvm.h
+++ b/include/hw/i386/microvm.h
@@ -66,7 +66,6 @@ typedef struct {
 bool kernel_cmdline_fixed;
 Notifier machine_done;
 Notifier powerdown_req;
-AcpiDeviceIf *acpi_dev;
 } MicrovmMachineState;
 
 #define TYPE_MICROVM_MACHINE   MACHINE_TYPE_NAME("microvm")
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index fe52e165b27c..0f7da2329b0f 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -29,7 +29,6 @@ struct PCMachineState {
 Notifier machine_done;
 
 /* Pointers to devices and objects: */
-HotplugHandler *acpi_dev;
 PCIBus *bus;
 I2CBus *smbus;
 PFlashCFI01 *flash[2];
diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
index a350ea3609f5..de74c831c3ab 100644
--- a/include/hw/i386/x86.h
+++ b/include/hw/i386/x86.h
@@ -50,6 +50,7 @@ typedef struct {
 FWCfgState *fw_cfg;
 qemu_irq *gsi;
 GMappedFile *initrd_mapped_file;
+HotplugHandler *acpi_dev;
 
 /* RAM information (sizes, addresses, configuration): */
 ram_addr_t below_4g_mem_size, above_4g_mem_size;
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index b7bc2a35..c356cc71fe08 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2431,7 +2431,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState 
*machine)
 
 acpi_add_table(table_offsets, tables_blob);
 acpi_build_madt(tables_blob, tables->linker, x86ms,
-ACPI_DEVICE_IF(pcms->acpi_dev), true);
+ACPI_DEVICE_IF(x86ms->acpi_dev), true);
 
 vmgenid_dev = find_vmgenid_dev();
 if (vmgenid_dev) {
diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c
index b9ce3768b263..df39c5d3bd90 100644
--- a/hw/i386/acpi-microvm.c
+++ b/hw/i386/acpi-microvm.c
@@ -108,7 +108,7 @@ build_dsdt_microvm(GArray *table_data, BIOSLinker *linker,
 sb_scope = aml_scope("_SB");
 fw_cfg_add_acpi_dsdt(sb_scope, x86ms->fw_cfg);
 isa_build_aml(ISA_BUS(isabus), sb_scope);
-build_ged_aml(sb_scope, GED_DEVICE, HOTPLUG_HANDLER(mms->acpi_dev),
+build_ged_aml(sb_scope, GED_DEVICE, x86ms->acpi_dev,
   GED_MMIO_IRQ, AML_SYSTEM_MEMORY, GED_MMIO_BASE);
 acpi_dsdt_add_power_button(sb_scope);
 acpi_dsdt_add_virtio(sb_scope, mms);
@@ -136,6 +136,7 @@ static void acpi_build_microvm(AcpiBuildTables *tables,
MicrovmMachineState *mms)
 {
 MachineState *machine = MACHINE(mms);
+X86MachineState *x86ms = X86_MACHINE(mms);
 GArray *table_offsets;
 GArray *tables_blob = tables->table_data;
 unsigned dsdt, xsdt;
@@ -183,7 +184,7 @@ static void acpi_build_microvm(AcpiBuildTables *tables,
 
 acpi_add_table(table_offsets, tables_blob);
 acpi_build_madt(tables_blob, tables->linker, X86_MACHINE(machine),
-mms->acpi_dev, false);
+ACPI_DEVICE_IF(x86ms->acpi_dev), false);
 
 xsdt = tables_blob->len;
 build_xsdt(tables_blob, tables->linker, table_offsets, NULL, NULL);
diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index 04209eb38fbe..9df15354ce0f 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -143,7 +143,7 @@ static void microvm_devices_init(MicrovmMachineState *mms)
 sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0,
x86ms->gsi[GED_MMIO_IRQ]);
 sysbus_realize(SYS_BUS_DEVICE(dev), &error_fatal);
-mms->acpi_dev = ACPI_DEVICE_IF(dev);
+x86ms->acpi_dev = HOTPLUG_HANDLER(dev);
 }
 
 if (mms->pic == ON_OFF_AUTO_ON || mms->pic == ON_OFF_AUTO_AUTO) {
@@ -469,11 +469,13 @@ static void microvm_powerdown_req(Notifier *notifier, 
void *data)
 {
 MicrovmMachineState *mms = container_of(notifier, MicrovmMachineState,
 powerdown_req);
+X86MachineState *x86ms = X86_MACHINE(mms);
 
-if (mms->acpi_dev) {
-Object *obj = OBJECT(mms->acpi_dev);
+if (x86ms->acpi_dev) {
+Object *obj = OBJECT(x86ms->acpi_dev);
 AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(obj);
-adevc->send_event(mms->acpi_dev, ACPI_POWER_DOWN_STATUS);
+adevc->send_event(ACPI_DEVICE_IF(x86ms->acpi_dev),
+  ACPI_POWER_DOWN_STATUS);
 }
 }
 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 5d8d5ef8b373..0bd6dbbd7bf6 100644
--- a/hw/i386/pc.

[PATCH v6 10/20] microvm/acpi: use GSI 16-23 for virtio

[Gerd Hoffmann]

With ACPI enabled and IO-APIC being properly declared in the ACPI tables
we can use interrupt lines 16-23 for virtio and avoid shared interrupts.

With acpi disabled we continue to use lines 5-12.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Sergio Lopez 
Reviewed-by: Igor Mammedov 
---
 hw/i386/microvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index e1b86da8a92e..ca0c9983f137 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -125,7 +125,7 @@ static void microvm_devices_init(MicrovmMachineState *mms)
 
 kvmclock_create();
 
-mms->virtio_irq_base = 5;
+mms->virtio_irq_base = x86_machine_is_acpi_enabled(x86ms) ? 16 : 5;
 for (i = 0; i < VIRTIO_NUM_TRANSPORTS; i++) {
 sysbus_create_simple("virtio-mmio",
  VIRTIO_MMIO_BASE + i * 512,
-- 
2.27.0

[PATCH v6 20/20] tests/acpi: update expected data files for microvm

[Gerd Hoffmann]

Also clear tests/qtest/bios-tables-test-allowed-diff.h

Signed-off-by: Gerd Hoffmann 
---
 tests/qtest/bios-tables-test-allowed-diff.h |   3 ---
 tests/data/acpi/microvm/APIC| Bin 0 -> 70 bytes
 tests/data/acpi/microvm/DSDT| Bin 0 -> 365 bytes
 tests/data/acpi/microvm/FACP| Bin 0 -> 268 bytes
 4 files changed, 3 deletions(-)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h 
b/tests/qtest/bios-tables-test-allowed-diff.h
index 97c3fa621b7f..dfb8523c8bf4 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,4 +1 @@
 /* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/microvm/APIC",
-"tests/data/acpi/microvm/DSDT",
-"tests/data/acpi/microvm/FACP",
diff --git a/tests/data/acpi/microvm/APIC b/tests/data/acpi/microvm/APIC
index 
e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..7472c7e830b6c7139720e93dd544d4441556661d
 100644
GIT binary patch
literal 70
zcmZ<^@N{-#U|?Xp?&R<65v<@85#a0y6k`O6f!H9Lf#JbFFwFr}2jnsGfW!{`1CcCj
H|A7JkCE=5t
zE;#2y@U)soH1M>j)Okp0N|?)B()oaTI~2uqaQfeg{#@eeMV8=e3}=mj(J&BYn|!sEgD;aBbPnKQWME*ebMklg2v%^42yk`-iUEZfKx`0ARlp*^z`()4z{IrRAIMxMsv1tD
QVL

[PATCH v6 07/20] microvm: make virtio irq base runtime configurable

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Sergio Lopez 
Reviewed-by: Igor Mammedov 
---
 include/hw/i386/microvm.h |  2 +-
 hw/i386/microvm.c | 11 +++
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/include/hw/i386/microvm.h b/include/hw/i386/microvm.h
index fd34b78e0d2a..03e735723726 100644
--- a/include/hw/i386/microvm.h
+++ b/include/hw/i386/microvm.h
@@ -27,7 +27,6 @@
 
 /* Platform virtio definitions */
 #define VIRTIO_MMIO_BASE  0xfeb0
-#define VIRTIO_IRQ_BASE   5
 #define VIRTIO_NUM_TRANSPORTS 8
 #define VIRTIO_CMDLINE_MAXLEN 64
 
@@ -57,6 +56,7 @@ typedef struct {
 bool auto_kernel_cmdline;
 
 /* Machine state */
+uint32_t virtio_irq_base;
 bool kernel_cmdline_fixed;
 } MicrovmMachineState;
 
diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index b1dc7e49c159..e4501f2cdfbd 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -121,10 +121,11 @@ static void microvm_devices_init(MicrovmMachineState *mms)
 
 kvmclock_create();
 
+mms->virtio_irq_base = 5;
 for (i = 0; i < VIRTIO_NUM_TRANSPORTS; i++) {
 sysbus_create_simple("virtio-mmio",
  VIRTIO_MMIO_BASE + i * 512,
- x86ms->gsi[VIRTIO_IRQ_BASE + i]);
+ x86ms->gsi[mms->virtio_irq_base + i]);
 }
 
 /* Optional and legacy devices */
@@ -227,7 +228,7 @@ static void microvm_memory_init(MicrovmMachineState *mms)
 x86ms->ioapic_as = &address_space_memory;
 }
 
-static gchar *microvm_get_mmio_cmdline(gchar *name)
+static gchar *microvm_get_mmio_cmdline(gchar *name, uint32_t virtio_irq_base)
 {
 gchar *cmdline;
 gchar *separator;
@@ -247,7 +248,7 @@ static gchar *microvm_get_mmio_cmdline(gchar *name)
 ret = g_snprintf(cmdline, VIRTIO_CMDLINE_MAXLEN,
  " virtio_mmio.device=512@0x%lx:%ld",
  VIRTIO_MMIO_BASE + index * 512,
- VIRTIO_IRQ_BASE + index);
+ virtio_irq_base + index);
 if (ret < 0 || ret >= VIRTIO_CMDLINE_MAXLEN) {
 g_free(cmdline);
 return NULL;
@@ -259,6 +260,7 @@ static gchar *microvm_get_mmio_cmdline(gchar *name)
 static void microvm_fix_kernel_cmdline(MachineState *machine)
 {
 X86MachineState *x86ms = X86_MACHINE(machine);
+MicrovmMachineState *mms = MICROVM_MACHINE(machine);
 BusState *bus;
 BusChild *kid;
 char *cmdline;
@@ -282,7 +284,8 @@ static void microvm_fix_kernel_cmdline(MachineState 
*machine)
 BusState *mmio_bus = &mmio_virtio_bus->parent_obj;
 
 if (!QTAILQ_EMPTY(&mmio_bus->children)) {
-gchar *mmio_cmdline = microvm_get_mmio_cmdline(mmio_bus->name);
+gchar *mmio_cmdline = microvm_get_mmio_cmdline
+(mmio_bus->name, mms->virtio_irq_base);
 if (mmio_cmdline) {
 char *newcmd = g_strjoin(NULL, cmdline, mmio_cmdline, 
NULL);
 g_free(mmio_cmdline);
-- 
2.27.0

[PATCH v6 08/20] microvm/acpi: add minimal acpi support

[Gerd Hoffmann]

$subject says all.  Can be controlled using -M microvm,acpi=on/off.

Signed-off-by: Gerd Hoffmann 
---
 hw/i386/acpi-microvm.h|   8 ++
 include/hw/i386/microvm.h |   9 ++
 hw/i386/acpi-microvm.c| 187 ++
 hw/i386/microvm.c |  40 
 hw/i386/Kconfig   |   1 +
 hw/i386/meson.build   |   2 +-
 6 files changed, 246 insertions(+), 1 deletion(-)
 create mode 100644 hw/i386/acpi-microvm.h
 create mode 100644 hw/i386/acpi-microvm.c

diff --git a/hw/i386/acpi-microvm.h b/hw/i386/acpi-microvm.h
new file mode 100644
index ..dfe853690e15
--- /dev/null
+++ b/hw/i386/acpi-microvm.h
@@ -0,0 +1,8 @@
+#ifndef HW_I386_ACPI_MICROVM_H
+#define HW_I386_ACPI_MICROVM_H
+
+#include "hw/i386/microvm.h"
+
+void acpi_setup_microvm(MicrovmMachineState *mms);
+
+#endif
diff --git a/include/hw/i386/microvm.h b/include/hw/i386/microvm.h
index 03e735723726..b6e0d4395af7 100644
--- a/include/hw/i386/microvm.h
+++ b/include/hw/i386/microvm.h
@@ -24,12 +24,18 @@
 
 #include "hw/boards.h"
 #include "hw/i386/x86.h"
+#include "hw/acpi/acpi_dev_interface.h"
 
 /* Platform virtio definitions */
 #define VIRTIO_MMIO_BASE  0xfeb0
 #define VIRTIO_NUM_TRANSPORTS 8
 #define VIRTIO_CMDLINE_MAXLEN 64
 
+#define GED_MMIO_BASE 0xfea0
+#define GED_MMIO_BASE_MEMHP   (GED_MMIO_BASE + 0x100)
+#define GED_MMIO_BASE_REGS(GED_MMIO_BASE + 0x200)
+#define GED_MMIO_IRQ  9
+
 /* Machine type options */
 #define MICROVM_MACHINE_PIT "pit"
 #define MICROVM_MACHINE_PIC "pic"
@@ -58,6 +64,9 @@ typedef struct {
 /* Machine state */
 uint32_t virtio_irq_base;
 bool kernel_cmdline_fixed;
+Notifier machine_done;
+Notifier powerdown_req;
+AcpiDeviceIf *acpi_dev;
 } MicrovmMachineState;
 
 #define TYPE_MICROVM_MACHINE   MACHINE_TYPE_NAME("microvm")
diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c
new file mode 100644
index ..06ef33949f5f
--- /dev/null
+++ b/hw/i386/acpi-microvm.c
@@ -0,0 +1,187 @@
+/* Support for generating ACPI tables and passing them to Guests
+ *
+ * Copyright (C) 2008-2010  Kevin O'Connor 
+ * Copyright (C) 2006 Fabrice Bellard
+ * Copyright (C) 2013 Red Hat Inc
+ *
+ * Author: Michael S. Tsirkin 
+ *
+ * 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 .
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+
+#include "exec/memory.h"
+#include "hw/acpi/acpi.h"
+#include "hw/acpi/aml-build.h"
+#include "hw/acpi/bios-linker-loader.h"
+#include "hw/acpi/generic_event_device.h"
+#include "hw/acpi/utils.h"
+#include "hw/boards.h"
+#include "hw/i386/fw_cfg.h"
+#include "hw/i386/microvm.h"
+
+#include "acpi-common.h"
+#include "acpi-microvm.h"
+
+static void
+build_dsdt_microvm(GArray *table_data, BIOSLinker *linker,
+   MicrovmMachineState *mms)
+{
+X86MachineState *x86ms = X86_MACHINE(mms);
+Aml *dsdt, *sb_scope, *scope, *pkg;
+bool ambiguous;
+Object *isabus;
+
+isabus = object_resolve_path_type("", TYPE_ISA_BUS, &ambiguous);
+assert(isabus);
+assert(!ambiguous);
+
+dsdt = init_aml_allocator();
+
+/* Reserve space for header */
+acpi_data_push(dsdt->buf, sizeof(AcpiTableHeader));
+
+sb_scope = aml_scope("_SB");
+fw_cfg_add_acpi_dsdt(sb_scope, x86ms->fw_cfg);
+isa_build_aml(ISA_BUS(isabus), sb_scope);
+build_ged_aml(sb_scope, GED_DEVICE, HOTPLUG_HANDLER(mms->acpi_dev),
+  GED_MMIO_IRQ, AML_SYSTEM_MEMORY, GED_MMIO_BASE);
+acpi_dsdt_add_power_button(sb_scope);
+aml_append(dsdt, sb_scope);
+
+/* ACPI 5.0: Table 7-209 System State Package */
+scope = aml_scope("\\");
+pkg = aml_package(4);
+aml_append(pkg, aml_int(ACPI_GED_SLP_TYP_S5));
+aml_append(pkg, aml_int(0)); /* ignored */
+aml_append(pkg, aml_int(0)); /* reserved */
+aml_append(pkg, aml_int(0)); /* reserved */
+aml_append(scope, aml_name_decl("_S5", pkg));
+aml_append(dsdt, scope);
+
+/* copy AML table into ACPI tables blob and patch header there */
+g_array_append_vals(table_data, dsdt->buf->data, dsdt->buf->len);
+build_header(linker, table_data,
+(void *)(table_data->data + table_data->len - dsdt->buf->len),
+"DSDT", dsdt->buf->len, 2, NULL, NULL);
+free_aml_allocator();
+}
+
+static void acpi_build_microvm(AcpiBuildTables *tables,
+ 

[PATCH v6 11/20] microvm/acpi: use seabios with acpi=on

[Gerd Hoffmann]

With acpi=off continue to use qboot.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Igor Mammedov 
---
 hw/i386/microvm.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
index ca0c9983f137..54510a03f754 100644
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -52,6 +52,7 @@
 #include "hw/xen/start_info.h"
 
 #define MICROVM_QBOOT_FILENAME "qboot.rom"
+#define MICROVM_BIOS_FILENAME  "bios-microvm.bin"
 
 static void microvm_set_rtc(MicrovmMachineState *mms, ISADevice *s)
 {
@@ -174,7 +175,9 @@ static void microvm_devices_init(MicrovmMachineState *mms)
 }
 
 if (bios_name == NULL) {
-bios_name = MICROVM_QBOOT_FILENAME;
+bios_name = x86_machine_is_acpi_enabled(x86ms)
+? MICROVM_BIOS_FILENAME
+: MICROVM_QBOOT_FILENAME;
 }
 x86_bios_rom_init(get_system_memory(), true);
 }
-- 
2.27.0