[spi_device] Late CSb de-assertion for op-complete commands

[eunchan]

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

[andreaskurth]

Triaged for spi_device. Assigning to M2.5 with Priority:P2 Priority: medium 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 Type:FutureRelease Not relevant to currently planned releases/milestones .

[hcallahan-lowrisc]

• If host de-asserts CSb prior to the full 8 bit opcode, HW IP silently drops

From the V2 review #15539, it sounds like #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

[andreaskurth]

From the V2 review #15539, it sounds like #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

opentitan/hw/dv/sv/spi_agent/spi_host_driver.sv

Lines 95 to 99 in 99d8423

	SpiTransIncompleteOpcode: begin
	// TODO(#15721), need to have at least 3 spi_clk when CSB is active, otherwise,
	// this incomplete req may have bad side effect due to `sys_csb_deasserted_pulse_i`
	// isn't set correctly
	int num_bits = $urandom_range(3, 7);

due to the known limitation in #15721

[hcallahan-lowrisc]

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. (#10058 #12747 #15517 #15721)

[moidx]

Postponed for production integration.

[a-will]

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.

[msfschaffner]

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).