[3/5] ARC: handle DSP presence in HW

In case of DSP extension presence in HW some instructions
(related to integer multiply, multiply-accumulate, and divide
operation) executes on this DSP execution unit. So their
execution will depend on dsp configuration register (DSP_CTRL)

As we want these instructions to execute the same way regardless
of DSP presence we need to set DSP_CTRL properly. However this
register can be modified bu any usersace app therefore any
usersace may break kernel execution.

Fix that by configure DSP_CTRL in CPU early code and in IRQs
entries.

Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
---
 arch/arc/Kconfig                   | 25 ++++++++++++++++-
 arch/arc/include/asm/arcregs.h     | 12 ++++++++
 arch/arc/include/asm/dsp-impl.h    | 45 ++++++++++++++++++++++++++++++
 arch/arc/include/asm/entry-arcv2.h |  3 ++
 arch/arc/kernel/head.S             |  4 +++
 arch/arc/kernel/setup.c            |  4 +++
 6 files changed, 92 insertions(+), 1 deletion(-)
 create mode 100644 arch/arc/include/asm/dsp-impl.h

On 12/27/19 10:03 AM, Eugeniy Paltsev wrote:
> In case of DSP extension presence in HW some instructions
> (related to integer multiply, multiply-accumulate, and divide
> operation) executes on this DSP execution unit. So their
> execution will depend on dsp configuration register (DSP_CTRL)
>
> As we want these instructions to execute the same way regardless
> of DSP presence we need to set DSP_CTRL properly. However this
> register can be modified bu any usersace app therefore any
> usersace may break kernel execution.
>
> Fix that by configure DSP_CTRL in CPU early code and in IRQs
> entries.
>
> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> ---
>  arch/arc/Kconfig                   | 25 ++++++++++++++++-
>  arch/arc/include/asm/arcregs.h     | 12 ++++++++
>  arch/arc/include/asm/dsp-impl.h    | 45 ++++++++++++++++++++++++++++++
>  arch/arc/include/asm/entry-arcv2.h |  3 ++
>  arch/arc/kernel/head.S             |  4 +++
>  arch/arc/kernel/setup.c            |  4 +++
>  6 files changed, 92 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arc/include/asm/dsp-impl.h
>
> diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
> index 8383155c8c82..b9cd7ce3f878 100644
> --- a/arch/arc/Kconfig
> +++ b/arch/arc/Kconfig
> @@ -404,13 +404,36 @@ config ARC_HAS_DIV_REM
>  	default y
>  
>  config ARC_HAS_ACCL_REGS
> -	bool "Reg Pair ACCL:ACCH (FPU and/or MPY > 6)"
> +	bool "Reg Pair ACCL:ACCH (FPU and/or MPY > 6 and/or DSP)"
>  	default y
>  	help
>  	  Depending on the configuration, CPU can contain accumulator reg-pair
>  	  (also referred to as r58:r59). These can also be used by gcc as GPR so
>  	  kernel needs to save/restore per process
>  
> +choice
> +	prompt "DSP support"
> +	default ARC_NO_DSP
> +	help
> +	  Depending on the configuration, CPU can contain DSP registers
> +	  (ACC0_GLO, ACC0_GHI, DSP_BFLY0, DSP_CTRL, DSP_FFT_CTRL).
> +	  Bellow is options describing how to handle these registers in
> +	  interrupt entry / exit and in context switch.
> +
> +config ARC_NO_DSP

Can this be called ARC_DSP_NONE for better intuitive regex searches !

> +	bool "No DSP extension presence in HW"
> +	help
> +	  No DSP extension presence in HW
> +
> +config ARC_DSP_KERNEL
> +	bool "DSP extension in HW, no support for userspace"
> +	select ARC_HAS_ACCL_REGS
> +	help
> +	  DSP extension presence in HW, no support for DSP-enabled userspace
> +	  applications. We don't save / restore DSP registers and only do
> +	  some minimal preparations so userspace won't be able to break kernel
> +endchoice
> +
>  config ARC_IRQ_NO_AUTOSAVE
>  	bool "Disable hardware autosave regfile on interrupts"
>  	default n
> diff --git a/arch/arc/include/asm/arcregs.h b/arch/arc/include/asm/arcregs.h
> index 5134f0baf33c..0004b1e9b325 100644
> --- a/arch/arc/include/asm/arcregs.h
> +++ b/arch/arc/include/asm/arcregs.h
> @@ -116,6 +116,18 @@
>  #define ARC_AUX_DPFP_2H         0x304
>  #define ARC_AUX_DPFP_STAT       0x305
>  
> +/*
> + * DSP-related registers
> + */
> +#define ARC_AUX_DSP_BUILD	0x7A
> +#define ARC_AUX_ACC0_LO		0x580
> +#define ARC_AUX_ACC0_GLO	0x581
> +#define ARC_AUX_ACC0_HI		0x582
> +#define ARC_AUX_ACC0_GHI	0x583
> +#define ARC_AUX_DSP_BFLY0	0x598
> +#define ARC_AUX_DSP_CTRL	0x59F
> +#define ARC_AUX_DSP_FFT_CTRL	0x59E
> +
>  #ifndef __ASSEMBLY__
>  
>  #include <soc/arc/aux.h>
> diff --git a/arch/arc/include/asm/dsp-impl.h b/arch/arc/include/asm/dsp-impl.h
> new file mode 100644
> index 000000000000..788093cbe689
> --- /dev/null
> +++ b/arch/arc/include/asm/dsp-impl.h
> @@ -0,0 +1,45 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2020 Synopsys, Inc. (www.synopsys.com)
> + *
> + * Author: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> + */
> +#ifndef __ASM_ARC_DSP_IMPL_H
> +#define __ASM_ARC_DSP_IMPL_H
> +
> +#define DSP_CTRL_DISABLED_ALL		0
> +
> +#ifdef __ASSEMBLY__
> +
> +/* clobbers r5 register */
> +.macro DSP_EARLY_INIT
> +	lr	r5, [ARC_AUX_DSP_BUILD]
> +	bmsk	r5, r5, 7
> +	breq    r5, 0, 1f
> +	mov	r5, DSP_CTRL_DISABLED_ALL
> +	sr	r5, [ARC_AUX_DSP_CTRL]
> +1:
> +.endm
> +
> +/* clobbers r58, r59 registers pair */
> +.macro DSP_SAVE_REGFILE_IRQ
> +#if defined(CONFIG_ARC_DSP_KERNEL)
> +	/* Drop any changes to DSP_CTRL made by userspace so userspace won't be
> +	 * able to break kernel */
> +	mov	r58, DSP_CTRL_DISABLED_ALL
> +	sr	r58, [ARC_AUX_DSP_CTRL]

In low level entry code, we typically use r10/r11 for scratch purposes, can u use
r10 here for consistency !

> +#endif /* ARC_DSP_KERNEL */
> +.endm
> +
> +#else /* __ASEMBLY__ */
> +
> +static inline bool dsp_exist(void)
> +{
> +	struct bcr_generic bcr;
> +
> +	READ_BCR(ARC_AUX_DSP_BUILD, bcr);
> +	return !!bcr.ver;

open code these use once / one liners in the call site itself.

> +}
> +
> +#endif /* __ASEMBLY__ */
> +#endif /* __ASM_ARC_DSP_IMPL_H */
> diff --git a/arch/arc/include/asm/entry-arcv2.h b/arch/arc/include/asm/entry-arcv2.h
> index 0b8b63d0bec1..e3f8bd3e2eba 100644
> --- a/arch/arc/include/asm/entry-arcv2.h
> +++ b/arch/arc/include/asm/entry-arcv2.h
> @@ -4,6 +4,7 @@
>  #define __ASM_ARC_ENTRY_ARCV2_H
>  
>  #include <asm/asm-offsets.h>
> +#include <asm/dsp-impl.h>
>  #include <asm/irqflags-arcv2.h>
>  #include <asm/thread_info.h>	/* For THREAD_SIZE */
>  
> @@ -165,6 +166,8 @@
>  	ST2	r58, r59, PT_r58
>  #endif
>  
> +	/* clobbers r58, r59 registers pair, so must be after r58, r59 save */
> +	DSP_SAVE_REGFILE_IRQ
>  .endm
>  
>  /*------------------------------------------------------------------------*/
> diff --git a/arch/arc/kernel/head.S b/arch/arc/kernel/head.S
> index 6f41265f6250..6eb23f1545ee 100644
> --- a/arch/arc/kernel/head.S
> +++ b/arch/arc/kernel/head.S
> @@ -14,6 +14,7 @@
>  #include <asm/entry.h>
>  #include <asm/arcregs.h>
>  #include <asm/cache.h>
> +#include <asm/dsp-impl.h>
>  #include <asm/irqflags.h>
>  
>  .macro CPU_EARLY_SETUP
> @@ -59,6 +60,9 @@
>  #endif
>  	kflag	r5
>  #endif
> +	; Config DSP_CTRL properly, so kernel may use integer multiply,
> +	; multiply-accumulate, and divide operations



> +	DSP_EARLY_INIT
>  .endm
>  
>  	.section .init.text, "ax",@progbits
> diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c
> index edb55b6ee278..b3995dd673d9 100644
> --- a/arch/arc/kernel/setup.c
> +++ b/arch/arc/kernel/setup.c
> @@ -26,6 +26,7 @@
>  #include <asm/unwind.h>
>  #include <asm/mach_desc.h>
>  #include <asm/smp.h>
> +#include <asm/dsp-impl.h>
>  
>  #define FIX_PTR(x)  __asm__ __volatile__(";" : "+r"(x))
>  
> @@ -444,6 +445,9 @@ static void arc_chk_core_config(void)
>  		/* Accumulator Low:High pair (r58:59) present if DSP MPY or FPU */
>  		present = cpu->extn_mpy.dsp | cpu->extn.fpu_sp | cpu->extn.fpu_dp;
>  		CHK_OPT_STRICT(CONFIG_ARC_HAS_ACCL_REGS, present);
> +
> +		present = dsp_exist();

Open code as suggested above.

> +		CHK_OPT_STRICT(CONFIG_ARC_DSP_KERNEL, present);
>  	}
>  }
>

On 1/6/20 5:03 PM, Vineet Gupta wrote:
>
>> +.macro DSP_SAVE_REGFILE_IRQ
>> +#if defined(CONFIG_ARC_DSP_KERNEL)
>> +	/* Drop any changes to DSP_CTRL made by userspace so userspace won't be
>> +	 * able to break kernel */
>> +	mov	r58, DSP_CTRL_DISABLED_ALL
>> +	sr	r58, [ARC_AUX_DSP_CTRL]
>

This also clears the sticky flag DSP_CTRL.SAT, can you check with DSP lib folks if
they rely on this bit in any way whatsoever !

Hi Vineet,

>From: Vineet Gupta <vgupta@synopsys.com>
>Sent: Tuesday, January 7, 2020 04:03
>To: Eugeniy Paltsev; linux-snps-arc@lists.infradead.org
>Cc: linux-kernel@vger.kernel.org; Alexey Brodkin
>Subject: Re: [PATCH 3/5] ARC: handle DSP presence in HW
>[snip]
>> +static inline bool dsp_exist(void)
>> +{
>> +     struct bcr_generic bcr;
>> +
>> +     READ_BCR(ARC_AUX_DSP_BUILD, bcr);
>> +     return !!bcr.ver;
>
>open code these use once / one liners in the call site itself.
>
>>
>> @@ -444,6 +445,9 @@ static void arc_chk_core_config(void)
>>               /* Accumulator Low:High pair (r58:59) present if DSP MPY or FPU */
>>               present = cpu->extn_mpy.dsp | cpu->extn.fpu_sp | cpu->extn.fpu_dp;
>>               CHK_OPT_STRICT(CONFIG_ARC_HAS_ACCL_REGS, present);
>> +
>> +             present = dsp_exist();
>
>Open code as suggested above.
>
>> +             CHK_OPT_STRICT(CONFIG_ARC_DSP_KERNEL, present);
>>       }

My idea here is to encapsulate implementation of everything dsp-related in the
file with dsp code. So I'm even thinking about moving the config check itself
to some function like
'arc_chk_dsp_config' which will be located in dsp.x file
and call it from arc_chk_core_config in setup.c

This requires to move config check helpers to separate file/header from the setup.c
However this allows encapsulate all DSP code (and some new subsystems code later on) in its files instead of spread it across the arc code.

What do you think about it? If you really dislike this idea I can drop it.
---
 Eugeniy Paltsev

On 1/10/20 6:54 AM, Eugeniy Paltsev wrote:
>
>>> +             CHK_OPT_STRICT(CONFIG_ARC_DSP_KERNEL, present);
>>>       }
> My idea here is to encapsulate implementation of everything dsp-related in the
> file with dsp code. So I'm even thinking about moving the config check itself
> to some function like
> 'arc_chk_dsp_config' which will be located in dsp.x file
> and call it from arc_chk_core_config in setup.c
>
> This requires to move config check helpers to separate file/header from the setup.c
> However this allows encapsulate all DSP code (and some new subsystems code later on) in its files instead of spread it across the arc code.
>
> What do you think about it? If you really dislike this idea I can drop it.

Ok makes sense then !

-Vineet