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

On 17 August 2026 14:06:20 BST, Vladimir Murzin <[email protected]>
wrote:
>Hi Will,
>
>On 8/11/26 15:37, Will Deacon wrote:
>> 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.
>> 
>
>Here is my understanding of how things work. Assuming nVHE mode, we
>restore the previously saved image X from the currently running kernel Y.
>I’ll use the suffixes _X and _Y for addresses belonging to the respective
>images.
>
>On the resume path, KVM teardown sets VBAR_EL2 to __hyp_stub_vectors_Y.
>
>swsusp_arch_resume() then temporarily (re)sets VBAR_EL2 to
>trans_pgd_stub_vectors
>while transitioning to image X:
>
>|        if (el2_reset_needed())
>|                __hyp_set_vectors(el2_vectors);
>
>
>Currently, swsusp_arch_suspend_exit() attempts to restore VBAR_EL2 to
>__hyp_stub_vectors_X:
>
>
>|       cbz     x24, 3f         /* Do we need to re-initialise EL2? */
>|       hvc     #0
>
>where x24 is resume_hdr.__hyp_stub_vectors (in other words snapshot of
>__hyp_stub_vectors from image X)
>
>However, that request is ignored, so VBAR_EL2 remains pointing to
>trans_pgd_stub_vectors
>
>We re-enter the kernel X.
>
>During KVM initialisation, we call __hyp_reset_vectors(). This is
>handled by trans_pgd_stub_vectors, but ignored since HVC_RESET_VECTORS
>is not recognised. We then call __hyp_set_vectors(), which is also
>handled by trans_pgd_stub_vectors, but this time VBAR_EL2 get set to
>__kvm_hyp_init_X.
>
>From that point on, __kvm_hyp_init_X handles subsequent HVC calls,
>including the later call that redirects VBAR_EL2 to
>__kvm_hyp_host_vector_X during KVM setup.
>
>The memory containing trans_pgd_stub_vectors is freed later by
>swsusp_free(). That perhaps explains why we do not see any crash.
>
>With the proposed fix, swsusp_arch_suspend_exit() successfully restore
>VBAR_EL2 to __hyp_stub_vectors_X just before we re-enter kernel X, and
>that vector is used for subsequent HVC calls.
>
>__hyp_stub_vectors is similar to trans_pgd_stub_vectors (or another
>way around?) - it handles both HVC_SET_VECTORS and HVC_SOFT_RESTART,
>and ignores HVC_RESET_VECTORS.
>
>So the fix makes sense to me. It also restores clear boundary between
>the temporary resume mechanism and handing control over to the
>restored image.
>
>FWIW,
>
>Reviewed-by: Vladimir Murzin <[email protected]>
>

Cheers!

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

Thanks!