11:04 p.m.
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
11:04 p.m.
New subject: [patch 1/4] Add function get_bootparam
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(a)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)
{
7:38 p.m.
New subject: [patch 1/4] Add function get_bootparam
On Sun, Oct 27, 2013 at 12:04:38PM +0800, dyoung(a)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(a)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(a)lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
8:13 p.m.
New subject: [patch 1/4] Add function get_bootparam
> @@ -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.
9:30 p.m.
New subject: [patch 1/4] Add function get_bootparam
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
11:20 p.m.
New subject: [patch 1/4] Add function get_bootparam
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.
12:06 a.m.
New subject: [patch 1/4] Add function get_bootparam
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.
1:08 a.m.
New subject: [patch 1/4] Add function get_bootparam
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.
11:04 p.m.
New subject: [patch 2/4] remove extra acpi_rsdp command line for efi
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(a)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. */
7:39 p.m.
New subject: [patch 2/4] remove extra acpi_rsdp command line for efi
On Sun, Oct 27, 2013 at 12:04:39PM +0800, dyoung(a)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(a)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. */
8:39 p.m.
New subject: [patch 2/4] remove extra acpi_rsdp command line for efi
On 10/28/13 at 09:39am, Simon Horman wrote:
On Sun, Oct 27, 2013 at 12:04:39PM +0800, dyoung(a)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
4:42 a.m.
New subject: [patch 2/4] remove extra acpi_rsdp command line for efi
On Sun, Oct 27, 2013 at 12:04:39PM +0800, dyoung(a)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?
--
Matthew Garrett | mjg59(a)srcf.ucam.org
4:54 a.m.
New subject: [patch 2/4] remove extra acpi_rsdp command line for efi
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(a)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
5:12 a.m.
New subject: [patch 2/4] remove extra acpi_rsdp command line for efi
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(a)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?
--
Matthew Garrett | mjg59(a)srcf.ucam.org
5:34 a.m.
New subject: [patch 2/4] remove extra acpi_rsdp command line for efi
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(a)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?
5:39 a.m.
New subject: [patch 2/4] remove extra acpi_rsdp command line for efi
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?
--
Matthew Garrett | mjg59(a)srcf.ucam.org
5:45 a.m.
New subject: [patch 2/4] remove extra acpi_rsdp command line for efi
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
5:51 a.m.
New subject: [patch 2/4] remove extra acpi_rsdp command line for efi
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.
--
Matthew Garrett | mjg59(a)srcf.ucam.org
5:53 a.m.
New subject: [patch 2/4] remove extra acpi_rsdp command line for efi
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.
10:05 p.m.
New subject: [patch 2/4] remove extra acpi_rsdp command line for efi
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.
10:34 p.m.
New subject: [patch 2/4] remove extra acpi_rsdp command line for efi
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.
3:50 a.m.
New subject: [patch 2/4] remove extra acpi_rsdp command line for efi
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?
9:57 a.m.
New subject: [patch 2/4] remove extra acpi_rsdp command line for efi
Seems reasonable to me.
Dave Young <dyoung(a)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?
--
Sent from my mobile phone. Please pardon brevity and lack of formatting.
11:10 a.m.
New subject: [patch 2/4] remove extra acpi_rsdp command line for efi
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
9:04 p.m.
New subject: [patch 2/4] remove extra acpi_rsdp command line for efi
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.
9:36 p.m.
New subject: [patch 2/4] remove extra acpi_rsdp command line for efi
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..
10:18 p.m.
New subject: [patch 2/4] remove extra acpi_rsdp command line for efi
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.
7:45 p.m.
New subject: [patch 2/4] remove extra acpi_rsdp command line for efi
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.
8:27 a.m.
New subject: [patch 2/4] remove extra acpi_rsdp command line for efi
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
11:04 p.m.
New subject: [patch 3/4] Add efi_info in x86 setup header
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(a)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;
11:04 p.m.
New subject: [patch 4/4] Passing efi related data via setup_data
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(a)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;
7:37 p.m.
New subject: [patch 4/4] Passing efi related data via setup_data
On Sun, Oct 27, 2013 at 12:04:41PM +0800, dyoung(a)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(a)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;
8:12 p.m.
New subject: [patch 4/4] Passing efi related data via setup_data
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
10:02 p.m.
New subject: [patch 4/4] Passing efi related data via setup_data
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
11:16 p.m.
New subject: [patch 4/4] Passing efi related data via setup_data
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.
8:28 p.m.
New subject: [patch 4/4] Passing efi related data via setup_data
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(a)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.
6:21 a.m.
New subject: [patch 4/4] Passing efi related data via setup_data
On 10/28/13 at 03:41am, H. Peter Anvin wrote:
On 10/26/2013 09:04 PM, dyoung(a)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.
3824
days inactive
3827
days old
37 comments
5 participants
participants (5)
-
Dave Young -
H. Peter Anvin -
Horms -
Matthew Garrett -
Vivek Goyal