[PATCH] drivers: cma: fix addressing on PAE machines

[PATCH] drivers: cma: fix addressing on PAE machines

[Vitaly Andrianov]

This patch fixes a couple of bugs that otherwise impair CMA functionality on
PAE machines:

  - alignment must be a 64-bit type when running on systems with 64-bit
    physical addresses.  If this is not the case, the limit calculation thunks
    allocations down to an address range < 4G.

  - The allocated range check is removed. On 32bit ARM kernel with LPAE
    enabled the base may be allocated outside the fist 4GB of physical
    memory (keystone SoC for example).

Signed-off-by: Vitaly Andrianov <vitalya@ti.com>
Signed-off-by: Cyril Chemparathy <cyril@ti.com>
---
 drivers/base/dma-contiguous.c |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
index 9a14694..7936b2e 100644
--- a/drivers/base/dma-contiguous.c
+++ b/drivers/base/dma-contiguous.c
@@ -234,7 +234,7 @@ int __init dma_declare_contiguous(struct device *dev, unsigned long size,
 				  phys_addr_t base, phys_addr_t limit)
 {
 	struct cma_reserved *r = &cma_reserved[cma_reserved_count];
-	unsigned long alignment;
+	phys_addr_t alignment;
 
 	pr_debug("%s(size %lx, base %08lx, limit %08lx)\n", __func__,
 		 (unsigned long)size, (unsigned long)base,
@@ -271,10 +271,6 @@ int __init dma_declare_contiguous(struct device *dev, unsigned long size,
 		if (!addr) {
 			base = -ENOMEM;
 			goto err;
-		} else if (addr + size > ~(unsigned long)0) {
-			memblock_free(addr, size);
-			base = -EINVAL;
-			goto err;
 		} else {
 			base = addr;
 		}
-- 
1.7.9.5

Re: [PATCH] drivers: cma: fix addressing on PAE machines

[Michal Nazarewicz]

[-- Attachment #1: Type: text/plain, Size: 2280 bytes --]


On Fri, Nov 30 2012, Vitaly Andrianov <vitalya@ti.com> wrote:
> This patch fixes a couple of bugs that otherwise impair CMA functionality on
> PAE machines:
>
>   - alignment must be a 64-bit type when running on systems with 64-bit
>     physical addresses.  If this is not the case, the limit calculation thunks
>     allocations down to an address range < 4G.
>
>   - The allocated range check is removed. On 32bit ARM kernel with LPAE
>     enabled the base may be allocated outside the fist 4GB of physical
>     memory (keystone SoC for example).
>
> Signed-off-by: Vitaly Andrianov <vitalya@ti.com>
> Signed-off-by: Cyril Chemparathy <cyril@ti.com>

To be consistent, you should also change the type in:

static long size_cmdline = -1;
static const unsigned long size_bytes = CMA_SIZE_MBYTES * SZ_1M;
static unsigned long __init __maybe_unused cma_early_percent_memory(void)

and in

void __init dma_contiguous_reserve(phys_addr_t limit)

> ---
>  drivers/base/dma-contiguous.c |    6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
> index 9a14694..7936b2e 100644
> --- a/drivers/base/dma-contiguous.c
> +++ b/drivers/base/dma-contiguous.c
> @@ -234,7 +234,7 @@ int __init dma_declare_contiguous(struct device *dev, unsigned long size,
>  				  phys_addr_t base, phys_addr_t limit)
>  {
>  	struct cma_reserved *r = &cma_reserved[cma_reserved_count];
> -	unsigned long alignment;
> +	phys_addr_t alignment;
>  
>  	pr_debug("%s(size %lx, base %08lx, limit %08lx)\n", __func__,
>  		 (unsigned long)size, (unsigned long)base,
> @@ -271,10 +271,6 @@ int __init dma_declare_contiguous(struct device *dev, unsigned long size,
>  		if (!addr) {
>  			base = -ENOMEM;
>  			goto err;
> -		} else if (addr + size > ~(unsigned long)0) {
> -			memblock_free(addr, size);
> -			base = -EINVAL;
> -			goto err;
>  		} else {
>  			base = addr;
>  		}
> -- 
> 1.7.9.5
>

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]

RE: [PATCH] drivers: cma: fix addressing on PAE machines

[Andrianov, Vitaly]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3086 bytes --]



> -----Original Message-----
> From: Michal Nazarewicz [mailto:mpn@google.com] On Behalf Of Michal
> Nazarewicz
> Sent: Friday, November 30, 2012 10:01 AM
> To: Andrianov, Vitaly; m.szyprowski@samsung.com;
> kyungmin.park@samsung.com; arnd@arndb.de; linux-kernel@vger.kernel.org;
> linux-keystone@list.ti.com - Linux developers for Keystone family of
> devices (May contain non-TIers)
> Cc: Andrianov, Vitaly; Chemparathy, Cyril
> Subject: Re: [PATCH] drivers: cma: fix addressing on PAE machines
> 
> 
> On Fri, Nov 30 2012, Vitaly Andrianov <vitalya@ti.com> wrote:
> > This patch fixes a couple of bugs that otherwise impair CMA
> > functionality on PAE machines:
> >
> >   - alignment must be a 64-bit type when running on systems with 64-
> bit
> >     physical addresses.  If this is not the case, the limit
> calculation thunks
> >     allocations down to an address range < 4G.
> >
> >   - The allocated range check is removed. On 32bit ARM kernel with
> LPAE
> >     enabled the base may be allocated outside the fist 4GB of
> physical
> >     memory (keystone SoC for example).
> >
> > Signed-off-by: Vitaly Andrianov <vitalya@ti.com>
> > Signed-off-by: Cyril Chemparathy <cyril@ti.com>
> 
> To be consistent, you should also change the type in:
> 
> static long size_cmdline = -1;
> static const unsigned long size_bytes = CMA_SIZE_MBYTES * SZ_1M; static
> unsigned long __init __maybe_unused cma_early_percent_memory(void)
> 
> and in
> 
> void __init dma_contiguous_reserve(phys_addr_t limit)
> 

Can you elaborate why we need to change type for size? 

> > ---
> >  drivers/base/dma-contiguous.c |    6 +-----
> >  1 file changed, 1 insertion(+), 5 deletions(-)
> >
> > diff --git a/drivers/base/dma-contiguous.c
> > b/drivers/base/dma-contiguous.c index 9a14694..7936b2e 100644
> > --- a/drivers/base/dma-contiguous.c
> > +++ b/drivers/base/dma-contiguous.c
> > @@ -234,7 +234,7 @@ int __init dma_declare_contiguous(struct device
> *dev, unsigned long size,
> >  				  phys_addr_t base, phys_addr_t limit)  {
> >  	struct cma_reserved *r = &cma_reserved[cma_reserved_count];
> > -	unsigned long alignment;
> > +	phys_addr_t alignment;
> >
> >  	pr_debug("%s(size %lx, base %08lx, limit %08lx)\n", __func__,
> >  		 (unsigned long)size, (unsigned long)base, @@ -271,10 +271,6
> @@ int
> > __init dma_declare_contiguous(struct device *dev, unsigned long size,
> >  		if (!addr) {
> >  			base = -ENOMEM;
> >  			goto err;
> > -		} else if (addr + size > ~(unsigned long)0) {
> > -			memblock_free(addr, size);
> > -			base = -EINVAL;
> > -			goto err;
> >  		} else {
> >  			base = addr;
> >  		}
> > --
> > 1.7.9.5
> >
> 
> --
> Best regards,                                         _     _
> .o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
> ..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
> ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

Thanks,
Vitaly
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH] drivers: cma: fix addressing on PAE machines

[Michal Nazarewicz]

[-- Attachment #1: Type: text/plain, Size: 2849 bytes --]

>> On Fri, Nov 30 2012, Vitaly Andrianov <vitalya@ti.com> wrote:
>> > This patch fixes a couple of bugs that otherwise impair CMA
>> > functionality on PAE machines:
>> >
>> >   - alignment must be a 64-bit type when running on systems with 64-
>> bit
>> >     physical addresses.  If this is not the case, the limit
>> calculation thunks
>> >     allocations down to an address range < 4G.
>> >
>> >   - The allocated range check is removed. On 32bit ARM kernel with
>> LPAE
>> >     enabled the base may be allocated outside the fist 4GB of
>> physical
>> >     memory (keystone SoC for example).
>> >
>> > Signed-off-by: Vitaly Andrianov <vitalya@ti.com>
>> > Signed-off-by: Cyril Chemparathy <cyril@ti.com>
>> 
>> To be consistent, you should also change the type in:
>> 
>> static long size_cmdline = -1;
>> static const unsigned long size_bytes = CMA_SIZE_MBYTES * SZ_1M; static
>> unsigned long __init __maybe_unused cma_early_percent_memory(void)
>> 
>> and in
>> 
>> void __init dma_contiguous_reserve(phys_addr_t limit)

On Fri, Nov 30 2012, Andrianov, Vitaly wrote:
> Can you elaborate why we need to change type for size? 

Like I've wrote, for consistency.  The size should use the same type as
alignment.  This most likely won't affect the way you use the code, but
it's the right thing to do.  It should be done that way from the start,
so it's really my mistake, but since you are touching the code, it'd
best to have the changes contained in a single commit.  Alternatively
I can just send my own patch with all those fixes (including yours).

>> > diff --git a/drivers/base/dma-contiguous.c
>> > b/drivers/base/dma-contiguous.c index 9a14694..7936b2e 100644
>> > --- a/drivers/base/dma-contiguous.c
>> > +++ b/drivers/base/dma-contiguous.c
>> > @@ -234,7 +234,7 @@ int __init dma_declare_contiguous(struct device
>> *dev, unsigned long size,
>> >  				  phys_addr_t base, phys_addr_t limit)  {
>> >  	struct cma_reserved *r = &cma_reserved[cma_reserved_count];
>> > -	unsigned long alignment;
>> > +	phys_addr_t alignment;
>> >
>> >  	pr_debug("%s(size %lx, base %08lx, limit %08lx)\n", __func__,
>> >  		 (unsigned long)size, (unsigned long)base, @@ -271,10 +271,6
>> @@ int
>> > __init dma_declare_contiguous(struct device *dev, unsigned long size,
>> >  		if (!addr) {
>> >  			base = -ENOMEM;
>> >  			goto err;
>> > -		} else if (addr + size > ~(unsigned long)0) {
>> > -			memblock_free(addr, size);
>> > -			base = -EINVAL;
>> > -			goto err;
>> >  		} else {
>> >  			base = addr;
>> >  		}

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]



[-- Attachment #2.2: Type: application/pgp-signature, Size: 835 bytes --]