[PATCH v4 0/5] riscv: Add support to no-FPU systems

[PATCH v4 0/5] riscv: Add support to no-FPU systems

[Alan Kao]

This patchset adds an option, CONFIG_FPU, to enable/disable floating-
point procedures.

Kernel's new behavior will be as follows:

* with CONFIG_FPU=y
  All FPU codes are reserved.  If no FPU is found during booting, a
  global flag will be set, and those functions will be bypassed with
  condition check to that flag.

* with CONFIG_FPU=n
  No floating-point instructions in kernel and all related settings
  are excluded.

Changes in v4:
 - Append a new patch to detect existence of FPU and followups.
 - Add SPDX header to newly created fpu.S.
 - Fix a build error, sorry for that.
 - Fix wording, style, etc.

Changes in v3:
 - Refactor the whole patch into independent ones.

Changes in v2:
 - Various code cleanups and style fixes.

Alan Kao (5):
  Extract FPU context operations from entry.S
  Refactor FPU code in signal setup/return procedures
  Cleanup ISA string setting
  Allow to disable FPU support
  Auto-detect whether a FPU exists

 arch/riscv/Kconfig                 |   9 +++
 arch/riscv/Makefile                |  19 +++---
 arch/riscv/include/asm/hwcap.h     |   3 +
 arch/riscv/include/asm/switch_to.h |  21 ++++++
 arch/riscv/kernel/Makefile         |   1 +
 arch/riscv/kernel/cpufeature.c     |  11 +++
 arch/riscv/kernel/entry.S          |  87 -----------------------
 arch/riscv/kernel/fpu.S            | 106 +++++++++++++++++++++++++++++
 arch/riscv/kernel/process.c        |   4 +-
 arch/riscv/kernel/signal.c         |  79 +++++++++++++--------
 10 files changed, 214 insertions(+), 126 deletions(-)
 create mode 100644 arch/riscv/kernel/fpu.S

-- 
2.18.0

[PATCH v4 1/5] Extract FPU context operations from entry.S

[Alan Kao]

We move __fstate_save and __fstate_restore to a new source
file, fpu.S.

Signed-off-by: Alan Kao <alankao@andestech.com>
Cc: Greentime Hu <greentime@andestech.com>
Cc: Vincent Chen <vincentc@andestech.com>
Cc: Zong Li <zong@andestech.com>
Cc: Nick Hu <nickhu@andestech.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 arch/riscv/kernel/Makefile |   1 +
 arch/riscv/kernel/entry.S  |  87 ------------------------------
 arch/riscv/kernel/fpu.S    | 106 +++++++++++++++++++++++++++++++++++++
 3 files changed, 107 insertions(+), 87 deletions(-)
 create mode 100644 arch/riscv/kernel/fpu.S

diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index e1274fc03af4..bd433efd915e 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -13,6 +13,7 @@ extra-y += vmlinux.lds
 obj-y	+= cpu.o
 obj-y	+= cpufeature.o
 obj-y	+= entry.o
+obj-y	+= fpu.o
 obj-y	+= irq.o
 obj-y	+= process.o
 obj-y	+= ptrace.o
diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index 9aaf6c986771..edcd5920ee4e 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -357,93 +357,6 @@ ENTRY(__switch_to)
 	ret
 ENDPROC(__switch_to)
 
-ENTRY(__fstate_save)
-	li  a2,  TASK_THREAD_F0
-	add a0, a0, a2
-	li t1, SR_FS
-	csrs sstatus, t1
-	frcsr t0
-	fsd f0,  TASK_THREAD_F0_F0(a0)
-	fsd f1,  TASK_THREAD_F1_F0(a0)
-	fsd f2,  TASK_THREAD_F2_F0(a0)
-	fsd f3,  TASK_THREAD_F3_F0(a0)
-	fsd f4,  TASK_THREAD_F4_F0(a0)
-	fsd f5,  TASK_THREAD_F5_F0(a0)
-	fsd f6,  TASK_THREAD_F6_F0(a0)
-	fsd f7,  TASK_THREAD_F7_F0(a0)
-	fsd f8,  TASK_THREAD_F8_F0(a0)
-	fsd f9,  TASK_THREAD_F9_F0(a0)
-	fsd f10, TASK_THREAD_F10_F0(a0)
-	fsd f11, TASK_THREAD_F11_F0(a0)
-	fsd f12, TASK_THREAD_F12_F0(a0)
-	fsd f13, TASK_THREAD_F13_F0(a0)
-	fsd f14, TASK_THREAD_F14_F0(a0)
-	fsd f15, TASK_THREAD_F15_F0(a0)
-	fsd f16, TASK_THREAD_F16_F0(a0)
-	fsd f17, TASK_THREAD_F17_F0(a0)
-	fsd f18, TASK_THREAD_F18_F0(a0)
-	fsd f19, TASK_THREAD_F19_F0(a0)
-	fsd f20, TASK_THREAD_F20_F0(a0)
-	fsd f21, TASK_THREAD_F21_F0(a0)
-	fsd f22, TASK_THREAD_F22_F0(a0)
-	fsd f23, TASK_THREAD_F23_F0(a0)
-	fsd f24, TASK_THREAD_F24_F0(a0)
-	fsd f25, TASK_THREAD_F25_F0(a0)
-	fsd f26, TASK_THREAD_F26_F0(a0)
-	fsd f27, TASK_THREAD_F27_F0(a0)
-	fsd f28, TASK_THREAD_F28_F0(a0)
-	fsd f29, TASK_THREAD_F29_F0(a0)
-	fsd f30, TASK_THREAD_F30_F0(a0)
-	fsd f31, TASK_THREAD_F31_F0(a0)
-	sw t0, TASK_THREAD_FCSR_F0(a0)
-	csrc sstatus, t1
-	ret
-ENDPROC(__fstate_save)
-
-ENTRY(__fstate_restore)
-	li  a2,  TASK_THREAD_F0
-	add a0, a0, a2
-	li t1, SR_FS
-	lw t0, TASK_THREAD_FCSR_F0(a0)
-	csrs sstatus, t1
-	fld f0,  TASK_THREAD_F0_F0(a0)
-	fld f1,  TASK_THREAD_F1_F0(a0)
-	fld f2,  TASK_THREAD_F2_F0(a0)
-	fld f3,  TASK_THREAD_F3_F0(a0)
-	fld f4,  TASK_THREAD_F4_F0(a0)
-	fld f5,  TASK_THREAD_F5_F0(a0)
-	fld f6,  TASK_THREAD_F6_F0(a0)
-	fld f7,  TASK_THREAD_F7_F0(a0)
-	fld f8,  TASK_THREAD_F8_F0(a0)
-	fld f9,  TASK_THREAD_F9_F0(a0)
-	fld f10, TASK_THREAD_F10_F0(a0)
-	fld f11, TASK_THREAD_F11_F0(a0)
-	fld f12, TASK_THREAD_F12_F0(a0)
-	fld f13, TASK_THREAD_F13_F0(a0)
-	fld f14, TASK_THREAD_F14_F0(a0)
-	fld f15, TASK_THREAD_F15_F0(a0)
-	fld f16, TASK_THREAD_F16_F0(a0)
-	fld f17, TASK_THREAD_F17_F0(a0)
-	fld f18, TASK_THREAD_F18_F0(a0)
-	fld f19, TASK_THREAD_F19_F0(a0)
-	fld f20, TASK_THREAD_F20_F0(a0)
-	fld f21, TASK_THREAD_F21_F0(a0)
-	fld f22, TASK_THREAD_F22_F0(a0)
-	fld f23, TASK_THREAD_F23_F0(a0)
-	fld f24, TASK_THREAD_F24_F0(a0)
-	fld f25, TASK_THREAD_F25_F0(a0)
-	fld f26, TASK_THREAD_F26_F0(a0)
-	fld f27, TASK_THREAD_F27_F0(a0)
-	fld f28, TASK_THREAD_F28_F0(a0)
-	fld f29, TASK_THREAD_F29_F0(a0)
-	fld f30, TASK_THREAD_F30_F0(a0)
-	fld f31, TASK_THREAD_F31_F0(a0)
-	fscsr t0
-	csrc sstatus, t1
-	ret
-ENDPROC(__fstate_restore)
-
-
 	.section ".rodata"
 	/* Exception vector table */
 ENTRY(excp_vect_table)
diff --git a/arch/riscv/kernel/fpu.S b/arch/riscv/kernel/fpu.S
new file mode 100644
index 000000000000..1defb0618aff
--- /dev/null
+++ b/arch/riscv/kernel/fpu.S
@@ -0,0 +1,106 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2012 Regents of the University of California
+ * Copyright (C) 2017 SiFive
+ *
+ *   This program is free software; you can redistribute it and/or
+ *   modify it under the terms of the GNU General Public License
+ *   as published by the Free Software Foundation, version 2.
+ *
+ *   This program is distributed in the hope that it will be useful,
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *   GNU General Public License for more details.
+ */
+
+#include <linux/linkage.h>
+
+#include <asm/asm.h>
+#include <asm/csr.h>
+#include <asm/asm-offsets.h>
+
+ENTRY(__fstate_save)
+	li  a2,  TASK_THREAD_F0
+	add a0, a0, a2
+	li t1, SR_FS
+	csrs sstatus, t1
+	frcsr t0
+	fsd f0,  TASK_THREAD_F0_F0(a0)
+	fsd f1,  TASK_THREAD_F1_F0(a0)
+	fsd f2,  TASK_THREAD_F2_F0(a0)
+	fsd f3,  TASK_THREAD_F3_F0(a0)
+	fsd f4,  TASK_THREAD_F4_F0(a0)
+	fsd f5,  TASK_THREAD_F5_F0(a0)
+	fsd f6,  TASK_THREAD_F6_F0(a0)
+	fsd f7,  TASK_THREAD_F7_F0(a0)
+	fsd f8,  TASK_THREAD_F8_F0(a0)
+	fsd f9,  TASK_THREAD_F9_F0(a0)
+	fsd f10, TASK_THREAD_F10_F0(a0)
+	fsd f11, TASK_THREAD_F11_F0(a0)
+	fsd f12, TASK_THREAD_F12_F0(a0)
+	fsd f13, TASK_THREAD_F13_F0(a0)
+	fsd f14, TASK_THREAD_F14_F0(a0)
+	fsd f15, TASK_THREAD_F15_F0(a0)
+	fsd f16, TASK_THREAD_F16_F0(a0)
+	fsd f17, TASK_THREAD_F17_F0(a0)
+	fsd f18, TASK_THREAD_F18_F0(a0)
+	fsd f19, TASK_THREAD_F19_F0(a0)
+	fsd f20, TASK_THREAD_F20_F0(a0)
+	fsd f21, TASK_THREAD_F21_F0(a0)
+	fsd f22, TASK_THREAD_F22_F0(a0)
+	fsd f23, TASK_THREAD_F23_F0(a0)
+	fsd f24, TASK_THREAD_F24_F0(a0)
+	fsd f25, TASK_THREAD_F25_F0(a0)
+	fsd f26, TASK_THREAD_F26_F0(a0)
+	fsd f27, TASK_THREAD_F27_F0(a0)
+	fsd f28, TASK_THREAD_F28_F0(a0)
+	fsd f29, TASK_THREAD_F29_F0(a0)
+	fsd f30, TASK_THREAD_F30_F0(a0)
+	fsd f31, TASK_THREAD_F31_F0(a0)
+	sw t0, TASK_THREAD_FCSR_F0(a0)
+	csrc sstatus, t1
+	ret
+ENDPROC(__fstate_save)
+
+ENTRY(__fstate_restore)
+	li  a2,  TASK_THREAD_F0
+	add a0, a0, a2
+	li t1, SR_FS
+	lw t0, TASK_THREAD_FCSR_F0(a0)
+	csrs sstatus, t1
+	fld f0,  TASK_THREAD_F0_F0(a0)
+	fld f1,  TASK_THREAD_F1_F0(a0)
+	fld f2,  TASK_THREAD_F2_F0(a0)
+	fld f3,  TASK_THREAD_F3_F0(a0)
+	fld f4,  TASK_THREAD_F4_F0(a0)
+	fld f5,  TASK_THREAD_F5_F0(a0)
+	fld f6,  TASK_THREAD_F6_F0(a0)
+	fld f7,  TASK_THREAD_F7_F0(a0)
+	fld f8,  TASK_THREAD_F8_F0(a0)
+	fld f9,  TASK_THREAD_F9_F0(a0)
+	fld f10, TASK_THREAD_F10_F0(a0)
+	fld f11, TASK_THREAD_F11_F0(a0)
+	fld f12, TASK_THREAD_F12_F0(a0)
+	fld f13, TASK_THREAD_F13_F0(a0)
+	fld f14, TASK_THREAD_F14_F0(a0)
+	fld f15, TASK_THREAD_F15_F0(a0)
+	fld f16, TASK_THREAD_F16_F0(a0)
+	fld f17, TASK_THREAD_F17_F0(a0)
+	fld f18, TASK_THREAD_F18_F0(a0)
+	fld f19, TASK_THREAD_F19_F0(a0)
+	fld f20, TASK_THREAD_F20_F0(a0)
+	fld f21, TASK_THREAD_F21_F0(a0)
+	fld f22, TASK_THREAD_F22_F0(a0)
+	fld f23, TASK_THREAD_F23_F0(a0)
+	fld f24, TASK_THREAD_F24_F0(a0)
+	fld f25, TASK_THREAD_F25_F0(a0)
+	fld f26, TASK_THREAD_F26_F0(a0)
+	fld f27, TASK_THREAD_F27_F0(a0)
+	fld f28, TASK_THREAD_F28_F0(a0)
+	fld f29, TASK_THREAD_F29_F0(a0)
+	fld f30, TASK_THREAD_F30_F0(a0)
+	fld f31, TASK_THREAD_F31_F0(a0)
+	fscsr t0
+	csrc sstatus, t1
+	ret
+ENDPROC(__fstate_restore)
-- 
2.18.0

[PATCH v4 2/5] Refactor FPU code in signal setup/return procedures

[Alan Kao]

FPU-related logic is separated from normal signal handling path in
this patch.  Kernel can easily be configured to exclude those procedures
for no-FPU systems.

Signed-off-by: Alan Kao <alankao@andestech.com>
Cc: Greentime Hu <greentime@andestech.com>
Cc: Vincent Chen <vincentc@andestech.com>
Cc: Zong Li <zong@andestech.com>
Cc: Nick Hu <nickhu@andestech.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 arch/riscv/kernel/signal.c | 68 +++++++++++++++++++++++---------------
 1 file changed, 41 insertions(+), 27 deletions(-)

diff --git a/arch/riscv/kernel/signal.c b/arch/riscv/kernel/signal.c
index 718d0c984ef0..6a18b9819ead 100644
--- a/arch/riscv/kernel/signal.c
+++ b/arch/riscv/kernel/signal.c
@@ -37,45 +37,63 @@ struct rt_sigframe {
 	struct ucontext uc;
 };
 
-static long restore_d_state(struct pt_regs *regs,
-	struct __riscv_d_ext_state __user *state)
+static long restore_fp_state(struct pt_regs *regs,
+			     union __riscv_fp_state *sc_fpregs)
 {
 	long err;
+	struct __riscv_d_ext_state __user *state = &sc_fpregs->d;
+	size_t i;
+
 	err = __copy_from_user(&current->thread.fstate, state, sizeof(*state));
-	if (likely(!err))
-		fstate_restore(current, regs);
+	if (unlikely(err))
+		return err;
+
+	fstate_restore(current, regs);
+
+	/* We support no other extension state at this time. */
+	for (i = 0; i < ARRAY_SIZE(sc_fpregs->q.reserved); i++) {
+		u32 value;
+
+		err = __get_user(value, &sc_fpregs->q.reserved[i]);
+		if (unlikely(err))
+			break;
+		if (value != 0)
+			return -EINVAL;
+	}
+
 	return err;
 }
 
-static long save_d_state(struct pt_regs *regs,
-	struct __riscv_d_ext_state __user *state)
+static long save_fp_state(struct pt_regs *regs,
+			  union __riscv_fp_state *sc_fpregs)
 {
+	long err;
+	struct __riscv_d_ext_state __user *state = &sc_fpregs->d;
+	size_t i;
+
 	fstate_save(current, regs);
-	return __copy_to_user(state, &current->thread.fstate, sizeof(*state));
+	err = __copy_to_user(state, &current->thread.fstate, sizeof(*state));
+	if (unlikely(err))
+		return err;
+
+	/* We support no other extension state at this time. */
+	for (i = 0; i < ARRAY_SIZE(sc_fpregs->q.reserved); i++) {
+		err = __put_user(0, &sc_fpregs->q.reserved[i]);
+		if (unlikely(err))
+			break;
+	}
+
+	return err;
 }
 
 static long restore_sigcontext(struct pt_regs *regs,
 	struct sigcontext __user *sc)
 {
 	long err;
-	size_t i;
 	/* sc_regs is structured the same as the start of pt_regs */
 	err = __copy_from_user(regs, &sc->sc_regs, sizeof(sc->sc_regs));
-	if (unlikely(err))
-		return err;
 	/* Restore the floating-point state. */
-	err = restore_d_state(regs, &sc->sc_fpregs.d);
-	if (unlikely(err))
-		return err;
-	/* We support no other extension state at this time. */
-	for (i = 0; i < ARRAY_SIZE(sc->sc_fpregs.q.reserved); i++) {
-		u32 value;
-		err = __get_user(value, &sc->sc_fpregs.q.reserved[i]);
-		if (unlikely(err))
-			break;
-		if (value != 0)
-			return -EINVAL;
-	}
+	err |= restore_fp_state(regs, &sc->sc_fpregs);
 	return err;
 }
 
@@ -124,14 +142,10 @@ static long setup_sigcontext(struct rt_sigframe __user *frame,
 {
 	struct sigcontext __user *sc = &frame->uc.uc_mcontext;
 	long err;
-	size_t i;
 	/* sc_regs is structured the same as the start of pt_regs */
 	err = __copy_to_user(&sc->sc_regs, regs, sizeof(sc->sc_regs));
 	/* Save the floating-point state. */
-	err |= save_d_state(regs, &sc->sc_fpregs.d);
-	/* We support no other extension state at this time. */
-	for (i = 0; i < ARRAY_SIZE(sc->sc_fpregs.q.reserved); i++)
-		err |= __put_user(0, &sc->sc_fpregs.q.reserved[i]);
+	err |= save_fp_state(regs, &sc->sc_fpregs);
 	return err;
 }
 
-- 
2.18.0

[PATCH v4 3/5] Cleanup ISA string setting

[Alan Kao]

Just a side note: (Assume that atomic and compressed is on)

Before this patch, assembler was always given the riscv64imafdc
MARCH string because there are fld/fsd's in entry.S; compiler was
always given riscv64imac because kernel doesn't need floating point
code generation.  After this, the MARCH string in AFLAGS and CFLAGS
become the same.

Signed-off-by: Alan Kao <alankao@andestech.com>
Cc: Greentime Hu <greentime@andestech.com>
Cc: Vincent Chen <vincentc@andestech.com>
Cc: Zong Li <zong@andestech.com>
Cc: Nick Hu <nickhu@andestech.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 arch/riscv/Makefile | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 6d4a5f6c3f4f..e0fe6790624f 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -26,7 +26,6 @@ ifeq ($(CONFIG_ARCH_RV64I),y)
 
 	KBUILD_CFLAGS += -mabi=lp64
 	KBUILD_AFLAGS += -mabi=lp64
-	KBUILD_MARCH = rv64im
 	LDFLAGS += -melf64lriscv
 else
 	BITS := 32
@@ -34,22 +33,20 @@ else
 
 	KBUILD_CFLAGS += -mabi=ilp32
 	KBUILD_AFLAGS += -mabi=ilp32
-	KBUILD_MARCH = rv32im
 	LDFLAGS += -melf32lriscv
 endif
 
 KBUILD_CFLAGS += -Wall
 
-ifeq ($(CONFIG_RISCV_ISA_A),y)
-	KBUILD_ARCH_A = a
-endif
-ifeq ($(CONFIG_RISCV_ISA_C),y)
-	KBUILD_ARCH_C = c
-endif
-
-KBUILD_AFLAGS += -march=$(KBUILD_MARCH)$(KBUILD_ARCH_A)fd$(KBUILD_ARCH_C)
+# ISA string setting
+riscv-march-$(CONFIG_ARCH_RV32I)	:= rv32im
+riscv-march-$(CONFIG_ARCH_RV64I)	:= rv64im
+riscv-march-$(CONFIG_RISCV_ISA_A)	:= $(riscv-march-y)a
+riscv-march-y				:= $(riscv-march-y)fd
+riscv-march-$(CONFIG_RISCV_ISA_C)	:= $(riscv-march-y)c
+KBUILD_CFLAGS += -march=$(riscv-march-y)
+KBUILD_AFLAGS += -march=$(riscv-march-y)
 
-KBUILD_CFLAGS += -march=$(KBUILD_MARCH)$(KBUILD_ARCH_A)$(KBUILD_ARCH_C)
 KBUILD_CFLAGS += -mno-save-restore
 KBUILD_CFLAGS += -DCONFIG_PAGE_OFFSET=$(CONFIG_PAGE_OFFSET)
 
-- 
2.18.0

[PATCH v4 3/5] Cleanup ISA string setting

[Palmer Dabbelt]

On Tue, 07 Aug 2018 20:24:43 PDT (-0700), alankao at andestech.com wrote:
> Just a side note: (Assume that atomic and compressed is on)
>
> Before this patch, assembler was always given the riscv64imafdc
> MARCH string because there are fld/fsd's in entry.S; compiler was
> always given riscv64imac because kernel doesn't need floating point
> code generation.  After this, the MARCH string in AFLAGS and CFLAGS
> become the same.
>
> Signed-off-by: Alan Kao <alankao@andestech.com>
> Cc: Greentime Hu <greentime@andestech.com>
> Cc: Vincent Chen <vincentc@andestech.com>
> Cc: Zong Li <zong@andestech.com>
> Cc: Nick Hu <nickhu@andestech.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
>  arch/riscv/Makefile | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> index 6d4a5f6c3f4f..e0fe6790624f 100644
> --- a/arch/riscv/Makefile
> +++ b/arch/riscv/Makefile
> @@ -26,7 +26,6 @@ ifeq ($(CONFIG_ARCH_RV64I),y)
>
>  	KBUILD_CFLAGS += -mabi=lp64
>  	KBUILD_AFLAGS += -mabi=lp64
> -	KBUILD_MARCH = rv64im
>  	LDFLAGS += -melf64lriscv
>  else
>  	BITS := 32
> @@ -34,22 +33,20 @@ else
>
>  	KBUILD_CFLAGS += -mabi=ilp32
>  	KBUILD_AFLAGS += -mabi=ilp32
> -	KBUILD_MARCH = rv32im
>  	LDFLAGS += -melf32lriscv
>  endif
>
>  KBUILD_CFLAGS += -Wall
>
> -ifeq ($(CONFIG_RISCV_ISA_A),y)
> -	KBUILD_ARCH_A = a
> -endif
> -ifeq ($(CONFIG_RISCV_ISA_C),y)
> -	KBUILD_ARCH_C = c
> -endif
> -
> -KBUILD_AFLAGS += -march=$(KBUILD_MARCH)$(KBUILD_ARCH_A)fd$(KBUILD_ARCH_C)
> +# ISA string setting
> +riscv-march-$(CONFIG_ARCH_RV32I)	:= rv32im
> +riscv-march-$(CONFIG_ARCH_RV64I)	:= rv64im
> +riscv-march-$(CONFIG_RISCV_ISA_A)	:= $(riscv-march-y)a
> +riscv-march-y				:= $(riscv-march-y)fd
> +riscv-march-$(CONFIG_RISCV_ISA_C)	:= $(riscv-march-y)c
> +KBUILD_CFLAGS += -march=$(riscv-march-y)
> +KBUILD_AFLAGS += -march=$(riscv-march-y)
>
> -KBUILD_CFLAGS += -march=$(KBUILD_MARCH)$(KBUILD_ARCH_A)$(KBUILD_ARCH_C)

We used to have "fd" disabled in KBUILD_CFLAGS because we wanted to ensure that 
any use of floating-point types within the kernel would trigger the inclusion 
of soft-float libraries rather than emitting hardware floating-point 
instructions.  The worry here is that we'd end up corrupting the user's 
floating-point state with the kernel accesses because of the lazy save/restore 
stuff going on.

I remember at some point it was illegal to use floating-point within the 
kernel, but for some reason I thought that had changed.  I do see a handful of 
references to "float" and "double" in the kernel source, and most of references 
to kernel_fpu_begin() appear to be in SSE-specific stuff.  My one worry are the 
usages in drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c, as I can't quickly 
demonstrate they won't happen.

If we do this I think we should at least ensure kernel_fpu_{begin,end}() do the 
right thing for RISC-V.  I'd still feel safer telling the C compiler to 
disallow floating-point, though, as I'm a bit paranoid that GCC might go and 
emit a floating-point instruction even when it's not explicitly asked for.  I 
just asked Jim, who actually understands GCC, and he said that it'll spill to 
floating-point registers if the cost function determines it's cheaper than the 
stack.  While he thinks that's unlikely, I don't really want to rely a GCC cost 
function for the correctness of our kernel port.

I'd like to avoid having "-march=*fd*" in CFLAGS, unless someone can convince 
me it's safe and that I'm just being too paranoid :).

>  KBUILD_CFLAGS += -mno-save-restore
>  KBUILD_CFLAGS += -DCONFIG_PAGE_OFFSET=$(CONFIG_PAGE_OFFSET)

[PATCH v4 3/5] Cleanup ISA string setting

[Alan Kao]

On Mon, Aug 20, 2018 at 03:22:55PM -0700, Palmer Dabbelt wrote:
> On Tue, 07 Aug 2018 20:24:43 PDT (-0700), alankao at andestech.com wrote:
> >Just a side note: (Assume that atomic and compressed is on)
> >
> >Before this patch, assembler was always given the riscv64imafdc
> >MARCH string because there are fld/fsd's in entry.S; compiler was
> >always given riscv64imac because kernel doesn't need floating point
> >code generation.  After this, the MARCH string in AFLAGS and CFLAGS
> >become the same.
> >
> >Signed-off-by: Alan Kao <alankao@andestech.com>
> >Cc: Greentime Hu <greentime@andestech.com>
> >Cc: Vincent Chen <vincentc@andestech.com>
> >Cc: Zong Li <zong@andestech.com>
> >Cc: Nick Hu <nickhu@andestech.com>
> >Reviewed-by: Christoph Hellwig <hch@lst.de>
> >---
> > arch/riscv/Makefile | 19 ++++++++-----------
> > 1 file changed, 8 insertions(+), 11 deletions(-)
> >
> >diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> >index 6d4a5f6c3f4f..e0fe6790624f 100644
> >--- a/arch/riscv/Makefile
> >+++ b/arch/riscv/Makefile
> >@@ -26,7 +26,6 @@ ifeq ($(CONFIG_ARCH_RV64I),y)
> >
> > 	KBUILD_CFLAGS += -mabi=lp64
> > 	KBUILD_AFLAGS += -mabi=lp64
> >-	KBUILD_MARCH = rv64im
> > 	LDFLAGS += -melf64lriscv
> > else
> > 	BITS := 32
> >@@ -34,22 +33,20 @@ else
> >
> > 	KBUILD_CFLAGS += -mabi=ilp32
> > 	KBUILD_AFLAGS += -mabi=ilp32
> >-	KBUILD_MARCH = rv32im
> > 	LDFLAGS += -melf32lriscv
> > endif
> >
> > KBUILD_CFLAGS += -Wall
> >
> >-ifeq ($(CONFIG_RISCV_ISA_A),y)
> >-	KBUILD_ARCH_A = a
> >-endif
> >-ifeq ($(CONFIG_RISCV_ISA_C),y)
> >-	KBUILD_ARCH_C = c
> >-endif
> >-
> >-KBUILD_AFLAGS += -march=$(KBUILD_MARCH)$(KBUILD_ARCH_A)fd$(KBUILD_ARCH_C)
> >+# ISA string setting
> >+riscv-march-$(CONFIG_ARCH_RV32I)	:= rv32im
> >+riscv-march-$(CONFIG_ARCH_RV64I)	:= rv64im
> >+riscv-march-$(CONFIG_RISCV_ISA_A)	:= $(riscv-march-y)a
> >+riscv-march-y				:= $(riscv-march-y)fd
> >+riscv-march-$(CONFIG_RISCV_ISA_C)	:= $(riscv-march-y)c
> >+KBUILD_CFLAGS += -march=$(riscv-march-y)
> >+KBUILD_AFLAGS += -march=$(riscv-march-y)
> >
> >-KBUILD_CFLAGS += -march=$(KBUILD_MARCH)$(KBUILD_ARCH_A)$(KBUILD_ARCH_C)
> 
> We used to have "fd" disabled in KBUILD_CFLAGS because we wanted to ensure
> that any use of floating-point types within the kernel would trigger the
> inclusion of soft-float libraries rather than emitting hardware
> floating-point instructions.  The worry here is that we'd end up corrupting
> the user's floating-point state with the kernel accesses because of the lazy
> save/restore stuff going on.

Thanks for pointing that out.

Just as you said, the use of KBUILD_CFLAGS=*fd* is based on the assumption that
not a single floating-point instruction involves in the kernel, and that might
be wrong.

> I remember at some point it was illegal to use floating-point within the
> kernel, but for some reason I thought that had changed.  I do see a handful
> of references to "float" and "double" in the kernel source, and most of
> references to kernel_fpu_begin() appear to be in SSE-specific stuff.  My one
> worry are the usages in drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c, as
> I can't quickly demonstrate they won't happen.

Empirically, this CFLAGS with *fd* did not cause any trouble to me, but of
course this is not a good statement to support this patch.

Meanwhile, I find a flaw in "[PATCH 4/5] Allow to Disable FPU Support." 
The purpose of this "[PATCH 3/5] Cleanup ISA String" was to make CONFIG_FPU
a switch for the appearance of "fd" in both KBUILD_CFLAGS and KBUILD_ASFLAGS,
but somehow the modification was forgotten.

> 
> If we do this I think we should at least ensure kernel_fpu_{begin,end}() do
> the right thing for RISC-V.  I'd still feel safer telling the C compiler to
> disallow floating-point, though, as I'm a bit paranoid that GCC might go and
> emit a floating-point instruction even when it's not explicitly asked for.
> I just asked Jim, who actually understands GCC, and he said that it'll spill
> to floating-point registers if the cost function determines it's cheaper
> than the stack.  While he thinks that's unlikely, I don't really want to
> rely a GCC cost function for the correctness of our kernel port.

The purpose of this whole patchset was to remove all floating-point-related
logic in kernel space (while remainging FPU systems work as usual), so
implementing the two APIs would be out of scope here.

I suggest that, some people have to provide these APIs if they do need hardware
floating-point calculation in kernel space (kernel or module) in the future.
It seems that we don't need those for the kernel and any already supported
hardware drivers at least for now.  Please correct me if my understanding is
out-of-date.

> 
> I'd like to avoid having "-march=*fd*" in CFLAGS, unless someone can
> convince me it's safe and that I'm just being too paranoid :).

I will send a new version of the patchset, which will make sure that CFLAGS has
no *fd* (3/5) and the CONFIG_FPU did remove *fd* from ASFLAGS (4/5).

> 
> > KBUILD_CFLAGS += -mno-save-restore
> > KBUILD_CFLAGS += -DCONFIG_PAGE_OFFSET=$(CONFIG_PAGE_OFFSET)

Thanks!

Alan

[PATCH v4 3/5] Cleanup ISA string setting

[Palmer Dabbelt]

On Mon, 20 Aug 2018 19:47:28 PDT (-0700), alankao at andestech.com wrote:
> On Mon, Aug 20, 2018 at 03:22:55PM -0700, Palmer Dabbelt wrote:
>> On Tue, 07 Aug 2018 20:24:43 PDT (-0700), alankao at andestech.com wrote:
>> >Just a side note: (Assume that atomic and compressed is on)
>> >
>> >Before this patch, assembler was always given the riscv64imafdc
>> >MARCH string because there are fld/fsd's in entry.S; compiler was
>> >always given riscv64imac because kernel doesn't need floating point
>> >code generation.  After this, the MARCH string in AFLAGS and CFLAGS
>> >become the same.
>> >
>> >Signed-off-by: Alan Kao <alankao@andestech.com>
>> >Cc: Greentime Hu <greentime@andestech.com>
>> >Cc: Vincent Chen <vincentc@andestech.com>
>> >Cc: Zong Li <zong@andestech.com>
>> >Cc: Nick Hu <nickhu@andestech.com>
>> >Reviewed-by: Christoph Hellwig <hch@lst.de>
>> >---
>> > arch/riscv/Makefile | 19 ++++++++-----------
>> > 1 file changed, 8 insertions(+), 11 deletions(-)
>> >
>> >diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
>> >index 6d4a5f6c3f4f..e0fe6790624f 100644
>> >--- a/arch/riscv/Makefile
>> >+++ b/arch/riscv/Makefile
>> >@@ -26,7 +26,6 @@ ifeq ($(CONFIG_ARCH_RV64I),y)
>> >
>> > 	KBUILD_CFLAGS += -mabi=lp64
>> > 	KBUILD_AFLAGS += -mabi=lp64
>> >-	KBUILD_MARCH = rv64im
>> > 	LDFLAGS += -melf64lriscv
>> > else
>> > 	BITS := 32
>> >@@ -34,22 +33,20 @@ else
>> >
>> > 	KBUILD_CFLAGS += -mabi=ilp32
>> > 	KBUILD_AFLAGS += -mabi=ilp32
>> >-	KBUILD_MARCH = rv32im
>> > 	LDFLAGS += -melf32lriscv
>> > endif
>> >
>> > KBUILD_CFLAGS += -Wall
>> >
>> >-ifeq ($(CONFIG_RISCV_ISA_A),y)
>> >-	KBUILD_ARCH_A = a
>> >-endif
>> >-ifeq ($(CONFIG_RISCV_ISA_C),y)
>> >-	KBUILD_ARCH_C = c
>> >-endif
>> >-
>> >-KBUILD_AFLAGS += -march=$(KBUILD_MARCH)$(KBUILD_ARCH_A)fd$(KBUILD_ARCH_C)
>> >+# ISA string setting
>> >+riscv-march-$(CONFIG_ARCH_RV32I)	:= rv32im
>> >+riscv-march-$(CONFIG_ARCH_RV64I)	:= rv64im
>> >+riscv-march-$(CONFIG_RISCV_ISA_A)	:= $(riscv-march-y)a
>> >+riscv-march-y				:= $(riscv-march-y)fd
>> >+riscv-march-$(CONFIG_RISCV_ISA_C)	:= $(riscv-march-y)c
>> >+KBUILD_CFLAGS += -march=$(riscv-march-y)
>> >+KBUILD_AFLAGS += -march=$(riscv-march-y)
>> >
>> >-KBUILD_CFLAGS += -march=$(KBUILD_MARCH)$(KBUILD_ARCH_A)$(KBUILD_ARCH_C)
>>
>> We used to have "fd" disabled in KBUILD_CFLAGS because we wanted to ensure
>> that any use of floating-point types within the kernel would trigger the
>> inclusion of soft-float libraries rather than emitting hardware
>> floating-point instructions.  The worry here is that we'd end up corrupting
>> the user's floating-point state with the kernel accesses because of the lazy
>> save/restore stuff going on.
>
> Thanks for pointing that out.
>
> Just as you said, the use of KBUILD_CFLAGS=*fd* is based on the assumption that
> not a single floating-point instruction involves in the kernel, and that might
> be wrong.
>
>> I remember at some point it was illegal to use floating-point within the
>> kernel, but for some reason I thought that had changed.  I do see a handful
>> of references to "float" and "double" in the kernel source, and most of
>> references to kernel_fpu_begin() appear to be in SSE-specific stuff.  My one
>> worry are the usages in drivers/gpu/drm/amd/display/dc/calcs/dcn_calcs.c, as
>> I can't quickly demonstrate they won't happen.
>
> Empirically, this CFLAGS with *fd* did not cause any trouble to me, but of
> course this is not a good statement to support this patch.
>
> Meanwhile, I find a flaw in "[PATCH 4/5] Allow to Disable FPU Support."
> The purpose of this "[PATCH 3/5] Cleanup ISA String" was to make CONFIG_FPU
> a switch for the appearance of "fd" in both KBUILD_CFLAGS and KBUILD_ASFLAGS,
> but somehow the modification was forgotten.
>
>>
>> If we do this I think we should at least ensure kernel_fpu_{begin,end}() do
>> the right thing for RISC-V.  I'd still feel safer telling the C compiler to
>> disallow floating-point, though, as I'm a bit paranoid that GCC might go and
>> emit a floating-point instruction even when it's not explicitly asked for.
>> I just asked Jim, who actually understands GCC, and he said that it'll spill
>> to floating-point registers if the cost function determines it's cheaper
>> than the stack.  While he thinks that's unlikely, I don't really want to
>> rely a GCC cost function for the correctness of our kernel port.
>
> The purpose of this whole patchset was to remove all floating-point-related
> logic in kernel space (while remainging FPU systems work as usual), so
> implementing the two APIs would be out of scope here.
>
> I suggest that, some people have to provide these APIs if they do need hardware
> floating-point calculation in kernel space (kernel or module) in the future.
> It seems that we don't need those for the kernel and any already supported
> hardware drivers at least for now.  Please correct me if my understanding is
> out-of-date.
>
>>
>> I'd like to avoid having "-march=*fd*" in CFLAGS, unless someone can
>> convince me it's safe and that I'm just being too paranoid :).
>
> I will send a new version of the patchset, which will make sure that CFLAGS has
> no *fd* (3/5) and the CONFIG_FPU did remove *fd* from ASFLAGS (4/5).

Thanks!

>
>>
>> > KBUILD_CFLAGS += -mno-save-restore
>> > KBUILD_CFLAGS += -DCONFIG_PAGE_OFFSET=$(CONFIG_PAGE_OFFSET)
>
> Thanks!
>
> Alan

[PATCH v4 4/5] Allow to disable FPU support

[Alan Kao]

FPU codes have been separated from common part in previous patches.
This patch add the CONFIG_FPU option and some stubs, so that a no-FPU
configuration is allowed.

Signed-off-by: Alan Kao <alankao@andestech.com>
Cc: Greentime Hu <greentime@andestech.com>
Cc: Vincent Chen <vincentc@andestech.com>
Cc: Zong Li <zong@andestech.com>
Cc: Nick Hu <nickhu@andestech.com>
---
 arch/riscv/Kconfig                 |  9 +++++++++
 arch/riscv/include/asm/switch_to.h | 10 ++++++++++
 arch/riscv/kernel/Makefile         |  2 +-
 arch/riscv/kernel/process.c        |  4 +++-
 arch/riscv/kernel/signal.c         |  5 +++++
 5 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 6debcc4afc72..6069597ba73f 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -232,6 +232,15 @@ config RISCV_BASE_PMU
 
 endmenu
 
+config FPU
+	bool "FPU support"
+	default y
+	help
+	  Say N here if you want to disable all floating-point related procedure
+	  in the kernel.
+
+	  If you don't know what to do here, say Y.
+
 endmenu
 
 menu "Kernel type"
diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h
index dd6b05bff75b..093050b03543 100644
--- a/arch/riscv/include/asm/switch_to.h
+++ b/arch/riscv/include/asm/switch_to.h
@@ -18,6 +18,7 @@
 #include <asm/ptrace.h>
 #include <asm/csr.h>
 
+#ifdef CONFIG_FPU
 extern void __fstate_save(struct task_struct *save_to);
 extern void __fstate_restore(struct task_struct *restore_from);
 
@@ -55,6 +56,15 @@ static inline void __switch_to_aux(struct task_struct *prev,
 	fstate_restore(next, task_pt_regs(next));
 }
 
+#define DEFAULT_SSTATUS (SR_SPIE | SR_FS_INITIAL)
+
+#else
+#define fstate_save(task, regs) do { } while (0)
+#define fstate_restore(task, regs) do { } while (0)
+#define __switch_to_aux(__prev, __next) do { } while (0)
+#define DEFAULT_SSTATUS (SR_SPIE | SR_FS_OFF)
+#endif
+
 extern struct task_struct *__switch_to(struct task_struct *,
 				       struct task_struct *);
 
diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index bd433efd915e..f13f7f276639 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -13,7 +13,6 @@ extra-y += vmlinux.lds
 obj-y	+= cpu.o
 obj-y	+= cpufeature.o
 obj-y	+= entry.o
-obj-y	+= fpu.o
 obj-y	+= irq.o
 obj-y	+= process.o
 obj-y	+= ptrace.o
@@ -32,6 +31,7 @@ obj-y	+= vdso/
 
 CFLAGS_setup.o := -mcmodel=medany
 
+obj-$(CONFIG_FPU)		+= fpu.o
 obj-$(CONFIG_SMP)		+= smpboot.o
 obj-$(CONFIG_SMP)		+= smp.o
 obj-$(CONFIG_MODULES)		+= module.o
diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
index cb209139ba53..3820d89e2db9 100644
--- a/arch/riscv/kernel/process.c
+++ b/arch/riscv/kernel/process.c
@@ -83,7 +83,7 @@ void show_regs(struct pt_regs *regs)
 void start_thread(struct pt_regs *regs, unsigned long pc,
 	unsigned long sp)
 {
-	regs->sstatus = SR_SPIE /* User mode, irqs on */ | SR_FS_INITIAL;
+	regs->sstatus = DEFAULT_SSTATUS;
 	regs->sepc = pc;
 	regs->sp = sp;
 	set_fs(USER_DS);
@@ -91,12 +91,14 @@ void start_thread(struct pt_regs *regs, unsigned long pc,
 
 void flush_thread(void)
 {
+#ifdef CONFIG_FPU
 	/*
 	 * Reset FPU context
 	 *	frm: round to nearest, ties to even (IEEE default)
 	 *	fflags: accrued exceptions cleared
 	 */
 	memset(&current->thread.fstate, 0, sizeof(current->thread.fstate));
+#endif
 }
 
 int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
diff --git a/arch/riscv/kernel/signal.c b/arch/riscv/kernel/signal.c
index 6a18b9819ead..2450b824d799 100644
--- a/arch/riscv/kernel/signal.c
+++ b/arch/riscv/kernel/signal.c
@@ -37,6 +37,7 @@ struct rt_sigframe {
 	struct ucontext uc;
 };
 
+#ifdef CONFIG_FPU
 static long restore_fp_state(struct pt_regs *regs,
 			     union __riscv_fp_state *sc_fpregs)
 {
@@ -85,6 +86,10 @@ static long save_fp_state(struct pt_regs *regs,
 
 	return err;
 }
+#else
+#define save_fp_state(task, regs) (0)
+#define restore_fp_state(task, regs) (0)
+#endif
 
 static long restore_sigcontext(struct pt_regs *regs,
 	struct sigcontext __user *sc)
-- 
2.18.0

[PATCH v4 4/5] Allow to disable FPU support

[Christoph Hellwig]

On Wed, Aug 08, 2018 at 11:24:44AM +0800, Alan Kao wrote:
> FPU codes have been separated from common part in previous patches.
> This patch add the CONFIG_FPU option and some stubs, so that a no-FPU
> configuration is allowed.
> 
> Signed-off-by: Alan Kao <alankao@andestech.com>
> Cc: Greentime Hu <greentime@andestech.com>
> Cc: Vincent Chen <vincentc@andestech.com>
> Cc: Zong Li <zong@andestech.com>
> Cc: Nick Hu <nickhu@andestech.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH v4 5/5] Auto-detect whether a FPU exists

[Alan Kao]

We expect that a kernel with CONFIG_FPU=y can still support no-FPU machines.
To do so, the kernel should first examine the existence of a FPU, then
do nothing if a FPU does exist; otherwise, it should disable/bypass all
FPU-related functions.

In this patch, a new global variable, no_fpu, is created and determined
when parsing the hardware capability from device tree during booting.
This variable is used in those FPU-related functions.

Signed-off-by: Alan Kao <alankao@andestech.com>
Cc: Greentime Hu <greentime@andestech.com>
Cc: Vincent Chen <vincentc@andestech.com>
Cc: Zong Li <zong@andestech.com>
Cc: Nick Hu <nickhu@andestech.com>
---
 arch/riscv/include/asm/hwcap.h     |  3 +++
 arch/riscv/include/asm/switch_to.h | 13 ++++++++++++-
 arch/riscv/kernel/cpufeature.c     | 11 +++++++++++
 arch/riscv/kernel/signal.c         |  6 ++++++
 4 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index 8a4ed7bbcbea..1b870086a869 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -33,5 +33,8 @@ enum {
 };
 
 extern unsigned long elf_hwcap;
+#ifdef CONFIG_FPU
+extern bool no_fpu;
+#endif
 #endif
 #endif
diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h
index 093050b03543..7278e3eb7a70 100644
--- a/arch/riscv/include/asm/switch_to.h
+++ b/arch/riscv/include/asm/switch_to.h
@@ -17,6 +17,7 @@
 #include <asm/processor.h>
 #include <asm/ptrace.h>
 #include <asm/csr.h>
+#include <asm/hwcap.h>
 
 #ifdef CONFIG_FPU
 extern void __fstate_save(struct task_struct *save_to);
@@ -30,6 +31,9 @@ static inline void __fstate_clean(struct pt_regs *regs)
 static inline void fstate_save(struct task_struct *task,
 			       struct pt_regs *regs)
 {
+	if (unlikely(no_fpu))
+		return;
+
 	if ((regs->sstatus & SR_FS) == SR_FS_DIRTY) {
 		__fstate_save(task);
 		__fstate_clean(regs);
@@ -39,6 +43,9 @@ static inline void fstate_save(struct task_struct *task,
 static inline void fstate_restore(struct task_struct *task,
 				  struct pt_regs *regs)
 {
+	if (unlikely(no_fpu))
+		return;
+
 	if ((regs->sstatus & SR_FS) != SR_FS_OFF) {
 		__fstate_restore(task);
 		__fstate_clean(regs);
@@ -50,13 +57,17 @@ static inline void __switch_to_aux(struct task_struct *prev,
 {
 	struct pt_regs *regs;
 
+	if (unlikely(no_fpu))
+		return;
+
 	regs = task_pt_regs(prev);
 	if (unlikely(regs->sstatus & SR_SD))
 		fstate_save(prev, regs);
 	fstate_restore(next, task_pt_regs(next));
 }
 
-#define DEFAULT_SSTATUS (SR_SPIE | SR_FS_INITIAL)
+#define DEFAULT_SSTATUS \
+	((unlikely(no_fpu)) ? (SR_SPIE | SR_FS_OFF) : (SR_SPIE | SR_FS_INITIAL))
 
 #else
 #define fstate_save(task, regs) do { } while (0)
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 17011a870044..bc269c1e0b1a 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -22,6 +22,9 @@
 #include <asm/hwcap.h>
 
 unsigned long elf_hwcap __read_mostly;
+#ifdef CONFIG_FPU
+bool no_fpu __read_mostly;
+#endif
 
 void riscv_fill_hwcap(void)
 {
@@ -58,4 +61,12 @@ void riscv_fill_hwcap(void)
 		elf_hwcap |= isa2hwcap[(unsigned char)(isa[i])];
 
 	pr_info("elf_hwcap is 0x%lx", elf_hwcap);
+
+#ifdef CONFIG_FPU
+	no_fpu = 0;
+	if (!(elf_hwcap & (COMPAT_HWCAP_ISA_F | COMPAT_HWCAP_ISA_D))) {
+		pr_info("Bypass FPU code.");
+		no_fpu = 1;
+	}
+#endif
 }
diff --git a/arch/riscv/kernel/signal.c b/arch/riscv/kernel/signal.c
index 2450b824d799..9714e4fccb69 100644
--- a/arch/riscv/kernel/signal.c
+++ b/arch/riscv/kernel/signal.c
@@ -45,6 +45,9 @@ static long restore_fp_state(struct pt_regs *regs,
 	struct __riscv_d_ext_state __user *state = &sc_fpregs->d;
 	size_t i;
 
+	if (unlikely(no_fpu))
+		return 0;
+
 	err = __copy_from_user(&current->thread.fstate, state, sizeof(*state));
 	if (unlikely(err))
 		return err;
@@ -72,6 +75,9 @@ static long save_fp_state(struct pt_regs *regs,
 	struct __riscv_d_ext_state __user *state = &sc_fpregs->d;
 	size_t i;
 
+	if (unlikely(no_fpu))
+		return 0;
+
 	fstate_save(current, regs);
 	err = __copy_to_user(state, &current->thread.fstate, sizeof(*state));
 	if (unlikely(err))
-- 
2.18.0

[PATCH v4 5/5] Auto-detect whether a FPU exists

[Christoph Hellwig]

>  extern unsigned long elf_hwcap;
> +#ifdef CONFIG_FPU
> +extern bool no_fpu;
> +#endif

No need to have an ifdef around an extern declaration.

>  static inline void fstate_save(struct task_struct *task,
>  			       struct pt_regs *regs)
>  {
> +	if (unlikely(no_fpu))
> +		return;
> +
>  	if ((regs->sstatus & SR_FS) == SR_FS_DIRTY) {

Wouldn't the sstatus check here always evaluate to false for the
no_fpu case anyway?

> @@ -39,6 +43,9 @@ static inline void fstate_save(struct task_struct *task,
>  static inline void fstate_restore(struct task_struct *task,
>  				  struct pt_regs *regs)
>  {
> +	if (unlikely(no_fpu))
> +		return;
> +

Same here?

> -#define DEFAULT_SSTATUS (SR_SPIE | SR_FS_INITIAL)
> +#define DEFAULT_SSTATUS \
> +	((unlikely(no_fpu)) ? (SR_SPIE | SR_FS_OFF) : (SR_SPIE | SR_FS_INITIAL))

Please don't hide this in a a macro.

I'd rather get rid of the macro and do this in start_thread:

	regs->sstatus = SR_SPIE /* User mode, irqs on */
	if (!no_fpu)
		regs->sstatus |= SR_FS_INITIAL;

and provide a stub for the no_fpu variable for the !CONFIG_CPU case.

In fact I'd probably invert the polarity of this variable and make it
'has_fpu'.  The for !CONFIG_FPU you define that as

#define has_cpu	false
		
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -22,6 +22,9 @@
>  #include <asm/hwcap.h>
>  
>  unsigned long elf_hwcap __read_mostly;
> +#ifdef CONFIG_FPU
> +bool no_fpu __read_mostly;
> +#endif
>  
>  void riscv_fill_hwcap(void)
>  {
> @@ -58,4 +61,12 @@ void riscv_fill_hwcap(void)
>  		elf_hwcap |= isa2hwcap[(unsigned char)(isa[i])];
>  
>  	pr_info("elf_hwcap is 0x%lx", elf_hwcap);
> +
> +#ifdef CONFIG_FPU
> +	no_fpu = 0;
> +	if (!(elf_hwcap & (COMPAT_HWCAP_ISA_F | COMPAT_HWCAP_ISA_D))) {
> +		pr_info("Bypass FPU code.");
> +		no_fpu = 1;
> +	}
> +#endif

Note that variables unless they are on stack in a function are always
initialized to zero.  So together with my above ideas this could become:

#ifdef CONFIG_FPU
	if (elf_hwcap & (COMPAT_HWCAP_ISA_F | COMPAT_HWCAP_ISA_D)
		has_fpu = true;
#endif

Note the use of true/false for booleans and dropping the printk.

> diff --git a/arch/riscv/kernel/signal.c b/arch/riscv/kernel/signal.c
> index 2450b824d799..9714e4fccb69 100644
> --- a/arch/riscv/kernel/signal.c
> +++ b/arch/riscv/kernel/signal.c
> @@ -45,6 +45,9 @@ static long restore_fp_state(struct pt_regs *regs,
>  	struct __riscv_d_ext_state __user *state = &sc_fpregs->d;
>  	size_t i;
>  
> +	if (unlikely(no_fpu))
> +		return 0;

I'd be tempted to move this into the caler, e.g.

	if (has_fpu) {
		restore_fp_state()..
	}

Also the unlikely annotations seem odd - this seems like something that
even the simplest branch predictor can handle.  If we really want to
optimize it (not for this series but in the future) we should implement
the alternatives mechanism for live patching.

[PATCH v4 0/5] riscv: Add support to no-FPU systems

[Palmer Dabbelt]

On Tue, 07 Aug 2018 20:24:40 PDT (-0700), alankao at andestech.com wrote:
> This patchset adds an option, CONFIG_FPU, to enable/disable floating-
> point procedures.
>
> Kernel's new behavior will be as follows:
>
> * with CONFIG_FPU=y
>   All FPU codes are reserved.  If no FPU is found during booting, a
>   global flag will be set, and those functions will be bypassed with
>   condition check to that flag.
>
> * with CONFIG_FPU=n
>   No floating-point instructions in kernel and all related settings
>   are excluded.
>
> Changes in v4:
>  - Append a new patch to detect existence of FPU and followups.
>  - Add SPDX header to newly created fpu.S.
>  - Fix a build error, sorry for that.
>  - Fix wording, style, etc.
>
> Changes in v3:
>  - Refactor the whole patch into independent ones.
>
> Changes in v2:
>  - Various code cleanups and style fixes.
>
> Alan Kao (5):
>   Extract FPU context operations from entry.S
>   Refactor FPU code in signal setup/return procedures
>   Cleanup ISA string setting
>   Allow to disable FPU support
>   Auto-detect whether a FPU exists
>
>  arch/riscv/Kconfig                 |   9 +++
>  arch/riscv/Makefile                |  19 +++---
>  arch/riscv/include/asm/hwcap.h     |   3 +
>  arch/riscv/include/asm/switch_to.h |  21 ++++++
>  arch/riscv/kernel/Makefile         |   1 +
>  arch/riscv/kernel/cpufeature.c     |  11 +++
>  arch/riscv/kernel/entry.S          |  87 -----------------------
>  arch/riscv/kernel/fpu.S            | 106 +++++++++++++++++++++++++++++
>  arch/riscv/kernel/process.c        |   4 +-
>  arch/riscv/kernel/signal.c         |  79 +++++++++++++--------
>  10 files changed, 214 insertions(+), 126 deletions(-)
>  create mode 100644 arch/riscv/kernel/fpu.S

Aside from the CFLAGS issue this looks good.  I've queued this up in 
kernel.org/palmer/linux.git/next-nofpu, but I'm not going to put this on 
for-next until after the merge window closes.

Thanks!