[hmac, rtl] Loading SHA-256 back thru DIGEST registers.

[ballifatih]

Description

I am running SHA-256 engine in streaming mode with the following vector (by updating sha256_functest):

static const uint8_t kLongMessage[] = {
0x45, 0x11, 0x01, 0x25, 0x0e, 0xc6, 0xf2, 0x66,
0x52, 0x24, 0x9d, 0x59, 0xdc, 0x97, 0x4b, 0x73,
0x61, 0xd5, 0x71, 0xa8, 0x10, 0x1c, 0xdf, 0xd3,
0x6a, 0xba, 0x3b, 0x58, 0x54, 0xd3, 0xae, 0x08,
0x6b, 0x5f, 0xdd, 0x45, 0x97, 0x72, 0x1b, 0x66,
0xe3, 0xc0, 0xdc, 0x5d, 0x8c, 0x60, 0x6d, 0x96,
0x57, 0xd0, 0xe3, 0x23, 0x28, 0x3a, 0x52, 0x17,
0xd1, 0xf5, 0x3f, 0x2f, 0x28, 0x4f, 0x57, 0xb8,
// here ends the first 512b block
0x5c, 0x8a, 0x61, 0xac, 0x89, 0x24, 0x71, 0x1f,
0x89, 0x5c, 0x5e, 0xd9, 0x0e, 0xf1, 0x77, 0x45,
0xed, 0x2d, 0x72, 0x8a, 0xbd, 0x22, 0xa5, 0xf7,
0xa1, 0x34, 0x79, 0xa4, 0x62, 0xd7, 0x1b, 0x56,
0xc1, 0x9a, 0x74, 0xa4, 0x0b, 0x65, 0x5c, 0x58,
0xed, 0xfe, 0x0a, 0x18, 0x8a, 0xd2, 0xcf, 0x46,
0xcb, 0xf3, 0x05, 0x24, 0xf6, 0x5d, 0x42, 0x3c,
0x83, 0x7d, 0xd1, 0xff, 0x2b, 0xf4, 0x62, 0xac,
// here ends the second 512b block
0x41, 0x98, 0x00, 0x73, 0x45, 0xbb, 0x44, 0xdb,
0xb7, 0xb1, 0xc8, 0x61, 0x29, 0x8c, 0xdf, 0x61,
0x98, 0x2a, 0x83, 0x3a, 0xfc, 0x72, 0x8f, 0xae,
0x1e, 0xda, 0x2f, 0x87, 0xaa, 0x2c, 0x94, 0x80,
0x85, 0x8b, 0xec,
// the partial block ends here (35 bytes)
};
static const uint8_t kLongMessageDigest[] = {
0x3c, 0x59, 0x3a, 0xa5, 0x39, 0xfd, 0xcd, 0xae, 0x51, 0x6c, 0xdf, 0x2f, 0x15, 0x00, 0x0f, 0x66, 0x34, 0x18, 0x5c, 0x88, 0xf5, 0x05, 0xb3, 0x97, 0x75, 0xfb, 0x9a, 0xb1, 0x37, 0xa1, 0x0a, 0xa2,
};

Here is what my driver implementation is doing:

• Initialize CFG registers, start operation and feed the first two 512b blocks continuously (which is done by hmac_init and hmac_update functions).
• Issue stop to HMAC, wait for interrupt and then store the context: DIGESTs, LOWER and UPPER.
• Later, reload the configuration and context regs DIGESTs, LOWER, UPPER and issue continue to HMAC.
• Feed the remaining 35 bytes and then issue process.
• Poll the interrupt register, and then read the result from the digest.

My local branch is https://github.com/lowRISC/opentitan/compare/master...ballifatih:opentitan:mv_hmac_impl To reproduce with verilator: ./bazelisk.sh test --test_output=streamed //sw/device/tests/crypto:sha256_functest_sim_verilator I am also running this as a TLT, with the following on my local branch: ./util/dvsim/dvsim.py hw/top_earlgrey/dv/chip_sim_cfg.hjson -i chip_sw_hmac --fixed-seed 1 --waves fsdb

From the waveform, I observed that when I am reloading DIGEST values (that I confirmed by looking at TL-UL transactions), they do not end up in the internal digest_o = digest_q register of prim_sha2.sv. Also there are actually 7 pulses from digest_sw_we signals instead of 8:

I suspect that the indexing part in the expression reg2hw.digest[i+1] might be the root cause:

https://github.com/lowRISC/opentitan/blob/07352ce012de17dde18313a8a1b3814375d02106/hw/ip/hmac/rtl/hmac.sv#L235-L242

cc: @gdessouky @martin-velay @moidx

[gdessouky]

yes, for starters, there was a change implemented in the digest indexing, and this line digest_sw_we[i] = reg2hw.digest[i+1].qe was not accordingly updated. As you suspect, this needs to be changed to digest_sw_we[i] = reg2hw.digest[i].qe.

[gdessouky]

I suspect a more critical issue as well though. In prim_sha2.sv digest_i flows to digest_d so long as sha_en is deasserted.

https://github.com/lowRISC/opentitan/blob/07352ce012de17dde18313a8a1b3814375d02106/hw/ip/prim/rtl/prim_sha2.sv#L157-L159

In hmac.sv, we check the configured digest size and update digest_sw[i] accordingly, but only once the hash is either started or continued, which happens after sha_en is asserted again, so it will not flow to prim_sha2.sv. https://github.com/lowRISC/opentitan/blob/07352ce012de17dde18313a8a1b3814375d02106/hw/ip/hmac/rtl/hmac.sv#L235-L242

We will probably need to change the checks in hmac.sv and have the reg2hw.digest flowing to digest_sw according to whatever digest size is configured, irrespective of whether the hash has been started/continued or not.

[gdessouky]

You need to deassert sha_en after context switching https://github.com/ballifatih/opentitan/blob/mv_hmac_impl/sw/device/lib/crypto/drivers/hmac.c#L191, and sha_en must stay deasserted when you write context back so that it can flow to prim_sha2.sv

[martin-velay]

You need to deassert sha_en after context switching https://github.com/ballifatih/opentitan/blob/mv_hmac_impl/sw/device/lib/crypto/drivers/hmac.c#L191, and sha_en must stay deasserted when you write context back so that it can flow to prim_sha2.sv

Hi Ghada, Is it a temporary fix or a behavior we want to keep? Otherwise, we need to update the HMAC Programmer's Guide accordingly as deasserting sha_en is not mentioned.

[ballifatih]

The root cause was the index, for which I have the small fix + doc update #23018.

Is it a temporary fix or a behavior we want to keep? Otherwise, we need to update the HMAC Programmer's Guide accordingly as deasserting sha_en is not mentioned.

I think sha_en having this tricky behavior is not ideal, but the driver can handle that. I think we can keep it for the moment.

The bigger issue is if we switch between different digest-sized streams, then there is again the issue of reloading the digest. digest_size_started_q will block us from updating the internal digest value. Here is the code that causes that:

https://github.com/lowRISC/opentitan/blob/07352ce012de17dde18313a8a1b3814375d02106/hw/ip/hmac/rtl/hmac.sv#L235

[gdessouky]

Yes, exactly, this is what I refer to in my comment above https://github.com/lowRISC/opentitan/issues/23014#issuecomment-2100595419. I'll need to look into the gating checks in hmac.sv to allow the digest CSRs to flow to digest_sw without requiring that the HMAC is continued or started.