[PATCH] docs/about/emulation: fix typo
Duplicated word "are". Signed-off-by: Sh4w --- docs/about/emulation.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/about/emulation.rst b/docs/about/emulation.rst index b510a54418..0ad0b86f0d 100644 --- a/docs/about/emulation.rst +++ b/docs/about/emulation.rst @@ -99,7 +99,7 @@ depending on the guest architecture. - Yes - A configurable 32 bit soft core now owned by Cadence -A number of features are are only available when running under +A number of features are only available when running under emulation including :ref:`Record/Replay` and :ref:`TCG Plugins`. .. _Semihosting: -- 2.32.0 (Apple Git-132)
Re: [PATCH v2 0/9] Hexagon (target/hexagon) New architecture support
On 5/10/23 21:58, Taylor Simpson wrote: Where can one find docs for this? The latest Hexagon SDK I can find is 3.5, which still ends at v67. I guess the folks at developer.qualcomm.com are behind in publishing specs. I'll work on getting these. Hi Richard, The documents have been posted on this page in the Documentation section. https://developer.qualcomm.com/software/hexagon-dsp-sdk/tools I'll update the README with the link to the latest versions. Are you planning to review these given that Anton Johansson has already done a review? If not, I'll go ahead and do the pull request. Go ahead with the pull. But thanks for the documentation update. r~
[PATCH 10/27] mkvenv: work around broken pip installations on Debian 10
This is a workaround intended for Debian 10, where the debian-patched
pip does not function correctly if accessed from within a virtual
environment.
We don't support Debian 10 as a build platform any longer, though we do
still utilize it for our build-tricore-softmmu CI test. It's also
possible that this bug might appear on other derivative platforms and
this workaround may prove useful.
Signed-off-by: Paolo Bonzini
Signed-off-by: John Snow
---
python/scripts/mkvenv.py | 68 ++--
1 file changed, 52 insertions(+), 16 deletions(-)
diff --git a/python/scripts/mkvenv.py b/python/scripts/mkvenv.py
index 7a9a14124d..1e072b3df3 100644
--- a/python/scripts/mkvenv.py
+++ b/python/scripts/mkvenv.py
@@ -230,26 +230,26 @@ def need_ensurepip() -> bool:
return True
-def check_ensurepip(with_pip: bool) -> None:
+def check_ensurepip(prefix: str = "", suggest_remedy: bool = False) -> None:
"""
Check that we have ensurepip.
Raise a fatal exception with a helpful hint if it isn't available.
"""
-if not with_pip:
-return
-
if not find_spec("ensurepip"):
msg = (
"Python's ensurepip module is not found.\n"
"It's normally part of the Python standard library, "
"maybe your distribution packages it separately?\n"
-"Either install ensurepip, or alleviate the need for it in the "
-"first place by installing pip and setuptools for "
-f"'{sys.executable}'.\n"
-"(Hint: Debian puts ensurepip in its python3-venv package.)"
+"(Debian puts ensurepip in its python3-venv package.)\n"
)
-raise Ouch(msg)
+if suggest_remedy:
+msg += (
+"Either install ensurepip, or alleviate the need for it in the"
+" first place by installing pip and setuptools for "
+f"'{sys.executable}'.\n"
+)
+raise Ouch(prefix + msg)
# ensurepip uses pyexpat, which can also go missing on us:
if not find_spec("pyexpat"):
@@ -257,12 +257,15 @@ def check_ensurepip(with_pip: bool) -> None:
"Python's pyexpat module is not found.\n"
"It's normally part of the Python standard library, "
"maybe your distribution packages it separately?\n"
-"Either install pyexpat, or alleviate the need for it in the "
-"first place by installing pip and setuptools for "
-f"'{sys.executable}'.\n\n"
-"(Hint: NetBSD's pkgsrc debundles this to e.g. 'py310-expat'.)"
+"(NetBSD's pkgsrc debundles this to e.g. 'py310-expat'.)\n"
)
-raise Ouch(msg)
+if suggest_remedy:
+msg += (
+"Either install pyexpat, or alleviate the need for it in the "
+"first place by installing pip and setuptools for "
+f"'{sys.executable}'.\n"
+)
+raise Ouch(prefix + msg)
def make_venv( # pylint: disable=too-many-arguments
@@ -310,7 +313,8 @@ def make_venv( # pylint: disable=too-many-arguments
with_pip = True if not system_site_packages else need_ensurepip()
logger.debug("with_pip unset, choosing %s", with_pip)
-check_ensurepip(with_pip)
+if with_pip:
+check_ensurepip(suggest_remedy=True)
if symlinks is None:
# Default behavior of standard venv CLI
@@ -534,6 +538,36 @@ def _get_entry_points() -> Iterator[Dict[str, str]]:
logger.debug("wrote '%s'", script_path)
+def checkpip() -> None:
+"""
+Debian10 has a pip that's broken when used inside of a virtual environment.
+
+We try to detect and correct that case here.
+"""
+try:
+# pylint: disable=import-outside-toplevel,unused-import,import-error
+import pip._internal # type: ignore # noqa: F401
+
+logger.debug("pip appears to be working correctly.")
+return
+except ModuleNotFoundError as exc:
+if exc.name == "pip._internal":
+# Uh, fair enough. They did say "internal".
+# Let's just assume it's fine.
+return
+logger.warning("pip appears to be malfunctioning: %s", str(exc))
+
+check_ensurepip("pip appears to be non-functional, and ")
+
+logger.debug("Attempting to repair pip ...")
+subprocess.run(
+(sys.executable, "-m", "ensurepip"),
+stdout=subprocess.DEVNULL,
+check=True,
+)
+logger.debug("Pip is now (hopefully) repaired!")
+
+
def pkgname_from_depspec(dep_spec: str) -> str:
"""
Parse package name out of a PEP-508 depspec.
@@ -748,7 +782,9 @@ def post_venv_setup() -> None:
This is intended to be run *inside the venv* after it is created.
"""
logger.debug("post_venv_setup()")
-# Generate a 'pip' script so the venv is usable in a normal
+# Test for a broken pip (Debian 10 or derivative?) and fix it if needed
+
[PATCH 09/27] mkvenv: create pip binary in virtual environment
From: Paolo Bonzini
While pip can be invoked as "python -m pip", it is more standard to
have it as a binary in the virtual environment. Instead of using
ensurepip, which is slow, use the console shim generation that was
just added for "mkvenv ensure".
Signed-off-by: John Snow
Signed-off-by: Paolo Bonzini
---
python/scripts/mkvenv.py | 34 ++
1 file changed, 34 insertions(+)
diff --git a/python/scripts/mkvenv.py b/python/scripts/mkvenv.py
index 9c99122603..7a9a14124d 100644
--- a/python/scripts/mkvenv.py
+++ b/python/scripts/mkvenv.py
@@ -11,6 +11,8 @@
Commands:
command Description
createcreate a venv
+post_init
+ post-venv initialization
ensureEnsure that the specified package is installed.
--
@@ -25,6 +27,13 @@
--
+usage: mkvenv post_init [-h]
+
+options:
+ -h, --help show this help message and exit
+
+--
+
usage: mkvenv ensure [-h] [--online] [--dir DIR] dep_spec...
positional arguments:
@@ -189,6 +198,13 @@ def post_post_setup(self, context: SimpleNamespace) ->
None:
with open(pth_file, "w", encoding="UTF-8") as file:
file.write(parent_libpath + os.linesep)
+args = [
+context.env_exe,
+__file__,
+"post_init",
+]
+subprocess.run(args, check=True)
+
def get_value(self, field: str) -> str:
"""
Get a string value from the context namespace after a call to build.
@@ -727,6 +743,17 @@ def ensure(
raise Ouch(diagnose(dep_specs[0], online, wheels_dir, prog)) from exc
+def post_venv_setup() -> None:
+"""
+This is intended to be run *inside the venv* after it is created.
+"""
+logger.debug("post_venv_setup()")
+# Generate a 'pip' script so the venv is usable in a normal
+# way from the CLI. This only happens when we inherited pip from a
+# parent/system-site and haven't run ensurepip in some way.
+generate_console_scripts(["pip"])
+
+
def _add_create_subcommand(subparsers: Any) -> None:
subparser = subparsers.add_parser("create", help="create a venv")
subparser.add_argument(
@@ -737,6 +764,10 @@ def _add_create_subcommand(subparsers: Any) -> None:
)
+def _add_post_init_subcommand(subparsers: Any) -> None:
+subparsers.add_parser("post_init", help="post-venv initialization")
+
+
def _add_ensure_subcommand(subparsers: Any) -> None:
subparser = subparsers.add_parser(
"ensure", help="Ensure that the specified package is installed."
@@ -790,6 +821,7 @@ def main() -> int:
)
_add_create_subcommand(subparsers)
+_add_post_init_subcommand(subparsers)
_add_ensure_subcommand(subparsers)
args = parser.parse_args()
@@ -800,6 +832,8 @@ def main() -> int:
system_site_packages=True,
clear=True,
)
+if args.command == "post_init":
+post_venv_setup()
if args.command == "ensure":
ensure(
dep_specs=args.dep_specs,
--
2.40.0
[PATCH 02/27] python: update pylint configuration
Pylint 2.17.x decided that SocketAddrT was a bad name for a Type Alias for some reason. Sure, fine, whatever. Signed-off-by: John Snow Reviewed-by: Daniel P. Berrangé --- python/setup.cfg | 1 + 1 file changed, 1 insertion(+) diff --git a/python/setup.cfg b/python/setup.cfg index 9e923d9762..3f36502954 100644 --- a/python/setup.cfg +++ b/python/setup.cfg @@ -132,6 +132,7 @@ good-names=i, fd, # fd = os.open(...) c, # for c in string: ... T, # for TypeVars. See pylint#3401 + SocketAddrT, # Not sure why this is invalid. [pylint.similarities] # Ignore imports when computing similarities. -- 2.40.0
[PATCH 20/27] configure: move --enable-docs and --disable-docs back to configure
Move this option back from meson into configure for the purposes of
using the configuration value to bootstrap Sphinx in different ways
based on this value.
Signed-off-by: John Snow
---
configure | 6 ++
1 file changed, 6 insertions(+)
diff --git a/configure b/configure
index 6e4499a68a..3e74c54f29 100755
--- a/configure
+++ b/configure
@@ -279,6 +279,7 @@ debug_tcg="no"
sanitizers="no"
tsan="no"
fortify_source="yes"
+docs="auto"
EXESUF=""
modules="no"
prefix="/usr/local"
@@ -751,6 +752,10 @@ for opt do
;;
--disable-debug-info) meson_option_add -Ddebug=false
;;
+ --enable-docs) docs=enabled
+ ;;
+ --disable-docs) docs=disabled
+ ;;
--enable-modules)
modules="yes"
;;
@@ -2630,6 +2635,7 @@ if test "$skip_meson" = no; then
# QEMU options
test "$cfi" != false && meson_option_add "-Dcfi=$cfi"
+ test "$docs" != auto && meson_option_add "-Ddocs=$docs"
test "$fdt" != auto && meson_option_add "-Dfdt=$fdt"
test -n "${LIB_FUZZING_ENGINE+xxx}" && meson_option_add
"-Dfuzzing_engine=$LIB_FUZZING_ENGINE"
test "$qemu_suffix" != qemu && meson_option_add "-Dqemu_suffix=$qemu_suffix"
--
2.40.0
[PATCH 24/27] configure: Add courtesy hint to Python version failure message
If we begin requiring Python 3.7+, a few platforms are going to need to install an additional Python interpreter package. As a courtesy to the user, suggest the optional package they might need to install. This will hopefully minimize any downtime caused by the change in Python dependency. Signed-off-by: John Snow Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20230221012456.2607692-3-js...@redhat.com> Signed-off-by: Paolo Bonzini --- configure | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/configure b/configure index 4175bb49b8..ff654a9f45 100755 --- a/configure +++ b/configure @@ -1092,7 +1092,10 @@ fi if ! check_py_version "$python"; then error_exit "Cannot use '$python', Python >= 3.7 is required." \ - "Use --python=/path/to/python to specify a supported Python." + "Use --python=/path/to/python to specify a supported Python." \ + "Maybe try:" \ + " openSUSE Leap 15.3+: zypper install python39" \ + " CentOS 8: dnf install python38" fi # Resolve PATH -- 2.40.0
[PATCH 26/27] python: bump some of the dependencies
From: Paolo Bonzini The version of pyflakes that is listed in python/tests/minreqs.txt breaks on Python 3.8 with the following message: AttributeError: 'FlakesChecker' object has no attribute 'CONSTANT' Now that we do not support EOL'd Python versions anymore, we can update to newer, fixed versions. It is a good time to do so, before Python packages start dropping support for Python 3.7 as well! Signed-off-by: Paolo Bonzini Signed-off-by: John Snow --- python/setup.cfg | 4 ++-- python/tests/minreqs.txt | 14 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/python/setup.cfg b/python/setup.cfg index 355e4410b7..7d75c168a8 100644 --- a/python/setup.cfg +++ b/python/setup.cfg @@ -35,11 +35,11 @@ packages = # Remember to update tests/minreqs.txt if changing anything below: devel = avocado-framework >= 90.0 -flake8 >= 3.6.0 +flake8 >= 5.0.4 fusepy >= 2.0.4 isort >= 5.1.2 mypy >= 0.780 -pylint >= 2.8.0 +pylint >= 2.17.3 tox >= 3.18.0 urwid >= 2.1.2 urwid-readline >= 0.13 diff --git a/python/tests/minreqs.txt b/python/tests/minreqs.txt index 55cc6b41d8..efb9ceb937 100644 --- a/python/tests/minreqs.txt +++ b/python/tests/minreqs.txt @@ -23,23 +23,23 @@ fusepy==2.0.4 avocado-framework==90.0 # Linters -flake8==3.6.0 +flake8==5.0.4 isort==5.1.2 mypy==0.780 -pylint==2.8.0 +pylint==2.17.3 # Transitive flake8 dependencies -mccabe==0.6.0 -pycodestyle==2.4.0 -pyflakes==2.0.0 +mccabe==0.7.0 +pycodestyle==2.9.1 +pyflakes==2.5.0 # Transitive mypy dependencies mypy-extensions==0.4.3 typed-ast==1.4.0 -typing-extensions==3.7.4 +typing-extensions==4.5.0 # Transitive pylint dependencies -astroid==2.5.4 +astroid==2.15.4 lazy-object-proxy==1.4.0 toml==0.10.0 wrapt==1.12.1 -- 2.40.0
[PATCH 07/27] mkvenv: add diagnose() method for ensure() failures
This is a routine that is designed to print some usable info for human
beings back out to the terminal if/when "mkvenv ensure" fails to locate
or install a package during configure time, such as meson or sphinx.
Since we are requiring that "meson" and "sphinx" are installed to the
same Python environment as QEMU is configured to build with, this can
produce some surprising failures when things are mismatched. This method
is here to try and ease that sting by offering some actionable
diagnosis.
Signed-off-by: John Snow
---
python/scripts/mkvenv.py | 165 ---
1 file changed, 152 insertions(+), 13 deletions(-)
diff --git a/python/scripts/mkvenv.py b/python/scripts/mkvenv.py
index c1982589c9..d9ba2e1532 100644
--- a/python/scripts/mkvenv.py
+++ b/python/scripts/mkvenv.py
@@ -51,6 +51,8 @@
import logging
import os
from pathlib import Path
+import re
+import shutil
import site
import subprocess
import sys
@@ -351,6 +353,115 @@ def _stringify(data: Union[str, bytes]) -> str:
print(builder.get_value("env_exe"))
+def pkgname_from_depspec(dep_spec: str) -> str:
+"""
+Parse package name out of a PEP-508 depspec.
+
+See https://peps.python.org/pep-0508/#names
+"""
+match = re.match(
+r"^([A-Z0-9]([A-Z0-9._-]*[A-Z0-9])?)", dep_spec, re.IGNORECASE
+)
+if not match:
+raise ValueError(
+f"dep_spec '{dep_spec}'"
+" does not appear to contain a valid package name"
+)
+return match.group(0)
+
+
+def diagnose(
+dep_spec: str,
+online: bool,
+wheels_dir: Optional[Union[str, Path]],
+prog: Optional[str],
+) -> str:
+"""
+Offer a summary to the user as to why a package failed to be installed.
+
+:param dep_spec: The package we tried to ensure, e.g. 'meson>=0.61.5'
+:param online: Did we allow PyPI access?
+:param prog:
+Optionally, a shell program name that can be used as a
+bellwether to detect if this program is installed elsewhere on
+the system. This is used to offer advice when a program is
+detected for a different python version.
+:param wheels_dir:
+Optionally, a directory that was searched for vendored packages.
+"""
+# pylint: disable=too-many-branches
+
+pkg_name = pkgname_from_depspec(dep_spec)
+pkg_version = None
+
+has_importlib = False
+try:
+# Python 3.8+ stdlib
+# pylint: disable=import-outside-toplevel
+# pylint: disable=no-name-in-module
+# pylint: disable=import-error
+from importlib.metadata import ( # type: ignore
+PackageNotFoundError,
+version,
+)
+
+has_importlib = True
+try:
+pkg_version = version(pkg_name)
+except PackageNotFoundError:
+pass
+except ModuleNotFoundError:
+pass
+
+lines = []
+
+if pkg_version:
+lines.append(
+f"Python package '{pkg_name}' version '{pkg_version}' was found,"
+" but isn't suitable."
+)
+elif has_importlib:
+lines.append(
+f"Python package '{pkg_name}' was not found nor installed."
+)
+else:
+lines.append(
+f"Python package '{pkg_name}' is either not found or"
+" not a suitable version."
+)
+
+if wheels_dir:
+lines.append(
+"No suitable version found in, or failed to install from"
+f" '{wheels_dir}'."
+)
+else:
+lines.append("No local package directory was searched.")
+
+if online:
+lines.append("A suitable version could not be obtained from PyPI.")
+else:
+lines.append(
+"mkvenv was configured to operate offline and did not check PyPI."
+)
+
+if prog and not pkg_version:
+which = shutil.which(prog)
+if which:
+pypath = Path(sys.executable).resolve()
+lines.append(
+f"'{prog}' was detected on your system at '{which}', "
+f"but the Python package '{pkg_name}' was not found by this "
+f"Python interpreter ('{pypath}'). "
+f"Typically this means that '{prog}' has been installed "
+"against a different Python interpreter on your system."
+)
+
+lines = [f" • {line}" for line in lines]
+lines.insert(0, f"Could not ensure availability of '{dep_spec}':")
+return os.linesep.join(lines)
+
+
def pip_install(
args: Sequence[str],
online: bool = False,
@@ -387,23 +498,11 @@ def pip_install(
)
-def ensure(
+def _do_ensure(
dep_specs: Sequence[str],
online: bool = False,
wheels_dir: Optional[Union[str, Path]] = None,
) -> None:
-"""
-Use pip to ensure we have the package specified by @dep_specs.
-
-If the package is already installed, do nothing. If online and
-wheels_dir are both provided, prefer packages
[PATCH 15/27] configure: create a python venv unconditionally
This patch changes the configure script so that it always creates and
uses a python virtual environment unconditionally.
Meson bootstrapping is temporarily altered to force the use of meson
from git or vendored source (as packaged in our source tarballs). A
subsequent commit restores the use of distribution-vendored Meson.
Signed-off-by: John Snow
---
configure | 34 +-
1 file changed, 29 insertions(+), 5 deletions(-)
diff --git a/configure b/configure
index 243e2e0a0d..1d7db92ee3 100755
--- a/configure
+++ b/configure
@@ -625,7 +625,6 @@ check_py_version() {
python=
first_python=
if test -z "${PYTHON}"; then
-explicit_python=no
# A bare 'python' is traditionally python 2.x, but some distros
# have it as python 3.x, so check in both places.
for binary in python3 python python3.11 python3.10 python3.9 python3.8
python3.7 python3.6; do
@@ -644,7 +643,6 @@ else
# Same as above, but only check the environment variable.
has "${PYTHON}" || error_exit "The PYTHON environment variable does not
point to an executable"
python=$(command -v "$PYTHON")
-explicit_python=yes
if check_py_version "$python"; then
# This one is good.
first_python=
@@ -729,7 +727,7 @@ for opt do
;;
--install=*)
;;
- --python=*) python="$optarg" ; explicit_python=yes
+ --python=*) python="$optarg"
;;
--skip-meson) skip_meson=yes
;;
@@ -1090,8 +1088,34 @@ if ! check_py_version "$python"; then
"Use --python=/path/to/python to specify a supported Python."
fi
-# Resolve PATH + suppress writing compiled files
-python="$(command -v "$python") -B"
+# Resolve PATH
+python="$(command -v "$python")"
+explicit_python=yes
+
+# Create a Python virtual environment using our configured python.
+# The stdout of this script will be the location of a symlink that
+# points to the configured Python.
+# Entry point scripts for pip, meson, and sphinx are generated if those
+# packages are present.
+
+# Defaults assumed for now:
+# - venv is cleared if it exists already;
+# - venv is allowed to use system packages;
+# - all setup is performed **offline**;
+# - No packages are installed by default;
+# - pip is not installed into the venv when possible,
+# but ensurepip is called as a fallback when necessary.
+
+echo "python determined to be '$python'"
+echo "python version: $($python --version)"
+
+python="$($python -B "${source_path}/python/scripts/mkvenv.py" create pyvenv)"
+if test "$?" -ne 0 ; then
+error_exit "python venv creation failed"
+fi
+
+# Suppress writing compiled files
+python="$python -B"
has_meson() {
local python_dir=$(dirname "$python")
--
2.40.0
[PATCH 11/27] tests/docker: add python3-venv dependency
Several debian-based tests need the python3-venv dependency as a consequence of Debian debundling the "ensurepip" module normally included with Python. As mkvenv.py stands as of this commit, Debian requires EITHER: (A) setuptools and pip, or (B) ensurepip mkvenv is a few seconds faster if you have setuptools and pip, so developers should prefer the first requirement. For the purposes of CI, the time-save is a wash; it's only a matter of who is responsible for installing pip and when; the timing is about the same. Arbitrarily, I chose adding ensurepip to the test configuration because it is normally part of the Python stdlib, and always having it allows us a more consistent cross-platform environment. Signed-off-by: John Snow Reviewed-by: Daniel P. Berrangé --- tests/docker/dockerfiles/debian-all-test-cross.docker | 3 ++- tests/docker/dockerfiles/debian-hexagon-cross.docker | 3 ++- tests/docker/dockerfiles/debian-riscv64-cross.docker | 3 ++- tests/docker/dockerfiles/debian-tricore-cross.docker | 3 ++- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/docker/dockerfiles/debian-all-test-cross.docker b/tests/docker/dockerfiles/debian-all-test-cross.docker index 981e9bdc7b..f9f401544a 100644 --- a/tests/docker/dockerfiles/debian-all-test-cross.docker +++ b/tests/docker/dockerfiles/debian-all-test-cross.docker @@ -57,7 +57,8 @@ RUN DEBIAN_FRONTEND=noninteractive eatmydata \ gcc-sh4-linux-gnu \ libc6-dev-sh4-cross \ gcc-sparc64-linux-gnu \ -libc6-dev-sparc64-cross +libc6-dev-sparc64-cross \ +python3-venv ENV QEMU_CONFIGURE_OPTS --disable-system --disable-docs --disable-tools ENV DEF_TARGET_LIST aarch64-linux-user,alpha-linux-user,arm-linux-user,hppa-linux-user,i386-linux-user,m68k-linux-user,mips-linux-user,mips64-linux-user,mips64el-linux-user,mipsel-linux-user,ppc-linux-user,ppc64-linux-user,ppc64le-linux-user,riscv64-linux-user,s390x-linux-user,sh4-linux-user,sparc64-linux-user diff --git a/tests/docker/dockerfiles/debian-hexagon-cross.docker b/tests/docker/dockerfiles/debian-hexagon-cross.docker index b99d99f943..c2cfb6a5d0 100644 --- a/tests/docker/dockerfiles/debian-hexagon-cross.docker +++ b/tests/docker/dockerfiles/debian-hexagon-cross.docker @@ -20,7 +20,8 @@ RUN apt-get update && \ bison \ flex \ git \ -ninja-build && \ +ninja-build \ +python3-venv && \ # Install QEMU build deps for use in CI DEBIAN_FRONTEND=noninteractive eatmydata \ apt build-dep -yy --arch-only qemu diff --git a/tests/docker/dockerfiles/debian-riscv64-cross.docker b/tests/docker/dockerfiles/debian-riscv64-cross.docker index 803afb9573..081404e014 100644 --- a/tests/docker/dockerfiles/debian-riscv64-cross.docker +++ b/tests/docker/dockerfiles/debian-riscv64-cross.docker @@ -28,7 +28,8 @@ RUN DEBIAN_FRONTEND=noninteractive eatmydata apt install -yy \ libglib2.0-dev \ ninja-build \ pkg-config \ -python3 +python3 \ +python3-venv # Add ports and riscv64 architecture RUN echo "deb http://ftp.ports.debian.org/debian-ports/ sid main" >> /etc/apt/sources.list diff --git a/tests/docker/dockerfiles/debian-tricore-cross.docker b/tests/docker/dockerfiles/debian-tricore-cross.docker index cfd2faf9a8..269bfa8d42 100644 --- a/tests/docker/dockerfiles/debian-tricore-cross.docker +++ b/tests/docker/dockerfiles/debian-tricore-cross.docker @@ -33,7 +33,8 @@ RUN apt update && \ pkgconf \ python3-pip \ python3-setuptools \ - python3-wheel + python3-wheel \ + python3-venv RUN curl -#SL https://github.com/bkoppelmann/package_940/releases/download/tricore-toolchain-9.40/tricore-toolchain-9.4.0.tar.gz \ | tar -xzC /usr/local/ -- 2.40.0
[PATCH 23/27] Python: Drop support for Python 3.6
From: Paolo Bonzini
Python 3.6 was EOL 2021-12-31. Newer versions of upstream libraries have
begun dropping support for this version and it is becoming more
cumbersome to support. Avocado-framework and qemu.qmp each have their
own reasons for wanting to drop Python 3.6, but won't until QEMU does.
Versions of Python available in our supported build platforms as of today,
with optional versions available in parentheses:
openSUSE Leap 15.4: 3.6.15 (3.9.10, 3.10.2)
CentOS Stream 8:3.6.8 (3.8.13, 3.9.16)
CentOS Stream 9:3.9.13
Fedora 36: 3.10
Fedora 37: 3.11
Debian 11: 3.9.2
Alpine 3.14, 3.15: 3.9.16
Alpine 3.16, 3.17: 3.10.10
Ubuntu 20.04 LTS: 3.8.10
Ubuntu 22.04 LTS: 3.10.4
NetBSD 9.3: 3.9.13*
FreeBSD 12.4: 3.9.16
FreeBSD 13.1: 3.9.16
OpenBSD 7.2:3.9.16
Note: Our VM tests install 3.9 explicitly for FreeBSD and 3.10 for
NetBSD; the default for "python" or "python3" in FreeBSD is
3.9.16. NetBSD does not appear to have a default meta-package, but
offers several options, the lowest of which is 3.7.15. "python39"
appears to be a pre-requisite to one of the other packages we request in
tests/vm/netbsd. pip, ensurepip and other Python essentials are
currently only available for Python 3.10 for NetBSD.
CentOS and OpenSUSE support parallel installation of multiple Python
interpreters, and binaries in /usr/bin will always use Python 3.6. However,
the newly introduced support for virtual environments ensures that all build
steps that execute QEMU Python code use a single interpreter.
Since it is safe to under our supported platform policy, bump our
minimum supported version of Python to 3.7.
Signed-off-by: John Snow
Reviewed-by: Daniel P. Berrangé
Signed-off-by: Paolo Bonzini
---
docs/about/build-platforms.rst | 2 +-
configure | 10 +-
python/Makefile| 10 +-
python/setup.cfg | 7 +++
python/tests/minreqs.txt | 2 +-
scripts/qapi/mypy.ini | 2 +-
6 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/docs/about/build-platforms.rst b/docs/about/build-platforms.rst
index 89cae5a6bb..0e2cb9e770 100644
--- a/docs/about/build-platforms.rst
+++ b/docs/about/build-platforms.rst
@@ -98,7 +98,7 @@ Python runtime
option of the ``configure`` script to point QEMU to a supported
version of the Python runtime.
- As of QEMU |version|, the minimum supported version of Python is 3.6.
+ As of QEMU |version|, the minimum supported version of Python is 3.7.
Python build dependencies
Some of QEMU's build dependencies are written in Python. Usually these
diff --git a/configure b/configure
index 590a1a3323..4175bb49b8 100755
--- a/configure
+++ b/configure
@@ -618,9 +618,9 @@ esac
check_py_version() {
-# We require python >= 3.6.
+# We require python >= 3.7.
# NB: a True python conditional creates a non-zero return code (Failure)
-"$1" -c 'import sys; sys.exit(sys.version_info < (3,6))'
+"$1" -c 'import sys; sys.exit(sys.version_info < (3,7))'
}
python=
@@ -629,7 +629,7 @@ first_python=
if test -z "${PYTHON}"; then
# A bare 'python' is traditionally python 2.x, but some distros
# have it as python 3.x, so check in both places.
-for binary in python3 python python3.11 python3.10 python3.9 python3.8
python3.7 python3.6; do
+for binary in python3 python python3.11 python3.10 python3.9 python3.8
python3.7; do
if has "$binary"; then
python=$(command -v "$binary")
if check_py_version "$python"; then
@@ -1078,7 +1078,7 @@ then
# If first_python is set, there was a binary somewhere even though
# it was not suitable. Use it for the error message.
if test -n "$first_python"; then
-error_exit "Cannot use '$first_python', Python >= 3.6 is required." \
+error_exit "Cannot use '$first_python', Python >= 3.7 is required." \
"Use --python=/path/to/python to specify a supported Python."
else
error_exit "Python not found. Use --python=/path/to/python"
@@ -1091,7 +1091,7 @@ then
fi
if ! check_py_version "$python"; then
- error_exit "Cannot use '$python', Python >= 3.6 is required." \
+ error_exit "Cannot use '$python', Python >= 3.7 is required." \
"Use --python=/path/to/python to specify a supported Python."
fi
diff --git a/python/Makefile b/python/Makefile
index 47560657d2..7c70dcc8d1 100644
--- a/python/Makefile
+++ b/python/Makefile
@@ -9,14 +9,14 @@ help:
@echo "make check-minreqs:"
@echo "Run tests in the minreqs virtual environment."
@echo "These tests use the oldest dependencies."
- @echo "Requires: Python 3.6"
- @echo "Hint (Fedora): 'sudo dnf install python3.6'"
+ @echo "Requires: Python 3.7"
+ @echo "Hint (Fedora): 'sudo dnf install python3.7'"
@echo ""
@echo "make check-tox:"
@echo "Run tests
[PATCH 22/27] configure: add --enable-pypi and --disable-pypi
In the event that there's no vendored source present and no sufficient
version of $package can be found, we will attempt to connect to PyPI to
install the package if '--disable-pypi' was not passed.
This means that PyPI access is "enabled by default", but there are some
subtleties that make this action occur much less frequently than you
might imagine:
(1) While --enable-pypi is the default, vendored source will always be
preferred when found, making PyPI a fallback. This should ensure
that configure-time venv building "just works" for almost everyone
in almost every circumstance.
(2) Because meson source is, at time of writing, vendored directly into
qemu.git, PyPI will never be used for sourcing meson.
(3) Because Sphinx is an optional dependency, if docs are set to "auto",
PyPI will not be used to obtain Sphinx source as a fallback and
instead docs will be disabled. If PyPI sourcing of sphinx is
desired, --enable-docs should be passed to force the lookup. I chose
this as the default behavior to avoid adding new internet lookups to
a "default" invocation of configure.
Signed-off-by: John Snow
---
configure | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index 77d632e10d..590a1a3323 100755
--- a/configure
+++ b/configure
@@ -624,6 +624,7 @@ check_py_version() {
}
python=
+pypi="enabled"
first_python=
if test -z "${PYTHON}"; then
# A bare 'python' is traditionally python 2.x, but some distros
@@ -889,6 +890,10 @@ for opt do
--with-git-submodules=*)
git_submodules_action="$optarg"
;;
+ --disable-pypi) pypi="disabled"
+ ;;
+ --enable-pypi) pypi="enabled"
+ ;;
--enable-plugins) if test "$mingw32" = "yes"; then
error_exit "TCG plugins not currently supported on
Windows platforms"
else
@@ -1102,7 +1107,9 @@ python="$(command -v "$python")"
# Defaults assumed for now:
# - venv is cleared if it exists already;
# - venv is allowed to use system packages;
-# - all setup is performed **offline**;
+# - all setup can be performed offline;
+# - missing packages may be fetched from PyPI,
+# unless --disable-pypi is passed.
# - pip is not installed into the venv when possible,
# but ensurepip is called as a fallback when necessary.
@@ -1118,7 +1125,13 @@ fi
python="$python -B"
mkvenv="$python ${source_path}/python/scripts/mkvenv.py"
+mkvenv_flags=""
+if test "$pypi" = "enabled" ; then
+mkvenv_flags="--online"
+fi
+
if ! $mkvenv ensure \
+ $mkvenv_flags \
--dir "${source_path}/python/wheels" \
--diagnose "meson" \
"meson>=0.61.5" ;
@@ -1149,8 +1162,14 @@ fi
# Conditionally ensure Sphinx is installed.
+mkvenv_flags=""
+if test "$pypi" = "enabled" -a "$docs" = "enabled" ; then
+mkvenv_flags="--online"
+fi
+
if test "$docs" != "disabled" ; then
if ! $mkvenv ensure \
+ $mkvenv_flags \
--diagnose "sphinx-build" \
"sphinx>=1.6.0" sphinx_rtd_theme;
then
--
2.40.0
[PATCH 18/27] qemu.git: drop meson git submodule
Now that meson is installed from a vendored wheel, we don't need the git submodule anymore. Drop it. Signed-off-by: John Snow --- .gitmodules | 3 --- meson | 1 - 2 files changed, 4 deletions(-) delete mode 16 meson diff --git a/.gitmodules b/.gitmodules index 6ce5bf49c5..2a3a12033c 100644 --- a/.gitmodules +++ b/.gitmodules @@ -49,9 +49,6 @@ [submodule "roms/qboot"] path = roms/qboot url = https://gitlab.com/qemu-project/qboot.git -[submodule "meson"] - path = meson - url = https://gitlab.com/qemu-project/meson.git [submodule "roms/vbootrom"] path = roms/vbootrom url = https://gitlab.com/qemu-project/vbootrom.git diff --git a/meson b/meson deleted file mode 16 index 3a9b285a55..00 --- a/meson +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 3a9b285a55b91b53b2acda987192274352ecb5be -- 2.40.0
[PATCH 19/27] tests: Use configure-provided pyvenv for tests
This patch changes how the avocado tests are provided, ever so slightly. Instead of creating a new testing venv, use the configure-provided 'pyvenv' instead, and install optional packages into that. Signed-off-by: John Snow --- docs/devel/acpi-bits.rst | 6 +++--- docs/devel/testing.rst | 14 +++--- .gitlab-ci.d/buildtest.yml | 6 +++--- scripts/ci/org.centos/stream/8/x86_64/test-avocado | 4 ++-- scripts/device-crash-test | 2 +- tests/Makefile.include | 10 +- tests/requirements.txt | 7 +-- 7 files changed, 26 insertions(+), 23 deletions(-) diff --git a/docs/devel/acpi-bits.rst b/docs/devel/acpi-bits.rst index 22e2580200..9677b0098f 100644 --- a/docs/devel/acpi-bits.rst +++ b/docs/devel/acpi-bits.rst @@ -61,19 +61,19 @@ Under ``tests/avocado/`` as the root we have: :: $ make check-venv (needed only the first time to create the venv) - $ ./tests/venv/bin/avocado run -t acpi tests/avocado + $ ./pyvenv/bin/avocado run -t acpi tests/avocado The above will run all acpi avocado tests including this one. In order to run the individual tests, perform the following: :: - $ ./tests/venv/bin/avocado run tests/avocado/acpi-bits.py --tap - + $ ./pyvenv/bin/avocado run tests/avocado/acpi-bits.py --tap - The above will produce output in tap format. You can omit "--tap -" in the end and it will produce output like the following: :: - $ ./tests/venv/bin/avocado run tests/avocado/acpi-bits.py + $ ./pyvenv/bin/avocado run tests/avocado/acpi-bits.py Fetching asset from tests/avocado/acpi-bits.py:AcpiBitsTest.test_acpi_smbios_bits JOB ID : eab225724da7b64c012c65705dc2fa14ab1defef JOB LOG: /home/anisinha/avocado/job-results/job-2022-10-10T17.58-eab2257/job.log diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst index 4071e72710..50664d9eb9 100644 --- a/docs/devel/testing.rst +++ b/docs/devel/testing.rst @@ -882,9 +882,9 @@ You can run the avocado tests simply by executing: make check-avocado -This involves the automatic creation of Python virtual environment -within the build tree (at ``tests/venv``) which will have all the -right dependencies, and will save tests results also within the +This involves the automatic installation, from PyPI, of all the +necessary avocado-framework dependencies into the QEMU venv within the +build tree (at ``./pyvenv``). Test results are also saved within the build tree (at ``tests/results``). Note: the build environment must be using a Python 3 stack, and have @@ -941,7 +941,7 @@ may be invoked by running: .. code:: - tests/venv/bin/avocado run $OPTION1 $OPTION2 tests/avocado/ + pyvenv/bin/avocado run $OPTION1 $OPTION2 tests/avocado/ Note that if ``make check-avocado`` was not executed before, it is possible to create the Python virtual environment with the dependencies @@ -956,20 +956,20 @@ a test file. To run tests from a single file within the build tree, use: .. code:: - tests/venv/bin/avocado run tests/avocado/$TESTFILE + pyvenv/bin/avocado run tests/avocado/$TESTFILE To run a single test within a test file, use: .. code:: - tests/venv/bin/avocado run tests/avocado/$TESTFILE:$TESTCLASS.$TESTNAME + pyvenv/bin/avocado run tests/avocado/$TESTFILE:$TESTCLASS.$TESTNAME Valid test names are visible in the output from any previous execution of Avocado or ``make check-avocado``, and can also be queried using: .. code:: - tests/venv/bin/avocado list tests/avocado + pyvenv/bin/avocado list tests/avocado Manual Installation ~~~ diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml index bb3650a51c..307cba1aab 100644 --- a/.gitlab-ci.d/buildtest.yml +++ b/.gitlab-ci.d/buildtest.yml @@ -103,7 +103,7 @@ crash-test-debian: script: - cd build - make NINJA=":" check-venv -- tests/venv/bin/python3 scripts/device-crash-test -q --tcg-only ./qemu-system-i386 +- pyvenv/bin/python3 scripts/device-crash-test -q --tcg-only ./qemu-system-i386 build-system-fedora: extends: @@ -146,8 +146,8 @@ crash-test-fedora: script: - cd build - make NINJA=":" check-venv -- tests/venv/bin/python3 scripts/device-crash-test -q ./qemu-system-ppc -- tests/venv/bin/python3 scripts/device-crash-test -q ./qemu-system-riscv32 +- pyvenv/bin/python3 scripts/device-crash-test -q ./qemu-system-ppc +- pyvenv/bin/python3 scripts/device-crash-test -q ./qemu-system-riscv32 build-system-centos: extends: diff --git a/scripts/ci/org.centos/stream/8/x86_64/test-avocado b/scripts/ci/org.centos/stream/8/x86_64/test-avocado index d2c0e5fb4c..7bb5b317b6 100755 --- a/scripts/ci/org.centos/stream/8/x86_64/test-avocado +++ b/scripts/ci/org.centos/stream/8/x86_64/test-avocado @@ -4,7 +4,7 @@ # KVM and x86_64,
[PATCH 27/27] mkvenv.py: experiment; use distlib to generate script entry points
This is an experiment: by using pip's internal vendored distlib, we can
generate script entry points for Windows, Mac and Linux using distlib's
mechanisms. This is the same mechanism used when running "pip install",
so it will be consistent with the most common method of package
installation on all platforms. It also allows us to delete a good bit of
vendored/borrowed code from inside of mkvenv.py, so there's less to
maintain and the license might be more straightforward.
As a downside, if we're not willing to actually add a distlib
requirement, we have to piggy-back on pip's vendored version, which
could have downsides if they move our cheese in the future.
Signed-off-by: John Snow
---
configure| 16 +-
python/scripts/mkvenv.py | 117 +--
python/setup.cfg | 7 ++-
3 files changed, 46 insertions(+), 94 deletions(-)
diff --git a/configure b/configure
index ff654a9f45..b559bdc040 100755
--- a/configure
+++ b/configure
@@ -1147,21 +1147,7 @@ fi
# We ignore PATH completely here: we want to use the venv's Meson
# *exclusively*.
-# "mkvenv ensure" has a limitation compared to "pip install": it is not
-# able to create launcher .exe files on Windows. This limitation exists
-# because "py.exe" is not guaranteed to exist on the machine (pip/setuptools
-# work around the issue by bundling the .exe files as resources).
-# This is not a problem for msys, since it emulates a POSIX environment;
-# it is also okay for programs that meson.build looks up with find_program(),
-# because in that case Meson checks the file for a shebang line. However,
-# when meson wants to invoke itself as part of a build recipe, we need
-# to convince it to put the python interpreter in front of the path to the
-# script. To do so, run it using '-m'.
-if test "$targetos" = windows; then
- meson="$python -m mesonbuild.mesonmain"
-else
- meson="$(cd pyvenv/bin; pwd)/meson"
-fi
+meson="$(cd pyvenv/bin; pwd)/meson"
# Conditionally ensure Sphinx is installed.
diff --git a/python/scripts/mkvenv.py b/python/scripts/mkvenv.py
index 8e097e4759..8faec0957b 100644
--- a/python/scripts/mkvenv.py
+++ b/python/scripts/mkvenv.py
@@ -63,14 +63,12 @@
import re
import shutil
import site
-import stat
import subprocess
import sys
import sysconfig
from types import SimpleNamespace
from typing import (
Any,
-Dict,
Iterator,
Optional,
Sequence,
@@ -376,7 +374,7 @@ def _stringify(data: Union[str, bytes]) -> str:
print(builder.get_value("env_exe"))
-def _gen_importlib(packages: Sequence[str]) -> Iterator[Dict[str, str]]:
+def _gen_importlib(packages: Sequence[str]) -> Iterator[str]:
# pylint: disable=import-outside-toplevel
# pylint: disable=no-name-in-module
# pylint: disable=import-error
@@ -394,14 +392,7 @@ def _gen_importlib(packages: Sequence[str]) ->
Iterator[Dict[str, str]]:
distribution,
)
-# Borrowed from CPython (Lib/importlib/metadata/__init__.py)
-pattern = re.compile(
-r"(?P[\w.]+)\s*"
-r"(:\s*(?P[\w.]+)\s*)?"
-r"((?P\[.*\])\s*)?$"
-)
-
-def _generator() -> Iterator[Dict[str, str]]:
+def _generator() -> Iterator[str]:
for package in packages:
try:
entry_points = distribution(package).entry_points
@@ -415,34 +406,17 @@ def _generator() -> Iterator[Dict[str, str]]:
)
for entry_point in entry_points:
-# Python 3.8 doesn't have 'module' or 'attr' attributes
-if not (
-hasattr(entry_point, "module")
-and hasattr(entry_point, "attr")
-):
-match = pattern.match(entry_point.value)
-assert match is not None
-module = match.group("module")
-attr = match.group("attr")
-else:
-module = entry_point.module
-attr = entry_point.attr
-yield {
-"name": entry_point.name,
-"module": module,
-"import_name": attr,
-"func": attr,
-}
+yield f"{entry_point.name} = {entry_point.value}"
return _generator()
-def _gen_pkg_resources(packages: Sequence[str]) -> Iterator[Dict[str, str]]:
+def _gen_pkg_resources(packages: Sequence[str]) -> Iterator[str]:
# pylint: disable=import-outside-toplevel
# Bundled with setuptools; has a good chance of being available.
import pkg_resources
-def _generator() -> Iterator[Dict[str, str]]:
+def _generator() -> Iterator[str]:
for package in packages:
try:
eps = pkg_resources.get_entry_map(package, "console_scripts")
@@ -450,28 +424,11 @@ def _generator() -> Iterator[Dict[str, str]]:
continue
for entry_point in eps.values():
-
[PATCH 12/27] tests/vm: Configure netbsd to use Python 3.10
NetBSD removes some packages from the Python stdlib, but only re-packages them for Python 3.10. Switch to using Python 3.10. Signed-off-by: John Snow Reviewed-by: Daniel P. Berrangé --- tests/vm/netbsd | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/vm/netbsd b/tests/vm/netbsd index 0b9536ca17..13eae109c0 100755 --- a/tests/vm/netbsd +++ b/tests/vm/netbsd @@ -30,6 +30,7 @@ class NetBSDVM(basevm.BaseVM): "git-base", "pkgconf", "xz", +"python310", "ninja-build", # gnu tools -- 2.40.0
[PATCH 01/27] python: shut up "pip install" during "make check-minreqs"
From: Paolo Bonzini "make check-minreqs" runs pip without the --disable-pip-version-check option, which causes the obnoxious "A new release of pip available" message. Recent versions of pip also complain that some of the dependencies in our virtual environment rely on "setup.py install" instead of providing a pyproject.toml file; apparently it is deprecated to install them directly from pip instead of letting the "wheel" package take care of them. So, install "wheel" in the virtual environment. Signed-off-by: Paolo Bonzini Signed-off-by: John Snow --- python/Makefile | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/python/Makefile b/python/Makefile index c5bd6ff83a..47560657d2 100644 --- a/python/Makefile +++ b/python/Makefile @@ -54,6 +54,7 @@ pipenv check-pipenv: @echo "pipenv was dropped; try 'make check-minreqs' or 'make min-venv'" @exit 1 +PIP_INSTALL = pip install --disable-pip-version-check .PHONY: min-venv min-venv: $(QEMU_MINVENV_DIR) $(QEMU_MINVENV_DIR)/bin/activate $(QEMU_MINVENV_DIR) $(QEMU_MINVENV_DIR)/bin/activate: setup.cfg tests/minreqs.txt @@ -62,10 +63,12 @@ $(QEMU_MINVENV_DIR) $(QEMU_MINVENV_DIR)/bin/activate: setup.cfg tests/minreqs.tx @( \ echo "ACTIVATE $(QEMU_MINVENV_DIR)";\ . $(QEMU_MINVENV_DIR)/bin/activate; \ + echo "INSTALL wheel $(QEMU_MINVENV_DIR)"; \ + $(PIP_INSTALL) wheel 1>/dev/null; \ echo "INSTALL -r tests/minreqs.txt $(QEMU_MINVENV_DIR)";\ - pip install -r tests/minreqs.txt 1>/dev/null; \ + $(PIP_INSTALL) -r tests/minreqs.txt 1>/dev/null;\ echo "INSTALL -e qemu $(QEMU_MINVENV_DIR)"; \ - pip install -e . 1>/dev/null; \ + $(PIP_INSTALL) -e . 1>/dev/null;\ ) @touch $(QEMU_MINVENV_DIR) @@ -100,7 +103,7 @@ check-dev: dev-venv .PHONY: develop develop: - pip3 install --disable-pip-version-check -e .[devel] + $(PIP_INSTALL) -e .[devel] .PHONY: check check: -- 2.40.0
[PATCH 25/27] mkvenv: mark command as required
From: Paolo Bonzini This is only available in Python 3.7+. Signed-off-by: Paolo Bonzini Signed-off-by: John Snow --- python/scripts/mkvenv.py | 1 + 1 file changed, 1 insertion(+) diff --git a/python/scripts/mkvenv.py b/python/scripts/mkvenv.py index 31193b3b22..8e097e4759 100644 --- a/python/scripts/mkvenv.py +++ b/python/scripts/mkvenv.py @@ -856,6 +856,7 @@ def main() -> int: subparsers = parser.add_subparsers( title="Commands", dest="command", +required=True, metavar="command", help="Description", ) -- 2.40.0
[PATCH 14/27] python: add vendor.py utility
This is a teeny-tiny script that just downloads any packages we want to
vendor from PyPI and stores them in qemu.git/python/wheels/. If I'm hit
by a meteor, it'll be easy to replicate what I have done in order to
udpate the vendored source.
We don't really care which python runs it; it exists as a meta-utility
with no external dependencies and we won't package or install it. It
will be monitored by the linters/type checkers, though; so it's
guaranteed safe on python 3.6+.
Signed-off-by: John Snow
---
python/scripts/vendor.py | 74
1 file changed, 74 insertions(+)
create mode 100755 python/scripts/vendor.py
diff --git a/python/scripts/vendor.py b/python/scripts/vendor.py
new file mode 100755
index 00..23708430ea
--- /dev/null
+++ b/python/scripts/vendor.py
@@ -0,0 +1,74 @@
+#!/usr/bin/env python3
+"""
+vendor - QEMU python vendoring utility
+
+usage: vendor [-h]
+
+QEMU python vendoring utility
+
+options:
+ -h, --help show this help message and exit
+"""
+
+# Copyright (C) 2023 Red Hat, Inc.
+#
+# Authors:
+# John Snow
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later. See the COPYING file in the top-level directory.
+
+import argparse
+import os
+from pathlib import Path
+import subprocess
+import sys
+import tempfile
+
+
+def main() -> int:
+"""Run the vendoring utility. See module-level docstring."""
+loud = False
+if os.environ.get("DEBUG") or os.environ.get("V"):
+loud = True
+
+# No options or anything for now, but I guess
+# you'll figure that out when you run --help.
+parser = argparse.ArgumentParser(
+prog="vendor",
+description="QEMU python vendoring utility",
+)
+parser.parse_args()
+
+packages = {
+"meson==0.61.5":
+"58c2ddb5f885da0e929f15d89f38d8a7d97f981f56815bcba008414f8511f59a",
+}
+
+vendor_dir = Path(__file__, "..", "..", "wheels").resolve()
+
+with tempfile.NamedTemporaryFile(mode="w", encoding="utf-8") as file:
+for dep_spec, checksum in packages.items():
+file.write(f"{dep_spec} --hash=sha256:{checksum}")
+file.flush()
+
+cli_args = [
+"pip",
+"download",
+"--dest",
+str(vendor_dir),
+"--require-hashes",
+"-r",
+file.name,
+]
+if loud:
+cli_args.append("-v")
+
+print(" ".join(cli_args))
+subprocess.run(cli_args, check=True)
+
+return 0
+
+
+if __name__ == "__main__":
+sys.exit(main())
--
2.40.0
[PATCH 21/27] configure: bootstrap sphinx with mkvenv
When docs are explicitly requested, require Sphinx>=1.6.0. When docs are
explicitly disabled, don't bother to check for Sphinx at all. If docs
are set to "auto", attempt to locate Sphinx, but continue onward if it
wasn't located.
Signed-off-by: Paolo Bonzini
Signed-off-by: John Snow
---
docs/conf.py | 9 -
docs/meson.build | 2 +-
configure | 21 +++--
meson_options.txt | 2 --
scripts/meson-buildoptions.sh | 3 ---
5 files changed, 20 insertions(+), 17 deletions(-)
diff --git a/docs/conf.py b/docs/conf.py
index 00767b0e24..c687ff2663 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -32,15 +32,6 @@
from distutils.version import LooseVersion
from sphinx.errors import ConfigError
-# Make Sphinx fail cleanly if using an old Python, rather than obscurely
-# failing because some code in one of our extensions doesn't work there.
-# In newer versions of Sphinx this will display nicely; in older versions
-# Sphinx will also produce a Python backtrace but at least the information
-# gets printed...
-if sys.version_info < (3,6):
-raise ConfigError(
-"QEMU requires a Sphinx that uses Python 3.6 or better\n")
-
# The per-manual conf.py will set qemu_docdir for a single-manual build;
# otherwise set it here if this is an entire-manual-set build.
# This is always the absolute path of the docs/ directory in the source tree.
diff --git a/docs/meson.build b/docs/meson.build
index f220800e3e..e4301703b4 100644
--- a/docs/meson.build
+++ b/docs/meson.build
@@ -1,4 +1,4 @@
-sphinx_build = find_program(get_option('sphinx_build'),
+sphinx_build = find_program(fs.parent(python.full_path()) / 'sphinx-build',
required: get_option('docs'))
# Check if tools are available to build documentation.
diff --git a/configure b/configure
index 3e74c54f29..77d632e10d 100755
--- a/configure
+++ b/configure
@@ -1116,9 +1116,9 @@ fi
# Suppress writing compiled files
python="$python -B"
+mkvenv="$python ${source_path}/python/scripts/mkvenv.py"
-
-if ! $python "${source_path}/python/scripts/mkvenv.py" ensure \
+if ! $mkvenv ensure \
--dir "${source_path}/python/wheels" \
--diagnose "meson" \
"meson>=0.61.5" ;
@@ -1147,6 +1147,23 @@ else
meson="$(cd pyvenv/bin; pwd)/meson"
fi
+# Conditionally ensure Sphinx is installed.
+
+if test "$docs" != "disabled" ; then
+if ! $mkvenv ensure \
+ --diagnose "sphinx-build" \
+ "sphinx>=1.6.0" sphinx_rtd_theme;
+then
+if test "$docs" = "enabled" ; then
+exit 1
+fi
+echo "Sphinx not found/usable, disabling docs."
+docs=disabled
+else
+docs=enabled
+fi
+fi
+
# Probe for ninja
if test -z "$ninja"; then
diff --git a/meson_options.txt b/meson_options.txt
index d8330a1f71..a350520f6a 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -12,8 +12,6 @@ option('pkgversion', type : 'string', value : '',
description: 'use specified string as sub-version of the package')
option('smbd', type : 'string', value : '',
description: 'Path to smbd for slirp networking')
-option('sphinx_build', type : 'string', value : 'sphinx-build',
- description: 'Use specified sphinx-build for building document')
option('iasl', type : 'string', value : '',
description: 'Path to ACPI disassembler')
option('tls_priority', type : 'string', value : 'NORMAL',
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index 2805d1c145..fedb93ada6 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -57,8 +57,6 @@ meson_options_help() {
printf "%s\n" ' --localedir=VALUELocale data directory
[share/locale]'
printf "%s\n" ' --localstatedir=VALUELocalstate data directory
[/var/local]'
printf "%s\n" ' --mandir=VALUE Manual page directory [share/man]'
- printf "%s\n" ' --sphinx-build=VALUE Use specified sphinx-build for
building document'
- printf "%s\n" ' [sphinx-build]'
printf "%s\n" ' --sysconfdir=VALUE Sysconf data directory [etc]'
printf "%s\n" ' --tls-priority=VALUE Default TLS protocol/cipher
priority string'
printf "%s\n" ' [NORMAL]'
@@ -425,7 +423,6 @@ _meson_option_parse() {
--disable-sndio) printf "%s" -Dsndio=disabled ;;
--enable-sparse) printf "%s" -Dsparse=enabled ;;
--disable-sparse) printf "%s" -Dsparse=disabled ;;
---sphinx-build=*) quote_sh "-Dsphinx_build=$2" ;;
--enable-spice) printf "%s" -Dspice=enabled ;;
--disable-spice) printf "%s" -Dspice=disabled ;;
--enable-spice-protocol) printf "%s" -Dspice_protocol=enabled ;;
--
2.40.0
[PATCH 05/27] mkvenv: add nested venv workaround
Python virtual environments do not typically nest; they may inherit from
the top-level system packages or not at all.
For our purposes, it would be convenient to emulate "nested" virtual
environments to allow callers of the configure script to install
specific versions of python utilities in order to test build system
features, utility version compatibility, etc.
While it is possible to install packages into the system environment
(say, by using the --user flag), it's nicer to install test packages
into a totally isolated environment instead.
As detailed in https://www.qemu.org/2023/03/24/python/, Emulate a nested
venv environment by using .pth files installed into the site-packages
folder that points to the parent environment when appropriate.
Signed-off-by: Paolo Bonzini
Signed-off-by: John Snow
---
python/scripts/mkvenv.py | 92 +---
1 file changed, 87 insertions(+), 5 deletions(-)
diff --git a/python/scripts/mkvenv.py b/python/scripts/mkvenv.py
index dbcd488c12..5ee9967421 100644
--- a/python/scripts/mkvenv.py
+++ b/python/scripts/mkvenv.py
@@ -38,8 +38,10 @@
import logging
import os
from pathlib import Path
+import site
import subprocess
import sys
+import sysconfig
from types import SimpleNamespace
from typing import Any, Optional, Union
import venv
@@ -52,6 +54,11 @@
logger = logging.getLogger("mkvenv")
+def inside_a_venv() -> bool:
+"""Returns True if it is executed inside of a virtual environment."""
+return sys.prefix != sys.base_prefix
+
+
class Ouch(RuntimeError):
"""An Exception class we can't confuse with a builtin."""
@@ -60,10 +67,9 @@ class QemuEnvBuilder(venv.EnvBuilder):
"""
An extension of venv.EnvBuilder for building QEMU's configure-time venv.
-As of this commit, it does not yet do anything particularly
-different than the standard venv-creation utility. The next several
-commits will gradually change that in small commits that highlight
-each feature individually.
+The primary difference is that it emulates a "nested" virtual
+environment when invoked from inside of an existing virtual
+environment by including packages from the parent.
Parameters for base class init:
- system_site_packages: bool = False
@@ -77,16 +83,89 @@ class QemuEnvBuilder(venv.EnvBuilder):
def __init__(self, *args: Any, **kwargs: Any) -> None:
logger.debug("QemuEnvBuilder.__init__(...)")
+
+# For nested venv emulation:
+self.use_parent_packages = False
+if inside_a_venv():
+# Include parent packages only if we're in a venv and
+# system_site_packages was True.
+self.use_parent_packages = kwargs.pop(
+"system_site_packages", False
+)
+# Include system_site_packages only when the parent,
+# The venv we are currently in, also does so.
+kwargs["system_site_packages"] = sys.base_prefix in site.PREFIXES
+
super().__init__(*args, **kwargs)
# Make the context available post-creation:
self._context: Optional[SimpleNamespace] = None
+def get_parent_libpath(self) -> Optional[str]:
+"""Return the libpath of the parent venv, if applicable."""
+if self.use_parent_packages:
+return sysconfig.get_path("purelib")
+return None
+
+@staticmethod
+def compute_venv_libpath(context: SimpleNamespace) -> str:
+"""
+Compatibility wrapper for context.lib_path for Python < 3.12
+"""
+# Python 3.12+, not strictly necessary because it's documented
+# to be the same as 3.10 code below:
+if sys.version_info >= (3, 12):
+return context.lib_path
+
+# Python 3.10+
+if "venv" in sysconfig.get_scheme_names():
+lib_path = sysconfig.get_path(
+"purelib", scheme="venv", vars={"base": context.env_dir}
+)
+assert lib_path is not None
+return lib_path
+
+# For Python <= 3.9 we need to hardcode this. Fortunately the
+# code below was the same in Python 3.6-3.10, so there is only
+# one case.
+if sys.platform == "win32":
+return os.path.join(context.env_dir, "Lib", "site-packages")
+return os.path.join(
+context.env_dir,
+"lib",
+"python%d.%d" % sys.version_info[:2],
+"site-packages",
+)
+
def ensure_directories(self, env_dir: DirType) -> SimpleNamespace:
logger.debug("ensure_directories(env_dir=%s)", env_dir)
self._context = super().ensure_directories(env_dir)
return self._context
+def create(self, env_dir: DirType) -> None:
+logger.debug("create(env_dir=%s)", env_dir)
+super().create(env_dir)
+assert self._context is not None
+self.post_post_setup(self._context)
+
+def post_post_setup(self,
[PATCH 13/27] tests/vm: add py310-expat to NetBSD
NetBSD cannot successfully run "ensurepip" without access to the pyexpat module, which NetBSD debundles. Like the Debian patch, it would be strictly faster long term to install pip/setuptools, and I recommend developers at their workstations take that approach instead. For the purposes of a throwaway VM, there's not really a speed difference for who is responsible for installing pip; us (needs py310-pip) or Python (needs py310-expat). Signed-off-by: John Snow Reviewed-by: Daniel P. Berrangé --- tests/vm/netbsd | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/vm/netbsd b/tests/vm/netbsd index 13eae109c0..c7e3f1e735 100755 --- a/tests/vm/netbsd +++ b/tests/vm/netbsd @@ -31,6 +31,7 @@ class NetBSDVM(basevm.BaseVM): "pkgconf", "xz", "python310", +"py310-expat", "ninja-build", # gnu tools -- 2.40.0
[PATCH 17/27] configure: use 'mkvenv ensure meson' to bootstrap meson
This commit changes how we detect and install meson. It notably removes
'--meson='.
Currently, configure creates a lightweight Python virtual environment
unconditionally using the user's configured $python that inherits system
packages. Temporarily, we forced the use of meson source present via git
submodule or in the release tarball.
With this patch, we restore the ability to use a system-provided meson:
If Meson is installed in the build venv and meets our minimum version
requirements, we will use that Meson. This includes a system provided
meson, which would be visible via system-site packages inside the venv.
In the event that Meson is installed but *not for the chosen Python
interpreter*, not found, or of insufficient version, we will attempt to
install Meson from vendored source into the newly created Python virtual
environment. This vendored installation replaces both the git submodule
and tarball source mechanisms for sourcing meson.
As a result of this patch, the Python interpreter we use for both our
own build scripts *and* Meson extensions are always known to be the
exact same Python. As a further benefit, there will also be a symlink
available in the build directory that points to the correct, configured
python and can be used by e.g. manual tests to invoke the correct,
configured Python unambiguously.
Signed-off-by: John Snow
---
configure | 80 ++---
.gitlab-ci.d/buildtest-template.yml | 4 +-
python/scripts/mkvenv.py| 4 ++
3 files changed, 32 insertions(+), 56 deletions(-)
diff --git a/configure b/configure
index 1d7db92ee3..6e4499a68a 100755
--- a/configure
+++ b/configure
@@ -731,8 +731,6 @@ for opt do
;;
--skip-meson) skip_meson=yes
;;
- --meson=*) meson="$optarg"
- ;;
--ninja=*) ninja="$optarg"
;;
--smbd=*) smbd="$optarg"
@@ -1017,7 +1015,6 @@ Advanced options (experts only):
--cross-prefix-ARCH=PREFIX cross compiler prefix when building ARCH guest
test cases
--make=MAKE use specified make [$make]
--python=PYTHON use specified python [$python]
- --meson=MESONuse specified meson [$meson]
--ninja=NINJAuse specified ninja [$ninja]
--smbd=SMBD use specified smbd [$smbd]
--with-git=GIT use specified git [$git]
@@ -1090,7 +1087,6 @@ fi
# Resolve PATH
python="$(command -v "$python")"
-explicit_python=yes
# Create a Python virtual environment using our configured python.
# The stdout of this script will be the location of a symlink that
@@ -1102,7 +1098,6 @@ explicit_python=yes
# - venv is cleared if it exists already;
# - venv is allowed to use system packages;
# - all setup is performed **offline**;
-# - No packages are installed by default;
# - pip is not installed into the venv when possible,
# but ensurepip is called as a fallback when necessary.
@@ -1117,59 +1112,36 @@ fi
# Suppress writing compiled files
python="$python -B"
-has_meson() {
- local python_dir=$(dirname "$python")
- # PEP405: pyvenv.cfg is either adjacent to the Python executable
- # or one directory above
- if test -f $python_dir/pyvenv.cfg || test -f $python_dir/../pyvenv.cfg; then
-# Ensure that Meson and Python come from the same virtual environment
-test -x "$python_dir/meson" &&
- test "$(command -v meson)" -ef "$python_dir/meson"
- else
-has meson
- fi
-}
-if test -z "$meson"; then
-if test "$explicit_python" = no && has_meson && version_ge "$(meson
--version)" 0.61.5; then
-meson=meson
-elif test "$git_submodules_action" != 'ignore' ; then
-meson=git
-elif test -e "${source_path}/meson/meson.py" ; then
-meson=internal
-else
-if test "$explicit_python" = yes; then
-error_exit "--python requires using QEMU's embedded Meson
distribution, but it was not found."
-else
-error_exit "Meson not found. Use --meson=/path/to/meson"
-fi
-fi
+if ! $python "${source_path}/python/scripts/mkvenv.py" ensure \
+ --dir "${source_path}/python/wheels" \
+ --diagnose "meson" \
+ "meson>=0.61.5" ;
+then
+exit 1
+fi
+
+# At this point, we expect Meson to be installed and available.
+# We expect mkvenv or pip to have created pyvenv/bin/meson for us.
+# We ignore PATH completely here: we want to use the venv's Meson
+# *exclusively*.
+
+# "mkvenv ensure" has a limitation compared to "pip install": it is not
+# able to create launcher .exe files on Windows. This limitation exists
+# because "py.exe" is not guaranteed to exist on the machine (pip/setuptools
+# work around the issue by bundling the .exe files as resources).
+# This is not a problem for msys, since it emulates a POSIX environment;
+# it is also okay for programs that meson.build looks up with find_program(),
+# because in that case Meson checks the file for a shebang line. However,
+# when meson wants to invoke itself as part of a build recipe,
[PATCH 06/27] mkvenv: add ensure subcommand
This command is to be used to add various packages (or ensure they're
already present) into the configure-provided venv in a modular fashion.
Examples:
mkvenv ensure --online --dir "${source_dir}/python/wheels/" "meson>=0.61.5"
mkvenv ensure --online "sphinx>=1.6.0"
mkvenv ensure "qemu.qmp==0.0.2"
It's designed to look for packages in three places, in order:
(1) In system packages, if the version installed is already good
enough. This way your distribution-provided meson, sphinx, etc are
always used as first preference.
(2) In a vendored packages directory. Here I am suggesting
qemu.git/python/wheels/ as that directory. This is intended to serve as
a replacement for vendoring the meson source for QEMU tarballs. It is
also highly likely to be extremely useful for packaging the "qemu.qmp"
package in source distributions for platforms that do not yet package
qemu.qmp separately.
(3) Online, via PyPI, ***only when "--online" is passed***. This is only
ever used as a fallback if the first two sources do not have an
appropriate package that meets the requirement. The ability to build
QEMU and run tests *completely offline* is not impinged.
Signed-off-by: Paolo Bonzini
Signed-off-by: John Snow
---
python/scripts/mkvenv.py | 130 ++-
1 file changed, 129 insertions(+), 1 deletion(-)
diff --git a/python/scripts/mkvenv.py b/python/scripts/mkvenv.py
index 5ee9967421..c1982589c9 100644
--- a/python/scripts/mkvenv.py
+++ b/python/scripts/mkvenv.py
@@ -11,6 +11,7 @@
Commands:
command Description
createcreate a venv
+ensureEnsure that the specified package is installed.
--
@@ -22,6 +23,18 @@
options:
-h, --help show this help message and exit
+--
+
+usage: mkvenv ensure [-h] [--online] [--dir DIR] dep_spec...
+
+positional arguments:
+ dep_specPEP 508 Dependency specification, e.g. 'meson>=0.61.5'
+
+options:
+ -h, --help show this help message and exit
+ --onlineInstall packages from PyPI, if necessary.
+ --dir DIR Path to vendored packages where we may install from.
+
"""
# Copyright (C) 2022-2023 Red Hat, Inc.
@@ -43,7 +56,12 @@
import sys
import sysconfig
from types import SimpleNamespace
-from typing import Any, Optional, Union
+from typing import (
+Any,
+Optional,
+Sequence,
+Union,
+)
import venv
@@ -333,6 +351,85 @@ def _stringify(data: Union[str, bytes]) -> str:
print(builder.get_value("env_exe"))
+def pip_install(
+args: Sequence[str],
+online: bool = False,
+wheels_dir: Optional[Union[str, Path]] = None,
+devnull: bool = False,
+) -> None:
+"""
+Use pip to install a package or package(s) as specified in @args.
+"""
+loud = bool(
+os.environ.get("DEBUG")
+or os.environ.get("GITLAB_CI")
+or os.environ.get("V")
+)
+
+full_args = [
+sys.executable,
+"-m",
+"pip",
+"install",
+"--disable-pip-version-check",
+"-v" if loud else "-q",
+]
+if not online:
+full_args += ["--no-index"]
+if wheels_dir:
+full_args += ["--find-links", f"file://{str(wheels_dir)}"]
+full_args += list(args)
+subprocess.run(
+full_args,
+check=True,
+stdout=subprocess.DEVNULL if devnull else None,
+stderr=subprocess.DEVNULL if devnull else None,
+)
+
+
+def ensure(
+dep_specs: Sequence[str],
+online: bool = False,
+wheels_dir: Optional[Union[str, Path]] = None,
+) -> None:
+"""
+Use pip to ensure we have the package specified by @dep_specs.
+
+If the package is already installed, do nothing. If online and
+wheels_dir are both provided, prefer packages found in wheels_dir
+first before connecting to PyPI.
+
+:param dep_specs:
+PEP 508 dependency specifications. e.g. ['meson>=0.61.5'].
+:param online: If True, fall back to PyPI.
+:param wheels_dir: If specified, search this path for packages.
+"""
+# This first install command will:
+# (A) Do nothing, if we already have a suitable package.
+# (B) Install the package from vendored source, if possible.
+# (C) Fail if neither A nor B.
+try:
+pip_install(
+dep_specs,
+online=False,
+wheels_dir=wheels_dir,
+devnull=online and not wheels_dir,
+)
+# (A) or (B) happened. Success.
+return
+except subprocess.CalledProcessError:
+# (C) Happened.
+# The package is missing or isn't a suitable version,
+# and we weren't able to install a suitable vendored package.
+if online:
+print(
+f"mkvenv: installing {', '.join(dep_specs)}", file=sys.stderr
+)
+pip_install(dep_specs, online=True)
+else:
+raise
+
+
def _add_create_subcommand(subparsers:
[PATCH 00/27] configure: create a python venv and ensure meson, sphinx
This patch series creates a mandatory python virtual environment
("venv") during configure time and uses it to ensure the availability of
meson and sphinx.
See https://www.qemu.org/2023/03/24/python/ for motivations. The summary
is that the goal of this series is to ensure that the `python` used to
run meson is the same `python` used to run Sphinx, tests, and any
build-time python scripting we have. As it stands, meson and sphinx (and
their extensions) *may* run in a different python environment than the
one configured and chosen by the user at configure/build time.
The effective change of this series is that QEMU will now
unconditionally create a venv at configure-time and will ensure that
meson (and sphinx, if docs are enabled) are available through that venv.
Some important points as a pre-emptive "FAQ":
- This venv is unconditionally created and lives at {build_dir}/pyvenv.
- The python interpreter used by this venv is always the one identified
by configure. (Which in turn is always the one specified by --python
or $PYTHON)
- *almost* all python scripts in qemu.git executed as part of the build
system, meson, sphinx, avocado tests, vm tests or CI are always
executed within this venv.
(iotests are not yet integrated; I plan to tackle this separately as a
follow-up in order to have a more tightly focused scope on that
series.)
- It remains possible to build and test fully offline.
(In most cases, you just need meson and sphinx from your distro's repo.)
- Distribution packaged 'meson' and 'sphinx' are still utilized whenever
possible as the highest preference.
- Vendored versions of e.g. 'meson' are always preferred to PyPI
versions for speed, repeatability and ensuring tarball builds work
as-is offline.
(Sphinx will not be vendored, just like it already isn't.)
- Missing dependencies, when possible, are fetched and installed
on-demand automatically to make developer environments "just work".
- Works for Python 3.6 and up, on Fedora, OpenSuSE, Red Hat, CentOS,
Alpine, Debian, Ubuntu, NetBSD, OpenBSD, and hopefully everywhere
- No new dependencies (...for most platforms. Debian and NetBSD get an
asterisk.)
- The meson git submodule is unused after this series and can be removed.
... I could write more, and I'm sure there's a few things that need
buffing out or changing here and there, but I think it's largely good.
So, enjoy!
~js
John Snow (22):
python: update pylint configuration
python: add mkvenv.py
mkvenv: add better error message for missing pyexpat module
mkvenv: add nested venv workaround
mkvenv: add ensure subcommand
mkvenv: add diagnose() method for ensure() failures
mkvenv: add console script entry point generation
mkvenv: work around broken pip installations on Debian 10
tests/docker: add python3-venv dependency
tests/vm: Configure netbsd to use Python 3.10
tests/vm: add py310-expat to NetBSD
python: add vendor.py utility
configure: create a python venv unconditionally
python/wheels: add vendored meson package
configure: use 'mkvenv ensure meson' to bootstrap meson
qemu.git: drop meson git submodule
tests: Use configure-provided pyvenv for tests
configure: move --enable-docs and --disable-docs back to configure
configure: bootstrap sphinx with mkvenv
configure: add --enable-pypi and --disable-pypi
configure: Add courtesy hint to Python version failure message
mkvenv.py: experiment; use distlib to generate script entry points
Paolo Bonzini (5):
python: shut up "pip install" during "make check-minreqs"
mkvenv: create pip binary in virtual environment
Python: Drop support for Python 3.6
mkvenv: mark command as required
python: bump some of the dependencies
docs/about/build-platforms.rst| 2 +-
docs/conf.py | 9 -
docs/devel/acpi-bits.rst | 6 +-
docs/devel/testing.rst| 14 +-
docs/meson.build | 2 +-
configure | 155 ++--
.gitlab-ci.d/buildtest-template.yml | 4 +-
.gitlab-ci.d/buildtest.yml| 6 +-
.gitmodules | 3 -
meson | 1 -
meson_options.txt | 2 -
python/Makefile | 19 +-
python/scripts/mkvenv.py | 858 ++
python/scripts/vendor.py | 74 ++
python/setup.cfg | 27 +-
python/tests/flake8.sh| 1 +
python/tests/isort.sh | 1 +
python/tests/minreqs.txt | 16 +-
python/tests/mypy.sh | 1 +
python/tests/pylint.sh| 1 +
python/wheels/meson-0.61.5-py3-none-any.whl | Bin 0 -> 862509 bytes
.../org.centos/stream/8/x86_64/test-avocado | 4 +-
[PATCH 08/27] mkvenv: add console script entry point generation
When creating a virtual environment that inherits system packages,
script entry points (like "meson", "sphinx-build", etc) are not
re-generated with the correct shebang. When you are *inside* of the
venv, this is not a problem, but if you are *outside* of it, you will
not have a script that engages the virtual environment appropriately.
Add a mechanism that generates new entry points for pre-existing
packages so that we can use these scripts to run "meson",
"sphinx-build", "pip", unambiguously inside the venv.
NOTE: the "FIXME" command regarding Windows launcher binaries can be
solved by using distlib. distlib is usually not installed on Linux
distribution, but it is a dependency of pip (and therefore should be
much more commonly available) on msys, where it is most useful.
Signed-off-by: Paolo Bonzini
Signed-off-by: John Snow
---
python/scripts/mkvenv.py | 174 ++-
python/setup.cfg | 1 +
2 files changed, 172 insertions(+), 3 deletions(-)
diff --git a/python/scripts/mkvenv.py b/python/scripts/mkvenv.py
index d9ba2e1532..9c99122603 100644
--- a/python/scripts/mkvenv.py
+++ b/python/scripts/mkvenv.py
@@ -54,12 +54,15 @@
import re
import shutil
import site
+import stat
import subprocess
import sys
import sysconfig
from types import SimpleNamespace
from typing import (
Any,
+Dict,
+Iterator,
Optional,
Sequence,
Union,
@@ -353,6 +356,168 @@ def _stringify(data: Union[str, bytes]) -> str:
print(builder.get_value("env_exe"))
+def _gen_importlib(packages: Sequence[str]) -> Iterator[Dict[str, str]]:
+# pylint: disable=import-outside-toplevel
+# pylint: disable=no-name-in-module
+# pylint: disable=import-error
+try:
+# First preference: Python 3.8+ stdlib
+from importlib.metadata import ( # type: ignore
+PackageNotFoundError,
+distribution,
+)
+except ImportError as exc:
+logger.debug("%s", str(exc))
+# Second preference: Commonly available PyPI backport
+from importlib_metadata import ( # type: ignore
+PackageNotFoundError,
+distribution,
+)
+
+# Borrowed from CPython (Lib/importlib/metadata/__init__.py)
+pattern = re.compile(
+r"(?P[\w.]+)\s*"
+r"(:\s*(?P[\w.]+)\s*)?"
+r"((?P\[.*\])\s*)?$"
+)
+
+def _generator() -> Iterator[Dict[str, str]]:
+for package in packages:
+try:
+entry_points = distribution(package).entry_points
+except PackageNotFoundError:
+continue
+
+# The EntryPoints type is only available in 3.10+,
+# treat this as a vanilla list and filter it ourselves.
+entry_points = filter(
+lambda ep: ep.group == "console_scripts", entry_points
+)
+
+for entry_point in entry_points:
+# Python 3.8 doesn't have 'module' or 'attr' attributes
+if not (
+hasattr(entry_point, "module")
+and hasattr(entry_point, "attr")
+):
+match = pattern.match(entry_point.value)
+assert match is not None
+module = match.group("module")
+attr = match.group("attr")
+else:
+module = entry_point.module
+attr = entry_point.attr
+yield {
+"name": entry_point.name,
+"module": module,
+"import_name": attr,
+"func": attr,
+}
+
+return _generator()
+
+
+def _gen_pkg_resources(packages: Sequence[str]) -> Iterator[Dict[str, str]]:
+# pylint: disable=import-outside-toplevel
+# Bundled with setuptools; has a good chance of being available.
+import pkg_resources
+
+def _generator() -> Iterator[Dict[str, str]]:
+for package in packages:
+try:
+eps = pkg_resources.get_entry_map(package, "console_scripts")
+except pkg_resources.DistributionNotFound:
+continue
+
+for entry_point in eps.values():
+yield {
+"name": entry_point.name,
+"module": entry_point.module_name,
+"import_name": ".".join(entry_point.attrs),
+"func": ".".join(entry_point.attrs),
+}
+
+return _generator()
+
+
+# Borrowed/adapted from pip's vendored version of distlib:
+SCRIPT_TEMPLATE = r"""#!{python_path:s}
+# -*- coding: utf-8 -*-
+import re
+import sys
+from {module:s} import {import_name:s}
+if __name__ == '__main__':
+sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
+sys.exit({func:s}())
+"""
+
+
+def generate_console_scripts(
+packages: Sequence[str],
+python_path: Optional[str] = None,
+bin_path:
[PATCH 04/27] mkvenv: add better error message for missing pyexpat module
NetBSD debundles pyexpat from python, but ensurepip needs pyexpat. Try
our best to offer a helpful error message instead of just failing
catastrophically.
Signed-off-by: John Snow
---
python/scripts/mkvenv.py | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/python/scripts/mkvenv.py b/python/scripts/mkvenv.py
index ce52b55976..dbcd488c12 100644
--- a/python/scripts/mkvenv.py
+++ b/python/scripts/mkvenv.py
@@ -118,7 +118,10 @@ def check_ensurepip(with_pip: bool) -> None:
Raise a fatal exception with a helpful hint if it isn't available.
"""
-if with_pip and not find_spec("ensurepip"):
+if not with_pip:
+return
+
+if not find_spec("ensurepip"):
msg = (
"Python's ensurepip module is not found.\n"
"It's normally part of the Python standard library, "
@@ -130,6 +133,19 @@ def check_ensurepip(with_pip: bool) -> None:
)
raise Ouch(msg)
+# ensurepip uses pyexpat, which can also go missing on us:
+if not find_spec("pyexpat"):
+msg = (
+"Python's pyexpat module is not found.\n"
+"It's normally part of the Python standard library, "
+"maybe your distribution packages it separately?\n"
+"Either install pyexpat, or alleviate the need for it in the "
+"first place by installing pip and setuptools for "
+f"'{sys.executable}'.\n\n"
+"(Hint: NetBSD's pkgsrc debundles this to e.g. 'py310-expat'.)"
+)
+raise Ouch(msg)
+
def make_venv( # pylint: disable=too-many-arguments
env_dir: Union[str, Path],
--
2.40.0
[PATCH 03/27] python: add mkvenv.py
This script will be responsible for building a lightweight Python
virtual environment at configure time. It works with Python 3.6 or
newer.
It has been designed to:
- work *offline*, no PyPI required.
- work *quickly*, The fast path is only ~65ms on my machine.
- work *robustly*, with multiple fallbacks to keep things working.
- work *cooperatively*, using system packages where possible.
(You can use your distro's meson, no problem.)
Due to its unique position in the build chain, it exists outside of the
installable python packages in-tree and *must* be runnable without any
third party dependencies.
Under normal circumstances, the only dependency required to execute this
script is Python 3.6+ itself. The script is *faster* by several seconds
when setuptools and pip are installed in the host environment, which is
probably the case for a typical multi-purpose developer workstation.
In the event that pip/setuptools are missing or not usable, additional
dependencies may be required on some distributions which remove certain
Python stdlib modules to package them separately:
- Debian may require python3-venv to provide "ensurepip"
- NetBSD may require py310-expat to provide "pyexpat" *
(* Or whichever version is current for NetBSD.)
Signed-off-by: Paolo Bonzini
Signed-off-by: John Snow
---
python/scripts/mkvenv.py | 289 +++
python/setup.cfg | 9 ++
python/tests/flake8.sh | 1 +
python/tests/isort.sh| 1 +
python/tests/mypy.sh | 1 +
python/tests/pylint.sh | 1 +
6 files changed, 302 insertions(+)
create mode 100644 python/scripts/mkvenv.py
diff --git a/python/scripts/mkvenv.py b/python/scripts/mkvenv.py
new file mode 100644
index 00..ce52b55976
--- /dev/null
+++ b/python/scripts/mkvenv.py
@@ -0,0 +1,289 @@
+"""
+mkvenv - QEMU pyvenv bootstrapping utility
+
+usage: mkvenv [-h] command ...
+
+QEMU pyvenv bootstrapping utility
+
+options:
+ -h, --help show this help message and exit
+
+Commands:
+ command Description
+createcreate a venv
+
+--
+
+usage: mkvenv create [-h] target
+
+positional arguments:
+ target Target directory to install virtual environment into.
+
+options:
+ -h, --help show this help message and exit
+
+"""
+
+# Copyright (C) 2022-2023 Red Hat, Inc.
+#
+# Authors:
+# John Snow
+# Paolo Bonzini
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later. See the COPYING file in the top-level directory.
+
+import argparse
+from importlib.util import find_spec
+import logging
+import os
+from pathlib import Path
+import subprocess
+import sys
+from types import SimpleNamespace
+from typing import Any, Optional, Union
+import venv
+
+
+# Do not add any mandatory dependencies from outside the stdlib:
+# This script *must* be usable standalone!
+
+DirType = Union[str, bytes, "os.PathLike[str]", "os.PathLike[bytes]"]
+logger = logging.getLogger("mkvenv")
+
+
+class Ouch(RuntimeError):
+"""An Exception class we can't confuse with a builtin."""
+
+
+class QemuEnvBuilder(venv.EnvBuilder):
+"""
+An extension of venv.EnvBuilder for building QEMU's configure-time venv.
+
+As of this commit, it does not yet do anything particularly
+different than the standard venv-creation utility. The next several
+commits will gradually change that in small commits that highlight
+each feature individually.
+
+Parameters for base class init:
+ - system_site_packages: bool = False
+ - clear: bool = False
+ - symlinks: bool = False
+ - upgrade: bool = False
+ - with_pip: bool = False
+ - prompt: Optional[str] = None
+ - upgrade_deps: bool = False (Since 3.9)
+"""
+
+def __init__(self, *args: Any, **kwargs: Any) -> None:
+logger.debug("QemuEnvBuilder.__init__(...)")
+super().__init__(*args, **kwargs)
+
+# Make the context available post-creation:
+self._context: Optional[SimpleNamespace] = None
+
+def ensure_directories(self, env_dir: DirType) -> SimpleNamespace:
+logger.debug("ensure_directories(env_dir=%s)", env_dir)
+self._context = super().ensure_directories(env_dir)
+return self._context
+
+def get_value(self, field: str) -> str:
+"""
+Get a string value from the context namespace after a call to build.
+
+For valid field names, see:
+
https://docs.python.org/3/library/venv.html#venv.EnvBuilder.ensure_directories
+"""
+ret = getattr(self._context, field)
+assert isinstance(ret, str)
+return ret
+
+
+def need_ensurepip() -> bool:
+"""
+Tests for the presence of setuptools and pip.
+
+:return: `True` if we do not detect both packages.
+"""
+# Don't try to actually import them, it's fraught with danger:
+# https://github.com/pypa/setuptools/issues/2993
+if find_spec("setuptools") and
Re: css_clear_io_interrupt() error handling
On Wed, 10 May 2023 08:32:12 +0200
Markus Armbruster wrote:
> Halil Pasic writes:
>
> > On Mon, 08 May 2023 11:01:55 +0200
> > Cornelia Huck wrote:
> >
> >> On Mon, May 08 2023, Markus Armbruster wrote:
[..]
> > and we do check for availability and cover that via -ENOSYS.
>
> Yes, kvm_s390_flic_realize() checks and sets ->clear_io_supported
> accordingly, and kvm_s390_clear_io_flic() returns -ENOSYS when it's
> false.
>
> Doc on the actual set:
Right. Sorry for the misinformation.
>
> 4.80 KVM_SET_DEVICE_ATTR/KVM_GET_DEVICE_ATTR
>
>
> :Capability: KVM_CAP_DEVICE_CTRL, KVM_CAP_VM_ATTRIBUTES for vm device,
>KVM_CAP_VCPU_ATTRIBUTES for vcpu device
>KVM_CAP_SYS_ATTRIBUTES for system (/dev/kvm) device (no set)
> :Type: device ioctl, vm ioctl, vcpu ioctl
> :Parameters: struct kvm_device_attr
> :Returns: 0 on success, -1 on error
>
> Errors:
>
> = =
> ENXIO The group or attribute is unknown/unsupported for this device
> or hardware support is missing.
> EPERM The attribute cannot (currently) be accessed this way
> (e.g. read-only attribute, or attribute that only makes
> sense when the device is in a different state)
> = =
>
> Other error conditions may be defined by individual device types.
>
> Gets/sets a specified piece of device configuration and/or state. The
> semantics are device-specific. See individual device documentation in
> the "devices" directory. As with ONE_REG, the size of the data
> transferred is defined by the particular attribute.
>
> ::
>
> struct kvm_device_attr {
> __u32 flags; /* no flags currently defined */
> __u32 group; /* device-defined */
> __u64 attr; /* group-defined */
> __u64 addr; /* userspace address of attr data */
> };
>
>
> kvm_s390_flic_realize() sets ->fd is to refer to the KVM_DEV_TYPE_FLIC
> it creates. I guess that means ENXIO and EPERM should never happen.
I agree.
>
> > For KVM_DEV_FLIC_CLEAR_IO_IRQ is just the following error code
> > documented in linux/Documentation/virt/kvm/devices/s390_flic.rst
> > which is to my knowledge the most authoritative source.
> > """
> > .. note:: The KVM_DEV_FLIC_CLEAR_IO_IRQ ioctl will return EINVAL in case a
> > zero schid is specified
> > """
> > but a look in the code will tell us that -EFAULT is also possible if the
> > supplied address is broken.
>
> Common behavior.
>
Makes sense, just that I did not find it in the interface
description/documentation.
> > To sum it up, there is nothing to go wrong with the given operation, and
> > to my best knowledge seeing an error code on the ioctl would either
> > indicate a programming error on the client side (QEMU messed it up) or
> > there is something wrong with the kernel.
>
> Abort on "QEMU messed up" is proper. Abort on "something wrong with the
> kernel" less so. More on that below.
>
I think I understand where are you coming from. IMHO it boils down
to how broken the kernel is.
> >> > Is the error condition fatal, i.e. continuing would be unsafe?
> >
> > If the kernel is broken, probably. It is certainly unexpected.
> >
> >> >
> >> > If it's a fatal programming error, then abort() is appropriate.
> >> >
> >> > If it's fatal, but not a programming error, we should exit(1) instead.
> >
> > It might not be a QEMU programming error. I really see no reason why
> > would a combination of a sane QEMU and a sane kernel give us another
> > error code than -ENOSYS.
> >
> >> >
> >> > If it's a survivable programming error, use of abort() is a matter of
> >> > taste.
> >
> > The fact that we might have failed to clear up some interrupts which we
> > are obligated to clean up by the s390 architecture is not expected to
> > have grave consequences.
>
> Good to know.
>
> >> From what I remember, this was introduced to clean up a potentially
> >> queued interrupt that is not supposed to be delivered, so the worst
> >> thing that could happen on failure is a spurious interrupt (same as what
> >> could happen if the kernel flic doesn't provide this function in the
> >> first place.) My main worry would be changes/breakages on the kernel
> >> side (while the QEMU side remains unchanged).
> >
> > Agreed. And I hope anybody changing the kernel would test the new error
> > code and notice the QEMU crashes. This was my intention in the first
> > place.
> >
> >> So, I think we should continue to log the error in any case; but I don't
> >> have a strong opinion as to whether we should use exit(1) (as I wouldn't
> >> consider it a programming error) or just continue. Halil, your choice :)
> >
> > Neither do I have a strong opinion.
Re: [PATCH 1/1] e1000e: Fix tx/rx counters
On Thu, May 11, 2023 at 2:54 AM Michael Tokarev wrote: > > 10.04.2023 18:27, timothee.coca...@gmail.com wrote: > > The bytes and packets counter registers are cleared on read. > > > > Copying the "total counter" registers to the "good counter" registers has > > side effects. > > If the "total" register is never read by the OS, it only gets incremented. > > This leads to exponential growth of the "good" register. > > > > This commit increments the counters individually to avoid this. > > Smells like a -stable material, is it not? Yes, tagged for stable. Thanks > > Thanks, > > /mjt >
[PATCH 1/2] linux-user/s390x: Fix single-stepping SVC
Currently single-stepping SVC executes two instructions. The reason is
that EXCP_DEBUG for the SVC instruction itself is masked by EXCP_SVC.
Fix by re-raising EXCP_DEBUG.
Signed-off-by: Ilya Leoshkevich
---
linux-user/s390x/cpu_loop.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/linux-user/s390x/cpu_loop.c b/linux-user/s390x/cpu_loop.c
index 285bc60071a..8b7ac2879ef 100644
--- a/linux-user/s390x/cpu_loop.c
+++ b/linux-user/s390x/cpu_loop.c
@@ -86,6 +86,15 @@ void cpu_loop(CPUS390XState *env)
} else if (ret != -QEMU_ESIGRETURN) {
env->regs[2] = ret;
}
+
+if (unlikely(cs->singlestep_enabled)) {
+/*
+ * cpu_tb_exec() did not raise EXCP_DEBUG, because it has seen
+ * that EXCP_SVC was already pending.
+ */
+cs->exception_index = EXCP_DEBUG;
+}
+
break;
case EXCP_DEBUG:
--
2.40.1
[PATCH 0/2] linux-user/s390x: Fix single-stepping SVC
Hi, I noticed that single-stepping SVC runs two instructions instead of one. The reason is that EXCP_SVC masks EXCP_DEBUG. Patch 1 fixes this problem, patch 2 adds a test. Btw, there is at least one more problem in that area, namely single-stepping instructions that cause e.g. SIGILL. Using the existing signals-s390x test as an example: (gdb) x/i $pc => 0x1001740 : .long 0x07fe (gdb) si Program received signal SIGILL, Illegal instruction. (gdb) x/i $pc => 0x1001742 :br %r14 # So far so good. (gdb) si (gdb) x/i $pc => 0x10017b6 : lay %r15,-344(%r15) # Missed the first signal handler instruction! I'm not sure what to do about it - the trivial fix to add gdb_handlesig(cpu, 0) to the end of handle_pending_signal() caused GDB to hang, and I haven't looked further yet. Best regards, Ilya Ilya Leoshkevich (2): linux-user/s390x: Fix single-stepping SVC tests/tcg/s390x: Test single-stepping SVC linux-user/s390x/cpu_loop.c | 9 tests/tcg/s390x/Makefile.target | 11 - tests/tcg/s390x/gdbstub/test-svc.py | 64 + tests/tcg/s390x/hello-s390x-asm.S | 20 + 4 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 tests/tcg/s390x/gdbstub/test-svc.py create mode 100644 tests/tcg/s390x/hello-s390x-asm.S -- 2.40.1
[PATCH 2/2] tests/tcg/s390x: Test single-stepping SVC
Add a small test to prevent regressions.
Signed-off-by: Ilya Leoshkevich
---
tests/tcg/s390x/Makefile.target | 11 -
tests/tcg/s390x/gdbstub/test-svc.py | 64 +
tests/tcg/s390x/hello-s390x-asm.S | 20 +
3 files changed, 94 insertions(+), 1 deletion(-)
create mode 100644 tests/tcg/s390x/gdbstub/test-svc.py
create mode 100644 tests/tcg/s390x/hello-s390x-asm.S
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 0031868b136..bd435e4dd63 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -74,7 +74,16 @@ run-gdbstub-signals-s390x: signals-s390x
--bin $< --test $(S390X_SRC)/gdbstub/test-signals-s390x.py, \
mixing signals and debugging)
-EXTRA_RUNS += run-gdbstub-signals-s390x
+hello-s390x-asm: CFLAGS+=-nostdlib
+
+run-gdbstub-svc: hello-s390x-asm
+ $(call run-test, $@, $(GDB_SCRIPT) \
+ --gdb $(HAVE_GDB_BIN) \
+ --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
+ --bin $< --test $(S390X_SRC)/gdbstub/test-svc.py, \
+ single-stepping svc)
+
+EXTRA_RUNS += run-gdbstub-signals-s390x run-gdbstub-svc
endif
# MVX versions of sha512
diff --git a/tests/tcg/s390x/gdbstub/test-svc.py
b/tests/tcg/s390x/gdbstub/test-svc.py
new file mode 100644
index 000..7851ca72846
--- /dev/null
+++ b/tests/tcg/s390x/gdbstub/test-svc.py
@@ -0,0 +1,64 @@
+"""Test single-stepping SVC.
+
+This runs as a sourced script (via -x, via run-test.py)."""
+from __future__ import print_function
+import gdb
+import sys
+
+
+n_failures = 0
+
+
+def report(cond, msg):
+"""Report success/fail of a test"""
+if cond:
+print("PASS: {}".format(msg))
+else:
+print("FAIL: {}".format(msg))
+global n_failures
+n_failures += 1
+
+
+def run_test():
+"""Run through the tests one by one"""
+report("lghi\t" in gdb.execute("x/i $pc", False, True), "insn #1")
+gdb.execute("si")
+report("larl\t" in gdb.execute("x/i $pc", False, True), "insn #2")
+gdb.execute("si")
+report("lghi\t" in gdb.execute("x/i $pc", False, True), "insn #3")
+gdb.execute("si")
+report("svc\t" in gdb.execute("x/i $pc", False, True), "insn #4")
+gdb.execute("si")
+report("xgr\t" in gdb.execute("x/i $pc", False, True), "insn #5")
+gdb.execute("si")
+report("svc\t" in gdb.execute("x/i $pc", False, True), "insn #6")
+gdb.execute("si")
+
+
+def main():
+"""Prepare the environment and run through the tests"""
+try:
+inferior = gdb.selected_inferior()
+print("ATTACHED: {}".format(inferior.architecture().name()))
+except (gdb.error, AttributeError):
+print("SKIPPING (not connected)")
+exit(0)
+
+if gdb.parse_and_eval('$pc') == 0:
+print("SKIP: PC not set")
+exit(0)
+
+try:
+# These are not very useful in scripts
+gdb.execute("set pagination off")
+gdb.execute("set confirm off")
+
+# Run the actual tests
+run_test()
+except gdb.error:
+report(False, "GDB Exception: {}".format(sys.exc_info()[0]))
+print("All tests complete: %d failures" % n_failures)
+exit(n_failures)
+
+
+main()
diff --git a/tests/tcg/s390x/hello-s390x-asm.S
b/tests/tcg/s390x/hello-s390x-asm.S
new file mode 100644
index 000..2e9faa16047
--- /dev/null
+++ b/tests/tcg/s390x/hello-s390x-asm.S
@@ -0,0 +1,20 @@
+/*
+ * Hello, World! in assembly.
+ */
+
+.globl _start
+_start:
+
+/* puts("Hello, World!"); */
+lghi %r2,1
+larl %r3,foo
+lghi %r4,foo_end-foo
+svc 4
+
+/* exit(0); */
+xgr %r2,%r2
+svc 1
+
+.align 2
+foo: .asciz "Hello, World!\n"
+foo_end:
--
2.40.1
Re: [PATCH v4 0/9] PC cleanups
Am 10. Mai 2023 19:20:59 UTC schrieb "Michael S. Tsirkin" :
>On Wed, May 10, 2023 at 06:26:54PM +, Bernhard Beschow wrote:
>>
>>
>> Am 5. März 2023 07:45:55 UTC schrieb Bernhard Beschow :
>> >
>> >
>> >Am 13. Februar 2023 16:45:05 UTC schrieb Bernhard Beschow
>> >:
>> >>
>> >>
>> >>Am 13. Februar 2023 16:19:55 UTC schrieb Bernhard Beschow
>> >>:
>> >>>This series contains some cleanups I came across when working on the PC
>> >>>
>> >>>machines. It consists of reducing the usage of global variables and
>> >>>eliminating
>> >>>
>> >>>some redundancies.
>> >>>
>> >>>
>> >>>
>> >>>One notable change is that the SMRAM memory region gets moved from the
>> >>>i440fx
>> >>>
>> >>>and q35 host bridges into the x86 machine. This will simplify cleaning up
>> >>>these
>> >>>
>> >>>host bridges which will be done in a separate series. Note that the
>> >>>movement of
>> >>>
>> >>>the SMRAM memory region apparently doesn't change migration ABI for the
>> >>>pc and
>> >>>
>> >>>q35 machines (see below).
>> >>>
>> >>>
>> >>>
>> >>>Testing done:
>> >>>
>> >>>* `make check`
>> >>>
>> >>>' `make check-avocado`
>> >>>
>> >>>* `qemu-system-x86_64 -M q35 -m 2G -cdrom \
>> >>>
>> >>> manjaro-kde-21.3.2-220704-linux515.iso`
>> >>>
>> >>>* `qemu-system-x86_64 -M pc -m 2G -cdrom
>> >>>manjaro-kde-21.3.2-220704-linux515.iso`
>> >>>
>> >>>* Confirm that JSON representation of migration files (pc & q35) are
>> >>>empty:
>> >>>
>> >>> 1. Create four migration files {pc,q35}-{before,after}.mig by running
>> >>>
>> >>> `qemu-system-x86_64 -M {pc,q35} -S` with QEMU built from master and
>> >>> from
>> >>>
>> >>> this series.
>> >>>
>> >>> 2. Run `./scripts/analyze-migration.py -d desc -f *.mig > *.json` on
>> >>> the four
>> >>>
>> >>> files
>> >>>
>> >>> 3. Compare the diffs -> both are empty
>> >>>
>> >>>
>> >>>
>> >>>v4:
>> >>>
>> >>>* Remove ram_memory variable in pc_q35 completely (Zoltan)
>> >>>
>> >>
>> >>The last two patches still need review. Comments welcome!
>> >
>> >Ping
>> >
>> >Can we queue the reviewed patches (all but the last two) already?
>>
>> Ping 2
>
>
>queued 1-7
Thanks -- also for the RTC ones!
Best regards,
Bernhard
>
>> >
>> >Thanks,
>> >Bernhard
>> >
>> >>
>> >>>
>> >>>
>> >>>v3:
>> >>>
>> >>>* Add three patches regarding init_pam() and SMRAM.
>> >>>
>> >>>* Drop 'hw/i386/pc_q35: Resolve redundant q35_host variable' since Phil
>> >>>posted
>> >>>
>> >>> a similar patch in a more comprehensive series:
>> >>>
>> >>>
>> >>> https://lore.kernel.org/qemu-devel/20230203180914.49112-13-phi...@linaro.org/
>> >>>
>> >>>* Drop 'hw/isa/lpc_ich9: Reuse memory and io address space of PCI bus'
>> >>>since
>> >>>
>> >>> it inadvertantly changed the memory hierarchy.
>> >>>
>> >>>* Drop ICH9 cleanups again in favor of a separate series.
>> >>>
>> >>>
>> >>>
>> >>>v2:
>> >>>
>> >>>* Factor out 'hw/i386/pc_q35: Reuse machine parameter' from
>> >>>'hw/i386/pc_q35:
>> >>>
>> >>> Resolve redundant q35_host variable' (Zoltan)
>> >>>
>> >>>* Lower type of phb to Object in 'hw/i386/pc_q35: Resolve redundant
>> >>>q35_host
>> >>>
>> >>> variable' (Zoltan)
>> >>>
>> >>>* Add ICH9 cleanups
>> >>>
>> >>>
>> >>>
>> >>>Bernhard Beschow (9):
>> >>>
>> >>> hw/pci-host/i440fx: Inline sysbus_add_io()
>> >>>
>> >>> hw/pci-host/q35: Inline sysbus_add_io()
>> >>>
>> >>> hw/i386/pc_q35: Reuse machine parameter
>> >>>
>> >>> hw/i386/pc_{q35,piix}: Reuse MachineClass::desc as SMB product name
>> >>>
>> >>> hw/i386/pc_{q35,piix}: Minimize usage of get_system_memory()
>> >>>
>> >>> hw/i386/pc: Initialize ram_memory variable directly
>> >>>
>> >>> hw/pci-host/pam: Make init_pam() usage more readable
>> >>>
>> >>> hw/i386/x86: Make TYPE_X86_MACHINE the owner of smram
>> >>>
>> >>> target/i386/tcg/sysemu/tcg-cpu: Avoid own opinion about smram size
>> >>>
>> >>>
>> >>>
>> >>> include/hw/i386/pc.h | 1 -
>> >>>
>> >>> include/hw/i386/x86.h| 2 ++
>> >>>
>> >>> include/hw/pci-host/i440fx.h | 7 ---
>> >>>
>> >>> include/hw/pci-host/pam.h| 5 +++--
>> >>>
>> >>> include/hw/pci-host/q35.h| 4 +++-
>> >>>
>> >>> hw/i386/pc.c | 2 --
>> >>>
>> >>> hw/i386/pc_piix.c| 10 +-
>> >>>
>> >>> hw/i386/pc_q35.c | 17 +
>> >>>
>> >>> hw/i386/x86.c| 4
>> >>>
>> >>> hw/pci-host/i440fx.c | 28 +---
>> >>>
>> >>> hw/pci-host/pam.c| 12 ++--
>> >>>
>> >>> hw/pci-host/q35.c| 31 ---
>> >>>
>> >>> target/i386/tcg/sysemu/tcg-cpu.c | 3 +--
>> >>>
>> >>> 13 files changed, 66 insertions(+), 60 deletions(-)
>> >>>
>> >>>
>> >>>
>> >>>-- >
>> >>>2.39.1
>> >>>
>> >>>
>> >>>
>
RE: [PATCH v2 0/9] Hexagon (target/hexagon) New architecture support
> -Original Message- > From: Taylor Simpson > Sent: Saturday, April 29, 2023 2:57 PM > To: Richard Henderson ; qemu- > de...@nongnu.org > Cc: phi...@linaro.org; a...@rev.ng; a...@rev.ng; Brian Cain > ; Matheus Bernardino (QUIC) > > Subject: RE: [PATCH v2 0/9] Hexagon (target/hexagon) New architecture > support > > > > > -Original Message- > > From: Richard Henderson > > Sent: Friday, April 28, 2023 4:45 PM > > To: Taylor Simpson ; qemu-devel@nongnu.org > > Cc: phi...@linaro.org; a...@rev.ng; a...@rev.ng; Brian Cain > > ; Matheus Bernardino (QUIC) > > > > Subject: Re: [PATCH v2 0/9] Hexagon (target/hexagon) New architecture > > support > > > > On 4/27/23 23:40, Taylor Simpson wrote: > > > Add support for new Hexagon architecture versions v68/v69/v71/v73 > > > > Where can one find docs for this? > > The latest Hexagon SDK I can find is 3.5, which still ends at v67. > > I guess the folks at developer.qualcomm.com are behind in publishing specs. > I'll work on getting these. Hi Richard, The documents have been posted on this page in the Documentation section. https://developer.qualcomm.com/software/hexagon-dsp-sdk/tools I'll update the README with the link to the latest versions. Are you planning to review these given that Anton Johansson has already done a review? If not, I'll go ahead and do the pull request. Thanks, Taylor
[PATCH 4/8] qemu-img: Take graph lock more selectively
If we take a reader lock, we can't call any functions that take a writer
lock internally without causing deadlocks once the reader lock is
actually enforced in the main thread, too. Take the reader lock only
where it is actually needed.
Signed-off-by: Kevin Wolf
---
qemu-img.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index 9f9f0a7629..27f48051b0 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2938,8 +2938,6 @@ static BlockGraphInfoList *collect_image_info_list(bool
image_opts,
}
bs = blk_bs(blk);
-GRAPH_RDLOCK_GUARD_MAINLOOP();
-
/*
* Note that the returned BlockGraphInfo object will not have
* information about this image's backing node, because we have opened
@@ -2947,7 +2945,10 @@ static BlockGraphInfoList *collect_image_info_list(bool
image_opts,
* duplicate the backing chain information that we obtain by walking
* the chain manually here.
*/
+bdrv_graph_rdlock_main_loop();
bdrv_query_block_graph_info(bs, , );
+bdrv_graph_rdunlock_main_loop();
+
if (err) {
error_report_err(err);
blk_unref(blk);
--
2.40.1
[PATCH 8/8] graph-lock: Honour read locks even in the main thread
There are some conditions under which we don't actually need to do
anything for taking a reader lock: Writing the graph is only possible
from the main context while holding the BQL. So if a reader is running
in the main context under the BQL and knows that it won't be interrupted
until the next writer runs, we don't actually need to do anything.
This is the case if the reader code neither has a nested event loop
(this is forbidden anyway while you hold the lock) nor is a coroutine
(because a writer could run when the coroutine has yielded).
These conditions are exactly what bdrv_graph_rdlock_main_loop() asserts.
They are not fulfilled in bdrv_graph_co_rdlock(), which always runs in a
coroutine.
This deletes the shortcuts in bdrv_graph_co_rdlock() that skip taking
the reader lock in the main thread.
Reported-by: Fiona Ebner
Signed-off-by: Kevin Wolf
---
block/graph-lock.c | 10 --
1 file changed, 10 deletions(-)
diff --git a/block/graph-lock.c b/block/graph-lock.c
index 377884c3a9..9c42bd9799 100644
--- a/block/graph-lock.c
+++ b/block/graph-lock.c
@@ -162,11 +162,6 @@ void coroutine_fn bdrv_graph_co_rdlock(void)
BdrvGraphRWlock *bdrv_graph;
bdrv_graph = qemu_get_current_aio_context()->bdrv_graph;
-/* Do not lock if in main thread */
-if (qemu_in_main_thread()) {
-return;
-}
-
for (;;) {
qatomic_set(_graph->reader_count,
bdrv_graph->reader_count + 1);
@@ -230,11 +225,6 @@ void coroutine_fn bdrv_graph_co_rdunlock(void)
BdrvGraphRWlock *bdrv_graph;
bdrv_graph = qemu_get_current_aio_context()->bdrv_graph;
-/* Do not lock if in main thread */
-if (qemu_in_main_thread()) {
-return;
-}
-
qatomic_store_release(_graph->reader_count,
bdrv_graph->reader_count - 1);
/* make sure writer sees reader_count before we check has_writer */
--
2.40.1
[PATCH 5/8] test-bdrv-drain: Take graph lock more selectively
If we take a reader lock, we can't call any functions that take a writer lock internally without causing deadlocks once the reader lock is actually enforced in the main thread, too. Take the reader lock only where it is actually needed. Signed-off-by: Kevin Wolf --- tests/unit/test-bdrv-drain.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c index 9a4c5e59d6..ae4299ccfa 100644 --- a/tests/unit/test-bdrv-drain.c +++ b/tests/unit/test-bdrv-drain.c @@ -1004,8 +1004,6 @@ static void coroutine_fn test_co_delete_by_drain(void *opaque) void *buffer = g_malloc(65536); QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buffer, 65536); -GRAPH_RDLOCK_GUARD(); - /* Pretend some internal write operation from parent to child. * Important: We have to read from the child, not from the parent! * Draining works by first propagating it all up the tree to the @@ -1014,7 +1012,9 @@ static void coroutine_fn test_co_delete_by_drain(void *opaque) * everything will be drained before we go back down the tree, but * we do not want that. We want to be in the middle of draining * when this following requests returns. */ +bdrv_graph_co_rdlock(); bdrv_co_preadv(tts->wait_child, 0, 65536, , 0); +bdrv_graph_co_rdunlock(); g_assert_cmpint(bs->refcnt, ==, 1); -- 2.40.1
[PATCH 7/8] blockjob: Adhere to rate limit even when reentered early
When jobs are sleeping, for example to enforce a given rate limit, they
can be reentered early, in particular in order to get paused, to update
the rate limit or to get cancelled.
Before this patch, they behave in this case as if they had fully
completed their rate limiting delay. This means that requests are sped
up beyond their limit, violating the constraints that the user gave us.
Change the block jobs to sleep in a loop until the necessary delay is
completed, while still allowing cancelling them immediately as well
pausing (handled by the pause point in job_sleep_ns()) and updating the
rate limit.
This change is also motivated by iotests cases being prone to fail
because drain operations pause and unpause them so often that block jobs
complete earlier than they are supposed to. In particular, the next
commit would fail iotests 030 without this change.
Signed-off-by: Kevin Wolf
---
include/block/blockjob_int.h | 14 ++
block/commit.c | 7 ++-
block/mirror.c | 23 ++-
block/stream.c | 7 ++-
blockjob.c | 22 --
5 files changed, 44 insertions(+), 29 deletions(-)
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index f008446285..104824040c 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -126,12 +126,18 @@ void block_job_user_resume(Job *job);
*/
/**
- * block_job_ratelimit_get_delay:
+ * block_job_ratelimit_processed_bytes:
*
- * Calculate and return delay for the next request in ns. See the documentation
- * of ratelimit_calculate_delay() for details.
+ * To be called after some work has been done. Adjusts the delay for the next
+ * request. See the documentation of ratelimit_calculate_delay() for details.
*/
-int64_t block_job_ratelimit_get_delay(BlockJob *job, uint64_t n);
+void block_job_ratelimit_processed_bytes(BlockJob *job, uint64_t n);
+
+/**
+ * Put the job to sleep (assuming that it wasn't canceled) to throttle it to
the
+ * right speed according to its rate limiting.
+ */
+void block_job_ratelimit_sleep(BlockJob *job);
/**
* block_job_error_action:
diff --git a/block/commit.c b/block/commit.c
index 2b20fd0fd4..aa45beb0f0 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -116,7 +116,6 @@ static int coroutine_fn commit_run(Job *job, Error **errp)
{
CommitBlockJob *s = container_of(job, CommitBlockJob, common.job);
int64_t offset;
-uint64_t delay_ns = 0;
int ret = 0;
int64_t n = 0; /* bytes */
QEMU_AUTO_VFREE void *buf = NULL;
@@ -149,7 +148,7 @@ static int coroutine_fn commit_run(Job *job, Error **errp)
/* Note that even when no rate limit is applied we need to yield
* with no pending I/O here so that bdrv_drain_all() returns.
*/
-job_sleep_ns(>common.job, delay_ns);
+block_job_ratelimit_sleep(>common);
if (job_is_cancelled(>common.job)) {
break;
}
@@ -184,9 +183,7 @@ static int coroutine_fn commit_run(Job *job, Error **errp)
job_progress_update(>common.job, n);
if (copy) {
-delay_ns = block_job_ratelimit_get_delay(>common, n);
-} else {
-delay_ns = 0;
+block_job_ratelimit_processed_bytes(>common, n);
}
}
diff --git a/block/mirror.c b/block/mirror.c
index 717442ca4d..b7d92d1378 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -471,12 +471,11 @@ static unsigned mirror_perform(MirrorBlockJob *s, int64_t
offset,
return bytes_handled;
}
-static uint64_t coroutine_fn mirror_iteration(MirrorBlockJob *s)
+static void coroutine_fn mirror_iteration(MirrorBlockJob *s)
{
BlockDriverState *source = s->mirror_top_bs->backing->bs;
MirrorOp *pseudo_op;
int64_t offset;
-uint64_t delay_ns = 0, ret = 0;
/* At least the first dirty chunk is mirrored in one iteration. */
int nb_chunks = 1;
bool write_zeroes_ok = bdrv_can_write_zeroes_with_unmap(blk_bs(s->target));
@@ -608,16 +607,13 @@ static uint64_t coroutine_fn
mirror_iteration(MirrorBlockJob *s)
assert(io_bytes);
offset += io_bytes;
nb_chunks -= DIV_ROUND_UP(io_bytes, s->granularity);
-delay_ns = block_job_ratelimit_get_delay(>common, io_bytes_acct);
+block_job_ratelimit_processed_bytes(>common, io_bytes_acct);
}
-ret = delay_ns;
fail:
QTAILQ_REMOVE(>ops_in_flight, pseudo_op, next);
qemu_co_queue_restart_all(_op->waiting_requests);
g_free(pseudo_op);
-
-return ret;
}
static void mirror_free_init(MirrorBlockJob *s)
@@ -1011,7 +1007,6 @@ static int coroutine_fn mirror_run(Job *job, Error **errp)
assert(!s->dbi);
s->dbi = bdrv_dirty_iter_new(s->dirty_bitmap);
for (;;) {
-uint64_t delay_ns = 0;
int64_t cnt, delta;
bool should_complete;
@@ -1051,7 +1046,7 @@ static int coroutine_fn mirror_run(Job *job, Error **errp)
[PATCH 6/8] test-bdrv-drain: Call bdrv_co_unref() in coroutine context
bdrv_unref() is a no_coroutine_fn, so calling it from coroutine context
is invalid. Use bdrv_co_unref() instead.
Signed-off-by: Kevin Wolf
---
tests/unit/test-bdrv-drain.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c
index ae4299ccfa..08bb0f9984 100644
--- a/tests/unit/test-bdrv-drain.c
+++ b/tests/unit/test-bdrv-drain.c
@@ -1019,7 +1019,7 @@ static void coroutine_fn test_co_delete_by_drain(void
*opaque)
g_assert_cmpint(bs->refcnt, ==, 1);
if (!dbdd->detach_instead_of_delete) {
-blk_unref(blk);
+blk_co_unref(blk);
} else {
BdrvChild *c, *next_c;
QLIST_FOREACH_SAFE(c, >children, next, next_c) {
--
2.40.1
[PATCH 2/8] block/export: Fix null pointer dereference in error path
There are some error paths in blk_exp_add() that jump to 'fail:' before
'exp' is even created. So we can't just unconditionally access exp->blk.
Add a NULL check, and switch from exp->blk to blk, which is available
earlier, just to be extra sure that we really cover all cases where
BlockDevOps could have been set for it (in practice, this only happens
in drv->create() today, so this part of the change isn't strictly
necessary).
Fixes: de79b52604e43fdeba6cee4f5af600b62169f2d2
Signed-off-by: Kevin Wolf
---
block/export/export.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/block/export/export.c b/block/export/export.c
index 62c7c22d45..a5c8f42f53 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -192,8 +192,10 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error
**errp)
return exp;
fail:
-blk_set_dev_ops(exp->blk, NULL, NULL);
-blk_unref(blk);
+if (blk) {
+blk_set_dev_ops(blk, NULL, NULL);
+blk_unref(blk);
+}
aio_context_release(ctx);
if (exp) {
g_free(exp->id);
--
2.40.1
[PATCH 1/8] block: Call .bdrv_co_create(_opts) unlocked
These are functions that modify the graph, so they must be able to take
a writer lock. This is impossible if they already hold the reader lock.
If they need a reader lock for some of their operations, they should
take it internally.
Many of them go through blk_*(), which will always take the lock itself.
Direct calls of bdrv_*() need to take the reader lock. Note that while
locking for bdrv_co_*() calls is checked by TSA, this is not the case
for the mixed_coroutine_fns bdrv_*(). Holding the lock is still required
when they are called from coroutine context like here!
This effectively reverts 4ec8df0183, but adds some internal locking
instead.
Signed-off-by: Kevin Wolf
---
include/block/block-global-state.h | 8 +++
include/block/block_int-common.h | 4 ++--
block.c| 1 -
block/create.c | 1 -
block/crypto.c | 25 ++--
block/parallels.c | 6 ++---
block/qcow.c | 6 ++---
block/qcow2.c | 37 +++---
block/qed.c| 6 ++---
block/raw-format.c | 2 +-
block/vdi.c| 11 +
block/vhdx.c | 8 ---
block/vmdk.c | 27 ++
block/vpc.c| 6 ++---
14 files changed, 78 insertions(+), 70 deletions(-)
diff --git a/include/block/block-global-state.h
b/include/block/block-global-state.h
index 2d93423d35..f347199bff 100644
--- a/include/block/block-global-state.h
+++ b/include/block/block-global-state.h
@@ -58,14 +58,14 @@ BlockDriver *bdrv_find_protocol(const char *filename,
Error **errp);
BlockDriver *bdrv_find_format(const char *format_name);
-int coroutine_fn GRAPH_RDLOCK
+int coroutine_fn GRAPH_UNLOCKED
bdrv_co_create(BlockDriver *drv, const char *filename, QemuOpts *opts,
Error **errp);
-int co_wrapper_bdrv_rdlock bdrv_create(BlockDriver *drv, const char *filename,
- QemuOpts *opts, Error **errp);
+int co_wrapper bdrv_create(BlockDriver *drv, const char *filename,
+ QemuOpts *opts, Error **errp);
-int coroutine_fn GRAPH_RDLOCK
+int coroutine_fn GRAPH_UNLOCKED
bdrv_co_create_file(const char *filename, QemuOpts *opts, Error **errp);
BlockDriverState *bdrv_new(void);
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index 4909876756..a19b85a6e1 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -245,10 +245,10 @@ struct BlockDriver {
BlockDriverState *bs, QDict *options, int flags, Error **errp);
void (*bdrv_close)(BlockDriverState *bs);
-int coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_create)(
+int coroutine_fn GRAPH_UNLOCKED_PTR (*bdrv_co_create)(
BlockdevCreateOptions *opts, Error **errp);
-int coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_create_opts)(
+int coroutine_fn GRAPH_UNLOCKED_PTR (*bdrv_co_create_opts)(
BlockDriver *drv, const char *filename, QemuOpts *opts, Error **errp);
int (*bdrv_amend_options)(BlockDriverState *bs,
diff --git a/block.c b/block.c
index dad9a4fa43..e8b51a4391 100644
--- a/block.c
+++ b/block.c
@@ -533,7 +533,6 @@ int coroutine_fn bdrv_co_create(BlockDriver *drv, const
char *filename,
int ret;
GLOBAL_STATE_CODE();
ERRP_GUARD();
-assert_bdrv_graph_readable();
if (!drv->bdrv_co_create_opts) {
error_setg(errp, "Driver '%s' does not support image creation",
diff --git a/block/create.c b/block/create.c
index bf67b9947c..6b23a21675 100644
--- a/block/create.c
+++ b/block/create.c
@@ -43,7 +43,6 @@ static int coroutine_fn blockdev_create_run(Job *job, Error
**errp)
int ret;
GLOBAL_STATE_CODE();
-GRAPH_RDLOCK_GUARD();
job_progress_set_remaining(>common, 1);
ret = s->drv->bdrv_co_create(s->opts, errp);
diff --git a/block/crypto.c b/block/crypto.c
index 30093cff9b..6ee8d46d30 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -99,12 +99,10 @@ struct BlockCryptoCreateData {
};
-static int block_crypto_create_write_func(QCryptoBlock *block,
- size_t offset,
- const uint8_t *buf,
- size_t buflen,
- void *opaque,
- Error **errp)
+static int coroutine_fn GRAPH_UNLOCKED
+block_crypto_create_write_func(QCryptoBlock *block, size_t offset,
+ const uint8_t *buf, size_t buflen, void *opaque,
+ Error **errp)
{
struct BlockCryptoCreateData *data = opaque;
ssize_t ret;
@@ -117,10 +115,9 @@ static int block_crypto_create_write_func(QCryptoBlock
*block,
return 0;
}
-static int
[PATCH 0/8] block: Honour graph read lock even in the main thread
Fiona asked why it's correct that bdrv_graph_co_rd_lock/unlock() do nothing if qemu_in_main_thread() returns true. As far as I can tell, it's not correct. The coroutine can yield while it believes to have the lock, and then the main loop can call some code that takes a write lock without waiting for the coroutine to finish. So this series - or more specifically the last patch - fixes this problem by enabling the locking even in the main thread. The patches before this are fixes for bugs that we hadn't discovered while they were only triggered with iothreads, and which are necessary so that all test cases still pass after the final patch. Kevin Wolf (8): block: Call .bdrv_co_create(_opts) unlocked block/export: Fix null pointer dereference in error path qcow2: Unlock the graph in qcow2_do_open() where necessary qemu-img: Take graph lock more selectively test-bdrv-drain: Take graph lock more selectively test-bdrv-drain: Call bdrv_co_unref() in coroutine context blockjob: Adhere to rate limit even when reentered early graph-lock: Honour read locks even in the main thread include/block/block-global-state.h | 8 +++--- include/block/block_int-common.h | 4 +-- include/block/blockjob_int.h | 14 +++--- block.c| 1 - block/commit.c | 7 ++--- block/create.c | 1 - block/crypto.c | 25 + block/export/export.c | 6 +++-- block/graph-lock.c | 10 --- block/mirror.c | 23 +++- block/parallels.c | 6 ++--- block/qcow.c | 6 ++--- block/qcow2.c | 43 +- block/qed.c| 6 ++--- block/raw-format.c | 2 +- block/stream.c | 7 ++--- block/vdi.c| 11 block/vhdx.c | 8 +++--- block/vmdk.c | 27 +-- block/vpc.c| 6 ++--- blockjob.c | 22 +-- qemu-img.c | 5 ++-- tests/unit/test-bdrv-drain.c | 6 ++--- 23 files changed, 138 insertions(+), 116 deletions(-) -- 2.40.1
[PATCH 3/8] qcow2: Unlock the graph in qcow2_do_open() where necessary
qcow2_do_open() calls a few no_co_wrappers that wrap functions taking
the graph lock internally as a writer. Therefore, it can't hold the
reader lock across these calls, it causes deadlocks. Drop the lock
temporarily around the calls.
Signed-off-by: Kevin Wolf
---
block/qcow2.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/block/qcow2.c b/block/qcow2.c
index 73f36447f9..b00b4e7575 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1619,9 +1619,11 @@ qcow2_do_open(BlockDriverState *bs, QDict *options, int
flags,
if (open_data_file) {
/* Open external data file */
+bdrv_graph_co_rdunlock();
s->data_file = bdrv_co_open_child(NULL, options, "data-file", bs,
_of_bds, BDRV_CHILD_DATA,
true, errp);
+bdrv_graph_co_rdlock();
if (*errp) {
ret = -EINVAL;
goto fail;
@@ -1629,10 +1631,12 @@ qcow2_do_open(BlockDriverState *bs, QDict *options, int
flags,
if (s->incompatible_features & QCOW2_INCOMPAT_DATA_FILE) {
if (!s->data_file && s->image_data_file) {
+bdrv_graph_co_rdunlock();
s->data_file = bdrv_co_open_child(s->image_data_file, options,
"data-file", bs,
_of_bds,
BDRV_CHILD_DATA, false,
errp);
+bdrv_graph_co_rdlock();
if (!s->data_file) {
ret = -EINVAL;
goto fail;
@@ -1857,7 +1861,9 @@ qcow2_do_open(BlockDriverState *bs, QDict *options, int
flags,
fail:
g_free(s->image_data_file);
if (open_data_file && has_data_file(bs)) {
+bdrv_graph_co_rdunlock();
bdrv_unref_child(bs, s->data_file);
+bdrv_graph_co_rdlock();
s->data_file = NULL;
}
g_free(s->unknown_header_fields);
--
2.40.1
[PATCH RESEND 4/6] gdbstub: Add support for info proc mappings
Currently the GDB's generate-core-file command doesn't work well with
qemu-user: the resulting dumps are huge [1] and at the same time
incomplete (argv and envp are missing). The reason is that GDB has no
access to proc mappings and therefore has to fall back to using
heuristics for discovering them. This is, in turn, because qemu-user
does not implement the Host I/O feature of the GDB Remote Serial
Protocol.
Implement vFile:{open,close,pread,readlink} and also
qXfer:exec-file:read+.
With that, generate-core-file begins to work on aarch64 and s390x,
albeit with two deficiencies:
* GDB still tries to dump the host mappings, because QEMU does not fake
/proc/$PID/smaps (as opposed to /proc/$PID/maps). The user-visible
effect is only a bunch of warnings.
* PT_LOAD segments lack PF_X flags (I have not debugged this).
The impact of these issues on usability is fairly low, so they can be
resolved later.
[1] https://sourceware.org/pipermail/gdb-patches/2023-May/199432.html
Co-developed-by: Dominik 'Disconnect3d' Czarnota
Signed-off-by: Ilya Leoshkevich
---
gdbstub/gdbstub.c | 45 +-
gdbstub/internals.h | 5 ++
gdbstub/user-target.c | 139 ++
3 files changed, 187 insertions(+), 2 deletions(-)
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 003db59b1b2..c4112d6eacd 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -1326,6 +1326,36 @@ static const GdbCmdParseEntry gdb_v_commands_table[] = {
.cmd = "Kill;",
.cmd_startswith = 1
},
+#ifdef CONFIG_USER_ONLY
+/*
+ * Host I/O Packets. See [1] for details.
+ * [1] https://sourceware.org/gdb/onlinedocs/gdb/Host-I_002fO-Packets.html
+ */
+{
+.handler = gdb_handle_v_file_open,
+.cmd = "File:open:",
+.cmd_startswith = 1,
+.schema = "s,L,L0"
+},
+{
+.handler = gdb_handle_v_file_close,
+.cmd = "File:close:",
+.cmd_startswith = 1,
+.schema = "l0"
+},
+{
+.handler = gdb_handle_v_file_pread,
+.cmd = "File:pread:",
+.cmd_startswith = 1,
+.schema = "l,L,L0"
+},
+{
+.handler = gdb_handle_v_file_readlink,
+.cmd = "File:readlink:",
+.cmd_startswith = 1,
+.schema = "s0"
+},
+#endif
};
static void handle_v_commands(GArray *params, void *user_ctx)
@@ -1471,11 +1501,14 @@ static void handle_query_supported(GArray *params, void
*user_ctx)
";ReverseStep+;ReverseContinue+");
}
-#if defined(CONFIG_USER_ONLY) && defined(CONFIG_LINUX)
+#if defined(CONFIG_USER_ONLY)
+#if defined(CONFIG_LINUX)
if (gdbserver_state.c_cpu->opaque) {
g_string_append(gdbserver_state.str_buf, ";qXfer:auxv:read+");
}
#endif
+g_string_append(gdbserver_state.str_buf, ";qXfer:exec-file:read+");
+#endif
if (params->len &&
strstr(get_param(params, 0)->data, "multiprocess+")) {
@@ -1614,13 +1647,21 @@ static const GdbCmdParseEntry gdb_gen_query_table[] = {
.cmd_startswith = 1,
.schema = "s:l,l0"
},
-#if defined(CONFIG_USER_ONLY) && defined(CONFIG_LINUX)
+#if defined(CONFIG_USER_ONLY)
+#if defined(CONFIG_LINUX)
{
.handler = gdb_handle_query_xfer_auxv,
.cmd = "Xfer:auxv:read::",
.cmd_startswith = 1,
.schema = "l,l0"
},
+#endif
+{
+.handler = gdb_handle_query_xfer_exec_file,
+.cmd = "Xfer:exec-file:read:",
+.cmd_startswith = 1,
+.schema = "l:l,l0"
+},
#endif
{
.handler = gdb_handle_query_attached,
diff --git a/gdbstub/internals.h b/gdbstub/internals.h
index 235f2551bd4..c1217337812 100644
--- a/gdbstub/internals.h
+++ b/gdbstub/internals.h
@@ -184,6 +184,11 @@ typedef union GdbCmdVariant {
void gdb_handle_query_rcmd(GArray *params, void *user_ctx); /* softmmu */
void gdb_handle_query_offsets(GArray *params, void *user_ctx); /* user */
void gdb_handle_query_xfer_auxv(GArray *params, void *user_ctx); /*user */
+void gdb_handle_v_file_open(GArray *params, void *user_ctx); /* user */
+void gdb_handle_v_file_close(GArray *params, void *user_ctx); /* user */
+void gdb_handle_v_file_pread(GArray *params, void *user_ctx); /* user */
+void gdb_handle_v_file_readlink(GArray *params, void *user_ctx); /* user */
+void gdb_handle_query_xfer_exec_file(GArray *params, void *user_ctx); /* user
*/
void gdb_handle_query_attached(GArray *params, void *user_ctx); /* both */
diff --git a/gdbstub/user-target.c b/gdbstub/user-target.c
index fa0e59ec9a5..09df05b5526 100644
--- a/gdbstub/user-target.c
+++ b/gdbstub/user-target.c
@@ -11,6 +11,10 @@
#include "exec/gdbstub.h"
#include "qemu.h"
#include "internals.h"
+#ifdef CONFIG_LINUX
+#include "linux-user/loader.h"
+#include "linux-user/qemu.h"
+#endif
/*
* Map target signal numbers to GDB protocol signal numbers and vice
@@ -281,3 +285,138 @@ void gdb_handle_query_xfer_auxv(GArray *params, void
*user_ctx)
[PATCH RESEND 1/6] linux-user: Expose do_guest_openat() and do_guest_readlink()
These functions will be required by the GDB stub in order to provide
the guest view of /proc to GDB.
Signed-off-by: Ilya Leoshkevich
---
linux-user/qemu.h| 3 +++
linux-user/syscall.c | 54
2 files changed, 38 insertions(+), 19 deletions(-)
diff --git a/linux-user/qemu.h b/linux-user/qemu.h
index e2e93fbd1d5..08bcdd7b7c5 100644
--- a/linux-user/qemu.h
+++ b/linux-user/qemu.h
@@ -165,6 +165,9 @@ typedef struct TaskState {
} TaskState;
abi_long do_brk(abi_ulong new_brk);
+int do_guest_openat(CPUArchState *cpu_env, int dirfd, const char *pathname,
+int flags, mode_t mode);
+ssize_t do_guest_readlink(const char *pathname, char *buf, size_t bufsiz);
/* user access */
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 69f740ff98c..80dbcfec426 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8317,7 +8317,8 @@ static int open_hardware(CPUArchState *cpu_env, int fd)
}
#endif
-static int do_openat(CPUArchState *cpu_env, int dirfd, const char *pathname,
int flags, mode_t mode)
+int do_guest_openat(CPUArchState *cpu_env, int dirfd, const char *pathname,
+int flags, mode_t mode)
{
struct fake_open {
const char *filename;
@@ -8388,6 +8389,36 @@ static int do_openat(CPUArchState *cpu_env, int dirfd,
const char *pathname, int
return safe_openat(dirfd, path(pathname), flags, mode);
}
+ssize_t do_guest_readlink(const char *pathname, char *buf, size_t bufsiz)
+{
+ssize_t ret;
+
+if (!pathname || !buf) {
+errno = EFAULT;
+return -1;
+}
+
+if (!bufsiz) {
+/* Short circuit this for the magic exe check. */
+errno = EINVAL;
+return -1;
+}
+
+if (is_proc_myself((const char *)pathname, "exe")) {
+/*
+ * Don't worry about sign mismatch as earlier mapping
+ * logic would have thrown a bad address error.
+ */
+ret = MIN(strlen(exec_path), bufsiz);
+/* We cannot NUL terminate the string. */
+memcpy(buf, exec_path, ret);
+} else {
+ret = readlink(path(pathname), buf, bufsiz);
+}
+
+return ret;
+}
+
static int do_execveat(CPUArchState *cpu_env, int dirfd,
abi_long pathname, abi_long guest_argp,
abi_long guest_envp, int flags)
@@ -8850,7 +8881,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int
num, abi_long arg1,
case TARGET_NR_open:
if (!(p = lock_user_string(arg1)))
return -TARGET_EFAULT;
-ret = get_errno(do_openat(cpu_env, AT_FDCWD, p,
+ret = get_errno(do_guest_openat(cpu_env, AT_FDCWD, p,
target_to_host_bitmask(arg2,
fcntl_flags_tbl),
arg3));
fd_trans_unregister(ret);
@@ -8860,7 +8891,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int
num, abi_long arg1,
case TARGET_NR_openat:
if (!(p = lock_user_string(arg2)))
return -TARGET_EFAULT;
-ret = get_errno(do_openat(cpu_env, arg1, p,
+ret = get_errno(do_guest_openat(cpu_env, arg1, p,
target_to_host_bitmask(arg3,
fcntl_flags_tbl),
arg4));
fd_trans_unregister(ret);
@@ -10031,22 +10062,7 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int
num, abi_long arg1,
void *p2;
p = lock_user_string(arg1);
p2 = lock_user(VERIFY_WRITE, arg2, arg3, 0);
-if (!p || !p2) {
-ret = -TARGET_EFAULT;
-} else if (!arg3) {
-/* Short circuit this for the magic exe check. */
-ret = -TARGET_EINVAL;
-} else if (is_proc_myself((const char *)p, "exe")) {
-/*
- * Don't worry about sign mismatch as earlier mapping
- * logic would have thrown a bad address error.
- */
-ret = MIN(strlen(exec_path), arg3);
-/* We cannot NUL terminate the string. */
-memcpy(p2, exec_path, ret);
-} else {
-ret = get_errno(readlink(path(p), p2, arg3));
-}
+ret = get_errno(do_guest_readlink(p, p2, arg3));
unlock_user(p2, arg2, ret);
unlock_user(p, arg1, 0);
}
--
2.40.1
[PATCH RESEND 5/6] docs: Document security implications of debugging
Now that the GDB stub implements reading host files, concerns may arise that it undermines security. Document the status quo, which is that the users are already responsible for securing the GDB connection themselves. Signed-off-by: Ilya Leoshkevich --- docs/system/gdb.rst | 15 +++ 1 file changed, 15 insertions(+) diff --git a/docs/system/gdb.rst b/docs/system/gdb.rst index 453eb73f6c4..3cc5167d928 100644 --- a/docs/system/gdb.rst +++ b/docs/system/gdb.rst @@ -192,3 +192,18 @@ The memory mode can be checked by sending the following command: ``maintenance packet Qqemu.PhyMemMode:0`` This will change it back to normal memory mode. + +Security considerations +=== + +Connecting to the GDB socket allows running arbitrary code inside the guest; +in case of the TCG emulation, which is not considered a security boundary, this +also means running arbitrary code on the host. Additionally, when debugging +qemu-user, it allows directly downloading any file readable by QEMU from the +host. + +The GDB socket is not protected by authentication, authorization or encryption. +It is therefore a responsibility of the user to make sure that only authorized +clients can connect to it, e.g., by using a unix socket with proper +permissions, or by opening a TCP socket only on interfaces that are not +reachable by potential attackers. -- 2.40.1
[PATCH RESEND 6/6] tests/tcg: Add a test for info proc mappings
Add a small test to prevent regressions.
Since there are issues with how GDB interprets QEMU's target.xml,
enable the test only on aarch64 and s390x for now.
Signed-off-by: Ilya Leoshkevich
---
tests/tcg/aarch64/Makefile.target | 3 +-
tests/tcg/multiarch/Makefile.target | 7 +++
.../multiarch/gdbstub/test-proc-mappings.py | 55 +++
tests/tcg/s390x/Makefile.target | 2 +-
4 files changed, 65 insertions(+), 2 deletions(-)
create mode 100644 tests/tcg/multiarch/gdbstub/test-proc-mappings.py
diff --git a/tests/tcg/aarch64/Makefile.target
b/tests/tcg/aarch64/Makefile.target
index 03157954871..38402b0ba1f 100644
--- a/tests/tcg/aarch64/Makefile.target
+++ b/tests/tcg/aarch64/Makefile.target
@@ -97,7 +97,8 @@ run-gdbstub-sve-ioctls: sve-ioctls
--bin $< --test $(AARCH64_SRC)/gdbstub/test-sve-ioctl.py, \
basic gdbstub SVE ZLEN support)
-EXTRA_RUNS += run-gdbstub-sysregs run-gdbstub-sve-ioctls
+EXTRA_RUNS += run-gdbstub-sysregs run-gdbstub-sve-ioctls \
+ run-gdbstub-proc-mappings
endif
endif
diff --git a/tests/tcg/multiarch/Makefile.target
b/tests/tcg/multiarch/Makefile.target
index 373db696481..cbc0b75787a 100644
--- a/tests/tcg/multiarch/Makefile.target
+++ b/tests/tcg/multiarch/Makefile.target
@@ -81,6 +81,13 @@ run-gdbstub-qxfer-auxv-read: sha1
--bin $< --test
$(MULTIARCH_SRC)/gdbstub/test-qxfer-auxv-read.py, \
basic gdbstub qXfer:auxv:read support)
+run-gdbstub-proc-mappings: sha1
+ $(call run-test, $@, $(GDB_SCRIPT) \
+ --gdb $(HAVE_GDB_BIN) \
+ --qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
+ --bin $< --test $(MULTIARCH_SRC)/gdbstub/test-proc-mappings.py,
\
+ proc mappings support)
+
run-gdbstub-thread-breakpoint: testthread
$(call run-test, $@, $(GDB_SCRIPT) \
--gdb $(HAVE_GDB_BIN) \
diff --git a/tests/tcg/multiarch/gdbstub/test-proc-mappings.py
b/tests/tcg/multiarch/gdbstub/test-proc-mappings.py
new file mode 100644
index 000..657e36a2fc7
--- /dev/null
+++ b/tests/tcg/multiarch/gdbstub/test-proc-mappings.py
@@ -0,0 +1,55 @@
+"""Test that gdbstub has access to proc mappings.
+
+This runs as a sourced script (via -x, via run-test.py)."""
+from __future__ import print_function
+import gdb
+import sys
+
+
+n_failures = 0
+
+
+def report(cond, msg):
+"""Report success/fail of a test"""
+if cond:
+print("PASS: {}".format(msg))
+else:
+print("FAIL: {}".format(msg))
+global n_failures
+n_failures += 1
+
+
+def run_test():
+"""Run through the tests one by one"""
+mappings = gdb.execute("info proc mappings", False, True)
+report(isinstance(mappings, str), "Fetched the mappings from the inferior")
+report("/sha1" in mappings, "Found the test binary name in the mappings")
+
+
+def main():
+"""Prepare the environment and run through the tests"""
+try:
+inferior = gdb.selected_inferior()
+print("ATTACHED: {}".format(inferior.architecture().name()))
+except (gdb.error, AttributeError):
+print("SKIPPING (not connected)")
+exit(0)
+
+if gdb.parse_and_eval('$pc') == 0:
+print("SKIP: PC not set")
+exit(0)
+
+try:
+# These are not very useful in scripts
+gdb.execute("set pagination off")
+gdb.execute("set confirm off")
+
+# Run the actual tests
+run_test()
+except gdb.error:
+report(False, "GDB Exception: {}".format(sys.exc_info()[0]))
+print("All tests complete: %d failures" % n_failures)
+exit(n_failures)
+
+
+main()
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 0031868b136..2934ac9adf2 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -74,7 +74,7 @@ run-gdbstub-signals-s390x: signals-s390x
--bin $< --test $(S390X_SRC)/gdbstub/test-signals-s390x.py, \
mixing signals and debugging)
-EXTRA_RUNS += run-gdbstub-signals-s390x
+EXTRA_RUNS += run-gdbstub-signals-s390x run-gdbstub-proc-mappings
endif
# MVX versions of sha512
--
2.40.1
[PATCH RESEND 3/6] gdbstub: Report the actual qemu-user pid
Currently qemu-user reports pid 1 to GDB. Resolve the TODO and report
the actual PID. Using getpid() relies on the assumption that there is
only one GDBProcess. Add an assertion to make sure that future changes
don't break it.
Signed-off-by: Ilya Leoshkevich
---
gdbstub/gdbstub.c | 25 +
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 207250c1c08..003db59b1b2 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -202,13 +202,16 @@ void gdb_memtox(GString *buf, const char *mem, int len)
static uint32_t gdb_get_cpu_pid(CPUState *cpu)
{
-/* TODO: In user mode, we should use the task state PID */
+#ifdef CONFIG_USER_ONLY
+return getpid();
+#else
if (cpu->cluster_index == UNASSIGNED_CLUSTER_INDEX) {
/* Return the default process' PID */
int index = gdbserver_state.process_num - 1;
return gdbserver_state.processes[index].pid;
}
return cpu->cluster_index + 1;
+#endif
}
GDBProcess *gdb_get_process(uint32_t pid)
@@ -2127,19 +2130,25 @@ void gdb_read_byte(uint8_t ch)
void gdb_create_default_process(GDBState *s)
{
GDBProcess *process;
-int max_pid = 0;
+int pid;
+#ifdef CONFIG_USER_ONLY
+assert(gdbserver_state.process_num == 0);
+pid = getpid();
+#else
if (gdbserver_state.process_num) {
-max_pid = s->processes[s->process_num - 1].pid;
+pid = s->processes[s->process_num - 1].pid;
+} else {
+pid = 0;
}
+/* We need an available PID slot for this process */
+assert(pid < UINT32_MAX);
+pid++;
+#endif
s->processes = g_renew(GDBProcess, s->processes, ++s->process_num);
process = >processes[s->process_num - 1];
-
-/* We need an available PID slot for this process */
-assert(max_pid < UINT32_MAX);
-
-process->pid = max_pid + 1;
+process->pid = pid;
process->attached = false;
process->target_xml[0] = '\0';
}
--
2.40.1
[PATCH RESEND 0/6] gdbstub: Add support for info proc mappings
[Apologies to people in To: and Cc:, who will get this the second time - I forgot to Cc: the mailing list initially.] Hi, this series partially implements the Host I/O feature of the GDB Remote Serial Protocol in order to make generate-core-file work with qemu-user. It borrows heavily from the abandoned patch by Dominik [1], hence 4/6 carries the respective Co-developed-by: tag. I hope that's okay. I also peeked at gdbserver/hostio.cc quite a few times. The changes compared to Dominik's patch are: - Implement readlink. - Move the main functionality to user-target.c. - Allocate buffers on heap. - Add a test. - Update gdb.rst. - Split refactorings to the existing code into separate patches. - Rename do_openat() to do_guest_openat(). - Do not retry pread(), since GDB is capable of doing it itself. - Add an extra sanity check to gdb_handle_query_xfer_exec_file(). - Replace citations of the spec by a single link. Best regards, Ilya [1] https://lore.kernel.org/all/20220221030910.3203063-1-dominik.b.czarn...@gmail.com/ Ilya Leoshkevich (6): linux-user: Expose do_guest_openat() and do_guest_readlink() gdbstub: Expose gdb_get_process() and gdb_get_first_cpu_in_process() gdbstub: Report the actual qemu-user pid gdbstub: Add support for info proc mappings docs: Document security implications of debugging tests/tcg: Add a test for info proc mappings docs/system/gdb.rst | 15 ++ gdbstub/gdbstub.c | 86 --- gdbstub/internals.h | 7 + gdbstub/user-target.c | 139 ++ linux-user/qemu.h | 3 + linux-user/syscall.c | 54 --- tests/tcg/aarch64/Makefile.target | 3 +- tests/tcg/multiarch/Makefile.target | 7 + .../multiarch/gdbstub/test-proc-mappings.py | 55 +++ tests/tcg/s390x/Makefile.target | 2 +- 10 files changed, 332 insertions(+), 39 deletions(-) create mode 100644 tests/tcg/multiarch/gdbstub/test-proc-mappings.py -- 2.40.1
[PATCH RESEND 2/6] gdbstub: Expose gdb_get_process() and gdb_get_first_cpu_in_process()
These functions will be needed by user-target.c in order to retrieve
the name of the executable.
Signed-off-by: Ilya Leoshkevich
---
gdbstub/gdbstub.c | 16
gdbstub/internals.h | 2 ++
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 0760d786858..207250c1c08 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -211,7 +211,7 @@ static uint32_t gdb_get_cpu_pid(CPUState *cpu)
return cpu->cluster_index + 1;
}
-static GDBProcess *gdb_get_process(uint32_t pid)
+GDBProcess *gdb_get_process(uint32_t pid)
{
int i;
@@ -247,7 +247,7 @@ static CPUState *find_cpu(uint32_t thread_id)
return NULL;
}
-static CPUState *get_first_cpu_in_process(GDBProcess *process)
+CPUState *gdb_get_first_cpu_in_process(GDBProcess *process)
{
CPUState *cpu;
@@ -325,7 +325,7 @@ static CPUState *gdb_get_cpu(uint32_t pid, uint32_t tid)
return NULL;
}
-return get_first_cpu_in_process(process);
+return gdb_get_first_cpu_in_process(process);
} else {
/* a specific thread */
cpu = find_cpu(tid);
@@ -354,7 +354,7 @@ static const char *get_feature_xml(const char *p, const
char **newp,
size_t len;
int i;
const char *name;
-CPUState *cpu = get_first_cpu_in_process(process);
+CPUState *cpu = gdb_get_first_cpu_in_process(process);
CPUClass *cc = CPU_GET_CLASS(cpu);
len = 0;
@@ -490,7 +490,7 @@ void gdb_register_coprocessor(CPUState *cpu,
static void gdb_process_breakpoint_remove_all(GDBProcess *p)
{
-CPUState *cpu = get_first_cpu_in_process(p);
+CPUState *cpu = gdb_get_first_cpu_in_process(p);
while (cpu) {
gdb_breakpoint_remove_all(cpu);
@@ -653,7 +653,7 @@ static int gdb_handle_vcont(const char *p)
goto out;
}
-cpu = get_first_cpu_in_process(process);
+cpu = gdb_get_first_cpu_in_process(process);
while (cpu) {
if (newstates[cpu->cpu_index] == 1) {
newstates[cpu->cpu_index] = cur_action;
@@ -1274,7 +1274,7 @@ static void handle_v_attach(GArray *params, void
*user_ctx)
goto cleanup;
}
-cpu = get_first_cpu_in_process(process);
+cpu = gdb_get_first_cpu_in_process(process);
if (!cpu) {
goto cleanup;
}
@@ -1392,7 +1392,7 @@ static void handle_query_curr_tid(GArray *params, void
*user_ctx)
* first thread).
*/
process = gdb_get_cpu_process(gdbserver_state.g_cpu);
-cpu = get_first_cpu_in_process(process);
+cpu = gdb_get_first_cpu_in_process(process);
g_string_assign(gdbserver_state.str_buf, "QC");
gdb_append_thread_id(cpu, gdbserver_state.str_buf);
gdb_put_strbuf();
diff --git a/gdbstub/internals.h b/gdbstub/internals.h
index 94ddff44958..235f2551bd4 100644
--- a/gdbstub/internals.h
+++ b/gdbstub/internals.h
@@ -124,6 +124,8 @@ void gdb_read_byte(uint8_t ch);
*/
bool gdb_got_immediate_ack(void);
/* utility helpers */
+GDBProcess *gdb_get_process(uint32_t pid);
+CPUState *gdb_get_first_cpu_in_process(GDBProcess *process);
CPUState *gdb_first_attached_cpu(void);
void gdb_append_thread_id(CPUState *cpu, GString *buf);
int gdb_get_cpu_index(CPUState *cpu);
--
2.40.1
[PATCH] migration: Add documentation for backwards compatiblity
State what are the requeriments to get migration working between qemu versions. And once there explain how one is supposed to implement a new feature/default value and not break migration. Signed-off-by: Juan Quintela --- Hi I will really appreciate reviews: - I don't speak natively .rst format, so let me what I have done wrong. - English is not my native language either (no points if had guessed that). - This is stuff is obvious to me, so let me when I have assumed things, things that need to be claried, explained better, etc. Thanks, Juan. --- docs/devel/migration.rst | 212 +++ 1 file changed, 212 insertions(+) diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst index 6f65c23b47..daa510da42 100644 --- a/docs/devel/migration.rst +++ b/docs/devel/migration.rst @@ -142,6 +142,218 @@ General advice for device developers may be different on the destination. This can result in the device state being loaded into the wrong device. +How backwards compatibility work + + +When we do migration, we have to qemus: source and target qemu. There +are two cases, they are the same version or they are a different +version. The easy case is when they are the same version. The +difficult one is when they are different versions. + +There are two things that are different, but they have very similar +names and sometimes get confused: +- qemu version +- machine version + +Let's start with a practical example, we start with: + +- qemu-system-x86_64 (v5.2), from now one qemu-5.2. +- qemu-system-x86_64 (v5.1), from now one qemu-5.1. + +Related to this are the "latest" machine types defined on each of +them: + +- pc-q35-5.2 (newer one in qemu-5.2) from now on pc-5.2 +- pc-q35-5.1 (newer one qemu-5.1) from now on pc-5.1 + +First of all, migration is only supposed to work if you use the same +machine type in both source and destination. The qemu configuration +needs to be the same also on source and destination. But that is not +relevant for this section. + +I am going to list the number of combinations that we can have. Let's +start with the trivial ones, qemu is the same on source and +destination: + +1 - qemu-5.2 -M pc-5.2 -> migrates to -> qemu-5.2 -M pc-5.2 + + This is the latest qemu with the latest machine type. + This have to work, and if it don't work it is a bug. + +2 - qemu-5.1 -M pc-5.1 -> migrates to -> qemu-5.1 -M pc-5.1 + + Exactly the same case than the previous one, but for 5.1. + Nothing to see here either. + +This are the easiest ones, we will not talk more about them in this +section. + +Now we start with the more interesting cases. Let start with the +same qemu but not the same machine type. + +3 - qemu-5.2 -M pc-5.1 -> migrates to -> qemu-5.2 -M pc-5.1 + + It needs to use the definition of pc-5.1 and the devices as they + were configured on 5.1, but this should be easy in the sense that + both sides are the same qemu and both sides have exactly the same + idea of what the pc-5.1 machine is. + +4 - qemu-5.1 -M pc-5.2 -> migrates to -> qemu-5.1 -M pc-5.2 + + This combination is not possible as the qemu-5.1 don't understand + pc-5.2 machine type. So nothing to worry here. + +Now it comes the interesting ones, when both qemus are different. +Notice also that the machine type needs to be pc-5.1, because we have +the limitation than qemu-5.1 don't know pc-5.2. So the possible cases +are: + +5 - qemu-5.2 -M pc-5.1 -> migrates to -> qemu-5.1 -M pc-5.1 + + This migration is known as newer to older. We need to make sure + when we are developing 5.2 we need to take care about not to break + migration to qemu-5.1. Notice that we can't make updates to + qemu-5.1 to understand whatever qemu-5.2 decides to change, so it is + in qemu-5.2 side to make the relevant changes. + +6 - qemu-5.1 -M pc-5.1 -> migrates to -> qemu-5.2 -M pc-5.1 + + This migration is known as older to newer. We need to make sure + than we are able to receive migrations from qemu-5.1. The problem is + similar to the previous one. + +If qemu-5.1 and qemu-5.2 were the same, there will not be any +compatibility problems. But the reason that we create qemu-5.2 is to +get new features, devices, defaults, etc. + +If we get a device that get a new feature, or change a default value, +we have a problem when we try to migrate between different qemu +versions. + +So we need a way to tell qemu-5.2 than when we are using machine type +pc-5.1, it needs to **not** use the feature, to be able to migrate to +read qemu-5.1. + +And the equivalent part when migrating from qemu-5.1 to qemu-5.2. +qemu-5.2 have to expect that it is not going to get data for the new +feature, because qemu-5.1 don't know about it. + +How do we tell qemu about this device feature changes? In +hw/core/machine.c:hw_compat_X_Y arrays. + +If we change a default value, we need to put back the old value on +that array. And the device, during initialization
Re: [PATCH v4 0/9] PC cleanups
On Wed, May 10, 2023 at 06:26:54PM +, Bernhard Beschow wrote:
>
>
> Am 5. März 2023 07:45:55 UTC schrieb Bernhard Beschow :
> >
> >
> >Am 13. Februar 2023 16:45:05 UTC schrieb Bernhard Beschow
> >:
> >>
> >>
> >>Am 13. Februar 2023 16:19:55 UTC schrieb Bernhard Beschow
> >>:
> >>>This series contains some cleanups I came across when working on the PC
> >>>
> >>>machines. It consists of reducing the usage of global variables and
> >>>eliminating
> >>>
> >>>some redundancies.
> >>>
> >>>
> >>>
> >>>One notable change is that the SMRAM memory region gets moved from the
> >>>i440fx
> >>>
> >>>and q35 host bridges into the x86 machine. This will simplify cleaning up
> >>>these
> >>>
> >>>host bridges which will be done in a separate series. Note that the
> >>>movement of
> >>>
> >>>the SMRAM memory region apparently doesn't change migration ABI for the pc
> >>>and
> >>>
> >>>q35 machines (see below).
> >>>
> >>>
> >>>
> >>>Testing done:
> >>>
> >>>* `make check`
> >>>
> >>>' `make check-avocado`
> >>>
> >>>* `qemu-system-x86_64 -M q35 -m 2G -cdrom \
> >>>
> >>> manjaro-kde-21.3.2-220704-linux515.iso`
> >>>
> >>>* `qemu-system-x86_64 -M pc -m 2G -cdrom
> >>>manjaro-kde-21.3.2-220704-linux515.iso`
> >>>
> >>>* Confirm that JSON representation of migration files (pc & q35) are empty:
> >>>
> >>> 1. Create four migration files {pc,q35}-{before,after}.mig by running
> >>>
> >>> `qemu-system-x86_64 -M {pc,q35} -S` with QEMU built from master and
> >>> from
> >>>
> >>> this series.
> >>>
> >>> 2. Run `./scripts/analyze-migration.py -d desc -f *.mig > *.json` on the
> >>> four
> >>>
> >>> files
> >>>
> >>> 3. Compare the diffs -> both are empty
> >>>
> >>>
> >>>
> >>>v4:
> >>>
> >>>* Remove ram_memory variable in pc_q35 completely (Zoltan)
> >>>
> >>
> >>The last two patches still need review. Comments welcome!
> >
> >Ping
> >
> >Can we queue the reviewed patches (all but the last two) already?
>
> Ping 2
queued 1-7
> >
> >Thanks,
> >Bernhard
> >
> >>
> >>>
> >>>
> >>>v3:
> >>>
> >>>* Add three patches regarding init_pam() and SMRAM.
> >>>
> >>>* Drop 'hw/i386/pc_q35: Resolve redundant q35_host variable' since Phil
> >>>posted
> >>>
> >>> a similar patch in a more comprehensive series:
> >>>
> >>>
> >>> https://lore.kernel.org/qemu-devel/20230203180914.49112-13-phi...@linaro.org/
> >>>
> >>>* Drop 'hw/isa/lpc_ich9: Reuse memory and io address space of PCI bus'
> >>>since
> >>>
> >>> it inadvertantly changed the memory hierarchy.
> >>>
> >>>* Drop ICH9 cleanups again in favor of a separate series.
> >>>
> >>>
> >>>
> >>>v2:
> >>>
> >>>* Factor out 'hw/i386/pc_q35: Reuse machine parameter' from
> >>>'hw/i386/pc_q35:
> >>>
> >>> Resolve redundant q35_host variable' (Zoltan)
> >>>
> >>>* Lower type of phb to Object in 'hw/i386/pc_q35: Resolve redundant
> >>>q35_host
> >>>
> >>> variable' (Zoltan)
> >>>
> >>>* Add ICH9 cleanups
> >>>
> >>>
> >>>
> >>>Bernhard Beschow (9):
> >>>
> >>> hw/pci-host/i440fx: Inline sysbus_add_io()
> >>>
> >>> hw/pci-host/q35: Inline sysbus_add_io()
> >>>
> >>> hw/i386/pc_q35: Reuse machine parameter
> >>>
> >>> hw/i386/pc_{q35,piix}: Reuse MachineClass::desc as SMB product name
> >>>
> >>> hw/i386/pc_{q35,piix}: Minimize usage of get_system_memory()
> >>>
> >>> hw/i386/pc: Initialize ram_memory variable directly
> >>>
> >>> hw/pci-host/pam: Make init_pam() usage more readable
> >>>
> >>> hw/i386/x86: Make TYPE_X86_MACHINE the owner of smram
> >>>
> >>> target/i386/tcg/sysemu/tcg-cpu: Avoid own opinion about smram size
> >>>
> >>>
> >>>
> >>> include/hw/i386/pc.h | 1 -
> >>>
> >>> include/hw/i386/x86.h| 2 ++
> >>>
> >>> include/hw/pci-host/i440fx.h | 7 ---
> >>>
> >>> include/hw/pci-host/pam.h| 5 +++--
> >>>
> >>> include/hw/pci-host/q35.h| 4 +++-
> >>>
> >>> hw/i386/pc.c | 2 --
> >>>
> >>> hw/i386/pc_piix.c| 10 +-
> >>>
> >>> hw/i386/pc_q35.c | 17 +
> >>>
> >>> hw/i386/x86.c| 4
> >>>
> >>> hw/pci-host/i440fx.c | 28 +---
> >>>
> >>> hw/pci-host/pam.c| 12 ++--
> >>>
> >>> hw/pci-host/q35.c| 31 ---
> >>>
> >>> target/i386/tcg/sysemu/tcg-cpu.c | 3 +--
> >>>
> >>> 13 files changed, 66 insertions(+), 60 deletions(-)
> >>>
> >>>
> >>>
> >>>-- >
> >>>2.39.1
> >>>
> >>>
> >>>
Re: [PATCH v8 00/23] Consolidate PIIX south bridges
On Wed, May 10, 2023 at 06:39:49PM +, Bernhard Beschow wrote: > > > Am 21. April 2023 16:40:47 UTC schrieb Bernhard Beschow : > > > > > >Am 21. April 2023 07:15:18 UTC schrieb "Michael S. Tsirkin" > >: > >>On Thu, Mar 02, 2023 at 10:21:38PM +0100, Bernhard Beschow wrote: > >>> This series consolidates the implementations of the PIIX3 and PIIX4 south > >>> bridges and is an extended version of [1]. The motivation is to share as > >>> much > >>> code as possible and to bring both device models to feature parity such > >>> that > >>> perhaps PIIX4 can become a drop-in-replacement for PIIX3 in the pc > >>> machine. This > >>> could resolve the "Frankenstein" PIIX4-PM problem in PIIX3 discussed on > >>> this > >>> list before. > >> > >>Hi! > >>No freeze is over, could you rebase pls? > >>I could try to resolve the conflicts but this is so big I'd rather > >>not take the risk of messing it up. > > > >Sure! Since this series is still under discussion I'd wait for the PIIX3 Xen > >decoupling series to land in master. This will simplify this series a bit by > >taking Xen out of the equation. > > Could we queue the first two RTC patches already? IMO they're useful general > PC cleanups on their own. > > Best regards, > Bernhard queues 1 and 2. > > > >Best regards, > >Bernhard > > > >> > >>> The series is structured as follows: > >>> > >>> Move sub devices into the PIIX3 south bridge, like PIIX4 does already: > >>> * hw/i386/pc: Create RTC controllers in south bridges > >>> * hw/i386/pc: No need for rtc_state to be an out-parameter > >>> * hw/i386/pc_piix: Allow for setting properties before realizing PIIX3 > >>> south bridge > >>> * hw/isa/piix3: Create USB controller in host device > >>> * hw/isa/piix3: Create power management controller in host device > >>> * hw/isa/piix3: Move ISA bus IRQ assignments into host device > >>> * hw/isa/piix3: Create IDE controller in host device > >>> * hw/isa/piix3: Wire up ACPI interrupt internally > >>> > >>> Make PIIX3 and PIIX4 south bridges more similar: > >>> * hw/isa/piix3: Resolve redundant PIIX_NUM_PIC_IRQS > >>> * hw/isa/piix3: Rename pci_piix3_props for sharing with PIIX4 > >>> * hw/isa/piix3: Rename piix3_reset() for sharing with PIIX4 > >>> * hw/isa/piix3: Drop the "3" from PIIX base class > >>> * hw/isa/piix4: Make PIIX4's ACPI and USB functions optional > >>> * hw/isa/piix4: Remove unused inbound ISA interrupt lines > >>> * hw/isa/piix4: Reuse struct PIIXState from PIIX3 > >>> * hw/isa/piix4: Create the "intr" property during init() already > >>> * hw/isa/piix4: Rename reset control operations to match PIIX3 > >>> > >>> This patch achieves the main goal of the series: > >>> * hw/isa/piix3: Merge hw/isa/piix4.c > >>> > >>> Perform some further consolidations which were easier to do after the > >>> merge: > >>> * hw/isa/piix: Harmonize names of reset control memory regions > >>> * hw/isa/piix: Rename functions to be shared for interrupt triggering > >>> * hw/isa/piix: Consolidate IRQ triggering > >>> * hw/isa/piix: Share PIIX3's base class with PIIX4 > >>> * hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4 > >>> > >>> One challenge was dealing with optional devices where Peter already gave > >>> advice > >>> in [1] which this series implements. > >>> > >>> There are still some differences in the device models: > >>> - PIIX4 instantiates its own PIC and PIT while PIIX3 doesn't > >>> - PIIX4 wires up the RTC IRQ itself while PIIX3 doesn't > >>> - Different binary layout in VM state > >>> > >>> v8: > >>> - Rebase onto master > >>> - Remove Reviewed-by tag from 'hw/isa/piix: Reuse PIIX3 base class' > >>> realize > >>> method in PIIX4' since it changed considerably in v7. > >>> > >>> Testing done (both on top of series as well as on 'hw/isa/piix3: Drop the > >>> "3" > >>> from PIIX base class'): > >>> * `make check` > >>> * `make check-avocado` > >>> * Boot live CD: > >>> * `qemu-system-x86_64 -M pc -m 2G -accel kvm -cpu host -cdrom > >>> manjaro-kde-21.3.2-220704-linux515.iso` > >>> * `qemu-system-x86_64 -M q35 -m 2G -accel kvm -cpu host -cdrom > >>> manjaro-kde-21.3.2-220704-linux515.iso` > >>> * 'qemu-system-mips64el -M malta -kernel vmlinux-3.2.0-4-5kc-malta -hda > >>> debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 > >>> console=ttyS0"` > >>> > >>> v7: > >>> - Rebase onto master > >>> - Avoid the PIC proxy (Phil) > >>> The motivation for the PIC proxy was to allow for wiring up ISA > >>> interrupts in > >>> the south bridges. ISA interrupt wiring requires the GPIO lines to be > >>> populated already but pc_piix assigned the interrupts only after > >>> realizing > >>> PIIX3. By shifting interrupt assignment before realizing, the ISA > >>> interrupts > >>> are already populated during PIIX3's realize phase where the ISA > >>> interrupts > >>> are wired up. > >>> - New patches: > >>> * hw/isa/piix4: Reuse struct PIIXState from PIIX3 > >>> * hw/isa/piix4: Create the "intr" property during
Re: [PATCH v2 00/12] simpletrace: refactor and general improvements
On Mon, May 8, 2023 at 11:16 AM Stefan Hajnoczi wrote: > > On Mon, May 08, 2023 at 03:28:04PM +0200, Mads Ynddal wrote: > > > > > A question for you: do you think it's possible to move simpletrace into > > > qemu/python/utils? This requires cleaning up the code to some fairly > > > pedantic standards, but helps protect it against rot as we change target > > > python versions. > > > > > > No problem if that's too much to ask. just want to know if you had looked > > > into it. > > > > Potentially, this would be possible. But `simpletrace.py` imports > > `qemu/scripts/tracetool/`, so that would have to be moved as well, or > > installed > > as a package. I haven't touched the `tracetool` code itself, so I'm not > > sure how > > feasible it is (i.e. how many other places use `tracetool`). > > tracetool is only used by QEMU's build system to generate code from the > trace-events files. In theory it's possible to move it. > > I'm not sure I understand the purpose of moving it to python/. What > infrastructure does python/ provide that's not available to > simpletrace.py in its current location? > > Stefan It's just directory-based: python/qemu/ are formatted as packages, with python/qemu/util/ serving as a package that houses a bag of scripts. You can install these things as a package into a venv and use them anywhere. code in python/ is checked with a linter, type checker, etc while code in scripts/ is not. It isn't installed anywhere and you may or may not have to carefully set up PYTHONPATH= or choose your CWD carefully to get the imports correct. Over time I want to move more python code over under python/ so it gets checked with the robots. The best stuff to move is stuff with imports and dependencies. Truly standalone scripts aren't as important to move. It's not important for today, so let's not worry about it for this series. --js
Re: [PATCH 1/1] e1000e: Fix tx/rx counters
10.04.2023 18:27, timothee.coca...@gmail.com wrote: The bytes and packets counter registers are cleared on read. Copying the "total counter" registers to the "good counter" registers has side effects. If the "total" register is never read by the OS, it only gets incremented. This leads to exponential growth of the "good" register. This commit increments the counters individually to avoid this. Smells like a -stable material, is it not? Thanks, /mjt
Re: [PATCH] disas: Fix tabs and braces in disas.c
On Wed, 10 May 2023 at 18:09, Richard Henderson wrote: > > Fix these before moving the file, for checkpatch.pl. > > Signed-off-by: Richard Henderson > --- > disas.c | 11 ++- Reviewed-by: Peter Maydell thanks -- PMM
Re: [PATCH v2 0/2] SDL2 usability fixes
Am 17. April 2023 19:21:37 UTC schrieb Bernhard Beschow : >I'm trying to use QEMU on Windows hosts for fun and for profit. While the GTK > >GUI doesn't seem to support OpenGL under Windows the SDL2 GUI does. Hence I > >used the SDL2 GUI where I ran into several issues of which two are fixed in > >this series, which are: > > > >* Alt+Tab switches tasks on the host rather than in the guest in fullscreen >mode > >* Alt+F4 closes QEMU rather than a graphical task in the guest > > > >More information about each issue is provided in the patches. > > > >v2: > >* Omit AltGr patch since Voker is taking care of it > >* Protect Alt+Tab hint with #ifdef (Volker) > > > >Bernhard Beschow (2): > > ui/sdl2: Grab Alt+Tab also in fullscreen mode > > ui/sdl2: Grab Alt+F4 also under Windows Ping All patches reviewed > > > > ui/sdl2.c | 4 > > 1 file changed, 4 insertions(+) > > > >-- > >2.40.0 > > >
Re: [PATCH v2 1/1] ui/sdl2: disable SDL_HINT_GRAB_KEYBOARD on Windows
Am 24. April 2023 03:33:05 UTC schrieb Stefan Weil :
>Am 18.04.23 um 08:56 schrieb Volker Rümelin:
>
>> Windows sends an extra left control key up/down input event for
>> every right alt key up/down input event for keyboards with
>> international layout. Since commit 830473455f ("ui/sdl2: fix
>> handling of AltGr key on Windows") QEMU uses a Windows low level
>> keyboard hook procedure to reliably filter out the special left
>> control key and to grab the keyboard on Windows.
>>
>> The SDL2 version 2.0.16 introduced its own Windows low level
>> keyboard hook procedure to grab the keyboard. Windows calls this
>> callback before the QEMU keyboard hook procedure. This disables
>> the special left control key filter when the keyboard is grabbed.
>>
>> To fix the problem, disable the SDL2 Windows low level keyboard
>> hook procedure.
>>
>> Reported-by: Bernhard Beschow
>> Signed-off-by: Volker Rümelin
>> ---
>> ui/sdl2.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/ui/sdl2.c b/ui/sdl2.c
>> index 00aadfae37..9d703200bf 100644
>> --- a/ui/sdl2.c
>> +++ b/ui/sdl2.c
>> @@ -855,7 +855,10 @@ static void sdl2_display_init(DisplayState *ds,
>> DisplayOptions *o)
>> #ifdef SDL_HINT_VIDEO_X11_NET_WM_BYPASS_COMPOSITOR /* only available since
>> SDL 2.0.8 */
>> SDL_SetHint(SDL_HINT_VIDEO_X11_NET_WM_BYPASS_COMPOSITOR, "0");
>> #endif
>> +#ifndef CONFIG_WIN32
>> +/* QEMU uses its own low level keyboard hook procecure on Windows */
>
>
>s/procecure/procedure/
>
>
>> SDL_SetHint(SDL_HINT_GRAB_KEYBOARD, "1");
>> +#endif
>> #ifdef SDL_HINT_ALLOW_ALT_TAB_WHILE_GRABBED
>> SDL_SetHint(SDL_HINT_ALLOW_ALT_TAB_WHILE_GRABBED, "0");
>> #endif
>
>
>The typo fix for the comment does not require a v3 and can be applied in the
>pull request.
>
>Reviewed-by: Stefan Weil
Ping
Re: [PATCH v8 00/23] Consolidate PIIX south bridges
Am 21. April 2023 16:40:47 UTC schrieb Bernhard Beschow : > > >Am 21. April 2023 07:15:18 UTC schrieb "Michael S. Tsirkin" : >>On Thu, Mar 02, 2023 at 10:21:38PM +0100, Bernhard Beschow wrote: >>> This series consolidates the implementations of the PIIX3 and PIIX4 south >>> bridges and is an extended version of [1]. The motivation is to share as >>> much >>> code as possible and to bring both device models to feature parity such that >>> perhaps PIIX4 can become a drop-in-replacement for PIIX3 in the pc machine. >>> This >>> could resolve the "Frankenstein" PIIX4-PM problem in PIIX3 discussed on this >>> list before. >> >>Hi! >>No freeze is over, could you rebase pls? >>I could try to resolve the conflicts but this is so big I'd rather >>not take the risk of messing it up. > >Sure! Since this series is still under discussion I'd wait for the PIIX3 Xen >decoupling series to land in master. This will simplify this series a bit by >taking Xen out of the equation. Could we queue the first two RTC patches already? IMO they're useful general PC cleanups on their own. Best regards, Bernhard > >Best regards, >Bernhard > >> >>> The series is structured as follows: >>> >>> Move sub devices into the PIIX3 south bridge, like PIIX4 does already: >>> * hw/i386/pc: Create RTC controllers in south bridges >>> * hw/i386/pc: No need for rtc_state to be an out-parameter >>> * hw/i386/pc_piix: Allow for setting properties before realizing PIIX3 >>> south bridge >>> * hw/isa/piix3: Create USB controller in host device >>> * hw/isa/piix3: Create power management controller in host device >>> * hw/isa/piix3: Move ISA bus IRQ assignments into host device >>> * hw/isa/piix3: Create IDE controller in host device >>> * hw/isa/piix3: Wire up ACPI interrupt internally >>> >>> Make PIIX3 and PIIX4 south bridges more similar: >>> * hw/isa/piix3: Resolve redundant PIIX_NUM_PIC_IRQS >>> * hw/isa/piix3: Rename pci_piix3_props for sharing with PIIX4 >>> * hw/isa/piix3: Rename piix3_reset() for sharing with PIIX4 >>> * hw/isa/piix3: Drop the "3" from PIIX base class >>> * hw/isa/piix4: Make PIIX4's ACPI and USB functions optional >>> * hw/isa/piix4: Remove unused inbound ISA interrupt lines >>> * hw/isa/piix4: Reuse struct PIIXState from PIIX3 >>> * hw/isa/piix4: Create the "intr" property during init() already >>> * hw/isa/piix4: Rename reset control operations to match PIIX3 >>> >>> This patch achieves the main goal of the series: >>> * hw/isa/piix3: Merge hw/isa/piix4.c >>> >>> Perform some further consolidations which were easier to do after the merge: >>> * hw/isa/piix: Harmonize names of reset control memory regions >>> * hw/isa/piix: Rename functions to be shared for interrupt triggering >>> * hw/isa/piix: Consolidate IRQ triggering >>> * hw/isa/piix: Share PIIX3's base class with PIIX4 >>> * hw/isa/piix: Reuse PIIX3 base class' realize method in PIIX4 >>> >>> One challenge was dealing with optional devices where Peter already gave >>> advice >>> in [1] which this series implements. >>> >>> There are still some differences in the device models: >>> - PIIX4 instantiates its own PIC and PIT while PIIX3 doesn't >>> - PIIX4 wires up the RTC IRQ itself while PIIX3 doesn't >>> - Different binary layout in VM state >>> >>> v8: >>> - Rebase onto master >>> - Remove Reviewed-by tag from 'hw/isa/piix: Reuse PIIX3 base class' realize >>> method in PIIX4' since it changed considerably in v7. >>> >>> Testing done (both on top of series as well as on 'hw/isa/piix3: Drop the >>> "3" >>> from PIIX base class'): >>> * `make check` >>> * `make check-avocado` >>> * Boot live CD: >>> * `qemu-system-x86_64 -M pc -m 2G -accel kvm -cpu host -cdrom >>> manjaro-kde-21.3.2-220704-linux515.iso` >>> * `qemu-system-x86_64 -M q35 -m 2G -accel kvm -cpu host -cdrom >>> manjaro-kde-21.3.2-220704-linux515.iso` >>> * 'qemu-system-mips64el -M malta -kernel vmlinux-3.2.0-4-5kc-malta -hda >>> debian_wheezy_mipsel_standard.qcow2 -append "root=/dev/sda1 console=ttyS0"` >>> >>> v7: >>> - Rebase onto master >>> - Avoid the PIC proxy (Phil) >>> The motivation for the PIC proxy was to allow for wiring up ISA >>> interrupts in >>> the south bridges. ISA interrupt wiring requires the GPIO lines to be >>> populated already but pc_piix assigned the interrupts only after realizing >>> PIIX3. By shifting interrupt assignment before realizing, the ISA >>> interrupts >>> are already populated during PIIX3's realize phase where the ISA >>> interrupts >>> are wired up. >>> - New patches: >>> * hw/isa/piix4: Reuse struct PIIXState from PIIX3 >>> * hw/isa/piix4: Create the "intr" property during init() already >>> - Patches with substantial changes (Reviewed-by dropped): >>> * hw/isa/piix3: Move ISA bus IRQ assignments into host device >>> >>> v6: >>> - Fix some comments about TYPE_ISA_PIC (Mark) ... and use it consistently >>> within the patch series. >>> - Incorporate series "[PATCH v2 0/3] Decouple INTx-to-LNKx routing from >>>
Re: [PATCH v4 0/9] PC cleanups
Am 5. März 2023 07:45:55 UTC schrieb Bernhard Beschow :
>
>
>Am 13. Februar 2023 16:45:05 UTC schrieb Bernhard Beschow :
>>
>>
>>Am 13. Februar 2023 16:19:55 UTC schrieb Bernhard Beschow :
>>>This series contains some cleanups I came across when working on the PC
>>>
>>>machines. It consists of reducing the usage of global variables and
>>>eliminating
>>>
>>>some redundancies.
>>>
>>>
>>>
>>>One notable change is that the SMRAM memory region gets moved from the i440fx
>>>
>>>and q35 host bridges into the x86 machine. This will simplify cleaning up
>>>these
>>>
>>>host bridges which will be done in a separate series. Note that the movement
>>>of
>>>
>>>the SMRAM memory region apparently doesn't change migration ABI for the pc
>>>and
>>>
>>>q35 machines (see below).
>>>
>>>
>>>
>>>Testing done:
>>>
>>>* `make check`
>>>
>>>' `make check-avocado`
>>>
>>>* `qemu-system-x86_64 -M q35 -m 2G -cdrom \
>>>
>>> manjaro-kde-21.3.2-220704-linux515.iso`
>>>
>>>* `qemu-system-x86_64 -M pc -m 2G -cdrom
>>>manjaro-kde-21.3.2-220704-linux515.iso`
>>>
>>>* Confirm that JSON representation of migration files (pc & q35) are empty:
>>>
>>> 1. Create four migration files {pc,q35}-{before,after}.mig by running
>>>
>>> `qemu-system-x86_64 -M {pc,q35} -S` with QEMU built from master and from
>>>
>>> this series.
>>>
>>> 2. Run `./scripts/analyze-migration.py -d desc -f *.mig > *.json` on the
>>> four
>>>
>>> files
>>>
>>> 3. Compare the diffs -> both are empty
>>>
>>>
>>>
>>>v4:
>>>
>>>* Remove ram_memory variable in pc_q35 completely (Zoltan)
>>>
>>
>>The last two patches still need review. Comments welcome!
>
>Ping
>
>Can we queue the reviewed patches (all but the last two) already?
Ping 2
>
>Thanks,
>Bernhard
>
>>
>>>
>>>
>>>v3:
>>>
>>>* Add three patches regarding init_pam() and SMRAM.
>>>
>>>* Drop 'hw/i386/pc_q35: Resolve redundant q35_host variable' since Phil
>>>posted
>>>
>>> a similar patch in a more comprehensive series:
>>>
>>>
>>> https://lore.kernel.org/qemu-devel/20230203180914.49112-13-phi...@linaro.org/
>>>
>>>* Drop 'hw/isa/lpc_ich9: Reuse memory and io address space of PCI bus' since
>>>
>>> it inadvertantly changed the memory hierarchy.
>>>
>>>* Drop ICH9 cleanups again in favor of a separate series.
>>>
>>>
>>>
>>>v2:
>>>
>>>* Factor out 'hw/i386/pc_q35: Reuse machine parameter' from 'hw/i386/pc_q35:
>>>
>>> Resolve redundant q35_host variable' (Zoltan)
>>>
>>>* Lower type of phb to Object in 'hw/i386/pc_q35: Resolve redundant q35_host
>>>
>>> variable' (Zoltan)
>>>
>>>* Add ICH9 cleanups
>>>
>>>
>>>
>>>Bernhard Beschow (9):
>>>
>>> hw/pci-host/i440fx: Inline sysbus_add_io()
>>>
>>> hw/pci-host/q35: Inline sysbus_add_io()
>>>
>>> hw/i386/pc_q35: Reuse machine parameter
>>>
>>> hw/i386/pc_{q35,piix}: Reuse MachineClass::desc as SMB product name
>>>
>>> hw/i386/pc_{q35,piix}: Minimize usage of get_system_memory()
>>>
>>> hw/i386/pc: Initialize ram_memory variable directly
>>>
>>> hw/pci-host/pam: Make init_pam() usage more readable
>>>
>>> hw/i386/x86: Make TYPE_X86_MACHINE the owner of smram
>>>
>>> target/i386/tcg/sysemu/tcg-cpu: Avoid own opinion about smram size
>>>
>>>
>>>
>>> include/hw/i386/pc.h | 1 -
>>>
>>> include/hw/i386/x86.h| 2 ++
>>>
>>> include/hw/pci-host/i440fx.h | 7 ---
>>>
>>> include/hw/pci-host/pam.h| 5 +++--
>>>
>>> include/hw/pci-host/q35.h| 4 +++-
>>>
>>> hw/i386/pc.c | 2 --
>>>
>>> hw/i386/pc_piix.c| 10 +-
>>>
>>> hw/i386/pc_q35.c | 17 +
>>>
>>> hw/i386/x86.c| 4
>>>
>>> hw/pci-host/i440fx.c | 28 +---
>>>
>>> hw/pci-host/pam.c| 12 ++--
>>>
>>> hw/pci-host/q35.c| 31 ---
>>>
>>> target/i386/tcg/sysemu/tcg-cpu.c | 3 +--
>>>
>>> 13 files changed, 66 insertions(+), 60 deletions(-)
>>>
>>>
>>>
>>>-- >
>>>2.39.1
>>>
>>>
>>>
Re: [PATCH] ui/cursor: incomplete check for integer overflow in cursor_alloc
08.05.2023 17:18, Mauro Matteo Cascella wrote:
The cursor_alloc function still accepts a signed integer for both the cursor
width and height. A specially crafted negative width/height could make datasize
wrap around and cause the next allocation to be 0, potentially leading to a
heap buffer overflow. Modify QEMUCursor struct and cursor_alloc prototype to
accept unsigned ints.
Fixes: CVE-2023-1601
Fixes: fa892e9a ("ui/cursor: fix integer overflow in cursor_alloc
(CVE-2021-4206)")
Looks like -stable material too?
Thanks,
/mjt
Re: [PATCH] target/i386: fix avx2 instructions vzeroall and vpermdq
10.05.2023 17:52, lixinyu...@ict.ac.cn wrote: From: Xinyu Li vzeroall: xmm_regs should be used instead of xmm_t0 vpermdq: bit 3 and 7 of imm should be considered -stable material, it looks like? Thanks, /mjt
[PULL 10/10] migration: block incoming colo when capability is disabled
From: Vladimir Sementsov-Ogievskiy
We generally require same set of capabilities on source and target.
Let's require x-colo capability to use COLO on target.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Juan Quintela
Reviewed-by: Peter Xu
Reviewed-by: Lukas Straub
Reviewed-by: Zhang Chen
Message-Id: <20230428194928.1426370-11-vsement...@yandex-team.ru>
Signed-off-by: Juan Quintela
---
docs/COLO-FT.txt | 1 +
migration/migration.c | 6 ++
2 files changed, 7 insertions(+)
diff --git a/docs/COLO-FT.txt b/docs/COLO-FT.txt
index 8ec653f81c..2e760a4aee 100644
--- a/docs/COLO-FT.txt
+++ b/docs/COLO-FT.txt
@@ -210,6 +210,7 @@ children.0=childs0 \
3. On Secondary VM's QEMU monitor, issue command
{"execute":"qmp_capabilities"}
+{"execute": "migrate-set-capabilities", "arguments": {"capabilities": [
{"capability": "x-colo", "state": true } ] } }
{"execute": "nbd-server-start", "arguments": {"addr": {"type": "inet", "data":
{"host": "0.0.0.0", "port": ""} } } }
{"execute": "nbd-server-add", "arguments": {"device": "parent0", "writable":
true } }
diff --git a/migration/migration.c b/migration/migration.c
index bb254e4f07..439e8651df 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -398,6 +398,12 @@ int migration_incoming_enable_colo(void)
return -ENOTSUP;
#endif
+if (!migrate_colo()) {
+error_report("ENABLE_COLO command come in migration stream, but c-colo
"
+ "capability is not set");
+return -EINVAL;
+}
+
if (ram_block_discard_disable(true)) {
error_report("COLO: cannot disable RAM discard");
return -EBUSY;
--
2.40.1
[PULL 02/10] ram: Let colo_flush_ram_cache take the bitmap_mutex
From: Lukas Straub
This is not required, colo_flush_ram_cache does not run concurrently
with the multifd threads since the cache is only flushed after
everything has been received. But it makes me more comfortable.
This will be used in the next commits to add colo support to multifd.
Signed-off-by: Lukas Straub
Reviewed-by: Juan Quintela
Message-Id:
<35cb23ba854151d38a31e3a5c8a1020e4283cb4a.1683572883.git.lukasstra...@web.de>
Signed-off-by: Juan Quintela
---
migration/ram.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/migration/ram.c b/migration/ram.c
index b5d03f85ab..f69d8d42b0 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3814,6 +3814,7 @@ void colo_flush_ram_cache(void)
unsigned long offset = 0;
memory_global_dirty_log_sync();
+qemu_mutex_lock(_state->bitmap_mutex);
WITH_RCU_READ_LOCK_GUARD() {
RAMBLOCK_FOREACH_NOT_IGNORED(block) {
ramblock_sync_dirty_bitmap(ram_state, block);
@@ -3848,6 +3849,7 @@ void colo_flush_ram_cache(void)
}
}
}
+qemu_mutex_unlock(_state->bitmap_mutex);
trace_colo_flush_ram_cache_end();
}
--
2.40.1
[PULL 07/10] migration: drop colo_incoming_thread from MigrationIncomingState
From: Vladimir Sementsov-Ogievskiy
have_colo_incoming_thread variable is unused. colo_incoming_thread can
be local.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Juan Quintela
Reviewed-by: Peter Xu
Reviewed-by: Zhang Chen
Message-Id: <20230428194928.1426370-6-vsement...@yandex-team.ru>
Signed-off-by: Juan Quintela
---
migration/migration.h | 2 --
migration/migration.c | 7 ---
2 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/migration/migration.h b/migration/migration.h
index 3a918514e7..7721c7658b 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -162,8 +162,6 @@ struct MigrationIncomingState {
int state;
-bool have_colo_incoming_thread;
-QemuThread colo_incoming_thread;
/* The coroutine we should enter (back) after failover */
Coroutine *migration_incoming_co;
QemuSemaphore colo_incoming_sem;
diff --git a/migration/migration.c b/migration/migration.c
index 61b316245d..c7f628caa6 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -544,6 +544,8 @@ process_incoming_migration_co(void *opaque)
/* we get COLO info, and know if we are in COLO mode */
if (!ret && migration_incoming_colo_enabled()) {
+QemuThread colo_incoming_thread;
+
/* Make sure all file formats throw away their mutable metadata */
bdrv_activate_all(_err);
if (local_err) {
@@ -551,14 +553,13 @@ process_incoming_migration_co(void *opaque)
goto fail;
}
-qemu_thread_create(>colo_incoming_thread, "COLO incoming",
+qemu_thread_create(_incoming_thread, "COLO incoming",
colo_process_incoming_thread, mis, QEMU_THREAD_JOINABLE);
-mis->have_colo_incoming_thread = true;
qemu_coroutine_yield();
qemu_mutex_unlock_iothread();
/* Wait checkpoint incoming thread exit before free resource */
-qemu_thread_join(>colo_incoming_thread);
+qemu_thread_join(_incoming_thread);
qemu_mutex_lock_iothread();
/* We hold the global iothread lock, so it is safe here */
colo_release_ram_cache();
--
2.40.1
[PULL 06/10] build: move COLO under CONFIG_REPLICATION
From: Vladimir Sementsov-Ogievskiy
We don't allow to use x-colo capability when replication is not
configured. So, no reason to build COLO when replication is disabled,
it's unusable in this case.
Note also that the check in migrate_caps_check() is not the only
restriction: some functions in migration/colo.c will just abort if
called with not defined CONFIG_REPLICATION, for example:
migration_iteration_finish()
case MIGRATION_STATUS_COLO:
migrate_start_colo_process()
colo_process_checkpoint()
abort()
It could probably make sense to have possibility to enable COLO without
REPLICATION, but this requires deeper audit of colo & replication code,
which may be done later if needed.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Acked-by: Dr. David Alan Gilbert
Reviewed-by: Juan Quintela
Message-Id: <20230428194928.1426370-4-vsement...@yandex-team.ru>
Signed-off-by: Juan Quintela
---
qapi/migration.json| 9 +---
migration/colo.c | 28
migration/migration-hmp-cmds.c | 2 ++
migration/migration.c | 6 ++
stubs/colo.c | 39 ++
hmp-commands.hx| 2 ++
migration/meson.build | 6 --
stubs/meson.build | 1 +
8 files changed, 60 insertions(+), 33 deletions(-)
create mode 100644 stubs/colo.c
diff --git a/qapi/migration.json b/qapi/migration.json
index 952d3e2c9a..179af0c4d8 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1366,7 +1366,8 @@
# <- { "return": {} }
##
{ 'command': 'x-colo-lost-heartbeat',
- 'features': [ 'unstable' ] }
+ 'features': [ 'unstable' ],
+ 'if': 'CONFIG_REPLICATION' }
##
# @migrate_cancel:
@@ -1638,7 +1639,8 @@
##
{ 'struct': 'COLOStatus',
'data': { 'mode': 'COLOMode', 'last-mode': 'COLOMode',
-'reason': 'COLOExitReason' } }
+'reason': 'COLOExitReason' },
+ 'if': 'CONFIG_REPLICATION' }
##
# @query-colo-status:
@@ -1655,7 +1657,8 @@
# Since: 3.1
##
{ 'command': 'query-colo-status',
- 'returns': 'COLOStatus' }
+ 'returns': 'COLOStatus',
+ 'if': 'CONFIG_REPLICATION' }
##
# @migrate-recover:
diff --git a/migration/colo.c b/migration/colo.c
index c9e0b909b9..6c7c313956 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -26,9 +26,7 @@
#include "qemu/rcu.h"
#include "migration/failover.h"
#include "migration/ram.h"
-#ifdef CONFIG_REPLICATION
#include "block/replication.h"
-#endif
#include "net/colo-compare.h"
#include "net/colo.h"
#include "block/block.h"
@@ -86,7 +84,6 @@ void colo_checkpoint_delay_set(void)
static void secondary_vm_do_failover(void)
{
/* COLO needs enable block-replication */
-#ifdef CONFIG_REPLICATION
int old_state;
MigrationIncomingState *mis = migration_incoming_get_current();
Error *local_err = NULL;
@@ -151,14 +148,10 @@ static void secondary_vm_do_failover(void)
if (mis->migration_incoming_co) {
qemu_coroutine_enter(mis->migration_incoming_co);
}
-#else
-abort();
-#endif
}
static void primary_vm_do_failover(void)
{
-#ifdef CONFIG_REPLICATION
MigrationState *s = migrate_get_current();
int old_state;
Error *local_err = NULL;
@@ -199,9 +192,6 @@ static void primary_vm_do_failover(void)
/* Notify COLO thread that failover work is finished */
qemu_sem_post(>colo_exit_sem);
-#else
-abort();
-#endif
}
COLOMode get_colo_mode(void)
@@ -235,7 +225,6 @@ void colo_do_failover(void)
}
}
-#ifdef CONFIG_REPLICATION
void qmp_xen_set_replication(bool enable, bool primary,
bool has_failover, bool failover,
Error **errp)
@@ -289,7 +278,6 @@ void qmp_xen_colo_do_checkpoint(Error **errp)
/* Notify all filters of all NIC to do checkpoint */
colo_notify_filters_event(COLO_EVENT_CHECKPOINT, errp);
}
-#endif
COLOStatus *qmp_query_colo_status(Error **errp)
{
@@ -453,15 +441,11 @@ static int colo_do_checkpoint_transaction(MigrationState
*s,
}
qemu_mutex_lock_iothread();
-#ifdef CONFIG_REPLICATION
replication_do_checkpoint_all(_err);
if (local_err) {
qemu_mutex_unlock_iothread();
goto out;
}
-#else
-abort();
-#endif
colo_send_message(s->to_dst_file, COLO_MESSAGE_VMSTATE_SEND, _err);
if (local_err) {
@@ -579,15 +563,11 @@ static void colo_process_checkpoint(MigrationState *s)
object_unref(OBJECT(bioc));
qemu_mutex_lock_iothread();
-#ifdef CONFIG_REPLICATION
replication_start_all(REPLICATION_MODE_PRIMARY, _err);
if (local_err) {
qemu_mutex_unlock_iothread();
goto out;
}
-#else
-abort();
-#endif
vm_start();
qemu_mutex_unlock_iothread();
@@ -755,7 +735,6 @@ static void
colo_incoming_process_checkpoint(MigrationIncomingState *mis,
return;
}
-#ifdef CONFIG_REPLICATION
[PULL 04/10] block/meson.build: prefer positive condition for replication
From: Vladimir Sementsov-Ogievskiy
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Juan Quintela
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Lukas Straub
Reviewed-by: Zhang Chen
Message-Id: <20230428194928.1426370-2-vsement...@yandex-team.ru>
Signed-off-by: Juan Quintela
---
block/meson.build | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/meson.build b/block/meson.build
index 13337bd070..486dda8b85 100644
--- a/block/meson.build
+++ b/block/meson.build
@@ -92,7 +92,7 @@ block_ss.add(when: 'CONFIG_WIN32', if_true:
files('file-win32.c', 'win32-aio.c')
block_ss.add(when: 'CONFIG_POSIX', if_true: [files('file-posix.c'), coref,
iokit])
block_ss.add(when: libiscsi, if_true: files('iscsi-opts.c'))
block_ss.add(when: 'CONFIG_LINUX', if_true: files('nvme.c'))
-if not get_option('replication').disabled()
+if get_option('replication').allowed()
block_ss.add(files('replication.c'))
endif
block_ss.add(when: libaio, if_true: files('linux-aio.c'))
--
2.40.1
[PULL 03/10] multifd: Add the ramblock to MultiFDRecvParams
From: Lukas Straub
This will be used in the next commits to add colo support to multifd.
Signed-off-by: Lukas Straub
Reviewed-by: Juan Quintela
Message-Id:
<88135197411df1a71d7832962b39abf60faf0021.1683572883.git.lukasstra...@web.de>
Signed-off-by: Juan Quintela
---
migration/multifd.h | 2 ++
migration/multifd.c | 11 +--
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/migration/multifd.h b/migration/multifd.h
index 7cfc265148..a835643b48 100644
--- a/migration/multifd.h
+++ b/migration/multifd.h
@@ -175,6 +175,8 @@ typedef struct {
uint32_t next_packet_size;
/* packets sent through this channel */
uint64_t num_packets;
+/* ramblock */
+RAMBlock *block;
/* ramblock host address */
uint8_t *host;
/* non zero pages recv through this channel */
diff --git a/migration/multifd.c b/migration/multifd.c
index 4e71c19292..5c4298eadf 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -281,7 +281,6 @@ static void multifd_send_fill_packet(MultiFDSendParams *p)
static int multifd_recv_unfill_packet(MultiFDRecvParams *p, Error **errp)
{
MultiFDPacket_t *packet = p->packet;
-RAMBlock *block;
int i;
packet->magic = be32_to_cpu(packet->magic);
@@ -331,21 +330,21 @@ static int multifd_recv_unfill_packet(MultiFDRecvParams
*p, Error **errp)
/* make sure that ramblock is 0 terminated */
packet->ramblock[255] = 0;
-block = qemu_ram_block_by_name(packet->ramblock);
-if (!block) {
+p->block = qemu_ram_block_by_name(packet->ramblock);
+if (!p->block) {
error_setg(errp, "multifd: unknown ram block %s",
packet->ramblock);
return -1;
}
-p->host = block->host;
+p->host = p->block->host;
for (i = 0; i < p->normal_num; i++) {
uint64_t offset = be64_to_cpu(packet->offset[i]);
-if (offset > (block->used_length - p->page_size)) {
+if (offset > (p->block->used_length - p->page_size)) {
error_setg(errp, "multifd: offset too long %" PRIu64
" (max " RAM_ADDR_FMT ")",
- offset, block->used_length);
+ offset, p->block->used_length);
return -1;
}
p->normal[i] = offset;
--
2.40.1
[PULL 05/10] colo: make colo_checkpoint_notify static and provide simpler API
From: Vladimir Sementsov-Ogievskiy
colo_checkpoint_notify() is mostly used in colo.c. Outside we use it
once when x-checkpoint-delay migration parameter is set. So, let's
simplify the external API to only that function - notify COLO that
parameter was set. This make external API more robust and hides
implementation details from external callers. Also this helps us to
make COLO module optional in further patch (i.e. we are going to add
possibility not build the COLO module).
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Juan Quintela
Reviewed-by: Peter Xu
Reviewed-by: Zhang Chen
Message-Id: <20230428194928.1426370-3-vsement...@yandex-team.ru>
Signed-off-by: Juan Quintela
---
include/migration/colo.h | 9 -
migration/colo.c | 29 ++---
migration/options.c | 4 +---
3 files changed, 27 insertions(+), 15 deletions(-)
diff --git a/include/migration/colo.h b/include/migration/colo.h
index 5fbe1a6d5d..7ef315473e 100644
--- a/include/migration/colo.h
+++ b/include/migration/colo.h
@@ -36,6 +36,13 @@ COLOMode get_colo_mode(void);
/* failover */
void colo_do_failover(void);
-void colo_checkpoint_notify(void *opaque);
+/*
+ * colo_checkpoint_delay_set
+ *
+ * Handles change of x-checkpoint-delay migration parameter, called from
+ * migrate_params_apply() to notify COLO module about the change.
+ */
+void colo_checkpoint_delay_set(void);
+
void colo_shutdown(void);
#endif
diff --git a/migration/colo.c b/migration/colo.c
index 07bfa21fea..c9e0b909b9 100644
--- a/migration/colo.c
+++ b/migration/colo.c
@@ -65,6 +65,24 @@ static bool colo_runstate_is_stopped(void)
return runstate_check(RUN_STATE_COLO) || !runstate_is_running();
}
+static void colo_checkpoint_notify(void *opaque)
+{
+MigrationState *s = opaque;
+int64_t next_notify_time;
+
+qemu_event_set(>colo_checkpoint_event);
+s->colo_checkpoint_time = qemu_clock_get_ms(QEMU_CLOCK_HOST);
+next_notify_time = s->colo_checkpoint_time + migrate_checkpoint_delay();
+timer_mod(s->colo_delay_timer, next_notify_time);
+}
+
+void colo_checkpoint_delay_set(void)
+{
+if (migration_in_colo_state()) {
+colo_checkpoint_notify(migrate_get_current());
+}
+}
+
static void secondary_vm_do_failover(void)
{
/* COLO needs enable block-replication */
@@ -644,17 +662,6 @@ out:
}
}
-void colo_checkpoint_notify(void *opaque)
-{
-MigrationState *s = opaque;
-int64_t next_notify_time;
-
-qemu_event_set(>colo_checkpoint_event);
-s->colo_checkpoint_time = qemu_clock_get_ms(QEMU_CLOCK_HOST);
-next_notify_time = s->colo_checkpoint_time + migrate_checkpoint_delay();
-timer_mod(s->colo_delay_timer, next_notify_time);
-}
-
void migrate_start_colo_process(MigrationState *s)
{
qemu_mutex_unlock_iothread();
diff --git a/migration/options.c b/migration/options.c
index 2e759cc306..9d92b15b76 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -1253,9 +1253,7 @@ static void migrate_params_apply(MigrateSetParameters
*params, Error **errp)
if (params->has_x_checkpoint_delay) {
s->parameters.x_checkpoint_delay = params->x_checkpoint_delay;
-if (migration_in_colo_state()) {
-colo_checkpoint_notify(s);
-}
+colo_checkpoint_delay_set();
}
if (params->has_block_incremental) {
--
2.40.1
[PULL 09/10] migration: disallow change capabilities in COLO state
From: Vladimir Sementsov-Ogievskiy
COLO is not listed as running state in migrate_is_running(), so, it's
theoretically possible to disable colo capability in COLO state and the
unexpected error in migration_iteration_finish() is reachable.
Let's disallow that in qmp_migrate_set_capabilities. Than the error
becomes absolutely unreachable: we can get into COLO state only with
enabled capability and can't disable it while we are in COLO state. So
substitute the error by simple assertion.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Juan Quintela
Reviewed-by: Peter Xu
Message-Id: <20230428194928.1426370-10-vsement...@yandex-team.ru>
Signed-off-by: Juan Quintela
---
migration/migration.c | 5 +
migration/options.c | 2 +-
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 140b2a4de6..bb254e4f07 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2785,10 +2785,7 @@ static void migration_iteration_finish(MigrationState *s)
runstate_set(RUN_STATE_POSTMIGRATE);
break;
case MIGRATION_STATUS_COLO:
-if (!migrate_colo()) {
-error_report("%s: critical error: calling COLO code without "
- "COLO enabled", __func__);
-}
+assert(migrate_colo());
migrate_start_colo_process(s);
s->vm_was_running = true;
/* Fallthrough */
diff --git a/migration/options.c b/migration/options.c
index 9d92b15b76..7ed88b7b32 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -598,7 +598,7 @@ void
qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
MigrationCapabilityStatusList *cap;
bool new_caps[MIGRATION_CAPABILITY__MAX];
-if (migration_is_running(s->state)) {
+if (migration_is_running(s->state) || migration_in_colo_state()) {
error_setg(errp, QERR_MIGRATION_ACTIVE);
return;
}
--
2.40.1
[PULL 08/10] migration: process_incoming_migration_co: simplify code flow around ret
From: Vladimir Sementsov-Ogievskiy
Signed-off-by: Vladimir Sementsov-Ogievskiy
Reviewed-by: Juan Quintela
Reviewed-by: Peter Xu
Reviewed-by: Zhang Chen
Message-Id: <20230428194928.1426370-7-vsement...@yandex-team.ru>
Signed-off-by: Juan Quintela
---
migration/migration.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index c7f628caa6..140b2a4de6 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -542,8 +542,13 @@ process_incoming_migration_co(void *opaque)
/* Else if something went wrong then just fall out of the normal exit
*/
}
+if (ret < 0) {
+error_report("load of migration failed: %s", strerror(-ret));
+goto fail;
+}
+
/* we get COLO info, and know if we are in COLO mode */
-if (!ret && migration_incoming_colo_enabled()) {
+if (migration_incoming_colo_enabled()) {
QemuThread colo_incoming_thread;
/* Make sure all file formats throw away their mutable metadata */
@@ -565,10 +570,6 @@ process_incoming_migration_co(void *opaque)
colo_release_ram_cache();
}
-if (ret < 0) {
-error_report("load of migration failed: %s", strerror(-ret));
-goto fail;
-}
mis->bh = qemu_bh_new(process_incoming_migration_bh, mis);
qemu_bh_schedule(mis->bh);
mis->migration_incoming_co = NULL;
--
2.40.1
[PULL 00/10] Migration 20230509 patches
The following changes since commit caa9cbd566877b34e9abcc04d936116fc5e0ab28: Merge tag 'for-upstream' of https://repo.or.cz/qemu/kevin into staging (2023-05-10 14:52:03 +0100) are available in the Git repository at: https://gitlab.com/juan.quintela/qemu.git tags/migration-20230509-pull-request for you to fetch changes up to 121ccedc2bf0c124e93991275336415d12d2e3df: migration: block incoming colo when capability is disabled (2023-05-10 18:48:12 +0200) Migration Pull request (20230509 vintage) take 2 Hi In this take 2: - Change uint -> uint32_t to fix mingw32 compilation. Please apply. [take 1] In this PULL request: - 1st part of colo support for multifd (lukas) - 1st part of disabling colo option (vladimir) Please, apply. Lukas Straub (3): ram: Add public helper to set colo bitmap ram: Let colo_flush_ram_cache take the bitmap_mutex multifd: Add the ramblock to MultiFDRecvParams Vladimir Sementsov-Ogievskiy (7): block/meson.build: prefer positive condition for replication colo: make colo_checkpoint_notify static and provide simpler API build: move COLO under CONFIG_REPLICATION migration: drop colo_incoming_thread from MigrationIncomingState migration: process_incoming_migration_co: simplify code flow around ret migration: disallow change capabilities in COLO state migration: block incoming colo when capability is disabled docs/COLO-FT.txt | 1 + qapi/migration.json| 9 -- include/migration/colo.h | 9 +- migration/migration.h | 2 -- migration/multifd.h| 2 ++ migration/ram.h| 1 + migration/colo.c | 57 +++--- migration/migration-hmp-cmds.c | 2 ++ migration/migration.c | 35 ++--- migration/multifd.c| 11 +++ migration/options.c| 6 ++-- migration/ram.c| 19 ++-- stubs/colo.c | 39 +++ block/meson.build | 2 +- hmp-commands.hx| 2 ++ migration/meson.build | 6 ++-- stubs/meson.build | 1 + 17 files changed, 131 insertions(+), 73 deletions(-) create mode 100644 stubs/colo.c -- 2.40.1
[PULL 01/10] ram: Add public helper to set colo bitmap
From: Lukas Straub
The overhead of the mutex in non-multifd mode is negligible,
because in that case its just the single thread taking the mutex.
This will be used in the next commits to add colo support to multifd.
Signed-off-by: Lukas Straub
Reviewed-by: Juan Quintela
Message-Id:
<22d83cb428f37929563155531bfb69fd8953cc61.1683572883.git.lukasstra...@web.de>
Signed-off-by: Juan Quintela
---
migration/ram.h | 1 +
migration/ram.c | 17 ++---
2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/migration/ram.h b/migration/ram.h
index 6fffbeb5f1..ea1f3c25b5 100644
--- a/migration/ram.h
+++ b/migration/ram.h
@@ -82,6 +82,7 @@ int colo_init_ram_cache(void);
void colo_flush_ram_cache(void);
void colo_release_ram_cache(void);
void colo_incoming_start_dirty_log(void);
+void colo_record_bitmap(RAMBlock *block, ram_addr_t *normal, uint32_t pages);
/* Background snapshot */
bool ram_write_tracking_available(void);
diff --git a/migration/ram.c b/migration/ram.c
index f78e9912cd..b5d03f85ab 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3408,6 +3408,18 @@ static ram_addr_t
host_page_offset_from_ram_block_offset(RAMBlock *block,
return ((uintptr_t)block->host + offset) & (block->page_size - 1);
}
+void colo_record_bitmap(RAMBlock *block, ram_addr_t *normal, uint32_t pages)
+{
+qemu_mutex_lock(_state->bitmap_mutex);
+for (int i = 0; i < pages; i++) {
+ram_addr_t offset = normal[i];
+ram_state->migration_dirty_pages += !test_and_set_bit(
+offset >> TARGET_PAGE_BITS,
+block->bmap);
+}
+qemu_mutex_unlock(_state->bitmap_mutex);
+}
+
static inline void *colo_cache_from_block_offset(RAMBlock *block,
ram_addr_t offset, bool record_bitmap)
{
@@ -3425,9 +3437,8 @@ static inline void *colo_cache_from_block_offset(RAMBlock
*block,
* It help us to decide which pages in ram cache should be flushed
* into VM's RAM later.
*/
-if (record_bitmap &&
-!test_and_set_bit(offset >> TARGET_PAGE_BITS, block->bmap)) {
-ram_state->migration_dirty_pages++;
+if (record_bitmap) {
+colo_record_bitmap(block, , 1);
}
return block->colo_cache + offset;
}
--
2.40.1
Re: [PATCH v2 4/4] migration/calc-dirty-rate: tool to predict migration time
Andrei Gudkov wrote: > Signed-off-by: Andrei Gudkov my python is very rusty, so I will let someone else to comment here. Later, Juan.
Re: [PATCH v2 3/4] migration/calc-dirty-rate: added n-zero-pages metric
Andrei Gudkov wrote:
> In sampling mode, a new metric is collected and reported:
> number of pages entirely filled with zeroes.
Good idea.
> @@ -331,11 +336,20 @@ static uint32_t compute_page_hash(void *ptr)
> v2 = QEMU_XXHASH_SEED + XXH_PRIME64_2;
> v3 = QEMU_XXHASH_SEED + 0;
> v4 = QEMU_XXHASH_SEED - XXH_PRIME64_1;
> -for (i = 0; i < TARGET_PAGE_SIZE / 8; i += 4) {
> -v1 = XXH64_round(v1, p[i + 0]);
> -v2 = XXH64_round(v2, p[i + 1]);
> -v3 = XXH64_round(v3, p[i + 2]);
> -v4 = XXH64_round(v4, p[i + 3]);
> +if (ptr) {
It smells like a hack, that is only going to be used once in the
program.
But the only other option that I can think is repeating the function for
the zero case.
No way to win here.
> +for (i = 0; i < TARGET_PAGE_SIZE / 8; i += 4) {
> +v1 = XXH64_round(v1, p[i + 0]);
> +v2 = XXH64_round(v2, p[i + 1]);
> +v3 = XXH64_round(v3, p[i + 2]);
> +v4 = XXH64_round(v4, p[i + 3]);
> +}
> +} else {
> +for (i = 0; i < TARGET_PAGE_SIZE / 8; i += 4) {
> +v1 = XXH64_round(v1, 0);
> +v2 = XXH64_round(v2, 0);
> +v3 = XXH64_round(v3, 0);
> +v4 = XXH64_round(v4, 0);
> +}
> }
> res = XXH64_mergerounds(v1, v2, v3, v4);
> res += TARGET_PAGE_SIZE;
> @@ -343,6 +357,17 @@ static uint32_t compute_page_hash(void *ptr)
> return (uint32_t)(res & UINT32_MAX);
> }
>
> +static uint32_t get_zero_page_hash(void)
> +{
> +static uint32_t hash;
> +static int is_computed;
bool?
Later, Juan.
Re: [PATCH v2 3/5] hw/display/virtio-gpu-virgl: define callbacks in realize function
Am 10. Mai 2023 07:56:15 UTC schrieb "Philippe Mathieu-Daudé"
:
>On 30/4/23 23:48, Bernhard Beschow wrote:
>>
>>
>> Am 28. April 2023 16:48:21 UTC schrieb Gurchetan Singh
>> :
>>> From: Gurchetan Singh
>>>
>>> This reduces the amount of renderer backend specific needed to
>>> be exposed to the GL device. We only need one realize function
>>> per renderer backend.
>>>
>>> Signed-off-by: Gurchetan Singh
>>> Reviewed-by: Philippe Mathieu-Daudé
>>> ---
>>> v1: - Remove NULL inits (Philippe)
>>> - Use VIRTIO_GPU_BASE where possible (Philippe)
>>> v2: - Fix unnecessary line break (Akihiko)
>>>
>>> hw/display/virtio-gpu-gl.c | 15 ++-
>>> hw/display/virtio-gpu-virgl.c | 35 --
>>> include/hw/virtio/virtio-gpu.h | 7 ---
>>> 3 files changed, 31 insertions(+), 26 deletions(-)
>
>
>>> void virtio_gpu_virgl_device_realize(DeviceState *qdev, Error **errp)
>>> {
>>> -VirtIOGPU *g = VIRTIO_GPU(qdev);
>>> +VirtIODevice *vdev = VIRTIO_DEVICE(qdev);
>>> +VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
>>> +
>>> +VirtIOGPUBase *bdev = VIRTIO_GPU_BASE(qdev);
>>> +VirtIOGPUBaseClass *vbc = VIRTIO_GPU_BASE_GET_CLASS(bdev);
>>> +
>>> +VirtIOGPU *gpudev = VIRTIO_GPU(qdev);
>>> +VirtIOGPUClass *vgc = VIRTIO_GPU_GET_CLASS(gpudev);
>>> +
>>> +vbc->gl_flushed = virtio_gpu_virgl_flushed;
>>> +vgc->handle_ctrl = virtio_gpu_virgl_handle_ctrl;
>>> +vgc->process_cmd = virtio_gpu_virgl_process_cmd;
>>> +vgc->update_cursor_data = virtio_gpu_virgl_update_cursor;
>>> +
>>> +vdc->reset = virtio_gpu_virgl_reset;
>>
>> A realize method is supposed to modify a single instance only while we're
>> modifying the behavior of whole classes here, i.e. will affect every
>> instance of these classes. This goes against QOM design principles and will
>> therefore be confusing for people who are familiar with QOM in particular
>> and OOP in general. I think the code should be cleaned up in a different way
>> if really needed.
>
>Doh I was too quick and totally missed this was an instance,
>thanks for being careful Bernhard!
My obligation ;)
I wonder if *_GET_CLASS() macros could return const pointers in order for the
compiler to catch such uses? Are there use cases at all in retrieving non-const
class pointers from instances?
Best regards,
Bernhard
Re: [PATCH v2 2/4] migration/calc-dirty-rate: detailed stats in sampling mode
Andrei Gudkov wrote:
> Collect number of dirty pages for progresseively increasing time
> periods starting with 125ms up to number of seconds specified with
> calc-dirty-rate. Report through qmp and hmp: 1) vector of dirty page
> measurements, 2) page size, 3) total number of VM pages, 4) number
> of sampled pages.
>
> Signed-off-by: Andrei Gudkov
> ---
> migration/dirtyrate.c | 165 +-
> migration/dirtyrate.h | 25 ++-
> qapi/migration.json | 24 +-
You need the equivalent of this in your .git/config file.
[diff]
orderFile = scripts/git.orderfile
In particular:
*json files cames first
*.h second
*.c third
> 3 files changed, 160 insertions(+), 54 deletions(-)
>
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index acba3213a3..4491bbe91a 100644
> --- a/migration/dirtyrate.c
> +++ b/migration/dirtyrate.c
> @@ -224,6 +224,7 @@ static struct DirtyRateInfo *query_dirty_rate_info(void)
> info->calc_time = DirtyStat.calc_time;
> info->sample_pages = DirtyStat.sample_pages;
> info->mode = dirtyrate_mode;
> +info->page_size = TARGET_PAGE_SIZE;
I thought we exported this trough ""info migrate"
but we do it only if we are in the middle of a migration. Perhaps we
should print it always.
> if (qatomic_read() == DIRTY_RATE_STATUS_MEASURED) {
> info->has_dirty_rate = true;
> @@ -245,6 +246,29 @@ static struct DirtyRateInfo *query_dirty_rate_info(void)
> info->vcpu_dirty_rate = head;
> }
>
> +if (dirtyrate_mode == DIRTY_RATE_MEASURE_MODE_PAGE_SAMPLING)
> {
see my comment at the end about int64 vs uint64/size
> +DirtyStat.page_sampling.n_total_pages = 0;
> +DirtyStat.page_sampling.n_sampled_pages = 0;
> +DirtyStat.page_sampling.n_readings = 0;
> +DirtyStat.page_sampling.readings =
> g_try_malloc0_n(MAX_DIRTY_READINGS,
> +
> sizeof(DirtyReading));
> break;
You do g_try_malloc0()
or you check for NULL return.
Even better, I think you can use here:
foo = g_new0(DirtyReading, MAX_DIRTY_READINGS);
MAX_DIRTY_READINGS is small enough that you can assume that there is
enough free memory.
> -DirtyStat.dirty_rate = dirtyrate;
> +if (DirtyStat.page_sampling.readings) {
> +free(DirtyStat.page_sampling.readings);
> +DirtyStat.page_sampling.readings = NULL;
> +}
What glib gives, glib takes.
g_malloc() -> g_free()
g_free() knows how to handle the NULL case so:
g_free(DirtyStat.page_sampling.readings);
DirtyStat.page_sampling.readings = NULL;
Without if should be good enough.
> -static bool compare_page_hash_info(struct RamblockDirtyInfo *info,
> +static int64_t compare_page_hash_info(struct RamblockDirtyInfo *info,
>int block_count)
bad indentantion.
> +static int64_t increase_period(int64_t prev_period, int64_t max_period)
> +{
> +int64_t delta;
> +int64_t next_period;
> +
> +if (prev_period < 500) {
> +delta = 125;
> +} else if (prev_period < 1000) {
> +delta = 250;
> +} else if (prev_period < 2000) {
> +delta = 500;
> +} else if (prev_period < 4000) {
> +delta = 1000;
> +} else if (prev_period < 1) {
> +delta = 2000;
> +} else {
> +delta = 5000;
> +}
> +
> +next_period = prev_period + delta;
> +if (next_period + delta >= max_period) {
> +next_period = max_period;
> +}
> +return next_period;
> +}
Is there any reason for this to be so complicated?
int64_t periods[] = { 125, 250, 375, 500, 750, 1000, 1500, 2000, 3000, 4000,
6000, 8000, 1,
15000, 2, 25000, 3, 35000, 4, 45000 };
And access it in the loop? This way you get what the values are.
You can put a limit to config.sample_period_seconds, or you want it
unlimited?
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 2c35b7b9cf..f818f51e0e 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -1805,6 +1805,22 @@
> # @vcpu-dirty-rate: dirtyrate for each vcpu if dirty-ring
> # mode specified (Since 6.2)
> #
> +# @page-size: page size in bytes (since 8.1)
> +#
> +# @n-total-pages: [page-sampling] total number of VM pages (since 8.1)
> +#
> +# @n-sampled-pages: [page-sampling] number of sampled VM pages (since 8.1)
> +#
> +# @periods: [page-sampling] array of time periods expressed in milliseconds
> +# for which dirty-sample measurements were collected (since 8.1)
> +#
> +# @n-dirty-pages: [page-sampling] number of pages among all sampled pages
> +# that were observed as changed during respective time
> period.
> +# i-th element of this array corresponds to the i-th element
> +# of the @periods array, i.e. @n-dirty-pages[i] is the number
> +#
Re: [PULL 86/89] target/riscv: Restore the predicate() NULL check behavior
08.05.2023 01:21, Alistair Francis wrote: On Fri, May 5, 2023 at 11:08 AM Alistair Francis wrote: From: Bin Meng When reading a non-existent CSR QEMU should raise illegal instruction exception, but currently it just exits due to the g_assert() check. This actually reverts commit 0ee342256af9205e7388efdf193a6d8f1ba1a617. Some comments are also added to indicate that predicate() must be provided for an implemented CSR. Reported-by: Fei Wu Signed-off-by: Bin Meng Reviewed-by: Daniel Henrique Barboza Reviewed-by: Weiwei Li Reviewed-by: Alistair Francis Reviewed-by: LIU Zhiwei Message-Id: <20230417043054.3125614-1-bm...@tinylab.org> Signed-off-by: Alistair Francis Sorry, I didn't realise I should have done this with the PR, but this is a good candidate for going into 8.0.1 Queued for 8.0, with minor context tweak. Thank you! /mjt
Re: [PULL 19/89] target/riscv: Fix itrigger when icount is used
08.05.2023 01:22, Alistair Francis wrote: On Fri, May 5, 2023 at 11:04 AM Alistair Francis wrote: From: LIU Zhiwei When I boot a ubuntu image, QEMU output a "Bad icount read" message and exit. The reason is that when execute helper_mret or helper_sret, it will cause a call to icount_get_raw_locked (), which needs set can_do_io flag on cpustate. Thus we setting this flag when execute these two instructions. Signed-off-by: LIU Zhiwei Reviewed-by: Weiwei Li Acked-by: Alistair Francis Message-Id: <20230324064011.976-1-zhiwei_...@linux.alibaba.com> Signed-off-by: Alistair Francis This is also a good candidate for 8.0.1 Queued for 8.0 (and for 7.2 if it will ever be). Thanks! /mjt
Re: [PULL 0/8] testing and misc (docker, docs, ci scripts, gitlab, avocado, Kconfig)
On 5/10/23 16:06, Alex Bennée wrote: The following changes since commit 568992e3440f11897e209bf676aa5b93251385fa: Merge tag 'pull-qapi-2023-05-09-v2' of https://repo.or.cz/qemu/armbru into staging (2023-05-10 13:11:29 +0100) are available in the Git repository at: https://gitlab.com/stsquad/qemu.git tags/pull-testing-updates-100523-1 for you to fetch changes up to b9353acfd7ae1fc59a64b9aea34bd77a415751d1: hw/arm: Select XLNX_USB_SUBSYS for xlnx-zcu102 machine (2023-05-10 16:02:58 +0100) Testing updates: - fix up xtensa docker container base to current Debian - document breakpoint and watchpoint support - clean up the ansible scripts for Ubuntu 22.04 - add a minimal device profile - drop https on mipsdistros URL - fix Kconfig bug for XLNX_VERSAL Applied, thanks. Please update https://wiki.qemu.org/ChangeLog/8.1 as appropriate. r~
[PATCH] disas: Fix tabs and braces in disas.c
Fix these before moving the file, for checkpatch.pl.
Signed-off-by: Richard Henderson
---
disas.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/disas.c b/disas.c
index b087c12c47..d46f638a72 100644
--- a/disas.c
+++ b/disas.c
@@ -226,11 +226,12 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong
code,
}
for (pc = code; size > 0; pc += count, size -= count) {
- fprintf(out, "0x" TARGET_FMT_lx ": ", pc);
- count = s.info.print_insn(pc, );
- fprintf(out, "\n");
- if (count < 0)
- break;
+fprintf(out, "0x" TARGET_FMT_lx ": ", pc);
+count = s.info.print_insn(pc, );
+fprintf(out, "\n");
+if (count < 0) {
+break;
+}
if (size < count) {
fprintf(out,
"Disassembler disagrees with translator over instruction "
--
2.34.1
Re: [PATCH v2 1/4] migration/calc-dirty-rate: replaced CRC32 with xxHash
Andrei Gudkov wrote: > This significantly reduces overhead of dirty page > rate calculation in sampling mode. > Tested using 32GiB VM on E5-2690 CPU. > > With CRC32: > total_pages=8388608 sampled_pages=16384 millis=71 > > With xxHash: > total_pages=8388608 sampled_pages=16384 millis=14 > > Signed-off-by: Andrei Gudkov Reviewed-by: Juan Quintela This changes nothing outside of dirtyrate, and if the hash function is faster, we should be good. queued.
Re: [PATCH] tcg: round-robin: do not use mb_read for rr_current_cpu
On 5/10/23 17:04, Paolo Bonzini wrote: Note that qatomic_mb_set can remain, similar to how Linux has smp_store_mb (an optimized version of following a store with a full memory barrier). Signed-off-by: Paolo Bonzini --- accel/tcg/tcg-accel-ops-rr.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v1 1/1] hw/pci: Disable PCI_ERR_UNCOR_MASK register for machine type < 8.0
Michael Tokarev wrote:
> 03.05.2023 03:27, Leonardo Bras пишет:
>> Since it's implementation on v8.0.0-rc0, having the PCI_ERR_UNCOR_MASK
>> set for machine types < 8.0 will cause migration to fail if the target
>> QEMU version is < 8.0.0 :
>> qemu-system-x86_64: get_pci_config_device: Bad config data: i=0x10a
>> read: 40 device: 0 cmask: ff wmask: 0 w1cmask:0
>> qemu-system-x86_64: Failed to load PCIDevice:config
>> qemu-system-x86_64: Failed to load e1000e:parent_obj
>> qemu-system-x86_64: error while loading state for instance 0x0 of device
>> ':00:02.0/e1000e'
>> qemu-system-x86_64: load of migration failed: Invalid argument
>> The above test migrated a 7.2 machine type from QEMU master to QEMU
>> 7.2.0,
>> with this cmdline:
>> ./qemu-system-x86_64 -M pc-q35-7.2 [-incoming XXX]
>> In order to fix this, property x-pcie-err-unc-mask was introduced to
>> control when PCI_ERR_UNCOR_MASK is enabled. This property is enabled by
>> default, but is disabled if machine type <= 7.2.
>> Fixes: 010746ae1d ("hw/pci/aer: Implement PCI_ERR_UNCOR_MASK
>> register")
>> Suggested-by: Michael S. Tsirkin
>> Signed-off-by: Leonardo Bras
>
> It looks like 8.0-stable material to me, is it not?
It is .
I thought we have already requested that. We lost the intent somewhere,
sorry.
Later, Juan.
Re: [PATCH 0/8] migration: Add precopy initial data capability and VFIO precopy support
Avihai Horon wrote:
>> You have a point here.
>> But I will approach this case in a different way:
>>
>> Destination QEMU needs to be older, because it don't have the feature.
>> So we need to NOT being able to do the switchover for older machine
>> types.
>> And have something like this is qemu/hw/machine.c
>>
>> GlobalProperty hw_compat_7_2[] = {
>> { "our_device", "explicit-switchover", "off" },
>> };
>>
>> Or whatever we want to call the device and the property, and not use it
>> for older machine types to allow migration for that.
>
> Let me see if I get this straight (I'm not that familiar with
> hw_compat_x_y):
>
> You mean that device Y which adds support for explicit-switchover in
> QEMU version Z should add a property
> like you wrote above, and use it to disable explicit-switchover usage
> for Y devices when Y device
> from QEMU older than Z is migrated?
More that "from" "to"
Let me elaborate. We have two QEMUs:
QEMU version X, has device dev. Let's call it qemu-X.
QEMU version Y (X+1) add feature foo to device dev. Let's call it qemu-Y.
We have two machine types (for this exercise we don't care about
architectures)
PC-X.0
PC-Y.0
So, the possible combinations are:
First the easy cases, same qemu on both sides. Different machine types.
$ qemu-X -M PC-X.0 -> to -> qemu-X -M PC-X.0
good. neither guest use feature foo.
$ qemu-X -M PC-Y.0 -> to -> qemu-X -M PC-Y.0
impossible. qemu-X don't have machine PC-Y.0. So nothing to see here.
$ qemu-Y -M PC-X.0 -> to -> qemu-Y -M PC-X.0
good. We have feature foo in both sides. Notice that I recomend here
to not use feature foo. We will see on the difficult cases.
$ qemu-Y -M PC-Y.0 -> to -> qemu-Y -M PC-Y.0
good. Both sides use feature foo.
Difficult cases, when we mix qemu versions.
$ qemu-X -M PC-X.0 -> to -> qemu-Y -M PC-X.0
source don't have feature foo. Destination have feature foo.
But if we disable it for machine PC-X.0, it will work.
$ qemu-Y -M PC-X.0 -> to -> qemu-X -M PC-X.0
same than previous example. But here we have feature foo on source
and not on destination. Disabling it for machine PC-X.0 fixes the
problem.
We can't migrate a PC-Y.0 when one of the qemu's is qemu-X, so that case
is impossible.
Does this makes more sense?
And now, how hw_compat_X_Y works.
It is an array of registers with the format:
- name of device (we give some rope here, for instance migration is a
device in this context)
- name of property: self explanatory. The important bit is that
we can get the value of the property in the device driver.
- value of the property: self explanatory.
With this mechanism what we do when we add a feature to a device that
matters for migration is:
- for the machine type of the version that we are "developing" feature
is enabled by default. For whatever that enable means.
- for old machine types we disable the feature, so it can be migrate
freely with old qemu. But using the old machine type.
- there is way to enable the feature on the command line even for old
machine types on new qemu, but only developers use that for testing.
Normal users/admins never do that.
what does hw_compat_7_2 means?
Ok, we need to know the versions. New version is 8.0.
hw_compat_7_2 has all the properties represensing "features", defaults,
whatever that has changed since 7.2. In other words, what features we
need to disable to get to the features that existed when 7.2 was
released.
To go to a real example.
In the development tree. We have:
GlobalProperty hw_compat_8_0[] = {
{ "migration", "multifd-flush-after-each-section", "on"},
};
const size_t hw_compat_8_0_len = G_N_ELEMENTS(hw_compat_8_0);
Feature is implemented in the following commits:
77c259a4cb1c9799754b48f570301ebf1de5ded8
b05292c237030343516d073b1a1e5f49ffc017a8
294e5a4034e81b3d8db03b4e0f691386f20d6ed3
When we are doing migration with multifd and we pass the end of memory
(i.e. we end one iteration through all the RAM) we need to make sure
that we don't send the same page through two channels, i.e. contents of
the page at iteration 1 through channel 1 and contents of the page at
iteration 2 through channel 2. The problem is that they could arrive
out of order and the of page of iteration 1 arrive later than iteration
2 and overwrite new data with old data. Which is undesirable.
We could use complex algorithms to fix that, but one easy way of doing
it is:
- When we finish a run through all memory (i.e.) one iteration, we flush
all channels and make sure that everything arrives to destination
before starting sending data o the next iteration. I call that
synchronize all channels.
And that is what we *should* have done. But when I implemented the
feature, I did this synchronization everytime that we finish a cycle
(around 100miliseconds). i.e. 10 times per second. This is called a
section for historical reasons. And when you are migrating
multiterabytes RAM machines with very fast
[PATCH] tests/tcg/i386: correct mask for VPERM2F128/VPERM2I128
The instructions also use bits 3 and 7 of their 8-byte immediate. Signed-off-by: Paolo Bonzini --- tests/tcg/i386/test-avx.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tcg/i386/test-avx.py b/tests/tcg/i386/test-avx.py index d9ca00a49e6c..641a2ef69ebc 100755 --- a/tests/tcg/i386/test-avx.py +++ b/tests/tcg/i386/test-avx.py @@ -49,7 +49,7 @@ 'VEXTRACT[FI]128': 0x01, 'VINSERT[FI]128': 0x01, 'VPBLENDD': 0xff, -'VPERM2[FI]128': 0x33, +'VPERM2[FI]128': 0xbb, 'VPERMPD': 0xff, 'VPERMQ': 0xff, 'VPERMILPS': 0xff, -- 2.40.1
Re: [PATCH] target/i386: fix avx2 instructions vzeroall and vpermdq
Queued, thanks. Paolo
Re: [PATCH v1 1/1] hw/pci: Disable PCI_ERR_UNCOR_MASK register for machine type < 8.0
On Wed, May 10, 2023 at 07:29:40PM +0300, Michael Tokarev wrote:
> 03.05.2023 03:27, Leonardo Bras пишет:
> > Since it's implementation on v8.0.0-rc0, having the PCI_ERR_UNCOR_MASK
> > set for machine types < 8.0 will cause migration to fail if the target
> > QEMU version is < 8.0.0 :
> >
> > qemu-system-x86_64: get_pci_config_device: Bad config data: i=0x10a read:
> > 40 device: 0 cmask: ff wmask: 0 w1cmask:0
> > qemu-system-x86_64: Failed to load PCIDevice:config
> > qemu-system-x86_64: Failed to load e1000e:parent_obj
> > qemu-system-x86_64: error while loading state for instance 0x0 of device
> > ':00:02.0/e1000e'
> > qemu-system-x86_64: load of migration failed: Invalid argument
> >
> > The above test migrated a 7.2 machine type from QEMU master to QEMU 7.2.0,
> > with this cmdline:
> >
> > ./qemu-system-x86_64 -M pc-q35-7.2 [-incoming XXX]
> >
> > In order to fix this, property x-pcie-err-unc-mask was introduced to
> > control when PCI_ERR_UNCOR_MASK is enabled. This property is enabled by
> > default, but is disabled if machine type <= 7.2.
> >
> > Fixes: 010746ae1d ("hw/pci/aer: Implement PCI_ERR_UNCOR_MASK register")
> > Suggested-by: Michael S. Tsirkin
> > Signed-off-by: Leonardo Bras
>
> It looks like 8.0-stable material to me, is it not?
>
> /mjt
Good point. Feel free to CC stable.
Re: [PATCH v1 1/1] hw/pci: Disable PCI_ERR_UNCOR_MASK register for machine type < 8.0
03.05.2023 03:27, Leonardo Bras пишет:
Since it's implementation on v8.0.0-rc0, having the PCI_ERR_UNCOR_MASK
set for machine types < 8.0 will cause migration to fail if the target
QEMU version is < 8.0.0 :
qemu-system-x86_64: get_pci_config_device: Bad config data: i=0x10a read: 40
device: 0 cmask: ff wmask: 0 w1cmask:0
qemu-system-x86_64: Failed to load PCIDevice:config
qemu-system-x86_64: Failed to load e1000e:parent_obj
qemu-system-x86_64: error while loading state for instance 0x0 of device
':00:02.0/e1000e'
qemu-system-x86_64: load of migration failed: Invalid argument
The above test migrated a 7.2 machine type from QEMU master to QEMU 7.2.0,
with this cmdline:
./qemu-system-x86_64 -M pc-q35-7.2 [-incoming XXX]
In order to fix this, property x-pcie-err-unc-mask was introduced to
control when PCI_ERR_UNCOR_MASK is enabled. This property is enabled by
default, but is disabled if machine type <= 7.2.
Fixes: 010746ae1d ("hw/pci/aer: Implement PCI_ERR_UNCOR_MASK register")
Suggested-by: Michael S. Tsirkin
Signed-off-by: Leonardo Bras
It looks like 8.0-stable material to me, is it not?
/mjt
Re: [PATCH v3 0/3] ACPI: i386: bump MADT to revision 5
On 5/10/23 10:45, Igor Mammedov wrote:
On Wed, 10 May 2023 10:08:50 -0500
Eric DeVolder wrote:
On 5/10/23 03:14, Igor Mammedov wrote:
On Fri, 5 May 2023 16:53:22 -0500
Eric DeVolder wrote:
Thoughts?
I still don't think we need to bump x86 to rev 5 in QEMU.
Linux v6.3 has the fix merged (so crisis averted 8).
The investigation allowed me to opportunistically provide this patch.
I think this should receive serious consideration for merging,
more so because generating MADT .revision 3 and reporting .revision 1
seems wrong to me.
It's a way much simpler to bump revision to 3 without introducing
OnlineCapable handling. So if you post rev3 patch, I'll gladly
ack it.
(+include kernel commit ids of kernel side fix, so if someone
stumbles upon it, one can easily find what to backport)
OK, I'll do that. It'll be next week as I'm on a short week this week.
This patch seems really straight forward, and low risk, now.
eric
On 4/21/23 16:48, Eric DeVolder wrote:
The following Linux kernel change broke CPU hotplug for MADT revision
less than 5.
e2869bd7af60 ("x86/acpi/boot: Do not register processors that cannot be onlined
for x2APIC")
Discussion on this topic can be located here:
https://lore.kernel.org/linux-acpi/20230327191026.3454-1-eric.devol...@oracle.com/T/#t
have your kernel fix landed up upstream?
Yes, merged and available in v6.3.
which resulted in the following fixes Linux in 6.3-rc5:
a74fabfbd1b7: ("x86/ACPI/boot: Use FADT version to check support for online
capable")
fed8d8773b8e: ("x86/acpi/boot: Correct acpi_is_processor_usable() check")
However, as part of the investigation into resolving this breakage, I
learned that i386 QEMU reports revision 1, while technically it
generates revision 3. Aarch64 generates and reports revision 4.
ACPI 6.3 bumps MADT revision to 5 as it introduces an Online Capable
flag that the above Linux patch utilizes to denote hot pluggable CPUs.
So in order to bump MADT to the current revision of 5, need to
validate that all MADT table changes between 1 and 5 are present
in QEMU.
Below is a table summarizing the changes to the MADT. This information
gleamed from the ACPI specs on uefi.org.
ACPIMADTWhat
Version Version
1.0 MADT not present
2.0 1 Section 5.2.10.4
3.0 2 Section 5.2.11.4
5.2.11.13 Local SAPIC Structure added two new fields:
ACPI Processor UID Value
ACPI Processor UID String
5.2.10.14 Platform Interrupt Sources Structure:
Reserved changed to Platform Interrupt Sources Flags
3.0b2 Section 5.2.11.4
Added a section describing guidelines for the ordering of
processors in the MADT to support proper boot processor
and multi-threaded logical processor operation.
4.0 3 Section 5.2.12
Adds Processor Local x2APIC structure type 9
Adds Local x2APIC NMI structure type 0xA
5.0 3 Section 5.2.12
6.0 3 Section 5.2.12
6.0a4 Section 5.2.12
Adds ARM GIC structure types 0xB-0xF
6.2a45 Section 5.2.12 <--- version 45, is indeed accurate!
6.2b5 Section 5.2.12
GIC ITS last Reserved offset changed to 16 from 20 (typo)
6.3 5 Section 5.2.12
Adds Local APIC Flags Online Capable!
Adds GICC SPE Overflow Interrupt field
6.4 5 Section 5.2.12
Adds Multiprocessor Wakeup Structure type 0x10
(change notes says structure previously misplaced?)
6.5 5 Section 5.2.12
For the MADT revision change 1 -> 2, the spec has a change to the
SAPIC structure. In general, QEMU does not generate/support SAPIC.
So the QEMU i386 MADT revision can safely be moved to 2.
For the MADT revision change 2 -> 3, the spec adds Local x2APIC
structures. QEMU has long supported x2apic ACPI structures. A simple
search of x2apic within QEMU source and hw/i386/acpi-common.c
specifically reveals this. So the QEMU i386 MADT revision can safely
be moved to 3.
For the MADT revision change 3 -> 4, the spec adds support for the ARM
GIC structures. QEMU ARM does in fact generate and report revision 4.
As these will not be used by i386 QEMU, so then the QEMU i386 MADT
revision can safely be moved to 4 as well.
Now for the MADT revision change 4 -> 5, the spec adds the Online
Capable flag to the Local APIC structure, and the ARM GICC SPE
Overflow Interrupt field.
All ARM stuff is irrelevant in x86 patch
sure
For i386, the ARM SPE is not applicable.
For the i386 Local APIC flag Online Capable, the spec has certain rules
about this value. And in particuar setting this value now explicitly
indicates a hotpluggable CPU.
So this patch makes the needed changes to move i386 MADT to
revision 5.
Without these
Re: [PATCH] make: clean after distclean deletes source files
Queued, thanks. Paolo
Re: [PATCH v2 3/3] target/openrisc: Setup FPU for detecting tininess before rounding
On 5/10/23 16:32, Stafford Horne wrote: OpenRISC defines tininess to be detected before rounding. Setup qemu to obey this. Signed-off-by: Stafford Horne --- Since v1: - Remove setting default NaN behavior. I discussed with the FPU developers and they mentioned the OpenRISC hardware should be IEEE compliant when handling and forwarding NaN payloads, and they don't want try change this. There is no such thing as IEEE compliant for NaN payloads. All of that is implementation defined. All OpenRISC needs to do is document its intentions (and then double-check that fpu/softfloat-specialize.c.inc does what is documented). Anyway, back to this patch, Reviewed-by: Richard Henderson :-) r~
Re: [PULL 00/28] Block layer patches
On 5/10/23 13:20, Kevin Wolf wrote: The following changes since commit b2896c1b09878fd1c4b485b3662f8beecbe0fef4: Merge tag 'vfio-updates-20230509.0' of https://gitlab.com/alex.williamson/qemu into staging (2023-05-10 11:20:35 +0100) are available in the Git repository at: https://repo.or.cz/qemu/kevin.git tags/for-upstream for you to fetch changes up to 58a2e3f5c37be02dac3086b81bdda9414b931edf: block: compile out assert_bdrv_graph_readable() by default (2023-05-10 14:16:54 +0200) Block layer patches - Graph locking, part 3 (more block drivers) - Compile out assert_bdrv_graph_readable() by default - Add configure options for vmdk, vhdx and vpc - Fix use after free in blockdev_mark_auto_del() - migration: Attempt disk reactivation in more failure scenarios - Coroutine correctness fixes Applied, thanks. Please update https://wiki.qemu.org/ChangeLog/8.1 as appropriate. r~
Re: [PATCH v9 0/6] block: refactor blockdev transactions
On 5/10/23 17:21, Vladimir Sementsov-Ogievskiy wrote: Interesting, I see two 5/6 letters, equal body, but a bit different headers (the second has empty "Sender").. for me all is OK
Re: [PATCH v2 2/3] target/openrisc: Set PC to cpu state on FPU exception
On 5/10/23 16:32, Stafford Horne wrote: Store the PC to ensure the correct value can be read in the exception handler. Signed-off-by: Stafford Horne --- Since v1: - Use function do_fpe (similar to do_range) to raise exception. target/openrisc/fpu_helper.c | 13 +++-- 1 file changed, 11 insertions(+), 2 deletions(-) Reviewed-by: Richard Henderson r~
Re: [PATCH v2 1/3] target/openrisc: Allow fpcsr access in user mode
On 5/10/23 16:32, Stafford Horne wrote:
void HELPER(mtspr)(CPUOpenRISCState *env, target_ulong spr, target_ulong rb)
{
-#ifndef CONFIG_USER_ONLY
OpenRISCCPU *cpu = env_archcpu(env);
+#ifndef CONFIG_USER_ONLY
CPUState *cs = env_cpu(env);
Pulled cpu out if ifdef here...
@@ -204,10 +220,22 @@ target_ulong HELPER(mfspr)(CPUOpenRISCState *env,
target_ulong rd,
OpenRISCCPU *cpu = env_archcpu(env);
CPUState *cs = env_cpu(env);
int idx;
+#else
+OpenRISCCPU *cpu = env_archcpu(env);
#endif
But replicated it here.
Otherwise,
Reviewed-by: Richard Henderson
r~
Re: [PATCH] scsi: check inquiry buffer length to prevent crash
On 4/26/23 15:37, Théo Maillart wrote:
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -191,7 +191,7 @@ static int scsi_handle_inquiry_reply(SCSIGenericReq *r,
SCSIDevice *s, int len)
if ((s->type == TYPE_DISK || s->type == TYPE_ZBC) &&
(r->req.cmd.buf[1] & 0x01)) {
page = r->req.cmd.buf[2];
-if (page == 0xb0) {
+if (page == 0xb0 && r->buflen >= 12) {
uint64_t max_transfer = calculate_max_transfer(s);
stl_be_p(>buf[8], max_transfer);
/* Also take care of the opt xfer len. */
--
This is not enough because right below there is a store of bytes 12..15.
The best thing to do is to:
1) do the stores in an "uint8_t buf[8]" on the stack, followed by a
memcpy to r->buf + 8.
2) add "&& r->buflen > 8" to the condition similar to what you've done
above.
Paolo
Re: [PATCH v3 0/3] ACPI: i386: bump MADT to revision 5
On Wed, 10 May 2023 10:08:50 -0500
Eric DeVolder wrote:
> On 5/10/23 03:14, Igor Mammedov wrote:
> > On Fri, 5 May 2023 16:53:22 -0500
> > Eric DeVolder wrote:
> >
> >> Thoughts?
> >
> > I still don't think we need to bump x86 to rev 5 in QEMU.
>
> Linux v6.3 has the fix merged (so crisis averted 8).
> The investigation allowed me to opportunistically provide this patch.
> I think this should receive serious consideration for merging,
> more so because generating MADT .revision 3 and reporting .revision 1
> seems wrong to me.
It's a way much simpler to bump revision to 3 without introducing
OnlineCapable handling. So if you post rev3 patch, I'll gladly
ack it.
(+include kernel commit ids of kernel side fix, so if someone
stumbles upon it, one can easily find what to backport)
> This patch seems really straight forward, and low risk, now.
>
> >
> >> eric
> >>
> >> On 4/21/23 16:48, Eric DeVolder wrote:
> >>> The following Linux kernel change broke CPU hotplug for MADT revision
> >>> less than 5.
> >>>
> >>>e2869bd7af60 ("x86/acpi/boot: Do not register processors that cannot
> >>> be onlined for x2APIC")
> >>>
> >>> Discussion on this topic can be located here:
> >>>
> >>>
> >>> https://lore.kernel.org/linux-acpi/20230327191026.3454-1-eric.devol...@oracle.com/T/#t
> >>>
> >
> > have your kernel fix landed up upstream?
> >
> Yes, merged and available in v6.3.
>
> >>>
> >>> which resulted in the following fixes Linux in 6.3-rc5:
> >>>
> >>>a74fabfbd1b7: ("x86/ACPI/boot: Use FADT version to check support for
> >>> online capable")
> >>>fed8d8773b8e: ("x86/acpi/boot: Correct acpi_is_processor_usable()
> >>> check")
> >>>
> >>> However, as part of the investigation into resolving this breakage, I
> >>> learned that i386 QEMU reports revision 1, while technically it
> >>> generates revision 3. Aarch64 generates and reports revision 4.
> >>>
> >>> ACPI 6.3 bumps MADT revision to 5 as it introduces an Online Capable
> >>> flag that the above Linux patch utilizes to denote hot pluggable CPUs.
> >>>
> >>> So in order to bump MADT to the current revision of 5, need to
> >>> validate that all MADT table changes between 1 and 5 are present
> >>> in QEMU.
> >>>
> >>> Below is a table summarizing the changes to the MADT. This information
> >>> gleamed from the ACPI specs on uefi.org.
> >>>
> >>> ACPIMADTWhat
> >>> Version Version
> >>> 1.0 MADT not present
> >>> 2.0 1 Section 5.2.10.4
> >>> 3.0 2 Section 5.2.11.4
> >>>5.2.11.13 Local SAPIC Structure added two new fields:
> >>> ACPI Processor UID Value
> >>> ACPI Processor UID String
> >>>5.2.10.14 Platform Interrupt Sources Structure:
> >>> Reserved changed to Platform Interrupt Sources Flags
> >>> 3.0b2 Section 5.2.11.4
> >>>Added a section describing guidelines for the ordering
> >>> of
> >>>processors in the MADT to support proper boot processor
> >>>and multi-threaded logical processor operation.
> >>> 4.0 3 Section 5.2.12
> >>>Adds Processor Local x2APIC structure type 9
> >>>Adds Local x2APIC NMI structure type 0xA
> >>> 5.0 3 Section 5.2.12
> >>> 6.0 3 Section 5.2.12
> >>> 6.0a4 Section 5.2.12
> >>>Adds ARM GIC structure types 0xB-0xF
> >>> 6.2a45 Section 5.2.12 <--- version 45, is indeed accurate!
> >>> 6.2b5 Section 5.2.12
> >>>GIC ITS last Reserved offset changed to 16 from 20
> >>> (typo)
> >>> 6.3 5 Section 5.2.12
> >>>Adds Local APIC Flags Online Capable!
> >>>Adds GICC SPE Overflow Interrupt field
> >>> 6.4 5 Section 5.2.12
> >>>Adds Multiprocessor Wakeup Structure type 0x10
> >>>(change notes says structure previously misplaced?)
> >>> 6.5 5 Section 5.2.12
> >>>
> >>> For the MADT revision change 1 -> 2, the spec has a change to the
> >>> SAPIC structure. In general, QEMU does not generate/support SAPIC.
> >>> So the QEMU i386 MADT revision can safely be moved to 2.
> >>>
> >>> For the MADT revision change 2 -> 3, the spec adds Local x2APIC
> >>> structures. QEMU has long supported x2apic ACPI structures. A simple
> >>> search of x2apic within QEMU source and hw/i386/acpi-common.c
> >>> specifically reveals this. So the QEMU i386 MADT revision can safely
> >>> be moved to 3.
> >>>
> >>> For the MADT revision change 3 -> 4, the spec adds support for the ARM
> >>> GIC structures. QEMU ARM does in fact generate and report revision 4.
> >>> As these will not be used by i386 QEMU, so then the QEMU i386 MADT
> >>> revision can safely be moved to 4 as well.
> >>>
> >>> Now for the MADT revision change 4 -> 5, the spec adds the Online
> >>> Capable flag to the
