[PATCH 00/21] powerpc/gamecube: make W=1 compilation errors free

[PATCH 00/21] powerpc/gamecube: make W=1 compilation errors free

[Mathieu Malaterre]

I started working on warnings treated as error on ppc32. As a first step,
here is a set of patches to make gamecube error free when using W=1:

$ make ARCH=powerpc gamecube_defconfig
$ make -j8 ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- W=1

Using:

$ powerpc-linux-gnu-gcc --version
powerpc-linux-gnu-gcc (Debian 6.3.0-18) 6.3.0 20170516


Mathieu Malaterre (21):
  powerpc: Remove warning on array size when empty
  powerpc: Move the inline keyword at the beginning of function
    declaration
  powerpc: Mark the variable earlycon_acpi_spcr_enable maybe_unused
  powerpc: Mark both tmp variables as unused
  powerpc: Avoid comparison of unsigned long >= 0 in pfn_valid
  powerpc: Avoid comparison of unsigned long >= 0 in __access_ok
  powerpc: Make functions flipper_pic_init & ug_udbg_putc static
  powerpc: Make function __giveup_fpu static
  powerpc: Make function save_all static
  powerpc: Add missing prototype for slb_miss_bad_addr
  powerpc: Add missing prototype for hdec_interrupt
  powerpc: Add missing prototype for time_init
  powerpc: Add missing prototype for arch_dup_task_struct
  powerpc: Add missing prototype for arch_irq_work_raise
  powerpc: Add missing prototype for MMU_setup
  powerpc: Add missing prototype for init_IRQ
  powerpc: Add missing prototype for sys_debug_setcontext
  powerpc: Add missing prototypes for sys_sigreturn & sys_rt_sigreturn
  powerpc: Add missing prototypes for hw_breakpoint_handler &
    arch_unregister_hw_breakpoint
  powerpc: Add missing prototypes for ppc_select & ppc_fadvise64_64
  powerpc: Add missing prototypes in setup_32.c

 arch/powerpc/include/asm/asm-prototypes.h          | 9 +++++++++
 arch/powerpc/include/asm/hw_breakpoint.h           | 2 ++
 arch/powerpc/include/asm/irq.h                     | 1 +
 arch/powerpc/include/asm/irq_work.h                | 1 +
 arch/powerpc/include/asm/page.h                    | 3 ++-
 arch/powerpc/include/asm/thread_info.h             | 1 +
 arch/powerpc/include/asm/time.h                    | 2 ++
 arch/powerpc/include/asm/uaccess.h                 | 2 +-
 arch/powerpc/kernel/process.c                      | 4 ++--
 arch/powerpc/kernel/prom.c                         | 3 +--
 arch/powerpc/kernel/setup.h                        | 5 +++++
 arch/powerpc/kernel/setup_32.c                     | 2 ++
 arch/powerpc/kernel/signal.h                       | 5 +++++
 arch/powerpc/kernel/signal_32.c                    | 4 ++--
 arch/powerpc/lib/sstep.c                           | 4 ++--
 arch/powerpc/mm/init_32.c                          | 1 +
 arch/powerpc/platforms/embedded6xx/flipper-pic.c   | 2 +-
 arch/powerpc/platforms/embedded6xx/usbgecko_udbg.c | 2 +-
 include/linux/serial_core.h                        | 1 +
 19 files changed, 42 insertions(+), 12 deletions(-)

-- 
2.11.0

[PATCH 01/21] powerpc: Remove warning on array size when empty

[Mathieu Malaterre]

When neither CONFIG_ALTIVEC, nor CONFIG_VSX or CONFIG_PPC64 is defined, the
array feature_properties is defined as an empty array, which in turn
triggers the following warning (treated as error on W=1):

  CC      arch/powerpc/kernel/prom.o
arch/powerpc/kernel/prom.c: In function ‘check_cpu_feature_properties’:
arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
  for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
                ^
cc1: all warnings being treated as errors

Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/powerpc/kernel/prom.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 4dffef947b8a..6e8e4122820e 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -291,11 +291,10 @@ static inline void identical_pvr_fixup(unsigned long node)
 
 static void __init check_cpu_feature_properties(unsigned long node)
 {
-	unsigned long i;
 	struct feature_property *fp = feature_properties;
 	const __be32 *prop;
 
-	for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
+	for (; fp != feature_properties + ARRAY_SIZE(feature_properties); ++fp) {
 		prop = of_get_flat_dt_prop(node, fp->name, NULL);
 		if (prop && be32_to_cpup(prop) >= fp->min_value) {
 			cur_cpu_spec->cpu_features |= fp->cpu_feature;
-- 
2.11.0

[PATCH 02/21] powerpc: Move the inline keyword at the beginning of function declaration

[Mathieu Malaterre]

The inline keyword was not at the beginning of the function declaration.
Fix the following warning (treated as error in W=1)

  CC      kernel/sys.o
arch/powerpc/lib/sstep.c:283:1: error: ‘inline’ is not at beginning of declaration [-Werror=old-style-declaration]
 static int nokprobe_inline copy_mem_in(u8 *dest, unsigned long ea, int nb,
 ^~~~~~
arch/powerpc/lib/sstep.c:388:1: error: ‘inline’ is not at beginning of declaration [-Werror=old-style-declaration]
 static int nokprobe_inline copy_mem_out(u8 *dest, unsigned long ea, int nb,
 ^~~~~~
  CC      arch/powerpc/kernel/setup_32.o

Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/powerpc/lib/sstep.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 70274b7b4773..34d68f1b1b40 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -280,7 +280,7 @@ static nokprobe_inline int read_mem_aligned(unsigned long *dest,
  * Copy from userspace to a buffer, using the largest possible
  * aligned accesses, up to sizeof(long).
  */
-static int nokprobe_inline copy_mem_in(u8 *dest, unsigned long ea, int nb,
+static nokprobe_inline int copy_mem_in(u8 *dest, unsigned long ea, int nb,
 				       struct pt_regs *regs)
 {
 	int err = 0;
@@ -385,7 +385,7 @@ static nokprobe_inline int write_mem_aligned(unsigned long val,
  * Copy from a buffer to userspace, using the largest possible
  * aligned accesses, up to sizeof(long).
  */
-static int nokprobe_inline copy_mem_out(u8 *dest, unsigned long ea, int nb,
+static nokprobe_inline int copy_mem_out(u8 *dest, unsigned long ea, int nb,
 					struct pt_regs *regs)
 {
 	int err = 0;
-- 
2.11.0

[PATCH 03/21] powerpc: Mark the variable earlycon_acpi_spcr_enable maybe_unused

[Mathieu Malaterre]

Re-use the object-like macro EARLYCON_USED_OR_UNUSED to mark
`earlycon_acpi_spcr_enable` as maybe_unused.

Fix the following warning (treated as error in W=1)

  CC      arch/powerpc/kernel/setup-common.o
In file included from ./include/linux/serial_8250.h:14:0,
                 from arch/powerpc/kernel/setup-common.c:33:
./include/linux/serial_core.h:382:19: error: ‘earlycon_acpi_spcr_enable’ defined but not used [-Werror=unused-const-variable=]
 static const bool earlycon_acpi_spcr_enable;
                   ^~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 include/linux/serial_core.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index b32df49a3bd5..4d14ecd7dbe8 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -379,6 +379,7 @@ extern int of_setup_earlycon(const struct earlycon_id *match,
 extern bool earlycon_acpi_spcr_enable __initdata;
 int setup_earlycon(char *buf);
 #else
+EARLYCON_USED_OR_UNUSED
 static const bool earlycon_acpi_spcr_enable;
 static inline int setup_earlycon(char *buf) { return 0; }
 #endif
-- 
2.11.0

[PATCH 04/21] powerpc: Mark both tmp variables as unused

[Mathieu Malaterre]

Since the value of `tmp` is never intended to be read, declare both `tmp`
variables as unused. Fix warning (treated as error in W=1):

  CC      arch/powerpc/kernel/signal_32.o
arch/powerpc/kernel/signal_32.c: In function ‘sys_swapcontext’:
arch/powerpc/kernel/signal_32.c:1048:16: error: variable ‘tmp’ set but not used [-Werror=unused-but-set-variable]
  unsigned char tmp;
                ^~~
arch/powerpc/kernel/signal_32.c: In function ‘sys_debug_setcontext’:
arch/powerpc/kernel/signal_32.c:1234:16: error: variable ‘tmp’ set but not used [-Werror=unused-but-set-variable]
  unsigned char tmp;
                ^~~
cc1: all warnings being treated as errors

Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/powerpc/kernel/signal_32.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index a46de0035214..492f03451877 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -1045,7 +1045,7 @@ long sys_swapcontext(struct ucontext __user *old_ctx,
 		     struct ucontext __user *new_ctx,
 		     int ctx_size, int r6, int r7, int r8, struct pt_regs *regs)
 {
-	unsigned char tmp;
+	unsigned char tmp __maybe_unused;
 	int ctx_has_vsx_region = 0;
 
 #ifdef CONFIG_PPC64
@@ -1231,7 +1231,7 @@ int sys_debug_setcontext(struct ucontext __user *ctx,
 {
 	struct sig_dbg_op op;
 	int i;
-	unsigned char tmp;
+	unsigned char tmp __maybe_unused;
 	unsigned long new_msr = regs->msr;
 #ifdef CONFIG_PPC_ADV_DEBUG_REGS
 	unsigned long new_dbcr0 = current->thread.debug.dbcr0;
-- 
2.11.0

[PATCH 05/21] powerpc: Avoid comparison of unsigned long >= 0 in pfn_valid

[Mathieu Malaterre]

Rewrite comparison since all values compared are of type `unsigned long`.

Fix a warning (treated as error in W=1):

  CC      arch/powerpc/kernel/irq.o
In file included from ./include/linux/bug.h:5:0,
                 from ./include/linux/cpumask.h:13,
                 from ./include/linux/smp.h:13,
                 from ./include/linux/kernel_stat.h:5,
                 from arch/powerpc/kernel/irq.c:35:
./include/linux/dma-mapping.h: In function ‘dma_map_resource’:
./arch/powerpc/include/asm/page.h:129:32: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
 #define pfn_valid(pfn)  ((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr)
                                ^
Suggested-by: Segher Boessenkool <segher@kernel.crashing.org>
Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/powerpc/include/asm/page.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index 8da5d4c1cab2..19dea64e7ed2 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -126,7 +126,8 @@ extern long long virt_phys_offset;
 
 #ifdef CONFIG_FLATMEM
 #define ARCH_PFN_OFFSET		((unsigned long)(MEMORY_START >> PAGE_SHIFT))
-#define pfn_valid(pfn)		((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr)
+#define pfn_valid(pfn) \
+		(((pfn) - ARCH_PFN_OFFSET) < (max_mapnr - ARCH_PFN_OFFSET))
 #endif
 
 #define virt_to_pfn(kaddr)	(__pa(kaddr) >> PAGE_SHIFT)
-- 
2.11.0

[PATCH 06/21] powerpc: Avoid comparison of unsigned long >= 0 in __access_ok

[Mathieu Malaterre]

Rewrite check size - 1 <= Y as size < Y since `size` is unsigned value.
Fix warning (treated as error in W=1):

  CC      arch/powerpc/kernel/signal_32.o
In file included from ./include/linux/uaccess.h:14:0,
                 from ./include/asm-generic/termios-base.h:8,
                 from ./arch/powerpc/include/asm/termios.h:20,
                 from ./include/uapi/linux/termios.h:6,
                 from ./include/linux/tty.h:7,
                 from arch/powerpc/kernel/signal_32.c:36:
./include/asm-generic/termios-base.h: In function ‘user_termio_to_kernel_termios’:
./arch/powerpc/include/asm/uaccess.h:52:35: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
   (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)))))
                                   ^
./arch/powerpc/include/asm/uaccess.h:58:3: note: in expansion of macro ‘__access_ok’
   __access_ok((__force unsigned long)(addr), (size), get_fs()))
   ^~~~~~~~~~~
./arch/powerpc/include/asm/uaccess.h:262:6: note: in expansion of macro ‘access_ok’
  if (access_ok(VERIFY_READ, __gu_addr, (size)))   \
      ^~~~~~~~~
./arch/powerpc/include/asm/uaccess.h:80:2: note: in expansion of macro ‘__get_user_check’
  __get_user_check((x), (ptr), sizeof(*(ptr)))
  ^~~~~~~~~~~~~~~~
./include/asm-generic/termios-base.h:36:6: note: in expansion of macro ‘get_user’
  if (get_user(termios->c_line, &termio->c_line) < 0)
      ^~~~~~~~
[...]
cc1: all warnings being treated as errors

Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/powerpc/include/asm/uaccess.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 51bfeb8777f0..fadc406bd39d 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -49,7 +49,7 @@
 
 #define __access_ok(addr, size, segment)	\
 	(((addr) <= (segment).seg) &&		\
-	 (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)))))
+	 (((size) == 0) || ((size) < ((segment).seg - (addr)))))
 
 #endif
 
-- 
2.11.0

[PATCH 07/21] powerpc: Make functions flipper_pic_init & ug_udbg_putc static

[Mathieu Malaterre]

Change signature of two functions, adding static keyword to prevent the
following two warnings (treated as errors on W=1):

  CC      kernel/sys.o
arch/powerpc/platforms/embedded6xx/flipper-pic.c:135:28: error: no previous prototype for ‘flipper_pic_init’ [-Werror=missing-prototypes]
 struct irq_domain * __init flipper_pic_init(struct device_node *np)
                            ^~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

and

  CC      arch/powerpc/platforms/embedded6xx/usbgecko_udbg.o
arch/powerpc/platforms/embedded6xx/usbgecko_udbg.c:172:6: error: no previous prototype for ‘ug_udbg_putc’ [-Werror=missing-prototypes]
 void ug_udbg_putc(char ch)
      ^~~~~~~~~~~~
cc1: all warnings being treated as errors

Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/powerpc/platforms/embedded6xx/flipper-pic.c   | 2 +-
 arch/powerpc/platforms/embedded6xx/usbgecko_udbg.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/embedded6xx/flipper-pic.c b/arch/powerpc/platforms/embedded6xx/flipper-pic.c
index ade83829d5e8..7206f3f573d4 100644
--- a/arch/powerpc/platforms/embedded6xx/flipper-pic.c
+++ b/arch/powerpc/platforms/embedded6xx/flipper-pic.c
@@ -132,7 +132,7 @@ static void __flipper_quiesce(void __iomem *io_base)
 	out_be32(io_base + FLIPPER_ICR, 0xffffffff);
 }
 
-struct irq_domain * __init flipper_pic_init(struct device_node *np)
+static struct irq_domain * __init flipper_pic_init(struct device_node *np)
 {
 	struct device_node *pi;
 	struct irq_domain *irq_domain = NULL;
diff --git a/arch/powerpc/platforms/embedded6xx/usbgecko_udbg.c b/arch/powerpc/platforms/embedded6xx/usbgecko_udbg.c
index 7feb325b636b..5c7e7ce6dbab 100644
--- a/arch/powerpc/platforms/embedded6xx/usbgecko_udbg.c
+++ b/arch/powerpc/platforms/embedded6xx/usbgecko_udbg.c
@@ -169,7 +169,7 @@ static int ug_getc(void)
 /*
  * Transmits a character.
  */
-void ug_udbg_putc(char ch)
+static void ug_udbg_putc(char ch)
 {
 	ug_putc(ch);
 }
-- 
2.11.0

[PATCH 08/21] powerpc: Make function __giveup_fpu static

[Mathieu Malaterre]

When CONFIG_PPC_FPU is defined the prototype exposed in asm/switch_to.h is:

  extern void giveup_fpu(struct task_struct *);

Change the function prototype of __giveup_fpu to static. Fix warning
(treated as error in W=1):

  CC      arch/powerpc/kernel/process.o
arch/powerpc/kernel/process.c:176:6: error: no previous prototype for ‘__giveup_fpu’ [-Werror=missing-prototypes]
 void __giveup_fpu(struct task_struct *tsk)
      ^~~~~~~~~~~~
cc1: all warnings being treated as errors

Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/powerpc/kernel/process.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 1738c4127b32..3ac98c0463d6 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -173,7 +173,7 @@ void __msr_check_and_clear(unsigned long bits)
 EXPORT_SYMBOL(__msr_check_and_clear);
 
 #ifdef CONFIG_PPC_FPU
-void __giveup_fpu(struct task_struct *tsk)
+static void __giveup_fpu(struct task_struct *tsk)
 {
 	unsigned long msr;
 
-- 
2.11.0

[PATCH 09/21] powerpc: Make function save_all static

[Mathieu Malaterre]

Since function `save_all` is not meant to be exported, change the signature
to `static`. Fix warning (treated as error with W=1):

  CC      arch/powerpc/kernel/process.o
arch/powerpc/kernel/process.c:559:6: error: no previous prototype for ‘save_all’ [-Werror=missing-prototypes]
 void save_all(struct task_struct *tsk)
      ^~~~~~~~
cc1: all warnings being treated as errors

Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/powerpc/kernel/process.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 3ac98c0463d6..ec4f363ebb89 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -556,7 +556,7 @@ void restore_math(struct pt_regs *regs)
 	regs->msr = msr;
 }
 
-void save_all(struct task_struct *tsk)
+static void save_all(struct task_struct *tsk)
 {
 	unsigned long usermsr;
 
-- 
2.11.0

[PATCH 10/21] powerpc: Add missing prototype for slb_miss_bad_addr

[Mathieu Malaterre]

In commit f0f558b131db ("powerpc/mm: Preserve CFAR value on SLB miss caused
by access to bogus address"), the function slb_miss_bad_addr was added
without a prototype. This commit adds it.

Fix a warning (treated as error in W=1):

  CC      arch/powerpc/kernel/traps.o
arch/powerpc/kernel/traps.c:1498:6: error: no previous prototype for ‘slb_miss_bad_addr’ [-Werror=missing-prototypes]
 void slb_miss_bad_addr(struct pt_regs *regs)
      ^~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/powerpc/include/asm/asm-prototypes.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h
index 7330150bfe34..0af1925e30db 100644
--- a/arch/powerpc/include/asm/asm-prototypes.h
+++ b/arch/powerpc/include/asm/asm-prototypes.h
@@ -62,6 +62,7 @@ void RunModeException(struct pt_regs *regs);
 void single_step_exception(struct pt_regs *regs);
 void program_check_exception(struct pt_regs *regs);
 void alignment_exception(struct pt_regs *regs);
+void slb_miss_bad_addr(struct pt_regs *regs);
 void StackOverflow(struct pt_regs *regs);
 void nonrecoverable_exception(struct pt_regs *regs);
 void kernel_fp_unavailable_exception(struct pt_regs *regs);
-- 
2.11.0

[PATCH 11/21] powerpc: Add missing prototype for hdec_interrupt

[Mathieu Malaterre]

In commit dabe859ec636 ("powerpc: Give hypervisor decrementer interrupts
their own handler") an empty body function was added, but no prototype was
declared. Fix warning (treated as error in W=1):

  CC      arch/powerpc/kernel/time.o
arch/powerpc/kernel/time.c:629:6: error: no previous prototype for ‘hdec_interrupt’ [-Werror=missing-prototypes]
 void hdec_interrupt(struct pt_regs *regs)
      ^~~~~~~~~~~~~~
cc1: all warnings being treated as errors

Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/powerpc/include/asm/time.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
index b240666b7bc1..a7a8a9ac5991 100644
--- a/arch/powerpc/include/asm/time.h
+++ b/arch/powerpc/include/asm/time.h
@@ -31,6 +31,7 @@ extern void to_tm(int tim, struct rtc_time * tm);
 extern void tick_broadcast_ipi_handler(void);
 
 extern void generic_calibrate_decr(void);
+extern void hdec_interrupt(struct pt_regs *regs);
 
 /* Some sane defaults: 125 MHz timebase, 1GHz processor */
 extern unsigned long ppc_proc_freq;
-- 
2.11.0

[PATCH 12/21] powerpc: Add missing prototype for time_init

[Mathieu Malaterre]

The function time_init did not have a prototype defined in the time.h
header. Fix the following warning (treated as error in W=1):

  CC      arch/powerpc/kernel/time.o
arch/powerpc/kernel/time.c:1068:13: error: no previous prototype for ‘time_init’ [-Werror=missing-prototypes]
 void __init time_init(void)
             ^~~~~~~~~
cc1: all warnings being treated as errors

Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/powerpc/include/asm/time.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
index a7a8a9ac5991..828ebe7ba7dc 100644
--- a/arch/powerpc/include/asm/time.h
+++ b/arch/powerpc/include/asm/time.h
@@ -205,6 +205,7 @@ struct cpu_usage {
 DECLARE_PER_CPU(struct cpu_usage, cpu_usage_array);
 
 extern void secondary_cpu_time_init(void);
+extern void __init time_init(void);
 
 DECLARE_PER_CPU(u64, decrementers_next_tb);
 
-- 
2.11.0

[PATCH 13/21] powerpc: Add missing prototype for arch_dup_task_struct

[Mathieu Malaterre]

In commit 55ccf3fe3f9a ("fork: move the real prepare_to_copy() users to
arch_dup_task_struct()") a new arch_dup_task_struct was added without a
prototype declared in thread_info.h header. Fix the following warning
(treated as error in W=1):

  CC      arch/powerpc/kernel/process.o
arch/powerpc/kernel/process.c:1609:5: error: no previous prototype for ‘arch_dup_task_struct’ [-Werror=missing-prototypes]
 int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
     ^~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/powerpc/include/asm/thread_info.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/include/asm/thread_info.h b/arch/powerpc/include/asm/thread_info.h
index 4a12c00f8de3..5964145db03d 100644
--- a/arch/powerpc/include/asm/thread_info.h
+++ b/arch/powerpc/include/asm/thread_info.h
@@ -70,6 +70,7 @@ static inline struct thread_info *current_thread_info(void)
 	return (struct thread_info *)val;
 }
 
+extern int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src);
 #endif /* __ASSEMBLY__ */
 
 /*
-- 
2.11.0

[PATCH 14/21] powerpc: Add missing prototype for arch_irq_work_raise

[Mathieu Malaterre]

In commit 4f8b50bbbe63 ("irq_work, ppc: Fix up arch hooks") a new function
arch_irq_work_raise was added without a prototype in header irq_work.h.

Fix the following warning (treated as error in W=1):

  CC      arch/powerpc/kernel/time.o
arch/powerpc/kernel/time.c:523:6: error: no previous prototype for ‘arch_irq_work_raise’ [-Werror=missing-prototypes]
 void arch_irq_work_raise(void)
      ^~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/powerpc/include/asm/irq_work.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/include/asm/irq_work.h b/arch/powerpc/include/asm/irq_work.h
index c6d3078bd8c3..b8b0be8f1a07 100644
--- a/arch/powerpc/include/asm/irq_work.h
+++ b/arch/powerpc/include/asm/irq_work.h
@@ -6,5 +6,6 @@ static inline bool arch_irq_work_has_interrupt(void)
 {
 	return true;
 }
+extern void arch_irq_work_raise(void);
 
 #endif /* _ASM_POWERPC_IRQ_WORK_H */
-- 
2.11.0

[PATCH 15/21] powerpc: Add missing prototype for MMU_setup

[Mathieu Malaterre]

Add a function declaration for MMU_setup at the beginning of the file to
fix a warning (treated as error in W=1):

  CC      kernel/sys.o
arch/powerpc/mm/init_32.c:102:13: error: no previous prototype for ‘MMU_setup’ [-Werror=missing-prototypes]
 void __init MMU_setup(void)
             ^~~~~~~~~
cc1: all warnings being treated as errors

Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/powerpc/mm/init_32.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/mm/init_32.c b/arch/powerpc/mm/init_32.c
index 6419b33ca309..e88bcad9a63b 100644
--- a/arch/powerpc/mm/init_32.c
+++ b/arch/powerpc/mm/init_32.c
@@ -79,6 +79,7 @@ EXPORT_SYMBOL(agp_special_page);
 #endif
 
 void MMU_init(void);
+void __init MMU_setup(void);
 
 /*
  * this tells the system to map all of ram with the segregs
-- 
2.11.0

[PATCH 16/21] powerpc: Add missing prototype for init_IRQ

[Mathieu Malaterre]

A function init_IRQ was added without a prototype declared in header irq.h.
Fix the following warning (treated as error in W=1):

  CC      arch/powerpc/kernel/irq.o
arch/powerpc/kernel/irq.c:662:13: error: no previous prototype for ‘init_IRQ’ [-Werror=missing-prototypes]
 void __init init_IRQ(void)
             ^~~~~~~~
cc1: all warnings being treated as errors

Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/powerpc/include/asm/irq.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/include/asm/irq.h b/arch/powerpc/include/asm/irq.h
index e8e3a0a04eb0..ee39ce56b2a2 100644
--- a/arch/powerpc/include/asm/irq.h
+++ b/arch/powerpc/include/asm/irq.h
@@ -66,6 +66,7 @@ extern void irq_ctx_init(void);
 extern void call_do_softirq(struct thread_info *tp);
 extern void call_do_irq(struct pt_regs *regs, struct thread_info *tp);
 extern void do_IRQ(struct pt_regs *regs);
+extern void __init init_IRQ(void);
 extern void __do_irq(struct pt_regs *regs);
 
 int irq_choose_cpu(const struct cpumask *mask);
-- 
2.11.0

[PATCH 17/21] powerpc: Add missing prototype for sys_debug_setcontext

[Mathieu Malaterre]

In commit 81e7009ea46c ("powerpc: merge ppc signal.c and ppc64 signal32.c")
the function sys_debug_setcontext was added without a prototype.

Fix compilation warning (treated as error in W=1):

  CC      arch/powerpc/kernel/signal_32.o
arch/powerpc/kernel/signal_32.c:1227:5: error: no previous prototype for ‘sys_debug_setcontext’ [-Werror=missing-prototypes]
 int sys_debug_setcontext(struct ucontext __user *ctx,
     ^~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/powerpc/include/asm/asm-prototypes.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h
index 0af1925e30db..7c23d9ead694 100644
--- a/arch/powerpc/include/asm/asm-prototypes.h
+++ b/arch/powerpc/include/asm/asm-prototypes.h
@@ -89,6 +89,10 @@ int sys_swapcontext(struct ucontext __user *old_ctx,
 long sys_swapcontext(struct ucontext __user *old_ctx,
 		    struct ucontext __user *new_ctx,
 		    int ctx_size, int r6, int r7, int r8, struct pt_regs *regs);
+int sys_debug_setcontext(struct ucontext __user *ctx,
+			 int ndbg, struct sig_dbg_op __user *dbg,
+			 int r6, int r7, int r8,
+			 struct pt_regs *regs);
 #endif
 long sys_switch_endian(void);
 notrace unsigned int __check_irq_replay(void);
-- 
2.11.0

[PATCH 18/21] powerpc: Add missing prototypes for sys_sigreturn & sys_rt_sigreturn

[Mathieu Malaterre]

Two functions did not have a prototype defined in signal.h header. Fix the
following two warnings (treated as errors in W=1):

  CC      arch/powerpc/kernel/signal_32.o
arch/powerpc/kernel/signal_32.c:1135:6: error: no previous prototype for ‘sys_rt_sigreturn’ [-Werror=missing-prototypes]
 long sys_rt_sigreturn(int r3, int r4, int r5, int r6, int r7, int r8,
      ^~~~~~~~~~~~~~~~
arch/powerpc/kernel/signal_32.c:1422:6: error: no previous prototype for ‘sys_sigreturn’ [-Werror=missing-prototypes]
 long sys_sigreturn(int r3, int r4, int r5, int r6, int r7, int r8,
      ^~~~~~~~~~~~~
cc1: all warnings being treated as errors

Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/powerpc/kernel/signal.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/powerpc/kernel/signal.h b/arch/powerpc/kernel/signal.h
index 7c59d88b9d86..a6467f843acf 100644
--- a/arch/powerpc/kernel/signal.h
+++ b/arch/powerpc/kernel/signal.h
@@ -49,6 +49,11 @@ extern int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
 
 #else /* CONFIG_PPC64 */
 
+extern long sys_rt_sigreturn(int r3, int r4, int r5, int r6, int r7, int r8,
+		     struct pt_regs *regs);
+extern long sys_sigreturn(int r3, int r4, int r5, int r6, int r7, int r8,
+		       struct pt_regs *regs);
+
 static inline int handle_rt_signal64(struct ksignal *ksig, sigset_t *set,
 				     struct task_struct *tsk)
 {
-- 
2.11.0

[PATCH 19/21] powerpc: Add missing prototypes for hw_breakpoint_handler & arch_unregister_hw_breakpoint

[Mathieu Malaterre]

In commit 5aae8a537080 ("powerpc, hw_breakpoints: Implement hw_breakpoints
for 64-bit server processors") function hw_breakpoint_handler and
arch_unregister_hw_breakpoint were added without function prototypes in
hw_breakpoint.h header.

Fix the following warning(s) (treated as error in W=1):

  AR      init/built-in.o
arch/powerpc/kernel/hw_breakpoint.c:106:6: error: no previous prototype for ‘arch_unregister_hw_breakpoint’ [-Werror=missing-prototypes]
 void arch_unregister_hw_breakpoint(struct perf_event *bp)
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/kernel/hw_breakpoint.c:209:5: error: no previous prototype for ‘hw_breakpoint_handler’ [-Werror=missing-prototypes]
 int hw_breakpoint_handler(struct die_args *args)
     ^~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/powerpc/include/asm/hw_breakpoint.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h
index ac6432d9be46..90c708e5e7c4 100644
--- a/arch/powerpc/include/asm/hw_breakpoint.h
+++ b/arch/powerpc/include/asm/hw_breakpoint.h
@@ -66,6 +66,7 @@ extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
 						unsigned long val, void *data);
 int arch_install_hw_breakpoint(struct perf_event *bp);
 void arch_uninstall_hw_breakpoint(struct perf_event *bp);
+void arch_unregister_hw_breakpoint(struct perf_event *bp);
 void hw_breakpoint_pmu_read(struct perf_event *bp);
 extern void flush_ptrace_hw_breakpoint(struct task_struct *tsk);
 
@@ -82,6 +83,7 @@ static inline void hw_breakpoint_disable(void)
 	__set_breakpoint(&brk);
 }
 extern void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs);
+int hw_breakpoint_handler(struct die_args *args);
 
 #else	/* CONFIG_HAVE_HW_BREAKPOINT */
 static inline void hw_breakpoint_disable(void) { }
-- 
2.11.0

[PATCH 20/21] powerpc: Add missing prototypes for ppc_select & ppc_fadvise64_64

[Mathieu Malaterre]

Add missing prototypes for ppc_select & ppc_fadvise64_64 to header
asm-prototypes.h. Fix the following warnings (treated as errors in W=1)

  CC      arch/powerpc/kernel/syscalls.o
arch/powerpc/kernel/syscalls.c:87:1: error: no previous prototype for ‘ppc_select’ [-Werror=missing-prototypes]
 ppc_select(int n, fd_set __user *inp, fd_set __user *outp, fd_set __user *exp, struct timeval __user *tvp)
 ^~~~~~~~~~
arch/powerpc/kernel/syscalls.c:119:6: error: no previous prototype for ‘ppc_fadvise64_64’ [-Werror=missing-prototypes]
 long ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32 offset_low,
      ^~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/powerpc/include/asm/asm-prototypes.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h
index 7c23d9ead694..4d8b89b46018 100644
--- a/arch/powerpc/include/asm/asm-prototypes.h
+++ b/arch/powerpc/include/asm/asm-prototypes.h
@@ -93,7 +93,11 @@ int sys_debug_setcontext(struct ucontext __user *ctx,
 			 int ndbg, struct sig_dbg_op __user *dbg,
 			 int r6, int r7, int r8,
 			 struct pt_regs *regs);
+int
+ppc_select(int n, fd_set __user *inp, fd_set __user *outp, fd_set __user *exp, struct timeval __user *tvp);
 #endif
+long ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32 offset_low,
+		      u32 len_high, u32 len_low);
 long sys_switch_endian(void);
 notrace unsigned int __check_irq_replay(void);
 void notrace restore_interrupts(void);
-- 
2.11.0

[PATCH 21/21] powerpc: Add missing prototypes in setup_32.c

[Mathieu Malaterre]

This commit add the prototypes for the following function:

- early_init
- machine_init
- ppc_setup_l2cr
- ppc_setup_l3cr
- ppc_init

the other missing ones were already added in setup.h previously. Fix the
following warnings (treated as error in W=1):

  AR      init/built-in.o
arch/powerpc/kernel/setup_32.c:68:30: error: no previous prototype for ‘early_init’ [-Werror=missing-prototypes]
 notrace unsigned long __init early_init(unsigned long dt_ptr)
                              ^~~~~~~~~~
arch/powerpc/kernel/setup_32.c:99:21: error: no previous prototype for ‘machine_init’ [-Werror=missing-prototypes]
 notrace void __init machine_init(u64 dt_ptr)
                     ^~~~~~~~~~~~
arch/powerpc/kernel/setup_32.c:124:12: error: no previous prototype for ‘ppc_setup_l2cr’ [-Werror=missing-prototypes]
 int __init ppc_setup_l2cr(char *str)
            ^~~~~~~~~~~~~~
arch/powerpc/kernel/setup_32.c:137:12: error: no previous prototype for ‘ppc_setup_l3cr’ [-Werror=missing-prototypes]
 int __init ppc_setup_l3cr(char *str)
            ^~~~~~~~~~~~~~
arch/powerpc/kernel/setup_32.c:183:12: error: no previous prototype for ‘ppc_init’ [-Werror=missing-prototypes]
 int __init ppc_init(void)
            ^~~~~~~~
arch/powerpc/kernel/setup_32.c:198:13: error: no previous prototype for ‘irqstack_early_init’ [-Werror=missing-prototypes]
 void __init irqstack_early_init(void)
             ^~~~~~~~~~~~~~~~~~~
arch/powerpc/kernel/setup_32.c:238:13: error: no previous prototype for ‘setup_power_save’ [-Werror=missing-prototypes]
 void __init setup_power_save(void)
             ^~~~~~~~~~~~~~~~
arch/powerpc/kernel/setup_32.c:253:13: error: no previous prototype for ‘initialize_cache_info’ [-Werror=missing-prototypes]
 __init void initialize_cache_info(void)
             ^~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/powerpc/kernel/setup.h    | 5 +++++
 arch/powerpc/kernel/setup_32.c | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/arch/powerpc/kernel/setup.h b/arch/powerpc/kernel/setup.h
index 3fc11e30308f..d768023a04bd 100644
--- a/arch/powerpc/kernel/setup.h
+++ b/arch/powerpc/kernel/setup.h
@@ -17,6 +17,11 @@ void irqstack_early_init(void);
 
 #ifdef CONFIG_PPC32
 void setup_power_save(void);
+unsigned long __init early_init(unsigned long dt_ptr);
+void __init machine_init(u64 dt_ptr);
+int __init ppc_setup_l2cr(char *str);
+int __init ppc_setup_l3cr(char *str);
+int __init ppc_init(void);
 #else
 static inline void setup_power_save(void) { };
 #endif
diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
index 51ebc01fff52..12bcab77a29f 100644
--- a/arch/powerpc/kernel/setup_32.c
+++ b/arch/powerpc/kernel/setup_32.c
@@ -40,6 +40,8 @@
 #include <asm/code-patching.h>
 #include <asm/cpu_has_feature.h>
 
+#include "setup.h"
+
 #define DBG(fmt...)
 
 extern void bootx_init(unsigned long r4, unsigned long phys);
-- 
2.11.0

Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

[Segher Boessenkool]

Hi!

On Sun, Feb 25, 2018 at 06:22:16PM +0100, Mathieu Malaterre wrote:
> When neither CONFIG_ALTIVEC, nor CONFIG_VSX or CONFIG_PPC64 is defined, the
> array feature_properties is defined as an empty array, which in turn
> triggers the following warning (treated as error on W=1):

> -	for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
> +	for (; fp != feature_properties + ARRAY_SIZE(feature_properties); ++fp) {

You could just write != instead of < ?  Seems more readable.

Maybe something can be done to ARRAY_SIZE to make this not warn.


Segher

Re: [PATCH 05/21] powerpc: Avoid comparison of unsigned long >= 0 in pfn_valid

[Christophe LEROY]

Le 25/02/2018 à 18:22, Mathieu Malaterre a écrit :
> Rewrite comparison since all values compared are of type `unsigned long`.
> 
> Fix a warning (treated as error in W=1):
> 
>    CC      arch/powerpc/kernel/irq.o
> In file included from ./include/linux/bug.h:5:0,
>                   from ./include/linux/cpumask.h:13,
>                   from ./include/linux/smp.h:13,
>                   from ./include/linux/kernel_stat.h:5,
>                   from arch/powerpc/kernel/irq.c:35:
> ./include/linux/dma-mapping.h: In function ‘dma_map_resource’:
> ./arch/powerpc/include/asm/page.h:129:32: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
>   #define pfn_valid(pfn)  ((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr)
>                                  ^
> Suggested-by: Segher Boessenkool <segher@kernel.crashing.org>
> Signed-off-by: Mathieu Malaterre <malat@debian.org>
> ---
>   arch/powerpc/include/asm/page.h | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
> index 8da5d4c1cab2..19dea64e7ed2 100644
> --- a/arch/powerpc/include/asm/page.h
> +++ b/arch/powerpc/include/asm/page.h
> @@ -126,7 +126,8 @@ extern long long virt_phys_offset;
>   
>   #ifdef CONFIG_FLATMEM
>   #define ARCH_PFN_OFFSET		((unsigned long)(MEMORY_START >> PAGE_SHIFT))
> -#define pfn_valid(pfn)		((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr)
> +#define pfn_valid(pfn) \
> +		(((pfn) - ARCH_PFN_OFFSET) < (max_mapnr - ARCH_PFN_OFFSET))

What will happen when ARCH_PFN_OFFSET is not nul and pfn is lower than 
ARCH_PFN_OFFSET ?

Christophe

>   #endif
>   
>   #define virt_to_pfn(kaddr)	(__pa(kaddr) >> PAGE_SHIFT)
> 

Re: [PATCH 06/21] powerpc: Avoid comparison of unsigned long >= 0 in __access_ok

[Christophe LEROY]

Le 25/02/2018 à 18:22, Mathieu Malaterre a écrit :
> Rewrite check size - 1 <= Y as size < Y since `size` is unsigned value.
> Fix warning (treated as error in W=1):
> 
>    CC      arch/powerpc/kernel/signal_32.o
> In file included from ./include/linux/uaccess.h:14:0,
>                   from ./include/asm-generic/termios-base.h:8,
>                   from ./arch/powerpc/include/asm/termios.h:20,
>                   from ./include/uapi/linux/termios.h:6,
>                   from ./include/linux/tty.h:7,
>                   from arch/powerpc/kernel/signal_32.c:36:
> ./include/asm-generic/termios-base.h: In function ‘user_termio_to_kernel_termios’:
> ./arch/powerpc/include/asm/uaccess.h:52:35: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
>     (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)))))
>                                     ^
> ./arch/powerpc/include/asm/uaccess.h:58:3: note: in expansion of macro ‘__access_ok’
>     __access_ok((__force unsigned long)(addr), (size), get_fs()))
>     ^~~~~~~~~~~
> ./arch/powerpc/include/asm/uaccess.h:262:6: note: in expansion of macro ‘access_ok’
>    if (access_ok(VERIFY_READ, __gu_addr, (size)))   \
>        ^~~~~~~~~
> ./arch/powerpc/include/asm/uaccess.h:80:2: note: in expansion of macro ‘__get_user_check’
>    __get_user_check((x), (ptr), sizeof(*(ptr)))
>    ^~~~~~~~~~~~~~~~
> ./include/asm-generic/termios-base.h:36:6: note: in expansion of macro ‘get_user’
>    if (get_user(termios->c_line, &termio->c_line) < 0)
>        ^~~~~~~~
> [...]
> cc1: all warnings being treated as errors
> 
> Signed-off-by: Mathieu Malaterre <malat@debian.org>
> ---
>   arch/powerpc/include/asm/uaccess.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
> index 51bfeb8777f0..fadc406bd39d 100644
> --- a/arch/powerpc/include/asm/uaccess.h
> +++ b/arch/powerpc/include/asm/uaccess.h
> @@ -49,7 +49,7 @@
>   
>   #define __access_ok(addr, size, segment)	\
>   	(((addr) <= (segment).seg) &&		\
> -	 (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)))))
> +	 (((size) == 0) || ((size) < ((segment).seg - (addr)))))

IIUC, ((2 - 1) <= 1) is the same as (2 < 1) ?????

Christophe

>   
>   #endif
>   
> 

Re: [PATCH 06/21] powerpc: Avoid comparison of unsigned long >= 0 in __access_ok

[Christophe LEROY]

Le 26/02/2018 à 07:34, Christophe LEROY a écrit :
> 
> 
> Le 25/02/2018 à 18:22, Mathieu Malaterre a écrit :
>> Rewrite check size - 1 <= Y as size < Y since `size` is unsigned value.
>> Fix warning (treated as error in W=1):
>>
>>    CC      arch/powerpc/kernel/signal_32.o
>> In file included from ./include/linux/uaccess.h:14:0,
>>                   from ./include/asm-generic/termios-base.h:8,
>>                   from ./arch/powerpc/include/asm/termios.h:20,
>>                   from ./include/uapi/linux/termios.h:6,
>>                   from ./include/linux/tty.h:7,
>>                   from arch/powerpc/kernel/signal_32.c:36:
>> ./include/asm-generic/termios-base.h: In function 
>> ‘user_termio_to_kernel_termios’:
>> ./arch/powerpc/include/asm/uaccess.h:52:35: error: comparison of 
>> unsigned expression >= 0 is always true [-Werror=type-limits]
>>     (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)))))
>>                                     ^
>> ./arch/powerpc/include/asm/uaccess.h:58:3: note: in expansion of macro 
>> ‘__access_ok’
>>     __access_ok((__force unsigned long)(addr), (size), get_fs()))
>>     ^~~~~~~~~~~
>> ./arch/powerpc/include/asm/uaccess.h:262:6: note: in expansion of 
>> macro ‘access_ok’
>>    if (access_ok(VERIFY_READ, __gu_addr, (size)))   \
>>        ^~~~~~~~~
>> ./arch/powerpc/include/asm/uaccess.h:80:2: note: in expansion of macro 
>> ‘__get_user_check’
>>    __get_user_check((x), (ptr), sizeof(*(ptr)))
>>    ^~~~~~~~~~~~~~~~
>> ./include/asm-generic/termios-base.h:36:6: note: in expansion of macro 
>> ‘get_user’
>>    if (get_user(termios->c_line, &termio->c_line) < 0)
>>        ^~~~~~~~
>> [...]
>> cc1: all warnings being treated as errors
>>
>> Signed-off-by: Mathieu Malaterre <malat@debian.org>
>> ---
>>   arch/powerpc/include/asm/uaccess.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/include/asm/uaccess.h 
>> b/arch/powerpc/include/asm/uaccess.h
>> index 51bfeb8777f0..fadc406bd39d 100644
>> --- a/arch/powerpc/include/asm/uaccess.h
>> +++ b/arch/powerpc/include/asm/uaccess.h
>> @@ -49,7 +49,7 @@
>>   #define __access_ok(addr, size, segment)    \
>>       (((addr) <= (segment).seg) &&        \
>> -     (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)))))
>> +     (((size) == 0) || ((size) < ((segment).seg - (addr)))))
> 
> IIUC, ((2 - 1) <= 1) is the same as (2 < 1) ?????


Note that I already try to submit a fix for this warning 3 years ago 
(https://patchwork.ozlabs.org/patch/418075/) and it was rejected with 
the following comment:

Again, I don't think Linux enables this warning.  What did you do to
produce this?  In any case, it's a bad warning that doesn't take macros
into account, and the answer is not to make the code less clear by hiding
the fact that zero is a special case.

Christophe


> 
> Christophe
> 
>>   #endif
>>

Re: [PATCH 06/21] powerpc: Avoid comparison of unsigned long >= 0 in __access_ok

[Mathieu Malaterre]

On Mon, Feb 26, 2018 at 7:50 AM, Christophe LEROY
<christophe.leroy@c-s.fr> wrote:
>
>
> Le 26/02/2018 à 07:34, Christophe LEROY a écrit :
>>
>>
>>
>> Le 25/02/2018 à 18:22, Mathieu Malaterre a écrit :
>>>
>>> Rewrite check size - 1 <= Y as size < Y since `size` is unsigned value.
>>> Fix warning (treated as error in W=1):
>>>
>>>    CC      arch/powerpc/kernel/signal_32.o
>>> In file included from ./include/linux/uaccess.h:14:0,
>>>                   from ./include/asm-generic/termios-base.h:8,
>>>                   from ./arch/powerpc/include/asm/termios.h:20,
>>>                   from ./include/uapi/linux/termios.h:6,
>>>                   from ./include/linux/tty.h:7,
>>>                   from arch/powerpc/kernel/signal_32.c:36:
>>> ./include/asm-generic/termios-base.h: In function
>>> ‘user_termio_to_kernel_termios’:
>>> ./arch/powerpc/include/asm/uaccess.h:52:35: error: comparison of unsigned
>>> expression >= 0 is always true [-Werror=type-limits]
>>>     (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)))))
>>>                                     ^
>>> ./arch/powerpc/include/asm/uaccess.h:58:3: note: in expansion of macro
>>> ‘__access_ok’
>>>     __access_ok((__force unsigned long)(addr), (size), get_fs()))
>>>     ^~~~~~~~~~~
>>> ./arch/powerpc/include/asm/uaccess.h:262:6: note: in expansion of macro
>>> ‘access_ok’
>>>    if (access_ok(VERIFY_READ, __gu_addr, (size)))   \
>>>        ^~~~~~~~~
>>> ./arch/powerpc/include/asm/uaccess.h:80:2: note: in expansion of macro
>>> ‘__get_user_check’
>>>    __get_user_check((x), (ptr), sizeof(*(ptr)))
>>>    ^~~~~~~~~~~~~~~~
>>> ./include/asm-generic/termios-base.h:36:6: note: in expansion of macro
>>> ‘get_user’
>>>    if (get_user(termios->c_line, &termio->c_line) < 0)
>>>        ^~~~~~~~
>>> [...]
>>> cc1: all warnings being treated as errors
>>>
>>> Signed-off-by: Mathieu Malaterre <malat@debian.org>
>>> ---
>>>   arch/powerpc/include/asm/uaccess.h | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/powerpc/include/asm/uaccess.h
>>> b/arch/powerpc/include/asm/uaccess.h
>>> index 51bfeb8777f0..fadc406bd39d 100644
>>> --- a/arch/powerpc/include/asm/uaccess.h
>>> +++ b/arch/powerpc/include/asm/uaccess.h
>>> @@ -49,7 +49,7 @@
>>>   #define __access_ok(addr, size, segment)    \
>>>       (((addr) <= (segment).seg) &&        \
>>> -     (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)))))
>>> +     (((size) == 0) || ((size) < ((segment).seg - (addr)))))
>>
>>
>> IIUC, ((2 - 1) <= 1) is the same as (2 < 1) ?????
>

The whole series was pretty mediocre, but this one was actually pretty
destructive. Thanks for catching this.

>
> Note that I already try to submit a fix for this warning 3 years ago
> (https://patchwork.ozlabs.org/patch/418075/) and it was rejected with the
> following comment:
>
> Again, I don't think Linux enables this warning.  What did you do to
> produce this?  In any case, it's a bad warning that doesn't take macros
> into account, and the answer is not to make the code less clear by hiding
> the fact that zero is a special case.

Right. I'll try to see how to make W=1 run without error with an
alternate solution.

> Christophe
>
>
>>
>> Christophe
>>
>>>   #endif
>>>
>

Re: [PATCH 05/21] powerpc: Avoid comparison of unsigned long >= 0 in pfn_valid

[Mathieu Malaterre]

On Mon, Feb 26, 2018 at 7:32 AM, Christophe LEROY
<christophe.leroy@c-s.fr> wrote:
>
>
> Le 25/02/2018 à 18:22, Mathieu Malaterre a écrit :
>>
>> Rewrite comparison since all values compared are of type `unsigned long`.
>>
>> Fix a warning (treated as error in W=1):
>>
>>    CC      arch/powerpc/kernel/irq.o
>> In file included from ./include/linux/bug.h:5:0,
>>                   from ./include/linux/cpumask.h:13,
>>                   from ./include/linux/smp.h:13,
>>                   from ./include/linux/kernel_stat.h:5,
>>                   from arch/powerpc/kernel/irq.c:35:
>> ./include/linux/dma-mapping.h: In function ‘dma_map_resource’:
>> ./arch/powerpc/include/asm/page.h:129:32: error: comparison of unsigned
>> expression >= 0 is always true [-Werror=type-limits]
>>   #define pfn_valid(pfn)  ((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr)
>>                                  ^
>> Suggested-by: Segher Boessenkool <segher@kernel.crashing.org>
>> Signed-off-by: Mathieu Malaterre <malat@debian.org>
>> ---
>>   arch/powerpc/include/asm/page.h | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/include/asm/page.h
>> b/arch/powerpc/include/asm/page.h
>> index 8da5d4c1cab2..19dea64e7ed2 100644
>> --- a/arch/powerpc/include/asm/page.h
>> +++ b/arch/powerpc/include/asm/page.h
>> @@ -126,7 +126,8 @@ extern long long virt_phys_offset;
>>     #ifdef CONFIG_FLATMEM
>>   #define ARCH_PFN_OFFSET               ((unsigned long)(MEMORY_START >>
>> PAGE_SHIFT))
>> -#define pfn_valid(pfn)         ((pfn) >= ARCH_PFN_OFFSET && (pfn) <
>> max_mapnr)
>> +#define pfn_valid(pfn) \
>> +               (((pfn) - ARCH_PFN_OFFSET) < (max_mapnr -
>> ARCH_PFN_OFFSET))
>
>
> What will happen when ARCH_PFN_OFFSET is not nul and pfn is lower than
> ARCH_PFN_OFFSET ?

I assumed that normal unsigned integers modulo would make the test
fail. But for some particular value of max_mapnr the two are indeed
not equivalent.

> Christophe
>
>
>>   #endif
>>     #define virt_to_pfn(kaddr)  (__pa(kaddr) >> PAGE_SHIFT)
>>
>

Re: [PATCH 05/21] powerpc: Avoid comparison of unsigned long >= 0 in pfn_valid

[Segher Boessenkool]

On Mon, Feb 26, 2018 at 07:32:03AM +0100, Christophe LEROY wrote:
> Le 25/02/2018 à 18:22, Mathieu Malaterre a écrit :
> >-#define pfn_valid(pfn)		((pfn) >= ARCH_PFN_OFFSET && (pfn) < 
> >max_mapnr)
> >+#define pfn_valid(pfn) \
> >+		(((pfn) - ARCH_PFN_OFFSET) < (max_mapnr - ARCH_PFN_OFFSET))
> 
> What will happen when ARCH_PFN_OFFSET is not nul and pfn is lower than 
> ARCH_PFN_OFFSET ?

It will work fine.

Say you are asking for  a <= x < b  so (in actual integers, no overflow)
that is  0 <= x-a < b-a  and you also assume x-a overflows, so that we
are actually comparing  x-a+M < b-a  with M = 2**32 or such (the maximum
value in the unsigned integer type plus one).  This comparison is
obviously always false.

(It also works if b < a btw).


Segher

Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

[Andy Shevchenko]

On Sun, Feb 25, 2018 at 7:22 PM, Mathieu Malaterre <malat@debian.org> wrote:
> When neither CONFIG_ALTIVEC, nor CONFIG_VSX or CONFIG_PPC64 is defined, the
> array feature_properties is defined as an empty array, which in turn
> triggers the following warning (treated as error on W=1):
>
>   CC      arch/powerpc/kernel/prom.o
> arch/powerpc/kernel/prom.c: In function ‘check_cpu_feature_properties’:
> arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
>   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>                 ^
> cc1: all warnings being treated as errors
>
> Signed-off-by: Mathieu Malaterre <malat@debian.org>
> ---
>  arch/powerpc/kernel/prom.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 4dffef947b8a..6e8e4122820e 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -291,11 +291,10 @@ static inline void identical_pvr_fixup(unsigned long node)
>
>  static void __init check_cpu_feature_properties(unsigned long node)
>  {
> -       unsigned long i;
>         struct feature_property *fp = feature_properties;
>         const __be32 *prop;
>

Much simpler is just add

if (ARRAY_SIZE() == 0)
 return;

> -       for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
> +       for (; fp != feature_properties + ARRAY_SIZE(feature_properties); ++fp) {
>                 prop = of_get_flat_dt_prop(node, fp->name, NULL);
>                 if (prop && be32_to_cpup(prop) >= fp->min_value) {
>                         cur_cpu_spec->cpu_features |= fp->cpu_feature;
> --
> 2.11.0
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

[Andy Shevchenko]

On Mon, Feb 26, 2018 at 4:44 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Sun, Feb 25, 2018 at 7:22 PM, Mathieu Malaterre <malat@debian.org> wrote:

>>  static void __init check_cpu_feature_properties(unsigned long node)
>>  {
>> -       unsigned long i;
>>         struct feature_property *fp = feature_properties;
>>         const __be32 *prop;
>>
>
> Much simpler is just add
>
> if (ARRAY_SIZE() == 0)
>  return;
>
>> -       for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>> +       for (; fp != feature_properties + ARRAY_SIZE(feature_properties); ++fp) {

...or convert to while(), which will be more readable.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 06/21] powerpc: Avoid comparison of unsigned long >= 0 in __access_ok

[Mathieu Malaterre]

On Mon, Feb 26, 2018 at 8:44 AM, Mathieu Malaterre <malat@debian.org> wrote:
> On Mon, Feb 26, 2018 at 7:50 AM, Christophe LEROY
> <christophe.leroy@c-s.fr> wrote:
>>
>>
>> Le 26/02/2018 à 07:34, Christophe LEROY a écrit :
>>>
>>>
>>>
>>> Le 25/02/2018 à 18:22, Mathieu Malaterre a écrit :
>>>>
>>>> Rewrite check size - 1 <= Y as size < Y since `size` is unsigned value.
>>>> Fix warning (treated as error in W=1):
>>>>
>>>>    CC      arch/powerpc/kernel/signal_32.o
>>>> In file included from ./include/linux/uaccess.h:14:0,
>>>>                   from ./include/asm-generic/termios-base.h:8,
>>>>                   from ./arch/powerpc/include/asm/termios.h:20,
>>>>                   from ./include/uapi/linux/termios.h:6,
>>>>                   from ./include/linux/tty.h:7,
>>>>                   from arch/powerpc/kernel/signal_32.c:36:
>>>> ./include/asm-generic/termios-base.h: In function
>>>> ‘user_termio_to_kernel_termios’:
>>>> ./arch/powerpc/include/asm/uaccess.h:52:35: error: comparison of unsigned
>>>> expression >= 0 is always true [-Werror=type-limits]
>>>>     (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)))))
>>>>                                     ^
>>>> ./arch/powerpc/include/asm/uaccess.h:58:3: note: in expansion of macro
>>>> ‘__access_ok’
>>>>     __access_ok((__force unsigned long)(addr), (size), get_fs()))
>>>>     ^~~~~~~~~~~
>>>> ./arch/powerpc/include/asm/uaccess.h:262:6: note: in expansion of macro
>>>> ‘access_ok’
>>>>    if (access_ok(VERIFY_READ, __gu_addr, (size)))   \
>>>>        ^~~~~~~~~
>>>> ./arch/powerpc/include/asm/uaccess.h:80:2: note: in expansion of macro
>>>> ‘__get_user_check’
>>>>    __get_user_check((x), (ptr), sizeof(*(ptr)))
>>>>    ^~~~~~~~~~~~~~~~
>>>> ./include/asm-generic/termios-base.h:36:6: note: in expansion of macro
>>>> ‘get_user’
>>>>    if (get_user(termios->c_line, &termio->c_line) < 0)
>>>>        ^~~~~~~~
>>>> [...]
>>>> cc1: all warnings being treated as errors
>>>>
>>>> Signed-off-by: Mathieu Malaterre <malat@debian.org>
>>>> ---
>>>>   arch/powerpc/include/asm/uaccess.h | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/powerpc/include/asm/uaccess.h
>>>> b/arch/powerpc/include/asm/uaccess.h
>>>> index 51bfeb8777f0..fadc406bd39d 100644
>>>> --- a/arch/powerpc/include/asm/uaccess.h
>>>> +++ b/arch/powerpc/include/asm/uaccess.h
>>>> @@ -49,7 +49,7 @@
>>>>   #define __access_ok(addr, size, segment)    \
>>>>       (((addr) <= (segment).seg) &&        \
>>>> -     (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)))))
>>>> +     (((size) == 0) || ((size) < ((segment).seg - (addr)))))
>>>
>>>
>>> IIUC, ((2 - 1) <= 1) is the same as (2 < 1) ?????
>>
>
> The whole series was pretty mediocre, but this one was actually pretty
> destructive. Thanks for catching this.
>
>>
>> Note that I already try to submit a fix for this warning 3 years ago
>> (https://patchwork.ozlabs.org/patch/418075/) and it was rejected with the
>> following comment:

Tested again today with gcc 6.3.0 and gcc is still producing the
original warning (treated as error).

>> Again, I don't think Linux enables this warning.  What did you do to
>> produce this?  In any case, it's a bad warning that doesn't take macros
>> into account, and the answer is not to make the code less clear by hiding
>> the fact that zero is a special case.
>
> Right. I'll try to see how to make W=1 run without error with an
> alternate solution.

So the other alternative is to update a bunch of ppc32 defconfig(s)
with: CONFIG_PPC_DISABLE_WERROR=y.

Would that be preferable ?

>> Christophe
>>
>>
>>>
>>> Christophe
>>>
>>>>   #endif
>>>>
>>

Re: [PATCH 06/21] powerpc: Avoid comparison of unsigned long >= 0 in __access_ok

[christophe leroy]

Le 26/02/2018 à 18:50, Mathieu Malaterre a écrit :
> On Mon, Feb 26, 2018 at 8:44 AM, Mathieu Malaterre <malat@debian.org> wrote:
>> On Mon, Feb 26, 2018 at 7:50 AM, Christophe LEROY
>> <christophe.leroy@c-s.fr> wrote:
>>>
>>>
>>> Le 26/02/2018 à 07:34, Christophe LEROY a écrit :
>>>>
>>>>
>>>>
>>>> Le 25/02/2018 à 18:22, Mathieu Malaterre a écrit :
>>>>>
>>>>> Rewrite check size - 1 <= Y as size < Y since `size` is unsigned value.
>>>>> Fix warning (treated as error in W=1):
>>>>>
>>>>>     CC      arch/powerpc/kernel/signal_32.o
>>>>> In file included from ./include/linux/uaccess.h:14:0,
>>>>>                    from ./include/asm-generic/termios-base.h:8,
>>>>>                    from ./arch/powerpc/include/asm/termios.h:20,
>>>>>                    from ./include/uapi/linux/termios.h:6,
>>>>>                    from ./include/linux/tty.h:7,
>>>>>                    from arch/powerpc/kernel/signal_32.c:36:
>>>>> ./include/asm-generic/termios-base.h: In function
>>>>> ‘user_termio_to_kernel_termios’:
>>>>> ./arch/powerpc/include/asm/uaccess.h:52:35: error: comparison of unsigned
>>>>> expression >= 0 is always true [-Werror=type-limits]
>>>>>      (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)))))
>>>>>                                      ^
>>>>> ./arch/powerpc/include/asm/uaccess.h:58:3: note: in expansion of macro
>>>>> ‘__access_ok’
>>>>>      __access_ok((__force unsigned long)(addr), (size), get_fs()))
>>>>>      ^~~~~~~~~~~
>>>>> ./arch/powerpc/include/asm/uaccess.h:262:6: note: in expansion of macro
>>>>> ‘access_ok’
>>>>>     if (access_ok(VERIFY_READ, __gu_addr, (size)))   \
>>>>>         ^~~~~~~~~
>>>>> ./arch/powerpc/include/asm/uaccess.h:80:2: note: in expansion of macro
>>>>> ‘__get_user_check’
>>>>>     __get_user_check((x), (ptr), sizeof(*(ptr)))
>>>>>     ^~~~~~~~~~~~~~~~
>>>>> ./include/asm-generic/termios-base.h:36:6: note: in expansion of macro
>>>>> ‘get_user’
>>>>>     if (get_user(termios->c_line, &termio->c_line) < 0)
>>>>>         ^~~~~~~~
>>>>> [...]
>>>>> cc1: all warnings being treated as errors
>>>>>
>>>>> Signed-off-by: Mathieu Malaterre <malat@debian.org>
>>>>> ---
>>>>>    arch/powerpc/include/asm/uaccess.h | 2 +-
>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/arch/powerpc/include/asm/uaccess.h
>>>>> b/arch/powerpc/include/asm/uaccess.h
>>>>> index 51bfeb8777f0..fadc406bd39d 100644
>>>>> --- a/arch/powerpc/include/asm/uaccess.h
>>>>> +++ b/arch/powerpc/include/asm/uaccess.h
>>>>> @@ -49,7 +49,7 @@
>>>>>    #define __access_ok(addr, size, segment)    \
>>>>>        (((addr) <= (segment).seg) &&        \
>>>>> -     (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)))))
>>>>> +     (((size) == 0) || ((size) < ((segment).seg - (addr)))))
>>>>
>>>>
>>>> IIUC, ((2 - 1) <= 1) is the same as (2 < 1) ?????
>>>
>>
>> The whole series was pretty mediocre, but this one was actually pretty
>> destructive. Thanks for catching this.
>>
>>>
>>> Note that I already try to submit a fix for this warning 3 years ago
>>> (https://patchwork.ozlabs.org/patch/418075/) and it was rejected with the
>>> following comment:
> 
> Tested again today with gcc 6.3.0 and gcc is still producing the
> original warning (treated as error).

That's right, it seems that recent versions of gcc are not happy anymore 
with that change.

Maybe Segher has a suggestion for that one ?

> 
>>> Again, I don't think Linux enables this warning.  What did you do to
>>> produce this?  In any case, it's a bad warning that doesn't take macros
>>> into account, and the answer is not to make the code less clear by hiding
>>> the fact that zero is a special case.
>>
>> Right. I'll try to see how to make W=1 run without error with an
>> alternate solution.
> 
> So the other alternative is to update a bunch of ppc32 defconfig(s)
> with: CONFIG_PPC_DISABLE_WERROR=y.
> 
> Would that be preferable ?

No I don't think it is the solution. PPC is built with WERROR in order 
to catch warnings on real problems.
You could disable it selectively when you want to run 'make W=1' and see 
all possible warnings, then select by yourself which warnings are worth 
fixing up.

Christophe

> 
>>> Christophe
>>>
>>>
>>>>
>>>> Christophe
>>>>
>>>>>    #endif
>>>>>
>>>

---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus

Re: [PATCH 06/21] powerpc: Avoid comparison of unsigned long >= 0 in __access_ok

[Segher Boessenkool]

On Mon, Feb 26, 2018 at 09:00:09PM +0100, christophe leroy wrote:
> Le 26/02/2018 à 18:50, Mathieu Malaterre a écrit :
> >On Mon, Feb 26, 2018 at 8:44 AM, Mathieu Malaterre <malat@debian.org> 
> >wrote:
> >>On Mon, Feb 26, 2018 at 7:50 AM, Christophe LEROY
> >><christophe.leroy@c-s.fr> wrote:
> >>>Note that I already try to submit a fix for this warning 3 years ago
> >>>(https://patchwork.ozlabs.org/patch/418075/) and it was rejected with the
> >>>following comment:
> >
> >Tested again today with gcc 6.3.0 and gcc is still producing the
> >original warning (treated as error).
> 
> That's right, it seems that recent versions of gcc are not happy anymore 
> with that change.
> 
> Maybe Segher has a suggestion for that one ?

Your patch:

 #define __access_ok(addr, size, segment)	\
 	(((addr) <= (segment).seg) &&		\
-	 (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)))))
+	 (((size) <= 1) || (((size) - 1) <= ((segment).seg - (addr)))))

Is there any reason to write this as a macro?  Let's make this more
readable:

static inline int __access_ok(unsigned long addr, unsigned long size,
			      mm_segment_t seg)
{
	if (addr > seg.seg)
		return 0;
	return (size == 0 || size - 1 <= seg.seg - addr);
}

and I think we are done already, or will this warn for any input?


Segher

Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

[Mathieu Malaterre]

On Mon, Feb 26, 2018 at 3:45 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Feb 26, 2018 at 4:44 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Sun, Feb 25, 2018 at 7:22 PM, Mathieu Malaterre <malat@debian.org> wrote:
>
>>>  static void __init check_cpu_feature_properties(unsigned long node)
>>>  {
>>> -       unsigned long i;
>>>         struct feature_property *fp = feature_properties;
>>>         const __be32 *prop;
>>>
>>
>> Much simpler is just add
>>
>> if (ARRAY_SIZE() == 0)
>>  return;
>>
>>> -       for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>>> +       for (; fp != feature_properties + ARRAY_SIZE(feature_properties); ++fp) {
>
> ...or convert to while(), which will be more readable.

So you'd prefer something like:

while (fp < feature_properties + ARRAY_SIZE(feature_properties)) {
  ...
  ++fp;
}

right ?

Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

[Christophe LEROY]

Le 27/02/2018 à 08:25, Mathieu Malaterre a écrit :
> On Mon, Feb 26, 2018 at 3:45 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Mon, Feb 26, 2018 at 4:44 PM, Andy Shevchenko
>> <andy.shevchenko@gmail.com> wrote:
>>> On Sun, Feb 25, 2018 at 7:22 PM, Mathieu Malaterre <malat@debian.org> wrote:
>>
>>>>   static void __init check_cpu_feature_properties(unsigned long node)
>>>>   {
>>>> -       unsigned long i;
>>>>          struct feature_property *fp = feature_properties;
>>>>          const __be32 *prop;
>>>>
>>>
>>> Much simpler is just add
>>>
>>> if (ARRAY_SIZE() == 0)
>>>   return;
>>>
>>>> -       for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>>>> +       for (; fp != feature_properties + ARRAY_SIZE(feature_properties); ++fp) {
>>
>> ...or convert to while(), which will be more readable.
> 
> So you'd prefer something like:
> 
> while (fp < feature_properties + ARRAY_SIZE(feature_properties)) {
>    ...
>    ++fp;
> }
> 
> right ?
> 


Why not do as suggested by Segher, ie just replace < by != in the 
original form ?
Or add in front:
if (!ARRAY_SIZE(feature_properties))
	return;

Christophe

Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

[Mathieu Malaterre]

On Tue, Feb 27, 2018 at 8:33 AM, Christophe LEROY
<christophe.leroy@c-s.fr> wrote:
>
>
> Le 27/02/2018 à 08:25, Mathieu Malaterre a écrit :
>>
>> On Mon, Feb 26, 2018 at 3:45 PM, Andy Shevchenko
>> <andy.shevchenko@gmail.com> wrote:
>>>
>>> On Mon, Feb 26, 2018 at 4:44 PM, Andy Shevchenko
>>> <andy.shevchenko@gmail.com> wrote:
>>>>
>>>> On Sun, Feb 25, 2018 at 7:22 PM, Mathieu Malaterre <malat@debian.org>
>>>> wrote:
>>>
>>>
>>>>>   static void __init check_cpu_feature_properties(unsigned long node)
>>>>>   {
>>>>> -       unsigned long i;
>>>>>          struct feature_property *fp = feature_properties;
>>>>>          const __be32 *prop;
>>>>>
>>>>
>>>> Much simpler is just add
>>>>
>>>> if (ARRAY_SIZE() == 0)
>>>>   return;
>>>>
>>>>> -       for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>>>>> +       for (; fp != feature_properties +
>>>>> ARRAY_SIZE(feature_properties); ++fp) {
>>>
>>>
>>> ...or convert to while(), which will be more readable.
>>
>>
>> So you'd prefer something like:
>>
>> while (fp < feature_properties + ARRAY_SIZE(feature_properties)) {
>>    ...
>>    ++fp;
>> }
>>
>> right ?
>>
>
>
> Why not do as suggested by Segher, ie just replace < by != in the original
> form ?

I can do that.

> Or add in front:
> if (!ARRAY_SIZE(feature_properties))
>         return;

(not tested) I believe the compiler still go over the for() loop and
will complain about the original unsigned comparison.

> Christophe

Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

[Andy Shevchenko]

On Tue, Feb 27, 2018 at 9:44 AM, Mathieu Malaterre <malat@debian.org> wrote:
> On Tue, Feb 27, 2018 at 8:33 AM, Christophe LEROY
> <christophe.leroy@c-s.fr> wrote:

>>>>> Much simpler is just add
>>>>>
>>>>> if (ARRAY_SIZE() == 0)
>>>>>   return;

>> Or add in front:
>> if (!ARRAY_SIZE(feature_properties))
>>         return;
>
> (not tested) I believe the compiler still go over the for() loop and
> will complain about the original unsigned comparison.

Did you run tests? Did you look into object file?

In kernel we much rely on the compiling away the code which is
deterministically not in use.
Here I'm pretty sure it will compile away entire function.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

[Mathieu Malaterre]

On Tue, Feb 27, 2018 at 4:52 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Tue, Feb 27, 2018 at 9:44 AM, Mathieu Malaterre <malat@debian.org> wrote:
>> On Tue, Feb 27, 2018 at 8:33 AM, Christophe LEROY
>> <christophe.leroy@c-s.fr> wrote:
>
>>>>>> Much simpler is just add
>>>>>>
>>>>>> if (ARRAY_SIZE() == 0)
>>>>>>   return;
>
>>> Or add in front:
>>> if (!ARRAY_SIZE(feature_properties))
>>>         return;
>>
>> (not tested) I believe the compiler still go over the for() loop and
>> will complain about the original unsigned comparison.
>
> Did you run tests? Did you look into object file?

The goal of this series is simply to remove warning treated as error.

I tried from home and I can still see the error using gcc 6.3.0, so
the original for() loop needs to be rewritten.

  CC      arch/powerpc/kernel/prom.o
arch/powerpc/kernel/prom.c: In function ‘check_cpu_feature_properties’:
arch/powerpc/kernel/prom.c:301:16: error: comparison of unsigned
expression < 0 is always false [-Werror=type-limits]
  for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
                ^
cc1: all warnings being treated as errors


> In kernel we much rely on the compiling away the code which is
> deterministically not in use.
> Here I'm pretty sure it will compile away entire function.
>
> --
> With Best Regards,
> Andy Shevchenko

Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

[Segher Boessenkool]

On Tue, Feb 27, 2018 at 05:52:06PM +0200, Andy Shevchenko wrote:
> On Tue, Feb 27, 2018 at 9:44 AM, Mathieu Malaterre <malat@debian.org> wrote:
> > On Tue, Feb 27, 2018 at 8:33 AM, Christophe LEROY
> > <christophe.leroy@c-s.fr> wrote:
> 
> >>>>> Much simpler is just add
> >>>>>
> >>>>> if (ARRAY_SIZE() == 0)
> >>>>>   return;
> 
> >> Or add in front:
> >> if (!ARRAY_SIZE(feature_properties))
> >>         return;
> >
> > (not tested) I believe the compiler still go over the for() loop and
> > will complain about the original unsigned comparison.
> 
> Did you run tests? Did you look into object file?
> 
> In kernel we much rely on the compiling away the code which is
> deterministically not in use.
> Here I'm pretty sure it will compile away entire function.

It does, but it also still warns (this warning is done very early in the
compiler pipeline).


Segher

Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

[Andy Shevchenko]

On Tue, Feb 27, 2018 at 10:42 PM, Segher Boessenkool
<segher@kernel.crashing.org> wrote:
> On Tue, Feb 27, 2018 at 05:52:06PM +0200, Andy Shevchenko wrote:
>> On Tue, Feb 27, 2018 at 9:44 AM, Mathieu Malaterre <malat@debian.org> wrote:
>> > On Tue, Feb 27, 2018 at 8:33 AM, Christophe LEROY
>> > <christophe.leroy@c-s.fr> wrote:
>>
>> >>>>> Much simpler is just add
>> >>>>>
>> >>>>> if (ARRAY_SIZE() == 0)
>> >>>>>   return;
>>
>> >> Or add in front:
>> >> if (!ARRAY_SIZE(feature_properties))
>> >>         return;
>> >
>> > (not tested) I believe the compiler still go over the for() loop and
>> > will complain about the original unsigned comparison.
>>
>> Did you run tests? Did you look into object file?
>>
>> In kernel we much rely on the compiling away the code which is
>> deterministically not in use.
>> Here I'm pretty sure it will compile away entire function.
>
> It does, but it also still warns (this warning is done very early in the
> compiler pipeline).

Oh, I see.
Then the while () approach looks to me the best here.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

[Michael Ellerman]

Mathieu Malaterre <malat@debian.org> writes:

> When neither CONFIG_ALTIVEC, nor CONFIG_VSX or CONFIG_PPC64 is defined, the
> array feature_properties is defined as an empty array, which in turn
> triggers the following warning (treated as error on W=1):
>
>   CC      arch/powerpc/kernel/prom.o
> arch/powerpc/kernel/prom.c: In function ‘check_cpu_feature_properties’:
> arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
>   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>                 ^
> cc1: all warnings being treated as errors

Ugh, that's annoying.

This seems to work?

diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 4dffef947b8a..5215119e249c 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -291,11 +291,11 @@ static inline void identical_pvr_fixup(unsigned long node)
 
 static void __init check_cpu_feature_properties(unsigned long node)
 {
-	unsigned long i;
 	struct feature_property *fp = feature_properties;
 	const __be32 *prop;
+	int i;
 
-	for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
+	for (i = 0; i < (int)ARRAY_SIZE(feature_properties); ++i, ++fp) {
 		prop = of_get_flat_dt_prop(node, fp->name, NULL);
 		if (prop && be32_to_cpup(prop) >= fp->min_value) {
 			cur_cpu_spec->cpu_features |= fp->cpu_feature;


cheers

Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

[Mathieu Malaterre]

On Thu, Mar 1, 2018 at 3:55 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Mathieu Malaterre <malat@debian.org> writes:
>
>> When neither CONFIG_ALTIVEC, nor CONFIG_VSX or CONFIG_PPC64 is defined, the
>> array feature_properties is defined as an empty array, which in turn
>> triggers the following warning (treated as error on W=1):
>>
>>   CC      arch/powerpc/kernel/prom.o
>> arch/powerpc/kernel/prom.c: In function ‘check_cpu_feature_properties’:
>> arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
>>   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>>                 ^
>> cc1: all warnings being treated as errors
>
> Ugh, that's annoying.
>
> This seems to work?
>
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 4dffef947b8a..5215119e249c 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -291,11 +291,11 @@ static inline void identical_pvr_fixup(unsigned long node)
>
>  static void __init check_cpu_feature_properties(unsigned long node)
>  {
> -       unsigned long i;
>         struct feature_property *fp = feature_properties;
>         const __be32 *prop;
> +       int i;
>
> -       for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
> +       for (i = 0; i < (int)ARRAY_SIZE(feature_properties); ++i, ++fp) {
>                 prop = of_get_flat_dt_prop(node, fp->name, NULL);
>                 if (prop && be32_to_cpup(prop) >= fp->min_value) {
>                         cur_cpu_spec->cpu_features |= fp->cpu_feature;
>

Indeed that looks like the less invasive solution, I'll re-submit.

Should I resubmit the entire patch series (21 indep patches) or
re-submit only the 3 patches that were discussed (as part of a
different series) ?

Thanks

Re: [PATCH 01/21] powerpc: Remove warning on array size when empty

[Michael Ellerman]

Mathieu Malaterre <malat@debian.org> writes:

> On Thu, Mar 1, 2018 at 3:55 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>> Mathieu Malaterre <malat@debian.org> writes:
>>
>>> When neither CONFIG_ALTIVEC, nor CONFIG_VSX or CONFIG_PPC64 is defined, the
>>> array feature_properties is defined as an empty array, which in turn
>>> triggers the following warning (treated as error on W=1):
>>>
>>>   CC      arch/powerpc/kernel/prom.o
>>> arch/powerpc/kernel/prom.c: In function ‘check_cpu_feature_properties’:
>>> arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
>>>   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>>>                 ^
>>> cc1: all warnings being treated as errors
>>
>> Ugh, that's annoying.
>>
>> This seems to work?
>>
>> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
>> index 4dffef947b8a..5215119e249c 100644
>> --- a/arch/powerpc/kernel/prom.c
>> +++ b/arch/powerpc/kernel/prom.c
>> @@ -291,11 +291,11 @@ static inline void identical_pvr_fixup(unsigned long node)
>>
>>  static void __init check_cpu_feature_properties(unsigned long node)
>>  {
>> -       unsigned long i;
>>         struct feature_property *fp = feature_properties;
>>         const __be32 *prop;
>> +       int i;
>>
>> -       for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>> +       for (i = 0; i < (int)ARRAY_SIZE(feature_properties); ++i, ++fp) {
>>                 prop = of_get_flat_dt_prop(node, fp->name, NULL);
>>                 if (prop && be32_to_cpup(prop) >= fp->min_value) {
>>                         cur_cpu_spec->cpu_features |= fp->cpu_feature;
>>
>
> Indeed that looks like the less invasive solution, I'll re-submit.

Thanks.

> Should I resubmit the entire patch series (21 indep patches) or
> re-submit only the 3 patches that were discussed (as part of a
> different series) ?

Just resubmit the ones that need changes. You can either send them as a
new series of 3, or post each as a reply to the original patch with v2
in the subject.

cheers

[PATCH v2 01/21] powerpc: Remove warning on array size when empty

[Mathieu Malaterre]

When neither CONFIG_ALTIVEC, nor CONFIG_VSX or CONFIG_PPC64 is defined, the
array feature_properties is defined as an empty array, which in turn
triggers the following warning (treated as error on W=1):

  CC      arch/powerpc/kernel/prom.o
arch/powerpc/kernel/prom.c: In function ‘check_cpu_feature_properties’:
arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
  for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
                ^
cc1: all warnings being treated as errors

Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/powerpc/kernel/prom.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index 4dffef947b8a..330c65f04820 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -291,11 +291,11 @@ static inline void identical_pvr_fixup(unsigned long node)
 
 static void __init check_cpu_feature_properties(unsigned long node)
 {
-	unsigned long i;
+	int i;
 	struct feature_property *fp = feature_properties;
 	const __be32 *prop;
 
-	for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
+	for (i = 0; i < (int)ARRAY_SIZE(feature_properties); ++i, ++fp) {
 		prop = of_get_flat_dt_prop(node, fp->name, NULL);
 		if (prop && be32_to_cpup(prop) >= fp->min_value) {
 			cur_cpu_spec->cpu_features |= fp->cpu_feature;
-- 
2.11.0

[PATCH v2 06/21] powerpc: Avoid comparison of unsigned long >= 0 in __access_ok

[Mathieu Malaterre]

Rewrite function-like macro into regular static inline function to avoid a
warning during macro expansion.
Fix warning (treated as error in W=1):

  CC      arch/powerpc/kernel/signal_32.o
In file included from ./include/linux/uaccess.h:14:0,
                 from ./include/asm-generic/termios-base.h:8,
                 from ./arch/powerpc/include/asm/termios.h:20,
                 from ./include/uapi/linux/termios.h:6,
                 from ./include/linux/tty.h:7,
                 from arch/powerpc/kernel/signal_32.c:36:
./include/asm-generic/termios-base.h: In function ‘user_termio_to_kernel_termios’:
./arch/powerpc/include/asm/uaccess.h:52:35: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
   (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)))))
                                   ^
./arch/powerpc/include/asm/uaccess.h:58:3: note: in expansion of macro ‘__access_ok’
   __access_ok((__force unsigned long)(addr), (size), get_fs()))
   ^~~~~~~~~~~
./arch/powerpc/include/asm/uaccess.h:262:6: note: in expansion of macro ‘access_ok’
  if (access_ok(VERIFY_READ, __gu_addr, (size)))   \
      ^~~~~~~~~
./arch/powerpc/include/asm/uaccess.h:80:2: note: in expansion of macro ‘__get_user_check’
  __get_user_check((x), (ptr), sizeof(*(ptr)))
  ^~~~~~~~~~~~~~~~
./include/asm-generic/termios-base.h:36:6: note: in expansion of macro ‘get_user’
  if (get_user(termios->c_line, &termio->c_line) < 0)
      ^~~~~~~~
[...]
cc1: all warnings being treated as errors

Suggested-by: Segher Boessenkool <segher@kernel.crashing.org>
Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/powerpc/include/asm/uaccess.h | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 51bfeb8777f0..a62ee663b2c8 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -47,9 +47,13 @@
 
 #else
 
-#define __access_ok(addr, size, segment)	\
-	(((addr) <= (segment).seg) &&		\
-	 (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)))))
+static inline int __access_ok(unsigned long addr, unsigned long size,
+			mm_segment_t seg)
+{
+	if (addr > seg.seg)
+		return 0;
+	return (size == 0 || size - 1 <= seg.seg - addr);
+}
 
 #endif
 
-- 
2.11.0

Re: [PATCH 05/21] powerpc: Avoid comparison of unsigned long >= 0 in pfn_valid

[Mathieu Malaterre]

On Mon, Feb 26, 2018 at 9:46 AM, Segher Boessenkool
<segher@kernel.crashing.org> wrote:
> On Mon, Feb 26, 2018 at 07:32:03AM +0100, Christophe LEROY wrote:
>> Le 25/02/2018 à 18:22, Mathieu Malaterre a écrit :
>> >-#define pfn_valid(pfn)              ((pfn) >= ARCH_PFN_OFFSET && (pfn) <
>> >max_mapnr)
>> >+#define pfn_valid(pfn) \
>> >+            (((pfn) - ARCH_PFN_OFFSET) < (max_mapnr - ARCH_PFN_OFFSET))
>>
>> What will happen when ARCH_PFN_OFFSET is not nul and pfn is lower than
>> ARCH_PFN_OFFSET ?
>
> It will work fine.
>
> Say you are asking for  a <= x < b  so (in actual integers, no overflow)
> that is  0 <= x-a < b-a  and you also assume x-a overflows, so that we
> are actually comparing  x-a+M < b-a  with M = 2**32 or such (the maximum
> value in the unsigned integer type plus one).  This comparison is
> obviously always false.
>
> (It also works if b < a btw).
>
>
Thanks Segher !

Christophe does that clarify things or do you want me to update the
commit message ?

Re: [PATCH v2 06/21] powerpc: Avoid comparison of unsigned long >= 0 in __access_ok

[christophe leroy]

Le 02/03/2018 à 20:50, Mathieu Malaterre a écrit :
> Rewrite function-like macro into regular static inline function to avoid a
> warning during macro expansion.
> Fix warning (treated as error in W=1):
> 
>    CC      arch/powerpc/kernel/signal_32.o
> In file included from ./include/linux/uaccess.h:14:0,
>                   from ./include/asm-generic/termios-base.h:8,
>                   from ./arch/powerpc/include/asm/termios.h:20,
>                   from ./include/uapi/linux/termios.h:6,
>                   from ./include/linux/tty.h:7,
>                   from arch/powerpc/kernel/signal_32.c:36:
> ./include/asm-generic/termios-base.h: In function ‘user_termio_to_kernel_termios’:
> ./arch/powerpc/include/asm/uaccess.h:52:35: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
>     (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)))))
>                                     ^
> ./arch/powerpc/include/asm/uaccess.h:58:3: note: in expansion of macro ‘__access_ok’
>     __access_ok((__force unsigned long)(addr), (size), get_fs()))
>     ^~~~~~~~~~~
> ./arch/powerpc/include/asm/uaccess.h:262:6: note: in expansion of macro ‘access_ok’
>    if (access_ok(VERIFY_READ, __gu_addr, (size)))   \
>        ^~~~~~~~~
> ./arch/powerpc/include/asm/uaccess.h:80:2: note: in expansion of macro ‘__get_user_check’
>    __get_user_check((x), (ptr), sizeof(*(ptr)))
>    ^~~~~~~~~~~~~~~~
> ./include/asm-generic/termios-base.h:36:6: note: in expansion of macro ‘get_user’
>    if (get_user(termios->c_line, &termio->c_line) < 0)
>        ^~~~~~~~
> [...]
> cc1: all warnings being treated as errors
> 
> Suggested-by: Segher Boessenkool <segher@kernel.crashing.org>
> Signed-off-by: Mathieu Malaterre <malat@debian.org>

Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>

> ---
>   arch/powerpc/include/asm/uaccess.h | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
> index 51bfeb8777f0..a62ee663b2c8 100644
> --- a/arch/powerpc/include/asm/uaccess.h
> +++ b/arch/powerpc/include/asm/uaccess.h
> @@ -47,9 +47,13 @@
>   
>   #else
>   
> -#define __access_ok(addr, size, segment)	\
> -	(((addr) <= (segment).seg) &&		\
> -	 (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)))))
> +static inline int __access_ok(unsigned long addr, unsigned long size,
> +			mm_segment_t seg)
> +{
> +	if (addr > seg.seg)
> +		return 0;
> +	return (size == 0 || size - 1 <= seg.seg - addr);
> +}
>   
>   #endif
>   
> 

---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus

Re: [PATCH 05/21] powerpc: Avoid comparison of unsigned long >= 0 in pfn_valid

[christophe leroy]

Le 02/03/2018 à 20:54, Mathieu Malaterre a écrit :
> On Mon, Feb 26, 2018 at 9:46 AM, Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
>> On Mon, Feb 26, 2018 at 07:32:03AM +0100, Christophe LEROY wrote:
>>> Le 25/02/2018 à 18:22, Mathieu Malaterre a écrit :
>>>> -#define pfn_valid(pfn)              ((pfn) >= ARCH_PFN_OFFSET && (pfn) <
>>>> max_mapnr)
>>>> +#define pfn_valid(pfn) \
>>>> +            (((pfn) - ARCH_PFN_OFFSET) < (max_mapnr - ARCH_PFN_OFFSET))
>>>
>>> What will happen when ARCH_PFN_OFFSET is not nul and pfn is lower than
>>> ARCH_PFN_OFFSET ?
>>
>> It will work fine.
>>
>> Say you are asking for  a <= x < b  so (in actual integers, no overflow)
>> that is  0 <= x-a < b-a  and you also assume x-a overflows, so that we
>> are actually comparing  x-a+M < b-a  with M = 2**32 or such (the maximum
>> value in the unsigned integer type plus one).  This comparison is
>> obviously always false.
>>
>> (It also works if b < a btw).
>>
>>
> Thanks Segher !
> 
> Christophe does that clarify things or do you want me to update the
> commit message ?
> 

No it is fine for me.

Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>

---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus

Re: [PATCH 03/21] powerpc: Mark the variable earlycon_acpi_spcr_enable maybe_unused

[Michael Ellerman]

Mathieu Malaterre <malat@debian.org> writes:

> Re-use the object-like macro EARLYCON_USED_OR_UNUSED to mark
> `earlycon_acpi_spcr_enable` as maybe_unused.
>
> Fix the following warning (treated as error in W=1)
>
>   CC      arch/powerpc/kernel/setup-common.o
> In file included from ./include/linux/serial_8250.h:14:0,
>                  from arch/powerpc/kernel/setup-common.c:33:
> ./include/linux/serial_core.h:382:19: error: ‘earlycon_acpi_spcr_enable’ defined but not used [-Werror=unused-const-variable=]
>  static const bool earlycon_acpi_spcr_enable;
>                    ^~~~~~~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
>
> Signed-off-by: Mathieu Malaterre <malat@debian.org>
> ---
>  include/linux/serial_core.h | 1 +

I can't take this one as that's not a file I maintain.

The script says:

  $ ./scripts/get_maintainer.pl include/linux/serial_core.h
  gregkh@linuxfoundation.org
  jslaby@suse.com
  linux-kernel@vger.kernel.org


Can you resend it to them?

> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index b32df49a3bd5..4d14ecd7dbe8 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -379,6 +379,7 @@ extern int of_setup_earlycon(const struct earlycon_id *match,
>  extern bool earlycon_acpi_spcr_enable __initdata;
>  int setup_earlycon(char *buf);
>  #else
> +EARLYCON_USED_OR_UNUSED
>  static const bool earlycon_acpi_spcr_enable;

The macro eventually turns into an __attribute__, which I think is
typically placed after the variable, so eg:

  static const bool earlycon_acpi_spcr_enable EARLYCON_USED_OR_UNUSED;


cheers

>  static inline int setup_earlycon(char *buf) { return 0; }
>  #endif
> -- 
> 2.11.0

Re: [PATCH 15/21] powerpc: Add missing prototype for MMU_setup

[Michael Ellerman]

Mathieu Malaterre <malat@debian.org> writes:
> Add a function declaration for MMU_setup at the beginning of the file to
> fix a warning (treated as error in W=1):
>
>   CC      kernel/sys.o
> arch/powerpc/mm/init_32.c:102:13: error: no previous prototype for ‘MMU_setup’ [-Werror=missing-prototypes]
>  void __init MMU_setup(void)
>              ^~~~~~~~~
> cc1: all warnings being treated as errors

Can't it be static instead?

  $ git grep -n MMU_setup
  arch/powerpc/mm/init_32.c:102:void __init MMU_setup(void)
  arch/powerpc/mm/init_32.c:135:  MMU_setup();

cheers

Re: [PATCH 17/21] powerpc: Add missing prototype for sys_debug_setcontext

[Michael Ellerman]

Mathieu Malaterre <malat@debian.org> writes:

> In commit 81e7009ea46c ("powerpc: merge ppc signal.c and ppc64 signal32.c")
> the function sys_debug_setcontext was added without a prototype.
>
> Fix compilation warning (treated as error in W=1):
>
>   CC      arch/powerpc/kernel/signal_32.o
> arch/powerpc/kernel/signal_32.c:1227:5: error: no previous prototype for ‘sys_debug_setcontext’ [-Werror=missing-prototypes]
>  int sys_debug_setcontext(struct ucontext __user *ctx,
>      ^~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors

This one should actually be using the SYSCALL_DEFINE syntax, so that it
can be used with CONFIG_FTRACE_SYSCALLS.

See eg. our mmap:

  SYSCALL_DEFINE6(mmap, unsigned long, addr, size_t, len,
  		unsigned long, prot, unsigned long, flags,
  		unsigned long, fd, off_t, offset)
  {
  	return do_mmap2(addr, len, prot, flags, fd, offset, PAGE_SHIFT);
  }


We probably still need this patch, but I'm not entirely sure because the
SYSCALL_DEFINE macro does all sorts of shenanigans.

cheers

Re: [PATCH 05/21] powerpc: Avoid comparison of unsigned long >= 0 in pfn_valid

[Michael Ellerman]

Mathieu Malaterre <malat@debian.org> writes:

> Rewrite comparison since all values compared are of type `unsigned long`.
>
> Fix a warning (treated as error in W=1):
>
>   CC      arch/powerpc/kernel/irq.o
> In file included from ./include/linux/bug.h:5:0,
>                  from ./include/linux/cpumask.h:13,
>                  from ./include/linux/smp.h:13,
>                  from ./include/linux/kernel_stat.h:5,
>                  from arch/powerpc/kernel/irq.c:35:
> ./include/linux/dma-mapping.h: In function ‘dma_map_resource’:
> ./arch/powerpc/include/asm/page.h:129:32: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
>  #define pfn_valid(pfn)  ((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr)
>                                 ^
> Suggested-by: Segher Boessenkool <segher@kernel.crashing.org>
> Signed-off-by: Mathieu Malaterre <malat@debian.org>
> ---
>  arch/powerpc/include/asm/page.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
> index 8da5d4c1cab2..19dea64e7ed2 100644
> --- a/arch/powerpc/include/asm/page.h
> +++ b/arch/powerpc/include/asm/page.h
> @@ -126,7 +126,8 @@ extern long long virt_phys_offset;
>  
>  #ifdef CONFIG_FLATMEM
>  #define ARCH_PFN_OFFSET		((unsigned long)(MEMORY_START >> PAGE_SHIFT))
> -#define pfn_valid(pfn)		((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr)
> +#define pfn_valid(pfn) \
> +		(((pfn) - ARCH_PFN_OFFSET) < (max_mapnr - ARCH_PFN_OFFSET))

I'm not a big fan of this one, because the original code is *far* more
obvious as to what it's doing.

I'm not sure if we can make this one a static inline, or whether that
would help, but it would be worth investigating.

cheers

Re: [PATCH 05/21] powerpc: Avoid comparison of unsigned long >= 0 in pfn_valid

[christophe leroy]

Le 04/03/2018 à 11:55, Michael Ellerman a écrit :
> Mathieu Malaterre <malat@debian.org> writes:
> 
>> Rewrite comparison since all values compared are of type `unsigned long`.
>>
>> Fix a warning (treated as error in W=1):
>>
>>    CC      arch/powerpc/kernel/irq.o
>> In file included from ./include/linux/bug.h:5:0,
>>                   from ./include/linux/cpumask.h:13,
>>                   from ./include/linux/smp.h:13,
>>                   from ./include/linux/kernel_stat.h:5,
>>                   from arch/powerpc/kernel/irq.c:35:
>> ./include/linux/dma-mapping.h: In function ‘dma_map_resource’:
>> ./arch/powerpc/include/asm/page.h:129:32: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
>>   #define pfn_valid(pfn)  ((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr)
>>                                  ^
>> Suggested-by: Segher Boessenkool <segher@kernel.crashing.org>
>> Signed-off-by: Mathieu Malaterre <malat@debian.org>
>> ---
>>   arch/powerpc/include/asm/page.h | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
>> index 8da5d4c1cab2..19dea64e7ed2 100644
>> --- a/arch/powerpc/include/asm/page.h
>> +++ b/arch/powerpc/include/asm/page.h
>> @@ -126,7 +126,8 @@ extern long long virt_phys_offset;
>>   
>>   #ifdef CONFIG_FLATMEM
>>   #define ARCH_PFN_OFFSET		((unsigned long)(MEMORY_START >> PAGE_SHIFT))
>> -#define pfn_valid(pfn)		((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr)
>> +#define pfn_valid(pfn) \
>> +		(((pfn) - ARCH_PFN_OFFSET) < (max_mapnr - ARCH_PFN_OFFSET))
> 
> I'm not a big fan of this one, because the original code is *far* more
> obvious as to what it's doing.
> 
> I'm not sure if we can make this one a static inline, or whether that
> would help, but it would be worth investigating.
> 

The following seems to give a good result:

diff --git a/arch/powerpc/include/asm/page.h 
b/arch/powerpc/include/asm/page.h
index 8da5d4c1cab2..6f74938483b7 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -126,7 +126,15 @@ extern long long virt_phys_offset;

  #ifdef CONFIG_FLATMEM
  #define ARCH_PFN_OFFSET		((unsigned long)(MEMORY_START >> PAGE_SHIFT))
-#define pfn_valid(pfn)		((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr)
+#ifndef __ASSEMBLY__
+extern unsigned long max_mapnr;
+static inline bool pfn_valid(unsigned long pfn)
+{
+	unsigned long min_pfn = ARCH_PFN_OFFSET;
+
+	return pfn >= min_pfn && pfn < max_mapnr;
+}
+#endif
  #endif

  #define virt_to_pfn(kaddr)	(__pa(kaddr) >> PAGE_SHIFT)


Christophe

---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus

[PATCH v2 15/21] powerpc: Make function MMU_setup static

[Mathieu Malaterre]

Since function `MMU_setup` is not meant to be exported, change the
signature to `static`. Fix warning (treated as error with W=1):

  CC      kernel/sys.o
arch/powerpc/mm/init_32.c:102:13: error: no previous prototype for ‘MMU_setup’ [-Werror=missing-prototypes]
 void __init MMU_setup(void)
             ^~~~~~~~~
cc1: all warnings being treated as errors

Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/powerpc/mm/init_32.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/mm/init_32.c b/arch/powerpc/mm/init_32.c
index 6419b33ca309..a2bf6965d04f 100644
--- a/arch/powerpc/mm/init_32.c
+++ b/arch/powerpc/mm/init_32.c
@@ -99,7 +99,7 @@ unsigned long __max_low_memory = MAX_LOW_MEM;
 /*
  * Check for command-line options that affect what MMU_init will do.
  */
-void __init MMU_setup(void)
+static void __init MMU_setup(void)
 {
 	/* Check for nobats option (used in mapin_ram). */
 	if (strstr(boot_command_line, "nobats")) {
-- 
2.11.0

[PATCH v2 05/21] powerpc: Avoid comparison of unsigned long >= 0 in pfn_valid

[Mathieu Malaterre]

Rewrite comparison since all values compared are of type `unsigned long`.

Instead of using unsigned properties and rewriting the original code as:
(originally suggested by Segher Boessenkool <segher@kernel.crashing.org>)

  #define pfn_valid(pfn) \
               (((pfn) - ARCH_PFN_OFFSET) < (max_mapnr - ARCH_PFN_OFFSET))

Prefer a static inline function to make code as readable as possible.

Fix a warning (treated as error in W=1):

  CC      arch/powerpc/kernel/irq.o
In file included from ./include/linux/bug.h:5:0,
                 from ./include/linux/cpumask.h:13,
                 from ./include/linux/smp.h:13,
                 from ./include/linux/kernel_stat.h:5,
                 from arch/powerpc/kernel/irq.c:35:
./include/linux/dma-mapping.h: In function ‘dma_map_resource’:
./arch/powerpc/include/asm/page.h:129:32: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
 #define pfn_valid(pfn)  ((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr)
                                ^
Suggested-by: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
 arch/powerpc/include/asm/page.h | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index 8da5d4c1cab2..6f74938483b7 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -126,7 +126,15 @@ extern long long virt_phys_offset;
 
 #ifdef CONFIG_FLATMEM
 #define ARCH_PFN_OFFSET		((unsigned long)(MEMORY_START >> PAGE_SHIFT))
-#define pfn_valid(pfn)		((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr)
+#ifndef __ASSEMBLY__
+extern unsigned long max_mapnr;
+static inline bool pfn_valid(unsigned long pfn)
+{
+	unsigned long min_pfn = ARCH_PFN_OFFSET;
+
+	return pfn >= min_pfn && pfn < max_mapnr;
+}
+#endif
 #endif
 
 #define virt_to_pfn(kaddr)	(__pa(kaddr) >> PAGE_SHIFT)
-- 
2.11.0

Re: [PATCH 17/21] powerpc: Add missing prototype for sys_debug_setcontext

[Mathieu Malaterre]

On Sun, Mar 4, 2018 at 11:54 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Mathieu Malaterre <malat@debian.org> writes:
>
>> In commit 81e7009ea46c ("powerpc: merge ppc signal.c and ppc64 signal32.c")
>> the function sys_debug_setcontext was added without a prototype.
>>
>> Fix compilation warning (treated as error in W=1):
>>
>>   CC      arch/powerpc/kernel/signal_32.o
>> arch/powerpc/kernel/signal_32.c:1227:5: error: no previous prototype for ‘sys_debug_setcontext’ [-Werror=missing-prototypes]
>>  int sys_debug_setcontext(struct ucontext __user *ctx,
>>      ^~~~~~~~~~~~~~~~~~~~
>> cc1: all warnings being treated as errors
>
> This one should actually be using the SYSCALL_DEFINE syntax, so that it
> can be used with CONFIG_FTRACE_SYSCALLS.
>
> See eg. our mmap:
>
>   SYSCALL_DEFINE6(mmap, unsigned long, addr, size_t, len,
>                 unsigned long, prot, unsigned long, flags,
>                 unsigned long, fd, off_t, offset)
>   {
>         return do_mmap2(addr, len, prot, flags, fd, offset, PAGE_SHIFT);
>   }
>
>
> We probably still need this patch, but I'm not entirely sure because the
> SYSCALL_DEFINE macro does all sorts of shenanigans.

I see. Could you please drop this patch then. The patch does not look
that trivial anymore. I'll need to dig a bit more on how to do the
syscall stuff with a 7 params function.

Thanks

Re: [PATCH 17/21] powerpc: Add missing prototype for sys_debug_setcontext

[Michael Ellerman]

Mathieu Malaterre <malat@debian.org> writes:

> On Sun, Mar 4, 2018 at 11:54 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>> Mathieu Malaterre <malat@debian.org> writes:
>>
>>> In commit 81e7009ea46c ("powerpc: merge ppc signal.c and ppc64 signal32.c")
>>> the function sys_debug_setcontext was added without a prototype.
>>>
>>> Fix compilation warning (treated as error in W=1):
>>>
>>>   CC      arch/powerpc/kernel/signal_32.o
>>> arch/powerpc/kernel/signal_32.c:1227:5: error: no previous prototype for ‘sys_debug_setcontext’ [-Werror=missing-prototypes]
>>>  int sys_debug_setcontext(struct ucontext __user *ctx,
>>>      ^~~~~~~~~~~~~~~~~~~~
>>> cc1: all warnings being treated as errors
>>
>> This one should actually be using the SYSCALL_DEFINE syntax, so that it
>> can be used with CONFIG_FTRACE_SYSCALLS.
>>
>> See eg. our mmap:
>>
>>   SYSCALL_DEFINE6(mmap, unsigned long, addr, size_t, len,
>>                 unsigned long, prot, unsigned long, flags,
>>                 unsigned long, fd, off_t, offset)
>>   {
>>         return do_mmap2(addr, len, prot, flags, fd, offset, PAGE_SHIFT);
>>   }
>>
>>
>> We probably still need this patch, but I'm not entirely sure because the
>> SYSCALL_DEFINE macro does all sorts of shenanigans.
>
> I see. Could you please drop this patch then. The patch does not look
> that trivial anymore. I'll need to dig a bit more on how to do the
> syscall stuff with a 7 params function.

Ergh, yuck, seems we're the first suckers to need do that.

I think I'll take this patch for now, it's still good for now at least,
and then the SYSCALL_DEFINE stuff can be an addition.

cheers

Re: [PATCH 21/21] powerpc: Add missing prototypes in setup_32.c

[Michael Ellerman]

Mathieu Malaterre <malat@debian.org> writes:

> This commit add the prototypes for the following function:
>
> - early_init
> - machine_init

I'd rather these were in asm-prototypes.h, it's the header for functions
called from asm. I made that change.

> - ppc_setup_l2cr
> - ppc_setup_l3cr
> - ppc_init

These three can just be static. So I did that.

cheers

Re: [PATCH 03/21] powerpc: Mark the variable earlycon_acpi_spcr_enable maybe_unused

[Mathieu Malaterre]

On Sun, Mar 4, 2018 at 11:54 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Mathieu Malaterre <malat@debian.org> writes:
>
>> Re-use the object-like macro EARLYCON_USED_OR_UNUSED to mark
>> `earlycon_acpi_spcr_enable` as maybe_unused.
>>
>> Fix the following warning (treated as error in W=1)
>>
>>   CC      arch/powerpc/kernel/setup-common.o
>> In file included from ./include/linux/serial_8250.h:14:0,
>>                  from arch/powerpc/kernel/setup-common.c:33:
>> ./include/linux/serial_core.h:382:19: error: ‘earlycon_acpi_spcr_enable’ defined but not used [-Werror=unused-const-variable=]
>>  static const bool earlycon_acpi_spcr_enable;
>>                    ^~~~~~~~~~~~~~~~~~~~~~~~~
>> cc1: all warnings being treated as errors
>>
>> Signed-off-by: Mathieu Malaterre <malat@debian.org>
>> ---
>>  include/linux/serial_core.h | 1 +
>
> I can't take this one as that's not a file I maintain.
>
> The script says:
>
>   $ ./scripts/get_maintainer.pl include/linux/serial_core.h
>   gregkh@linuxfoundation.org
>   jslaby@suse.com
>   linux-kernel@vger.kernel.org
>
>
> Can you resend it to them?

Ah right, thanks.
>
>> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
>> index b32df49a3bd5..4d14ecd7dbe8 100644
>> --- a/include/linux/serial_core.h
>> +++ b/include/linux/serial_core.h
>> @@ -379,6 +379,7 @@ extern int of_setup_earlycon(const struct earlycon_id *match,
>>  extern bool earlycon_acpi_spcr_enable __initdata;
>>  int setup_earlycon(char *buf);
>>  #else
>> +EARLYCON_USED_OR_UNUSED
>>  static const bool earlycon_acpi_spcr_enable;
>
> The macro eventually turns into an __attribute__, which I think is
> typically placed after the variable, so eg:
>
>   static const bool earlycon_acpi_spcr_enable EARLYCON_USED_OR_UNUSED;
>

Indeed makes it more consistent with style.  Thanks!

> cheers
>
>>  static inline int setup_earlycon(char *buf) { return 0; }
>>  #endif
>> --
>> 2.11.0

Re: [02/21] powerpc: Move the inline keyword at the beginning of function declaration

[Michael Ellerman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 844 bytes --]

On Sun, 2018-02-25 at 17:22:17 UTC, Mathieu Malaterre wrote:
> The inline keyword was not at the beginning of the function declaration.
> Fix the following warning (treated as error in W=1)
> 
>   CC      kernel/sys.o
> arch/powerpc/lib/sstep.c:283:1: error: ‘inline’ is not at beginning of declaration [-Werror=old-style-declaration]
>  static int nokprobe_inline copy_mem_in(u8 *dest, unsigned long ea, int nb,
>  ^~~~~~
> arch/powerpc/lib/sstep.c:388:1: error: ‘inline’ is not at beginning of declaration [-Werror=old-style-declaration]
>  static int nokprobe_inline copy_mem_out(u8 *dest, unsigned long ea, int nb,
>  ^~~~~~
>   CC      arch/powerpc/kernel/setup_32.o
> 
> Signed-off-by: Mathieu Malaterre <malat@debian.org>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/174b701d3dcd14514f869e2bc08ac4

cheers

Re: [04/21] powerpc: Mark both tmp variables as unused

[Michael Ellerman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 929 bytes --]

On Sun, 2018-02-25 at 17:22:19 UTC, Mathieu Malaterre wrote:
> Since the value of `tmp` is never intended to be read, declare both `tmp`
> variables as unused. Fix warning (treated as error in W=1):
> 
>   CC      arch/powerpc/kernel/signal_32.o
> arch/powerpc/kernel/signal_32.c: In function ‘sys_swapcontext’:
> arch/powerpc/kernel/signal_32.c:1048:16: error: variable ‘tmp’ set but not used [-Werror=unused-but-set-variable]
>   unsigned char tmp;
>                 ^~~
> arch/powerpc/kernel/signal_32.c: In function ‘sys_debug_setcontext’:
> arch/powerpc/kernel/signal_32.c:1234:16: error: variable ‘tmp’ set but not used [-Werror=unused-but-set-variable]
>   unsigned char tmp;
>                 ^~~
> cc1: all warnings being treated as errors
> 
> Signed-off-by: Mathieu Malaterre <malat@debian.org>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/67b464a89c21c9edd45ad15c457bb5

cheers

Re: [07/21] powerpc: Make functions flipper_pic_init & ug_udbg_putc static

[Michael Ellerman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 997 bytes --]

On Sun, 2018-02-25 at 17:22:22 UTC, Mathieu Malaterre wrote:
> Change signature of two functions, adding static keyword to prevent the
> following two warnings (treated as errors on W=1):
> 
>   CC      kernel/sys.o
> arch/powerpc/platforms/embedded6xx/flipper-pic.c:135:28: error: no previous prototype for ‘flipper_pic_init’ [-Werror=missing-prototypes]
>  struct irq_domain * __init flipper_pic_init(struct device_node *np)
>                             ^~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> 
> and
> 
>   CC      arch/powerpc/platforms/embedded6xx/usbgecko_udbg.o
> arch/powerpc/platforms/embedded6xx/usbgecko_udbg.c:172:6: error: no previous prototype for ‘ug_udbg_putc’ [-Werror=missing-prototypes]
>  void ug_udbg_putc(char ch)
>       ^~~~~~~~~~~~
> cc1: all warnings being treated as errors
> 
> Signed-off-by: Mathieu Malaterre <malat@debian.org>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/8b51e679a54e808bdf1f2cc6552cf2

cheers

Re: [10/21] powerpc: Add missing prototype for slb_miss_bad_addr

[Michael Ellerman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 751 bytes --]

On Sun, 2018-02-25 at 17:22:25 UTC, Mathieu Malaterre wrote:
> In commit f0f558b131db ("powerpc/mm: Preserve CFAR value on SLB miss caused
> by access to bogus address"), the function slb_miss_bad_addr was added
> without a prototype. This commit adds it.
> 
> Fix a warning (treated as error in W=1):
> 
>   CC      arch/powerpc/kernel/traps.o
> arch/powerpc/kernel/traps.c:1498:6: error: no previous prototype for ‘slb_miss_bad_addr’ [-Werror=missing-prototypes]
>  void slb_miss_bad_addr(struct pt_regs *regs)
>       ^~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> 
> Signed-off-by: Mathieu Malaterre <malat@debian.org>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/45b4d27a3897d6094bcf84bc877439

cheers

Re: [11/21] powerpc: Add missing prototype for hdec_interrupt

[Michael Ellerman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 701 bytes --]

On Sun, 2018-02-25 at 17:22:26 UTC, Mathieu Malaterre wrote:
> In commit dabe859ec636 ("powerpc: Give hypervisor decrementer interrupts
> their own handler") an empty body function was added, but no prototype was
> declared. Fix warning (treated as error in W=1):
> 
>   CC      arch/powerpc/kernel/time.o
> arch/powerpc/kernel/time.c:629:6: error: no previous prototype for ‘hdec_interrupt’ [-Werror=missing-prototypes]
>  void hdec_interrupt(struct pt_regs *regs)
>       ^~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> 
> Signed-off-by: Mathieu Malaterre <malat@debian.org>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/8b604faff7d421904ebd1fa65d642f

cheers

Re: [12/21] powerpc: Add missing prototype for time_init

[Michael Ellerman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 618 bytes --]

On Sun, 2018-02-25 at 17:22:27 UTC, Mathieu Malaterre wrote:
> The function time_init did not have a prototype defined in the time.h
> header. Fix the following warning (treated as error in W=1):
> 
>   CC      arch/powerpc/kernel/time.o
> arch/powerpc/kernel/time.c:1068:13: error: no previous prototype for ‘time_init’ [-Werror=missing-prototypes]
>  void __init time_init(void)
>              ^~~~~~~~~
> cc1: all warnings being treated as errors
> 
> Signed-off-by: Mathieu Malaterre <malat@debian.org>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/848092faa0c7687a99bf465808f7da

cheers

Re: [13/21] powerpc: Add missing prototype for arch_dup_task_struct

[Michael Ellerman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 798 bytes --]

On Sun, 2018-02-25 at 17:22:28 UTC, Mathieu Malaterre wrote:
> In commit 55ccf3fe3f9a ("fork: move the real prepare_to_copy() users to
> arch_dup_task_struct()") a new arch_dup_task_struct was added without a
> prototype declared in thread_info.h header. Fix the following warning
> (treated as error in W=1):
> 
>   CC      arch/powerpc/kernel/process.o
> arch/powerpc/kernel/process.c:1609:5: error: no previous prototype for ‘arch_dup_task_struct’ [-Werror=missing-prototypes]
>  int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
>      ^~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> 
> Signed-off-by: Mathieu Malaterre <malat@debian.org>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/fd70d9f96d8182a67e28b99c999157

cheers

Re: [14/21] powerpc: Add missing prototype for arch_irq_work_raise

[Michael Ellerman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 706 bytes --]

On Sun, 2018-02-25 at 17:22:29 UTC, Mathieu Malaterre wrote:
> In commit 4f8b50bbbe63 ("irq_work, ppc: Fix up arch hooks") a new function
> arch_irq_work_raise was added without a prototype in header irq_work.h.
> 
> Fix the following warning (treated as error in W=1):
> 
>   CC      arch/powerpc/kernel/time.o
> arch/powerpc/kernel/time.c:523:6: error: no previous prototype for ‘arch_irq_work_raise’ [-Werror=missing-prototypes]
>  void arch_irq_work_raise(void)
>       ^~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> 
> Signed-off-by: Mathieu Malaterre <malat@debian.org>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/f5246862f82f1e16bbf84cda4cddf2

cheers

Re: [16/21] powerpc: Add missing prototype for init_IRQ

[Michael Ellerman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 610 bytes --]

On Sun, 2018-02-25 at 17:22:31 UTC, Mathieu Malaterre wrote:
> A function init_IRQ was added without a prototype declared in header irq.h.
> Fix the following warning (treated as error in W=1):
> 
>   CC      arch/powerpc/kernel/irq.o
> arch/powerpc/kernel/irq.c:662:13: error: no previous prototype for ‘init_IRQ’ [-Werror=missing-prototypes]
>  void __init init_IRQ(void)
>              ^~~~~~~~
> cc1: all warnings being treated as errors
> 
> Signed-off-by: Mathieu Malaterre <malat@debian.org>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/23a6d8b9634897add6ebff32372f34

cheers

Re: [17/21] powerpc: Add missing prototype for sys_debug_setcontext

[Michael Ellerman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 733 bytes --]

On Sun, 2018-02-25 at 17:22:32 UTC, Mathieu Malaterre wrote:
> In commit 81e7009ea46c ("powerpc: merge ppc signal.c and ppc64 signal32.c")
> the function sys_debug_setcontext was added without a prototype.
> 
> Fix compilation warning (treated as error in W=1):
> 
>   CC      arch/powerpc/kernel/signal_32.o
> arch/powerpc/kernel/signal_32.c:1227:5: error: no previous prototype for ‘sys_debug_setcontext’ [-Werror=missing-prototypes]
>  int sys_debug_setcontext(struct ucontext __user *ctx,
>      ^~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> 
> Signed-off-by: Mathieu Malaterre <malat@debian.org>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/0d60619e1c0ca20eb2610361034992

cheers

Re: [18/21] powerpc: Add missing prototypes for sys_sigreturn & sys_rt_sigreturn

[Michael Ellerman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 887 bytes --]

On Sun, 2018-02-25 at 17:22:33 UTC, Mathieu Malaterre wrote:
> Two functions did not have a prototype defined in signal.h header. Fix the
> following two warnings (treated as errors in W=1):
> 
>   CC      arch/powerpc/kernel/signal_32.o
> arch/powerpc/kernel/signal_32.c:1135:6: error: no previous prototype for ‘sys_rt_sigreturn’ [-Werror=missing-prototypes]
>  long sys_rt_sigreturn(int r3, int r4, int r5, int r6, int r7, int r8,
>       ^~~~~~~~~~~~~~~~
> arch/powerpc/kernel/signal_32.c:1422:6: error: no previous prototype for ‘sys_sigreturn’ [-Werror=missing-prototypes]
>  long sys_sigreturn(int r3, int r4, int r5, int r6, int r7, int r8,
>       ^~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> 
> Signed-off-by: Mathieu Malaterre <malat@debian.org>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/b53875c4b4f212a7a8e505e2d10635

cheers

Re: [19/21] powerpc: Add missing prototypes for hw_breakpoint_handler & arch_unregister_hw_breakpoint

[Michael Ellerman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1064 bytes --]

On Sun, 2018-02-25 at 17:22:34 UTC, Mathieu Malaterre wrote:
> In commit 5aae8a537080 ("powerpc, hw_breakpoints: Implement hw_breakpoints
> for 64-bit server processors") function hw_breakpoint_handler and
> arch_unregister_hw_breakpoint were added without function prototypes in
> hw_breakpoint.h header.
> 
> Fix the following warning(s) (treated as error in W=1):
> 
>   AR      init/built-in.o
> arch/powerpc/kernel/hw_breakpoint.c:106:6: error: no previous prototype for ‘arch_unregister_hw_breakpoint’ [-Werror=missing-prototypes]
>  void arch_unregister_hw_breakpoint(struct perf_event *bp)
>       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> arch/powerpc/kernel/hw_breakpoint.c:209:5: error: no previous prototype for ‘hw_breakpoint_handler’ [-Werror=missing-prototypes]
>  int hw_breakpoint_handler(struct die_args *args)
>      ^~~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> 
> Signed-off-by: Mathieu Malaterre <malat@debian.org>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/b0d876da1d1cd33a1c28049512a136

cheers

Re: [20/21] powerpc: Add missing prototypes for ppc_select & ppc_fadvise64_64

[Michael Ellerman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 928 bytes --]

On Sun, 2018-02-25 at 17:22:35 UTC, Mathieu Malaterre wrote:
> Add missing prototypes for ppc_select & ppc_fadvise64_64 to header
> asm-prototypes.h. Fix the following warnings (treated as errors in W=1)
> 
>   CC      arch/powerpc/kernel/syscalls.o
> arch/powerpc/kernel/syscalls.c:87:1: error: no previous prototype for ‘ppc_select’ [-Werror=missing-prototypes]
>  ppc_select(int n, fd_set __user *inp, fd_set __user *outp, fd_set __user *exp, struct timeval __user *tvp)
>  ^~~~~~~~~~
> arch/powerpc/kernel/syscalls.c:119:6: error: no previous prototype for ‘ppc_fadvise64_64’ [-Werror=missing-prototypes]
>  long ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32 offset_low,
>       ^~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> 
> Signed-off-by: Mathieu Malaterre <malat@debian.org>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/bf7fb32dd5fc8a6bd25aaab05d3acd

cheers

Re: [08/21] powerpc: Make function __giveup_fpu static

[Michael Ellerman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 735 bytes --]

On Sun, 2018-02-25 at 17:22:23 UTC, Mathieu Malaterre wrote:
> When CONFIG_PPC_FPU is defined the prototype exposed in asm/switch_to.h is:
> 
>   extern void giveup_fpu(struct task_struct *);
> 
> Change the function prototype of __giveup_fpu to static. Fix warning
> (treated as error in W=1):
> 
>   CC      arch/powerpc/kernel/process.o
> arch/powerpc/kernel/process.c:176:6: error: no previous prototype for ‘__giveup_fpu’ [-Werror=missing-prototypes]
>  void __giveup_fpu(struct task_struct *tsk)
>       ^~~~~~~~~~~~
> cc1: all warnings being treated as errors
> 
> Signed-off-by: Mathieu Malaterre <malat@debian.org>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/1cdf039bf82a39d816ca4b8161b01c

cheers

Re: [21/21] powerpc: Add missing prototypes in setup_32.c

[Michael Ellerman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2173 bytes --]

On Sun, 2018-02-25 at 17:22:36 UTC, Mathieu Malaterre wrote:
> This commit add the prototypes for the following function:
> 
> - early_init
> - machine_init
> - ppc_setup_l2cr
> - ppc_setup_l3cr
> - ppc_init
> 
> the other missing ones were already added in setup.h previously. Fix the
> following warnings (treated as error in W=1):
> 
>   AR      init/built-in.o
> arch/powerpc/kernel/setup_32.c:68:30: error: no previous prototype for ‘early_init’ [-Werror=missing-prototypes]
>  notrace unsigned long __init early_init(unsigned long dt_ptr)
>                               ^~~~~~~~~~
> arch/powerpc/kernel/setup_32.c:99:21: error: no previous prototype for ‘machine_init’ [-Werror=missing-prototypes]
>  notrace void __init machine_init(u64 dt_ptr)
>                      ^~~~~~~~~~~~
> arch/powerpc/kernel/setup_32.c:124:12: error: no previous prototype for ‘ppc_setup_l2cr’ [-Werror=missing-prototypes]
>  int __init ppc_setup_l2cr(char *str)
>             ^~~~~~~~~~~~~~
> arch/powerpc/kernel/setup_32.c:137:12: error: no previous prototype for ‘ppc_setup_l3cr’ [-Werror=missing-prototypes]
>  int __init ppc_setup_l3cr(char *str)
>             ^~~~~~~~~~~~~~
> arch/powerpc/kernel/setup_32.c:183:12: error: no previous prototype for ‘ppc_init’ [-Werror=missing-prototypes]
>  int __init ppc_init(void)
>             ^~~~~~~~
> arch/powerpc/kernel/setup_32.c:198:13: error: no previous prototype for ‘irqstack_early_init’ [-Werror=missing-prototypes]
>  void __init irqstack_early_init(void)
>              ^~~~~~~~~~~~~~~~~~~
> arch/powerpc/kernel/setup_32.c:238:13: error: no previous prototype for ‘setup_power_save’ [-Werror=missing-prototypes]
>  void __init setup_power_save(void)
>              ^~~~~~~~~~~~~~~~
> arch/powerpc/kernel/setup_32.c:253:13: error: no previous prototype for ‘initialize_cache_info’ [-Werror=missing-prototypes]
>  __init void initialize_cache_info(void)
>              ^~~~~~~~~~~~~~~~~~~~~
> cc1: all warnings being treated as errors
> 
> Signed-off-by: Mathieu Malaterre <malat@debian.org>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/e82d70cf965072a3872ea97b7d8df4

cheers

Re: [v2,01/21] powerpc: Remove warning on array size when empty

[Michael Ellerman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 857 bytes --]

On Fri, 2018-03-02 at 19:49:18 UTC, Mathieu Malaterre wrote:
> When neither CONFIG_ALTIVEC, nor CONFIG_VSX or CONFIG_PPC64 is defined, the
> array feature_properties is defined as an empty array, which in turn
> triggers the following warning (treated as error on W=1):
> 
>   CC      arch/powerpc/kernel/prom.o
> arch/powerpc/kernel/prom.c: In function ‘check_cpu_feature_properties’:
> arch/powerpc/kernel/prom.c:298:16: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
>   for (i = 0; i < ARRAY_SIZE(feature_properties); ++i, ++fp) {
>                 ^
> cc1: all warnings being treated as errors
> 
> Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
> Signed-off-by: Mathieu Malaterre <malat@debian.org>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/4f1f40f7b2b4487f582ecafec64076

cheers

Re: [v2, 05/21] powerpc: Avoid comparison of unsigned long >= 0 in pfn_valid

[Michael Ellerman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1355 bytes --]

On Wed, 2018-03-07 at 20:34:35 UTC, Mathieu Malaterre wrote:
> Rewrite comparison since all values compared are of type `unsigned long`.
> 
> Instead of using unsigned properties and rewriting the original code as:
> (originally suggested by Segher Boessenkool <segher@kernel.crashing.org>)
> 
>   #define pfn_valid(pfn) \
>                (((pfn) - ARCH_PFN_OFFSET) < (max_mapnr - ARCH_PFN_OFFSET))
> 
> Prefer a static inline function to make code as readable as possible.
> 
> Fix a warning (treated as error in W=1):
> 
>   CC      arch/powerpc/kernel/irq.o
> In file included from ./include/linux/bug.h:5:0,
>                  from ./include/linux/cpumask.h:13,
>                  from ./include/linux/smp.h:13,
>                  from ./include/linux/kernel_stat.h:5,
>                  from arch/powerpc/kernel/irq.c:35:
> ./include/linux/dma-mapping.h: In function ‘dma_map_resource’:
> ./arch/powerpc/include/asm/page.h:129:32: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
>  #define pfn_valid(pfn)  ((pfn) >= ARCH_PFN_OFFSET && (pfn) < max_mapnr)
>                                 ^
> Suggested-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Signed-off-by: Mathieu Malaterre <malat@debian.org>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/603b892200e653dd7e86a0e4a31556

cheers

Re: [v2, 06/21] powerpc: Avoid comparison of unsigned long >= 0 in __access_ok

[Michael Ellerman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1940 bytes --]

On Fri, 2018-03-02 at 19:50:51 UTC, Mathieu Malaterre wrote:
> Rewrite function-like macro into regular static inline function to avoid a
> warning during macro expansion.
> Fix warning (treated as error in W=1):
> 
>   CC      arch/powerpc/kernel/signal_32.o
> In file included from ./include/linux/uaccess.h:14:0,
>                  from ./include/asm-generic/termios-base.h:8,
>                  from ./arch/powerpc/include/asm/termios.h:20,
>                  from ./include/uapi/linux/termios.h:6,
>                  from ./include/linux/tty.h:7,
>                  from arch/powerpc/kernel/signal_32.c:36:
> ./include/asm-generic/termios-base.h: In function ‘user_termio_to_kernel_termios’:
> ./arch/powerpc/include/asm/uaccess.h:52:35: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
>    (((size) == 0) || (((size) - 1) <= ((segment).seg - (addr)))))
>                                    ^
> ./arch/powerpc/include/asm/uaccess.h:58:3: note: in expansion of macro ‘__access_ok’
>    __access_ok((__force unsigned long)(addr), (size), get_fs()))
>    ^~~~~~~~~~~
> ./arch/powerpc/include/asm/uaccess.h:262:6: note: in expansion of macro ‘access_ok’
>   if (access_ok(VERIFY_READ, __gu_addr, (size)))   \
>       ^~~~~~~~~
> ./arch/powerpc/include/asm/uaccess.h:80:2: note: in expansion of macro ‘__get_user_check’
>   __get_user_check((x), (ptr), sizeof(*(ptr)))
>   ^~~~~~~~~~~~~~~~
> ./include/asm-generic/termios-base.h:36:6: note: in expansion of macro ‘get_user’
>   if (get_user(termios->c_line, &termio->c_line) < 0)
>       ^~~~~~~~
> [...]
> cc1: all warnings being treated as errors
> 
> Suggested-by: Segher Boessenkool <segher@kernel.crashing.org>
> Signed-off-by: Mathieu Malaterre <malat@debian.org>
> Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/ef85dffd4251ff6c23056651f6f83b

cheers

Re: [v2,15/21] powerpc: Make function MMU_setup static

[Michael Ellerman]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 602 bytes --]

On Wed, 2018-03-07 at 20:32:55 UTC, Mathieu Malaterre wrote:
> Since function `MMU_setup` is not meant to be exported, change the
> signature to `static`. Fix warning (treated as error with W=1):
> 
>   CC      kernel/sys.o
> arch/powerpc/mm/init_32.c:102:13: error: no previous prototype for ‘MMU_setup’ [-Werror=missing-prototypes]
>  void __init MMU_setup(void)
>              ^~~~~~~~~
> cc1: all warnings being treated as errors
> 
> Signed-off-by: Mathieu Malaterre <malat@debian.org>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/d15a261d876da3267c8c706ef21e7f

cheers

Re: [PATCH 04/21] powerpc: Mark both tmp variables as unused

[Christophe LEROY]

Le 25/02/2018 à 18:22, Mathieu Malaterre a écrit :
> Since the value of `tmp` is never intended to be read, declare both `tmp`
> variables as unused. Fix warning (treated as error in W=1):

What about using fault_in_pages_readable() instead ?

Christophe

> 
>    CC      arch/powerpc/kernel/signal_32.o
> arch/powerpc/kernel/signal_32.c: In function ‘sys_swapcontext’:
> arch/powerpc/kernel/signal_32.c:1048:16: error: variable ‘tmp’ set but not used [-Werror=unused-but-set-variable]
>    unsigned char tmp;
>                  ^~~
> arch/powerpc/kernel/signal_32.c: In function ‘sys_debug_setcontext’:
> arch/powerpc/kernel/signal_32.c:1234:16: error: variable ‘tmp’ set but not used [-Werror=unused-but-set-variable]
>    unsigned char tmp;
>                  ^~~
> cc1: all warnings being treated as errors
> 
> Signed-off-by: Mathieu Malaterre <malat@debian.org>
> ---
>   arch/powerpc/kernel/signal_32.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
> index a46de0035214..492f03451877 100644
> --- a/arch/powerpc/kernel/signal_32.c
> +++ b/arch/powerpc/kernel/signal_32.c
> @@ -1045,7 +1045,7 @@ long sys_swapcontext(struct ucontext __user *old_ctx,
>   		     struct ucontext __user *new_ctx,
>   		     int ctx_size, int r6, int r7, int r8, struct pt_regs *regs)
>   {
> -	unsigned char tmp;
> +	unsigned char tmp __maybe_unused;
>   	int ctx_has_vsx_region = 0;
>   
>   #ifdef CONFIG_PPC64
> @@ -1231,7 +1231,7 @@ int sys_debug_setcontext(struct ucontext __user *ctx,
>   {
>   	struct sig_dbg_op op;
>   	int i;
> -	unsigned char tmp;
> +	unsigned char tmp __maybe_unused;
>   	unsigned long new_msr = regs->msr;
>   #ifdef CONFIG_PPC_ADV_DEBUG_REGS
>   	unsigned long new_dbcr0 = current->thread.debug.dbcr0;
> 

Re: [PATCH 04/21] powerpc: Mark both tmp variables as unused

[Michael Ellerman]

Christophe LEROY <christophe.leroy@c-s.fr> writes:

> Le 25/02/2018 à 18:22, Mathieu Malaterre a écrit :
>> Since the value of `tmp` is never intended to be read, declare both `tmp`
>> variables as unused. Fix warning (treated as error in W=1):
>
> What about using fault_in_pages_readable() instead ?

Yeah that looks like it would work.

I'd be happy to take a tested patch :D

cheers

Re: [PATCH 17/21] powerpc: Add missing prototype for sys_debug_setcontext

[Mathieu Malaterre]

Michael,

On Thu, Mar 8, 2018 at 11:36 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Mathieu Malaterre <malat@debian.org> writes:
>
>> On Sun, Mar 4, 2018 at 11:54 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>>> Mathieu Malaterre <malat@debian.org> writes:
>>>
>>>> In commit 81e7009ea46c ("powerpc: merge ppc signal.c and ppc64 signal32.c")
>>>> the function sys_debug_setcontext was added without a prototype.
>>>>
>>>> Fix compilation warning (treated as error in W=1):
>>>>
>>>>   CC      arch/powerpc/kernel/signal_32.o
>>>> arch/powerpc/kernel/signal_32.c:1227:5: error: no previous prototype for ‘sys_debug_setcontext’ [-Werror=missing-prototypes]
>>>>  int sys_debug_setcontext(struct ucontext __user *ctx,
>>>>      ^~~~~~~~~~~~~~~~~~~~
>>>> cc1: all warnings being treated as errors
>>>
>>> This one should actually be using the SYSCALL_DEFINE syntax, so that it
>>> can be used with CONFIG_FTRACE_SYSCALLS.
>>>
>>> See eg. our mmap:
>>>
>>>   SYSCALL_DEFINE6(mmap, unsigned long, addr, size_t, len,
>>>                 unsigned long, prot, unsigned long, flags,
>>>                 unsigned long, fd, off_t, offset)
>>>   {
>>>         return do_mmap2(addr, len, prot, flags, fd, offset, PAGE_SHIFT);
>>>   }
>>>
>>>
>>> We probably still need this patch, but I'm not entirely sure because the
>>> SYSCALL_DEFINE macro does all sorts of shenanigans.
>>
>> I see. Could you please drop this patch then. The patch does not look
>> that trivial anymore. I'll need to dig a bit more on how to do the
>> syscall stuff with a 7 params function.
>
> Ergh, yuck, seems we're the first suckers to need do that.
>
> I think I'll take this patch for now, it's still good for now at least,
> and then the SYSCALL_DEFINE stuff can be an addition.

Just to close the loop (for later reference). Here is what Al Viro
told me about this function

[begin quote]
int sys_debug_setcontext(struct ucontext __user *ctx,
                         int ndbg, struct sig_dbg_op __user *dbg,
                         int r6, int r7, int r8,
                         struct pt_regs *regs)

can't be converted to SYSCALL_DEFINE... not because it's a 7-argument
syscall (it isn't); the problem is that the last argument here does
*not* come from userland.  What it really is trying to be is
3-argument syscall:
SYSCALL_DEFINE3(debug_setcontext, struct ucontext __user *, ctx,
                                  int, ndbg,
                                  struct sig_dbg_op __user *, dbg)
with no dummy r6/r7/r8.  The thing is, ppc dispatcher combines the
"pass 6 arguments" and "pass pt_regs *" by passing both.

debug_setcontext(), swapcontext(), sigreturn() and rt_sigreturn() are,
AFAICS, the only ppc syscalls that make use of that argument.  All of
those are relying upon regs == current_pt_regs() == current->thread.regs,
[...]
But in any case, these are not 7-argument syscalls - debug_setcontext(2)
and swapcontext(2) are 3-argument and sigreturn()/rt_sigreturn() have
no arguments at all.
[end quote]

Sorry about the wrong analysis in the number of arguments count. I
believe commit 0d60619e1c0ca (powerpc/next) should be just fine for
now.

2cts

Re: [PATCH 17/21] powerpc: Add missing prototype for sys_debug_setcontext

[Michael Ellerman]

Hi Mathieu,

Mathieu Malaterre <malat@debian.org> writes:
> On Thu, Mar 8, 2018 at 11:36 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>> Mathieu Malaterre <malat@debian.org> writes:
>>> On Sun, Mar 4, 2018 at 11:54 AM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>>>> Mathieu Malaterre <malat@debian.org> writes:
>>>>> In commit 81e7009ea46c ("powerpc: merge ppc signal.c and ppc64 signal32.c")
>>>>> the function sys_debug_setcontext was added without a prototype.
>>>>>
>>>>> Fix compilation warning (treated as error in W=1):
>>>>>
>>>>>   CC      arch/powerpc/kernel/signal_32.o
>>>>> arch/powerpc/kernel/signal_32.c:1227:5: error: no previous prototype for ‘sys_debug_setcontext’ [-Werror=missing-prototypes]
>>>>>  int sys_debug_setcontext(struct ucontext __user *ctx,
>>>>>      ^~~~~~~~~~~~~~~~~~~~
>>>>> cc1: all warnings being treated as errors
>>>>
>>>> This one should actually be using the SYSCALL_DEFINE syntax, so that it
>>>> can be used with CONFIG_FTRACE_SYSCALLS.
>>>>
>>>> See eg. our mmap:
>>>>
>>>>   SYSCALL_DEFINE6(mmap, unsigned long, addr, size_t, len,
>>>>                 unsigned long, prot, unsigned long, flags,
>>>>                 unsigned long, fd, off_t, offset)
>>>>   {
>>>>         return do_mmap2(addr, len, prot, flags, fd, offset, PAGE_SHIFT);
>>>>   }
>>>>
>>>>
>>>> We probably still need this patch, but I'm not entirely sure because the
>>>> SYSCALL_DEFINE macro does all sorts of shenanigans.
>>>
>>> I see. Could you please drop this patch then. The patch does not look
>>> that trivial anymore. I'll need to dig a bit more on how to do the
>>> syscall stuff with a 7 params function.
>>
>> Ergh, yuck, seems we're the first suckers to need do that.
>>
>> I think I'll take this patch for now, it's still good for now at least,
>> and then the SYSCALL_DEFINE stuff can be an addition.
>
> Just to close the loop (for later reference). Here is what Al Viro
> told me about this function
>
> [begin quote]
> int sys_debug_setcontext(struct ucontext __user *ctx,
>                          int ndbg, struct sig_dbg_op __user *dbg,
>                          int r6, int r7, int r8,
>                          struct pt_regs *regs)
>
> can't be converted to SYSCALL_DEFINE... not because it's a 7-argument
> syscall (it isn't); the problem is that the last argument here does
> *not* come from userland.  What it really is trying to be is
> 3-argument syscall:
> SYSCALL_DEFINE3(debug_setcontext, struct ucontext __user *, ctx,
>                                   int, ndbg,
>                                   struct sig_dbg_op __user *, dbg)
> with no dummy r6/r7/r8.  The thing is, ppc dispatcher combines the
> "pass 6 arguments" and "pass pt_regs *" by passing both.
>
> debug_setcontext(), swapcontext(), sigreturn() and rt_sigreturn() are,
> AFAICS, the only ppc syscalls that make use of that argument.  All of
> those are relying upon regs == current_pt_regs() == current->thread.regs,
> [...]
> But in any case, these are not 7-argument syscalls - debug_setcontext(2)
> and swapcontext(2) are 3-argument and sigreturn()/rt_sigreturn() have
> no arguments at all.
> [end quote]

Awesome follow-up thanks.

I should have realised that's what the code was doing, but I didn't look
closely enough. Apologies to Al for having to stare at our horrible
signal code again :)

We should really clean all that up, I can't see any reason those
syscalls aren't using current_pt_regs().

That would then mean we could use SYSCALL_DEFINE.

I wrote that down so we don't (might not) forget:

  https://github.com/linuxppc/linux/issues/131

> Sorry about the wrong analysis in the number of arguments count. I
> believe commit 0d60619e1c0ca (powerpc/next) should be just fine for
> now.

No worries.

cheers