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

[Bradley Morgan <include@grrlz.net>]

On 11 August 2026 15:37:01 BST, Will Deacon <will@kernel.org> wrote:
>On Tue, Aug 11, 2026 at 01:50:14PM +0100, Bradley Morgan wrote:
>> 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.
>
>Right, but did you manage to reproduce a crash? 

No, I had a read of the code, yk, summer holidays are here for me, so
I have this kinda time

You're implying that this
>hasn't worked for five years, which makes me wonder why we bother to try
>to maintain this code!

the path only runs when is_hyp_nvhe(), hence VHE machines never get there, 



>
>> >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.
>
>I think I probably need to spend some time understanding how all this is
>supposed to work. I can't currently tell how we end up with the stub
>vectors installed to start with nor why we can't do all this from C code.
>

head.S installs __hyp_stub_vectors in vbar_el2 before dropping to EL1.
The last set can't come from C code because at that point the image
has been restored over the kernel. The safe page copy is the only code
still running and it's about to hand control back. The hvc is how it
reaches EL2.

>> >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.
>
>Isn't vbar_el2 always only writable from EL2?
>

Yes, that phrasing was sloppy on my end. All I meant was the safe page
code runs at EL1, so the hvc is the way it reaches EL2.

>> >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.
>
>Where does kexec use it? I could only spot it making use of
>HVC_SOFT_RESTART.
>


The non kexec_file path. machine_kexec.c calls
__hyp_set_vectors(kimage->arch.el2_vectors) before jumping to the
relocation code when is_hyp_nvhe(). The kexec_file path takes
cpu_soft_restart, which is where HVC_SOFT_RESTART comes in. So kexec
uses both, one per path........


Well, I also will think about this approach, if you have any suggestions,
feel free to show me. Please :)

>Will
>

Thanks!