From 9c67f33fcab654d6b7f9f4236c5a21ea946e0931 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Thu, 14 Sep 2023 10:00:59 -0400 Subject: virtio-blk: add lock to protect s->rq s->rq is accessed from IO_CODE and GLOBAL_STATE_CODE. Introduce a lock to protect s->rq and eliminate reliance on the AioContext lock. Signed-off-by: Stefan Hajnoczi Message-ID: <20230914140101.1065008-3-stefanha@redhat.com> Reviewed-by: Kevin Wolf Reviewed-by: Eric Blake Reviewed-by: Michael S. Tsirkin Signed-off-by: Kevin Wolf --- include/hw/virtio/virtio-blk.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'include/hw') diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index dafec43..9881009 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -54,7 +54,8 @@ struct VirtIOBlockReq; struct VirtIOBlock { VirtIODevice parent_obj; BlockBackend *blk; - void *rq; + QemuMutex rq_lock; + void *rq; /* protected by rq_lock */ VirtIOBlkConf conf; unsigned short sector_mask; bool original_wce; -- cgit v1.1 From eaad0fe26050c227dc5dad63205835bac4912a51 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 4 Dec 2023 11:42:56 -0500 Subject: scsi: only access SCSIDevice->requests from one thread Stop depending on the AioContext lock and instead access SCSIDevice->requests from only one thread at a time: - When the VM is running only the BlockBackend's AioContext may access the requests list. - When the VM is stopped only the main loop may access the requests list. These constraints protect the requests list without the need for locking in the I/O code path. Note that multiple IOThreads are not supported yet because the code assumes all SCSIRequests are executed from a single AioContext. Leave that as future work. Signed-off-by: Stefan Hajnoczi Reviewed-by: Eric Blake Message-ID: <20231204164259.1515217-2-stefanha@redhat.com> Signed-off-by: Kevin Wolf --- include/hw/scsi/scsi.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'include/hw') diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h index 3692ca8..10c4e82 100644 --- a/include/hw/scsi/scsi.h +++ b/include/hw/scsi/scsi.h @@ -69,14 +69,19 @@ struct SCSIDevice { DeviceState qdev; VMChangeStateEntry *vmsentry; - QEMUBH *bh; uint32_t id; BlockConf conf; SCSISense unit_attention; bool sense_is_ua; uint8_t sense[SCSI_SENSE_BUF_SIZE]; uint32_t sense_len; + + /* + * The requests list is only accessed from the AioContext that executes + * requests or from the main loop when IOThread processing is stopped. + */ QTAILQ_HEAD(, SCSIRequest) requests; + uint32_t channel; uint32_t lun; int blocksize; -- cgit v1.1 From ed18b1ed4f34888872b5fbc2f217c65d62c95cfd Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Tue, 5 Dec 2023 13:19:58 -0500 Subject: virtio-scsi: replace AioContext lock with tmf_bh_lock Protect the Task Management Function BH state with a lock. The TMF BH runs in the main loop thread. An IOThread might process a TMF at the same time as the TMF BH is running. Therefore tmf_bh_list and tmf_bh must be protected by a lock. Run TMF request completion in the IOThread using aio_wait_bh_oneshot(). This avoids more locking to protect the virtqueue and SCSI layer state. Signed-off-by: Stefan Hajnoczi Reviewed-by: Eric Blake Reviewed-by: Kevin Wolf Message-ID: <20231205182011.1976568-2-stefanha@redhat.com> Signed-off-by: Kevin Wolf --- include/hw/virtio/virtio-scsi.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'include/hw') diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h index 779568a..da8cb92 100644 --- a/include/hw/virtio/virtio-scsi.h +++ b/include/hw/virtio/virtio-scsi.h @@ -85,8 +85,9 @@ struct VirtIOSCSI { /* * TMFs deferred to main loop BH. These fields are protected by - * virtio_scsi_acquire(). + * tmf_bh_lock. */ + QemuMutex tmf_bh_lock; QEMUBH *tmf_bh; QTAILQ_HEAD(, VirtIOSCSIReq) tmf_bh_list; -- cgit v1.1 From 4f36b1384756914a6c4ffe04080a96da149c4c03 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Tue, 5 Dec 2023 13:20:05 -0500 Subject: scsi: remove AioContext locking The AioContext lock no longer has any effect. Remove it. Signed-off-by: Stefan Hajnoczi Reviewed-by: Eric Blake Message-ID: <20231205182011.1976568-9-stefanha@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- include/hw/virtio/virtio-scsi.h | 14 -------------- 1 file changed, 14 deletions(-) (limited to 'include/hw') diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h index da8cb92..7f0573b 100644 --- a/include/hw/virtio/virtio-scsi.h +++ b/include/hw/virtio/virtio-scsi.h @@ -101,20 +101,6 @@ struct VirtIOSCSI { uint32_t host_features; }; -static inline void virtio_scsi_acquire(VirtIOSCSI *s) -{ - if (s->ctx) { - aio_context_acquire(s->ctx); - } -} - -static inline void virtio_scsi_release(VirtIOSCSI *s) -{ - if (s->ctx) { - aio_context_release(s->ctx); - } -} - void virtio_scsi_common_realize(DeviceState *dev, VirtIOHandleOutput ctrl, VirtIOHandleOutput evt, -- cgit v1.1 From 350147a871a545ab56b4a1062c8485635d9ffc24 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 20 Dec 2023 08:47:52 -0500 Subject: qdev-properties: alias all object class properties qdev_alias_all_properties() aliases a DeviceState's qdev properties onto an Object. This is used for VirtioPCIProxy types so that --device virtio-blk-pci has properties of its embedded --device virtio-blk-device object. Currently this function is implemented using qdev properties. Change the function to use QOM object class properties instead. This works because qdev properties create QOM object class properties, but it also catches any QOM object class-only properties that have no qdev properties. This change ensures that properties of devices are shown with --device foo,\? even if they are QOM object class properties. Signed-off-by: Stefan Hajnoczi Message-ID: <20231220134755.814917-2-stefanha@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- include/hw/qdev-properties.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'include/hw') diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h index 25743a2..09aa04c 100644 --- a/include/hw/qdev-properties.h +++ b/include/hw/qdev-properties.h @@ -230,8 +230,8 @@ void qdev_property_add_static(DeviceState *dev, Property *prop); * @target: Device which has properties to be aliased * @source: Object to add alias properties to * - * Add alias properties to the @source object for all qdev properties on - * the @target DeviceState. + * Add alias properties to the @source object for all properties on the @target + * DeviceState. * * This is useful when @target is an internal implementation object * owned by @source, and you want to expose all the properties of that -- cgit v1.1 From cf03a152c5d749fd0083bfe540df9524f1d2ff1d Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 20 Dec 2023 08:47:54 -0500 Subject: qdev: add IOThreadVirtQueueMappingList property type virtio-blk and virtio-scsi devices will need a way to specify the mapping between IOThreads and virtqueues. At the moment all virtqueues are assigned to a single IOThread or the main loop. This single thread can be a CPU bottleneck, so it is necessary to allow finer-grained assignment to spread the load. Introduce DEFINE_PROP_IOTHREAD_VQ_MAPPING_LIST() so devices can take a parameter that maps virtqueues to IOThreads. The command-line syntax for this new property is as follows: --device '{"driver":"foo","iothread-vq-mapping":[{"iothread":"iothread0","vqs":[0,1,2]},...]}' IOThreads are specified by name and virtqueues are specified by 0-based index. It will be common to simply assign virtqueues round-robin across a set of IOThreads. A convenient syntax that does not require specifying individual virtqueue indices is available: --device '{"driver":"foo","iothread-vq-mapping":[{"iothread":"iothread0"},{"iothread":"iothread1"},...]}' Signed-off-by: Stefan Hajnoczi Message-ID: <20231220134755.814917-4-stefanha@redhat.com> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- include/hw/qdev-properties-system.h | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'include/hw') diff --git a/include/hw/qdev-properties-system.h b/include/hw/qdev-properties-system.h index 91f7a24..06c359c 100644 --- a/include/hw/qdev-properties-system.h +++ b/include/hw/qdev-properties-system.h @@ -24,6 +24,7 @@ extern const PropertyInfo qdev_prop_off_auto_pcibar; extern const PropertyInfo qdev_prop_pcie_link_speed; extern const PropertyInfo qdev_prop_pcie_link_width; extern const PropertyInfo qdev_prop_cpus390entitlement; +extern const PropertyInfo qdev_prop_iothread_vq_mapping_list; #define DEFINE_PROP_PCI_DEVFN(_n, _s, _f, _d) \ DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_pci_devfn, int32_t) @@ -82,4 +83,8 @@ extern const PropertyInfo qdev_prop_cpus390entitlement; DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_cpus390entitlement, \ CpuS390Entitlement) +#define DEFINE_PROP_IOTHREAD_VQ_MAPPING_LIST(_name, _state, _field) \ + DEFINE_PROP(_name, _state, _field, qdev_prop_iothread_vq_mapping_list, \ + IOThreadVirtQueueMappingList *) + #endif -- cgit v1.1 From b6948ab01df068bef591868c22d1f873d2d05cde Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 20 Dec 2023 08:47:55 -0500 Subject: virtio-blk: add iothread-vq-mapping parameter Add the iothread-vq-mapping parameter to assign virtqueues to IOThreads. Store the vq:AioContext mapping in the new struct VirtIOBlockDataPlane->vq_aio_context[] field and refactor the code to use the per-vq AioContext instead of the BlockDriverState's AioContext. Reimplement --device virtio-blk-pci,iothread= and non-IOThread mode by assigning all virtqueues to the IOThread and main loop's AioContext in vq_aio_context[], respectively. The comment in struct VirtIOBlockDataPlane about EventNotifiers is stale. Remove it. Signed-off-by: Stefan Hajnoczi Message-ID: <20231220134755.814917-5-stefanha@redhat.com> Signed-off-by: Kevin Wolf --- include/hw/virtio/virtio-blk.h | 2 ++ 1 file changed, 2 insertions(+) (limited to 'include/hw') diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index 9881009..5e4091e 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -21,6 +21,7 @@ #include "sysemu/block-backend.h" #include "sysemu/block-ram-registrar.h" #include "qom/object.h" +#include "qapi/qapi-types-virtio.h" #define TYPE_VIRTIO_BLK "virtio-blk-device" OBJECT_DECLARE_SIMPLE_TYPE(VirtIOBlock, VIRTIO_BLK) @@ -37,6 +38,7 @@ struct VirtIOBlkConf { BlockConf conf; IOThread *iothread; + IOThreadVirtQueueMappingList *iothread_vq_mapping_list; char *serial; uint32_t request_merging; uint16_t num_queues; -- cgit v1.1