Re: [PATCH 2/4] tests/lcitool: Remove g++ from the containers (except for the MinGW one)

[Thomas Huth]

On 13/05/2024 12.28, Daniel P. Berrangé wrote:

On Mon, May 13, 2024 at 12:22:50PM +0200, Thomas Huth wrote:

We don't need C++ for the normal QEMU builds anymore, so installing
g++ in each and every container seems to be a waste of time and disk
space. The only container that still needs it is the Fedora MinGW
container that builds the only remaining C++ code in ./qga/vss-win32/
and we can install it here with an extra RUN statement instead.

This way we can also add the mingw-w64-tools package quite easily
which contains the x86_64-w64-mingw32-widl program that is required
for compiling the vss code of the guest agent (it was missing before
this change, so the VSS code was actually never compiled in the CI).

Signed-off-by: Thomas Huth 
---
  tests/lcitool/projects/qemu.yml |  1 -
  tests/lcitool/refresh   | 10 --
  2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/tests/lcitool/projects/qemu.yml b/tests/lcitool/projects/qemu.yml
index 9173d1e36e..b63b6bd850 100644
--- a/tests/lcitool/projects/qemu.yml
+++ b/tests/lcitool/projects/qemu.yml
@@ -22,7 +22,6 @@ packages:
   - findutils
   - flex
   - fuse3
- - g++
   - gcc
   - gcc-native
   - gcovr
diff --git a/tests/lcitool/refresh b/tests/lcitool/refresh
index 24a735a3f2..dda07ddcd1 100755
--- a/tests/lcitool/refresh
+++ b/tests/lcitool/refresh
@@ -109,6 +109,11 @@ debian12_extras = [
  "ENV QEMU_CONFIGURE_OPTS --enable-netmap\n"
  ]
  
+fedora_mingw_extras = [ "\n"

+"RUN nosync dnf install -y mingw64-gcc-c++ mingw-w64-tools && \\\n"
+"  ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/x86_64-w64-mingw32-c++ && 
\\\n"
+"  ln -s /usr/bin/ccache 
/usr/libexec/ccache-wrappers/x86_64-w64-mingw32-g++\n\n"
+]
  
  def cross_build(prefix, targets):

  conf = "ENV QEMU_CONFIGURE_OPTS --cross-prefix=%s\n" % (prefix)
@@ -193,8 +198,9 @@ try:
  
  generate_dockerfile("fedora-win64-cross", "fedora-38",

  cross="mingw64",
-trailer=cross_build("x86_64-w64-mingw32-",
-"x86_64-softmmu"))
+trailer="".join(fedora_mingw_extras)
++ cross_build("x86_64-w64-mingw32-",
+  "x86_64-softmmu"))
  
  #

  # Cirrus packages lists for GitLab


A better way to handle this would be to define a separate project

   'tests/lcitool/projects/qemu-win-installer.yml'

With

packages
  - g++

Then enable the extra project for win64

 generate_dockerfile("fedora-win64-cross", "fedora-38",
 project='qemu,qemu-win-installer',
 cross="mingw64",
 trailer=cross_build("x86_64-w64-mingw32-",
 "x86_64-softmmu"))

which should result in an identical container to what we have today
for win64, while letting us slim the other containers.


Ok, good idea! ... but then we need to teach lcitool about mingw-w64-tools 
first, otherwise that vss code won't get built due to the missing "widl" tool.


 Thomas

[PATCH v2] configure: Fix error message when C compiler is not working

[Thomas Huth]

If you try to run the configure script on a system without a working
C compiler, you get a very misleading error message:

 ERROR: Unrecognized host OS (uname -s reports 'Linux')

Some people already opened bug tickets because of this problem:

 https://gitlab.com/qemu-project/qemu/-/issues/2057
 https://gitlab.com/qemu-project/qemu/-/issues/2288

We should rather tell the user that we were not able to use the C
compiler instead, otherwise they will have a hard time to figure
out what was going wrong.

While we're at it, let's also suppress the "unrecognized host CPU"
message in this case since it is rather misleading than helpful.

Fixes: 264b803721 ("configure: remove compiler sanity check")
Signed-off-by: Thomas Huth 
---
 v2: Reworked the patch according to Peter's suggestions:
 https://lists.gnu.org/archive/html/qemu-devel/2024-03/msg04643.html

 configure | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 1dca3d94c0..519f1bb6e2 100755
--- a/configure
+++ b/configure
@@ -411,7 +411,9 @@ else
   # Using uname is really broken, but it is just a fallback for architectures
   # that are going to use TCI anyway
   cpu=$(uname -m)
-  echo "WARNING: unrecognized host CPU, proceeding with 'uname -m' output 
'$cpu'"
+  if test "$host_os" != "bogus"; then
+echo "WARNING: unrecognized host CPU, proceeding with 'uname -m' output 
'$cpu'"
+  fi
 fi
 
 # Normalise host CPU name to the values used by Meson cross files and in source
@@ -894,6 +896,13 @@ EOF
 exit 0
 fi
 
+# Now that we are sure that the user did not only want to print the --help
+# information, we should double-check that the C compiler really works:
+write_c_skeleton
+if ! compile_object ; then
+error_exit "C compiler \"$cc\" either does not exist or does not work."
+fi
+
 # Remove old dependency files to make sure that they get properly regenerated
 rm -f ./*/config-devices.mak.d
 
-- 
2.45.0

Re: [PATCH 3/3] gitlab: use 'setarch -R' to workaround tsan bug

[Thomas Huth]

On 13/05/2024 13.15, Daniel P. Berrangé wrote:

The TSAN job started failing when gitlab rolled out their latest
release. The root cause is a change in the Google COS version used
on shared runners. This brings a kernel running with

  vm.mmap_rnd_bits = 31

which is incompatible with TSAN in LLVM < 18, which only supports
upto '28'. LLVM 18 can support upto '30', and failing that will
re-exec itself to turn off VA randomization.

Our LLVM is too old for now, but we can run with 'setarch -R make ..'
to turn off VA randomization ourselves.

Signed-off-by: Daniel P. Berrangé 
---
  .gitlab-ci.d/buildtest.yml | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index bab6194564..d864562628 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -575,6 +575,9 @@ tsan-build:
  CONFIGURE_ARGS: --enable-tsan --cc=clang --cxx=clang++
--enable-trace-backends=ust --disable-slirp
  TARGETS: x86_64-softmmu ppc64-softmmu riscv64-softmmu x86_64-linux-user
+# Remove when we switch to a distro with clang >= 18
+# https://github.com/google/sanitizers/issues/1716
+MAKE: setarch -R make


Thanks for tackling this!

Reviewed-by: Thomas Huth 

Re: [PATCH 2/3] gitlab: use $MAKE instead of 'make'

[Thomas Huth]

On 13/05/2024 13.15, Daniel P. Berrangé wrote:

The lcitool generated containers have '$MAKE' set to the path
of the right 'make' binary. Using the env variable makes it
possible to override the choice per job.

Signed-off-by: Daniel P. Berrangé 
---
  .gitlab-ci.d/buildtest-template.yml | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)


Reviewed-by: Thomas Huth 

Re: [PATCH 1/3] dockerfiles: add 'MAKE' env variable to remaining containers

[Thomas Huth]

On 13/05/2024 13.15, Daniel P. Berrangé wrote:

All the lcitool generated containers define a "MAKE" env. It will be
convenient for later patches if all containers do this.

Signed-off-by: Daniel P. Berrangé 
---
  tests/docker/dockerfiles/debian-all-test-cross.docker| 1 +
  tests/docker/dockerfiles/debian-hexagon-cross.docker | 1 +
  tests/docker/dockerfiles/debian-legacy-test-cross.docker | 1 +
  tests/docker/dockerfiles/debian-loongarch-cross.docker   | 1 +
  tests/docker/dockerfiles/debian-tricore-cross.docker | 1 +
  tests/docker/dockerfiles/debian-xtensa-cross.docker  | 1 +
  tests/docker/dockerfiles/fedora-cris-cross.docker| 1 +
  7 files changed, 7 insertions(+)


Reviewed-by: Thomas Huth 

[PATCH 3/4] tests/lcitool/projects/qemu.yml: Sort entries alphabetically again

[Thomas Huth]

Let's try to keep the entries in alphabetical order here!

Signed-off-by: Thomas Huth 
---
 tests/lcitool/projects/qemu.yml | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/tests/lcitool/projects/qemu.yml b/tests/lcitool/projects/qemu.yml
index b63b6bd850..752079b858 100644
--- a/tests/lcitool/projects/qemu.yml
+++ b/tests/lcitool/projects/qemu.yml
@@ -35,12 +35,12 @@ packages:
  - hostname
  - json-c
  - libaio
- - libattr
  - libasan
+ - libattr
  - libbpf
- - libc-static
  - libcacard
  - libcap-ng
+ - libc-static
  - libcurl
  - libdrm
  - libepoxy
@@ -54,6 +54,7 @@ packages:
  - libjpeg
  - libnfs
  - libnuma
+ - libpipewire-dev
  - libpmem
  - libpng
  - librbd
@@ -73,27 +74,26 @@ packages:
  - llvm
  - lttng-ust
  - lzo
+ - make
+ - mesa-libgbm
+ - meson
  - mtools
+ - ncursesw
  - netcat
  - nettle
  - ninja
  - nsis
- - make
- - mesa-libgbm
- - meson
- - ncursesw
  - pam
  - pcre-static
  - pixman
- - libpipewire-dev
  - pkg-config
  - pulseaudio
  - python3
- - python3-PyYAML
  - python3-numpy
  - python3-opencv
  - python3-pillow
  - python3-pip
+ - python3-PyYAML
  - python3-sphinx
  - python3-sphinx-rtd-theme
  - python3-sqlite3
@@ -121,6 +121,6 @@ packages:
  - which
  - xen
  - xorriso
- - zstdtools
  - zlib
  - zlib-static
+ - zstdtools
-- 
2.45.0

[PATCH 2/4] tests/lcitool: Remove g++ from the containers (except for the MinGW one)

[Thomas Huth]

We don't need C++ for the normal QEMU builds anymore, so installing
g++ in each and every container seems to be a waste of time and disk
space. The only container that still needs it is the Fedora MinGW
container that builds the only remaining C++ code in ./qga/vss-win32/
and we can install it here with an extra RUN statement instead.

This way we can also add the mingw-w64-tools package quite easily
which contains the x86_64-w64-mingw32-widl program that is required
for compiling the vss code of the guest agent (it was missing before
this change, so the VSS code was actually never compiled in the CI).

Signed-off-by: Thomas Huth 
---
 tests/lcitool/projects/qemu.yml |  1 -
 tests/lcitool/refresh   | 10 --
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/tests/lcitool/projects/qemu.yml b/tests/lcitool/projects/qemu.yml
index 9173d1e36e..b63b6bd850 100644
--- a/tests/lcitool/projects/qemu.yml
+++ b/tests/lcitool/projects/qemu.yml
@@ -22,7 +22,6 @@ packages:
  - findutils
  - flex
  - fuse3
- - g++
  - gcc
  - gcc-native
  - gcovr
diff --git a/tests/lcitool/refresh b/tests/lcitool/refresh
index 24a735a3f2..dda07ddcd1 100755
--- a/tests/lcitool/refresh
+++ b/tests/lcitool/refresh
@@ -109,6 +109,11 @@ debian12_extras = [
 "ENV QEMU_CONFIGURE_OPTS --enable-netmap\n"
 ]
 
+fedora_mingw_extras = [ "\n"
+"RUN nosync dnf install -y mingw64-gcc-c++ mingw-w64-tools && \\\n"
+"  ln -s /usr/bin/ccache 
/usr/libexec/ccache-wrappers/x86_64-w64-mingw32-c++ && \\\n"
+"  ln -s /usr/bin/ccache 
/usr/libexec/ccache-wrappers/x86_64-w64-mingw32-g++\n\n"
+]
 
 def cross_build(prefix, targets):
 conf = "ENV QEMU_CONFIGURE_OPTS --cross-prefix=%s\n" % (prefix)
@@ -193,8 +198,9 @@ try:
 
 generate_dockerfile("fedora-win64-cross", "fedora-38",
 cross="mingw64",
-trailer=cross_build("x86_64-w64-mingw32-",
-"x86_64-softmmu"))
+trailer="".join(fedora_mingw_extras)
++ cross_build("x86_64-w64-mingw32-",
+  "x86_64-softmmu"))
 
 #
 # Cirrus packages lists for GitLab
-- 
2.45.0

[PATCH 4/4] tests/docker/dockerfiles: Update container files with "lcitool-refresh"

[Thomas Huth]

Run "make lcitool-refresh" after the previous changes to the lcitool
files. This removes the g++ and xfslibs-dev packages from the dockerfiles.

Signed-off-by: Thomas Huth 
---
 tests/docker/dockerfiles/alpine.docker| 4 
 tests/docker/dockerfiles/centos9.docker   | 4 
 tests/docker/dockerfiles/debian-amd64-cross.docker| 4 
 tests/docker/dockerfiles/debian-arm64-cross.docker| 4 
 tests/docker/dockerfiles/debian-armel-cross.docker| 4 
 tests/docker/dockerfiles/debian-armhf-cross.docker| 4 
 tests/docker/dockerfiles/debian-i686-cross.docker | 4 
 tests/docker/dockerfiles/debian-mips64el-cross.docker | 4 
 tests/docker/dockerfiles/debian-mipsel-cross.docker   | 4 
 tests/docker/dockerfiles/debian-ppc64el-cross.docker  | 4 
 tests/docker/dockerfiles/debian-s390x-cross.docker| 4 
 tests/docker/dockerfiles/debian.docker| 4 
 tests/docker/dockerfiles/fedora-win64-cross.docker| 8 +---
 tests/docker/dockerfiles/fedora.docker| 4 
 tests/docker/dockerfiles/opensuse-leap.docker | 4 
 tests/docker/dockerfiles/ubuntu2204.docker| 4 
 16 files changed, 5 insertions(+), 63 deletions(-)

diff --git a/tests/docker/dockerfiles/alpine.docker 
b/tests/docker/dockerfiles/alpine.docker
index cd9d7af1ce..554464f31e 100644
--- a/tests/docker/dockerfiles/alpine.docker
+++ b/tests/docker/dockerfiles/alpine.docker
@@ -32,7 +32,6 @@ RUN apk update && \
 findutils \
 flex \
 fuse3-dev \
-g++ \
 gcc \
 gcovr \
 gettext \
@@ -110,7 +109,6 @@ RUN apk update && \
 vte3-dev \
 which \
 xen-dev \
-xfsprogs-dev \
 xorriso \
 zlib-dev \
 zlib-static \
@@ -119,10 +117,8 @@ RUN apk update && \
 rm -f /usr/lib*/python3*/EXTERNALLY-MANAGED && \
 apk list --installed | sort > /packages.txt && \
 mkdir -p /usr/libexec/ccache-wrappers && \
-ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/c++ && \
 ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/cc && \
 ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/clang && \
-ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/g++ && \
 ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/gcc
 
 ENV CCACHE_WRAPPERSDIR "/usr/libexec/ccache-wrappers"
diff --git a/tests/docker/dockerfiles/centos9.docker 
b/tests/docker/dockerfiles/centos9.docker
index 6cf47ce786..0256865b9e 100644
--- a/tests/docker/dockerfiles/centos9.docker
+++ b/tests/docker/dockerfiles/centos9.docker
@@ -34,7 +34,6 @@ RUN dnf distro-sync -y && \
 flex \
 fuse3-devel \
 gcc \
-gcc-c++ \
 gettext \
 git \
 glib2-devel \
@@ -115,7 +114,6 @@ RUN dnf distro-sync -y && \
 util-linux \
 vte291-devel \
 which \
-xfsprogs-devel \
 xorriso \
 zlib-devel \
 zlib-static \
@@ -125,10 +123,8 @@ RUN dnf distro-sync -y && \
 rm -f /usr/lib*/python3*/EXTERNALLY-MANAGED && \
 rpm -qa | sort > /packages.txt && \
 mkdir -p /usr/libexec/ccache-wrappers && \
-ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/c++ && \
 ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/cc && \
 ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/clang && \
-ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/g++ && \
 ln -s /usr/bin/ccache /usr/libexec/ccache-wrappers/gcc
 
 ENV CCACHE_WRAPPERSDIR "/usr/libexec/ccache-wrappers"
diff --git a/tests/docker/dockerfiles/debian-amd64-cross.docker 
b/tests/docker/dockerfiles/debian-amd64-cross.docker
index d0b0e9778e..f8c61d1191 100644
--- a/tests/docker/dockerfiles/debian-amd64-cross.docker
+++ b/tests/docker/dockerfiles/debian-amd64-cross.docker
@@ -79,7 +79,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
 eatmydata apt-get dist-upgrade -y && \
 eatmydata apt-get install --no-install-recommends -y dpkg-dev && \
 eatmydata apt-get install --no-install-recommends -y \
-  g++-x86-64-linux-gnu \
   gcc-x86-64-linux-gnu \
   libaio-dev:amd64 \
   libasan6:amd64 \
@@ -149,7 +148,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
   libzstd-dev:amd64 \
   nettle-dev:amd64 \
   systemtap-sdt-dev:amd64 \
-  xfslibs-dev:amd64 \
   zlib1g-dev:amd64 && \
 eatmydata apt-get autoremove -y && \
 eatmydata apt-get autoclean -y && \
@@ -167,9 +165,7 @@ cpu = 'x86_64'\n\
 endian = 'little'\n" > /usr/local/share/meson/cross/x86_64-linux-gnu &&a

[PATCH 1/4] tests/lcitool: Remove 'xfsprogs' from QEMU

[Thomas Huth]

From: Philippe Mathieu-Daudé 

QEMU's commit a5730b8bd3 ("block/file-posix: Simplify the
XFS_IOC_DIOINFO handling") removed the need for the 'xfsprogs'
package.

Signed-off-by: Philippe Mathieu-Daudé 
[thuth: Adjusted the patch from the lcitools repo to QEMU's repo]
Signed-off-by: Thomas Huth 
---
 tests/lcitool/projects/qemu.yml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/lcitool/projects/qemu.yml b/tests/lcitool/projects/qemu.yml
index 149b15de57..9173d1e36e 100644
--- a/tests/lcitool/projects/qemu.yml
+++ b/tests/lcitool/projects/qemu.yml
@@ -121,7 +121,6 @@ packages:
  - vte
  - which
  - xen
- - xfsprogs
  - xorriso
  - zstdtools
  - zlib
-- 
2.45.0

[PATCH 0/4] tests: Remove xfsprogs and g++ from the dockerfiles

[Thomas Huth]

The xfsprogs package is not necessary anymore since commit a5730b8bd3
and we don't need g++ in any of the containers (except for the mingw
cross compiler container which can be handled differently), so this
patch series simplifies the dockerfiles a little bit accordingly.

Philippe Mathieu-Daudé (1):
  tests/lcitool: Remove 'xfsprogs' from QEMU

Thomas Huth (3):
  tests/lcitool: Remove g++ from the containers (except for the MinGW
one)
  tests/lcitool/projects/qemu.yml: Sort entries alphabetically again
  tests/docker/dockerfiles: Update container files with
"lcitool-refresh"

 tests/docker/dockerfiles/alpine.docker|  4 
 tests/docker/dockerfiles/centos9.docker   |  4 
 .../dockerfiles/debian-amd64-cross.docker |  4 
 .../dockerfiles/debian-arm64-cross.docker |  4 
 .../dockerfiles/debian-armel-cross.docker |  4 
 .../dockerfiles/debian-armhf-cross.docker |  4 
 .../dockerfiles/debian-i686-cross.docker  |  4 
 .../dockerfiles/debian-mips64el-cross.docker  |  4 
 .../dockerfiles/debian-mipsel-cross.docker|  4 
 .../dockerfiles/debian-ppc64el-cross.docker   |  4 
 .../dockerfiles/debian-s390x-cross.docker |  4 
 tests/docker/dockerfiles/debian.docker|  4 
 .../dockerfiles/fedora-win64-cross.docker |  8 +---
 tests/docker/dockerfiles/fedora.docker|  4 
 tests/docker/dockerfiles/opensuse-leap.docker |  4 
 tests/docker/dockerfiles/ubuntu2204.docker|  4 
 tests/lcitool/projects/qemu.yml   | 20 +--
 tests/lcitool/refresh | 10 --
 18 files changed, 22 insertions(+), 76 deletions(-)

-- 
2.45.0

Re: [PATCH 1/1] tests/fp/meson: don't build fp-bench test if fenv.h is missing

[Thomas Huth]

On 11/05/2024 13.09, Dario Binacchi wrote:

On Sat, May 11, 2024 at 12:25 PM Richard Henderson
 wrote:


On 5/11/24 12:11, Dario Binacchi wrote:

Gentle ping.


Gentle reminder that I strongly suspect that your buildroot is corrupt.
There *should* be a  present.


I don't think so. In fact, the patch has already been merged into Buildroot:
https://patchwork.ozlabs.org/project/buildroot/patch/20240502072327.741463-1-dario.binac...@amarulasolutions.com/

As mentioned earlier:
"The fenv support is not enabled in our default uClibc configurations"
https://lists.buildroot.org/pipermail/buildroot/2013-May/072440.html


So the missing information from that page is: It's apparently possible to 
build uClibc without fenv support, it's only optional there!


So IMHO this patch is fine and should be included.

 Thomas

[PULL 8/8] tests/qtest: Add some test cases support on LoongArch

[Thomas Huth]

From: Bibo Mao 

Add boot-serial-test and filter test cases support on LoongArch system.

Signed-off-by: Bibo Mao 
Message-ID: <20240509084745.2514607-1-maob...@loongson.cn>
Signed-off-by: Thomas Huth 
---
 tests/qtest/boot-serial-test.c | 10 ++
 tests/qtest/meson.build|  3 +++
 2 files changed, 13 insertions(+)

diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial-test.c
index e3b7d65fe5..df389adeeb 100644
--- a/tests/qtest/boot-serial-test.c
+++ b/tests/qtest/boot-serial-test.c
@@ -26,6 +26,14 @@ static const uint8_t bios_avr[] = {
 0x80, 0x93, 0xc6, 0x00, /* sts 0x00C6, r24 ; Output 'T' */
 };
 
+static const uint8_t bios_loongarch64[] = {
+0x0c, 0xc0, 0x3f, 0x14, /* lu12i.w $t0, 0x1fe00*/
+0x8c, 0x81, 0x87, 0x03, /* ori $t0, $t0, 0x1e0 */
+0x0d, 0x50, 0x81, 0x03, /* li.w$t1, 'T'*/
+0x8d, 0x01, 0x00, 0x29, /* st.b$t1, $t0, 0 */
+0xff, 0xf3, 0xff, 0x53, /*  b  -16  # loop */
+};
+
 static const uint8_t kernel_mcf5208[] = {
 0x41, 0xf9, 0xfc, 0x06, 0x00, 0x00, /* lea 0xfc06,%a0 */
 0x10, 0x3c, 0x00, 0x54, /* move.b #'T',%d0 */
@@ -167,6 +175,8 @@ static const testdef_t tests[] = {
 { "sparc", "SS-600MP", "", "TMS390Z55" },
 { "sparc64", "sun4u", "", "UltraSPARC" },
 { "s390x", "s390-ccw-virtio", "", "device" },
+{ "loongarch64", "virt", "-cpu max", "TT", sizeof(bios_loongarch64),
+  NULL, bios_loongarch64 },
 { "m68k", "mcf5208evb", "", "TT", sizeof(kernel_mcf5208), kernel_mcf5208 },
 { "m68k", "next-cube", "", "TT", sizeof(bios_nextcube), 0, bios_nextcube },
 { "microblaze", "petalogix-s3adsp1800", "", "TT",
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 6f2f594ace..86293051dc 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -139,6 +139,9 @@ qtests_hppa = ['boot-serial-test'] + \
   qtests_filter + \
   (config_all_devices.has_key('CONFIG_VGA') ? ['display-vga-test'] : [])
 
+qtests_loongarch64 = qtests_filter + \
+  ['boot-serial-test']
+
 qtests_m68k = ['boot-serial-test'] + \
   qtests_filter
 
-- 
2.45.0

[PULL 6/8] target/s390x: flag te and cte as deprecated

[Thomas Huth]

From: Collin Walling 

Add the CONSTRAINT_TRANSACTIONAL_EXE (cte) and TRANSACTIONAL_EXE (te)
to the list of deprecated features.

Signed-off-by: Collin Walling 
Reviewed-by: David Hildenbrand 
Message-ID: <20240429191059.11806-3-wall...@linux.ibm.com>
Signed-off-by: Thomas Huth 
---
 target/s390x/cpu_features.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
index efafc9711c..cb4e2b8920 100644
--- a/target/s390x/cpu_features.c
+++ b/target/s390x/cpu_features.c
@@ -218,6 +218,9 @@ void s390_get_deprecated_features(S390FeatBitmap features)
  /* CSSKE is deprecated on newer generations */
  S390_FEAT_CONDITIONAL_SSKE,
  S390_FEAT_BPB,
+ /* Deprecated on z16 */
+ S390_FEAT_CONSTRAINT_TRANSACTIONAL_EXE,
+ S390_FEAT_TRANSACTIONAL_EXE
 };
 int i;
 
-- 
2.45.0

[PULL 1/8] hw/s390x: Attach the sclpconsole to /machine/sclp/s390-sclp-event-facility

[Thomas Huth]

The sclpconsole currently does not have a proper parent in the QOM
tree, so it shows up under /machine/unattached - which is somewhat
ugly. We should rather attach it to /machine/sclp/s390-sclp-event-facility
where the other devices of type TYPE_SCLP_EVENT already reside.

Message-ID: <20240430190843.453903-1-th...@redhat.com>
Reviewed-by: Eric Farman 
Reviewed-by: Cédric Le Goater 
Reviewed-by: David Hildenbrand 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Thomas Huth 
---
 hw/s390x/s390-virtio-ccw.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 4dcc213820..726c2ab436 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -239,11 +239,13 @@ static void s390_create_virtio_net(BusState *bus, const 
char *name)
 
 static void s390_create_sclpconsole(const char *type, Chardev *chardev)
 {
+BusState *ev_fac_bus = sclp_get_event_facility_bus();
 DeviceState *dev;
 
 dev = qdev_new(type);
+object_property_add_child(OBJECT(ev_fac_bus->parent), type, OBJECT(dev));
 qdev_prop_set_chr(dev, "chardev", chardev);
-qdev_realize_and_unref(dev, sclp_get_event_facility_bus(), _fatal);
+qdev_realize_and_unref(dev, ev_fac_bus, _fatal);
 }
 
 static void ccw_init(MachineState *machine)
-- 
2.45.0

[PULL 4/8] s390x/sclp: Simplify get_sclp_device()

[Thomas Huth]

From: Cédric Le Goater 

get_sclp_device() scans the whole machine to find a TYPE_SCLP object.
Now that the SCLPDevice instance is available under the machine state,
use it to simplify the lookup. While at it, remove the inline to let
the compiler decide on how to optimize.

Signed-off-by: Cédric Le Goater 
Message-ID: <20240502131533.377719-4-...@redhat.com>
Reviewed-by: Thomas Huth 
Signed-off-by: Thomas Huth 
---
 hw/s390x/sclp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index d236dbaf0b..e725dcd5fd 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -21,13 +21,14 @@
 #include "hw/s390x/s390-pci-bus.h"
 #include "hw/s390x/ipl.h"
 #include "hw/s390x/cpu-topology.h"
+#include "hw/s390x/s390-virtio-ccw.h"
 
-static inline SCLPDevice *get_sclp_device(void)
+static SCLPDevice *get_sclp_device(void)
 {
 static SCLPDevice *sclp;
 
 if (!sclp) {
-sclp = SCLP(object_resolve_path_type("", TYPE_SCLP, NULL));
+sclp = S390_CCW_MACHINE(qdev_get_machine())->sclp;
 }
 return sclp;
 }
-- 
2.45.0

[PULL 7/8] qemu-options: Deprecate "-runas" and introduce "-run-with user=..." instead

[Thomas Huth]

The old "-runas" option has the disadvantage that it is not visible
in the QAPI schema, so it is not available via the normal introspection
mechanisms. We've recently introduced the "-run-with" option for exactly
this purpose, which is meant to handle the options that affect the
runtime behavior. Thus let's introduce a "user=..." parameter here now
and deprecate the old "-runas" option.

Message-ID: <20240506112058.51446-1-th...@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Thomas Huth 
---
 docs/about/deprecated.rst |  6 ++
 system/vl.c   | 15 +++
 qemu-options.hx   | 15 +++
 3 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index e22acb17f2..6f709746db 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -61,6 +61,12 @@ configurations (e.g. -smp drawers=1,books=1,clusters=1 for 
x86 PC machine) is
 marked deprecated since 9.0, users have to ensure that all the topology members
 described with -smp are supported by the target machine.
 
+``-runas`` (since 9.1)
+--
+
+Use ``-run-with user=..`` instead.
+
+
 User-mode emulator command line arguments
 -
 
diff --git a/system/vl.c b/system/vl.c
index 7756eac81e..b031427440 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -773,6 +773,10 @@ static QemuOptsList qemu_run_with_opts = {
 .name = "chroot",
 .type = QEMU_OPT_STRING,
 },
+{
+.name = "user",
+.type = QEMU_OPT_STRING,
+},
 { /* end of list */ }
 },
 };
@@ -3586,6 +3590,7 @@ void qemu_init(int argc, char **argv)
 break;
 #if defined(CONFIG_POSIX)
 case QEMU_OPTION_runas:
+warn_report("-runas is deprecated, use '-run-with user=...' 
instead");
 if (!os_set_runas(optarg)) {
 error_report("User \"%s\" doesn't exist"
  " (and is not :)",
@@ -3612,6 +3617,16 @@ void qemu_init(int argc, char **argv)
 if (str) {
 os_set_chroot(str);
 }
+str = qemu_opt_get(opts, "user");
+if (str) {
+if (!os_set_runas(str)) {
+error_report("User \"%s\" doesn't exist"
+ " (and is not :)",
+ optarg);
+exit(1);
+}
+}
+
 break;
 }
 #endif /* CONFIG_POSIX */
diff --git a/qemu-options.hx b/qemu-options.hx
index cf61f6b863..f5c01eeeb4 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4824,7 +4824,8 @@ DEF("runas", HAS_ARG, QEMU_OPTION_runas, \
 SRST
 ``-runas user``
 Immediately before starting guest execution, drop root privileges,
-switching to the specified user.
+switching to the specified user. This option is deprecated, use
+``-run-with user=...`` instead.
 ERST
 
 DEF("prom-env", HAS_ARG, QEMU_OPTION_prom_env,
@@ -4990,13 +4991,15 @@ DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log, "", 
QEMU_ARCH_ALL)
 
 #ifdef CONFIG_POSIX
 DEF("run-with", HAS_ARG, QEMU_OPTION_run_with,
-"-run-with [async-teardown=on|off][,chroot=dir]\n"
+"-run-with [async-teardown=on|off][,chroot=dir][user=username|uid:gid]\n"
 "Set miscellaneous QEMU process lifecycle options:\n"
 "async-teardown=on enables asynchronous teardown (Linux 
only)\n"
-"chroot=dir chroot to dir just before starting the VM\n",
+"chroot=dir chroot to dir just before starting the VM\n"
+"user=username switch to the specified user before 
starting the VM\n"
+"user=uid:gid ditto, but use specified user-ID and 
group-ID instead\n",
 QEMU_ARCH_ALL)
 SRST
-``-run-with [async-teardown=on|off][,chroot=dir]``
+``-run-with [async-teardown=on|off][,chroot=dir][user=username|uid:gid]``
 Set QEMU process lifecycle options.
 
 ``async-teardown=on`` enables asynchronous teardown. A new process called
@@ -5013,6 +5016,10 @@ SRST
 ``chroot=dir`` can be used for doing a chroot to the specified directory
 immediately before starting the guest execution. This is especially useful
 in combination with -runas.
+
+``user=username`` or ``user=uid:gid`` can be used to drop root privileges
+by switching to the specified user (via username) or user and group
+(via uid:gid) immediately before starting guest execution.
 ERST
 #endif
 
-- 
2.45.0

[PULL 0/8] s390x and misc patches

[Thomas Huth]

The following changes since commit 36fa7c686e9eac490002ffc439c4affaa352c17c:

  gitlab: Update msys2-64bit runner tags (2024-05-09 05:46:21 +0200)

are available in the Git repository at:

  https://gitlab.com/thuth/qemu.git tags/pull-request-2024-05-10

for you to fetch changes up to 0d497106a71a3b17b0228cb87922ef794296cb24:

  tests/qtest: Add some test cases support on LoongArch (2024-05-10 08:34:29 
+0200)


* Attach s390x sclpconsole to a proper parent in the QOM tree
* SCLP related clean-ups
* Report deprecated-props in cpu-model-expansion reply on s390x
* Deprecate "-runas" and introduce "-run-with user=..." instead
* Add some more qtest cases on LoongArch

NB: I'm seeing test failures with "tsan-build" job in my CI, but
they also occur on the master branch in my repository, so
I assume they are not related to the patches in this PR.


Bibo Mao (1):
  tests/qtest: Add some test cases support on LoongArch

Collin L. Walling (2):
  target/s390x: report deprecated-props in cpu-model-expansion reply
  target/s390x: flag te and cte as deprecated

Cédric Le Goater (3):
  s390x: Introduce a SCLPDevice pointer under the machine
  s390x/event-facility: Simplify sclp_get_event_facility_bus()
  s390x/sclp: Simplify get_sclp_device()

Thomas Huth (2):
  hw/s390x: Attach the sclpconsole to /machine/sclp/s390-sclp-event-facility
  qemu-options: Deprecate "-runas" and introduce "-run-with user=..." 
instead

 docs/about/deprecated.rst  |  6 ++
 qapi/machine-target.json   |  7 ++-
 include/hw/s390x/event-facility.h  |  2 +-
 include/hw/s390x/s390-virtio-ccw.h |  3 +++
 include/hw/s390x/sclp.h|  2 --
 target/s390x/cpu_features.h|  1 +
 hw/s390x/event-facility.c  | 13 ++---
 hw/s390x/s390-virtio-ccw.c | 18 +-
 hw/s390x/sclp.c| 15 +++
 system/vl.c| 15 +++
 target/s390x/cpu_features.c| 17 +
 target/s390x/cpu_models_sysemu.c   |  8 
 tests/qtest/boot-serial-test.c | 10 ++
 qemu-options.hx| 15 +++
 tests/qtest/meson.build|  3 +++
 15 files changed, 99 insertions(+), 36 deletions(-)

[PULL 5/8] target/s390x: report deprecated-props in cpu-model-expansion reply

[Thomas Huth]

From: Collin Walling 

Retain a list of deprecated features disjoint from any particular
CPU model. A query-cpu-model-expansion reply will now provide a list of
properties (i.e. features) that are flagged as deprecated. Example:

{
  "return": {
"model": {
  "name": "z14.2-base",
  "deprecated-props": [
"bpb",
"csske"
  ],
  "props": {
"pfmfi": false,
"exrl": true,
...a lot more props...
"skey": false,
"vxpdeh2": false
  }
}
  }
}

It is recommended that s390 guests operate with these features
explicitly disabled to ensure compatibility with future hardware.

Signed-off-by: Collin Walling 
Acked-by: Markus Armbruster 
Reviewed-by: David Hildenbrand 
Message-ID: <20240429191059.11806-2-wall...@linux.ibm.com>
Signed-off-by: Thomas Huth 
---
 qapi/machine-target.json |  7 ++-
 target/s390x/cpu_features.h  |  1 +
 target/s390x/cpu_features.c  | 14 ++
 target/s390x/cpu_models_sysemu.c |  8 
 4 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index 29e695aa06..2942853092 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -20,11 +20,16 @@
 #
 # @props: a dictionary of QOM properties to be applied
 #
+# @deprecated-props: a list of properties that are flagged as deprecated
+# by the CPU vendor.  These props are a subset of the full model's
+# definition list of properties. (since 9.1)
+#
 # Since: 2.8
 ##
 { 'struct': 'CpuModelInfo',
   'data': { 'name': 'str',
-'*props': 'any' } }
+'*props': 'any',
+'*deprecated-props': ['str'] } }
 
 ##
 # @CpuModelExpansionType:
diff --git a/target/s390x/cpu_features.h b/target/s390x/cpu_features.h
index a9bd68a2e1..661a8cd6db 100644
--- a/target/s390x/cpu_features.h
+++ b/target/s390x/cpu_features.h
@@ -69,6 +69,7 @@ void s390_add_from_feat_block(S390FeatBitmap features, 
S390FeatType type,
   uint8_t *data);
 void s390_feat_bitmap_to_ascii(const S390FeatBitmap features, void *opaque,
void (*fn)(const char *name, void *opaque));
+void s390_get_deprecated_features(S390FeatBitmap features);
 
 /* Definition of a CPU feature group */
 typedef struct {
diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
index d28eb65845..efafc9711c 100644
--- a/target/s390x/cpu_features.c
+++ b/target/s390x/cpu_features.c
@@ -212,6 +212,20 @@ void s390_feat_bitmap_to_ascii(const S390FeatBitmap 
features, void *opaque,
 };
 }
 
+void s390_get_deprecated_features(S390FeatBitmap features)
+{
+static const int feats[] = {
+ /* CSSKE is deprecated on newer generations */
+ S390_FEAT_CONDITIONAL_SSKE,
+ S390_FEAT_BPB,
+};
+int i;
+
+for (i = 0; i < ARRAY_SIZE(feats); i++) {
+set_bit(feats[i], features);
+}
+}
+
 #define FEAT_GROUP_INIT(_name, _group, _desc)\
 {\
 .name = _name,   \
diff --git a/target/s390x/cpu_models_sysemu.c b/target/s390x/cpu_models_sysemu.c
index 15be729c3d..977fbc6522 100644
--- a/target/s390x/cpu_models_sysemu.c
+++ b/target/s390x/cpu_models_sysemu.c
@@ -206,6 +206,14 @@ static void cpu_info_from_model(CpuModelInfo *info, const 
S390CPUModel *model,
 } else {
 info->props = QOBJECT(qdict);
 }
+
+/* features flagged as deprecated */
+bitmap_zero(bitmap, S390_FEAT_MAX);
+s390_get_deprecated_features(bitmap);
+
+bitmap_and(bitmap, bitmap, model->def->full_feat, S390_FEAT_MAX);
+s390_feat_bitmap_to_ascii(bitmap, >deprecated_props, list_add_feat);
+info->has_deprecated_props = !!info->deprecated_props;
 }
 
 CpuModelExpansionInfo *qmp_query_cpu_model_expansion(CpuModelExpansionType 
type,
-- 
2.45.0

[PULL 2/8] s390x: Introduce a SCLPDevice pointer under the machine

[Thomas Huth]

From: Cédric Le Goater 

Initialize directly SCLPDevice from the machine init handler and
remove s390_sclp_init(). We will use the SCLPDevice pointer later to
create the consoles.

Signed-off-by: Cédric Le Goater 
Message-ID: <20240502131533.377719-2-...@redhat.com>
Reviewed-by: Thomas Huth 
Signed-off-by: Thomas Huth 
---
 include/hw/s390x/s390-virtio-ccw.h |  3 +++
 include/hw/s390x/sclp.h|  2 --
 hw/s390x/s390-virtio-ccw.c |  6 +-
 hw/s390x/sclp.c| 10 --
 4 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/include/hw/s390x/s390-virtio-ccw.h 
b/include/hw/s390x/s390-virtio-ccw.h
index c1d46e78af..7605d06bff 100644
--- a/include/hw/s390x/s390-virtio-ccw.h
+++ b/include/hw/s390x/s390-virtio-ccw.h
@@ -13,6 +13,7 @@
 
 #include "hw/boards.h"
 #include "qom/object.h"
+#include "hw/s390x/sclp.h"
 
 #define TYPE_S390_CCW_MACHINE   "s390-ccw-machine"
 
@@ -28,6 +29,8 @@ struct S390CcwMachineState {
 bool dea_key_wrap;
 bool pv;
 uint8_t loadparm[8];
+
+SCLPDevice *sclp;
 };
 
 #define S390_PTF_REASON_NONE (0x00 << 8)
diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
index b405a387b6..d32f6180e0 100644
--- a/include/hw/s390x/sclp.h
+++ b/include/hw/s390x/sclp.h
@@ -221,8 +221,6 @@ static inline int sccb_data_len(SCCB *sccb)
 return be16_to_cpu(sccb->h.length) - sizeof(sccb->h);
 }
 
-
-void s390_sclp_init(void);
 void sclp_service_interrupt(uint32_t sccb);
 void raise_irq_cpu_hotplug(void);
 int sclp_service_call(S390CPU *cpu, uint64_t sccb, uint32_t code);
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 726c2ab436..159f67d05c 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -251,11 +251,15 @@ static void s390_create_sclpconsole(const char *type, 
Chardev *chardev)
 static void ccw_init(MachineState *machine)
 {
 MachineClass *mc = MACHINE_GET_CLASS(machine);
+S390CcwMachineState *ms = S390_CCW_MACHINE(machine);
 int ret;
 VirtualCssBus *css_bus;
 DeviceState *dev;
 
-s390_sclp_init();
+ms->sclp = SCLP(object_new(TYPE_SCLP));
+object_property_add_child(OBJECT(machine), TYPE_SCLP, OBJECT(ms->sclp));
+qdev_realize_and_unref(DEVICE(ms->sclp), NULL, _fatal);
+
 /* init memory + setup max page size. Required for the CPU model */
 s390_memory_init(machine->ram);
 
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 893e71a41b..d236dbaf0b 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -378,16 +378,6 @@ void sclp_service_interrupt(uint32_t sccb)
 }
 
 /* qemu object creation and initialization functions */
-
-void s390_sclp_init(void)
-{
-Object *new = object_new(TYPE_SCLP);
-
-object_property_add_child(qdev_get_machine(), TYPE_SCLP, new);
-object_unref(new);
-qdev_realize(DEVICE(new), NULL, _fatal);
-}
-
 static void sclp_realize(DeviceState *dev, Error **errp)
 {
 MachineState *machine = MACHINE(qdev_get_machine());
-- 
2.45.0

[PULL 3/8] s390x/event-facility: Simplify sclp_get_event_facility_bus()

[Thomas Huth]

From: Cédric Le Goater 

sclp_get_event_facility_bus() scans the whole machine to find a
TYPE_SCLP_EVENTS_BUS object. The SCLPDevice instance is now available
under the machine state, use it to simplify the lookup and adjust the
creation of the consoles.

Signed-off-by: Cédric Le Goater 
Message-ID: <20240502131533.377719-3-...@redhat.com>
Reviewed-by: Thomas Huth 
Signed-off-by: Thomas Huth 
---
 include/hw/s390x/event-facility.h |  2 +-
 hw/s390x/event-facility.c | 13 ++---
 hw/s390x/s390-virtio-ccw.c| 12 +++-
 3 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/include/hw/s390x/event-facility.h 
b/include/hw/s390x/event-facility.h
index 3ffd575d8f..ff874e792d 100644
--- a/include/hw/s390x/event-facility.h
+++ b/include/hw/s390x/event-facility.h
@@ -203,6 +203,6 @@ struct SCLPEventFacilityClass {
 bool (*event_pending)(SCLPEventFacility *ef);
 };
 
-BusState *sclp_get_event_facility_bus(void);
+BusState *sclp_get_event_facility_bus(SCLPEventFacility *ef);
 
 #endif
diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index f9829de953..06c1da0ece 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -523,16 +523,7 @@ static void register_types(void)
 
 type_init(register_types)
 
-BusState *sclp_get_event_facility_bus(void)
+BusState *sclp_get_event_facility_bus(SCLPEventFacility *ef)
 {
-Object *busobj;
-SCLPEventsBus *sbus;
-
-busobj = object_resolve_path_type("", TYPE_SCLP_EVENTS_BUS, NULL);
-sbus = OBJECT_CHECK(SCLPEventsBus, busobj, TYPE_SCLP_EVENTS_BUS);
-if (!sbus) {
-return NULL;
-}
-
-return >qbus;
+return BUS(>sbus);
 }
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 159f67d05c..2afaf45ce6 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -237,13 +237,15 @@ static void s390_create_virtio_net(BusState *bus, const 
char *name)
 }
 }
 
-static void s390_create_sclpconsole(const char *type, Chardev *chardev)
+static void s390_create_sclpconsole(SCLPDevice *sclp,
+const char *type, Chardev *chardev)
 {
-BusState *ev_fac_bus = sclp_get_event_facility_bus();
+SCLPEventFacility *ef = sclp->event_facility;
+BusState *ev_fac_bus = sclp_get_event_facility_bus(ef);
 DeviceState *dev;
 
 dev = qdev_new(type);
-object_property_add_child(OBJECT(ev_fac_bus->parent), type, OBJECT(dev));
+object_property_add_child(OBJECT(ef), type, OBJECT(dev));
 qdev_prop_set_chr(dev, "chardev", chardev);
 qdev_realize_and_unref(dev, ev_fac_bus, _fatal);
 }
@@ -308,10 +310,10 @@ static void ccw_init(MachineState *machine)
 
 /* init consoles */
 if (serial_hd(0)) {
-s390_create_sclpconsole("sclpconsole", serial_hd(0));
+s390_create_sclpconsole(ms->sclp, "sclpconsole", serial_hd(0));
 }
 if (serial_hd(1)) {
-s390_create_sclpconsole("sclplmconsole", serial_hd(1));
+s390_create_sclpconsole(ms->sclp, "sclplmconsole", serial_hd(1));
 }
 
 /* init the TOD clock */
-- 
2.45.0

Re: [PATCH v4 12/12] tests/qtest/vhost-user-test: add a test case for memory-backend-shm

[Thomas Huth]

On 08/05/2024 09.44, Stefano Garzarella wrote:

`memory-backend-shm` can be used with vhost-user devices, so let's
add a new test case for it.

Signed-off-by: Stefano Garzarella 
---
  tests/qtest/vhost-user-test.c | 23 +++
  1 file changed, 23 insertions(+)

diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
index d4e437265f..8c1d903b2a 100644
--- a/tests/qtest/vhost-user-test.c
+++ b/tests/qtest/vhost-user-test.c
@@ -44,6 +44,8 @@
  "mem-path=%s,share=on -numa node,memdev=mem"
  #define QEMU_CMD_MEMFD  " -m %d -object 
memory-backend-memfd,id=mem,size=%dM," \
  " -numa node,memdev=mem"
+#define QEMU_CMD_SHM" -m %d -object memory-backend-shm,id=mem,size=%dM," \
+" -numa node,memdev=mem"
  #define QEMU_CMD_CHR" -chardev socket,id=%s,path=%s%s"
  #define QEMU_CMD_NETDEV " -netdev vhost-user,id=hs0,chardev=%s,vhostforce=on"
  
@@ -195,6 +197,7 @@ enum test_memfd {

  TEST_MEMFD_AUTO,
  TEST_MEMFD_YES,
  TEST_MEMFD_NO,
+TEST_MEMFD_SHM,
  };
  
  static void append_vhost_net_opts(TestServer *s, GString *cmd_line,

@@ -228,6 +231,8 @@ static void append_mem_opts(TestServer *server, GString 
*cmd_line,
  
  if (memfd == TEST_MEMFD_YES) {

  g_string_append_printf(cmd_line, QEMU_CMD_MEMFD, size, size);
+} else if (memfd == TEST_MEMFD_SHM) {
+g_string_append_printf(cmd_line, QEMU_CMD_SHM, size, size);
  } else {
  const char *root = init_hugepagefs() ? : server->tmpfs;
  
@@ -788,6 +793,19 @@ static void *vhost_user_test_setup_memfd(GString *cmd_line, void *arg)

  return server;
  }
  
+static void *vhost_user_test_setup_shm(GString *cmd_line, void *arg)

+{
+TestServer *server = test_server_new("vhost-user-test", arg);
+test_server_listen(server);
+
+append_mem_opts(server, cmd_line, 256, TEST_MEMFD_SHM);
+server->vu_ops->append_opts(server, cmd_line, "");
+
+g_test_queue_destroy(vhost_user_test_cleanup, server);
+
+return server;
+}
+
  static void test_read_guest_mem(void *obj, void *arg, QGuestAllocator *alloc)
  {
  TestServer *server = arg;
@@ -1081,6 +1099,11 @@ static void register_vhost_user_test(void)
   "virtio-net",
   test_read_guest_mem, );
  
+opts.before = vhost_user_test_setup_shm;

+qos_add_test("vhost-user/read-guest-mem/shm",
+ "virtio-net",
+ test_read_guest_mem, );
+
  if (qemu_memfd_check(MFD_ALLOW_SEALING)) {
  opts.before = vhost_user_test_setup_memfd;
  qos_add_test("vhost-user/read-guest-mem/memfd",


Acked-by: Thomas Huth 

Re: [PATCH v4 12/12] tests/qtest/vhost-user-test: add a test case for memory-backend-shm

[Thomas Huth]

On 08/05/2024 09.44, Stefano Garzarella wrote:

`memory-backend-shm` can be used with vhost-user devices, so let's
add a new test case for it.

Signed-off-by: Stefano Garzarella 
---
  tests/qtest/vhost-user-test.c | 23 +++
  1 file changed, 23 insertions(+)

diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
index d4e437265f..8c1d903b2a 100644
--- a/tests/qtest/vhost-user-test.c
+++ b/tests/qtest/vhost-user-test.c
@@ -44,6 +44,8 @@
  "mem-path=%s,share=on -numa node,memdev=mem"
  #define QEMU_CMD_MEMFD  " -m %d -object 
memory-backend-memfd,id=mem,size=%dM," \
  " -numa node,memdev=mem"
+#define QEMU_CMD_SHM" -m %d -object memory-backend-shm,id=mem,size=%dM," \
+" -numa node,memdev=mem"
  #define QEMU_CMD_CHR" -chardev socket,id=%s,path=%s%s"
  #define QEMU_CMD_NETDEV " -netdev vhost-user,id=hs0,chardev=%s,vhostforce=on"
  
@@ -195,6 +197,7 @@ enum test_memfd {

  TEST_MEMFD_AUTO,
  TEST_MEMFD_YES,
  TEST_MEMFD_NO,
+TEST_MEMFD_SHM,
  };
  
  static void append_vhost_net_opts(TestServer *s, GString *cmd_line,

@@ -228,6 +231,8 @@ static void append_mem_opts(TestServer *server, GString 
*cmd_line,
  
  if (memfd == TEST_MEMFD_YES) {

  g_string_append_printf(cmd_line, QEMU_CMD_MEMFD, size, size);
+} else if (memfd == TEST_MEMFD_SHM) {
+g_string_append_printf(cmd_line, QEMU_CMD_SHM, size, size);
  } else {
  const char *root = init_hugepagefs() ? : server->tmpfs;
  
@@ -788,6 +793,19 @@ static void *vhost_user_test_setup_memfd(GString *cmd_line, void *arg)

  return server;
  }
  
+static void *vhost_user_test_setup_shm(GString *cmd_line, void *arg)

+{
+TestServer *server = test_server_new("vhost-user-test", arg);
+test_server_listen(server);
+
+append_mem_opts(server, cmd_line, 256, TEST_MEMFD_SHM);
+server->vu_ops->append_opts(server, cmd_line, "");
+
+g_test_queue_destroy(vhost_user_test_cleanup, server);
+
+return server;
+}
+
  static void test_read_guest_mem(void *obj, void *arg, QGuestAllocator *alloc)
  {
  TestServer *server = arg;
@@ -1081,6 +1099,11 @@ static void register_vhost_user_test(void)
   "virtio-net",
   test_read_guest_mem, );
  
+opts.before = vhost_user_test_setup_shm;

+qos_add_test("vhost-user/read-guest-mem/shm",
+ "virtio-net",
+ test_read_guest_mem, );
+
  if (qemu_memfd_check(MFD_ALLOW_SEALING)) {
  opts.before = vhost_user_test_setup_memfd;
  qos_add_test("vhost-user/read-guest-mem/memfd",


Acked-by: Thomas Huth 

Re: [PATCH v4 11/12] tests/qtest/vhost-user-blk-test: use memory-backend-shm

[Thomas Huth]

On 08/05/2024 09.44, Stefano Garzarella wrote:

`memory-backend-memfd` is available only on Linux while the new
`memory-backend-shm` can be used on any POSIX-compliant operating
system. Let's use it so we can run the test in multiple environments.

Signed-off-by: Stefano Garzarella 
---
  tests/qtest/vhost-user-blk-test.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qtest/vhost-user-blk-test.c 
b/tests/qtest/vhost-user-blk-test.c
index 117b9acd10..e945f6abf2 100644
--- a/tests/qtest/vhost-user-blk-test.c
+++ b/tests/qtest/vhost-user-blk-test.c
@@ -906,7 +906,7 @@ static void start_vhost_user_blk(GString *cmd_line, int 
vus_instances,
 vhost_user_blk_bin);
  
  g_string_append_printf(cmd_line,

-" -object memory-backend-memfd,id=mem,size=256M,share=on "
+" -object memory-backend-shm,id=mem,size=256M,share=on "
  " -M memory-backend=mem -m 256M ");
  
  for (i = 0; i < vus_instances; i++) {


Acked-by: Thomas Huth 

Re: [PATCH v4 11/12] tests/qtest/vhost-user-blk-test: use memory-backend-shm

[Thomas Huth]

On 08/05/2024 09.44, Stefano Garzarella wrote:

`memory-backend-memfd` is available only on Linux while the new
`memory-backend-shm` can be used on any POSIX-compliant operating
system. Let's use it so we can run the test in multiple environments.

Signed-off-by: Stefano Garzarella 
---
  tests/qtest/vhost-user-blk-test.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qtest/vhost-user-blk-test.c 
b/tests/qtest/vhost-user-blk-test.c
index 117b9acd10..e945f6abf2 100644
--- a/tests/qtest/vhost-user-blk-test.c
+++ b/tests/qtest/vhost-user-blk-test.c
@@ -906,7 +906,7 @@ static void start_vhost_user_blk(GString *cmd_line, int 
vus_instances,
 vhost_user_blk_bin);
  
  g_string_append_printf(cmd_line,

-" -object memory-backend-memfd,id=mem,size=256M,share=on "
+" -object memory-backend-shm,id=mem,size=256M,share=on "
  " -M memory-backend=mem -m 256M ");
  
  for (i = 0; i < vus_instances; i++) {


Acked-by: Thomas Huth 

Re: [PATCH 13/13] tests/qtest: arm: fix operation in a build without any boards or devices

[Thomas Huth]

On 09/05/2024 19.00, Paolo Bonzini wrote:

ARM/aarch64 are easy to fix because they already have to pass a machine
type by hand.  Just guard the tests with a check that the machine actually
exists.

Signed-off-by: Paolo Bonzini 
---
  tests/qtest/arm-cpu-features.c | 4 
  tests/qtest/migration-test.c   | 6 ++
  tests/qtest/numa-test.c| 4 
  3 files changed, 14 insertions(+)



Reviewed-by: Thomas Huth 

Re: [PATCH 03/13] s390: move css_migration_enabled from machine to css.c

[Thomas Huth]

On 09/05/2024 19.00, Paolo Bonzini wrote:

The CSS subsystem uses global variables, just face the truth and use
a variable also for whether the CSS vmstate is in use; remove the
indirection of fetching it from the machine type, which makes the
TCG code depend unnecessarily on the virtio-ccw machine.

Signed-off-by: Paolo Bonzini 
---
  include/hw/s390x/css.h |  6 ++
  include/hw/s390x/s390-virtio-ccw.h |  7 ---
  hw/s390x/css.c | 10 +++---
  hw/s390x/s390-virtio-ccw.c | 15 +++
  4 files changed, 16 insertions(+), 22 deletions(-)

diff --git a/include/hw/s390x/css.h b/include/hw/s390x/css.h
index ba72ee3dd20..8289e458370 100644
--- a/include/hw/s390x/css.h
+++ b/include/hw/s390x/css.h
@@ -333,4 +333,10 @@ static inline int ccw_dstream_read_buf(CcwDataStream *cds, 
void *buff, int len)
  #define ccw_dstream_read(cds, v) ccw_dstream_read_buf((cds), &(v), sizeof(v))
  #define ccw_dstream_write(cds, v) ccw_dstream_write_buf((cds), &(v), 
sizeof(v))
  
+/**

+ * true if (vmstate based) migration of the channel subsystem
+ * is enabled, false if it is disabled.
+ */
+extern bool css_migration_enabled;
+
  #endif
diff --git a/include/hw/s390x/s390-virtio-ccw.h 
b/include/hw/s390x/s390-virtio-ccw.h
index c1d46e78af8..c0494e511cb 100644
--- a/include/hw/s390x/s390-virtio-ccw.h
+++ b/include/hw/s390x/s390-virtio-ccw.h
@@ -43,7 +43,6 @@ struct S390CcwMachineClass {
  /*< public >*/
  bool ri_allowed;
  bool cpu_model_allowed;
-bool css_migration_enabled;
  bool hpage_1m_allowed;
  int max_threads;
  };
@@ -55,10 +54,4 @@ bool cpu_model_allowed(void);
  /* 1M huge page mappings allowed by the machine */
  bool hpage_1m_allowed(void);
  
-/**

- * Returns true if (vmstate based) migration of the channel subsystem
- * is enabled, false if it is disabled.
- */
-bool css_migration_enabled(void);
-
  #endif
diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 295530963a6..b2d5327dbf4 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -23,6 +23,8 @@
  #include "hw/s390x/s390-virtio-ccw.h"
  #include "hw/s390x/s390-ccw.h"
  
+bool css_migration_enabled = true;

+
  typedef struct CrwContainer {
  CRW crw;
  QTAILQ_ENTRY(CrwContainer) sibling;
@@ -180,7 +182,7 @@ static const VMStateDescription vmstate_orb = {
  
  static bool vmstate_schdev_orb_needed(void *opaque)

  {
-return css_migration_enabled();
+return css_migration_enabled;
  }
  
  static const VMStateDescription vmstate_schdev_orb = {

@@ -388,7 +390,7 @@ static int subch_dev_post_load(void *opaque, int version_id)
  css_subch_assign(s->cssid, s->ssid, s->schid, s->devno, s);
  }
  
-if (css_migration_enabled()) {

+if (css_migration_enabled) {
  /* No compat voodoo to do ;) */
  return 0;
  }
@@ -412,7 +414,9 @@ static int subch_dev_post_load(void *opaque, int version_id)
  
  void css_register_vmstate(void)

  {
-vmstate_register(NULL, 0, _css, _subsys);
+if (css_migration_enabled) {
+vmstate_register(NULL, 0, _css, _subsys);
+}
  }
  
  IndAddr *get_indicator(hwaddr ind_addr, int len)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 1383e47eeb5..aa90703d518 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -275,11 +275,9 @@ static void ccw_init(MachineState *machine)
  s390_enable_css_support(s390_cpu_addr2state(0));
  
  ret = css_create_css_image(VIRTUAL_CSSID, true);

-
  assert(ret == 0);
-if (css_migration_enabled()) {
-css_register_vmstate();
-}
+
+css_register_vmstate();
  
  /* Create VirtIO network adapters */

  s390_create_virtio_net(BUS(css_bus), mc->default_nic);
@@ -741,7 +739,6 @@ static void ccw_machine_class_init(ObjectClass *oc, void 
*data)
  
  s390mc->ri_allowed = true;

  s390mc->cpu_model_allowed = true;
-s390mc->css_migration_enabled = true;
  s390mc->hpage_1m_allowed = true;
  s390mc->max_threads = 1;
  mc->init = ccw_init;
@@ -811,11 +808,6 @@ static const TypeInfo ccw_machine_info = {
  },
  };
  
-bool css_migration_enabled(void)

-{
-return get_machine_class()->css_migration_enabled;
-}
-
  #define DEFINE_CCW_MACHINE(suffix, verstr, latest)
\
  static void ccw_machine_##suffix##_class_init(ObjectClass *oc,
\
void *data) 
\
@@ -1171,7 +1163,6 @@ static void ccw_machine_2_9_instance_options(MachineState 
*machine)
  
  static void ccw_machine_2_9_class_options(MachineClass *mc)

  {
-S390CcwMachineClass *s390mc = S390_CCW_MACHINE_CLASS(mc);
  static GlobalProperty compat[] = {
  { TYPE_S390_STATTRIB, "migration-enabled", "off", },
  { TYPE_S390_FLIC_COMMON, "migration-enabled", "off", },
@@ -1180,7 +1171,7 @@ static void ccw_machine_2_9_class_options(MachineClass 
*mc)
  ccw_machine_2_10_class_options(mc);
  

Re: [PATCH 05/13] tests/qtest: s390x: fix operation in a build without any boards or devices

[Thomas Huth]

On 09/05/2024 19.00, Paolo Bonzini wrote:

Do the bare minimum to ensure that at least a vanilla
--without-default-devices build works for all targets except i386,
x86_64 and ppc64.  In particular this fixes s390x-softmmu; i386 and
x86_64 have about a dozen failing tests that do not pass -M and therefore
require a default machine type; ppc64 has the same issue, though only
with numa-test.

If we can for now ignore the cases where boards and devices are picked
by hand, drive_del-test however can be fixed easily; almost all tests
check for the virtio-blk or virtio-scsi device that they use, and are
already skipped.  Only one didn't get the memo; plus another one does
not need a machine at all and can be run with -M none.

Signed-off-by: Paolo Bonzini 
---
  tests/qtest/drive_del-test.c | 7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/drive_del-test.c b/tests/qtest/drive_del-test.c
index 8a6f3ac963d..7b67a4bbee4 100644
--- a/tests/qtest/drive_del-test.c
+++ b/tests/qtest/drive_del-test.c
@@ -173,7 +173,7 @@ static void test_drive_without_dev(void)
  QTestState *qts;
  
  /* Start with an empty drive */

-qts = qtest_init("-drive if=none,id=drive0");
+qts = qtest_init("-drive if=none,id=drive0 -M none");
  
  /* Delete the drive */

  drive_del(qts);
@@ -192,6 +192,11 @@ static void test_after_failed_device_add(void)
  QDict *response;
  QTestState *qts;
  
+if (!has_device_builtin("virtio-blk")) {

+g_test_skip("Device virtio-blk is not available");
+return;
+}
+
  snprintf(driver, sizeof(driver), "virtio-blk-%s",
   qvirtio_get_dev_type());
  


Reviewed-by: Thomas Huth 

Re: [PATCH 04/13] s390x: select correct components for no-board build

[Thomas Huth]

On 09/05/2024 19.00, Paolo Bonzini wrote:

Signed-off-by: Paolo Bonzini 
---
  .gitlab-ci.d/buildtest.yml | 4 ++--
  target/s390x/Kconfig   | 2 ++
  2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 13afd0df1f0..f8502905203 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -650,7 +650,7 @@ build-tci:
  # Check our reduced build configurations
  # requires libfdt: aarch64, arm, i386, loongarch64, microblaze, microblazeel,
  #   mips64el, or1k, ppc, ppc64, riscv32, riscv64, rx, x86_64
-# does not build without boards: i386, s390x, x86_64
+# does not build without boards: i386, x86_64
  build-without-defaults:
extends: .native_build_job_template
needs:
@@ -666,7 +666,7 @@ build-without-defaults:
--disable-strip
  TARGETS: alpha-softmmu avr-softmmu cris-softmmu hppa-softmmu m68k-softmmu
mips-softmmu mips64-softmmu mipsel-softmmu
-  sh4-softmmu sh4eb-softmmu sparc-softmmu
+  s390x-softmmu sh4-softmmu sh4eb-softmmu sparc-softmmu
sparc64-softmmu tricore-softmmu xtensa-softmmu xtensaeb-softmmu
hexagon-linux-user i386-linux-user s390x-linux-user
  MAKE_CHECK_ARGS: check
diff --git a/target/s390x/Kconfig b/target/s390x/Kconfig
index 72da48136c6..d886be48b47 100644
--- a/target/s390x/Kconfig
+++ b/target/s390x/Kconfig
@@ -1,2 +1,4 @@
  config S390X
  bool
+select PCI
+select S390_FLIC


Reviewed-by: Thomas Huth 

Re: [PATCH 02/13] s390_flic: add migration-enabled property

[Thomas Huth]

On 09/05/2024 19.00, Paolo Bonzini wrote:

Instead of mucking with css_migration_enabled(), add a property specific to
the FLIC device, similar to what is done for TYPE_S390_STATTRIB.

Signed-off-by: Paolo Bonzini 
---
  include/hw/s390x/s390_flic.h | 1 +
  hw/intc/s390_flic.c  | 6 +-
  hw/s390x/s390-virtio-ccw.c   | 1 +
  3 files changed, 7 insertions(+), 1 deletion(-)


Reviewed-by: Thomas Huth 

(BTW: It's really good that Daniel's patch series is going to mark the old 
machine types as deprecated, too ... migration stuff has been so hacky in 
the 2.x days...)



diff --git a/include/hw/s390x/s390_flic.h b/include/hw/s390x/s390_flic.h
index 3907a13d076..bcb081def58 100644
--- a/include/hw/s390x/s390_flic.h
+++ b/include/hw/s390x/s390_flic.h
@@ -47,6 +47,7 @@ struct S390FLICState {
  /* to limit AdapterRoutes.num_routes for compat */
  uint32_t adapter_routes_max_batch;
  bool ais_supported;
+bool migration_enabled;
  };
  
  
diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c

index f4a848460b8..7f930800877 100644
--- a/hw/intc/s390_flic.c
+++ b/hw/intc/s390_flic.c
@@ -405,6 +405,8 @@ static void qemu_s390_flic_class_init(ObjectClass *oc, void 
*data)
  static Property s390_flic_common_properties[] = {
  DEFINE_PROP_UINT32("adapter_routes_max_batch", S390FLICState,
 adapter_routes_max_batch, ADAPTER_ROUTES_MAX_GSI),
+DEFINE_PROP_BOOL("migration-enabled", S390FLICState,
+ migration_enabled, true),
  DEFINE_PROP_END_OF_LIST(),
  };
  
@@ -457,7 +459,9 @@ type_init(qemu_s390_flic_register_types)
  
  static bool adapter_info_so_needed(void *opaque)

  {
-return css_migration_enabled();
+S390FLICState *fs = S390_FLIC_COMMON(opaque);
+
+return fs->migration_enabled;
  }
  
  const VMStateDescription vmstate_adapter_info_so = {

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index feabc173eb3..1383e47eeb5 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -1174,6 +1174,7 @@ static void ccw_machine_2_9_class_options(MachineClass 
*mc)
  S390CcwMachineClass *s390mc = S390_CCW_MACHINE_CLASS(mc);
  static GlobalProperty compat[] = {
  { TYPE_S390_STATTRIB, "migration-enabled", "off", },
+{ TYPE_S390_FLIC_COMMON, "migration-enabled", "off", },
  };
  
  ccw_machine_2_10_class_options(mc);

Re: [PATCH 01/13] s390x: move s390_cpu_addr2state to target/s390x/sigp.c

[Thomas Huth]

On 09/05/2024 19.00, Paolo Bonzini wrote:

This function has no dependency on the virtio-ccw machine type, though it
assumes that the CPU address corresponds to the core_id and the index.

If there is any need of something different or more fancy (unlikely)
S390 can include a MachineClass subclass and implement it there.  For
now, move it to sigp.c for simplicity.

Signed-off-by: Paolo Bonzini 
---
  hw/s390x/s390-virtio-ccw.c | 16 
  target/s390x/sigp.c| 17 +
  2 files changed, 17 insertions(+), 16 deletions(-)


Reviewed-by: Thomas Huth 




diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 4dcc2138200..feabc173eb3 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -50,22 +50,6 @@
  
  static Error *pv_mig_blocker;
  
-S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)

-{
-static MachineState *ms;
-
-if (!ms) {
-ms = MACHINE(qdev_get_machine());
-g_assert(ms->possible_cpus);
-}
-
-/* CPU address corresponds to the core_id and the index */
-if (cpu_addr >= ms->possible_cpus->len) {
-return NULL;
-}
-return S390_CPU(ms->possible_cpus->cpus[cpu_addr].cpu);
-}
-
  static S390CPU *s390x_new_cpu(const char *typename, uint32_t core_id,
Error **errp)
  {
diff --git a/target/s390x/sigp.c b/target/s390x/sigp.c
index 9dd977349ab..ad0ad61177d 100644
--- a/target/s390x/sigp.c
+++ b/target/s390x/sigp.c
@@ -11,6 +11,7 @@
  #include "qemu/osdep.h"
  #include "cpu.h"
  #include "s390x-internal.h"
+#include "hw/boards.h"
  #include "sysemu/hw_accel.h"
  #include "sysemu/runstate.h"
  #include "exec/address-spaces.h"
@@ -435,6 +436,22 @@ static int sigp_set_architecture(S390CPU *cpu, uint32_t 
param,
  return SIGP_CC_STATUS_STORED;
  }
  
+S390CPU *s390_cpu_addr2state(uint16_t cpu_addr)

+{
+static MachineState *ms;
+
+if (!ms) {
+ms = MACHINE(qdev_get_machine());
+g_assert(ms->possible_cpus);
+}
+
+/* CPU address corresponds to the core_id and the index */
+if (cpu_addr >= ms->possible_cpus->len) {
+return NULL;
+}
+return S390_CPU(ms->possible_cpus->cpus[cpu_addr].cpu);
+}
+
  int handle_sigp(CPUS390XState *env, uint8_t order, uint64_t r1, uint64_t r3)
  {
  uint64_t *status_reg = >regs[r1];

Re: [PATCH v2] tests/qtest: Add some test cases support on LoongArch

[Thomas Huth]

On 09/05/2024 10.47, Bibo Mao wrote:

Add boot-serial-test and filter test cases support on LoongArch system.

Signed-off-by: Bibo Mao 
---
v1 ... v2:
  1. Refresh the changelog, adding filter test case support also.
  2. Adjust order of loongarch qtest in alphabetical order.
---



Reviewed-by: Thomas Huth 

Re: [PATCH v2] qemu-options: Deprecate "-runas" and introduce "-run-with user=..." instead

[Thomas Huth]

On 08/05/2024 15.20, Claudio Fontana wrote:

On 5/6/24 13:20, Thomas Huth wrote:

The old "-runas" option has the disadvantage that it is not visible
in the QAPI schema, so it is not available via the normal introspection
mechanisms. We've recently introduced the "-run-with" option for exactly
this purpose, which is meant to handle the options that affect the
runtime behavior. Thus let's introduce a "user=..." parameter here now
and deprecate the old "-runas" option.

Signed-off-by: Thomas Huth 
---
  v2: Add missing part in qemu-options.hx as suggested by Philippe

  docs/about/deprecated.rst |  6 ++
  system/vl.c   | 15 +++
  qemu-options.hx   | 15 +++
  3 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 3310df3274..fe69e2d44c 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -61,6 +61,12 @@ configurations (e.g. -smp drawers=1,books=1,clusters=1 for 
x86 PC machine) is
  marked deprecated since 9.0, users have to ensure that all the topology 
members
  described with -smp are supported by the target machine.
  
+``-runas`` (since 9.1)

+--
+
+Use ``-run-with user=..`` instead.
+
+
  User-mode emulator command line arguments
  -
  
diff --git a/system/vl.c b/system/vl.c

index 7756eac81e..b031427440 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -773,6 +773,10 @@ static QemuOptsList qemu_run_with_opts = {
  .name = "chroot",
  .type = QEMU_OPT_STRING,
  },
+{
+.name = "user",
+.type = QEMU_OPT_STRING,
+},
  { /* end of list */ }
  },
  };
@@ -3586,6 +3590,7 @@ void qemu_init(int argc, char **argv)
  break;
  #if defined(CONFIG_POSIX)
  case QEMU_OPTION_runas:
+warn_report("-runas is deprecated, use '-run-with user=...' 
instead");
  if (!os_set_runas(optarg)) {
  error_report("User \"%s\" doesn't exist"
   " (and is not :)",
@@ -3612,6 +3617,16 @@ void qemu_init(int argc, char **argv)
  if (str) {
  os_set_chroot(str);
  }
+str = qemu_opt_get(opts, "user");
+if (str) {
+if (!os_set_runas(str)) {
+error_report("User \"%s\" doesn't exist"
+ " (and is not :)",
+ optarg);
+exit(1);
+}
+}
+
  break;
  }
  #endif /* CONFIG_POSIX */
diff --git a/qemu-options.hx b/qemu-options.hx
index cf61f6b863..3031479a15 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4824,7 +4824,8 @@ DEF("runas", HAS_ARG, QEMU_OPTION_runas, \
  SRST
  ``-runas user``
  Immediately before starting guest execution, drop root privileges,
-switching to the specified user.
+switching to the specified user. This option is deprecated, use
+``-run-with user=...`` instead.
  ERST
  
  DEF("prom-env", HAS_ARG, QEMU_OPTION_prom_env,

@@ -4990,13 +4991,15 @@ DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log, "", 
QEMU_ARCH_ALL)
  
  #ifdef CONFIG_POSIX

  DEF("run-with", HAS_ARG, QEMU_OPTION_run_with,
-"-run-with [async-teardown=on|off][,chroot=dir]\n"
+"-run-with [async-teardown=on|off][,chroot=dir][user=username|uid:gid]\n"
  "Set miscellaneous QEMU process lifecycle options:\n"
  "async-teardown=on enables asynchronous teardown (Linux 
only)\n"
-"chroot=dir chroot to dir just before starting the VM\n",
+"chroot=dir chroot to dir just before starting the VM\n"
+"user=username switch to the specified user before starting the 
VM\n"
+"user=uid:gid dito, but use specified user-ID and group-ID 
instead\n",


nit: ditto?


Thanks, I'll fix it!

 Thomas

Re: [PATCH v2] qemu-options: Deprecate "-runas" and introduce "-run-with user=..." instead

[Thomas Huth]

On 08/05/2024 15.20, Claudio Fontana wrote:

On 5/6/24 13:20, Thomas Huth wrote:

The old "-runas" option has the disadvantage that it is not visible
in the QAPI schema, so it is not available via the normal introspection
mechanisms. We've recently introduced the "-run-with" option for exactly
this purpose, which is meant to handle the options that affect the
runtime behavior. Thus let's introduce a "user=..." parameter here now
and deprecate the old "-runas" option.

Signed-off-by: Thomas Huth 
---
  v2: Add missing part in qemu-options.hx as suggested by Philippe

  docs/about/deprecated.rst |  6 ++
  system/vl.c   | 15 +++
  qemu-options.hx   | 15 +++
  3 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 3310df3274..fe69e2d44c 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -61,6 +61,12 @@ configurations (e.g. -smp drawers=1,books=1,clusters=1 for 
x86 PC machine) is
  marked deprecated since 9.0, users have to ensure that all the topology 
members
  described with -smp are supported by the target machine.
  
+``-runas`` (since 9.1)

+--
+
+Use ``-run-with user=..`` instead.
+
+
  User-mode emulator command line arguments
  -
  
diff --git a/system/vl.c b/system/vl.c

index 7756eac81e..b031427440 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -773,6 +773,10 @@ static QemuOptsList qemu_run_with_opts = {
  .name = "chroot",
  .type = QEMU_OPT_STRING,
  },
+{
+.name = "user",
+.type = QEMU_OPT_STRING,
+},
  { /* end of list */ }
  },
  };
@@ -3586,6 +3590,7 @@ void qemu_init(int argc, char **argv)
  break;
  #if defined(CONFIG_POSIX)
  case QEMU_OPTION_runas:
+warn_report("-runas is deprecated, use '-run-with user=...' 
instead");
  if (!os_set_runas(optarg)) {
  error_report("User \"%s\" doesn't exist"
   " (and is not :)",
@@ -3612,6 +3617,16 @@ void qemu_init(int argc, char **argv)
  if (str) {
  os_set_chroot(str);
  }
+str = qemu_opt_get(opts, "user");
+if (str) {
+if (!os_set_runas(str)) {
+error_report("User \"%s\" doesn't exist"
+ " (and is not :)",
+ optarg);
+exit(1);
+}
+}
+
  break;
  }
  #endif /* CONFIG_POSIX */
diff --git a/qemu-options.hx b/qemu-options.hx
index cf61f6b863..3031479a15 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4824,7 +4824,8 @@ DEF("runas", HAS_ARG, QEMU_OPTION_runas, \
  SRST
  ``-runas user``
  Immediately before starting guest execution, drop root privileges,
-switching to the specified user.
+switching to the specified user. This option is deprecated, use
+``-run-with user=...`` instead.
  ERST
  
  DEF("prom-env", HAS_ARG, QEMU_OPTION_prom_env,

@@ -4990,13 +4991,15 @@ DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log, "", 
QEMU_ARCH_ALL)
  
  #ifdef CONFIG_POSIX

  DEF("run-with", HAS_ARG, QEMU_OPTION_run_with,
-"-run-with [async-teardown=on|off][,chroot=dir]\n"
+"-run-with [async-teardown=on|off][,chroot=dir][user=username|uid:gid]\n"
  "Set miscellaneous QEMU process lifecycle options:\n"
  "async-teardown=on enables asynchronous teardown (Linux 
only)\n"
-"chroot=dir chroot to dir just before starting the VM\n",
+"chroot=dir chroot to dir just before starting the VM\n"
+"user=username switch to the specified user before starting the 
VM\n"
+"user=uid:gid dito, but use specified user-ID and group-ID 
instead\n",


nit: ditto?


Thanks, I'll fix it!

 Thomas

___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org

Re: [kvm-unit-tests PATCH v9 07/31] scripts: allow machine option to be specified in unittests.cfg

[Thomas Huth]

On 08/05/2024 14.58, Thomas Huth wrote:

On 08/05/2024 14.55, Thomas Huth wrote:

On 08/05/2024 14.27, Nicholas Piggin wrote:

On Wed May 8, 2024 at 1:08 AM AEST, Thomas Huth wrote:

On 04/05/2024 14.28, Nicholas Piggin wrote:

This allows different machines with different requirements to be
supported by run_tests.sh, similarly to how different accelerators
are handled.

Acked-by: Thomas Huth 
Acked-by: Andrew Jones 
Signed-off-by: Nicholas Piggin 
---
   docs/unittests.txt   |  7 +++
   scripts/common.bash  |  8 ++--
   scripts/runtime.bash | 16 
   3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/docs/unittests.txt b/docs/unittests.txt
index 7cf2c55ad..6449efd78 100644
--- a/docs/unittests.txt
+++ b/docs/unittests.txt
@@ -42,6 +42,13 @@ For / directories that support multiple 
architectures, this restricts

   the test to the specified arch. By default, the test will run on any
   architecture.
+machine
+---
+For those architectures that support multiple machine types, this 
restricts

+the test to the specified machine. By default, the test will run on
+any machine type. (Note, the machine can be specified with the MACHINE=
+environment variable, and defaults to the architecture's default.)
+
   smp
   ---
   smp = 
diff --git a/scripts/common.bash b/scripts/common.bash
index 5e9ad53e2..3aa557c8c 100644
--- a/scripts/common.bash
+++ b/scripts/common.bash
@@ -10,6 +10,7 @@ function for_each_unittest()
   local opts
   local groups
   local arch
+    local machine
   local check
   local accel
   local timeout
@@ -21,7 +22,7 @@ function for_each_unittest()
   if [[ "$line" =~ ^\[(.*)\]$ ]]; then
   rematch=${BASH_REMATCH[1]}
   if [ -n "${testname}" ]; then
-    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" 
"$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" 
"$kernel" "$opts" "$arch" "$machine" "$check" "$accel" "$timeout"

   fi
   testname=$rematch
   smp=1
@@ -29,6 +30,7 @@ function for_each_unittest()
   opts=""
   groups=""
   arch=""
+    machine=""
   check=""
   accel=""
   timeout=""
@@ -58,6 +60,8 @@ function for_each_unittest()
   groups=${BASH_REMATCH[1]}
   elif [[ $line =~ ^arch\ *=\ *(.*)$ ]]; then
   arch=${BASH_REMATCH[1]}
+    elif [[ $line =~ ^machine\ *=\ *(.*)$ ]]; then
+    machine=${BASH_REMATCH[1]}
   elif [[ $line =~ ^check\ *=\ *(.*)$ ]]; then
   check=${BASH_REMATCH[1]}
   elif [[ $line =~ ^accel\ *=\ *(.*)$ ]]; then
@@ -67,7 +71,7 @@ function for_each_unittest()
   fi
   done
   if [ -n "${testname}" ]; then
-    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" 
"$opts" "$arch" "$check" "$accel" "$timeout"
+    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" 
"$opts" "$arch" "$machine" "$check" "$accel" "$timeout"

   fi
   exec {fd}<&-
   }
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 177b62166..0c96d6ea2 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -32,7 +32,7 @@ premature_failure()
   get_cmdline()
   {
   local kernel=$1
-    echo "TESTNAME=$testname TIMEOUT=$timeout ACCEL=$accel 
$RUNTIME_arch_run $kernel -smp $smp $opts"
+    echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine 
ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"

   }
   skip_nodefault()
@@ -80,9 +80,10 @@ function run()
   local kernel="$4"
   local opts="$5"
   local arch="$6"
-    local check="${CHECK:-$7}"
-    local accel="$8"
-    local timeout="${9:-$TIMEOUT}" # unittests.cfg overrides the default
+    local machine="$7"
+    local check="${CHECK:-$8}"
+    local accel="$9"
+    local timeout="${10:-$TIMEOUT}" # unittests.cfg overrides the default
   if [ "${CONFIG_EFI}" == "y" ]; then
   kernel=${kernel/%.flat/.efi}
@@ -116,6 +117,13 @@ function run()
   return 2
   fi
+    if [ -n "$machine" ] && [ -n "$MACHINE" ] && [ "$machine" != 
"$MACHINE" ]; then

+    print_result "SKI

Re: [kvm-unit-tests PATCH v9 07/31] scripts: allow machine option to be specified in unittests.cfg

[Thomas Huth]

On 08/05/2024 14.55, Thomas Huth wrote:

On 08/05/2024 14.27, Nicholas Piggin wrote:

On Wed May 8, 2024 at 1:08 AM AEST, Thomas Huth wrote:

On 04/05/2024 14.28, Nicholas Piggin wrote:

This allows different machines with different requirements to be
supported by run_tests.sh, similarly to how different accelerators
are handled.

Acked-by: Thomas Huth 
Acked-by: Andrew Jones 
Signed-off-by: Nicholas Piggin 
---
   docs/unittests.txt   |  7 +++
   scripts/common.bash  |  8 ++--
   scripts/runtime.bash | 16 
   3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/docs/unittests.txt b/docs/unittests.txt
index 7cf2c55ad..6449efd78 100644
--- a/docs/unittests.txt
+++ b/docs/unittests.txt
@@ -42,6 +42,13 @@ For / directories that support multiple 
architectures, this restricts

   the test to the specified arch. By default, the test will run on any
   architecture.
+machine
+---
+For those architectures that support multiple machine types, this 
restricts

+the test to the specified machine. By default, the test will run on
+any machine type. (Note, the machine can be specified with the MACHINE=
+environment variable, and defaults to the architecture's default.)
+
   smp
   ---
   smp = 
diff --git a/scripts/common.bash b/scripts/common.bash
index 5e9ad53e2..3aa557c8c 100644
--- a/scripts/common.bash
+++ b/scripts/common.bash
@@ -10,6 +10,7 @@ function for_each_unittest()
   local opts
   local groups
   local arch
+    local machine
   local check
   local accel
   local timeout
@@ -21,7 +22,7 @@ function for_each_unittest()
   if [[ "$line" =~ ^\[(.*)\]$ ]]; then
   rematch=${BASH_REMATCH[1]}
   if [ -n "${testname}" ]; then
-    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" 
"$kernel" "$opts" "$arch" "$check" "$accel" "$timeout"
+    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" 
"$kernel" "$opts" "$arch" "$machine" "$check" "$accel" "$timeout"

   fi
   testname=$rematch
   smp=1
@@ -29,6 +30,7 @@ function for_each_unittest()
   opts=""
   groups=""
   arch=""
+    machine=""
   check=""
   accel=""
   timeout=""
@@ -58,6 +60,8 @@ function for_each_unittest()
   groups=${BASH_REMATCH[1]}
   elif [[ $line =~ ^arch\ *=\ *(.*)$ ]]; then
   arch=${BASH_REMATCH[1]}
+    elif [[ $line =~ ^machine\ *=\ *(.*)$ ]]; then
+    machine=${BASH_REMATCH[1]}
   elif [[ $line =~ ^check\ *=\ *(.*)$ ]]; then
   check=${BASH_REMATCH[1]}
   elif [[ $line =~ ^accel\ *=\ *(.*)$ ]]; then
@@ -67,7 +71,7 @@ function for_each_unittest()
   fi
   done
   if [ -n "${testname}" ]; then
-    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" 
"$opts" "$arch" "$check" "$accel" "$timeout"
+    $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" 
"$opts" "$arch" "$machine" "$check" "$accel" "$timeout"

   fi
   exec {fd}<&-
   }
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 177b62166..0c96d6ea2 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -32,7 +32,7 @@ premature_failure()
   get_cmdline()
   {
   local kernel=$1
-    echo "TESTNAME=$testname TIMEOUT=$timeout ACCEL=$accel 
$RUNTIME_arch_run $kernel -smp $smp $opts"
+    echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine 
ACCEL=$accel $RUNTIME_arch_run $kernel -smp $smp $opts"

   }
   skip_nodefault()
@@ -80,9 +80,10 @@ function run()
   local kernel="$4"
   local opts="$5"
   local arch="$6"
-    local check="${CHECK:-$7}"
-    local accel="$8"
-    local timeout="${9:-$TIMEOUT}" # unittests.cfg overrides the default
+    local machine="$7"
+    local check="${CHECK:-$8}"
+    local accel="$9"
+    local timeout="${10:-$TIMEOUT}" # unittests.cfg overrides the default
   if [ "${CONFIG_EFI}" == "y" ]; then
   kernel=${kernel/%.flat/.efi}
@@ -116,6 +117,13 @@ function run()
   return 2
   fi
+    if [ -n "$machine" ] && [ -n "$MACHINE" ] && [ "$machine" != 
"$MACHINE" ]; then

+    print_result "SKIP" $testname "" "$m

Re: [kvm-unit-tests PATCH v9 07/31] scripts: allow machine option to be specified in unittests.cfg

[Thomas Huth]

On 08/05/2024 14.27, Nicholas Piggin wrote:

On Wed May 8, 2024 at 1:08 AM AEST, Thomas Huth wrote:

On 04/05/2024 14.28, Nicholas Piggin wrote:

This allows different machines with different requirements to be
supported by run_tests.sh, similarly to how different accelerators
are handled.

Acked-by: Thomas Huth 
Acked-by: Andrew Jones 
Signed-off-by: Nicholas Piggin 
---
   docs/unittests.txt   |  7 +++
   scripts/common.bash  |  8 ++--
   scripts/runtime.bash | 16 
   3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/docs/unittests.txt b/docs/unittests.txt
index 7cf2c55ad..6449efd78 100644
--- a/docs/unittests.txt
+++ b/docs/unittests.txt
@@ -42,6 +42,13 @@ For / directories that support multiple architectures, 
this restricts
   the test to the specified arch. By default, the test will run on any
   architecture.
   
+machine

+---
+For those architectures that support multiple machine types, this restricts
+the test to the specified machine. By default, the test will run on
+any machine type. (Note, the machine can be specified with the MACHINE=
+environment variable, and defaults to the architecture's default.)
+
   smp
   ---
   smp = 
diff --git a/scripts/common.bash b/scripts/common.bash
index 5e9ad53e2..3aa557c8c 100644
--- a/scripts/common.bash
+++ b/scripts/common.bash
@@ -10,6 +10,7 @@ function for_each_unittest()
local opts
local groups
local arch
+   local machine
local check
local accel
local timeout
@@ -21,7 +22,7 @@ function for_each_unittest()
if [[ "$line" =~ ^\[(.*)\]$ ]]; then
rematch=${BASH_REMATCH[1]}
if [ -n "${testname}" ]; then
-   $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" 
"$check" "$accel" "$timeout"
+   $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" 
"$machine" "$check" "$accel" "$timeout"
fi
testname=$rematch
smp=1
@@ -29,6 +30,7 @@ function for_each_unittest()
opts=""
groups=""
arch=""
+   machine=""
check=""
accel=""
timeout=""
@@ -58,6 +60,8 @@ function for_each_unittest()
groups=${BASH_REMATCH[1]}
elif [[ $line =~ ^arch\ *=\ *(.*)$ ]]; then
arch=${BASH_REMATCH[1]}
+   elif [[ $line =~ ^machine\ *=\ *(.*)$ ]]; then
+   machine=${BASH_REMATCH[1]}
elif [[ $line =~ ^check\ *=\ *(.*)$ ]]; then
check=${BASH_REMATCH[1]}
elif [[ $line =~ ^accel\ *=\ *(.*)$ ]]; then
@@ -67,7 +71,7 @@ function for_each_unittest()
fi
done
if [ -n "${testname}" ]; then
-   $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" 
"$accel" "$timeout"
+   $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$machine" 
"$check" "$accel" "$timeout"
fi
exec {fd}<&-
   }
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 177b62166..0c96d6ea2 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -32,7 +32,7 @@ premature_failure()
   get_cmdline()
   {
   local kernel=$1
-echo "TESTNAME=$testname TIMEOUT=$timeout ACCEL=$accel $RUNTIME_arch_run 
$kernel -smp $smp $opts"
+echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine ACCEL=$accel 
$RUNTIME_arch_run $kernel -smp $smp $opts"
   }
   
   skip_nodefault()

@@ -80,9 +80,10 @@ function run()
   local kernel="$4"
   local opts="$5"
   local arch="$6"
-local check="${CHECK:-$7}"
-local accel="$8"
-local timeout="${9:-$TIMEOUT}" # unittests.cfg overrides the default
+local machine="$7"
+local check="${CHECK:-$8}"
+local accel="$9"
+local timeout="${10:-$TIMEOUT}" # unittests.cfg overrides the default
   
   if [ "${CONFIG_EFI}" == "y" ]; then

   kernel=${kernel/%.flat/.efi}
@@ -116,6 +117,13 @@ function run()
   return 2
   fi
  

Re: [PATCH] tests/qtest/boot-serial-test: Add support on LoongArch system

[Thomas Huth]

On 08/05/2024 10.55, Bibo Mao wrote:

Add boot-serial-test test case support on LoongArch system.


... and also the filter tests?


Signed-off-by: Bibo Mao 
---
  tests/qtest/boot-serial-test.c | 10 ++
  tests/qtest/meson.build|  4 
  2 files changed, 14 insertions(+)

diff --git a/tests/qtest/boot-serial-test.c b/tests/qtest/boot-serial-test.c
index e3b7d65fe5..631015e8c8 100644
--- a/tests/qtest/boot-serial-test.c
+++ b/tests/qtest/boot-serial-test.c
@@ -129,6 +129,14 @@ static const uint8_t kernel_stm32vldiscovery[] = {
  0x04, 0x38, 0x01, 0x40  /* 0x40013804 = USART1 TXD */
  };
  
+static const uint8_t bios_loongarch64[] = {

+0x0c, 0xc0, 0x3f, 0x14, /* lu12i.w $t0, 0x1fe00*/
+0x8c, 0x81, 0x87, 0x03, /* ori $t0, $t0, 0x1e0 */
+0x0d, 0x50, 0x81, 0x03, /* li.w$t1, 'T'*/
+0x8d, 0x01, 0x00, 0x29, /* st.b$t1, $t0, 0 */
+0xff, 0xf3, 0xff, 0x53, /*  b  -16  # loop */
+};
+
  typedef struct testdef {
  const char *arch;   /* Target architecture */
  const char *machine;/* Name of the machine */
@@ -181,6 +189,8 @@ static const testdef_t tests[] = {
  { "arm", "microbit", "", "T", sizeof(kernel_nrf51), kernel_nrf51 },
  { "arm", "stm32vldiscovery", "", "T",
sizeof(kernel_stm32vldiscovery), kernel_stm32vldiscovery },
+{ "loongarch64", "virt", "-cpu max", "TT", sizeof(bios_loongarch64),
+  NULL, bios_loongarch64 },
  
  { NULL }

  };
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 6f2f594ace..6619b630e6 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -256,6 +256,10 @@ qtests_s390x = \
  qtests_riscv32 = \
(config_all_devices.has_key('CONFIG_SIFIVE_E_AON') ? 
['sifive-e-aon-watchdog-test'] : [])
  
+ qtests_loongarch64 = \

+  qtests_filter + \
+  ['boot-serial-test']


It's already a little bit messed up, but I think we originally had the 
entries in alphabetical order, so I'd like to suggest to add this between 
qtests_hppa and qtests_m68k instead.


 Thomas



  qos_test_ss = ss.source_set()
  qos_test_ss.add(
'ac97-test.c',

base-commit: 4e66a08546a2588a4667766a1edab9caccf24ce3

Re: [RFC PATCH-for-9.1 3/4] hw/i2c: Convert to spec v7 terminology (automatically)

[Thomas Huth]

On 08/04/2024 23.33, Philippe Mathieu-Daudé wrote:

One of the biggest change from I2C spec v6 -> v7 is:

   • Updated the terms "master/slave" to "controller/target"

Since it follows the inclusive terminology from the "Conscious
Language in your Open Source Projects" guidelines [*], replace
the I2C terminology.

Mechanical transformation running:

   $ cat i2c_rename.txt | while read old new; do \
   sed -i -e "s/$old/$new/g" $(git grep -l $old); \
 done

Having:

   $ cat i2c_rename.txt
   i2c_bus_master i2c_bus_controller
   i2c_schedule_pending_master i2c_schedule_pending_controller
   I2CPendingMasters I2CPendingControllers
   I2CPendingMaster I2CPendingController
   pending_masters pending_controllers
   I2C_SLAVE_CLASS I2C_TARGET_CLASS
   I2C_SLAVE_GET_CLASS I2C_TARGET_GET_CLASS
   I2CSlaveClass I2CTargetClass
   I2CSlave I2CTarget
   TYPE_I2C_SLAVE TYPE_I2C_TARGET
   I2C_SLAVE I2C_TARGET
   i2c_slave_new i2c_target_new
   i2c_slave_create_simple i2c_target_create_simple
   i2c_slave_realize_and_unref i2c_target_realize_and_unref
   i2c_slave_set_address i2c_target_set_address
   VMSTATE_I2C_SLAVE VMSTATE_I2C_TARGET
   vmstate_i2c_slave vmstate_i2c_target

Note, the QOM type definition is not modified, TYPE_I2C_TARGET
remains defined as "i2c-slave".

[*] https://github.com/conscious-lang/conscious-lang-docs/blob/main/faq.md

Inspired-by: Wolfram Sang 
Signed-off-by: Philippe Mathieu-Daudé 
---

...

@@ -649,29 +649,29 @@ static void witherspoon_bmc_i2c_init(AspeedMachineState 
*bmc)
  qdev_connect_gpio_out(dev, pca1_leds[i].gpio_id,
qdev_get_gpio_in(DEVICE(led), 0));
  }
-i2c_slave_create_simple(aspeed_i2c_get_bus(>i2c, 3),
+i2c_target_create_simple(aspeed_i2c_get_bus(>i2c, 3),
 "dps310", 0x76);


So the location of the opening parenthesis changes in a lot of lines in your 
patch, but you don't adapt the following line accordingly, so the 
indentation is off now. ==> One more reason for not breaking the long lines 
in your previous patch.


 Thomas

Re: [RFC PATCH-for-9.1 2/4] hw/i2c: Fix checkpatch line over 80 chars warnings

[Thomas Huth]

On 08/04/2024 23.33, Philippe Mathieu-Daudé wrote:

We are going to modify these lines, fix their style
in order to avoid checkpatch.pl warnings:

   WARNING: line over 80 characters


I think the common sense is nowadays that it is OK to have lines with e.g. 
82 characters when it looks better than having ugly wrapped lines otherwise. 
That's why it's only a warning, and not an error.


So for me, for most hunks in this file, it seems like keeping the long lines 
looks better than breaking them.


 Thomas

Re: hw/usb/hcd-ohci: Fix #1510, #303: pid not IN or OUT

[Thomas Huth]

On 07/05/2024 22.20, Cord Amfmgm wrote:



On Wed, Apr 24, 2024 at 3:43 PM Cord Amfmgm > wrote:


On Thu, Apr 18, 2024 at 10:43 AM Michael Tokarev mailto:m...@tls.msk.ru>> wrote:

06.02.2024 10:13, Cord Amfmgm wrote:
 > This changes the ohci validation to not assert if invalid
 > data is fed to the ohci controller. The poc suggested in
 > https://bugs.launchpad.net/qemu/+bug/1907042

 > and then migrated to bug #303 does the following to
 > feed it a SETUP pid and EndPt of 1:
 >
 >          uint32_t MaxPacket = 64;
 >          uint32_t TDFormat = 0;
 >          uint32_t Skip = 0;
 >          uint32_t Speed = 0;
 >          uint32_t Direction = 0;  /* #define OHCI_TD_DIR_SETUP 0 */
 >          uint32_t EndPt = 1;
 >          uint32_t FuncAddress = 0;
 >          ed->attr = (MaxPacket << 16) | (TDFormat << 15) | (Skip
<< 14)
 >                     | (Speed << 13) | (Direction << 11) | (EndPt
<< 7)
 >                     | FuncAddress;
 >          ed->tailp = /*TDQTailPntr= */ 0;
 >          ed->headp = ((/*TDQHeadPntr= */ [0]) & 0xfff0)
 >                     | (/* ToggleCarry= */ 0 << 1);
 >          ed->next_ed = (/* NextED= */ 0 & 0xfff0)
 >
 > qemu-fuzz also caught the same issue in #1510. They are
 > both fixed by this patch.
 >
 > The if (td.cbp > td.be ) logic in ohci_service_td()
causes an
 > ohci_die(). My understanding of the OHCI spec 4.3.1.2
 > Table 4-2 allows td.cbp to be one byte more than td.be
 to
 > signal the buffer has zero length. The new check in qemu
 > appears to have been added since qemu-4.2. This patch
 > includes both fixes since they are located very close
 > together.
 >
 > Signed-off-by: David Hubbard mailto:dmamf...@gmail.com>>


Your Signed-off-by line does not match the From: line ... could you please 
fix this? (see 
https://www.qemu.org/docs/master/devel/submitting-a-patch.html#patch-emails-must-include-a-signed-off-by-line 
, too)



Wonder if this got lost somehow.  Or is it not needed?

Thanks,

/mjt


Friendly ping! Gerd, can you chime in with how you would like to
approach this? I still need this patch to unblock my qemu workflow -
custom OS development.


Can I please ask for an update on this? I'm attempting to figure out if this 
patch has been rejected and I need to resubmit / rework it at HEAD?


Looks like it's hard to find someone who still can review OHCI patches these 
days...


Anyway, I tried to get the reproducer running that had been added to the 
original patch (installed an Ubuntu 18.04 guest and compiled and ran that 
ohci_poc program in it), but so far, I failed. Could you please provide 
detailed steps how you can still produce this issue with the latest version 
of QEMU, please?


 Thanks,
  Thomas

Re: [PATCH] gitlab: Update msys2-64bit runner tags

[Thomas Huth]

On 07/05/2024 19.53, Richard Henderson wrote:

Gitlab has deprecated and removed support for windows-1809
and shared-windows.  Update to saas-windows-medium-amd64 per

https://about.gitlab.com/blog/2024/01/22/windows-2022-support-for-gitlab-saas-runners/

Signed-off-by: Richard Henderson 
---
  .gitlab-ci.d/windows.yml | 4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/.gitlab-ci.d/windows.yml b/.gitlab-ci.d/windows.yml
index d26dbdd0c0..a83f23a786 100644
--- a/.gitlab-ci.d/windows.yml
+++ b/.gitlab-ci.d/windows.yml
@@ -1,9 +1,7 @@
  msys2-64bit:
extends: .base_job_template
tags:
-  - shared-windows
-  - windows
-  - windows-1809
+  - saas-windows-medium-amd64
cache:
  key: "$CI_JOB_NAME"
  paths:


Reviewed-by: Thomas Huth 
Tested-by: Thomas Huth 

(NB: my test run took 86 minutes ... that's pretty long already...)

Re: How unsubscribe

[Thomas Huth]

On 08/05/2024 00.20, Jean-Christophe wrote:

Hello,
How unsubscribe ?


Go to https://lists.nongnu.org/mailman/listinfo/qemu-devel and look at the 
bottom.


 HTH,
  Thomas

Re: [kvm-unit-tests PATCH v9 07/31] scripts: allow machine option to be specified in unittests.cfg

[Thomas Huth]

On 04/05/2024 14.28, Nicholas Piggin wrote:

This allows different machines with different requirements to be
supported by run_tests.sh, similarly to how different accelerators
are handled.

Acked-by: Thomas Huth 
Acked-by: Andrew Jones 
Signed-off-by: Nicholas Piggin 
---
  docs/unittests.txt   |  7 +++
  scripts/common.bash  |  8 ++--
  scripts/runtime.bash | 16 
  3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/docs/unittests.txt b/docs/unittests.txt
index 7cf2c55ad..6449efd78 100644
--- a/docs/unittests.txt
+++ b/docs/unittests.txt
@@ -42,6 +42,13 @@ For / directories that support multiple architectures, 
this restricts
  the test to the specified arch. By default, the test will run on any
  architecture.
  
+machine

+---
+For those architectures that support multiple machine types, this restricts
+the test to the specified machine. By default, the test will run on
+any machine type. (Note, the machine can be specified with the MACHINE=
+environment variable, and defaults to the architecture's default.)
+
  smp
  ---
  smp = 
diff --git a/scripts/common.bash b/scripts/common.bash
index 5e9ad53e2..3aa557c8c 100644
--- a/scripts/common.bash
+++ b/scripts/common.bash
@@ -10,6 +10,7 @@ function for_each_unittest()
local opts
local groups
local arch
+   local machine
local check
local accel
local timeout
@@ -21,7 +22,7 @@ function for_each_unittest()
if [[ "$line" =~ ^\[(.*)\]$ ]]; then
rematch=${BASH_REMATCH[1]}
if [ -n "${testname}" ]; then
-   $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" 
"$check" "$accel" "$timeout"
+   $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" 
"$machine" "$check" "$accel" "$timeout"
fi
testname=$rematch
smp=1
@@ -29,6 +30,7 @@ function for_each_unittest()
opts=""
groups=""
arch=""
+   machine=""
check=""
accel=""
timeout=""
@@ -58,6 +60,8 @@ function for_each_unittest()
groups=${BASH_REMATCH[1]}
elif [[ $line =~ ^arch\ *=\ *(.*)$ ]]; then
arch=${BASH_REMATCH[1]}
+   elif [[ $line =~ ^machine\ *=\ *(.*)$ ]]; then
+   machine=${BASH_REMATCH[1]}
elif [[ $line =~ ^check\ *=\ *(.*)$ ]]; then
check=${BASH_REMATCH[1]}
elif [[ $line =~ ^accel\ *=\ *(.*)$ ]]; then
@@ -67,7 +71,7 @@ function for_each_unittest()
fi
done
if [ -n "${testname}" ]; then
-   $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$check" 
"$accel" "$timeout"
+   $(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$opts" "$arch" "$machine" 
"$check" "$accel" "$timeout"
fi
exec {fd}<&-
  }
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index 177b62166..0c96d6ea2 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -32,7 +32,7 @@ premature_failure()
  get_cmdline()
  {
  local kernel=$1
-echo "TESTNAME=$testname TIMEOUT=$timeout ACCEL=$accel $RUNTIME_arch_run 
$kernel -smp $smp $opts"
+echo "TESTNAME=$testname TIMEOUT=$timeout MACHINE=$machine ACCEL=$accel 
$RUNTIME_arch_run $kernel -smp $smp $opts"
  }
  
  skip_nodefault()

@@ -80,9 +80,10 @@ function run()
  local kernel="$4"
  local opts="$5"
  local arch="$6"
-local check="${CHECK:-$7}"
-local accel="$8"
-local timeout="${9:-$TIMEOUT}" # unittests.cfg overrides the default
+local machine="$7"
+local check="${CHECK:-$8}"
+local accel="$9"
+local timeout="${10:-$TIMEOUT}" # unittests.cfg overrides the default
  
  if [ "${CONFIG_EFI}" == "y" ]; then

  kernel=${kernel/%.flat/.efi}
@@ -116,6 +117,13 @@ function run()
  return 2
  fi
  
+if [ -n "$machine" ] && [ -n "$MACHINE" ] && [ "$machine" != "$MACHINE&quo

Re: [kvm-unit-tests PATCH v9 17/31] powerpc: Add cpu_relax

[Thomas Huth]

On 04/05/2024 14.28, Nicholas Piggin wrote:

Add a cpu_relax variant that uses SMT priority nop instructions like
Linux. This was split out of the SMP patch because it affects the sprs
test case.

Signed-off-by: Nicholas Piggin 
---
  lib/ppc64/asm/barrier.h | 1 +
  powerpc/sprs.c  | 4 ++--
  2 files changed, 3 insertions(+), 2 deletions(-)



Reviewed-by: Thomas Huth 

Re: [kvm-unit-tests PATCH v9 12/31] powerpc: general interrupt tests

[Thomas Huth]

On 04/05/2024 14.28, Nicholas Piggin wrote:

Add basic testing of various kinds of interrupts, machine check,
page fault, illegal, decrementer, trace, syscall, etc.

This has a known failure on QEMU TCG pseries machines where MSR[ME]
can be incorrectly set to 0.

Signed-off-by: Nicholas Piggin 
---
  lib/powerpc/asm/processor.h |   4 +
  lib/powerpc/asm/reg.h   |  17 ++
  lib/powerpc/setup.c |  11 +
  lib/ppc64/asm/ptrace.h  |  16 ++
  powerpc/Makefile.common |   3 +-
  powerpc/interrupts.c| 414 
  powerpc/unittests.cfg   |   3 +
  7 files changed, 467 insertions(+), 1 deletion(-)
  create mode 100644 powerpc/interrupts.c


Acked-by: Thomas Huth 

Re: [kvm-unit-tests PATCH v9 03/31] powerpc: Mark known failing tests as kfail

[Thomas Huth]

On 07/05/2024 06.07, Nicholas Piggin wrote:

On Mon May 6, 2024 at 5:37 PM AEST, Thomas Huth wrote:

On 04/05/2024 14.28, Nicholas Piggin wrote:

Mark the failing h_cede_tm and spapr_vpa tests as kfail.

Signed-off-by: Nicholas Piggin 
---
   powerpc/spapr_vpa.c | 3 ++-
   powerpc/tm.c| 3 ++-
   2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/powerpc/spapr_vpa.c b/powerpc/spapr_vpa.c
index c2075e157..46fa0485c 100644
--- a/powerpc/spapr_vpa.c
+++ b/powerpc/spapr_vpa.c
@@ -150,7 +150,8 @@ static void test_vpa(void)
report_fail("Could not deregister after registration");
   
   	disp_count1 = be32_to_cpu(vpa->vp_dispatch_count);

-   report(disp_count1 % 2 == 1, "Dispatch count is odd after deregister");
+   /* TCG known fail, could be wrong test, must verify against PowerVM */
+   report_kfail(true, disp_count1 % 2 == 1, "Dispatch count is odd after 
deregister");


Using "true" as first argument looks rather pointless - then you could also
simply delete the test completely if it can never be tested reliably.

Thus could you please introduce a helper function is_tcg() that could be
used to check whether we run under TCG (and not KVM)? I think you could
check for "linux,kvm" in the "compatible" property in /hypervisor in the
device tree to see whether we're running in KVM mode or in TCG mode.


This I added in patch 30.

The reason for the suboptimal patch ordering was just me being lazy and
avoiding rebasing annoyance. I'd written a bunch of failing test cases
for QEMU work, but hadn't done the kvm/tcg test yet. It had a few
conflicts so I put it at the end... can rebase if you'd really prefer.


Ah, ok, no need to rebase then, as long it's there in the end, it's fine.

 Thanks,
  Thomas

Re: [PATCH] configure: quote -D options that are passed to meson

[Thomas Huth]

On 07/05/2024 12.49, Paolo Bonzini wrote:

Ensure that they go through unmodified, instead of removing one layer
of quoting.

Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini 
---
  configure | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index bd85377a6ae..10fbe10ad9c 100755
--- a/configure
+++ b/configure
@@ -764,7 +764,7 @@ for opt do
--*) meson_option_parse "$opt" "$optarg"
;;
# Pass through -D options to meson
-  -D*) meson_options="$meson_options $opt"
+  -D*) meson_option_add "$opt"
;;
    esac
  done


Reviewed-by: Thomas Huth 

Re: [PATCH v3 1/5] hw/loongarch: Rename LOONGARCH_MACHINE with VIRT_MACHINE

[Thomas Huth]

On 07/05/2024 03.18, maobibo wrote:



On 2024/5/6 下午2:09, maobibo wrote:



On 2024/5/6 下午12:24, Thomas Huth wrote:

On 06/05/2024 05.02, Bibo Mao wrote:

On LoongArch system, there is only virt machine type now, name
LOONGARCH_MACHINE is confused, rename it with VIRT_MACHINE. Machine name
about Other real hw boards can be added in future.

Signed-off-by: Bibo Mao 
---

...
@@ -1245,7 +1244,7 @@ static void loongarch_class_init(ObjectClass *oc, 
void *data)

  static const TypeInfo loongarch_machine_types[] = {
  {
-    .name   = TYPE_LOONGARCH_MACHINE,
+    .name   = TYPE_VIRT_MACHINE,
  .parent = TYPE_MACHINE,
  .instance_size  = sizeof(LoongArchMachineState),
  .class_init = loongarch_class_init,
diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
index 4e14bf6060..5ea2f0370d 100644
--- a/include/hw/loongarch/virt.h
+++ b/include/hw/loongarch/virt.h
@@ -73,8 +73,8 @@ struct LoongArchMachineState {
  struct loongarch_boot_info bootinfo;
  };
-#define TYPE_LOONGARCH_MACHINE  MACHINE_TYPE_NAME("virt")
-OBJECT_DECLARE_SIMPLE_TYPE(LoongArchMachineState, LOONGARCH_MACHINE)
+#define TYPE_VIRT_MACHINE  MACHINE_TYPE_NAME("virt")
+OBJECT_DECLARE_SIMPLE_TYPE(LoongArchMachineState, VIRT_MACHINE)
  bool loongarch_is_acpi_enabled(LoongArchMachineState *lams);
  void loongarch_acpi_setup(LoongArchMachineState *lams);
  #endif


  Hi,

there are currently some efforts going on to create the possibility to 
link a QEMU binary that contains all targets in one binary. Since we 
already have a TYPE_VIRT_MACHINE for other targets, I wonder whether it 
might be better to use LOONGARCH_VIRT_MACHINE than just VIRT_MACHINE 
here? Philippe, could you comment on this?


It is great if there is one QEMU binary which supports different targets. 
And LOONGARCH_VIRT_MACHINE is ok for me.

Hi Thomas, Philippe,

Does machine name "virt" need be changed if LOONGARCH_VIRT_MACHINE is used? 
There will be compatible issues if "virt" machine type is not suggested to use.


However CPU type "max" is not widely used now, can we get different 
architectures from CPU type rather than machine type for one QEMU binary 
which supports different targets?


I assume it should be fine to keep the "virt" machine name and "max" CPU 
type for each target, we've got a bunch of those already. I assume we'll 
keep the binary names as symlinks to the generic binary around and then 
decide via argv[0] about the main target...? Philippe, do you have already 
concrete plans for this?


 Thomas

Re: [PATCH] gitlab: Rename ubuntu-22.04-s390x-all to *-system

[Thomas Huth]

On 06/05/2024 22.23, Richard Henderson wrote:

We already build the linux-user binaries with
ubuntu-22.04-s390x-all-linux, so there's no need to do it again.

Signed-off-by: Richard Henderson 
---
  .gitlab-ci.d/custom-runners/ubuntu-22.04-s390x.yml | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.gitlab-ci.d/custom-runners/ubuntu-22.04-s390x.yml 
b/.gitlab-ci.d/custom-runners/ubuntu-22.04-s390x.yml
index 3a2b1e1d24..c3f16d77bb 100644
--- a/.gitlab-ci.d/custom-runners/ubuntu-22.04-s390x.yml
+++ b/.gitlab-ci.d/custom-runners/ubuntu-22.04-s390x.yml
@@ -21,7 +21,7 @@ ubuntu-22.04-s390x-all-linux:
   - make --output-sync check-tcg
   - make --output-sync -j`nproc` check
  
-ubuntu-22.04-s390x-all:

+ubuntu-22.04-s390x-all-system:
   extends: .custom_runner_template
   needs: []
   stage: build
@@ -35,7 +35,7 @@ ubuntu-22.04-s390x-all:
   script:
   - mkdir build
   - cd build
- - ../configure --disable-libssh
+ - ../configure --disable-libssh --disable-user


While you're at it, you could also drop the --disable-libssh now (it was a 
work-around for a bug in Ubuntu 18.04 IIRC).


Reviewed-by: Thomas Huth 

Re: [PATCH] gitlab: Drop --static from s390x linux-user build

[Thomas Huth]

On 06/05/2024 22.20, Richard Henderson wrote:

The host does not have the correct libraries installed for static pie,
which causes host/guest address space interference for some tests.
There's no real gain from linking statically, so drop it.

Signed-off-by: Richard Henderson 
---
Per my suggestion in

https://lore.kernel.org/qemu-devel/50c27a9f-fd75-4f8e-9a2d-488d8df4f...@linaro.org


r~
---
  .gitlab-ci.d/custom-runners/ubuntu-22.04-s390x.yml | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.gitlab-ci.d/custom-runners/ubuntu-22.04-s390x.yml 
b/.gitlab-ci.d/custom-runners/ubuntu-22.04-s390x.yml
index 105981879f..3a2b1e1d24 100644
--- a/.gitlab-ci.d/custom-runners/ubuntu-22.04-s390x.yml
+++ b/.gitlab-ci.d/custom-runners/ubuntu-22.04-s390x.yml
@@ -2,7 +2,7 @@
  # setup by the scripts/ci/setup/build-environment.yml task
  # "Install basic packages to build QEMU on Ubuntu 22.04"
  
-ubuntu-22.04-s390x-all-linux-static:

+ubuntu-22.04-s390x-all-linux:
   extends: .custom_runner_template
   needs: []
   stage: build
@@ -15,7 +15,7 @@ ubuntu-22.04-s390x-all-linux-static:
   script:
   - mkdir build
   - cd build
- - ../configure --enable-debug --static --disable-system
+ - ../configure --enable-debug-tcg --disable-system --disable-tools 
--disable-docs


Maybe mention the --disable-tools and --disable-docs in the commit message, too?

Anyway:
Reviewed-by: Thomas Huth 

Re: [PATCH v4 0/2] query-cpu-model-expansion: report deprecated features

[Thomas Huth]

On 29/04/2024 21.10, Collin Walling wrote:

Changelog

 v4
 - updated cover letter to show example output
 - deprecated features are now a subset of the full CPU model's
 list of features
 - value:
 1. no longer listing the deprecated features for CPU
  models that never had these features available in the
  first place
 2. deprecated features will not show up for future CPU
  models that out-right drop these features
 - updated qapi documentation
 - now reflects that these props are a subset of the full
 model's definition of properties
 - added Since: tag to deprecated-props (assuming 9.1)

 v3
 - removed optional disable-deprecated-feats argument
 - added deprecated-props array to CpuModelInfo struct
 - amended cover letter language to reflect design

 v2
 - removed "static-recommended" expansion type
 - implemented optional disable-deprecated-feats argument

---

The current implementation of query-cpu-model-expansion is lacking a way to 
retrieve
CPU models with properties (i.e. features) that are flagged as deprecated.  To 
remedy
this, a list of deprecated-props has been appended to the CpuModelInfo struct, 
and
will currently be reported by a query-cpu-model-expansion.  The features 
reported in
the output are a subset of the full CPU model expansion.



Thanks, queued for my next pull request now.

 Thomas

[PATCH v2] qemu-options: Deprecate "-runas" and introduce "-run-with user=..." instead

[Thomas Huth]

The old "-runas" option has the disadvantage that it is not visible
in the QAPI schema, so it is not available via the normal introspection
mechanisms. We've recently introduced the "-run-with" option for exactly
this purpose, which is meant to handle the options that affect the
runtime behavior. Thus let's introduce a "user=..." parameter here now
and deprecate the old "-runas" option.

Signed-off-by: Thomas Huth 
---
 v2: Add missing part in qemu-options.hx as suggested by Philippe

 docs/about/deprecated.rst |  6 ++
 system/vl.c   | 15 +++
 qemu-options.hx   | 15 +++
 3 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 3310df3274..fe69e2d44c 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -61,6 +61,12 @@ configurations (e.g. -smp drawers=1,books=1,clusters=1 for 
x86 PC machine) is
 marked deprecated since 9.0, users have to ensure that all the topology members
 described with -smp are supported by the target machine.
 
+``-runas`` (since 9.1)
+--
+
+Use ``-run-with user=..`` instead.
+
+
 User-mode emulator command line arguments
 -
 
diff --git a/system/vl.c b/system/vl.c
index 7756eac81e..b031427440 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -773,6 +773,10 @@ static QemuOptsList qemu_run_with_opts = {
 .name = "chroot",
 .type = QEMU_OPT_STRING,
 },
+{
+.name = "user",
+.type = QEMU_OPT_STRING,
+},
 { /* end of list */ }
 },
 };
@@ -3586,6 +3590,7 @@ void qemu_init(int argc, char **argv)
 break;
 #if defined(CONFIG_POSIX)
 case QEMU_OPTION_runas:
+warn_report("-runas is deprecated, use '-run-with user=...' 
instead");
 if (!os_set_runas(optarg)) {
 error_report("User \"%s\" doesn't exist"
  " (and is not :)",
@@ -3612,6 +3617,16 @@ void qemu_init(int argc, char **argv)
 if (str) {
 os_set_chroot(str);
 }
+str = qemu_opt_get(opts, "user");
+if (str) {
+if (!os_set_runas(str)) {
+error_report("User \"%s\" doesn't exist"
+ " (and is not :)",
+ optarg);
+exit(1);
+}
+}
+
 break;
 }
 #endif /* CONFIG_POSIX */
diff --git a/qemu-options.hx b/qemu-options.hx
index cf61f6b863..3031479a15 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4824,7 +4824,8 @@ DEF("runas", HAS_ARG, QEMU_OPTION_runas, \
 SRST
 ``-runas user``
 Immediately before starting guest execution, drop root privileges,
-switching to the specified user.
+switching to the specified user. This option is deprecated, use
+``-run-with user=...`` instead.
 ERST
 
 DEF("prom-env", HAS_ARG, QEMU_OPTION_prom_env,
@@ -4990,13 +4991,15 @@ DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log, "", 
QEMU_ARCH_ALL)
 
 #ifdef CONFIG_POSIX
 DEF("run-with", HAS_ARG, QEMU_OPTION_run_with,
-"-run-with [async-teardown=on|off][,chroot=dir]\n"
+"-run-with [async-teardown=on|off][,chroot=dir][user=username|uid:gid]\n"
 "Set miscellaneous QEMU process lifecycle options:\n"
 "async-teardown=on enables asynchronous teardown (Linux 
only)\n"
-"chroot=dir chroot to dir just before starting the VM\n",
+"chroot=dir chroot to dir just before starting the VM\n"
+"user=username switch to the specified user before 
starting the VM\n"
+"user=uid:gid dito, but use specified user-ID and group-ID 
instead\n",
 QEMU_ARCH_ALL)
 SRST
-``-run-with [async-teardown=on|off][,chroot=dir]``
+``-run-with [async-teardown=on|off][,chroot=dir][user=username|uid:gid]``
 Set QEMU process lifecycle options.
 
 ``async-teardown=on`` enables asynchronous teardown. A new process called
@@ -5013,6 +5016,10 @@ SRST
 ``chroot=dir`` can be used for doing a chroot to the specified directory
 immediately before starting the guest execution. This is especially useful
 in combination with -runas.
+
+``user=username`` or ``user=uid:gid`` can be used to drop root privileges
+by switching to the specified user (via username) or user and group
+(via uid:gid) immediately before starting guest execution.
 ERST
 #endif
 
-- 
2.45.0

[PATCH v2] qemu-options: Deprecate "-runas" and introduce "-run-with user=..." instead

[Thomas Huth]

The old "-runas" option has the disadvantage that it is not visible
in the QAPI schema, so it is not available via the normal introspection
mechanisms. We've recently introduced the "-run-with" option for exactly
this purpose, which is meant to handle the options that affect the
runtime behavior. Thus let's introduce a "user=..." parameter here now
and deprecate the old "-runas" option.

Signed-off-by: Thomas Huth 
---
 v2: Add missing part in qemu-options.hx as suggested by Philippe

 docs/about/deprecated.rst |  6 ++
 system/vl.c   | 15 +++
 qemu-options.hx   | 15 +++
 3 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 3310df3274..fe69e2d44c 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -61,6 +61,12 @@ configurations (e.g. -smp drawers=1,books=1,clusters=1 for 
x86 PC machine) is
 marked deprecated since 9.0, users have to ensure that all the topology members
 described with -smp are supported by the target machine.
 
+``-runas`` (since 9.1)
+--
+
+Use ``-run-with user=..`` instead.
+
+
 User-mode emulator command line arguments
 -
 
diff --git a/system/vl.c b/system/vl.c
index 7756eac81e..b031427440 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -773,6 +773,10 @@ static QemuOptsList qemu_run_with_opts = {
 .name = "chroot",
 .type = QEMU_OPT_STRING,
 },
+{
+.name = "user",
+.type = QEMU_OPT_STRING,
+},
 { /* end of list */ }
 },
 };
@@ -3586,6 +3590,7 @@ void qemu_init(int argc, char **argv)
 break;
 #if defined(CONFIG_POSIX)
 case QEMU_OPTION_runas:
+warn_report("-runas is deprecated, use '-run-with user=...' 
instead");
 if (!os_set_runas(optarg)) {
 error_report("User \"%s\" doesn't exist"
  " (and is not :)",
@@ -3612,6 +3617,16 @@ void qemu_init(int argc, char **argv)
 if (str) {
 os_set_chroot(str);
 }
+str = qemu_opt_get(opts, "user");
+if (str) {
+if (!os_set_runas(str)) {
+error_report("User \"%s\" doesn't exist"
+ " (and is not :)",
+ optarg);
+exit(1);
+}
+}
+
 break;
 }
 #endif /* CONFIG_POSIX */
diff --git a/qemu-options.hx b/qemu-options.hx
index cf61f6b863..3031479a15 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4824,7 +4824,8 @@ DEF("runas", HAS_ARG, QEMU_OPTION_runas, \
 SRST
 ``-runas user``
 Immediately before starting guest execution, drop root privileges,
-switching to the specified user.
+switching to the specified user. This option is deprecated, use
+``-run-with user=...`` instead.
 ERST
 
 DEF("prom-env", HAS_ARG, QEMU_OPTION_prom_env,
@@ -4990,13 +4991,15 @@ DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log, "", 
QEMU_ARCH_ALL)
 
 #ifdef CONFIG_POSIX
 DEF("run-with", HAS_ARG, QEMU_OPTION_run_with,
-"-run-with [async-teardown=on|off][,chroot=dir]\n"
+"-run-with [async-teardown=on|off][,chroot=dir][user=username|uid:gid]\n"
 "Set miscellaneous QEMU process lifecycle options:\n"
 "async-teardown=on enables asynchronous teardown (Linux 
only)\n"
-"chroot=dir chroot to dir just before starting the VM\n",
+"chroot=dir chroot to dir just before starting the VM\n"
+"user=username switch to the specified user before 
starting the VM\n"
+"user=uid:gid dito, but use specified user-ID and group-ID 
instead\n",
 QEMU_ARCH_ALL)
 SRST
-``-run-with [async-teardown=on|off][,chroot=dir]``
+``-run-with [async-teardown=on|off][,chroot=dir][user=username|uid:gid]``
 Set QEMU process lifecycle options.
 
 ``async-teardown=on`` enables asynchronous teardown. A new process called
@@ -5013,6 +5016,10 @@ SRST
 ``chroot=dir`` can be used for doing a chroot to the specified directory
 immediately before starting the guest execution. This is especially useful
 in combination with -runas.
+
+``user=username`` or ``user=uid:gid`` can be used to drop root privileges
+by switching to the specified user (via username) or user and group
+(via uid:gid) immediately before starting guest execution.
 ERST
 #endif
 
-- 
2.45.0
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org

Re: [qemu-web PATCH] blog: KVM Forum 2024 CFP

[Thomas Huth]

On 06/05/2024 12.55, Stefan Hajnoczi wrote:

The mistakes were mine. Thanks for pointing them out, Thomas!


All right, so I went ahead and fixed the deadline date and link and pushed 
it to the repository now.


 Thanks,
  Thomas

Re: [qemu-web PATCH] blog: KVM Forum 2024 CFP

[Thomas Huth]

On 06/05/2024 12.13, Paolo Bonzini wrote:

Add a new post linking to the KVM Forum 2024 Call for Presentations.
Thanks to Stefan Hajnoczi for providing a draft of this post!

Cc: Stefan Hajnoczi 
Signed-off-by: Paolo Bonzini 
---
  _posts/2024-05-06-kvm-forum-cfp.md | 36 ++
  1 file changed, 36 insertions(+)
  create mode 100644 _posts/2024-05-06-kvm-forum-cfp.md

diff --git a/_posts/2024-05-06-kvm-forum-cfp.md 
b/_posts/2024-05-06-kvm-forum-cfp.md
new file mode 100644
index 000..23398fe
--- /dev/null
+++ b/_posts/2024-05-06-kvm-forum-cfp.md
@@ -0,0 +1,36 @@
+---
+layout: post
+title:  "KVM Forum 2024: Call for presentations"
+date:   2024-05-06 07:00:00 +0100
+categories: [presentations, conferences]
+---
+
+The [KVM Forum 2024](https://kvm-forum.qemu.org/2024/) conference will take
+place in Brno, Czech Republic on September 22-23, 2024. KVM Forum brings
+together the Linux virtualization community, especially around the KVM stack,
+including QEMU and other virtual machine monitors.
+
+The Call for Presentations is open until June 1, 2024. You are invited to
+submit presentation proposals via the [KVM Forum CfP
+page](https://kvm-forum.org/2024/cfp).


This link is wrong, the right one is:

 https://kvm-forum.qemu.org/2024/cfp/

 Thomas

Re: [qemu-web PATCH] blog: KVM Forum 2024 CFP

[Thomas Huth]

On 06/05/2024 12.13, Paolo Bonzini wrote:

Add a new post linking to the KVM Forum 2024 Call for Presentations.
Thanks to Stefan Hajnoczi for providing a draft of this post!

Cc: Stefan Hajnoczi 
Signed-off-by: Paolo Bonzini 
---
  _posts/2024-05-06-kvm-forum-cfp.md | 36 ++
  1 file changed, 36 insertions(+)
  create mode 100644 _posts/2024-05-06-kvm-forum-cfp.md

diff --git a/_posts/2024-05-06-kvm-forum-cfp.md 
b/_posts/2024-05-06-kvm-forum-cfp.md
new file mode 100644
index 000..23398fe
--- /dev/null
+++ b/_posts/2024-05-06-kvm-forum-cfp.md
@@ -0,0 +1,36 @@
+---
+layout: post
+title:  "KVM Forum 2024: Call for presentations"
+date:   2024-05-06 07:00:00 +0100
+categories: [presentations, conferences]
+---
+
+The [KVM Forum 2024](https://kvm-forum.qemu.org/2024/) conference will take
+place in Brno, Czech Republic on September 22-23, 2024. KVM Forum brings
+together the Linux virtualization community, especially around the KVM stack,
+including QEMU and other virtual machine monitors.
+
+The Call for Presentations is open until June 1, 2024.


In the "Call for Presentations" mail that you send to the qemu-devel mailing 
list in parallel, you wrote: "The deadline for submitting presentations is 
June 8" ... so what's the correct date now?


Apart from that, the blog post looks fine to me.

 Thomas


You are invited to
+submit presentation proposals via the [KVM Forum CfP
+page](https://kvm-forum.org/2024/cfp). All presentation slots will be
+25 minutes + 5 minutes for questions.
+
+Suggested topics include:
+
+* Scalability and Optimization
+* Hardening and security
+* Confidential computing
+* Testing
+* KVM and the Linux Kernel
+  * New Features and Architecture Ports
+  * Device Passthrough: VFIO, mdev, vDPA
+  * Network Virtualization
+  * Virtio and vhost
+* Virtual Machine Monitors and Management
+  * VMM Implementation: APIs, Live Migration, Performance Tuning, etc.
+  * Multi-process VMMs: vhost-user, vfio-user, QEMU Storage Daemon, SPDK
+  * QEMU without KVM: Hypervisor.framework, Windows Hypervisor Platform, etc.
+  * Managing KVM: Libvirt, KubeVirt, Kata Containers
+* Emulation
+  * New Devices, Boards and Architectures
+  * CPU Emulation and Binary Translation

Re: [kvm-unit-tests PATCH v9 02/31] report: Add known failure reporting option

[Thomas Huth]

On 06/05/2024 10.01, Andrew Jones wrote:

On Mon, May 06, 2024 at 09:25:37AM GMT, Thomas Huth wrote:

On 04/05/2024 14.28, Nicholas Piggin wrote:

There are times we would like to test a function that is known to fail
in some conditions due to a bug in implementation (QEMU, KVM, or even
hardware). It would be nice to count these as known failures and not
report a summary failure.

xfail is not the same thing, xfail means failure is required and a pass
causes the test to fail. So add kfail for known failures.


Actually, I wonder whether that's not rather a bug in report_xfail()
instead. Currently, when you call report_xfail(true, ...), the result is
*always* counted as a failure, either as an expected failure (if the test
really failed), or as a normal failure (if the test succeeded). What's the
point of counting a successful test as a failure??

Andrew, you've originally introduced report_xfail in commit a5af7b8a67e,
could you please comment on this?



An expected failure passes when the test fails and fails when the test
passes, i.e.

   XFAIL == PASS (but separately accounted with 'xfailures')
   XPASS == FAIL

If we expect something to fail and it passes then this may be due to the
thing being fixed, so we should change the test to expect success, or
due to the test being written incorrectly for our expectations. Either
way, when an expected failure doesn't fail, it means our expectations are
wrong and we need to be alerted to that, hence a FAIL is reported.


Ok, so this was on purpose, indeed. Maybe we should add this information in 
a comment right in front of the function, so that others don't scratch their 
head, too?


Anyway, this patch here is fine then:
Reviewed-by: Thomas Huth 

Re: [kvm-unit-tests PATCH v9 03/31] powerpc: Mark known failing tests as kfail

[Thomas Huth]

On 04/05/2024 14.28, Nicholas Piggin wrote:

Mark the failing h_cede_tm and spapr_vpa tests as kfail.

Signed-off-by: Nicholas Piggin 
---
  powerpc/spapr_vpa.c | 3 ++-
  powerpc/tm.c| 3 ++-
  2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/powerpc/spapr_vpa.c b/powerpc/spapr_vpa.c
index c2075e157..46fa0485c 100644
--- a/powerpc/spapr_vpa.c
+++ b/powerpc/spapr_vpa.c
@@ -150,7 +150,8 @@ static void test_vpa(void)
report_fail("Could not deregister after registration");
  
  	disp_count1 = be32_to_cpu(vpa->vp_dispatch_count);

-   report(disp_count1 % 2 == 1, "Dispatch count is odd after deregister");
+   /* TCG known fail, could be wrong test, must verify against PowerVM */
+   report_kfail(true, disp_count1 % 2 == 1, "Dispatch count is odd after 
deregister");


Using "true" as first argument looks rather pointless - then you could also 
simply delete the test completely if it can never be tested reliably.


Thus could you please introduce a helper function is_tcg() that could be 
used to check whether we run under TCG (and not KVM)? I think you could 
check for "linux,kvm" in the "compatible" property in /hypervisor in the 
device tree to see whether we're running in KVM mode or in TCG mode.



report_prefix_pop();
  }
diff --git a/powerpc/tm.c b/powerpc/tm.c
index 6b1ceeb6e..d9e7f455d 100644
--- a/powerpc/tm.c
+++ b/powerpc/tm.c
@@ -133,7 +133,8 @@ int main(int argc, char **argv)
report_skip("TM is not available");
goto done;
}
-   report(cpus_with_tm == nr_cpus,
+   /* KVM does not report TM in secondary threads in POWER9 */
+   report_kfail(true, cpus_with_tm == nr_cpus,
   "TM available in all 'ibm,pa-features' properties");


Could you check the PVR for POWER9 here instead of using "true" as first 
parameter?


 Thomas

Re: [kvm-unit-tests PATCH v9 02/31] report: Add known failure reporting option

[Thomas Huth]

On 04/05/2024 14.28, Nicholas Piggin wrote:

There are times we would like to test a function that is known to fail
in some conditions due to a bug in implementation (QEMU, KVM, or even
hardware). It would be nice to count these as known failures and not
report a summary failure.

xfail is not the same thing, xfail means failure is required and a pass
causes the test to fail. So add kfail for known failures.


Actually, I wonder whether that's not rather a bug in report_xfail() 
instead. Currently, when you call report_xfail(true, ...), the result is 
*always* counted as a failure, either as an expected failure (if the test 
really failed), or as a normal failure (if the test succeeded). What's the 
point of counting a successful test as a failure??


Andrew, you've originally introduced report_xfail in commit a5af7b8a67e, 
could you please comment on this?


IMHO we should rather do something like this instead:

diff --git a/lib/report.c b/lib/report.c
--- a/lib/report.c
+++ b/lib/report.c
@@ -98,7 +98,7 @@ static void va_report(const char *msg_fmt,
skipped++;
else if (xfail && !pass)
xfailures++;
-   else if (xfail || !pass)
+   else if (!xfail && !pass)
failures++;

spin_unlock();

 Thomas

Re: [kvm-unit-tests PATCH v9 01/31] doc: update unittests doc

[Thomas Huth]

On 04/05/2024 14.28, Nicholas Piggin wrote:

This adds a few minor fixes.

Signed-off-by: Nicholas Piggin 
---
  docs/unittests.txt | 12 +++-
  1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/docs/unittests.txt b/docs/unittests.txt
index 3192a60ec..7cf2c55ad 100644
--- a/docs/unittests.txt
+++ b/docs/unittests.txt
@@ -15,8 +15,8 @@ unittests.cfg format
  
  # is the comment symbol, all following contents of the line is ignored.
  
-Each unit test is defined with a [unit-test-name] line, followed by

-a set of parameters that control how the test case is run. The name is
+Each unit test is defined with a [unit-test-name] line, followed by a
+set of parameters that control how the test case is run. The name is
  arbitrary and appears in the status reporting output.
  
  Parameters appear on their own lines under the test name, and have a

@@ -62,8 +62,8 @@ groups
  groups =   ...
  
  Used to group the test cases for the `run_tests.sh -g ...` run group

-option. Adding a test to the nodefault group will cause it to not be
-run by default.
+option. The group name is arbitrary, aside from the nodefault group
+which makes the test to not be run by default.
  
  accel

  -
@@ -82,8 +82,10 @@ Optional timeout in seconds, after which the test will be 
killed and fail.
  
  check

  -
-check = =<
+check = =
  
  Check a file for a particular value before running a test. The check line

  can contain multiple files to check separated by a space, but each check
  parameter needs to be of the form =
+
+The path and value can not contain space, =, or shell wildcard characters.


Could you comment on my feedback here, please:

 https://lore.kernel.org/kvm/951ccd88-0e39-4379-8d86-718e72594...@redhat.com/

 Thanks,
  Thomas

Re: [PATCH] Fixes: Indentation using TABs and improve formatting

[Thomas Huth]

On 04/05/2024 22.34, Michael Tokarev wrote:

04.05.2024 21:58, Tanmay wrote:

Hi,

I have attached a patch file that fixes indentation and formatting for 
some files as listed in https://gitlab.com/qemu-project/qemu/-/issues/373 
.


it is sort of good you posted this patch to stable@.  It has absolutely 
nothing to do
with stable, but it serves as a an example of things which should - in my 
opinion -

not be done at all.


I disagree. Yes, clean-up patches like this make it somewhat difficult to 
backport other patches to stable, but that should not be the reason to not 
do cleanups at all. If we keep badly formatted code in the repository, 
people will copy-n-paste it to other places, or if you have to do fixes in 
sources that have mixed TABs and spaces, you often get complaints from 
checkpatch.pl though it is not your fault. So we should get this straight at 
one point in time.


So, Tanmay, could you please resend your patch, this time to 
qemu-devel@nongnu.org instead of qemu-stable, and CC: qemu-...@nongnu.org 
and the corresponding ARM maintainers (you can use 
scripts/get_maintainers.pl to find out the correct maintainers that should 
be CC:-ed). And if possible, please send your patch inline and not as an 
attachment (so it's possible to comment on the patch via hitting the reply 
button), preferably with "git send-email" instead of using your e-mail program.


 Thanks!
  Thomas

Re: [PATCH v3 1/5] hw/loongarch: Rename LOONGARCH_MACHINE with VIRT_MACHINE

[Thomas Huth]

On 06/05/2024 05.02, Bibo Mao wrote:

On LoongArch system, there is only virt machine type now, name
LOONGARCH_MACHINE is confused, rename it with VIRT_MACHINE. Machine name
about Other real hw boards can be added in future.

Signed-off-by: Bibo Mao 
---

...

@@ -1245,7 +1244,7 @@ static void loongarch_class_init(ObjectClass *oc, void 
*data)
  
  static const TypeInfo loongarch_machine_types[] = {

  {
-.name   = TYPE_LOONGARCH_MACHINE,
+.name   = TYPE_VIRT_MACHINE,
  .parent = TYPE_MACHINE,
  .instance_size  = sizeof(LoongArchMachineState),
  .class_init = loongarch_class_init,
diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
index 4e14bf6060..5ea2f0370d 100644
--- a/include/hw/loongarch/virt.h
+++ b/include/hw/loongarch/virt.h
@@ -73,8 +73,8 @@ struct LoongArchMachineState {
  struct loongarch_boot_info bootinfo;
  };
  
-#define TYPE_LOONGARCH_MACHINE  MACHINE_TYPE_NAME("virt")

-OBJECT_DECLARE_SIMPLE_TYPE(LoongArchMachineState, LOONGARCH_MACHINE)
+#define TYPE_VIRT_MACHINE  MACHINE_TYPE_NAME("virt")
+OBJECT_DECLARE_SIMPLE_TYPE(LoongArchMachineState, VIRT_MACHINE)
  bool loongarch_is_acpi_enabled(LoongArchMachineState *lams);
  void loongarch_acpi_setup(LoongArchMachineState *lams);
  #endif


 Hi,

there are currently some efforts going on to create the possibility to link 
a QEMU binary that contains all targets in one binary. Since we already have 
a TYPE_VIRT_MACHINE for other targets, I wonder whether it might be better 
to use LOONGARCH_VIRT_MACHINE than just VIRT_MACHINE here? Philippe, could 
you comment on this?


 Thomas

Re: [PATCH 4/4] tests/qtest: Check STM32L4x5 clock connections

[Thomas Huth]

On 05/05/2024 16.05, Inès Varhol wrote:

For USART, GPIO and SYSCFG devices, check that clock frequency before
and after enabling the peripheral clock in RCC is correct.

Signed-off-by: Inès Varhol 
---
Hello,

Should these tests be regrouped in stm32l4x5_rcc-test.c ?


 Hi,

sounds mostly like a matter of taste at a first glance. Or what would be the 
benefit of putting everything into the *rcc-test.c file? Could you maybe 
consolidate the get_clock_freq_hz() function that way? (maybe that 
get_clock_freq_hz() function could also be consolidated as a inline function 
in a shared header instead?)


 Thomas

Re: [PATCH] qga/commands-posix: fix typo in qmp_guest_set_user_password

[Thomas Huth]

On 03/05/2024 19.13, Paolo Bonzini wrote:

qga/commands-posix.c does not compile on FreeBSD due to a confusion
between "chpasswdata" (wrong) and "chpasswddata" (used in the #else
branch).

Signed-off-by: Paolo Bonzini 
---
  qga/commands-posix.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 7a065c4085c..7f05996495a 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2173,7 +2173,7 @@ void qmp_guest_set_user_password(const char *username,
  }
  
  #ifdef __FreeBSD__

-g_autofree char *chpasswdata = g_strdup(rawpasswddata);
+g_autofree char *chpasswddata = g_strdup(rawpasswddata);
  const char *crypt_flag = crypted ? "-H" : "-h";
  const char *argv[] = {"pw", "usermod", "-n", username,
crypt_flag, "0", NULL};


Fixes: 0e5b75a390 ("qga/commands-posix: qmp_guest_set_user_password: use 
ga_run_command helper")

Reviewed-by: Thomas Huth 

Re: [PATCH 03/14] target/s390x: Update CR9 bits

[Thomas Huth]

On 02/05/2024 07.44, Richard Henderson wrote:

Update from the PoO 14th edition.

Signed-off-by: Richard Henderson 
---
  target/s390x/cpu.h | 18 +++---
  target/s390x/tcg/misc_helper.c |  2 +-
  2 files changed, 12 insertions(+), 8 deletions(-)



Reviewed-by: Thomas Huth 

Re: [PATCH v2] Re-enable riscv64-debian-cross-container (debian riscv64 is finally usable again!)

[Thomas Huth]

On 03/05/2024 09.16, Michael Tokarev wrote:

Revert "gitlab-ci: Disable the riscv64-debian-cross-container by default"
This reverts commit f51f90c65ed7706c3c4f7a889ce3d6b7ab75ef6a.

riscv64 in debian has been non-functioning for almost a year, after the
architecture has been promoted to release architecture and all binary
packages started to be re-built, making the port not multi-arch-co-installable
for a long time (in debian, multi-arch packages must be of the same version,
but when a package is rebuilt on one architecture it gets a version bump too).
Later on, debiah had a long time64_t transition which made sid unusable for
quite some time too.  Both such events happens in debian very rarely (like,
once in 10 years or so - for example, previous big transition like that was
libc5 => libc6 transition).  Now both of these are finished (where qemu is
concerned anyway).

Hopefully debian unstable wont be very unstable.  At the very least it is
better to have sporadic CI failures here than no riscv64 coverage at all.

Signed-off-by: Michael Tokarev 
---
v2: drop a TODO comment which turned out to be confused, replacing it
  with the description why debian riscv64 were unusable.

  .gitlab-ci.d/container-cross.yml | 1 -
  1 file changed, 1 deletion(-)

diff --git a/.gitlab-ci.d/container-cross.yml b/.gitlab-ci.d/container-cross.yml
index e3103940a0..dbffed3f21 100644
--- a/.gitlab-ci.d/container-cross.yml
+++ b/.gitlab-ci.d/container-cross.yml
@@ -77,7 +77,6 @@ riscv64-debian-cross-container:
allow_failure: true
variables:
  NAME: debian-riscv64-cross
-QEMU_JOB_OPTIONAL: 1


Reviewed-by: Thomas Huth 

Re: [PATCH v2] KVM: selftests: Use TAP interface in the set_memory_region test

[Thomas Huth]

On 02/05/2024 21.37, Sean Christopherson wrote:

On Fri, Apr 26, 2024, Thomas Huth wrote:

Use the kselftest_harness.h interface in this test to get TAP
output, so that it is easier for the user to see what the test
is doing. (Note: We are not using the KVM_ONE_VCPU_TEST_SUITE()
macro here since these tests are creating their VMs with the
vm_create_barebones() function, not with vm_create_with_one_vcpu())

Reviewed-by: Andrew Jones 
Signed-off-by: Thomas Huth 
---
  v2:
  - Rebase to linux-next branch
  - Make "loops" variable static
  - Added Andrew's Reviewed-by

  .../selftests/kvm/set_memory_region_test.c| 86 +--
  1 file changed, 42 insertions(+), 44 deletions(-)

diff --git a/tools/testing/selftests/kvm/set_memory_region_test.c 
b/tools/testing/selftests/kvm/set_memory_region_test.c
index 68c899d27561..a5c9bee5235a 100644
--- a/tools/testing/selftests/kvm/set_memory_region_test.c
+++ b/tools/testing/selftests/kvm/set_memory_region_test.c
@@ -16,6 +16,7 @@
  #include 
  #include 
  #include 
+#include "kselftest_harness.h"
  
  /*

   * s390x needs at least 1MB alignment, and the x86_64 MOVE/DELETE tests need a
@@ -38,6 +39,8 @@ extern const uint64_t final_rip_end;
  
  static sem_t vcpu_ready;
  
+static int loops;


...


-static void test_add_overlapping_private_memory_regions(void)
+TEST(add_overlapping_private_memory_regions)
  {
struct kvm_vm *vm;
int memfd;
int r;
  
-	pr_info("Testing ADD of overlapping KVM_MEM_GUEST_MEMFD memory regions\n");

+   if (!has_cap_guest_memfd())
+   SKIP(return, "Missing KVM_MEM_GUEST_MEMFD / 
KVM_X86_SW_PROTECTED_VM");


I like that we can actually report sub-tests as being skipped, but I don't like
having multiple ways to express requirements.  And IMO, this is much less 
readable
than TEST_REQUIRE(has_cap_guest_memfd());

AIUI, each test runs in a child process, so TEST_REQUIRE() can simply exit(), it
just needs to avoid ksft_exit_skip() so that a sub-test doesn't spit out the 
full
test summary.

And if using exit() isn't an option, setjmp()+longjmp() will do the trick (I got
that working for KVM_ONE_VCPU_TEST() before I realized tests run as a child).

The below is lightly tested, but I think it does what we want?


Not quite ... for example, if I force vmx_pmu_caps_test to skip the last 
test, I get:


TAP version 13
1..5
# Starting 5 tests from 1 test cases.
#  RUN   vmx_pmu_caps.guest_wrmsr_perf_capabilities ...
#OK  vmx_pmu_caps.guest_wrmsr_perf_capabilities
ok 1 vmx_pmu_caps.guest_wrmsr_perf_capabilities
#  RUN   vmx_pmu_caps.basic_perf_capabilities ...
#OK  vmx_pmu_caps.basic_perf_capabilities
ok 2 vmx_pmu_caps.basic_perf_capabilities
#  RUN   vmx_pmu_caps.fungible_perf_capabilities ...
#OK  vmx_pmu_caps.fungible_perf_capabilities
ok 3 vmx_pmu_caps.fungible_perf_capabilities
#  RUN   vmx_pmu_caps.immutable_perf_capabilities ...
#OK  vmx_pmu_caps.immutable_perf_capabilities
ok 4 vmx_pmu_caps.immutable_perf_capabilities
#  RUN   vmx_pmu_caps.lbr_perf_capabilities ...
ok 5 # SKIP - Requirement not met: host_cap.lbr_format && 0
#OK  vmx_pmu_caps.lbr_perf_capabilities
ok 5 vmx_pmu_caps.lbr_perf_capabilities
# PASSED: 5 / 5 tests passed.
# Totals: pass:5 fail:0 xfail:0 xpass:0 skip:0 error:0

As you can see, the "ok 5" line is duplicated now, once marked with "# SKIP" 
and once as successfull. I don't think that this is valid TAP anymore?



I also think we would effectively forbid direct use of TEST().  Partly because
it's effectively necessary to use TEST_REQUIRE(), but also so that all tests 
will
have an existing single point of contact if we need/want to make similar changes
in the future.


Ok, but I wrote in the patch description, KVM_ONE_VCPU_TEST_SUITE() does not 
work for the set_memory_region test since it does not like to have a 
pre-defined vcpu ... so if we want to forbid TEST(), I assume we'd need 
another macro like KVM_BAREBONE_TEST_SUITE() ?


Not sure whether I really like it, though, since I'd prefer if we could keep 
the possibility to use the original selftest macros (for people who are 
already used to those macros from other selftests).


 Thomas

Re: [PULL 7/9] qga/commands-posix: qmp_guest_set_user_password: use ga_run_command helper

[Thomas Huth]

On 01/05/2024 09.43, Konstantin Kostiuk wrote:

From: Andrey Drobyshev 

There's no need to check for the existence of the "chpasswd", "pw"
executables, as the exec() call will do that for us.

Signed-off-by: Andrey Drobyshev 
Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Konstantin Kostiuk 
Link: 
https://lore.kernel.org/r/20240320161648.158226-8-andrey.drobys...@virtuozzo.com
Signed-off-by: Konstantin Kostiuk 
---
  qga/commands-posix.c | 96 ++--
  1 file changed, 13 insertions(+), 83 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 9910957ff5..7a065c4085 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2151,14 +2151,8 @@ void qmp_guest_set_user_password(const char *username,
   Error **errp)
  {
  Error *local_err = NULL;
-char *passwd_path = NULL;
-pid_t pid;
-int status;
-int datafd[2] = { -1, -1 };
-char *rawpasswddata = NULL;
+g_autofree char *rawpasswddata = NULL;
  size_t rawpasswdlen;
-char *chpasswddata = NULL;
-size_t chpasswdlen;
  
  rawpasswddata = (char *)qbase64_decode(password, -1, , errp);

  if (!rawpasswddata) {
@@ -2169,95 +2163,31 @@ void qmp_guest_set_user_password(const char *username,
  
  if (strchr(rawpasswddata, '\n')) {

  error_setg(errp, "forbidden characters in raw password");
-goto out;
+return;
  }
  
  if (strchr(username, '\n') ||

  strchr(username, ':')) {
  error_setg(errp, "forbidden characters in username");
-goto out;
+return;
  }
  
  #ifdef __FreeBSD__

-chpasswddata = g_strdup(rawpasswddata);
-passwd_path = g_find_program_in_path("pw");
+g_autofree char *chpasswdata = g_strdup(rawpasswddata);


 Hi!

This broke compilation on FreeBSD:

 https://gitlab.com/qemu-project/qemu/-/jobs/6760232764#L7125

Looks like a typo in the variable name?

 Thomas

Re: [PATCH 02/14] target/s390x: Move cpu_get_tb_cpu_state out of line

[Thomas Huth]

On 02/05/2024 07.44, Richard Henderson wrote:

Signed-off-by: Richard Henderson 
---
  target/s390x/cpu.h | 23 ++-
  target/s390x/cpu.c | 22 ++
  2 files changed, 24 insertions(+), 21 deletions(-)


Reviewed-by: Thomas Huth 

Re: [PATCH] Re-enable riscv64-debian-cross-container (debian riscv64 is finally usable again!)

[Thomas Huth]

On 02/05/2024 22.08, Michael Tokarev wrote:

02.05.2024 23:05, Richard Henderson wrote:

On 5/2/24 12:40, Michael Tokarev wrote:

Revert "gitlab-ci: Disable the riscv64-debian-cross-container by default"
This reverts commit f51f90c65ed7706c3c4f7a889ce3d6b7ab75ef6a.

Hopefully it wont be very unstable.

Since riscv64 is now a release architecture in debian, we can try switching
to debian testing instead of debian sid.  Also, debian-ports part of the
archive isn't needed anymore.


So... please update debian-riscv64-cross.docker.


debian-riscv64-cross.docker is generated by lcitool.
It looks like lcitool has to be updated for this.


Updating lcitool isn't too hard. Simply type "make lcitool-refresh" in your 
build directory - this should initialize the tests/lcitool/libvirt-ci 
submodule and regenerate the dockerfiles. Then you can do your change in 
tests/lcitool/libvirt-ci and run the refresh command again to update the 
dockerfiles. Once you're happy with the results, fork 
https://gitlab.com/libvirt/libvirt-ci on gitlab, push your changes into your 
fork and then open a merge request against the upstream libvirt-ci repository.



Signed-off-by: Michael Tokarev 
---
  .gitlab-ci.d/container-cross.yml | 1 -
  1 file changed, 1 deletion(-)

diff --git a/.gitlab-ci.d/container-cross.yml 
b/.gitlab-ci.d/container-cross.yml

index e3103940a0..dbffed3f21 100644
--- a/.gitlab-ci.d/container-cross.yml
+++ b/.gitlab-ci.d/container-cross.yml
@@ -77,7 +77,6 @@ riscv64-debian-cross-container:
    allow_failure: true
    variables:
  NAME: debian-riscv64-cross
-    QEMU_JOB_OPTIONAL: 1


But you can't do this until the above is done.


I don't see a reason why not.  Adding debian-ports archive does nothing
and doesn't hurt either.

Switching to testing is a different matter.

This thing works now with sid/unstable, this is what this patch is about.


But this doesn't match the patch description - there you talk about 
switching to testing and that the ports are not needed anymore, so this is 
confusing. I'd suggest to update at least the patch description if we 
continue with sid.


 Thomas

Re: [PATCH 3/3] s390x/sclp: Simplify get_sclp_device()

[Thomas Huth]

On 02/05/2024 15.15, Cédric Le Goater wrote:

get_sclp_device() scans the whole machine to find a TYPE_SCLP object.
Now that the SCLPDevice instance is available under the machine state,
use it to simplify the lookup. While at it, remove the inline to let
the compiler decide on how to optimize.

Signed-off-by: Cédric Le Goater 
---
  hw/s390x/sclp.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 
d236dbaf0bdd15c3fc07749a98a5813e05cfb9a1..e725dcd5fdfd159f20307e930a38bed3326c9e0e
 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -21,13 +21,14 @@
  #include "hw/s390x/s390-pci-bus.h"
  #include "hw/s390x/ipl.h"
  #include "hw/s390x/cpu-topology.h"
+#include "hw/s390x/s390-virtio-ccw.h"
  
-static inline SCLPDevice *get_sclp_device(void)

+static SCLPDevice *get_sclp_device(void)
  {
  static SCLPDevice *sclp;
  
  if (!sclp) {

-sclp = SCLP(object_resolve_path_type("", TYPE_SCLP, NULL));
+sclp = S390_CCW_MACHINE(qdev_get_machine())->sclp;
  }
  return sclp;
  }


Reviewed-by: Thomas Huth 

Re: [PATCH 2/3] s390x/event-facility: Simplify sclp_get_event_facility_bus()

[Thomas Huth]

On 02/05/2024 15.15, Cédric Le Goater wrote:

sclp_get_event_facility_bus() scans the whole machine to find a
TYPE_SCLP_EVENTS_BUS object. The SCLPDevice instance is now available
under the machine state, use it to simplify the lookup and adjust the
creation of the consoles.

Signed-off-by: Cédric Le Goater 
---
  include/hw/s390x/event-facility.h |  2 +-
  hw/s390x/event-facility.c | 13 ++---
  hw/s390x/s390-virtio-ccw.c| 12 +++-
  3 files changed, 10 insertions(+), 17 deletions(-)


Reviewed-by: Thomas Huth 

Re: [PATCH 1/3] s390x: Introduce a SCLPDevice pointer under the machine

[Thomas Huth]

On 02/05/2024 15.15, Cédric Le Goater wrote:

Initialize directly SCLPDevice from the machine init handler and
remove s390_sclp_init(). We will use the SCLPDevice pointer later to
create the consoles.

Signed-off-by: Cédric Le Goater 
---


Reviewed-by: Thomas Huth 

Re: [PATCH 01/14] include/hw: add helpers for defining versioned machine types

[Thomas Huth]

On 02/05/2024 16.57, Eric Blake wrote:

On Wed, May 01, 2024 at 07:27:46PM +0100, Daniel P. Berrangé wrote:

The various targets which define versioned machine types have
a bunch of obfuscated macro code for defining unique function
and variable names using string concatenation.

This addes a couple of helpers to improve the clarity of such
code macro.

Signed-off-by: Daniel P. Berrangé 
---

...

+ *  // Normal 2 digit eg 'virt-9.0'
+ *  #define DEFINE_VIRT_MACHINE(major, minor) \
+ *  DEFINE_VIRT_MACHINE_IMPL(false, major, minor)
+ *
+ *  // Bugfix 3 digit  eg 'virt-9.0.1'


Inconsistent on whether you are using one or two spaces before 'eg'.


While you're at it, maybe also better spell it with dots: e.g.

 Thomas

Re: [PATCH 02/14] hw/arm: convert 'virt' machine definitions to use new macros

[Thomas Huth]

On 01/05/2024 20.27, Daniel P. Berrangé wrote:

This changes the DEFINE_VIRT_MACHINE macro to use the common
helpers for constructing versioned symbol names and strings,
bringing greater consistency across targets.

Signed-off-by: Daniel P. Berrangé 
---
  hw/arm/virt.c | 28 +++-
  1 file changed, 15 insertions(+), 13 deletions(-)


Reviewed-by: Thomas Huth 

Re: [PATCH 13/14] hw/i386: remove obsolete manual deprecation reason string of i440fx machines

[Thomas Huth]

On 01/05/2024 20.27, Daniel P. Berrangé wrote:

The automatic deprecation mechanism introduced in the preceeding patches
will mark every i440fx machine upto and including 2.12 as deprecated. As
such we can revert the manually added deprecation which was a subset:

   commit c7437f0ddb8ee45bf96d949ddfcbb7697ae3d415
   Author: Thomas Huth 
   Date:   Fri Oct 6 09:52:47 2023 +0200

 docs/about: Mark the old pc-i440fx-2.0 - 2.3 machine types as deprecated

Signed-off-by: Daniel P. Berrangé 
---
  hw/i386/pc_piix.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 9f92504cc4..4137e03f6f 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -812,7 +812,6 @@ static void pc_i440fx_machine_2_3_options(MachineClass *m)
  {
  pc_i440fx_machine_2_4_options(m);
  m->hw_version = "2.3.0";
-m->deprecation_reason = "old and unattended - use a newer version instead";
  compat_props_add(m->compat_props, hw_compat_2_3, hw_compat_2_3_len);
  compat_props_add(m->compat_props, pc_compat_2_3, pc_compat_2_3_len);
  }


This will conflict with Philippe's "hw/i386: Remove deprecated pc-i440fx-2.0 
-> 2.3 machines" patch series (which should easy to resolve, though).


Anyway:
Reviewed-by: Thomas Huth 

Re: [PATCH 12/14] hw/ppc: remove obsolete manual deprecation reason string of spapr machines

[Thomas Huth]

On 01/05/2024 20.27, Daniel P. Berrangé wrote:

The automatic deprecation mechanism introduced in the preceeding patches
will mark every spapr machine upto and including 2.12 as deprecated. As
such we can revert the manually added deprecation which was a subset:

   commit 1392617d35765d5d912625fbb5cab1ffbed8e140
   Author: Cédric Le Goater 
   Date:   Tue Jan 23 16:37:02 2024 +1000

 spapr: Tag pseries-2.1 - 2.11 machines as deprecated

Signed-off-by: Daniel P. Berrangé 
---
  hw/ppc/spapr.c | 1 -
  1 file changed, 1 deletion(-)


Reviewed-by: Thomas Huth 

Re: [PATCH 11/14] hw: skip registration of outdated versioned machine types

[Thomas Huth]

On 01/05/2024 20.27, Daniel P. Berrangé wrote:

This calls the MACHINE_VER_DELETION() macro in the machine type
registration method, so that when a versioned machine type reaches
the end of its life, it is no longer registered with QOM and thus
cannot be used.

The actual definition of the machine type should be deleted at
this point, but experience shows that can easily be forgotten.
By skipping registration the manual code deletion task can be
done at any later date.

Signed-off-by: Daniel P. Berrangé 
---
  hw/arm/virt.c  | 1 +
  hw/m68k/virt.c | 1 +
  hw/ppc/spapr.c | 1 +
  hw/s390x/s390-virtio-ccw.c | 1 +
  include/hw/i386/pc.h   | 1 +
  5 files changed, 5 insertions(+)


Reviewed-by: Thomas Huth 

Re: [PATCH 05/14] hw/m68k: convert 'virt' machine definitions to use new macros

[Thomas Huth]

On 01/05/2024 20.27, Daniel P. Berrangé wrote:

This changes the DEFINE_VIRT_MACHINE macro to use the common
helpers for constructing versioned symbol names and strings,
bringing greater consistency across targets.

A DEFINE_VIRT_MACHINE_AS_LATEST helper is added so that it
is not required to pass 'false' for every single historical
machine type.

Signed-off-by: Daniel P. Berrangé 
---
  hw/m68k/virt.c | 51 --
  1 file changed, 29 insertions(+), 22 deletions(-)


Reviewed-by: Thomas Huth 

Re: [PATCH 04/14] hw/ppc: convert 'spapr' machine definitions to use new macros

[Thomas Huth]

On 01/05/2024 20.27, Daniel P. Berrangé wrote:

This changes the DEFINE_SPAPR_MACHINE macro to use the common
helpers for constructing versioned symbol names and strings,
bringing greater consistency across targets.

The added benefit is that it avoids the need to repeat the
version number twice in two different formats in the calls
to DEFINE_SPAPR_MACHINE.

A DEFINE_SPAPR_MACHINE_AS_LATEST helper is added so that it
is not required to pass 'false' for every single historical
machine type.

Due to the odd-ball '2.12-sxxm' machine type version, this
commit introduces a DEFINE_SPAPR_MACHINE_TAGGED helper to
allow defining of "tagged" machine types which have a string
suffix.

Signed-off-by: Daniel P. Berrangé 
---
  hw/ppc/spapr.c | 93 +++---
  1 file changed, 51 insertions(+), 42 deletions(-)


Reviewed-by: Thomas Huth 

Re: [PATCH 03/14] hw/s390x: convert 'ccw' machine definitions to use new macros

[Thomas Huth]

On 01/05/2024 20.27, Daniel P. Berrangé wrote:

This changes the DEFINE_CCW_MACHINE macro to use the common
helpers for constructing versioned symbol names and strings,
bringing greater consistency across targets.

The added benefit is that it avoids the need to repeat the
version number twice in two different formats in the calls
to DEFINE_CCW_MACHINE.

A DEFINE_CCW_MACHINE_AS_LATEST helper is added so that it
is not required to pass 'false' for every single historical
machine type.

Signed-off-by: Daniel P. Berrangé 
---
  hw/s390x/s390-virtio-ccw.c | 96 +-
  1 file changed, 53 insertions(+), 43 deletions(-)


Reviewed-by: Thomas Huth 

Re: [PATCH 10/14] hw: set deprecation info for all versioned machine types

[Thomas Huth]

On 01/05/2024 20.27, Daniel P. Berrangé wrote:

This calls the MACHINE_VER_DEPRECATION() macro in the definition of
all machine type classes which support versioning. This ensures
that they will automatically get deprecation info set when they
reach the appropriate point in their lifecycle.

Signed-off-by: Daniel P. Berrangé 
---
  hw/arm/virt.c  | 1 +
  hw/m68k/virt.c | 1 +
  hw/ppc/spapr.c | 1 +
  hw/s390x/s390-virtio-ccw.c | 1 +
  include/hw/i386/pc.h   | 1 +
  5 files changed, 5 insertions(+)


Reviewed-by: Thomas Huth 

Re: [PATCH 09/14] hw: temporarily disable deletion of versioned machine types

[Thomas Huth]

On 01/05/2024 20.27, Daniel P. Berrangé wrote:

The new deprecation and deletion policy for versioned machine types is
being introduced in QEMU 9.1.0.

Under the new policy a number of old machine types (any prior to 2.12)
would be liable for immediate deletion which would be a violation of our
historical deprecation and removal policy

Thus automatic deletions (by skipping QOM registration) are temporarily
gated on existance of the env variable "QEMU_DELETE_MACHINES" / QEMU
version number >= 10.1.0. This allows opt-in testing of the automatic
deletion logic, while activating it fully in QEMU >= 10.1.0.


If we include your patches now, the old machine types will be marked as 
deprecated in QEMU 9.1 and 9.2, so it should be OK to remove them in 10.0 
already, shouldn't it?


 Thomas

Re: [PATCH 08/14] include/hw: add macros for deprecation & removal of versioned machines

[Thomas Huth]

On 01/05/2024 20.27, Daniel P. Berrangé wrote:

Versioned machines live for a long time to provide back compat for
incoming migration and restore of saved images. To guide users away from
usage of old machines, however, we want to deprecate any older than 3
years (equiv of 9 releases), and delete any older than 6 years (equiva
of 18 releases).

To get a standardized deprecation message and avoid having to remember
to manually add it after three years, this introduces two macros to be
used by targets when defining versioned machines.

* MACHINE_VER_DEPRECATION(major, minor)

   Automates the task of setting the 'deprecation_reason' field on the
   machine, if-and-only-if the major/minor version is older than 3 years.

* MACHINE_VER_DEPRECATION(major, minor)


That should be MACHINE_VER_DELETION instead.


   Simulates the deletion of by skipping registration of the QOM type
   for a versioned machine, if-and-only-if the major/minor version is
   older than 6 years.

By using these two macros there is no longer any manual work required
per-release to deprecate old machines. By preventing the use of machines
that have reached their deletion date, it is also no neccessary to


s/neccessary/necessary/


manually delete machines per-release. Deletion can be batched up once a
year or whenever makes most sense.

Signed-off-by: Daniel P. Berrangé 
---
  include/hw/boards.h | 84 +
  1 file changed, 84 insertions(+)


With the typos fixed:
Reviewed-by: Thomas Huth 

Re: [PATCH 01/14] include/hw: add helpers for defining versioned machine types

[Thomas Huth]

On 01/05/2024 20.27, Daniel P. Berrangé wrote:

The various targets which define versioned machine types have
a bunch of obfuscated macro code for defining unique function
and variable names using string concatenation.

This addes a couple of helpers to improve the clarity of such
code macro.

Signed-off-by: Daniel P. Berrangé 
---
  include/hw/boards.h | 166 
  1 file changed, 166 insertions(+)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index 2fa800f11a..47ca450fca 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -414,6 +414,172 @@ struct MachineState {
  struct NumaState *numa_state;
  };
  
+/*

+ * The macros which follow are intended to facilitate the
+ * definition of versioned machine types, using a somewhat
+ * similar pattern across targets:


I'd suggest to add a sentence at the end saying something like this: "For 
example, to create the macro for setting up a versioned "virt" machine could 
look like this:" (otherwise it's not immediately clear whether the example 
is about only the "macros which follow" or whether it's about how to set up 
a machine type)



+ *  #define DEFINE_VIRT_MACHINE_IMPL(latest, ...) \
+ *  static void MACHINE_VER_SYM(class_init, virt, __VA_ARGS__)( \
+ *  ObjectClass *oc, \
+ *  void *data) \
+ *  { \
+ *  MachineClass *mc = MACHINE_CLASS(oc); \
+ *  MACHINE_VER_SYM(options, virt, __VA_ARGS__)(mc); \
+ *  mc->desc = "QEMU " MACHINE_VER_STR(__VA_ARGS__) " Virtual 
Machine"; \
+ *  if (latest) { \
+ *  mc->alias = "virt"; \
+ *  } \
+ *  } \
+ *  static const TypeInfo MACHINE_VER_SYM(info, virt, __VA_ARGS__) = { \
+ *  .name = MACHINE_VER_TYPE_NAME("virt", __VA_ARGS__), \
+ *  .parent = TYPE_VIRT_MACHINE, \
+ *  .class_init = MACHINE_VER_SYM(class_init, virt, __VA_ARGS__), \
+ *  }; \
+ *  static void MACHINE_VER_SYM(register, virt, __VA_ARGS__)(void) \
+ *  { \
+ *  type_register_static(_VER_SYM(info, virt, __VA_ARGS__)); \
+ *  } \
+ *  type_init(MACHINE_VER_SYM(register, virt, __VA_ARGS__));
+ *
+ * Following this, one (or more) helpers can be added for
+ * whichever scenarios need to be catered for with a machine:
+ *
+ *  // Normal 2 digit, marked as latest eg 'virt-9.0'
+ *  #define DEFINE_VIRT_MACHINE_LATEST(major, minor) \
+ *  DEFINE_VIRT_MACHINE_IMPL(true, major, minor)
+ *
+ *  // Normal 2 digit eg 'virt-9.0'
+ *  #define DEFINE_VIRT_MACHINE(major, minor) \
+ *  DEFINE_VIRT_MACHINE_IMPL(false, major, minor)
+ *
+ *  // Bugfix 3 digit  eg 'virt-9.0.1'
+ *  #define DEFINE_VIRT_MACHINE_BUGFIX(major, minor, micro) \
+ *  DEFINE_VIRT_MACHINE_IMPL(false, major, minor, micro)
+ *
+ *  // Tagged 2 digit  eg 'virt-9.0-extra'
+ *  #define DEFINE_VIRT_MACHINE_TAGGED(major, minor, tag) \
+ *  DEFINE_VIRT_MACHINE_IMPL(false, major, minor, _, tag)
+ *
+ *  // Tagged bugffix 2 digit  eg 'virt-9.0.1-extra'


s/bugffix/bugfix/


+ *  #define DEFINE_VIRT_MACHINE_TAGGED(major, minor, micro, tag) \
+ *  DEFINE_VIRT_MACHINE_IMPL(false, major, minor, micro, _, tag)
+ */


I'd suggest to add a separate comment for the macro below, explaining that 
it is supposed to be used with __VA_ARGS__ to pick a certain other macro 
depending on the amount of entries in __VA_ARGS__.



+#define _MACHINE_VER_PICK(x1, x2, x3, x4, x5, x6, ...) x6
+
+/*
+ * Construct a human targetted machine version string.


s/targetted/targeted/ according to my spell checker ?

Apart from the nits:
Reviewed-by: Thomas Huth 

 Thomas

Re: [PATCH 14/14] docs: document special exception for machine type deprecation & removal

[Thomas Huth]

On 01/05/2024 20.27, Daniel P. Berrangé wrote:

This extends the deprecation policy to indicate that versioned machine
types will be marked deprecated after 3 years, and then subject to
removal after a further 3 years has passed.

Signed-off-by: Daniel P. Berrangé 
---
  docs/about/deprecated.rst | 12 
  1 file changed, 12 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 7b8aafa15b..55120e774c 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -11,6 +11,18 @@ releases, the feature is liable to be removed. Deprecated 
features may also
  generate warnings on the console when QEMU starts up, or if activated via a
  monitor command, however, this is not a mandatory requirement.
  
+As a special exception to this general timeframe, rather than have an

+indefinite lifetime, versioned machine types are only intended to be
+supported for a period of 6 years, equivalent to 18 QEMU releases. All
+versioned machine types will be automatically marked deprecated after an
+initial 3 years (9 QEMU releases) has passed, and will then be deleted after


Should there be the word "period" after "3 years" ? Otherwise it sounds a 
little bit weird to me - but I'm also not a native speaker, so I might be wrong.



+a further 3 year period has passed. It is recommended that a deprecated
+machine type is only used for incoming migrations and restore of saved state,
+for pre-existing VM deployments.


Should we maybe add a sentence that they should ideally be updated to a 
newer machine type during a service window with downtime? (well, it might be 
also obvious, so maybe not worth to mention it)



 Newly deployed VMs should exclusively use a
+non-deprecated machine type, with use of the most recent version highly
+recommended. Non-versioned machine types follow the general feature
+deprecation policy.


Anyway:
Reviewed-by: Thomas Huth 

Re: [PATCH v2] hw/s390x: Attach the sclpconsole to /machine/sclp/s390-sclp-event-facility

[Thomas Huth]

On 02/05/2024 09.57, Cédric Le Goater wrote:

On 4/30/24 21:08, Thomas Huth wrote:

The sclpconsole currently does not have a proper parent in the QOM
tree, so it shows up under /machine/unattached - which is somewhat
ugly. We should rather attach it to /machine/sclp/s390-sclp-event-facility
where the other devices of type TYPE_SCLP_EVENT already reside.

Signed-off-by: Thomas Huth 


Reviewed-by: Cédric Le Goater 


---
  hw/s390x/s390-virtio-ccw.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 5c83d1ea17..41be8bf857 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -242,11 +242,13 @@ static void s390_create_virtio_net(BusState *bus, 
const char *name)

  static void s390_create_sclpconsole(const char *type, Chardev *chardev)
  {
+    BusState *ev_fac_bus = sclp_get_event_facility_bus();


This routine sclp_get_event_facility_bus() which scans all the machine
could be avoided and even removed. For that, SCLPDevice should be an
attribute of the machine. This means reshuffling definitions and
object instanciations a little in the code. Would you be OK with that ?


Sounds like a good clean-up, yes!

 Thomas

Re: QEMU headers in C++

[Thomas Huth]

On 02/05/2024 06.40, Roman Kiryanov wrote:

Hi QEMU,

I work in Android Studio Emulator and we would like to develop devices
in C++. Unfortunately, QEMU headers cannot be used with C++ as is
(e.g. they use C++ keywords as variable names or implicitly cast void*
to T*).


Can't you simply use something like:

extern "C" {
#include 
}

to include the QEMU headers?

 Thomas

[PATCH v2] hw/s390x: Attach the sclpconsole to /machine/sclp/s390-sclp-event-facility

[Thomas Huth]

The sclpconsole currently does not have a proper parent in the QOM
tree, so it shows up under /machine/unattached - which is somewhat
ugly. We should rather attach it to /machine/sclp/s390-sclp-event-facility
where the other devices of type TYPE_SCLP_EVENT already reside.

Signed-off-by: Thomas Huth 
---
 hw/s390x/s390-virtio-ccw.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 5c83d1ea17..41be8bf857 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -242,11 +242,13 @@ static void s390_create_virtio_net(BusState *bus, const 
char *name)
 
 static void s390_create_sclpconsole(const char *type, Chardev *chardev)
 {
+BusState *ev_fac_bus = sclp_get_event_facility_bus();
 DeviceState *dev;
 
 dev = qdev_new(type);
+object_property_add_child(OBJECT(ev_fac_bus->parent), type, OBJECT(dev));
 qdev_prop_set_chr(dev, "chardev", chardev);
-qdev_realize_and_unref(dev, sclp_get_event_facility_bus(), _fatal);
+qdev_realize_and_unref(dev, ev_fac_bus, _fatal);
 }
 
 static void ccw_init(MachineState *machine)
-- 
2.44.0

Re: [PATCH] hw/s390x: Attach the sclpconsole to the /machine/sclp node

[Thomas Huth]

On 30/04/2024 16.24, Thomas Huth wrote:

On 30/04/2024 13.58, Cédric Le Goater wrote:

On 4/30/24 10:04, Thomas Huth wrote:

The sclpconsole currently does not have a proper parent in the QOM
tree, so it shows up under /machine/unattached - which is somewhat
ugly. Let's attach it to /machine/sclp instead.

Signed-off-by: Thomas Huth 
---
  include/hw/s390x/sclp.h    |  2 +-
  hw/s390x/s390-virtio-ccw.c | 11 +++
  hw/s390x/sclp.c    |  4 +++-
  3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
index b405a387b6..abfd6d8868 100644
--- a/include/hw/s390x/sclp.h
+++ b/include/hw/s390x/sclp.h
@@ -222,7 +222,7 @@ static inline int sccb_data_len(SCCB *sccb)
  }
-void s390_sclp_init(void);
+Object *s390_sclp_init(void);
  void sclp_service_interrupt(uint32_t sccb);
  void raise_irq_cpu_hotplug(void);
  int sclp_service_call(S390CPU *cpu, uint64_t sccb, uint32_t code);
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 4dcc213820..e2f9206ded 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -237,11 +237,13 @@ static void s390_create_virtio_net(BusState *bus, 
const char *name)

  }
  }
-static void s390_create_sclpconsole(const char *type, Chardev *chardev)
+static void s390_create_sclpconsole(Object *sclp, const char *type,
+    Chardev *chardev)
  {
  DeviceState *dev;
  dev = qdev_new(type);
+    object_property_add_child(sclp, type, OBJECT(dev));
  qdev_prop_set_chr(dev, "chardev", chardev);
  qdev_realize_and_unref(dev, sclp_get_event_facility_bus(), 
_fatal);

  }
@@ -252,8 +254,9 @@ static void ccw_init(MachineState *machine)
  int ret;
  VirtualCssBus *css_bus;
  DeviceState *dev;
+    Object *sclp;
-    s390_sclp_init();
+    sclp = s390_sclp_init();


I would simply drop s390_sclp_init(), same for :

   void s390_init_tod(void);
   void s390_init_ap(void);
   void s390_stattrib_init(void);
   void s390_skeys_init(void);
   void s390_flic_init(void);

These routines all do the same and are not very useful TBH, and I would
add pointers under the s390x MachineState possibly.


Some of them seem to do a little bit more things, like checking whether the 
feature is available or not, e.g. s390_init_ap() ... IMHO it makes sense to 
keep at least those?


But for s390_sclp_init ... it could be inlined, indeed, especially if we 
also switch the object_unref + qdev_realize in there into 
qdev_realize_and_unref. Let me try to do that in a v2 ...


Actually, after looking at the code a little bit longer, it seems to me like 
the sclpconsole should be attached to /machine/sclp/s390-sclp-event-facility
instead of just /machine/sclp, since the other devices of type 
TYPE_SCLP_EVENT are also located there. That makes the patch even easier 
since we already have the pointer from sclp_get_event_facility_bus() in that 
function.


 Thomas

Re: [PATCH] hw/s390x: Attach the sclpconsole to the /machine/sclp node

[Thomas Huth]

On 30/04/2024 13.58, Cédric Le Goater wrote:

On 4/30/24 10:04, Thomas Huth wrote:

The sclpconsole currently does not have a proper parent in the QOM
tree, so it shows up under /machine/unattached - which is somewhat
ugly. Let's attach it to /machine/sclp instead.

Signed-off-by: Thomas Huth 
---
  include/hw/s390x/sclp.h    |  2 +-
  hw/s390x/s390-virtio-ccw.c | 11 +++
  hw/s390x/sclp.c    |  4 +++-
  3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
index b405a387b6..abfd6d8868 100644
--- a/include/hw/s390x/sclp.h
+++ b/include/hw/s390x/sclp.h
@@ -222,7 +222,7 @@ static inline int sccb_data_len(SCCB *sccb)
  }
-void s390_sclp_init(void);
+Object *s390_sclp_init(void);
  void sclp_service_interrupt(uint32_t sccb);
  void raise_irq_cpu_hotplug(void);
  int sclp_service_call(S390CPU *cpu, uint64_t sccb, uint32_t code);
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 4dcc213820..e2f9206ded 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -237,11 +237,13 @@ static void s390_create_virtio_net(BusState *bus, 
const char *name)

  }
  }
-static void s390_create_sclpconsole(const char *type, Chardev *chardev)
+static void s390_create_sclpconsole(Object *sclp, const char *type,
+    Chardev *chardev)
  {
  DeviceState *dev;
  dev = qdev_new(type);
+    object_property_add_child(sclp, type, OBJECT(dev));
  qdev_prop_set_chr(dev, "chardev", chardev);
  qdev_realize_and_unref(dev, sclp_get_event_facility_bus(), 
_fatal);

  }
@@ -252,8 +254,9 @@ static void ccw_init(MachineState *machine)
  int ret;
  VirtualCssBus *css_bus;
  DeviceState *dev;
+    Object *sclp;
-    s390_sclp_init();
+    sclp = s390_sclp_init();


I would simply drop s390_sclp_init(), same for :

   void s390_init_tod(void);
   void s390_init_ap(void);
   void s390_stattrib_init(void);
   void s390_skeys_init(void);
   void s390_flic_init(void);

These routines all do the same and are not very useful TBH, and I would
add pointers under the s390x MachineState possibly.


Some of them seem to do a little bit more things, like checking whether the 
feature is available or not, e.g. s390_init_ap() ... IMHO it makes sense to 
keep at least those?


But for s390_sclp_init ... it could be inlined, indeed, especially if we 
also switch the object_unref + qdev_realize in there into 
qdev_realize_and_unref. Let me try to do that in a v2 ...


 Thomas

Re: [PATCH] docs/about: Automatically deprecate versioned machine types older than 6 years

[Thomas Huth]

On 30/04/2024 11.55, Daniel P. Berrangé wrote:

On Tue, Apr 30, 2024 at 08:45:29AM +0200, Thomas Huth wrote:

Old machine types often have bugs or work-arounds that affect our
possibilities to move forward with the QEMU code base (see for example
https://gitlab.com/qemu-project/qemu/-/issues/2213 for a bug that likely
cannot be fixed without breaking live migration with old machine types,
or https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg04516.html or
commit ea985d235b86). So instead of going through the process of manually
deprecating old machine types again and again, let's rather add an entry
that can stay, which declares that machine types older than 6 years are
considered as deprecated automatically. Six years should be sufficient to
support the release cycles of most Linux distributions.


Reading this again, I think we're mixing two concepts here.

With this 6 year cut off, we're declaring the actual *removal* date,
not the deprecation date.

A deprecation is something that happens prior to removal normally,
to give people a warning of /future/ removal, as a suggestion
that they stop using it.

If we never set the 'deprecation_reason' on a machine type, then
unless someone reads this doc, they'll never realize they are on
a deprecated machine.

When it comes to machine types, I see deprecation as a way to tell
people they should not deploy a /new/ VM on a machine type, only
use it for back compat (incoming migration / restore from saved
image) with existing deployed VMs.

If we delete a machine on the 6 year anniversary, then users
don't want to be deploying /new/ VMs using that on the
5 year anniversary as it only gives a 1 year upgrade window.

So how long far back do we consider it reasonable for a user
to deploy a /new/ VM on an old machine type ? 1 year, 2 years,
3 years ?


How about picking the half way point ?  3 years ?

ie, set deprecation_reason for any machine that is 3 years
old, but declare that our deprecation cycle lasts for
3 years, instead of the normal 1 year, when applied to
machine types.

This would give a strong hint that users should get off the
old machine type, several years before its finally deleted.


Sounds like a good idea, too! Since I have to drop this patch here anyway, 
could you maybe write such a new patch? (or do you want me to try to 
formulate this?)


 Thomas

Re: [PATCH] docs/about: Automatically deprecate versioned machine types older than 6 years

[Thomas Huth]

On 30/04/2024 11.55, Daniel P. Berrangé wrote:

On Tue, Apr 30, 2024 at 08:45:29AM +0200, Thomas Huth wrote:

Old machine types often have bugs or work-arounds that affect our
possibilities to move forward with the QEMU code base (see for example
https://gitlab.com/qemu-project/qemu/-/issues/2213 for a bug that likely
cannot be fixed without breaking live migration with old machine types,
or https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg04516.html or
commit ea985d235b86). So instead of going through the process of manually
deprecating old machine types again and again, let's rather add an entry
that can stay, which declares that machine types older than 6 years are
considered as deprecated automatically. Six years should be sufficient to
support the release cycles of most Linux distributions.


Reading this again, I think we're mixing two concepts here.

With this 6 year cut off, we're declaring the actual *removal* date,
not the deprecation date.

A deprecation is something that happens prior to removal normally,
to give people a warning of /future/ removal, as a suggestion
that they stop using it.

If we never set the 'deprecation_reason' on a machine type, then
unless someone reads this doc, they'll never realize they are on
a deprecated machine.

When it comes to machine types, I see deprecation as a way to tell
people they should not deploy a /new/ VM on a machine type, only
use it for back compat (incoming migration / restore from saved
image) with existing deployed VMs.

If we delete a machine on the 6 year anniversary, then users
don't want to be deploying /new/ VMs using that on the
5 year anniversary as it only gives a 1 year upgrade window.

So how long far back do we consider it reasonable for a user
to deploy a /new/ VM on an old machine type ? 1 year, 2 years,
3 years ?


How about picking the half way point ?  3 years ?

ie, set deprecation_reason for any machine that is 3 years
old, but declare that our deprecation cycle lasts for
3 years, instead of the normal 1 year, when applied to
machine types.

This would give a strong hint that users should get off the
old machine type, several years before its finally deleted.


Sounds like a good idea, too! Since I have to drop this patch here anyway, 
could you maybe write such a new patch? (or do you want me to try to 
formulate this?)


 Thomas
___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org

Re: [PATCH] docs/about: Automatically deprecate versioned machine types older than 6 years

[Thomas Huth]

On 30/04/2024 11.40, Philippe Mathieu-Daudé wrote:

Hi Thomas,

On 30/4/24 08:45, Thomas Huth wrote:

Old machine types often have bugs or work-arounds that affect our
possibilities to move forward with the QEMU code base (see for example
https://gitlab.com/qemu-project/qemu/-/issues/2213 for a bug that likely
cannot be fixed without breaking live migration with old machine types,
or https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg04516.html or
commit ea985d235b86). So instead of going through the process of manually
deprecating old machine types again and again, let's rather add an entry
that can stay, which declares that machine types older than 6 years are
considered as deprecated automatically. Six years should be sufficient to
support the release cycles of most Linux distributions.


Thanks for taking that out of my plate :)

IIRC 6 years was because of some old RHEL version, otherwise could
5 years be enough? (maybe it could be good enough for this old RHEL
version as of QEMU v10.0).


My thinking was like this:
1) We recently talked about marking all machine types up to 2.12 as 
deprecated recently. 2.12 has been released in 2018, so if we feel confident 
that those old machine types could go away now/soon, 2024 - 2018 = 6 years 
seem to be a good rule of thumb.
2) RHEL and OpenSuSE tend to provide feature updates in their enterprise 
distros for 5 (RHEL) or 6 (current OpenSuSE AFAIK) years, so keeping old 
machine types in upstream QEMU for that amount of time certainly also helps 
with downstream distros.



diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 6d595de3b6..fe69e2d44c 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -220,6 +220,17 @@ is a chance the code will bitrot without anyone 
noticing.

  System emulator machines
  
+Versioned machine types older than 6 years
+''
+
+Starting with the release of QEMU 10.0, versioned machine types older than


Why can't we start with QEMU 9.1?


If I put 9.1 now there, that would make it possible to immediately remove 
machines in 9.2, i.e. we would skip the 
must-be-marked-as-deprecated-for-two-releases rule here.


 Thomas

Re: [PATCH] docs/about: Automatically deprecate versioned machine types older than 6 years

[Thomas Huth]

On 30/04/2024 11.40, Philippe Mathieu-Daudé wrote:

Hi Thomas,

On 30/4/24 08:45, Thomas Huth wrote:

Old machine types often have bugs or work-arounds that affect our
possibilities to move forward with the QEMU code base (see for example
https://gitlab.com/qemu-project/qemu/-/issues/2213 for a bug that likely
cannot be fixed without breaking live migration with old machine types,
or https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg04516.html or
commit ea985d235b86). So instead of going through the process of manually
deprecating old machine types again and again, let's rather add an entry
that can stay, which declares that machine types older than 6 years are
considered as deprecated automatically. Six years should be sufficient to
support the release cycles of most Linux distributions.


Thanks for taking that out of my plate :)

IIRC 6 years was because of some old RHEL version, otherwise could
5 years be enough? (maybe it could be good enough for this old RHEL
version as of QEMU v10.0).


My thinking was like this:
1) We recently talked about marking all machine types up to 2.12 as 
deprecated recently. 2.12 has been released in 2018, so if we feel confident 
that those old machine types could go away now/soon, 2024 - 2018 = 6 years 
seem to be a good rule of thumb.
2) RHEL and OpenSuSE tend to provide feature updates in their enterprise 
distros for 5 (RHEL) or 6 (current OpenSuSE AFAIK) years, so keeping old 
machine types in upstream QEMU for that amount of time certainly also helps 
with downstream distros.



diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 6d595de3b6..fe69e2d44c 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -220,6 +220,17 @@ is a chance the code will bitrot without anyone 
noticing.

  System emulator machines
  
+Versioned machine types older than 6 years
+''
+
+Starting with the release of QEMU 10.0, versioned machine types older than


Why can't we start with QEMU 9.1?


If I put 9.1 now there, that would make it possible to immediately remove 
machines in 9.2, i.e. we would skip the 
must-be-marked-as-deprecated-for-two-releases rule here.


 Thomas

___
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-le...@lists.libvirt.org

Re: [PATCH] hw/s390x: Attach the sclpconsole to the /machine/sclp node

[Thomas Huth]

On 30/04/2024 10.19, David Hildenbrand wrote:

On 30.04.24 10:04, Thomas Huth wrote:

The sclpconsole currently does not have a proper parent in the QOM
tree, so it shows up under /machine/unattached - which is somewhat
ugly. Let's attach it to /machine/sclp instead.


IIRC, this should not affect migration



Yes, that's my understanding, too. I also did a quick check from one QEMU 
without this patch to a QEMU that includes this patch, and migration just 
succeeded fine.



Reviewed-by: David Hildenbrand 


Thanks!

 Thomas

[PATCH] hw/s390x: Attach the sclpconsole to the /machine/sclp node

[Thomas Huth]

The sclpconsole currently does not have a proper parent in the QOM
tree, so it shows up under /machine/unattached - which is somewhat
ugly. Let's attach it to /machine/sclp instead.

Signed-off-by: Thomas Huth 
---
 include/hw/s390x/sclp.h|  2 +-
 hw/s390x/s390-virtio-ccw.c | 11 +++
 hw/s390x/sclp.c|  4 +++-
 3 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
index b405a387b6..abfd6d8868 100644
--- a/include/hw/s390x/sclp.h
+++ b/include/hw/s390x/sclp.h
@@ -222,7 +222,7 @@ static inline int sccb_data_len(SCCB *sccb)
 }
 
 
-void s390_sclp_init(void);
+Object *s390_sclp_init(void);
 void sclp_service_interrupt(uint32_t sccb);
 void raise_irq_cpu_hotplug(void);
 int sclp_service_call(S390CPU *cpu, uint64_t sccb, uint32_t code);
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 4dcc213820..e2f9206ded 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -237,11 +237,13 @@ static void s390_create_virtio_net(BusState *bus, const 
char *name)
 }
 }
 
-static void s390_create_sclpconsole(const char *type, Chardev *chardev)
+static void s390_create_sclpconsole(Object *sclp, const char *type,
+Chardev *chardev)
 {
 DeviceState *dev;
 
 dev = qdev_new(type);
+object_property_add_child(sclp, type, OBJECT(dev));
 qdev_prop_set_chr(dev, "chardev", chardev);
 qdev_realize_and_unref(dev, sclp_get_event_facility_bus(), _fatal);
 }
@@ -252,8 +254,9 @@ static void ccw_init(MachineState *machine)
 int ret;
 VirtualCssBus *css_bus;
 DeviceState *dev;
+Object *sclp;
 
-s390_sclp_init();
+sclp = s390_sclp_init();
 /* init memory + setup max page size. Required for the CPU model */
 s390_memory_init(machine->ram);
 
@@ -302,10 +305,10 @@ static void ccw_init(MachineState *machine)
 
 /* init consoles */
 if (serial_hd(0)) {
-s390_create_sclpconsole("sclpconsole", serial_hd(0));
+s390_create_sclpconsole(sclp, "sclpconsole", serial_hd(0));
 }
 if (serial_hd(1)) {
-s390_create_sclpconsole("sclplmconsole", serial_hd(1));
+s390_create_sclpconsole(sclp, "sclplmconsole", serial_hd(1));
 }
 
 /* init the TOD clock */
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 893e71a41b..75d45fb184 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -379,13 +379,15 @@ void sclp_service_interrupt(uint32_t sccb)
 
 /* qemu object creation and initialization functions */
 
-void s390_sclp_init(void)
+Object *s390_sclp_init(void)
 {
 Object *new = object_new(TYPE_SCLP);
 
 object_property_add_child(qdev_get_machine(), TYPE_SCLP, new);
 object_unref(new);
 qdev_realize(DEVICE(new), NULL, _fatal);
+
+return new;
 }
 
 static void sclp_realize(DeviceState *dev, Error **errp)
-- 
2.44.0

[PULL 19/19] .gitlab-ci.d/cirrus: Remove the netbsd and openbsd jobs

[Thomas Huth]

During the past months, the netbsd and openbsd jobs in the Cirrus-CI
were broken most of the time - the setup to run a BSD in KVM on Cirrus-CI
from gitlab via the cirrus-run script was very fragile, and since the
jobs were not run by default, it used to bitrot very fast.

Now Cirrus-CI also introduce a limit on the amount of free CI minutes
that you get there, so it is not appealing at all anymore to run
these BSDs in this setup - it's better to run the checks locally via
"make vm-build-openbsd" and "make vm-build-netbsd" instead. Thus let's
remove these CI jobs now.

Message-ID: <20240426113742.654748-1-th...@redhat.com>
Signed-off-by: Thomas Huth 
---
 .gitlab-ci.d/cirrus.yml   | 37 ---
 .gitlab-ci.d/cirrus/kvm-build.yml | 31 --
 2 files changed, 68 deletions(-)
 delete mode 100644 .gitlab-ci.d/cirrus/kvm-build.yml

diff --git a/.gitlab-ci.d/cirrus.yml b/.gitlab-ci.d/cirrus.yml
index 49f86fadaf..75df1273bc 100644
--- a/.gitlab-ci.d/cirrus.yml
+++ b/.gitlab-ci.d/cirrus.yml
@@ -91,40 +91,3 @@ aarch64-macos-14-base-build:
 PKG_CONFIG_PATH: 
/opt/homebrew/curl/lib/pkgconfig:/opt/homebrew/ncurses/lib/pkgconfig:/opt/homebrew/readline/lib/pkgconfig
 TEST_TARGETS: check-unit check-block check-qapi-schema check-softfloat 
check-qtest-x86_64
 QEMU_JOB_OPTIONAL: 1
-
-
-# The following jobs run VM-based tests via KVM on a Linux-based Cirrus-CI job
-.cirrus_kvm_job:
-  extends: .base_job_template
-  stage: build
-  image: registry.gitlab.com/libvirt/libvirt-ci/cirrus-run:master
-  needs: []
-  timeout: 80m
-  script:
-- sed -e "s|[@]CI_REPOSITORY_URL@|$CI_REPOSITORY_URL|g"
-  -e "s|[@]CI_COMMIT_REF_NAME@|$CI_COMMIT_REF_NAME|g"
-  -e "s|[@]CI_COMMIT_SHA@|$CI_COMMIT_SHA|g"
-  -e "s|[@]NAME@|$NAME|g"
-  -e "s|[@]CONFIGURE_ARGS@|$CONFIGURE_ARGS|g"
-  -e "s|[@]TEST_TARGETS@|$TEST_TARGETS|g"
-  <.gitlab-ci.d/cirrus/kvm-build.yml >.gitlab-ci.d/cirrus/$NAME.yml
-- cat .gitlab-ci.d/cirrus/$NAME.yml
-- cirrus-run -v --show-build-log always .gitlab-ci.d/cirrus/$NAME.yml
-  variables:
-QEMU_JOB_CIRRUS: 1
-QEMU_JOB_OPTIONAL: 1
-
-
-x86-netbsd:
-  extends: .cirrus_kvm_job
-  variables:
-NAME: netbsd
-CONFIGURE_ARGS: --target-list=x86_64-softmmu,ppc64-softmmu,aarch64-softmmu
-TEST_TARGETS: check
-
-x86-openbsd:
-  extends: .cirrus_kvm_job
-  variables:
-NAME: openbsd
-CONFIGURE_ARGS: --target-list=i386-softmmu,riscv64-softmmu,mips64-softmmu
-TEST_TARGETS: check
diff --git a/.gitlab-ci.d/cirrus/kvm-build.yml 
b/.gitlab-ci.d/cirrus/kvm-build.yml
deleted file mode 100644
index a93881aa8b..00
--- a/.gitlab-ci.d/cirrus/kvm-build.yml
+++ /dev/null
@@ -1,31 +0,0 @@
-container:
-  image: fedora:35
-  cpu: 4
-  memory: 8Gb
-  kvm: true
-
-env:
-  CIRRUS_CLONE_DEPTH: 1
-  CI_REPOSITORY_URL: "@CI_REPOSITORY_URL@"
-  CI_COMMIT_REF_NAME: "@CI_COMMIT_REF_NAME@"
-  CI_COMMIT_SHA: "@CI_COMMIT_SHA@"
-
-@NAME@_task:
-  @NAME@_vm_cache:
-folder: $HOME/.cache/qemu-vm
-  install_script:
-- dnf update -y
-- dnf install -y git make openssh-clients qemu-img qemu-system-x86 wget 
meson
-  clone_script:
-- git clone --depth 100 "$CI_REPOSITORY_URL" .
-- git fetch origin "$CI_COMMIT_REF_NAME"
-- git reset --hard "$CI_COMMIT_SHA"
-  build_script:
-- if [ -f $HOME/.cache/qemu-vm/images/@NAME@.img ]; then
-make vm-build-@NAME@ J=$(getconf _NPROCESSORS_ONLN)
-  EXTRA_CONFIGURE_OPTS="@CONFIGURE_ARGS@"
-  BUILD_TARGET="@TEST_TARGETS@" ;
-  else
-make vm-build-@NAME@ J=$(getconf _NPROCESSORS_ONLN) BUILD_TARGET=help
-  EXTRA_CONFIGURE_OPTS="--disable-system --disable-user 
--disable-tools" ;
-  fi
-- 
2.44.0

[PULL 07/19] hw: misc: edu: fix 2 off-by-one errors

[Thomas Huth]

From: Chris Friedt 

In the case that size1 was zero, because of the explicit
'end1 > addr' check, the range check would fail and the error
message would read as shown below. The correct comparison
is 'end1 >= addr'.

EDU: DMA range 0x4-0x3 out of bounds (0x4-0x40fff)!

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1254
Signed-off-by: Chris Friedt 
[thuth: Adjust patch with regards to the "end1 <= end2" check]
Message-ID: <20221018122551.94567-1-cfri...@meta.com>
Signed-off-by: Thomas Huth 
---
 hw/misc/edu.c | 17 -
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/hw/misc/edu.c b/hw/misc/edu.c
index 2a976ca2b1..14250e0ac3 100644
--- a/hw/misc/edu.c
+++ b/hw/misc/edu.c
@@ -103,19 +103,18 @@ static void edu_lower_irq(EduState *edu, uint32_t val)
 }
 }
 
-static bool within(uint64_t addr, uint64_t start, uint64_t end)
-{
-return start <= addr && addr < end;
-}
-
-static void edu_check_range(uint64_t addr, uint64_t size1, uint64_t start,
-uint64_t size2)
+static void edu_check_range(uint64_t addr, uint64_t size1,
+uint64_t start, uint64_t size2)
 {
 uint64_t end1 = addr + size1;
 uint64_t end2 = start + size2;
 
-if (within(addr, start, end2) &&
-end1 > addr && end1 <= end2) {
+/*
+ * 1. ensure we aren't overflowing
+ * 2. ensure that [addr, end1) is within [start, size2)
+ */
+if (end2 >= start && end1 >= addr &&
+addr >= start && end1 <= end2) {
 return;
 }
 
-- 
2.44.0

[PULL 18/19] .gitlab-ci.d/cirrus.yml: Shorten the runtime of the macOS and FreeBSD jobs

[Thomas Huth]

Cirrus-CI introduced limitations to the free CI minutes. To avoid that
we are consuming them too fast, let's drop the usual targets that are
not that important since they are either a subset of another target
(like i386 or ppc being a subset of x86_64 or ppc64 respectively), or
since there is still a similar target with the opposite endianness
(like xtensa/xtensael, microblaze/microblazeel etc.).

Message-ID: <20240429100113.53357-1-th...@redhat.com>
Signed-off-by: Thomas Huth 
---
 .gitlab-ci.d/cirrus.yml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.gitlab-ci.d/cirrus.yml b/.gitlab-ci.d/cirrus.yml
index 4671f069c3..49f86fadaf 100644
--- a/.gitlab-ci.d/cirrus.yml
+++ b/.gitlab-ci.d/cirrus.yml
@@ -57,6 +57,7 @@ x64-freebsd-13-build:
 CIRRUS_VM_RAM: 8G
 UPDATE_COMMAND: pkg update; pkg upgrade -y
 INSTALL_COMMAND: pkg install -y
+CONFIGURE_ARGS: 
--target-list-exclude=arm-softmmu,i386-softmmu,microblaze-softmmu,mips64el-softmmu,mipsel-softmmu,mips-softmmu,ppc-softmmu,sh4eb-softmmu,xtensa-softmmu
 TEST_TARGETS: check
 
 aarch64-macos-13-base-build:
@@ -72,6 +73,7 @@ aarch64-macos-13-base-build:
 INSTALL_COMMAND: brew install
 PATH_EXTRA: /opt/homebrew/ccache/libexec:/opt/homebrew/gettext/bin
 PKG_CONFIG_PATH: 
/opt/homebrew/curl/lib/pkgconfig:/opt/homebrew/ncurses/lib/pkgconfig:/opt/homebrew/readline/lib/pkgconfig
+CONFIGURE_ARGS: 
--target-list-exclude=arm-softmmu,i386-softmmu,microblazeel-softmmu,mips64-softmmu,mipsel-softmmu,mips-softmmu,ppc-softmmu,sh4-softmmu,xtensaeb-softmmu
 TEST_TARGETS: check-unit check-block check-qapi-schema check-softfloat 
check-qtest-x86_64
 
 aarch64-macos-14-base-build:
-- 
2.44.0