In virtio_queue_host_notifier_aio_poll, not all "!virtio_queue_empty()"
cases are making true progress.
Currently the offending one is virtio-scsi event queue, whose handler
does nothing if no event is pending. As a result aio_poll() will spin on
the "non-empty" VQ and take 100% host CPU.
Fix this by reporting actual progress from virtio queue aio handlers.
Reported-by: Ed Swierk <eswierk@skyportsystems.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Tested-by: Ed Swierk <eswierk@skyportsystems.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Currently, all virtio devices bypass IOMMU completely. This is because
address_space_memory is assumed and used during DMA emulation. This
patch converts the virtio core API to use DMA API. This idea is
- introducing a new transport specific helper to query the dma address
space. (only pci version is implemented).
- query and use this address space during virtio device guest memory
accessing when iommu platform (VIRTIO_F_IOMMU_PLATFORM) was enabled
for this device.
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Amit Shah <amit.shah@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-block@nongnu.org
Signed-off-by: Jason Wang <jasowang@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
The guest does not need to kick the virtqueue while we are processing
it. This reduces the number of vmexits during periods of heavy I/O.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20161201192652.9509-9-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
These will be used more as soon as the acquire/release is pushed down to
the ioeventfd handlers.
Reviewed-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
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 <stefanha@redhat.com>
Tested-by: Farhan Ali <alifm@linux.vnet.ibm.com>
Tested-by: Alex Williamson <alex.williamson@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Override start_ioeventfd and stop_ioeventfd to start/stop the
whole dataplane logic. This has some positive side effects:
- no need anymore for virtio_add_queue_aio (i.e. a revert of
commit 1c627137c1)
- no need anymore to switch from generic ioeventfd handlers to
dataplane
It detects some errors better:
$ qemu-system-x86_64 -object iothread,id=io \
-device virtio-scsi-pci,ioeventfd=off,iothread=io
qemu-system-x86_64: -device virtio-scsi-pci,ioeventfd=off,iothread=io:
ioeventfd is required for iothread
while previously it would have started just fine.
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Now all the usages of the old version of VMSTATE_VIRTIO_DEVICE are gone,
so we can get rid of the conditionals, and the old macro.
Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Use the new VMSTATE_VIRTIO_DEVICE macro.
Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
This error is caused by a buggy guest: let's switch the device to the
broken state instead of terminating QEMU.
Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
The virtio_scsi_bad_req() function is called when a guest sends a
request with missing or ill-sized headers. This generally happens
when the virtio_scsi_parse_req() function returns an error.
With this patch, virtio_scsi_bad_req() will mark the device as broken,
detach the request from the virtqueue and free it, instead of forcing
QEMU to exit.
In nearly all locations where virtio_scsi_bad_req() is called, the only
thing to do next is to return to the caller.
The virtio_scsi_handle_cmd_req_prepare() function is an exception though.
It is called in a loop by virtio_scsi_handle_cmd_vq() and passed requests
freshly popped from a cmd virtqueue; virtio_scsi_handle_cmd_req_prepare()
does some sanity checks on the request and returns a boolean flag to
indicate whether the request should be queued or not. In the latter case,
virtio_scsi_handle_cmd_req_prepare() has detected a non-fatal error and
sent a response back to the guest.
We have now a new condition to take into account: the device is broken
and should stop all processing.
The return value of virtio_scsi_handle_cmd_req_prepare() is hence changed
to an int. A return value of zero means that the request should be queued.
Other non-fatal error cases where the request shoudn't be queued return
a negative errno (values are vaguely inspired by the error condition, but
the only goal here is to discriminate the case we're interested in).
And finally, if virtio_scsi_bad_req() was called, -EINVAL is returned. In
this case, virtio_scsi_handle_cmd_vq() detaches and frees already queued
requests, instead of submitting them.
Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
With an ejected block backend, blk_get_aio_context() would return
qemu_aio_context. In this case don't assert.
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <1473848224-24809-3-git-send-email-famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Forcibly convert it to a vmstate wrapper; proper conversion
comes later.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
There is a new common one in virtio.h, use it.
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
AIO based handler is more appropriate here because it will then
cooperate with bdrv_drained_begin/end. It is needed by the coming
revert patch.
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Tracked down with an ugly, brittle and probably buggy Perl script.
Also move includes converted to <...> up so they get included before
ours where that's obviously okay.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Tested-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Richard Henderson <rth@twiddle.net>
This reverts commit 10a06fd65f.
Dataplane has used the same virtqueue code as non-dataplane since
commits e24a47c5b7 ("virtio-scsi: do not
use vring in dataplane") and 03de2f5274
("virtio-blk: do not use vring in dataplane"). It is no longer
necessary to stop dataplane in order to sync state since there is no
duplicated virtqueue state.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Reviewed-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
Message-id: 1466503331-9831-1-git-send-email-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
The virtio_queue_get_id() function is the lesser used duplicate of
virtio_get_queue_index(). Use the latter instead.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1463767461-17922-1-git-send-email-stefanha@redhat.com
The previous patch dropped all op blockers from virtio-blk data plane.
The situation of virtio-scsi is exactly the same it can drop them too.
Signed-off-by: Fam Zheng <famz@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Message-id: 1463969978-24970-5-git-send-email-famz@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
In addition to handling IO in vcpu thread and in io thread, dataplane
introduces yet another mode: handling it by AioContext.
This reuses the same handler as previous modes, which triggers races as
these were not designed to be reentrant. Use a separate handler just
for aio, and disable regular handlers when dataplane is active.
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Add two missing checks for s->dataplane_fenced. In one case, QEMU
would skip injecting an IRQ due to a write to an uninitialized
EventNotifier's file descriptor.
In the second case, the dataplane_disabled field was used by mistake;
in fact after fixing this occurrence it is completely unused.
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Commit 57cb38b included qapi/error.h into qemu/osdep.h to get the
Error typedef. Since then, we've moved to include qemu/osdep.h
everywhere. Its file comment explains: "To avoid getting into
possible circular include dependencies, this file should not include
any other QEMU headers, with the exceptions of config-host.h,
compiler.h, os-posix.h and os-win32.h, all of which are doing a
similar job to this file and are under similar constraints."
qapi/error.h doesn't do a similar job, and it doesn't adhere to
similar constraints: it includes qapi-types.h. That's in excess of
100KiB of crap most .c files don't actually need.
Add the typedef to qemu/typedefs.h, and include that instead of
qapi/error.h. Include qapi/error.h in .c files that need it and don't
get it now. Include qapi-types.h in qom/object.h for uint16List.
Update scripts/clean-includes accordingly. Update it further to match
reality: replace config.h by config-target.h, add sysemu/os-posix.h,
sysemu/os-win32.h. Update the list of includes in the qemu/osdep.h
comment quoted above similarly.
This reduces the number of objects depending on qapi/error.h from "all
of them" to less than a third. Unfortunately, the number depending on
qapi-types.h shrinks only a little. More work is needed for that one.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
[Fix compilation without the spice devel packages. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Move allocation to virtio functions also when loading/saving a
VirtQueueElement. This will also let the load/save functions
keep backwards compatibility when the VirtQueueElement layout
is changed.
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
The return code of virtqueue_pop/vring_pop is unused except to check for
errors or 0. We can thus easily move allocation inside the functions
and just return a pointer to the VirtQueueElement.
The advantage is that we will be able to allocate only the space that
is needed for the actual size of the s/g list instead of the full
VIRTQUEUE_MAX_SIZE items. Currently VirtQueueElement takes about 48K
of memory, and this kind of allocation puts a lot of stress on malloc.
By cutting the size by two or three orders of magnitude, malloc can
use much more efficient algorithms.
The patch is pretty large, but changes to each device are testable
more or less independently. Splitting it would mostly add churn.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
The next patch will make virtqueue_pop/vring_pop allocate memory for
the VirtQueueElement. In some cases (blk, scsi, gpu) the device wants
to extend VirtQueueElement with device-specific fields and, until now,
the place of the VirtQueueElement within the containing struct didn't
matter. When allocating the entire block in virtqueue_pop/vring_pop,
however, the containing struct must basically be a "subclass" of
VirtQueueElement, with the VirtQueueElement as the first field. Make
that the case for blk and scsi; gpu is already doing it.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Make use of the BDS-BB removal and insertion notifiers to remove or set
up, respectively, virtio-scsi's op blockers.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.
This commit was created with scripts/clean-includes.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 1453832250-766-15-git-send-email-peter.maydell@linaro.org
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <1452073066-28319-1-git-send-email-caoj.fnst@cn.fujitsu.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Make sure that we actually have a device when checking the aio
context. Otherwise guests could trigger QEMU crashes.
Signed-off-by: "Eugene (jno) Dvurechenski" <jno@linux.vnet.ibm.com>
Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Message-Id: <1448549135-6582-2-git-send-email-jno@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
When creating snapshot with the dataplane enabled, the snapshot file gets
not the actual state of virtqueue, because the current state is stored in
VirtIOBlockDataPlane. Therefore, before saving snapshot need to sync
the dataplane vring state to the virtqueue. The dataplane will resume its
work at the next notify virtqueue.
When snapshot loads with loadvm we get a message:
VQ 0 size 0x80 Guest index 0x15f5 inconsistent with Host index 0x0:
delta 0x15f5
error while loading state for instance 0x0 of device
'0000:00:08.0/virtio-blk'
Error -1 while loading VM state
to reproduce the error I used the following hmp commands:
savevm snap1
loadvm snap1
qemu parameters:
--enable-kvm -smp 4 -m 1024 -drive file=/var/lib/libvirt/images/centos6.4.qcow2,if=none,id=drive-virtio-disk0,format=qcow2,cache=none,aio=native -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x8,drive=drive-virtio-disk0,id=virtio-disk0 -set device.virtio-disk0.x-data-plane=on
Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Message-id: 1445859777-2982-1-git-send-email-den@openvz.org
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: "Michael S. Tsirkin" <mst@redhat.com>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Note: virtqueue_map already validates input
so virtio-scsi does not have to.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Simplify memory allocation by sticking with a single API. GSlice
is not that fast anyway (tcmalloc/jemalloc are better).
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Commit ef546f1275 ("virtio: add
feature checking helpers") introduced a helper __virtio_has_feature.
We don't want to use reserved identifiers, though, so let's
rename __virtio_has_feature to virtio_has_feature and virtio_has_feature
to virtio_vdev_has_feature.
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
The VirtQueueElement struct is serialized during migration but the
in_sg[]/out_sg[] iovec arrays are not usable on the destination host
because the pointers are meaningless.
Use virtqueue_map_sg() to refresh in_sg[]/out_sg[] to valid pointers
based on in_addr[]/out_addr[] hwaddrs.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <1438262173-11546-2-git-send-email-stefanha@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
As only one place in virtio-scsi.c uses DEFINE_VIRTIO_SCSI_PROPERTIES
and DEFINE_VIRTIO_SCSI_FEATURES, there is no need to expose them. Inline
them into virtio-scsi.c to avoid wrongly use.
Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Make features 64bit wide everywhere.
On migration a full 64bit guest_features field is sent if one of the
high bits is set, in addition to the lower 32bit guest_features field
which must stay for compatibility reasons. That way we send the lower
32 feature bits twice, but the code is simpler because we don't have
to split and compose the 64bit features into two 32bit fields.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
VIRTIO_PCI_QUEUE_MAX is not only used for pci, so rename it be generic.
Cc: Amit Shah <amit.shah@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
So far virtio-scsi-device can't expose host features to guest while
using virtio-mmio because it doesn't set DEFINE_VIRTIO_SCSI_FEATURES on
backend or transport.
The host features belong to the backends while virtio-scsi-pci,
virtio-scsi-s390 and virtio-scsi-ccw set the DEFINE_VIRTIO_SCSI_FEATURES
on transports. But they already have the ability to forward property
accesses to the backend child. So if we move the host features to
backends, it doesn't break the backwards compatibility for them and
make host features work while using virtio-mmio.
Move DEFINE_VIRTIO_SCSI_FEATURES to the backend virtio-scsi. The
transports just sync the host features from backends.
Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Hotplugging a scsi-disk may trigger the assertion in qemu_sgl_concat.
qemu-system-x86_64: qemu/hw/scsi/virtio-scsi.c:115: qemu_sgl_concat:
Assertion `skip == 0' failed.
This is introduced by commit 55783a55 (virtio-scsi: work around bug in
old BIOSes) which didn't check out_num when accessing out_sg[0].iov_len
(the same to in sg). For virtio_scsi_push_event, looking into out_sg
doesn't make sense because 0 req_size is intended.
Cc: qemu-stable@nongnu.org
[Cc'ing qemu-stable because 55783a55 did it too]
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <1426233354-525-1-git-send-email-famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
cdb is now part of cmd, drop it from req.
There's also nothing to check using build assert now.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Commit "virtio-scsi: use standard-headers" added
cdb and sense into req/rep structures, which
breaks uses of sizeof for these structures,
since qemu adds its own arrays on top.
To fix, redefine CDB/sense field size to 0.
Reported-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
A bunch of fixes all over the place.
All of ACPI refactoring has been merged.
Legacy pci commands have been dropped.
virtio header cleanup
initial patches from virtio-1.0 branch
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
iQEcBAABAgAGBQJU/CoXAAoJECgfDbjSjVRpX7EH/RMmgtsDO4wvqJu++lHvkB/q
kSaXZYTpJTo0i5JE7n2brwuXA4902tTg9g5TMUpGPh9Pt2QRg7RTgGC1vqZyOBos
MPw+4BO2v66S6qgX7bOf222z7r64cHTY7pLkQlrfD4usPlu2eusZ64UTW6Ru51fW
WF9E9aunbl+HnuCGq6Iez3sCLscTBJpU/lEr6oSyHhuq3aa0CjjraEeV0E/QcwJG
HTUeFymL8NFvlXZblsLI++VOv7Mxpi6yiCQ5XoKpFgGMvidwo41Aso6gB3ySGxOd
w8O3Nbu77Iw/StDRNCg/5/GapabMKh2bE4UCsYY5OS63ZtD0fl0CCblhzm/ZFPw=
=LY/j
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging
pci, pc, virtio fixes and cleanups
A bunch of fixes all over the place.
All of ACPI refactoring has been merged.
Legacy pci commands have been dropped.
virtio header cleanup
initial patches from virtio-1.0 branch
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
* remotes/mst/tags/for_upstream: (130 commits)
acpi: drop unused code
aml-build: comment fix
acpi-build: fix typo in comment
acpi: update generated files
vhost user:support vhost user nic for non msi guests
aml-build: fix build for glib < 2.22
acpi: update generated files
Makefile.target: binary depends on config-devices
acpi-test-data: update after pci rewrite
acpi, mem-hotplug: use PC_DIMM_SLOT_PROP in acpi_memory_plug_cb().
pci-hotplug-old: Has been dead for five major releases, bury
pci: Give a few helpers internal linkage
acpi: make build_*() routines static to aml-build.c
pc: acpi: remove not used anymore ssdt-[misc|pcihp].hex.generated blobs
pc: acpi-build: drop template patching and create PCI bus tree dynamically
tests: ACPI: update pc/SSDT.bridge due to new alg of PCI tree creation
pc: acpi-build: simplify PCI bus tree generation
tests: add ACPI blobs for qemu with bridge cases
tests: bios-tables-test: add support for testing bridges
tests: ACPI test blobs update due to PCI0._CRS changes
...
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Conflicts:
hw/pci/pci-hotplug-old.c
s->blocker is really only used in hw/scsi/virtio-scsi.c; the only places
where it is used in hw/scsi/virtio-scsi-dataplane.c is when it is
allocated and when it is freed. That does not make a whole lot of sense
(and is actually wrong because this leads to s->blocker potentially
being NULL when blk_op_block_all() is called in virtio-scsi.c), so move
the allocation and destruction of s->blocker to the device realization
and unrealization in virtio-scsi.c, respectively.
Case in point:
$ echo -e 'eject drv\nquit' | \
x86_64-softmmu/qemu-system-x86_64 \
-monitor stdio -machine accel=qtest -display none \
-object iothread,id=thr -device virtio-scsi-pci,iothread=thr \
-drive if=none,file=test.qcow2,format=qcow2,id=drv \
-device scsi-cd,drive=drv
Without this patch:
(qemu) eject drv
[1] 10102 done
10103 segmentation fault (core dumped)
With this patch:
(qemu) eject drv
Device 'drv' is busy: block device is in use by data plane
(qemu) quit
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <1425057113-26940-1-git-send-email-mreitz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
It's not safe to call blk_set_aio_context from outside BQL because of
the bdrv_drain_all there. Let's put it in the hotplug callback which
will be called by qdev device realization for each scsi device attached
to the bus.
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <1423969591-23646-3-git-send-email-famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Add a helper function for checking whether a bit is set in the guest
features for a vdev as well as one that works on a feature bit set.
Convert code that open-coded this: It cleans up the code and makes it
easier to extend the guest feature bits.
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Several places check against the feature bit number instead of against
the feature bit. Fix them.
Cc: qemu-stable@nongnu.org
Reported-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
g_new(T, n) is neater than g_malloc(sizeof(T) * n). It's also safer,
for two reasons. One, it catches multiplication overflowing size_t.
Two, it returns T * rather than void *, which lets the compiler catch
more type errors.
This commit only touches allocations with size arguments of the form
sizeof(T).
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Old BIOSes left some padding by mistake after the req_size/resp_size.
New QEMU does not like it, thinking it is a bidirectional command.
As a workaround, we can check if the ANY_LAYOUT bit is set; if not, we
always consider the first buffer as the virtio-scsi request/response,
because, back when QEMU did not support ANY_LAYOUT, it expected the
payload to start at the second element of the iovec.
This can show up during migration.
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
We need to count the ctrlq and eventq, and also cleanup before
returning. Besides, the format string should be unsigned.
The number could never be less than zero.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The local variable "type" in virtio_scsi_handle_ctl represents the tmf
command type from the guest and it has the same meaning as the
req->req.tmf.type. However, before the invoking of virtio_scsi_parse_req
the req->req.tmf.type doesn't has the correct value(just initialized to
zero). Therefore, we need to use the "type" variable to judge the case.
Cc: qemu-stable@nongnu.org
Signed-off-by: Bin Wu <wu.wubin@huawei.com>
[Actually make it compile, "type" must be uint32_t in order to pass
it to virtio_tswap32s. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
If req->resp.cmd.status is not GOOD, the address of sense for
qemu_iovec_from_buf should be modified from &req->resp to sense.
Cc: qemu-stable@nongnu.org
Signed-off-by: Ting Wang <kathy.wangting@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
We need this to protect dataplane thread from race conditions with block
jobs until the latter is made dataplane-safe.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Device models should access their block backends only through the
block-backend.h API. Convert them, and drop direct includes of
inappropriate headers.
Just four uses of BlockDriverState are left:
* The Xen paravirtual block device backend (xen_disk.c) opens images
itself when set up via xenbus, bypassing blockdev.c. I figure it
should go through qmp_blockdev_add() instead.
* Device model "usb-storage" prompts for keys. No other device model
does, and this one probably shouldn't do it, either.
* ide_issue_trim_cb() uses bdrv_aio_discard() instead of
blk_aio_discard() because it fishes its backend out of a BlockAIOCB,
which has only the BlockDriverState.
* PC87312State has an unused BlockDriverState[] member.
The next two commits take care of the latter two.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
scsi_req_continue can complete the request and cause the VirtIOSCSIReq
to be freed. Fetch req->sreq just once to avoid the bug.
Reported-by: Richard Jones <rjones@redhat.com>
Tested-by: Richard Jones <rjones@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
For VIRTIO_SCSI_T_TMF_ABORT_TASK and VIRTIO_SCSI_T_TMF_ABORT_TASK_SET,
use scsi_req_cancel_async to start the cancellation.
Because each tmf command may cancel multiple requests, we need to use a
counter to track the number of remaining requests we still need to wait
for.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Queue the popped requests while calling
virtio_scsi_handle_cmd_req_prepare(), then submit them after all
prepared.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Mechanical change, in preparation for bdrv_io_plug/bdrv_io_unplug.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Similar to virtio-blk-dataplane, we stop the iothread while migration
starts and restart it when migration finishes.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This enables the virtio-scsi-dataplane code by setting the iothread
in virtio-scsi device, and makes any function that is called by
back from dataplane to cooperate with the caller: they need to be
vring/iothread aware when handling the requests and using scsi devices
on the bus.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Similar to this property in virtio-blk for dataplane, add it as a QOM
link in virtio-scsi and an alias in virtio-scsi-pci and virtio-scsi-ccw,
in order to assign an iothread to the device.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This is the "common part" to handle one cmd request. Refactor out for
later usage of dataplane iothread code.
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The command direction according to the guest-passed buffers
is already stored in the VirtIOSCSIReq. We can use it instead
of computing it again from req->elem.
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The VirtQueueElement is a very big structure (>48k!), since it will be
initialzed by virtqueue_pop, we can save the expensive zeroing here.
This saves a few microseconds per request in my test:
[fio-test] rw bs iodepth jobs bw iops latency
--------------------------------------------------------------------------------------------
Before read 4k 1 1 110 28269 34
After read 4k 1 1 131 33745 28
Whereas,
virtio-blk read 4k 1 1 217 55673 16
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
No cmd vq surprises guest (Linux panics in virtscsi_probe), too many
queues abort qemu (in the following virtio_add_queue).
Signed-off-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Virtio SCSI Events need to be byteswapped before being pushed
when host and guest have a different endianness. Not doing so
breaks hotplug of virtio scsi disks, with the following error
message being printed in the guest console:
virtio_scsi: Unsupport virtio scsi event 1000000
This issue got uncovered while testing disk hotplug with a PowerKVM
ppc64le guest. I have checked that this issue also affects a x86_64
guest run on a ppc64 host.
Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
[ Ported from PowerKVM,
Greg Kurz <gkurz@linux.vnet.ibm.com> ]
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Hotplug of a virtio scsi disk is currently broken: no disk appears in the
guest (verified with a fedora 20 host running a fedora 20 guest with KVM).
Bisect leeds to Paolo's patches to support any_layout, especially this
commit:
commit 36b15c79aa
Author: Paolo Bonzini <pbonzini@redhat.com>
Date: Tue Jun 10 16:21:18 2014 +0200
virtio-scsi: start preparing for any_layout
It modifies virtio_scsi_pop_req() so that it is up to the callers to parse
the virtio scsi request. It seems that virtio_scsi_push_event() was not
modified accordingly...
This patch adds a call to virtio_scsi_parse_req(). It also drops some
sanity checks that are already performed by virtio_scsi_parse_req().
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Note that st*_raw and ld*_raw are effectively replaced by st*_p and ld*_p.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>
[ pass VirtIODevice * to memory accessors,
converted new tswap locations to virtio_tswap,
Greg Kurz <gkurz@linux.vnet.ibm.com> ]
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
Reviewed-by: Alexander Graf <agraf@suse.de>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
In order to migrate virtio subsections, they should be streamed after
the device itself. We need the device specific code to be called from
the common migration code to achieve this. This patch introduces load
and save methods for this purpose.
Suggested-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
Reviewed-by: Alexander Graf <agraf@suse.de>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
vhost userspace needn't to handle vq's notification from guest,
so define dummy handle_output callback for all vqs of vhost-scsi.
In some corner cases(such as when handling vq's reset from VM), virtio-pci
still trys to handle pending virtio-scsi events, then object check failure
inside virtio_scsi_handle_event() for vhost-scsi can be triggered.
The issue can be reproduced by 'rmmod virtio-scsi', 'system sleep' or reboot
inside VM.
Cc: qemu-stable@nongnu.org
Cc: Anthony Liguori <aliguori@amazon.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Store the request and response headers by value, and let
virtio_scsi_parse_req check that there is only one of datain
and dataout.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Retrieve sense and copy it to guest memory, to prepare for when we will use
qemu_iovec_from_buf.
Swap response and request, since we'll use the tail of VirtIOSCSIReq
for the CDB.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
- Introduce virtio_scsi_init_req and virtio_scsi_free_req
- rename qemu_sgl_init_external to qemu_sgl_concat
- move virtio_scsi_parse_req from virtio_scsi_pop_req to callers
and add header length checks to virtio_scsi_parse_req.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The ld_raw and st_raw definitions are only needed in code that
must compile for both user-mode and softmmu emulation. Device
models can use the equivalent ld_p/st_p which are simple
pointer accessors.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* remotes/bonzini/scsi-next:
megasas: remove buildtime strings
block: iscsi build fix if LIBISCSI_FEATURE_IOVECTOR is not defined
virtio-scsi: Plug memory leak on virtio_scsi_push_event() error path
scsi: Document intentional fall through in scsi_req_length()
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Spotted by Coverity.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
CVE-2013-4542
hw/scsi/scsi-bus.c invokes load_request.
virtio_scsi_load_request does:
qemu_get_buffer(f, (unsigned char *)&req->elem, sizeof(req->elem));
this probably can make elem invalid, for example,
make in_num or out_num huge, then:
virtio_scsi_parse_req(s, vs->cmd_vqs[n], req);
will do:
if (req->elem.out_num > 1) {
qemu_sgl_init_external(req, &req->elem.out_sg[1],
&req->elem.out_addr[1],
req->elem.out_num - 1);
} else {
qemu_sgl_init_external(req, &req->elem.in_sg[1],
&req->elem.in_addr[1],
req->elem.in_num - 1);
}
and this will access out of array bounds.
Note: this adds security checks within assert calls since
SCSIBusInfo's load_request cannot fail.
For now simply disable builds with NDEBUG - there seems
to be little value in supporting these.
Cc: Andreas Färber <afaerber@suse.de>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
In some cases, an unplug can cause events to be dropped, which
leads to an assertion failure when preparing to notify the guest
kernel.
Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
There is still a small window that occurs when a cancel I/O affects
an asynchronous I/O operation that hasn't started. In other words,
when the residual data length equals the expected data length.
Today, the routine virtio_scsi_command_complete fails because the
VirtIOSCSIReq pointer (from the hba_private field in SCSIRequest)
was cleared earlier when virtio_scsi_complete_req was called by
the virtio_scsi_request_cancelled routine. As a result, the
virtio_scsi_command_complete routine needs to simply return when
it is processing a SCSIRequest block that was marked canceled.
Signed-off-by: Eric Farman <farman@linux.vnet.ibm.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Rename qdev -> dev since that's what realize's argument is called by
convention. No need to keep more "qdev" around than necessary.
Avoid duplicate VIRTIO_DEVICE() cast.
Signed-off-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This ensures hot-unplug is handled properly by the proxy, and avoids
leaking bus_name which is freed by virtio_device_exit.
Cc: qemu-stable@nongnu.org
Acked-by: Andreas Faerber <afaerber@suse.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
It's the abstract base of virtio-scsi-device and vhost-scsi.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Fam Zheng
Jason Wang
Stefan Hajnoczi
Paolo Bonzini
Halil Pasic
Greg Kurz
Dr. David Alan Gilbert
Markus Armbruster
Max Reitz
Peter Maydell
Cao jin
Eugene (jno) Dvurechenski
Pavel Butsykin
Michael S. Tsirkin
Cornelia Huck
Shannon Zhao
Gerd Hoffmann
Bin Wu
Ting Wang
Igor Mammedov
Cédric Le Goater
Greg Kurz
Rusty Russell
Ming Lei
Eric Farman
Andreas Färber