|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [XEN][PATCH v12 14/20] common/device_tree: Add rwlock for dt_host
On Tue, 5 Sep 2023, Vikram Garhwal wrote:
> 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>
Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> ---
> Changes from v11:
> Update check state >= active for checking dt_host_lock to be locked
> 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..075fb25a37 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
>
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |