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 <[email protected]> >>> --- >>> 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: [email protected] 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 _______________________________________________ iommu mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/iommu
