LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] powerpc/Makefile: fix build failure by disabling attribute-alias warning
From: Christophe LEROY @ 2018-05-28 14:56 UTC (permalink / raw)
  To: Segher Boessenkool
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	linux-kernel, linuxppc-dev
In-Reply-To: <20180528143719.GB17342@gate.crashing.org>



Le 28/05/2018 à 16:37, Segher Boessenkool a écrit :
> On Mon, May 28, 2018 at 02:17:49PM +0000, Christophe Leroy wrote:
>> Latest GCC version emit many warnings similar to the following one.
> 
> You didn't actually show an example?

Yes I forgot:

In file included from arch/powerpc/kernel/syscalls.c:24:
./include/linux/syscalls.h:233:18: warning: 'sys_mmap2' alias between 
functions of incompatible types 'long int(long unsigned int,  size_t, 
long unsigned int,  long unsigned int,  long unsigned int,  long 
unsigned int)' {aka 'long int(long unsigned int,  unsigned int,  long 
unsigned int,  long unsigned int,  long unsigned int,  long unsigned 
int)'} and 'long int(long int,  long int,  long int,  long int,  long 
int,  long int)' [-Wattribute-alias]
   asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \
                   ^~~
./include/linux/syscalls.h:222:2: note: in expansion of macro 
'__SYSCALL_DEFINEx'
   __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
   ^~~~~~~~~~~~~~~~~
./include/linux/syscalls.h:216:36: note: in expansion of macro 
'SYSCALL_DEFINEx'
  #define SYSCALL_DEFINE6(name, ...) SYSCALL_DEFINEx(6, _##name, 
__VA_ARGS__)
                                     ^~~~~~~~~~~~~~~
arch/powerpc/kernel/syscalls.c:65:1: note: in expansion of macro 
'SYSCALL_DEFINE6'
  SYSCALL_DEFINE6(mmap2, unsigned long, addr, size_t, len,
  ^~~~~~~~~~~~~~~
./include/linux/syscalls.h:238:18: note: aliased declaration here
   asmlinkage long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__)) \
                   ^~~~~~~~
./include/linux/syscalls.h:222:2: note: in expansion of macro 
'__SYSCALL_DEFINEx'
   __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
   ^~~~~~~~~~~~~~~~~
./include/linux/syscalls.h:216:36: note: in expansion of macro 
'SYSCALL_DEFINEx'
  #define SYSCALL_DEFINE6(name, ...) SYSCALL_DEFINEx(6, _##name, 
__VA_ARGS__)
                                     ^~~~~~~~~~~~~~~
arch/powerpc/kernel/syscalls.c:65:1: note: in expansion of macro 
'SYSCALL_DEFINE6'
  SYSCALL_DEFINE6(mmap2, unsigned long, addr, size_t, len,
  ^~~~~~~~~~~~~~~

> 
> This warning should detect serious problems, so don't disable it without
> first investigating please.

It's been a discussion on this topic, ref 
https://lkml.org/lkml/2017/12/5/581

It says "The new warning seems reasonable in principle, but it doesn't
help us here, since we rely on the type mismatch to sanitize the
system call arguments. After I reported this as GCC PR82435, a new
-Wno-attribute-alias option was added that could be used to turn the
warning off globally on the command line, but I'd prefer to do it a
little more fine-grained"


> 
> What do you call "latest version", btw?  Trunk, aka 9.0?  Or the most
> advanced release, 8.1?  Or the latest release (which also is 8.1 :-) )

I encounter it with 8.1
According the refered discusion, it linked to GCC 8

Christophe

> 
> 
> Segher
> 

^ permalink raw reply

* Re: [PATCH] powerpc/Makefile: fix build failure by disabling attribute-alias warning
From: Segher Boessenkool @ 2018-05-28 15:07 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	linuxppc-dev, linux-kernel
In-Reply-To: <e81f81b7-2bf5-7f9a-919f-87972bf1a9db@c-s.fr>

On Mon, May 28, 2018 at 02:39:37PM +0000, Christophe Leroy wrote:
> In file included from arch/powerpc/kernel/syscalls.c:24:
> ./include/linux/syscalls.h:233:18: warning: 'sys_mmap2' alias between 
> functions of incompatible types 'long int(long unsigned int,  size_t, 
> long unsigned int,  long unsigned int,  long unsigned int,  long 
> unsigned int)' {aka 'long int(long unsigned int,  unsigned int,  long 
> unsigned int,  long unsigned int,  long unsigned int,  long unsigned 
> int)'} and 'long int(long int,  long int,  long int,  long int,  long 
> int,  long int)' [-Wattribute-alias]

So yes, these are actually different (int vs. long).

> In file included from arch/powerpc/kernel/signal_32.c:29:
> ./include/linux/syscalls.h:233:18: warning: 'sys_swapcontext' alias 
> between functions of incompatible types 'long int(struct ucontext *, 
> struct ucontext *, long int)' and 'long int(long int,  long int,  long 
> int)' [-Wattribute-alias]

And this one is quite spectacularly different.


Try putting this before the wacko aliases:

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wattribute-alias"

and this after:

#pragma GCC diagnostic pop

so that you will get this quite useful warning for all other code.


Segher

^ permalink raw reply

* [PATCH] powerpc/64s: Enhance the information in cpu_show_spectre_v1()
From: Michal Suchanek @ 2018-05-28 13:19 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Michal Suchanek, Mauricio Faria de Oliveira, Nicholas Piggin,
	Michael Neuling, linuxppc-dev, linux-kernel
In-Reply-To: <20180424041559.32410-1-mpe@ellerman.id.au>


We now have barrier_nospec as mitigation so print it in
cpu_show_spectre_v1 when enabled.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 arch/powerpc/kernel/security.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index 0239383c7e4d..a0c32d53980b 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c
@@ -120,7 +120,10 @@ ssize_t cpu_show_spectre_v1(struct device *dev, struct device_attribute *attr, c
 	if (!security_ftr_enabled(SEC_FTR_BNDS_CHK_SPEC_BAR))
 		return sprintf(buf, "Not affected\n");
 
-	return sprintf(buf, "Vulnerable\n");
+	if (barrier_nospec_enabled)
+		return sprintf(buf, "Mitigation: __user pointer sanitization\n");
+	else
+		return sprintf(buf, "Vulnerable\n");
 }
 
 ssize_t cpu_show_spectre_v2(struct device *dev, struct device_attribute *attr, char *buf)
-- 
2.13.6

^ permalink raw reply related

* Re: [PATCH] powerpc/Makefile: fix build failure by disabling attribute-alias warning
From: Mathieu Malaterre @ 2018-05-28 16:18 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: Christophe Leroy, Paul Mackerras, linuxppc-dev, LKML
In-Reply-To: <20180528150726.GC17342@gate.crashing.org>

On Mon, May 28, 2018 at 5:07 PM, Segher Boessenkool
<segher@kernel.crashing.org> wrote:
> On Mon, May 28, 2018 at 02:39:37PM +0000, Christophe Leroy wrote:
>> In file included from arch/powerpc/kernel/syscalls.c:24:
>> ./include/linux/syscalls.h:233:18: warning: 'sys_mmap2' alias between
>> functions of incompatible types 'long int(long unsigned int,  size_t,
>> long unsigned int,  long unsigned int,  long unsigned int,  long
>> unsigned int)' {aka 'long int(long unsigned int,  unsigned int,  long
>> unsigned int,  long unsigned int,  long unsigned int,  long unsigned
>> int)'} and 'long int(long int,  long int,  long int,  long int,  long
>> int,  long int)' [-Wattribute-alias]
>
> So yes, these are actually different (int vs. long).
>
>> In file included from arch/powerpc/kernel/signal_32.c:29:
>> ./include/linux/syscalls.h:233:18: warning: 'sys_swapcontext' alias
>> between functions of incompatible types 'long int(struct ucontext *,
>> struct ucontext *, long int)' and 'long int(long int,  long int,  long
>> int)' [-Wattribute-alias]
>
> And this one is quite spectacularly different.

https://patchwork.kernel.org/patch/10375607/

>
> Try putting this before the wacko aliases:
>
> #pragma GCC diagnostic push
> #pragma GCC diagnostic ignored "-Wattribute-alias"
>
> and this after:
>
> #pragma GCC diagnostic pop
>
> so that you will get this quite useful warning for all other code.
>
>
> Segher

^ permalink raw reply

* [PATCH] powerpc/64: Fix build failure with GCC 8.1
From: Christophe Leroy @ 2018-05-28 16:37 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linux-kernel, linuxppc-dev

  CC      arch/powerpc/kernel/nvram_64.o
arch/powerpc/kernel/nvram_64.c: In function 'nvram_create_partition':
arch/powerpc/kernel/nvram_64.c:1042:2: error: 'strncpy' specified bound 12 equals destination size [-Werror=stringop-truncation]
  strncpy(new_part->header.name, name, 12);
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  CC      arch/powerpc/kernel/trace/ftrace.o
In function 'make_field',
    inlined from 'ps3_repository_read_boot_dat_address' at arch/powerpc/platforms/ps3/repository.c:900:9:
arch/powerpc/platforms/ps3/repository.c:106:2: error: 'strncpy' output truncated before terminating nul copying 8 bytes from a string of the same length [-Werror=stringop-truncation]
  strncpy((char *)&n, text, 8);
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/kernel/nvram_64.c          | 2 +-
 arch/powerpc/platforms/ps3/repository.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
index ba681dac7b46..7507448cd904 100644
--- a/arch/powerpc/kernel/nvram_64.c
+++ b/arch/powerpc/kernel/nvram_64.c
@@ -1039,7 +1039,7 @@ loff_t __init nvram_create_partition(const char *name, int sig,
 	new_part->index = free_part->index;
 	new_part->header.signature = sig;
 	new_part->header.length = size;
-	strncpy(new_part->header.name, name, 12);
+	strncpy(new_part->header.name, name, sizeof(new_part->header.name) - 1);
 	new_part->header.checksum = nvram_checksum(&new_part->header);
 
 	rc = nvram_write_header(new_part);
diff --git a/arch/powerpc/platforms/ps3/repository.c b/arch/powerpc/platforms/ps3/repository.c
index 50dbaf24b1ee..b4d6628eec5e 100644
--- a/arch/powerpc/platforms/ps3/repository.c
+++ b/arch/powerpc/platforms/ps3/repository.c
@@ -101,9 +101,9 @@ static u64 make_first_field(const char *text, u64 index)
 
 static u64 make_field(const char *text, u64 index)
 {
-	u64 n;
+	u64 n = 0;
 
-	strncpy((char *)&n, text, 8);
+	memcpy((char *)&n, text, min(sizeof(n), strlen(text)));
 	return n + index;
 }
 
-- 
2.13.3

^ permalink raw reply related

* [PATCH] cpuidle/powernv : Add Description for cpuidle state
From: Abhishek Goel @ 2018-05-28 17:34 UTC (permalink / raw)
  To: rjw, daniel.lezcano, benh, paulus, mpe, linux-pm, linuxppc-dev,
	linux-kernel
  Cc: Abhishek Goel

Names of cpuidle states were being used for description of states
in POWER as no descriptions were added in device tree. This patch
reads description for idle states which have been added in device
tree.
The description for idle states in case of POWER can be printed
using "cpupower monitor -l" or "cpupower idle-info".

Signed-off-by: Abhishek Goel <huntbag@linux.vnet.ibm.com>
---

The skiboot patch which adds description for idle states in device tree
can be found here: https://patchwork.ozlabs.org/patch/921637/

 drivers/cpuidle/cpuidle-powernv.c | 17 +++++++++++++----
 include/linux/cpuidle.h           |  2 +-
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
index 1a8234e706bc..d3915a965128 100644
--- a/drivers/cpuidle/cpuidle-powernv.c
+++ b/drivers/cpuidle/cpuidle-powernv.c
@@ -133,7 +133,7 @@ static int stop_loop(struct cpuidle_device *dev,
 static struct cpuidle_state powernv_states[CPUIDLE_STATE_MAX] = {
 	{ /* Snooze */
 		.name = "snooze",
-		.desc = "snooze",
+		.desc = "idle polling state",
 		.exit_latency = 0,
 		.target_residency = 0,
 		.enter = snooze_loop },
@@ -206,6 +206,7 @@ static int powernv_cpuidle_driver_init(void)
 }
 
 static inline void add_powernv_state(int index, const char *name,
+				     const char *desc,
 				     unsigned int flags,
 				     int (*idle_fn)(struct cpuidle_device *,
 						    struct cpuidle_driver *,
@@ -215,7 +216,7 @@ static inline void add_powernv_state(int index, const char *name,
 				     u64 psscr_val, u64 psscr_mask)
 {
 	strlcpy(powernv_states[index].name, name, CPUIDLE_NAME_LEN);
-	strlcpy(powernv_states[index].desc, name, CPUIDLE_NAME_LEN);
+	strlcpy(powernv_states[index].desc, desc, CPUIDLE_DESC_LEN);
 	powernv_states[index].flags = flags;
 	powernv_states[index].target_residency = target_residency;
 	powernv_states[index].exit_latency = exit_latency;
@@ -250,6 +251,7 @@ static int powernv_add_idle_states(void)
 	u64 psscr_val[CPUIDLE_STATE_MAX];
 	u64 psscr_mask[CPUIDLE_STATE_MAX];
 	const char *names[CPUIDLE_STATE_MAX];
+	const char *descs[CPUIDLE_STATE_MAX];
 	u32 has_stop_states = 0;
 	int i, rc;
 	u32 supported_flags = pnv_get_supported_cpuidle_states();
@@ -311,6 +313,11 @@ static int powernv_add_idle_states(void)
 		pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-names in DT\n");
 		goto out;
 	}
+	if (of_property_read_string_array(power_mgt,
+		"ibm,cpu-idle-state-descs", descs, dt_idle_states) < 0) {
+		pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-descs in DT\n");
+		goto out;
+	}
 
 	/*
 	 * If the idle states use stop instruction, probe for psscr values
@@ -414,10 +421,11 @@ static int powernv_add_idle_states(void)
 				target_residency = 100;
 			/* Add NAP state */
 			add_powernv_state(nr_idle_states, "Nap",
+					  "stop processor execution",
 					  CPUIDLE_FLAG_NONE, nap_loop,
 					  target_residency, exit_latency, 0, 0);
 		} else if (has_stop_states && !stops_timebase) {
-			add_powernv_state(nr_idle_states, names[i],
+			add_powernv_state(nr_idle_states, names[i], descs[i],
 					  CPUIDLE_FLAG_NONE, stop_loop,
 					  target_residency, exit_latency,
 					  psscr_val[i], psscr_mask[i]);
@@ -434,11 +442,12 @@ static int powernv_add_idle_states(void)
 				target_residency = 300000;
 			/* Add FASTSLEEP state */
 			add_powernv_state(nr_idle_states, "FastSleep",
+					  "Core and L2 clock gating",
 					  CPUIDLE_FLAG_TIMER_STOP,
 					  fastsleep_loop,
 					  target_residency, exit_latency, 0, 0);
 		} else if (has_stop_states && stops_timebase) {
-			add_powernv_state(nr_idle_states, names[i],
+			add_powernv_state(nr_idle_states, names[i], descs[i],
 					  CPUIDLE_FLAG_TIMER_STOP, stop_loop,
 					  target_residency, exit_latency,
 					  psscr_val[i], psscr_mask[i]);
diff --git a/include/linux/cpuidle.h b/include/linux/cpuidle.h
index 1eefabf1621f..5094755cb132 100644
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -17,7 +17,7 @@
 
 #define CPUIDLE_STATE_MAX	10
 #define CPUIDLE_NAME_LEN	16
-#define CPUIDLE_DESC_LEN	32
+#define CPUIDLE_DESC_LEN	60
 
 struct module;
 
-- 
2.14.1

^ permalink raw reply related

* Re: [PATCH][RFC] [powerpc] arch_ptrace() uses of access_ok() are pointless
From: Mathieu Malaterre @ 2018-05-28 19:25 UTC (permalink / raw)
  To: Al Viro; +Cc: linuxppc-dev, LKML
In-Reply-To: <20180527223403.GT30522@ZenIV.linux.org.uk>

On Mon, May 28, 2018 at 12:34 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> make it use copy_{from,to}_user(), rather than access_ok() +
> __copy_...
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  arch/powerpc/kernel/ptrace.c | 22 +++++++---------------
>  1 file changed, 7 insertions(+), 15 deletions(-)
>
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index d23cf632edf0..d8b0fd2fa3aa 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -3081,27 +3081,19 @@ long arch_ptrace(struct task_struct *child, long =
request,
>  #endif /* CONFIG_HAVE_HW_BREAKPOINT */
>  #endif /* CONFIG_PPC_ADV_DEBUG_REGS */
>
> -               if (!access_ok(VERIFY_WRITE, datavp,
> -                              sizeof(struct ppc_debug_info)))
> +               if (unlikely(copy_to_user(datavp, &dbginfo,
> +                                    sizeof(struct ppc_debug_info)))


Maybe this is just an RFC, but:

  CALL    ../arch/powerpc/kernel/systbl_chk.sh
../arch/powerpc/kernel/ptrace.c: In function =E2=80=98arch_ptrace=E2=80=99:
../arch/powerpc/kernel/ptrace.c:3086:4: error: expected =E2=80=98)=E2=80=99=
 before =E2=80=98return=E2=80=99
    return -EFAULT;
    ^~~~~~

Missing closing parenthesis.

>                         return -EFAULT;
> -               ret =3D __copy_to_user(datavp, &dbginfo,
> -                                    sizeof(struct ppc_debug_info)) ?
> -                     -EFAULT : 0;
> -               break;
> +               return 0;
>         }
>
>         case PPC_PTRACE_SETHWDEBUG: {
>                 struct ppc_hw_breakpoint bp_info;
>
> -               if (!access_ok(VERIFY_READ, datavp,
> -                              sizeof(struct ppc_hw_breakpoint)))
> -                       return -EFAULT;
> -               ret =3D __copy_from_user(&bp_info, datavp,
> -                                      sizeof(struct ppc_hw_breakpoint)) =
?
> -                     -EFAULT : 0;
> -               if (!ret)
> -                       ret =3D ppc_set_hwdebug(child, &bp_info);
> -               break;
> +               if (unlikely(copy_from_user(&bp_info, datavp,
> +                                      sizeof(struct ppc_hw_breakpoint)))
> +                     return -EFAULT;
> +               return ppc_set_hwdebug(child, &bp_info);
>         }
>
>         case PPC_PTRACE_DELHWDEBUG: {
> --
> 2.11.0
>

^ permalink raw reply

* Re: [PATCH][RFC] [powerpc] arch_ptrace() uses of access_ok() are pointless
From: Al Viro @ 2018-05-28 20:30 UTC (permalink / raw)
  To: Mathieu Malaterre; +Cc: linuxppc-dev, LKML
In-Reply-To: <CA+7wUswhpVRQ4yECc-Zh9djHxiT=9-ALuP7_=mfLMtU-NXDnfg@mail.gmail.com>


> Maybe this is just an RFC, but:
> 
>   CALL    ../arch/powerpc/kernel/systbl_chk.sh
> ../arch/powerpc/kernel/ptrace.c: In function ‘arch_ptrace’:
> ../arch/powerpc/kernel/ptrace.c:3086:4: error: expected ‘)’ before ‘return’
>     return -EFAULT;
>     ^~~~~~

and the same a few lines later.  What's more, those 'unlikely' are pointless
there.  Fixed variant follows; only build-tested, though.

make it use copy_{from,to}_user(), rather than access_ok() +
__copy_...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 arch/powerpc/kernel/ptrace.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index d23cf632edf0..f557322621e0 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -3081,27 +3081,19 @@ long arch_ptrace(struct task_struct *child, long request,
 #endif /* CONFIG_HAVE_HW_BREAKPOINT */
 #endif /* CONFIG_PPC_ADV_DEBUG_REGS */
 
-		if (!access_ok(VERIFY_WRITE, datavp,
-			       sizeof(struct ppc_debug_info)))
+		if (copy_to_user(datavp, &dbginfo,
+				     sizeof(struct ppc_debug_info)))
 			return -EFAULT;
-		ret = __copy_to_user(datavp, &dbginfo,
-				     sizeof(struct ppc_debug_info)) ?
-		      -EFAULT : 0;
-		break;
+		return 0;
 	}
 
 	case PPC_PTRACE_SETHWDEBUG: {
 		struct ppc_hw_breakpoint bp_info;
 
-		if (!access_ok(VERIFY_READ, datavp,
-			       sizeof(struct ppc_hw_breakpoint)))
-			return -EFAULT;
-		ret = __copy_from_user(&bp_info, datavp,
-				       sizeof(struct ppc_hw_breakpoint)) ?
-		      -EFAULT : 0;
-		if (!ret)
-			ret = ppc_set_hwdebug(child, &bp_info);
-		break;
+		if (copy_from_user(&bp_info, datavp,
+				       sizeof(struct ppc_hw_breakpoint)))
+		      return -EFAULT;
+		return ppc_set_hwdebug(child, &bp_info);
 	}
 
 	case PPC_PTRACE_DELHWDEBUG: {
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH] powerpc/64: Fix build failure with GCC 8.1
From: Benjamin Herrenschmidt @ 2018-05-28 23:08 UTC (permalink / raw)
  To: Christophe Leroy, Paul Mackerras, Michael Ellerman
  Cc: linux-kernel, linuxppc-dev
In-Reply-To: <1d3fd6a6c35e41d3e25ebe5256adf021369ada7c.1527525305.git.christophe.leroy@c-s.fr>

On Mon, 2018-05-28 at 16:37 +0000, Christophe Leroy wrote:
>   CC      arch/powerpc/kernel/nvram_64.o
> arch/powerpc/kernel/nvram_64.c: In function 'nvram_create_partition':
> arch/powerpc/kernel/nvram_64.c:1042:2: error: 'strncpy' specified bound 12 equals destination size [-Werror=stringop-truncation]
>   strncpy(new_part->header.name, name, 12);
>   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
>   CC      arch/powerpc/kernel/trace/ftrace.o
> In function 'make_field',
>     inlined from 'ps3_repository_read_boot_dat_address' at arch/powerpc/platforms/ps3/repository.c:900:9:
> arch/powerpc/platforms/ps3/repository.c:106:2: error: 'strncpy' output truncated before terminating nul copying 8 bytes from a string of the same length [-Werror=stringop-truncation]
>   strncpy((char *)&n, text, 8);
>   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~

That's not completely correct, here it's gcc warning being over-
zealous. The old specs I could find define the format to be a 12
characters strings *or* a less-than-12 characters NUL terminated
string. So it's perfectly ok to drop the trailing 0 if the name
happens to be exactly 12 characters.

Cheers,
Ben.

> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
>  arch/powerpc/kernel/nvram_64.c          | 2 +-
>  arch/powerpc/platforms/ps3/repository.c | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
> index ba681dac7b46..7507448cd904 100644
> --- a/arch/powerpc/kernel/nvram_64.c
> +++ b/arch/powerpc/kernel/nvram_64.c
> @@ -1039,7 +1039,7 @@ loff_t __init nvram_create_partition(const char *name, int sig,
>  	new_part->index = free_part->index;
>  	new_part->header.signature = sig;
>  	new_part->header.length = size;
> -	strncpy(new_part->header.name, name, 12);
> +	strncpy(new_part->header.name, name, sizeof(new_part->header.name) - 1);
>  	new_part->header.checksum = nvram_checksum(&new_part->header);
>  
>  	rc = nvram_write_header(new_part);
> diff --git a/arch/powerpc/platforms/ps3/repository.c b/arch/powerpc/platforms/ps3/repository.c
> index 50dbaf24b1ee..b4d6628eec5e 100644
> --- a/arch/powerpc/platforms/ps3/repository.c
> +++ b/arch/powerpc/platforms/ps3/repository.c
> @@ -101,9 +101,9 @@ static u64 make_first_field(const char *text, u64 index)
>  
>  static u64 make_field(const char *text, u64 index)
>  {
> -	u64 n;
> +	u64 n = 0;
>  
> -	strncpy((char *)&n, text, 8);
> +	memcpy((char *)&n, text, min(sizeof(n), strlen(text)));
>  	return n + index;
>  }
>  

^ permalink raw reply

* [PATCH v2] selftests/powerpc: Add perf breakpoint test
From: Michael Neuling @ 2018-05-28 23:22 UTC (permalink / raw)
  To: mpe; +Cc: linuxppc-dev, mikey

This tests perf hardware breakpoints (ie PERF_TYPE_BREAKPOINT) on
powerpc.

Signed-off-by: Michael Neuling <mikey@neuling.org>
---
v2:
  - fix failure noticed by mpe
  - rewrite to explicitly test all options (rather than randomly)
---
 .../selftests/powerpc/ptrace/.gitignore       |   1 +
 .../testing/selftests/powerpc/ptrace/Makefile |   2 +-
 .../selftests/powerpc/ptrace/perf-hwbreak.c   | 195 ++++++++++++++++++
 3 files changed, 197 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/powerpc/ptrace/perf-hwbreak.c

diff --git a/tools/testing/selftests/powerpc/ptrace/.gitignore b/tools/testing/selftests/powerpc/ptrace/.gitignore
index 9dcc16ea81..07ec449a27 100644
--- a/tools/testing/selftests/powerpc/ptrace/.gitignore
+++ b/tools/testing/selftests/powerpc/ptrace/.gitignore
@@ -9,3 +9,4 @@ ptrace-tm-vsx
 ptrace-tm-spd-vsx
 ptrace-tm-spr
 ptrace-hwbreak
+perf-hwbreak
diff --git a/tools/testing/selftests/powerpc/ptrace/Makefile b/tools/testing/selftests/powerpc/ptrace/Makefile
index 0e2f4601d1..aaf7044771 100644
--- a/tools/testing/selftests/powerpc/ptrace/Makefile
+++ b/tools/testing/selftests/powerpc/ptrace/Makefile
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 TEST_PROGS := ptrace-gpr ptrace-tm-gpr ptrace-tm-spd-gpr \
               ptrace-tar ptrace-tm-tar ptrace-tm-spd-tar ptrace-vsx ptrace-tm-vsx \
-              ptrace-tm-spd-vsx ptrace-tm-spr ptrace-hwbreak
+              ptrace-tm-spd-vsx ptrace-tm-spr ptrace-hwbreak perf-hwbreak
 
 include ../../lib.mk
 
diff --git a/tools/testing/selftests/powerpc/ptrace/perf-hwbreak.c b/tools/testing/selftests/powerpc/ptrace/perf-hwbreak.c
new file mode 100644
index 0000000000..60df0b5e62
--- /dev/null
+++ b/tools/testing/selftests/powerpc/ptrace/perf-hwbreak.c
@@ -0,0 +1,195 @@
+/*
+ * perf events self profiling example test case for hw breakpoints.
+ *
+ * This tests perf PERF_TYPE_BREAKPOINT parameters
+ * 1) tests all variants of the break on read/write flags
+ * 2) tests exclude_user == 0 and 1
+ * 3) test array matches (if DAWR is supported))
+ * 4) test different numbers of breakpoints matches
+ *
+ * Configure this breakpoint, then read and write the data a number of
+ * times. Then check the output count from perf is as expected.
+ *
+ * Based on:
+ *   http://ozlabs.org/~anton/junkcode/perf_events_example1.c
+ *
+ * Copyright (C) 2018 Michael Neuling, IBM Corporation.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+#include <unistd.h>
+#include <assert.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <elf.h>
+#include <pthread.h>
+#include <sys/syscall.h>
+#include <linux/perf_event.h>
+#include <linux/hw_breakpoint.h>
+#include "utils.h"
+
+#define MAX_LOOPS 10000
+
+#define DAWR_LENGTH_MAX ((0x3f + 1) * 8)
+
+static inline int sys_perf_event_open(struct perf_event_attr *attr, pid_t pid,
+				      int cpu, int group_fd,
+				      unsigned long flags)
+{
+	attr->size = sizeof(*attr);
+	return syscall(__NR_perf_event_open, attr, pid, cpu, group_fd, flags);
+}
+
+static inline bool breakpoint_test(int len)
+{
+	struct perf_event_attr attr;
+	int fd;
+
+	/* setup counters */
+	memset(&attr, 0, sizeof(attr));
+	attr.disabled = 1;
+	attr.type = PERF_TYPE_BREAKPOINT;
+	attr.bp_type = HW_BREAKPOINT_R;
+	/* bp_addr can point anywhere but needs to be aligned */
+	attr.bp_addr = (__u64)(&attr) & 0xfffffffffffff800;
+	attr.bp_len = len;
+	fd = sys_perf_event_open(&attr, 0, -1, -1, 0);
+	if (fd < 0)
+		return false;
+	close(fd);
+	return true;
+}
+
+static inline bool perf_breakpoint_supported(void)
+{
+	return breakpoint_test(4);
+}
+
+static inline bool dawr_supported(void)
+{
+	return breakpoint_test(DAWR_LENGTH_MAX);
+}
+
+static int runtestsingle(int readwriteflag, int exclude_user, int arraytest)
+{
+	int i,j;
+	struct perf_event_attr attr;
+	size_t res;
+	unsigned long long breaks, needed;
+	int readint;
+	int readintarraybig[2*DAWR_LENGTH_MAX/sizeof(int)];
+	int *readintalign;
+	volatile int *ptr;
+	int break_fd;
+	int loop_num = MAX_LOOPS - (rand() % 100); /* provide some variability */
+	volatile int *k;
+
+	/* align to 0x400 boundary as required by DAWR */
+	readintalign = (int *)(((unsigned long)readintarraybig + 0x7ff) &
+			       0xfffffffffffff800);
+
+	ptr = &readint;
+	if (arraytest)
+		ptr = &readintalign[0];
+
+	/* setup counters */
+	memset(&attr, 0, sizeof(attr));
+	attr.disabled = 1;
+	attr.type = PERF_TYPE_BREAKPOINT;
+	attr.bp_type = readwriteflag;
+	attr.bp_addr = (__u64)ptr;
+	attr.bp_len = sizeof(int);
+	if (arraytest)
+		attr.bp_len = DAWR_LENGTH_MAX;
+	attr.exclude_user = exclude_user;
+	break_fd = sys_perf_event_open(&attr, 0, -1, -1, 0);
+	if (break_fd < 0) {
+		perror("sys_perf_event_open");
+		exit(1);
+	}
+
+	/* start counters */
+	ioctl(break_fd, PERF_EVENT_IOC_ENABLE);
+
+	/* Test a bunch of reads and writes */
+	k = &readint;
+	for (i = 0; i < loop_num; i++) {
+		if (arraytest)
+			k = &(readintalign[i % (DAWR_LENGTH_MAX/sizeof(int))]);
+
+		j = *k;
+		*k = j;
+	}
+
+	/* stop counters */
+	ioctl(break_fd, PERF_EVENT_IOC_DISABLE);
+
+	/* read and check counters */
+	res = read(break_fd, &breaks, sizeof(unsigned long long));
+	assert(res == sizeof(unsigned long long));
+	/* we read and write each loop, so subtract the ones we are counting */
+	needed = 0;
+	if (readwriteflag & HW_BREAKPOINT_R)
+		needed += loop_num;
+	if (readwriteflag & HW_BREAKPOINT_W)
+		needed += loop_num;
+	needed = needed * (1 - exclude_user);
+	printf("TESTED: addr:0x%lx brks:% 8lld loops:% 8i rw:%i !user:%i array:%i\n",
+	       (unsigned long int)ptr, breaks, loop_num, readwriteflag, exclude_user, arraytest);
+	if (breaks != needed) {
+		printf("FAILED: 0x%lx brks:%lld needed:%lli %i %i %i\n\n",
+		       (unsigned long int)ptr, breaks, needed, loop_num, readwriteflag, exclude_user);
+		return 1;
+	}
+	close(break_fd);
+
+	return 0;
+}
+
+static int runtest(void)
+{
+	int rwflag;
+	int exclude_user;
+	int ret;
+
+	/*
+	 * perf defines rwflag as two bits read and write and at least
+	 * one must be set.  So range 1-3.
+	 */
+	for (rwflag = 1 ; rwflag < 4; rwflag++) {
+		for (exclude_user = 0 ; exclude_user < 2; exclude_user++) {
+			ret = runtestsingle(rwflag, exclude_user, 0);
+			if (ret)
+				return ret;
+
+			/* if we have the dawr, we can do an array test */
+			if (!dawr_supported())
+				continue;
+			ret = runtestsingle(rwflag, exclude_user, 1);
+			if (ret)
+				return ret;
+		}
+	}
+	return 0;
+}
+
+
+static int perf_hwbreak(void)
+{
+	srand ( time(NULL) );
+
+	SKIP_IF(!perf_breakpoint_supported());
+
+	return runtest();
+}
+
+int main(int argc, char *argv[], char **envp)
+{
+	return test_harness(perf_hwbreak, "perf_hwbreak");
+}
-- 
2.17.0

^ permalink raw reply related

* Re: [RFC V2] virtio: Add platform specific DMA API translation for virito devices
From: Benjamin Herrenschmidt @ 2018-05-28 23:48 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Anshuman Khandual, virtualization, linux-kernel, linuxppc-dev,
	aik, robh, joe, elfring, david, jasowang, mpe, hch
In-Reply-To: <20180525202300-mutt-send-email-mst@kernel.org>

On Fri, 2018-05-25 at 20:45 +0300, Michael S. Tsirkin wrote:
> On Thu, May 24, 2018 at 08:27:04AM +1000, Benjamin Herrenschmidt wrote:
> > On Wed, 2018-05-23 at 21:50 +0300, Michael S. Tsirkin wrote:
> > 
> > > I re-read that discussion and I'm still unclear on the
> > > original question, since I got several apparently
> > > conflicting answers.
> > > 
> > > I asked:
> > > 
> > > 	Why isn't setting VIRTIO_F_IOMMU_PLATFORM on the
> > > 	hypervisor side sufficient?
> > 
> > I thought I had replied to this...
> > 
> > There are a couple of reasons:
> > 
> > - First qemu doesn't know that the guest will switch to "secure mode"
> > in advance. There is no difference between a normal and a secure
> > partition until the partition does the magic UV call to "enter secure
> > mode" and qemu doesn't see any of it. So who can set the flag here ?
> 
> Not sure I understand. Just set the flag e.g. on qemu command line.
> I might be wrong, but these secure mode things usually
> a. require hypervisor side tricks anyway

The way our secure mode architecture is designed, there doesn't need at
this point to be any knowledge at qemu level whatsoever. Well at least
until we do migration but that's a different kettle of fish. In any
case, the guest starts normally (which means as a non-secure guest, and
thus expects normal virtio, our FW today doesn't handle
VIRTIO_F_IOMMU_PLATFORM, though granted, we can fix this), and later
that guest issues some special Ultravisor call that turns it into a
secure guest.

There is some involvement of the hypervisor, but not qemu at this
stage. We would very much like to avoid that, as it would be a hassle
for users to have to use different libvirt options etc... bcs the guest
might turn itself into a secure VM.

> > - Second, when using VIRTIO_F_IOMMU_PLATFORM, we also make qemu (or
> > vhost) go through the emulated MMIO for every access to the guest,
> > which adds additional overhead.
> > 
> > Cheers,
> > Ben.
> 
> Well it's not supposed to be much slower for the static case.
> 
> vhost has a cache so should be fine.
> 
> A while ago Paolo implemented a translation cache which should be
> perfect for this case - most of the code got merged but
> never enabled because of stability issues.
> 
> If all else fails, we could teach QEMU to handle the no-iommu case
> as if VIRTIO_F_IOMMU_PLATFORM was off.

Any serious reason why not just getting that 2 line patch allowing our
arch code to force virtio to use the DMA API ?

It's not particularly invasive and solves our problem rather nicely
without adding overhead or additional knowledge to qemu/libvirt/mgmnt
tools etc... that it doesn't need etc....

The guest knows it's going secure so the guest arch code can do the
right thing rather trivially.

Long term we should probably make virtio always use the DMA API anyway,
and interpose "1:1" dma_ops for the traditional virtio case, that would
reduce code clutter significantly. In that case, it would become just a
matter of having a platform hook to override the dma_ops used.

Cheers,
Ben.

> 
> 
> > > 
> > > 
> > > >  arch/powerpc/include/asm/dma-mapping.h |  6 ++++++
> > > >  arch/powerpc/platforms/pseries/iommu.c | 11 +++++++++++
> > > >  drivers/virtio/virtio_ring.c           | 10 ++++++++++
> > > >  3 files changed, 27 insertions(+)
> > > > 
> > > > diff --git a/arch/powerpc/include/asm/dma-mapping.h b/arch/powerpc/include/asm/dma-mapping.h
> > > > index 8fa3945..056e578 100644
> > > > --- a/arch/powerpc/include/asm/dma-mapping.h
> > > > +++ b/arch/powerpc/include/asm/dma-mapping.h
> > > > @@ -115,4 +115,10 @@ extern u64 __dma_get_required_mask(struct device *dev);
> > > >  #define ARCH_HAS_DMA_MMAP_COHERENT
> > > >  
> > > >  #endif /* __KERNEL__ */
> > > > +
> > > > +#define platform_forces_virtio_dma platform_forces_virtio_dma
> > > > +
> > > > +struct virtio_device;
> > > > +
> > > > +extern bool platform_forces_virtio_dma(struct virtio_device *vdev);
> > > >  #endif	/* _ASM_DMA_MAPPING_H */
> > > > diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> > > > index 06f0296..a2ec15a 100644
> > > > --- a/arch/powerpc/platforms/pseries/iommu.c
> > > > +++ b/arch/powerpc/platforms/pseries/iommu.c
> > > > @@ -38,6 +38,7 @@
> > > >  #include <linux/of.h>
> > > >  #include <linux/iommu.h>
> > > >  #include <linux/rculist.h>
> > > > +#include <linux/virtio.h>
> > > >  #include <asm/io.h>
> > > >  #include <asm/prom.h>
> > > >  #include <asm/rtas.h>
> > > > @@ -1396,3 +1397,13 @@ static int __init disable_multitce(char *str)
> > > >  __setup("multitce=", disable_multitce);
> > > >  
> > > >  machine_subsys_initcall_sync(pseries, tce_iommu_bus_notifier_init);
> > > > +
> > > > +bool platform_forces_virtio_dma(struct virtio_device *vdev)
> > > > +{
> > > > +	/*
> > > > +	 * On protected guest platforms, force virtio core to use DMA
> > > > +	 * MAP API for all virtio devices. But there can also be some
> > > > +	 * exceptions for individual devices like virtio balloon.
> > > > +	 */
> > > > +	return (of_find_compatible_node(NULL, NULL, "ibm,ultravisor") != NULL);
> > > > +}
> > > 
> > > Isn't this kind of slow?  vring_use_dma_api is on
> > > data path and supposed to be very fast.
> > > 
> > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > > index 21d464a..47ea6c3 100644
> > > > --- a/drivers/virtio/virtio_ring.c
> > > > +++ b/drivers/virtio/virtio_ring.c
> > > > @@ -141,8 +141,18 @@ struct vring_virtqueue {
> > > >   * unconditionally on data path.
> > > >   */
> > > >  
> > > > +#ifndef platform_forces_virtio_dma
> > > > +static inline bool platform_forces_virtio_dma(struct virtio_device *vdev)
> > > > +{
> > > > +	return false;
> > > > +}
> > > > +#endif
> > > > +
> > > >  static bool vring_use_dma_api(struct virtio_device *vdev)
> > > >  {
> > > > +	if (platform_forces_virtio_dma(vdev))
> > > > +		return true;
> > > > +
> > > >  	if (!virtio_has_iommu_quirk(vdev))
> > > >  		return true;
> > > >  
> > > > -- 
> > > > 2.9.3

^ permalink raw reply

* Re: [RFC V2] virtio: Add platform specific DMA API translation for virito devices
From: Benjamin Herrenschmidt @ 2018-05-28 23:56 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Anshuman Khandual, virtualization, linux-kernel, linuxppc-dev,
	aik, robh, joe, elfring, david, jasowang, mpe, hch
In-Reply-To: <6fff9f5d67361653e6072570a857cf0d1009a123.camel@kernel.crashing.org>

On Tue, 2018-05-29 at 09:48 +1000, Benjamin Herrenschmidt wrote:
> > Well it's not supposed to be much slower for the static case.
> > 
> > vhost has a cache so should be fine.
> > 
> > A while ago Paolo implemented a translation cache which should be
> > perfect for this case - most of the code got merged but
> > never enabled because of stability issues.
> > 
> > If all else fails, we could teach QEMU to handle the no-iommu case
> > as if VIRTIO_F_IOMMU_PLATFORM was off.
> 
> Any serious reason why not just getting that 2 line patch allowing our
> arch code to force virtio to use the DMA API ?
> 
> It's not particularly invasive and solves our problem rather nicely
> without adding overhead or additional knowledge to qemu/libvirt/mgmnt
> tools etc... that it doesn't need etc....
> 
> The guest knows it's going secure so the guest arch code can do the
> right thing rather trivially.
> 
> Long term we should probably make virtio always use the DMA API anyway,
> and interpose "1:1" dma_ops for the traditional virtio case, that would
> reduce code clutter significantly. In that case, it would become just a
> matter of having a platform hook to override the dma_ops used.

To elaborate a bit ....

What we are trying to solve here is entirely a guest problem, I don't
think involving qemu in the solution is the right thing to do.

The guest can only allow external parties (qemu, potentially PCI
devices, etc...) access to some restricted portions of memory (insecure
memory). Thus the guest need to do some bounce buffering/swiotlb type
tricks.

This is completely orthogonal to whether there is an actual iommu
between the guest and the device (or emulated device/virtio).

This is why I think the solution should reside in the guest kernel, by
proper manipulation (by the arch code) of the dma ops.

I don't think forcing the addition of an emulated iommu in the middle
just to work around the fact that virtio "cheats" and doesn't use the
dma API unless there is one, is the right "fix".

The right long term fix is to always use the DMA API, reducing code
path etc... and just have a single point where virtio can "chose"
alternate DMA ops (via an arch hook to deal with our case).

In the meantime, having the hook we propose gets us going, but if you
agree with the approach, we should also work on the long term approach.

Cheers,
Ben.

^ permalink raw reply

* [PATCH] printk: make printk_safe_flush safe in NMI context
From: Hoeun Ryu @ 2018-05-28 23:34 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Petr Mladek, Sergey Senozhatsky, Steven Rostedt, Eric Biederman,
	Nicholas Piggin, Cyril Bur, Ram Pai, Christophe Leroy,
	Michael Neuling, Thomas Gleixner, Don Zickus, Ingo Molnar,
	Andrew Morton, Kees Cook, Borislav Petkov, Andi Kleen,
	Josh Poimboeuf, Liu, Changcheng, Greg Kroah-Hartman
  Cc: Hoeun Ryu, Philippe Ombredanne, linuxppc-dev, linux-kernel, kexec

From: Hoeun Ryu <hoeun.ryu@lge.com>

 Make printk_safe_flush() safe in NMI context. And printk_safe_flush_on_panic() is
folded into this function. The prototype of printk_safe_flush() is changed to
"void printk_safe_flush(bool panic)".

 nmi_trigger_cpumask_backtrace() can be called in NMI context. For example the
function is called in watchdog_overflow_callback() if the flag of hardlockup
backtrace (sysctl_hardlockup_all_cpu_backtrace) is true and
watchdog_overflow_callback() function is called in NMI context on some
architectures.
 Calling printk_safe_flush() in nmi_trigger_cpumask_backtrace() eventually tries to
lock logbuf_lock in vprintk_emit() but the logbuf_lock can be already locked in
preempted contexts (task or irq in this case) or by other CPUs and it may cause
deadlocks.
 By making printk_safe_flush() safe in NMI context, the backtrace triggering CPU
just skips flushing if the lock is not avaiable in NMI context. The messages in
per-cpu nmi buffer of the backtrace triggering CPU can be lost if the CPU is in
hard lockup (because irq is disabled here) but if panic() is not called. The
flushing can be delayed by the next irq work in normal cases.

Suggested-by: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>
Signed-off-by: Hoeun Ryu <hoeun.ryu@lge.com>
---
 arch/powerpc/kernel/traps.c    |  2 +-
 arch/powerpc/kernel/watchdog.c |  2 +-
 include/linux/printk.h         |  9 ++----
 kernel/kexec_core.c            |  2 +-
 kernel/panic.c                 |  4 +--
 kernel/printk/printk_safe.c    | 62 +++++++++++++++++++++---------------------
 lib/nmi_backtrace.c            |  2 +-
 7 files changed, 39 insertions(+), 44 deletions(-)

diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 0904492..c50749c 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -160,7 +160,7 @@ extern void panic_flush_kmsg_start(void)
 
 extern void panic_flush_kmsg_end(void)
 {
-	printk_safe_flush_on_panic();
+	printk_safe_flush(true);
 	kmsg_dump(KMSG_DUMP_PANIC);
 	bust_spinlocks(0);
 	debug_locks_off();
diff --git a/arch/powerpc/kernel/watchdog.c b/arch/powerpc/kernel/watchdog.c
index 6256dc3..3c9138b 100644
--- a/arch/powerpc/kernel/watchdog.c
+++ b/arch/powerpc/kernel/watchdog.c
@@ -173,7 +173,7 @@ static void watchdog_smp_panic(int cpu, u64 tb)
 
 	wd_smp_unlock(&flags);
 
-	printk_safe_flush();
+	printk_safe_flush(false);
 	/*
 	 * printk_safe_flush() seems to require another print
 	 * before anything actually goes out to console.
diff --git a/include/linux/printk.h b/include/linux/printk.h
index 6d7e800..495fe26 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -203,8 +203,7 @@ void dump_stack_print_info(const char *log_lvl);
 void show_regs_print_info(const char *log_lvl);
 extern asmlinkage void dump_stack(void) __cold;
 extern void printk_safe_init(void);
-extern void printk_safe_flush(void);
-extern void printk_safe_flush_on_panic(void);
+extern void printk_safe_flush(bool panic);
 #else
 static inline __printf(1, 0)
 int vprintk(const char *s, va_list args)
@@ -273,11 +272,7 @@ static inline void printk_safe_init(void)
 {
 }
 
-static inline void printk_safe_flush(void)
-{
-}
-
-static inline void printk_safe_flush_on_panic(void)
+static inline void printk_safe_flush(bool panic)
 {
 }
 #endif
diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 20fef1a..1b0876e 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -961,7 +961,7 @@ void crash_kexec(struct pt_regs *regs)
 	old_cpu = atomic_cmpxchg(&panic_cpu, PANIC_CPU_INVALID, this_cpu);
 	if (old_cpu == PANIC_CPU_INVALID) {
 		/* This is the 1st CPU which comes here, so go ahead. */
-		printk_safe_flush_on_panic();
+		printk_safe_flush(true);
 		__crash_kexec(regs);
 
 		/*
diff --git a/kernel/panic.c b/kernel/panic.c
index 42e4874..2f2c86c 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -193,7 +193,7 @@ void panic(const char *fmt, ...)
 	 * Bypass the panic_cpu check and call __crash_kexec directly.
 	 */
 	if (!_crash_kexec_post_notifiers) {
-		printk_safe_flush_on_panic();
+		printk_safe_flush(true);
 		__crash_kexec(NULL);
 
 		/*
@@ -218,7 +218,7 @@ void panic(const char *fmt, ...)
 	atomic_notifier_call_chain(&panic_notifier_list, 0, buf);
 
 	/* Call flush even twice. It tries harder with a single online CPU */
-	printk_safe_flush_on_panic();
+	printk_safe_flush(true);
 	kmsg_dump(KMSG_DUMP_PANIC);
 
 	/*
diff --git a/kernel/printk/printk_safe.c b/kernel/printk/printk_safe.c
index 3e3c200..35ea941 100644
--- a/kernel/printk/printk_safe.c
+++ b/kernel/printk/printk_safe.c
@@ -244,49 +244,49 @@ static void __printk_safe_flush(struct irq_work *work)
 }
 
 /**
- * printk_safe_flush - flush all per-cpu nmi buffers.
+ * printk_safe_flush - flush all per-cpu nmi buffers. it can be called even in NMI
+ *                     context.
+ * @panic: true when the system goes down. It does the best effort to get NMI messages
+ *         into the main ring buffer. Note that it could try harder when there is only
+ *         one CPU online.
  *
- * The buffers are flushed automatically via IRQ work. This function
+ * The buffers are flushed automatically via IRQ work in normal cases. This function
  * is useful only when someone wants to be sure that all buffers have
  * been flushed at some point.
  */
-void printk_safe_flush(void)
+void printk_safe_flush(bool panic)
 {
 	int cpu;
 
-	for_each_possible_cpu(cpu) {
-#ifdef CONFIG_PRINTK_NMI
-		__printk_safe_flush(&per_cpu(nmi_print_seq, cpu).work);
-#endif
-		__printk_safe_flush(&per_cpu(safe_print_seq, cpu).work);
-	}
-}
-
-/**
- * printk_safe_flush_on_panic - flush all per-cpu nmi buffers when the system
- *	goes down.
- *
- * Similar to printk_safe_flush() but it can be called even in NMI context when
- * the system goes down. It does the best effort to get NMI messages into
- * the main ring buffer.
- *
- * Note that it could try harder when there is only one CPU online.
- */
-void printk_safe_flush_on_panic(void)
-{
 	/*
 	 * Make sure that we could access the main ring buffer.
-	 * Do not risk a double release when more CPUs are up.
+	 * Do not risk a double release when more CPUs are up on panic.
 	 */
-	if (in_nmi() && raw_spin_is_locked(&logbuf_lock)) {
-		if (num_online_cpus() > 1)
+	if (this_cpu_read(printk_context) & PRINTK_NMI_CONTEXT_MASK) {
+		if (panic) {
+			if (num_online_cpus() > 1)
+				return;
+
+			debug_locks_off();
+			raw_spin_lock_init(&logbuf_lock);
+		} else {
+			/*
+			 * Just avoid deadlocks here, we could loose the messages in
+			 * per-cpu nmi buffer in the case that hardlockup happens but
+			 * panic() is not called (irq_work won't work).
+			 * The flushing can be delayed by the next irq_work if flushing
+			 * is skiped here in normal cases.
+			 */
 			return;
-
-		debug_locks_off();
-		raw_spin_lock_init(&logbuf_lock);
+		}
 	}
 
-	printk_safe_flush();
+	for_each_possible_cpu(cpu) {
+#ifdef CONFIG_PRINTK_NMI
+		__printk_safe_flush(&per_cpu(nmi_print_seq, cpu).work);
+#endif
+		__printk_safe_flush(&per_cpu(safe_print_seq, cpu).work);
+	}
 }
 
 #ifdef CONFIG_PRINTK_NMI
@@ -404,5 +404,5 @@ void __init printk_safe_init(void)
 	printk_safe_irq_ready = 1;
 
 	/* Flush pending messages that did not have scheduled IRQ works. */
-	printk_safe_flush();
+	printk_safe_flush(false);
 }
diff --git a/lib/nmi_backtrace.c b/lib/nmi_backtrace.c
index 61a6b5a..6781698 100644
--- a/lib/nmi_backtrace.c
+++ b/lib/nmi_backtrace.c
@@ -79,7 +79,7 @@ void nmi_trigger_cpumask_backtrace(const cpumask_t *mask,
 	 * Force flush any remote buffers that might be stuck in IRQ context
 	 * and therefore could not run their irq_work.
 	 */
-	printk_safe_flush();
+	printk_safe_flush(false);
 
 	clear_bit_unlock(0, &backtrace_flag);
 	put_cpu();
-- 
2.1.4

^ permalink raw reply related

* Re: [PATCH-RESEND] cxl: Disable prefault_mode in Radix mode
From: Michael Ellerman @ 2018-05-29  1:18 UTC (permalink / raw)
  To: Vaibhav Jain, linuxppc-dev, Frederic Barrat
  Cc: Philippe Bergheaud, Vaibhav Jain, stable, Alastair D'Silva,
	Andrew Donnellan, Vaibhav Jain, Christophe Lombard
In-Reply-To: <20180518094223.786-1-vaibhav@linux.vnet.ibm.com>

Vaibhav Jain <vaibhav@linux.vnet.ibm.com> writes:

> From: Vaibhav Jain <vaibhav@linux.ibm.com>
>
> Currently we see a kernel-oops reported on Power-9 while attaching a
> context to an AFU, with radix-mode and sysfs attr 'prefault_mode' set
> to anything other than 'none'. The backtrace of the oops is of this
> form:
>
> Unable to handle kernel paging request for data at address 0x00000080
> Faulting instruction address: 0xc00800000bcf3b20
> cpu 0x1: Vector: 300 (Data Access) at [c00000037f003800]
>     pc: c00800000bcf3b20: cxl_load_segment+0x178/0x290 [cxl]
>     lr: c00800000bcf39f0: cxl_load_segment+0x48/0x290 [cxl]
>     sp: c00000037f003a80
>    msr: 9000000000009033
>    dar: 80
>  dsisr: 40000000
>   current = 0xc00000037f280000
>   paca    = 0xc0000003ffffe600   softe: 3        irq_happened: 0x01
>     pid   = 3529, comm = afp_no_int
> <snip>
> [c00000037f003af0] c00800000bcf4424 cxl_prefault+0xfc/0x248 [cxl]
> [c00000037f003b50] c00800000bcf8a40 process_element_entry_psl9+0xd8/0x1a0 [cxl]
> [c00000037f003b90] c00800000bcf944c cxl_attach_dedicated_process_psl9+0x44/0x130 [cxl]
> [c00000037f003bd0] c00800000bcf5448 native_attach_process+0xc0/0x130 [cxl]
> [c00000037f003c50] c00800000bcf16cc afu_ioctl+0x3f4/0x5e0 [cxl]
> [c00000037f003d00] c00000000039d98c do_vfs_ioctl+0xdc/0x890
> [c00000037f003da0] c00000000039e1a8 ksys_ioctl+0x68/0xf0
> [c00000037f003df0] c00000000039e270 sys_ioctl+0x40/0xa0
> [c00000037f003e30] c00000000000b320 system_call+0x58/0x6c
> --- Exception: c01 (System Call) at 0000000010053bb0
  ^^^
This tells patch/git-am to drop the rest of the change log, which is not
what we want.

I tend to indent stack traces etc with two spaces, which avoids the
problem. Or in this case we can just drop the line as it's not really
that informative.

I've fixed it up.

cheers

^ permalink raw reply

* Re: [PATCH] powerpc/mm/hugetlb: Use the correct page size when flushing hugepage tlb
From: Michael Ellerman @ 2018-05-29  1:33 UTC (permalink / raw)
  To: Aneesh Kumar K.V, benh, paulus; +Cc: linuxppc-dev, Aneesh Kumar K.V
In-Reply-To: <20180522091209.9084-1-aneesh.kumar@linux.ibm.com>

"Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:

> We used wrong page size (mmu_virtual_psize) when doing a tlbflush after
> pte update. This patch update the flush to use hugetlb page size.
> The page size is derived from hugetlb hstate.

This sounds bad. Or is it not for some reason?

Either way a Fixes tag would be nice. Maybe:

  Fixes: b3603e174fc8 ("powerpc/mm: update radix__ptep_set_access_flag to not do full mm tlb flush")

I think this is only a problem on Radix, but the change log doesn't say.

cheers

> Now that ptep_set_access_flags won't be called for hugetlb remove
> the is_vm_hugetlb_page() check and add the assert of pte lock
> unconditionally.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/hugetlb.h | 19 +++--------------
>  arch/powerpc/mm/pgtable.c          | 33 ++++++++++++++++++++++++++++--
>  2 files changed, 34 insertions(+), 18 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/hugetlb.h b/arch/powerpc/include/asm/hugetlb.h
> index 78540c074d70..b4404a6da74f 100644
> --- a/arch/powerpc/include/asm/hugetlb.h
> +++ b/arch/powerpc/include/asm/hugetlb.h
> @@ -166,22 +166,9 @@ static inline pte_t huge_pte_wrprotect(pte_t pte)
>  	return pte_wrprotect(pte);
>  }
>  
> -static inline int huge_ptep_set_access_flags(struct vm_area_struct *vma,
> -					     unsigned long addr, pte_t *ptep,
> -					     pte_t pte, int dirty)
> -{
> -#ifdef HUGETLB_NEED_PRELOAD
> -	/*
> -	 * The "return 1" forces a call of update_mmu_cache, which will write a
> -	 * TLB entry.  Without this, platforms that don't do a write of the TLB
> -	 * entry in the TLB miss handler asm will fault ad infinitum.
> -	 */
> -	ptep_set_access_flags(vma, addr, ptep, pte, dirty);
> -	return 1;
> -#else
> -	return ptep_set_access_flags(vma, addr, ptep, pte, dirty);
> -#endif
> -}
> +extern int huge_ptep_set_access_flags(struct vm_area_struct *vma,
> +				      unsigned long addr, pte_t *ptep,
> +				      pte_t pte, int dirty);
>  
>  static inline pte_t huge_ptep_get(pte_t *ptep)
>  {
> diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c
> index 9f361ae571e9..e70af9939379 100644
> --- a/arch/powerpc/mm/pgtable.c
> +++ b/arch/powerpc/mm/pgtable.c
> @@ -221,14 +221,43 @@ int ptep_set_access_flags(struct vm_area_struct *vma, unsigned long address,
>  	entry = set_access_flags_filter(entry, vma, dirty);
>  	changed = !pte_same(*(ptep), entry);
>  	if (changed) {
> -		if (!is_vm_hugetlb_page(vma))
> -			assert_pte_locked(vma->vm_mm, address);
> +		assert_pte_locked(vma->vm_mm, address);
>  		__ptep_set_access_flags(vma->vm_mm, ptep, entry, address);
>  		flush_tlb_page(vma, address);
>  	}
>  	return changed;
>  }
>  
> +#ifdef CONFIG_HUGETLB_PAGE
> +extern int huge_ptep_set_access_flags(struct vm_area_struct *vma,
> +				      unsigned long addr, pte_t *ptep,
> +				      pte_t pte, int dirty)
> +{
> +#ifdef HUGETLB_NEED_PRELOAD
> +	/*
> +	 * The "return 1" forces a call of update_mmu_cache, which will write a
> +	 * TLB entry.  Without this, platforms that don't do a write of the TLB
> +	 * entry in the TLB miss handler asm will fault ad infinitum.
> +	 */
> +	ptep_set_access_flags(vma, addr, ptep, pte, dirty);
> +	return 1;
> +#else
> +	int changed;
> +
> +	pte = set_access_flags_filter(pte, vma, dirty);
> +	changed = !pte_same(*(ptep), pte);
> +	if (changed) {
> +#ifdef CONFIG_DEBUG_VM
> +		assert_spin_locked(&vma->vm_mm->page_table_lock);
> +#endif
> +		__ptep_set_access_flags(vma->vm_mm, ptep, pte, addr);
> +		flush_hugetlb_page(vma, addr);
> +	}
> +	return changed;
> +#endif
> +}
> +#endif /* CONFIG_HUGETLB_PAGE */
> +
>  #ifdef CONFIG_DEBUG_VM
>  void assert_pte_locked(struct mm_struct *mm, unsigned long addr)
>  {
> -- 
> 2.17.0

^ permalink raw reply

* Re: [PATCH] cpuidle/powernv : Add Description for cpuidle state
From: Stewart Smith @ 2018-05-29  1:39 UTC (permalink / raw)
  To: Abhishek Goel, rjw, daniel.lezcano, benh, paulus, mpe, linux-pm,
	linuxppc-dev, linux-kernel
  Cc: Abhishek Goel
In-Reply-To: <20180528173442.100642-1-huntbag@linux.vnet.ibm.com>

Abhishek Goel <huntbag@linux.vnet.ibm.com> writes:
> @@ -215,7 +216,7 @@ static inline void add_powernv_state(int index, const char *name,
>  				     u64 psscr_val, u64 psscr_mask)
>  {
>  	strlcpy(powernv_states[index].name, name, CPUIDLE_NAME_LEN);
> -	strlcpy(powernv_states[index].desc, name, CPUIDLE_NAME_LEN);
> +	strlcpy(powernv_states[index].desc, desc, CPUIDLE_DESC_LEN);

We should still fall back to using name in the event of desc being null,
as not all firmware will expose the descriptions.

> @@ -311,6 +313,11 @@ static int powernv_add_idle_states(void)
>  		pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-names in DT\n");
>  		goto out;
>  	}
> +	if (of_property_read_string_array(power_mgt,
> +		"ibm,cpu-idle-state-descs", descs, dt_idle_states) < 0) {
> +		pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-descs in DT\n");
> +		goto out;
> +	}

I don't think pr_warn is appropriate here, as for all current released
firmware we don't have that property. I think perhaps just silently
continuing on is okay, as we have to keep compatibility with that firmware.

> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -17,7 +17,7 @@
>
>  #define CPUIDLE_STATE_MAX	10
>  #define CPUIDLE_NAME_LEN	16
> -#define CPUIDLE_DESC_LEN	32
> +#define CPUIDLE_DESC_LEN	60

Do we really get that long?

-- 
Stewart Smith
OPAL Architect, IBM.

^ permalink raw reply

* Re: [PATCH-RESEND] cxl: Disable prefault_mode in Radix mode
From: Vaibhav Jain @ 2018-05-29  1:55 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev, Frederic Barrat
  Cc: Philippe Bergheaud, stable, Alastair D'Silva,
	Andrew Donnellan, Christophe Lombard
In-Reply-To: <871sdv5kev.fsf@concordia.ellerman.id.au>

Michael Ellerman <mpe@ellerman.id.au> writes:

> Vaibhav Jain <vaibhav@linux.vnet.ibm.com> writes:
>
>> From: Vaibhav Jain <vaibhav@linux.ibm.com>
>>
>> Currently we see a kernel-oops reported on Power-9 while attaching a
>> context to an AFU, with radix-mode and sysfs attr 'prefault_mode' set
>> to anything other than 'none'. The backtrace of the oops is of this
>> form:
>>
>> Unable to handle kernel paging request for data at address 0x00000080
>> Faulting instruction address: 0xc00800000bcf3b20
>> cpu 0x1: Vector: 300 (Data Access) at [c00000037f003800]
>>     pc: c00800000bcf3b20: cxl_load_segment+0x178/0x290 [cxl]
>>     lr: c00800000bcf39f0: cxl_load_segment+0x48/0x290 [cxl]
>>     sp: c00000037f003a80
>>    msr: 9000000000009033
>>    dar: 80
>>  dsisr: 40000000
>>   current = 0xc00000037f280000
>>   paca    = 0xc0000003ffffe600   softe: 3        irq_happened: 0x01
>>     pid   = 3529, comm = afp_no_int
>> <snip>
>> [c00000037f003af0] c00800000bcf4424 cxl_prefault+0xfc/0x248 [cxl]
>> [c00000037f003b50] c00800000bcf8a40 process_element_entry_psl9+0xd8/0x1a0 [cxl]
>> [c00000037f003b90] c00800000bcf944c cxl_attach_dedicated_process_psl9+0x44/0x130 [cxl]
>> [c00000037f003bd0] c00800000bcf5448 native_attach_process+0xc0/0x130 [cxl]
>> [c00000037f003c50] c00800000bcf16cc afu_ioctl+0x3f4/0x5e0 [cxl]
>> [c00000037f003d00] c00000000039d98c do_vfs_ioctl+0xdc/0x890
>> [c00000037f003da0] c00000000039e1a8 ksys_ioctl+0x68/0xf0
>> [c00000037f003df0] c00000000039e270 sys_ioctl+0x40/0xa0
>> [c00000037f003e30] c00000000000b320 system_call+0x58/0x6c
>> --- Exception: c01 (System Call) at 0000000010053bb0
>   ^^^
> This tells patch/git-am to drop the rest of the change log, which is not
> what we want.
>
> I tend to indent stack traces etc with two spaces, which avoids the
> problem. Or in this case we can just drop the line as it's not really
> that informative.
>
> I've fixed it up.
>
> cheers
>

Sorry for missing that and thanks for fixing it in the patch.

-- 
Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
Linux Technology Center, IBM India Pvt. Ltd.

^ permalink raw reply

* Re: [PATCH] powerpc/mm/hugetlb: Use the correct page size when flushing hugepage tlb
From: Nicholas Piggin @ 2018-05-29  2:09 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: Aneesh Kumar K.V, benh, paulus, linuxppc-dev
In-Reply-To: <87y3g3454r.fsf@concordia.ellerman.id.au>

On Tue, 29 May 2018 11:33:56 +1000
Michael Ellerman <mpe@ellerman.id.au> wrote:

> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> 
> > We used wrong page size (mmu_virtual_psize) when doing a tlbflush after
> > pte update. This patch update the flush to use hugetlb page size.
> > The page size is derived from hugetlb hstate.  
> 
> This sounds bad. Or is it not for some reason?

It's not all that bad because the flush is mostly superfluous (one of
my tlbie optimisation patches gets rid of it except for accelerators).

> 
> Either way a Fixes tag would be nice. Maybe:
> 
>   Fixes: b3603e174fc8 ("powerpc/mm: update radix__ptep_set_access_flag to not do full mm tlb flush")
> 
> I think this is only a problem on Radix, but the change log doesn't say.

huge_ptep_set_access_flags->ptep_set_access_flags->flush_tlb_page->

void radix__flush_tlb_page(struct vm_area_struct *vma, unsigned long vmaddr)
{
#ifdef CONFIG_HUGETLB_PAGE
        if (is_vm_hugetlb_page(vma))
                return radix__flush_hugetlb_page(vma, vmaddr);
#endif
        radix__flush_tlb_page_psize(vma->vm_mm, vmaddr, mmu_virtual_psize);
}

So I'm still not sure how the size is going wrong here. What am I
missig?

Thanks,
Nick

^ permalink raw reply

* Re: [PATCH] powerpc/mm/hugetlb: Use the correct page size when flushing hugepage tlb
From: Aneesh Kumar K.V @ 2018-05-29  2:38 UTC (permalink / raw)
  To: Nicholas Piggin, Michael Ellerman; +Cc: benh, paulus, linuxppc-dev
In-Reply-To: <20180529120950.547e3a6c@roar.ozlabs.ibm.com>

On 05/29/2018 07:39 AM, Nicholas Piggin wrote:
> On Tue, 29 May 2018 11:33:56 +1000
> Michael Ellerman <mpe@ellerman.id.au> wrote:
> 
>> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
>>
>>> We used wrong page size (mmu_virtual_psize) when doing a tlbflush after
>>> pte update. This patch update the flush to use hugetlb page size.
>>> The page size is derived from hugetlb hstate.
>>
>> This sounds bad. Or is it not for some reason?
> 
> It's not all that bad because the flush is mostly superfluous (one of
> my tlbie optimisation patches gets rid of it except for accelerators).
> 
>>
>> Either way a Fixes tag would be nice. Maybe:
>>
>>    Fixes: b3603e174fc8 ("powerpc/mm: update radix__ptep_set_access_flag to not do full mm tlb flush")
>>
>> I think this is only a problem on Radix, but the change log doesn't say.
> 
> huge_ptep_set_access_flags->ptep_set_access_flags->flush_tlb_page->
> 
> void radix__flush_tlb_page(struct vm_area_struct *vma, unsigned long vmaddr)
> {
> #ifdef CONFIG_HUGETLB_PAGE
>          if (is_vm_hugetlb_page(vma))
>                  return radix__flush_hugetlb_page(vma, vmaddr);
> #endif
>          radix__flush_tlb_page_psize(vma->vm_mm, vmaddr, mmu_virtual_psize);
> }
> 
> So I'm still not sure how the size is going wrong here. What am I
> missig?
> 

My mistake, I didn't look at the radix expansion. I was looking at 
huge_ptep_clear_flush() where we use flush_hugetlb_page().

-aneesh

^ permalink raw reply

* [PATCH v2] powerpc/64: Fix build failure with GCC 8.1
From: Christophe Leroy @ 2018-05-29  6:03 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linux-kernel, linuxppc-dev

  CC      arch/powerpc/kernel/nvram_64.o
arch/powerpc/kernel/nvram_64.c: In function 'nvram_create_partition':
arch/powerpc/kernel/nvram_64.c:1042:2: error: 'strncpy' specified bound 12 equals destination size [-Werror=stringop-truncation]
  strncpy(new_part->header.name, name, 12);
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  CC      arch/powerpc/kernel/trace/ftrace.o
In function 'make_field',
    inlined from 'ps3_repository_read_boot_dat_address' at arch/powerpc/platforms/ps3/repository.c:900:9:
arch/powerpc/platforms/ps3/repository.c:106:2: error: 'strncpy' output truncated before terminating nul copying 8 bytes from a string of the same length [-Werror=stringop-truncation]
  strncpy((char *)&n, text, 8);
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
v2:
 - Using strnlen(src, sizeof(dest)) instead of min(strlen(src), sizeof(dest))
 - Changed nvram one to memcpy() to still fit the entire fied (thanks to benh)

 arch/powerpc/kernel/nvram_64.c          | 2 +-
 arch/powerpc/platforms/ps3/repository.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
index ba681dac7b46..cf7772cdc3fd 100644
--- a/arch/powerpc/kernel/nvram_64.c
+++ b/arch/powerpc/kernel/nvram_64.c
@@ -1039,7 +1039,7 @@ loff_t __init nvram_create_partition(const char *name, int sig,
 	new_part->index = free_part->index;
 	new_part->header.signature = sig;
 	new_part->header.length = size;
-	strncpy(new_part->header.name, name, 12);
+	memcpy(new_part->header.name, name, strnlen(name, sizeof(new_part->header.name)));
 	new_part->header.checksum = nvram_checksum(&new_part->header);
 
 	rc = nvram_write_header(new_part);
diff --git a/arch/powerpc/platforms/ps3/repository.c b/arch/powerpc/platforms/ps3/repository.c
index 50dbaf24b1ee..e49c887787c4 100644
--- a/arch/powerpc/platforms/ps3/repository.c
+++ b/arch/powerpc/platforms/ps3/repository.c
@@ -101,9 +101,9 @@ static u64 make_first_field(const char *text, u64 index)
 
 static u64 make_field(const char *text, u64 index)
 {
-	u64 n;
+	u64 n = 0;
 
-	strncpy((char *)&n, text, 8);
+	memcpy((char *)&n, text, strnlen(text, sizeof(n)));
 	return n + index;
 }
 
-- 
2.13.3

^ permalink raw reply related

* [PATCH] powerpc64/module elfv1: Set opd addresses after module relocation
From: Naveen N. Rao @ 2018-05-29  6:51 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: Sergey Senozhatsky, linuxppc-dev

module_frob_arch_sections() is called before the module is moved to its
final location. The function descriptor section addresses we are setting
here are thus invalid. Fix this by processing opd section during 
module_finalize()

Fixes: 5633e85b2c313 ("powerpc64: Add .opd based function descriptor dereference")
Cc: stable@vger.kernel.org # v4.16
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
This can easily be seen by doing:

  $ sudo perf probe -L module_frob_arch_sections | grep -A5 opd
       20  		else if (!strcmp(secstrings + sechdrs[i].sh_name, ".opd")) {
       21  			me->arch.start_opd = sechdrs[i].sh_addr;
       22  			me->arch.end_opd = sechdrs[i].sh_addr +
						     sechdrs[i].sh_size;
			  }

			  /* We don't handle .init for the moment: rename to _init */
       27  		while ((p = strstr(secstrings + sechdrs[i].sh_name, ".init")))
  $ sudo perf probe module_frob_arch_sections:27 me-\>arch.start_opd me-\>arch.end_opd
  Added new events:
    probe:module_frob_arch_sections (on module_frob_arch_sections:27 with start_opd=me->arch.start_opd end_opd=me->arch.end_opd)
    probe:module_frob_arch_sections_1 (on module_frob_arch_sections:27 with start_opd=me->arch.start_opd end_opd=me->arch.end_opd)

  You can now use it in all perf tools, such as:

	  perf record -e probe:module_frob_arch_sections_1 -aR sleep 1

  $ sudo perf record -e probe:* modprobe kprobe_example
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.011 MB perf.data (48 samples) ]
  $ sudo perf script
	  modprobe 10463 [001] 311838.332208:   probe:module_frob_arch_sections: (c000000000043b0c) start_opd=0xd000000000910750 end_opd=0xd0000000009107a0
	  modprobe 10463 [001] 311838.332209:   probe:module_frob_arch_sections: (c000000000043b0c) start_opd=0xd000000000910750 end_opd=0xd0000000009107a0
  $ sudo cat /proc/modules | grep kprobe_example
  kprobe_example 3716 0 - Live 0xd000000000970000

With this patch, probing on module_finalize() shows the expected values.


- Naveen


 arch/powerpc/kernel/module.c    | 8 ++++++++
 arch/powerpc/kernel/module_64.c | 5 -----
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kernel/module.c b/arch/powerpc/kernel/module.c
index 3f7ba0f5bf29..fc9fa24cfe05 100644
--- a/arch/powerpc/kernel/module.c
+++ b/arch/powerpc/kernel/module.c
@@ -72,6 +72,14 @@ int module_finalize(const Elf_Ehdr *hdr,
 		do_feature_fixups(powerpc_firmware_features,
 				  (void *)sect->sh_addr,
 				  (void *)sect->sh_addr + sect->sh_size);
+
+#ifdef PPC64_ELF_ABI_v1
+	sect = find_section(hdr, sechdrs, ".opd");
+	if (sect != NULL) {
+		me->arch.start_opd = sect->sh_addr;
+		me->arch.end_opd = sect->sh_addr + sect->sh_size;
+	}
+#endif
 #endif
 
 	sect = find_section(hdr, sechdrs, "__lwsync_fixup");
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index f7667e2ebfcb..a45204b48d56 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -360,11 +360,6 @@ int module_frob_arch_sections(Elf64_Ehdr *hdr,
 		else if (strcmp(secstrings+sechdrs[i].sh_name,"__versions")==0)
 			dedotify_versions((void *)hdr + sechdrs[i].sh_offset,
 					  sechdrs[i].sh_size);
-		else if (!strcmp(secstrings + sechdrs[i].sh_name, ".opd")) {
-			me->arch.start_opd = sechdrs[i].sh_addr;
-			me->arch.end_opd = sechdrs[i].sh_addr +
-					   sechdrs[i].sh_size;
-		}
 
 		/* We don't handle .init for the moment: rename to _init */
 		while ((p = strstr(secstrings + sechdrs[i].sh_name, ".init")))
-- 
2.17.0

^ permalink raw reply related

* [PATCH v2] powerpc: fix build failure by disabling attribute-alias warning
From: Christophe Leroy @ 2018-05-29  6:58 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linux-kernel, linuxppc-dev

Latest GCC version emit the following warnings

As arch/powerpc code is built with -Werror, this breaks build with
GCC 8.1

This patch inhibits those warnings

  CC      arch/powerpc/kernel/syscalls.o
In file included from arch/powerpc/kernel/syscalls.c:24:
./include/linux/syscalls.h:233:18: error: 'sys_mmap2' alias between functions of incompatible types 'long int(long unsigned int,  size_t,  long unsigned int,  long unsigned int,  long unsigned int,  long unsigned int)' {aka 'long int(long unsigned int,  long unsigned int,  long unsigned int,  long unsigned int,  long unsigned int,  long unsigned int)'} and 'long int(long int,  long int,  long int,  long int,  long int,  long int)' [-Werror=attribute-alias]
  asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \
                  ^~~
./include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx'
  __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
  ^~~~~~~~~~~~~~~~~
./include/linux/syscalls.h:216:36: note: in expansion of macro 'SYSCALL_DEFINEx'
 #define SYSCALL_DEFINE6(name, ...) SYSCALL_DEFINEx(6, _##name, __VA_ARGS__)
                                    ^~~~~~~~~~~~~~~
arch/powerpc/kernel/syscalls.c:65:1: note: in expansion of macro 'SYSCALL_DEFINE6'
 SYSCALL_DEFINE6(mmap2, unsigned long, addr, size_t, len,
 ^~~~~~~~~~~~~~~
./include/linux/syscalls.h:238:18: note: aliased declaration here
  asmlinkage long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__)) \
                  ^~~~~~~~
./include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx'
  __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
  ^~~~~~~~~~~~~~~~~
./include/linux/syscalls.h:216:36: note: in expansion of macro 'SYSCALL_DEFINEx'
 #define SYSCALL_DEFINE6(name, ...) SYSCALL_DEFINEx(6, _##name, __VA_ARGS__)
                                    ^~~~~~~~~~~~~~~
arch/powerpc/kernel/syscalls.c:65:1: note: in expansion of macro 'SYSCALL_DEFINE6'
 SYSCALL_DEFINE6(mmap2, unsigned long, addr, size_t, len,
 ^~~~~~~~~~~~~~~
./include/linux/syscalls.h:233:18: error: 'sys_mmap' alias between functions of incompatible types 'long int(long unsigned int,  size_t,  long unsigned int,  long unsigned int,  long unsigned int,  off_t)' {aka 'long int(long unsigned int,  long unsigned int,  long unsigned int,  long unsigned int,  long unsigned int,  long int)'} and 'long int(long int,  long int,  long int,  long int,  long int,  long int)' [-Werror=attribute-alias]
  asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \
                  ^~~
./include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx'
  __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
  ^~~~~~~~~~~~~~~~~
./include/linux/syscalls.h:216:36: note: in expansion of macro 'SYSCALL_DEFINEx'
 #define SYSCALL_DEFINE6(name, ...) SYSCALL_DEFINEx(6, _##name, __VA_ARGS__)
                                    ^~~~~~~~~~~~~~~
arch/powerpc/kernel/syscalls.c:72:1: note: in expansion of macro 'SYSCALL_DEFINE6'
 SYSCALL_DEFINE6(mmap, unsigned long, addr, size_t, len,
 ^~~~~~~~~~~~~~~
./include/linux/syscalls.h:238:18: note: aliased declaration here
  asmlinkage long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__)) \
                  ^~~~~~~~
./include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx'
  __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
  ^~~~~~~~~~~~~~~~~
./include/linux/syscalls.h:216:36: note: in expansion of macro 'SYSCALL_DEFINEx'
 #define SYSCALL_DEFINE6(name, ...) SYSCALL_DEFINEx(6, _##name, __VA_ARGS__)
                                    ^~~~~~~~~~~~~~~
arch/powerpc/kernel/syscalls.c:72:1: note: in expansion of macro 'SYSCALL_DEFINE6'
 SYSCALL_DEFINE6(mmap, unsigned long, addr, size_t, len,
 ^~~~~~~~~~~~~~~
  CC      arch/powerpc/kernel/signal_32.o
In file included from arch/powerpc/kernel/signal_32.c:31:
./include/linux/compat.h:74:18: error: 'compat_sys_swapcontext' alias between functions of incompatible types 'long int(struct ucontext32 *, struct ucontext32 *, int)' and 'long int(long int,  long int,  long int)' [-Werror=attribute-alias]
  asmlinkage long compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \
                  ^~~~~~~~~~
./include/linux/compat.h:58:2: note: in expansion of macro 'COMPAT_SYSCALL_DEFINEx'
  COMPAT_SYSCALL_DEFINEx(3, _##name, __VA_ARGS__)
  ^~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/kernel/signal_32.c:1041:1: note: in expansion of macro 'COMPAT_SYSCALL_DEFINE3'
 COMPAT_SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
 ^~~~~~~~~~~~~~~~~~~~~~
./include/linux/compat.h:79:18: note: aliased declaration here
  asmlinkage long __se_compat_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__)) \
                  ^~~~~~~~~~~~~~~
./include/linux/compat.h:58:2: note: in expansion of macro 'COMPAT_SYSCALL_DEFINEx'
  COMPAT_SYSCALL_DEFINEx(3, _##name, __VA_ARGS__)
  ^~~~~~~~~~~~~~~~~~~~~~
arch/powerpc/kernel/signal_32.c:1041:1: note: in expansion of macro 'COMPAT_SYSCALL_DEFINE3'
 COMPAT_SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
 ^~~~~~~~~~~~~~~~~~~~~~
  CC      arch/powerpc/kernel/signal_64.o
In file included from arch/powerpc/kernel/signal_64.c:27:
./include/linux/syscalls.h:233:18: error: 'sys_swapcontext' alias between functions of incompatible types 'long int(struct ucontext *, struct ucontext *, long int)' and 'long int(long int,  long int,  long int)' [-Werror=attribute-alias]
  asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \
                  ^~~
./include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx'
  __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
  ^~~~~~~~~~~~~~~~~
./include/linux/syscalls.h:213:36: note: in expansion of macro 'SYSCALL_DEFINEx'
 #define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, __VA_ARGS__)
                                    ^~~~~~~~~~~~~~~
arch/powerpc/kernel/signal_64.c:628:1: note: in expansion of macro 'SYSCALL_DEFINE3'
 SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
 ^~~~~~~~~~~~~~~
./include/linux/syscalls.h:238:18: note: aliased declaration here
  asmlinkage long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__)) \
                  ^~~~~~~~
./include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx'
  __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
  ^~~~~~~~~~~~~~~~~
./include/linux/syscalls.h:213:36: note: in expansion of macro 'SYSCALL_DEFINEx'
 #define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, __VA_ARGS__)
                                    ^~~~~~~~~~~~~~~
arch/powerpc/kernel/signal_64.c:628:1: note: in expansion of macro 'SYSCALL_DEFINE3'
 SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
 ^~~~~~~~~~~~~~~
  CC      arch/powerpc/kernel/rtas.o
In file included from arch/powerpc/kernel/rtas.c:29:
./include/linux/syscalls.h:233:18: error: 'sys_rtas' alias between functions of incompatible types 'long int(struct rtas_args *)' and 'long int(long int)' [-Werror=attribute-alias]
  asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \
                  ^~~
./include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx'
  __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
  ^~~~~~~~~~~~~~~~~
./include/linux/syscalls.h:211:36: note: in expansion of macro 'SYSCALL_DEFINEx'
 #define SYSCALL_DEFINE1(name, ...) SYSCALL_DEFINEx(1, _##name, __VA_ARGS__)
                                    ^~~~~~~~~~~~~~~
arch/powerpc/kernel/rtas.c:1054:1: note: in expansion of macro 'SYSCALL_DEFINE1'
 SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs)
 ^~~~~~~~~~~~~~~
./include/linux/syscalls.h:238:18: note: aliased declaration here
  asmlinkage long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__)) \
                  ^~~~~~~~
./include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx'
  __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
  ^~~~~~~~~~~~~~~~~
./include/linux/syscalls.h:211:36: note: in expansion of macro 'SYSCALL_DEFINEx'
 #define SYSCALL_DEFINE1(name, ...) SYSCALL_DEFINEx(1, _##name, __VA_ARGS__)
                                    ^~~~~~~~~~~~~~~
arch/powerpc/kernel/rtas.c:1054:1: note: in expansion of macro 'SYSCALL_DEFINE1'
 SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs)
 ^~~~~~~~~~~~~~~
  CC      arch/powerpc/kernel/pci_64.o
In file included from arch/powerpc/kernel/pci_64.c:23:
./include/linux/syscalls.h:233:18: error: 'sys_pciconfig_iobase' alias between functions of incompatible types 'long int(long int,  long unsigned int,  long unsigned int)' and 'long int(long int,  long int,  long int)' [-Werror=attribute-alias]
  asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \
                  ^~~
./include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx'
  __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
  ^~~~~~~~~~~~~~~~~
./include/linux/syscalls.h:213:36: note: in expansion of macro 'SYSCALL_DEFINEx'
 #define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, __VA_ARGS__)
                                    ^~~~~~~~~~~~~~~
arch/powerpc/kernel/pci_64.c:206:1: note: in expansion of macro 'SYSCALL_DEFINE3'
 SYSCALL_DEFINE3(pciconfig_iobase, long, which, unsigned long, in_bus,
 ^~~~~~~~~~~~~~~
./include/linux/syscalls.h:238:18: note: aliased declaration here
  asmlinkage long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__)) \
                  ^~~~~~~~
./include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx'
  __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
  ^~~~~~~~~~~~~~~~~
./include/linux/syscalls.h:213:36: note: in expansion of macro 'SYSCALL_DEFINEx'
 #define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, __VA_ARGS__)
                                    ^~~~~~~~~~~~~~~
arch/powerpc/kernel/pci_64.c:206:1: note: in expansion of macro 'SYSCALL_DEFINE3'
 SYSCALL_DEFINE3(pciconfig_iobase, long, which, unsigned long, in_bus,
 ^~~~~~~~~~~~~~~

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/kernel/pci_64.c    | 3 +++
 arch/powerpc/kernel/rtas.c      | 3 +++
 arch/powerpc/kernel/signal_32.c | 6 ++++++
 arch/powerpc/kernel/signal_64.c | 3 +++
 arch/powerpc/kernel/syscalls.c  | 3 +++
 arch/powerpc/mm/subpage-prot.c  | 3 +++
 6 files changed, 21 insertions(+)

diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
index dff28f903512..a8bf98c1cf0c 100644
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c
@@ -203,6 +203,8 @@ void pcibios_setup_phb_io_space(struct pci_controller *hose)
 #define IOBASE_ISA_IO		3
 #define IOBASE_ISA_MEM		4
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wattribute-alias"
 SYSCALL_DEFINE3(pciconfig_iobase, long, which, unsigned long, in_bus,
 			  unsigned long, in_devfn)
 {
@@ -256,6 +258,7 @@ SYSCALL_DEFINE3(pciconfig_iobase, long, which, unsigned long, in_bus,
 
 	return -EOPNOTSUPP;
 }
+#pragma GCC diagnostic pop
 
 #ifdef CONFIG_NUMA
 int pcibus_to_node(struct pci_bus *bus)
diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
index 8afd146bc9c7..c1713cef1642 100644
--- a/arch/powerpc/kernel/rtas.c
+++ b/arch/powerpc/kernel/rtas.c
@@ -1051,6 +1051,8 @@ struct pseries_errorlog *get_pseries_errorlog(struct rtas_error_log *log,
 }
 
 /* We assume to be passed big endian arguments */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wattribute-alias"
 SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs)
 {
 	struct rtas_args args;
@@ -1137,6 +1139,7 @@ SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs)
 
 	return 0;
 }
+#pragma GCC diagnostic pop
 
 /*
  * Call early during boot, before mem init, to retrieve the RTAS
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 9cf8a03d3bc7..db1042f37b94 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -1037,6 +1037,8 @@ static int do_setcontext_tm(struct ucontext __user *ucp,
 }
 #endif
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wattribute-alias"
 #ifdef CONFIG_PPC64
 COMPAT_SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
 		       struct ucontext __user *, new_ctx, int, ctx_size)
@@ -1132,6 +1134,7 @@ SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
 	set_thread_flag(TIF_RESTOREALL);
 	return 0;
 }
+#pragma GCC diagnostic pop
 
 #ifdef CONFIG_PPC64
 COMPAT_SYSCALL_DEFINE0(rt_sigreturn)
@@ -1228,6 +1231,8 @@ SYSCALL_DEFINE0(rt_sigreturn)
 	return 0;
 }
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wattribute-alias"
 #ifdef CONFIG_PPC32
 SYSCALL_DEFINE3(debug_setcontext, struct ucontext __user *, ctx,
 			 int, ndbg, struct sig_dbg_op __user *, dbg)
@@ -1333,6 +1338,7 @@ SYSCALL_DEFINE3(debug_setcontext, struct ucontext __user *, ctx,
 	return 0;
 }
 #endif
+#pragma GCC diagnostic pop
 
 /*
  * OK, we're invoking a handler
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index 83d51bf586c7..d2e8da8cf27f 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -625,6 +625,8 @@ static long setup_trampoline(unsigned int syscall, unsigned int __user *tramp)
 /*
  * Handle {get,set,swap}_context operations
  */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wattribute-alias"
 SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
 		struct ucontext __user *, new_ctx, long, ctx_size)
 {
@@ -690,6 +692,7 @@ SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
 	set_thread_flag(TIF_RESTOREALL);
 	return 0;
 }
+#pragma GCC diagnostic pop
 
 
 /*
diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
index 466216506eb2..d08119590185 100644
--- a/arch/powerpc/kernel/syscalls.c
+++ b/arch/powerpc/kernel/syscalls.c
@@ -62,6 +62,8 @@ static inline long do_mmap2(unsigned long addr, size_t len,
 	return ret;
 }
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wattribute-alias"
 SYSCALL_DEFINE6(mmap2, unsigned long, addr, size_t, len,
 		unsigned long, prot, unsigned long, flags,
 		unsigned long, fd, unsigned long, pgoff)
@@ -75,6 +77,7 @@ SYSCALL_DEFINE6(mmap, unsigned long, addr, size_t, len,
 {
 	return do_mmap2(addr, len, prot, flags, fd, offset, PAGE_SHIFT);
 }
+#pragma GCC diagnostic pop
 
 #ifdef CONFIG_PPC32
 /*
diff --git a/arch/powerpc/mm/subpage-prot.c b/arch/powerpc/mm/subpage-prot.c
index 9d16ee251fc0..edf09fd4ba80 100644
--- a/arch/powerpc/mm/subpage-prot.c
+++ b/arch/powerpc/mm/subpage-prot.c
@@ -186,6 +186,8 @@ static void subpage_mark_vma_nohuge(struct mm_struct *mm, unsigned long addr,
  * in a 2-bit field won't allow writes to a page that is otherwise
  * write-protected.
  */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wattribute-alias"
 SYSCALL_DEFINE3(subpage_prot, unsigned long, addr,
 		unsigned long, len, u32 __user *, map)
 {
@@ -269,3 +271,4 @@ SYSCALL_DEFINE3(subpage_prot, unsigned long, addr,
 	up_write(&mm->mmap_sem);
 	return err;
 }
+#pragma GCC diagnostic pop
-- 
2.13.3

^ permalink raw reply related

* [PATCH] powerpc/cell: fix build failure by disabling attribute-alias warning
From: Christophe Leroy @ 2018-05-29  6:58 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Arnd Bergmann
  Cc: linux-kernel, linuxppc-dev

Latest GCC version emit the following warnings

As arch/powerpc code is built with -Werror, this breaks build with
GCC 8.1

This patch inhibits those warnings

  CC      arch/powerpc/platforms/cell/spu_syscalls.o
In file included from arch/powerpc/platforms/cell/spu_syscalls.c:26:
./include/linux/syscalls.h:233:18: error: 'sys_spu_create' alias between functions of incompatible types 'long int(const char *, unsigned int,  umode_t,  int)' {aka 'long int(const char *, unsigned int,  short unsigned int,  int)'} and 'long int(long int,  long int,  long int,  long int)' [-Werror=attribute-alias]
  asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \
                  ^~~
./include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx'
  __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
  ^~~~~~~~~~~~~~~~~
./include/linux/syscalls.h:214:36: note: in expansion of macro 'SYSCALL_DEFINEx'
 #define SYSCALL_DEFINE4(name, ...) SYSCALL_DEFINEx(4, _##name, __VA_ARGS__)
                                    ^~~~~~~~~~~~~~~
arch/powerpc/platforms/cell/spu_syscalls.c:70:1: note: in expansion of macro 'SYSCALL_DEFINE4'
 SYSCALL_DEFINE4(spu_create, const char __user *, name, unsigned int, flags,
 ^~~~~~~~~~~~~~~
./include/linux/syscalls.h:238:18: note: aliased declaration here
  asmlinkage long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__)) \
                  ^~~~~~~~
./include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx'
  __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
  ^~~~~~~~~~~~~~~~~
./include/linux/syscalls.h:214:36: note: in expansion of macro 'SYSCALL_DEFINEx'
 #define SYSCALL_DEFINE4(name, ...) SYSCALL_DEFINEx(4, _##name, __VA_ARGS__)
                                    ^~~~~~~~~~~~~~~
arch/powerpc/platforms/cell/spu_syscalls.c:70:1: note: in expansion of macro 'SYSCALL_DEFINE4'
 SYSCALL_DEFINE4(spu_create, const char __user *, name, unsigned int, flags,
 ^~~~~~~~~~~~~~~
./include/linux/syscalls.h:233:18: error: 'sys_spu_run' alias between functions of incompatible types 'long int(int,  __u32 *, __u32 *)' {aka 'long int(int,  unsigned int *, unsigned int *)'} and 'long int(long int,  long int,  long int)' [-Werror=attribute-alias]
  asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \
                  ^~~
./include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx'
  __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
  ^~~~~~~~~~~~~~~~~
./include/linux/syscalls.h:213:36: note: in expansion of macro 'SYSCALL_DEFINEx'
 #define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, __VA_ARGS__)
                                    ^~~~~~~~~~~~~~~
arch/powerpc/platforms/cell/spu_syscalls.c:94:1: note: in expansion of macro 'SYSCALL_DEFINE3'
 SYSCALL_DEFINE3(spu_run,int, fd, __u32 __user *, unpc, __u32 __user *, ustatus)
 ^~~~~~~~~~~~~~~
./include/linux/syscalls.h:238:18: note: aliased declaration here
  asmlinkage long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__)) \
                  ^~~~~~~~
./include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx'
  __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
  ^~~~~~~~~~~~~~~~~
./include/linux/syscalls.h:213:36: note: in expansion of macro 'SYSCALL_DEFINEx'
 #define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, __VA_ARGS__)
                                    ^~~~~~~~~~~~~~~
arch/powerpc/platforms/cell/spu_syscalls.c:94:1: note: in expansion of macro 'SYSCALL_DEFINE3'
 SYSCALL_DEFINE3(spu_run,int, fd, __u32 __user *, unpc, __u32 __user *, ustatus)
 ^~~~~~~~~~~~~~~

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/platforms/cell/spu_syscalls.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/powerpc/platforms/cell/spu_syscalls.c b/arch/powerpc/platforms/cell/spu_syscalls.c
index 263413a34823..9afc91dfbdcd 100644
--- a/arch/powerpc/platforms/cell/spu_syscalls.c
+++ b/arch/powerpc/platforms/cell/spu_syscalls.c
@@ -67,6 +67,8 @@ static inline void spufs_calls_put(struct spufs_calls *calls) { }
 
 #endif /* CONFIG_SPU_FS_MODULE */
 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wattribute-alias"
 SYSCALL_DEFINE4(spu_create, const char __user *, name, unsigned int, flags,
 	umode_t, mode, int, neighbor_fd)
 {
@@ -111,6 +113,7 @@ SYSCALL_DEFINE3(spu_run,int, fd, __u32 __user *, unpc, __u32 __user *, ustatus)
 	spufs_calls_put(calls);
 	return ret;
 }
+#pragma GCC diagnostic pop
 
 #ifdef CONFIG_COREDUMP
 int elf_coredump_extra_notes_size(void)
-- 
2.13.3

^ permalink raw reply related

* Re: [PATCH v2] powerpc: fix build failure by disabling attribute-alias warning
From: Christophe Leroy @ 2018-05-29  7:46 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Segher Boessenkool
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <871bd59bfbec78acb31082df2c3f1be43658bcb5.1527576941.git.christophe.leroy@c-s.fr>



On 05/29/2018 06:58 AM, Christophe Leroy wrote:
> Latest GCC version emit the following warnings
> 
> As arch/powerpc code is built with -Werror, this breaks build with
> GCC 8.1

Argh ...

Now, I get the following error with older GCC:

arch/powerpc/kernel/syscalls.c:66:9: error: unknown option after 
‘#pragma GCC diagnostic’ kind [-Werror=pragmas]

Is there a way to get something compatible with both GCC versions ?

Christophe

> 
> This patch inhibits those warnings
> 
>    CC      arch/powerpc/kernel/syscalls.o
> In file included from arch/powerpc/kernel/syscalls.c:24:
> ./include/linux/syscalls.h:233:18: error: 'sys_mmap2' alias between functions of incompatible types 'long int(long unsigned int,  size_t,  long unsigned int,  long unsigned int,  long unsigned int,  long unsigned int)' {aka 'long int(long unsigned int,  long unsigned int,  long unsigned int,  long unsigned int,  long unsigned int,  long unsigned int)'} and 'long int(long int,  long int,  long int,  long int,  long int,  long int)' [-Werror=attribute-alias]
>    asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \
>                    ^~~
> ./include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx'
>    __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
>    ^~~~~~~~~~~~~~~~~
> ./include/linux/syscalls.h:216:36: note: in expansion of macro 'SYSCALL_DEFINEx'
>   #define SYSCALL_DEFINE6(name, ...) SYSCALL_DEFINEx(6, _##name, __VA_ARGS__)
>                                      ^~~~~~~~~~~~~~~
> arch/powerpc/kernel/syscalls.c:65:1: note: in expansion of macro 'SYSCALL_DEFINE6'
>   SYSCALL_DEFINE6(mmap2, unsigned long, addr, size_t, len,
>   ^~~~~~~~~~~~~~~
> ./include/linux/syscalls.h:238:18: note: aliased declaration here
>    asmlinkage long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__)) \
>                    ^~~~~~~~
> ./include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx'
>    __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
>    ^~~~~~~~~~~~~~~~~
> ./include/linux/syscalls.h:216:36: note: in expansion of macro 'SYSCALL_DEFINEx'
>   #define SYSCALL_DEFINE6(name, ...) SYSCALL_DEFINEx(6, _##name, __VA_ARGS__)
>                                      ^~~~~~~~~~~~~~~
> arch/powerpc/kernel/syscalls.c:65:1: note: in expansion of macro 'SYSCALL_DEFINE6'
>   SYSCALL_DEFINE6(mmap2, unsigned long, addr, size_t, len,
>   ^~~~~~~~~~~~~~~
> ./include/linux/syscalls.h:233:18: error: 'sys_mmap' alias between functions of incompatible types 'long int(long unsigned int,  size_t,  long unsigned int,  long unsigned int,  long unsigned int,  off_t)' {aka 'long int(long unsigned int,  long unsigned int,  long unsigned int,  long unsigned int,  long unsigned int,  long int)'} and 'long int(long int,  long int,  long int,  long int,  long int,  long int)' [-Werror=attribute-alias]
>    asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \
>                    ^~~
> ./include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx'
>    __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
>    ^~~~~~~~~~~~~~~~~
> ./include/linux/syscalls.h:216:36: note: in expansion of macro 'SYSCALL_DEFINEx'
>   #define SYSCALL_DEFINE6(name, ...) SYSCALL_DEFINEx(6, _##name, __VA_ARGS__)
>                                      ^~~~~~~~~~~~~~~
> arch/powerpc/kernel/syscalls.c:72:1: note: in expansion of macro 'SYSCALL_DEFINE6'
>   SYSCALL_DEFINE6(mmap, unsigned long, addr, size_t, len,
>   ^~~~~~~~~~~~~~~
> ./include/linux/syscalls.h:238:18: note: aliased declaration here
>    asmlinkage long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__)) \
>                    ^~~~~~~~
> ./include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx'
>    __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
>    ^~~~~~~~~~~~~~~~~
> ./include/linux/syscalls.h:216:36: note: in expansion of macro 'SYSCALL_DEFINEx'
>   #define SYSCALL_DEFINE6(name, ...) SYSCALL_DEFINEx(6, _##name, __VA_ARGS__)
>                                      ^~~~~~~~~~~~~~~
> arch/powerpc/kernel/syscalls.c:72:1: note: in expansion of macro 'SYSCALL_DEFINE6'
>   SYSCALL_DEFINE6(mmap, unsigned long, addr, size_t, len,
>   ^~~~~~~~~~~~~~~
>    CC      arch/powerpc/kernel/signal_32.o
> In file included from arch/powerpc/kernel/signal_32.c:31:
> ./include/linux/compat.h:74:18: error: 'compat_sys_swapcontext' alias between functions of incompatible types 'long int(struct ucontext32 *, struct ucontext32 *, int)' and 'long int(long int,  long int,  long int)' [-Werror=attribute-alias]
>    asmlinkage long compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \
>                    ^~~~~~~~~~
> ./include/linux/compat.h:58:2: note: in expansion of macro 'COMPAT_SYSCALL_DEFINEx'
>    COMPAT_SYSCALL_DEFINEx(3, _##name, __VA_ARGS__)
>    ^~~~~~~~~~~~~~~~~~~~~~
> arch/powerpc/kernel/signal_32.c:1041:1: note: in expansion of macro 'COMPAT_SYSCALL_DEFINE3'
>   COMPAT_SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
>   ^~~~~~~~~~~~~~~~~~~~~~
> ./include/linux/compat.h:79:18: note: aliased declaration here
>    asmlinkage long __se_compat_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__)) \
>                    ^~~~~~~~~~~~~~~
> ./include/linux/compat.h:58:2: note: in expansion of macro 'COMPAT_SYSCALL_DEFINEx'
>    COMPAT_SYSCALL_DEFINEx(3, _##name, __VA_ARGS__)
>    ^~~~~~~~~~~~~~~~~~~~~~
> arch/powerpc/kernel/signal_32.c:1041:1: note: in expansion of macro 'COMPAT_SYSCALL_DEFINE3'
>   COMPAT_SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
>   ^~~~~~~~~~~~~~~~~~~~~~
>    CC      arch/powerpc/kernel/signal_64.o
> In file included from arch/powerpc/kernel/signal_64.c:27:
> ./include/linux/syscalls.h:233:18: error: 'sys_swapcontext' alias between functions of incompatible types 'long int(struct ucontext *, struct ucontext *, long int)' and 'long int(long int,  long int,  long int)' [-Werror=attribute-alias]
>    asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \
>                    ^~~
> ./include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx'
>    __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
>    ^~~~~~~~~~~~~~~~~
> ./include/linux/syscalls.h:213:36: note: in expansion of macro 'SYSCALL_DEFINEx'
>   #define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, __VA_ARGS__)
>                                      ^~~~~~~~~~~~~~~
> arch/powerpc/kernel/signal_64.c:628:1: note: in expansion of macro 'SYSCALL_DEFINE3'
>   SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
>   ^~~~~~~~~~~~~~~
> ./include/linux/syscalls.h:238:18: note: aliased declaration here
>    asmlinkage long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__)) \
>                    ^~~~~~~~
> ./include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx'
>    __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
>    ^~~~~~~~~~~~~~~~~
> ./include/linux/syscalls.h:213:36: note: in expansion of macro 'SYSCALL_DEFINEx'
>   #define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, __VA_ARGS__)
>                                      ^~~~~~~~~~~~~~~
> arch/powerpc/kernel/signal_64.c:628:1: note: in expansion of macro 'SYSCALL_DEFINE3'
>   SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
>   ^~~~~~~~~~~~~~~
>    CC      arch/powerpc/kernel/rtas.o
> In file included from arch/powerpc/kernel/rtas.c:29:
> ./include/linux/syscalls.h:233:18: error: 'sys_rtas' alias between functions of incompatible types 'long int(struct rtas_args *)' and 'long int(long int)' [-Werror=attribute-alias]
>    asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \
>                    ^~~
> ./include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx'
>    __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
>    ^~~~~~~~~~~~~~~~~
> ./include/linux/syscalls.h:211:36: note: in expansion of macro 'SYSCALL_DEFINEx'
>   #define SYSCALL_DEFINE1(name, ...) SYSCALL_DEFINEx(1, _##name, __VA_ARGS__)
>                                      ^~~~~~~~~~~~~~~
> arch/powerpc/kernel/rtas.c:1054:1: note: in expansion of macro 'SYSCALL_DEFINE1'
>   SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs)
>   ^~~~~~~~~~~~~~~
> ./include/linux/syscalls.h:238:18: note: aliased declaration here
>    asmlinkage long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__)) \
>                    ^~~~~~~~
> ./include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx'
>    __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
>    ^~~~~~~~~~~~~~~~~
> ./include/linux/syscalls.h:211:36: note: in expansion of macro 'SYSCALL_DEFINEx'
>   #define SYSCALL_DEFINE1(name, ...) SYSCALL_DEFINEx(1, _##name, __VA_ARGS__)
>                                      ^~~~~~~~~~~~~~~
> arch/powerpc/kernel/rtas.c:1054:1: note: in expansion of macro 'SYSCALL_DEFINE1'
>   SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs)
>   ^~~~~~~~~~~~~~~
>    CC      arch/powerpc/kernel/pci_64.o
> In file included from arch/powerpc/kernel/pci_64.c:23:
> ./include/linux/syscalls.h:233:18: error: 'sys_pciconfig_iobase' alias between functions of incompatible types 'long int(long int,  long unsigned int,  long unsigned int)' and 'long int(long int,  long int,  long int)' [-Werror=attribute-alias]
>    asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \
>                    ^~~
> ./include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx'
>    __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
>    ^~~~~~~~~~~~~~~~~
> ./include/linux/syscalls.h:213:36: note: in expansion of macro 'SYSCALL_DEFINEx'
>   #define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, __VA_ARGS__)
>                                      ^~~~~~~~~~~~~~~
> arch/powerpc/kernel/pci_64.c:206:1: note: in expansion of macro 'SYSCALL_DEFINE3'
>   SYSCALL_DEFINE3(pciconfig_iobase, long, which, unsigned long, in_bus,
>   ^~~~~~~~~~~~~~~
> ./include/linux/syscalls.h:238:18: note: aliased declaration here
>    asmlinkage long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__)) \
>                    ^~~~~~~~
> ./include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx'
>    __SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
>    ^~~~~~~~~~~~~~~~~
> ./include/linux/syscalls.h:213:36: note: in expansion of macro 'SYSCALL_DEFINEx'
>   #define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, __VA_ARGS__)
>                                      ^~~~~~~~~~~~~~~
> arch/powerpc/kernel/pci_64.c:206:1: note: in expansion of macro 'SYSCALL_DEFINE3'
>   SYSCALL_DEFINE3(pciconfig_iobase, long, which, unsigned long, in_bus,
>   ^~~~~~~~~~~~~~~
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
>   arch/powerpc/kernel/pci_64.c    | 3 +++
>   arch/powerpc/kernel/rtas.c      | 3 +++
>   arch/powerpc/kernel/signal_32.c | 6 ++++++
>   arch/powerpc/kernel/signal_64.c | 3 +++
>   arch/powerpc/kernel/syscalls.c  | 3 +++
>   arch/powerpc/mm/subpage-prot.c  | 3 +++
>   6 files changed, 21 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
> index dff28f903512..a8bf98c1cf0c 100644
> --- a/arch/powerpc/kernel/pci_64.c
> +++ b/arch/powerpc/kernel/pci_64.c
> @@ -203,6 +203,8 @@ void pcibios_setup_phb_io_space(struct pci_controller *hose)
>   #define IOBASE_ISA_IO		3
>   #define IOBASE_ISA_MEM		4
>   
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wattribute-alias"
>   SYSCALL_DEFINE3(pciconfig_iobase, long, which, unsigned long, in_bus,
>   			  unsigned long, in_devfn)
>   {
> @@ -256,6 +258,7 @@ SYSCALL_DEFINE3(pciconfig_iobase, long, which, unsigned long, in_bus,
>   
>   	return -EOPNOTSUPP;
>   }
> +#pragma GCC diagnostic pop
>   
>   #ifdef CONFIG_NUMA
>   int pcibus_to_node(struct pci_bus *bus)
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 8afd146bc9c7..c1713cef1642 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -1051,6 +1051,8 @@ struct pseries_errorlog *get_pseries_errorlog(struct rtas_error_log *log,
>   }
>   
>   /* We assume to be passed big endian arguments */
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wattribute-alias"
>   SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs)
>   {
>   	struct rtas_args args;
> @@ -1137,6 +1139,7 @@ SYSCALL_DEFINE1(rtas, struct rtas_args __user *, uargs)
>   
>   	return 0;
>   }
> +#pragma GCC diagnostic pop
>   
>   /*
>    * Call early during boot, before mem init, to retrieve the RTAS
> diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
> index 9cf8a03d3bc7..db1042f37b94 100644
> --- a/arch/powerpc/kernel/signal_32.c
> +++ b/arch/powerpc/kernel/signal_32.c
> @@ -1037,6 +1037,8 @@ static int do_setcontext_tm(struct ucontext __user *ucp,
>   }
>   #endif
>   
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wattribute-alias"
>   #ifdef CONFIG_PPC64
>   COMPAT_SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
>   		       struct ucontext __user *, new_ctx, int, ctx_size)
> @@ -1132,6 +1134,7 @@ SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
>   	set_thread_flag(TIF_RESTOREALL);
>   	return 0;
>   }
> +#pragma GCC diagnostic pop
>   
>   #ifdef CONFIG_PPC64
>   COMPAT_SYSCALL_DEFINE0(rt_sigreturn)
> @@ -1228,6 +1231,8 @@ SYSCALL_DEFINE0(rt_sigreturn)
>   	return 0;
>   }
>   
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wattribute-alias"
>   #ifdef CONFIG_PPC32
>   SYSCALL_DEFINE3(debug_setcontext, struct ucontext __user *, ctx,
>   			 int, ndbg, struct sig_dbg_op __user *, dbg)
> @@ -1333,6 +1338,7 @@ SYSCALL_DEFINE3(debug_setcontext, struct ucontext __user *, ctx,
>   	return 0;
>   }
>   #endif
> +#pragma GCC diagnostic pop
>   
>   /*
>    * OK, we're invoking a handler
> diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
> index 83d51bf586c7..d2e8da8cf27f 100644
> --- a/arch/powerpc/kernel/signal_64.c
> +++ b/arch/powerpc/kernel/signal_64.c
> @@ -625,6 +625,8 @@ static long setup_trampoline(unsigned int syscall, unsigned int __user *tramp)
>   /*
>    * Handle {get,set,swap}_context operations
>    */
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wattribute-alias"
>   SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
>   		struct ucontext __user *, new_ctx, long, ctx_size)
>   {
> @@ -690,6 +692,7 @@ SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx,
>   	set_thread_flag(TIF_RESTOREALL);
>   	return 0;
>   }
> +#pragma GCC diagnostic pop
>   
>   
>   /*
> diff --git a/arch/powerpc/kernel/syscalls.c b/arch/powerpc/kernel/syscalls.c
> index 466216506eb2..d08119590185 100644
> --- a/arch/powerpc/kernel/syscalls.c
> +++ b/arch/powerpc/kernel/syscalls.c
> @@ -62,6 +62,8 @@ static inline long do_mmap2(unsigned long addr, size_t len,
>   	return ret;
>   }
>   
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wattribute-alias"
>   SYSCALL_DEFINE6(mmap2, unsigned long, addr, size_t, len,
>   		unsigned long, prot, unsigned long, flags,
>   		unsigned long, fd, unsigned long, pgoff)
> @@ -75,6 +77,7 @@ SYSCALL_DEFINE6(mmap, unsigned long, addr, size_t, len,
>   {
>   	return do_mmap2(addr, len, prot, flags, fd, offset, PAGE_SHIFT);
>   }
> +#pragma GCC diagnostic pop
>   
>   #ifdef CONFIG_PPC32
>   /*
> diff --git a/arch/powerpc/mm/subpage-prot.c b/arch/powerpc/mm/subpage-prot.c
> index 9d16ee251fc0..edf09fd4ba80 100644
> --- a/arch/powerpc/mm/subpage-prot.c
> +++ b/arch/powerpc/mm/subpage-prot.c
> @@ -186,6 +186,8 @@ static void subpage_mark_vma_nohuge(struct mm_struct *mm, unsigned long addr,
>    * in a 2-bit field won't allow writes to a page that is otherwise
>    * write-protected.
>    */
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wattribute-alias"
>   SYSCALL_DEFINE3(subpage_prot, unsigned long, addr,
>   		unsigned long, len, u32 __user *, map)
>   {
> @@ -269,3 +271,4 @@ SYSCALL_DEFINE3(subpage_prot, unsigned long, addr,
>   	up_write(&mm->mmap_sem);
>   	return err;
>   }
> +#pragma GCC diagnostic pop
> 

^ permalink raw reply

* Re: [PATCH v2] powerpc/64: Fix build failure with GCC 8.1
From: Geert Uytterhoeven @ 2018-05-29  7:47 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Linux Kernel Mailing List, linuxppc-dev, Geoff Levand
In-Reply-To: <f5f2de3e1a39613f7303bfdc0d2f2210d4c91910.1527573345.git.christophe.leroy@c-s.fr>

Hi Christophe,

CC Geoff

On Tue, May 29, 2018 at 8:03 AM, Christophe Leroy
<christophe.leroy@c-s.fr> wrote:
>   CC      arch/powerpc/kernel/nvram_64.o
> arch/powerpc/kernel/nvram_64.c: In function 'nvram_create_partition':
> arch/powerpc/kernel/nvram_64.c:1042:2: error: 'strncpy' specified bound 12 equals destination size [-Werror=stringop-truncation]
>   strncpy(new_part->header.name, name, 12);
>   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>   CC      arch/powerpc/kernel/trace/ftrace.o
> In function 'make_field',
>     inlined from 'ps3_repository_read_boot_dat_address' at arch/powerpc/platforms/ps3/repository.c:900:9:
> arch/powerpc/platforms/ps3/repository.c:106:2: error: 'strncpy' output truncated before terminating nul copying 8 bytes from a string of the same length [-Werror=stringop-truncation]
>   strncpy((char *)&n, text, 8);
>   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

Thanks for your patch!

> --- a/arch/powerpc/kernel/nvram_64.c
> +++ b/arch/powerpc/kernel/nvram_64.c
> @@ -1039,7 +1039,7 @@ loff_t __init nvram_create_partition(const char *name, int sig,
>         new_part->index = free_part->index;
>         new_part->header.signature = sig;
>         new_part->header.length = size;
> -       strncpy(new_part->header.name, name, 12);
> +       memcpy(new_part->header.name, name, strnlen(name, sizeof(new_part->header.name)));

The comment for nvram_header.lgnth says:

        /* Terminating null required only for names < 12 chars. */

This will not terminate the string with a zero (the struct is
allocated with kmalloc).
So the original code is correct, the new one isn't.

>         new_part->header.checksum = nvram_checksum(&new_part->header);
>
>         rc = nvram_write_header(new_part);
> diff --git a/arch/powerpc/platforms/ps3/repository.c b/arch/powerpc/platforms/ps3/repository.c
> index 50dbaf24b1ee..e49c887787c4 100644
> --- a/arch/powerpc/platforms/ps3/repository.c
> +++ b/arch/powerpc/platforms/ps3/repository.c
> @@ -101,9 +101,9 @@ static u64 make_first_field(const char *text, u64 index)
>
>  static u64 make_field(const char *text, u64 index)
>  {
> -       u64 n;
> +       u64 n = 0;
>
> -       strncpy((char *)&n, text, 8);
> +       memcpy((char *)&n, text, strnlen(text, sizeof(n)));

This changes behavior: strncpy() fills the remainder of the buffer with
zeroes.  I don't remember the details of the PS3 repository structure,
but given this writes to a fixed size u64 buffer, I'd expect the PS3
hypervisor code to (1) rely on the zero padding, and (2) not need a zero
terminator if there are 8 characters in the buffer, so probably the
original code is correct, and the "fixed" code isn't.

Has this been tested on a PS3?

>         return n + index;
>  }

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox