[alsa-devel] Question on arm64 unaligned faults during playback

[Abhilash Kesavan]

Hi Will,

On Mon, Dec 8, 2014 at 5:01 PM, Will Deacon <will.deacon at arm.com> wrote:
> On Mon, Dec 08, 2014 at 11:15:52AM +0000, Lars-Peter Clausen wrote:
>> Added ARM64 folks to Cc.
>>
>> 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
>> >>>
>> >>> We are using the internal sram available for sound, for DMA buffer
>> >>> allocation, using the generic SRAM driver. From the above log, the
>> >>> buffer address offset is not 8-byte aligned and as we are using the
>> >>> SRAM driver which maps the memory as device memory we are getting an
>> >>> unaligned fault. Is it incorrect to use the generic sram driver for
>> >>> arm64 or am I missing something ?
>> >>
>> >> When you give the ALSA framework a memory buffer, it is assumed that it
>> >> is general-purpose memory, i.e., can be accessed with any alignment.
>> >>
>> >> If your memory does not allow such accesses, you have to do all the
>> >> accesses in your driver, i.e., do everything in the .copy/.silence
>> >> callbacks.  (You also have to add constraints so that periods and buffer
>> >> are correctly aligned.)
>> >
>> > You can set runtime->min_align in the own hw_params callback so that
>> > the PCM core handle it well, at least, for non-mmap mode.
>> >
>> > But this feature has been rarely used (currently no driver sets it
>> > explicitly), so there might be still some bugs there, too :)
>>
>> As far as I can see min_align only aligns the hardware pointer, but not the
>> application pointer. The later causes the issue here.
>>
>> E.g. if a application writes a unaligned amount of bytes the next write will
>> start at an unaligned buffer address.
>>
>> How safe is it in general to use IO mapped memory as general purpose memory
>> on ARM64? Are there maybe even more constraints than just that access has to
>> be aligned? Maybe the driver should rather use copy_from_user_toio() instead
>> of copy_from_user() to make sure that things work correctly.
>
> If you're simply mapping RAM as device memory, then you need to ensure:
>
>   - Naturally aligned access
>   - No exclusive instructions (e.g atomics, locks, semaphores ...)
>
> Out of interest, why are you using device mappings for memory? By design,
> they prohibit speculation (and, depending on the subtype, re-ordering,
> gathering and early write response) and will be significantly slower than
> normal memory accesses.

Thanks for the comments.

We are using the generic SRAM allocation driver (drivers/misc/sram.c)
to use the internal audio SRAM for buffer allocation. The generic
driver ioremaps the sram and exposes the obtained pool via the
genalloc API to other drivers such as audio. There already exists
support for internal sram in the audio framework. The specific use
case is in an Exynos7 low power state where the DRAM cannot be used.

>
> Could you use something like ioremap_wc, which will give you normal,
> non-cacheable memory instead?

Modifying ioremap to ioremap_wc gets rid of the crash. Do you think
the sram driver ioremap usage should be modified to use ioremap_wc ?

Regards,
Abhilash
>
> Will