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.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>
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/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/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.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.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/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/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>
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/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/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>
Paolo Bonzini [Wed, 27 Sep 2023 13:48:31 +0000 (15:48 +0200)]
meson: clean up static_library keyword arguments
These are either built because they are dependencies of other targets,
or not needed at all because they are used via extract_objects().
Mark them as "build_by_default: false"; if applicable, mark them
as "fa" so that -Wl,--whole-archive does not interact with the
linker script used for fuzzing.
(The "fa" hack is brittle; updating to Meson 1.1 would allow using
declare_dependency(objects: ...) instead).
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1044 Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Paolo Bonzini [Tue, 26 Sep 2023 10:31:40 +0000 (12:31 +0200)]
make-release: do not ship dtc sources
A new enough libfdt is included in all of Debian 11, Ubuntu 20.04
and MSYS2. It has also been included for several minor releases
in Fedora and openSUSE Leap, as well as in CentOS. Therefore
there is no need anymore to ship the sources together with the QEMU
tarballs.
Keep the wrap file so that it can be used with --enable-download,
but do not ship the sources anymore with either archive-source.sh
or make-release.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Akihiko Odaki [Thu, 1 Dec 2022 10:27:28 +0000 (19:27 +0900)]
accel/kvm/kvm-all: Handle register access errors
A register access error typically means something seriously wrong
happened so that anything bad can happen after that and recovery is
impossible.
Even failing one register access is catastorophic as
architecture-specific code are not written so that it torelates such
failures.
Make sure the VM stop and nothing worse happens if such an error occurs.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Message-ID: <20221201102728.69751-1-akihiko.odaki@daynix.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Our linker script for optionroms specifies only the placement of the
.text section, leaving the linker free to place the remaining sections
at arbitrary places in the file.
Since at least binutils 2.39, the .note.gnu.build-id section is now
being placed at the start of the file, which causes label addresses to
be shifted. For linuxboot_dma.bin that means that the PnP header
(among others) will not be found when determining the type of ROM at
optionrom_setup():
(0x1c is the label _pnph, where the magic "PnP" is)
Using a freshly built linuxboot_dma.bin ROM results in a broken boot:
SeaBIOS (version rel-1.16.2-0-gea1b7a073390-prebuilt.qemu.org)
Booting from Hard Disk...
Boot failed: could not read the boot disk
Booting from Floppy...
Boot failed: could not read the boot disk
No bootable device.
We're not using the build-id section, so pass the --build-id=none
option to the linker to remove it entirely.
Note: In theory, this same issue could happen with any other
section. The ideal solution would be to have all unused sections
discarded in the linker script. However that would be a larger change,
specially for the pvh rom which uses the .bss and COMMON sections so
I'm addressing only the immediate issue here.
Reported-by: Vasiliy Ulyanov <vulyanov@suse.de> Signed-off-by: Fabiano Rosas <farosas@suse.de> Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20230926192502.15986-1-farosas@suse.de> Cc: stable@vger.kernel.org Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
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():
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>
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>
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>
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>
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>
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: 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>
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.
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>
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>
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):
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>
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: 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>
Peter Xu [Mon, 18 Sep 2023 17:28:15 +0000 (14:28 -0300)]
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>
Stefan Hajnoczi [Wed, 27 Sep 2023 17:55:18 +0000 (13:55 -0400)]
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]
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>
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>
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>
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>
The call to `getargspec` was deprecated and in Python 3.11 it has been
removed in favor of `getfullargspec`. `getfullargspec` is compatible
with QEMU's requirement of at least Python version 3.6.
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-6-mads@ynddal.dk Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
simpletrace: improve parsing of sys.argv; fix files never closed.
The arguments extracted from `sys.argv` named and unpacked to make it
clear what the arguments are and what they're used for.
The two input files were opened, but never explicitly closed. File usage
changed to use `with` statement to take care of this. At the same time,
ownership of the file-object is moved up to `run` function. Added option
to process to support file-like objects.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Mads Ynddal <m.ynddal@samsung.com>
Message-id: 20230926103436.25700-4-mads@ynddal.dk Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Paolo Bonzini [Wed, 12 Apr 2023 09:23:00 +0000 (11:23 +0200)]
block: mark mixed functions that can suspend
The marking should be extended transitively to all functions that call
these ones, so that static analysis can be done much more efficiently.
However, this is a start and makes it possible to use vrc's path-based
searches to find potential bugs where coroutine_fns call blocking functions.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Paolo Bonzini [Mon, 25 Sep 2023 10:27:24 +0000 (12:27 +0200)]
m48t59-test: avoid possible overflow on ABS
Originally meant to avoid a shadowed variable "s", which was fixed by
renaming the outer declaration to "qts". Avoid the chance of an overflow
in the computation of ABS(t - s).
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* tag 'pull-nbd-2023-09-25' of https://repo.or.cz/qemu/ericb:
nbd/server: Refactor handling of command sanity checks
nbd: Prepare for 64-bit request effect lengths
nbd: Add types for extended headers
nbd/client: Pass mode through to nbd_send_request
nbd: Replace bool structured_reply with mode enum
iotests: improve 'not run' message for nbd-multiconn test
iotests: use TEST_IMG_FILE instead of TEST_IMG in _require_large_file
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Paolo Bonzini [Mon, 25 Sep 2023 11:05:58 +0000 (13:05 +0200)]
ui/vnc: fix handling of VNC_FEATURE_XVP
VNC_FEATURE_XVP was not shifted left before adding it to vs->features,
so it was never enabled; but it was also checked the wrong way with
a logical AND instead of vnc_has_feature. Fix both places.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
in the code here (e.g. in scsi_disk_emulate_mode_sense()), so if
the blocksize is smaller than BDRV_SECTOR_SIZE (=512), this crashes
with a division by 0 exception. Thus disallow block sizes of 256
bytes to avoid this situation.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1813
CVE: 2023-42467 Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-ID: <20230925091854.49198-1-thuth@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Paolo Bonzini [Fri, 15 Sep 2023 11:57:11 +0000 (13:57 +0200)]
pc_piix: remove pc-i440fx-1.4 up to pc-i440fx-1.7
These are the last users of the 128K SeaBIOS blob in the i440FX family.
Removing them allows us to drop PCI support from the 128K blob,
thus making it easier to update SeaBIOS to newer versions.
Reviewed-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
projects / mirror_qemu.git / log