[PATCH 0/8] riscv: resolve most warnings from sparse

[PATCH 0/8] riscv: resolve most warnings from sparse

[Paul Walmsley]

Resolve most warnings from the 'sparse' static analysis tool for the
arch/riscv codebase.  This makes life easier for us as maintainers,
and makes it easier for developers to use static analysis tools on
their own changes.

This patch series incorporates some changes based on feedback from
Christoph Hellwig <hch@lst.de>.

Applies on the current riscv fixes branch that is based on v5.4-rc3.


- Paul


Paul Walmsley (8):
  riscv: add prototypes for assembly language functions from entry.S
  riscv: add prototypes for assembly language functions from head.S
  riscv: init: merge split string literals in preprocessor directive
  riscv: ensure RISC-V C model definitions are passed to static
    analyzers
  riscv: add missing prototypes
  riscv: mark some code and data as file-static
  riscv: add missing header file includes
  riscv: fp: add missing __user pointer annotations

Kernel object size difference:
   text	   data     bss	    dec	    hex	filename
6664206 2136568  312608 9113382  8b0f26	vmlinux.orig
6664186 2136552	 312608 9113346  8b0f02	vmlinux.patched

 arch/riscv/Makefile                 |  2 ++
 arch/riscv/include/asm/irq.h        |  6 ++++++
 arch/riscv/include/asm/pgtable.h    |  2 ++
 arch/riscv/include/asm/processor.h  |  4 ++++
 arch/riscv/include/asm/ptrace.h     |  4 ++++
 arch/riscv/include/asm/smp.h        |  2 ++
 arch/riscv/include/asm/switch_to.h  |  1 +
 arch/riscv/kernel/cpufeature.c      |  1 +
 arch/riscv/kernel/entry.h           | 29 +++++++++++++++++++++++++++++
 arch/riscv/kernel/head.h            | 21 +++++++++++++++++++++
 arch/riscv/kernel/module-sections.c |  1 +
 arch/riscv/kernel/process.c         |  2 ++
 arch/riscv/kernel/reset.c           |  1 +
 arch/riscv/kernel/setup.c           |  2 ++
 arch/riscv/kernel/signal.c          |  6 ++++--
 arch/riscv/kernel/smp.c             |  2 ++
 arch/riscv/kernel/smpboot.c         |  3 +++
 arch/riscv/kernel/stacktrace.c      |  6 ++++--
 arch/riscv/kernel/syscall_table.c   |  1 +
 arch/riscv/kernel/time.c            |  1 +
 arch/riscv/kernel/traps.c           |  2 ++
 arch/riscv/kernel/vdso.c            |  3 ++-
 arch/riscv/mm/context.c             |  1 +
 arch/riscv/mm/fault.c               |  2 ++
 arch/riscv/mm/init.c                | 17 ++++++++++-------
 arch/riscv/mm/sifive_l2_cache.c     |  2 +-
 26 files changed, 111 insertions(+), 13 deletions(-)
 create mode 100644 arch/riscv/kernel/entry.h
 create mode 100644 arch/riscv/kernel/head.h

-- 
2.23.0

[PATCH 1/8] riscv: add prototypes for assembly language functions from entry.S

[Paul Walmsley]

Add prototypes for assembly language functions defined in entry.S,
and include these prototypes into C source files that call those
functions.

This patch resolves the following warnings from sparse:

arch/riscv/kernel/signal.c:32:53: warning: incorrect type in initializer (different address spaces)
arch/riscv/kernel/signal.c:45:23: warning: incorrect type in initializer (different address spaces)
arch/riscv/kernel/signal.c:59:53: warning: incorrect type in initializer (different address spaces)
arch/riscv/kernel/signal.c:69:23: warning: incorrect type in initializer (different address spaces)
arch/riscv/kernel/signal.c:89:48: warning: incorrect type in argument 2 (different address spaces)
arch/riscv/kernel/signal.c:142:45: warning: incorrect type in argument 2 (different address spaces)
arch/riscv/kernel/signal.c:295:17: warning: symbol 'do_notify_resume' was not declared. Should it be static?
arch/riscv/kernel/traps.c:91:1: warning: symbol 'do_trap_unknown' was not declared. Should it be static?
arch/riscv/kernel/traps.c:93:1: warning: symbol 'do_trap_insn_misaligned' was not declared. Should it be static?
arch/riscv/kernel/traps.c:95:1: warning: symbol 'do_trap_insn_fault' was not declared. Should it be static?
arch/riscv/kernel/traps.c:97:1: warning: symbol 'do_trap_insn_illegal' was not declared. Should it be static?
arch/riscv/kernel/traps.c:99:1: warning: symbol 'do_trap_load_misaligned' was not declared. Should it be static?
arch/riscv/kernel/traps.c:101:1: warning: symbol 'do_trap_load_fault' was not declared. Should it be static?
arch/riscv/kernel/traps.c:103:1: warning: symbol 'do_trap_store_misaligned' was not declared. Should it be static?
arch/riscv/kernel/traps.c:105:1: warning: symbol 'do_trap_store_fault' was not declared. Should it be static?
arch/riscv/kernel/traps.c:107:1: warning: symbol 'do_trap_ecall_u' was not declared. Should it be static?
arch/riscv/kernel/traps.c:109:1: warning: symbol 'do_trap_ecall_s' was not declared. Should it be static?
arch/riscv/kernel/traps.c:111:1: warning: symbol 'do_trap_ecall_m' was not declared. Should it be static?
arch/riscv/kernel/traps.c:125:17: warning: symbol 'do_trap_break' was not declared. Should it be static?
arch/riscv/kernel/traps.c:163:13: warning: symbol 'trap_init' was not declared. Should it be static?

This change should have no functional impact.

Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com>
---
 arch/riscv/kernel/entry.h  | 29 +++++++++++++++++++++++++++++
 arch/riscv/kernel/signal.c |  2 ++
 arch/riscv/kernel/traps.c  |  2 ++
 3 files changed, 33 insertions(+)
 create mode 100644 arch/riscv/kernel/entry.h

diff --git a/arch/riscv/kernel/entry.h b/arch/riscv/kernel/entry.h
new file mode 100644
index 000000000000..73bfcda993d0
--- /dev/null
+++ b/arch/riscv/kernel/entry.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2019 SiFive, Inc.
+ */
+#ifndef __ASM_ENTRY_H
+#define __ASM_ENTRY_H
+
+#include <linux/linkage.h>
+#include <linux/init.h>
+
+asmlinkage void do_trap_unknown(struct pt_regs *regs);
+asmlinkage void do_trap_insn_misaligned(struct pt_regs *regs);
+asmlinkage void do_trap_insn_fault(struct pt_regs *regs);
+asmlinkage void do_trap_insn_illegal(struct pt_regs *regs);
+asmlinkage void do_trap_load_misaligned(struct pt_regs *regs);
+asmlinkage void do_trap_load_fault(struct pt_regs *regs);
+asmlinkage void do_trap_store_misaligned(struct pt_regs *regs);
+asmlinkage void do_trap_store_fault(struct pt_regs *regs);
+asmlinkage void do_trap_ecall_u(struct pt_regs *regs);
+asmlinkage void do_trap_ecall_s(struct pt_regs *regs);
+asmlinkage void do_trap_ecall_m(struct pt_regs *regs);
+asmlinkage void do_trap_break(struct pt_regs *regs);
+
+asmlinkage void do_notify_resume(struct pt_regs *regs,
+				 unsigned long thread_info_flags);
+
+void __init trap_init(void);
+
+#endif /* __ASM__H */
diff --git a/arch/riscv/kernel/signal.c b/arch/riscv/kernel/signal.c
index b14d7647d800..85c700ad47e9 100644
--- a/arch/riscv/kernel/signal.c
+++ b/arch/riscv/kernel/signal.c
@@ -17,6 +17,8 @@
 #include <asm/switch_to.h>
 #include <asm/csr.h>
 
+#include "entry.h"
+
 #define DEBUG_SIG 0
 
 struct rt_sigframe {
diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
index 1ac75f7d0bff..eff679c3b618 100644
--- a/arch/riscv/kernel/traps.c
+++ b/arch/riscv/kernel/traps.c
@@ -19,6 +19,8 @@
 #include <asm/ptrace.h>
 #include <asm/csr.h>
 
+#include "entry.h"
+
 int show_unhandled_signals = 1;
 
 extern asmlinkage void handle_exception(void);
-- 
2.23.0

[PATCH 2/8] riscv: add prototypes for assembly language functions from head.S

[Paul Walmsley]

Add prototypes for assembly language functions defined in head.S,
and include these prototypes into C source files that call those
functions.

This patch resolves the following warnings from sparse:

arch/riscv/kernel/setup.c:39:10: warning: symbol 'hart_lottery' was not declared. Should it be static?
arch/riscv/kernel/setup.c:42:13: warning: symbol 'parse_dtb' was not declared. Should it be static?
arch/riscv/kernel/smpboot.c:33:6: warning: symbol '__cpu_up_stack_pointer' was not declared. Should it be static?
arch/riscv/kernel/smpboot.c:34:6: warning: symbol '__cpu_up_task_pointer' was not declared. Should it be static?
arch/riscv/kernel/smpboot.c:133:24: warning: symbol 'smp_callin' was not declared. Should it be static?
arch/riscv/mm/fault.c:25:17: warning: symbol 'do_page_fault' was not declared. Should it be static?

This change should have no functional impact.

Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com>
---
 arch/riscv/kernel/head.h    | 21 +++++++++++++++++++++
 arch/riscv/kernel/setup.c   |  2 ++
 arch/riscv/kernel/smpboot.c |  2 ++
 arch/riscv/mm/fault.c       |  2 ++
 arch/riscv/mm/init.c        |  2 ++
 5 files changed, 29 insertions(+)
 create mode 100644 arch/riscv/kernel/head.h

diff --git a/arch/riscv/kernel/head.h b/arch/riscv/kernel/head.h
new file mode 100644
index 000000000000..105fb0496b24
--- /dev/null
+++ b/arch/riscv/kernel/head.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2019 SiFive, Inc.
+ */
+#ifndef __ASM_HEAD_H
+#define __ASM_HEAD_H
+
+#include <linux/linkage.h>
+#include <linux/init.h>
+
+extern atomic_t hart_lottery;
+
+asmlinkage void do_page_fault(struct pt_regs *regs);
+asmlinkage void __init setup_vm(uintptr_t dtb_pa);
+
+extern void *__cpu_up_stack_pointer[];
+extern void *__cpu_up_task_pointer[];
+
+void __init parse_dtb(void);
+
+#endif /* __ASM_HEAD_H */
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index a990a6cb184f..845ae0e12115 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -24,6 +24,8 @@
 #include <asm/tlbflush.h>
 #include <asm/thread_info.h>
 
+#include "head.h"
+
 #ifdef CONFIG_DUMMY_CONSOLE
 struct screen_info screen_info = {
 	.orig_video_lines	= 30,
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 18ae6da5115e..59fa59e013d4 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -30,6 +30,8 @@
 #include <asm/sections.h>
 #include <asm/sbi.h>
 
+#include "head.h"
+
 void *__cpu_up_stack_pointer[NR_CPUS];
 void *__cpu_up_task_pointer[NR_CPUS];
 static DECLARE_COMPLETION(cpu_running);
diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
index 96add1427a75..ec15a9b15448 100644
--- a/arch/riscv/mm/fault.c
+++ b/arch/riscv/mm/fault.c
@@ -18,6 +18,8 @@
 #include <asm/ptrace.h>
 #include <asm/tlbflush.h>
 
+#include "../head.h"
+
 /*
  * This routine handles page faults.  It determines the address and the
  * problem, and then passes it off to one of the appropriate routines.
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 83f7d12042fb..fa8748a74414 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -19,6 +19,8 @@
 #include <asm/pgtable.h>
 #include <asm/io.h>
 
+#include "../kernel/head.h"
+
 unsigned long empty_zero_page[PAGE_SIZE / sizeof(unsigned long)]
 							__page_aligned_bss;
 EXPORT_SYMBOL(empty_zero_page);
-- 
2.23.0

[PATCH 3/8] riscv: init: merge split string literals in preprocessor directive

[Paul Walmsley]

sparse complains loudly when string literals associated with
preprocessor directives are split into multiple, separately quoted
strings across different lines:

arch/riscv/mm/init.c:341:9: error: Expected ; at the end of type declaration
arch/riscv/mm/init.c:341:9: error: got "not use absolute addressing."
arch/riscv/mm/init.c:358:9: error: Trying to use reserved word 'do' as identifier
arch/riscv/mm/init.c:358:9: error: Expected ; at end of declaration
[ ... ]

The compiler doesn't seem to mind the split string literal, but it's
pretty ugly to my eyes - enough to outweigh the value of the 80-column
warning from checkpatch.  Fix by concatenating the strings.

This patch should have no functional impact.

Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com>
---
 arch/riscv/mm/init.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index fa8748a74414..fe68e94ea946 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -339,8 +339,7 @@ static uintptr_t __init best_map_size(phys_addr_t base, phys_addr_t size)
  */
 
 #ifndef __riscv_cmodel_medany
-#error "setup_vm() is called from head.S before relocate so it should "
-	"not use absolute addressing."
+#error "setup_vm() is called from head.S before relocate so it should not use absolute addressing."
 #endif
 
 asmlinkage void __init setup_vm(uintptr_t dtb_pa)
-- 
2.23.0

[PATCH 4/8] riscv: ensure RISC-V C model definitions are passed to static analyzers

[Paul Walmsley]

Static analysis tools such as sparse don't set the RISC-V C model
preprocessor directives such as "__riscv_cmodel_medany", set by the C
compilers.  This causes the static analyzers to evaluate different
preprocessor paths than C compilers would.  Fix this by defining the
appropriate C model macros in the static analyzer command lines.

Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com>
---
 arch/riscv/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index f5e914210245..0247a90bd4d8 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -47,9 +47,11 @@ KBUILD_CFLAGS += -DCONFIG_PAGE_OFFSET=$(CONFIG_PAGE_OFFSET)
 
 ifeq ($(CONFIG_CMODEL_MEDLOW),y)
 	KBUILD_CFLAGS += -mcmodel=medlow
+	CHECKFLAGS += -D__riscv_cmodel_medlow
 endif
 ifeq ($(CONFIG_CMODEL_MEDANY),y)
 	KBUILD_CFLAGS += -mcmodel=medany
+	CHECKFLAGS += -D__riscv_cmodel_medany
 endif
 ifeq ($(CONFIG_MODULE_SECTIONS),y)
 	KBUILD_LDS_MODULE += $(srctree)/arch/riscv/kernel/module.lds
-- 
2.23.0

[PATCH 5/8] riscv: add missing prototypes

[Paul Walmsley]

sparse identifies these missing prototypes when building arch/riscv:

arch/riscv/kernel/cpu.c:149:29: warning: symbol 'cpuinfo_op' was not declared. Should it be static?
arch/riscv/kernel/irq.c:27:29: warning: symbol 'do_IRQ' was not declared. Should it be static?
arch/riscv/kernel/irq.c:57:13: warning: symbol 'init_IRQ' was not declared. Should it be static?
arch/riscv/kernel/syscall_table.c:15:6: warning: symbol 'sys_call_table' was not declared. Should it be static?
arch/riscv/kernel/time.c:15:13: warning: symbol 'time_init' was not declared. Should it be static?
arch/riscv/kernel/smpboot.c:135:24: warning: symbol 'smp_callin' was not declared. Should it be static?
arch/riscv/kernel/smp.c:72:5: warning: symbol 'setup_profiling_timer' was not declared. Should it be static?
arch/riscv/mm/init.c:151:7: warning: symbol 'trampoline_pg_dir' was not declared. Should it be static?
arch/riscv/mm/init.c:157:7: warning: symbol 'early_pg_dir' was not declared. Should it be static?
arch/riscv/kernel/process.c:32:6: warning: symbol 'show_regs' was not declared. Should it be static?
arch/riscv/kernel/ptrace.c:151:6: warning: symbol 'do_syscall_trace_enter' was not declared. Should it be static?
arch/riscv/kernel/ptrace.c:165:6: warning: symbol 'do_syscall_trace_exit' was not declared. Should it be static?

Fix by adding the missing prototypes to the appropriate header files.

This change should have no functional impact.

Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com>
---
 arch/riscv/include/asm/irq.h       | 3 +++
 arch/riscv/include/asm/pgtable.h   | 2 ++
 arch/riscv/include/asm/processor.h | 4 ++++
 arch/riscv/include/asm/ptrace.h    | 4 ++++
 arch/riscv/include/asm/smp.h       | 2 ++
 5 files changed, 15 insertions(+)

diff --git a/arch/riscv/include/asm/irq.h b/arch/riscv/include/asm/irq.h
index 75576424c0f7..589e2d9fb2a6 100644
--- a/arch/riscv/include/asm/irq.h
+++ b/arch/riscv/include/asm/irq.h
@@ -12,6 +12,9 @@
 void riscv_timer_interrupt(void);
 void riscv_software_interrupt(void);
 
+asmlinkage void do_IRQ(struct pt_regs *regs);
+void __init init_IRQ(void);
+
 #include <asm-generic/irq.h>
 
 #endif /* _ASM_RISCV_IRQ_H */
diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index 42292d99cc74..7fc5e4a56715 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -61,6 +61,8 @@
 
 #define PAGE_TABLE		__pgprot(_PAGE_TABLE)
 
+extern pgd_t trampoline_pg_dir[];
+extern pgd_t early_pg_dir[];
 extern pgd_t swapper_pg_dir[];
 
 /* MAP_PRIVATE permissions: xwr (copy-on-write) */
diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
index f539149d04c2..ab56435de629 100644
--- a/arch/riscv/include/asm/processor.h
+++ b/arch/riscv/include/asm/processor.h
@@ -78,6 +78,10 @@ int riscv_of_processor_hartid(struct device_node *node);
 
 extern void riscv_fill_hwcap(void);
 
+extern const struct seq_operations cpuinfo_op;
+
+void time_init(void);
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* _ASM_RISCV_PROCESSOR_H */
diff --git a/arch/riscv/include/asm/ptrace.h b/arch/riscv/include/asm/ptrace.h
index d48d1e13973c..78562f3317e6 100644
--- a/arch/riscv/include/asm/ptrace.h
+++ b/arch/riscv/include/asm/ptrace.h
@@ -101,6 +101,10 @@ static inline unsigned long regs_return_value(struct pt_regs *regs)
 	return regs->a0;
 }
 
+void do_syscall_trace_enter(struct pt_regs *regs);
+void do_syscall_trace_exit(struct pt_regs *regs);
+void show_regs(struct pt_regs *regs);
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* _ASM_RISCV_PTRACE_H */
diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h
index a83451d73a4e..d19dd2e2e1da 100644
--- a/arch/riscv/include/asm/smp.h
+++ b/arch/riscv/include/asm/smp.h
@@ -15,6 +15,8 @@
 struct seq_file;
 extern unsigned long boot_cpu_hartid;
 
+asmlinkage void __init smp_callin(void);
+
 #ifdef CONFIG_SMP
 /*
  * Mapping between linux logical cpu index and hartid.
-- 
2.23.0

[PATCH 6/8] riscv: mark some code and data as file-static

[Paul Walmsley]

Several functions and arrays which are only used in the files in which
they are declared are missing "static" qualifiers.  Warnings for these
symbols are reported by sparse:

arch/riscv/kernel/stacktrace.c:22:14: warning: symbol 'walk_stackframe' was not declared. Should it be static?
arch/riscv/kernel/vdso.c:28:18: warning: symbol 'vdso_data' was not declared. Should it be static?
arch/riscv/mm/init.c:42:6: warning: symbol 'setup_zero_page' was not declared. Should it be static?
arch/riscv/mm/init.c:152:7: warning: symbol 'fixmap_pte' was not declared. Should it be static?
arch/riscv/mm/init.c:211:7: warning: symbol 'trampoline_pmd' was not declared. Should it be static?
arch/riscv/mm/init.c:212:7: warning: symbol 'fixmap_pmd' was not declared. Should it be static?
arch/riscv/mm/init.c:219:7: warning: symbol 'early_pmd' was not declared. Should it be static?
arch/riscv/mm/sifive_l2_cache.c:145:12: warning: symbol 'sifive_l2_init' was not declared. Should it be static?

Resolve these warnings by marking them as static.

Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com>
---
 arch/riscv/kernel/stacktrace.c  |  6 ++++--
 arch/riscv/kernel/vdso.c        |  2 +-
 arch/riscv/mm/init.c            | 12 +++++++-----
 arch/riscv/mm/sifive_l2_cache.c |  2 +-
 4 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c
index 0940681d2f68..fd908baed51c 100644
--- a/arch/riscv/kernel/stacktrace.c
+++ b/arch/riscv/kernel/stacktrace.c
@@ -19,8 +19,10 @@ struct stackframe {
 	unsigned long ra;
 };
 
-void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
-			     bool (*fn)(unsigned long, void *), void *arg)
+static void notrace walk_stackframe(struct task_struct *task,
+				    struct pt_regs *regs,
+				    bool (*fn)(unsigned long, void *),
+				    void *arg)
 {
 	unsigned long fp, sp, pc;
 
diff --git a/arch/riscv/kernel/vdso.c b/arch/riscv/kernel/vdso.c
index c9c21e0d5641..e24fccab8185 100644
--- a/arch/riscv/kernel/vdso.c
+++ b/arch/riscv/kernel/vdso.c
@@ -25,7 +25,7 @@ static union {
 	struct vdso_data	data;
 	u8			page[PAGE_SIZE];
 } vdso_data_store __page_aligned_data;
-struct vdso_data *vdso_data = &vdso_data_store.data;
+static struct vdso_data *vdso_data = &vdso_data_store.data;
 
 static int __init vdso_init(void)
 {
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index fe68e94ea946..79cfb35f1e0e 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -40,7 +40,7 @@ static void __init zone_sizes_init(void)
 	free_area_init_nodes(max_zone_pfns);
 }
 
-void setup_zero_page(void)
+static void setup_zero_page(void)
 {
 	memset((void *)empty_zero_page, 0, PAGE_SIZE);
 }
@@ -150,7 +150,7 @@ EXPORT_SYMBOL(pfn_base);
 void *dtb_early_va;
 pgd_t swapper_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
 pgd_t trampoline_pg_dir[PTRS_PER_PGD] __page_aligned_bss;
-pte_t fixmap_pte[PTRS_PER_PTE] __page_aligned_bss;
+static pte_t fixmap_pte[PTRS_PER_PTE] __page_aligned_bss;
 static bool mmu_enabled;
 
 #define MAX_EARLY_MAPPING_SIZE	SZ_128M
@@ -209,15 +209,17 @@ static void __init create_pte_mapping(pte_t *ptep,
 
 #ifndef __PAGETABLE_PMD_FOLDED
 
-pmd_t trampoline_pmd[PTRS_PER_PMD] __page_aligned_bss;
-pmd_t fixmap_pmd[PTRS_PER_PMD] __page_aligned_bss;
+static pmd_t trampoline_pmd[PTRS_PER_PMD] __page_aligned_bss;
+static pmd_t fixmap_pmd[PTRS_PER_PMD] __page_aligned_bss;
 
 #if MAX_EARLY_MAPPING_SIZE < PGDIR_SIZE
 #define NUM_EARLY_PMDS		1UL
 #else
 #define NUM_EARLY_PMDS		(1UL + MAX_EARLY_MAPPING_SIZE / PGDIR_SIZE)
 #endif
-pmd_t early_pmd[PTRS_PER_PMD * NUM_EARLY_PMDS] __initdata __aligned(PAGE_SIZE);
+
+#define NUM_EARLY_PMDS_PTRS	(PTRS_PER_PMD * NUM_EARLY_PMDS)
+static pmd_t early_pmd[NUM_EARLY_PMDS_PTRS] __initdata __aligned(PAGE_SIZE);
 
 static pmd_t *__init get_pmd_virt(phys_addr_t pa)
 {
diff --git a/arch/riscv/mm/sifive_l2_cache.c b/arch/riscv/mm/sifive_l2_cache.c
index 2e637ad71c05..a9ffff3277c7 100644
--- a/arch/riscv/mm/sifive_l2_cache.c
+++ b/arch/riscv/mm/sifive_l2_cache.c
@@ -142,7 +142,7 @@ static irqreturn_t l2_int_handler(int irq, void *device)
 	return IRQ_HANDLED;
 }
 
-int __init sifive_l2_init(void)
+static int __init sifive_l2_init(void)
 {
 	struct device_node *np;
 	struct resource res;
-- 
2.23.0

[PATCH 7/8] riscv: add missing header file includes

[Paul Walmsley]

sparse identifies several missing prototypes caused by missing
preprocessor include directives:

arch/riscv/kernel/cpufeature.c:16:6: warning: symbol 'has_fpu' was not declared. Should it be static?
arch/riscv/kernel/process.c:26:6: warning: symbol 'arch_cpu_idle' was not declared. Should it be static?
arch/riscv/kernel/process.c:93:5: warning: symbol 'arch_dup_task_struct' was not declared. Should it be static?
arch/riscv/kernel/reset.c:15:6: warning: symbol 'pm_power_off' was not declared. Should it be static?
arch/riscv/kernel/syscall_table.c:15:6: warning: symbol 'sys_call_table' was not declared. Should it be static?
arch/riscv/kernel/time.c:14:13: warning: symbol 'time_init' was not declared. Should it be static?
arch/riscv/kernel/vdso.c:54:5: warning: symbol 'arch_setup_additional_pages' was not declared. Should it be static?
arch/riscv/kernel/smpboot.c:134:24: warning: symbol 'smp_callin' was not declared. Should it be static?
arch/riscv/kernel/smp.c:64:6: warning: symbol 'arch_match_cpu_phys_id' was not declared. Should it be static?
arch/riscv/kernel/smp.c:70:5: warning: symbol 'setup_profiling_timer' was not declared. Should it be static?
arch/riscv/kernel/module-sections.c:89:5: warning: symbol 'module_frob_arch_sections' was not declared. Should it be static?
arch/riscv/mm/context.c:42:6: warning: symbol 'switch_mm' was not declared. Should it be static?

Fix by including the appropriate header files in the appropriate
source files.

This patch should have no functional impact.

Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com>
---
 arch/riscv/include/asm/irq.h        | 3 +++
 arch/riscv/include/asm/switch_to.h  | 1 +
 arch/riscv/kernel/cpufeature.c      | 1 +
 arch/riscv/kernel/module-sections.c | 1 +
 arch/riscv/kernel/process.c         | 2 ++
 arch/riscv/kernel/reset.c           | 1 +
 arch/riscv/kernel/smp.c             | 2 ++
 arch/riscv/kernel/smpboot.c         | 1 +
 arch/riscv/kernel/syscall_table.c   | 1 +
 arch/riscv/kernel/time.c            | 1 +
 arch/riscv/kernel/vdso.c            | 1 +
 arch/riscv/mm/context.c             | 1 +
 12 files changed, 16 insertions(+)

diff --git a/arch/riscv/include/asm/irq.h b/arch/riscv/include/asm/irq.h
index 589e2d9fb2a6..f0e9df6e6049 100644
--- a/arch/riscv/include/asm/irq.h
+++ b/arch/riscv/include/asm/irq.h
@@ -7,6 +7,9 @@
 #ifndef _ASM_RISCV_IRQ_H
 #define _ASM_RISCV_IRQ_H
 
+#include <linux/interrupt.h>
+#include <linux/linkage.h>
+
 #define NR_IRQS         0
 
 void riscv_timer_interrupt(void);
diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h
index f0227bdce0f0..ee4f0ac62c9d 100644
--- a/arch/riscv/include/asm/switch_to.h
+++ b/arch/riscv/include/asm/switch_to.h
@@ -6,6 +6,7 @@
 #ifndef _ASM_RISCV_SWITCH_TO_H
 #define _ASM_RISCV_SWITCH_TO_H
 
+#include <linux/sched/task_stack.h>
 #include <asm/processor.h>
 #include <asm/ptrace.h>
 #include <asm/csr.h>
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index b1ade9a49347..a5ad00043104 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -10,6 +10,7 @@
 #include <asm/processor.h>
 #include <asm/hwcap.h>
 #include <asm/smp.h>
+#include <asm/switch_to.h>
 
 unsigned long elf_hwcap __read_mostly;
 #ifdef CONFIG_FPU
diff --git a/arch/riscv/kernel/module-sections.c b/arch/riscv/kernel/module-sections.c
index c9ae48333114..e264e59e596e 100644
--- a/arch/riscv/kernel/module-sections.c
+++ b/arch/riscv/kernel/module-sections.c
@@ -8,6 +8,7 @@
 #include <linux/elf.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/moduleloader.h>
 
 unsigned long module_emit_got_entry(struct module *mod, unsigned long val)
 {
diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
index fb3a082362eb..85e3c39bb60b 100644
--- a/arch/riscv/kernel/process.c
+++ b/arch/riscv/kernel/process.c
@@ -7,6 +7,7 @@
  * Copyright (C) 2017 SiFive
  */
 
+#include <linux/cpu.h>
 #include <linux/kernel.h>
 #include <linux/sched.h>
 #include <linux/sched/task_stack.h>
@@ -19,6 +20,7 @@
 #include <asm/csr.h>
 #include <asm/string.h>
 #include <asm/switch_to.h>
+#include <asm/thread_info.h>
 
 extern asmlinkage void ret_from_fork(void);
 extern asmlinkage void ret_from_kernel_thread(void);
diff --git a/arch/riscv/kernel/reset.c b/arch/riscv/kernel/reset.c
index d0fe623bfb8f..aa56bb135ec4 100644
--- a/arch/riscv/kernel/reset.c
+++ b/arch/riscv/kernel/reset.c
@@ -4,6 +4,7 @@
  */
 
 #include <linux/reboot.h>
+#include <linux/pm.h>
 #include <asm/sbi.h>
 
 static void default_power_off(void)
diff --git a/arch/riscv/kernel/smp.c b/arch/riscv/kernel/smp.c
index b18cd6c8e8fb..5c9ec78422c2 100644
--- a/arch/riscv/kernel/smp.c
+++ b/arch/riscv/kernel/smp.c
@@ -8,7 +8,9 @@
  * Copyright (C) 2017 SiFive
  */
 
+#include <linux/cpu.h>
 #include <linux/interrupt.h>
+#include <linux/profile.h>
 #include <linux/smp.h>
 #include <linux/sched.h>
 #include <linux/seq_file.h>
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 59fa59e013d4..ec0be2f6a2e8 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -29,6 +29,7 @@
 #include <asm/tlbflush.h>
 #include <asm/sections.h>
 #include <asm/sbi.h>
+#include <asm/smp.h>
 
 #include "head.h"
 
diff --git a/arch/riscv/kernel/syscall_table.c b/arch/riscv/kernel/syscall_table.c
index e5dd52d8f633..f1ead9df96ca 100644
--- a/arch/riscv/kernel/syscall_table.c
+++ b/arch/riscv/kernel/syscall_table.c
@@ -8,6 +8,7 @@
 #include <linux/syscalls.h>
 #include <asm-generic/syscalls.h>
 #include <asm/vdso.h>
+#include <asm/syscall.h>
 
 #undef __SYSCALL
 #define __SYSCALL(nr, call)	[nr] = (call),
diff --git a/arch/riscv/kernel/time.c b/arch/riscv/kernel/time.c
index 9dd1f2e64db1..6a53c02e9c73 100644
--- a/arch/riscv/kernel/time.c
+++ b/arch/riscv/kernel/time.c
@@ -7,6 +7,7 @@
 #include <linux/clocksource.h>
 #include <linux/delay.h>
 #include <asm/sbi.h>
+#include <asm/processor.h>
 
 unsigned long riscv_timebase;
 EXPORT_SYMBOL_GPL(riscv_timebase);
diff --git a/arch/riscv/kernel/vdso.c b/arch/riscv/kernel/vdso.c
index e24fccab8185..484d95a70907 100644
--- a/arch/riscv/kernel/vdso.c
+++ b/arch/riscv/kernel/vdso.c
@@ -6,6 +6,7 @@
  * Copyright (C) 2015 Regents of the University of California
  */
 
+#include <linux/elf.h>
 #include <linux/mm.h>
 #include <linux/slab.h>
 #include <linux/binfmts.h>
diff --git a/arch/riscv/mm/context.c b/arch/riscv/mm/context.c
index beeb5d7f92ea..ca66d44156b6 100644
--- a/arch/riscv/mm/context.c
+++ b/arch/riscv/mm/context.c
@@ -7,6 +7,7 @@
 #include <linux/mm.h>
 #include <asm/tlbflush.h>
 #include <asm/cacheflush.h>
+#include <asm/mmu_context.h>
 
 /*
  * When necessary, performs a deferred icache flush for the given MM context,
-- 
2.23.0

[PATCH 8/8] riscv: fp: add missing __user pointer annotations

[Paul Walmsley]

The __user annotations were removed from the {save,restore}_fp_state()
function signatures by commit 007f5c358957 ("Refactor FPU code in
signal setup/return procedures"), but should be present, and sparse
warns when they are not applied.  Add them back in.

This change should have no functional impact.

Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com>
Fixes: 007f5c358957 ("Refactor FPU code in signal setup/return procedures")
Cc: Alan Kao <alankao@andestech.com>
---
 arch/riscv/kernel/signal.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/kernel/signal.c b/arch/riscv/kernel/signal.c
index 85c700ad47e9..9437167f463e 100644
--- a/arch/riscv/kernel/signal.c
+++ b/arch/riscv/kernel/signal.c
@@ -28,7 +28,7 @@ struct rt_sigframe {
 
 #ifdef CONFIG_FPU
 static long restore_fp_state(struct pt_regs *regs,
-			     union __riscv_fp_state *sc_fpregs)
+			     union __riscv_fp_state __user *sc_fpregs)
 {
 	long err;
 	struct __riscv_d_ext_state __user *state = &sc_fpregs->d;
@@ -55,7 +55,7 @@ static long restore_fp_state(struct pt_regs *regs,
 }
 
 static long save_fp_state(struct pt_regs *regs,
-			  union __riscv_fp_state *sc_fpregs)
+			  union __riscv_fp_state __user *sc_fpregs)
 {
 	long err;
 	struct __riscv_d_ext_state __user *state = &sc_fpregs->d;
-- 
2.23.0

Re: [PATCH 2/8] riscv: add prototypes for assembly language functions from head.S

[Paul Walmsley]

On Thu, 17 Oct 2019, Paul Walmsley wrote:

> Add prototypes for assembly language functions defined in head.S,
> and include these prototypes into C source files that call those
> functions.

[ ... ]

> diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
> index 96add1427a75..ec15a9b15448 100644
> --- a/arch/riscv/mm/fault.c
> +++ b/arch/riscv/mm/fault.c
> @@ -18,6 +18,8 @@
>  #include <asm/ptrace.h>
>  #include <asm/tlbflush.h>
>  
> +#include "../head.h"
> +

"../kernel/head.h", rather.


- Paul

Re: [PATCH 0/8] riscv: resolve most warnings from sparse

[Anup Patel]

On Fri, Oct 18, 2019 at 6:19 AM Paul Walmsley <paul.walmsley@sifive.com> wrote:
>
> Resolve most warnings from the 'sparse' static analysis tool for the
> arch/riscv codebase.  This makes life easier for us as maintainers,
> and makes it easier for developers to use static analysis tools on
> their own changes.
>
> This patch series incorporates some changes based on feedback from
> Christoph Hellwig <hch@lst.de>.
>
> Applies on the current riscv fixes branch that is based on v5.4-rc3.

This series certainly conflict's with Christoph's NOMMU series so
please rebase it on NOMMU series.

Regards,
Anup

>
>
> - Paul
>
>
> Paul Walmsley (8):
>   riscv: add prototypes for assembly language functions from entry.S
>   riscv: add prototypes for assembly language functions from head.S
>   riscv: init: merge split string literals in preprocessor directive
>   riscv: ensure RISC-V C model definitions are passed to static
>     analyzers
>   riscv: add missing prototypes
>   riscv: mark some code and data as file-static
>   riscv: add missing header file includes
>   riscv: fp: add missing __user pointer annotations
>
> Kernel object size difference:
>    text    data     bss     dec     hex filename
> 6664206 2136568  312608 9113382  8b0f26 vmlinux.orig
> 6664186 2136552  312608 9113346  8b0f02 vmlinux.patched
>
>  arch/riscv/Makefile                 |  2 ++
>  arch/riscv/include/asm/irq.h        |  6 ++++++
>  arch/riscv/include/asm/pgtable.h    |  2 ++
>  arch/riscv/include/asm/processor.h  |  4 ++++
>  arch/riscv/include/asm/ptrace.h     |  4 ++++
>  arch/riscv/include/asm/smp.h        |  2 ++
>  arch/riscv/include/asm/switch_to.h  |  1 +
>  arch/riscv/kernel/cpufeature.c      |  1 +
>  arch/riscv/kernel/entry.h           | 29 +++++++++++++++++++++++++++++
>  arch/riscv/kernel/head.h            | 21 +++++++++++++++++++++
>  arch/riscv/kernel/module-sections.c |  1 +
>  arch/riscv/kernel/process.c         |  2 ++
>  arch/riscv/kernel/reset.c           |  1 +
>  arch/riscv/kernel/setup.c           |  2 ++
>  arch/riscv/kernel/signal.c          |  6 ++++--
>  arch/riscv/kernel/smp.c             |  2 ++
>  arch/riscv/kernel/smpboot.c         |  3 +++
>  arch/riscv/kernel/stacktrace.c      |  6 ++++--
>  arch/riscv/kernel/syscall_table.c   |  1 +
>  arch/riscv/kernel/time.c            |  1 +
>  arch/riscv/kernel/traps.c           |  2 ++
>  arch/riscv/kernel/vdso.c            |  3 ++-
>  arch/riscv/mm/context.c             |  1 +
>  arch/riscv/mm/fault.c               |  2 ++
>  arch/riscv/mm/init.c                | 17 ++++++++++-------
>  arch/riscv/mm/sifive_l2_cache.c     |  2 +-
>  26 files changed, 111 insertions(+), 13 deletions(-)
>  create mode 100644 arch/riscv/kernel/entry.h
>  create mode 100644 arch/riscv/kernel/head.h
>
> --
> 2.23.0
>

Re: [PATCH 3/8] riscv: init: merge split string literals in preprocessor directive

[Luc Van Oostenryck]

On Thu, Oct 17, 2019 at 05:49:24PM -0700, Paul Walmsley wrote:
> sparse complains loudly when string literals associated with
> preprocessor directives are split into multiple, separately quoted
> strings across different lines:

...
 
>  #ifndef __riscv_cmodel_medany
> -#error "setup_vm() is called from head.S before relocate so it should "
> -	"not use absolute addressing."
> +#error "setup_vm() is called from head.S before relocate so it should not use absolute addressing."
>  #endif

Using a blacslash should do the trick :
	#error "blablablablablablablablablablablabla" \
			"and blablabla again"
Or if need I cn fix Sparse if needed and desiable.
 
Best regards
-- Luc Van Oostenryck

Re: [PATCH 4/8] riscv: ensure RISC-V C model definitions are passed to static analyzers

[Luc Van Oostenryck]

On Thu, Oct 17, 2019 at 05:49:25PM -0700, Paul Walmsley wrote:
> Static analysis tools such as sparse don't set the RISC-V C model
> preprocessor directives such as "__riscv_cmodel_medany", set by the C
> compilers.  This causes the static analyzers to evaluate different
> preprocessor paths than C compilers would.  Fix this by defining the
> appropriate C model macros in the static analyzer command lines.
> 
> Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com>
> ---
>  arch/riscv/Makefile | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> index f5e914210245..0247a90bd4d8 100644
> --- a/arch/riscv/Makefile
> +++ b/arch/riscv/Makefile
> @@ -47,9 +47,11 @@ KBUILD_CFLAGS += -DCONFIG_PAGE_OFFSET=$(CONFIG_PAGE_OFFSET)
>  
>  ifeq ($(CONFIG_CMODEL_MEDLOW),y)
>  	KBUILD_CFLAGS += -mcmodel=medlow
> +	CHECKFLAGS += -D__riscv_cmodel_medlow
>  endif
>  ifeq ($(CONFIG_CMODEL_MEDANY),y)
>  	KBUILD_CFLAGS += -mcmodel=medany
> +	CHECKFLAGS += -D__riscv_cmodel_medany

I can teach sparse about this in the following days.

-- Luc Van Oostenryck

Re: [PATCH 5/8] riscv: add missing prototypes

[Luc Van Oostenryck]

On Thu, Oct 17, 2019 at 05:49:26PM -0700, Paul Walmsley wrote:
> sparse identifies these missing prototypes when building arch/riscv:
> 
> arch/riscv/kernel/cpu.c:149:29: warning: symbol 'cpuinfo_op' was not declared. Should it be static?
> arch/riscv/kernel/irq.c:27:29: warning: symbol 'do_IRQ' was not declared. Should it be static?
> arch/riscv/kernel/irq.c:57:13: warning: symbol 'init_IRQ' was not declared. Should it be static?
> arch/riscv/kernel/syscall_table.c:15:6: warning: symbol 'sys_call_table' was not declared. Should it be static?
> arch/riscv/kernel/time.c:15:13: warning: symbol 'time_init' was not declared. Should it be static?
> arch/riscv/kernel/smpboot.c:135:24: warning: symbol 'smp_callin' was not declared. Should it be static?
> arch/riscv/kernel/smp.c:72:5: warning: symbol 'setup_profiling_timer' was not declared. Should it be static?
> arch/riscv/mm/init.c:151:7: warning: symbol 'trampoline_pg_dir' was not declared. Should it be static?
> arch/riscv/mm/init.c:157:7: warning: symbol 'early_pg_dir' was not declared. Should it be static?
> arch/riscv/kernel/process.c:32:6: warning: symbol 'show_regs' was not declared. Should it be static?
> arch/riscv/kernel/ptrace.c:151:6: warning: symbol 'do_syscall_trace_enter' was not declared. Should it be static?
> arch/riscv/kernel/ptrace.c:165:6: warning: symbol 'do_syscall_trace_exit' was not declared. Should it be static?
> 
> Fix by adding the missing prototypes to the appropriate header files.

For functions defined in C but used ony in assembly, you can also simply mark
them as '__visible'  (aka __attribute__((exrernally_visible)) ).
 
Best regards,
-- Luc

Re: [PATCH 3/8] riscv: init: merge split string literals in preprocessor directive

[Paul Walmsley]

On Fri, 18 Oct 2019, Luc Van Oostenryck wrote:

> On Thu, Oct 17, 2019 at 05:49:24PM -0700, Paul Walmsley wrote:
> > sparse complains loudly when string literals associated with
> > preprocessor directives are split into multiple, separately quoted
> > strings across different lines:
> 
> ...
>  
> >  #ifndef __riscv_cmodel_medany
> > -#error "setup_vm() is called from head.S before relocate so it should "
> > -	"not use absolute addressing."
> > +#error "setup_vm() is called from head.S before relocate so it should not use absolute addressing."
> >  #endif
> 
> Using a blacslash should do the trick :
> 	#error "blablablablablablablablablablablabla" \
> 			"and blablabla again"
> Or if need I cn fix Sparse if needed and desiable.

Thanks for the kind offer!

The backslashless syntax is pretty horrible to my eyes.  As far as I can 
tell from a brief glance, the instance fixed by this patch was the only 
instance of its kind in the kernel.  The existing kernel precedents appear 
to be to simply use a single long line.  Example:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/compiler-gcc.h#n3

So, from a kernel point of view, we should just fix this specific 
instance.  It doesn't seem worth changing sparse for such a rare case.

On the other hand, gcc seems to support the non-backslashed syntax.  So if 
the intention is for sparse to follow the gcc practice, and to be used 
beyond the kernel, maybe it's worth aligning sparse to gcc?  Only if 
you're bored, I suppose...


- Paul

Re: [PATCH 4/8] riscv: ensure RISC-V C model definitions are passed to static analyzers

[Paul Walmsley]

On Fri, 18 Oct 2019, Luc Van Oostenryck wrote:

> On Thu, Oct 17, 2019 at 05:49:25PM -0700, Paul Walmsley wrote:
> > Static analysis tools such as sparse don't set the RISC-V C model
> > preprocessor directives such as "__riscv_cmodel_medany", set by the C
> > compilers.  This causes the static analyzers to evaluate different
> > preprocessor paths than C compilers would.  Fix this by defining the
> > appropriate C model macros in the static analyzer command lines.
> > 
> > Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com>
> > ---
> >  arch/riscv/Makefile | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> > index f5e914210245..0247a90bd4d8 100644
> > --- a/arch/riscv/Makefile
> > +++ b/arch/riscv/Makefile
> > @@ -47,9 +47,11 @@ KBUILD_CFLAGS += -DCONFIG_PAGE_OFFSET=$(CONFIG_PAGE_OFFSET)
> >  
> >  ifeq ($(CONFIG_CMODEL_MEDLOW),y)
> >  	KBUILD_CFLAGS += -mcmodel=medlow
> > +	CHECKFLAGS += -D__riscv_cmodel_medlow
> >  endif
> >  ifeq ($(CONFIG_CMODEL_MEDANY),y)
> >  	KBUILD_CFLAGS += -mcmodel=medany
> > +	CHECKFLAGS += -D__riscv_cmodel_medany
> 
> I can teach sparse about this in the following days.

That would be great.  Would you be willing to follow up with me via E-mail 
or mailing list post when it's fixed?  If so, then in the meantime, I'll 
just drop this patch.


- Paul

Re: [PATCH 4/8] riscv: ensure RISC-V C model definitions are passed to static analyzers

[Luc Van Oostenryck]

On Thu, Oct 17, 2019 at 09:39:29PM -0700, Paul Walmsley wrote:
> On Fri, 18 Oct 2019, Luc Van Oostenryck wrote:
> > On Thu, Oct 17, 2019 at 05:49:25PM -0700, Paul Walmsley wrote:
....
> > >  ifeq ($(CONFIG_CMODEL_MEDANY),y)
> > >  	KBUILD_CFLAGS += -mcmodel=medany
> > > +	CHECKFLAGS += -D__riscv_cmodel_medany
> > 
> > I can teach sparse about this in the following days.
> 
> That would be great.  Would you be willing to follow up with me via E-mail 
> or mailing list post when it's fixed?  If so, then in the meantime, I'll 
> just drop this patch.

For sure.
-- Luc

Re: [PATCH 3/8] riscv: init: merge split string literals in preprocessor directive

[Luc Van Oostenryck]

On Thu, Oct 17, 2019 at 09:38:18PM -0700, Paul Walmsley wrote:
> On Fri, 18 Oct 2019, Luc Van Oostenryck wrote:
> 
> > On Thu, Oct 17, 2019 at 05:49:24PM -0700, Paul Walmsley wrote:
> > > sparse complains loudly when string literals associated with
> > > preprocessor directives are split into multiple, separately quoted
> > > strings across different lines:
> > 
> > ...
> >  
> > >  #ifndef __riscv_cmodel_medany
> > > -#error "setup_vm() is called from head.S before relocate so it should "
> > > -	"not use absolute addressing."
> > > +#error "setup_vm() is called from head.S before relocate so it should not use absolute addressing."
> > >  #endif
> > 
> > Using a blacslash should do the trick :
> > 	#error "blablablablablablablablablablablabla" \
> > 			"and blablabla again"
> > Or if need I cn fix Sparse if needed and desiable.
> 
> Thanks for the kind offer!
> 
> The backslashless syntax is pretty horrible to my eyes.  As far as I can 
> tell from a brief glance, the instance fixed by this patch was the only 
> instance of its kind in the kernel.  The existing kernel precedents appear 
> to be to simply use a single long line.  Example:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/compiler-gcc.h#n3
> 
> So, from a kernel point of view, we should just fix this specific 
> instance.  It doesn't seem worth changing sparse for such a rare case.
> 
> On the other hand, gcc seems to support the non-backslashed syntax.  So if 
> the intention is for sparse to follow the gcc practice, and to be used 
> beyond the kernel, maybe it's worth aligning sparse to gcc?  Only if 
> you're bored, I suppose...

I'll first see what is acceptable for the point of view of the standard
(and maybe some justifications from GCC).

-- Luc 

Re: [PATCH 3/8] riscv: init: merge split string literals in preprocessor directive

[Luc Van Oostenryck]

On Thu, Oct 17, 2019 at 09:38:18PM -0700, Paul Walmsley wrote:
> On Fri, 18 Oct 2019, Luc Van Oostenryck wrote:
> 
> > On Thu, Oct 17, 2019 at 05:49:24PM -0700, Paul Walmsley wrote:
> > > sparse complains loudly when string literals associated with
> > > preprocessor directives are split into multiple, separately quoted
> > > strings across different lines:
> > 
> > ...
> >  
> > >  #ifndef __riscv_cmodel_medany
> > > -#error "setup_vm() is called from head.S before relocate so it should "
> > > -	"not use absolute addressing."
> > > +#error "setup_vm() is called from head.S before relocate so it should not use absolute addressing."
> > >  #endif

...
 
> On the other hand, gcc seems to support the non-backslashed syntax.  So if 
> the intention is for sparse to follow the gcc practice, and to be used 
> beyond the kernel, maybe it's worth aligning sparse to gcc?  Only if 
> you're bored, I suppose...

I quickly checked and gcc also complain about the second line:
	$ cat y.c 
#ifndef __riscv_cmodel_medany
#error "setup_vm() is called from head.S before relocate so it should "
       "not use absolute addressing."
#endif

	$ gcc -c y.c
y.c:2:2: error: #error "setup_vm() is called from head.S before relocate so it should "
 #error "setup_vm() is called from head.S before relocate so it should "
  ^~~~~
y.c:3:8: error: expected identifier or '(' before string constant
        "not use absolute addressing."
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

So it seems that gcc doesn't join these lines.
Fell free to add my:
Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

-- Luc

Re: [PATCH 5/8] riscv: add missing prototypes

[Paul Walmsley]

On Fri, 18 Oct 2019, Luc Van Oostenryck wrote:

> On Thu, Oct 17, 2019 at 05:49:26PM -0700, Paul Walmsley wrote:
> > sparse identifies these missing prototypes when building arch/riscv:
> > 
> > arch/riscv/kernel/cpu.c:149:29: warning: symbol 'cpuinfo_op' was not declared. Should it be static?
> > arch/riscv/kernel/irq.c:27:29: warning: symbol 'do_IRQ' was not declared. Should it be static?
> > arch/riscv/kernel/irq.c:57:13: warning: symbol 'init_IRQ' was not declared. Should it be static?
> > arch/riscv/kernel/syscall_table.c:15:6: warning: symbol 'sys_call_table' was not declared. Should it be static?
> > arch/riscv/kernel/time.c:15:13: warning: symbol 'time_init' was not declared. Should it be static?
> > arch/riscv/kernel/smpboot.c:135:24: warning: symbol 'smp_callin' was not declared. Should it be static?
> > arch/riscv/kernel/smp.c:72:5: warning: symbol 'setup_profiling_timer' was not declared. Should it be static?
> > arch/riscv/mm/init.c:151:7: warning: symbol 'trampoline_pg_dir' was not declared. Should it be static?
> > arch/riscv/mm/init.c:157:7: warning: symbol 'early_pg_dir' was not declared. Should it be static?
> > arch/riscv/kernel/process.c:32:6: warning: symbol 'show_regs' was not declared. Should it be static?
> > arch/riscv/kernel/ptrace.c:151:6: warning: symbol 'do_syscall_trace_enter' was not declared. Should it be static?
> > arch/riscv/kernel/ptrace.c:165:6: warning: symbol 'do_syscall_trace_exit' was not declared. Should it be static?
> > 
> > Fix by adding the missing prototypes to the appropriate header files.
> 
> For functions defined in C but used ony in assembly, you can also simply mark
> them as '__visible'  (aka __attribute__((exrernally_visible)) ).

Thanks, I'll take this suggestion.


- Paul

Re: [PATCH 3/8] riscv: init: merge split string literals in preprocessor directive

[Paul Walmsley]

On Fri, 18 Oct 2019, Luc Van Oostenryck wrote:

> I quickly checked and gcc also complain about the second line:
> 	$ cat y.c 
> #ifndef __riscv_cmodel_medany
> #error "setup_vm() is called from head.S before relocate so it should "
>        "not use absolute addressing."
> #endif
> 
> 	$ gcc -c y.c
> y.c:2:2: error: #error "setup_vm() is called from head.S before relocate so it should "
>  #error "setup_vm() is called from head.S before relocate so it should "
>   ^~~~~
> y.c:3:8: error: expected identifier or '(' before string constant
>         "not use absolute addressing."
>         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> So it seems that gcc doesn't join these lines.

I guess that's what I get for assuming that the original code was tested.  
Thanks for doing that, and sorry for the noise.

> Fell free to add my:
> Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

Done.


- Paul

Re: [PATCH 4/8] riscv: ensure RISC-V C model definitions are passed to static analyzers

[Palmer Dabbelt]

On Thu, 17 Oct 2019 21:39:29 PDT (-0700), Paul Walmsley wrote:
> On Fri, 18 Oct 2019, Luc Van Oostenryck wrote:
>
>> On Thu, Oct 17, 2019 at 05:49:25PM -0700, Paul Walmsley wrote:
>> > Static analysis tools such as sparse don't set the RISC-V C model
>> > preprocessor directives such as "__riscv_cmodel_medany", set by the C
>> > compilers.  This causes the static analyzers to evaluate different
>> > preprocessor paths than C compilers would.  Fix this by defining the
>> > appropriate C model macros in the static analyzer command lines.
>> >
>> > Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com>
>> > ---
>> >  arch/riscv/Makefile | 2 ++
>> >  1 file changed, 2 insertions(+)
>> >
>> > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
>> > index f5e914210245..0247a90bd4d8 100644
>> > --- a/arch/riscv/Makefile
>> > +++ b/arch/riscv/Makefile
>> > @@ -47,9 +47,11 @@ KBUILD_CFLAGS += -DCONFIG_PAGE_OFFSET=$(CONFIG_PAGE_OFFSET)
>> >
>> >  ifeq ($(CONFIG_CMODEL_MEDLOW),y)
>> >  	KBUILD_CFLAGS += -mcmodel=medlow
>> > +	CHECKFLAGS += -D__riscv_cmodel_medlow
>> >  endif
>> >  ifeq ($(CONFIG_CMODEL_MEDANY),y)
>> >  	KBUILD_CFLAGS += -mcmodel=medany
>> > +	CHECKFLAGS += -D__riscv_cmodel_medany
>>
>> I can teach sparse about this in the following days.
>
> That would be great.  Would you be willing to follow up with me via E-mail
> or mailing list post when it's fixed?  If so, then in the meantime, I'll
> just drop this patch.

It's probably worth going through all our argument-dependent builtin 
definitions at the same time.  They're generated by riscv_cpu_cpp_builtins(): 
https://github.com/gcc-mirror/gcc/blob/master/gcc/config/riscv/riscv-c.c#L35 .

>
>
> - Paul
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 4/8] riscv: ensure RISC-V C model definitions are passed to static analyzers

[Luc Van Oostenryck]

On Tue, Oct 22, 2019 at 08:09:59AM -0700, Palmer Dabbelt wrote:
> 
> It's probably worth going through all our argument-dependent builtin
> definitions at the same time.  They're generated by
> riscv_cpu_cpp_builtins():
> https://github.com/gcc-mirror/gcc/blob/master/gcc/config/riscv/riscv-c.c#L35

Yes, I agree.
However, these are higly dependent on parsing -march and this is quite
arch-specific which sparse is not really needed for.

I'll add some infrastructure for this in the followings weeks.

-- Luc