From be9a2d13ddcc8b8d69675308ce16f0cd8ee1f65a Mon Sep 17 00:00:00 2001 From: Rupert Swarbrick Date: Fri, 24 Apr 2026 18:07:19 +0100 Subject: [PATCH] [aes,dv] Allow aes_reseed_vseq resets when passing new sideload key To do so, we have to pull out the code that passes the new sideload key and add checks for if reset is asserted, exiting the task early when that happens. There are some loops that wait for an event by doing backdoor HDL reads on negative edges of the clock. To do this tidily, I've added a wait_n_clks_or_rst task to clk_rst_if (to match the wait_clks_or_rst task that already existed). One part of the changes in this commit is to add an "early exit on reset" to aes_base_vseq::set_sideload. Since there are a couple of equivalent tasks next to it, I've factored out the common code and taught them about resets too. Honestly, these tasks seem a bit strange to me: most of them have their only call site in aes_base_vseq::setup_dut. This function performs a chain of double-writes to the same register, one for each field. This is a bit inefficient! What's more, these sequence tasks do register prediction (presumably, there was no scoreboard when it was originally written). In both cases, the code is a bit awkward, but further changes to that part of the sequence are probably out of scope for this commit. Signed-off-by: Rupert Swarbrick --- hw/dv/sv/common_ifs/clk_rst_if.sv | 11 ++ hw/ip/aes/dv/env/seq_lib/aes_base_vseq.sv | 39 +++-- hw/ip/aes/dv/env/seq_lib/aes_reseed_vseq.sv | 166 +++++++++++++------- 3 files changed, 140 insertions(+), 76 deletions(-) diff --git a/hw/dv/sv/common_ifs/clk_rst_if.sv b/hw/dv/sv/common_ifs/clk_rst_if.sv index abd64efbdb259..27a89bebe5407 100644 --- a/hw/dv/sv/common_ifs/clk_rst_if.sv +++ b/hw/dv/sv/common_ifs/clk_rst_if.sv @@ -128,6 +128,17 @@ interface clk_rst_if ( end join endtask + // Wait for 'num_clks' clocks based on the negative clock edge or reset, whichever comes first. + task automatic wait_n_clks_or_rst(int num_clks); + fork begin : isolation_fork + fork + wait_n_clks(num_clks); + wait_for_reset(.wait_negedge(1'b1), .wait_posedge(1'b0)); + join_any + disable fork; + end join + endtask + // wait for rst_n to assert and then deassert task automatic wait_for_reset(bit wait_negedge = 1'b1, bit wait_posedge = 1'b1); if (wait_negedge && ($isunknown(rst_n) || rst_n === 1'b1)) @(negedge rst_n); diff --git a/hw/ip/aes/dv/env/seq_lib/aes_base_vseq.sv b/hw/ip/aes/dv/env/seq_lib/aes_base_vseq.sv index 310dd435dbbc2..95cb0922f40be 100644 --- a/hw/ip/aes/dv/env/seq_lib/aes_base_vseq.sv +++ b/hw/ip/aes/dv/env/seq_lib/aes_base_vseq.sv @@ -168,31 +168,36 @@ class aes_base_vseq extends cip_base_vseq #( end endtask // set_key_len + // Set and update a shadowed register field + // + // This uses csr_update to do the double-write that's necessary for shadowed fields. + protected task set_shadowed_field(uvm_reg_field field, uvm_reg_data_t value); + // If the field already has the value, there's nothing to do + if (field.get_mirrored_value() == value) return; - virtual task set_sideload(bit sideload); - if (ral.ctrl_shadowed.sideload.get_mirrored_value() != sideload) begin - ral.ctrl_shadowed.sideload.set(sideload); - csr_update(.csr(ral.ctrl_shadowed), .en_shadow_wr(1'b1), .blocking(1)); - void'(ral.ctrl_shadowed.sideload.predict(sideload)); + field.set(value); + csr_update(.csr(field.get_parent()), .en_shadow_wr(1'b1), .blocking(1)); + if (cfg.under_reset) return; + + if (!field.predict(value, UVM_PREDICT_WRITE)) begin + `uvm_fatal(get_full_name(), + $sformatf("Failed to predict %0s.%0s after an observed write.", + field.get_parent().get_name(), field.get_name())) end endtask + task set_sideload(bit sideload); + set_shadowed_field(ral.ctrl_shadowed.sideload, sideload); + endtask + - virtual task set_prng_reseed_rate(prs_rate_e reseed_rate); - if (ral.ctrl_shadowed.prng_reseed_rate.get_mirrored_value() != reseed_rate) begin - ral.ctrl_shadowed.prng_reseed_rate.set(reseed_rate); - csr_update(.csr(ral.ctrl_shadowed), .en_shadow_wr(1'b1), .blocking(1)); - void'(ral.ctrl_shadowed.prng_reseed_rate.predict(reseed_rate)); - end + task set_prng_reseed_rate(prs_rate_e reseed_rate); + set_shadowed_field(ral.ctrl_shadowed.prng_reseed_rate, reseed_rate); endtask - virtual task set_manual_operation(bit manual_operation); - if (ral.ctrl_shadowed.manual_operation.get_mirrored_value() != manual_operation) begin - ral.ctrl_shadowed.manual_operation.set(manual_operation); - csr_update(.csr(ral.ctrl_shadowed), .en_shadow_wr(1'b1), .blocking(1)); - void'(ral.ctrl_shadowed.manual_operation.predict(manual_operation)); - end + task set_manual_operation(bit manual_operation); + set_shadowed_field(ral.ctrl_shadowed.manual_operation, manual_operation); endtask diff --git a/hw/ip/aes/dv/env/seq_lib/aes_reseed_vseq.sv b/hw/ip/aes/dv/env/seq_lib/aes_reseed_vseq.sv index 825dc46d54021..9ad2ef67f4702 100644 --- a/hw/ip/aes/dv/env/seq_lib/aes_reseed_vseq.sv +++ b/hw/ip/aes/dv/env/seq_lib/aes_reseed_vseq.sv @@ -92,9 +92,9 @@ class aes_reseed_vseq extends aes_base_vseq; if (cfg.under_reset) return; if (cfg.aes_reseed_vif.entropy_clearing_req) - `uvm_error(get_name(), "entropy_clearing_req should not have been set.") + `uvm_error(`gfn, "entropy_clearing_req should not have been set.") if (cfg.aes_reseed_vif.entropy_masking_req) - `uvm_error(get_name(), "entropy_masking_req should not have been set.") + `uvm_error(`gfn, "entropy_masking_req should not have been set.") endtask // Trigger reseed by writing a new key to the initial key registers. In case @@ -103,7 +103,7 @@ class aes_reseed_vseq extends aes_base_vseq; local task write_key_regs(); bit [7:0][31:0] init_key[2]; - if (!std::randomize(init_key)) `uvm_fatal(get_name(), "Failed to randomize init_key") + if (!std::randomize(init_key)) `uvm_fatal(`gfn, "Failed to randomize init_key") // Wait for the DUT to be idle before writing the key. csr_spinwait(.ptr(ral.status.idle), .exp_data(1'b1)); @@ -120,6 +120,104 @@ class aes_reseed_vseq extends aes_base_vseq; end endtask + // Do a backdoor read to get the value of the keymgr_key_i.valid port, treating 'x and 'z as + // invalid. + local function bit snoop_sideload_valid(); + string sideload_valid_path = "tb.dut.keymgr_key_i.valid"; + logic valid; + + if (!uvm_hdl_check_path(sideload_valid_path)) begin + `uvm_fatal(`gfn, $sformatf("\n\t ----| PATH '%0s' NOT FOUND", sideload_valid_path)) + end + + if (!uvm_hdl_read(sideload_valid_path, valid)) begin + `uvm_fatal(`gfn, $sformatf("Failed to backdoor read from %0s", sideload_valid_path)) + end + + return valid === 1; + endfunction + + // Trigger reseed by loading a new key via sideload interface. In case KEY_TOUCH_FORCES_RESEED is + // not set, no reseed operation is supposed to be triggered. Wait for the DUT to be idle before + // enabling sideload. + local task pass_new_sideload_key(); + bit sideload_setup_done; + + while (!sideload_setup_done) begin + bit sideload_valid; + bit sideload_enabled; + + csr_spinwait(.ptr(ral.status.idle), .exp_data(1'b1)); + if (cfg.under_reset) return; + + // Make sure sideload is disabled. + set_sideload(1'b0); + if (cfg.under_reset) return; + + // Wait for sideload key to be valid before enabling sideload, timing out after + // wait_timeout_cycles. + fork : isolation_fork begin + fork + begin + cfg.clk_rst_vif.wait_n_clks(wait_timeout_cycles); + `uvm_error(`gfn, "Timeout waiting for valid sideload key") + end + begin + while (!sideload_valid && !cfg.under_reset) begin + cfg.clk_rst_vif.wait_n_clks_or_rst(1); + sideload_valid = snoop_sideload_valid(); + end + end + join_any + disable fork; + end join + if (cfg.under_reset) return; + + // Enable sideload by writing to CTRL_SHADOWED, doing repeated backdoor reads of the sideload + // valid flag and clearing sideload_valid if it becomes false. + // + // If reset is asserted, set_sideload will exit and the first process will set + // sideload_enabled, causing the second process to complete. + fork + // Enable sideload. + begin + set_sideload(1'b1); + sideload_enabled = 1'b1; + end + // Detect if the sideload valid bit gets de-asserted while trying to enable sideload. + // + // This detection is a backdoor HDL access, which we perform on negative edges of the clock + // to avoid racing with the posedge state updates. + // + // If reset is asserted, this process will finish immediately, because set_sideload will + // exit in the other process, causing that process to set sideload_enabled. + begin + while (sideload_valid && !sideload_enabled) begin + cfg.clk_rst_vif.wait_n_clks_or_rst(1); + sideload_valid = snoop_sideload_valid(); + end + end + join + if (cfg.under_reset) return; + + // If the sideload valid bit got de-asserted again before fully enabling sideload, the key + // did not get loaded and we have to repeat the setup procedure. Otherwise, sideload was + // enabled successfully. + if (sideload_valid) begin + sideload_setup_done = 1; + end + end + if (cfg.under_reset) return; + + // Sideload got enabled with a valid sideload key present. This must trigger a reseed in + // case KEY_TOUCH_FORCES_RESEED is set. + if (cfg.do_reseed) begin + check_prng_reseed(); + end else begin + check_no_prng_reseed(); + end + endtask + task check_reseed_rate(); bit [BlockCtrWidth-1:0] block_ctr; bit [BlockCtrWidth-1:0] block_ctr_set_val = BlockCtrWidth'(3); @@ -208,11 +306,6 @@ class aes_reseed_vseq extends aes_base_vseq; // // This task is run in an isolation fork. task body_main_thread(); - bit sideload_setup_done; - bit sideload_valid; - string sideload_valid_path = "tb.dut.keymgr_key_i.valid"; - bit sideload_enabled; - // Trigger reseed by manually setting the PRNG_RESEED bit in the TRIGGER register. `uvm_info(`gfn, "Triggering PRNG reseed via trigger register", UVM_LOW) fork @@ -221,7 +314,7 @@ class aes_reseed_vseq extends aes_base_vseq; join if (cfg.under_reset) return; - `uvm_info(get_name(), + `uvm_info(`gfn, $sformatf("Writing a new key, which should%0s trigger a PRNG reseed.", cfg.do_reseed ? "" : " not"), UVM_LOW) @@ -231,56 +324,11 @@ class aes_reseed_vseq extends aes_base_vseq; // Trigger reseed by loading a new key via sideload interface. In case // KEY_TOUCH_FORCES_RESEED is not set, no reseed operation is supposed to be triggered. // Wait for the DUT to be idle before enabling sideload. - `uvm_info(`gfn, "Potentially triggering PRNG reseed by providing a new sideload key", - UVM_LOW) - if (!uvm_hdl_check_path(sideload_valid_path)) begin - `uvm_fatal(`gfn, $sformatf("\n\t ----| PATH NOT FOUND")) - end - sideload_setup_done = 0; - while (!sideload_setup_done) begin - csr_spinwait(.ptr(ral.status.idle), .exp_data(1'b1)); - // Make sure sideload is disabled. - set_sideload(1'b0); - sideload_enabled = 1'b0; - // Wait for sideload key to be valid before enabling sideload. - sideload_valid = 0; - `DV_SPINWAIT_EXIT( - while (!sideload_valid) begin - cfg.clk_rst_vif.wait_n_clks(1); - `DV_CHECK_FATAL(uvm_hdl_read(sideload_valid_path, sideload_valid)) - end, - cfg.clk_rst_vif.wait_n_clks(wait_timeout_cycles);, - "Timeout waiting for valid sideload key") - fork - // Enable sideload. - begin - set_sideload(1'b1); - sideload_enabled = 1'b1; - end - // Detect if the sideload valid bit gets de-asserted while trying to enable sideload. - begin - while (sideload_valid && !sideload_enabled) begin - cfg.clk_rst_vif.wait_n_clks(1); - `DV_CHECK_FATAL(uvm_hdl_read(sideload_valid_path, sideload_valid)) - end - end - join - - // If the sideload valid bit got de-asserted again before fully enabling sideload, the key - // did not get loaded and we have to repeat the setup procedure. Otherwise, sideload was - // enabled successfully. - if (sideload_valid) begin - sideload_setup_done = 1; - end - end - - // Sideload got enabled with a valid sideload key present. This must trigger a reseed in - // case KEY_TOUCH_FORCES_RESEED is set. - if (cfg.do_reseed) begin - check_prng_reseed(); - end else begin - check_no_prng_reseed(); - end + `uvm_info(`gfn, + "Potentially triggering PRNG reseed by providing a new sideload key", + UVM_LOW) + pass_new_sideload_key(); + if (cfg.under_reset) return; // Test that the PRNGs are reseeded at the proper rate during message processing. `uvm_info(`gfn, "Testing automatic / block counter based reseeding of the masking PRNG",