IP32 prom crashes due to __pa() funkiness

Initially, booting a straight git checkout on an IP32 will cause it to prom crash, usually somewhere in between init_bootmem() and init_bootmem_core(). I bisected git to trace this back to one of the inital __pa() introduction patches from commit d4df6d4 (get ride of CPHYSADDR()). It actually appears that the actual commit that broke things was 620a480 (Make __pa() aware of XKPHYS/CKSEG0 address mix for 64 bit kernels).

The [short-term] fix highlighted by Ilya is to make __pa() unconditionally be defined to "((unsigned long)(x) < CKSEG0 ? PAGE_OFFSET : CKSEG0)"; Discovered by building IP32 with CONFIG_BUILD_ELF64=n.

Normally, this shouldn't be possible, as CONFIG_BUILD_ELF64=n was originally only allowed by using the old o64 hack, which has subsequently died and been replaced with the newer -msym32 form. As far as I know, CONFIG_BUILD_ELF64 is apparently supposed to be removed at some point in the future, since I believe it existed only for quirky 64bit-in-32bit kernels for IP22 and more commonly, IP32. From 2.6.17 onwards, I've been building IP32 kernels with CONFIG_BUILD_ELF64=y and using gcc-4, and haven't had problems up until now.

Anyways, I'm not sure if this is an IP32-specific oddity or not (probably is), but it needs the define highlighted above to work properly. Plain PAGE_OFFSET won't work for these machines. Given the same trick os -msym32 is used for the rare IP22 64bit kernel, I would not be surprised if the same problem and fix both occur and work for those machines as well. Something to maybe test later, I suppose.

But for now, anyone got thoughts as to a sane workaround for this? Perhaps some conditional tweaks in mach-ip32/*.h files somewheres, would it be simpler to just switch to:

#if defined(CONFIG_64BIT) && (!defined(CONFIG_BUILD_ELF64) || defined(CONFIG_SGI_IP32))

(assuming that IP22 doesn't need it; I'll find out later)

Initially, booting a straight git checkout on an IP32 will cause it to prom crash, usually somewhere in between init_bootmem() and init_bootmem_core(). I bisected git to trace this back to one of the inital __pa() introduction patches from commit d4df6d4 (get ride of CPHYSADDR()). It actually appears that the actual commit that broke things was 620a480 (Make __pa() aware of XKPHYS/CKSEG0 address mix for 64 bit kernels).

The [short-term] fix highlighted by Ilya is to make __pa() unconditionally be defined to "((unsigned long)(x) < CKSEG0 ? PAGE_OFFSET : CKSEG0)"; Discovered by building IP32 with CONFIG_BUILD_ELF64=n.

Normally, this shouldn't be possible, as CONFIG_BUILD_ELF64=n was originally only allowed by using the old o64 hack, which has subsequently died and been replaced with the newer -msym32 form. As far as I know, CONFIG_BUILD_ELF64 is apparently supposed to be removed at some point in the future, since I believe it existed only for quirky

Date: Thu, 1 Mar 2007 10:39:08 +0100
From: Franck Bui-Huu <[email protected]>
To: Kumba <[email protected]>
Cc: Linux MIPS List <[email protected]>
Subject: Re: IP32 prom crashes due to __pa() funkiness

Hi,

Quoted 9 lines · Kumba

Initially, booting a straight git checkout on an IP32 will cause it to prom crash, usually somewhere in between init_bootmem() and init_bootmem_core(). I bisected git to trace this back to one of the inital __pa() introduction patches from commit d4df6d4 (get ride of CPHYSADDR()). It actually appears that the actual commit that broke things was 620a480 (Make __pa() aware of XKPHYS/CKSEG0 address mix for 64 bit kernels).

Quoted 5 lines

The [short-term] fix highlighted by Ilya is to make __pa() unconditionally be defined to "((unsigned long)(x) < CKSEG0 ? PAGE_OFFSET : CKSEG0)"; Discovered by building IP32 with CONFIG_BUILD_ELF64=n.

Well, it means that you previously used CONFIG_BUILD_ELF64=y (this implied that PAGE_OFFSET is in XKPHYS) whereas your kernel has CKSEG load address (symbols need PAGE_OFFSET in CKSEG for address translation).

...

Hi,

Well, it means that you previously used CONFIG_BUILD_ELF64=y (this implied that PAGE_OFFSET is in XKPHYS) whereas your kernel has CKSEG load address (symbols need PAGE_OFFSET in CKSEG for address translation).

So the question is why can't you use CONFIG_BUILD_ELF64=n (and reagarding the current definition of CONFIG_BUILD_ELF64).

It makes me think that I posted a patch for that a couple of weeks ago:

http://marc.theaimsgroup.com/?l=linux-mips&m=117154480225936&w=2
http://marc.theaimsgroup.com/?l=linux-mips&m=117154480126802&w=2
http://marc.theaimsgroup.com/?l=linux-mips&m=117154587014827&w=2

Basically this patch removes CONFIG_BUILD_ELF64 and makes Kbuild to use '-msym32' switch if you really need it. Kbuild makes its choice according the load address of your kernel image.

Could you give it a try ? This patch was based on 2.6.20 but it should apply fine on a 2.6.21-rc[12].

So with the changes brought in by __pa(), I suppose a new, RightWay(TM) to build IP32 (and conversely, 64bit IP22) kernels needs to be found.

I don't know if it is the RightWay(TM), but I am running here a fresh IP32 kernel (l.m.o git updated yesterday). It was compiled with CONFIG_BUILD_ELF64=n, and I am using vmlinux.

$ file /boot/vmlinux-2.6.21-rc3-g839fd555 /boot/vmlinux-2.6.21-rc3-g839fd555: ELF 64-bit MSB executable, MIPS, MIPS-IV version 1 (SYSV), statically linked, not stripped

If it makes a difference, I am using arcboot.

Quoted 5 lines · Arnaud Giersch

I don't know if it is the RightWay(TM), but I am running here a fresh IP32 kernel (l.m.o git updated yesterday). It was compiled with CONFIG_BUILD_ELF64=n, and I am using vmlinux.

Quoted 5 lines

$ file /boot/vmlinux-2.6.21-rc3-g839fd555
/boot/vmlinux-2.6.21-rc3-g839fd555: ELF 64-bit MSB executable, MIPS,
MIPS-IV version 1 (SYSV), statically linked, not stripped

If it makes a difference, I am using arcboot.

I suppose then the question is, which is better for IP32?

CONFIG_BUILD_ELF64=y or CONFIG_BUILD_ELF64=n. The reason the o64 hack used to exist, if my memory serves me correctly, was that someone once said that when built and run as a pure 64bit binary converted to 32bit via objcopy, 6 extra insns were run every cycle (I think), introducing unneeded slowdown. This changed to 2 insns by going the o64 route,

which involved CONFIG_BUILD_ELF64=n. So 4 less insns technically resulted in a quicker kernel, though the layman might not notice such. I believe that all changed at some point, which is why CONFIG_BUILD_ELF64=y was an A-OK thing prior to the __pa() introduction.

Now I guess we're back to CONFIG_BUILD_ELF64=n? I guess the real question is, which way is the OneWay(TM), RightWay(TM) and OnlyWay(TM)?

Quoted 2 lines

Now I guess we're back to CONFIG_BUILD_ELF64=n? I guess the real question is, which way is the OneWay(TM), RightWay(TM) and OnlyWay(TM)?

Now it's clear that CONFIG_BUILD_ELF64 is really confusing. I would say that whatever the value of CONFIG_BUILD_ELF64, your kernel should run fine. BUT it really depends on your kernel load address:

if CONFIG_BUILD_ELF64=y then kernel load address must be in XKPHYS if CONFIG_BUILD_ELF64=n then kernel load address must be in CKSEG0

All others configs (I think) are buggy...

That's said, it seems that IPxx kernels are really special beasts. Take from MIPS makefile:

"""
Some machines like the Indy need 32-bit ELF binaries for booting purposes.
"""

Why? A ELF64 kernel for CKSEG0 should be fine, at least in principle, even if it doesn't work wih the current codebase.

Now it's clear that CONFIG_BUILD_ELF64 is really confusing. I would say that whatever the value of CONFIG_BUILD_ELF64, your kernel should run fine. BUT it really depends on your kernel load address:

if CONFIG_BUILD_ELF64=y then kernel load address must be in XKPHYS if CONFIG_BUILD_ELF64=n then kernel load address must be in CKSEG0

OK that's the reason why the last patch you applied has no effect. If you're using 'objcopy' your kernel can still be miss configured.

BTW, does your gcc support '-sym32' switch ?

My current feeling is that __pa() introduction now breaks configs which were initialy _broken_, IMHO. CPHYSADDR() hide them because it can happily convert any virutal address form both CKSEG0 or XKPHYS.

It may be a good thing to crash at this point. CONFIG_BUILD_ELF64 is used in some others places, where it can introduce some others bugs harder to find (if miss configured of course). The sad thing is that the kernel does not report any useful messages. That's why I think adding some specific sanity checks for 64 bit kernels in boot mem init code may be usefull.

Now it's clear that CONFIG_BUILD_ELF64 is really confusing. I would say that whatever the value of CONFIG_BUILD_ELF64, your kernel should run fine. BUT it really depends on your kernel load address:

if CONFIG_BUILD_ELF64=y then kernel load address must be in XKPHYS if CONFIG_BUILD_ELF64=n then kernel load address must be in CKSEG0

All others configs (I think) are buggy...

That's said, it seems that IPxx kernels are really special beasts. Take from MIPS makefile:

"""
Some machines like the Indy need 32-bit ELF binaries for booting purposes.
"""

So I dunno if this statement is still correct but it sounds that you have no other choice thatn CONFIG_BUILD_ELF64=n and load address in CKSEG0.

I'm sorry but my IPxx background is 0 ;)

Most of this is because IP22 (Indy/Idigo2 R4xx) and IP32 (O2 R5xxx), while supporting 64bit kernels (same for cobalt, since it's a mips4-level CPU), we had to "trick" them into accepting 64bit code. IP32 at one point ran 32bit kernels only, but it was later converted to supporting only 64bit kernels, hence the hackery involved. We describe it as wrapping 64bit code into a 32bit object, because their proms will only recognize 32bit objects (specifically, IP22 will only boot 32bit objects; crash on 64bit; IP32 will take both, but likes 32bit better).

So really, CONFIG_BUILD_ELF64 was probably part of this "magic" to stuff 64bit code into a candy-coated 32bit wrapper for the Indy (And later the O2) to suck down w/o complaint. Hence, __pa() probably needs to replicate this support, or we all need to brainstorm a proper way to get these systems to boot.

Most of this is because IP22 (Indy/Idigo2 R4xx) and IP32 (O2 R5xxx), while supporting 64bit kernels (same for cobalt, since it's a mips4-level CPU), we had to "trick" them into accepting 64bit code. IP32 at one point ran 32bit kernels only, but it was later converted to supporting only 64bit kernels, hence the hackery involved. We describe it as wrapping 64bit code into a 32bit object, because their proms will only recognize 32bit objects (specifically, IP22 will only boot 32bit objects; crash on 64bit; IP32 will take both, but likes 32bit better).

So really, CONFIG_BUILD_ELF64 was probably part of this "magic" to stuff 64bit code into a candy-coated 32bit wrapper for the Indy (And later the O2) to suck down w/o complaint. Hence, __pa() probably needs to replicate this support, or we all need to brainstorm a proper way to get these systems to boot.

Octane, Origin, IP28 (Indigo2 R10000). et al, don't complain, and don't need this hacker, because their proms boot proper 64bit binaries only (they reject all else).

So probably the following is true (someone correct me if not)

if (CONFIG_BUILD_ELF64=y and (!CONFIG_SGI_IP22 or !CONFIG_SGI_IP32 or !CONFIG_COBALT)) then kernel load address must be in XKPHYS else load address must be in CKSEG0
if CONFIG_BUILD_ELF64=n then kernel load address must be in CKSEG0

Most of this is because IP22 (Indy/Idigo2 R4xx) and IP32 (O2 R5xxx), while supporting 64bit kernels (same for cobalt, since it's a mips4-level CPU), we had to "trick" them into accepting 64bit code. IP32 at one point ran 32bit kernels only, but it was later converted to supporting only 64bit kernels, hence the hackery involved. We describe it as wrapping 64bit code into a 32bit object, because their proms will only recognize 32bit objects (specifically, IP22 will only boot 32bit objects; crash on 64bit; IP32 will take both, but likes 32bit better).

So really, CONFIG_BUILD_ELF64 was probably part of this "magic" to stuff 64bit code into a candy-coated 32bit wrapper for the Indy (And later the O2) to suck down w/o complaint. Hence, __pa() probably needs to replicate this support, or we all need to brainstorm a proper way to get these systems to boot.

Quoted 1 line

I'm sorry but my IPxx background is 0 ;)

Time to buy an O2 :)

I'm really not confident with all your tricks you described. Maybe a config that I believed to be uninsteresting and useless should be supported still.

Can you try the attached patch with a plain linux-mips kernel ? This patch restore CPHYSADDR() for 64 bits kernels _only_. I guess it's ok because we won't need to support mapped kernels on 64 bits machines...

Could others give their opinions ?

After going through all the fun to get rid of CPHYSADDR, I see no point really to bring it back.

Plus that patch unnecessarily complicates those defines. As I
highlighted in my original mail, O2 doesn't need CPHYSADDR added to __pa(), it just needs the conditional define for __pa_page_offset to be a little bit more flexible.

Date: Sat, 17 Mar 2007 14:53:23 -0400
From: Kumba <[email protected]>
To: Franck Bui-Huu <[email protected]>
Cc: peter fuerst <[email protected]>
Subject: Re: IP32 prom crashes due to __pa() funkiness

Quoted 2 lines · Franck Bui-Huu

Hi,

[snip]

Quoted 9 lines

Well, it means that you previously used CONFIG_BUILD_ELF64=y (this implied that PAGE_OFFSET is in XKPHYS) whereas your kernel has CKSEG load address (symbols need PAGE_OFFSET in CKSEG for address translation).

So the question is why can't you use CONFIG_BUILD_ELF64=n (and reagarding the current definition of CONFIG_BUILD_ELF64).

[snip]

Quoted 7 lines

It makes me think that I posted a patch for that a couple of weeks ago:

http://marc.theaimsgroup.com/?l=linux-mips&m=117154480225936&w=2
http://marc.theaimsgroup.com/?l=linux-mips&m=117154480126802&w=2
http://marc.theaimsgroup.com/?l=linux-mips&m=117154587014827&w=2

Quoted 6 lines

Basically this patch removes CONFIG_BUILD_ELF64 and makes Kbuild to use '-msym32' switch if you really need it. Kbuild makes its choice according the load address of your kernel image.

Could you give it a try ? This patch was based on 2.6.20 but it should apply fine on a 2.6.21-rc[12].

Tested, and it still failed.

And I didn't always use CONFIG_BUILD_ELF64. In fact, for the longest time (up until 2.6.17) I built IP32 and 64bit IP22 kernels without CONFIG_BUILD_eLF64, passing -mabi=o64 (a.k.a., the binutils hack). This let me use the plain 'vmlinux' target w/o the need for an objcopy to boot these systems. After 2.6.17, the nature of o64 was mostly neutered, so I switched to using CONFIG_BUILD_ELF64 and the 'vmlinux.32' target, as this is apparently the way Debian builds their IP32 kernels (and was the way geoman/spbecker said I should've been using all along).

So with the changes brought in by __pa(), I suppose a new, RightWay(TM) to build IP32 (and conversely, 64bit IP22) kernels needs to be found. These two systems are particularly wacky, hence why they need a bit more special care than more traditional, proper, 64bit systems like Origin and Octane.

Also, Peter raises a good point in this case. It sounds like a re-thinking of how all this address stuff is handled will fix not only special cases like IP32/IP22, but the really weird systems, like IP28, as well. Which would be a big plus in my opinion.

--Kumba

--
Gentoo/MIPS Team Lead

"Such is oft the course of deeds that move the wheels of the world: small hands do them because they must, while the eyes of the great are elsewhere." --Elrond

Hi!

Did you try to use PHYS_OFFSET > 0 ? __pa() ist still (as of d3fbd83ff545e49e2a0a5ca0f00dda4eedaf8be7) defined as (casts omitted):

#define __pa(x) (x - (x < CKSEG0 ? PAGE_OFFSET:CKSEG0) + PHYS_OFFSET)

This gives __pa(CKSEG0) == PHYS_OFFSET, which will never work for PHYS_OFFSET > 0. A quick fix (assuming this was the cause for failure) could be:

========================================================================

include/asm-mips/page.h +1 -1

@@ -150,7 +150,7 @@ typedef struct { unsigned long pgprot; }
  * __pa()/__va() should be used only during mem init.
  */
 #if defined(CONFIG_64BIT) && !defined(CONFIG_BUILD_ELF64)
-#define __pa_page_offset(x)	((unsigned long)(x) < CKSEG0 ? PAGE_OFFSET : CKSEG0)
+#define __pa_page_offset(x)	((unsigned long)(x) < CKSEG0 ? PAGE_OFFSET : CKSEG0+PHYS_OFFSET)
 #else
 #define __pa_page_offset(x)	PAGE_OFFSET
 #endif

Signed-off-by peter fuerst <[email protected]>

========================================================================

Of course, "#define PAGE_OFFSET (CAC_BASE + PHYS_OFFSET)" is also needed.

kind regards

peter

Quoted 35 lines · peter fuerst

Hi!

Did you try to use PHYS_OFFSET > 0 ? __pa() ist still (as of d3fbd83ff545e49e2a0a5ca0f00dda4eedaf8be7) defined as (casts omitted):

#define __pa(x) (x - (x < CKSEG0 ? PAGE_OFFSET:CKSEG0) + PHYS_OFFSET)

This gives __pa(CKSEG0) == PHYS_OFFSET, which will never work for PHYS_OFFSET > 0. A quick fix (assuming this was the cause for failure) could be:

========================================================================
---
d3fbd83ff545e49e2a0a5ca0f00dda4eedaf8be7/include/asm-mips/page.h Sat Mar 10 08:43:17 2007
+++ quickfix/include/asm-mips/page.h Sun Mar 18 10:24:34 2007

include/asm-mips/page.h +1 -1

@@ -150,7 +150,7 @@ typedef struct { unsigned long pgprot; }
  * __pa()/__va() should be used only during mem init.
  */
 #if defined(CONFIG_64BIT) && !defined(CONFIG_BUILD_ELF64)
-#define __pa_page_offset(x)    ((unsigned long)(x) < CKSEG0 ? 
 PAGE_OFFSET : CKSEG0)
+#define __pa_page_offset(x)    ((unsigned long)(x) < CKSEG0 ? 
 PAGE_OFFSET : CKSEG0+PHYS_OFFSET)
 #else

#define __pa_page_offset(x) PAGE_OFFSET
#endif

Signed-off-by peter fuerst <[email protected]>

========================================================================

Of course, "#define PAGE_OFFSET (CAC_BASE + PHYS_OFFSET)" is also needed.

Quoted 3 lines

kind regards

peter

Hmm, I can't find where PHYS_OFFSET is defined for mips. A grep shows it mostly existing for the arm arch. Is there an alternative macro available, or is this something that needs porting over?

--Kumba

Hi!

Did you try to use PHYS_OFFSET > 0 ? __pa() ist still (as of d3fbd83ff545e49e2a0a5ca0f00dda4eedaf8be7) defined as (casts omitted):

#define __pa(x) (x - (x < CKSEG0 ? PAGE_OFFSET:CKSEG0) + PHYS_OFFSET)

This gives __pa(CKSEG0) == PHYS_OFFSET, which will never work for PHYS_OFFSET > 0. A quick fix (assuming this was the cause for failure) could be:

========================================================================

include/asm-mips/page.h +1 -1

@@ -150,7 +150,7 @@ typedef struct { unsigned long pgprot; }
  * __pa()/__va() should be used only during mem init.
  */
 #if defined(CONFIG_64BIT) && !defined(CONFIG_BUILD_ELF64)
-#define __pa_page_offset(x)	((unsigned long)(x) < CKSEG0 ? PAGE_OFFSET : CKSEG0)
+#define __pa_page_offset(x)	((unsigned long)(x) < CKSEG0 ? PAGE_OFFSET : CKSEG0+PHYS_OFFSET)
 #else
 #define __pa_page_offset(x)	PAGE_OFFSET
 #endif

Signed-off-by peter fuerst <[email protected]>

========================================================================

Of course, "#define PAGE_OFFSET (CAC_BASE + PHYS_OFFSET)" is also needed.

kind regards

peter

Backported the patches that bring in PHYS_OFFSET and tried both with and without Frank's patches to remove CONFIG_BUILD_ELF64 from the Makefile, plus this change, and still no go. W/o CONFIG_BUILD_ELF64, both the vmlinux and vmlinux.32 targets just silently hang. With CONFIG_BUILD_ELF64 in the Makefile, the same Prom crash.