| Message ID | f4dccad78815ca0a2dd7926be7052759d099b920.1565369764.git.robin.murphy@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Arm SMMU refactoring | expand |
On Fri, Aug 09, 2019 at 06:07:41PM +0100, Robin Murphy wrote: > To keep register-access quirks manageable, we want to structure things > to avoid needing too many individual overrides. It seems fairly clean to > have a single interface which handles both global and context registers > in terms of the architectural pages, so the first preparatory step is to > rework cb_base into a page number rather than an absolute address. > > Signed-off-by: Robin Murphy <robin.murphy@arm.com> > --- > drivers/iommu/arm-smmu.c | 22 ++++++++++++---------- > 1 file changed, 12 insertions(+), 10 deletions(-) > > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c > index d9a93e5f422f..463bc8d98adb 100644 > --- a/drivers/iommu/arm-smmu.c > +++ b/drivers/iommu/arm-smmu.c > @@ -95,7 +95,7 @@ > #endif > > /* Translation context bank */ > -#define ARM_SMMU_CB(smmu, n) ((smmu)->cb_base + ((n) << (smmu)->pgshift)) > +#define ARM_SMMU_CB(smmu, n) ((smmu)->base + (((smmu)->cb_base + (n)) << (smmu)->pgshift)) > > #define MSI_IOVA_BASE 0x8000000 > #define MSI_IOVA_LENGTH 0x100000 > @@ -168,8 +168,8 @@ struct arm_smmu_device { > struct device *dev; > > void __iomem *base; > - void __iomem *cb_base; > - unsigned long pgshift; > + unsigned int cb_base; I think this is now a misnomer. Would you be able to rename it cb_pfn or something, please? Otherwise, this seems fine. Will
On 14/08/2019 19:05, Will Deacon wrote: > On Fri, Aug 09, 2019 at 06:07:41PM +0100, Robin Murphy wrote: >> To keep register-access quirks manageable, we want to structure things >> to avoid needing too many individual overrides. It seems fairly clean to >> have a single interface which handles both global and context registers >> in terms of the architectural pages, so the first preparatory step is to >> rework cb_base into a page number rather than an absolute address. >> >> Signed-off-by: Robin Murphy <robin.murphy@arm.com> >> --- >> drivers/iommu/arm-smmu.c | 22 ++++++++++++---------- >> 1 file changed, 12 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c >> index d9a93e5f422f..463bc8d98adb 100644 >> --- a/drivers/iommu/arm-smmu.c >> +++ b/drivers/iommu/arm-smmu.c >> @@ -95,7 +95,7 @@ >> #endif >> >> /* Translation context bank */ >> -#define ARM_SMMU_CB(smmu, n) ((smmu)->cb_base + ((n) << (smmu)->pgshift)) >> +#define ARM_SMMU_CB(smmu, n) ((smmu)->base + (((smmu)->cb_base + (n)) << (smmu)->pgshift)) >> >> #define MSI_IOVA_BASE 0x8000000 >> #define MSI_IOVA_LENGTH 0x100000 >> @@ -168,8 +168,8 @@ struct arm_smmu_device { >> struct device *dev; >> >> void __iomem *base; >> - void __iomem *cb_base; >> - unsigned long pgshift; >> + unsigned int cb_base; > > I think this is now a misnomer. Would you be able to rename it cb_pfn or > something, please? Good point; in the architectural terms (section 8.1 of the spec), SMMU_CB_BASE is strictly a byte offset from SMMU_BASE, and the quantity we now have here is actually NUMPAGE. I've renamed it as such and tweaked the comments to be a bit more useful too. Robin.
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index d9a93e5f422f..463bc8d98adb 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -95,7 +95,7 @@ #endif /* Translation context bank */ -#define ARM_SMMU_CB(smmu, n) ((smmu)->cb_base + ((n) << (smmu)->pgshift)) +#define ARM_SMMU_CB(smmu, n) ((smmu)->base + (((smmu)->cb_base + (n)) << (smmu)->pgshift)) #define MSI_IOVA_BASE 0x8000000 #define MSI_IOVA_LENGTH 0x100000 @@ -168,8 +168,8 @@ struct arm_smmu_device { struct device *dev; void __iomem *base; - void __iomem *cb_base; - unsigned long pgshift; + unsigned int cb_base; + unsigned int pgshift; #define ARM_SMMU_FEAT_COHERENT_WALK (1 << 0) #define ARM_SMMU_FEAT_STREAM_MATCH (1 << 1) @@ -1815,7 +1815,7 @@ static int arm_smmu_id_size_to_bits(int size) static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu) { - unsigned long size; + unsigned int size; void __iomem *gr0_base = ARM_SMMU_GR0(smmu); u32 id; bool cttw_reg, cttw_fw = smmu->features & ARM_SMMU_FEAT_COHERENT_WALK; @@ -1899,7 +1899,7 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu) return -ENOMEM; dev_notice(smmu->dev, - "\tstream matching with %lu register groups", size); + "\tstream matching with %u register groups", size); } /* s2cr->type == 0 means translation, so initialise explicitly */ smmu->s2crs = devm_kmalloc_array(smmu->dev, size, sizeof(*smmu->s2crs), @@ -1925,11 +1925,12 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu) /* Check for size mismatch of SMMU address space from mapped region */ size = 1 << (FIELD_GET(ID1_NUMPAGENDXB, id) + 1); - size <<= smmu->pgshift; - if (smmu->cb_base != gr0_base + size) + if (smmu->cb_base != 2 * size << smmu->pgshift) dev_warn(smmu->dev, - "SMMU address space size (0x%lx) differs from mapped region size (0x%tx)!\n", - size * 2, (smmu->cb_base - gr0_base) * 2); + "SMMU address space size (0x%x) differs from mapped region size (0x%x)!\n", + 2 * size << smmu->pgshift, smmu->cb_base); + /* Now cb_base can reach its final form: a page number */ + smmu->cb_base = size; smmu->num_s2_context_banks = FIELD_GET(ID1_NUMS2CB, id); smmu->num_context_banks = FIELD_GET(ID1_NUMCB, id); @@ -2200,7 +2201,8 @@ static int arm_smmu_device_probe(struct platform_device *pdev) smmu->base = devm_ioremap_resource(dev, res); if (IS_ERR(smmu->base)) return PTR_ERR(smmu->base); - smmu->cb_base = smmu->base + resource_size(res) / 2; + /* We'll finish calculating this later once we know the page size */ + smmu->cb_base = resource_size(res); num_irqs = 0; while ((res = platform_get_resource(pdev, IORESOURCE_IRQ, num_irqs))) {
To keep register-access quirks manageable, we want to structure things to avoid needing too many individual overrides. It seems fairly clean to have a single interface which handles both global and context registers in terms of the architectural pages, so the first preparatory step is to rework cb_base into a page number rather than an absolute address. Signed-off-by: Robin Murphy <robin.murphy@arm.com> --- drivers/iommu/arm-smmu.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-)