| Message ID | 15d196d9a8e160c5df0e0340e23a432482389f9f.1512038236.git.robin.murphy@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Dec 14, 2017 at 04:58:53PM +0000, Robin Murphy wrote: > Stage 1 input addresses are effectively 64-bit in SMMUv3 anyway, so > really all that's involved is letting io-pgtable know the appropriate > upper bound for T0SZ. > > Tested-by: Nate Watterson <nwatters@codeaurora.org> > Signed-off-by: Robin Murphy <robin.murphy@arm.com> > --- > > v2: No change > > drivers/iommu/arm-smmu-v3.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c > index c9c4e6132e27..7059a0805bd1 100644 > --- a/drivers/iommu/arm-smmu-v3.c > +++ b/drivers/iommu/arm-smmu-v3.c > @@ -102,6 +102,7 @@ > #define IDR5_OAS_44_BIT (4 << IDR5_OAS_SHIFT) > #define IDR5_OAS_48_BIT (5 << IDR5_OAS_SHIFT) > #define IDR5_OAS_52_BIT (6 << IDR5_OAS_SHIFT) > +#define IDR5_VAX (1 << 10) I think this is actually a two-bit field, so we should check the whole thing. > > #define ARM_SMMU_CR0 0x20 > #define CR0_CMDQEN (1 << 3) > @@ -604,6 +605,7 @@ struct arm_smmu_device { > #define ARM_SMMU_FEAT_STALLS (1 << 11) > #define ARM_SMMU_FEAT_HYP (1 << 12) > #define ARM_SMMU_FEAT_STALL_FORCE (1 << 13) > +#define ARM_SMMU_FEAT_VAX (1 << 14) > u32 features; > > #define ARM_SMMU_OPT_SKIP_PREFETCH (1 << 0) > @@ -1656,6 +1658,8 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain) > switch (smmu_domain->stage) { > case ARM_SMMU_DOMAIN_S1: > ias = VA_BITS; > + if (VA_BITS > 48 && !(smmu->features & ARM_SMMU_FEAT_VAX)) nit, but I'd rather the if condition was checking ias instead of VA_BITS. > + ias = 48; > oas = smmu->ias; Should we also be sanitising the oas here if the SMMU doesn't support 64k pages? It looks like the ias/oas sanitisation isn't cleanly split between the pgtable code and the SMMU driver. Will
On 26/02/18 18:05, Will Deacon wrote: > On Thu, Dec 14, 2017 at 04:58:53PM +0000, Robin Murphy wrote: >> Stage 1 input addresses are effectively 64-bit in SMMUv3 anyway, so >> really all that's involved is letting io-pgtable know the appropriate >> upper bound for T0SZ. >> >> Tested-by: Nate Watterson <nwatters@codeaurora.org> >> Signed-off-by: Robin Murphy <robin.murphy@arm.com> >> --- >> >> v2: No change >> >> drivers/iommu/arm-smmu-v3.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c >> index c9c4e6132e27..7059a0805bd1 100644 >> --- a/drivers/iommu/arm-smmu-v3.c >> +++ b/drivers/iommu/arm-smmu-v3.c >> @@ -102,6 +102,7 @@ >> #define IDR5_OAS_44_BIT (4 << IDR5_OAS_SHIFT) >> #define IDR5_OAS_48_BIT (5 << IDR5_OAS_SHIFT) >> #define IDR5_OAS_52_BIT (6 << IDR5_OAS_SHIFT) >> +#define IDR5_VAX (1 << 10) > > I think this is actually a two-bit field, so we should check the whole > thing. Yes, I either overlooked that or took a cheeky shortcut; will fix. >> >> #define ARM_SMMU_CR0 0x20 >> #define CR0_CMDQEN (1 << 3) >> @@ -604,6 +605,7 @@ struct arm_smmu_device { >> #define ARM_SMMU_FEAT_STALLS (1 << 11) >> #define ARM_SMMU_FEAT_HYP (1 << 12) >> #define ARM_SMMU_FEAT_STALL_FORCE (1 << 13) >> +#define ARM_SMMU_FEAT_VAX (1 << 14) >> u32 features; >> >> #define ARM_SMMU_OPT_SKIP_PREFETCH (1 << 0) >> @@ -1656,6 +1658,8 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain) >> switch (smmu_domain->stage) { >> case ARM_SMMU_DOMAIN_S1: >> ias = VA_BITS; >> + if (VA_BITS > 48 && !(smmu->features & ARM_SMMU_FEAT_VAX)) > > nit, but I'd rather the if condition was checking ias instead of VA_BITS. > >> + ias = 48; >> oas = smmu->ias; > > Should we also be sanitising the oas here if the SMMU doesn't support 64k > pages? It looks like the ias/oas sanitisation isn't cleanly split between > the pgtable code and the SMMU driver. Right, based on my previous argument it would be cleaner to set 48/52 here based on the SMMU features, then constrain to VA_BITS in io-pgtable; I think I was more focused on minimising the diff initially. Robin.
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index c9c4e6132e27..7059a0805bd1 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -102,6 +102,7 @@ #define IDR5_OAS_44_BIT (4 << IDR5_OAS_SHIFT) #define IDR5_OAS_48_BIT (5 << IDR5_OAS_SHIFT) #define IDR5_OAS_52_BIT (6 << IDR5_OAS_SHIFT) +#define IDR5_VAX (1 << 10) #define ARM_SMMU_CR0 0x20 #define CR0_CMDQEN (1 << 3) @@ -604,6 +605,7 @@ struct arm_smmu_device { #define ARM_SMMU_FEAT_STALLS (1 << 11) #define ARM_SMMU_FEAT_HYP (1 << 12) #define ARM_SMMU_FEAT_STALL_FORCE (1 << 13) +#define ARM_SMMU_FEAT_VAX (1 << 14) u32 features; #define ARM_SMMU_OPT_SKIP_PREFETCH (1 << 0) @@ -1656,6 +1658,8 @@ static int arm_smmu_domain_finalise(struct iommu_domain *domain) switch (smmu_domain->stage) { case ARM_SMMU_DOMAIN_S1: ias = VA_BITS; + if (VA_BITS > 48 && !(smmu->features & ARM_SMMU_FEAT_VAX)) + ias = 48; oas = smmu->ias; fmt = ARM_64_LPAE_S1; finalise_stage_fn = arm_smmu_domain_finalise_s1; @@ -2694,6 +2698,10 @@ static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu) if (reg & IDR5_GRAN4K) smmu->pgsize_bitmap |= SZ_4K | SZ_2M | SZ_1G; + /* Input address size */ + if (reg & IDR5_VAX) + smmu->features |= ARM_SMMU_FEAT_VAX; + /* Output address size */ switch (reg & IDR5_OAS_MASK << IDR5_OAS_SHIFT) { case IDR5_OAS_32_BIT: