On Tue, Oct 19, 2021 at 12:50:22PM +0100, Mark Rutland wrote:

On Tue, Oct 19, 2021 at 12:29:55PM +0100, Will Deacon wrote:

quoted

On Wed, Oct 13, 2021 at 12:00:56PM +0100, Mark Rutland wrote:

quoted

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 <redacted>
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?

That's because the size of each entry is now 12 bytes, and 
`.align 3` aligns to 8 bytes, which would leave a gap between entries.
We only require the fields are naturally aligned, so `.align 2` is
sufficient, and doesn't waste space.

I'll update the commit message to call that out.

I think the part which is confusing me is that I would expect the alignment
here to match the alignment of the corresponding C type, but the old value
of '3' doesn't seem to do that, so is this patch fixing an earlier bug?

Without your patches in the picture, we're using a '.align 3' in
_asm_extable, but with:

struct exception_table_entry
{
	int insn, fixup;
};

I suppose it works out because that over-alignment doesn't result in any
additional padding, but I think we could reduce the current alignment
without any of these other changes, no?

quoted

quoted

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 :(

Beware that's true in linux-next, but not mainline, as that changes in
commit:

  46d28947d9876fc0 ("x86/extable: Rework the exception table mechanics")

A patch to unify the two is trivial, but will cause a cross-tree
dependency, so I'd suggest having this separate for now and sending a
unification patch come -rc1.

I can note something to that effect in the commit message, if that
helps?

Yeah, I suppose. It's not worth tripping over the x86 changes, but we
should try to remember to come back and unify things.

Will

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