[Qemu-commits] [qemu/qemu] 600f5c: virtio-crypto: fix virtio_queue_set_notification()...

[GitHub]

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 600f5ce356b44d8fa5a611ff6b034eb95ecf04e7
      
https://github.com/qemu/qemu/commit/600f5ce356b44d8fa5a611ff6b034eb95ecf04e7
  Author: Stefan Hajnoczi <address@hidden>
  Date:   2016-11-18 (Fri, 18 Nov 2016)

  Changed paths:
    M hw/virtio/virtio-crypto.c

  Log Message:
  -----------
  virtio-crypto: fix virtio_queue_set_notification() race

We must check for new virtqueue buffers after re-enabling notifications.
This prevents the race condition where the guest added buffers just
after we stopped popping the virtqueue but before we re-enabled
notifications.

I think the virtio-crypto code was based on virtio-net but this crucial
detail was missed.  virtio-net does not have the race condition because
it processes the virtqueue one more time after re-enabling
notifications.

Cc: Gonglei <address@hidden>
Signed-off-by: Stefan Hajnoczi <address@hidden>
Tested-by: Alexey Kardashevskiy <address@hidden>
Reviewed-by: Michael S. Tsirkin <address@hidden>
Signed-off-by: Michael S. Tsirkin <address@hidden>
Reviewed-by: Gonglei <address@hidden>


  Commit: 310837de6c1e0badfd736b1b316b1698c53120a7
      
https://github.com/qemu/qemu/commit/310837de6c1e0badfd736b1b316b1698c53120a7
  Author: Paolo Bonzini <address@hidden>
  Date:   2016-11-18 (Fri, 18 Nov 2016)

  Changed paths:
    M hw/virtio/vhost.c
    M hw/virtio/virtio-bus.c
    M hw/virtio/virtio.c
    M include/hw/virtio/virtio-bus.h
    M include/hw/virtio/virtio.h

  Log Message:
  -----------
  virtio: introduce grab/release_ioeventfd to fix vhost

Following the recent refactoring of virtio notifiers [1], more specifically
the patch ed08a2a0b ("virtio: use virtio_bus_set_host_notifier to
start/stop ioeventfd") that uses virtio_bus_set_host_notifier [2]
by default, core virtio code requires 'ioeventfd_started' to be set
to true/false when the host notifiers are configured.

When vhost is stopped and started, however, there is a stop followed by
another start. Since ioeventfd_started was never set to true, the 'stop'
operation triggered by virtio_bus_set_host_notifier() will not result
in a call to virtio_pci_ioeventfd_assign(assign=false). This leaves
the memory regions with stale notifiers and results on the next start
triggering the following assertion:

  kvm_mem_ioeventfd_add: error adding ioeventfd: File exists
  Aborted

This patch reintroduces (hopefully in a cleaner way) the concept
that was present with ioeventfd_disabled before the refactoring.
When ioeventfd_grabbed>0, ioeventfd_started tracks whether ioeventfd
should be enabled or not, but ioeventfd is actually not started at
all until vhost releases the host notifiers.

[1] http://lists.nongnu.org/archive/html/qemu-devel/2016-10/msg07748.html
[2] http://lists.nongnu.org/archive/html/qemu-devel/2016-10/msg07760.html

Reported-by: Felipe Franciosi <address@hidden>
Reported-by: Christian Borntraeger <address@hidden>
Reported-by: Alex Williamson <address@hidden>
Fixes: ed08a2a0b ("virtio: use virtio_bus_set_host_notifier to start/stop 
ioeventfd")
Reviewed-by: Cornelia Huck <address@hidden>
Reviewed-by: Stefan Hajnoczi <address@hidden>
Tested-by: Alexey Kardashevskiy <address@hidden>
Tested-by: Farhan Ali <address@hidden>
Tested-by: Alex Williamson <address@hidden>
Signed-off-by: Paolo Bonzini <address@hidden>
Reviewed-by: Michael S. Tsirkin <address@hidden>
Signed-off-by: Michael S. Tsirkin <address@hidden>


  Commit: 0687c37c5eeef8580b31cc6e1202d874833ae38a
      
https://github.com/qemu/qemu/commit/0687c37c5eeef8580b31cc6e1202d874833ae38a
  Author: Paolo Bonzini <address@hidden>
  Date:   2016-11-18 (Fri, 18 Nov 2016)

  Changed paths:
    M hw/virtio/virtio-mmio.c
    M hw/virtio/virtio-pci.c
    M hw/virtio/virtio.c

  Log Message:
  -----------
  virtio: access ISR atomically

This will be needed once dataplane will be able to set it outside
the big QEMU lock.

Reviewed-by: Stefan Hajnoczi <address@hidden>
Tested-by: Farhan Ali <address@hidden>
Tested-by: Alex Williamson <address@hidden>
Signed-off-by: Paolo Bonzini <address@hidden>
Reviewed-by: Michael S. Tsirkin <address@hidden>
Signed-off-by: Michael S. Tsirkin <address@hidden>


  Commit: 83d768b5640946b7da55ce8335509df297e2c7cd
      
https://github.com/qemu/qemu/commit/83d768b5640946b7da55ce8335509df297e2c7cd
  Author: Paolo Bonzini <address@hidden>
  Date:   2016-11-18 (Fri, 18 Nov 2016)

  Changed paths:
    M hw/block/dataplane/virtio-blk.c
    M hw/scsi/virtio-scsi-dataplane.c
    M hw/scsi/virtio-scsi.c
    M hw/virtio/trace-events
    M hw/virtio/virtio.c
    M include/hw/virtio/virtio-scsi.h
    M include/hw/virtio/virtio.h

  Log Message:
  -----------
  virtio: set ISR on dataplane notifications

Dataplane has been omitting forever the step of setting ISR when
an interrupt is raised.  This caused little breakage, because the
specification actually says that ISR may not be updated in MSI mode.

Some versions of the Windows drivers however didn't clear MSI mode
correctly, and proceeded using polling mode (using ISR, not the used
ring index!) for crashdump and hibernation.  If it were just crashdump
and hibernation it would not be a big deal, but recent releases of
Windows do not really shut down, but rather log out and hibernate to
make the next startup faster.  Hence, this manifested as a more serious
hang during shutdown with e.g. Windows 8.1 and virtio-win 1.8.0 RPMs.
Newer versions fixed this, while older versions do not use MSI at all.

The failure has always been there for virtio dataplane, but it became
visible after commits 9ffe337 ("virtio-blk: always use dataplane path
if ioeventfd is active", 2016-10-30) and ad07cd6 ("virtio-scsi: always
use dataplane path if ioeventfd is active", 2016-10-30) made virtio-blk
and virtio-scsi always use the dataplane code under KVM.  The good news
therefore is that it was not a bug in the patches---they were doing
exactly what they were meant for, i.e. shake out remaining dataplane bugs.

The fix is not hard, so it's worth arranging for the broken drivers.
The virtio_should_notify+event_notifier_set pair that is common to
virtio-blk and virtio-scsi dataplane is replaced with a new public
function virtio_notify_irqfd that also sets ISR.  The irqfd emulation
code now need not set ISR anymore, so virtio_irq is removed.

Reviewed-by: Stefan Hajnoczi <address@hidden>
Tested-by: Farhan Ali <address@hidden>
Tested-by: Alex Williamson <address@hidden>
Signed-off-by: Paolo Bonzini <address@hidden>
Reviewed-by: Michael S. Tsirkin <address@hidden>
Signed-off-by: Michael S. Tsirkin <address@hidden>


  Commit: be4e0d737527d8670dc271712faae0de6a181b4e
      
https://github.com/qemu/qemu/commit/be4e0d737527d8670dc271712faae0de6a181b4e
  Author: Zhuang Yanying <address@hidden>
  Date:   2016-11-18 (Fri, 18 Nov 2016)

  Changed paths:
    M hw/misc/ivshmem.c

  Log Message:
  -----------
  ivshmem: Fix 64 bit memory bar configuration

Device ivshmem property use64=0 is designed to make the device
expose a 32 bit shared memory BAR instead of 64 bit one.  The
default is a 64 bit BAR, except pc-1.2 and older retain a 32 bit
BAR.  A 32 bit BAR can support only up to 1 GiB of shared memory.

This worked as designed until commit 5400c02 accidentally flipped
its sense: since then, we misinterpret use64=0 as use64=1 and vice
versa.  Worse, the default got flipped as well.  Devices
ivshmem-plain and ivshmem-doorbell are not affected.

Fix by restoring the test of IVShmemState member not_legacy_32bit
that got messed up in commit 5400c02.  Also update its
initialization for devices ivhsmem-plain and ivshmem-doorbell.
Without that, they'd regress to 32 bit BARs.

Cc: address@hidden
Signed-off-by: Zhuang Yanying <address@hidden>
Reviewed-by: Gonglei <address@hidden>
Reviewed-by: Marc-André Lureau <address@hidden>
Reviewed-by: Michael S. Tsirkin <address@hidden>
Signed-off-by: Michael S. Tsirkin <address@hidden>
Reviewed-by: Markus Armbruster <address@hidden>


  Commit: d668fc4c7c69a3251be5965601015f3c17800818
      
https://github.com/qemu/qemu/commit/d668fc4c7c69a3251be5965601015f3c17800818
  Author: ZhuangYanying <address@hidden>
  Date:   2016-11-18 (Fri, 18 Nov 2016)

  Changed paths:
    M hw/ipmi/isa_ipmi_kcs.c

  Log Message:
  -----------
  ipmi: fix qemu crash while migrating with ipmi

Qemu crash in the source side while migrating, after starting ipmi service 
inside vm.

./x86_64-softmmu/qemu-system-x86_64 --enable-kvm -smp 4 -m 4096 \
-drive 
file=/work/suse/suse11_sp3_64_vt,format=raw,if=none,id=drive-virtio-disk0,cache=none
 \
-device 
virtio-blk-pci,scsi=off,bus=pci.0,addr=0x5,drive=drive-virtio-disk0,id=virtio-disk0
 \
-vnc :99 -monitor vc -device ipmi-bmc-sim,id=bmc0 -device 
isa-ipmi-kcs,bmc=bmc0,ioport=0xca2

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ffec4268700 (LWP 7657)]
__memcpy_ssse3_back () at ../sysdeps/x86_64/multiarch/memcpy-ssse3-back.S:2757
(gdb) bt
 #0  __memcpy_ssse3_back () at 
../sysdeps/x86_64/multiarch/memcpy-ssse3-back.S:2757
 #1  0x00005555559ef775 in memcpy (__len=3, __src=0xc1421c, __dest=<optimized 
out>)
     at /usr/include/bits/string3.h:51
 #2  qemu_put_buffer (f=0x555557a97690, buf=0xc1421c <Address 0xc1421c out of 
bounds>, size=3)
     at migration/qemu-file.c:346
 #3  0x00005555559eef66 in vmstate_save_state (address@hidden,
     vmsd=0x555555f8a5a0 <vmstate_ISAIPMIKCSDevice>, opaque=0x555557231160,
     address@hidden) at migration/vmstate.c:333
 #4  0x00005555557cfe45 in vmstate_save (address@hidden, address@hidden,
     address@hidden) at /mnt/sdb/zyy/qemu/migration/savevm.c:720
 #5  0x00005555557d2be7 in qemu_savevm_state_complete_precopy (f=0x555557a97690,
     address@hidden) at /mnt/sdb/zyy/qemu/migration/savevm.c:1128
 #6  0x00005555559ea102 in migration_completion (start_time=<synthetic pointer>,
     old_vm_running=<synthetic pointer>, current_active_state=<optimized out>,
     s=0x5555560eaa80 <current_migration.44078>) at migration/migration.c:1707
 #7  migration_thread (opaque=0x5555560eaa80 <current_migration.44078>) at 
migration/migration.c:1855
 #8  0x00007ffff3900dc5 in start_thread (arg=0x7ffec4268700) at 
pthread_create.c:308
 #9  0x00007fffefc6c71d in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:113

Signed-off-by: Zhuang Yanying <address@hidden>
Reviewed-by: Michael S. Tsirkin <address@hidden>
Signed-off-by: Michael S. Tsirkin <address@hidden>


  Commit: 4b5b47abbf23246bd8dde4c6faaed8b7249d8654
      
https://github.com/qemu/qemu/commit/4b5b47abbf23246bd8dde4c6faaed8b7249d8654
  Author: Eduardo Habkost <address@hidden>
  Date:   2016-11-18 (Fri, 18 Nov 2016)

  Changed paths:
    M hw/i386/acpi-build.c

  Log Message:
  -----------
  acpi: Use apic_id_limit when calculating legacy ACPI table size

The code that calculates the legacy ACPI table size for migration
compatibility uses max_cpus when calculating legacy_aml_len (the size of
the DSDT and SSDT tables). However, the SSDT grows according to APIC ID
limit, not max_cpus.

The bug is not triggered very often because of the 4k alignment on the
table size. But it can be triggered if you are unlucky enough to cross a
4k boundary.

Change the legacy_aml_len calculation to use apic_id_limit, to calculate
the right size.

Signed-off-by: Eduardo Habkost <address@hidden>
Reviewed-by: Michael S. Tsirkin <address@hidden>
Signed-off-by: Michael S. Tsirkin <address@hidden>


  Commit: c36ed06e9159fa484b711dfdd27ec64d7ac3d17a
      
https://github.com/qemu/qemu/commit/c36ed06e9159fa484b711dfdd27ec64d7ac3d17a
  Author: Stefan Hajnoczi <address@hidden>
  Date:   2016-11-21 (Mon, 21 Nov 2016)

  Changed paths:
    M hw/block/dataplane/virtio-blk.c
    M hw/i386/acpi-build.c
    M hw/ipmi/isa_ipmi_kcs.c
    M hw/scsi/virtio-scsi-dataplane.c
    M hw/scsi/virtio-scsi.c
    M hw/virtio/trace-events
    M hw/virtio/vhost.c
    M hw/virtio/virtio-bus.c
    M hw/virtio/virtio-crypto.c
    M hw/virtio/virtio-mmio.c
    M hw/virtio/virtio-pci.c
    M hw/virtio/virtio.c
    M include/hw/virtio/virtio-bus.h
    M include/hw/virtio/virtio-scsi.h
    M include/hw/virtio/virtio.h

  Log Message:
  -----------
  Merge remote-tracking branch 'mst/tags/for_upstream' into staging

virtio, vhost, pc: fixes

Most notably this fixes a regression with vhost introduced by the pull before
last.

Signed-off-by: Michael S. Tsirkin <address@hidden>

# gpg: Signature made Fri 18 Nov 2016 03:51:55 PM GMT
# gpg:                using RSA key 0x281F0DB8D28D5469
# gpg: Good signature from "Michael S. Tsirkin <address@hidden>"
# gpg:                 aka "Michael S. Tsirkin <address@hidden>"
# Primary key fingerprint: 0270 606B 6F3C DF3D 0B17  0970 C350 3912 AFBE 8E67
#      Subkey fingerprint: 5D09 FD08 71C8 F85B 94CA  8A0D 281F 0DB8 D28D 5469

* mst/tags/for_upstream:
  acpi: Use apic_id_limit when calculating legacy ACPI table size
  ipmi: fix qemu crash while migrating with ipmi
  ivshmem: Fix 64 bit memory bar configuration
  virtio: set ISR on dataplane notifications
  virtio: access ISR atomically
  virtio: introduce grab/release_ioeventfd to fix vhost
  virtio-crypto: fix virtio_queue_set_notification() race

Message-id: address@hidden
Signed-off-by: Stefan Hajnoczi <address@hidden>


Compare: https://github.com/qemu/qemu/compare/d93b1fb009b6...c36ed06e9159