[PATCH] efivars: check enabled {2.6.6 doesn't boot with 4k stacks}

[PATCH] efivars: check enabled {2.6.6 doesn't boot with 4k stacks}

[Randy.Dunlap]

| Perhaps I missed some readme, but just in case. One of my computer, P4HT[*]
| dies during booting. There is no log because nothing is still mounted, 
| but I took a photograph of the stack dump in the screen.
| 
| http://mnm.uib.es/~gallir/tmp/hang1s.jpg

Yes, easy to reproduce...

| The .config file:
| http://mnm.uib.es/~gallir/tmp/2.6.6-bad-config.txt
| 
| I unchecked the 4K stack option and it's working nice.


drivers/firmware/efivars.c::efivars_init & efivars_exit
need to check for efi_enabled before doing anything.

or you could disable CONFIG_EFI_VARS, but the kernel shouldn't
crash like that.


// linux-266
// efivars_init and efivars_exit need to check efi_enabled
// instead of assuming that the system is using EFI;

diffstat:=
 drivers/firmware/efivars.c |    6 ++++++
 1 files changed, 6 insertions(+)


diff -Naurp ./drivers/firmware/efivars.c~efi_check_enabled ./drivers/firmware/efivars.c
--- ./drivers/firmware/efivars.c~efi_check_enabled	2004-05-09 19:33:13.000000000 -0700
+++ ./drivers/firmware/efivars.c	2004-05-10 20:45:55.000000000 -0700
@@ -664,6 +664,9 @@ efivars_init(void)
 	unsigned long variable_name_size = 1024;
 	int i, rc = 0, error = 0;
 
+	if (!efi_enabled)
+		return 0;
+
 	printk(KERN_INFO "EFI Variables Facility v%s\n", EFIVARS_VERSION);
 
 	/*
@@ -733,6 +736,9 @@ efivars_exit(void)
 {
 	struct list_head *pos, *n;
 
+	if (!efi_enabled)
+		return;
+
 	list_for_each_safe(pos, n, &efivar_list)
 		efivar_unregister(get_efivar_entry(pos));
 




--
http://www.madrone.org/quotes/2003.0226.akpm.txt

Re: [PATCH] efivars: check enabled {2.6.6 doesn't boot with 4k stacks}

[Matt Domsch]

On Mon, May 10, 2004 at 09:02:43PM -0700, Randy.Dunlap wrote:
> // linux-266
> // efivars_init and efivars_exit need to check efi_enabled
> // instead of assuming that the system is using EFI;

Good catch.  I missed the creation of that in the x86 EFI merge.

> +++ ./drivers/firmware/efivars.c	2004-05-10 20:45:55.000000000 -0700
> @@ -664,6 +664,9 @@ efivars_init(void)
>  	unsigned long variable_name_size = 1024;
>  	int i, rc = 0, error = 0;
>  
> +	if (!efi_enabled)
> +		return 0;

I would think this would be return -ENODEV; instead, yes?

Thanks,
Matt

-- 
Matt Domsch
Sr. Software Engineer, Lead Engineer
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

Re: [PATCH](2) efivars: check enabled {2.6.6 doesn't boot with 4k stacks}

[Randy.Dunlap]

On Tue, 11 May 2004 09:38:33 -0500 Matt Domsch wrote:

| On Mon, May 10, 2004 at 09:02:43PM -0700, Randy.Dunlap wrote:
| > // linux-266
| > // efivars_init and efivars_exit need to check efi_enabled
| > // instead of assuming that the system is using EFI;
| 
| Good catch.  I missed the creation of that in the x86 EFI merge.
| 
| > +++ ./drivers/firmware/efivars.c	2004-05-10 20:45:55.000000000 -0700
| > @@ -664,6 +664,9 @@ efivars_init(void)
| >  	unsigned long variable_name_size = 1024;
| >  	int i, rc = 0, error = 0;
| >  
| > +	if (!efi_enabled)
| > +		return 0;
| 
| I would think this would be return -ENODEV; instead, yes?

um, OK.  I suppose that is a good idea... to prevent the module
from remaining loaded.

Here's an updated diff.

--
~Randy



// linux-266
// efivars_init and _exit need to check for efi_enabled (or else crash);

diffstat:=
 drivers/firmware/efivars.c |    6 ++++++
 1 files changed, 6 insertions(+)


diff -Naurp ./drivers/firmware/efivars.c~efivars_check_enabled ./drivers/firmware/efivars.c
--- ./drivers/firmware/efivars.c~efivars_check_enabled	2004-05-09 19:33:13.000000000 -0700
+++ ./drivers/firmware/efivars.c	2004-05-11 08:43:07.000000000 -0700
@@ -664,6 +664,9 @@ efivars_init(void)
 	unsigned long variable_name_size = 1024;
 	int i, rc = 0, error = 0;
 
+	if (!efi_enabled)
+		return -ENODEV;
+
 	printk(KERN_INFO "EFI Variables Facility v%s\n", EFIVARS_VERSION);
 
 	/*
@@ -733,6 +736,9 @@ efivars_exit(void)
 {
 	struct list_head *pos, *n;
 
+	if (!efi_enabled)
+		return;
+
 	list_for_each_safe(pos, n, &efivar_list)
 		efivar_unregister(get_efivar_entry(pos));