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 B6237C5CFE7 for ; Tue, 11 Aug 2026 14:37:17 +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:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=0mfdfTDuJCfvilSFFKrwBMVPwUNKeCnOVk1kDLav9IE=; b=ytds46R9Owx8S+ke+IjMyiV51o Onab+wqwY1ghZ1W+wQd3loNvX4/xfGOyXARrzYoRx89dLXwNAgQfIIqlHE3dGfXWU0oJx9G3Cwdkm obWhrQEvGbq3hoHJ5wtLPRJLahaPiYV5Y87F1q91FZwq1rzqBJpqZk2X9C20l1LwlG1fRK97XZ8wI K/qBFvYz5PxjjQUZaGVKn6XZdX5X5FktBvPLDIYqxUZXUCTJwIeu0raJwIv1CetyEThw0B+4Am2Xo 53ZiyNuqtlPv38Y87JrvNEK2bQMPwVKcusuPIwKdpt1CFo+DgiQgzqF8F3HEIhAy4JdHbvBOMWSZm y5MSVBQg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.99.1 #2 (Red Hat Linux)) id 1wtnbI-0000000EERM-0vdo; Tue, 11 Aug 2026 14:37:08 +0000 Received: from sea.source.kernel.org ([172.234.252.31]) by bombadil.infradead.org with esmtps (Exim 4.99.1 #2 (Red Hat Linux)) id 1wtnbH-0000000EERE-253a for linux-arm-kernel@lists.infradead.org; Tue, 11 Aug 2026 14:37:07 +0000 Received: from smtp.kernel.org (quasi.space.kernel.org [100.103.45.18]) by sea.source.kernel.org (Postfix) with ESMTP id 160084197E; Tue, 11 Aug 2026 14:37:07 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 45B2D1F00A3D; Tue, 11 Aug 2026 14:37:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1786459027; bh=0mfdfTDuJCfvilSFFKrwBMVPwUNKeCnOVk1kDLav9IE=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=lZ6i4MlLk+uPiOfTJvAMSPRirdhAFHy9i6pEwwOUbNVr+IixR3uDlgCFUCtOd2hb7 QiUJtDp5cvXsI7m+NaRzJ14xY7q6wonT5lxDRbV4YHwCV2FVLfn4Q1e0QMykdXEk2M mcXTZGBHJP88Jcu9/zy3ySQcDFDNtazk7BddZHFEvhADbw4UIr8T6yCXugj0dAYkMZ O1RsUY88EcdfgLeN4r0VpafSZvTWLdO4P+v0mA1K/HTfaiUjRgOd1TGtW4BXLk7cm0 cQNdb1mQ6ipfxZHb5XnyFN9NHlfK/oHidG/uYMjkQeMoABT1KoGnqEaywx+A3rLKm8 KAekgZyZw4FdQ== Date: Tue, 11 Aug 2026 15:37:01 +0100 From: Will Deacon To: Bradley Morgan 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: Re: [PATCH] arm64: hibernate: pass HVC_SET_VECTORS args to the resume hvc Message-ID: References: <20260809213615.11646-1-include@grrlz.net> <463DA5C2-C933-45EC-A200-21AAF6258418@grrlz.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <463DA5C2-C933-45EC-A200-21AAF6258418@grrlz.net> 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 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. > >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