[RFC PATCH 0/5] powerpc: sstep: Emulation test infrastructure

[RFC PATCH 0/5] powerpc: sstep: Emulation test infrastructure

[Sandipan Das]

This aims to add a test infrastructure for the in-kernel instruction
emulation code. This is currently limited to testing only the basic
integer operations and supports verification of the GPRs, LR, XER and
CR.

There can be multiple test cases for each instruction. Each test case
has to be provided with the initial register state (in the form of a
struct pt_regs) and the 32-bit instruction to test.

Apart from verifying the end result, problems with the behaviour of
certain instructions for things like setting certain bits in CR or
XER (which can also be processor dependent) can be identified.
For example, the newly introduced CA32 bit in XER, exclusive to P9
CPUs as of now, was not being set when expected for some of the
arithmetic and shift instructions. With this infrastructure, it will
be easier to identify such problems and rectify them. The test cases
for the addc[.] instruction demonstrate this for different scenarios
where the CA and CA32 bits of XER should be set.

Sandipan Das (5):
  powerpc: Add bitmasks for D-form instruction fields
  powerpc: Add bitmask for Rc instruction field
  powerpc: sstep: Add instruction emulation selftests
  powerpc: sstep: Add selftests for add[.] instruction
  powerpc: sstep: Add selftests for addc[.] instruction

 arch/powerpc/Kconfig.debug            |   5 +
 arch/powerpc/include/asm/ppc-opcode.h |   5 +
 arch/powerpc/lib/Makefile             |   1 +
 arch/powerpc/lib/exec_test_instr.S    | 150 +++++++
 arch/powerpc/lib/sstep_tests.c        | 564 ++++++++++++++++++++++++++
 5 files changed, 725 insertions(+)
 create mode 100644 arch/powerpc/lib/exec_test_instr.S
 create mode 100644 arch/powerpc/lib/sstep_tests.c

-- 
2.19.2

[RFC PATCH 1/5] powerpc: Add bitmasks for D-form instruction fields

[Sandipan Das]

This adds the bitmask definitions of D, SI and UI fields
found in D-form instructions.

Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
 arch/powerpc/include/asm/ppc-opcode.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index 19a8834e0398..9bc7dd6116a7 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -403,6 +403,9 @@
 #define __PPC_CT(t)	(((t) & 0x0f) << 21)
 #define __PPC_SPR(r)	((((r) & 0x1f) << 16) | ((((r) >> 5) & 0x1f) << 11))
 #define __PPC_RC21	(0x1 << 10)
+#define __PPC_D(d)	((d) & 0xffff)
+#define __PPC_SI(i)	__PPC_D(i)
+#define __PPC_UI(i)	__PPC_D(i)
 
 /*
  * Only use the larx hint bit on 64bit CPUs. e500v1/v2 based CPUs will treat a
-- 
2.19.2

[RFC PATCH 2/5] powerpc: Add bitmask for Rc instruction field

[Sandipan Das]

This adds the bitmask definition for the Record bit that
is available at the end (bit 31) of some instructions.

Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
 arch/powerpc/include/asm/ppc-opcode.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index 9bc7dd6116a7..07bdb404571c 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -403,6 +403,7 @@
 #define __PPC_CT(t)	(((t) & 0x0f) << 21)
 #define __PPC_SPR(r)	((((r) & 0x1f) << 16) | ((((r) >> 5) & 0x1f) << 11))
 #define __PPC_RC21	(0x1 << 10)
+#define __PPC_RC31	(0x1)
 #define __PPC_D(d)	((d) & 0xffff)
 #define __PPC_SI(i)	__PPC_D(i)
 #define __PPC_UI(i)	__PPC_D(i)
-- 
2.19.2

[RFC PATCH 3/5] powerpc: sstep: Add instruction emulation selftests

[Sandipan Das]

This adds a selftest framework for the in-kernel instruction
emulation infrastructure. This currently does not support the
load/store and branch instructions and is limited to integer
ALU instructions. Support for SPRs is also limited to LR, CR
and XER for now.

Tests run at boot time if CONFIG_DEBUG_KERNEL, CONFIG_PPC64
and CONFIG_EMULATE_STEP_SELFTEST were set before the kernel
build.

When writing the tests, one must not use any instructions
that might overwrite the Stack Pointer (GPR1) or the Thread
Pointer (GPR13).

Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
 arch/powerpc/Kconfig.debug         |   5 +
 arch/powerpc/lib/Makefile          |   1 +
 arch/powerpc/lib/exec_test_instr.S | 150 +++++++++++++++++++++++++++
 arch/powerpc/lib/sstep_tests.c     | 158 +++++++++++++++++++++++++++++
 4 files changed, 314 insertions(+)
 create mode 100644 arch/powerpc/lib/exec_test_instr.S
 create mode 100644 arch/powerpc/lib/sstep_tests.c

diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
index f4961fbcb48d..d75c165538e9 100644
--- a/arch/powerpc/Kconfig.debug
+++ b/arch/powerpc/Kconfig.debug
@@ -56,6 +56,11 @@ config CODE_PATCHING_SELFTEST
 	bool "Run self-tests of the code-patching code"
 	depends on DEBUG_KERNEL
 
+config EMULATE_STEP_SELFTEST
+	bool "Run self-tests of the instruction emulation code"
+	depends on PPC64 && DEBUG_KERNEL
+	default n
+
 config JUMP_LABEL_FEATURE_CHECKS
 	bool "Enable use of jump label for cpu/mmu_has_feature()"
 	depends on JUMP_LABEL
diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index 3bf9fc6fd36c..c4485e004838 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -31,6 +31,7 @@ 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_EMULATE_STEP_SELFTEST)	+= exec_test_instr.o sstep_tests.o
 
 obj-y			+= checksum_$(BITS).o checksum_wrappers.o \
 			   string_$(BITS).o memcmp_$(BITS).o
diff --git a/arch/powerpc/lib/exec_test_instr.S b/arch/powerpc/lib/exec_test_instr.S
new file mode 100644
index 000000000000..217e83415eaf
--- /dev/null
+++ b/arch/powerpc/lib/exec_test_instr.S
@@ -0,0 +1,150 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Non-emulated single-stepping support limited to basic integer ops for
+ * validating the instruction emulation infrastructure.
+ *
+ * Copyright (C) 2019 IBM Corporation
+ */
+
+#include <asm/asm-offsets.h>
+#include <asm/ppc_asm.h>
+#include <linux/errno.h>
+
+/* int exec_instr(struct pt_regs *regs) */
+_GLOBAL(exec_instr)
+
+	/*
+	 * Stack frame layout (INT_FRAME_SIZE bytes)
+	 *   In-memory pt_regs	(SP + STACK_FRAME_OVERHEAD)
+	 *   Scratch space	(SP + 8)
+	 *   Back chain		(SP + 0)
+	 */
+
+	/*
+	 * Allocate a new stack frame with enough space to hold the register
+	 * states in an in-memory pt_regs and also create the back chain to
+	 * the caller's stack frame.
+	 */
+	stdu	r1, -INT_FRAME_SIZE(r1)
+
+	/*
+	 * Save non-volatile GPRs on stack. This includes TOC pointer (GPR2)
+	 * and local variables (GPR14 to GPR31). The register for the pt_regs
+	 * parameter (GPR3) is saved additionally to ensure that the resulting
+	 * register state can still be saved even if GPR3 gets overwritten
+	 * when loading the initial register state for the test instruction.
+	 * The stack pointer (GPR1) and the thread pointer (GPR13) are not
+	 * saved as these should not be modified anyway.
+	 */
+	SAVE_2GPRS(2, r1)
+	SAVE_NVGPRS(r1)
+
+	/*
+	 * Save LR on stack to ensure that the return address is available
+	 * even if it gets overwritten by the test instruction.
+	 */
+	mflr	r0
+	std	r0, _LINK(r1)
+
+	/*
+	 * Save CR on stack. For simplicity, the entire register is saved
+	 * even though only fields 2 to 4 are non-volatile.
+	 */
+	mfcr	r0
+	std	r0, _CCR(r1)
+
+	/*
+	 * Load register state for the test instruction without touching the
+	 * critical non-volatile registers. The register state is passed as a
+	 * pointer to a pt_regs instance.
+	 */
+	subi	r31, r3, GPR0
+
+	/* Load LR from pt_regs */
+	ld	r0, _LINK(r31)
+	mtlr	r0
+
+	/* Load CR from pt_regs */
+	ld	r0, _CCR(r31)
+	mtcr	r0
+
+	/* Load XER from pt_regs */
+	ld	r0, _XER(r31)
+	mtxer	r0
+
+	/* Load GPRs from pt_regs */
+	REST_GPR(0, r31)
+	REST_10GPRS(2, r31)
+	REST_GPR(12, r31)
+	REST_NVGPRS(r31)
+
+	.global	exec_instr_execute
+exec_instr_execute:
+	/* Placeholder for the test instruction */
+1:	nop
+
+	/*
+	 * Since GPR3 is overwritten, temporarily restore it back to its
+	 * original state, i.e. the pointer to pt_regs, to ensure that the
+	 * resulting register state can be saved. Before doing this, a copy
+	 * of it is created in the scratch space which is used later on to
+	 * save it to pt_regs.
+	 */
+	std	r3, 8(r1)
+	REST_GPR(3, r1)
+
+	/* Save resulting GPR state to pt_regs */
+	subi	r3, r3, GPR0
+	SAVE_GPR(0, r3)
+	SAVE_GPR(2, r3)
+	SAVE_8GPRS(4, r3)
+	SAVE_GPR(12, r3)
+	SAVE_NVGPRS(r3)
+
+	/* Save resulting LR to pt_regs */
+	mflr	r0
+	std	r0, _LINK(r3)
+
+	/* Save resulting CR to pt_regs */
+	mfcr	r0
+	std	r0, _CCR(r3)
+
+	/* Save resulting XER to pt_regs */
+	mfxer	r0
+	std	r0, _XER(r3)
+
+	/* Restore resulting GPR3 from scratch space and save it to pt_regs */
+	ld	r0, 8(r1)
+	std	r0, GPR3(r3)
+
+	/* Set return value to denote execution success */
+	li	r3, 0
+
+	/* Continue */
+	b	3f
+
+	/* Set return value to denote execution failure */
+2:	li	r3, -EFAULT
+
+	/* Restore the non-volatile GPRs from stack */
+3:	REST_GPR(2, r1)
+	REST_NVGPRS(r1)
+
+	/* Restore LR from stack to be able to return */
+	ld	r0, _LINK(r1)
+	mtlr	r0
+
+	/* Restore CR from stack */
+	ld	r0, _CCR(r1)
+	mtcr	r0
+
+	/* Tear down stack frame */
+	addi	r1, r1, INT_FRAME_SIZE
+
+	/* Return */
+	blr
+
+	/* Setup exception table */
+	EX_TABLE(1b, 2b)
+
+_ASM_NOKPROBE_SYMBOL(exec_instr)
diff --git a/arch/powerpc/lib/sstep_tests.c b/arch/powerpc/lib/sstep_tests.c
new file mode 100644
index 000000000000..a610c778044d
--- /dev/null
+++ b/arch/powerpc/lib/sstep_tests.c
@@ -0,0 +1,158 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Selftests for the instruction emulation infrastructure.
+ *
+ * Copyright (C) 2019 IBM Corporation
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/ptrace.h>
+
+#include <asm/code-patching.h>
+#include <asm/ppc-opcode.h>
+#include <asm/sstep.h>
+
+#define MAX_SUBTESTS	16
+#define MAX_INSNS	32
+
+#define PASS	1
+#define FAIL	0
+
+#define IGNORE_GPR(n)	(0x1UL << (n))
+#define IGNORE_XER	(0x1UL << 32)
+#define IGNORE_CCR	(0x1UL << 33)
+
+struct sstep_test {
+	const char *mnemonic;
+	struct {
+		const char *descr;
+		unsigned long flags;
+		unsigned int instr;
+		struct pt_regs regs;
+	} subtests[MAX_SUBTESTS + 1];
+};
+
+static struct sstep_test tests[] = {
+	{
+		.mnemonic = "nop",
+		.subtests =
+		{
+			{
+				.descr = "R0 = LONG_MAX",
+				.instr = PPC_INST_NOP | ___PPC_RA(0) | ___PPC_RS(0) | __PPC_UI(0x0),
+				.regs =
+				{
+					.gpr[0] = LONG_MAX,
+				}
+			}
+		}
+	},
+};
+
+int emulate_instr(struct pt_regs *regs, unsigned int instr)
+{
+	struct instruction_op op;
+
+	if (!regs || !instr)
+		return -EINVAL;
+
+	if (analyse_instr(&op, regs, instr) != 1 || GETTYPE(op.type) != COMPUTE) {
+		pr_info("emulation failed or not supported, opcode = 0x%08x\n", instr);
+		return -EFAULT;
+	}
+
+	emulate_update_regs(regs, &op);
+	return 0;
+}
+
+int execute_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 */
+	patch_instruction(&exec_instr_execute[0], instr);
+	if (exec_instr(regs)) {
+		pr_info("execution failed, opcode = 0x%08x\n", instr);
+		return -EFAULT;
+	}
+
+	return 0;
+}
+
+static int __init run_sstep_tests(void)
+{
+	bool ignore_gpr;
+	bool ignore_xer;
+	bool ignore_ccr;
+	unsigned int instr;
+	unsigned long flags;
+	struct pt_regs a, b;
+	const char *mnemonic, *descr;
+	unsigned int i, j, k, result;
+
+	for (i = 0; i < ARRAY_SIZE(tests); i++) {
+		mnemonic = tests[i].mnemonic;
+
+		for (j = 0; j < MAX_SUBTESTS && tests[i].subtests[j].descr; j++) {
+			descr = tests[i].subtests[j].descr;
+			instr = tests[i].subtests[j].instr;
+			flags = tests[i].subtests[j].flags;
+			ignore_xer = flags & IGNORE_XER;
+			ignore_ccr = flags & IGNORE_CCR;
+			result = PASS;
+
+			memcpy(&a, &tests[i].subtests[j].regs, sizeof(struct pt_regs));
+			memcpy(&b, &tests[i].subtests[j].regs, sizeof(struct pt_regs));
+
+			/*
+			 * Set a compatible MSR value explicitly to ensure
+			 * that XER and CR bits are updated appropriately
+			 */
+			a.msr = b.msr = MSR_KERNEL;
+
+			if (emulate_instr(&a, instr) || execute_instr(&b, instr)) {
+				result = FAIL;
+				goto print;
+			}
+
+			/* Verify GPR values */
+			for (k = 0; k < 32; k++) {
+				ignore_gpr = flags & IGNORE_GPR(k);
+				if (!ignore_gpr && a.gpr[k] != b.gpr[k]) {
+					result = FAIL;
+					pr_info("GPR%u got = 0x%016lx, exp = 0x%016lx\n", k, a.gpr[k], b.gpr[k]);
+				}
+			}
+
+			/* Verify LR value */
+			if (a.link != b.link) {
+				result = FAIL;
+				pr_info("LR got = 0x%016lx, exp = 0x%016lx\n", a.link, b.link);
+			}
+
+			/* Verify XER value */
+			if (!ignore_xer && a.xer != b.xer) {
+				result = FAIL;
+				pr_info("XER got = 0x%016lx, exp = 0x%016lx\n", a.xer, b.xer);
+			}
+
+			/* Verify CR value */
+			if (!ignore_ccr && a.ccr != b.ccr) {
+				result = FAIL;
+				pr_info("CR got = 0x%08lx, exp = 0x%08lx\n", a.ccr, b.ccr);
+			}
+
+print:
+			pr_info("%-8s: %-50s [%s]", mnemonic, descr, result ? "PASS" : "FAIL");
+		}
+	}
+
+	return 0;
+}
+late_initcall(run_sstep_tests);
-- 
2.19.2

[RFC PATCH 4/5] powerpc: sstep: Add selftests for add[.] instruction

[Sandipan Das]

This adds test cases for the add[.] instruction.

Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
 arch/powerpc/lib/sstep_tests.c | 194 +++++++++++++++++++++++++++++++++
 1 file changed, 194 insertions(+)

diff --git a/arch/powerpc/lib/sstep_tests.c b/arch/powerpc/lib/sstep_tests.c
index a610c778044d..fe6201a2add7 100644
--- a/arch/powerpc/lib/sstep_tests.c
+++ b/arch/powerpc/lib/sstep_tests.c
@@ -49,6 +49,200 @@ static struct sstep_test tests[] = {
 			}
 		}
 	},
+	{
+		.mnemonic = "add",
+		.subtests =
+		{
+			{
+				.descr = "RA = LONG_MIN, RB = LONG_MIN",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = LONG_MIN,
+					.gpr[22] = LONG_MIN,
+				}
+			},
+			{
+				.descr = "RA = LONG_MIN, RB = LONG_MAX",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = LONG_MIN,
+					.gpr[22] = LONG_MAX,
+				}
+			},
+			{
+				.descr = "RA = LONG_MAX, RB = LONG_MAX",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = LONG_MAX,
+					.gpr[22] = LONG_MAX,
+				}
+			},
+			{
+				.descr = "RA = ULONG_MAX, RB = ULONG_MAX",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = ULONG_MAX,
+					.gpr[22] = ULONG_MAX,
+				}
+			},
+			{
+				.descr = "RA = ULONG_MAX, RB = 0x1",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = ULONG_MAX,
+					.gpr[22] = 0x1,
+				}
+			},
+			{
+				.descr = "RA = INT_MIN, RB = INT_MIN",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = INT_MIN,
+					.gpr[22] = INT_MIN,
+				}
+			},
+			{
+				.descr = "RA = INT_MIN, RB = INT_MAX",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = INT_MIN,
+					.gpr[22] = INT_MAX,
+				}
+			},
+			{
+				.descr = "RA = INT_MAX, RB = INT_MAX",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = INT_MAX,
+					.gpr[22] = INT_MAX,
+				}
+			},
+			{
+				.descr = "RA = UINT_MAX, RB = UINT_MAX",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = UINT_MAX,
+					.gpr[22] = UINT_MAX,
+				}
+			},
+			{
+				.descr = "RA = UINT_MAX, RB = 0x1",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = UINT_MAX,
+					.gpr[22] = 0x1,
+				}
+			}
+		}
+	},
+	{
+		.mnemonic = "add.",
+		.subtests =
+		{
+			{
+				.descr = "RA = LONG_MIN, RB = LONG_MIN",
+				.flags = IGNORE_CCR,
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = LONG_MIN,
+					.gpr[22] = LONG_MIN,
+				}
+			},
+			{
+				.descr = "RA = LONG_MIN, RB = LONG_MAX",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = LONG_MIN,
+					.gpr[22] = LONG_MAX,
+				}
+			},
+			{
+				.descr = "RA = LONG_MAX, RB = LONG_MAX",
+				.flags = IGNORE_CCR,
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = LONG_MAX,
+					.gpr[22] = LONG_MAX,
+				}
+			},
+			{
+				.descr = "RA = ULONG_MAX, RB = ULONG_MAX",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = ULONG_MAX,
+					.gpr[22] = ULONG_MAX,
+				}
+			},
+			{
+				.descr = "RA = ULONG_MAX, RB = 0x1",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = ULONG_MAX,
+					.gpr[22] = 0x1,
+				}
+			},
+			{
+				.descr = "RA = INT_MIN, RB = INT_MIN",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = INT_MIN,
+					.gpr[22] = INT_MIN,
+				}
+			},
+			{
+				.descr = "RA = INT_MIN, RB = INT_MAX",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = INT_MIN,
+					.gpr[22] = INT_MAX,
+				}
+			},
+			{
+				.descr = "RA = INT_MAX, RB = INT_MAX",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = INT_MAX,
+					.gpr[22] = INT_MAX,
+				}
+			},
+			{
+				.descr = "RA = UINT_MAX, RB = UINT_MAX",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = UINT_MAX,
+					.gpr[22] = UINT_MAX,
+				}
+			},
+			{
+				.descr = "RA = UINT_MAX, RB = 0x1",
+				.instr = PPC_INST_ADD | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = UINT_MAX,
+					.gpr[22] = 0x1,
+				}
+			}
+		}
+	},
 };
 
 int emulate_instr(struct pt_regs *regs, unsigned int instr)
-- 
2.19.2

[RFC PATCH 5/5] powerpc: sstep: Add selftests for addc[.] instruction

[Sandipan Das]

This adds test cases for the addc[.] instruction.

Signed-off-by: Sandipan Das <sandipan@linux.ibm.com>
---
 arch/powerpc/include/asm/ppc-opcode.h |   1 +
 arch/powerpc/lib/sstep_tests.c        | 212 ++++++++++++++++++++++++++
 2 files changed, 213 insertions(+)

diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index 07bdb404571c..c0fe90173977 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -326,6 +326,7 @@
 #define PPC_INST_ADDI			0x38000000
 #define PPC_INST_ADDIS			0x3c000000
 #define PPC_INST_ADD			0x7c000214
+#define PPC_INST_ADDC			0x7c000014
 #define PPC_INST_SUB			0x7c000050
 #define PPC_INST_BLR			0x4e800020
 #define PPC_INST_BLRL			0x4e800021
diff --git a/arch/powerpc/lib/sstep_tests.c b/arch/powerpc/lib/sstep_tests.c
index fe6201a2add7..d2f4bb66f66f 100644
--- a/arch/powerpc/lib/sstep_tests.c
+++ b/arch/powerpc/lib/sstep_tests.c
@@ -243,6 +243,218 @@ static struct sstep_test tests[] = {
 			}
 		}
 	},
+	{
+		.mnemonic = "addc",
+		.subtests =
+		{
+			{
+				.descr = "RA = LONG_MIN, RB = LONG_MIN",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = LONG_MIN,
+					.gpr[22] = LONG_MIN,
+				}
+			},
+			{
+				.descr = "RA = LONG_MIN, RB = LONG_MAX",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = LONG_MIN,
+					.gpr[22] = LONG_MAX,
+				}
+			},
+			{
+				.descr = "RA = LONG_MAX, RB = LONG_MAX",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = LONG_MAX,
+					.gpr[22] = LONG_MAX,
+				}
+			},
+			{
+				.descr = "RA = ULONG_MAX, RB = ULONG_MAX",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = ULONG_MAX,
+					.gpr[22] = ULONG_MAX,
+				}
+			},
+			{
+				.descr = "RA = ULONG_MAX, RB = 0x1",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = ULONG_MAX,
+					.gpr[22] = 0x1,
+				}
+			},
+			{
+				.descr = "RA = INT_MIN, RB = INT_MIN",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = INT_MIN,
+					.gpr[22] = INT_MIN,
+				}
+			},
+			{
+				.descr = "RA = INT_MIN, RB = INT_MAX",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = INT_MIN,
+					.gpr[22] = INT_MAX,
+				}
+			},
+			{
+				.descr = "RA = INT_MAX, RB = INT_MAX",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = INT_MAX,
+					.gpr[22] = INT_MAX,
+				}
+			},
+			{
+				.descr = "RA = UINT_MAX, RB = UINT_MAX",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = UINT_MAX,
+					.gpr[22] = UINT_MAX,
+				}
+			},
+			{
+				.descr = "RA = UINT_MAX, RB = 0x1",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = UINT_MAX,
+					.gpr[22] = 0x1,
+				}
+			},
+			{
+				.descr = "RA = LONG_MIN | INT_MIN, RB = LONG_MIN | INT_MIN",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
+				.regs =
+				{
+					.gpr[21] = LONG_MIN | (uint) INT_MIN,
+					.gpr[22] = LONG_MIN | (uint) INT_MIN,
+				}
+			}
+		}
+	},
+	{
+		.mnemonic = "addc.",
+		.subtests =
+		{
+			{
+				.descr = "RA = LONG_MIN, RB = LONG_MIN",
+				.flags = IGNORE_CCR,
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = LONG_MIN,
+					.gpr[22] = LONG_MIN,
+				}
+			},
+			{
+				.descr = "RA = LONG_MIN, RB = LONG_MAX",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = LONG_MIN,
+					.gpr[22] = LONG_MAX,
+				}
+			},
+			{
+				.descr = "RA = LONG_MAX, RB = LONG_MAX",
+				.flags = IGNORE_CCR,
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = LONG_MAX,
+					.gpr[22] = LONG_MAX,
+				}
+			},
+			{
+				.descr = "RA = ULONG_MAX, RB = ULONG_MAX",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = ULONG_MAX,
+					.gpr[22] = ULONG_MAX,
+				}
+			},
+			{
+				.descr = "RA = ULONG_MAX, RB = 0x1",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = ULONG_MAX,
+					.gpr[22] = 0x1,
+				}
+			},
+			{
+				.descr = "RA = INT_MIN, RB = INT_MIN",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = INT_MIN,
+					.gpr[22] = INT_MIN,
+				}
+			},
+			{
+				.descr = "RA = INT_MIN, RB = INT_MAX",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = INT_MIN,
+					.gpr[22] = INT_MAX,
+				}
+			},
+			{
+				.descr = "RA = INT_MAX, RB = INT_MAX",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = INT_MAX,
+					.gpr[22] = INT_MAX,
+				}
+			},
+			{
+				.descr = "RA = UINT_MAX, RB = UINT_MAX",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = UINT_MAX,
+					.gpr[22] = UINT_MAX,
+				}
+			},
+			{
+				.descr = "RA = UINT_MAX, RB = 0x1",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = UINT_MAX,
+					.gpr[22] = 0x1,
+				}
+			},
+			{
+				.descr = "RA = LONG_MIN | INT_MIN, RB = LONG_MIN | INT_MIN",
+				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22) | __PPC_RC31,
+				.regs =
+				{
+					.gpr[21] = LONG_MIN | (uint) INT_MIN,
+					.gpr[22] = LONG_MIN | (uint) INT_MIN,
+				}
+			}
+		}
+	},
 };
 
 int emulate_instr(struct pt_regs *regs, unsigned int instr)
-- 
2.19.2

Re: [RFC PATCH 3/5] powerpc: sstep: Add instruction emulation selftests

[Daniel Axtens]

Hi Sandipan,

I'm not really confident to review the asm, but I did have a couple of
questions about the C:

> +#define MAX_INSNS	32
This doesn't seem to be used...

> +int execute_instr(struct pt_regs *regs, unsigned int instr)
> +{
> +	extern unsigned int exec_instr_execute[];
> +	extern int exec_instr(struct pt_regs *regs);

These externs sit inside the function scope. This feels less than ideal
to me - is there a reason not to have these at global scope?

> +
> +	if (!regs || !instr)
> +		return -EINVAL;
> +
> +	/* Patch the NOP with the actual instruction */
> +	patch_instruction(&exec_instr_execute[0], instr);
> +	if (exec_instr(regs)) {
> +		pr_info("execution failed, opcode = 0x%08x\n", instr);
> +		return -EFAULT;
> +	}
> +
> +	return 0;
> +}

> +late_initcall(run_sstep_tests);
A design question: is there a reason to run these as an initcall rather
than as a module that could either be built in or loaded separately? I'm
not saying you have to do this, but I was wondering if you had
considered it?

Lastly, snowpatch reports some checkpatch issues for this and your
remaining patches: https://patchwork.ozlabs.org/patch/1035683/ (You are
allowed to violate checkpatch rules with justification, FWIW)

Regards,
Daniel
> -- 
> 2.19.2

Re: [RFC PATCH 5/5] powerpc: sstep: Add selftests for addc[.] instruction

[Daniel Axtens]

Hi Sandipan,

> +			{
> +				.descr = "RA = LONG_MIN | INT_MIN, RB = LONG_MIN | INT_MIN",
> +				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
> +				.regs =
> +				{
> +					.gpr[21] = LONG_MIN | (uint) INT_MIN,
> +					.gpr[22] = LONG_MIN | (uint) INT_MIN,
> +				}
> +			}
I don't know what this bit pattern is supposed to represent - is it
supposed to be the smallest 32bit integer and the smallest 64bit
integer 8000000080000000 - so you test 32 and 64 bit overflow at the
same time? 


For the series:
Tested-by: Daniel Axtens <dja@axtens.net> # Power8 LE

I notice the output is quite verbose, and doesn't include a line when it
starts:

[    0.826181] Running code patching self-tests ...
[    0.826607] Running feature fixup self-tests ...
[    0.826615] nop     : R0 = LONG_MAX                                      [PASS]
[    0.826617] add     : RA = LONG_MIN, RB = LONG_MIN                       [PASS]

Maybe it would be good to include a line saying "Running single-step
emulation self-tests" and perhaps by default on printing when there is a
failure.

Finally, I think you might be able to squash patches 1 and 2 and patches
4 and 5, but that's just my personal preference.

Regards,
Daniel

Re: [RFC PATCH 0/5] powerpc: sstep: Emulation test infrastructure

[Daniel Axtens]

Hi Sandipan,

> This aims to add a test infrastructure for the in-kernel instruction
> emulation code. This is currently limited to testing only the basic
> integer operations and supports verification of the GPRs, LR, XER and
> CR.
>
> There can be multiple test cases for each instruction. Each test case
> has to be provided with the initial register state (in the form of a
> struct pt_regs) and the 32-bit instruction to test.
>
> Apart from verifying the end result, problems with the behaviour of
> certain instructions for things like setting certain bits in CR or
> XER (which can also be processor dependent) can be identified.
> For example, the newly introduced CA32 bit in XER, exclusive to P9
> CPUs as of now, was not being set when expected for some of the
> arithmetic and shift instructions. With this infrastructure, it will
> be easier to identify such problems and rectify them. The test cases
> for the addc[.] instruction demonstrate this for different scenarios
> where the CA and CA32 bits of XER should be set.
>

I forgot to mention this in my other emails - I think this is a really
good idea, thank you for doing it.

Regards,
Daniel

> Sandipan Das (5):
>   powerpc: Add bitmasks for D-form instruction fields
>   powerpc: Add bitmask for Rc instruction field
>   powerpc: sstep: Add instruction emulation selftests
>   powerpc: sstep: Add selftests for add[.] instruction
>   powerpc: sstep: Add selftests for addc[.] instruction
>
>  arch/powerpc/Kconfig.debug            |   5 +
>  arch/powerpc/include/asm/ppc-opcode.h |   5 +
>  arch/powerpc/lib/Makefile             |   1 +
>  arch/powerpc/lib/exec_test_instr.S    | 150 +++++++
>  arch/powerpc/lib/sstep_tests.c        | 564 ++++++++++++++++++++++++++
>  5 files changed, 725 insertions(+)
>  create mode 100644 arch/powerpc/lib/exec_test_instr.S
>  create mode 100644 arch/powerpc/lib/sstep_tests.c
>
> -- 
> 2.19.2

Re: [RFC PATCH 5/5] powerpc: sstep: Add selftests for addc[.] instruction

[Sandipan Das]

On 11/02/19 6:30 AM, Daniel Axtens wrote:
> Hi Sandipan,
> 
>> +			{
>> +				.descr = "RA = LONG_MIN | INT_MIN, RB = LONG_MIN | INT_MIN",
>> +				.instr = PPC_INST_ADDC | ___PPC_RT(20) | ___PPC_RA(21) | ___PPC_RB(22),
>> +				.regs =
>> +				{
>> +					.gpr[21] = LONG_MIN | (uint) INT_MIN,
>> +					.gpr[22] = LONG_MIN | (uint) INT_MIN,
>> +				}
>> +			}
> I don't know what this bit pattern is supposed to represent - is it
> supposed to be the smallest 32bit integer and the smallest 64bit
> integer 8000000080000000 - so you test 32 and 64 bit overflow at the
> same time? 
> 

Yes, exactly.

> 
> For the series:
> Tested-by: Daniel Axtens <dja@axtens.net> # Power8 LE
> 
> I notice the output is quite verbose, and doesn't include a line when it
> starts:
> 
> [    0.826181] Running code patching self-tests ...
> [    0.826607] Running feature fixup self-tests ...
> [    0.826615] nop     : R0 = LONG_MAX                                      [PASS]
> [    0.826617] add     : RA = LONG_MIN, RB = LONG_MIN                       [PASS]
> 
> Maybe it would be good to include a line saying "Running single-step
> emulation self-tests" and perhaps by default on printing when there is a
> failure.
> 

That makes sense. Will include it in the next revision.

> Finally, I think you might be able to squash patches 1 and 2 and patches
> 4 and 5, but that's just my personal preference.
> 
> Regards,
> Daniel
> 

Re: [RFC PATCH 3/5] powerpc: sstep: Add instruction emulation selftests

[Sandipan Das]

On 11/02/19 6:17 AM, Daniel Axtens wrote:
> Hi Sandipan,
> 
> I'm not really confident to review the asm, but I did have a couple of
> questions about the C:
> 
>> +#define MAX_INSNS	32
> This doesn't seem to be used...
> 

True. Thanks for pointing this out.

>> +int execute_instr(struct pt_regs *regs, unsigned int instr)
>> +{
>> +	extern unsigned int exec_instr_execute[];
>> +	extern int exec_instr(struct pt_regs *regs);
> 
> These externs sit inside the function scope. This feels less than ideal
> to me - is there a reason not to have these at global scope?
> 

Currently, execute_instr() is the only consumer. So, I thought I'd keep
them local for now.

>> +
>> +	if (!regs || !instr)
>> +		return -EINVAL;
>> +
>> +	/* Patch the NOP with the actual instruction */
>> +	patch_instruction(&exec_instr_execute[0], instr);
>> +	if (exec_instr(regs)) {
>> +		pr_info("execution failed, opcode = 0x%08x\n", instr);
>> +		return -EFAULT;
>> +	}
>> +
>> +	return 0;
>> +}
> 
>> +late_initcall(run_sstep_tests);
> A design question: is there a reason to run these as an initcall rather
> than as a module that could either be built in or loaded separately? I'm
> not saying you have to do this, but I was wondering if you had
> considered it?
> 

I did. As of now, there are some existing tests in test_emulate_step.c
which use the same approach. So, I thought I'd stick with that approach
to start off. This is anyway controlled by a Kconfig option.

> Lastly, snowpatch reports some checkpatch issues for this and your
> remaining patches: https://patchwork.ozlabs.org/patch/1035683/ (You are
> allowed to violate checkpatch rules with justification, FWIW)
> 

Will look into them.

> Regards,
> Daniel
>> -- 
>> 2.19.2
>