* [PATCH 0/2] make kexec work with efi=noruntime or efi=old_map @ 2019-01-15 9:58 Kairui Song 2019-01-15 9:58 ` [PATCH v2 1/2] x86, kexec_file_load: Don't setup EFI info if EFI runtime is not enabled Kairui Song 2019-01-15 9:58 ` [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map Kairui Song 0 siblings, 2 replies; 25+ messages in thread From: Kairui Song @ 2019-01-15 9:58 UTC (permalink / raw) To: linux-kernel Cc: tglx, mingo, bp, hpa, x86, dyoung, bhe, kexec, akpm, robert.moore, erik.schmauss, rafael.j.wysocki, lenb, Kairui Song This patch series fix the kexec panic on efi=noruntime or efi=old_map and leverage acpi_rsdp_addr to make the second kernel boot up properly. Kairui Song (2): x86, kexec_file_load: Don't setup EFI info if EFI runtime is not enabled x86, kexec_file_load: make it work with efi=noruntime or efi=old_map Update from V1: - Add a cover letter and fix some type in commit message - Previous patches are not sent in a single thread arch/x86/kernel/kexec-bzimage64.c | 24 ++++++++++++++++++++++++ drivers/acpi/acpica/tbxfroot.c | 3 +-- include/acpi/acpixf.h | 2 +- 3 files changed, 26 insertions(+), 3 deletions(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v2 1/2] x86, kexec_file_load: Don't setup EFI info if EFI runtime is not enabled 2019-01-15 9:58 [PATCH 0/2] make kexec work with efi=noruntime or efi=old_map Kairui Song @ 2019-01-15 9:58 ` Kairui Song 2019-01-15 9:58 ` [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map Kairui Song 1 sibling, 0 replies; 25+ messages in thread From: Kairui Song @ 2019-01-15 9:58 UTC (permalink / raw) To: linux-kernel Cc: tglx, mingo, bp, hpa, x86, dyoung, bhe, kexec, akpm, robert.moore, erik.schmauss, rafael.j.wysocki, lenb, Kairui Song Currently with "efi=noruntime" in kernel command line, calling kexec_file_load will raise below problem: [ 97.967067] BUG: unable to handle kernel NULL pointer dereference at 0000000000000000 [ 97.967894] #PF error: [normal kernel read fault] ... [ 97.980456] Call Trace: [ 97.980724] efi_runtime_map_copy+0x28/0x30 [ 97.981267] bzImage64_load+0x688/0x872 [ 97.981794] arch_kexec_kernel_image_load+0x6d/0x70 [ 97.982441] kimage_file_alloc_init+0x13e/0x220 [ 97.983035] __x64_sys_kexec_file_load+0x144/0x290 [ 97.983586] do_syscall_64+0x55/0x1a0 [ 97.983962] entry_SYSCALL_64_after_hwframe+0x44/0xa9 When efi runtime is not enabled, efi memmap is not mapped, so just skip EFI info setup. Suggested-by: Dave Young <dyoung@redhat.com> Signed-off-by: Kairui Song <kasong@redhat.com> --- arch/x86/kernel/kexec-bzimage64.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c index 0d5efa34f359..53917a3ebf94 100644 --- a/arch/x86/kernel/kexec-bzimage64.c +++ b/arch/x86/kernel/kexec-bzimage64.c @@ -167,6 +167,9 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr, struct efi_info *current_ei = &boot_params.efi_info; struct efi_info *ei = ¶ms->efi_info; + if (!efi_enabled(EFI_RUNTIME_SERVICES)) + return 0; + if (!current_ei->efi_memmap_size) return 0; -- 2.20.1 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map 2019-01-15 9:58 [PATCH 0/2] make kexec work with efi=noruntime or efi=old_map Kairui Song 2019-01-15 9:58 ` [PATCH v2 1/2] x86, kexec_file_load: Don't setup EFI info if EFI runtime is not enabled Kairui Song @ 2019-01-15 9:58 ` Kairui Song 2019-01-15 23:10 ` Borislav Petkov 1 sibling, 1 reply; 25+ messages in thread From: Kairui Song @ 2019-01-15 9:58 UTC (permalink / raw) To: linux-kernel Cc: tglx, mingo, bp, hpa, x86, dyoung, bhe, kexec, akpm, robert.moore, erik.schmauss, rafael.j.wysocki, lenb, Kairui Song When efi=noruntime or efi=oldmap is used, EFI services won't be available in the second kernel, therefore the second kernel will not be able to get the ACPI RSDP address from firmware by calling EFI services and won't boot. Previously we are expecting the user to set the acpi_rsdp=<addr> on kernel command line for second kernel as there was no way to pass RSDP address to second kernel. After commit e6e094e053af ('x86/acpi, x86/boot: Take RSDP address from boot params if available'), now it's possible to set an acpi_rsdp_addr parameter in the boot_params passed to second kernel, this commit make use of it, detect and set the RSDP address when it's required for second kernel to boot. Tested with an EFI enabled KVM VM with efi=noruntime. Suggested-by: Dave Young <dyoung@redhat.com> Signed-off-by: Kairui Song <kasong@redhat.com> --- arch/x86/kernel/kexec-bzimage64.c | 21 +++++++++++++++++++++ drivers/acpi/acpica/tbxfroot.c | 3 +-- include/acpi/acpixf.h | 2 +- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c index 53917a3ebf94..0a90dcbd041f 100644 --- a/arch/x86/kernel/kexec-bzimage64.c +++ b/arch/x86/kernel/kexec-bzimage64.c @@ -20,6 +20,7 @@ #include <linux/mm.h> #include <linux/efi.h> #include <linux/verification.h> +#include <linux/acpi.h> #include <asm/bootparam.h> #include <asm/setup.h> @@ -255,8 +256,28 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params, /* Setup EFI state */ setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz, efi_setup_data_offset); + +#ifdef CONFIG_ACPI + /* Setup ACPI RSDP pointer in case EFI is not available in second kernel */ + if (!acpi_disabled && (!efi_enabled(EFI_RUNTIME_SERVICES) || efi_enabled(EFI_OLD_MEMMAP))) { + /* Copied from acpi_os_get_root_pointer accordingly */ + params->acpi_rsdp_addr = boot_params.acpi_rsdp_addr; + if (!params->acpi_rsdp_addr) { + if (efi_enabled(EFI_CONFIG_TABLES)) { + if (efi.acpi20 != EFI_INVALID_TABLE_ADDR) + params->acpi_rsdp_addr = efi.acpi20; + else if (efi.acpi != EFI_INVALID_TABLE_ADDR) + params->acpi_rsdp_addr = efi.acpi; + } else if (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) { + acpi_find_root_pointer(¶ms->acpi_rsdp_addr); + } + } + if (!params->acpi_rsdp_addr) + pr_warn("RSDP is not available for second kernel\n"); + } #endif +#endif /* Setup EDD info */ memcpy(params->eddbuf, boot_params.eddbuf, EDDMAXNR * sizeof(struct edd_info)); diff --git a/drivers/acpi/acpica/tbxfroot.c b/drivers/acpi/acpica/tbxfroot.c index 483d0ce5180a..dac1e34a931c 100644 --- a/drivers/acpi/acpica/tbxfroot.c +++ b/drivers/acpi/acpica/tbxfroot.c @@ -108,8 +108,7 @@ acpi_status acpi_tb_validate_rsdp(struct acpi_table_rsdp *rsdp) * ******************************************************************************/ -acpi_status ACPI_INIT_FUNCTION -acpi_find_root_pointer(acpi_physical_address *table_address) +acpi_status acpi_find_root_pointer(acpi_physical_address *table_address) { u8 *table_ptr; u8 *mem_rover; diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h index 7aa38b648564..869d75ecaf7d 100644 --- a/include/acpi/acpixf.h +++ b/include/acpi/acpixf.h @@ -474,7 +474,7 @@ ACPI_EXTERNAL_RETURN_STATUS(acpi_status ACPI_INIT_FUNCTION ACPI_EXTERNAL_RETURN_STATUS(acpi_status ACPI_INIT_FUNCTION acpi_reallocate_root_table(void)) -ACPI_EXTERNAL_RETURN_STATUS(acpi_status ACPI_INIT_FUNCTION +ACPI_EXTERNAL_RETURN_STATUS(acpi_status acpi_find_root_pointer(acpi_physical_address *rsdp_address)) ACPI_EXTERNAL_RETURN_STATUS(acpi_status -- 2.20.1 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map 2019-01-15 9:58 ` [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map Kairui Song @ 2019-01-15 23:10 ` Borislav Petkov 2019-01-16 3:32 ` Dave Young 2019-01-16 7:08 ` Kairui Song 0 siblings, 2 replies; 25+ messages in thread From: Borislav Petkov @ 2019-01-15 23:10 UTC (permalink / raw) To: Kairui Song Cc: linux-kernel, tglx, mingo, hpa, x86, dyoung, bhe, kexec, akpm, robert.moore, erik.schmauss, rafael.j.wysocki, lenb On Tue, Jan 15, 2019 at 05:58:34PM +0800, Kairui Song wrote: > When efi=noruntime or efi=oldmap is used, EFI services won't be available > in the second kernel, therefore the second kernel will not be able to get > the ACPI RSDP address from firmware by calling EFI services and won't > boot. Previously we are expecting the user to set the acpi_rsdp=<addr> > on kernel command line for second kernel as there was no way to pass RSDP > address to second kernel. > > After commit e6e094e053af ('x86/acpi, x86/boot: Take RSDP address from > boot params if available'), now it's possible to set an acpi_rsdp_addr > parameter in the boot_params passed to second kernel, this commit make > use of it, detect and set the RSDP address when it's required for second > kernel to boot. > > Tested with an EFI enabled KVM VM with efi=noruntime. > > Suggested-by: Dave Young <dyoung@redhat.com> > Signed-off-by: Kairui Song <kasong@redhat.com> > --- > arch/x86/kernel/kexec-bzimage64.c | 21 +++++++++++++++++++++ > drivers/acpi/acpica/tbxfroot.c | 3 +-- > include/acpi/acpixf.h | 2 +- > 3 files changed, 23 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c > index 53917a3ebf94..0a90dcbd041f 100644 > --- a/arch/x86/kernel/kexec-bzimage64.c > +++ b/arch/x86/kernel/kexec-bzimage64.c > @@ -20,6 +20,7 @@ > #include <linux/mm.h> > #include <linux/efi.h> > #include <linux/verification.h> > +#include <linux/acpi.h> > > #include <asm/bootparam.h> > #include <asm/setup.h> > @@ -255,8 +256,28 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params, > /* Setup EFI state */ > setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz, > efi_setup_data_offset); > + > +#ifdef CONFIG_ACPI > + /* Setup ACPI RSDP pointer in case EFI is not available in second kernel */ > + if (!acpi_disabled && (!efi_enabled(EFI_RUNTIME_SERVICES) || efi_enabled(EFI_OLD_MEMMAP))) { > + /* Copied from acpi_os_get_root_pointer accordingly */ > + params->acpi_rsdp_addr = boot_params.acpi_rsdp_addr; > + if (!params->acpi_rsdp_addr) { > + if (efi_enabled(EFI_CONFIG_TABLES)) { > + if (efi.acpi20 != EFI_INVALID_TABLE_ADDR) > + params->acpi_rsdp_addr = efi.acpi20; > + else if (efi.acpi != EFI_INVALID_TABLE_ADDR) > + params->acpi_rsdp_addr = efi.acpi; > + } else if (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) { > + acpi_find_root_pointer(¶ms->acpi_rsdp_addr); > + } > + } > + if (!params->acpi_rsdp_addr) > + pr_warn("RSDP is not available for second kernel\n"); > + } > #endif Amazing the amount of ACPI RDSP parsing and fiddling patches flying around these days... In any case, this needs to be synchronized with: https://lkml.kernel.org/r/20190107032243.25324-1-fanc.fnst@cn.fujitsu.com and checked whether the above can be used instead of sprinkling of ACPI parsing code left and right. Thx. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map 2019-01-15 23:10 ` Borislav Petkov @ 2019-01-16 3:32 ` Dave Young 2019-01-16 5:09 ` Kairui Song 2019-01-16 7:08 ` Kairui Song 1 sibling, 1 reply; 25+ messages in thread From: Dave Young @ 2019-01-16 3:32 UTC (permalink / raw) To: Borislav Petkov Cc: Kairui Song, linux-kernel, tglx, mingo, hpa, x86, bhe, kexec, akpm, robert.moore, erik.schmauss, rafael.j.wysocki, lenb, Chao Fan On 01/16/19 at 12:10am, Borislav Petkov wrote: > On Tue, Jan 15, 2019 at 05:58:34PM +0800, Kairui Song wrote: > > When efi=noruntime or efi=oldmap is used, EFI services won't be available > > in the second kernel, therefore the second kernel will not be able to get > > the ACPI RSDP address from firmware by calling EFI services and won't > > boot. Previously we are expecting the user to set the acpi_rsdp=<addr> > > on kernel command line for second kernel as there was no way to pass RSDP > > address to second kernel. > > > > After commit e6e094e053af ('x86/acpi, x86/boot: Take RSDP address from > > boot params if available'), now it's possible to set an acpi_rsdp_addr > > parameter in the boot_params passed to second kernel, this commit make > > use of it, detect and set the RSDP address when it's required for second > > kernel to boot. > > > > Tested with an EFI enabled KVM VM with efi=noruntime. > > > > Suggested-by: Dave Young <dyoung@redhat.com> > > Signed-off-by: Kairui Song <kasong@redhat.com> > > --- > > arch/x86/kernel/kexec-bzimage64.c | 21 +++++++++++++++++++++ > > drivers/acpi/acpica/tbxfroot.c | 3 +-- > > include/acpi/acpixf.h | 2 +- > > 3 files changed, 23 insertions(+), 3 deletions(-) > > > > diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c > > index 53917a3ebf94..0a90dcbd041f 100644 > > --- a/arch/x86/kernel/kexec-bzimage64.c > > +++ b/arch/x86/kernel/kexec-bzimage64.c > > @@ -20,6 +20,7 @@ > > #include <linux/mm.h> > > #include <linux/efi.h> > > #include <linux/verification.h> > > +#include <linux/acpi.h> > > > > #include <asm/bootparam.h> > > #include <asm/setup.h> > > @@ -255,8 +256,28 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params, > > /* Setup EFI state */ > > setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz, > > efi_setup_data_offset); > > + > > +#ifdef CONFIG_ACPI > > + /* Setup ACPI RSDP pointer in case EFI is not available in second kernel */ > > + if (!acpi_disabled && (!efi_enabled(EFI_RUNTIME_SERVICES) || efi_enabled(EFI_OLD_MEMMAP))) { > > + /* Copied from acpi_os_get_root_pointer accordingly */ > > + params->acpi_rsdp_addr = boot_params.acpi_rsdp_addr; > > + if (!params->acpi_rsdp_addr) { > > + if (efi_enabled(EFI_CONFIG_TABLES)) { > > + if (efi.acpi20 != EFI_INVALID_TABLE_ADDR) > > + params->acpi_rsdp_addr = efi.acpi20; > > + else if (efi.acpi != EFI_INVALID_TABLE_ADDR) > > + params->acpi_rsdp_addr = efi.acpi; > > + } else if (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) { > > + acpi_find_root_pointer(¶ms->acpi_rsdp_addr); > > + } > > + } > > + if (!params->acpi_rsdp_addr) > > + pr_warn("RSDP is not available for second kernel\n"); > > + } > > #endif > > Amazing the amount of ACPI RDSP parsing and fiddling patches flying > around these days... > > In any case, this needs to be synchronized with: > > https://lkml.kernel.org/r/20190107032243.25324-1-fanc.fnst@cn.fujitsu.com > > and checked whether the above can be used instead of sprinkling of ACPI > parsing code left and right. Both Baoquan and Chao are cced for comments. The above KASLR patches seems some early code to parsing acpi, but I think in this patch just call acpi function to get the root pointer no need to add the duplicate logic of if/else/else if. Kairui, do you have any reason for the checking? Is there some simple acpi function to just return the root pointer? > > Thx. > > -- > Regards/Gruss, > Boris. > > Good mailing practices for 400: avoid top-posting and trim the reply. Thanks Dave ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map 2019-01-16 3:32 ` Dave Young @ 2019-01-16 5:09 ` Kairui Song 2019-01-16 6:51 ` Dave Young 0 siblings, 1 reply; 25+ messages in thread From: Kairui Song @ 2019-01-16 5:09 UTC (permalink / raw) To: Dave Young Cc: Borislav Petkov, linux-kernel, tglx, mingo, hpa, x86, Baoquan He, kexec, akpm, robert.moore, erik.schmauss, rafael.j.wysocki, Len Brown, Chao Fan On Wed, Jan 16, 2019 at 11:32 AM Dave Young <dyoung@redhat.com> wrote: > > On 01/16/19 at 12:10am, Borislav Petkov wrote: > > On Tue, Jan 15, 2019 at 05:58:34PM +0800, Kairui Song wrote: > > > When efi=noruntime or efi=oldmap is used, EFI services won't be available > > > in the second kernel, therefore the second kernel will not be able to get > > > the ACPI RSDP address from firmware by calling EFI services and won't > > > boot. Previously we are expecting the user to set the acpi_rsdp=<addr> > > > on kernel command line for second kernel as there was no way to pass RSDP > > > address to second kernel. > > > > > > After commit e6e094e053af ('x86/acpi, x86/boot: Take RSDP address from > > > boot params if available'), now it's possible to set an acpi_rsdp_addr > > > parameter in the boot_params passed to second kernel, this commit make > > > use of it, detect and set the RSDP address when it's required for second > > > kernel to boot. > > > > > > Tested with an EFI enabled KVM VM with efi=noruntime. > > > > > > Suggested-by: Dave Young <dyoung@redhat.com> > > > Signed-off-by: Kairui Song <kasong@redhat.com> > > > --- > > > arch/x86/kernel/kexec-bzimage64.c | 21 +++++++++++++++++++++ > > > drivers/acpi/acpica/tbxfroot.c | 3 +-- > > > include/acpi/acpixf.h | 2 +- > > > 3 files changed, 23 insertions(+), 3 deletions(-) > > > > > > diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c > > > index 53917a3ebf94..0a90dcbd041f 100644 > > > --- a/arch/x86/kernel/kexec-bzimage64.c > > > +++ b/arch/x86/kernel/kexec-bzimage64.c > > > @@ -20,6 +20,7 @@ > > > #include <linux/mm.h> > > > #include <linux/efi.h> > > > #include <linux/verification.h> > > > +#include <linux/acpi.h> > > > > > > #include <asm/bootparam.h> > > > #include <asm/setup.h> > > > @@ -255,8 +256,28 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params, > > > /* Setup EFI state */ > > > setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz, > > > efi_setup_data_offset); > > > + > > > +#ifdef CONFIG_ACPI > > > + /* Setup ACPI RSDP pointer in case EFI is not available in second kernel */ > > > + if (!acpi_disabled && (!efi_enabled(EFI_RUNTIME_SERVICES) || efi_enabled(EFI_OLD_MEMMAP))) { > > > + /* Copied from acpi_os_get_root_pointer accordingly */ > > > + params->acpi_rsdp_addr = boot_params.acpi_rsdp_addr; > > > + if (!params->acpi_rsdp_addr) { > > > + if (efi_enabled(EFI_CONFIG_TABLES)) { > > > + if (efi.acpi20 != EFI_INVALID_TABLE_ADDR) > > > + params->acpi_rsdp_addr = efi.acpi20; > > > + else if (efi.acpi != EFI_INVALID_TABLE_ADDR) > > > + params->acpi_rsdp_addr = efi.acpi; > > > + } else if (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) { > > > + acpi_find_root_pointer(¶ms->acpi_rsdp_addr); > > > + } > > > + } > > > + if (!params->acpi_rsdp_addr) > > > + pr_warn("RSDP is not available for second kernel\n"); > > > + } > > > #endif > > > > Amazing the amount of ACPI RDSP parsing and fiddling patches flying > > around these days... > > > > In any case, this needs to be synchronized with: > > > > https://lkml.kernel.org/r/20190107032243.25324-1-fanc.fnst@cn.fujitsu.com > > > > and checked whether the above can be used instead of sprinkling of ACPI > > parsing code left and right. > > Both Baoquan and Chao are cced for comments. > The above KASLR patches seems some early code to parsing acpi, but I think in this > patch just call acpi function to get the root pointer no need to add the > duplicate logic of if/else/else if. > > Kairui, do you have any reason for the checking? Is there some simple > acpi function to just return the root pointer? Hi, I'm afraid that would require moving multiple structure and function out of .init, acpi_os_get_root_pointer is an ideal function to do the job, but it's in .init and (on x86) it will call x86_init.acpi.get_root_pointer (by calling acpi_arch_get_root_pointer) which will also be freed after init, unless I change the x86_init, move they out of .init which is not a good idea. Maybe I could split acpi_os_get_root_pointer into two, one gets freed after init, one for later use. But the "acpi_rsdp_addr" trick only works for x86, and this would change more common acpi driver code so not sure if a good idea. > > > > > Thx. > > > > -- > > Regards/Gruss, > > Boris. > > > > Good mailing practices for 400: avoid top-posting and trim the reply. > > Thanks > Dave -- Best Regards, Kairui Song ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map 2019-01-16 5:09 ` Kairui Song @ 2019-01-16 6:51 ` Dave Young 2019-01-16 22:24 ` Rafael J. Wysocki 0 siblings, 1 reply; 25+ messages in thread From: Dave Young @ 2019-01-16 6:51 UTC (permalink / raw) To: Kairui Song Cc: Borislav Petkov, linux-kernel, tglx, mingo, hpa, x86, Baoquan He, kexec, akpm, robert.moore, erik.schmauss, rafael.j.wysocki, Len Brown, Chao Fan On 01/16/19 at 01:09pm, Kairui Song wrote: > On Wed, Jan 16, 2019 at 11:32 AM Dave Young <dyoung@redhat.com> wrote: > > > > On 01/16/19 at 12:10am, Borislav Petkov wrote: > > > On Tue, Jan 15, 2019 at 05:58:34PM +0800, Kairui Song wrote: > > > > When efi=noruntime or efi=oldmap is used, EFI services won't be available > > > > in the second kernel, therefore the second kernel will not be able to get > > > > the ACPI RSDP address from firmware by calling EFI services and won't > > > > boot. Previously we are expecting the user to set the acpi_rsdp=<addr> > > > > on kernel command line for second kernel as there was no way to pass RSDP > > > > address to second kernel. > > > > > > > > After commit e6e094e053af ('x86/acpi, x86/boot: Take RSDP address from > > > > boot params if available'), now it's possible to set an acpi_rsdp_addr > > > > parameter in the boot_params passed to second kernel, this commit make > > > > use of it, detect and set the RSDP address when it's required for second > > > > kernel to boot. > > > > > > > > Tested with an EFI enabled KVM VM with efi=noruntime. > > > > > > > > Suggested-by: Dave Young <dyoung@redhat.com> > > > > Signed-off-by: Kairui Song <kasong@redhat.com> > > > > --- > > > > arch/x86/kernel/kexec-bzimage64.c | 21 +++++++++++++++++++++ > > > > drivers/acpi/acpica/tbxfroot.c | 3 +-- > > > > include/acpi/acpixf.h | 2 +- > > > > 3 files changed, 23 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c > > > > index 53917a3ebf94..0a90dcbd041f 100644 > > > > --- a/arch/x86/kernel/kexec-bzimage64.c > > > > +++ b/arch/x86/kernel/kexec-bzimage64.c > > > > @@ -20,6 +20,7 @@ > > > > #include <linux/mm.h> > > > > #include <linux/efi.h> > > > > #include <linux/verification.h> > > > > +#include <linux/acpi.h> > > > > > > > > #include <asm/bootparam.h> > > > > #include <asm/setup.h> > > > > @@ -255,8 +256,28 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params, > > > > /* Setup EFI state */ > > > > setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz, > > > > efi_setup_data_offset); > > > > + > > > > +#ifdef CONFIG_ACPI > > > > + /* Setup ACPI RSDP pointer in case EFI is not available in second kernel */ > > > > + if (!acpi_disabled && (!efi_enabled(EFI_RUNTIME_SERVICES) || efi_enabled(EFI_OLD_MEMMAP))) { > > > > + /* Copied from acpi_os_get_root_pointer accordingly */ > > > > + params->acpi_rsdp_addr = boot_params.acpi_rsdp_addr; > > > > + if (!params->acpi_rsdp_addr) { > > > > + if (efi_enabled(EFI_CONFIG_TABLES)) { > > > > + if (efi.acpi20 != EFI_INVALID_TABLE_ADDR) > > > > + params->acpi_rsdp_addr = efi.acpi20; > > > > + else if (efi.acpi != EFI_INVALID_TABLE_ADDR) > > > > + params->acpi_rsdp_addr = efi.acpi; > > > > + } else if (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) { > > > > + acpi_find_root_pointer(¶ms->acpi_rsdp_addr); > > > > + } > > > > + } > > > > + if (!params->acpi_rsdp_addr) > > > > + pr_warn("RSDP is not available for second kernel\n"); > > > > + } > > > > #endif > > > > > > Amazing the amount of ACPI RDSP parsing and fiddling patches flying > > > around these days... > > > > > > In any case, this needs to be synchronized with: > > > > > > https://lkml.kernel.org/r/20190107032243.25324-1-fanc.fnst@cn.fujitsu.com > > > > > > and checked whether the above can be used instead of sprinkling of ACPI > > > parsing code left and right. > > > > Both Baoquan and Chao are cced for comments. > > The above KASLR patches seems some early code to parsing acpi, but I think in this > > patch just call acpi function to get the root pointer no need to add the > > duplicate logic of if/else/else if. > > > > Kairui, do you have any reason for the checking? Is there some simple > > acpi function to just return the root pointer? > > Hi, I'm afraid that would require moving multiple structure and > function out of .init, > acpi_os_get_root_pointer is an ideal function to do the job, but it's > in .init and (on x86) it will call x86_init.acpi.get_root_pointer (by > calling acpi_arch_get_root_pointer) which will also be freed after > init, unless I change the x86_init, move they out of .init which is > not a good idea. > > Maybe I could split acpi_os_get_root_pointer into two, one gets freed > after init, one for later use. But the "acpi_rsdp_addr" trick only > works for x86, and this would change more common acpi driver code so > not sure if a good idea. Can the acpi root pointer be saved after initialized? If that is good then a new function to get it would be easier. But need opinion from acpi people. Thanks Dave ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map 2019-01-16 6:51 ` Dave Young @ 2019-01-16 22:24 ` Rafael J. Wysocki 0 siblings, 0 replies; 25+ messages in thread From: Rafael J. Wysocki @ 2019-01-16 22:24 UTC (permalink / raw) To: Dave Young, Kairui Song Cc: Borislav Petkov, linux-kernel, tglx, mingo, hpa, x86, Baoquan He, kexec, akpm, robert.moore, erik.schmauss, Len Brown, Chao Fan On 1/16/2019 7:51 AM, Dave Young wrote: > On 01/16/19 at 01:09pm, Kairui Song wrote: >> On Wed, Jan 16, 2019 at 11:32 AM Dave Young <dyoung@redhat.com> wrote: >>> On 01/16/19 at 12:10am, Borislav Petkov wrote: >>>> On Tue, Jan 15, 2019 at 05:58:34PM +0800, Kairui Song wrote: >>>>> When efi=noruntime or efi=oldmap is used, EFI services won't be available >>>>> in the second kernel, therefore the second kernel will not be able to get >>>>> the ACPI RSDP address from firmware by calling EFI services and won't >>>>> boot. Previously we are expecting the user to set the acpi_rsdp=<addr> >>>>> on kernel command line for second kernel as there was no way to pass RSDP >>>>> address to second kernel. >>>>> >>>>> After commit e6e094e053af ('x86/acpi, x86/boot: Take RSDP address from >>>>> boot params if available'), now it's possible to set an acpi_rsdp_addr >>>>> parameter in the boot_params passed to second kernel, this commit make >>>>> use of it, detect and set the RSDP address when it's required for second >>>>> kernel to boot. >>>>> >>>>> Tested with an EFI enabled KVM VM with efi=noruntime. >>>>> >>>>> Suggested-by: Dave Young <dyoung@redhat.com> >>>>> Signed-off-by: Kairui Song <kasong@redhat.com> >>>>> --- >>>>> arch/x86/kernel/kexec-bzimage64.c | 21 +++++++++++++++++++++ >>>>> drivers/acpi/acpica/tbxfroot.c | 3 +-- >>>>> include/acpi/acpixf.h | 2 +- >>>>> 3 files changed, 23 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c >>>>> index 53917a3ebf94..0a90dcbd041f 100644 >>>>> --- a/arch/x86/kernel/kexec-bzimage64.c >>>>> +++ b/arch/x86/kernel/kexec-bzimage64.c >>>>> @@ -20,6 +20,7 @@ >>>>> #include <linux/mm.h> >>>>> #include <linux/efi.h> >>>>> #include <linux/verification.h> >>>>> +#include <linux/acpi.h> >>>>> >>>>> #include <asm/bootparam.h> >>>>> #include <asm/setup.h> >>>>> @@ -255,8 +256,28 @@ setup_boot_parameters(struct kimage *image, struct boot_params *params, >>>>> /* Setup EFI state */ >>>>> setup_efi_state(params, params_load_addr, efi_map_offset, efi_map_sz, >>>>> efi_setup_data_offset); >>>>> + >>>>> +#ifdef CONFIG_ACPI >>>>> + /* Setup ACPI RSDP pointer in case EFI is not available in second kernel */ >>>>> + if (!acpi_disabled && (!efi_enabled(EFI_RUNTIME_SERVICES) || efi_enabled(EFI_OLD_MEMMAP))) { >>>>> + /* Copied from acpi_os_get_root_pointer accordingly */ >>>>> + params->acpi_rsdp_addr = boot_params.acpi_rsdp_addr; >>>>> + if (!params->acpi_rsdp_addr) { >>>>> + if (efi_enabled(EFI_CONFIG_TABLES)) { >>>>> + if (efi.acpi20 != EFI_INVALID_TABLE_ADDR) >>>>> + params->acpi_rsdp_addr = efi.acpi20; >>>>> + else if (efi.acpi != EFI_INVALID_TABLE_ADDR) >>>>> + params->acpi_rsdp_addr = efi.acpi; >>>>> + } else if (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) { >>>>> + acpi_find_root_pointer(¶ms->acpi_rsdp_addr); >>>>> + } >>>>> + } >>>>> + if (!params->acpi_rsdp_addr) >>>>> + pr_warn("RSDP is not available for second kernel\n"); >>>>> + } >>>>> #endif >>>> Amazing the amount of ACPI RDSP parsing and fiddling patches flying >>>> around these days... >>>> >>>> In any case, this needs to be synchronized with: >>>> >>>> https://lkml.kernel.org/r/20190107032243.25324-1-fanc.fnst@cn.fujitsu.com >>>> >>>> and checked whether the above can be used instead of sprinkling of ACPI >>>> parsing code left and right. >>> Both Baoquan and Chao are cced for comments. >>> The above KASLR patches seems some early code to parsing acpi, but I think in this >>> patch just call acpi function to get the root pointer no need to add the >>> duplicate logic of if/else/else if. >>> >>> Kairui, do you have any reason for the checking? Is there some simple >>> acpi function to just return the root pointer? >> Hi, I'm afraid that would require moving multiple structure and >> function out of .init, >> acpi_os_get_root_pointer is an ideal function to do the job, but it's >> in .init and (on x86) it will call x86_init.acpi.get_root_pointer (by >> calling acpi_arch_get_root_pointer) which will also be freed after >> init, unless I change the x86_init, move they out of .init which is >> not a good idea. >> >> Maybe I could split acpi_os_get_root_pointer into two, one gets freed >> after init, one for later use. But the "acpi_rsdp_addr" trick only >> works for x86, and this would change more common acpi driver code so >> not sure if a good idea. > Can the acpi root pointer be saved after initialized? If that is good > then a new function to get it would be easier. But need opinion from > acpi people. For that, please repost the series with a CC to linux-acpi@vger.kernel.org. Thanks! ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map 2019-01-15 23:10 ` Borislav Petkov 2019-01-16 3:32 ` Dave Young @ 2019-01-16 7:08 ` Kairui Song 2019-01-16 9:46 ` Borislav Petkov 1 sibling, 1 reply; 25+ messages in thread From: Kairui Song @ 2019-01-16 7:08 UTC (permalink / raw) To: Borislav Petkov Cc: linux-kernel, tglx, mingo, hpa, x86, Dave Young, Baoquan He, kexec, akpm, robert.moore, erik.schmauss, rafael.j.wysocki, Len Brown, Chao Fan On Wed, Jan 16, 2019 at 7:10 AM Borislav Petkov <bp@alien8.de> wrote: > > +#ifdef CONFIG_ACPI > > + /* Setup ACPI RSDP pointer in case EFI is not available in second kernel */ > > + if (!acpi_disabled && (!efi_enabled(EFI_RUNTIME_SERVICES) || efi_enabled(EFI_OLD_MEMMAP))) { > > + /* Copied from acpi_os_get_root_pointer accordingly */ > > + params->acpi_rsdp_addr = boot_params.acpi_rsdp_addr; > > + if (!params->acpi_rsdp_addr) { > > + if (efi_enabled(EFI_CONFIG_TABLES)) { > > + if (efi.acpi20 != EFI_INVALID_TABLE_ADDR) > > + params->acpi_rsdp_addr = efi.acpi20; > > + else if (efi.acpi != EFI_INVALID_TABLE_ADDR) > > + params->acpi_rsdp_addr = efi.acpi; > > + } else if (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) { > > + acpi_find_root_pointer(¶ms->acpi_rsdp_addr); > > + } > > + } > > + if (!params->acpi_rsdp_addr) > > + pr_warn("RSDP is not available for second kernel\n"); > > + } > > #endif > > Amazing the amount of ACPI RDSP parsing and fiddling patches flying > around these days... > > In any case, this needs to be synchronized with: > > https://lkml.kernel.org/r/20190107032243.25324-1-fanc.fnst@cn.fujitsu.com > > and checked whether the above can be used instead of sprinkling of ACPI > parsing code left and right. > > Thx. Hi thanks for the suggestion. I didn't see a way to reuse things in that patch series, situation is different, in that patch it needs to get RSDP in very early boot stage so it did everything from scratch, in this patch kexec_file_load need to get RSDP too, but everything is well setup so things are a lot easier, just read from current boot_prams, efi and fallback to acpi_find_root_pointer should be good. And that patch series also need to consider boot_params.acpi_rsdp_addr value, or it won't work if the system is rebooted with kexec, efi is disabled and acpi_rsdp is provided by boot_params (Xen PVH guests also need to get acpi_rsdp from boot_params according to commit message of ae7e1238e68f2a472a125673ab506d49158c1889). Will add some comment and discuss. > > -- > Regards/Gruss, > Boris. > > Good mailing practices for 400: avoid top-posting and trim the reply. -- Best Regards, Kairui Song ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map 2019-01-16 7:08 ` Kairui Song @ 2019-01-16 9:46 ` Borislav Petkov 2019-01-17 7:41 ` Kairui Song 0 siblings, 1 reply; 25+ messages in thread From: Borislav Petkov @ 2019-01-16 9:46 UTC (permalink / raw) To: Kairui Song Cc: linux-kernel, tglx, mingo, hpa, x86, Dave Young, Baoquan He, kexec, akpm, robert.moore, erik.schmauss, rafael.j.wysocki, Len Brown, Chao Fan On Wed, Jan 16, 2019 at 03:08:42PM +0800, Kairui Song wrote: > I didn't see a way to reuse things in that patch series, situation is > different, in that patch it needs to get RSDP in very early boot stage > so it did everything from scratch, in this patch kexec_file_load need > to get RSDP too, but everything is well setup so things are a lot > easier, just read from current boot_prams, efi and fallback to > acpi_find_root_pointer should be good. No no. Early code should find out that venerable RSDP thing once and will save it somewhere for further use. No gazillion parsings of it. Just once and share it with the rest of the code that needs it. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map 2019-01-16 9:46 ` Borislav Petkov @ 2019-01-17 7:41 ` Kairui Song 2019-01-17 7:49 ` Chao Fan ` (2 more replies) 0 siblings, 3 replies; 25+ messages in thread From: Kairui Song @ 2019-01-17 7:41 UTC (permalink / raw) To: Borislav Petkov Cc: linux-kernel, tglx, mingo, hpa, x86, Dave Young, Baoquan He, kexec, akpm, robert.moore, erik.schmauss, rafael.j.wysocki, Len Brown, Chao Fan On Wed, Jan 16, 2019 at 5:46 PM Borislav Petkov <bp@alien8.de> wrote: > > On Wed, Jan 16, 2019 at 03:08:42PM +0800, Kairui Song wrote: > > I didn't see a way to reuse things in that patch series, situation is > > different, in that patch it needs to get RSDP in very early boot stage > > so it did everything from scratch, in this patch kexec_file_load need > > to get RSDP too, but everything is well setup so things are a lot > > easier, just read from current boot_prams, efi and fallback to > > acpi_find_root_pointer should be good. > > No no. Early code should find out that venerable RSDP thing once and > will save it somewhere for further use. No gazillion parsings of it. > Just once and share it with the rest of the code that needs it. > How about we refill the boot_params.acpi_rsdp_addr if it is not valid in early code, so it could be used as a reliable RSDP address source? That should make things easier. But if early code should parse it and store it should be done in Chao's patch, or I can post another patch to do it if Chao's patch is merged. For now I think good to have something like this in this patch series to always keep storing acpi_rsdp in late code, acpi_os_get_root_pointer_late (maybe comeup with a better name later) could be used anytime to get RSDP and no extra parsing: --- a/drivers/acpi/osl.c +++ b/drivers/acpi/osl.c @@ -180,8 +180,8 @@ void acpi_os_vprintf(const char *fmt, va_list args) #endif } -#ifdef CONFIG_KEXEC static unsigned long acpi_rsdp; +#ifdef CONFIG_KEXEC static int __init setup_acpi_rsdp(char *arg) { return kstrtoul(arg, 16, &acpi_rsdp); @@ -189,28 +189,38 @@ static int __init setup_acpi_rsdp(char *arg) early_param("acpi_rsdp", setup_acpi_rsdp); #endif +acpi_physical_address acpi_os_get_root_pointer_late(void) { + return acpi_rsdp; +} + acpi_physical_address __init acpi_os_get_root_pointer(void) { acpi_physical_address pa; -#ifdef CONFIG_KEXEC if (acpi_rsdp) return acpi_rsdp; -#endif + pa = acpi_arch_get_root_pointer(); - if (pa) + if (pa) { + acpi_rsdp = pa; return pa; + } if (efi_enabled(EFI_CONFIG_TABLES)) { - if (efi.acpi20 != EFI_INVALID_TABLE_ADDR) + if (efi.acpi20 != EFI_INVALID_TABLE_ADDR) { + acpi_rsdp = efi.acpi20; return efi.acpi20; - if (efi.acpi != EFI_INVALID_TABLE_ADDR) + } + if (efi.acpi != EFI_INVALID_TABLE_ADDR) { + acpi_rsdp = efi.acpi; return efi.acpi; + } pr_err(PREFIX "System description tables not found\n"); } else if (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) { acpi_find_root_pointer(&pa); } + acpi_rsdp = pa; return pa; } > -- > Regards/Gruss, > Boris. > > Good mailing practices for 400: avoid top-posting and trim the reply. -- Best Regards, Kairui Song ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map 2019-01-17 7:41 ` Kairui Song @ 2019-01-17 7:49 ` Chao Fan 2019-01-17 8:20 ` Kairui Song 2019-01-17 8:54 ` Dave Young 2019-01-17 8:53 ` Dave Young 2019-01-18 10:26 ` Borislav Petkov 2 siblings, 2 replies; 25+ messages in thread From: Chao Fan @ 2019-01-17 7:49 UTC (permalink / raw) To: Kairui Song Cc: Borislav Petkov, linux-kernel, tglx, mingo, hpa, x86, Dave Young, Baoquan He, kexec, akpm, robert.moore, erik.schmauss, rafael.j.wysocki, Len Brown On Thu, Jan 17, 2019 at 03:41:13PM +0800, Kairui Song wrote: >On Wed, Jan 16, 2019 at 5:46 PM Borislav Petkov <bp@alien8.de> wrote: >> >> On Wed, Jan 16, 2019 at 03:08:42PM +0800, Kairui Song wrote: >> > I didn't see a way to reuse things in that patch series, situation is >> > different, in that patch it needs to get RSDP in very early boot stage >> > so it did everything from scratch, in this patch kexec_file_load need >> > to get RSDP too, but everything is well setup so things are a lot >> > easier, just read from current boot_prams, efi and fallback to >> > acpi_find_root_pointer should be good. >> >> No no. Early code should find out that venerable RSDP thing once and >> will save it somewhere for further use. No gazillion parsings of it. >> Just once and share it with the rest of the code that needs it. >> > >How about we refill the boot_params.acpi_rsdp_addr if it is not valid >in early code, so it could be used as a reliable RSDP address source? >That should make things easier. I think it's OK. Try to read it, if get RSDP, use it. If not, search in EFI/BIOS/... and refill the RSDP to boot_params.acpi_rsdp_addr. By the way, I search kernel code, I didn't find other code fill and use it, only you(KEXEC) are trying to fill it. If I miss something, please let me know. Thanks, Chao Fan > >But if early code should parse it and store it should be done in >Chao's patch, or I can post another patch to do it if Chao's patch is >merged. > >For now I think good to have something like this in this patch series >to always keep storing acpi_rsdp in late code, >acpi_os_get_root_pointer_late (maybe comeup with a better name later) >could be used anytime to get RSDP and no extra parsing: > >--- a/drivers/acpi/osl.c >+++ b/drivers/acpi/osl.c >@@ -180,8 +180,8 @@ void acpi_os_vprintf(const char *fmt, va_list args) > #endif > } > >-#ifdef CONFIG_KEXEC > static unsigned long acpi_rsdp; >+#ifdef CONFIG_KEXEC > static int __init setup_acpi_rsdp(char *arg) > { > return kstrtoul(arg, 16, &acpi_rsdp); >@@ -189,28 +189,38 @@ static int __init setup_acpi_rsdp(char *arg) > early_param("acpi_rsdp", setup_acpi_rsdp); > #endif > >+acpi_physical_address acpi_os_get_root_pointer_late(void) { >+ return acpi_rsdp; >+} >+ > acpi_physical_address __init acpi_os_get_root_pointer(void) > { > acpi_physical_address pa; > >-#ifdef CONFIG_KEXEC > if (acpi_rsdp) > return acpi_rsdp; >-#endif >+ > pa = acpi_arch_get_root_pointer(); >- if (pa) >+ if (pa) { >+ acpi_rsdp = pa; > return pa; >+ } > > if (efi_enabled(EFI_CONFIG_TABLES)) { >- if (efi.acpi20 != EFI_INVALID_TABLE_ADDR) >+ if (efi.acpi20 != EFI_INVALID_TABLE_ADDR) { >+ acpi_rsdp = efi.acpi20; > return efi.acpi20; >- if (efi.acpi != EFI_INVALID_TABLE_ADDR) >+ } >+ if (efi.acpi != EFI_INVALID_TABLE_ADDR) { >+ acpi_rsdp = efi.acpi; > return efi.acpi; >+ } > pr_err(PREFIX "System description tables not found\n"); > } else if (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) { > acpi_find_root_pointer(&pa); > } > > + acpi_rsdp = pa; > return pa; > } > >> -- >> Regards/Gruss, >> Boris. >> >> Good mailing practices for 400: avoid top-posting and trim the reply. >-- >Best Regards, >Kairui Song > > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map 2019-01-17 7:49 ` Chao Fan @ 2019-01-17 8:20 ` Kairui Song 2019-01-17 8:54 ` Dave Young 1 sibling, 0 replies; 25+ messages in thread From: Kairui Song @ 2019-01-17 8:20 UTC (permalink / raw) To: Chao Fan Cc: Borislav Petkov, linux-kernel, tglx, mingo, hpa, x86, Dave Young, Baoquan He, kexec, akpm, robert.moore, erik.schmauss, rafael.j.wysocki, Len Brown On Thu, Jan 17, 2019 at 3:51 PM Chao Fan <fanc.fnst@cn.fujitsu.com> wrote: > > On Thu, Jan 17, 2019 at 03:41:13PM +0800, Kairui Song wrote: > >On Wed, Jan 16, 2019 at 5:46 PM Borislav Petkov <bp@alien8.de> wrote: > >> > >> On Wed, Jan 16, 2019 at 03:08:42PM +0800, Kairui Song wrote: > >> > I didn't see a way to reuse things in that patch series, situation is > >> > different, in that patch it needs to get RSDP in very early boot stage > >> > so it did everything from scratch, in this patch kexec_file_load need > >> > to get RSDP too, but everything is well setup so things are a lot > >> > easier, just read from current boot_prams, efi and fallback to > >> > acpi_find_root_pointer should be good. > >> > >> No no. Early code should find out that venerable RSDP thing once and > >> will save it somewhere for further use. No gazillion parsings of it. > >> Just once and share it with the rest of the code that needs it. > >> > > > >How about we refill the boot_params.acpi_rsdp_addr if it is not valid > >in early code, so it could be used as a reliable RSDP address source? > >That should make things easier. > > I think it's OK. > Try to read it, if get RSDP, use it. > If not, search in EFI/BIOS/... and refill the RSDP to > boot_params.acpi_rsdp_addr. > By the way, I search kernel code, I didn't find other code fill and > use it, only you(KEXEC) are trying to fill it. > If I miss something, please let me know. Yes, kexec would read RSDP again to pass it to second kernel, and only if EFI is disabled (efi=noruntime/old_map, else second kernel will get rsdp just fine). Not sure if any other component would use it. > > Thanks, > Chao Fan > -- Best Regards, Kairui Song ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map 2019-01-17 7:49 ` Chao Fan 2019-01-17 8:20 ` Kairui Song @ 2019-01-17 8:54 ` Dave Young 1 sibling, 0 replies; 25+ messages in thread From: Dave Young @ 2019-01-17 8:54 UTC (permalink / raw) To: Chao Fan Cc: Kairui Song, Borislav Petkov, linux-kernel, tglx, mingo, hpa, x86, Baoquan He, kexec, akpm, robert.moore, erik.schmauss, rafael.j.wysocki, Len Brown, linux-acpi + linux-acpi list On 01/17/19 at 03:49pm, Chao Fan wrote: > On Thu, Jan 17, 2019 at 03:41:13PM +0800, Kairui Song wrote: > >On Wed, Jan 16, 2019 at 5:46 PM Borislav Petkov <bp@alien8.de> wrote: > >> > >> On Wed, Jan 16, 2019 at 03:08:42PM +0800, Kairui Song wrote: > >> > I didn't see a way to reuse things in that patch series, situation is > >> > different, in that patch it needs to get RSDP in very early boot stage > >> > so it did everything from scratch, in this patch kexec_file_load need > >> > to get RSDP too, but everything is well setup so things are a lot > >> > easier, just read from current boot_prams, efi and fallback to > >> > acpi_find_root_pointer should be good. > >> > >> No no. Early code should find out that venerable RSDP thing once and > >> will save it somewhere for further use. No gazillion parsings of it. > >> Just once and share it with the rest of the code that needs it. > >> > > > >How about we refill the boot_params.acpi_rsdp_addr if it is not valid > >in early code, so it could be used as a reliable RSDP address source? > >That should make things easier. > > I think it's OK. > Try to read it, if get RSDP, use it. > If not, search in EFI/BIOS/... and refill the RSDP to > boot_params.acpi_rsdp_addr. > By the way, I search kernel code, I didn't find other code fill and > use it, only you(KEXEC) are trying to fill it. > If I miss something, please let me know. > > Thanks, > Chao Fan > > > > >But if early code should parse it and store it should be done in > >Chao's patch, or I can post another patch to do it if Chao's patch is > >merged. > > > >For now I think good to have something like this in this patch series > >to always keep storing acpi_rsdp in late code, > >acpi_os_get_root_pointer_late (maybe comeup with a better name later) > >could be used anytime to get RSDP and no extra parsing: > > > >--- a/drivers/acpi/osl.c > >+++ b/drivers/acpi/osl.c > >@@ -180,8 +180,8 @@ void acpi_os_vprintf(const char *fmt, va_list args) > > #endif > > } > > > >-#ifdef CONFIG_KEXEC > > static unsigned long acpi_rsdp; > >+#ifdef CONFIG_KEXEC > > static int __init setup_acpi_rsdp(char *arg) > > { > > return kstrtoul(arg, 16, &acpi_rsdp); > >@@ -189,28 +189,38 @@ static int __init setup_acpi_rsdp(char *arg) > > early_param("acpi_rsdp", setup_acpi_rsdp); > > #endif > > > >+acpi_physical_address acpi_os_get_root_pointer_late(void) { > >+ return acpi_rsdp; > >+} > >+ > > acpi_physical_address __init acpi_os_get_root_pointer(void) > > { > > acpi_physical_address pa; > > > >-#ifdef CONFIG_KEXEC > > if (acpi_rsdp) > > return acpi_rsdp; > >-#endif > >+ > > pa = acpi_arch_get_root_pointer(); > >- if (pa) > >+ if (pa) { > >+ acpi_rsdp = pa; > > return pa; > >+ } > > > > if (efi_enabled(EFI_CONFIG_TABLES)) { > >- if (efi.acpi20 != EFI_INVALID_TABLE_ADDR) > >+ if (efi.acpi20 != EFI_INVALID_TABLE_ADDR) { > >+ acpi_rsdp = efi.acpi20; > > return efi.acpi20; > >- if (efi.acpi != EFI_INVALID_TABLE_ADDR) > >+ } > >+ if (efi.acpi != EFI_INVALID_TABLE_ADDR) { > >+ acpi_rsdp = efi.acpi; > > return efi.acpi; > >+ } > > pr_err(PREFIX "System description tables not found\n"); > > } else if (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) { > > acpi_find_root_pointer(&pa); > > } > > > > + acpi_rsdp = pa; > > return pa; > > } > > > >> -- > >> Regards/Gruss, > >> Boris. > >> > >> Good mailing practices for 400: avoid top-posting and trim the reply. > >-- > >Best Regards, > >Kairui Song > > > > > > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map 2019-01-17 7:41 ` Kairui Song 2019-01-17 7:49 ` Chao Fan @ 2019-01-17 8:53 ` Dave Young 2019-01-17 9:39 ` Rafael J. Wysocki 2019-01-18 10:26 ` Borislav Petkov 2 siblings, 1 reply; 25+ messages in thread From: Dave Young @ 2019-01-17 8:53 UTC (permalink / raw) To: Kairui Song Cc: Borislav Petkov, linux-kernel, tglx, mingo, hpa, x86, Baoquan He, kexec, akpm, robert.moore, erik.schmauss, rafael.j.wysocki, Len Brown, Chao Fan, linux-acpi Add linux-acpi list On 01/17/19 at 03:41pm, Kairui Song wrote: > On Wed, Jan 16, 2019 at 5:46 PM Borislav Petkov <bp@alien8.de> wrote: > > > > On Wed, Jan 16, 2019 at 03:08:42PM +0800, Kairui Song wrote: > > > I didn't see a way to reuse things in that patch series, situation is > > > different, in that patch it needs to get RSDP in very early boot stage > > > so it did everything from scratch, in this patch kexec_file_load need > > > to get RSDP too, but everything is well setup so things are a lot > > > easier, just read from current boot_prams, efi and fallback to > > > acpi_find_root_pointer should be good. > > > > No no. Early code should find out that venerable RSDP thing once and > > will save it somewhere for further use. No gazillion parsings of it. > > Just once and share it with the rest of the code that needs it. > > > > How about we refill the boot_params.acpi_rsdp_addr if it is not valid > in early code, so it could be used as a reliable RSDP address source? > That should make things easier. > > But if early code should parse it and store it should be done in > Chao's patch, or I can post another patch to do it if Chao's patch is > merged. > > For now I think good to have something like this in this patch series > to always keep storing acpi_rsdp in late code, > acpi_os_get_root_pointer_late (maybe comeup with a better name later) > could be used anytime to get RSDP and no extra parsing: > > --- a/drivers/acpi/osl.c > +++ b/drivers/acpi/osl.c > @@ -180,8 +180,8 @@ void acpi_os_vprintf(const char *fmt, va_list args) > #endif > } > > -#ifdef CONFIG_KEXEC > static unsigned long acpi_rsdp; > +#ifdef CONFIG_KEXEC > static int __init setup_acpi_rsdp(char *arg) > { > return kstrtoul(arg, 16, &acpi_rsdp); > @@ -189,28 +189,38 @@ static int __init setup_acpi_rsdp(char *arg) > early_param("acpi_rsdp", setup_acpi_rsdp); > #endif > > +acpi_physical_address acpi_os_get_root_pointer_late(void) { > + return acpi_rsdp; > +} > + > acpi_physical_address __init acpi_os_get_root_pointer(void) > { > acpi_physical_address pa; > > -#ifdef CONFIG_KEXEC > if (acpi_rsdp) > return acpi_rsdp; > -#endif > + > pa = acpi_arch_get_root_pointer(); > - if (pa) > + if (pa) { > + acpi_rsdp = pa; > return pa; > + } > > if (efi_enabled(EFI_CONFIG_TABLES)) { > - if (efi.acpi20 != EFI_INVALID_TABLE_ADDR) > + if (efi.acpi20 != EFI_INVALID_TABLE_ADDR) { > + acpi_rsdp = efi.acpi20; > return efi.acpi20; > - if (efi.acpi != EFI_INVALID_TABLE_ADDR) > + } > + if (efi.acpi != EFI_INVALID_TABLE_ADDR) { > + acpi_rsdp = efi.acpi; > return efi.acpi; > + } > pr_err(PREFIX "System description tables not found\n"); > } else if (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) { > acpi_find_root_pointer(&pa); > } > > + acpi_rsdp = pa; > return pa; > } > > > -- > > Regards/Gruss, > > Boris. > > > > Good mailing practices for 400: avoid top-posting and trim the reply. > -- > Best Regards, > Kairui Song ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map 2019-01-17 8:53 ` Dave Young @ 2019-01-17 9:39 ` Rafael J. Wysocki 2019-01-18 4:47 ` Kairui Song 0 siblings, 1 reply; 25+ messages in thread From: Rafael J. Wysocki @ 2019-01-17 9:39 UTC (permalink / raw) To: Dave Young Cc: Kairui Song, Borislav Petkov, Linux Kernel Mailing List, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, the arch/x86 maintainers, Baoquan He, kexec, Andrew Morton, Robert Moore, Schmauss, Erik, Rafael Wysocki, Len Brown, Chao Fan, ACPI Devel Maling List On Thu, Jan 17, 2019 at 9:53 AM Dave Young <dyoung@redhat.com> wrote: > > Add linux-acpi list Well, thanks, but please resend the patches with a CC to linux-acpi. > On 01/17/19 at 03:41pm, Kairui Song wrote: > > On Wed, Jan 16, 2019 at 5:46 PM Borislav Petkov <bp@alien8.de> wrote: > > > > > > On Wed, Jan 16, 2019 at 03:08:42PM +0800, Kairui Song wrote: > > > > I didn't see a way to reuse things in that patch series, situation is > > > > different, in that patch it needs to get RSDP in very early boot stage > > > > so it did everything from scratch, in this patch kexec_file_load need > > > > to get RSDP too, but everything is well setup so things are a lot > > > > easier, just read from current boot_prams, efi and fallback to > > > > acpi_find_root_pointer should be good. > > > > > > No no. Early code should find out that venerable RSDP thing once and > > > will save it somewhere for further use. No gazillion parsings of it. > > > Just once and share it with the rest of the code that needs it. > > > > > > > How about we refill the boot_params.acpi_rsdp_addr if it is not valid > > in early code, so it could be used as a reliable RSDP address source? > > That should make things easier. > > > > But if early code should parse it and store it should be done in > > Chao's patch, or I can post another patch to do it if Chao's patch is > > merged. > > > > For now I think good to have something like this in this patch series > > to always keep storing acpi_rsdp in late code, > > acpi_os_get_root_pointer_late (maybe comeup with a better name later) > > could be used anytime to get RSDP and no extra parsing: > > > > --- a/drivers/acpi/osl.c > > +++ b/drivers/acpi/osl.c > > @@ -180,8 +180,8 @@ void acpi_os_vprintf(const char *fmt, va_list args) > > #endif > > } > > > > -#ifdef CONFIG_KEXEC > > static unsigned long acpi_rsdp; > > +#ifdef CONFIG_KEXEC > > static int __init setup_acpi_rsdp(char *arg) > > { > > return kstrtoul(arg, 16, &acpi_rsdp); > > @@ -189,28 +189,38 @@ static int __init setup_acpi_rsdp(char *arg) > > early_param("acpi_rsdp", setup_acpi_rsdp); > > #endif > > > > +acpi_physical_address acpi_os_get_root_pointer_late(void) { > > + return acpi_rsdp; > > +} > > + > > acpi_physical_address __init acpi_os_get_root_pointer(void) > > { > > acpi_physical_address pa; > > > > -#ifdef CONFIG_KEXEC > > if (acpi_rsdp) > > return acpi_rsdp; > > -#endif > > + > > pa = acpi_arch_get_root_pointer(); > > - if (pa) > > + if (pa) { > > + acpi_rsdp = pa; > > return pa; > > + } > > > > if (efi_enabled(EFI_CONFIG_TABLES)) { > > - if (efi.acpi20 != EFI_INVALID_TABLE_ADDR) > > + if (efi.acpi20 != EFI_INVALID_TABLE_ADDR) { > > + acpi_rsdp = efi.acpi20; > > return efi.acpi20; > > - if (efi.acpi != EFI_INVALID_TABLE_ADDR) > > + } > > + if (efi.acpi != EFI_INVALID_TABLE_ADDR) { > > + acpi_rsdp = efi.acpi; > > return efi.acpi; > > + } > > pr_err(PREFIX "System description tables not found\n"); > > } else if (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) { > > acpi_find_root_pointer(&pa); > > } > > > > + acpi_rsdp = pa; > > return pa; > > } > > > > > -- > > > Regards/Gruss, > > > Boris. > > > > > > Good mailing practices for 400: avoid top-posting and trim the reply. > > -- > > Best Regards, > > Kairui Song ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map 2019-01-17 9:39 ` Rafael J. Wysocki @ 2019-01-18 4:47 ` Kairui Song 2019-01-18 9:45 ` Rafael J. Wysocki 0 siblings, 1 reply; 25+ messages in thread From: Kairui Song @ 2019-01-18 4:47 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Dave Young, Borislav Petkov, Linux Kernel Mailing List, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, the arch/x86 maintainers, Baoquan He, kexec, Andrew Morton, Robert Moore, Schmauss, Erik, Rafael Wysocki, Len Brown, Chao Fan, ACPI Devel Maling List On Thu, Jan 17, 2019 at 5:40 PM Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Thu, Jan 17, 2019 at 9:53 AM Dave Young <dyoung@redhat.com> wrote: > > > > Add linux-acpi list > > Well, thanks, but please resend the patches with a CC to linux-acpi. > Hi, sure will do. Any thought on adding an acpi_os_get_root_pointer_late and store rsdp pointer as mentioned? Will updat the patch and post V2, and cc linux-acpi as well later. > > On 01/17/19 at 03:41pm, Kairui Song wrote: > > > On Wed, Jan 16, 2019 at 5:46 PM Borislav Petkov <bp@alien8.de> wrote: > > > > > > > > On Wed, Jan 16, 2019 at 03:08:42PM +0800, Kairui Song wrote: > > > > > I didn't see a way to reuse things in that patch series, situation is > > > > > different, in that patch it needs to get RSDP in very early boot stage > > > > > so it did everything from scratch, in this patch kexec_file_load need > > > > > to get RSDP too, but everything is well setup so things are a lot > > > > > easier, just read from current boot_prams, efi and fallback to > > > > > acpi_find_root_pointer should be good. > > > > > > > > No no. Early code should find out that venerable RSDP thing once and > > > > will save it somewhere for further use. No gazillion parsings of it. > > > > Just once and share it with the rest of the code that needs it. > > > > > > > > > > How about we refill the boot_params.acpi_rsdp_addr if it is not valid > > > in early code, so it could be used as a reliable RSDP address source? > > > That should make things easier. > > > > > > But if early code should parse it and store it should be done in > > > Chao's patch, or I can post another patch to do it if Chao's patch is > > > merged. > > > > > > For now I think good to have something like this in this patch series > > > to always keep storing acpi_rsdp in late code, > > > acpi_os_get_root_pointer_late (maybe comeup with a better name later) > > > could be used anytime to get RSDP and no extra parsing: > > > > > > --- a/drivers/acpi/osl.c > > > +++ b/drivers/acpi/osl.c > > > @@ -180,8 +180,8 @@ void acpi_os_vprintf(const char *fmt, va_list args) > > > #endif > > > } > > > > > > -#ifdef CONFIG_KEXEC > > > static unsigned long acpi_rsdp; > > > +#ifdef CONFIG_KEXEC > > > static int __init setup_acpi_rsdp(char *arg) > > > { > > > return kstrtoul(arg, 16, &acpi_rsdp); > > > @@ -189,28 +189,38 @@ static int __init setup_acpi_rsdp(char *arg) > > > early_param("acpi_rsdp", setup_acpi_rsdp); > > > #endif > > > > > > +acpi_physical_address acpi_os_get_root_pointer_late(void) { > > > + return acpi_rsdp; > > > +} > > > + > > > acpi_physical_address __init acpi_os_get_root_pointer(void) > > > { > > > acpi_physical_address pa; > > > > > > -#ifdef CONFIG_KEXEC > > > if (acpi_rsdp) > > > return acpi_rsdp; > > > -#endif > > > + > > > pa = acpi_arch_get_root_pointer(); > > > - if (pa) > > > + if (pa) { > > > + acpi_rsdp = pa; > > > return pa; > > > + } > > > > > > if (efi_enabled(EFI_CONFIG_TABLES)) { > > > - if (efi.acpi20 != EFI_INVALID_TABLE_ADDR) > > > + if (efi.acpi20 != EFI_INVALID_TABLE_ADDR) { > > > + acpi_rsdp = efi.acpi20; > > > return efi.acpi20; > > > - if (efi.acpi != EFI_INVALID_TABLE_ADDR) > > > + } > > > + if (efi.acpi != EFI_INVALID_TABLE_ADDR) { > > > + acpi_rsdp = efi.acpi; > > > return efi.acpi; > > > + } > > > pr_err(PREFIX "System description tables not found\n"); > > > } else if (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) { > > > acpi_find_root_pointer(&pa); > > > } > > > > > > + acpi_rsdp = pa; > > > return pa; > > > } > > > > > > > -- > > > > Regards/Gruss, > > > > Boris. > > > > > > > > Good mailing practices for 400: avoid top-posting and trim the reply. > > > -- > > > Best Regards, > > > Kairui Song -- Best Regards, Kairui Song ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map 2019-01-18 4:47 ` Kairui Song @ 2019-01-18 9:45 ` Rafael J. Wysocki 0 siblings, 0 replies; 25+ messages in thread From: Rafael J. Wysocki @ 2019-01-18 9:45 UTC (permalink / raw) To: Kairui Song Cc: Rafael J. Wysocki, Dave Young, Borislav Petkov, Linux Kernel Mailing List, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, the arch/x86 maintainers, Baoquan He, kexec, Andrew Morton, Robert Moore, Schmauss, Erik, Rafael Wysocki, Len Brown, Chao Fan, ACPI Devel Maling List On Fri, Jan 18, 2019 at 5:47 AM Kairui Song <kasong@redhat.com> wrote: > > On Thu, Jan 17, 2019 at 5:40 PM Rafael J. Wysocki <rafael@kernel.org> wrote: > > > > On Thu, Jan 17, 2019 at 9:53 AM Dave Young <dyoung@redhat.com> wrote: > > > > > > Add linux-acpi list > > > > Well, thanks, but please resend the patches with a CC to linux-acpi. > > > > Hi, sure will do. > Any thought on adding an acpi_os_get_root_pointer_late and store rsdp > pointer as mentioned? It is a good idea in general IMO, but all depends on how the code changes will look like. > Will updat the patch and post V2, and cc linux-acpi as well later. Thanks! ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map 2019-01-17 7:41 ` Kairui Song 2019-01-17 7:49 ` Chao Fan 2019-01-17 8:53 ` Dave Young @ 2019-01-18 10:26 ` Borislav Petkov 2019-01-21 1:18 ` Chao Fan 2 siblings, 1 reply; 25+ messages in thread From: Borislav Petkov @ 2019-01-18 10:26 UTC (permalink / raw) To: Kairui Song Cc: linux-kernel, tglx, mingo, hpa, x86, Dave Young, Baoquan He, kexec, akpm, robert.moore, erik.schmauss, rafael.j.wysocki, Len Brown, Chao Fan On Thu, Jan 17, 2019 at 03:41:13PM +0800, Kairui Song wrote: > How about we refill the boot_params.acpi_rsdp_addr if it is not valid > in early code, so it could be used as a reliable RSDP address source? > That should make things easier. > > But if early code should parse it and store it should be done in > Chao's patch, or I can post another patch to do it if Chao's patch is > merged. Chao's stuff does the as early as possible parsing of RDSP. Then, it should be saved into boot_params and everything else should read it from there. Simple. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map 2019-01-18 10:26 ` Borislav Petkov @ 2019-01-21 1:18 ` Chao Fan 2019-01-21 8:29 ` Borislav Petkov 0 siblings, 1 reply; 25+ messages in thread From: Chao Fan @ 2019-01-21 1:18 UTC (permalink / raw) To: Borislav Petkov Cc: Kairui Song, linux-kernel, tglx, mingo, hpa, x86, Dave Young, Baoquan He, kexec, akpm, robert.moore, erik.schmauss, rafael.j.wysocki, Len Brown On Fri, Jan 18, 2019 at 11:26:36AM +0100, Borislav Petkov wrote: >On Thu, Jan 17, 2019 at 03:41:13PM +0800, Kairui Song wrote: >> How about we refill the boot_params.acpi_rsdp_addr if it is not valid >> in early code, so it could be used as a reliable RSDP address source? >> That should make things easier. >> >> But if early code should parse it and store it should be done in >> Chao's patch, or I can post another patch to do it if Chao's patch is >> merged. > >Chao's stuff does the as early as possible parsing of RDSP. Then, it >should be saved into boot_params and everything else should read it from >there. Simple. Hi Boris, So I have changed as this method and put in my mail thread, you may not notice, so I put here for my function if I need to fill the boot_parameters: static inline acpi_physical_address get_boot_params_rsdp(void) { return boot_params->acpi_rsdp_addr; } static acpi_physical_address get_rsdp_addr(void) { bool boot_params_rsdp_exist; acpi_physical_address pa; pa = get_acpi_rsdp(); if (!pa) pa = get_boot_params_rsdp(); if (!pa) { pa = efi_get_rsdp_addr(); boot_params_rsdp_exist = false; } else boot_params_rsdp_exist = true; if (!pa) pa = bios_get_rsdp_addr(); if (pa && !boot_params_rsdp_exist) boot_params.acpi_rsdp_addr = pa; return pa; } At the same time, I notice kernel only parses it when "#ifdef CONFIG_ACPI", we should keep sync with kernel, but I think we are parsing SRAT, CONFIG_ACPI is needed sure, so I am going to update the define of EARLY_SRAT_PARSE: config EARLY_SRAT_PARSE bool "EARLY SRAT parsing" def_bool y depends on RANDOMIZE_BASE && MEMORY_HOTREMOVE && ACPI If I miss something, please let me know. Or in my PATCHSET, I don't need to fill boot_parameters, just leave the job another PATCH? Thanks, Chao Fan > >-- >Regards/Gruss, > Boris. > >Good mailing practices for 400: avoid top-posting and trim the reply. > > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map 2019-01-21 1:18 ` Chao Fan @ 2019-01-21 8:29 ` Borislav Petkov 2019-01-21 8:43 ` Chao Fan 2019-01-22 3:32 ` Chao Fan 0 siblings, 2 replies; 25+ messages in thread From: Borislav Petkov @ 2019-01-21 8:29 UTC (permalink / raw) To: Chao Fan Cc: Kairui Song, linux-kernel, tglx, mingo, hpa, x86, Dave Young, Baoquan He, kexec, akpm, robert.moore, erik.schmauss, rafael.j.wysocki, Len Brown On Mon, Jan 21, 2019 at 09:18:30AM +0800, Chao Fan wrote: > So I have changed as this method and put in my mail thread, you may not > notice, so I put here for my function if I need to fill the > boot_parameters: > > static inline acpi_physical_address get_boot_params_rsdp(void) > { > return boot_params->acpi_rsdp_addr; > } Why do you need that silly wrapper? > static acpi_physical_address get_rsdp_addr(void) > { > bool boot_params_rsdp_exist; What's that bool supposed to do? > acpi_physical_address pa; > > pa = get_acpi_rsdp(); > > if (!pa) > pa = get_boot_params_rsdp(); > > if (!pa) { > pa = efi_get_rsdp_addr(); > boot_params_rsdp_exist = false; > } > else > boot_params_rsdp_exist = true; > > if (!pa) > pa = bios_get_rsdp_addr(); > > if (pa && !boot_params_rsdp_exist) > boot_params.acpi_rsdp_addr = pa; > > return pa; > } > > At the same time, I notice kernel only parses it when > "#ifdef CONFIG_ACPI", we should keep sync with kernel, but I think > we are parsing SRAT, CONFIG_ACPI is needed sure, so I am going to > update the define of EARLY_SRAT_PARSE: > > config EARLY_SRAT_PARSE > bool "EARLY SRAT parsing" > def_bool y > depends on RANDOMIZE_BASE && MEMORY_HOTREMOVE && ACPI Actually, you don't need that anymore - make it unconditionally built-in. Because there are a bunch of users which need this and instead of complicating this config option with a bunch of dependencies, we can just as well have it always on. Thx. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map 2019-01-21 8:29 ` Borislav Petkov @ 2019-01-21 8:43 ` Chao Fan 2019-01-21 9:19 ` Borislav Petkov 2019-01-22 3:32 ` Chao Fan 1 sibling, 1 reply; 25+ messages in thread From: Chao Fan @ 2019-01-21 8:43 UTC (permalink / raw) To: Borislav Petkov Cc: Kairui Song, linux-kernel, tglx, mingo, hpa, x86, Dave Young, Baoquan He, kexec, akpm, robert.moore, erik.schmauss, rafael.j.wysocki, Len Brown On Mon, Jan 21, 2019 at 09:29:32AM +0100, Borislav Petkov wrote: >On Mon, Jan 21, 2019 at 09:18:30AM +0800, Chao Fan wrote: >> So I have changed as this method and put in my mail thread, you may not >> notice, so I put here for my function if I need to fill the >> boot_parameters: >> >> static inline acpi_physical_address get_boot_params_rsdp(void) >> { >> return boot_params->acpi_rsdp_addr; >> } > >Why do you need that silly wrapper? Will drop it. > >> static acpi_physical_address get_rsdp_addr(void) >> { >> bool boot_params_rsdp_exist; > >What's that bool supposed to do? Since I didn't see where Xen to fill the value, if boot_params->acpi_rsdp_addr is filled before my code, I just need to read it. If when I try to read it but not found, then parse RSDP and fill the RSDP address to boot_params->acpi_rsdp_addr. > >> acpi_physical_address pa; >> >> pa = get_acpi_rsdp(); >> >> if (!pa) >> pa = get_boot_params_rsdp(); >> >> if (!pa) { >> pa = efi_get_rsdp_addr(); >> boot_params_rsdp_exist = false; >> } >> else >> boot_params_rsdp_exist = true; >> >> if (!pa) >> pa = bios_get_rsdp_addr(); >> >> if (pa && !boot_params_rsdp_exist) >> boot_params.acpi_rsdp_addr = pa; >> >> return pa; >> } >> >> At the same time, I notice kernel only parses it when >> "#ifdef CONFIG_ACPI", we should keep sync with kernel, but I think >> we are parsing SRAT, CONFIG_ACPI is needed sure, so I am going to >> update the define of EARLY_SRAT_PARSE: >> >> config EARLY_SRAT_PARSE >> bool "EARLY SRAT parsing" >> def_bool y >> depends on RANDOMIZE_BASE && MEMORY_HOTREMOVE && ACPI > >Actually, you don't need that anymore - make it unconditionally >built-in. Because there are a bunch of users which need this and instead >of complicating this config option with a bunch of dependencies, we can >just as well have it always on. If I need to fill boot_params->acpi_rsdp_addr, I think we can make it unconditionally built-in. Thanks, Chao Fan > >Thx. > >-- >Regards/Gruss, > Boris. > >Good mailing practices for 400: avoid top-posting and trim the reply. > > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map 2019-01-21 8:43 ` Chao Fan @ 2019-01-21 9:19 ` Borislav Petkov 0 siblings, 0 replies; 25+ messages in thread From: Borislav Petkov @ 2019-01-21 9:19 UTC (permalink / raw) To: Chao Fan Cc: Kairui Song, linux-kernel, tglx, mingo, hpa, x86, Dave Young, Baoquan He, kexec, akpm, robert.moore, erik.schmauss, rafael.j.wysocki, Len Brown On Mon, Jan 21, 2019 at 04:43:52PM +0800, Chao Fan wrote: > Since I didn't see where Xen to fill the value, if > boot_params->acpi_rsdp_addr is filled before my code, I just need to > read it. If when I try to read it but not found, then parse RSDP and > fill the RSDP address to boot_params->acpi_rsdp_addr. And you can't simply do: if (boot_params.acpi_rsdp_addr) return; at the beginning of that function? -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map 2019-01-21 8:29 ` Borislav Petkov 2019-01-21 8:43 ` Chao Fan @ 2019-01-22 3:32 ` Chao Fan 2019-01-22 12:17 ` Borislav Petkov 1 sibling, 1 reply; 25+ messages in thread From: Chao Fan @ 2019-01-22 3:32 UTC (permalink / raw) To: Borislav Petkov Cc: Kairui Song, linux-kernel, tglx, mingo, hpa, x86, Dave Young, Baoquan He, kexec, akpm, robert.moore, erik.schmauss, rafael.j.wysocki, Len Brown On Mon, Jan 21, 2019 at 09:29:32AM +0100, Borislav Petkov wrote: >On Mon, Jan 21, 2019 at 09:18:30AM +0800, Chao Fan wrote: >> So I have changed as this method and put in my mail thread, you may not >> notice, so I put here for my function if I need to fill the >> boot_parameters: >> >> static inline acpi_physical_address get_boot_params_rsdp(void) >> { >> return boot_params->acpi_rsdp_addr; >> } > >Why do you need that silly wrapper? > >> static acpi_physical_address get_rsdp_addr(void) >> { >> bool boot_params_rsdp_exist; > >What's that bool supposed to do? > >> acpi_physical_address pa; >> >> pa = get_acpi_rsdp(); >> >> if (!pa) >> pa = get_boot_params_rsdp(); >> >> if (!pa) { >> pa = efi_get_rsdp_addr(); >> boot_params_rsdp_exist = false; >> } >> else >> boot_params_rsdp_exist = true; >> >> if (!pa) >> pa = bios_get_rsdp_addr(); >> >> if (pa && !boot_params_rsdp_exist) >> boot_params.acpi_rsdp_addr = pa; >> >> return pa; >> } >> >> At the same time, I notice kernel only parses it when >> "#ifdef CONFIG_ACPI", we should keep sync with kernel, but I think >> we are parsing SRAT, CONFIG_ACPI is needed sure, so I am going to >> update the define of EARLY_SRAT_PARSE: >> >> config EARLY_SRAT_PARSE >> bool "EARLY SRAT parsing" >> def_bool y >> depends on RANDOMIZE_BASE && MEMORY_HOTREMOVE && ACPI > >Actually, you don't need that anymore - make it unconditionally >built-in. Because there are a bunch of users which need this and instead >of complicating this config option with a bunch of dependencies, we can >just as well have it always on. According to your reply, I change it as: vmlinux-objs-y += $(obj)/acpi.o But I notice the only function call entry is in kaslr.c which needs RANDOMIZE_BASE, so do I need change it as: vmlinux-objs-$(CONFIG_RANDOMIZE_BASE) += $(obj)/acpi.o Thanks, Chao Fan > >Thx. > >-- >Regards/Gruss, > Boris. > >Good mailing practices for 400: avoid top-posting and trim the reply. > > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map 2019-01-22 3:32 ` Chao Fan @ 2019-01-22 12:17 ` Borislav Petkov 0 siblings, 0 replies; 25+ messages in thread From: Borislav Petkov @ 2019-01-22 12:17 UTC (permalink / raw) To: Chao Fan Cc: Kairui Song, linux-kernel, tglx, mingo, hpa, x86, Dave Young, Baoquan He, kexec, akpm, robert.moore, erik.schmauss, rafael.j.wysocki, Len Brown On Tue, Jan 22, 2019 at 11:32:41AM +0800, Chao Fan wrote: > But I notice the only function call entry is in kaslr.c which needs > RANDOMIZE_BASE, so do I need change it as: > vmlinux-objs-$(CONFIG_RANDOMIZE_BASE) += $(obj)/acpi.o Well, the very first patch in this thread doesn't have anything to do with CONFIG_RANDOMIZE_BASE but wants RDSP too. -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply. ^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2019-01-22 12:17 UTC | newest] Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-01-15 9:58 [PATCH 0/2] make kexec work with efi=noruntime or efi=old_map Kairui Song 2019-01-15 9:58 ` [PATCH v2 1/2] x86, kexec_file_load: Don't setup EFI info if EFI runtime is not enabled Kairui Song 2019-01-15 9:58 ` [PATCH v2 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=old_map Kairui Song 2019-01-15 23:10 ` Borislav Petkov 2019-01-16 3:32 ` Dave Young 2019-01-16 5:09 ` Kairui Song 2019-01-16 6:51 ` Dave Young 2019-01-16 22:24 ` Rafael J. Wysocki 2019-01-16 7:08 ` Kairui Song 2019-01-16 9:46 ` Borislav Petkov 2019-01-17 7:41 ` Kairui Song 2019-01-17 7:49 ` Chao Fan 2019-01-17 8:20 ` Kairui Song 2019-01-17 8:54 ` Dave Young 2019-01-17 8:53 ` Dave Young 2019-01-17 9:39 ` Rafael J. Wysocki 2019-01-18 4:47 ` Kairui Song 2019-01-18 9:45 ` Rafael J. Wysocki 2019-01-18 10:26 ` Borislav Petkov 2019-01-21 1:18 ` Chao Fan 2019-01-21 8:29 ` Borislav Petkov 2019-01-21 8:43 ` Chao Fan 2019-01-21 9:19 ` Borislav Petkov 2019-01-22 3:32 ` Chao Fan 2019-01-22 12:17 ` Borislav Petkov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).