Re: [PULL 3/6] migration/multifd: Fix loop conditions in multifd_zstd_send_prepare and multifd_zstd_recv

[Stefan Weil via]

Am 17.09.24 um 23:55 schrieb Peter Xu:


From: Stefan Weil via 



How can I avoid that my author name/email is changed so often?

Will this be fixed automatically before the commit is merged?

Stefan

[PATCH] migration/multifd: Fix loop conditions in multifd_zstd_send_prepare and multifd_zstd_recv

[Stefan Weil via]

GitHub's CodeQL reports four critical errors which are fixed by this commit:

Unsigned difference expression compared to zero

An expression (u - v > 0) with unsigned values u, v is only false if u == v,
so all changed expressions did not work as expected.

Signed-off-by: Stefan Weil 
---

I don't know what effect the errors will have.
Please check whether the fix should be backported to existing versions of QEMU.

And I think that it might be a good idea to add the security check to
https://github.com/qemu/qemu, too. The critical errors here and in 
net/colo-compare.c
were not reported by other static code analyzers as far as I know.
Paolo, if desired, I can send a patch for CodeQL.

Stefan W.

 migration/multifd-zstd.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/migration/multifd-zstd.c b/migration/multifd-zstd.c
index 53da33e048..abed140855 100644
--- a/migration/multifd-zstd.c
+++ b/migration/multifd-zstd.c
@@ -123,9 +123,9 @@ static int multifd_zstd_send_prepare(MultiFDSendParams *p, 
Error **errp)
  */
 do {
 ret = ZSTD_compressStream2(z->zcs, &z->out, &z->in, flush);
-} while (ret > 0 && (z->in.size - z->in.pos > 0)
- && (z->out.size - z->out.pos > 0));
-if (ret > 0 && (z->in.size - z->in.pos > 0)) {
+} while (ret > 0 && (z->in.size > z->in.pos)
+ && (z->out.size > z->out.pos));
+if (ret > 0 && (z->in.size > z->in.pos)) {
 error_setg(errp, "multifd %u: compressStream buffer too small",
p->id);
 return -1;
@@ -243,7 +243,7 @@ static int multifd_zstd_recv(MultiFDRecvParams *p, Error 
**errp)
  */
 do {
 ret = ZSTD_decompressStream(z->zds, &z->out, &z->in);
-} while (ret > 0 && (z->in.size - z->in.pos > 0)
+} while (ret > 0 && (z->in.size > z->in.pos)
  && (z->out.pos < page_size));
 if (ret > 0 && (z->out.pos < page_size)) {
 error_setg(errp, "multifd %u: decompressStream buffer too small",
-- 
2.39.2

[PATCH] Fix calculation of minimum in colo_compare_tcp

[Stefan Weil via]

GitHub's CodeQL reports a critical error which is fixed by using the MIN macro:

Unsigned difference expression compared to zero

Signed-off-by: Stefan Weil 
---
 net/colo-compare.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/colo-compare.c b/net/colo-compare.c
index c4ad0ab71f..39f90c4065 100644
--- a/net/colo-compare.c
+++ b/net/colo-compare.c
@@ -412,8 +412,7 @@ static void colo_compare_tcp(CompareState *s, Connection 
*conn)
  * can ensure that the packet's payload is acknowledged by
  * primary and secondary.
 */
-uint32_t min_ack = conn->pack - conn->sack > 0 ?
-   conn->sack : conn->pack;
+uint32_t min_ack = MIN(conn->pack, conn->sack);
 
 pri:
 if (g_queue_is_empty(&conn->primary_list)) {
-- 
2.39.3 (Apple Git-146)

Re: [PATCH 1/3] ui/sdl2: reenable the SDL2 Windows keyboard hook procedure

[Stefan Weil via]

Am 09.09.24 um 09:26 schrieb Marc-André Lureau:


Hi

On Mon, Sep 9, 2024 at 10:22 AM Volker Rümelin  wrote:

Windows only:

The libSDL2 Windows message loop needs the libSDL2 Windows low
level keyboard hook procedure to grab the left and right Windows
keys correctly. Reenable the SDL2 Windows keyboard hook procedure.

Because the QEMU Windows keyboard hook procedure is still needed
to filter out the special left Control key event for every Alt Gr
key event, it's important to install the two keyboard hook
procedures in the following order. First the SDL2 procedure, then
the QEMU procedure.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2139
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2323
Tested-by: Howard Spoelstra 
Signed-off-by: Volker Rümelin 
---
  ui/sdl2.c   | 53 ++---
  ui/win32-kbd-hook.c |  3 +++
  2 files changed, 38 insertions(+), 18 deletions(-)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 98ed974371..ac37c173a1 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c

[...]

@@ -877,6 +883,17 @@ static void sdl2_display_init(DisplayState *ds, 
DisplayOptions *o)
  SDL_EnableScreenSaver();
  memset(&info, 0, sizeof(info));
  SDL_VERSION(&info.version);
+/*
+ * Since version 2.16.0 under Windows, SDL2 has its own low level
+ * keyboard hook procedure to grab the keyboard. The remaining task of
+ * QEMU's low level keyboard hook procedure is to filter out the special
+ * left Control up/down key event for every Alt Gr key event on keyboards
+ * with an international layout.
+ */
+SDL_GetVersion(&ver);
+if (ver.major == 2 && ver.minor < 16) {
+win32_kbd_grab = true;
+}


Note: there is no 2.16 release. They jumped from 2.0.22 to 2.24 (see
https://github.com/libsdl-org/SDL/releases/tag/release-2.24.0)

The windows hook was indeed added in 2.0.16, released on Aug 10, 2021.

Given the distribution nature of the Windows binaries, I think we
could simply depend on a much recent version without worrying about
compatibility with < 2.0.16. This would help reduce the potential
combinations of versions and bugs reports.


[...]

I agree. My builds for Windows typically use the very latest versions, 
for example mingw-w64-i686-SDL2-2.30.7-1-any for the next build. So 
depending on a recent SDL version would be fine for me.


Stefan W.

Re: [PATCH] .travis.yml: Install python3-tomli in all build jobs

[Stefan Weil via]

Am 24.06.24 um 12:31 schrieb Thomas Huth:

On 24/06/2024 12.09, Alex Bennée wrote:

Thomas Huth  writes:


Since commit 1f97715c83 ('Revert "python: use vendored tomli"')
this package is a hard requirement for compiling QEMU, so install
it now in all Travis jobs, too.


AFAICT the only repo currently running these tests is your github
mirror:

   https://app.travis-ci.com/github/huth/qemu/builds?serverType=git

Because both the official github mirror and the gitlab project haven't
run anything for a while:

   https://app.travis-ci.com/gitlab/qemu-project/qemu/branches
   https://app.travis-ci.com/github/qemu/qemu/branches


AFAIK the gitlab integration broke completely at one point in time. But 
as you can see with my account, it still works fine for the github repo 
- I just need to write a short mail to the travis support once a year to 
ask them to extend the open source credits for my repo. So apart from 
that minor effort, it's a very convenient way to test QEMU on non-x86 
hosts.



https://app.travis-ci.com/github/qemu/qemu/requests shows recent 
requests. But all requests are refused: "Owner qemu is not on a new 
pricing". So whoever is "qemu", he/she might do the same as you did and 
write to the Travis support.


And https://travis-ci.org/qemu/qemu could be replaced by 
https://app.travis-ci.com/github/qemu/qemu in MAINTAINERS.


Stefan

Re: [PATCH] meson.build: Check for the availability of __attribute__((gcc_struct)) on MSYS2

[Stefan Weil via]

Am 15.08.24 um 14:27 schrieb Thomas Huth:


Since quite a while MSYS2 now supports Clang as a compiler, too.
Unfortunately, this compiler is lacking the __attribute__((gcc_struct))
that we need for compiling on Windows. But since the compiler is
available now, some people started to use it to compile QEMU on MSYS2,
apparently ignoring the compiler warnings (see for example the ticket at
https://gitlab.com/qemu-project/qemu/-/issues/2476 ). These builds are
likely broken in a couple of spots, so let's make sure that we rather
bail out early in the configuration phase instead of allowing the build
to succeed with warnings.

Signed-off-by: Thomas Huth 
---
  meson.build | 5 +
  1 file changed, 5 insertions(+)

diff --git a/meson.build b/meson.build
index 81ecd4bae7..fbda17c987 100644
--- a/meson.build
+++ b/meson.build
@@ -315,6 +315,11 @@ elif host_os == 'sunos'
qemu_common_flags += '-D__EXTENSIONS__'
  elif host_os == 'haiku'
qemu_common_flags += ['-DB_USE_POSITIVE_POSIX_ERRORS', '-D_BSD_SOURCE', 
'-fPIC']
+elif host_os == 'windows'
+  if not compiler.compiles('struct x { int y; } __attribute__((gcc_struct));',
+   args: '-Werror')
+error('Your compiler does not support __attribute__((gcc_struct)) - please 
use GCC instead of Clang')
+  endif
  endif
  
  # __sync_fetch_and_and requires at least -march=i486. Many toolchains



Tested-by: Stefan Weil 

[PATCH-for-9.1] docs: Fix some typos (found by typos) and grammar issues

[Stefan Weil via]

Fix the misspellings of "overriden" also in code comments.

Signed-off-by: Stefan Weil 
---
 docs/devel/migration/uadk-compression.rst | 4 ++--
 docs/interop/qemu-ga.rst  | 2 +-
 docs/tools/qemu-vmsr-helper.rst   | 4 ++--
 hw/arm/smmu-common.c  | 2 +-
 include/exec/memory.h | 2 +-
 qapi/rocker.json  | 4 ++--
 qga/main.c| 2 +-
 7 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/docs/devel/migration/uadk-compression.rst 
b/docs/devel/migration/uadk-compression.rst
index 3f73345dd5..64cadebd21 100644
--- a/docs/devel/migration/uadk-compression.rst
+++ b/docs/devel/migration/uadk-compression.rst
@@ -114,7 +114,7 @@ Make sure all these above kernel configurations are 
selected.
 
 Accelerator dev node permissions
 
-Harware accelerators(eg: HiSilicon Kunpeng Zip accelerator) gets registered to
+Hardware accelerators (eg: HiSilicon Kunpeng Zip accelerator) gets registered 
to
 UADK and char devices are created in dev directory. In order to access 
resources
 on hardware accelerator devices, write permission should be provided to user.
 
@@ -134,7 +134,7 @@ How To Use UADK Compression In QEMU Migration
   Set ``migrate_set_parameter multifd-compression uadk``
 
 Since UADK uses Shared Virtual Addressing(SVA) and device access virtual memory
-directly it is possible that SMMUv3 may enounter page faults while walking the
+directly it is possible that SMMUv3 may encounter page faults while walking the
 IO page tables. This may impact the performance. In order to mitigate this,
 please make sure to specify ``-mem-prealloc`` parameter to the destination VM
 boot parameters.
diff --git a/docs/interop/qemu-ga.rst b/docs/interop/qemu-ga.rst
index 9c7380896a..11f7bae460 100644
--- a/docs/interop/qemu-ga.rst
+++ b/docs/interop/qemu-ga.rst
@@ -50,7 +50,7 @@ Options
 .. option:: -c, --config=PATH
 
   Configuration file path (the default is |CONFDIR|\ ``/qemu-ga.conf``,
-  unless overriden by the QGA_CONF environment variable)
+  unless overridden by the QGA_CONF environment variable)
 
 .. option:: -m, --method=METHOD
 
diff --git a/docs/tools/qemu-vmsr-helper.rst b/docs/tools/qemu-vmsr-helper.rst
index 6ec87b49d9..9ce10b9af9 100644
--- a/docs/tools/qemu-vmsr-helper.rst
+++ b/docs/tools/qemu-vmsr-helper.rst
@@ -17,8 +17,8 @@ driver to advertise and monitor the power consumption or 
accumulated energy
 consumption of different power domains, such as CPU packages, DRAM, and other
 components when available.
 
-However those register are accesible under priviliged access (CAP_SYS_RAWIO).
-QEMU can use an external helper to access those priviliged register.
+However those registers are accessible under privileged access (CAP_SYS_RAWIO).
+QEMU can use an external helper to access those privileged registers.
 
 :program:`qemu-vmsr-helper` is that external helper; it creates a listener
 socket which will accept incoming connections for communication with QEMU.
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index d73ad62211..3f82728758 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -674,7 +674,7 @@ error:
 
 /*
  * combine S1 and S2 TLB entries into a single entry.
- * As a result the S1 entry is overriden with combined data.
+ * As a result the S1 entry is overridden with combined data.
  */
 static void combine_tlb(SMMUTLBEntry *tlbe, SMMUTLBEntry *tlbe_s2,
 dma_addr_t iova, SMMUTransCfg *cfg)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 02f7528ec0..296fd068c0 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1852,7 +1852,7 @@ void memory_region_iommu_replay(IOMMUMemoryRegion 
*iommu_mr, IOMMUNotifier *n);
  * memory_region_unregister_iommu_notifier: unregister a notifier for
  * changes to IOMMU translation entries.
  *
- * @mr: the memory region which was observed and for which notity_stopped()
+ * @mr: the memory region which was observed and for which notify_stopped()
  *  needs to be called
  * @n: the notifier to be removed.
  */
diff --git a/qapi/rocker.json b/qapi/rocker.json
index 6950ca9602..73c7363b16 100644
--- a/qapi/rocker.json
+++ b/qapi/rocker.json
@@ -42,7 +42,7 @@
 ##
 # @RockerPortDuplex:
 #
-# An eumeration of port duplex states.
+# An enumeration of port duplex states.
 #
 # @half: half duplex
 #
@@ -55,7 +55,7 @@
 ##
 # @RockerPortAutoneg:
 #
-# An eumeration of port autoneg states.
+# An enumeration of port autoneg states.
 #
 # @off: autoneg is off
 #
diff --git a/qga/main.c b/qga/main.c
index b8f7b1e4a3..50186760bf 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -257,7 +257,7 @@ QEMU_COPYRIGHT "\n"
 "\n"
 "  -c, --config=PATH configuration file path (default is\n"
 "%s/qemu-ga.conf\n"
-"unless overriden by the QGA_CONF environment variable)\n"
+"unless overridden by the QGA_CONF environment variab

Drop support for Python 3.7?

[Stefan Weil via]

Hi,

I just saw that the documentation still mentions that QEMU supports 
Python 3.7.


Python 3.7 is an unsupported Python version since about one year. 
Therefore I suggest to update the documentation for QEMU 9.1.0 and 
replace 3.7 by 3.8 as lowest supported version.


In addition the code which still mentions Python 3.7 or even 3.5 and 3.6 
could be reviewed and maybe simplified, but I think this is less urgent 
and can be done after QEMU release 9.1.0.


Regards
Stefan W.

$ git grep -i python.*3.7
docs/about/build-platforms.rst:  As of QEMU |version|, the minimum 
supported version of Python is 3.7.

python/qemu/qmp/util.py:Python 3.7+.
scripts/qapi/introspect.py:# TODO: Remove after Python 3.7 adds 
@dataclass:

scripts/qapi/source.py:# Replace with @dataclass in Python 3.7+
tests/qapi-schema/test-qapi.py:# dict (requires Python 3.7)

Re: [PATCH 1/7] block/vdi.c: Avoid potential overflow when calculating size of write

[Stefan Weil via]

Am 31.07.24 um 16:36 schrieb Peter Maydell:


In vdi_co_pwritev() we multiply a sector count by SECTOR_SIZE to
get the size to write in bytes. Coverity notes that this means that
we do the multiply as a 32x32->32 multiply before converting to
64 bits, which has the potential to overflow.

This is very unlikely to happen, since the block map has 4 bytes per
block and the maximum number of blocks in the image must fit into a
32-bit integer.  But we can keep Coverity happy by including a cast
so we do a 64-bit multiply here.

Resolves: Coverity CID 1508076
Signed-off-by: Peter Maydell 
---
  block/vdi.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/vdi.c b/block/vdi.c
index 6363da08cee..27c60ba18d0 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -728,7 +728,7 @@ nonallocating_write:
  logout("will write %u block map sectors starting from entry %u\n",
 n_sectors, bmap_first);
  ret = bdrv_co_pwrite(bs->file, bmap_offset * SECTOR_SIZE,
- n_sectors * SECTOR_SIZE, base, 0);
+ n_sectors * (uint64_t)SECTOR_SIZE, base, 0);
  }
  
  return ret;


Thanks.

Reviewed-by: Stefan Weil 

Re: [PATCH v7 1/2] hw/misc/riscv_iopmp: Add RISC-V IOPMP device

[Stefan Weil via]

Am 12.06.24 um 05:17 schrieb Ethan Chen via:

Support basic functions of IOPMP specification v0.9.1 rapid-k model.
The specification url:
https://github.com/riscv-non-isa/iopmp-spec/releases/tag/v0.9.1

IOPMP check memory access from device is valid or not. This implementation uses
IOMMU to change address space that device access. There are three possible
results of an access: valid, blocked, and stalled(stall is not supported in this
  patch).

If an access is valid, target address space is downstream_as.
If an access is blocked, it will go to blocked_io_as. The operation of
blocked_io_as could be a bus error, or it can respond a success with fabricated
data depending on IOPMP ERR_CFG register value.

Signed-off-by: Ethan Chen 
---
  hw/misc/Kconfig   |3 +
  hw/misc/meson.build   |1 +
  hw/misc/riscv_iopmp.c | 1002 +
  hw/misc/trace-events  |4 +
  include/hw/misc/riscv_iopmp.h |  152 +
  5 files changed, 1162 insertions(+)
  create mode 100644 hw/misc/riscv_iopmp.c
  create mode 100644 include/hw/misc/riscv_iopmp.h


Should both new files have SPDX license identifiers?

Regards,
Stefan W.

Re: Timeouts in CI jobs

[Stefan Weil via]

Am 24.04.24 um 19:09 schrieb Daniel P. Berrangé:


On Wed, Apr 24, 2024 at 06:27:58PM +0200, Stefan Weil via wrote:

I think the timeouts are caused by running too many parallel processes
during testing.

The CI uses parallel builds:

make -j$(expr $(nproc) + 1) all check-build $MAKE_CHECK_ARGS

Note that command is running both the compile and test phases of
the job. Overcommitting CPUs for the compile phase is a good
idea to keep CPUs busy while another process is waiting on
I/O, and is almost always safe todo.



Thank you for your answer.

Overcommitting for the build is safe, but in my experience the positive 
effect is typically very small on modern hosts with fast disk I/O and 
large buffer caches.


And there is also a negative impact because this requires scheduling 
with process switches.


Therefore I am not so sure that overcommitting is a good idea, 
especially not on cloud servers where the jobs are running in VMs.



Overcommitting CPUs for the test phase is less helpful and
can cause a variety of problems as you say.


It looks like `nproc` returns 8, and make runs with 9 threads.
`meson test` uses the same value to run 9 test processes in parallel:

/builds/qemu-project/qemu/build/pyvenv/bin/meson test  --no-rebuild -t 1
--num-processes 9 --print-errorlogs

Since the host can only handle 8 parallel threads, 9 threads might already
cause some tests to run non-deterministically.

In contributor forks, gitlab CI will be using the public shared
runners. These are Google Cloud VMs, which only have 2 vCPUs.

In the primary QEMU repo, we have a customer runner registered
that uses Azure based VMs. Not sure on the resources we have
configured for them offhand.


I was talking about the primary QEMU.


The important thing there is that what you see for CI speed in
your fork repo is not neccessarily a match for CI speed in QEMU
upstream repo.


I did not run tests in my GitLab fork because I still have to figure out 
how to do that.


In my initial answer to Peter's mail I had described my tests and the 
test environment in detail.


My test environment was an older (= slow) VM with 4 cores. I tested with 
different values for --num-processes. As expected higher values raised 
the number of timeouts. And the most interesting result was that 
`--num-processes 1` avoided timeouts, used less CPU time and did not 
increase the duration.



In my tests setting --num-processes to a lower value not only avoided
timeouts but also reduced the processing overhead without increasing the
runtime.

Could we run all tests with `--num-processes 1`?

The question is what impact that has on the overall job execution
time. A lot of our jobs are already quite long, which is bad for
the turnaround time of CI testing.  Reliable CI though is arguably
the #1 priority though, otherwise developers cease trusting it.
We need to find the balance between avoiding timeouts, while having
the shortest practical job time.  The TCI job you show about came
out at 22 minutes, which is not our worst job, so there is some
scope for allowing it to run longer with less parallelism.


The TCI job terminates after less than 7 minutes in my test runs with 
less parallelism.


Obviously there are tests which already do their own multithreading, and 
maybe other tests run single threaded. So maybe we need different values 
for `--num-processes` depending on the number of threads which the 
single tests use?


Regards,

Stefan

Timeouts in CI jobs (was: cross-i686-tci CI job is flaky again (timeouts): can somebody who cares about TCI investigate?)

[Stefan Weil via]

Am 20.04.24 um 22:25 schrieb Stefan Weil:

Am 16.04.24 um 14:17 schrieb Stefan Weil:

Am 16.04.24 um 14:10 schrieb Peter Maydell:


The cross-i686-tci job is flaky again, with persistent intermittent
failures due to jobs timing out.

[...]

Some of these timeouts are very high -- no test should be taking
10 minutes, even given TCI and a slowish CI runner -- which suggests
to me that there's some kind of intermittent deadlock going on.

Can somebody who cares about TCI investigate, please, and track
down whatever this is?


I'll have a look.


Short summary:

The "persistent intermittent failures due to jobs timing out" are not 
related to TCI: they also occur if the same tests are run with the 
normal TCG. I suggest that the CI tests should run single threaded.


Hi Paolo,

I need help from someone who knows the CI and the build and test 
framework better.


Peter reported intermittent timeouts for the cross-i686-tci job, causing 
it to fail. I can reproduce such timeouts locally, but noticed that they 
are not limited to TCI. The GitLab CI also shows other examples, such as 
this job:


https://gitlab.com/qemu-project/qemu/-/jobs/6700955287

I think the timeouts are caused by running too many parallel processes 
during testing.


The CI uses parallel builds:

make -j$(expr $(nproc) + 1) all check-build $MAKE_CHECK_ARGS

It looks like `nproc` returns 8, and make runs with 9 threads.
`meson test` uses the same value to run 9 test processes in parallel:

/builds/qemu-project/qemu/build/pyvenv/bin/meson test  --no-rebuild -t 1 
 --num-processes 9 --print-errorlogs


Since the host can only handle 8 parallel threads, 9 threads might 
already cause some tests to run non-deterministically.


But if some of the individual tests also use multithreading (according 
to my tests they do so with at least 3 or 4 threads), things get even 
worse. Then there are up to 4 * 9 = 36 threads competing to run on the 
available 8 cores.


In this scenario timeouts are expected and can occur randomly.

In my tests setting --num-processes to a lower value not only avoided 
timeouts but also reduced the processing overhead without increasing the 
runtime.


Could we run all tests with `--num-processes 1`?

Thanks,
Stefan

Re: cross-i686-tci CI job is flaky again (timeouts): can somebody who cares about TCI investigate?

[Stefan Weil via]

Am 16.04.24 um 14:17 schrieb Stefan Weil:

Am 16.04.24 um 14:10 schrieb Peter Maydell:


The cross-i686-tci job is flaky again, with persistent intermittent
failures due to jobs timing out.

[...]

Some of these timeouts are very high -- no test should be taking
10 minutes, even given TCI and a slowish CI runner -- which suggests
to me that there's some kind of intermittent deadlock going on.

Can somebody who cares about TCI investigate, please, and track
down whatever this is?


I'll have a look.


Short summary:

The "persistent intermittent failures due to jobs timing out" are not 
related to TCI: they also occur if the same tests are run with the 
normal TCG. I suggest that the CI tests should run single threaded.


But let's have a look on details of my results.

I have run `time make test` using different scenarios on the rather old 
and not so performant VM which I typically use for QEMU builds. I did 
not restrict the tests to selected architectures like it is done in the 
QEMU CI tests. Therefore I had more tests which all ended successfully:


Ok: 848
Expected Fail:  0
Fail:   0
Unexpected Pass:0
Skipped:68
Timeout:0

---

1st test with normal TCG

`nohup time ../configure --enable-modules --disable-werror && nohup time 
make -j4 && nohup time make test`

[...]
Full log written to 
/home/stefan/src/gitlab/qemu-project/qemu/bin/ndebug/x86_64-linux-gnu/meson-logs/testlog.txt
2296.08user 1525.02system 21:49.78elapsed 291%CPU (0avgtext+0avgdata 
633476maxresident)k

1730448inputs+14140528outputs (11668major+56827263minor)pagefaults 0swaps

---

2nd test with TCI

`nohup time ../configure --enable-modules --disable-werror 
--enable-tcg-interpreter && nohup time make -j4 && nohup time make test`

[...]
Full log written to 
/home/stefan/src/gitlab/qemu-project/qemu/bin/ndebug/x86_64-linux-gnu/meson-logs/testlog.txt
3766.74user 1521.38system 26:50.51elapsed 328%CPU (0avgtext+0avgdata 
633012maxresident)k

32768inputs+14145080outputs (3033major+56121586minor)pagefaults 0swaps

---

So the total test time with TCI was 26:50.51 minutes while for the 
normal test it was 21:49.78 minutes.


These 10 single tests had the longest duration:

1st test with normal TCG

  94/916 qtest-arm / qtest-arm/qom-test 373.41s
  99/916 qtest-aarch64 / qtest-aarch64/qom-test 398.43s
100/916 qtest-i386 / qtest-i386/bios-tables-test   188.06s
103/916 qtest-x86_64 / qtest-x86_64/bios-tables-test   228.33s
106/916 qtest-aarch64 / qtest-aarch64/migration-test   201.15s
119/916 qtest-i386 / qtest-i386/migration-test 253.58s
126/916 qtest-x86_64 / qtest-x86_64/migration-test 266.66s
143/916 qtest-arm / qtest-arm/test-hmp 101.72s
144/916 qtest-aarch64 / qtest-aarch64/test-hmp 113.10s
163/916 qtest-arm / qtest-arm/aspeed_smc-test  256.92s

2nd test with TCI

  68/916 qtest-arm / qtest-arm/qom-test 375.35s
  82/916 qtest-aarch64 / qtest-aarch64/qom-test 403.50s
  93/916 qtest-i386 / qtest-i386/bios-tables-test   192.22s
  99/916 qtest-aarch64 / qtest-aarch64/bios-tables-test 379.92s
100/916 qtest-x86_64 / qtest-x86_64/bios-tables-test   240.19s
103/916 qtest-aarch64 / qtest-aarch64/migration-test   223.49s
106/916 qtest-ppc64 / qtest-ppc64/pxe-test 418.42s
113/916 qtest-i386 / qtest-i386/migration-test 284.96s
118/916 qtest-arm / qtest-arm/aspeed_smc-test  271.10s
119/916 qtest-x86_64 / qtest-x86_64/migration-test 287.36s

---

Summary:

TCI is not much slower than the normal TCG. Surprisingly it was even 
faster for the tests 99 and 103. For other tests like test 106 TCI was 
about half as fast as normal TCG, but in summary it is not "factors" 
slower. A total test time of 26:50 minutes is also not so bad compared 
with the 21:49 minutes of the normal TCG.


No single test (including subtests) with TCI exceeded 10 minutes, the 
longest one was well below that margin with 418 seconds.


---

The tests above were running with x86_64, and I could not reproduce 
timeouts. The Gitlab CI tests were running with i686 and used different 
configure options. Therefore I made additional tests with 32 bit builds 
in a chroot environment (Debian GNU Linux bullseye i386) with the 
original configure options. As expected that reduced the number of tests 
to 250. All tests passed with `make test`:


3rd test with normal TCG

Ok: 250
Expected Fail:  0
Fail:   0
Unexpected Pass:0
Skipped:8
Timeout:0

Full log written to 
/root/qemu/bin/ndebug/i586-linux-gnu/meson-logs/testlog.txt
855.30user 450.53system 6:45.57elapsed 321%CPU (0avgtext+0avgdata 
609180maxresident)k

28232inputs+4772968outputs (64944major+8328814minor)pagefaults 0swaps


4th test with TCI

Ok: 250
Expected Fail:  0
Fail:   0
Unexpected Pass:0
Skipped:8
Timeout:0

Full log writte

Re: cross-i686-tci CI job is flaky again (timeouts): can somebody who cares about TCI investigate?

[Stefan Weil via]

Am 16.04.24 um 14:10 schrieb Peter Maydell:


The cross-i686-tci job is flaky again, with persistent intermittent
failures due to jobs timing out.

[...]

Some of these timeouts are very high -- no test should be taking
10 minutes, even given TCI and a slowish CI runner -- which suggests
to me that there's some kind of intermittent deadlock going on.

Can somebody who cares about TCI investigate, please, and track
down whatever this is?



I'll have a look.

Regards

Stefan

[PATCH for-9.0] Fix some typos in documentation (found by codespell)

[Stefan Weil via]

Signed-off-by: Stefan Weil 
---
 docs/devel/atomics.rst | 2 +-
 docs/devel/ci-jobs.rst.inc | 2 +-
 docs/devel/clocks.rst  | 2 +-
 docs/system/i386/sgx.rst   | 2 +-
 qapi/qom.json  | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/docs/devel/atomics.rst b/docs/devel/atomics.rst
index ff9b5ee30c..b77c6e13e1 100644
--- a/docs/devel/atomics.rst
+++ b/docs/devel/atomics.rst
@@ -119,7 +119,7 @@ The only guarantees that you can rely upon in this case are:
   ordinary accesses instead cause data races if they are concurrent with
   other accesses of which at least one is a write.  In order to ensure this,
   the compiler will not optimize accesses out of existence, create unsolicited
-  accesses, or perform other similar optimzations.
+  accesses, or perform other similar optimizations.
 
 - acquire operations will appear to happen, with respect to the other
   components of the system, before all the LOAD or STORE operations
diff --git a/docs/devel/ci-jobs.rst.inc b/docs/devel/ci-jobs.rst.inc
index ec33e6ee2b..be06322279 100644
--- a/docs/devel/ci-jobs.rst.inc
+++ b/docs/devel/ci-jobs.rst.inc
@@ -115,7 +115,7 @@ CI pipeline.
 QEMU_JOB_SKIPPED
 
 
-The job is not reliably successsful in general, so is not
+The job is not reliably successful in general, so is not
 currently suitable to be run by default. Ideally this should
 be a temporary marker until the problems can be addressed, or
 the job permanently removed.
diff --git a/docs/devel/clocks.rst b/docs/devel/clocks.rst
index b2d1148cdb..177ee1c90d 100644
--- a/docs/devel/clocks.rst
+++ b/docs/devel/clocks.rst
@@ -279,7 +279,7 @@ You can change the multiplier and divider of a clock at 
runtime,
 so you can use this to model clock controller devices which
 have guest-programmable frequency multipliers or dividers.
 
-Similary to ``clock_set()``, ``clock_set_mul_div()`` returns ``true`` if
+Similarly to ``clock_set()``, ``clock_set_mul_div()`` returns ``true`` if
 the clock state was modified; that is, if the multiplier or the diviser
 or both were changed by the call.
 
diff --git a/docs/system/i386/sgx.rst b/docs/system/i386/sgx.rst
index 0f0a73f758..c293f7f44e 100644
--- a/docs/system/i386/sgx.rst
+++ b/docs/system/i386/sgx.rst
@@ -6,7 +6,7 @@ Overview
 
 Intel Software Guard eXtensions (SGX) is a set of instructions and mechanisms
 for memory accesses in order to provide security accesses for sensitive
-applications and data. SGX allows an application to use it's pariticular
+applications and data. SGX allows an application to use its particular
 address space as an *enclave*, which is a protected area provides 
confidentiality
 and integrity even in the presence of privileged malware. Accesses to the
 enclave memory area from any software not resident in the enclave are 
prevented,
diff --git a/qapi/qom.json b/qapi/qom.json
index 8d4ca8ed92..85e6b4f84a 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -802,7 +802,7 @@
 #
 # @fd: file descriptor name previously passed via 'getfd' command,
 # which represents a pre-opened /dev/iommu.  This allows the
-# iommufd object to be shared accross several subsystems (VFIO,
+# iommufd object to be shared across several subsystems (VFIO,
 # VDPA, ...), and the file descriptor to be shared with other
 # process, e.g. DPDK.  (default: QEMU opens /dev/iommu by itself)
 #
-- 
2.39.2

Re: [PATCH 3/7] scripts/nsis.py: Automatically package required DLLs of QEMU executables

[Stefan Weil via]

Am 26.02.24 um 05:35 schrieb Bin Meng:


On Mon, Feb 26, 2024 at 1:37 AM Stefan Weil  wrote:

Am 10.09.22 um 02:37 schrieb Bin Meng:

On Sat, Sep 10, 2022 at 12:49 AM Mark Cave-Ayland
  wrote:

On 08/09/2022 14:28, Bin Meng wrote:


From: Bin Meng

At present packaging the required DLLs of QEMU executables is a
manual process, and error prone.

Actually build/config-host.mak contains a GLIB_BINDIR variable
which is the directory where glib and other DLLs reside. This
works for both Windows native build and cross-build on Linux.
We can use it as the search directory for DLLs and automate
the whole DLL packaging process.

Signed-off-by: Bin Meng
---

meson.build |  1 +
scripts/nsis.py | 46 ++
2 files changed, 43 insertions(+), 4 deletions(-)


[...]>>> diff --git a/scripts/nsis.py b/scripts/nsis.py

index baa6ef9594..03ed7608a2 100644
--- a/scripts/nsis.py
+++ b/scripts/nsis.py
@@ -18,12 +18,36 @@ def signcode(path):
return
subprocess.run([cmd, path])

+def find_deps(exe_or_dll, search_path, analyzed_deps):
+deps = [exe_or_dll]
+output = subprocess.check_output(["objdump", "-p", exe_or_dll], text=True)

This fails on non x86 hosts where objdump does not know how to handle a
Windows x86_64 exe file.

Does this command work in the MSYS2 environment on Windows Arm?



I don't know and cannot test that, because I don't run Windows on ARM.


+output = output.split("\n")
+for line in output:
+if not line.startswith("\tDLL Name: "):
+continue
+
+dep = line.split("DLL Name: ")[1].strip()
+if dep in analyzed_deps:
+continue
+
+dll = os.path.join(search_path, dep)
+if not os.path.exists(dll):
+# assume it's a Windows provided dll, skip it
+continue
+
+analyzed_deps.add(dep)
+# locate the dll dependencies recursively
+rdeps = find_deps(dll, search_path, analyzed_deps)
+deps.extend(rdeps)
+
+return deps

[...]

FWIW I wrote a similar script a while back to help package a custom Windows 
build for
a client, however I used ldd instead of objdump since it provided the full 
paths for
DLLs installed in the msys2/mingw-w64 environment via pacman which were outside 
the
QEMU build tree.


Yep, ldd also works, but only on Windows native build. objdump can
work on both Windows native and Linux cross builds.


objdump fails on Linux cross builds on any non x86 host, because objdump
typically only supports the native host architecture.

Therefore I get an error on an ARM64 host (podman on Mac M1):

  objdump: /tmp/tmpvae5u0qm/qemu/qemu-system-aarch64.exe: file
format not recognized

I could use x86_64-w64-mingw32-objdump to fix the cross builds, but then
native builds might fail because they don't have x86_64-w64-mingw32-objdump.

Is there a simple way how we can get the information here whether
objdump requires a cross build prefix?

For QEMU Windows, I believe the only supported architecture is x86_64,
correct? Do we want to support (cross) building QEMU for Windows Arm?



Yes, I think we only support QEMU on Windows x86_64. I also don't know 
anyone who has tried building for Windows ARM. And up to now I also was 
never asked for that, so obviously there is still no need for it.




Based on your observation, objdump on Linux cross builds on any x86
host works, but not on non x86 host.
Maybe it's a hint to ask for binutils guys to include the PE format
support for objdump Arm by default.



I am afraid that we'll have to find a solution on the QEMU side, not 
wait until all binutils support the PE format for x86_64 (which would 
make the binaries or the library much larger).


Stefan

Re: [PATCH 3/7] scripts/nsis.py: Automatically package required DLLs of QEMU executables

[Stefan Weil via]

Am 10.09.22 um 02:37 schrieb Bin Meng:

On Sat, Sep 10, 2022 at 12:49 AM Mark Cave-Ayland
 wrote:


On 08/09/2022 14:28, Bin Meng wrote:


From: Bin Meng 

At present packaging the required DLLs of QEMU executables is a
manual process, and error prone.

Actually build/config-host.mak contains a GLIB_BINDIR variable
which is the directory where glib and other DLLs reside. This
works for both Windows native build and cross-build on Linux.
We can use it as the search directory for DLLs and automate
the whole DLL packaging process.

Signed-off-by: Bin Meng 
---

   meson.build |  1 +
   scripts/nsis.py | 46 ++
   2 files changed, 43 insertions(+), 4 deletions(-)


[...]>>> diff --git a/scripts/nsis.py b/scripts/nsis.py

index baa6ef9594..03ed7608a2 100644
--- a/scripts/nsis.py
+++ b/scripts/nsis.py
@@ -18,12 +18,36 @@ def signcode(path):
   return
   subprocess.run([cmd, path])

+def find_deps(exe_or_dll, search_path, analyzed_deps):
+deps = [exe_or_dll]
+output = subprocess.check_output(["objdump", "-p", exe_or_dll], text=True)


This fails on non x86 hosts where objdump does not know how to handle a 
Windows x86_64 exe file.



+output = output.split("\n")
+for line in output:
+if not line.startswith("\tDLL Name: "):
+continue
+
+dep = line.split("DLL Name: ")[1].strip()
+if dep in analyzed_deps:
+continue
+
+dll = os.path.join(search_path, dep)
+if not os.path.exists(dll):
+# assume it's a Windows provided dll, skip it
+continue
+
+analyzed_deps.add(dep)
+# locate the dll dependencies recursively
+rdeps = find_deps(dll, search_path, analyzed_deps)
+deps.extend(rdeps)
+
+return deps

[...]


FWIW I wrote a similar script a while back to help package a custom Windows 
build for
a client, however I used ldd instead of objdump since it provided the full 
paths for
DLLs installed in the msys2/mingw-w64 environment via pacman which were outside 
the
QEMU build tree.



Yep, ldd also works, but only on Windows native build. objdump can
work on both Windows native and Linux cross builds.



objdump fails on Linux cross builds on any non x86 host, because objdump 
typically only supports the native host architecture.


Therefore I get an error on an ARM64 host (podman on Mac M1):

objdump: /tmp/tmpvae5u0qm/qemu/qemu-system-aarch64.exe: file 
format not recognized


I could use x86_64-w64-mingw32-objdump to fix the cross builds, but then 
native builds might fail because they don't have x86_64-w64-mingw32-objdump.


Is there a simple way how we can get the information here whether 
objdump requires a cross build prefix?


Stefan

Files without license statement

[Stefan Weil via]

Hi Paolo,

I just noticed that scripts/fix-multiline-comments.sh has a copyright 
statement, but no license statement. Should that be fixed?


It looks like there exist more files with the same problem (if it is a 
problem), for example docs/devel/loads-stores.rst (written by Peter).


LICENSE says that "Source files with no licensing information are 
released under the GNU General Public License [...]". Does that include 
shell and Python scripts, documentation and other files which might not 
be regarded as "source files"? Or should that be updated to "Any files 
with no licensing information [...]" which would also include files 
which have neither a copyright nor a license statement?


Kind regards
Stefan W.

Re: dropping 32-bit Windows host support

[Stefan Weil via]

Am 19.02.24 um 17:26 schrieb Thomas Huth:


On 19/02/2024 16.53, Daniel P. Berrangé wrote:

On Mon, Feb 19, 2024 at 03:37:31PM +, Peter Maydell wrote:

Our msys2 32-bit Windows host CI job has been failing recently
because upstream MSYS2 are starting to phase out 32-bit windows
host support and are steadily removing i686 versions of packages.
The latest is dtc:
https://gitlab.com/qemu-project/qemu/-/issues/2177
[...]



I agree with all your comments and also think that support for 32-bit 
Windows hosts can be dropped.


As Peter noted, I have been building 64-bit installers only since QEMU 
8.0.0. I have not received any complaints about this.


Best regards

Stefan

GPL 3.0 in TCG test code

[Stefan Weil via]

Dear all,

some QEMU code under tests/tcg uses GPL 3.0 or later:

tests/tcg/aarch64/semicall.h: * SPDX-License-Identifier: GPL-3.0-or-later
tests/tcg/arm/semicall.h: * SPDX-License-Identifier: GPL-3.0-or-later
tests/tcg/i386/system/boot.S: * SPDX-License-Identifier: GPL-3.0-or-later
tests/tcg/multiarch/arm-compat-semi/semiconsole.c: * 
SPDX-License-Identifier: GPL-3.0-or-later
tests/tcg/multiarch/arm-compat-semi/semihosting.c: * 
SPDX-License-Identifier: GPL-3.0-or-later
tests/tcg/multiarch/float_convd.c: * SPDX-License-Identifier: 
GPL-3.0-or-later
tests/tcg/multiarch/float_convs.c: * SPDX-License-Identifier: 
GPL-3.0-or-later
tests/tcg/multiarch/float_helpers.h: * SPDX-License-Identifier: 
GPL-3.0-or-later
tests/tcg/multiarch/float_madds.c: * SPDX-License-Identifier: 
GPL-3.0-or-later
tests/tcg/multiarch/libs/float_helpers.c: * SPDX-License-Identifier: 
GPL-3.0-or-later

tests/tcg/riscv64/semicall.h: * SPDX-License-Identifier: GPL-3.0-or-later
tests/tcg/x86_64/system/boot.S: * SPDX-License-Identifier: GPL-3.0-or-later

I don't think that there is a conflict with the QEMU license (GPL 2.0 or 
later) because that code is only used in tests.


But maybe it should be mentioned in LICENSE?

Regards,
Stefan

Re: [PATCH v2 1/4] util/uri: Remove uri_string_unescape()

[Stefan Weil via]

Am 23.01.24 um 19:22 schrieb Thomas Huth:


uri_string_unescape() basically does the same as the glib function
g_uri_unescape_segment(). So we can get rid of our implementation
completely by simply using the glib function instead.

Suggested-by: Stefan Weil  [g_uri_unescape_string()]
Suggested-by: Paolo Bonzini  [g_uri_unescape_segment()]
Signed-off-by: Thomas Huth 
---
  include/qemu/uri.h |  1 -
  util/uri.c | 97 ++
  2 files changed, 11 insertions(+), 87 deletions(-)



Reviewed-by: Stefan Weil

Thank you, Thomas and Paolo.

Re: [PATCH 2/5] util/uri: Simplify uri_string_unescape()

[Stefan Weil via]

Am 22.01.24 um 20:17 schrieb Thomas Huth:


uri_string_unescape() basically does the same as the glib function
g_uri_unescape_string(), with just an additional length parameter.
So we can simplify this function a lot by limiting the length with
g_strndup() first and then by calling g_uri_unescape_string() instead
of walking through the string manually.

Suggested-by: Stefan Weil


Can my e-mail address be replaced by another one (s...@weilnetz.de)?


Signed-off-by: Thomas Huth
---
  util/uri.c | 49 +++--
  1 file changed, 3 insertions(+), 46 deletions(-)

diff --git a/util/uri.c b/util/uri.c
index 33b6c7214e..2a75f535ba 100644
--- a/util/uri.c
+++ b/util/uri.c
@@ -1561,15 +1561,6 @@ done_cd:
  return 0;
  }
  
-static int is_hex(char c)

-{
-if (((c >= '0') && (c <= '9')) || ((c >= 'a') && (c <= 'f')) ||
-((c >= 'A') && (c <= 'F'))) {
-return 1;
-}
-return 0;
-}
-
  /**
   * uri_string_unescape:
   * @str:  the string to unescape
@@ -1585,8 +1576,7 @@ static int is_hex(char c)
   */
  char *uri_string_unescape(const char *str, int len)
  {
-char *ret, *out;
-const char *in;
+g_autofree char *lstr = NULL;



Is it necessary to assign NULL? It does not look so.


  
  if (str == NULL) {

  return NULL;
@@ -1594,42 +1584,9 @@ char *uri_string_unescape(const char *str, int len)
  if (len <= 0) {
  len = strlen(str);
  }
-if (len < 0) {
-return NULL;
-}
-
-ret = g_malloc(len + 1);
+lstr = g_strndup(str, len);
  
-in = str;

-out = ret;
-while (len > 0) {
-if ((len > 2) && (*in == '%') && (is_hex(in[1])) && (is_hex(in[2]))) {
-in++;
-if ((*in >= '0') && (*in <= '9')) {
-*out = (*in - '0');
-} else if ((*in >= 'a') && (*in <= 'f')) {
-*out = (*in - 'a') + 10;
-} else if ((*in >= 'A') && (*in <= 'F')) {
-*out = (*in - 'A') + 10;
-}
-in++;
-if ((*in >= '0') && (*in <= '9')) {
-*out = *out * 16 + (*in - '0');
-} else if ((*in >= 'a') && (*in <= 'f')) {
-*out = *out * 16 + (*in - 'a') + 10;
-} else if ((*in >= 'A') && (*in <= 'F')) {
-*out = *out * 16 + (*in - 'A') + 10;
-}
-in++;
-len -= 3;
-out++;
-} else {
-*out++ = *in++;
-len--;
-}
-}
-*out = 0;
-return ret;
+return g_uri_unescape_string(lstr, NULL);
  }
  
  /**



Thank you.

Reviewed-by: Stefan Weil 

Re: [PATCH 5/5] util/uri: Remove unused macros ISA_RESERVED() and ISA_GEN_DELIM()

[Stefan Weil via]

Am 22.01.24 um 20:17 schrieb Thomas Huth:


They are not used anywhere, so there's no need to keep them around.

Signed-off-by: Thomas Huth 
---
  util/uri.c | 13 -
  1 file changed, 13 deletions(-)



Reviewed-by: Stefan Weil 

Re: [PATCH 4/5] util/uri: Remove unused functions uri_resolve() and uri_resolve_relative()

[Stefan Weil via]

Am 22.01.24 um 20:17 schrieb Thomas Huth:


These rather complex functions have never been used since they've been
introduced in 2012, so looks like they are not really useful for QEMU.
And since the static normalize_uri_path() function is also only used by
uri_resolve(), we can remove that function now, too.

Signed-off-by: Thomas Huth 
---
  include/qemu/uri.h |   2 -
  util/uri.c | 689 -
  2 files changed, 691 deletions(-)



This patch should be applied before patch 3 (so switch the order of the 
patches 3 and 4). With this change:


Reviewed-by: Stefan Weil 

Re: [PATCH 3/5] util/uri: Remove the uri_string_escape() function

[Stefan Weil via]

Am 22.01.24 um 20:17 schrieb Thomas Huth:


It is not used in QEMU - and if somebody needs this functionality,
they can simply use g_uri_escape_string() from the glib instead.

Signed-off-by: Thomas Huth 
---
  include/qemu/uri.h |  1 -
  util/uri.c | 64 --
  2 files changed, 65 deletions(-)



The removed function is used in util/uri.c, so this patch breaks the 
compilation.


That can be fixed by applying patch 4 before this one.

With that re-ordering you may add my signature:

Reviewed-by: Stefan Weil 

Re: [PATCH 1/5] util/uri: Remove the unused "target" argument from uri_string_unescape()

[Stefan Weil via]

Am 22.01.24 um 20:17 schrieb Thomas Huth:


All callers pass NULL as target, so we can simplify the code by
dropping this parameter.

Signed-off-by: Thomas Huth 
---
  include/qemu/uri.h |  2 +-
  util/uri.c | 32 ++--
  2 files changed, 15 insertions(+), 19 deletions(-)



Reviewed-by: Stefan Weil 

Re: [PATCH] util/uri: Remove is_hex() function

[Stefan Weil via]

Am 12.01.24 um 07:35 schrieb Markus Armbruster:

Thomas Huth  writes:


We can simply use the g_ascii_isxdigit() from the glib instead.


... or even use unescape_string() from the glib?

https://docs.gtk.org/glib/type_func.Uri.unescape_string.html

Regards,
Stefan

Re: [PATCH] mailmap: Fix Stefan Weil author email again

[Stefan Weil via]

Am 27.12.23 um 10:12 schrieb Philippe Mathieu-Daudé:


On 27/12/23 10:09, Michael Tokarev wrote:

27.12.2023 11:59, Philippe Mathieu-Daudé:

Commit 5204b499a6 ("mailmap: Fix Stefan Weil author email")
corrected authorship for patch received at qemu-devel@nongnu.org,
correct now for patch received at qemu-triv...@nongnu.org.

Fixes: d819fc9516 ("virtio-blk: Fix potential nullptr read access")


Do you think a single commit warrants an entry in mailmap?


If I cared enough to write and post a patch, I suppose so...

In the past the only limitation was whether someone was willing
to do the work and send a patch, not the size of the .mailmap
file.


Thank you Philippe and Michael.

It looks like I have some more identities. :-)

   1 Author: Stefan Weil 
   1 Author: Stefan Weil 
 697 Author: Stefan Weil 
 361 Author: Stefan Weil 
   1 Author: Stefan Weil via 

My very old address w...@mail.berlios.de might be more interesting for 
mailmap.


Regards,

Stefan

Re: [PATCH] target/hexagon/idef-parser/prepare: use env to invoke bash

[Stefan Weil via]

Am 25.12.23 um 09:05 schrieb Michael Tokarev:

23.11.2023 23:57, Samuel Tardieu :

This file is the only one involved in the compilation process which
still uses the /bin/bash path.

Signed-off-by: Samuel Tardieu 
---
  target/hexagon/idef-parser/prepare | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/hexagon/idef-parser/prepare 
b/target/hexagon/idef-parser/prepare

index 72d6fcbd21..cb3622d4f8 100755
--- a/target/hexagon/idef-parser/prepare
+++ b/target/hexagon/idef-parser/prepare
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/usr/bin/env bash


What's the reason for this indirection?  bash has been /bin/bash for 
decades,
it is used this way in many other places in qemu code and in other 
projects.

Yes I know about current move /bin => /usr/bin etc, but the thing is that
traditional paths like this one (or like /bin/sh) is not going away any 
time

soon.  What's the matter here?

/mjt



On MacOS /bin/bash is the pre-installed bash:

% /bin/bash --version
GNU bash, version 3.2.57(1)-release (arm64-apple-darwin23)
Copyright (C) 2007 Free Software Foundation, Inc.

With /usr/bin/env I get a recent one from Homebrew:

% /usr/bin/env bash --version
GNU bash, version 5.2.21(1)-release (aarch64-apple-darwin23.0.0)
Copyright (C) 2022 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later 



This is free software; you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

So if a bash script uses bash syntax which was added after 2007, that 
would not work with /bin/bash. That's why I had to use the indirection 
for another open source project. I don't know whether the QEMU bash 
scripts require a newer version of bash.


Regards
Stefan

[PATCH] virtio-blk: Fix potential nullpointer read access in virtio_blk_data_plane_destroy

[Stefan Weil via]

Fixes: CID 1532828
Fixes: b6948ab01d ("virtio-blk: add iothread-vq-mapping parameter")
Signed-off-by: Stefan Weil 
---
 hw/block/dataplane/virtio-blk.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 6debd4401e..97a302cf49 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -152,7 +152,7 @@ bool virtio_blk_data_plane_create(VirtIODevice *vdev, 
VirtIOBlkConf *conf,
 void virtio_blk_data_plane_destroy(VirtIOBlockDataPlane *s)
 {
 VirtIOBlock *vblk;
-VirtIOBlkConf *conf = s->conf;
+VirtIOBlkConf *conf;
 
 if (!s) {
 return;
@@ -160,6 +160,7 @@ void virtio_blk_data_plane_destroy(VirtIOBlockDataPlane *s)
 
 vblk = VIRTIO_BLK(s->vdev);
 assert(!vblk->dataplane_started);
+conf = s->conf;
 
 if (conf->iothread_vq_mapping_list) {
 IOThreadVirtQueueMappingList *node;
-- 
2.39.2

[PATCH for-8.2] Fix broken build for QEMU guest agent

[Stefan Weil via]

Meson setup failed:

qga/meson.build:148:4: ERROR: Unknown variable "project".

Fixes commit e20d68aa ("configure, meson: use command line options [...]").

Signed-off-by: Stefan Weil 
---
 qga/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qga/meson.build b/qga/meson.build
index 940a51d55d..ff7a8496e4 100644
--- a/qga/meson.build
+++ b/qga/meson.build
@@ -146,7 +146,7 @@ if targetos == 'windows'
   libpcre = 'libpcre2'
 endif
 qga_msi_version = get_option('qemu_ga_version') == '' \
-  ? project.version() \
+  ? meson.project_version() \
   : get_option('qemu_ga_version')
 qga_msi = custom_target('QGA MSI',
 input: files('installer/qemu-ga.wxs'),
-- 
2.39.2

Re: [PATCH] spelling: hw/audio/virtio-snd.c: initalize

[Stefan Weil via]

Am 13.11.23 um 22:20 schrieb Michael Tokarev:

Fixes: eb9ad377bb94 "virtio-sound: handle control messages and streams"
Signed-off-by: Michael Tokarev 
---
  hw/audio/virtio-snd.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index a18a9949a7..2fe966e311 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -1126,7 +1126,7 @@ static void virtio_snd_realize(DeviceState *dev, Error 
**errp)
  status = virtio_snd_set_pcm_params(vsnd, i, &default_params);
  if (status != cpu_to_le32(VIRTIO_SND_S_OK)) {
  error_setg(errp,
-   "Can't initalize stream params, device responded with 
%s.",
+   "Can't initialize stream params, device responded with 
%s.",
 print_code(status));
  return;
  }


Reviewed-by: Stefan Weil 

Thanks,
Stefan

[PATCH] Fix some typos in documentation and comments

[Stefan Weil via]

Signed-off-by: Stefan Weil 
---

This patch was triggered by a spelling check for the generated
QEMU documentation using codespell. It does not try to fix all
typos which still exist in the QEMU code, but has a focus on
those required to fix the documentation. Nevertheless some code
comments with the same typos were fixed, too.

I think the patch is trivial, so maybe it can still be included
in the upcoming release, but that's not strictly necessary.

Stefan

 docs/about/deprecated.rst| 2 +-
 docs/devel/qom.rst   | 2 +-
 docs/system/devices/nvme.rst | 2 +-
 hw/core/loader.c | 4 ++--
 include/exec/memory.h| 2 +-
 ui/vnc-enc-tight.c   | 2 +-
 6 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 1c35f55666..92a2bafd2b 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -369,7 +369,7 @@ mapping permissions et al by using its 'mapped' security 
model option.
 Nowadays it would make sense to reimplement the ``proxy`` backend by using
 QEMU's ``vhost`` feature, which would eliminate the high latency costs under
 which the 9p ``proxy`` backend currently suffers. However as of to date nobody
-has indicated plans for such kind of reimplemention unfortunately.
+has indicated plans for such kind of reimplementation unfortunately.
 
 
 Block device options
diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst
index 0b506426d7..9918fac7f2 100644
--- a/docs/devel/qom.rst
+++ b/docs/devel/qom.rst
@@ -30,7 +30,7 @@ user configuration.
 Creating a QOM class
 
 
-A simple minimal device implementation may look something like bellow:
+A simple minimal device implementation may look something like below:
 
 .. code-block:: c
:caption: Creating a minimal type
diff --git a/docs/system/devices/nvme.rst b/docs/system/devices/nvme.rst
index a8bb8d729c..2a3af268f7 100644
--- a/docs/system/devices/nvme.rst
+++ b/docs/system/devices/nvme.rst
@@ -232,7 +232,7 @@ parameters:
   Set the number of Reclaim Groups.
 
 ``fdp.nruh`` (default: ``0``)
-  Set the number of Reclaim Unit Handles. This is a mandatory paramater and
+  Set the number of Reclaim Unit Handles. This is a mandatory parameter and
   must be non-zero.
 
 ``fdp.runs`` (default: ``96M``)
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 8b7fd9e9e5..4dd5a71fb7 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -863,7 +863,7 @@ ssize_t load_image_gzipped(const char *filename, hwaddr 
addr, uint64_t max_sz)
 
 /*
  * The Linux header magic number for a EFI PE/COFF
- * image targetting an unspecified architecture.
+ * image targeting an unspecified architecture.
  */
 #define EFI_PE_LINUX_MAGIC"\xcd\x23\x82\x81"
 
@@ -1492,7 +1492,7 @@ RomGap rom_find_largest_gap_between(hwaddr base, size_t 
size)
 if (rom->mr || rom->fw_file) {
 continue;
 }
-/* ignore anything finishing bellow base */
+/* ignore anything finishing below base */
 if (rom->addr + rom->romsize <= base) {
 continue;
 }
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 7f5c11a0cc..68284428f8 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -942,7 +942,7 @@ struct MemoryListener {
  *
  * @listener: The #MemoryListener.
  * @last_stage: The last stage to synchronize the log during migration.
- * The caller should gurantee that the synchronization with true for
+ * The caller should guarantee that the synchronization with true for
  * @last_stage is triggered for once after all VCPUs have been stopped.
  */
 void (*log_sync_global)(MemoryListener *listener, bool last_stage);
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
index 09200d71b8..ee853dcfcb 100644
--- a/ui/vnc-enc-tight.c
+++ b/ui/vnc-enc-tight.c
@@ -77,7 +77,7 @@ static int tight_send_framebuffer_update(VncState *vs, int x, 
int y,
 
 #ifdef CONFIG_VNC_JPEG
 static const struct {
-double jpeg_freq_min;   /* Don't send JPEG if the freq is bellow */
+double jpeg_freq_min;   /* Don't send JPEG if the freq is below */
 double jpeg_freq_threshold; /* Always send JPEG if the freq is above */
 int jpeg_idx;   /* Allow indexed JPEG */
 int jpeg_full;  /* Allow full color JPEG */
-- 
2.39.2

Re: [PATCH v2 1/1] ui/sdl2: disable SDL_HINT_GRAB_KEYBOARD on Windows

[Stefan Weil via]

Am 18.04.23 um 08:56 schrieb Volker Rümelin:


Windows sends an extra left control key up/down input event for
every right alt key up/down input event for keyboards with
international layout. Since commit 830473455f ("ui/sdl2: fix
handling of AltGr key on Windows") QEMU uses a Windows low level
keyboard hook procedure to reliably filter out the special left
control key and to grab the keyboard on Windows.

The SDL2 version 2.0.16 introduced its own Windows low level
keyboard hook procedure to grab the keyboard. Windows calls this
callback before the QEMU keyboard hook procedure. This disables
the special left control key filter when the keyboard is grabbed.

To fix the problem, disable the SDL2 Windows low level keyboard
hook procedure.

Reported-by: Bernhard Beschow 
Signed-off-by: Volker Rümelin 
---
  ui/sdl2.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index 00aadfae37..9d703200bf 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -855,7 +855,10 @@ static void sdl2_display_init(DisplayState *ds, 
DisplayOptions *o)
  #ifdef SDL_HINT_VIDEO_X11_NET_WM_BYPASS_COMPOSITOR /* only available since 
SDL 2.0.8 */
  SDL_SetHint(SDL_HINT_VIDEO_X11_NET_WM_BYPASS_COMPOSITOR, "0");
  #endif
+#ifndef CONFIG_WIN32
+/* QEMU uses its own low level keyboard hook procecure on Windows */



s/procecure/procedure/



  SDL_SetHint(SDL_HINT_GRAB_KEYBOARD, "1");
+#endif
  #ifdef SDL_HINT_ALLOW_ALT_TAB_WHILE_GRABBED
  SDL_SetHint(SDL_HINT_ALLOW_ALT_TAB_WHILE_GRABBED, "0");
  #endif



The typo fix for the comment does not require a v3 and can be applied in 
the pull request.


Reviewed-by: Stefan Weil 

Re: source fails to compile on msys2

[Stefan Weil via]

Am 12.04.23 um 15:13 schrieb Howard Spoelstra:

Hello Peter,

My source was cloned today. I just cloned again and I still see the 
tokens reversed:
git clone https://www.gitlab.com/qemu/qemu 
 qemu-master-clean


The official URL is https://gitlab.com/qemu-project/qemu/.

Regards,
Stefan

Re: source fails to compile on msys2

[Stefan Weil via]

Am 12.04.23 um 14:12 schrieb BALATON Zoltan:


On Wed, 12 Apr 2023, Howard Spoelstra wrote:

It seems the current source fails to compile with up to date msys2.


See here: https://qemu.weilnetz.de/
I think there are some patches there that aren't upstream. I don't 
know why and also don't know why those binaries are not built from 
QEMU master.



My related Git repository is https://repo.or.cz/w/qemu/ar7.git/. I 
merged v8.0.0-rc3 two days ago, and that code builds with no problems 
for Windows.


And yes, my code includes commits which are (still) missing upstream. 
Some of them are for special hardware which was either removed from QEMU 
master or which I think is not interesting for upstream. Others were 
already sent to qemu-devel, so might become part of QEMU master later. 
In addition there are commits for my Windows build environment which 
also uses a higher warning level than QEMU master. But usually the 
differences to the latest tagged QEMU release are small.


Stefan

[PATCH for-8.0] docs/cxl: Fix sentence

[Stefan Weil via]

Signed-off-by: Stefan Weil 
---

If my change is okay I suggest to apply the patch for 8.0
because it fixes documentation.

Regards,
Stefan W.

 docs/system/devices/cxl.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/system/devices/cxl.rst b/docs/system/devices/cxl.rst
index f25783a4ec..4c38223069 100644
--- a/docs/system/devices/cxl.rst
+++ b/docs/system/devices/cxl.rst
@@ -111,7 +111,7 @@ Interfaces provided include:
 
 CXL Root Ports (CXL RP)
 ~~~
-A CXL Root Port servers te same purpose as a PCIe Root Port.
+A CXL Root Port serves the same purpose as a PCIe Root Port.
 There are a number of CXL specific Designated Vendor Specific
 Extended Capabilities (DVSEC) in PCIe Configuration Space
 and associated component register access via PCI bars.
-- 
2.39.2

[PATCH for-8.0] docs: Fix typo (wphx => whpx)

[Stefan Weil via]

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1529
Signed-off-by: Stefan Weil 
---

I suggest to apply the patch for 8.0 because it fixes documentation.

Regards
Stefan W.

 docs/system/introduction.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/system/introduction.rst b/docs/system/introduction.rst
index c8a9fe6c1d..3e256f8326 100644
--- a/docs/system/introduction.rst
+++ b/docs/system/introduction.rst
@@ -27,7 +27,7 @@ Tiny Code Generator (TCG) capable of emulating many CPUs.
   * - Hypervisor Framework (hvf)
 - MacOS
 - x86 (64 bit only), Arm (64 bit only)
-  * - Windows Hypervisor Platform (wphx)
+  * - Windows Hypervisor Platform (whpx)
 - Windows
 - x86
   * - NetBSD Virtual Machine Monitor (nvmm)
-- 
2.39.2

[PATCH] hw/arm: Fix some typos in comments (most found by codespell)

[Stefan Weil via]

Signed-off-by: Stefan Weil 
---

The patch does not change code and could also be applied for 8.0.

 hw/arm/Kconfig| 2 +-
 hw/arm/exynos4210.c   | 4 ++--
 hw/arm/musicpal.c | 2 +-
 hw/arm/omap1.c| 2 +-
 hw/arm/omap2.c| 2 +-
 hw/arm/virt-acpi-build.c  | 2 +-
 hw/arm/virt.c | 2 +-
 hw/arm/xlnx-versal-virt.c | 2 +-
 8 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index b5aed4aff5..db1105c717 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -126,7 +126,7 @@ config OLIMEX_STM32_H405
 config NSERIES
 bool
 select OMAP
-select TMP105   # tempature sensor
+select TMP105   # temperature sensor
 select BLIZZARD # LCD/TV controller
 select ONENAND
 select TSC210X  # touchscreen/sensors/audio
diff --git a/hw/arm/exynos4210.c b/hw/arm/exynos4210.c
index 6f2dda13f6..de39fb0ece 100644
--- a/hw/arm/exynos4210.c
+++ b/hw/arm/exynos4210.c
@@ -326,7 +326,7 @@ static int mapline_size(const int *mapline)
 
 /*
  * Initialize board IRQs.
- * These IRQs contain splitted Int/External Combiner and External Gic IRQs.
+ * These IRQs contain split Int/External Combiner and External Gic IRQs.
  */
 static void exynos4210_init_board_irqs(Exynos4210State *s)
 {
@@ -744,7 +744,7 @@ static void exynos4210_realize(DeviceState *socdev, Error 
**errp)
  * - SDMA
  * - ADMA2
  *
- * As this part of the Exynos4210 is not publically available,
+ * As this part of the Exynos4210 is not publicly available,
  * we used the "HS-MMC Controller S3C2416X RISC Microprocessor"
  * public datasheet which is very similar (implementing
  * MMC Specification Version 4.0 being the only difference noted)
diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index c9010b2ffb..58f3d30c9b 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -100,7 +100,7 @@
 #define MP_LCD_SPI_CMD  0x00104011
 #define MP_LCD_SPI_INVALID  0x
 
-/* Commmands */
+/* Commands */
 #define MP_LCD_INST_SETPAGE00xB0
 /* ... */
 #define MP_LCD_INST_SETPAGE70xB7
diff --git a/hw/arm/omap1.c b/hw/arm/omap1.c
index 559c066ce9..d5438156ee 100644
--- a/hw/arm/omap1.c
+++ b/hw/arm/omap1.c
@@ -4057,7 +4057,7 @@ struct omap_mpu_state_s *omap310_mpu_init(MemoryRegion 
*dram,
 s->led[1] = omap_lpg_init(system_memory,
   0xfffbd800, omap_findclk(s, "clk32-kHz"));
 
-/* Register mappings not currenlty implemented:
+/* Register mappings not currently implemented:
  * MCSI2 Comm  fffb2000 - fffb27ff (not mapped on OMAP310)
  * MCSI1 Bluetooth fffb2800 - fffb2fff (not mapped on OMAP310)
  * USB W2FCfffb4000 - fffb47ff
diff --git a/hw/arm/omap2.c b/hw/arm/omap2.c
index 366d6af1b6..d5a2ae7af6 100644
--- a/hw/arm/omap2.c
+++ b/hw/arm/omap2.c
@@ -2523,7 +2523,7 @@ struct omap_mpu_state_s *omap2420_mpu_init(MemoryRegion 
*sdram,
 omap_findclk(s, "func_96m_clk"),
 omap_findclk(s, "core_l4_iclk"));
 
-/* All register mappings (includin those not currenlty implemented):
+/* All register mappings (including those not currently implemented):
  * SystemControlMod4800 - 48000fff
  * SystemControlL4 48001000 - 48001fff
  * 32kHz Timer Mod 48004000 - 48004fff
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 4156111d49..4af0de8b24 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -694,7 +694,7 @@ static void build_append_gicr(GArray *table_data, uint64_t 
base, uint32_t size)
 build_append_int_noprefix(table_data, 0xE, 1);  /* Type */
 build_append_int_noprefix(table_data, 16, 1);   /* Length */
 build_append_int_noprefix(table_data, 0, 2);/* Reserved */
-/* Discovery Range Base Addres */
+/* Discovery Range Base Address */
 build_append_int_noprefix(table_data, base, 8);
 build_append_int_noprefix(table_data, size, 4); /* Discovery Range Length 
*/
 }
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ac626b3bef..4983f5fc93 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2052,7 +2052,7 @@ static void machvirt_init(MachineState *machine)
 int pa_bits;
 
 /*
- * Instanciate a temporary CPU object to find out about what
+ * Instantiate a temporary CPU object to find out about what
  * we are about to deal with. Once this is done, get rid of
  * the object.
  */
diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
index 37fc9b919c..668a9d65a4 100644
--- a/hw/arm/xlnx-versal-virt.c
+++ b/hw/arm/xlnx-versal-virt.c
@@ -659,7 +659,7 @@ static void versal_virt_init(MachineState *machine)
 fdt_add_clk_node(s, "/clk25", 2500, s->phandle.clk_25Mhz);
 
 /* Make the APU cpu address space visible to virtio and other
- * modules unaware of muliple address-spaces.  */
+ * modules unaware of multiple a

Re: [PATCH 19/41] compiler.h: replace QEMU_NORETURN with G_NORETURN

[Stefan Weil via]

Am 07.04.23 um 19:01 schrieb Stefan Weil:
Please excuse the late report, but this old patch causes a build failure 
for me:


I just noticed that this is already fixed in latest code (I tested the 
build with v8.0.0-rc0). So nothing to do. Sorry for the noise.


Stefan

Re: [PATCH 19/41] compiler.h: replace QEMU_NORETURN with G_NORETURN

[Stefan Weil via]

Please excuse the late report, but this old patch causes a build failure 
for me:


Am 20.04.22 um 15:26 schrieb marcandre.lur...@redhat.com:

From: Marc-André Lureau 

G_NORETURN was introduced in glib 2.68, fallback to G_GNUC_NORETURN in
glib-compat.

Note that this attribute must be placed before the function declaration
(bringing a bit of consistency in qemu codebase usage).

Signed-off-by: Marc-André Lureau 
---

[...]

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 848916f5165c..14b6b65a5fa9 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -177,7 +177,8 @@ extern "C" {
   * supports QEMU_ERROR, this will be reported at compile time; otherwise
   * this will be reported at link time due to the missing symbol.
   */
-extern void QEMU_NORETURN QEMU_ERROR("code path is reachable")
+extern G_NORETURN
+void QEMU_ERROR("code path is reachable")
  qemu_build_not_reached_always(void);
  #if defined(__OPTIMIZE__) && !defined(__NO_INLINE__)
  #define qemu_build_not_reached()  qemu_build_not_reached_always()


The placement of G_NORETURN causes a compiler error for C++ code in 
cross builds for Windows (see below). C++ expects the attribute 
[[noreturn]] before the extern statement.


I updated my Debian build environment to Debian bookworm and a recent 
cross glib, so maybe the problem was hidden in previous builds because I 
used a rather old glib or an older g++ cross compiler.


Regards,
Stefan


In file included from /mingw64/lib/glib-2.0/include/glibconfig.h:9,
 from /mingw64/include/glib-2.0/glib/gtypes.h:34,
 from /mingw64/include/glib-2.0/glib/galloca.h:34,
 from /mingw64/include/glib-2.0/glib.h:32,
 from 
/home/stefan/src/qemu/repo.or.cz/qemu/ar7/include/glib-compat.h:32,
 from 
/home/stefan/src/qemu/repo.or.cz/qemu/ar7/include/qemu/osdep.h:144,

 from ../../../qga/vss-win32/requester.cpp:13:
/mingw64/include/glib-2.0/glib/gmacros.h:1076:21: error: standard 
attributes in middle of decl-specifiers

 1076 | # define G_NORETURN [[noreturn]]
  | ^
/home/stefan/src/qemu/repo.or.cz/qemu/ar7/include/qemu/osdep.h:240:8: 
note: in expansion of macro ‘G_NORETURN’

  240 | extern G_NORETURN
  |^~
/mingw64/include/glib-2.0/glib/gmacros.h:1076:21: note: standard 
attributes must precede the decl-specifiers to apply to the declaration, 
or follow them to apply to the type

 1076 | # define G_NORETURN [[noreturn]]
  | ^
/home/stefan/src/qemu/repo.or.cz/qemu/ar7/include/qemu/osdep.h:240:8: 
note: in expansion of macro ‘G_NORETURN’

  240 | extern G_NORETURN
  |^~
/mingw64/include/glib-2.0/glib/gmacros.h:1076:21: warning: attribute 
ignored [-Wattributes]

 1076 | # define G_NORETURN [[noreturn]]
  | ^
/home/stefan/src/qemu/repo.or.cz/qemu/ar7/include/qemu/osdep.h:240:8: 
note: in expansion of macro ‘G_NORETURN’

  240 | extern G_NORETURN
  |^~
/mingw64/include/glib-2.0/glib/gmacros.h:1076:21: note: an attribute 
that appertains to a type-specifier is ignored

 1076 | # define G_NORETURN [[noreturn]]
  | ^
/home/stefan/src/qemu/repo.or.cz/qemu/ar7/include/qemu/osdep.h:240:8: 
note: in expansion of macro ‘G_NORETURN’

  240 | extern G_NORETURN
  |^~
../../../qga/vss-win32/requester.cpp: In function ‘HRESULT 
requester_init()’:
../../../qga/vss-win32/requester.cpp:72:34: warning: cast between 
incompatible function types from ‘FARPROC’ {aka ‘long long int (*)()’} 
to ‘t_CreateVssBackupComponents’ {aka ‘long int 
(*)(IVssBackupComponents**)’} [-Wcast-function-type]

   72 | pCreateVssBackupComponents = (t_CreateVssBackupComponents)
  |  ^
   73 | GetProcAddress(hLib,
  | 
   74 | #ifdef _WIN64 /* 64bit environment */
  | ~
   75 | 
"?CreateVssBackupComponents@@YAJPEAPEAVIVssBackupComponents@@@Z"
  | 


   76 | #else /* 32bit environment */
  | ~
   77 | 
"?CreateVssBackupComponents@@YGJPAPAVIVssBackupComponents@@@Z"
  | 
~~

   78 | #endif
  | ~~
   79 | );
  | ~
../../../qga/vss-win32/requester.cpp:85:34: warning: cast between 
incompatible function types from ‘FARPROC’ {aka ‘long long int (*)()’} 
to ‘t_VssFreeSnapshotProperties’ {aka ‘void (*)(VSS_SNAPSHOT_PROP*)’} 
[-Wcast-function-type]

   85 | pVssFreeSnapshotProperties = (t_VssFreeSnapshotProperties)
  |  ^
   86 | GetProcAddress(hLib, "VssFreeSnapshotProperties");
  | ~
ninja: build stopped: su

Re: [PATCH-for-8.0] block/dmg: Ignore C99 prototype declaration mismatch from

[Stefan Weil via]

Am 27.03.23 um 23:09 schrieb Paolo Bonzini:

Il lun 27 mar 2023, 20:58 Philippe Mathieu-Daudé  
ha scritto:


> The warning can also be suppressed if the build uses `-isystem
> /opt/homebrew/include` instead of `-I/opt/homebrew/include` as I
just
> have tested.

Is that option added by QEMU's configure or meson.build? Or is it 
added by homebrew? The fact that /opt/homebrew/include it isn't 
considered a system seems to be a homebrew decision.


IIUC by design meson only allows including *relative* directories,
and manage the system ones:
https://mesonbuild.com/Include-directories.html

That's for includes that are part of QEMU.

Meson has as_system for dependency objects 
(https://mesonbuild.com/Reference-manual_returned_dep.html) but lzfse 
doesn't have a .pc file, its detection has to be done by hand.


Paolo

> If we can find a solution how to implement that I thing it would
look
> nicer. Technically the patch looks good.
>
> Reviewed-by: Stefan Weil 

Thanks!



Typically I configure the build on macOS with `./configure 
--extra-cflags=-I/opt/homebrew/include 
--extra-ldflags=-L/opt/homebrew/lib --disable-werror`. With that 
configuration I get the two warnings for lzfse.h.


When I use `./configure '--extra-cflags=-isystem /opt/homebrew/include' 
--extra-ldflags=-L/opt/homebrew/lib --disable-werror` instead, I get no 
compiler warnings (and `--disable-werror` could be ommitted).


So at least for macOS with Homebrew in /opt/homebrew (M1 / M2 Macs) the 
patch is not needed when the right configure options (`--extra-cflags`) 
were used.


Stefan

Re: [PATCH-for-8.0] block/dmg: Ignore C99 prototype declaration mismatch from

[Stefan Weil via]

Am 27.03.23 um 17:13 schrieb Philippe Mathieu-Daudé:


When liblzfe (Apple LZFSE compression library) is present
(for example installed via 'brew') on Darwin, QEMU build
fails as:

   Has header "lzfse.h" : YES
   Library lzfse found: YES

 Dependencies
   lzo support  : NO
   snappy support   : NO
   bzip2 support: YES
   lzfse support: YES
   zstd support : YES 1.5.2

 User defined options
   dmg  : enabled
   lzfse: enabled

   [221/903] Compiling C object libblock.fa.p/block_dmg-lzfse.c.o
   FAILED: libblock.fa.p/block_dmg-lzfse.c.o
   /opt/homebrew/Cellar/lzfse/1.0/include/lzfse.h:56:43: error: this function 
declaration is not a prototype [-Werror,-Wstrict-prototypes]
   LZFSE_API size_t lzfse_encode_scratch_size();
 ^
  void
   /opt/homebrew/Cellar/lzfse/1.0/include/lzfse.h:94:43: error: this function 
declaration is not a prototype [-Werror,-Wstrict-prototypes]
   LZFSE_API size_t lzfse_decode_scratch_size();
 ^
  void
   2 errors generated.
   ninja: build stopped: subcommand failed.

This issue has been reported in the lzfse project in 2016:
https://github.com/lzfse/lzfse/issues/3#issuecomment-226574719

Since the project seems unmaintained, simply ignore the
strict-prototypes warning check for the  header,
similarly to how we deal with the GtkItemFactoryCallback
prototype from , indirectly included
by .

Cc: Julio Faracco 
Signed-off-by: Philippe Mathieu-Daudé 
---
  block/dmg-lzfse.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/block/dmg-lzfse.c b/block/dmg-lzfse.c
index 6798cf4fbf..0abc970bf6 100644
--- a/block/dmg-lzfse.c
+++ b/block/dmg-lzfse.c
@@ -23,7 +23,12 @@
   */
  #include "qemu/osdep.h"
  #include "dmg.h"
+
+/* Work around an -Wstrict-prototypes warning in LZFSE headers */



"Work around a -Wstrict-prototypes" ("a" instead of "an")?



+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wstrict-prototypes"
  #include 
+#pragma GCC diagnostic pop
  
  static int dmg_uncompress_lzfse_do(char *next_in, unsigned int avail_in,

 char *next_out, unsigned int avail_out)



The warning can also be suppressed if the build uses `-isystem 
/opt/homebrew/include` instead of `-I/opt/homebrew/include` as I just 
have tested.


If we can find a solution how to implement that I thing it would look 
nicer. Technically the patch looks good.


Reviewed-by: Stefan Weil 

Thanks,

Stefan

Re: [PATCH] Updated the FSF address to

[Stefan Weil via]

Am 20.02.23 um 08:01 schrieb Khadija Kamran:

From: Khadija Kamran 

The Free Software Foundation moved to a new address and some sources in QEMU 
referred to their old location.
The address should be updated and replaced to a pointer to 



... replaced by a pointer ... ?


Resolves: https://gitlab.com/qemu-project/qemu/-/issues/379

Signed-off-by: Khadija Kamran 
---
  contrib/gitdm/filetypes.txt | 3 +--
  hw/scsi/viosrp.h| 3 +--
  hw/sh4/sh7750_regs.h| 3 +--
  include/hw/arm/raspi_platform.h | 3 +--
  include/qemu/uri.h  | 3 +--
  tests/qemu-iotests/022  | 4 +---
  tests/unit/rcutorture.c | 3 +--
  tests/unit/test-rcu-list.c  | 3 +--
  util/uri.c  | 3 +--
  9 files changed, 9 insertions(+), 19 deletions(-)



Reviewed-by: Stefan Weil 

Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems

[Stefan Weil via]

Am 19.02.23 um 12:27 schrieb Reinoud Zandijk:


On Fri, Feb 17, 2023 at 12:05:46PM +0100, Stefan Weil wrote:

So there still seems to be a certain small need for QEMU installers for
32-bit Windows: 170 users für 32 bit only, 339 users for both 32 and 64 bit,
5132 users for 64 bit only.

As you seem to have access to download stats could you check generic download
stats too i.e. not only for Windows installers?



Sorry, but I only have statistics for my own server 
https://qemu.weilnetz.de/ which provides Windows installers.


Stefan

Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems

[Stefan Weil via]

On Fri, Feb 17, 2023 at 11:36:41AM +0100, Markus Armbruster wrote:


Which 32-bit hosts are still useful, and why?



Citing my previous mail:

   I now checked all downloads of the latests installers since 2022-12-30.

   qemu-w32-setup-20221230.exe – 509 different IP addresses
   qemu-w64-setup-20221230.exe - 5471 different IP addresses

   339 unique IP addresses are common for 32- and 64-bit, either
   crawlers or people who simply got both variants. So there remain 170
   IP addresses which only downloaded the 32-bit variant in the last week.

   I see 437 different strings for the browser type, but surprisingly
   none of them looks like a crawler.

So there still seems to be a certain small need for QEMU installers for 
32-bit Windows: 170 users für 32 bit only, 339 users for both 32 and 64 
bit, 5132 users for 64 bit only.


Stefan

Re: [PATCH v2 1/2] qga-win: add logging to Windows event log

[Stefan Weil via]

Am 23.01.23 um 20:38 schrieb Andrey Drobyshev:


Hi Stefan,

On 1/23/23 19:28, Stefan Weil wrote:

Hi,

cross builds fail with this code. Please see details below.

Am 29.11.22 um 18:38 schrieb Andrey Drobyshev via:

This commit allows QGA to write to Windows event log using Win32 API's
ReportEvent() [1], much like syslog() under *nix guests.

In order to generate log message definitions we use a very basic message
text file [2], so that every QGA's message gets ID 1.  The tools
"windmc" and "windres" respectively are used to generate ".rc" file and
COFF object file, and then the COFF file is linked into qemu-ga.exe.

[1]
https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-reporteventa
[2]
https://learn.microsoft.com/en-us/windows/win32/eventlog/message-text-files

Originally-by: Yuri Pudgorodskiy 
Signed-off-by: Andrey Drobyshev 
---
   configure |  3 +++
   qga/installer/qemu-ga.wxs |  5 +
   qga/main.c    | 16 +---
   qga/meson.build   | 19 ++-
   qga/messages-win32.mc |  9 +
   5 files changed, 48 insertions(+), 4 deletions(-)
   create mode 100644 qga/messages-win32.mc

diff --git a/configure b/configure
index 26c7bc5154..789a4f6cc9 100755
--- a/configure
+++ b/configure
@@ -372,6 +372,7 @@ smbd="$SMBD"
   strip="${STRIP-${cross_prefix}strip}"
   widl="${WIDL-${cross_prefix}widl}"
   windres="${WINDRES-${cross_prefix}windres}"
+windmc="${WINDMC-${cross_prefix}windmc}"

Here the needed cross prefix is added ...


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

[...]

diff --git a/qga/meson.build b/qga/meson.build
index 3cfb9166e5..1ff159edc1 100644
--- a/qga/meson.build
+++ b/qga/meson.build
@@ -98,7 +98,24 @@ if targetos == 'windows'
     endif
   endif
   -qga = executable('qemu-ga', qga_ss.sources(),
+qga_objs = []
+if targetos == 'windows'
+  windmc = find_program('windmc', required: true)

... but here the cross prefix is missing and the cross build aborts
because windmc does not exist.

There's no need for the cross prefix here.  After you've run ./configure
with --cross-prefix, argument, you'll see the following in
build/config-meson.cross file:

[binaries]

widl = ['x86_64-w64-mingw32-widl']
windres = ['x86_64-w64-mingw32-windres']
windmc = ['x86_64-w64-mingw32-windmc']

And these are the actual values meson's find_program() is going to be
looking for.  So it doesn't seem like there's anything broken here, it's
a matter of build requirements.



My configure terminates with an error because of the missing windmc 
before it has written config-meson.cross. I have run an incremental build:


Program windmc found: NO

../../../qga/meson.build:103:2: ERROR: Program 'windmc' not found or not 
executable


A full log can be found at 
/qemu/bin/debug/x86_64-w64-mingw32/meson-logs/meson-log.txt

ninja: error: rebuilding 'build.ninja': subcommand failed
FAILED: build.ninja
/usr/bin/python3 /qemu/meson/meson.py --internal regenerate /qemu 
/home/stefan/src/gitlab/qemu-project/qemu/bin/debug/x86_64-w64-mingw32 
--backend ninja

make: *** [Makefile:165: run-ninja] Fehler 1
make: Verzeichnis „/qemu/bin/debug/x86_64-w64-mingw32“ wird verlassen

A clean fresh build works indeed fine.

Stefan

Re: [PATCH v2 1/2] qga-win: add logging to Windows event log

[Stefan Weil via]

Hi,

cross builds fail with this code. Please see details below.

Am 29.11.22 um 18:38 schrieb Andrey Drobyshev via:

This commit allows QGA to write to Windows event log using Win32 API's
ReportEvent() [1], much like syslog() under *nix guests.

In order to generate log message definitions we use a very basic message
text file [2], so that every QGA's message gets ID 1.  The tools
"windmc" and "windres" respectively are used to generate ".rc" file and
COFF object file, and then the COFF file is linked into qemu-ga.exe.

[1] 
https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-reporteventa
[2] https://learn.microsoft.com/en-us/windows/win32/eventlog/message-text-files

Originally-by: Yuri Pudgorodskiy 
Signed-off-by: Andrey Drobyshev 
---
  configure |  3 +++
  qga/installer/qemu-ga.wxs |  5 +
  qga/main.c| 16 +---
  qga/meson.build   | 19 ++-
  qga/messages-win32.mc |  9 +
  5 files changed, 48 insertions(+), 4 deletions(-)
  create mode 100644 qga/messages-win32.mc

diff --git a/configure b/configure
index 26c7bc5154..789a4f6cc9 100755
--- a/configure
+++ b/configure
@@ -372,6 +372,7 @@ smbd="$SMBD"
  strip="${STRIP-${cross_prefix}strip}"
  widl="${WIDL-${cross_prefix}widl}"
  windres="${WINDRES-${cross_prefix}windres}"
+windmc="${WINDMC-${cross_prefix}windmc}"


Here the needed cross prefix is added ...


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

[...]

diff --git a/qga/meson.build b/qga/meson.build
index 3cfb9166e5..1ff159edc1 100644
--- a/qga/meson.build
+++ b/qga/meson.build
@@ -98,7 +98,24 @@ if targetos == 'windows'
endif
  endif
  
-qga = executable('qemu-ga', qga_ss.sources(),

+qga_objs = []
+if targetos == 'windows'
+  windmc = find_program('windmc', required: true)


... but here the cross prefix is missing and the cross build aborts 
because windmc does not exist.


Regards
Stefan

Re: Announcement of aborting HAXM maintenance

[Stefan Weil via]

Am 19.01.23 um 11:12 schrieb Daniel P. Berrangé:

On Thu, Jan 19, 2023 at 03:56:04AM +, Wang, Wenchao wrote:

Hi, Philippe,

Intel decided to abort the development of HAXM and the maintenance
of its QEMU part. Should we submit a patch to mark the Guest CPU
Cores (HAXM) status as Orphan and remove the maintainers from the
corresponding list? Meanwhile, should the code enabling HAX in QEMU
once committed by the community be retained?


If you no longer intend to work on QEMU bits related to HAXM, then
yes, you should send a patch for the MAINTAINERS file to remove you
name and mark it as "Orphan" status.

We would not normally delete code from QEMU, merely because it has
been orphaned. If it is still known to work then we would retain
it indefinitely, unless some compelling reason arises to drop it.
This gives time for any potential users to adjust their plans,
and/or opportunity for other interested people to take over the
maintenance role.


HAXM will not only be no longer maintained in QEMU, but also the 
necessary framework for macOS and Windows will be retired. See 
https://github.com/intel/haxm#status on their GitHub page. As stated 
there, macOS provides HVF which can be used instead of HAXM, and Windows 
users can use WHPX. Both HVF and WHPX are supported by QEMU. As far as I 
know HAXM could only provide a limited RAM size (2 GiB?). Maybe it still 
has more deficits. And unmaintained HAXM drivers for macOS and Windows 
might be a security risk, too. It is also not clear whether the last 
downloads of those drivers will be available in the future.


Therefore I'd prefer to remove the whole HAXM code in QEMU soon, even in 
a minor update for this special case.


Stefan

Re: MSYS2 and libfdt

[Stefan Weil via]

Am 19.01.23 um 09:14 schrieb Thomas Huth:



 Hi all,

in some spare minutes, I started playing with a patch to try to remove 
the dtc submodule from the QEMU git repository - according to 
https://repology.org/project/dtc/versions our supported build 
platforms should now all provide the minimum required version.


However, I'm hitting a problem with Windows / MSYS2 in the CI jobs: 
The libfdt is packaged as part of the dtc package there:


 https://packages.msys2.org/package/dtc

... meaning that it is added with a usr/include and usr/lib path 
prefix instead of mingw64/include and mingw64/lib like other packages 
are using (see e.g. 
https://packages.msys2.org/package/mingw-w64-x86_64-zlib?repo=mingw64). 
Thus the compiler does not find the library there. Also there does not 
seem to be a difference between a i686 (32-bit) and x86_64 (64-bit) 
variant available here? Does anybody know how libfdt is supposed to be 
used with MSYS2 ?


 Thomas



Hi Thomas,

"dtc" is not the right package for cross builds. We'd require 
mingw-w64-i686-dtc and mingw-w64-x86_64-dtc packages for the QEMU build, 
but those packages are currently not provided by MSYS2.


See https://packages.msys2.org/search?t=binpkg&q=zlib for the zlib 
packages and related cross build packages.


Stefan

Re: building qemu on windows 11

[Stefan Weil via]

Am 12.01.23 um 16:16 schrieb Neal Elliott:

Hello,
              is it possible, or has anyone built qemu from the master 
branch using visual studio? I attempted to
build the code using mingw64, but it failed to build. is there a current 
build document for windows?


Building with Visual Studio is not supported.

Building on Windows with Mingw64 might fail (see 
https://gitlab.com/qemu-project/qemu/-/issues/1386), and the current 
documentation requires an update.


I suggest to run a cross build on Linux which works and is also much 
faster than building on Windows. Here is an example of such a build 
running as a GitHub action:

https://github.com/stweil/qemu/actions/runs/3903880614/jobs/6668872989

The related files are win.yml, build.sh and pacman.sh
from https://github.com/stweil/qemu/tree/master/.github/workflows.
The two shell scripts should also work on a typical Debian / Ubuntu or 
similar Linux host.


Regards
Stefan Weil

Re: [PATCH] .gitlab-ci.d/windows: Do not run the qtests in the msys2-32bit job

[Stefan Weil via]

Am 06.01.23 um 09:19 schrieb Thomas Huth:


On 06/01/2023 09.15, Stefan Weil wrote:


Download numbers from yesterday for my latest Windows installers:

qemu-w32-setup-20221230.exe - 243

qemu-w64-setup-20221230.exe - 6540

On Wednesday the ratio was 288 : 3516.

As expected the 64-bit variant is used much more often, but it looks 
like there is still a certain desire for the 32-bit variant.


OK, thanks. Could you maybe also check the browser types in the logs? 
... I'm wondering whether a big part of those w32 downloads were just 
automatic web crawlers?


 Thomas



I now checked all downloads of the latests installers since 2022-12-30.

qemu-w32-setup-20221230.exe – 509 different IP addresses
qemu-w64-setup-20221230.exe - 5471 different IP addresses

339 unique IP addresses are common for 32- and 64-bit, either crawlers 
or people who simply got both variants. So there remain 170 IP addresses 
which only downloaded the 32-bit variant in the last week.


I see 437 different strings for the browser type, but surprisingly none 
of them looks like a crawler.


Stefan

Re: [PATCH] .gitlab-ci.d/windows: Do not run the qtests in the msys2-32bit job

[Stefan Weil via]

Am 06.01.23 um 08:49 schrieb Thomas Huth:


On 05/01/2023 22.42, Philippe Mathieu-Daudé wrote:

> That said, maybe it is time to deprecate the 32-bit
> hosts?

Certainly fine for me, but that's up to the Windows folks to decide. 
Maybe you could just suggest a patch to start the discussion?


 Thomas



Download numbers from yesterday for my latest Windows installers:

qemu-w32-setup-20221230.exe - 243

qemu-w64-setup-20221230.exe - 6540

On Wednesday the ratio was 288 : 3516.

As expected the 64-bit variant is used much more often, but it looks 
like there is still a certain desire for the 32-bit variant.


Stefan

Re: [PATCH v3 04/26] configure: don't enable cross compilers unless in target_list

[Stefan Weil via]

Am 21.10.22 um 00:10 schrieb Richard Henderson:

On 10/20/22 21:51, Alex Bennée wrote:

This avoids the unfortunate effect of always builds the pc-bios blobs
for targets the user isn't interested in.

Suggested-by: Paolo Bonzini 
Signed-off-by: Alex Bennée 
---
  configure | 9 +
  1 file changed, 9 insertions(+)

diff --git a/configure b/configure
index 81561be7c1..dd6f58dcde 100755
--- a/configure
+++ b/configure
@@ -1877,6 +1877,15 @@ probe_target_compiler() {
    container_cross_ranlib=
    container_cross_strip=
+  # We shall skip configuring the target compiler if the user didn't
+  # bother enabling an appropriate guest. This avoids building
+  # extraneous firmware images and tests.
+  if test "${target_list#*$1}" != "$1"; then
+  break;



Isn't break limited for exiting from for, while, or until loop? (*)
If yes, it's wrongly used here. sh does not complain, but other
shells do.

Stefan

*) https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html

Re: [PATCH v2] target/i386: Remove compilation errors when -Werror=maybe-uninitialized

[Stefan Weil via]

Am 22.12.22 um 12:51 schrieb Philippe Mathieu-Daudé:


On 22/12/22 12:18, Eric Auger wrote:

Hi All,

On 12/22/22 12:09, Daniel P. Berrangé wrote:

On Thu, Dec 22, 2022 at 11:07:31AM +0100, Eric Auger wrote:

Hi Philippe,

On 12/22/22 10:01, Philippe Mathieu-Daudé wrote:

On 22/12/22 09:18, Paolo Bonzini wrote:

On 12/21/22 17:36, Eric Auger wrote:
To avoid compilation errors when -Werror=maybe-uninitialized is 
used,

replace 'case 3' by 'default'.

Otherwise we get:

../target/i386/ops_sse.h: In function ‘helper_vpermdq_ymm’:
../target/i386/ops_sse.h:2495:13: error: ‘r3’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
 2495 | d->Q(3) = r3;
  | ^~~~
../target/i386/ops_sse.h:2494:13: error: ‘r2’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
 2494 | d->Q(2) = r2;
  | ^~~~
../target/i386/ops_sse.h:2493:13: error: ‘r1’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
 2493 | d->Q(1) = r1;
  | ^~~~
../target/i386/ops_sse.h:2492:13: error: ‘r0’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
 2492 | d->Q(0) = r0;
  | ^~~~

With what compiler? Is that a supported one?
https://lore.kernel.org/qemu-devel/3aab489e-9d90-c1ad-0b6b-b2b5d80db...@redhat.com/ 


Signed-off-by: Eric Auger 
Suggested-by: Stefan Weil 
Fixes: 790684776861 ("target/i386: reimplement 0x0f 0x3a, add AVX")
---
   target/i386/ops_sse.h | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/i386/ops_sse.h b/target/i386/ops_sse.h
index 3cbc36a59d..c442c8c10c 100644
--- a/target/i386/ops_sse.h
+++ b/target/i386/ops_sse.h
@@ -2466,7 +2466,7 @@ void helper_vpermdq_ymm(Reg *d, Reg *v, Reg
*s, uint32_t order)
   r0 = s->Q(0);
   r1 = s->Q(1);
   break;
-    case 3:
+    default:
   r0 = s->Q(2);
   r1 = s->Q(3);
   break;
@@ -2484,7 +2484,7 @@ void helper_vpermdq_ymm(Reg *d, Reg *v, Reg
*s, uint32_t order)
   r2 = s->Q(0);
   r3 = s->Q(1);
   break;
-    case 3:
+    default:
   r2 = s->Q(2);
   r3 = s->Q(3);
   break;

Queued, but this compiler sucks. :)

Can't we simply add a dumb 'default' case? So when reviewing we don't
have to evaluate 'default' means 3 here.

-- >8 --
--- a/target/i386/ops_sse.h
+++ b/target/i386/ops_sse.h
@@ -2470,6 +2470,8 @@ void helper_vpermdq_ymm(Reg *d, Reg *v, Reg *s,
uint32_t order)
  r0 = s->Q(2);
  r1 = s->Q(3);
  break;
+    default:
+    qemu_build_not_reached();
  }
  switch ((order >> 4) & 3) {
  case 0:
@@ -2488,6 +2490,8 @@ void helper_vpermdq_ymm(Reg *d, Reg *v, Reg *s,
uint32_t order)
  r2 = s->Q(2);
  r3 = s->Q(3);
  break;
+    default:
+    qemu_build_not_reached();
  }
I guess this won't fix the fact r0, r1, r2, r3 are not initialized, 
will it?

This ultimately expands to assert() and the compiler should see that it
terminates the control flow at this point, so shouldn't have a reason
to warn.


OK so with qemu_build_not_reached(); I get

/home/augere/UPSTREAM/qemu/include/qemu/osdep.h:184:35: error: call to
‘qemu_build_not_reached_always’ declared with attribute error: code path
is reachable
   184 | #define qemu_build_not_reached() 
qemu_build_not_reached_always()

   | ^~~


However with g_assert_not_reached(), it does not complain and errors are
removed. So I will respin with g_assert_not_reached() if nobody advises
me against that.


Thank you!



As noted by Paolo a better compiler could know that 0, 1, 2 and 3 are 
the only possible cases. Such a better compiler might complain that an 
additional default case is never reached. Therefore the proposed code 
might cause future compiler warnings.


But we could use this code pattern to make the intention of the code 
clearer:


case 3:
default: /* default case added to help the compiler to avoid warnings */
    ...

Stefan

Re: [PATCH v2] target/i386: Remove compilation errors when -Werror=maybe-uninitialized

[Stefan Weil via]

Am 21.12.22 um 17:36 schrieb Eric Auger:


To avoid compilation errors when -Werror=maybe-uninitialized is used,
replace 'case 3' by 'default'.

Otherwise we get:

../target/i386/ops_sse.h: In function ‘helper_vpermdq_ymm’:
../target/i386/ops_sse.h:2495:13: error: ‘r3’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
2495 | d->Q(3) = r3;
 | ^~~~
../target/i386/ops_sse.h:2494:13: error: ‘r2’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
2494 | d->Q(2) = r2;
 | ^~~~
../target/i386/ops_sse.h:2493:13: error: ‘r1’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
2493 | d->Q(1) = r1;
 | ^~~~
../target/i386/ops_sse.h:2492:13: error: ‘r0’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
2492 | d->Q(0) = r0;
 | ^~~~

Signed-off-by: Eric Auger 
Suggested-by: Stefan Weil 
Fixes: 790684776861 ("target/i386: reimplement 0x0f 0x3a, add AVX")
---
  target/i386/ops_sse.h | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/i386/ops_sse.h b/target/i386/ops_sse.h
index 3cbc36a59d..c442c8c10c 100644
--- a/target/i386/ops_sse.h
+++ b/target/i386/ops_sse.h
@@ -2466,7 +2466,7 @@ void helper_vpermdq_ymm(Reg *d, Reg *v, Reg *s, uint32_t 
order)
  r0 = s->Q(0);
  r1 = s->Q(1);
  break;
-case 3:
+default:
  r0 = s->Q(2);
  r1 = s->Q(3);
  break;
@@ -2484,7 +2484,7 @@ void helper_vpermdq_ymm(Reg *d, Reg *v, Reg *s, uint32_t 
order)
  r2 = s->Q(0);
  r3 = s->Q(1);
  break;
-case 3:
+default:
  r2 = s->Q(2);
  r3 = s->Q(3);
  break;



Reviewed-by: Stefan Weil 

Thank you and merry Christmas!




OpenPGP_0xE08C21D5677450AD.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH 1/6] disas: add G_GNUC_PRINTF to gstring_printf

[Stefan Weil via]

Am 19.12.22 um 14:02 schrieb Daniel P. Berrangé:

Signed-off-by: Daniel P. Berrangé 
---
  disas.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/disas.c b/disas.c
index 94d3b45042..31df8f5b89 100644
--- a/disas.c
+++ b/disas.c
@@ -239,6 +239,7 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong 
code,
  }
  }
  
+G_GNUC_PRINTF(2, 3)

  static int gstring_printf(FILE *stream, const char *fmt, ...)
  {
  /* We abuse the FILE parameter to pass a GString. */


The current code uses a different pattern:

static RETURN_TYPE G_GNUC_PRINTF(2, 3) function(argument list)

So I would have expected

static int G_GNUC_PRINTF(2, 3)
gstring_printf(FILE *stream, const char *fmt, ...)

Does that matter? Do we care for different patterns?

Stefan


OpenPGP_0xE08C21D5677450AD.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH] mailmap: Fix Stefan Weil author email

[Stefan Weil via]

Am 08.12.22 um 16:55 schrieb Philippe Mathieu-Daudé:


Fix authorship of commits 266aaedc37~..ac14949821. See commit
3bd2608db7 ("maint: Add .mailmap entries for patches claiming
list authorship") for rationale.

Signed-off-by: Philippe Mathieu-Daudé 
---
  .mailmap | 1 +
  1 file changed, 1 insertion(+)

diff --git a/.mailmap b/.mailmap
index 35dddbe27b..fad2aff5aa 100644
--- a/.mailmap
+++ b/.mailmap
@@ -45,6 +45,7 @@ Ed Swierk  Ed Swierk via Qemu-devel 
 Ian McKellar via Qemu-devel 

  Julia Suvorova  Julia Suvorova via Qemu-devel 

  Justin Terry (VM)  Justin Terry (VM) via Qemu-devel 

+Stefan Weil  Stefan Weil via 
  
  # Next, replace old addresses by a more recent one.

  Aleksandar Markovic  




Signed-off-by: Stefan Weil 

Thanks!




OpenPGP_0xE08C21D5677450AD.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PULL 11/21] docs: Build and install all the docs in a single manual

[Stefan Weil via]

Am 12.01.21 um 17:57 schrieb Peter Maydell:
[...]

diff --git a/docs/meson.build b/docs/meson.build
index fae9849b79b..bb14eaebd3b 100644
--- a/docs/meson.build
+++ b/docs/meson.build
@@ -46,19 +46,11 @@ if build_docs
meson.source_root() / 'docs/sphinx/qmp_lexer.py',
qapi_gen_depends ]
  
-  configure_file(output: 'index.html',

- input: files('index.html.in'),
- configuration: {'VERSION': meson.project_version()},
- install_dir: qemu_docdir)
-  manuals = [ 'devel', 'interop', 'tools', 'specs', 'system', 'user' ]
man_pages = {
-'interop' : {
  'qemu-ga.8': (have_tools ? 'man8' : ''),
  'qemu-ga-ref.7': 'man7',
  'qemu-qmp-ref.7': 'man7',
  'qemu-storage-daemon-qmp-ref.7': (have_tools ? 'man7' : ''),
-},
-'tools': {
  'qemu-img.1': (have_tools ? 'man1' : ''),
  'qemu-nbd.8': (have_tools ? 'man8' : ''),
  'qemu-pr-helper.8': (have_tools ? 'man8' : ''),
@@ -66,53 +58,47 @@ if build_docs
  'qemu-trace-stap.1': (config_host.has_key('CONFIG_TRACE_SYSTEMTAP') ? 
'man1' : ''),
  'virtfs-proxy-helper.1': (have_virtfs_proxy_helper ? 'man1' : ''),
  'virtiofsd.1': (have_virtiofsd ? 'man1' : ''),
-},
-'system': {
  'qemu.1': 'man1',
  'qemu-block-drivers.7': 'man7',
  'qemu-cpu-models.7': 'man7'
-},
}
  
sphinxdocs = []

sphinxmans = []
-  foreach manual : manuals
-private_dir = meson.current_build_dir() / (manual + '.p')
-output_dir = meson.current_build_dir() / manual
-input_dir = meson.current_source_dir() / manual
  
-this_manual = custom_target(manual + ' manual',

+  private_dir = meson.current_build_dir() / 'manual.p'
+  output_dir = meson.current_build_dir() / 'manual'
+  input_dir = meson.current_source_dir()
+
+  this_manual = custom_target('QEMU manual',
  build_by_default: build_docs,
-output: [manual + '.stamp'],
-input: [files('conf.py'), files(manual / 'conf.py')],
-depfile: manual + '.d',
+output: 'docs.stamp',
+input: files('conf.py'),
+depfile: 'docs.d',
  depend_files: sphinx_extn_depends,
  command: [SPHINX_ARGS, '-Ddepfile=@DEPFILE@',
'-Ddepfile_stamp=@OUTPUT0@',
'-b', 'html', '-d', private_dir,
input_dir, output_dir])
-sphinxdocs += this_manual
-if build_docs and manual != 'devel'
-  install_subdir(output_dir, install_dir: qemu_docdir)
-endif
+  sphinxdocs += this_manual
+  install_subdir(output_dir, install_dir: qemu_docdir, strip_directory: true)


This line causes a build warning with the latest code:

../../../docs/meson.build:74: WARNING: Project targets '>=0.61.3' but 
uses feature deprecated since '0.60.0': install_subdir with empty 
directory. It worked by accident and is buggy. Use install_emptydir instead.


It looks like `qemu_docdir` is no longer defined anywhere.

I still did not find out whether this is an issue which needs a fix for 7.2.

Stefan

  
-these_man_pages = []

-install_dirs = []

[...]

Compiler warnings with maximum warning level (was: Re: [PATCH for 7.2?] target/i386: Remove compilation errors when -Werror=maybe-uninitialized)

[Stefan Weil via]

Am 07.12.22 um 20:11 schrieb Stefan Weil:

On 12/7/22 14:24, Eric Auger wrote:

Initialize r0-3 to avoid compilation errors when
-Werror=maybe-uninitialized is used

../target/i386/ops_sse.h: In function ‘helper_vpermdq_ymm’:
../target/i386/ops_sse.h:2495:13: error: ‘r3’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
   2495 | d->Q(3) = r3;
    | ^~~~
../target/i386/ops_sse.h:2494:13: error: ‘r2’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
   2494 | d->Q(2) = r2;
    | ^~~~
../target/i386/ops_sse.h:2493:13: error: ‘r1’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
   2493 | d->Q(1) = r1;
    | ^~~~
../target/i386/ops_sse.h:2492:13: error: ‘r0’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
   2492 | d->Q(0) = r0;
    | ^~~~

Signed-off-by: Eric Auger 
Fixes: 790684776861 ("target/i386: reimplement 0x0f 0x3a, add AVX")

---

Am I the only one getting this? Or anything wrong in my setup.


Hi Eric,

no, you are not the only one. I regularly build with higher warning 
levels, for example with -Weverything on macOS, and get a much longer 
list which includes the mentioned warnings (see below).


The latest QEMU code produces 6780505 compiler warnings and a build log 
file with 2.7 GB (!) with compiler option `-Weverything` on macOS.


Many warnings occur more than once, but there remain 193313 unique 
warnings for the QEMU code (see 
https://qemu.weilnetz.de/test/warnings-20221207.txt). Here is a list of 
all kinds of warnings sorted by frequency:


   1 -Wkeyword-macro
   1 -Wundeclared-selector
   1 -Wunreachable-code-loop-increment
   1 -Wunused-but-set-parameter
   2 -Wgnu-union-cast
   2 -Woverlength-strings
   3 -Walloca
   5 -Wflexible-array-extensions
   5 -Wstrict-selector-match
   5 -Wstring-conversion
   5 -Wtautological-value-range-compare
   6 -Wcstring-format-directive
   8 -Wstatic-in-inline
  13 -Wobjc-messaging-id
  13 -Wvla
  14 -Wobjc-interface-ivars
  16 -Wimplicit-float-conversion
  17 -Wformat-nonliteral
  24 -Wredundant-parens
  39 -Wfloat-equal
  44 -Wc++-compat
  47 -Wzero-length-array
  53 -Wdouble-promotion
  53 -Wvariadic-macros
  65 -Wpacked
  74 -Wcomma
  82 -Wunreachable-code-return
  90 -Wformat-pedantic
  90 -Wmissing-noreturn
  94 -Wgnu-flexible-array-initializer
 120 -Wcovered-switch-default
 132 -Wdirect-ivar-access
 136 -Wconditional-uninitialized
 144 -Wgnu-designator
 147 -Wdisabled-macro-expansion
 150 -Wgnu-conditional-omitted-operand
 161 -Wunreachable-code-break
 184 -Wcompound-token-split-by-space
 228 -Wfloat-conversion
 248 -Wunreachable-code
 348 -Wgnu-binary-literal
 443 -Wshadow
 534 -Wmissing-variable-declarations
 563 -Wshift-sign-overflow
 613 -Wembedded-directive
 620 -Wgnu-zero-variadic-macro-arguments
 742 -Wswitch-enum
 843 -Wdocumentation
 897 -Wgnu-case-range
1292 -Wassign-enum
1621 -Wgnu-empty-struct
1700 -Wextra-semi
1779 -Wpointer-arith
1847 -Wbad-function-cast
2176 -Wdocumentation-unknown-command
2221 -Wmissing-field-initializers
3101 -Wsign-compare
3238 -Wunused-macros
3559 -Wcast-align
4528 -Wcast-qual
7066 -Wgnu-statement-expression
7651 -Wnull-pointer-subtraction
7995 -Wimplicit-int-conversion
8854 -Wpadded
9737 -Wshorten-64-to-32
10596 -Wgnu-empty-initializer
13274 -Wlanguage-extension-token
13899 -Wunused-parameter
15642 -Wused-but-marked-unused
18669 -Wpedantic
44737 -Wsign-conversion

Re: [PATCH for 7.2?] target/i386: Remove compilation errors when -Werror=maybe-uninitialized

[Stefan Weil via]

Am 07.12.22 um 19:22 schrieb Eric Auger:


On 12/7/22 17:55, Philippe Mathieu-Daudé wrote:

On 7/12/22 15:33, Eric Auger wrote:

On 12/7/22 15:09, Stefan Hajnoczi wrote:

On Wed, 7 Dec 2022 at 08:31, Eric Auger  wrote:

On 12/7/22 14:24, Eric Auger wrote:

Initialize r0-3 to avoid compilation errors when
-Werror=maybe-uninitialized is used

../target/i386/ops_sse.h: In function ‘helper_vpermdq_ymm’:
../target/i386/ops_sse.h:2495:13: error: ‘r3’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
   2495 | d->Q(3) = r3;
    | ^~~~
../target/i386/ops_sse.h:2494:13: error: ‘r2’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
   2494 | d->Q(2) = r2;
    | ^~~~
../target/i386/ops_sse.h:2493:13: error: ‘r1’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
   2493 | d->Q(1) = r1;
    | ^~~~
../target/i386/ops_sse.h:2492:13: error: ‘r0’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
   2492 | d->Q(0) = r0;
    | ^~~~

Signed-off-by: Eric Auger 
Fixes: 790684776861 ("target/i386: reimplement 0x0f 0x3a, add AVX")

---

Am I the only one getting this? Or anything wrong in my setup.


Hi Eric,

no, you are not the only one. I regularly build with higher warning 
levels, for example with -Weverything on macOS, and get a much longer 
list which includes the mentioned warnings (see below).


The warnings for ops_sse.h are false positives, so I think no fix is 
needed for 7.2. The compiler is not clever enough to see that the switch 
statements handle all possible cases. It should be sufficient to replace 
`case 3` by `default` to help the compiler and fix the warning. Your fix 
might produce new compiler warnings because setting the variables to 0 
has no effect.


Cheers
Stefan

../block/mirror.c:1024:13: warning: variable 'iostatus' may be 
uninitialized when used here [-Wconditional-uninitialized]
../block/mirror.c:1498:20: warning: variable 'bounce_buf' may be 
uninitialized when used here [-Wconditional-uninitialized]
../block/nbd.c:1208:24: warning: variable 'request_ret' may be 
uninitialized when used here [-Wconditional-uninitialized]
../block/nbd.c:1266:24: warning: variable 'request_ret' may be 
uninitialized when used here [-Wconditional-uninitialized]
../block/nbd.c:1424:20: warning: variable 'request_ret' may be 
uninitialized when used here [-Wconditional-uninitialized]
../block/qcow2-snapshot.c:423:51: warning: variable 'snapshots_size' may 
be uninitialized when used here [-Wconditional-uninitialized]
../block/qcow2.c:3236:23: warning: variable 'cur_bytes' may be 
uninitialized when used here [-Wconditional-uninitialized]
../block/ssh.c:306:52: warning: variable 'server_hash_len' may be 
uninitialized when used here [-Wconditional-uninitialized]
../block/ssh.c:313:45: warning: variable 'pubkey_type' may be 
uninitialized when used here [-Wconditional-uninitialized]
../contrib/elf2dmp/main.c:138:17: warning: variable 'kwn' may be 
uninitialized when used here [-Wconditional-uninitialized]
../contrib/elf2dmp/main.c:138:22: warning: variable 'kwa' may be 
uninitialized when used here [-Wconditional-uninitialized]
../contrib/elf2dmp/main.c:138:27: warning: variable 
'KdpDataBlockEncoded' may be uninitialized when used here 
[-Wconditional-uninitialized]
../crypto/block-luks.c:844:29: warning: variable 'splitkeylen' may be 
uninitialized when used here [-Wconditional-uninitialized]
../disas/m68k.c:1513:47: warning: variable 'flval' may be uninitialized 
when used here [-Wconditional-uninitialized]
../dump/win_dump.c:105:18: warning: variable 'ptr64' may be 
uninitialized when used here [-Wconditional-uninitialized]
../dump/win_dump.c:105:26: warning: variable 'ptr32' may be 
uninitialized when used here [-Wconditional-uninitialized]
../gdbstub/gdbstub.c:1191:39: warning: variable 'pid' may be 
uninitialized when used here [-Wconditional-uninitialized]
../gdbstub/gdbstub.c:1209:36: warning: variable 'tid' may be 
uninitialized when used here [-Wconditional-uninitialized]
../hw/9pfs/9p.c:1911:13: warning: variable 'fidst' may be uninitialized 
when used here [-Wconditional-uninitialized]
../hw/block/block.c:110:33: warning: variable 'bs' may be uninitialized 
when used here [-Wconditional-uninitialized]
../hw/core/generic-loader.c:160:23: warning: variable 'entry' may be 
uninitialized when used here [-Wconditional-uninitialized]
../hw/i386/intel_iommu.c:323:12: warning: variable 'entry' may be 
uninitialized when used here [-Wconditional-uninitialized]
../hw/ide/ahci.c:968:60: warning: variable 'tbl_entry_size' may be 
uninitialized when used here [-Wconditional-uninitialized]
../hw/microblaze/boot.c:107:42: warning: variable 'fdt_size' may be 
uninitialized when used here [-Wconditional-uninitialized]
../hw/net/rtl8139.c:1801:20: warning: variable 'buf2' may be 
uninitialized when used here [-Wconditional-uninitialized]
../hw/nios2/boo

Re: [PATCH v3 for-7.2 0/6] Add format attributes and fix format strings

[Stefan Weil via]

Am 27.11.22 um 19:23 schrieb Stefan Hajnoczi:


We need to wait for Michael to agree to maintainership in patch 5. If
we run out of time I suggest splitting out patch 5.

Reviewed-by: Stefan Hajnoczi



Citing Michael from a v2 email: "pls do".

Stefan



Hello Michael,

I just noticed that MAINTAINERS has no entry for the files in
subprojects/libvhost-user, so I did not cc you in my previous e-mails.
Should that directory be added to the "vhost" section"?

Stefan


   pls do

Re: [PATCH v3 for-7.2 4/6] libvhost-user: Add format attribute to local function vu_panic

[Stefan Weil via]

Am 27.11.22 um 19:14 schrieb Stefan Hajnoczi:


On Sat, 26 Nov 2022 at 10:25, Stefan Weil  wrote:

Signed-off-by: Stefan Weil 
Reviewed-by: Marc-André Lureau 
Message-Id: <20220422070144.1043697-4...@weilnetz.de>
Signed-off-by: Laurent Vivier 
---
  subprojects/libvhost-user/libvhost-user.c | 13 -
  1 file changed, 12 insertions(+), 1 deletion(-)

I would rather not merge something that can cause new build failures
this late in the release cycle.

If you respin, please make this a separate 8.0 patch.



It would only cause a build failure if there remained more format bugs, 
but the last ones were fixed in patch 3. I tested a build for 32 bit ARM 
(which previously failed without patch 3), and it works now.


But you are right, patch 4 is not release critical as it is not a bug 
fix (like the other ones), so not including it in 7.2 might be an option.


Stefan

[PATCH v3 for-7.2 4/6] libvhost-user: Add format attribute to local function vu_panic

[Stefan Weil via]

Signed-off-by: Stefan Weil 
Reviewed-by: Marc-André Lureau 
Message-Id: <20220422070144.1043697-4...@weilnetz.de>
Signed-off-by: Laurent Vivier 
---
 subprojects/libvhost-user/libvhost-user.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c 
b/subprojects/libvhost-user/libvhost-user.c
index 80f9952e71..d6ee6e7d91 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -45,6 +45,17 @@
 #include "libvhost-user.h"
 
 /* usually provided by GLib */
+#if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ > 4)
+#if !defined(__clang__) && (__GNUC__ == 4 && __GNUC_MINOR__ == 4)
+#define G_GNUC_PRINTF(format_idx, arg_idx) \
+  __attribute__((__format__(gnu_printf, format_idx, arg_idx)))
+#else
+#define G_GNUC_PRINTF(format_idx, arg_idx) \
+  __attribute__((__format__(__printf__, format_idx, arg_idx)))
+#endif
+#else   /* !__GNUC__ */
+#define G_GNUC_PRINTF(format_idx, arg_idx)
+#endif  /* !__GNUC__ */
 #ifndef MIN
 #define MIN(x, y) ({\
 typeof(x) _min1 = (x);  \
@@ -151,7 +162,7 @@ vu_request_to_string(unsigned int req)
 }
 }
 
-static void
+static void G_GNUC_PRINTF(2, 3)
 vu_panic(VuDev *dev, const char *msg, ...)
 {
 char *buf = NULL;
-- 
2.35.1

[PATCH v3 for-7.2 5/6] MAINTAINERS: Add subprojects/libvhost-user to section "vhost"

[Stefan Weil via]

Signed-off-by: Stefan Weil 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index cf24910249..6966490c94 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2005,6 +2005,7 @@ F: docs/interop/vhost-user.rst
 F: contrib/vhost-user-*/
 F: backends/vhost-user.c
 F: include/sysemu/vhost-user-backend.h
+F: subprojects/libvhost-user/
 
 virtio
 M: Michael S. Tsirkin 
-- 
2.35.1

[PATCH v3 for-7.2 6/6] Add G_GNUC_PRINTF to function qemu_set_info_str and fix related issues

[Stefan Weil via]

With the G_GNUC_PRINTF function attribute the compiler detects
two potential insecure format strings:

../../../net/stream.c:248:31: warning: format string is not a string literal 
(potentially insecure) [-Wformat-security]
qemu_set_info_str(&s->nc, uri);
  ^~~
../../../net/stream.c:322:31: warning: format string is not a string literal 
(potentially insecure) [-Wformat-security]
qemu_set_info_str(&s->nc, uri);
  ^~~

There are also two other warnings:

../../../net/socket.c:182:35: warning: zero-length gnu_printf format string 
[-Wformat-zero-length]
  182 | qemu_set_info_str(&s->nc, "");
  |   ^~
../../../net/stream.c:170:35: warning: zero-length gnu_printf format string 
[-Wformat-zero-length]
  170 | qemu_set_info_str(&s->nc, "");

Reviewed-by: Peter Maydell 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Stefan Weil 
---
 include/net/net.h | 3 ++-
 net/socket.c  | 2 +-
 net/stream.c  | 6 +++---
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index 3db75ff841..dc20b31e9f 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -177,7 +177,8 @@ ssize_t qemu_send_packet_async(NetClientState *nc, const 
uint8_t *buf,
 void qemu_purge_queued_packets(NetClientState *nc);
 void qemu_flush_queued_packets(NetClientState *nc);
 void qemu_flush_or_purge_queued_packets(NetClientState *nc, bool purge);
-void qemu_set_info_str(NetClientState *nc, const char *fmt, ...);
+void qemu_set_info_str(NetClientState *nc,
+   const char *fmt, ...) G_GNUC_PRINTF(2, 3);
 void qemu_format_nic_info_str(NetClientState *nc, uint8_t macaddr[6]);
 bool qemu_has_ufo(NetClientState *nc);
 bool qemu_has_vnet_hdr(NetClientState *nc);
diff --git a/net/socket.c b/net/socket.c
index 4944bb70d5..e62137c839 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -179,7 +179,7 @@ static void net_socket_send(void *opaque)
 s->fd = -1;
 net_socket_rs_init(&s->rs, net_socket_rs_finalize, false);
 s->nc.link_down = true;
-qemu_set_info_str(&s->nc, "");
+qemu_set_info_str(&s->nc, "%s", "");
 
 return;
 }
diff --git a/net/stream.c b/net/stream.c
index 53b7040cc4..37ff727e0c 100644
--- a/net/stream.c
+++ b/net/stream.c
@@ -167,7 +167,7 @@ static gboolean net_stream_send(QIOChannel *ioc,
 
 net_socket_rs_init(&s->rs, net_stream_rs_finalize, false);
 s->nc.link_down = true;
-qemu_set_info_str(&s->nc, "");
+qemu_set_info_str(&s->nc, "%s", "");
 
 qapi_event_send_netdev_stream_disconnected(s->nc.name);
 
@@ -245,7 +245,7 @@ static void net_stream_listen(QIONetListener *listener,
 }
 g_assert(addr != NULL);
 uri = socket_uri(addr);
-qemu_set_info_str(&s->nc, uri);
+qemu_set_info_str(&s->nc, "%s", uri);
 g_free(uri);
 qapi_event_send_netdev_stream_connected(s->nc.name, addr);
 qapi_free_SocketAddress(addr);
@@ -319,7 +319,7 @@ static void net_stream_client_connected(QIOTask *task, 
gpointer opaque)
 addr = qio_channel_socket_get_remote_address(sioc, NULL);
 g_assert(addr != NULL);
 uri = socket_uri(addr);
-qemu_set_info_str(&s->nc, uri);
+qemu_set_info_str(&s->nc, "%s", uri);
 g_free(uri);
 
 ret = qemu_socket_try_set_nonblock(sioc->fd);
-- 
2.35.1

[PATCH v3 for-7.2 3/6] libvhost-user: Fix two more format strings

[Stefan Weil via]

This fix is required for 32 bit hosts. The bug was detected by CI
for arm-linux, but is also relevant for i386-linux.

Reported-by: Stefan Hajnoczi 
Signed-off-by: Stefan Weil 
---
 subprojects/libvhost-user/libvhost-user.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c 
b/subprojects/libvhost-user/libvhost-user.c
index d67953a1c3..80f9952e71 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -651,7 +651,8 @@ generate_faults(VuDev *dev) {
 
 if (ioctl(dev->postcopy_ufd, UFFDIO_REGISTER, ®_struct)) {
 vu_panic(dev, "%s: Failed to userfault region %d "
-  "@%" PRIx64 " + size:%zx offset: %zx: (ufd=%d)%s\n",
+  "@%" PRIx64 " + size:%" PRIx64 " offset: %" PRIx64
+  ": (ufd=%d)%s\n",
  __func__, i,
  dev_region->mmap_addr,
  dev_region->size, dev_region->mmap_offset,
-- 
2.35.1

[PATCH v3 for-7.2 0/6] Add format attributes and fix format strings

[Stefan Weil via]

v3:
- Fix description for patch 3
- Add patches 5 and 6

The patches 3 and 5 still need reviews!

[PATCH v3 for-7.2 1/6] libvhost-user: Fix wrong type of argument to
[PATCH v3 for-7.2 2/6] libvhost-user: Fix format strings
[PATCH v3 for-7.2 3/6] libvhost-user: Fix two more format strings
[PATCH v3 for-7.2 4/6] libvhost-user: Add format attribute to local
[PATCH v3 for-7.2 5/6] MAINTAINERS: Add subprojects/libvhost-user to
[PATCH v3 for-7.2 6/6] Add G_GNUC_PRINTF to function

[PATCH v3 for-7.2 2/6] libvhost-user: Fix format strings

[Stefan Weil via]

Signed-off-by: Stefan Weil 
Reviewed-by: Marc-André Lureau 
Message-Id: <20220422070144.1043697-3...@weilnetz.de>
Signed-off-by: Laurent Vivier 
---
 subprojects/libvhost-user/libvhost-user.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c 
b/subprojects/libvhost-user/libvhost-user.c
index d9a6e3e556..d67953a1c3 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -700,7 +700,7 @@ vu_add_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
 if (vmsg->size < VHOST_USER_MEM_REG_SIZE) {
 close(vmsg->fds[0]);
 vu_panic(dev, "VHOST_USER_ADD_MEM_REG requires a message size of at "
-  "least %d bytes and only %d bytes were received",
+  "least %zu bytes and only %d bytes were received",
   VHOST_USER_MEM_REG_SIZE, vmsg->size);
 return false;
 }
@@ -826,7 +826,7 @@ vu_rem_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
 if (vmsg->size < VHOST_USER_MEM_REG_SIZE) {
 vmsg_close_fds(vmsg);
 vu_panic(dev, "VHOST_USER_REM_MEM_REG requires a message size of at "
-  "least %d bytes and only %d bytes were received",
+  "least %zu bytes and only %d bytes were received",
   VHOST_USER_MEM_REG_SIZE, vmsg->size);
 return false;
 }
-- 
2.35.1

[PATCH v3 for-7.2 1/6] libvhost-user: Fix wrong type of argument to formatting function (reported by LGTM)

[Stefan Weil via]

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Stefan Weil 
Message-Id: <20220422070144.1043697-2...@weilnetz.de>
Signed-off-by: Laurent Vivier 
---
 subprojects/libvhost-user/libvhost-user.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c 
b/subprojects/libvhost-user/libvhost-user.c
index ffed4729a3..d9a6e3e556 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -651,7 +651,7 @@ generate_faults(VuDev *dev) {
 
 if (ioctl(dev->postcopy_ufd, UFFDIO_REGISTER, ®_struct)) {
 vu_panic(dev, "%s: Failed to userfault region %d "
-  "@%p + size:%zx offset: %zx: (ufd=%d)%s\n",
+  "@%" PRIx64 " + size:%zx offset: %zx: (ufd=%d)%s\n",
  __func__, i,
  dev_region->mmap_addr,
  dev_region->size, dev_region->mmap_offset,
-- 
2.35.1

Re: [PATCH for 7.2-rc? v2 0/5] continuing efforts to fix vhost-user issues

[Stefan Weil via]

Am 25.11.22 um 18:30 schrieb Alex Bennée:

Hi,

This is continuing to attempt to fix the various vhost-user issues
that are currently plaguing the release. One concrete bug I've come
across is that all qtest MMIO devices where being treated as legacy
which caused the VIRTIO_F_VERSION_1 flag to get missed causing s390x
to fall back to trying to set the endian value for the virt-queues.


Do you want to add my 4 commits which fix format strings for 
libvhost-user to your series, or should they be handled separately?


Regards
Stefan

Re: [PATCH for-7.2 v2 3/4] libvhost-user: Fix two more format strings

[Stefan Weil via]

Am 23.11.22 um 07:35 schrieb Stefan Weil:

Am 15.11.22 um 08:25 schrieb Stefan Weil:

Am 05.11.22 um 11:24 schrieb Stefan Weil:


This fix is required for 32 bit host. The bug was detected by CI
for arm-linux, but is also relevant for i386-linux.

Reported-by: Stefan Hajnoczi 
Signed-off-by: Stefan Weil 
---
  subprojects/libvhost-user/libvhost-user.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c 
b/subprojects/libvhost-user/libvhost-user.c

index d67953a1c3..80f9952e71 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -651,7 +651,8 @@ generate_faults(VuDev *dev) {
  if (ioctl(dev->postcopy_ufd, UFFDIO_REGISTER, ®_struct)) {
  vu_panic(dev, "%s: Failed to userfault region %d "
-  "@%" PRIx64 " + size:%zx offset: %zx: 
(ufd=%d)%s\n",
+  "@%" PRIx64 " + size:%" PRIx64 " offset: 
%" PRIx64

+  ": (ufd=%d)%s\n",
   __func__, i,
   dev_region->mmap_addr,
   dev_region->size, dev_region->mmap_offset,



There is still no review for this patch.

I added "for-7.2" to the subject here in my answer. How can we get all 
4 patches of this series into the new release?


Stefan


Ping? Those bug fixes are still missing in -rc2.

Stefan


Hello Michael,

I just noticed that MAINTAINERS has no entry for the files in 
subprojects/libvhost-user, so I did not cc you in my previous e-mails. 
Should that directory be added to the "vhost" section"?


Stefan

Re: [PATCH for-7.2 v2 3/4] libvhost-user: Fix two more format strings

[Stefan Weil via]

Am 15.11.22 um 08:25 schrieb Stefan Weil:

Am 05.11.22 um 11:24 schrieb Stefan Weil:


This fix is required for 32 bit host. The bug was detected by CI
for arm-linux, but is also relevant for i386-linux.

Reported-by: Stefan Hajnoczi 
Signed-off-by: Stefan Weil 
---
  subprojects/libvhost-user/libvhost-user.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c 
b/subprojects/libvhost-user/libvhost-user.c

index d67953a1c3..80f9952e71 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -651,7 +651,8 @@ generate_faults(VuDev *dev) {
  if (ioctl(dev->postcopy_ufd, UFFDIO_REGISTER, ®_struct)) {
  vu_panic(dev, "%s: Failed to userfault region %d "
-  "@%" PRIx64 " + size:%zx offset: %zx: 
(ufd=%d)%s\n",
+  "@%" PRIx64 " + size:%" PRIx64 " offset: %" 
PRIx64

+  ": (ufd=%d)%s\n",
   __func__, i,
   dev_region->mmap_addr,
   dev_region->size, dev_region->mmap_offset,



There is still no review for this patch.

I added "for-7.2" to the subject here in my answer. How can we get all 4 
patches of this series into the new release?


Stefan


Ping? Those bug fixes are still missing in -rc2.

Stefan


OpenPGP_0xE08C21D5677450AD.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH for-7.2 v2 3/4] libvhost-user: Fix two more format strings

[Stefan Weil via]

Am 05.11.22 um 11:24 schrieb Stefan Weil:


This fix is required for 32 bit host. The bug was detected by CI
for arm-linux, but is also relevant for i386-linux.

Reported-by: Stefan Hajnoczi 
Signed-off-by: Stefan Weil 
---
  subprojects/libvhost-user/libvhost-user.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c 
b/subprojects/libvhost-user/libvhost-user.c
index d67953a1c3..80f9952e71 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -651,7 +651,8 @@ generate_faults(VuDev *dev) {
  
  if (ioctl(dev->postcopy_ufd, UFFDIO_REGISTER, ®_struct)) {

  vu_panic(dev, "%s: Failed to userfault region %d "
-  "@%" PRIx64 " + size:%zx offset: %zx: (ufd=%d)%s\n",
+  "@%" PRIx64 " + size:%" PRIx64 " offset: %" PRIx64
+  ": (ufd=%d)%s\n",
   __func__, i,
   dev_region->mmap_addr,
   dev_region->size, dev_region->mmap_offset,



There is still no review for this patch.

I added "for-7.2" to the subject here in my answer. How can we get all 4 
patches of this series into the new release?


Stefan

[PATCH for-7.2] Add G_GNUC_PRINTF to function qemu_set_info_str and fix related issues

[Stefan Weil via]

With the G_GNUC_PRINTF function attribute the compiler detects
two potential insecure format strings:

../../../net/stream.c:248:31: warning: format string is not a string literal 
(potentially insecure) [-Wformat-security]
qemu_set_info_str(&s->nc, uri);
  ^~~
../../../net/stream.c:322:31: warning: format string is not a string literal 
(potentially insecure) [-Wformat-security]
qemu_set_info_str(&s->nc, uri);
  ^~~

There are also two other warnings:

../../../net/socket.c:182:35: warning: zero-length gnu_printf format string 
[-Wformat-zero-length]
  182 | qemu_set_info_str(&s->nc, "");
  |   ^~
../../../net/stream.c:170:35: warning: zero-length gnu_printf format string 
[-Wformat-zero-length]
  170 | qemu_set_info_str(&s->nc, "");

Signed-off-by: Stefan Weil 
---
 include/net/net.h | 3 ++-
 net/socket.c  | 2 +-
 net/stream.c  | 6 +++---
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index 3db75ff841..dc20b31e9f 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -177,7 +177,8 @@ ssize_t qemu_send_packet_async(NetClientState *nc, const 
uint8_t *buf,
 void qemu_purge_queued_packets(NetClientState *nc);
 void qemu_flush_queued_packets(NetClientState *nc);
 void qemu_flush_or_purge_queued_packets(NetClientState *nc, bool purge);
-void qemu_set_info_str(NetClientState *nc, const char *fmt, ...);
+void qemu_set_info_str(NetClientState *nc,
+   const char *fmt, ...) G_GNUC_PRINTF(2, 3);
 void qemu_format_nic_info_str(NetClientState *nc, uint8_t macaddr[6]);
 bool qemu_has_ufo(NetClientState *nc);
 bool qemu_has_vnet_hdr(NetClientState *nc);
diff --git a/net/socket.c b/net/socket.c
index 4944bb70d5..e62137c839 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -179,7 +179,7 @@ static void net_socket_send(void *opaque)
 s->fd = -1;
 net_socket_rs_init(&s->rs, net_socket_rs_finalize, false);
 s->nc.link_down = true;
-qemu_set_info_str(&s->nc, "");
+qemu_set_info_str(&s->nc, "%s", "");
 
 return;
 }
diff --git a/net/stream.c b/net/stream.c
index 53b7040cc4..37ff727e0c 100644
--- a/net/stream.c
+++ b/net/stream.c
@@ -167,7 +167,7 @@ static gboolean net_stream_send(QIOChannel *ioc,
 
 net_socket_rs_init(&s->rs, net_stream_rs_finalize, false);
 s->nc.link_down = true;
-qemu_set_info_str(&s->nc, "");
+qemu_set_info_str(&s->nc, "%s", "");
 
 qapi_event_send_netdev_stream_disconnected(s->nc.name);
 
@@ -245,7 +245,7 @@ static void net_stream_listen(QIONetListener *listener,
 }
 g_assert(addr != NULL);
 uri = socket_uri(addr);
-qemu_set_info_str(&s->nc, uri);
+qemu_set_info_str(&s->nc, "%s", uri);
 g_free(uri);
 qapi_event_send_netdev_stream_connected(s->nc.name, addr);
 qapi_free_SocketAddress(addr);
@@ -319,7 +319,7 @@ static void net_stream_client_connected(QIOTask *task, 
gpointer opaque)
 addr = qio_channel_socket_get_remote_address(sioc, NULL);
 g_assert(addr != NULL);
 uri = socket_uri(addr);
-qemu_set_info_str(&s->nc, uri);
+qemu_set_info_str(&s->nc, "%s", uri);
 g_free(uri);
 
 ret = qemu_socket_try_set_nonblock(sioc->fd);
-- 
2.30.2

Re: [PATCH] s390x: Fix spelling errors

[Stefan Weil via]

Am 11.11.22 um 19:28 schrieb Thomas Huth:


Fix typos (discovered with the 'codespell' utility).

Signed-off-by: Thomas Huth 
---
  hw/s390x/ipl.h  | 2 +-
  pc-bios/s390-ccw/cio.h  | 2 +-
  pc-bios/s390-ccw/iplb.h | 2 +-
  target/s390x/cpu_models.h   | 4 ++--
  hw/s390x/s390-pci-vfio.c| 2 +-
  hw/s390x/s390-virtio-ccw.c  | 6 +++---
  target/s390x/ioinst.c   | 2 +-
  target/s390x/tcg/excp_helper.c  | 2 +-
  target/s390x/tcg/fpu_helper.c   | 2 +-
  target/s390x/tcg/misc_helper.c  | 2 +-
  target/s390x/tcg/translate.c| 4 ++--
  target/s390x/tcg/translate_vx.c.inc | 6 +++---
  pc-bios/s390-ccw/start.S| 2 +-
  13 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
index dfc6dfd89c..7fc86e7905 100644
--- a/hw/s390x/ipl.h
+++ b/hw/s390x/ipl.h
@@ -140,7 +140,7 @@ void s390_ipl_clear_reset_request(void);
   * have an offset of 4 + n * 8 bytes within the struct in order
   * to keep it double-word aligned.
   * The total size of the struct must never exceed 28 bytes.
- * This definition must be kept in sync with the defininition
+ * This definition must be kept in sync with the definition
   * in pc-bios/s390-ccw/iplb.h.
   */
  struct QemuIplParameters {
diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h
index 1e5d4e92e1..88a88adfd2 100644
--- a/pc-bios/s390-ccw/cio.h
+++ b/pc-bios/s390-ccw/cio.h
@@ -20,7 +20,7 @@ struct pmcw {
  __u32 intparm;  /* interruption parameter */
  __u32 qf:1; /* qdio facility */
  __u32 w:1;
-__u32 isc:3;/* interruption sublass */
+__u32 isc:3;/* interruption subclass */
  __u32 res5:3;   /* reserved zeros */
  __u32 ena:1;/* enabled */
  __u32 lm:2; /* limit mode */
diff --git a/pc-bios/s390-ccw/iplb.h b/pc-bios/s390-ccw/iplb.h
index 772d5c57c9..cb6ac8a880 100644
--- a/pc-bios/s390-ccw/iplb.h
+++ b/pc-bios/s390-ccw/iplb.h
@@ -81,7 +81,7 @@ extern IplParameterBlock iplb 
__attribute__((__aligned__(PAGE_SIZE)));
  #define QIPL_FLAG_BM_OPTS_ZIPL  0x40
  
  /*

- * This definition must be kept in sync with the defininition
+ * This definition must be kept in sync with the definition
   * in hw/s390x/ipl.h
   */
  struct QemuIplParameters {
diff --git a/target/s390x/cpu_models.h b/target/s390x/cpu_models.h
index 74d1f87e4f..15c0f0dcfe 100644
--- a/target/s390x/cpu_models.h
+++ b/target/s390x/cpu_models.h
@@ -24,13 +24,13 @@ struct S390CPUDef {
  uint8_t gen;/* hw generation identification */
  uint16_t type;  /* cpu type identification */
  uint8_t ec_ga;  /* EC GA version (on which also the BC is based) 
*/
-uint8_t mha_pow;/* Maximum Host Adress Power, mha = 2^pow-1 */
+uint8_t mha_pow;/* Maximum Host Address Power, mha = 2^pow-1 */



This comment could use lower case words.



  uint32_t hmfai; /* hypervisor-managed facilities */
  /* base/min features, must never be changed between QEMU versions */
  S390FeatBitmap base_feat;
  /* used to init base_feat from generated data */
  S390FeatInit base_init;
-/* deafault features, QEMU version specific */
+/* default features, QEMU version specific */
  S390FeatBitmap default_feat;
  /* used to init default_feat from generated data */
  S390FeatInit default_init;
diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c
index 2aefa508a0..5f0adb0b4a 100644
--- a/hw/s390x/s390-pci-vfio.c
+++ b/hw/s390x/s390-pci-vfio.c
@@ -313,7 +313,7 @@ retry:
  /*
   * Get the host function handle from the vfio CLP capabilities chain.  Returns
   * true if a fh value was placed into the provided buffer.  Returns false
- * if a fh could not be obtained (ioctl failed or capabilitiy version does
+ * if a fh could not be obtained (ioctl failed or capability version does
   * not include the fh)
   */
  bool s390_pci_get_host_fh(S390PCIBusDevice *pbdev, uint32_t *fh)
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 7d80bc1837..2e64ffab45 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -354,7 +354,7 @@ static int s390_machine_protect(S390CcwMachineState *ms)
  }
  
  error_setg(&pv_mig_blocker,

-   "protected VMs are currently not migrateable.");
+   "protected VMs are currently not migratable.");



This might again be a different British / American spelling.



  rc = migrate_add_blocker(pv_mig_blocker, &local_err);
  if (rc) {
  ram_block_discard_disable(false);
@@ -449,7 +449,7 @@ static void s390_machine_reset(MachineState *machine, 
ShutdownCause reason)
  break;
  case S390_RESET_MODIFIED_CLEAR:
  /*
- * Susbsystem reset needs to be done before we unshare memory
+ * Subsystem reset needs to be done before we unshare memory
   * and lose access to VIRTI

[RFC: PATCH v2] Add new build targets 'check-spelling' and 'check-spelling-docs'

[Stefan Weil via]

`make check-spelling` can now be used to get a list of spelling errors.
It uses the latest version of codespell, a spell checker implemented in Python.

'make check-spelling-docs' checks the generated documentation.

Signed-off-by: Stefan Weil 
---

v2: Additional target check-spelling-docs, updated ignore-words

This RFC can already be used for manual tests, but still reports false
positives, mostly because some variable names are interpreted as words.
These words can either be ignored in the check, or in some cases the code
might be changed to use different variable names.

The check currently only skips a few directories and files, so for example
checked out submodules are also checked.

The rule can be extended to allow user provided ignore and skip lists,
for example by introducing Makefile variables CODESPELL_SKIP=userfile
or CODESPELL_IGNORE=userfile. A limited check could be implemented by
providing a base directory CODESPELL_START=basedirectory, for example
CODESPELL_START=docs.

After fixing some typos (patch was just sent to the list), these
"typos" remain in the generated documentation:

SUMMARY:
crypted   3
ede   4
inflight 32
informations  3
mor   1
vas   1

Regards,
Stefan

 tests/Makefile.include   | 19 +++
 tests/codespell/README.rst   | 18 ++
 tests/codespell/exclude-file |  3 +++
 tests/codespell/ignore-words | 21 +
 tests/requirements.txt   |  1 +
 5 files changed, 62 insertions(+)
 create mode 100644 tests/codespell/README.rst
 create mode 100644 tests/codespell/exclude-file
 create mode 100644 tests/codespell/ignore-words

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 9422ddaece..66424c2eac 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -155,6 +155,25 @@ check-acceptance-deprecated-warning:
 
 check-acceptance: check-acceptance-deprecated-warning | check-avocado
 
+CODESPELL_DIR=$(SRC_PATH)/tests/codespell
+
+.PHONY: check-spelling
+check-spelling: check-venv
+   source $(TESTS_VENV_DIR)/bin/activate && \
+   cd "$(SRC_PATH)" && \
+   codespell -s . \
+ --exclude-file=$(CODESPELL_DIR)/exclude-file \
+ --ignore-words=$(CODESPELL_DIR)/ignore-words \
+ --skip="./.git,./bin,./build,./linux-headers,./roms,*.patch,nohup.out"
+
+.PHONY: check-spelling-docs
+check-spelling-docs: check-venv
+   source $(TESTS_VENV_DIR)/bin/activate && \
+   codespell -s docs \
+ --exclude-file=$(CODESPELL_DIR)/exclude-file \
+ --ignore-words=$(CODESPELL_DIR)/ignore-words \
+ --skip="docs/manual/_static,docs/manual/searchindex.js,nohup.out"
+
 # Consolidated targets
 
 .PHONY: check check-clean get-vm-images
diff --git a/tests/codespell/README.rst b/tests/codespell/README.rst
new file mode 100644
index 00..67e070d631
--- /dev/null
+++ b/tests/codespell/README.rst
@@ -0,0 +1,18 @@
+=
+Check spelling with codespell
+=
+
+`make check-spelling` can be used to get a list of spelling errors.
+It reports files with spelling errors and a summary of all misspelled words.
+The report is generated by the latest version of codespell, a spell checker
+implemented in Python.
+
+See https://github.com/codespell-project/codespell for more information.
+
+Some file patterns are excluded from the check.
+
+In addition tests/codespell includes several files which are used to
+suppress certain false positives in the codespell report.
+
+exclude-file - complete lines which should be ignored
+ignore-words - list of words which should be ignored
diff --git a/tests/codespell/exclude-file b/tests/codespell/exclude-file
new file mode 100644
index 00..57de81a4eb
--- /dev/null
+++ b/tests/codespell/exclude-file
@@ -0,0 +1,3 @@
+ * VAS controller.
+number generator daemon such as the one found in the vhost-device crate of
+introspection.  The latter can conceivably confuse clients, so tread
diff --git a/tests/codespell/ignore-words b/tests/codespell/ignore-words
new file mode 100644
index 00..0deb4cc65f
--- /dev/null
+++ b/tests/codespell/ignore-words
@@ -0,0 +1,21 @@
+asign
+bu
+buid
+busses
+conectix
+dout
+falt
+fpr
+hace
+hax
+hda
+nd
+ot
+pard
+parm
+ptd
+ser
+som
+synopsys
+te
+ue
diff --git a/tests/requirements.txt b/tests/requirements.txt
index 0ba561b6bd..dd44e6768f 100644
--- a/tests/requirements.txt
+++ b/tests/requirements.txt
@@ -4,3 +4,4 @@
 # Note that qemu.git/python/ is always implicitly installed.
 avocado-framework==88.1
 pycdlib==1.11.0
+codespell
-- 
2.30.2

[PATCH for-7.2] Fix several typos in documentation (found by codespell)

[Stefan Weil via]

Those typos are in files which are used to generate the QEMU manual.

Signed-off-by: Stefan Weil 
---

I did not fix memory_region_init_resizeable_ram. That might be done after 7.2.

Stefan

 docs/devel/acpi-bits.rst   | 2 +-
 docs/system/devices/can.rst| 2 +-
 hw/scsi/esp.c  | 6 +++---
 include/exec/memory.h  | 6 +++---
 qapi/virtio.json   | 4 ++--
 qemu-options.hx| 6 +++---
 tests/qtest/libqos/qgraph.h| 2 +-
 tests/qtest/libqos/virtio-9p.c | 2 +-
 8 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/docs/devel/acpi-bits.rst b/docs/devel/acpi-bits.rst
index c9564d871a..5e22be8ef6 100644
--- a/docs/devel/acpi-bits.rst
+++ b/docs/devel/acpi-bits.rst
@@ -132,7 +132,7 @@ Under ``tests/avocado/`` as the root we have:
 
(a) They are python2.7 based scripts and not python 3 scripts.
(b) They are run from within the bios bits VM and is not subjected to QEMU
-   build/test python script maintainance and dependency resolutions.
+   build/test python script maintenance and dependency resolutions.
(c) They need not be loaded by avocado framework when running tests.
 
 
diff --git a/docs/system/devices/can.rst b/docs/system/devices/can.rst
index fe37af8223..24b0d4cf41 100644
--- a/docs/system/devices/can.rst
+++ b/docs/system/devices/can.rst
@@ -169,7 +169,7 @@ and with bitrate switch::
 
   cangen can0 -b
 
-The test can be run viceversa, generate messages in the guest system and 
capture them
+The test can be run vice-versa, generate messages in the guest system and 
capture them
 in the host one and much more combinations.
 
 Links to other resources
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index e5b281e836..e52188d022 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -515,7 +515,7 @@ static void do_dma_pdma_cb(ESPState *s)
 } else {
 /*
  * Extra message out bytes received: update cmdfifo_cdb_offset
- * and then switch to commmand phase
+ * and then switch to command phase
  */
 s->cmdfifo_cdb_offset = fifo8_num_used(&s->cmdfifo);
 s->rregs[ESP_RSTAT] = STAT_TC | STAT_CD;
@@ -627,7 +627,7 @@ static void esp_do_dma(ESPState *s)
 } else {
 /*
  * Extra message out bytes received: update cmdfifo_cdb_offset
- * and then switch to commmand phase
+ * and then switch to command phase
  */
 s->cmdfifo_cdb_offset = fifo8_num_used(&s->cmdfifo);
 s->rregs[ESP_RSTAT] = STAT_TC | STAT_CD;
@@ -738,7 +738,7 @@ static void esp_do_nodma(ESPState *s)
 } else {
 /*
  * Extra message out bytes received: update cmdfifo_cdb_offset
- * and then switch to commmand phase
+ * and then switch to command phase
  */
 s->cmdfifo_cdb_offset = fifo8_num_used(&s->cmdfifo);
 s->rregs[ESP_RSTAT] = STAT_TC | STAT_CD;
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 80fa75baa1..91f8a2395a 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -561,7 +561,7 @@ typedef void (*ReplayRamDiscard)(MemoryRegionSection 
*section, void *opaque);
  * A #RamDiscardManager coordinates which parts of specific RAM #MemoryRegion
  * regions are currently populated to be used/accessed by the VM, notifying
  * after parts were discarded (freeing up memory) and before parts will be
- * populated (consuming memory), to be used/acessed by the VM.
+ * populated (consuming memory), to be used/accessed by the VM.
  *
  * A #RamDiscardManager can only be set for a RAM #MemoryRegion while the
  * #MemoryRegion isn't mapped yet; it cannot change while the #MemoryRegion is
@@ -585,7 +585,7 @@ typedef void (*ReplayRamDiscard)(MemoryRegionSection 
*section, void *opaque);
  * Listeners are called in multiples of the minimum granularity (unless it
  * would exceed the registered range) and changes are aligned to the minimum
  * granularity within the #MemoryRegion. Listeners have to prepare for memory
- * becomming discarded in a different granularity than it was populated and the
+ * becoming discarded in a different granularity than it was populated and the
  * other way around.
  */
 struct RamDiscardManagerClass {
@@ -1247,7 +1247,7 @@ void memory_region_init_ram_flags_nomigrate(MemoryRegion 
*mr,
 Error **errp);
 
 /**
- * memory_region_init_resizeable_ram:  Initialize memory region with resizeable
+ * memory_region_init_resizeable_ram:  Initialize memory region with resizable
  * RAM.  Accesses into the region will
  * modify memory directly.  Only an initial
  * portion of this RAM is actually used.
diff --git a/qapi/virtio.json b/qapi/virtio.json
index 872c7e3623..019d2d1987 100644
--- a/qapi/virtio.json
+++ b/qapi/virtio.json
@@

Re: [PATCH trivial for 7.2 1/2] hw/usb/hcd-xhci.c: spelling: tranfer

[Stefan Weil via]

Am 05.11.22 um 22:24 schrieb Michael Tokarev:


05.11.2022 15:23, Stefan Weil via wrote:
..

All typos from this series were also found by codespell.

See https://qemu.weilnetz.de/test/typos7 for many more.
That list was produced with `make check-spelling` from
my previous patch).


Yeah, codespell is a good thing. But qemu has just TOO MANY typos, and
non-typos too (eg addd). I only patched a few places which are visible
in the binaries.

/mjt



More typos which are visible in binaries like firwmare, Changeing, 
Unknow, accomodate, migrateable, facilties, ... can be found with `git 
grep '"' | codespell -s -.


Stefan

Re: [PATCH trivial for 7.2 1/2] hw/usb/hcd-xhci.c: spelling: tranfer

[Stefan Weil via]

Am 05.11.22 um 12:57 schrieb Stefan Weil via:

Am 05.11.22 um 12:48 schrieb Michael Tokarev:

Fixes: effaf5a240e03020f4ae953e10b764622c3e87cc
Signed-off-by: Michael Tokarev 
---
  hw/usb/hcd-xhci.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


All typos from this series were also found by codespell.

See https://qemu.weilnetz.de/test/typos7 for many more.
That list was produced with `make check-spelling` from
my previous patch).

Stefan



OpenPGP_0xE08C21D5677450AD.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH trivial for 7.2] hw/ssi/sifive_spi.c: spelling: reigster

[Stefan Weil via]

Am 05.11.22 um 12:53 schrieb Michael Tokarev:

Fixes: 0694dabe9763847f3010b54ab3ec7d367d2f0ff0
Signed-off-by: Michael Tokarev 
---
  hw/ssi/sifive_spi.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ssi/sifive_spi.c b/hw/ssi/sifive_spi.c
index 03540cf5ca..1b4a401ca1 100644
--- a/hw/ssi/sifive_spi.c
+++ b/hw/ssi/sifive_spi.c
@@ -267,7 +267,7 @@ static void sifive_spi_write(void *opaque, hwaddr addr,
  case R_RXDATA:
  case R_IP:
  qemu_log_mask(LOG_GUEST_ERROR,
-  "%s: invalid write to read-only reigster 0x%"
+  "%s: invalid write to read-only register 0x%"
HWADDR_PRIx " with 0x%x\n", __func__, addr << 2, value);
  break;
  


Reviewed-by: Stefan Weil 


OpenPGP_0xE08C21D5677450AD.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH trivial for 7.2 2/2] hw/virtio/virtio.c: spelling: suppoted

[Stefan Weil via]

Am 05.11.22 um 12:48 schrieb Michael Tokarev:

Fixes: f3034ad71fcd0a6a58bc37830f182b307f089159
Signed-off-by: Michael Tokarev 
---
  hw/virtio/virtio.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 808446b4c9..e76218bdd5 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -340,7 +340,7 @@ qmp_virtio_feature_map_t virtio_net_feature_map[] = {
  qmp_virtio_feature_map_t virtio_scsi_feature_map[] = {
  FEATURE_ENTRY(VIRTIO_SCSI_F_INOUT, \
  "VIRTIO_SCSI_F_INOUT: Requests including read and writable data "
-"buffers suppoted"),
+"buffers supported"),
  FEATURE_ENTRY(VIRTIO_SCSI_F_HOTPLUG, \
  "VIRTIO_SCSI_F_HOTPLUG: Reporting and handling hot-plug events "
  "supported"),


Reviewed-by: Stefan Weil 


OpenPGP_0xE08C21D5677450AD.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH trivial for 7.2 1/2] hw/usb/hcd-xhci.c: spelling: tranfer

[Stefan Weil via]

Am 05.11.22 um 12:48 schrieb Michael Tokarev:

Fixes: effaf5a240e03020f4ae953e10b764622c3e87cc
Signed-off-by: Michael Tokarev 
---
  hw/usb/hcd-xhci.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 8299f35e66..b89b618ec2 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -796,7 +796,7 @@ static int xhci_ring_chain_length(XHCIState *xhci, const 
XHCIRing *ring)
   */
  } while (length < TRB_LINK_LIMIT * 65536 / TRB_SIZE);
  
-qemu_log_mask(LOG_GUEST_ERROR, "%s: exceeded maximum tranfer ring size!\n",

+qemu_log_mask(LOG_GUEST_ERROR, "%s: exceeded maximum transfer ring 
size!\n",
__func__);
  
  return -1;


Reviewed-by: Stefan Weil 


OpenPGP_0xE08C21D5677450AD.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

[PATCH for-7.2] tests/qtest: Fix two format strings

[Stefan Weil via]

Signed-off-by: Stefan Weil 
---
 tests/qtest/migration-test.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index d2eb107f0c..f574331b7b 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -2188,7 +2188,7 @@ static void calc_dirty_rate(QTestState *who, uint64_t 
calc_time)
 qobject_unref(qmp_command(who,
   "{ 'execute': 'calc-dirty-rate',"
   "'arguments': { "
-  "'calc-time': %ld,"
+  "'calc-time': %" PRIu64 ","
   "'mode': 'dirty-ring' }}",
   calc_time));
 }
@@ -2203,7 +2203,7 @@ static void dirtylimit_set_all(QTestState *who, uint64_t 
dirtyrate)
 qobject_unref(qmp_command(who,
   "{ 'execute': 'set-vcpu-dirty-limit',"
   "'arguments': { "
-  "'dirty-rate': %ld } }",
+  "'dirty-rate': %" PRIu64 " } }",
   dirtyrate));
 }
 
-- 
2.30.2

[PATCH for-7.2] accel/tcg: Suppress compiler warning with flag -Wclobbered

[Stefan Weil via]

At least some versions of gcc show a warning when compiler flag -Wclobbered
is used (tested with gcc on Debian bookworm i386 and with cross gcc for
Windows on Debian bullseye).

Signed-off-by: Stefan Weil 
---
 accel/tcg/translate-all.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 921944a5ab..90191d97ec 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -743,6 +743,8 @@ void page_collection_unlock(struct page_collection *set)
 #endif /* !CONFIG_USER_ONLY */
 
 /* Called with mmap_lock held for user mode emulation.  */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wclobbered"
 TranslationBlock *tb_gen_code(CPUState *cpu,
   target_ulong pc, target_ulong cs_base,
   uint32_t flags, int cflags)
@@ -1020,6 +1022,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
 }
 return tb;
 }
+#pragma GCC diagnostic pop
 
 /* user-mode: call with mmap_lock held */
 void tb_check_watchpoint(CPUState *cpu, uintptr_t retaddr)
-- 
2.30.2

Re: [PATCH v2 3/4] libvhost-user: Fix two more format strings

[Stefan Weil via]

Am 05.11.22 um 11:24 schrieb Stefan Weil via:


This fix is required for 32 bit host. The bug was detected by CI
for arm-linux, but is also relevant for i386-linux.



s/host/hosts/

I won't send a v3 for that. Maybe it can be fixed when merging this patch.

Stefan

[PATCH v2 for-7.2 0/4] libvhost-user: Add format attribute and fix format strings

[Stefan Weil via]

v2: Add patch 3 to fix two more format strings before applying patch 4

[PATCH v2 1/4] libvhost-user: Fix wrong type of argument to
[PATCH v2 2/4] libvhost-user: Fix format strings
[PATCH v2 3/4] libvhost-user: Fix two more format strings
[PATCH v2 4/4] libvhost-user: Add format attribute to local function

[PATCH v2 1/4] libvhost-user: Fix wrong type of argument to formatting function (reported by LGTM)

[Stefan Weil via]

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Stefan Weil 
Message-Id: <20220422070144.1043697-2...@weilnetz.de>
Signed-off-by: Laurent Vivier 
---
 subprojects/libvhost-user/libvhost-user.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c 
b/subprojects/libvhost-user/libvhost-user.c
index ffed4729a3..d9a6e3e556 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -651,7 +651,7 @@ generate_faults(VuDev *dev) {
 
 if (ioctl(dev->postcopy_ufd, UFFDIO_REGISTER, ®_struct)) {
 vu_panic(dev, "%s: Failed to userfault region %d "
-  "@%p + size:%zx offset: %zx: (ufd=%d)%s\n",
+  "@%" PRIx64 " + size:%zx offset: %zx: (ufd=%d)%s\n",
  __func__, i,
  dev_region->mmap_addr,
  dev_region->size, dev_region->mmap_offset,
-- 
2.30.2

[PATCH v2 4/4] libvhost-user: Add format attribute to local function vu_panic

[Stefan Weil via]

Signed-off-by: Stefan Weil 
Reviewed-by: Marc-André Lureau 
Message-Id: <20220422070144.1043697-4...@weilnetz.de>
Signed-off-by: Laurent Vivier 
---
 subprojects/libvhost-user/libvhost-user.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c 
b/subprojects/libvhost-user/libvhost-user.c
index 80f9952e71..d6ee6e7d91 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -45,6 +45,17 @@
 #include "libvhost-user.h"
 
 /* usually provided by GLib */
+#if __GNUC__ > 2 || (__GNUC__ == 2 && __GNUC_MINOR__ > 4)
+#if !defined(__clang__) && (__GNUC__ == 4 && __GNUC_MINOR__ == 4)
+#define G_GNUC_PRINTF(format_idx, arg_idx) \
+  __attribute__((__format__(gnu_printf, format_idx, arg_idx)))
+#else
+#define G_GNUC_PRINTF(format_idx, arg_idx) \
+  __attribute__((__format__(__printf__, format_idx, arg_idx)))
+#endif
+#else   /* !__GNUC__ */
+#define G_GNUC_PRINTF(format_idx, arg_idx)
+#endif  /* !__GNUC__ */
 #ifndef MIN
 #define MIN(x, y) ({\
 typeof(x) _min1 = (x);  \
@@ -151,7 +162,7 @@ vu_request_to_string(unsigned int req)
 }
 }
 
-static void
+static void G_GNUC_PRINTF(2, 3)
 vu_panic(VuDev *dev, const char *msg, ...)
 {
 char *buf = NULL;
-- 
2.30.2

[PATCH v2 2/4] libvhost-user: Fix format strings

[Stefan Weil via]

Signed-off-by: Stefan Weil 
Reviewed-by: Marc-André Lureau 
Message-Id: <20220422070144.1043697-3...@weilnetz.de>
Signed-off-by: Laurent Vivier 
---
 subprojects/libvhost-user/libvhost-user.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c 
b/subprojects/libvhost-user/libvhost-user.c
index d9a6e3e556..d67953a1c3 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -700,7 +700,7 @@ vu_add_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
 if (vmsg->size < VHOST_USER_MEM_REG_SIZE) {
 close(vmsg->fds[0]);
 vu_panic(dev, "VHOST_USER_ADD_MEM_REG requires a message size of at "
-  "least %d bytes and only %d bytes were received",
+  "least %zu bytes and only %d bytes were received",
   VHOST_USER_MEM_REG_SIZE, vmsg->size);
 return false;
 }
@@ -826,7 +826,7 @@ vu_rem_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
 if (vmsg->size < VHOST_USER_MEM_REG_SIZE) {
 vmsg_close_fds(vmsg);
 vu_panic(dev, "VHOST_USER_REM_MEM_REG requires a message size of at "
-  "least %d bytes and only %d bytes were received",
+  "least %zu bytes and only %d bytes were received",
   VHOST_USER_MEM_REG_SIZE, vmsg->size);
 return false;
 }
-- 
2.30.2

[PATCH v2 3/4] libvhost-user: Fix two more format strings

[Stefan Weil via]

This fix is required for 32 bit host. The bug was detected by CI
for arm-linux, but is also relevant for i386-linux.

Reported-by: Stefan Hajnoczi 
Signed-off-by: Stefan Weil 
---
 subprojects/libvhost-user/libvhost-user.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c 
b/subprojects/libvhost-user/libvhost-user.c
index d67953a1c3..80f9952e71 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -651,7 +651,8 @@ generate_faults(VuDev *dev) {
 
 if (ioctl(dev->postcopy_ufd, UFFDIO_REGISTER, ®_struct)) {
 vu_panic(dev, "%s: Failed to userfault region %d "
-  "@%" PRIx64 " + size:%zx offset: %zx: (ufd=%d)%s\n",
+  "@%" PRIx64 " + size:%" PRIx64 " offset: %" PRIx64
+  ": (ufd=%d)%s\n",
  __func__, i,
  dev_region->mmap_addr,
  dev_region->size, dev_region->mmap_offset,
-- 
2.30.2

Re: [PULL 04/10] libvhost-user: Fix wrong type of argument to formatting function (reported by LGTM)

[Stefan Weil via]

Am 04.11.22 um 17:16 schrieb Laurent Vivier:


Hi Stefan,

Le 03/11/2022 à 17:17, Laurent Vivier a écrit :

From: Stefan Weil 

Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Stefan Weil 
Message-Id: <20220422070144.1043697-2...@weilnetz.de>
Signed-off-by: Laurent Vivier 
---
  subprojects/libvhost-user/libvhost-user.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/subprojects/libvhost-user/libvhost-user.c 
b/subprojects/libvhost-user/libvhost-user.c

index ffed4729a3dc..d9a6e3e5560f 100644
--- a/subprojects/libvhost-user/libvhost-user.c
+++ b/subprojects/libvhost-user/libvhost-user.c
@@ -651,7 +651,7 @@ generate_faults(VuDev *dev) {
    if (ioctl(dev->postcopy_ufd, UFFDIO_REGISTER, 
®_struct)) {

  vu_panic(dev, "%s: Failed to userfault region %d "
-  "@%p + size:%zx offset: %zx: (ufd=%d)%s\n",
+  "@%" PRIx64 " + size:%zx offset: %zx: 
(ufd=%d)%s\n",

   __func__, i,
   dev_region->mmap_addr,
   dev_region->size, dev_region->mmap_offset,


They all need PRIx64:

typedef struct VuDevRegion {
    /* Guest Physical address. */
    uint64_t gpa;
    /* Memory region size. */
    uint64_t size;
    /* QEMU virtual address (userspace). */
    uint64_t qva;
    /* Starting offset in our mmaped space. */
    uint64_t mmap_offset;
    /* Start address of mmaped space. */
    uint64_t mmap_addr;
} VuDevRegion;

Could you fix your patch?



The patch fixes one error ("%p"). The two "%zx" are old errors which I 
did not notice because they are only relevant for platforms with 
sizeof(void *) != sizeof(uint64_t), and 32 bit Windows builds don't 
compile libvhost-user. So we need an additional patch which fixes the 
"%zx" before patch 06/10 which adds the format attribute is applied.


Stefan, I suggest to merge the trivial branch without patch 06/10. That 
should fix the build failure and fixes at least some of the format 
errors. Then a patch which fixes the remaining format errors can be 
applied later together with the omitted patch 06/10.


Regards,

Stefan

Re: [PATCH] meson: avoid unused arguments of main() in compiler tests

[Stefan Weil via]

Am 03.11.22 um 18:21 schrieb Paolo Bonzini:


meson.build has one test where "main" is declared unnecessarily
with argc and argv arguments, but does not use them.  Because
the test needs -Werror too, HAVE_BROKEN_SIZE_MAX is defined
incorrectly.

Fix the test and, for consistency, remove argc and argv whenever
they are not needed.

Signed-off-by: Paolo Bonzini 
---



Reviewed-by: Stefan Weil 

Thanks, Stefan



  meson.build | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/meson.build b/meson.build
index 17834b3c3def..7beeac6b5194 100644
--- a/meson.build
+++ b/meson.build
@@ -2143,7 +2143,7 @@ config_host_data.set('CONFIG_SPLICE', 
cc.links(gnu_source_prefix + '''
  
  config_host_data.set('HAVE_MLOCKALL', cc.links(gnu_source_prefix + '''

#include 
-  int main(int argc, char *argv[]) {
+  int main(void) {
  return mlockall(MCL_FUTURE);
}'''))
  
@@ -2188,7 +2188,7 @@ config_host_data.set('HAVE_FSXATTR', cc.links('''

  config_host_data.set('HAVE_BROKEN_SIZE_MAX', not cc.compiles('''
  #include 
  #include 
-int main(int argc, char *argv[]) {
+int main(void) {
  return printf("%zu", SIZE_MAX);
  }''', args: ['-Werror']))
  
@@ -2305,7 +2305,7 @@ config_host_data.set('CONFIG_AVX2_OPT', get_option('avx2') \

__m256i x = *(__m256i *)a;
return _mm256_testz_si256(x, x);
  }
-int main(int argc, char *argv[]) { return bar(argv[0]); }
+int main(int argc, char *argv[]) { return bar(argv[argc - 1]); }
'''), error_message: 'AVX2 not available').allowed())
  
  config_host_data.set('CONFIG_AVX512F_OPT', get_option('avx512f') \

@@ -2319,7 +2319,7 @@ config_host_data.set('CONFIG_AVX512F_OPT', 
get_option('avx512f') \
__m512i x = *(__m512i *)a;
return _mm512_test_epi64_mask(x, x);
  }
-int main(int argc, char *argv[]) { return bar(argv[0]); }
+int main(int argc, char *argv[]) { return bar(argv[argc - 1]); }
'''), error_message: 'AVX512F not available').allowed())
  
  have_pvrdma = get_option('pvrdma') \


OpenPGP_0xE08C21D5677450AD.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH for 7.2] Fix broken configure with -Wunused-parameter

[Stefan Weil via]

Am 03.11.22 um 12:48 schrieb Peter Maydell:


On Wed, 2 Nov 2022 at 20:24, Stefan Weil via  wrote:

The configure script fails because it tries to compile small C programs
with a main function which is declared with arguments argc and argv
although those arguments are unused.

Running `configure -extra-cflags=-Wunused-parameter` triggers the problem.
configure for a native build does abort but shows the error in config.log.
A cross build configure for Windows with Debian stable aborts with an
error.

Avoiding unused arguments fixes this.

Signed-off-by: Stefan Weil 
---

See https://gitlab.com/qemu-project/qemu/-/issues/1295.

I noticed the problem because I often compile with -Wextra.

Stefan

  configure | 7 ---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 4275f5419f..1106c04fea 100755
--- a/configure
+++ b/configure
@@ -1258,6 +1258,7 @@ if test "$stack_protector" != "no"; then
cat > $TMPC << EOF
  int main(int argc, char *argv[])
  {
+(void)argc;

I'm not a huge fan of this syntax, and it doesn't match the way
we deal with "argument is unused" elsewhere in the codebase
(where we either don't care about it or else use the GCC 'unused'
attribute hidden behind the glib G_GNUC_UNUSED macro).



Any other variant is also fine for me, for example "using" argc by a 
"return argc == 0;" instead of "return 0;". Would that be better? If 
there is an accepted variant, I can either send a v2 patch, or maybe 
such a trivial change can be applied when merging.




I am surprised that this didn't get caught by the check in
do_compiler_werror(), which is supposed to report "this
configure test passed without -Werror but failed with
-Werror, so configure is probably buggy.". That's what's
supposed to catch "your compiler warns on stuff our doesn't
in the test case programs".

If you're building with --disable-werror then configure
should be OK anyway. This is probably a good idea if you want
to build with extra warning arguments in --extra-cflags.
If it doesn't work right even with --disable-werror that's
also something we should investigate.



Cross builds for Windows fail with and without --disable-werror. See 
also my bug report https://gitlab.com/qemu-project/qemu/-/issues/1295.


You are right that this is strange and should be investigated, 
especially because native builds don't fail like that.




  char arr[64], *p = arr, *c = argv[0];
  while (*c) {
  *p++ = *c++;
@@ -1607,7 +1608,7 @@ fi

  if test "$safe_stack" = "yes"; then
  cat > $TMPC << EOF
-int main(int argc, char *argv[])
+int main(void)
  {
  #if ! __has_feature(safe_stack)
  #error SafeStack Disabled
@@ -1629,7 +1630,7 @@ EOF
fi
  else
  cat > $TMPC << EOF
-int main(int argc, char *argv[])
+int main(void)
  {
  #if defined(__has_feature)
  #if __has_feature(safe_stack)
@@ -1675,7 +1676,7 @@ static const int Z = 1;
  #define TAUT(X) ((X) == Z)
  #define PAREN(X, Y) (X == Y)
  #define ID(X) (X)
-int main(int argc, char *argv[])
+int main(void)
  {
  int x = 0, y = 0;
  x = ID(x);

No objection to the cases where we can pass "void", that's
a neater way to write the test anyway.

thanks
-- PMM



OpenPGP_0xE08C21D5677450AD.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH for 7.2] Fix broken configure with -Wunused-parameter

[Stefan Weil via]

Am 03.11.22 um 09:58 schrieb Daniel P. Berrangé:


On Wed, Nov 02, 2022 at 09:22:58PM +0100, Stefan Weil via wrote:

The configure script fails because it tries to compile small C programs
with a main function which is declared with arguments argc and argv
although those arguments are unused.

Running `configure -extra-cflags=-Wunused-parameter` triggers the problem.
configure for a native build does abort but shows the error in config.log.
A cross build configure for Windows with Debian stable aborts with an
error.

Avoiding unused arguments fixes this.

I'm not convinced that we should allow -extra-cflags to influence
the configure compile checks at all, as there are likely more cases
where arbitrary -W$warn flag will impact the checks, potentially
causing configure to silently take the wrong action.



I partially agree, but configure should fail if invalid -extra-cflags 
are specified, and the checks must also respect additional include paths 
given by -extra-cflags of course.


And I think that the changes in my patch are an improvement in any case.

Stefan



OpenPGP_0xE08C21D5677450AD.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

[PATCH] Add missing include statement for global xml_builtin

[Stefan Weil via]

This fixes some compiler warnings with compiler flag
-Wmissing-variable-declarations (tested with clang):

aarch64_be-linux-user-gdbstub-xml.c:564:19: warning: no previous extern 
declaration for non-static variable 'xml_builtin' 
[-Wmissing-variable-declarations]
aarch64-linux-user-gdbstub-xml.c:564:19: warning: no previous extern 
declaration for non-static variable 'xml_builtin' 
[-Wmissing-variable-declarations]
aarch64-softmmu-gdbstub-xml.c:1763:19: warning: no previous extern 
declaration for non-static variable 'xml_builtin' 
[-Wmissing-variable-declarations]

Signed-off-by: Stefan Weil 
---
 scripts/feature_to_c.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/feature_to_c.sh b/scripts/feature_to_c.sh
index b1169899c1..c1f67c8f6a 100644
--- a/scripts/feature_to_c.sh
+++ b/scripts/feature_to_c.sh
@@ -56,6 +56,7 @@ for input; do
 done
 
 echo
+echo '#include "exec/gdbstub.h"'
 echo "const char *const xml_builtin[][2] = {"
 
 for input; do
-- 
2.30.2

Re: [PATCH] Run docker probe only if docker or podman are available

[Stefan Weil via]

Am 30.10.22 um 09:35 schrieb Stefan Weil:


The docker probe uses "sudo -n" which can cause an e-mail with a security 
warning
each time when configure is run. Therefore run docker probe only if either 
docker
or podman are available.

That avoids the problematic "sudo -n" on build environments which have neither
docker nor podman installed.

Fixes: c4575b59155e2e00 ("configure: store container engine in config-host.mak")
Signed-off-by: Stefan Weil 
---
  configure | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index 81561be7c1..3af99282b7 100755
--- a/configure
+++ b/configure
@@ -1779,7 +1779,7 @@ fi
  # functions to probe cross compilers
  
  container="no"

-if test $use_containers = "yes"; then
+if test $use_containers = "yes" && (has "docker" || has "podman"); then
  case $($python "$source_path"/tests/docker/docker.py probe) in
  *docker) container=docker ;;
  podman) container=podman ;;



Can this patch be applied by qemu-trivial? For me those security e-mails 
are a bug and should be at least avoided as far as possible in the new 
QEMU release.


Thanks, Stefan

[PATCH for 7.2] Fix broken configure with -Wunused-parameter

[Stefan Weil via]

The configure script fails because it tries to compile small C programs
with a main function which is declared with arguments argc and argv
although those arguments are unused.

Running `configure -extra-cflags=-Wunused-parameter` triggers the problem.
configure for a native build does abort but shows the error in config.log.
A cross build configure for Windows with Debian stable aborts with an
error.

Avoiding unused arguments fixes this.

Signed-off-by: Stefan Weil 
---

See https://gitlab.com/qemu-project/qemu/-/issues/1295.

I noticed the problem because I often compile with -Wextra.

Stefan

 configure | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index 4275f5419f..1106c04fea 100755
--- a/configure
+++ b/configure
@@ -1258,6 +1258,7 @@ if test "$stack_protector" != "no"; then
   cat > $TMPC << EOF
 int main(int argc, char *argv[])
 {
+(void)argc;
 char arr[64], *p = arr, *c = argv[0];
 while (*c) {
 *p++ = *c++;
@@ -1607,7 +1608,7 @@ fi
 
 if test "$safe_stack" = "yes"; then
 cat > $TMPC << EOF
-int main(int argc, char *argv[])
+int main(void)
 {
 #if ! __has_feature(safe_stack)
 #error SafeStack Disabled
@@ -1629,7 +1630,7 @@ EOF
   fi
 else
 cat > $TMPC << EOF
-int main(int argc, char *argv[])
+int main(void)
 {
 #if defined(__has_feature)
 #if __has_feature(safe_stack)
@@ -1675,7 +1676,7 @@ static const int Z = 1;
 #define TAUT(X) ((X) == Z)
 #define PAREN(X, Y) (X == Y)
 #define ID(X) (X)
-int main(int argc, char *argv[])
+int main(void)
 {
 int x = 0, y = 0;
 x = ID(x);
-- 
2.30.2

Re: [PATCH 4/5] disas/nanomips: Remove headers already included by "qemu/osdep.h"

[Stefan Weil via]

Am 01.11.22 um 12:44 schrieb Philippe Mathieu-Daudé:

Signed-off-by: Philippe Mathieu-Daudé 
---
  disas/nanomips.c | 4 
  1 file changed, 4 deletions(-)

diff --git a/disas/nanomips.c b/disas/nanomips.c
index 3f45447292..821d4f8832 100644
--- a/disas/nanomips.c
+++ b/disas/nanomips.c
@@ -30,10 +30,6 @@
  #include "qemu/osdep.h"
  #include "disas/dis-asm.h"
  
-#include 

-#include 
-#include 
-
  typedef int64_t int64;
  typedef uint64_t uint64;
  typedef uint32_t uint32;


Removing those three typedefs and replacing the related types would also 
be good (in another patch).


Reviewed-by: Stefan Weil 



OpenPGP_0xE08C21D5677450AD.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature

Re: [PATCH 3/5] disas/nanomips: Use G_GNUC_PRINTF to avoid invalid string formats

[Stefan Weil via]

Am 01.11.22 um 12:44 schrieb Philippe Mathieu-Daudé:


Suggested-by: Stefan Weil 
Signed-off-by: Philippe Mathieu-Daudé 
---
  disas/nanomips.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/disas/nanomips.c b/disas/nanomips.c
index e4b21e7c45..3f45447292 100644
--- a/disas/nanomips.c
+++ b/disas/nanomips.c
@@ -95,7 +95,7 @@ typedef struct Pool {
  #define IMGASSERTONCE(test)
  
  
-static char *img_format(const char *format, ...)

+static char * G_GNUC_PRINTF(1, 2) img_format(const char *format, ...)
  {
  char *buffer;
  va_list args;



Reviewed-by: Stefan Weil 



OpenPGP_0xE08C21D5677450AD.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature