Re: [PATCH 1/4] iommu/msm: Add iommu_group support

[Sricharan R <sricharan-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]

Hi Robin,

> As the last step to making groups mandatory, clean up the remaining
> drivers by adding basic support. Whilst it may not perfectly reflect the
> isolation capabilities of the hardware, using generic_device_group()
> should at least maintain existing behaviour with respect to the API.
> 
> Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
> ---
>  drivers/iommu/msm_iommu.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
> index d0448353d501..04f4d51ffacb 100644
> --- a/drivers/iommu/msm_iommu.c
> +++ b/drivers/iommu/msm_iommu.c
> @@ -393,6 +393,7 @@ static struct msm_iommu_dev *find_iommu_for_dev(struct device *dev)
>  static int msm_iommu_add_device(struct device *dev)
>  {
>  	struct msm_iommu_dev *iommu;
> +	struct iommu_group *group;
>  	unsigned long flags;
>  	int ret = 0;
>  
> @@ -406,7 +407,16 @@ static int msm_iommu_add_device(struct device *dev)
>  
>  	spin_unlock_irqrestore(&msm_iommu_lock, flags);
>  
> -	return ret;
> +	if (ret)
> +		return ret;
> +
> +	group = iommu_group_get_for_dev(dev);
> +	if (IS_ERR(group))
> +		return PTR_ERR(group);
> +
> +	iommu_group_put(group);
> +
> +	return 0;
>  }
>  

 While this is correct for completing the group support, this adds the default domain and
 that might break in the driver while attaching a private domain. The msm_iomm_attach_dev
 needs to be fixed by calling msm_iommu_detach_dev while trying to attach a new domain when
 already connected to a default one. But let me test and confirm this.

Regards,
 Sricharan

-- 
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus