Re: [XEN][PATCH v11 09/20] xen/iommu: Move spin_lock from iommu_dt_device_is_assigned to caller

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 31 Aug 2023, Vikram Garhwal wrote:
> Rename iommu_dt_device_is_assigned() to iommu_dt_device_is_assigned_locked().
> 
> Moving spin_lock to caller was done to prevent the concurrent access to
> iommu_dt_device_is_assigned while doing add/remove/assign/deassign. Follow-up
> patches in this series introduces node add/remove feature.
> 
> Signed-off-by: Vikram Garhwal <vikram.garhwal@xxxxxxx>
> Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>

Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>



> ---
> Changes from v10:
>     Move spin_lock before iommu_dt_device_is_assigned_locked() call.
>     Rebase the patch with latest Xen.
> Changes from v9:
>     Make iommu_dt_device_is_assigned_locked() static and delete header.
>     Move dtdevs_lock before iommu_dt_device_is_assigned_locked().
> Changes from v7:
>     Update commit message.
>     Add ASSERT().
> ---
> ---
>  xen/drivers/passthrough/device_tree.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/device_tree.c 
> b/xen/drivers/passthrough/device_tree.c
> index 3c0322c797..a7d86a20a0 100644
> --- a/xen/drivers/passthrough/device_tree.c
> +++ b/xen/drivers/passthrough/device_tree.c
> @@ -83,16 +83,16 @@ fail:
>      return rc;
>  }
>  
> -static bool iommu_dt_device_is_assigned(const struct dt_device_node *dev)
> +static bool iommu_dt_device_is_assigned_locked(const struct dt_device_node 
> *dev)
>  {
>      bool assigned = false;
>  
> +    ASSERT(spin_is_locked(&dtdevs_lock));
> +
>      if ( !dt_device_is_protected(dev) )
>          return 0;
>  
> -    spin_lock(&dtdevs_lock);
>      assigned = !list_empty(&dev->domain_list);
> -    spin_unlock(&dtdevs_lock);
>  
>      return assigned;
>  }
> @@ -225,12 +225,16 @@ int iommu_do_dt_domctl(struct xen_domctl *domctl, 
> struct domain *d,
>  
>          if ( domctl->cmd == XEN_DOMCTL_test_assign_device )
>          {
> -            if ( iommu_dt_device_is_assigned(dev) )
> +            spin_lock(&dtdevs_lock);
> +
> +            if ( iommu_dt_device_is_assigned_locked(dev) )
>              {
>                  printk(XENLOG_G_ERR "%s already assigned.\n",
>                         dt_node_full_name(dev));
>                  ret = -EINVAL;
>              }
> +
> +            spin_unlock(&dtdevs_lock);
>              break;
>          }
>  
> -- 
> 2.17.1
>