Re: [PATCH] PXE support for grub2

[Bean]

On Sat, Aug 2, 2008 at 6:48 PM, Robert Millan <address@hidden> wrote:
> On Thu, Jul 31, 2008 at 12:50:22AM +0800, Bean wrote:
>> +#if 1
>> +#define GRUB_PXE_BOOTP_DHCPVEND      1024    /* DHCP extended vendor field 
>> size */
>> +#else
>> +#define GRUB_PXE_BOOTP_DHCPVEND      312     /* DHCP standard vendor field 
>> size */
>> +#endif
>
> A comment would be helpful to explain the 'if 1' option.
>
>> --- a/kern/i386/pc/init.c
>> +++ b/kern/i386/pc/init.c
>> @@ -71,14 +71,19 @@ make_install_device (void)
>>        if (grub_root_drive == 0xFF)
>>          grub_root_drive = grub_boot_drive;
>>
>> -      grub_sprintf (dev, "(%cd%u", (grub_root_drive & 0x80) ? 'h' : 'f',
>> -                    grub_root_drive & 0x7f);
>> +      if (grub_root_drive == 0x7F)
>> +        grub_strcpy (dev, "(pxe");
>> +      else
>> +        {
>> +          grub_sprintf (dev, "(%cd%u", (grub_root_drive & 0x80) ? 'h' : 'f',
>> +                        grub_root_drive & 0x7f);
>>
>> -      if (grub_install_dos_part >= 0)
>> -     grub_sprintf (dev + grub_strlen (dev), ",%u", grub_install_dos_part + 
>> 1);
>> +          if (grub_install_dos_part >= 0)
>> +         grub_sprintf (dev + grub_strlen (dev), ",%u", 
>> grub_install_dos_part + 1);
>>
>> -      if (grub_install_bsd_part >= 0)
>> -     grub_sprintf (dev + grub_strlen (dev), ",%c", grub_install_bsd_part + 
>> 'a');
>> +          if (grub_install_bsd_part >= 0)
>> +         grub_sprintf (dev + grub_strlen (dev), ",%c", 
>> grub_install_bsd_part + 'a');
>> +     }
>>
>>        grub_sprintf (dev + grub_strlen (dev), ")%s", grub_prefix);
>>        grub_strcpy (grub_prefix, dev);
>
> Can we avoid this?  I wish make_install_device() was reduced and eventually
> disappeared.
>
> Since we already know at install time that the boot drive will be 0x7f, why 
> not
> just set the drive via grub-mkimage --prefix?

Hi,

Yes, we can set this in grub-mkimage.

>
>> --- a/kern/i386/pc/startup.S
>> +++ b/kern/i386/pc/startup.S
>> @@ -2053,3 +2053,102 @@ FUNCTION(grub_vbe_bios_set_palette_data)
>>       popl    %ebx
>>       popl    %ebp
>>       ret
>> +
>> +
>> +pxe_rm_entry:
>> +     .long   0
>> +
>> +/*
>> + * struct grub_pxenv *grub_pxe_scan (void);
>> + */
>> +FUNCTION(grub_pxe_scan)
>
> Could these go in a module?  If they're only used by disk/i386/pc/pxe.c,
> maybe they could be merged with that?

The problem is that they need to call real mode function provided by
the pxe rom, so this code must be loaded under 1m. We can solve this
with real mode stub or the patch suggested by daChaac, but it's not
there yet. So we need to keep it in startup.S for some time.

-- 
Bean