Re: [OpenBIOS] [PATCH] libopenbios/bootinfo_load.c: Implement Mac OS 9.2 boot script support

[BALATON Zoltan]

On Tue, 26 Apr 2016, Programmingkid wrote:
> On Apr 26, 2016, at 4:11 PM, BALATON Zoltan wrote:
> > On Tue, 26 Apr 2016, Programmingkid wrote:
> > > This patch makes booting Mac OS 9.2 possible. It replaces all occurrences of
> > > > r and r> with sub_>r and sub_r>. This allows for the return stack to be left
> > >
> > > alone.
> >
> > Can we have Segher's Forth version of this instead:
> >
> > https://www.coreboot.org/pipermail/openbios/2016-April/009338.html
> >
> > That should be equivalent and would make the C code much simpler and 
> > allow dropping most of these patches.
>
> That code doesn't work. It causes a "Dictionary space overflow" message
> to be printed. My code does work.

Maybe you need to increase the available cells to have enough space? I 
don't know much about Forth to understand the code better.
> Here is my patch I made to test out that code. If you have the ability
> please test out any changes before submitting any patches.
> Index: arch/ppc/ppc.fs
>
> --- arch/ppc/ppc.fs	(revision 1395)
> +++ arch/ppc/ppc.fs	(working copy)
> @@ -66,3 +66,11 @@
>
> \ Set by BootX when booting Mac OS X
> defer spin
>
> +hex
> +20 cells CREATE stash  VARIABLE #stash  #stash off
> +: stash-push  stash #stash cells + !  1 stash +! ;
> +: stash-pop   -1 stash +!  stash #stash cells + @ ;
> +: >r  state @ IF postpone >r EXIT THEN  stash-push ; immediate
> +: r>  state @ IF postpone r> EXIT THEN  stash-pop ; immediate
> +: r@  state @ IF postpone r@ EXIT THEN  stash-pop dup stash-push ; immediate

Actually what is the problem with the default implementations of these 
words that makes us need a replacement for them? Is it that the space they 
alloc from is too small and cannot be increased due to ROM limits? Can we 
change them to alloc from memory then or are they used while setting up 
memory?

I think the only difference with your patch is maybe that yours alloc from 
memory while these use some Forth space which may not be large enough, so 
probably the right fix could be increasing the Forth space by moving that 
to RAM instead of the limited ROM area or changing these words to move the 
stack to RAM when they run out of ROM space.

These all are just guessing so I might be completely off here though, I
haven't checked the code.

Also it worked with just adding \r as line terminator, so maybe the 
default implementation of the r-stack is OK and only the below part for 
line-by-line execution is needed? Have you tested that?

Regards,
BALATON Zoltan
> Index: libopenbios/bootinfo_load.c
>
> --- libopenbios/bootinfo_load.c	(revision 1395)
> +++ libopenbios/bootinfo_load.c	(working copy)
> @@ -116,6 +116,33 @@
>    return LOADER_NOT_SUPPORT;
> }
>
> +/* Runs the bootscript one line at a time */
> +void run_script(char *bootscript)
> +{
> int index = 0, buf_index = 0;
> char c;
> char *buffer = malloc(1000 * sizeof(char));
> while (1) {
>    c = bootscript[index];
>    // fill up buffer
>    while (c != '\n' && c != '\r' && c != '\0') {
>        buffer[buf_index++] = c;
>        c = bootscript[++index];
>    }
>    buffer[buf_index] = '\0';
>    buf_index = 0;
>    index++;
>    printk("%s\n", buffer);
>    feval(buffer);
>    if (c == '\0') {
>        break;
>    }
> }
> free(buffer);
>
> +}
>
> /*
>   Parse SGML structure like:
>
>   <chrp-boot>
> @@ -262,7 +289,8 @@
>    /* If the payload is bootinfo then we execute it immediately */
>    if (scriptvalid) {
>    	DPRINTF("bootscript: %s\n", bootscript);
> -		feval(bootscript);
> +		//feval(bootscript);
> +        run_script(bootscript);
>    }
>    else
>    	DPRINTF("Unable to parse bootinfo bootscript\n");