[PATCH v2 6/6] powerpc/64: Add tests for out-of-line static calls

Christophe Leroy christophe.leroy at csgroup.eu
Tue Sep 27 00:55:59 AEST 2022



Le 26/09/2022 à 08:43, Benjamin Gray a écrit :
> KUnit tests for the various combinations of caller/trampoline/target and
> kernel/module. They must be run from a module loaded at runtime to
> guarantee they have a different TOC to the kernel.
> 
> The tests try to mitigate the chance of panicing by restoring the
> TOC after every static call. Not all possible errors can be caught
> by this (we can't stop a trampoline from using a bad TOC itself),
> but it makes certain errors easier to debug.
> 
> Signed-off-by: Benjamin Gray <bgray at linux.ibm.com>
> ---
>   arch/powerpc/Kconfig                   |  10 +
>   arch/powerpc/kernel/Makefile           |   1 +
>   arch/powerpc/kernel/static_call.c      |  61 ++++++
>   arch/powerpc/kernel/static_call_test.c | 251 +++++++++++++++++++++++++
>   arch/powerpc/kernel/static_call_test.h |  56 ++++++
>   5 files changed, 379 insertions(+)
>   create mode 100644 arch/powerpc/kernel/static_call_test.c
>   create mode 100644 arch/powerpc/kernel/static_call_test.h
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index e7a66635eade..0ca60514c0e2 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -1023,6 +1023,16 @@ config PPC_RTAS_FILTER
>   	  Say Y unless you know what you are doing and the filter is causing
>   	  problems for you.
>   
> +config PPC_STATIC_CALL_KUNIT_TEST
> +	tristate "KUnit tests for PPC64 ELF ABI V2 static calls"
> +	default KUNIT_ALL_TESTS
> +	depends on HAVE_STATIC_CALL && PPC64_ELF_ABI_V2 && KUNIT && m

Is there a reason why it is dedicated to PPC64 ? In that case, can you 
make it explicit with the name of the config option, and with the name 
of the file below ?

> +	help
> +	  Tests that check the TOC is kept consistent across all combinations
> +	  of caller/trampoline/target being kernel/module. Must be built as a
> +	  module and loaded at runtime to ensure the module has a different
> +	  TOC to the kernel.
> +
>   endmenu
>   
>   config ISA_DMA_API
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index a30d0d0f5499..22c07e3d34df 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -131,6 +131,7 @@ obj-$(CONFIG_RELOCATABLE)	+= reloc_$(BITS).o
>   obj-$(CONFIG_PPC32)		+= entry_32.o setup_32.o early_32.o
>   obj-$(CONFIG_PPC64)		+= dma-iommu.o iommu.o
>   obj-$(CONFIG_HAVE_STATIC_CALL)	+= static_call.o
> +obj-$(CONFIG_PPC_STATIC_CALL_KUNIT_TEST)	+= static_call_test.o
>   obj-$(CONFIG_KGDB)		+= kgdb.o
>   obj-$(CONFIG_BOOTX_TEXT)	+= btext.o
>   obj-$(CONFIG_SMP)		+= smp.o
> diff --git a/arch/powerpc/kernel/static_call.c b/arch/powerpc/kernel/static_call.c
> index ecbb74e1b4d3..8d338917b70e 100644
> --- a/arch/powerpc/kernel/static_call.c
> +++ b/arch/powerpc/kernel/static_call.c
> @@ -113,3 +113,64 @@ void arch_static_call_transform(void *site, void *tramp, void *func, bool tail)
>   		panic("%s: patching failed %pS at %pS\n", __func__, func, tramp);
>   }
>   EXPORT_SYMBOL_GPL(arch_static_call_transform);
> +
> +
> +#if IS_MODULE(CONFIG_PPC_STATIC_CALL_KUNIT_TEST)
> +
> +#include "static_call_test.h"
> +
> +int ppc_sc_kernel_target_1(struct kunit* test)
> +{
> +	toc_fixup(test);
> +	return 1;
> +}
> +
> +int ppc_sc_kernel_target_2(struct kunit* test)
> +{
> +	toc_fixup(test);
> +	return 2;
> +}
> +
> +DEFINE_STATIC_CALL(ppc_sc_kernel, ppc_sc_kernel_target_1);
> +
> +int ppc_sc_kernel_call(struct kunit* test)
> +{
> +	return PROTECTED_SC(test, int, static_call(ppc_sc_kernel)(test));
> +}
> +
> +int ppc_sc_kernel_call_indirect(struct kunit* test, int (*fn)(struct kunit*))
> +{
> +	return PROTECTED_SC(test, int, fn(test));
> +}
> +
> +long ppc_sc_kernel_target_big(struct kunit* test,
> +			      long a,
> +			      long b,
> +			      long c,
> +			      long d,
> +			      long e,
> +			      long f,
> +			      long g,
> +			      long h,
> +			      long i)
> +{
> +	toc_fixup(test);
> +	KUNIT_EXPECT_EQ(test, a, b);
> +	KUNIT_EXPECT_EQ(test, a, c);
> +	KUNIT_EXPECT_EQ(test, a, d);
> +	KUNIT_EXPECT_EQ(test, a, e);
> +	KUNIT_EXPECT_EQ(test, a, f);
> +	KUNIT_EXPECT_EQ(test, a, g);
> +	KUNIT_EXPECT_EQ(test, a, h);
> +	KUNIT_EXPECT_EQ(test, a, i);
> +	return ~a;
> +}
> +
> +EXPORT_SYMBOL_GPL(ppc_sc_kernel_target_1);
> +EXPORT_SYMBOL_GPL(ppc_sc_kernel_target_2);
> +EXPORT_SYMBOL_GPL(ppc_sc_kernel_target_big);
> +EXPORT_STATIC_CALL_GPL(ppc_sc_kernel);
> +EXPORT_SYMBOL_GPL(ppc_sc_kernel_call);
> +EXPORT_SYMBOL_GPL(ppc_sc_kernel_call_indirect);
> +
> +#endif /* IS_MODULE(CONFIG_PPC_STATIC_CALL_KUNIT_TEST) */
> diff --git a/arch/powerpc/kernel/static_call_test.c b/arch/powerpc/kernel/static_call_test.c
> new file mode 100644
> index 000000000000..2d69524d935f
> --- /dev/null
> +++ b/arch/powerpc/kernel/static_call_test.c
> @@ -0,0 +1,251 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include "static_call_test.h"
> +
> +#include <linux/kconfig.h>
> +#include <linux/module.h>
> +#include <linux/static_call.h>
> +
> +/*
> + * Tests to ensure correctness in a variety of cases for static calls.
> + *
> + * The tests focus on ensuring the TOC is kept consistent across the
> + * module-kernel boundary, as compilers can't see that a trampoline
> + * defined locally to a caller might be jumping to a function with a
> + * different TOC. So it's important that these tests are compiled as
> + * a module to ensure the TOC will be different to the kernel's.
> + */
> +
> +/* Utils to hold a copy of the old register values while we test.
> + *
> + * We can't use the KUnit init/exit hooks because when the hooks and
> + * test cases return they will be in the KUnit context that doesn't know
> + * we've reserved and modified some non-volatile registers.
> + */
> +static void* regsaves[3];
> +
> +#define SAVE_REGS() \
> +	regsaves[0] = actual_toc; \
> +	regsaves[1] = module_toc; \
> +	regsaves[2] = kernel_toc; \
> +	module_toc = current_toc; \
> +	kernel_toc = (void*) get_paca()->kernel_toc;
> +
> +#define RESTORE_REGS() \
> +	actual_toc = regsaves[0]; \
> +	module_toc = regsaves[1]; \
> +	kernel_toc = regsaves[2];
> +
> +static int module_target_11(struct kunit *test)
> +{
> +	toc_fixup(test);
> +	return 11;
> +}
> +
> +static int module_target_12(struct kunit *test)
> +{
> +	toc_fixup(test);
> +	return 12;
> +}
> +
> +DEFINE_STATIC_CALL(module_sc, module_target_11);
> +
> +DEFINE_STATIC_CALL_RET0(module_sc_ret0, int(void));
> +DEFINE_STATIC_CALL_NULL(module_sc_null, int(int));
> +
> +static int add_one(int *val)
> +{
> +	return (*val)++;
> +}
> +
> +static void null_function_test(struct kunit *test)
> +{
> +	int val = 0;
> +
> +	SAVE_REGS();
> +
> +	/* Check argument unconditionally evaluated */
> +	static_call_cond(module_sc_null)(add_one(&val));
> +	KUNIT_ASSERT_EQ(test, 1, val);
> +
> +	RESTORE_REGS();
> +}
> +
> +static void return_zero_test(struct kunit *test)
> +{
> +	int ret;
> +
> +	SAVE_REGS();
> +
> +	ret = PROTECTED_SC(test, int, static_call(module_sc_ret0)());
> +	KUNIT_ASSERT_EQ(test, 0, ret);
> +
> +	static_call_update(ppc_sc_kernel, (void*)__static_call_return0);
> +	ret = PROTECTED_SC(test, int, static_call(ppc_sc_kernel)(test));
> +	KUNIT_ASSERT_EQ(test, 0, ret);
> +
> +	static_call_update(module_sc, (void*)__static_call_return0);
> +	ret = PROTECTED_SC(test, int, static_call(module_sc)(test));
> +	KUNIT_ASSERT_EQ(test, 0, ret);
> +
> +	RESTORE_REGS();
> +}
> +
> +static void kernel_kernel_kernel_test(struct kunit *test)
> +{
> +	SAVE_REGS();
> +
> +	static_call_update(ppc_sc_kernel, ppc_sc_kernel_target_1);
> +	KUNIT_ASSERT_EQ(test, 1, ppc_sc_kernel_call(test));
> +
> +	static_call_update(ppc_sc_kernel, ppc_sc_kernel_target_2);
> +	KUNIT_ASSERT_EQ(test, 2, ppc_sc_kernel_call(test));
> +
> +	RESTORE_REGS();
> +}
> +
> +static void kernel_kernel_module_test(struct kunit *test)
> +{
> +	SAVE_REGS();
> +
> +	static_call_update(ppc_sc_kernel, module_target_11);
> +	KUNIT_ASSERT_EQ(test, 11, ppc_sc_kernel_call(test));
> +
> +	static_call_update(ppc_sc_kernel, module_target_12);
> +	KUNIT_ASSERT_EQ(test, 12, ppc_sc_kernel_call(test));
> +
> +	RESTORE_REGS();
> +}
> +
> +static void kernel_module_kernel_test(struct kunit *test)
> +{
> +	SAVE_REGS();
> +
> +	static_call_update(module_sc, ppc_sc_kernel_target_1);
> +	KUNIT_ASSERT_EQ(test, 1, ppc_sc_kernel_call_indirect(test, static_call(module_sc)));
> +
> +	static_call_update(module_sc, ppc_sc_kernel_target_2);
> +	KUNIT_ASSERT_EQ(test, 2, ppc_sc_kernel_call_indirect(test, static_call(module_sc)));
> +
> +	RESTORE_REGS();
> +}
> +
> +static void kernel_module_module_test(struct kunit *test)
> +{
> +	SAVE_REGS();
> +
> +	static_call_update(module_sc, module_target_11);
> +	KUNIT_ASSERT_EQ(test, 11, ppc_sc_kernel_call_indirect(test, static_call(module_sc)));
> +
> +	static_call_update(module_sc, module_target_12);
> +	KUNIT_ASSERT_EQ(test, 12, ppc_sc_kernel_call_indirect(test, static_call(module_sc)));
> +
> +	RESTORE_REGS();
> +}
> +
> +static void module_kernel_kernel_test(struct kunit *test)
> +{
> +	int ret;
> +
> +	SAVE_REGS();
> +
> +	static_call_update(ppc_sc_kernel, ppc_sc_kernel_target_1);
> +	ret = PROTECTED_SC(test, int, static_call(ppc_sc_kernel)(test));
> +	KUNIT_ASSERT_EQ(test, 1, ret);
> +
> +	static_call_update(ppc_sc_kernel, ppc_sc_kernel_target_2);
> +	ret = PROTECTED_SC(test, int, static_call(ppc_sc_kernel)(test));
> +	KUNIT_ASSERT_EQ(test, 2, ret);
> +
> +	RESTORE_REGS();
> +}
> +
> +static void module_kernel_module_test(struct kunit *test)
> +{
> +	int ret;
> +
> +	SAVE_REGS();
> +
> +	static_call_update(ppc_sc_kernel, module_target_11);
> +	ret = PROTECTED_SC(test, int, static_call(ppc_sc_kernel)(test));
> +	KUNIT_ASSERT_EQ(test, 11, ret);
> +
> +	static_call_update(ppc_sc_kernel, module_target_12);
> +	ret = PROTECTED_SC(test, int, static_call(ppc_sc_kernel)(test));
> +	KUNIT_ASSERT_EQ(test, 12, ret);
> +
> +	RESTORE_REGS();
> +}
> +
> +static void module_module_kernel_test(struct kunit *test)
> +{
> +	int ret;
> +
> +	SAVE_REGS();
> +
> +	static_call_update(module_sc, ppc_sc_kernel_target_1);
> +	ret = PROTECTED_SC(test, int, static_call(module_sc)(test));
> +	KUNIT_ASSERT_EQ(test, 1, ret);
> +
> +	static_call_update(module_sc, ppc_sc_kernel_target_2);
> +	ret = PROTECTED_SC(test, int, static_call(module_sc)(test));
> +	KUNIT_ASSERT_EQ(test, 2, ret);
> +
> +	RESTORE_REGS();
> +}
> +
> +static void module_module_module_test(struct kunit *test)
> +{
> +	int ret;
> +
> +	SAVE_REGS();
> +
> +	static_call_update(module_sc, module_target_11);
> +	ret = PROTECTED_SC(test, int, static_call(module_sc)(test));
> +	KUNIT_ASSERT_EQ(test, 11, ret);
> +
> +	static_call_update(module_sc, module_target_12);
> +	ret = PROTECTED_SC(test, int, static_call(module_sc)(test));
> +	KUNIT_ASSERT_EQ(test, 12, ret);
> +
> +	RESTORE_REGS();
> +}
> +
> +DEFINE_STATIC_CALL(module_sc_stack_params, ppc_sc_kernel_target_big);
> +
> +static void stack_parameters_test(struct kunit *test)
> +{
> +	long m = 0x1234567887654321;
> +	long ret;
> +
> +	SAVE_REGS();
> +
> +	ret = PROTECTED_SC(test, long, static_call(module_sc_stack_params)(test, m, m, m, m, m, m, m, m, m));
> +	KUNIT_ASSERT_EQ(test, ~m, ret);
> +
> +	RESTORE_REGS();
> +}
> +
> +static struct kunit_case static_call_test_cases[] = {
> +	KUNIT_CASE(null_function_test),
> +	KUNIT_CASE(return_zero_test),
> +	KUNIT_CASE(stack_parameters_test),
> +	KUNIT_CASE(kernel_kernel_kernel_test),
> +	KUNIT_CASE(kernel_kernel_module_test),
> +	KUNIT_CASE(kernel_module_kernel_test),
> +	KUNIT_CASE(kernel_module_module_test),
> +	KUNIT_CASE(module_kernel_kernel_test),
> +	KUNIT_CASE(module_kernel_module_test),
> +	KUNIT_CASE(module_module_kernel_test),
> +	KUNIT_CASE(module_module_module_test),
> +	{}
> +};
> +
> +static struct kunit_suite ppc_static_call_test_suite = {
> +	.name = "ppc-static-call",
> +	.test_cases = static_call_test_cases,
> +};
> +kunit_test_suite(ppc_static_call_test_suite);
> +
> +MODULE_AUTHOR("Benjamin Gray <bgray at linux.ibm.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/arch/powerpc/kernel/static_call_test.h b/arch/powerpc/kernel/static_call_test.h
> new file mode 100644
> index 000000000000..597da89297fa
> --- /dev/null
> +++ b/arch/powerpc/kernel/static_call_test.h
> @@ -0,0 +1,56 @@
> +#ifndef _POWERPC_STATIC_CALL_TEST_
> +#define _POWERPC_STATIC_CALL_TEST_
> +
> +#include <kunit/test.h>
> +
> +/* Reserve these registers for testing so that a TOC error
> + * doesn't necessarily crash the whole kernel.
> + *
> + * The tests ensure the contents are restored before returning.
> + */
> +register void* current_toc asm ("r2");
> +register void* actual_toc asm ("r14");  /* Copy of TOC before fixup */
> +register void* module_toc asm ("r15");
> +register void* kernel_toc asm ("r16");
> +
> +DECLARE_STATIC_CALL(ppc_sc_kernel, int(struct kunit*));
> +int ppc_sc_kernel_target_1(struct kunit* test);
> +int ppc_sc_kernel_target_2(struct kunit* test);
> +long ppc_sc_kernel_target_big(struct kunit* test,
> +				     long a,
> +				     long b,
> +				     long c,
> +				     long d,
> +				     long e,
> +				     long f,
> +				     long g,
> +				     long h,
> +				     long i);
> +int ppc_sc_kernel_call(struct kunit* test);
> +int ppc_sc_kernel_call_indirect(struct kunit* test, int(*fn)(struct kunit*));
> +
> +#ifdef MODULE
> +
> +#define toc_fixup(test) \
> +	actual_toc = current_toc; \
> +	current_toc = module_toc; \
> +	KUNIT_EXPECT_PTR_EQ(test, module_toc, actual_toc)
> +
> +#else /* KERNEL */
> +
> +#define toc_fixup(test) \
> +	actual_toc = current_toc; \
> +	current_toc = kernel_toc; \
> +	KUNIT_EXPECT_PTR_EQ(test, kernel_toc, actual_toc)
> +
> +#endif /* MODULE */
> +
> +#define PROTECTED_SC(test, ret_type, call) \
> +({ \
> +	ret_type ret; \
> +	ret = call; \
> +	toc_fixup(test); \
> +	ret; \
> +})
> +
> +#endif /* _POWERPC_STATIC_CALL_TEST_ */


More information about the Linuxppc-dev mailing list