On Tue, Dec 09, 2014 at 02:56:59AM +0000, Abhilash Kesavan wrote:
On Mon, Dec 8, 2014 at 5:55 PM, Catalin Marinas catalin.marinas@arm.com wrote:
On Mon, Dec 08, 2014 at 11:15:52AM +0000, Lars-Peter Clausen wrote:
On 12/08/2014 11:41 AM, Takashi Iwai wrote:
At Mon, 08 Dec 2014 09:49:37 +0100, Clemens Ladisch wrote:
Abhilash Kesavan wrote:
I am working on a 64-bit ARM SoC (Samsung's Exynos7) and have observed unaligned faults while testing certain sound streams with aplay.
[ 24.535661] snd_pcm_lib_write_transfer:sound/core/pcm_lib.c hwbuf is ffffff8000085624, runtime->dma_area is ffffff8000080000, hwoff is 5513, frames_to_bytes is 22052, frames is 5513 [ 24.551244] Unhandled fault: alignment fault (0x96000061) at 0xffffff8000085624 [ 24.579944] PC is at __copy_from_user+0x14/0x60 [ 24.584450] LR is at snd_pcm_lib_write_transfer+0xe4/0x104 [ 24.922054] Call trace: [ 24.924488] [<ffffffc0002d4784>] __copy_from_user+0x14/0x60 [ 24.930040] [<ffffffc0004b7574>] snd_pcm_lib_write1+0x1fc/0x384
I can see copy_from_user() here, I assume the source is a user space buffer and the destination is the ioremap'ed SRAM (which causes the unaligned fault). Such function is not supposed copy into device memory (its prototype does not specify __iomem for the destination pointer).
If you really want Device memory for the SRAM, I think we need some other API. memcpy_toio() could copy into such buffer but the source must be kernel. Maybe something like copy_from_user_toio()?
Thanks for the response.
Changing copy_from_user() to copy_from_user_toio() in snd_pcm_lib_write_transfer() (sound/core/pcm_lib.c) works. I am not sure about other effects of making such a change in a core sound file, can someone comment ?
The default implementation uses copy_from_user() to an intermediate buffer and another memcpy_toio() to the SRAM destination. So, it will be slower.
In general it may be better to copy this function to some more generic place and let the arch code override it with a more optimal implementation.
Otherwise, if you can remap the buffer as Normal (non-cacheable) memory, you could use something like vmap() with pgprot_writecombine for the mapping. Even though ioremap_wc() returns (currently) Normal non-cacheable memory, such pointer (__iomem) should only be used with specific IO accessors (though I can see this assumption broken in a few places in the kernel).
Currently, if I continue to use the SRAM allocation driver, two options are working:
- ioremap to ioremap_wc in drivers/misc/sram.c.
I think the SRAM driver does the correct devm_ioremap_resource() call but the implementation in lib/devres.c assumes that if IORESOURCE_CACHEABLE it should use devm_ioremap(). AFAIK, ioremap() always returns Device memory (even on x86, the default ioremap behaviour is nocache).
- copy_from_user to copy_from_user_toio in sound/core/pcm_lib.c
Which would be the preferred option ?
I would prefer the sram changed. It provides an allocation pool from SRAM which could as well be mapped as write-combine. If no-one complains about changing the SRAM mapping to WC, you could introduce a devm_ioremap_wc() in lib/devres.c and call that directly from sram_probe(). But I can't tell for sure what the memory type expectations for SRAM are.