Re: [PATCH v2] x86/boot: Use efi_setup_data for searching RSDP on kexec-ed kernel
On 04/03/19 at 04:23pm, Dave Young wrote: > On 04/03/19 at 03:50pm, Baoquan He wrote: > > On 04/03/19 at 03:30pm, Chao Fan wrote: > > > On Wed, Apr 03, 2019 at 02:39:11PM +0800, Dave Young wrote: > > > >> Actually I got some different kexec test results. > > > >> > > > >> Yesterday, with my installed kernel (based on git head several weeks > > > >> ago), kexec kernel panics. > > > >> > > > >> Then I tried latest mainline with git pull, everything works, (with or > > > >> without the patch, and can not reproduce the bug this patch is fixing) > > > >> > > > >> Today, test again, kexec reboot hangs (with or without your patch), but > > > >> kdump works always (with or without the patch) > > > >> > > > >> It is weird to me. Probably I need find out why I can not reproduce the > > > >> bug this patch is addressing first. > > > >> > > > >> earlyprintk seems not working for me anymore, it is not easy to debug > > > >> on > > > >> laptop now. > > > >> > > > >> But the patch itself is clear, I think it should be good. There might > > > >> be > > > >> other things broken. > > > > > > > >Disable your immovable mem code then everything works for me. There > > > >might be something wrong in the code. Also "nokaslr" does not help, it > > > >should be another problem > > > > > > If "nokaslr" doesn't help, so I think > > > >+ /*num_immovable_mem = count_immovable_mem_regions();*/ > > > also doesn't help. I think the problem is from get_rsdp_addr(). > > > > Yes, seems get_rsdp_addr() has issue in this case. I am wondering if we > > can adjust the postion of get_rsdp_addr() calling. If nokaslr is > > specified, no need to get rsdp? > > I assumed the early code is only be useful for kaslr, if this is true > then no need to get rsdp in case nokaslr. > > But I vaguely remember Boris want the boot_params rsdp pointer be a > general thing, I did not followed up these patch discussions, need to see how > Boris thinks about this. > Personally I would like to have a cmdline to bypass the early acpi code because early code is hard to debug, if we have something weird happens we can try to gate out these code, and then get some idea..
Re: [PATCH v2] x86/boot: Use efi_setup_data for searching RSDP on kexec-ed kernel
On 04/03/19 at 01:53pm, Dave Young wrote:
> On 04/03/19 at 01:35pm, Chao Fan wrote:
> > On Tue, Apr 02, 2019 at 08:03:19PM +0800, Dave Young wrote:
> > >On 04/01/19 at 12:08am, Junichi Nomura wrote:
> > >> Commit 3a63f70bf4c3a ("x86/boot: Early parse RSDP and save it in
> > >> boot_params") broke kexec boot on EFI systems. efi_get_rsdp_addr()
> > >> in the early parsing code tries to search RSDP from EFI table but
> > >> that will crash because the table address is virtual when the kernel
> > >> was booted by kexec.
> > >>
> > >> In the case of kexec, physical address of EFI tables is provided
> > >> via efi_setup_data in boot_params, which is set up by kexec(1).
> > >>
> > >> Factor out the table parsing code and use different pointers depending
> > >> on whether the kernel is booted by kexec or not.
> > >>
> > >> Fixes: 3a63f70bf4c3a ("x86/boot: Early parse RSDP and save it in
> > >> boot_params")
> > >> Signed-off-by: Jun'ichi Nomura
> > >> Acked-by: Baoquan He
> > >> Cc: Chao Fan
> > >> Cc: Borislav Petkov
> > >> Cc: Dave Young
> > [...]
> > >
> > >I failed to kexec reboot on my laptop, kernel panics too quick, I'm not
> > >sure this is
> > >caused by your patch though.
> > >
> > >Actually there are something probably i915 changes break kexec, the
> > >above test is with "nomodeset" which should work.
> > >
> > >Let me do more testing and update here tomorrow.
> > >
> >
> > Hi Dave,
> >
> > Last day I was testing the normal kexec, today I have tested the kdump
> > issue. Since the kdump has set "nokaslr" to cmdline, so I drop from
> > KDUMP_COMMANDLINE_APPEND
> > And it booted OK, so the PATCH works in both normal kexec and kdump.
> >
>
> Actually I got some different kexec test results.
>
> Yesterday, with my installed kernel (based on git head several weeks
> ago), kexec kernel panics.
>
> Then I tried latest mainline with git pull, everything works, (with or
> without the patch, and can not reproduce the bug this patch is fixing)
>
> Today, test again, kexec reboot hangs (with or without your patch), but
> kdump works always (with or without the patch)
>
> It is weird to me. Probably I need find out why I can not reproduce the
> bug this patch is addressing first.
>
> earlyprintk seems not working for me anymore, it is not easy to debug on
> laptop now.
>
> But the patch itself is clear, I think it should be good. There might be
> other things broken.
Disable your immovable mem code then everything works for me. There
might be something wrong in the code. Also "nokaslr" does not help, it
should be another problem
diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index 2e53c056ba20..e760c9159662 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -416,7 +416,7 @@ static void mem_avoid_init(unsigned long input, unsigned
long input_size,
handle_mem_options();
/* Enumerate the immovable memory regions */
- num_immovable_mem = count_immovable_mem_regions();
+ /*num_immovable_mem = count_immovable_mem_regions();*/
#ifdef CONFIG_X86_VERBOSE_BOOTUP
/* Make sure video RAM can be used. */
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index c0d6c560df69..1bc6f46d3aa7 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -352,7 +352,7 @@ asmlinkage __visible void *extract_kernel(void *rmode,
memptr heap,
boot_params->hdr.loadflags &= ~KASLR_FLAG;
/* Save RSDP address for later use. */
- boot_params->acpi_rsdp_addr = get_rsdp_addr();
+/* boot_params->acpi_rsdp_addr = get_rsdp_addr(); */
sanitize_boot_params(boot_params);
Re: [PATCH v2] x86/boot: Use efi_setup_data for searching RSDP on kexec-ed kernel
On 04/01/19 at 12:08am, Junichi Nomura wrote:
> Commit 3a63f70bf4c3a ("x86/boot: Early parse RSDP and save it in
> boot_params") broke kexec boot on EFI systems. efi_get_rsdp_addr()
> in the early parsing code tries to search RSDP from EFI table but
> that will crash because the table address is virtual when the kernel
> was booted by kexec.
>
> In the case of kexec, physical address of EFI tables is provided
> via efi_setup_data in boot_params, which is set up by kexec(1).
>
> Factor out the table parsing code and use different pointers depending
> on whether the kernel is booted by kexec or not.
>
> Fixes: 3a63f70bf4c3a ("x86/boot: Early parse RSDP and save it in boot_params")
> Signed-off-by: Jun'ichi Nomura
> Acked-by: Baoquan He
> Cc: Chao Fan
> Cc: Borislav Petkov
> Cc: Dave Young
>
> --
> v2: Added comments above __efi_get_rsdp_addr() and kexec_get_rsdp_addr()
>
> diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c
> --- a/arch/x86/boot/compressed/acpi.c
> +++ b/arch/x86/boot/compressed/acpi.c
> @@ -44,17 +44,112 @@ static acpi_physical_address get_acpi_rsdp(void)
> return addr;
> }
>
> -/* Search EFI system tables for RSDP. */
> -static acpi_physical_address efi_get_rsdp_addr(void)
> +#ifdef CONFIG_EFI
> +static unsigned long efi_get_kexec_setup_data_addr(void)
> +{
> + struct setup_data *data;
> + u64 pa_data;
> +
> + pa_data = boot_params->hdr.setup_data;
> + while (pa_data) {
> + data = (struct setup_data *) pa_data;
> + if (data->type == SETUP_EFI)
> + return pa_data + sizeof(struct setup_data);
> + pa_data = data->next;
> + }
> + return 0;
> +}
> +
> +/*
> + * Search EFI system tables for RSDP. If both ACPI_20_TABLE_GUID and
> + * ACPI_TABLE_GUID are found, take the former, which has more features.
> + */
> +static acpi_physical_address
> +__efi_get_rsdp_addr(unsigned long config_tables, unsigned int nr_tables,
> + bool efi_64)
> {
> acpi_physical_address rsdp_addr = 0;
> + int i;
> +
> + /* Get EFI tables from systab. */
> + for (i = 0; i < nr_tables; i++) {
> + acpi_physical_address table;
> + efi_guid_t guid;
> +
> + if (efi_64) {
> + efi_config_table_64_t *tbl = (efi_config_table_64_t *)
> config_tables + i;
> +
> + guid = tbl->guid;
> + table = tbl->table;
> +
> + if (!IS_ENABLED(CONFIG_X86_64) && table >> 32) {
> + debug_putstr("Error getting RSDP address: EFI
> config table located above 4GB.\n");
> + return 0;
> + }
> + } else {
> + efi_config_table_32_t *tbl = (efi_config_table_32_t *)
> config_tables + i;
>
> + guid = tbl->guid;
> + table = tbl->table;
> + }
> +
> + if (!(efi_guidcmp(guid, ACPI_TABLE_GUID)))
> + rsdp_addr = table;
> + else if (!(efi_guidcmp(guid, ACPI_20_TABLE_GUID)))
> + return table;
> + }
> +
> + return rsdp_addr;
> +}
> +#endif
> +
> +/*
> + * EFI/kexec support is only added for 64bit. So we don't have to
> + * care 32bit case.
> + */
> +static acpi_physical_address kexec_get_rsdp_addr(void)
> +{
> #ifdef CONFIG_EFI
> - unsigned long systab, systab_tables, config_tables;
> + efi_system_table_64_t *systab;
> + struct efi_setup_data *esd;
> + struct efi_info *ei;
> + char *sig;
> +
> + esd = (struct efi_setup_data *) efi_get_kexec_setup_data_addr();
> + if (!esd)
> + return 0;
> +
> + if (!esd->tables) {
> + debug_putstr("Wrong kexec SETUP_EFI data.\n");
> + return 0;
> + }
> +
> + ei = _params->efi_info;
> + sig = (char *)>efi_loader_signature;
> + if (strncmp(sig, EFI64_LOADER_SIGNATURE, 4)) {
> + debug_putstr("Wrong kexec EFI loader signature.\n");
> + return 0;
> + }
> +
> + /* Get systab from boot params. */
> + systab = (efi_system_table_64_t *) (ei->efi_systab |
> ((__u64)ei->efi_systab_hi << 32));
> + if (!systab)
> + error("EFI system table not found in kexec boot_params.");
> +
> + return __efi_get_rsdp_addr((unsigned long) esd->tables,
> +systab->nr_tables, true);
> +#else
> + return 0;
> +#endif
> +}
> +
> +static acpi_physical_address efi_get_rsdp_addr(void)
> +{
> +#ifdef CONFIG_EFI
> + unsigned long systab, config_tables;
> unsigned int nr_tables;
> struct efi_info *ei;
> bool efi_64;
> - int size, i;
> char *sig;
>
> ei = _params->efi_info;
> @@ -88,49 +183,20 @@ static acpi_physical_address efi_get_rsdp_addr(void)
>
> config_tables = stbl->tables;
> nr_tables = stbl->nr_tables;
> -
Re: [PATCH v2] x86/boot: Use efi_setup_data for searching RSDP on kexec-ed kernel
On Tue, Apr 02, 2019 at 09:53:51AM +, Junichi Nomura wrote: >On Tue, Apr 02, 2019 at 05:41:49PM +0800, Chao Fan wrote: >> [ 77.989030] kexec_core: Starting new kernel >> early console in extract_kernel >> input_data: 0x00017f6033b1 >> input_len: 0x008412d4 >> output: 0x00017e00 >> output_len: 0x01e15844 >> kernel_total_size: 0x01e2c000 >> trampoline_32bit: 0x0009d000 >> booted via startup_64() >> >> >> Physical KASLR disabled: no suitable memory region! >> -- >> >> I am not sure whether I have done the right test. >> This guest is booted from EFI. Here we can see the kexeced kernel >> has completed the compressed boot stage. So I think your PATCH works. > >Thanks for testing. If your test bed doesn't boot even with the patch, >you could check what was found as RSDP with a debug patch like below. Oh no, it booted. I just put the compressed stag log. Thanks, Chao Fan > >diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c >--- a/arch/x86/boot/compressed/misc.c >+++ b/arch/x86/boot/compressed/misc.c >@@ -379,6 +379,7 @@ asmlinkage __visible void *extract_kernel(void *rmode, >memptr heap, > debug_putaddr(output); > debug_putaddr(output_len); > debug_putaddr(kernel_total_size); >+ debug_putaddr(boot_params->acpi_rsdp_addr); > > #ifdef CONFIG_X86_64 > /* Report address of 32-bit trampoline */ > >
