[U-Boot] [PATCH v4 1/1] efi_loader: PSCI reset and shutdown
Heinrich Schuchardt
xypron.glpk at gmx.de
Wed Oct 17 18:06:32 UTC 2018
On 10/17/2018 07:37 PM, Alexander Graf wrote:
>
>
> 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
U-Boot is the little servant that helps the rider on the horse and
neither horse nor rider.
Hence U-Boot should not unnecessarily interfere with the communication
between the operating system and the PSCI firmware.
I cannot imagine any use case where the authorized user in the operating
system executes a reboot or shutdown command and expects the system
simply not to do it.
Best regards
Heinrich
More information about the U-Boot
mailing list