[PATCH] ppc/ppc64: Compile purgatory code with gcc option -msoft-float

[PATCH] ppc/ppc64: Compile purgatory code with gcc option -msoft-float

[Vivek Goyal]

Recently we faced an issue on power7 machine where kernel hanged in purgatory.
Some investigation revealed that gcc is generating hardware FPU instructions.
I have been told we can't use it at this point of time and as kernel is
compiled with -msoft-float for ppc/ppc64, so should be purgatory (as it runs
inside kernel context).

Thanks to Jakub Jelinek and Lingzhu Xiang for debugging and coming up with
a fix for this issue.

Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Tested-by: Lingzhu Xiang <lxiang@redhat.com>
Tested-by: Suzuki K. Poulose <suzuki@in.ibm.com> 
---
 purgatory/arch/ppc/Makefile   |    2 ++
 purgatory/arch/ppc64/Makefile |    2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

Index: kexec-tools/purgatory/arch/ppc64/Makefile
===================================================================
--- kexec-tools.orig/purgatory/arch/ppc64/Makefile	2012-02-20 14:45:25.000000000 -0500
+++ kexec-tools/purgatory/arch/ppc64/Makefile	2012-06-06 16:19:09.000696306 -0400
@@ -9,7 +9,7 @@ ppc64_PURGATORY_SRCS += purgatory/arch/p
 ppc64_PURGATORY_SRCS += purgatory/arch/ppc64/crashdump_backup.c
 ppc64_PURGATORY_SRCS += purgatory/arch/ppc64/misc.S
 
-ppc64_PURGATORY_EXTRA_CFLAGS += -m64 -mcall-aixdesc
+ppc64_PURGATORY_EXTRA_CFLAGS += -m64 -mcall-aixdesc -msoft-float
 ppc64_PURGATORY_EXTRA_ASFLAGS += -m64 -mcall-aixdesc
 ppc64_PURGATORY_EXTRA_LDFLAGS += -melf64ppc
 
Index: kexec-tools/purgatory/arch/ppc/Makefile
===================================================================
--- kexec-tools.orig/purgatory/arch/ppc/Makefile	2011-03-18 17:09:38.000000000 -0400
+++ kexec-tools/purgatory/arch/ppc/Makefile	2012-06-06 16:33:50.444406235 -0400
@@ -7,6 +7,8 @@ ppc_PURGATORY_SRCS += purgatory/arch/ppc
 ppc_PURGATORY_SRCS += purgatory/arch/ppc/purgatory-ppc.c
 ppc_PURGATORY_SRCS += purgatory/arch/ppc/console-ppc.c
 
+ppc_PURGATORY_EXTRA_CFLAGS += -msoft-float
+
 dist += purgatory/arch/ppc/Makefile $(ppc_PURGATORY_SRCS)		\
 	purgatory/arch/ppc/purgatory-ppc.h purgatory/arch/ppc/ppc_asm.h
 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] ppc/ppc64: Compile purgatory code with gcc option -msoft-float

[Vivek Goyal]

On Fri, Jun 08, 2012 at 03:48:01PM +0200, Jakub Jelinek wrote:
> On Fri, Jun 08, 2012 at 09:43:15AM -0400, Vivek Goyal wrote:
> > Recently we faced an issue on power7 machine where kernel hanged in purgatory.
> > Some investigation revealed that gcc is generating hardware FPU instructions.
> > I have been told we can't use it at this point of time and as kernel is
> > compiled with -msoft-float for ppc/ppc64, so should be purgatory (as it runs
> > inside kernel context).
> > 
> > Thanks to Jakub Jelinek and Lingzhu Xiang for debugging and coming up with
> > a fix for this issue.
> 
> IMNSHO as a follow-up all other architectures should be changed to
> compile kernel code with the right kernel CFLAGS (e.g. -mcmodel=kernel
> -mno-sse -mno-mmx should be used probably on x86_64, etc.).

Sure. These changes can be done in separate patches.

BTW, What does -mcmodel do and why should we use -mcmodel? purgatory, is a
separate binary which we reloate at turn time based on physical memory
location. IOW, why should we make any assumptions about running location
of purgatory and pass -mcmodel flag.

Eric, would have have any objections to making use of above gcc flags
for purgatory?

> 
> Furthermore, is there still a reason to compile sha256.c with -O0?  If
> there was a compiler bug? on ia64 6 years ago, with which gcc version
> it was, has the problem been reported and resolved?  Compiling code like
> hash computation unconditionally everywhere at -O0 must IMHO pessimize
> things unnecessarily.

git blame points to following commit for forcing -O0 on sha256. CCing
Jermey, if we can give some details why -O2 was a problem on IA6 and
if problem is still there if he has objections if we swtich back to -O2.

commit adf85cbc1756e56c1f52f56235a3c08b3c5b8e05
Author: Jeremy Kerr <jk@ozlabs.org>
Date:   Thu Dec 13 16:18:53 2007 +0900

    Build system simplification/standardisation
    
Thanks
Vivek

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] ppc/ppc64: Compile purgatory code with gcc option -msoft-float

[Vivek Goyal]

On Fri, Jun 08, 2012 at 04:51:13PM +0200, Jakub Jelinek wrote:
> On Fri, Jun 08, 2012 at 10:39:49AM -0400, Vivek Goyal wrote:
> > BTW, What does -mcmodel do and why should we use -mcmodel? purgatory, is a
> > separate binary which we reloate at turn time based on physical memory
> > location. IOW, why should we make any assumptions about running location
> > of purgatory and pass -mcmodel flag.
> 
> The default x86_64 -mcmodel=small requires that the binary (and all
> corresponding data (rodata etc.) sections must be within the low 2GB of
> address space.  Kernel code is usually in the topmost 2GB of address space,
> and -mcmodel=kernel is the memory model for that (all code and all other
> sections must be in topmost 2GB of address space).  Of course pointers
> are 64-bit in both cases, so any memory you access solely through pointers,
> not through symbols, can be anywhere.  Haven't looked at where purgatory
> relocates itself into the address space.

IIRC, by the time purgatory runs we turn the paging off and we are
operating in protected mode. And by that time purgatory is running from
low 2GB of address space.

It is relocated by kexec-tools itself before it calls kernel. So in
user space we decide a physical address where purgatory can be run
from. Relocate this binary so that it can run from that address and
then call kernel to load this binary at this address. (Kernel might
store the binary at some other location but it is moved to it final
destination just before kexec and just before we jump to it).

Relocation happens here.

kexec-tools/kexec/arch/i386/kexec-bzImage.c

***********************************************************************

        /* Load the trampoline.  This must load at a higher address
         * the the argument/parameter segment or the kernel will stomp
         * it's gdt.
         *
         * x86_64 purgatory code has got relocations type R_X86_64_32S
         * that means purgatory got to be loaded within first 2G otherwise
         * overflow takes place while applying relocations.
         */
        if (!real_mode_entry && relocatable_kernel)
                elf_rel_build_load(info, &info->rhdr, purgatory,
purgatory_size,
                                        0x3000, 0x7fffffff, -1, 0);
        else
                elf_rel_build_load(info, &info->rhdr, purgatory,
purgatory_size,
                                        0x3000, 640*1024, -1, 0);
        dbgprintf("Loaded purgatory at addr 0x%lx\n",
info->rhdr.rel_addr);

*************************************************************************

So at this point of time, it does not look like that we should be passing
-mcmodel for purgatory code.

> 
> -mno-sse -mno-mmx will tell gcc not to use any %mmX or %xmmX registers.
> i?86 also passes -msoft-float in addition to -mno-sse -mno-mmx I think.
> 

Yes, i386 is also passing -msoft-float in kernel. Personally I have no
issues in adding -mno-sse and -mno-mmx and -msoft-float for i386. It is
that kind of thing that one gets the motivation to change these flags
once something breaks down.

[..]
> > > hash computation unconditionally everywhere at -O0 must IMHO pessimize
> > > things unnecessarily.
> > 
> > git blame points to following commit for forcing -O0 on sha256. CCing
> > Jermey, if we can give some details why -O2 was a problem on IA6 and
> > if problem is still there if he has objections if we swtich back to -O2.
> > 
> > commit adf85cbc1756e56c1f52f56235a3c08b3c5b8e05
> > Author: Jeremy Kerr <jk@ozlabs.org>
> > Date:   Thu Dec 13 16:18:53 2007 +0900
> > 
> >     Build system simplification/standardisation
> 
> I think the change was even older, this one just moved the -O0 CFLAGS from
> one spot to a different one.

You are right. Here is the original change.

commit e90343c521b68237c17ea4e4ed376c4a726f0789
Author: Simon Horman <horms@verge.net.au>
Date:   Fri Oct 6 12:27:58 2006 +0900

    build-util_lib-optimisation-flags
    
    [BUILD] Make sure sha256.c is compiled without optimisation


It is almost 6 years old now. Simon, can it be changed to -O2 now?

Thanks
Vivek

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] ppc/ppc64: Compile purgatory code with gcc option -msoft-float

[Eric W. Biederman]

Vivek Goyal <vgoyal@redhat.com> writes:

> On Fri, Jun 08, 2012 at 04:51:13PM +0200, Jakub Jelinek wrote:
>> On Fri, Jun 08, 2012 at 10:39:49AM -0400, Vivek Goyal wrote:
>> > BTW, What does -mcmodel do and why should we use -mcmodel? purgatory, is a
>> > separate binary which we reloate at turn time based on physical memory
>> > location. IOW, why should we make any assumptions about running location
>> > of purgatory and pass -mcmodel flag.
>> 
>> The default x86_64 -mcmodel=small requires that the binary (and all
>> corresponding data (rodata etc.) sections must be within the low 2GB of
>> address space.  Kernel code is usually in the topmost 2GB of address space,
>> and -mcmodel=kernel is the memory model for that (all code and all other
>> sections must be in topmost 2GB of address space).  Of course pointers
>> are 64-bit in both cases, so any memory you access solely through pointers,
>> not through symbols, can be anywhere.  Haven't looked at where purgatory
>> relocates itself into the address space.
>
> IIRC, by the time purgatory runs we turn the paging off and we are
> operating in protected mode. And by that time purgatory is running from
> low 2GB of address space.
>
> It is relocated by kexec-tools itself before it calls kernel. So in
> user space we decide a physical address where purgatory can be run
> from. Relocate this binary so that it can run from that address and
> then call kernel to load this binary at this address. (Kernel might
> store the binary at some other location but it is moved to it final
> destination just before kexec and just before we jump to it).
>
> Relocation happens here.
>
> kexec-tools/kexec/arch/i386/kexec-bzImage.c
>
> ***********************************************************************
>
>         /* Load the trampoline.  This must load at a higher address
>          * the the argument/parameter segment or the kernel will stomp
>          * it's gdt.
>          *
>          * x86_64 purgatory code has got relocations type R_X86_64_32S
>          * that means purgatory got to be loaded within first 2G otherwise
>          * overflow takes place while applying relocations.
>          */
>         if (!real_mode_entry && relocatable_kernel)
>                 elf_rel_build_load(info, &info->rhdr, purgatory,
> purgatory_size,
>                                         0x3000, 0x7fffffff, -1, 0);
>         else
>                 elf_rel_build_load(info, &info->rhdr, purgatory,
> purgatory_size,
>                                         0x3000, 640*1024, -1, 0);
>         dbgprintf("Loaded purgatory at addr 0x%lx\n",
> info->rhdr.rel_addr);
>
> *************************************************************************
>
> So at this point of time, it does not look like that we should be passing
> -mcmodel for purgatory code.

I agree.  The current purgatory code very much makes sense with
-mcmodel=small.  However ideally purgatory would be compiled like a
shared library and able to live anywhere in the lower half of the
64bit address space.  

>> -mno-sse -mno-mmx will tell gcc not to use any %mmX or %xmmX registers.
>> i?86 also passes -msoft-float in addition to -mno-sse -mno-mmx I think.
>> 
>
> Yes, i386 is also passing -msoft-float in kernel. Personally I have no
> issues in adding -mno-sse and -mno-mmx and -msoft-float for i386. It is
> that kind of thing that one gets the motivation to change these flags
> once something breaks down.

I didn't think there was enough code in purgatory for there to be these
kinds of issues but we should definitely do something like this.  If
there was actually an advantage to using sse or mmx we might actually be
able to use them on x86_64 as we don't have the kernels task switching
reasons not to use them.  But still it is probably easier to tell gcc
not to bother.

Eric


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] ppc/ppc64: Compile purgatory code with gcc option -msoft-float

[Simon Horman]

On Fri, Jun 08, 2012 at 11:19:36AM -0400, Vivek Goyal wrote:
> On Fri, Jun 08, 2012 at 04:51:13PM +0200, Jakub Jelinek wrote:
> > On Fri, Jun 08, 2012 at 10:39:49AM -0400, Vivek Goyal wrote:
> > > BTW, What does -mcmodel do and why should we use -mcmodel? purgatory, is a
> > > separate binary which we reloate at turn time based on physical memory
> > > location. IOW, why should we make any assumptions about running location
> > > of purgatory and pass -mcmodel flag.
> > 
> > The default x86_64 -mcmodel=small requires that the binary (and all
> > corresponding data (rodata etc.) sections must be within the low 2GB of
> > address space.  Kernel code is usually in the topmost 2GB of address space,
> > and -mcmodel=kernel is the memory model for that (all code and all other
> > sections must be in topmost 2GB of address space).  Of course pointers
> > are 64-bit in both cases, so any memory you access solely through pointers,
> > not through symbols, can be anywhere.  Haven't looked at where purgatory
> > relocates itself into the address space.
> 
> IIRC, by the time purgatory runs we turn the paging off and we are
> operating in protected mode. And by that time purgatory is running from
> low 2GB of address space.
> 
> It is relocated by kexec-tools itself before it calls kernel. So in
> user space we decide a physical address where purgatory can be run
> from. Relocate this binary so that it can run from that address and
> then call kernel to load this binary at this address. (Kernel might
> store the binary at some other location but it is moved to it final
> destination just before kexec and just before we jump to it).
> 
> Relocation happens here.
> 
> kexec-tools/kexec/arch/i386/kexec-bzImage.c
> 
> ***********************************************************************
> 
>         /* Load the trampoline.  This must load at a higher address
>          * the the argument/parameter segment or the kernel will stomp
>          * it's gdt.
>          *
>          * x86_64 purgatory code has got relocations type R_X86_64_32S
>          * that means purgatory got to be loaded within first 2G otherwise
>          * overflow takes place while applying relocations.
>          */
>         if (!real_mode_entry && relocatable_kernel)
>                 elf_rel_build_load(info, &info->rhdr, purgatory,
> purgatory_size,
>                                         0x3000, 0x7fffffff, -1, 0);
>         else
>                 elf_rel_build_load(info, &info->rhdr, purgatory,
> purgatory_size,
>                                         0x3000, 640*1024, -1, 0);
>         dbgprintf("Loaded purgatory at addr 0x%lx\n",
> info->rhdr.rel_addr);
> 
> *************************************************************************
> 
> So at this point of time, it does not look like that we should be passing
> -mcmodel for purgatory code.
> 
> > 
> > -mno-sse -mno-mmx will tell gcc not to use any %mmX or %xmmX registers.
> > i?86 also passes -msoft-float in addition to -mno-sse -mno-mmx I think.
> > 
> 
> Yes, i386 is also passing -msoft-float in kernel. Personally I have no
> issues in adding -mno-sse and -mno-mmx and -msoft-float for i386. It is
> that kind of thing that one gets the motivation to change these flags
> once something breaks down.
> 
> [..]
> > > > hash computation unconditionally everywhere at -O0 must IMHO pessimize
> > > > things unnecessarily.
> > > 
> > > git blame points to following commit for forcing -O0 on sha256. CCing
> > > Jermey, if we can give some details why -O2 was a problem on IA6 and
> > > if problem is still there if he has objections if we swtich back to -O2.
> > > 
> > > commit adf85cbc1756e56c1f52f56235a3c08b3c5b8e05
> > > Author: Jeremy Kerr <jk@ozlabs.org>
> > > Date:   Thu Dec 13 16:18:53 2007 +0900
> > > 
> > >     Build system simplification/standardisation
> > 
> > I think the change was even older, this one just moved the -O0 CFLAGS from
> > one spot to a different one.
> 
> You are right. Here is the original change.
> 
> commit e90343c521b68237c17ea4e4ed376c4a726f0789
> Author: Simon Horman <horms@verge.net.au>
> Date:   Fri Oct 6 12:27:58 2006 +0900
> 
>     build-util_lib-optimisation-flags
>     
>     [BUILD] Make sure sha256.c is compiled without optimisation
> 
> 
> It is almost 6 years old now. Simon, can it be changed to -O2 now?

I have no objections.

I will add your original patch. Do you want to provide a follow-up patch
as well?

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec