Re: [PATCH 10/13] arm64: extable: add `type` and `data` fields

From: Will Deacon <will@kernel.org>
To: Mark Rutland <mark.rutland@arm.com>
Cc: linux-arm-kernel@lists.infradead.org, alexandru.elisei@arm.com,
	andrii@kernel.org, ardb@kernel.org, ast@kernel.org,
	broonie@kernel.org, catalin.marinas@arm.com,
	daniel@iogearbox.net, dvyukov@google.com, james.morse@arm.com,
	jean-philippe@linaro.org, jpoimboe@redhat.com, maz@kernel.org,
	peterz@infradead.org, robin.murphy@arm.com,
	suzuki.poulose@arm.com
Subject: Re: [PATCH 10/13] arm64: extable: add `type` and `data` fields
Date: Tue, 19 Oct 2021 12:29:55 +0100	[thread overview]
Message-ID: <20211019112954.GG13251@willie-the-truck> (raw)
In-Reply-To: <20211013110059.10324-11-mark.rutland@arm.com>

On Wed, Oct 13, 2021 at 12:00:56PM +0100, Mark Rutland wrote:
> Subsequent patches will add specialized handlers for fixups, in addition
> to the simple PC fixup and BPF handlers we have today. In preparation,
> this patch adds a new `type` field to struct exception_table_entry, and
> uses this to distinguish the fixup and BPF cases. A `data` field is also
> added so that subsequent patches can associate data specific to each
> exception site (e.g. register numbers).
> 
> Handlers are named ex_handler_*() for consistency, following the exmaple
> of x86. At the same time, get_ex_fixup() is split out into a helper so
> that it can be used by other ex_handler_*() functions ins subsequent
> patches.
> 
> This patch will increase the size of the exception tables, which will be
> remedied by subsequent patches removing redundant fixup code. There
> should be no functional change as a result of this patch.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Andrii Nakryiko <andrii@kernel.org>
> Cc: Ard Biesheuvel <ardb@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: James Morse <james.morse@arm.com>
> Cc: Jean-Philippe Brucker <jean-philippe@linaro.org>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Will Deacon <will@kernel.org>
> ---
>  arch/arm64/include/asm/asm-extable.h | 32 ++++++++++++++++++++------------
>  arch/arm64/include/asm/extable.h     | 19 +++++++++++++++----
>  arch/arm64/mm/extable.c              | 29 +++++++++++++++++++++++++----
>  arch/arm64/net/bpf_jit_comp.c        |  7 +++++--
>  scripts/sorttable.c                  | 30 ++++++++++++++++++++++++++++++
>  5 files changed, 95 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/asm-extable.h b/arch/arm64/include/asm/asm-extable.h
> index 986b4c0d4792..5ee748edaef1 100644
> --- a/arch/arm64/include/asm/asm-extable.h
> +++ b/arch/arm64/include/asm/asm-extable.h
> @@ -2,13 +2,19 @@
>  #ifndef __ASM_ASM_EXTABLE_H
>  #define __ASM_ASM_EXTABLE_H
>  
> +#define EX_TYPE_NONE			0
> +#define EX_TYPE_FIXUP			1
> +#define EX_TYPE_BPF			2
> +
>  #ifdef __ASSEMBLY__
>  
> -#define __ASM_EXTABLE_RAW(insn, fixup)		\
> -	.pushsection	__ex_table, "a";	\
> -	.align		3;			\
> -	.long		((insn) - .);		\
> -	.long		((fixup) - .);		\
> +#define __ASM_EXTABLE_RAW(insn, fixup, type, data)	\
> +	.pushsection	__ex_table, "a";		\
> +	.align		2;				\
> +	.long		((insn) - .);			\
> +	.long		((fixup) - .);			\
> +	.short		(type);				\
> +	.short		(data);				\

Why are you reducing the alignment here?

> diff --git a/scripts/sorttable.c b/scripts/sorttable.c
> index 6ee4fa882919..ee95bb47a50d 100644
> --- a/scripts/sorttable.c
> +++ b/scripts/sorttable.c
> @@ -231,6 +231,34 @@ static void sort_relative_table(char *extab_image, int image_size)
>  	}
>  }
>  
> +static void arm64_sort_relative_table(char *extab_image, int image_size)
> +{
> +	int i = 0;
> +
> +	while (i < image_size) {
> +		uint32_t *loc = (uint32_t *)(extab_image + i);
> +
> +		w(r(loc) + i, loc);
> +		w(r(loc + 1) + i + 4, loc + 1);
> +		/* Don't touch the fixup type or data */
> +
> +		i += sizeof(uint32_t) * 3;
> +	}
> +
> +	qsort(extab_image, image_size / 12, 12, compare_relative_table);
> +
> +	i = 0;
> +	while (i < image_size) {
> +		uint32_t *loc = (uint32_t *)(extab_image + i);
> +
> +		w(r(loc) - i, loc);
> +		w(r(loc + 1) - (i + 4), loc + 1);
> +		/* Don't touch the fixup type or data */
> +
> +		i += sizeof(uint32_t) * 3;
> +	}
> +}

This is very nearly a direct copy of x86_sort_relative_table() (magic
numbers and all). It would be nice to tidy that up, but I couldn't
immediately see a good way to do it :(

Will

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