All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Cc: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>,
	iommu@lists.linux-foundation.org, linux-sh@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Will Deacon <will.deacon@arm.com>
Subject: Re: [PATCH 3/9] iommu/ipmmu-vmsa: Define driver-specific page directory sizes
Date: Mon, 21 Apr 2014 22:52:14 +0200	[thread overview]
Message-ID: <11850321.Q0Kc9vhtF9@avalon> (raw)
In-Reply-To: <53556BDC.6040705@cogentembedded.com>

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.

-- 
Regards,

Laurent Pinchart


WARNING: multiple messages have this Message-ID (diff)
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 3/9] iommu/ipmmu-vmsa: Define driver-specific page directory sizes
Date: Mon, 21 Apr 2014 20:52:14 +0000	[thread overview]
Message-ID: <11850321.Q0Kc9vhtF9@avalon> (raw)
In-Reply-To: <53556BDC.6040705@cogentembedded.com>

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.

-- 
Regards,

Laurent Pinchart


WARNING: multiple messages have this Message-ID (diff)
From: laurent.pinchart@ideasonboard.com (Laurent Pinchart)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/9] iommu/ipmmu-vmsa: Define driver-specific page directory sizes
Date: Mon, 21 Apr 2014 22:52:14 +0200	[thread overview]
Message-ID: <11850321.Q0Kc9vhtF9@avalon> (raw)
In-Reply-To: <53556BDC.6040705@cogentembedded.com>

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.

-- 
Regards,

Laurent Pinchart

  reply	other threads:[~2014-04-21 20:52 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-21 14:13 [PATCH 0/9] Renesas ipmmu-vmsa: Miscellaneous cleanups and fixes Laurent Pinchart
2014-04-21 14:13 ` Laurent Pinchart
2014-04-21 14:13 ` Laurent Pinchart
     [not found] ` <1398089589-9181-1-git-send-email-laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
2014-04-21 14:13   ` [PATCH 1/9] iommu/ipmmu-vmsa: Cleanup failures of ARM mapping creation or attachment Laurent Pinchart
2014-04-21 14:13     ` Laurent Pinchart
2014-04-21 14:13     ` Laurent Pinchart
2014-04-21 14:13   ` [PATCH 4/9] iommu/ipmmu-vmsa: Set the PTE contiguous hint bit when possible Laurent Pinchart
2014-04-21 14:13     ` Laurent Pinchart
2014-04-21 14:13     ` Laurent Pinchart
2014-04-21 14:13 ` [PATCH 2/9] iommu/ipmmu-vmsa: Fix the supported page sizes Laurent Pinchart
2014-04-21 14:13   ` Laurent Pinchart
2014-04-21 14:13   ` Laurent Pinchart
2014-04-21 14:13 ` [PATCH 3/9] iommu/ipmmu-vmsa: Define driver-specific page directory sizes Laurent Pinchart
2014-04-21 14:13   ` Laurent Pinchart
2014-04-21 14:13   ` Laurent Pinchart
     [not found]   ` <1398089589-9181-4-git-send-email-laurent.pinchart+renesas-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
2014-04-21 19:05     ` Sergei Shtylyov
2014-04-21 19:05       ` Sergei Shtylyov
2014-04-21 19:05       ` Sergei Shtylyov
2014-04-21 20:52       ` Laurent Pinchart [this message]
2014-04-21 20:52         ` Laurent Pinchart
2014-04-21 20:52         ` Laurent Pinchart
2014-04-21 14:13 ` [PATCH 5/9] iommu/ipmmu-vmsa: PMD is never folded, PUD always is Laurent Pinchart
2014-04-21 14:13   ` Laurent Pinchart
2014-04-21 14:13   ` Laurent Pinchart
2014-04-21 14:13 ` [PATCH 6/9] iommu/ipmmu-vmsa: Rewrite page table management Laurent Pinchart
2014-04-21 14:13   ` Laurent Pinchart
2014-04-21 14:13   ` Laurent Pinchart
2014-04-21 14:13 ` [PATCH 7/9] iommu/ipmmu-vmsa: Support 2MB mappings Laurent Pinchart
2014-04-21 14:13   ` Laurent Pinchart
2014-04-21 14:13   ` Laurent Pinchart
2014-04-21 14:13 ` [PATCH 8/9] iommu/ipmmu-vmsa: Remove stage 2 PTE bits definitions Laurent Pinchart
2014-04-21 14:13   ` Laurent Pinchart
2014-04-21 14:13   ` Laurent Pinchart
2014-04-21 14:13 ` [PATCH 9/9] iommu/ipmmu-vmsa: Support clearing mappings Laurent Pinchart
2014-04-21 14:13   ` Laurent Pinchart
2014-04-21 14:13   ` Laurent Pinchart
2014-04-22 11:34 ` [PATCH 0/9] Renesas ipmmu-vmsa: Miscellaneous cleanups and fixes Will Deacon
2014-04-22 11:34   ` Will Deacon
2014-04-22 11:34   ` Will Deacon
2014-04-22 11:44   ` Laurent Pinchart
2014-04-22 11:44     ` Laurent Pinchart
2014-04-22 11:44     ` Laurent Pinchart

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=11850321.Q0Kc9vhtF9@avalon \
    --to=laurent.pinchart@ideasonboard.com \
    --cc=iommu@lists.linux-foundation.org \
    --cc=laurent.pinchart+renesas@ideasonboard.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-sh@vger.kernel.org \
    --cc=sergei.shtylyov@cogentembedded.com \
    --cc=will.deacon@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.