Merge tag 'pull-shadow-2023-09-29' of https://repo.or.cz/qemu/armbru into staging

-Wshadow=local patches patches for 2023-09-29

# -----BEGIN PGP SIGNATURE-----
#
# iQJGBAABCAAwFiEENUvIs9frKmtoZ05fOHC0AOuRhlMFAmUWhnsSHGFybWJydUBy
# ZWRoYXQuY29tAAoJEDhwtADrkYZTDBkP/2E8cyH+fn7yehNAZT8fjBuDBaj0x3wf
# Bs4++bMEZpgfA/11le/Mm+N9BFDtoGj4dnDwQ0yN6bcKcfmNvxh+M+lNaRO+xvXA
# qs/kJtFYkJYuEj1wgKK2XXd4YcD/S4Qap+FSuUBv8KE/oeALkB1fEpvMcwtJtQqc
# 7POQEqYNQfUe+MX/wKZ+qditbbrFRwX69dAd8+nGTbFestXd2uFA5I5kv3ebxELg
# VjTBgQdp7s82iTvoXpTtmQ6A9ba13zmelxmsAMLlAihkbffMwbtbrkQ7qIIUOW1o
# I4WPxhIXXyZbB48qARUq5G3GQuh+7dRArcpYWaFel2a6cjm2Z6NmWJeRAr0cIaWV
# P5B79k7DO551YsBZn+ubH0U+qwMLw+zq2apQ+SeH/loE0pP/c2OBOPtaVI46D0Dh
# 2kgaSuTIy9AByAHoYBxKnxy4TVwPKzk8hdzCQdiRSO7KJdMqMsV+/w1eR4oH9dsf
# CAvJXVzLicFMMABA/4O99K+1yjIOQpwmiqAjc+gV6FdhwllSH3yQDiK4RMWNAwRu
# bRQHBCk143t7cM3ts09T+5QxkWB3U0iGMJ4rpn43yjH5xwlWmpTlztvd7XlXwyTR
# 8j2Z+8qxe992HmVk34rKdkGnu0qz4AhJBgAEEk2e0oepZvjfigqodQwEMCQsse5t
# cH51HzTDuen/
# =XVKC
# -----END PGP SIGNATURE-----
# gpg: Signature made Fri 29 Sep 2023 04:10:35 EDT
# gpg:                using RSA key 354BC8B3D7EB2A6B68674E5F3870B400EB918653
# gpg:                issuer "armbru@redhat.com"
# gpg: Good signature from "Markus Armbruster <armbru@redhat.com>" [full]
# gpg:                 aka "Markus Armbruster <armbru@pond.sub.org>" [full]
# Primary key fingerprint: 354B C8B3 D7EB 2A6B 6867  4E5F 3870 B400 EB91 8653

* tag 'pull-shadow-2023-09-29' of https://repo.or.cz/qemu/armbru: (56 commits)
  disas/m68k: clean up local variable shadowing
  hw/nvme: Clean up local variable shadowing in nvme_ns_init()
  softmmu/device_tree: Fixup local variables shadowing
  target/riscv: vector_helper: Fixup local variables shadowing
  target/riscv: cpu: Fixup local variables shadowing
  hw/riscv: opentitan: Fixup local variables shadowing
  qemu-nbd: changes towards enabling -Wshadow=local
  seccomp: avoid shadowing of 'action' variable
  crypto: remove shadowed 'ret' variable
  intel_iommu: Fix shadow local variables on "size"
  aspeed/timer: Clean up local variable shadowing
  aspeed/i3c: Rename variable shadowing a local
  aspeed: Clean up local variable shadowing
  aspeed/i2c: Clean up local variable shadowing
  hw/arm/smmuv3-internal.h: Don't use locals in statement macros
  hw/arm/smmuv3.c: Avoid shadowing variable
  hw/misc/arm_sysctl.c: Avoid shadowing local variable
  hw/intc/arm_gicv3_its: Avoid shadowing variable in do_process_its_cmd()
  hw/acpi: changes towards enabling -Wshadow=local
  test-throttle: don't shadow 'index' variable in do_test_accounting()
  ...

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Merge tag 'pull-tricore-20230929' of https://github.com/bkoppelmann/qemu into staging

- Add FTOU, CRCN, FTOHP, and HPTOF insns

# -----BEGIN PGP SIGNATURE-----
#
# iQJTBAABCgA9FiEEbmNqfoPy3Qz6bm43CtLGOWtpyhQFAmUWb2sfHGtiYXN0aWFu
# QG1haWwudW5pLXBhZGVyYm9ybi5kZQAKCRAK0sY5a2nKFPn0D/0S+Zth2okyfe6H
# YdoFB49PWlcafIvZHr1TDswp3LvSDnrjHLJfEW1Gx3mtDkw+/7uid0eMTQ8sDlxJ
# t7spJdZDZ5dkm+9K5MzGkW0zo0jDY6kbS1A3HJRPcpJJJk4zBBL1K4KC1FBUD6IK
# 7n41f5vExgWhIhOgZmT9WTMbBfh73/+Cu8h6M9RAI1VI0O6N5jOETpKTBFsPOx+A
# Kd429cB1c9QeAj0iEXdMn2/Xg2cAII86jrOcYkLYltxir/r6Cia9hfp/F6OXpcZI
# QqKzn11djvbCCL7m9OXhuI3ZP+TIcX7QOabSstfghHlNG1qs/RkXwIRqKHsfRXNG
# nywBTjwIDSiZ4cbZVJ6OjXxbU9OBRkmDgh+SYEVMlFi4E+t3WeTMC8gxUsjfITpK
# JXFoduN2P0yKRjkWQ2OSQ7xX4StFPikXBH1eC8RNnW4IY00wMiJ0tM/0+j+qJLLM
# Ft/bceIZhnGs+axN0jF1EtR03uLZ0kmy3YqsH/KnBnufrag3ytpC/kAtl9Scd6m+
# N4pAT9cfgxqXv/yXAKGupoNPwPGvvSKV6XQTJt2Hn7PBadHWlvlBkgYqGIejpHDM
# x9EghA8o4q5rTu9zTqBv36bOHJEDbJhmq5dYqJTS/q1ORjnWQQsLxv+6XGN3wrbb
# OuexPdD8fH3mWrjeJJ3KDKojOYyGyg==
# =gUyL
# -----END PGP SIGNATURE-----
# gpg: Signature made Fri 29 Sep 2023 02:32:11 EDT
# gpg:                using RSA key 6E636A7E83F2DD0CFA6E6E370AD2C6396B69CA14
# gpg:                issuer "kbastian@mail.uni-paderborn.de"
# gpg: Good signature from "Bastian Koppelmann <kbastian@mail.uni-paderborn.de>" [unknown]
# gpg: WARNING: The key's User ID is not certified with a trusted signature!
# gpg:          There is no indication that the signature belongs to the owner.
# Primary key fingerprint: 6E63 6A7E 83F2 DD0C FA6E  6E37 0AD2 C639 6B69 CA14

* tag 'pull-tricore-20230929' of https://github.com/bkoppelmann/qemu:
  target/tricore: Change effective address (ea) to target_ulong
  target/tricore: Remove CSFRs from cpu.h
  tests/tcg: Reset result register after each test
  hw/tricore: Log failing test in testdevice
  tests/tcg/tricore: Extended and non-extened regs now match
  target/tricore: Fix FTOUZ being ISA v1.3.1 up
  target/tricore: Replace cpu_*_code with translator_*
  target/tricore: Swap src and dst reg for RCRR_INSERT
  target/tricore: Fix RCPW/RRPW_INSERT insns for width = 0
  target/tricore: Implement hptof insn
  target/tricore: Implement ftohp insn
  target/tricore: Clarify special case for FTOUZ insn
  target/tricore: Implement FTOU insn
  target/tricore: Correctly handle FPU RM from PSW
  target/tricore: Implement CRCN insn
  tests/tcg/tricore: Bump cpu to tc37x

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Merge tag 'pull-tcg-20230928' of https://gitlab.com/rth7680/qemu into staging

accel/tcg: Always require can_do_io, for #1866

# -----BEGIN PGP SIGNATURE-----
#
# iQFRBAABCgA7FiEEekgeeIaLTbaoWgXAZN846K9+IV8FAmUV1sgdHHJpY2hhcmQu
# aGVuZGVyc29uQGxpbmFyby5vcmcACgkQZN846K9+IV/8UAgAjFSrUvW5cGTiaow7
# cTvvoFnQzVCM7gFbC2T9W29Hrv0s3tqJHSdUFftzzMXB1atylwb85XN5wegC98zk
# 7+OzrUvTN2fRHVTnWdUg2Tgj+pR/Sw+9HVy851c3B/e3oFegqAIQaNK5w9N3ZpWd
# gpGN8Lau6dEdte8ckMTzG0Sw4LryZyvcgz9+vDv+YX03JaiSMqziHoJGwHfjqr9Q
# FOI1f4gQw+Y2ct6Xop1vVILH2I26US40Y2DvEMEDuZpTkTN0L2V8ipz+tWgGuCMa
# 7HRySJAw5HiDdtAV9/H5dfGJVgjsgwuWoYm+viJtJKJ+X350a5YI77hkbglPxQu+
# Q+dCVA==
# =lQYd
# -----END PGP SIGNATURE-----
# gpg: Signature made Thu 28 Sep 2023 15:40:56 EDT
# gpg:                using RSA key 7A481E78868B4DB6A85A05C064DF38E8AF7E215F
# gpg:                issuer "richard.henderson@linaro.org"
# gpg: Good signature from "Richard Henderson <richard.henderson@linaro.org>" [full]
# Primary key fingerprint: 7A48 1E78 868B 4DB6 A85A  05C0 64DF 38E8 AF7E 215F

* tag 'pull-tcg-20230928' of https://gitlab.com/rth7680/qemu:
  accel/tcg: Always require can_do_io
  accel/tcg: Always set CF_LAST_IO with CF_NOIRQ
  accel/tcg: Improve setting of can_do_io at start of TB
  accel/tcg: Track current value of can_do_io in the TB
  accel/tcg: Hoist CF_MEMI_ONLY check outside translation loop
  accel/tcg: Avoid load of icount_decr if unused

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

Merge tag 'tracing-pull-request' of https://gitlab.com/stefanha/qemu into staging

Pull request

- Mads Ynddal's improvements to simpletrace.py

# -----BEGIN PGP SIGNATURE-----
#
# iQEzBAABCAAdFiEEhpWov9P5fNqsNXdanKSrs4Grc8gFAmUVgyAACgkQnKSrs4Gr
# c8jxkAf/TVtEZTPDvQMz7PNEsAdGLREwYKrsUXHtqOrINbVL1slrJ1qYmygeJltq
# 37CAcxBzvnaH7y2jF8JPKWK3sO3x1fc2ocA72BZt3kuim5T7MojJgFfAN/PdwRHf
# q1a+lYJHKOi0BV4yWPmPHnCSXOohHbJfRBxwZgwFEZMbWdlkAm6m+JFUqn+Vd5ee
# 4vEBxrNMY28H02OeoFNBQPCLRHrkeH4EVj+DP5TAk1H1ehoZeH45UoUsopEfD51z
# IyiXxzumtgpCMyC/jXS5BY6VStW7lDLoDscflMQ/l7ITEtm7Hj2fnPcp/aRKoEUk
# p1JuoSm8YrwnwMokFsQCbkYWkBWieQ==
# =rrnI
# -----END PGP SIGNATURE-----
# gpg: Signature made Thu 28 Sep 2023 09:44:00 EDT
# gpg:                using RSA key 8695A8BFD3F97CDAAC35775A9CA4ABB381AB73C8
# gpg: Good signature from "Stefan Hajnoczi <stefanha@redhat.com>" [ultimate]
# gpg:                 aka "Stefan Hajnoczi <stefanha@gmail.com>" [ultimate]
# Primary key fingerprint: 8695 A8BF D3F9 7CDA AC35  775A 9CA4 ABB3 81AB 73C8

* tag 'tracing-pull-request' of https://gitlab.com/stefanha/qemu:
  scripts/analyse-locks-simpletrace.py: changed iteritems() to items()
  MAINTAINERS: add maintainer of simpletrace.py
  simpletrace: added simplified Analyzer2 class
  simpletrace: move event processing to Analyzer class
  simpletrace: move logic of process into internal function
  simpletrace: refactor to separate responsibilities
  simpletrace: made Analyzer into context-manager
  simpletrace: define exception and add handling
  simpletrace: improved error handling on struct unpack
  simpletrace: update code for Python 3.11
  simpletrace: changed naming of edict and idtoname to improve readability
  simpletrace: improve parsing of sys.argv; fix files never closed.
  simpletrace: annotate magic constants from QEMU code
  simpletrace: add __all__ to define public interface

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

disas/m68k: clean up local variable shadowing

Fix following warnings

.../disas/m68k.c: In function ‘print_insn_arg’:
.../disas/m68k.c:1635:13: warning: declaration of ‘val’ shadows a previous local [-Wshadow=compatible-local]
 1635 |         int val = fetch_arg (buffer, place, 5, info);
      |             ^~~
.../disas/m68k.c:1093:7: note: shadowed declaration is here
 1093 |   int val = 0;
      |       ^~~

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Message-ID: <20230925084455.395150-1-laurent@vivier.eu>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

hw/nvme: Clean up local variable shadowing in nvme_ns_init()

Fix local variable shadowing in nvme_ns_init().

Reported-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
Message-ID: <20230925-fix-local-shadowing-v1-1-3a1172132377@samsung.com>
Reviewed-by: Jesper Wendel Devantier <j.devantier@samsung.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

softmmu/device_tree: Fixup local variables shadowing

Local variables shadowing other local variables or parameters make the
code needlessly hard to understand.  Bugs love to hide in such code.
Evidence: "[PATCH v3 1/7] migration/rdma: Fix save_page method to fail
on polling error".

This patch removes the local variable shadowing. Tested by adding:

    --extra-cflags='-Wshadow=local -Wno-error=shadow=local -Wno-error=shadow=compatible-local'

To configure

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Message-ID: <20230925043023.71448-5-alistair.francis@wdc.com>
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

target/riscv: vector_helper: Fixup local variables shadowing

Local variables shadowing other local variables or parameters make the
code needlessly hard to understand.  Bugs love to hide in such code.
Evidence: "[PATCH v3 1/7] migration/rdma: Fix save_page method to fail
on polling error".

This patch removes the local variable shadowing. Tested by adding:

    --extra-cflags='-Wshadow=local -Wno-error=shadow=local -Wno-error=shadow=compatible-local'

To configure

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Message-ID: <20230925043023.71448-4-alistair.francis@wdc.com>
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

target/riscv: cpu: Fixup local variables shadowing

Local variables shadowing other local variables or parameters make the
code needlessly hard to understand.  Bugs love to hide in such code.
Evidence: "[PATCH v3 1/7] migration/rdma: Fix save_page method to fail
on polling error".

This patch removes the local variable shadowing. Tested by adding:

    --extra-cflags='-Wshadow=local -Wno-error=shadow=local -Wno-error=shadow=compatible-local'

To configure

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Message-ID: <20230925043023.71448-3-alistair.francis@wdc.com>
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

hw/riscv: opentitan: Fixup local variables shadowing

Local variables shadowing other local variables or parameters make the
code needlessly hard to understand.  Bugs love to hide in such code.
Evidence: "[PATCH v3 1/7] migration/rdma: Fix save_page method to fail
on polling error".

This patch removes the local variable shadowing. Tested by adding:

    --extra-cflags='-Wshadow=local -Wno-error=shadow=local -Wno-error=shadow=compatible-local'

To configure

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Message-ID: <20230925043023.71448-2-alistair.francis@wdc.com>
Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

qemu-nbd: changes towards enabling -Wshadow=local

Address all compiler complaints from -Wshadow in qemu-nbd.  Several
instances of 'int ret' became shadows when commit 4fbec260 added 'ret'
at a higher scope in main.  More interesting was the 'void *ret'
capturing the result of a pthread; where we were conceptually doing
'(void*)(intptr_t)EXIT_FAILURE != NULL' which just feels wrong (even
though it happens to compile correctly), so it was worth a better
cleanup.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-ID: <20230922205019.2755352-2-eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

seccomp: avoid shadowing of 'action' variable

This is confusing as one 'action' variable is used for storing
a SCMP_ enum value, while the other 'action' variable is used
for storing a SECCOMP_ enum value.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-ID: <20230922160644.438631-3-berrange@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

crypto: remove shadowed 'ret' variable

Both instances of 'ret' are used to store a gnutls API return code.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-ID: <20230922160644.438631-2-berrange@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

intel_iommu: Fix shadow local variables on "size"

This patch fixes the warning of shadowed local variable:

../hw/i386/intel_iommu.c: In function ‘vtd_address_space_unmap’:
../hw/i386/intel_iommu.c:3773:18: warning: declaration of ‘size’ shadows a previous local [-Wshadow=compatible-local]
 3773 |         uint64_t size = mask + 1;
      |                  ^~~~
../hw/i386/intel_iommu.c:3747:12: note: shadowed declaration is here
 3747 |     hwaddr size, remain;
      |            ^~~~

Cc: Jason Wang <jasowang@redhat.com>
Cc: Eric Auger <eric.auger@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-ID: <20230922160410.138786-1-peterx@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

aspeed/timer: Clean up local variable shadowing

commit 8137355e850f ("aspeed/timer: Fix behaviour running Linux")
introduced a MAX() expression to calculate the next timer deadline :

    return calculate_time(t, MAX(MAX(t->match[0], t->match[1]), 0));

The second MAX() is not necessary since the compared values are an
unsigned and 0. Simply remove it and fix warning :

  ../hw/timer/aspeed_timer.c: In function ‘calculate_next’:
  ../include/qemu/osdep.h:396:31: warning: declaration of ‘_a’ shadows a previous local [-Wshadow=compatible-local]
    396 |         typeof(1 ? (a) : (b)) _a = (a), _b = (b);       \
        |                               ^~
  ../hw/timer/aspeed_timer.c:170:12: note: in expansion of macro ‘MAX’
    170 |     next = MAX(MAX(calculate_match(t, 0), calculate_match(t, 1)), 0);
        |            ^~~
  ../hw/timer/aspeed_timer.c:170:16: note: in expansion of macro ‘MAX’
    170 |     next = MAX(MAX(calculate_match(t, 0), calculate_match(t, 1)), 0);
        |                ^~~
  /home/legoater/work/qemu/qemu-aspeed.git/include/qemu/osdep.h:396:31: note: shadowed declaration is here
    396 |         typeof(1 ? (a) : (b)) _a = (a), _b = (b);       \
        |                               ^~
  ../hw/timer/aspeed_timer.c:170:12: note: in expansion of macro ‘MAX’
    170 |     next = MAX(MAX(calculate_match(t, 0), calculate_match(t, 1)), 0);
        |            ^~~

Cc: Joel Stanley <joel@jms.id.au>
Cc: Andrew Jeffery <andrew@aj.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
Message-ID: <20230922155924.1172019-5-clg@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

aspeed/i3c: Rename variable shadowing a local

to fix warning :

  ../hw/i3c/aspeed_i3c.c: In function ‘aspeed_i3c_realize’:
  ../hw/i3c/aspeed_i3c.c:1959:17: warning: declaration of ‘dev’ shadows a parameter [-Wshadow=local]
   1959 |         Object *dev = OBJECT(&s->devices[i]);
        |                 ^~~
  ../hw/i3c/aspeed_i3c.c:1942:45: note: shadowed declaration is here
   1942 | static void aspeed_i3c_realize(DeviceState *dev, Error **errp)
        |                                ~~~~~~~~~~~~~^~~

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Message-ID: <20230922155924.1172019-4-clg@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

aspeed: Clean up local variable shadowing

Remove superfluous local 'irq' variables and use the one define at the
top of the routine. This fixes warnings in aspeed_soc_ast2600_realize()
such as :

  ../hw/arm/aspeed_ast2600.c: In function ‘aspeed_soc_ast2600_realize’:
  ../hw/arm/aspeed_ast2600.c:420:18: warning: declaration of ‘irq’ shadows a previous local [-Wshadow=compatible-local]
    420 |         qemu_irq irq = aspeed_soc_get_irq(s, ASPEED_DEV_TIMER1 + i);
        |                  ^~~
  ../hw/arm/aspeed_ast2600.c:312:14: note: shadowed declaration is here
    312 |     qemu_irq irq;
        |              ^~~

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Message-ID: <20230922155924.1172019-3-clg@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

aspeed/i2c: Clean up local variable shadowing

Remove superfluous local 'data' variable and use the one define at the
top of the routine. This fixes :

  ../hw/i2c/aspeed_i2c.c: In function ‘aspeed_i2c_bus_recv’:
  ../hw/i2c/aspeed_i2c.c:315:17: warning: declaration of ‘data’ shadows a previous local [-Wshadow=compatible-local]
    315 |         uint8_t data;
        |                 ^~~~
  ../hw/i2c/aspeed_i2c.c:288:13: note: shadowed declaration is here
    288 |     uint8_t data;
        |             ^~~~

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Message-ID: <20230922155924.1172019-2-clg@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

hw/arm/smmuv3-internal.h: Don't use locals in statement macros

The STE_CTXPTR() and STE_S2TTB() macros both extract two halves
of an address from fields in the STE and combine them into a
single value to return. The current code for this uses a GCC
statement expression. There are two problems with this:

(1) The type chosen for the variable in the statement expr
is 'unsigned long', which might not be 64 bits

(2) the name chosen for the variable causes -Wshadow warnings
because it's the same as a variable in use at the callsite:

In file included from ../../hw/arm/smmuv3.c:34:
../../hw/arm/smmuv3.c: In function ‘smmu_get_cd’:
../../hw/arm/smmuv3-internal.h:538:23: warning: declaration of ‘addr’ shadows a previous local [-Wshadow=compatible-local]
  538 |         unsigned long addr;                                     \
      |                       ^~~~
../../hw/arm/smmuv3.c:339:23: note: in expansion of macro ‘STE_CTXPTR’
  339 |     dma_addr_t addr = STE_CTXPTR(ste);
      |                       ^~~~~~~~~~
../../hw/arm/smmuv3.c:339:16: note: shadowed declaration is here
  339 |     dma_addr_t addr = STE_CTXPTR(ste);
      |                ^~~~

Sidestep both of these problems by just using a single
expression rather than a statement expr.

For CMD_ADDR, we got the type of the variable right but still
run into -Wshadow problems:

In file included from ../../hw/arm/smmuv3.c:34:
../../hw/arm/smmuv3.c: In function ‘smmuv3_range_inval’:
../../hw/arm/smmuv3-internal.h:334:22: warning: declaration of ‘addr’ shadows a previous local [-Wshadow=compatible-local]
  334 |             uint64_t addr = high << 32 | (low << 12);         \
      |                      ^~~~
../../hw/arm/smmuv3.c:1104:28: note: in expansion of macro ‘CMD_ADDR’
 1104 |     dma_addr_t end, addr = CMD_ADDR(cmd);
      |                            ^~~~~~~~
../../hw/arm/smmuv3.c:1104:21: note: shadowed declaration is here
 1104 |     dma_addr_t end, addr = CMD_ADDR(cmd);
      |                     ^~~~

so convert it too.

CD_TTB has neither problem, but it is the only other macro in
the file that uses this pattern, so we convert it also for
consistency's sake.

We use extract64() rather than extract32() to avoid having
to explicitly cast the result to uint64_t.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-ID: <20230922152944.3583438-5-peter.maydell@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

hw/arm/smmuv3.c: Avoid shadowing variable

Avoid shadowing a variable in smmuv3_notify_iova():

../../hw/arm/smmuv3.c: In function ‘smmuv3_notify_iova’:
../../hw/arm/smmuv3.c:1043:23: warning: declaration of ‘event’ shadows a previous local [-Wshadow=local]
 1043 |         SMMUEventInfo event = {.inval_ste_allowed = true};
      |                       ^~~~~
../../hw/arm/smmuv3.c:1038:19: note: shadowed declaration is here
 1038 |     IOMMUTLBEvent event;
      |                   ^~~~~

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-ID: <20230922152944.3583438-4-peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

hw/misc/arm_sysctl.c: Avoid shadowing local variable

Avoid shadowing a local variable in arm_sysctl_write():

../../hw/misc/arm_sysctl.c: In function ‘arm_sysctl_write’:
../../hw/misc/arm_sysctl.c:537:26: warning: declaration of ‘val’ shadows a parameter [-Wshadow=local]
  537 |                 uint32_t val;
      |                          ^~~
../../hw/misc/arm_sysctl.c:388:39: note: shadowed declaration is here
  388 |                              uint64_t val, unsigned size)
      |                              ~~~~~~~~~^~~

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-ID: <20230922152944.3583438-3-peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

hw/intc/arm_gicv3_its: Avoid shadowing variable in do_process_its_cmd()

Avoid shadowing a local variable in do_process_its_cmd():

../../hw/intc/arm_gicv3_its.c:548:17: warning: declaration of ‘ite’ shadows a previous local [-Wshadow=compatible-local]
  548 |         ITEntry ite = {};
      |                 ^~~
../../hw/intc/arm_gicv3_its.c:518:13: note: shadowed declaration is here
  518 |     ITEntry ite;
      |             ^~~

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-ID: <20230922152944.3583438-2-peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

hw/acpi: changes towards enabling -Wshadow=local

Code changes in acpi that addresses all compiler complaints coming from enabling
-Wshadow flags. Enabling -Wshadow catches cases of local variables shadowing
other local variables or parameters. These makes the code confusing and/or adds
bugs that are difficult to catch.  See also

    Subject: Help wanted for enabling -Wshadow=local
    Message-Id: <87r0mqlf9x.fsf@pond.sub.org>
    https://lore.kernel.org/qemu-devel/87r0mqlf9x.fsf@pond.sub.org

The code is tested to build with and without the flag turned on.

CC: Markus Armbruster <armbru@redhat.com>
CC: Philippe Mathieu-Daude <philmd@linaro.org>
CC: mst@redhat.com
CC: imammedo@redhat.com
Signed-off-by: Ani Sinha <anisinha@redhat.com>
Message-ID: <20230922124203.127110-1-anisinha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
[Commit message tweaked]
Signed-off-by: Markus Armbruster <armbru@redhat.com>

test-throttle: don't shadow 'index' variable in do_test_accounting()

Fixes build with -Wshadow=local

Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-ID: <20230922105742.81317-1-berto@igalia.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

spapr/drc: Clean up local variable shadowing in prop_get_fdt()

Rename 'name' variable to avoid this warning :

  ../hw/ppc/spapr_drc.c: In function ‘prop_get_fdt’:
  ../hw/ppc/spapr_drc.c:344:21: warning: declaration of ‘name’ shadows a parameter [-Wshadow=compatible-local]
    344 |         const char *name = NULL;
        |                     ^~~~
  ../hw/ppc/spapr_drc.c:325:63: note: shadowed declaration is here
    325 | static void prop_get_fdt(Object *obj, Visitor *v, const char *name,
        |                                                   ~~~~~~~~~~~~^~~~

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Message-ID: <20230918145850.241074-9-clg@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

spapr/pci: Clean up local variable shadowing in spapr_phb_realize()

Rename SysBusDevice variable to avoid this warning :

  ../hw/ppc/spapr_pci.c: In function ‘spapr_phb_realize’:
  ../hw/ppc/spapr_pci.c:1872:24: warning: declaration of ‘s’ shadows a previous local [-Wshadow=local]
   1872 |         SpaprPhbState *s;
        |                        ^
  ../hw/ppc/spapr_pci.c:1829:19: note: shadowed declaration is here
   1829 |     SysBusDevice *s = SYS_BUS_DEVICE(dev);
        |                   ^

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Message-ID: <20230918145850.241074-8-clg@kaod.org>
Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

spapr/drc: Clean up local variable shadowing in rtas_ibm_configure_connector()

Remove extra 'drc_index' variable to avoid this warning :

  ../hw/ppc/spapr_drc.c: In function ‘rtas_ibm_configure_connector’:
  ../hw/ppc/spapr_drc.c:1240:26: warning: declaration of ‘drc_index’ shadows a previous local [-Wshadow=compatible-local]
   1240 |                 uint32_t drc_index = spapr_drc_index(drc);
        |                          ^~~~~~~~~
  ../hw/ppc/spapr_drc.c:1155:14: note: shadowed declaration is here
   1155 |     uint32_t drc_index;
        |              ^~~~~~~~~

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Message-ID: <20230918145850.241074-7-clg@kaod.org>
Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

spapr: Clean up local variable shadowing in spapr_get_fw_dev_path()

Rename PCIDevice variable to avoid this warning :

  ../hw/ppc/spapr.c: In function ‘spapr_get_fw_dev_path’:
  ../hw/ppc/spapr.c:3217:20: warning: declaration of ‘pcidev’ shadows a previous local [-Wshadow=compatible-local]
   3217 |         PCIDevice *pcidev = CAST(PCIDevice, dev, TYPE_PCI_DEVICE);
        |                    ^~~~~~
  ../hw/ppc/spapr.c:3147:16: note: shadowed declaration is here
   3147 |     PCIDevice *pcidev = CAST(PCIDevice, dev, TYPE_PCI_DEVICE);
        |                ^~~~~~

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Message-ID: <20230918145850.241074-6-clg@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

spapr: Clean up local variable shadowing in spapr_init_cpus()

Remove extra 'i' variable to fix this warning :

  ../hw/ppc/spapr.c: In function ‘spapr_init_cpus’:
  ../hw/ppc/spapr.c:2668:13: warning: declaration of ‘i’ shadows a previous local [-Wshadow=compatible-local]
   2668 |         int i;
        |             ^
  ../hw/ppc/spapr.c:2645:9: note: shadowed declaration is here
   2645 |     int i;
        |         ^

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Message-ID: <20230918145850.241074-5-clg@kaod.org>
Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

spapr: Clean up local variable shadowing in spapr_dt_cpus()

Introduce a helper routine defining one CPU device node to fix this
warning :

  ../hw/ppc/spapr.c: In function ‘spapr_dt_cpus’:
  ../hw/ppc/spapr.c:812:19: warning: declaration of ‘cs’ shadows a previous local [-Wshadow=compatible-local]
    812 |         CPUState *cs = rev[i];
        |                   ^~
  ../hw/ppc/spapr.c:786:15: note: shadowed declaration is here
    786 |     CPUState *cs;
        |               ^~

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Message-ID: <20230918145850.241074-4-clg@kaod.org>
Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

pnv/psi: Clean up local variable shadowing

to fix :

  ../hw/ppc/pnv_psi.c: In function ‘pnv_psi_p9_mmio_write’:
  ../hw/ppc/pnv_psi.c:741:24: warning: declaration of ‘addr’ shadows a parameter [-Wshadow=compatible-local]
    741 |                 hwaddr addr = val & ~(PSIHB9_ESB_CI_VALID | PSIHB10_ESB_CI_64K);
        |                        ^~~~
  ../hw/ppc/pnv_psi.c:702:56: note: shadowed declaration is here
    702 | static void pnv_psi_p9_mmio_write(void *opaque, hwaddr addr,
        |                                                 ~~~~~~~^~~~

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Message-ID: <20230918145850.241074-3-clg@kaod.org>
Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

hw/ppc: Clean up local variable shadowing in _FDT helper routine

this fixes numerous warnings of this type :

  In file included from ../hw/ppc/spapr_pci.c:43:
  ../hw/ppc/spapr_pci.c: In function ‘spapr_dt_phb’:
  ../include/hw/ppc/fdt.h:18:13: warning: declaration of ‘ret’ shadows a previous local [-Wshadow=compatible-local]
     18 |         int ret = (exp);                                           \
        |             ^~~
  ../hw/ppc/spapr_pci.c:2355:5: note: in expansion of macro ‘_FDT’
   2355 |     _FDT(bus_off = fdt_add_subnode(fdt, 0, phb->dtbusname));
        |     ^~~~
  ../hw/ppc/spapr_pci.c:2311:24: note: shadowed declaration is here
   2311 |     int bus_off, i, j, ret;
        |                        ^~~

Signed-off-by: Cédric Le Goater <clg@kaod.org>
Message-ID: <20230918145850.241074-2-clg@kaod.org>
Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

hw/intc/openpic: Clean up local variable shadowing

Fix:

  hw/intc/openpic.c: In function ‘openpic_gbl_write’:
  hw/intc/openpic.c:614:17: warning: declaration of ‘idx’ shadows a previous local [-Wshadow=compatible-local]
    614 |             int idx;
        |                 ^~~
  hw/intc/openpic.c:568:9: note: shadowed declaration is here
    568 |     int idx;
        |         ^~~

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20230904162824.85385-3-philmd@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

hw/core/machine: Clean up local variable shadowing

Fix:

  hw/core/machine.c: In function ‘machine_initfn’:
  hw/core/machine.c:1081:17: warning: declaration of ‘obj’ shadows a parameter [-Wshadow=compatible-local]
   1081 |         Object *obj = OBJECT(ms);
        |                 ^~~
  hw/core/machine.c:1065:36: note: shadowed declaration is here
   1065 | static void machine_initfn(Object *obj)
        |                            ~~~~~~~~^~~

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20230904162824.85385-2-philmd@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

softmmu/physmem: Clean up local variable shadowing

Fix:

  softmmu/physmem.c: In function ‘cpu_physical_memory_snapshot_and_clear_dirty’:
  softmmu/physmem.c:916:27: warning: declaration of ‘offset’ shadows a parameter [-Wshadow=compatible-local]
    916 |             unsigned long offset = page % DIRTY_MEMORY_BLOCK_SIZE;
        |                           ^~~~~~
  softmmu/physmem.c:892:31: note: shadowed declaration is here
    892 |     (MemoryRegion *mr, hwaddr offset, hwaddr length, unsigned client)
        |                        ~~~~~~~^~~~~~

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20230904161235.84651-23-philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

softmmu/memory: Clean up local variable shadowing

Fix:

  softmmu/memory.c: In function ‘mtree_print_mr’:
  softmmu/memory.c:3236:27: warning: declaration of ‘ml’ shadows a previous local [-Wshadow=compatible-local]
   3236 |         MemoryRegionList *ml;
        |                           ^~
  softmmu/memory.c:3213:32: note: shadowed declaration is here
   3213 |     MemoryRegionList *new_ml, *ml, *next_ml;
        |                                ^~

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20230904161235.84651-22-philmd@linaro.org>
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

sysemu/device_tree: Clean up local variable shadowing

Fix:

  hw/mips/boston.c:472:5: error: declaration shadows a local variable [-Werror,-Wshadow]
    qemu_fdt_setprop_cells(fdt, name, "reg", reg_base, reg_size);
    ^
  include/sysemu/device_tree.h:129:13: note: expanded from macro 'qemu_fdt_setprop_cells'
        int i;
            ^
  hw/mips/boston.c:461:9: note: previous declaration is here
    int i;
        ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20230904161235.84651-21-philmd@linaro.org>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

linux-user/strace: Clean up local variable shadowing

Fix:

  linux-user/strace.c: In function ‘print_sockaddr’:
  linux-user/strace.c:370:17: warning: declaration of ‘i’ shadows a previous local [-Wshadow=compatible-local]
    370 |             int i;
        |                 ^
  linux-user/strace.c:361:9: note: shadowed declaration is here
    361 |     int i;
        |         ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20230904161235.84651-20-philmd@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

util/vhost-user-server: Clean up local variable shadowing

Fix:

  util/vhost-user-server.c: In function ‘set_watch’:
  util/vhost-user-server.c:274:20: warning: declaration of ‘vu_fd_watch’ shadows a previous local [-Wshadow=compatible-local]
    274 |         VuFdWatch *vu_fd_watch = g_new0(VuFdWatch, 1);
        |                    ^~~~~~~~~~~
  util/vhost-user-server.c:271:16: note: shadowed declaration is here
    271 |     VuFdWatch *vu_fd_watch = find_vu_fd_watch(server, fd);
        |                ^~~~~~~~~~~

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20230904161235.84651-18-philmd@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

crypto/cipher-gnutls.c: Clean up local variable shadowing

Fix:

  In file included from crypto/cipher.c:140:
  crypto/cipher-gnutls.c.inc: In function ‘qcrypto_gnutls_cipher_encrypt’:
  crypto/cipher-gnutls.c.inc:116:17: warning: declaration of ‘err’ shadows a previous local [-Wshadow=compatible-local]
    116 |             int err = gnutls_cipher_init(&handle, ctx->galg, &gkey, NULL);
        |                 ^~~
  crypto/cipher-gnutls.c.inc:94:9: note: shadowed declaration is here
     94 |     int err;
        |         ^~~
       ---

  crypto/cipher-gnutls.c.inc: In function ‘qcrypto_gnutls_cipher_decrypt’:
  crypto/cipher-gnutls.c.inc:177:17: warning: declaration of ‘err’ shadows a previous local [-Wshadow=compatible-local]
    177 |             int err = gnutls_cipher_init(&handle, ctx->galg, &gkey, NULL);
        |                 ^~~
  crypto/cipher-gnutls.c.inc:154:9: note: shadowed declaration is here
    154 |     int err;
        |         ^~~

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20230904161235.84651-17-philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

net/eth: Clean up local variable shadowing

Fix:

  net/eth.c:435:20: error: declaration shadows a local variable [-Werror,-Wshadow]
            size_t input_size = iov_size(pkt, pkt_frags);
                   ^
  net/eth.c:413:16: note: previous declaration is here
        size_t input_size = iov_size(pkt, pkt_frags);
               ^

Suggested-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20230904161235.84651-16-philmd@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

hw/nios2: Clean up local variable shadowing

Fix:

  hw/nios2/10m50_devboard.c: In function ‘nios2_10m50_ghrd_init’:
  hw/nios2/10m50_devboard.c:101:22: warning: declaration of ‘dev’ shadows a previous local [-Wshadow=compatible-local]
    101 |         DeviceState *dev = qdev_new(TYPE_NIOS2_VIC);
        |                      ^~~
  hw/nios2/10m50_devboard.c:60:18: note: shadowed declaration is here
     60 |     DeviceState *dev;
        |                  ^~~

  hw/nios2/10m50_devboard.c:110:18: warning: declaration of ‘i’ shadows a previous local [-Wshadow=compatible-local]
    110 |         for (int i = 0; i < 32; i++) {
        |                  ^
  hw/nios2/10m50_devboard.c:67:9: note: shadowed declaration is here
     67 |     int i;
        |         ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20230904161235.84651-15-philmd@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

hw/microblaze: Clean up local variable shadowing

Fix:

  hw/microblaze/petalogix_ml605_mmu.c: In function ‘petalogix_ml605_init’:
  hw/microblaze/petalogix_ml605_mmu.c:186:24: warning: declaration of ‘dinfo’ shadows a previous local [-Wshadow=compatible-local]
    186 |             DriveInfo *dinfo = drive_get(IF_MTD, 0, i);
        |                        ^~~~~
  hw/microblaze/petalogix_ml605_mmu.c:78:16: note: shadowed declaration is here
     78 |     DriveInfo *dinfo;
        |                ^~~~~

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20230904161235.84651-14-philmd@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

hw/m68k: Clean up local variable shadowing

Fix:

  hw/m68k/virt.c:263:13: error: declaration shadows a local variable [-Werror,-Wshadow]
            BOOTINFOSTR(param_ptr, BI_COMMAND_LINE,
            ^
  hw/m68k/bootinfo.h:47:13: note: expanded from macro 'BOOTINFOSTR'
        int i; \
            ^
  hw/m68k/virt.c:130:9: note: previous declaration is here
    int i;
        ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20230904161235.84651-13-philmd@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

hw/arm/allwinner: Clean up local variable shadowing

Fix:

  hw/arm/allwinner-r40.c:412:14: error: declaration shadows a local variable [-Werror,-Wshadow]
    for (int i = 0; i < AW_R40_NUM_MMCS; i++) {
             ^
  hw/arm/allwinner-r40.c:299:14: note: previous declaration is here
    unsigned i;
             ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-ID: <20230904161235.84651-10-philmd@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

hw/arm/virt: Clean up local variable shadowing

Fix:

  hw/arm/virt.c:821:22: error: declaration shadows a local variable [-Werror,-Wshadow]
            qemu_irq irq = qdev_get_gpio_in(vms->gic,
                     ^
  hw/arm/virt.c:803:13: note: previous declaration is here
        int irq;
            ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-ID: <20230904161235.84651-9-philmd@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

hw/arm/armv7m: Clean up local variable shadowing

Fix:

  hw/arm/armv7m.c: In function ‘armv7m_realize’:
  hw/arm/armv7m.c:520:27: warning: declaration of ‘sbd’ shadows a previous local [-Wshadow=compatible-local]
    520 |             SysBusDevice *sbd = SYS_BUS_DEVICE(&s->bitband[i]);
        |                           ^~~
  hw/arm/armv7m.c:278:19: note: shadowed declaration is here
    278 |     SysBusDevice *sbd;
        |                   ^~~
       ---

  hw/arm/armsse.c: In function ‘armsse_realize’:
  hw/arm/armsse.c:1471:27: warning: declaration of ‘mr’ shadows a previous local [-Wshadow=compatible-local]
   1471 |             MemoryRegion *mr;
        |                           ^~
  hw/arm/armsse.c:917:19: note: shadowed declaration is here
    917 |     MemoryRegion *mr;
        |                   ^~
       ---

  hw/arm/armsse.c:1608:22: warning: declaration of ‘dev_splitter’ shadows a previous local [-Wshadow=compatible-local]
   1608 |         DeviceState *dev_splitter = DEVICE(splitter);
        |                      ^~~~~~~~~~~~
  hw/arm/armsse.c:923:18: note: shadowed declaration is here
    923 |     DeviceState *dev_splitter;
        |                  ^~~~~~~~~~~~

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20230904161235.84651-8-philmd@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

target/tricore: Clean up local variable shadowing

Fix:

  target/tricore/translate.c:5016:18: warning: declaration of ‘temp’ shadows a previous local [-Wshadow=compatible-local]
   5016 |             TCGv temp = tcg_constant_i32(const9);
        |                  ^~~~
  target/tricore/translate.c:4958:10: note: shadowed declaration is here
   4958 |     TCGv temp;
        |          ^~~~

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20230904161235.84651-7-philmd@linaro.org>
Reviewed-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

target/m68k: Clean up local variable shadowing

Fix:

  target/m68k/translate.c:828:18: error: declaration shadows a local variable [-Werror,-Wshadow]
            TCGv tmp = tcg_temp_new();
                 ^
  target/m68k/translate.c:801:15: note: previous declaration is here
    TCGv reg, tmp, result;
              ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-ID: <20230904161235.84651-6-philmd@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

target/mips: Clean up local variable shadowing

Fix:

  target/mips/tcg/nanomips_translate.c.inc:4410:33: error: declaration shadows a local variable [-Werror,-Wshadow]
                        int32_t imm = extract32(ctx->opcode, 1, 13) |
                                ^
  target/mips/tcg/nanomips_translate.c.inc:3577:9: note: previous declaration is here
    int imm;
        ^
  target/mips/tcg/translate.c:15578:19: error: declaration shadows a local variable [-Werror,-Wshadow]
    for (unsigned i = 1; i < 32; i++) {
                  ^
  target/mips/tcg/translate.c:15567:9: note: previous declaration is here
    int i;
        ^
  target/mips/tcg/msa_helper.c:7478:13: error: declaration shadows a local variable [-Werror,-Wshadow]
            MSA_FLOAT_MAXOP(pwx->w[0], min, pws->w[0], pws->w[0], 32);
            ^
  target/mips/tcg/msa_helper.c:7434:23: note: expanded from macro 'MSA_FLOAT_MAXOP'
        float_status *status = &env->active_tc.msa_fp_status;
                      ^

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20230904161235.84651-5-philmd@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

target/arm/hvf: Clean up local variable shadowing

Per Peter Maydell analysis [*]:

  The hvf_vcpu_exec() function is not documented, but in practice
  its caller expects it to return either EXCP_DEBUG (for "this was
  a guest debug exception you need to deal with") or something else
  (presumably the intention being 0 for OK).

  The hvf_sysreg_read() and hvf_sysreg_write() functions are also not
  documented, but they return 0 on success, or 1 for a completely
  unrecognized sysreg where we've raised the UNDEF exception (but
  not if we raised an UNDEF exception for an unrecognized GIC sysreg --
  I think this is a bug). We use this return value to decide whether
  we need to advance the PC past the insn or not. It's not the same
  as the return value we want to return from hvf_vcpu_exec().

  Retain the variable as locally scoped but give it a name that
  doesn't clash with the other function-scoped variable.

This fixes:

  target/arm/hvf/hvf.c:1936:13: error: declaration shadows a local variable [-Werror,-Wshadow]
        int ret = 0;
            ^
  target/arm/hvf/hvf.c:1807:9: note: previous declaration is here
    int ret;
        ^
[*] https://lore.kernel.org/qemu-devel/CAFEAcA_e+fU6JKtS+W63wr9cCJ6btu_hT_ydZWOwC0kBkDYYYQ@mail.gmail.com/

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20230904161235.84651-4-philmd@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

target/arm/tcg: Clean up local variable shadowing

Fix:

  target/arm/tcg/translate-m-nocp.c: In function ‘gen_M_fp_sysreg_read’:
  target/arm/tcg/translate-m-nocp.c:509:18: warning: declaration of ‘tmp’ shadows a previous local [-Wshadow=compatible-local]
    509 |         TCGv_i32 tmp = load_cpu_field(v7m.fpdscr[M_REG_NS]);
        |                  ^~~
  target/arm/tcg/translate-m-nocp.c:433:14: note: shadowed declaration is here
    433 |     TCGv_i32 tmp;
        |              ^~~
       ---

  target/arm/tcg/mve_helper.c: In function ‘helper_mve_vqshlsb’:
  target/arm/tcg/mve_helper.c:1259:19: warning: declaration of ‘r’ shadows a previous local [-Wshadow=compatible-local]
   1259 |         typeof(N) r = FN(N, (int8_t)(M), sizeof(N) * 8, ROUND, &su32);  \
        |                   ^
  target/arm/tcg/mve_helper.c:1267:5: note: in expansion of macro ‘WRAP_QRSHL_HELPER’
   1267 |     WRAP_QRSHL_HELPER(do_sqrshl_bhs, N, M, false, satp)
        |     ^~~~~~~~~~~~~~~~~
  target/arm/tcg/mve_helper.c:927:22: note: in expansion of macro ‘DO_SQSHL_OP’
    927 |             TYPE r = FN(n[H##ESIZE(e)], m[H##ESIZE(e)], &sat);          \
        |                      ^~
  target/arm/tcg/mve_helper.c:945:5: note: in expansion of macro ‘DO_2OP_SAT’
    945 |     DO_2OP_SAT(OP##b, 1, int8_t, FN)            \
        |     ^~~~~~~~~~
  target/arm/tcg/mve_helper.c:1277:1: note: in expansion of macro ‘DO_2OP_SAT_S’
   1277 | DO_2OP_SAT_S(vqshls, DO_SQSHL_OP)
        | ^~~~~~~~~~~~
       ---

  target/arm/tcg/mve_helper.c: In function ‘do_sqrshl48_d’:
  target/arm/tcg/mve_helper.c:2463:17: warning: declaration of ‘extval’ shadows a previous local [-Wshadow=compatible-local]
   2463 |         int64_t extval = sextract64(src << shift, 0, 48);
        |                 ^~~~~~
  target/arm/tcg/mve_helper.c:2443:18: note: shadowed declaration is here
   2443 |     int64_t val, extval;
        |                  ^~~~~~
       ---

  target/arm/tcg/mve_helper.c: In function ‘do_uqrshl48_d’:
  target/arm/tcg/mve_helper.c:2495:18: warning: declaration of ‘extval’ shadows a previous local [-Wshadow=compatible-local]
   2495 |         uint64_t extval = extract64(src << shift, 0, 48);
        |                  ^~~~~~
  target/arm/tcg/mve_helper.c:2479:19: note: shadowed declaration is here
   2479 |     uint64_t val, extval;
        |                   ^~~~~~

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20230904161235.84651-3-philmd@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

tcg: Clean up local variable shadowing

Fix:

  tcg/tcg.c:2551:27: error: declaration shadows a local variable [-Werror,-Wshadow]
                    MemOp op = get_memop(oi);
                          ^
  tcg/tcg.c:2437:12: note: previous declaration is here
    TCGOp *op;
           ^
  accel/tcg/tb-maint.c:245:18: error: declaration shadows a local variable [-Werror,-Wshadow]
        for (int i = 0; i < V_L2_SIZE; i++) {
                 ^
  accel/tcg/tb-maint.c:210:9: note: previous declaration is here
    int i;
        ^

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20230904161235.84651-2-philmd@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>

target/tricore: Change effective address (ea) to target_ulong

as this is an effective address and those cannot be signed,
it should not be a signed integer.

Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Message-ID: <20230913105326.40832-11-kbastian@mail.uni-paderborn.de>

target/tricore: Remove CSFRs from cpu.h

these are already defined in 'csfr.h.inc'. We don't need to duplicate
these registers.

Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Message-ID: <20230913105326.40832-10-kbastian@mail.uni-paderborn.de>

tests/tcg: Reset result register after each test

some insns use the result register implicitly as an input. Thus, we
could end up with data from the previous insn spilling over.

Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Message-ID: <20230913105326.40832-4-kbastian@mail.uni-paderborn.de>

hw/tricore: Log failing test in testdevice

Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Message-ID: <20230913105326.40832-3-kbastian@mail.uni-paderborn.de>

qobject atomics osdep: Make a few macros more hygienic

Variables declared in macros can shadow other variables.  Much of the
time, this is harmless, e.g.:

    #define _FDT(exp)                                                  \
        do {                                                           \
            int ret = (exp);                                           \
            if (ret < 0) {                                             \
                error_report("error creating device tree: %s: %s",   \
                        #exp, fdt_strerror(ret));                      \
                exit(1);                                               \
            }                                                          \
        } while (0)

Harmless shadowing in h_client_architecture_support():

        target_ulong ret;

        [...]

        ret = do_client_architecture_support(cpu, spapr, vec, fdt_bufsize);
        if (ret == H_SUCCESS) {
            _FDT((fdt_pack(spapr->fdt_blob)));
            [...]
        }

        return ret;

However, we can get in trouble when the shadowed variable is used in a
macro argument:

    #define QOBJECT(obj) ({                                 \
        typeof(obj) o = (obj);                              \
        o ? container_of(&(o)->base, QObject, base) : NULL; \
     })

QOBJECT(o) expands into

    ({
--->    typeof(o) o = (o);
        o ? container_of(&(o)->base, QObject, base) : NULL;
    })

Unintended variable name capture at --->.  We'd be saved by
-Winit-self.  But I could certainly construct more elaborate death
traps that don't trigger it.

To reduce the risk of trapping ourselves, we use variable names in
macros that no sane person would use elsewhere.  Here's our actual
definition of QOBJECT():

    #define QOBJECT(obj) ({                                         \
        typeof(obj) _obj = (obj);                                   \
        _obj ? container_of(&(_obj)->base, QObject, base) : NULL;   \
    })

Works well enough until we nest macro calls.  For instance, with

    #define qobject_ref(obj) ({                     \
        typeof(obj) _obj = (obj);                   \
        qobject_ref_impl(QOBJECT(_obj));            \
        _obj;                                       \
    })

the expression qobject_ref(obj) expands into

    ({
        typeof(obj) _obj = (obj);
        qobject_ref_impl(
            ({
--->            typeof(_obj) _obj = (_obj);
                _obj ? container_of(&(_obj)->base, QObject, base) : NULL;
            }));
        _obj;
    })

Unintended variable name capture at --->.

The only reliable way to prevent unintended variable name capture is
-Wshadow.

One blocker for enabling it is shadowing hiding in function-like
macros like

     qdict_put(dict, "name", qobject_ref(...))

qdict_put() wraps its last argument in QOBJECT(), and the last
argument here contains another QOBJECT().

Use dark preprocessor sorcery to make the macros that give us this
problem use different variable names on every call.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-ID: <20230921121312.1301864-8-armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

block: Clean up local variable shadowing

Local variables shadowing other local variables or parameters make the
code needlessly hard to understand.  Tracked down with -Wshadow=local.
Clean up: delete inner declarations when they are actually redundant,
else rename variables.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Acked-by: Ilya Dryomov <idryomov@gmail.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20230921121312.1301864-7-armbru@redhat.com>

block/vdi: Clean up local variable shadowing

Local variables shadowing other local variables or parameters make the
code needlessly hard to understand.  Tracked down with -Wshadow=local.
Clean up: delete inner declarations when they are actually redundant,
else rename variables.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20230921121312.1301864-6-armbru@redhat.com>

block/dirty-bitmap: Clean up local variable shadowing

Local variables shadowing other local variables or parameters make the
code needlessly hard to understand.  Tracked down with -Wshadow=local.
Clean up: rename both the pair of parameters and the pair of local
variables.  While there, move the local variables to function scope.

Suggested-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20230921121312.1301864-5-armbru@redhat.com>

ui: Clean up local variable shadowing

Local variables shadowing other local variables or parameters make the
code needlessly hard to understand.  Tracked down with -Wshadow=local.
Clean up: delete inner declarations when they are actually redundant,
else rename variables.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-ID: <20230921121312.1301864-4-armbru@redhat.com>

migration: Clean up local variable shadowing

Local variables shadowing other local variables or parameters make the
code needlessly hard to understand.  Tracked down with -Wshadow=local.
Clean up: delete inner declarations when they are actually redundant,
else rename variables.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
Message-ID: <20230921121312.1301864-3-armbru@redhat.com>

migration/rdma: Fix save_page method to fail on polling error

qemu_rdma_save_page() reports polling error with error_report(), then
succeeds anyway.  This is because the variable holding the polling
status *shadows* the variable the function returns.  The latter
remains zero.

Broken since day one, and duplicated more recently.

Fixes: 2da776db4846 (rdma: core logic)
Fixes: b390afd8c50b (migration/rdma: Fix out of order wrid)
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Li Zhijian <lizhijian@fujitsu.com>
Message-ID: <20230921121312.1301864-2-armbru@redhat.com>

accel/tcg: Always require can_do_io

Require i/o as the last insn of a TranslationBlock always,
not only with icount.  This is required for i/o that alters
the address space, such as a pci config space write.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1866
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

accel/tcg: Always set CF_LAST_IO with CF_NOIRQ

Without this we can get see loops through cpu_io_recompile,
in which the cpu makes no progress.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

accel/tcg: Improve setting of can_do_io at start of TB

Initialize can_do_io to true if this the TB has CF_LAST_IO
and will consist of a single instruction.  This avoids a
set to 0 followed immediately by a set to 1.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

accel/tcg: Track current value of can_do_io in the TB

Simplify translator_io_start by recording the current
known value of can_do_io within DisasContextBase.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

accel/tcg: Hoist CF_MEMI_ONLY check outside translation loop

The condition checked is loop invariant; check it only once.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

accel/tcg: Avoid load of icount_decr if unused

With CF_NOIRQ and without !CF_USE_ICOUNT, the load isn't used.
Avoid emitting it.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

tests/tcg/tricore: Extended and non-extened regs now match

RSx for d regs and e regs now use the same numbering. This makes sure
that mixing d and e registers in an insn test will not overwrite data
between registers.

Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Message-ID: <20230913105326.40832-2-kbastian@mail.uni-paderborn.de>

target/tricore: Fix FTOUZ being ISA v1.3.1 up

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Message-ID: <20230828112651.522058-12-kbastian@mail.uni-paderborn.de>

target/tricore: Replace cpu_*_code with translator_*

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Message-ID: <20230828112651.522058-11-kbastian@mail.uni-paderborn.de>

target/tricore: Swap src and dst reg for RCRR_INSERT

Acked-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Message-ID: <20230828112651.522058-10-kbastian@mail.uni-paderborn.de>

target/tricore: Fix RCPW/RRPW_INSERT insns for width = 0

we would crash if width was 0 for these insns, as tcg_gen_deposit() is
undefined for that case. For TriCore, width = 0 is a mov from the src reg
to the dst reg, so we special case this here.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Message-ID: <20230828112651.522058-9-kbastian@mail.uni-paderborn.de>

target/tricore: Implement hptof insn

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1667
Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Message-ID: <20230828112651.522058-8-kbastian@mail.uni-paderborn.de>

target/tricore: Implement ftohp insn

reported in https://gitlab.com/qemu-project/qemu/-/issues/1667

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Message-ID: <20230828112651.522058-7-kbastian@mail.uni-paderborn.de>

target/tricore: Clarify special case for FTOUZ insn

this is not something other ISAs do, so clarify it with a comment.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Message-ID: <20230828112651.522058-6-kbastian@mail.uni-paderborn.de>

target/tricore: Implement FTOU insn

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Message-ID: <20230828112651.522058-5-kbastian@mail.uni-paderborn.de>

target/tricore: Correctly handle FPU RM from PSW

when we reconstructed PSW using psw_read(), we were trying to clear the
cached USB bits out of env->PSW. The mask was wrong and we would clear
PSW.RM as well.

when we write the PSW using psw_write() we update the rounding modes in
env->fp_status for softfloat. The order of bits used by TriCore is not
the one used by softfloat.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Message-ID: <20230828112651.522058-4-kbastian@mail.uni-paderborn.de>

target/tricore: Implement CRCN insn

reported in https://gitlab.com/qemu-project/qemu/-/issues/1667

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Message-ID: <20230828112651.522058-3-kbastian@mail.uni-paderborn.de>

tests/tcg/tricore: Bump cpu to tc37x

we don't want to exclude ISA v1.6.2 insns from our tests.

Acked-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Message-Id: <20230828112651.522058-2-kbastian@mail.uni-paderborn.de>

migration: Move return path cleanup to main migration thread

Now that the return path thread is allowed to finish during a paused
migration, we can move the cleanup of the QEMUFiles to the main
migration thread.

Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-ID: <20230918172822.19052-9-farosas@suse.de>

migration: Replace the return path retry logic

Replace the return path retry logic with finishing and restarting the
thread. This fixes a race when resuming the migration that leads to a
segfault.

Currently when doing postcopy we consider that an IO error on the
return path file could be due to a network intermittency. We then keep
the thread alive but have it do cleanup of the 'from_dst_file' and
wait on the 'postcopy_pause_rp' semaphore. When the user issues a
migrate resume, a new return path is opened and the thread is allowed
to continue.

There's a race condition in the above mechanism. It is possible for
the new return path file to be setup *before* the cleanup code in the
return path thread has had a chance to run, leading to the *new* file
being closed and the pointer set to NULL. When the thread is released
after the resume, it tries to dereference 'from_dst_file' and crashes:

Thread 7 "return path" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffd1dbf700 (LWP 9611)]
0x00005555560e4893 in qemu_file_get_error_obj (f=0x0, errp=0x0) at ../migration/qemu-file.c:154
154         return f->last_error;

(gdb) bt
 #0  0x00005555560e4893 in qemu_file_get_error_obj (f=0x0, errp=0x0) at ../migration/qemu-file.c:154
 #1  0x00005555560e4983 in qemu_file_get_error (f=0x0) at ../migration/qemu-file.c:206
 #2  0x0000555555b9a1df in source_return_path_thread (opaque=0x555556e06000) at ../migration/migration.c:1876
 #3  0x000055555602e14f in qemu_thread_start (args=0x55555782e780) at ../util/qemu-thread-posix.c:541
 #4  0x00007ffff38d76ea in start_thread (arg=0x7fffd1dbf700) at pthread_create.c:477
 #5  0x00007ffff35efa6f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Here's the race (important bit is open_return_path happening before
migration_release_dst_files):

migration                 | qmp                         | return path
--------------------------+-----------------------------+---------------------------------
    qmp_migrate_pause()
     shutdown(ms->to_dst_file)
      f->last_error = -EIO
migrate_detect_error()
 postcopy_pause()
  set_state(PAUSED)
  wait(postcopy_pause_sem)
    qmp_migrate(resume)
    migrate_fd_connect()
     resume = state == PAUSED
     open_return_path <-- TOO SOON!
     set_state(RECOVER)
     post(postcopy_pause_sem)
(incoming closes to_src_file)
res = qemu_file_get_error(rp)
migration_release_dst_files()
ms->rp_state.from_dst_file = NULL
  post(postcopy_pause_rp_sem)
postcopy_pause_return_path_thread()
  wait(postcopy_pause_rp_sem)
rp = ms->rp_state.from_dst_file
goto retry
qemu_file_get_error(rp)
SIGSEGV
-------------------------------------------------------------------------------------------

We can keep the retry logic without having the thread alive and
waiting. The only piece of data used by it is the 'from_dst_file' and
it is only allowed to proceed after a migrate resume is issued and the
semaphore released at migrate_fd_connect().

Move the retry logic to outside the thread by waiting for the thread
to finish before pausing the migration.

Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-ID: <20230918172822.19052-8-farosas@suse.de>

migration: Consolidate return path closing code

We'll start calling the await_return_path_close_on_source() function
from other parts of the code, so move all of the related checks and
tracepoints into it.

Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-ID: <20230918172822.19052-7-farosas@suse.de>

migration: Remove redundant cleanup of postcopy_qemufile_src

This file is owned by the return path thread which is already doing
cleanup.

Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-ID: <20230918172822.19052-6-farosas@suse.de>

migration: Fix possible race when shutting down to_dst_file

It's not safe to call qemu_file_shutdown() on the to_dst_file without
first checking for the file's presence under the lock. The cleanup of
this file happens at postcopy_pause() and migrate_fd_cleanup() which
are not necessarily running in the same thread as migrate_fd_cancel().

Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-ID: <20230918172822.19052-5-farosas@suse.de>

migration: Fix possible races when shutting down the return path

We cannot call qemu_file_shutdown() on the return path file without
taking the file lock. The return path thread could be running it's
cleanup code and have just cleared the from_dst_file pointer.

Checking ms->to_dst_file for errors could also race with
migrate_fd_cleanup() which clears the to_dst_file pointer.

Protect both accesses by taking the file lock.

This was caught by inspection, it should be rare, but the next patches
will start calling this code from other places, so let's do the
correct thing.

Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-ID: <20230918172822.19052-4-farosas@suse.de>

migration: Fix possible race when setting rp_state.error

We don't need to set the rp_state.error right after a shutdown because
qemu_file_shutdown() always sets the QEMUFile error, so the return
path thread would have seen it and set the rp error itself.

Setting the error outside of the thread is also racy because the
thread could clear it after we set it.

Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-ID: <20230918172822.19052-3-farosas@suse.de>

migration: Fix race that dest preempt thread close too early

We hit intermit CI issue on failing at migration-test over the unit test
preempt/plain:

qemu-system-x86_64: Unable to read from socket: Connection reset by peer
Memory content inconsistency at 5b43000 first_byte = bd last_byte = bc current = 4f hit_edge = 1
**
ERROR:../tests/qtest/migration-test.c:300:check_guests_ram: assertion failed: (bad == 0)
(test program exited with status code -6)

Fabiano debugged into it and found that the preempt thread can quit even
without receiving all the pages, which can cause guest not receiving all
the pages and corrupt the guest memory.

To make sure preempt thread finished receiving all the pages, we can rely
on the page_requested_count being zero because preempt channel will only
receive requested page faults. Note, not all the faulted pages are required
to be sent via the preempt channel/thread; imagine the case when a
requested page is just queued into the background main channel for
migration, the src qemu will just still send it via the background channel.

Here instead of spinning over reading the count, we add a condvar so the
main thread can wait on it if that unusual case happened, without burning
the cpu for no good reason, even if the duration is short; so even if we
spin in this rare case is probably fine.  It's just better to not do so.

The condvar is only used when that special case is triggered.  Some memory
ordering trick is needed to guarantee it from happening (against the
preempt thread status field), so the main thread will always get a kick
when that triggers correctly.

Closes: https://gitlab.com/qemu-project/qemu/-/issues/1886
Debugged-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-ID: <20230918172822.19052-2-farosas@suse.de>

Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging

* new round of audio cleanups
* various shadowed local variable fixes in vl, mptsas, pm_smbus, target/i386
* remove deprecated pc-i440fx-1.4 up to pc-i440fx-1.7
* remove PCI drivers from 128K bios.bin
* remove unused variable in user-exec-stub.c
* small fixes for ui/vnc
* scsi-disk: Disallow block sizes smaller than 512 [CVE-2023-42467]

# -----BEGIN PGP SIGNATURE-----
#
# iQFIBAABCAAyFiEE8TM4V0tmI4mGbHaCv/vSX3jHroMFAmUTDaoUHHBib256aW5p
# QHJlZGhhdC5jb20ACgkQv/vSX3jHroMvEgf+NrSaP4pmHrYcVtm43fnKXoLHFrCx
# KYfoK9Lke/DDkTff6rrcfW/Wyqid6Pp9Ch4Rrpr/X71X5gi+c6xb5klC8cpSfLg4
# gtuGctj7WL7KR/067EsLqHvzBob/iebFhZwhtsBrI+z65X+J9pOK78efBTdhezq4
# EEHTWohMAg1I/MWBK5VnOk2fI4+9z9K9zP5AtWmJzwwJkQUoEyl+YDkVmIhMYoGn
# CapRO7i2wIvtoF4wuQUCGsOLmrcWTvRIOcV13k3b6PYCPC40/N9AOpiiyg3XqNah
# UKKM9CcgVnCzCc4Jar2QD+MzkTDxhmQSyLFJgtzrW7CQSE5YB3sUHj3CXg==
# =8nvs
# -----END PGP SIGNATURE-----
# gpg: Signature made Tue 26 Sep 2023 12:58:18 EDT
# gpg:                using RSA key F13338574B662389866C7682BFFBD25F78C7AE83
# gpg:                issuer "pbonzini@redhat.com"
# gpg: Good signature from "Paolo Bonzini <bonzini@gnu.org>" [full]
# gpg:                 aka "Paolo Bonzini <pbonzini@redhat.com>" [full]
# Primary key fingerprint: 46F5 9FBD 57D6 12E7 BFD4  E2F7 7E15 100C CD36 69B1
#      Subkey fingerprint: F133 3857 4B66 2389 866C  7682 BFFB D25F 78C7 AE83

* tag 'for-upstream' of https://gitlab.com/bonzini/qemu:
  audio: remove shadowed locals
  compiler: introduce QEMU_ANNOTATE
  block: mark mixed functions that can suspend
  target/i386/svm_helper: eliminate duplicate local variable
  target/i386/seg_helper: remove shadowed variable
  target/i386/seg_helper: introduce tss_set_busy
  target/i386/translate: avoid shadowed local variables
  target/i386/cpu: avoid shadowed local variables
  target/i386/kvm: eliminate shadowed local variables
  m48t59-test: avoid possible overflow on ABS
  pm_smbus: rename variable to avoid shadowing
  mptsas: avoid shadowed local variables
  ui/vnc: fix handling of VNC_FEATURE_XVP
  ui/vnc: fix debug output for invalid audio message
  vl: remove shadowed local variables
  hw/scsi/scsi-disk: Disallow block sizes smaller than 512 [CVE-2023-42467]
  user-exec-stub: remove unused variable
  seabios: remove PCI drivers from bios.bin
  pc_piix: remove pc-i440fx-1.4 up to pc-i440fx-1.7

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

scripts/analyse-locks-simpletrace.py: changed iteritems() to items()

Python 3 removed `dict.iteritems()` in favor of `dict.items()`. This
means the script currently doesn't work on Python 3.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Mads Ynddal <m.ynddal@samsung.com>
Message-id: 20230926103436.25700-15-mads@ynddal.dk
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

MAINTAINERS: add maintainer of simpletrace.py

In my work to refactor simpletrace.py, I noticed that there's no
maintainer of it, and has the status of "odd fixes". I'm using it from
time to time, so I'd like to maintain the script.

I've added myself as reviewer under "Tracing" to be informed of changes
that might affect simpletrace.py.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Mads Ynddal <m.ynddal@samsung.com>
Message-id: 20230926103436.25700-14-mads@ynddal.dk
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

simpletrace: added simplified Analyzer2 class

By moving the dynamic argument construction to keyword-arguments,
we can remove all of the specialized handling, and streamline it.
If a tracing method wants to access these, they can define the
kwargs, or ignore it be placing `**kwargs` at the end of the
function's arguments list.

Added deprecation warning to Analyzer class to make users aware
of the Analyzer2 class. No removal date is planned.

Signed-off-by: Mads Ynddal <m.ynddal@samsung.com>
Message-id: 20230926103436.25700-13-mads@ynddal.dk
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

simpletrace: move event processing to Analyzer class

Moved event processing to the Analyzer class to separate specific analyzer
logic (like caching and function signatures) from the _process function.
This allows for new types of Analyzer-based subclasses without changing
the core code.

Note, that the fn_cache is important for performance in cases where the
analyzer is branching away from the catch-all a lot. The cache has no
measurable performance penalty.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Mads Ynddal <m.ynddal@samsung.com>
Message-id: 20230926103436.25700-12-mads@ynddal.dk
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

simpletrace: move logic of process into internal function

To avoid duplicate code depending on input types and to better handle
open/close of log with a context-manager, we move the logic of process into
_process.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Mads Ynddal <m.ynddal@samsung.com>
Message-id: 20230926103436.25700-11-mads@ynddal.dk
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

simpletrace: refactor to separate responsibilities

Moved event_mapping and event_id_to_name down one level in the function
call-stack to keep variable instantiation and usage closer (`process`
and `run` has no use of the variables; `read_trace_records` does).

Instead of passing event_mapping and event_id_to_name to the bottom of
the call-stack, we move their use to `read_trace_records`. This
separates responsibility and ownership of the information.

`read_record` now just reads the arguments from the file-object by
knowning the total number of bytes. Parsing it to specific arguments is
moved up to `read_trace_records`.

Special handling of dropped events removed, as they can be handled
by the general code.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Mads Ynddal <m.ynddal@samsung.com>
Message-id: 20230926103436.25700-10-mads@ynddal.dk
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

simpletrace: made Analyzer into context-manager

Instead of explicitly calling `begin` and `end`, we can change the class
to use the context-manager paradigm. This is mostly a styling choice,
used in modern Python code. But it also allows for more advanced analyzers
to handle exceptions gracefully in the `__exit__` method (not
demonstrated here).

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Mads Ynddal <m.ynddal@samsung.com>
Message-id: 20230926103436.25700-9-mads@ynddal.dk
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

simpletrace: define exception and add handling

Define `SimpleException` to differentiate our exceptions from generic
exceptions (IOError, etc.). Adapted simpletrace to support this and
output to stderr.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Mads Ynddal <m.ynddal@samsung.com>
Message-id: 20230926103436.25700-8-mads@ynddal.dk
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

simpletrace: improved error handling on struct unpack

A failed call to `read_header` wouldn't be handled the same for the two
different code paths (one path would try to use `None` as a list).
Changed to raise exception to be handled centrally. This also allows for
easier unpacking, as errors has been filtered out.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Mads Ynddal <m.ynddal@samsung.com>
Message-id: 20230926103436.25700-7-mads@ynddal.dk
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>