aboutsummaryrefslogtreecommitdiff
path: root/hw/scsi/virtio-scsi.c
AgeCommit message (Collapse)Author
2023-06-26virtio-scsi: avoid dangling host notifier in ->ioeventfd_stop()Stefan Hajnoczi
virtio_scsi_dataplane_stop() calls blk_drain_all(), which invokes ->drained_begin()/->drained_end() after we've already detached the host notifier. virtio_scsi_drained_end() currently attaches the host notifier again and leaves it dangling after dataplane has stopped. This results in the following assertion failure because virtio_scsi_defer_to_dataplane() is called from the IOThread instead of the main loop thread: qemu-system-x86_64: ../softmmu/memory.c:1111: memory_region_transaction_commit: Assertion `qemu_mutex_iothread_locked()' failed. Buglink: https://gitlab.com/qemu-project/qemu/-/issues/1680 Reported-by: Jean-Louis Dupond <jean-louis@dupond.be> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20230611193924.2444914-1-stefanha@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
2023-06-01block: add blk_io_plug_call() APIStefan Hajnoczi
Introduce a new API for thread-local blk_io_plug() that does not traverse the block graph. The goal is to make blk_io_plug() multi-queue friendly. Instead of having block drivers track whether or not we're in a plugged section, provide an API that allows them to defer a function call until we're unplugged: blk_io_plug_call(fn, opaque). If blk_io_plug_call() is called multiple times with the same fn/opaque pair, then fn() is only called once at the end of the function - resulting in batching. This patch introduces the API and changes blk_io_plug()/blk_io_unplug(). blk_io_plug()/blk_io_unplug() no longer require a BlockBackend argument because the plug state is now thread-local. Later patches convert block drivers to blk_io_plug_call() and then we can finally remove .bdrv_co_io_plug() once all block drivers have been converted. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> Acked-by: Kevin Wolf <kwolf@redhat.com> Message-id: 20230530180959.1108766-2-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2023-05-30virtio-scsi: implement BlockDevOps->drained_begin()Stefan Hajnoczi
The virtio-scsi Host Bus Adapter provides access to devices on a SCSI bus. Those SCSI devices typically have a BlockBackend. When the BlockBackend enters a drained section, the SCSI device must temporarily stop submitting new I/O requests. Implement this behavior by temporarily stopping virtio-scsi virtqueue processing when one of the SCSI devices enters a drained section. The new scsi_device_drained_begin() API allows scsi-disk to message the virtio-scsi HBA. scsi_device_drained_begin() uses a drain counter so that multiple SCSI devices can have overlapping drained sections. The HBA only sees one pair of .drained_begin/end() calls. After this commit, virtio-scsi no longer depends on hw/virtio's ioeventfd aio_set_event_notifier(is_external=true). This commit is a step towards removing the aio_disable_external() API. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20230516190238.8401-19-stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-05-30virtio-scsi: stop using aio_disable_external() during unplugStefan Hajnoczi
This patch is part of an effort to remove the aio_disable_external() API because it does not fit in a multi-queue block layer world where many AioContexts may be submitting requests to the same disk. The SCSI emulation code is already in good shape to stop using aio_disable_external(). It was only used by commit 9c5aad84da1c ("virtio-scsi: fixed virtio_scsi_ctx_check failed when detaching scsi disk") to ensure that virtio_scsi_hotunplug() works while the guest driver is submitting I/O. Ensure virtio_scsi_hotunplug() is safe as follows: 1. qdev_simple_device_unplug_cb() -> qdev_unrealize() -> device_set_realized() calls qatomic_set(&dev->realized, false) so that future scsi_device_get() calls return NULL because they exclude SCSIDevices with realized=false. That means virtio-scsi will reject new I/O requests to this SCSIDevice with VIRTIO_SCSI_S_BAD_TARGET even while virtio_scsi_hotunplug() is still executing. We are protected against new requests! 2. scsi_qdev_unrealize() already contains a call to scsi_device_purge_requests() so that in-flight requests are cancelled synchronously. This ensures that no in-flight requests remain once qdev_simple_device_unplug_cb() returns. Thanks to these two conditions we don't need aio_disable_external() anymore. Cc: Zhengui Li <lizhengui@huawei.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20230516190238.8401-5-stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-05-30virtio-scsi: avoid race between unplug and transport eventStefan Hajnoczi
Only report a transport reset event to the guest after the SCSIDevice has been unrealized by qdev_simple_device_unplug_cb(). qdev_simple_device_unplug_cb() sets the SCSIDevice's qdev.realized field to false so that scsi_device_find/get() no longer see it. scsi_target_emulate_report_luns() also needs to be updated to filter out SCSIDevices that are unrealized. Change virtio_scsi_push_event() to take event information as an argument instead of the SCSIDevice. This allows virtio_scsi_hotunplug() to emit a VIRTIO_SCSI_T_TRANSPORT_RESET event after the SCSIDevice has already been unrealized. These changes ensure that the guest driver does not see the SCSIDevice that's being unplugged if it responds very quickly to the transport reset event. Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20230516190238.8401-4-stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-02-23virtio-scsi: reset SCSI devices from main loop threadStefan Hajnoczi
When an IOThread is configured, the ctrl virtqueue is processed in the IOThread. TMFs that reset SCSI devices are currently called directly from the IOThread and trigger an assertion failure in blk_drain() from the following call stack: virtio_scsi_handle_ctrl_req -> virtio_scsi_do_tmf -> device_code_reset -> scsi_disk_reset -> scsi_device_purge_requests -> blk_drain ../block/block-backend.c:1780: void blk_drain(BlockBackend *): Assertion `qemu_in_main_thread()' failed. The blk_drain() function is not designed to be called from an IOThread because it needs the Big QEMU Lock (BQL). This patch defers TMFs that reset SCSI devices to a Bottom Half (BH) that runs in the main loop thread under the BQL. This way it's safe to call blk_drain() and the assertion failure is avoided. Introduce s->tmf_bh_list for tracking TMF requests that have been deferred to the BH. When the BH runs it will grab the entire list and process all requests. Care must be taken to clear the list when the virtio-scsi device is reset or unrealized. Otherwise deferred TMF requests could execute later and lead to use-after-free or other undefined behavior. The s->resetting counter that's used by TMFs that reset SCSI devices is accessed from multiple threads. This patch makes that explicit by using atomic accessor functions. With this patch applied the counter is only modified by the main loop thread under the BQL but can be read by any thread. Reported-by: Qing Wang <qinwang@redhat.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20230221212218.1378734-4-stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2023-01-08include/hw/pci: Break inclusion loop pci_bridge.h and cxl.hMarkus Armbruster
hw/pci/pci_bridge.h and hw/cxl/cxl.h include each other. Fortunately, breaking the loop is merely a matter of deleting unnecessary includes from headers, and adding them back in places where they are now missing. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20221222100330.380143-2-armbru@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
2022-10-18scsi: Use device_cold_reset() and bus_cold_reset()Peter Maydell
In the SCSI subsystem we currently use the legacy functions qdev_reset_all() and qbus_reset_all(). These perform a recursive reset, starting from either a qbus or a qdev. However they do not permit any of the devices in the tree to use three-phase reset, because device reset goes through the device_legacy_reset() function that only calls the single DeviceClass::reset method. Switch to using the device_cold_reset() and bus_cold_reset() functions. These also perform a recursive reset, where first the children are reset and then finally the parent, but they use the new (...in 2020...) Resettable mechanism, which supports both the old style single-reset method and also the new 3-phase reset handling. Since no devices attached to SCSI buses currently try to use 3-phase reset, this should be a no-behaviour-change commit which just reduces the use of a deprecated API. Commit created with: sed -i -e 's/qdev_reset_all/device_cold_reset/g;s/qbus_reset_all/bus_cold_reset/g' hw/scsi/*.c Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Message-Id: <20221013160623.1296109-2-peter.maydell@linaro.org> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2022-10-13virtio-scsi: Send "REPORTED LUNS CHANGED" sense data upon disk hotplug eventsVenu Busireddy
Section 5.6.6.3 of VirtIO specification states, "Events will also be reported via sense codes..." However, no sense data is sent when VIRTIO_SCSI_EVT_RESET_RESCAN or VIRTIO_SCSI_EVT_RESET_REMOVED events are reported (when disk hotplug/hotunplug events occur). SCSI layer on Solaris depends on this sense data, and hence does not handle disk hotplug/hotunplug events. When the disk inventory changes, use the bus unit attention mechanism to return a CHECK_CONDITION status with sense data of 0x06/0x3F/0x0E (sense code REPORTED_LUNS_CHANGED). The first device on the bus to execute a command successfully will report and consume the unit attention status. Signed-off-by: Venu Busireddy <venu.busireddy@oracle.com> Message-Id: <20221006194946.24134-1-venu.busireddy@oracle.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2022-09-01scsi: Add buf_len parameter to scsi_req_new()John Millikin
When a SCSI command is received from the guest, the CDB length implied by the first byte might exceed the number of bytes the guest sent. In this case scsi_req_new() will read uninitialized data, causing unpredictable behavior. Adds the buf_len parameter to scsi_req_new() and plumbs it through the call stack. Signed-off-by: John Millikin <john@john-millikin.com> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1127 Message-Id: <20220817053458.698416-1-john@john-millikin.com> [Fill in correct length for adapters other than ESP. - Paolo] Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2022-05-16virtio: drop name parameter for virtio_init()Jonah Palmer
This patch drops the name parameter for the virtio_init function. The pair between the numeric device ID and the string device ID (name) of a virtio device already exists, but not in a way that lets us map between them. This patch lets us do this and removes the need for the name parameter in the virtio_init function. Signed-off-by: Jonah Palmer <jonah.palmer@oracle.com> Message-Id: <1648819405-25696-2-git-send-email-jonah.palmer@oracle.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
2022-05-09virtio-scsi: move request-related items from .h to .cStefan Hajnoczi
There is no longer a need to expose the request and related APIs in virtio-scsi.h since there are no callers outside virtio-scsi.c. Note the block comment in VirtIOSCSIReq has been adjusted to meet the coding style. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Message-id: 20220427143541.119567-7-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2022-05-09virtio-scsi: clean up virtio_scsi_handle_cmd_vq()Stefan Hajnoczi
virtio_scsi_handle_cmd_vq() is only called from hw/scsi/virtio-scsi.c now and its return value is no longer used. Remove the function prototype from virtio-scsi.h and drop the return value. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Message-id: 20220427143541.119567-6-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2022-05-09virtio-scsi: clean up virtio_scsi_handle_ctrl_vq()Stefan Hajnoczi
virtio_scsi_handle_ctrl_vq() is only called from hw/scsi/virtio-scsi.c now and its return value is no longer used. Remove the function prototype from virtio-scsi.h and drop the return value. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Message-id: 20220427143541.119567-5-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2022-05-09virtio-scsi: clean up virtio_scsi_handle_event_vq()Stefan Hajnoczi
virtio_scsi_handle_event_vq() is only called from hw/scsi/virtio-scsi.c now and its return value is no longer used. Remove the function prototype from virtio-scsi.h and drop the return value. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Message-id: 20220427143541.119567-4-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2022-05-09virtio-scsi: fix ctrl and event handler functions in dataplane modeStefan Hajnoczi
Commit f34e8d8b8d48d73f36a67b6d5e492ef9784b5012 ("virtio-scsi: prepare virtio_scsi_handle_cmd for dataplane") prepared the virtio-scsi cmd virtqueue handler function to be used in both the dataplane and non-datpalane code paths. It failed to convert the ctrl and event virtqueue handler functions, which are not designed to be called from the dataplane code path but will be since the ioeventfd is set up for those virtqueues when dataplane starts. Convert the ctrl and event virtqueue handler functions now so they operate correctly when called from the dataplane code path. Avoid code duplication by extracting this code into a helper function. Fixes: f34e8d8b8d48d73f36a67b6d5e492ef9784b5012 ("virtio-scsi: prepare virtio_scsi_handle_cmd for dataplane") Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Message-id: 20220427143541.119567-2-stefanha@redhat.com [Fixed s/by used/be used/ typo pointed out by Michael Tokarev <mjt@tls.msk.ru>. --Stefan] Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2022-01-12virtio-scsi: prepare virtio_scsi_handle_cmd for dataplaneStefan Hajnoczi
Prepare virtio_scsi_handle_cmd() to be used by both dataplane and non-dataplane by making the condition for starting ioeventfd more specific. This way it won't trigger when dataplane has already been started. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> Message-id: 20211207132336.36627-5-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2021-09-30scsi: Replace scsi_bus_new() with scsi_bus_init(), scsi_bus_init_named()Peter Maydell
The function scsi_bus_new() creates a new SCSI bus; callers can either pass in a name argument to specify the name of the new bus, or they can pass in NULL to allow the bus to be given an automatically generated unique name. Almost all callers want to use the autogenerated name; the only exception is the virtio-scsi device. Taking a name argument that should almost always be NULL is an easy-to-misuse API design -- it encourages callers to think perhaps they should pass in some standard name like "scsi" or "scsi-bus". We don't do this anywhere for SCSI, but we do (incorrectly) do it for other bus types such as i2c. The function name also implies that it will return a newly allocated object, when it in fact does in-place allocation. We more commonly name such functions foo_init(), with foo_new() being the allocate-and-return variant. Replace all the scsi_bus_new() callsites with either: * scsi_bus_init() for the usual case where the caller wants an autogenerated bus name * scsi_bus_init_named() for the rare case where the caller needs to specify the bus name and document that for the _named() version it's then the caller's responsibility to think about uniqueness of bus names. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Acked-by: Paolo Bonzini <pbonzini@redhat.com> Message-id: 20210923121153.23754-2-peter.maydell@linaro.org
2021-03-06scsi: move host_status handling into SCSI driversHannes Reinecke
Some SCSI drivers like virtio have an internal mapping for the host_status. This patch moves the host_status translation into the SCSI drivers to allow those drivers to set up the correct values. Signed-off-by: Hannes Reinecke <hare@suse.de>. [Added default handling to avoid touching all drivers. - Paolo] Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2021-02-25scsi: drop 'result' argument from command_complete callbackHannes Reinecke
The command complete callback has a SCSIRequest as the first argument, and the status field of that structure is identical to the 'status' argument. So drop the argument from the callback. Signed-off-by: Hannes Reinecke <hare@suse.de> Message-Id: <20201116184041.60465-3-hare@suse.de> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2021-01-20block: Honor blk_set_aio_context() context requirementsSergio Lopez
The documentation for bdrv_set_aio_context_ignore() states this: * The caller must own the AioContext lock for the old AioContext of bs, but it * must not own the AioContext lock for new_context (unless new_context is the * same as the current context of bs). As blk_set_aio_context() makes use of this function, this rule also applies to it. Fix all occurrences where this rule wasn't honored. Suggested-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Sergio Lopez <slp@redhat.com> Message-Id: <20201214170519.223781-2-slp@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com>
2021-01-12virtio-scsi: trace eventsHannes Reinecke
Add trace events for virtio command and response tracing. Signed-off-by: Hannes Reinecke <hare@suse.de> Message-Id: <20201116183114.55703-2-hare@suse.de> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-10-12virtio-scsi: use scsi_device_getMaxim Levitsky
This will help us to avoid the scsi device disappearing after we took a reference to it. It doesn't by itself forbid case when we try to access an unrealized device Suggested-by: Stefan Hajnoczi <stefanha@gmail.com> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20200913160259.32145-9-mlevitsk@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20201006123904.610658-13-mlevitsk@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-10-12device-core: use RCU for list of children of a busMaxim Levitsky
This fixes the race between device emulation code that tries to find a child device to dispatch the request to (e.g a scsi disk), and hotplug of a new device to that bus. Note that this doesn't convert all the readers of the list but only these that might go over that list without BQL held. This is a very small first step to make this code thread safe. Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20200913160259.32145-5-mlevitsk@redhat.com> [Use RCU_READ_LOCK_GUARD in more places, adjust testcase now that the delay in DEVICE_DELETED due to RCU is more consistent. - Paolo] Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20201006123904.610658-9-mlevitsk@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-08-27virtio-scsi-pci: default num_queues to -smp NStefan Hajnoczi
Automatically size the number of virtio-scsi-pci, vhost-scsi-pci, and vhost-user-scsi-pci request virtqueues to match the number of vCPUs. Other transports continue to default to 1 request virtqueue. A 1:1 virtqueue:vCPU mapping ensures that completion interrupts are handled on the same vCPU that submitted the request. No IPI is necessary to complete an I/O request and performance is improved. The maximum number of MSI-X vectors and virtqueues limit are respected. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20200818143348.310613-6-stefanha@redhat.com> Reviewed-by: Cornelia Huck <cohuck@redhat.com> Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
2020-08-27virtio-scsi: introduce a constant for fixed virtqueuesStefan Hajnoczi
The event and control virtqueues are always present, regardless of the multi-queue configuration. Define a constant so that virtqueue number calculations are easier to read. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Cornelia Huck <cohuck@redhat.com> Reviewed-by: Pankaj Gupta <pankaj.gupta.linux@gmail.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com> Message-Id: <20200818143348.310613-5-stefanha@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
2020-07-02qdev: Drop qbus_set_hotplug_handler() parameter @errpMarkus Armbruster
qbus_set_hotplug_handler() is a simple wrapper around object_property_set_link(). object_property_set_link() fails when the property doesn't exist, is not settable, or its .check() method fails. These are all programming errors here, so passing &error_abort to qbus_set_hotplug_handler() is appropriate. Most of its callers do. Exceptions: * pcie_cap_slot_init(), shpc_init(), spapr_phb_realize() pass NULL, i.e. they ignore errors. * spapr_machine_init() passes &error_fatal. * s390_pcihost_realize(), virtio_serial_device_realize(), s390_pcihost_plug() pass the error to their callers. The latter two keep going after the error, which looks wrong. Drop the @errp parameter, and instead pass &error_abort to object_property_set_link(). Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: "Daniel P. Berrangé" <berrange@redhat.com> Cc: Eduardo Habkost <ehabkost@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20200630090351.1247703-15-armbru@redhat.com>
2020-05-15qdev: Unrealize must not failMarkus Armbruster
Devices may have component devices and buses. Device realization may fail. Realization is recursive: a device's realize() method realizes its components, and device_set_realized() realizes its buses (which should in turn realize the devices on that bus, except bus_set_realized() doesn't implement that, yet). When realization of a component or bus fails, we need to roll back: unrealize everything we realized so far. If any of these unrealizes failed, the device would be left in an inconsistent state. Must not happen. device_set_realized() lets it happen: it ignores errors in the roll back code starting at label child_realize_fail. Since realization is recursive, unrealization must be recursive, too. But how could a partly failed unrealize be rolled back? We'd have to re-realize, which can fail. This design is fundamentally broken. device_set_realized() does not roll back at all. Instead, it keeps unrealizing, ignoring further errors. It can screw up even for a device with no buses: if the lone dc->unrealize() fails, it still unregisters vmstate, and calls listeners' unrealize() callback. bus_set_realized() does not roll back either. Instead, it stops unrealizing. Fortunately, no unrealize method can fail, as we'll see below. To fix the design error, drop parameter @errp from all the unrealize methods. Any unrealize method that uses @errp now needs an update. This leads us to unrealize() methods that can fail. Merely passing it to another unrealize method cannot cause failure, though. Here are the ones that do other things with @errp: * virtio_serial_device_unrealize() Fails when qbus_set_hotplug_handler() fails, but still does all the other work. On failure, the device would stay realized with its resources completely gone. Oops. Can't happen, because qbus_set_hotplug_handler() can't actually fail here. Pass &error_abort to qbus_set_hotplug_handler() instead. * hw/ppc/spapr_drc.c's unrealize() Fails when object_property_del() fails, but all the other work is already done. On failure, the device would stay realized with its vmstate registration gone. Oops. Can't happen, because object_property_del() can't actually fail here. Pass &error_abort to object_property_del() instead. * spapr_phb_unrealize() Fails and bails out when remove_drcs() fails, but other work is already done. On failure, the device would stay realized with some of its resources gone. Oops. remove_drcs() fails only when chassis_from_bus()'s object_property_get_uint() fails, and it can't here. Pass &error_abort to remove_drcs() instead. Therefore, no unrealize method can fail before this patch. device_set_realized()'s recursive unrealization via bus uses object_property_set_bool(). Can't drop @errp there, so pass &error_abort. We similarly unrealize with object_property_set_bool() elsewhere, always ignoring errors. Pass &error_abort instead. Several unrealize methods no longer handle errors from other unrealize methods: virtio_9p_device_unrealize(), virtio_input_device_unrealize(), scsi_qdev_unrealize(), ... Much of the deleted error handling looks wrong anyway. One unrealize methods no longer ignore such errors: usb_ehci_pci_exit(). Several realize methods no longer ignore errors when rolling back: v9fs_device_realize_common(), pci_qdev_unrealize(), spapr_phb_realize(), usb_qdev_realize(), vfio_ccw_realize(), virtio_device_realize(). Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20200505152926.18877-17-armbru@redhat.com>
2020-02-22virtio: increase virtqueue size for virtio-scsi and virtio-blkDenis Plotnikov
The goal is to reduce the amount of requests issued by a guest on 1M reads/writes. This rises the performance up to 4% on that kind of disk access pattern. The maximum chunk size to be used for the guest disk accessing is limited with seg_max parameter, which represents the max amount of pices in the scatter-geather list in one guest disk request. Since seg_max is virqueue_size dependent, increasing the virtqueue size increases seg_max, which, in turn, increases the maximum size of data to be read/write from a guest disk. More details in the original problem statment: https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg03721.html Suggested-by: Denis V. Lunev <den@openvz.org> Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com> Message-id: 20200214074648.958-1-dplotnikov@virtuozzo.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
2020-01-24qdev: set properties with device_class_set_props()Marc-André Lureau
The following patch will need to handle properties registration during class_init time. Let's use a device_class_set_props() setter. spatch --macro-file scripts/cocci-macro-file.h --sp-file ./scripts/coccinelle/qdev-set-props.cocci --keep-comments --in-place --dir . @@ typedef DeviceClass; DeviceClass *d; expression val; @@ - d->props = val + device_class_set_props(d, val) Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-Id: <20200110153039.1379601-20-marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-01-24virtio-scsi: convert to new virtio_delete_queuePan Nengyuan
Use virtio_delete_queue to make it more clear. Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20200117075547.60864-3-pannengyuan@huawei.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-01-24virtio-scsi: delete vqs in unrealize to avoid memleaksPan Nengyuan
This patch fix memleaks when attaching/detaching virtio-scsi device, the memory leak stack is as follow: Direct leak of 21504 byte(s) in 3 object(s) allocated from: #0 0x7f491f2f2970 (/lib64/libasan.so.5+0xef970) ??:? #1 0x7f491e94649d (/lib64/libglib-2.0.so.0+0x5249d) ??:? #2 0x564d0f3919fa (./x86_64-softmmu/qemu-system-x86_64+0x2c3e9fa) /mnt/sdb/qemu/hw/virtio/virtio.c:2333 #3 0x564d0f2eca55 (./x86_64-softmmu/qemu-system-x86_64+0x2b99a55) /mnt/sdb/qemu/hw/scsi/virtio-scsi.c:912 #4 0x564d0f2ece7b (./x86_64-softmmu/qemu-system-x86_64+0x2b99e7b) /mnt/sdb/qemu/hw/scsi/virtio-scsi.c:924 #5 0x564d0f39ee47 (./x86_64-softmmu/qemu-system-x86_64+0x2c4be47) /mnt/sdb/qemu/hw/virtio/virtio.c:3531 #6 0x564d0f980224 (./x86_64-softmmu/qemu-system-x86_64+0x322d224) /mnt/sdb/qemu/hw/core/qdev.c:865 Reported-by: Euler Robot <euler.robot@huawei.com> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20200117075547.60864-2-pannengyuan@huawei.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2020-01-06virtio: make seg_max virtqueue size dependentDenis Plotnikov
Before the patch, seg_max parameter was immutable and hardcoded to 126 (128 - 2) without respect to queue size. This has two negative effects: 1. when queue size is < 128, we have Virtio 1.1 specfication violation: (2.6.5.3.1 Driver Requirements) seq_max must be <= queue_size. This violation affects the old Linux guests (ver < 4.14). These guests crash on these queue_size setups. 2. when queue_size > 128, as was pointed out by Denis Lunev <den@virtuozzo.com>, seg_max restrics guest's block request length which affects guests' performance making them issues more block request than needed. https://lists.gnu.org/archive/html/qemu-devel/2017-12/msg03721.html To mitigate this two effects, the patch adds the property adjusting seg_max to queue size automaticaly. Since seg_max is a guest visible parameter, the property is machine type managable and allows to choose between old (seg_max = 126 always) and new (seg_max = queue_size - 2) behaviors. Not to change the behavior of the older VMs, prevent setting the default seg_max_adjust value for older machine types. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com> Message-Id: <20191220140905.1718-2-dplotnikov@virtuozzo.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
2020-01-05virtio: don't enable notifications during pollingStefan Hajnoczi
Virtqueue notifications are not necessary during polling, so we disable them. This allows the guest driver to avoid MMIO vmexits. Unfortunately the virtio-blk and virtio-scsi handler functions re-enable notifications, defeating this optimization. Fix virtio-blk and virtio-scsi emulation so they leave notifications disabled. The key thing to remember for correctness is that polling always checks one last time after ending its loop, therefore it's safe to lose the race when re-enabling notifications at the end of polling. There is a measurable performance improvement of 5-10% with the null-co block driver. Real-life storage configurations will see a smaller improvement because the MMIO vmexit overhead contributes less to latency. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20191209210957.65087-1-stefanha@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
2019-10-25virtio: basic packed virtqueue supportJason Wang
This patch implements basic support for the packed virtqueue. Compare the split virtqueue which has three rings, packed virtqueue only have one which is supposed to have better cache utilization and more hardware friendly. Please refer virtio specification for more information. Signed-off-by: Wei Xu <wexu@redhat.com> Signed-off-by: Jason Wang <jasowang@redhat.com> Signed-off-by: Eugenio Pérez <eperezma@redhat.com> Message-Id: <20191025083527.30803-6-eperezma@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
2019-08-16Include hw/qdev-properties.h lessMarkus Armbruster
In my "build everything" tree, changing hw/qdev-properties.h triggers a recompile of some 2700 out of 6600 objects (not counting tests and objects that don't depend on qemu/osdep.h). Many places including hw/qdev-properties.h (directly or via hw/qdev.h) actually need only hw/qdev-core.h. Include hw/qdev-core.h there instead. hw/qdev.h is actually pointless: all it does is include hw/qdev-core.h and hw/qdev-properties.h, which in turn includes hw/qdev-core.h. Replace the remaining uses of hw/qdev.h by hw/qdev-properties.h. While there, delete a few superfluous inclusions of hw/qdev-core.h. Touching hw/qdev-properties.h now recompiles some 1200 objects. Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: "Daniel P. Berrangé" <berrange@redhat.com> Cc: Eduardo Habkost <ehabkost@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Message-Id: <20190812052359.30071-22-armbru@redhat.com>
2019-08-16Include migration/qemu-file-types.h a lot lessMarkus Armbruster
In my "build everything" tree, changing migration/qemu-file-types.h triggers a recompile of some 2600 out of 6600 objects (not counting tests and objects that don't depend on qemu/osdep.h). The culprit is again hw/hw.h, which supposedly includes it for convenience. Include migration/qemu-file-types.h only where it's needed. Touching it now recompiles less than 200 objects. Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20190812052359.30071-10-armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
2019-07-24virtio-scsi: fixed virtio_scsi_ctx_check failed when detaching scsi diskZhengui li
commit a6f230c move blockbackend back to main AioContext on unplug. It set the AioContext of SCSIDevice to the main AioContex, but s->ctx is still the iothread AioContex(if the scsi controller is configure with iothread). So if there are having in-flight requests during unplug, a failing assertion happend. The bt is below: (gdb) bt #0 0x0000ffff86aacbd0 in raise () from /lib64/libc.so.6 #1 0x0000ffff86aadf7c in abort () from /lib64/libc.so.6 #2 0x0000ffff86aa6124 in __assert_fail_base () from /lib64/libc.so.6 #3 0x0000ffff86aa61a4 in __assert_fail () from /lib64/libc.so.6 #4 0x0000000000529118 in virtio_scsi_ctx_check (d=<optimized out>, s=<optimized out>, s=<optimized out>) at /home/qemu-4.0.0/hw/scsi/virtio-scsi.c:246 #5 0x0000000000529ec4 in virtio_scsi_handle_cmd_req_prepare (s=0x2779ec00, req=0xffff740397d0) at /home/qemu-4.0.0/hw/scsi/virtio-scsi.c:559 #6 0x000000000052a228 in virtio_scsi_handle_cmd_vq (s=0x2779ec00, vq=0xffff7c6d7110) at /home/qemu-4.0.0/hw/scsi/virtio-scsi.c:603 #7 0x000000000052afa8 in virtio_scsi_data_plane_handle_cmd (vdev=<optimized out>, vq=0xffff7c6d7110) at /home/qemu-4.0.0/hw/scsi/virtio-scsi-dataplane.c:59 #8 0x000000000054d94c in virtio_queue_host_notifier_aio_poll (opaque=<optimized out>) at /home/qemu-4.0.0/hw/virtio/virtio.c:2452 assert(blk_get_aio_context(d->conf.blk) == s->ctx) failed. To avoid assertion failed, moving the "if" after qdev_simple_device_unplug_cb. In addition, to avoid another qemu crash below, add aio_disable_external before qdev_simple_device_unplug_cb, which disable the further processing of external clients when doing qdev_simple_device_unplug_cb. (gdb) bt #0 scsi_req_unref (req=0xffff6802c6f0) at hw/scsi/scsi-bus.c:1283 #1 0x00000000005294a4 in virtio_scsi_handle_cmd_req_submit (req=<optimized out>, s=<optimized out>) at /home/qemu-4.0.0/hw/scsi/virtio-scsi.c:589 #2 0x000000000052a2a8 in virtio_scsi_handle_cmd_vq (s=s@entry=0x9c90e90, vq=vq@entry=0xffff7c05f110) at /home/qemu-4.0.0/hw/scsi/virtio-scsi.c:625 #3 0x000000000052afd8 in virtio_scsi_data_plane_handle_cmd (vdev=<optimized out>, vq=0xffff7c05f110) at /home/qemu-4.0.0/hw/scsi/virtio-scsi-dataplane.c:60 #4 0x000000000054d97c in virtio_queue_host_notifier_aio_poll (opaque=<optimized out>) at /home/qemu-4.0.0/hw/virtio/virtio.c:2447 #5 0x00000000009b204c in run_poll_handlers_once (ctx=ctx@entry=0x6efea40, timeout=timeout@entry=0xffff7d7f7308) at util/aio-posix.c:521 #6 0x00000000009b2b64 in run_poll_handlers (ctx=ctx@entry=0x6efea40, max_ns=max_ns@entry=4000, timeout=timeout@entry=0xffff7d7f7308) at util/aio-posix.c:559 #7 0x00000000009b2ca0 in try_poll_mode (ctx=ctx@entry=0x6efea40, timeout=0xffff7d7f7308, timeout@entry=0xffff7d7f7348) at util/aio-posix.c:594 #8 0x00000000009b31b8 in aio_poll (ctx=0x6efea40, blocking=blocking@entry=true) at util/aio-posix.c:636 #9 0x00000000006973cc in iothread_run (opaque=0x6ebd800) at iothread.c:75 #10 0x00000000009b592c in qemu_thread_start (args=0x6efef60) at util/qemu-thread-posix.c:502 #11 0x0000ffff8057f8bc in start_thread () from /lib64/libpthread.so.0 #12 0x0000ffff804e5f8c in thread_start () from /lib64/libc.so.6 (gdb) p bus $1 = (SCSIBus *) 0x0 Signed-off-by: Zhengui li <lizhengui@huawei.com> Message-Id: <1563696502-7972-1-git-send-email-lizhengui@huawei.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <1563829520-17525-1-git-send-email-pbonzini@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2019-07-19virtio-scsi: remove unused argument to virtio_scsi_common_realizePaolo Bonzini
The argument is not used and passing it clutters error propagation in the callers. So, get rid of it. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2019-06-12Include qemu/module.h where needed, drop it from qemu-common.hMarkus Armbruster
Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20190523143508.25387-4-armbru@redhat.com> [Rebased with conflicts resolved automatically, except for hw/usb/dev-hub.c hw/misc/exynos4210_rng.c hw/misc/bcm2835_rng.c hw/misc/aspeed_scu.c hw/display/virtio-vga.c hw/arm/stm32f205_soc.c; ui/cocoa.m fixed up]
2019-06-06hw/scsi: Use the QOM BUS() macro to access BusState.qbusPhilippe Mathieu-Daudé
Rather than looking inside the definition of a BusState with "s->bus.qbus", use the QOM prefered style: "BUS(&s->bus)". This patch was generated using the following Coccinelle script: // Use BUS() macros to access BusState.qbus @use_bus_macro_to_access_qbus@ expression obj; identifier bus; @@ -&obj->bus.qbus +BUS(&obj->bus) Suggested-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Laurent Vivier <laurent@vivier.eu> Message-Id: <20190528164020.32250-3-philmd@redhat.com> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
2019-06-04scsi-disk: Use qdev_prop_drive_iothreadKevin Wolf
This makes use of qdev_prop_drive_iothread for scsi-disk so that the disk can be attached to a node that is already in the target AioContext. We need to check that the HBA actually supports iothreads, otherwise scsi-disk must make sure that the node is already in the main AioContext. This changes the error message for conflicting iothread settings. Previously, virtio-scsi produced the error message, now it comes from blk_set_aio_context(). Update a test case accordingly. Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2019-06-04block: Add Error to blk_set_aio_context()Kevin Wolf
Add an Error parameter to blk_set_aio_context() and use bdrv_child_try_set_aio_context() internally to check whether all involved nodes can actually support the AioContext switch. Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2019-03-11virtio-scsi: Fix build with gcc 9Greg Kurz
Build fails with gcc 9: CC ppc64-softmmu/hw/scsi/virtio-scsi.o hw/scsi/virtio-scsi.c: In function ‘virtio_scsi_do_tmf’: hw/scsi/virtio-scsi.c:265:39: error: taking address of packed member of ‘struct virtio_scsi_ctrl_tmf_req’ may result in an unaligned pointer value [-Werror=address-of-packed-member] 265 | virtio_tswap32s(VIRTIO_DEVICE(s), &req->req.tmf.subtype); | ^~~~~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors All the fields in struct virtio_scsi_ctrl_tmf_req are naturally aligned, so we could in theory drop QEMU_PACKED. Unfortunately, the header file is imported from linux which already has the packed attribute. Trying to fix that in the update-linux-headers.sh script is likely to produce ugliness. Turn the call to virtio_tswap32s() into an assignment instead. Signed-off-by: Greg Kurz <groug@kaod.org> Message-Id: <155137678223.44753.5438092367451176318.stgit@bahia.lan> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2019-02-17qdev: pass an Object * to qbus_set_hotplug_handler()Michael Roth
Certain devices types, like memory/CPU, are now being handled using a hotplug interface provided by a top-level MachineClass. Hotpluggable host bridges are another such device where it makes sense to use a machine-level hotplug handler. However, unlike those devices, host-bridges have a parent bus (the main system bus), and devices with a parent bus use a different mechanism for registering their hotplug handlers: qbus_set_hotplug_handler(). This interface currently expects a handler to be a subclass of DeviceClass, but this is not the case for MachineClass, which derives directly from ObjectClass. Internally, the interface only requires an ObjectClass, so expose that in qbus_set_hotplug_handler(). Cc: Michael S. Tsirkin <mst@redhat.com> Cc: Eduardo Habkost <ehabkost@redhat.com> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com> Signed-off-by: Greg Kurz <groug@kaod.org> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Cornelia Huck <cohuck@redhat.com> Acked-by: Halil Pasic <pasic@linux.ibm.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Message-Id: <154999589921.690774.3640149277362188566.stgit@bahia.lan> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
2019-02-01virtio-scsi: Forbid devices with different iothreads sharing a blockdevAlberto Garcia
This patch forbids attaching a disk to a SCSI device if its using a different AioContext. Test case included. Signed-off-by: Alberto Garcia <berto@igalia.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2019-02-01virtio-scsi: Move BlockBackend back to the main AioContext on unplugAlberto Garcia
This fixes a crash when attaching a disk to a SCSI device using iothreads, then detaching it and reattaching it again. Test case included. Signed-off-by: Alberto Garcia <berto@igalia.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2018-10-19call HotplugHandler->plug() as the last step in device realizationIgor Mammedov
When [2] was fixed it was agreed that adding and calling post_plug() callback after device_reset() was low risk approach to hotfix issue right before release. So it was merged instead of moving already existing plug() callback after device_reset() is called which would be more risky and require all plug() callbacks audit. Looking at the current plug() callbacks, it doesn't seem that moving plug() callback after device_reset() is breaking anything, so here goes agreed upon [3] proper fix which essentially reverts [1][2] and moves plug() callback after device_reset(). This way devices always comes to plug() stage, after it's been fully initialized (including being reset), which fixes race condition [2] without need for an extra post_plug() callback. 1. (25e897881 "qdev: add HotplugHandler->post_plug() callback") 2. (8449bcf94 "virtio-scsi: fix hotplug ->reset() vs event race") 3. https://www.mail-archive.com/qemu-devel@nongnu.org/msg549915.html Signed-off-by: Igor Mammedov <imammedo@redhat.com> Message-Id: <1539696820-273275-1-git-send-email-imammedo@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Tested-by: Pierre Morel<pmorel@linux.ibm.com> Acked-by: Pierre Morel<pmorel@linux.ibm.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2018-07-16virtio-scsi: fix hotplug ->reset() vs event raceStefan Hajnoczi
There is a race condition during hotplug when iothread is used. It occurs because virtio-scsi may be processing command queues in the iothread while the monitor performs SCSI device hotplug. When a SCSI device is hotplugged the HotplugHandler->plug() callback is invoked and virtio-scsi emits a rescan event to the guest. If the guest submits a SCSI command at this point then it may be cancelled before hotplug completes. This happens because ->reset() is called by hw/core/qdev.c:device_set_realized() after HotplugHandler->plug() has been called and hw/scsi/scsi-disk.c:scsi_disk_reset() purges all requests. This patch uses the new HotplugHandler->post_plug() callback to emit the rescan event after ->reset(). This eliminates the race conditions where requests could be cancelled. Reported-by: l00284672 <lizhengui@huawei.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Fam Zheng <famz@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20180716083732.3347-3-stefanha@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2017-09-19scsi: move block/scsi.h to include/scsi/constants.hPaolo Bonzini
Complete the transition by renaming this header, which was shared by block/iscsi.c and the SCSI emulation code. Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>