Skip to contentSkip to conversation

memtest86+ fix

PATCH In review Robert Millan 3 participants 8 messages Jan 2, 2008 – Jan 5, 2008

lore
Share & tools
lore

Robert Millan

author
#0 lore raw

PATCH 1/1 memtest86+ fix

This fixes boot for memtest86+ (and actually, Linux zImages since they were all affected).

The problem was that the first code in grub_linux_boot_zimage copied the payload from 0x100000 to 0x10000. Since GRUB starts at 0x8200 and is typicaly more than 8 kiB in size, it'll most likely overwrite part of it.

A bit of reestructuring was necessary to allow grub_dl_unload_all() to happen unconditionally as first step, since the information on whether this is a "big linux" was not promptly available.

Comments?

Signature · 5 lines
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)

2008-01-02 Robert Millan <[email protected]>

* kern/i386/loader.S (grub_linux_big): New variable. (grub_linux_boot_zimage): Rename to ... (grub_linux_boot): ... this. (grub_linux_boot_bzimage): Merge with `grub_linux_boot_zimage'. (grub_linux_boot_zimage): Conditionalize zImage copy.

* include/grub/i386/loader.h (grub_linux_big): Add prototype. (grub_linux_boot_bzimage): Remove prototype. (grub_linux_boot_zimage): Rename to ... (grub_linux_boot): ... this.

* loader/i386/pc/linux.c (big_linux): Replace with `grub_linux_big'. (grub_linux_boot): Remove function.

include/grub/i386/loader.h +2 -2
diff -urp grub2/include/grub/i386/loader.h grub2.mt/include/grub/i386/loader.h
@@ -26,11 +26,11 @@
 extern grub_uint32_t EXPORT_VAR(grub_linux_prot_size);
 extern char *EXPORT_VAR(grub_linux_tmp_addr);
 extern char *EXPORT_VAR(grub_linux_real_addr);
+extern grub_int32_t EXPORT_VAR(grub_linux_big);
 extern grub_addr_t EXPORT_VAR(grub_os_area_addr);
 extern grub_size_t EXPORT_VAR(grub_os_area_size);
 
-void EXPORT_FUNC(grub_linux_boot_zimage) (void) __attribute__ ((noreturn));
-void EXPORT_FUNC(grub_linux_boot_bzimage) (void) __attribute__ ((noreturn));
+void EXPORT_FUNC(grub_linux_boot) (void) __attribute__ ((noreturn));
 
 /* The asm part of the multiboot loader.  */
 void EXPORT_FUNC(grub_multiboot_real_boot) (grub_addr_t entry,
kern/i386/loader.S +11 -4
diff -urp grub2/kern/i386/loader.S grub2.mt/kern/i386/loader.S
@@ -56,8 +56,17 @@ VARIABLE(grub_linux_tmp_addr)
 	.long	0
 VARIABLE(grub_linux_real_addr)
 	.long	0
+VARIABLE(grub_linux_big)
+	.long	0
 	
-FUNCTION(grub_linux_boot_zimage)
+FUNCTION(grub_linux_boot)
+	/* Must be done before zImage copy.  */
+	call	EXT_C(grub_dl_unload_all)
+
+	movl	EXT_C(grub_linux_big), %ebx
+	test	%ebx, %ebx
+	jne bzimage
+
 	/* copy the kernel */
 	movl	EXT_C(grub_linux_prot_size), %ecx
 	addl	$3, %ecx

@@ -68,9 +77,7 @@ FUNCTION(grub_linux_boot_zimage)
 	rep
 	movsl
 
-FUNCTION(grub_linux_boot_bzimage)
-	call	EXT_C(grub_dl_unload_all)
-	
+bzimage:
 	movl	EXT_C(grub_linux_real_addr), %ebx
 
 	/* copy the real mode code */
loader/i386/pc/linux.c +4 -17
diff -urp grub2/loader/i386/pc/linux.c grub2.mt/loader/i386/pc/linux.c
@@ -33,23 +33,10 @@
 
 static grub_dl_t my_mod;
 
-static int big_linux;
 static grub_size_t linux_mem_size;
 static int loaded;
 
 static grub_err_t
-grub_linux_boot (void)
-{
-  if (big_linux)
-    grub_linux_boot_bzimage ();
-  else
-    grub_linux_boot_zimage ();
-
-  /* Never reach here.  */
-  return GRUB_ERR_NONE;
-}
-
-static grub_err_t
 grub_linux_unload (void)
 {
   grub_dl_unref (my_mod);

@@ -106,14 +93,14 @@ grub_rescue_cmd_linux (int argc, char *a
       goto fail;
     }
 
-  big_linux = 0;
+  grub_linux_big = 0;
   setup_sects = lh.setup_sects;
   linux_mem_size = 0;
   
   if (lh.header == grub_cpu_to_le32 (GRUB_LINUX_MAGIC_SIGNATURE)
       && grub_le_to_cpu16 (lh.version) >= 0x0200)
     {
-      big_linux = (lh.loadflags & GRUB_LINUX_FLAG_BIG_KERNEL);
+      grub_linux_big = (lh.loadflags & GRUB_LINUX_FLAG_BIG_KERNEL);
       lh.type_of_loader = GRUB_LINUX_BOOT_LOADER_TYPE;
       
       /* Put the real mode part at as a high location as possible.  */

@@ -158,7 +145,7 @@ grub_rescue_cmd_linux (int argc, char *a
   
   grub_linux_tmp_addr = (char *) GRUB_LINUX_BZIMAGE_ADDR + prot_size;
 
-  if (! big_linux
+  if (! grub_linux_big
       && prot_size > (grub_size_t) (grub_linux_real_addr
 				    - (char *) GRUB_LINUX_ZIMAGE_ADDR))
     {

@@ -177,7 +164,7 @@ grub_rescue_cmd_linux (int argc, char *a
     }
 
   grub_printf ("   [Linux-%s, setup=0x%x, size=0x%x]\n",
-	       big_linux ? "bzImage" : "zImage", real_size, prot_size);
+	       grub_linux_big ? "bzImage" : "zImage", real_size, prot_size);
 
   for (i = 1; i < argc; i++)
     if (grub_memcmp (argv[i], "vga=", 4) == 0)
#1 lore raw
Quoted 7 lines · Robert Millan

This fixes boot for memtest86+ (and actually, Linux zImages since they were all affected).

The problem was that the first code in grub_linux_boot_zimage copied the payload from 0x100000 to 0x10000. Since GRUB starts at 0x8200 and is typicaly more than 8 kiB in size, it'll most likely overwrite part of it.

Quoted 5 lines

A bit of reestructuring was necessary to allow grub_dl_unload_all() to happen unconditionally as first step, since the information on whether this is a "big linux" was not promptly available.

Comments?

It is okay, except for "grub_linux_big"... I cannot figure out what it wants to say. As you want to make it global, please choose a more expressive name, such as "grub_big_linux_loaded".

Okuji

author
#2 lore raw
Quoted 12 lines · Yoshinori K. Okuji
Quoted 11 lines · Robert Millan

This fixes boot for memtest86+ (and actually, Linux zImages since they were all affected).

The problem was that the first code in grub_linux_boot_zimage copied the payload from 0x100000 to 0x10000. Since GRUB starts at 0x8200 and is typicaly more than 8 kiB in size, it'll most likely overwrite part of it.

A bit of reestructuring was necessary to allow grub_dl_unload_all() to happen unconditionally as first step, since the information on whether this is a "big linux" was not promptly available.

Quoted 5 lines
Quoted 1 line

Comments?

It is okay, except for "grub_linux_big"... I cannot figure out what it wants to say. As you want to make it global, please choose a more expressive name, such as "grub_big_linux_loaded".

How about "grub_linux_big_loaded", for consistency with the other vars in <machine/kernel.h> that also start with grub_linux_ ?

Signature · 5 lines
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)
#3 lore raw
Quoted 14 lines · Robert Millan
Quoted 13 lines · Yoshinori K. Okuji
Quoted 12 lines · Robert Millan

This fixes boot for memtest86+ (and actually, Linux zImages since they were all affected).

The problem was that the first code in grub_linux_boot_zimage copied the payload from 0x100000 to 0x10000. Since GRUB starts at 0x8200 and is typicaly more than 8 kiB in size, it'll most likely overwrite part of it.

A bit of reestructuring was necessary to allow grub_dl_unload_all() to happen unconditionally as first step, since the information on whether this is a "big linux" was not promptly available.

Quoted 8 lines
Quoted 5 lines
Quoted 1 line

Comments?

It is okay, except for "grub_linux_big"... I cannot figure out what it wants to say. As you want to make it global, please choose a more expressive name, such as "grub_big_linux_loaded".

How about "grub_linux_big_loaded", for consistency with the other vars in <machine/kernel.h> that also start with grub_linux_ ?

"big_loaded" sounds horrible to me. How about grub_linux_big_image_loaded?

Okuji

#4 lore raw
Quoted 1 line · Yoshinori K. Okuji

"big_loaded" sounds horrible to me. How about grub_linux_big_image_loaded?

"big image" is normally abbreviated as bzimage, as opposed to zimage, so I would suggest something of this kind:

grub_is_bzimage
grub_linux_is_bzimage
grub_bzimage_loaded
grub_linux_bzimage_loaded

Signature · 2 lines
Regards,
Pavel Roskin
author
#5 lore raw
Quoted 11 lines · Pavel Roskin
Quoted 1 line · Yoshinori K. Okuji

"big_loaded" sounds horrible to me. How about grub_linux_big_image_loaded?

"big image" is normally abbreviated as bzimage, as opposed to zimage, so I would suggest something of this kind:

grub_is_bzimage
grub_linux_is_bzimage
grub_bzimage_loaded
grub_linux_bzimage_loaded

Any name that matches "grub_linux_.*" is fine with me. Can we just agree on something ? There are people waiting to use memtest86+ ! :-)

Signature · 5 lines
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)
#6 lore raw
Quoted 8 lines · Robert Millan
Quoted 7 lines · Pavel Roskin
Quoted 2 lines · Yoshinori K. Okuji

"big_loaded" sounds horrible to me. How about
grub_linux_big_image_loaded?

"big image" is normally abbreviated as bzimage, as opposed to zimage, so I would suggest something of this kind:

Quoted 7 lines
Quoted 4 lines

grub_is_bzimage
grub_linux_is_bzimage
grub_bzimage_loaded
grub_linux_bzimage_loaded

Any name that matches "grub_linux_.*" is fine with me. Can we just agree on something ? There are people waiting to use memtest86+ ! :-)

grub_linux_is_bzimage sounds good to me.

Okuji

author
#7 lore raw
Quoted 14 lines · Yoshinori K. Okuji
Quoted 13 lines · Robert Millan
Quoted 11 lines · Pavel Roskin
Quoted 2 lines · Yoshinori K. Okuji

"big_loaded" sounds horrible to me. How about
grub_linux_big_image_loaded?

"big image" is normally abbreviated as bzimage, as opposed to zimage, so I would suggest something of this kind:

grub_is_bzimage
grub_linux_is_bzimage
grub_bzimage_loaded
grub_linux_bzimage_loaded

Quoted 4 lines
Quoted 2 lines

Any name that matches "grub_linux_.*" is fine with me. Can we just agree on something ? There are people waiting to use memtest86+ ! :-)

grub_linux_is_bzimage sounds good to me.

Ok, committed.

Signature · 5 lines
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)