aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2015-07-27eepro100: Drop nic_can_receiveFam Zheng
nic_receive already checks the conditions and drop packets if false. Due to the new semantics since 6e99c63 ("net/socket: Drop net_socket_can_send"), having .can_receive returning 0 requires us to explicitly flush the queued packets when the conditions are becoming true, but queuing the packets when guest driver is not ready doesn't make much sense. Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Jason Wang <jasowang@redhat.com> Message-id: 1436955553-22791-4-git-send-email-famz@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-07-27pcnet: Drop pcnet_can_receiveFam Zheng
pcnet_receive already checks the conditions and drop packets if false. Due to the new semantics since 6e99c63 ("net/socket: Drop net_socket_can_send"), having .can_receive returning 0 requires us to explicitly flush the queued packets when the conditions are becoming true, but queuing the packets when guest driver is not ready doesn't make much sense. Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Jason Wang <jasowang@redhat.com> Message-id: 1436955553-22791-3-git-send-email-famz@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-07-27xgmac: Drop packets with eth_can_rx is false.Fam Zheng
Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Jason Wang <jasowang@redhat.com> Message-id: 1436955553-22791-2-git-send-email-famz@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-07-27hw/net: fix mcf_fec driver receiverGreg Ungerer
The network mcf_fec driver emulated receive side method is returning a result of 0 causing the network layer to disable receive for this emulated device. This results in the guest only ever receiving one packet. Fix the recieve side processing to return the number of bytes that we passed back through to the guest. Signed-off-by: Greg Ungerer <gerg@uclinux.org> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 1435296436-12152-5-git-send-email-gerg@uclinux.org Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-07-27hw/net: add simple phy support to mcf_fec driverGreg Ungerer
The Linux fec driver needs at least basic phy support to probe and work. The current qemu mcf_fec emulation has no support for the reading or writing of the MDIO lines to access an attached phy. This code adds a very simple set of register results for a fixed phy setup - very similar to that used on an m5208evb board. This is enough to probe and identify an emulated attached phy. Signed-off-by: Greg Ungerer <gerg@uclinux.org> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 1435296436-12152-4-git-send-email-gerg@uclinux.org Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-07-27hw/net: add ANLPAR bit definitions to generic miiGreg Ungerer
Add a base set of bit definitions for the standard MII phy "Auto-Negotiation Link Partner Ability Register" (ANLPAR). The original definitions moved into mii.h from the allwinner_emac driver did not define these. Signed-off-by: Greg Ungerer <gerg@uclinux.org> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 1435296436-12152-3-git-send-email-gerg@uclinux.org Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-07-27hw/net: create common collection of MII definitionsGreg Ungerer
Create a common set of definitions of address and register values for ethernet MII phys. A few of the current ethernet drivers have at least a partial set of these definitions. Others just use hard coded raw constant numbers. This initial set is copied directly from the allwinner_emac code. Signed-off-by: Greg Ungerer <gerg@uclinux.org> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 1435296436-12152-2-git-send-email-gerg@uclinux.org Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-07-24Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into stagingPeter Maydell
* qemu-char fixes * SCSI fixes (including CVE-2015-5158) * RCU fixes * Framebuffer logic to set DIRTY_MEMORY_VGA * Fix compiler warning for --disable-vnc * qemu-doc fixes * x86 TCG pasto fix # gpg: Signature made Fri Jul 24 12:57:52 2015 BST using RSA key ID 78C7AE83 # gpg: Good signature from "Paolo Bonzini <bonzini@gnu.org>" # gpg: aka "Paolo Bonzini <pbonzini@redhat.com>" # gpg: WARNING: This key is not certified with sufficiently trusted signatures! # gpg: It is not certain that the signature belongs to the owner. # 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 * remotes/bonzini/tags/for-upstream: target-i386/FPU: a misprint in helper_fistll_ST0 qemu-doc: fix typos framebuffer: set DIRTY_MEMORY_VGA on RAM that is used for the framebuffer memory: count number of active VGA logging clients vl: Fix compiler warning for builds without VNC scsi: Handle no media case for scsi_get_configuration rcu: actually register threads that have RCU read-side critical sections scsi: fix buffer overflow in scsi_req_parse_cdb (CVE-2015-5158) vnc: fix memory leak qemu-char: Fix missed data on unix socket qemu-char: handle EINTR for TCP character devices exec.c: Use atomic_rcu_read() to access dispatch in memory_region_section_get_iotlb() Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2015-07-24target-i386/FPU: a misprint in helper_fistll_ST0Dmitry Poletaev
There is a cut-and-paste mistake in the patch https://lists.gnu.org/archive/html/qemu-devel/2014-11/msg01657.html . It cause errors in guest work. Here is the bugfix. Signed-off-by: Dmitry Poletaev <poletaev-qemu@yandex.ru> Reported-by: Kirill Batuzov <batuzovk@ispras.ru> Message-Id: <2692911436348920@web2m.yandex.ru> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2015-07-24qemu-doc: fix typosGonglei
Signed-off-by: Gonglei <arei.gonglei@huawei.com> Message-Id: <1435917057-9396-1-git-send-email-arei.gonglei@huawei.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2015-07-24framebuffer: set DIRTY_MEMORY_VGA on RAM that is used for the framebufferPaolo Bonzini
The MemoryRegionSection contains enough information to access the RAM region underlying the framebuffer, and can be cached inside the display device. By doing this, the new framebuffer_update_memory_section function can enable dirty memory logging on the relevant RAM region. The function must be called whenever the stride or base of the framebuffer changes; a simple way to cover these cases is to call it on every full frame invalidation, which is a rare case. framebuffer_update_display now works entirely on a MemoryRegionSection, without going through cpu_physical_memory_map/unmap. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2015-07-24memory: count number of active VGA logging clientsPaolo Bonzini
For a board that has multiple framebuffer devices, both of them might want to use DIRTY_MEMORY_VGA on the same memory region. The lack of reference counting in memory_region_set_log makes this very awkward to implement. Suggested-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2015-07-24vl: Fix compiler warning for builds without VNCStefan Weil
This regression was caused by commit 70b94331. CC vl.o vl.c: In function ‘select_display’: vl.c:2064:12: error: unused variable ‘err’ [-Werror=unused-variable] Error *err = NULL; ^ Reported-by: Claudio Fontana <claudio.fontana@huawei.com> Signed-off-by: Stefan Weil <sw@weilnetz.de> Message-Id: <1437587610-26433-1-git-send-email-sw@weilnetz.de> Reviewed-by: Wen Congyang <wency@cn.fujitsu.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2015-07-24scsi: Handle no media case for scsi_get_configurationMatthew Rosato
Currently, scsi_get_configuration always returns a current profile (DVD or CD), even when there is actually no media present. By comparison, ide/atapi uses a default profile of 0 (MMC_PROFILE_NONE) for this case and checks for tray_open, so let's do the same for scsi. This fixes a problem I'm seeing with Fedora 22 guests where systemd cdrom_id fails to unmount after a QEMU-initiated eject against a scsi cdrom device because it believes the media is still present (but unreadable). Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com> Message-Id: <1436986352-10695-1-git-send-email-mjrosato@linux.vnet.ibm.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2015-07-24rcu: actually register threads that have RCU read-side critical sectionsPaolo Bonzini
Otherwise, grace periods are detected too early! Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2015-07-24scsi: fix buffer overflow in scsi_req_parse_cdb (CVE-2015-5158)Paolo Bonzini
This is a guest-triggerable buffer overflow present in QEMU 2.2.0 and newer. scsi_cdb_length returns -1 as an error value, but the caller does not check it. Luckily, the massive overflow means that QEMU will just SIGSEGV, making the impact much smaller. Reported-by: Zhu Donghai (朱东海) <donghai.zdh@alibaba-inc.com> Fixes: 1894df02811f6b79ea3ffbf1084599d96f316173 Reviewed-by: Fam Zheng <famz@redhat.com> Cc: qemu-stable@nongnu.org Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2015-07-24vnc: fix memory leakGonglei
If vnc's password is configured, it will leak memory which cipher variable pointed on every vnc connection. Cc: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Signed-off-by: Gonglei <arei.gonglei@huawei.com> Message-Id: <1437556133-11268-1-git-send-email-arei.gonglei@huawei.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2015-07-24Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20150723' into stagingPeter Maydell
Last minute fixes for 2.4. # gpg: Signature made Fri Jul 24 04:42:31 2015 BST using RSA key ID 4DD0279B # gpg: Good signature from "Richard Henderson <rth7680@gmail.com>" # gpg: aka "Richard Henderson <rth@redhat.com>" # gpg: aka "Richard Henderson <rth@twiddle.net>" * remotes/rth/tags/pull-tcg-20150723: tcg/optimize: fix tcg_opt_gen_movi tcg/aarch64: use 32-bit offset for 32-bit softmmu emulation tcg/aarch64: use 32-bit offset for 32-bit user-mode emulation tcg/aarch64: add ext argument to tcg_out_insn_3310 tcg/i386: Extend addresses for 32-bit guests Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2015-07-24Merge remote-tracking branch 'remotes/awilliam/tags/vfio-fixes-20150723.0' ↵Peter Maydell
into staging VFIO fixes for v2.4.0-rc3 - Fix Realtek NIC quirk (Alex Williamson) - Restore bootindex functionality (Alex Williamson) # gpg: Signature made Thu Jul 23 19:51:23 2015 BST using RSA key ID 3BB08B22 # gpg: Good signature from "Alex Williamson <alex.williamson@redhat.com>" # gpg: aka "Alex Williamson <alex@shazbot.org>" # gpg: aka "Alex Williamson <alwillia@redhat.com>" # gpg: aka "Alex Williamson <alex.l.williamson@gmail.com>" * remotes/awilliam/tags/vfio-fixes-20150723.0: vfio/pci: Fix bootindex vfio/pci: Fix RTL8168 NIC quirks Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2015-07-23tcg/optimize: fix tcg_opt_gen_moviAurelien Jarno
Due to a copy&paste, the new op value is tested against mov_i32 instead of movi_i32. The test is therefore always false. Fix that. Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> Message-Id: <1436544211-2769-1-git-send-email-aurelien@aurel32.net> Signed-off-by: Richard Henderson <rth@twiddle.net>
2015-07-23tcg/aarch64: use 32-bit offset for 32-bit softmmu emulationRichard Henderson
Similar to the same fix for user-mode, except this instance occurs on the softmmu path. Again, the tlb addend must be the base register, while the guest address is the index. Reviewed-by: Aurelien Jarno <aurelien@aurel32.net> Signed-off-by: Richard Henderson <rth@twiddle.net>
2015-07-23tcg/aarch64: use 32-bit offset for 32-bit user-mode emulationPaolo Bonzini
Thanks to the previous patch, it is now easy for tcg_out_qemu_ld and tcg_out_qemu_st to use a 32-bit zero extended offset. However, the guest base register x28 must be the base and addr_reg must be the index. Reported-by: Leon Alrae <leon.alrae@imgtec.com> Reviewed-by: Aurelien Jarno <aurelien@aurel32.net> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <1436974021-28978-3-git-send-email-pbonzini@redhat.com> Signed-off-by: Richard Henderson <rth@twiddle.net>
2015-07-23tcg/aarch64: add ext argument to tcg_out_insn_3310Paolo Bonzini
The new argument lets you pick uxtw or uxtx mode for the offset register. For now, all callers pass TCG_TYPE_I64 so that uxtx is generated. The bits for uxtx are removed from I3312_TO_I3310. Reported-by: Leon Alrae <leon.alrae@imgtec.com> Reviewed-by: Aurelien Jarno <aurelien@aurel32.net> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <1436974021-28978-2-git-send-email-pbonzini@redhat.com> Signed-off-by: Richard Henderson <rth@twiddle.net>
2015-07-23tcg/i386: Extend addresses for 32-bit guestsRichard Henderson
Removing the ??? comment explaining why it (mostly) worked. Reviewed-by: Aurelien Jarno <aurelien@aurel32.net> Signed-off-by: Richard Henderson <rth@twiddle.net> Message-Id: <1437081950-7206-2-git-send-email-rth@twiddle.net>
2015-07-23Merge remote-tracking branch 'remotes/ehabkost/tags/numa-pull-request' into ↵Peter Maydell
staging NUMA queue, 2015-07-22 # gpg: Signature made Wed Jul 22 19:11:04 2015 BST using RSA key ID 984DC5A6 # gpg: Good signature from "Eduardo Habkost <ehabkost@redhat.com>" # gpg: WARNING: This key is not certified with sufficiently trusted signatures! # gpg: It is not certain that the signature belongs to the owner. # Primary key fingerprint: 5A32 2FD5 ABC4 D3DB ACCF D1AA 2807 936F 984D C5A6 * remotes/ehabkost/tags/numa-pull-request: hostmem: Fix qemu_opt_get_bool() crash in host_memory_backend_init() Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2015-07-23qemu-char: Fix missed data on unix socketNils Carlson
Commit 812c1057 introduced HUP detection on unix and tcp sockets prior to a read in tcp_chr_read. This unfortunately broke CloudStack 4.2 which relied on the old behaviour where data on a socket was readable even if a HUP was present. A working solution is to properly check the return values from recv, handling a closed socket once there is no more data to read. Also enable polling for G_IO_NVAL to ensure the callback is called for all possible events as these should now be possible to handle with the improved error detection. Signed-off-by: Nils Carlson <pyssling@ludd.ltu.se> Message-Id: <1437338396-22336-1-git-send-email-pyssling@ludd.ltu.se> [Do not handle EINTR; use socket_error(). - Paolo] Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2015-07-23qemu-char: handle EINTR for TCP character devicesPaolo Bonzini
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2015-07-23exec.c: Use atomic_rcu_read() to access dispatch in ↵Peter Maydell
memory_region_section_get_iotlb() When accessing the dispatch pointer in an AddressSpace within an RCU critical section we should always use atomic_rcu_read(). Fix an access within memory_region_section_get_iotlb() which was incorrectly doing a direct pointer access. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-Id: <1437391637-31576-1-git-send-email-peter.maydell@linaro.org> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2015-07-22vfio/pci: Fix bootindexAlex Williamson
bootindex was incorrectly changed to a device Property during the platform code split, resulting in it no longer working. Remove it. Signed-off-by: Alex Williamson <alex.williamson@redhat.com> Cc: qemu-stable@nongnu.org # v2.3+
2015-07-22vfio/pci: Fix RTL8168 NIC quirksAlex Williamson
The RTL8168 quirk correctly describes using bit 31 as a signal to mark a latch/completion, but the code mistakenly uses bit 28. This causes the Realtek driver to spin on this register for quite a while, 20k cycles on Windows 7 v7.092 driver. Then it gets frustrated and tries to set the bit itself and spins for another 20k cycles. For some this still results in a working driver, for others not. About the only thing the code really does in its current form is protect the guest from sneaking in writes to the real hardware MSI-X table. The fix is obviously to use bit 31 as we document that we should. The other problem doesn't seem to affect current drivers as nobody seems to use these window registers for writes to the MSI-X table, but we need to use the stored data when a write is triggered, not the value of the current write, which only provides the offset. Note that only the Windows drivers from Realtek seem to use these registers, the Microsoft drivers provided with Windows 8.1 do not access them, nor do Linux in-kernel drivers. Link: https://bugs.launchpad.net/qemu/+bug/1384892 Signed-off-by: Alex Williamson <alex.williamson@redhat.com> Cc: qemu-stable@nongnu.org # v2.1+
2015-07-22hostmem: Fix qemu_opt_get_bool() crash in host_memory_backend_init()Eduardo Habkost
This fixes the following crash, introduced by commit 49d2e648e8087d154d8bf8b91f27c8e05e79d5a6: $ gdb --args qemu-system-x86_64 -machine pc,mem-merge=off -object memory-backend-ram,id=ram-node0,size=1024 [...] Program received signal SIGABRT, Aborted. (gdb) bt #0 0x00007ffff253b8c7 in raise () at /lib64/libc.so.6 #1 0x00007ffff253d52a in abort () at /lib64/libc.so.6 #2 0x00007ffff253446d in __assert_fail_base () at /lib64/libc.so.6 #3 0x00007ffff2534522 in () at /lib64/libc.so.6 #4 0x00005555558bb80a in qemu_opt_get_bool_helper (opts=0x55555621b650, name=name@entry=0x5555558ec922 "mem-merge", defval=defval@entry=true, del=del@entry=false) at qemu/util/qemu-option.c:388 #5 0x00005555558bbb5a in qemu_opt_get_bool (opts=<optimized out>, name=name@entry=0x5555558ec922 "mem-merge", defval=defval@entry=true) at qemu/util/qemu-option.c:398 #6 0x0000555555720a24 in host_memory_backend_init (obj=0x5555562ac970) at qemu/backends/hostmem.c:226 Instead of using qemu_opt_get_bool(), that didn't work with qemu_machine_opts for a long time, we can use the corresponding MachineState fields. Reviewed-by: Marcel Apfelbaum <marcel@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
2015-07-22Update version for v2.4.0-rc2 releasev2.4.0-rc2Peter Maydell
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2015-07-22Merge remote-tracking branch 'remotes/elmarco/tags/for-upstream' into stagingPeter Maydell
qxl: build fix for 2.4 # gpg: Signature made Wed Jul 22 15:55:00 2015 BST using DSA key ID F43F0992 # gpg: Good signature from "Marc-André Lureau <marcandre.lureau@redhat.com>" # gpg: aka "Marc-Andre Lureau <marcandre.lureau@gmail.com>" # gpg: aka "Marc-Andre Lureau <marc-andre.lureau@nokia.com>" # gpg: aka "Marc-André Lureau <marc-andre.lureau@nokia.com>" # gpg: aka "Marc-André Lureau (elmarco) <marcandre.lureau@gmail.com>" # 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: 7346 2483 9404 4E20 ABFF 7D48 D864 9487 F43F 0992 * remotes/elmarco/tags/for-upstream: qxl: Fix new function name for spice-server library Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2015-07-22qxl: Fix new function name for spice-server libraryFrediano Ziglio
The new spice-server function to limit the number of monitors (0.12.6) changed while development from spice_qxl_set_monitors_config_limit to spice_qxl_max_monitors (accepted upstream). By mistake I post patch with former name. This patch fix the function name. Signed-off-by: Frediano Ziglio <fziglio@redhat.com> Acked-by: Christophe Fergeau <cfergeau@redhat.com> Acked-by: Martin Kletzander <mkletzan@redhat.com> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
2015-07-22Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into ↵Peter Maydell
staging # gpg: Signature made Wed Jul 22 12:43:35 2015 BST using RSA key ID 81AB73C8 # gpg: Good signature from "Stefan Hajnoczi <stefanha@redhat.com>" # gpg: aka "Stefan Hajnoczi <stefanha@gmail.com>" * remotes/stefanha/tags/block-pull-request: AioContext: optimize clearing the EventNotifier AioContext: fix broken placement of event_notifier_test_and_clear AioContext: fix broken ctx->dispatching optimization aio-win32: reorganize polling loop tests: remove irrelevant assertions from test-aio qemu-timer: initialize "timers_done_ev" to set mirror: Speed up bitmap initial scanning Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2015-07-22AioContext: optimize clearing the EventNotifierPaolo Bonzini
It is pretty rare for aio_notify to actually set the EventNotifier. It can happen with worker threads such as thread-pool.c's, but otherwise it should never be set thanks to the ctx->notify_me optimization. The previous patch, unfortunately, added an unconditional call to event_notifier_test_and_clear; now add a userspace fast path that avoids the call. Note that it is not possible to do the same with event_notifier_set; it would break, as proved (again) by the included formal model. This patch survived over 3000 reboots on aarch64 KVM. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Tested-by: Richard W.M. Jones <rjones@redhat.com> Message-id: 1437487673-23740-7-git-send-email-pbonzini@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-07-22AioContext: fix broken placement of event_notifier_test_and_clearPaolo Bonzini
event_notifier_test_and_clear must be called before processing events. Otherwise, an aio_poll could "eat" the notification before the main I/O thread invokes ppoll(). The main I/O thread then never wakes up. This is an example of what could happen: i/o thread vcpu thread worker thread --------------------------------------------------------------------- lock_iothread notify_me = 1 ... unlock_iothread bh->scheduled = 1 event_notifier_set lock_iothread notify_me = 3 ppoll notify_me = 1 aio_dispatch aio_bh_poll thread_pool_completion_bh bh->scheduled = 1 event_notifier_set node->io_read(node->opaque) event_notifier_test_and_clear ppoll *** hang *** "Tracing" with qemu_clock_get_ns shows pretty much the same behavior as in the previous bug, so there are no new tricks here---just stare more at the code until it is apparent. One could also use a formal model, of course. The included one shows this with three processes: notifier corresponds to a QEMU thread pool worker, temporary_waiter to a VCPU thread that invokes aio_poll(), waiter to the main I/O thread. I would be happy to say that the formal model found the bug for me, but actually I wrote it after the fact. This patch is a bit of a big hammer. The next one optimizes it, with help (this time for real rather than a posteriori :)) from another, similar formal model. Reported-by: Richard W. M. Jones <rjones@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Tested-by: Richard W.M. Jones <rjones@redhat.com> Message-id: 1437487673-23740-6-git-send-email-pbonzini@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-07-22AioContext: fix broken ctx->dispatching optimizationPaolo Bonzini
This patch rewrites the ctx->dispatching optimization, which was the cause of some mysterious hangs that could be reproduced on aarch64 KVM only. The hangs were indirectly caused by aio_poll() and in particular by flash memory updates's call to blk_write(), which invokes aio_poll(). Fun stuff: they had an extremely short race window, so much that adding all kind of tracing to either the kernel or QEMU made it go away (a single printf made it half as reproducible). On the plus side, the failure mode (a hang until the next keypress) made it very easy to examine the state of the process with a debugger. And there was a very nice reproducer from Laszlo, which failed pretty often (more than half of the time) on any version of QEMU with a non-debug kernel; it also failed fast, while still in the firmware. So, it could have been worse. For some unknown reason they happened only with virtio-scsi, but that's not important. It's more interesting that they disappeared with io=native, making thread-pool.c a likely suspect for where the bug arose. thread-pool.c is also one of the few places which use bottom halves across threads, by the way. I hope that no other similar bugs exist, but just in case :) I am going to describe how the successful debugging went... Since the likely culprit was the ctx->dispatching optimization, which mostly affects bottom halves, the first observation was that there are two qemu_bh_schedule() invocations in the thread pool: the one in the aio worker and the one in thread_pool_completion_bh. The latter always causes the optimization to trigger, the former may or may not. In order to restrict the possibilities, I introduced new functions qemu_bh_schedule_slow() and qemu_bh_schedule_fast(): /* qemu_bh_schedule_slow: */ ctx = bh->ctx; bh->idle = 0; if (atomic_xchg(&bh->scheduled, 1) == 0) { event_notifier_set(&ctx->notifier); } /* qemu_bh_schedule_fast: */ ctx = bh->ctx; bh->idle = 0; assert(ctx->dispatching); atomic_xchg(&bh->scheduled, 1); Notice how the atomic_xchg is still in qemu_bh_schedule_slow(). This was already debated a few months ago, so I assumed it to be correct. In retrospect this was a very good idea, as you'll see later. Changing thread_pool_completion_bh() to qemu_bh_schedule_fast() didn't trigger the assertion (as expected). Changing the worker's invocation to qemu_bh_schedule_slow() didn't hide the bug (another assumption which luckily held). This already limited heavily the amount of interaction between the threads, hinting that the problematic events must have triggered around thread_pool_completion_bh(). As mentioned early, invoking a debugger to examine the state of a hung process was pretty easy; the iothread was always waiting on a poll(..., -1) system call. Infinite timeouts are much rarer on x86, and this could be the reason why the bug was never observed there. With the buggy sequence more or less resolved to an interaction between thread_pool_completion_bh() and poll(..., -1), my "tracing" strategy was to just add a few qemu_clock_get_ns(QEMU_CLOCK_REALTIME) calls, hoping that the ordering of aio_ctx_prepare(), aio_ctx_dispatch, poll() and qemu_bh_schedule_fast() would provide some hint. The output was: (gdb) p last_prepare $3 = 103885451 (gdb) p last_dispatch $4 = 103876492 (gdb) p last_poll $5 = 115909333 (gdb) p last_schedule $6 = 115925212 Notice how the last call to qemu_poll_ns() came after aio_ctx_dispatch(). This makes little sense unless there is an aio_poll() call involved, and indeed with a slightly different instrumentation you can see that there is one: (gdb) p last_prepare $3 = 107569679 (gdb) p last_dispatch $4 = 107561600 (gdb) p last_aio_poll $5 = 110671400 (gdb) p last_schedule $6 = 110698917 So the scenario becomes clearer: iothread VCPU thread -------------------------------------------------------------------------- aio_ctx_prepare aio_ctx_check qemu_poll_ns(timeout=-1) aio_poll aio_dispatch thread_pool_completion_bh qemu_bh_schedule() At this point bh->scheduled = 1 and the iothread has not been woken up. The solution must be close, but this alone should not be a problem, because the bottom half is only rescheduled to account for rare situations (see commit 3c80ca1, thread-pool: avoid deadlock in nested aio_poll() calls, 2014-07-15). Introducing a third thread---a thread pool worker thread, which also does qemu_bh_schedule()---does bring out the problematic case. The third thread must be awakened *after* the callback is complete and thread_pool_completion_bh has redone the whole loop, explaining the short race window. And then this is what happens: thread pool worker -------------------------------------------------------------------------- <I/O completes> qemu_bh_schedule() Tada, bh->scheduled is already 1, so qemu_bh_schedule() does nothing and the iothread is never woken up. This is where the bh->scheduled optimization comes into play---it is correct, but removing it would have masked the bug. So, what is the bug? Well, the question asked by the ctx->dispatching optimization ("is any active aio_poll dispatching?") was wrong. The right question to ask instead is "is any active aio_poll *not* dispatching", i.e. in the prepare or poll phases? In that case, the aio_poll is sleeping or might go to sleep anytime soon, and the EventNotifier must be invoked to wake it up. In any other case (including if there is *no* active aio_poll at all!) we can just wait for the next prepare phase to pick up the event (e.g. a bottom half); the prepare phase will avoid the blocking and service the bottom half. Expressing the invariant with a logic formula, the broken one looked like: !(exists(thread): in_dispatching(thread)) => !optimize or equivalently: !(exists(thread): in_aio_poll(thread) && in_dispatching(thread)) => !optimize In the correct one, the negation is in a slightly different place: (exists(thread): in_aio_poll(thread) && !in_dispatching(thread)) => !optimize or equivalently: (exists(thread): in_prepare_or_poll(thread)) => !optimize Even if the difference boils down to moving an exclamation mark :) the implementation is quite different. However, I think the new one is simpler to understand. In the old implementation, the "exists" was implemented with a boolean value. This didn't really support well the case of multiple concurrent event loops, but I thought that this was okay: aio_poll holds the AioContext lock so there cannot be concurrent aio_poll invocations, and I was just considering nested event loops. However, aio_poll _could_ indeed be concurrent with the GSource. This is why I came up with the wrong invariant. In the new implementation, "exists" is computed simply by counting how many threads are in the prepare or poll phases. There are some interesting points to consider, but the gist of the idea remains: 1) AioContext can be used through GSource as well; as mentioned in the patch, bit 0 of the counter is reserved for the GSource. 2) the counter need not be updated for a non-blocking aio_poll, because it won't sleep forever anyway. This is just a matter of checking the "blocking" variable. This requires some changes to the win32 implementation, but is otherwise not too complicated. 3) as mentioned above, the new implementation will not call aio_notify when there is *no* active aio_poll at all. The tests have to be adjusted for this change. The calls to aio_notify in async.c are fine; they only want to kick aio_poll out of a blocking wait, but need not do anything if aio_poll is not running. 4) nested aio_poll: these just work with the new implementation; when a nested event loop is invoked, the outer event loop is never in the prepare or poll phases. The outer event loop thus has already decremented the counter. Reported-by: Richard W. M. Jones <rjones@redhat.com> Reported-by: Laszlo Ersek <lersek@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Tested-by: Richard W.M. Jones <rjones@redhat.com> Message-id: 1437487673-23740-5-git-send-email-pbonzini@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-07-22aio-win32: reorganize polling loopPaolo Bonzini
Preparatory bugfixes and tweaks to the loop before the next patch: - disable dispatch optimization during aio_prepare. This fixes a bug. - do not modify "blocking" until after the first WaitForMultipleObjects call. This is needed in the next patch. - change the loop to do...while. This makes it obvious that the loop is always entered at least once. In the next patch this is important because the first iteration undoes the ctx->notify_me increment that happened before entering the loop. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Tested-by: Richard W.M. Jones <rjones@redhat.com> Message-id: 1437487673-23740-4-git-send-email-pbonzini@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-07-22tests: remove irrelevant assertions from test-aioPaolo Bonzini
In these tests, the purpose of the initial calls to aio_poll and g_main_context_iteration is simply to put the AioContext in a known state; the return value of the function does not really matter. The next patch will change those return values; change the assertions to a while loop which expresses the intention better. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Tested-by: Richard W.M. Jones <rjones@redhat.com> Message-id: 1437487673-23740-3-git-send-email-pbonzini@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-07-22qemu-timer: initialize "timers_done_ev" to setPaolo Bonzini
The normal value for the event is to be set. If we do not do this, pause_all_vcpus (through qemu_clock_enable) hangs unless timerlist_run_timers has been run at least once for the timerlist. This can happen with the following patches, that make aio_notify do nothing most of the time. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Tested-by: Richard W.M. Jones <rjones@redhat.com> Message-id: 1437487673-23740-2-git-send-email-pbonzini@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-07-22mirror: Speed up bitmap initial scanningFam Zheng
Limiting to sectors_per_chunk for each bdrv_is_allocated_above is slow, because the underlying protocol driver would issue much more queries than necessary. We should coalesce the query. Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: <1436413678-7114-4-git-send-email-famz@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2015-07-21Merge remote-tracking branch 'remotes/mdroth/tags/qga-pull-2015-07-21-tag' ↵Peter Maydell
into staging tag for qga-pull-2015-07-21 Small fix to correct schema versioning annotations for recently-added GuestDiskBusType enum values. Not the end of the world, but ideally this inconsistency would be corrected prior to 2.4 release. # gpg: Signature made Tue Jul 21 20:43:24 2015 BST using RSA key ID F108B584 # gpg: Good signature from "Michael Roth <flukshun@gmail.com>" # gpg: aka "Michael Roth <mdroth@utexas.edu>" # gpg: aka "Michael Roth <mdroth@linux.vnet.ibm.com>" # gpg: WARNING: This key is not certified with sufficiently trusted signatures! # gpg: It is not certain that the signature belongs to the owner. # Primary key fingerprint: CEAC C9E1 5534 EBAB B82D 3FA0 3353 C9CE F108 B584 * remotes/mdroth/tags/qga-pull-2015-07-21-tag: qga: fixed versions for guest bus types in qapi-schema Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2015-07-21qga: fixed versions for guest bus types in qapi-schemaOlga Krishtal
Signed-off-by: Olga Krishtal <okrishtal@virtuozzo.com> Signed-off-by: Denis V. Lunev <den@openvz.org> CC: Eric Blake <eblake@redhat.com> CC: Michael Roth <mdroth@linux.vnet.ibm.com> Reviewed-by: Eric Blake <eblake@redhat.com> *added semi-colon to better delineate 2.2 vs. 2.4 versioning Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
2015-07-21Merge remote-tracking branch ↵Peter Maydell
'remotes/pmaydell/tags/pull-target-arm-20150721' into staging target-arm queue: * don't sync CNTVCT with kernel all the time (fixes VM time weirdnesses) * fix a warning compiling disas/arm-a64 with -Wextra # gpg: Signature made Tue Jul 21 12:15:33 2015 BST using RSA key ID 14360CDE # gpg: Good signature from "Peter Maydell <peter.maydell@linaro.org>" * remotes/pmaydell/tags/pull-target-arm-20150721: disas/arm-a64: Add missing compiler attribute GCC_FMT_ATTR target-arm: kvm: Differentiate registers based on write-back levels Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2015-07-21disas/arm-a64: Add missing compiler attribute GCC_FMT_ATTRStefan Weil
Type fprintf_function which fits here was defined with this attribute. Signed-off-by: Stefan Weil <sw@weilnetz.de> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Message-id: 1437208027-14584-1-git-send-email-sw@weilnetz.de Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2015-07-21target-arm: kvm: Differentiate registers based on write-back levelsChristoffer Dall
Some registers like the CNTVCT register should only be written to the kernel as part of machine initialization or on vmload operations, but never during runtime, as this can potentially make time go backwards or create inconsistent time observations between VCPUs. Introduce a list of registers that should not be written back at runtime and check this list on syncing the register state to the KVM state. Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org> Message-id: 1437046488-10773-1-git-send-email-christoffer.dall@linaro.org [PMM: tweaked a few comments, added the new argument to the stub write_list_to_kvmstate() in target-arm/kvm-stub.c] Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2015-07-21Merge remote-tracking branch 'remotes/jnsnow/tags/ide-pull-request' into stagingPeter Maydell
# gpg: Signature made Mon Jul 20 19:27:04 2015 BST using RSA key ID AAFC390E # gpg: Good signature from "John Snow (John Huston) <jsnow@redhat.com>" # gpg: WARNING: This key is not certified with sufficiently trusted signatures! # gpg: It is not certain that the signature belongs to the owner. # Primary key fingerprint: FAEB 9711 A12C F475 812F 18F2 88A9 064D 1835 61EB # Subkey fingerprint: F9B7 ABDB BCAC DF95 BE76 CBD0 7DEF 8106 AAFC 390E * remotes/jnsnow/tags/ide-pull-request: tests: Fix broken targets check-report-qtest-* ahci: Force ICC bits in PxCMD to zero qtest/ide: add another short PRDT test flavor Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
2015-07-20tests: Fix broken targets check-report-qtest-*Stefan Weil
They need QTEST_QEMU_IMG. Without it, the tests raise an assertion: $ make -C bin check-report-qtest-i386.xml make: Entering directory 'bin' GTESTER check-report-qtest-i386.xml blkdebug: Suspended request 'A' blkdebug: Resuming request 'A' ahci-test: tests/libqos/libqos.c:162: mkimg: Assertion `qemu_img_path' failed. main-loop: WARNING: I/O thread spun for 1000 iterations Signed-off-by: Stefan Weil <sw@weilnetz.de> Reviewed-by: John Snow <jsnow@redhat.com> Message-id: 1437231284-17455-1-git-send-email-sw@weilnetz.de Signed-off-by: John Snow <jsnow@redhat.com>
2015-07-20Merge remote-tracking branch 'remotes/stefanha/tags/net-pull-request' into ↵Peter Maydell
staging # gpg: Signature made Mon Jul 20 18:25:14 2015 BST using RSA key ID 81AB73C8 # gpg: Good signature from "Stefan Hajnoczi <stefanha@redhat.com>" # gpg: aka "Stefan Hajnoczi <stefanha@gmail.com>" * remotes/stefanha/tags/net-pull-request: net: Flush queued packets when guest resumes lan9118: Drop lan9118_can_receive etraxfs_eth: Drop eth_can_receive musicpal: Drop eth_can_receive net/vmxnet3: Fix RX TCP/UDP checksum on partially summed packets net/vmxnet3: Refactor 'vmxnet_rx_pkt_attach_data' socket: pass correct size in net_socket_send() Signed-off-by: Peter Maydell <peter.maydell@linaro.org>