Skip to contentSkip to conversation

kdump: pass acpi_rsdp= to 2nd kernel for efi booting

PATCH In review Dave Young 5 participants 8 messages Oct 18, 2012 – Oct 23, 2012

Subsystems (2)
lore
Share & tools
lore

Dave Young

author
#0 lore raw

PATCH v2 1/1 kdump: pass acpi_rsdp= to 2nd kernel for efi booting

In case efi booting, kdump need kernel parameter acpi_rsdp= to retrieve the acpi root table physical address.

Add a function cmdline_add_efi to get the address from /sys/firmware/efi/systab If there's no such file or read fail the function will just do nothing.

Tested efi boot Fedora 17 on thinkpad T420.

Some background info for this issue:
http://lists.infradead.org/pipermail/kexec/2010-March/003889.html

[v1 -> v2]:
Address comments from Khalid and Simon
use fgets instead of read(2) to iterate the file
do not add 'noefi' because kexec does not construct EFI signature in bootloader signature in boot_params, so kexec'd kernel will disable EFI automatically even without noefi.

---
 kexec/arch/i386/crashdump-x86.c |   35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)
kexec/arch/i386/crashdump-x86.c +35 -0
@@ -835,6 +835,40 @@ static int cmdline_add_memmap_acpi(char 
 	return 0;
 }
 
+/* Appends 'acpi_rsdp=' commandline for efi boot crash dump */
+static void cmdline_add_efi(char *cmdline)
+{
+	FILE *fp;
+	int cmdlen, len;
+	char line[MAX_LINE], *s;
+	const char *acpis = " acpi_rsdp=";
+
+	fp = fopen("/sys/firmware/efi/systab", "r");
+	if (!fp)
+		return;
+
+	while(fgets(line, sizeof(line), fp) != 0) {
+		/* ACPI20= always goes before ACPI= */
+		if ((strstr(line, "ACPI20=")) || (strstr(line, "ACPI="))) {
+		        line[strlen(line) - 1] = '\0';
+			s = strchr(line, '=');
+			s += 1;
+			len = strlen(s) + strlen(acpis);
+			cmdlen = strlen(cmdline) + len;
+			if (cmdlen > (COMMAND_LINE_SIZE - 1))
+				die("Command line overflow\n");
+			strcat(cmdline, acpis);
+			strcat(cmdline, s);
+			dbgprintf("Command line after adding efi\n");
+			dbgprintf("%s\n", cmdline);
+
+			break;
+		}
+	}
+
+	fclose(fp);
+}
+
 static void get_backup_area(struct kexec_info *info,
 				struct memory_range *range, int ranges)
 {
@@ -998,6 +1032,7 @@ int load_crashdump_segments(struct kexec
 	if (delete_memmap(memmap_p, elfcorehdr, memsz) < 0)
 		return -1;
 	cmdline_add_memmap(mod_cmdline, memmap_p);
+	cmdline_add_efi(mod_cmdline);
 	cmdline_add_elfcorehdr(mod_cmdline, elfcorehdr);
 
 	/* Inform second kernel about the presence of ACPI tables. */

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

Acked-by on v2 1/1
#1 lore raw
Quoted 11 lines · Dave Young

In case efi booting, kdump need kernel parameter acpi_rsdp= to retrieve the acpi root table physical address.

Add a function cmdline_add_efi to get the address from /sys/firmware/efi/systab If there's no such file or read fail the function will just do nothing.

Tested efi boot Fedora 17 on thinkpad T420.

Some background info for this issue:
http://lists.infradead.org/pipermail/kexec/2010-March/003889.html

Quoted 6 lines

[v1 -> v2]:
Address comments from Khalid and Simon
use fgets instead of read(2) to iterate the file
do not add 'noefi' because kexec does not construct EFI signature in bootloader signature in boot_params, so kexec'd kernel will disable EFI automatically even without noefi.

I don't have any problems with this patch.

I have a question. In the case where this fails are we successfully passing the ACPI sections in the e820 map?

If we are passing the acpi sections is that not enough for the kernel to find the rdsp area? I'm just a bit surprised we need this patch is all.

Somehow it seems a bit ugly to pass information that could be conveyed in the memory map on the command line.

But I am all in favor of doing what works.

Quoted 57 lines
---
 kexec/arch/i386/crashdump-x86.c |   35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)
kexec/arch/i386/crashdump-x86.c +35 -0
@@ -835,6 +835,40 @@ static int cmdline_add_memmap_acpi(char 
 	return 0;
 }
 
+/* Appends 'acpi_rsdp=' commandline for efi boot crash dump */
+static void cmdline_add_efi(char *cmdline)
+{
+	FILE *fp;
+	int cmdlen, len;
+	char line[MAX_LINE], *s;
+	const char *acpis = " acpi_rsdp=";
+
+	fp = fopen("/sys/firmware/efi/systab", "r");
+	if (!fp)
+		return;
+
+	while(fgets(line, sizeof(line), fp) != 0) {
+		/* ACPI20= always goes before ACPI= */
+		if ((strstr(line, "ACPI20=")) || (strstr(line, "ACPI="))) {
+		        line[strlen(line) - 1] = '\0';
+			s = strchr(line, '=');
+			s += 1;
+			len = strlen(s) + strlen(acpis);
+			cmdlen = strlen(cmdline) + len;
+			if (cmdlen > (COMMAND_LINE_SIZE - 1))
+				die("Command line overflow\n");
+			strcat(cmdline, acpis);
+			strcat(cmdline, s);
+			dbgprintf("Command line after adding efi\n");
+			dbgprintf("%s\n", cmdline);
+
+			break;
+		}
+	}
+
+	fclose(fp);
+}
+
 static void get_backup_area(struct kexec_info *info,
 				struct memory_range *range, int ranges)
 {
@@ -998,6 +1032,7 @@ int load_crashdump_segments(struct kexec
 	if (delete_memmap(memmap_p, elfcorehdr, memsz) < 0)
 		return -1;
 	cmdline_add_memmap(mod_cmdline, memmap_p);
+	cmdline_add_efi(mod_cmdline);
 	cmdline_add_elfcorehdr(mod_cmdline, elfcorehdr);
 
 	/* Inform second kernel about the presence of ACPI tables. */
Quoted 4 lines

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#2 lore raw
Quoted 29 lines · Eric W. Biederman
Quoted 11 lines · Dave Young

In case efi booting, kdump need kernel parameter acpi_rsdp= to retrieve the acpi root table physical address.

Add a function cmdline_add_efi to get the address from /sys/firmware/efi/systab If there's no such file or read fail the function will just do nothing.

Tested efi boot Fedora 17 on thinkpad T420.

Some background info for this issue:
http://lists.infradead.org/pipermail/kexec/2010-March/003889.html

Quoted 6 lines

[v1 -> v2]:
Address comments from Khalid and Simon
use fgets instead of read(2) to iterate the file
do not add 'noefi' because kexec does not construct EFI signature in bootloader signature in boot_params, so kexec'd kernel will disable EFI automatically even without noefi.

I don't have any problems with this patch.

I have a question. In the case where this fails are we successfully passing the ACPI sections in the e820 map?

If we are passing the acpi sections is that not enough for the kernel to find the rdsp area? I'm just a bit surprised we need this patch is all.

Quoted 6 lines

Somehow it seems a bit ugly to pass information that could be conveyed in the memory map on the command line.

But I am all in favor of doing what works.

Thanks, I have applied this patch.

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#3 lore raw
In reply to Eric W. Biederman
Quoted 20 lines · Eric W. Biederman
Quoted 11 lines · Dave Young

In case efi booting, kdump need kernel parameter acpi_rsdp= to retrieve the acpi root table physical address.

Add a function cmdline_add_efi to get the address from /sys/firmware/efi/systab If there's no such file or read fail the function will just do nothing.

Tested efi boot Fedora 17 on thinkpad T420.

Some background info for this issue:
http://lists.infradead.org/pipermail/kexec/2010-March/003889.html

Quoted 6 lines

[v1 -> v2]:
Address comments from Khalid and Simon
use fgets instead of read(2) to iterate the file
do not add 'noefi' because kexec does not construct EFI signature in bootloader signature in boot_params, so kexec'd kernel will disable EFI automatically even without noefi.

Quoted 4 lines

I don't have any problems with this patch.

I have a question. In the case where this fails are we successfully passing the ACPI sections in the e820 map?

For kdump we pass everything on commandline using memmap=. This also includes passing ACPI regions. I don't think we exclude those memmap= options for EFI.

Also I am wondering that for kdump case why are we passing new memory map using command line which ultimately overides the e820 map we put into bootparams.

It should make sense to just put modified map in e820 map in bootparam and not append all these options on command line. Also simplifiying the command line.

This is orthogonal to question you asked but I think this is one improvement we should do for kexec on panic code.

Quoted 7 lines

If we are passing the acpi sections is that not enough for the kernel to find the rdsp area? I'm just a bit surprised we need this patch is all.

Somehow it seems a bit ugly to pass information that could be conveyed in the memory map on the command line.

I have no idea how UEFI case is working. Dave/Khalid might have more details on this.

Thanks
Vivek

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#4 lore raw
Quoted 11 lines · Vivek Goyal
Quoted 7 lines · Eric W. Biederman

If we are passing the acpi sections is that not enough for the kernel to find the rdsp area? I'm just a bit surprised we need this patch is all.

Somehow it seems a bit ugly to pass information that could be conveyed in the memory map on the command line.

I have no idea how UEFI case is working. Dave/Khalid might have more details on this.

On a UEFI machine with CSM, kernel finds RSDP in EBDA. On a UEFI machine with no CSM, kernel fails to find RSDP and here is the message I see:

ACPI BIOS Bug: Error: A valid RSDP was not found (20120913/tbxfroot-219)

--
Khalid

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#5 lore raw
In reply to Vivek Goyal
Quoted 9 lines · Vivek Goyal
Quoted 3 lines · Eric W. Biederman

I have a question. In the case where this fails are we successfully passing the ACPI sections in the e820 map?

For kdump we pass everything on commandline using memmap=. This also includes passing ACPI regions. I don't think we exclude those memmap= options for EFI.

Quoted 7 lines

Also I am wondering that for kdump case why are we passing new memory map using command line which ultimately overides the e820 map we put into bootparams.

It should make sense to just put modified map in e820 map in bootparam and not append all these options on command line. Also simplifiying the command line.

As I recall we pass in the real memory map in e820 which allows for /proc/oldmem and friends to work, and use the memory map to pass in which information we really want to use.

Given that what we use for the old memory map is the ELF headers passed from the crashed kernel there does seem to be room for cleanup in that area.

Quoted 2 lines

This is orthogonal to question you asked but I think this is one improvement we should do for kexec on panic code.

Certainly it is worth working on.

Quoted 9 lines
Quoted 6 lines

If we are passing the acpi sections is that not enough for the kernel to find the rdsp area? I'm just a bit surprised we need this patch is all.

Somehow it seems a bit ugly to pass information that could be conveyed in the memory map on the command line.

I have no idea how UEFI case is working. Dave/Khalid might have more details on this.

It is the non-pure UEFI case where non-UEFI table scans work.

Of course it puzzles me why we can't find the table via scanning memory when running in a pure UEFI environment. Ah well that is a problem for another day.

Eric

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#6 lore raw

On Sat, Oct 20, 2012 at 08:06:23PM -0700, Eric W. Biederman wrote:

[..]

Quoted 5 lines

It is the non-pure UEFI case where non-UEFI table scans work.

Of course it puzzles me why we can't find the table via scanning memory when running in a pure UEFI environment. Ah well that is a problem for another day.

I have the same question. Why can we find acpi tables by scaning in non-uefi case and why same is not possible in case of UEFI.

Thanks
Vivek

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec

#7 lore raw
Quoted 11 lines · Vivek Goyal

On Sat, Oct 20, 2012 at 08:06:23PM -0700, Eric W. Biederman wrote:

[..]

Quoted 5 lines

It is the non-pure UEFI case where non-UEFI table scans work.

Of course it puzzles me why we can't find the table via scanning memory when running in a pure UEFI environment. Ah well that is a problem for another day.

I have the same question. Why can we find acpi tables by scaning in non-uefi case and why same is not possible in case of UEFI.

I had sent an explanation out on Friday but looks like my email got stuck somewhere, and I don't see it on kexec list archive.

On a UEFI machine with CSM, kernel finds RSDP by searching through EBDA. On a UEFI machine with no CSM, there is no EBDA I believe and kernel fails to find RSDP. Here is the message I see on a UEFI machine with no CSM:

ACPI BIOS Bug: Error: A valid RSDP was not found (20120913/tbxfroot-219)

--
Khalid

_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec