* [PATCH 1/8] iommu/arm-smmu-qcom: Rename qcom_smmu_impl to qcom_smmu500_impl
2020-09-26 12:59 [PATCH 0/8] Implement firmware quirks for Qualcomm ARM-SMMUv2 kholk11
@ 2020-09-26 12:59 ` kholk11
2020-09-26 12:59 ` [PATCH 2/8] iommu/arm-smmu-qcom: Add QC SMMUv2 VA Size quirk for SDM660 kholk11
` (6 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: kholk11 @ 2020-09-26 12:59 UTC (permalink / raw)
To: will
Cc: robin.murphy, joro, bjorn.andersson, linux-arm-kernel,
devicetree, kholk11, marijns95, konradybcio, martin.botka1,
linux-arm-msm, phone-devel, linux-kernel
From: AngeloGioacchino Del Regno <kholk11@gmail.com>
Rename qcom_smmu_impl to qcom_smmu500_impl, as it refers only to the
MMU-500 in Qualcomm SoCs, in preparation for adding implementation
details for ones having SMMUv2.
Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
---
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index be4318044f96..7859fd0db22a 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -60,7 +60,7 @@ static int qcom_smmu500_reset(struct arm_smmu_device *smmu)
return 0;
}
-static const struct arm_smmu_impl qcom_smmu_impl = {
+static const struct arm_smmu_impl qcom_smmu500_impl = {
.def_domain_type = qcom_smmu_def_domain_type,
.reset = qcom_smmu500_reset,
};
@@ -75,7 +75,7 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu)
qsmmu->smmu = *smmu;
- qsmmu->smmu.impl = &qcom_smmu_impl;
+ qsmmu->smmu.impl = &qcom_smmu500_impl;
devm_kfree(smmu->dev, smmu);
return &qsmmu->smmu;
--
2.28.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 2/8] iommu/arm-smmu-qcom: Add QC SMMUv2 VA Size quirk for SDM660
2020-09-26 12:59 [PATCH 0/8] Implement firmware quirks for Qualcomm ARM-SMMUv2 kholk11
2020-09-26 12:59 ` [PATCH 1/8] iommu/arm-smmu-qcom: Rename qcom_smmu_impl to qcom_smmu500_impl kholk11
@ 2020-09-26 12:59 ` kholk11
2020-10-14 15:43 ` Robin Murphy
2020-09-26 12:59 ` [PATCH 3/8] dt-bindings: arm-smmu: add binding for SMMUv2 on Qualcomm SDM660 kholk11
` (5 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: kholk11 @ 2020-09-26 12:59 UTC (permalink / raw)
To: will
Cc: robin.murphy, joro, bjorn.andersson, linux-arm-kernel,
devicetree, kholk11, marijns95, konradybcio, martin.botka1,
linux-arm-msm, phone-devel, linux-kernel
From: AngeloGioacchino Del Regno <kholk11@gmail.com>
Some IOMMUs are getting set-up for Shared Virtual Address, but:
1. They are secured by the Hypervisor, so any configuration
change will generate a hyp-fault and crash the system
2. This 39-bits Virtual Address size deviates from the ARM
System MMU Architecture specification for SMMUv2, hence
it is non-standard. In this case, the only way to keep the
IOMMU as the firmware did configure it, is to hardcode a
maximum VA size of 39 bits (because of point 1).
This gives the need to add implementation details bits for
at least some of the SoCs having this kind of configuration,
which are at least SDM630, SDM636 and SDM660.
These implementation details will be enabled on finding the
qcom,sdm660-smmu-v2 compatible.
Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
---
drivers/iommu/arm/arm-smmu/arm-smmu-impl.c | 3 ++-
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 31 +++++++++++++++++++++-
2 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
index f4ff124a1967..9d753f8af2cc 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
@@ -216,7 +216,8 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu)
if (of_device_is_compatible(np, "nvidia,tegra194-smmu"))
return nvidia_smmu_impl_init(smmu);
- if (of_device_is_compatible(np, "qcom,sdm845-smmu-500") ||
+ if (of_device_is_compatible(np, "qcom,sdm660-smmu-v2") ||
+ of_device_is_compatible(np, "qcom,sdm845-smmu-500") ||
of_device_is_compatible(np, "qcom,sc7180-smmu-500") ||
of_device_is_compatible(np, "qcom,sm8150-smmu-500") ||
of_device_is_compatible(np, "qcom,sm8250-smmu-500"))
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index 7859fd0db22a..f5bbfe86ef30 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -65,8 +65,33 @@ static const struct arm_smmu_impl qcom_smmu500_impl = {
.reset = qcom_smmu500_reset,
};
+static int qcom_smmuv2_cfg_probe(struct arm_smmu_device *smmu)
+{
+ /*
+ * Some IOMMUs are getting set-up for Shared Virtual Address, but:
+ * 1. They are secured by the Hypervisor, so any configuration
+ * change will generate a hyp-fault and crash the system
+ * 2. This 39-bits Virtual Address size deviates from the ARM
+ * System MMU Architecture specification for SMMUv2, hence
+ * it is non-standard. In this case, the only way to keep the
+ * IOMMU as the firmware did configure it, is to hardcode a
+ * maximum VA size of 39 bits (because of point 1).
+ */
+ if (smmu->va_size > 39UL)
+ dev_notice(smmu->dev,
+ "\tenabling workaround for QCOM SMMUv2 VA size\n");
+ smmu->va_size = min(smmu->va_size, 39UL);
+
+ return 0;
+}
+
+static const struct arm_smmu_impl qcom_smmuv2_impl = {
+ .cfg_probe = qcom_smmuv2_cfg_probe,
+};
+
struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu)
{
+ const struct device_node *np = smmu->dev->of_node;
struct qcom_smmu *qsmmu;
qsmmu = devm_kzalloc(smmu->dev, sizeof(*qsmmu), GFP_KERNEL);
@@ -75,7 +100,11 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu)
qsmmu->smmu = *smmu;
- qsmmu->smmu.impl = &qcom_smmu500_impl;
+ if (of_device_is_compatible(np, "qcom,sdm660-smmu-v2")) {
+ qsmmu->smmu.impl = &qcom_smmuv2_impl;
+ } else {
+ qsmmu->smmu.impl = &qcom_smmu500_impl;
+ }
devm_kfree(smmu->dev, smmu);
return &qsmmu->smmu;
--
2.28.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 2/8] iommu/arm-smmu-qcom: Add QC SMMUv2 VA Size quirk for SDM660
2020-09-26 12:59 ` [PATCH 2/8] iommu/arm-smmu-qcom: Add QC SMMUv2 VA Size quirk for SDM660 kholk11
@ 2020-10-14 15:43 ` Robin Murphy
0 siblings, 0 replies; 14+ messages in thread
From: Robin Murphy @ 2020-10-14 15:43 UTC (permalink / raw)
To: kholk11, will
Cc: joro, bjorn.andersson, linux-arm-kernel, devicetree, marijns95,
konradybcio, martin.botka1, linux-arm-msm, phone-devel,
linux-kernel
On 2020-09-26 13:59, kholk11@gmail.com wrote:
> From: AngeloGioacchino Del Regno <kholk11@gmail.com>
>
> Some IOMMUs are getting set-up for Shared Virtual Address, but:
> 1. They are secured by the Hypervisor, so any configuration
> change will generate a hyp-fault and crash the system
> 2. This 39-bits Virtual Address size deviates from the ARM
> System MMU Architecture specification for SMMUv2, hence
This is a little confusing, since it reads like the 39-bit VA
configuration is itself non-architectural, which it obviously isn't.
> it is non-standard. In this case, the only way to keep the
> IOMMU as the firmware did configure it, is to hardcode a
> maximum VA size of 39 bits (because of point 1).
The *only* way to preserve an existing configuration is to make a
hard-coded assumption about what it probably is? Really? Not, say,
actually reading back the currently-configured value? :/
> This gives the need to add implementation details bits for
> at least some of the SoCs having this kind of configuration,
> which are at least SDM630, SDM636 and SDM660.
>
> These implementation details will be enabled on finding the
> qcom,sdm660-smmu-v2 compatible.
>
> Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu-impl.c | 3 ++-
> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 31 +++++++++++++++++++++-
> 2 files changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
> index f4ff124a1967..9d753f8af2cc 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
> @@ -216,7 +216,8 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu)
> if (of_device_is_compatible(np, "nvidia,tegra194-smmu"))
> return nvidia_smmu_impl_init(smmu);
>
> - if (of_device_is_compatible(np, "qcom,sdm845-smmu-500") ||
> + if (of_device_is_compatible(np, "qcom,sdm660-smmu-v2") ||
> + of_device_is_compatible(np, "qcom,sdm845-smmu-500") ||
> of_device_is_compatible(np, "qcom,sc7180-smmu-500") ||
> of_device_is_compatible(np, "qcom,sm8150-smmu-500") ||
> of_device_is_compatible(np, "qcom,sm8250-smmu-500"))
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index 7859fd0db22a..f5bbfe86ef30 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -65,8 +65,33 @@ static const struct arm_smmu_impl qcom_smmu500_impl = {
> .reset = qcom_smmu500_reset,
> };
>
> +static int qcom_smmuv2_cfg_probe(struct arm_smmu_device *smmu)
> +{
> + /*
> + * Some IOMMUs are getting set-up for Shared Virtual Address, but:
> + * 1. They are secured by the Hypervisor, so any configuration
> + * change will generate a hyp-fault and crash the system
> + * 2. This 39-bits Virtual Address size deviates from the ARM
> + * System MMU Architecture specification for SMMUv2, hence
> + * it is non-standard. In this case, the only way to keep the
> + * IOMMU as the firmware did configure it, is to hardcode a
> + * maximum VA size of 39 bits (because of point 1).
> + */
What's the actual UBS value reported? If it's 40 then arguably the main
driver could be cleverer and use 39 bits for stage 1 domains anyway to
save a level of pagetable. However...
I'm concerned that "any configuration change" and "maximum" don't appear
to add up. What if we decided we want to use short-descriptor format (or
just pick a 32-bit VA size for other reasons)? That's happily less than
39 bits, but would still represent a change *from* 39 bits, so would it
also kill the system? IIRC Jordan's split pagetable support will end up
configuring contexts with TxSZ based on va_bits / 2, so now 38 bits - is
that going to work?
If you need to impose specific restrictions on context bank format, just
bodging va_bits isn't going to cut it. You'd need to adjust the domain
and pagetable configuration directly in ->init_context() in very much
the same way as the split pagetable support itself does.
That said, I still hold the opinion that if you can't reprogram the
context banks then this is qcom-iommu territory, not arm-smmu.
Robin.
> + if (smmu->va_size > 39UL)
> + dev_notice(smmu->dev,
> + "\tenabling workaround for QCOM SMMUv2 VA size\n");
> + smmu->va_size = min(smmu->va_size, 39UL);
> +
> + return 0;
> +}
> +
> +static const struct arm_smmu_impl qcom_smmuv2_impl = {
> + .cfg_probe = qcom_smmuv2_cfg_probe,
> +};
> +
> struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu)
> {
> + const struct device_node *np = smmu->dev->of_node;
> struct qcom_smmu *qsmmu;
>
> qsmmu = devm_kzalloc(smmu->dev, sizeof(*qsmmu), GFP_KERNEL);
> @@ -75,7 +100,11 @@ struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu)
>
> qsmmu->smmu = *smmu;
>
> - qsmmu->smmu.impl = &qcom_smmu500_impl;
> + if (of_device_is_compatible(np, "qcom,sdm660-smmu-v2")) {
> + qsmmu->smmu.impl = &qcom_smmuv2_impl;
> + } else {
> + qsmmu->smmu.impl = &qcom_smmu500_impl;
> + }
> devm_kfree(smmu->dev, smmu);
>
> return &qsmmu->smmu;
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/8] dt-bindings: arm-smmu: add binding for SMMUv2 on Qualcomm SDM660
2020-09-26 12:59 [PATCH 0/8] Implement firmware quirks for Qualcomm ARM-SMMUv2 kholk11
2020-09-26 12:59 ` [PATCH 1/8] iommu/arm-smmu-qcom: Rename qcom_smmu_impl to qcom_smmu500_impl kholk11
2020-09-26 12:59 ` [PATCH 2/8] iommu/arm-smmu-qcom: Add QC SMMUv2 VA Size quirk for SDM660 kholk11
@ 2020-09-26 12:59 ` kholk11
2020-09-29 19:10 ` Rob Herring
2020-09-26 13:00 ` [PATCH 4/8] iommu/arm-smmu: Support test_smr_masks implementation detail deviation kholk11
` (4 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: kholk11 @ 2020-09-26 12:59 UTC (permalink / raw)
To: will
Cc: robin.murphy, joro, bjorn.andersson, linux-arm-kernel,
devicetree, kholk11, marijns95, konradybcio, martin.botka1,
linux-arm-msm, phone-devel, linux-kernel
From: AngeloGioacchino Del Regno <kholk11@gmail.com>
Add the binding for the SMMUv2 found on Qualcomm SDM660.
Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
---
Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
index 503160a7b9a0..fdad89fbf130 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
@@ -26,6 +26,7 @@ properties:
- description: Qcom SoCs implementing "arm,smmu-v2"
items:
- enum:
+ - qcom,sdm660-smmu-v2
- qcom,msm8996-smmu-v2
- qcom,msm8998-smmu-v2
- qcom,sc7180-smmu-v2
--
2.28.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 3/8] dt-bindings: arm-smmu: add binding for SMMUv2 on Qualcomm SDM660
2020-09-26 12:59 ` [PATCH 3/8] dt-bindings: arm-smmu: add binding for SMMUv2 on Qualcomm SDM660 kholk11
@ 2020-09-29 19:10 ` Rob Herring
0 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2020-09-29 19:10 UTC (permalink / raw)
To: kholk11
Cc: will, marijns95, martin.botka1, konradybcio, devicetree,
linux-arm-kernel, phone-devel, linux-arm-msm, joro, robin.murphy,
bjorn.andersson, linux-kernel
On Sat, 26 Sep 2020 14:59:59 +0200, kholk11@gmail.com wrote:
> From: AngeloGioacchino Del Regno <kholk11@gmail.com>
>
> Add the binding for the SMMUv2 found on Qualcomm SDM660.
>
> Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
> ---
> Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 4/8] iommu/arm-smmu: Support test_smr_masks implementation detail deviation
2020-09-26 12:59 [PATCH 0/8] Implement firmware quirks for Qualcomm ARM-SMMUv2 kholk11
` (2 preceding siblings ...)
2020-09-26 12:59 ` [PATCH 3/8] dt-bindings: arm-smmu: add binding for SMMUv2 on Qualcomm SDM660 kholk11
@ 2020-09-26 13:00 ` kholk11
2020-10-14 15:52 ` Robin Murphy
2020-09-26 13:00 ` [PATCH 5/8] iommu/arm-smmu-qcom: Add test_smr_masks detail to QCOM SMMUv2 kholk11
` (3 subsequent siblings)
7 siblings, 1 reply; 14+ messages in thread
From: kholk11 @ 2020-09-26 13:00 UTC (permalink / raw)
To: will
Cc: robin.murphy, joro, bjorn.andersson, linux-arm-kernel,
devicetree, kholk11, marijns95, konradybcio, martin.botka1,
linux-arm-msm, phone-devel, linux-kernel
From: AngeloGioacchino Del Regno <kholk11@gmail.com>
At least some Qualcomm SoCs do need to override the function
arm_smmu_test_smr_masks entirely: add a test_smr_masks function
to the implementation details structure and call it properly.
Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
---
drivers/iommu/arm/arm-smmu/arm-smmu.c | 6 ++++++
drivers/iommu/arm/arm-smmu/arm-smmu.h | 1 +
2 files changed, 7 insertions(+)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 09c42af9f31e..446a78dde9cd 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -977,6 +977,12 @@ static void arm_smmu_test_smr_masks(struct arm_smmu_device *smmu)
if (!smmu->smrs)
return;
+
+ if (smmu->impl && smmu->impl->test_smr_masks) {
+ smmu->impl->test_smr_masks(smmu);
+ return;
+ }
+
/*
* If we've had to accommodate firmware memory regions, we may
* have live SMRs by now; tread carefully...
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h
index d890a4a968e8..2cd3d126f675 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
@@ -387,6 +387,7 @@ struct arm_smmu_impl {
int (*cfg_probe)(struct arm_smmu_device *smmu);
int (*reset)(struct arm_smmu_device *smmu);
int (*init_context)(struct arm_smmu_domain *smmu_domain);
+ void (*test_smr_masks)(struct arm_smmu_device *smmu);
void (*tlb_sync)(struct arm_smmu_device *smmu, int page, int sync,
int status);
int (*def_domain_type)(struct device *dev);
--
2.28.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 4/8] iommu/arm-smmu: Support test_smr_masks implementation detail deviation
2020-09-26 13:00 ` [PATCH 4/8] iommu/arm-smmu: Support test_smr_masks implementation detail deviation kholk11
@ 2020-10-14 15:52 ` Robin Murphy
0 siblings, 0 replies; 14+ messages in thread
From: Robin Murphy @ 2020-10-14 15:52 UTC (permalink / raw)
To: kholk11, will
Cc: joro, bjorn.andersson, linux-arm-kernel, devicetree, marijns95,
konradybcio, martin.botka1, linux-arm-msm, phone-devel,
linux-kernel
On 2020-09-26 14:00, kholk11@gmail.com wrote:
> From: AngeloGioacchino Del Regno <kholk11@gmail.com>
>
> At least some Qualcomm SoCs do need to override the function
> arm_smmu_test_smr_masks entirely: add a test_smr_masks function
> to the implementation details structure and call it properly.
>
> Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu.c | 6 ++++++
> drivers/iommu/arm/arm-smmu/arm-smmu.h | 1 +
> 2 files changed, 7 insertions(+)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index 09c42af9f31e..446a78dde9cd 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -977,6 +977,12 @@ static void arm_smmu_test_smr_masks(struct arm_smmu_device *smmu)
>
> if (!smmu->smrs)
> return;
> +
> + if (smmu->impl && smmu->impl->test_smr_masks) {
> + smmu->impl->test_smr_masks(smmu);
Meh, this doesn't need a special hook - just have ->cfg_probe()
initialise your masks early and bail out here if smr_mask_mask is
already set. You could actually bypass this test as-is by marking all
your SMR entries as valid, but that's likely to cause far more problems
elsewhere than it solves here ;)
Robin.
> + return;
> + }
> +
> /*
> * If we've had to accommodate firmware memory regions, we may
> * have live SMRs by now; tread carefully...
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> index d890a4a968e8..2cd3d126f675 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> @@ -387,6 +387,7 @@ struct arm_smmu_impl {
> int (*cfg_probe)(struct arm_smmu_device *smmu);
> int (*reset)(struct arm_smmu_device *smmu);
> int (*init_context)(struct arm_smmu_domain *smmu_domain);
> + void (*test_smr_masks)(struct arm_smmu_device *smmu);
> void (*tlb_sync)(struct arm_smmu_device *smmu, int page, int sync,
> int status);
> int (*def_domain_type)(struct device *dev);
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 5/8] iommu/arm-smmu-qcom: Add test_smr_masks detail to QCOM SMMUv2
2020-09-26 12:59 [PATCH 0/8] Implement firmware quirks for Qualcomm ARM-SMMUv2 kholk11
` (3 preceding siblings ...)
2020-09-26 13:00 ` [PATCH 4/8] iommu/arm-smmu: Support test_smr_masks implementation detail deviation kholk11
@ 2020-09-26 13:00 ` kholk11
2020-09-26 13:00 ` [PATCH 6/8] iommu/arm-smmu: Move stream mapping reset to separate function kholk11
` (2 subsequent siblings)
7 siblings, 0 replies; 14+ messages in thread
From: kholk11 @ 2020-09-26 13:00 UTC (permalink / raw)
To: will
Cc: robin.murphy, joro, bjorn.andersson, linux-arm-kernel,
devicetree, kholk11, marijns95, konradybcio, martin.botka1,
linux-arm-msm, phone-devel, linux-kernel
From: AngeloGioacchino Del Regno <kholk11@gmail.com>
On some Qualcomm SoCs with certain hypervisor configurations,
writing the streamid masks to the SMRs will trigger a hyp-fault
and crash the system.
This is seen on at least Qualcomm SDM630, SDM636 and SDM660.
Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
---
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index f5bbfe86ef30..b18e70bddf29 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -85,8 +85,21 @@ static int qcom_smmuv2_cfg_probe(struct arm_smmu_device *smmu)
return 0;
}
+static void qcom_smmuv2_test_smr_masks(struct arm_smmu_device *smmu)
+{
+ /*
+ * Broken firmware quirk:
+ * On some Qualcomm SoCs with certain hypervisor configurations,
+ * writing the streamid masks to the SMRs will trigger a hyp-fault
+ * and crash the system.
+ */
+ smmu->streamid_mask = 0x7FFF;
+ smmu->smr_mask_mask = 0x7FFF;
+}
+
static const struct arm_smmu_impl qcom_smmuv2_impl = {
.cfg_probe = qcom_smmuv2_cfg_probe,
+ .test_smr_masks = qcom_smmuv2_test_smr_masks,
};
struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu)
--
2.28.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 6/8] iommu/arm-smmu: Move stream mapping reset to separate function
2020-09-26 12:59 [PATCH 0/8] Implement firmware quirks for Qualcomm ARM-SMMUv2 kholk11
` (4 preceding siblings ...)
2020-09-26 13:00 ` [PATCH 5/8] iommu/arm-smmu-qcom: Add test_smr_masks detail to QCOM SMMUv2 kholk11
@ 2020-09-26 13:00 ` kholk11
2020-10-14 15:56 ` Robin Murphy
2020-09-26 13:00 ` [PATCH 7/8] iommu/arm-smmu: Support stream_mapping_reset implementation detail kholk11
2020-09-26 13:00 ` [PATCH 8/8] iommu/arm-smmu-qcom: Add stream_mapping_reset detail to QCOM SMMUv2 kholk11
7 siblings, 1 reply; 14+ messages in thread
From: kholk11 @ 2020-09-26 13:00 UTC (permalink / raw)
To: will
Cc: robin.murphy, joro, bjorn.andersson, linux-arm-kernel,
devicetree, kholk11, marijns95, konradybcio, martin.botka1,
linux-arm-msm, phone-devel, linux-kernel
From: AngeloGioacchino Del Regno <kholk11@gmail.com>
Move the stream mapping reset logic from arm_smmu_device_reset into
a separate arm_smmu_stream_mapping_reset function, in preparation
for implementing an implementation detail.
This commit brings no functional changes.
Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
---
drivers/iommu/arm/arm-smmu/arm-smmu.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 446a78dde9cd..8c070c493315 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -1652,14 +1652,9 @@ static struct iommu_ops arm_smmu_ops = {
.pgsize_bitmap = -1UL, /* Restricted during device attach */
};
-static void arm_smmu_device_reset(struct arm_smmu_device *smmu)
+static void arm_smmu_stream_mapping_reset(struct arm_smmu_device *smmu)
{
int i;
- u32 reg;
-
- /* clear global FSR */
- reg = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_sGFSR);
- arm_smmu_gr0_write(smmu, ARM_SMMU_GR0_sGFSR, reg);
/*
* Reset stream mapping groups: Initial values mark all SMRn as
@@ -1673,6 +1668,18 @@ static void arm_smmu_device_reset(struct arm_smmu_device *smmu)
arm_smmu_write_context_bank(smmu, i);
arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_FSR, ARM_SMMU_FSR_FAULT);
}
+}
+
+static void arm_smmu_device_reset(struct arm_smmu_device *smmu)
+{
+ u32 reg;
+
+ /* clear global FSR */
+ reg = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_sGFSR);
+ arm_smmu_gr0_write(smmu, ARM_SMMU_GR0_sGFSR, reg);
+
+ /* Reset stream mapping */
+ arm_smmu_stream_mapping_reset(smmu);
/* Invalidate the TLB, just in case */
arm_smmu_gr0_write(smmu, ARM_SMMU_GR0_TLBIALLH, QCOM_DUMMY_VAL);
--
2.28.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 6/8] iommu/arm-smmu: Move stream mapping reset to separate function
2020-09-26 13:00 ` [PATCH 6/8] iommu/arm-smmu: Move stream mapping reset to separate function kholk11
@ 2020-10-14 15:56 ` Robin Murphy
0 siblings, 0 replies; 14+ messages in thread
From: Robin Murphy @ 2020-10-14 15:56 UTC (permalink / raw)
To: kholk11, will
Cc: joro, bjorn.andersson, linux-arm-kernel, devicetree, marijns95,
konradybcio, martin.botka1, linux-arm-msm, phone-devel,
linux-kernel
On 2020-09-26 14:00, kholk11@gmail.com wrote:
> From: AngeloGioacchino Del Regno <kholk11@gmail.com>
>
> Move the stream mapping reset logic from arm_smmu_device_reset into
> a separate arm_smmu_stream_mapping_reset function, in preparation
> for implementing an implementation detail.
>
> This commit brings no functional changes.
Please coordinate with Bjorn's series so you're not wasting time
developing 'competing' implementations of the exact same thing. You
don't need these last 3 patches, for reasons I've already explained over
there.
Robin.
> Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu.c | 19 +++++++++++++------
> 1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index 446a78dde9cd..8c070c493315 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -1652,14 +1652,9 @@ static struct iommu_ops arm_smmu_ops = {
> .pgsize_bitmap = -1UL, /* Restricted during device attach */
> };
>
> -static void arm_smmu_device_reset(struct arm_smmu_device *smmu)
> +static void arm_smmu_stream_mapping_reset(struct arm_smmu_device *smmu)
> {
> int i;
> - u32 reg;
> -
> - /* clear global FSR */
> - reg = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_sGFSR);
> - arm_smmu_gr0_write(smmu, ARM_SMMU_GR0_sGFSR, reg);
>
> /*
> * Reset stream mapping groups: Initial values mark all SMRn as
> @@ -1673,6 +1668,18 @@ static void arm_smmu_device_reset(struct arm_smmu_device *smmu)
> arm_smmu_write_context_bank(smmu, i);
> arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_FSR, ARM_SMMU_FSR_FAULT);
> }
> +}
> +
> +static void arm_smmu_device_reset(struct arm_smmu_device *smmu)
> +{
> + u32 reg;
> +
> + /* clear global FSR */
> + reg = arm_smmu_gr0_read(smmu, ARM_SMMU_GR0_sGFSR);
> + arm_smmu_gr0_write(smmu, ARM_SMMU_GR0_sGFSR, reg);
> +
> + /* Reset stream mapping */
> + arm_smmu_stream_mapping_reset(smmu);
>
> /* Invalidate the TLB, just in case */
> arm_smmu_gr0_write(smmu, ARM_SMMU_GR0_TLBIALLH, QCOM_DUMMY_VAL);
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 7/8] iommu/arm-smmu: Support stream_mapping_reset implementation detail
2020-09-26 12:59 [PATCH 0/8] Implement firmware quirks for Qualcomm ARM-SMMUv2 kholk11
` (5 preceding siblings ...)
2020-09-26 13:00 ` [PATCH 6/8] iommu/arm-smmu: Move stream mapping reset to separate function kholk11
@ 2020-09-26 13:00 ` kholk11
2020-09-26 13:00 ` [PATCH 8/8] iommu/arm-smmu-qcom: Add stream_mapping_reset detail to QCOM SMMUv2 kholk11
7 siblings, 0 replies; 14+ messages in thread
From: kholk11 @ 2020-09-26 13:00 UTC (permalink / raw)
To: will
Cc: robin.murphy, joro, bjorn.andersson, linux-arm-kernel,
devicetree, kholk11, marijns95, konradybcio, martin.botka1,
linux-arm-msm, phone-devel, linux-kernel
From: AngeloGioacchino Del Regno <kholk11@gmail.com>
Some IOMMUs may be in need of overriding the stream mapping reset
function and this is seen on at least some Qualcomm SoCs:
add a stream_mapping_reset function to the implementation details
and call it in the appropriate function.
Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
---
drivers/iommu/arm/arm-smmu/arm-smmu.c | 5 +++++
drivers/iommu/arm/arm-smmu/arm-smmu.h | 1 +
2 files changed, 6 insertions(+)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 8c070c493315..44571873f148 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -1656,6 +1656,11 @@ static void arm_smmu_stream_mapping_reset(struct arm_smmu_device *smmu)
{
int i;
+ if (smmu->impl && smmu->impl->stream_mapping_reset) {
+ smmu->impl->stream_mapping_reset(smmu);
+ return;
+ }
+
/*
* Reset stream mapping groups: Initial values mark all SMRn as
* invalid and all S2CRn as bypass unless overridden.
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h b/drivers/iommu/arm/arm-smmu/arm-smmu.h
index 2cd3d126f675..9c045594b8cf 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
@@ -387,6 +387,7 @@ struct arm_smmu_impl {
int (*cfg_probe)(struct arm_smmu_device *smmu);
int (*reset)(struct arm_smmu_device *smmu);
int (*init_context)(struct arm_smmu_domain *smmu_domain);
+ void (*stream_mapping_reset)(struct arm_smmu_device *smmu);
void (*test_smr_masks)(struct arm_smmu_device *smmu);
void (*tlb_sync)(struct arm_smmu_device *smmu, int page, int sync,
int status);
--
2.28.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 8/8] iommu/arm-smmu-qcom: Add stream_mapping_reset detail to QCOM SMMUv2
2020-09-26 12:59 [PATCH 0/8] Implement firmware quirks for Qualcomm ARM-SMMUv2 kholk11
` (6 preceding siblings ...)
2020-09-26 13:00 ` [PATCH 7/8] iommu/arm-smmu: Support stream_mapping_reset implementation detail kholk11
@ 2020-09-26 13:00 ` kholk11
2020-10-14 22:14 ` Bjorn Andersson
7 siblings, 1 reply; 14+ messages in thread
From: kholk11 @ 2020-09-26 13:00 UTC (permalink / raw)
To: will
Cc: robin.murphy, joro, bjorn.andersson, linux-arm-kernel,
devicetree, kholk11, marijns95, konradybcio, martin.botka1,
linux-arm-msm, phone-devel, linux-kernel
From: AngeloGioacchino Del Regno <kholk11@gmail.com>
On some Qualcomm SoCs with certain hypervisor configurations,
some context banks are hyp-protected and cannot be disabled,
nor the relative S2CRs can be set as bypass, or a hyp-fault
will be triggered and the system will hang.
This is seen on at least Qualcomm SDM630, SDM636 and SDM660.
Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
---
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index b18e70bddf29..364908cc2adf 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -85,6 +85,18 @@ static int qcom_smmuv2_cfg_probe(struct arm_smmu_device *smmu)
return 0;
}
+static void qcom_smmuv2_stream_mapping_reset(struct arm_smmu_device *smmu)
+{
+ /*
+ * Broken firmware quirk:
+ * On some Qualcomm SoCs with certain hypervisor configurations,
+ * some context banks are hyp-protected and cannot be disabled,
+ * nor the relative S2CRs can be set as bypass, or a hyp-fault
+ * will be triggered and the system will hang.
+ */
+ return;
+}
+
static void qcom_smmuv2_test_smr_masks(struct arm_smmu_device *smmu)
{
/*
@@ -99,6 +111,7 @@ static void qcom_smmuv2_test_smr_masks(struct arm_smmu_device *smmu)
static const struct arm_smmu_impl qcom_smmuv2_impl = {
.cfg_probe = qcom_smmuv2_cfg_probe,
+ .stream_mapping_reset = qcom_smmuv2_stream_mapping_reset,
.test_smr_masks = qcom_smmuv2_test_smr_masks,
};
--
2.28.0
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 8/8] iommu/arm-smmu-qcom: Add stream_mapping_reset detail to QCOM SMMUv2
2020-09-26 13:00 ` [PATCH 8/8] iommu/arm-smmu-qcom: Add stream_mapping_reset detail to QCOM SMMUv2 kholk11
@ 2020-10-14 22:14 ` Bjorn Andersson
0 siblings, 0 replies; 14+ messages in thread
From: Bjorn Andersson @ 2020-10-14 22:14 UTC (permalink / raw)
To: kholk11
Cc: will, robin.murphy, joro, linux-arm-kernel, devicetree,
marijns95, konradybcio, martin.botka1, linux-arm-msm,
phone-devel, linux-kernel
On Sat 26 Sep 08:00 CDT 2020, kholk11@gmail.com wrote:
> From: AngeloGioacchino Del Regno <kholk11@gmail.com>
>
> On some Qualcomm SoCs with certain hypervisor configurations,
> some context banks are hyp-protected and cannot be disabled,
> nor the relative S2CRs can be set as bypass, or a hyp-fault
> will be triggered and the system will hang.
>
> This is seen on at least Qualcomm SDM630, SDM636 and SDM660.
>
> Signed-off-by: AngeloGioacchino Del Regno <kholk11@gmail.com>
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index b18e70bddf29..364908cc2adf 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -85,6 +85,18 @@ static int qcom_smmuv2_cfg_probe(struct arm_smmu_device *smmu)
> return 0;
> }
>
> +static void qcom_smmuv2_stream_mapping_reset(struct arm_smmu_device *smmu)
> +{
> + /*
> + * Broken firmware quirk:
> + * On some Qualcomm SoCs with certain hypervisor configurations,
> + * some context banks are hyp-protected and cannot be disabled,
Wouldn't you run into the same problem when init_domain_context() later
comes along and "accidentally" pick one of these context banks?
Do we have any way of knowing which banks this is, so we can mark them
as busy?
> + * nor the relative S2CRs can be set as bypass, or a hyp-fault
On platforms such as SDM845, SM8150, SM8250 etc, writing S2CR of type
BYPASS is trapped by the hypervisor and FAULT is actually written to the
hardware - resulting in a system reset when the associated hardware
tries to perform a memory access.
Is it the actual S2CR write that causes the problem you're seeing or the
fact that it happens to be that you shoot down the display stream as
soon as you touch these registers?
Regards,
Bjorn
> + * will be triggered and the system will hang.
> + */
> + return;
> +}
> +
> static void qcom_smmuv2_test_smr_masks(struct arm_smmu_device *smmu)
> {
> /*
> @@ -99,6 +111,7 @@ static void qcom_smmuv2_test_smr_masks(struct arm_smmu_device *smmu)
>
> static const struct arm_smmu_impl qcom_smmuv2_impl = {
> .cfg_probe = qcom_smmuv2_cfg_probe,
> + .stream_mapping_reset = qcom_smmuv2_stream_mapping_reset,
> .test_smr_masks = qcom_smmuv2_test_smr_masks,
> };
>
> --
> 2.28.0
>
^ permalink raw reply [flat|nested] 14+ messages in thread