Re: [PATCH 2/6] arm64: dts: qcom: sm8250: add apps_smmu node
On Mon 25 May 04:53 PDT 2020, Jonathan Marek wrote: [..] > I guess the commit message is ambiguous, that's not what I meant. Is "Now > that the kernel initializes the iommu, the bypass mappings set by the > bootloader are cleared. Adding the iommus property is required so that new > mappings are created for UFS." better? > This looks better, but it's actually not a bypass mapping that we inherit from the bootloader, it's the stream mapping pointing to a disabled (~ARM_SMMU_SCTLR_M) context bank. So when we wipe the stream mappings we will fault on the unmatched stream - which secure world "handles" for us... As such, I think you should replace "bypass" with "stream". Regards, Bjorn
Re: [PATCH 2/6] arm64: dts: qcom: sm8250: add apps_smmu node
On 2020-05-25 17:23, Jonathan Marek wrote:
On 5/25/20 7:40 AM, Sai Prakash Ranjan wrote:
On 2020-05-25 16:57, Jonathan Marek wrote:
On 5/25/20 7:17 AM, Sai Prakash Ranjan wrote:
Hi,
On 2020-05-25 16:38, Jonathan Marek wrote:
On 5/25/20 6:54 AM, Sai Prakash Ranjan wrote:
On 2020-05-25 15:39, Jonathan Marek wrote:
Hi,
On 5/25/20 5:42 AM, Sai Prakash Ranjan wrote:
Hi Jonathan,
On 2020-05-24 08:08, Jonathan Marek wrote:
Add the apps_smmu node for sm8250. Note that adding the iommus
field for
UFS is required because initializing the iommu removes the
bypass mapping
that created by the bootloader.
This statement doesn't seem right, you can just say since the
bypass is disabled
by default now, we need to add this property to enable
translation and avoid global faults.
If I use this patch [1] then the UFS iommu property isn't needed.
In
downstream, the identity (bypass?) stream mapping is inherited
from
the bootloader, and UFS is used without any iommu property.
Setting
ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n doesn't make it work on its
own
(without the UFS iommu property), so there's more to it than just
"bypass is disabled by default now".
https://patchwork.kernel.org/patch/11310757/
"iommus" property is not about inheriting stream mapping from
bootloader,
it is used to enable SMMU address translation for the
corresponding
master when specified. So when you have disabled bypass, i.e.,
ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y or via cmdline
"arm-smmu.disable_bypass=1"
and iommus property with SID and mask is not specified, then it
will result
in SMMU global faults.
Downstream has bypass
enabled(ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n),so you
won't see any global faults if you do not have iommus property.
Patch in your link is for display because of the usecase for
splash screen
on android and some other devices where the bootloader will
configure SMMU,
it has not yet merged and not likely to get merged in the current
state.
So yes "there is *not* much more to it than bypass is disabled by
default now"
and you have to specify "iommus" for the master devices or you
should enable bypass,
i.e., ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n or
arm-smmu.disable_bypass=n
Try without the patch in the link and without iommus for UFS and
ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y and you will see.
-Sai
I know that the "iommus" property is not about inheriting stream
mapping. Probing the iommu removes the stream mapping created by
the
bootloader, the iommus property is added so that new mappings are
created to replace what was removed.
You seem to be under the impression that the SM8150/SM8250
bootloader
does not configure SMMU. It does, for both UFS and SDHC, just like
it
does for display/splash screen on some devices.
It could be that bootloader does configure SMMU for UFS and SDHC,
but the
upstream SMMU driver doesnt allow to inherit stream mapping from the
bootloader
yet, so adding iommus property based on the assumption that it is
inherited seems
wrong.
I never said adding the iommus property is for inheriting stream
mapping. I mentioned inheriting to say UFS works without the iommus
property on downstream (it inherits a identity/bypass mapping).
Your commit description says "adding the iommus field for UFS is
required
because initializing the iommu removes the bypass mapping that created
by the
bootloader". So here it would mean like iommus property for UFS is not
for
enabling address translation by SMMU for UFS but to avoid removing
mappings
created by the bootloader which is not exactly what iommus property is
for.
I guess the commit message is ambiguous, that's not what I meant. Is
"Now that the kernel initializes the iommu, the bypass mappings set by
the bootloader are cleared. Adding the iommus property is required so
that new mappings are created for UFS." better?
Yes looks good.
With either value of ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT, it will
not
work without the iommus property.
I'm pretty sure that if you have
ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n and
without iommus, it should work.
It doesn't work, with either ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n or
ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y.
Ok since you are very sure about this, I will try with your patches on
SM8150 MTP tomorrow since I do not have access to one now.
Also just to make sure, please remove all the extra SMMU patches you
have
in your tree which are not yet merged or from downstream kernel.
FYI, I have a branch [1] integrating patches for upstream. All the
patches up to 34fff8a519cc075933 ("arm64: dts: qcom: add sm8250 GPU
nodes") have been submitted (and the patches after that are
unnecessary for testing on sm8150).
[1] https://github.com/flto/linux/commits/sm8x50-upstream
Thanks, I will test this out.
-Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 2/6] arm64: dts: qcom: sm8250: add apps_smmu node
On 5/25/20 7:40 AM, Sai Prakash Ranjan wrote:
On 2020-05-25 16:57, Jonathan Marek wrote:
On 5/25/20 7:17 AM, Sai Prakash Ranjan wrote:
Hi,
On 2020-05-25 16:38, Jonathan Marek wrote:
On 5/25/20 6:54 AM, Sai Prakash Ranjan wrote:
On 2020-05-25 15:39, Jonathan Marek wrote:
Hi,
On 5/25/20 5:42 AM, Sai Prakash Ranjan wrote:
Hi Jonathan,
On 2020-05-24 08:08, Jonathan Marek wrote:
Add the apps_smmu node for sm8250. Note that adding the iommus
field for
UFS is required because initializing the iommu removes the
bypass mapping
that created by the bootloader.
This statement doesn't seem right, you can just say since the
bypass is disabled
by default now, we need to add this property to enable
translation and avoid global faults.
If I use this patch [1] then the UFS iommu property isn't needed. In
downstream, the identity (bypass?) stream mapping is inherited from
the bootloader, and UFS is used without any iommu property. Setting
ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n doesn't make it work on its own
(without the UFS iommu property), so there's more to it than just
"bypass is disabled by default now".
https://patchwork.kernel.org/patch/11310757/
"iommus" property is not about inheriting stream mapping from
bootloader,
it is used to enable SMMU address translation for the corresponding
master when specified. So when you have disabled bypass, i.e.,
ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y or via cmdline
"arm-smmu.disable_bypass=1"
and iommus property with SID and mask is not specified, then it
will result
in SMMU global faults.
Downstream has bypass
enabled(ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n),so you
won't see any global faults if you do not have iommus property.
Patch in your link is for display because of the usecase for splash
screen
on android and some other devices where the bootloader will
configure SMMU,
it has not yet merged and not likely to get merged in the current
state.
So yes "there is *not* much more to it than bypass is disabled by
default now"
and you have to specify "iommus" for the master devices or you
should enable bypass,
i.e., ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n or
arm-smmu.disable_bypass=n
Try without the patch in the link and without iommus for UFS and
ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y and you will see.
-Sai
I know that the "iommus" property is not about inheriting stream
mapping. Probing the iommu removes the stream mapping created by the
bootloader, the iommus property is added so that new mappings are
created to replace what was removed.
You seem to be under the impression that the SM8150/SM8250 bootloader
does not configure SMMU. It does, for both UFS and SDHC, just like it
does for display/splash screen on some devices.
It could be that bootloader does configure SMMU for UFS and SDHC, but
the
upstream SMMU driver doesnt allow to inherit stream mapping from the
bootloader
yet, so adding iommus property based on the assumption that it is
inherited seems
wrong.
I never said adding the iommus property is for inheriting stream
mapping. I mentioned inheriting to say UFS works without the iommus
property on downstream (it inherits a identity/bypass mapping).
Your commit description says "adding the iommus field for UFS is required
because initializing the iommu removes the bypass mapping that created
by the
bootloader". So here it would mean like iommus property for UFS is not for
enabling address translation by SMMU for UFS but to avoid removing mappings
created by the bootloader which is not exactly what iommus property is for.
I guess the commit message is ambiguous, that's not what I meant. Is
"Now that the kernel initializes the iommu, the bypass mappings set by
the bootloader are cleared. Adding the iommus property is required so
that new mappings are created for UFS." better?
With either value of ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT, it will not
work without the iommus property.
I'm pretty sure that if you have ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n
and
without iommus, it should work.
It doesn't work, with either ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n or
ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y.
Ok since you are very sure about this, I will try with your patches on
SM8150 MTP tomorrow since I do not have access to one now.
Also just to make sure, please remove all the extra SMMU patches you have
in your tree which are not yet merged or from downstream kernel.
FYI, I have a branch [1] integrating patches for upstream. All the
patches up to 34fff8a519cc075933 ("arm64: dts: qcom: add sm8250 GPU
nodes") have been submitted (and the patches after that are unnecessary
for testing on sm8150).
[1] https://github.com/flto/linux/commits/sm8x50-upstream
-Sai
Re: [PATCH 2/6] arm64: dts: qcom: sm8250: add apps_smmu node
On 2020-05-25 16:57, Jonathan Marek wrote: On 5/25/20 7:17 AM, Sai Prakash Ranjan wrote: Hi, On 2020-05-25 16:38, Jonathan Marek wrote: On 5/25/20 6:54 AM, Sai Prakash Ranjan wrote: On 2020-05-25 15:39, Jonathan Marek wrote: Hi, On 5/25/20 5:42 AM, Sai Prakash Ranjan wrote: Hi Jonathan, On 2020-05-24 08:08, Jonathan Marek wrote: Add the apps_smmu node for sm8250. Note that adding the iommus field for UFS is required because initializing the iommu removes the bypass mapping that created by the bootloader. This statement doesn't seem right, you can just say since the bypass is disabled by default now, we need to add this property to enable translation and avoid global faults. If I use this patch [1] then the UFS iommu property isn't needed. In downstream, the identity (bypass?) stream mapping is inherited from the bootloader, and UFS is used without any iommu property. Setting ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n doesn't make it work on its own (without the UFS iommu property), so there's more to it than just "bypass is disabled by default now". https://patchwork.kernel.org/patch/11310757/ "iommus" property is not about inheriting stream mapping from bootloader, it is used to enable SMMU address translation for the corresponding master when specified. So when you have disabled bypass, i.e., ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y or via cmdline "arm-smmu.disable_bypass=1" and iommus property with SID and mask is not specified, then it will result in SMMU global faults. Downstream has bypass enabled(ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n),so you won't see any global faults if you do not have iommus property. Patch in your link is for display because of the usecase for splash screen on android and some other devices where the bootloader will configure SMMU, it has not yet merged and not likely to get merged in the current state. So yes "there is *not* much more to it than bypass is disabled by default now" and you have to specify "iommus" for the master devices or you should enable bypass, i.e., ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n or arm-smmu.disable_bypass=n Try without the patch in the link and without iommus for UFS and ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y and you will see. -Sai I know that the "iommus" property is not about inheriting stream mapping. Probing the iommu removes the stream mapping created by the bootloader, the iommus property is added so that new mappings are created to replace what was removed. You seem to be under the impression that the SM8150/SM8250 bootloader does not configure SMMU. It does, for both UFS and SDHC, just like it does for display/splash screen on some devices. It could be that bootloader does configure SMMU for UFS and SDHC, but the upstream SMMU driver doesnt allow to inherit stream mapping from the bootloader yet, so adding iommus property based on the assumption that it is inherited seems wrong. I never said adding the iommus property is for inheriting stream mapping. I mentioned inheriting to say UFS works without the iommus property on downstream (it inherits a identity/bypass mapping). Your commit description says "adding the iommus field for UFS is required because initializing the iommu removes the bypass mapping that created by the bootloader". So here it would mean like iommus property for UFS is not for enabling address translation by SMMU for UFS but to avoid removing mappings created by the bootloader which is not exactly what iommus property is for. With either value of ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT, it will not work without the iommus property. I'm pretty sure that if you have ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n and without iommus, it should work. It doesn't work, with either ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n or ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y. Ok since you are very sure about this, I will try with your patches on SM8150 MTP tomorrow since I do not have access to one now. Also just to make sure, please remove all the extra SMMU patches you have in your tree which are not yet merged or from downstream kernel. -Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 2/6] arm64: dts: qcom: sm8250: add apps_smmu node
On 5/25/20 7:17 AM, Sai Prakash Ranjan wrote: Hi, On 2020-05-25 16:38, Jonathan Marek wrote: On 5/25/20 6:54 AM, Sai Prakash Ranjan wrote: On 2020-05-25 15:39, Jonathan Marek wrote: Hi, On 5/25/20 5:42 AM, Sai Prakash Ranjan wrote: Hi Jonathan, On 2020-05-24 08:08, Jonathan Marek wrote: Add the apps_smmu node for sm8250. Note that adding the iommus field for UFS is required because initializing the iommu removes the bypass mapping that created by the bootloader. This statement doesn't seem right, you can just say since the bypass is disabled by default now, we need to add this property to enable translation and avoid global faults. If I use this patch [1] then the UFS iommu property isn't needed. In downstream, the identity (bypass?) stream mapping is inherited from the bootloader, and UFS is used without any iommu property. Setting ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n doesn't make it work on its own (without the UFS iommu property), so there's more to it than just "bypass is disabled by default now". https://patchwork.kernel.org/patch/11310757/ "iommus" property is not about inheriting stream mapping from bootloader, it is used to enable SMMU address translation for the corresponding master when specified. So when you have disabled bypass, i.e., ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y or via cmdline "arm-smmu.disable_bypass=1" and iommus property with SID and mask is not specified, then it will result in SMMU global faults. Downstream has bypass enabled(ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n),so you won't see any global faults if you do not have iommus property. Patch in your link is for display because of the usecase for splash screen on android and some other devices where the bootloader will configure SMMU, it has not yet merged and not likely to get merged in the current state. So yes "there is *not* much more to it than bypass is disabled by default now" and you have to specify "iommus" for the master devices or you should enable bypass, i.e., ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n or arm-smmu.disable_bypass=n Try without the patch in the link and without iommus for UFS and ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y and you will see. -Sai I know that the "iommus" property is not about inheriting stream mapping. Probing the iommu removes the stream mapping created by the bootloader, the iommus property is added so that new mappings are created to replace what was removed. You seem to be under the impression that the SM8150/SM8250 bootloader does not configure SMMU. It does, for both UFS and SDHC, just like it does for display/splash screen on some devices. It could be that bootloader does configure SMMU for UFS and SDHC, but the upstream SMMU driver doesnt allow to inherit stream mapping from the bootloader yet, so adding iommus property based on the assumption that it is inherited seems wrong. I never said adding the iommus property is for inheriting stream mapping. I mentioned inheriting to say UFS works without the iommus property on downstream (it inherits a identity/bypass mapping). With either value of ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT, it will not work without the iommus property. I'm pretty sure that if you have ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n and without iommus, it should work. It doesn't work, with either ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n or ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y. -Sai
Re: [PATCH 2/6] arm64: dts: qcom: sm8250: add apps_smmu node
Hi, On 2020-05-25 16:38, Jonathan Marek wrote: On 5/25/20 6:54 AM, Sai Prakash Ranjan wrote: On 2020-05-25 15:39, Jonathan Marek wrote: Hi, On 5/25/20 5:42 AM, Sai Prakash Ranjan wrote: Hi Jonathan, On 2020-05-24 08:08, Jonathan Marek wrote: Add the apps_smmu node for sm8250. Note that adding the iommus field for UFS is required because initializing the iommu removes the bypass mapping that created by the bootloader. This statement doesn't seem right, you can just say since the bypass is disabled by default now, we need to add this property to enable translation and avoid global faults. If I use this patch [1] then the UFS iommu property isn't needed. In downstream, the identity (bypass?) stream mapping is inherited from the bootloader, and UFS is used without any iommu property. Setting ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n doesn't make it work on its own (without the UFS iommu property), so there's more to it than just "bypass is disabled by default now". https://patchwork.kernel.org/patch/11310757/ "iommus" property is not about inheriting stream mapping from bootloader, it is used to enable SMMU address translation for the corresponding master when specified. So when you have disabled bypass, i.e., ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y or via cmdline "arm-smmu.disable_bypass=1" and iommus property with SID and mask is not specified, then it will result in SMMU global faults. Downstream has bypass enabled(ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n),so you won't see any global faults if you do not have iommus property. Patch in your link is for display because of the usecase for splash screen on android and some other devices where the bootloader will configure SMMU, it has not yet merged and not likely to get merged in the current state. So yes "there is *not* much more to it than bypass is disabled by default now" and you have to specify "iommus" for the master devices or you should enable bypass, i.e., ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n or arm-smmu.disable_bypass=n Try without the patch in the link and without iommus for UFS and ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y and you will see. -Sai I know that the "iommus" property is not about inheriting stream mapping. Probing the iommu removes the stream mapping created by the bootloader, the iommus property is added so that new mappings are created to replace what was removed. You seem to be under the impression that the SM8150/SM8250 bootloader does not configure SMMU. It does, for both UFS and SDHC, just like it does for display/splash screen on some devices. It could be that bootloader does configure SMMU for UFS and SDHC, but the upstream SMMU driver doesnt allow to inherit stream mapping from the bootloader yet, so adding iommus property based on the assumption that it is inherited seems wrong. With either value of ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT, it will not work without the iommus property. I'm pretty sure that if you have ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n and without iommus, it should work. -Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 2/6] arm64: dts: qcom: sm8250: add apps_smmu node
On 5/25/20 6:54 AM, Sai Prakash Ranjan wrote: On 2020-05-25 15:39, Jonathan Marek wrote: Hi, On 5/25/20 5:42 AM, Sai Prakash Ranjan wrote: Hi Jonathan, On 2020-05-24 08:08, Jonathan Marek wrote: Add the apps_smmu node for sm8250. Note that adding the iommus field for UFS is required because initializing the iommu removes the bypass mapping that created by the bootloader. This statement doesn't seem right, you can just say since the bypass is disabled by default now, we need to add this property to enable translation and avoid global faults. If I use this patch [1] then the UFS iommu property isn't needed. In downstream, the identity (bypass?) stream mapping is inherited from the bootloader, and UFS is used without any iommu property. Setting ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n doesn't make it work on its own (without the UFS iommu property), so there's more to it than just "bypass is disabled by default now". https://patchwork.kernel.org/patch/11310757/ "iommus" property is not about inheriting stream mapping from bootloader, it is used to enable SMMU address translation for the corresponding master when specified. So when you have disabled bypass, i.e., ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y or via cmdline "arm-smmu.disable_bypass=1" and iommus property with SID and mask is not specified, then it will result in SMMU global faults. Downstream has bypass enabled(ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n),so you won't see any global faults if you do not have iommus property. Patch in your link is for display because of the usecase for splash screen on android and some other devices where the bootloader will configure SMMU, it has not yet merged and not likely to get merged in the current state. So yes "there is *not* much more to it than bypass is disabled by default now" and you have to specify "iommus" for the master devices or you should enable bypass, i.e., ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n or arm-smmu.disable_bypass=n Try without the patch in the link and without iommus for UFS and ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y and you will see. -Sai I know that the "iommus" property is not about inheriting stream mapping. Probing the iommu removes the stream mapping created by the bootloader, the iommus property is added so that new mappings are created to replace what was removed. You seem to be under the impression that the SM8150/SM8250 bootloader does not configure SMMU. It does, for both UFS and SDHC, just like it does for display/splash screen on some devices. With either value of ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT, it will not work without the iommus property.
Re: [PATCH 2/6] arm64: dts: qcom: sm8250: add apps_smmu node
On 2020-05-25 15:39, Jonathan Marek wrote: Hi, On 5/25/20 5:42 AM, Sai Prakash Ranjan wrote: Hi Jonathan, On 2020-05-24 08:08, Jonathan Marek wrote: Add the apps_smmu node for sm8250. Note that adding the iommus field for UFS is required because initializing the iommu removes the bypass mapping that created by the bootloader. This statement doesn't seem right, you can just say since the bypass is disabled by default now, we need to add this property to enable translation and avoid global faults. If I use this patch [1] then the UFS iommu property isn't needed. In downstream, the identity (bypass?) stream mapping is inherited from the bootloader, and UFS is used without any iommu property. Setting ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n doesn't make it work on its own (without the UFS iommu property), so there's more to it than just "bypass is disabled by default now". https://patchwork.kernel.org/patch/11310757/ "iommus" property is not about inheriting stream mapping from bootloader, it is used to enable SMMU address translation for the corresponding master when specified. So when you have disabled bypass, i.e., ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y or via cmdline "arm-smmu.disable_bypass=1" and iommus property with SID and mask is not specified, then it will result in SMMU global faults. Downstream has bypass enabled(ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n),so you won't see any global faults if you do not have iommus property. Patch in your link is for display because of the usecase for splash screen on android and some other devices where the bootloader will configure SMMU, it has not yet merged and not likely to get merged in the current state. So yes "there is *not* much more to it than bypass is disabled by default now" and you have to specify "iommus" for the master devices or you should enable bypass, i.e., ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n or arm-smmu.disable_bypass=n Try without the patch in the link and without iommus for UFS and ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=y and you will see. -Sai -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 2/6] arm64: dts: qcom: sm8250: add apps_smmu node
Hi,
On 5/25/20 5:42 AM, Sai Prakash Ranjan wrote:
Hi Jonathan,
On 2020-05-24 08:08, Jonathan Marek wrote:
Add the apps_smmu node for sm8250. Note that adding the iommus field for
UFS is required because initializing the iommu removes the bypass mapping
that created by the bootloader.
This statement doesn't seem right, you can just say since the bypass is
disabled
by default now, we need to add this property to enable translation and
avoid global faults.
If I use this patch [1] then the UFS iommu property isn't needed. In
downstream, the identity (bypass?) stream mapping is inherited from the
bootloader, and UFS is used without any iommu property. Setting
ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n doesn't make it work on its own
(without the UFS iommu property), so there's more to it than just
"bypass is disabled by default now".
https://patchwork.kernel.org/patch/11310757/
Signed-off-by: Jonathan Marek
---
arch/arm64/boot/dts/qcom/sm8250.dtsi | 107 +++
1 file changed, 107 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi
b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index 2f99c350c287..43c5e48c15e2 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -323,6 +323,8 @@ ufs_mem_hc: ufshc@1d84000 {
power-domains = <&gcc UFS_PHY_GDSC>;
+ iommus = <&apps_smmu 0x0e0 0>, <&apps_smmu 0x4e0 0>;
+
clock-names =
"core_clk",
"bus_aggr_clk",
@@ -428,6 +430,111 @@ tlmm: pinctrl@f10 {
wakeup-parent = <&pdc>;
};
+ apps_smmu: iommu@1500 {
+ compatible = "qcom,sdm845-smmu-500", "arm,mmu-500";
This should be qcom,sm8250-smmu-500 and also you need to update the
arm-smmu
binding with this compatible in a separate patch.
-Sai
Re: [PATCH 2/6] arm64: dts: qcom: sm8250: add apps_smmu node
Hi Jonathan,
On 2020-05-24 08:08, Jonathan Marek wrote:
Add the apps_smmu node for sm8250. Note that adding the iommus field
for
UFS is required because initializing the iommu removes the bypass
mapping
that created by the bootloader.
This statement doesn't seem right, you can just say since the bypass is
disabled
by default now, we need to add this property to enable translation and
avoid global faults.
Signed-off-by: Jonathan Marek
---
arch/arm64/boot/dts/qcom/sm8250.dtsi | 107 +++
1 file changed, 107 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi
b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index 2f99c350c287..43c5e48c15e2 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -323,6 +323,8 @@ ufs_mem_hc: ufshc@1d84000 {
power-domains = <&gcc UFS_PHY_GDSC>;
+ iommus = <&apps_smmu 0x0e0 0>, <&apps_smmu 0x4e0 0>;
+
clock-names =
"core_clk",
"bus_aggr_clk",
@@ -428,6 +430,111 @@ tlmm: pinctrl@f10 {
wakeup-parent = <&pdc>;
};
+ apps_smmu: iommu@1500 {
+ compatible = "qcom,sdm845-smmu-500", "arm,mmu-500";
This should be qcom,sm8250-smmu-500 and also you need to update the
arm-smmu
binding with this compatible in a separate patch.
-Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a
member
of Code Aurora Forum, hosted by The Linux Foundation
[PATCH 2/6] arm64: dts: qcom: sm8250: add apps_smmu node
Add the apps_smmu node for sm8250. Note that adding the iommus field for
UFS is required because initializing the iommu removes the bypass mapping
that created by the bootloader.
Signed-off-by: Jonathan Marek
---
arch/arm64/boot/dts/qcom/sm8250.dtsi | 107 +++
1 file changed, 107 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi
b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index 2f99c350c287..43c5e48c15e2 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -323,6 +323,8 @@ ufs_mem_hc: ufshc@1d84000 {
power-domains = <&gcc UFS_PHY_GDSC>;
+ iommus = <&apps_smmu 0x0e0 0>, <&apps_smmu 0x4e0 0>;
+
clock-names =
"core_clk",
"bus_aggr_clk",
@@ -428,6 +430,111 @@ tlmm: pinctrl@f10 {
wakeup-parent = <&pdc>;
};
+ apps_smmu: iommu@1500 {
+ compatible = "qcom,sdm845-smmu-500", "arm,mmu-500";
+ reg = <0 0x1500 0 0x10>;
+ #iommu-cells = <2>;
+ #global-interrupts = <2>;
+ interrupts =,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+
