| Message ID | 20220825102025.53916-3-joey.gouly@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Use CNTVCTSS_EL0 in gettimeofday() | expand |
On Thu, Aug 25, 2022 at 11:20:24AM +0100, Joey Gouly wrote: > Make it possible to use alternatives in the vDSO, so that better > implementations can be used if possible. > > Signed-off-by: Joey Gouly <joey.gouly@arm.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will@kernel.org> > Cc: Vincenzo Frascino <vincenzo.frascino@arm.com> > Cc: Mark Rutland <mark.rutland@arm.com> > --- > arch/arm64/include/asm/vdso.h | 3 +++ > arch/arm64/kernel/alternative.c | 25 +++++++++++++++++++++++++ > arch/arm64/kernel/vdso.c | 3 --- > arch/arm64/kernel/vdso/vdso.lds.S | 7 +++++++ > 4 files changed, 35 insertions(+), 3 deletions(-) > > diff --git a/arch/arm64/include/asm/vdso.h b/arch/arm64/include/asm/vdso.h > index f99dcb94b438..b4ae32109932 100644 > --- a/arch/arm64/include/asm/vdso.h > +++ b/arch/arm64/include/asm/vdso.h > @@ -26,6 +26,9 @@ > (void *)(vdso_offset_##name - VDSO_LBASE + (unsigned long)(base)); \ > }) > > +extern char vdso_start[], vdso_end[]; > +extern char vdso32_start[], vdso32_end[]; > + > #endif /* !__ASSEMBLY__ */ > > #endif /* __ASM_VDSO_H */ > diff --git a/arch/arm64/kernel/alternative.c b/arch/arm64/kernel/alternative.c > index 9bcaa5eacf16..a4036194a5cd 100644 > --- a/arch/arm64/kernel/alternative.c > +++ b/arch/arm64/kernel/alternative.c > @@ -10,11 +10,14 @@ > > #include <linux/init.h> > #include <linux/cpu.h> > +#include <linux/elf.h> > #include <asm/cacheflush.h> > #include <asm/alternative.h> > #include <asm/cpufeature.h> > #include <asm/insn.h> > +#include <asm/module.h> > #include <asm/sections.h> > +#include <asm/vdso.h> > #include <linux/stop_machine.h> > > #define __ALT_PTR(a, f) ((void *)&(a)->f + (a)->f) > @@ -192,6 +195,27 @@ static void __nocfi __apply_alternatives(struct alt_region *region, bool is_modu > } > } > > +void apply_alternatives_vdso(unsigned long *feature_mask) > +{ > + struct alt_region region; > + const struct elf64_hdr *hdr; > + const struct elf64_shdr *shdr; > + const struct elf64_shdr *alt; > + > + hdr = (struct elf64_hdr *)vdso_start; > + shdr = (void *)hdr + hdr->e_shoff; > + alt = find_section(hdr, shdr, ".altinstructions"); > + if (!alt) > + return; > + > + region = (struct alt_region){ > + .begin = (void *)hdr + alt->sh_offset, > + .end = (void *)hdr + alt->sh_offset + alt->sh_size, > + }; > + > + __apply_alternatives(®ion, false, feature_mask); > +} > + > /* > * We might be patching the stop_machine state machine, so implement a > * really simple polling protocol here. > @@ -216,6 +240,7 @@ static int __apply_alternatives_multi_stop(void *unused) > > BUG_ON(all_alternatives_applied); > __apply_alternatives(®ion, false, remaining_capabilities); > + apply_alternatives_vdso(remaining_capabilities); Since we didn't patch the vdso in apply_boot_alternatives(), using `remaining_capabilities` means that we could in theory miss alternatives for features which were detected on the boot CPU. Since the VDSO cannot be concurrently executed within the kernel, we could hoist the call to apply_alternatives_vdso() out of __apply_alternatives_multi_stop(), and call it before the stop_machine in apply_alternatives_all(). That would keep __apply_alternatives_multi_stop() simple (and easier to make noinstr-safe), and we could use the same mask logic as apply_alternatives_module(), e.g. void apply_alternatives_vdso(void) { DECLARE_BITMAP(all_capabilities, ARM64_NPATCHABLE); bitmap_fill(all_capabilities, ARM64_NPATCHABLE); struct alt_region region; const struct elf64_hdr *hdr; const struct elf64_shdr *shdr; const struct elf64_shdr *alt; hdr = (struct elf64_hdr *)vdso_start; shdr = (void *)hdr + hdr->e_shoff; alt = find_section(hdr, shdr, ".altinstructions"); if (!alt) return; region = (struct alt_region){ .begin = (void *)hdr + alt->sh_offset, .end = (void *)hdr + alt->sh_offset + alt->sh_size, }; __apply_alternatives(®ion, false, &all_capabilities[0]); } ... does that sound ok to you? Thanks, Mark. > /* Barriers provided by the cache flushing */ > all_alternatives_applied = 1; > } > diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c > index a61fc4f989b3..ac93a2ee9c07 100644 > --- a/arch/arm64/kernel/vdso.c > +++ b/arch/arm64/kernel/vdso.c > @@ -29,9 +29,6 @@ > #include <asm/signal32.h> > #include <asm/vdso.h> > > -extern char vdso_start[], vdso_end[]; > -extern char vdso32_start[], vdso32_end[]; > - > enum vdso_abi { > VDSO_ABI_AA64, > VDSO_ABI_AA32, > diff --git a/arch/arm64/kernel/vdso/vdso.lds.S b/arch/arm64/kernel/vdso/vdso.lds.S > index e69fb4aaaf3e..6028f1fe2d1c 100644 > --- a/arch/arm64/kernel/vdso/vdso.lds.S > +++ b/arch/arm64/kernel/vdso/vdso.lds.S > @@ -48,6 +48,13 @@ SECTIONS > PROVIDE (_etext = .); > PROVIDE (etext = .); > > + . = ALIGN(4); > + .altinstructions : { > + __alt_instructions = .; > + *(.altinstructions) > + __alt_instructions_end = .; > + } > + > .dynamic : { *(.dynamic) } :text :dynamic > > .rela.dyn : ALIGN(8) { *(.rela .rela*) } > -- > 2.17.1 >
Hi Mark, Thanks for the review! On Thu, Aug 25, 2022 at 02:19:32PM +0100, Mark Rutland wrote: > On Thu, Aug 25, 2022 at 11:20:24AM +0100, Joey Gouly wrote: > > Make it possible to use alternatives in the vDSO, so that better > > implementations can be used if possible. > > > > Signed-off-by: Joey Gouly <joey.gouly@arm.com> > > Cc: Catalin Marinas <catalin.marinas@arm.com> > > Cc: Will Deacon <will@kernel.org> > > Cc: Vincenzo Frascino <vincenzo.frascino@arm.com> > > Cc: Mark Rutland <mark.rutland@arm.com> > > --- > > arch/arm64/include/asm/vdso.h | 3 +++ > > arch/arm64/kernel/alternative.c | 25 +++++++++++++++++++++++++ > > arch/arm64/kernel/vdso.c | 3 --- > > arch/arm64/kernel/vdso/vdso.lds.S | 7 +++++++ > > 4 files changed, 35 insertions(+), 3 deletions(-) > > [..] > > +void apply_alternatives_vdso(unsigned long *feature_mask) > > +{ > > + struct alt_region region; > > + const struct elf64_hdr *hdr; > > + const struct elf64_shdr *shdr; > > + const struct elf64_shdr *alt; > > + > > + hdr = (struct elf64_hdr *)vdso_start; > > + shdr = (void *)hdr + hdr->e_shoff; > > + alt = find_section(hdr, shdr, ".altinstructions"); > > + if (!alt) > > + return; > > + > > + region = (struct alt_region){ > > + .begin = (void *)hdr + alt->sh_offset, > > + .end = (void *)hdr + alt->sh_offset + alt->sh_size, > > + }; > > + > > + __apply_alternatives(®ion, false, feature_mask); > > +} > > + > > /* > > * We might be patching the stop_machine state machine, so implement a > > * really simple polling protocol here. > > @@ -216,6 +240,7 @@ static int __apply_alternatives_multi_stop(void *unused) > > > > BUG_ON(all_alternatives_applied); > > __apply_alternatives(®ion, false, remaining_capabilities); > > + apply_alternatives_vdso(remaining_capabilities); > > Since we didn't patch the vdso in apply_boot_alternatives(), using > `remaining_capabilities` means that we could in theory miss alternatives for > features which were detected on the boot CPU. > > Since the VDSO cannot be concurrently executed within the kernel, we could > hoist the call to apply_alternatives_vdso() out of > __apply_alternatives_multi_stop(), and call it before the stop_machine in > apply_alternatives_all(). > > That would keep __apply_alternatives_multi_stop() simple (and easier to make > noinstr-safe), and we could use the same mask logic as > apply_alternatives_module(), e.g. > > void apply_alternatives_vdso(void) > { > DECLARE_BITMAP(all_capabilities, ARM64_NPATCHABLE); > bitmap_fill(all_capabilities, ARM64_NPATCHABLE); > > struct alt_region region; > const struct elf64_hdr *hdr; > const struct elf64_shdr *shdr; > const struct elf64_shdr *alt; > > hdr = (struct elf64_hdr *)vdso_start; > shdr = (void *)hdr + hdr->e_shoff; > alt = find_section(hdr, shdr, ".altinstructions"); > if (!alt) > return; > > region = (struct alt_region){ > .begin = (void *)hdr + alt->sh_offset, > .end = (void *)hdr + alt->sh_offset + alt->sh_size, > }; > > __apply_alternatives(®ion, false, &all_capabilities[0]); > } > > ... does that sound ok to you? Yep, this makes sense, will send a v2 with this and the fixes for the other patch. Thanks, Joey
diff --git a/arch/arm64/include/asm/vdso.h b/arch/arm64/include/asm/vdso.h index f99dcb94b438..b4ae32109932 100644 --- a/arch/arm64/include/asm/vdso.h +++ b/arch/arm64/include/asm/vdso.h @@ -26,6 +26,9 @@ (void *)(vdso_offset_##name - VDSO_LBASE + (unsigned long)(base)); \ }) +extern char vdso_start[], vdso_end[]; +extern char vdso32_start[], vdso32_end[]; + #endif /* !__ASSEMBLY__ */ #endif /* __ASM_VDSO_H */ diff --git a/arch/arm64/kernel/alternative.c b/arch/arm64/kernel/alternative.c index 9bcaa5eacf16..a4036194a5cd 100644 --- a/arch/arm64/kernel/alternative.c +++ b/arch/arm64/kernel/alternative.c @@ -10,11 +10,14 @@ #include <linux/init.h> #include <linux/cpu.h> +#include <linux/elf.h> #include <asm/cacheflush.h> #include <asm/alternative.h> #include <asm/cpufeature.h> #include <asm/insn.h> +#include <asm/module.h> #include <asm/sections.h> +#include <asm/vdso.h> #include <linux/stop_machine.h> #define __ALT_PTR(a, f) ((void *)&(a)->f + (a)->f) @@ -192,6 +195,27 @@ static void __nocfi __apply_alternatives(struct alt_region *region, bool is_modu } } +void apply_alternatives_vdso(unsigned long *feature_mask) +{ + struct alt_region region; + const struct elf64_hdr *hdr; + const struct elf64_shdr *shdr; + const struct elf64_shdr *alt; + + hdr = (struct elf64_hdr *)vdso_start; + shdr = (void *)hdr + hdr->e_shoff; + alt = find_section(hdr, shdr, ".altinstructions"); + if (!alt) + return; + + region = (struct alt_region){ + .begin = (void *)hdr + alt->sh_offset, + .end = (void *)hdr + alt->sh_offset + alt->sh_size, + }; + + __apply_alternatives(®ion, false, feature_mask); +} + /* * We might be patching the stop_machine state machine, so implement a * really simple polling protocol here. @@ -216,6 +240,7 @@ static int __apply_alternatives_multi_stop(void *unused) BUG_ON(all_alternatives_applied); __apply_alternatives(®ion, false, remaining_capabilities); + apply_alternatives_vdso(remaining_capabilities); /* Barriers provided by the cache flushing */ all_alternatives_applied = 1; } diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c index a61fc4f989b3..ac93a2ee9c07 100644 --- a/arch/arm64/kernel/vdso.c +++ b/arch/arm64/kernel/vdso.c @@ -29,9 +29,6 @@ #include <asm/signal32.h> #include <asm/vdso.h> -extern char vdso_start[], vdso_end[]; -extern char vdso32_start[], vdso32_end[]; - enum vdso_abi { VDSO_ABI_AA64, VDSO_ABI_AA32, diff --git a/arch/arm64/kernel/vdso/vdso.lds.S b/arch/arm64/kernel/vdso/vdso.lds.S index e69fb4aaaf3e..6028f1fe2d1c 100644 --- a/arch/arm64/kernel/vdso/vdso.lds.S +++ b/arch/arm64/kernel/vdso/vdso.lds.S @@ -48,6 +48,13 @@ SECTIONS PROVIDE (_etext = .); PROVIDE (etext = .); + . = ALIGN(4); + .altinstructions : { + __alt_instructions = .; + *(.altinstructions) + __alt_instructions_end = .; + } + .dynamic : { *(.dynamic) } :text :dynamic .rela.dyn : ALIGN(8) { *(.rela .rela*) }
Make it possible to use alternatives in the vDSO, so that better implementations can be used if possible. Signed-off-by: Joey Gouly <joey.gouly@arm.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Will Deacon <will@kernel.org> Cc: Vincenzo Frascino <vincenzo.frascino@arm.com> Cc: Mark Rutland <mark.rutland@arm.com> --- arch/arm64/include/asm/vdso.h | 3 +++ arch/arm64/kernel/alternative.c | 25 +++++++++++++++++++++++++ arch/arm64/kernel/vdso.c | 3 --- arch/arm64/kernel/vdso/vdso.lds.S | 7 +++++++ 4 files changed, 35 insertions(+), 3 deletions(-)