[aes, pre-dv] Pre-dv Verilator testbench for AES #11
Conversation
andrea-caforio
force-pushed
the
aes-pre-dv-tb
branch
3 times, most recently
from
26b1dd7 to
ca13f34
Compare
There was a problem hiding this comment.
Thanks @andrea-caforio for your PR. I have some comments and questions but it's all minor. This looks nice!
|
|
||
| `include "prim_assert.sv" | ||
|
|
||
| module tlul_adapter |
There was a problem hiding this comment.
I am not fully happy with the name here but I don't have a better one. Basically this is just the thing to convert the generic TB requests to TLUL. And the other one converts valid-hold protocol requests to TLUL. I wouldn't rename the other one but maybe this one here should just be a tlul_adapter_tb_reqs or aes_tb_reqs_tlul_adapter? I don't really know. What is your view on this?
There was a problem hiding this comment.
I changed it to tlul_adapter_tb_reqs. With this name it still reflects the generic nature of the requests which are not necessarily tied to AES.
| logic pending_d, pending_q; | ||
| logic req_ack, resp_ack; | ||
|
|
||
| assign req_ack = tlul_en_i & tl_i.a_ready & tl_o.a_valid; |
There was a problem hiding this comment.
The whole logic in here is very similar to the tlul_adapter_shim but that's probably because the TB requests have been designed with the tlul_adapter_shim in mind. I think this is okay and still appreciate having this very simple option available, too.
There was a problem hiding this comment.
Indeed, it's lifted from the shim adapter. It's a consequence of having written the shim before the new tlul_adapter_tb_reqs. I didn't want to change the shim RTL, so this new adapter follows the design of the shim even if it introduces some code duplication.
| .id_i ( '0 ) | ||
| ); | ||
| end else begin : gen_tlul_adapter | ||
| tlul_adapter #( |
There was a problem hiding this comment.
Question: what happens for the final read_caliptra(CALIPTRA_NAME_0_OFFSET), calls in case the shim is not there?
There was a problem hiding this comment.
First, I thought the AES register file would signal an error for a read with an invalid address but it seems to accept it and just return 0 instead. I added a comment about this on the docstring of the read_caliptra function.
andrea-caforio
force-pushed
the
aes-pre-dv-tb
branch
2 times, most recently
from
fdbdf19 to
241d89b
Compare
|
Thank you @vogelpi for the review. :-) |
There was a problem hiding this comment.
Thanks @andrea-caforio !
LGTM except for one unresolved comment, there was a misunderstanding regarding the line breaks in MarkDown files. It should be 1 sentence per line.
This commit contains a comprehensive pre-dv testbench for the AES IP block, invoking all the possible modes of operations (+GCM) with different key and inputs sizes. By default, the communication with the IP happens over the TLUL bus, but if a TLUL/shim adapter is configured, messages can be relayed through the shim first. There are two generic modules `tlul_adapter` and `tlul_delayer` that are agnostic to the actual testbench and can be reused in other testbenches that require a robust TLUL handler. This testbench mimics already existing pre-dv suites for Ascon and KMAC. Signed-off-by: Andrea Caforio <[email protected]>
andrea-caforio
force-pushed
the
aes-pre-dv-tb
branch
from
241d89b to
4021b74
Compare
This commit contains a comprehensive pre-dv testbench for the AES IP block, invoking all the possible modes of operations (+GCM) with different key and inputs sizes. By default, the communication with the IP happens over the TLUL bus, but if a TLUL/shim adapter is configured, messages can be relayed through the shim first.
There are two generic modules
tlul_adapterandtlul_delayerthat are agnostic to the actual testbench and can be reused in other testbenches that require a robust TLUL handler.This testbench mimics already existing pre-dv suites for Ascon and KMAC.
I'm opening this PR here instead of upstream due to the GCM dependency