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

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

[Bradley Morgan]

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

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

[Will Deacon]

[+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)
1. Why the original hibernation code set the vectors twice.
2. Assuming they only need to be set once, whether we can drop the hvc
   from the swsusp_arch_suspend_exit assembly code entirely.
3. Whether we can then drop the HVC_SET_VECTORS handling from this set
   of vectors.

Will

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

[Bradley Morgan]

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!

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

[Will Deacon]

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

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

[Bradley Morgan]

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!

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

[Vladimir Murzin]

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 <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? 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 <vladimir.murzin@arm.com>


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

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

[Bradley Morgan]

On 17 August 2026 14:06:20 BST, Vladimir Murzin <vladimir.murzin@arm.com>
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 <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? 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 <vladimir.murzin@arm.com>
>

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!