lowRISC / opentitan

OpenTitan: Open source silicon root of trust
https://www.opentitan.org
Apache License 2.0
2.44k stars 730 forks source link

Specify Entropy Distribution Network (EDN) #2693

Closed imphil closed 3 years ago

imphil commented 4 years ago

Define peripheral interface to Random Number Generator distribution network to be used by AES and OTBN and potentially other users of randomness.

imphil commented 4 years ago

CC @mwbranstad

imphil commented 4 years ago

Something along those lines could work (as done by @vogelpi in https://github.com/lowRISC/opentitan/pull/2140)

package csrng_pkg;
  ///////////////////////////
  // Peripheral Interfaces //
  ///////////////////////////

  parameter int PeriphEntropyUpdateWidth = 1;
  parameter int PeriphEntropyRequestWidth = 64;

  // Entropy update interface
  typedef struct packed {
    logic                                update;
    logic [PeriphEntropyUpdateWidth-1:0] entropy;
  } csrng_entropy_upd_t;

  parameter csrng_entropy_upd_t CSRNG_ENTROPY_UPD_DEFAULT = '{default: '0};

  // Entropy request interface
  typedef struct packed {
    logic                                 entropy_req;
  } csrng_entropy_req_t;
  typedef struct packed {
    logic                                 entropy_ack;
    logic [PeriphEntropyRequestWidth-1:0] entropy;
  } csrng_entropy_rsp_t;

  parameter csrng_entropy_req_t CSRNG_ENTROPY_REQ_DEFAULT = '{default: '0};
  parameter csrng_entropy_rsp_t CSRNG_ENTROPY_RSP_DEFAULT = '{default: '0};

endpackage : csrng_pkg
martin-lueker commented 3 years ago

Hi @vogelpi, Hi @imphil, Thanks for opening this issue. So after a couple aborted attempts, and a careful study of #2140, I must confess that I've forgotten at least one of the details of this interface.
I the latter two csrng_entropy_rsp_t and csrng_entropy_req_t structures seem clear. However can someone please remind me what is the intent behind the csrng_entropy_upd_t structure? (I've now spent a bit of time digging through old commits, and I now fear that the original explanation here has been squashed away.) My guess is that the rsp and req structures are likely sufficient for what I have been envisioning.

martin-lueker commented 3 years ago

Also, are there any needs for wider or narrower interfaces for various IP's? My hope is no, but I would like to confirm.

moidx commented 3 years ago

@tjaychen, @zi-v, @msfschaffner, @eunchan, @vogelpi, @cdgori PTAL

tjaychen commented 3 years ago

sorry let me clarify. I think the req/ack and upd idea may have come from me. My original thinking is that there are two types of csrng consumers. Those that consume large amounts of entropy in one go, but do not necessarily need it frequently (i imagine these to be keymgr, aes, bignum etc, who would request say...64b~128b of high quality entropy at a time, and then use it locally to expand for further usage). There are also those that do not necessarily need high quality entropy, but just entropy bits to mix into a local lfsr (for example alert senders and receivers). These do not explicitly need a req/ack, but can take an update entropy whenever csrng has them available to send out. Does this make sense?

We could of course generalize to req/ack completely if that is easier, but i always imagined two different classes of entropy consumers in hardware.

On Thu, Aug 6, 2020 at 10:16 AM moidx notifications@github.com wrote:

@tjaychen https://github.com/tjaychen, @zi-v https://github.com/zi-v, @msfschaffner https://github.com/msfschaffner, @eunchan https://github.com/eunchan, @vogelpi https://github.com/vogelpi, @cdgori https://github.com/cdgori PTAL

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/lowRISC/opentitan/issues/2693#issuecomment-670062714, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAH2RSUDFCIF3OXXYEDBMMDR7LQNPANCNFSM4OOWZWPQ .

tjaychen commented 3 years ago

to clarify, i basically thought of the req/ack group as point to point, and the update group as a broadcast. If that makes sense? This of course complicates things, because it potentially means you need to maintain two separate queues. Although the broadcast type IMO does not need to be very deep, and the update frequency itself does not need to be very high.

On Thu, Aug 6, 2020 at 7:46 PM Timothy Chen timothytim@google.com wrote:

sorry let me clarify. I think the req/ack and upd idea may have come from me. My original thinking is that there are two types of csrng consumers. Those that consume large amounts of entropy in one go, but do not necessarily need it frequently (i imagine these to be keymgr, aes, bignum etc, who would request say...64b~128b of high quality entropy at a time, and then use it locally to expand for further usage). There are also those that do not necessarily need high quality entropy, but just entropy bits to mix into a local lfsr (for example alert senders and receivers). These do not explicitly need a req/ack, but can take an update entropy whenever csrng has them available to send out. Does this make sense?

We could of course generalize to req/ack completely if that is easier, but i always imagined two different classes of entropy consumers in hardware.

On Thu, Aug 6, 2020 at 10:16 AM moidx notifications@github.com wrote:

@tjaychen https://github.com/tjaychen, @zi-v https://github.com/zi-v, @msfschaffner https://github.com/msfschaffner, @eunchan https://github.com/eunchan, @vogelpi https://github.com/vogelpi, @cdgori https://github.com/cdgori PTAL

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/lowRISC/opentitan/issues/2693#issuecomment-670062714, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAH2RSUDFCIF3OXXYEDBMMDR7LQNPANCNFSM4OOWZWPQ .

mwbranstad commented 3 years ago

@tjaychen I just read this update since the Ziv email response. I see what you are asking for in the broadcast case. I would imagine that we create a 4th distribution where the request line is on all of the time (ready line in my model). This would constantly request for new genbits from csrng in that case, unless some timer was attached to this distribution. Regarding quality of entropy, we discussed this recently in the group. This would need to be requested from the application and be propagated all the way to the entropy_src block I believe, based on the limited discussions. We can discuss that more later.

mwbranstad commented 3 years ago

One more add - I am not hung up on the dist names, so req/ack are fine also. However, I would ask that the simple rule of data transfer occurs when both signals are asserted.

tjaychen commented 3 years ago

Maybe it makes sense to have everything point to point for now? Because I think long term if we did want a broadcast, we could add a gasket pretty simply that internally still makes a req / ack on a timer (as you suggested) and broadcast out from there. What do you think?

On Fri, Aug 7, 2020 at 7:50 AM mwbranstad notifications@github.com wrote:

One more add - I am not hung up on the dist names, so req/ack are fine also. However, I would ask that the simple rule of data transfer occurs when both signals are asserted.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/lowRISC/opentitan/issues/2693#issuecomment-670554887, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAH2RSWMRR3S4XPY3VFID5LR7QIBZANCNFSM4OOWZWPQ .

msfschaffner commented 3 years ago

Indeed, it seems that the req / ack interface is sufficient for now, and we can always convert that to an update-only broadcast as @tjaychen describes.

One thing I wanted to mention also is that if we where to use a req / ack interface (where the data payload is transferred when both signals are high) we should be able to reuse the generalized DV interfaces that @udinator is going to work on. We have other interfaces in the system (e.g. between OTP and SRAMs) that use the same req / ack protocol.

RE the bitwidth of the interface... would a variable bitwidth interface be overly complicated to implement? If so, can we maybe standardize on a couple of aligned bitwidths that are supported by the distribution network? E.g. 32bit, 64bit and 128bit?

msfschaffner commented 3 years ago

CC @sriyerg

mwbranstad commented 3 years ago

@msfschaffner thanks for making these points, I was going to say about the same thing. For p2p vs broadcast, I think the edn block can be made to be programmable such that either req/ack or broadcast can be selected, with a timer being the proxy for the peripherals. Regarding bus width, powers of two will be no problem (1,2,4,..128).

tjaychen commented 3 years ago

Yeah i would suggest just standardizing the widths. The consumers can easily have a small piece of on their end to correctly batch up the requests before usage. Should hopefully make the source end point a bit more streamlined.

On Fri, Aug 7, 2020 at 1:14 PM mwbranstad notifications@github.com wrote:

@msfschaffner https://github.com/msfschaffner thanks for making these points, I was going to say about the same thing. For p2p vs broadcast, I think the edn block can be made to be programmable such that either req/ack or broadcast can be selected, with a timer being the proxy for the peripherals. Regarding bus width, powers of two will be no problem (1,2,4,..128).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/lowRISC/opentitan/issues/2693#issuecomment-670694912, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAH2RSUNTLDZFYPANAJ3KRLR7ROBJANCNFSM4OOWZWPQ .

msfschaffner commented 3 years ago

@mwbranstad Powers of two in the range that you suggested sounds good to me!

vogelpi commented 3 years ago

Thanks everybod for kicking off the discussion! This all makes sense.

I was wondering about how to support multiple bus widths. Since SV structs are not parameterizable, I think we will have to define different versions of the csrng_entropy_rsp_t struct above to either

  1. Have a couple of versions with different bus widths such as csrng_entropy_rsp_64_t and csrng_entropy_rsp_128_t from which consumers can select, or
  2. Have a consumer-specific version of the struct defined in the peripheral-specific package such as aes_csrng_entropy_rsp_t. Then the entropy distribution network needs to have all peripheral packages.

I am not fully convinced by these options and was wondering if there is maybe a better solution. Any ideas?

imphil commented 3 years ago

Spec is being worked on in #3391

imphil commented 3 years ago

Doc is now at https://docs.opentitan.org/hw/ip/edn/doc/index.html

martin-lueker commented 3 years ago

Thanks Philipp.