On 30/06/2020 13:13, Robin Murphy wrote: > On 2020-06-30 09:37, Jon Hunter wrote: >> >> On 30/06/2020 01:10, Krishna Reddy wrote: >>> Add global/context fault hooks to allow NVIDIA SMMU implementation >>> handle faults across multiple SMMUs. >> >> Nit ... this is not just for NVIDIA, but this allows anyone to add >> custom global/context and fault hooks. So I think that the changelog >> should be clear that this change permits custom fault hooks and that >> custom fault hooks are needed for the Tegra194 SMMU. You may also want >> to say why. >> >>> >>> Signed-off-by: Krishna Reddy <vdumpa@xxxxxxxxxx> >>> --- >>> drivers/iommu/arm-smmu-nvidia.c | 98 +++++++++++++++++++++++++++++++++ >>> drivers/iommu/arm-smmu.c | 17 +++++- >>> drivers/iommu/arm-smmu.h | 3 + >>> 3 files changed, 116 insertions(+), 2 deletions(-) ... >>> @@ -835,7 +836,13 @@ static int arm_smmu_init_domain_context(struct >>> iommu_domain *domain, >>> * handler seeing a half-initialised domain state. >>> */ >>> irq = smmu->irqs[smmu->num_global_irqs + cfg->irptndx]; >>> - ret = devm_request_irq(smmu->dev, irq, arm_smmu_context_fault, >>> + >>> + if (smmu->impl && smmu->impl->context_fault) >>> + context_fault = smmu->impl->context_fault; >>> + else >>> + context_fault = arm_smmu_context_fault; >> >> Why not see the default smmu->impl->context_fault to >> arm_smmu_context_fault in arm_smmu_impl_init() and then allow the >> various implementations to override as necessary? Then you can get rid >> of this context_fault variable here and just use >> smmu->impl->context_fault below. > > Because the default smmu->impl is NULL. And as I've said before, NAK to > forcing the common case to allocate a set of "quirks" purely to override > the default IRQ handler with the default IRQ handler ;) Ah OK, makes sense. Sorry I am a bit late to the review :-) Jon -- nvpublic
