[PATCH] ntldr support

[PATCH] ntldr support

[Robert Millan]

[-- Attachment #1: Type: text/plain, Size: 581 bytes --]


Hi,

This patch implements a loader for NTLDR boot semantics (which are
the same in BootMGR, hence both are supported).

It still needs some cleanup in chainloader.c before it can be merged [1],
but I submit it so that others can test it and report if it works for
them.

[1] ideally, we should be able to share most code with the chainloader

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

[-- Attachment #2: ntldr.diff --]
[-- Type: text/x-diff, Size: 7002 bytes --]

Index: kern/i386/pc/startup.S
===================================================================
--- kern/i386/pc/startup.S	(revision 2465)
+++ kern/i386/pc/startup.S	(working copy)
@@ -1,6 +1,6 @@
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 1999,2000,2001,2002,2003,2005,2006,2007,2008 Free Software Foundation, Inc.
+ *  Copyright (C) 1999,2000,2001,2002,2003,2005,2006,2007,2008,2009 Free Software Foundation, Inc.
  *
  *  GRUB is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -576,6 +576,35 @@
 	ljmp	$0, $GRUB_MEMORY_MACHINE_BOOT_LOADER_ADDR
 	.code32
 
+/*
+ *  void grub_ntldr_real_boot (grub_uint8_t *boot_file, int drive, grub_uint32_t boot_file_size)
+ *
+ *  This starts NTLDR.
+ *
+ *  Register allocations for parameters:
+ *  %eax		*boot_file
+ *  %edx		drive
+ *  %ecx		boot_file_size
+ */
+
+ntldr_tmp_stack:
+	.long	0
+FUNCTION(grub_ntldr_real_boot)
+	/* Copy NTLDR from heap to its link address.  */
+	movl	%eax, %esi
+	movl	$GRUB_MEMORY_MACHINE_NTLDR_ADDR, %edi
+	cld
+	rep
+	movsb
+
+	/* Relocation may have taken over our stack, so use this one to
+	   hold our return address.  */
+	movl	$(ntldr_tmp_stack + 4), %esp
+	call	prot_to_real
+	.code16
+	ljmp	$(GRUB_MEMORY_MACHINE_NTLDR_ADDR >> 4), $0x0
+	.code32
+
 #include "../loader.S"
 
 /*
Index: include/grub/i386/pc/console.h
===================================================================
--- include/grub/i386/pc/console.h	(revision 2465)
+++ include/grub/i386/pc/console.h	(working copy)
@@ -51,7 +51,7 @@
 void grub_console_init (void);
 
 /* Finish the console system.  */
-void grub_console_fini (void);
+void EXPORT_FUNC(grub_console_fini) (void);
 
 #endif
 
Index: include/grub/i386/pc/loader.h
===================================================================
--- include/grub/i386/pc/loader.h	(revision 2465)
+++ include/grub/i386/pc/loader.h	(working copy)
@@ -1,6 +1,6 @@
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2002,2003,2004,2007  Free Software Foundation, Inc.
+ *  Copyright (C) 2002,2003,2004,2007,2009  Free Software Foundation, Inc.
  *
  *  GRUB is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -25,4 +25,7 @@
 /* This is an asm part of the chainloader.  */
 void EXPORT_FUNC(grub_chainloader_real_boot) (int drive, void *part_addr) __attribute__ ((noreturn));
 
+void EXPORT_FUNC(grub_ntldr_real_boot) (grub_uint8_t *boot_file, int drive,
+					grub_uint32_t boot_file_size) __attribute__ ((noreturn));
+
 #endif /* ! GRUB_LOADER_MACHINE_HEADER */
Index: include/grub/i386/pc/memory.h
===================================================================
--- include/grub/i386/pc/memory.h	(revision 2465)
+++ include/grub/i386/pc/memory.h	(working copy)
@@ -1,7 +1,7 @@
 /* memory.h - describe the memory map */
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2002,2007,2008  Free Software Foundation, Inc.
+ *  Copyright (C) 2002,2007,2008,2009  Free Software Foundation, Inc.
  *
  *  GRUB is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -63,6 +63,9 @@
 /* The address where another boot loader is loaded.  */
 #define GRUB_MEMORY_MACHINE_BOOT_LOADER_ADDR	0x7c00
 
+/* The address where NTLDR is loaded.  */
+#define GRUB_MEMORY_MACHINE_NTLDR_ADDR		0x20000
+
 /* The flag for protected mode.  */
 #define GRUB_MEMORY_MACHINE_CR0_PE_ON		0x1
 
Index: loader/i386/pc/chainloader.c
===================================================================
--- loader/i386/pc/chainloader.c	(revision 2465)
+++ loader/i386/pc/chainloader.c	(working copy)
@@ -1,7 +1,7 @@
 /* chainloader.c - boot another boot loader */
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2002,2004,2007  Free Software Foundation, Inc.
+ *  Copyright (C) 2002,2004,2007,2009  Free Software Foundation, Inc.
  *
  *  GRUB is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -20,6 +20,7 @@
 #include <grub/loader.h>
 #include <grub/machine/loader.h>
 #include <grub/machine/chainloader.h>
+#include <grub/machine/console.h>
 #include <grub/file.h>
 #include <grub/err.h>
 #include <grub/device.h>
@@ -36,12 +37,17 @@
 static grub_dl_t my_mod;
 static int boot_drive;
 static void *boot_part_addr;
+static grub_uint8_t *boot_file;
+static grub_off_t boot_file_size;
 
 static grub_err_t
 grub_chainloader_boot (void)
 {
-  grub_chainloader_real_boot (boot_drive, boot_part_addr);
+  /* NTLDR expects console in text mode.  */
+  grub_console_fini ();
 
+  grub_ntldr_real_boot (boot_file, boot_drive, (grub_uint32_t) boot_file_size);
+
   /* Never reach here.  */
   return GRUB_ERR_NONE;
 }
@@ -68,24 +74,11 @@
   if (! file)
     goto fail;
 
-  /* Read the first block.  */
-  if (grub_file_read (file, (void *) 0x7C00, GRUB_DISK_SECTOR_SIZE)
-      != GRUB_DISK_SECTOR_SIZE)
-    {
-      if (grub_errno == GRUB_ERR_NONE)
-	grub_error (GRUB_ERR_BAD_OS, "too small");
+  boot_file_size = file->size;
 
-      goto fail;
-    }
+  boot_file = grub_malloc (boot_file_size);
 
-  /* Check the signature.  */
-  signature = *((grub_uint16_t *) (0x7C00 + GRUB_DISK_SECTOR_SIZE - 2));
-  if (signature != grub_le_to_cpu16 (0xaa55)
-      && ! (flags & GRUB_CHAINLOADER_FORCE))
-    {
-      grub_error (GRUB_ERR_BAD_OS, "invalid signature");
-      goto fail;
-    }
+  grub_file_read (file, boot_file, boot_file_size);
 
   grub_file_close (file);
 
@@ -95,11 +88,26 @@
   if (dev && dev->disk && dev->disk->partition)
     {
       grub_disk_read (dev->disk, dev->disk->partition->offset, 446, 64,
-		      (void *) GRUB_MEMORY_MACHINE_PART_TABLE_ADDR);
+		      GRUB_MEMORY_MACHINE_PART_TABLE_ADDR);
       part_addr = (void *) (GRUB_MEMORY_MACHINE_PART_TABLE_ADDR
 			    + (dev->disk->partition->index << 4));
     }
 
+  if (! dev)
+    grub_fatal ("boo");
+
+  /* Read the first block.  */
+  grub_disk_read (dev->disk, 0, 0, GRUB_DISK_SECTOR_SIZE, (void *) 0x7C00);
+
+  /* Check the signature.  */
+  signature = *((grub_uint16_t *) (0x7C00 + GRUB_DISK_SECTOR_SIZE - 2));
+  if (signature != grub_le_to_cpu16 (0xaa55)
+      && ! (flags & GRUB_CHAINLOADER_FORCE))
+    {
+      grub_error (GRUB_ERR_BAD_OS, "invalid signature");
+      goto fail;
+    }
+
   if (dev)
     grub_device_close (dev);
   
@@ -109,7 +117,7 @@
   boot_drive = drive;
   boot_part_addr = part_addr;
 
-  grub_loader_set (grub_chainloader_boot, grub_chainloader_unload, 1);
+  grub_loader_set (grub_chainloader_boot, grub_chainloader_unload, 0);
   return;
 
  fail:
@@ -145,8 +153,8 @@
 
 GRUB_MOD_INIT(chainloader)
 {
-  cmd = grub_register_command ("chainloader", grub_cmd_chainloader,
-			       0, "load another boot loader");
+  cmd = grub_register_command ("ntldr", grub_cmd_chainloader,
+			       0, "load NTLDR");
   my_mod = mod;
 }
 

Re: [PATCH] ntldr support

[Christian Franke]

Robert Millan wrote:
> This patch implements a loader for NTLDR boot semantics (which are
> the same in BootMGR, hence both are supported).
>
> It still needs some cleanup in chainloader.c before it can be merged [1],
> but I submit it so that others can test it and report if it works for
> them.
>
>   

Tested with WinXP in a VM:

Before the test, original boot code behind partition boot sector was 
destroyed.
(dd if=/dev/zero of=/dev/hda1 bs=512 seek=1 count=20)

Then this fails:

 root=(hd0,1)
 chainloader (hd0,1)+1
 boot

But with the patch this still works:

 root=(hd0,1)
 ntldr /ntldr
 boot

Thanks.

-- 
Regards,
Christian Franke

Re: [PATCH] ntldr support

[Robert Millan]

After thinking a bit about this, I don't think we want this command in
its current form.

The problem is it is misleading.  It leads the user to think it can load
ntldr as a standalone file, but in fact it is reading the PBR behind the
scenes.

If we want this feature at all, I think it should be an option in the
chainloader rather than a standalone command.  It's almost the same as
the chainloader really, the only difference is that ntldr is load after
PBR by GRUB instead of by PBR itself.

But I'm not even sure it makes sense to support this at all.  Perhaps if
people who are interested in it explain the use case and how did they got
there, I would change my mind about it.

On Mon, Aug 03, 2009 at 01:24:22AM +0200, Robert Millan wrote:
> 
> Hi,
> 
> This patch implements a loader for NTLDR boot semantics (which are
> the same in BootMGR, hence both are supported).
> 
> It still needs some cleanup in chainloader.c before it can be merged [1],
> but I submit it so that others can test it and report if it works for
> them.
> 
> [1] ideally, we should be able to share most code with the chainloader
> 
> -- 
> Robert Millan
> 
>   The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
>   how) you may access your data; but nobody's threatening your freedom: we
>   still allow you to remove your data and not access it at all."

> Index: kern/i386/pc/startup.S
> ===================================================================
> --- kern/i386/pc/startup.S	(revision 2465)
> +++ kern/i386/pc/startup.S	(working copy)
> @@ -1,6 +1,6 @@
>  /*
>   *  GRUB  --  GRand Unified Bootloader
> - *  Copyright (C) 1999,2000,2001,2002,2003,2005,2006,2007,2008 Free Software Foundation, Inc.
> + *  Copyright (C) 1999,2000,2001,2002,2003,2005,2006,2007,2008,2009 Free Software Foundation, Inc.
>   *
>   *  GRUB is free software: you can redistribute it and/or modify
>   *  it under the terms of the GNU General Public License as published by
> @@ -576,6 +576,35 @@
>  	ljmp	$0, $GRUB_MEMORY_MACHINE_BOOT_LOADER_ADDR
>  	.code32
>  
> +/*
> + *  void grub_ntldr_real_boot (grub_uint8_t *boot_file, int drive, grub_uint32_t boot_file_size)
> + *
> + *  This starts NTLDR.
> + *
> + *  Register allocations for parameters:
> + *  %eax		*boot_file
> + *  %edx		drive
> + *  %ecx		boot_file_size
> + */
> +
> +ntldr_tmp_stack:
> +	.long	0
> +FUNCTION(grub_ntldr_real_boot)
> +	/* Copy NTLDR from heap to its link address.  */
> +	movl	%eax, %esi
> +	movl	$GRUB_MEMORY_MACHINE_NTLDR_ADDR, %edi
> +	cld
> +	rep
> +	movsb
> +
> +	/* Relocation may have taken over our stack, so use this one to
> +	   hold our return address.  */
> +	movl	$(ntldr_tmp_stack + 4), %esp
> +	call	prot_to_real
> +	.code16
> +	ljmp	$(GRUB_MEMORY_MACHINE_NTLDR_ADDR >> 4), $0x0
> +	.code32
> +
>  #include "../loader.S"
>  
>  /*
> Index: include/grub/i386/pc/console.h
> ===================================================================
> --- include/grub/i386/pc/console.h	(revision 2465)
> +++ include/grub/i386/pc/console.h	(working copy)
> @@ -51,7 +51,7 @@
>  void grub_console_init (void);
>  
>  /* Finish the console system.  */
> -void grub_console_fini (void);
> +void EXPORT_FUNC(grub_console_fini) (void);
>  
>  #endif
>  
> Index: include/grub/i386/pc/loader.h
> ===================================================================
> --- include/grub/i386/pc/loader.h	(revision 2465)
> +++ include/grub/i386/pc/loader.h	(working copy)
> @@ -1,6 +1,6 @@
>  /*
>   *  GRUB  --  GRand Unified Bootloader
> - *  Copyright (C) 2002,2003,2004,2007  Free Software Foundation, Inc.
> + *  Copyright (C) 2002,2003,2004,2007,2009  Free Software Foundation, Inc.
>   *
>   *  GRUB is free software: you can redistribute it and/or modify
>   *  it under the terms of the GNU General Public License as published by
> @@ -25,4 +25,7 @@
>  /* This is an asm part of the chainloader.  */
>  void EXPORT_FUNC(grub_chainloader_real_boot) (int drive, void *part_addr) __attribute__ ((noreturn));
>  
> +void EXPORT_FUNC(grub_ntldr_real_boot) (grub_uint8_t *boot_file, int drive,
> +					grub_uint32_t boot_file_size) __attribute__ ((noreturn));
> +
>  #endif /* ! GRUB_LOADER_MACHINE_HEADER */
> Index: include/grub/i386/pc/memory.h
> ===================================================================
> --- include/grub/i386/pc/memory.h	(revision 2465)
> +++ include/grub/i386/pc/memory.h	(working copy)
> @@ -1,7 +1,7 @@
>  /* memory.h - describe the memory map */
>  /*
>   *  GRUB  --  GRand Unified Bootloader
> - *  Copyright (C) 2002,2007,2008  Free Software Foundation, Inc.
> + *  Copyright (C) 2002,2007,2008,2009  Free Software Foundation, Inc.
>   *
>   *  GRUB is free software: you can redistribute it and/or modify
>   *  it under the terms of the GNU General Public License as published by
> @@ -63,6 +63,9 @@
>  /* The address where another boot loader is loaded.  */
>  #define GRUB_MEMORY_MACHINE_BOOT_LOADER_ADDR	0x7c00
>  
> +/* The address where NTLDR is loaded.  */
> +#define GRUB_MEMORY_MACHINE_NTLDR_ADDR		0x20000
> +
>  /* The flag for protected mode.  */
>  #define GRUB_MEMORY_MACHINE_CR0_PE_ON		0x1
>  
> Index: loader/i386/pc/chainloader.c
> ===================================================================
> --- loader/i386/pc/chainloader.c	(revision 2465)
> +++ loader/i386/pc/chainloader.c	(working copy)
> @@ -1,7 +1,7 @@
>  /* chainloader.c - boot another boot loader */
>  /*
>   *  GRUB  --  GRand Unified Bootloader
> - *  Copyright (C) 2002,2004,2007  Free Software Foundation, Inc.
> + *  Copyright (C) 2002,2004,2007,2009  Free Software Foundation, Inc.
>   *
>   *  GRUB is free software: you can redistribute it and/or modify
>   *  it under the terms of the GNU General Public License as published by
> @@ -20,6 +20,7 @@
>  #include <grub/loader.h>
>  #include <grub/machine/loader.h>
>  #include <grub/machine/chainloader.h>
> +#include <grub/machine/console.h>
>  #include <grub/file.h>
>  #include <grub/err.h>
>  #include <grub/device.h>
> @@ -36,12 +37,17 @@
>  static grub_dl_t my_mod;
>  static int boot_drive;
>  static void *boot_part_addr;
> +static grub_uint8_t *boot_file;
> +static grub_off_t boot_file_size;
>  
>  static grub_err_t
>  grub_chainloader_boot (void)
>  {
> -  grub_chainloader_real_boot (boot_drive, boot_part_addr);
> +  /* NTLDR expects console in text mode.  */
> +  grub_console_fini ();
>  
> +  grub_ntldr_real_boot (boot_file, boot_drive, (grub_uint32_t) boot_file_size);
> +
>    /* Never reach here.  */
>    return GRUB_ERR_NONE;
>  }
> @@ -68,24 +74,11 @@
>    if (! file)
>      goto fail;
>  
> -  /* Read the first block.  */
> -  if (grub_file_read (file, (void *) 0x7C00, GRUB_DISK_SECTOR_SIZE)
> -      != GRUB_DISK_SECTOR_SIZE)
> -    {
> -      if (grub_errno == GRUB_ERR_NONE)
> -	grub_error (GRUB_ERR_BAD_OS, "too small");
> +  boot_file_size = file->size;
>  
> -      goto fail;
> -    }
> +  boot_file = grub_malloc (boot_file_size);
>  
> -  /* Check the signature.  */
> -  signature = *((grub_uint16_t *) (0x7C00 + GRUB_DISK_SECTOR_SIZE - 2));
> -  if (signature != grub_le_to_cpu16 (0xaa55)
> -      && ! (flags & GRUB_CHAINLOADER_FORCE))
> -    {
> -      grub_error (GRUB_ERR_BAD_OS, "invalid signature");
> -      goto fail;
> -    }
> +  grub_file_read (file, boot_file, boot_file_size);
>  
>    grub_file_close (file);
>  
> @@ -95,11 +88,26 @@
>    if (dev && dev->disk && dev->disk->partition)
>      {
>        grub_disk_read (dev->disk, dev->disk->partition->offset, 446, 64,
> -		      (void *) GRUB_MEMORY_MACHINE_PART_TABLE_ADDR);
> +		      GRUB_MEMORY_MACHINE_PART_TABLE_ADDR);
>        part_addr = (void *) (GRUB_MEMORY_MACHINE_PART_TABLE_ADDR
>  			    + (dev->disk->partition->index << 4));
>      }
>  
> +  if (! dev)
> +    grub_fatal ("boo");
> +
> +  /* Read the first block.  */
> +  grub_disk_read (dev->disk, 0, 0, GRUB_DISK_SECTOR_SIZE, (void *) 0x7C00);
> +
> +  /* Check the signature.  */
> +  signature = *((grub_uint16_t *) (0x7C00 + GRUB_DISK_SECTOR_SIZE - 2));
> +  if (signature != grub_le_to_cpu16 (0xaa55)
> +      && ! (flags & GRUB_CHAINLOADER_FORCE))
> +    {
> +      grub_error (GRUB_ERR_BAD_OS, "invalid signature");
> +      goto fail;
> +    }
> +
>    if (dev)
>      grub_device_close (dev);
>    
> @@ -109,7 +117,7 @@
>    boot_drive = drive;
>    boot_part_addr = part_addr;
>  
> -  grub_loader_set (grub_chainloader_boot, grub_chainloader_unload, 1);
> +  grub_loader_set (grub_chainloader_boot, grub_chainloader_unload, 0);
>    return;
>  
>   fail:
> @@ -145,8 +153,8 @@
>  
>  GRUB_MOD_INIT(chainloader)
>  {
> -  cmd = grub_register_command ("chainloader", grub_cmd_chainloader,
> -			       0, "load another boot loader");
> +  cmd = grub_register_command ("ntldr", grub_cmd_chainloader,
> +			       0, "load NTLDR");
>    my_mod = mod;
>  }
>  


-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [PATCH] ntldr support

[Vladimir 'phcoder' Serbinenko]

On Tue, Aug 4, 2009 at 9:27 PM, Robert Millan<rmh@aybabtu.com> wrote:
>
> After thinking a bit about this, I don't think we want this command in
> its current form.
>
> The problem is it is misleading.  It leads the user to think it can load
> ntldr as a standalone file, but in fact it is reading the PBR behind the
> scenes.
>
I don't think it's of any problem since ntldr uses this PBR only as
superblock to identify the partition. As such I would rather consider
this loading as a special case of passing $root, just the form of it
is a bit weird

-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git

Re: [PATCH] ntldr support

[Christian Franke]

Vladimir 'phcoder' Serbinenko wrote:
> On Tue, Aug 4, 2009 at 9:27 PM, Robert Millan<...> wrote:
>   
>> After thinking a bit about this, I don't think we want this command in
>> its current form.
>>
>> The problem is it is misleading.  It leads the user to think it can load
>> ntldr as a standalone file, but in fact it is reading the PBR behind the
>> scenes.
>>
>>     
> I don't think it's of any problem since ntldr uses this PBR only as
> superblock to identify the partition. As such I would rather consider
> this loading as a special case of passing $root, just the form of it
> is a bit weird
>
>   

Agree.

A 'ntldr' or 'chainloader --ntldr' command is not mandatory. But it is 
'nice to have' because it allows to boot even if the boot code (6 
sectors) in the area behind the PBR is not present for whatever reason. 
See my previsions mail with the test case.

-- 
Regards,
Christian Franke

Re: [PATCH] ntldr support

[Robert Millan]

On Wed, Aug 05, 2009 at 07:35:32AM +0200, Christian Franke wrote:
> Vladimir 'phcoder' Serbinenko wrote:
>> On Tue, Aug 4, 2009 at 9:27 PM, Robert Millan<...> wrote:
>>   
>>> After thinking a bit about this, I don't think we want this command in
>>> its current form.
>>>
>>> The problem is it is misleading.  It leads the user to think it can load
>>> ntldr as a standalone file, but in fact it is reading the PBR behind the
>>> scenes.
>>>
>>>     
>> I don't think it's of any problem since ntldr uses this PBR only as
>> superblock to identify the partition. As such I would rather consider
>> this loading as a special case of passing $root, just the form of it
>> is a bit weird
>>
>>   
>
> Agree.
>
> A 'ntldr' or 'chainloader --ntldr' command is not mandatory. But it is  
> 'nice to have' because it allows to boot even if the boot code (6  
> sectors) in the area behind the PBR is not present for whatever reason.  
> See my previsions mail with the test case.

Ok.  Should we settle with 'ntldr' or 'chainloader --ntldr'?  In either case,
we should try making them share the code (with ifdef trick if necessary), but
I'm not completely sure.  Will users be confused if they put ntldr in an
unrelated partition and try to boot their Windows using it?  If ntldr may
hang or crash because of it, then I would go for 'chainloader --ntldr' and
if it fails with a nice error then 'ntldr'.

Thoughts?

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [PATCH] ntldr support

[Michal Suchanek]

2009/8/7 Robert Millan <rmh@aybabtu.com>:
> On Wed, Aug 05, 2009 at 07:35:32AM +0200, Christian Franke wrote:
>> Vladimir 'phcoder' Serbinenko wrote:
>>> On Tue, Aug 4, 2009 at 9:27 PM, Robert Millan<...> wrote:
>>>
>>>> After thinking a bit about this, I don't think we want this command in
>>>> its current form.
>>>>
>>>> The problem is it is misleading.  It leads the user to think it can load
>>>> ntldr as a standalone file, but in fact it is reading the PBR behind the
>>>> scenes.
>>>>
>>>>
>>> I don't think it's of any problem since ntldr uses this PBR only as
>>> superblock to identify the partition. As such I would rather consider
>>> this loading as a special case of passing $root, just the form of it
>>> is a bit weird
>>>
>>>
>>
>> Agree.
>>
>> A 'ntldr' or 'chainloader --ntldr' command is not mandatory. But it is
>> 'nice to have' because it allows to boot even if the boot code (6
>> sectors) in the area behind the PBR is not present for whatever reason.
>> See my previsions mail with the test case.
>
> Ok.  Should we settle with 'ntldr' or 'chainloader --ntldr'?  In either case,
> we should try making them share the code (with ifdef trick if necessary), but
> I'm not completely sure.  Will users be confused if they put ntldr in an
> unrelated partition and try to boot their Windows using it?  If ntldr may
> hang or crash because of it, then I would go for 'chainloader --ntldr' and
> if it fails with a nice error then 'ntldr'.
>
> Thoughts?
>

ntldr is a boot loader like any other and it needs its configuration
and support files to work. Without them it fails (not sure how) but
that is not unexpected.

The Microsoft code for loading ntldr is broken wrt disk geometry so
replacing the MS loader with a grub chainloader option might allow
booting a disk taken from a system where the BIOS interprets the disk
geometry differently. This needs some testing, though.

Thanks

Michal

Re: [PATCH] ntldr support

[Robert Millan]

On Fri, Aug 07, 2009 at 01:17:30PM +0200, Michal Suchanek wrote:
> 
> ntldr is a boot loader like any other and it needs its configuration
> and support files to work. Without them it fails (not sure how) but
> that is not unexpected.

I tend to agree, but in this particular case, it's conceivable that Microsoft
didn't expect anyone to do this, and therefore neglected to provide a graceful
response.

Could anyone test it and report how this behaves?

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [PATCH] ntldr support

[Michal Suchanek]

2009/8/7 Robert Millan <rmh@aybabtu.com>:
> On Fri, Aug 07, 2009 at 01:17:30PM +0200, Michal Suchanek wrote:
>>
>> ntldr is a boot loader like any other and it needs its configuration
>> and support files to work. Without them it fails (not sure how) but
>> that is not unexpected.
>
> I tend to agree, but in this particular case, it's conceivable that Microsoft
> didn't expect anyone to do this, and therefore neglected to provide a graceful
> response.
>
> Could anyone test it and report how this behaves?
>
> --
> Robert Millan
>
>  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
>  how) you may access your data; but nobody's threatening your freedom: we
>  still allow you to remove your data and not access it at all."
>

I tried putting the ntldr binary on an empty fat32 disk and loading
with the grub ntldr command.

The system just reboots.

Thanks

Michal

Re: [PATCH] ntldr support

[Yves Blusseau]

Le 7 août 09 à 13:43, Robert Millan a écrit :

> On Fri, Aug 07, 2009 at 01:17:30PM +0200, Michal Suchanek wrote:
>>
>> ntldr is a boot loader like any other and it needs its configuration
>> and support files to work. Without them it fails (not sure how) but
>> that is not unexpected.
>
> I tend to agree, but in this particular case, it's conceivable that  
> Microsoft
> didn't expect anyone to do this, and therefore neglected to provide  
> a graceful
> response.
>
> Could anyone test it and report how this behaves?
>

Hi i try to boot my vista with ntldr command directly and it works  
like a charm.

I have try to put the file on another partition and made:
root= partition where BCD are
ntldr /bootmgr

but without success. But i think it's because of a bad BCD file.

About the command, i think that it will be simpler for the user if we  
have only one command: chainloader (like in grub4dos) that will try to  
detect the type of the bootloader. This is only my personal opinion.

Yves Blusseau

Re: [PATCH] ntldr support

[Vladimir 'phcoder' Serbinenko]

> About the command, i think that it will be simpler for the user if we have
> only one command: chainloader (like in grub4dos) that will try to detect the
> type of the bootloader. This is only my personal opinion.
I don't agree with this. chainloader and ntldr don't share the same
syntax: chainloader expects a bootsector whereas ntldr expects an
ntldr ot bootmgr file. GRUB2 is done to break with bad design
decisions of GRUB1 one of them being "kernel" command. GRUB4DOS
follows GRUB1 on this subject.
Additionally people may want to embed chainloader or ntldr into
core.img to be able to have an emergency boot method. For this reason
I consider we should split functionality into 2 modules which requires
2 different commands
>
> Yves Blusseau
>
>
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git

Re: [PATCH] ntldr support

[Christian Franke]

Michal Suchanek wrote:
>
> I tried putting the ntldr binary on an empty fat32 disk and loading
> with the grub ntldr command.
>
> The system just reboots.
>
>   

This also happens when ntldr is loaded by the regular boot code in 
PBR+boot area.
It is apparently the normal(?-) behavior when ntdetect.com is missing.

-- 
Regards,
Christian Franke

Re: [PATCH] ntldr support

[Christian Franke]

Vladimir 'phcoder' Serbinenko wrote:
>> About the command, i think that it will be simpler for the user if we have
>> only one command: chainloader (like in grub4dos) that will try to detect the
>> type of the bootloader. This is only my personal opinion.
>>     
> I don't agree with this. chainloader and ntldr don't share the same
> syntax: chainloader expects a bootsector whereas ntldr expects an
> ntldr ot bootmgr file. GRUB2 is done to break with bad design
> decisions of GRUB1 one of them being "kernel" command. GRUB4DOS
> follows GRUB1 on this subject.
>   

I agree.

It probably would make sense that the 'ntldr' command does simple 
signature checks and fail on unknown files unless '--force' is specified.

-- 
Regards,
Christian Franke

Re: [PATCH] ntldr support

[Robert Millan]

On Sat, Aug 08, 2009 at 11:57:48PM +0200, Christian Franke wrote:
> Vladimir 'phcoder' Serbinenko wrote:
>>> About the command, i think that it will be simpler for the user if we have
>>> only one command: chainloader (like in grub4dos) that will try to detect the
>>> type of the bootloader. This is only my personal opinion.
>>>     
>> I don't agree with this. chainloader and ntldr don't share the same
>> syntax: chainloader expects a bootsector whereas ntldr expects an
>> ntldr ot bootmgr file. GRUB2 is done to break with bad design
>> decisions of GRUB1 one of them being "kernel" command. GRUB4DOS
>> follows GRUB1 on this subject.
>>   
>
> I agree.

Alright.  Let's make it a separate command.  I think it should still share
code with chainloader.c though (with some ifdefs).

I'll sort this out and make a newer patch when I find a free minute.

> It probably would make sense that the 'ntldr' command does simple  
> signature checks and fail on unknown files unless '--force' is specified.

You mean checking for the PE signature?  Yes, this would be nice too.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [PATCH] ntldr support

[Christian Franke]

Robert Millan wrote:
>> It probably would make sense that the 'ntldr' command does simple  
>> signature checks and fail on unknown files unless '--force' is specified.
>>     
>
> You mean checking for the PE signature?  Yes, this would be nice too.
>
>   

A check of the first byte (jmp, 0xe9) and some file size range check 
(e.g. 0x30000...0x40000) may be enough for a first ntldr command. May 
also work for bootmgr.exe.

EXE ("MZ") and PE headers appear at larger offsets:

ntldr from XP SP2: size 251184, EXE header at 0x4d30, PE at 0x4e00
ntldr from XP SP3: size 251712, EXE header at 0x4d40, PE at 0x4e10
bootmgr.exe from Vista: ???


grub4dos checks for ntldr as follows:
- file starts with 0xe9, 0x??, 0x01,
- first sector does not end with bootsector signature 0x55,0xaa,
- file size exceeds 0x30000.

-- 
Regards
Christian Franke

Re: [PATCH] ntldr support

[Vladimir 'phcoder' Serbinenko]

> grub4dos checks for ntldr as follows:
> - file starts with 0xe9, 0x??, 0x01,
> - first sector does not end with bootsector signature 0x55,0xaa,
> - file size exceeds 0x30000.
For me it sounds like a heuristic. I would prefer to trust user rather
than introducing heuristics to check file type.
>
> --
> Regards
> Christian Franke
>
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>



-- 
Regards
Vladimir 'phcoder' Serbinenko

Personal git repository: http://repo.or.cz/w/grub2/phcoder.git

Re: [PATCH] ntldr support

[Christian Franke]

Vladimir 'phcoder' Serbinenko wrote:
>> grub4dos checks for ntldr as follows:
>> - file starts with 0xe9, 0x??, 0x01,
>> - first sector does not end with bootsector signature 0x55,0xaa,
>> - file size exceeds 0x30000.
>>     
> For me it sounds like a heuristic. I would prefer to trust user rather
> than introducing heuristics to check file type.
>   

I agree that such a heuristic is not needed. Grub2 uses different 
commands for different loaders (which is good) and therefore there is no 
need to auto-detect the loader type.

But a simple health check (like in 'chainloader') would IMO make sense 
for any load command. For 'ntldr': check that the file size is 
reasonable and that code starts with a jmp instruction. Allow to 
override the check with '--force'.

-- 
Regards,
Christian Franke

Re: [PATCH] ntldr support

[Robert Millan]

On Mon, Oct 26, 2009 at 10:28:21AM +1100, Nando wrote:
> 
> I've just checked the 0.97 release and find the ntldr patch is not
> included. What version of grub2 was the original ntldr.diff patch
> against??

Unfortunately it missed the time for 1.97 release, but we expect to include
this with 1.98.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [PATCH] ntldr support

[Nando]

[-- Attachment #1: Type: text/plain, Size: 942 bytes --]

> Agree.

>
> A 'ntldr' or 'chainloader --ntldr' command is not mandatory. But it is
'nice to have' because it allows to boot even if the boot
> code (6 sectors) in the area > behind the PBR is not present for whatever
reason. See my previsions mail with the test case.
>

> --
> Regards,
> Christian Franke

I've just checked the 0.97 release and find the ntldr patch is not
included. What version of grub2 was the original ntldr.diff patch
against??

I would definitely would like to see this implemented. Plenty of ppl
out there with "bootmgr is missing" error messages requiring
Win7/Vista
bootup into recovery console to correct..This solution would rectify
that. Also means could bootup into XP or Win7/Vista with
separate entries in grub.conf, rather than navigating the Vista
bootloader to do it. Eg:

menuentry "XP on /dev/sda1" {		
root (hd0,1)
ntldr /ntldr }

menuentry "Win7 on /dev/sda2" {		
root (hd0,2)
ntldr /bootmgr }

Nando

[-- Attachment #2: Type: text/html, Size: 1277 bytes --]