On 15/08/2019 11:56, Will Deacon wrote:
On Fri, Aug 09, 2019 at 06:07:52PM +0100, Robin Murphy wrote:
Allocating and initialising a context for a domain is another point
where certain implementations are known to want special behaviour.
Currently the other half of the Cavium workaround comes into play here,
so let's finish the job to get the whole thing right out of the way.
Signed-off-by: Robin Murphy <robin.mur...@arm.com>
---
drivers/iommu/arm-smmu-impl.c | 39 +++++++++++++++++++++++++--
drivers/iommu/arm-smmu.c | 51 +++++++----------------------------
drivers/iommu/arm-smmu.h | 42 +++++++++++++++++++++++++++--
3 files changed, 86 insertions(+), 46 deletions(-)
diff --git a/drivers/iommu/arm-smmu-impl.c b/drivers/iommu/arm-smmu-impl.c
index c8904da08354..7a657d47b6ec 100644
--- a/drivers/iommu/arm-smmu-impl.c
+++ b/drivers/iommu/arm-smmu-impl.c
@@ -48,6 +48,12 @@ const struct arm_smmu_impl calxeda_impl = {
};
+struct cavium_smmu {
+ struct arm_smmu_device smmu;
+ u32 id_base;
+};
+#define to_csmmu(s) container_of(s, struct cavium_smmu, smmu)
To be honest with you, I'd just use container_of directly for the two
callsites that need it. "to_csmmu" isn't a great name when we're also got
the calxeda thing in here.
Sure, by this point I was mostly just going for completeness in terms of
sketching out an example for subclassing arm_smmu_device. The Tegra
patches will now serve as a more complete example anyway, so indeed we
can live without it here.
static int cavium_cfg_probe(struct arm_smmu_device *smmu)
{
static atomic_t context_count = ATOMIC_INIT(0);
@@ -56,17 +62,46 @@ static int cavium_cfg_probe(struct arm_smmu_device *smmu)
* Ensure ASID and VMID allocation is unique across all SMMUs in
* the system.
*/
- smmu->cavium_id_base = atomic_fetch_add(smmu->num_context_banks,
+ to_csmmu(smmu)->id_base = atomic_fetch_add(smmu->num_context_banks,
&context_count);
dev_notice(smmu->dev, "\tenabling workaround for Cavium erratum
27704\n");
return 0;
}
+int cavium_init_context(struct arm_smmu_domain *smmu_domain)
+{
+ u32 id_base = to_csmmu(smmu_domain->smmu)->id_base;
+
+ if (smmu_domain->stage == ARM_SMMU_DOMAIN_S2)
+ smmu_domain->cfg.vmid += id_base;
+ else
+ smmu_domain->cfg.asid += id_base;
+
+ return 0;
+}
+
const struct arm_smmu_impl cavium_impl = {
.cfg_probe = cavium_cfg_probe,
+ .init_context = cavium_init_context,
};
+struct arm_smmu_device *cavium_smmu_impl_init(struct arm_smmu_device *smmu)
+{
+ struct cavium_smmu *csmmu;
+
+ csmmu = devm_kzalloc(smmu->dev, sizeof(*csmmu), GFP_KERNEL);
+ if (!csmmu)
+ return ERR_PTR(-ENOMEM);
+
+ csmmu->smmu = *smmu;
+ csmmu->smmu.impl = &cavium_impl;
+
+ devm_kfree(smmu->dev, smmu);
+
+ return &csmmu->smmu;
+}
+
#define ARM_MMU500_ACTLR_CPRE (1 << 1)
@@ -121,7 +156,7 @@ struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu)
smmu->impl = &calxeda_impl;
if (smmu->model == CAVIUM_SMMUV2)
- smmu->impl = &cavium_impl;
+ return cavium_smmu_impl_init(smmu);
if (smmu->model == ARM_MMU500)
smmu->impl = &arm_mmu500_impl;
Maybe rework this so we do the calxeda detection first (and return if we
match), followed by a switch on smmu->model to make it crystal clear that
we match only one?
As I see it, "match only one" is really only a short-term thing, though,
so I didn't want to get *too* hung up on it. Ultimately we're going to
have cases where we need to combine e.g. MMU-500 implementation quirks
with platform integration quirks - I've been mostly planning on coming
back to think about that (and potentially rework this whole logic)
later, but I guess it wouldn't hurt to plan out a bit more structure
from the start.
I'll have a hack on that (and all the other comments) today and
hopefully have a v2 by tomorrow.
Robin.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
