[PATCH 1/2] Reduce the special variables area in kern/i386/pc/startup.S

[PATCH 1/2] Reduce the special variables area in kern/i386/pc/startup.S

[Pavel Roskin]

ChangeLog:

	* include/grub/i386/pc/kernel.h (grub_boot_drive): Change type
	to grub_uint8_t.
	(grub_root_drive): Likewise.
	* kern/i386/pc/startup.S (grub_boot_drive): Change size to byte,
	remove alignment.
	(grub_root_drive): Change size to byte.
	(grub_start_addr): Remove.
	(grub_end_addr): Likewise.
	(grub_apm_bios_info): Likewise.

This patch will require trivial adjustments to the patches changing
END_SYMBOL.  We don't need END_SYMBOL for grub_end_addr.
---
 include/grub/i386/pc/kernel.h |    7 ++-----
 kern/i386/pc/startup.S        |   22 ++--------------------
 2 files changed, 4 insertions(+), 25 deletions(-)

diff --git a/include/grub/i386/pc/kernel.h b/include/grub/i386/pc/kernel.h
index b6650bc..5acc883 100644
--- a/include/grub/i386/pc/kernel.h
+++ b/include/grub/i386/pc/kernel.h
@@ -69,13 +69,10 @@ extern grub_int32_t grub_install_bsd_part;
 extern char grub_prefix[];
 
 /* The boot BIOS drive number.  */
-extern grub_int32_t EXPORT_VAR(grub_boot_drive);
+extern grub_uint8_t EXPORT_VAR(grub_boot_drive);
 
 /* The root BIOS drive number.  */
-extern grub_int32_t grub_root_drive;
-
-/* The end address of the kernel.  */
-extern grub_addr_t grub_end_addr;
+extern grub_uint8_t grub_root_drive;
 
 #endif /* ! ASM_FILE */
 
diff --git a/kern/i386/pc/startup.S b/kern/i386/pc/startup.S
index 8e8b661..fc83c4c 100644
--- a/kern/i386/pc/startup.S
+++ b/kern/i386/pc/startup.S
@@ -273,30 +273,12 @@ codestart:
  *  This is the area for all of the special variables.
  */
 
-	.p2align	2	/* force 4-byte alignment */
-
 VARIABLE(grub_boot_drive)
-	.long	0
+	.byte	0
 
 VARIABLE(grub_root_drive)
-	.long	0
+	.byte	0
 
-VARIABLE(grub_start_addr)
-	.long	_start
-
-VARIABLE(grub_end_addr)
-	.long	END_SYMBOL
-	
-VARIABLE(grub_apm_bios_info)
-	.word	0	/* version */
-	.word	0	/* cseg */
-	.long	0	/* offset */
-	.word	0	/* cseg_16 */
-	.word	0	/* dseg_16 */
-	.word	0	/* cseg_len */
-	.word	0	/* cseg_16_len */
-	.word	0	/* dseg_16_len */
-	
 	.p2align	2	/* force 4-byte alignment */
 	
 /*

[PATCH 2/2] Eliminate incorrect check for grub_I386_CHECK_REGPARM_BUG

[Pavel Roskin]

NESTED_FUNC_ATTR is defined to "__attribute__ ((__regparm__ (1))" for
i386 targets regardless of the outcome of the test.  Since
NESTED_FUNC_ATTR is on the way out, a better test won't be written.

To be safe, define NESTED_FUNC_ATTR whenever we are compiling the
specific file for i386 architecture, even the target platform is not
i386 (i.e. compiling utilities when the host is i386 and the target is
not).

ChangeLog:

	* aclocal.m4 (grub_I386_CHECK_REGPARM_BUG): Remove.
	* configure.ac: Don't call grub_I386_CHECK_REGPARM_BUG.  Define
	NESTED_FUNC_ATTR by AH_BOTTOM.  Base NESTED_FUNC_ATTR value
	solely on the compile time architecture.
---
 aclocal.m4   |   54 ------------------------------------------------------
 configure.ac |    7 ++++++-
 2 files changed, 6 insertions(+), 55 deletions(-)

diff --git a/aclocal.m4 b/aclocal.m4
index 779df3d..38a9a4a 100644
--- a/aclocal.m4
+++ b/aclocal.m4
@@ -301,60 +301,6 @@ else
 fi
 ])
 
-dnl Check if the C compiler has a bug while using nested functions when
-dnl mregparm is used on the i386.  Some gcc versions do not pass the third
-dnl parameter correctly to the nested function.
-dnl Written by Marco Gerards.
-AC_DEFUN(grub_I386_CHECK_REGPARM_BUG,
-[AC_REQUIRE([AC_PROG_CC])
-AC_MSG_CHECKING([if GCC has the regparm=3 bug])
-AC_CACHE_VAL(grub_cv_i386_check_nested_functions,
-[AC_RUN_IFELSE([AC_LANG_SOURCE(
-[[
-static int
-test (int *n)
-{
-  return *n == -1;
-}
-
-static int
-testfunc (int __attribute__ ((__regparm__ (3))) (*hook) (int a, int b, int *c))
-{
-  int a = 0;
-  int b = 0;
-  int c = -1;
-  return hook (a, b, &c);
-}
-
-int
-main (void)
-{
-  int __attribute__ ((__regparm__ (3))) nestedfunc (int a, int b, int *c)
-    {
-      return a == b && test (c);
-    }
-  return testfunc (nestedfunc) ? 0 : 1;
-}
-]])],
-	[grub_cv_i386_check_nested_functions=no],
-	[grub_cv_i386_check_nested_functions=yes],
-	[grub_cv_i386_check_nested_functions=yes])])
-
-AC_MSG_RESULT([$grub_cv_i386_check_nested_functions])
-
-if test "x$grub_cv_i386_check_nested_functions" = xyes; then
-  AC_DEFINE([NESTED_FUNC_ATTR], 
-	[__attribute__ ((__regparm__ (1)))],
-	[Catch gcc bug])
-else
-dnl Unfortunately, the above test does not detect a bug in gcc-4.0.
-dnl So use regparm 2 until a better test is found.
-  AC_DEFINE([NESTED_FUNC_ATTR], 
-	[__attribute__ ((__regparm__ (1)))],
-	[Catch gcc bug])
-fi
-])
-
 dnl Check if the C compiler generates calls to `__enable_execute_stack()'.
 AC_DEFUN(grub_CHECK_ENABLE_EXECUTE_STACK,[
 AC_MSG_CHECKING([whether `$CC' generates calls to `__enable_execute_stack()'])
diff --git a/configure.ac b/configure.ac
index d84e2b6..0ba197e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -391,11 +391,16 @@ if test "x$target_cpu" = xi386; then
   grub_I386_ASM_PREFIX_REQUIREMENT
   grub_I386_ASM_ADDR32
   grub_I386_ASM_ABSOLUTE_WITHOUT_ASTERISK
-  grub_I386_CHECK_REGPARM_BUG
 else
   AC_DEFINE([NESTED_FUNC_ATTR], [], [Catch gcc bug])
 fi
 
+AH_BOTTOM([#ifdef __i386__
+#define NESTED_FUNC_ATTR __attribute__ ((__regparm__ (1)))
+#else
+#define NESTED_FUNC_ATTR
+#endif])
+
 # Restore the flags.
 CC="$tmp_CC"
 CFLAGS="$tmp_CFLAGS"

Re: [PATCH 2/2] Eliminate incorrect check for grub_I386_CHECK_REGPARM_BUG

[Pavel Roskin]

On Fri, 2009-05-22 at 00:36 -0400, Pavel Roskin wrote:
> NESTED_FUNC_ATTR is defined to "__attribute__ ((__regparm__ (1))" for
> i386 targets regardless of the outcome of the test.  Since
> NESTED_FUNC_ATTR is on the way out, a better test won't be written.
> 
> To be safe, define NESTED_FUNC_ATTR whenever we are compiling the
> specific file for i386 architecture, even the target platform is not
> i386 (i.e. compiling utilities when the host is i386 and the target is
> not).
> 
> ChangeLog:
> 
> 	* aclocal.m4 (grub_I386_CHECK_REGPARM_BUG): Remove.
> 	* configure.ac: Don't call grub_I386_CHECK_REGPARM_BUG.  Define
> 	NESTED_FUNC_ATTR by AH_BOTTOM.  Base NESTED_FUNC_ATTR value
> 	solely on the compile time architecture.

I've committed this with a minor change.  regparm(1) is not used for the
utilities, as they are compiled without -mregparm(3).  This fixes
grub-setup on i386.

We still need to get rid of NESTED_FUNC_ATTR eventually, but the
immediate user-visible issue is resolved now.

-- 
Regards,
Pavel Roskin

Re: [PATCH 1/2] Reduce the special variables area in kern/i386/pc/startup.S

[Pavel Roskin]

On Fri, 2009-05-22 at 00:36 -0400, Pavel Roskin wrote:
> ChangeLog:
> 
> 	* include/grub/i386/pc/kernel.h (grub_boot_drive): Change type
> 	to grub_uint8_t.
> 	(grub_root_drive): Likewise.
> 	* kern/i386/pc/startup.S (grub_boot_drive): Change size to byte,
> 	remove alignment.
> 	(grub_root_drive): Change size to byte.
> 	(grub_start_addr): Remove.
> 	(grub_end_addr): Likewise.
> 	(grub_apm_bios_info): Likewise.

Committed.

-- 
Regards,
Pavel Roskin