Re: [PATCH v2 2/4] ARC: handle DSP presence in HW

[Vineet Gupta <Vineet.Gupta1@synopsys.com>]

On 3/5/20 12:02 PM, 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.

How about below ....

"When DSP extensions are present, some of the regular integer instructions such as
DIV, MACD etc are executed in the DSP unit with semantics alterable by flags in
DSP_CTRL aux register. This register is writable by userspace and thus can
potentially affect corresponding instructions in kernel code, intentionally or
otherwise. So safegaurd kernel by effectively disabling DSP_CTRL upon bootup and
every entry to kernel.

Do note that for this config we simply zero out the DSP_CTRL reg assuming
userspace doesn't really care about DSP. The next patch caters to the DSP aware
userspace which this actually saved/restored upon kernel entry."



> 
> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
> ---
>  arch/arc/Kconfig                   | 29 +++++++++++++++-
>  arch/arc/include/asm/arcregs.h     | 12 +++++++
>  arch/arc/include/asm/dsp-impl.h    | 54 ++++++++++++++++++++++++++++++
>  arch/arc/include/asm/entry-arcv2.h |  3 ++
>  arch/arc/kernel/head.S             |  4 +++
>  arch/arc/kernel/setup.c            |  3 ++
>  6 files changed, 104 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arc/include/asm/dsp-impl.h
> 
> diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
> index 7124ab82dfa3..55432a8fc20d 100644
> --- a/arch/arc/Kconfig
> +++ b/arch/arc/Kconfig
> @@ -401,13 +401,40 @@ 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
>  
> +config ARC_DSP_HANDLED
> +	def_bool n
> +
> +choice
> +	prompt "DSP support"
> +	default ARC_DSP_NONE
> +	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

typo: Below

Looks good otherwise. No need to respin just for this.

Reviewed-by: Vineet Gupta <vgupta@synopsys.com>