[kmac] Change fifo_empty interrupt type to status

[vogelpi]

This resolves #21049.

Note that this PR currently contains a cherry-picked commit from @msfschaffner . In addition, some automated TLT sequence needs to be adjusted as well for this PR to pass CI. I am mainly creating now for the sake of visilbility.
Together with @andreaskurth , I've come to the conclusion that it is better to not signal the empty status interrupt if software is not using the module. For example, software shouldn't get interrupted if an application interface is using KMAC (the app interface will handle the FIFO condition autonomously) or if the KMAC block is completely idle. But only if the FIFO runs empty while software is using the block and had to stop writing data at some point. I've now changed the design accordingly. This should work independently of potential TLT changes and CIP library modifications.

Since this change is software visible, one could argue to create a new version of the IP for this which would also require to go back to D1/V1. However, it seems that the current software only uses the untouched Done and Err interrupts but not the FIFO_Empty interrupt changed with this PR. For example:

• Grepping for KMAC_INTR_ENABLE_FIFO_EMPTY, i.e. the bit offset for enabling the interrupt in software yields nothing.
• Grepping for KMAC_INTR_STATE_FIFO_EMPTY gives one hit inside sw/device/silicon_creator/lib/drivers/kmac_unittest.cc only.
• Grepping for KMAC_INTR_TEST_FIFO_EMPTY just gives the SV parameter definition for the reset value of this bit in kmac_reg_pkg.sv (auto-generated).
• Grepping for KMAC_INTR_COMMON_FIFO_EMPTY gives one function kmac_get_irq_bit_index() inside sw/device/lib/dif/autogen/dif_kmac_autogen.c. This function is called from within
  • dif_kmac_irq_is_pending()
  • dif_kmac_irq_acknowledge()
  • dif_kmac_irq_force()
  • dif_kmac_irq_set_enabled()
  • dif_kmac_irq_set_enabled()
    None of these functions are called with kDifKmacIrqFifoEmpty as argument.

Most likely, FIFO empty interrupt is not used because it's not very usable as long as it's of type "event". I thus suggest avoid the hassle and not introduce a new version and remain at D2S/V2S if possible.

[vogelpi]

@jadephilipoom I am wondering if the current implementation of this PR makes sense from a software perspective. To do a KMAC operation, software would:

1. Configure the hardware block including keys etc.
2. Provide the Start command. After this the message FIFO becomes writable and the empty interrupt is immediately raised. -> I believe raising the interrupt here is not meaningful because it did not have a chance to write anything at this point. It's expected that the FIFO is empty. To work around this, software can disable the empty interrupt inside KMAC.
3. Write the message to the FIFO.
   • For messages shorter than 576 bits, the entire message can be pushed in one shot.
   • For longer messages, push as much as possible / until the FIFO is full. Then wait for the interrupt to push more. Enable the interrupt after filling the FIFO once.
4. Write the final Process command. After this, the hardware empties the FIFO (interrupt asserts again) and does the final processing. I don't think this interrupt is useful and software would most likely disable it again inside KMAC.
5. Manually triggered squeezing without providing new input. Also in this phase the interrupt is not useful.

What is your view, does this make sense? I am not sure if disabling/enabling interrupts at this granularity is practical for software. If yes, I could add information on this to the programmer's guide. If no, I could try to implement this in hardware.

[jadephilipoom]

I can confirm the existing KMAC software doesn't use this interrupt (details below). My opinion is that using the interrupt to detect when to write to the FIFO is a little bit of an annoying interface for software, which is why it's not currently being used, so I'm in favor of a higher-level or non-interrupt-based software interface.

The DIF calculates for itself how much it can write:

opentitan/sw/device/lib/dif/dif_kmac.c

Lines 645 to 650 in 89f6bec

	// Calculate the remaining space in the message FIFO based on the
	// `FIFO_DEPTH` status field.
	size_t free_entries = (KMAC_PARAM_NUM_ENTRIES_MSG_FIFO - status.fifo_depth);
	size_t max_len = free_entries * KMAC_PARAM_NUM_BYTES_MSG_FIFO_ENTRY;
	size_t write_len = (len < max_len) ? len : max_len;
	msg_fifo_write(kmac, data, write_len);

The cryptolib driver simply writes the message straight out, but there's an open issue that says it should eventually follow the DIF:
#16410

The silicon_creator driver also writes the message straight out, because it's for SPHINCS+ and can assume that KMAC is only going to be used for SHAKE operations:

opentitan/sw/device/silicon_creator/lib/drivers/kmac.c

Lines 262 to 270 in 89f6bec

	// This implementation does not poll `STATUS.fifo_depth` as recommended in
	// the KMAC documentation. Normally, polling is required to prevent a
	// deadlock scenario between Ibex, KMAC, and EDN. However, in this case it is
	// safe to skip because `kmac_shake256_configure()` sets KMAC to use
	// software-only entropy, and sets `kmac_en` to false (so KMAC will not
	// produce entropy requests anyway). Since KMAC will therefore not block on
	// EDN, it is guaranteed to keep processing message blocks. For more details,
	// see the KMAC documentation:
	// https://docs.opentitan.org/hw/ip/kmac/doc/#fifo-depth-and-empty-status

CC @ballifatih

[vogelpi]

Thanks for your feedback @jadephilipoom !

What you wrote makes a lot of sense to me. And I think this means even how the interrupt works right now with this PR is not going to be very useful.

I think I am going to try to change the hardware to only assert the interrupt in Phase 3 above and only if the FIFO has been filled up to a certain threshold once. In practice, KMAC will most likely empty the FIFO faster than software can fill it up, unless the core is currently processing or waiting for entropy. If software then observes that FIFO depth, it knows 1) that it will take some time until KMAC empties the FIFO again, 2) that KMAC will signal this with an interrupt. But we don't want so send pointless interrupts to software. In my view this is the only way this interrupt could ever become useful for software.

[vogelpi]

I've now updated the interrupt implementation to only make the interrupt firing if the message FIFO has run full previously. This can happen e.g. if the SHA3 engine is busy and cannot accept more data currently, or if the KMAC block is waiting for entropy from EDN (can take hundreds of clock cycles). In all other cases, hardware will consume the data faster than software can provide it and there is no point in interrupting the software to tell it to work faster :-)

Also, the interrupt is not firing if 1) a hardware application interface is using the block, 2) if the SHA3 engine is not in the Absorb state (i.e., not accepting input data anyway), or 3) if software triggered the Process command already (no more input data is accepted).

[andreaskurth]

Thanks for this change, @vogelpi 👍 LGTM with two suggestions

[vogelpi]

Thanks for your review @andreaskurth and the feedback. I've addressed your comments and pushed again.

[vogelpi]

I've now rebased this PR on top of @msfschaffner 's flash_ctrl status type interrupt PR to include some relevant changes and avoid merge skew issues.

[andreaskurth]

Thanks @vogelpi! Merging