Re: [PATCH v13 03/18] arm64: hyp-stub: Move el1_sync into the vectors

From: Marc Zyngier <maz@kernel.org>
To: Pavel Tatashin <pasha.tatashin@soleen.com>
Cc: jmorris@namei.org, sashal@kernel.org, ebiederm@xmission.com,
	kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
	corbet@lwn.net, catalin.marinas@arm.com, will@kernel.org,
	linux-arm-kernel@lists.infradead.org, james.morse@arm.com,
	vladimir.murzin@arm.com, matthias.bgg@gmail.com,
	linux-mm@kvack.org, mark.rutland@arm.com, steve.capper@arm.com,
	rfontana@redhat.com, tglx@linutronix.de, selindag@gmail.com,
	tyhicks@linux.microsoft.com, kernelfans@gmail.com
Subject: Re: [PATCH v13 03/18] arm64: hyp-stub: Move el1_sync into the vectors
Date: Thu, 08 Apr 2021 11:24:05 +0100	[thread overview]
Message-ID: <480272f8fb3e46d24a0ee1418fb85809@kernel.org> (raw)
In-Reply-To: <20210408040537.2703241-4-pasha.tatashin@soleen.com>

On 2021-04-08 05:05, Pavel Tatashin wrote:
> From: James Morse <james.morse@arm.com>
> 
> The hyp-stub's el1_sync code doesn't do very much, this can easily fit
> in the vectors.
> 
> With this, all of the hyp-stubs behaviour is contained in its vectors.
> This lets kexec and hibernate copy the hyp-stub when they need its
> behaviour, instead of re-implementing it.
> 
> Signed-off-by: James Morse <james.morse@arm.com>
> 
> [Fixed merging issues]

That's a pretty odd fix IMO.

> 
> Signed-off-by: Pavel Tatashin <pasha.tatashin@soleen.com>
> ---
>  arch/arm64/kernel/hyp-stub.S | 59 ++++++++++++++++++------------------
>  1 file changed, 29 insertions(+), 30 deletions(-)
> 
> diff --git a/arch/arm64/kernel/hyp-stub.S 
> b/arch/arm64/kernel/hyp-stub.S
> index ff329c5c074d..d1a73d0f74e0 100644
> --- a/arch/arm64/kernel/hyp-stub.S
> +++ b/arch/arm64/kernel/hyp-stub.S
> @@ -21,6 +21,34 @@ SYM_CODE_START_LOCAL(\label)
>  	.align 7
>  	b	\label
>  SYM_CODE_END(\label)
> +.endm
> +
> +.macro hyp_stub_el1_sync
> +SYM_CODE_START_LOCAL(hyp_stub_el1_sync)
> +	.align 7
> +	cmp	x0, #HVC_SET_VECTORS
> +	b.ne	2f
> +	msr	vbar_el2, x1
> +	b	9f
> +
> +2:	cmp	x0, #HVC_SOFT_RESTART
> +	b.ne	3f
> +	mov	x0, x2
> +	mov	x2, x4
> +	mov	x4, x1
> +	mov	x1, x3
> +	br	x4				// no return
> +
> +3:	cmp	x0, #HVC_RESET_VECTORS
> +	beq	9f				// Nothing to reset!
> +
> +	/* Someone called kvm_call_hyp() against the hyp-stub... */
> +	mov_q	x0, HVC_STUB_ERR
> +	eret
> +
> +9:	mov	x0, xzr
> +	eret
> +SYM_CODE_END(hyp_stub_el1_sync)

You said you tested this on a TX2. I guess you don't care whether
it runs VHE or not...

         M.

>  .endm
> 
>  	.text
> @@ -39,7 +67,7 @@ SYM_CODE_START(__hyp_stub_vectors)
>  	invalid_vector	hyp_stub_el2h_fiq_invalid	// FIQ EL2h
>  	invalid_vector	hyp_stub_el2h_error_invalid	// Error EL2h
> 
> -	ventry	el1_sync			// Synchronous 64-bit EL1
> +	hyp_stub_el1_sync				// Synchronous 64-bit EL1
>  	invalid_vector	hyp_stub_el1_irq_invalid	// IRQ 64-bit EL1
>  	invalid_vector	hyp_stub_el1_fiq_invalid	// FIQ 64-bit EL1
>  	invalid_vector	hyp_stub_el1_error_invalid	// Error 64-bit EL1
> @@ -55,35 +83,6 @@ SYM_CODE_END(__hyp_stub_vectors)
>  # Check the __hyp_stub_vectors didn't overflow
>  .org . - (__hyp_stub_vectors_end - __hyp_stub_vectors) + SZ_2K
> 
> -
> -SYM_CODE_START_LOCAL(el1_sync)
> -	cmp	x0, #HVC_SET_VECTORS
> -	b.ne	1f
> -	msr	vbar_el2, x1
> -	b	9f
> -
> -1:	cmp	x0, #HVC_VHE_RESTART
> -	b.eq	mutate_to_vhe
> -
> -2:	cmp	x0, #HVC_SOFT_RESTART
> -	b.ne	3f
> -	mov	x0, x2
> -	mov	x2, x4
> -	mov	x4, x1
> -	mov	x1, x3
> -	br	x4				// no return
> -
> -3:	cmp	x0, #HVC_RESET_VECTORS
> -	beq	9f				// Nothing to reset!
> -
> -	/* Someone called kvm_call_hyp() against the hyp-stub... */
> -	mov_q	x0, HVC_STUB_ERR
> -	eret
> -
> -9:	mov	x0, xzr
> -	eret
> -SYM_CODE_END(el1_sync)
> -
>  // nVHE? No way! Give me the real thing!
>  SYM_CODE_START_LOCAL(mutate_to_vhe)
>  	// Sanity check: MMU *must* be off

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