2020-03-05 20:04:46

by Eugeniy Paltsev

[permalink] [raw]
Subject: [PATCH v2 2/4] 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 <[email protected]>
---
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
+ interrupt entry / exit and in context switch.
+
+config ARC_DSP_NONE
+ 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
+ select ARC_DSP_HANDLED
+ 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 f7e432448e4b..135f6ec08a69 100644
--- a/arch/arc/include/asm/arcregs.h
+++ b/arch/arc/include/asm/arcregs.h
@@ -118,6 +118,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..606620383eca
--- /dev/null
+++ b/arch/arc/include/asm/dsp-impl.h
@@ -0,0 +1,54 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2020 Synopsys, Inc. (http://www.synopsys.com)
+ *
+ * Author: Eugeniy Paltsev <[email protected]>
+ */
+#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 r10, r11 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 - reset it to DSP_CTRL_DISABLED_ALL value
+ */
+ mov r10, DSP_CTRL_DISABLED_ALL
+ sr r10, [ARC_AUX_DSP_CTRL]
+#endif /* ARC_DSP_KERNEL */
+.endm
+
+#else /* __ASEMBLY__ */
+
+#include <asm/asserts.h>
+
+static inline bool dsp_exist(void)
+{
+ struct bcr_generic bcr;
+
+ READ_BCR(ARC_AUX_DSP_BUILD, bcr);
+ return !!bcr.ver;
+}
+
+static inline void dsp_config_check(void)
+{
+ CHK_OPT_STRICT(CONFIG_ARC_DSP_HANDLED, dsp_exist());
+}
+
+#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..dd6aa18b51ca 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 r10, r11 registers pair */
+ 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 820c0cfb0702..1ed1528d9045 100644
--- a/arch/arc/kernel/setup.c
+++ b/arch/arc/kernel/setup.c
@@ -27,6 +27,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))

@@ -440,6 +441,8 @@ 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);
+
+ dsp_config_check();
}
}

--
2.21.1


2020-03-07 00:14:35

by Vineet Gupta

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

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 <[email protected]>
> ---
> 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 <[email protected]>

2020-03-10 12:19:17

by Eugeniy Paltsev

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

>From: Vineet Gupta <[email protected]>
>Sent: Saturday, March 7, 2020 03:12
>To: Eugeniy Paltsev; [email protected]
>Cc: Alexey Brodkin; [email protected]
>Subject: Re: [PATCH v2 2/4] ARC: handle DSP presence in HW
>
>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."
>From: Vineet Gupta <[email protected]>
>Sent: Saturday, March 7, 2020 03:12
>To: Eugeniy Paltsev; [email protected]
>Cc: Alexey Brodkin; [email protected]
>Subject: Re: [PATCH v2 2/4] ARC: handle DSP presence in HW
>
>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."

2020-03-10 12:20:10

by Eugeniy Paltsev

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

>From: Vineet Gupta <[email protected]>
>Sent: Saturday, March 7, 2020 03:12
>To: Eugeniy Paltsev; [email protected]
>Cc: Alexey Brodkin; [email protected]
>Subject: Re: [PATCH v2 2/4] ARC: handle DSP presence in HW
>
>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 ....

Good description, ACK.

> "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."