On 03.07.2020 10:24, Robin Murphy wrote:
On 2020-07-02 21:16, Tomasz Nowicki wrote:
We already have 'cfg_probe' hook which meant to override and apply
workarounds while probing ID registers. However, 'cfg_probe' is called
at the very end and therefore for some cases fixing up things becomes complex or requires exporting of SMMU driver structures. Hence, seems it is better and
cleaner to do ID fixup right away. In preparation for adding Marvell
errata add an extra ID2 fixup hook.

Hmm, the intent of ->cfg_probe was very much to give impl a chance to adjust the detected features before we start consuming them, with this exact case in mind. Since the Cavium quirk isn't actually doing that - it just needs to run *anywhere* in the whole probe process - I'm under no illusion that I put the hook in exactly the right place first time around ;)

The diff below should be more on the mark...

Robin.

----->8-----
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 243bc4cb2705..884ffca5b1eb 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1891,6 +1891,9 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
              smmu->features |= ARM_SMMU_FEAT_FMT_AARCH64_64K;
      }

+    if (smmu->impl && smmu->impl->cfg_probe)
+        return smmu->impl->cfg_probe(smmu);
+
      /* Now we've corralled the various formats, what'll it do? */
      if (smmu->features & ARM_SMMU_FEAT_FMT_AARCH32_S)
          smmu->pgsize_bitmap |= SZ_4K | SZ_64K | SZ_1M | SZ_16M;
@@ -1909,7 +1912,6 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
      dev_notice(smmu->dev, "\tSupported page sizes: 0x%08lx\n",
             smmu->pgsize_bitmap);

-
      if (smmu->features & ARM_SMMU_FEAT_TRANS_S1)
          dev_notice(smmu->dev, "\tStage-1: %lu-bit VA -> %lu-bit IPA\n",
                 smmu->va_size, smmu->ipa_size);
@@ -1918,9 +1920,6 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
          dev_notice(smmu->dev, "\tStage-2: %lu-bit IPA -> %lu-bit PA\n",
                 smmu->ipa_size, smmu->pa_size);

-    if (smmu->impl && smmu->impl->cfg_probe)
-        return smmu->impl->cfg_probe(smmu);
-
      return 0;
  }


I was under impression that ->cfg_probe was meant for Cavium alike errata (position independent). Then I will move ->cfg_probe as you suggested. I prefer not to add yet another impl hook too :)

Thanks,
Tomasz
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to