Re: [PATCH v1 2/3] arm64: alternative: patch alternatives in the vDSO

[Joey Gouly <joey.gouly@arm.com>]

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(&region, 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(&region, 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(&region, 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

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