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