sparc32 __context initialization

[Artyom Tarasenko]

According to the IEEE-1275 SPARC supplement, the default load address
is the virtual address 0x4000, so looks like we don't have to map the
addresses 0x0 - 0x3fff.

I thought it might be useful actually not to map at least the page 0,
as it would help to catch null pointer dereferences.

So, I went on and made a trivial modification:

--- arch/sparc32/lib.c  (revision 1024)
+++ arch/sparc32/lib.c  (working copy)
@@ -461,7 +461,7 @@
     map_pages(pa, va, size, ofmem_arch_default_translation_mode(pa));

// 1:1 mapping for RAM
-    map_pages(0, 0, LOWMEMSZ, ofmem_arch_default_translation_mode(0));
+    map_pages(0x4000, 0x4000, LOWMEMSZ,
ofmem_arch_default_translation_mode(0));

/*
      * Flush cache
___

Surprisingly, with this modification a null pointer dereference
happens much earlier than I'd expected: for some reason  __context
doesn't get initialized in arch/sparc32/context.c, so OpenBIOS dies
before saying hello.

If I initialize it in start_main the same way it should have been
initialized statically,

start_main(void)
 {
+    __context = &main_ctx;

then OpenBIOS starts and is able to boot at least Debian.
Any idea what might be wrong with the current static initialization?
For convenience I paste the code from arch/sparc32/context.c :

static struct context main_ctx = {
    .regs[REG_SP] = (uint32_t) &_estack - 96,
    .pc = (uint32_t) start_main,
    .npc = (uint32_t) start_main + 4,
    .return_addr = (uint32_t) __exit_context,
};

/* This is used by assembly routine to load/store the context which
 * it is to switch/switched.  */
struct context *__context = &main_ctx;

[Mark Cave-Ayland]

On 09/02/11 17:10, Artyom Tarasenko wrote:
> Surprisingly, with this modification a null pointer dereference
> happens much earlier than I'd expected: for some reason  __context
> doesn't get initialized in arch/sparc32/context.c, so OpenBIOS dies
> before saying hello.
>
> If I initialize it in start_main the same way it should have been
> initialized statically,
>
> start_main(void)
>   {
> __context =&main_ctx;
>
> then OpenBIOS starts and is able to boot at least Debian.
> Any idea what might be wrong with the current static initialization?
> For convenience I paste the code from arch/sparc32/context.c :
>
> static struct context main_ctx = {
>      .regs[REG_SP] = (uint32_t)&_estack - 96,
>      .pc = (uint32_t) start_main,
>      .npc = (uint32_t) start_main + 4,
>      .return_addr = (uint32_t) __exit_context,
> };
>
> /* This is used by assembly routine to load/store the context which
> it is to switch/switched.  */
>
> struct context *__context =&main_ctx;

Hmmm possibly this could be an overflow in the setup of main_ctx which 
causes it to scribble over __context?

Note that with gdb/Qemu you can set watchpoints on a variable so that 
gdb breaks when the value at a particular memory location changes. This 
is really useful and how we found the dictionary overflow problem in 
SPARC64.

HTH,

Mark.

[Blue Swirl]

On Wed, Feb 9, 2011 at 7:25 PM, Mark Cave-Ayland
 wrote:
> On 09/02/11 17:10, Artyom Tarasenko wrote:
> > Surprisingly, with this modification a null pointer dereference
> > happens much earlier than I'd expected: for some reason  __context
> > doesn't get initialized in arch/sparc32/context.c, so OpenBIOS dies
> > before saying hello.
> >
> > If I initialize it in start_main the same way it should have been
> > initialized statically,
> >
> > start_main(void)
> >  {
> > __context =&main_ctx;
> >
> > then OpenBIOS starts and is able to boot at least Debian.
> > Any idea what might be wrong with the current static initialization?
> > For convenience I paste the code from arch/sparc32/context.c :
> >
> > static struct context main_ctx = {
> >     .regs[REG_SP] = (uint32_t)&_estack - 96,
> >     .pc = (uint32_t) start_main,
> >     .npc = (uint32_t) start_main + 4,
> >     .return_addr = (uint32_t) __exit_context,
> > };
> >
> > /* This is used by assembly routine to load/store the context which
> >  * it is to switch/switched.  */
> > struct context *__context =&main_ctx;
>
> Hmmm possibly this could be an overflow in the setup of main_ctx which
> causes it to scribble over __context?

IIRC this variable is the first one in data segment, perhaps BSS
clearing overwrites it (but I think I already fixed that one once), or
copying the data from ROM skips one word?

[Artyom Tarasenko]

On Wed, Feb 9, 2011 at 6:52 PM, Blue Swirl  wrote:
> IIRC this variable is the first one in data segment, perhaps BSS
> clearing overwrites it (but I think I already fixed that one once), or
> copying the data from ROM skips one word?

speaking of BSS, should we worry about the following warning?

BFD: openbios-builtin.elf: warning: allocated section `.bss' not in segment

[Andreas Färber]

Am 09.02.2011 um 19:23 schrieb Artyom Tarasenko:
> On Wed, Feb 9, 2011 at 6:52 PM, Blue Swirl   
> wrote:
> > IIRC this variable is the first one in data segment, perhaps BSS
> > clearing overwrites it (but I think I already fixed that one once),  
> > or
> > copying the data from ROM skips one word?
>
> speaking of BSS, should we worry about the following warning?
>
> BFD: openbios-builtin.elf: warning: allocated section `.bss' not in  
> segment

You should. That would've been my first guess: Check arch/sparc32/ 
ldscript for missing sections.

Andreas

[Mark Cave-Ayland]

On 09/02/11 18:37, Andreas Färber wrote:
> > speaking of BSS, should we worry about the following warning?
> >
> > BFD: openbios-builtin.elf: warning: allocated section `.bss' not in
> > segment
>
> You should. That would've been my first guess: Check
> arch/sparc32/ldscript for missing sections.
>
> Andreas

I don't see that warning at all here - is it specific to a particular 
gcc version?

ATB,

Mark.

[Artyom Tarasenko]

On Thu, Feb 10, 2011 at 1:23 PM, Mark Cave-Ayland
 wrote:
> On 09/02/11 18:37, Andreas Färber wrote:
> > > speaking of BSS, should we worry about the following warning?
> > >
> > > BFD: openbios-builtin.elf: warning: allocated section `.bss' not in
> > > segment
> >
> > You should. That would've been my first guess: Check
> > arch/sparc32/ldscript for missing sections.
> >
> > Andreas
>
> I don't see that warning at all here - is it specific to a particular gcc
> version?

I've noticed it when build it directly in obj-sparc32 directory.
Otherwise the warning lands into build.log, not on console. Also I
build it natively, on debian 4.0r8.

LINK  openbios-builtin.elf
  GEN   openbios-builtin.elf.syms
  STRIP openbios-builtin.elf
BFD: openbios-builtin.elf: warning: allocated section `.bss' not in segment
make[1]: Leaving directory `/projects/openbios-devel/obj-sparc32'

$  gcc --version
gcc (GCC) 4.1.2 20061115 (prerelease) (Debian 4.1.1-21)
Copyright (C) 2006 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Artyom

[Mark Cave-Ayland]

On 10/02/11 12:54, Artyom Tarasenko wrote:
> I've noticed it when build it directly in obj-sparc32 directory.
> Otherwise the warning lands into build.log, not on console. Also I
> build it natively, on debian 4.0r8.

Wow. That's Debian etch which is 2 versions before current stable :)
> LINK  openbios-builtin.elf
>    GEN   openbios-builtin.elf.syms
>    STRIP openbios-builtin.elf
> BFD: openbios-builtin.elf: warning: allocated section `.bss' not in segment
> make[1]: Leaving directory `/projects/openbios-devel/obj-sparc32'
>
> $  gcc --version
> gcc (GCC) 4.1.2 20061115 (prerelease) (Debian 4.1.1-21)
> Copyright (C) 2006 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

My setup here is:

$ sparc64-elf-gcc --version

sparc64-elf-gcc (GCC) 4.5.0

Copyright (C) 2010 Free Software Foundation, Inc.

This is free software; see the source for copying conditions.  There is 
NO

warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR 
PURPOSE.

$ gcc --version

gcc (Debian 
4.4.5-8) 4.4.5

Copyright (C) 2010 Free Software Foundation, Inc.

This is free software; see the source for copying conditions.  There is 
NO

warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

HTH,

Mark.

[Artyom Tarasenko]

On Thu, Feb 10, 2011 at 4:14 PM, Mark Cave-Ayland
 wrote:
> On 10/02/11 12:54, Artyom Tarasenko wrote:
> > I've noticed it when build it directly in obj-sparc32 directory.
> > Otherwise the warning lands into build.log, not on console. Also I
> > build it natively, on debian 4.0r8.
>
> Wow. That's Debian etch which is 2 versions before current stable :)

Which is AFAIK the last version supporting sparc32.
> > LINK  openbios-builtin.elf
> >   GEN   openbios-builtin.elf.syms
> >   STRIP openbios-builtin.elf
> > BFD: openbios-builtin.elf: warning: allocated section `.bss' not in
> > segment
> > make[1]: Leaving directory `/projects/openbios-devel/obj-sparc32'
> >
> > $  gcc --version
> > gcc (GCC) 4.1.2 20061115 (prerelease) (Debian 4.1.1-21)
> > Copyright (C) 2006 Free Software Foundation, Inc.
> > This is free software; see the source for copying conditions.  There is NO
> > warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR
> > PURPOSE.
>
> My setup here is:
>
> $ sparc64-elf-gcc --version
> sparc64-elf-gcc (GCC) 4.5.0
>
> Copyright (C) 2010 Free Software Foundation, Inc.
>
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>
> $ gcc --version
>
> gcc (Debian 4.4.5-8)
> 4.4.5
>
> Copyright (C) 2010 Free Software Foundation, Inc.
>
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

You mean the native compilation won't be supported?

Artyom

[Mark Cave-Ayland]

On 10/02/11 15:57, Artyom Tarasenko wrote:
> You mean the native compilation won't be supported?

Hmmmm I hadn't actually thought of doing that ;)  Then again, the 
message is a WARNING (non-fatal) rather than an ERROR so it should still 
work - have you actually tried booting qemu with the resulting image?

If it's a warning, then it's something we should be able to fix by 
altering the ldscript file I would hope.

ATB,

Mark.

[Artyom Tarasenko]

On Wed, Feb 9, 2011 at 6:25 PM, Mark Cave-Ayland
 wrote:
> On 09/02/11 17:10, Artyom Tarasenko wrote:
> > Surprisingly, with this modification a null pointer dereference
> > happens much earlier than I'd expected: for some reason  __context
> > doesn't get initialized in arch/sparc32/context.c, so OpenBIOS dies
> > before saying hello.
> >
> > If I initialize it in start_main the same way it should have been
> > initialized statically,
> >
> > start_main(void)
> >  {
> > __context =&main_ctx;
> >
> > then OpenBIOS starts and is able to boot at least Debian.
> > Any idea what might be wrong with the current static initialization?
> > For convenience I paste the code from arch/sparc32/context.c :
> >
> > static struct context main_ctx = {
> >     .regs[REG_SP] = (uint32_t)&_estack - 96,
> >     .pc = (uint32_t) start_main,
> >     .npc = (uint32_t) start_main + 4,
> >     .return_addr = (uint32_t) __exit_context,
> > };
> >
> > /* This is used by assembly routine to load/store the context which
> >  * it is to switch/switched.  */
> > struct context *__context =&main_ctx;
>
> Hmmm possibly this could be an overflow in the setup of main_ctx which
> causes it to scribble over __context?
>
> Note that with gdb/Qemu you can set watchpoints on a variable so that gdb
> breaks when the value at a particular memory location changes. This is
> really useful and how we found the dictionary overflow problem in SPARC64.

Thanks for the hints! You are right, it is properly initialized and
then gets corrupted:

Old value = (struct context *) 0xffd78000
New value = (struct context *) 0x0
__switch_context_nosave () at ../arch/sparc32/switch.S:89
89              ld      [%fp + 4], %g1

__switch_context_nosave shouldn't change __context. I'll make a patch.

Artyom