Re: [PATCH 2/2] x86, kexec_file_load: make it work with efi=noruntime or efi=oldmap

From: Jung-uk Kim
Date: Wed Jan 16 2019 - 17:05:32 EST


On 19. 1. 16., Schmauss, Erik wrote:
>> -----Original Message-----
>> From: Dave Young [mailto:dyoung@xxxxxxxxxx]
>> Sent: Tuesday, January 15, 2019 12:42 AM
>> To: Kairui Song <kasong@xxxxxxxxxx>
>> Cc: linux-kernel@xxxxxxxxxxxxxxx; tglx@xxxxxxxxxxxxx;
>> mingo@xxxxxxxxxx; bp@xxxxxxxxx; hpa@xxxxxxxxx; x86@xxxxxxxxxx;
>> akpm@xxxxxxxxxxxxxxxxxxxx; Moore, Robert
>> <robert.moore@xxxxxxxxx>; Schmauss, Erik
>> <erik.schmauss@xxxxxxxxx>; Wysocki, Rafael J
>> <rafael.j.wysocki@xxxxxxxxx>; lenb@xxxxxxxxxx
>> Subject: Re: [PATCH 2/2] x86, kexec_file_load: make it work with
>> efi=noruntime or efi=oldmap
>>
>> On 01/09/19 at 02:47pm, 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 a 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@xxxxxxxxxx>
>>> Signed-off-by: Kairui Song <kasong@xxxxxxxxxx>
>>> ---
>>> 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(&params-
>>> 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
>
> @JK
>
> Do you see any issues with this? Does free BSD use ACPI_INIT_FUNCTION?

No, FreeBSD doesn't use ACPI_INIT_FUNCTION.

Jung-uk Kim

>> acpi_find_root_pointer(acpi_physical_address
>>> *rsdp_address))
>>> ACPI_EXTERNAL_RETURN_STATUS(acpi_status
>>> --
>>> 2.20.1
>>>
>>
>> Kairui, thanks for the patches, did a test, it works for me.
>>
>> Seems the two patches are not in a thread, can you resend them
>> together?

Attachment: signature.asc
Description: OpenPGP digital signature