Re: [PATCH v2] arm64: move efi_reboot to restart handler

[Guenter Roeck]

On 1/28/22 05:50, Krzysztof Adamski wrote:
> On EFI enabled arm64 systems, efi_reboot was called before
> do_kernel_restart, completely omitting the reset_handlers functionality.
> By registering efi_reboot as part of the chain with slightly elevated
> priority, we make it run before the default handler but still allow
> plugging in other handlers.
> Thanks to that, things like gpio_restart, restart handlers in
> watchdog_core, mmc or mtds are working on those platforms.
>
> The priority 130 is one higher than PSCI, to overrule that but still
> allow to easily register higher prio handlers, if needed.
>
> Signed-off-by: Krzysztof Adamski <krzysztof.adamski@xxxxxxxxx>

Reviewed-by: Guenter Roeck <linux@xxxxxxxxxxxx>

> ---
>
> Changes in v2:
>    - Register the handler in EFI code, instead of arm64 setup.c
>    - Remove the contdition from the handler - it should be run in all
>      cases when it is registered
>    - Bump the priority to 130 to make it completly obious this should be
>      run before PSCI (which has priority of 129)
>
>   arch/arm64/kernel/process.c        |  7 -------
>   drivers/firmware/efi/arm-runtime.c | 21 +++++++++++++++++++++
>   2 files changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 5369e649fa79..b86ef77bb0c8 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -130,13 +130,6 @@ void machine_restart(char *cmd)
>   	local_irq_disable();
>   	smp_send_stop();
>   
> -	/*
> -	 * UpdateCapsule() depends on the system being reset via
> -	 * ResetSystem().
> -	 */
> -	if (efi_enabled(EFI_RUNTIME_SERVICES))
> -		efi_reboot(reboot_mode, NULL);
> -
>   	/* Now call the architecture specific reboot code. */
>   	do_kernel_restart(cmd);
>   
> diff --git a/drivers/firmware/efi/arm-runtime.c b/drivers/firmware/efi/arm-runtime.c
> index 3359ae2adf24..b9a2cdbe80b4 100644
> --- a/drivers/firmware/efi/arm-runtime.c
> +++ b/drivers/firmware/efi/arm-runtime.c
> @@ -80,6 +80,24 @@ static bool __init efi_virtmap_init(void)
>   	return true;
>   }
>   
> +static int efi_restart(struct notifier_block *nb, unsigned long action,
> +		       void *data)
> +{
> +	/*
> +	 * UpdateCapsule() depends on the system being reset via
> +	 * ResetSystem().
> +	 */
> +	efi_reboot(reboot_mode, NULL);
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block efi_restart_nb = {
> +	.notifier_call = efi_restart,
> +	/* We want this to take priority over PSCI which has priority of 129. */
> +	.priority = 130,
> +};
> +
>   /*
>    * Enable the UEFI Runtime Services if all prerequisites are in place, i.e.,
>    * non-early mapping of the UEFI system table and virtual mappings for all
> @@ -148,6 +166,9 @@ static int __init arm_enable_runtime_services(void)
>   	efi_native_runtime_setup();
>   	set_bit(EFI_RUNTIME_SERVICES, &efi.flags);
>   
> +	if (IS_ENABLED(CONFIG_ARM64))
> +		register_restart_handler(&efi_restart_nb);
> +
>   	return 0;
>   }
>   early_initcall(arm_enable_runtime_services);