-
Notifications
You must be signed in to change notification settings - Fork 28
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Full system hooks #8
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andreafioraldi
pushed a commit
that referenced
this pull request
Jul 6, 2022
There is such error info when running linux kernel: tcg_handle_interrupt: assertion failed: (qemu_mutex_iothread_locked()). calling stack: #0 in raise () at /lib64/libc.so.6 #1 in abort () at /lib64/libc.so.6 #2 in g_assertion_message_expr.cold () at /lib64/libglib-2.0.so.0 #3 in g_assertion_message_expr () at /lib64/libglib-2.0.so.0 #4 in tcg_handle_interrupt (cpu=0x632000030800, mask=2) at ../accel/tcg/tcg-accel-ops.c:79 #5 in cpu_interrupt (cpu=0x632000030800, mask=2) at ../softmmu/cpus.c:248 #6 in loongarch_cpu_set_irq (opaque=0x632000030800, irq=11, level=0) at ../target/loongarch/cpu.c:100 #7 in helper_csrwr_ticlr (env=0x632000039440, val=1) at ../target/loongarch/csr_helper.c:85 #8 in code_gen_buffer () #9 in cpu_tb_exec (cpu=0x632000030800, itb=0x7fff946ac280, tb_exit=0x7ffe4fcb6c30) at ../accel/tcg/cpu-exec.c:358 Add mutex iothread lock around loongarch_cpu_set_irq in csrwr_ticlr() to fix the bug. Signed-off-by: Xiaojuan Yang <yangxiaojuan@loongson.cn> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-Id: <20220701093407.2150607-10-yangxiaojuan@loongson.cn> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Since the MacOS SCSI implementation is quite old (and Apple added some firmware customisations to their drives for m68k Macs) there is need to add a mechanism to correctly handle Apple-specific quirks. Add a new quirks bitmap to SCSIDiskState that can be used to enable these features as required. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Reviewed-by: Laurent Vivier <laurent@vivier.eu> Message-Id: <20220622105314.802852-2-mark.cave-ayland@ilande.co.uk> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
One of the mechanisms MacOS uses to identify CDROM drives compatible with MacOS is to send a custom MODE SELECT command for page 0x30 to the drive. The response to this is a hard-coded manufacturer string which must match in order for the CDROM to be usable within MacOS. Add an implementation of the MODE SELECT page 0x30 response guarded by a newly defined SCSI_DISK_QUIRK_MODE_PAGE_APPLE_VENDOR quirk bit so that CDROM drives attached to non-Apple machines function exactly as before. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Reviewed-by: Laurent Vivier <laurent@vivier.eu> Message-Id: <20220622105314.802852-3-mark.cave-ayland@ilande.co.uk> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
…or scsi-cd devices By default quirk_mode_page_apple_vendor should be enabled for all scsi-cd devices connected to the q800 machine to enable MacOS to detect and use them. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Message-Id: <20220622105314.802852-4-mark.cave-ayland@ilande.co.uk> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
…tosh During SCSI bus enumeration A/UX sends a MODE SENSE command to the CDROM with the DBD bit unset and expects the response to include a block descriptor. As per the latest SCSI documentation, QEMU currently force-disables the block descriptor for CDROM devices but the A/UX driver expects the requested block descriptor to be returned. If the block descriptor is not returned in the response then A/UX becomes confused, since the block descriptor returned in the MODE SENSE response is used to generate a subsequent MODE SELECT command which is then invalid. Add a new SCSI_DISK_QUIRK_MODE_SENSE_ROM_USE_DBD quirk to allow this behaviour to be enabled as required. Note that an additional workaround is required for the previous SCSI_DISK_QUIRK_MODE_PAGE_APPLE_VENDOR quirk which must never return a block descriptor even though the DBD bit is left unset. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Message-Id: <20220622105314.802852-5-mark.cave-ayland@ilande.co.uk> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
…or scsi-cd devices By default quirk_mode_sense_rom_use_dbd should be enabled for all scsi-cd devices connected to the q800 machine to correctly report the CDROM block descriptor back to A/UX. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Reviewed-by: Laurent Vivier <laurent@vivier.eu> Message-Id: <20220622105314.802852-6-mark.cave-ayland@ilande.co.uk> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
…for Macintosh Both MacOS and A/UX make use of vendor-specific MODE SELECT commands with PF=0 to identify SCSI devices: - MacOS sends a MODE SELECT command with PF=0 for the MODE_PAGE_VENDOR_SPECIFIC (0x0) mode page containing 2 bytes before initialising a disk - A/UX (installed on disk) sends a MODE SELECT command with PF=0 during SCSI bus enumeration, and gets stuck in an infinite loop if it fails Add a new SCSI_DISK_QUIRK_MODE_PAGE_VENDOR_SPECIFIC_APPLE quirk to allow both PF=0 MODE SELECT commands and implement a MODE_PAGE_VENDOR_SPECIFIC (0x0) mode page which is compatible with MacOS. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Message-Id: <20220622105314.802852-7-mark.cave-ayland@ilande.co.uk> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
…c_apple for scsi devices By default quirk_mode_page_vendor_specific_apple should be enabled for both scsi-hd and scsi-cd devices to allow MacOS to format SCSI disk devices, and A/UX to enumerate SCSI CDROM devices succesfully without getting stuck in a loop. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Message-Id: <20220622105314.802852-8-mark.cave-ayland@ilande.co.uk> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
When initialising a drive ready to install MacOS, Apple HD SC Setup first attempts to format the drive. Add a simple FORMAT UNIT command which simply returns success to allow the format to succeed. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Message-Id: <20220622105314.802852-9-mark.cave-ayland@ilande.co.uk> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
When A/UX configures the CDROM device it sends a truncated MODE SELECT request for page 1 (MODE_PAGE_R_W_ERROR) which is only 6 bytes in length rather than 10. This seems to be due to bug in Apple's code which calculates the CDB message length incorrectly. The work at [1] suggests that this truncated request is accepted on real hardware whereas in QEMU it generates an INVALID_PARAM_LEN sense code which causes A/UX to get stuck in a loop retrying the command in an attempt to succeed. Alter the mode page request length check so that truncated requests are allowed if the SCSI_DISK_QUIRK_MODE_PAGE_TRUNCATED quirk is enabled, whilst also adding a trace event to enable the condition to be detected. [1] https://68kmla.org/bb/index.php?threads/scsi2sd-project-anyone-interested.29040/page-7#post-316444 Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Message-Id: <20220622105314.802852-10-mark.cave-ayland@ilande.co.uk> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
…scsi-cd devices By default quirk_mode_page_truncated should be enabled for all scsi-cd devices connected to the q800 machine to allow A/UX to enumerate SCSI CDROM devices without hanging. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Message-Id: <20220622105314.802852-11-mark.cave-ayland@ilande.co.uk> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
…r CDROM drives A/UX sends a MODE_PAGE_R_W_ERROR command with the AWRE bit set to 0 when enumerating CDROM drives. Since the bit is currently hardcoded to 1 then indicate that the AWRE bit can be changed (even though we don't care about the value) so that the MODE_PAGE_R_W_ERROR page can be set successfully. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Message-Id: <20220622105314.802852-12-mark.cave-ayland@ilande.co.uk> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The MODE SELECT command can contain an optional block descriptor that can be used to set the device block size. If the block descriptor is present then update the block size on the SCSI device accordingly. This allows CDROMs to be used with A/UX which requires a CDROM drive which is capable of switching from a 2048 byte sector size to a 512 byte sector size. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Message-Id: <20220622105314.802852-13-mark.cave-ayland@ilande.co.uk> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The Apple HD SC Setup program uses a SCSI INQUIRY command to check that any SCSI hard disks detected match a whitelist of vendors and products before allowing the "Initialise" button to prepare an empty disk. Add known-good default vendor and product information using the existing compat_prop mechanism so the user doesn't have to use long command lines to set the qdev properties manually. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Reviewed-by: Laurent Vivier <laurent@vivier.eu> Message-Id: <20220622105314.802852-14-mark.cave-ayland@ilande.co.uk> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The MacOS CDROM driver uses a SCSI INQUIRY command to check that any SCSI CDROMs detected match a whitelist of vendors and products before adding them to the list of available devices. Add known-good default vendor and product information using the existing compat_prop mechanism so the user doesn't have to use long command lines to set the qdev properties manually. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Reviewed-by: Laurent Vivier <laurent@vivier.eu> Message-Id: <20220622105314.802852-15-mark.cave-ayland@ilande.co.uk> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The option generates a lot of warnings for integers casted to pointers, for example: /home/pbonzini/work/upstream/qemu/pc-bios/s390-ccw/dasd-ipl.c:174:19: warning: array subscript 0 is outside array bounds of ‘CcwSeekData[0]’ [-Warray-bounds] 174 | seekData->cyl = 0x00; | ~~~~~~~~~~~~~~^~~~~~ Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
MIPS patches queue - Cavium Octeon MIPS extension and CPU model (Pavel Dovgalyuk) - Semihosting cleanup (Richard Henderson) # gpg: Signature made Tue 12 Jul 2022 21:52:52 BST # gpg: using RSA key FAABE75E12917221DCFD6BB2E3E32C2CDEADC0DE # gpg: Good signature from "Philippe Mathieu-Daudé (F4BUG) <f4bug@amsat.org>" [full] # Primary key fingerprint: FAAB E75E 1291 7221 DCFD 6BB2 E3E3 2C2C DEAD C0DE * tag 'mips-20220712' of https://github.com/philmd/qemu: target/mips: Remove GET_TARGET_STRING and FREE_TARGET_STRING target/mips: Simplify UHI_argnlen and UHI_argn semihosting: Remove qemu_semihosting_log_out target/mips: Use error_report for UHI_assert target/mips: Avoid qemu_semihosting_log_out for UHI_plog target/mips: Use semihosting/syscalls.h target/mips: Drop link syscall from semihosting target/mips: Create report_fault for semihosting target/mips: introduce Cavium Octeon CPU model target/mips: implement Octeon-specific arithmetic instructions target/mips: implement Octeon-specific BBIT instructions target/mips: introduce decodetree structure for Cavium Octeon extension Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
…aging Darwin patches: - Enable RDTSCP support on HVF - ui/cocoa: Take refresh rate into account Few buildsys fixes: - Restrict TCG to emulation - Remove a unused-but-set-variable warning - Allow running Avocado from pseudo-"in source tree" builds # gpg: Signature made Tue 12 Jul 2022 23:14:37 BST # gpg: using RSA key FAABE75E12917221DCFD6BB2E3E32C2CDEADC0DE # gpg: Good signature from "Philippe Mathieu-Daudé (F4BUG) <f4bug@amsat.org>" [full] # Primary key fingerprint: FAAB E75E 1291 7221 DCFD 6BB2 E3E3 2C2C DEAD C0DE * tag 'darwin-20220712' of https://github.com/philmd/qemu: avocado: Fix BUILD_DIR if it's equal to SOURCE_DIR ui/cocoa: Take refresh rate into account ui/cocoa: Fix switched_to_fullscreen warning configure: Restrict TCG to emulation hvf: Enable RDTSCP support Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
…nto staging qga-win32-pull-2022-07-13 # gpg: Signature made Wed 13 Jul 2022 11:13:32 BST # gpg: using RSA key C2C2C109EA43C63C1423EB84EF5D5E8161BA84E7 # gpg: Good signature from "Kostiantyn Kostiuk (Upstream PR sign) <kkostiuk@redhat.com>" [unknown] # gpg: WARNING: This key is not certified with a trusted signature! # gpg: There is no indication that the signature belongs to the owner. # Primary key fingerprint: C2C2 C109 EA43 C63C 1423 EB84 EF5D 5E81 61BA 84E7 * tag 'qga-win32-pull-2022-07-13' of github.com:kostyanf14/qemu: qga: add command 'guest-get-cpustats' qapi: Avoid generating C identifier 'linux' MAINTAINERS: Add myself as Guest Agent reviewer Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
In order to correctly report secure boot running firmware the values of certain registers must be set. We don't yet have documentation from ASPEED on what they mean. The meaning is inferred from u-boot's use of them. Introduce properties so the settings can be configured per-machine. Reviewed-by: Peter Delevoryas <pdel@fb.com> Tested-by: Peter Delevoryas <pdel@fb.com> Signed-off-by: Joel Stanley <joel@jms.id.au> Message-Id: <20220628154740.1117349-4-clg@kaod.org> Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Peter Delevoryas <pdel@fb.com> Reviewed-by: Titus Rwantare <titusr@google.com> Message-Id: <20220701000626.77395-2-me@pjd.dev> Signed-off-by: Cédric Le Goater <clg@kaod.org>
This commit adds a passthrough for PMBUS_IC_DEVICE_ID to allow Renesas voltage regulators to return the integrated circuit device ID if they would like to. The behavior is very device specific, so it hasn't been added to the general PMBUS model. Additionally, if the device ID hasn't been set, then the voltage regulator will respond with the error byte value. The guest error message will change slightly for IC_DEVICE_ID with this commit. Signed-off-by: Peter Delevoryas <pdel@fb.com> Reviewed-by: Titus Rwantare <titusr@google.com> Message-Id: <20220701000626.77395-3-me@pjd.dev> Signed-off-by: Cédric Le Goater <clg@kaod.org>
This adds the ISL69259, using all the same functionality as the existing ISL69260 but overriding the IC_DEVICE_ID. Signed-off-by: Peter Delevoryas <pdel@fb.com> Reviewed-by: Titus Rwantare <titusr@google.com> Message-Id: <20220701000626.77395-4-me@pjd.dev> Signed-off-by: Cédric Le Goater <clg@kaod.org>
To support multiple SoC's running simultaneously, we need a unique name for each RAM region. DRAM is created by the machine, but SRAM is created by the SoC, since in hardware it is part of the SoC's internals. We need a way to uniquely identify each SRAM region though, for VM migration. Since each of the SoC's CPU's has an index which identifies it uniquely from other CPU's in the machine, we can use the index of any of the CPU's in the SoC to uniquely identify differentiate the SRAM name from other SoC SRAM's. In this change, I just elected to use the index of the first CPU in each SoC. Signed-off-by: Peter Delevoryas <peter@pjd.dev> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20220705191400.41632-3-peter@pjd.dev> Signed-off-by: Cédric Le Goater <clg@kaod.org>
This change moves the code that connects the SoC UART's to serial_hd's to the machine. It makes each UART a proper child member of the SoC, and then allows the machine to selectively initialize the chardev for each UART with a serial_hd. This should preserve backwards compatibility, but also allow multi-SoC boards to completely change the wiring of serial devices from the command line to specific SoC UART's. This also removes the uart-default property from the SoC, since the SoC doesn't need to know what UART is the "default" on the machine anymore. I tested this using the images and commands from the previous refactoring, and another test image for the ast1030: wget https://github.com/facebook/openbmc/releases/download/v2021.49.0/fuji.mtd wget https://github.com/facebook/openbmc/releases/download/v2021.49.0/wedge100.mtd wget https://github.com/peterdelevoryas/OpenBIC/releases/download/oby35-cl-2022.13.01/Y35BCL.elf Fuji uses UART1: qemu-system-arm -machine fuji-bmc \ -drive file=fuji.mtd,format=raw,if=mtd \ -nographic ast2600-evb uses uart-default=UART5: qemu-system-arm -machine ast2600-evb \ -drive file=fuji.mtd,format=raw,if=mtd \ -serial null -serial mon:stdio -display none Wedge100 uses UART3: qemu-system-arm -machine palmetto-bmc \ -drive file=wedge100.mtd,format=raw,if=mtd \ -serial null -serial null -serial null \ -serial mon:stdio -display none AST1030 EVB uses UART5: qemu-system-arm -machine ast1030-evb \ -kernel Y35BCL.elf -nographic Fixes: 6827ff2 ("hw: aspeed: Init all UART's with serial devices") Signed-off-by: Peter Delevoryas <peter@pjd.dev> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20220705191400.41632-4-peter@pjd.dev> Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Peter Delevoryas <peter@pjd.dev> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20220705191400.41632-5-peter@pjd.dev> Signed-off-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Peter Delevoryas <peter@pjd.dev> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20220705191400.41632-6-peter@pjd.dev> Signed-off-by: Cédric Le Goater <clg@kaod.org>
You can test booting the BMC with both '-device loader' and '-drive file'. This is necessary because of how the fb-openbmc boot sequence works (jump to 0x20000000 after U-Boot SPL). wget https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd qemu-system-arm -machine fby35 -nographic \ -device loader,file=fby35.mtd,addr=0,cpu-num=0 -drive file=fby35.mtd,format=raw,if=mtd Signed-off-by: Peter Delevoryas <peter@pjd.dev> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20220705191400.41632-7-peter@pjd.dev> Signed-off-by: Cédric Le Goater <clg@kaod.org>
The BMC boots from the first flash device by fetching instructions from the flash contents. Add an alias region on 0x0 for this purpose. There are currently performance issues with this method (TBs being flushed too often), so as a faster alternative, install the flash contents as a ROM in the BMC memory space. See commit 1a15311 ("hw/arm/aspeed: add a 'execute-in-place' property to boot directly from CE0") Signed-off-by: Cédric Le Goater <clg@kaod.org> Signed-off-by: Peter Delevoryas <peter@pjd.dev> [ clg: blk_pread() fixes ] Message-Id: <20220705191400.41632-8-peter@pjd.dev> Signed-off-by: Cédric Le Goater <clg@kaod.org>
With the BIC, the easiest way to run everything is to create two pty's for each SoC and reserve stdin/stdout for the monitor: wget https://github.com/facebook/openbmc/releases/download/openbmc-e2294ff5d31d/fby35.mtd wget https://github.com/peterdelevoryas/OpenBIC/releases/download/oby35-cl-2022.13.01/Y35BCL.elf qemu-system-arm -machine fby35 \ -drive file=fby35.mtd,format=raw,if=mtd \ -device loader,file=fby35.mtd,addr=0,cpu-num=0 \ -serial pty -serial pty -serial mon:stdio -display none -S screen /dev/ttys0 screen /dev/ttys1 (qemu) c This commit only adds the the first server board's Bridge IC, but in the future we'll try to include the other three server board Bridge IC's too. Signed-off-by: Peter Delevoryas <peter@pjd.dev> Reviewed-by: Cédric Le Goater <clg@kaod.org> Message-Id: <20220705191400.41632-9-peter@pjd.dev> Signed-off-by: Cédric Le Goater <clg@kaod.org>
* Boolean statistics for KVM * Fix build on Haiku # gpg: Signature made Tue 19 Jul 2022 10:32:34 BST # 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: util: Fix broken build on Haiku kvm: add support for boolean statistics monitor: add support for boolean statistics Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
andreafioraldi
pushed a commit
that referenced
this pull request
Oct 13, 2022
As soon as virtio_scsi_data_plane_start() attaches host notifiers the IOThread may start virtqueue processing. There is a race between IOThread virtqueue processing and virtio_scsi_data_plane_start() because it only assigns s->dataplane_started after attaching host notifiers. When a virtqueue handler function in the IOThread calls virtio_scsi_defer_to_dataplane() it may see !s->dataplane_started and attempt to start dataplane even though we're already in the IOThread: #0 0x00007f67b360857c __pthread_kill_implementation (libc.so.6 + 0xa257c) #1 0x00007f67b35bbd56 raise (libc.so.6 + 0x55d56) #2 0x00007f67b358e833 abort (libc.so.6 + 0x28833) #3 0x00007f67b358e75b __assert_fail_base.cold (libc.so.6 + 0x2875b) #4 0x00007f67b35b4cd6 __assert_fail (libc.so.6 + 0x4ecd6) #5 0x000055ca87fd411b memory_region_transaction_commit (qemu-kvm + 0x67511b) #6 0x000055ca87e17811 virtio_pci_ioeventfd_assign (qemu-kvm + 0x4b8811) #7 0x000055ca87e14836 virtio_bus_set_host_notifier (qemu-kvm + 0x4b5836) #8 0x000055ca87f8e14e virtio_scsi_set_host_notifier (qemu-kvm + 0x62f14e) #9 0x000055ca87f8dd62 virtio_scsi_dataplane_start (qemu-kvm + 0x62ed62) #10 0x000055ca87e14610 virtio_bus_start_ioeventfd (qemu-kvm + 0x4b5610) #11 0x000055ca87f8c29a virtio_scsi_handle_ctrl (qemu-kvm + 0x62d29a) #12 0x000055ca87fa5902 virtio_queue_host_notifier_read (qemu-kvm + 0x646902) #13 0x000055ca882c099e aio_dispatch_handler (qemu-kvm + 0x96199e) #14 0x000055ca882c1761 aio_poll (qemu-kvm + 0x962761) #15 0x000055ca880e1052 iothread_run (qemu-kvm + 0x782052) #16 0x000055ca882c562a qemu_thread_start (qemu-kvm + 0x96662a) This patch assigns s->dataplane_started before attaching host notifiers so that virtqueue handler functions that run in the IOThread before virtio_scsi_data_plane_start() returns correctly identify that dataplane does not need to be started. This fix is taken from the virtio-blk dataplane code and it's worth adding a comment in virtio-blk as well to explain why it works. Note that s->dataplane_started does not need the AioContext lock because it is set before attaching host notifiers and cleared after detaching host notifiers. In other words, the IOThread always sees the value true and the main loop thread does not modify it while the IOThread is active. Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2099541 Reported-by: Qing Wang <qinwang@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20220808162134.240405-1-stefanha@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
andreafioraldi
pushed a commit
that referenced
this pull request
Oct 13, 2022
The DMA engine is started by I/O access and then itself accesses the I/O registers, triggering a reentrancy bug. The following log can reveal it: ==5637==ERROR: AddressSanitizer: stack-overflow #0 0x5595435f6078 in tulip_xmit_list_update qemu/hw/net/tulip.c:673 #1 0x5595435f204a in tulip_write qemu/hw/net/tulip.c:805:13 #2 0x559544637f86 in memory_region_write_accessor qemu/softmmu/memory.c:492:5 #3 0x5595446379fa in access_with_adjusted_size qemu/softmmu/memory.c:554:18 #4 0x5595446372fa in memory_region_dispatch_write qemu/softmmu/memory.c #5 0x55954468b74c in flatview_write_continue qemu/softmmu/physmem.c:2825:23 #6 0x559544683662 in flatview_write qemu/softmmu/physmem.c:2867:12 #7 0x5595446833f3 in address_space_write qemu/softmmu/physmem.c:2963:18 #8 0x5595435fb082 in dma_memory_rw_relaxed qemu/include/sysemu/dma.h:87:12 #9 0x5595435fb082 in dma_memory_rw qemu/include/sysemu/dma.h:130:12 #10 0x5595435fb082 in dma_memory_write qemu/include/sysemu/dma.h:171:12 #11 0x5595435fb082 in stl_le_dma qemu/include/sysemu/dma.h:272:1 #12 0x5595435fb082 in stl_le_pci_dma qemu/include/hw/pci/pci.h:910:1 #13 0x5595435fb082 in tulip_desc_write qemu/hw/net/tulip.c:101:9 #14 0x5595435f7e3d in tulip_xmit_list_update qemu/hw/net/tulip.c:706:9 #15 0x5595435f204a in tulip_write qemu/hw/net/tulip.c:805:13 Fix this bug by restricting the DMA engine to memories regions. Signed-off-by: Zheyu Ma <zheyuma97@gmail.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
andreafioraldi
pushed a commit
that referenced
this pull request
Oct 13, 2022
Setup an ARM virtual machine of machine virt and execute qmp "query-acpi-ospm-status" causes segmentation fault with following dumpstack: #1 0x0000aaaaab64235c in qmp_query_acpi_ospm_status (errp=errp@entry=0xfffffffff030) at ../monitor/qmp-cmds.c:312 #2 0x0000aaaaabfc4e20 in qmp_marshal_query_acpi_ospm_status (args=<optimized out>, ret=0xffffea4ffe90, errp=0xffffea4ffe88) at qapi/qapi-commands-acpi.c:63 #3 0x0000aaaaabff8ba0 in do_qmp_dispatch_bh (opaque=0xffffea4ffe98) at ../qapi/qmp-dispatch.c:128 #4 0x0000aaaaac02e594 in aio_bh_call (bh=0xffffe0004d80) at ../util/async.c:150 #5 aio_bh_poll (ctx=ctx@entry=0xaaaaad0f6040) at ../util/async.c:178 #6 0x0000aaaaac00bd40 in aio_dispatch (ctx=ctx@entry=0xaaaaad0f6040) at ../util/aio-posix.c:421 #7 0x0000aaaaac02e010 in aio_ctx_dispatch (source=0xaaaaad0f6040, callback=<optimized out>, user_data=<optimized out>) at ../util/async.c:320 #8 0x0000fffff76f6884 in g_main_context_dispatch () at /usr/lib64/libglib-2.0.so.0 #9 0x0000aaaaac0452d4 in glib_pollfds_poll () at ../util/main-loop.c:297 #10 os_host_main_loop_wait (timeout=0) at ../util/main-loop.c:320 #11 main_loop_wait (nonblocking=nonblocking@entry=0) at ../util/main-loop.c:596 #12 0x0000aaaaab5c9e50 in qemu_main_loop () at ../softmmu/runstate.c:734 #13 0x0000aaaaab185370 in qemu_main (argc=argc@entry=47, argv=argv@entry=0xfffffffff518, envp=envp@entry=0x0) at ../softmmu/main.c:38 #14 0x0000aaaaab16f99c in main (argc=47, argv=0xfffffffff518) at ../softmmu/main.c:47 Fixes: ebb6207 ("hw/acpi: Add ACPI Generic Event Device Support") Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Message-id: 20220816094957.31700-1-zhukeqian1@huawei.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
andreafioraldi
pushed a commit
that referenced
this pull request
Jan 16, 2023
This is below memleak detected when to quit the qemu-system-x86_64 (with vhost-scsi-pci). (qemu) quit ================================================================= ==15568==ERROR: LeakSanitizer: detected memory leaks Direct leak of 40 byte(s) in 1 object(s) allocated from: #0 0x7f00aec57917 in __interceptor_calloc (/lib64/libasan.so.6+0xb4917) #1 0x7f00ada0d7b5 in g_malloc0 (/lib64/libglib-2.0.so.0+0x517b5) #2 0x5648ffd38bac in vhost_scsi_start ../hw/scsi/vhost-scsi.c:92 #3 0x5648ffd38d52 in vhost_scsi_set_status ../hw/scsi/vhost-scsi.c:131 #4 0x5648ffda340e in virtio_set_status ../hw/virtio/virtio.c:2036 #5 0x5648ff8de281 in virtio_ioport_write ../hw/virtio/virtio-pci.c:431 #6 0x5648ff8deb29 in virtio_pci_config_write ../hw/virtio/virtio-pci.c:576 #7 0x5648ffe5c0c2 in memory_region_write_accessor ../softmmu/memory.c:493 #8 0x5648ffe5c424 in access_with_adjusted_size ../softmmu/memory.c:555 #9 0x5648ffe6428f in memory_region_dispatch_write ../softmmu/memory.c:1515 #10 0x5648ffe8613d in flatview_write_continue ../softmmu/physmem.c:2825 #11 0x5648ffe86490 in flatview_write ../softmmu/physmem.c:2867 #12 0x5648ffe86d9f in address_space_write ../softmmu/physmem.c:2963 #13 0x5648ffe86e57 in address_space_rw ../softmmu/physmem.c:2973 #14 0x5648fffbfb3d in kvm_handle_io ../accel/kvm/kvm-all.c:2639 #15 0x5648fffc0e0d in kvm_cpu_exec ../accel/kvm/kvm-all.c:2890 #16 0x5648fffc90a7 in kvm_vcpu_thread_fn ../accel/kvm/kvm-accel-ops.c:51 #17 0x56490042400a in qemu_thread_start ../util/qemu-thread-posix.c:505 #18 0x7f00ac3b6ea4 in start_thread (/lib64/libpthread.so.0+0x7ea4) Free the vsc->inflight at the 'stop' path. Fixes: b82526c ("vhost-scsi: support inflight io track") Cc: Joe Jin <joe.jin@oracle.com> Cc: Li Feng <fengli@smartx.com> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com> Message-Id: <20230104160433.21353-1-dongli.zhang@oracle.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
andreafioraldi
pushed a commit
that referenced
this pull request
Feb 23, 2023
Fixes the appended use-after-free. The root cause is that during tb invalidation we use CPU_FOREACH, and therefore to safely free a vCPU we must wait for an RCU grace period to elapse. $ x86_64-linux-user/qemu-x86_64 tests/tcg/x86_64-linux-user/munmap-pthread ================================================================= ==1800604==ERROR: AddressSanitizer: heap-use-after-free on address 0x62d0005f7418 at pc 0x5593da6704eb bp 0x7f4961a7ac70 sp 0x7f4961a7ac60 READ of size 8 at 0x62d0005f7418 thread T2 #0 0x5593da6704ea in tb_jmp_cache_inval_tb ../accel/tcg/tb-maint.c:244 #1 0x5593da6704ea in do_tb_phys_invalidate ../accel/tcg/tb-maint.c:290 #2 0x5593da670631 in tb_phys_invalidate__locked ../accel/tcg/tb-maint.c:306 #3 0x5593da670631 in tb_invalidate_phys_page_range__locked ../accel/tcg/tb-maint.c:542 #4 0x5593da67106d in tb_invalidate_phys_range ../accel/tcg/tb-maint.c:614 #5 0x5593da6a64d4 in target_munmap ../linux-user/mmap.c:766 #6 0x5593da6dba05 in do_syscall1 ../linux-user/syscall.c:10105 #7 0x5593da6f564c in do_syscall ../linux-user/syscall.c:13329 #8 0x5593da49e80c in cpu_loop ../linux-user/x86_64/../i386/cpu_loop.c:233 #9 0x5593da6be28c in clone_func ../linux-user/syscall.c:6633 #10 0x7f496231cb42 in start_thread nptl/pthread_create.c:442 #11 0x7f49623ae9ff (/lib/x86_64-linux-gnu/libc.so.6+0x1269ff) 0x62d0005f7418 is located 28696 bytes inside of 32768-byte region [0x62d0005f0400,0x62d0005f8400) freed by thread T148 here: #0 0x7f49627b6460 in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:52 #1 0x5593da5ac057 in cpu_exec_unrealizefn ../cpu.c:180 #2 0x5593da81f851 (/home/cota/src/qemu/build/qemu-x86_64+0x484851) Signed-off-by: Emilio Cota <cota@braap.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-Id: <20230111151628.320011-2-cota@braap.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Message-Id: <20230124180127.1881110-27-alex.bennee@linaro.org>
andreafioraldi
pushed a commit
that referenced
this pull request
Feb 23, 2023
Fixes this tsan crash, easy to reproduce with any large enough program: $ tests/unit/test-qht 1..2 ThreadSanitizer: CHECK failed: sanitizer_deadlock_detector.h:67 "((n_all_locks_)) < (((sizeof(all_locks_with_contexts_)/sizeof((all_locks_with_contexts_)[0]))))" (0x40, 0x40) (tid=1821568) #0 __tsan::CheckUnwind() ../../../../src/libsanitizer/tsan/tsan_rtl.cpp:353 (libtsan.so.2+0x90034) #1 __sanitizer::CheckFailed(char const*, int, char const*, unsigned long long, unsigned long long) ../../../../src/libsanitizer/sanitizer_common/sanitizer_termination.cpp:86 (libtsan.so.2+0xca555) #2 __sanitizer::DeadlockDetectorTLS<__sanitizer::TwoLevelBitVector<1ul, __sanitizer::BasicBitVector<unsigned long> > >::addLock(unsigned long, unsigned long, unsigned int) ../../../../src/libsanitizer/sanitizer_common/sanitizer_deadlock_detector.h:67 (libtsan.so.2+0xb3616) #3 __sanitizer::DeadlockDetectorTLS<__sanitizer::TwoLevelBitVector<1ul, __sanitizer::BasicBitVector<unsigned long> > >::addLock(unsigned long, unsigned long, unsigned int) ../../../../src/libsanitizer/sanitizer_common/sanitizer_deadlock_detector.h:59 (libtsan.so.2+0xb3616) #4 __sanitizer::DeadlockDetector<__sanitizer::TwoLevelBitVector<1ul, __sanitizer::BasicBitVector<unsigned long> > >::onLockAfter(__sanitizer::DeadlockDetectorTLS<__sanitizer::TwoLevelBitVector<1ul, __sanitizer::BasicBitVector<unsigned long> > >*, unsigned long, unsigned int) ../../../../src/libsanitizer/sanitizer_common/sanitizer_deadlock_detector.h:216 (libtsan.so.2+0xb3616) #5 __sanitizer::DD::MutexAfterLock(__sanitizer::DDCallback*, __sanitizer::DDMutex*, bool, bool) ../../../../src/libsanitizer/sanitizer_common/sanitizer_deadlock_detector1.cpp:169 (libtsan.so.2+0xb3616) #6 __tsan::MutexPostLock(__tsan::ThreadState*, unsigned long, unsigned long, unsigned int, int) ../../../../src/libsanitizer/tsan/tsan_rtl_mutex.cpp:200 (libtsan.so.2+0xa3382) #7 __tsan_mutex_post_lock ../../../../src/libsanitizer/tsan/tsan_interface_ann.cpp:384 (libtsan.so.2+0x76bc3) #8 qemu_spin_lock /home/cota/src/qemu/include/qemu/thread.h:259 (test-qht+0x44a97) #9 qht_map_lock_buckets ../util/qht.c:253 (test-qht+0x44a97) #10 do_qht_iter ../util/qht.c:809 (test-qht+0x45f33) #11 qht_iter ../util/qht.c:821 (test-qht+0x45f33) #12 iter_check ../tests/unit/test-qht.c:121 (test-qht+0xe473) #13 qht_do_test ../tests/unit/test-qht.c:202 (test-qht+0xe473) #14 qht_test ../tests/unit/test-qht.c:240 (test-qht+0xe7c1) #15 test_default ../tests/unit/test-qht.c:246 (test-qht+0xe828) #16 <null> <null> (libglib-2.0.so.0+0x7daed) #17 <null> <null> (libglib-2.0.so.0+0x7d80a) #18 <null> <null> (libglib-2.0.so.0+0x7d80a) #19 g_test_run_suite <null> (libglib-2.0.so.0+0x7dfe9) #20 g_test_run <null> (libglib-2.0.so.0+0x7e055) #21 main ../tests/unit/test-qht.c:259 (test-qht+0xd2c6) #22 __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 (libc.so.6+0x29d8f) #23 __libc_start_main_impl ../csu/libc-start.c:392 (libc.so.6+0x29e3f) #24 _start <null> (test-qht+0xdb44) Signed-off-by: Emilio Cota <cota@braap.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-Id: <20230111151628.320011-5-cota@braap.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Message-Id: <20230124180127.1881110-30-alex.bennee@linaro.org>
andreafioraldi
pushed a commit
that referenced
this pull request
Sep 5, 2023
in order to avoid requests being stuck in a BlockBackend's request queue during cleanup. Having such requests can lead to a deadlock [0] with a virtio-scsi-pci device using iothread that's busy with IO when initiating a shutdown with QMP 'quit'. There is a race where such a queued request can continue sometime (maybe after bdrv_child_free()?) during bdrv_root_unref_child() [1]. The completion will hold the AioContext lock and wait for the BQL during SCSI completion, but the main thread will hold the BQL and wait for the AioContext as part of bdrv_root_unref_child(), leading to the deadlock [0]. [0]: > Thread 3 (Thread 0x7f3bbd87b700 (LWP 135952) "qemu-system-x86"): > #0 __lll_lock_wait (futex=futex@entry=0x564183365f00 <qemu_global_mutex>, private=0) at lowlevellock.c:52 > #1 0x00007f3bc1c0d843 in __GI___pthread_mutex_lock (mutex=0x564183365f00 <qemu_global_mutex>) at ../nptl/pthread_mutex_lock.c:80 > #2 0x0000564182939f2e in qemu_mutex_lock_impl (mutex=0x564183365f00 <qemu_global_mutex>, file=0x564182b7f774 "../softmmu/physmem.c", line=2593) at ../util/qemu-thread-posix.c:94 > #3 0x000056418247cc2a in qemu_mutex_lock_iothread_impl (file=0x564182b7f774 "../softmmu/physmem.c", line=2593) at ../softmmu/cpus.c:504 > #4 0x00005641826d5325 in prepare_mmio_access (mr=0x5641856148a0) at ../softmmu/physmem.c:2593 > #5 0x00005641826d6fe7 in address_space_stl_internal (as=0x56418679b310, addr=4276113408, val=16418, attrs=..., result=0x0, endian=DEVICE_LITTLE_ENDIAN) at /home/febner/repos/qemu/memory_ldst.c.inc:318 > #6 0x00005641826d7154 in address_space_stl_le (as=0x56418679b310, addr=4276113408, val=16418, attrs=..., result=0x0) at /home/febner/repos/qemu/memory_ldst.c.inc:357 > #7 0x0000564182374b07 in pci_msi_trigger (dev=0x56418679b0d0, msg=...) at ../hw/pci/pci.c:359 > #8 0x000056418237118b in msi_send_message (dev=0x56418679b0d0, msg=...) at ../hw/pci/msi.c:379 > #9 0x0000564182372c10 in msix_notify (dev=0x56418679b0d0, vector=8) at ../hw/pci/msix.c:542 > #10 0x000056418243719c in virtio_pci_notify (d=0x56418679b0d0, vector=8) at ../hw/virtio/virtio-pci.c:77 > #11 0x00005641826933b0 in virtio_notify_vector (vdev=0x5641867a34a0, vector=8) at ../hw/virtio/virtio.c:1985 > #12 0x00005641826948d6 in virtio_irq (vq=0x5641867ac078) at ../hw/virtio/virtio.c:2461 > #13 0x0000564182694978 in virtio_notify (vdev=0x5641867a34a0, vq=0x5641867ac078) at ../hw/virtio/virtio.c:2473 > #14 0x0000564182665b83 in virtio_scsi_complete_req (req=0x7f3bb000e5d0) at ../hw/scsi/virtio-scsi.c:115 > #15 0x00005641826670ce in virtio_scsi_complete_cmd_req (req=0x7f3bb000e5d0) at ../hw/scsi/virtio-scsi.c:641 > #16 0x000056418266736b in virtio_scsi_command_complete (r=0x7f3bb0010560, resid=0) at ../hw/scsi/virtio-scsi.c:712 > #17 0x000056418239aac6 in scsi_req_complete (req=0x7f3bb0010560, status=2) at ../hw/scsi/scsi-bus.c:1526 > #18 0x000056418239e090 in scsi_handle_rw_error (r=0x7f3bb0010560, ret=-123, acct_failed=false) at ../hw/scsi/scsi-disk.c:242 > #19 0x000056418239e13f in scsi_disk_req_check_error (r=0x7f3bb0010560, ret=-123, acct_failed=false) at ../hw/scsi/scsi-disk.c:265 > #20 0x000056418239e482 in scsi_dma_complete_noio (r=0x7f3bb0010560, ret=-123) at ../hw/scsi/scsi-disk.c:340 > #21 0x000056418239e5d9 in scsi_dma_complete (opaque=0x7f3bb0010560, ret=-123) at ../hw/scsi/scsi-disk.c:371 > #22 0x00005641824809ad in dma_complete (dbs=0x7f3bb000d9d0, ret=-123) at ../softmmu/dma-helpers.c:107 > #23 0x0000564182480a72 in dma_blk_cb (opaque=0x7f3bb000d9d0, ret=-123) at ../softmmu/dma-helpers.c:127 > #24 0x00005641827bf78a in blk_aio_complete (acb=0x7f3bb00021a0) at ../block/block-backend.c:1563 > #25 0x00005641827bfa5e in blk_aio_write_entry (opaque=0x7f3bb00021a0) at ../block/block-backend.c:1630 > #26 0x000056418295638a in coroutine_trampoline (i0=-1342102448, i1=32571) at ../util/coroutine-ucontext.c:177 > #27 0x00007f3bc0caed40 in ?? () from /lib/x86_64-linux-gnu/libc.so.6 > #28 0x00007f3bbd8757f0 in ?? () > #29 0x0000000000000000 in ?? () > > Thread 1 (Thread 0x7f3bbe3e9280 (LWP 135944) "qemu-system-x86"): > #0 __lll_lock_wait (futex=futex@entry=0x5641856f2a00, private=0) at lowlevellock.c:52 > #1 0x00007f3bc1c0d8d1 in __GI___pthread_mutex_lock (mutex=0x5641856f2a00) at ../nptl/pthread_mutex_lock.c:115 > #2 0x0000564182939f2e in qemu_mutex_lock_impl (mutex=0x5641856f2a00, file=0x564182c0e319 "../util/async.c", line=728) at ../util/qemu-thread-posix.c:94 > #3 0x000056418293a140 in qemu_rec_mutex_lock_impl (mutex=0x5641856f2a00, file=0x564182c0e319 "../util/async.c", line=728) at ../util/qemu-thread-posix.c:149 > #4 0x00005641829532d5 in aio_context_acquire (ctx=0x5641856f29a0) at ../util/async.c:728 > #5 0x000056418279d5df in bdrv_set_aio_context_commit (opaque=0x5641856e6e50) at ../block.c:7493 > #6 0x000056418294e288 in tran_commit (tran=0x56418630bfe0) at ../util/transactions.c:87 > #7 0x000056418279d880 in bdrv_try_change_aio_context (bs=0x5641856f7130, ctx=0x56418548f810, ignore_child=0x0, errp=0x0) at ../block.c:7626 > #8 0x0000564182793f39 in bdrv_root_unref_child (child=0x5641856f47d0) at ../block.c:3242 > #9 0x00005641827be137 in blk_remove_bs (blk=0x564185709880) at ../block/block-backend.c:914 > #10 0x00005641827bd689 in blk_remove_all_bs () at ../block/block-backend.c:583 > #11 0x0000564182798699 in bdrv_close_all () at ../block.c:5117 > #12 0x000056418248a5b2 in qemu_cleanup () at ../softmmu/runstate.c:821 > #13 0x0000564182738603 in qemu_default_main () at ../softmmu/main.c:38 > #14 0x0000564182738631 in main (argc=30, argv=0x7ffd675a8a48) at ../softmmu/main.c:48 > > (gdb) p *((QemuMutex*)0x5641856f2a00) > $1 = {lock = {__data = {__lock = 2, __count = 2, __owner = 135952, ... > (gdb) p *((QemuMutex*)0x564183365f00) > $2 = {lock = {__data = {__lock = 2, __count = 0, __owner = 135944, ... [1]: > Thread 1 "qemu-system-x86" hit Breakpoint 5, bdrv_drain_all_end () at ../block/io.c:551 > #0 bdrv_drain_all_end () at ../block/io.c:551 > #1 0x00005569810f0376 in bdrv_graph_wrlock (bs=0x0) at ../block/graph-lock.c:156 > #2 0x00005569810bd3e0 in bdrv_replace_child_noperm (child=0x556982e2d7d0, new_bs=0x0) at ../block.c:2897 > #3 0x00005569810bdef2 in bdrv_root_unref_child (child=0x556982e2d7d0) at ../block.c:3227 > #4 0x00005569810e8137 in blk_remove_bs (blk=0x556982e42880) at ../block/block-backend.c:914 > #5 0x00005569810e7689 in blk_remove_all_bs () at ../block/block-backend.c:583 > #6 0x00005569810c2699 in bdrv_close_all () at ../block.c:5117 > #7 0x0000556980db45b2 in qemu_cleanup () at ../softmmu/runstate.c:821 > #8 0x0000556981062603 in qemu_default_main () at ../softmmu/main.c:38 > #9 0x0000556981062631 in main (argc=30, argv=0x7ffd7a82a418) at ../softmmu/main.c:48 > [Switching to Thread 0x7fe76dab2700 (LWP 103649)] > > Thread 3 "qemu-system-x86" hit Breakpoint 4, blk_inc_in_flight (blk=0x556982e42880) at ../block/block-backend.c:1505 > #0 blk_inc_in_flight (blk=0x556982e42880) at ../block/block-backend.c:1505 > #1 0x00005569810e8f36 in blk_wait_while_drained (blk=0x556982e42880) at ../block/block-backend.c:1312 > #2 0x00005569810e9231 in blk_co_do_pwritev_part (blk=0x556982e42880, offset=3422961664, bytes=4096, qiov=0x556983028060, qiov_offset=0, flags=0) at ../block/block-backend.c:1402 > #3 0x00005569810e9a4b in blk_aio_write_entry (opaque=0x556982e2cfa0) at ../block/block-backend.c:1628 > #4 0x000055698128038a in coroutine_trampoline (i0=-2090057872, i1=21865) at ../util/coroutine-ucontext.c:177 > #5 0x00007fe770f50d40 in ?? () from /lib/x86_64-linux-gnu/libc.so.6 > #6 0x00007ffd7a829570 in ?? () > #7 0x0000000000000000 in ?? () Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Message-ID: <20230706131418.423713-1-f.ebner@proxmox.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
andreafioraldi
pushed a commit
that referenced
this pull request
Sep 25, 2023
Otherwise tcg_handle_interrupt() triggers an assertion failure: #5 0x0000555555c97369 in tcg_handle_interrupt (cpu=0x555557434cb0, mask=2) at ../accel/tcg/tcg-accel-ops.c:83 #6 tcg_handle_interrupt (cpu=0x555557434cb0, mask=2) at ../accel/tcg/tcg-accel-ops.c:81 #7 0x0000555555b4d58b in pic_irq_request (opaque=<optimized out>, irq=<optimized out>, level=1) at ../hw/i386/x86.c:555 #8 0x0000555555b4f218 in gsi_handler (opaque=0x5555579423d0, n=13, level=1) at ../hw/i386/x86.c:611 #9 0x00007fffa42bde14 in code_gen_buffer () #10 0x0000555555c724bb in cpu_tb_exec (cpu=cpu@entry=0x555557434cb0, itb=<optimized out>, tb_exit=tb_exit@entry=0x7fffe9bfd658) at ../accel/tcg/cpu-exec.c:457 Cc: qemu-stable@nongnu.org Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1808 Reported-by: NyanCatTW1 <https://gitlab.com/a0939712328> Co-developed-by: Richard Henderson <richard.henderson@linaro.org>' Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
andreafioraldi
pushed a commit
that referenced
this pull request
Sep 25, 2023
virtio_load() as a whole should run in coroutine context because it reads from the migration stream and we don't want this to block. However, it calls virtio_set_features_nocheck() and devices don't expect their .set_features callback to run in a coroutine and therefore call functions that may not be called in coroutine context. To fix this, drop out of coroutine context for calling virtio_set_features_nocheck(). Without this fix, the following crash was reported: #0 __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44 #1 0x00007efc738c05d3 in __pthread_kill_internal (signo=6, threadid=<optimized out>) at pthread_kill.c:78 #2 0x00007efc73873d26 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26 #3 0x00007efc738477f3 in __GI_abort () at abort.c:79 #4 0x00007efc7384771b in __assert_fail_base (fmt=0x7efc739dbcb8 "", assertion=assertion@entry=0x560aebfbf5cf "!qemu_in_coroutine()", file=file@entry=0x560aebfcd2d4 "../block/graph-lock.c", line=line@entry=275, function=function@entry=0x560aebfcd34d "void bdrv_graph_rdlock_main_loop(void)") at assert.c:92 #5 0x00007efc7386ccc6 in __assert_fail (assertion=0x560aebfbf5cf "!qemu_in_coroutine()", file=0x560aebfcd2d4 "../block/graph-lock.c", line=275, function=0x560aebfcd34d "void bdrv_graph_rdlock_main_loop(void)") at assert.c:101 #6 0x0000560aebcd8dd6 in bdrv_register_buf () #7 0x0000560aeb97ed97 in ram_block_added.llvm () #8 0x0000560aebb8303f in ram_block_add.llvm () #9 0x0000560aebb834fa in qemu_ram_alloc_internal.llvm () #10 0x0000560aebb2ac98 in vfio_region_mmap () #11 0x0000560aebb3ea0f in vfio_bars_register () #12 0x0000560aebb3c628 in vfio_realize () #13 0x0000560aeb90f0c2 in pci_qdev_realize () #14 0x0000560aebc40305 in device_set_realized () #15 0x0000560aebc48e07 in property_set_bool.llvm () #16 0x0000560aebc46582 in object_property_set () #17 0x0000560aebc4cd58 in object_property_set_qobject () #18 0x0000560aebc46ba7 in object_property_set_bool () #19 0x0000560aeb98b3ca in qdev_device_add_from_qdict () #20 0x0000560aebb1fbaf in virtio_net_set_features () #21 0x0000560aebb46b51 in virtio_set_features_nocheck () #22 0x0000560aebb47107 in virtio_load () #23 0x0000560aeb9ae7ce in vmstate_load_state () #24 0x0000560aeb9d2ee9 in qemu_loadvm_state_main () #25 0x0000560aeb9d45e1 in qemu_loadvm_state () #26 0x0000560aeb9bc32c in process_incoming_migration_co.llvm () #27 0x0000560aebeace56 in coroutine_trampoline.llvm () Cc: qemu-stable@nongnu.org Buglink: https://issues.redhat.com/browse/RHEL-832 Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230905145002.46391-3-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
andreafioraldi
pushed a commit
that referenced
this pull request
Nov 17, 2023
If there is a pending DMA operation during ide_bus_reset(), the fact that the IDEState is already reset before the operation is canceled can be problematic. In particular, ide_dma_cb() might be called and then use the reset IDEState which contains the signature after the reset. When used to construct the IO operation this leads to ide_get_sector() returning 0 and nsector being 1. This is particularly bad, because a write command will thus destroy the first sector which often contains a partition table or similar. Traces showing the unsolicited write happening with IDEState 0x5595af6949d0 being used after reset: > ahci_port_write ahci(0x5595af6923f0)[0]: port write [reg:PxSCTL] @ 0x2c: 0x00000300 > ahci_reset_port ahci(0x5595af6923f0)[0]: reset port > ide_reset IDEstate 0x5595af6949d0 > ide_reset IDEstate 0x5595af694da8 > ide_bus_reset_aio aio_cancel > dma_aio_cancel dbs=0x7f64600089a0 > dma_blk_cb dbs=0x7f64600089a0 ret=0 > dma_complete dbs=0x7f64600089a0 ret=0 cb=0x5595acd40b30 > ahci_populate_sglist ahci(0x5595af6923f0)[0] > ahci_dma_prepare_buf ahci(0x5595af6923f0)[0]: prepare buf limit=512 prepared=512 > ide_dma_cb IDEState 0x5595af6949d0; sector_num=0 n=1 cmd=DMA WRITE > dma_blk_io dbs=0x7f6420802010 bs=0x5595ae2c6c30 offset=0 to_dev=1 > dma_blk_cb dbs=0x7f6420802010 ret=0 > (gdb) p *qiov > $11 = {iov = 0x7f647c76d840, niov = 1, {{nalloc = 1, local_iov = {iov_base = 0x0, > iov_len = 512}}, {__pad = "\001\000\000\000\000\000\000\000\000\000\000", > size = 512}}} > (gdb) bt > #0 blk_aio_pwritev (blk=0x5595ae2c6c30, offset=0, qiov=0x7f6420802070, flags=0, > cb=0x5595ace6f0b0 <dma_blk_cb>, opaque=0x7f6420802010) > at ../block/block-backend.c:1682 > #1 0x00005595ace6f185 in dma_blk_cb (opaque=0x7f6420802010, ret=<optimized out>) > at ../softmmu/dma-helpers.c:179 > #2 0x00005595ace6f778 in dma_blk_io (ctx=0x5595ae0609f0, > sg=sg@entry=0x5595af694d00, offset=offset@entry=0, align=align@entry=512, > io_func=io_func@entry=0x5595ace6ee30 <dma_blk_write_io_func>, > io_func_opaque=io_func_opaque@entry=0x5595ae2c6c30, > cb=0x5595acd40b30 <ide_dma_cb>, opaque=0x5595af6949d0, > dir=DMA_DIRECTION_TO_DEVICE) at ../softmmu/dma-helpers.c:244 > #3 0x00005595ace6f90a in dma_blk_write (blk=0x5595ae2c6c30, > sg=sg@entry=0x5595af694d00, offset=offset@entry=0, align=align@entry=512, > cb=cb@entry=0x5595acd40b30 <ide_dma_cb>, opaque=opaque@entry=0x5595af6949d0) > at ../softmmu/dma-helpers.c:280 > #4 0x00005595acd40e18 in ide_dma_cb (opaque=0x5595af6949d0, ret=<optimized out>) > at ../hw/ide/core.c:953 > #5 0x00005595ace6f319 in dma_complete (ret=0, dbs=0x7f64600089a0) > at ../softmmu/dma-helpers.c:107 > #6 dma_blk_cb (opaque=0x7f64600089a0, ret=0) at ../softmmu/dma-helpers.c:127 > #7 0x00005595ad12227d in blk_aio_complete (acb=0x7f6460005b10) > at ../block/block-backend.c:1527 > #8 blk_aio_complete (acb=0x7f6460005b10) at ../block/block-backend.c:1524 > #9 blk_aio_write_entry (opaque=0x7f6460005b10) at ../block/block-backend.c:1594 > #10 0x00005595ad258cfb in coroutine_trampoline (i0=<optimized out>, > i1=<optimized out>) at ../util/coroutine-ucontext.c:177 Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Tested-by: simon.rowe@nutanix.com Message-ID: <20230906130922.142845-1-f.ebner@proxmox.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
rmalmain
pushed a commit
that referenced
this pull request
Feb 5, 2024
There is no architectural requirement that SME implies SVE, but our implementation currently assumes it. (FEAT_SME_FA64 does imply SVE.) So if you try to run a CPU with eg "-cpu max,sve=off" you quickly run into an assert when the guest tries to write to SMCR_EL1: #6 0x00007ffff4b38e96 in __GI___assert_fail (assertion=0x5555566e69cb "sm", file=0x5555566e5b24 "../../target/arm/helper.c", line=6865, function=0x5555566e82f0 <__PRETTY_FUNCTION__.31> "sve_vqm1_for_el_sm") at ./assert/assert.c:101 #7 0x0000555555ee33aa in sve_vqm1_for_el_sm (env=0x555557d291f0, el=2, sm=false) at ../../target/arm/helper.c:6865 #8 0x0000555555ee3407 in sve_vqm1_for_el (env=0x555557d291f0, el=2) at ../../target/arm/helper.c:6871 #9 0x0000555555ee3724 in smcr_write (env=0x555557d291f0, ri=0x555557da23b0, value=2147483663) at ../../target/arm/helper.c:6995 #10 0x0000555555fd1dba in helper_set_cp_reg64 (env=0x555557d291f0, rip=0x555557da23b0, value=2147483663) at ../../target/arm/tcg/op_helper.c:839 #11 0x00007fff60056781 in code_gen_buffer () Avoid this unsupported and slightly odd combination by disabling SME when SVE is not present. Cc: qemu-stable@nongnu.org Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2005 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20231127173318.674758-1-peter.maydell@linaro.org
rmalmain
pushed a commit
that referenced
this pull request
Feb 5, 2024
…ock_status Using fleecing backup like in [0] on a qcow2 image (with metadata preallocation) can lead to the following assertion failure: > bdrv_co_do_block_status: Assertion `!(ret & BDRV_BLOCK_ZERO)' failed. In the reproducer [0], it happens because the BDRV_BLOCK_RECURSE flag will be set by the qcow2 driver, so the caller will recursively check the file child. Then the BDRV_BLOCK_ZERO set too. Later up the call chain, in bdrv_co_do_block_status() for the snapshot-access driver, the assertion failure will happen, because both flags are set. To fix it, clear the recurse flag after the recursive check was done. In detail: > #0 qcow2_co_block_status Returns 0x45 = BDRV_BLOCK_RECURSE | BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID. > #1 bdrv_co_do_block_status Because of the data flag, bdrv_co_do_block_status() will now also set BDRV_BLOCK_ALLOCATED. Because of the recurse flag, bdrv_co_do_block_status() for the bdrv_file child will be called, which returns 0x16 = BDRV_BLOCK_ALLOCATED | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_ZERO. Now the return value inherits the zero flag. Returns 0x57 = BDRV_BLOCK_RECURSE | BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_ALLOCATED | BDRV_BLOCK_ZERO. > #2 bdrv_co_common_block_status_above > #3 bdrv_co_block_status_above > #4 bdrv_co_block_status > #5 cbw_co_snapshot_block_status > #6 bdrv_co_snapshot_block_status > #7 snapshot_access_co_block_status > #8 bdrv_co_do_block_status Return value is propagated all the way up to here, where the assertion failure happens, because BDRV_BLOCK_RECURSE and BDRV_BLOCK_ZERO are both set. > #9 bdrv_co_common_block_status_above > #10 bdrv_co_block_status_above > #11 block_copy_block_status > #12 block_copy_dirty_clusters > #13 block_copy_common > #14 block_copy_async_co_entry > #15 coroutine_trampoline [0]: > #!/bin/bash > rm /tmp/disk.qcow2 > ./qemu-img create /tmp/disk.qcow2 -o preallocation=metadata -f qcow2 1G > ./qemu-img create /tmp/fleecing.qcow2 -f qcow2 1G > ./qemu-img create /tmp/backup.qcow2 -f qcow2 1G > ./qemu-system-x86_64 --qmp stdio \ > --blockdev qcow2,node-name=node0,file.driver=file,file.filename=/tmp/disk.qcow2 \ > --blockdev qcow2,node-name=node1,file.driver=file,file.filename=/tmp/fleecing.qcow2 \ > --blockdev qcow2,node-name=node2,file.driver=file,file.filename=/tmp/backup.qcow2 \ > <<EOF > {"execute": "qmp_capabilities"} > {"execute": "blockdev-add", "arguments": { "driver": "copy-before-write", "file": "node0", "target": "node1", "node-name": "node3" } } > {"execute": "blockdev-add", "arguments": { "driver": "snapshot-access", "file": "node3", "node-name": "snap0" } } > {"execute": "blockdev-backup", "arguments": { "device": "snap0", "target": "node1", "sync": "full", "job-id": "backup0" } } > EOF Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Message-id: 20240116154839.401030-1-f.ebner@proxmox.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> (cherry picked from commit 8a9be79) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
rmalmain
pushed a commit
to rmalmain/qemu-libafl-bridge
that referenced
this pull request
Apr 11, 2024
rmalmain
pushed a commit
to rmalmain/qemu-libafl-bridge
that referenced
this pull request
Apr 11, 2024
Otherwise tcg_handle_interrupt() triggers an assertion failure: AFLplusplus#5 0x0000555555c97369 in tcg_handle_interrupt (cpu=0x555557434cb0, mask=2) at ../accel/tcg/tcg-accel-ops.c:83 AFLplusplus#6 tcg_handle_interrupt (cpu=0x555557434cb0, mask=2) at ../accel/tcg/tcg-accel-ops.c:81 AFLplusplus#7 0x0000555555b4d58b in pic_irq_request (opaque=<optimized out>, irq=<optimized out>, level=1) at ../hw/i386/x86.c:555 AFLplusplus#8 0x0000555555b4f218 in gsi_handler (opaque=0x5555579423d0, n=13, level=1) at ../hw/i386/x86.c:611 AFLplusplus#9 0x00007fffa42bde14 in code_gen_buffer () AFLplusplus#10 0x0000555555c724bb in cpu_tb_exec (cpu=cpu@entry=0x555557434cb0, itb=<optimized out>, tb_exit=tb_exit@entry=0x7fffe9bfd658) at ../accel/tcg/cpu-exec.c:457 Cc: qemu-stable@nongnu.org Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1808 Reported-by: NyanCatTW1 <https://gitlab.com/a0939712328> Co-developed-by: Richard Henderson <richard.henderson@linaro.org>' Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> (cherry picked from commit c1f27a0) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
rmalmain
pushed a commit
to rmalmain/qemu-libafl-bridge
that referenced
this pull request
Apr 11, 2024
virtio_load() as a whole should run in coroutine context because it reads from the migration stream and we don't want this to block. However, it calls virtio_set_features_nocheck() and devices don't expect their .set_features callback to run in a coroutine and therefore call functions that may not be called in coroutine context. To fix this, drop out of coroutine context for calling virtio_set_features_nocheck(). Without this fix, the following crash was reported: #0 __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44 #1 0x00007efc738c05d3 in __pthread_kill_internal (signo=6, threadid=<optimized out>) at pthread_kill.c:78 AFLplusplus#2 0x00007efc73873d26 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26 AFLplusplus#3 0x00007efc738477f3 in __GI_abort () at abort.c:79 AFLplusplus#4 0x00007efc7384771b in __assert_fail_base (fmt=0x7efc739dbcb8 "", assertion=assertion@entry=0x560aebfbf5cf "!qemu_in_coroutine()", file=file@entry=0x560aebfcd2d4 "../block/graph-lock.c", line=line@entry=275, function=function@entry=0x560aebfcd34d "void bdrv_graph_rdlock_main_loop(void)") at assert.c:92 AFLplusplus#5 0x00007efc7386ccc6 in __assert_fail (assertion=0x560aebfbf5cf "!qemu_in_coroutine()", file=0x560aebfcd2d4 "../block/graph-lock.c", line=275, function=0x560aebfcd34d "void bdrv_graph_rdlock_main_loop(void)") at assert.c:101 AFLplusplus#6 0x0000560aebcd8dd6 in bdrv_register_buf () AFLplusplus#7 0x0000560aeb97ed97 in ram_block_added.llvm () AFLplusplus#8 0x0000560aebb8303f in ram_block_add.llvm () AFLplusplus#9 0x0000560aebb834fa in qemu_ram_alloc_internal.llvm () AFLplusplus#10 0x0000560aebb2ac98 in vfio_region_mmap () AFLplusplus#11 0x0000560aebb3ea0f in vfio_bars_register () AFLplusplus#12 0x0000560aebb3c628 in vfio_realize () AFLplusplus#13 0x0000560aeb90f0c2 in pci_qdev_realize () AFLplusplus#14 0x0000560aebc40305 in device_set_realized () AFLplusplus#15 0x0000560aebc48e07 in property_set_bool.llvm () AFLplusplus#16 0x0000560aebc46582 in object_property_set () AFLplusplus#17 0x0000560aebc4cd58 in object_property_set_qobject () AFLplusplus#18 0x0000560aebc46ba7 in object_property_set_bool () AFLplusplus#19 0x0000560aeb98b3ca in qdev_device_add_from_qdict () AFLplusplus#20 0x0000560aebb1fbaf in virtio_net_set_features () AFLplusplus#21 0x0000560aebb46b51 in virtio_set_features_nocheck () AFLplusplus#22 0x0000560aebb47107 in virtio_load () AFLplusplus#23 0x0000560aeb9ae7ce in vmstate_load_state () AFLplusplus#24 0x0000560aeb9d2ee9 in qemu_loadvm_state_main () AFLplusplus#25 0x0000560aeb9d45e1 in qemu_loadvm_state () AFLplusplus#26 0x0000560aeb9bc32c in process_incoming_migration_co.llvm () AFLplusplus#27 0x0000560aebeace56 in coroutine_trampoline.llvm () Cc: qemu-stable@nongnu.org Buglink: https://issues.redhat.com/browse/RHEL-832 Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230905145002.46391-3-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> (cherry picked from commit 92e2e6a) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
rmalmain
pushed a commit
that referenced
this pull request
Apr 18, 2024
…ng ARM_FEATURE_PMU It doesn't make sense to read the value of MDCR_EL2 on a non-A-profile CPU, and in fact if you try to do it we will assert: #6 0x00007ffff4b95e96 in __GI___assert_fail (assertion=0x5555565a8c70 "!arm_feature(env, ARM_FEATURE_M)", file=0x5555565a6e5c "../../target/arm/helper.c", line=12600, function=0x5555565a9560 <__PRETTY_FUNCTION__.0> "arm_security_space_below_el3") at ./assert/assert.c:101 #7 0x0000555555ebf412 in arm_security_space_below_el3 (env=0x555557bc8190) at ../../target/arm/helper.c:12600 #8 0x0000555555ea6f89 in arm_is_el2_enabled (env=0x555557bc8190) at ../../target/arm/cpu.h:2595 #9 0x0000555555ea942f in arm_mdcr_el2_eff (env=0x555557bc8190) at ../../target/arm/internals.h:1512 We might call pmu_counter_enabled() on an M-profile CPU (for example from the migration pre/post hooks in machine.c); this should always return false because these CPUs don't set ARM_FEATURE_PMU. Avoid the assertion by not calling arm_mdcr_el2_eff() before we have done the early return for "PMU not present". This fixes an assertion failure if you try to do a loadvm or savevm for an M-profile board. Cc: qemu-stable@nongnu.org Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2155 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20240208153346.970021-1-peter.maydell@linaro.org (cherry picked from commit ac1d88e) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
rmalmain
added a commit
that referenced
this pull request
Apr 24, 2024
* migration: Revert mapped-ram multifd support to fd: URI This reverts commit decdc76772c453ff1444612e910caa0d45cd8eac in full and also the relevant migration-tests from 7a09f092834641b7a793d50a3a261073bbb404a6. After the addition of the new QAPI-based migration address API in 8.2 we've been converting an "fd:" URI into a SocketAddress, missing the fact that the "fd:" syntax could also be used for a plain file instead of a socket. This is a problem because the SocketAddress is part of the API, so we're effectively asking users to create a "socket" channel to pass in a plain file. The easiest way to fix this situation is to deprecate the usage of both SocketAddress and "fd:" when used with a plain file for migration. Since this has been possible since 8.2, we can wait until 9.1 to deprecate it. For 9.0, however, we should avoid adding further support to migration to a plain file using the old "fd:" syntax or the new SocketAddress API, and instead require the usage of either the old-style "file:" URI or the FileMigrationArgs::filename field of the new API with the "/dev/fdset/NN" syntax, both of which are already supported. Signed-off-by: Fabiano Rosas <farosas@suse.de> Link: https://lore.kernel.org/r/20240319210941.1907-1-farosas@suse.de Signed-off-by: Peter Xu <peterx@redhat.com> * migration/postcopy: Fix high frequency sync With current code base I can observe extremely high sync count during precopy, as long as one enables postcopy-ram=on before switchover to postcopy. To provide some context of when QEMU decides to do a full sync: it checks must_precopy (which implies "data must be sent during precopy phase"), and as long as it is lower than the threshold size we calculated (out of bandwidth and expected downtime) QEMU will kick off the slow/exact sync. However, when postcopy is enabled (even if still during precopy phase), RAM only reports all pages as can_postcopy, and report must_precopy==0. Then "must_precopy <= threshold_size" mostly always triggers and enforces a slow sync for every call to migration_iteration_run() when postcopy is enabled even if not used. That is insane. It turns out it was a regress bug introduced in the previous refactoring in 8.0 as reported by Nina [1]: (a) c8df4a7aef ("migration: Split save_live_pending() into state_pending_*") Then a workaround patch is applied at the end of release (8.0-rc4) to fix it: (b) 28ef5339c3 ("migration: fix ram_state_pending_exact()") However that "workaround" was overlooked when during the cleanup in this 9.0 release in this commit.. (c) b0504edd40 ("migration: Drop unnecessary check in ram's pending_exact()") Then the issue was re-exposed as reported by Nina [1]. The problem with (b) is that it only fixed the case for RAM, rather than all the rest of iterators. Here a slow sync should only be required if all dirty data (precopy+postcopy) is less than the threshold_size that QEMU calculated. It is even debatable whether a sync is needed when switched to postcopy. Currently ram_state_pending_exact() will be mostly noop if switched to postcopy, and that logic seems to apply too for all the rest of iterators, as sync dirty bitmap during a postcopy doesn't make much sense. However let's leave such change for later, as we're in rc phase. So rather than reusing commit (b), this patch provides the complete fix for all iterators. When at it, cleanup a little bit on the lines around. [1] https://gitlab.com/qemu-project/qemu/-/issues/1565 Reported-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com> Fixes: b0504edd40 ("migration: Drop unnecessary check in ram's pending_exact()") Reviewed-by: Fabiano Rosas <farosas@suse.de> Link: https://lore.kernel.org/r/20240320214453.584374-1-peterx@redhat.com Signed-off-by: Peter Xu <peterx@redhat.com> * migration/multifd: Fix clearing of mapped-ram zero pages When the zero page detection is done in the multifd threads, we need to iterate the second part of the pages->offset array and clear the file bitmap for each zero page. The piece of code we merged to do that is wrong. The reason this has passed all the tests is because the bitmap is initialized with zeroes already, so clearing the bits only really has an effect during live migration and when a data page goes from having data to no data. Fixes: 303e6f54f9 ("migration/multifd: Implement zero page transmission on the multifd thread.") Signed-off-by: Fabiano Rosas <farosas@suse.de> Link: https://lore.kernel.org/r/20240321201242.6009-1-farosas@suse.de Signed-off-by: Peter Xu <peterx@redhat.com> * .travis.yml: Shorten the runtime of the problematic jobs The "[s390x] GCC (other-system)" and the "[s390x] GCC check-tcg" jobs are hitting the 50 minutes timeout in Travis quite frequently since a while. To fix it, we've got to drop a lot of the targets from the target list in the jobs to make them work again. With regards to the "check-tcg" test, we can move the check with "s390x-linux-user" to the "user" job instead which also builds the s390x-linux-user target. And while we're at it, remove the "--enable-fdt=system" configure switch (since this is not required nowadays anymore). Message-ID: <20240320104144.823425-2-thuth@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com> * .travis.yml: Remove the unused xfslib-dev package Drop the "xfslibs-dev" package which should not be necessary anymore since commit a5730b8bd3 ("block/file-posix: Simplify the XFS_IOC_DIOINFO handling"). Message-ID: <20240320104144.823425-3-thuth@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Thomas Huth <thuth@redhat.com> * hw/microblaze: Do not allow xlnx-zynqmp-pmu-soc to be created by the user Using xlnx-zynqmp-pmu-soc on the command line causes QEMU to crash: ./qemu-system-microblazeel -M petalogix-ml605 -device xlnx-zynqmp-pmu-soc ** ERROR:tcg/tcg.c:813:tcg_register_thread: assertion failed: (n < tcg_max_ctxs) Bail out! Aborted (core dumped) Mark the device with "user_creatable = false" to avoid that this can happen. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2229 Message-ID: <20240322183153.1023359-1-thuth@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com> * aspeed: Make the ast2600-a3 SoC not user creatable Aspeed SoCs are complex devices that can not be specified on the command line. Fix that to avoid QEMU aborts. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2227 Fixes: f25c0ae1079d ("aspeed/soc: Add AST2600 support") Reported-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Cédric Le Goater <clg@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Message-ID: <20240319150903.413662-1-clg@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com> * aspeed: Make the ast1030-a1 SoC not user creatable Aspeed SoCs are complex devices that can not be specified on the command line. Fix that to avoid QEMU aborts. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2227 Fixes: 356b230ed138 ("aspeed/soc : Add AST1030 support") Reported-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Cédric Le Goater <clg@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Message-ID: <20240319150903.413662-2-clg@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com> * tests/qtest/npcm7xx_emc_test: Don't leak cmd_line In test_rx() and test_tx() we allocate a GString *cmd_line but never free it. This is pretty harmless in a test case, but Coverity spotted it. Resolves: Coverity CID 1507122 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-id: 20240312183810.557768-2-peter.maydell@linaro.org * tests/unit/socket-helpers: Don't close(-1) In socket_check_afunix_support() we call socket(PF_UNIX, SOCK_STREAM, 0) to see if it works, but we call close() on the result whether it worked or not. Only close the fd if the socket() call succeeded. Spotted by Coverity. Resolves: Coverity CID 1497481 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-id: 20240312183810.557768-3-peter.maydell@linaro.org * net/af-xdp.c: Don't leak sock_fds array in net_init_af_xdp() In net_init_af_xdp() we parse the arguments and allocate a buffer of ints into sock_fds. However, although we free this in the error exit path, we don't ever free it in the successful return path. Coverity spots this leak. Switch to g_autofree so we don't need to manually free the array. Resolves: Coverity CID 1534906 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-id: 20240312183810.557768-4-peter.maydell@linaro.org * hw/misc/pca9554: Correct error check bounds in get/set pin functions In pca9554_get_pin() and pca9554_set_pin(), we try to detect an incorrect pin value, but we get the condition wrong, using ">" when ">=" was intended. This has no actual effect, because in pca9554_initfn() we use the correct test when creating the properties and so we'll never be called with an out of range value. However, Coverity complains about the mismatch between the check and the later use of the pin value in a shift operation. Use the correct condition. Resolves: Coverity CID 1534917 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-id: 20240312183810.557768-5-peter.maydell@linaro.org * hw/nvram/mac_nvram: Report failure to write data There's no way for the macio_nvram device to report failure to write data, but we can at least report it to the user with error_report() as we do in other devices like xlnx-efuse. Spotted by Coverity. Resolves: Coverity CID 1507628 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Message-id: 20240312183810.557768-6-peter.maydell@linaro.org * misc/pca955*: Move models under hw/gpio The PCA9552 and PCA9554 devices are both I2C GPIO controllers and the PCA9552 also can drive LEDs. Do all the necessary adjustments to move the models under hw/gpio. Cc: Glenn Miles <milesg@linux.vnet.ibm.com> Signed-off-by: Cédric Le Goater <clg@redhat.com> Message-ID: <20240325134833.1484265-1-clg@redhat.com> Reviewed-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com> * libqos/virtio.c: Correct 'flags' reading in qvirtqueue_kick In qvirtqueue_kick(), the 'flags' were previously being incorrectly read from vq->avail instead of the correct vq->used location. This update ensures 'flags' are read from the correct location as per the virtio standard. Signed-off-by: Zheyu Ma <zheyuma97@gmail.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-ID: <20240320090442.267525-1-zheyuma97@gmail.com> Signed-off-by: Thomas Huth <thuth@redhat.com> * target/s390x: Use mutable temporary value for op_ts Otherwise TCG would assume the register that holds t1 would be constant and reuse whenever it needs the value within it. Cc: qemu-stable@nongnu.org Fixes: f1ea739bd598 ("target/s390x: Use tcg_constant_* in local contexts") Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> [iii: Adjust a newline and capitalization, add tags] Signed-off-by: Ido Plat <ido.plat@ibm.com> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> Reviewed-by: David Hildenbrand <david@redhat.com> Message-ID: <20240318202722.20675-1-iii@linux.ibm.com> Signed-off-by: Thomas Huth <thuth@redhat.com> * tests/tcg/s390x: Test TEST AND SET Add a small test to prevent regressions. Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-ID: <20240318202722.20675-2-iii@linux.ibm.com> Signed-off-by: Thomas Huth <thuth@redhat.com> * tests/unit/test-throttle: Avoid unintended integer division In test_compute_wait() we do double units = bkt.max / 10; which does an integer division and then assigns it to a double variable, and similarly later on in the expression for an assertion. Use 10.0 so that we do a floating point division and calculate the exact value, rather than doing an integer division. Spotted by Coverity. Resolves: Coverity CID 1432564 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Message-id: 20240312183810.557768-7-peter.maydell@linaro.org * tests/qtest/libqtest.c: Check for g_setenv() failure Coverity points out that g_setenv() can fail and we don't check for this in qtest_inproc_init(). In practice this will only fail if a memory allocation failed in setenv() or if the caller passed an invalid architecture name (e.g. one with an '=' in it), so rather than requiring the callsite to check for failure, make g_setenv() failure fatal here, similarly to what we did in commit aca68d95c515. Resolves: Coverity CID 1497485 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Message-id: 20240312183810.557768-8-peter.maydell@linaro.org * qapi: Improve migration TLS documentation MigrateSetParameters is about setting parameters, and MigrationParameters is about querying them. Their documentation of @tls-creds and @tls-hostname has residual damage from a failed attempt at de-duplicating them (see commit de63ab61241 "migrate: Share common MigrationParameters struct" and commit 1bda8b3c695 "migration: Unshare MigrationParameters struct for now"). MigrateSetParameters documentation issues: * It claims plain text mode "was reported by omitting tls-creds" before 2.9. MigrateSetParameters is not used for reporting, so this is misleading. Delete. * It similarly claims hostname defaulting to migration URI "was reported by omitting tls-hostname" before 2.9. Delete as well. Rephrase the remaining @tls-hostname contents for clarity. Enum MigrationParameter mirrors the members of struct MigrateSetParameters. Differences to MigrateSetParameters's member documentation are pointless. Copy the new text to MigrationParameter. MigrationParameters documentation issues: * @tls-creds runs the two last sentences together without punctuation. Fix that. * Much of the contents on @tls-hostname only applies to setting parameters, resulting in confusion. Replace by a suitable abridged version of the new MigrateSetParameters text, and a note on @tls-hostname omission in 2.8. Additional damage is due to flawed doc fix commit 66fcb9d651d (qapi/migration: Add missing tls-authz documentation): since it copied the missing MigrateSetParameters text from MigrationParameters instead of MigrationParameter, the part on recreating @tls-authz on the fly is missing. Copy that, too. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240322135117.195489-2-armbru@redhat.com> Reviewed-by: Peter Xu <peterx@redhat.com> [Some typos corrected] * qapi: Resync MigrationParameter and MigrateSetParameters Enum MigrationParameter mirrors the members of struct MigrateSetParameters. Differences to MigrateSetParameters's member documentation are pointless. Clean them up: * @compress-level, @compress-threads, @decompress-threads, and x-checkpoint-delay are more thoroughly documented for MigrationParameter, so use that version for both. * @max-cpu-throttle is almost the same. Use MigrationParameter's version for both. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240322135117.195489-3-armbru@redhat.com> Reviewed-by: Fabiano Rosas <farosas@suse.de> Reviewed-by: Peter Xu <peterx@redhat.com> * qapi: Fix bogus documentation of query-migrationthreads The doc comment documents an argument that doesn't exist. Would fail compilation if it was marked up correctly. Delete. The Returns: section fails to refer to the data type, leaving the user to guess. Fix that. The command name violates QAPI naming rules: it should be query-migration-threads. Too late to fix. Reported-by: John Snow <jsnow@redhat.com> Fixes: 671326201dac (migration: Introduce interface query-migrationthreads) Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240322135117.195489-4-armbru@redhat.com> Reviewed-by: Fabiano Rosas <farosas@suse.de> Reviewed-by: Peter Xu <peterx@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> * qapi: Drop stray Arguments: line from qmp_capabilities docs Reported-by: John Snow <jsnow@redhat.com> Fixes: 119ebac1feb2 (qapi-schema: use generated marshaller for 'qmp_capabilities') Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240322140910.328840-2-armbru@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> * qapi: Expand a few awkward abbreviations in documentation Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240322140910.328840-3-armbru@redhat.com> * qapi: Tidy up block-latency-histogram-set documentation some more Commit a937b6aa739 (qapi: Reformat doc comments to conform to current conventions) reflowed some text that should have been left alone. Revert that. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240322140910.328840-4-armbru@redhat.com> * qapi: Tidy up indentation of add_client's example Commit d23055b8db8 (qapi: Require descriptions and tagged sections to be indented) indented add_client's example too much. Revert that. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240322140910.328840-5-armbru@redhat.com> [Move a stray hunk to the later patch it belongs to] * qapi: Fix argument markup in drive-mirror documentation Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240322140910.328840-6-armbru@redhat.com> * qapi: Fix typo in request-ebpf documentation Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240322140910.328840-7-armbru@redhat.com> * qapi: Fix abbreviation punctuation in doc comments Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240322140910.328840-8-armbru@redhat.com> * qapi: Start sentences with a capital letter, end them with a period Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240322140910.328840-9-armbru@redhat.com> * qapi: Don't repeat member type in its documentation text Documentation generated for the arguments of MEMORY_FAILURE looks like "recipient": "MemoryFailureRecipient" recipient is defined as "MemoryFailureRecipient". "action": "MemoryFailureAction" action that has been taken. action is defined as "MemoryFailureAction". "flags": "MemoryFailureFlags" flags for MemoryFailureAction. action is defined as "MemoryFailureFlags". The "action is defined as ..." are redundant. Drop. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240322140910.328840-10-armbru@redhat.com> * qapi: Refill doc comments to conform to current conventions For legibility, wrap text paragraphs so every line is at most 70 characters long. To check the generated documentation does not change, I compared the generated HTML before and after this commit with "wdiff -3". Finds no differences. Comparing with diff is not useful, as the refilled paragraphs are visible there. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240322140910.328840-11-armbru@redhat.com> * qapi: Correct documentation indentation and whitespace Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240322140910.328840-12-armbru@redhat.com> [Add a previous patch's stray hunk] * qga/qapi-schema: Refill doc comments to conform to current conventions For legibility, wrap text paragraphs so every line is at most 70 characters long. To check the generated documentation does not change, I compared the generated HTML before and after this commit with "wdiff -3". Finds no differences. Comparing with diff is not useful, as the refilled paragraphs are visible there. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-ID: <20240322140910.328840-13-armbru@redhat.com> * qapi: document InputMultiTouchType Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-ID: <20240325095648.2835381-1-marcandre.lureau@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> [Update qapi/pragma.json] Signed-off-by: Markus Armbruster <armbru@redhat.com> * qapi: document leftover members in qapi/run-state.json Suggested-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-ID: <20240325104502.1358693-1-pbonzini@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> [Capitalize "ID", update qapi/pragma.json] Signed-off-by: Markus Armbruster <armbru@redhat.com> * qapi: document leftover members in qapi/stats.json Suggested-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-ID: <20240325104504.1358734-1-pbonzini@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> [Update qapi/pragma.json] Signed-off-by: Markus Armbruster <armbru@redhat.com> * qapi/block-core: improve Qcow2OverlapCheckFlags documentation Most of fields have no description at all. Let's fix that. Still, no reason to place here more detailed descriptions of what these structures are, as we have public Qcow2 format specification. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Message-ID: <20240325120054.2693236-1-vsementsov@yandex-team.ru> Acked-by: Markus Armbruster <armbru@redhat.com> [Capitalize "QEMU", update qapi/pragma.json] Signed-off-by: Markus Armbruster <armbru@redhat.com> * qapi: document parameters of query-cpu-model-* QAPI commands Let's document the parameters of these commands, so we can remove them from the "documentation-exceptions" list. While at it, extend the "Returns:" documentation as well, fixing a wrong use of CpuModelBaselineInfo vs. CpuModelCompareInfo for query-cpu-model-comparison. Cc: Markus Armbruster <armbru@redhat.com> Cc: Eric Blake <eblake@redhat.com> Cc: Eduardo Habkost <eduardo@habkost.net> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org> Cc: Yanan Wang <wangyanan55@huawei.com> Signed-off-by: David Hildenbrand <david@redhat.com> Message-ID: <20240325150141.342720-1-david@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> [Punctuation tweaked] Signed-off-by: Markus Armbruster <armbru@redhat.com> * tests/qemu-iotests: Test 157 and 227 require virtio-blk Tests 157 and 227 use the virtio-blk device, so we have to mark these tests accordingly to be skipped if this devices is not available (e.g. when running the tests with qemu-system-avr only). Signed-off-by: Thomas Huth <thuth@redhat.com> Message-ID: <20240325154737.1305063-1-thuth@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> * vdpa-dev: Fix initialisation order to restore VDUSE compatibility VDUSE requires that virtqueues are first enabled before the DRIVER_OK status flag is set; with the current API of the kernel module, it is impossible to enable the opposite order in our block export code because userspace is not notified when a virtqueue is enabled. This requirement also mathces the normal initialisation order as done by the generic vhost code in QEMU. However, commit 6c482547 accidentally changed the order for vdpa-dev and broke access to VDUSE devices with this. This changes vdpa-dev to use the normal order again and use the standard vhost callback .vhost_set_vring_enable for this. VDUSE devices can be used with vdpa-dev again after this fix. vhost_net intentionally avoided enabling the vrings for vdpa and does this manually later while it does enable them for other vhost backends. Reflect this in the vhost_net code and return early for vdpa, so that the behaviour doesn't change for this device. Cc: qemu-stable@nongnu.org Fixes: 6c4825476a43 ('vdpa: move vhost_vdpa_set_vring_ready to the caller') Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20240315155949.86066-1-kwolf@redhat.com> Reviewed-by: Eugenio Pérez <eperezma@redhat.com> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> * block/io: accept NULL qiov in bdrv_pad_request Some operations, e.g. block-stream, perform reads while discarding the results (only copy-on-read matters). In this case, they will pass NULL as the target QEMUIOVector, which will however trip bdrv_pad_request, since it wants to extend its passed vector. In particular, this is the case for the blk_co_preadv() call in stream_populate(). If there is no qiov, no operation can be done with it, but the bytes and offset still need to be updated, so the subsequent aligned read will actually be aligned and not run into an assertion failure. In particular, this can happen when the request alignment of the top node is larger than the allocated part of the bottom node, in which case padding becomes necessary. For example: > ./qemu-img create /tmp/backing.qcow2 -f qcow2 64M -o cluster_size=32768 > ./qemu-io -c "write -P42 0x0 0x1" /tmp/backing.qcow2 > ./qemu-img create /tmp/top.qcow2 -f qcow2 64M -b /tmp/backing.qcow2 -F qcow2 > ./qemu-system-x86_64 --qmp stdio \ > --blockdev qcow2,node-name=node0,file.driver=file,file.filename=/tmp/top.qcow2 \ > <<EOF > {"execute": "qmp_capabilities"} > {"execute": "blockdev-add", "arguments": { "driver": "compress", "file": "node0", "node-name": "node1" } } > {"execute": "block-stream", "arguments": { "job-id": "stream0", "device": "node1" } } > EOF Originally-by: Stefan Reiter <s.reiter@proxmox.com> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com> [FE: do update bytes and offset in any case add reproducer to commit message] Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Message-ID: <20240322095009.346989-2-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> * block-backend: fix edge case in bdrv_next() where BDS associated to BB changes The old_bs variable in bdrv_next() is currently determined by looking at the old block backend. However, if the block graph changes before the next bdrv_next() call, it might be that the associated BDS is not the same that was referenced previously. In that case, the wrong BDS is unreferenced, leading to an assertion failure later: > bdrv_unref: Assertion `bs->refcnt > 0' failed. In particular, this can happen in the context of bdrv_flush_all(), when polling for bdrv_co_flush() in the generated co-wrapper leads to a graph change (for example with a stream block job [0]). A racy reproducer: > #!/bin/bash > rm -f /tmp/backing.qcow2 > rm -f /tmp/top.qcow2 > ./qemu-img create /tmp/backing.qcow2 -f qcow2 64M > ./qemu-io -c "write -P42 0x0 0x1" /tmp/backing.qcow2 > ./qemu-img create /tmp/top.qcow2 -f qcow2 64M -b /tmp/backing.qcow2 -F qcow2 > ./qemu-system-x86_64 --qmp stdio \ > --blockdev qcow2,node-name=node0,file.driver=file,file.filename=/tmp/top.qcow2 \ > <<EOF > {"execute": "qmp_capabilities"} > {"execute": "block-stream", "arguments": { "job-id": "stream0", "device": "node0" } } > {"execute": "quit"} > EOF [0]: > #0 bdrv_replace_child_tran (child=..., new_bs=..., tran=...) > #1 bdrv_replace_node_noperm (from=..., to=..., auto_skip=..., tran=..., errp=...) > #2 bdrv_replace_node_common (from=..., to=..., auto_skip=..., detach_subchain=..., errp=...) > #3 bdrv_drop_filter (bs=..., errp=...) > #4 bdrv_cor_filter_drop (cor_filter_bs=...) > #5 stream_prepare (job=...) > #6 job_prepare_locked (job=...) > #7 job_txn_apply_locked (fn=..., job=...) > #8 job_do_finalize_locked (job=...) > #9 job_exit (opaque=...) > #10 aio_bh_poll (ctx=...) > #11 aio_poll (ctx=..., blocking=...) > #12 bdrv_poll_co (s=...) > #13 bdrv_flush (bs=...) > #14 bdrv_flush_all () > #15 do_vm_stop (state=..., send_stop=...) > #16 vm_shutdown () Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Message-ID: <20240322095009.346989-3-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> * block-backend: fix edge case in bdrv_next_cleanup() where BDS associated to BB changes Same rationale as for commit "block-backend: fix edge case in bdrv_next() where BDS associated to BB changes". The block graph might change between the bdrv_next() call and the bdrv_next_cleanup() call, so it could be that the associated BDS is not the same that was referenced previously anymore. Instead, rely on bdrv_next() to set it->bs to the BDS it referenced and unreference that one in any case. Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Message-ID: <20240322095009.346989-4-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> * iotests: add test for stream job with an unaligned prefetch read Previously, bdrv_pad_request() could not deal with a NULL qiov when a read needed to be aligned. During prefetch, a stream job will pass a NULL qiov. Add a test case to cover this scenario. By accident, also covers a previous race during shutdown, where block graph changes during iteration in bdrv_flush_all() could lead to unreferencing the wrong block driver state and an assertion failure later. Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Message-ID: <20240322095009.346989-5-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> * target/i386/tcg: Enable page walking from MMIO memory CXL emulation of interleave requires read and write hooks due to requirement for subpage granularity. The Linux kernel stack now enables using this memory as conventional memory in a separate NUMA node. If a process is deliberately forced to run from that node $ numactl --membind=1 ls the page table walk on i386 fails. Useful part of backtrace: (cpu=cpu@entry=0x555556fd9000, fmt=fmt@entry=0x555555fe3378 "cpu_io_recompile: could not find TB for pc=%p") at ../../cpu-target.c:359 (retaddr=0, addr=19595792376, attrs=..., xlat=<optimized out>, cpu=0x555556fd9000, out_offset=<synthetic pointer>) at ../../accel/tcg/cputlb.c:1339 (cpu=0x555556fd9000, full=0x7fffee0d96e0, ret_be=ret_be@entry=0, addr=19595792376, size=size@entry=8, mmu_idx=4, type=MMU_DATA_LOAD, ra=0) at ../../accel/tcg/cputlb.c:2030 (cpu=cpu@entry=0x555556fd9000, p=p@entry=0x7ffff56fddc0, mmu_idx=<optimized out>, type=type@entry=MMU_DATA_LOAD, memop=<optimized out>, ra=ra@entry=0) at ../../accel/tcg/cputlb.c:2356 (cpu=cpu@entry=0x555556fd9000, addr=addr@entry=19595792376, oi=oi@entry=52, ra=ra@entry=0, access_type=access_type@entry=MMU_DATA_LOAD) at ../../accel/tcg/cputlb.c:2439 at ../../accel/tcg/ldst_common.c.inc:301 at ../../target/i386/tcg/sysemu/excp_helper.c:173 (err=0x7ffff56fdf80, out=0x7ffff56fdf70, mmu_idx=0, access_type=MMU_INST_FETCH, addr=18446744072116178925, env=0x555556fdb7c0) at ../../target/i386/tcg/sysemu/excp_helper.c:578 (cs=0x555556fd9000, addr=18446744072116178925, size=<optimized out>, access_type=MMU_INST_FETCH, mmu_idx=0, probe=<optimized out>, retaddr=0) at ../../target/i386/tcg/sysemu/excp_helper.c:604 Avoid this by plumbing the address all the way down from x86_cpu_tlb_fill() where is available as retaddr to the actual accessors which provide it to probe_access_full() which already handles MMIO accesses. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2180 Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2220 Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Suggested-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Gregory Price <gregory.price@memverge.com> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Message-ID: <20240307155304.31241-2-Jonathan.Cameron@huawei.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> * hw/scsi/scsi-generic: Fix io_timeout property not applying The io_timeout property, introduced in c9b6609 (part of 6.0) is silently overwritten by the hardcoded default value of 30 seconds (DEFAULT_IO_TIMEOUT) in scsi_generic_realize because that function is being called after the properties have already been applied. The property definition already has a default value which is applied correctly when no value is explicitly set, so we can just remove the code which overrides the io_timeout completely. This has been tested by stracing SG_IO operations with the io_timeout property set and unset and now sets the timeout field in the ioctl request to the proper value. Fixes: c9b6609b69facad ("scsi: make io_timeout configurable") Signed-off-by: Lorenz Brun <lorenz@brun.one> Message-ID: <20240315145831.2531695-1-lorenz@brun.one> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> * monitor/hmp-cmds-target: Append a space in error message in gpa2hva() In qemu monitor mode, when we use gpa2hva command to print the host virtual address corresponding to a guest physical address, if the gpa is not in RAM, the error message is below: (qemu) gpa2hva 0x750000000 Memory at address 0x750000000is not RAM A space is missed between '0x750000000' and 'is'. Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com> Fixes: e9628441df ("hmp: gpa2hva and gpa2hpa hostaddr command") Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Dr. David Alan Gilbert <dave@treblig.org> Message-ID: <20240319021610.2423844-1-ruansy.fnst@fujitsu.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> * target/tricore/helper: Use correct string format in cpu_tlb_fill() 'address' got converted from target_ulong to vaddr in commit 68d6eee73c ("target/tricore: Convert to CPUClass::tlb_fill"). Use the corresponding format string to avoid casting. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Message-ID: <20240319051413.6956-1-philmd@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de> * hw/clock: Let clock_set_mul_div() return a boolean value Let clock_set_mul_div() return a boolean value whether the clock has been updated or not, similarly to clock_set(). Return early when clock_set_mul_div() is called with same mul/div values the clock has. Acked-by: Luc Michel <luc@lmichel.fr> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Message-Id: <20240325152827.73817-2-philmd@linaro.org> * hw/misc/stm32l4x5_rcc: Inline clock_update() in clock_mux_update() Trivial inlining in preliminary patch to make the next one easier to review. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Message-Id: <20240325152827.73817-3-philmd@linaro.org> * hw/misc/stm32l4x5_rcc: Propagate period when enabling a clock The "clock_set_mul_div" function doesn't propagate the clock period to the children if it is changed (e.g. by enabling/disabling a clock multiplexer). This was overlooked during the implementation due to late changes. This commit propagates the change if the multiplier or divider changes. Fixes: ec7d83acbd ("hw/misc/stm32l4x5_rcc: Add an internal clock multiplexer object") Signed-off-by: Arnaud Minier <arnaud.minier@telecom-paris.fr> Signed-off-by: Inès Varhol <ines.varhol@telecom-paris.fr> Message-ID: <20240317103918.44375-2-arnaud.minier@telecom-paris.fr> [PMD: Check clock_set_mul_div() return value] Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Message-Id: <20240325152827.73817-4-philmd@linaro.org> * docs/system/ppc/amigang.rst: Fix formatting Add missing space to fix character formatting where it was missed in two places. Fixes: 623d9065b6 (docs/system/ppc: Document running Linux on AmigaNG machines) Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-ID: <20240324161148.4650D4E601F@zero.eik.bme.hu> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> * contrib/plugins/execlog: Fix compiler warning 1. The g_pattern_match_string() is deprecated when glib2 version >= 2.70. Use g_pattern_spec_match_string() instead to avoid this problem. 2. The type of second parameter in g_ptr_array_add() is 'gpointer' {aka 'void *'}, but the type of reg->name is 'const char*'. Cast the type of reg->name to 'gpointer' to avoid this problem. compiler warning message: contrib/plugins/execlog.c:330:17: warning: ‘g_pattern_match_string’ is deprecated: Use 'g_pattern_spec_match_string' instead [-Wdeprecated-declarations] 330 | if (g_pattern_match_string(pat, rd->name) || | ^~ In file included from /usr/include/glib-2.0/glib.h:67, from contrib/plugins/execlog.c:9: /usr/include/glib-2.0/glib/gpattern.h:57:15: note: declared here 57 | gboolean g_pattern_match_string (GPatternSpec *pspec, | ^~~~~~~~~~~~~~~~~~~~~~ contrib/plugins/execlog.c:331:21: warning: ‘g_pattern_match_string’ is deprecated: Use 'g_pattern_spec_match_string' instead [-Wdeprecated-declarations] 331 | g_pattern_match_string(pat, rd_lower)) { | ^~~~~~~~~~~~~~~~~~~~~~ /usr/include/glib-2.0/glib/gpattern.h:57:15: note: declared here 57 | gboolean g_pattern_match_string (GPatternSpec *pspec, | ^~~~~~~~~~~~~~~~~~~~~~ contrib/plugins/execlog.c:339:63: warning: passing argument 2 of ‘g_ptr_array_add’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] 339 | g_ptr_array_add(all_reg_names, reg->name); | ~~~^~~~~~ In file included from /usr/include/glib-2.0/glib.h:33: /usr/include/glib-2.0/glib/garray.h:198:62: note: expected ‘gpointer’ {aka ‘void *’} but argument is of type ‘const char *’ 198 | gpointer data); | ~~~~~~~~~~~~~~~~~~^~~~ Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2210 Signed-off-by: Yao Xingtao <yaoxt.fnst@fujitsu.com> Message-ID: <20240326015257.21516-1-yaoxt.fnst@fujitsu.com> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> * hw/smbios: add stub for smbios_get_table_legacy() QEMU build fails with hw/i386/fw_cfg.c:74: undefined reference to `smbios_get_table_legacy' when it's built with only 'microvm' enabled i.e. with config patch +++ b/configs/devices/i386-softmmu/default.mak @@ -26,7 +26,7 @@ # Boards: # -CONFIG_ISAPC=y -CONFIG_I440FX=y -CONFIG_Q35=y +CONFIG_ISAPC=n +CONFIG_I440FX=n +CONFIG_Q35=n It happens because I've fogotten/lost smbios_get_table_legacy() stub. Fix it by adding missing stub as Philippe suggested. Fixes: b42b0e4daaa5 "smbios: build legacy mode code only for 'pc' machine" Reported-by: Michael Tokarev <mjt@tls.msk.ru> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Igor Mammedov <imammedo@redhat.com> Message-ID: <20240326122630.85989-1-imammedo@redhat.com> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org> * ui/cocoa: Fix aspect ratio [NSWindow setContentAspectRatio:] does not trigger window resize itself, so the wrong aspect ratio will persist if nothing resizes the window. Call [NSWindow setContentSize:] in such a case. Fixes: 91aa508d0274 ("ui/cocoa: Let the platform toggle fullscreen") Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-ID: <20240323-fixes-v2-1-18651a2b0394@daynix.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> * ui/cocoa: Resize window after toggling zoom-to-fit Resize the window so that the content will fit without zooming. Fixes: 91aa508d0274 ("ui/cocoa: Let the platform toggle fullscreen") Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-ID: <20240323-fixes-v2-2-18651a2b0394@daynix.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> * ui/cocoa: Use NSTrackingInVisibleRect I observed [NSTrackingArea rect] becomes de-synchronized with the view frame with some unknown condition, and fails to track mouse movement on some area of the view. Specify NSTrackingInVisibleRect option to let Cocoa automatically update NSTrackingArea, which also saves code for synchronization. Fixes: 91aa508d0274 ("ui/cocoa: Let the platform toggle fullscreen") Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-ID: <20240323-fixes-v2-3-18651a2b0394@daynix.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> * Update version for v9.0.0-rc1 release Signed-off-by: Peter Maydell <peter.maydell@linaro.org> * linux-user: Fix semctl() strace The indices of arguments used with semctl() are all off-by-1, because arg1 is the ipc() command. Fix them. While at it, reuse print_semctl(). New output (for a small test program): 3540333 semctl(999,888,SEM_INFO,0x00007fe5051ee9a0) = -1 errno=14 (Bad address) Fixes: 7ccfb2eb5f9d ("Fix warnings that would be caused by gcc flag -Wwrite-strings") Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> Message-Id: <20240325192436.561154-2-iii@linux.ibm.com> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> * linux-user: Fix shmat() strace The indices of arguments passed to print_shmat() are all off-by-1, because arg1 is the ipc() command. Fix them. New output for linux-shmat-maps test: 3501769 shmat(4784214,0x0000000000800000,SHM_RND) = 0 Fixes: 9f7c97324c27 ("linux-user: Add strace for shmat") Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> Message-Id: <20240325192436.561154-3-iii@linux.ibm.com> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> * linux-user: Fix shmat(NULL) for h != g In the h != g && shmaddr == NULL && !reserved_va case, target_shmat() incorrectly mmap()s the initial anonymous range with MAP_FIXED_NOREPLACE, even though the earlier mmap_find_vma() has already reserved the respective address range. Fix by using MAP_FIXED when "mapped", which is set after mmap_find_vma(), is true. Fixes: 78bc8ed9a8f0 ("linux-user: Rewrite target_shmat") Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> Message-Id: <20240325192436.561154-4-iii@linux.ibm.com> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> * tests/tcg: Test shmat(NULL) Add a small test to prevent regressions. Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> Message-Id: <20240325192436.561154-5-iii@linux.ibm.com> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> * target/hppa: Fix BE,L set of sr0 The return address comes from IA*Q_Next, and IASQ_Next is always equal to IASQ_Back, not IASQ_Front. Tested-by: Helge Deller <deller@gmx.de> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> * target/hppa: Fix B,GATE for wide mode Do not clobber the high bits of the address by using a 32-bit deposit. Reviewed-by: Helge Deller <deller@gmx.de> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> * target/hppa: Handle unit conditions for wide mode Wide mode provides two more conditions, add them. Fixes: 59963d8fdf42 ("target/hppa: Pass d to do_unit_cond") Signed-off-by: Sven Schnelle <svens@stackframe.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-Id: <20240321184228.611897-1-svens@stackframe.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> * target/hppa: Fix ADD/SUB trap on overflow for narrow mode Fixes: c53e401ed9ff ("target/hppa: Remove TARGET_REGISTER_BITS") Signed-off-by: Sven Schnelle <svens@stackframe.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-Id: <20240321184228.611897-2-svens@stackframe.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> * target/hppa: Mark interval timer write as io Reviewed-by: Helge Deller <deller@gmx.de> Tested-by: Helge Deller <deller@gmx.de> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> * target/hppa: Tidy read of interval timer The call to gen_helper_read_interval_timer is identical on both sides of the IF. Reviewed-by: Helge Deller <deller@gmx.de> Tested-by: Helge Deller <deller@gmx.de> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> * target/hppa: Fix EIRR, EIEM versus icount Call translator_io_start before write to EIRR. Move evaluation of EIRR vs EIEM to hppa_cpu_exec_interrupt. Exit TB after write to EIEM, but otherwise use a straight store. Reviewed-by: Helge Deller <deller@gmx.de> Tested-by: Helge Deller <deller@gmx.de> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> * target/hppa: Use gva_offset_mask() everywhere Move it to cpu.h, so it can also be used in hppa_form_gva_psw(). Signed-off-by: Sven Schnelle <svens@stackframe.org> Reviewed-by: Helge Deller <deller@gmx.de> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-Id: <20240324080945.991100-2-svens@stackframe.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> * target/hppa: Fix DCOR reconstruction of carry bits The carry bits for each nibble N are located in bit (N+1)*4, so the shift by 3 was off by one. Furthermore, the carry bit for the most significant carry bit is indeed located in bit 64, which is located in a different storage word. Use a double-word shift-right to reassemble into a single word and place them all at bit 0 of their respective nibbles. Tested-by: Helge Deller <deller@gmx.de> Reviewed-by: Helge Deller <deller@gmx.de> Fixes: b2167459ae4 ("target-hppa: Implement basic arithmetic") Signed-off-by: Richard Henderson <richard.henderson@linaro.org> * target/hppa: Optimize UADDCM with no condition With r1 as zero is by far the most common usage of UADDCM, as the easiest way to invert a register. The compiler does occasionally use the addition step as well, and we can simplify that to avoid a temp and write directly into the destination. Tested-by: Helge Deller <deller@gmx.de> Reviewed-by: Helge Deller <deller@gmx.de> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> * target/hppa: Fix unit carry conditions Split do_unit_cond to do_unit_zero_cond to only handle conditions versus zero. These are the only ones that are legal for UXOR. Simplify trans_uxor accordingly. Rename do_unit to do_unit_addsub, since xor has been split. Properly compute carry-out bits for add and subtract, mirroring the code in do_add and do_sub. Tested-by: Helge Deller <deller@gmx.de> Reviewed-by: Helge Deller <deller@gmx.de> Fixes: b2167459ae4 ("target-hppa: Implement basic arithmetic") Signed-off-by: Richard Henderson <richard.henderson@linaro.org> * target/hppa: Squash d for pa1.x during decode The cond_need_ext predicate was created while we still had a 32-bit compilation mode. It now makes more sense to treat D as an absolute indicator of a 64-bit operation. Tested-by: Helge Deller <deller@gmx.de> Reviewed-by: Helge Deller <deller@gmx.de> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> * target/hppa: Replace c with uv in do_cond Prepare for proper indication of shladd unsigned overflow. The UV indicator will be zero/not-zero instead of a single bit. Tested-by: Helge Deller <deller@gmx.de> Reviewed-by: Helge Deller <deller@gmx.de> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> * target/hppa: Fix overflow computation for shladd Overflow indicator should include the effect of the shift step. We had previously left ??? comments about the issue. Tested-by: Helge Deller <deller@gmx.de> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> * qtest/virtio-9p-test.c: create/remove temp dirs after each test The local 9p driver in virtio-9p-test.c its temporary dir right at the start of qos-test (via virtio_9p_create_local_test_dir()) and only deletes it after qos-test is finished (via virtio_9p_remove_local_test_dir()). This means that any qos-test machine that ends up running virtio-9p-test local tests more than once will end up re-using the same temp dir. This is what's happening in [1] after we introduced the riscv machine nodes: if we enable slow tests with the '-m slow' flag using qemu-system-riscv64, this is what happens: - a temp dir is created; - virtio-9p-device tests will run virtio-9p-test successfully; - virtio-9p-pci tests will run virtio-9p-test, and fail right at the first slow test at fs_create_dir() because the "01" file was already created by fs_create_dir() test when running with the virtio-9p-device. The root cause is that we're creating a single temporary dir, via the construct/destruct callbacks, and this temp dir is kept for the entire qos-test run. We can change each test to clean after themselves. This approach would make the 'create' tests obsolete since we would need to create and delete dirs/files/symlinks for the cleanup, turning them into the 'unlinkat' tests that comes right after. We chose a different approach that handles the root cause: do not use constructor/destructor to create the temp dir. Create one temp dir for each test, and remove it after the test is complete. This is the approach taken for other qtests like vhost-user-test.c where each test requires a setup() and a subsequent cleanup(), all of those instantiated in the .before callback. [1] https://mail.gnu.org/archive/html/qemu-devel/2024-03/msg05807.html Reported-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> Message-Id: <20240327142011.805728-2-dbarboza@ventanamicro.com> Reviewed-by: Greg Kurz <groug@kaod.org> Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Tested-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> * qtest/virtio-9p-test.c: remove g_test_slow() gate Commit 558f5c42ef gated the local tests with g_test_slow() to skip them in 'make check'. The reported issue back then was this following CI problem: https://lists.nongnu.org/archive/html/qemu-devel/2020-11/msg05510.html This problem ended up being fixed after it was detected with the recently added risc-v machine nodes [1]. virtio-9p-test.c is now creating and removing temporary dirs for each test run, instead of creating a single dir for the entire qos-test scope. We're now able to run these tests with 'make check' in the CI, so let's go ahead and re-enable them. This reverts commit 558f5c42efded3e0d0b20a90bce2a9a14580d824. [1] https://mail.gnu.org/archive/html/qemu-devel/2024-03/msg05807.html Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> Message-Id: <20240327142011.805728-3-dbarboza@ventanamicro.com> Reviewed-by: Greg Kurz <groug@kaod.org> Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com> Tested-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com> * virtio-net: Fix vhost virtqueue notifiers for RSS virtio_net_guest_notifier_pending() and virtio_net_guest_notifier_mask() checked VIRTIO_NET_F_MQ to know there are multiple queues, but VIRTIO_NET_F_RSS also enables multiple queues. Refer to n->multiqueue, which is set to true either of VIRTIO_NET_F_MQ or VIRTIO_NET_F_RSS is enabled. Fixes: 68b0a6395f36 ("virtio-net: align ctrl_vq index for non-mq guest for vhost_vdpa") Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> Signed-off-by: Jason Wang <jasowang@redhat.com> * ebpf: Fix indirections table setting The kernel documentation says: > The value stored can be of any size, however, all array elements are > aligned to 8 bytes. https://www.kernel.org/doc/html/v6.8/bpf/map_array.html Fixes: 333b3e5fab75 ("ebpf: Added eBPF map update through mmap.") Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> Acked-by: Andrew Melnychenko <andrew@daynix.com> Signed-off-by: Jason Wang <jasowang@redhat.com> * hw/net/net_tx_pkt: Fix virtio header without checksum offloading It is incorrect to have the VIRTIO_NET_HDR_F_NEEDS_CSUM set when checksum offloading is disabled so clear the bit. TCP/UDP checksum is usually offloaded when the peer requires virtio headers because they can instruct the peer to compute checksum. However, igb disables TX checksum offloading when a VF is enabled whether the peer requires virtio headers because a transmitted packet can be routed to it and it expects the packet has a proper checksum. Therefore, it is necessary to have a correct virtio header even when checksum offloading is disabled. A real TCP/UDP checksum will be computed and saved in the buffer when checksum offloading is disabled. The virtio specification requires to set the packet checksum stored in the buffer to the TCP/UDP pseudo header when the VIRTIO_NET_HDR_F_NEEDS_CSUM bit is set so the bit must be cleared in that case. Fixes: ffbd2dbd8e64 ("e1000e: Perform software segmentation for loopback") Buglink: https://issues.redhat.com/browse/RHEL-23067 Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> Signed-off-by: Jason Wang <jasowang@redhat.com> * tap-win32: Remove unnecessary stubs Some of them are only necessary for POSIX systems. The others are assigned to function pointers in NetClientInfo that can actually be NULL. Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> Signed-off-by: Jason Wang <jasowang@redhat.com> * Revert "tap: setting error appropriately when calling net_init_tap_one()" This reverts commit 46d4d36d0bf2b24b205f2f604f0905db80264eef. The reverted commit changed to emit warnings instead of errors when vhost is requested but vhost initialization fails if vhostforce option is not set. However, vhostforce is not meant to ignore vhost errors. It was once introduced as an option to commit 5430a28fe4 ("vhost: force vhost off for non-MSI guests") to force enabling vhost for non-MSI guests, which will have worse performance with vhost. The option was deprecated with commit 1e7398a140 ("vhost: enable vhost without without MSI-X") and changed to behave identical with the vhost option for compatibility. Worse, commit bf769f742c ("virtio: del net client if net_init_tap_one failed") changed to delete the client when vhost fails even when the failure only results in a warning. The leads to an assertion failure for the -netdev command line option. The reverted commit was intended to avoid that the vhost initialization failure won't result in a corrupted netdev. This problem should have been fixed by deleting netdev when the initialization fails instead of ignoring the failure with an arbitrary option. Fortunately, commit bf769f742c ("virtio: del net client if net_init_tap_one failed"), mentioned earlier, implements this behavior. Restore the correct semantics and fix the assertion failure for the -netdev command line option by reverting the problematic commit. Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> Signed-off-by: Jason Wang <jasowang@redhat.com> * target/hppa: Generate getshadowregs inline This operation is trivial and does not require a helper. Reviewed-by: Helge Deller <deller@gmx.de> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> * target/hppa: Move diag argument handling to decodetree Split trans_diag into per-operation functions. Reviewed-by: Helge Deller <deller@gmx.de> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> * target/hppa: Add diag instructions to set/restore shadow registers The 32-bit PA-7300LC (PCX-L2) CPU and the 64-bit PA8700 (PCX-W2) CPU use different diag instructions to save or restore the CPU registers to/from the shadow registers. Implement those per-CPU architecture diag instructions to fix those parts of the HP ODE testcases (L2DIAG and WDIAG, section 1) which test the shadow registers. Signed-off-by: Helge Deller <deller@gmx.de> [rth: Use decodetree to distinguish cases] Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Helge Deller <deller@gmx.de> Tested-by: Helge Deller <deller@gmx.de> * target/hppa: Clear psw_n for BE on use_nullify_skip path Along this path we have already skipped the insn to be nullified, so the subsequent insn should be executed. Cc: qemu-stable@nongnu.org Reported-by: Sven Schnelle <svens@stackframe.org> Tested-by: Sven Schnelle <svens@stackframe.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> * tcg/optimize: Fix sign_mask for logical right-shift The 'sign' computation is attempting to locate the sign bit that has been repeated, so that we can test if that bit is known zero. That computation can be zero if there are no known sign repetitions. Cc: qemu-stable@nongnu.org Fixes: 93a967fbb57 ("tcg/optimize: Propagate sign info for shifting") Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2248 Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> * disas: Show opcodes for target_disas and monitor_disas Fixes: 83b4613ba83 ("disas: introduce show_opcodes") Signed-off-by: Richard Henderson <richard.henderson@linaro.org> * accel/tcg: Use CPUState.get_pc in cpu_io_recompile Using log_pc produces the pc at the beginning of TB, not the actual pc installed by cpu_restore_state_from_tb, which could be any of the guest instructions within TB. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> * spapr: nested: use bitwise NOT operator for flags check Check for flag bit in H_GUEST_GETSET_STATE_FLAG_GUEST_WIDE need to use bitwise NOT operator to ensure no other flag bits are set. Resolves: Coverity CID 1540008 Resolves: Coverity CID 1540009 Reported-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off by: Harsh Prateek Bora <harshpb@linux.ibm.com> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> * hw/ppc/spapr: Include missing 'sysemu/tcg.h' header "sysemu/tcg.h" declares tcg_enabled(), and is implicitly included. Include it explicitly to avoid the following error when refactoring headers: hw/ppc/spapr.c:2612:9: error: call to undeclared function 'tcg_enabled'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] if (tcg_enabled()) { ^ Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> * target/ppc/mmu-radix64: Use correct string format in walk_tree() 'mask', 'nlb' and 'base_addr' are all uin64_t types. Use the corresponding PRIx64 format. Fixes: d2066bc50d ("target/ppc: Check page dir/table base alignment") Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> * target/ppc: Restore [H]DEXCR to 64-bits The DEXCR emulation was recently changed to a 32-bit register, possibly because it does have a 32-bit read-only view. It is a full 64-bit SPR though, so use the corresponding 64-bit write functions. Fixes: fbda88f7abdee ("target/ppc: Fix width of some 32-bit SPRs") Reviewed-by: Nicholas Piggin <npiggin@gmail.com> Signed-off-by: Benjamin Gray <bgray@linux.ibm.com> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> * target/ppc: Fix GDB register indexing on secondary CPUs The GDB server protocol assigns an arbitrary numbering of the SPRs. We track this correspondence on each SPR with gdb_id, using it to resolve any SPR requests GDB makes. Early on we generate an XML representation of the SPRs to give GDB, including this numbering. However the XML is cached globally, and we skip setting the SPR gdb_id values on subsequent threads if we detect it is cached. This causes QEMU to fail to resolve SPR requests against secondary CPUs because it cannot find the matching gdb_id value on that thread's SPRs. This is a minimal fix to first assign the gdb_id values, then return early if the XML is cached. Otherwise we generate the XML using the now already initialised gdb_id values. Fixes: 1b53948ff8f7 ("target/ppc: Use GDBFeature for dynamic XML") Reviewed-by: Nicholas Piggin <npiggin@gmail.com> Signed-off-by: Benjamin Gray <bgray@linux.ibm.com> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> * target/ppc: Do not clear MSR[ME] on MCE interrupts to supervisor Hardware clears the MSR[ME] bit when delivering a machine check interrupt, so that is what QEMU does. The spapr environment runs in supervisor mode though, and receives machine check interrupts after they are processed by the hypervisor, and MSR[ME] must always be enabled in supervisor mode (otherwise it could checkstop the system). So MSR[ME] must not be cleared when delivering machine checks to the supervisor. The fix to prevent supervisor mode from modifying MSR[ME] also prevented it from re-enabling the incorrectly cleared MSR[ME] bit when returning from handling the interrupt. Before that fix, the problem was not very noticable with well-behaved code. So the Fixes tag is not strictly correct, but practically they go together. Found by kvm-unit-tests machine check tests (not yet upstream). Fixes: 678b6f1af75ef ("target/ppc: Prevent supervisor from modifying MSR[ME]") Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> * tests/avocado: Fix ppc_hv_tests.py xorriso dependency guard For some reason the skipIf missing_deps() check fails to skip the test if it comes after the skipUnless lines, causing an error running on systems without xorriso. Avocado implements skipUnless is just an inverted skipIf, so it's not clear what the bug is or why this fixes it. For now it's enough to get things working. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2246 Fixes: c9cb496710758 ("tests/avocado: ppc add hypervisor tests") Signed-off-by: Nicholas Piggin <npiggin@gmail.com> * tests/avocado: ppc_hv_tests.py set alpine time before setup-alpine If the time is wrong, setup-alpine SSL certificate checks can fail. setup-alpine is used to bring up the network, but it doesn't seem to to set NTP time before the failing SSL checks. This test has recently started failing presumably because the default time has now fallen too far behind. Fix this by setting time from the host time before running setup-alpine. Fixes: c9cb496710758 ("tests/avocado: ppc add hypervisor tests") Signed-off-by: Nicholas Piggin <npiggin@gmail.com> * migration: Set migration error in migration_completion…
rmalmain
pushed a commit
that referenced
this pull request
Jun 18, 2024
…coroutine()' failed. bdrv_activate_all() should not be called from the coroutine context, move it to the QEMU thread colo_process_incoming_thread() with the bql_lock protected. The backtrace is as follows: #4 0x0000561af7948362 in bdrv_graph_rdlock_main_loop () at ../block/graph-lock.c:260 #5 0x0000561af7907a68 in graph_lockable_auto_lock_mainloop (x=0x7fd29810be7b) at /patch/to/qemu/include/block/graph-lock.h:259 #6 0x0000561af79167d1 in bdrv_activate_all (errp=0x7fd29810bed0) at ../block.c:6906 #7 0x0000561af762b4af in colo_incoming_co () at ../migration/colo.c:935 #8 0x0000561af7607e57 in process_incoming_migration_co (opaque=0x0) at ../migration/migration.c:793 #9 0x0000561af7adbeeb in coroutine_trampoline (i0=-106876144, i1=22042) at ../util/coroutine-ucontext.c:175 #10 0x00007fd2a5cf21c0 in () at /lib64/libc.so.6 Cc: qemu-stable@nongnu.org Cc: Fabiano Rosas <farosas@suse.de> Closes: https://gitlab.com/qemu-project/qemu/-/issues/2277 Fixes: 2b3912f ("block: Mark bdrv_first_blk() and bdrv_is_root_node() GRAPH_RDLOCK") Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> Reviewed-by: Zhang Chen <chen.zhang@intel.com> Tested-by: Zhang Chen <chen.zhang@intel.com> Reviewed-by: Fabiano Rosas <farosas@suse.de> Link: https://lore.kernel.org/r/20240417025634.1014582-1-lizhijian@fujitsu.com Signed-off-by: Peter Xu <peterx@redhat.com> (cherry picked from commit 2cc637f1ea08d2a1b19fc5b1a30bc609f948de93) Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
rmalmain
pushed a commit
that referenced
this pull request
Jun 18, 2024
…B changes The old_bs variable in bdrv_next() is currently determined by looking at the old block backend. However, if the block graph changes before the next bdrv_next() call, it might be that the associated BDS is not the same that was referenced previously. In that case, the wrong BDS is unreferenced, leading to an assertion failure later: > bdrv_unref: Assertion `bs->refcnt > 0' failed. In particular, this can happen in the context of bdrv_flush_all(), when polling for bdrv_co_flush() in the generated co-wrapper leads to a graph change (for example with a stream block job [0]). A racy reproducer: > #!/bin/bash > rm -f /tmp/backing.qcow2 > rm -f /tmp/top.qcow2 > ./qemu-img create /tmp/backing.qcow2 -f qcow2 64M > ./qemu-io -c "write -P42 0x0 0x1" /tmp/backing.qcow2 > ./qemu-img create /tmp/top.qcow2 -f qcow2 64M -b /tmp/backing.qcow2 -F qcow2 > ./qemu-system-x86_64 --qmp stdio \ > --blockdev qcow2,node-name=node0,file.driver=file,file.filename=/tmp/top.qcow2 \ > <<EOF > {"execute": "qmp_capabilities"} > {"execute": "block-stream", "arguments": { "job-id": "stream0", "device": "node0" } } > {"execute": "quit"} > EOF [0]: > #0 bdrv_replace_child_tran (child=..., new_bs=..., tran=...) > #1 bdrv_replace_node_noperm (from=..., to=..., auto_skip=..., tran=..., errp=...) > #2 bdrv_replace_node_common (from=..., to=..., auto_skip=..., detach_subchain=..., errp=...) > #3 bdrv_drop_filter (bs=..., errp=...) > #4 bdrv_cor_filter_drop (cor_filter_bs=...) > #5 stream_prepare (job=...) > #6 job_prepare_locked (job=...) > #7 job_txn_apply_locked (fn=..., job=...) > #8 job_do_finalize_locked (job=...) > #9 job_exit (opaque=...) > #10 aio_bh_poll (ctx=...) > #11 aio_poll (ctx=..., blocking=...) > #12 bdrv_poll_co (s=...) > #13 bdrv_flush (bs=...) > #14 bdrv_flush_all () > #15 do_vm_stop (state=..., send_stop=...) > #16 vm_shutdown () Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Message-ID: <20240322095009.346989-3-f.ebner@proxmox.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
rmalmain
pushed a commit
that referenced
this pull request
Jun 18, 2024
When vhost-user or vhost-kernel is handling virtio net datapath, QEMU should not touch used ring. But with vhost-user socket reconnect scenario, in a very rare case (has pending kick event). VRING_USED_F_NO_NOTIFY is set by QEMU in following code path: #0 virtio_queue_split_set_notification (vq=0x7ff5f4c920a8, enable=0) at ../hw/virtio/virtio.c:511 #1 0x0000559d6dbf033b in virtio_queue_set_notification (vq=0x7ff5f4c920a8, enable=0) at ../hw/virtio/virtio.c:576 #2 0x0000559d6dbbbdbc in virtio_net_handle_tx_bh (vdev=0x559d703a6aa0, vq=0x7ff5f4c920a8) at ../hw/net/virtio-net.c:2801 #3 0x0000559d6dbf4791 in virtio_queue_notify_vq (vq=0x7ff5f4c920a8) at ../hw/virtio/virtio.c:2248 #4 0x0000559d6dbf79da in virtio_queue_host_notifier_read (n=0x7ff5f4c9211c) at ../hw/virtio/virtio.c:3525 #5 0x0000559d6d9a5814 in virtio_bus_cleanup_host_notifier (bus=0x559d703a6a20, n=1) at ../hw/virtio/virtio-bus.c:321 #6 0x0000559d6dbf83c9 in virtio_device_stop_ioeventfd_impl (vdev=0x559d703a6aa0) at ../hw/virtio/virtio.c:3774 #7 0x0000559d6d9a55c8 in virtio_bus_stop_ioeventfd (bus=0x559d703a6a20) at ../hw/virtio/virtio-bus.c:259 #8 0x0000559d6d9a53e8 in virtio_bus_grab_ioeventfd (bus=0x559d703a6a20) at ../hw/virtio/virtio-bus.c:199 #9 0x0000559d6dbf841c in virtio_device_grab_ioeventfd (vdev=0x559d703a6aa0) at ../hw/virtio/virtio.c:3783 #10 0x0000559d6d9bde18 in vhost_dev_enable_notifiers (hdev=0x559d707edd70, vdev=0x559d703a6aa0) at ../hw/virtio/vhost.c:1592 #11 0x0000559d6d89a0b8 in vhost_net_start_one (net=0x559d707edd70, dev=0x559d703a6aa0) at ../hw/net/vhost_net.c:266 #12 0x0000559d6d89a6df in vhost_net_start (dev=0x559d703a6aa0, ncs=0x559d7048d890, data_queue_pairs=31, cvq=0) at ../hw/net/vhost_net.c:412 #13 0x0000559d6dbb5b89 in virtio_net_vhost_status (n=0x559d703a6aa0, status=15 '\017') at ../hw/net/virtio-net.c:311 #14 0x0000559d6dbb5e34 in virtio_net_set_status (vdev=0x559d703a6aa0, status=15 '\017') at ../hw/net/virtio-net.c:392 #15 0x0000559d6dbb60d8 in virtio_net_set_link_status (nc=0x559d7048d890) at ../hw/net/virtio-net.c:455 #16 0x0000559d6da64863 in qmp_set_link (name=0x559d6f0b83d0 "hostnet1", up=true, errp=0x7ffdd76569f0) at ../net/net.c:1459 #17 0x0000559d6da7226e in net_vhost_user_event (opaque=0x559d6f0b83d0, event=CHR_EVENT_OPENED) at ../net/vhost-user.c:301 #18 0x0000559d6ddc7f63 in chr_be_event (s=0x559d6f2ffea0, event=CHR_EVENT_OPENED) at ../chardev/char.c:62 #19 0x0000559d6ddc7fdc in qemu_chr_be_event (s=0x559d6f2ffea0, event=CHR_EVENT_OPENED) at ../chardev/char.c:82 This issue causes guest kernel stop kicking device and traffic stop. Add vhost_started check in virtio_net_handle_tx_bh to fix this wrong VRING_USED_F_NO_NOTIFY set. Signed-off-by: Yajun Wu <yajunw@nvidia.com> Reviewed-by: Jiri Pirko <jiri@nvidia.com> Acked-by: Michael S. Tsirkin <mst@redhat.com> Message-ID: <20240402045109.97729-1-yajunw@nvidia.com> [PMD: Use unlikely()] Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
rmalmain
pushed a commit
that referenced
this pull request
Jun 18, 2024
This patch improves error handling in virtio_snd_handle_tx_xfer() and virtio_snd_handle_rx_xfer() in the VirtIO sound driver. Previously, 'goto' statements were used for error paths, leading to unnecessary processing and potential null pointer dereferences. Now, 'continue' is used to skip the rest of the current loop iteration for errors such as message size discrepancies or null streams, reducing crash risks. ASAN log illustrating the issue addressed: ERROR: AddressSanitizer: SEGV on unknown address 0x0000000000b4 #0 0x57cea39967b8 in qemu_mutex_lock_impl qemu/util/qemu-thread-posix.c:92:5 #1 0x57cea128c462 in qemu_mutex_lock qemu/include/qemu/thread.h:122:5 #2 0x57cea128d72f in qemu_lockable_lock qemu/include/qemu/lockable.h:95:5 #3 0x57cea128c294 in qemu_lockable_auto_lock qemu/include/qemu/lockable.h:105:5 #4 0x57cea1285eb2 in virtio_snd_handle_rx_xfer qemu/hw/audio/virtio-snd.c:1026:9 #5 0x57cea2caebbc in virtio_queue_notify_vq qemu/hw/virtio/virtio.c:2268:9 #6 0x57cea2cae412 in virtio_queue_host_notifier_read qemu/hw/virtio/virtio.c:3671:9 #7 0x57cea39822f1 in aio_dispatch_handler qemu/util/aio-posix.c:372:9 #8 0x57cea3979385 in aio_dispatch_handlers qemu/util/aio-posix.c:414:20 #9 0x57cea3978eb1 in aio_dispatch qemu/util/aio-posix.c:424:5 #10 0x57cea3a1eede in aio_ctx_dispatch qemu/util/async.c:360:5 Signed-off-by: Zheyu Ma <zheyuma97@gmail.com> Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org> Message-Id: <20240322110827.568412-1-zheyuma97@gmail.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
rmalmain
pushed a commit
that referenced
this pull request
Jun 18, 2024
Replace qemu_bh_new_guarded() by virtio_bh_new_guarded() so the bus and device use the same guard. Otherwise the DMA-reentrancy protection can be bypassed: $ cat << EOF | qemu-system-i386 -display none -nodefaults \ -machine q35,accel=qtest \ -m 512M \ -device virtio-gpu \ -qtest stdio outl 0xcf8 0x80000820 outl 0xcfc 0xe0004000 outl 0xcf8 0x80000804 outw 0xcfc 0x06 write 0xe0004030 0x4 0x024000e0 write 0xe0004028 0x1 0xff write 0xe0004020 0x4 0x00009300 write 0xe000401c 0x1 0x01 write 0x101 0x1 0x04 write 0x103 0x1 0x1c write 0x9301c8 0x1 0x18 write 0x105 0x1 0x1c write 0x107 0x1 0x1c write 0x109 0x1 0x1c write 0x10b 0x1 0x00 write 0x10d 0x1 0x00 write 0x10f 0x1 0x00 write 0x111 0x1 0x00 write 0x113 0x1 0x00 write 0x115 0x1 0x00 write 0x117 0x1 0x00 write 0x119 0x1 0x00 write 0x11b 0x1 0x00 write 0x11d 0x1 0x00 write 0x11f 0x1 0x00 write 0x121 0x1 0x00 write 0x123 0x1 0x00 write 0x125 0x1 0x00 write 0x127 0x1 0x00 write 0x129 0x1 0x00 write 0x12b 0x1 0x00 write 0x12d 0x1 0x00 write 0x12f 0x1 0x00 write 0x131 0x1 0x00 write 0x133 0x1 0x00 write 0x135 0x1 0x00 write 0x137 0x1 0x00 write 0x139 0x1 0x00 write 0xe0007003 0x1 0x00 EOF ... ================================================================= ==276099==ERROR: AddressSanitizer: heap-use-after-free on address 0x60d000011178 at pc 0x562cc3b736c7 bp 0x7ffed49dee60 sp 0x7ffed49dee58 READ of size 8 at 0x60d000011178 thread T0 #0 0x562cc3b736c6 in virtio_gpu_ctrl_response hw/display/virtio-gpu.c:180:42 #1 0x562cc3b7c40b in virtio_gpu_ctrl_response_nodata hw/display/virtio-gpu.c:192:5 #2 0x562cc3b7c40b in virtio_gpu_simple_process_cmd hw/display/virtio-gpu.c:1015:13 #3 0x562cc3b82873 in virtio_gpu_process_cmdq hw/display/virtio-gpu.c:1050:9 #4 0x562cc4a85514 in aio_bh_call util/async.c:169:5 #5 0x562cc4a85c52 in aio_bh_poll util/async.c:216:13 #6 0x562cc4a1a79b in aio_dispatch util/aio-posix.c:423:5 #7 0x562cc4a8a2da in aio_ctx_dispatch util/async.c:358:5 #8 0x7f36840547a8 in g_main_context_dispatch (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x547a8) #9 0x562cc4a8b753 in glib_pollfds_poll util/main-loop.c:290:9 #10 0x562cc4a8b753 in os_host_main_loop_wait util/main-loop.c:313:5 #11 0x562cc4a8b753 in main_loop_wait util/main-loop.c:592:11 #12 0x562cc3938186 in qemu_main_loop system/runstate.c:782:9 #13 0x562cc43b7af5 in qemu_default_main system/main.c:37:14 #14 0x7f3683a6c189 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 #15 0x7f3683a6c244 in __libc_start_main csu/../csu/libc-start.c:381:3 #16 0x562cc2a58ac0 in _start (qemu-system-i386+0x231bac0) 0x60d000011178 is located 56 bytes inside of 136-byte region [0x60d000011140,0x60d0000111c8) freed by thread T0 here: #0 0x562cc2adb662 in __interceptor_free (qemu-system-i386+0x239e662) #1 0x562cc3b86b21 in virtio_gpu_reset hw/display/virtio-gpu.c:1524:9 #2 0x562cc416e20e in virtio_reset hw/virtio/virtio.c:2145:9 #3 0x562cc37c5644 in virtio_pci_reset hw/virtio/virtio-pci.c:2249:5 #4 0x562cc4233758 in memory_region_write_accessor system/memory.c:497:5 #5 0x562cc4232eea in access_with_adjusted_size system/memory.c:573:18 previously allocated by thread T0 here: #0 0x562cc2adb90e in malloc (qemu-system-i386+0x239e90e) #1 0x7f368405a678 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x5a678) #2 0x562cc4163ffc in virtqueue_split_pop hw/virtio/virtio.c:1612:12 #3 0x562cc4163ffc in virtqueue_pop hw/virtio/virtio.c:1783:16 #4 0x562cc3b91a95 in virtio_gpu_handle_ctrl hw/display/virtio-gpu.c:1112:15 #5 0x562cc4a85514 in aio_bh_call util/async.c:169:5 #6 0x562cc4a85c52 in aio_bh_poll util/async.c:216:13 #7 0x562cc4a1a79b in aio_dispatch util/aio-posix.c:423:5 SUMMARY: AddressSanitizer: heap-use-after-free hw/display/virtio-gpu.c:180:42 in virtio_gpu_ctrl_response With this change, the same reproducer triggers: qemu-system-i386: warning: Blocked re-entrant IO on MemoryRegion: virtio-pci-common-virtio-gpu at addr: 0x6 Fixes: CVE-2024-3446 Cc: qemu-stable@nongnu.org Reported-by: Alexander Bulekov <alxndr@bu.edu> Reported-by: Yongkang Jia <kangel@zju.edu.cn> Reported-by: Xiao Lei <nop.leixiao@gmail.com> Reported-by: Yiming Tao <taoym@zju.edu.cn> Buglink: https://bugs.launchpad.net/qemu/+bug/1888606 Reviewed-by: Gerd Hoffmann <kraxel@redhat.com> Acked-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Message-Id: <20240409105537.18308-3-philmd@linaro.org>
rmalmain
pushed a commit
that referenced
this pull request
Jun 18, 2024
nand_command() and nand_getio() don't check @offset points into the block, nor the available data length (s->iolen) is not negative. In order to fix: - check the offset is in range in nand_blk_load_NAND_PAGE_SIZE(), - do not set @iolen if blk_load() failed. Reproducer: $ cat << EOF | qemu-system-arm -machine tosa \ -monitor none -serial none \ -display none -qtest stdio write 0x10000111 0x1 0xca write 0x10000104 0x1 0x47 write 0x1000ca04 0x1 0xd7 write 0x1000ca01 0x1 0xe0 write 0x1000ca04 0x1 0x71 write 0x1000ca00 0x1 0x50 write 0x1000ca04 0x1 0xd7 read 0x1000ca02 0x1 write 0x1000ca01 0x1 0x10 EOF ================================================================= ==15750==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61f000000de0 at pc 0x560e61557210 bp 0x7ffcfc4a59f0 sp 0x7ffcfc4a59e8 READ of size 1 at 0x61f000000de0 thread T0 #0 0x560e6155720f in mem_and hw/block/nand.c:101:20 #1 0x560e6155ac9c in nand_blk_write_512 hw/block/nand.c:663:9 #2 0x560e61544200 in nand_command hw/block/nand.c:293:13 #3 0x560e6153cc83 in nand_setio hw/block/nand.c:520:13 #4 0x560e61a0a69e in tc6393xb_nand_writeb hw/display/tc6393xb.c:380:13 #5 0x560e619f9bf7 in tc6393xb_writeb hw/display/tc6393xb.c:524:9 #6 0x560e647c7d03 in memory_region_write_accessor softmmu/memory.c:492:5 #7 0x560e647c7641 in access_with_adjusted_size softmmu/memory.c:554:18 #8 0x560e647c5f66 in memory_region_dispatch_write softmmu/memory.c:1514:16 #9 0x560e6485409e in flatview_write_continue softmmu/physmem.c:2825:23 #10 0x560e648421eb in flatview_write softmmu/physmem.c:2867:12 #11 0x560e64841ca8 in address_space_write softmmu/physmem.c:2963:18 #12 0x560e61170162 in qemu_writeb tests/qtest/videzzo/videzzo_qemu.c:1080:5 #13 0x560e6116eef7 in dispatch_mmio_write tests/qtest/videzzo/videzzo_qemu.c:1227:28 0x61f000000de0 is located 0 bytes to the right of 3424-byte region [0x61f000000080,0x61f000000de0) allocated by thread T0 here: #0 0x560e611276cf in malloc /root/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3 #1 0x7f7959a87e98 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x57e98) #2 0x560e64b98871 in object_new qom/object.c:749:12 #3 0x560e64b5d1a1 in qdev_new hw/core/qdev.c:153:19 #4 0x560e61547ea5 in nand_init hw/block/nand.c:639:11 #5 0x560e619f8772 in tc6393xb_init hw/display/tc6393xb.c:558:16 #6 0x560e6390bad2 in tosa_init hw/arm/tosa.c:250:12 SUMMARY: AddressSanitizer: heap-buffer-overflow hw/block/nand.c:101:20 in mem_and ==15750==ABORTING Broken since introduction in commit 3e3d581 ("NAND Flash memory emulation and ECC calculation helpers for use by NAND controllers"). Cc: qemu-stable@nongnu.org Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1445 Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1446 Reported-by: Qiang Liu <cyruscyliu@gmail.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Message-Id: <20240409135944.24997-4-philmd@linaro.org>
rmalmain
pushed a commit
that referenced
this pull request
Jun 18, 2024
When the MAC Interface Layer (MIL) transmit FIFO is full, truncate the packet, and raise the Transmitter Error (TXE) flag. Broken since model introduction in commit 2a42499 ("LAN9118 emulation"). When using the reproducer from https://gitlab.com/qemu-project/qemu/-/issues/2267 we get: hw/net/lan9118.c:798:17: runtime error: index 2048 out of bounds for type 'uint8_t[2048]' (aka 'unsigned char[2048]') #0 0x563ec9a057b1 in tx_fifo_push hw/net/lan9118.c:798:43 #1 0x563ec99fbb28 in lan9118_writel hw/net/lan9118.c:1042:9 #2 0x563ec99f2de2 in lan9118_16bit_mode_write hw/net/lan9118.c:1205:9 #3 0x563ecbf78013 in memory_region_write_accessor system/memory.c:497:5 #4 0x563ecbf776f5 in access_with_adjusted_size system/memory.c:573:18 #5 0x563ecbf75643 in memory_region_dispatch_write system/memory.c:1521:16 #6 0x563ecc01bade in flatview_write_continue_step system/physmem.c:2713:18 #7 0x563ecc01b374 in flatview_write_continue system/physmem.c:2743:19 #8 0x563ecbff1c9b in flatview_write system/physmem.c:2774:12 #9 0x563ecbff1768 in address_space_write system/physmem.c:2894:18 ... [*] LAN9118 DS00002266B.pdf, Table 5.3.3 "INTERRUPT STATUS REGISTER" Cc: qemu-stable@nongnu.org Reported-by: Will Lester Reported-by: Chuhong Yuan <hslester96@gmail.com> Suggested-by: Peter Maydell <peter.maydell@linaro.org> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2267 Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-Id: <20240409133801.23503-3-philmd@linaro.org>
rmalmain
pushed a commit
that referenced
this pull request
Jun 18, 2024
Per "SD Host Controller Standard Specification Version 3.00": * 2.2.5 Transfer Mode Register (Offset 00Ch) Writes to this register shall be ignored when the Command Inhibit (DAT) in the Present State register is 1. Do not update the TRNMOD register when Command Inhibit (DAT) bit is set to avoid the present-status register going out of sync, leading to malicious guest using DMA mode and overflowing the FIFO buffer: $ cat << EOF | qemu-system-i386 \ -display none -nographic -nodefaults \ -machine accel=qtest -m 512M \ -device sdhci-pci,sd-spec-version=3 \ -device sd-card,drive=mydrive \ -drive if=none,index=0,file=null-co://,format=raw,id=mydrive \ -qtest stdio outl 0xcf8 0x80001013 outl 0xcfc 0x91 outl 0xcf8 0x80001001 outl 0xcfc 0x06000000 write 0x9100002c 0x1 0x05 write 0x91000058 0x1 0x16 write 0x91000005 0x1 0x04 write 0x91000028 0x1 0x08 write 0x16 0x1 0x21 write 0x19 0x1 0x20 write 0x9100000c 0x1 0x01 write 0x9100000e 0x1 0x20 write 0x9100000f 0x1 0x00 write 0x9100000c 0x1 0x00 write 0x91000020 0x1 0x00 EOF Stack trace (part): ================================================================= ==89993==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x615000029900 at pc 0x55d5f885700d bp 0x7ffc1e1e9470 sp 0x7ffc1e1e9468 WRITE of size 1 at 0x615000029900 thread T0 #0 0x55d5f885700c in sdhci_write_dataport hw/sd/sdhci.c:564:39 #1 0x55d5f8849150 in sdhci_write hw/sd/sdhci.c:1223:13 #2 0x55d5fa01db63 in memory_region_write_accessor system/memory.c:497:5 #3 0x55d5fa01d245 in access_with_adjusted_size system/memory.c:573:18 #4 0x55d5fa01b1a9 in memory_region_dispatch_write system/memory.c:1521:16 #5 0x55d5fa09f5c9 in flatview_write_continue system/physmem.c:2711:23 #6 0x55d5fa08f78b in flatview_write system/physmem.c:2753:12 #7 0x55d5fa08f258 in address_space_write system/physmem.c:2860:18 ... 0x615000029900 is located 0 bytes to the right of 512-byte region [0x615000029700,0x615000029900) allocated by thread T0 here: #0 0x55d5f7237b27 in __interceptor_calloc #1 0x7f9e36dd4c50 in g_malloc0 #2 0x55d5f88672f7 in sdhci_pci_realize hw/sd/sdhci-pci.c:36:5 #3 0x55d5f844b582 in pci_qdev_realize hw/pci/pci.c:2092:9 #4 0x55d5fa2ee74b in device_set_realized hw/core/qdev.c:510:13 #5 0x55d5fa325bfb in property_set_bool qom/object.c:2358:5 #6 0x55d5fa31ea45 in object_property_set qom/object.c:1472:5 #7 0x55d5fa332509 in object_property_set_qobject om/qom-qobject.c:28:10 #8 0x55d5fa31f6ed in object_property_set_bool qom/object.c:1541:15 #9 0x55d5fa2e2948 in qdev_realize hw/core/qdev.c:292:12 #10 0x55d5f8eed3f1 in qdev_device_add_from_qdict system/qdev-monitor.c:719:10 #11 0x55d5f8eef7ff in qdev_device_add system/qdev-monitor.c:738:11 #12 0x55d5f8f211f0 in device_init_func system/vl.c:1200:11 #13 0x55d5fad0877d in qemu_opts_foreach util/qemu-option.c:1135:14 #14 0x55d5f8f0df9c in qemu_create_cli_devices system/vl.c:2638:5 #15 0x55d5f8f0db24 in qmp_x_exit_preconfig system/vl.c:2706:5 #16 0x55d5f8f14dc0 in qemu_init system/vl.c:3737:9 ... SUMMARY: AddressSanitizer: heap-buffer-overflow hw/sd/sdhci.c:564:39 in sdhci_write_dataport Add assertions to ensure the fifo_buffer[] is not overflowed by malicious accesses to the Buffer Data Port register. Fixes: CVE-2024-3447 Cc: qemu-stable@nongnu.org Fixes: d7dfca0 ("hw/sdhci: introduce standard SD host controller") Buglink: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=58813 Reported-by: Alexander Bulekov <alxndr@bu.edu> Reported-by: Chuhong Yuan <hslester96@gmail.com> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-Id: <CAFEAcA9iLiv1XGTGKeopgMa8Y9+8kvptvsb8z2OBeuy+5=NUfg@mail.gmail.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Message-Id: <20240409145524.27913-1-philmd@linaro.org>
rmalmain
pushed a commit
that referenced
this pull request
Jun 18, 2024
If a fragmented packet size is too short, do not try to calculate its checksum. Reproduced using: $ cat << EOF | qemu-system-i386 -display none -nodefaults \ -machine q35,accel=qtest -m 32M \ -device igb,netdev=net0 \ -netdev user,id=net0 \ -qtest stdio outl 0xcf8 0x80000810 outl 0xcfc 0xe0000000 outl 0xcf8 0x80000804 outw 0xcfc 0x06 write 0xe0000403 0x1 0x02 writel 0xe0003808 0xffffffff write 0xe000381a 0x1 0x5b write 0xe000381b 0x1 0x00 EOF Assertion failed: (offset == 0), function iov_from_buf_full, file util/iov.c, line 39. #1 0x5575e81e952a in iov_from_buf_full qemu/util/iov.c:39:5 #2 0x5575e6500768 in net_tx_pkt_update_sctp_checksum qemu/hw/net/net_tx_pkt.c:144:9 #3 0x5575e659f3e1 in igb_setup_tx_offloads qemu/hw/net/igb_core.c:478:11 #4 0x5575e659f3e1 in igb_tx_pkt_send qemu/hw/net/igb_core.c:552:10 #5 0x5575e659f3e1 in igb_process_tx_desc qemu/hw/net/igb_core.c:671:17 #6 0x5575e659f3e1 in igb_start_xmit qemu/hw/net/igb_core.c:903:9 #7 0x5575e659f3e1 in igb_set_tdt qemu/hw/net/igb_core.c:2812:5 #8 0x5575e657d6a4 in igb_core_write qemu/hw/net/igb_core.c:4248:9 Fixes: CVE-2024-3567 Cc: qemu-stable@nongnu.org Reported-by: Zheyu Ma <zheyuma97@gmail.com> Fixes: f199b13 ("igb: Implement Tx SCTP CSO") Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2273 Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com> Acked-by: Jason Wang <jasowang@redhat.com> Message-Id: <20240410070459.49112-1-philmd@linaro.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.