Re: [PATCH 1/3] powerpc: sstep: Add tests for compute type instructions

2019-02-21 Thread Sandipan Das
Hi Michael,

On 21/02/19 4:43 PM, Michael Ellerman wrote:
> Sandipan Das  writes:
>> This enhances the current selftest framework for validating
>> the in-kernel instruction emulation infrastructure by adding
>> support for compute type instructions i.e. integer ALU-based
>> instructions. Originally, this framework was limited to only
>> testing load and store instructions.
>>
>> While most of the GPRs can be validated, support for SPRs is
>> limited to LR, CR and XER for now.
>>
>> When writing the test cases, one must ensure that the Stack
>> Pointer (GPR1) or the Thread Pointer (GPR13) are not touched
>> by any means as these are vital non-volatile registers.
>>
>> Signed-off-by: Sandipan Das 
>> ---
>>  arch/powerpc/lib/Makefile |   3 +-
>>  arch/powerpc/lib/test_emulate_step.c  | 167 +-
>>  .../lib/test_emulate_step_exec_instr.S| 150 
>>  3 files changed, 315 insertions(+), 5 deletions(-)
>>  create mode 100644 arch/powerpc/lib/test_emulate_step_exec_instr.S
> 
> Hi Sandipan,
> 
> Thanks for the exceptionally well written asm, I wish all our asm code
> was that neat and well commented :)
> 
> I'd like to get this merged today so I tweaked it slightly when
> applying to use the new patch_site helpers we added recently, see diff
> below.
> 
> cheers
> 
> 

Thanks for modernizing it :)

- Sandipan



Re: [PATCH 1/3] powerpc: sstep: Add tests for compute type instructions

2019-02-21 Thread Michael Ellerman
Sandipan Das  writes:
> This enhances the current selftest framework for validating
> the in-kernel instruction emulation infrastructure by adding
> support for compute type instructions i.e. integer ALU-based
> instructions. Originally, this framework was limited to only
> testing load and store instructions.
>
> While most of the GPRs can be validated, support for SPRs is
> limited to LR, CR and XER for now.
>
> When writing the test cases, one must ensure that the Stack
> Pointer (GPR1) or the Thread Pointer (GPR13) are not touched
> by any means as these are vital non-volatile registers.
>
> Signed-off-by: Sandipan Das 
> ---
>  arch/powerpc/lib/Makefile |   3 +-
>  arch/powerpc/lib/test_emulate_step.c  | 167 +-
>  .../lib/test_emulate_step_exec_instr.S| 150 
>  3 files changed, 315 insertions(+), 5 deletions(-)
>  create mode 100644 arch/powerpc/lib/test_emulate_step_exec_instr.S

Hi Sandipan,

Thanks for the exceptionally well written asm, I wish all our asm code
was that neat and well commented :)

I'd like to get this merged today so I tweaked it slightly when
applying to use the new patch_site helpers we added recently, see diff
below.

cheers


diff --git a/arch/powerpc/lib/test_emulate_step.c 
b/arch/powerpc/lib/test_emulate_step.c
index 1c13b3bebeca..9992c1ea7a1d 100644
--- a/arch/powerpc/lib/test_emulate_step.c
+++ b/arch/powerpc/lib/test_emulate_step.c
@@ -865,14 +865,14 @@ static int __init emulate_compute_instr(struct pt_regs 
*regs,
 static int __init execute_compute_instr(struct pt_regs *regs,
unsigned int instr)
 {
-   extern unsigned int exec_instr_execute[];
extern int exec_instr(struct pt_regs *regs);
+   extern s32 patch__exec_instr;
 
if (!regs || !instr)
return -EINVAL;
 
/* Patch the NOP with the actual instruction */
-   patch_instruction(_instr_execute[0], instr);
+   patch_instruction_site(__exec_instr, instr);
if (exec_instr(regs)) {
pr_info("execution failed, instruction = 0x%08x\n", instr);
return -EFAULT;
diff --git a/arch/powerpc/lib/test_emulate_step_exec_instr.S 
b/arch/powerpc/lib/test_emulate_step_exec_instr.S
index 84cef7d78d9d..1580f34f4f4f 100644
--- a/arch/powerpc/lib/test_emulate_step_exec_instr.S
+++ b/arch/powerpc/lib/test_emulate_step_exec_instr.S
@@ -8,6 +8,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 /* int exec_instr(struct pt_regs *regs) */
@@ -78,10 +79,9 @@ _GLOBAL(exec_instr)
REST_GPR(12, r31)
REST_NVGPRS(r31)
 
-   .global exec_instr_execute
-exec_instr_execute:
/* Placeholder for the test instruction */
 1: nop
+   patch_site 1b patch__exec_instr
 
/*
 * Since GPR3 is overwritten, temporarily restore it back to its


[PATCH 1/3] powerpc: sstep: Add tests for compute type instructions

2019-02-19 Thread Sandipan Das
This enhances the current selftest framework for validating
the in-kernel instruction emulation infrastructure by adding
support for compute type instructions i.e. integer ALU-based
instructions. Originally, this framework was limited to only
testing load and store instructions.

While most of the GPRs can be validated, support for SPRs is
limited to LR, CR and XER for now.

When writing the test cases, one must ensure that the Stack
Pointer (GPR1) or the Thread Pointer (GPR13) are not touched
by any means as these are vital non-volatile registers.

Signed-off-by: Sandipan Das 
---
 arch/powerpc/lib/Makefile |   3 +-
 arch/powerpc/lib/test_emulate_step.c  | 167 +-
 .../lib/test_emulate_step_exec_instr.S| 150 
 3 files changed, 315 insertions(+), 5 deletions(-)
 create mode 100644 arch/powerpc/lib/test_emulate_step_exec_instr.S

diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index 3bf9fc6fd36c..79396e184bca 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -30,7 +30,8 @@ obj64-y   += copypage_64.o copyuser_64.o mem_64.o 
hweight_64.o \
 
 obj64-$(CONFIG_SMP)+= locks.o
 obj64-$(CONFIG_ALTIVEC)+= vmx-helper.o
-obj64-$(CONFIG_KPROBES_SANITY_TEST) += test_emulate_step.o
+obj64-$(CONFIG_KPROBES_SANITY_TEST)+= test_emulate_step.o \
+  test_emulate_step_exec_instr.o
 
 obj-y  += checksum_$(BITS).o checksum_wrappers.o \
   string_$(BITS).o memcmp_$(BITS).o
diff --git a/arch/powerpc/lib/test_emulate_step.c 
b/arch/powerpc/lib/test_emulate_step.c
index 6c47daa61614..3d7f7bae51cc 100644
--- a/arch/powerpc/lib/test_emulate_step.c
+++ b/arch/powerpc/lib/test_emulate_step.c
@@ -1,5 +1,5 @@
 /*
- * Simple sanity test for emulate_step load/store instructions.
+ * Simple sanity tests for instruction emulation infrastructure.
  *
  * Copyright IBM Corp. 2016
  *
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #define IMM_L(i)   ((uintptr_t)(i) & 0x)
 
@@ -49,6 +50,11 @@
 #define TEST_LXVD2X(s, a, b)   (PPC_INST_LXVD2X | VSX_XX1((s), R##a, R##b))
 #define TEST_STXVD2X(s, a, b)  (PPC_INST_STXVD2X | VSX_XX1((s), R##a, R##b))
 
+#define MAX_SUBTESTS   16
+
+#define IGNORE_GPR(n)  (0x1UL << (n))
+#define IGNORE_XER (0x1UL << 32)
+#define IGNORE_CCR (0x1UL << 33)
 
 static void __init init_pt_regs(struct pt_regs *regs)
 {
@@ -72,9 +78,15 @@ static void __init init_pt_regs(struct pt_regs *regs)
msr_cached = true;
 }
 
-static void __init show_result(char *ins, char *result)
+static void __init show_result(char *mnemonic, char *result)
 {
-   pr_info("%-14s : %s\n", ins, result);
+   pr_info("%-14s : %s\n", mnemonic, result);
+}
+
+static void __init show_result_with_descr(char *mnemonic, char *descr,
+ char *result)
+{
+   pr_info("%-14s : %-50s %s\n", mnemonic, descr, result);
 }
 
 static void __init test_ld(void)
@@ -426,7 +438,7 @@ static void __init test_lxvd2x_stxvd2x(void)
 }
 #endif /* CONFIG_VSX */
 
-static int __init test_emulate_step(void)
+static void __init run_tests_load_store(void)
 {
test_ld();
test_lwz();
@@ -437,6 +449,153 @@ static int __init test_emulate_step(void)
test_lfdx_stfdx();
test_lvx_stvx();
test_lxvd2x_stxvd2x();
+}
+
+struct compute_test {
+   char *mnemonic;
+   struct {
+   char *descr;
+   unsigned long flags;
+   unsigned int instr;
+   struct pt_regs regs;
+   } subtests[MAX_SUBTESTS + 1];
+};
+
+static struct compute_test compute_tests[] = {
+   {
+   .mnemonic = "nop",
+   .subtests = {
+   {
+   .descr = "R0 = LONG_MAX",
+   .instr = PPC_INST_NOP,
+   .regs = {
+   .gpr[0] = LONG_MAX,
+   }
+   }
+   }
+   }
+};
+
+static int __init emulate_compute_instr(struct pt_regs *regs,
+   unsigned int instr)
+{
+   struct instruction_op op;
+
+   if (!regs || !instr)
+   return -EINVAL;
+
+   if (analyse_instr(, regs, instr) != 1 ||
+   GETTYPE(op.type) != COMPUTE) {
+   pr_info("emulation failed, instruction = 0x%08x\n", instr);
+   return -EFAULT;
+   }
+
+   emulate_update_regs(regs, );
+   return 0;
+}
+
+static int __init execute_compute_instr(struct pt_regs *regs,
+   unsigned int instr)
+{
+   extern unsigned int exec_instr_execute[];
+   extern int exec_instr(struct pt_regs *regs);
+
+   if (!regs || !instr)
+   return -EINVAL;
+
+   /* Patch the NOP with the actual instruction */
+