Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <c92e2db9-00b4-8800-e800-046d564f52d9@nokia.com>
Date:   Mon, 17 Jan 2022 13:44:41 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.3.0
Subject: Re: [PATCH] arm64: move efi_reboot to restart handler
Content-Language: en-US
To:     Krzysztof Adamski <krzysztof.adamski@nokia.com>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will@kernel.org>,
        Mark Rutland <mark.rutland@arm.com>,
        Peter Collingbourne <pcc@google.com>,
        Guenter Roeck <linux@roeck-us.net>,
        Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc:     Matija Glavinic-Pecotic <matija.glavinic-pecotic.ext@nokia.com>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
References: <YeVhtL2gCLkhTPdv@localhost.localdomain>
From:   Alexander Sverdlin <alexander.sverdlin@nokia.com>
In-Reply-To: <YeVhtL2gCLkhTPdv@localhost.localdomain>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?MTVTZE5WQktPZVZQOTV1YWN0Vm92VnhkcVAwTnJGSGdLb1JiZlFScll2ZU11?=
 =?utf-8?B?WHlPK1VPZzIxNktibUNqUFRrRVA2a1dtSy92NmcvUE5HSHRUbENlaUdqYnBF?=
 =?utf-8?B?TWFCd0RtQkhoc2p4SEpQSmtuVll2em0vYWs3YzBMU3dieWovSk0yMndudkNC?=
 =?utf-8?B?VWdHV1pXQWpFY2M0VGY3SURjamFDMlZzMU1FOWI3b2lQOVI5ZGR0NG1XTGVR?=
 =?utf-8?B?SlNYNGJqNVQyNDdhVHBiS0NSakh4S0hLSlgrS2dsZWlJY0g0QlZXbDM5Umx5?=
 =?utf-8?B?K1ptSnpMWXJsL0p2empaMTBnQ3c3dTl2OE5icC9IT3dqZ0hyTlY1U1JkVWpG?=
 =?utf-8?B?Qkp1bXlxdEI1czByd2Zqb0tMSXQzc3pMdENPczFQdnc2OEE1K3ZXKzViMlFY?=
 =?utf-8?B?ODEzY2xlWHhHL3JqOWJrdkg2b0dSZWhJYUtFb0tTeFpCZEVUVEgyYUFXL3ZU?=
 =?utf-8?B?c2FVK3BxelpibmNyOGtwUWhOQkQrV1FMdUhKOVlsU3o5dzkvSEJxblpQM2Yw?=
 =?utf-8?B?R2ovZENkVjlrS3hzT1k2UHBFVVV5RS9Kek9ISDVrU3JxcHZFTE1JSitxSEhk?=
 =?utf-8?B?SmZoUlpPNGp1TU9keDliS1ZSTllDczE3bFk0QXpxYlV2OThkWlhCMEYzcCtF?=
 =?utf-8?B?V3lBQ3BqRUVyTUpxRHd3TWtXU3RaclIxZmNGUEhyZlYrMWlXbWtBUWhxeDR5?=
 =?utf-8?B?dlRuTnZQdlovWUVZUUxxZ2xTUVFyck4zRWFKY1JGQjNCbmRrUzQ3UFQrMit1?=
 =?utf-8?B?b2QwN3R0bHpQVSs4MDZJd2w5VnFxTHhmMUdTL29FRGRGWFJLN2xGbHdGd0Ur?=
 =?utf-8?B?M0tialEyaHBvclJxSTYrWlBEeGZpSVl5WWorNzhxcDhZaEg2aExqWTdVdDNV?=
 =?utf-8?B?dFdHbi93ODRBUXdDaVdBNHRmbEo5ZnRTaHpXOVpzd1dGM3h6cjE5N1FRVUZU?=
 =?utf-8?B?d2J5cU9zTDdZcVdlVDVwWmk5VzFmNEJBeWNsZnRCUld5YzJ3ZG1NQW9NclQ4?=
 =?utf-8?B?TlpjRTJpWHBXN3BzWFlHTzd6c201K3pTTU1TV3Q5RVZpby9rOXBJZElURkhl?=
 =?utf-8?B?c040OHBCVk5oeHdHQ3BVbXBVMWI4Q2ZIY3JWU1c2T0M1MGoyRjd3WWZzNXBk?=
 =?utf-8?B?WnNqYXJiaFlIamY1c3p4QWdBaHZHb3FudEV6dlVHa1BUcE1wamNEUlMwbjF3?=
 =?utf-8?B?NEg1WFNHNEJsY2lVOFhleFVVb3BCOHc0eHJOcUVjTFcxWlNKSllTbWdBVXhG?=
 =?utf-8?B?VGMzYkl3bWZrTE1XSzhya3VYNVo3Z3VPTDJJUkRaTXlQeHBndTFvNnUybXg2?=
 =?utf-8?B?dGdGck9EMUxvckw5dTl5VG5vMitBa3lzQnp3T0xZVG11YTZOS2gwSlFUc1Ew?=
 =?utf-8?B?R1luNE5IUytsV3NSajAvR3Q0cUZ1b1B0em0wOTRSWGprT2JrVStIY3FwMnM1?=
 =?utf-8?B?LzZhOTVHZG5sZ29TVTVuU2tPOG4zUDFCNlBmQVFFRXZNVXVBWG9SZFdNd1lY?=
 =?utf-8?B?a1RPZkdMRmZoUDhaV3NjbXFlUUdadWtnTGxiL1ZMUE10dUErbVJKelJXNXRj?=
 =?utf-8?B?UFR4TnREeU4rc2lyZkpyVlA1SUFGenY0cUJkOG1KSzc4SGc1bWVJK2VGM2lH?=
 =?utf-8?B?dkY2cUo0Y01rUUdlOVpKSkFxdFBQdG9uc0ZRZU8zMnV6WjBpaWdtWWZwcHhP?=
 =?utf-8?B?RGViakM1V0hyZE43ZTQwcG5LcjgzMEs4cWJhNnBXNVBMd2RSZG83RVk4cTla?=
 =?utf-8?B?bHlZN254enZOVkEwOFUwNmlzcW9NN3RWTzRSMFBCbzBRR1dEM1F4ZHljbUE1?=
 =?utf-8?B?NG13dGp1MjQyNXl4U0NWWFdxWU1jRFE3UVJ4ZWRqa1NDV01GaW9NN0hzbFFE?=
 =?utf-8?B?Qlg5YVFJczBoQ0l3aWNzd2gzWkN0MTNpRW95WXFVczBOSmdsT21nSVFWMXlB?=
 =?utf-8?B?WGNWQkl2a3B1QWltWjk1VTQ1TlBUNlFJWnVwNEdWL2Z5VGtmUjc1K2pyR1JY?=
 =?utf-8?B?dG1aSlhnaFBMUEpsK1JqU0tibXB6V0JrQkFab1BnUDF4bG9xeGQ5OFNPM09T?=
 =?utf-8?B?VHlHb0lYVjNNTXdUeDRzais2RG83cVZ1M1lzYVlSS202SCtSeWFSMllBMjZB?=
 =?utf-8?B?VTZEb2h0aERpbUNWekVBZnJJV3h0TmNPRStCd29rN1pISzRTeVBWTUd6U3ZJ?=
 =?utf-8?B?NWVOLzNUdTNnNjVuaS81VjBhTzNJMXlhS1NZaXJGVlMzV3JKS1o2Smk5Rlcz?=
 =?utf-8?B?U0RaOG0yclBXOU0wV1lvQlFrVjJnPT0=?=
X-OriginatorOrg: nokia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 50970c0b-ed31-416c-a361-08d9d9b725d4
X-MS-Exchange-CrossTenant-AuthSource: AM0PR07MB4531.eurprd07.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Jan 2022 12:44:48.8289
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 5d471751-9675-428d-917b-70f44f9630b0
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: aH7tg0DeeJ3HfZ2DEPdhnfXc1JdKf2/vKx9OvM3+qI/zu6QvC4yH5rFJ7pPMjk+7SJdTdXBjwKbrRES9gKM1kS68XmqxK4ArxtyOBmrGcHU=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5PR0701MB2961
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi!

On 17/01/2022 13:31, 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 129 should be high enough as we will likely be the first
> one to register on this prio so we will be called before others, like
> PSCI handler.
> 
> Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>

Reviewed-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>

> ---
>  arch/arm64/kernel/process.c |  7 -------
>  arch/arm64/kernel/setup.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/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index f70573928f1b..5fa95980ba73 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -12,6 +12,7 @@
>  #include <linux/stddef.h>
>  #include <linux/ioport.h>
>  #include <linux/delay.h>
> +#include <linux/reboot.h>
>  #include <linux/initrd.h>
>  #include <linux/console.h>
>  #include <linux/cache.h>
> @@ -298,6 +299,24 @@ u64 cpu_logical_map(unsigned int cpu)
>  	return __cpu_logical_map[cpu];
>  }
>  
> +static int efi_restart(struct notifier_block *nb, unsigned long action,
> +		       void *data)
> +{
> +	/*
> +	 * UpdateCapsule() depends on the system being reset via
> +	 * ResetSystem().
> +	 */
> +	if (efi_enabled(EFI_RUNTIME_SERVICES))
> +		efi_reboot(reboot_mode, NULL);
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block efi_restart_nb = {
> +	.notifier_call = efi_restart,
> +	.priority = 129,
> +};
> +
>  void __init __no_sanitize_address setup_arch(char **cmdline_p)
>  {
>  	setup_initial_init_mm(_stext, _etext, _edata, _end);
> @@ -346,6 +365,8 @@ void __init __no_sanitize_address setup_arch(char **cmdline_p)
>  
>  	paging_init();
>  
> +	register_restart_handler(&efi_restart_nb);
> +
>  	acpi_table_upgrade();
>  
>  	/* Parse the ACPI tables for possible boot-time configuration */

-- 
Best regards,
Alexander Sverdlin.