[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6b80df9d-d0f2-d1e1-8e4b-b65531b938d9@synopsys.com>
Date: Tue, 7 Jan 2020 01:03:10 +0000
From: Vineet Gupta <Vineet.Gupta1@...opsys.com>
To: Eugeniy Paltsev <Eugeniy.Paltsev@...opsys.com>,
"linux-snps-arc@...ts.infradead.org"
<linux-snps-arc@...ts.infradead.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"Alexey Brodkin" <Alexey.Brodkin@...opsys.com>
Subject: Re: [PATCH 3/5] ARC: handle DSP presence in HW
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@...opsys.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@...opsys.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);
> }
> }
>
Powered by blists - more mailing lists