From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 3D6A7C5DF74 for ; Mon, 17 Aug 2026 14:16:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:MIME-Version:Message-ID:References:In-Reply-To:Subject:CC:To: From:Date:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=NMnB0ARmdP778N6mHAZ1T0Isjzjk4NPT25+O0JS7mqM=; b=FQO2qGgTDdU2g0Wt0rrCB5hlBV 26uw78AAxCPxuF4t3DyfMOAQ3m+WGER9JeetKW8aM5onWDSLmc7/h3N50UlDVpzRPE89XKKZPQlUg uOeRn0DM9/nKBF42YrC+0iwvq9kYZzxehDPkfTpc5SsFqC2Izwkp9u1IjSOyabupw7z1fGNEoNtpl N1+b6vJ6IPGKFL9FYCiT8L2my05TMxQogEvhBEVLZy5Qhgnd5iQ8OzHnSn8RvFa1lxtMbL+FJMfOJ ciYHzZ3S1aYzObxfxOwJ1kdiok/KFAOvOYWqw0w6vDc6Y+Q8mFh4ccZyOjde85yVC1J5lmlR1Yayo /OWQFgUA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.99.1 #2 (Red Hat Linux)) id 1wvy80-00000006HQF-0WYC; Mon, 17 Aug 2026 14:15:52 +0000 Received: from devianza.investici.org ([198.167.222.108]) by bombadil.infradead.org with esmtps (Exim 4.99.1 #2 (Red Hat Linux)) id 1wvy7x-00000006HPP-0OvD for linux-arm-kernel@lists.infradead.org; Mon, 17 Aug 2026 14:15:51 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=grrlz.net; s=stigmate; t=1786976143; bh=NMnB0ARmdP778N6mHAZ1T0Isjzjk4NPT25+O0JS7mqM=; h=Date:From:To:CC:Subject:In-Reply-To:References:From; b=kpW6BFVzzvYjW2EkZIqfCHXv3Utuu2udlmKuto2Lj5+0+qpBoeGlcKVY6tFHwd9iM e4F52Q0Yv9Zx99AZKdEGNVJPGbrArWXpxlo8SXTwOdtS8nFHttNoqeD1U+XQX5mCM7 rMwwzW3dpQjxuX548C7hr4hXIkKlgQeTG+3O4QbY= Received: from mx2.investici.org (unknown [127.0.0.1]) by devianza.investici.org (Postfix) with ESMTP id 4hNvxq3DYpz6vKY; Mon, 17 Aug 2026 14:15:43 +0000 (UTC) Received: by mx2.investici.org (Postfix) id 4hNvxq05bRz4y6W; Mon, 17 Aug 2026 14:15:42 +0000 (UTC) Date: Mon, 17 Aug 2026 15:15:44 +0100 From: Bradley Morgan To: Vladimir Murzin , Will Deacon CC: Catalin Marinas , Mark Rutland , Pasha Tatashin , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, maz@kernel.org, james.morse@arm.com Subject: =?US-ASCII?Q?Re=3A_=5BPATCH=5D_arm64=3A_hibernate=3A_pass_H?= =?US-ASCII?Q?VC=5FSET=5FVECTORS_args_to_the_resume_hvc?= In-Reply-To: <7ca33f92-2ded-4749-ab67-2fb564919b99@arm.com> References: <20260809213615.11646-1-include@grrlz.net> <463DA5C2-C933-45EC-A200-21AAF6258418@grrlz.net> <7ca33f92-2ded-4749-ab67-2fb564919b99@arm.com> Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.9.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20260817_071549_428471_F92E8FBC X-CRM114-Status: GOOD ( 26.70 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 17 August 2026 14:06:20 BST, Vladimir Murzin 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 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 >>>>> --- >>>>> 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 > 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!