Re: [PATCH v2 13/13] iommu/rockchip: Support sharing IOMMU between masters

From: JeffyChen
Date: Wed Jan 17 2018 - 08:32:25 EST


Hi Robin,

On 01/17/2018 09:00 PM, Robin Murphy wrote:
On 16/01/18 13:25, Jeffy Chen wrote:
There would be some masters sharing the same IOMMU device. Put them in
the same iommu group and share the same iommu domain.

Signed-off-by: Jeffy Chen <jeffy.chen@xxxxxxxxxxxxxx>
---

Changes in v2: None

drivers/iommu/rockchip-iommu.c | 39
+++++++++++++++++++++++++++++++--------
1 file changed, 31 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/rockchip-iommu.c
b/drivers/iommu/rockchip-iommu.c
index c8de1456a016..6c316dd0dd2d 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -100,11 +100,13 @@ struct rk_iommu {
struct iommu_device iommu;
struct list_head node; /* entry in rk_iommu_domain.iommus */
struct iommu_domain *domain; /* domain to which iommu is
attached */
+ struct iommu_group *group;
struct mutex pm_mutex; /* serializes power transitions */
};
struct rk_iommudata {
struct device_link *link; /* runtime PM link from IOMMU to
master */
+ struct iommu_domain *domain; /* domain to which device is
attached */

I don't see why this is needed - for example, mtk_iommu does the same
thing without needing to track the active domain in more than one place.

Fundamentally, for this kind of IOMMU without the notion of multiple
translation contexts, the logic should look like:

iommudrv_attach_device(dev, domain) {
iommu = dev_get_iommu(dev);
if (iommu->curr_domain != domain) {
update_hw_state(iommu, domain);
iommu->curr_domain = domain;
}
}

which I think is essentially what you have anyway. When a group contains
multiple devices, you update the IOMMU state for the first device, then
calls for subsequent devices in the group do nothing since they see the
IOMMU state is already up-to-date with the new domain.

right, will remove it.
Robin.