This patchset is for adding efi runtime support on kexec kernel kernel patches see below thread: http://thread.gmane.org/gmane.linux.kernel.efi/2491
in kexec-tools, this patchset will do below: 1. retrieve efi_info from debugfs boot_params, and fill the x86 setup header
2. collect data efi runtime needed: /sys/firmware/efi/systab: fw_vendor, runtime, config_tables and smbios /sys/firmware/efi/efi-runtime-map/*, the phys-virt mappings in 1st kernel
3. assemble setup_data based on data get in 2) then pass it to 2nd kernel
Tested on OVMF, dell laptop, lenovo laptop and HP workstation
Please help to review -- Thanks Dave
Not only setup_subarch will get data from debugfs file boot_params/data, later code for adding efi_info will also need do same thing. Thus add a common function here for later use.
Signed-off-by: Dave Young dyoung@redhat.com --- kexec/arch/i386/x86-linux-setup.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
--- kexec-tools.orig/kexec/arch/i386/x86-linux-setup.c +++ kexec-tools/kexec/arch/i386/x86-linux-setup.c @@ -436,10 +436,9 @@ char *find_mnt_by_fsname(char *fsname) return mntdir; }
-void setup_subarch(struct x86_linux_param_header *real_mode) +void get_bootparam(void *buf, off_t offset, size_t size) { int data_file; - const off_t offset = offsetof(typeof(*real_mode), hardware_subarch); char *debugfs_mnt; char filename[PATH_MAX];
@@ -447,7 +446,7 @@ void setup_subarch(struct x86_linux_para if (!debugfs_mnt) return; snprintf(filename, PATH_MAX, "%s/%s", debugfs_mnt, "boot_params/data"); - filename[PATH_MAX-1] = 0; + filename[PATH_MAX - 1] = 0; free(debugfs_mnt);
data_file = open(filename, O_RDONLY); @@ -455,11 +454,18 @@ void setup_subarch(struct x86_linux_para return; if (lseek(data_file, offset, SEEK_SET) < 0) goto close; - read(data_file, &real_mode->hardware_subarch, sizeof(uint32_t)); + read(data_file, buf, size); close: close(data_file); }
+void setup_subarch(struct x86_linux_param_header *real_mode) +{ + off_t offset = offsetof(typeof(*real_mode), hardware_subarch); + + get_bootparam(&real_mode->hardware_subarch, offset, sizeof(uint32_t)); +} + void setup_linux_system_parameters(struct kexec_info *info, struct x86_linux_param_header *real_mode) {
On Sun, Oct 27, 2013 at 12:04:38PM +0800, dyoung@redhat.com wrote:
Not only setup_subarch will get data from debugfs file boot_params/data, later code for adding efi_info will also need do same thing. Thus add a common function here for later use.
Signed-off-by: Dave Young dyoung@redhat.com
kexec/arch/i386/x86-linux-setup.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)
--- kexec-tools.orig/kexec/arch/i386/x86-linux-setup.c +++ kexec-tools/kexec/arch/i386/x86-linux-setup.c @@ -436,10 +436,9 @@ char *find_mnt_by_fsname(char *fsname) return mntdir; }
-void setup_subarch(struct x86_linux_param_header *real_mode) +void get_bootparam(void *buf, off_t offset, size_t size) { int data_file;
- const off_t offset = offsetof(typeof(*real_mode), hardware_subarch); char *debugfs_mnt; char filename[PATH_MAX];
@@ -447,7 +446,7 @@ void setup_subarch(struct x86_linux_para if (!debugfs_mnt) return; snprintf(filename, PATH_MAX, "%s/%s", debugfs_mnt, "boot_params/data");
- filename[PATH_MAX-1] = 0;
- filename[PATH_MAX - 1] = 0; free(debugfs_mnt);
This change appears to be unrelated to the rest of the patch.
data_file = open(filename, O_RDONLY); @@ -455,11 +454,18 @@ void setup_subarch(struct x86_linux_para return; if (lseek(data_file, offset, SEEK_SET) < 0) goto close;
- read(data_file, &real_mode->hardware_subarch, sizeof(uint32_t));
- read(data_file, buf, size);
close: close(data_file); }
+void setup_subarch(struct x86_linux_param_header *real_mode) +{
- off_t offset = offsetof(typeof(*real_mode), hardware_subarch);
- get_bootparam(&real_mode->hardware_subarch, offset, sizeof(uint32_t));
+}
void setup_linux_system_parameters(struct kexec_info *info, struct x86_linux_param_header *real_mode) {
kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
@@ -447,7 +446,7 @@ void setup_subarch(struct x86_linux_para if (!debugfs_mnt) return; snprintf(filename, PATH_MAX, "%s/%s", debugfs_mnt, "boot_params/data");
- filename[PATH_MAX-1] = 0;
- filename[PATH_MAX - 1] = 0; free(debugfs_mnt);
This change appears to be unrelated to the rest of the patch.
Will remove the change from the patch.
On 10/28/13 at 09:13am, Dave Young wrote:
@@ -447,7 +446,7 @@ void setup_subarch(struct x86_linux_para if (!debugfs_mnt) return; snprintf(filename, PATH_MAX, "%s/%s", debugfs_mnt, "boot_params/data");
- filename[PATH_MAX-1] = 0;
- filename[PATH_MAX - 1] = 0; free(debugfs_mnt);
This change appears to be unrelated to the rest of the patch.
Will remove the change from the patch.
Relooking it, with this patch the line "filename[PATH_MAX - 1] = 0;" is in the share function get_bootparam, it's not in setup_subarch any more so I think it's ok, what do you think?
-- Thanks Dave
On Mon, Oct 28, 2013 at 10:30:32AM +0800, Dave Young wrote:
On 10/28/13 at 09:13am, Dave Young wrote:
@@ -447,7 +446,7 @@ void setup_subarch(struct x86_linux_para if (!debugfs_mnt) return; snprintf(filename, PATH_MAX, "%s/%s", debugfs_mnt, "boot_params/data");
- filename[PATH_MAX-1] = 0;
- filename[PATH_MAX - 1] = 0; free(debugfs_mnt);
This change appears to be unrelated to the rest of the patch.
Will remove the change from the patch.
Relooking it, with this patch the line "filename[PATH_MAX - 1] = 0;" is in the share function get_bootparam, it's not in setup_subarch any more so I think it's ok, what do you think?
I'm a bit confused.
My reading of the hunk above is that it is a whitespace-only change. If so it is not a change that I object to but also not one that I feel belongs in this patch. If not I am somehow mistaken.
On 10/28/13 at 01:20pm, Simon Horman wrote:
On Mon, Oct 28, 2013 at 10:30:32AM +0800, Dave Young wrote:
On 10/28/13 at 09:13am, Dave Young wrote:
@@ -447,7 +446,7 @@ void setup_subarch(struct x86_linux_para if (!debugfs_mnt) return; snprintf(filename, PATH_MAX, "%s/%s", debugfs_mnt, "boot_params/data");
- filename[PATH_MAX-1] = 0;
- filename[PATH_MAX - 1] = 0; free(debugfs_mnt);
This change appears to be unrelated to the rest of the patch.
Will remove the change from the patch.
Relooking it, with this patch the line "filename[PATH_MAX - 1] = 0;" is in the share function get_bootparam, it's not in setup_subarch any more so I think it's ok, what do you think?
I'm a bit confused.
My reading of the hunk above is that it is a whitespace-only change. If so it is not a change that I object to but also not one that I feel belongs in this patch. If not I am somehow mistaken.
The diff context is cheating us, it might because of the context (+- 3 lines) is same with original setup_arch, but it is really moving to a new function.
On Mon, Oct 28, 2013 at 01:06:45PM +0800, Dave Young wrote:
On 10/28/13 at 01:20pm, Simon Horman wrote:
On Mon, Oct 28, 2013 at 10:30:32AM +0800, Dave Young wrote:
On 10/28/13 at 09:13am, Dave Young wrote:
@@ -447,7 +446,7 @@ void setup_subarch(struct x86_linux_para if (!debugfs_mnt) return; snprintf(filename, PATH_MAX, "%s/%s", debugfs_mnt, "boot_params/data");
- filename[PATH_MAX-1] = 0;
- filename[PATH_MAX - 1] = 0; free(debugfs_mnt);
This change appears to be unrelated to the rest of the patch.
Will remove the change from the patch.
Relooking it, with this patch the line "filename[PATH_MAX - 1] = 0;" is in the share function get_bootparam, it's not in setup_subarch any more so I think it's ok, what do you think?
I'm a bit confused.
My reading of the hunk above is that it is a whitespace-only change. If so it is not a change that I object to but also not one that I feel belongs in this patch. If not I am somehow mistaken.
The diff context is cheating us, it might because of the context (+- 3 lines) is same with original setup_arch, but it is really moving to a new function.
Ok, in that case I have no objections.
Removing code to pass acpi_rsdp because this patch series will support efi runtime, it's not necessary any more. EFI initialization code will take the functionality.
Signed-off-by: Dave Young dyoung@redhat.com --- include/x86/x86-linux.h | 3 ++- kexec/arch/i386/crashdump-x86.c | 35 ----------------------------------- kexec/arch/i386/x86-linux-setup.c | 8 ++++++++ 3 files changed, 10 insertions(+), 36 deletions(-)
--- kexec-tools.orig/kexec/arch/i386/crashdump-x86.c +++ kexec-tools/kexec/arch/i386/crashdump-x86.c @@ -848,40 +848,6 @@ 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) { @@ -1046,7 +1012,6 @@ 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. */
On Sun, Oct 27, 2013 at 12:04:39PM +0800, dyoung@redhat.com wrote:
Removing code to pass acpi_rsdp because this patch series will support efi runtime, it's not necessary any more. EFI initialization code will take the functionality.
I wonder if it would make more sense to make this the last patch in the series or squash it into the last patch of the series.
Signed-off-by: Dave Young dyoung@redhat.com
include/x86/x86-linux.h | 3 ++- kexec/arch/i386/crashdump-x86.c | 35 ----------------------------------- kexec/arch/i386/x86-linux-setup.c | 8 ++++++++ 3 files changed, 10 insertions(+), 36 deletions(-)
--- kexec-tools.orig/kexec/arch/i386/crashdump-x86.c +++ kexec-tools/kexec/arch/i386/crashdump-x86.c @@ -848,40 +848,6 @@ 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) { @@ -1046,7 +1012,6 @@ 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. */
On 10/28/13 at 09:39am, Simon Horman wrote:
On Sun, Oct 27, 2013 at 12:04:39PM +0800, dyoung@redhat.com wrote:
Removing code to pass acpi_rsdp because this patch series will support efi runtime, it's not necessary any more. EFI initialization code will take the functionality.
I wonder if it would make more sense to make this the last patch in the series or squash it into the last patch of the series.
Sure, logiclly it should be the last one.
-- Thanks Dave
On Sun, Oct 27, 2013 at 12:04:39PM +0800, dyoung@redhat.com wrote:
Removing code to pass acpi_rsdp because this patch series will support efi runtime, it's not necessary any more. EFI initialization code will take the functionality.
Won't this break kexec of old kernels that don't have the new UEFI setup code?
Hi, Thanks for review.
On 10/28/13 at 09:42am, Matthew Garrett wrote:
On Sun, Oct 27, 2013 at 12:04:39PM +0800, dyoung@redhat.com wrote:
Removing code to pass acpi_rsdp because this patch series will support efi runtime, it's not necessary any more. EFI initialization code will take the functionality.
Won't this break kexec of old kernels that don't have the new UEFI setup code?
For old kernels the efi setup_data esdata will be NULL, it wil switch to old logic. Also for old kernel user need to pass acpi_rsdp in cmdline
Anyway I will do test with old/new kernel/userspace test results.
-- Thanks Dave
On Mon, Oct 28, 2013 at 05:54:59PM +0800, Dave Young wrote:
Hi, Thanks for review.
On 10/28/13 at 09:42am, Matthew Garrett wrote:
On Sun, Oct 27, 2013 at 12:04:39PM +0800, dyoung@redhat.com wrote:
Removing code to pass acpi_rsdp because this patch series will support efi runtime, it's not necessary any more. EFI initialization code will take the functionality.
Won't this break kexec of old kernels that don't have the new UEFI setup code?
For old kernels the efi setup_data esdata will be NULL, it wil switch to old logic. Also for old kernel user need to pass acpi_rsdp in cmdline
Right, but previously acpi_rsdp was passed automatically and now it won't be?
On 10/28/13 at 10:12am, Matthew Garrett wrote:
On Mon, Oct 28, 2013 at 05:54:59PM +0800, Dave Young wrote:
Hi, Thanks for review.
On 10/28/13 at 09:42am, Matthew Garrett wrote:
On Sun, Oct 27, 2013 at 12:04:39PM +0800, dyoung@redhat.com wrote:
Removing code to pass acpi_rsdp because this patch series will support efi runtime, it's not necessary any more. EFI initialization code will take the functionality.
Won't this break kexec of old kernels that don't have the new UEFI setup code?
For old kernels the efi setup_data esdata will be NULL, it wil switch to old logic. Also for old kernel user need to pass acpi_rsdp in cmdline
Right, but previously acpi_rsdp was passed automatically and now it won't be?
Yes, it was. I'm removing them in kexec-tools patches for efi runtime support.
Correct myself about esdata, for old kernels there's no setup_data type for efi so parse_efi_setup will not be called, thus there will be no effect with passing new setup_data because nobody use it?
On Mon, Oct 28, 2013 at 06:34:12PM +0800, Dave Young wrote:
On 10/28/13 at 10:12am, Matthew Garrett wrote:
Right, but previously acpi_rsdp was passed automatically and now it won't be?
Yes, it was. I'm removing them in kexec-tools patches for efi runtime support.
If I upgrade kexec-tools and try to launch an old kernel, I now need to add an extra parameter?
On 10/28/13 at 10:39am, Matthew Garrett wrote:
On Mon, Oct 28, 2013 at 06:34:12PM +0800, Dave Young wrote:
On 10/28/13 at 10:12am, Matthew Garrett wrote:
Right, but previously acpi_rsdp was passed automatically and now it won't be?
Yes, it was. I'm removing them in kexec-tools patches for efi runtime support.
If I upgrade kexec-tools and try to launch an old kernel, I now need to add an extra parameter?
Yes, it should work by passing the acpi_rsdp= via --append
On Mon, Oct 28, 2013 at 06:45:32PM +0800, Dave Young wrote:
On 10/28/13 at 10:39am, Matthew Garrett wrote:
On Mon, Oct 28, 2013 at 06:34:12PM +0800, Dave Young wrote:
On 10/28/13 at 10:12am, Matthew Garrett wrote:
Right, but previously acpi_rsdp was passed automatically and now it won't be?
Yes, it was. I'm removing them in kexec-tools patches for efi runtime support.
If I upgrade kexec-tools and try to launch an old kernel, I now need to add an extra parameter?
Yes, it should work by passing the acpi_rsdp= via --append
Yes, that's my point. You're breaking old configurations by requiring the user to pass an additional argument.
On 10/28/13 at 10:51am, Matthew Garrett wrote:
On Mon, Oct 28, 2013 at 06:45:32PM +0800, Dave Young wrote:
On 10/28/13 at 10:39am, Matthew Garrett wrote:
On Mon, Oct 28, 2013 at 06:34:12PM +0800, Dave Young wrote:
On 10/28/13 at 10:12am, Matthew Garrett wrote:
Right, but previously acpi_rsdp was passed automatically and now it won't be?
Yes, it was. I'm removing them in kexec-tools patches for efi runtime support.
If I upgrade kexec-tools and try to launch an old kernel, I now need to add an extra parameter?
Yes, it should work by passing the acpi_rsdp= via --append
Yes, that's my point. You're breaking old configurations by requiring the user to pass an additional argument.
Hmm, I will do some test without this patch, if it works well it's not harmful to drop this one.
On 10/28/13 at 06:53pm, Dave Young wrote:
On 10/28/13 at 10:51am, Matthew Garrett wrote:
On Mon, Oct 28, 2013 at 06:45:32PM +0800, Dave Young wrote:
On 10/28/13 at 10:39am, Matthew Garrett wrote:
On Mon, Oct 28, 2013 at 06:34:12PM +0800, Dave Young wrote:
On 10/28/13 at 10:12am, Matthew Garrett wrote:
Right, but previously acpi_rsdp was passed automatically and now it won't be?
Yes, it was. I'm removing them in kexec-tools patches for efi runtime support.
If I upgrade kexec-tools and try to launch an old kernel, I now need to add an extra parameter?
Yes, it should work by passing the acpi_rsdp= via --append
Yes, that's my point. You're breaking old configurations by requiring the user to pass an additional argument.
Hmm, I will do some test without this patch, if it works well it's not harmful to drop this one.
Removing this patch does not work because in this series efi_info is passed into 2nd kernel thus 2nd kernel will initilize efi but it does not have related code to handle the converted talbe addresses and it will endter virtual mode again.
I will think about how to solve this problem.
On 10/29/13 at 11:05am, Dave Young wrote:
On 10/28/13 at 06:53pm, Dave Young wrote:
On 10/28/13 at 10:51am, Matthew Garrett wrote:
On Mon, Oct 28, 2013 at 06:45:32PM +0800, Dave Young wrote:
On 10/28/13 at 10:39am, Matthew Garrett wrote:
On Mon, Oct 28, 2013 at 06:34:12PM +0800, Dave Young wrote:
On 10/28/13 at 10:12am, Matthew Garrett wrote: > Right, but previously acpi_rsdp was passed automatically and now it > won't be?
Yes, it was. I'm removing them in kexec-tools patches for efi runtime support.
If I upgrade kexec-tools and try to launch an old kernel, I now need to add an extra parameter?
Yes, it should work by passing the acpi_rsdp= via --append
Yes, that's my point. You're breaking old configurations by requiring the user to pass an additional argument.
Hmm, I will do some test without this patch, if it works well it's not harmful to drop this one.
Removing this patch does not work because in this series efi_info is passed into 2nd kernel thus 2nd kernel will initilize efi but it does not have related code to handle the converted talbe addresses and it will endter virtual mode again.
I will think about how to solve this problem.
The only way what I can think out is introduce a flag in x86 setup header. Will try this way, the userspace code will use old logic if the kernel is old.
On 10/29/13 at 11:34am, Dave Young wrote:
On 10/29/13 at 11:05am, Dave Young wrote:
On 10/28/13 at 06:53pm, Dave Young wrote:
On 10/28/13 at 10:51am, Matthew Garrett wrote:
On Mon, Oct 28, 2013 at 06:45:32PM +0800, Dave Young wrote:
On 10/28/13 at 10:39am, Matthew Garrett wrote:
On Mon, Oct 28, 2013 at 06:34:12PM +0800, Dave Young wrote: > On 10/28/13 at 10:12am, Matthew Garrett wrote: > > Right, but previously acpi_rsdp was passed automatically and now it > > won't be? > > Yes, it was. I'm removing them in kexec-tools patches for efi runtime support.
If I upgrade kexec-tools and try to launch an old kernel, I now need to add an extra parameter?
Yes, it should work by passing the acpi_rsdp= via --append
Yes, that's my point. You're breaking old configurations by requiring the user to pass an additional argument.
Hmm, I will do some test without this patch, if it works well it's not harmful to drop this one.
Removing this patch does not work because in this series efi_info is passed into 2nd kernel thus 2nd kernel will initilize efi but it does not have related code to handle the converted talbe addresses and it will endter virtual mode again.
I will think about how to solve this problem.
The only way what I can think out is introduce a flag in x86 setup header. Will try this way, the userspace code will use old logic if the kernel is old.
Firstly I want to ask opinion from HPA, what do you think about adding a flag to xloadflags for this purpose?
Seems reasonable to me.
Dave Young dyoung@redhat.com wrote:
On 10/29/13 at 11:34am, Dave Young wrote:
On 10/29/13 at 11:05am, Dave Young wrote:
On 10/28/13 at 06:53pm, Dave Young wrote:
On 10/28/13 at 10:51am, Matthew Garrett wrote:
On Mon, Oct 28, 2013 at 06:45:32PM +0800, Dave Young wrote:
On 10/28/13 at 10:39am, Matthew Garrett wrote: > On Mon, Oct 28, 2013 at 06:34:12PM +0800, Dave Young wrote: > > On 10/28/13 at 10:12am, Matthew Garrett wrote: > > > Right, but previously acpi_rsdp was passed
automatically and now it
> > > won't be? > > > > Yes, it was. I'm removing them in kexec-tools patches for
efi runtime support.
> > If I upgrade kexec-tools and try to launch an old kernel, I
now need to
> add an extra parameter?
Yes, it should work by passing the acpi_rsdp= via --append
Yes, that's my point. You're breaking old configurations by
requiring
the user to pass an additional argument.
Hmm, I will do some test without this patch, if it works well
it's not
harmful to drop this one.
Removing this patch does not work because in this series efi_info
is passed into
2nd kernel thus 2nd kernel will initilize efi but it does not have
related code
to handle the converted talbe addresses and it will endter virtual
mode again.
I will think about how to solve this problem.
The only way what I can think out is introduce a flag in x86 setup
header.
Will try this way, the userspace code will use old logic if the
kernel is old.
Firstly I want to ask opinion from HPA, what do you think about adding a flag to xloadflags for this purpose?
On Mon, Oct 28, 2013 at 10:51:19AM +0000, Matthew Garrett wrote:
On Mon, Oct 28, 2013 at 06:45:32PM +0800, Dave Young wrote:
On 10/28/13 at 10:39am, Matthew Garrett wrote:
On Mon, Oct 28, 2013 at 06:34:12PM +0800, Dave Young wrote:
On 10/28/13 at 10:12am, Matthew Garrett wrote:
Right, but previously acpi_rsdp was passed automatically and now it won't be?
Yes, it was. I'm removing them in kexec-tools patches for efi runtime support.
If I upgrade kexec-tools and try to launch an old kernel, I now need to add an extra parameter?
Yes, it should work by passing the acpi_rsdp= via --append
Yes, that's my point. You're breaking old configurations by requiring the user to pass an additional argument.
Yes this is a problem. Of course solution is easy by always passing acpi_rsdp on command line. But in long term this is a problem. In the sense, I am not sure how to cleanup the kexec-tools code as things improve. Now we will support the EFI properly and still pass acpi_rsdp always in an effort to matain backward compatibility.
For a very long time kexec-tools were not automatically appending acpi_rsdp and user were supposed to add it on command line. We were carrying this change in kdump scripts and pushed this change into kexec-tools. In hindsight, it looks like that hardcoding parameters in kexec-tools is a bad idea. It is hard to get rid of them in future.
Thanks Vivek
On Mon, Oct 28, 2013 at 12:10:40PM -0400, Vivek Goyal wrote:
On Mon, Oct 28, 2013 at 10:51:19AM +0000, Matthew Garrett wrote:
On Mon, Oct 28, 2013 at 06:45:32PM +0800, Dave Young wrote:
On 10/28/13 at 10:39am, Matthew Garrett wrote:
On Mon, Oct 28, 2013 at 06:34:12PM +0800, Dave Young wrote:
On 10/28/13 at 10:12am, Matthew Garrett wrote:
Right, but previously acpi_rsdp was passed automatically and now it won't be?
Yes, it was. I'm removing them in kexec-tools patches for efi runtime support.
If I upgrade kexec-tools and try to launch an old kernel, I now need to add an extra parameter?
Yes, it should work by passing the acpi_rsdp= via --append
Yes, that's my point. You're breaking old configurations by requiring the user to pass an additional argument.
Yes this is a problem. Of course solution is easy by always passing acpi_rsdp on command line. But in long term this is a problem. In the sense, I am not sure how to cleanup the kexec-tools code as things improve. Now we will support the EFI properly and still pass acpi_rsdp always in an effort to matain backward compatibility.
For a very long time kexec-tools were not automatically appending acpi_rsdp and user were supposed to add it on command line. We were carrying this change in kdump scripts and pushed this change into kexec-tools. In hindsight, it looks like that hardcoding parameters in kexec-tools is a bad idea. It is hard to get rid of them in future.
I tend to agree. However, if the parameters end up being hard coded elsewhere, for example in widely used wrapper scripts, then I think that the same problem still exists. Just outside of kexec-tools itself.
On 10/29/13 at 11:04am, Simon Horman wrote:
On Mon, Oct 28, 2013 at 12:10:40PM -0400, Vivek Goyal wrote:
On Mon, Oct 28, 2013 at 10:51:19AM +0000, Matthew Garrett wrote:
On Mon, Oct 28, 2013 at 06:45:32PM +0800, Dave Young wrote:
On 10/28/13 at 10:39am, Matthew Garrett wrote:
On Mon, Oct 28, 2013 at 06:34:12PM +0800, Dave Young wrote:
On 10/28/13 at 10:12am, Matthew Garrett wrote: > Right, but previously acpi_rsdp was passed automatically and now it > won't be?
Yes, it was. I'm removing them in kexec-tools patches for efi runtime support.
If I upgrade kexec-tools and try to launch an old kernel, I now need to add an extra parameter?
Yes, it should work by passing the acpi_rsdp= via --append
Yes, that's my point. You're breaking old configurations by requiring the user to pass an additional argument.
Yes this is a problem. Of course solution is easy by always passing acpi_rsdp on command line. But in long term this is a problem. In the sense, I am not sure how to cleanup the kexec-tools code as things improve. Now we will support the EFI properly and still pass acpi_rsdp always in an effort to matain backward compatibility.
For a very long time kexec-tools were not automatically appending acpi_rsdp and user were supposed to add it on command line. We were carrying this change in kdump scripts and pushed this change into kexec-tools. In hindsight, it looks like that hardcoding parameters in kexec-tools is a bad idea. It is hard to get rid of them in future.
I tend to agree. However, if the parameters end up being hard coded elsewhere, for example in widely used wrapper scripts, then I think that the same problem still exists. Just outside of kexec-tools itself.
Kernel has a rule of not breaking userspace, does kexec-tools also have the policy? Maybe user or distribution carry that will be slight better so upstream kexec code will be cleaner..
On 10/29/13 at 10:36am, Dave Young wrote:
On 10/29/13 at 11:04am, Simon Horman wrote:
On Mon, Oct 28, 2013 at 12:10:40PM -0400, Vivek Goyal wrote:
On Mon, Oct 28, 2013 at 10:51:19AM +0000, Matthew Garrett wrote:
On Mon, Oct 28, 2013 at 06:45:32PM +0800, Dave Young wrote:
On 10/28/13 at 10:39am, Matthew Garrett wrote:
On Mon, Oct 28, 2013 at 06:34:12PM +0800, Dave Young wrote: > On 10/28/13 at 10:12am, Matthew Garrett wrote: > > Right, but previously acpi_rsdp was passed automatically and now it > > won't be? > > Yes, it was. I'm removing them in kexec-tools patches for efi runtime support.
If I upgrade kexec-tools and try to launch an old kernel, I now need to add an extra parameter?
Yes, it should work by passing the acpi_rsdp= via --append
Yes, that's my point. You're breaking old configurations by requiring the user to pass an additional argument.
Yes this is a problem. Of course solution is easy by always passing acpi_rsdp on command line. But in long term this is a problem. In the sense, I am not sure how to cleanup the kexec-tools code as things improve. Now we will support the EFI properly and still pass acpi_rsdp always in an effort to matain backward compatibility.
For a very long time kexec-tools were not automatically appending acpi_rsdp and user were supposed to add it on command line. We were carrying this change in kdump scripts and pushed this change into kexec-tools. In hindsight, it looks like that hardcoding parameters in kexec-tools is a bad idea. It is hard to get rid of them in future.
I tend to agree. However, if the parameters end up being hard coded elsewhere, for example in widely used wrapper scripts, then I think that the same problem still exists. Just outside of kexec-tools itself.
Kernel has a rule of not breaking userspace, does kexec-tools also have the policy? Maybe user or distribution carry that will be slight better so upstream kexec code will be cleaner..
Forgot what I said above about the policy, they are different things.
I just replied mjg that even carry this acpi_rsdp, old kernel can not be used because there's efi info in setup_header.
On Tue, Oct 29, 2013 at 10:36:36AM +0800, Dave Young wrote:
On 10/29/13 at 11:04am, Simon Horman wrote:
On Mon, Oct 28, 2013 at 12:10:40PM -0400, Vivek Goyal wrote:
On Mon, Oct 28, 2013 at 10:51:19AM +0000, Matthew Garrett wrote:
On Mon, Oct 28, 2013 at 06:45:32PM +0800, Dave Young wrote:
On 10/28/13 at 10:39am, Matthew Garrett wrote:
On Mon, Oct 28, 2013 at 06:34:12PM +0800, Dave Young wrote: > On 10/28/13 at 10:12am, Matthew Garrett wrote: > > Right, but previously acpi_rsdp was passed automatically and now it > > won't be? > > Yes, it was. I'm removing them in kexec-tools patches for efi runtime support.
If I upgrade kexec-tools and try to launch an old kernel, I now need to add an extra parameter?
Yes, it should work by passing the acpi_rsdp= via --append
Yes, that's my point. You're breaking old configurations by requiring the user to pass an additional argument.
Yes this is a problem. Of course solution is easy by always passing acpi_rsdp on command line. But in long term this is a problem. In the sense, I am not sure how to cleanup the kexec-tools code as things improve. Now we will support the EFI properly and still pass acpi_rsdp always in an effort to matain backward compatibility.
For a very long time kexec-tools were not automatically appending acpi_rsdp and user were supposed to add it on command line. We were carrying this change in kdump scripts and pushed this change into kexec-tools. In hindsight, it looks like that hardcoding parameters in kexec-tools is a bad idea. It is hard to get rid of them in future.
I tend to agree. However, if the parameters end up being hard coded elsewhere, for example in widely used wrapper scripts, then I think that the same problem still exists. Just outside of kexec-tools itself.
Kernel has a rule of not breaking userspace, does kexec-tools also have the policy? Maybe user or distribution carry that will be slight better so upstream kexec code will be cleaner..
I'm not sure that we have had to make a policy decision with regards to this. At least not recently. But yes, I think it should be a goal of kexec-tools not to break its users.
On Tue, Oct 29, 2013 at 11:04:28AM +0900, Simon Horman wrote:
[..]
Yes, that's my point. You're breaking old configurations by requiring the user to pass an additional argument.
Yes this is a problem. Of course solution is easy by always passing acpi_rsdp on command line. But in long term this is a problem. In the sense, I am not sure how to cleanup the kexec-tools code as things improve. Now we will support the EFI properly and still pass acpi_rsdp always in an effort to matain backward compatibility.
For a very long time kexec-tools were not automatically appending acpi_rsdp and user were supposed to add it on command line. We were carrying this change in kdump scripts and pushed this change into kexec-tools. In hindsight, it looks like that hardcoding parameters in kexec-tools is a bad idea. It is hard to get rid of them in future.
I tend to agree. However, if the parameters end up being hard coded elsewhere, for example in widely used wrapper scripts, then I think that the same problem still exists. Just outside of kexec-tools itself.
I think getting rid of them in vendor scripts is easier beacause control the range of kexec-tools and kernel version which will be used in a particular release life cycle. So when we know that we are not going to support other kernels than vendor shipped kernel, we can remove parameters from scripts.
Thanks Vivek
For supporting efi runtime on kexec kernel we need to fill the efi_info struct in setup_header. I just get the info in kernel exported boot_params data in debugfs.
Signed-off-by: Dave Young dyoung@redhat.com --- --- kexec-tools.orig/include/x86/x86-linux.h +++ kexec-tools/include/x86/x86-linux.h @@ -115,7 +115,8 @@ struct x86_linux_param_header { uint32_t ext_ramdisk_image; /* 0xc0 */ uint32_t ext_ramdisk_size; /* 0xc4 */ uint32_t ext_cmd_line_ptr; /* 0xc8 */ - uint8_t reserved4_1[0x1e0 - 0xcc]; /* 0xcc */ + uint8_t reserved4_1[0x1c0 - 0xcc]; /* 0xcc */ + uint8_t efi_info[32]; /* 0x1c0 */ uint32_t alt_mem_k; /* 0x1e0 */ uint8_t reserved5[4]; /* 0x1e4 */ uint8_t e820_map_nr; /* 0x1e8 */ --- kexec-tools.orig/kexec/arch/i386/x86-linux-setup.c +++ kexec-tools/kexec/arch/i386/x86-linux-setup.c @@ -466,6 +466,13 @@ void setup_subarch(struct x86_linux_para get_bootparam(&real_mode->hardware_subarch, offset, sizeof(uint32_t)); }
+void setup_efi_info(struct x86_linux_param_header *real_mode) +{ + off_t offset = offsetof(typeof(*real_mode), efi_info); + + get_bootparam(&real_mode->efi_info, offset, 32); +} + void setup_linux_system_parameters(struct kexec_info *info, struct x86_linux_param_header *real_mode) { @@ -475,6 +482,7 @@ void setup_linux_system_parameters(struc
/* get subarch from running kernel */ setup_subarch(real_mode); + setup_efi_info(real_mode); /* Default screen size */ real_mode->orig_x = 0;
For supporting efi runtime, several efi physical addresses fw_vendor, runtime, config tables, smbios and the whole runtime mapping info need to be used in kexec kernel. Thus introduce setup_data struct for passing these data.
collect the varialbes from /sys/firmware/efi/systab and /sys/firmware/efi/efi-runtime-map
Tested on qemu+ovmf, dell laptop, lenovo laptop and HP workstation.
Signed-off-by: Dave Young dyoung@redhat.com --- include/x86/x86-linux.h | 2 kexec/arch/i386/x86-linux-setup.c | 120 +++++++++++++++++++++++++++++++++++++- 2 files changed, 120 insertions(+), 2 deletions(-)
--- kexec-tools.orig/include/x86/x86-linux.h +++ kexec-tools/include/x86/x86-linux.h @@ -115,7 +115,7 @@ struct x86_linux_param_header { uint32_t ext_ramdisk_image; /* 0xc0 */ uint32_t ext_ramdisk_size; /* 0xc4 */ uint32_t ext_cmd_line_ptr; /* 0xc8 */ - uint8_t reserved4_1[0x1c0 - 0xcc]; /* 0xcc */ + uint8_t reserved4_1[0x1c0 - 0xcc]; /* 0xe4 */ uint8_t efi_info[32]; /* 0x1c0 */ uint32_t alt_mem_k; /* 0x1e0 */ uint8_t reserved5[4]; /* 0x1e4 */ --- kexec-tools.orig/kexec/arch/i386/x86-linux-setup.c +++ kexec-tools/kexec/arch/i386/x86-linux-setup.c @@ -473,6 +473,124 @@ void setup_efi_info(struct x86_linux_par get_bootparam(&real_mode->efi_info, offset, 32); }
+struct efi_mem_descriptor { + uint32_t type; + uint32_t pad; + uint64_t phys_addr; + uint64_t virt_addr; + uint64_t num_pages; + uint64_t attribute; +}; + +struct efi_setup_data { + uint64_t fw_vendor; + uint64_t runtime; + uint64_t tables; + uint64_t smbios; + uint64_t reserved[8]; + struct efi_mem_descriptor map[0]; +}; + +struct setup_data { + __uint64_t next; + __u32 type; + __u32 len; + __u8 data[0]; +}__attribute__ ((packed)); + +static void _get_efi_value(char *line, const char *pattern, __uint64_t *val) +{ + char *s, *end; + s = strstr(line, pattern); + if (s) + *val = strtoull(s + strlen(pattern), &end, 16); +} + +static void get_efi_value(struct efi_setup_data *esd) +{ + FILE *fp; + char line[1024]; + + fp = fopen("/sys/firmware/efi/systab", "r"); + if (!fp) + return; + + while(fgets(line, sizeof(line), fp) != 0) { + _get_efi_value(line, "fw_vendor=0x", &esd->fw_vendor); + _get_efi_value(line, "runtime=0x", &esd->runtime); + _get_efi_value(line, "config_tables=0x", &esd->tables); + _get_efi_value(line, "SMBIOS=0x", &esd->smbios); + } + + fclose(fp); +} + +static int get_efi_runtime_map(struct efi_setup_data **esd) +{ + DIR * dirp; + struct dirent * entry; + char filename[1024]; + struct efi_mem_descriptor md; + int nr_maps = 0; + + dirp = opendir("/sys/firmware/efi/efi-runtime-map"); + while ((entry = readdir(dirp)) != NULL) { + sprintf(filename, "/sys/firmware/efi/efi-runtime-map/%s", (char *)entry->d_name); + if (*entry->d_name == '.' ) + continue; + file_scanf(filename, "type", "0x%x", (unsigned int *)&md.type); + file_scanf(filename, "phys_addr", "0x%llx", (unsigned long long *)&md.phys_addr); + file_scanf(filename, "virt_addr", "0x%llx", (unsigned long long *)&md.virt_addr); + file_scanf(filename, "num_pages", "0x%llx", (unsigned long long *)&md.num_pages); + file_scanf(filename, "attribute", "0x%llx", (unsigned long long *)&md.attribute); + *esd = realloc(*esd, sizeof(struct efi_setup_data) + (nr_maps + 1) * sizeof(struct efi_mem_descriptor)); + *((*esd)->map + nr_maps) = md; + nr_maps++; + } + + closedir(dirp); + return nr_maps; +} + +static void setup_efi_setup_data(struct kexec_info *info, + struct x86_linux_param_header *real_mode) +{ + int nr_maps; + int64_t setup_data_paddr; + struct setup_data *sd; + struct efi_setup_data *esd; + int size, sdsize; + int has_efi = 0; + + has_efi = access("/sys/firmware/efi/systab", F_OK); + if (has_efi < 0) + return; + + esd = malloc(sizeof(struct efi_setup_data)); + if (!esd) + return; + memset(esd, 0, sizeof(struct efi_setup_data)); + get_efi_value(esd); + nr_maps = get_efi_runtime_map(&esd); + size = nr_maps * sizeof(struct efi_mem_descriptor) + sizeof(struct efi_setup_data); + sd = malloc(sizeof(struct setup_data) + size); + if (!sd) { + free(esd); + return; + } + + memset(sd, 0, sizeof(struct setup_data) + size); + sd->next = 0; + sd->type = 4; + sd->len = size; + memcpy(sd->data, esd, size); + sdsize = sd->len + sizeof(struct setup_data); + setup_data_paddr = add_buffer(info, sd, sdsize, sdsize, getpagesize(), 0x100000, ULONG_MAX, INT_MAX); + + real_mode->setup_data = setup_data_paddr; +} + + void setup_linux_system_parameters(struct kexec_info *info, struct x86_linux_param_header *real_mode) { @@ -483,7 +601,7 @@ void setup_linux_system_parameters(struc /* get subarch from running kernel */ setup_subarch(real_mode); setup_efi_info(real_mode); - + setup_efi_setup_data(info, real_mode); /* Default screen size */ real_mode->orig_x = 0; real_mode->orig_y = 0;
On Sun, Oct 27, 2013 at 12:04:41PM +0800, dyoung@redhat.com wrote:
For supporting efi runtime, several efi physical addresses fw_vendor, runtime, config tables, smbios and the whole runtime mapping info need to be used in kexec kernel. Thus introduce setup_data struct for passing these data.
collect the varialbes from /sys/firmware/efi/systab and /sys/firmware/efi/efi-runtime-map
Tested on qemu+ovmf, dell laptop, lenovo laptop and HP workstation.
Signed-off-by: Dave Young dyoung@redhat.com
include/x86/x86-linux.h | 2 kexec/arch/i386/x86-linux-setup.c | 120 +++++++++++++++++++++++++++++++++++++- 2 files changed, 120 insertions(+), 2 deletions(-)
--- kexec-tools.orig/include/x86/x86-linux.h +++ kexec-tools/include/x86/x86-linux.h @@ -115,7 +115,7 @@ struct x86_linux_param_header { uint32_t ext_ramdisk_image; /* 0xc0 */ uint32_t ext_ramdisk_size; /* 0xc4 */ uint32_t ext_cmd_line_ptr; /* 0xc8 */
- uint8_t reserved4_1[0x1c0 - 0xcc]; /* 0xcc */
- uint8_t reserved4_1[0x1c0 - 0xcc]; /* 0xe4 */ uint8_t efi_info[32]; /* 0x1c0 */ uint32_t alt_mem_k; /* 0x1e0 */ uint8_t reserved5[4]; /* 0x1e4 */
--- kexec-tools.orig/kexec/arch/i386/x86-linux-setup.c +++ kexec-tools/kexec/arch/i386/x86-linux-setup.c @@ -473,6 +473,124 @@ void setup_efi_info(struct x86_linux_par get_bootparam(&real_mode->efi_info, offset, 32); }
+struct efi_mem_descriptor {
uint32_t type;uint32_t pad;uint64_t phys_addr;uint64_t virt_addr;uint64_t num_pages;uint64_t attribute;+};
+struct efi_setup_data {
uint64_t fw_vendor;uint64_t runtime;uint64_t tables;uint64_t smbios;uint64_t reserved[8];struct efi_mem_descriptor map[0];+};
+struct setup_data {
__uint64_t next;__u32 type;__u32 len;__u8 data[0];+}__attribute__ ((packed));
+static void _get_efi_value(char *line, const char *pattern, __uint64_t *val) +{
- char *s, *end;
- s = strstr(line, pattern);
- if (s)
*val = strtoull(s + strlen(pattern), &end, 16);+}
+static void get_efi_value(struct efi_setup_data *esd) +{
- FILE *fp;
- char line[1024];
- fp = fopen("/sys/firmware/efi/systab", "r");
- if (!fp)
return;- while(fgets(line, sizeof(line), fp) != 0) {
_get_efi_value(line, "fw_vendor=0x", &esd->fw_vendor);_get_efi_value(line, "runtime=0x", &esd->runtime);_get_efi_value(line, "config_tables=0x", &esd->tables);_get_efi_value(line, "SMBIOS=0x", &esd->smbios);- }
- fclose(fp);
+}
+static int get_efi_runtime_map(struct efi_setup_data **esd) +{
- DIR * dirp;
- struct dirent * entry;
- char filename[1024];
- struct efi_mem_descriptor md;
- int nr_maps = 0;
- dirp = opendir("/sys/firmware/efi/efi-runtime-map");
- while ((entry = readdir(dirp)) != NULL) {
sprintf(filename, "/sys/firmware/efi/efi-runtime-map/%s", (char *)entry->d_name);if (*entry->d_name == '.' )continue;file_scanf(filename, "type", "0x%x", (unsigned int *)&md.type);file_scanf(filename, "phys_addr", "0x%llx", (unsigned long long *)&md.phys_addr);file_scanf(filename, "virt_addr", "0x%llx", (unsigned long long *)&md.virt_addr);file_scanf(filename, "num_pages", "0x%llx", (unsigned long long *)&md.num_pages);file_scanf(filename, "attribute", "0x%llx", (unsigned long long *)&md.attribute);*esd = realloc(*esd, sizeof(struct efi_setup_data) + (nr_maps + 1) * sizeof(struct efi_mem_descriptor));*((*esd)->map + nr_maps) = md;nr_maps++;- }
- closedir(dirp);
- return nr_maps;
+}
+static void setup_efi_setup_data(struct kexec_info *info,
struct x86_linux_param_header *real_mode)+{
- int nr_maps;
- int64_t setup_data_paddr;
- struct setup_data *sd;
- struct efi_setup_data *esd;
- int size, sdsize;
- int has_efi = 0;
The indentation of the line below is inconsistent with the line above.
has_efi = access("/sys/firmware/efi/systab", F_OK);if (has_efi < 0)return;- esd = malloc(sizeof(struct efi_setup_data));
- if (!esd)
return;
This function appears to leak esd.
- memset(esd, 0, sizeof(struct efi_setup_data));
- get_efi_value(esd);
- nr_maps = get_efi_runtime_map(&esd);
- size = nr_maps * sizeof(struct efi_mem_descriptor) + sizeof(struct efi_setup_data);
- sd = malloc(sizeof(struct setup_data) + size);
- if (!sd) {
free(esd);return;- }
This function appears to leak sd.
- memset(sd, 0, sizeof(struct setup_data) + size);
- sd->next = 0;
- sd->type = 4;
- sd->len = size;
- memcpy(sd->data, esd, size);
- sdsize = sd->len + sizeof(struct setup_data);
- setup_data_paddr = add_buffer(info, sd, sdsize, sdsize, getpagesize(), 0x100000, ULONG_MAX, INT_MAX);
- real_mode->setup_data = setup_data_paddr;
+}
void setup_linux_system_parameters(struct kexec_info *info, struct x86_linux_param_header *real_mode) { @@ -483,7 +601,7 @@ void setup_linux_system_parameters(struc /* get subarch from running kernel */ setup_subarch(real_mode); setup_efi_info(real_mode);
- setup_efi_setup_data(info, real_mode); /* Default screen size */ real_mode->orig_x = 0; real_mode->orig_y = 0;
Hi, Simon
Thanks for review.
- int has_efi = 0;
The indentation of the line below is inconsistent with the line above.
Will fix
has_efi = access("/sys/firmware/efi/systab", F_OK);if (has_efi < 0)return;- esd = malloc(sizeof(struct efi_setup_data));
- if (!esd)
return;This function appears to leak esd.
I left the error handling after testing but later forgot to add them. Will fix
- memset(esd, 0, sizeof(struct efi_setup_data));
- get_efi_value(esd);
- nr_maps = get_efi_runtime_map(&esd);
- size = nr_maps * sizeof(struct efi_mem_descriptor) + sizeof(struct efi_setup_data);
- sd = malloc(sizeof(struct setup_data) + size);
- if (!sd) {
free(esd);return;- }
This function appears to leak sd.
Will fix
-- Thanks Dave
On 10/28/13 at 09:12am, Dave Young wrote:
- size = nr_maps * sizeof(struct efi_mem_descriptor) + sizeof(struct efi_setup_data);
- sd = malloc(sizeof(struct setup_data) + size);
- if (!sd) {
free(esd);return;- }
This function appears to leak sd.
Will fix
sd will be passed to add_buffer, it should be freed after kexec_load, is it really necessary, or I misunderstand something?
-- Thanks Dave
On Mon, Oct 28, 2013 at 11:02:59AM +0800, Dave Young wrote:
On 10/28/13 at 09:12am, Dave Young wrote:
- size = nr_maps * sizeof(struct efi_mem_descriptor) + sizeof(struct efi_setup_data);
- sd = malloc(sizeof(struct setup_data) + size);
- if (!sd) {
free(esd);return;- }
This function appears to leak sd.
Will fix
sd will be passed to add_buffer, it should be freed after kexec_load, is it really necessary, or I misunderstand something?
Thanks and sorry for missing that. I now think the current handling of sd in this patch seems fine.
On 10/27/13 at 12:04pm, Dave Young wrote:
For supporting efi runtime, several efi physical addresses fw_vendor, runtime, config tables, smbios and the whole runtime mapping info need to be used in kexec kernel. Thus introduce setup_data struct for passing these data.
collect the varialbes from /sys/firmware/efi/systab and /sys/firmware/efi/efi-runtime-map
Tested on qemu+ovmf, dell laptop, lenovo laptop and HP workstation.
Signed-off-by: Dave Young dyoung@redhat.com
include/x86/x86-linux.h | 2 kexec/arch/i386/x86-linux-setup.c | 120 +++++++++++++++++++++++++++++++++++++- 2 files changed, 120 insertions(+), 2 deletions(-)
--- kexec-tools.orig/include/x86/x86-linux.h +++ kexec-tools/include/x86/x86-linux.h @@ -115,7 +115,7 @@ struct x86_linux_param_header { uint32_t ext_ramdisk_image; /* 0xc0 */ uint32_t ext_ramdisk_size; /* 0xc4 */ uint32_t ext_cmd_line_ptr; /* 0xc8 */
- uint8_t reserved4_1[0x1c0 - 0xcc]; /* 0xcc */
- uint8_t reserved4_1[0x1c0 - 0xcc]; /* 0xe4 */
Above change should be in previous patch. Will update in next version.
On 10/28/13 at 03:41am, H. Peter Anvin wrote:
On 10/26/2013 09:04 PM, dyoung@redhat.com wrote:
__uint64_t next;WTF is __uint64_t?
bits/types.h:typedef unsigned long int __uint64_t;
I meant to use uint64_t instead of __unit64_t
Will fix.
-
Dave Young -
H. Peter Anvin -
Horms -
Matthew Garrett -
Vivek Goyal