* [alsa-devel] Question on arm64 unaligned faults during playback
[not found] ` <s5hppbumnis.wl-tiwai@suse.de>
@ 2014-12-08 11:15 ` Lars-Peter Clausen
2014-12-08 11:31 ` Will Deacon
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Lars-Peter Clausen @ 2014-12-08 11:15 UTC (permalink / raw)
To: linux-arm-kernel
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.
- Lars
^ permalink raw reply [flat|nested] 9+ messages in thread
* [alsa-devel] Question on arm64 unaligned faults during playback
2014-12-08 11:15 ` [alsa-devel] Question on arm64 unaligned faults during playback Lars-Peter Clausen
@ 2014-12-08 11:31 ` Will Deacon
2014-12-09 2:56 ` Abhilash Kesavan
2014-12-08 12:04 ` Takashi Iwai
2014-12-08 12:25 ` Catalin Marinas
2 siblings, 1 reply; 9+ messages in thread
From: Will Deacon @ 2014-12-08 11:31 UTC (permalink / raw)
To: linux-arm-kernel
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.
Could you use something like ioremap_wc, which will give you normal,
non-cacheable memory instead?
Will
^ permalink raw reply [flat|nested] 9+ messages in thread
* [alsa-devel] Question on arm64 unaligned faults during playback
2014-12-08 11:15 ` [alsa-devel] Question on arm64 unaligned faults during playback Lars-Peter Clausen
2014-12-08 11:31 ` Will Deacon
@ 2014-12-08 12:04 ` Takashi Iwai
2014-12-08 12:25 ` Catalin Marinas
2 siblings, 0 replies; 9+ messages in thread
From: Takashi Iwai @ 2014-12-08 12:04 UTC (permalink / raw)
To: linux-arm-kernel
At Mon, 08 Dec 2014 12:15:52 +0100,
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.
Oh right, and I think we should fix the alignment of applptr, too.
We can set min_align zero as default and allow the unaligned applptr
update while aligning to the size if given explicitly.
Takashi
^ permalink raw reply [flat|nested] 9+ messages in thread
* [alsa-devel] Question on arm64 unaligned faults during playback
2014-12-08 11:15 ` [alsa-devel] Question on arm64 unaligned faults during playback Lars-Peter Clausen
2014-12-08 11:31 ` Will Deacon
2014-12-08 12:04 ` Takashi Iwai
@ 2014-12-08 12:25 ` Catalin Marinas
2014-12-09 2:56 ` Abhilash Kesavan
2 siblings, 1 reply; 9+ messages in thread
From: Catalin Marinas @ 2014-12-08 12:25 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Dec 08, 2014 at 11:15:52AM +0000, Lars-Peter Clausen wrote:
> Added ARM64 folks to Cc.
Thanks.
> 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()?
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).
--
Catalin
^ permalink raw reply [flat|nested] 9+ messages in thread
* [alsa-devel] Question on arm64 unaligned faults during playback
2014-12-08 11:31 ` Will Deacon
@ 2014-12-09 2:56 ` Abhilash Kesavan
0 siblings, 0 replies; 9+ messages in thread
From: Abhilash Kesavan @ 2014-12-09 2:56 UTC (permalink / raw)
To: linux-arm-kernel
Hi Will,
On Mon, Dec 8, 2014 at 5:01 PM, Will Deacon <will.deacon@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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [alsa-devel] Question on arm64 unaligned faults during playback
2014-12-08 12:25 ` Catalin Marinas
@ 2014-12-09 2:56 ` Abhilash Kesavan
2014-12-09 8:19 ` Clemens Ladisch
2014-12-09 11:43 ` Catalin Marinas
0 siblings, 2 replies; 9+ messages in thread
From: Abhilash Kesavan @ 2014-12-09 2:56 UTC (permalink / raw)
To: linux-arm-kernel
Hi Catalin,
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:
>> Added ARM64 folks to Cc.
>
> Thanks.
>
>> 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 ?
>
> 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:
1) ioremap to ioremap_wc in drivers/misc/sram.c.
2) copy_from_user to copy_from_user_toio in sound/core/pcm_lib.c
Which would be the preferred option ?
Regards,
Abhilash
>
> --
> Catalin
^ permalink raw reply [flat|nested] 9+ messages in thread
* [alsa-devel] Question on arm64 unaligned faults during playback
2014-12-09 2:56 ` Abhilash Kesavan
@ 2014-12-09 8:19 ` Clemens Ladisch
2014-12-09 11:43 ` Catalin Marinas
1 sibling, 0 replies; 9+ messages in thread
From: Clemens Ladisch @ 2014-12-09 8:19 UTC (permalink / raw)
To: linux-arm-kernel
Abhilash Kesavan wrote:
> Currently, if I continue to use the SRAM allocation driver, two
> options are working:
> 1) ioremap to ioremap_wc in drivers/misc/sram.c.
> 2) copy_from_user to copy_from_user_toio in sound/core/pcm_lib.c
>
> Which would be the preferred option ?
I don't know how to tell the SRAM allocator to hand out WC memory, but
copy_from_user_toio() is what you would put into your .copy callback.
Regards,
Clemens
^ permalink raw reply [flat|nested] 9+ messages in thread
* [alsa-devel] Question on arm64 unaligned faults during playback
2014-12-09 2:56 ` Abhilash Kesavan
2014-12-09 8:19 ` Clemens Ladisch
@ 2014-12-09 11:43 ` Catalin Marinas
2014-12-10 4:10 ` Abhilash Kesavan
1 sibling, 1 reply; 9+ messages in thread
From: Catalin Marinas @ 2014-12-09 11:43 UTC (permalink / raw)
To: linux-arm-kernel
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:
> 1) 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).
> 2) 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.
--
Catalin
^ permalink raw reply [flat|nested] 9+ messages in thread
* [alsa-devel] Question on arm64 unaligned faults during playback
2014-12-09 11:43 ` Catalin Marinas
@ 2014-12-10 4:10 ` Abhilash Kesavan
0 siblings, 0 replies; 9+ messages in thread
From: Abhilash Kesavan @ 2014-12-10 4:10 UTC (permalink / raw)
To: linux-arm-kernel
Hi Catalin,
On Tue, Dec 9, 2014 at 5:13 PM, Catalin Marinas <catalin.marinas@arm.com> wrote:
> 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:
>> 1) 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).
>
>> 2) 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.
Thanks for the response. I will post patches to do this.
Also, adding guys who have been working on the generic sram driver and
internal sram support for sound in case they have any comments
regarding the change to ioremap_wc.
Regards,
Abhilash
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-12-10 4:10 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CAM4voa=kiLsiLEzUbo3D+f1go5px78fXBSjSi7kGK4QCdnwPyQ@mail.gmail.com>
[not found] ` <54856621.8090107@ladisch.de>
[not found] ` <s5hppbumnis.wl-tiwai@suse.de>
2014-12-08 11:15 ` [alsa-devel] Question on arm64 unaligned faults during playback Lars-Peter Clausen
2014-12-08 11:31 ` Will Deacon
2014-12-09 2:56 ` Abhilash Kesavan
2014-12-08 12:04 ` Takashi Iwai
2014-12-08 12:25 ` Catalin Marinas
2014-12-09 2:56 ` Abhilash Kesavan
2014-12-09 8:19 ` Clemens Ladisch
2014-12-09 11:43 ` Catalin Marinas
2014-12-10 4:10 ` Abhilash Kesavan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).