opentitan icon indicating copy to clipboard operation
opentitan copied to clipboard

[kmac] Add CAP CSR

Open eunchan opened this issue 3 years ago • 6 comments

Add Capability CSR to KMAC that includes the configurations but not limited to:

  • Masked/ Unmasked
  • MSGFIFO Depth / Width
  • Secret Key Size

CC: @jadephilipoom @cindychip @vogelpi @ballifatih @tjaychen

eunchan avatar Dec 20 '22 18:12 eunchan

ah this will probably eventually be absorbed into the versioning scheme that @cfrantz and @msfschaffner are working on.

tjaychen avatar Dec 20 '22 18:12 tjaychen

Summarizing from an offline chat with @eunchan about the message FIFO interface.

For software it would be convenient to know exactly how many bytes in the message FIFO are free. Currently, STATUS.FIFO_DEPTH only exposes how many entries are occupied, so to know how much is safe to write (as described here) software has to calculate (number of entries - fifo depth) * entry size.

That calculation is a tad cumbersome, and requires exposing implementation details like entry size. It's also slightly coarse; even if there is only one byte in a 64b entry, STATUS.FIFO_DEPTH will record it as one entry, and software has to assume all 64 bits are potentially occupied. Exposing the exact number of free bytes would be nice.

jadephilipoom avatar Dec 20 '22 18:12 jadephilipoom

Indeed, the plan is to add a standardized region to every IP where such parameter values can be exposed. If this is urgent, it would be ok to move on with the patch that @jadephilipoom has prepared - with the understanding that we will have to align these registers once support for this standardized region has been added.

msfschaffner avatar Dec 21 '22 16:12 msfschaffner

to represent precise remained space (in bytes), the packer module needs to expose the internal position info (how many bits are occupied). So that the Status may show how many bytes it has in space. (entries - depth) * entry_size - (packer_bits / 8).

eunchan avatar Dec 21 '22 18:12 eunchan

I think the changes proposed in this issue are nice, but I do not think they are high priority for the current release:

  • We already have an implementation that computes the available FIFO entries, and though not elegant for SW, it is possible. FIFO also supports mixing arbitrary byte/word writes, and the internal logic handles the alignment automatically. We use the number of entries only to infer whether potential stalling in KMAC is caused by EDN.
  • Secret key size and Masked/Unmasked info is useful for SW, but not an essential. At the moment, we can make do with the current HW.

I am updating the labels accordingly.

ballifatih avatar Apr 09 '24 07:04 ballifatih

Thanks @ballifatih , I support your assessment. As you say, the number of entries are only relevant if KMAC is potentially stalling due to EDN. During regular operation, the hardware is much faster in consuming the data written to the message FIFO than software providing it. The depth is always 0 in that case anyway.

Also, exposing configuration details is something that should be considered in a wider context. If we really want to do this, we should agree on a common way for all IPs similar to what #17117 aimed for. For specific reasons, we have reverted this in #19287.

For now, I am taking this off the M4 milestone. If we really want to do this in the future, we should consider doing something like in #17117 for all blocks.

vogelpi avatar May 06 '24 08:05 vogelpi