Re: [PATCH 4/5] iommu/exynos: Add default_domain check in iommu_attach_device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Robin,


On 2016-11-24 13:25, Robin Murphy wrote:
Hi Marek,

On 24/11/16 11:20, Marek Szyprowski wrote:
This patch adds default_domain check before calling
exynos_iommu_detach_device. This path was intended only to cope with
default domains, which are automatically attached by the iommu core, so
return error if user tries to attach device, which is already attached
to other (non-default) domain.
I think this only applies to the current state of 32-bit ARM, where the
initial allocation of a default domain fails without CONFIG_IOMMU_DMA.
Since the device has a group, iommu_attach_device() will end up calling
into __iommu_attach_group(), which does this check before calling the
driver's .attach_dev:

	if (group->default_domain && group->domain != group->default_domain)
		return -EBUSY;

which if group->default_domain is non-NULL would make the new check
below redundant unless owner->domain can change independently of
dev->iommu_group->domain, but that sounds like it would be a bigger
problem anyway.

Thanks for your review. Default domains are used on ARM64 and they are
automatically attached by IOMMU core. This was the reason for looking into
this. But you are right, that there is no need for the additional check, as
this is already handled in the iommu core.

Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
---
  drivers/iommu/exynos-iommu.c | 11 ++++++++++-
  1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 426b1534d4d3..63d9358a6d9c 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -859,8 +859,17 @@ static int exynos_iommu_attach_device(struct iommu_domain *iommu_domain,
  	if (!has_sysmmu(dev))
  		return -ENODEV;
- if (owner->domain)
+	if (owner->domain) {
+		struct iommu_group *group = iommu_group_get(dev);
+
+		if (!group ||
+		    owner->domain != iommu_group_default_domain(group)) {
Similarly, I think this prevents reattaching to a default domain from
iommu_detach_device(), since __iommu_detach_group() won't call
.detach_dev first if default_domain is non-NULL, therefore owner->domain
is presumably still pointing to the outgoing non-default domain.

Right, I forgot about reattaching to the default domain. Please drop this patch,
the previous code was correct.


+			iommu_group_put(group);
+			return -EINVAL;
+		}
+		iommu_group_put(group);
  		exynos_iommu_detach_device(owner->domain, dev);
+	}
mutex_lock(&owner->rpm_lock);


Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland

--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux SoC Development]     [Linux Rockchip Development]     [Linux USB Development]     [Video for Linux]     [Linux Audio Users]     [Linux SCSI]     [Yosemite News]

  Powered by Linux