[otbn,dv] Remove weak attributes for some more DPI functions The cunning trick of using a weak symbol for a DPI export that may or may not exist doesn't work on Xcelium. They were needed here because we supported a use case for otbn_core_model where there was no RTL to inspect. Fortunately, that use case is no longer supported (as of commit 0d0fa1a09) so we can drop them entirely. The other associated changes in the commit add some assertions to make sure that the DesignScope parameter is a nonempty string. Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>
diff --git a/hw/ip/otbn/dv/model/otbn_core_model.sv b/hw/ip/otbn/dv/model/otbn_core_model.sv index c2131c8..1eae2e0 100644 --- a/hw/ip/otbn/dv/model/otbn_core_model.sv +++ b/hw/ip/otbn/dv/model/otbn_core_model.sv
@@ -20,9 +20,9 @@ // The scope that contains the instruction and data memory (for DPI) parameter string MemScope = "", - // Scope of an RTL OTBN implementation (for DPI). If empty, this is a "standalone" model, which - // should update DMEM on completion. If not empty, we assume it's the scope for the top-level of a - // real implementation running alongside and we check DMEM contents on completion. + // Scope of an RTL OTBN implementation (for DPI). This should be give the scope for the top-level + // of a real implementation running alongside. We will use it to check DMEM and register file + // contents on completion of an operation. parameter string DesignScope = "", // Enable internal secure wipe @@ -269,34 +269,30 @@ assign status_o = status_q; assign insn_cnt_o = insn_cnt_q; - // If DesignScope is not empty, we have a design to check. Bind a copy of otbn_rf_snooper_if into - // each register file. The otbn_model_check() function will use these to extract memory contents. - if (DesignScope != "") begin: g_check_design - // TODO: This bind is by module, rather than by instance, because I couldn't get the by-instance - // syntax plus upwards name referencing to work with Verilator. Obviously, this won't work with - // multiple OTBN instances, so it would be nice to get it right. - bind otbn_rf_base_ff otbn_rf_snooper_if #( - .Width (BaseIntgWidth), - .Depth (NGpr), - .IntegrityEnabled(1) - ) u_snooper ( - .rf (rf_reg) - ); + // TODO: This bind is by module, rather than by instance, because I couldn't get the by-instance + // syntax plus upwards name referencing to work with Verilator. Obviously, this won't work with + // multiple OTBN instances, so it would be nice to get it right. + bind otbn_rf_base_ff otbn_rf_snooper_if #( + .Width (BaseIntgWidth), + .Depth (NGpr), + .IntegrityEnabled(1) + ) u_snooper ( + .rf (rf_reg) + ); - bind otbn_rf_bignum_ff otbn_rf_snooper_if #( - .Width (ExtWLEN), - .Depth (NWdr), - .IntegrityEnabled(1) - ) u_snooper ( - .rf (rf) - ); + bind otbn_rf_bignum_ff otbn_rf_snooper_if #( + .Width (ExtWLEN), + .Depth (NWdr), + .IntegrityEnabled(1) + ) u_snooper ( + .rf (rf) + ); - bind otbn_rf_base otbn_stack_snooper_if #(.StackIntgWidth(39), .StackWidth(32), .StackDepth(8)) - u_call_stack_snooper ( - .stack_storage(u_call_stack.stack_storage), - .stack_wr_ptr_q(u_call_stack.stack_wr_ptr_q) - ); - end + bind otbn_rf_base otbn_stack_snooper_if #(.StackIntgWidth(39), .StackWidth(32), .StackDepth(8)) + u_call_stack_snooper ( + .stack_storage(u_call_stack.stack_storage), + .stack_wr_ptr_q(u_call_stack.stack_wr_ptr_q) + ); assign err_o = failed_step | failed_check | check_mismatch_q | failed_lc_escalate;
diff --git a/hw/ip/otbn/dv/model/otbn_model.cc b/hw/ip/otbn/dv/model/otbn_model.cc index cd762a5..fcf3e68 100644 --- a/hw/ip/otbn/dv/model/otbn_model.cc +++ b/hw/ip/otbn/dv/model/otbn_model.cc
@@ -19,12 +19,8 @@ #include "sv_utils.h" extern "C" { -// These functions are only implemented if DesignScope != "", i.e. if we're -// running a block-level simulation. Code needs to check at runtime if -// otbn_rf_peek() and otbn_stack_element_peek() are available before calling -// them. -int otbn_rf_peek(int index, svBitVecVal *val) __attribute__((weak)); -int otbn_stack_element_peek(int index, svBitVecVal *val) __attribute__((weak)); +int otbn_rf_peek(int index, svBitVecVal *val); +int otbn_stack_element_peek(int index, svBitVecVal *val); } #define RUNNING_BIT (1U << 0) @@ -124,11 +120,6 @@ // 32/sizeof(svBitVecVal) words on the stack. svBitVecVal buf[256 / 8 / sizeof(svBitVecVal)]; - // The implementation of otbn_rf_peek() is only available if DesignScope != "" - // (the function is implemented in SystemVerilog, and imported through DPI). - // We should not reach the code here if that's the case. - assert(otbn_rf_peek); - for (int i = 0; i < 32; ++i) { if (!otbn_rf_peek(i, buf)) { std::ostringstream oss; @@ -154,12 +145,6 @@ // 32/sizeof(svBitVecVal) words on the stack. svBitVecVal buf[256 / 8 / sizeof(svBitVecVal)]; - // The implementation of otbn_stack_element_peek() is only available if - // DesignScope != "" (the function is implemented in SystemVerilog, and - // imported through DPI). We should not reach the code here if that's the - // case. - assert(otbn_stack_element_peek); - int i = 0; while (1) { @@ -197,7 +182,9 @@ const std::string &design_scope, bool enable_secure_wipe) : mem_util_(mem_scope), design_scope_(design_scope), - enable_secure_wipe_(enable_secure_wipe) {} + enable_secure_wipe_(enable_secure_wipe) { + assert(mem_scope.size() && design_scope.size()); +} OtbnModel::~OtbnModel() {} @@ -592,8 +579,6 @@ } bool OtbnModel::check_regs(ISSWrapper &iss) const { - assert(design_scope_.size()); - std::string base_scope = design_scope_ + ".u_otbn_rf_base.gen_rf_base_ff.u_otbn_rf_base_inner.u_snooper"; @@ -653,8 +638,6 @@ } bool OtbnModel::check_call_stack(ISSWrapper &iss) const { - assert(design_scope_.size()); - std::string call_stack_snooper_scope = design_scope_ + ".u_otbn_rf_base.u_call_stack_snooper";