ams: Lots of kworker activity after wake from hibernate on Powerbook

2016-04-13 Thread Mathieu Malaterre
Dear all,

I am hoping someone could help me diagnose the following bug report:

https://bugs.debian.org/798102

[...]
After waking from hibernate, I get lots of kworker activity that makes my
fans kick in.  Top shows a process called kworker/0:3 always active using
around 3% cpu, and Powertop shows a process using 320 ms/s with the Category
Kwork and the Description ams_worker (Powertop also shows about 33% cpu usage).
I assume ams stands for Apple Motion Sensor, and when I unload the module
"ams" everything returns to normal and my fans quiet down.

This is on an aluminum Powerbook (PowerBook5,6).
[...]


Is there any tool to use to check the behavior of the ams kernel module ?

Thanks much
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: PROBLEM: monotonic clock going backwards on ppc64

2019-02-27 Thread Mathieu Malaterre
On Tue, Feb 26, 2019 at 9:36 PM Jakub Drnec  wrote:
>
> Hi all,
>
> I think I observed a potential problem, is this the correct place to report 
> it? (CC me, not on list)
>
> [1.] One line summary: monotonic clock can be made to decrease on ppc64
> [2.] Full description:
> Setting the realtime clock can sometimes make the monotonic clock go back by 
> over a hundred years.
> Decreasing the realtime clock across the y2k38 threshold is one reliable way 
> to reproduce.
> Allegedly this can also happen just by running ntpd, I have not managed to 
> reproduce that other
> than booting with rtc at >2038 and then running ntp.

Isn't it the expected behavior. Here is what I see for powermac:

$ git show 22db552b50fa
...
This changes the logic to cast to an unsigned 32-bit number first for
the Macintosh time and then convert that to the Unix time, which then
gives us a time in the documented 1904..2040 year range. I decided not
to use the longer 1970..2106 range that other drivers use, for
consistency with the literal interpretation of the register, but that
could be easily changed if we decide we want to support any Mac after
2040.
...

> When this happens, anything with timers (e.g. openjdk) breaks rather badly.
>
> [3.] Keywords: gettimeofday, ppc64, vdso
> [4.] Kernel information
> [4.1.] Kernel version: any (tested on 4.19)
> [4.2.] Kernel .config file: any
> [5.] Most recent kernel version which did not have the bug: not a regression
> [6.] Output of Oops..: not applicable
> [7.] Example program which triggers the problem
> --- testcase.c
> #include 
> #include 
> #include 
> #include 
>
> long get_time() {
>   struct timespec tp;
>   if (clock_gettime(CLOCK_MONOTONIC, &tp) != 0) {
> perror("clock_gettime failed");
> exit(1);
>   }
>   long result = tp.tv_sec + tp.tv_nsec / 10;
>   return result;
> }
>
> int main() {
>   printf("monitoring monotonic clock...\n");
>   long last = get_time();
>   while(1) {
> long now = get_time();
> if (now < last) {
>   printf("clock went backwards by %ld seconds!\n",
> last - now);
> }
> last = now;
> sleep(1);
>   }
>   return 0;
> }
> ---
> when running
> # date -s 2040-1-1
> # date -s 2037-1-1
> program outputs: clock went backwards by 4294967295 seconds!
>
> [8.] Environment: any ppc64, currently reproducing on qemu-system-ppc64le 
> running debian unstable
> [X.] Other notes, patches, fixes, workarounds:
> The problem seems to be in vDSO code in 
> arch/powerpc/kernel/vdso64/gettimeofday.S.
> (possibly because some values used in the calculation are only 32 bit?)
> Slightly silly workaround:
> nuke the "cmpwi cr1,r3,CLOCK_MONOTONIC" in __kernel_clock_gettime
> Now it always goes through the syscall fallback which does not have the same 
> problem.
>
> Regards,
> Jakub Drnec


[PATCH] powerpc/64s: Include header file to fix a warning

2019-03-12 Thread Mathieu Malaterre
Make sure to include  to provide the following prototype:
hv_nmi_check_nonrecoverable.

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

  arch/powerpc/kernel/traps.c:393:6: error: no previous prototype for 
'hv_nmi_check_nonrecoverable' [-Werror=missing-prototypes]

Signed-off-by: Mathieu Malaterre 
---
 arch/powerpc/kernel/traps.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index a21200c6aaea..1fd45a8650e1 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -71,6 +71,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #if defined(CONFIG_DEBUGGER) || defined(CONFIG_KEXEC_CORE)
 int (*__debugger)(struct pt_regs *regs) __read_mostly;
-- 
2.20.1



[PATCH] powerpc: sstep: Mark variable `rc` as unused in function 'analyse_instr'

2019-03-12 Thread Mathieu Malaterre
Add gcc attribute unused for `rc` variable.

Fix warnings treated as errors with W=1:

  arch/powerpc/lib/sstep.c:1172:31: error: variable 'rc' set but not used 
[-Werror=unused-but-set-variable]

Signed-off-by: Mathieu Malaterre 
---
 arch/powerpc/lib/sstep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 3d33fb509ef4..32d092f62ae0 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -1169,7 +1169,7 @@ static nokprobe_inline int trap_compare(long v1, long v2)
 int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
  unsigned int instr)
 {
-   unsigned int opcode, ra, rb, rc, rd, spr, u;
+   unsigned int opcode, ra, rb, rc __maybe_unused, rd, spr, u;
unsigned long int imm;
unsigned long int val, val2;
unsigned int mb, me, sh;
-- 
2.20.1



[PATCH] powerpc/64s: Mark 'dummy_copy_buffer' as used

2019-03-12 Thread Mathieu Malaterre
In commit 07d2a628bc00 ("powerpc/64s: Avoid cpabort in context switch
when possible") a buffer 'dummy_copy_buffer' was introduced. gcc does
not see this buffer being used in the inline assembly within function
'__switch_to', explicitly marked this variable as being used.

Prefer using '__aligned' to get passed line over 80 characters warning
in checkpatch.

This remove the following warning:

  arch/powerpc/kernel/process.c:1156:17: error: 'dummy_copy_buffer' defined but 
not used [-Werror=unused-const-variable=]

Cc: Nicholas Piggin 
Signed-off-by: Mathieu Malaterre 
---
 arch/powerpc/kernel/process.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 77e44275d025..5acf63d45802 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1153,7 +1153,7 @@ static inline void restore_sprs(struct thread_struct 
*old_thread,
 
 #ifdef CONFIG_PPC_BOOK3S_64
 #define CP_SIZE 128
-static const u8 dummy_copy_buffer[CP_SIZE] __attribute__((aligned(CP_SIZE)));
+static const u8 dummy_copy_buffer[CP_SIZE] __aligned(CP_SIZE) __used;
 #endif
 
 struct task_struct *__switch_to(struct task_struct *prev,
-- 
2.20.1



[PATCH] powerpc: Make some functions static

2019-03-12 Thread Mathieu Malaterre
In commit cb9e4d10c448 ("[POWERPC] Add support for 750CL Holly board")
new functions were added. Since these functions can be made static,
make it so. While doing so, it turns out that holly_power_off and
holly_halt are unused, so remove them.

Silence the following warnings triggered using W=1:

  arch/powerpc/platforms/embedded6xx/holly.c:47:5: error: no previous prototype 
for 'holly_exclude_device' [-Werror=missing-prototypes]
  arch/powerpc/platforms/embedded6xx/holly.c:190:6: error: no previous 
prototype for 'holly_show_cpuinfo' [-Werror=missing-prototypes]
  arch/powerpc/platforms/embedded6xx/holly.c:196:17: error: no previous 
prototype for 'holly_restart' [-Werror=missing-prototypes]
  arch/powerpc/platforms/embedded6xx/holly.c:236:6: error: no previous 
prototype for 'holly_power_off' [-Werror=missing-prototypes]
  arch/powerpc/platforms/embedded6xx/holly.c:243:6: error: no previous 
prototype for 'holly_halt' [-Werror=missing-prototypes]

Signed-off-by: Mathieu Malaterre 
---
 arch/powerpc/platforms/embedded6xx/holly.c | 19 ---
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/platforms/embedded6xx/holly.c 
b/arch/powerpc/platforms/embedded6xx/holly.c
index 0409714e8070..829bf3697dc9 100644
--- a/arch/powerpc/platforms/embedded6xx/holly.c
+++ b/arch/powerpc/platforms/embedded6xx/holly.c
@@ -44,7 +44,8 @@
 
 #define HOLLY_PCI_CFG_PHYS 0x7c00
 
-int holly_exclude_device(struct pci_controller *hose, u_char bus, u_char devfn)
+static int holly_exclude_device(struct pci_controller *hose, u_char bus,
+   u_char devfn)
 {
if (bus == 0 && PCI_SLOT(devfn) == 0)
return PCIBIOS_DEVICE_NOT_FOUND;
@@ -187,13 +188,13 @@ static void __init holly_init_IRQ(void)
tsi108_write_reg(TSI108_MPIC_OFFSET + 0x30c, 0);
 }
 
-void holly_show_cpuinfo(struct seq_file *m)
+static void holly_show_cpuinfo(struct seq_file *m)
 {
seq_printf(m, "vendor\t\t: IBM\n");
seq_printf(m, "machine\t\t: PPC750 GX/CL\n");
 }
 
-void __noreturn holly_restart(char *cmd)
+static void __noreturn holly_restart(char *cmd)
 {
__be32 __iomem *ocn_bar1 = NULL;
unsigned long bar;
@@ -233,18 +234,6 @@ void __noreturn holly_restart(char *cmd)
for (;;) ;
 }
 
-void holly_power_off(void)
-{
-   local_irq_disable();
-   /* No way to shut power off with software */
-   for (;;) ;
-}
-
-void holly_halt(void)
-{
-   holly_power_off();
-}
-
 /*
  * Called very early, device-tree isn't unflattened
  */
-- 
2.20.1



Re: [PATCH] powerpc: sstep: Mark variable `rc` as unused in function 'analyse_instr'

2019-03-12 Thread Mathieu Malaterre
On Tue, Mar 12, 2019 at 9:56 PM Christophe Leroy
 wrote:
>
>
>
> Le 12/03/2019 à 21:20, Mathieu Malaterre a écrit :
> > Add gcc attribute unused for `rc` variable.
> >
> > Fix warnings treated as errors with W=1:
> >
> >arch/powerpc/lib/sstep.c:1172:31: error: variable 'rc' set but not used 
> > [-Werror=unused-but-set-variable]
> >
> > Signed-off-by: Mathieu Malaterre 
> > ---
> >   arch/powerpc/lib/sstep.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> > index 3d33fb509ef4..32d092f62ae0 100644
> > --- a/arch/powerpc/lib/sstep.c
> > +++ b/arch/powerpc/lib/sstep.c
> > @@ -1169,7 +1169,7 @@ static nokprobe_inline int trap_compare(long v1, long 
> > v2)
> >   int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
> > unsigned int instr)
> >   {
> > - unsigned int opcode, ra, rb, rc, rd, spr, u;
> > + unsigned int opcode, ra, rb, rc __maybe_unused, rd, spr, u;
>
> I think it would be better to enclose 'rc' inside a #ifdef CONFIG_PPC64

Hum odd, I would have bet you would have suggested me to use
IS_ENABLED with some crazy scheme (I was not able to mix it with the
switch case nicely).

Anyway I'll try your suggestion and post a v2.

> Christophe
>
> >   unsigned long int imm;
> >   unsigned long int val, val2;
> >   unsigned int mb, me, sh;
> >


[PATCH v2] powerpc/32: sstep: Move variable `rc` within CONFIG_PPC64 sentinels

2019-03-12 Thread Mathieu Malaterre
Fix warnings treated as errors with W=1:

  arch/powerpc/lib/sstep.c:1172:31: error: variable 'rc' set but not used 
[-Werror=unused-but-set-variable]

Suggested-by: Christophe Leroy 
Signed-off-by: Mathieu Malaterre 
---
v2: as suggested prefer CONFIG_PPC64 sentinel instead of unused keyword

 arch/powerpc/lib/sstep.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 3d33fb509ef4..9996dc7a0b46 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -1169,7 +1169,10 @@ static nokprobe_inline int trap_compare(long v1, long v2)
 int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
  unsigned int instr)
 {
-   unsigned int opcode, ra, rb, rc, rd, spr, u;
+   unsigned int opcode, ra, rb, rd, spr, u;
+#ifdef CONFIG_PPC64
+   unsigned int rc;
+#endif
unsigned long int imm;
unsigned long int val, val2;
unsigned int mb, me, sh;
@@ -1292,7 +1295,9 @@ int analyse_instr(struct instruction_op *op, const struct 
pt_regs *regs,
rd = (instr >> 21) & 0x1f;
ra = (instr >> 16) & 0x1f;
rb = (instr >> 11) & 0x1f;
+#ifdef CONFIG_PPC64
rc = (instr >> 6) & 0x1f;
+#endif
 
switch (opcode) {
 #ifdef __powerpc64__
-- 
2.20.1



[PATCH v2] powerpc/64s: Remove 'dummy_copy_buffer'

2019-03-13 Thread Mathieu Malaterre
In commit 2bf1071a8d50 ("powerpc/64s: Remove POWER9 DD1 support") the
function __switch_to remove usage for 'dummy_copy_buffer'. Since it is
not used anywhere else, remove it completely.

This remove the following warning:

  arch/powerpc/kernel/process.c:1156:17: error: 'dummy_copy_buffer' defined but 
not used [-Werror=unused-const-variable=]

Cc: Nicholas Piggin 
Suggested-by: Christophe Leroy 
Signed-off-by: Mathieu Malaterre 
---
v2: complely remove dummy_copy_buffer instead of marking it as used (since not 
anymore)
 arch/powerpc/kernel/process.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 77e44275d025..b2f0daa2fcf1 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1151,11 +1151,6 @@ static inline void restore_sprs(struct thread_struct 
*old_thread,
thread_pkey_regs_restore(new_thread, old_thread);
 }
 
-#ifdef CONFIG_PPC_BOOK3S_64
-#define CP_SIZE 128
-static const u8 dummy_copy_buffer[CP_SIZE] __attribute__((aligned(CP_SIZE)));
-#endif
-
 struct task_struct *__switch_to(struct task_struct *prev,
struct task_struct *new)
 {
-- 
2.20.1



Re: [PATCH] powerpc: sstep: Mark variable `rc` as unused in function 'analyse_instr'

2019-03-13 Thread Mathieu Malaterre
On Tue, Mar 12, 2019 at 10:26 PM Christophe Leroy
 wrote:
>
>
>
> Le 12/03/2019 à 22:12, Mathieu Malaterre a écrit :
> > On Tue, Mar 12, 2019 at 9:56 PM Christophe Leroy
> >  wrote:
> >>
> >>
> >>
> >> Le 12/03/2019 à 21:20, Mathieu Malaterre a écrit :
> >>> Add gcc attribute unused for `rc` variable.
> >>>
> >>> Fix warnings treated as errors with W=1:
> >>>
> >>> arch/powerpc/lib/sstep.c:1172:31: error: variable 'rc' set but not 
> >>> used [-Werror=unused-but-set-variable]
> >>>
> >>> Signed-off-by: Mathieu Malaterre 
> >>> ---
> >>>arch/powerpc/lib/sstep.c | 2 +-
> >>>1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> >>> index 3d33fb509ef4..32d092f62ae0 100644
> >>> --- a/arch/powerpc/lib/sstep.c
> >>> +++ b/arch/powerpc/lib/sstep.c
> >>> @@ -1169,7 +1169,7 @@ static nokprobe_inline int trap_compare(long v1, 
> >>> long v2)
> >>>int analyse_instr(struct instruction_op *op, const struct pt_regs 
> >>> *regs,
> >>>  unsigned int instr)
> >>>{
> >>> - unsigned int opcode, ra, rb, rc, rd, spr, u;
> >>> + unsigned int opcode, ra, rb, rc __maybe_unused, rd, spr, u;
> >>
> >> I think it would be better to enclose 'rc' inside a #ifdef CONFIG_PPC64
> >
> > Hum odd, I would have bet you would have suggested me to use
> > IS_ENABLED with some crazy scheme (I was not able to mix it with the
> > switch case nicely).
>
> Well I guess yes, you could also get rid of the #ifdef __powerpc64__ and
> instead add the following just after the 'case 4:'
>
> if (!IS_ENABLED(CONFIG_64))
> break;
>
> That's less uggly than adding two #ifdef/#endif

So you mean changing:

#ifdef __powerpc64__
  case 4:
if (!cpu_has_feature(CPU_FTR_ARCH_300))
  return -1;

into:

  case 4:
if (!IS_ENABLED(CONFIG_PPC64))
  break;
if (!cpu_has_feature(CPU_FTR_ARCH_300))
  return -1;

So suddenly case label '4' becomes visible for ppc32, is that really
what you wanted ?

> Christophe
>
> >
> > Anyway I'll try your suggestion and post a v2.
> >
> >> Christophe
> >>
> >>>unsigned long int imm;
> >>>unsigned long int val, val2;
> >>>unsigned int mb, me, sh;
> >>>


Re: [PATCH] powerpc: Make some functions static

2019-03-13 Thread Mathieu Malaterre
On Tue, Mar 12, 2019 at 10:14 PM Christophe Leroy
 wrote:
>
>
>
> Le 12/03/2019 à 21:31, Mathieu Malaterre a écrit :
> > In commit cb9e4d10c448 ("[POWERPC] Add support for 750CL Holly board")
> > new functions were added. Since these functions can be made static,
> > make it so. While doing so, it turns out that holly_power_off and
> > holly_halt are unused, so remove them.
>
> I would have said 'since these functions are only used in this C file,
> make them static'.
>
> I think this could be split in two patches:
> 1/ Remove unused functions, ie holly_halt() and holly_power_off().
> 2/ Make the other ones static.

Michael do you want two patches ?

> Christophe
>
> >
> > Silence the following warnings triggered using W=1:
> >
> >arch/powerpc/platforms/embedded6xx/holly.c:47:5: error: no previous 
> > prototype for 'holly_exclude_device' [-Werror=missing-prototypes]
> >arch/powerpc/platforms/embedded6xx/holly.c:190:6: error: no previous 
> > prototype for 'holly_show_cpuinfo' [-Werror=missing-prototypes]
> >arch/powerpc/platforms/embedded6xx/holly.c:196:17: error: no previous 
> > prototype for 'holly_restart' [-Werror=missing-prototypes]
> >arch/powerpc/platforms/embedded6xx/holly.c:236:6: error: no previous 
> > prototype for 'holly_power_off' [-Werror=missing-prototypes]
> >arch/powerpc/platforms/embedded6xx/holly.c:243:6: error: no previous 
> > prototype for 'holly_halt' [-Werror=missing-prototypes]
> >
> > Signed-off-by: Mathieu Malaterre 
> > ---
> >   arch/powerpc/platforms/embedded6xx/holly.c | 19 ---
> >   1 file changed, 4 insertions(+), 15 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/embedded6xx/holly.c 
> > b/arch/powerpc/platforms/embedded6xx/holly.c
> > index 0409714e8070..829bf3697dc9 100644
> > --- a/arch/powerpc/platforms/embedded6xx/holly.c
> > +++ b/arch/powerpc/platforms/embedded6xx/holly.c
> > @@ -44,7 +44,8 @@
> >
> >   #define HOLLY_PCI_CFG_PHYS 0x7c00
> >
> > -int holly_exclude_device(struct pci_controller *hose, u_char bus, u_char 
> > devfn)
> > +static int holly_exclude_device(struct pci_controller *hose, u_char bus,
> > + u_char devfn)
> >   {
> >   if (bus == 0 && PCI_SLOT(devfn) == 0)
> >   return PCIBIOS_DEVICE_NOT_FOUND;
> > @@ -187,13 +188,13 @@ static void __init holly_init_IRQ(void)
> >   tsi108_write_reg(TSI108_MPIC_OFFSET + 0x30c, 0);
> >   }
> >
> > -void holly_show_cpuinfo(struct seq_file *m)
> > +static void holly_show_cpuinfo(struct seq_file *m)
> >   {
> >   seq_printf(m, "vendor\t\t: IBM\n");
> >   seq_printf(m, "machine\t\t: PPC750 GX/CL\n");
> >   }
> >
> > -void __noreturn holly_restart(char *cmd)
> > +static void __noreturn holly_restart(char *cmd)
> >   {
> >   __be32 __iomem *ocn_bar1 = NULL;
> >   unsigned long bar;
> > @@ -233,18 +234,6 @@ void __noreturn holly_restart(char *cmd)
> >   for (;;) ;
> >   }
> >
> > -void holly_power_off(void)
> > -{
> > - local_irq_disable();
> > - /* No way to shut power off with software */
> > - for (;;) ;
> > -}
> > -
> > -void holly_halt(void)
> > -{
> > - holly_power_off();
> > -}
> > -
> >   /*
> >* Called very early, device-tree isn't unflattened
> >*/
> >


Re: [PATCH] powerpc: use $(origin ARCH) to select KBUILD_DEFCONFIG

2019-03-13 Thread Mathieu Malaterre
On Sat, Feb 16, 2019 at 3:26 AM Masahiro Yamada
 wrote:
>
> On Sat, Feb 16, 2019 at 1:11 AM Mathieu Malaterre  wrote:
> >
> > On Fri, Feb 15, 2019 at 10:41 AM Masahiro Yamada
> >  wrote:
> > >
> > > I often test all Kconfig commands for all architectures. To ease my
> > > workflow, I want 'make defconfig' at least working without any cross
> > > compiler.
> > >
> > > Currently, arch/powerpc/Makefile checks CROSS_COMPILE to decide the
> > > default defconfig source.
> > >
> > > If CROSS_COMPILE is unset, it is likely to be the native build, so
> > > 'uname -m' is useful to choose the defconfig. If CROSS_COMPILE is set,
> > > the user is cross-building (i.e. 'uname -m' is probably x86_64), so
> > > it falls back to ppc64_defconfig. Yup, make sense.
> > >
> > > However, I want to run 'make ARCH=* defconfig' without setting
> > > CROSS_COMPILE for each architecture.
> > >
> > > My suggestion is to check $(origin ARCH).
> > >
> > > When you cross-compile the kernel, you need to set ARCH from your
> > > environment or from the command line.
> > >
> > > For the native build, you do not need to set ARCH. The default in
> > > the top Makefile is used:
> > >
> > >   ARCH?= $(SUBARCH)
> > >
> > > Hence, $(origin ARCH) returns 'file'.
> > >
> > > Before this commit, 'make ARCH=powerpc defconfig' failed:
> >
> > In case you have not seen it, please check:
> >
> > http://patchwork.ozlabs.org/patch/1037835/
>
>
> I did not know that because I do not subscribe to ppc ML.
>
>
> Michael's patch looks good to me.

OK

>
> If you mimic x86, the following will work:
>

Nice! Michael do you have a preference ?

>
>
> diff --git a/Makefile b/Makefile
> index 86cf35d..eb9552d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -356,6 +356,11 @@ ifeq ($(ARCH),sh64)
> SRCARCH := sh
>  endif
>
> +# Additional ARCH settings for powerpc
> +ifneq ($(filter ppc%,$(ARCH)),)
> +   SRCARCH := powerpc
> +endif
> +
>  KCONFIG_CONFIG ?= .config
>  export KCONFIG_CONFIG
>
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index 488c9ed..ff01fef 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -34,10 +34,10 @@ ifdef CONFIG_PPC_BOOK3S_32
>  KBUILD_CFLAGS  += -mcpu=powerpc
>  endif
>
> -ifeq ($(CROSS_COMPILE),)
> -KBUILD_DEFCONFIG := $(shell uname -m)_defconfig
> -else
> +ifeq ($(ARCH),powerpc)
>  KBUILD_DEFCONFIG := ppc64_defconfig
> +else
> +KBUILD_DEFCONFIG := $(ARCH)_defconfig
>  endif
>
>  ifdef CONFIG_PPC64
> diff --git a/scripts/subarch.include b/scripts/subarch.include
> index 6506828..c98323f 100644
> --- a/scripts/subarch.include
> +++ b/scripts/subarch.include
> @@ -8,6 +8,6 @@ SUBARCH := $(shell uname -m | sed -e s/i.86/x86/ -e
> s/x86_64/x86/ \
>   -e s/sun4u/sparc64/ \
>   -e s/arm.*/arm/ -e s/sa110/arm/ \
>   -e s/s390x/s390/ -e s/parisc64/parisc/ \
> - -e s/ppc.*/powerpc/ -e s/mips.*/mips/ \
> + -e s/mips.*/mips/ \
>   -e s/sh[234].*/sh/ -e s/aarch64.*/arm64/ \
>   -e s/riscv.*/riscv/)
>
>
>
>
>
>
>
>
> --
> Best Regards
> Masahiro Yamada


Re: Mac Mini G4 hang on boot with git master

2019-03-15 Thread Mathieu Malaterre
Mark,

On Fri, Mar 15, 2019 at 3:08 PM Mark Cave-Ayland
 wrote:
>
> Hi all,
>
> I've just done a git pull and rebuilt master on my Mac Mini G4 in order to 
> test
> Michael's merge of my KVM PR fix, and unfortunately my kernel now hangs on 
> boot :(

Ouch :(

> My last working git checkout was somewhere around the 5.0-rc stage, so I 
> suspect it's
> something that's been merged for 5.1.

OK. My last kernel is also somewhere around here on same hardware.

> The hang occurs just after the boot console is disabled which makes me wonder 
> if
> something is going wrong during PCI bus enumeration. Does anyone have an idea 
> as to
> what may be causing this? I can obviously bisect it down, but on slow 
> hardware it can
> take some time...

When doing git bisect I compile from my amd64 machine using:

make O=g4 ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- my_defconfig
make -j8 O=g4 ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- bindeb-pkg
scp *image*.deb macminig4:

On Debian simply install:

# apt-get install crossbuild-essential-powerpc

>
> ATB,
>
> Mark.


[PATCH v2 2/2] Remove functions holly_power_off and holly_halt since unused

2019-03-26 Thread Mathieu Malaterre
Silence the following warnings triggered using W=1:

  arch/powerpc/platforms/embedded6xx/holly.c:236:6: error: no previous 
prototype for 'holly_power_off' [-Werror=missing-prototypes]
  arch/powerpc/platforms/embedded6xx/holly.c:243:6: error: no previous 
prototype for 'holly_halt' [-Werror=missing-prototypes]

Signed-off-by: Mathieu Malaterre 
---
 arch/powerpc/platforms/embedded6xx/holly.c | 12 
 1 file changed, 12 deletions(-)

diff --git a/arch/powerpc/platforms/embedded6xx/holly.c 
b/arch/powerpc/platforms/embedded6xx/holly.c
index 9d2eefef7b7b..829bf3697dc9 100644
--- a/arch/powerpc/platforms/embedded6xx/holly.c
+++ b/arch/powerpc/platforms/embedded6xx/holly.c
@@ -234,18 +234,6 @@ static void __noreturn holly_restart(char *cmd)
for (;;) ;
 }
 
-void holly_power_off(void)
-{
-   local_irq_disable();
-   /* No way to shut power off with software */
-   for (;;) ;
-}
-
-void holly_halt(void)
-{
-   holly_power_off();
-}
-
 /*
  * Called very early, device-tree isn't unflattened
  */
-- 
2.20.1



[PATCH v2 1/2] powerpc: Make some functions static

2019-03-26 Thread Mathieu Malaterre
In commit cb9e4d10c448 ("[POWERPC] Add support for 750CL Holly board")
new functions were added. Since most of these functions can be made static,
make it so.

Both holly_power_off and holly_halt functions were not changed since they
are unused, making them static would have triggered the following warning
(treated as error):

  arch/powerpc/platforms/embedded6xx/holly.c:244:13: error: 'holly_halt' 
defined but not used [-Werror=unused-function]

Silence the following warnings triggered using W=1:

  arch/powerpc/platforms/embedded6xx/holly.c:47:5: error: no previous prototype 
for 'holly_exclude_device' [-Werror=missing-prototypes]
  arch/powerpc/platforms/embedded6xx/holly.c:190:6: error: no previous 
prototype for 'holly_show_cpuinfo' [-Werror=missing-prototypes]
  arch/powerpc/platforms/embedded6xx/holly.c:196:17: error: no previous 
prototype for 'holly_restart' [-Werror=missing-prototypes]

Signed-off-by: Mathieu Malaterre 
---
v2: Split the patch in two operations

 arch/powerpc/platforms/embedded6xx/holly.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/embedded6xx/holly.c 
b/arch/powerpc/platforms/embedded6xx/holly.c
index 0409714e8070..9d2eefef7b7b 100644
--- a/arch/powerpc/platforms/embedded6xx/holly.c
+++ b/arch/powerpc/platforms/embedded6xx/holly.c
@@ -44,7 +44,8 @@
 
 #define HOLLY_PCI_CFG_PHYS 0x7c00
 
-int holly_exclude_device(struct pci_controller *hose, u_char bus, u_char devfn)
+static int holly_exclude_device(struct pci_controller *hose, u_char bus,
+   u_char devfn)
 {
if (bus == 0 && PCI_SLOT(devfn) == 0)
return PCIBIOS_DEVICE_NOT_FOUND;
@@ -187,13 +188,13 @@ static void __init holly_init_IRQ(void)
tsi108_write_reg(TSI108_MPIC_OFFSET + 0x30c, 0);
 }
 
-void holly_show_cpuinfo(struct seq_file *m)
+static void holly_show_cpuinfo(struct seq_file *m)
 {
seq_printf(m, "vendor\t\t: IBM\n");
seq_printf(m, "machine\t\t: PPC750 GX/CL\n");
 }
 
-void __noreturn holly_restart(char *cmd)
+static void __noreturn holly_restart(char *cmd)
 {
__be32 __iomem *ocn_bar1 = NULL;
unsigned long bar;
-- 
2.20.1



Re: [PATCH] powerpc: config: skiroot: Add (back) MLX5 ethernet support

2019-04-03 Thread Mathieu Malaterre
On Wed, Apr 3, 2019 at 2:51 AM Joel Stanley  wrote:
>
> It turns out that some defconfig changes and kernel config option
> changes meant we accidentally dropped Ethernet support for Mellanox CLX5
> cards.
>
> Reported-by: Carol L Soto 
> Suggested-by: Carol L Soto 
> Signed-off-by: Stewart Smith 
> Signed-off-by: Joel Stanley 

Fixes: cbc39809a398 ("powerpc/configs: Update skiroot defconfig")

?

> ---
>  arch/powerpc/configs/skiroot_defconfig | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/powerpc/configs/skiroot_defconfig 
> b/arch/powerpc/configs/skiroot_defconfig
> index 5ba131c30f6b..6038b9347d9e 100644
> --- a/arch/powerpc/configs/skiroot_defconfig
> +++ b/arch/powerpc/configs/skiroot_defconfig
> @@ -163,6 +163,8 @@ CONFIG_S2IO=m
>  CONFIG_MLX4_EN=m
>  # CONFIG_MLX4_CORE_GEN2 is not set
>  CONFIG_MLX5_CORE=m
> +CONFIG_MLX5_CORE_EN=y
> +# CONFIG_MLX5_EN_RXNFC is not set
>  # CONFIG_NET_VENDOR_MICREL is not set
>  # CONFIG_NET_VENDOR_MICROSEMI is not set
>  CONFIG_MYRI10GE=m
> --
> 2.20.1
>


Re: [PATCH v2 06/11] MIPS: mark __fls() as __always_inline

2019-04-19 Thread Mathieu Malaterre
Hi,

On Fri, Apr 19, 2019 at 12:06 PM Masahiro Yamada
 wrote:
>
> This prepares to move CONFIG_OPTIMIZE_INLINING from x86 to a common
> place. We need to eliminate potential issues beforehand.
>
> If it is enabled for mips, the following errors are reported:
>
> arch/mips/mm/sc-mips.o: In function `mips_sc_prefetch_enable.part.2':
> sc-mips.c:(.text+0x98): undefined reference to `mips_gcr_base'
> sc-mips.c:(.text+0x9c): undefined reference to `mips_gcr_base'
> sc-mips.c:(.text+0xbc): undefined reference to `mips_gcr_base'
> sc-mips.c:(.text+0xc8): undefined reference to `mips_gcr_base'
> sc-mips.c:(.text+0xdc): undefined reference to `mips_gcr_base'
> arch/mips/mm/sc-mips.o:sc-mips.c:(.text.unlikely+0x44): more undefined 
> references to `mips_gcr_base'

Tested with success on ppc32/G4. But on CI20 (ci20_defconfig from
master), I get:

  MODPOST vmlinux.o
mipsel-linux-gnu-ld: arch/mips/kernel/traps.o: in function
`addr_gcr_err_control':
/home/mathieu/tmp/linux/linux/ci20/../arch/mips/include/asm/mips-cm.h:169:
undefined reference to `mips_gcr_base'
mipsel-linux-gnu-ld:
/home/mathieu/linux/linux/ci20/../arch/mips/include/asm/mips-cm.h:169:
undefined reference to `mips_gcr_base'
mipsel-linux-gnu-ld: arch/mips/mm/sc-mips.o: in function
`addr_gcr_l2_pft_control':
/home/mathieu/linux/linux/ci20/../arch/mips/include/asm/mips-cm.h:246:
undefined reference to `mips_gcr_base'
mipsel-linux-gnu-ld:
/home/mathieu/linux/linux/ci20/../arch/mips/include/asm/mips-cm.h:246:
undefined reference to `mips_gcr_base'
mipsel-linux-gnu-ld:
/home/mathieu/linux/linux/ci20/../arch/mips/include/asm/mips-cm.h:246:
undefined reference to `mips_gcr_base'
mipsel-linux-gnu-ld:
arch/mips/mm/sc-mips.o:/home/mathieu/linux/linux/ci20/../arch/mips/include/asm/mips-cm.h:246:
more undefined references to `mips_gcr_base' follow


> Signed-off-by: Masahiro Yamada 
> ---
>
> Changes in v2:
>   - new patch
>
>  arch/mips/include/asm/bitops.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/mips/include/asm/bitops.h b/arch/mips/include/asm/bitops.h
> index 830c93a010c3..6a26ead1c2b6 100644
> --- a/arch/mips/include/asm/bitops.h
> +++ b/arch/mips/include/asm/bitops.h
> @@ -482,7 +482,7 @@ static inline void __clear_bit_unlock(unsigned long nr, 
> volatile unsigned long *
>   * Return the bit position (0..63) of the most significant 1 bit in a word
>   * Returns -1 if no 1 bit exists
>   */
> -static inline unsigned long __fls(unsigned long word)
> +static __always_inline unsigned long __fls(unsigned long word)
>  {
> int num;
>
> --
> 2.17.1
>


Re: Build regressions/improvements in v4.17-rc1

2018-08-08 Thread Mathieu Malaterre
On Wed, Aug 8, 2018 at 12:34 PM Michael Ellerman  wrote:
>
> Andrew Morton  writes:
> > On Mon, 6 Aug 2018 12:39:21 +0200 Geert Uytterhoeven  
> > wrote:
> >
> >> CC Dan, Michael, AKPM, powerpc
> >>
> >> On Mon, Apr 16, 2018 at 3:10 PM Geert Uytterhoeven  
> >> wrote:
> >> > Below is the list of build error/warning regressions/improvements in
> >> > v4.17-rc1[1] compared to v4.16[2].
> >>
> >> I'd like to point your attention to:
> >>
> >> >   + warning: vmlinux.o(.text+0x376518): Section mismatch in reference 
> >> > from the function .devm_memremap_pages() to the function 
> >> > .meminit.text:.arch_add_memory():  => N/A
> >> >   + warning: vmlinux.o(.text+0x376d64): Section mismatch in reference 
> >> > from the function .devm_memremap_pages_release() to the function 
> >> > .meminit.text:.arch_remove_memory():  => N/A
> >
> > hm.  Dan isn't around at present so we're on our own with this one.
> >
> > x86 doesn't put arch_add_memory and arch_remove_memory into __meminit.
> > x86 does
> >
> > #ifdef CONFIG_MEMORY_HOTPLUG
> > int arch_add_memory(int nid, u64 start, u64 size, struct vmem_altmap 
> > *altmap,
> >   bool want_memblock)
> > {
> >   ...
> >
> >
> > So I guess powerpc should do that as well?
>
> But we only recently added it to fix a section mismatch warning:
>
>   WARNING: vmlinux.o(.text+0x6da88): Section mismatch in reference from the 
> function .arch_add_memory() to the function 
> .meminit.text:.create_section_mapping()
>   The function .arch_add_memory() references
>   the function __meminit .create_section_mapping().
>   This is often because .arch_add_memory lacks a __meminit
>   annotation or the annotation of .create_section_mapping is wrong.
>
>
> I think the problem is that the section mismatch logic isn't able to
> cope with __meminit's changing semantics.
>
> When CONFIG_MEMORY_HOTPLUG=y references from .text to .meminit.text
> should be allowed, because they're just folded in together in the linker
> script.
>
> When CONFIG_MEMORY_HOTPLUG=n references from .text to .meminit.text
> should NOT be allowed, because .meminit.text becomes .init.text and will
> be freed.
>
> I don't see anything in the section mismatch logic to cope with that
> difference.
>
> It looks like __meminit is saving us about 1K on powerpc, so I'm
> strongly inclined to just remove it entirely from arch/powerpc.
>
> Also I haven't been seeing this in my local builds because I have:
>
> CONFIG_LD_DEAD_CODE_DATA_ELIMINATION=y
>
> So I guess we need to work out why that's interfering with section
> mismatch analysis.

...

Well that's a good question actually. Section mismatch
analysis is done on the throwaway vmlinux.o which is not linked
with --gc-sections (and is not a final link), so the via_pmu_driver
symbol should exist and be picked up.

I wonder if something about the  -ffunction-sections is breaking
the reference detection.
...



ref:
https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg135431.html

>
> cheers


Re: cxl: remove a dead branch

2018-08-13 Thread Mathieu Malaterre
Frederic,

Could you double check with Michael what is now best to do.

Thanks

On Mon, Aug 13, 2018 at 1:23 PM Michael Ellerman
 wrote:
>
> On Thu, 2018-03-22 at 21:05:28 UTC, Mathieu Malaterre wrote:
> > In commit 14baf4d9c739 ("cxl: Add guest-specific code") the following code
> > was added:
> >
> >   if (afu->crs_len < 0) {
> >   dev_err(&afu->dev, "Unexpected configuration record size 
> > value\n");
> >   return -EINVAL;
> >   }
> >
> > However the variable `crs_len` is of type u64 and cannot be compared < 0.
> > Remove the dead code section. Fix the following warning treated as error
> > with W=1:
> >
> > ../drivers/misc/cxl/guest.c:919:19: error: comparison of unsigned 
> > expression < 0 is always false [-Werror=type-limits]
> >
> > Signed-off-by: Mathieu Malaterre 
>
> Applied to powerpc next, thanks.
>
> https://git.kernel.org/powerpc/c/e4ecafb14fd9cd77d8f4320af1922e
>
> cheers


Re: [RFT] powerpc/boot: Fix missing crc32poly.h when building with KERNEL_XZ

2018-08-29 Thread Mathieu Malaterre
On Wed, Aug 29, 2018 at 9:59 AM Krzysztof Kozlowski  wrote:
>
> On Wed, 29 Aug 2018 at 09:32, Krzysztof Kozlowski  wrote:
> >
> > After commit faa16bc404d7 ("lib: Use existing define with
> > polynomial") the lib/xz/xz_crc32.c includes a header from include/linux
> > directory thus any other user of this code should define proper include
> > path.
> >
> > This fixes the build error on powerpc with CONFIG_KERNEL_XZ:
> >
> > In file included from 
> > ../arch/powerpc/boot/../../../lib/decompress_unxz.c:233:0,
> >  from ../arch/powerpc/boot/decompress.c:42:
> > ../arch/powerpc/boot/../../../lib/xz/xz_crc32.c:18:29: fatal error: 
> > linux/crc32poly.h: No such file or directory
> >
> > Reported-by: Michal Kubecek 
>
> Reported earlier by Kbuild:
> https://lkml.org/lkml/2018/8/23/47
>
> for the credits:
> Reported-by: kbuild test robot 

Technically Meelis reported it earlier:

https://lkml.org/lkml/2018/8/22/365

;)


[PATCH] powerpc: Add missing include

2018-10-17 Thread Mathieu Malaterre
In commit 88b0fe175735 ("powerpc: Add show_user_instructions()") the
function show_user_instructions was added.

This commit adds an include of header file  to provide
the missing function prototype. Silence the following gcc warning
(treated as error with W=1):

  arch/powerpc/kernel/process.c:1302:6: error: no previous prototype for 
‘show_user_instructions’ [-Werror=missing-prototypes]

Signed-off-by: Mathieu Malaterre 
---
 arch/powerpc/kernel/process.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index bb6ac471a784..1c64491e9702 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -65,6 +65,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
-- 
2.11.0



Re: [PATCH] powerpc: Add missing include

2018-10-17 Thread Mathieu Malaterre
On Thu, Oct 18, 2018 at 6:37 AM Christophe LEROY
 wrote:
>
>
>
> Le 17/10/2018 à 21:25, Mathieu Malaterre a écrit :
> > In commit 88b0fe175735 ("powerpc: Add show_user_instructions()") the
> > function show_user_instructions was added.
> >
> > This commit adds an include of header file  to provide
> > the missing function prototype. Silence the following gcc warning
> > (treated as error with W=1):
> >
> >arch/powerpc/kernel/process.c:1302:6: error: no previous prototype for 
> > ‘show_user_instructions’ [-Werror=missing-prototypes]
> >
> > Signed-off-by: Mathieu Malaterre 
>
> This is already fixed, see
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=c9386bfd37d37f29588de9ea9add455510049c33

Excellent !

Sorry for the noise :(

> Christophe
>
> > ---
> >   arch/powerpc/kernel/process.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> > index bb6ac471a784..1c64491e9702 100644
> > --- a/arch/powerpc/kernel/process.c
> > +++ b/arch/powerpc/kernel/process.c
> > @@ -65,6 +65,7 @@
> >   #include 
> >   #include 
> >   #include 
> > +#include 
> >
> >   #include 
> >   #include 
> >


[PATCH] powerpc/mm: remove const type qualifier from function ‘pud_pfn’

2018-10-31 Thread Mathieu Malaterre
Type qualifier on return type is ignored. Remove warning in W=1:

  arch/powerpc/include/asm/book3s/64/pgtable.h:1268:25: error: type qualifiers 
ignored on function return type [-Werror=ignored-qualifiers]

Signed-off-by: Mathieu Malaterre 
---
 arch/powerpc/include/asm/book3s/64/pgtable.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h 
b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 6c99e846a8c9..2e6ada28da64 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -1304,7 +1304,7 @@ static inline int pgd_devmap(pgd_t pgd)
 }
 #endif /* CONFIG_TRANSPARENT_HUGEPAGE */
 
-static inline const int pud_pfn(pud_t pud)
+static inline int pud_pfn(pud_t pud)
 {
/*
 * Currently all calls to pud_pfn() are gated around a pud_devmap()
-- 
2.11.0



[PATCH] powerpc/32: Add .data..Lubsan_data*/.data..Lubsan_type* sections explicitly

2018-10-31 Thread Mathieu Malaterre
When both `CONFIG_LD_DEAD_CODE_DATA_ELIMINATION=y` and `CONFIG_UBSAN=y`
are set, link step typically produce numberous warnings about orphan
section:

  + powerpc-linux-gnu-ld -EB -m elf32ppc -Bstatic --orphan-handling=warn 
--build-id --gc-sections -X -o .tmp_vmlinux1 -T 
./arch/powerpc/kernel/vmlinux.lds --who
  le-archive built-in.a --no-whole-archive --start-group lib/lib.a --end-group
  powerpc-linux-gnu-ld: warning: orphan section `.data..Lubsan_data393' from 
`init/main.o' being placed in section `.data..Lubsan_data393'.
  powerpc-linux-gnu-ld: warning: orphan section `.data..Lubsan_data394' from 
`init/main.o' being placed in section `.data..Lubsan_data394'.
  ...
  powerpc-linux-gnu-ld: warning: orphan section `.data..Lubsan_type11' from 
`init/main.o' being placed in section `.data..Lubsan_type11'.
  powerpc-linux-gnu-ld: warning: orphan section `.data..Lubsan_type12' from 
`init/main.o' being placed in section `.data..Lubsan_type12'.
  ...

This commit remove those warnings produced at W=1.

Link: https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg135407.html
Suggested-by: Nicholas Piggin 
Signed-off-by: Mathieu Malaterre 
---
 arch/powerpc/kernel/vmlinux.lds.S | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/kernel/vmlinux.lds.S 
b/arch/powerpc/kernel/vmlinux.lds.S
index 434581bcd5b4..1148c3c60c3b 100644
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -308,6 +308,10 @@ SECTIONS
 #ifdef CONFIG_PPC32
.data : AT(ADDR(.data) - LOAD_OFFSET) {
DATA_DATA
+#ifdef CONFIG_UBSAN
+   *(.data..Lubsan_data*)
+   *(.data..Lubsan_type*)
+#endif
*(.data.rel*)
*(SDATA_MAIN)
*(.sdata2)
-- 
2.11.0



[PATCH] powerpc: Mark variable `cpumsr` as unused

2018-11-07 Thread Mathieu Malaterre
Add gcc attribute unused for `cpumsr` variable.

Fix warnings treated as errors with W=1:

  arch/powerpc/kernel/process.c:231:16: error: variable ‘cpumsr’ set but not 
used [-Werror=unused-but-set-variable]
  arch/powerpc/kernel/process.c:296:16: error: variable ‘cpumsr’ set but not 
used [-Werror=unused-but-set-variable]

Signed-off-by: Mathieu Malaterre 
---
 arch/powerpc/kernel/process.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 96f34730010f..b9f1a2408738 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -228,7 +228,7 @@ EXPORT_SYMBOL_GPL(flush_fp_to_thread);
 
 void enable_kernel_fp(void)
 {
-   unsigned long cpumsr;
+   unsigned long cpumsr __maybe_unused;
 
WARN_ON(preemptible());
 
@@ -293,7 +293,7 @@ EXPORT_SYMBOL(giveup_altivec);
 
 void enable_kernel_altivec(void)
 {
-   unsigned long cpumsr;
+   unsigned long cpumsr __maybe_unused;
 
WARN_ON(preemptible());
 
-- 
2.11.0



Re: [PATCH] powerpc: Mark variable `cpumsr` as unused

2018-11-07 Thread Mathieu Malaterre
On Thu, Nov 8, 2018 at 7:09 AM Christophe Leroy  wrote:
>
>
>
> On 11/07/2018 08:26 PM, Mathieu Malaterre wrote:
> > Add gcc attribute unused for `cpumsr` variable.
> >
> > Fix warnings treated as errors with W=1:
> >
> >arch/powerpc/kernel/process.c:231:16: error: variable ‘cpumsr’ set but 
> > not used [-Werror=unused-but-set-variable]
> >arch/powerpc/kernel/process.c:296:16: error: variable ‘cpumsr’ set but 
> > not used [-Werror=unused-but-set-variable]
> >
> > Signed-off-by: Mathieu Malaterre 
>
> I don't think this is the good way to fix that. This problem was
> introduced by commit 5c784c8414fb ("powerpc/tm: Remove
> msr_tm_active()"). That commit should be reverted and fixed.

I see, it makes sense.

> That commit should have removed the macro and kept the inline function.

Breno, what do you think ?

> Christophe
>
> > ---
> >   arch/powerpc/kernel/process.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> > index 96f34730010f..b9f1a2408738 100644
> > --- a/arch/powerpc/kernel/process.c
> > +++ b/arch/powerpc/kernel/process.c
> > @@ -228,7 +228,7 @@ EXPORT_SYMBOL_GPL(flush_fp_to_thread);
> >
> >   void enable_kernel_fp(void)
> >   {
> > - unsigned long cpumsr;
> > + unsigned long cpumsr __maybe_unused;
> >
> >   WARN_ON(preemptible());
> >
> > @@ -293,7 +293,7 @@ EXPORT_SYMBOL(giveup_altivec);
> >
> >   void enable_kernel_altivec(void)
> >   {
> > - unsigned long cpumsr;
> > + unsigned long cpumsr __maybe_unused;
> >
> >   WARN_ON(preemptible());
> >
> >


Re: [PATCH 2/7] powerpc: change CONFIG_6xx to CONFIG_PPC_BOOK3S_32

2018-12-03 Thread Mathieu Malaterre
On Sat, Nov 17, 2018 at 11:29 AM Christophe Leroy
 wrote:
>
> Today we have:
>
> config PPC_BOOK3S_32
> bool "512x/52xx/6xx/7xx/74xx/82xx/83xx/86xx"
> [depends on PPC32 within a choice]
>
> config PPC_BOOK3S
> def_bool y
> depends on PPC_BOOK3S_32 || PPC_BOOK3S_64
>
> config 6xx
> def_bool y
> depends on PPC32 && PPC_BOOK3S
>
> 6xx is therefore redundant with PPC_BOOK3S_32.
>
> In order to make the code clearer, lets use preferably PPC_BOOK3S_32.
> This will allow to remove CONFIG_6xx in a later patch.
>
> Signed-off-by: Christophe Leroy 
> ---
>  arch/powerpc/Makefile |  2 +-
>  arch/powerpc/include/asm/cache.h  |  2 +-
>  arch/powerpc/include/asm/mmu.h|  2 +-
>  arch/powerpc/include/asm/reg.h|  2 +-
>  arch/powerpc/include/asm/time.h   |  2 +-
>  arch/powerpc/kernel/Makefile  |  2 +-
>  arch/powerpc/kernel/cpu_setup_6xx.S   |  2 +-
>  arch/powerpc/kernel/entry_32.S| 10 +-
>  arch/powerpc/kernel/head_32.S | 14 +++---
>  arch/powerpc/kernel/misc_32.S |  4 ++--
>  arch/powerpc/kernel/pmc.c |  2 +-
>  arch/powerpc/kernel/setup_32.c|  2 +-
>  arch/powerpc/kernel/sysfs.c   |  2 +-
>  arch/powerpc/mm/mmu_decl.h|  2 +-
>  arch/powerpc/oprofile/Makefile|  2 +-
>  arch/powerpc/oprofile/common.c|  2 +-
>  arch/powerpc/platforms/powermac/cache.S   |  4 ++--
>  arch/powerpc/platforms/powermac/feature.c |  2 +-
>  arch/powerpc/platforms/powermac/sleep.S   |  4 ++--
>  arch/powerpc/sysdev/Makefile  |  2 +-
>  20 files changed, 33 insertions(+), 33 deletions(-)
>
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index 8a2ce14d68d0..e259b8a2dd44 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -241,7 +241,7 @@ KBUILD_CFLAGS   += $(call 
> cc-option,-fno-dwarf2-cfi-asm)
>  # often slow when they are implemented at all
>  KBUILD_CFLAGS  += $(call cc-option,-mno-string)
>
> -ifdef CONFIG_6xx
> +ifdef CONFIG_PPC_BOOK3S_32
>  KBUILD_CFLAGS  += -mcpu=powerpc
>  endif

I never quite understood this part. Let say I want to specify 'power4'
in arch/powerpc/platforms/Kconfig.cputype as new TARGET_CPU. The line
above will always append -mcpu=powerpc *after* a TARGET_CPU=power4
which defeat the whole purpose, right ?

> diff --git a/arch/powerpc/include/asm/cache.h 
> b/arch/powerpc/include/asm/cache.h
> index 66298461b640..40ea5b3781c6 100644
> --- a/arch/powerpc/include/asm/cache.h
> +++ b/arch/powerpc/include/asm/cache.h
> @@ -71,7 +71,7 @@ extern struct ppc64_caches ppc64_caches;
>  #else
>  #define __read_mostly __attribute__((__section__(".data..read_mostly")))
>
> -#ifdef CONFIG_6xx
> +#ifdef CONFIG_PPC_BOOK3S_32
>  extern long _get_L2CR(void);
>  extern long _get_L3CR(void);
>  extern void _set_L2CR(unsigned long);
> diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h
> index eb20eb3b8fb0..47b651deff5b 100644
> --- a/arch/powerpc/include/asm/mmu.h
> +++ b/arch/powerpc/include/asm/mmu.h
> @@ -48,7 +48,7 @@
>  #define MMU_FTR_USE_HIGH_BATS  ASM_CONST(0x0001)
>
>  /* Enable >32-bit physical addresses on 32-bit processor, only used
> - * by CONFIG_6xx currently as BookE supports that from day 1
> + * by CONFIG_PPC_BOOK3S_32 currently as BookE supports that from day 1
>   */
>  #define MMU_FTR_BIG_PHYS   ASM_CONST(0x0002)
>
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index de52c3166ba4..0d2139a0d5b9 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -582,7 +582,7 @@
>  #define HID0_POWER9_RADIX  __MASK(63 - 8)
>
>  #define SPRN_HID1  0x3F1   /* Hardware Implementation Register 1 
> */
> -#ifdef CONFIG_6xx
> +#ifdef CONFIG_PPC_BOOK3S_32
>  #define HID1_EMCP  (1<<31) /* 7450 Machine Check Pin Enable */
>  #define HID1_DFS   (1<<22) /* 7447A Dynamic Frequency Scaling */
>  #define HID1_PC0   (1<<16) /* 7450 PLL_CFG[0] */
> diff --git a/arch/powerpc/include/asm/time.h b/arch/powerpc/include/asm/time.h
> index b80d492ceb29..54bf7e68a7e1 100644
> --- a/arch/powerpc/include/asm/time.h
> +++ b/arch/powerpc/include/asm/time.h
> @@ -43,7 +43,7 @@ struct div_result {
>
>  /* Accessor functions for the timebase (RTC on 601) registers. */
>  /* If one day CONFIG_POWER is added just define __USE_RTC as 1 */
> -#ifdef CONFIG_6xx
> +#ifdef CONFIG_PPC_BOOK3S_32
>  #define __USE_RTC()(cpu_has_feature(CPU_FTR_USE_RTC))
>  #else
>  #define __USE_RTC()0
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index 53d4b8d5b54d..a5a6a243f3cf 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -69,7 +69,7 @@ obj-$(CONFIG_FA_DUMP) += fadump.o
>  ifdef CONFIG_PPC32
>  obj-$(CONFIG_E500)   

Re: ppc440gp "ebony" board manual/specs

2018-12-04 Thread Mathieu Malaterre
On Tue, Dec 4, 2018 at 12:09 PM Benjamin Herrenschmidt
 wrote:
>
> Hi folks !
>
> Does anybody still has the manual or schematics (or both!) of the old
> "ebony" ppc440gp eval board around by any chance ?

A google search for:

IBM/AMCC "PPC440GP" Evaluation Board datasheet

leads to:

https://4donline.ihs.com/images/VipMasterIC/IC/AMCC/AMCCS00334/AMCCS00334-1.pdf?hkey=EF798316E3902B6ED9A73243A3159BB0


[PATCH] powerpc/32: Move the old 6xx -mcpu logic before the TARGET_CPU logic

2018-12-04 Thread Mathieu Malaterre
The code:

  ifdef CONFIG_6xx
  KBUILD_CFLAGS  += -mcpu=powerpc
  endif

was added in 2006 in commit f48b8296b315 ("[PATCH] powerpc32: Set cpu
explicitly in kernel compiles"). This change was acceptable since the
TARGET_CPU logic was 64-bit only.

Since commit 0e00a8c9fd92 ("powerpc: Allow CPU selection
also on PPC32") this logic is no longer acceptable after the TARGET_CPU
specific. It currently appends -mcpu=powerpc at the end of the command
line, after any TARGET_CPU specific:

  gcc -Wp,-MD,init/.do_mounts.o.d ...
-mcpu=powerpc -mbig-endian -m32 ...
-mcpu=e300c2 ...
-mcpu=powerpc ...
../init/do_mounts.c

Cc: Christophe Leroy 
Fixes: 0e00a8c9fd92 ("powerpc: Allow CPU selection also on PPC32")
Suggested-by: Michael Ellerman 
Link: https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg142315.html
Signed-off-by: Mathieu Malaterre 
---
 arch/powerpc/Makefile | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 8a2ce14d68d0..544b30667ea5 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -30,6 +30,10 @@ endif
 endif
 endif
 
+ifdef CONFIG_6xx
+KBUILD_CFLAGS  += -mcpu=powerpc
+endif
+
 ifeq ($(CROSS_COMPILE),)
 KBUILD_DEFCONFIG := $(shell uname -m)_defconfig
 else
@@ -241,10 +245,6 @@ KBUILD_CFLAGS  += $(call cc-option,-fno-dwarf2-cfi-asm)
 # often slow when they are implemented at all
 KBUILD_CFLAGS  += $(call cc-option,-mno-string)
 
-ifdef CONFIG_6xx
-KBUILD_CFLAGS  += -mcpu=powerpc
-endif
-
 cpu-as-$(CONFIG_4xx)   += -Wa,-m405
 cpu-as-$(CONFIG_ALTIVEC)   += $(call as-option,-Wa$(comma)-maltivec)
 cpu-as-$(CONFIG_E200)  += -Wa,-me200
-- 
2.19.2



Re: [PATCH] powerpc/32: Move the old 6xx -mcpu logic before the TARGET_CPU logic

2018-12-04 Thread Mathieu Malaterre
On Tue, Dec 4, 2018 at 10:28 PM Christophe LEROY
 wrote:
>
>
>
> Le 04/12/2018 à 21:53, Mathieu Malaterre a écrit :
> > The code:
> >
> >ifdef CONFIG_6xx
> >KBUILD_CFLAGS  += -mcpu=powerpc
> >endif
> >
> > was added in 2006 in commit f48b8296b315 ("[PATCH] powerpc32: Set cpu
> > explicitly in kernel compiles"). This change was acceptable since the
> > TARGET_CPU logic was 64-bit only.
> >
> > Since commit 0e00a8c9fd92 ("powerpc: Allow CPU selection
> > also on PPC32") this logic is no longer acceptable after the TARGET_CPU
> > specific. It currently appends -mcpu=powerpc at the end of the command
> > line, after any TARGET_CPU specific:
> >
> >gcc -Wp,-MD,init/.do_mounts.o.d ...
> >  -mcpu=powerpc -mbig-endian -m32 ...
> >  -mcpu=e300c2 ...
> >  -mcpu=powerpc ...
> >  ../init/do_mounts.c
> >
> > Cc: Christophe Leroy 
> > Fixes: 0e00a8c9fd92 ("powerpc: Allow CPU selection also on PPC32")
> > Suggested-by: Michael Ellerman 
> > Link: 
> > https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg142315.html
> > Signed-off-by: Mathieu Malaterre 
> > ---
> >   arch/powerpc/Makefile | 8 
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> > index 8a2ce14d68d0..544b30667ea5 100644
> > --- a/arch/powerpc/Makefile
> > +++ b/arch/powerpc/Makefile
> > @@ -30,6 +30,10 @@ endif
> >   endif
> >   endif
> >
> > +ifdef CONFIG_6xx
> > +KBUILD_CFLAGS+= -mcpu=powerpc
> > +endif
> > +
>
> Could you make the patch on top of my serie (ie after the change of
> CONFIG_6xx to CONFIG_BOOK3S_32 ?

Sure. That will defeat the point of the Fixes: line, but I guess who
cares anyway ? Will re-submit asap.

> The serie is in origin/next-test
>
> Thanks,
> Christophe
>
> >   ifeq ($(CROSS_COMPILE),)
> >   KBUILD_DEFCONFIG := $(shell uname -m)_defconfig
> >   else
> > @@ -241,10 +245,6 @@ KBUILD_CFLAGS+= $(call 
> > cc-option,-fno-dwarf2-cfi-asm)
> >   # often slow when they are implemented at all
> >   KBUILD_CFLAGS   += $(call cc-option,-mno-string)
> >
> > -ifdef CONFIG_6xx
> > -KBUILD_CFLAGS+= -mcpu=powerpc
> > -endif
> > -
> >   cpu-as-$(CONFIG_4xx)+= -Wa,-m405
> >   cpu-as-$(CONFIG_ALTIVEC)+= $(call as-option,-Wa$(comma)-maltivec)
> >   cpu-as-$(CONFIG_E200)   += -Wa,-me200
> >


[PATCH v2] powerpc/32: Move the old 6xx -mcpu logic before the TARGET_CPU logic

2018-12-05 Thread Mathieu Malaterre
The code:

  ifdef CONFIG_6xx
  KBUILD_CFLAGS  += -mcpu=powerpc
  endif

was added in 2006 in commit f48b8296b315 ("[PATCH] powerpc32: Set cpu
explicitly in kernel compiles"). This change was acceptable since the
TARGET_CPU logic was 64-bit only.

Since commit 0e00a8c9fd92 ("powerpc: Allow CPU selection
also on PPC32") this logic is no longer acceptable after the TARGET_CPU
specific. It currently appends -mcpu=powerpc at the end of the command
line, after any TARGET_CPU specific:

  gcc -Wp,-MD,init/.do_mounts.o.d ...
-mcpu=powerpc -mbig-endian -m32 ...
-mcpu=e300c2 ...
-mcpu=powerpc ...
../init/do_mounts.c

Cc: Christophe Leroy 
Fixes: 0e00a8c9fd92 ("powerpc: Allow CPU selection also on PPC32")
Suggested-by: Michael Ellerman 
Link: https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg142315.html
Signed-off-by: Mathieu Malaterre 
---
v2: As suggested by Christophe rebase onto next-test

 arch/powerpc/Makefile | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 40bbeeeb5b4a..4b16588c920f 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -30,6 +30,10 @@ endif
 endif
 endif
 
+ifdef CONFIG_PPC_BOOK3S_32
+KBUILD_CFLAGS  += -mcpu=powerpc
+endif
+
 ifeq ($(CROSS_COMPILE),)
 KBUILD_DEFCONFIG := $(shell uname -m)_defconfig
 else
@@ -241,10 +245,6 @@ KBUILD_CFLAGS  += $(call cc-option,-fno-dwarf2-cfi-asm)
 # often slow when they are implemented at all
 KBUILD_CFLAGS  += $(call cc-option,-mno-string)
 
-ifdef CONFIG_PPC_BOOK3S_32
-KBUILD_CFLAGS  += -mcpu=powerpc
-endif
-
 cpu-as-$(CONFIG_4xx)   += -Wa,-m405
 cpu-as-$(CONFIG_ALTIVEC)   += $(call as-option,-Wa$(comma)-maltivec)
 cpu-as-$(CONFIG_E200)  += -Wa,-me200
-- 
2.19.2



error: no previous prototype for ‘pt_regs_check’

2018-12-07 Thread Mathieu Malaterre
Michael,

I have been wondering for a while now, but I failed to make sense of
this function: pt_regs_check (commit 002af9391bfbe). What is this
function meant for ?

Background, it breaks my W=1 build with:

../arch/powerpc/kernel/ptrace.c:3339:13: error: no previous prototype
for ‘pt_regs_check’ [-Werror=missing-prototypes]

I failed to find a usage of it within the linux source tree... (git
grep pt_regs_check returns nothing).

Thanks,


[PATCH] powerpc/ptrace: Add prototype for function pt_regs_check

2018-12-08 Thread Mathieu Malaterre
`pt_regs_check` is a dummy function, its purpose is to break the build
if struct pt_regs and struct user_pt_regs don't match.

This function has no functionnal purpose, and will get eliminated at
link time or after init depending on CONFIG_LD_DEAD_CODE_DATA_ELIMINATION

This commit adds a prototype to fix warning at W=1:

  arch/powerpc/kernel/ptrace.c:3339:13: error: no previous prototype for 
‘pt_regs_check’ [-Werror=missing-prototypes]

Suggested-by: Christophe Leroy 
Signed-off-by: Mathieu Malaterre 
---
 arch/powerpc/kernel/ptrace.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index a398999d0770..341c0060b4c8 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -3338,6 +3338,10 @@ void do_syscall_trace_leave(struct pt_regs *regs)
user_enter();
 }
 
+void __init pt_regs_check(void);
+/* dummy function, its purpose is to break the build if struct pt_regs and
+ * struct user_pt_regs don't match.
+ */
 void __init pt_regs_check(void)
 {
BUILD_BUG_ON(offsetof(struct pt_regs, gpr) !=
-- 
2.19.2



Re: [PATCH] powerpc: enable a 30-bit ZONE_DMA for 32-bit pmac

2019-06-14 Thread Mathieu Malaterre
On Thu, Jun 13, 2019 at 10:27 AM Christoph Hellwig  wrote:
>
> With the strict dma mask checking introduced with the switch to
> the generic DMA direct code common wifi chips on 32-bit powerbooks
> stopped working.  Add a 30-bit ZONE_DMA to the 32-bit pmac builds
> to allow them to reliably allocate dma coherent memory.
>
> Fixes: 65a21b71f948 ("powerpc/dma: remove dma_nommu_dma_supported")
> Reported-by: Aaro Koskinen 
> Signed-off-by: Christoph Hellwig 
> ---
>  arch/powerpc/include/asm/page.h | 7 +++
>  arch/powerpc/mm/mem.c   | 3 ++-
>  arch/powerpc/platforms/powermac/Kconfig | 1 +
>  3 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
> index b8286a2013b4..0d52f57fca04 100644
> --- a/arch/powerpc/include/asm/page.h
> +++ b/arch/powerpc/include/asm/page.h
> @@ -319,6 +319,13 @@ struct vm_area_struct;
>  #endif /* __ASSEMBLY__ */
>  #include 
>
> +/*
> + * Allow 30-bit DMA for very limited Broadcom wifi chips on many powerbooks.

nit: would it be possible to mention explicit reference to b43-legacy.
Using b43 on my macmini g4 never showed those symptoms (using
5.2.0-rc2+)

$ dmesg | grep b43
[   14.327189] bus: 'pci': add driver b43-pci-bridge
[   14.345354] bus: 'pci': driver_probe_device: matched device
0001:10:12.0 with driver b43-pci-bridge
[   14.380110] bus: 'pci': really_probe: probing driver b43-pci-bridge
with device 0001:10:12.0
[   14.440295] b43-pci-bridge 0001:10:12.0: enabling device (0004 -> 0006)
[   14.637223] b43-pci-bridge 0001:10:12.0: Sonics Silicon Backplane
found on PCI device 0001:10:12.0
[   14.644858] driver: 'b43-pci-bridge': driver_bound: bound to device
'0001:10:12.0'
[   14.743341] bus: 'pci': really_probe: bound device 0001:10:12.0 to
driver b43-pci-bridge
[   18.724343] bus: 'bcma': add driver b43
[   18.728635] bus: 'ssb': add driver b43
[   18.734305] bus: 'ssb': driver_probe_device: matched device ssb0:0
with driver b43
[   18.743155] bus: 'ssb': really_probe: probing driver b43 with device ssb0:0
[   18.747782] b43-phy0: Broadcom 4306 WLAN found (core revision 5)
[   18.767439] b43-phy0: Found PHY: Analog 2, Type 2 (G), Revision 2
[   18.771759] b43-phy0: Found Radio: Manuf 0x17F, ID 0x2050, Revision
2, Version 0
[   18.795467] driver: 'b43': driver_bound: bound to device 'ssb0:0'
[   18.801533] bus: 'ssb': really_probe: bound device ssb0:0 to driver b43
[   22.143084] b43-phy0: Loading firmware version 666.2 (2011-02-23 01:15:07)
[   25.133011] b43 ssb0:0 wlan0: disabling HT as WMM/QoS is not
supported by the AP
[   25.140221] b43 ssb0:0 wlan0: disabling VHT as WMM/QoS is not
supported by the AP


> + */
> +#ifdef CONFIG_PPC32
> +#define ARCH_ZONE_DMA_BITS 30
> +#else
>  #define ARCH_ZONE_DMA_BITS 31
> +#endif
>
>  #endif /* _ASM_POWERPC_PAGE_H */
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index cba29131bccc..2540d3b2588c 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -248,7 +248,8 @@ void __init paging_init(void)
>(long int)((top_of_ram - total_ram) >> 20));
>
>  #ifdef CONFIG_ZONE_DMA
> -   max_zone_pfns[ZONE_DMA] = min(max_low_pfn, 0x7fffUL >> 
> PAGE_SHIFT);
> +   max_zone_pfns[ZONE_DMA] = min(max_low_pfn,
> +   ((1UL << ARCH_ZONE_DMA_BITS) - 1) >> PAGE_SHIFT);
>  #endif
> max_zone_pfns[ZONE_NORMAL] = max_low_pfn;
>  #ifdef CONFIG_HIGHMEM
> diff --git a/arch/powerpc/platforms/powermac/Kconfig 
> b/arch/powerpc/platforms/powermac/Kconfig
> index f834a19ed772..c02d8c503b29 100644
> --- a/arch/powerpc/platforms/powermac/Kconfig
> +++ b/arch/powerpc/platforms/powermac/Kconfig
> @@ -7,6 +7,7 @@ config PPC_PMAC
> select PPC_INDIRECT_PCI if PPC32
> select PPC_MPC106 if PPC32
> select PPC_NATIVE
> +   select ZONE_DMA if PPC32
> default y
>
>  config PPC_PMAC64
> --
> 2.20.1
>


Re: [PATCH] powerpc: enable a 30-bit ZONE_DMA for 32-bit pmac

2019-06-19 Thread Mathieu Malaterre
On Wed, Jun 19, 2019 at 4:18 PM Benjamin Herrenschmidt
 wrote:
>
> On Wed, 2019-06-19 at 22:32 +1000, Michael Ellerman wrote:
> > Christoph Hellwig  writes:
> > > Any chance this could get picked up to fix the regression?
> >
> > Was hoping Ben would Ack it. He's still powermac maintainer :)
> >
> > I guess he OK'ed it in the other thread, will add it to my queue.
>
> Yeah ack. If I had written it myself, I would have made the DMA bits a
> variable and only set it down to 30 if I see that device in the DT
> early on, but I can't be bothered now, if it works, ship it :-)
>
> Note: The patch affects all ppc32, though I don't think it will cause
> any significant issue on those who don't need it.

Thanks, that answer my earlier question.

> Cheers,
> Ben.
>
> > cheers
> >
> > > On Thu, Jun 13, 2019 at 10:24:46AM +0200, Christoph Hellwig wrote:
> > > > With the strict dma mask checking introduced with the switch to
> > > > the generic DMA direct code common wifi chips on 32-bit
> > > > powerbooks
> > > > stopped working.  Add a 30-bit ZONE_DMA to the 32-bit pmac builds
> > > > to allow them to reliably allocate dma coherent memory.
> > > >
> > > > Fixes: 65a21b71f948 ("powerpc/dma: remove
> > > > dma_nommu_dma_supported")
> > > > Reported-by: Aaro Koskinen 
> > > > Signed-off-by: Christoph Hellwig 
> > > > ---
> > > >  arch/powerpc/include/asm/page.h | 7 +++
> > > >  arch/powerpc/mm/mem.c   | 3 ++-
> > > >  arch/powerpc/platforms/powermac/Kconfig | 1 +
> > > >  3 files changed, 10 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/arch/powerpc/include/asm/page.h
> > > > b/arch/powerpc/include/asm/page.h
> > > > index b8286a2013b4..0d52f57fca04 100644
> > > > --- a/arch/powerpc/include/asm/page.h
> > > > +++ b/arch/powerpc/include/asm/page.h
> > > > @@ -319,6 +319,13 @@ struct vm_area_struct;
> > > >  #endif /* __ASSEMBLY__ */
> > > >  #include 
> > > >
> > > > +/*
> > > > + * Allow 30-bit DMA for very limited Broadcom wifi chips on many
> > > > powerbooks.
> > > > + */
> > > > +#ifdef CONFIG_PPC32
> > > > +#define ARCH_ZONE_DMA_BITS 30
> > > > +#else
> > > >  #define ARCH_ZONE_DMA_BITS 31
> > > > +#endif
> > > >
> > > >  #endif /* _ASM_POWERPC_PAGE_H */
> > > > diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> > > > index cba29131bccc..2540d3b2588c 100644
> > > > --- a/arch/powerpc/mm/mem.c
> > > > +++ b/arch/powerpc/mm/mem.c
> > > > @@ -248,7 +248,8 @@ void __init paging_init(void)
> > > >  (long int)((top_of_ram - total_ram) >> 20));
> > > >
> > > >  #ifdef CONFIG_ZONE_DMA
> > > > - max_zone_pfns[ZONE_DMA] = min(max_low_pfn, 0x7fffUL
> > > > >> PAGE_SHIFT);
> > > > + max_zone_pfns[ZONE_DMA] = min(max_low_pfn,
> > > > + ((1UL << ARCH_ZONE_DMA_BITS) - 1) >>
> > > > PAGE_SHIFT);
> > > >  #endif
> > > >   max_zone_pfns[ZONE_NORMAL] = max_low_pfn;
> > > >  #ifdef CONFIG_HIGHMEM
> > > > diff --git a/arch/powerpc/platforms/powermac/Kconfig
> > > > b/arch/powerpc/platforms/powermac/Kconfig
> > > > index f834a19ed772..c02d8c503b29 100644
> > > > --- a/arch/powerpc/platforms/powermac/Kconfig
> > > > +++ b/arch/powerpc/platforms/powermac/Kconfig
> > > > @@ -7,6 +7,7 @@ config PPC_PMAC
> > > >   select PPC_INDIRECT_PCI if PPC32
> > > >   select PPC_MPC106 if PPC32
> > > >   select PPC_NATIVE
> > > > + select ZONE_DMA if PPC32
> > > >   default y
> > > >
> > > >  config PPC_PMAC64
> > > > --
> > > > 2.20.1
> > >
> > > ---end quoted text---
>


[PATCH] powerpc/lib/xor_vmx: Relax frame size for clang

2019-06-21 Thread Mathieu Malaterre
When building with clang-8 the frame size limit is hit:

  ../arch/powerpc/lib/xor_vmx.c:119:6: error: stack frame size of 1200 bytes in 
function '__xor_altivec_5' [-Werror,-Wframe-larger-than=]

Follow the same approach as commit 9c87156cce5a ("powerpc/xmon: Relax
frame size for clang") until a proper fix is implemented upstream in
clang and relax requirement for clang.

Link: https://github.com/ClangBuiltLinux/linux/issues/563
Cc: Joel Stanley 
Signed-off-by: Mathieu Malaterre 
---
 arch/powerpc/lib/Makefile | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index c55f9c27bf79..b3f7d64caaf0 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -58,5 +58,9 @@ obj-$(CONFIG_FTR_FIXUP_SELFTEST) += feature-fixups-test.o
 
 obj-$(CONFIG_ALTIVEC)  += xor_vmx.o xor_vmx_glue.o
 CFLAGS_xor_vmx.o += -maltivec $(call cc-option,-mabi=altivec)
+ifdef CONFIG_CC_IS_CLANG
+# See https://github.com/ClangBuiltLinux/linux/issues/563
+CFLAGS_xor_vmx.o += -Wframe-larger-than=4096
+endif
 
 obj-$(CONFIG_PPC64) += $(obj64-y)
-- 
2.20.1



Re: [RFC PATCH v1 11/13] powerpc/ptrace: create ppc_gethwdinfo()

2019-06-25 Thread Mathieu Malaterre
On Tue, Jun 25, 2019 at 1:22 PM Christophe Leroy
 wrote:
>
> Create ippc_gethwdinfo() to handle PPC_PTRACE_GETHWDBGINFO and

s/ippc_gethwdinfo/ppc_gethwdinfo/

> reduce ifdef mess
>
> Signed-off-by: Christophe Leroy 
> ---
>  arch/powerpc/kernel/ptrace/ptrace-adv.c   | 15 +++
>  arch/powerpc/kernel/ptrace/ptrace-decl.h  |  1 +
>  arch/powerpc/kernel/ptrace/ptrace-noadv.c | 20 +++
>  arch/powerpc/kernel/ptrace/ptrace.c   | 32 
> +--
>  4 files changed, 37 insertions(+), 31 deletions(-)
>
> diff --git a/arch/powerpc/kernel/ptrace/ptrace-adv.c 
> b/arch/powerpc/kernel/ptrace/ptrace-adv.c
> index dcc765940344..f5f334484ebc 100644
> --- a/arch/powerpc/kernel/ptrace/ptrace-adv.c
> +++ b/arch/powerpc/kernel/ptrace/ptrace-adv.c
> @@ -83,6 +83,21 @@ void user_disable_single_step(struct task_struct *task)
> clear_tsk_thread_flag(task, TIF_SINGLESTEP);
>  }
>
> +void ppc_gethwdinfo(struct ppc_debug_info *dbginfo)

Would it be possible to rename it to `ppc_gethwdbginfo`, I find it
easier to read.

> +{
> +   dbginfo->version = 1;
> +   dbginfo->num_instruction_bps = CONFIG_PPC_ADV_DEBUG_IACS;
> +   dbginfo->num_data_bps = CONFIG_PPC_ADV_DEBUG_DACS;
> +   dbginfo->num_condition_regs = CONFIG_PPC_ADV_DEBUG_DVCS;
> +   dbginfo->data_bp_alignment = 4;
> +   dbginfo->sizeof_condition = 4;
> +   dbginfo->features = PPC_DEBUG_FEATURE_INSN_BP_RANGE |
> +   PPC_DEBUG_FEATURE_INSN_BP_MASK;
> +   if (IS_ENABLED(CONFIG_PPC_ADV_DEBUG_DAC_RANGE))
> +   dbginfo->features |= PPC_DEBUG_FEATURE_DATA_BP_RANGE |
> +PPC_DEBUG_FEATURE_DATA_BP_MASK;
> +}
> +
>  int ptrace_get_debugreg(struct task_struct *child, unsigned long addr,
> unsigned long __user *datalp)
>  {
> diff --git a/arch/powerpc/kernel/ptrace/ptrace-decl.h 
> b/arch/powerpc/kernel/ptrace/ptrace-decl.h
> index cd5b8256ba56..2404b987b23c 100644
> --- a/arch/powerpc/kernel/ptrace/ptrace-decl.h
> +++ b/arch/powerpc/kernel/ptrace/ptrace-decl.h
> @@ -141,6 +141,7 @@ int tm_cgpr32_set(struct task_struct *target, const 
> struct user_regset *regset,
>  extern const struct user_regset_view user_ppc_native_view;
>
>  /* ptrace-(no)adv */
> +void ppc_gethwdinfo(struct ppc_debug_info *dbginfo);
>  int ptrace_get_debugreg(struct task_struct *child, unsigned long addr,
> unsigned long __user *datalp);
>  int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, 
> unsigned long data);
> diff --git a/arch/powerpc/kernel/ptrace/ptrace-noadv.c 
> b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
> index 985cca136f85..426fedd7ab6c 100644
> --- a/arch/powerpc/kernel/ptrace/ptrace-noadv.c
> +++ b/arch/powerpc/kernel/ptrace/ptrace-noadv.c
> @@ -64,6 +64,26 @@ void user_disable_single_step(struct task_struct *task)
> clear_tsk_thread_flag(task, TIF_SINGLESTEP);
>  }
>
> +void ppc_gethwdinfo(struct ppc_debug_info *dbginfo)
> +{
> +   dbginfo->version = 1;
> +   dbginfo->num_instruction_bps = 0;
> +   if (ppc_breakpoint_available())
> +   dbginfo->num_data_bps = 1;
> +   else
> +   dbginfo->num_data_bps = 0;
> +   dbginfo->num_condition_regs = 0;
> +   dbginfo->data_bp_alignment = sizeof(long);
> +   dbginfo->sizeof_condition = 0;
> +   if (IS_ENABLED(CONFIG_HAVE_HW_BREAKPOINT)) {
> +   dbginfo->features = PPC_DEBUG_FEATURE_DATA_BP_RANGE;
> +   if (dawr_enabled())
> +   dbginfo->features |= PPC_DEBUG_FEATURE_DATA_BP_DAWR;
> +   } else {
> +   dbginfo->features = 0;
> +   }
> +}
> +
>  int ptrace_get_debugreg(struct task_struct *child, unsigned long addr,
> unsigned long __user *datalp)
>  {
> diff --git a/arch/powerpc/kernel/ptrace/ptrace.c 
> b/arch/powerpc/kernel/ptrace/ptrace.c
> index e789afae6f56..31e8c5a9171e 100644
> --- a/arch/powerpc/kernel/ptrace/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace/ptrace.c
> @@ -159,37 +159,7 @@ long arch_ptrace(struct task_struct *child, long request,
> case PPC_PTRACE_GETHWDBGINFO: {
> struct ppc_debug_info dbginfo;
>
> -   dbginfo.version = 1;
> -#ifdef CONFIG_PPC_ADV_DEBUG_REGS
> -   dbginfo.num_instruction_bps = CONFIG_PPC_ADV_DEBUG_IACS;
> -   dbginfo.num_data_bps = CONFIG_PPC_ADV_DEBUG_DACS;
> -   dbginfo.num_condition_regs = CONFIG_PPC_ADV_DEBUG_DVCS;
> -   dbginfo.data_bp_alignment = 4;
> -   dbginfo.sizeof_condition = 4;
> -   dbginfo.features = PPC_DEBUG_FEATURE_INSN_BP_RANGE |
> -  PPC_DEBUG_FEATURE_INSN_BP_MASK;
> -#ifdef CONFIG_PPC_ADV_DEBUG_DAC_RANGE
> -   dbginfo.features |=
> -  PPC_DEBUG_FEATURE_DATA_BP_RANGE |
> -  PPC_DEBUG_FEATURE_DATA_BP_MASK;
> -#endif
> -#e

5.2.7 kernel doesn't boot on G5

2019-08-15 Thread Mathieu Malaterre
Does that ring a bell to anyone here ? Thanks

-- Forwarded message -
To: 


Hi,

No log only a photo :

https://www.deb-multimedia.org/tests/20190812_143628.jpg

Christian


[PATCH] powerpc/ptrace: fix empty-body warning

2018-12-18 Thread Mathieu Malaterre
In commit a225f1567405 ("powerpc/ptrace: replace ptrace_report_syscall()
with a tracehook call") an empty body if(); was added.

Replace ; with {} to remove a warning (treated as error) reported by gcc
using W=1:

  arch/powerpc/kernel/ptrace.c: In function ‘do_syscall_trace_enter’:
  arch/powerpc/kernel/ptrace.c:3281:4: error: suggest braces around empty body 
in an ‘if’ statement [-Werror=empty-body]

Fixes: a225f1567405 ("powerpc/ptrace: replace ptrace_report_syscall() with a 
tracehook call")
Signed-off-by: Mathieu Malaterre 
---
 arch/powerpc/kernel/ptrace.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 8314e8fed0ee..e1988892b3e7 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -3277,8 +3277,8 @@ long do_syscall_trace_enter(struct pt_regs *regs)
 * avoid clobbering any register also, thus, not 'gotoing'
 * skip label.
 */
-   if (tracehook_report_syscall_entry(regs))
-   ;
+   if (tracehook_report_syscall_entry(regs)) {
+   }
return -1;
}
 
-- 
2.19.2



[PATCH] Remove 'type' argument from access_ok() function

2019-01-04 Thread Mathieu Malaterre
In commit 05a4ab823983 ("powerpc/uaccess: fix warning/error with
access_ok()") an attempt was made to remove a warning by referencing the
variable `type`, however in commit 96d4f267e40f ("Remove 'type' argument
from access_ok() function") the variable `type` has been removed.

Revert commit 05a4ab823983 ("powerpc/uaccess: fix warning/error with
access_ok()") to fix the following compilation error:

  arch/powerpc/include/asm/uaccess.h:66:32: error: ‘type’ undeclared (first use 
in this function)

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

diff --git a/arch/powerpc/include/asm/uaccess.h 
b/arch/powerpc/include/asm/uaccess.h
index b31bf45eebd4..5f0c98e511a0 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -63,7 +63,7 @@ static inline int __access_ok(unsigned long addr, unsigned 
long size,
 #endif
 
 #define access_ok(addr, size)  \
-   (__chk_user_ptr(addr), (void)(type),\
+   (__chk_user_ptr(addr),  \
 __access_ok((__force unsigned long)(addr), (size), get_fs()))
 
 /*
-- 
2.19.2



[PATCH] powerpc: Allow CPU selection of G4/74xx variant

2019-01-14 Thread Mathieu Malaterre
GCC supports -mcpu=G4

This patch gives the opportunity to select ALTIVEC for this variant.

Signed-off-by: Mathieu Malaterre 
---
 arch/powerpc/platforms/Kconfig.cputype | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/arch/powerpc/platforms/Kconfig.cputype 
b/arch/powerpc/platforms/Kconfig.cputype
index 8c7464c3f27f..7c544607ba32 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -153,6 +153,11 @@ config E300C3_CPU
bool "e300c3 (831x)"
depends on PPC_BOOK3S_32
 
+config G4_CPU
+   bool "G4 (74xx)"
+   depends on PPC_BOOK3S_32
+   select ALTIVEC
+
 endchoice
 
 config TARGET_CPU_BOOL
@@ -171,6 +176,7 @@ config TARGET_CPU
default "860" if 860_CPU
default "e300c2" if E300C2_CPU
default "e300c3" if E300C3_CPU
+   default "G4" if G4_CPU
 
 config PPC_BOOK3S
def_bool y
-- 
2.19.2



gcc 6.3 vs 8.2 Re: [RFC PATCH] powerpc: Enable kcov

2019-01-14 Thread Mathieu Malaterre
[Sorry to hijack this thread. ]

On Tue, Jan 15, 2019 at 5:22 AM Andrew Donnellan
 wrote:
>
> kcov provides kernel coverage data that's useful for fuzzing tools like
> syzkaller.
>
> Wire up kcov support on powerpc. Disable kcov instrumentation on the same
> files where we currently disable gcov and UBSan instrumentation.
>
> Signed-off-by: Andrew Donnellan 
>
> ---
>
> kcov looks like it's working okay, both kcovtrace and syzkaller seem to be
> working. I did see some issues with compiling and booting kernels with gcc
> 6.3 and earlier versions which disappeared when I upgraded to gcc 8.2, I
> need to investigate that more.

This is also my setup. Would you be able to tell why I get a kconfig
option loop when using gcc 8.2 which I cannot reproduce using gcc 6.3
?

gcc 6-3 is ok doing:

$ make ARCH=powerpc custom_defconfig
$ make ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu-

while gcc 8.2 leads to (linux/master):

...
$ make ARCH=powerpc custom_defconfig
$ make ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu-
  GEN Makefile
scripts/kconfig/conf  --syncconfig Kconfig
*
* Restart config...
*
*
* General architecture-dependent options
*
OProfile system profiling (OPROFILE) [M/n/y/?] m
Kprobes (KPROBES) [Y/n/?] y
Optimize very unlikely/likely branches (JUMP_LABEL) [N/y/?] n
Stack Protector buffer overflow detection (STACKPROTECTOR) [Y/n/?] (NEW)
...

I did check that `custom_defconfig` is the minimal defconfig generated
by `savedefconfig` in both cases.

Hints or comments welcome, thanks much.

> ---
>  arch/powerpc/Kconfig| 1 +
>  arch/powerpc/kernel/Makefile| 7 ++-
>  arch/powerpc/kernel/trace/Makefile  | 3 ++-
>  arch/powerpc/kernel/vdso32/Makefile | 1 +
>  arch/powerpc/kernel/vdso64/Makefile | 1 +
>  arch/powerpc/xmon/Makefile  | 1 +
>  6 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 2890d36eb531..d3698dae0e60 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -134,6 +134,7 @@ config PPC
> select ARCH_HAS_ELF_RANDOMIZE
> select ARCH_HAS_FORTIFY_SOURCE
> select ARCH_HAS_GCOV_PROFILE_ALL
> +   select ARCH_HAS_KCOV
> select ARCH_HAS_PHYS_TO_DMA
> select ARCH_HAS_PMEM_APIif PPC64
> select ARCH_HAS_PTE_SPECIAL
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index cb7f0bb9ee71..961f44eabb65 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -142,16 +142,21 @@ endif
>  obj-$(CONFIG_EPAPR_PARAVIRT)   += epapr_paravirt.o epapr_hcalls.o
>  obj-$(CONFIG_KVM_GUEST)+= kvm.o kvm_emul.o
>
> -# Disable GCOV & sanitizers in odd or sensitive code
> +# Disable GCOV, KCOV & sanitizers in odd or sensitive code
>  GCOV_PROFILE_prom_init.o := n
> +KCOV_INSTRUMENT_prom_init.o := n
>  UBSAN_SANITIZE_prom_init.o := n
>  GCOV_PROFILE_machine_kexec_64.o := n
> +KCOV_INSTRUMENT_machine_kexec_64.o := n
>  UBSAN_SANITIZE_machine_kexec_64.o := n
>  GCOV_PROFILE_machine_kexec_32.o := n
> +KCOV_INSTRUMENT_machine_kexec_32.o := n
>  UBSAN_SANITIZE_machine_kexec_32.o := n
>  GCOV_PROFILE_kprobes.o := n
> +KCOV_INSTRUMENT_kprobes.o := n
>  UBSAN_SANITIZE_kprobes.o := n
>  GCOV_PROFILE_kprobes-ftrace.o := n
> +KCOV_INSTRUMENT_kprobes-ftrace.o := n
>  UBSAN_SANITIZE_kprobes-ftrace.o := n
>  UBSAN_SANITIZE_vdso.o := n
>
> diff --git a/arch/powerpc/kernel/trace/Makefile 
> b/arch/powerpc/kernel/trace/Makefile
> index b1725ad3e13d..858503775c58 100644
> --- a/arch/powerpc/kernel/trace/Makefile
> +++ b/arch/powerpc/kernel/trace/Makefile
> @@ -23,6 +23,7 @@ obj-$(CONFIG_TRACING) += trace_clock.o
>  obj-$(CONFIG_PPC64)+= $(obj64-y)
>  obj-$(CONFIG_PPC32)+= $(obj32-y)
>
> -# Disable GCOV & sanitizers in odd or sensitive code
> +# Disable GCOV, KCOV & sanitizers in odd or sensitive code
>  GCOV_PROFILE_ftrace.o := n
> +KCOV_INSTRUMENT_ftrace.o := n
>  UBSAN_SANITIZE_ftrace.o := n
> diff --git a/arch/powerpc/kernel/vdso32/Makefile 
> b/arch/powerpc/kernel/vdso32/Makefile
> index 50112d4473bb..ce199f6e4256 100644
> --- a/arch/powerpc/kernel/vdso32/Makefile
> +++ b/arch/powerpc/kernel/vdso32/Makefile
> @@ -23,6 +23,7 @@ targets := $(obj-vdso32) vdso32.so vdso32.so.dbg
>  obj-vdso32 := $(addprefix $(obj)/, $(obj-vdso32))
>
>  GCOV_PROFILE := n
> +KCOV_INSTRUMENT := n
>  UBSAN_SANITIZE := n
>
>  ccflags-y := -shared -fno-common -fno-builtin
> diff --git a/arch/powerpc/kernel/vdso64/Makefile 
> b/arch/powerpc/kernel/vdso64/Makefile
> index 69cecb346269..28e7d112aa2f 100644
> --- a/arch/powerpc/kernel/vdso64/Makefile
> +++ b/arch/powerpc/kernel/vdso64/Makefile
> @@ -9,6 +9,7 @@ targets := $(obj-vdso64) vdso64.so vdso64.so.dbg
>  obj-vdso64 := $(addprefix $(obj)/, $(obj-vdso64))
>
>  GCOV_PROFILE := n
> +KCOV_INSTRUMENT := n
>  UBSAN_SANITIZE := n
>
>  ccflags-y := -shared -fno-common -fno-builtin
> diff --git a/arch/powerpc/xmon/Make

Re: [PATCH] kbuild: mark prepare0 as PHONY to fix external module build

2019-01-15 Thread Mathieu Malaterre
On Tue, Jan 15, 2019 at 8:22 AM Masahiro Yamada
 wrote:
>
> Commit c3ff2a5193fa ("powerpc/32: add stack protector support")
> caused kernel panic on PowerPC if an external module is used with
> CONFIG_STACKPROTECTOR because the 'prepare' target was not executed
> for the external module build.
>
> Commit e07db28eea38 ("kbuild: fix single target build for external
> module") turned it into a build error because the 'prepare' target is
> now executed but the 'prepare0' target is missing for the external
> module build.
>
> External module on arm/arm64 with CONFIG_STACKPROTECTOR_PER_TASK is
> also broken in the same way.
>
> Move 'PHONY += prepare0' to the common place. Make is fine with missing
> rule for phony targets.
>
> I minimize the change so it can be easily backported to 4.20.x
>
> To fix v4.20 for external modules of PowerPC, please backport
> e07db28eea38 ("kbuild: fix single target build for external module"),
> and then this commit.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=201891
> Fixes: e07db28eea38 ("kbuild: fix single target build for external module")
> Fixes: c3ff2a5193fa ("powerpc/32: add stack protector support")
> Fixes: 189af4657186 ("ARM: smp: add support for per-task stack canaries")
> Fixes: 0a1213fa7432 ("arm64: enable per-task stack canaries")
> Cc: linux-stable  # v4.20
> Reported-by: Samuel Holland 
> Reported-by: Alexey Kardashevskiy 
> Signed-off-by: Masahiro Yamada 
> ---
>
>  Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 499b968..789b332 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -955,6 +955,7 @@ ifdef CONFIG_STACK_VALIDATION
>endif
>  endif
>
> +PHONY += prepare0
>
>  ifeq ($(KBUILD_EXTMOD),)
>  core-y += kernel/ certs/ mm/ fs/ ipc/ security/ crypto/ block/
> @@ -1061,8 +1062,7 @@ scripts: scripts_basic scripts_dtc
>  # archprepare is used in arch Makefiles and when processed asm symlink,
>  # version.h and scripts_basic is processed / created.
>
> -# Listed in dependency order

The above comment may need to be tweaked a bit

> -PHONY += prepare archprepare prepare0 prepare1 prepare2 prepare3
> +PHONY += prepare archprepare prepare1 prepare2 prepare3
>
>  # prepare3 is used to check if we are building in a separate output 
> directory,
>  # and if so do:
> --
> 2.7.4
>


Re: gcc 6.3 vs 8.2 Re: [RFC PATCH] powerpc: Enable kcov

2019-01-15 Thread Mathieu Malaterre
On Wed, Jan 16, 2019 at 6:09 AM Michael Ellerman  wrote:
>
> Christophe Leroy  writes:
> > Le 15/01/2019 à 08:26, Mathieu Malaterre a écrit :
> ...
> >>
> >> I did check that `custom_defconfig` is the minimal defconfig generated
> >> by `savedefconfig` in both cases.
> >>
> >> Hints or comments welcome, thanks much.
> >
> > I think you should do:
> >
> > make ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- custom_defconfig
> > make ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu-
>
> Yep, you definitely need to use the same compiler for configuring and
> building.


That was it. Thanks for the help !


Re: ptrace compile failure with gcc-8.2 on 32-bit powerpc

2019-01-18 Thread Mathieu Malaterre
> Yeah I noticed this just yesterday.

Just FYI this has been accepted as regression in gcc, and fixed is
provided for GCC 9.0:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88273


Re: G5 Quad hangs early on 4.20.2 / 5.0-rc2+

2019-01-18 Thread Mathieu Malaterre
On Fri, Jan 18, 2019 at 1:53 AM Tobias Ulmer  wrote:
>
> On Fri, Jan 18, 2019 at 09:32:07AM +1100, Benjamin Herrenschmidt wrote:
> > On Thu, 2019-01-17 at 10:42 +0100, Tobias Ulmer wrote:
> > > On Wed, Jan 16, 2019 at 12:15:14PM +1100, Benjamin Herrenschmidt wrote:
> > > > On Tue, 2019-01-15 at 23:49 +0100, Tobias Ulmer wrote:
> > > > > Hi,
> > > > >
> > > > > both the latest stable 4.20.2 and 5.0 rc2+ hang early on the G5 Quad.
> > > > >
> > > > > Surely I'm not the first to run into this, but I couldn't find any
> > > > > discussion or bug report. Sorry if you're already aware.
> > > > >
> > > > > You can see it hang here (5.0 rc2+, 4.20.2 is nearly identical) until
> > > > > the watchdog triggers a reboot:
> > > > >
> > > > > https://i.imgur.com/UiCVRuG.jpg
> > > > >
> ...
> > I'll be back on monday or tuesday, let me know where you got up to then
> > and I'll take it from there. Also email me your .config please.
>
>
> Hi,
>
> this was caused by 5c63e407aaabb0464236cfc6279a2d79aede7073
> (fbdev: Convert to using %pOFn instead of device_node.name)

See:

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



> name can't be NULL into offb_init_fb, there's a printk and the call to
> offb_init_palette_hacks where name is matched against OF device names.
>
> With the partial revert below, both 5.0-rc2+ and 4.20 are happy.
>
> diff --git a/drivers/video/fbdev/offb.c b/drivers/video/fbdev/offb.c
> index 31f769d67195..77c0a2f45b3b 100644
> --- a/drivers/video/fbdev/offb.c
> +++ b/drivers/video/fbdev/offb.c
> @@ -419,13 +419,9 @@ static void __init offb_init_fb(const char *name,
> var = &info->var;
> info->par = par;
>
> -   if (name) {
> -   strcpy(fix->id, "OFfb ");
> -   strncat(fix->id, name, sizeof(fix->id) - sizeof("OFfb "));
> -   fix->id[sizeof(fix->id) - 1] = '\0';
> -   } else
> -   snprintf(fix->id, sizeof(fix->id), "OFfb %pOFn", dp);
> -
> +   strcpy(fix->id, "OFfb ");
> +   strncat(fix->id, name, sizeof(fix->id) - sizeof("OFfb "));
> +   fix->id[sizeof(fix->id) - 1] = '\0';
>
> var->xres = var->xres_virtual = width;
> var->yres = var->yres_virtual = height;
> @@ -648,7 +644,7 @@ static void __init offb_init_nodriver(struct device_node 
> *dp, int no_real_node)
> /* kludge for valkyrie */
> if (strcmp(dp->name, "valkyrie") == 0)
> address += 0x1000;
> -   offb_init_fb(no_real_node ? "bootx" : NULL,
> +   offb_init_fb(no_real_node ? "bootx" : dp->name,
>  width, height, depth, pitch, address,
>  foreign_endian, no_real_node ? NULL : dp);
> }


[PATCH] powerpc: Remove trailing semicolon after curly brace

2019-02-02 Thread Mathieu Malaterre
There is not point in having a trailing semicolon after a closing curly
brace. Remove it.

Signed-off-by: Mathieu Malaterre 
---
 arch/powerpc/sysdev/tsi108_dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/sysdev/tsi108_dev.c b/arch/powerpc/sysdev/tsi108_dev.c
index 1fd0717ade02..1f1af12f23e2 100644
--- a/arch/powerpc/sysdev/tsi108_dev.c
+++ b/arch/powerpc/sysdev/tsi108_dev.c
@@ -51,7 +51,7 @@ phys_addr_t get_csrbase(void)
const void *prop = of_get_property(tsi, "reg", &size);
tsi108_csr_base = of_translate_address(tsi, prop);
of_node_put(tsi);
-   };
+   }
return tsi108_csr_base;
 }
 
-- 
2.19.2



[PATCH] Move static keyword at beginning of declaration

2019-02-02 Thread Mathieu Malaterre
Move the static keyword around to remove the following warnings (W=1):

  arch/powerpc/platforms/ps3/os-area.c:212:1: error: 'static' is not at 
beginning of declaration [-Werror=old-style-declaration]
  arch/powerpc/platforms/ps3/system-bus.c:45:1: error: 'static' is not at 
beginning of declaration [-Werror=old-style-declaration]

Signed-off-by: Mathieu Malaterre 
---
 arch/powerpc/platforms/ps3/os-area.c| 4 ++--
 arch/powerpc/platforms/ps3/system-bus.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/platforms/ps3/os-area.c 
b/arch/powerpc/platforms/ps3/os-area.c
index f5387ad82279..4d65c5380020 100644
--- a/arch/powerpc/platforms/ps3/os-area.c
+++ b/arch/powerpc/platforms/ps3/os-area.c
@@ -205,11 +205,11 @@ static const struct os_area_db_id os_area_db_id_rtc_diff 
= {
  *  3) The number of seconds from 1970 to 2000.
  */
 
-struct saved_params {
+static struct saved_params {
unsigned int valid;
s64 rtc_diff;
unsigned int av_multi_out;
-} static saved_params;
+} saved_params;
 
 static struct property property_rtc_diff = {
.name = "linux,rtc_diff",
diff --git a/arch/powerpc/platforms/ps3/system-bus.c 
b/arch/powerpc/platforms/ps3/system-bus.c
index 5cc35d6b94b6..7c227e784247 100644
--- a/arch/powerpc/platforms/ps3/system-bus.c
+++ b/arch/powerpc/platforms/ps3/system-bus.c
@@ -37,12 +37,12 @@ static struct device ps3_system_bus = {
 };
 
 /* FIXME: need device usage counters! */
-struct {
+static struct {
struct mutex mutex;
int sb_11; /* usb 0 */
int sb_12; /* usb 0 */
int gpu;
-} static usage_hack;
+} usage_hack;
 
 static int ps3_is_device(struct ps3_system_bus_device *dev, u64 bus_id,
 u64 dev_id)
-- 
2.19.2



Re: [PATCH 1/2] powerpc/32: Add ppc_defconfig

2019-02-07 Thread Mathieu Malaterre
On Thu, Feb 7, 2019 at 6:18 AM Michael Ellerman  wrote:
>
> Add a generic 32-bit defconfig called ppc_defconfig. This means we'll
> have a defconfig matching "uname -m" for all cases.

Looks good to me:

$ make defconfig
*** Default configuration is based on 'ppc_defconfig'
arch/powerpc/configs/ppc_defconfig:175:warning: symbol value 'm'
invalid for NF_TABLES_INET
arch/powerpc/configs/ppc_defconfig:176:warning: symbol value 'm'
invalid for NF_TABLES_NETDEV
arch/powerpc/configs/ppc_defconfig:304:warning: symbol value 'm'
invalid for NF_TABLES_ARP
arch/powerpc/configs/ppc_defconfig:358:warning: symbol value 'm'
invalid for NF_TABLES_BRIDGE
arch/powerpc/configs/ppc_defconfig:1427:warning: symbol value 'm'
invalid for LIRC
#
# configuration written to .config
#

Tested-by: Mathieu Malaterre 

> This config is mostly intended for build testing but if someone wants
> to tweak it to get it booting on something that would be fine too.
>
> Signed-off-by: Michael Ellerman 
> ---
>  arch/powerpc/Makefile | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index ac033341ed55..70e6e8119aeb 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -367,6 +367,10 @@ PHONY += ppc32_allmodconfig
> $(Q)$(MAKE) 
> KCONFIG_ALLCONFIG=$(srctree)/arch/powerpc/configs/book3s_32.config \
> -f $(srctree)/Makefile allmodconfig
>
> +PHONY += ppc_defconfig
> +ppc_defconfig:
> +   $(call merge_into_defconfig,book3s_32.config,)
> +
>  PHONY += ppc64le_allmodconfig
>  ppc64le_allmodconfig:
> $(Q)$(MAKE) 
> KCONFIG_ALLCONFIG=$(srctree)/arch/powerpc/configs/le.config \
> --
> 2.20.1
>


Re: [PATCH 2/2] powerpc: Fix defconfig choice logic when cross compiling

2019-02-07 Thread Mathieu Malaterre
On Thu, Feb 7, 2019 at 6:20 AM Michael Ellerman  wrote:
>
> Our logic for choosing defconfig doesn't work well in some situations.
>
> For example if you're on a ppc64le machine but you specify a non-empty
> CROSS_COMPILE, in order to use a non-default toolchain, then defconfig
> will give you ppc64_defconfig (big endian):
>
>   $ make CROSS_COMPILE=~/toolchains/gcc-8/bin/powerpc-linux- defconfig
>   *** Default configuration is based on 'ppc64_defconfig'
>
> This is because we assume that CROSS_COMPILE being set means we
> can't be on a ppc machine and rather than checking we just default to
> ppc64_defconfig.
>
> We should just ignore CROSS_COMPILE, instead check the machine with
> uname and if it's one of ppc, ppc64 or ppc64le then use that
> defconfig. If it's none of those then we fall back to ppc64_defconfig.

How about shamelessly copying x86:

diff --git a/Makefile b/Makefile
index 3142e67d03f1..041616742142 100644
--- a/Makefile
+++ b/Makefile
@@ -351,6 +351,14 @@ ifeq ($(ARCH),sparc64)
SRCARCH := sparc
 endif

+# Additional ARCH settings for ppc
+ifeq ($(ARCH),ppc64)
+   SRCARCH := powerpc
+endif
+ifeq ($(ARCH),ppc64el)
+   SRCARCH := powerpc
+endif
+
 # Additional ARCH settings for sh
 ifeq ($(ARCH),sh64)
SRCARCH := sh
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 10bf5dc7cdf0..cbb679fb7162 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -34,10 +34,17 @@ ifdef CONFIG_PPC_BOOK3S_32
 KBUILD_CFLAGS  += -mcpu=powerpc
 endif

-# If we're on a ppc/ppc64/ppc64le machine use that defconfig,
otherwise just use
-# ppc64_defconfig because we have nothing better to go on.
-uname := $(shell uname -m)
-KBUILD_DEFCONFIG := $(if $(filter ppc%,$(uname)),$(uname),ppc64)_defconfig
+# select defconfig based on actual architecture
+ifeq ($(ARCH),powerpc)
+  ifeq ($(shell uname -m),ppc64)
+KBUILD_DEFCONFIG := ppc64_defconfig
+  else
+KBUILD_DEFCONFIG := ppc_defconfig
+  endif
+else
+KBUILD_DEFCONFIG := $(ARCH)_defconfig
+endif
+

 ifdef CONFIG_PPC64
 new_nm := $(shell if $(NM) --help 2>&1 | grep -- '--synthetic' >
/dev/null; then echo y; else echo n; fi)


> Signed-off-by: Michael Ellerman 
> ---
>  arch/powerpc/Makefile | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index 70e6e8119aeb..81563986a30e 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -34,11 +34,10 @@ ifdef CONFIG_PPC_BOOK3S_32
>  KBUILD_CFLAGS  += -mcpu=powerpc
>  endif
>
> -ifeq ($(CROSS_COMPILE),)
> -KBUILD_DEFCONFIG := $(shell uname -m)_defconfig
> -else
> -KBUILD_DEFCONFIG := ppc64_defconfig
> -endif
> +# If we're on a ppc/ppc64/ppc64le machine use that defconfig, otherwise just 
> use
> +# ppc64_defconfig because we have nothing better to go on.
> +uname := $(shell uname -m)
> +KBUILD_DEFCONFIG := $(if $(filter ppc%,$(uname)),$(uname),ppc64)_defconfig
>
>  ifdef CONFIG_PPC64
>  new_nm := $(shell if $(NM) --help 2>&1 | grep -- '--synthetic' > /dev/null; 
> then echo y; else echo n; fi)
> --
> 2.20.1
>


Re: [PATCH] powerpc/ptrace: Simplify vr_get/set() to avoid GCC warning

2019-02-15 Thread Mathieu Malaterre
On Fri, Feb 15, 2019 at 7:14 AM Michael Ellerman  wrote:
>
> GCC 8 warns about the logic in vr_get/set(), which with -Werror breaks
> the build:
>
>   In function ‘user_regset_copyin’,
>   inlined from ‘vr_set’ at arch/powerpc/kernel/ptrace.c:628:9:
>   include/linux/regset.h:295:4: error: ‘memcpy’ offset [-527, -529] is
>   out of the bounds [0, 16] of object ‘vrsave’ with type ‘union
>   ’ [-Werror=array-bounds]
>   arch/powerpc/kernel/ptrace.c: In function ‘vr_set’:
>   arch/powerpc/kernel/ptrace.c:623:5: note: ‘vrsave’ declared here
>  } vrsave;
>
> This has been identified as a regression in GCC, see GCC bug 88273.

Good point, this does not seems this will be backported.

> However we can avoid the warning and also simplify the logic and make
> it more robust.
>
> Currently we pass -1 as end_pos to user_regset_copyout(). This says
> "copy up to the end of the regset".
>
> The definition of the regset is:
> [REGSET_VMX] = {
> .core_note_type = NT_PPC_VMX, .n = 34,
> .size = sizeof(vector128), .align = sizeof(vector128),
> .active = vr_active, .get = vr_get, .set = vr_set
> },
>
> The end is calculated as (n * size), ie. 34 * sizeof(vector128).
>
> In vr_get/set() we pass start_pos as 33 * sizeof(vector128), meaning
> we can copy up to sizeof(vector128) into/out-of vrsave.
>
> The on-stack vrsave is defined as:
>   union {
>   elf_vrreg_t reg;
>   u32 word;
>   } vrsave;
>
> And elf_vrreg_t is:
>   typedef __vector128 elf_vrreg_t;
>
> So there is no bug, but we rely on all those sizes lining up,
> otherwise we would have a kernel stack exposure/overwrite on our
> hands.
>
> Rather than relying on that we can pass an explict end_pos based on
> the sizeof(vrsave). The result should be exactly the same but it's
> more obviously not over-reading/writing the stack and it avoids the
> compiler warning.
>

maybe:

Link: https://lkml.org/lkml/2018/8/16/117

In any case the warning is now gone:

Tested-by: Mathieu Malaterre 

> Reported-by: Meelis Roos 
> Reported-by: Mathieu Malaterre 
> Cc: sta...@vger.kernel.org
> Signed-off-by: Michael Ellerman 
> ---
>  arch/powerpc/kernel/ptrace.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index 7535f89e08cd..d9ac7d94656e 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -567,6 +567,7 @@ static int vr_get(struct task_struct *target, const 
> struct user_regset *regset,
> /*
>  * Copy out only the low-order word of vrsave.
>  */
> +   int start, end;
> union {
> elf_vrreg_t reg;
> u32 word;
> @@ -575,8 +576,10 @@ static int vr_get(struct task_struct *target, const 
> struct user_regset *regset,
>
> vrsave.word = target->thread.vrsave;
>
> +   start = 33 * sizeof(vector128);
> +   end = start + sizeof(vrsave);
> ret = user_regset_copyout(&pos, &count, &kbuf, &ubuf, &vrsave,
> - 33 * sizeof(vector128), -1);
> + start, end);
> }
>
> return ret;
> @@ -614,6 +617,7 @@ static int vr_set(struct task_struct *target, const 
> struct user_regset *regset,
> /*
>  * We use only the first word of vrsave.
>  */
> +   int start, end;
> union {
> elf_vrreg_t reg;
> u32 word;
> @@ -622,8 +626,10 @@ static int vr_set(struct task_struct *target, const 
> struct user_regset *regset,
>
> vrsave.word = target->thread.vrsave;
>
> +   start = 33 * sizeof(vector128);
> +   end = start + sizeof(vrsave);
> ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &vrsave,
> -33 * sizeof(vector128), -1);
> +start, end);
> if (!ret)
> target->thread.vrsave = vrsave.word;
> }
> --
> 2.20.1
>


Re: [PATCH] powerpc/ptrace: Add prototype for function pt_regs_check

2019-02-15 Thread Mathieu Malaterre
On Sat, Dec 8, 2018 at 4:46 PM Mathieu Malaterre  wrote:
>
> `pt_regs_check` is a dummy function, its purpose is to break the build
> if struct pt_regs and struct user_pt_regs don't match.
>
> This function has no functionnal purpose, and will get eliminated at
> link time or after init depending on CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
>
> This commit adds a prototype to fix warning at W=1:
>
>   arch/powerpc/kernel/ptrace.c:3339:13: error: no previous prototype for 
> ‘pt_regs_check’ [-Werror=missing-prototypes]
>
> Suggested-by: Christophe Leroy 
> Signed-off-by: Mathieu Malaterre 
> ---
>  arch/powerpc/kernel/ptrace.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index a398999d0770..341c0060b4c8 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -3338,6 +3338,10 @@ void do_syscall_trace_leave(struct pt_regs *regs)
> user_enter();
>  }
>
> +void __init pt_regs_check(void);
> +/* dummy function, its purpose is to break the build if struct pt_regs and
> + * struct user_pt_regs don't match.
> + */

Another trick which seems to work with GCC is:

-void __init pt_regs_check(void)
+static inline void __init pt_regs_check(void)

>  void __init pt_regs_check(void)
>  {
> BUILD_BUG_ON(offsetof(struct pt_regs, gpr) !=
> --
> 2.19.2
>


Re: [PATCH] powerpc/ptrace: Add prototype for function pt_regs_check

2019-02-15 Thread Mathieu Malaterre
On Fri, Feb 15, 2019 at 9:21 AM Christophe Leroy
 wrote:
>
>
>
> Le 15/02/2019 à 09:11, Mathieu Malaterre a écrit :
> > On Sat, Dec 8, 2018 at 4:46 PM Mathieu Malaterre  wrote:
> >>
> >> `pt_regs_check` is a dummy function, its purpose is to break the build
> >> if struct pt_regs and struct user_pt_regs don't match.
> >>
> >> This function has no functionnal purpose, and will get eliminated at
> >> link time or after init depending on CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
> >>
> >> This commit adds a prototype to fix warning at W=1:
> >>
> >>arch/powerpc/kernel/ptrace.c:3339:13: error: no previous prototype for 
> >> ‘pt_regs_check’ [-Werror=missing-prototypes]
> >>
> >> Suggested-by: Christophe Leroy 
> >> Signed-off-by: Mathieu Malaterre 
> >> ---
> >>   arch/powerpc/kernel/ptrace.c | 4 
> >>   1 file changed, 4 insertions(+)
> >>
> >> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> >> index a398999d0770..341c0060b4c8 100644
> >> --- a/arch/powerpc/kernel/ptrace.c
> >> +++ b/arch/powerpc/kernel/ptrace.c
> >> @@ -3338,6 +3338,10 @@ void do_syscall_trace_leave(struct pt_regs *regs)
> >>  user_enter();
> >>   }
> >>
> >> +void __init pt_regs_check(void);
> >> +/* dummy function, its purpose is to break the build if struct pt_regs and
> >> + * struct user_pt_regs don't match.
> >> + */
> >
> > Another trick which seems to work with GCC is:
> >
> > -void __init pt_regs_check(void)
> > +static inline void __init pt_regs_check(void)
>
> Does this really work ? Did you test to ensure that the BUILD_BUG_ON
> still detect mismatch between struct pt_regs and struct user_pt_regs ?
>

My bad, I was unaware of GCC behavior for static inline in this case.
Sorry for the noise.
Original ugly patch does work though.
>
> >
> >>   void __init pt_regs_check(void)
> >>   {
> >>  BUILD_BUG_ON(offsetof(struct pt_regs, gpr) !=
> >> --
> >> 2.19.2
> >>


Re: [PATCH] powerpc: use $(origin ARCH) to select KBUILD_DEFCONFIG

2019-02-15 Thread Mathieu Malaterre
On Fri, Feb 15, 2019 at 10:41 AM Masahiro Yamada
 wrote:
>
> I often test all Kconfig commands for all architectures. To ease my
> workflow, I want 'make defconfig' at least working without any cross
> compiler.
>
> Currently, arch/powerpc/Makefile checks CROSS_COMPILE to decide the
> default defconfig source.
>
> If CROSS_COMPILE is unset, it is likely to be the native build, so
> 'uname -m' is useful to choose the defconfig. If CROSS_COMPILE is set,
> the user is cross-building (i.e. 'uname -m' is probably x86_64), so
> it falls back to ppc64_defconfig. Yup, make sense.
>
> However, I want to run 'make ARCH=* defconfig' without setting
> CROSS_COMPILE for each architecture.
>
> My suggestion is to check $(origin ARCH).
>
> When you cross-compile the kernel, you need to set ARCH from your
> environment or from the command line.
>
> For the native build, you do not need to set ARCH. The default in
> the top Makefile is used:
>
>   ARCH?= $(SUBARCH)
>
> Hence, $(origin ARCH) returns 'file'.
>
> Before this commit, 'make ARCH=powerpc defconfig' failed:

In case you have not seen it, please check:

http://patchwork.ozlabs.org/patch/1037835/

>   $ make ARCH=powerpc defconfig
>   *** Default configuration is based on target 'x86_64_defconfig'
>   ***
>   *** Can't find default configuration 
> "arch/powerpc/configs/x86_64_defconfig"!
>   ***
>
> After this commit, it will succeed:
>
>   $ make ARCH=powerpc defconfig
>   *** Default configuration is based on 'ppc64_defconfig'
>   #
>   # configuration written to .config
>   #
>
> Signed-off-by: Masahiro Yamada 
> ---
>
>  arch/powerpc/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index ac03334..f989979 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -34,7 +34,7 @@ ifdef CONFIG_PPC_BOOK3S_32
>  KBUILD_CFLAGS  += -mcpu=powerpc
>  endif
>
> -ifeq ($(CROSS_COMPILE),)
> +ifeq ($(origin ARCH), file)
>  KBUILD_DEFCONFIG := $(shell uname -m)_defconfig
>  else
>  KBUILD_DEFCONFIG := ppc64_defconfig
> --
> 2.7.4
>


ptrace_syscall_dropped.c:298:TRACE_syscall.ptrace_syscall_dropped:Expected 1 (1) == syscall(286) (4294967295)

2018-05-26 Thread Mathieu Malaterre
Hi,

On Tue, May 8, 2018 at 4:34 PM, Michael Ellerman  wrote:
> Mathieu Malaterre  writes:
>
>> Hi there,
>>
>> Quick question (I have not investigate root cause): is support for
>> seccomp complete on ppc32 ?
>
> Doesn't look like it does it :)
>
>> $ make KBUILD_OUTPUT=/tmp/kselftest TARGETS=seccomp kselftest
>> ...
>> seccomp_bpf.c:1804:TRACE_syscall.ptrace_syscall_dropped:Expected 1 (1)
>> == syscall(286) (4294967295)
>> TRACE_syscall.ptrace_syscall_dropped: Test failed at step #13
>> [ FAIL ] TRACE_syscall.ptrace_syscall_dropped
>> ...
>> [ RUN  ] global.get_metadata
>> seccomp_bpf.c:2880:global.get_metadata:Expected 0 (0) == seccomp(1, 2,
>> &prog) (4294967295)
>> seccomp_bpf.c:2892:global.get_metadata:Expected 1 (1) ==
>> read(pipefd[0], &buf, 1) (0)
>> global.get_metadata: Test terminated by assertion
>> [ FAIL ] global.get_metadata
>
> I'm not sure sorry.
>
> That could be a test case bug, hard to say without looking at the
> details.

I've reduced the test case to the attached file. Does that help, or
should I reduce it some more ?

$ gcc -m32 -o ptrace_syscall_dropped ptrace_syscall_dropped.c

running it as root:

# ./ptrace_syscall_dropped
[==] Running 1 tests from 1 test cases.
[ RUN  ] TRACE_syscall.ptrace_syscall_dropped
ptrace_syscall_dropped.c:298:TRACE_syscall.ptrace_syscall_dropped:Expected
1 (1) == syscall(286) (4294967295)
TRACE_syscall.ptrace_syscall_dropped: Test failed at step #13
[ FAIL ] TRACE_syscall.ptrace_syscall_dropped
[==] 0 / 1 tests passed.
[  FAILED  ]


Thanks
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 

#define _GNU_SOURCE
#include 
#include 

#include "tools/testing/selftests/kselftest_harness.h"

#define IS_SECCOMP_EVENT(status) ((status >> 16) == PTRACE_EVENT_SECCOMP)
static bool tracer_running;
static void tracer_stop(int sig)
{
	tracer_running = false;
}

typedef void tracer_func_t(struct __test_metadata *_metadata,
			   pid_t tracee, int status, void *args);

static void start_tracer(struct __test_metadata *_metadata, int fd, pid_t tracee,
	tracer_func_t tracer_func, void *args, bool ptrace_syscall)
{
	int ret = -1;
	struct sigaction action = {
		.sa_handler = tracer_stop,
	};

	/* Allow external shutdown. */
	tracer_running = true;
	ASSERT_EQ(0, sigaction(SIGUSR1, &action, NULL));

	errno = 0;
	while (ret == -1 && errno != EINVAL)
		ret = ptrace(PTRACE_ATTACH, tracee, NULL, 0);
	ASSERT_EQ(0, ret) {
		kill(tracee, SIGKILL);
	}
	/* Wait for attach stop */
	wait(NULL);

	ret = ptrace(PTRACE_SETOPTIONS, tracee, NULL, ptrace_syscall ?
		  PTRACE_O_TRACESYSGOOD :
		  PTRACE_O_TRACESECCOMP);
	ASSERT_EQ(0, ret) {
		TH_LOG("Failed to set PTRACE_O_TRACESECCOMP");
		kill(tracee, SIGKILL);
	}
	ret = ptrace(ptrace_syscall ? PTRACE_SYSCALL : PTRACE_CONT,
		 tracee, NULL, 0);
	ASSERT_EQ(0, ret);

	/* Unblock the tracee */
	ASSERT_EQ(1, write(fd, "A", 1));
	ASSERT_EQ(0, close(fd));

	/* Run until we're shut down. Must assert to stop execution. */
	while (tracer_running) {
		int status;

		if (wait(&status) != tracee)
			continue;
		if (WIFSIGNALED(status) || WIFEXITED(status))
			/* Child is dead. Time to go. */
			return;

		/* Check if this is a seccomp event. */
		ASSERT_EQ(!ptrace_syscall, IS_SECCOMP_EVENT(status));

		tracer_func(_metadata, tracee, status, args);

		ret = ptrace(ptrace_syscall ? PTRACE_SYSCALL : PTRACE_CONT,
			 tracee, NULL, 0);
		ASSERT_EQ(0, ret);
	}
	/* Directly report the status of our test harness results. */
	syscall(__NR_exit, _metadata->passed ? EXIT_SUCCESS : EXIT_FAILURE);
}

/* Common tracer setup/teardown functions. */
static void cont_handler(int num)
{ }
static pid_t setup_trace_fixture(struct __test_metadata *_metadata,
			  tracer_func_t func, void *args, bool ptrace_syscall)
{
	char sync;
	int pipefd[2];
	pid_t tracer_pid;
	pid_t tracee = getpid();

	/* Setup a pipe for clean synchronization. */
	ASSERT_EQ(0, pipe(pipefd));

	/* Fork a child which we'll promote to tracer */
	tracer_pid = fork();
	ASSERT_LE(0, tracer_pid);
	signal(SIGALRM, cont_handler);
	if (tracer_pid == 0) {
		close(pipefd[0]);
		start_tracer(_metadata, pipefd[1], tracee, func, args,
			 ptrace_syscall);
		syscall(__NR_exit, 0);
	}
	close(pipefd[1]);
	prctl(PR_SET_PTRACER, tracer_pid, 0, 0, 0);
	read(pipefd[0], &sync, 1);
	close(pipefd[0]);

	return tracer_pid;
}
static void teardown_trace_fixture(struct __test_metadata *_metadata,
			pid_t tracer)
{
	if (tracer) {
		int status;
		/*
		 * Extract the exit code from the other process and
		 * adopt it for ourselves in case its asserts failed.
		 */
		ASSERT_EQ(0, kill(tracer, SIGUSR1));
		ASSERT_EQ(tracer, waitpid(tracer, &status, 0));
		if (WEXITSTATUS(status))
			_metadata-

Re: [PATCH] powerpc/Makefile: fix build failure by disabling attribute-alias warning

2018-05-28 Thread Mathieu Malaterre
On Mon, May 28, 2018 at 5:07 PM, Segher Boessenkool
 wrote:
> On Mon, May 28, 2018 at 02:39:37PM +, 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


Re: [PATCH][RFC] [powerpc] arch_ptrace() uses of access_ok() are pointless

2018-05-28 Thread Mathieu Malaterre
On Mon, May 28, 2018 at 12:34 AM, Al Viro  wrote:
> make it use copy_{from,to}_user(), rather than access_ok() +
> __copy_...
>
> Signed-off-by: Al Viro 
> ---
>  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 ‘arch_ptrace’:
../arch/powerpc/kernel/ptrace.c:3086:4: error: expected ‘)’ before ‘return’
return -EFAULT;
^~

Missing closing parenthesis.

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


Re: linux-next: boot messages from today's linux-next

2018-05-29 Thread Mathieu Malaterre
On Tue, May 29, 2018 at 1:02 PM, Stephen Rothwell  wrote:
> Hi all,
>
> My qemu boots of today's powerpc linux-next kernel produced the following
> boot message differences:
>
> -Max number of cores passed to firmware: 2048 (NR_CPUS = 2048)
> +Max number of cores passed to firmware: u (NR_CPUS = 2048)
>
> Seemingly caused by commit
>
>   eae5f709a4d7 ("powerpc: Add __printf verification to prom_printf")

My bad. Patch is pending (thanks Michael!):

https://patchwork.ozlabs.org/patch/921948/

> --
> Cheers,
> Stephen Rothwell


Re: [PATCH] powerpc/prom: Fix %llx usage since prom_printf() change

2018-05-29 Thread Mathieu Malaterre
On Tue, May 29, 2018 at 12:15 PM, Michael Ellerman  wrote:
> We recently added the __printf attribute to prom_printf(), which means
> GCC started warning about type/format mismatches. As part of that
> commit we changed some "%lx" formats to "%llx" where the type is
> actually unsigned long long.
>
> Unfortunately prom_printf() doesn't know how to print "%llx", it just
> prints a literal "lx", eg:
>
>   reserved memory map:
> lx - lx
> lx - lx

Sorry about that. Patch looks good, will try to work on handling llx/llu.

Reviewed-by: Mathieu Malaterre 

> We should fix that at some point, but for now just cast the relevant
> values to unsigned long to get things printing again. On 64-bit that
> has no effect on the output, because both types are 64-bit wide. On
> 32-bit it means we're potentially not printing the high 32-bits of
> some values, but most of them are pointers anyway, and we've lived
> with that behaviour up until now.
>
> Fixes: eae5f709a4d7 ("powerpc: Add __printf verification to prom_printf")
> Signed-off-by: Michael Ellerman 
> ---
>  arch/powerpc/kernel/prom_init.c | 22 +++---
>  1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index 425992e393bc..662dd68c3fb8 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -1580,7 +1580,7 @@ static void __init prom_instantiate_opal(void)
> return;
> }
>
> -   prom_printf("instantiating opal at 0x%llx...", base);
> +   prom_printf("instantiating opal at 0x%lx...", (unsigned long)base);
>
> if (call_prom_ret("call-method", 4, 3, rets,
>   ADDR("load-opal-runtime"),
> @@ -1596,10 +1596,10 @@ static void __init prom_instantiate_opal(void)
>
> reserve_mem(base, size);
>
> -   prom_debug("opal base = 0x%llx\n", base);
> -   prom_debug("opal align= 0x%llx\n", align);
> -   prom_debug("opal entry= 0x%llx\n", entry);
> -   prom_debug("opal size = 0x%llx\n", size);
> +   prom_debug("opal base = 0x%lx\n", (unsigned long)base);
> +   prom_debug("opal align= 0x%lx\n", (unsigned long)align);
> +   prom_debug("opal entry= 0x%lx\n", (unsigned long)entry);
> +   prom_debug("opal size = 0x%lx\n", (unsigned long)size);
>
> prom_setprop(opal_node, "/ibm,opal", "opal-base-address",
>  &base, sizeof(base));
> @@ -1734,7 +1734,7 @@ static void __init prom_instantiate_sml(void)
> if (base == 0)
> prom_panic("Could not allocate memory for sml\n");
>
> -   prom_printf("instantiating sml at 0x%llx...", base);
> +   prom_printf("instantiating sml at 0x%lx...", (unsigned long)base);
>
> memset((void *)base, 0, size);
>
> @@ -1753,7 +1753,7 @@ static void __init prom_instantiate_sml(void)
> prom_setprop(ibmvtpm_node, "/vdevice/vtpm", "linux,sml-size",
>  &size, sizeof(size));
>
> -   prom_debug("sml base = 0x%llx\n", base);
> +   prom_debug("sml base = 0x%lx\n", (unsigned long)base);
> prom_debug("sml size = 0x%x\n", size);
>
> prom_debug("prom_instantiate_sml: end...\n");
> @@ -1847,7 +1847,7 @@ static void __init prom_initialize_tce_table(void)
>
> prom_debug("TCE table: %s\n", path);
> prom_debug("\tnode = 0x%x\n", node);
> -   prom_debug("\tbase = 0x%llx\n", base);
> +   prom_debug("\tbase = 0x%lx\n", (unsigned long)base);
> prom_debug("\tsize = 0x%x\n", minsize);
>
> /* Initialize the table to have a one-to-one mapping
> @@ -2559,9 +2559,9 @@ static void __init flatten_device_tree(void)
> int i;
> prom_printf("reserved memory map:\n");
> for (i = 0; i < mem_reserve_cnt; i++)
> -   prom_printf("  %llx - %llx\n",
> -   be64_to_cpu(mem_reserve_map[i].base),
> -   be64_to_cpu(mem_reserve_map[i].size));
> +   prom_printf("  %lx - %lx\n",
> +   (unsigned 
> long)be64_to_cpu(mem_reserve_map[i].base),
> +   (unsigned 
> long)be64_to_cpu(mem_reserve_map[i].size));
> }
>  #endif
> /* Bump mem_reserve_cnt to cause further reservations to fail
> --
> 2.14.1
>


Re: linux-next: boot messages from today's linux-next

2018-05-29 Thread Mathieu Malaterre
On Tue, May 29, 2018 at 1:56 PM, Mathieu Malaterre  wrote:
> On Tue, May 29, 2018 at 1:02 PM, Stephen Rothwell  
> wrote:
>> Hi all,
>>
>> My qemu boots of today's powerpc linux-next kernel produced the following
>> boot message differences:
>>
>> -Max number of cores passed to firmware: 2048 (NR_CPUS = 2048)
>> +Max number of cores passed to firmware: u (NR_CPUS = 2048)
>>
>> Seemingly caused by commit
>>
>>   eae5f709a4d7 ("powerpc: Add __printf verification to prom_printf")
>
> My bad. Patch is pending (thanks Michael!):
>
> https://patchwork.ozlabs.org/patch/921948/

Nope, nevermind this is unrelated.

>> --
>> Cheers,
>> Stephen Rothwell


Re: linux-next: boot messages from today's linux-next

2018-05-29 Thread Mathieu Malaterre
Michael,


On Tue, May 29, 2018 at 2:00 PM, Mathieu Malaterre  wrote:
> On Tue, May 29, 2018 at 1:56 PM, Mathieu Malaterre  wrote:
>> On Tue, May 29, 2018 at 1:02 PM, Stephen Rothwell  
>> wrote:
>>> Hi all,
>>>
>>> My qemu boots of today's powerpc linux-next kernel produced the following
>>> boot message differences:
>>>
>>> -Max number of cores passed to firmware: 2048 (NR_CPUS = 2048)
>>> +Max number of cores passed to firmware: u (NR_CPUS = 2048)
>>>
>>> Seemingly caused by commit
>>>
>>>   eae5f709a4d7 ("powerpc: Add __printf verification to prom_printf")
>>
>> My bad. Patch is pending (thanks Michael!):
>>
>> https://patchwork.ozlabs.org/patch/921948/
>
> Nope, nevermind this is unrelated.

Could you update your patch "powerpc/prom: Fix %llx usage since
prom_printf() change" to also do "powerpc/prom: Fix %u usage since
prom_printf() change"

I did not pay attention that "%u" was also not supported in prom_printf.

I may be able to provide a patch tonight, if this is not acceptable,
please revert my faulty patch. Sorry for the mess.


>>> --
>>> Cheers,
>>> Stephen Rothwell


Re: linux-next: boot messages from today's linux-next

2018-05-29 Thread Mathieu Malaterre
Stephen,

On Tue, May 29, 2018 at 2:23 PM, Stephen Rothwell  wrote:
> Hi Mathieu,
>
> On Tue, 29 May 2018 13:56:48 +0200 Mathieu Malaterre  wrote:
>>
>> On Tue, May 29, 2018 at 1:02 PM, Stephen Rothwell  
>> wrote:
>> > Hi all,
>> >
>> > My qemu boots of today's powerpc linux-next kernel produced the following
>> > boot message differences:
>> >
>> > -Max number of cores passed to firmware: 2048 (NR_CPUS = 2048)
>> > +Max number of cores passed to firmware: u (NR_CPUS = 2048)
>> >
>> > Seemingly caused by commit
>> >
>> >   eae5f709a4d7 ("powerpc: Add __printf verification to prom_printf")
>>
>> My bad. Patch is pending (thanks Michael!):
>>
>> https://patchwork.ozlabs.org/patch/921948/
>
> Umm, that fixes some other messages, but not this particular one.

Here is what I tried to say:

diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index 4d62f561f272..72ebe5896c1d 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -379,6 +379,11 @@ static void __init prom_printf(const char *format, ...)
}
prom_print_dec(vs);
break;
+   case 'u':
+   ++q;
+   vs = va_arg(args, unsigned long);
+   prom_print_dec(vs);
+   break;
case 'l':
++q;
if (*q == 0)


> --
> Cheers,
> Stephen Rothwell


[PATCH] powerpc/prom: Fix %u/%llx usage since prom_printf() change

2018-05-29 Thread Mathieu Malaterre
In commit eae5f709a4d7 ("powerpc: Add __printf verification to
prom_printf") __printf attribute was added to prom_printf(), which
means GCC started warning about type/format mismatches. As part of that
commit we changed some "%lx" formats to "%llx" where the type is
actually unsigned long long.

Unfortunately prom_printf() doesn't know how to print "%llx", it just
prints a literal "lx", eg:

  reserved memory map:
lx - lx
lx - lx

prom_printf() also doesn't know how to print "%u" (only "%lu"), it just
print a literal "u", eg:

  Max number of cores passed to firmware: u (NR_CPUS = 2048)

instead of:

  Max number of cores passed to firmware: 2048 (NR_CPUS = 2048)

This commit adds support for the missing formatters.

Fixes: eae5f709a4d7 ("powerpc: Add __printf verification to prom_printf")
Reported-by: Michael Ellerman 
Reported-by: Stephen Rothwell 
Signed-off-by: Mathieu Malaterre 
---
I've reviewed all formatters added in eae5f709a4d7 and only %u and %llx were
actually missing (eg. llu or lld are not used)

 arch/powerpc/kernel/prom_init.c | 72 +++--
 1 file changed, 48 insertions(+), 24 deletions(-)

diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index 4d62f561f272..2c04516fe274 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -301,6 +301,9 @@ static void __init prom_print(const char *msg)
 }
 
 
+/* both prom_print_hex & prom_print_dec takes an unsigned long as input so that
+ * we do not need __udivdi3 or __umoddi3 on 32bits
+ */
 static void __init prom_print_hex(unsigned long val)
 {
int i, nibbles = sizeof(val)*2;
@@ -341,6 +344,7 @@ static void __init prom_printf(const char *format, ...)
va_list args;
unsigned long v;
long vs;
+   int n = 0;
 
va_start(args, format);
for (p = format; *p != 0; p = q) {
@@ -359,6 +363,10 @@ static void __init prom_printf(const char *format, ...)
++q;
if (*q == 0)
break;
+   while (*q == 'l') {
+   ++q;
+   ++n;
+   }
switch (*q) {
case 's':
++q;
@@ -367,39 +375,55 @@ static void __init prom_printf(const char *format, ...)
break;
case 'x':
++q;
-   v = va_arg(args, unsigned long);
+   switch (n) {
+   case 0:
+   v = va_arg(args, unsigned int);
+   break;
+   case 1:
+   v = va_arg(args, unsigned long);
+   break;
+   case 2:
+   default:
+   v = va_arg(args, unsigned long long);
+   break;
+   }
prom_print_hex(v);
break;
-   case 'd':
+   case 'u':
++q;
-   vs = va_arg(args, int);
-   if (vs < 0) {
-   prom_print("-");
-   vs = -vs;
+   switch (n) {
+   case 0:
+   v = va_arg(args, unsigned int);
+   break;
+   case 1:
+   v = va_arg(args, unsigned long);
+   break;
+   case 2:
+   default:
+   v = va_arg(args, unsigned long long);
+   break;
}
-   prom_print_dec(vs);
+   prom_print_dec(v);
break;
-   case 'l':
+   case 'd':
++q;
-   if (*q == 0)
+   switch (n) {
+   case 0:
+   vs = va_arg(args, int);
break;
-   else if (*q == 'x') {
-   ++q;
-   v = va_arg(args, unsigned long);
-   prom_print_hex(v);
-   } else if (*q == 'u') { /* '%lu' */
-   ++q;
-   v = va_arg(args, unsigned long);
-   prom_print_dec(v);
-   } else if (*q == 'd') { /* %ld */
-   ++q;
+   

Re: [PATCH v5 3/3] powerpc/lib: optimise PPC32 memcmp

2018-05-29 Thread Mathieu Malaterre
On Mon, May 28, 2018 at 12:49 PM, Christophe Leroy
 wrote:
> At the time being, memcmp() compares two chunks of memory
> byte per byte.
>
> This patch optimises the comparison by comparing word by word.
>
> A small benchmark performed on an 8xx comparing two chuncks
> of 512 bytes performed 10 times gives:
>
> Before : 5852274 TB ticks
> After:   1488638 TB ticks
>
> This is almost 4 times faster
>
> Signed-off-by: Christophe Leroy 
> ---
>  arch/powerpc/lib/string_32.S | 37 +++--
>  1 file changed, 27 insertions(+), 10 deletions(-)

Would it possible for you to move the actual code instead to:

./arch/powerpc/lib/memcmp_32.S

This will seat right next to memcmp_64.S implementation.

> diff --git a/arch/powerpc/lib/string_32.S b/arch/powerpc/lib/string_32.S
> index 40a576d56ac7..4fbaa046aa84 100644
> --- a/arch/powerpc/lib/string_32.S
> +++ b/arch/powerpc/lib/string_32.S
> @@ -16,17 +16,34 @@
> .text
>
>  _GLOBAL(memcmp)
> -   cmpwi   cr0, r5, 0
> -   beq-2f
> -   mtctr   r5
> -   addir6,r3,-1
> -   addir4,r4,-1
> -1: lbzur3,1(r6)
> -   lbzur0,1(r4)
> -   subf.   r3,r0,r3
> -   bdnzt   2,1b
> +   srawi.  r7, r5, 2   /* Divide len by 4 */
> +   mr  r6, r3
> +   beq-3f
> +   mtctr   r7
> +   li  r7, 0
> +1: lwzxr3, r6, r7
> +   lwzxr0, r4, r7
> +   addir7, r7, 4
> +   cmplw   cr0, r3, r0
> +   bdnzt   eq, 1b
> +   bne 5f
> +3: andi.   r3, r5, 3
> +   beqlr
> +   cmplwi  cr1, r3, 2
> +   blt-cr1, 4f
> +   lhzxr3, r6, r7
> +   lhzxr0, r4, r7
> +   addir7, r7, 2
> +   subf.   r3, r0, r3
> +   beqlr   cr1
> +   bnelr
> +4: lbzxr3, r6, r7
> +   lbzxr0, r4, r7
> +   subf.   r3, r0, r3
> blr
> -2: li  r3,0
> +5: li  r3, 1
> +   bgtlr
> +   li  r3, -1
> blr
>  EXPORT_SYMBOL(memcmp)
>
> --
> 2.13.3
>


Re: [PATCH] fsl/qe: ucc: copy and paste bug in ucc_get_tdm_sync_shift()

2018-06-04 Thread Mathieu Malaterre
Where did the original go ?

https://patchwork.ozlabs.org/patch/868158/


On Mon, Jun 4, 2018 at 2:02 PM Dan Carpenter  wrote:
>
> There is a copy and paste bug so we accidentally use the RX_ shift when
> we're in TX_ mode.
>
> Fixes: bb8b2062aff3 ("fsl/qe: setup clock source for TDM mode")
> Signed-off-by: Dan Carpenter 
> ---
> Static analysis work.  Not tested.  This affects the success path, so
> we should probably test it.
>
> diff --git a/drivers/soc/fsl/qe/ucc.c b/drivers/soc/fsl/qe/ucc.c
> index c646d8713861..681f7d4b7724 100644
> --- a/drivers/soc/fsl/qe/ucc.c
> +++ b/drivers/soc/fsl/qe/ucc.c
> @@ -626,7 +626,7 @@ static u32 ucc_get_tdm_sync_shift(enum comm_dir mode, u32 
> tdm_num)
>  {
> u32 shift;
>
> -   shift = (mode == COMM_DIR_RX) ? RX_SYNC_SHIFT_BASE : 
> RX_SYNC_SHIFT_BASE;
> +   shift = (mode == COMM_DIR_RX) ? RX_SYNC_SHIFT_BASE : 
> TX_SYNC_SHIFT_BASE;
> shift -= tdm_num * 2;
>
> return shift;


UBSAN: Undefined behaviour in ../include/linux/percpu_counter.h:137:13

2018-06-07 Thread Mathieu Malaterre
Hi there,

I have a reproducible UBSAN appearing in dmesg after a while on my G4
(*). Could anyone suggest a way to diagnose the actual root issue here
(or is it just a false positive) ?

Thanks,

(*)
[41877.514338] 

[41877.514364] UBSAN: Undefined behaviour in
../include/linux/percpu_counter.h:137:13
[41877.514373] signed integer overflow:
[41877.514378] 9223352809007201260 + 41997676517838 cannot be
represented in type 'long long int'
[41877.514389] CPU: 0 PID: 0 Comm: swapper Not tainted 4.17.0+ #54
[41877.514394] Call Trace:
[41877.514411] [dffedd30] [c047a5f8] ubsan_epilogue+0x18/0x4c (unreliable)
[41877.514422] [dffedd40] [c047af98] handle_overflow+0xbc/0xdc
[41877.514437] [dffeddc0] [c043aaa8] cfq_completed_request+0x560/0x1234
[41877.514446] [dffede40] [c03f595c] __blk_put_request+0xb0/0x2dc
[41877.514460] [dffede80] [c05aa41c] scsi_end_request+0x19c/0x344
[41877.514469] [dffedeb0] [c05abba0] scsi_io_completion+0x4b4/0x854
[41877.514482] [dffedf10] [c040604c] blk_done_softirq+0xe4/0x1e0
[41877.514496] [dffedf60] [c07eef84] __do_softirq+0x16c/0x5f0
[41877.514508] [dffedfd0] [c0065160] irq_exit+0x110/0x1a8
[41877.514520] [dffedff0] [c001646c] call_do_irq+0x24/0x3c
[41877.514533] [c0ce5e80] [c0009a2c] do_IRQ+0x98/0x1a0
[41877.514541] [c0ce5eb0] [c001b93c] ret_from_except+0x0/0x14
[41877.514549] --- interrupt: 501 at arch_cpu_idle+0x30/0x78
   LR = arch_cpu_idle+0x30/0x78
[41877.514558] [c0ce5f70] [c0ce4000] 0xc0ce4000 (unreliable)
[41877.514570] [c0ce5f80] [c00a3928] do_idle+0xc4/0x158
[41877.514577] [c0ce5fb0] [c00a3b74] cpu_startup_entry+0x24/0x28
[41877.514585] [c0ce5fc0] [c0988820] start_kernel+0x47c/0x490
[41877.514592] [c0ce5ff0] [3444] 0x3444
[41877.514597] 

[41886.390210] 

[41886.390236] UBSAN: Undefined behaviour in
../include/linux/percpu_counter.h:137:13
[41886.390245] signed integer overflow:
[41886.390250] 9223366156262940402 + 42006563339289 cannot be
represented in type 'long long int'
[41886.390260] CPU: 0 PID: 0 Comm: swapper Not tainted 4.17.0+ #54
[41886.390265] Call Trace:
[41886.390282] [dffedd30] [c047a5f8] ubsan_epilogue+0x18/0x4c (unreliable)
[41886.390293] [dffedd40] [c047af98] handle_overflow+0xbc/0xdc
[41886.390309] [dffeddc0] [c043a8c4] cfq_completed_request+0x37c/0x1234
[41886.390317] [dffede40] [c03f595c] __blk_put_request+0xb0/0x2dc
[41886.390331] [dffede80] [c05aa41c] scsi_end_request+0x19c/0x344
[41886.390340] [dffedeb0] [c05abba0] scsi_io_completion+0x4b4/0x854
[41886.390353] [dffedf10] [c040604c] blk_done_softirq+0xe4/0x1e0
[41886.390367] [dffedf60] [c07eef84] __do_softirq+0x16c/0x5f0
[41886.390379] [dffedfd0] [c0065160] irq_exit+0x110/0x1a8
[41886.390391] [dffedff0] [c001646c] call_do_irq+0x24/0x3c
[41886.390404] [c0ce5e80] [c0009a2c] do_IRQ+0x98/0x1a0
[41886.390411] [c0ce5eb0] [c001b93c] ret_from_except+0x0/0x14
[41886.390420] --- interrupt: 501 at arch_cpu_idle+0x30/0x78
   LR = arch_cpu_idle+0x30/0x78
[41886.390429] [c0ce5f70] [c0ce4000] 0xc0ce4000 (unreliable)
[41886.390441] [c0ce5f80] [c00a3928] do_idle+0xc4/0x158
[41886.390449] [c0ce5fb0] [c00a3b74] cpu_startup_entry+0x24/0x28
[41886.390457] [c0ce5fc0] [c0988820] start_kernel+0x47c/0x490
[41886.390463] [c0ce5ff0] [3444] 0x3444
[41886.390468] 



Re: 4.17.0-10146-gf0dc7f9c6dd9: hw csum failure on powerpc+sungem

2018-06-11 Thread Mathieu Malaterre
Hi Meelis,

On Mon, Jun 11, 2018 at 1:21 PM Meelis Roos  wrote:
>
> I am seeing this on PowerMac G4 with sungem ethernet driver. 4.17 was
> OK, 4.17.0-10146-gf0dc7f9c6dd9 is problematic.

Same here.

> [  140.518664] eth0: hw csum failure
> [  140.518699] CPU: 0 PID: 1237 Comm: postconf Not tainted 
> 4.17.0-10146-gf0dc7f9c6dd9 #83
> [  140.518707] Call Trace:
> [  140.518734] [effefd90] [c03d6db8] __skb_checksum_complete+0xd8/0xdc 
> (unreliable)
> [  140.518759] [effefdb0] [c04c1284] icmpv6_rcv+0x248/0x4ec
> [  140.518775] [effefdd0] [c049a448] ip6_input_finish.constprop.0+0x11c/0x5f4
> [  140.518786] [effefe10] [c049b1c0] ip6_mc_input+0xcc/0x100
> [  140.518807] [effefe20] [c03e110c] __netif_receive_skb_core+0x310/0x944
> [  140.518820] [effefe70] [c03e76ec] napi_gro_receive+0xd0/0xe8
> [  140.518845] [effefe80] [f3e1f66c] gem_poll+0x618/0x1274 [sungem]
> [  140.518856] [effeff30] [c03e6f0c] net_rx_action+0x198/0x374
> [  140.518872] [effeff90] [c0501a88] __do_softirq+0x120/0x278
> [  140.518890] [effeffe0] [c0036188] irq_exit+0xd8/0xdc
> [  140.518908] [effefff0] [c000f478] call_do_irq+0x24/0x3c
> [  140.518925] [d05a5d30] [c0007120] do_IRQ+0x74/0xf0
> [  140.518941] [d05a5d50] [c0012474] ret_from_except+0x0/0x14
> [  140.518960] --- interrupt: 501 at copy_page+0x40/0x90
>LR = copy_user_page+0x18/0x30
> [  140.518973] [d05a5e10] [d058cd80] 0xd058cd80 (unreliable)
> [  140.518989] [d05a5e20] [c00fa2bc] wp_page_copy+0xec/0x654
> [  140.519002] [d05a5e60] [c00fd3a4] do_wp_page+0xa8/0x5b4
> [  140.519013] [d05a5e90] [c00fe934] handle_mm_fault+0x564/0xa84
> [  140.519025] [d05a5f00] [c0016230] do_page_fault+0x1bc/0x7e8
> [  140.519037] [d05a5f40] [c0012300] handle_page_fault+0x14/0x40
> [  140.519048] --- interrupt: 301 at 0xb78b6864
>LR = 0xb78b6c54
>

For some reason if I do a git bisect it returns that:

$ git bisect good
3036bc45364f98515a2c446d7fac2c34dcfbeff4 is the first bad commit

Could you also check on your side please.

> --
> Meelis Roos (mr...@linux.ee)


Re: 4.17.0-10146-gf0dc7f9c6dd9: hw csum failure on powerpc+sungem

2018-06-12 Thread Mathieu Malaterre
Hi Balbir,

On Tue, Jun 12, 2018 at 9:39 AM Balbir Singh  wrote:
>
>
> On 12/06/18 06:20, Mathieu Malaterre wrote:
>
> > Hi Meelis,
> >
> > On Mon, Jun 11, 2018 at 1:21 PM Meelis Roos  wrote:
> >> I am seeing this on PowerMac G4 with sungem ethernet driver. 4.17 was
> >> OK, 4.17.0-10146-gf0dc7f9c6dd9 is problematic.
> > Same here.
> >
> >> [  140.518664] eth0: hw csum failure
> >> [  140.518699] CPU: 0 PID: 1237 Comm: postconf Not tainted 
> >> 4.17.0-10146-gf0dc7f9c6dd9 #83
> >> [  140.518707] Call Trace:
> >> [  140.518734] [effefd90] [c03d6db8] __skb_checksum_complete+0xd8/0xdc 
> >> (unreliable)
> >> [  140.518759] [effefdb0] [c04c1284] icmpv6_rcv+0x248/0x4ec
> >> [  140.518775] [effefdd0] [c049a448] 
> >> ip6_input_finish.constprop.0+0x11c/0x5f4
> >> [  140.518786] [effefe10] [c049b1c0] ip6_mc_input+0xcc/0x100
> >> [  140.518807] [effefe20] [c03e110c] __netif_receive_skb_core+0x310/0x944
> >> [  140.518820] [effefe70] [c03e76ec] napi_gro_receive+0xd0/0xe8
> >> [  140.518845] [effefe80] [f3e1f66c] gem_poll+0x618/0x1274 [sungem]
> >> [  140.518856] [effeff30] [c03e6f0c] net_rx_action+0x198/0x374
> >> [  140.518872] [effeff90] [c0501a88] __do_softirq+0x120/0x278
> >> [  140.518890] [effeffe0] [c0036188] irq_exit+0xd8/0xdc
> >> [  140.518908] [effefff0] [c000f478] call_do_irq+0x24/0x3c
> >> [  140.518925] [d05a5d30] [c0007120] do_IRQ+0x74/0xf0
> >> [  140.518941] [d05a5d50] [c0012474] ret_from_except+0x0/0x14
> >> [  140.518960] --- interrupt: 501 at copy_page+0x40/0x90
> >>LR = copy_user_page+0x18/0x30
> >> [  140.518973] [d05a5e10] [d058cd80] 0xd058cd80 (unreliable)
> >> [  140.518989] [d05a5e20] [c00fa2bc] wp_page_copy+0xec/0x654
> >> [  140.519002] [d05a5e60] [c00fd3a4] do_wp_page+0xa8/0x5b4
> >> [  140.519013] [d05a5e90] [c00fe934] handle_mm_fault+0x564/0xa84
> >> [  140.519025] [d05a5f00] [c0016230] do_page_fault+0x1bc/0x7e8
> >> [  140.519037] [d05a5f40] [c0012300] handle_page_fault+0x14/0x40
> >> [  140.519048] --- interrupt: 301 at 0xb78b6864
> >>LR = 0xb78b6c54
> >>
> > For some reason if I do a git bisect it returns that:
> >
> > $ git bisect good
> > 3036bc45364f98515a2c446d7fac2c34dcfbeff4 is the first bad commit
> >
> > Could you also check on your side please.
>
> Don't have a G4, but the related commits look like
>
> 373e098e1e788d7b89ec0f31765a6c08e2ea0f7c and 
> e9c4943a107b56696e4872cdffdba6b0c7277c77 Balbir
>

Indeed that makes more sense. I must have messed up during my
git-bisect operation. Will try a simple git-revert on those and report
back.

Thanks


Re: 4.17.0-10146-gf0dc7f9c6dd9: hw csum failure on powerpc+sungem

2018-06-13 Thread Mathieu Malaterre
Hi all,

On Tue, Jun 12, 2018 at 10:15 AM Mathieu Malaterre  wrote:
>
> Hi Balbir,
>
> On Tue, Jun 12, 2018 at 9:39 AM Balbir Singh  wrote:
> >
> >
> > On 12/06/18 06:20, Mathieu Malaterre wrote:
> >
> > > Hi Meelis,
> > >
> > > On Mon, Jun 11, 2018 at 1:21 PM Meelis Roos  wrote:
> > >> I am seeing this on PowerMac G4 with sungem ethernet driver. 4.17 was
> > >> OK, 4.17.0-10146-gf0dc7f9c6dd9 is problematic.
> > > Same here.
> > >
> > >> [  140.518664] eth0: hw csum failure
> > >> [  140.518699] CPU: 0 PID: 1237 Comm: postconf Not tainted 
> > >> 4.17.0-10146-gf0dc7f9c6dd9 #83
> > >> [  140.518707] Call Trace:
> > >> [  140.518734] [effefd90] [c03d6db8] __skb_checksum_complete+0xd8/0xdc 
> > >> (unreliable)
> > >> [  140.518759] [effefdb0] [c04c1284] icmpv6_rcv+0x248/0x4ec
> > >> [  140.518775] [effefdd0] [c049a448] 
> > >> ip6_input_finish.constprop.0+0x11c/0x5f4
> > >> [  140.518786] [effefe10] [c049b1c0] ip6_mc_input+0xcc/0x100
> > >> [  140.518807] [effefe20] [c03e110c] __netif_receive_skb_core+0x310/0x944
> > >> [  140.518820] [effefe70] [c03e76ec] napi_gro_receive+0xd0/0xe8
> > >> [  140.518845] [effefe80] [f3e1f66c] gem_poll+0x618/0x1274 [sungem]
> > >> [  140.518856] [effeff30] [c03e6f0c] net_rx_action+0x198/0x374
> > >> [  140.518872] [effeff90] [c0501a88] __do_softirq+0x120/0x278
> > >> [  140.518890] [effeffe0] [c0036188] irq_exit+0xd8/0xdc
> > >> [  140.518908] [effefff0] [c000f478] call_do_irq+0x24/0x3c
> > >> [  140.518925] [d05a5d30] [c0007120] do_IRQ+0x74/0xf0
> > >> [  140.518941] [d05a5d50] [c0012474] ret_from_except+0x0/0x14
> > >> [  140.518960] --- interrupt: 501 at copy_page+0x40/0x90
> > >>LR = copy_user_page+0x18/0x30
> > >> [  140.518973] [d05a5e10] [d058cd80] 0xd058cd80 (unreliable)
> > >> [  140.518989] [d05a5e20] [c00fa2bc] wp_page_copy+0xec/0x654
> > >> [  140.519002] [d05a5e60] [c00fd3a4] do_wp_page+0xa8/0x5b4
> > >> [  140.519013] [d05a5e90] [c00fe934] handle_mm_fault+0x564/0xa84
> > >> [  140.519025] [d05a5f00] [c0016230] do_page_fault+0x1bc/0x7e8
> > >> [  140.519037] [d05a5f40] [c0012300] handle_page_fault+0x14/0x40
> > >> [  140.519048] --- interrupt: 301 at 0xb78b6864
> > >>LR = 0xb78b6c54
> > >>
> > > For some reason if I do a git bisect it returns that:
> > >
> > > $ git bisect good
> > > 3036bc45364f98515a2c446d7fac2c34dcfbeff4 is the first bad commit
> > >
> > > Could you also check on your side please.
> >
> > Don't have a G4, but the related commits look like
> >
> > 373e098e1e788d7b89ec0f31765a6c08e2ea0f7c and 
> > e9c4943a107b56696e4872cdffdba6b0c7277c77 Balbir
> >
>
> Indeed that makes more sense. I must have messed up during my
> git-bisect operation. Will try a simple git-revert on those and report
> back.

Here is what I get.

Current git HEAD is: 0c14e43a42e4e44f70963f8ccf89461290c4e4da if I do:

$ git revert e9c4943a107b56696e4872cdffdba6b0c7277c77.
$ git revert 373e098e1e788d7b89ec0f31765a6c08e2ea0f7c
$ rm -rf g4
$ make O=g4 ARCH=powerpc g4_defconfig
$ make -j8 O=g4 ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- bindeb-pkg

-> I can still see the error in dmesg.

> Thanks


Re: UBSAN: Undefined behaviour in ../include/linux/percpu_counter.h:137:13

2018-06-13 Thread Mathieu Malaterre
On Wed, Jun 13, 2018 at 3:43 AM Michael Ellerman  wrote:
>
> Mathieu Malaterre  writes:
>
> > Hi there,
> >
> > I have a reproducible UBSAN appearing in dmesg after a while on my G4
> > (*). Could anyone suggest a way to diagnose the actual root issue here
> > (or is it just a false positive) ?
>
> It looks like a real overflow, I guess the question is why are we seeing it.
>
> The first thing to work out would be what exactly is overflowing.
>
> Is it in here?
>
> cfqg_stats_update_completion(cfqq->cfqg, rq->start_time_ns,
>  rq->io_start_time_ns, rq->cmd_flags);
>
>
> If so that would suggest something is taking multiple hours to complete,
> which seems unlikely. Is time going backward?

There is also something suspicious in the kern.log file:

Jun 12 20:09:04 debian kernel: [5.504182]

Jun 12 20:09:04 debian kernel: [5.508945] UBSAN: Undefined
behaviour in ../drivers/rtc/rtc-lib.c:87:22
Jun 12 20:09:04 debian kernel: [5.513658] signed integer overflow:
Jun 12 20:09:04 debian kernel: [5.518211] 1193024 * 3600 cannot be
represented in type 'int'
Jun 12 20:09:04 debian kernel: [5.522866] CPU: 0 PID: 1 Comm:
swapper Not tainted 4.17.0+ #1
Jun 12 20:09:04 debian kernel: [5.527567] Call Trace:
Jun 12 20:09:04 debian kernel: [5.532200] [df4e7b00] [c0481074]
ubsan_epilogue+0x18/0x4c (unreliable)
Jun 12 20:09:04 debian kernel: [5.537019] [df4e7b10] [c0481a14]
handle_overflow+0xbc/0xdc
Jun 12 20:09:04 debian kernel: [5.541832] [df4e7b90] [c060d698]
rtc_time64_to_tm+0x344/0x388
Jun 12 20:09:04 debian kernel: [5.546655] [df4e7bd0] [c001076c]
rtc_generic_get_time+0x2c/0x40
Jun 12 20:09:04 debian kernel: [5.551477] [df4e7be0] [c06113d4]
__rtc_read_time+0x70/0x13c
Jun 12 20:09:04 debian kernel: [5.556288] [df4e7c00] [c061150c]
rtc_read_time+0x6c/0x130
Jun 12 20:09:04 debian kernel: [5.561088] [df4e7c30] [c061271c]
__rtc_read_alarm+0x34/0x684
Jun 12 20:09:04 debian kernel: [5.565884] [df4e7ce0] [c060f234]
rtc_device_register+0x88/0x218
Jun 12 20:09:04 debian kernel: [5.570695] [df4e7d40] [c060f428]
devm_rtc_device_register+0x64/0xc4
Jun 12 20:09:04 debian kernel: [5.575528] [df4e7d60] [c09d15d4]
generic_rtc_probe+0x50/0x78
Jun 12 20:09:04 debian kernel: [5.580359] [df4e7d70] [c055e4a4]
platform_drv_probe+0xa8/0x128
Jun 12 20:09:04 debian kernel: [5.585210] [df4e7d90] [c0559d28]
driver_probe_device+0x354/0x6fc
Jun 12 20:09:04 debian kernel: [5.590064] [df4e7dd0] [c055a270]
__driver_attach+0x1a0/0x22c
Jun 12 20:09:04 debian kernel: [5.594917] [df4e7df0] [c0555b70]
bus_for_each_dev+0x84/0xdc
Jun 12 20:09:04 debian kernel: [5.599750] [df4e7e20] [c0558420]
bus_add_driver+0x188/0x348
Jun 12 20:09:04 debian kernel: [5.604584] [df4e7e40] [c055b7b4]
driver_register+0xa0/0x18c
Jun 12 20:09:04 debian kernel: [5.609433] [df4e7e50] [c055e950]
__platform_driver_probe+0x8c/0x198
Jun 12 20:09:04 debian kernel: [5.614330] [df4e7e70] [c0005800]
do_one_initcall+0x64/0x280
Jun 12 20:09:04 debian kernel: [5.619237] [df4e7ee0] [c0997c04]
kernel_init_freeable+0x3a4/0x444
Jun 12 20:09:04 debian kernel: [5.624145] [df4e7f30] [c00049f8]
kernel_init+0x24/0x118
Jun 12 20:09:04 debian kernel: [5.629029] [df4e7f40] [c001b1c4]
ret_from_kernel_thread+0x14/0x1c
Jun 12 20:09:04 debian kernel: [5.633878]



Grep-ing all leads to:

$ grep  "cannot be represented" kern.log | colrm 1 45|sort -u
 1193022 * 3600 cannot be represented in type 'int'
 1193024 * 3600 cannot be represented in type 'int'
 1193032 * 3600 cannot be represented in type 'int'
 1193033 * 3600 cannot be represented in type 'int'
 1193034 * 3600 cannot be represented in type 'int'
 1193035 * 3600 cannot be represented in type 'int'

How come tm_hour can store a value of 1193035 ?

> cheers
>
> > (*)
> > [41877.514338] 
> > 
> > [41877.514364] UBSAN: Undefined behaviour in
> > ../include/linux/percpu_counter.h:137:13
> > [41877.514373] signed integer overflow:
> > [41877.514378] 9223352809007201260 + 41997676517838 cannot be
> > represented in type 'long long int'
> > [41877.514389] CPU: 0 PID: 0 Comm: swapper Not tainted 4.17.0+ #54
> > [41877.514394] Call Trace:
> > [41877.514411] [dffedd30] [c047a5f8] ubsan_epilogue+0x18/0x4c (unreliable)
> > [41877.514422] [dffedd40] [c047af98] handle_overflow+0xbc/0xdc
> > [41877.514437] [dffeddc0] [c043aaa8] cfq_completed_request+0x560/0x1234
> > [41877.514446] [dffede40] [c03f595c] __blk_put

Re: UBSAN: Undefined behaviour in ../include/linux/percpu_counter.h:137:13

2018-06-13 Thread Mathieu Malaterre
On Wed, Jun 13, 2018 at 10:43 AM Mathieu Malaterre  wrote:
>
> On Wed, Jun 13, 2018 at 3:43 AM Michael Ellerman  wrote:
> >
> > Mathieu Malaterre  writes:
> >
> > > Hi there,
> > >
> > > I have a reproducible UBSAN appearing in dmesg after a while on my G4
> > > (*). Could anyone suggest a way to diagnose the actual root issue here
> > > (or is it just a false positive) ?
> >
> > It looks like a real overflow, I guess the question is why are we seeing it.
> >
> > The first thing to work out would be what exactly is overflowing.
> >
> > Is it in here?
> >
> > cfqg_stats_update_completion(cfqq->cfqg, rq->start_time_ns,
> >  rq->io_start_time_ns, rq->cmd_flags);
> >
> >
> > If so that would suggest something is taking multiple hours to complete,
> > which seems unlikely. Is time going backward?
>
> There is also something suspicious in the kern.log file:
>
> Jun 12 20:09:04 debian kernel: [5.504182]
> 
> Jun 12 20:09:04 debian kernel: [5.508945] UBSAN: Undefined
> behaviour in ../drivers/rtc/rtc-lib.c:87:22
> Jun 12 20:09:04 debian kernel: [5.513658] signed integer overflow:
> Jun 12 20:09:04 debian kernel: [5.518211] 1193024 * 3600 cannot be
> represented in type 'int'
> Jun 12 20:09:04 debian kernel: [5.522866] CPU: 0 PID: 1 Comm:
> swapper Not tainted 4.17.0+ #1
> Jun 12 20:09:04 debian kernel: [5.527567] Call Trace:
> Jun 12 20:09:04 debian kernel: [5.532200] [df4e7b00] [c0481074]
> ubsan_epilogue+0x18/0x4c (unreliable)
> Jun 12 20:09:04 debian kernel: [5.537019] [df4e7b10] [c0481a14]
> handle_overflow+0xbc/0xdc
> Jun 12 20:09:04 debian kernel: [5.541832] [df4e7b90] [c060d698]
> rtc_time64_to_tm+0x344/0x388
> Jun 12 20:09:04 debian kernel: [5.546655] [df4e7bd0] [c001076c]
> rtc_generic_get_time+0x2c/0x40
> Jun 12 20:09:04 debian kernel: [5.551477] [df4e7be0] [c06113d4]
> __rtc_read_time+0x70/0x13c
> Jun 12 20:09:04 debian kernel: [5.556288] [df4e7c00] [c061150c]
> rtc_read_time+0x6c/0x130
> Jun 12 20:09:04 debian kernel: [5.561088] [df4e7c30] [c061271c]
> __rtc_read_alarm+0x34/0x684
> Jun 12 20:09:04 debian kernel: [5.565884] [df4e7ce0] [c060f234]
> rtc_device_register+0x88/0x218
> Jun 12 20:09:04 debian kernel: [5.570695] [df4e7d40] [c060f428]
> devm_rtc_device_register+0x64/0xc4
> Jun 12 20:09:04 debian kernel: [5.575528] [df4e7d60] [c09d15d4]
> generic_rtc_probe+0x50/0x78
> Jun 12 20:09:04 debian kernel: [5.580359] [df4e7d70] [c055e4a4]
> platform_drv_probe+0xa8/0x128
> Jun 12 20:09:04 debian kernel: [5.585210] [df4e7d90] [c0559d28]
> driver_probe_device+0x354/0x6fc
> Jun 12 20:09:04 debian kernel: [5.590064] [df4e7dd0] [c055a270]
> __driver_attach+0x1a0/0x22c
> Jun 12 20:09:04 debian kernel: [5.594917] [df4e7df0] [c0555b70]
> bus_for_each_dev+0x84/0xdc
> Jun 12 20:09:04 debian kernel: [5.599750] [df4e7e20] [c0558420]
> bus_add_driver+0x188/0x348
> Jun 12 20:09:04 debian kernel: [5.604584] [df4e7e40] [c055b7b4]
> driver_register+0xa0/0x18c
> Jun 12 20:09:04 debian kernel: [5.609433] [df4e7e50] [c055e950]
> __platform_driver_probe+0x8c/0x198
> Jun 12 20:09:04 debian kernel: [5.614330] [df4e7e70] [c0005800]
> do_one_initcall+0x64/0x280
> Jun 12 20:09:04 debian kernel: [5.619237] [df4e7ee0] [c0997c04]
> kernel_init_freeable+0x3a4/0x444
> Jun 12 20:09:04 debian kernel: [5.624145] [df4e7f30] [c00049f8]
> kernel_init+0x24/0x118
> Jun 12 20:09:04 debian kernel: [5.629029] [df4e7f40] [c001b1c4]
> ret_from_kernel_thread+0x14/0x1c
> Jun 12 20:09:04 debian kernel: [5.633878]
> 
>
>
> Grep-ing all leads to:
>
> $ grep  "cannot be represented" kern.log | colrm 1 45|sort -u
>  1193022 * 3600 cannot be represented in type 'int'
>  1193024 * 3600 cannot be represented in type 'int'
>  1193032 * 3600 cannot be represented in type 'int'
>  1193033 * 3600 cannot be represented in type 'int'
>  1193034 * 3600 cannot be represented in type 'int'
>  1193035 * 3600 cannot be represented in type 'int'
>
> How come tm_hour can store a value of 1193035 ?

It appears that I am getting a negative value for  time64_t :

[5.495470] devices_kset: Moving rtc-generic to end of list
[5.502584]  mm DBG: TIME64_T -2068738348
[5.507205] WARNING: CPU: 0 PID: 1 at ../drivers/rtc/rtc-lib.c:60
rtc_time64_to_tm+0x48/0x38c
[5.511843] Modules linked in:
[5.516306] CPU: 0 P

powerpc: use time64_t in read_persistent_clock

2018-06-13 Thread Mathieu Malaterre
Arnd,

In 5bfd643583b2e I can see that you changed:

$ git show 5bfd643583b2e -- arch/powerpc/platforms/powermac/time.c
[...]
 #ifdef CONFIG_ADB_PMU
-static unsigned long pmu_get_time(void)
+static time64_t pmu_get_time(void)
 {
struct adb_request req;
-   unsigned int now;
+   time64_t now;

if (pmu_request(&req, NULL, 1, PMU_READ_RTC) < 0)
return 0;
@@ -160,10 +151,10 @@ static unsigned long pmu_get_time(void)
   req.reply_len);
now = (req.reply[0] << 24) + (req.reply[1] << 16)
+ (req.reply[2] << 8) + req.reply[3];
-   return ((unsigned long)now) - RTC_OFFSET;
+   return now - RTC_OFFSET;
 }
[...]

As far as I can tell the old function would never return a negative
value and rely on unsigned long roll over. Now I am seeing negative
value being returned and it seems to break later on in the rtc
library.

Could you comment why you removed the cast to (unsigned long) in your commit ?

Thanks


Re: powerpc: use time64_t in read_persistent_clock

2018-06-14 Thread Mathieu Malaterre
On Thu, Jun 14, 2018 at 1:46 PM Arnd Bergmann  wrote:
>
> On Wed, Jun 13, 2018 at 10:24 PM, Mathieu Malaterre  wrote:
> > Arnd,
> >
> > In 5bfd643583b2e I can see that you changed:
> >
> > $ git show 5bfd643583b2e -- arch/powerpc/platforms/powermac/time.c
> > [...]
> >  #ifdef CONFIG_ADB_PMU
> > -static unsigned long pmu_get_time(void)
> > +static time64_t pmu_get_time(void)
> >  {
> > struct adb_request req;
> > -   unsigned int now;
> > +   time64_t now;
> >
> > if (pmu_request(&req, NULL, 1, PMU_READ_RTC) < 0)
> > return 0;
> > @@ -160,10 +151,10 @@ static unsigned long pmu_get_time(void)
> >req.reply_len);
> > now = (req.reply[0] << 24) + (req.reply[1] << 16)
> > + (req.reply[2] << 8) + req.reply[3];
> > -   return ((unsigned long)now) - RTC_OFFSET;
> > +   return now - RTC_OFFSET;
> >  }
> > [...]
> >
> > As far as I can tell the old function would never return a negative
> > value and rely on unsigned long roll over. Now I am seeing negative
> > value being returned and it seems to break later on in the rtc
> > library.
> >
> > Could you comment why you removed the cast to (unsigned long) in your 
> > commit ?
>
> I'm sorry my patch caused a regression. Even with your bug report
> it took me a while to see what exactly went wrong, and how I got
> mixed up the integer conversion rules.
>
> I mistakenly assume that fiven
>
>long long now;
>unsigned char reply[4];
>now = reply[0] << 24;
>
> we would always end up with a positive number, but since reply[0] is
> promoted to a signed integer, the right-hand side of the assignment
> ends up as a negative number for the usual contents of the RTC.

Ah right. My diagnosis was invalid.

> Can you confirm that this patch addresses your problem?

Yes !

Before:
[5.986710] rtc-generic rtc-generic: hctosys: unable to read the
hardware clock

After:
[5.579611] rtc-generic rtc-generic: setting system clock to
2018-06-14 18:57:00 UTC (1529002620)

So for the above:

Tested-by: Mathieu Malaterre 

> diff --git a/arch/powerpc/platforms/powermac/time.c
> b/arch/powerpc/platforms/powermac/time.c
> index 7c968e46736f..a7fdcd3051f9 100644
> --- a/arch/powerpc/platforms/powermac/time.c
> +++ b/arch/powerpc/platforms/powermac/time.c
> @@ -97,7 +97,7 @@ static time64_t cuda_get_time(void)
> if (req.reply_len != 7)
> printk(KERN_ERR "cuda_get_time: got %d byte reply\n",
>req.reply_len);
> -   now = (req.reply[3] << 24) + (req.reply[4] << 16)
> +   now = (u32)(req.reply[3] << 24) + (req.reply[4] << 16)
> + (req.reply[5] << 8) + req.reply[6];
> return now - RTC_OFFSET;
>  }
> @@ -140,7 +140,7 @@ static time64_t pmu_get_time(void)
> if (req.reply_len != 4)
> printk(KERN_ERR "pmu_get_time: got %d byte reply from PMU\n",
>req.reply_len);
> -   now = (req.reply[0] << 24) + (req.reply[1] << 16)
> +   now = (u32)(req.reply[0] << 24) + (req.reply[1] << 16)
> + (req.reply[2] << 8) + req.reply[3];
> return now - RTC_OFFSET;
>  }
>
> On a related note, we do have some freedom in how we want to
> interpret values beyond year 2038/2040 when the RTC does wrap
> around.
>
> - With the original code, we converted the time into a signed
>   32-bit integer based on the 1970 Unix epoch, converting RTC
>   years 2038 through 2040 into Linux years 1902 through 1904,
>   which doesn't seem very helpful.
>
> - With my fix above, we would interpret the numbers as documented,
>   with 2040 wrapping around to 1904.
>
> - If we want, we could reinterpret the 1904-1970 range as later
>   times between 2040 and 2106, like this:
>
> --- a/arch/powerpc/platforms/powermac/time.c
> +++ b/arch/powerpc/platforms/powermac/time.c
> @@ -88,7 +88,7 @@ long __init pmac_time_init(void)
>  static time64_t cuda_get_time(void)
>  {
> struct adb_request req;
> -   time64_t now;
> +   u32 now;
>
> if (cuda_request(&req, NULL, 2, CUDA_PACKET, CUDA_GET_TIME) < 0)
> return 0;
> @@ -132,7 +132,7 @@ static int cuda_set_rtc_time(struct rtc_time *tm)
>  static time64_t pmu_get_time(void)
>  {
> struct adb_request req;
> -   time64_t now;
> +   u32 now;
>
> if (pmu_request(&req, NULL, 1, PMU_READ_RTC) < 0)
> return 0;
>
> On the upside, this would possibly extend the life of some machines
> by another 66 years, on the downside it might cause issues when
> an empty RTC battery causes the initial system time to be
> above the 32-bit TIME_T_MAX (instead of going back to 1904).

I would submit the first patch and add the above as comment. I doubt
anyone would still have a running system by then.

Thanks


Re: [PATCH] Revert "net: pskb_trim_rcsum() and CHECKSUM_COMPLETE are friends"

2018-06-16 Thread Mathieu Malaterre
Hi Eric,

On Fri, Jun 15, 2018 at 9:14 PM Eric Dumazet  wrote:
>
>
>
> On 06/15/2018 11:56 AM, Mathieu Malaterre wrote:
> > This reverts commit 88078d98d1bb085d72af8437707279e203524fa5.
> >
> > It causes regressions for people using chips driven by the sungem
> > driver. Suspicion is that the skb->csum value isn't being adjusted
> > properly.
> >
> > Symptoms as seen on G4+sungem are:
> >
> > [   34.023281] eth0: hw csum failure
> > [   34.023438] CPU: 0 PID: 0 Comm: swapper Not tainted 4.17.0+ #2
> > [   34.023618] Call Trace:
> > [   34.023707] [dffedbd0] [c069ddac] __skb_checksum_complete+0xf0/0x108 
> > (unreliable)
> > [   34.023948] [dffedbf0] [c0777a70] tcp_v4_rcv+0x604/0xe00
> > [   34.024118] [dffedc70] [c0731624] ip_local_deliver_finish+0xa8/0x3c4
> > [   34.024315] [dffedcb0] [c0732430] ip_local_deliver+0xf0/0x154
> > [   34.024493] [dffedcf0] [c07328dc] ip_rcv+0x448/0x774
> > [   34.024653] [dffedd50] [c06aeae0] __netif_receive_skb_core+0x5e8/0x1184
> > [   34.024857] [dffedde0] [c06bba20] napi_gro_receive+0x160/0x22c
> > [   34.025044] [dffede10] [e14b2590] gem_poll+0x7fc/0x1ac0 [sungem]
> > [   34.025228] [dffedee0] [c06bacf0] net_rx_action+0x34c/0x618
> > [   34.025402] [dffedf60] [c07fd27c] __do_softirq+0x16c/0x5f0
> > [   34.025575] [dffedfd0] [c0064c7c] irq_exit+0x110/0x1a8
> > [   34.025738] [dffedff0] [c0016170] call_do_irq+0x24/0x3c
> > [   34.025903] [c0cf7e80] [c0009a84] do_IRQ+0x98/0x1a0
> > [   34.026055] [c0cf7eb0] [c001b474] ret_from_except+0x0/0x14
> > [   34.026225] --- interrupt: 501 at arch_cpu_idle+0x30/0x78
> >LR = arch_cpu_idle+0x30/0x78
> > [   34.026510] [c0cf7f70] [c0cf6000] 0xc0cf6000 (unreliable)
> > [   34.026682] [c0cf7f80] [c00a3868] do_idle+0xc4/0x158
> > [   34.026835] [c0cf7fb0] [c00a3ab0] cpu_startup_entry+0x20/0x28
> > [   34.027013] [c0cf7fc0] [c0998820] start_kernel+0x47c/0x490
> > [   34.027181] [c0cf7ff0] [3444] 0x3444
> >
> > See commit 7ce5a27f2ef8 ("Revert "net: Handle CHECKSUM_COMPLETE more
> > adequately in pskb_trim_rcsum()."") for previous reference.
>
> This fix seems to hide a bug in csum functions on this architecture.

That's odd since it seems to only affect g4+sungem user. None of the
ppc64 seems to be having it. And some ppc32 users are not even seeing
it.

> Or a bug on this NIC when receiving a small packet (less than 60 bytes).
> Maybe the padding bytes are not included in NIC provided csum, and not 0.

Ok in that case the bug is located in
./drivers/net/ethernet/sun/sungem.c that seems more likely. I'll try
to understand that code, then.

Thanks

>
>
>


Re: [PATCH] Revert "net: pskb_trim_rcsum() and CHECKSUM_COMPLETE are friends"

2018-06-18 Thread Mathieu Malaterre
On Mon, Jun 18, 2018 at 7:54 PM Andreas Schwab  wrote:
>
> On Jun 17 2018, Eric Dumazet  wrote:
>
> > Oh this is silly, please try :
> >
> > diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> > index 
> > c642304f178ce0a4e1358d59e45032a39f76fb3f..54dd9c18ecad817812898d6f335e1794a07dabbe
> >  100644
> > --- a/net/core/skbuff.c
> > +++ b/net/core/skbuff.c
> > @@ -1845,10 +1845,9 @@ EXPORT_SYMBOL(___pskb_trim);
> >  int pskb_trim_rcsum_slow(struct sk_buff *skb, unsigned int len)
> >  {
> > if (skb->ip_summed == CHECKSUM_COMPLETE) {
> > -   int delta = skb->len - len;
> > +   __wsum csumdiff = skb_checksum(skb, len, skb->len - len, 0);
> >
> > -   skb->csum = csum_sub(skb->csum,
> > -skb_checksum(skb, len, delta, 0));
> > +   skb->csum = csum_block_sub(skb->csum, csumdiff, len);
> > }
> > return __pskb_trim(skb, len);
> >  }
>
> That doesn't help either.

seconded (setup g4+sungem):

[  100.272676] eth0: hw csum failure
[  100.272710] CPU: 0 PID: 0 Comm: swapper Not tainted 4.17.0+ #6
[  100.272716] Call Trace:
[  100.272733] [dffedbd0] [c069ddb8]
__skb_checksum_complete+0xf0/0x108 (unreliable)
[  100.272752] [dffedbf0] [c078ea28] __udp4_lib_rcv+0x238/0xf98
[  100.272767] [dffedc70] [c0731630] ip_local_deliver_finish+0xa8/0x3c4
[  100.272777] [dffedcb0] [c073243c] ip_local_deliver+0xf0/0x154
[  100.272786] [dffedcf0] [c07328e8] ip_rcv+0x448/0x774
[  100.272800] [dffedd50] [c06aeaec] __netif_receive_skb_core+0x5e8/0x1184
[  100.272811] [dffedde0] [c06bba2c] napi_gro_receive+0x160/0x22c
[  100.272828] [dffede10] [e1571590] gem_poll+0x7fc/0x1ac0 [sungem]
[  100.272837] [dffedee0] [c06bacfc] net_rx_action+0x34c/0x618
[  100.272849] [dffedf60] [c07fd28c] __do_softirq+0x16c/0x5f0
[  100.272863] [dffedfd0] [c0064c7c] irq_exit+0x110/0x1a8
[  100.272877] [dffedff0] [c0016170] call_do_irq+0x24/0x3c
[  100.272890] [c0cf7e80] [c0009a84] do_IRQ+0x98/0x1a0
[  100.272900] [c0cf7eb0] [c001b474] ret_from_except+0x0/0x14
[  100.272911] --- interrupt: 501 at arch_cpu_idle+0x30/0x78
   LR = arch_cpu_idle+0x30/0x78
[  100.272920] [c0cf7f70] [c0cf6000] 0xc0cf6000 (unreliable)
[  100.272935] [c0cf7f80] [c00a3868] do_idle+0xc4/0x158
[  100.272944] [c0cf7fb0] [c00a3ab4] cpu_startup_entry+0x24/0x28
[  100.272955] [c0cf7fc0] [c0998820] start_kernel+0x47c/0x490
[  100.272963] [c0cf7ff0] [3444] 0x3444


> Andreas.
>
> --
> Andreas Schwab, sch...@linux-m68k.org
> GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
> "And now for something completely different."


Re: [PATCH] Revert "net: pskb_trim_rcsum() and CHECKSUM_COMPLETE are friends"

2018-06-18 Thread Mathieu Malaterre
On Mon, Jun 18, 2018 at 8:18 PM Eric Dumazet  wrote:
>
>
>
> On 06/18/2018 10:54 AM, Andreas Schwab wrote:
> > On Jun 17 2018, Eric Dumazet  wrote:
> >
> >> Oh this is silly, please try :
> >>
> >> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> >> index 
> >> c642304f178ce0a4e1358d59e45032a39f76fb3f..54dd9c18ecad817812898d6f335e1794a07dabbe
> >>  100644
> >> --- a/net/core/skbuff.c
> >> +++ b/net/core/skbuff.c
> >> @@ -1845,10 +1845,9 @@ EXPORT_SYMBOL(___pskb_trim);
> >>  int pskb_trim_rcsum_slow(struct sk_buff *skb, unsigned int len)
> >>  {
> >> if (skb->ip_summed == CHECKSUM_COMPLETE) {
> >> -   int delta = skb->len - len;
> >> +   __wsum csumdiff = skb_checksum(skb, len, skb->len - len, 
> >> 0);
> >>
> >> -   skb->csum = csum_sub(skb->csum,
> >> -skb_checksum(skb, len, delta, 0));
> >> +   skb->csum = csum_block_sub(skb->csum, csumdiff, len);
> >> }
> >> return __pskb_trim(skb, len);
> >>  }
> >
> > That doesn't help either.
> >
> > Andreas.
> >
>
> Then maybe NIC provided csum is not correct.
>
> It does not compute a checksum on all the frame, but part of it.
>
> You could use this patch to double check.
>
> diff --git a/drivers/net/ethernet/sun/sungem.c 
> b/drivers/net/ethernet/sun/sungem.c
> index 
> 7a16d40a72d13cf1d522e8a3a396c826fe76f9b9..277859ea73e35271a10b02011120fca248ec8e71
>  100644
> --- a/drivers/net/ethernet/sun/sungem.c
> +++ b/drivers/net/ethernet/sun/sungem.c
> @@ -857,6 +857,11 @@ static int gem_rx(struct gem *gp, int work_to_do)
>
> csum = (__force __sum16)htons((status & RXDCTRL_TCPCSUM) ^ 
> 0x);
> skb->csum = csum_unfold(csum);
> +   {
> +   __wsum rsum = csum_partial(skb->data + ETH_HLEN, len - 
> ETH_HLEN, 0);
> +   if (csum != csum_fold(rsum))
> +   pr_err_ratelimited("sungem wrong csum : %x/%x, len %u 
> bytes\n", csum, csum_fold(rsum), len);
> +   }
> skb->ip_summed = CHECKSUM_COMPLETE;
> skb->protocol = eth_type_trans(skb, gp->dev);
>
>

Here is what I get on my side

[   53.628847] sungem: sungem wrong csum : 4e04/f97, len 64 bytes
[   53.667063] sungem: sungem wrong csum : eea8/6eec, len 149 bytes
[   58.648952] sungem: sungem wrong csum : 2095/3d06, len 64 bytes
[   58.669414] sungem: sungem wrong csum : 5245/b50, len 149 bytes
[   63.674451] sungem: sungem wrong csum : 2d8/5abd, len 149 bytes
[   68.678233] sungem: sungem wrong csum : b8fc/a498, len 149 bytes
[   73.685771] sungem: sungem wrong csum : 374/5a21, len 149 bytes
[   78.689089] sungem: sungem wrong csum : d81/5014, len 149 bytes
[   83.683261] sungem: sungem wrong csum : 4e04/f97, len 64 bytes
[   83.690193] sungem: sungem wrong csum : c2f7/9a9d, len 149 bytes
[   88.692511] sungem: sungem wrong csum : f4d8/68bc, len 149 bytes
[   93.699915] sungem: sungem wrong csum : 1370/4a25, len 149 bytes
[   98.703136] sungem: sungem wrong csum : e1b5/7bdf, len 149 bytes
[  103.704230] sungem: sungem wrong csum : 5321/a74, len 149 bytes
[  108.688912] sungem: sungem wrong csum : 2095/3d06, len 64 bytes
[  108.706559] sungem: sungem wrong csum : ddbc/7fd8, len 149 bytes
[  113.713189] sungem: sungem wrong csum : 5a65/330, len 149 bytes
[  113.891697] sungem: sungem wrong csum : 4e04/f97, len 64 bytes
[  118.717151] sungem: sungem wrong csum : f7c8/65cc, len 149 bytes
[  123.722680] sungem: sungem wrong csum : 3d7a/201b, len 149 bytes
[  128.726524] sungem: sungem wrong csum : c8fd/9497, len 149 bytes
[  133.732045] sungem: sungem wrong csum : de0d/7f87, len 149 bytes
[  135.529163] sungem: sungem wrong csum : 3089/b6dd, len 96 bytes
[  135.529208] eth0: hw csum failure
[  135.529220] CPU: 0 PID: 0 Comm: swapper Not tainted 4.17.0+ #7
[  135.529226] Call Trace:
[  135.529243] [dffedbe0] [c069ddac]
__skb_checksum_complete+0xf0/0x108 (unreliable)


>
>
>


Re: [PATCH 1/3] powerpc: mac: fix rtc read functions

2018-06-18 Thread Mathieu Malaterre
On Mon, Jun 18, 2018 at 4:07 PM Arnd Bergmann  wrote:
>
> As Mathieu pointed out, my conversion to time64_t was incorrect and resulted
> in negative times to be read from the RTC. The problem is that during the
> conversion from a byte array to a time64_t, the 'unsigned char' variable
> holding the top byte gets turned into a negative signed 32-bit integer
> before being assigned to the 64-bit variable for any times after 1972.
>
> This changes the logic to cast to an unsigned 32-bit number first for
> the Macintosh time and then convert that to the Unix time, which then gives
> us a time in the documented 1904..2040 year range. I decided not to use
> the longer 1970..2106 range that other drivers use, for consistency with
> the literal interpretation of the register, but that could be easily
> changed if we decide we want to support any Mac after 2040.
>
> Just to be on the safe side, I'm also adding a WARN_ON that will trigger
> if either the year 2040 has come and is observed by this driver, or we
> run into an RTC that got set back to a pre-1970 date for some reason
> (the two are indistinguishable).
>
> The same code exists in arch/m68k/ and is patched in an identical way now
> in a separate patch.
>
> Fixes: 5bfd643583b2 ("powerpc: use time64_t in read_persistent_clock")
> Reported-by: Mathieu Malaterre 
> Signed-off-by: Arnd Bergmann 
> ---
>  arch/powerpc/platforms/powermac/time.c | 21 -
>  1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powermac/time.c 
> b/arch/powerpc/platforms/powermac/time.c
> index 7c968e46736f..173a80630169 100644
> --- a/arch/powerpc/platforms/powermac/time.c
> +++ b/arch/powerpc/platforms/powermac/time.c
> @@ -42,7 +42,11 @@
>  #define DBG(x...)
>  #endif
>
> -/* Apparently the RTC stores seconds since 1 Jan 1904 */
> +/*
> + * Offset between Unix time (1970-based) and Mac time (1904-based). Cuda and 
> PMU
> + * times wrap in 2040. If we need to handle later times, the read_time 
> functions
> + * need to be changed to interpret wrapped times as post-2040.
> + */
>  #define RTC_OFFSET 2082844800
>
>  /*
> @@ -97,8 +101,11 @@ static time64_t cuda_get_time(void)
> if (req.reply_len != 7)
> printk(KERN_ERR "cuda_get_time: got %d byte reply\n",
>req.reply_len);
> -   now = (req.reply[3] << 24) + (req.reply[4] << 16)
> -   + (req.reply[5] << 8) + req.reply[6];
> +   now = (u32)((req.reply[3] << 24) + (req.reply[4] << 16) +
> +   (req.reply[5] << 8) + req.reply[6]);
> +   /* it's either after year 2040, or the RTC has gone backwards */
> +   WARN_ON(now < RTC_OFFSET);
> +
> return now - RTC_OFFSET;
>  }
>
> @@ -140,8 +147,12 @@ static time64_t pmu_get_time(void)
> if (req.reply_len != 4)
> printk(KERN_ERR "pmu_get_time: got %d byte reply from PMU\n",
>req.reply_len);
> -   now = (req.reply[0] << 24) + (req.reply[1] << 16)
> -   + (req.reply[2] << 8) + req.reply[3];
> +   now = (u32)((req.reply[0] << 24) + (req.reply[1] << 16) +
> +   (req.reply[2] << 8) + req.reply[3]);
> +
> +   /* it's either after year 2040, or the RTC has gone backwards */
> +   WARN_ON(now < RTC_OFFSET);
> +
> return now - RTC_OFFSET;
>  }

Sadly, trying again today does not work anymore. Adding some printk
just before WARN_ON:

+printk(KERN_ERR " rtc DBG pmu_get_time1: %lld %d %lld \n", now,
RTC_OFFSET, now - RTC_OFFSET );
+printk(KERN_ERR " rtc DBG pmu_get_time2: %x %x %x %x %d \n",
req.reply[0], req.reply[1], req.reply[2], req.reply[3] ,
req.reply_len);

leads to:

[0.00]  rtc DBG pmu_get_time1: 14096662 2082844800 -2068748138
[0.00]  rtc DBG pmu_get_time2: 0 d7 19 16 4
[0.00] WARNING: CPU: 0 PID: 0 at
../arch/powerpc/platforms/powermac/time.c:158 pmu_get_time+0x11c/0x16c
[0.00] Modules linked in:
[0.00] CPU: 0 PID: 0 Comm: swapper Not tainted 4.17.0+ #8
[0.00] NIP:  c0035658 LR: c0035640 CTR: c00d4a5c
[0.00] REGS: c0cf7da0 TRAP: 0700   Not tainted  (4.17.0+)
[0.00] MSR:  00021032   CR: 84000848  XER: 
[0.00]
   GPR00: c0035640 c0cf7e50 c0a41bd0 0025 0001
0051 c0d56430 
   GPR08: 0051 0001 c0aa4b38 0004 24000842
 ffbbf4c0 0013da1c
   GPR16: 0013da18 0013da20   00b81044
01696028 01697e02 4014
   GPR24:  00d71000 c09debec dfff1120 01a4
00d71916  84b16896

Re: [PATCH 1/3] powerpc: mac: fix rtc read functions

2018-06-18 Thread Mathieu Malaterre
On Mon, Jun 18, 2018 at 10:04 PM Andreas Schwab  wrote:
>
> On Jun 18 2018, Mathieu Malaterre  wrote:
>
> > Sadly, trying again today does not work anymore. Adding some printk
> > just before WARN_ON:
> >
> > +printk(KERN_ERR " rtc DBG pmu_get_time1: %lld %d %lld \n", now,
> > RTC_OFFSET, now - RTC_OFFSET );
> > +printk(KERN_ERR " rtc DBG pmu_get_time2: %x %x %x %x %d \n",
> > req.reply[0], req.reply[1], req.reply[2], req.reply[3] ,
> > req.reply_len);
> >
> > leads to:
> >
> > [0.00]  rtc DBG pmu_get_time1: 14096662 2082844800 -2068748138
> > [0.00]  rtc DBG pmu_get_time2: 0 d7 19 16 4
>
> A good value would have 0xd7 in the first byte.  The problem is that
> pmu_set_rtc_time is also broken, and leads to an invalid time value
> stored in the RTC.  Since pmu_request is a varargs function passing
> values of type time64_t without casting won't work.
>
> You need to reset your RTC before you can continue.

Indeed that was silly, I was not paying attention. I'll try again tonight.

> I think the right fix is to change nowtime in pmu_set_rtc_time and
> cuda_set_rtc_time back to unsigned int (or to u32).
>
> Andreas.
>
> --
> Andreas Schwab, sch...@linux-m68k.org
> GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
> "And now for something completely different."


Re: [PATCH 1/3] [v2] powerpc: mac: fix rtc read/write functions

2018-06-20 Thread Mathieu Malaterre
On Tue, Jun 19, 2018 at 4:04 PM Arnd Bergmann  wrote:
>
> As Mathieu pointed out, my conversion to time64_t was incorrect and resulted
> in negative times to be read from the RTC. The problem is that during the
> conversion from a byte array to a time64_t, the 'unsigned char' variable
> holding the top byte gets turned into a negative signed 32-bit integer
> before being assigned to the 64-bit variable for any times after 1972.
>
> This changes the logic to cast to an unsigned 32-bit number first for
> the Macintosh time and then convert that to the Unix time, which then gives
> us a time in the documented 1904..2040 year range. I decided not to use
> the longer 1970..2106 range that other drivers use, for consistency with
> the literal interpretation of the register, but that could be easily
> changed if we decide we want to support any Mac after 2040.
>
> Just to be on the safe side, I'm also adding a WARN_ON that will trigger
> if either the year 2040 has come and is observed by this driver, or we
> run into an RTC that got set back to a pre-1970 date for some reason
> (the two are indistinguishable).
>
> For the RTC write functions, Andreas found another problem: both
> pmu_request() and cuda_request() are varargs functions, so changing
> the type of the arguments passed into them from 32 bit to 64 bit
> breaks the API for the set_rtc_time functions. This changes it
> back to 32 bits.
>
> The same code exists in arch/m68k/ and is patched in an identical way now
> in a separate patch.
>
> Fixes: 5bfd643583b2 ("powerpc: use time64_t in read_persistent_clock")
> Reported-by: Mathieu Malaterre 
> Reported-by: Andreas Schwab 
> Signed-off-by: Arnd Bergmann 

Doing a reboot on Debian sets the hardware clock from system clock and
upon reboot everything is back in shape:

[5.645082] rtc-generic rtc-generic: setting system clock to
2018-06-20 07:12:04 UTC (1529478724)


Tested-by: Mathieu Malaterre 

Thanks!
> ---
>  arch/powerpc/platforms/powermac/time.c | 29 -
>  1 file changed, 20 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powermac/time.c 
> b/arch/powerpc/platforms/powermac/time.c
> index 7c968e46736f..12e6e4d30602 100644
> --- a/arch/powerpc/platforms/powermac/time.c
> +++ b/arch/powerpc/platforms/powermac/time.c
> @@ -42,7 +42,11 @@
>  #define DBG(x...)
>  #endif
>
> -/* Apparently the RTC stores seconds since 1 Jan 1904 */
> +/*
> + * Offset between Unix time (1970-based) and Mac time (1904-based). Cuda and 
> PMU
> + * times wrap in 2040. If we need to handle later times, the read_time 
> functions
> + * need to be changed to interpret wrapped times as post-2040.
> + */
>  #define RTC_OFFSET 2082844800
>
>  /*
> @@ -97,8 +101,11 @@ static time64_t cuda_get_time(void)
> if (req.reply_len != 7)
> printk(KERN_ERR "cuda_get_time: got %d byte reply\n",
>req.reply_len);
> -   now = (req.reply[3] << 24) + (req.reply[4] << 16)
> -   + (req.reply[5] << 8) + req.reply[6];
> +   now = (u32)((req.reply[3] << 24) + (req.reply[4] << 16) +
> +   (req.reply[5] << 8) + req.reply[6]);
> +   /* it's either after year 2040, or the RTC has gone backwards */
> +   WARN_ON(now < RTC_OFFSET);
> +
> return now - RTC_OFFSET;
>  }
>
> @@ -106,10 +113,10 @@ static time64_t cuda_get_time(void)
>
>  static int cuda_set_rtc_time(struct rtc_time *tm)
>  {
> -   time64_t nowtime;
> +   u32 nowtime;
> struct adb_request req;
>
> -   nowtime = rtc_tm_to_time64(tm) + RTC_OFFSET;
> +   nowtime = lower_32_bits(rtc_tm_to_time64(tm) + RTC_OFFSET);
> if (cuda_request(&req, NULL, 6, CUDA_PACKET, CUDA_SET_TIME,
>  nowtime >> 24, nowtime >> 16, nowtime >> 8,
>  nowtime) < 0)
> @@ -140,8 +147,12 @@ static time64_t pmu_get_time(void)
> if (req.reply_len != 4)
> printk(KERN_ERR "pmu_get_time: got %d byte reply from PMU\n",
>req.reply_len);
> -   now = (req.reply[0] << 24) + (req.reply[1] << 16)
> -   + (req.reply[2] << 8) + req.reply[3];
> +   now = (u32)((req.reply[0] << 24) + (req.reply[1] << 16) +
> +   (req.reply[2] << 8) + req.reply[3]);
> +
> +   /* it's either after year 2040, or the RTC has gone backwards */
> +   WARN_ON(now < RTC_OFFSET);
> +
> return now - RTC_OFFSET;
>  }
>
> @@ -149,10 +160,10 @@ static time64_t pmu_get_time(void)
>
>  static int pmu_set_rtc_time(struct rtc_time *tm)
>  {
> -   time64_t nowtime;
> +   u32 nowtime;
> struct adb_request req;
>
> -   nowtime = rtc_tm_to_time64(tm) + RTC_OFFSET;
> +   nowtime = lower_32_bits(rtc_tm_to_time64(tm) + RTC_OFFSET);
> if (pmu_request(&req, NULL, 5, PMU_SET_RTC, nowtime >> 24,
> nowtime >> 16, nowtime >> 8, nowtime) < 0)
> return -ENXIO;
> --
> 2.9.0
>


arch/powerpc/boot/addnote.c:75:47: warning: right shift count >= width of type [-Wshift-count-overflow]

2018-06-20 Thread Mathieu Malaterre
Hi there,

For some reason I am seeing some new warning (W=0) on some old code.
Anyone else seeing them ?

  HOSTCC  arch/powerpc/boot/addnote
arch/powerpc/boot/addnote.c: In function ‘main’:
arch/powerpc/boot/addnote.c:75:47: warning: right shift count >= width
of type [-Wshift-count-overflow]
 #define PUT_64BE(off, v)((PUT_32BE((off), (v) >> 32L), \
   ^
arch/powerpc/boot/addnote.c:72:39: note: in definition of macro ‘PUT_16BE’
 #define PUT_16BE(off, v)(buf[off] = ((v) >> 8) & 0xff, \
   ^
arch/powerpc/boot/addnote.c:75:27: note: in expansion of macro ‘PUT_32BE’
 #define PUT_64BE(off, v)((PUT_32BE((off), (v) >> 32L), \
   ^~~~
arch/powerpc/boot/addnote.c:94:50: note: in expansion of macro ‘PUT_64BE’
 #define PUT_64(off, v)  (e_data == ELFDATA2MSB ? PUT_64BE(off, v) : \
  ^~~~
arch/powerpc/boot/addnote.c:183:3: note: in expansion of macro ‘PUT_64’
   PUT_64(ph + PH_OFFSET, ns);
   ^~
arch/powerpc/boot/addnote.c:75:47: warning: right shift count >= width
of type [-Wshift-count-overflow]
 #define PUT_64BE(off, v)((PUT_32BE((off), (v) >> 32L), \
   ^
arch/powerpc/boot/addnote.c:73:23: note: in definition of macro ‘PUT_16BE’
 buf[(off) + 1] = (v) & 0xff)


[PATCH v3] powerpc/32: Remove left over function prototypes

2018-06-20 Thread Mathieu Malaterre
In commit 4aea909eeba3 ("powerpc: Add missing prototypes in setup_32.c")
prototypes for

- ppc_setup_l2cr
- ppc_setup_l3cr
- ppc_init

were added but at the same time in commit d15a261d876d ("powerpc/32: Make
some functions static") those same functions were made static. Fix
conflicting changes by removing the prototypes and leave the function as
static. Fix the following warnings, treated as errors with W=1:

  arch/powerpc/kernel/setup_32.c:127:19: error: static declaration of 
‘ppc_setup_l2cr’ follows non-static declaration
  arch/powerpc/kernel/setup_32.c:140:19: error: static declaration of 
‘ppc_setup_l3cr’ follows non-static declaration
  arch/powerpc/kernel/setup_32.c:186:19: error: static declaration of 
‘ppc_init’ follows non-static declaration

Signed-off-by: Mathieu Malaterre 
---
v3: correct subject line to be less confusing
v2: Previous version contained the reverted patch, correct that.

 arch/powerpc/kernel/setup.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/arch/powerpc/kernel/setup.h b/arch/powerpc/kernel/setup.h
index 35ca309848d7..829ed66f0a40 100644
--- a/arch/powerpc/kernel/setup.h
+++ b/arch/powerpc/kernel/setup.h
@@ -19,9 +19,6 @@ void irqstack_early_init(void);
 void setup_power_save(void);
 unsigned long __init early_init(unsigned long dt_ptr);
 void __init machine_init(u64 dt_ptr);
-int __init ppc_setup_l2cr(char *str);
-int __init ppc_setup_l3cr(char *str);
-int __init ppc_init(void);
 #else
 static inline void setup_power_save(void) { };
 #endif
-- 
2.11.0



Re: [PATCH v3] powerpc/32: Remove left over function prototypes

2018-06-22 Thread Mathieu Malaterre
On Thu, Jun 21, 2018 at 1:27 PM Michael Ellerman  wrote:
>
> Mathieu Malaterre  writes:
>
> > In commit 4aea909eeba3 ("powerpc: Add missing prototypes in setup_32.c")
>
> I don't have that commit ^ ?
>
> That might be because I squashed some of your fixes together or something?

I am doing an awful lots of mistakes these days. Indeed you've changed
one of my patch:

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

This one appeared after a git rebase on my side.

> > diff --git a/arch/powerpc/kernel/setup.h b/arch/powerpc/kernel/setup.h
> > index 35ca309848d7..829ed66f0a40 100644
> > --- a/arch/powerpc/kernel/setup.h
> > +++ b/arch/powerpc/kernel/setup.h
> > @@ -19,9 +19,6 @@ void irqstack_early_init(void);
> >  void setup_power_save(void);
> >  unsigned long __init early_init(unsigned long dt_ptr);
> >  void __init machine_init(u64 dt_ptr);
> > -int __init ppc_setup_l2cr(char *str);
> > -int __init ppc_setup_l3cr(char *str);
> > -int __init ppc_init(void);
> >  #else
> >  static inline void setup_power_save(void) { };
> >  #endif
>
> I have:
>
> #ifdef CONFIG_PPC32
> void setup_power_save(void);
> #else
> static inline void setup_power_save(void) { };
> #endif

Correct.

Sorry for the noise.

>
> cheers


Re: [PATCH v2 02/19] powerpc/powermac: Mark variable x as unused

2018-06-22 Thread Mathieu Malaterre
On Thu, Mar 29, 2018 at 6:09 PM LEROY Christophe
 wrote:
>
> Mathieu Malaterre  a écrit :
>
> > Since the value of x is never intended to be read, remove it. Fix warning
> > treated as error with W=1:
> >
> >   arch/powerpc/platforms/powermac/udbg_scc.c:76:9: error: variable
> > ‘x’ set but not used [-Werror=unused-but-set-variable]
> >
> > Suggested-by: Christophe Leroy 
> > Signed-off-by: Mathieu Malaterre 
>
> Reviewed-by: Christophe Leroy 

Michael, can you take this one ?

> > ---
> > v2: remove x completely
> >
> >  arch/powerpc/platforms/powermac/udbg_scc.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/powermac/udbg_scc.c
> > b/arch/powerpc/platforms/powermac/udbg_scc.c
> > index d83135a9830e..8901973ed683 100644
> > --- a/arch/powerpc/platforms/powermac/udbg_scc.c
> > +++ b/arch/powerpc/platforms/powermac/udbg_scc.c
> > @@ -73,7 +73,7 @@ void udbg_scc_init(int force_scc)
> >   struct device_node *stdout = NULL, *escc = NULL, *macio = NULL;
> >   struct device_node *ch, *ch_def = NULL, *ch_a = NULL;
> >   const char *path;
> > - int i, x;
> > + int i;
> >
> >   escc = of_find_node_by_name(NULL, "escc");
> >   if (escc == NULL)
> > @@ -120,7 +120,7 @@ void udbg_scc_init(int force_scc)
> >   mb();
> >
> >   for (i = 2; i != 0; --i)
> > - x = in_8(sccc);
> > + in_8(sccc);
> >   out_8(sccc, 0x09);  /* reset A or B side */
> >   out_8(sccc, 0xc0);
> >
> > --
> > 2.11.0
>
>


Re: [PATCH v3 03/19] powerpc: Move `path` variable inside DEBUG_PROM

2018-06-22 Thread Mathieu Malaterre
This one should be ok.

On Wed, Apr 4, 2018 at 10:08 PM Mathieu Malaterre  wrote:
>
> Add gcc attribute unused for two variables. Fix warnings treated as errors
> with W=1:
>
>   arch/powerpc/kernel/prom_init.c:1388:8: error: variable ‘path’ set but not 
> used [-Werror=unused-but-set-variable]
>
> Suggested-by: Christophe Leroy 
> Signed-off-by: Mathieu Malaterre 
> ---
> v3: really move path within ifdef DEBUG_PROM
> v2: move path within ifdef DEBUG_PROM
>
>  arch/powerpc/kernel/prom_init.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index acf4b2e0530c..223b35acbbdd 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -603,7 +603,7 @@ static void __init early_cmdline_parse(void)
> const char *opt;
>
> char *p;
> -   int l = 0;
> +   int l __maybe_unused = 0;
>
> prom_cmd_line[0] = 0;
> p = prom_cmd_line;
> @@ -1385,7 +1385,10 @@ static void __init reserve_mem(u64 base, u64 size)
>  static void __init prom_init_mem(void)
>  {
> phandle node;
> -   char *path, type[64];
> +#ifdef DEBUG_PROM
> +   char *path;
> +#endif
> +   char type[64];
> unsigned int plen;
> cell_t *p, *endp;
> __be32 val;
> @@ -1406,7 +1409,9 @@ static void __init prom_init_mem(void)
> prom_debug("root_size_cells: %x\n", rsc);
>
> prom_debug("scanning memory:\n");
> +#ifdef DEBUG_PROM
> path = prom_scratch;
> +#endif
>
> for (node = 0; prom_next_node(&node); ) {
> type[0] = 0;
> --
> 2.11.0
>


Re: [PATCH 16/19] powerpc/powermac: Add missing include of header pmac.h

2018-06-22 Thread Mathieu Malaterre
This one is also ok.

On Thu, Mar 22, 2018 at 9:21 PM Mathieu Malaterre  wrote:
>
> The header `pmac.h` was not included, leading to the following warnings,
> treated as error with W=1:
>
>   arch/powerpc/platforms/powermac/time.c:69:13: error: no previous prototype 
> for ‘pmac_time_init’ [-Werror=missing-prototypes]
>   arch/powerpc/platforms/powermac/time.c:207:15: error: no previous prototype 
> for ‘pmac_get_boot_time’ [-Werror=missing-prototypes]
>   arch/powerpc/platforms/powermac/time.c:222:6: error: no previous prototype 
> for ‘pmac_get_rtc_time’ [-Werror=missing-prototypes]
>   arch/powerpc/platforms/powermac/time.c:240:5: error: no previous prototype 
> for ‘pmac_set_rtc_time’ [-Werror=missing-prototypes]
>   arch/powerpc/platforms/powermac/time.c:259:12: error: no previous prototype 
> for ‘via_calibrate_decr’ [-Werror=missing-prototypes]
>   arch/powerpc/platforms/powermac/time.c:311:13: error: no previous prototype 
> for ‘pmac_calibrate_decr’ [-Werror=missing-prototypes]
>
> The function `via_calibrate_decr` was made static to silence a warning.
>
> Signed-off-by: Mathieu Malaterre 
> ---
>  arch/powerpc/platforms/powermac/time.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/powermac/time.c 
> b/arch/powerpc/platforms/powermac/time.c
> index 274af6fa388e..5cc6fa40fcc4 100644
> --- a/arch/powerpc/platforms/powermac/time.c
> +++ b/arch/powerpc/platforms/powermac/time.c
> @@ -34,6 +34,8 @@
>  #include 
>  #include 
>
> +#include "pmac.h"
> +
>  #undef DEBUG
>
>  #ifdef DEBUG
> @@ -256,7 +258,7 @@ int pmac_set_rtc_time(struct rtc_time *tm)
>   * Calibrate the decrementer register using VIA timer 1.
>   * This is used both on powermacs and CHRP machines.
>   */
> -int __init via_calibrate_decr(void)
> +static int __init via_calibrate_decr(void)
>  {
> struct device_node *vias;
> volatile unsigned char __iomem *via;
> --
> 2.11.0
>


Re: [PATCH v2 07/19] powerpc/powermac: Make some functions static

2018-06-22 Thread Mathieu Malaterre
This one should also be good to go.

On Wed, Mar 28, 2018 at 9:39 PM Mathieu Malaterre  wrote:
>
> These functions can all be static, make it so. Fix warnings treated as
> errors with W=1:
>
>   arch/powerpc/platforms/powermac/pci.c:1022:6: error: no previous prototype 
> for ‘pmac_pci_fixup_ohci’ [-Werror=missing-prototypes]
>   arch/powerpc/platforms/powermac/pci.c:1057:6: error: no previous prototype 
> for ‘pmac_pci_fixup_cardbus’ [-Werror=missing-prototypes]
>   arch/powerpc/platforms/powermac/pci.c:1094:6: error: no previous prototype 
> for ‘pmac_pci_fixup_pciata’ [-Werror=missing-prototypes]
>
> Remove has_address declaration and assignment since not used. Also add gcc
> attribute unused to fix a warning treated as error with W=1:
>
>   arch/powerpc/platforms/powermac/pci.c:784:19: error: variable ‘has_address’ 
> set but not used [-Werror=unused-but-set-variable]
>   arch/powerpc/platforms/powermac/pci.c:907:22: error: variable ‘ht’ set but 
> not used [-Werror=unused-but-set-variable]
>
> Suggested-by: Christophe Leroy 
> Signed-off-by: Mathieu Malaterre 
> ---
> v2: remove has_address variable since not used
>  arch/powerpc/platforms/powermac/pci.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powermac/pci.c 
> b/arch/powerpc/platforms/powermac/pci.c
> index 0b8174a79993..67c497093e0a 100644
> --- a/arch/powerpc/platforms/powermac/pci.c
> +++ b/arch/powerpc/platforms/powermac/pci.c
> @@ -781,12 +781,12 @@ static int __init pmac_add_bridge(struct device_node 
> *dev)
> struct resource rsrc;
> char *disp_name;
> const int *bus_range;
> -   int primary = 1, has_address = 0;
> +   int primary = 1;
>
> DBG("Adding PCI host bridge %pOF\n", dev);
>
> /* Fetch host bridge registers address */
> -   has_address = (of_address_to_resource(dev, 0, &rsrc) == 0);
> +   of_address_to_resource(dev, 0, &rsrc);
>
> /* Get bus range if any */
> bus_range = of_get_property(dev, "bus-range", &len);
> @@ -904,7 +904,7 @@ static int pmac_pci_root_bridge_prepare(struct 
> pci_host_bridge *bridge)
>  void __init pmac_pci_init(void)
>  {
> struct device_node *np, *root;
> -   struct device_node *ht = NULL;
> +   struct device_node *ht __maybe_unused = NULL;
>
> pci_set_flags(PCI_CAN_SKIP_ISA_ALIGN);
>
> @@ -1019,7 +1019,7 @@ static bool pmac_pci_enable_device_hook(struct pci_dev 
> *dev)
> return true;
>  }
>
> -void pmac_pci_fixup_ohci(struct pci_dev *dev)
> +static void pmac_pci_fixup_ohci(struct pci_dev *dev)
>  {
> struct device_node *node = pci_device_to_OF_node(dev);
>
> @@ -1054,7 +1054,7 @@ void __init pmac_pcibios_after_init(void)
> }
>  }
>
> -void pmac_pci_fixup_cardbus(struct pci_dev* dev)
> +static void pmac_pci_fixup_cardbus(struct pci_dev *dev)
>  {
> if (!machine_is(powermac))
> return;
> @@ -1091,7 +1091,7 @@ void pmac_pci_fixup_cardbus(struct pci_dev* dev)
>
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_TI, PCI_ANY_ID, 
> pmac_pci_fixup_cardbus);
>
> -void pmac_pci_fixup_pciata(struct pci_dev* dev)
> +static void pmac_pci_fixup_pciata(struct pci_dev *dev)
>  {
> u8 progif = 0;
>
> --
> 2.11.0
>


Re: [PATCH] powerpc/mm: fix always true/false warning in slice.c

2018-06-22 Thread Mathieu Malaterre
On Fri, Jun 22, 2018 at 3:49 PM Christophe Leroy
 wrote:
>
> This patch fixes the following warnings (obtained with make W=1).
>
> arch/powerpc/mm/slice.c: In function 'slice_range_to_mask':
> arch/powerpc/mm/slice.c:73:12: error: comparison is always true due to 
> limited range of data type [-Werror=type-limits]
>   if (start < SLICE_LOW_TOP) {
> ^
> arch/powerpc/mm/slice.c:81:20: error: comparison is always false due to 
> limited range of data type [-Werror=type-limits]
>   if ((start + len) > SLICE_LOW_TOP) {
> ^
> arch/powerpc/mm/slice.c: In function 'slice_mask_for_free':
> arch/powerpc/mm/slice.c:136:17: error: comparison is always true due to 
> limited range of data type [-Werror=type-limits]
>   if (high_limit <= SLICE_LOW_TOP)
>  ^
> arch/powerpc/mm/slice.c: In function 'slice_check_range_fits':
> arch/powerpc/mm/slice.c:185:12: error: comparison is always true due to 
> limited range of data type [-Werror=type-limits]
>   if (start < SLICE_LOW_TOP) {
> ^
> arch/powerpc/mm/slice.c:195:39: error: comparison is always false due to 
> limited range of data type [-Werror=type-limits]
>   if (SLICE_NUM_HIGH && ((start + len) > SLICE_LOW_TOP)) {
>^
> arch/powerpc/mm/slice.c: In function 'slice_scan_available':
> arch/powerpc/mm/slice.c:306:11: error: comparison is always true due to 
> limited range of data type [-Werror=type-limits]
>   if (addr < SLICE_LOW_TOP) {
>^
> arch/powerpc/mm/slice.c: In function 'get_slice_psize':
> arch/powerpc/mm/slice.c:709:11: error: comparison is always true due to 
> limited range of data type [-Werror=type-limits]
>   if (addr < SLICE_LOW_TOP) {
>^
>
> Signed-off-by: Christophe Leroy 
> ---
>  arch/powerpc/mm/slice.c | 21 ++---
>  1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/mm/slice.c b/arch/powerpc/mm/slice.c
> index 9530c6db406a..17c57760e06c 100644
> --- a/arch/powerpc/mm/slice.c
> +++ b/arch/powerpc/mm/slice.c
> @@ -62,6 +62,13 @@ static void slice_print_mask(const char *label, const 
> struct slice_mask *mask) {
>
>  #endif
>
> +static inline bool slice_addr_is_low(unsigned long addr)
> +{
> +   u64 tmp = (u64)addr;
> +
> +   return tmp < SLICE_LOW_TOP;
> +}
> +
>  static void slice_range_to_mask(unsigned long start, unsigned long len,
> struct slice_mask *ret)
>  {
> @@ -71,7 +78,7 @@ static void slice_range_to_mask(unsigned long start, 
> unsigned long len,
> if (SLICE_NUM_HIGH)
> bitmap_zero(ret->high_slices, SLICE_NUM_HIGH);
>
> -   if (start < SLICE_LOW_TOP) {
> +   if (slice_addr_is_low(start)) {
> unsigned long mend = min(end,
>  (unsigned long)(SLICE_LOW_TOP - 1));
>
> @@ -79,7 +86,7 @@ static void slice_range_to_mask(unsigned long start, 
> unsigned long len,
> - (1u << GET_LOW_SLICE_INDEX(start));
> }
>
> -   if ((start + len) > SLICE_LOW_TOP) {
> +   if (!slice_addr_is_low(end)) {
> unsigned long start_index = GET_HIGH_SLICE_INDEX(start);
> unsigned long align_end = ALIGN(end, (1UL << 
> SLICE_HIGH_SHIFT));
> unsigned long count = GET_HIGH_SLICE_INDEX(align_end) - 
> start_index;
> @@ -134,7 +141,7 @@ static void slice_mask_for_free(struct mm_struct *mm, 
> struct slice_mask *ret,
> if (!slice_low_has_vma(mm, i))
> ret->low_slices |= 1u << i;
>
> -   if (high_limit <= SLICE_LOW_TOP)
> +   if (slice_addr_is_low(high_limit - 1))

Is high_limit ever going to be 0 ?

> return;
>
> for (i = 0; i < GET_HIGH_SLICE_INDEX(high_limit); i++)
> @@ -183,7 +190,7 @@ static bool slice_check_range_fits(struct mm_struct *mm,
> unsigned long end = start + len - 1;
> u64 low_slices = 0;
>
> -   if (start < SLICE_LOW_TOP) {
> +   if (slice_addr_is_low(start)) {
> unsigned long mend = min(end,
>  (unsigned long)(SLICE_LOW_TOP - 1));
>
> @@ -193,7 +200,7 @@ static bool slice_check_range_fits(struct mm_struct *mm,
> if ((low_slices & available->low_slices) != low_slices)
> return false;
>
> -   if (SLICE_NUM_HIGH && ((start + len) > SLICE_LOW_TOP)) {
> +   if (SLICE_NUM_HIGH && !slice_addr_is_low(end)) {
> unsigned long start_index = GET_HIGH_SLICE_INDEX(start);
> unsigned long align_end = ALIGN(end, (1UL << 
> SLICE_HIGH_SHIFT));
> unsigned long count = GET_HIGH_SLICE_INDEX(align_end) - 
> start_index;
> @@ -304,7 +311,7 @@ static bool slice_scan_available(unsigned long addr,
>  int end, unsigned long *boundary_addr)
>  {
> unsigned long slice;
> -   if (addr < SLICE_LOW_TOP) {
> +   if (slice_addr_is_low(addr)) {

[PATCH] powerpc: include setup.h header file to fix warnings

2018-06-22 Thread Mathieu Malaterre
Make sure to include setup.h to provide the following prototypes:

- irqstack_early_init
- setup_power_save
- initialize_cache_info

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

  arch/powerpc/kernel/setup_32.c:198:13: error: no previous prototype for 
‘irqstack_early_init’ [-Werror=missing-prototypes]
  arch/powerpc/kernel/setup_32.c:238:13: error: no previous prototype for 
‘setup_power_save’ [-Werror=missing-prototypes]
  arch/powerpc/kernel/setup_32.c:253:13: error: no previous prototype for 
‘initialize_cache_info’ [-Werror=missing-prototypes]

Signed-off-by: Mathieu Malaterre 
---
 arch/powerpc/kernel/setup_32.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/kernel/setup_32.c b/arch/powerpc/kernel/setup_32.c
index 74457485574b..9827b6b3 100644
--- a/arch/powerpc/kernel/setup_32.c
+++ b/arch/powerpc/kernel/setup_32.c
@@ -41,6 +41,8 @@
 #include 
 #include 
 
+#include "setup.h"
+
 #define DBG(fmt...)
 
 extern void bootx_init(unsigned long r4, unsigned long phys);
-- 
2.11.0



[PATCH] powerpc/xmon: avoid warnings about variables that might be clobbered by ‘longjmp’

2018-06-22 Thread Mathieu Malaterre
Move initialization of variables after data definitions. This silence
warnings treated as error with W=1:

  arch/powerpc/xmon/xmon.c:3389:14: error: variable ‘name’ might be clobbered 
by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
  arch/powerpc/xmon/xmon.c:3100:22: error: variable ‘tsk’ might be clobbered by 
‘longjmp’ or ‘vfork’ [-Werror=clobbered]

Signed-off-by: Mathieu Malaterre 
---
 arch/powerpc/xmon/xmon.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 47166ad2a669..982848c784ff 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -3097,10 +3097,11 @@ static void show_pte(unsigned long addr)
 static void show_tasks(void)
 {
unsigned long tskv;
-   struct task_struct *tsk = NULL;
+   struct task_struct *tsk;
 
printf(" task_struct ->thread.kspPID   PPID S  P CMD\n");
 
+   tsk = NULL;
if (scanhex(&tskv))
tsk = (struct task_struct *)tskv;
 
@@ -3386,10 +3387,11 @@ static void xmon_print_symbol(unsigned long address, 
const char *mid,
  const char *after)
 {
char *modname;
-   const char *name = NULL;
+   const char *name;
unsigned long offset, size;
 
printf(REG, address);
+   name = NULL;
if (setjmp(bus_error_jmp) == 0) {
catch_memory_errors = 1;
sync();
-- 
2.11.0



[PATCH] powerpc/mm: remove warning about ‘type’ being set

2018-06-22 Thread Mathieu Malaterre
‘type’ is only used when CONFIG_DEBUG_HIGHMEM is set. So add a possibly
unused tag to variable. Remove warning treated as error with W=1:

  arch/powerpc/mm/highmem.c:59:6: error: variable ‘type’ set but not used 
[-Werror=unused-but-set-variable]

Signed-off-by: Mathieu Malaterre 
---
 arch/powerpc/mm/highmem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/mm/highmem.c b/arch/powerpc/mm/highmem.c
index 668e87d03f9e..82a0e37557a5 100644
--- a/arch/powerpc/mm/highmem.c
+++ b/arch/powerpc/mm/highmem.c
@@ -56,7 +56,7 @@ EXPORT_SYMBOL(kmap_atomic_prot);
 void __kunmap_atomic(void *kvaddr)
 {
unsigned long vaddr = (unsigned long) kvaddr & PAGE_MASK;
-   int type;
+   int type __maybe_unused;
 
if (vaddr < __fix_to_virt(FIX_KMAP_END)) {
pagefault_enable();
-- 
2.11.0



Re: [PATCH] powerpc/mm: remove warning about ‘type’ being set

2018-06-25 Thread Mathieu Malaterre
On Sat, Jun 23, 2018 at 7:12 PM christophe leroy
 wrote:
>
>
>
> Le 22/06/2018 à 21:27, Mathieu Malaterre a écrit :
> > ‘type’ is only used when CONFIG_DEBUG_HIGHMEM is set. So add a possibly
> > unused tag to variable. Remove warning treated as error with W=1:
> >
> >arch/powerpc/mm/highmem.c:59:6: error: variable ‘type’ set but not used 
> > [-Werror=unused-but-set-variable]
>
> Is type neeeded at all when CONFIG_DEBUG_HIGHMEM is not set ?
>
> The calltype = kmap_atomic_idx();  seems useless when
> CONFIG_DEBUG_HIGHMEM isn't set. Couldn't we just most type definition
> and setting inside the CONFIG_DEBUG_HIGHMEM {} below ?
>
> Alternatively, maybe you could replace the #ifdef CONFIG_DEBUG_HIGHMEM
> by anif (IS_ENABLED(CONFIG_DEBUG_HIGHMEM)) ?

I am not familiar with this code. But starring at other arch
implementations (eg. `arch/x86/mm/highmem_32.c`), it feels like
powerpc is skipping `pte_clear(&init_mm, vaddr, kmap_pte-idx);` unless
`CONFIG_DEBUG_HIGHMEM=y`. Could someone please confirm this is the
correct behavior ?

Or else I can rewrite the code a bit like `arch/arm/mm/highmem.c`
which skips `set_fixmap_pte(idx, __pte(0));` unless
`CONFIG_DEBUG_HIGHMEM=y`.

> Christophe
>
> >
> > Signed-off-by: Mathieu Malaterre 
> > ---
> >   arch/powerpc/mm/highmem.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/mm/highmem.c b/arch/powerpc/mm/highmem.c
> > index 668e87d03f9e..82a0e37557a5 100644
> > --- a/arch/powerpc/mm/highmem.c
> > +++ b/arch/powerpc/mm/highmem.c
> > @@ -56,7 +56,7 @@ EXPORT_SYMBOL(kmap_atomic_prot);
> >   void __kunmap_atomic(void *kvaddr)
> >   {
> >   unsigned long vaddr = (unsigned long) kvaddr & PAGE_MASK;
> > - int type;
> > + int type __maybe_unused;
> >
> >   if (vaddr < __fix_to_virt(FIX_KMAP_END)) {
> >   pagefault_enable();
> >
>
> ---
> L'absence de virus dans ce courrier électronique a été vérifiée par le 
> logiciel antivirus Avast.
> https://www.avast.com/antivirus
>


Re: [PATCH] powerpc/xmon: avoid warnings about variables that might be clobbered by ‘longjmp’

2018-06-25 Thread Mathieu Malaterre
On Sat, Jun 23, 2018 at 9:47 PM Segher Boessenkool
 wrote:
>
> On Sat, Jun 23, 2018 at 06:59:27PM +0200, christophe leroy wrote:
> >
> >
> > Le 22/06/2018 à 21:27, Mathieu Malaterre a écrit :
> > >Move initialization of variables after data definitions. This silence
> > >warnings treated as error with W=1:
> > >
> > >   arch/powerpc/xmon/xmon.c:3389:14: error: variable ‘name’ might be
> > >   clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
> > >   arch/powerpc/xmon/xmon.c:3100:22: error: variable ‘tsk’ might be
> > >   clobbered by ‘longjmp’ or ‘vfork’ [-Werror=clobbered]
> >
> > Is that an invalid warning ?
>
> No, both are correct warnings.  GCC can not see which functions it only
> has a declaration of can call longjmp.

I assumed those were false positive warnings, given how easy it was to
defeat them. Let give it another try.

> > Otherwise, I'd expect one to fix the warning, not just cheat on GCC.
>
> Yes, the patch seems to change the code in such a way that some versions
> of GCC will no longer warn.  Which does not make to code any more correct.
>
> Either restructure the code, or make the var non-automatic, or make it
> volatile.
>
>
> Segher


Re: [PATCH v4 01/11] macintosh/via-pmu: Fix section mismatch warning

2018-07-02 Thread Mathieu Malaterre
On Mon, Jul 2, 2018 at 10:25 AM Finn Thain  wrote:
>
> The pmu_init() function has the __init qualifier, but the ops struct
> that holds a pointer to it does not. This causes a build warning.
> The driver works fine because the pointer is only dereferenced early.
>
> The function is so small that there's negligible benefit from using
> the __init qualifier. Remove it to fix the warning, consistent with
> the other ADB drivers.

Would you mind copy/pasting the warning you are seeing.

Make sure you have:

58935176ad17 powerpc/via-pmu: Fix section mismatch warning

Thanks

> Tested-by: Stan Johnson 
> Signed-off-by: Finn Thain 
> Reviewed-by: Geert Uytterhoeven 
> ---
>  drivers/macintosh/via-pmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
> index 25c1ce811053..f8a2c917201f 100644
> --- a/drivers/macintosh/via-pmu.c
> +++ b/drivers/macintosh/via-pmu.c
> @@ -378,7 +378,7 @@ static int pmu_probe(void)
> return vias == NULL? -ENODEV: 0;
>  }
>
> -static int __init pmu_init(void)
> +static int pmu_init(void)
>  {
> if (vias == NULL)
> return -ENODEV;
> --
> 2.16.4
>


83a092cf95f28: powerpc: Link warning for orphan sections

2018-07-03 Thread Mathieu Malaterre
Hi Nick,

I am building my kernel (ppc32) with both
CONFIG_LD_DEAD_CODE_DATA_ELIMINATION=y and CONFIG_UBSAN=y. This leads
to ~316428 warnings such as:

+ powerpc-linux-gnu-ld -EB -m elf32ppc -Bstatic --orphan-handling=warn
--build-id --gc-sections -X -o .tmp_vmlinux1 -T
./arch/powerpc/kernel/vmlinux.lds --who
le-archive built-in.a --no-whole-archive --start-group lib/lib.a --end-group
powerpc-linux-gnu-ld: warning: orphan section `.data..Lubsan_data393'
from `init/main.o' being placed in section `.data..Lubsan_data393'.
powerpc-linux-gnu-ld: warning: orphan section `.data..Lubsan_data394'
from `init/main.o' being placed in section `.data..Lubsan_data394'.
powerpc-linux-gnu-ld: warning: orphan section `.data..Lubsan_data395'
from `init/main.o' being placed in section `.data..Lubsan_data395'.
powerpc-linux-gnu-ld: warning: orphan section `.data..Lubsan_data396'
from `init/main.o' being placed in section `.data..Lubsan_data396'.
...

What would you recommend to reduce the number of warnings produced ? I
tried `--warn-once` but that only affect undefined symbols.

Thanks


Re: 83a092cf95f28: powerpc: Link warning for orphan sections

2018-07-03 Thread Mathieu Malaterre
On Tue, Jul 3, 2018 at 9:23 AM Nicholas Piggin  wrote:
>
> On Tue, 3 Jul 2018 09:03:46 +0200
> Mathieu Malaterre  wrote:
>
> > Hi Nick,
> >
> > I am building my kernel (ppc32) with both
> > CONFIG_LD_DEAD_CODE_DATA_ELIMINATION=y and CONFIG_UBSAN=y. This leads
> > to ~316428 warnings such as:
> >
> > + powerpc-linux-gnu-ld -EB -m elf32ppc -Bstatic --orphan-handling=warn
> > --build-id --gc-sections -X -o .tmp_vmlinux1 -T
> > ./arch/powerpc/kernel/vmlinux.lds --who
> > le-archive built-in.a --no-whole-archive --start-group lib/lib.a --end-group
> > powerpc-linux-gnu-ld: warning: orphan section `.data..Lubsan_data393'
> > from `init/main.o' being placed in section `.data..Lubsan_data393'.
> > powerpc-linux-gnu-ld: warning: orphan section `.data..Lubsan_data394'
> > from `init/main.o' being placed in section `.data..Lubsan_data394'.
> > powerpc-linux-gnu-ld: warning: orphan section `.data..Lubsan_data395'
> > from `init/main.o' being placed in section `.data..Lubsan_data395'.
> > powerpc-linux-gnu-ld: warning: orphan section `.data..Lubsan_data396'
> > from `init/main.o' being placed in section `.data..Lubsan_data396'.
> > ...
> >
> > What would you recommend to reduce the number of warnings produced ? I
> > tried `--warn-once` but that only affect undefined symbols.
>
> I'm not sure if the linker can be quietened. You could try putting
> *(.data..Lubsan_data*) into the .data section in
> powerpc/kernel/vmlinux.lds.S

Nice ! Will submit a patch. Thanks

> Thanks,
> Nick


CONFIG_LD_DEAD_CODE_DATA_ELIMINATION: Section mismatch in reference from the variable via_pmu_driver to the function .init.text:pmu_init()

2018-07-03 Thread Mathieu Malaterre
Hi Nick,

Would you consider this a bug:

$ touch drivers/macintosh/via-pmu.c
$ CONFIG_LD_DEAD_CODE_DATA_ELIMINATION=n make ARCH=powerpc
CROSS_COMPILE=powerpc-linux-gnu-
...
  LD  vmlinux.o
  MODPOST vmlinux.o
WARNING: vmlinux.o(.data+0x216018): Section mismatch in reference from
the variable via_pmu_driver to the function .init.text:pmu_init()
The variable via_pmu_driver references
the function __init pmu_init()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console

While:

$ touch drivers/macintosh/via-pmu.c
$ CONFIG_LD_DEAD_CODE_DATA_ELIMINATION=y make ARCH=powerpc
CROSS_COMPILE=powerpc-linux-gnu-
...
  AR  init/built-in.a
  AR  built-in.a
  LD  vmlinux.o
  MODPOST vmlinux.o
  KSYM.tmp_kallsyms1.o
  KSYM.tmp_kallsyms2.o
  LD  vmlinux
  SORTEX  vmlinux
  SYSMAP  System.map
...

Thanks for comment


Re: CONFIG_LD_DEAD_CODE_DATA_ELIMINATION: Section mismatch in reference from the variable via_pmu_driver to the function .init.text:pmu_init()

2018-07-03 Thread Mathieu Malaterre
On Tue, Jul 3, 2018 at 11:40 AM Mathieu Malaterre  wrote:
>
> Hi Nick,
>
> Would you consider this a bug:
>
> $ touch drivers/macintosh/via-pmu.c
> $ CONFIG_LD_DEAD_CODE_DATA_ELIMINATION=n make ARCH=powerpc
> CROSS_COMPILE=powerpc-linux-gnu-
> ...
>   LD  vmlinux.o
>   MODPOST vmlinux.o
> WARNING: vmlinux.o(.data+0x216018): Section mismatch in reference from
> the variable via_pmu_driver to the function .init.text:pmu_init()
> The variable via_pmu_driver references
> the function __init pmu_init()
> If the reference is valid then annotate the
> variable with __init* or __refdata (see linux/init.h) or name the variable:
> *_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console
>
> While:
>
> $ touch drivers/macintosh/via-pmu.c
> $ CONFIG_LD_DEAD_CODE_DATA_ELIMINATION=y make ARCH=powerpc
> CROSS_COMPILE=powerpc-linux-gnu-
> ...
>   AR  init/built-in.a
>   AR  built-in.a
>   LD  vmlinux.o
>   MODPOST vmlinux.o
>   KSYM.tmp_kallsyms1.o
>   KSYM.tmp_kallsyms2.o
>   LD  vmlinux
>   SORTEX  vmlinux
>   SYSMAP  System.map
> ...
>
> Thanks for comment

Just to clarify I reverted 58935176ad17976b7a7f6ea25c0ceb2ca4308a30
just as to reproduce a warning. So my question (rephrased):

Is this expected that CONFIG_LD_DEAD_CODE_DATA_ELIMINATION=y inhibit
the behavior of  CONFIG_DEBUG_SECTION_MISMATCH=y ?

Thanks


Re: powerpc: 32BIT vs. 64BIT (PPC32 vs. PPC64)

2018-07-09 Thread Mathieu Malaterre
On Sun, Jul 8, 2018 at 1:53 PM Michael Ellerman  wrote:
>
> Randy Dunlap  writes:
> > Hi,
> >
> > Is there a good way (or a shortcut) to do something like:
>
> The best I know of is:
>
> > $ make ARCH=powerpc O=PPC32 [other_options] allmodconfig
> >   to get a PPC32/32BIT allmodconfig
>
> $ echo CONFIG_PPC64=n > allmod.config
> $ KCONFIG_ALLCONFIG=1 make allmodconfig
> $ grep PPC32 .config
> CONFIG_PPC32=y
>
> Which is still a bit clunky.
>
>
> I looked at this a while back and the problem we have is that the 32-bit
> kernel is not a single thing. There are multiple 32-bit platforms which
> are mutually exclusive.
>
> eg, from menuconfig:
>
>  - 512x/52xx/6xx/7xx/74xx/82xx/83xx/86xx
>  - Freescale 85xx
>  - Freescale 8xx
>  - AMCC 40x
>  - AMCC 44x, 46x or 47x
>  - Freescale e200

Most Linux distro seems to have drop support for ppc32. So I'd suggest
to pick Debian powperc default config (but I agree that I am a little
biased here).

>
> So we could have a 32-bit allmodconfig, but we'd need to chose one of
> the above, and we'd still only be testing some of the code.
>
> Having said that you're the 2nd person to ask about this, so we should
> clearly do something to make a 32-bit allmodconfig easier, even if it's
> not perfect.
>
> cheers


Re: [PATCH 1/2] powerpc: Add ppc32_allmodconfig defconfig target

2018-07-10 Thread Mathieu Malaterre
On Mon, Jul 9, 2018 at 4:24 PM Michael Ellerman  wrote:
>
> Because the allmodconfig logic just sets every symbol to M or Y, it
> has the effect of always generating a 64-bit config, because
> CONFIG_PPC64 becomes Y.
>
> So to make it easier for folks to test 32-bit code, provide a phony
> defconfig target that generates a 32-bit allmodconfig.
>
> The 32-bit port has several mutually exclusive CPU types, we choose
> the Book3S variants as that's what the help text in Kconfig says is
> most common.

Ok then.

> Signed-off-by: Michael Ellerman 
> ---
>  arch/powerpc/Makefile | 5 +
>  arch/powerpc/configs/book3s_32.config | 2 ++
>  2 files changed, 7 insertions(+)
>  create mode 100644 arch/powerpc/configs/book3s_32.config
>
> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> index 2ea575cb3401..2556c2182789 100644
> --- a/arch/powerpc/Makefile
> +++ b/arch/powerpc/Makefile
> @@ -354,6 +354,11 @@ mpc86xx_smp_defconfig:
> $(call merge_into_defconfig,mpc86xx_basic_defconfig,\
> 86xx-smp 86xx-hw fsl-emb-nonhw)
>
> +PHONY += ppc32_allmodconfig
> +ppc32_allmodconfig:
> +   $(Q)$(MAKE) 
> KCONFIG_ALLCONFIG=$(srctree)/arch/powerpc/configs/book3s_32.config \
> +   -f $(srctree)/Makefile allmodconfig
> +

I this a good time to update line 34 at the same time:

KBUILD_DEFCONFIG := $(shell uname -m)_defconfig

?

>  define archhelp
>@echo '* zImage  - Build default images selected by kernel config'
>@echo '  zImage.*- Compressed kernel image 
> (arch/$(ARCH)/boot/zImage.*)'
> diff --git a/arch/powerpc/configs/book3s_32.config 
> b/arch/powerpc/configs/book3s_32.config
> new file mode 100644
> index ..8721eb7b1294
> --- /dev/null
> +++ b/arch/powerpc/configs/book3s_32.config
> @@ -0,0 +1,2 @@
> +CONFIG_PPC64=n
> +CONFIG_PPC_BOOK3S_32=y
> --
> 2.14.1
>


Re: [PATCH 1/2] powerpc: Add ppc32_allmodconfig defconfig target

2018-07-11 Thread Mathieu Malaterre
On Tue, Jul 10, 2018 at 3:47 PM Michael Ellerman  wrote:
>
> Mathieu Malaterre  writes:
> > On Mon, Jul 9, 2018 at 4:24 PM Michael Ellerman  wrote:
> >>
> >> Because the allmodconfig logic just sets every symbol to M or Y, it
> >> has the effect of always generating a 64-bit config, because
> >> CONFIG_PPC64 becomes Y.
> >>
> >> So to make it easier for folks to test 32-bit code, provide a phony
> >> defconfig target that generates a 32-bit allmodconfig.
> >>
> >> The 32-bit port has several mutually exclusive CPU types, we choose
> >> the Book3S variants as that's what the help text in Kconfig says is
> >> most common.
> >
> > Ok then.
>
> That was just me taking a stab in the dark. You suggested we should
> mimic the Debian config, what does that use?

Sorry got confused for a minute. This is the correct value (at least
the one used in Debian).

> >> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
> >> index 2ea575cb3401..2556c2182789 100644
> >> --- a/arch/powerpc/Makefile
> >> +++ b/arch/powerpc/Makefile
> >> @@ -354,6 +354,11 @@ mpc86xx_smp_defconfig:
> >> $(call merge_into_defconfig,mpc86xx_basic_defconfig,\
> >> 86xx-smp 86xx-hw fsl-emb-nonhw)
> >>
> >> +PHONY += ppc32_allmodconfig
> >> +ppc32_allmodconfig:
> >> +   $(Q)$(MAKE) 
> >> KCONFIG_ALLCONFIG=$(srctree)/arch/powerpc/configs/book3s_32.config \
> >> +   -f $(srctree)/Makefile allmodconfig
> >> +
> >
> > I this a good time to update line 34 at the same time:
> >
> > KBUILD_DEFCONFIG := $(shell uname -m)_defconfig
> >
> > ?
>
> 34 or 36?
>
>   ifeq ($(CROSS_COMPILE),)
>   KBUILD_DEFCONFIG := $(shell uname -m)_defconfig
>   else
>   KBUILD_DEFCONFIG := ppc64_defconfig
>   endif
>
> Do you mean the else case?

As far as I know uname -m on powerpc returns 'ppc' so the following
has always failed from a ppc32be machine:

$ make ARCH=powerpc defconfig

I was simply suggesting to mimic what was done for ppc64:

ifeq ($(CROSS_COMPILE),)
KBUILD_DEFCONFIG := ppc32_defconfig
else
KBUILD_DEFCONFIG := ppc64_defconfig
endif

If I followed the discussion one would want the file `ppc32_defconfig`
to contains pretty much the same thing as the .config generated from
`book3s_32.config`, right ?

> cheers


  1   2   3   4   >