Re: [RFC 1/6] arm64: hibernate: Introduce new entry point to kernel

From: Marc Zyngier <maz@kernel.org>
To: Vivek Kumar <quic_vivekuma@quicinc.com>
Cc: corbet@lwn.net, catalin.marinas@arm.com, will@kernel.org,
	tglx@linutronix.de, axboe@kernel.dk, rafael@kernel.org,
	akpm@linux-foundation.org, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-block@vger.kernel.org, linux-pm@vger.kernel.org,
	linux-mm@kvack.org, len.brown@intel.com, pavel@ucw.cz,
	paulmck@kernel.org, bp@suse.de, keescook@chromium.org,
	songmuchun@bytedance.com, rdunlap@infradead.org,
	damien.lemoal@opensource.wdc.com, pasha.tatashin@soleen.com,
	tabba@google.com, ardb@kernel.org, tsoni@quicinc.com,
	quic_psodagud@quicinc.com, quic_svaddagi@quicinc.com,
	Prasanna Kumar <quic_kprasan@quicinc.com>
Subject: Re: [RFC 1/6] arm64: hibernate: Introduce new entry point to kernel
Date: Thu, 19 May 2022 16:27:26 +0100	[thread overview]
Message-ID: <1d517a7598f7833196ec0c8258816aba@kernel.org> (raw)
In-Reply-To: <1652860121-24092-2-git-send-email-quic_vivekuma@quicinc.com>

On 2022-05-18 08:48, Vivek Kumar wrote:
> Introduce a new entry point to hibernated kernel image.
> This is generally needed when bootloader restores the
> hibernated image from disc to ddr and passes control
> to it by turning off the mmu, also initialize this new
> entry point with cpu_resume which turns on the mmu and
> then proceeds with restore routines.
> 
> Signed-off-by: Vivek Kumar <quic_vivekuma@quicinc.com>
> Signed-off-by: Prasanna Kumar <quic_kprasan@quicinc.com>
> ---
>  arch/arm64/kernel/hibernate.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm64/kernel/hibernate.c 
> b/arch/arm64/kernel/hibernate.c
> index 6328308..4e294b3 100644
> --- a/arch/arm64/kernel/hibernate.c
> +++ b/arch/arm64/kernel/hibernate.c
> @@ -74,6 +74,14 @@ static struct arch_hibernate_hdr {
>  	void		(*reenter_kernel)(void);
> 
>  	/*
> +	 * Another entry point if jump to kernel happens with mmu disabled,
> +	 * generally done when restoring hibernation image from bootloader
> +	 * context
> +	 */
> +
> +	phys_addr_t	phys_reenter_kernel;
> +
> +	/*
>  	 * We need to know where the __hyp_stub_vectors are after restore to
>  	 * re-configure el2.
>  	 */
> @@ -116,6 +124,7 @@ int arch_hibernation_header_save(void *addr,
> unsigned int max_size)
>  	arch_hdr_invariants(&hdr->invariants);
>  	hdr->ttbr1_el1		= __pa_symbol(swapper_pg_dir);
>  	hdr->reenter_kernel	= _cpu_resume;
> +	hdr->phys_reenter_kernel  = __pa(cpu_resume);
> 
>  	/* We can't use __hyp_get_vectors() because kvm may still be loaded 
> */
>  	if (el2_reset_needed())

So here, you are creating a new ABI with the bootloader, based on
a data structure that isn't mean't to be ABI. It means that we
wouldn't be allowed to ever change this data structure, as this
would mean having to update the bootloader in sync.

Clearly, this isn't acceptable.

         M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel