From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1752260AbdLAL6m (ORCPT <rfc822;w@1wt.eu>);
        Fri, 1 Dec 2017 06:58:42 -0500
Received: from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70]:40010 "EHLO
        foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1751989AbdLAL6l (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Fri, 1 Dec 2017 06:58:41 -0500
Date: Fri, 1 Dec 2017 11:58:36 +0000
From: Mark Rutland <mark.rutland@arm.com>
To: Will Deacon <will.deacon@arm.com>
Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        catalin.marinas@arm.com, ard.biesheuvel@linaro.org,
        sboyd@codeaurora.org, dave.hansen@linux.intel.com,
        keescook@chromium.org, msalter@redhat.com, labbott@redhat.com,
        tglx@linutronix.de
Subject: Re: [PATCH v2 12/18] arm64: entry: Explicitly pass exception level
 to kernel_ventry macro
Message-ID: <20171201115836.jaxzbjsnpnuau56s@lakrids.cambridge.arm.com>
References: <1512059986-21325-1-git-send-email-will.deacon@arm.com>
 <1512059986-21325-13-git-send-email-will.deacon@arm.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1512059986-21325-13-git-send-email-will.deacon@arm.com>
User-Agent: NeoMutt/20170113 (1.7.2)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, Nov 30, 2017 at 04:39:40PM +0000, Will Deacon wrote:
> We will need to treat exceptions from EL0 differently in kernel_ventry,
> so rework the macro to take the exception level as an argument and
> construct the branch target using that.
> 
> Signed-off-by: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm64/kernel/entry.S | 46 +++++++++++++++++++++++-----------------------
>  1 file changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index dea196f287a0..688e52f65a8d 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -71,7 +71,7 @@
>  #define BAD_FIQ		2
>  #define BAD_ERROR	3
>  
> -	.macro kernel_ventry	label
> +	.macro kernel_ventry, el, label, regsize = 64
>  	.align 7
>  	sub	sp, sp, #S_FRAME_SIZE
>  #ifdef CONFIG_VMAP_STACK
> @@ -84,7 +84,7 @@
>  	tbnz	x0, #THREAD_SHIFT, 0f
>  	sub	x0, sp, x0			// x0'' = sp' - x0' = (sp + x0) - sp = x0
>  	sub	sp, sp, x0			// sp'' = sp' - x0 = (sp + x0) - x0 = sp
> -	b	\label
> +	b	el\()\el\()_\label
>  
>  0:
>  	/*
> @@ -116,7 +116,7 @@
>  	sub	sp, sp, x0
>  	mrs	x0, tpidrro_el0
>  #endif
> -	b	\label
> +	b	el\()\el\()_\label
>  	.endm
>  
>  	.macro	kernel_entry, el, regsize = 64
> @@ -369,31 +369,31 @@ tsk	.req	x28		// current thread_info
>  
>  	.align	11
>  ENTRY(vectors)
> -	kernel_ventry	el1_sync_invalid		// Synchronous EL1t
> -	kernel_ventry	el1_irq_invalid			// IRQ EL1t
> -	kernel_ventry	el1_fiq_invalid			// FIQ EL1t
> -	kernel_ventry	el1_error_invalid		// Error EL1t
> +	kernel_ventry	1, sync_invalid			// Synchronous EL1t
> +	kernel_ventry	1, irq_invalid			// IRQ EL1t
> +	kernel_ventry	1, fiq_invalid			// FIQ EL1t
> +	kernel_ventry	1, error_invalid		// Error EL1t

Using the el paramter to build the branch name has the unfortunate
property of obscuring the branch name. For example, that makes it
difficult to jump around the entry asm with ctags, which is somewhat
painful.

Could we leave the full branch name in place, e.g.

	kernel_ventry	1, el1_sync_invalid		// Synchronous EL1t
	kernel_ventry	1, el1_irq_invalid		// IRQ EL1t
	kernel_ventry	1, el1_fiq_invalid		// FIQ EL1t
	kernel_ventry	1, el1_error_invalid		// Error EL1t

... or have separate kernel_ventry and user_ventry macros that
implicitly encoded the source EL, also leaving the label name as-is.

Otherwise, this looks fine to me.

Thanks,
Mark.