Message ID | 1398089589-9181-4-git-send-email-laurent.pinchart+renesas@ideasonboard.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hello. On 04/21/2014 06:13 PM, Laurent Pinchart wrote: > The PTRS_PER_(PGD|PMD|PTE) macros evaluate to different values depending > on whether LPAE is enabled. The IPMMU driver uses a long descriptor > format regardless of LPAE, making those macros mismatch the IPMMU > configuration on non-LPAE systems. > Replace the macros by driver-specific versions that always evaluate to > the right value. > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> > --- > drivers/iommu/ipmmu-vmsa.c | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c > index 1ae97d8..7c8c21e 100644 > --- a/drivers/iommu/ipmmu-vmsa.c > +++ b/drivers/iommu/ipmmu-vmsa.c > @@ -210,6 +210,10 @@ static LIST_HEAD(ipmmu_devices); > #define ARM_VMSA_PTE_MEMATTR_NC (((pteval_t)0x5) << 2) > #define ARM_VMSA_PTE_MEMATTR_DEV (((pteval_t)0x1) << 2) > > +#define IPMMU_PTRS_PER_PTE 512 > +#define IPMMU_PTRS_PER_PMD 512 > +#define IPMMU_PTRS_PER_PGD 4 > + [...] > @@ -487,7 +491,7 @@ static void ipmmu_free_puds(pgd_t *pgd) > unsigned int i; > > pud = pud_base; > - for (i = 0; i < PTRS_PER_PUD; ++i) { > + for (i = 0; i < IPMMU_PTRS_PER_PUD; ++i) { I don't see where you #define IPMMU_PTRS_PER_PUD... [...] WBR, Sergei
Hi Sergei, Thank you for the review. On Monday 21 April 2014 23:05:00 Sergei Shtylyov wrote: > On 04/21/2014 06:13 PM, Laurent Pinchart wrote: > > The PTRS_PER_(PGD|PMD|PTE) macros evaluate to different values depending > > on whether LPAE is enabled. The IPMMU driver uses a long descriptor > > format regardless of LPAE, making those macros mismatch the IPMMU > > configuration on non-LPAE systems. > > > > Replace the macros by driver-specific versions that always evaluate to > > the right value. > > > > Signed-off-by: Laurent Pinchart > > <laurent.pinchart+renesas@ideasonboard.com> > > --- > > > > drivers/iommu/ipmmu-vmsa.c | 14 +++++++++----- > > 1 file changed, 9 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c > > index 1ae97d8..7c8c21e 100644 > > --- a/drivers/iommu/ipmmu-vmsa.c > > +++ b/drivers/iommu/ipmmu-vmsa.c > > @@ -210,6 +210,10 @@ static LIST_HEAD(ipmmu_devices); > > > > #define ARM_VMSA_PTE_MEMATTR_NC (((pteval_t)0x5) << 2) > > #define ARM_VMSA_PTE_MEMATTR_DEV (((pteval_t)0x1) << 2) > > > > +#define IPMMU_PTRS_PER_PTE 512 > > +#define IPMMU_PTRS_PER_PMD 512 > > +#define IPMMU_PTRS_PER_PGD 4 > > + > > [...] > > > @@ -487,7 +491,7 @@ static void ipmmu_free_puds(pgd_t *pgd) > > > > unsigned int i; > > > > pud = pud_base; > > > > - for (i = 0; i < PTRS_PER_PUD; ++i) { > > + for (i = 0; i < IPMMU_PTRS_PER_PUD; ++i) { > > I don't see where you #define IPMMU_PTRS_PER_PUD... Come on, it's easy. I don't define it anywhere :-) The line is changed in a further patch, it seems I've forgotten to compile- test all intermediate versions. Thank you for catching the problem, I'll fix that in v2 and will make sure to properly test all patches individually.
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c index 1ae97d8..7c8c21e 100644 --- a/drivers/iommu/ipmmu-vmsa.c +++ b/drivers/iommu/ipmmu-vmsa.c @@ -210,6 +210,10 @@ static LIST_HEAD(ipmmu_devices); #define ARM_VMSA_PTE_MEMATTR_NC (((pteval_t)0x5) << 2) #define ARM_VMSA_PTE_MEMATTR_DEV (((pteval_t)0x1) << 2) +#define IPMMU_PTRS_PER_PTE 512 +#define IPMMU_PTRS_PER_PMD 512 +#define IPMMU_PTRS_PER_PGD 4 + /* ----------------------------------------------------------------------------- * Read/Write Access */ @@ -328,7 +332,7 @@ static int ipmmu_domain_init_context(struct ipmmu_vmsa_domain *domain) /* TTBR0 */ ipmmu_flush_pgtable(domain->mmu, domain->pgd, - PTRS_PER_PGD * sizeof(*domain->pgd)); + IPMMU_PTRS_PER_PGD * sizeof(*domain->pgd)); ttbr = __pa(domain->pgd); ipmmu_ctx_write(domain, IMTTLBR0, ttbr); ipmmu_ctx_write(domain, IMTTUBR0, ttbr >> 32); @@ -470,7 +474,7 @@ static void ipmmu_free_pmds(pud_t *pud) unsigned int i; pmd = pmd_base; - for (i = 0; i < PTRS_PER_PMD; ++i) { + for (i = 0; i < IPMMU_PTRS_PER_PMD; ++i) { if (pmd_none(*pmd)) continue; @@ -487,7 +491,7 @@ static void ipmmu_free_puds(pgd_t *pgd) unsigned int i; pud = pud_base; - for (i = 0; i < PTRS_PER_PUD; ++i) { + for (i = 0; i < IPMMU_PTRS_PER_PUD; ++i) { if (pud_none(*pud)) continue; @@ -510,7 +514,7 @@ static void ipmmu_free_pgtables(struct ipmmu_vmsa_domain *domain) * tables. */ pgd = pgd_base; - for (i = 0; i < PTRS_PER_PGD; ++i) { + for (i = 0; i < IPMMU_PTRS_PER_PGD; ++i) { if (pgd_none(*pgd)) continue; ipmmu_free_puds(pgd); @@ -695,7 +699,7 @@ static int ipmmu_domain_init(struct iommu_domain *io_domain) spin_lock_init(&domain->lock); - domain->pgd = kzalloc(PTRS_PER_PGD * sizeof(pgd_t), GFP_KERNEL); + domain->pgd = kzalloc(IPMMU_PTRS_PER_PGD * sizeof(pgd_t), GFP_KERNEL); if (!domain->pgd) { kfree(domain); return -ENOMEM;
The PTRS_PER_(PGD|PMD|PTE) macros evaluate to different values depending on whether LPAE is enabled. The IPMMU driver uses a long descriptor format regardless of LPAE, making those macros mismatch the IPMMU configuration on non-LPAE systems. Replace the macros by driver-specific versions that always evaluate to the right value. Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com> --- drivers/iommu/ipmmu-vmsa.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)