OpenBIOS August 2011

openbios@openbios.org

19 participants
88 discussions

Re: [OpenBIOS] [PATCH 9/10] Divide by zero check
by Mark Cave-Ayland 10 Aug '11

10 Aug '11

On 09/08/11 22:55, William Hahne wrote: > It is possible for BootX to purposefully divide by zero expecting 0 as > the result. > > Index: kernel/forth.c > =================================================================== > --- kernel/forth.c (revision 1041) > +++ kernel/forth.c (working copy) > @@ -1157,6 +1157,12 @@ > { > const ucell b = POP(); > const ducell a = DPOP(); > + > + if (b == 0) { // can't divide by zero > + PUSH(0); > + DPUSH(0); > + return; > + } > #ifdef NEED_FAKE_INT128_T > if (a.hi != 0) { > fprintf(stderr, "mudivmod called (0x%016llx %016llx / > 0x%016llx)\n", There is something wrong here: nothing will purposefully divide by zero as it will invoke a trap somewhere. Again, I think this is hiding another bug somewhere and is not necessarily the correct fix. ATB, Mark. -- Mark Cave-Ayland - Senior Technical Architect PostgreSQL - PostGIS Sirius Corporation plc - control through freedom http://www.siriusit.co.uk t: +44 870 608 0063 Sirius Labs: http://www.siriusit.co.uk/labs

2 1

Re: [OpenBIOS] [PATCH 8/10] Check that stdout variable is not null
by Mark Cave-Ayland 10 Aug '11

10 Aug '11

On 09/08/11 22:55, William Hahne wrote: > BootX will sometimes set the stdout variable to null and then > subsequently call io-emit which can eventually result in an infinite loop. > > Index: forth/admin/iocontrol.fs > =================================================================== > --- forth/admin/iocontrol.fs (revision 1041) > +++ forth/admin/iocontrol.fs (working copy) > @@ -80,6 +80,7 @@ > ; > : io-emit ( char -- ) > + stdout @ 0= if drop exit then \ make sure stdout pointer is not null > io-out-char c! > io-out-char 1 " write" stdout @ $call-method drop > ; Hmmm isn't this just hiding another bug which is that the stdout variable/defer is being set? ATB, Mark. -- Mark Cave-Ayland - Senior Technical Architect PostgreSQL - PostGIS Sirius Corporation plc - control through freedom http://www.siriusit.co.uk t: +44 870 608 0063 Sirius Labs: http://www.siriusit.co.uk/labs

2 1

Re: [OpenBIOS] [PATCH 7/10] Apple local variables implementation in Forth
by Mark Cave-Ayland 10 Aug '11

10 Aug '11

On 09/08/11 22:55, William Hahne wrote: > This is an implementation of Apple local variables which is required to > execute the Forth scripts in BootX > > Index: forth/bootstrap/bootstrap.fs > =================================================================== > --- forth/bootstrap/bootstrap.fs (revision 1041) > +++ forth/bootstrap/bootstrap.fs (working copy) > @@ -1413,10 +1413,135 @@ > here last @ , latest ! \ write backlink and set latest > ; > +\ > +\ Defer required by BootX > +\ > +defer spin > + > +\ > +\ Local Variables (Apple specific) > +\ > + > +\ > +: has-locals ( -- true/false ) > + skipws > + ib >in @ + c@ 7b <> > + if false exit then > + ib >in @ + 1+ c@ 20 > > + if false exit then > + > + true > + ; > + > +: comp-str-char ( str len char -- true/false ) > + swap 1 <> if 2drop false exit then > + swap c@ <> if false exit then > + true > + ; > + > +variable locals_wordlist > +variable locals_state \ 0 - reading args > + \ 1 - ; > + \ 2 - reading vars > +: read-locals ( -- addr0 .. addrN addrCount ) > + has-locals > + not if 0 false exit then \ no locals > + > + 0 locals_state ! > + 0 >r > + > + s" get-current" $find drop execute > + s" wordlist" $find drop execute > + dup locals_wordlist ! > + s" set-current" $find drop execute > + > + parse-word 2drop \ ditch the { > + > + begin > + parse-word > + > + 2dup 3b comp-str-char \ check for ; > + if 1 locals_state ! then > + > + 2dup 7d comp-str-char \ check for } > + not > + while > + locals_state @ 1 <> if \ the ; is not a local variable so ignore it > + header > + > + locals_state @ 0= if \ only save the address if it is an arg > + r> > + here na1+ >r > + 1+ >r > + then > + > + 3 , 0 , > + reveal > + else \ if we hit a ; then move to next state > + 2drop > + 2 locals_state ! > + then > + repeat > + > + 2drop > + > + s" set-current" $find drop execute > + > + r> 0 > + begin > + 2dup > + > > + while > + r> -rot > + 1+ > + repeat > + > + drop > + > + true > + ; > + > +: begin-locals ( addr0 .. addrN count hasLocals -- ) > + not if drop exit then > + > + dup 0> if > + 0 do > + ['] (lit) , , ['] ! , > + loop > + else drop then > + > + s" get-order" $find drop execute > + locals_wordlist @ > + swap 1+ > + s" set-order" $find drop execute > + ; > + > +: end-locals ( -- ) > + locals_wordlist @ 0= if exit then > + > + 0 locals_wordlist ! > + > + s" get-order" $find drop execute > + swap drop 1- > + s" set-order" $find drop execute > + ; > + > +: -> parse-word $find drop na1+ > + ['] (lit) , , ['] ! , > + ; immediate > + > +\ > +\ 7.3.9.1 Defining words > +\ > + > : : > - parse-word header > - 1 , ] > + parse-word >r >r > + read-locals > + r> r> header > + 1 , > + begin-locals > + ] > ; > : :noname > @@ -1426,6 +1551,7 @@ > ; > : ; > + end-locals > ['] (semis) , reveal ['] [ execute > ; immediate > Very interesting. I think this needs someone with quite strong Forth-fu (Stefan) to review this one. Also I can't see any related documentation with this patch related to locals support? ATB, Mark. -- Mark Cave-Ayland - Senior Technical Architect PostgreSQL - PostGIS Sirius Corporation plc - control through freedom http://www.siriusit.co.uk t: +44 870 608 0063 Sirius Labs: http://www.siriusit.co.uk/labs

2 1

Re: [OpenBIOS] [PATCH 6/10] Wrapped HFS+ incorrectly recognized
by Mark Cave-Ayland 10 Aug '11

10 Aug '11

On 09/08/11 22:54, William Hahne wrote: > The code for checking for a wrapped HFS+ volume looks at the wrong offset. > > Index: fs/hfsplus/hfsp_volume.c > =================================================================== > --- fs/hfsplus/hfsp_volume.c (revision 1041) > +++ fs/hfsplus/hfsp_volume.c (working copy) > @@ -302,7 +302,7 @@ > os_read(fd, vol, 2, HFSP_BLOCKSZ_BITS); > if (__be16_to_cpu(vol[0]) == HFS_VOLHEAD_SIG && > - __be16_to_cpu(vol[0x7c]) == HFSP_VOLHEAD_SIG) { > + __be16_to_cpu(vol[0x3e]) == HFSP_VOLHEAD_SIG) { > ret = -1; > } else if (__be16_to_cpu(vol[0]) == HFSP_VOLHEAD_SIG) { > ret = -1; Same comment as for patch 2/10. ATB, Mark. -- Mark Cave-Ayland - Senior Technical Architect PostgreSQL - PostGIS Sirius Corporation plc - control through freedom http://www.siriusit.co.uk t: +44 870 608 0063 Sirius Labs: http://www.siriusit.co.uk/labs

2 1

Re: [OpenBIOS] [PATCH 3/10] Don't stop at APPLE_VOID partition
by Mark Cave-Ayland 10 Aug '11

10 Aug '11

On 09/08/11 22:54, William Hahne wrote: > The void partitions can be occasionally found on Mac OS discs (10.2 for > example.) Previously mac-parts would just stop at the first void > partition meaning it would never try to boot from the HFS partition. > > Index: packages/mac-parts.c > =================================================================== > --- packages/mac-parts.c (revision 1041) > +++ packages/mac-parts.c (working copy) > @@ -165,9 +165,12 @@ > for (parnum = 1; parnum <= __be32_to_cpu(par.pmMapBlkCnt); parnum++) { > SEEK( bs * parnum ); > READ( &par, sizeof(par) ); > - if( __be16_to_cpu(par.pmSig) != DESC_PART_SIGNATURE || > - !__be32_to_cpu(par.pmPartBlkCnt) ) > + if( __be16_to_cpu(par.pmSig) != DESC_PART_SIGNATURE) { > break; > + } > + if ( !__be32_to_cpu(par.pmPartBlkCnt) ) { > + continue; /* Just a void partition, ignore it. */ > + } > DPRINTF("found partition type: %s with status %x\n", par.pmPartType, > __be32_to_cpu(par.pmPartStatus)); Same as my comment for patch 2/10. ATB, Mark. -- Mark Cave-Ayland - Senior Technical Architect PostgreSQL - PostGIS Sirius Corporation plc - control through freedom http://www.siriusit.co.uk t: +44 870 608 0063 Sirius Labs: http://www.siriusit.co.uk/labs

2 1

Re: [OpenBIOS] [PATCH 2/10] Ignore extraneous partitions
by Mark Cave-Ayland 10 Aug '11

10 Aug '11

On 09/08/11 22:54, William Hahne wrote: > BootX will only check the first few partitions from mac-parts to find > the Mac OS partition. So it must expect drivers and void partitions to > be ignored. This patch makes it so that if you specify the partition > cd:1 and partition 1 is a driver it will keep checking partitions until > reaching the first non-driver and non-void partition. > > Index: packages/mac-parts.c > =================================================================== > --- packages/mac-parts.c (revision 1041) > +++ packages/mac-parts.c (working copy) > @@ -200,15 +203,26 @@ > } else { > /* Another partition was explicitly requested */ > - SEEK( bs * parnum ); > - READ( &par, sizeof(par) ); > + > + /* We have to find the first valid partition corresponding to the > given number. > + This is because Apple sometimes includes a bunch of extra partitions > that > + BootX expects to be ignored */ > + while (parnum < par.pmMapBlkCnt) { > + SEEK( bs * parnum ); > + READ( &par, sizeof(par) ); > - if( (__be32_to_cpu(par.pmPartStatus) & kPartitionAUXIsValid) && > - (__be32_to_cpu(par.pmPartStatus) & kPartitionAUXIsAllocated) && > - (__be32_to_cpu(par.pmPartStatus) & kPartitionAUXIsReadable) ) { > + /* Check if valid, also device driver partitions are expected to be > ignored */ > + if( (__be32_to_cpu(par.pmPartStatus) & kPartitionAUXIsValid) && > + (__be32_to_cpu(par.pmPartStatus) & kPartitionAUXIsAllocated) && > + (__be32_to_cpu(par.pmPartStatus) & kPartitionAUXIsReadable) && > + !(__be32_to_cpu(par.pmPartStatus) & kPartitionIsRealDeviceDriver)) { > - offs = (long long)__be32_to_cpu(par.pmPyPartStart) * bs; > - size = (long long)__be32_to_cpu(par.pmPartBlkCnt) * bs; > + offs = (long long)__be32_to_cpu(par.pmPyPartStart) * bs; > + size = (long long)__be32_to_cpu(par.pmPartBlkCnt) * bs; > + break; > + } > + > + parnum++; > } > } I remember last time I reworked this, I had to spend a lot of time reading the Apple HFS documentation with regard to this. Could you add a link to the documentation on the Apple site into the commit message, and confirm whether this is something I missed which is documented or it's new behaviour that you have discoverd through testing. ATB, Mark. -- Mark Cave-Ayland - Senior Technical Architect PostgreSQL - PostGIS Sirius Corporation plc - control through freedom http://www.siriusit.co.uk t: +44 870 608 0063 Sirius Labs: http://www.siriusit.co.uk/labs

2 1

Re: [OpenBIOS] [PATCH 1/10] Dummy get-key-map in keyboard device package
by Mark Cave-Ayland 10 Aug '11

10 Aug '11

On 09/08/11 22:54, William Hahne wrote: > "get-key-map" is a Forth word that provides a map of all the keys > currently pressed on the keyboard. Since it is only used by BootX to > determine if one of the shortcuts for verbose or single user mode is > pressed. It is a waste of time to provide a full implementation. This > just hands out a dummy key map which specifies verbose mode. > > Index: drivers/adb_kbd.c > =================================================================== > --- drivers/adb_kbd.c (revision 1041) > +++ drivers/adb_kbd.c (working copy) > @@ -43,10 +43,13 @@ > static void keyboard_read(void); > +static void keyboard_get_key_map(void); > + > NODE_METHODS( keyboard ) = { > { "open", keyboard_open }, > { "close", keyboard_close }, > { "read", keyboard_read }, > + { "get-key-map", keyboard_get_key_map }, > }; > /* VT100 escape sequences */ > @@ -566,3 +569,18 @@ > } > PUSH(i); > } > + > +static unsigned char fake_keymap[32] = { > + 0x00, 0x00, 0x00, 0x40, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > + 0x00, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x00 > +}; > + > +static void keyboard_get_key_map(void) > +{ > + /* This is a bit of a hack since BootX requires a get-key-map word. > + In the future this should be changed to give the actual current > keymap, > + but since this only effects keyboard input in BootX it isn't a > priority. */ > + PUSH( (int)fake_keymap ); > +} Can you add a reference in the commit message to the documentation for the keymap layout? It's not clear how you generated the contents of the keymap blob. ATB, Mark. -- Mark Cave-Ayland - Senior Technical Architect PostgreSQL - PostGIS Sirius Corporation plc - control through freedom http://www.siriusit.co.uk t: +44 870 608 0063 Sirius Labs: http://www.siriusit.co.uk/labs

2 1

Re: [OpenBIOS] [PATCH 4/10] Place framebuffer address in frame-buffer-adr
by Mark Cave-Ayland 10 Aug '11

10 Aug '11

On 09/08/11 22:54, William Hahne wrote: > BootX gets the framebuffer address to provide to Mac OS from the > frame-buffer-adr value. This value was previously just set to null. > > Index: packages/video.c > =================================================================== > --- packages/video.c (revision 1041) > +++ packages/video.c (working copy) > @@ -332,6 +369,9 @@ > video.has_video = 1; > video.pal = malloc( 256 * sizeof(unsigned long) ); > + PUSH(video.fb.mphys); > + feval("to frame-buffer-adr"); > + > #ifdef CONFIG_PPC > s = (video.fb.mphys & 0xfff); > size = ((video.fb.h * video.fb.rb + s) + 0xfff) & ~0xfff; > I think this looks sensible, but will defer to the PPC maintainers. ATB, Mark. -- Mark Cave-Ayland - Senior Technical Architect PostgreSQL - PostGIS Sirius Corporation plc - control through freedom http://www.siriusit.co.uk t: +44 870 608 0063 Sirius Labs: http://www.siriusit.co.uk/labs

1 0

Re: [OpenBIOS] [PATCH 0/10] Patches for booting Mac OS X
by Mark Cave-Ayland 10 Aug '11

10 Aug '11

On 09/08/11 22:54, William Hahne wrote: > Hello, > > I apologize for my earlier post which I failed to break up. I have not > done much posting to mailing lists. These 10 patches it all relate to > getting OpenBIOS able to boot Mac OS X on Qemu. While Mac OS does not > yet fully boot, with these patches I have gotten the Mac OS 10.2 kernel > to load and begin booting. The rest of the work will mostly be in Qemu. > > William Hahne > > M kernel/forth.c > M kernel/bootstrap.c > M forth/admin/iocontrol.fs > M forth/bootstrap/bootstrap.fs > M fs/hfsplus/hfsp_volume.c > M arch/ppc/qemu/init.c > M packages/video.c > M packages/mac-parts.c > M drivers/adb_kbd.c Hi William, Thanks for this - this looks like a very interesting patch series. Comments to follow shortly. ATB, Mark. -- Mark Cave-Ayland - Senior Technical Architect PostgreSQL - PostGIS Sirius Corporation plc - control through freedom http://www.siriusit.co.uk t: +44 870 608 0063 Sirius Labs: http://www.siriusit.co.uk/labs

1 0

Re: [OpenBIOS] [PATCH 9/10] Divide by zero check
by Artyom Tarasenko 10 Aug '11

10 Aug '11

On Tue, Aug 9, 2011 at 11:55 PM, William Hahne <will07c5(a)gmail.com> wrote: > It is possible for BootX to purposefully divide by zero expecting 0 as the > result. OBP behaves differently. It puts the result only if there was no devision by zero: ok showstack ok 0 0 / Trap 3e ok 1 0 / Trap 3e ok 10 8 / 2 ok Maybe it has to be done platform-dependent. > Index: kernel/forth.c > =================================================================== > --- kernel/forth.c (revision 1041) > +++ kernel/forth.c (working copy) > @@ -1157,6 +1157,12 @@ > { > const ucell b = POP(); > const ducell a = DPOP(); > + > + if (b == 0) { // can't divide by zero > + PUSH(0); > + DPUSH(0); > + return; > + } > #ifdef NEED_FAKE_INT128_T > if (a.hi != 0) { > fprintf(stderr, "mudivmod called (0x%016llx %016llx / > 0x%016llx)\n", > -- > OpenBIOS http://openbios.org/ > Mailinglist: http://lists.openbios.org/mailman/listinfo > Free your System - May the Forth be with you > -- Regards, Artyom Tarasenko solaris/sparc under qemu blog: http://tyom.blogspot.com/

1 0

← Newer
1
2
3
4
5
6
7
8
9
Older →