Re: [PATCH] Work around for enabling CONFIG_CMDLINE on ppc64le

2016-09-27 Thread Akshay Adiga 

Hi Michael,

Here is the link to the bug raised on launchpad.
https://bugs.launchpad.net/ubuntu/+source/gcc-5/+bug/1628207


On 09/23/2016 09:45 AM, Akshay Adiga wrote:

Hi Michael,

Anton found this bug and raised it against gcc v7.0 and a fix is 
available

 in upstream gcc.

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

Currently, gcc v5.4.0  and v6.1.1 shipped with Ubuntu 16.04 and 16.10  
respectively,

 are hitting this problem.

I have also raised bug against Ubuntu for fixing gcc for 16.04.

https://bugzilla.linux.ibm.com/show_bug.cgi?id=146668


On 09/22/2016 03:51 PM, Michael Ellerman wrote:

Akshay Adiga  writes:


Observed that boot arguments (passed as CONFIG_CMDLINE)  are not being
picked up by kernel while using gcc-ppc64-linux-gnu v5.4.0 and v6.1.1.
While it works as expected with v5.3.1 .

Found that in init/main.c in  setup_command_line() the pointers 
passed to

strcpy() is messed up.

Hi Akshay,

Thanks for debugging this.


The problem goes away when compiler optimization is restricted to -O1.
diff --git a/init/main.c b/init/main.c
index a8a58e2..4259c42 100644
--- a/init/main.c
+++ b/init/main.c
@@ -358,7 +358,13 @@ static inline void smp_prepare_cpus(unsigned 
int maxcpus) { }

   * parsing is performed in place, and we should allow a component to
   * store reference of name/value for future reference.
   */
-static void __init setup_command_line(char *command_line)
+static void __init
+#ifdef CONFIG_PPC64
+#if  GCC_VERSION > 50301
+__attribute__((optimize("-O1")))
+#endif
+#endif
+setup_command_line(char *command_line)
  {
  saved_command_line =
  memblock_virt_alloc(strlen(boot_command_line) + 1, 0);

But I can't merge that patch.

Our options are one or both of:
  - get GCC fixed and backport the fix to the compilers we care about.
  - blacklist the broken compiler versions.

Is there a GCC bug filed for this?

cheers

Re: [PATCH] Work around for enabling CONFIG_CMDLINE on ppc64le

2016-09-27 Thread Akshay Adiga 

Hi Michael,

Here is the link to the bug raised on launchpad.
https://bugs.launchpad.net/ubuntu/+source/gcc-5/+bug/1628207


On 09/23/2016 09:45 AM, Akshay Adiga wrote:

Hi Michael,

Anton found this bug and raised it against gcc v7.0 and a fix is 
available

 in upstream gcc.

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

Currently, gcc v5.4.0  and v6.1.1 shipped with Ubuntu 16.04 and 16.10  
respectively,

 are hitting this problem.

I have also raised bug against Ubuntu for fixing gcc for 16.04.

https://bugzilla.linux.ibm.com/show_bug.cgi?id=146668


On 09/22/2016 03:51 PM, Michael Ellerman wrote:

Akshay Adiga  writes:


Observed that boot arguments (passed as CONFIG_CMDLINE)  are not being
picked up by kernel while using gcc-ppc64-linux-gnu v5.4.0 and v6.1.1.
While it works as expected with v5.3.1 .

Found that in init/main.c in  setup_command_line() the pointers 
passed to

strcpy() is messed up.

Hi Akshay,

Thanks for debugging this.


The problem goes away when compiler optimization is restricted to -O1.
diff --git a/init/main.c b/init/main.c
index a8a58e2..4259c42 100644
--- a/init/main.c
+++ b/init/main.c
@@ -358,7 +358,13 @@ static inline void smp_prepare_cpus(unsigned 
int maxcpus) { }

   * parsing is performed in place, and we should allow a component to
   * store reference of name/value for future reference.
   */
-static void __init setup_command_line(char *command_line)
+static void __init
+#ifdef CONFIG_PPC64
+#if  GCC_VERSION > 50301
+__attribute__((optimize("-O1")))
+#endif
+#endif
+setup_command_line(char *command_line)
  {
  saved_command_line =
  memblock_virt_alloc(strlen(boot_command_line) + 1, 0);

But I can't merge that patch.

Our options are one or both of:
  - get GCC fixed and backport the fix to the compilers we care about.
  - blacklist the broken compiler versions.

Is there a GCC bug filed for this?

cheers

Re: [PATCH] Work around for enabling CONFIG_CMDLINE on ppc64le

2016-09-22 Thread Akshay Adiga 

Hi Michael,

Anton found this bug and raised it against gcc v7.0 and a fix is available
 in upstream gcc.

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

Currently, gcc v5.4.0  and v6.1.1 shipped with Ubuntu 16.04 and 16.10  
respectively,
 are hitting this problem.

I have also raised bug against Ubuntu for fixing gcc for 16.04.

https://bugzilla.linux.ibm.com/show_bug.cgi?id=146668


On 09/22/2016 03:51 PM, Michael Ellerman wrote:

Akshay Adiga  writes:


Observed that boot arguments (passed as CONFIG_CMDLINE)  are not being
picked up by kernel while using gcc-ppc64-linux-gnu v5.4.0 and v6.1.1.
While it works as expected with v5.3.1 .

Found that in init/main.c in  setup_command_line() the pointers passed to
strcpy() is messed up.

Hi Akshay,

Thanks for debugging this.


The problem goes away when compiler optimization is restricted to -O1.
diff --git a/init/main.c b/init/main.c
index a8a58e2..4259c42 100644
--- a/init/main.c
+++ b/init/main.c
@@ -358,7 +358,13 @@ static inline void smp_prepare_cpus(unsigned int maxcpus) 
{ }
   * parsing is performed in place, and we should allow a component to
   * store reference of name/value for future reference.
   */
-static void __init setup_command_line(char *command_line)
+static void __init
+#ifdef CONFIG_PPC64
+   #if  GCC_VERSION > 50301
+   __attribute__((optimize("-O1")))
+   #endif
+#endif
+   setup_command_line(char *command_line)
  {
saved_command_line =
memblock_virt_alloc(strlen(boot_command_line) + 1, 0);

But I can't merge that patch.

Our options are one or both of:
  - get GCC fixed and backport the fix to the compilers we care about.
  - blacklist the broken compiler versions.

Is there a GCC bug filed for this?

cheers

Re: [PATCH] Work around for enabling CONFIG_CMDLINE on ppc64le

2016-09-22 Thread Akshay Adiga 

Hi Michael,

Anton found this bug and raised it against gcc v7.0 and a fix is available
 in upstream gcc.

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

Currently, gcc v5.4.0  and v6.1.1 shipped with Ubuntu 16.04 and 16.10  
respectively,
 are hitting this problem.

I have also raised bug against Ubuntu for fixing gcc for 16.04.

https://bugzilla.linux.ibm.com/show_bug.cgi?id=146668


On 09/22/2016 03:51 PM, Michael Ellerman wrote:

Akshay Adiga  writes:


Observed that boot arguments (passed as CONFIG_CMDLINE)  are not being
picked up by kernel while using gcc-ppc64-linux-gnu v5.4.0 and v6.1.1.
While it works as expected with v5.3.1 .

Found that in init/main.c in  setup_command_line() the pointers passed to
strcpy() is messed up.

Hi Akshay,

Thanks for debugging this.


The problem goes away when compiler optimization is restricted to -O1.
diff --git a/init/main.c b/init/main.c
index a8a58e2..4259c42 100644
--- a/init/main.c
+++ b/init/main.c
@@ -358,7 +358,13 @@ static inline void smp_prepare_cpus(unsigned int maxcpus) 
{ }
   * parsing is performed in place, and we should allow a component to
   * store reference of name/value for future reference.
   */
-static void __init setup_command_line(char *command_line)
+static void __init
+#ifdef CONFIG_PPC64
+   #if  GCC_VERSION > 50301
+   __attribute__((optimize("-O1")))
+   #endif
+#endif
+   setup_command_line(char *command_line)
  {
saved_command_line =
memblock_virt_alloc(strlen(boot_command_line) + 1, 0);

But I can't merge that patch.

Our options are one or both of:
  - get GCC fixed and backport the fix to the compilers we care about.
  - blacklist the broken compiler versions.

Is there a GCC bug filed for this?

cheers

Re: [PATCH] Work around for enabling CONFIG_CMDLINE on ppc64le

2016-09-22 Thread Anton Blanchard 
Hi,

> But I can't merge that patch.
> 
> Our options are one or both of:
>  - get GCC fixed and backport the fix to the compilers we care about.
>  - blacklist the broken compiler versions.
> 
> Is there a GCC bug filed for this?

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

We need confirmation this patch fixes the 6.x issue too and that we need
a backport.

Anton

Re: [PATCH] Work around for enabling CONFIG_CMDLINE on ppc64le

2016-09-22 Thread Anton Blanchard 
Hi,

> But I can't merge that patch.
> 
> Our options are one or both of:
>  - get GCC fixed and backport the fix to the compilers we care about.
>  - blacklist the broken compiler versions.
> 
> Is there a GCC bug filed for this?

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

We need confirmation this patch fixes the 6.x issue too and that we need
a backport.

Anton

Re: [PATCH] Work around for enabling CONFIG_CMDLINE on ppc64le

2016-09-22 Thread Michael Ellerman 
Akshay Adiga  writes:

> Observed that boot arguments (passed as CONFIG_CMDLINE)  are not being
> picked up by kernel while using gcc-ppc64-linux-gnu v5.4.0 and v6.1.1.
> While it works as expected with v5.3.1 .
>
> Found that in init/main.c in  setup_command_line() the pointers passed to
> strcpy() is messed up.

Hi Akshay,

Thanks for debugging this.

> The problem goes away when compiler optimization is restricted to -O1.

> diff --git a/init/main.c b/init/main.c
> index a8a58e2..4259c42 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -358,7 +358,13 @@ static inline void smp_prepare_cpus(unsigned int 
> maxcpus) { }
>   * parsing is performed in place, and we should allow a component to
>   * store reference of name/value for future reference.
>   */
> -static void __init setup_command_line(char *command_line)
> +static void __init
> +#ifdef CONFIG_PPC64
> + #if  GCC_VERSION > 50301
> + __attribute__((optimize("-O1")))
> + #endif
> +#endif
> + setup_command_line(char *command_line)
>  {
>   saved_command_line =
>   memblock_virt_alloc(strlen(boot_command_line) + 1, 0);

But I can't merge that patch.

Our options are one or both of:
 - get GCC fixed and backport the fix to the compilers we care about.
 - blacklist the broken compiler versions.

Is there a GCC bug filed for this?

cheers

Re: [PATCH] Work around for enabling CONFIG_CMDLINE on ppc64le

2016-09-22 Thread Michael Ellerman 
Akshay Adiga  writes:

> Observed that boot arguments (passed as CONFIG_CMDLINE)  are not being
> picked up by kernel while using gcc-ppc64-linux-gnu v5.4.0 and v6.1.1.
> While it works as expected with v5.3.1 .
>
> Found that in init/main.c in  setup_command_line() the pointers passed to
> strcpy() is messed up.

Hi Akshay,

Thanks for debugging this.

> The problem goes away when compiler optimization is restricted to -O1.

> diff --git a/init/main.c b/init/main.c
> index a8a58e2..4259c42 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -358,7 +358,13 @@ static inline void smp_prepare_cpus(unsigned int 
> maxcpus) { }
>   * parsing is performed in place, and we should allow a component to
>   * store reference of name/value for future reference.
>   */
> -static void __init setup_command_line(char *command_line)
> +static void __init
> +#ifdef CONFIG_PPC64
> + #if  GCC_VERSION > 50301
> + __attribute__((optimize("-O1")))
> + #endif
> +#endif
> + setup_command_line(char *command_line)
>  {
>   saved_command_line =
>   memblock_virt_alloc(strlen(boot_command_line) + 1, 0);

But I can't merge that patch.

Our options are one or both of:
 - get GCC fixed and backport the fix to the compilers we care about.
 - blacklist the broken compiler versions.

Is there a GCC bug filed for this?

cheers

[PATCH] Work around for enabling CONFIG_CMDLINE on ppc64le

2016-09-22 Thread Akshay Adiga 
Observed that boot arguments (passed as CONFIG_CMDLINE)  are not being
picked up by kernel while using gcc-ppc64-linux-gnu v5.4.0 and v6.1.1.
While it works as expected with v5.3.1 .

Found that in init/main.c in  setup_command_line() the pointers passed to
strcpy() is messed up.

source for setup_command_line from init/main.c:
void setup_command_line(char *command_line)
{
saved_command_line =
memblock_virt_alloc(strlen(boot_command_line) + 1, 0);
initcall_command_line =
memblock_virt_alloc(strlen(boot_command_line) + 1, 0);
static_command_line = memblock_virt_alloc(strlen(command_line) + 1, 0);
strcpy(saved_command_line, boot_command_line);
strcpy(static_command_line, command_line);
}

Following is the asm dump for strcpy:

char *strcpy(char *dest, const char *src)
{
c0161408:   ff ff 84 38 addir4,r4,-1
c016140c:   ff ff 43 39 addir10,r3,-1
char *tmp = dest;

while ((*dest++ = *src++) != '\0')
c0161410:   01 00 24 8d lbzur9,1(r4)
c0161414:   00 00 a9 2f cmpdi   cr7,r9,0
c0161418:   01 00 2a 9d stbur9,1(r10)
c016141c:   f4 ff 9e 40 bne cr7,c0161410

/* nothing */;
return tmp;
}

Following are the asm dump for the working and non working binaries which
concluded that the argument for the second strcpy() is not loaded into r3 and
is getting clobbered with the return value of previous strcpy().

Not Working asm dump :

c03308d8:   38 c4 6a f8 std r3,-15304(r10)
strcpy(saved_command_line, boot_command_line);
c03308dc:   06 00 62 3c addis   r3,r2,6
c03308e0:   28 c4 63 e8 ld  r3,-15320(r3)
c03308e4:   25 0b e3 4b bl  c0161408

c03308e8:   00 00 00 60 nop
strcpy(static_command_line, command_line);
c03308ec:   78 f3 c4 7f mr  r4,r30
c03308f0:   19 0b e3 4b bl  c0161408

c03308f4:   00 00 00 60 nop

Working asm dump :

c03308d4:   38 c4 c3 fb std r30,-15304(r3)
strcpy(saved_command_line, boot_command_line);
c03308d8:   06 00 62 3c addis   r3,r2,6
c03308dc:   28 c4 63 e8 ld  r3,-15320(r3)
c03308e0:   6d 08 e3 4b bl  c016114c

c03308e4:   00 00 00 60 nop
strcpy(static_command_line, command_line);
c03308e8:   78 eb a4 7f mr  r4,r29
c03308ec:   78 f3 c3 7f mr  r3,r30
c03308f0:   5d 08 e3 4b bl  c016114c

c03308f4:   00 00 00 60 nop

The problem goes away when compiler optimization is restricted to -O1.

Reported-by: Madhavan Srinivasan 
Signed-off-by: Akshay Adiga 
---
 init/main.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/init/main.c b/init/main.c
index a8a58e2..4259c42 100644
--- a/init/main.c
+++ b/init/main.c
@@ -358,7 +358,13 @@ static inline void smp_prepare_cpus(unsigned int maxcpus) 
{ }
  * parsing is performed in place, and we should allow a component to
  * store reference of name/value for future reference.
  */
-static void __init setup_command_line(char *command_line)
+static void __init
+#ifdef CONFIG_PPC64
+   #if  GCC_VERSION > 50301
+   __attribute__((optimize("-O1")))
+   #endif
+#endif
+   setup_command_line(char *command_line)
 {
saved_command_line =
memblock_virt_alloc(strlen(boot_command_line) + 1, 0);
-- 
2.5.5

[PATCH] Work around for enabling CONFIG_CMDLINE on ppc64le

2016-09-22 Thread Akshay Adiga 
Observed that boot arguments (passed as CONFIG_CMDLINE)  are not being
picked up by kernel while using gcc-ppc64-linux-gnu v5.4.0 and v6.1.1.
While it works as expected with v5.3.1 .

Found that in init/main.c in  setup_command_line() the pointers passed to
strcpy() is messed up.

source for setup_command_line from init/main.c:
void setup_command_line(char *command_line)
{
saved_command_line =
memblock_virt_alloc(strlen(boot_command_line) + 1, 0);
initcall_command_line =
memblock_virt_alloc(strlen(boot_command_line) + 1, 0);
static_command_line = memblock_virt_alloc(strlen(command_line) + 1, 0);
strcpy(saved_command_line, boot_command_line);
strcpy(static_command_line, command_line);
}

Following is the asm dump for strcpy:

char *strcpy(char *dest, const char *src)
{
c0161408:   ff ff 84 38 addir4,r4,-1
c016140c:   ff ff 43 39 addir10,r3,-1
char *tmp = dest;

while ((*dest++ = *src++) != '\0')
c0161410:   01 00 24 8d lbzur9,1(r4)
c0161414:   00 00 a9 2f cmpdi   cr7,r9,0
c0161418:   01 00 2a 9d stbur9,1(r10)
c016141c:   f4 ff 9e 40 bne cr7,c0161410

/* nothing */;
return tmp;
}

Following are the asm dump for the working and non working binaries which
concluded that the argument for the second strcpy() is not loaded into r3 and
is getting clobbered with the return value of previous strcpy().

Not Working asm dump :

c03308d8:   38 c4 6a f8 std r3,-15304(r10)
strcpy(saved_command_line, boot_command_line);
c03308dc:   06 00 62 3c addis   r3,r2,6
c03308e0:   28 c4 63 e8 ld  r3,-15320(r3)
c03308e4:   25 0b e3 4b bl  c0161408

c03308e8:   00 00 00 60 nop
strcpy(static_command_line, command_line);
c03308ec:   78 f3 c4 7f mr  r4,r30
c03308f0:   19 0b e3 4b bl  c0161408

c03308f4:   00 00 00 60 nop

Working asm dump :

c03308d4:   38 c4 c3 fb std r30,-15304(r3)
strcpy(saved_command_line, boot_command_line);
c03308d8:   06 00 62 3c addis   r3,r2,6
c03308dc:   28 c4 63 e8 ld  r3,-15320(r3)
c03308e0:   6d 08 e3 4b bl  c016114c

c03308e4:   00 00 00 60 nop
strcpy(static_command_line, command_line);
c03308e8:   78 eb a4 7f mr  r4,r29
c03308ec:   78 f3 c3 7f mr  r3,r30
c03308f0:   5d 08 e3 4b bl  c016114c

c03308f4:   00 00 00 60 nop

The problem goes away when compiler optimization is restricted to -O1.

Reported-by: Madhavan Srinivasan 
Signed-off-by: Akshay Adiga 
---
 init/main.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/init/main.c b/init/main.c
index a8a58e2..4259c42 100644
--- a/init/main.c
+++ b/init/main.c
@@ -358,7 +358,13 @@ static inline void smp_prepare_cpus(unsigned int maxcpus) 
{ }
  * parsing is performed in place, and we should allow a component to
  * store reference of name/value for future reference.
  */
-static void __init setup_command_line(char *command_line)
+static void __init
+#ifdef CONFIG_PPC64
+   #if  GCC_VERSION > 50301
+   __attribute__((optimize("-O1")))
+   #endif
+#endif
+   setup_command_line(char *command_line)
 {
saved_command_line =
memblock_virt_alloc(strlen(boot_command_line) + 1, 0);
-- 
2.5.5