diff mbox

[v2,3/5] ARM: add support for kernel mode NEON

Message ID 1372191891-24574-4-git-send-email-ard.biesheuvel@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Ard Biesheuvel June 25, 2013, 8:24 p.m. UTC
In order to safely support the use of NEON instructions in
kernel mode, some precautions need to be taken:
- the userland context that may be present in the registers (even
  if the NEON/VFP is currently disabled) must be stored under the
  correct task (which may not be 'current' in the UP case),
- to avoid having to keep track of additional vfpstates for the
  kernel side, disallow the use of NEON in interrupt context
  and run with preemption disabled,
- after use, re-enable preemption and re-enable the lazy restore
  machinery by disabling the NEON/VFP unit.

This patch adds the functions kernel_neon_begin() and
kernel_neon_end() which take care of the above. It also adds
the Kconfig symbol KERNEL_MODE_NEON to enable it.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Acked-by: Nicolas Pitre <nico@linaro.org>
---
 arch/arm/Kconfig            |  7 ++++++
 arch/arm/include/asm/neon.h | 36 ++++++++++++++++++++++++++++++
 arch/arm/vfp/vfpmodule.c    | 54 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 97 insertions(+)
 create mode 100644 arch/arm/include/asm/neon.h

Comments

Ard Biesheuvel June 26, 2013, 10:55 a.m. UTC | #1
Replying to self:

On 25 June 2013 22:24, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> +void kernel_neon_end(void)
> +{
> +       /* Disable the NEON/VFP unit. */
> +       fmxr(FPEXC, fmrx(FPEXC) & ~FPEXC_EN);
> +       barrier();
> +       dec_preempt_count();
> +}
> +EXPORT_SYMBOL(kernel_neon_end);

Meh. This is not going to please the RT crowd, as preempt_schedule()
will not be called on PREEMPT builds in this case.

Propose to replace it with

    preempt_enable();
#ifndef CONFIG_PREEMPT_COUNT
    /* in this case, the preempt_enable() right above is just a barrier() */
    dec_preempt_count();
#endif

(and the converse in kernel_neon_begin())

In that case, preempt_disable will either be just a barrier(), or it
will re-enable preemption, and potentially reschedule if the preempt
count has dropped to zero.
Will Deacon June 26, 2013, 11:14 a.m. UTC | #2
On Wed, Jun 26, 2013 at 11:55:33AM +0100, Ard Biesheuvel wrote:
> Replying to self:
> 
> On 25 June 2013 22:24, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > +void kernel_neon_end(void)
> > +{
> > +       /* Disable the NEON/VFP unit. */
> > +       fmxr(FPEXC, fmrx(FPEXC) & ~FPEXC_EN);
> > +       barrier();
> > +       dec_preempt_count();
> > +}
> > +EXPORT_SYMBOL(kernel_neon_end);
> 
> Meh. This is not going to please the RT crowd, as preempt_schedule()
> will not be called on PREEMPT builds in this case.
> 
> Propose to replace it with
> 
>     preempt_enable();
> #ifndef CONFIG_PREEMPT_COUNT
>     /* in this case, the preempt_enable() right above is just a barrier() */
>     dec_preempt_count();
> #endif
> 
> (and the converse in kernel_neon_begin())

Yuck, that's ugly as sin! How does x86 deal with this? Looking at
kernel_fpu_{begin,end}, they just disable preemption so I guess that they
assume the caller is non-blocking? There's an aside about the use of
preempt-notifiers for KVM, so it does sound like the onus is on the caller
not to shoot themselves in the face.

Will
Ard Biesheuvel June 26, 2013, 11:28 a.m. UTC | #3
On 26 June 2013 13:14, Will Deacon <will.deacon@arm.com> wrote:
> On Wed, Jun 26, 2013 at 11:55:33AM +0100, Ard Biesheuvel wrote:
>> Replying to self:
>>
>> On 25 June 2013 22:24, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> > +void kernel_neon_end(void)
>> > +{
>> > +       /* Disable the NEON/VFP unit. */
>> > +       fmxr(FPEXC, fmrx(FPEXC) & ~FPEXC_EN);
>> > +       barrier();
>> > +       dec_preempt_count();
>> > +}
>> > +EXPORT_SYMBOL(kernel_neon_end);
>>
>> Meh. This is not going to please the RT crowd, as preempt_schedule()
>> will not be called on PREEMPT builds in this case.
>>
>> Propose to replace it with
>>
>>     preempt_enable();
>> #ifndef CONFIG_PREEMPT_COUNT
>>     /* in this case, the preempt_enable() right above is just a barrier() */
>>     dec_preempt_count();
>> #endif
>>
>> (and the converse in kernel_neon_begin())
>
> Yuck, that's ugly as sin! How does x86 deal with this? Looking at
> kernel_fpu_{begin,end}, they just disable preemption so I guess that they
> assume the caller is non-blocking? There's an aside about the use of
> preempt-notifiers for KVM, so it does sound like the onus is on the caller
> not to shoot themselves in the face.
>

Even if x86 doesn't care about this, do you really think we should
take the risk of silently clobbering the NEON registers if the caller
does something that may end up sleeping? Anyway, I don't remember
exactly who suggested using inc_preempt_count() directly, but doing so
brings about the responsibility of calling preempt_schedule() when
leaving the critical section, and just using both (without the #ifdef)
is also not an option.

So can you suggest a better way of making sure schedule_debug() shoots
us down if calling schedule() between kernel_neon_begin and
kernel_neon_end, even on non-preempt builds?
Will Deacon June 26, 2013, 12:40 p.m. UTC | #4
Hi Ard,

On Wed, Jun 26, 2013 at 12:28:49PM +0100, Ard Biesheuvel wrote:
> On 26 June 2013 13:14, Will Deacon <will.deacon@arm.com> wrote:
> > On Wed, Jun 26, 2013 at 11:55:33AM +0100, Ard Biesheuvel wrote:
> >> Propose to replace it with
> >>
> >>     preempt_enable();
> >> #ifndef CONFIG_PREEMPT_COUNT
> >>     /* in this case, the preempt_enable() right above is just a barrier() */
> >>     dec_preempt_count();
> >> #endif
> >>
> >> (and the converse in kernel_neon_begin())
> >
> > Yuck, that's ugly as sin! How does x86 deal with this? Looking at
> > kernel_fpu_{begin,end}, they just disable preemption so I guess that they
> > assume the caller is non-blocking? There's an aside about the use of
> > preempt-notifiers for KVM, so it does sound like the onus is on the caller
> > not to shoot themselves in the face.
> >
> 
> Even if x86 doesn't care about this, do you really think we should
> take the risk of silently clobbering the NEON registers if the caller
> does something that may end up sleeping? Anyway, I don't remember
> exactly who suggested using inc_preempt_count() directly, but doing so
> brings about the responsibility of calling preempt_schedule() when
> leaving the critical section, and just using both (without the #ifdef)
> is also not an option.
> 
> So can you suggest a better way of making sure schedule_debug() shoots
> us down if calling schedule() between kernel_neon_begin and
> kernel_neon_end, even on non-preempt builds?

With what we currently have in the kernel, no, I can't think of a better
way. However, I also don't think that smuggling in a back-end hack is a good
idea either. How about we follow x86's lead on this and rely on the caller
not to sleep for the timebeing? Then, separately to this patch series, you
could look at augmenting the scheduler so that schedule_debug can complain
if it encounters a task that is not expected to sleep? That seems like the
right place to fix this problem, and will benefit other architectures too.

Will
Ard Biesheuvel June 26, 2013, 12:52 p.m. UTC | #5
On 26 June 2013 14:40, Will Deacon <will.deacon@arm.com> wrote:
> On Wed, Jun 26, 2013 at 12:28:49PM +0100, Ard Biesheuvel wrote:
>> So can you suggest a better way of making sure schedule_debug() shoots
>> us down if calling schedule() between kernel_neon_begin and
>> kernel_neon_end, even on non-preempt builds?
>
> With what we currently have in the kernel, no, I can't think of a better
> way. However, I also don't think that smuggling in a back-end hack is a good
> idea either. How about we follow x86's lead on this and rely on the caller
> not to sleep for the timebeing? Then, separately to this patch series, you
> could look at augmenting the scheduler so that schedule_debug can complain
> if it encounters a task that is not expected to sleep? That seems like the
> right place to fix this problem, and will benefit other architectures too.
>

Good point. As preempt_enable/disable already have this side effect on
PREEMPT builds, perhaps it wouldn't be such a bad idea to modify them
in the non-PREEMPT case to at least complain if schedule() is invoked
during such a section.

@Russell: you mentioned spinlocks at some point to prevent sleeping.
Are you ok with Will's suggestion instead, i.e., to rely on
preempt_disable() to do the right thing, and fix it later because
currently, it doesn't on non-PREEMPT?
Ard Biesheuvel June 26, 2013, 1:13 p.m. UTC | #6
On 26 June 2013 14:52, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 26 June 2013 14:40, Will Deacon <will.deacon@arm.com> wrote:
>> On Wed, Jun 26, 2013 at 12:28:49PM +0100, Ard Biesheuvel wrote:
>>> So can you suggest a better way of making sure schedule_debug() shoots
>>> us down if calling schedule() between kernel_neon_begin and
>>> kernel_neon_end, even on non-preempt builds?
>>
>> With what we currently have in the kernel, no, I can't think of a better
>> way. However, I also don't think that smuggling in a back-end hack is a good
>> idea either. How about we follow x86's lead on this and rely on the caller
>> not to sleep for the timebeing? Then, separately to this patch series, you
>> could look at augmenting the scheduler so that schedule_debug can complain
>> if it encounters a task that is not expected to sleep? That seems like the
>> right place to fix this problem, and will benefit other architectures too.
>>
>
> Good point. As preempt_enable/disable already have this side effect on
> PREEMPT builds, perhaps it wouldn't be such a bad idea to modify them
> in the non-PREEMPT case to at least complain if schedule() is invoked
> during such a section.
>

It appears we have this already, but in the non-PREEMPT case, it needs
CONFIG_DEBUG_ATOMIC_SLEEP to be defined.
Let's just rely on preempt_disable() to do the right thing...
Ard Biesheuvel June 27, 2013, 1:11 p.m. UTC | #7
On 26 June 2013 15:13, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 26 June 2013 14:52, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> On 26 June 2013 14:40, Will Deacon <will.deacon@arm.com> wrote:
>>> With what we currently have in the kernel, no, I can't think of a better
>>> way. However, I also don't think that smuggling in a back-end hack is a good
>>> idea either. How about we follow x86's lead on this and rely on the caller
>>> not to sleep for the timebeing? Then, separately to this patch series, you
>>> could look at augmenting the scheduler so that schedule_debug can complain
>>> if it encounters a task that is not expected to sleep? That seems like the
>>> right place to fix this problem, and will benefit other architectures too.
>>>
>>
>> Good point. As preempt_enable/disable already have this side effect on
>> PREEMPT builds, perhaps it wouldn't be such a bad idea to modify them
>> in the non-PREEMPT case to at least complain if schedule() is invoked
>> during such a section.
>>
>
> It appears we have this already, but in the non-PREEMPT case, it needs
> CONFIG_DEBUG_ATOMIC_SLEEP to be defined.
> Let's just rely on preempt_disable() to do the right thing...
>

OK, just one more question before I respin the next (hopefully final) version:
if a caller does sleep after calling kernel_neon_begin() (and thus
receives no warning if he runs a non-PREEMPT build with
CONFIG_DEBUG_ATOMIC_SLEEP disabled), he will most likely find the
NEON/VFP unit disabled after waking up (as we disable it on a context
switch), so any subsequent NEON instructions will trigger the undef
handler.

Should I perhaps expand the vfp_kmode_exception() function which gets
invoked in this case to be more helpful in identifying this condition?
Currently it just BUG()s on conditions that indicate dependence on
support code, and reports an undefined instruction otherwise.
Will Deacon June 27, 2013, 3:09 p.m. UTC | #8
On Thu, Jun 27, 2013 at 02:11:27PM +0100, Ard Biesheuvel wrote:
> On 26 June 2013 15:13, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > On 26 June 2013 14:52, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >> On 26 June 2013 14:40, Will Deacon <will.deacon@arm.com> wrote:
> >>> With what we currently have in the kernel, no, I can't think of a better
> >>> way. However, I also don't think that smuggling in a back-end hack is a good
> >>> idea either. How about we follow x86's lead on this and rely on the caller
> >>> not to sleep for the timebeing? Then, separately to this patch series, you
> >>> could look at augmenting the scheduler so that schedule_debug can complain
> >>> if it encounters a task that is not expected to sleep? That seems like the
> >>> right place to fix this problem, and will benefit other architectures too.
> >>>
> >>
> >> Good point. As preempt_enable/disable already have this side effect on
> >> PREEMPT builds, perhaps it wouldn't be such a bad idea to modify them
> >> in the non-PREEMPT case to at least complain if schedule() is invoked
> >> during such a section.
> >>
> >
> > It appears we have this already, but in the non-PREEMPT case, it needs
> > CONFIG_DEBUG_ATOMIC_SLEEP to be defined.
> > Let's just rely on preempt_disable() to do the right thing...
> >
> 
> OK, just one more question before I respin the next (hopefully final) version:
> if a caller does sleep after calling kernel_neon_begin() (and thus
> receives no warning if he runs a non-PREEMPT build with
> CONFIG_DEBUG_ATOMIC_SLEEP disabled), he will most likely find the
> NEON/VFP unit disabled after waking up (as we disable it on a context
> switch), so any subsequent NEON instructions will trigger the undef
> handler.
> 
> Should I perhaps expand the vfp_kmode_exception() function which gets
> invoked in this case to be more helpful in identifying this condition?
> Currently it just BUG()s on conditions that indicate dependence on
> support code, and reports an undefined instruction otherwise.

I don't think you need to worry too much about this. We can enable the debug
option if we want proper debugging and the BUG is a good indicator to go and
investigate a potential problem.

Will
Catalin Marinas June 27, 2013, 3:13 p.m. UTC | #9
On Thu, Jun 27, 2013 at 02:11:27PM +0100, Ard Biesheuvel wrote:
> On 26 June 2013 15:13, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > On 26 June 2013 14:52, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >> On 26 June 2013 14:40, Will Deacon <will.deacon@arm.com> wrote:
> >>> With what we currently have in the kernel, no, I can't think of a better
> >>> way. However, I also don't think that smuggling in a back-end hack is a good
> >>> idea either. How about we follow x86's lead on this and rely on the caller
> >>> not to sleep for the timebeing? Then, separately to this patch series, you
> >>> could look at augmenting the scheduler so that schedule_debug can complain
> >>> if it encounters a task that is not expected to sleep? That seems like the
> >>> right place to fix this problem, and will benefit other architectures too.
> >>>
> >>
> >> Good point. As preempt_enable/disable already have this side effect on
> >> PREEMPT builds, perhaps it wouldn't be such a bad idea to modify them
> >> in the non-PREEMPT case to at least complain if schedule() is invoked
> >> during such a section.
> >>
> >
> > It appears we have this already, but in the non-PREEMPT case, it needs
> > CONFIG_DEBUG_ATOMIC_SLEEP to be defined.
> > Let's just rely on preempt_disable() to do the right thing...
> 
> OK, just one more question before I respin the next (hopefully final) version:
> if a caller does sleep after calling kernel_neon_begin() (and thus
> receives no warning if he runs a non-PREEMPT build with
> CONFIG_DEBUG_ATOMIC_SLEEP disabled), he will most likely find the
> NEON/VFP unit disabled after waking up (as we disable it on a context
> switch), so any subsequent NEON instructions will trigger the undef
> handler.

Can you check on the VFP context switch path whether kernel_neon_begin()
has been called and we are moving away from the task? You could even
store the LR in kernel_neon_begin() to give better error information.
Ard Biesheuvel June 27, 2013, 3:43 p.m. UTC | #10
On 27 June 2013 17:13, Catalin Marinas <catalin.marinas@arm.com> wrote:
> On Thu, Jun 27, 2013 at 02:11:27PM +0100, Ard Biesheuvel wrote:
>> OK, just one more question before I respin the next (hopefully final) version:
>> if a caller does sleep after calling kernel_neon_begin() (and thus
>> receives no warning if he runs a non-PREEMPT build with
>> CONFIG_DEBUG_ATOMIC_SLEEP disabled), he will most likely find the
>> NEON/VFP unit disabled after waking up (as we disable it on a context
>> switch), so any subsequent NEON instructions will trigger the undef
>> handler.
>
> Can you check on the VFP context switch path whether kernel_neon_begin()
> has been called and we are moving away from the task? You could even
> store the LR in kernel_neon_begin() to give better error information.
>

I guess it should be quite doable to add the LR of the most recent
kernel_neon_begin() call to the vfpstate (and clear it in
kernel_neon_end), so we can check it when vfp_notifier() is called.
We'll still hit the vfp undef handler on swapping the task back in, so
it may be a bit redundant.

What I could do is use this LR field in vfpstate when hitting the
undef handler to distinguish between (a) kernel_neon_begin()
erroneously not having called at all and (b) kernel_neon_begin()
having been called before sleeping (and print a stacktrace in the
latter case)
Ard Biesheuvel June 28, 2013, 10:25 a.m. UTC | #11
On 27 June 2013 17:13, Catalin Marinas <catalin.marinas@arm.com> wrote:
> On Thu, Jun 27, 2013 at 02:11:27PM +0100, Ard Biesheuvel wrote:
>> OK, just one more question before I respin the next (hopefully final) version:
>> if a caller does sleep after calling kernel_neon_begin() (and thus
>> receives no warning if he runs a non-PREEMPT build with
>> CONFIG_DEBUG_ATOMIC_SLEEP disabled), he will most likely find the
>> NEON/VFP unit disabled after waking up (as we disable it on a context
>> switch), so any subsequent NEON instructions will trigger the undef
>> handler.
>
> Can you check on the VFP context switch path whether kernel_neon_begin()
> has been called and we are moving away from the task? You could even
> store the LR in kernel_neon_begin() to give better error information.
>

I wil take this suggestion for the arm64 case, and propose a new
version next week. For arm, I think we should be ok without this, as
Will also suggested, because you will always hit the BUG() in
vfp_kmode_exception() if you touch the NEON from the kernel after a
context switch (there are only two ways to get the NEON enabled, one
is through lazy restore, which now only works from userland, and the
other is through kernel_neon_begin())
Jean-Christophe PLAGNIOL-VILLARD June 28, 2013, 1:46 p.m. UTC | #12
On 12:55 Wed 26 Jun     , Ard Biesheuvel wrote:
> Replying to self:
> 
> On 25 June 2013 22:24, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > +void kernel_neon_end(void)
> > +{
> > +       /* Disable the NEON/VFP unit. */
> > +       fmxr(FPEXC, fmrx(FPEXC) & ~FPEXC_EN);
> > +       barrier();
> > +       dec_preempt_count();
> > +}
> > +EXPORT_SYMBOL(kernel_neon_end);
> 
> Meh. This is not going to please the RT crowd, as preempt_schedule()
> will not be called on PREEMPT builds in this case.
> 
> Propose to replace it with
> 
>     preempt_enable();
> #ifndef CONFIG_PREEMPT_COUNT
if (IS_ENABLED(CONFIG_xxx)) at least
>     /* in this case, the preempt_enable() right above is just a barrier() */
>     dec_preempt_count();
> #endif

but why do you need to call inc_preempt and dec directly?

Best Regards,
J.
Ard Biesheuvel June 28, 2013, 2 p.m. UTC | #13
On 28 June 2013 15:46, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com> wrote:
> On 12:55 Wed 26 Jun     , Ard Biesheuvel wrote:
>> Meh. This is not going to please the RT crowd, as preempt_schedule()
>> will not be called on PREEMPT builds in this case.
>>
>> Propose to replace it with
>>
>>     preempt_enable();
>> #ifndef CONFIG_PREEMPT_COUNT
> if (IS_ENABLED(CONFIG_xxx)) at least
>>     /* in this case, the preempt_enable() right above is just a barrier() */
>>     dec_preempt_count();
>> #endif
>
> but why do you need to call inc_preempt and dec directly?
>

There is a concern that violations of the rule that a task should not
sleep between kernel_neon_begin and kernel_neon_end calls may not be
spotted on non-PREEMPT builds that don't have
CONFIG_DEBUG_ATOMIC_SLEEP set. However, in this case (as I pointed out
in my previous mail), you will at least oops the kernel with a message
that points to in-kernel use of the NEON/VFP, so perhaps we should not
be too paranoid about this. On the other hand, considering that this
stuff is intended to be used for RAID-6 checksumming etc, it's better
to err on the side of caution.

On arm64, it's a bit worse, as there is not lazy restore for the FP
context (and hence no oops if you sleep in the wrong place), so
context switches that clobber the NEON register contents may not be
detectable at all.
Catalin Marinas June 28, 2013, 3:46 p.m. UTC | #14
On Fri, Jun 28, 2013 at 03:00:02PM +0100, Ard Biesheuvel wrote:
> On 28 June 2013 15:46, Jean-Christophe PLAGNIOL-VILLARD
> <plagnioj@jcrosoft.com> wrote:
> > On 12:55 Wed 26 Jun     , Ard Biesheuvel wrote:
> >> Meh. This is not going to please the RT crowd, as preempt_schedule()
> >> will not be called on PREEMPT builds in this case.
> >>
> >> Propose to replace it with
> >>
> >>     preempt_enable();
> >> #ifndef CONFIG_PREEMPT_COUNT
> > if (IS_ENABLED(CONFIG_xxx)) at least
> >>     /* in this case, the preempt_enable() right above is just a barrier() */
> >>     dec_preempt_count();
> >> #endif
> >
> > but why do you need to call inc_preempt and dec directly?
> 
> There is a concern that violations of the rule that a task should not
> sleep between kernel_neon_begin and kernel_neon_end calls may not be
> spotted on non-PREEMPT builds that don't have
> CONFIG_DEBUG_ATOMIC_SLEEP set.

Would an explicit call to schedule() trigger with
CONFIG_DEBUG_ATOMIC_SLEEP? It looks that this config option only
triggers for explicit might_sleep() calls but we don't have one for
explicit schedule() calls (cond_resched() call has might_sleep()).
Ard Biesheuvel June 28, 2013, 8:17 p.m. UTC | #15
On 28 June 2013 17:46, Catalin Marinas <catalin.marinas@arm.com> wrote:
> On Fri, Jun 28, 2013 at 03:00:02PM +0100, Ard Biesheuvel wrote:
>> On 28 June 2013 15:46, Jean-Christophe PLAGNIOL-VILLARD
>> <plagnioj@jcrosoft.com> wrote:
>> > On 12:55 Wed 26 Jun     , Ard Biesheuvel wrote:
>> >> Meh. This is not going to please the RT crowd, as preempt_schedule()
>> >> will not be called on PREEMPT builds in this case.
>> >>
>> >> Propose to replace it with
>> >>
>> >>     preempt_enable();
>> >> #ifndef CONFIG_PREEMPT_COUNT
>> > if (IS_ENABLED(CONFIG_xxx)) at least
>> >>     /* in this case, the preempt_enable() right above is just a barrier() */
>> >>     dec_preempt_count();
>> >> #endif
>> >
>> > but why do you need to call inc_preempt and dec directly?
>>
>> There is a concern that violations of the rule that a task should not
>> sleep between kernel_neon_begin and kernel_neon_end calls may not be
>> spotted on non-PREEMPT builds that don't have
>> CONFIG_DEBUG_ATOMIC_SLEEP set.
>
> Would an explicit call to schedule() trigger with
> CONFIG_DEBUG_ATOMIC_SLEEP? It looks that this config option only
> triggers for explicit might_sleep() calls but we don't have one for
> explicit schedule() calls (cond_resched() call has might_sleep()).
>

CONFIG_DEBUG_ATOMIC_SLEEP enables CONFIG_PREEMPT_COUNT, which is
enough for schedule_debug() to barf if schedule() is called in a
preempt_enable/disable section. (Hence my original approach to
increase/decrease the preempt count, but the problem with that is that
it doesn't force the schedule() to occur when the preempt count drops
to zero)
diff mbox

Patch

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 2651b1d..1187e64 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -2183,6 +2183,13 @@  config NEON
 	  Say Y to include support code for NEON, the ARMv7 Advanced SIMD
 	  Extension.
 
+config KERNEL_MODE_NEON
+	bool "Support for NEON in kernel mode"
+	default n
+	depends on NEON
+	help
+	  Say Y to include support for NEON in kernel mode.
+
 endmenu
 
 menu "Userspace binary formats"
diff --git a/arch/arm/include/asm/neon.h b/arch/arm/include/asm/neon.h
new file mode 100644
index 0000000..8f730fe
--- /dev/null
+++ b/arch/arm/include/asm/neon.h
@@ -0,0 +1,36 @@ 
+/*
+ * linux/arch/arm/include/asm/neon.h
+ *
+ * Copyright (C) 2013 Linaro Ltd <ard.biesheuvel@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <asm/hwcap.h>
+
+#define cpu_has_neon()		(!!(elf_hwcap & HWCAP_NEON))
+
+#ifdef __ARM_NEON__
+
+/*
+ * If you are affected by the BUILD_BUG below, it probably means that you are
+ * using NEON code /and/ calling the kernel_neon_begin() function from the same
+ * compilation unit. To prevent issues that may arise from GCC reordering or
+ * generating(1) NEON instructions outside of these begin/end functions, the
+ * only supported way of using NEON code in the kernel is by isolating it in a
+ * separate compilation unit, and calling it from another unit from inside a
+ * kernel_neon_begin/kernel_neon_end pair.
+ *
+ * (1) Current GCC (4.7) might generate NEON instructions at O3 level if
+ *     -mpfu=neon is set.
+ */
+
+#define kernel_neon_begin() \
+	BUILD_BUG_ON_MSG(1, "kernel_neon_begin() called from NEON code")
+
+#else
+void kernel_neon_begin(void);
+#endif
+void kernel_neon_end(void);
diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c
index fd1466c..b64ca77 100644
--- a/arch/arm/vfp/vfpmodule.c
+++ b/arch/arm/vfp/vfpmodule.c
@@ -20,6 +20,7 @@ 
 #include <linux/init.h>
 #include <linux/uaccess.h>
 #include <linux/user.h>
+#include <linux/export.h>
 
 #include <asm/cp15.h>
 #include <asm/cputype.h>
@@ -659,6 +660,59 @@  void vfp_kmode_exception(void)
 	BUG_ON(fmrx(FPEXC) & FPEXC_EN);
 }
 
+#ifdef CONFIG_KERNEL_MODE_NEON
+
+/*
+ * Kernel-side NEON support functions
+ */
+void kernel_neon_begin(void)
+{
+	struct thread_info *thread = current_thread_info();
+	unsigned int cpu;
+	u32 fpexc;
+
+	/*
+	 * Kernel mode NEON is only allowed outside of interrupt context
+	 * with preemption disabled. This will make sure that the kernel
+	 * mode NEON register contents never need to be preserved.
+	 *
+	 * Use inc_preempt_count() instead of preempt_disable() so sleeping
+	 * complains noisily even on builds that have kernel preemption
+	 * disabled.
+	 */
+	BUG_ON(in_interrupt());
+	inc_preempt_count();
+	barrier();
+	cpu = smp_processor_id();
+
+	fpexc = fmrx(FPEXC) | FPEXC_EN;
+	fmxr(FPEXC, fpexc);
+
+	/*
+	 * Save the userland NEON/VFP state. Under UP,
+	 * the owner could be a task other than 'current'
+	 */
+	if (vfp_state_in_hw(cpu, thread))
+		vfp_save_state(&thread->vfpstate, fpexc);
+#ifndef CONFIG_SMP
+	else if (vfp_current_hw_state[cpu] != NULL)
+		vfp_save_state(vfp_current_hw_state[cpu], fpexc);
+#endif
+	vfp_current_hw_state[cpu] = NULL;
+}
+EXPORT_SYMBOL(kernel_neon_begin);
+
+void kernel_neon_end(void)
+{
+	/* Disable the NEON/VFP unit. */
+	fmxr(FPEXC, fmrx(FPEXC) & ~FPEXC_EN);
+	barrier();
+	dec_preempt_count();
+}
+EXPORT_SYMBOL(kernel_neon_end);
+
+#endif /* CONFIG_KERNEL_MODE_NEON */
+
 /*
  * VFP support code initialisation.
  */