diff mbox series

[v5] iommu/arm-smmu-qcom: Request direct mapping for modem device

Message ID 20200507192157.6831-1-sibis@codeaurora.org (mailing list archive)
State Superseded
Headers show
Series [v5] iommu/arm-smmu-qcom: Request direct mapping for modem device | expand

Commit Message

Sibi Sankar May 7, 2020, 7:21 p.m. UTC
The modem remote processor has two modes of access to the DDR, a direct
mode and through a SMMU which requires direct mapping. The configuration
of the modem SIDs is handled in TrustZone. On platforms where TrustZone
is absent this needs to be explicitly done from kernel. Add compatibles
for modem to opt in for direct mapping on such platforms.

Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
---

V5
 * Reword commit message and drop unnecessary details

 drivers/iommu/arm-smmu-qcom.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Stephen Boyd May 7, 2020, 9:14 p.m. UTC | #1
Quoting Sibi Sankar (2020-05-07 12:21:57)
> The modem remote processor has two modes of access to the DDR, a direct
> mode and through a SMMU which requires direct mapping. The configuration
> of the modem SIDs is handled in TrustZone.

Is it "The configuration of the modem SIDs is typically handled by
code running in the ARM CPU's secure mode, i.e. secure EL1"? And is that
even true? I though it was programmed by EL2.

> On platforms where TrustZone

TrustZone is always there.

> is absent this needs to be explicitly done from kernel. Add compatibles
> for modem to opt in for direct mapping on such platforms.
> 
> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>

Is Sai the author? Or does this need a co-developed-by tag?

> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> ---
> 
> V5
>  * Reword commit message and drop unnecessary details

I don't see any improvement! Probably because I don't understand _why_
the modem needs a direct mapping. The commit text basically says "we
need to do it because it isn't done in secure world sometimes". This is
probably wrong what I wrote below, but I'd like to clarify to confirm my
understanding. Maybe the commit text should say:

The modem remote processor has two access paths to DDR. One path is
directly connected to DDR and another path goes through an SMMU. The
SMMU path is configured to be a direct mapping because it's used by
various peripherals in the modem subsystem. Typically this direct
mapping is configured by programming modem SIDs into the SMMU when EL2
responds to a hyp call from the code that loads the modem binary in the
secure world.

In certain firmware configurations, especially when the kernel is
entered at EL2, we don't want secure mode to make hyp calls to program
the SMMU because the kernel is in full control of the SMMU. Let's add
compatibles here so that we can have the kernel program the SIDs for the
modem in these cases.
Sibi Sankar May 8, 2020, 5:23 p.m. UTC | #2
Hey Stephen,
Thanks for taking time to review the patch.

On 5/8/20 2:44 AM, Stephen Boyd wrote:
> Quoting Sibi Sankar (2020-05-07 12:21:57)
>> The modem remote processor has two modes of access to the DDR, a direct
>> mode and through a SMMU which requires direct mapping. The configuration
>> of the modem SIDs is handled in TrustZone.
> 
> Is it "The configuration of the modem SIDs is typically handled by
> code running in the ARM CPU's secure mode, i.e. secure EL1"? And is that
> even true? I though it was programmed by EL2.
What I meant to say was qcom implementation of TZ or qcom
proprietary bootloaders. I should have been more specific
and mentioned that the configuration is done at EL2 by QHEE
(Qualcomm's Hypervisor Execution Environment) before we enter
the kernel.

> 
>> On platforms where TrustZone
> 
> TrustZone is always there.
> 
>> is absent this needs to be explicitly done from kernel. Add compatibles
>> for modem to opt in for direct mapping on such platforms.
>>
>> Signed-off-by: Sai Prakash Ranjan <saiprakash.ranjan@codeaurora.org>
> 
> Is Sai the author? Or does this need a co-developed-by tag?

I decided to include Sai's S-b just to show I took back
ownership of the patch from his patch series. I'll drop
it in the next re-spin.

> 
>> Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>> ---
>>
>> V5
>>   * Reword commit message and drop unnecessary details
> 
> I don't see any improvement! Probably because I don't understand _why_
> the modem needs a direct mapping. The commit text basically says "we
> need to do it because it isn't done in secure world sometimes". This is
> probably wrong what I wrote below, but I'd like to clarify to confirm my
> understanding. Maybe the commit text should say:

Thanks for taking time to reword the commit message will use
the same with a few corrections.

> 
> The modem remote processor has two access paths to DDR. One path is
> directly connected to DDR and another path goes through an SMMU. The
> SMMU path is configured to be a direct mapping because it's used by
> various peripherals in the modem subsystem. 

I'll use ^^ as is.

> Typically this direct
> mapping is configured by programming modem SIDs into the SMMU when EL2
> responds to a hyp call from the code that loads the modem binary in the
> secure world.

Typically this direct mapping is configured statically at EL2
by QHEE (Qualcomm's Hypervisor Execution Environment) before
the kernel is entered.

> 
> In certain firmware configurations, especially when the kernel is
> entered at EL2, we don't want secure mode to make hyp calls to program
> the SMMU because the kernel is in full control of the SMMU. Let's add
> compatibles here so that we can have the kernel program the SIDs for the
> modem in these cases.

In certain firmware configuration, especially when the kernel is already 
in full control of the SMMU, defer programming the modem SIDs to the
kernel. Let's add compatibles here so that we can have the kernel
program the SIDs for the modem in these cases.

Will/Stephen,
Let me know if the above changes are okay? I'll re-spin the patch
based on your feedback.

>
diff mbox series

Patch

diff --git a/drivers/iommu/arm-smmu-qcom.c b/drivers/iommu/arm-smmu-qcom.c
index 5bedf21587a56..cf01d0215a397 100644
--- a/drivers/iommu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm-smmu-qcom.c
@@ -17,7 +17,9 @@  static const struct of_device_id qcom_smmu_client_of_match[] = {
 	{ .compatible = "qcom,mdp4" },
 	{ .compatible = "qcom,mdss" },
 	{ .compatible = "qcom,sc7180-mdss" },
+	{ .compatible = "qcom,sc7180-mss-pil" },
 	{ .compatible = "qcom,sdm845-mdss" },
+	{ .compatible = "qcom,sdm845-mss-pil" },
 	{ }
 };