[U-Boot] [PATCH v4 1/1] efi_loader: PSCI reset and shutdown

Alexander Graf agraf at suse.de
Wed Oct 17 17:37:48 UTC 2018



On 17.10.18 19:05, Heinrich Schuchardt wrote:
> When an operating system started via bootefi tries to reset or power off
> this is done by calling the EFI runtime ResetSystem(). On most ARMv8 system
> the actual reset relies on PSCI. Depending on whether the PSCI firmware
> resides the hypervisor (EL2) or in the secure monitor (EL3) either an HVC
> or an SMC command has to be issued.
> 
> The current implementation always uses SMC. This results in crashes on
> systems where the PSCI firmware is implemented in the hypervisor, e.g.
> qemu-arm64_defconfig.
> 
> The logic to decide which call is needed based on the device tree is
> already implemented in the PSCI firmware driver. During the EFI runtime
> the device driver model is not available. But we can minimize code
> duplication by merging the EFI runtime reset and poweroff code with
> the PSCI firmware driver.
> 
> As the same HVC/SMC problem is also evident for the ARMv8 do_poweroff
> and reset_misc routines let's move them into the same code module.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> Reviewed-by: Sumit Garg <sumit.garg at linaro.org>
> Tested-by: Sumit Garg <sumit.garg at linaro.org>
> ---
> Travis reporteded no errors in
> https://travis-ci.org/xypron2/u-boot/builds/442613876
> 
> v4:
> 	fix reset_misc() too
> 	avoid switch statement
> v3:
> 	update commit message
> v2:
> 	Use PSCI firmware driver to detect call method.
> 	Thanks to Sumit for pointing me at dtb parsing as part of
> 	"psci_probe()
> ---
>  arch/arm/cpu/armv7/smccc-call.S |   2 +
>  arch/arm/cpu/armv8/Kconfig      |   1 +
>  arch/arm/cpu/armv8/fwcall.c     |  52 ++---------------
>  arch/arm/cpu/armv8/smccc-call.S |   2 +
>  drivers/firmware/psci.c         | 100 +++++++++++++++++++++++++++-----
>  include/linux/psci.h            |   6 +-
>  6 files changed, 96 insertions(+), 67 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv7/smccc-call.S b/arch/arm/cpu/armv7/smccc-call.S
> index 0d8b59eb6b..eae69e36c3 100644
> --- a/arch/arm/cpu/armv7/smccc-call.S
> +++ b/arch/arm/cpu/armv7/smccc-call.S
> @@ -7,6 +7,8 @@
>  #include <asm/opcodes-sec.h>
>  #include <asm/opcodes-virt.h>
>  
> +	.section	.text.efi_runtime
> +
>  #define UNWIND(x...)
>  	/*
>  	 * Wrap c macros in asm macros to delay expansion until after the
> diff --git a/arch/arm/cpu/armv8/Kconfig b/arch/arm/cpu/armv8/Kconfig
> index c8bebabdf6..d643981b73 100644
> --- a/arch/arm/cpu/armv8/Kconfig
> +++ b/arch/arm/cpu/armv8/Kconfig
> @@ -96,6 +96,7 @@ endmenu
>  config PSCI_RESET
>  	bool "Use PSCI for reset and shutdown"
>  	default y
> +	select ARM_SMCCC if OF_CONTROL
>  	depends on !ARCH_EXYNOS7 && !ARCH_BCM283X && \
>  		   !TARGET_LS2080A_SIMU && !TARGET_LS2080AQDS && \
>  		   !TARGET_LS2080ARDB && !TARGET_LS2080A_EMU && \
> diff --git a/arch/arm/cpu/armv8/fwcall.c b/arch/arm/cpu/armv8/fwcall.c
> index 0ba3dad8cc..9957c2974b 100644
> --- a/arch/arm/cpu/armv8/fwcall.c
> +++ b/arch/arm/cpu/armv8/fwcall.c
> @@ -7,7 +7,6 @@
>  
>  #include <asm-offsets.h>
>  #include <config.h>
> -#include <efi_loader.h>
>  #include <version.h>
>  #include <asm/macro.h>
>  #include <asm/psci.h>
> @@ -19,7 +18,7 @@
>   * x0~x7: input arguments
>   * x0~x3: output arguments
>   */
> -static void __efi_runtime hvc_call(struct pt_regs *args)
> +static void hvc_call(struct pt_regs *args)
>  {
>  	asm volatile(
>  		"ldr x0, %0\n"
> @@ -53,7 +52,7 @@ static void __efi_runtime hvc_call(struct pt_regs *args)
>   * x0~x3: output arguments
>   */
>  
> -void __efi_runtime smc_call(struct pt_regs *args)
> +void smc_call(struct pt_regs *args)
>  {
>  	asm volatile(
>  		"ldr x0, %0\n"
> @@ -83,9 +82,9 @@ void __efi_runtime smc_call(struct pt_regs *args)
>   * use PSCI on U-Boot running below a hypervisor, please detect
>   * this and set the flag accordingly.
>   */
> -static const __efi_runtime_data bool use_smc_for_psci = true;
> +static const bool use_smc_for_psci = true;
>  
> -void __noreturn __efi_runtime psci_system_reset(void)
> +void __noreturn psci_system_reset(void)
>  {
>  	struct pt_regs regs;
>  
> @@ -100,7 +99,7 @@ void __noreturn __efi_runtime psci_system_reset(void)
>  		;
>  }
>  
> -void __noreturn __efi_runtime psci_system_off(void)
> +void __noreturn psci_system_off(void)
>  {
>  	struct pt_regs regs;
>  
> @@ -114,44 +113,3 @@ void __noreturn __efi_runtime psci_system_off(void)
>  	while (1)
>  		;
>  }
> -
> -#ifdef CONFIG_CMD_POWEROFF
> -int do_poweroff(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> -{
> -	puts("poweroff ...\n");
> -
> -	udelay(50000); /* wait 50 ms */
> -
> -	disable_interrupts();
> -
> -	psci_system_off();
> -
> -	/*NOTREACHED*/
> -	return 0;
> -}
> -#endif
> -
> -#ifdef CONFIG_PSCI_RESET
> -void reset_misc(void)
> -{
> -	psci_system_reset();
> -}
> -
> -#ifdef CONFIG_EFI_LOADER
> -void __efi_runtime EFIAPI efi_reset_system(
> -			enum efi_reset_type reset_type,
> -			efi_status_t reset_status,
> -			unsigned long data_size, void *reset_data)
> -{
> -	if (reset_type == EFI_RESET_COLD ||
> -	    reset_type == EFI_RESET_WARM ||
> -	    reset_type == EFI_RESET_PLATFORM_SPECIFIC) {
> -		psci_system_reset();
> -	} else if (reset_type == EFI_RESET_SHUTDOWN) {
> -		psci_system_off();
> -	}
> -
> -	while (1) { }
> -}
> -#endif /* CONFIG_EFI_LOADER */
> -#endif /* CONFIG_PSCI_RESET */
> diff --git a/arch/arm/cpu/armv8/smccc-call.S b/arch/arm/cpu/armv8/smccc-call.S
> index 16c9e298b4..86de4b4089 100644
> --- a/arch/arm/cpu/armv8/smccc-call.S
> +++ b/arch/arm/cpu/armv8/smccc-call.S
> @@ -6,6 +6,8 @@
>  #include <linux/arm-smccc.h>
>  #include <generated/asm-offsets.h>
>  
> +	.section	.text.efi_runtime
> +
>  	.macro SMCCC instr
>  	.cfi_startproc
>  	\instr	#0
> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
> index 2cb35f356f..7ec5eb094b 100644
> --- a/drivers/firmware/psci.c
> +++ b/drivers/firmware/psci.c
> @@ -9,31 +9,38 @@
>  #include <common.h>
>  #include <dm.h>
>  #include <dm/lists.h>
> +#include <efi_loader.h>
>  #include <linux/libfdt.h>
>  #include <linux/arm-smccc.h>
>  #include <linux/errno.h>
>  #include <linux/printk.h>
>  #include <linux/psci.h>
>  
> -psci_fn *invoke_psci_fn;
> +#define DRIVER_NAME "psci"
>  
> -static unsigned long __invoke_psci_fn_hvc(unsigned long function_id,
> -			unsigned long arg0, unsigned long arg1,
> -			unsigned long arg2)
> -{
> -	struct arm_smccc_res res;
> +#define PSCI_METHOD_HVC 1
> +#define PSCI_METHOD_SMC 2
>  
> -	arm_smccc_hvc(function_id, arg0, arg1, arg2, 0, 0, 0, 0, &res);
> -	return res.a0;
> -}
> +int __efi_runtime_data psci_method;
>  
> -static unsigned long __invoke_psci_fn_smc(unsigned long function_id,
> -			unsigned long arg0, unsigned long arg1,
> -			unsigned long arg2)
> +unsigned long __efi_runtime invoke_psci_fn
> +		(unsigned long function_id, unsigned long arg0,
> +		 unsigned long arg1, unsigned long arg2)
>  {
>  	struct arm_smccc_res res;
>  
> -	arm_smccc_smc(function_id, arg0, arg1, arg2, 0, 0, 0, 0, &res);
> +	/*
> +	 * In the __efi_runtime we need to avoid the switch statement. In some
> +	 * cases the compiler creates lookup tables to implement switch. These
> +	 * tables are not correctly relocated when SetVirtualAddressMap is
> +	 * called.
> +	 */
> +	if (psci_method == PSCI_METHOD_SMC)
> +		arm_smccc_smc(function_id, arg0, arg1, arg2, 0, 0, 0, 0, &res);
> +	else if (psci_method == PSCI_METHOD_HVC)
> +		arm_smccc_hvc(function_id, arg0, arg1, arg2, 0, 0, 0, 0, &res);
> +	else
> +		res.a0 = PSCI_RET_DISABLED;
>  	return res.a0;
>  }
>  
> @@ -67,9 +74,9 @@ static int psci_probe(struct udevice *dev)
>  	}
>  
>  	if (!strcmp("hvc", method)) {
> -		invoke_psci_fn = __invoke_psci_fn_hvc;
> +		psci_method = PSCI_METHOD_HVC;
>  	} else if (!strcmp("smc", method)) {
> -		invoke_psci_fn = __invoke_psci_fn_smc;
> +		psci_method = PSCI_METHOD_SMC;
>  	} else {
>  		pr_warn("invalid \"method\" property: %s\n", method);
>  		return -EINVAL;
> @@ -78,6 +85,67 @@ static int psci_probe(struct udevice *dev)
>  	return 0;
>  }
>  
> +/**
> + * void do_psci_probe() - probe PSCI firmware driver
> + *
> + * Ensure that psci_method is initialized.
> + */
> +static void __maybe_unused do_psci_probe(void)
> +{
> +	struct udevice *dev;
> +
> +	uclass_get_device_by_name(UCLASS_FIRMWARE, DRIVER_NAME, &dev);
> +}
> +
> +#ifdef CONFIG_EFI_LOADER

I think this should be #if IS_ENABLED(CONFIG_EFI_LOADER) &&
IS_ENABLED(CONFIG_PSCI_RESET), as otherwise we may trigger unexpected
PSCI resets from efi land that wouldn't happen with normal cmds.


Alex

> +efi_status_t efi_reset_system_init(void)
> +{
> +	do_psci_probe();
> +	return EFI_SUCCESS;
> +}
> +
> +void __efi_runtime EFIAPI efi_reset_system(enum efi_reset_type reset_type,
> +					   efi_status_t reset_status,
> +					   unsigned long data_size,
> +					   void *reset_data)
> +{
> +	if (reset_type == EFI_RESET_COLD ||
> +	    reset_type == EFI_RESET_WARM ||
> +	    reset_type == EFI_RESET_PLATFORM_SPECIFIC) {
> +		invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
> +	} else if (reset_type == EFI_RESET_SHUTDOWN) {
> +		invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
> +	}
> +	while (1)
> +		;
> +}
> +#endif /* CONFIG_EFI_LOADER */
> +
> +#ifdef CONFIG_PSCI_RESET
> +void reset_misc(void)
> +{
> +	do_psci_probe();
> +	invoke_psci_fn(PSCI_0_2_FN_SYSTEM_RESET, 0, 0, 0);
> +}
> +#endif /* CONFIG_PSCI_RESET */
> +
> +#ifdef CONFIG_CMD_POWEROFF
> +int do_poweroff(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> +	do_psci_probe();
> +
> +	puts("poweroff ...\n");
> +	udelay(50000); /* wait 50 ms */
> +
> +	disable_interrupts();
> +	invoke_psci_fn(PSCI_0_2_FN_SYSTEM_OFF, 0, 0, 0);
> +	enable_interrupts();
> +
> +	log_err("Power off not supported on this platform\n");
> +	return CMD_RET_FAILURE;
> +}
> +#endif
> +
>  static const struct udevice_id psci_of_match[] = {
>  	{ .compatible = "arm,psci" },
>  	{ .compatible = "arm,psci-0.2" },
> @@ -86,7 +154,7 @@ static const struct udevice_id psci_of_match[] = {
>  };
>  
>  U_BOOT_DRIVER(psci) = {
> -	.name = "psci",
> +	.name = DRIVER_NAME,
>  	.id = UCLASS_FIRMWARE,
>  	.of_match = psci_of_match,
>  	.bind = psci_bind,
> diff --git a/include/linux/psci.h b/include/linux/psci.h
> index 8d13bd2702..9433df836b 100644
> --- a/include/linux/psci.h
> +++ b/include/linux/psci.h
> @@ -88,10 +88,8 @@
>  #define PSCI_RET_DISABLED			-8
>  
>  #ifdef CONFIG_ARM_PSCI_FW
> -typedef unsigned long (psci_fn)(unsigned long, unsigned long,
> -				unsigned long, unsigned long);
> -
> -extern psci_fn *invoke_psci_fn;
> +unsigned long invoke_psci_fn(unsigned long a0, unsigned long a1,
> +			     unsigned long a2, unsigned long a3);
>  #else
>  unsigned long invoke_psci_fn(unsigned long a0, unsigned long a1,
>  			     unsigned long a2, unsigned long a3)
> 


More information about the U-Boot mailing list