2018-08-02 05:43:31

by Alan Kao

[permalink] [raw]
Subject: [PATCH v3 0/4] riscv: Add support to no-FPU systems

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

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

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

Alan Kao (4):
Extract FPU context operations from entry.S
Refactor FPU codes in signal setup/return procedures
Cleanup ISA string setting
Add an option to support no-FPU systems

arch/riscv/Kconfig | 9 +++
arch/riscv/Makefile | 19 +++---
arch/riscv/include/asm/switch_to.h | 12 ++++
arch/riscv/kernel/Makefile | 1 +
arch/riscv/kernel/entry.S | 87 ------------------------
arch/riscv/kernel/fpu.S | 105 +++++++++++++++++++++++++++++
arch/riscv/kernel/process.c | 4 +-
arch/riscv/kernel/signal.c | 70 +++++++++++--------
8 files changed, 181 insertions(+), 126 deletions(-)
create mode 100644 arch/riscv/kernel/fpu.S

--
2.18.0



2018-08-02 05:41:28

by Alan Kao

[permalink] [raw]
Subject: [PATCH v3 1/4] Extract FPU context operations from entry.S

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

Signed-off-by: Alan Kao <[email protected]>
Cc: Greentime Hu <[email protected]>
Cc: Vincent Chen <[email protected]>
Cc: Zong Li <[email protected]>
Cc: Nick Hu <[email protected]>
---
arch/riscv/kernel/Makefile | 1 +
arch/riscv/kernel/entry.S | 87 ------------------------------
arch/riscv/kernel/fpu.S | 105 +++++++++++++++++++++++++++++++++++++
3 files changed, 106 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..3210ef502e55
--- /dev/null
+++ b/arch/riscv/kernel/fpu.S
@@ -0,0 +1,105 @@
+/*
+ * 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


2018-08-02 05:41:33

by Alan Kao

[permalink] [raw]
Subject: [PATCH v3 2/4] Refactor FPU codes in signal setup/return procedures

Signed-off-by: Alan Kao <[email protected]>
Cc: Greentime Hu <[email protected]>
Cc: Vincent Chen <[email protected]>
Cc: Zong Li <[email protected]>
Cc: Nick Hu <[email protected]>
---
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..bfce852d5ddc 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


2018-08-02 05:41:59

by Alan Kao

[permalink] [raw]
Subject: [PATCH v3 3/4] Cleanup ISA string setting

The reason that we cannot follow the review's suggestion in
https://lkml.org/lkml/2018/6/21/39 is because using "+=" as the
connector in Makefile introduces blanks bewteen the left-hand
side alphabets.

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 <[email protected]>
Cc: Greentime Hu <[email protected]>
Cc: Vincent Chen <[email protected]>
Cc: Zong Li <[email protected]>
Cc: Nick Hu <[email protected]>
---
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


2018-08-02 05:43:03

by Alan Kao

[permalink] [raw]
Subject: [PATCH v3 4/4] Add an option to support no-FPU systems

FP codes have been separated from common part in previous patches.
This patch add the CONFIG_FPU option and some stubs to support
no-FPU systems.

Signed-off-by: Alan Kao <[email protected]>
Cc: Greentime Hu <[email protected]>
Cc: Vincent Chen <[email protected]>
Cc: Zong Li <[email protected]>
Cc: Nick Hu <[email protected]>
---
arch/riscv/Kconfig | 9 +++++++++
arch/riscv/include/asm/switch_to.h | 12 ++++++++++++
arch/riscv/kernel/Makefile | 2 +-
arch/riscv/kernel/process.c | 4 +++-
arch/riscv/kernel/signal.c | 2 ++
5 files changed, 27 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..94a08d28ccf5 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,17 @@ 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 save_fp_state(task, regs) (0)
+#define restore_fp_state(task, regs) (0)
+#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 bfce852d5ddc..b7bbb5c33594 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,7 @@ static long save_fp_state(struct pt_regs *regs,

return err;
}
+#endif

static long restore_sigcontext(struct pt_regs *regs,
struct sigcontext __user *sc)
--
2.18.0


2018-08-02 11:37:12

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] Extract FPU context operations from entry.S

> diff --git a/arch/riscv/kernel/fpu.S b/arch/riscv/kernel/fpu.S
> new file mode 100644
> index 000000000000..3210ef502e55
> --- /dev/null
> +++ b/arch/riscv/kernel/fpu.S
> @@ -0,0 +1,105 @@
> +/*
> + * 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.
> + */

It would be good to have SPDX headers fo every new file (and eventually
switch the other RISC-V code over).

Otherwise this looks good:

Reviewed-by: Christoph Hellwig <[email protected]>

2018-08-02 11:39:02

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v3 2/4] Refactor FPU codes in signal setup/return procedures

s/codes/code/ in the subject. Also a little explanation of why
you refactor it in the patch description would be helpful.

> +static long restore_fp_state(struct pt_regs *regs,
> + union __riscv_fp_state *sc_fpregs)

Please add another tab for the indentation here even if the original
code got it wrong.

> +static long save_fp_state(struct pt_regs *regs,
> + union __riscv_fp_state *sc_fpregs)

Same here.

Otherwise looks good:

Reviewed-by: Christoph Hellwig <[email protected]>

2018-08-02 11:40:50

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v3 3/4] Cleanup ISA string setting



On Thu, Aug 02, 2018 at 01:39:50PM +0800, Alan Kao wrote:
> The reason that we cannot follow the review's suggestion in
> https://lkml.org/lkml/2018/6/21/39 is because using "+=" as the
> connector in Makefile introduces blanks bewteen the left-hand
> side alphabets.

That normally goes into the reply to the suggestion, not into the
changelog, so please drop it.

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

Otherwise looks good:

Reviewed-by: Christoph Hellwig <[email protected]>

2018-08-02 11:42:55

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] Add an option to support no-FPU systems

On Thu, Aug 02, 2018 at 01:39:51PM +0800, Alan Kao wrote:
> FP codes have been separated from common part in previous patches.
> This patch add the CONFIG_FPU option and some stubs to support
> no-FPU systems.

I think the subject should be 'allow to disable FPU support'.

As discussed in the other thread we should be able to detect
systems without FPU and handle them fine even with FPU support built
in. Even with that I think this patch is otherwise fine and the
detection can be layered on top. One more nitpick below:

> +#else
> +#define save_fp_state(task, regs) (0)
> +#define restore_fp_state(task, regs) (0)
> +#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

Please move the stubs for functions that are static in signal.c
into signal.c as well - you already have a CONFIG_FPU ifdef block
in that file anyway.

2018-08-03 06:47:05

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v3 2/4] Refactor FPU codes in signal setup/return procedures

Hi Alan,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.18-rc7 next-20180802]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Alan-Kao/Extract-FPU-context-operations-from-entry-S/20180803-064749
config: riscv-defconfig (attached as .config)
compiler: riscv64-linux-gcc (GCC) 8.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.1.0 make.cross ARCH=riscv

All errors (new ones prefixed by >>):

In file included from include/asm-generic/bug.h:18,
from arch/riscv/include/asm/bug.h:75,
from include/linux/bug.h:5,
from include/linux/signal.h:5,
from arch/riscv/kernel/signal.c:22:
arch/riscv/kernel/signal.c: In function 'save_fp_state':
>> arch/riscv/kernel/signal.c:80:38: error: 'sc_fpregs' is a pointer; did you mean to use '->'?
for (i = 0; i < ARRAY_SIZE(sc_fpregs.q.reserved); i++) {
^
include/linux/kernel.h:72:33: note: in definition of macro 'ARRAY_SIZE'
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
^~~
>> arch/riscv/kernel/signal.c:80:38: error: 'sc_fpregs' is a pointer; did you mean to use '->'?
for (i = 0; i < ARRAY_SIZE(sc_fpregs.q.reserved); i++) {
^
include/linux/kernel.h:72:48: note: in definition of macro 'ARRAY_SIZE'
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
^~~
In file included from include/linux/kernel.h:15,
from include/asm-generic/bug.h:18,
from arch/riscv/include/asm/bug.h:75,
from include/linux/bug.h:5,
from include/linux/signal.h:5,
from arch/riscv/kernel/signal.c:22:
>> arch/riscv/kernel/signal.c:80:38: error: 'sc_fpregs' is a pointer; did you mean to use '->'?
for (i = 0; i < ARRAY_SIZE(sc_fpregs.q.reserved); i++) {
^
include/linux/build_bug.h:29:56: note: in definition of macro 'BUILD_BUG_ON_ZERO'
#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:(-!!(e)); }))
^
include/linux/compiler-gcc.h:65:46: note: in expansion of macro '__same_type'
#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
^~~~~~~~~~~
include/linux/kernel.h:72:59: note: in expansion of macro '__must_be_array'
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
^~~~~~~~~~~~~~~
arch/riscv/kernel/signal.c:80:18: note: in expansion of macro 'ARRAY_SIZE'
for (i = 0; i < ARRAY_SIZE(sc_fpregs.q.reserved); i++) {
^~~~~~~~~~
>> arch/riscv/kernel/signal.c:80:38: error: 'sc_fpregs' is a pointer; did you mean to use '->'?
for (i = 0; i < ARRAY_SIZE(sc_fpregs.q.reserved); i++) {
^
include/linux/build_bug.h:29:56: note: in definition of macro 'BUILD_BUG_ON_ZERO'
#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:(-!!(e)); }))
^
include/linux/compiler-gcc.h:65:46: note: in expansion of macro '__same_type'
#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
^~~~~~~~~~~
include/linux/kernel.h:72:59: note: in expansion of macro '__must_be_array'
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
^~~~~~~~~~~~~~~
arch/riscv/kernel/signal.c:80:18: note: in expansion of macro 'ARRAY_SIZE'
for (i = 0; i < ARRAY_SIZE(sc_fpregs.q.reserved); i++) {
^~~~~~~~~~
include/linux/build_bug.h:29:45: error: bit-field '<anonymous>' width not an integer constant
#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:(-!!(e)); }))
^
include/linux/compiler-gcc.h:65:28: note: in expansion of macro 'BUILD_BUG_ON_ZERO'
#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
^~~~~~~~~~~~~~~~~
include/linux/kernel.h:72:59: note: in expansion of macro '__must_be_array'
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
^~~~~~~~~~~~~~~
arch/riscv/kernel/signal.c:80:18: note: in expansion of macro 'ARRAY_SIZE'
for (i = 0; i < ARRAY_SIZE(sc_fpregs.q.reserved); i++) {
^~~~~~~~~~
In file included from include/linux/uaccess.h:14,
from arch/riscv/kernel/signal.c:23:
arch/riscv/kernel/signal.c:81:33: error: 'sc_fpregs' is a pointer; did you mean to use '->'?
err = __put_user(0, &sc_fpregs.q.reserved[i]);
^
arch/riscv/include/asm/uaccess.h:348:15: note: in definition of macro '__put_user'
__typeof__(*(ptr)) __user *__gu_ptr = (ptr); \
^~~
arch/riscv/kernel/signal.c:81:33: error: 'sc_fpregs' is a pointer; did you mean to use '->'?
err = __put_user(0, &sc_fpregs.q.reserved[i]);
^
arch/riscv/include/asm/uaccess.h:348:41: note: in definition of macro '__put_user'
__typeof__(*(ptr)) __user *__gu_ptr = (ptr); \
^~~

vim +80 arch/riscv/kernel/signal.c

> 22 #include <linux/signal.h>
23 #include <linux/uaccess.h>
24 #include <linux/syscalls.h>
25 #include <linux/tracehook.h>
26 #include <linux/linkage.h>
27
28 #include <asm/ucontext.h>
29 #include <asm/vdso.h>
30 #include <asm/switch_to.h>
31 #include <asm/csr.h>
32
33 #define DEBUG_SIG 0
34
35 struct rt_sigframe {
36 struct siginfo info;
37 struct ucontext uc;
38 };
39
40 static long restore_fp_state(struct pt_regs *regs,
41 union __riscv_fp_state *sc_fpregs)
42 {
43 long err;
44 struct __riscv_d_ext_state __user *state = &sc_fpregs->d;
45 size_t i;
46
47 err = __copy_from_user(&current->thread.fstate, state, sizeof(*state));
48 if (unlikely(err))
49 return err;
50
51 fstate_restore(current, regs);
52
53 /* We support no other extension state at this time. */
54 for (i = 0; i < ARRAY_SIZE(sc_fpregs->q.reserved); i++) {
55 u32 value;
56
57 err = __get_user(value, &sc_fpregs->q.reserved[i]);
58 if (unlikely(err))
59 break;
60 if (value != 0)
61 return -EINVAL;
62 }
63
64 return err;
65 }
66
67 static long save_fp_state(struct pt_regs *regs,
68 union __riscv_fp_state *sc_fpregs)
69 {
70 long err;
71 struct __riscv_d_ext_state __user *state = &sc_fpregs->d;
72 size_t i;
73
74 fstate_save(current, regs);
75 err = __copy_to_user(state, &current->thread.fstate, sizeof(*state));
76 if (unlikely(err))
77 return err;
78
79 /* We support no other extension state at this time. */
> 80 for (i = 0; i < ARRAY_SIZE(sc_fpregs.q.reserved); i++) {
81 err = __put_user(0, &sc_fpregs.q.reserved[i]);
82 if (unlikely(err))
83 break;
84 }
85
86 return err;
87 }
88

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (8.05 kB)
.config.gz (15.94 kB)
Download all attachments