[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[XEN][PATCH v11 14/20] common/device_tree: Add rwlock for dt_host


  • To: <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Vikram Garhwal <vikram.garhwal@xxxxxxx>
  • Date: Thu, 31 Aug 2023 21:59:41 -0700
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=lists.xenproject.org smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=jHjqTJ1UaUiiJx0UNTFIv0uoiG+uui8ndyNCkHPJ/Yg=; b=hllnZopp2EQJWo2615T2uaFsDmL5eLMT7FrWXxWQBt6ZoTrTtsGT5LVIqhF2BqBfKHrY449+NnU6mvt9fdKIcsvhHLRo5hInDiHyHpZJJ9tY7wJxxwaijyj3DOHe025XUcJSrs1rNKyCCWJXYsatK+n4hwGh4B7kDXVti+1XXoMZOCOLmXLG5H9hX40VkAFWXszhElZ723hNwRFbf3hxyScVI/lUrXREaN2zo0BRJGd8M8v7k7C3LXqAH/RW23JwXVc+4k/ZVpauemyVyf5vkw9pd1KgLodOPKW0TQPpNIseslpMtovYFfsjwZDDBp35KtZBZ1ld7SkXSuo4QxPt/w==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=gfZo7KZhLvVDthkBZjr4z6Is4q0UFGKOKCsNb7xrK4Fwae0YZVdxoxMLJfLpo8ZJGLS1mh5Li4Pg9uScT1XDX6lncr8SKwR4W97xjcDX8h9C4Ye4iZVv+IBmyGFk1fiCnYXbBDIIhGrKaSZOxzJy3BQB6dlwDX7GuYVSYBgSSF+WdYoZFLyvPURP1DS2zHUHK0uGOeDQ+WTf+Dg7sxxsPZAYPy0WMccra136hmHqTTRmnRYY7CTjidR0Kxgyhvw2a6Xj1mgfzaFal+bsf8z5SEihMhps2x7HoWP70dD0Fx3/TJSLovsHbMuethM4SnFQErbB+nO0fSxwOIVnOZYsCg==
  • Cc: <vikram.garhwal@xxxxxxx>, <julien@xxxxxxx>, <michal.orzel@xxxxxxx>, <sstabellini@xxxxxxxxxx>
  • Delivery-date: Fri, 01 Sep 2023 05:11:28 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Dynamic programming ops will modify the dt_host and there might be other
functions which are browsing the dt_host at the same time. To avoid the race
conditions, adding rwlock for browsing the dt_host during runtime. dt_host
writer will be added in the follow-up patch for device tree overlay
functionalities.

Reason behind adding rwlock instead of spinlock:
    For now, dynamic programming is the sole modifier of dt_host in Xen during
    run time. All other access functions like iommu_release_dt_device() are
    just reading the dt_host during run-time. So, there is a need to protect
    others from browsing the dt_host while dynamic programming is modifying
    it. rwlock is better suitable for this task as spinlock won't be able to
    differentiate between read and write access.

Signed-off-by: Vikram Garhwal <vikram.garhwal@xxxxxxx>
Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>
---
Changes from v10:
    Add ASSERT for iommu_assign_dt_device() and iommu_add_dt_device().
Changes from v9:
    Update commit message and fix indentation.
    Add ASSERT() for iommu_deassign_dt_device() and iommu_remove_dt_device().
    Fix code styles.
    Remove rwlock_init in unflatten_device_tree() and do DEFINE_RWLOCK in
        device-tree.c
Changes from v7:
    Keep one lock for dt_host instead of lock for each node under dt_host.
---
---
 xen/common/device_tree.c              |  1 +
 xen/drivers/passthrough/device_tree.c | 28 +++++++++++++++++++++++++--
 xen/include/xen/device_tree.h         |  7 +++++++
 3 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
index f38f51ec0b..b1c2952951 100644
--- a/xen/common/device_tree.c
+++ b/xen/common/device_tree.c
@@ -31,6 +31,7 @@ dt_irq_xlate_func dt_irq_xlate;
 struct dt_device_node *dt_host;
 /* Interrupt controller node*/
 const struct dt_device_node *dt_interrupt_controller;
+DEFINE_RWLOCK(dt_host_lock);
 
 /**
  * struct dt_alias_prop - Alias property in 'aliases' node
diff --git a/xen/drivers/passthrough/device_tree.c 
b/xen/drivers/passthrough/device_tree.c
index 80f6efc606..1f9cfccf95 100644
--- a/xen/drivers/passthrough/device_tree.c
+++ b/xen/drivers/passthrough/device_tree.c
@@ -31,6 +31,8 @@ int iommu_assign_dt_device(struct domain *d, struct 
dt_device_node *dev)
     int rc = -EBUSY;
     struct domain_iommu *hd = dom_iommu(d);
 
+    ASSERT(system_state <= SYS_STATE_active || rw_is_locked(&dt_host_lock));
+
     if ( !is_iommu_enabled(d) )
         return -EINVAL;
 
@@ -62,6 +64,8 @@ int iommu_deassign_dt_device(struct domain *d, struct 
dt_device_node *dev)
     const struct domain_iommu *hd = dom_iommu(d);
     int rc;
 
+    ASSERT(rw_is_locked(&dt_host_lock));
+
     if ( !is_iommu_enabled(d) )
         return -EINVAL;
 
@@ -113,6 +117,8 @@ int iommu_release_dt_devices(struct domain *d)
     if ( !is_iommu_enabled(d) )
         return 0;
 
+    read_lock(&dt_host_lock);
+
     list_for_each_entry_safe(dev, _dev, &hd->dt_devices, domain_list)
     {
         rc = iommu_deassign_dt_device(d, dev);
@@ -120,10 +126,14 @@ int iommu_release_dt_devices(struct domain *d)
         {
             dprintk(XENLOG_ERR, "Failed to deassign %s in domain %u\n",
                     dt_node_full_name(dev), d->domain_id);
+            read_unlock(&dt_host_lock);
+
             return rc;
         }
     }
 
+    read_unlock(&dt_host_lock);
+
     return 0;
 }
 
@@ -133,6 +143,8 @@ int iommu_remove_dt_device(struct dt_device_node *np)
     struct device *dev = dt_to_dev(np);
     int rc;
 
+    ASSERT(rw_is_locked(&dt_host_lock));
+
     if ( !iommu_enabled )
         return 1;
 
@@ -177,6 +189,8 @@ int iommu_add_dt_device(struct dt_device_node *np)
     struct device *dev = dt_to_dev(np);
     int rc = 1, index = 0;
 
+    ASSERT(system_state <= SYS_STATE_active || rw_is_locked(&dt_host_lock));
+
     if ( !iommu_enabled )
         return 1;
 
@@ -249,6 +263,8 @@ int iommu_do_dt_domctl(struct xen_domctl *domctl, struct 
domain *d,
     int ret;
     struct dt_device_node *dev;
 
+    read_lock(&dt_host_lock);
+
     switch ( domctl->cmd )
     {
     case XEN_DOMCTL_assign_device:
@@ -289,7 +305,10 @@ int iommu_do_dt_domctl(struct xen_domctl *domctl, struct 
domain *d,
         }
 
         if ( d == dom_io )
-            return -EINVAL;
+        {
+            ret = -EINVAL;
+            break;
+        }
 
         ret = iommu_add_dt_device(dev);
         if ( ret < 0 )
@@ -327,7 +346,10 @@ int iommu_do_dt_domctl(struct xen_domctl *domctl, struct 
domain *d,
             break;
 
         if ( d == dom_io )
-            return -EINVAL;
+        {
+            ret = -EINVAL;
+            break;
+        }
 
         ret = iommu_deassign_dt_device(d, dev);
 
@@ -342,5 +364,7 @@ int iommu_do_dt_domctl(struct xen_domctl *domctl, struct 
domain *d,
         break;
     }
 
+    read_unlock(&dt_host_lock);
+
     return ret;
 }
diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
index 44d315c8ba..a262bba2ed 100644
--- a/xen/include/xen/device_tree.h
+++ b/xen/include/xen/device_tree.h
@@ -18,6 +18,7 @@
 #include <xen/string.h>
 #include <xen/types.h>
 #include <xen/list.h>
+#include <xen/rwlock.h>
 
 #define DEVICE_TREE_MAX_DEPTH 16
 
@@ -218,6 +219,12 @@ extern struct dt_device_node *dt_host;
  */
 extern const struct dt_device_node *dt_interrupt_controller;
 
+/*
+ * Lock that protects r/w updates to unflattened device tree i.e. dt_host 
during
+ * runtime. Lock may not be taken for boot only code.
+ */
+extern rwlock_t dt_host_lock;
+
 /**
  * Find the interrupt controller
  * For the moment we handle only one interrupt controller: the first
-- 
2.17.1




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.