Skip to contentSkip to conversation

Add generic ARM ISA condition code check v3

PATCH New Leif Lindholm 3 participants 10 messages Dec 9, 2011 – Dec 10, 2011 Arm (32-bit)

lore
Share & tools
lore
#0 lore raw

cover Add generic ARM ISA condition code check v3

There are several locations in the kernel where software needs to inspect the condition codes of trapped instructions. The original bitmask implementation in the nwfpe code does this in an efficient manner. This series breaks this code out of nwfpe/fpopcode.{ch} into a standalone file for opcode operations, and contains additional patches to kprobes and SWP eumlation to use this interface.

---

Set updated based on Will's feedback.

Leif Lindholm (4):
Add generic ARM instruction set condition code checks. Use generic ARM instruction set condition code checks for nwfpe. Add condition code checking to SWP emulation handler. Use generic ARM instruction set condition code checks for kprobes.

 arch/arm/include/asm/opcodes.h |   20 +++++++++++
 arch/arm/kernel/Makefile       |    2 +
 arch/arm/kernel/kprobes-test.c |   66 ++++---------------------------------
 arch/arm/kernel/opcodes.c      |   72 ++++++++++++++++++++++++++++++++++++++++
 arch/arm/kernel/swp_emulate.c  |   16 +++++++++
 arch/arm/nwfpe/entry.S         |    8 +++-
 arch/arm/nwfpe/fpopcode.c      |   26 --------------
 arch/arm/nwfpe/fpopcode.h      |    3 --
 8 files changed, 121 insertions(+), 92 deletions(-)
 create mode 100644 arch/arm/include/asm/opcodes.h
 create mode 100644 arch/arm/kernel/opcodes.c
Signature · 1 line
Signature
#1 lore raw

PATCH 1/4 Add generic ARM instruction set condition code checks.

This patch breaks the ARM condition checking code out of nwfpe/fpopcode.{ch} into a standalone file for opcode operations. It also modifies the code somewhat for coding style adherence, and adds some temporary variables for increased readability.

---
 arch/arm/include/asm/opcodes.h |   20 +++++++++++
 arch/arm/kernel/Makefile       |    2 +
 arch/arm/kernel/opcodes.c      |   72 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 93 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/include/asm/opcodes.h
 create mode 100644 arch/arm/kernel/opcodes.c
arch/arm/include/asm/opcodes.h new file +20 -0
new file mode 100644
index 0000000..aea97bf
@@ -0,0 +1,20 @@
+/*
+ *  arch/arm/include/asm/opcodes.h
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __ASM_ARM_OPCODES_H
+#define __ASM_ARM_OPCODES_H
+
+#ifndef __ASSEMBLY__
+extern unsigned int arm_check_condition(unsigned int opcode, unsigned int psr);
+#endif
+
+#define ARM_OPCODE_CONDTEST_FAIL   0
+#define ARM_OPCODE_CONDTEST_PASS   1
+#define ARM_OPCODE_CONDTEST_UNCOND 2
+
+#endif /* __ASM_ARM_OPCODES_H */
arch/arm/kernel/Makefile +1 -1
index 16eed6a..43b740d 100644
@@ -13,7 +13,7 @@ CFLAGS_REMOVE_return_address.o = -pg
 
 # Object file lists.
 
-obj-y		:= elf.o entry-armv.o entry-common.o irq.o \
+obj-y		:= elf.o entry-armv.o entry-common.o irq.o opcodes.o \
 		   process.o ptrace.o return_address.o setup.o signal.o \
 		   sys_arm.o stacktrace.o time.o traps.o
 
arch/arm/kernel/opcodes.c new file +72 -0

72 lines not on this page · raw message

#2 lore raw

PATCH 1/4 Add generic ARM instruction set condition code checks.

Quoted 10 lines · Leif Lindholm
arch/arm/include/asm/opcodes.h new file +4 -0
new file mode 100644
index 0000000..aea97bf
@@ -0,0 +1,20 @@
+/*
+ *  arch/arm/include/asm/opcodes.h
+ *
+ * This program is free software; you can redistribute it and/or modify

arch/arm/include/asm/opcodes.h line 14

Quoted 10 lines
arch/arm/include/asm/opcodes.h new file +10 -0
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __ASM_ARM_OPCODES_H
+#define __ASM_ARM_OPCODES_H
+
+#ifndef __ASSEMBLY__
+extern unsigned int arm_check_condition(unsigned int opcode, unsigned int psr);
+#endif

You forgot to update the prototype arguments.

With that fixed,

Will

#3 lore raw

PATCH 2/4 Use generic ARM instruction set condition code checks for nwfpe.

In reply to Leif Lindholm

This patch changes the nwfpe implementation to use the new generic ARM instruction set condition code checks, rather than a local implementation. It also removes the existing condition code checking, which has been used for the generic support (in kernel/opcodes.{ch}).

This code has not been tested beyond building, linking and booting.

---
 arch/arm/nwfpe/entry.S    |    8 +++++---
 arch/arm/nwfpe/fpopcode.c |   26 --------------------------
 arch/arm/nwfpe/fpopcode.h |    3 ---
 3 files changed, 5 insertions(+), 32 deletions(-)
arch/arm/nwfpe/entry.S +5 -3
index cafa183..d18dde9 100644
@@ -20,6 +20,8 @@
     Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
 */
 
+#include <asm/opcodes.h>
+
 /* This is the kernel's entry point into the floating point emulator.
 It is called from the kernel with code similar to this:
 
@@ -81,11 +83,11 @@ nwfpe_enter:
 	mov	r6, r0			@ save the opcode
 emulate:
 	ldr	r1, [sp, #S_PSR]	@ fetch the PSR
-	bl	checkCondition		@ check the condition
-	cmp	r0, #0			@ r0 = 0 ==> condition failed
+	bl	arm_check_condition	@ check the condition
+	cmp	r0, #ARM_OPCODE_CONDTEST_PASS	@ condition passed?
 
 	@ if condition code failed to match, next insn
-	beq	next			@ get the next instruction;
+	bne	next			@ get the next instruction;
 
 	mov	r0, r6			@ prepare for EmulateAll()
 	bl	EmulateAll		@ emulate the instruction
arch/arm/nwfpe/fpopcode.c +0 -26
index 922b811..ff98346 100644
@@ -61,29 +61,3 @@ const float32 float32Constant[] = {
 	0x41200000		/* single 10.0 */
 };
 
-/* condition code lookup table
- index into the table is test code: EQ, NE, ... LT, GT, AL, NV
- bit position in short is condition code: NZCV */
-static const unsigned short aCC[16] = {
-	0xF0F0,			// EQ == Z set
-	0x0F0F,			// NE
-	0xCCCC,			// CS == C set
-	0x3333,			// CC
-	0xFF00,			// MI == N set
-	0x00FF,			// PL
-	0xAAAA,			// VS == V set
-	0x5555,			// VC
-	0x0C0C,			// HI == C set && Z clear
-	0xF3F3,			// LS == C clear || Z set
-	0xAA55,			// GE == (N==V)
-	0x55AA,			// LT == (N!=V)
-	0x0A05,			// GT == (!Z && (N==V))
-	0xF5FA,			// LE == (Z || (N!=V))
-	0xFFFF,			// AL always
-	0			// NV
-};
-
-unsigned int checkCondition(const unsigned int opcode, const unsigned int ccodes)
-{
-	return (aCC[opcode >> 28] >> (ccodes >> 28)) & 1;
-}
arch/arm/nwfpe/fpopcode.h +0 -3
index 786e4c9..78f02db 100644
@@ -475,9 +475,6 @@ static inline unsigned int getDestinationSize(const unsigned int opcode)
 	return (nRc);
 }
 
-extern unsigned int checkCondition(const unsigned int opcode,
-				   const unsigned int ccodes);
-
 extern const float64 float64Constant[];
 extern const float32 float32Constant[];
 
#4 lore raw

PATCH 2/4 Use generic ARM instruction set condition code checks for nwfpe.

Quoted 5 lines · Leif Lindholm

This patch changes the nwfpe implementation to use the new generic ARM instruction set condition code checks, rather than a local implementation. It also removes the existing condition code checking, which has been used for the generic support (in kernel/opcodes.{ch}).

Quoted 8 lines

This code has not been tested beyond building, linking and booting.

---
 arch/arm/nwfpe/entry.S    |    8 +++++---
 arch/arm/nwfpe/fpopcode.c |   26 --------------------------
 arch/arm/nwfpe/fpopcode.h |    3 ---
 3 files changed, 5 insertions(+), 32 deletions(-)

Will

#5 lore raw

PATCH 3/4 Add condition code checking to SWP emulation handler.

In reply to Leif Lindholm

This patch fixes two separate issues with the SWP emulation handler:
1: Certain processors implementing ARMv7-A can (legally) take an
undef exception even when the condition code would have meant that the instruction should not have been executed.
2: Opcodes with all flags set (condition code = 0xf) have been reused
in recent, and not-so-recent, versions of the ARM architecture to implement unconditional extensions to the instruction set. The existing code would still have processed any undefs triggered by executing an opcode with such a value.

This patch uses the new generic ARM instruction set condition code checks to implement proper handling of these situations.

---
 arch/arm/kernel/swp_emulate.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)
arch/arm/kernel/swp_emulate.c +16 -0
index 5f452f8..df74518 100644
@@ -25,6 +25,7 @@
 #include <linux/syscalls.h>
 #include <linux/perf_event.h>
 
+#include <asm/opcodes.h>
 #include <asm/traps.h>
 #include <asm/uaccess.h>
 
@@ -185,6 +186,21 @@ static int swp_handler(struct pt_regs *regs, unsigned int instr)
 
 	perf_sw_event(PERF_COUNT_SW_EMULATION_FAULTS, 1, regs, regs->ARM_pc);
 
+	res = arm_check_condition(instr, regs->ARM_cpsr);
+	switch (res) {
+	case ARM_OPCODE_CONDTEST_PASS:
+		break;
+	case ARM_OPCODE_CONDTEST_FAIL:
+		/* Condition failed - return to next instruction */
+		regs->ARM_pc += 4;
+		return 0;
+	case ARM_OPCODE_CONDTEST_UNCOND:
+		/* If unconditional encoding - not a SWP, undef */
+		return -EFAULT;
+	default:
+		return -EINVAL;
+	}
+
 	if (current->pid != previous_pid) {
 		pr_debug("\"%s\" (%ld) uses deprecated SWP{B} instruction\n",
 			 current->comm, (unsigned long)current->pid);
#6 lore raw

PATCH 3/4 Add condition code checking to SWP emulation handler.

Quoted 10 lines · Leif Lindholm

This patch fixes two separate issues with the SWP emulation handler:
1: Certain processors implementing ARMv7-A can (legally) take an
undef exception even when the condition code would have meant that the instruction should not have been executed.
2: Opcodes with all flags set (condition code = 0xf) have been reused
in recent, and not-so-recent, versions of the ARM architecture to implement unconditional extensions to the instruction set. The existing code would still have processed any undefs triggered by executing an opcode with such a value.

Quoted 7 lines

This patch uses the new generic ARM instruction set condition code checks to implement proper handling of these situations.

---
 arch/arm/kernel/swp_emulate.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

Will

#7 lore raw

PATCH 4/4 Use generic ARM instruction set condition code checks for kprobes.

In reply to Leif Lindholm

This patch changes the kprobes implementation to use the generic ARM instruction set condition code checks, rather than a dedicated implementation.

---
Tixy's ACK removed as patch modified.

 arch/arm/kernel/kprobes-test.c |   66 ++++------------------------------------
 1 files changed, 7 insertions(+), 59 deletions(-)
arch/arm/kernel/kprobes-test.c +7 -59
index e17cdd6..1862d8f 100644
@@ -202,6 +202,8 @@
 #include <linux/slab.h>
 #include <linux/kprobes.h>
 
+#include <asm/opcodes.h>
+
 #include "kprobes.h"
 #include "kprobes-test.h"
 
@@ -1050,65 +1052,9 @@ static int test_instance;
 
 static unsigned long test_check_cc(int cc, unsigned long cpsr)
 {
-	unsigned long temp;
-
-	switch (cc) {
-	case 0x0: /* eq */
-		return cpsr & PSR_Z_BIT;
-
-	case 0x1: /* ne */
-		return (~cpsr) & PSR_Z_BIT;
-
-	case 0x2: /* cs */
-		return cpsr & PSR_C_BIT;
-
-	case 0x3: /* cc */
-		return (~cpsr) & PSR_C_BIT;
-
-	case 0x4: /* mi */
-		return cpsr & PSR_N_BIT;
-
-	case 0x5: /* pl */
-		return (~cpsr) & PSR_N_BIT;
-
-	case 0x6: /* vs */
-		return cpsr & PSR_V_BIT;
-
-	case 0x7: /* vc */
-		return (~cpsr) & PSR_V_BIT;
+	int ret = arm_check_condition(cc << 28, cpsr);
 
-	case 0x8: /* hi */
-		cpsr &= ~(cpsr >> 1); /* PSR_C_BIT &= ~PSR_Z_BIT */
-		return cpsr & PSR_C_BIT;
-
-	case 0x9: /* ls */
-		cpsr &= ~(cpsr >> 1); /* PSR_C_BIT &= ~PSR_Z_BIT */
-		return (~cpsr) & PSR_C_BIT;
-
-	case 0xa: /* ge */
-		cpsr ^= (cpsr << 3); /* PSR_N_BIT ^= PSR_V_BIT */
-		return (~cpsr) & PSR_N_BIT;
-
-	case 0xb: /* lt */
-		cpsr ^= (cpsr << 3); /* PSR_N_BIT ^= PSR_V_BIT */
-		return cpsr & PSR_N_BIT;
-
-	case 0xc: /* gt */
-		temp = cpsr ^ (cpsr << 3); /* PSR_N_BIT ^= PSR_V_BIT */
-		temp |= (cpsr << 1);	   /* PSR_N_BIT |= PSR_Z_BIT */
-		return (~temp) & PSR_N_BIT;
-
-	case 0xd: /* le */
-		temp = cpsr ^ (cpsr << 3); /* PSR_N_BIT ^= PSR_V_BIT */
-		temp |= (cpsr << 1);	   /* PSR_N_BIT |= PSR_Z_BIT */
-		return temp & PSR_N_BIT;
-
-	case 0xe: /* al */
-	case 0xf: /* unconditional */
-		return true;
-	}
-	BUG();
-	return false;
+	return (ret != ARM_OPCODE_CONDTEST_FAIL);
 }
 
 static int is_last_scenario;
@@ -1128,7 +1074,9 @@ static unsigned long test_context_cpsr(int scenario)
 
 	if (!test_case_is_thumb) {
 		/* Testing ARM code */
-		probe_should_run = test_check_cc(current_instruction >> 28, cpsr) != 0;
+		int cc = current_instruction >> 28;
+
+		probe_should_run = test_check_cc(cc, cpsr) != 0;
 		if (scenario == 15)
 			is_last_scenario = true;
 
#8 lore raw

PATCH 4/4 Use generic ARM instruction set condition code checks for kprobes.

Quoted 5 lines · Leif Lindholm

This patch changes the kprobes implementation to use the generic ARM instruction set condition code checks, rather than a dedicated implementation.

Quoted 105 lines

---
Tixy's ACK removed as patch modified.

 arch/arm/kernel/kprobes-test.c |   66 ++++------------------------------------
 1 files changed, 7 insertions(+), 59 deletions(-)
arch/arm/kernel/kprobes-test.c +7 -59
index e17cdd6..1862d8f 100644
@@ -202,6 +202,8 @@
 #include <linux/slab.h>
 #include <linux/kprobes.h>
 
+#include <asm/opcodes.h>
+
 #include "kprobes.h"
 #include "kprobes-test.h"
 
@@ -1050,65 +1052,9 @@ static int test_instance;
 
 static unsigned long test_check_cc(int cc, unsigned long cpsr)
 {
-	unsigned long temp;
-
-	switch (cc) {
-	case 0x0: /* eq */
-		return cpsr & PSR_Z_BIT;
-
-	case 0x1: /* ne */
-		return (~cpsr) & PSR_Z_BIT;
-
-	case 0x2: /* cs */
-		return cpsr & PSR_C_BIT;
-
-	case 0x3: /* cc */
-		return (~cpsr) & PSR_C_BIT;
-
-	case 0x4: /* mi */
-		return cpsr & PSR_N_BIT;
-
-	case 0x5: /* pl */
-		return (~cpsr) & PSR_N_BIT;
-
-	case 0x6: /* vs */
-		return cpsr & PSR_V_BIT;
-
-	case 0x7: /* vc */
-		return (~cpsr) & PSR_V_BIT;
+	int ret = arm_check_condition(cc << 28, cpsr);
 
-	case 0x8: /* hi */
-		cpsr &= ~(cpsr >> 1); /* PSR_C_BIT &= ~PSR_Z_BIT */
-		return cpsr & PSR_C_BIT;
-
-	case 0x9: /* ls */
-		cpsr &= ~(cpsr >> 1); /* PSR_C_BIT &= ~PSR_Z_BIT */
-		return (~cpsr) & PSR_C_BIT;
-
-	case 0xa: /* ge */
-		cpsr ^= (cpsr << 3); /* PSR_N_BIT ^= PSR_V_BIT */
-		return (~cpsr) & PSR_N_BIT;
-
-	case 0xb: /* lt */
-		cpsr ^= (cpsr << 3); /* PSR_N_BIT ^= PSR_V_BIT */
-		return cpsr & PSR_N_BIT;
-
-	case 0xc: /* gt */
-		temp = cpsr ^ (cpsr << 3); /* PSR_N_BIT ^= PSR_V_BIT */
-		temp |= (cpsr << 1);	   /* PSR_N_BIT |= PSR_Z_BIT */
-		return (~temp) & PSR_N_BIT;
-
-	case 0xd: /* le */
-		temp = cpsr ^ (cpsr << 3); /* PSR_N_BIT ^= PSR_V_BIT */
-		temp |= (cpsr << 1);	   /* PSR_N_BIT |= PSR_Z_BIT */
-		return temp & PSR_N_BIT;
-
-	case 0xe: /* al */
-	case 0xf: /* unconditional */
-		return true;
-	}
-	BUG();
-	return false;
+	return (ret != ARM_OPCODE_CONDTEST_FAIL);
 }
 
 static int is_last_scenario;
@@ -1128,7 +1074,9 @@ static unsigned long test_context_cpsr(int scenario)
 
 	if (!test_case_is_thumb) {
 		/* Testing ARM code */
-		probe_should_run = test_check_cc(current_instruction >> 28, cpsr) != 0;
+		int cc = current_instruction >> 28;
+
+		probe_should_run = test_check_cc(cc, cpsr) != 0;
 		if (scenario == 15)
 			is_last_scenario = true;
 

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

#9 lore raw

PATCH 4/4 Use generic ARM instruction set condition code checks for kprobes.

In reply to Leif Lindholm
Quoted 11 lines · Leif Lindholm

This patch changes the kprobes implementation to use the generic ARM instruction set condition code checks, rather than a dedicated implementation.

---
Tixy's ACK removed as patch modified.

 arch/arm/kernel/kprobes-test.c |   66 ++++------------------------------------
 1 files changed, 7 insertions(+), 59 deletions(-)

This looks good to me. Given that Tixy's re-acked it, my reviewed-by is fairly worthless, but here it is anyway (take it or leave it!):

Will