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