Arc processors may have DSP extension which is optional.
In this patch series we:
* Handle issues caused by DSP extension presence in HW
* Add optional support for DSP-enabled applications in
userspace (with optional AGU extension support)
* Do minor cleanups
Eugeniy Paltsev (5):
ARC: pt_regs: remove hardcoded registers offset
ARC: add helpers to sanitize config options
ARC: handle DSP presence in HW
ARC: add support for DSP-enabled userspace applications
ARC: allow userspace DSP applications to use AGU extensions
arch/arc/Kconfig | 39 +++++++-
arch/arc/include/asm/arcregs.h | 26 ++++++
arch/arc/include/asm/dsp-impl.h | 141 +++++++++++++++++++++++++++++
arch/arc/include/asm/dsp.h | 50 ++++++++++
arch/arc/include/asm/entry-arcv2.h | 14 ++-
arch/arc/include/asm/processor.h | 4 +
arch/arc/include/asm/ptrace.h | 4 +
arch/arc/include/asm/switch_to.h | 2 +
arch/arc/kernel/asm-offsets.c | 16 ++++
arch/arc/kernel/head.S | 4 +
arch/arc/kernel/setup.c | 47 +++++++---
11 files changed, 329 insertions(+), 18 deletions(-)
create mode 100644 arch/arc/include/asm/dsp-impl.h
create mode 100644 arch/arc/include/asm/dsp.h
--
2.21.0
To be able to run DSP-enabled userspace applications we need to
save and restore following DSP-related registers:
At IRQ/exception entry/exit:
* ACC0_GLO, ACC0_GHI, DSP_CTRL
* ACC0_LO, ACC0_HI (we already save them as r58, r59 pair)
At context switch:
* DSP_BFLY0, DSP_FFT_CTRL
Signed-off-by: Eugeniy Paltsev <[email protected]>
---
arch/arc/Kconfig | 7 +++
arch/arc/include/asm/arcregs.h | 2 +
arch/arc/include/asm/dsp-impl.h | 75 +++++++++++++++++++++++++++++-
arch/arc/include/asm/dsp.h | 42 +++++++++++++++++
arch/arc/include/asm/entry-arcv2.h | 3 ++
arch/arc/include/asm/processor.h | 4 ++
arch/arc/include/asm/ptrace.h | 4 ++
arch/arc/include/asm/switch_to.h | 2 +
arch/arc/kernel/asm-offsets.c | 7 +++
arch/arc/kernel/setup.c | 2 +-
10 files changed, 146 insertions(+), 2 deletions(-)
create mode 100644 arch/arc/include/asm/dsp.h
diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
index b9cd7ce3f878..c3210754a3d2 100644
--- a/arch/arc/Kconfig
+++ b/arch/arc/Kconfig
@@ -432,6 +432,13 @@ config ARC_DSP_KERNEL
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
+
+config ARC_DSP_USERSPACE
+ bool "Support DSP for userspace apps"
+ select ARC_HAS_ACCL_REGS
+ help
+ DSP extension presence in HW, support save / restore DSP registers to
+ run DSP-enabled userspace applications
endchoice
config ARC_IRQ_NO_AUTOSAVE
diff --git a/arch/arc/include/asm/arcregs.h b/arch/arc/include/asm/arcregs.h
index 0004b1e9b325..a713819cab3c 100644
--- a/arch/arc/include/asm/arcregs.h
+++ b/arch/arc/include/asm/arcregs.h
@@ -118,6 +118,8 @@
/*
* DSP-related registers
+ * Registers names must correspond to dsp_callee_regs structure fields names
+ * for automatic offset calculation in DSP_AUX_SAVE_RESTORE macros.
*/
#define ARC_AUX_DSP_BUILD 0x7A
#define ARC_AUX_ACC0_LO 0x580
diff --git a/arch/arc/include/asm/dsp-impl.h b/arch/arc/include/asm/dsp-impl.h
index 788093cbe689..7b640a680dfc 100644
--- a/arch/arc/include/asm/dsp-impl.h
+++ b/arch/arc/include/asm/dsp-impl.h
@@ -7,6 +7,8 @@
#ifndef __ASM_ARC_DSP_IMPL_H
#define __ASM_ARC_DSP_IMPL_H
+#include <asm/dsp.h>
+
#define DSP_CTRL_DISABLED_ALL 0
#ifdef __ASSEMBLY__
@@ -28,11 +30,82 @@
* able to break kernel */
mov r58, DSP_CTRL_DISABLED_ALL
sr r58, [ARC_AUX_DSP_CTRL]
-#endif /* ARC_DSP_KERNEL */
+
+#elif defined(ARC_DSP_SAVE_RESTORE_REGS)
+ lr r58, [ARC_AUX_ACC0_GLO]
+ lr r59, [ARC_AUX_ACC0_GHI]
+ ST2 r58, r59, PT_ACC0_GLO
+
+ lr r58, [ARC_AUX_DSP_CTRL]
+ st r58, [sp, PT_DSP_CTRL]
+
+#endif
+.endm
+
+/* clobbers r58, r59 registers pair */
+.macro DSP_RESTORE_REGFILE_IRQ
+#if defined(ARC_DSP_SAVE_RESTORE_REGS)
+ LD2 r58, r59, PT_ACC0_GLO
+ sr r58, [ARC_AUX_ACC0_GLO]
+ sr r59, [ARC_AUX_ACC0_GHI]
+
+ ld r58, [sp, PT_DSP_CTRL]
+ sr r58, [ARC_AUX_DSP_CTRL]
+
+#endif
.endm
#else /* __ASEMBLY__ */
+#include <linux/sched.h>
+#include <asm/switch_to.h>
+
+#ifdef ARC_DSP_SAVE_RESTORE_REGS
+
+/*
+ * As we save new and restore old AUX register value in the same place we
+ * can optimize a bit and use AEX instruction (swap contents of an auxiliary
+ * register with a core register) instead of LR + SR pair.
+ */
+#define AUX_SAVE_RESTORE(_saveto, _readfrom, _offt, _aux, _scratch) \
+do { \
+ __asm__ __volatile__( \
+ "ld %0, [%2, %4] \n" \
+ "aex %0, [%3] \n" \
+ "st %0, [%1, %4] \n" \
+ : \
+ "=&r" (_scratch) /* must be early clobber */ \
+ : \
+ "r" (_saveto), \
+ "r" (_readfrom), \
+ "I" (_aux), \
+ "I" (_offt) \
+ : \
+ "memory" \
+ ); \
+} while (0)
+
+#define DSP_AUX_SAVE_RESTORE(_saveto, _readfrom, _aux, _scratch) \
+ AUX_SAVE_RESTORE(_saveto, _readfrom, \
+ offsetof(struct dsp_callee_regs, _aux), \
+ ARC_AUX_##_aux, _scratch)
+
+static inline void arc_dsp_save_restore(struct task_struct *prev,
+ struct task_struct *next)
+{
+ long unsigned int *saveto = &prev->thread.dsp.DSP_BFLY0;
+ long unsigned int *readfrom = &next->thread.dsp.DSP_BFLY0;
+
+ long unsigned int zero = 0;
+
+ DSP_AUX_SAVE_RESTORE(saveto, readfrom, DSP_BFLY0, zero);
+ DSP_AUX_SAVE_RESTORE(saveto, readfrom, DSP_FFT_CTRL, zero);
+}
+
+#else /* !ARC_DSP_SAVE_RESTORE_REGS */
+#define arc_dsp_save_restore(p, n)
+#endif /* ARC_DSP_SAVE_RESTORE_REGS */
+
static inline bool dsp_exist(void)
{
struct bcr_generic bcr;
diff --git a/arch/arc/include/asm/dsp.h b/arch/arc/include/asm/dsp.h
new file mode 100644
index 000000000000..68507f04dea4
--- /dev/null
+++ b/arch/arc/include/asm/dsp.h
@@ -0,0 +1,42 @@
+/* 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_H
+#define __ASM_ARC_DSP_H
+
+#if defined(CONFIG_ARC_DSP_USERSPACE)
+#define ARC_DSP_SAVE_RESTORE_REGS 1
+#endif
+
+#ifndef __ASSEMBLY__
+
+/* some defines to simplify config sanitize in kernel/setup.c */
+#if defined(CONFIG_ARC_DSP_KERNEL) || \
+ defined(CONFIG_ARC_DSP_USERSPACE)
+#define ARC_DSP_HANDLED 1
+#else
+#define ARC_DSP_HANDLED 0
+#endif
+
+#if defined(CONFIG_ARC_DSP_USERSPACE)
+#define ARC_DSP_OPT_NAME "CONFIG_ARC_DSP_USERSPACE"
+#else
+#define ARC_DSP_OPT_NAME "CONFIG_ARC_DSP_KERNEL"
+#endif
+
+/*
+ * DSP-related saved registers - need to be saved only when you are
+ * scheduled out.
+ * structure fields name must correspond to aux register defenitions for
+ * automatic offset calculation in DSP_AUX_SAVE_RESTORE macros
+ */
+struct dsp_callee_regs {
+ unsigned long DSP_BFLY0, DSP_FFT_CTRL;
+};
+
+#endif /* !__ASSEMBLY__ */
+
+#endif /* __ASM_ARC_DSP_H */
diff --git a/arch/arc/include/asm/entry-arcv2.h b/arch/arc/include/asm/entry-arcv2.h
index e3f8bd3e2eba..5d079f0b6243 100644
--- a/arch/arc/include/asm/entry-arcv2.h
+++ b/arch/arc/include/asm/entry-arcv2.h
@@ -192,6 +192,9 @@
ld r25, [sp, PT_user_r25]
#endif
+ /* clobbers r58, r59 registers pair, so must be before r58, r59 restore */
+ DSP_RESTORE_REGFILE_IRQ
+
#ifdef CONFIG_ARC_HAS_ACCL_REGS
LD2 r58, r59, PT_r58
#endif
diff --git a/arch/arc/include/asm/processor.h b/arch/arc/include/asm/processor.h
index 706edeaa5583..1716df0f4472 100644
--- a/arch/arc/include/asm/processor.h
+++ b/arch/arc/include/asm/processor.h
@@ -14,6 +14,7 @@
#ifndef __ASSEMBLY__
#include <asm/ptrace.h>
+#include <asm/dsp.h>
#ifdef CONFIG_ARC_FPU_SAVE_RESTORE
/* These DPFP regs need to be saved/restored across ctx-sw */
@@ -39,6 +40,9 @@ struct thread_struct {
unsigned long ksp; /* kernel mode stack pointer */
unsigned long callee_reg; /* pointer to callee regs */
unsigned long fault_address; /* dbls as brkpt holder as well */
+#ifdef ARC_DSP_SAVE_RESTORE_REGS
+ struct dsp_callee_regs dsp;
+#endif
#ifdef CONFIG_ARC_FPU_SAVE_RESTORE
struct arc_fpu fpu;
#endif
diff --git a/arch/arc/include/asm/ptrace.h b/arch/arc/include/asm/ptrace.h
index ba9854ef39e8..a5851ee141de 100644
--- a/arch/arc/include/asm/ptrace.h
+++ b/arch/arc/include/asm/ptrace.h
@@ -8,6 +8,7 @@
#define __ASM_ARC_PTRACE_H
#include <uapi/asm/ptrace.h>
+#include <asm/dsp.h>
#ifndef __ASSEMBLY__
@@ -91,6 +92,9 @@ struct pt_regs {
#ifdef CONFIG_ARC_HAS_ACCL_REGS
unsigned long r58, r59; /* ACCL/ACCH used by FPU / DSP MPY */
#endif
+#ifdef ARC_DSP_SAVE_RESTORE_REGS
+ unsigned long ACC0_GLO, ACC0_GHI, DSP_CTRL;
+#endif
/*------- Below list auto saved by h/w -----------*/
unsigned long r0, r1, r2, r3, r4, r5, r6, r7, r8, r9, r10, r11;
diff --git a/arch/arc/include/asm/switch_to.h b/arch/arc/include/asm/switch_to.h
index 77f123385e96..b49f025136af 100644
--- a/arch/arc/include/asm/switch_to.h
+++ b/arch/arc/include/asm/switch_to.h
@@ -9,6 +9,7 @@
#ifndef __ASSEMBLY__
#include <linux/sched.h>
+#include <asm/dsp-impl.h>
#ifdef CONFIG_ARC_FPU_SAVE_RESTORE
@@ -37,6 +38,7 @@ struct task_struct *__switch_to(struct task_struct *p, struct task_struct *n);
do { \
ARC_EZNPS_DP_PREV(prev, next); \
ARC_FPU_PREV(prev, next); \
+ arc_dsp_save_restore(prev, next);\
last = __switch_to(prev, next);\
ARC_FPU_NEXT(next); \
mb(); \
diff --git a/arch/arc/kernel/asm-offsets.c b/arch/arc/kernel/asm-offsets.c
index c783bcd35eb8..f74426a71ba8 100644
--- a/arch/arc/kernel/asm-offsets.c
+++ b/arch/arc/kernel/asm-offsets.c
@@ -11,6 +11,8 @@
#include <linux/ptrace.h>
#include <asm/hardirq.h>
#include <asm/page.h>
+#include <asm/dsp.h>
+
int main(void)
{
@@ -75,6 +77,11 @@ int main(void)
OFFSET(PT_r58, pt_regs, r58);
OFFSET(PT_r59, pt_regs, r59);
#endif
+#ifdef ARC_DSP_SAVE_RESTORE_REGS
+ OFFSET(PT_ACC0_GLO, pt_regs, ACC0_GLO);
+ OFFSET(PT_ACC0_GHI, pt_regs, ACC0_GHI);
+ OFFSET(PT_DSP_CTRL, pt_regs, DSP_CTRL);
+#endif
return 0;
}
diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c
index b3995dd673d9..30d31579c51d 100644
--- a/arch/arc/kernel/setup.c
+++ b/arch/arc/kernel/setup.c
@@ -447,7 +447,7 @@ static void arc_chk_core_config(void)
CHK_OPT_STRICT(CONFIG_ARC_HAS_ACCL_REGS, present);
present = dsp_exist();
- CHK_OPT_STRICT(CONFIG_ARC_DSP_KERNEL, present);
+ chk_opt_strict(ARC_DSP_OPT_NAME, present, ARC_DSP_HANDLED);
}
}
--
2.21.0
Replace hardcoded registers offset numbers by calculated via
offsetof.
Signed-off-by: Eugeniy Paltsev <[email protected]>
---
arch/arc/include/asm/entry-arcv2.h | 8 ++++----
arch/arc/kernel/asm-offsets.c | 9 +++++++++
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/arch/arc/include/asm/entry-arcv2.h b/arch/arc/include/asm/entry-arcv2.h
index 41b16f21beec..0b8b63d0bec1 100644
--- a/arch/arc/include/asm/entry-arcv2.h
+++ b/arch/arc/include/asm/entry-arcv2.h
@@ -162,7 +162,7 @@
#endif
#ifdef CONFIG_ARC_HAS_ACCL_REGS
- ST2 r58, r59, PT_sp + 12
+ ST2 r58, r59, PT_r58
#endif
.endm
@@ -172,8 +172,8 @@
LD2 gp, fp, PT_r26 ; gp (r26), fp (r27)
- ld r12, [sp, PT_sp + 4]
- ld r30, [sp, PT_sp + 8]
+ ld r12, [sp, PT_r12]
+ ld r30, [sp, PT_r30]
; Restore SP (into AUX_USER_SP) only if returning to U mode
; - for K mode, it will be implicitly restored as stack is unwound
@@ -190,7 +190,7 @@
#endif
#ifdef CONFIG_ARC_HAS_ACCL_REGS
- LD2 r58, r59, PT_sp + 12
+ LD2 r58, r59, PT_r58
#endif
.endm
diff --git a/arch/arc/kernel/asm-offsets.c b/arch/arc/kernel/asm-offsets.c
index 631ebb5d3458..c783bcd35eb8 100644
--- a/arch/arc/kernel/asm-offsets.c
+++ b/arch/arc/kernel/asm-offsets.c
@@ -67,5 +67,14 @@ int main(void)
DEFINE(SZ_CALLEE_REGS, sizeof(struct callee_regs));
DEFINE(SZ_PT_REGS, sizeof(struct pt_regs));
+#ifdef CONFIG_ISA_ARCV2
+ OFFSET(PT_r12, pt_regs, r12);
+ OFFSET(PT_r30, pt_regs, r30);
+#endif
+#ifdef CONFIG_ARC_HAS_ACCL_REGS
+ OFFSET(PT_r58, pt_regs, r58);
+ OFFSET(PT_r59, pt_regs, r59);
+#endif
+
return 0;
}
--
2.21.0
We'll use this macro in coming patches extensively.
Signed-off-by: Eugeniy Paltsev <[email protected]>
---
arch/arc/kernel/setup.c | 29 ++++++++++++++++-------------
1 file changed, 16 insertions(+), 13 deletions(-)
diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c
index 7ee89dc61f6e..edb55b6ee278 100644
--- a/arch/arc/kernel/setup.c
+++ b/arch/arc/kernel/setup.c
@@ -389,11 +389,23 @@ static char *arc_extn_mumbojumbo(int cpu_id, char *buf, int len)
return buf;
}
+static void chk_opt_strict(char *opt_name, bool hw_exists, bool opt_ena)
+{
+ if (hw_exists && !opt_ena)
+ pr_warn(" ! Enable %s for working apps\n", opt_name);
+ else if (!hw_exists && opt_ena)
+ panic("Disable %s, hardware NOT present\n", opt_name);
+}
+
+#define CHK_OPT_STRICT(opt_name, hw_exists) \
+({ \
+ chk_opt_strict(#opt_name, hw_exists, IS_ENABLED(opt_name)); \
+})
+
static void arc_chk_core_config(void)
{
struct cpuinfo_arc *cpu = &cpuinfo_arc700[smp_processor_id()];
- int saved = 0, present = 0;
- char *opt_nm = NULL;
+ int present = 0;
if (!cpu->extn.timer0)
panic("Timer0 is not present!\n");
@@ -425,23 +437,14 @@ static void arc_chk_core_config(void)
*/
if (is_isa_arcompact()) {
- opt_nm = "CONFIG_ARC_FPU_SAVE_RESTORE";
- saved = IS_ENABLED(CONFIG_ARC_FPU_SAVE_RESTORE);
-
/* only DPDP checked since SP has no arch visible regs */
present = cpu->extn.fpu_dp;
+ CHK_OPT_STRICT(CONFIG_ARC_FPU_SAVE_RESTORE, present);
} else {
- opt_nm = "CONFIG_ARC_HAS_ACCL_REGS";
- saved = IS_ENABLED(CONFIG_ARC_HAS_ACCL_REGS);
-
/* 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);
}
-
- if (present && !saved)
- pr_warn("Enable %s for working apps\n", opt_nm);
- else if (!present && saved)
- panic("Disable %s, hardware NOT present\n", opt_nm);
}
/*
--
2.21.0
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 | 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_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. (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 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 <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();
+ CHK_OPT_STRICT(CONFIG_ARC_DSP_KERNEL, present);
}
}
--
2.21.0
To be able to run DSP-enabled userspace applications with AGU
(address generation unit) extensions we additionally need to
save and restore following registers at context switch:
* AGU_AP*
* AGU_OS*
* AGU_MOD*
Signed-off-by: Eugeniy Paltsev <[email protected]>
---
arch/arc/Kconfig | 7 +++++++
arch/arc/include/asm/arcregs.h | 12 ++++++++++++
arch/arc/include/asm/dsp-impl.h | 23 +++++++++++++++++++++++
arch/arc/include/asm/dsp.h | 12 ++++++++++--
arch/arc/kernel/setup.c | 14 ++++++++++++++
5 files changed, 66 insertions(+), 2 deletions(-)
diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
index c3210754a3d2..c27bb7900ebd 100644
--- a/arch/arc/Kconfig
+++ b/arch/arc/Kconfig
@@ -439,6 +439,13 @@ config ARC_DSP_USERSPACE
help
DSP extension presence in HW, support save / restore DSP registers to
run DSP-enabled userspace applications
+
+config ARC_DSP_AGU_USERSPACE
+ bool "Support DSP with AGU for userspace apps"
+ select ARC_HAS_ACCL_REGS
+ help
+ DSP and AGU extensions presence in HW, support save / restore DSP
+ and AGU registers to run DSP-enabled userspace applications
endchoice
config ARC_IRQ_NO_AUTOSAVE
diff --git a/arch/arc/include/asm/arcregs.h b/arch/arc/include/asm/arcregs.h
index a713819cab3c..9f6abed8a336 100644
--- a/arch/arc/include/asm/arcregs.h
+++ b/arch/arc/include/asm/arcregs.h
@@ -130,6 +130,18 @@
#define ARC_AUX_DSP_CTRL 0x59F
#define ARC_AUX_DSP_FFT_CTRL 0x59E
+#define ARC_AUX_AGU_BUILD 0xCC
+#define ARC_AUX_AGU_AP0 0x5C0
+#define ARC_AUX_AGU_AP1 0x5C1
+#define ARC_AUX_AGU_AP2 0x5C2
+#define ARC_AUX_AGU_AP3 0x5C3
+#define ARC_AUX_AGU_OS0 0x5D0
+#define ARC_AUX_AGU_OS1 0x5D1
+#define ARC_AUX_AGU_MOD0 0x5E0
+#define ARC_AUX_AGU_MOD1 0x5E1
+#define ARC_AUX_AGU_MOD2 0x5E2
+#define ARC_AUX_AGU_MOD3 0x5E3
+
#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
index 7b640a680dfc..d352be2d9f07 100644
--- a/arch/arc/include/asm/dsp-impl.h
+++ b/arch/arc/include/asm/dsp-impl.h
@@ -100,6 +100,21 @@ static inline void arc_dsp_save_restore(struct task_struct *prev,
DSP_AUX_SAVE_RESTORE(saveto, readfrom, DSP_BFLY0, zero);
DSP_AUX_SAVE_RESTORE(saveto, readfrom, DSP_FFT_CTRL, zero);
+
+#ifdef CONFIG_ARC_DSP_AGU_USERSPACE
+ DSP_AUX_SAVE_RESTORE(saveto, readfrom, AGU_AP0, zero);
+ DSP_AUX_SAVE_RESTORE(saveto, readfrom, AGU_AP1, zero);
+ DSP_AUX_SAVE_RESTORE(saveto, readfrom, AGU_AP2, zero);
+ DSP_AUX_SAVE_RESTORE(saveto, readfrom, AGU_AP3, zero);
+
+ DSP_AUX_SAVE_RESTORE(saveto, readfrom, AGU_OS0, zero);
+ DSP_AUX_SAVE_RESTORE(saveto, readfrom, AGU_OS1, zero);
+
+ DSP_AUX_SAVE_RESTORE(saveto, readfrom, AGU_MOD0, zero);
+ DSP_AUX_SAVE_RESTORE(saveto, readfrom, AGU_MOD1, zero);
+ DSP_AUX_SAVE_RESTORE(saveto, readfrom, AGU_MOD2, zero);
+ DSP_AUX_SAVE_RESTORE(saveto, readfrom, AGU_MOD3, zero);
+#endif /* CONFIG_ARC_DSP_AGU_USERSPACE */
}
#else /* !ARC_DSP_SAVE_RESTORE_REGS */
@@ -114,5 +129,13 @@ static inline bool dsp_exist(void)
return !!bcr.ver;
}
+static inline bool agu_exist(void)
+{
+ struct bcr_generic bcr;
+
+ READ_BCR(ARC_AUX_AGU_BUILD, bcr);
+ return !!bcr.ver;
+}
+
#endif /* __ASEMBLY__ */
#endif /* __ASM_ARC_DSP_IMPL_H */
diff --git a/arch/arc/include/asm/dsp.h b/arch/arc/include/asm/dsp.h
index 68507f04dea4..1619cf9c6d28 100644
--- a/arch/arc/include/asm/dsp.h
+++ b/arch/arc/include/asm/dsp.h
@@ -7,7 +7,7 @@
#ifndef __ASM_ARC_DSP_H
#define __ASM_ARC_DSP_H
-#if defined(CONFIG_ARC_DSP_USERSPACE)
+#if defined(CONFIG_ARC_DSP_USERSPACE) || defined(CONFIG_ARC_DSP_AGU_USERSPACE)
#define ARC_DSP_SAVE_RESTORE_REGS 1
#endif
@@ -15,7 +15,8 @@
/* some defines to simplify config sanitize in kernel/setup.c */
#if defined(CONFIG_ARC_DSP_KERNEL) || \
- defined(CONFIG_ARC_DSP_USERSPACE)
+ defined(CONFIG_ARC_DSP_USERSPACE) || \
+ defined(CONFIG_ARC_DSP_AGU_USERSPACE)
#define ARC_DSP_HANDLED 1
#else
#define ARC_DSP_HANDLED 0
@@ -23,6 +24,8 @@
#if defined(CONFIG_ARC_DSP_USERSPACE)
#define ARC_DSP_OPT_NAME "CONFIG_ARC_DSP_USERSPACE"
+#elif defined(CONFIG_ARC_DSP_AGU_USERSPACE)
+#define ARC_DSP_OPT_NAME "CONFIG_ARC_DSP_AGU_USERSPACE"
#else
#define ARC_DSP_OPT_NAME "CONFIG_ARC_DSP_KERNEL"
#endif
@@ -35,6 +38,11 @@
*/
struct dsp_callee_regs {
unsigned long DSP_BFLY0, DSP_FFT_CTRL;
+#ifdef CONFIG_ARC_DSP_AGU_USERSPACE
+ unsigned long AGU_AP0, AGU_AP1, AGU_AP2, AGU_AP3;
+ unsigned long AGU_OS0, AGU_OS1;
+ unsigned long AGU_MOD0, AGU_MOD1, AGU_MOD2, AGU_MOD3;
+#endif
};
#endif /* !__ASSEMBLY__ */
diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c
index 30d31579c51d..17d58ec69113 100644
--- a/arch/arc/kernel/setup.c
+++ b/arch/arc/kernel/setup.c
@@ -398,11 +398,22 @@ static void chk_opt_strict(char *opt_name, bool hw_exists, bool opt_ena)
panic("Disable %s, hardware NOT present\n", opt_name);
}
+static void chk_opt_weak(char *opt_name, bool hw_exists, bool opt_ena)
+{
+ if (!hw_exists && opt_ena)
+ panic("Disable %s, hardware NOT present\n", opt_name);
+}
+
#define CHK_OPT_STRICT(opt_name, hw_exists) \
({ \
chk_opt_strict(#opt_name, hw_exists, IS_ENABLED(opt_name)); \
})
+#define CHK_OPT_WEAK(opt_name, hw_exists) \
+({ \
+ chk_opt_weak(#opt_name, hw_exists, IS_ENABLED(opt_name)); \
+})
+
static void arc_chk_core_config(void)
{
struct cpuinfo_arc *cpu = &cpuinfo_arc700[smp_processor_id()];
@@ -448,6 +459,9 @@ static void arc_chk_core_config(void)
present = dsp_exist();
chk_opt_strict(ARC_DSP_OPT_NAME, present, ARC_DSP_HANDLED);
+
+ present = agu_exist();
+ CHK_OPT_WEAK(CONFIG_ARC_DSP_AGU_USERSPACE, present);
}
}
--
2.21.0
On 12/27/19 10:03 AM, Eugeniy Paltsev wrote:
> Replace hardcoded registers offset numbers by calculated via
> offsetof.
>
> Signed-off-by: Eugeniy Paltsev <[email protected]>
LGTM. This seems like an independent cleanup so added to for-curr
Thx,
-Vineet
On 12/27/19 10:03 AM, Eugeniy Paltsev wrote:
> We'll use this macro in coming patches extensively.
>
> Signed-off-by: Eugeniy Paltsev <[email protected]>
LGTM.
-Vineet
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 <[email protected]>
> ---
> 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. (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 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 !
On 12/27/19 10:03 AM, Eugeniy Paltsev wrote:
> To be able to run DSP-enabled userspace applications we need to
> save and restore following DSP-related registers:
> At IRQ/exception entry/exit:
> * ACC0_GLO, ACC0_GHI, DSP_CTRL
> * ACC0_LO, ACC0_HI (we already save them as r58, r59 pair)
> At context switch:
> * DSP_BFLY0, DSP_FFT_CTRL
Good summary: but the question is this more than needed.
For kernel proper, you've disabled guard bits, saturation etc already. AFAIKS gcc
won't generate complex/fractional math for kernel so your bullet #1 can likely be
considered user space only hence can go to bullet #3. Do you have reasons to
believe otherwise ?
> Signed-off-by: Eugeniy Paltsev <[email protected]>
> ---
> arch/arc/Kconfig | 7 +++
> arch/arc/include/asm/arcregs.h | 2 +
> arch/arc/include/asm/dsp-impl.h | 75 +++++++++++++++++++++++++++++-
> arch/arc/include/asm/dsp.h | 42 +++++++++++++++++
> arch/arc/include/asm/entry-arcv2.h | 3 ++
> arch/arc/include/asm/processor.h | 4 ++
> arch/arc/include/asm/ptrace.h | 4 ++
> arch/arc/include/asm/switch_to.h | 2 +
> arch/arc/kernel/asm-offsets.c | 7 +++
> arch/arc/kernel/setup.c | 2 +-
> 10 files changed, 146 insertions(+), 2 deletions(-)
> create mode 100644 arch/arc/include/asm/dsp.h
>
> diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
> index b9cd7ce3f878..c3210754a3d2 100644
> --- a/arch/arc/Kconfig
> +++ b/arch/arc/Kconfig
> @@ -432,6 +432,13 @@ config ARC_DSP_KERNEL
> 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
> +
> +config ARC_DSP_USERSPACE
> + bool "Support DSP for userspace apps"
> + select ARC_HAS_ACCL_REGS
> + help
> + DSP extension presence in HW, support save / restore DSP registers to
> + run DSP-enabled userspace applications
> endchoice
>
> config ARC_IRQ_NO_AUTOSAVE
> diff --git a/arch/arc/include/asm/arcregs.h b/arch/arc/include/asm/arcregs.h
> index 0004b1e9b325..a713819cab3c 100644
> --- a/arch/arc/include/asm/arcregs.h
> +++ b/arch/arc/include/asm/arcregs.h
> @@ -118,6 +118,8 @@
>
> /*
> * DSP-related registers
> + * Registers names must correspond to dsp_callee_regs structure fields names
> + * for automatic offset calculation in DSP_AUX_SAVE_RESTORE macros.
good idea for preventing carbon errors.
> */
> #define ARC_AUX_DSP_BUILD 0x7A
> #define ARC_AUX_ACC0_LO 0x580
> diff --git a/arch/arc/include/asm/dsp-impl.h b/arch/arc/include/asm/dsp-impl.h
> index 788093cbe689..7b640a680dfc 100644
> --- a/arch/arc/include/asm/dsp-impl.h
> +++ b/arch/arc/include/asm/dsp-impl.h
> @@ -7,6 +7,8 @@
> #ifndef __ASM_ARC_DSP_IMPL_H
> #define __ASM_ARC_DSP_IMPL_H
>
> +#include <asm/dsp.h>
> +
> #define DSP_CTRL_DISABLED_ALL 0
>
> #ifdef __ASSEMBLY__
> @@ -28,11 +30,82 @@
> * able to break kernel */
> mov r58, DSP_CTRL_DISABLED_ALL
> sr r58, [ARC_AUX_DSP_CTRL]
> -#endif /* ARC_DSP_KERNEL */
> +
> +#elif defined(ARC_DSP_SAVE_RESTORE_REGS)
> + lr r58, [ARC_AUX_ACC0_GLO]
> + lr r59, [ARC_AUX_ACC0_GHI]
> + ST2 r58, r59, PT_ACC0_GLO
> +
> + lr r58, [ARC_AUX_DSP_CTRL]
> + st r58, [sp, PT_DSP_CTRL]
> +
> +#endif
> +.endm
> +
> +/* clobbers r58, r59 registers pair */
> +.macro DSP_RESTORE_REGFILE_IRQ
> +#if defined(ARC_DSP_SAVE_RESTORE_REGS)
> + LD2 r58, r59, PT_ACC0_GLO
> + sr r58, [ARC_AUX_ACC0_GLO]
> + sr r59, [ARC_AUX_ACC0_GHI]
> +
> + ld r58, [sp, PT_DSP_CTRL]
> + sr r58, [ARC_AUX_DSP_CTRL]
> +
> +#endif
Assuming you still need this, consider using different scratch registers if
possible. I understand it gets tricky in restore path but there even more
registers are available to clobber as they will be restored after this code.
> +#ifdef ARC_DSP_SAVE_RESTORE_REGS
> +
> +/*
> + * As we save new and restore old AUX register value in the same place we
> + * can optimize a bit and use AEX instruction (swap contents of an auxiliary
> + * register with a core register) instead of LR + SR pair.
> + */
> +#define AUX_SAVE_RESTORE(_saveto, _readfrom, _offt, _aux, _scratch) \
> +do { \
> + __asm__ __volatile__( \
> + "ld %0, [%2, %4] \n" \
> + "aex %0, [%3] \n" \
> + "st %0, [%1, %4] \n" \
> + : \
> + "=&r" (_scratch) /* must be early clobber */ \
Define the scratch variable locally for clarity and better liveness tracking - for
both code reader and compiler :-)
Also avoids having to initialize it.
> + : \
> + "r" (_saveto), \
> + "r" (_readfrom), \
> + "I" (_aux), \
> + "I" (_offt) \
> + : \
AEX with "I" constraint will likely be an 8 byte instructions always. Best to give
compiler wiggle room with "Ir"
> + "memory" \
> + ); \
> +} while (0)
> +
> +#define DSP_AUX_SAVE_RESTORE(_saveto, _readfrom, _aux, _scratch) \
> + AUX_SAVE_RESTORE(_saveto, _readfrom, \
> + offsetof(struct dsp_callee_regs, _aux), \
> + ARC_AUX_##_aux, _scratch)
> +
> +static inline void arc_dsp_save_restore(struct task_struct *prev,
> + struct task_struct *next)
> +{
> + long unsigned int *saveto = &prev->thread.dsp.DSP_BFLY0;
> + long unsigned int *readfrom = &next->thread.dsp.DSP_BFLY0;
> +
> + long unsigned int zero = 0;
See comment about pushing scratch to the relevant code block.
> +
> + DSP_AUX_SAVE_RESTORE(saveto, readfrom, DSP_BFLY0, zero);
> + DSP_AUX_SAVE_RESTORE(saveto, readfrom, DSP_FFT_CTRL, zero);
> +}
> +
[snip]
> +
> +#if defined(CONFIG_ARC_DSP_USERSPACE)
> +#define ARC_DSP_SAVE_RESTORE_REGS 1
> +#endif
I can see u added this for consistency with below which is a really bad idea: see
below.
> +
> +#ifndef __ASSEMBLY__
> +
> +/* some defines to simplify config sanitize in kernel/setup.c */
> +#if defined(CONFIG_ARC_DSP_KERNEL) || \
> + defined(CONFIG_ARC_DSP_USERSPACE)
> +#define ARC_DSP_HANDLED 1
> +#else
> +#define ARC_DSP_HANDLED 0
> +#endif
This is a really bad idea - u r introducing explicit include dependencies which
can change even outside of arch changes !
We've dealt with enough of these problems with current.h, so best to avoid, even
if there is some code clutter.
> +
> +#if defined(CONFIG_ARC_DSP_USERSPACE)
> +#define ARC_DSP_OPT_NAME "CONFIG_ARC_DSP_USERSPACE"
> +#else
> +#define ARC_DSP_OPT_NAME "CONFIG_ARC_DSP_KERNEL"
> +#endif
> +
> +/*
> + * DSP-related saved registers - need to be saved only when you are
> + * scheduled out.
> + * structure fields name must correspond to aux register defenitions for
> + * automatic offset calculation in DSP_AUX_SAVE_RESTORE macros
> + */
> +struct dsp_callee_regs {
> + unsigned long DSP_BFLY0, DSP_FFT_CTRL;
> +};
> +
> +#endif /* !__ASSEMBLY__ */
> +
> +#endif /* __ASM_ARC_DSP_H */
> diff --git a/arch/arc/include/asm/entry-arcv2.h b/arch/arc/include/asm/entry-arcv2.h
> index e3f8bd3e2eba..5d079f0b6243 100644
> --- a/arch/arc/include/asm/entry-arcv2.h
> +++ b/arch/arc/include/asm/entry-arcv2.h
> @@ -192,6 +192,9 @@
> ld r25, [sp, PT_user_r25]
> #endif
>
> + /* clobbers r58, r59 registers pair, so must be before r58, r59 restore */
> + DSP_RESTORE_REGFILE_IRQ
> +
> #ifdef CONFIG_ARC_HAS_ACCL_REGS
> LD2 r58, r59, PT_r58
> #endif
> diff --git a/arch/arc/include/asm/processor.h b/arch/arc/include/asm/processor.h
> index 706edeaa5583..1716df0f4472 100644
> --- a/arch/arc/include/asm/processor.h
> +++ b/arch/arc/include/asm/processor.h
> @@ -14,6 +14,7 @@
> #ifndef __ASSEMBLY__
>
> #include <asm/ptrace.h>
> +#include <asm/dsp.h>
>
> #ifdef CONFIG_ARC_FPU_SAVE_RESTORE
> /* These DPFP regs need to be saved/restored across ctx-sw */
> @@ -39,6 +40,9 @@ struct thread_struct {
> unsigned long ksp; /* kernel mode stack pointer */
> unsigned long callee_reg; /* pointer to callee regs */
> unsigned long fault_address; /* dbls as brkpt holder as well */
> +#ifdef ARC_DSP_SAVE_RESTORE_REGS
> + struct dsp_callee_regs dsp;
> +#endif
> #ifdef CONFIG_ARC_FPU_SAVE_RESTORE
> struct arc_fpu fpu;
> #endif
> diff --git a/arch/arc/include/asm/ptrace.h b/arch/arc/include/asm/ptrace.h
> index ba9854ef39e8..a5851ee141de 100644
> --- a/arch/arc/include/asm/ptrace.h
> +++ b/arch/arc/include/asm/ptrace.h
> @@ -8,6 +8,7 @@
> #define __ASM_ARC_PTRACE_H
>
> #include <uapi/asm/ptrace.h>
> +#include <asm/dsp.h>
>
> #ifndef __ASSEMBLY__
>
> @@ -91,6 +92,9 @@ struct pt_regs {
> #ifdef CONFIG_ARC_HAS_ACCL_REGS
> unsigned long r58, r59; /* ACCL/ACCH used by FPU / DSP MPY */
> #endif
> +#ifdef ARC_DSP_SAVE_RESTORE_REGS
Use the Kconfig option name directly or !CONFIG_NO_DSP etc
> + unsigned long ACC0_GLO, ACC0_GHI, DSP_CTRL;
> +#endif
see comments at top.
>
> /*------- Below list auto saved by h/w -----------*/
> unsigned long r0, r1, r2, r3, r4, r5, r6, r7, r8, r9, r10, r11;
[snip]
> }
> diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c
> index b3995dd673d9..30d31579c51d 100644
> --- a/arch/arc/kernel/setup.c
> +++ b/arch/arc/kernel/setup.c
> @@ -447,7 +447,7 @@ static void arc_chk_core_config(void)
> CHK_OPT_STRICT(CONFIG_ARC_HAS_ACCL_REGS, present);
>
> present = dsp_exist();
> - CHK_OPT_STRICT(CONFIG_ARC_DSP_KERNEL, present);
> + chk_opt_strict(ARC_DSP_OPT_NAME, present, ARC_DSP_HANDLED);
This needs to be reworked given the header fudge is not a good idea.
On 12/27/19 10:03 AM, Eugeniy Paltsev wrote:
> To be able to run DSP-enabled userspace applications with AGU
> (address generation unit) extensions we additionally need to
> save and restore following registers at context switch:
> * AGU_AP*
> * AGU_OS*
> * AGU_MOD*
>
> Signed-off-by: Eugeniy Paltsev <[email protected]>
Similar comments apply to this as 4/5. Looks good otherwise !
-Vineet
Hi Vineet,
>From: Vineet Gupta <[email protected]>
>On 12/27/19 10:03 AM, Eugeniy Paltsev wrote:
>> To be able to run DSP-enabled userspace applications we need to
>> save and restore following DSP-related registers:
>> At IRQ/exception entry/exit:
>> * ACC0_GLO, ACC0_GHI, DSP_CTRL
>> * ACC0_LO, ACC0_HI (we already save them as r58, r59 pair)
>> At context switch:
>> * DSP_BFLY0, DSP_FFT_CTRL
[snip]
>> +
>> +#ifndef __ASSEMBLY__
>> +
>> +/* some defines to simplify config sanitize in kernel/setup.c */
>> +#if defined(CONFIG_ARC_DSP_KERNEL) || \
>> + defined(CONFIG_ARC_DSP_USERSPACE)
>> +#define ARC_DSP_HANDLED 1
>> +#else
>> +#define ARC_DSP_HANDLED 0
>> +#endif
>
>This is a really bad idea - u r introducing explicit include dependencies which
>can change even outside of arch changes !
>We've dealt with enough of these problems with current.h, so best to avoid, even
>if there is some code clutter.
Hmm, would it be OK if I add this option as a private kconfig option?
I.E (for ARC_DSP_HANDLED):
---------------->8----------------------
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
config ARC_DSP_USERSPACE
bool "Support DSP for userspace apps"
select ARC_HAS_ACCL_REGS
select ARC_DSP_HANDLED
help
DSP extension presence in HW, support save / restore DSP registers to
run DSP-enabled userspace applications
endchoice
---------------->8----------------------
---
Eugeniy Paltsev
On 1/9/20 11:01 AM, Eugeniy Paltsev wrote:
> Hi Vineet,
>
>> From: Vineet Gupta <[email protected]>
>> On 12/27/19 10:03 AM, Eugeniy Paltsev wrote:
>>> To be able to run DSP-enabled userspace applications we need to
>>> save and restore following DSP-related registers:
>>> At IRQ/exception entry/exit:
>>> * ACC0_GLO, ACC0_GHI, DSP_CTRL
>>> * ACC0_LO, ACC0_HI (we already save them as r58, r59 pair)
>>> At context switch:
>>> * DSP_BFLY0, DSP_FFT_CTRL
> [snip]
>>> +
>>> +#ifndef __ASSEMBLY__
>>> +
>>> +/* some defines to simplify config sanitize in kernel/setup.c */
>>> +#if defined(CONFIG_ARC_DSP_KERNEL) || \
>>> + defined(CONFIG_ARC_DSP_USERSPACE)
>>> +#define ARC_DSP_HANDLED 1
>>> +#else
>>> +#define ARC_DSP_HANDLED 0
>>> +#endif
>> This is a really bad idea - u r introducing explicit include dependencies which
>> can change even outside of arch changes !
>> We've dealt with enough of these problems with current.h, so best to avoid, even
>> if there is some code clutter.
> Hmm, would it be OK if I add this option as a private kconfig option?
> I.E (for ARC_DSP_HANDLED):
Yes that would be good.
-Vineet
Hi Vineet,
>From: Vineet Gupta <[email protected]>
>Sent: Tuesday, January 7, 2020 04:03
>To: Eugeniy Paltsev; [email protected]
>Cc: [email protected]; 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
Hi Vineet,
>From: Vineet Gupta <[email protected]>
>Sent: Tuesday, January 7, 2020 21:25
>To: Eugeniy Paltsev; [email protected]
>Cc: [email protected]; Alexey Brodkin
>Subject: Re: [PATCH 4/5] ARC: add support for DSP-enabled userspace applications
>> +/*
>> + * As we save new and restore old AUX register value in the same place we
>> + * can optimize a bit and use AEX instruction (swap contents of an auxiliary
>> + * register with a core register) instead of LR + SR pair.
>> + */
>> +#define AUX_SAVE_RESTORE(_saveto, _readfrom, _offt, _aux, _scratch) \
>> +do { \
>> + __asm__ __volatile__( \
>> + "ld %0, [%2, %4] \n" \
>> + "aex %0, [%3] \n" \
>> + "st %0, [%1, %4] \n" \
>> + : \
>> + "=&r" (_scratch) /* must be early clobber */ \
>> + : \
>> + "r" (_saveto), \
>> + "r" (_readfrom), \
>> + "I" (_aux), \
>> + "I" (_offt) \
>> + : \
>
>AEX with "I" constraint will likely be an 8 byte instructions always. Best to give
>compiler wiggle room with "Ir"
Could you please explain how "Ir" will work in this case?
Does this mean that compiler can pass the value either as constant ('I') or
via register ('r')?
Note that in this case both _aux and _offt are compile-time constants -
_aux comes from define and _offt comes from offsetof().
>> + "memory" \
>> + ); \
>> +} while (0)
>> +
On 3/4/20 3:51 AM, Eugeniy Paltsev wrote:
>>> + "r" (_saveto), \
>>> + "r" (_readfrom), \
>>> + "I" (_aux), \
>>> + "I" (_offt) \
>>> + : \
>>
>> AEX with "I" constraint will likely be an 8 byte instructions always. Best to give
>> compiler wiggle room with "Ir"
>
> Could you please explain how "Ir" will work in this case?
> Does this mean that compiler can pass the value either as constant ('I') or
> via register ('r')?
Right. If the same compile-time const is used multiple times, say SR of same value
to a reg, if can decide to hoist the value in a register, if you provide it "r" or
"Ir", leading to shorter overall code.
-Vineet