Re: [patch] rough Mac OS X loader

[Marco Gerards <mgerards@xs4all.nl>]

Hollis Blanchard <hollis@penguinppc.org> writes:

Hi Hollis,

> I've been working on a Mac OS X loader for GRUB. During the usual OS X
> boot process, the firmware loads the BootX bootloader (which is a
> hybrid CHRP script and XCOFF binary), which has a Mach-O loader and
> loads the OS X Mach kernel.

This is really great news!  Thanks a lot for doing this great job. :-)

> Rather than trying to load and run the Mach kernel directly, I chose
> to focus on the BootX bootloader, since it seems to do a fair amount
> of setup work that I would rather not duplicate in GRUB. Also
> important: if OS X is ever updated (for example, the parameter-passing
> mechanism between BootX and the kernel), we would need to track those
> changes, which is a lot more long-term maintenance.

Agreed.

> Unfortunately, BootX attempts to claim all memory from 0 to 96MiB,
> which conflicts with GRUB running in this area. To get around that
> problem, I install a trampoline just above 96MiB which releases all of
> GRUB's memory and jumps into BootX. It's a bit of a hack, and for
> example if the code being copied exceeds some fixed size (currently
> 0x200 bytes) things will fail mysteriously. I'm open to suggestions on
> that problem.
>
> I'm afraid I don't know if this code works yet. Because the current
> HFS+ driver doesn't support the HFS wrapper found on all Apple
> filesystems, I can't boot from an HFS+ installation. I've copied BootX
> to a plain HFS partition, and that works somewhat (I expect it fails
> when it cannot find /mach_kernel). I'm relatively optimistic, and I'm
> working on the HFS wrapper support now.

Cool.  If you need any help let me know.  I will look into this as
well, I assume we can discuss things in detail on IRC.

> There are a few rough edges still, but if there are no comments I will
> be checking this patch in largely unchanged some day.

Can you provide a changelog entry? :-)

There are some comments below, I hope some of them even make sense. ;)

>  include $(srcdir)/conf/common.mk
> Index: include/grub/xcoff.h
> ===================================================================

Is it possible to use the grub_ namespace in this file?  In elf.h this
is not done because it should be possible to update it from glibc,
AFAIK.

> Index: include/grub/powerpc/ieee1275/kernel.h
> ===================================================================
> RCS file: /cvsroot/grub/grub2/include/grub/powerpc/ieee1275/kernel.h,v
> retrieving revision 1.3
> diff -u -p -r1.3 kernel.h
> --- include/grub/powerpc/ieee1275/kernel.h	3 Aug 2005 22:53:50 -0000	1.3
> +++ include/grub/powerpc/ieee1275/kernel.h	2 Jan 2006 00:34:11 -0000
> @@ -26,6 +26,9 @@ void EXPORT_FUNC (abort) (void);
>  void EXPORT_FUNC (grub_reboot) (void);
>  void EXPORT_FUNC (grub_halt) (void);
>  
> +void EXPORT_FUNC (grub_jump) (unsigned long text, unsigned long stack,
> +			      unsigned long arg1, unsigned long arg2);

Shouldn't a pointer be used here?

> +/* BootX, the Mac OS X bootloader, is an XCOFF executable with a CHRP script
> + * prepended to it.  We skip the script and load the XCOFF file.  */

What is in this script?  Are you completely sure it can be skipped?

Can you please remove the `*' on the second line so the comment
matches the style of the other sourcecode?


> +static grub_err_t
> +grub_macosx_release_mem (void)
> +{
> +  /* XXX write me */

I assume you encountered a bug in the firmware while writing this
function? :-)

> +  /* Load all section headers.  */
> +  scnhdr_bytes = filehdr->f_nscns * sizeof (struct xcoff_scnhdr);
> +  scnhdrs = grub_malloc (scnhdr_bytes);
> +  if (! scnhdrs)
> +    return grub_errno;
> +
> +  if (grub_file_read (file, (void *)scnhdrs, scnhdr_bytes) != scnhdr_bytes)
> +    return grub_error (GRUB_ERR_READ_ERROR,
> +		       "could not read XCOFF section headers");

You don't free scnhdrs here, is that intentional?

> +/* Find NULL-terminated `needle' in non-terminated `haystack'.  */
> +static void *
> +grub_memstr (void *haystack, int len, char *needle)

Perhaps it is better to move this to kern/misc.c?

> +/* grub_chain_trampoline is copied somewhere other than its link address
> + * and executes after all other code has been blown away. In order to call
> + * other functions from here, they must have been copied into a safe place and
> + * their addresses passed as parameters. */

Same as above.

> +typedef void (*kernel_entry_t) (unsigned long, unsigned long, int (void *));
> +typedef int (*release_t) (grub_addr_t addr, grub_size_t size);
> +static void
> +grub_chain_trampoline (kernel_entry_t entry, release_t release)
> +{
> +  release (0x00003000, 0x06000000 - 0x3000);
> +  entry (0, 0, grub_ieee1275_entry_fn);
> +}
> +
> +static int
> +grub_macosx_install_trampoline (void)
> +{
> +  int rc;
> +
> +  /* XXX don't use magic numbers here */
> +  rc = grub_claimmap (GRUB_MACOSX_CHAIN_AREA, 0x200);
> +  rc |= grub_claimmap (GRUB_MACOSX_RELEASE_AREA, 0x200);
> +  rc |= grub_claimmap (GRUB_MACOSX_STACK_AREA, 0x200);
> +  if (rc)
> +    return rc;
> +
> +  grub_memcpy ((void *) GRUB_MACOSX_CHAIN_AREA, grub_chain_trampoline, 0x200);
> +  grub_arch_sync_caches ((void *) GRUB_MACOSX_CHAIN_AREA, 0x200);
> +  grub_memcpy ((void *) GRUB_MACOSX_RELEASE_AREA, grub_ieee1275_release, 0x200);
> +  grub_arch_sync_caches ((void *) GRUB_MACOSX_RELEASE_AREA, 0x200);

What is 0x200?  The maximum size of grub_chain_trampoline?  Perhaps
you can do something like: `grub_macosx_release_area -
grub_chain_trampoline', or do you think that is too unpredictable and
too ugly?

Thanks,
Marco