From 5aa9a9562dc4f411c0191adf7bd805d4baf57306 Mon Sep 17 00:00:00 2001 From: Cindy Chen Date: Mon, 8 Feb 2021 17:22:14 -0800 Subject: [PATCH] [dv/enable_regs] Support enable registers have more than one field This PR implements the DV support for PR #5128, which allows an enable register to be a multi-reg. This enable register will have many fields, each field locks a certain set of registers. This PR support it by moving the `locked_reg_q` to dv_base_reg_field, and copied enable register related methods to dv_base_reg_field. If the enable register has only one field, normal methods still work (which means we do not need to change current testbench). If the enable register has more than one field, CSR automation test will work, but if user want to access the `locked_reg_q`, they have to call from dv_base_reg_field rather than dv_base_reg. This PR also updated the UVM RAL register generate script. Signed-off-by: Cindy Chen --- hw/dv/sv/dv_base_reg/dv_base_reg.sv | 83 +++++++++++++---------- hw/dv/sv/dv_base_reg/dv_base_reg_block.sv | 18 ----- hw/dv/sv/dv_base_reg/dv_base_reg_field.sv | 60 ++++++++++++++-- hw/dv/sv/dv_base_reg/dv_base_reg_pkg.sv | 22 +++++- hw/ip/keymgr/dv/env/keymgr_scoreboard.sv | 4 +- util/reggen/uvm_reg.sv.tpl | 14 +++- 6 files changed, 137 insertions(+), 64 deletions(-) diff --git a/hw/dv/sv/dv_base_reg/dv_base_reg.sv b/hw/dv/sv/dv_base_reg/dv_base_reg.sv index 8efdaace8cd8e..9f18a4e8624fd 100644 --- a/hw/dv/sv/dv_base_reg/dv_base_reg.sv +++ b/hw/dv/sv/dv_base_reg/dv_base_reg.sv @@ -8,7 +8,6 @@ class dv_base_reg extends uvm_reg; // hence, backdoor write isn't available local bit is_ext_reg; - local dv_base_reg locked_regs[$]; local uvm_reg_data_t staged_shadow_val, committed_val, shadowed_val; local bit is_shadowed; local bit shadow_wr_staged; // stage the first shadow reg write @@ -53,36 +52,44 @@ class dv_base_reg extends uvm_reg; end endfunction - // if the register is an enable reg, it will add controlled registers in the queue - function void add_locked_reg(dv_base_reg locked_reg); - locked_regs.push_back(locked_reg); + // Wen reg/fld can lock specific groups of fields' write acces. The lockable fields are called + // lockable flds. + function void add_lockable_reg_or_fld(uvm_object lockable_obj); + dv_base_reg_field wen_fld; + `DV_CHECK_FATAL(m_fields.size(), 1, "This register has more than one field.\ + Please use register field's add_lockable_reg_or_fld() method instead.") + `downcast(wen_fld, m_fields[0]) + wen_fld.add_lockable_reg_or_fld(lockable_obj); endfunction - function bit is_inside_locked_regs(dv_base_reg csr); - if (csr inside {locked_regs}) return 1; - else return 0; + // Returns true if this register/field can lock the specified register/field, else return false. + function bit locks_reg_or_fld(uvm_object obj); + dv_base_reg_field wen_fld; + `DV_CHECK_FATAL(m_fields.size(), 1, "This register has more than one field.\ + Please use register field's locks_reg_or_fld() method instead.") + `downcast(wen_fld, m_fields[0]) + return wen_fld.locks_reg_or_fld(obj); endfunction - function bit is_enable_reg(); - return (locked_regs.size() > 0); + function void get_lockable_flds(ref dv_base_reg_field lockable_flds_q[$]); + dv_base_reg_field wen_fld; + `DV_CHECK_FATAL(m_fields.size(), 1, "This register has more than one field.\ + Please use register field's get_lockable_flds() method instead.") + `downcast(wen_fld, m_fields[0]) + wen_fld.get_lockable_flds(lockable_flds_q); endfunction - function bit is_staged(); - return shadow_wr_staged; - endfunction - - // if enable register is set to 1, the locked registers will be set to RO access - // once enable register is reset to 0, the locked registers will be set back to original access - function void set_locked_regs_access(string access = "original_access"); - foreach (locked_regs[i]) begin - dv_base_reg_field locked_fields[$]; - locked_regs[i].get_dv_base_reg_fields(locked_fields); - foreach (locked_fields[i]) locked_fields[i].set_locked_fields_access(access); + function bit is_wen_reg(); + foreach (m_fields[i]) begin + dv_base_reg_field fld; + `downcast(fld, m_fields[i]) + if (fld.is_wen_fld()) return 1; end + return 0; endfunction - function void get_locked_regs(ref dv_base_reg locked_regs_q[$]); - locked_regs_q = locked_regs; + function bit is_staged(); + return shadow_wr_staged; endfunction // is_shadowed bit is only one-time programmable @@ -128,8 +135,8 @@ class dv_base_reg extends uvm_reg; endfunction // post_write callback to handle special regs: - // - shadow register, enable reg won't be updated until the second write has no error - // - enable register, if enable_reg is disabled, change access policy to all the locked_regs + // - shadow register: shadow reg won't be updated until the second write has no error + // - lock register: if wen_fld is set to 0, change access policy to all the lockable_flds // TODO: create an `enable_field_access_policy` variable and set the template code during // automation. virtual task post_write(uvm_reg_item rw); @@ -156,17 +163,23 @@ class dv_base_reg extends uvm_reg; shadowed_val = ~committed_val; end end - if (is_enable_reg()) begin - get_dv_base_reg_fields(fields); - field_access = fields[0].get_access(); - case (field_access) - // rw.value is a dynamic array - // discussed in issue #1922: enable register is standarized to W0C or RO (if HW has write - // access). - "W0C": if (rw.value[0][0] == 1'b0) set_locked_regs_access("RO"); - "RO": ; // if RO, it's updated by design, need to predict in scb - default:`uvm_fatal(`gfn, $sformatf("enable register invalid access %s", field_access)) - endcase + if (is_wen_reg()) begin + foreach (m_fields[i]) begin + dv_base_reg_field fld; + `downcast(fld, m_fields[i]) + if (fld.is_wen_fld()) begin + // rw.value is a dynamic array + uvm_reg_data_t field_val = rw.value[0] & fld.get_field_mask(); + field_access = fld.get_access(); + case (field_access) + // discussed in issue #1922: enable register is standarized to W0C or RO (if HW has + // write access). + "W0C": if (field_val == 1'b0) fld.set_lockable_flds_access(1); + "RO": ; // if RO, it's updated by design, need to predict in scb + default:`uvm_fatal(`gfn, $sformatf("lock register invalid access %s", field_access)) + endcase + end + end end endtask diff --git a/hw/dv/sv/dv_base_reg/dv_base_reg_block.sv b/hw/dv/sv/dv_base_reg/dv_base_reg_block.sv index 34dbf8617e618..a71aefb963f73 100644 --- a/hw/dv/sv/dv_base_reg/dv_base_reg_block.sv +++ b/hw/dv/sv/dv_base_reg/dv_base_reg_block.sv @@ -48,14 +48,6 @@ class dv_base_reg_block extends uvm_reg_block; end endfunction - function void get_enable_regs(ref dv_base_reg enable_regs[$]); - dv_base_reg all_regs[$]; - this.get_dv_base_regs(all_regs); - foreach (all_regs[i]) begin - if (all_regs[i].is_enable_reg()) enable_regs.push_back(all_regs[i]); - end - endfunction - function void get_shadowed_regs(ref dv_base_reg shadowed_regs[$]); dv_base_reg all_regs[$]; this.get_dv_base_regs(all_regs); @@ -64,16 +56,6 @@ class dv_base_reg_block extends uvm_reg_block; end endfunction - // override RAL's reset function to support enable registers - // when reset issued - the locked registers' access will be reset to original access - virtual function void reset(string kind = "HARD"); - dv_base_reg enable_regs[$]; - `uvm_info(`gfn, "Resetting RAL reg block", UVM_MEDIUM) - super.reset(kind); - get_enable_regs(enable_regs); - foreach (enable_regs[i]) enable_regs[i].set_locked_regs_access(); - endfunction - // Internal function, used to compute the address mask for this register block. // // This is quite an expensive computation, so we memoize the results in addr_mask[map]. diff --git a/hw/dv/sv/dv_base_reg/dv_base_reg_field.sv b/hw/dv/sv/dv_base_reg/dv_base_reg_field.sv index 088246704eafd..60a27bd1c65df 100644 --- a/hw/dv/sv/dv_base_reg/dv_base_reg_field.sv +++ b/hw/dv/sv/dv_base_reg/dv_base_reg_field.sv @@ -5,6 +5,7 @@ // base register reg class which will be used to generate the reg field class dv_base_reg_field extends uvm_reg_field; local string m_original_access; + local dv_base_reg_field lockable_flds[$]; `uvm_object_utils(dv_base_reg_field) `uvm_object_new @@ -32,6 +33,11 @@ class dv_base_reg_field extends uvm_reg_field; value.rand_mode(is_rand); endfunction + virtual function dv_base_reg get_dv_base_reg_parent(); + uvm_reg csr = get_parent(); + `downcast(get_dv_base_reg_parent, csr) + endfunction + // when use UVM_PREDICT_WRITE and the CSR access is WO, this function will return the default // val of the register, rather than the written value virtual function uvm_reg_data_t XpredictX(uvm_reg_data_t cur_val, @@ -46,6 +52,11 @@ class dv_base_reg_field extends uvm_reg_field; return m_original_access; endfunction + virtual function uvm_reg_data_t get_field_mask(); + get_field_mask = (1'b1 << this.get_n_bits()) - 1; + get_field_mask = get_field_mask << this.get_lsb_pos(); + endfunction + virtual function void set_original_access(string access); if (m_original_access == "") begin m_original_access = access; @@ -54,12 +65,49 @@ class dv_base_reg_field extends uvm_reg_field; end endfunction - virtual function void set_locked_fields_access(string access = "original_access"); - case (access) - "RO": void'(this.set_access(access)); - "original_access": void'(this.set_access(m_original_access)); - default: `uvm_fatal(`gfn, $sformatf("attempt to set access to %s", access)) - endcase + // Only used for lock regsiter and reset. + local function void set_fld_access(bit lock); + if (lock) void'(this.set_access("RO")); + else void'(this.set_access(m_original_access)); + endfunction + + // If input is a reg, add all fields under the reg; if input is a field, add the specific field. + function void add_lockable_reg_or_fld(uvm_object lockable_obj); + dv_base_reg_field flds[$]; + uvm_reg_block ral = this.get_parent().get_parent(); + `DV_CHECK_EQ_FATAL(ral.is_locked(), 0, "RAL is locked, cannot add lockable reg or fld!") + get_flds_from_uvm_object(lockable_obj, `gfn, flds); + foreach (flds[i]) lockable_flds.push_back(flds[i]); + endfunction + + // Returns true if this field can lock the specified register/field, else return false. + function bit locks_reg_or_fld(uvm_object obj); + dv_base_reg_field flds[$]; + get_flds_from_uvm_object(obj, `gfn, flds); + + foreach(flds[i]) if (!(flds[i] inside {lockable_flds})) return 0; + return 1; + endfunction + + function bit is_wen_fld(); + return (lockable_flds.size() > 0); + endfunction + + // If lock is set to 1, lockable fields access policy will be set to RO access. + // If lock resets to 0, lockable fields will be set back to their original accesses. + function void set_lockable_flds_access(bit lock); + foreach (lockable_flds[i]) lockable_flds[i].set_fld_access(lock); + endfunction + + function void get_lockable_flds(ref dv_base_reg_field lockable_flds_q[$]); + lockable_flds_q = lockable_flds; + endfunction + + // override RAL's reset function to support enable registers + // when reset issued - the lockable field's access will be reset to original access + virtual function void reset(string kind = "HARD"); + super.reset(kind); + set_fld_access(0); endfunction endclass diff --git a/hw/dv/sv/dv_base_reg/dv_base_reg_pkg.sv b/hw/dv/sv/dv_base_reg/dv_base_reg_pkg.sv index b411f8297566a..9353b19dd6894 100644 --- a/hw/dv/sv/dv_base_reg/dv_base_reg_pkg.sv +++ b/hw/dv/sv/dv_base_reg/dv_base_reg_pkg.sv @@ -46,8 +46,26 @@ package dv_base_reg_pkg; BkdrRegPathGlsShdow // backdoor path for shadow reg's shadow val in GLS } bkdr_reg_path_e; - // package sources - // base ral + + typedef class dv_base_reg; + typedef class dv_base_reg_field; + + function automatic void get_flds_from_uvm_object(input uvm_object obj, + input string msg = "dv_base_reg_pkg", + ref dv_base_reg_field flds[$]); + dv_base_reg csr; + dv_base_reg_field fld; + + if ($cast(csr, obj)) begin + csr.get_dv_base_reg_fields(flds); + end else if ($cast(fld, obj)) begin + flds.push_back(fld); + end else begin + `uvm_fatal(msg, $sformatf("obj %0s is not of type uvm_reg or uvm_reg_field", + obj.get_full_name())) + end + endfunction + `include "csr_excl_item.sv" `include "dv_base_reg_field.sv" `include "dv_base_reg.sv" diff --git a/hw/ip/keymgr/dv/env/keymgr_scoreboard.sv b/hw/ip/keymgr/dv/env/keymgr_scoreboard.sv index 9037e8d9792b9..98a07bdd09da1 100644 --- a/hw/ip/keymgr/dv/env/keymgr_scoreboard.sv +++ b/hw/ip/keymgr/dv/env/keymgr_scoreboard.sv @@ -247,7 +247,7 @@ class keymgr_scoreboard extends cip_base_scoreboard #( update_state(get_next_state(current_state)); // set sw_binding_regwen after advance OP void'(ral.sw_binding_regwen.predict(.value(1))); - ral.sw_binding_regwen.set_locked_regs_access("original_access"); + ral.sw_binding_regwen.en.set_locked_regs_access("original_access"); end end keymgr_pkg::OpDisable: begin @@ -336,7 +336,7 @@ class keymgr_scoreboard extends cip_base_scoreboard #( // in StReset, can't change sw_binding_regwen value // set related locked reg back to original_access as this is updated automatic in post_write #0; // push below update to be done after post_write - ral.sw_binding_regwen.set_locked_regs_access("original_access"); + ral.sw_binding_regwen.en.set_locked_regs_access("original_access"); end // process the csr req diff --git a/util/reggen/uvm_reg.sv.tpl b/util/reggen/uvm_reg.sv.tpl index e51e692ec04ef..a4d932ba730f6 100644 --- a/util/reggen/uvm_reg.sv.tpl +++ b/util/reggen/uvm_reg.sv.tpl @@ -275,7 +275,19 @@ package ${block.name}_ral_pkg; // assign locked reg to its regwen reg % for r in regs_flat: % if r.regwen: - ${r.regwen.lower()}.add_locked_reg(${r.name.lower()}); + % for reg in regs_flat: + % if r.regwen.lower() == reg.name.lower(): + ${r.regwen.lower()}.add_lockable_reg_or_fld(${r.name.lower()}); +<% break %>\ + % elif reg.name.lower() in r.regwen.lower(): + % for field in reg.get_field_list(): + % if r.regwen.lower() == (reg.name.lower() + "_" + field.name.lower()): + ${r.regwen.lower()}.${field.name.lower()}.add_lockable_reg_or_fld(${r.name.lower()}); +<% break %>\ + % endif + % endfor + % endif + % endfor % endif % endfor