Re: RFC: x86: kill binutils 2.16.x?

Re: RFC: x86: kill binutils 2.16.x?

[H. Peter Anvin]

On 03/02/2011 01:17 PM, Thomas Gleixner wrote:
> On Wed, 2 Mar 2011, H. Peter Anvin wrote:
> 
>> On 03/02/2011 12:54 PM, Thomas Gleixner wrote:
>>>
>>> Please lets get rid of known to be broken shite instead of trying to
>>> work around it for no good reasons. How old is that crap again ?
>>>
>>
>> 2005-2006.
> 
> Time enough even for enterprise folks to sort that out. Please kill it
> ASAP.
> 
> Thanks,
> 

kbuild people: is there a way to test for a specific assembler version
in Kbuild (and error out the build for it?)

	-hpa

Re: RFC: x86: kill binutils 2.16.x?

[Ingo Molnar]

* H. Peter Anvin <hpa@zytor.com> wrote:

> On 03/02/2011 01:17 PM, Thomas Gleixner wrote:
> > On Wed, 2 Mar 2011, H. Peter Anvin wrote:
> > 
> >> On 03/02/2011 12:54 PM, Thomas Gleixner wrote:
> >>>
> >>> Please lets get rid of known to be broken shite instead of trying to
> >>> work around it for no good reasons. How old is that crap again ?
> >>>
> >>
> >> 2005-2006.
> > 
> > Time enough even for enterprise folks to sort that out. Please kill it
> > ASAP.
> > 
> > Thanks,
> > 
> 
> kbuild people: is there a way to test for a specific assembler version
> in Kbuild (and error out the build for it?)

Could we add a testcase for one of the more egregious breakages and bail out then? 
That way we don't have to get the version information right - broken prereleases 
would be covered as well.

For example this sequence:

.irp idx,0,1,2
.if 0 > \idx
.endif
.endr

Will break on 2.16, right? It builds fine on 2.20.

This is how specific GAS functionality is tested in arch/powerpc:

        @if ! /bin/echo dssall | $(AS) -many -o $(TOUT) >/dev/null 2>&1 ; then \
                echo -n '*** ${VERSION}.${PATCHLEVEL} kernels no longer build ' ; \
                echo 'correctly with old versions of binutils.' ; \
                echo '*** Please upgrade your binutils to 2.12.1 or newer' ; \
                false ; \
        fi

This would also be a 'constructive' (and safest) way of blacklisting binutils: we'd 
really only exclude binutils that is truly buggy.

Thanks,

	Ingo

Re: RFC: x86: kill binutils 2.16.x?

[Kyle Moffett]

On Thu, Mar 3, 2011 at 03:30, Ingo Molnar <mingo@elte.hu> wrote:
> This is how specific GAS functionality is tested in arch/powerpc:
>
>        @if ! /bin/echo dssall | $(AS) -many -o $(TOUT) >/dev/null 2>&1 ; then \
>                echo -n '*** ${VERSION}.${PATCHLEVEL} kernels no longer build ' ; \
>                echo 'correctly with old versions of binutils.' ; \
>                echo '*** Please upgrade your binutils to 2.12.1 or newer' ; \
>                false ; \
>        fi
>
> This would also be a 'constructive' (and safest) way of blacklisting binutils: we'd
> really only exclude binutils that is truly buggy.

Hrm... well... actually...

It's funny that you brought up this particular case.  While I agree
that it's good in general, it's causing problems for me building a
kernel using a recent e500 gcc/binutils (triplet
"powerpc-linux-gnuspe").

Specifically the e500 doesn't have a normal PowerPC FPU, it has a
custom FPU built using extended integer registers instead, and it
happens to borrow the AltiVec opcode range to do it.

When trying to port Debian to the platform we were getting SIGILL's
all over the place until binutils got updated to reject all of the
unsupported opcodes on this particular platform.  Now of course we get
build errors, but that's a lot easier to debug and fix. :-D

Basically, binutils no longer supports "-many" (because too many
opcodes conflict), and the test itself would fail anyways (because
"dssall" is not valid on "any" PowerPC).

So while I think that it is entirely reasonable to add a similar test
for buggy x86 binutils, I'm actually about to send a patch to remove
that particular check from the powerpc Makefile.  Since the "required"
binutils 2.12.1 was released in May 2002 (almost 9 years ago) it's
probably not even worth testing for anymore.

Cheers,
Kyle Moffett

Re: RFC: x86: kill binutils 2.16.x?

[Benjamin Herrenschmidt]

On Tue, 2011-03-08 at 14:57 -0500, Kyle Moffett wrote:
> 
> Specifically the e500 doesn't have a normal PowerPC FPU, it has a
> custom FPU built using extended integer registers instead, and it
> happens to borrow the AltiVec opcode range to do it.
> 
> When trying to port Debian to the platform we were getting SIGILL's
> all over the place until binutils got updated to reject all of the
> unsupported opcodes on this particular platform.  Now of course we get
> build errors, but that's a lot easier to debug and fix. :-D
> 
> Basically, binutils no longer supports "-many" (because too many
> opcodes conflict), and the test itself would fail anyways (because
> "dssall" is not valid on "any" PowerPC). 

Note that this freescale "SPE" fiasco is just that ... a fiasco :-) I
don't think there's that many cases of opcode overlap outside of it.

Now regarding the kernel, the best is probably for nasty cases like that
to use hand coded opcodes (see ppc-opcodes.h) and stick to a more
"generic" setting for binutils, since it should be possible to build
kernels that support multiple types of BookE CPUs with different
floating point units.

Cheers,
Ben.

Re: RFC: x86: kill binutils 2.16.x?

[Scott Wood]

On Wed, 9 Mar 2011 08:28:36 +1100
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Tue, 2011-03-08 at 14:57 -0500, Kyle Moffett wrote:
> > 
> > Specifically the e500 doesn't have a normal PowerPC FPU, it has a
> > custom FPU built using extended integer registers instead, and it
> > happens to borrow the AltiVec opcode range to do it.
> > 
> > When trying to port Debian to the platform we were getting SIGILL's
> > all over the place until binutils got updated to reject all of the
> > unsupported opcodes on this particular platform.  Now of course we get
> > build errors, but that's a lot easier to debug and fix. :-D
> > 
> > Basically, binutils no longer supports "-many" (because too many
> > opcodes conflict), and the test itself would fail anyways (because
> > "dssall" is not valid on "any" PowerPC). 
> 
> Note that this freescale "SPE" fiasco is just that ... a fiasco :-) I
> don't think there's that many cases of opcode overlap outside of it.
> 
> Now regarding the kernel, the best is probably for nasty cases like that
> to use hand coded opcodes (see ppc-opcodes.h) and stick to a more
> "generic" setting for binutils, since it should be possible to build
> kernels that support multiple types of BookE CPUs with different
> floating point units.

Combined kernels between e500v1/2 and e500mc are currently not supported
for other reasons (current kconfig doesn't prohibit it, but it doesn't
work), such as a cache line size difference (a hardcoded L1_CACHE_BYTES is
used in various places for loops), an inability to enable SPE when e500mc
is enabled, etc.

Kumar recently internally said we're not going to bother making it work.
I'm inclined to agree, given you can't even run the same userspace (unless
you don't use hard floating point at all).  It's one thing to not want to
require a separate kernel for each board, but there's a point of
diminishing returns.

-Scott

Re: RFC: x86: kill binutils 2.16.x?

[Kyle Moffett]

On Tue, Mar 8, 2011 at 16:28, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Tue, 2011-03-08 at 14:57 -0500, Kyle Moffett wrote:
>> Specifically the e500 doesn't have a normal PowerPC FPU, it has a
>> custom FPU built using extended integer registers instead, and it
>> happens to borrow the AltiVec opcode range to do it.
>>
>> When trying to port Debian to the platform we were getting SIGILL's
>> all over the place until binutils got updated to reject all of the
>> unsupported opcodes on this particular platform.  Now of course we get
>> build errors, but that's a lot easier to debug and fix. :-D
>>
>> Basically, binutils no longer supports "-many" (because too many
>> opcodes conflict), and the test itself would fail anyways (because
>> "dssall" is not valid on "any" PowerPC).
>
> Note that this freescale "SPE" fiasco is just that ... a fiasco :-) I
> don't think there's that many cases of opcode overlap outside of it.

I absolutely agree on the "fiasco" part, although I'm pretty sure that
there's a large number of incompatible ARM variants (even 16-bit vs.
32-bit opcodes).  Unfortunately there's a lot of shipped hardware to
be supported and maintained...


> Now regarding the kernel, the best is probably for nasty cases like that
> to use hand coded opcodes (see ppc-opcodes.h) and stick to a more
> "generic" setting for binutils, since it should be possible to build
> kernels that support multiple types of BookE CPUs with different
> floating point units.

The problem is not with the kernel compile itself, but with the 2.12
"dssall" binutils test.  Basically, recent binutils treats e500 as
effectively a separate architecture that happens to share *most* of
the opcodes with regular PowerPC.  Any opcode which is not understood
by the e500 chip is either convert to an equivalent opcode which is
understood (IE: lwsync => sync), or failed with an error.  This means
that the kernel compile aborts early telling me to upgrade to a newer
version of binutils.

This was *critical* for getting an actual Debian distribution
bootstrapped on the e500 cores, because so much software assumes
PowerPC == altivec (ffmpeg), hardcodes 'asm("lwsync")' for memory
barriers (80+ packages in Debian), or includes hand-coded
floating-point ASM instructions (libffi).  Noisy build errors are
better than silent runtime failures any day of the week.

At the very least that test needs to be turned off if
CONFIG_ALTIVEC=n, because the kernel builds and runs fine otherwise.

Cheers,
Kyle Moffett

Re: RFC: x86: kill binutils 2.16.x?

[Benjamin Herrenschmidt]

On Tue, 2011-03-08 at 16:59 -0500, Kyle Moffett wrote:
> 
> The problem is not with the kernel compile itself, but with the 2.12
> "dssall" binutils test.  Basically, recent binutils treats e500 as
> effectively a separate architecture that happens to share *most* of
> the opcodes with regular PowerPC. 

This is bogus. Newer e500 don't have that SPE crap afaik and BookI and
II of the architecture have been converged. In fact, Scott, don't newer
FSL chips also support real lwsync ?

>  Any opcode which is not understood
> by the e500 chip is either convert to an equivalent opcode which is
> understood (IE: lwsync => sync), or failed with an error.  This means
> that the kernel compile aborts early telling me to upgrade to a newer
> version of binutils.

This is more bogosity in binutils. lwsync is designed to fallback as
sync if not supported in -HW-, binutils shouldn't silently swallow it.

Or is it that FSL failed on the original e500 and make lwsync actually
trap ?

> This was *critical* for getting an actual Debian distribution
> bootstrapped on the e500 cores, because so much software assumes
> PowerPC == altivec (ffmpeg), hardcodes 'asm("lwsync")' for memory
> barriers (80+ packages in Debian), or includes hand-coded
> floating-point ASM instructions (libffi).  Noisy build errors are
> better than silent runtime failures any day of the week.
> 
> At the very least that test needs to be turned off if
> CONFIG_ALTIVEC=n, because the kernel builds and runs fine otherwise.

I think the right thing is to keep that as e500-legacy or something,
because afaik, newer e500's don't have most of these issues and could be
treated as "normal" powerpc again.

Cheers,
Ben.

Re: RFC: x86: kill binutils 2.16.x?

[Kyle Moffett]

On Tue, Mar 8, 2011 at 18:13, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Tue, 2011-03-08 at 16:59 -0500, Kyle Moffett wrote:
>>
>> The problem is not with the kernel compile itself, but with the 2.12
>> "dssall" binutils test.  Basically, recent binutils treats e500 as
>> effectively a separate architecture that happens to share *most* of
>> the opcodes with regular PowerPC.
>
> This is bogus. Newer e500 don't have that SPE crap afaik and BookI and
> II of the architecture have been converged. In fact, Scott, don't newer
> FSL chips also support real lwsync ?

When I'm talking about "e500" I'm specifically referring to the SPE
stuff.  The "e500mc" cores are a whole different beast with a regular
FPU, but those have their own Kconfig option: "PPC_E500MC".

Actually, looking at it again, the "PPC_E500MC" depends on "E500", it
should select "PPC_FSL_BOOK3E" instead.  There are probably bugs
lurking here.

There really is fundamentally no good way to build a single system
image that supports both E500 (spe-based) and E500MC (classic FPU).
You can sort-of run classic FPU emulation on the E500, but the
performance is terrifyingly bad.

The naming confusion really is really bad too, IMO.

>>  Any opcode which is not understood
>> by the e500 chip is either convert to an equivalent opcode which is
>> understood (IE: lwsync => sync), or failed with an error.  This means
>> that the kernel compile aborts early telling me to upgrade to a newer
>> version of binutils.
>
> This is more bogosity in binutils. lwsync is designed to fallback as
> sync if not supported in -HW-, binutils shouldn't silently swallow it.
>
> Or is it that FSL failed on the original e500 and make lwsync actually
> trap ?

Yeah... for some reason FreeScale made the "lwsync" operation trap on
e500 (again, only talking about "e500" with SPE, not "e500mc").

Unfortunately it's used frequently enough that there's a very
noticeable performance gain (~5% for some programs) by substituting it
in binutils, so that's what happens.


>> This was *critical* for getting an actual Debian distribution
>> bootstrapped on the e500 cores, because so much software assumes
>> PowerPC == altivec (ffmpeg), hardcodes 'asm("lwsync")' for memory
>> barriers (80+ packages in Debian), or includes hand-coded
>> floating-point ASM instructions (libffi).  Noisy build errors are
>> better than silent runtime failures any day of the week.
>>
>> At the very least that test needs to be turned off if
>> CONFIG_ALTIVEC=n, because the kernel builds and runs fine otherwise.
>
> I think the right thing is to keep that as e500-legacy or something,
> because afaik, newer e500's don't have most of these issues and could be
> treated as "normal" powerpc again.

There is a separate "-me500mc" option for GAS that does the right
thing, but the kernel currently does not seem to use it.

arch/powerpc/Makefile has this:
    cpu-as-$(CONFIG_4xx) += -Wa,-m405
    cpu-as-$(CONFIG_6xx) += -Wa,-maltivec
    cpu-as-$(CONFIG_POWER4) += -Wa,-maltivec
    cpu-as-$(CONFIG_E500) += -Wa,-me500
    cpu-as-$(CONFIG_E200) += -Wa,-me200

As it is, I strongly suspect that the kernel is broken for
CONFIG_E500MC with current development releases of binutils.

The real "solution" is that "e500" needs to be treated as an almost
entirely different architecture from "all other powerpc (including
e500mc)".  Userspace is only ABI-compatible if you use "-msoft-float"
on both sides.

I've been dealing with it for a while now, it really is a fiasco all around...

After a month of wrestling with the official Debian "powerpc" port we
gave up and created a new port "powerpcspe" specifically for the
"e500" SPE-based chips.  Then we tripped over 3 relatively major GCC
bugs which had been lurking since e500 support was initially added.

See http://wiki.debian.org/PowerPCSPEPort for a very out-of-date
status on that whole thing.

Cheers,
Kyle Moffett

Re: RFC: x86: kill binutils 2.16.x?

[Segher Boessenkool]

> The problem is not with the kernel compile itself, but with the 2.12
> "dssall" binutils test.  Basically, recent binutils treats e500 as
> effectively a separate architecture that happens to share *most* of
> the opcodes with regular PowerPC.  Any opcode which is not understood
> by the e500 chip is either convert to an equivalent opcode which is
> understood (IE: lwsync => sync), or failed with an error.  This means
> that the kernel compile aborts early telling me to upgrade to a newer
> version of binutils.

$ echo dssall | powerpc-linux-as -many -me500
$ powerpc-linux-objdump -d a.out | grep 0:
   0:   7e 00 06 6c     dssall
$ powerpc-linux-as --version | head -1
GNU assembler (GNU Binutils) 2.21.51.20110309


What version of binutils does not work?  (I also checked with
-me500x2, -me500mc, -mspe, and various combinations.  lwsync
is indeed converted to a regular sync (well, "msync") for e500
and e500x2).


Segher

Re: RFC: x86: kill binutils 2.16.x?

[Michal Marek]

On 3.3.2011 09:30, Ingo Molnar wrote:
> * H. Peter Anvin <hpa@zytor.com> wrote:
>> kbuild people: is there a way to test for a specific assembler version
>> in Kbuild (and error out the build for it?)
> 
> Could we add a testcase for one of the more egregious breakages and bail out then? 
> That way we don't have to get the version information right - broken prereleases 
> would be covered as well.
> 
> For example this sequence:
> 
> .irp idx,0,1,2
> .if 0 > \idx
> .endif
> .endr
>
> Will break on 2.16, right? It builds fine on 2.20.

This seems to work for me with the binutils version from sles10 (even
with a vanilla build of binutils):
$ as -v <<EOF; echo $?
> .irp idx,0,1,2
> .if 0 > \idx
> .endif
> .endr
> EOF
GNU assembler version 2.16.91.0.5 (i586-suse-linux) using BFD version
2.16.91.0.5 20051219
0
$

So either the bug is fixed in that version already or you picked a wrong
example (or I did not understand what should fail here). But don't get
me wrong, I'm all for checking for actual bugs instead of innocent
version strings.

Michal

Re: RFC: x86: kill binutils 2.16.x?

[Ingo Molnar]

* Michal Marek <mmarek@suse.cz> wrote:

> On 3.3.2011 09:30, Ingo Molnar wrote:
> > * H. Peter Anvin <hpa@zytor.com> wrote:
> >> kbuild people: is there a way to test for a specific assembler version
> >> in Kbuild (and error out the build for it?)
> > 
> > Could we add a testcase for one of the more egregious breakages and bail out then? 
> > That way we don't have to get the version information right - broken prereleases 
> > would be covered as well.
> > 
> > For example this sequence:
> > 
> > .irp idx,0,1,2
> > .if 0 > \idx
> > .endif
> > .endr
> >
> > Will break on 2.16, right? It builds fine on 2.20.
> 
> This seems to work for me with the binutils version from sles10 (even
> with a vanilla build of binutils):
> $ as -v <<EOF; echo $?
> > .irp idx,0,1,2
> > .if 0 > \idx
> > .endif
> > .endr
> > EOF
> GNU assembler version 2.16.91.0.5 (i586-suse-linux) using BFD version
> 2.16.91.0.5 20051219
> 0
> $
> 
> So either the bug is fixed in that version already or you picked a wrong
> example (or I did not understand what should fail here). But don't get
> me wrong, I'm all for checking for actual bugs instead of innocent
> version strings.

I cited an incorrect testcase most likely. Note that Jan was able to work around the 
limitations in 2.16 after all - see the workaround commit that i have queued up in 
x86/mm, attached below.

Thanks,

	Ingo

--------------->
From d04c579f971bf7d995db1ef7a7161c0143068859 Mon Sep 17 00:00:00 2001
From: Jan Beulich <JBeulich@novell.com>
Date: Thu, 3 Mar 2011 10:55:29 +0000
Subject: [PATCH] x86: Work around old gas bug

Add extra parentheses around a couple of definitions introduced
by "x86: Cleanup vector usage" and used in assembly macro
arguments, and remove spaces. Without that old (2.16.1) gas
would see more macro arguments than were actually specified.

Reported-and-tested-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Jan Beulich <jbeulich@novell.com>
Cc: Shaohua Li <shaohua.li@intel.com>
LKML-Reference: <4D6F81B10200007800034B0B@vpn.id2.novell.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/include/asm/irq_vectors.h |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/irq_vectors.h b/arch/x86/include/asm/irq_vectors.h
index 4980f48..6e976ee 100644
--- a/arch/x86/include/asm/irq_vectors.h
+++ b/arch/x86/include/asm/irq_vectors.h
@@ -126,14 +126,14 @@
 
 /* up to 32 vectors used for spreading out TLB flushes: */
 #if NR_CPUS <= 32
-# define NUM_INVALIDATE_TLB_VECTORS NR_CPUS
+# define NUM_INVALIDATE_TLB_VECTORS	(NR_CPUS)
 #else
-# define NUM_INVALIDATE_TLB_VECTORS 32
+# define NUM_INVALIDATE_TLB_VECTORS	(32)
 #endif
 
-#define INVALIDATE_TLB_VECTOR_END	0xee
+#define INVALIDATE_TLB_VECTOR_END	(0xee)
 #define INVALIDATE_TLB_VECTOR_START	\
-	(INVALIDATE_TLB_VECTOR_END - NUM_INVALIDATE_TLB_VECTORS + 1)
+	(INVALIDATE_TLB_VECTOR_END-NUM_INVALIDATE_TLB_VECTORS+1)
 
 #define NR_VECTORS			 256