Re: [PATCH] arm64: hibernate: pass HVC_SET_VECTORS args to the resume hvc

[Bradley Morgan <include@grrlz.net>]

On 11 August 2026 11:18:27 BST, Will Deacon <will@kernel.org> wrote:
>[+Maz, Pasha and James]
>
>On Sun, Aug 09, 2026 at 09:36:15PM +0000, Bradley Morgan wrote:
>> swsusp_arch_suspend_exit() reinstalls the restored kernel's hyp stub
>> vectors with an hvc, but never passes the arguments. x0 is not set to
>> HVC_SET_VECTORS and x1 is not set to the vector address, so the stub
>> dispatch falls through and returns without writing vbar_el2. EL2 is
>> left pointing at the trans_pgd copy of the vectors, a page that
>> swsusp_free() releases right after resume.
>> 
>> Set the arguments up the same way __hyp_set_vectors() does.
>> 
>> Fixes: 788bfdd97434 ("arm64: trans_pgd: hibernate: Add
>trans_pgd_copy_el2_vectors")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Bradley Morgan <include@grrlz.net>
>> ---
>>  arch/arm64/kernel/hibernate-asm.S | 2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/arch/arm64/kernel/hibernate-asm.S
>b/arch/arm64/kernel/hibernate-asm.S
>> index 0e1d9c3c6a93..2baefe7a82d3 100644
>> --- a/arch/arm64/kernel/hibernate-asm.S
>> +++ b/arch/arm64/kernel/hibernate-asm.S
>> @@ -89,6 +89,8 @@ alternative_insn "dc cvau, x4",  "dc civac, x4", 
>ARM64_WORKAROUND_CLEAN_CACHE
>>  	isb
>>  
>>  	cbz	x24, 3f		/* Do we need to re-initialise EL2? */
>> +	mov	x1, x24
>> +	mov	x0, #HVC_SET_VECTORS
>>  	hvc	#0
>>  3:	ret
>>  SYM_CODE_END(swsusp_arch_suspend_exit)
>
>I'm having a really hard time figuring out what's supposed to be going
>on here!
>
>The original hibernation code added by James in 82869ac57b5d ("arm64:
>kernel: Add support for hibernate/suspend-to-disk") unconditionally
>set the vectors in the exception handler:
>
>+el1_sync:
>+       msr     vbar_el2, x24
>+       eret
>+ENDPROC(el1_sync)
>
>However, it _also_ set the vectors from C code in swsusp_arch_resume():
>
>+       if (el2_reset_needed()) {
>+               phys_addr_t el2_vectors = phys_hibernate_exit;  /* base */
>+               el2_vectors += hibernate_el2_vectors -
>+                              __hibernate_exit_text_start;     /* offset */
>+
>+               __hyp_set_vectors(el2_vectors);
>+       }
>
>Later, Pasha refactored the assembly so that it could be shared with
>kexec in 788bfdd97434 ("arm64: trans_pgd: hibernate: Add
>trans_pgd_copy_el2_vectors"), however this added arguments to the
>exception handler without updating the hypercall on the hibernation path.
>
>So I think we need to figure out:
>
>0. Whether this code is actually broken atm (I have a feeling it might
>   happen to work)

Yes, since 788bfdd97434.


>1. Why the original hibernation code set the vectors twice.

They do different jobs. The C call parks EL2 on the safe page copy
before the restore overwrites the current table. The asm call installs
the final __hyp_stub_vectors afterwards.


>2. Assuming they only need to be set once, whether we can drop the hvc
>   from the swsusp_arch_suspend_exit assembly code entirely.

No. After the restore vbar_el2 is only writable from EL2, and the
temporary copy cannot stay. swsusp_free() frees it right after resume.

>3. Whether we can then drop the HVC_SET_VECTORS handling from this set
>   of vectors.
>

No, this hvc uses it, and so does kexec.

>Will
>

Thanks!