Hi Robin,
On 7/24/2017 3:25 PM, Robin Murphy wrote:
> On 24/07/17 08:34, Sricharan R wrote:
>> 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@xxxxxxx>
>>> ---
>>> 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.
>
> The default domain shouldn't matter, since msm_iommu_domain_alloc() will
> refuse to create DMA ops or identity domains in the first place. In the
> absence of a default domain, __iommu_detach_group() will still end up
> calling ops->detach_dev, so I don't think the ultimate behaviour is any
> different with this change. Testing is of course welcome, though ;)
Ha, you are right. Sorry, overlooked that only DOMAIN_UNMANAGED is supported here.
Meanwhile, lost access to the board. So would give test feedback once i get it.
Otherwise, Reviewed-by: sricharan@xxxxxxxxxxxxxx
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
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
