Re: [PATCH] ninjatool: quote dollars in variables
On 200827 0613, Paolo Bonzini wrote: > Il mer 26 ago 2020, 21:34 Peter Maydell ha > scritto: > > > On Wed, 26 Aug 2020 at 20:03, Paolo Bonzini wrote: > > > > > > Otherwise, dollars (such as in the special $ORIGIN rpath) are > > > eaten by Make. > > > > Incidentally, why are we using rpath anyway? I'm pretty > > sure the old build system didn't need it, and it's one of > > those features I have mentally filed away under "liable > > to confusing and non-portable weirdness"... > > > > It's only done in the build tree, to allow running against uninstalled > shared_library. Installed binaries have no rpath (distros don't want it > anyway). QEMU doesn't need it since it has no shared library yet. > Its an obscure example, but we use it in scripts/oss-fuzz/build.sh to build a qemu-fuzz binary(qemu-system with some bells and whistles) that is portable between containers and can be deployed on oss-fuzz. The other option is to build a static binary, which AFAIK is not supported for softmmu builds. I guess this is a prime example of "confusing and non-portable weirdness". In defense, it wasn't our idea. The oss-fuzz documentation suggests it: https://google.github.io/oss-fuzz/further-reading/fuzzer-environment/#runtime-dependencies > Paolo > > >
[PATCH v2 4/6] configure: Fix include and linkage issue on msys2
From: Yonggang Luo
On msys2, the -I/e/path/to/qemu -L/e/path/to/qemu are not recognized by the
compiler
Cause $PWD are result posix style path such as /e/path/to/qemu that can not be
recognized
by mingw gcc, and `pwd -W` are result Windows style path such as
E:/path/to/qemu that can
be recognized by the mingw gcc. So we replace all $PWD with $build_path that can
building qemu under msys2/mingw environment.
Signed-off-by: Yonggang Luo
---
configure | 28 +++-
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/configure b/configure
index b1e11397a8..3b9e79923d 100755
--- a/configure
+++ b/configure
@@ -13,8 +13,13 @@ export CCACHE_RECACHE=yes
# make source path absolute
source_path=$(cd "$(dirname -- "$0")"; pwd)
+build_path=$PWD
+if [ "$MSYSTEM" = "MINGW64" -o "$MSYSTEM" = "MINGW32" ]; then
+source_path=$(cd "$(dirname -- "$0")"; pwd -W)
+build_path=`pwd -W`
+fi
-if test "$PWD" = "$source_path"
+if test "$build_path" = "$source_path"
then
echo "Using './build' as the directory for build output"
@@ -346,7 +351,12 @@ ld_has() {
$ld --help 2>/dev/null | grep ".$1" >/dev/null 2>&1
}
-if printf %s\\n "$source_path" "$PWD" | grep -q "[[:space:]:]";
+check_valid_build_path="[[:space:]:]"
+if [ "$MSYSTEM" = "MINGW64" -o "$MSYSTEM" = "MINGW32" ]; then
+check_valid_build_path="[[:space:]]"
+fi
+
+if printf %s\\n "$source_path" "$build_path" | grep -q
"$check_valid_build_path";
then
error_exit "main directory cannot contain spaces nor colons"
fi
@@ -942,7 +952,7 @@ Linux)
linux="yes"
linux_user="yes"
kvm="yes"
- QEMU_INCLUDES="-isystem ${source_path}/linux-headers -I$PWD/linux-headers
$QEMU_INCLUDES"
+ QEMU_INCLUDES="-isystem ${source_path}/linux-headers
-I${build_path}/linux-headers $QEMU_INCLUDES"
libudev="yes"
;;
esac
@@ -4283,7 +4293,7 @@ EOF
symlink "$source_path/dtc/Makefile" "dtc/Makefile"
fi
fdt_cflags="-I${source_path}/dtc/libfdt"
- fdt_ldflags="-L$PWD/dtc/libfdt"
+ fdt_ldflags="-L${build_path}/dtc/libfdt"
fdt_libs="$fdt_libs"
elif test "$fdt" = "yes" ; then
# Not a git build & no libfdt found, prompt for system install
@@ -5268,7 +5278,7 @@ case "$capstone" in
else
LIBCAPSTONE=libcapstone.a
fi
-capstone_libs="-L$PWD/capstone -lcapstone"
+capstone_libs="-L${build_path}/capstone -lcapstone"
capstone_cflags="-I${source_path}/capstone/include"
;;
@@ -6268,8 +6278,8 @@ case "$slirp" in
git_submodules="${git_submodules} slirp"
fi
mkdir -p slirp
-slirp_cflags="-I${source_path}/slirp/src -I$PWD/slirp/src"
-slirp_libs="-L$PWD/slirp -lslirp"
+slirp_cflags="-I${source_path}/slirp/src -I${build_path}/slirp/src"
+slirp_libs="-L${build_path}/slirp -lslirp"
if test "$mingw32" = "yes" ; then
slirp_libs="$slirp_libs -lws2_32 -liphlpapi"
fi
@@ -8212,7 +8222,7 @@ fi
mv $cross config-meson.cross
rm -rf meson-private meson-info meson-logs
-NINJA=$PWD/ninjatool $meson setup \
+NINJA="${build_path}/ninjatool" $meson setup \
--prefix "${pre_prefix}$prefix" \
--libdir "${pre_prefix}$libdir" \
--libexecdir "${pre_prefix}$libexecdir" \
@@ -8232,7 +8242,7 @@ NINJA=$PWD/ninjatool $meson setup \
-Dvnc=$vnc -Dvnc_sasl=$vnc_sasl -Dvnc_jpeg=$vnc_jpeg -Dvnc_png=$vnc_png
\
-Dgettext=$gettext -Dxkbcommon=$xkbcommon \
$cross_arg \
-"$PWD" "$source_path"
+"$build_path" "$source_path"
if test "$?" -ne 0 ; then
error_exit "meson setup failed"
--
2.27.0.windows.1
[PATCH v2 6/6] meson: Convert undefsym.sh to undefsym.py
From: Yonggang Luo
undefsym.sh are not msys2 compatible, convert it to python script
Signed-off-by: Yonggang Luo
---
meson.build | 2 +-
scripts/undefsym.py | 57 +
scripts/undefsym.sh | 20
3 files changed, 58 insertions(+), 21 deletions(-)
create mode 100644 scripts/undefsym.py
delete mode 100755 scripts/undefsym.sh
diff --git a/meson.build b/meson.build
index 1644bbd83c..d6e3bcea7e 100644
--- a/meson.build
+++ b/meson.build
@@ -845,7 +845,7 @@ foreach d, list : modules
endforeach
nm = find_program('nm')
-undefsym = find_program('scripts/undefsym.sh')
+undefsym = find_program('scripts/undefsym.py')
block_syms = custom_target('block.syms', output: 'block.syms',
input: [libqemuutil, block_mods],
capture: true,
diff --git a/scripts/undefsym.py b/scripts/undefsym.py
new file mode 100644
index 00..c690f88c7a
--- /dev/null
+++ b/scripts/undefsym.py
@@ -0,0 +1,57 @@
+#!/usr/bin/env python3
+# -*- coding: utf-8 -*-
+
+# Before a shared module's DSO is produced, a static library is built for it
+# and passed to this script. The script generates -Wl,-u options to force
+# the inclusion of symbol from libqemuutil.a if the shared modules need them,
+# This is necessary because the modules may use functions not needed by the
+# executable itself, which would cause the function to not be linked in.
+# Then the DSO loading would fail because of the missing symbol.
+
+
+"""
+Compare the static library with the shared module for compute the symbol
duplication
+"""
+
+import sys
+import subprocess
+
+def filter_lines_set(stdout, is_static = True):
+linesSet = set()
+for line in stdout.splitlines():
+tokens = line.split(b' ')
+if len(tokens) >= 1:
+if len(tokens) > 1:
+if is_static and tokens[1] == b'U':
+continue
+if not is_static and tokens[1] != b'U':
+continue
+new_line = b'-Wl,-u,' + tokens[0]
+if not new_line in linesSet:
+linesSet.add(new_line)
+return linesSet
+
+def main(args):
+if len(args) <= 3:
+sys.exit(0)
+
+nm = args[1]
+staticlib = args[2]
+pc = subprocess.run([nm, "-P", "-g", staticlib], stdout=subprocess.PIPE)
+if pc.returncode != 0:
+sys.exit(-1)
+lines_set_left = filter_lines_set(pc.stdout)
+
+shared_modules = args[3:]
+pc = subprocess.run([nm, "-P", "-g"] + shared_modules,
stdout=subprocess.PIPE)
+if pc.returncode != 0:
+sys.exit(-1)
+lines_set_right = filter_lines_set(pc.stdout, False)
+lines = []
+for line in sorted(list(lines_set_right)):
+if line in lines_set_left:
+lines.append(line)
+sys.stdout.write(b'\n'.join(lines).decode())
+
+if __name__ == "__main__":
+main(sys.argv)
diff --git a/scripts/undefsym.sh b/scripts/undefsym.sh
deleted file mode 100755
index b9ec332e95..00
--- a/scripts/undefsym.sh
+++ /dev/null
@@ -1,20 +0,0 @@
-#! /usr/bin/env bash
-
-# Before a shared module's DSO is produced, a static library is built for it
-# and passed to this script. The script generates -Wl,-u options to force
-# the inclusion of symbol from libqemuutil.a if the shared modules need them,
-# This is necessary because the modules may use functions not needed by the
-# executable itself, which would cause the function to not be linked in.
-# Then the DSO loading would fail because of the missing symbol.
-
-if test $# -le 2; then
- exit 0
-fi
-
-NM=$1
-staticlib=$2
-shift 2
-# Find symbols defined in static libraries and undefined in shared modules
-comm -12 \
- <( $NM -P -g $staticlib | awk '$2!="U"{print "-Wl,-u," $1}' | sort -u) \
- <( $NM -P -g "$@" | awk '$2=="U"{print "-Wl,-u," $1}' | sort -u)
--
2.27.0.windows.1
[PATCH v2 3/6] meson: Mingw64 gcc doesn't recognize system include_type for sdl2
From: Yonggang Luo
Fixes this for msys2/mingw64 by remove the include_type for sdl2 discovery in
meson
Signed-off-by: Yonggang Luo
---
meson.build | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/meson.build b/meson.build
index f0fe5f8799..1644bbd83c 100644
--- a/meson.build
+++ b/meson.build
@@ -224,8 +224,7 @@ 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 = dependency('sdl2', required: get_option('sdl'), static: enable_static)
sdl_image = not_found
if sdl.found()
# work around 2.0.8 bug
--
2.27.0.windows.1
[PATCH v2 1/6] meson: Fixes the ninjatool issue that E$$: are generated in Makefile.ninja
From: Yonggang Luo
SIMPLE_PATH_RE should match the full path token.
Or the $ and : contained in path would not matched if the path are start with
C:/ and E:/
Signed-off-by: Yonggang Luo
---
scripts/ninjatool.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/ninjatool.py b/scripts/ninjatool.py
index cc77d51aa8..6ca8be6f10 100755
--- a/scripts/ninjatool.py
+++ b/scripts/ninjatool.py
@@ -55,7 +55,7 @@ else:
PATH_RE = r"[^$\s:|]+|\$[$ :]|\$[a-zA-Z0-9_-]+|\$\{[a-zA-Z0-9_.-]+\}"
-SIMPLE_PATH_RE = re.compile(r"[^$\s:|]+")
+SIMPLE_PATH_RE = re.compile(r"^[^$\s:|]+$")
IDENT_RE = re.compile(r"[a-zA-Z0-9_.-]+$")
STRING_RE = re.compile(r"(" + PATH_RE + r"|[\s:|])(?:\r?\n)?|.")
TOPLEVEL_RE = re.compile(r"([=:#]|\|\|?|^ +|(?:" + PATH_RE + r")+)\s*|.")
--
2.27.0.windows.1
[PATCH v2 5/6] meson: Fixes ninjatool can not be recognized as script under Window/MSYS2
From: Yonggang Luo
use ninja instead ${build_path}/ninjatool
Signed-off-by: Yonggang Luo
---
configure | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/configure b/configure
index 3b9e79923d..2ad0c58492 100755
--- a/configure
+++ b/configure
@@ -8222,7 +8222,7 @@ fi
mv $cross config-meson.cross
rm -rf meson-private meson-info meson-logs
-NINJA="${build_path}/ninjatool" $meson setup \
+NINJA="ninja" $meson setup \
--prefix "${pre_prefix}$prefix" \
--libdir "${pre_prefix}$libdir" \
--libexecdir "${pre_prefix}$libexecdir" \
--
2.27.0.windows.1
[PATCH v2 2/6] meson: fixes relpath may fail on win32.
From: Yonggang Luo
On win32, os.path.relpath would raise exception when do the following relpath:
C:/msys64/mingw64/x.exe relative to E:/path/qemu-build would fail.
So we try catch it for stopping it from raise exception on msys2
Signed-off-by: Yonggang Luo
---
scripts/mtest2make.py | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/scripts/mtest2make.py b/scripts/mtest2make.py
index bdb257bbd9..d7a51bf97e 100644
--- a/scripts/mtest2make.py
+++ b/scripts/mtest2make.py
@@ -53,9 +53,16 @@ i = 0
for test in json.load(sys.stdin):
env = ' '.join(('%s=%s' % (shlex.quote(k), shlex.quote(v))
for k, v in test['env'].items()))
-executable = os.path.relpath(test['cmd'][0])
+executable = test['cmd'][0]
+try:
+executable = os.path.relpath(executable)
+except:
+pass
if test['workdir'] is not None:
-test['cmd'][0] = os.path.relpath(test['cmd'][0], test['workdir'])
+try:
+test['cmd'][0] = os.path.relpath(executable, test['workdir'])
+except:
+test['cmd'][0] = executable
else:
test['cmd'][0] = executable
cmd = '$(.test.env) %s %s' % (env, ' '.join((shlex.quote(x) for x in
test['cmd'])))
--
2.27.0.windows.1
[PATCH v2 0/6] Fixes msys2 building after the meson convert
From: Yonggang Luo These patch series fixes the building of newest qemu under msys2 environment Yonggang Luo (6): meson: Fixes the ninjatool issue that E$$: are generated in Makefile.ninja meson: fixes relpath may fail on win32. meson: Mingw64 gcc doesn't recognize system include_type for sdl2 configure: Fix include and linkage issue on msys2 meson: Fixes ninjatool can not be recognized as script under Window/MSYS2 meson: Convert undefsym.sh to undefsym.py configure | 28 ++--- meson.build | 5 ++-- scripts/mtest2make.py | 11 +++-- scripts/ninjatool.py | 2 +- scripts/undefsym.py | 57 +++ scripts/undefsym.sh | 20 --- 6 files changed, 88 insertions(+), 35 deletions(-) create mode 100644 scripts/undefsym.py delete mode 100755 scripts/undefsym.sh -- 2.27.0.windows.1
Re: [PATCH] meson: move pixman detection to meson
On 26/08/2020 09.02, Paolo Bonzini wrote:
> 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
The "else" part now got completely lost, didn't it? pixman is required
for building the softmmu targets, so we should error out if configure is
run with a softmmu target on systems where pixman is not installed.
Maybe you can simply replace the above check with:
if test "$softmmu" = "yes" && ! $pkg_config --atleast-version=0.21.8
pixman-1 > /dev/null 2>&1; then
error_exit "pixman >= 0.21.8 not present." \
"Please install the pixman devel package."
fi
?
Thomas
> ##
> # 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
[PATCH 5/6] meson: Fixes ninjatool can not be recognized as script under Window/MSYS2
From: Yonggang Luo
use ninja instead ${build_path}/ninjatool
Signed-off-by: Yonggang Luo
---
configure | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/configure b/configure
index 3b9e79923d..2ad0c58492 100755
--- a/configure
+++ b/configure
@@ -8222,7 +8222,7 @@ fi
mv $cross config-meson.cross
rm -rf meson-private meson-info meson-logs
-NINJA="${build_path}/ninjatool" $meson setup \
+NINJA="ninja" $meson setup \
--prefix "${pre_prefix}$prefix" \
--libdir "${pre_prefix}$libdir" \
--libexecdir "${pre_prefix}$libexecdir" \
--
2.27.0.windows.1
[PATCH 2/6] meson: fixes relpath may fail on win32.
From: Yonggang Luo
On win32, os.path.relpath would raise exception when do the following relpath:
C:/msys64/mingw64/x.exe relative to E:/path/qemu-build would fail.
So we try catch it for stopping it from raise exception on msys2
Signed-off-by: Yonggang Luo
---
scripts/mtest2make.py | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/scripts/mtest2make.py b/scripts/mtest2make.py
index bdb257bbd9..d7a51bf97e 100644
--- a/scripts/mtest2make.py
+++ b/scripts/mtest2make.py
@@ -53,9 +53,16 @@ i = 0
for test in json.load(sys.stdin):
env = ' '.join(('%s=%s' % (shlex.quote(k), shlex.quote(v))
for k, v in test['env'].items()))
-executable = os.path.relpath(test['cmd'][0])
+executable = test['cmd'][0]
+try:
+executable = os.path.relpath(executable)
+except:
+pass
if test['workdir'] is not None:
-test['cmd'][0] = os.path.relpath(test['cmd'][0], test['workdir'])
+try:
+test['cmd'][0] = os.path.relpath(executable, test['workdir'])
+except:
+test['cmd'][0] = executable
else:
test['cmd'][0] = executable
cmd = '$(.test.env) %s %s' % (env, ' '.join((shlex.quote(x) for x in
test['cmd'])))
--
2.27.0.windows.1
[PATCH 6/6] meson: Convert undefsym.sh to undefsym.py
From: Yonggang Luo
undefsym.sh are not msys2 compatible, convert it to python script
Signed-off-by: Yonggang Luo
---
meson.build | 2 +-
scripts/undefsym.py | 57 +
scripts/undefsym.sh | 20
3 files changed, 58 insertions(+), 21 deletions(-)
create mode 100644 scripts/undefsym.py
delete mode 100755 scripts/undefsym.sh
diff --git a/meson.build b/meson.build
index 1644bbd83c..d6e3bcea7e 100644
--- a/meson.build
+++ b/meson.build
@@ -845,7 +845,7 @@ foreach d, list : modules
endforeach
nm = find_program('nm')
-undefsym = find_program('scripts/undefsym.sh')
+undefsym = find_program('scripts/undefsym.py')
block_syms = custom_target('block.syms', output: 'block.syms',
input: [libqemuutil, block_mods],
capture: true,
diff --git a/scripts/undefsym.py b/scripts/undefsym.py
new file mode 100644
index 00..c690f88c7a
--- /dev/null
+++ b/scripts/undefsym.py
@@ -0,0 +1,57 @@
+#!/usr/bin/env python3
+# -*- coding: utf-8 -*-
+
+# Before a shared module's DSO is produced, a static library is built for it
+# and passed to this script. The script generates -Wl,-u options to force
+# the inclusion of symbol from libqemuutil.a if the shared modules need them,
+# This is necessary because the modules may use functions not needed by the
+# executable itself, which would cause the function to not be linked in.
+# Then the DSO loading would fail because of the missing symbol.
+
+
+"""
+Compare the static library with the shared module for compute the symbol
duplication
+"""
+
+import sys
+import subprocess
+
+def filter_lines_set(stdout, is_static = True):
+linesSet = set()
+for line in stdout.splitlines():
+tokens = line.split(b' ')
+if len(tokens) >= 1:
+if len(tokens) > 1:
+if is_static and tokens[1] == b'U':
+continue
+if not is_static and tokens[1] != b'U':
+continue
+new_line = b'-Wl,-u,' + tokens[0]
+if not new_line in linesSet:
+linesSet.add(new_line)
+return linesSet
+
+def main(args):
+if len(args) <= 3:
+sys.exit(0)
+
+nm = args[1]
+staticlib = args[2]
+pc = subprocess.run([nm, "-P", "-g", staticlib], stdout=subprocess.PIPE)
+if pc.returncode != 0:
+sys.exit(-1)
+lines_set_left = filter_lines_set(pc.stdout)
+
+shared_modules = args[3:]
+pc = subprocess.run([nm, "-P", "-g"] + shared_modules,
stdout=subprocess.PIPE)
+if pc.returncode != 0:
+sys.exit(-1)
+lines_set_right = filter_lines_set(pc.stdout, False)
+lines = []
+for line in sorted(list(lines_set_right)):
+if line in lines_set_left:
+lines.append(line)
+sys.stdout.write(b'\n'.join(lines).decode())
+
+if __name__ == "__main__":
+main(sys.argv)
diff --git a/scripts/undefsym.sh b/scripts/undefsym.sh
deleted file mode 100755
index b9ec332e95..00
--- a/scripts/undefsym.sh
+++ /dev/null
@@ -1,20 +0,0 @@
-#! /usr/bin/env bash
-
-# Before a shared module's DSO is produced, a static library is built for it
-# and passed to this script. The script generates -Wl,-u options to force
-# the inclusion of symbol from libqemuutil.a if the shared modules need them,
-# This is necessary because the modules may use functions not needed by the
-# executable itself, which would cause the function to not be linked in.
-# Then the DSO loading would fail because of the missing symbol.
-
-if test $# -le 2; then
- exit 0
-fi
-
-NM=$1
-staticlib=$2
-shift 2
-# Find symbols defined in static libraries and undefined in shared modules
-comm -12 \
- <( $NM -P -g $staticlib | awk '$2!="U"{print "-Wl,-u," $1}' | sort -u) \
- <( $NM -P -g "$@" | awk '$2=="U"{print "-Wl,-u," $1}' | sort -u)
--
2.27.0.windows.1
[PATCH 3/6] meson: Mingw64 gcc doesn't recognize system include_type for sdl2
From: Yonggang Luo
Fixes this for msys2/mingw64 by remove the include_type for sdl2 discovery in
meson
Signed-off-by: Yonggang Luo
---
meson.build | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/meson.build b/meson.build
index f0fe5f8799..1644bbd83c 100644
--- a/meson.build
+++ b/meson.build
@@ -224,8 +224,7 @@ 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 = dependency('sdl2', required: get_option('sdl'), static: enable_static)
sdl_image = not_found
if sdl.found()
# work around 2.0.8 bug
--
2.27.0.windows.1
[PATCH 4/6] configure: Fix include and linkage issue on msys2
From: Yonggang Luo
On msys2, the -I/e/path/to/qemu -L/e/path/to/qemu are not recognized by the
compiler
Cause $PWD are result posix style path such as /e/path/to/qemu that can not be
recognized
by mingw gcc, and `pwd -W` are result Windows style path such as
E:/path/to/qemu that can
be recognized by the mingw gcc. So we replace all $PWD with $build_path that can
building qemu under msys2/mingw environment.
Signed-off-by: Yonggang Luo
---
configure | 28 +++-
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/configure b/configure
index b1e11397a8..3b9e79923d 100755
--- a/configure
+++ b/configure
@@ -13,8 +13,13 @@ export CCACHE_RECACHE=yes
# make source path absolute
source_path=$(cd "$(dirname -- "$0")"; pwd)
+build_path=$PWD
+if [ "$MSYSTEM" = "MINGW64" -o "$MSYSTEM" = "MINGW32" ]; then
+source_path=$(cd "$(dirname -- "$0")"; pwd -W)
+build_path=`pwd -W`
+fi
-if test "$PWD" = "$source_path"
+if test "$build_path" = "$source_path"
then
echo "Using './build' as the directory for build output"
@@ -346,7 +351,12 @@ ld_has() {
$ld --help 2>/dev/null | grep ".$1" >/dev/null 2>&1
}
-if printf %s\\n "$source_path" "$PWD" | grep -q "[[:space:]:]";
+check_valid_build_path="[[:space:]:]"
+if [ "$MSYSTEM" = "MINGW64" -o "$MSYSTEM" = "MINGW32" ]; then
+check_valid_build_path="[[:space:]]"
+fi
+
+if printf %s\\n "$source_path" "$build_path" | grep -q
"$check_valid_build_path";
then
error_exit "main directory cannot contain spaces nor colons"
fi
@@ -942,7 +952,7 @@ Linux)
linux="yes"
linux_user="yes"
kvm="yes"
- QEMU_INCLUDES="-isystem ${source_path}/linux-headers -I$PWD/linux-headers
$QEMU_INCLUDES"
+ QEMU_INCLUDES="-isystem ${source_path}/linux-headers
-I${build_path}/linux-headers $QEMU_INCLUDES"
libudev="yes"
;;
esac
@@ -4283,7 +4293,7 @@ EOF
symlink "$source_path/dtc/Makefile" "dtc/Makefile"
fi
fdt_cflags="-I${source_path}/dtc/libfdt"
- fdt_ldflags="-L$PWD/dtc/libfdt"
+ fdt_ldflags="-L${build_path}/dtc/libfdt"
fdt_libs="$fdt_libs"
elif test "$fdt" = "yes" ; then
# Not a git build & no libfdt found, prompt for system install
@@ -5268,7 +5278,7 @@ case "$capstone" in
else
LIBCAPSTONE=libcapstone.a
fi
-capstone_libs="-L$PWD/capstone -lcapstone"
+capstone_libs="-L${build_path}/capstone -lcapstone"
capstone_cflags="-I${source_path}/capstone/include"
;;
@@ -6268,8 +6278,8 @@ case "$slirp" in
git_submodules="${git_submodules} slirp"
fi
mkdir -p slirp
-slirp_cflags="-I${source_path}/slirp/src -I$PWD/slirp/src"
-slirp_libs="-L$PWD/slirp -lslirp"
+slirp_cflags="-I${source_path}/slirp/src -I${build_path}/slirp/src"
+slirp_libs="-L${build_path}/slirp -lslirp"
if test "$mingw32" = "yes" ; then
slirp_libs="$slirp_libs -lws2_32 -liphlpapi"
fi
@@ -8212,7 +8222,7 @@ fi
mv $cross config-meson.cross
rm -rf meson-private meson-info meson-logs
-NINJA=$PWD/ninjatool $meson setup \
+NINJA="${build_path}/ninjatool" $meson setup \
--prefix "${pre_prefix}$prefix" \
--libdir "${pre_prefix}$libdir" \
--libexecdir "${pre_prefix}$libexecdir" \
@@ -8232,7 +8242,7 @@ NINJA=$PWD/ninjatool $meson setup \
-Dvnc=$vnc -Dvnc_sasl=$vnc_sasl -Dvnc_jpeg=$vnc_jpeg -Dvnc_png=$vnc_png
\
-Dgettext=$gettext -Dxkbcommon=$xkbcommon \
$cross_arg \
-"$PWD" "$source_path"
+"$build_path" "$source_path"
if test "$?" -ne 0 ; then
error_exit "meson setup failed"
--
2.27.0.windows.1
[PATCH 1/6] meson: Fixes the ninjatool issue that E$$: are generated in Makefile.ninja
From: Yonggang Luo
SIMPLE_PATH_RE should match the full path token.
Or the $ and : contained in path would not matched if the path are start with
C:/ and E:/
Signed-off-by: Yonggang Luo
---
scripts/ninjatool.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/ninjatool.py b/scripts/ninjatool.py
index cc77d51aa8..6ca8be6f10 100755
--- a/scripts/ninjatool.py
+++ b/scripts/ninjatool.py
@@ -55,7 +55,7 @@ else:
PATH_RE = r"[^$\s:|]+|\$[$ :]|\$[a-zA-Z0-9_-]+|\$\{[a-zA-Z0-9_.-]+\}"
-SIMPLE_PATH_RE = re.compile(r"[^$\s:|]+")
+SIMPLE_PATH_RE = re.compile(r"^[^$\s:|]+$")
IDENT_RE = re.compile(r"[a-zA-Z0-9_.-]+$")
STRING_RE = re.compile(r"(" + PATH_RE + r"|[\s:|])(?:\r?\n)?|.")
TOPLEVEL_RE = re.compile(r"([=:#]|\|\|?|^ +|(?:" + PATH_RE + r")+)\s*|.")
--
2.27.0.windows.1
[PATCH 0/6] *** Fixes msys2 building after the meson convert ***
From: Yonggang Luo msys2 environment *** BLURB HERE *** Yonggang Luo (6): meson: Fixes the ninjatool issue that E$$: are generated in Makefile.ninja meson: fixes relpath may fail on win32. meson: Mingw64 gcc doesn't recognize system include_type for sdl2 configure: Fix include and linkage issue on msys2 meson: Fixes ninjatool can not be recognized as script under Window/MSYS2 meson: Convert undefsym.sh to undefsym.py configure | 28 ++--- meson.build | 5 ++-- scripts/mtest2make.py | 11 +++-- scripts/ninjatool.py | 2 +- scripts/undefsym.py | 57 +++ scripts/undefsym.sh | 20 --- 6 files changed, 88 insertions(+), 35 deletions(-) create mode 100644 scripts/undefsym.py delete mode 100755 scripts/undefsym.sh -- 2.27.0.windows.1
Re: [PATCH 4/8] sclpconsole: Use TYPE_* constants
On 26/08/2020 20.43, Eduardo Habkost wrote:
> This will make future conversion to use OBJECT_DECLARE* easier.
>
> Signed-off-by: Eduardo Habkost
> ---
> Cc: Cornelia Huck
> Cc: Halil Pasic
> Cc: Christian Borntraeger
> Cc: Thomas Huth
> Cc: "Marc-André Lureau"
> Cc: Paolo Bonzini
> Cc: qemu-s3...@nongnu.org
> Cc: qemu-devel@nongnu.org
> ---
> hw/char/sclpconsole-lm.c | 2 +-
> hw/char/sclpconsole.c| 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/char/sclpconsole-lm.c b/hw/char/sclpconsole-lm.c
> index 2b5f37b6a2..5848b4e9c5 100644
> --- a/hw/char/sclpconsole-lm.c
> +++ b/hw/char/sclpconsole-lm.c
> @@ -355,7 +355,7 @@ static void console_class_init(ObjectClass *klass, void
> *data)
> }
>
> static const TypeInfo sclp_console_info = {
> -.name = "sclplmconsole",
> +.name = TYPE_SCLPLM_CONSOLE,
> .parent= TYPE_SCLP_EVENT,
> .instance_size = sizeof(SCLPConsoleLM),
> .class_init= console_class_init,
> diff --git a/hw/char/sclpconsole.c b/hw/char/sclpconsole.c
> index 5c7664905e..d6f7da0818 100644
> --- a/hw/char/sclpconsole.c
> +++ b/hw/char/sclpconsole.c
> @@ -271,7 +271,7 @@ static void console_class_init(ObjectClass *klass, void
> *data)
> }
>
> static const TypeInfo sclp_console_info = {
> -.name = "sclpconsole",
> +.name = TYPE_SCLP_CONSOLE,
> .parent= TYPE_SCLP_EVENT,
> .instance_size = sizeof(SCLPConsole),
> .class_init= console_class_init,
>
Reviewed-by: Thomas Huth
Re: Contributor wanting to get started with simple contributions
On 26/08/2020 17.00, Rohit Shinde wrote: > Hey Thomas, > > I didn't really have any specific questions. I wanted to know if there > was any part of qemu that I could contribute to. Qemu is overwhelmingly > vast and without some pointers, I felt very lost. Ok, that's true - QEMU is really a huge project. I'd really recommend to pick some topics from https://wiki.qemu.org/Contribute/BiteSizedTasks first to get a feeling for contributing patches to QEMU. Since you're interested in emulation, maybe the topics from the "Device models" section would also be a good fit? > > I plan to stay and become a long term contributor. Is there any CS > > What does "CS" stand for? > > Computer Science :) Oh, well, thanks, ok, that was too easy. I guess there are just too many abbreviations around ;-) > > > theory that I would need to know other than what I mentioned > above? I'd recommend to browse the various KVM forum presentations on http://www.linux-kvm.org/page/Category:Conferences to see if there is something that catches your eye. You can find the recordings of most presentations on https://www.youtube.com/channel/UCRCSQmAOh7yzgheq-emy1xA , too. > > Is it possible to "learn on the go"? > > You certainly have to "learn on the go", since it is likely quite > impossible to grasp a huge project like QEMU at once. > > I am interested in contributing to something like device emulation. > There might be lots of devices which Qemu might want to emulate but > which haven't yet been emulated. Sure, but I think you first need a target you're interested in first. E.g. do you want to focus on x86, ARM, PPC, m68k, ... ? Depending on that, you can start looking around in the hw/ directory for "missing" or "TODO" items. Thomas
Re: [PATCH] ninjatool: quote dollars in variables
Il mer 26 ago 2020, 21:34 Peter Maydell ha scritto: > On Wed, 26 Aug 2020 at 20:03, Paolo Bonzini wrote: > > > > Otherwise, dollars (such as in the special $ORIGIN rpath) are > > eaten by Make. > > Incidentally, why are we using rpath anyway? I'm pretty > sure the old build system didn't need it, and it's one of > those features I have mentally filed away under "liable > to confusing and non-portable weirdness"... > It's only done in the build tree, to allow running against uninstalled shared_library. Installed binaries have no rpath (distros don't want it anyway). QEMU doesn't need it since it has no shared library yet. Paolo >
Re: [RFC PATCH v3 24/34] Hexagon (target/hexagon) opcode data structures
On 8/26/20 4:52 PM, Taylor Simpson wrote: >> And using qemu/bitops.h if possible, as discussed earlier vs attribs.h. > > Do you mean replace the GET_ATTRIB macro with test_bit from qemu/bitops.h? No, just define GET_ATTRIB in terms of test_bit, and define opcode_attribs using BITS_TO_LONGS. r~
[PATCH v3] virtio-gpu: fix unmap the already mapped items
we go here either (!(*iov)[i].iov_base) or (len != l), so we need to consider
to unmap the 'i'th item as well when the 'i'th item is not nil
CC: Li Qiang
Signed-off-by: Li Zhijian
---
v2: address Gerd's comments
v3: leave (*iov)[i].iov_len as the real mapped len (Li Qiang)
---
hw/display/virtio-gpu.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 5f0dd7c150..90be4e3ed7 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -646,9 +646,9 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
uint64_t a = le64_to_cpu(ents[i].addr);
uint32_t l = le32_to_cpu(ents[i].length);
hwaddr len = l;
-(*iov)[i].iov_len = l;
(*iov)[i].iov_base = dma_memory_map(VIRTIO_DEVICE(g)->dma_as,
a, , DMA_DIRECTION_TO_DEVICE);
+(*iov)[i].iov_len = len;
if (addr) {
(*addr)[i] = a;
}
@@ -656,6 +656,9 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map MMIO memory for"
" resource %d element %d\n",
__func__, ab->resource_id, i);
+if ((*iov)[i].iov_base) {
+i++; /* cleanup the 'i'th map */
+}
virtio_gpu_cleanup_mapping_iov(g, *iov, i);
g_free(ents);
*iov = NULL;
--
2.28.0
Re: [PATCH v2] virtio-gpu: fix unmap the already mapped items
On 8/26/20 10:54 PM, Li Qiang wrote:
Li Zhijian 于2020年8月21日周五 下午7:34写道：
we go here either (!(*iov)[i].iov_base) or (len != l), so we need to consider
to unmap the 'i'th item as well when the 'i'th item is not nil
Signed-off-by: Li Zhijian
---
v2: address Gerd's comments
---
hw/display/virtio-gpu.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 5f0dd7c150..e93f99932a 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -656,6 +656,9 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map MMIO memory for"
" resource %d element %d\n",
__func__, ab->resource_id, i);
+if ((*iov)[i].iov_base) {
+i++; /* cleanup the 'i'th map */
Should we also reset (*iov)[i].iov_len to 'len' so the
dma_memory_unmap has the right size?
Indeed, good caught, thanks
Thanks,
Li Qiang
+}
virtio_gpu_cleanup_mapping_iov(g, *iov, i);
g_free(ents);
*iov = NULL;
--
2.17.1
Re: [PATCH 08/12] migration/colo: Plug memleaks in colo_process_incoming_thread
On 2020/8/26 20:37, Li Qiang wrote:
> Pan Nengyuan 于2020年8月14日周五 下午6:52写道：
>>
>> 'local_err' forgot to free in colo_process_incoming_thread error path.
>> Fix that.
>>
>> Reported-by: Euler Robot
>> Signed-off-by: Pan Nengyuan
>> ---
>> Cc: Hailiang Zhang
>> Cc: Juan Quintela
>> Cc: "Dr. David Alan Gilbert"
>> ---
>> migration/colo.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/migration/colo.c b/migration/colo.c
>> index ea7d1e9d4e..17b5afc6b5 100644
>> --- a/migration/colo.c
>> +++ b/migration/colo.c
>> @@ -870,6 +870,7 @@ void *colo_process_incoming_thread(void *opaque)
>> replication_start_all(REPLICATION_MODE_SECONDARY, _err);
>> if (local_err) {
>> qemu_mutex_unlock_iothread();
>> +error_report_err(local_err);
>> goto out;
>> }
>> #else
>> @@ -882,6 +883,7 @@ void *colo_process_incoming_thread(void *opaque)
>> colo_send_message(mis->to_src_file, COLO_MESSAGE_CHECKPOINT_READY,
>>_err);
>> if (local_err) {
>> +error_report_err(local_err);
>> goto out;
>> }
>>
>
> Could we arrange 'error_report_err' in 'out' label?
> Like this:
>
> if (local_err) {
> error_report_err(local_err);
> }
Similar to the other place in the same function, I didn't arrange them in 'out'
label:
while (mis->state == MIGRATION_STATUS_COLO) {
colo_wait_handle_message(mis, fb, bioc, _err);
if (local_err) {
error_report_err(local_err);
break;
}
But I think it's a good idea to arrange them in 'out' label. I will change it.
Thanks.
>
> Thanks,
> Li Qiang
>
>
>
>> --
>> 2.18.2
>>
>>
> .
>
Re: [PATCH] meson: Mingw64 gcc doesn't recognize system include_type for sdl2
Patchew URL: https://patchew.org/QEMU/20200825001649.1811-1-luoyongg...@gmail.com/ Hi, This series seems to have some coding style problems. See output below for more information: Type: series Message-id: 20200825001649.1811-1-luoyongg...@gmail.com Subject: [PATCH] meson: Mingw64 gcc doesn't recognize system include_type for sdl2 === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 From https://github.com/patchew-project/qemu - [tag update] patchew/20200826184334.4120620-1-ehabk...@redhat.com -> patchew/20200826184334.4120620-1-ehabk...@redhat.com Switched to a new branch 'test' 700cdc8 meson: Mingw64 gcc doesn't recognize system include_type for sdl2 === OUTPUT BEGIN === ERROR: Missing Signed-off-by: line(s) total: 1 errors, 0 warnings, 9 lines checked Commit 700cdc84f4ae (meson: Mingw64 gcc doesn't recognize system include_type for sdl2) has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. === OUTPUT END === Test command exited with code: 1 The full log is available at http://patchew.org/logs/20200825001649.1811-1-luoyongg...@gmail.com/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-de...@redhat.com
Re: [RFC PATCH v3 2/2] hw/riscv: sifive_u: Add write-once protection.
On Wed, Aug 19, 2020 at 1:09 AM Green Wan wrote:
>
> Add array to store the 'written' status for all bit of OTP to block
> the write operation to the same bit. Ignore the control register
> offset from 0x0 to 0x38 of OTP memory mapping.
nits: please remove the ending period in the commit title
>
> Signed-off-by: Green Wan
> ---
> hw/riscv/sifive_u_otp.c | 21 +
> include/hw/riscv/sifive_u_otp.h | 1 +
> 2 files changed, 22 insertions(+)
>
> diff --git a/hw/riscv/sifive_u_otp.c b/hw/riscv/sifive_u_otp.c
> index 4552b2409e..3a25652735 100644
> --- a/hw/riscv/sifive_u_otp.c
> +++ b/hw/riscv/sifive_u_otp.c
> @@ -27,6 +27,12 @@
> #include "sysemu/blockdev.h"
> #include "sysemu/block-backend.h"
>
> +#define SET_WRITTEN_BIT(map, idx, bit)\
> +(map[idx] |= (0x1 << bit))
> +
> +#define GET_WRITTEN_BIT(map, idx, bit)\
> +((map[idx] >> bit) & 0x1)
> +
> static uint64_t sifive_u_otp_read(void *opaque, hwaddr addr, unsigned int
> size)
> {
> SiFiveUOTPState *s = opaque;
> @@ -135,6 +141,18 @@ static void sifive_u_otp_write(void *opaque, hwaddr addr,
> s->ptrim = val32;
> break;
> case SIFIVE_U_OTP_PWE:
> +/* Keep written state for data only and PWE is enabled. Ignore PAS=1
> */
> +if ((s->pa > SIFIVE_U_OTP_PWE) && (val32 & 0x1) && !s->pas) {
> +if (GET_WRITTEN_BIT(s->fuse_wo, s->pa, s->paio)) {
> +qemu_log_mask(LOG_GUEST_ERROR,
> + "Error: write idx<%u>, bit<%u>\n",
> + s->pa, s->paio);
> +break;
> +} else {
> +SET_WRITTEN_BIT(s->fuse_wo, s->pa, s->paio);
Shouldn't the write operation below be moved to this else branch?
> +}
> +}
> +
> /* write to backend */
> if (s->blk) {
> blk_pwrite(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD, ,
> @@ -215,6 +233,9 @@ static void sifive_u_otp_reset(DeviceState *dev)
> /* Make a valid content of serial number */
> s->fuse[SIFIVE_U_OTP_SERIAL_ADDR] = s->serial;
> s->fuse[SIFIVE_U_OTP_SERIAL_ADDR + 1] = ~(s->serial);
> +
> +/* Initialize write-once map */
> +memset(s->fuse_wo, 0x00, sizeof(s->fuse_wo));
> }
>
> static void sifive_u_otp_class_init(ObjectClass *klass, void *data)
> diff --git a/include/hw/riscv/sifive_u_otp.h b/include/hw/riscv/sifive_u_otp.h
> index dea1df6f6c..ab6e46b013 100644
> --- a/include/hw/riscv/sifive_u_otp.h
> +++ b/include/hw/riscv/sifive_u_otp.h
> @@ -74,6 +74,7 @@ typedef struct SiFiveUOTPState {
> uint32_t ptrim;
> uint32_t pwe;
> uint32_t fuse[SIFIVE_U_OTP_NUM_FUSES];
> +uint32_t fuse_wo[SIFIVE_U_OTP_NUM_FUSES];
> /* config */
> uint32_t serial;
> BlockBackend *blk;
> --
Regards,
Bin
Re: oslib-posix：Fix handle fd leak in qemu_write_pidfile()
On 2020/8/26 18:18, Daniel P. Berrangé wrote: > 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. > OK, I got it, thanks. Thanks Alex
Re: [RFC PATCH v3 1/2] hw/riscv: sifive_u: Add backend drive support
On Wed, Aug 19, 2020 at 1:09 AM Green Wan wrote:
>
> Add '-drive' support to OTP device. Allow users to assign a raw file
> as OTP image.
>
> Signed-off-by: Green Wan
> ---
> hw/riscv/sifive_u_otp.c | 50 +
> include/hw/riscv/sifive_u_otp.h | 2 ++
> 2 files changed, 52 insertions(+)
>
> diff --git a/hw/riscv/sifive_u_otp.c b/hw/riscv/sifive_u_otp.c
> index f6ecbaa2ca..4552b2409e 100644
> --- a/hw/riscv/sifive_u_otp.c
> +++ b/hw/riscv/sifive_u_otp.c
> @@ -24,6 +24,8 @@
> #include "qemu/log.h"
> #include "qemu/module.h"
> #include "hw/riscv/sifive_u_otp.h"
> +#include "sysemu/blockdev.h"
> +#include "sysemu/block-backend.h"
>
> static uint64_t sifive_u_otp_read(void *opaque, hwaddr addr, unsigned int
> size)
> {
> @@ -46,6 +48,16 @@ static uint64_t sifive_u_otp_read(void *opaque, hwaddr
> addr, unsigned int size)
> if ((s->pce & SIFIVE_U_OTP_PCE_EN) &&
> (s->pdstb & SIFIVE_U_OTP_PDSTB_EN) &&
> (s->ptrim & SIFIVE_U_OTP_PTRIM_EN)) {
> +
> +/* read from backend*/
nits: need a space before */
> +if (s->blk) {
> +int32_t buf;
> +
> +blk_pread(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD, ,
> + SIFIVE_U_OTP_FUSE_WORD);
> +return buf;
> +}
> +
> return s->fuse[s->pa & SIFIVE_U_OTP_PA_MASK];
> } else {
> return 0xff;
> @@ -123,6 +135,12 @@ static void sifive_u_otp_write(void *opaque, hwaddr addr,
> s->ptrim = val32;
> break;
> case SIFIVE_U_OTP_PWE:
> +/* write to backend */
> +if (s->blk) {
> +blk_pwrite(s->blk, s->pa * SIFIVE_U_OTP_FUSE_WORD, ,
> + SIFIVE_U_OTP_FUSE_WORD, 0);
> +}
> +
> s->pwe = val32;
> break;
> default:
> @@ -143,16 +161,48 @@ static const MemoryRegionOps sifive_u_otp_ops = {
>
> static Property sifive_u_otp_properties[] = {
> DEFINE_PROP_UINT32("serial", SiFiveUOTPState, serial, 0),
> +DEFINE_PROP_DRIVE("drive", SiFiveUOTPState, blk),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> static void sifive_u_otp_realize(DeviceState *dev, Error **errp)
> {
> SiFiveUOTPState *s = SIFIVE_U_OTP(dev);
> +DriveInfo *dinfo;
>
> memory_region_init_io(>mmio, OBJECT(dev), _u_otp_ops, s,
>TYPE_SIFIVE_U_OTP, SIFIVE_U_OTP_REG_SIZE);
> sysbus_init_mmio(SYS_BUS_DEVICE(dev), >mmio);
> +
> +dinfo = drive_get_next(IF_NONE);
> +if (dinfo) {
> +int ret;
> +uint64_t perm;
> +int filesize;
> +BlockBackend *blk;
nits: keep one space in between
> +
> +blk = blk_by_legacy_dinfo(dinfo);
> +filesize = SIFIVE_U_OTP_NUM_FUSES * SIFIVE_U_OTP_FUSE_WORD;
> +if (blk_getlength(blk) < filesize) {
> +qemu_log_mask(LOG_GUEST_ERROR, "OTP drive size < 16K\n");
> +return;
> +}
> +
> +qdev_prop_set_drive(dev, "drive", blk_by_legacy_dinfo(dinfo));
Use blk for blk_by_legacy_dinfo(dinfo)
> +
> +perm = BLK_PERM_CONSISTENT_READ |
> +(blk_is_read_only(s->blk) ? 0 : BLK_PERM_WRITE);
> +ret = blk_set_perm(s->blk, perm, BLK_PERM_ALL, errp);
> +if (ret < 0) {
> +qemu_log_mask(LOG_GUEST_ERROR, "set perm error.");
> +}
> +
> +if (blk_pread(s->blk, 0, s->fuse, filesize) != filesize) {
> +qemu_log_mask(LOG_GUEST_ERROR,
> + "failed to read the initial flash content");
> +return;
> +}
> +}
> }
>
> static void sifive_u_otp_reset(DeviceState *dev)
> diff --git a/include/hw/riscv/sifive_u_otp.h b/include/hw/riscv/sifive_u_otp.h
> index 639297564a..dea1df6f6c 100644
> --- a/include/hw/riscv/sifive_u_otp.h
> +++ b/include/hw/riscv/sifive_u_otp.h
> @@ -43,6 +43,7 @@
>
> #define SIFIVE_U_OTP_PA_MASK0xfff
> #define SIFIVE_U_OTP_NUM_FUSES 0x1000
> +#define SIFIVE_U_OTP_FUSE_WORD 4
> #define SIFIVE_U_OTP_SERIAL_ADDR0xfc
>
> #define SIFIVE_U_OTP_REG_SIZE 0x1000
> @@ -75,6 +76,7 @@ typedef struct SiFiveUOTPState {
> uint32_t fuse[SIFIVE_U_OTP_NUM_FUSES];
> /* config */
> uint32_t serial;
> +BlockBackend *blk;
nits: keep one space in between
> } SiFiveUOTPState;
>
> #endif /* HW_SIFIVE_U_OTP_H */
> --
Regards,
Bin
Re: [PATCH 8/8] dc390: Use TYPE_DC390_DEVICE constant
Eduardo Habkost 于2020年8月27日周四 上午2:44写道：
>
> This will make future conversion to use OBJECT_DECLARE* easier.
>
> Signed-off-by: Eduardo Habkost
Reviewed-by: Li Qiang
> ---
> Cc: Paolo Bonzini
> Cc: Fam Zheng
> Cc: qemu-devel@nongnu.org
> ---
> hw/scsi/esp-pci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
> index 497a8d5901..90432ef107 100644
> --- a/hw/scsi/esp-pci.c
> +++ b/hw/scsi/esp-pci.c
> @@ -521,7 +521,7 @@ static void dc390_class_init(ObjectClass *klass, void
> *data)
> }
>
> static const TypeInfo dc390_info = {
> -.name = "dc390",
> +.name = TYPE_DC390_DEVICE,
> .parent = TYPE_AM53C974_DEVICE,
> .instance_size = sizeof(DC390State),
> .class_init = dc390_class_init,
> --
> 2.26.2
>
>
Re: [PATCH 6/8] tosa: Use TYPE_TOSA_MISC_GPIO constant
Eduardo Habkost 于2020年8月27日周四 上午2:50写道：
>
> This will make future conversion to use OBJECT_DECLARE* easier.
>
> Signed-off-by: Eduardo Habkost
Reviewed-by: Li Qiang
> ---
> Cc: Andrzej Zaborowski
> Cc: Peter Maydell
> Cc: qemu-...@nongnu.org
> Cc: qemu-devel@nongnu.org
> ---
> hw/arm/tosa.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/arm/tosa.c b/hw/arm/tosa.c
> index e29566f7b3..90eef1f14d 100644
> --- a/hw/arm/tosa.c
> +++ b/hw/arm/tosa.c
> @@ -316,7 +316,7 @@ static const TypeInfo tosa_ssp_info = {
> };
>
> static const TypeInfo tosa_misc_gpio_info = {
> -.name = "tosa-misc-gpio",
> +.name = TYPE_TOSA_MISC_GPIO,
> .parent= TYPE_SYS_BUS_DEVICE,
> .instance_size = sizeof(TosaMiscGPIOState),
> .instance_init = tosa_misc_gpio_init,
> --
> 2.26.2
>
>
Re: [PATCH 7/8] ppce500: Use TYPE_PPC_E500_PCI_BRIDGE constant
Eduardo Habkost 于2020年8月27日周四 上午2:51写道：
>
> This will make future conversion to use OBJECT_DECLARE* easier.
>
> Signed-off-by: Eduardo Habkost
Reviewed-by: Li Qiang
> ---
> Cc: David Gibson
> Cc: qemu-...@nongnu.org
> Cc: qemu-devel@nongnu.org
> ---
> hw/pci-host/ppce500.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
> index d71072731d..1a62b2f8cc 100644
> --- a/hw/pci-host/ppce500.c
> +++ b/hw/pci-host/ppce500.c
> @@ -509,7 +509,7 @@ static void e500_host_bridge_class_init(ObjectClass
> *klass, void *data)
> }
>
> static const TypeInfo e500_host_bridge_info = {
> -.name = "e500-host-bridge",
> +.name = TYPE_PPC_E500_PCI_BRIDGE,
> .parent= TYPE_PCI_DEVICE,
> .instance_size = sizeof(PPCE500PCIBridgeState),
> .class_init= e500_host_bridge_class_init,
> --
> 2.26.2
>
>
Re: [PATCH 3/8] amd_iommu: Use TYPE_AMD_IOMMU_PCI constant
Eduardo Habkost 于2020年8月27日周四 上午2:48写道：
>
> This will make future conversion to use OBJECT_DECLARE* easier.
>
> Signed-off-by: Eduardo Habkost
Reviewed-by: Li Qiang
> ---
> Cc: "Michael S. Tsirkin"
> Cc: Marcel Apfelbaum
> Cc: Paolo Bonzini
> Cc: Richard Henderson
> Cc: Eduardo Habkost
> Cc: qemu-devel@nongnu.org
> ---
> hw/i386/amd_iommu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
> index 087f601666..18411f1dec 100644
> --- a/hw/i386/amd_iommu.c
> +++ b/hw/i386/amd_iommu.c
> @@ -1622,7 +1622,7 @@ static const TypeInfo amdvi = {
> };
>
> static const TypeInfo amdviPCI = {
> -.name = "AMDVI-PCI",
> +.name = TYPE_AMD_IOMMU_PCI,
> .parent = TYPE_PCI_DEVICE,
> .instance_size = sizeof(AMDVIPCIState),
> .interfaces = (InterfaceInfo[]) {
> --
> 2.26.2
>
>
Re: [PATCH 5/8] xlnx-zcu102: Use TYPE_ZCU102_MACHINE constant
Eduardo Habkost 于2020年8月27日周四 上午2:47写道：
>
> This will make future conversion to use OBJECT_DECLARE* easier.
>
> Signed-off-by: Eduardo Habkost
Reviewed-by: Li Qiang
> ---
> Cc: Alistair Francis
> Cc: "Edgar E. Iglesias"
> Cc: Peter Maydell
> Cc: qemu-...@nongnu.org
> Cc: qemu-devel@nongnu.org
> ---
> hw/arm/xlnx-zcu102.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
> index 5997262459..672d9d4bd1 100644
> --- a/hw/arm/xlnx-zcu102.c
> +++ b/hw/arm/xlnx-zcu102.c
> @@ -238,7 +238,7 @@ static void xlnx_zcu102_machine_class_init(ObjectClass
> *oc, void *data)
> }
>
> static const TypeInfo xlnx_zcu102_machine_init_typeinfo = {
> -.name = MACHINE_TYPE_NAME("xlnx-zcu102"),
> +.name = TYPE_ZCU102_MACHINE,
> .parent = TYPE_MACHINE,
> .class_init = xlnx_zcu102_machine_class_init,
> .instance_init = xlnx_zcu102_machine_instance_init,
> --
> 2.26.2
>
>
Re: [PATCH 1/8] etsec: Use TYPE_ETSEC_COMMON constant
Eduardo Habkost 于2020年8月27日周四 上午2:46写道：
>
> This will make future conversion to use OBJECT_DECLARE* easier.
>
> Signed-off-by: Eduardo Habkost
Reviewed-by: Li Qiang
> ---
> Cc: David Gibson
> Cc: Jason Wang
> Cc: qemu-...@nongnu.org
> Cc: qemu-devel@nongnu.org
> ---
> hw/net/fsl_etsec/etsec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/net/fsl_etsec/etsec.c b/hw/net/fsl_etsec/etsec.c
> index 7035cf4eb9..ad20b22cdd 100644
> --- a/hw/net/fsl_etsec/etsec.c
> +++ b/hw/net/fsl_etsec/etsec.c
> @@ -430,7 +430,7 @@ static void etsec_class_init(ObjectClass *klass, void
> *data)
> }
>
> static TypeInfo etsec_info = {
> -.name = "eTSEC",
> +.name = TYPE_ETSEC_COMMON,
> .parent= TYPE_SYS_BUS_DEVICE,
> .instance_size = sizeof(eTSEC),
> .class_init= etsec_class_init,
> --
> 2.26.2
>
>
Re: [PATCH 4/8] sclpconsole: Use TYPE_* constants
Eduardo Habkost 于2020年8月27日周四 上午2:45写道：
>
> This will make future conversion to use OBJECT_DECLARE* easier.
>
> Signed-off-by: Eduardo Habkost
Reviewed-by: Li Qiang
> ---
> Cc: Cornelia Huck
> Cc: Halil Pasic
> Cc: Christian Borntraeger
> Cc: Thomas Huth
> Cc: "Marc-André Lureau"
> Cc: Paolo Bonzini
> Cc: qemu-s3...@nongnu.org
> Cc: qemu-devel@nongnu.org
> ---
> hw/char/sclpconsole-lm.c | 2 +-
> hw/char/sclpconsole.c| 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/char/sclpconsole-lm.c b/hw/char/sclpconsole-lm.c
> index 2b5f37b6a2..5848b4e9c5 100644
> --- a/hw/char/sclpconsole-lm.c
> +++ b/hw/char/sclpconsole-lm.c
> @@ -355,7 +355,7 @@ static void console_class_init(ObjectClass *klass, void
> *data)
> }
>
> static const TypeInfo sclp_console_info = {
> -.name = "sclplmconsole",
> +.name = TYPE_SCLPLM_CONSOLE,
> .parent= TYPE_SCLP_EVENT,
> .instance_size = sizeof(SCLPConsoleLM),
> .class_init= console_class_init,
> diff --git a/hw/char/sclpconsole.c b/hw/char/sclpconsole.c
> index 5c7664905e..d6f7da0818 100644
> --- a/hw/char/sclpconsole.c
> +++ b/hw/char/sclpconsole.c
> @@ -271,7 +271,7 @@ static void console_class_init(ObjectClass *klass, void
> *data)
> }
>
> static const TypeInfo sclp_console_info = {
> -.name = "sclpconsole",
> +.name = TYPE_SCLP_CONSOLE,
> .parent= TYPE_SCLP_EVENT,
> .instance_size = sizeof(SCLPConsole),
> .class_init= console_class_init,
> --
> 2.26.2
>
>
Re: [PATCH 2/8] nios2_iic: Use TYPE_ALTERA_IIC constant
Eduardo Habkost 于2020年8月27日周四 上午2:44写道：
>
> This will make future conversion to use OBJECT_DECLARE* easier.
>
> Signed-off-by: Eduardo Habkost
Reviewed-by: Li Qiang
> ---
> Cc: Chris Wulff
> Cc: Marek Vasut
> Cc: qemu-devel@nongnu.org
> ---
> hw/intc/nios2_iic.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/intc/nios2_iic.c b/hw/intc/nios2_iic.c
> index 1a5df8c89a..86d088f9b5 100644
> --- a/hw/intc/nios2_iic.c
> +++ b/hw/intc/nios2_iic.c
> @@ -80,7 +80,7 @@ static void altera_iic_class_init(ObjectClass *klass, void
> *data)
> }
>
> static TypeInfo altera_iic_info = {
> -.name = "altera,iic",
> +.name = TYPE_ALTERA_IIC,
> .parent= TYPE_SYS_BUS_DEVICE,
> .instance_size = sizeof(AlteraIIC),
> .instance_init = altera_iic_init,
> --
> 2.26.2
>
>
Re: [PATCH v5] qapi/opts-visitor: Added missing fallthrough annotations
I am just compiling with cflag set to -Wimplicit-fallthrough. I am using
gcc.
On Tue, Aug 25, 2020 at 2:03 AM Markus Armbruster wrote:
> Rohit Shinde writes:
>
> > Added fallthrough comment on line 270 to prevent the compiler from
> > throwing an error while compiling with the -Wimplicit-fallthrough flag
>
> None of the compilers I know warns there. Which one are you using?
>
> Commit message style tip: use the imperative mood
> https://chris.beams.io/posts/git-commit/#imperative
>
> > Signed-off-by: Rohit Shinde
> > ---
> > qapi/opts-visitor.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
> > index 7781c23a42..3422ff265e 100644
> > --- a/qapi/opts-visitor.c
> > +++ b/qapi/opts-visitor.c
> > @@ -266,6 +266,7 @@ opts_next_list(Visitor *v, GenericList *tail, size_t
> size)
> > }
> > ov->list_mode = LM_IN_PROGRESS;
> > /* range has been completed, fall through in order to pop
> option */
> > +/* fallthrough */
> >
> > case LM_IN_PROGRESS: {
> > const QemuOpt *opt;
>
>
RE: [RFC PATCH v3 09/34] Hexagon (target/hexagon) architecture types
> -Original Message- > From: Richard Henderson > Sent: Wednesday, August 26, 2020 8:19 AM > To: Taylor Simpson ; qemu-devel@nongnu.org > Cc: phi...@redhat.com; laur...@vivier.eu; riku.voi...@iki.fi; > aleksandar.m.m...@gmail.com; a...@rev.ng > Subject: Re: [RFC PATCH v3 09/34] Hexagon (target/hexagon) architecture > types > > > +#include > > Do you really need to re-include this? > This was done in "qemu/osdep.h". > > In general, osdep.h must be included first, and it takes care of all of the > basic system includes. OK
RE: [RFC PATCH v3 16/34] Hexagon (target/hexagon) utility functions
> -Original Message- > From: Richard Henderson > Sent: Wednesday, August 26, 2020 9:11 AM > To: Taylor Simpson ; qemu-devel@nongnu.org > Cc: phi...@redhat.com; laur...@vivier.eu; riku.voi...@iki.fi; > aleksandar.m.m...@gmail.com; a...@rev.ng > Subject: Re: [RFC PATCH v3 16/34] Hexagon (target/hexagon) utility functions > > Did you really need to reinvent qemu/int128.h? I remember looking at qemu/int128.h briefly but can't remember why I ended up not using it. I'll take another look.
RE: [RFC PATCH v3 14/34] Hexagon (target/hexagon) instruction/packet decode
> -Original Message- > From: Richard Henderson > Sent: Wednesday, August 26, 2020 9:07 AM > To: Taylor Simpson ; qemu-devel@nongnu.org > Cc: phi...@redhat.com; laur...@vivier.eu; riku.voi...@iki.fi; > aleksandar.m.m...@gmail.com; a...@rev.ng > Subject: Re: [RFC PATCH v3 14/34] Hexagon (target/hexagon) > instruction/packet decode > > On 8/18/20 8:50 AM, Taylor Simpson wrote: > > +#define DECODE_NEW_TABLE(TAG, SIZE, WHATNOT) \ > > +static struct _dectree_table_struct dectree_table_##TAG; > > All of these little structures should be const. > > Which probably requires these links to be const, and a few uses within the > functions that use them. > > That should move 78K worth of tables into .data.rel.ro, where they can't be > modified after relocation. OK > I can't help but thinking that all of this string manipulation is not the most > ideal way to implement a decoder. Surely all this can be pre-processed into > code, or flags, or an enumeration or something. I'll have to think about this one. > > Oh, and g_assert_not_reached() will never return. You don't have to keep > putting return statements after it. Please remove all of those, everywhere. OK > I'm going to ignore the rest of the decoder, as it's somewhat bewildering. > Even in the final patches-applied form. It could really use some more > commentary. OK
RE: [RFC PATCH v3 24/34] Hexagon (target/hexagon) opcode data structures
> -Original Message-
> From: Richard Henderson
> Sent: Wednesday, August 26, 2020 9:26 AM
> To: Taylor Simpson ; qemu-devel@nongnu.org
> Cc: phi...@redhat.com; laur...@vivier.eu; riku.voi...@iki.fi;
> aleksandar.m.m...@gmail.com; a...@rev.ng
> Subject: Re: [RFC PATCH v3 24/34] Hexagon (target/hexagon) opcode data
> structures
>
> > +extern const char *opcode_names[];
> > +
> > +extern const char *opcode_reginfo[];
> > +extern const char *opcode_rregs[];
> > +extern const char *opcode_wregs[];
>
> const char * const
OK
> > +extern opcode_encoding_t opcode_encodings[XX_LAST_OPCODE];
>
> const.
OK
> > +extern size4u_t
> > +opcode_attribs[XX_LAST_OPCODE][(A_ZZ_LASTATTRIB /
> ATTRIB_WIDTH) + 1];
>
> const.
OK
> And using qemu/bitops.h if possible, as discussed earlier vs attribs.h.
Do you mean replace the GET_ATTRIB macro with test_bit from qemu/bitops.h?
> Just initialize opcode_short_semantics with shortcode_generated.h in the
> first
> place. Then you don't need to create a table of NULL and overwrite at
> startup.
>
> And you can also make the table constant.
OK
> > +if (p == NULL) {
> > +g_assert_not_reached();
> > +return 0;
> > +}
>
> I prefer assert(p != NULL) to if (test) { abort(); }, where possible. E.g.
> this later one is fine:
OK
RE: [RFC PATCH v3 07/34] Hexagon (target/hexagon) scalar core helpers
> -Original Message-
> From: Richard Henderson
> Sent: Wednesday, August 26, 2020 8:17 AM
> To: Taylor Simpson ; qemu-devel@nongnu.org
> Cc: phi...@redhat.com; laur...@vivier.eu; riku.voi...@iki.fi;
> aleksandar.m.m...@gmail.com; a...@rev.ng
> Subject: Re: [RFC PATCH v3 07/34] Hexagon (target/hexagon) scalar core
> helpers
>
> > +DEF_HELPER_3(merge_inflight_store8u, s64, env, s32, s64)
>
> Please use DEF_HELPER_FLAGS_N when possible.
OK
> > +static inline void log_pred_write(CPUHexagonState *env, int pnum,
> > + target_ulong val)
>
> You might be better off letting the compiler decide about inlining.
Isn't "inline" just a hint to the compiler?
> > +union {
> > +uint8_t bytes[8];
> > +uint32_t data32;
> > +uint64_t data64;
> > +} retdata, storedata;
>
> Red flag here. This is assuming that the host and the target are both
> little-endian.
OK
> > +int bigmask = ((-load_width) & (-store_width));
> > +if ((load_addr & bigmask) != (store_addr & bigmask)) {
>
> Huh? This doesn't detect overlap. Counter example:
>
> load_addr = 0, load_width = 4,
> store_addr = 1, store_width = 1.
>
> bigmask = -4 & -1 -> -4.
>
> (0 & -4) != (1 & -4) -> 0 != 1
OK
> > +while ((i < load_width) && (j < store_width)) {
> > +retdata.bytes[i] = storedata.bytes[j];
> > +i++;
> > +j++;
> > +}
> > +return retdata.data64;
>
> This most definitely requires host-endian adjustment.
OK
> > +/* Helpful for printing intermediate values within instructions */
> > +void HELPER(debug_value)(CPUHexagonState *env, int32_t value)
> > +{
> > +HEX_DEBUG_LOG("value = 0x%x\n", value);
> > +}
> > +
> > +void HELPER(debug_value_i64)(CPUHexagonState *env, int64_t value)
> > +{
> > +HEX_DEBUG_LOG("value_i64 = 0x%lx\n", value);
> > +}
>
> I think we need to lose all of this.
> There are other ways to debug TCG.
OK
RE: [RFC PATCH v3 10/34] Hexagon (target/hexagon) instruction and packet types
> -Original Message-
> From: Richard Henderson
> Sent: Wednesday, August 26, 2020 8:22 AM
> To: Taylor Simpson ; qemu-devel@nongnu.org
> Cc: a...@rev.ng; riku.voi...@iki.fi; laur...@vivier.eu; phi...@redhat.com;
> aleksandar.m.m...@gmail.com
> Subject: Re: [RFC PATCH v3 10/34] Hexagon (target/hexagon) instruction and
> packet types
>
> On 8/18/20 8:50 AM, Taylor Simpson wrote:
> > +struct Instruction {
> > +semantic_insn_t generate;/* pointer to genptr routine */
> > +size1u_t regno[REG_OPERANDS_MAX];/* reg operands including
> predicates */
> > +size2u_t opcode;
> > +
> > +size4u_t iclass:6;
> > +size4u_t slot:3;
> > +size4u_t part1:1;/*
> > + * cmp-jumps are split into two insns.
> > + * set for the compare and clear for the jump
> > + */
> > +size4u_t extension_valid:1; /* Has a constant extender attached */
> > +size4u_t which_extended:1;/* If has an extender, which immediate
> */
> > +size4u_t is_endloop:1; /* This is an end of loop */
> > +size4u_t new_value_producer_slot:4;
> > +size4s_t immed[IMMEDS_MAX];/* immediate field */
> > +};
>
> Is this an imported file or not?
>
> If it is not imported, I'd very much prefer that we stick to the stdint.h type
> names.
Agreed. My goal is to stick with stdint.h types outside the imported
directory. I'll change this.
RE: [RFC PATCH v3 17/34] Hexagon (target/hexagon/imported) arch import - macro definitions
> -Original Message- > From: Richard Henderson > Sent: Wednesday, August 26, 2020 9:17 AM > To: Taylor Simpson ; qemu-devel@nongnu.org > Cc: phi...@redhat.com; laur...@vivier.eu; riku.voi...@iki.fi; > aleksandar.m.m...@gmail.com; a...@rev.ng > Subject: Re: [RFC PATCH v3 17/34] Hexagon (target/hexagon/imported) arch > import - macro definitions > > You might as well squash all of the patches dealing with imported files, > because they're beyond review. They are what they are: another project's > files. > > Give that squashed patch my > > Acked-by: Richard Henderson OK > PS: I notice that the README mentions archlib, but does not give a pointer to > it. Other than what we import here, it's not open source.
RE: [RFC PATCH v3 06/34] Hexagon (disas) disassembler
> -Original Message- > From: Richard Henderson > Sent: Wednesday, August 26, 2020 7:52 AM > To: Taylor Simpson ; qemu-devel@nongnu.org > Cc: phi...@redhat.com; laur...@vivier.eu; riku.voi...@iki.fi; > aleksandar.m.m...@gmail.com; a...@rev.ng > Subject: Re: [RFC PATCH v3 06/34] Hexagon (disas) disassembler > > Normally our disassemblers print the instruction address; sometimes the raw > bytes (or word) of the instruction. OK
RE: [RFC PATCH v3 13/34] Hexagon (target/hexagon) register map
> -Original Message- > From: Richard Henderson > Sent: Wednesday, August 26, 2020 8:36 AM > To: Taylor Simpson ; qemu-devel@nongnu.org > Cc: phi...@redhat.com; laur...@vivier.eu; riku.voi...@iki.fi; > aleksandar.m.m...@gmail.com; a...@rev.ng > Subject: Re: [RFC PATCH v3 13/34] Hexagon (target/hexagon) register map > > > +DEF_REGMAP(V__16, 16, 0, 2, 4, 6, 8, 10, 12, 14, 16, 18, 20, 22, 24, 26, > > 28, > 30) > > Given that DEF_REGMAP itself is defined in decode.c, and not even in > another > header file, why do these not live in decode.c as well? I'll move them to decode.c.
RE: [RFC PATCH v3 11/34] Hexagon (target/hexagon) register fields
> -Original Message-
> From: Richard Henderson
> Sent: Wednesday, August 26, 2020 8:30 AM
> To: Taylor Simpson ; qemu-devel@nongnu.org
> Cc: phi...@redhat.com; laur...@vivier.eu; riku.voi...@iki.fi;
> aleksandar.m.m...@gmail.com; a...@rev.ng
> Subject: Re: [RFC PATCH v3 11/34] Hexagon (target/hexagon) register fields
>
> > +#define NUM_GEN_REGS 32
>
> What's this? It doesn't appear to be field related.
Not needed, will remove it.
> > +extern reg_field_t reg_field_info[];
>
> const.
OK
> > +enum reg_fields_enum {
>
> Doesn't follow naming guidelines. But you don't actually use the name at all,
> so better to just drop the name entirely?
OK
> > +/* USR fields */
> > +DEF_REG_FIELD(USR_OVF,
> > +"ovf", 0, 1,
> > +"Sticky Saturation Overflow - "
> > +"Set when saturation occurs while executing instruction that specifies
> > "
> > +"optional saturation, remains set until explicitly cleared by a USR=Rs
> > "
> > +"instruction.")
>
> Is the description as a string really useful, or even used?
> A comment would seem to do just as well, not consume space in the final
> binary,
> and even then seems redundant with the actual architecture manual.
I thought they help make the code more readable. You are right that they
shouldn't take space in the binary.
I can either change so they don't go into the binary or remove them altogether
- guess I'll remove them altogether.
RE: [RFC PATCH v3 04/34] Hexagon (target/hexagon) scalar core definition
Thanks for the feedback, Richard!!
> -Original Message-
> From: Richard Henderson
> Sent: Wednesday, August 26, 2020 7:36 AM
> To: Taylor Simpson ; qemu-devel@nongnu.org
> Cc: phi...@redhat.com; laur...@vivier.eu; riku.voi...@iki.fi;
> aleksandar.m.m...@gmail.com; a...@rev.ng
> Subject: Re: [RFC PATCH v3 04/34] Hexagon (target/hexagon) scalar core
> definition
>
> On 8/18/20 8:50 AM, Taylor Simpson wrote:
> > +#include
>
> This should not be in cpu.h. What's up?
We're not using qemu softfloat (it's on the TODO list), so there's a fenv_t
field in CPUHexagonState.
> > +#define TARGET_PAGE_BITS 16 /* 64K pages */
> > +#define TARGET_LONG_BITS 32
>
> Belongs in cpu-param.h
OK
> > +#ifdef CONFIG_USER_ONLY
> > +#define TOTAL_PER_THREAD_REGS 64
> > +#else
> ...
> > +target_ulong gpr[TOTAL_PER_THREAD_REGS];
>
> Do I not understand hexagon enough to know why the number of general
> registers
> would vary with system mode? Why is the define conditional on user-only?
Yes, there are some registers that are only available in system mode. Since
this series is only for linux-user mode, I'll remove the ifdef for now.
We're working on system mode. When that series is ready, we can put the ifdef
back in. At that time, you'll also see the extra registers in hex_regs.h.
> No. There are plenty of bad examples of this in qemu, let's not add another.
>
> First, the lock doesn't do what you think.
> Second, stderr is never right.
> Third, just about any time you want this, there's a tracepoint that you could
> add that would be better, correct, and toggleable from the command-line.
OK
> I understand your desire for this sort of comparison. What I don't
> understand
> is the method. Surely it would be preferable to actually change the stack
> location in qemu, rather than constantly adjust for it.
>
> Add a per-target hook to linux-user/hexagon/target_elf.h that controls the
> allocation of the stack in elfload.c, setup_arg_pages().
OK, will look into this. Thanks for the advice, I wasn't aware this was
possible.
>
> > +static void hexagon_dump(CPUHexagonState *env, FILE *f)
> > +{
> > +static target_ulong last_pc;
> > +int i;
> > +
> > +/*
> > + * When comparing with LLDB, it doesn't step through single-cycle
> > + * hardware loops the same way. So, we just skip them here
> > + */
> > +if (env->gpr[HEX_REG_PC] == last_pc) {
> > +return;
> > +}
>
> Multi-threaded data race. Might as well move last_pc to env->dump_last_pc
> or
> something.
>
> But I'd also suggest that all of this lldb compatibility stuff be optional,
> switchable from the command-line with a cpu property. Because there are
> going
> to be real cases where *not* single-stepping will result in dumps from the
> same
> PC, and you've just squashed all of those.
>
> Call the property x-lldb-compat, or something, and default it to off. You
> then
> turn it on with "-cpu v67,x-lldb-compat=on".
OK
Re: [PATCH v2 2/2] linux-user: Add support for utimensat_time64() and semtimedop_time64()
Le 26/08/2020 à 15:58, Laurent Vivier a écrit :
> Le 25/08/2020 à 16:23, Laurent Vivier a écrit :
>> Le 25/08/2020 à 09:18, Laurent Vivier a écrit :
>>> Le 25/08/2020 à 00:30, Filip Bozuta a écrit :
This patch introduces functionality for following time64 syscalls:
*utimensat_time64()
int utimensat(int dirfd, const char *pathname,
const struct timespec times[2], int flags);
-- change file timestamps with nanosecond precision --
man page: https://man7.org/linux/man-pages/man2/utimensat.2.html
*semtimedop_time64()
int semtimedop(int semid, struct sembuf *sops, size_t nsops,
const struct timespec *timeout);
-- System V semaphore operations --
man page: https://www.man7.org/linux/man-pages/man2/semtimedop.2.html
Implementation notes:
Syscall 'utimensat_time64()' is implemented in similar way as its
regular variants only difference being that time64 converting function
is used to convert values of 'struct timespec' between host and target
('target_to_host_timespec64()').
For syscall 'semtimedop_time64()' and additional argument is added
in function 'do_semtimedop()' through which the aproppriate 'struct
timespec'
converting function is called (false for regular
target_to_host_timespec()
and true for target_to_host_timespec64()). For 'do_ipc()' a
check was added as that additional argument: 'TARGET_ABI_BITS == 64'.
Signed-off-by: Filip Bozuta
Reviewed-by: Laurent Vivier
---
linux-user/syscall.c | 60
1 file changed, 50 insertions(+), 10 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index fc6a6e32e4..4d460af744 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -1253,7 +1253,8 @@ static inline abi_long
target_to_host_timespec(struct timespec *host_ts,
#endif
#if defined(TARGET_NR_clock_settime64) || defined(TARGET_NR_futex_time64)
|| \
-defined(TARGET_NR_pselect6_time64) || defined(TARGET_NR_ppoll_time64)
+defined(TARGET_NR_pselect6_time64) || defined(TARGET_NR_ppoll_time64)
|| \
+defined(TARGET_NR_utimensat_time64) ||
defined(TARGET_NR_semtimedop_time64)
static inline abi_long target_to_host_timespec64(struct timespec *host_ts,
abi_ulong target_addr)
{
@@ -4117,7 +4118,7 @@ static inline abi_long target_to_host_sembuf(struct
sembuf *host_sembuf,
}
#if defined(TARGET_NR_ipc) || defined(TARGET_NR_semop) || \
-defined(TARGET_NR_semtimedop)
+defined(TARGET_NR_semtimedop) || defined(TARGET_NR_semtimedop_time64)
/*
* This macro is required to handle the s390 variants, which passes the
@@ -4134,7 +4135,7 @@ static inline abi_long target_to_host_sembuf(struct
sembuf *host_sembuf,
static inline abi_long do_semtimedop(int semid,
abi_long ptr,
unsigned nsops,
- abi_long timeout)
+ abi_long timeout, bool time64)
{
struct sembuf sops[nsops];
struct timespec ts, *pts = NULL;
@@ -4142,8 +4143,14 @@ static inline abi_long do_semtimedop(int semid,
if (timeout) {
pts =
-if (target_to_host_timespec(pts, timeout)) {
-return -TARGET_EFAULT;
+if (time64) {
+if (target_to_host_timespec64(pts, timeout)) {
+return -TARGET_EFAULT;
+}
+} else {
+if (target_to_host_timespec(pts, timeout)) {
+return -TARGET_EFAULT;
+}
}
}
@@ -4657,7 +4664,7 @@ static abi_long do_ipc(CPUArchState *cpu_env,
switch (call) {
case IPCOP_semop:
-ret = do_semtimedop(first, ptr, second, 0);
+ret = do_semtimedop(first, ptr, second, 0, false);
break;
case IPCOP_semtimedop:
/*
@@ -4667,9 +4674,9 @@ static abi_long do_ipc(CPUArchState *cpu_env,
* to a struct timespec where the generic variant uses fifth
parameter.
*/
#if defined(TARGET_S390X)
-ret = do_semtimedop(first, ptr, second, third);
+ret = do_semtimedop(first, ptr, second, third, TARGET_ABI_BITS ==
64);
#else
-ret = do_semtimedop(first, ptr, second, fifth);
+ret = do_semtimedop(first, ptr, second, fifth, TARGET_ABI_BITS ==
64);
#endif
break;
@@ -9887,11 +9894,15 @@
[Bug 1892604] Re: qemu-system-arm: ../hw/usb/hcd-dwc2.c:666: dwc2_glbreg_read: Assertion `addr <= GINTSTS2' failed.
Hmm, yes agreed. I started a 2-week holiday on Monday, I can work on this after I get back on Sept. 7 -- 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
[REPORT] Nightly Performance Tests - Wednesday, August 26, 2020
Host CPU : Intel(R) Core(TM) i7-8750H CPU @ 2.20GHz Host Memory : 15.49 GB Start Time (UTC) : 2020-08-26 21:30:02 End Time (UTC) : 2020-08-26 22:03:25 Execution Time : 0:33:22.684015 Status : SUCCESS Note: Changes denoted by '-' are less than 0.01%. SUMMARY REPORT - COMMIT 25f6dc28 AVERAGE RESULTS Target Instructions Latest v5.1.0 -- -- -- aarch642 158 343 131 - +1.692% alpha 1 914 977 713 - +3.525% arm8 076 400 021 - +2.304% hppa 4 261 678 154 - +3.163% m68k 2 690 261 170 - +7.131% mips 1 862 023 351 - +2.493% mipsel 2 008 208 091 - +2.674% mips64 1 918 635 853 - +2.818% mips64el 2 051 571 520 - +3.026% ppc2 480 152 604 - +3.107% ppc64 2 576 708 898 - +3.142% ppc64le2 558 863 471 - +3.173% riscv641 406 706 018 - +2.65% s390x 3 158 137 523 - +3.118% sh42 364 463 007 - +3.331% sparc643 318 544 246 - +3.851% x86_64 1 775 842 110 - +2.156% DETAILED RESULTS Test Program: dijkstra_double Target Instructions Latest v5.1.0 -- -- -- aarch643 062 571 701 - +1.423% alpha 3 191 875 352 - +3.696% arm 16 357 154 152 - +2.347% hppa 7 228 368 174 - +3.086% m68k 4 294 003 802 - +9.692% mips 3 051 405 821 - +2.426% mipsel 3 231 506 827 - +2.869% mips64 3 245 837 618 - +2.596% mips64el 3 414 203 643 - +3.021% ppc4 914 532 207 - +4.74% ppc64 5 098 149 119 - +4.565% ppc64le5 082 428 275 - +4.58% riscv642 192 299 304 - +1.956% s390x 4 584 501 630 - +2.896% sh43 949 051 181 - +3.464% sparc644 586 202 420 - +4.237% x86_64 2 484 087 711 - +1.75% Test Program: dijkstra_int32 Target Instructions Latest v5.1.0 -- -- -- aarch642 210 184 587 - +1.493% alpha 1 494 141 516 - +2.151% arm8 262 932 780 - +2.665% hppa 5 207 310 153 - +3.047% m68k 1 725 847 586 - +2.526% mips 1 495 219 183 - +1.491% mipsel 1 497 145 499 - +1.479% mips64 1 715 391 181 - +1.892% mips64el 1 695 282 408 - +1.913% ppc2 014 571 983 - +1.82% ppc64 2 206 263 388 - +2.138% ppc64le2 198 011 065 - +2.147% riscv641 354 917 093 - +2.396% s390x 2 916 244 976 - +1.241% sh41 990 543 099 - +2.67% sparc642 872 232 296 - +3.758% x86_64 1 553 980 145 - +2.12% Test Program: matmult_double Target Instructions Latest v5.1.0 -- -- -- aarch641 412 257 972 - +0.301% alpha 3 234 002 286 - +7.474% arm8 545 170 759 - +1.088% hppa 3 483 589 580 - +4.468% m68k 3 919 052 824 -+18.431% mips 2 344 763 681 - +4.09% mipsel 3 329 886 020 - +5.177% mips64 2 359 046 809 - +4.076%
Re: [PATCH v2 1/1] core/register: Specify instance_size in the TypeInfo
On Wed, Aug 26, 2020 at 12:49 PM Eduardo Habkost wrote:
>
> On Tue, Aug 25, 2020 at 10:30:59AM -0700, Alistair Francis wrote:
> > 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 = [index];
> >
> > -*r = (RegisterInfo) {
> > -.data = data + data_size * index,
> > -.data_size = data_size,
> > -.access = [i],
> > -.opaque = owner,
> > -};
> > -register_init(r);
> > +if (data + data_size * index == 0 || ![i]) {
>
> Do you know what's the goal of this check?
>
> Can `data` or `rae` be NULL? If not, it seems impossible for
> this condition to be true. If they can, this seems to be a weird
> and fragile way of checking for NULL arguments.
I think the idea is that some sections in rae could be NULL or parts
of the data array could be NULL and we are checking for that here.
It seems unlikely that will be the case but I don't think the check
really hurts us.
Alistair
>
> > +continue;
> > +}
> > +
> > +/* Init the register, this will zero it. */
> > +object_initialize((void *)r, sizeof(*r), TYPE_REGISTER);
> > +
> > +/* Set the properties of the register */
> > +r->data = data + data_size * index;
> > +r->data_size = data_size;
> > +r->access = [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
> >
>
> --
> Eduardo
>
Re: [RFC v4 00/70] support vector extension v1.0
On Wed, Aug 26, 2020 at 11:13 AM Frank Chang wrote: > > On Thu, Aug 27, 2020 at 2:03 AM Alistair Francis wrote: >> >> On Wed, Aug 26, 2020 at 10:39 AM Frank Chang wrote: >> > >> > On Thu, Aug 27, 2020 at 12:56 AM Alistair Francis >> > wrote: >> >> >> >> On Tue, Aug 25, 2020 at 1:29 AM Frank Chang >> >> wrote: >> >> > >> >> > On Mon, Aug 17, 2020 at 4:50 PM wrote: >> >> >> >> >> >> From: Frank Chang >> >> >> >> >> >> This patchset implements the vector extension v1.0 for RISC-V on QEMU. >> >> >> >> >> >> This patchset is sent as RFC because RVV v1.0 is still in draft state. >> >> >> v2 patchset was sent for RVV v0.9 and bumped to RVV v1.0 since v3 >> >> >> patchset. >> >> >> >> >> >> The port is available here: >> >> >> https://github.com/sifive/qemu/tree/rvv-1.0-upstream-v4 >> >> >> >> >> >> You can change the cpu argument: vext_spec to v1.0 (i.e. >> >> >> vext_spec=v1.0) >> >> >> to run with RVV v1.0 instructions. >> >> >> >> >> >> Note: This patchset depends on two other patchsets listed in Based-on >> >> >> section below so it might not able to be built unless those two >> >> >> patchsets are applied. >> >> >> >> >> >> Changelog: >> >> >> >> >> >> v4 >> >> >> * remove explicit float flmul variable in DisasContext. >> >> >> * replace floating-point calculations with shift operations to >> >> >> improve performance. >> >> >> * relax RV_VLEN_MAX to 512-bits. >> >> >> >> >> >> v3 >> >> >> * apply nan-box helpers from Richard Henderson. >> >> >> * remove fp16 api changes as they are sent independently in another >> >> >> pathcset by Chih-Min Chao. >> >> >> * remove all tail elements clear functions as tail elements can >> >> >> retain unchanged for either VTA set to undisturbed or agnostic. >> >> >> * add fp16 nan-box check generator function. >> >> >> * add floating-point rounding mode enum. >> >> >> * replace flmul arithmetic with shifts to avoid floating-point >> >> >> conversions. >> >> >> * add Zvqmac extension. >> >> >> * replace gdbstub vector register xml files with dynamic generator. >> >> >> * bumped to RVV v1.0. >> >> >> * RVV v1.0 related changes: >> >> >> * add vlre.v and vsr.v vector whole register >> >> >> load/store instructions >> >> >> * add vrgatherei16 instruction. >> >> >> * rearranged bits in vtype to make vlmul bits into a contiguous >> >> >> field. >> >> >> >> >> >> v2 >> >> >> * drop v0.7.1 support. >> >> >> * replace invisible return check macros with functions. >> >> >> * move mark_vs_dirty() to translators. >> >> >> * add SSTATUS_VS flag for s-mode. >> >> >> * nan-box scalar fp register for floating-point operations. >> >> >> * add gdbstub files for vector registers to allow system-mode >> >> >> debugging with GDB. >> >> >> >> >> >> Based-on: <20200724002807.441147-1-richard.hender...@linaro.org/> >> >> >> Based-on: <1596102747-20226-1-git-send-email-chihmin.c...@sifive.com/> >> >> >> >> >> >> Frank Chang (62): >> >> >> target/riscv: drop vector 0.7.1 and add 1.0 support >> >> >> target/riscv: Use FIELD_EX32() to extract wd field >> >> >> target/riscv: rvv-1.0: introduce writable misa.v field >> >> >> target/riscv: rvv-1.0: remove rvv related codes from fcsr registers >> >> >> target/riscv: rvv-1.0: check MSTATUS_VS when accessing vector csr >> >> >> registers >> >> >> target/riscv: rvv-1.0: remove MLEN calculations >> >> >> target/riscv: rvv-1.0: add fractional LMUL >> >> >> target/riscv: rvv-1.0: add VMA and VTA >> >> >> target/riscv: rvv-1.0: update check functions >> >> >> target/riscv: introduce more imm value modes in translator functions >> >> >> target/riscv: rvv:1.0: add translation-time nan-box helper function >> >> >> target/riscv: rvv-1.0: configure instructions >> >> >> target/riscv: rvv-1.0: stride load and store instructions >> >> >> target/riscv: rvv-1.0: index load and store instructions >> >> >> target/riscv: rvv-1.0: fix address index overflow bug of indexed >> >> >> load/store insns >> >> >> target/riscv: rvv-1.0: fault-only-first unit stride load >> >> >> target/riscv: rvv-1.0: amo operations >> >> >> target/riscv: rvv-1.0: load/store whole register instructions >> >> >> target/riscv: rvv-1.0: update vext_max_elems() for load/store insns >> >> >> target/riscv: rvv-1.0: take fractional LMUL into vector max elements >> >> >> calculation >> >> >> target/riscv: rvv-1.0: floating-point square-root instruction >> >> >> target/riscv: rvv-1.0: floating-point classify instructions >> >> >> target/riscv: rvv-1.0: mask population count instruction >> >> >> target/riscv: rvv-1.0: find-first-set mask bit instruction >> >> >> target/riscv: rvv-1.0: set-X-first mask bit instructions >> >> >> target/riscv: rvv-1.0: iota instruction >> >> >> target/riscv: rvv-1.0: element index instruction >> >> >> target/riscv: rvv-1.0: allow load element with sign-extended >> >> >> target/riscv: rvv-1.0: register
Re: [PATCH 03/10] spapr: robustify NVLink2 NUMA node logic
On 8/19/20 11:14 PM, David Gibson wrote:
On Fri, Aug 14, 2020 at 05:54:17PM -0300, Daniel Henrique Barboza wrote:
NVLink2 GPUs are allocated in their own NUMA node, at maximum
distance from every other resource in the board. The existing
logic makes some assumptions that don't scale well:
- only NVLink2 GPUs will ever require such mechanism, meaning
that the GPU logic is tightly coupled with the NUMA setup of
the machine, via how ibm,max-associativity-domains is set.
- the code is relying on the lack of support for sparse NUMA
nodes in QEMU. Eventually this support can be implemented, and
then the assumption that spapr->gpu_numa_id represents the total
of NUMA nodes plus all generated NUMA ids for the GPUs, which
relies on all QEMU NUMA nodes not being sparsed, has a good
potential for disaster.
This patch aims to fix both assumptions by creating a generic
mechanism to get an available NUMA node, regardless of the
NUMA setup being sparse or not. The idea is to rename the existing
spapr->gpu_numa_id to spapr->current_numa_id and add a new
spapr->extra_numa_nodes attribute. They are used in a new function
called spapr_pci_get_available_numa_id(), that takes into account
that the NUMA conf can be sparsed or not, to retrieve an available
NUMA id for the caller. Each consecutive call of
spapr_pci_get_available_numa_id() will generate a new ID, up
to the limit of numa_state->num_nodes + spapr->extra_numa_nodes
exceeding MAX_NODES. This is a generic code being used only by
NVLink2 ATM, being available to be used in the future by any
other device.
With this new function in place, we can decouple
ibm,max-associativity-domains logic from NVLink2 logic by
using the new spapr->extra_numa_nodes to define the maxdomains
of the forth NUMA level. Instead of defining it as gpu_numa_id,
use num_nodes + extra_numa_nodes. This also makes it resilient
to any future change in the support of sparse NUMA nodes.
Despite all the code juggling, no functional change was made
because sparse NUMA nodes isn't a thing and we do not support
distinct NUMA distances via user input. Next patches will
change that.
Signed-off-by: Daniel Henrique Barboza
---
hw/ppc/spapr.c | 15 ++-
hw/ppc/spapr_pci.c | 33 +
hw/ppc/spapr_pci_nvlink2.c | 10 ++
include/hw/pci-host/spapr.h | 2 ++
include/hw/ppc/spapr.h | 4 +++-
5 files changed, 54 insertions(+), 10 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3b16edaf4c..22e78cfc84 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -910,13 +910,13 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, void
*fdt)
cpu_to_be32(SPAPR_MEMORY_BLOCK_SIZE & 0x),
cpu_to_be32(ms->smp.max_cpus / ms->smp.threads),
};
-uint32_t maxdomain = cpu_to_be32(spapr->gpu_numa_id > 1 ? 1 : 0);
+uint32_t maxdomain = cpu_to_be32(spapr->extra_numa_nodes > 1 ? 1 : 0);
uint32_t maxdomains[] = {
cpu_to_be32(4),
maxdomain,
maxdomain,
maxdomain,
-cpu_to_be32(spapr->gpu_numa_id),
+cpu_to_be32(ms->numa_state->num_nodes + spapr->extra_numa_nodes),
};
_FDT(rtas = fdt_add_subnode(fdt, 0, "rtas"));
@@ -2824,13 +2824,18 @@ static void spapr_machine_init(MachineState *machine)
/*
* NVLink2-connected GPU RAM needs to be placed on a separate NUMA node.
* We assign a new numa ID per GPU in spapr_pci_collect_nvgpu() which is
- * called from vPHB reset handler so we initialize the counter here.
+ * called from vPHB reset handler. We have code to generate an extra numa
+ * id to place the GPU via 'extra_numa_nodes' and 'current_numa_node',
which
+ * are initialized here.
+ *
* If no NUMA is configured from the QEMU side, we start from 1 as GPU RAM
* must be equally distant from any other node.
- * The final value of spapr->gpu_numa_id is going to be written to
+ *
+ * The extra NUMA node ids generated for GPU usage will be written to
* max-associativity-domains in spapr_build_fdt().
*/
-spapr->gpu_numa_id = MAX(1, machine->numa_state->num_nodes);
+spapr->current_numa_id = 0;
+spapr->extra_numa_nodes = 0;
if ((!kvm_enabled() || kvmppc_has_cap_mmu_radix()) &&
ppc_type_check_compat(machine->cpu_type, CPU_POWERPC_LOGICAL_3_00, 0,
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 0a418f1e67..09ac58fd7f 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -2492,3 +2492,36 @@ void spapr_pci_switch_vga(bool big_endian)
_endian);
}
}
+
+unsigned spapr_pci_get_available_numa_id(Error **errp)
+{
+MachineState *machine = MACHINE(qdev_get_machine());
+SpaprMachineState *spapr = SPAPR_MACHINE(machine);
+NodeInfo *numa_info = machine->numa_state->nodes;
+unsigned i, start;
+
+if (machine->numa_state->num_nodes + spapr->extra_numa_nodes >=
[PULL v5 04/12] hw/display/artist.c: fix out of bounds check
From: Sven Schnelle
Fix the following runtime warning with artist framebuffer:
"write outside bounds: wants 1256x1023, max size 1280x1024"
Reviewed-by: Richard Henderson
Signed-off-by: Sven Schnelle
Signed-off-by: Helge Deller
---
hw/display/artist.c | 24 +++-
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/hw/display/artist.c b/hw/display/artist.c
index 6261bfe65b..de56200dbf 100644
--- a/hw/display/artist.c
+++ b/hw/display/artist.c
@@ -340,14 +340,13 @@ static void vram_bit_write(ARTISTState *s, int posx, int
posy, bool incr_x,
{
struct vram_buffer *buf;
uint32_t vram_bitmask = s->vram_bitmask;
-int mask, i, pix_count, pix_length, offset, height, width;
+int mask, i, pix_count, pix_length, offset, width;
uint8_t *data8, *p;
pix_count = vram_write_pix_per_transfer(s);
pix_length = vram_pixel_length(s);
buf = vram_write_buffer(s);
-height = buf->height;
width = buf->width;
if (s->cmap_bm_access) {
@@ -367,13 +366,6 @@ static void vram_bit_write(ARTISTState *s, int posx, int
posy, bool incr_x,
pix_count = size * 8;
}
-if (posy * width + posx + pix_count > buf->size) {
-qemu_log("write outside bounds: wants %dx%d, max size %dx%d\n",
- posx, posy, width, height);
-return;
-}
-
-
switch (pix_length) {
case 0:
if (s->image_bitmap_op & 0x2000) {
@@ -381,8 +373,11 @@ static void vram_bit_write(ARTISTState *s, int posx, int
posy, bool incr_x,
}
for (i = 0; i < pix_count; i++) {
-artist_rop8(s, p + offset + pix_count - 1 - i,
-(data & 1) ? (s->plane_mask >> 24) : 0);
+uint32_t off = offset + pix_count - 1 - i;
+if (off < buf->size) {
+artist_rop8(s, p + off,
+(data & 1) ? (s->plane_mask >> 24) : 0);
+}
data >>= 1;
}
memory_region_set_dirty(>mr, offset, pix_count);
@@ -398,7 +393,10 @@ static void vram_bit_write(ARTISTState *s, int posx, int
posy, bool incr_x,
for (i = 3; i >= 0; i--) {
if (!(s->image_bitmap_op & 0x2000) ||
s->vram_bitmask & (1 << (28 + i))) {
-artist_rop8(s, p + offset + 3 - i, data8[ROP8OFF(i)]);
+uint32_t off = offset + 3 - i;
+if (off < buf->size) {
+artist_rop8(s, p + off, data8[ROP8OFF(i)]);
+}
}
}
memory_region_set_dirty(>mr, offset, 3);
@@ -420,7 +418,7 @@ static void vram_bit_write(ARTISTState *s, int posx, int
posy, bool incr_x,
break;
}
-for (i = 0; i < pix_count; i++) {
+for (i = 0; i < pix_count && offset + i < buf->size; i++) {
mask = 1 << (pix_count - 1 - i);
if (!(s->image_bitmap_op & 0x2000) ||
--
2.21.3
Re: [PATCH v2 1/1] core/register: Specify instance_size in the TypeInfo
On Wed, Aug 26, 2020 at 2:40 AM Philippe Mathieu-Daudé wrote:
>
> 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 = [index];
>>
>> -*r = (RegisterInfo) {
>> -.data = data + data_size * index,
>> -.data_size = data_size,
>> -.access = [i],
>> -.opaque = owner,
>> -};
>> -register_init(r);
>> +if (data + data_size * index == 0 || ![i]) {
>> +continue;
>> +}
>> +
>> +/* Init the register, this will zero it. */
>> +object_initialize((void *)r, sizeof(*r), TYPE_REGISTER);
>
>
> Easier to review [index] than that void* cast IMO.
I find that more complex as then we aren't using the local variable we
created. I'm going to leave it as is, I hope that's ok.
> Otherwise:
> Reviewed-by: Philippe Mathieu-Daudé
Thanks
Alistair
>
>> +
>> +/* Set the properties of the register */
>> +r->data = data + data_size * index;
>> +r->data_size = data_size;
>> +r->access = [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
>>
>>
[PULL v5 09/12] hw/display/artist: Prevent out of VRAM buffer accesses
Simplify various bounds checks by changing parameters like row and column
numbers to become unsigned instead of signed.
With that we can check if the calculated offset is bigger than the size of the
VRAM region and bail out if not.
Reported-by: LLVM libFuzzer
Reported-by: Alexander Bulekov
Buglink: https://bugs.launchpad.net/qemu/+bug/1880326
Buglink: https://bugs.launchpad.net/qemu/+bug/1890310
Buglink: https://bugs.launchpad.net/qemu/+bug/1890311
Buglink: https://bugs.launchpad.net/qemu/+bug/1890312
Buglink: https://bugs.launchpad.net/qemu/+bug/1890370
Acked-by: Alexander Bulekov
Signed-off-by: Helge Deller
---
hw/display/artist.c | 110 +++-
1 file changed, 69 insertions(+), 41 deletions(-)
diff --git a/hw/display/artist.c b/hw/display/artist.c
index f37aa9eb49..46eaa10dae 100644
--- a/hw/display/artist.c
+++ b/hw/display/artist.c
@@ -35,9 +35,9 @@
struct vram_buffer {
MemoryRegion mr;
uint8_t *data;
-int size;
-int width;
-int height;
+unsigned int size;
+unsigned int width;
+unsigned int height;
};
typedef struct ARTISTState {
@@ -274,15 +274,15 @@ static artist_rop_t artist_get_op(ARTISTState *s)
}
static void artist_rop8(ARTISTState *s, struct vram_buffer *buf,
-int offset, uint8_t val)
+unsigned int offset, uint8_t val)
{
const artist_rop_t op = artist_get_op(s);
uint8_t plane_mask;
uint8_t *dst;
-if (offset < 0 || offset >= buf->size) {
+if (offset >= buf->size) {
qemu_log_mask(LOG_GUEST_ERROR,
- "rop8 offset:%d bufsize:%u\n", offset, buf->size);
+ "rop8 offset:%u bufsize:%u\n", offset, buf->size);
return;
}
dst = buf->data + offset;
@@ -294,8 +294,7 @@ static void artist_rop8(ARTISTState *s, struct vram_buffer
*buf,
break;
case ARTIST_ROP_COPY:
-*dst &= ~plane_mask;
-*dst |= val & plane_mask;
+*dst = (*dst & ~plane_mask) | (val & plane_mask);
break;
case ARTIST_ROP_XOR:
@@ -349,7 +348,8 @@ static void vram_bit_write(ARTISTState *s, int posx, int
posy, bool incr_x,
{
struct vram_buffer *buf;
uint32_t vram_bitmask = s->vram_bitmask;
-int mask, i, pix_count, pix_length, offset, width;
+int mask, i, pix_count, pix_length;
+unsigned int offset, width;
uint8_t *data8, *p;
pix_count = vram_write_pix_per_transfer(s);
@@ -364,8 +364,7 @@ static void vram_bit_write(ARTISTState *s, int posx, int
posy, bool incr_x,
offset = posy * width + posx;
}
-if (!buf->size) {
-qemu_log("write to non-existent buffer\n");
+if (!buf->size || offset >= buf->size) {
return;
}
@@ -394,7 +393,9 @@ static void vram_bit_write(ARTISTState *s, int posx, int
posy, bool incr_x,
case 3:
if (s->cmap_bm_access) {
-*(uint32_t *)(p + offset) = data;
+if (offset + 3 < buf->size) {
+*(uint32_t *)(p + offset) = data;
+}
break;
}
data8 = (uint8_t *)
@@ -464,12 +465,14 @@ static void vram_bit_write(ARTISTState *s, int posx, int
posy, bool incr_x,
}
}
-static void block_move(ARTISTState *s, int source_x, int source_y, int dest_x,
- int dest_y, int width, int height)
+static void block_move(ARTISTState *s,
+ unsigned int source_x, unsigned int source_y,
+ unsigned int dest_x, unsigned int dest_y,
+ unsigned int width,unsigned int height)
{
struct vram_buffer *buf;
int line, endline, lineincr, startcolumn, endcolumn, columnincr, column;
-uint32_t dst, src;
+unsigned int dst, src;
trace_artist_block_move(source_x, source_y, dest_x, dest_y, width, height);
@@ -481,6 +484,12 @@ static void block_move(ARTISTState *s, int source_x, int
source_y, int dest_x,
}
buf = >vram_buffer[ARTIST_BUFFER_AP];
+if (height > buf->height) {
+height = buf->height;
+}
+if (width > buf->width) {
+width = buf->width;
+}
if (dest_y > source_y) {
/* move down */
@@ -507,24 +516,27 @@ static void block_move(ARTISTState *s, int source_x, int
source_y, int dest_x,
}
for ( ; line != endline; line += lineincr) {
-src = source_x + ((line + source_y) * buf->width);
-dst = dest_x + ((line + dest_y) * buf->width);
+src = source_x + ((line + source_y) * buf->width) + startcolumn;
+dst = dest_x + ((line + dest_y) * buf->width) + startcolumn;
for (column = startcolumn; column != endcolumn; column += columnincr) {
-if (dst + column > buf->size || src + column > buf->size) {
+if (dst >= buf->size || src >= buf->size) {
continue;
}
-artist_rop8(s, buf, dst + column, buf->data[src + column]);
+artist_rop8(s, buf,
[PULL v5 08/12] Revert "hw/display/artist: Avoid drawing line when nothing to display"
This reverts commit b0f6455feac97e41045ee394e11c24d92c370f6e.
It's wrong. A line could even be a dot.
Signed-off-by: Helge Deller
---
hw/display/artist.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/hw/display/artist.c b/hw/display/artist.c
index 47de17b9e9..f37aa9eb49 100644
--- a/hw/display/artist.c
+++ b/hw/display/artist.c
@@ -591,9 +591,6 @@ static void draw_line(ARTISTState *s, int x1, int y1, int
x2, int y2,
} else {
dy = y1 - y2;
}
-if (!dx || !dy) {
-return;
-}
c1 = false;
if (dy > dx) {
--
2.21.3
[PULL v5 07/12] hw/display/artist: Refactor artist_rop8() to avoid buffer over-run
From: Philippe Mathieu-Daudé
Invalid I/O writes can craft an offset out of the vram_buffer range.
Instead of passing an unsafe pointer to artist_rop8(), pass the vram_buffer and
the offset. We can now check if the offset is in range before accessing it.
We avoid:
Program terminated with signal SIGSEGV, Segmentation fault.
284 *dst &= ~plane_mask;
(gdb) bt
#0 0x56367b2085c0 in artist_rop8 (s=0x56367d38b510, dst=0x7f9f972f
, val=0 '\000') at
hw/display/artist.c:284
#1 0x56367b209325 in draw_line (s=0x56367d38b510, x1=-20480, y1=-1,
x2=0, y2=17920, update_start=true, skip_pix=-1, max_pix=-1) at
hw/display/artist.c:646
Reported-by: LLVM libFuzzer
Buglink: https://bugs.launchpad.net/qemu/+bug/1880326
Signed-off-by: Philippe Mathieu-Daudé
Signed-off-by: Helge Deller
---
hw/display/artist.c | 40 +---
1 file changed, 25 insertions(+), 15 deletions(-)
diff --git a/hw/display/artist.c b/hw/display/artist.c
index a206afe641..47de17b9e9 100644
--- a/hw/display/artist.c
+++ b/hw/display/artist.c
@@ -273,11 +273,20 @@ static artist_rop_t artist_get_op(ARTISTState *s)
return (s->image_bitmap_op >> 8) & 0xf;
}
-static void artist_rop8(ARTISTState *s, uint8_t *dst, uint8_t val)
+static void artist_rop8(ARTISTState *s, struct vram_buffer *buf,
+int offset, uint8_t val)
{
-
const artist_rop_t op = artist_get_op(s);
-uint8_t plane_mask = s->plane_mask & 0xff;
+uint8_t plane_mask;
+uint8_t *dst;
+
+if (offset < 0 || offset >= buf->size) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "rop8 offset:%d bufsize:%u\n", offset, buf->size);
+return;
+}
+dst = buf->data + offset;
+plane_mask = s->plane_mask & 0xff;
switch (op) {
case ARTIST_ROP_CLEAR:
@@ -375,7 +384,7 @@ static void vram_bit_write(ARTISTState *s, int posx, int
posy, bool incr_x,
for (i = 0; i < pix_count; i++) {
uint32_t off = offset + pix_count - 1 - i;
if (off < buf->size) {
-artist_rop8(s, p + off,
+artist_rop8(s, buf, off,
(data & 1) ? (s->plane_mask >> 24) : 0);
}
data >>= 1;
@@ -395,7 +404,7 @@ static void vram_bit_write(ARTISTState *s, int posx, int
posy, bool incr_x,
s->vram_bitmask & (1 << (28 + i))) {
uint32_t off = offset + 3 - i;
if (off < buf->size) {
-artist_rop8(s, p + off, data8[ROP8OFF(i)]);
+artist_rop8(s, buf, off, data8[ROP8OFF(i)]);
}
}
}
@@ -424,10 +433,10 @@ static void vram_bit_write(ARTISTState *s, int posx, int
posy, bool incr_x,
if (!(s->image_bitmap_op & 0x2000) ||
(vram_bitmask & mask)) {
if (data & mask) {
-artist_rop8(s, p + offset + i, s->fg_color);
+artist_rop8(s, buf, offset + i, s->fg_color);
} else {
if (!(s->image_bitmap_op & 0x1002)) {
-artist_rop8(s, p + offset + i, s->bg_color);
+artist_rop8(s, buf, offset + i, s->bg_color);
}
}
}
@@ -505,7 +514,7 @@ static void block_move(ARTISTState *s, int source_x, int
source_y, int dest_x,
if (dst + column > buf->size || src + column > buf->size) {
continue;
}
-artist_rop8(s, buf->data + dst + column, buf->data[src + column]);
+artist_rop8(s, buf, dst + column, buf->data[src + column]);
}
}
@@ -546,7 +555,7 @@ static void fill_window(ARTISTState *s, int startx, int
starty,
offset = y * s->width;
for (x = startx; x < startx + width; x++) {
-artist_rop8(s, buf->data + offset + x, color);
+artist_rop8(s, buf, offset + x, color);
}
}
artist_invalidate_lines(buf, starty, height);
@@ -559,7 +568,6 @@ static void draw_line(ARTISTState *s, int x1, int y1, int
x2, int y2,
uint8_t color;
int dx, dy, t, e, x, y, incy, diago, horiz;
bool c1;
-uint8_t *p;
trace_artist_draw_line(x1, y1, x2, y2);
@@ -628,16 +636,18 @@ static void draw_line(ARTISTState *s, int x1, int y1, int
x2, int y2,
color = artist_get_color(s);
do {
+int ofs;
+
if (c1) {
-p = buf->data + x * s->width + y;
+ofs = x * s->width + y;
} else {
-p = buf->data + y * s->width + x;
+ofs = y * s->width + x;
}
if (skip_pix > 0) {
skip_pix--;
} else {
-artist_rop8(s, p, color);
+artist_rop8(s, buf, ofs, color);
}
if (e > 0) {
@@ -771,10 +781,10 @@ static void font_write16(ARTISTState *s, uint16_t val)
for (i = 0; i < 16; i++) {
mask =
Re: [PATCH 05/10] spapr: make ibm,max-associativity-domains scale with user input
On 8/19/20 11:55 PM, David Gibson wrote:
On Fri, Aug 14, 2020 at 05:54:19PM -0300, Daniel Henrique Barboza wrote:
The ibm,max-associativity-domains is considering that only a single
associativity domain can exist in the same NUMA level. This is true
today because we do not support any type of NUMA distance user
customization, and all nodes are in the same distance to each other.
To enhance NUMA distance support in the pSeries machine we need to
make this limit flexible. This patch rewrites the max-associativity
logic to consider that multiple associativity domains can co-exist
in the same NUMA level. We're using the legacy_numa() helper to
avoid leaking unneeded guest changes.
Hrm. I find the above a bit hard to understand. Having the limit be
one less than the number of nodes at every level except the last seems
kind of odd to me.
I took a bit to reply on this because I was reconsidering this logic.
I tried to "not be greedy" with this maximum number and ended up doing
something that breaks in a simple scenario. Today, in a single conf with
a single NUMA node with a single CPU, and say 2 GPUs, given that all GPUs
are in their own associativity domains, we would have something like:
cpu0: 0 0 0 0 0 0
gpu1: gpu_1 gpu_1 gpu_1 gpu_1
gpu2: gpu_2 gpu_2 gpu_2 gpu_2
This would already break apart what I did there. I think we should simplify
and just set maxdomains to be all nodes in all levels, like we do today
but using spapr->gpu_numa_id as an alias to maxnodes.
Thanks,
DHB
Signed-off-by: Daniel Henrique Barboza
---
hw/ppc/spapr.c | 18 --
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 073a59c47d..b0c4b80a23 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -919,13 +919,20 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, void
*fdt)
cpu_to_be32(SPAPR_MEMORY_BLOCK_SIZE & 0x),
cpu_to_be32(ms->smp.max_cpus / ms->smp.threads),
};
-uint32_t maxdomain = cpu_to_be32(spapr->extra_numa_nodes > 1 ? 1 : 0);
+
+/* The maximum domains for a given NUMA level, supposing that every
+ * additional NUMA node belongs to the same domain (aside from the
+ * 4th level, where we must support all available NUMA domains), is
+ * total number of domains - 1. */
+uint32_t total_nodes_number = ms->numa_state->num_nodes +
+ spapr->extra_numa_nodes;
+uint32_t maxdomain = cpu_to_be32(total_nodes_number - 1);
uint32_t maxdomains[] = {
cpu_to_be32(4),
maxdomain,
maxdomain,
maxdomain,
-cpu_to_be32(ms->numa_state->num_nodes + spapr->extra_numa_nodes),
+cpu_to_be32(total_nodes_number),
};
_FDT(rtas = fdt_add_subnode(fdt, 0, "rtas"));
@@ -962,6 +969,13 @@ static void spapr_dt_rtas(SpaprMachineState *spapr, void
*fdt)
qemu_hypertas->str, qemu_hypertas->len));
g_string_free(qemu_hypertas, TRUE);
+if (spapr_machine_using_legacy_numa(spapr)) {
+maxdomain = cpu_to_be32(spapr->extra_numa_nodes > 1 ? 1 : 0);
+maxdomains[1] = maxdomain;
+maxdomains[2] = maxdomain;
+maxdomains[3] = maxdomain;
+}
+
if (smc->pre_5_1_assoc_refpoints) {
nr_refpoints = 2;
}
[PULL v5 06/12] hw/display/artist: Check offset in draw_line to avoid buffer over-run
From: Philippe Mathieu-Daudé
Invalid I/O writes can craft an offset out of the vram_buffer range.
We avoid:
Program terminated with signal SIGSEGV, Segmentation fault.
284 *dst &= ~plane_mask;
(gdb) bt
#0 0x55d5dccdc5c0 in artist_rop8 (s=0x55d5defee510, dst=0x7f8e84ed8216
, val=0 '\000') at
hw/display/artist.c:284
#1 0x55d5dccdcf83 in fill_window (s=0x55d5defee510, startx=22,
starty=5674, width=65, height=5697) at hw/display/artist.c:551
#2 0x55d5dccddfb9 in artist_reg_write (opaque=0x55d5defee510,
addr=1051140, val=4265537, size=4) at hw/display/artist.c:902
#3 0x55d5dcb42a7c in memory_region_write_accessor (mr=0x55d5defeea10,
addr=1051140, value=0x7ffe57db08c8, size=4, shift=0, mask=4294967295,
attrs=...) at memory.c:483
Reported-by: LLVM libFuzzer
Signed-off-by: Philippe Mathieu-Daudé
Signed-off-by: Helge Deller
---
hw/display/artist.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/hw/display/artist.c b/hw/display/artist.c
index de56200dbf..a206afe641 100644
--- a/hw/display/artist.c
+++ b/hw/display/artist.c
@@ -555,7 +555,7 @@ static void fill_window(ARTISTState *s, int startx, int
starty,
static void draw_line(ARTISTState *s, int x1, int y1, int x2, int y2,
bool update_start, int skip_pix, int max_pix)
{
-struct vram_buffer *buf;
+struct vram_buffer *buf = >vram_buffer[ARTIST_BUFFER_AP];
uint8_t color;
int dx, dy, t, e, x, y, incy, diago, horiz;
bool c1;
@@ -563,6 +563,12 @@ static void draw_line(ARTISTState *s, int x1, int y1, int
x2, int y2,
trace_artist_draw_line(x1, y1, x2, y2);
+if (x1 * y1 >= buf->size || x2 * y2 >= buf->size) {
+qemu_log_mask(LOG_GUEST_ERROR,
+ "draw_line (%d,%d) (%d,%d)\n", x1, y1, x2, y2);
+return;
+}
+
if (update_start) {
s->vram_start = (x2 << 16) | y2;
}
@@ -620,7 +626,6 @@ static void draw_line(ARTISTState *s, int x1, int y1, int
x2, int y2,
x = x1;
y = y1;
color = artist_get_color(s);
-buf = >vram_buffer[ARTIST_BUFFER_AP];
do {
if (c1) {
--
2.21.3
[PULL v5 12/12] hw/display/artist: Fix invalidation of lines near screen border
From: Sven Schnelle
If parts of the invalidated screen lines are outside of the VRAM buffer,
the code skips the whole invalidate. This is incorrect when only parts
of the buffer are invisble - which is the case when the mouse cursor is
located near the screen border.
Signed-off-by: Sven Schnelle
Signed-off-by: Helge Deller
---
hw/display/artist.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/hw/display/artist.c b/hw/display/artist.c
index a959b2c158..71982559c6 100644
--- a/hw/display/artist.c
+++ b/hw/display/artist.c
@@ -206,7 +206,12 @@ static void artist_invalidate_lines(struct vram_buffer
*buf,
int starty, int height)
{
int start = starty * buf->width;
-int size = height * buf->width;
+int size;
+
+if (starty + height > buf->height)
+height = buf->height - starty;
+
+size = height * buf->width;
if (start + size <= buf->size) {
memory_region_set_dirty(>mr, start, size);
--
2.21.3
[PULL v5 01/12] hw/hppa: Sync hppa_hardware.h file with SeaBIOS sources
The hppa_hardware.h file is shared with SeaBIOS. Sync it.
Acked-by: Richard Henderson
Signed-off-by: Helge Deller
---
hw/hppa/hppa_hardware.h | 6 ++
hw/hppa/lasi.c | 2 --
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/hw/hppa/hppa_hardware.h b/hw/hppa/hppa_hardware.h
index 4a2fe2df60..cdb7fa6240 100644
--- a/hw/hppa/hppa_hardware.h
+++ b/hw/hppa/hppa_hardware.h
@@ -17,6 +17,7 @@
#define LASI_UART_HPA 0xffd05000
#define LASI_SCSI_HPA 0xffd06000
#define LASI_LAN_HPA0xffd07000
+#define LASI_RTC_HPA0xffd09000
#define LASI_LPT_HPA0xffd02000
#define LASI_AUDIO_HPA 0xffd04000
#define LASI_PS2KBD_HPA 0xffd08000
@@ -37,10 +38,15 @@
#define PORT_PCI_CMD(PCI_HPA + DINO_PCI_ADDR)
#define PORT_PCI_DATA (PCI_HPA + DINO_CONFIG_DATA)
+/* QEMU fw_cfg interface port */
+#define QEMU_FW_CFG_IO_BASE (MEMORY_HPA + 0x80)
+
#define PORT_SERIAL1(DINO_UART_HPA + 0x800)
#define PORT_SERIAL2(LASI_UART_HPA + 0x800)
#define HPPA_MAX_CPUS 8 /* max. number of SMP CPUs */
#define CPU_CLOCK_MHZ 250 /* emulate a 250 MHz CPU */
+#define CPU_HPA_CR_REG 7 /* store CPU HPA in cr7 (SeaBIOS internal) */
+
#endif
diff --git a/hw/hppa/lasi.c b/hw/hppa/lasi.c
index 19974034f3..ffcbb988b8 100644
--- a/hw/hppa/lasi.c
+++ b/hw/hppa/lasi.c
@@ -54,8 +54,6 @@
#define LASI_CHIP(obj) \
OBJECT_CHECK(LasiState, (obj), TYPE_LASI_CHIP)
-#define LASI_RTC_HPA(LASI_HPA + 0x9000)
-
typedef struct LasiState {
PCIHostState parent_obj;
--
2.21.3
[PULL v5 11/12] hw/display/artist: Fix invalidation of lines in artist_draw_line()
From: Sven Schnelle
The old code didn't invalidate correctly when vertical lines were drawn.
Fix this and move the invalidation out of the loop.
Signed-off-by: Sven Schnelle
Signed-off-by: Helge Deller
---
hw/display/artist.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/display/artist.c b/hw/display/artist.c
index 44bb67bbc3..a959b2c158 100644
--- a/hw/display/artist.c
+++ b/hw/display/artist.c
@@ -662,7 +662,6 @@ static void draw_line(ARTISTState *s,
}
if (e > 0) {
-artist_invalidate_lines(buf, y, 1);
y += incy;
e += diago;
} else {
@@ -670,6 +669,10 @@ static void draw_line(ARTISTState *s,
}
x++;
} while (x <= x2 && (max_pix == -1 || --max_pix > 0));
+if (c1)
+artist_invalidate_lines(buf, x, dy+1);
+else
+artist_invalidate_lines(buf, y, dx+1);
}
static void draw_line_pattern_start(ARTISTState *s)
--
2.21.3
[PULL v5 00/12] The following changes since commit 3461487523b897d324e8d91f3fd20ed55f849544:
Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200727'
into staging (2020-07-28 18:43:48 +0100)
are available in the Git repository at:
g...@github.com:hdeller/qemu-hppa.git tags/target-hppa-v3-pull-request
for you to fetch changes up to 2f8cd515477edab1cbf38ecbdbfa2cac13ce1550:
hw/display/artist: Fix invalidation of lines near screen border (2020-08-26
23:04:00 +0200)
artist out of bounds fixes
Helge Deller (7):
hw/hppa: Sync hppa_hardware.h file with SeaBIOS sources
seabios-hppa: Update to SeaBIOS hppa version 1
hw/hppa: Implement proper SeaBIOS version check
hw/hppa/lasi: Don't abort on invalid IMR value
Revert "hw/display/artist: Avoid drawing line when nothing to display"
hw/display/artist: Prevent out of VRAM buffer accesses
hw/display/artist: Unbreak size mismatch memory accesses
Philippe Mathieu-Daudé (2):
hw/display/artist: Check offset in draw_line to avoid buffer over-run
hw/display/artist: Refactor artist_rop8() to avoid buffer over-run
Sven Schnelle (3):
hw/display/artist.c: fix out of bounds check
hw/display/artist: Fix invalidation of lines in artist_draw_line()
hw/display/artist: Fix invalidation of lines near screen border
hw/display/artist.c | 186 --
hw/hppa/hppa_hardware.h | 6 ++
hw/hppa/lasi.c| 10 ++-
hw/hppa/machine.c | 22 ++
pc-bios/hppa-firmware.img | Bin 766136 -> 783192 bytes
roms/seabios-hppa | 2 +-
6 files changed, 149 insertions(+), 77 deletions(-)
-
target-hppa fixes v4
A few fixes for target-hppa:
* Fix the SeaBIOS-hppa firmware build with gcc-10 on Debian
* Fix the SeaBIOS-hppa firmware to boot NetBSD again
* Fix many artist framebuffer out-of-bounds accesses as found by Alexander
Bulekov
* Fix artist memory access bugs due to commit 5d971f9e6725 ("memory: Revert
"memory: accept mismatching sizes in memory_region_access_valid")
* Fix various artist screen updates when running dtwm on HP-UX
In addition the SeaBIOS-hppa firmware now includes a version check to prevent
starting when it's incompatible to the emulated qemu hardware.
The patchset can be pulled from
https://github.com/hdeller/qemu-hppa.git target-hppa
Helge
Changes to v3:
* Fix format string error in lasi on Win32
* Fix memory fallouts due to commit 5d971f9e6725
* Fix graphic rendering bugs and screen refreshes with dtwm on HP-UX
Changes to v2:
* added more Acks by Richard Henderson
* added more artist framebuffer out-of-bounds fixes by
Philippe Mathieu-Daudé which were reported by Alexander Bulekov
* fix NetBSD boot
Changes to v1:
* added Ack by Richard Henderson for the first patch
* revised out of bounds check based on Richards feedback
Helge Deller (7):
hw/hppa: Sync hppa_hardware.h file with SeaBIOS sources
seabios-hppa: Update to SeaBIOS hppa version 1
hw/hppa: Implement proper SeaBIOS version check
hw/hppa/lasi: Don't abort on invalid IMR value
Revert "hw/display/artist: Avoid drawing line when nothing to display"
hw/display/artist: Prevent out of VRAM buffer accesses
hw/display/artist: Unbreak size mismatch memory accesses
Philippe Mathieu-Daudé (2):
hw/display/artist: Check offset in draw_line to avoid buffer over-run
hw/display/artist: Refactor artist_rop8() to avoid buffer over-run
Sven Schnelle (3):
hw/display/artist.c: fix out of bounds check
hw/display/artist: Fix invalidation of lines in artist_draw_line()
hw/display/artist: Fix invalidation of lines near screen border
hw/display/artist.c | 186 +++---
hw/hppa/hppa_hardware.h | 6 ++
hw/hppa/lasi.c| 10 +-
hw/hppa/machine.c | 22 +
pc-bios/hppa-firmware.img | Bin 766136 -> 783192 bytes
roms/seabios-hppa | 2 +-
6 files changed, 149 insertions(+), 77 deletions(-)
--
2.21.3
[PULL v5 05/12] hw/hppa/lasi: Don't abort on invalid IMR value
NetBSD initializes the LASI IMR value with 0x to disable all LASI interrupts. This triggered an assert() and stopped the emulation. By replacing the check with a warning in the guest log we now allow NetBSD to boot again. Signed-off-by: Helge Deller --- hw/hppa/lasi.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/hw/hppa/lasi.c b/hw/hppa/lasi.c index ffcbb988b8..194aa3e619 100644 --- a/hw/hppa/lasi.c +++ b/hw/hppa/lasi.c @@ -11,6 +11,7 @@ #include "qemu/osdep.h" #include "qemu/units.h" +#include "qemu/log.h" #include "qapi/error.h" #include "cpu.h" #include "trace.h" @@ -170,8 +171,11 @@ static MemTxResult lasi_chip_write_with_attrs(void *opaque, hwaddr addr, /* read-only. */ break; case LASI_IMR: -s->imr = val; /* 0x20 ?? */ -assert((val & LASI_IRQ_BITS) == val); +s->imr = val; +if (((val & LASI_IRQ_BITS) != val) && (val != 0x)) +qemu_log_mask(LOG_GUEST_ERROR, +"LASI: tried to set invalid %lx IMR value.\n", +(unsigned long) val); break; case LASI_IPR: /* Any write to IPR clears the register. */ -- 2.21.3
[PULL v5 03/12] hw/hppa: Implement proper SeaBIOS version check
It's important that the SeaBIOS hppa firmware is at least at a minimal
level to ensure proper interaction between qemu and firmware.
Implement a proper firmware version check by telling SeaBIOS via the
fw_cfg interface which minimal SeaBIOS version is required by this
running qemu instance. If the firmware detects that it's too old, it
will stop.
Signed-off-by: Helge Deller
---
hw/hppa/machine.c | 22 ++
1 file changed, 22 insertions(+)
diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index 49155537cd..90aeefe2a4 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -25,6 +25,8 @@
#define MAX_IDE_BUS 2
+#define MIN_SEABIOS_HPPA_VERSION 1 /* require at least this fw version */
+
static ISABus *hppa_isa_bus(void)
{
ISABus *isa_bus;
@@ -56,6 +58,23 @@ static uint64_t cpu_hppa_to_phys(void *opaque, uint64_t addr)
static HPPACPU *cpu[HPPA_MAX_CPUS];
static uint64_t firmware_entry;
+static FWCfgState *create_fw_cfg(MachineState *ms)
+{
+FWCfgState *fw_cfg;
+uint64_t val;
+
+fw_cfg = fw_cfg_init_mem(QEMU_FW_CFG_IO_BASE, QEMU_FW_CFG_IO_BASE + 4);
+fw_cfg_add_i16(fw_cfg, FW_CFG_NB_CPUS, ms->smp.cpus);
+fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, HPPA_MAX_CPUS);
+fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, ram_size);
+
+val = cpu_to_le64(MIN_SEABIOS_HPPA_VERSION);
+fw_cfg_add_file(fw_cfg, "/etc/firmware-min-version",
+g_memdup(, sizeof(val)), sizeof(val));
+
+return fw_cfg;
+}
+
static void machine_hppa_init(MachineState *machine)
{
const char *kernel_filename = machine->kernel_filename;
@@ -118,6 +137,9 @@ static void machine_hppa_init(MachineState *machine)
115200, serial_hd(0), DEVICE_BIG_ENDIAN);
}
+/* fw_cfg configuration interface */
+create_fw_cfg(machine);
+
/* SCSI disk setup. */
dev = DEVICE(pci_create_simple(pci_bus, -1, "lsi53c895a"));
lsi53c8xx_handle_legacy_cmdline(dev);
--
2.21.3
[PULL v5 10/12] hw/display/artist: Unbreak size mismatch memory accesses
Commit 5d971f9e6725 ("memory: Revert "memory: accept mismatching sizes
in memory_region_access_valid") broke the artist driver in a way that
the dtwm window manager on HP-UX rendered wrong.
Fixes: 5d971f9e6725 ("memory: Revert "memory: accept mismatching sizes in
memory_region_access_valid")
Signed-off-by: Sven Schnelle
Signed-off-by: Helge Deller
---
hw/display/artist.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/hw/display/artist.c b/hw/display/artist.c
index 46eaa10dae..44bb67bbc3 100644
--- a/hw/display/artist.c
+++ b/hw/display/artist.c
@@ -1237,20 +1237,16 @@ static const MemoryRegionOps artist_reg_ops = {
.read = artist_reg_read,
.write = artist_reg_write,
.endianness = DEVICE_NATIVE_ENDIAN,
-.valid = {
-.min_access_size = 1,
-.max_access_size = 4,
-},
+.impl.min_access_size = 1,
+.impl.max_access_size = 4,
};
static const MemoryRegionOps artist_vram_ops = {
.read = artist_vram_read,
.write = artist_vram_write,
.endianness = DEVICE_NATIVE_ENDIAN,
-.valid = {
-.min_access_size = 1,
-.max_access_size = 4,
-},
+.impl.min_access_size = 1,
+.impl.max_access_size = 4,
};
static void artist_draw_cursor(ARTISTState *s)
--
2.21.3
Re: [PULL v2 00/12] target-hppa fixes pull request v2
On Mon, 10 Aug 2020 at 14:24, Helge Deller wrote:
>
> Please pull from
> https://github.com/hdeller/qemu-hppa.git target-hppa
> to fix those bugs in target-hppa:
>
> * Fix the SeaBIOS-hppa firmware build with gcc-10 on Debian
>
> * Fix the SeaBIOS-hppa firmware to boot NetBSD again
>
> * Fix many artist framebuffer out-of-bounds accesses as found by Alexander
> Bulekov
>
> * Fix artist memory access bugs due to commit 5d971f9e6725 ("memory: Revert
> "memory: accept mismatching sizes in memory_region_access_valid")
>
> * Fix various artist screen updates when running dtwm on HP-UX
>
> In addition the SeaBIOS-hppa firmware now includes a version check to prevent
> starting when it's incompatible to the emulated qemu hardware.
Hi; this (via the tag target-hppa-v2-pull-request) has
a format string issue on windows and 32bit:
In file included from ../../hw/hppa/lasi.c:14:0:
../../hw/hppa/lasi.c: In function 'lasi_chip_write_with_attrs':
../../hw/hppa/lasi.c:177:17: error: format '%lx' expects argument of
type 'long unsigned int', but argument 2 has type 'uint64_t {aka long
long unsigned int}' [-Werror=format=]
"LASI: tried to set invalid %lx IMR value.\n", val);
^
/home/peter.maydell/qemu/include/qemu/log.h:120:22: note: in
definition of macro 'qemu_log_mask'
qemu_log(FMT, ## __VA_ARGS__); \
^~~
PS: also I've just noticed that your pullreq email doesn't
have the standard git request-pull cover letter phrasing;
if you don't use that then my email filters will not notice
your pullreqs.
https://wiki.qemu.org/Contribute/SubmitAPullRequest is worth
reading if you haven't found it already.
thanks
-- PMM
Re: [PATCH 0/5] hw/avr: Start using the Clock API
Tested-by: Michael Rolnik On Fri, Aug 14, 2020 at 7:39 PM Philippe Mathieu-Daudé wrote: > In this series we slowly start to use the recently added > Clock API in the AVR ATmega MCU. > > As the Clock Control Unit is not yet modelled, we simply > connect the XTAL sink to the UART and Timer sources. > > Philippe Mathieu-Daudé (5): > hw/avr/atmega: Introduce the I/O clock > hw/timer/avr_timer16: Use the Clock API > hw/char/avr_usart: Restrict register definitions to source > hw/char/avr_usart: Use the Clock API > hw/char/avr_usart: Trace baudrate changes > > hw/avr/atmega.h| 2 ++ > include/hw/char/avr_usart.h| 32 ++- > include/hw/timer/avr_timer16.h | 3 ++- > hw/avr/atmega.c| 8 -- > hw/char/avr_usart.c| 46 ++ > hw/timer/avr_timer16.c | 12 +++-- > hw/char/trace-events | 3 +++ > 7 files changed, 65 insertions(+), 41 deletions(-) > > -- > 2.21.3 > > -- Best Regards, Michael Rolnik
Re: [PATCH 08/10] spapr: introduce SpaprMachineClass::numa_assoc_domains
On 8/20/20 1:26 AM, David Gibson wrote:
On Fri, Aug 14, 2020 at 05:54:22PM -0300, Daniel Henrique Barboza wrote:
We can't use the input from machine->numa_state->nodes directly
in the pSeries machine because PAPR does not work with raw distance
values, like ACPI SLIT does. We need to determine common
associativity domains, based on similar performance/distance of the
resources, and set these domains in the associativy array that goes
to the FDT of each resource.
To ease the translation between regular ACPI NUMA distance info
to our PAPR dialect, let's create a matrix called numa_assoc_domains
in the SpaprMachineClass. This matrix will be initiated during
machine init, where we will read NUMA information from user input,
apply a heuristic to determine the associativity domains for each node,
then populate numa_assoc_domains accordingly.
The changes are mostly centered in the spapr_set_associativity()
helper that will use the values of numa_assoc_domains instead of
using 0x0, with spapr_dt_dynamic_reconfiguration_memory() and
h_home_node_associativity() being the exceptions.
To keep the changes under control, we'll plug in the matrix usage
in the existing code first. The actual heuristic to determine
the associativity domains for each NUMA node will come in a follow-up
patch.
Note that the matrix is initiated with zeros, meaning that there is
no guest changes implemented in this patch. We'll keep these
changes from legacy NUMA guests by not initiating the matrix
in these cases.
Signed-off-by: Daniel Henrique Barboza
IIUC, what this is basically doing is that instead of doing the
translation from qemu's internal NUMA representation to PAPRs at the
point(s) we emit the PAPR information, we're moving it to persistent
data we calculate for each (qemu) numa node then just copy out when we
emit the information?
Yes. The original intention was to not go straight from QEMU numa_state
to associativity arrays, given that for each numa_state entry we need to
(1) round the value up to what the kernel understands (10,20,40,80,160)
and (2) determine the associativity domains for each position of the
associativity array. I could made basically the same thing on top of
the existing numa_state info, but I wasn't sure if overwriting it was
a good idea.
That could be a reasonable idea, and indeed the rest of the series
might be easier to understand if this change were earlier in the
series. In particular it means we might be able localise all the
hacks for calculating the right vectors depending on machine
version/options into one place.
This didn't cross my mind when I first wrote it but it is a good idea to
put all the NUMA related code into the same function. I considered creating
a spapr_numa.c to avoid putting more stuff into spapr.c but, for the
amount of code being added, I didn't think it was justified.
A couple of nits, though:
---
hw/ppc/spapr.c| 46 +++
hw/ppc/spapr_hcall.c | 13 --
hw/ppc/spapr_nvdimm.c | 13 +-
hw/ppc/spapr_pci.c| 3 ++-
include/hw/ppc/spapr.h| 7 +-
include/hw/ppc/spapr_nvdimm.h | 5 ++--
6 files changed, 59 insertions(+), 28 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index b80a6f6936..4f50ab21ee 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -201,8 +201,13 @@ static int spapr_fixup_cpu_smt_dt(void *fdt, int offset,
PowerPCCPU *cpu,
return ret;
}
-void spapr_set_associativity(uint32_t *assoc, int node_id, int cpu_index)
+void spapr_set_associativity(uint32_t *assoc, int node_id, int cpu_index,
+ MachineState *machine)
{
+SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine);
+uint8_t assoc_domain1 = smc->numa_assoc_domains[node_id][0];
+uint8_t assoc_domain2 = smc->numa_assoc_domains[node_id][1];
+uint8_t assoc_domain3 = smc->numa_assoc_domains[node_id][2];
uint8_t assoc_size = 0x4;
if (cpu_index >= 0) {
@@ -211,17 +216,18 @@ void spapr_set_associativity(uint32_t *assoc, int
node_id, int cpu_index)
}
assoc[0] = cpu_to_be32(assoc_size);
-assoc[1] = cpu_to_be32(0x0);
-assoc[2] = cpu_to_be32(0x0);
-assoc[3] = cpu_to_be32(0x0);
+assoc[1] = cpu_to_be32(assoc_domain1);
+assoc[2] = cpu_to_be32(assoc_domain2);
+assoc[3] = cpu_to_be32(assoc_domain3);
assoc[4] = cpu_to_be32(node_id);
So spapr_set_associativity() is already a slightly dangerous function,
because the required buffer space for 'assoc' varies in a non-obvious
way depending on if cpu_index is >= 0. I didn't comment on that when
it was introduced, because it's not really any worse than what it
replaced.
But with this change, I think we can do better. I'd suggest storing
the full PAPR associativity vector for each qemu numa node verbatim,
so it can just be copied straight into the device tree without
interpretation. Then the helper can actually do the
Help with usb-host hostdevice property
Hi, I'm trying to use the new usb-host hostdevice property that was added here: https://git.qemu.org/?p=qemu.git;a=commit;h=9f815e83e983d247a3cd67579d2d9c1765adc644 This is the command line that I am using: qemu-system-x86_64 -enable-kvm -hda arch-zoom.qcow2 -m 4G -device qemu-xhci,id=xhci -device usb-host,bus=xhci.0,hostdevice=/dev/bus/usb/002/004 -device intel-hda -device hda-duplex -vga virtio My user has permissions to access /dev/bus/usb/002/004 -- I am trying to pass-through this device: Bus 002 Device 004: ID 04f2:b449 Chicony Electronics Co., Ltd Integrated Camera When I boot up the guest I get this: usb 1-1: Invalid ep0 maxpacket: 64 usb usb1-port1: unable to enumerate USB device This works though: qemu-system-x86_64 -enable-kvm -hda arch-zoom.qcow2 -m 4G -device qemu-xhci,id=xhci -device usb-host,bus=xhci.0,vendorid=0x04f2,productid=0xb449 -device intel-hda -device hda-duplex -vga virtio I am using QEMU 5.1.0 on Arch Linux. Arch Linux is running on the guest as well as on the host. Any ideas what I'm doing wrong here please? Thanks, Diego
Re: [PATCH v2 1/1] core/register: Specify instance_size in the TypeInfo
On Tue, Aug 25, 2020 at 10:30:59AM -0700, Alistair Francis wrote:
> 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 = [index];
>
> -*r = (RegisterInfo) {
> -.data = data + data_size * index,
> -.data_size = data_size,
> -.access = [i],
> -.opaque = owner,
> -};
> -register_init(r);
> +if (data + data_size * index == 0 || ![i]) {
Do you know what's the goal of this check?
Can `data` or `rae` be NULL? If not, it seems impossible for
this condition to be true. If they can, this seems to be a weird
and fragile way of checking for NULL arguments.
> +continue;
> +}
> +
> +/* Init the register, this will zero it. */
> +object_initialize((void *)r, sizeof(*r), TYPE_REGISTER);
> +
> +/* Set the properties of the register */
> +r->data = data + data_size * index;
> +r->data_size = data_size;
> +r->access = [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
>
--
Eduardo
Re: [PATCH 1/4] sev/i386: Add initial support for SEV-ES
On 8/26/20 2:07 PM, Connor Kuehl wrote:
On 8/25/20 2:05 PM, Tom Lendacky wrote:
From: Tom Lendacky
Provide initial support for SEV-ES. This includes creating a function to
indicate the guest is an SEV-ES guest (which will return false until all
support is in place), performing the proper SEV initialization and
ensuring that the guest CPU state is measured as part of the launch.
Co-developed-by: Jiri Slaby
Signed-off-by: Jiri Slaby
Signed-off-by: Tom Lendacky
Hi Tom!
Hi Connor,
Overall I think the patch set looks good. I mainly just have 1 question
regarding some error handling and a couple of checkpatch related messages.
Ugh, I was positive I ran checkpatch, but obviously I didn't.
---
target/i386/cpu.c | 1 +
target/i386/sev-stub.c | 5 +
target/i386/sev.c | 46 --
target/i386/sev_i386.h | 1 +
4 files changed, 51 insertions(+), 2 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 588f32e136..bbbe581d35 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5969,6 +5969,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t
index, uint32_t count,
break;
case 0x801F:
*eax = sev_enabled() ? 0x2 : 0;
+ *eax |= sev_es_enabled() ? 0x8 : 0;
*ebx = sev_get_cbit_position();
*ebx |= sev_get_reduced_phys_bits() << 6;
*ecx = 0;
diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index 88e3f39a1e..040ac90563 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -49,3 +49,8 @@ SevCapability *sev_get_capabilities(Error **errp)
error_setg(errp, "SEV is not available in this QEMU");
return NULL;
}
+
+bool sev_es_enabled(void)
I don't think this bothers checkpatch, but it'd be consistent with the
rest of your series if this function put the return type on the line above.
I was being consistent with the file that it is in where all the other
functions are defined this way.
+{
+ return false;
+}
diff --git a/target/i386/sev.c b/target/i386/sev.c
index c3ecf86704..6c9cd0854b 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -359,6 +359,12 @@ sev_enabled(void)
return !!sev_guest;
}
+bool
+sev_es_enabled(void)
+{
+ return false;
+}
+
uint64_t
sev_get_me_mask(void)
{
@@ -578,6 +584,22 @@ sev_launch_update_data(SevGuestState *sev, uint8_t
*addr, uint64_t len)
return ret;
}
+static int
+sev_launch_update_vmsa(SevGuestState *sev)
+{
+ int ret, fw_error;
+
+ ret = sev_ioctl(sev->sev_fd, KVM_SEV_LAUNCH_UPDATE_VMSA, NULL,
_error);
+ if (ret) {
+ error_report("%s: LAUNCH_UPDATE_VMSA ret=%d fw_error=%d '%s'",
+ __func__, ret, fw_error, fw_error_to_str(fw_error));
+ goto err;
+ }
+
+err:
+ return ret;
+}
+
static void
sev_launch_get_measure(Notifier *notifier, void *unused)
{
@@ -590,6 +612,14 @@ sev_launch_get_measure(Notifier *notifier, void
*unused)
return;
}
+ if (sev_es_enabled()) {
+ /* measure all the VM save areas before getting launch_measure */
+ ret = sev_launch_update_vmsa(sev);
+ if (ret) {
+ exit(1);
Disclaimer: I'm still learning the QEMU source code, sorry if this comes
across as naive.
Is exit() what we want here? I was looking around the rest of the source
code and unfortunately the machine_init_done_notifiers mechanism doesn't
allow for a return value to indicate an error, so I'm wondering if there's
a more appropriate place in the initialization code to handle these
fallible operations and if so, propagate the error down. This way if there
are other resources that need to be cleaned up on the way out, they can
be. Thoughts?
I was following the existing method of terminating that is being performed
in this file. I'll see if others have an idea about how to handle these
types of errors, which could probably be addressed as a separate patch series.
Thanks,
Tom
+ }
+ }
+
measurement = g_new0(struct kvm_sev_launch_measure, 1);
/* query the measurement blob length */
@@ -684,7 +714,7 @@ sev_guest_init(const char *id)
{
SevGuestState *sev;
char *devname;
- int ret, fw_error;
+ int ret, fw_error, cmd;
uint32_t ebx;
uint32_t host_cbitpos;
struct sev_user_data_status status = {};
@@ -745,8 +775,20 @@ sev_guest_init(const char *id)
sev->api_major = status.api_major;
sev->api_minor = status.api_minor;
+ if (sev_es_enabled()) {
+ if (!(status.flags & SEV_STATUS_FLAGS_CONFIG_ES)) {
+ error_report("%s: guest policy requires SEV-ES, but "
+ "host SEV-ES support unavailable",
+ __func__);
+ goto err;
+ }
+ cmd = KVM_SEV_ES_INIT;
+ } else {
+ cmd = KVM_SEV_INIT;
+ }
+
trace_kvm_sev_init();
- ret = sev_ioctl(sev->sev_fd, KVM_SEV_INIT, NULL, _error);
+ ret =
Re: [PATCH 5/8] xlnx-zcu102: Use TYPE_ZCU102_MACHINE constant
On Wed, Aug 26, 2020 at 11:47 AM Eduardo Habkost wrote:
>
> This will make future conversion to use OBJECT_DECLARE* easier.
>
> Signed-off-by: Eduardo Habkost
Reviewed-by: Alistair Francis
Alistair
> ---
> Cc: Alistair Francis
> Cc: "Edgar E. Iglesias"
> Cc: Peter Maydell
> Cc: qemu-...@nongnu.org
> Cc: qemu-devel@nongnu.org
> ---
> hw/arm/xlnx-zcu102.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
> index 5997262459..672d9d4bd1 100644
> --- a/hw/arm/xlnx-zcu102.c
> +++ b/hw/arm/xlnx-zcu102.c
> @@ -238,7 +238,7 @@ static void xlnx_zcu102_machine_class_init(ObjectClass
> *oc, void *data)
> }
>
> static const TypeInfo xlnx_zcu102_machine_init_typeinfo = {
> -.name = MACHINE_TYPE_NAME("xlnx-zcu102"),
> +.name = TYPE_ZCU102_MACHINE,
> .parent = TYPE_MACHINE,
> .class_init = xlnx_zcu102_machine_class_init,
> .instance_init = xlnx_zcu102_machine_instance_init,
> --
> 2.26.2
>
>
Re: [PATCH] ninjatool: quote dollars in variables
On Wed, 26 Aug 2020 at 20:03, Paolo Bonzini wrote: > > Otherwise, dollars (such as in the special $ORIGIN rpath) are > eaten by Make. Incidentally, why are we using rpath anyway? I'm pretty sure the old build system didn't need it, and it's one of those features I have mentally filed away under "liable to confusing and non-portable weirdness"... thanks -- PMM
Re: [PATCH 2/4] sev/i386: Allow AP booting under SEV-ES
On 8/26/20 2:07 PM, Connor Kuehl wrote: On 8/25/20 2:05 PM, Tom Lendacky wrote: From: Tom Lendacky When SEV-ES is enabled, it is not possible modify the guests register state after it has been initially created, encrypted and measured. Normally, an INIT-SIPI-SIPI request is used to boot the AP. However, the hypervisor cannot emulate this because it cannot update the AP register state. For the very first boot by an AP, the reset vector CS segment value and the EIP value must be programmed before the register has been encrypted and measured. Signed-off-by: Tom Lendacky --- accel/kvm/kvm-all.c | 60 ++ accel/stubs/kvm-stub.c | 5 hw/i386/pc_sysfw.c | 10 ++- include/sysemu/kvm.h | 16 +++ include/sysemu/sev.h | 2 ++ target/i386/kvm.c | 2 ++ target/i386/sev.c | 47 + 7 files changed, 141 insertions(+), 1 deletion(-) Just a heads-up: ./scripts/checkpatch.pl does report a couple of style errors. I've seen other series go by where maintainers didn't mind the line length errors, but there are a couple that have to do with braces around if-statement contents that may need to be addressed. Yup, I'll run checkpatch and make the necessary changes. Thanks, Tom
Re: [PATCH 3/4] sev/i386: Don't allow a system reset under an SEV-ES guest
On 8/25/20 2:05 PM, Tom Lendacky wrote:
From: Tom Lendacky
An SEV-ES guest does not allow register state to be altered once it has
been measured. When a SEV-ES guest issues a reboot command, Qemu will
reset the vCPU state and resume the guest. This will cause failures under
SEV-ES, so prevent that from occurring.
Signed-off-by: Tom Lendacky
---
accel/kvm/kvm-all.c | 8
include/sysemu/cpus.h | 2 ++
include/sysemu/hw_accel.h | 4
include/sysemu/kvm.h | 2 ++
softmmu/cpus.c| 5 +
softmmu/vl.c | 5 -
6 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 54e8fd098d..1d925821b2 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -2384,6 +2384,14 @@ void kvm_flush_coalesced_mmio_buffer(void)
s->coalesced_flush_in_progress = false;
}
+bool kvm_cpu_check_resettable(void)
+{
+/* If we have a valid reset vector override, then SEV-ES is active
+ * and the CPU can't be reset.
+ */
+return !kvm_state->reset_valid;
+}
+
static void do_kvm_cpu_synchronize_state(CPUState *cpu, run_on_cpu_data arg)
{
if (!cpu->vcpu_dirty) {
diff --git a/include/sysemu/cpus.h b/include/sysemu/cpus.h
index 3c1da6a018..6d688c757f 100644
--- a/include/sysemu/cpus.h
+++ b/include/sysemu/cpus.h
@@ -24,6 +24,8 @@ void dump_drift_info(void);
void qemu_cpu_kick_self(void);
void qemu_timer_notify_cb(void *opaque, QEMUClockType type);
+bool cpu_is_resettable(void);
+
void cpu_synchronize_all_states(void);
void cpu_synchronize_all_post_reset(void);
void cpu_synchronize_all_post_init(void);
diff --git a/include/sysemu/hw_accel.h b/include/sysemu/hw_accel.h
index e128f8b06b..1fddf4f5e1 100644
--- a/include/sysemu/hw_accel.h
+++ b/include/sysemu/hw_accel.h
@@ -17,6 +17,10 @@
#include "sysemu/hvf.h"
#include "sysemu/whpx.h"
+static inline bool cpu_check_resettable(void)
+{
+return kvm_enabled() ? kvm_cpu_check_resettable() : true;
+}
There's a missing newline here that would separate the closing brace
from the function below this one.
static inline void cpu_synchronize_state(CPUState *cpu)
{
if (kvm_enabled()) {
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index f74cfa85ab..eb94bbbff9 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -494,6 +494,8 @@ int kvm_physical_memory_addr_from_host(KVMState *s, void
*ram_addr,
#endif /* NEED_CPU_H */
+bool kvm_cpu_check_resettable(void);
+
void kvm_cpu_synchronize_state(CPUState *cpu);
void kvm_cpu_synchronize_post_reset(CPUState *cpu);
void kvm_cpu_synchronize_post_init(CPUState *cpu);
diff --git a/softmmu/cpus.c b/softmmu/cpus.c
index a802e899ab..32f286643f 100644
--- a/softmmu/cpus.c
+++ b/softmmu/cpus.c
@@ -927,6 +927,11 @@ void hw_error(const char *fmt, ...)
abort();
}
+bool cpu_is_resettable(void)
+{
+return cpu_check_resettable();
+}
+
void cpu_synchronize_all_states(void)
{
CPUState *cpu;
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 4eb9d1f7fd..422fbb1650 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1475,7 +1475,10 @@ void qemu_system_guest_crashloaded(GuestPanicInformation
*info)
void qemu_system_reset_request(ShutdownCause reason)
{
-if (no_reboot && reason != SHUTDOWN_CAUSE_SUBSYSTEM_RESET) {
+if (!cpu_is_resettable()) {
+error_report("cpus are not resettable, terminating");
+shutdown_requested = reason;
+} else if (no_reboot && reason != SHUTDOWN_CAUSE_SUBSYSTEM_RESET) {
shutdown_requested = reason;
} else {
reset_requested = reason;
Re: [PATCH 4/4] sev/i386: Enable an SEV-ES guest based on SEV policy
On 8/25/20 2:05 PM, Tom Lendacky wrote:
From: Tom Lendacky
Update the sev_es_enabled() function return value to be based on the SEV
policy that has been specified. SEV-ES is enabled if SEV is enabled and
the SEV-ES policy bit is set in the policy object.
Signed-off-by: Tom Lendacky
---
target/i386/sev.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 5146b788fb..153c2cba2c 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -70,6 +70,8 @@ struct SevGuestState {
#define DEFAULT_GUEST_POLICY0x1 /* disable debug */
#define DEFAULT_SEV_DEVICE "/dev/sev"
+#define GUEST_POLICY_SEV_ES_BIT (1 << 2)
+
/* SEV Information Block GUID = 00f771de-1a7e-4fcb-890e-68c77e2fb44e */
#define SEV_INFO_BLOCK_GUID
"\xde\x71\xf7\x00\x7e\x1a\xcb\x4f\x89\x0e\x68\xc7\x7e\x2f\xb4\x4e"
@@ -374,7 +376,7 @@ sev_enabled(void)
bool
sev_es_enabled(void)
{
-return false;
+return (sev_enabled() && (sev_guest->policy & GUEST_POLICY_SEV_ES_BIT));
checkpatch wants these outer parentheses removed :-)
}
uint64_t
Re: [PATCH 2/4] sev/i386: Allow AP booting under SEV-ES
On 8/25/20 2:05 PM, Tom Lendacky wrote: From: Tom Lendacky When SEV-ES is enabled, it is not possible modify the guests register state after it has been initially created, encrypted and measured. Normally, an INIT-SIPI-SIPI request is used to boot the AP. However, the hypervisor cannot emulate this because it cannot update the AP register state. For the very first boot by an AP, the reset vector CS segment value and the EIP value must be programmed before the register has been encrypted and measured. Signed-off-by: Tom Lendacky --- accel/kvm/kvm-all.c| 60 ++ accel/stubs/kvm-stub.c | 5 hw/i386/pc_sysfw.c | 10 ++- include/sysemu/kvm.h | 16 +++ include/sysemu/sev.h | 2 ++ target/i386/kvm.c | 2 ++ target/i386/sev.c | 47 + 7 files changed, 141 insertions(+), 1 deletion(-) Just a heads-up: ./scripts/checkpatch.pl does report a couple of style errors. I've seen other series go by where maintainers didn't mind the line length errors, but there are a couple that have to do with braces around if-statement contents that may need to be addressed.
Re: [PATCH 1/4] sev/i386: Add initial support for SEV-ES
On 8/25/20 2:05 PM, Tom Lendacky wrote:
From: Tom Lendacky
Provide initial support for SEV-ES. This includes creating a function to
indicate the guest is an SEV-ES guest (which will return false until all
support is in place), performing the proper SEV initialization and
ensuring that the guest CPU state is measured as part of the launch.
Co-developed-by: Jiri Slaby
Signed-off-by: Jiri Slaby
Signed-off-by: Tom Lendacky
Hi Tom!
Overall I think the patch set looks good. I mainly just have 1 question
regarding some error handling and a couple of checkpatch related messages.
---
target/i386/cpu.c | 1 +
target/i386/sev-stub.c | 5 +
target/i386/sev.c | 46 --
target/i386/sev_i386.h | 1 +
4 files changed, 51 insertions(+), 2 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 588f32e136..bbbe581d35 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5969,6 +5969,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
uint32_t count,
break;
case 0x801F:
*eax = sev_enabled() ? 0x2 : 0;
+*eax |= sev_es_enabled() ? 0x8 : 0;
*ebx = sev_get_cbit_position();
*ebx |= sev_get_reduced_phys_bits() << 6;
*ecx = 0;
diff --git a/target/i386/sev-stub.c b/target/i386/sev-stub.c
index 88e3f39a1e..040ac90563 100644
--- a/target/i386/sev-stub.c
+++ b/target/i386/sev-stub.c
@@ -49,3 +49,8 @@ SevCapability *sev_get_capabilities(Error **errp)
error_setg(errp, "SEV is not available in this QEMU");
return NULL;
}
+
+bool sev_es_enabled(void)
I don't think this bothers checkpatch, but it'd be consistent with the
rest of your series if this function put the return type on the line above.
+{
+return false;
+}
diff --git a/target/i386/sev.c b/target/i386/sev.c
index c3ecf86704..6c9cd0854b 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -359,6 +359,12 @@ sev_enabled(void)
return !!sev_guest;
}
+bool
+sev_es_enabled(void)
+{
+return false;
+}
+
uint64_t
sev_get_me_mask(void)
{
@@ -578,6 +584,22 @@ sev_launch_update_data(SevGuestState *sev, uint8_t *addr,
uint64_t len)
return ret;
}
+static int
+sev_launch_update_vmsa(SevGuestState *sev)
+{
+int ret, fw_error;
+
+ret = sev_ioctl(sev->sev_fd, KVM_SEV_LAUNCH_UPDATE_VMSA, NULL, _error);
+if (ret) {
+error_report("%s: LAUNCH_UPDATE_VMSA ret=%d fw_error=%d '%s'",
+__func__, ret, fw_error, fw_error_to_str(fw_error));
+goto err;
+}
+
+err:
+return ret;
+}
+
static void
sev_launch_get_measure(Notifier *notifier, void *unused)
{
@@ -590,6 +612,14 @@ sev_launch_get_measure(Notifier *notifier, void *unused)
return;
}
+if (sev_es_enabled()) {
+/* measure all the VM save areas before getting launch_measure */
+ret = sev_launch_update_vmsa(sev);
+if (ret) {
+exit(1);
Disclaimer: I'm still learning the QEMU source code, sorry if this comes
across as naive.
Is exit() what we want here? I was looking around the rest of the source
code and unfortunately the machine_init_done_notifiers mechanism doesn't
allow for a return value to indicate an error, so I'm wondering if
there's a more appropriate place in the initialization code to handle
these fallible operations and if so, propagate the error down. This way
if there are other resources that need to be cleaned up on the way out,
they can be. Thoughts?
+}
+}
+
measurement = g_new0(struct kvm_sev_launch_measure, 1);
/* query the measurement blob length */
@@ -684,7 +714,7 @@ sev_guest_init(const char *id)
{
SevGuestState *sev;
char *devname;
-int ret, fw_error;
+int ret, fw_error, cmd;
uint32_t ebx;
uint32_t host_cbitpos;
struct sev_user_data_status status = {};
@@ -745,8 +775,20 @@ sev_guest_init(const char *id)
sev->api_major = status.api_major;
sev->api_minor = status.api_minor;
+if (sev_es_enabled()) {
+if (!(status.flags & SEV_STATUS_FLAGS_CONFIG_ES)) {
+error_report("%s: guest policy requires SEV-ES, but "
+ "host SEV-ES support unavailable",
+ __func__);
+goto err;
+}
+cmd = KVM_SEV_ES_INIT;
+} else {
+cmd = KVM_SEV_INIT;
+}
+
trace_kvm_sev_init();
-ret = sev_ioctl(sev->sev_fd, KVM_SEV_INIT, NULL, _error);
+ret = sev_ioctl(sev->sev_fd, cmd, NULL, _error);
if (ret) {
error_report("%s: failed to initialize ret=%d fw_error=%d '%s'",
__func__, ret, fw_error, fw_error_to_str(fw_error));
diff --git a/target/i386/sev_i386.h b/target/i386/sev_i386.h
index 4db6960f60..4f9a5e9b21 100644
--- a/target/i386/sev_i386.h
+++ b/target/i386/sev_i386.h
@@ -29,6 +29,7 @@
#define SEV_POLICY_SEV 0x20
extern bool sev_enabled(void);
+extern
[PATCH v2] configure: add --ninja option
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 9db9bb89b9..6ecaff429b 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 "$c")
+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
@@ -8197,7 +8211,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] ninjatool: quote dollars in variables
Otherwise, dollars (such as in the special $ORIGIN rpath) are
eaten by Make.
Reported-by: Laurent Vivier
Signed-off-by: Paolo Bonzini
---
scripts/ninjatool.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/scripts/ninjatool.py b/scripts/ninjatool.py
index cc77d51aa8..c33eafb5a0 100755
--- a/scripts/ninjatool.py
+++ b/scripts/ninjatool.py
@@ -834,7 +834,8 @@ class Ninja2Make(NinjaParserEventsWithVars):
self.print()
for targets in self.build_vars:
for name, value in self.build_vars[targets].items():
-self.print('%s: private .var.%s := %s' % (targets, name,
value))
+self.print('%s: private .var.%s := %s' %
+ (targets, name, value.replace('$', '$$')))
self.print()
if not self.seen_default:
default_targets = sorted(self.all_outs - self.all_ins,
key=natural_sort_key)
--
2.26.2
Re: [PATCH 5/8] xlnx-zcu102: Use TYPE_ZCU102_MACHINE constant
On Wed, Aug 26, 2020 at 02:43:31PM -0400, Eduardo Habkost wrote:
> This will make future conversion to use OBJECT_DECLARE* easier.
>
> Signed-off-by: Eduardo Habkost
Reviewed-by: Edgar E. Iglesias
> ---
> Cc: Alistair Francis
> Cc: "Edgar E. Iglesias"
> Cc: Peter Maydell
> Cc: qemu-...@nongnu.org
> Cc: qemu-devel@nongnu.org
> ---
> hw/arm/xlnx-zcu102.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
> index 5997262459..672d9d4bd1 100644
> --- a/hw/arm/xlnx-zcu102.c
> +++ b/hw/arm/xlnx-zcu102.c
> @@ -238,7 +238,7 @@ static void xlnx_zcu102_machine_class_init(ObjectClass
> *oc, void *data)
> }
>
> static const TypeInfo xlnx_zcu102_machine_init_typeinfo = {
> -.name = MACHINE_TYPE_NAME("xlnx-zcu102"),
> +.name = TYPE_ZCU102_MACHINE,
> .parent = TYPE_MACHINE,
> .class_init = xlnx_zcu102_machine_class_init,
> .instance_init = xlnx_zcu102_machine_instance_init,
> --
> 2.26.2
>
[PATCH 7/8] ppce500: Use TYPE_PPC_E500_PCI_BRIDGE constant
This will make future conversion to use OBJECT_DECLARE* easier.
Signed-off-by: Eduardo Habkost
---
Cc: David Gibson
Cc: qemu-...@nongnu.org
Cc: qemu-devel@nongnu.org
---
hw/pci-host/ppce500.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
index d71072731d..1a62b2f8cc 100644
--- a/hw/pci-host/ppce500.c
+++ b/hw/pci-host/ppce500.c
@@ -509,7 +509,7 @@ static void e500_host_bridge_class_init(ObjectClass *klass,
void *data)
}
static const TypeInfo e500_host_bridge_info = {
-.name = "e500-host-bridge",
+.name = TYPE_PPC_E500_PCI_BRIDGE,
.parent= TYPE_PCI_DEVICE,
.instance_size = sizeof(PPCE500PCIBridgeState),
.class_init= e500_host_bridge_class_init,
--
2.26.2
[PATCH 6/8] tosa: Use TYPE_TOSA_MISC_GPIO constant
This will make future conversion to use OBJECT_DECLARE* easier.
Signed-off-by: Eduardo Habkost
---
Cc: Andrzej Zaborowski
Cc: Peter Maydell
Cc: qemu-...@nongnu.org
Cc: qemu-devel@nongnu.org
---
hw/arm/tosa.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/arm/tosa.c b/hw/arm/tosa.c
index e29566f7b3..90eef1f14d 100644
--- a/hw/arm/tosa.c
+++ b/hw/arm/tosa.c
@@ -316,7 +316,7 @@ static const TypeInfo tosa_ssp_info = {
};
static const TypeInfo tosa_misc_gpio_info = {
-.name = "tosa-misc-gpio",
+.name = TYPE_TOSA_MISC_GPIO,
.parent= TYPE_SYS_BUS_DEVICE,
.instance_size = sizeof(TosaMiscGPIOState),
.instance_init = tosa_misc_gpio_init,
--
2.26.2
[PATCH 3/8] amd_iommu: Use TYPE_AMD_IOMMU_PCI constant
This will make future conversion to use OBJECT_DECLARE* easier.
Signed-off-by: Eduardo Habkost
---
Cc: "Michael S. Tsirkin"
Cc: Marcel Apfelbaum
Cc: Paolo Bonzini
Cc: Richard Henderson
Cc: Eduardo Habkost
Cc: qemu-devel@nongnu.org
---
hw/i386/amd_iommu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 087f601666..18411f1dec 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1622,7 +1622,7 @@ static const TypeInfo amdvi = {
};
static const TypeInfo amdviPCI = {
-.name = "AMDVI-PCI",
+.name = TYPE_AMD_IOMMU_PCI,
.parent = TYPE_PCI_DEVICE,
.instance_size = sizeof(AMDVIPCIState),
.interfaces = (InterfaceInfo[]) {
--
2.26.2
[PATCH 5/8] xlnx-zcu102: Use TYPE_ZCU102_MACHINE constant
This will make future conversion to use OBJECT_DECLARE* easier.
Signed-off-by: Eduardo Habkost
---
Cc: Alistair Francis
Cc: "Edgar E. Iglesias"
Cc: Peter Maydell
Cc: qemu-...@nongnu.org
Cc: qemu-devel@nongnu.org
---
hw/arm/xlnx-zcu102.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
index 5997262459..672d9d4bd1 100644
--- a/hw/arm/xlnx-zcu102.c
+++ b/hw/arm/xlnx-zcu102.c
@@ -238,7 +238,7 @@ static void xlnx_zcu102_machine_class_init(ObjectClass *oc,
void *data)
}
static const TypeInfo xlnx_zcu102_machine_init_typeinfo = {
-.name = MACHINE_TYPE_NAME("xlnx-zcu102"),
+.name = TYPE_ZCU102_MACHINE,
.parent = TYPE_MACHINE,
.class_init = xlnx_zcu102_machine_class_init,
.instance_init = xlnx_zcu102_machine_instance_init,
--
2.26.2
RE: [PATCH v5 0/8] Remove EPYC mode apicid decode and use generic decode
> -Original Message-
> From: Dr. David Alan Gilbert
> Sent: Wednesday, August 26, 2020 1:34 PM
> To: Moger, Babu
> Cc: Igor Mammedov ; Daniel P. Berrangé
> ; ehabk...@redhat.com; m...@redhat.com; Michal
> Privoznik ; qemu-devel@nongnu.org;
> pbonz...@redhat.com; r...@twiddle.net
> Subject: Re: [PATCH v5 0/8] Remove EPYC mode apicid decode and use generic
> decode
>
> * Babu Moger (babu.mo...@amd.com) wrote:
> >
> > > -Original Message-
> > > From: Igor Mammedov
> > > Sent: Wednesday, August 26, 2020 8:31 AM
> > > To: Daniel P. Berrangé
> > > Cc: Moger, Babu ; pbonz...@redhat.com;
> > > r...@twiddle.net; ehabk...@redhat.com; qemu-devel@nongnu.org;
> > > m...@redhat.com; Michal Privoznik
> > > Subject: Re: [PATCH v5 0/8] Remove EPYC mode apicid decode and use
> > > generic decode
> > >
> > > On Wed, 26 Aug 2020 13:50:59 +0100
> > > Daniel P. Berrangé wrote:
> > >
> > > > On Wed, Aug 26, 2020 at 02:38:49PM +0200, Igor Mammedov wrote:
> > > > > On Fri, 21 Aug 2020 17:12:19 -0500 Babu Moger
> > > > > wrote:
> > > > >
> > > > > > To support some of the complex topology, we introduced EPYC
> > > > > > mode
> > > apicid decode.
> > > > > > But, EPYC mode decode is running into problems. Also it can
> > > > > > become quite a maintenance problem in the future. So, it was
> > > > > > decided to remove that code and use the generic decode which
> > > > > > works for majority of the topology. Most of the SPECed
> > > > > > configuration would work just fine. With some non-SPECed user
> > > > > > inputs, it will create some sub-
> > > optimal configuration.
> > > > > > Here is the discussion thread.
> > > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2
> > > > > > F%2F
> > > > > > lore.kernel.org%2Fqemu-devel%2Fc0bcc1a6-1d84-a6e7-e468-
> > > d5b437c1b25
> > > > > >
> > >
> 4%40amd.com%2Fdata=02%7C01%7Cbabu.moger%40amd.com%7C8a5c
> > > 52f92
> > > > > >
> > >
> 3f04082a40808d849c43d49%7C3dd8961fe4884e608e11a82d994e183d%7C0%7
> > > C0
> > > > > >
> > >
> %7C637340454473508873sdata=VnW28H1v4XwK3GaNGFxu%2BhwiMeA
> > > YO%2B
> > > > > > 3WAzo3DeY5Ha8%3Dreserved=0
> > > > > >
> > > > > > This series removes all the EPYC mode specific apicid changes
> > > > > > and use the generic apicid decode.
> > > > >
> > > > > the main difference between EPYC and all other CPUs is that it
> > > > > requires numa configuration (it's not optional) so we need an
> > > > > extra
> > No, That is not true. Because of that assumption we made all these
> > apicid changes. And here we are now.
> >
> > AMD supports varies mixed configurations. In case of EPYC-Rome, we
> > have NPS1, NPS2 and NPS4(Numa Nodes per socket). In case of NPS1,
> > basically we have all the cores in a socket under one numa node. This
> > is non-numa configuration.
> > Looking at the various configurations and also discussing internally,
> > it is not advisable to have (epyc && !numa) check.
>
> Indeed on real hardware, I don't think we always see NUMA; my single socket,
> 16 core/32 thread 7302P Dell box, shows the kernel printing 'No NUMA
> configuration found...Faking a node.'
>
> So if real hardware hasn't got a NUMA node, what's the real problem?
I don't see any problem once we revert all these changes(patch 1-7).
We don't need if (epyc && !numa) error check or auto_enable_numa=true
unconditionally.
>
> Dave
>
> > > > > patch on top of this series to enfoce that, i.e:
> > > > >
> > > > > if (epyc && !numa)
> > > > > error("EPYC cpu requires numa to be configured")
> > > >
> > > > Please no. This will break 90% of current usage of the EPYC CPU in
> > > > real world QEMU deployments. That is way too user hostile to
> > > > introduce as a requirement.
> > > >
> > > > Why do we need to force this ? People have been successfuly using
> > > > EPYC CPUs without NUMA in QEMU for years now.
> > > >
> > > > It might not match behaviour of bare metal silicon, but that
> > > > hasn't obviously caused the world to come crashing down.
> > > So far it produces warning in linux kernel (RHBZ1728166), (resulting
> > > performance might be suboptimal), but I haven't seen anyone reporting
> crashes yet.
> > >
> > >
> > > What other options do we have?
> > > Perhaps we can turn on strict check for new machine types only, so
> > > old configs can keep broken topology (CPUID), while new ones would
> > > require -numa and produce correct topology.
> > >
> > >
> > > >
> > > > Regards,
> > > > Daniel
> >
> >
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
[PATCH 1/8] etsec: Use TYPE_ETSEC_COMMON constant
This will make future conversion to use OBJECT_DECLARE* easier.
Signed-off-by: Eduardo Habkost
---
Cc: David Gibson
Cc: Jason Wang
Cc: qemu-...@nongnu.org
Cc: qemu-devel@nongnu.org
---
hw/net/fsl_etsec/etsec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/net/fsl_etsec/etsec.c b/hw/net/fsl_etsec/etsec.c
index 7035cf4eb9..ad20b22cdd 100644
--- a/hw/net/fsl_etsec/etsec.c
+++ b/hw/net/fsl_etsec/etsec.c
@@ -430,7 +430,7 @@ static void etsec_class_init(ObjectClass *klass, void *data)
}
static TypeInfo etsec_info = {
-.name = "eTSEC",
+.name = TYPE_ETSEC_COMMON,
.parent= TYPE_SYS_BUS_DEVICE,
.instance_size = sizeof(eTSEC),
.class_init= etsec_class_init,
--
2.26.2
[PATCH 2/8] nios2_iic: Use TYPE_ALTERA_IIC constant
This will make future conversion to use OBJECT_DECLARE* easier.
Signed-off-by: Eduardo Habkost
---
Cc: Chris Wulff
Cc: Marek Vasut
Cc: qemu-devel@nongnu.org
---
hw/intc/nios2_iic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/intc/nios2_iic.c b/hw/intc/nios2_iic.c
index 1a5df8c89a..86d088f9b5 100644
--- a/hw/intc/nios2_iic.c
+++ b/hw/intc/nios2_iic.c
@@ -80,7 +80,7 @@ static void altera_iic_class_init(ObjectClass *klass, void
*data)
}
static TypeInfo altera_iic_info = {
-.name = "altera,iic",
+.name = TYPE_ALTERA_IIC,
.parent= TYPE_SYS_BUS_DEVICE,
.instance_size = sizeof(AlteraIIC),
.instance_init = altera_iic_init,
--
2.26.2
[PATCH 4/8] sclpconsole: Use TYPE_* constants
This will make future conversion to use OBJECT_DECLARE* easier.
Signed-off-by: Eduardo Habkost
---
Cc: Cornelia Huck
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: Thomas Huth
Cc: "Marc-André Lureau"
Cc: Paolo Bonzini
Cc: qemu-s3...@nongnu.org
Cc: qemu-devel@nongnu.org
---
hw/char/sclpconsole-lm.c | 2 +-
hw/char/sclpconsole.c| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/char/sclpconsole-lm.c b/hw/char/sclpconsole-lm.c
index 2b5f37b6a2..5848b4e9c5 100644
--- a/hw/char/sclpconsole-lm.c
+++ b/hw/char/sclpconsole-lm.c
@@ -355,7 +355,7 @@ static void console_class_init(ObjectClass *klass, void
*data)
}
static const TypeInfo sclp_console_info = {
-.name = "sclplmconsole",
+.name = TYPE_SCLPLM_CONSOLE,
.parent= TYPE_SCLP_EVENT,
.instance_size = sizeof(SCLPConsoleLM),
.class_init= console_class_init,
diff --git a/hw/char/sclpconsole.c b/hw/char/sclpconsole.c
index 5c7664905e..d6f7da0818 100644
--- a/hw/char/sclpconsole.c
+++ b/hw/char/sclpconsole.c
@@ -271,7 +271,7 @@ static void console_class_init(ObjectClass *klass, void
*data)
}
static const TypeInfo sclp_console_info = {
-.name = "sclpconsole",
+.name = TYPE_SCLP_CONSOLE,
.parent= TYPE_SCLP_EVENT,
.instance_size = sizeof(SCLPConsole),
.class_init= console_class_init,
--
2.26.2
[PATCH 0/8] qom: Use TYPE_* constants
Clean up code that uses hardcoded strings instead of TYPE_* constants when defining QOM types. Eduardo Habkost (8): etsec: Use TYPE_ETSEC_COMMON constant nios2_iic: Use TYPE_ALTERA_IIC constant amd_iommu: Use TYPE_AMD_IOMMU_PCI constant sclpconsole: Use TYPE_* constants xlnx-zcu102: Use TYPE_ZCU102_MACHINE constant tosa: Use TYPE_TOSA_MISC_GPIO constant ppce500: Use TYPE_PPC_E500_PCI_BRIDGE constant dc390: Use TYPE_DC390_DEVICE constant hw/arm/tosa.c| 2 +- hw/arm/xlnx-zcu102.c | 2 +- hw/char/sclpconsole-lm.c | 2 +- hw/char/sclpconsole.c| 2 +- hw/i386/amd_iommu.c | 2 +- hw/intc/nios2_iic.c | 2 +- hw/net/fsl_etsec/etsec.c | 2 +- hw/pci-host/ppce500.c| 2 +- hw/scsi/esp-pci.c| 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) -- 2.26.2
[PATCH 8/8] dc390: Use TYPE_DC390_DEVICE constant
This will make future conversion to use OBJECT_DECLARE* easier.
Signed-off-by: Eduardo Habkost
---
Cc: Paolo Bonzini
Cc: Fam Zheng
Cc: qemu-devel@nongnu.org
---
hw/scsi/esp-pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
index 497a8d5901..90432ef107 100644
--- a/hw/scsi/esp-pci.c
+++ b/hw/scsi/esp-pci.c
@@ -521,7 +521,7 @@ static void dc390_class_init(ObjectClass *klass, void *data)
}
static const TypeInfo dc390_info = {
-.name = "dc390",
+.name = TYPE_DC390_DEVICE,
.parent = TYPE_AM53C974_DEVICE,
.instance_size = sizeof(DC390State),
.class_init = dc390_class_init,
--
2.26.2
Re: [PATCH 0/1] qcow2: Skip copy-on-write when allocating a zero cluster
On Tue 25 Aug 2020 09:47:24 PM CEST, Brian Foster wrote: > My fio fallocates the entire file by default with this command. Is that > the intent of this particular test? I added --fallocate=none to my test > runs to incorporate the allocation cost in the I/Os. That wasn't intentional, you're right, it should use --fallocate=none (I don't see a big difference in my test anyway). >> The Linux version is 4.19.132-1 from Debian. > > Thanks. I don't have LUKS in the mix on my box, but I was running on a > more recent kernel (Fedora 5.7.15-100). I threw v4.19 on the box and > saw a bit more of a delta between XFS (~14k iops) and ext4 (~24k). The > same test shows ~17k iops for XFS and ~19k iops for ext4 on v5.7. If I > increase the size of the LVM volume from 126G to >1TB, ext4 runs at > roughly the same rate and XFS closes the gap to around ~19k iops as > well. I'm not sure what might have changed since v4.19, but care to > see if this is still an issue on a more recent kernel? Ok, I gave 5.7.10-1 a try but I still get similar numbers. Perhaps with a larger filesystem there would be a difference? I don't know. Berto
Re: [PATCH v2 (BROKEN) 0/6] migration: bring improved savevm/loadvm/delvm to QMP
On Wed, Aug 26, 2020 at 07:28:24PM +0100, Daniel P. Berrangé wrote: > On Wed, Aug 26, 2020 at 05:52:06PM +0200, Markus Armbruster wrote: > > Open questions: > > > > * Do we want the QMP command to delete existing snapshots with > > conflicting tag / ID, like HMP savevm does? Or do we want it to fail > > the transaction? > > The intent is for the QMP commands to operate exclusively on > 'tags', and never consider "ID". I forgot that even HMP ignores "ID" now and works exclusively in terms of tags since: commit 6ca080453ea403959ccde661030ca16264acc181 Author: Daniel Henrique Barboza Date: Wed Nov 7 11:09:58 2018 -0200 block/snapshot.c: eliminate use of ID input in snapshot operations 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 0/8] Remove EPYC mode apicid decode and use generic decode
* Babu Moger (babu.mo...@amd.com) wrote:
>
> > -Original Message-
> > From: Igor Mammedov
> > Sent: Wednesday, August 26, 2020 8:31 AM
> > To: Daniel P. Berrangé
> > Cc: Moger, Babu ; pbonz...@redhat.com;
> > r...@twiddle.net; ehabk...@redhat.com; qemu-devel@nongnu.org;
> > m...@redhat.com; Michal Privoznik
> > Subject: Re: [PATCH v5 0/8] Remove EPYC mode apicid decode and use generic
> > decode
> >
> > On Wed, 26 Aug 2020 13:50:59 +0100
> > Daniel P. Berrangé wrote:
> >
> > > On Wed, Aug 26, 2020 at 02:38:49PM +0200, Igor Mammedov wrote:
> > > > On Fri, 21 Aug 2020 17:12:19 -0500
> > > > Babu Moger wrote:
> > > >
> > > > > To support some of the complex topology, we introduced EPYC mode
> > apicid decode.
> > > > > But, EPYC mode decode is running into problems. Also it can become
> > > > > quite a maintenance problem in the future. So, it was decided to
> > > > > remove that code and use the generic decode which works for
> > > > > majority of the topology. Most of the SPECed configuration would
> > > > > work just fine. With some non-SPECed user inputs, it will create some
> > > > > sub-
> > optimal configuration.
> > > > > Here is the discussion thread.
> > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> > > > > lore.kernel.org%2Fqemu-devel%2Fc0bcc1a6-1d84-a6e7-e468-
> > d5b437c1b25
> > > > >
> > 4%40amd.com%2Fdata=02%7C01%7Cbabu.moger%40amd.com%7C8a5c
> > 52f92
> > > > >
> > 3f04082a40808d849c43d49%7C3dd8961fe4884e608e11a82d994e183d%7C0%7
> > C0
> > > > >
> > %7C637340454473508873sdata=VnW28H1v4XwK3GaNGFxu%2BhwiMeA
> > YO%2B
> > > > > 3WAzo3DeY5Ha8%3Dreserved=0
> > > > >
> > > > > This series removes all the EPYC mode specific apicid changes and
> > > > > use the generic apicid decode.
> > > >
> > > > the main difference between EPYC and all other CPUs is that it
> > > > requires numa configuration (it's not optional) so we need an extra
> No, That is not true. Because of that assumption we made all these apicid
> changes. And here we are now.
>
> AMD supports varies mixed configurations. In case of EPYC-Rome, we have
> NPS1, NPS2 and NPS4(Numa Nodes per socket). In case of NPS1, basically we
> have all the cores in a socket under one numa node. This is non-numa
> configuration.
> Looking at the various configurations and also discussing internally, it
> is not advisable to have (epyc && !numa) check.
Indeed on real hardware, I don't think we always see NUMA; my single
socket, 16 core/32 thread 7302P Dell box, shows the kernel printing
'No NUMA configuration found...Faking a node.'
So if real hardware hasn't got a NUMA node, what's the real problem?
Dave
> > > > patch on top of this series to enfoce that, i.e:
> > > >
> > > > if (epyc && !numa)
> > > > error("EPYC cpu requires numa to be configured")
> > >
> > > Please no. This will break 90% of current usage of the EPYC CPU in
> > > real world QEMU deployments. That is way too user hostile to introduce
> > > as a requirement.
> > >
> > > Why do we need to force this ? People have been successfuly using
> > > EPYC CPUs without NUMA in QEMU for years now.
> > >
> > > It might not match behaviour of bare metal silicon, but that hasn't
> > > obviously caused the world to come crashing down.
> > So far it produces warning in linux kernel (RHBZ1728166), (resulting
> > performance
> > might be suboptimal), but I haven't seen anyone reporting crashes yet.
> >
> >
> > What other options do we have?
> > Perhaps we can turn on strict check for new machine types only, so old
> > configs
> > can keep broken topology (CPUID), while new ones would require -numa and
> > produce correct topology.
> >
> >
> > >
> > > Regards,
> > > Daniel
>
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [PATCH v2 (BROKEN) 0/6] migration: bring improved savevm/loadvm/delvm to QMP
On Wed, Aug 26, 2020 at 05:52:06PM +0200, Markus Armbruster wrote: > Sorry for taking so long to reply. > > Daniel P. Berrangé writes: > > > A followup to: > > > > v1: https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg00866.html > > > > When QMP was first introduced some 10+ years ago now, the snapshot > > related commands (savevm/loadvm/delvm) were not converted. This was > > primarily because their implementation causes blocking of the thread > > running the monitor commands. This was (and still is) considered > > undesirable behaviour both in HMP and QMP. > > One of several reasons. > > > In theory someone was supposed to fix this flaw at some point in the > > past 10 years and bring them into the QMP world. Sadly, thus far it > > hasn't happened as people always had more important things to work > > on. Enterprise apps were much more interested in external snapshots > > than internal snapshots as they have many more features. > > Several attempts have been made to bring the functionality to QMP. > Sadly, they went nowhere. > > I posted an analysis of the issues in reply to one of the more serious > attempts: > > Message-ID: <87lh7l783q@blackfin.pond.sub.org> > https://lists.nongnu.org/archive/html/qemu-devel/2016-01/msg03593.html > > I'd like to hear your take on it. I append the relevant part for your > convenience. Perhaps your code is already close to what I describe > there. I'm interested in where it falls short. > > > Meanwhile users still want to use internal snapshots as there is > > a certainly simplicity in having everything self-contained in one > > image, even though it has limitations. Thus the apps that end up > > executing the savevm/loadvm/delvm via the "human-monitor-command" > > QMP command. > > > > IOW, the problematic blocking behaviour that was one of the reasons > > for not having savevm/loadvm/delvm in QMP is experienced by applications > > regardless. By not portting the commands to QMP due to one design flaw, > > we've forced apps and users to suffer from other design flaws of HMP ( > > bad error reporting, strong type checking of args, no introspection) for > > an additional 10 years. This feels rather sub-optimal :-( > > > > In practice users don't appear to care strongly about the fact that these > > commands block the VM while they run. I might have seen one bug report > > about it, but it certainly isn't something that comes up as a frequent > > topic except among us QEMU maintainers. Users do care about having > > access to the snapshot feature. > > > > Where I am seeing frequent complaints is wrt the use of OVMF combined > > with snapshots which has some serious pain points. This is getting worse > > as the push to ditch legacy BIOS in favour of UEFI gain momentum both > > across OS vendors and mgmt apps. Solving it requires new parameters to > > the commands, but doing this in HMP is super unappealing. > > > > After 10 years, I think it is time for us to be a little pragmatic about > > our handling of snapshots commands. My desire is that libvirt should never > > use "human-monitor-command" under any circumstances, because of the > > inherant flaws in HMP as a protocol for machine consumption. > > > > Thus in this series I'm proposing a fairly direct mapping of the existing > > HMP commands for savevm/loadvm/delvm into QMP as a first step. This does > > not solve the blocking thread problem, but it does put in a place a > > design using the jobs framework which can facilitate solving it later. > > It does also solve the error reporting, type checking and introspection > > problems inherant to HMP. So we're winning on 3 out of the 4 problems, > > and pushed apps to a QMP design that will let us solve the last > > remaining problem. > > > > With a QMP variant, we reasonably deal with the problems related to OVMF: > > > > - The logic to pick which disk to store the vmstate in is not > >satsifactory. > > > >The first block driver state cannot be assumed to be the root disk > >image, it might be OVMF varstore and we don't want to store vmstate > >in there. > > Yes, this is one of the issues. Glad you're addressing it. > > > - The logic to decide which disks must be snapshotted is hardwired > >to all disks which are writable > > > >Again with OVMF there might be a writable varstore, but this can be > >raw rather than qcow2 format, and thus unable to be snapshotted. > >While users might wish to snapshot their varstore, in some/many/most > >cases it is entirely uneccessary. Users are blocked from snapshotting > >their VM though due to this varstore. > > Another one. Glad again. > > > These are solved by adding two parameters to the commands. The first is > > a block device node name that identifies the image to store vmstate in, > > and the second is a list of node names to include for the snapshots. > > If the list of nodes isn't given, it falls back to the historical > > behaviour of using all
Re: [RFC v4 00/70] support vector extension v1.0
On Thu, Aug 27, 2020 at 2:03 AM Alistair Francis wrote: > On Wed, Aug 26, 2020 at 10:39 AM Frank Chang > wrote: > > > > On Thu, Aug 27, 2020 at 12:56 AM Alistair Francis > wrote: > >> > >> On Tue, Aug 25, 2020 at 1:29 AM Frank Chang > wrote: > >> > > >> > On Mon, Aug 17, 2020 at 4:50 PM wrote: > >> >> > >> >> From: Frank Chang > >> >> > >> >> This patchset implements the vector extension v1.0 for RISC-V on > QEMU. > >> >> > >> >> This patchset is sent as RFC because RVV v1.0 is still in draft > state. > >> >> v2 patchset was sent for RVV v0.9 and bumped to RVV v1.0 since v3 > patchset. > >> >> > >> >> The port is available here: > >> >> https://github.com/sifive/qemu/tree/rvv-1.0-upstream-v4 > >> >> > >> >> You can change the cpu argument: vext_spec to v1.0 (i.e. > vext_spec=v1.0) > >> >> to run with RVV v1.0 instructions. > >> >> > >> >> Note: This patchset depends on two other patchsets listed in Based-on > >> >> section below so it might not able to be built unless those two > >> >> patchsets are applied. > >> >> > >> >> Changelog: > >> >> > >> >> v4 > >> >> * remove explicit float flmul variable in DisasContext. > >> >> * replace floating-point calculations with shift operations to > >> >> improve performance. > >> >> * relax RV_VLEN_MAX to 512-bits. > >> >> > >> >> v3 > >> >> * apply nan-box helpers from Richard Henderson. > >> >> * remove fp16 api changes as they are sent independently in another > >> >> pathcset by Chih-Min Chao. > >> >> * remove all tail elements clear functions as tail elements can > >> >> retain unchanged for either VTA set to undisturbed or agnostic. > >> >> * add fp16 nan-box check generator function. > >> >> * add floating-point rounding mode enum. > >> >> * replace flmul arithmetic with shifts to avoid floating-point > >> >> conversions. > >> >> * add Zvqmac extension. > >> >> * replace gdbstub vector register xml files with dynamic generator. > >> >> * bumped to RVV v1.0. > >> >> * RVV v1.0 related changes: > >> >> * add vlre.v and vsr.v vector whole register > >> >> load/store instructions > >> >> * add vrgatherei16 instruction. > >> >> * rearranged bits in vtype to make vlmul bits into a contiguous > >> >> field. > >> >> > >> >> v2 > >> >> * drop v0.7.1 support. > >> >> * replace invisible return check macros with functions. > >> >> * move mark_vs_dirty() to translators. > >> >> * add SSTATUS_VS flag for s-mode. > >> >> * nan-box scalar fp register for floating-point operations. > >> >> * add gdbstub files for vector registers to allow system-mode > >> >> debugging with GDB. > >> >> > >> >> Based-on: <20200724002807.441147-1-richard.hender...@linaro.org/> > >> >> Based-on: < > 1596102747-20226-1-git-send-email-chihmin.c...@sifive.com/> > >> >> > >> >> Frank Chang (62): > >> >> target/riscv: drop vector 0.7.1 and add 1.0 support > >> >> target/riscv: Use FIELD_EX32() to extract wd field > >> >> target/riscv: rvv-1.0: introduce writable misa.v field > >> >> target/riscv: rvv-1.0: remove rvv related codes from fcsr registers > >> >> target/riscv: rvv-1.0: check MSTATUS_VS when accessing vector csr > >> >> registers > >> >> target/riscv: rvv-1.0: remove MLEN calculations > >> >> target/riscv: rvv-1.0: add fractional LMUL > >> >> target/riscv: rvv-1.0: add VMA and VTA > >> >> target/riscv: rvv-1.0: update check functions > >> >> target/riscv: introduce more imm value modes in translator > functions > >> >> target/riscv: rvv:1.0: add translation-time nan-box helper function > >> >> target/riscv: rvv-1.0: configure instructions > >> >> target/riscv: rvv-1.0: stride load and store instructions > >> >> target/riscv: rvv-1.0: index load and store instructions > >> >> target/riscv: rvv-1.0: fix address index overflow bug of indexed > >> >> load/store insns > >> >> target/riscv: rvv-1.0: fault-only-first unit stride load > >> >> target/riscv: rvv-1.0: amo operations > >> >> target/riscv: rvv-1.0: load/store whole register instructions > >> >> target/riscv: rvv-1.0: update vext_max_elems() for load/store insns > >> >> target/riscv: rvv-1.0: take fractional LMUL into vector max > elements > >> >> calculation > >> >> target/riscv: rvv-1.0: floating-point square-root instruction > >> >> target/riscv: rvv-1.0: floating-point classify instructions > >> >> target/riscv: rvv-1.0: mask population count instruction > >> >> target/riscv: rvv-1.0: find-first-set mask bit instruction > >> >> target/riscv: rvv-1.0: set-X-first mask bit instructions > >> >> target/riscv: rvv-1.0: iota instruction > >> >> target/riscv: rvv-1.0: element index instruction > >> >> target/riscv: rvv-1.0: allow load element with sign-extended > >> >> target/riscv: rvv-1.0: register gather instructions > >> >> target/riscv: rvv-1.0: integer scalar move instructions > >> >> target/riscv: rvv-1.0: floating-point move instruction > >> >> target/riscv:
[PATCH] armsse: Define ARMSSEClass correctly
TYPE_ARM_SSE is a TYPE_SYS_BUS_DEVICE subclass, but
ARMSSEClass::parent_class is declared as DeviceClass.
It never caused any problems by pure luck:
We were not setting class_size for TYPE_ARM_SSE, so class_size of
TYPE_SYS_BUS_DEVICE was being used (sizeof(SysBusDeviceClass)).
This made the system allocate enough memory for TYPE_ARM_SSE
devices even though ARMSSEClass was too small for a sysbus
device.
Additionally, the ARMSSEClass::info field ended up at the same
offset as SysBusDeviceClass::explicit_ofw_unit_address. This
would make sysbus_get_fw_dev_path() crash for the device.
Luckily, sysbus_get_fw_dev_path() never gets called for
TYPE_ARM_SSE devices, because qdev_get_fw_dev_path() is only used
by the boot device code, and TYPE_ARM_SSE devices don't appear at
the fw_boot_order list.
Signed-off-by: Eduardo Habkost
---
Cc: Peter Maydell
Cc: qemu-...@nongnu.org
Cc: qemu-devel@nongnu.org
---
include/hw/arm/armsse.h | 2 +-
hw/arm/armsse.c | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/hw/arm/armsse.h b/include/hw/arm/armsse.h
index 84080c2299..b10173beab 100644
--- a/include/hw/arm/armsse.h
+++ b/include/hw/arm/armsse.h
@@ -220,7 +220,7 @@ typedef struct ARMSSE {
typedef struct ARMSSEInfo ARMSSEInfo;
typedef struct ARMSSEClass {
-DeviceClass parent_class;
+SysBusDeviceClass parent_class;
const ARMSSEInfo *info;
} ARMSSEClass;
diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
index dcbff9bd8f..6381bbd94d 100644
--- a/hw/arm/armsse.c
+++ b/hw/arm/armsse.c
@@ -1160,6 +1160,7 @@ static const TypeInfo armsse_info = {
.name = TYPE_ARMSSE,
.parent = TYPE_SYS_BUS_DEVICE,
.instance_size = sizeof(ARMSSE),
+.class_size = sizeof(ARMSSEClass),
.instance_init = armsse_init,
.abstract = true,
.interfaces = (InterfaceInfo[]) {
--
2.26.2
Re: [PATCH 00/77] target/microblaze improvements
On Tue, Aug 25, 2020 at 01:58:33PM -0700, Richard Henderson wrote: > Well, this is larger than I expected. > > I started off thinking conversion to decodetree would be quick, > after I reviewed the mttcg patches last week. Then I realized > that this could also use conversion to the generic translation loop. > Then I realized that there were a number of bugs, and some > inefficiencies, that could be fixed by using tcg exception unwind > properly. > > Hopefully most of these are small and easy to understand. > > I begin by adding enough stuff to test/tcg to make use of a > self-built musl cross-environment. I'll note that linuxtest > does not pass before or after this patch set -- I think that > linux-user/microblaze/signal.c needs work -- but that the > other tests do work. > > I also have an old copy of a microblaze system test image, > which I think used to be hosted on our wiki. After basic kernel > boot, it contains a "selftest" script which runs a bunch of > user-land isa tests. That still works fine too. > > HOWEVER: That's not nearly complete. There are cpu config options > that aren't default and I don't know how to change or test. > > In addition, the manual is really not clear on what's supposed to > happen under various edge conditions, especially with MSR[EE] unset. > E.g. unaligned access: Does the insn get nop-ed out? Do the low > bits of the address get ignored? Right now (before and after) the > access simply happens unaligned, which doesn't seem correct. > I assume the reason for having this configure option is to reduce > the size of the FPGA so that the unaligned access handling hw > simply isn't present. Yes, I verified with the RTL designers and this particular case will result in the core issuing the load/store with the lower address bits ignored. Cheers, Edgar
