From 22fb6eb571387172f41878866f4438b6bae21f0e Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 7 Oct 2020 12:23:56 -0400 Subject: qom: fix objects with improper parent type Some objects accidentally inherit ObjectClass instead of Object. They compile silently but may crash after downcasting. In this patch, we introduce a coccinelle script to find broken declarations and fix them manually with proper base type. Signed-off-by: Sergey Nizovtsev Signed-off-by: Paolo Bonzini --- include/hw/acpi/vmgenid.h | 2 +- include/hw/misc/vmcoreinfo.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'include/hw') diff --git a/include/hw/acpi/vmgenid.h b/include/hw/acpi/vmgenid.h index d50fbac..cb4ad37 100644 --- a/include/hw/acpi/vmgenid.h +++ b/include/hw/acpi/vmgenid.h @@ -19,7 +19,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(VmGenIdState, VMGENID) struct VmGenIdState { - DeviceClass parent_obj; + DeviceState parent_obj; QemuUUID guid; /* The 128-bit GUID seen by the guest */ uint8_t vmgenid_addr_le[8]; /* Address of the GUID (little-endian) */ }; diff --git a/include/hw/misc/vmcoreinfo.h b/include/hw/misc/vmcoreinfo.h index ebada66..0b7b55d 100644 --- a/include/hw/misc/vmcoreinfo.h +++ b/include/hw/misc/vmcoreinfo.h @@ -24,7 +24,7 @@ DECLARE_INSTANCE_CHECKER(VMCoreInfoState, VMCOREINFO, typedef struct fw_cfg_vmcoreinfo FWCfgVMCoreInfo; struct VMCoreInfoState { - DeviceClass parent_obj; + DeviceState parent_obj; bool has_vmcoreinfo; FWCfgVMCoreInfo vmcoreinfo; -- cgit v1.1 From bb755ba47f3747251c0eadf681ee68b9033309b8 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 6 Oct 2020 15:38:55 +0300 Subject: qdev: add "check if address free" callback for buses Check if an address is free on the bus before plugging in the device. This makes it possible to do the check without any side effects, and to detect the problem early without having to do it in the realize callback. Signed-off-by: Paolo Bonzini Message-Id: <20201006123904.610658-5-mlevitsk@redhat.com> Signed-off-by: Paolo Bonzini --- include/hw/qdev-core.h | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'include/hw') diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 72064f4..14d476c 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -210,13 +210,24 @@ struct BusClass { /* FIXME first arg should be BusState */ void (*print_dev)(Monitor *mon, DeviceState *dev, int indent); char *(*get_dev_path)(DeviceState *dev); + /* * This callback is used to create Open Firmware device path in accordance * with OF spec http://forthworks.com/standards/of1275.pdf. Individual bus * bindings can be found at http://playground.sun.com/1275/bindings/. */ char *(*get_fw_dev_path)(DeviceState *dev); + void (*reset)(BusState *bus); + + /* + * Return whether the device can be added to @bus, + * based on the address that was set (via device properties) + * before realize. If not, on return @errp contains the + * human-readable error message. + */ + bool (*check_address)(BusState *bus, DeviceState *dev, Error **errp); + BusRealize realize; BusUnrealize unrealize; @@ -788,7 +799,7 @@ const char *qdev_fw_name(DeviceState *dev); Object *qdev_get_machine(void); /* FIXME: make this a link<> */ -void qdev_set_parent_bus(DeviceState *dev, BusState *bus); +bool qdev_set_parent_bus(DeviceState *dev, BusState *bus, Error **errp); extern bool qdev_hotplug; extern bool qdev_hot_removed; -- cgit v1.1 From 2d24a64661549732fc77f632928318dd52f5bce5 Mon Sep 17 00:00:00 2001 From: Maxim Levitsky Date: Tue, 6 Oct 2020 15:38:59 +0300 Subject: device-core: use RCU for list of children of a bus 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 Signed-off-by: Maxim Levitsky Reviewed-by: Stefan Hajnoczi 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 Message-Id: <20201006123904.610658-9-mlevitsk@redhat.com> Signed-off-by: Paolo Bonzini --- include/hw/qdev-core.h | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'include/hw') diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 14d476c..2c6307e 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -3,6 +3,8 @@ #include "qemu/queue.h" #include "qemu/bitmap.h" +#include "qemu/rcu.h" +#include "qemu/rcu_queue.h" #include "qom/object.h" #include "hw/hotplug.h" #include "hw/resettable.h" @@ -238,6 +240,7 @@ struct BusClass { }; typedef struct BusChild { + struct rcu_head rcu; DeviceState *child; int index; QTAILQ_ENTRY(BusChild) sibling; @@ -258,6 +261,12 @@ struct BusState { int max_index; bool realized; int num_children; + + /* + * children is a RCU QTAILQ, thus readers must use RCU to access it, + * and writers must hold the big qemu lock + */ + QTAILQ_HEAD(, BusChild) children; QLIST_ENTRY(BusState) sibling; ResettableState reset; -- cgit v1.1 From a23151e8cc8cc08546252dc9c7671171d9c44615 Mon Sep 17 00:00:00 2001 From: Maxim Levitsky Date: Tue, 6 Oct 2020 15:39:00 +0300 Subject: device-core: use atomic_set on .realized property Some code might race with placement of new devices on a bus. We currently first place a (unrealized) device on the bus and then realize it. As a workaround, users that scan the child device list, can check the realized property to see if it is safe to access such a device. Use an atomic write here too to aid with this. A separate discussion is what to do with devices that are unrealized: It looks like for this case we only call the hotplug handler's unplug callback and its up to it to unrealize the device. An atomic operation doesn't cause harm for this code path though. Signed-off-by: Maxim Levitsky Reviewed-by: Stefan Hajnoczi Message-Id: <20200913160259.32145-6-mlevitsk@redhat.com> Signed-off-by: Paolo Bonzini Message-Id: <20201006123904.610658-10-mlevitsk@redhat.com> Signed-off-by: Paolo Bonzini --- include/hw/qdev-core.h | 2 ++ 1 file changed, 2 insertions(+) (limited to 'include/hw') diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 2c6307e..8689733 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -163,6 +163,8 @@ struct NamedClockList { /** * DeviceState: * @realized: Indicates whether the device has been fully constructed. + * When accessed outsize big qemu lock, must be accessed with + * atomic_load_acquire() * @reset: ResettableState for the device; handled by Resettable interface. * * This structure should not be accessed directly. We declare it here -- cgit v1.1 From 8ff34495601067d02edb54b4346cace84ec4e1df Mon Sep 17 00:00:00 2001 From: Maxim Levitsky Date: Tue, 6 Oct 2020 15:39:02 +0300 Subject: scsi/scsi_bus: Add scsi_device_get Add scsi_device_get which finds the scsi device and takes a reference to it. Suggested-by: Stefan Hajnoczi Signed-off-by: Maxim Levitsky Message-Id: <20200913160259.32145-8-mlevitsk@redhat.com> Signed-off-by: Paolo Bonzini Message-Id: <20201006123904.610658-12-mlevitsk@redhat.com> Signed-off-by: Paolo Bonzini --- include/hw/scsi/scsi.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include/hw') diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h index 7a55cdb..09fa5c9 100644 --- a/include/hw/scsi/scsi.h +++ b/include/hw/scsi/scsi.h @@ -190,6 +190,7 @@ int scsi_device_get_sense(SCSIDevice *dev, uint8_t *buf, int len, bool fixed); int scsi_SG_IO_FROM_DEV(BlockBackend *blk, uint8_t *cmd, uint8_t cmd_size, uint8_t *buf, uint8_t buf_size); SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int target, int lun); +SCSIDevice *scsi_device_get(SCSIBus *bus, int channel, int target, int lun); /* scsi-generic.c. */ extern const SCSIReqOps scsi_generic_req_ops; -- cgit v1.1