File device-core-use-atomic_set-on-.realized-.patch of Package qemu.28164

Overview Repositories Revisions Requests Users Attributes Meta

File device-core-use-atomic_set-on-.realized-.patch of Package qemu.28164

From: Maxim Levitsky <mlevitsk@redhat.com>
Date: Tue, 6 Oct 2020 15:39:00 +0300
Subject: device-core: use atomic_set on .realized property

Git-commit: a23151e8cc8cc08546252dc9c7671171d9c44615
References: bsc#1184574

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 <mlevitsk@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20200913160259.32145-6-mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20201006123904.610658-10-mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Lin Ma <lma@suse.com>
---
 hw/core/qdev.c         | 19 ++++++++++++++++++-
 include/hw/qdev-core.h |  2 ++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 917f3f6ae2efbcf01c8ed65a3d34..d261c36e760db0cbabcda626d187 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -937,7 +937,25 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
             }
        }
 
+       atomic_store_release(&dev->realized, value);
+
     } else if (!value && dev->realized) {
+
+        /*
+         * Change the value so that any concurrent users are aware
+         * that the device is going to be unrealized
+         *
+         * TODO: change .realized property to enum that states
+         * each phase of the device realization/unrealization
+         */
+
+        atomic_set(&dev->realized, value);
+        /*
+         * Ensure that concurrent users see this update prior to
+         * any other changes done by unrealize.
+         */
+        smp_wmb();
+
         Error **local_errp = NULL;
         QLIST_FOREACH(bus, &dev->child_bus, sibling) {
             local_errp = local_err ? NULL : &local_err;
@@ -959,7 +977,6 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
         goto fail;
     }
 
-    dev->realized = value;
     return;
 
 child_realize_fail:
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index bcc0c572c5a4ed431219fd902ece..66d031683f461a5fa6b854057299 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -143,6 +143,8 @@ struct NamedGPIOList {
 /**
  * DeviceState:
  * @realized: Indicates whether the device has been fully constructed.
+ *            When accessed outsize big qemu lock, must be accessed with
+ *            atomic_load_acquire()
  *
  * This structure should not be accessed directly.  We declare it here
  * so that it can be embedded in individual device state structures.

Places

File device-core-use-atomic_set-on-.realized-.patch of Package qemu.28164

Places