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

On Tue, Aug 11, 2026 at 01:50:14PM +0100, Bradley Morgan wrote:
> On 11 August 2026 11:18:27 BST, Will Deacon <[email protected]> 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: [email protected]
> >> Signed-off-by: Bradley Morgan <[email protected]>
> >> ---
> >>  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? You're implying that this
hasn't worked for five years, which makes me wonder why we bother to try
to maintain this code!

> >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.

> >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?

> >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.

Will