On Mon, Nov 04, 2024 at 02:54:12PM +0100, Ard Biesheuvel wrote:
On Fri, 1 Nov 2024 at 18:49, Lorenzo Pieralisi lpieralisi@kernel.org wrote:
[+Ard, Sami, for EFI]
On Thu, Oct 31, 2024 at 06:55:43PM +0100, Lorenzo Pieralisi wrote:
On Sat, Oct 19, 2024 at 06:15:47PM +0100, David Woodhouse wrote:
[...]
+#ifdef CONFIG_HIBERNATION +static int psci_sys_hibernate(struct sys_off_data *data) +{
- /*
- Zero is an acceptable alternative to PSCI_1_3_OFF_TYPE_HIBERNATE_OFF
- and is supported by hypervisors implementing an earlier version
- of the pSCI v1.3 spec.
- */
It is obvious but with this patch applied a host kernel would start executing SYSTEM_OFF2 too if supported in firmware to hibernate, it is not a hypervisor only code path.
Related to that: is it now always safe to override
commit 60c0d45a7f7a ("efi/arm64: use UEFI for system reset and poweroff")
for hibernation ? It is not very clear to me why overriding PSCI for poweroff was the right thing to do - tried to follow that patch history but the question remains (it is related to UpdateCapsule() but I don't know how that applies to the hibernation use case).
RFC: It is unclear to me what happens in current mainline if we try to hibernate with EFI runtime services enabled and a capsule update pending (we issue EFI ResetSystem(EFI_RESET_SHUTDOWN,..) which might not be compatible with the reset required by the pending capsule update request) what happens in this case I don't know but at least the choice is all contained in EFI firmware.
Then if in the same scenario now we are switching to PSCI SYSTEM_OFF2 for the hibernate reset I suspect that what happens to the in-flight capsule update requests strictly depends on what "reset" PSCI SYSTEM_OFF2 will end up doing ?
I think this is just a corner case and it is unlikely it has been ever tested (is it even possible ? Looking at EFI folks) - it would be good to clarify it at least to make sure we understand this code path.
I'm not aware of any OS that actually uses capsule update at runtime (both Windows and Linux queue up the capsule and call the UpdateCapsule() runtime service at boot time after a reboot).
So it is unlikely that this would break anything, and I'd actually be inclined to disable capsule update at runtime altogether.
I will also note that hibernation with EFI is flaky in general, given that EFI memory regions may move around
Thank you for chiming in, I think we are OK (I don't think this patch will create more issues than the ones that are already there for hibernate anyway) - the reasoning behind the change is in the commit logs.
Lorenzo