Re: [PATCH 1/2] dt-bindings: arm-smmu: Add qcom,last-ctx-bank-reserved

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 19/08/2024 12:37 pm, Marc Gonzalez wrote:
On 18/08/2024 17:25, Rob Herring wrote:

On Wed, Aug 14, 2024 at 03:59:55PM +0200, Marc Gonzalez wrote:

On qcom msm8998, writing to the last context bank of lpass_q6_smmu
(base address 0x05100000) produces a system freeze & reboot.

Specifically, here:

	qsmmu->bypass_cbndx = smmu->num_context_banks - 1;
	arm_smmu_cb_write(smmu, qsmmu->bypass_cbndx, ARM_SMMU_CB_SCTLR, 0);

and here:

	arm_smmu_write_context_bank(smmu, i);
	arm_smmu_cb_write(smmu, i, ARM_SMMU_CB_FSR, ARM_SMMU_CB_FSR_FAULT);

It is likely that FW reserves the last context bank for its own use,
thus a simple work-around would be: DON'T USE IT in Linux.

Signed-off-by: Marc Gonzalez <mgonzalez@xxxxxxxxxx>
---
  Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 6 ++++++
  1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
index 280b4e49f2191..f9b23aef351b0 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
@@ -204,6 +204,12 @@ properties:
        access to SMMU configuration registers. In this case non-secure aliases of
        secure registers have to be used during SMMU configuration.
+ qcom,last-ctx-bank-reserved:
+    type: boolean
+    description:
+      FW reserves the last context bank of this SMMU for its own use.
+      If Linux tries to use it, Linux gets nuked.

How is this Qualcomm specific? Presumably any implementation could do
this if there's no way to properly partition things. Robin?

Obviously, there is nothing Qualcomm specific about reserving
an SMMU context bank for the FW / hypervisor, other than it
appears that qcom is the first to do it; or at least the
LPASS SMMU on qcom msm8998 is the first known SMMU where such
a work-around is required.

Yes, the Qualcomm-specific aspect is that it's Qualcomm's hypervisor which is broken and reporting a larger number in its emulated SMMU_IDR1.NUMCB than the number of context banks it's actually willing to emulate.

What is the correct nomenclature?

Can we just drop the vendor prefix if a property is generic
across vendors? But does it require a subsystem prefix like
"iommu" in order to not clash with generic props in other subsystems?

I guess if we *were* to consider a generic property to endorse violating the SMMU architecture, then it would logically be vendored to Arm as the owner of the SMMU architecture. However I am strongly against that idea, not only because I obviously don't want to normalise hypervisors emulating non-architectural behaviour which every DT-consuming OS will have to understand how to work around, but it's also less than great for the user to have a workaround that's not compatible with existing DTBs.

Luckily, in this case it seems straightforward enough to be able to see that if we have a "qcom,msm8996-smmu-v2" with 13 context banks then we should just treat it as if it has 12 - it's also notable that it only reports NUMSMRG=12, so we couldn't use more than that many S1 context banks at once anyway.

Thanks,
Robin.

Also, this property isn't very flexible. What happens when it is not the
last bank or more than 1 bank reserved? This should probably be a mask
instead.

OK, I'm getting conflicting requests here.

Bjorn has recommended dropping the property altogether:

It also seems, as the different SMMUs in this platform behave
differently it might be worth giving them further specific compatibles,
in which case we could just check if it's the qcom,msm8998-lpass-smmu,
instead of inventing a property for this quirk.


I'll send a patch series in line with Bjorn's request.

Regards





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux