From 7db3fa539f2bdff323dc6a707771c560657e2e15 Mon Sep 17 00:00:00 2001 From: Peter Rugg Date: Mon, 20 Apr 2020 11:21:51 +0100 Subject: [PATCH] Add ALU bounds check --- src_Core/ISA/ISA_Decls_CHERI.bsv | 3 ++ .../procs/RV64G_OOO/AluExePipeline.bsv | 15 ++++-- .../RISCY_OOO/procs/RV64G_OOO/RenameStage.bsv | 2 +- src_Core/RISCY_OOO/procs/lib/Decode.bsv | 53 +++++++++++++++---- src_Core/RISCY_OOO/procs/lib/Exec.bsv | 45 ++++++++++++++-- src_Core/RISCY_OOO/procs/lib/ProcTypes.bsv | 46 ++++++++++++++-- .../RISCY_OOO/procs/lib/ReorderBuffer.bsv | 28 ++++++---- 7 files changed, 159 insertions(+), 33 deletions(-) diff --git a/src_Core/ISA/ISA_Decls_CHERI.bsv b/src_Core/ISA/ISA_Decls_CHERI.bsv index 435f162..4d45b26 100755 --- a/src_Core/ISA/ISA_Decls_CHERI.bsv +++ b/src_Core/ISA/ISA_Decls_CHERI.bsv @@ -67,6 +67,9 @@ typedef struct { CHERIException cheri_exc_code; } CSR_XCapCause deriving(Bits, FShow); +CSR_XCapCause noCapCause = CSR_XCapCause {cheri_exc_code: None, + cheri_exc_reg: unpack(0)}; + function Bit#(64) xccsr_to_word(CSR_XCapCause xccsr); return zeroExtend({xccsr.cheri_exc_reg, pack(xccsr.cheri_exc_code), 3'b0, 1'b1, 1'b1}); endfunction diff --git a/src_Core/RISCY_OOO/procs/RV64G_OOO/AluExePipeline.bsv b/src_Core/RISCY_OOO/procs/RV64G_OOO/AluExePipeline.bsv index 727f98e..816c70c 100755 --- a/src_Core/RISCY_OOO/procs/RV64G_OOO/AluExePipeline.bsv +++ b/src_Core/RISCY_OOO/procs/RV64G_OOO/AluExePipeline.bsv @@ -89,7 +89,8 @@ typedef struct { Maybe#(Data) csrData; // data to write CSR file Maybe#(CapPipe) scrData; // datat to write to special capability register file. ControlFlow controlFlow; - Maybe#(CHERIException) capException; + Maybe#(CSR_XCapCause) capException; + Maybe#(BoundsCheck) check; // speculation Maybe#(SpecTag) spec_tag; `ifdef RVFI @@ -157,7 +158,7 @@ interface AluExeInput; Maybe#(Data) csrData, Maybe#(CapPipe) scrData, ControlFlow cf, - Maybe#(CHERIException) capCause, + Maybe#(CSR_XCapCause) capCause, CapPipe pcc `ifdef RVFI , ExtraTraceBundle tb @@ -326,7 +327,8 @@ module mkAluExePipeline#(AluExeInput inIfc)(AluExePipeline); // This means that we will have instructions that both write SCR registers and also get mispredictions, unlike // the CSR file. Given the assertions above, this seems dangerous... scrData: isValid(x.dInst.scr) ? Valid (exec_result.scrData) : tagged Invalid, - capException: isValid(exec_result.capException) ? (Valid (exec_result.capException.Valid.cheri_exc_code)) : Invalid, + capException: exec_result.capException, + check: exec_result.boundsCheck, `ifdef RVFI traceBundle: ExtraTraceBundle{ regWriteData: getAddr(exec_result.data), @@ -352,6 +354,13 @@ module mkAluExePipeline#(AluExeInput inIfc)(AluExePipeline); inIfc.writeRegFile(dst.indx, x.data); end + if (x.check matches tagged Valid .check &&& x.capException matches tagged Invalid) begin + if (!( (check.check_low >= check.authority_base) && + (check.check_inclusive ? (check.check_high <= check.authority_top ) + : (check.check_high < check.authority_top )))) + x.capException = Valid(CSR_XCapCause{cheri_exc_reg: check.authority_idx, cheri_exc_code: LengthViolation}); + end + // update the instruction in the reorder buffer. inIfc.rob_setExecuted( x.tag, diff --git a/src_Core/RISCY_OOO/procs/RV64G_OOO/RenameStage.bsv b/src_Core/RISCY_OOO/procs/RV64G_OOO/RenameStage.bsv index df28f75..92bee4b 100644 --- a/src_Core/RISCY_OOO/procs/RV64G_OOO/RenameStage.bsv +++ b/src_Core/RISCY_OOO/procs/RV64G_OOO/RenameStage.bsv @@ -358,7 +358,7 @@ module mkRenameStage#(RenameInput inIfc)(RenameStage); // This avoids doing incorrect work incrEpochStallFetch; Maybe#(TrapWithCap) trapWithCap = Invalid; - if (firstTrap matches tagged Valid .trap) trapWithCap = tagged Valid TrapWithCap{trap: trap, capExp: None}; + if (firstTrap matches tagged Valid .trap) trapWithCap = tagged Valid TrapWithCap{trap: trap, capExp: noCapCause}; // just place it in the reorder buffer let y = ToReorderBuffer{pc: setAddr(almightyCap, pc).value, orig_inst: orig_inst, diff --git a/src_Core/RISCY_OOO/procs/lib/Decode.bsv b/src_Core/RISCY_OOO/procs/lib/Decode.bsv index 7ec0360..092aa58 100755 --- a/src_Core/RISCY_OOO/procs/lib/Decode.bsv +++ b/src_Core/RISCY_OOO/procs/lib/Decode.bsv @@ -741,7 +741,10 @@ function DecodeResult decode(Instruction inst); dInst.capChecks.src1_tag = True; dInst.capChecks.src1_unsealed = True; - // TODO bounds check + dInst.capChecks.check_enable = True; + dInst.capChecks.check_authority_src = Src1; + dInst.capChecks.check_low_src = Src1Addr; + dInst.capChecks.check_high_src = ResultAddr; dInst.iType = Alu; regs.dst = Valid(tagged Gpr rd); @@ -787,9 +790,12 @@ function DecodeResult decode(Instruction inst); illegalInst = True; dInst.capChecks.src1_tag = True; dInst.capChecks.src1_unsealed = True; - // TODO assert exact here instead of in exec func? + // TODO assert exact - // TODO bounds check + dInst.capChecks.check_enable = True; + dInst.capChecks.check_authority_src = Src1; + dInst.capChecks.check_low_src = Src1Addr; + dInst.capChecks.check_high_src = ResultAddr; dInst.iType = Alu; regs.dst = Valid(tagged Gpr rd); @@ -836,6 +842,12 @@ function DecodeResult decode(Instruction inst); dInst.capChecks.src2_permit_seal = True; dInst.capChecks.src2_addr_valid_type = True; + dInst.capChecks.check_enable = True; + dInst.capChecks.check_authority_src = Src2; + dInst.capChecks.check_low_src = Src2Addr; + dInst.capChecks.check_high_src = Src2Addr; + dInst.capChecks.check_inclusive = False; + dInst.iType = Alu; regs.dst = Valid(tagged Gpr rd); regs.src1 = Valid(tagged Gpr rs1); @@ -880,6 +892,12 @@ function DecodeResult decode(Instruction inst); dInst.capChecks.src2_points_to_src1_type = True; dInst.capChecks.src2_permit_unseal = True; + dInst.capChecks.check_enable = True; + dInst.capChecks.check_authority_src = Src2; + dInst.capChecks.check_low_src = Src2Addr; + dInst.capChecks.check_high_src = Src2Addr; + dInst.capChecks.check_inclusive = False; + dInst.iType = Alu; regs.dst = Valid(tagged Gpr rd); regs.src1 = Valid(tagged Gpr rs1); @@ -901,6 +919,11 @@ function DecodeResult decode(Instruction inst); dInst.capChecks.src1_tag = True; dInst.capChecks.src1_unsealed = True; + dInst.capChecks.check_enable = True; + dInst.capChecks.check_authority_src = Src1; + dInst.capChecks.check_low_src = Src2Type; + dInst.capChecks.check_high_src = Src2Type; + dInst.iType = Alu; regs.dst = Valid(tagged Gpr rd); regs.src1 = Valid(tagged Gpr rs1); @@ -970,17 +993,25 @@ function DecodeResult decode(Instruction inst); dInst.execFunc = Alu (Sub); end f7_cap_CBuildCap: begin - illegalInst = True; - // TODO - dInst.capChecks.src1_tag = True; - dInst.capChecks.src1_unsealed = True; - dInst.capChecks.src2_perm_subset_src1 = True; - dInst.capChecks.src2_derivable = True; + dInst.capChecks.src2_tag = True; + dInst.capChecks.src2_unsealed = True; + dInst.capChecks.src1_perm_subset_src2 = True; + dInst.capChecks.src1_derivable = True; + dInst.capChecks.check_enable = True; + dInst.capChecks.check_authority_src = Src2; + dInst.capChecks.check_low_src = Src1Base; + dInst.capChecks.check_high_src = Src1Top; + + // Swap arguments so SCR possibly goes in RS2 dInst.iType = Alu; regs.dst = Valid(tagged Gpr rd); - regs.src1 = Valid(tagged Gpr rs1); - regs.src2 = Valid(tagged Gpr rs2); + regs.src1 = Valid(tagged Gpr rs2); + if (rs1 == 0) begin + dInst.scr = Valid(SCR_DDC); + end else begin + regs.src2 = Valid(tagged Gpr rs1); + end dInst.imm = Invalid; dInst.execFunc = CapModify (BuildCap); end diff --git a/src_Core/RISCY_OOO/procs/lib/Exec.bsv b/src_Core/RISCY_OOO/procs/lib/Exec.bsv index 8fd5270..528a67c 100755 --- a/src_Core/RISCY_OOO/procs/lib/Exec.bsv +++ b/src_Core/RISCY_OOO/procs/lib/Exec.bsv @@ -66,11 +66,49 @@ function Maybe#(CapException) capChecks(CapPipe a, CapPipe b, CapChecks toCheck) result = e2(TypeViolation); else if (toCheck.src2_addr_valid_type && !validAsType(b, truncate(getAddr(b)))) result = e2(LengthViolation); - else if (toCheck.src2_perm_subset_src1 && (getPerms(a) & getPerms(b)) != getPerms(b)) + else if (toCheck.src1_perm_subset_src2 && (getPerms(a) & getPerms(b)) != getPerms(a)) result = e2(SoftwarePermViolation); + else if (toCheck.src1_derivable && !isDerivable(a)) + result = e1(LengthViolation); return result; endfunction +(* noinline *) +function Maybe#(BoundsCheck) prepareBoundsCheck(CapPipe a, CapPipe b, CapPipe c, CapChecks toCheck); + BoundsCheck ret = ?; + CapPipe authority = ?; + case(toCheck.check_authority_src) + Src1: begin + authority = a; + ret.authority_idx = toCheck.rn1; + end + Src2: begin + authority = b; + ret.authority_idx = toCheck.rn2; + end + endcase + ret.authority_base = getBase(authority); + ret.authority_top = getTop(authority); + + case(toCheck.check_low_src) + Src1Addr: ret.check_low = getAddr(a); + Src1Base: ret.check_low = getBase(a); + Src2Addr: ret.check_low = getAddr(b); + Src2Type: ret.check_low = zeroExtend(getType(b)); + endcase + + case(toCheck.check_high_src) + Src1Top: ret.check_high = getTop(a); + Src2Addr: ret.check_high = {0,getAddr(b)}; + Src2Type: ret.check_high = zeroExtend(getType(b)); + ResultAddr: ret.check_high = {0,getAddr(c)}; + endcase + + ret.check_inclusive = toCheck.check_inclusive; + if (toCheck.check_enable) return Valid(ret); + else return Invalid; +endfunction + (* noinline *) function Data alu(Data a, Data b, AluFunc func); Data res = (case(func) @@ -245,7 +283,8 @@ function ExecResult basicExec(DecodedInst dInst, CapPipe rVal1, CapPipe rVal2, C Data inspect_result = capInspect(rVal1, aluVal2, dInst.execFunc.CapInspect); CapModifyFunc modFunc = ccall ? (Unseal (Src2)):dInst.execFunc.CapModify; CapPipe modify_result = capModify(rVal1, aluVal2, modFunc); - Maybe#(CapException) capException = capChecks(rVal1, aluVal2, dInst.capChecks); // TODO use this to throw exceptions + Maybe#(CapException) capException = capChecks(rVal1, aluVal2, dInst.capChecks); + Maybe#(BoundsCheck) boundsCheck = prepareBoundsCheck(rVal1, aluVal2, modify_result, dInst.capChecks); CapPipe cap_alu_result = case (dInst.execFunc) matches tagged CapInspect .x: nullWithAddr(inspect_result); tagged CapModify .x: modify_result; @@ -281,7 +320,7 @@ function ExecResult basicExec(DecodedInst dInst, CapPipe rVal1, CapPipe rVal2, C endcase); CapPipe scr_data = modify_result; - return ExecResult{data: data, csrData: csr_data, scrData: scr_data, addr: addr, controlFlow: cf, capException: capException}; + return ExecResult{data: data, csrData: csr_data, scrData: scr_data, addr: addr, controlFlow: cf, capException: capException, boundsCheck: boundsCheck}; endfunction (* noinline *) diff --git a/src_Core/RISCY_OOO/procs/lib/ProcTypes.bsv b/src_Core/RISCY_OOO/procs/lib/ProcTypes.bsv index d70b4a1..ef1f274 100755 --- a/src_Core/RISCY_OOO/procs/lib/ProcTypes.bsv +++ b/src_Core/RISCY_OOO/procs/lib/ProcTypes.bsv @@ -572,8 +572,8 @@ typedef union tagged { typedef struct { Trap trap; - CHERIException capExp; -} TrapWithCap deriving(Bits, Eq, FShow); + CSR_XCapCause capExp; +} TrapWithCap deriving(Bits, FShow); // privilege modes Bit#(2) prvU = 0; @@ -676,6 +676,36 @@ typedef struct { Bool illegalInst; } DecodeResult deriving(Bits, Eq, FShow); +typedef enum { + Src1, + Src2 +} CheckAuthoritySrc deriving(Bits, Eq, FShow); + +typedef enum { + Src1Addr, + Src2Addr, + Src2Type, + Src1Base +} CheckLowSrc deriving(Bits, Eq, FShow); + +typedef enum { + Src1Top, + Src2Addr, + Src2Type, + ResultAddr +} CheckHighSrc deriving(Bits, Eq, FShow); + +typedef struct { + Data authority_base; + CapTop authority_top; + Bit#(6) authority_idx; + Data check_low; + CapTop check_high; + Bool check_inclusive; +} BoundsCheck deriving(Bits, Eq, FShow); + +typedef Bit#(65) CapTop; + typedef Bit#(32) ImmData; // 32-bit decoded immediate data typedef struct { @@ -695,8 +725,15 @@ typedef struct { Bool src2_permit_seal; Bool src2_points_to_src1_type; Bool src2_addr_valid_type; - Bool src2_perm_subset_src1; - Bool src2_derivable; + Bool src1_perm_subset_src2; + Bool src1_derivable; + + Bool check_enable; + CheckAuthoritySrc check_authority_src; + CheckLowSrc check_low_src; + CheckHighSrc check_high_src; + Bool check_inclusive; + Bit#(6) rn1; Bit#(6) rn2; } CapChecks deriving(Bits, Eq, FShow); @@ -723,6 +760,7 @@ typedef struct { CapPipe addr; ControlFlow controlFlow; Maybe#(CapException) capException; + Maybe#(BoundsCheck) boundsCheck; } ExecResult deriving(Bits, FShow); // MMIO diff --git a/src_Core/RISCY_OOO/procs/lib/ReorderBuffer.bsv b/src_Core/RISCY_OOO/procs/lib/ReorderBuffer.bsv index 7932aae..4a9ad86 100755 --- a/src_Core/RISCY_OOO/procs/lib/ReorderBuffer.bsv +++ b/src_Core/RISCY_OOO/procs/lib/ReorderBuffer.bsv @@ -117,7 +117,7 @@ interface Row_setExecuted_doFinishAlu; Maybe#(Data) csrData, Maybe#(CapPipe) scrData, ControlFlow cf, - Maybe#(CHERIException) cause, + Maybe#(CSR_XCapCause) cause, CapPipe pcc `ifdef RVFI , ExtraTraceBundle tb @@ -289,7 +289,7 @@ module mkReorderBufferRowEhr(ReorderBufferRowEhr#(aluExeNum, fpuMulDivExeNum)) p Maybe#(Data) csrData, Maybe#(CapPipe) scrData, ControlFlow cf, - Maybe#(CHERIException) cause, + Maybe#(CSR_XCapCause) cause, CapPipe pcc `ifdef RVFI , ExtraTraceBundle tb @@ -313,10 +313,12 @@ module mkReorderBufferRowEhr(ReorderBufferRowEhr#(aluExeNum, fpuMulDivExeNum)) p CapPipe new_pcc = setAddrUnsafe(pcc, getAddr(pc[pc_finishAlu_port(i)])); pc[pc_finishAlu_port(i)] <= new_pcc; if (!isInBounds(new_pcc, False)) begin - trap[trap_finishAlu_port(i)] <= Valid (TrapWithCap{trap: tagged Exception CHERIFault, capExp: LengthViolation}); + trap[trap_finishAlu_port(i)] <= Valid (TrapWithCap{trap: tagged Exception CHERIFault, + capExp: CSR_XCapCause {cheri_exc_code: LengthViolation, + cheri_exc_reg: {1,pack(SCR_PCC)}}}); tval[trap_finishAlu_port(i)] <= tval[trap_finishAlu_port(i)]; end else if (cause matches tagged Valid .exp) begin - trap[trap_finishAlu_port(i)] <= Valid (TrapWithCap{trap: tagged Exception CHERIFault, capExp: fromMaybe(None, cause)}); + trap[trap_finishAlu_port(i)] <= Valid (TrapWithCap{trap: tagged Exception CHERIFault, capExp: exp}); tval[trap_finishAlu_port(i)] <= tval[trap_finishAlu_port(i)]; end `ifdef RVFI @@ -346,10 +348,12 @@ module mkReorderBufferRowEhr(ReorderBufferRowEhr#(aluExeNum, fpuMulDivExeNum)) p fflags[fflags_finishFpuMulDiv_port(i)] <= new_fflags; CapPipe new_pcc = setAddrUnsafe(pcc, getAddr(pc[pc_finishAlu_port(i)])); if (!isInBounds(new_pcc, False)) begin - trap[trap_finishFpuMulDiv_port(i)] <= Valid (TrapWithCap{trap: tagged Exception CHERIFault, capExp: LengthViolation}); + trap[trap_finishFpuMulDiv_port(i)] <= Valid (TrapWithCap{trap: tagged Exception CHERIFault, + capExp: CSR_XCapCause {cheri_exc_code: LengthViolation, + cheri_exc_reg: {1,pack(SCR_PCC)}}}); tval[trap_finishFpuMulDiv_port(i)] <= tval[trap_finishAlu_port(i)]; end else if (cause matches tagged Valid .exp) begin - trap[trap_finishFpuMulDiv_port(i)] <= Valid (TrapWithCap{trap: tagged Exception exp, capExp: None}); + trap[trap_finishFpuMulDiv_port(i)] <= Valid (TrapWithCap{trap: tagged Exception exp, capExp: noCapCause}); tval[trap_finishFpuMulDiv_port(i)] <= tval[trap_finishAlu_port(i)]; end //pc[pc_finishFpuMulDiv_port(i)] <= newPcc; //XXX add pcc checks on FPU instructions @@ -403,10 +407,12 @@ module mkReorderBufferRowEhr(ReorderBufferRowEhr#(aluExeNum, fpuMulDivExeNum)) p CapPipe new_pcc = setAddrUnsafe(pcc, getAddr(pc[pc_finishMem_port])); pc[pc_finishMem_port] <= new_pcc; if (!isInBounds(new_pcc, False)) begin - mem_early_trap[0] <= Valid (TrapWithCap{trap: tagged Exception CHERIFault, capExp: LengthViolation}); + mem_early_trap[0] <= Valid (TrapWithCap{trap: tagged Exception CHERIFault, + capExp: CSR_XCapCause {cheri_exc_code: LengthViolation, + cheri_exc_reg: {1,pack(SCR_PCC)}}}); tval[trap_finishMem_port] <= tval[trap_finishMem_port]; end else if (cause matches tagged Valid .exp) begin - mem_early_trap[0] <= Valid (TrapWithCap{trap: tagged Exception exp, capExp: None}); + mem_early_trap[0] <= Valid (TrapWithCap{trap: tagged Exception exp, capExp: noCapCause}); tval[trap_finishMem_port] <= tval[trap_finishMem_port]; end endmethod @@ -533,7 +539,7 @@ module mkReorderBufferRowEhr(ReorderBufferRowEhr#(aluExeNum, fpuMulDivExeNum)) p // record trap //doAssert(!isValid(trap[trap_deqLSQ_port]), "cannot have trap"); if (isValid(mem_early_trap[0])) trap[trap_deqLSQ_port] <= mem_early_trap[0]; - else if(cause matches tagged Valid .e) trap[trap_deqLSQ_port] <= Valid (TrapWithCap{trap: tagged Exception e, capExp: None}); + else if(cause matches tagged Valid .e) trap[trap_deqLSQ_port] <= Valid (TrapWithCap{trap: tagged Exception e, capExp: noCapCause}); // TODO: shouldn't we record tval here as well? // record ld misspeculation ldKilled[ldKill_deqLSQ_port] <= ld_killed; @@ -595,7 +601,7 @@ interface ROB_setExecuted_doFinishAlu; Maybe#(Data) csrData, Maybe#(CapPipe) scrData, ControlFlow cf, - Maybe#(CHERIException) cause, + Maybe#(CSR_XCapCause) cause, CapPipe pcc `ifdef RVFI , ExtraTraceBundle tb @@ -1144,7 +1150,7 @@ module mkSupReorderBuffer#( Maybe#(Data) csrData, Maybe#(CapPipe) scrData, ControlFlow cf, - Maybe#(CHERIException) cause, + Maybe#(CSR_XCapCause) cause, CapPipe pcc `ifdef RVFI , ExtraTraceBundle tb