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

2020-01-13 Thread Vineet Gupta
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
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc


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

2020-01-10 Thread Eugeniy Paltsev
Hi Vineet,

>From: Vineet Gupta 
>Sent: Tuesday, January 7, 2020 04:03
>To: Eugeniy Paltsev; linux-snps-arc@lists.infradead.org
>Cc: linux-ker...@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

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc


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

2020-01-06 Thread Vineet Gupta
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 !

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc


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

2020-01-06 Thread Vineet Gupta
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 
> ---
>  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_BUILD0x7A
> +#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_BFLY00x598
> +#define ARC_AUX_DSP_CTRL 0x59F
> +#define ARC_AUX_DSP_FFT_CTRL 0x59E
> +
>  #ifndef __ASSEMBLY__
>  
>  #include 
> diff --git a/arch/arc/include/asm/dsp-impl.h b/arch/arc/include/asm/dsp-impl.h
> new file mode 100644
> index ..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 
> + */
> +#ifndef __ASM_ARC_DSP_IMPL_H
> +#define __ASM_ARC_DSP_IMPL_H
> +
> +#define DSP_CTRL_DISABLED_ALL0
> +
> +#ifdef __ASSEMBLY__
> +
> +/* clobbers r5 register */
> +.macro DSP_EARLY_INIT
> + lr  r5, [ARC_AUX_DSP_BUILD]
> + bmskr5, r5, 7
> + breqr5, 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 

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

2019-12-27 Thread Eugeniy Paltsev
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 
---
 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
+   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_LO0x580
+#define ARC_AUX_ACC0_GLO   0x581
+#define ARC_AUX_ACC0_HI0x582
+#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 
diff --git a/arch/arc/include/asm/dsp-impl.h b/arch/arc/include/asm/dsp-impl.h
new file mode 100644
index ..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 
+ */
+#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]
+   bmskr5, r5, 7
+   breqr5, 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]
+#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;
+}
+
+#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 
+#include 
 #include 
 #include/* 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 */
+