linux-snps-arc.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [RFC 0/5] ARC: handle DSP presence in HW
@ 2019-12-27 18:03 Eugeniy Paltsev
  2019-12-27 18:03 ` [PATCH 1/5] ARC: pt_regs: remove hardcoded registers offset Eugeniy Paltsev
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Eugeniy Paltsev @ 2019-12-27 18:03 UTC (permalink / raw)
  To: linux-snps-arc, Vineet Gupta
  Cc: Alexey Brodkin, Eugeniy Paltsev, linux-kernel

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


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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/5] ARC: pt_regs: remove hardcoded registers offset
  2019-12-27 18:03 [RFC 0/5] ARC: handle DSP presence in HW Eugeniy Paltsev
@ 2019-12-27 18:03 ` Eugeniy Paltsev
  2019-12-28 21:06   ` Vineet Gupta
  2019-12-27 18:03 ` [PATCH 2/5] ARC: add helpers to sanitize config options Eugeniy Paltsev
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Eugeniy Paltsev @ 2019-12-27 18:03 UTC (permalink / raw)
  To: linux-snps-arc, Vineet Gupta
  Cc: Alexey Brodkin, Eugeniy Paltsev, linux-kernel

Replace hardcoded registers offset numbers by calculated via
offsetof.

Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
---
 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


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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/5] ARC: add helpers to sanitize config options
  2019-12-27 18:03 [RFC 0/5] ARC: handle DSP presence in HW Eugeniy Paltsev
  2019-12-27 18:03 ` [PATCH 1/5] ARC: pt_regs: remove hardcoded registers offset Eugeniy Paltsev
@ 2019-12-27 18:03 ` Eugeniy Paltsev
  2020-01-06 22:47   ` Vineet Gupta
  2019-12-27 18:03 ` [PATCH 3/5] ARC: handle DSP presence in HW Eugeniy Paltsev
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Eugeniy Paltsev @ 2019-12-27 18:03 UTC (permalink / raw)
  To: linux-snps-arc, Vineet Gupta
  Cc: Alexey Brodkin, Eugeniy Paltsev, linux-kernel

We'll use this macro in coming patches extensively.

Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
---
 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


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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/5] ARC: handle DSP presence in HW
  2019-12-27 18:03 [RFC 0/5] ARC: handle DSP presence in HW Eugeniy Paltsev
  2019-12-27 18:03 ` [PATCH 1/5] ARC: pt_regs: remove hardcoded registers offset Eugeniy Paltsev
  2019-12-27 18:03 ` [PATCH 2/5] ARC: add helpers to sanitize config options Eugeniy Paltsev
@ 2019-12-27 18:03 ` Eugeniy Paltsev
  2020-01-07  1:03   ` Vineet Gupta
  2019-12-27 18:03 ` [PATCH 4/5] ARC: add support for DSP-enabled userspace applications Eugeniy Paltsev
  2019-12-27 18:03 ` [PATCH 5/5] ARC: allow userspace DSP applications to use AGU extensions Eugeniy Paltsev
  4 siblings, 1 reply; 18+ messages in thread
From: Eugeniy Paltsev @ 2019-12-27 18:03 UTC (permalink / raw)
  To: linux-snps-arc, Vineet Gupta
  Cc: Alexey Brodkin, Eugeniy Paltsev, linux-kernel

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
+	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]
+#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


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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/5] ARC: add support for DSP-enabled userspace applications
  2019-12-27 18:03 [RFC 0/5] ARC: handle DSP presence in HW Eugeniy Paltsev
                   ` (2 preceding siblings ...)
  2019-12-27 18:03 ` [PATCH 3/5] ARC: handle DSP presence in HW Eugeniy Paltsev
@ 2019-12-27 18:03 ` Eugeniy Paltsev
  2020-01-07 18:25   ` Vineet Gupta
  2019-12-27 18:03 ` [PATCH 5/5] ARC: allow userspace DSP applications to use AGU extensions Eugeniy Paltsev
  4 siblings, 1 reply; 18+ messages in thread
From: Eugeniy Paltsev @ 2019-12-27 18:03 UTC (permalink / raw)
  To: linux-snps-arc, Vineet Gupta
  Cc: Alexey Brodkin, Eugeniy Paltsev, linux-kernel

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 <Eugeniy.Paltsev@synopsys.com>
---
 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. (www.synopsys.com)
+ *
+ * Author: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>
+ */
+#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


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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 5/5] ARC: allow userspace DSP applications to use AGU extensions
  2019-12-27 18:03 [RFC 0/5] ARC: handle DSP presence in HW Eugeniy Paltsev
                   ` (3 preceding siblings ...)
  2019-12-27 18:03 ` [PATCH 4/5] ARC: add support for DSP-enabled userspace applications Eugeniy Paltsev
@ 2019-12-27 18:03 ` Eugeniy Paltsev
  2020-01-07 18:30   ` Vineet Gupta
  4 siblings, 1 reply; 18+ messages in thread
From: Eugeniy Paltsev @ 2019-12-27 18:03 UTC (permalink / raw)
  To: linux-snps-arc, Vineet Gupta
  Cc: Alexey Brodkin, Eugeniy Paltsev, linux-kernel

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 <Eugeniy.Paltsev@synopsys.com>
---
 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


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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/5] ARC: pt_regs: remove hardcoded registers offset
  2019-12-27 18:03 ` [PATCH 1/5] ARC: pt_regs: remove hardcoded registers offset Eugeniy Paltsev
@ 2019-12-28 21:06   ` Vineet Gupta
  0 siblings, 0 replies; 18+ messages in thread
From: Vineet Gupta @ 2019-12-28 21:06 UTC (permalink / raw)
  To: Eugeniy Paltsev, linux-snps-arc; +Cc: Alexey Brodkin, linux-kernel

On 12/27/19 10:03 AM, Eugeniy Paltsev wrote:
> Replace hardcoded registers offset numbers by calculated via
> offsetof.
>
> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>

LGTM. This seems like an independent cleanup so added to for-curr

Thx,
-Vineet


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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/5] ARC: add helpers to sanitize config options
  2019-12-27 18:03 ` [PATCH 2/5] ARC: add helpers to sanitize config options Eugeniy Paltsev
@ 2020-01-06 22:47   ` Vineet Gupta
  0 siblings, 0 replies; 18+ messages in thread
From: Vineet Gupta @ 2020-01-06 22:47 UTC (permalink / raw)
  To: Eugeniy Paltsev, linux-snps-arc; +Cc: Alexey Brodkin, linux-kernel

On 12/27/19 10:03 AM, Eugeniy Paltsev wrote:
> We'll use this macro in coming patches extensively.
> 
> Signed-off-by: Eugeniy Paltsev <Eugeniy.Paltsev@synopsys.com>

LGTM.

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/5] ARC: handle DSP presence in HW
  2019-12-27 18:03 ` [PATCH 3/5] ARC: handle DSP presence in HW Eugeniy Paltsev
@ 2020-01-07  1:03   ` Vineet Gupta
  2020-01-07  1:30     ` Vineet Gupta
  2020-01-10 14:54     ` Eugeniy Paltsev
  0 siblings, 2 replies; 18+ messages in thread
From: Vineet Gupta @ 2020-01-07  1:03 UTC (permalink / raw)
  To: Eugeniy Paltsev, linux-snps-arc; +Cc: Alexey Brodkin, linux-kernel

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);
>  	}
>  }
>  

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/5] ARC: handle DSP presence in HW
  2020-01-07  1:03   ` Vineet Gupta
@ 2020-01-07  1:30     ` Vineet Gupta
  2020-01-10 14:54     ` Eugeniy Paltsev
  1 sibling, 0 replies; 18+ messages in thread
From: Vineet Gupta @ 2020-01-07  1:30 UTC (permalink / raw)
  To: Vineet Gupta, Eugeniy Paltsev, linux-snps-arc
  Cc: Alexey Brodkin, linux-kernel

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/5] ARC: add support for DSP-enabled userspace applications
  2019-12-27 18:03 ` [PATCH 4/5] ARC: add support for DSP-enabled userspace applications Eugeniy Paltsev
@ 2020-01-07 18:25   ` Vineet Gupta
  2020-01-09 19:01     ` Eugeniy Paltsev
  2020-03-04 11:51     ` Eugeniy Paltsev
  0 siblings, 2 replies; 18+ messages in thread
From: Vineet Gupta @ 2020-01-07 18:25 UTC (permalink / raw)
  To: Eugeniy Paltsev, linux-snps-arc; +Cc: Alexey Brodkin, linux-kernel

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 <Eugeniy.Paltsev@synopsys.com>
> ---
>  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.

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/5] ARC: allow userspace DSP applications to use AGU extensions
  2019-12-27 18:03 ` [PATCH 5/5] ARC: allow userspace DSP applications to use AGU extensions Eugeniy Paltsev
@ 2020-01-07 18:30   ` Vineet Gupta
  0 siblings, 0 replies; 18+ messages in thread
From: Vineet Gupta @ 2020-01-07 18:30 UTC (permalink / raw)
  To: Eugeniy Paltsev, linux-snps-arc; +Cc: Alexey Brodkin, linux-kernel

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 <Eugeniy.Paltsev@synopsys.com>

Similar comments apply to this as 4/5. Looks good otherwise !

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/5] ARC: add support for DSP-enabled userspace applications
  2020-01-07 18:25   ` Vineet Gupta
@ 2020-01-09 19:01     ` Eugeniy Paltsev
  2020-01-09 22:46       ` Vineet Gupta
  2020-03-04 11:51     ` Eugeniy Paltsev
  1 sibling, 1 reply; 18+ messages in thread
From: Eugeniy Paltsev @ 2020-01-09 19:01 UTC (permalink / raw)
  To: Vineet Gupta; +Cc: linux-snps-arc, Alexey Brodkin, linux-kernel

Hi Vineet,

>From: Vineet Gupta <vgupta@synopsys.com>
>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
_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/5] ARC: add support for DSP-enabled userspace applications
  2020-01-09 19:01     ` Eugeniy Paltsev
@ 2020-01-09 22:46       ` Vineet Gupta
  0 siblings, 0 replies; 18+ messages in thread
From: Vineet Gupta @ 2020-01-09 22:46 UTC (permalink / raw)
  To: Eugeniy Paltsev; +Cc: arcml, Alexey Brodkin, lkml

On 1/9/20 11:01 AM, Eugeniy Paltsev wrote:
> Hi Vineet,
>
>> From: Vineet Gupta <vgupta@synopsys.com>
>> 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

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/5] ARC: handle DSP presence in HW
  2020-01-07  1:03   ` Vineet Gupta
  2020-01-07  1:30     ` Vineet Gupta
@ 2020-01-10 14:54     ` Eugeniy Paltsev
  2020-01-13 16:49       ` Vineet Gupta
  1 sibling, 1 reply; 18+ messages in thread
From: Eugeniy Paltsev @ 2020-01-10 14:54 UTC (permalink / raw)
  To: Vineet Gupta, linux-snps-arc; +Cc: Alexey Brodkin, linux-kernel

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

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/5] ARC: handle DSP presence in HW
  2020-01-10 14:54     ` Eugeniy Paltsev
@ 2020-01-13 16:49       ` Vineet Gupta
  0 siblings, 0 replies; 18+ messages in thread
From: Vineet Gupta @ 2020-01-13 16:49 UTC (permalink / raw)
  To: Eugeniy Paltsev, linux-snps-arc; +Cc: Alexey Brodkin, linux-kernel

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/5] ARC: add support for DSP-enabled userspace applications
  2020-01-07 18:25   ` Vineet Gupta
  2020-01-09 19:01     ` Eugeniy Paltsev
@ 2020-03-04 11:51     ` Eugeniy Paltsev
  2020-03-04 17:58       ` Vineet Gupta
  1 sibling, 1 reply; 18+ messages in thread
From: Eugeniy Paltsev @ 2020-03-04 11:51 UTC (permalink / raw)
  To: Vineet Gupta, linux-snps-arc; +Cc: Alexey Brodkin, linux-kernel

Hi Vineet,

>From: Vineet Gupta <vgupta@synopsys.com>
>Sent: Tuesday, January 7, 2020 21:25
>To: Eugeniy Paltsev; linux-snps-arc@lists.infradead.org
>Cc: linux-kernel@vger.kernel.org; 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)
>> +
_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/5] ARC: add support for DSP-enabled userspace applications
  2020-03-04 11:51     ` Eugeniy Paltsev
@ 2020-03-04 17:58       ` Vineet Gupta
  0 siblings, 0 replies; 18+ messages in thread
From: Vineet Gupta @ 2020-03-04 17:58 UTC (permalink / raw)
  To: Eugeniy Paltsev, linux-snps-arc; +Cc: Alexey Brodkin, linux-kernel

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


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

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2020-03-04 17:58 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-27 18:03 [RFC 0/5] ARC: handle DSP presence in HW Eugeniy Paltsev
2019-12-27 18:03 ` [PATCH 1/5] ARC: pt_regs: remove hardcoded registers offset Eugeniy Paltsev
2019-12-28 21:06   ` Vineet Gupta
2019-12-27 18:03 ` [PATCH 2/5] ARC: add helpers to sanitize config options Eugeniy Paltsev
2020-01-06 22:47   ` Vineet Gupta
2019-12-27 18:03 ` [PATCH 3/5] ARC: handle DSP presence in HW Eugeniy Paltsev
2020-01-07  1:03   ` Vineet Gupta
2020-01-07  1:30     ` Vineet Gupta
2020-01-10 14:54     ` Eugeniy Paltsev
2020-01-13 16:49       ` Vineet Gupta
2019-12-27 18:03 ` [PATCH 4/5] ARC: add support for DSP-enabled userspace applications Eugeniy Paltsev
2020-01-07 18:25   ` Vineet Gupta
2020-01-09 19:01     ` Eugeniy Paltsev
2020-01-09 22:46       ` Vineet Gupta
2020-03-04 11:51     ` Eugeniy Paltsev
2020-03-04 17:58       ` Vineet Gupta
2019-12-27 18:03 ` [PATCH 5/5] ARC: allow userspace DSP applications to use AGU extensions Eugeniy Paltsev
2020-01-07 18:30   ` Vineet Gupta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).