Re: [PATCH] arm64/sve: Make kernel FPU protection RT friendly

[Dave Martin <Dave.Martin@arm.com>]

On Thu, Jul 29, 2021 at 12:52:15PM +0200, Sebastian Andrzej Siewior wrote:
> Non RT kernels need to protect FPU against preemption and bottom half
> processing. This is achieved by disabling bottom halves via
> local_bh_disable() which implictly disables preemption.
> 
> On RT kernels this protection mechanism is not sufficient because
> local_bh_disable() does not disable preemption. It serializes bottom half
> related processing via a CPU local lock.
> 
> As bottom halves are running always in thread context on RT kernels
> disabling preemption is the proper choice as it implicitly prevents bottom
> half processing.
> 
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
>  arch/arm64/kernel/fpsimd.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> index e098f6c67b1de..a208514bd69a9 100644
> --- a/arch/arm64/kernel/fpsimd.c
> +++ b/arch/arm64/kernel/fpsimd.c
> @@ -177,10 +177,19 @@ static void __get_cpu_fpsimd_context(void)
>   *
>   * The double-underscore version must only be called if you know the task
>   * can't be preempted.
> + *
> + * On RT kernels local_bh_disable() is not sufficient because it only
> + * serializes soft interrupt related sections via a local lock, but stays
> + * preemptible. Disabling preemption is the right choice here as bottom
> + * half processing is always in thread context on RT kernels so it
> + * implicitly prevents bottom half processing as well.
>   */
>  static void get_cpu_fpsimd_context(void)
>  {
> -	local_bh_disable();
> +	if (!IS_ENABLED(CONFIG_PREEMPT_RT))
> +		local_bh_disable();
> +	else
> +		preempt_disable();

Is this wrongly abstracted for RT?

The requirement here is that the code should temporarily be
nonpreemptible by anything except hardirq context.

Having to do this conditional everywhere that is required feels fragile.
Is a similar thing needed anywhere else?

If bh (as a preempting context) doesn't exist on RT, then can't
local_bh_disable() just suppress all preemption up to but excluding
hardirq?  Would anything break?

[...]

Cheers
---Dave

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel