[regression] in linux-next: sh_mobile_ceu_camera broken by "ARM: Prohibit ioremap() on kernel managed RAM"

[regression] in linux-next: sh_mobile_ceu_camera broken by "ARM: Prohibit ioremap() on kernel managed RAM"

[Arnd Hannemann]

Hi Russell,

your commit 309caa9cc6ff39d261264ec4ff10e29489afc8f8
ARM: Prohibit ioremap() on kernel managed RAM

lets drivers/base/dma-coherent.c::dma_declare_coherent_memory()
fail for the sh_mobile_ceu_camera driver (see backtrace below).
I think, other configurations (i.MX31 users of the mx3_camera driver:
pcm037 and mx31moboard) will have the same problem.

Since I have no idea how to fix this, I post this regression report here...

[   49.703125] ------------[ cut here ]------------
[   49.703125] WARNING: at arch/arm/mm/ioremap.c:207 __arm_ioremap_pfn_caller+0x50/0x18c()
[   49.710937] Modules linked in: sh_mobile_ceu_camera(+) videobuf_dma_contig sh_mobile_csi2 soc_camera videobuf_core v4l2_common videodev v4l1_compat soc_mediabus tmio_mmc
[   49.726562] Backtrace:
[   49.734375] [<c0029044>] (dump_backtrace+0x0/0x110) from [<c01ffae4>] (dump_stack+0x18/0x1c)
[   49.742187]  r6:c025bab7 r5:000000cf r4:00000000 r3:c029eebc
[   49.742187] [<c01ffacc>] (dump_stack+0x0/0x1c) from [<c003525c>] (warn_slowpath_common+0x54/0x6c)
[   49.750000] [<c0035208>] (warn_slowpath_common+0x0/0x6c) from [<c0035298>] (warn_slowpath_null+0x24/0x2c)
[   49.757812]  r8:d09de000 r7:00000000 r6:00000000 r5:0004e000 r4:00800000
[   49.765625] r3:00000009
[   49.765625] [<c0035274>] (warn_slowpath_null+0x0/0x2c) from [<c002bb94>] (__arm_ioremap_pfn_caller+0x50/0x18c)
[   49.773437] [<c002bb44>] (__arm_ioremap_pfn_caller+0x0/0x18c) from [<c002bd50>] (__arm_ioremap_caller+0x60/0x68)
[   49.781250] [<c002bcf0>] (__arm_ioremap_caller+0x0/0x68) from [<c002bd6c>] (__arm_ioremap+0x14/0x18)
[   49.789062]  r5:4e000000 r4:c029d9c0
[   49.789062] [<c002bd58>] (__arm_ioremap+0x0/0x18) from [<c0158354>] (dma_declare_coherent_memory+0x44/0xe8)
[   49.796875] [<c0158310>] (dma_declare_coherent_memory+0x0/0xe8) from [<bf059834>] (sh_mobile_ceu_probe+0x1d4/0x3dc [sh_mobile_ceu_camera])
[   49.804687] [<bf059660>] (sh_mobile_ceu_probe+0x0/0x3dc [sh_mobile_ceu_camera]) from [<c0155f30>] (platform_drv_probe+0x1c/0x20)
[   49.812500] [<c0155f14>] (platform_drv_probe+0x0/0x20) from [<c0154f24>] (driver_probe_device+0xb0/0x160)
[   49.820312] [<c0154e74>] (driver_probe_device+0x0/0x160) from [<c015503c>] (__driver_attach+0x68/0x8c)
[   49.828125]  r7:00000000 r6:bf059e60 r5:c029d9f4 r4:c029d9c0
[   49.835937] [<c0154fd4>] (__driver_attach+0x0/0x8c) from [<c0154744>] (bus_for_each_dev+0x54/0x84)
[   49.843750]  r6:00000000 r5:c0154fd4 r4:bf059e60 r3:00000000
[   49.843750] [<c01546f0>] (bus_for_each_dev+0x0/0x84) from [<c0154d88>] (driver_attach+0x20/0x28)
[   49.851562]  r6:c02b22b0 r5:cf0be5a0 r4:bf059e60
[   49.851562] [<c0154d68>] (driver_attach+0x0/0x28) from [<c0154018>] (bus_add_driver+0xa8/0x228)
[   49.859375] [<c0153f70>] (bus_add_driver+0x0/0x228) from [<c0155354>] (driver_register+0xb0/0x140)
[   49.867187] [<c01552a4>] (driver_register+0x0/0x140) from [<c01561d4>] (platform_driver_register+0x4c/0x60)
[   49.875000]  r8:c0025fe4 r7:00028c36 r6:00000000 r5:00000000 r4:bf05d000
[   49.882812] r3:00000000
[   49.882812] [<c0156188>] (platform_driver_register+0x0/0x60) from [<bf05d020>] (sh_mobile_ceu_init+0x20/0x2c [sh_mobile_ceu_camera])
[   49.890625] [<bf05d000>] (sh_mobile_ceu_init+0x0/0x2c [sh_mobile_ceu_camera]) from [<c00253a8>] (do_one_initcall+0x60/0x1bc)
[   49.898437] [<c0025348>] (do_one_initcall+0x0/0x1bc) from [<c005a664>] (sys_init_module+0x9c/0x1b4)
[   49.906250]  r7:00028c36 r6:0001cfc8 r5:00000000 r4:bf059f04
[   49.914062] [<c005a5c8>] (sys_init_module+0x0/0x1b4) from [<c0025e60>] (ret_fast_syscall+0x0/0x30)
[   49.921875]  r7:00000080 r6:00000000 r5:00000000 r4:0001b070
[   49.929687] ---[ end trace 1b83ab63d709ff41 ]---
[   49.929687] sh_mobile_ceu sh_mobile_ceu.0: Unable to declare CEU memory.

Best regards,
Arnd

[regression] in linux-next: sh_mobile_ceu_camera broken by "ARM: Prohibit ioremap() on kernel managed RAM"

[Russell King - ARM Linux]

On Sun, Aug 08, 2010 at 05:45:26PM +0200, Arnd Hannemann wrote:
> Hi Russell,
> 
> your commit 309caa9cc6ff39d261264ec4ff10e29489afc8f8
> ARM: Prohibit ioremap() on kernel managed RAM
> 
> lets drivers/base/dma-coherent.c::dma_declare_coherent_memory()
> fail for the sh_mobile_ceu_camera driver (see backtrace below).
> I think, other configurations (i.MX31 users of the mx3_camera driver:
> pcm037 and mx31moboard) will have the same problem.
> 
> Since I have no idea how to fix this, I post this regression report here...

Me neither!  However, reverting the commit isn't the answer.

ARM shmobile platforms are ARM architecture V6 or V7, both of which have
the architectural restriction that multiple mappings of the same physical
address space must have the same memory type and cache attributes.  We
know that some ARMv7 systems do bad things when multiple different
mappings exist - and as they're using ARM's own Cortex CPU designs, and
I doubt shmobile is any different in that...

So, basically going forward with the advent of VIPT caches on ARM, any
kernel interface which allows system RAM to be remapped with different
attributes from the lowmem mapping is bad news - that means (eg) using
vmap() with a non-PGPROT_KERNEL pgprot has become illegal.  Certainly
using ioremap() on mapped system RAM on VIPT has become illegal, and
that's what has now been prevented.

It's actually a restriction which x86 gained some time ago, which I
stupidly continued to permit on ARM.  Now that our hardware has gained
the same restriction, we're now going to be into the same learning
curve...

I haven't worked out how sh_mobile_ceu gets used on the ARM shmobile
platforms - afaics, nothing in arch/arm/mach-shmobile declares a
sh_mobile_ceu platform device.

(When sparsemem's inability to deal with... sparse memory gets fixed
and we have a pfn_valid() which says it does what it says on the tin,
ioremap() might be able to be used with memory which hasn't been
declared to the kernel - as is currently the case with flatmem.)

[regression] in linux-next: sh_mobile_ceu_camera broken by "ARM: Prohibit ioremap() on kernel managed RAM"

[Philippe Rétornaz]

Le dimanche, 8 ao?t 2010 17.45:26, Arnd Hannemann a ?crit :
> Hi Russell,
> 
> your commit 309caa9cc6ff39d261264ec4ff10e29489afc8f8
> ARM: Prohibit ioremap() on kernel managed RAM
> 
> lets drivers/base/dma-coherent.c::dma_declare_coherent_memory()
> fail for the sh_mobile_ceu_camera driver (see backtrace below).
> I think, other configurations (i.MX31 users of the mx3_camera driver:
> pcm037 and mx31moboard) will have the same problem.
> 
> Since I have no idea how to fix this, I post this regression report here...

Have you found a solution to this problem ? As you said, the mx3_camera driver 
doesn't work anymore on mx31moboard.

Thanks ! 

Philippe

[regression] in linux-next: sh_mobile_ceu_camera broken by "ARM: Prohibit ioremap() on kernel managed RAM"

[Russell King - ARM Linux]

On Tue, Aug 10, 2010 at 10:45:03AM +0200, Philippe R?tornaz wrote:
> Le dimanche, 8 ao?t 2010 17.45:26, Arnd Hannemann a ?crit :
> > Hi Russell,
> > 
> > your commit 309caa9cc6ff39d261264ec4ff10e29489afc8f8
> > ARM: Prohibit ioremap() on kernel managed RAM
> > 
> > lets drivers/base/dma-coherent.c::dma_declare_coherent_memory()
> > fail for the sh_mobile_ceu_camera driver (see backtrace below).
> > I think, other configurations (i.MX31 users of the mx3_camera driver:
> > pcm037 and mx31moboard) will have the same problem.
> > 
> > Since I have no idea how to fix this, I post this regression report here...
> 
> Have you found a solution to this problem ? As you said, the mx3_camera driver 
> doesn't work anymore on mx31moboard.

MX3 is ARMv6 which has the architectural restriction as well I'm afraid.

[regression] in linux-next: sh_mobile_ceu_camera broken by "ARM: Prohibit ioremap() on kernel managed RAM"

[Stuart Menefy]

On 08/08/10 23:00, Russell King - ARM Linux wrote:
> On Sun, Aug 08, 2010 at 05:45:26PM +0200, Arnd Hannemann wrote:
>> Hi Russell,
>>
>> your commit 309caa9cc6ff39d261264ec4ff10e29489afc8f8
>> ARM: Prohibit ioremap() on kernel managed RAM
>>
>> lets drivers/base/dma-coherent.c::dma_declare_coherent_memory()
>> fail for the sh_mobile_ceu_camera driver (see backtrace below).
>> I think, other configurations (i.MX31 users of the mx3_camera driver:
>> pcm037 and mx31moboard) will have the same problem.
>>
>> Since I have no idea how to fix this, I post this regression report here...
> 
> Me neither!  However, reverting the commit isn't the answer.
> 
> ARM shmobile platforms are ARM architecture V6 or V7, both of which have
> the architectural restriction that multiple mappings of the same physical
> address space must have the same memory type and cache attributes.  We
> know that some ARMv7 systems do bad things when multiple different
> mappings exist - and as they're using ARM's own Cortex CPU designs, and
> I doubt shmobile is any different in that...
> 
> So, basically going forward with the advent of VIPT caches on ARM, any
> kernel interface which allows system RAM to be remapped with different
> attributes from the lowmem mapping is bad news - that means (eg) using
> vmap() with a non-PGPROT_KERNEL pgprot has become illegal.  Certainly
> using ioremap() on mapped system RAM on VIPT has become illegal, and
> that's what has now been prevented.
> 
> It's actually a restriction which x86 gained some time ago, which I
> stupidly continued to permit on ARM.  Now that our hardware has gained
> the same restriction, we're now going to be into the same learning
> curve...

Doesn't dma_alloc_coherent() suffer from the same problems? Pages are
allocated using alloc_pages, and in the non-coherent case will be
mapped uncached into consistent mapping region, creating a second
mapping with different attributes.

Stuart

[regression] in linux-next: sh_mobile_ceu_camera broken by "ARM: Prohibit ioremap() on kernel managed RAM"

[Russell King - ARM Linux]

On Tue, Aug 10, 2010 at 03:07:09PM +0100, Stuart Menefy wrote:
> Doesn't dma_alloc_coherent() suffer from the same problems? Pages are
> allocated using alloc_pages, and in the non-coherent case will be
> mapped uncached into consistent mapping region, creating a second
> mapping with different attributes.

Yes, and it's just waiting to corrupt your data.  It's the next item on
the list of things to fix for ARMv6/ARMv7 for the next merge window.

This merge window contains a small step in that direction by changing
the way the DMA region allocator works - aligning allocations to their
natural size.

Potentially, that means people may need a larger DMA region - there
will be allocation patterns which fail (eg, 1.5MB, 256MB, 16MB, 240MB)
which used to succeed - though problems due to fragmentation should
be reduced due to the natural alignment of blocks below 1MB.

There are two solutions I'm contemplating:

1. We pre-allocate the memory for the DMA region at boot and never free
   it for other uses.  This means that the DMA coherent region will
   always be fully populated regardless of how much memory is actually
   in use.

2. We allocate memory for the DMA region in sections, and unmap that
   memory from the lowmem pool.

(1) has the problem that people won't like it due to the obvious waste,
but it is the easy solution.

(2) is rather disgusting to achieve because of the problems of updating
all page tables in the system, especially on SMP systems.

The option of doing nothing is not realistic - as ARM processors progress,
the speculative prefetch benaviour is only going to become more aggressive,
and therefore memory which is mapped multiple times with different
attributes _will_ fail to behave as expected in random and hard to debug
ways.

[regression] in linux-next: sh_mobile_ceu_camera broken by "ARM: Prohibit ioremap() on kernel managed RAM"

[Guennadi Liakhovetski]

Hi Russell

On Tue, 10 Aug 2010, Russell King - ARM Linux wrote:

> On Tue, Aug 10, 2010 at 10:45:03AM +0200, Philippe R?tornaz wrote:
> > Le dimanche, 8 ao?t 2010 17.45:26, Arnd Hannemann a ?crit :
> > > Hi Russell,
> > > 
> > > your commit 309caa9cc6ff39d261264ec4ff10e29489afc8f8
> > > ARM: Prohibit ioremap() on kernel managed RAM
> > > 
> > > lets drivers/base/dma-coherent.c::dma_declare_coherent_memory()
> > > fail for the sh_mobile_ceu_camera driver (see backtrace below).
> > > I think, other configurations (i.MX31 users of the mx3_camera driver:
> > > pcm037 and mx31moboard) will have the same problem.
> > > 
> > > Since I have no idea how to fix this, I post this regression report here...
> > 
> > Have you found a solution to this problem ? As you said, the mx3_camera driver 
> > doesn't work anymore on mx31moboard.
> 
> MX3 is ARMv6 which has the architectural restriction as well I'm afraid.

So, if I understand this right, you cannot call 
dma_declare_coherent_memory() on already mapped RAM on ARM. So, to use 
this on physical RAM we have to remove it from the kernel mapping? Would 
using the "memmap=" kernel parameter suffice? Or is there a better 
solution for this?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[regression] in linux-next: sh_mobile_ceu_camera broken by "ARM: Prohibit ioremap() on kernel managed RAM"

[Russell King - ARM Linux]

On Tue, Aug 10, 2010 at 09:55:29PM +0200, Guennadi Liakhovetski wrote:
> Hi Russell
> 
> On Tue, 10 Aug 2010, Russell King - ARM Linux wrote:
> 
> > On Tue, Aug 10, 2010 at 10:45:03AM +0200, Philippe R?tornaz wrote:
> > > Le dimanche, 8 ao?t 2010 17.45:26, Arnd Hannemann a ?crit :
> > > > Hi Russell,
> > > > 
> > > > your commit 309caa9cc6ff39d261264ec4ff10e29489afc8f8
> > > > ARM: Prohibit ioremap() on kernel managed RAM
> > > > 
> > > > lets drivers/base/dma-coherent.c::dma_declare_coherent_memory()
> > > > fail for the sh_mobile_ceu_camera driver (see backtrace below).
> > > > I think, other configurations (i.MX31 users of the mx3_camera driver:
> > > > pcm037 and mx31moboard) will have the same problem.
> > > > 
> > > > Since I have no idea how to fix this, I post this regression report here...
> > > 
> > > Have you found a solution to this problem ? As you said, the mx3_camera driver 
> > > doesn't work anymore on mx31moboard.
> > 
> > MX3 is ARMv6 which has the architectural restriction as well I'm afraid.
> 
> So, if I understand this right, you cannot call 
> dma_declare_coherent_memory() on already mapped RAM on ARM.

It would appear so - unfortunately this API was designed by ARM folk back
in the VIVT cached days when this kind of restriction wasn't present.

> So, to use this on physical RAM we have to remove it from the kernel
> mapping? Would using the "memmap=" kernel parameter suffice? Or is
> there a better solution for this?

I don't know what "memmap=" is, which suggests that as we don't process
it, it'll do nothing.  (This is one of the problems of
kernel-parameters.txt - it doesn't tell you what are architecture
specific parameters.)

You could instead use mem= or omit a chunk of memory from the ATAGs.
However, as I've already covered, because of the brokenness of
sparsemem not being able to handle sparse memory, its implementation
of pfn_valid() will be over-happy to return 'true' even for memory
you've omitted.  Flatmem doesn't suffer this problem.

Or just make the DMA coherent region larger and use that instead (and
the DMA coherent region I hope will be fixed to omit the dual-mapping
for the next merge window.)

But as I've said, the whole "ioremap system RAM" idea has to die.  It's
something that just isn't workable with later ARMs, and it's something
that historically hasn't been supported on x86 either for very similar
reasons (except for very exceptional corner cases.)

[regression] in linux-next: sh_mobile_ceu_camera broken by "ARM: Prohibit ioremap() on kernel managed RAM"

[hisao munakata]

Hi Russell

This is Munakata of Renesas. I am a kind of manager and not a kernel 
developer. So I wonder this message may not include any topic interest for you, 
Initially I want to express my great appreciation for your help to add Renesas 
SH-Mobile series kernel support. Actually majority of patches to support 
SH-Mobile series are written by Renesas Linux development team colleagues 
like Magnus, Guennadi, Arnd and others those are working in my team.

As you may know, we used to develop SuperH CPU support with the
help of SuperH maintainer Paul.  Now he also works inside my team. 
That means CPU architecture provider, chip vendor and community 
kernel maintainer are working at the same place.  And luckily or unluckily
there are not so many relevant party around SuperH.  Now I am getting
to learn ARM development requires much much tough coordination work
against various instruction set, each chip implementations and others. 

I am not sure how can I make this happen, but Renesas want to
help your work if possible.  At least I do not want to have our
private kernel tree out side community master repository. So
I try to submit all required patches including ARM core part code
from now on. Is that OK for you ?

If you have any request for us as a chip vendor and community
developer, please kindly tell me your thought. I am planing to
attend CE Linux Forum ELCE (Embedded Linux conference Europe)
hold at Cambridge UK this fall ( October 27th and 28th). If we can 
have a chance to discuss this kind of topic around then, that
will be really great.

-- 
hisao munakata <hisao,munakata.vt@renesas.com>
Executive manager 
System Business Div.
Renesas Solutions Corp.

[regression] in linux-next: sh_mobile_ceu_camera broken by "ARM: Prohibit ioremap() on kernel managed RAM"

[Guennadi Liakhovetski]

Resuming the week-old thread, to try to get a solution for this problem 
for 2.6.36.

On Tue, 10 Aug 2010, Russell King - ARM Linux wrote:

> On Tue, Aug 10, 2010 at 09:55:29PM +0200, Guennadi Liakhovetski wrote:
> > Hi Russell
> > 
> > On Tue, 10 Aug 2010, Russell King - ARM Linux wrote:
> > 
> > > On Tue, Aug 10, 2010 at 10:45:03AM +0200, Philippe R?tornaz wrote:
> > > > Le dimanche, 8 ao?t 2010 17.45:26, Arnd Hannemann a ?crit :
> > > > > Hi Russell,
> > > > > 
> > > > > your commit 309caa9cc6ff39d261264ec4ff10e29489afc8f8
> > > > > ARM: Prohibit ioremap() on kernel managed RAM
> > > > > 
> > > > > lets drivers/base/dma-coherent.c::dma_declare_coherent_memory()
> > > > > fail for the sh_mobile_ceu_camera driver (see backtrace below).
> > > > > I think, other configurations (i.MX31 users of the mx3_camera driver:
> > > > > pcm037 and mx31moboard) will have the same problem.
> > > > > 
> > > > > Since I have no idea how to fix this, I post this regression report here...
> > > > 
> > > > Have you found a solution to this problem ? As you said, the mx3_camera driver 
> > > > doesn't work anymore on mx31moboard.
> > > 
> > > MX3 is ARMv6 which has the architectural restriction as well I'm afraid.
> > 
> > So, if I understand this right, you cannot call 
> > dma_declare_coherent_memory() on already mapped RAM on ARM.
> 
> It would appear so - unfortunately this API was designed by ARM folk back
> in the VIVT cached days when this kind of restriction wasn't present.
> 
> > So, to use this on physical RAM we have to remove it from the kernel
> > mapping? Would using the "memmap=" kernel parameter suffice? Or is
> > there a better solution for this?
> 
> I don't know what "memmap=" is, which suggests that as we don't process
> it, it'll do nothing.  (This is one of the problems of
> kernel-parameters.txt - it doesn't tell you what are architecture
> specific parameters.)
> 
> You could instead use mem= or omit a chunk of memory from the ATAGs.
> However, as I've already covered, because of the brokenness of
> sparsemem not being able to handle sparse memory, its implementation
> of pfn_valid() will be over-happy to return 'true' even for memory
> you've omitted.  Flatmem doesn't suffer this problem.

Ok, so, this would work on flatmem (which is what platforms, that I know 
use this API for video buffer allocation), but is not very elegant.

> Or just make the DMA coherent region larger and use that instead (and
> the DMA coherent region I hope will be fixed to omit the dual-mapping
> for the next merge window.)

Sorry, don't quite understand. Currently what those platforms do is they 
allocate at platform init time a sufficient DMA doherent region per

	buf = dma_alloc_coherent(NULL, buf_size, &dma_handle, GFP_KERNEL);

and then "assign" it to the video platform device per

	dma = dma_declare_coherent_memory(&mx3_camera.dev,
					dma_handle, dma_handle, buf_size,
					DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE);

Now the driver will be able to use the videobuf-dma-contig video-buffer 
allocation, which does

	mem->vaddr = dma_alloc_coherent(q->dev, mem->size,
					&mem->dma_handle, GFP_KERNEL);

The sole purpose of the preallocation in the platform code is to avoid 
memory fragmentation. Sorry, if I'm repeating obvious things. So, we are 
already allocating DMA coherent memory. The ioremap() problem in this case 
is actually bogus - we do not need that ioremap(). The problem would be 
solved, if ioremap() would just not be called in these cases. This is what 
the drivers/staging/dt3155v4l/dt3155vfl.c driver is open-coding in its 
dt3155_alloc_coherent() function, as pointed out by Marin Mitov (CCed) in 
another thread. So, would it be acceptable to provide a second function, 
doing exactly the same as dma_declare_coherent_memory() but without an 
ioremap(), or add a new DMA_MEMORY_... flag to skip ioremap() in 
dma_declare_coherent_memory(), as suggested by Uwe (CCed too)?

> But as I've said, the whole "ioremap system RAM" idea has to die.  It's
> something that just isn't workable with later ARMs, and it's something
> that historically hasn't been supported on x86 either for very similar
> reasons (except for very exceptional corner cases.)

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[regression] in linux-next: sh_mobile_ceu_camera broken by "ARM: Prohibit ioremap() on kernel managed RAM"

[Russell King - ARM Linux]

On Wed, Aug 18, 2010 at 09:23:25PM +0200, Guennadi Liakhovetski wrote:
> Resuming the week-old thread, to try to get a solution for this problem 
> for 2.6.36.
> 
> On Tue, 10 Aug 2010, Russell King - ARM Linux wrote:
> > Or just make the DMA coherent region larger and use that instead (and
> > the DMA coherent region I hope will be fixed to omit the dual-mapping
> > for the next merge window.)
> 
> Sorry, don't quite understand. Currently what those platforms do is they 
> allocate at platform init time a sufficient DMA doherent region per
> 
> 	buf = dma_alloc_coherent(NULL, buf_size, &dma_handle, GFP_KERNEL);
> 
> and then "assign" it to the video platform device per
> 
> 	dma = dma_declare_coherent_memory(&mx3_camera.dev,
> 					dma_handle, dma_handle, buf_size,
> 					DMA_MEMORY_MAP | DMA_MEMORY_EXCLUSIVE);

So, why have the DMA memory mapped in _correctly_ by dma_alloc_coherent(),
and then have dma_declare_coherent_memory() map it in using ioremap(),
thereby creating an incompatible DEVICE aliasing mapping of the already
existing memory mapping.

This definitely falls outside of predicatible behaviour on ARMv6 and
ARMv7, and doesn't even fall within the limits of the mitigated
conditions for existing CPUs.

The above will _not_ work reliably.  Good, my patch has identified
something that's definitely broken.

> Now the driver will be able to use the videobuf-dma-contig video-buffer 
> allocation, which does
> 
> 	mem->vaddr = dma_alloc_coherent(q->dev, mem->size,
> 					&mem->dma_handle, GFP_KERNEL);
> 
> The sole purpose of the preallocation in the platform code is to avoid 
> memory fragmentation. Sorry, if I'm repeating obvious things. So, we are 
> already allocating DMA coherent memory. The ioremap() problem in this case 
> is actually bogus - we do not need that ioremap(). The problem would be 
> solved, if ioremap() would just not be called in these cases. This is what 
> the drivers/staging/dt3155v4l/dt3155vfl.c driver is open-coding in its 
> dt3155_alloc_coherent() function, as pointed out by Marin Mitov (CCed) in 
> another thread. So, would it be acceptable to provide a second function, 
> doing exactly the same as dma_declare_coherent_memory() but without an 
> ioremap(), or add a new DMA_MEMORY_... flag to skip ioremap() in 
> dma_declare_coherent_memory(), as suggested by Uwe (CCed too)?

Sounds like a sane approach to fixing this to me.

[regression] in linux-next: sh_mobile_ceu_camera broken by "ARM: Prohibit ioremap() on kernel managed RAM"

[Guennadi Liakhovetski]

On Wed, 18 Aug 2010, Russell King - ARM Linux wrote:

> On Wed, Aug 18, 2010 at 09:23:25PM +0200, Guennadi Liakhovetski wrote:

[snip]

> > The sole purpose of the preallocation in the platform code is to avoid 
> > memory fragmentation. Sorry, if I'm repeating obvious things. So, we are 
> > already allocating DMA coherent memory. The ioremap() problem in this case 
> > is actually bogus - we do not need that ioremap(). The problem would be 
> > solved, if ioremap() would just not be called in these cases. This is what 
> > the drivers/staging/dt3155v4l/dt3155vfl.c driver is open-coding in its 
> > dt3155_alloc_coherent() function, as pointed out by Marin Mitov (CCed) in 
> > another thread. So, would it be acceptable to provide a second function, 
> > doing exactly the same as dma_declare_coherent_memory() but without an 
> > ioremap(), or add a new DMA_MEMORY_... flag to skip ioremap() in 
> > dma_declare_coherent_memory(), as suggested by Uwe (CCed too)?
> 
> Sounds like a sane approach to fixing this to me.

I assume, you mean adding a new flag to skip ioremap(). But then we have 
to pass the virtual address to the function. Its prototype is

int dma_declare_coherent_memory(struct device *dev, dma_addr_t bus_addr,
				dma_addr_t device_addr, size_t size, int flags);

bus_addr is unused in this case, but I don't think abusing it to pass a 
"void *" would be an acceptable solution - apart from all the ugly 
type-casting, if we ever get 64-bit virtual addresses on ARM with 32-bit 
DMA addresses, we've got a problem. Or is this never going to happen? Or 
whould I rather add a new function?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

[regression] in linux-next: sh_mobile_ceu_camera broken by "ARM: Prohibit ioremap() on kernel managed RAM"

[Russell King - ARM Linux]

On Wed, Aug 18, 2010 at 10:31:10PM +0200, Guennadi Liakhovetski wrote:
> On Wed, 18 Aug 2010, Russell King - ARM Linux wrote:
> > Sounds like a sane approach to fixing this to me.
> 
> I assume, you mean adding a new flag to skip ioremap(). But then we have 
> to pass the virtual address to the function. Its prototype is
> 
> int dma_declare_coherent_memory(struct device *dev, dma_addr_t bus_addr,
> 				dma_addr_t device_addr, size_t size, int flags);
> 
> bus_addr is unused in this case, but I don't think abusing it to pass a 
> "void *" would be an acceptable solution - apart from all the ugly 
> type-casting, if we ever get 64-bit virtual addresses on ARM with 32-bit 
> DMA addresses, we've got a problem. Or is this never going to happen? Or 
> whould I rather add a new function?

A new function sounds saner.

[regression] in linux-next: sh_mobile_ceu_camera broken by "ARM: Prohibit ioremap() on kernel managed RAM"

[Russell King - ARM Linux]

On Wed, Aug 18, 2010 at 10:31:10PM +0200, Guennadi Liakhovetski wrote:
> I assume, you mean adding a new flag to skip ioremap(). But then we have 
> to pass the virtual address to the function. Its prototype is
> 
> int dma_declare_coherent_memory(struct device *dev, dma_addr_t bus_addr,
> 				dma_addr_t device_addr, size_t size, int flags);
> 
> bus_addr is unused in this case, but I don't think abusing it to pass a 
> "void *" would be an acceptable solution - apart from all the ugly 
> type-casting, if we ever get 64-bit virtual addresses on ARM with 32-bit 
> DMA addresses, we've got a problem. Or is this never going to happen? Or 
> whould I rather add a new function?

Oh, and if it's going to be used as per your previous message,
how about:

int dma_preallocate_coherent_memory(struct device *dev, size_t size);

so that it contains the dma_alloc_coherent() logic itself too?