[spi_device] Late CSb de-assertion for op-complete commands
The current SPI_DEVICE HWIP has a few limitation of detecting the host system's misbehaviors:
- If host de-asserts CSb prior to the full 8 bit opcode, HW IP silently drops
- If host de-assrets CSb later than 8th beat, but the received opcode expects op-complete command (such as chip erase), it is expected to discard the command. The current version of SPI_DEVICE, if host terminates the command at 9th beat ~15th beat, the HW uploads the opcode but does not report any errors.
CC: @tjaychen @a-will @weicaiyang @jeoongp
Triaged for spi_device. Assigning to M2.5 with https://github.com/lowRISC/opentitan/labels/Priority%3AP2 because even though this is about robustness in an environment with errors, we should check with the product team if the behavior of spi_device is acceptable. If yes, we can defer this to https://github.com/lowRISC/opentitan/labels/Type%3AFutureRelease.
- If host de-asserts CSb prior to the full 8 bit opcode, HW IP silently drops
From the V2 review https://github.com/lowRISC/opentitan/pull/15539, it sounds like https://github.com/lowRISC/opentitan/pull/15637 was merged to check the block does not lock up if CSB de-asserts in bits 0-7.
- If host de-assrets CSb later than 8th beat, but the received opcode expects op-complete command (such as chip erase), it is expected to discard the command. The current version of SPI_DEVICE, if host terminates the command at 9th beat ~15th beat, the HW uploads the opcode but does not report any errors.
This condition appears to be currently untested, follow up to check priority of a fix + test of the expected discard behaviour.
estimate 8 remaining 2023-03-22 8
From the V2 review https://github.com/lowRISC/opentitan/issues/15539, it sounds like https://github.com/lowRISC/opentitan/pull/15637 was merged to check the block does not lock up if CSB de-asserts in bits 0-7.
Caution: Since PR #15770, this test has been weakened to https://github.com/lowRISC/opentitan/blob/99d8423fdf0babb4c8032e48cdf210b8056c1abc/hw/dv/sv/spi_agent/spi_host_driver.sv?plain=1#L95-L99
due to the known limitation in #15721
Note that there is a number of open issues about CSB behaviour and robustness to errors, so some estimated effort (if deemed necessary) may overlap between those issues. (https://github.com/lowRISC/opentitan/issues/10058 https://github.com/lowRISC/opentitan/issues/12747 https://github.com/lowRISC/opentitan/issues/15517 https://github.com/lowRISC/opentitan/issues/15721)
Postponed for production integration.
I don't think the host aborting commands needs to be reported to SW, but it might be good to upload only commands that are a valid length.
Agreed on that first point. The second point would be nice to have, but ok to implement in a future revision because it is technically not required from a functionality behavior (this functionality would be detecting out of spec behavior).