Re: [PATCH 2/2] iommu/arm-smmu: Allow client devices to select direct mapping

From: Sai Prakash Ranjan
Date: Thu Apr 16 2020 - 13:44:12 EST


Hi Robin,

On 2020-04-16 22:47, Robin Murphy wrote:
On 2020-04-16 5:23 pm, Sai Prakash Ranjan wrote:
Hi Robin,

On 2020-04-16 19:28, Robin Murphy wrote:
On 2020-01-22 11:48 am, Sai Prakash Ranjan wrote:
From: Jordan Crouse <jcrouse@xxxxxxxxxxxxxx>

Some client devices want to directly map the IOMMU themselves instead
of using the DMA domain. Allow those devices to opt in to direct
mapping by way of a list of compatible strings.

Signed-off-by: Jordan Crouse <jcrouse@xxxxxxxxxxxxxx>
Co-developed-by: Sai Prakash Ranjan <saiprakash.ranjan@xxxxxxxxxxxxxx>
Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@xxxxxxxxxxxxxx>
---
 drivers/iommu/arm-smmu-qcom.c | 39 +++++++++++++++++++++++++++++++++++
 drivers/iommu/arm-smmu.c | 3 +++
 drivers/iommu/arm-smmu.h | 5 +++++
 3 files changed, 47 insertions(+)

diff --git a/drivers/iommu/arm-smmu-qcom.c b/drivers/iommu/arm-smmu-qcom.c
index 64a4ab270ab7..ff746acd1c81 100644
--- a/drivers/iommu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm-smmu-qcom.c
@@ -3,6 +3,7 @@
ÂÂ * Copyright (c) 2019, The Linux Foundation. All rights reserved.
ÂÂ */
 +#include <linux/of_device.h>
 #include <linux/qcom_scm.h>
ÂÂÂ #include "arm-smmu.h"
@@ -11,6 +12,43 @@ struct qcom_smmu {
ÂÂÂÂÂ struct arm_smmu_device smmu;
 };
 +static const struct arm_smmu_client_match_data qcom_adreno = {
+ÂÂÂ .direct_mapping = true,
+};
+
+static const struct arm_smmu_client_match_data qcom_mdss = {
+ÂÂÂ .direct_mapping = true,
+};

Might it make sense to group these by the desired SMMU behaviour
rather than (apparently) what kind of device the client happens to be,
which seems like a completely arbitrary distinction from the SMMU
driver's PoV?


Sorry, I did not get the "grouping by the desired SMMU behaviour" thing.
Could you please give some more details?

I mean this pattern:

device_a_data {
.thing = this;
}

device_b_data {
.thing = this;
}

device_c_data {
.thing = that;
}

match[] = {
{ .compatible = "A", .data = &device_a_data },
{ .compatible = "B", .data = &device_b_data },
{ .compatible = "C", .data = &device_c_data },
};

...vs. this pattern:

do_this {
.thing = this;
}

do_that {
.thing = that;
}

match[] = {
{ .compatible = "A", .data = &do_this },
{ .compatible = "B", .data = &do_this },
{ .compatible = "C", .data = &do_that },
};

From the perspective of the thing doing the thing, grouping the data
by device is meaningless if all that matters is whether to do this or
that. The second pattern expresses that more naturally.

Of course if every device turns out to need a unique combination of
several behaviours, then there ends up being no practical difference
except that it's probably easier to come up with nice names under the
first pattern. I guess it's up to how you see this developing in
future; whether you're likely to need fine-grained per-device control,
or don't expect it to go much beyond domain type.


Thanks for explaining *this thing* :)
I will update the patch to follow the 2nd pattern as it makes more sense
to do_this or do_that directly. I'm not expecting anything other than
domain type atleast for now but hey we can always add the functionality
if the need arises.

Thanks,
Sai

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