From: jsg <jsg@openbsd.org>
Date: Sun, 9 May 2021 23:22:25 +0000 (+0000)
Subject: fpu_valid_opcode() did not correctly handle 16 bit fp instructions
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=a3b8ef658ea40424e94905915b454b8102d7973a;p=openbsd

fpu_valid_opcode() did not correctly handle 16 bit fp instructions
such as an stval of 0xaa22.  The RISC-V Instruction Set Manual states
that setting stval to a non-zero value with the instruction on illegal
instruction exception is an optional feature so instead of changing
fpu_valid_opcode() remove it entirely.

ok deraadt@ kettenis@ drahn@
---

diff --git a/sys/arch/riscv64/include/cpu.h b/sys/arch/riscv64/include/cpu.h
index db001abf4c6..0a7f7bff3ed 100644
--- a/sys/arch/riscv64/include/cpu.h
+++ b/sys/arch/riscv64/include/cpu.h
@@ -275,7 +275,6 @@ intr_restore(u_long s)
 void	delay (unsigned);
 #define	DELAY(x)	delay(x)
 
-int fpu_valid_opcode(uint32_t);
 void fpu_save(struct proc *, struct trapframe *);
 void fpu_load(struct proc *);
 void fpu_discard(struct proc *p);
diff --git a/sys/arch/riscv64/riscv64/fpu.c b/sys/arch/riscv64/riscv64/fpu.c
index beb90897e17..c82758ea809 100644
--- a/sys/arch/riscv64/riscv64/fpu.c
+++ b/sys/arch/riscv64/riscv64/fpu.c
@@ -27,49 +27,6 @@ fpu_clear(struct fpreg *fp)
 	bzero(fp, sizeof (*fp));
 }
 
-// may look into optimizing this, bit map lookup ?
-
-int
-fpu_valid_opcode(uint32_t instr)
-{
-	int opcode = instr & 0x7f;
-	int valid = 0;
-
-	if ((opcode & 0x3) == 0x3) {
-		/* 32 bit instruction */
-		switch(opcode) {
-		case 0x07:	// LOAD-FP
-		case 0x27:	// STORE-FP
-		case 0x53:	// OP-FP
-			valid = 1;
-			break;
-		default:
-			;
-		}
-	} else {
-		/* 16 bit instruction */
-		int opcode16 = instr & 0xe003;
-		switch (opcode16) {
-		case 0x1000:	// C.FLD
-		case 0xa000:	// C.SLD
-			valid = 1;
-			break;
-		case 0x2002:	// C.FLDSP
-			// must verify dest register is float
-			valid = opcode16 & (1 << 11);
-			break;
-		case 0xa002:	// C.FSDSP
-			// must verify dest register is float
-			valid = opcode16 & (1 << 6);
-			break;
-		default:
-			;
-		}
-	}
-	//printf("FPU check requested %d\n", valid);
-	return valid;
-}
-
 void
 fpu_discard(struct proc *p)
 {
diff --git a/sys/arch/riscv64/riscv64/trap.c b/sys/arch/riscv64/riscv64/trap.c
index 82ec476ad16..66177ed85e8 100644
--- a/sys/arch/riscv64/riscv64/trap.c
+++ b/sys/arch/riscv64/riscv64/trap.c
@@ -162,16 +162,9 @@ do_trap_user(struct trapframe *frame)
 		break;
 	case EXCP_ILLEGAL_INSTRUCTION:
 		if ((frame->tf_sstatus & SSTATUS_FS_MASK) == SSTATUS_FS_OFF) {
-			if (fpu_valid_opcode(frame->tf_stval)) {
-
-				/* XXX do this here or should it be in the
-				 * trap handler in the restore path?
-				 */
-				fpu_load(p);
-
-				frame->tf_sstatus &= ~SSTATUS_FS_MASK;
-				break;
-			}
+			fpu_load(p);
+			frame->tf_sstatus &= ~SSTATUS_FS_MASK;
+			break;
 		}
 		printf("ILL at %lx scause %lx stval %lx\n", frame->tf_sepc,
 		    frame->tf_scause, frame->tf_stval);