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

[Jung-uk Kim <jkim@FreeBSD.org>]

[-- Attachment #1.1: Type: text/plain, Size: 5233 bytes --]

On 19. 1. 16., Schmauss, Erik wrote:
>> -----Original Message-----
>> From: Dave Young [mailto:dyoung@redhat.com]
>> Sent: Tuesday, January 15, 2019 12:42 AM
>> To: Kairui Song <kasong@redhat.com>
>> Cc: linux-kernel@vger.kernel.org; tglx@linutronix.de;
>> mingo@redhat.com; bp@alien8.de; hpa@zytor.com; x86@kernel.org;
>> akpm@linux-foundation.org; Moore, Robert
>> <robert.moore@intel.com>; Schmauss, Erik
>> <erik.schmauss@intel.com>; Wysocki, Rafael J
>> <rafael.j.wysocki@intel.com>; lenb@kernel.org
>> 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@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(&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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]