| Message ID | 1347385155-11643-3-git-send-email-cyril@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 11 Sep 2012, Cyril Chemparathy wrote: > This patch adds basic sanity tests to ensure that the instruction patching > results in valid instruction encodings. This is done by verifying the output > of the patch process against a vector of assembler generated instructions at > init time. > > Signed-off-by: Cyril Chemparathy <cyril@ti.com> > --- > arch/arm/Kconfig | 12 +++++++ > arch/arm/kernel/runtime-patch.c | 75 +++++++++++++++++++++++++++++++++++++++ > 2 files changed, 87 insertions(+) > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index 36de4ea..bfcd29d 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -207,6 +207,18 @@ config ARM_PATCH_PHYS_VIRT > this feature (eg, building a kernel for a single machine) and > you need to shrink the kernel to the minimal size. > > +config ARM_RUNTIME_PATCH_TEST > + bool "Self test runtime patching mechanism" if ARM_RUNTIME_PATCH > + default y Here you probably want this instead: bool "Self test runtime patching mechanism" default y depends on ARM_RUNTIME_PATCH Otherwise ARM_RUNTIME_PATCH_TEST will be forced to y whenever ARM_RUNTIME_PATCH is unset. That doesn't currently affect the build since the containing .c file is only compiled when ARM_RUNTIME_PATCH is set but that is still not strictly right. [...] > @@ -189,5 +261,8 @@ void __init runtime_patch_kernel(void) > const void *start = &__runtime_patch_table_begin; > const void *end = &__runtime_patch_table_end; > > +#ifdef CONFIG_ARM_RUNTIME_PATCH_TEST > + runtime_patch_test(); > +#endif > BUG_ON(runtime_patch(start, end - start)); I think you shoulld have runtime_patch_test() return a possible error code and use BUG_ON() with it as well. With those minor changes you can add... Reviewed-by: Nicolas Pitre <nico@linaro.org> Nicolas
On 9/21/2012 1:40 PM, Nicolas Pitre wrote: > On Tue, 11 Sep 2012, Cyril Chemparathy wrote: > >> This patch adds basic sanity tests to ensure that the instruction patching >> results in valid instruction encodings. This is done by verifying the output >> of the patch process against a vector of assembler generated instructions at >> init time. >> >> Signed-off-by: Cyril Chemparathy <cyril@ti.com> >> --- >> arch/arm/Kconfig | 12 +++++++ >> arch/arm/kernel/runtime-patch.c | 75 +++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 87 insertions(+) >> >> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig >> index 36de4ea..bfcd29d 100644 >> --- a/arch/arm/Kconfig >> +++ b/arch/arm/Kconfig >> @@ -207,6 +207,18 @@ config ARM_PATCH_PHYS_VIRT >> this feature (eg, building a kernel for a single machine) and >> you need to shrink the kernel to the minimal size. >> >> +config ARM_RUNTIME_PATCH_TEST >> + bool "Self test runtime patching mechanism" if ARM_RUNTIME_PATCH >> + default y > > Here you probably want this instead: > > bool "Self test runtime patching mechanism" > default y > depends on ARM_RUNTIME_PATCH > > Otherwise ARM_RUNTIME_PATCH_TEST will be forced to y whenever > ARM_RUNTIME_PATCH is unset. That doesn't currently affect the build > since the containing .c file is only compiled when ARM_RUNTIME_PATCH is > set but that is still not strictly right. > Indeed. Excellent. Thanks. > [...] >> @@ -189,5 +261,8 @@ void __init runtime_patch_kernel(void) >> const void *start = &__runtime_patch_table_begin; >> const void *end = &__runtime_patch_table_end; >> >> +#ifdef CONFIG_ARM_RUNTIME_PATCH_TEST >> + runtime_patch_test(); >> +#endif >> BUG_ON(runtime_patch(start, end - start)); > > I think you shoulld have runtime_patch_test() return a possible error > code and use BUG_ON() with it as well. > Sure. Will do in v4. > With those minor changes you can add... > > Reviewed-by: Nicolas Pitre <nico@linaro.org> > Thanks.
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 36de4ea..bfcd29d 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -207,6 +207,18 @@ config ARM_PATCH_PHYS_VIRT this feature (eg, building a kernel for a single machine) and you need to shrink the kernel to the minimal size. +config ARM_RUNTIME_PATCH_TEST + bool "Self test runtime patching mechanism" if ARM_RUNTIME_PATCH + default y + help + Select this to enable init time self checking for the runtime kernel + patching mechanism. This enables an ISA specific set of tests that + ensure that the instructions generated by the patch process are + consistent with those generated by the assembler at compile time. + + Only disable this option if you need to shrink the kernel to the + minimal size. + config NEED_MACH_IO_H bool help diff --git a/arch/arm/kernel/runtime-patch.c b/arch/arm/kernel/runtime-patch.c index 28a6367..0be9ef3 100644 --- a/arch/arm/kernel/runtime-patch.c +++ b/arch/arm/kernel/runtime-patch.c @@ -168,6 +168,78 @@ static int apply_patch_imm8(const struct patch_info *p) return 0; } +#ifdef CONFIG_ARM_RUNTIME_PATCH_TEST + +struct patch_test_imm8 { + u16 imm; + u16 shift; + u32 insn; +}; + +static void __init __used __naked __patch_test_code_imm8(void) +{ + __asm__ __volatile__ ( + + /* a single test case */ + " .macro test_one, imm, sft\n" + " .hword \\imm\n" + " .hword \\sft\n" + " add r1, r2, #(\\imm << \\sft)\n" + " .endm\n" + + /* a sequence of tests at 'inc' increments of shift */ + " .macro test_seq, imm, sft, max, inc\n" + " test_one \\imm, \\sft\n" + " .if \\sft < \\max\n" + " test_seq \\imm, (\\sft + \\inc), \\max, \\inc\n" + " .endif\n" + " .endm\n" + + /* an empty record to mark the end */ + " .macro test_end\n" + " .hword 0, 0\n" + " .word 0\n" + " .endm\n" + + /* finally generate the test sequences */ + " test_seq 0x41, 0, 24, 1\n" + " test_seq 0x81, 0, 24, 2\n" + " test_end\n" + : : : "r1", "r2", "cc"); +} + +static void __init test_patch_imm8(void) +{ + u32 test_code_addr = (u32)(&__patch_test_code_imm8); + struct patch_test_imm8 *test = (void *)(test_code_addr & ~1); + u32 ninsn, insn, patched_insn; + int i, err; + + insn = test[0].insn; + for (i = 0; test[i].insn; i++) { + err = do_patch_imm8(insn, test[i].imm << test[i].shift, &ninsn); + __patch_text(&patched_insn, ninsn); + + if (err) { + pr_err("rtpatch imm8: failed at imm %x, shift %d\n", + test[i].imm, test[i].shift); + } else if (patched_insn != test[i].insn) { + pr_err("rtpatch imm8: failed, need %x got %x\n", + test[i].insn, patched_insn); + } else { + pr_debug("rtpatch imm8: imm %x, shift %d, %x -> %x\n", + test[i].imm, test[i].shift, insn, + patched_insn); + } + } +} + +static void __init runtime_patch_test(void) +{ + test_patch_imm8(); +} +#endif + int runtime_patch(const void *table, unsigned size) { const struct patch_info *p = table, *end = (table + size); @@ -189,5 +261,8 @@ void __init runtime_patch_kernel(void) const void *start = &__runtime_patch_table_begin; const void *end = &__runtime_patch_table_end; +#ifdef CONFIG_ARM_RUNTIME_PATCH_TEST + runtime_patch_test(); +#endif BUG_ON(runtime_patch(start, end - start)); }
This patch adds basic sanity tests to ensure that the instruction patching results in valid instruction encodings. This is done by verifying the output of the patch process against a vector of assembler generated instructions at init time. Signed-off-by: Cyril Chemparathy <cyril@ti.com> --- arch/arm/Kconfig | 12 +++++++ arch/arm/kernel/runtime-patch.c | 75 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+)