Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Sender: Guenter Roeck <groeck7@gmail.com>
Message-ID: <489b76f9-fbaf-dae0-c90d-c52ee0de92a4@roeck-us.net>
Date:   Tue, 15 Feb 2022 06:30:26 -0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Content-Language: en-US
To:     Alexander Sverdlin <alexander.sverdlin@nokia.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Ard Biesheuvel <ardb@kernel.org>
Cc:     Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will@kernel.org>,
        Peter Collingbourne <pcc@google.com>,
        Wolfram Sang <wsa+renesas@sang-engineering.com>,
        Matija Glavinic-Pecotic <matija.glavinic-pecotic.ext@nokia.com>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        Krzysztof Adamski <krzysztof.adamski@nokia.com>,
        linux-efi@vger.kernel.org
References: <YfP0osb45uJldtM9@localhost.localdomain>
 <YfQTZTUNaeGi+8tG@FVFF77S0Q05N> <YfRorCpk0seVGI+5@localhost.localdomain>
 <Yfk8hQB1eon7oeYU@FVFF77S0Q05N>
 <79bcce92-abb2-4c3e-7193-d18e144da8a0@nokia.com>
From:   Guenter Roeck <linux@roeck-us.net>
Subject: Re: [PATCH v2] arm64: move efi_reboot to restart handler
In-Reply-To: <79bcce92-abb2-4c3e-7193-d18e144da8a0@nokia.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2/15/22 00:44, Alexander Sverdlin wrote:
> Hello Mark, Ard,
> 
> On 01/02/2022 14:58, Mark Rutland wrote:
>>> You could argue that restart handlers were not created for that but they
>>> suit this purpose ideally and it wouldn't make much sense (in my
>>> opinion) to add yet another notifier chain that would run before reset
>>> notifiers, for code that is not supposed to reset the whole system and
>>> this is exacly what I would have to do if efi_reboot() is forced to be
>>> called before all handlers.
>>
>> As above, I think that's just using the wrong interface, and the reboot
>> notifier mechanism *already* exists, so I'm really confused here.
>>
>> Have I misunderstood what you're trying to achieve?
>>
>> Is there some problem with the reboot notifier mechanism that I am unaware of?
>> e.g. do we bypass them in some case where you think they're needed?
>>
>> Are you simply unaware of reboot notifiers?
> 
> Could you please check the simple case of pwrseq_emmc.c?
> 
> While that's currently the only example of this kind upstream I can imagine
> further use-cases, especially in storage area like above.
> 
> Would you suggest it's illegal usage of register_restart_handler()?
> Do we need to fix pwrseq_emmc.c by introducing new atomic notifier chain
> which will be called before restart handlers, so that it works on
> emergency_restart()?
> 

Abuse isn't just about using an API for something it isn't originally intended
for, abuse is also to intentionally _not_ use an API, as it is currently done
by the EFI restart code for arm64. Also, keep in mind that the same argument
(our restart handler _must_ be executed under all circumstances and does therefore
not use the restart API) is likely going to be used again in the future. All
it takes is for some other standard (or chip vendor, for that matter) to declare
their restart handler mandatory if present. That means there will be other
non-users of the restart API in the future, and you simply can not rely on it
being used. That was clearly not the intention of the restart API - quite the
opposite - but it is what it is.

Given that, I'd suggest to cut your losses and try to find another solution
for your problem. That may be to introduce yet another API, one that is called
before the EFI restart handling but that is always called, unlike reboot
notifiers, or simply stick with out-of-tree code.

Guenter