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: <f0575d3c-15a0-8a33-9fbd-8ebe1be33034@nokia.com>
Date:   Fri, 28 Jan 2022 15:21:30 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.3.0
Subject: Re: [PATCH v2] 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: <YfP0osb45uJldtM9@localhost.localdomain>
From:   Alexander Sverdlin <alexander.sverdlin@nokia.com>
In-Reply-To: <YfP0osb45uJldtM9@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?THRBbkhyMzR6S09vbVRlcWUzK1R1dmQwQVBYUHhLTjFYRHlWWElYcTJTa0lz?=
 =?utf-8?B?R0pPTEUvNmlDTDBhL0x2eHdYR2xIWXN2a0RsSlRiL01JaWhQTkhwcjNTNUhI?=
 =?utf-8?B?M1dwQ0ZsRGl5L3B2TXYrSVlUODBsZHpNaFc4OFdGaHV4NVJGR0FXOXpDSElE?=
 =?utf-8?B?R2lzejhteENYaUdEVnFFU2FzaXVlOHhFb1V0bUpqeEpWeTdNRCtvN2hDUm5k?=
 =?utf-8?B?MGJsUjVROHNjSHEwMnh3YXp0OVJGdzRRTm53OFRCRU9kSkZiVVFnZmdkTWVx?=
 =?utf-8?B?ZzQ5NmdJblBEYzBGSGdXVU5yTjRtc2J5SkRIWG1GR1F6VmR6ZzM3azBmMEZp?=
 =?utf-8?B?a042NGRyTTVZMUIyREwva3hITW9MRU1LUjZWcEw3aWxrdFdyUWF4REUvWmEy?=
 =?utf-8?B?cUJMNThUZ05LVmtCdWlmZG5aSDRPY3lHZjNjMExaNHN2RGlFWU96VWhTSmNG?=
 =?utf-8?B?NjR4TndQOGhzRTZqWmFyelpuRHJqR3B3UWF4ZzB2TWxORVdnZ0dmQmk4UkxI?=
 =?utf-8?B?NEtxaWh5QnkwSXBjbVhWbXJnZkZZUHpZd3E5bHhYM3JucUNEQko4VnFQbG03?=
 =?utf-8?B?aUViNjFCcVlkTG9zNVoxMjJHSHBVWXg5TzhWbzVOMGE4NFM0dzVPWDBpQmg2?=
 =?utf-8?B?ZUYrcDZBTG12NWsxN0NqYkJaS3lrRjBSelBiMDc3RFo4dWhZbVFrVG4wbXBz?=
 =?utf-8?B?dzM1N0VjSnRnSWFQYndFODYraUpHZEY3R2dlTW5obzU0TloyYWZkK0dONnln?=
 =?utf-8?B?S0p2LzlsVC8yWkFhNFFjWmhhMW5iSjlsbGRDWFlOT1NjNVVzYWU3a0ZhelpC?=
 =?utf-8?B?R29PNjN1czlFOFdDYkROZ0dUQjU4WVBMUURHNFFGbWFSaU9NR2lybVpUbW5j?=
 =?utf-8?B?bmgrYlNSZlhiSXF5WFUyZnlqOTQyT1BuK2ZTcTlqQ1gwYjZuTWgvZzlyMjFi?=
 =?utf-8?B?WGZIQXRpME5nbDlpWGVSVWQ2SnR4SzJaTnBHck51ZWFBQWJHM0U2b3Q0UXNt?=
 =?utf-8?B?cWplVDhwTHVaZ0pocHZTR3dsNHN3K3p5S3h2eisxRjdOT2loYWFnWWlWeUxV?=
 =?utf-8?B?RmNFVklKZWJjVm9MeG5aN3ZRL3lKd0xEMGYyVkZrRjZyaFlabS9kMU9rcE5C?=
 =?utf-8?B?N0Z3dXZRc3hPd21oUmppM25ycmk3UkRQUFgxOHFhMmtUNXZJZG5uaS9Zd2py?=
 =?utf-8?B?TWYyaDZvT1RmVWFCdkFkaUVRMmIwZy9qSFRBY1hBVkdwWm95aWNXSGQ0UGVJ?=
 =?utf-8?B?UTQzUWE1aE9hT2NrRlFoZEpqNldZbE1hb0ZMQytTakZCYVVhTHRDUm12YmdE?=
 =?utf-8?B?T2hWNXN2L2t6bHhhKzBPa3VEMHFvTUw2NUNNR3dJcDgyZ1RTVW9Gc2xTL3Va?=
 =?utf-8?B?b0hWcDJ2REFzREtIK2dOY0pBVDN1ZVNzRWdQb2J6RzZSZURkdXhDTDIwWmNX?=
 =?utf-8?B?amtqUUM3RHlIVHdHQkhVQlZZTUpnN3lDenluT1grRldNZGNadzNObnlpZlJv?=
 =?utf-8?B?c3llemFGaEFka0JKSEVNUG1TSHhZazNLbnZCRWNvNkVMWngzRHdpdWx2YUpN?=
 =?utf-8?B?dkVMaXEzQmExZjd3dEI0bGYvSUgySGRzR2w0Z3EwRnF0N2s5M1ErelFFdVJh?=
 =?utf-8?B?di9zaDhSYmtPbC9sc3FIc29INFBBZm1OU0s0MFQ4cnQveC8rN2dpOXRPUXUy?=
 =?utf-8?B?RThKZDhNb1FJV3UvN0FkLzd5T3duMmZ1S09ML2dMZ3ZCVnhwc0NMVjh0RWwy?=
 =?utf-8?B?SUh0MjRKdUdHOWludEhkTDFjcU05TzJiZHJ6VEtobFRaTFdiaU9UNkNuaW1T?=
 =?utf-8?B?R2RvRWlpOC95SU4raGN3ZXJLODIzbndRcWlTdEpUa2FDYkZsZE8zay82em5B?=
 =?utf-8?B?K3dOTU40NktBZTV1a2M3V2x6QVJtSkxncjRKbkU4Z3Z2VkJNdm5VemE1dWlz?=
 =?utf-8?B?NGNyeHQxeHB3S3JKdi9GbExXaFhkV09Eb0N6cmQrbnA0OHZzR2JWcXhWSTRk?=
 =?utf-8?B?UlMwRFlLSEp4OUpZY1hTVWV0bG9ObUdQdWNLdG14Vm9Ub1llVVEzdHJPcXV6?=
 =?utf-8?B?M21xNFhtTnFReEMvc1hFS0E2VWJRNmZNdmcyNjlJNERycHlTemJkRldSWng4?=
 =?utf-8?B?MlVUL3BsNDBSM3U2YUdUZmF5MGM4cDJOdUFOdVIyYWZjSTRWTGFIV0RWSmpG?=
 =?utf-8?B?bWZUcmhaajhaOWhTWmtDUUdXTUt2aUR5azhWMnpKd0VFWlAwUWlrTHNMZktH?=
 =?utf-8?Q?fNyfFZU40eYIs8a8FGSCg2EPhIOqO/6/XJaG7XTRTk=3D?=
X-OriginatorOrg: nokia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c6052a97-9fb8-4b84-ac07-08d9e2697dd3
X-MS-Exchange-CrossTenant-AuthSource: AM0PR07MB4531.eurprd07.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Jan 2022 14:21:36.1665
 (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: m29u+ALdTfnae9pla49gvqdO12Q/qpyuOqWHjXeApSabg7THMq1nyGDYOE2S8YNt0NR61e8JOS7pa9yo+yQ12AwwEb9EY3yVgXqPZwlNEzI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR07MB7591
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi!

On 28/01/2022 14: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@nokia.com>

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

> ---
> 
> 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);

-- 
Best regards,
Alexander Sverdlin.