From f2f2285f75b7fa902298cf637806e52ae9725ff5 Mon Sep 17 00:00:00 2001 From: Jonathan Woodruff Date: Wed, 25 Mar 2020 15:20:03 +0000 Subject: [PATCH] Check the bounds on PCC and report the correct exception in Xcause registers. This required makeing the Exception type wider by one. The actual "inBounds" check is currently implemented in the reorder buffer rows, which duplicates the logic ~80 times (number of outstanding instructions), which isn't ideal, but it's using the quick in-bounds check that only compares the mantissa-sized things. --- src_Core/CPU/CsrFile.bsv | 32 +++++++------- src_Core/RISCY_OOO/procs/lib/ProcTypes.bsv | 40 ++++++++--------- .../RISCY_OOO/procs/lib/ReorderBuffer.bsv | 43 ++++++++++++++----- 3 files changed, 70 insertions(+), 45 deletions(-) diff --git a/src_Core/CPU/CsrFile.bsv b/src_Core/CPU/CsrFile.bsv index b4337f0..61c5d03 100644 --- a/src_Core/CPU/CsrFile.bsv +++ b/src_Core/CPU/CsrFile.bsv @@ -1,7 +1,7 @@ // Copyright (c) 2017 Massachusetts Institute of Technology // Portions Copyright (c) 2019-2020 Bluespec, Inc. -// +// // Permission is hereby granted, free of charge, to any person // obtaining a copy of this software and associated documentation // files (the "Software"), to deal in the Software without @@ -9,10 +9,10 @@ // modify, merge, publish, distribute, sublicense, and/or sell copies // of the Software, and to permit persons to whom the Software is // furnished to do so, subject to the following conditions: -// +// // The above copyright notice and this permission notice shall be // included in all copies or substantial portions of the Software. -// +// // THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, // EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF // MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND @@ -49,6 +49,8 @@ import SoC_Map :: *; // ================================================================ // Information returned on traps and mret/sret/uret +typedef Bit#(SizeOf#(Exception)) Cause; + typedef struct { Addr new_pc; @@ -307,7 +309,7 @@ module mkCsrFile #(Data hartid)(CsrFile); // current prv level (this is not a csr...) Reg#(Bit#(2)) prv_reg <- mkCsrReg(prvM); - + // Machine level CSRs // mstatus Reg#(Bit#(2)) xs_reg <- mkReadOnlyReg(0); // XXX no extension @@ -441,12 +443,12 @@ module mkCsrFile #(Data hartid)(CsrFile); Reg#(Data) mepc_csr <- mkCsrReg(0); // mcause Reg#(Bit#(1)) mcause_interrupt_reg <- mkCsrReg(0); - Reg#(Bit#(4)) mcause_code_reg <- mkCsrReg(0); + Reg#(Cause) mcause_code_reg <- mkCsrReg(0); Reg#(Data) mcause_csr = concatReg3( - mcause_interrupt_reg, readOnlyReg(59'b0), mcause_code_reg + mcause_interrupt_reg, readOnlyReg(0), mcause_code_reg ); - function Data fn_mcause_val (Bit #(1) mcause_interrupt_val, Bit #(4) mcause_code_val); - return { mcause_interrupt_val, 59'b0, mcause_code_val }; + function Data fn_mcause_val (Bit #(1) mcause_interrupt_val, Cause mcause_code_val); + return { mcause_interrupt_val, 'b0, mcause_code_val }; endfunction // mtval (mbadaddr in spike) @@ -560,12 +562,12 @@ module mkCsrFile #(Data hartid)(CsrFile); Reg#(Data) sepc_csr <- mkCsrReg(0); // scause Reg#(Bit#(1)) scause_interrupt_reg <- mkCsrReg(0); - Reg#(Bit#(4)) scause_code_reg <- mkCsrReg(0); + Reg#(Cause) scause_code_reg <- mkCsrReg(0); Reg#(Data) scause_csr = concatReg3( - scause_interrupt_reg, readOnlyReg(59'b0), scause_code_reg + scause_interrupt_reg, readOnlyReg('b0), scause_code_reg ); - function Data fn_scause_val (Bit #(1) scause_interrupt_val, Bit #(4) scause_code_val); - return { scause_interrupt_val, 59'b0, scause_code_val }; + function Data fn_scause_val (Bit #(1) scause_interrupt_val, Cause scause_code_val); + return { scause_interrupt_val, 0, scause_code_val }; endfunction // stval (sbadaddr in spike) @@ -598,7 +600,7 @@ module mkCsrFile #(Data hartid)(CsrFile); // User level CSRs // According to spike, any write to fflags/frm/fcsr will set fs_reg as // dirty, regardless of whether the write truly changes value or not. - // Besides, any non-zero FP exception flags will also make fs_reg dirty. + // Besides, any non-zero FP exception flags will also make fs_reg dirty. // fflags: if we directly change fflags_reg (instead of fflags_csr), then // we must set fs_reg manually Reg#(Bit#(5)) fflags_reg <- mkCsrReg(0); @@ -960,7 +962,7 @@ module mkCsrFile #(Data hartid)(CsrFile); method ActionValue#(Trap_Updates) trap(Trap t, Addr pc, Addr addr, Bit #(32) orig_inst); // figure out trap cause & trap val Bit#(1) cause_interrupt = 0; - Bit#(4) cause_code = 0; + Cause cause_code = 0; Data trap_val = 0; case(t) matches tagged Exception .e: begin @@ -978,7 +980,7 @@ module mkCsrFile #(Data hartid)(CsrFile); endcase); end tagged Interrupt .i: begin - cause_code = pack(i); + cause_code = zeroExtend(pack(i)); cause_interrupt = 1; end endcase diff --git a/src_Core/RISCY_OOO/procs/lib/ProcTypes.bsv b/src_Core/RISCY_OOO/procs/lib/ProcTypes.bsv index ce1e3d8..ce6cf33 100644 --- a/src_Core/RISCY_OOO/procs/lib/ProcTypes.bsv +++ b/src_Core/RISCY_OOO/procs/lib/ProcTypes.bsv @@ -1,7 +1,7 @@ // Copyright (c) 2017 Massachusetts Institute of Technology // Portions (c) 2020 Bluespec, Inc. -// +// // Permission is hereby granted, free of charge, to any person // obtaining a copy of this software and associated documentation // files (the "Software"), to deal in the Software without @@ -9,10 +9,10 @@ // modify, merge, publish, distribute, sublicense, and/or sell copies // of the Software, and to permit persons to whom the Software is // furnished to do so, subject to the following conditions: -// +// // The above copyright notice and this permission notice shall be // included in all copies or substantial portions of the Software. -// +// // THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, // EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF // MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND @@ -195,7 +195,7 @@ function Opcode unpackOpcode(Bit#(7) x); return (case(x) pack(Opcode'(Load )): (Load ); pack(Opcode'(LoadFp )): (LoadFp ); - pack(Opcode'(MiscMem)): (MiscMem); + pack(Opcode'(MiscMem)): (MiscMem); pack(Opcode'(OpImm )): (OpImm ); pack(Opcode'(Auipc )): (Auipc ); pack(Opcode'(OpImm32)): (OpImm32); @@ -455,20 +455,21 @@ typedef enum { } RVRoundMode deriving(Bits, Eq, FShow); typedef enum { - InstAddrMisaligned = 4'd0, - InstAccessFault = 4'd1, - IllegalInst = 4'd2, - Breakpoint = 4'd3, - LoadAddrMisaligned = 4'd4, - LoadAccessFault = 4'd5, - StoreAddrMisaligned = 4'd6, - StoreAccessFault = 4'd7, - EnvCallU = 4'd8, - EnvCallS = 4'd9, - EnvCallM = 4'd11, - InstPageFault = 4'd12, - LoadPageFault = 4'd13, - StorePageFault = 4'd15 + InstAddrMisaligned = 5'd0, + InstAccessFault = 5'd1, + IllegalInst = 5'd2, + Breakpoint = 5'd3, + LoadAddrMisaligned = 5'd4, + LoadAccessFault = 5'd5, + StoreAddrMisaligned = 5'd6, + StoreAccessFault = 5'd7, + EnvCallU = 5'd8, + EnvCallS = 5'd9, + EnvCallM = 5'd11, + InstPageFault = 5'd12, + LoadPageFault = 5'd13, + StorePageFault = 5'd15, + CapabilityFault = 5'd28 } Exception deriving(Bits, Eq, FShow); typedef enum { @@ -968,7 +969,7 @@ function Fmt showInst(Instruction inst); privMRET: fshow("mret"); privWFI: fshow("wfi"); default: ( - funct7 == privSFENCEVMA ? + funct7 == privSFENCEVMA ? (fshow("sfence.vma ") + fshow(rs1) + fshow(" ") + fshow(rs2)) : fshow("SYSTEM not implemented") ); @@ -985,4 +986,3 @@ function Fmt showInst(Instruction inst); return ret; endfunction - diff --git a/src_Core/RISCY_OOO/procs/lib/ReorderBuffer.bsv b/src_Core/RISCY_OOO/procs/lib/ReorderBuffer.bsv index 5ed90d6..f42761e 100644 --- a/src_Core/RISCY_OOO/procs/lib/ReorderBuffer.bsv +++ b/src_Core/RISCY_OOO/procs/lib/ReorderBuffer.bsv @@ -178,9 +178,10 @@ module mkReorderBufferRowEhr(ReorderBufferRowEhr#(aluExeNum, fpuMulDivExeNum)) p ); Integer trap_deq_port = 0; function Integer trap_finishAlu_port(Integer i) = i; - Integer trap_deqLSQ_port = valueof(aluExeNum); - Integer trap_finishMem_port = valueof(aluExeNum); // write trap - Integer trap_enq_port = 1 + valueof(aluExeNum); + function Integer trap_finishFpuMulDiv_port(Integer i) = valueof(aluExeNum) + i; + Integer trap_deqLSQ_port = valueof(TAdd#(aluExeNum, TDiv#(aluExeNum,2))); + Integer trap_finishMem_port = valueof(TAdd#(aluExeNum, TDiv#(aluExeNum,2))); // write trap + Integer trap_enq_port = 1 + valueof(TAdd#(aluExeNum, TDiv#(aluExeNum,2))); Integer pc_deq_port = 0; function Integer pc_finishAlu_port(Integer i) = i; @@ -241,8 +242,8 @@ module mkReorderBufferRowEhr(ReorderBufferRowEhr#(aluExeNum, fpuMulDivExeNum)) p `endif Reg#(Maybe#(CSR)) csr <- mkRegU; Reg#(Bool) claimed_phy_reg <- mkRegU; - Ehr#(TAdd#(2, aluExeNum), Maybe#(Trap)) trap <- mkEhr(?); - Ehr#(TAdd#(2, aluExeNum), Addr) tval <- mkEhr(?); + Ehr#(TAdd#(TAdd#(2, TDiv#(aluExeNum,2)), aluExeNum), Maybe#(Trap)) trap <- mkEhr(?); + Ehr#(TAdd#(TAdd#(2, TDiv#(aluExeNum,2)), aluExeNum), Addr) tval <- mkEhr(?); Ehr#(TAdd#(2, aluExeNum), PPCVAddrCSRData) ppc_vaddr_csrData <- mkEhr(?); Ehr#(TAdd#(1, fpuMulDivExeNum), Bit#(5)) fflags <- mkEhr(?); Reg#(Bool) will_dirty_fpu_state <- mkRegU; @@ -294,9 +295,15 @@ module mkReorderBufferRowEhr(ReorderBufferRowEhr#(aluExeNum, fpuMulDivExeNum)) p else begin ppc_vaddr_csrData[pvc_finishAlu_port(i)] <= PPC (setAddr(almightyCap, cf.nextPc).value); end - pc[pc_finishAlu_port(i)] <= setAddrUnsafe(pcc, getAddr(pc[pc_finishAlu_port(i)])); - trap[trap_finishAlu_port(i)] <= trap[trap_finishAlu_port(i)]; - tval[trap_finishAlu_port(i)] <= tval[trap_finishAlu_port(i)]; + 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 (tagged Exception CapabilityFault); + 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 (tagged Exception exp); + tval[trap_finishAlu_port(i)] <= tval[trap_finishAlu_port(i)]; + end `ifdef RVFI //$display("%t : traceBundle = ", $time(), fshow(tb), " in Row_setExecuted_doFinishAlu for %x", pc); traceBundle[pvc_finishAlu_port(i)] <= tb; @@ -315,7 +322,15 @@ module mkReorderBufferRowEhr(ReorderBufferRowEhr#(aluExeNum, fpuMulDivExeNum)) p rg_dst_data <= dst_data; // update fflags fflags[fflags_finishFpuMulDiv_port(i)] <= new_fflags; - //pc[pc_finishFpuMulDiv_port(i)] <= setAddrUnsafe(pcc, getAddr(pc[pc_finishFpuMulDiv_port(i)])).value; //XXX add pcc checks on FPU instructions + CapPipe new_pcc = setAddrUnsafe(pcc, getAddr(pc[pc_finishAlu_port(i)])); + if (!isInBounds(new_pcc, False)) begin + trap[trap_finishFpuMulDiv_port(i)] <= Valid (tagged Exception CapabilityFault); + 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 (tagged Exception exp); + tval[trap_finishFpuMulDiv_port(i)] <= tval[trap_finishAlu_port(i)]; + end + //pc[pc_finishFpuMulDiv_port(i)] <= newPcc; //XXX add pcc checks on FPU instructions endmethod endinterface); end @@ -359,7 +374,15 @@ module mkReorderBufferRowEhr(ReorderBufferRowEhr#(aluExeNum, fpuMulDivExeNum)) p memAccessAtCommit[accessCom_finishMem_port] <= access_at_commit; // udpate non mmio st nonMMIOStDone[nonMMIOSt_finishMem_port] <= non_mmio_st_done; - pc[pc_finishMem_port] <= setAddrUnsafe(pcc, getAddr(pc[pc_finishMem_port])); + CapPipe new_pcc = setAddrUnsafe(pcc, getAddr(pc[pc_finishMem_port])); + pc[pc_finishMem_port] <= new_pcc; + if (!isInBounds(new_pcc, False)) begin + trap[trap_finishMem_port] <= Valid (tagged Exception CapabilityFault); + tval[trap_finishMem_port] <= tval[trap_finishMem_port]; + end else if (cause matches tagged Valid .exp) begin + trap[trap_finishMem_port] <= Valid (tagged Exception exp); + tval[trap_finishMem_port] <= tval[trap_finishMem_port]; + end endmethod `ifdef INCLUDE_TANDEM_VERIF