Skip to content

Commit

Permalink
[hmac] Block the configuration change while processing
Browse files Browse the repository at this point in the history 
There were a few test failures on the sudden CFG register update during
SHA/HMAC process. A few bugs caused system hang. They blocked the
interconnect so that the software cannot release the errors.

Related commits: 14a4312 13678cf

Latest one that @cindychip found at Issue #991 also caused hang
condition inside HMAC. Rather fixing every hang condition, it seems to
be better to block the CFG update in the middle of the process. If the
software wants to update CFG, it can update CFG but it only can be
visible to the hardware after the current HMAC/SHA process is completed.

This PR also blocks the secret key update while the hash engine is
active. Secret key (KEY0 .. KEY7) can only be updated after the engine
completes the computing of hash and before the new message is fed.

If the event occurs (secret key is updated in process), the error will
be reported to the software through the ERROR FIFO.

This is related to #991

Signed-off-by: Eunchan Kim <eunchan@opentitan.org>
  • Loading branch information
Eunchan Kim committed Nov 20, 2019
1 parent d42948c commit 3594d96
Show file tree
Hide file tree
Showing 6 changed files with 216 additions and 233 deletions.
17 changes: 15 additions & 2 deletions hw/ip/hmac/data/hmac.hjson
View file
Expand Up @@ -31,9 +31,16 @@
regwidth: "32",
registers: [
{ name: "CFG",
desc: "HMAC Configuration register",
desc: '''HMAC Configuration register.

The register is updated when the engine is in Idle.
If the software updates the register while the engine computes the hash,
the updated value is discarded.
''',
hwext: "true",
hwqe: "true",
swaccess: "rw",
hwaccess: "hro",
hwaccess: "hrw",
fields: [
{ bits: "0",
name: "hmac_en",
Expand Down Expand Up @@ -137,9 +144,15 @@
SHA256 assumes secret key is hashed 256bit key.
Order of the secret key is:
key[255:0] = {KEY0, KEY1, KEY2, ... , KEY7};

The registers are allowed to be updated when the engine is in Idle state.
If the engine computes the hash, it discards any attempts to update the secret keys
and report an error.
''',
count: "NumWords",
cname: "HMAC",
hwext: "true",
hwqe : "true",
swaccess: "wo",
hwaccess: "hrw",
fields: [
Expand Down
7 changes: 7 additions & 0 deletions hw/ip/hmac/doc/_index.md
View file
Expand Up @@ -269,6 +269,13 @@ void run_hmac(uint32_t *msg, uint32_t msg_len, uint32_t *hash) {
}
```

## Updating the configurations

The HMAC IP prevents {{< regref "CFG" >}} and {{< regref "KEY" >}} registers from updating during the engine is processing the messages.
The attempts are discarded.
The {{< regref "KEY" >}} discards the attempt of the secret key access in the middle of the process.
In case of when the software tries to update the KEY, the IP reports an error through the Error FIFO. The error code is `SwUpdateSecretKeyInProcess`, `0x0003`.

## Interrupt Handling

### FIFO_FULL
Expand Down
78 changes: 68 additions & 10 deletions hw/ip/hmac/rtl/hmac.sv
View file
Expand Up @@ -91,6 +91,9 @@ module hmac (

sha_word_t [7:0] digest;

hmac_reg2hw_cfg_reg_t cfg_reg;
logic cfg_block; // Prevent changing config

///////////////////////
// Connect registers //
///////////////////////
Expand All @@ -102,20 +105,40 @@ module hmac (
assign wipe_secret = reg2hw.wipe_secret.qe;
assign wipe_v = reg2hw.wipe_secret.q;

always_ff @(posedge clk_i or negedge rst_ni) begin
if (!rst_ni) begin
secret_key <= '0;
end else if (wipe_secret) begin
secret_key <= secret_key ^ {8{wipe_v}};
end else if (!cfg_block) begin
// Allow updating secret key only when the engine is in Idle.
for (int i = 0; i < 8; i++) begin
if (reg2hw.key[7-i].qe) begin
secret_key[32*i+:32] <= reg2hw.key[7-i].q;
end
end
end
end

for (genvar i = 0; i < 8; i++) begin : gen_key_digest
// secret key
assign secret_key[32*i +: 32] = reg2hw.key[7-i].q;
assign hw2reg.key[i].de = wipe_secret;
assign hw2reg.key[7-i].d = secret_key[32*i +: 32] ^ wipe_v;
assign hw2reg.key[7-i].d = '0;
// digest
assign hw2reg.digest[i].d = conv_endian(digest[i], digest_swap);
end

logic [3:0] unused_cfg_qe;

assign unused_cfg_qe = {cfg_reg.sha_en.qe, cfg_reg.hmac_en.qe,
cfg_reg.endian_swap.qe, cfg_reg.digest_swap.qe};

assign sha_en = reg2hw.cfg.sha_en.q;
assign hmac_en = reg2hw.cfg.hmac_en.q;
assign endian_swap = reg2hw.cfg.endian_swap.q;
assign digest_swap = reg2hw.cfg.digest_swap.q;
assign sha_en = cfg_reg.sha_en.q;
assign hmac_en = cfg_reg.hmac_en.q;
assign endian_swap = cfg_reg.endian_swap.q;
assign digest_swap = cfg_reg.digest_swap.q;
assign hw2reg.cfg.hmac_en.d = cfg_reg.hmac_en.q;
assign hw2reg.cfg.sha_en.d = cfg_reg.sha_en.q;
assign hw2reg.cfg.endian_swap.d = cfg_reg.endian_swap.q;
assign hw2reg.cfg.digest_swap.d = cfg_reg.digest_swap.q;

assign reg_hash_start = reg2hw.cmd.hash_start.qe & reg2hw.cmd.hash_start.q;
assign reg_hash_process = reg2hw.cmd.hash_process.qe & reg2hw.cmd.hash_process.q;
Expand All @@ -129,6 +152,23 @@ module hmac (
/////////////////////
assign hash_start = reg_hash_start & sha_en;

always_ff @(posedge clk_i or negedge rst_ni) begin
if (!rst_ni) begin
cfg_block <= '0;
end else if (hash_start) begin
cfg_block <= 1'b 1;
end else if (reg_hash_done) begin
cfg_block <= 1'b 0;
end
end
// Hold the configuration during the process
always_ff @(posedge clk_i or negedge rst_ni) begin
if (!rst_ni) begin
cfg_reg <= '{endian_swap: '{q: 1'b1, qe: 1'b0}, default:'0};
end else if (!cfg_block && reg2hw.cfg.hmac_en.qe) begin
cfg_reg <= reg2hw.cfg ;
end
end
////////////////
// Interrupts //
////////////////
Expand Down Expand Up @@ -376,16 +416,30 @@ module hmac (
/////////////////////////
// HMAC Error Handling //
/////////////////////////
logic msg_push_sha_disabled, hash_start_sha_disabled;
logic msg_push_sha_disabled, hash_start_sha_disabled, update_seckey_inprocess;
assign msg_push_sha_disabled = msg_write & ~sha_en;
assign hash_start_sha_disabled = reg_hash_start & ~sha_en;

always_comb begin
update_seckey_inprocess = 1'b0;
if (cfg_block) begin
for (int i = 0 ; i < 8 ; i++) begin
if (reg2hw.key[i].qe) begin
update_seckey_inprocess = update_seckey_inprocess | 1'b1;
end
end
end else begin
update_seckey_inprocess = 1'b0;
end
end

// Update ERR_CODE register and interrupt only when no pending interrupt.
// This ensures only the first event of the series of events can be seen to sw.
// It is recommended that the software reads ERR_CODE register when interrupt
// is pending to avoid any race conditions.
assign err_valid = ~reg2hw.intr_state.hmac_err.q &
( msg_push_sha_disabled | hash_start_sha_disabled );
( msg_push_sha_disabled | hash_start_sha_disabled
| update_seckey_inprocess);

always_comb begin
err_code = NoError;
Expand All @@ -397,6 +451,10 @@ module hmac (
err_code = SwHashStartWhenShaDisabled;
end

update_seckey_inprocess: begin
err_code = SwUpdateSecretKeyInProcess;
end

default: begin
err_code = NoError;
end
Expand Down
3 changes: 2 additions & 1 deletion hw/ip/hmac/rtl/hmac_pkg.sv
View file
Expand Up @@ -86,7 +86,8 @@ package hmac_pkg;
typedef enum logic [31:0] {
NoError = 32'h 0000_0000,
SwPushMsgWhenShaDisabled = 32'h 0000_0001,
SwHashStartWhenShaDisabled = 32'h 0000_0002
SwHashStartWhenShaDisabled = 32'h 0000_0002,
SwUpdateSecretKeyInProcess = 32'h 0000_0003
} err_code_e;

endpackage : hmac_pkg
50 changes: 35 additions & 15 deletions hw/ip/hmac/rtl/hmac_reg_pkg.sv
View file
Expand Up @@ -54,15 +54,19 @@ package hmac_reg_pkg;
typedef struct packed {
struct packed {
logic q;
logic qe;
} hmac_en;
struct packed {
logic q;
logic qe;
} sha_en;
struct packed {
logic q;
logic qe;
} endian_swap;
struct packed {
logic q;
logic qe;
} digest_swap;
} hmac_reg2hw_cfg_reg_t;

Expand All @@ -84,6 +88,7 @@ package hmac_reg_pkg;

typedef struct packed {
logic [31:0] q;
logic qe;
} hmac_reg2hw_key_mreg_t;


Expand All @@ -102,6 +107,21 @@ package hmac_reg_pkg;
} hmac_err;
} hmac_hw2reg_intr_state_reg_t;

typedef struct packed {
struct packed {
logic d;
} hmac_en;
struct packed {
logic d;
} sha_en;
struct packed {
logic d;
} endian_swap;
struct packed {
logic d;
} digest_swap;
} hmac_hw2reg_cfg_reg_t;

typedef struct packed {
struct packed {
logic d;
Expand All @@ -121,7 +141,6 @@ package hmac_reg_pkg;

typedef struct packed {
logic [31:0] d;
logic de;
} hmac_hw2reg_key_mreg_t;

typedef struct packed {
Expand All @@ -143,26 +162,27 @@ package hmac_reg_pkg;
// Register to internal design logic //
///////////////////////////////////////
typedef struct packed {
hmac_reg2hw_intr_state_reg_t intr_state; // [308:306]
hmac_reg2hw_intr_enable_reg_t intr_enable; // [305:303]
hmac_reg2hw_intr_test_reg_t intr_test; // [302:297]
hmac_reg2hw_cfg_reg_t cfg; // [296:293]
hmac_reg2hw_cmd_reg_t cmd; // [292:289]
hmac_reg2hw_wipe_secret_reg_t wipe_secret; // [288:256]
hmac_reg2hw_key_mreg_t [7:0] key; // [255:0]
hmac_reg2hw_intr_state_reg_t intr_state; // [320:318]
hmac_reg2hw_intr_enable_reg_t intr_enable; // [317:315]
hmac_reg2hw_intr_test_reg_t intr_test; // [314:309]
hmac_reg2hw_cfg_reg_t cfg; // [308:301]
hmac_reg2hw_cmd_reg_t cmd; // [300:297]
hmac_reg2hw_wipe_secret_reg_t wipe_secret; // [296:264]
hmac_reg2hw_key_mreg_t [7:0] key; // [263:0]
} hmac_reg2hw_t;

///////////////////////////////////////
// Internal design logic to register //
///////////////////////////////////////
typedef struct packed {
hmac_hw2reg_intr_state_reg_t intr_state; // [631:629]
hmac_hw2reg_status_reg_t status; // [628:629]
hmac_hw2reg_err_code_reg_t err_code; // [628:629]
hmac_hw2reg_key_mreg_t [7:0] key; // [628:365]
hmac_hw2reg_digest_mreg_t [7:0] digest; // [364:109]
hmac_hw2reg_msg_length_lower_reg_t msg_length_lower; // [108:109]
hmac_hw2reg_msg_length_upper_reg_t msg_length_upper; // [108:109]
hmac_hw2reg_intr_state_reg_t intr_state; // [627:625]
hmac_hw2reg_cfg_reg_t cfg; // [624:617]
hmac_hw2reg_status_reg_t status; // [616:617]
hmac_hw2reg_err_code_reg_t err_code; // [616:617]
hmac_hw2reg_key_mreg_t [7:0] key; // [616:361]
hmac_hw2reg_digest_mreg_t [7:0] digest; // [360:105]
hmac_hw2reg_msg_length_lower_reg_t msg_length_lower; // [104:105]
hmac_hw2reg_msg_length_upper_reg_t msg_length_upper; // [104:105]
} hmac_hw2reg_t;

// Register Address
Expand Down

0 comments on commit 3594d96

Please sign in to comment.