Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[hmac,rtl/dv] DV synchronization and error handling fixes #23383

Merged
merged 1 commit into from
Jun 2, 2024
Merged

[hmac,rtl/dv] DV synchronization and error handling fixes #23383

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 14 additions & 11 deletions hw/ip/hmac/dv/env/hmac_env_pkg.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,26 @@ package hmac_env_pkg;

// local parameters and types
parameter uint32 HMAC_MSG_FIFO_DEPTH_WR = 32;
parameter uint32 HMAC_MSG_FIFO_DEPTH_RD = 16;
parameter uint32 HMAC_MSG_FIFO_DEPTH_256 = 16;
parameter uint32 HMAC_MSG_FIFO_DEPTH_512 = 32;
// SHA-2 256 block size = 16 32-bit words, so FIFO reading depth is up to 16 words
parameter uint32 HMAC_MSG_FIFO_DEPTH_RD_256 = 16;
// SHA-2 385/512 block size = 16 64-bit words, so FIFO reading depth is up to 32 words
parameter uint32 HMAC_MSG_FIFO_DEPTH_RD_512 = 32;
parameter uint32 HMAC_MSG_FIFO_DEPTH_BYTES = HMAC_MSG_FIFO_DEPTH_WR * 4;
parameter uint32 HMAC_MSG_FIFO_SIZE = 2048;
parameter uint32 HMAC_MSG_FIFO_BASE = 32'h1000;
parameter uint32 HMAC_MSG_FIFO_LAST_ADDR = HMAC_MSG_FIFO_BASE + HMAC_MSG_FIFO_SIZE - 1;
parameter uint32 HMAC_BLK_SIZE_SHA2_256 = 512/8; // Nb bytes
parameter uint32 HMAC_BLK_SIZE_SHA2_384_512 = 1024/8; // Nb bytes

// 48 cycles of hashing, 16 cycles to rd next 16 words, 1 cycle to update digest
parameter uint32 HMAC_MSG_PROCESS_CYCLES = 65;
// 80 cycles for hmac key padding
parameter uint32 HMAC_KEY_PROCESS_CYCLES = 80;
parameter uint32 HMAC_MSG_PROCESS_CYCLES_256 = 65;
parameter uint32 HMAC_MSG_PROCESS_CYCLES_512 = 81;
// 80 (64+16) cycles for hmac key padding for SHA-2 256
parameter uint32 HMAC_KEY_PROCESS_CYCLES_256 = 80;
// 112 (80+32) cycles for hmac key padding for SHA-2 384/512
parameter uint32 HMAC_KEY_PROCESS_CYCLES_512 = 112;
// 1 cycles to write a msg word to hmac_msg_fifo
parameter uint32 HMAC_WR_WORD_CYCLE = 1;
parameter uint32 HMAC_WR_WORD_CYCLE = 1;

parameter uint NUM_DIGESTS = 16;
parameter uint NUM_KEYS = 32;
Expand All @@ -54,8 +59,7 @@ package hmac_env_pkg;

// HMAC status register indices
typedef enum int {
// TODO verify HmacIdle
HmacIdle = 0,
HmacStaIdle = 0,
HmacStaMsgFifoEmpty = 1,
HmacStaMsgFifoFull = 2,
HmacStaMsgFifoDepthLsb = 4,
Expand All @@ -68,8 +72,7 @@ package hmac_env_pkg;
ShaEn = 1,
EndianSwap = 2,
DigestSwap = 3,
// TODO (issue #23245)
KeySwap = 4,
KeySwap = 4,
DigestSizeLsb = 5,
DigestSizeMsb = 8,
KeyLengthLsb = 9,
Expand Down
67 changes: 45 additions & 22 deletions hw/ip/hmac/dv/env/hmac_scoreboard.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@ class hmac_scoreboard extends cip_base_scoreboard #(.CFG_T (hmac_env_cfg),
`uvm_component_utils(hmac_scoreboard)
`uvm_component_new

bit sha_en, fifo_empty;
bit sha_en, fifo_empty, hmac_idle;
bit [7:0] msg_q[$];
bit hmac_start, hmac_process;
int hmac_wr_cnt, hmac_rd_cnt;
int fifo_rd_depth;
int block_process_cycles;
int key_process_cycles;
bit [TL_DW-1:0] key[NUM_KEYS];
bit [TL_DW-1:0] exp_digest[NUM_DIGESTS];
bit [3:0] digest_size, previous_digest_size, expected_digest_size;
Expand All @@ -35,8 +38,7 @@ class hmac_scoreboard extends cip_base_scoreboard #(.CFG_T (hmac_env_cfg),

virtual task process_tl_access(tl_seq_item item, tl_channels_e channel, string ral_name);
uvm_reg csr;
// TODO: enable back do_read_check once synchronization works for all digest sizes
bit do_read_check = 1'b0;
bit do_read_check = 1'b1;
bit do_cycle_accurate_check = 1'b1;
bit write = item.is_write();
string csr_name;
Expand All @@ -60,7 +62,7 @@ class hmac_scoreboard extends cip_base_scoreboard #(.CFG_T (hmac_env_cfg),
// push the msg into msg_fifo
if ((item.a_addr & addr_mask) inside {[HMAC_MSG_FIFO_BASE : HMAC_MSG_FIFO_LAST_ADDR]}) begin
if (!sha_en) begin
update_err_intr_code(SwHashStartWhenShaDisabled);
update_err_intr_code(SwPushMsgWhenDisallowed);
end else if (!hmac_start) begin
update_err_intr_code(SwPushMsgWhenDisallowed);
end else if (hmac_start && !cfg.under_reset) begin
Expand All @@ -84,6 +86,15 @@ class hmac_scoreboard extends cip_base_scoreboard #(.CFG_T (hmac_env_cfg),
bit do_predict = 1'b1;
case (csr_name)
"cmd": begin
// check that HMAC is configured correctly before starting
invalid_cfg = (ral.cfg.digest_size.get_mirrored_value() == SHA2_None) |
((ral.cfg.key_length.get_mirrored_value() == Key_None) &
ral.cfg.hmac_en.get_mirrored_value()) |
((ral.cfg.digest_size.get_mirrored_value() == SHA2_256) &
(ral.cfg.key_length.get_mirrored_value() == Key_1024) &
ral.cfg.hmac_en.get_mirrored_value());
`uvm_info(`gfn, $sformatf("invalid config at starting: %1b", invalid_cfg), UVM_LOW)

if (sha_en && !(hmac_start && item.a_data[HashStart])) begin
if (item.a_data[HashProcess] && hmac_start) begin
{hmac_process, hmac_start} = item.a_data[1:0];
Expand All @@ -99,16 +110,6 @@ class hmac_scoreboard extends cip_base_scoreboard #(.CFG_T (hmac_env_cfg),
end
end else if (item.a_data[HashStart]) begin
{hmac_process, hmac_start} = item.a_data[1:0];
// check that HMAC is configured correctly before starting
invalid_cfg = (ral.cfg.digest_size.get_mirrored_value() == SHA2_None) |
((ral.cfg.key_length.get_mirrored_value() == Key_None) &
ral.cfg.hmac_en.get_mirrored_value()) |
((ral.cfg.digest_size.get_mirrored_value() == SHA2_256) &
(ral.cfg.key_length.get_mirrored_value() == Key_1024) &
ral.cfg.hmac_en.get_mirrored_value());

`uvm_info(`gfn, $sformatf("invalid config at starting: %1b", invalid_cfg), UVM_LOW)

if (invalid_cfg) begin
update_err_intr_code(SwInvalidConfig);
end else begin
Expand All @@ -118,12 +119,16 @@ class hmac_scoreboard extends cip_base_scoreboard #(.CFG_T (hmac_env_cfg),
// update digest size to the new one only at the start signal
previous_digest_size = ral.cfg.digest_size.get_mirrored_value();
`uvm_info(`gfn, $sformatf(
"setting previous digest: %4b", previous_digest_size), UVM_LOW)
"setting previous digest: %4b", previous_digest_size), UVM_HIGH)
end
end
end else if (item.a_data[HashStart]) begin
if (!sha_en) begin
if (!sha_en && !invalid_cfg) begin
// so long as configuration is valid
update_err_intr_code(SwHashStartWhenShaDisabled);
end else if (invalid_cfg) begin
// otherwise signalling invalid config takes priority
update_err_intr_code(SwInvalidConfig);
end else begin
update_err_intr_code(SwHashStartWhenActive);
end
Expand Down Expand Up @@ -228,7 +233,8 @@ class hmac_scoreboard extends cip_base_scoreboard #(.CFG_T (hmac_env_cfg),
bit [5:0] hmac_fifo_depth = hmac_wr_cnt - hmac_rd_cnt;
bit hmac_fifo_full = hmac_fifo_depth == HMAC_MSG_FIFO_DEPTH_WR;
bit hmac_fifo_empty = hmac_fifo_depth == 0;
bit [TL_DW-1:0] hmac_status_data = (hmac_fifo_empty << HmacStaMsgFifoEmpty) |
bit [TL_DW-1:0] hmac_status_data = (hmac_idle << HmacStaIdle) |
(hmac_fifo_empty << HmacStaMsgFifoEmpty) |
(hmac_fifo_full << HmacStaMsgFifoFull) |
(hmac_fifo_depth << HmacStaMsgFifoDepthLsb);
void'(ral.status.predict(.value(hmac_status_data), .kind(UVM_PREDICT_READ)));
Expand Down Expand Up @@ -330,6 +336,9 @@ class hmac_scoreboard extends cip_base_scoreboard #(.CFG_T (hmac_env_cfg),
"status": begin
if (!do_cycle_accurate_check) do_read_check = 0;
if (cfg.en_cov) cov.status_cg.sample(item.d_data, ral.cfg.get_mirrored_value());
// TODO (issue #23380): Verify idle status now exposed to SW
hmac_idle = item.d_data[HmacStaIdle];
void'(ral.status.hmac_idle.predict(.value(hmac_idle), .kind(UVM_PREDICT_READ)));
end
"msg_length_lower": begin
if (cfg.en_cov) begin
Expand Down Expand Up @@ -359,7 +368,7 @@ class hmac_scoreboard extends cip_base_scoreboard #(.CFG_T (hmac_env_cfg),
endcase
if (do_read_check) begin
`uvm_info(`gfn, $sformatf("%s reg is checked with expected value %0h",
csr_name, csr.get_mirrored_value()), UVM_LOW)
csr_name, csr.get_mirrored_value()), UVM_HIGH)
`DV_CHECK_EQ(csr.get_mirrored_value(), item.d_data, csr_name)
end
void'(csr.predict(.value(item.d_data), .kind(UVM_PREDICT_READ)));
Expand All @@ -375,6 +384,7 @@ class hmac_scoreboard extends cip_base_scoreboard #(.CFG_T (hmac_env_cfg),
key = '{default:0};
exp_digest = '{default:0};
fifo_empty = ral.status.fifo_empty.get_reset();
hmac_idle = ral.status.hmac_idle.get_reset();
hmac_start = ral.cmd.hash_start.get_reset();
sha_en = ral.cfg.sha_en.get_reset();
key_length = ral.cfg.key_length.get_reset();
Expand Down Expand Up @@ -465,8 +475,13 @@ class hmac_scoreboard extends cip_base_scoreboard #(.CFG_T (hmac_env_cfg),
begin : key_padding
wait(hmac_start && sha_en);
if (ral.cfg.hmac_en.get_mirrored_value() && hmac_rd_cnt == 0 && !invalid_cfg) begin
// 80 cycles for hmac key padding, 1 cycle for hash_start reg to reset
cfg.clk_rst_vif.wait_clks(HMAC_KEY_PROCESS_CYCLES + 1);
if (ral.cfg.digest_size.get_mirrored_value() == SHA2_256) begin
key_process_cycles = HMAC_KEY_PROCESS_CYCLES_256;
end else begin
key_process_cycles = HMAC_KEY_PROCESS_CYCLES_512;
end
// key_process_cycles for hmac key padding + 1 cycle for hash_start reg to reset
cfg.clk_rst_vif.wait_clks(key_process_cycles + 1);
@(negedge cfg.clk_rst_vif.clk);
key_processed = 1;
end
Expand Down Expand Up @@ -506,9 +521,17 @@ class hmac_scoreboard extends cip_base_scoreboard #(.CFG_T (hmac_env_cfg),
cfg.clk_rst_vif.wait_n_clks(1);
hmac_rd_cnt++;
`uvm_info(`gfn, $sformatf("increase rd cnt %0d", hmac_rd_cnt), UVM_HIGH)
if (hmac_rd_cnt % HMAC_MSG_FIFO_DEPTH_RD == 0) begin
// select correct FIFO read depth and message processing cycles
if (ral.cfg.digest_size.get_mirrored_value() == SHA2_256) begin
fifo_rd_depth = HMAC_MSG_FIFO_DEPTH_RD_256;
block_process_cycles = HMAC_MSG_PROCESS_CYCLES_256;
end else begin
fifo_rd_depth = HMAC_MSG_FIFO_DEPTH_RD_512;
block_process_cycles = HMAC_MSG_PROCESS_CYCLES_512;
end
if (hmac_rd_cnt % fifo_rd_depth == 0) begin
`uvm_info(`gfn, $sformatf("start waiting on message processing now"), UVM_HIGH)
cfg.clk_rst_vif.wait_n_clks(HMAC_MSG_PROCESS_CYCLES);
cfg.clk_rst_vif.wait_n_clks(block_process_cycles);
`uvm_info(`gfn, $sformatf("message processing has completed"), UVM_HIGH)
end
end
Expand Down
1 change: 1 addition & 0 deletions hw/ip/hmac/dv/env/seq_lib/hmac_base_vseq.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class hmac_base_vseq extends cip_base_vseq #(.CFG_T (hmac_env_cfg)
rand bit [5:0] key_length;
rand bit endian_swap;
rand bit digest_swap;
// TODO (issue #23245): verify key_swap
rand bit key_swap;

constraint wr_addr_c {
Expand Down
18 changes: 14 additions & 4 deletions hw/ip/hmac/dv/env/seq_lib/hmac_smoke_vseq.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ class hmac_smoke_vseq extends hmac_base_vseq;
rand bit re_enable_sha;
rand wipe_secret_req_e do_wipe_secret;

int key_process_cycles;

constraint num_trans_c {
num_trans inside {[1:50]};
}
Expand Down Expand Up @@ -156,11 +158,19 @@ class hmac_smoke_vseq extends hmac_base_vseq;
// If the last two fifo_rds are back-to-back, then design will have one cycle delay before
// the last fifo_rd in order to switch the state.
// If the last two fifo_rds are not back-to-back, then there won't be any delay for the
// last fifo_rd
// the wait_clk below is implemented to avoid checking intr_state during this corner case
// last fifo_rd.
// the wait_clk below is implemented to avoid checking intr_state during this period of time
// for such corner cases, because it is hard to align the scb with the fifo_empty interrupt.
// Since prim_packer can hold more data, the ignored period of time is extended by * 2.
// TODO revisit this and understand why this particular delay is selected
if (ral.cfg.digest_size.get_mirrored_value() == SHA2_256) begin
key_process_cycles = HMAC_KEY_PROCESS_CYCLES_256;
end else begin
key_process_cycles = HMAC_KEY_PROCESS_CYCLES_512;
end
cfg.clk_rst_vif.wait_clks((msg.size() % 4 || !legal_seq_c.constraint_mode()) ?
HMAC_KEY_PROCESS_CYCLES * 2 :
$urandom_range(0, HMAC_KEY_PROCESS_CYCLES * 2));
key_process_cycles * 2 :
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a comment to explain why this factor 2?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still not entirely sure to be honest, I went down memory lane and git blame takes it back to this 1d1febc. It is a delay to avoid checking fifo_empty interrupt since it is hard to synchronize with the scoreboard and it is doubled since the prim_packer may also be holding more data. I haven't figured it out, I will rework the existing comment and add a TODO, so we can revisit it post M4.

$urandom_range(0, key_process_cycles * 2));

if (do_hash_start) begin
fork
Expand Down
9 changes: 3 additions & 6 deletions hw/ip/prim/rtl/prim_sha2.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ module prim_sha2 import prim_sha2_pkg::*;
input logic [7:0] digest_we_i,
output sha_word64_t [7:0] digest_o, // tie off unused port slice when MultimodeEn = 0
output logic digest_on_blk_o, // digest being computed for a complete block
output fifoctl_state_e fifo_st_o,
output logic hash_running_o, // `1` iff hash computation is active (as opposed to `idle_o`, which
// is also `0` and thus 'busy' when waiting for a FIFO input)
output logic idle_o
Expand Down Expand Up @@ -306,14 +307,10 @@ module prim_sha2 import prim_sha2_pkg::*;
else hash_done_o <= hash_done_next;
end

typedef enum logic [1:0] {
FifoIdle,
FifoLoadFromFifo,
FifoWait
} fifoctl_state_e;

fifoctl_state_e fifo_st_q, fifo_st_d;

assign fifo_st_o = fifo_st_q;

always_ff @(posedge clk_i or negedge rst_ni) begin
if (!rst_ni) fifo_st_q <= FifoIdle;
else fifo_st_q <= fifo_st_d;
Expand Down
21 changes: 15 additions & 6 deletions hw/ip/prim/rtl/prim_sha2_32.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ module prim_sha2_32 import prim_sha2_pkg::*;
// to hash.
assign hash_go = hash_start_i | hash_continue_i;

fifoctl_state_e fifo_st;

// tie off unused ports/port slices
if (!MultimodeEn) begin : gen_tie_unused
logic unused_signals;
Expand Down Expand Up @@ -78,8 +80,13 @@ module prim_sha2_32 import prim_sha2_pkg::*;
// accumulate most significant 32 bits of word and mask bits
word_buffer_d.data[63:32] = fifo_rdata_i.data;
word_buffer_d.mask[7:4] = fifo_rdata_i.mask;
word_part_inc = 1'b1;
fifo_rready_o = 1'b1;
if (fifo_st == FifoLoadFromFifo) begin
fifo_rready_o = 1'b1; // load word from FIFO
word_part_inc = 1'b1;
end else begin
fifo_rready_o = 1'b0; // do not load from FIFO
word_part_inc = 1'b0;
end
end else begin // SHA2_256 so pad and push out the word
word_valid = 1'b1;
// store the word with most significant padding
Expand All @@ -98,8 +105,8 @@ module prim_sha2_32 import prim_sha2_pkg::*;
fifo_rready_o = 1'b0;
end
end

end else if (word_part_count_q == 2'b01) begin
fifo_rready_o = 1'b1; // buffer still has room for another word
// accumulate least significant 32 bits and mask
word_buffer_d.data [31:0] = fifo_rdata_i.data;
word_buffer_d.mask [3:0] = fifo_rdata_i.mask;
Expand All @@ -116,12 +123,12 @@ module prim_sha2_32 import prim_sha2_pkg::*;
end
if (sha_ready == 1'b1) begin
// word has been consumed
fifo_rready_o = 1'b1; // word pushed out to SHA engine so word buffer ready
fifo_rready_o = 1'b1; // word pushed out to SHA engine so word buffer ready
word_part_reset = 1'b1;
word_part_inc = 1'b0;
end else begin
fifo_rready_o = 1'b1;
word_part_inc = 1'b1;
fifo_rready_o = 1'b0;
word_part_inc = 1'b0;
end
end else if (word_part_count_q == 2'b10) begin // word buffer is full and not loaded out yet
// fifo_rready_o is now deasserted: accumulated word is waiting to be pushed out
Expand Down Expand Up @@ -206,6 +213,7 @@ module prim_sha2_32 import prim_sha2_pkg::*;
.digest_we_i (digest_we_i),
.digest_o (digest_o),
.digest_on_blk_o (digest_on_blk_o),
.fifo_st_o (fifo_st),
.hash_running_o (hash_running_o),
.idle_o (idle_o)
);
Expand Down Expand Up @@ -259,6 +267,7 @@ module prim_sha2_32 import prim_sha2_pkg::*;
.digest_we_i (digest_we_i),
.digest_o (digest_o),
.digest_on_blk_o (digest_on_blk_o),
.fifo_st_o (fifo_st),
.hash_running_o (hash_running_o),
.idle_o (idle_o)
);
Expand Down
6 changes: 6 additions & 0 deletions hw/ip/prim/rtl/prim_sha2_pkg.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ package prim_sha2_pkg;
// set to all-1 for word-aligned input
} sha_fifo64_t;

typedef enum logic [1:0] {
FifoIdle,
FifoLoadFromFifo,
FifoWait
} fifoctl_state_e;

// one-hot encoded
typedef enum logic [3:0] {
SHA2_256 = 4'b0001,
Expand Down
Loading