SPI device: major regression with non-flash mode

[rivos-eblot]

TPM support has introduced a major regression, as SPI device enters TPM mode while TPM is not enabled, breaking support for flash and passthrough modes.

It seems that several functions are using the wrong register bank, namely spi_regs to index TPM registers, whereas the proper bank is likely tpm_regs.

What is more surprising is that the CI did not catch it?

static bool ot_spi_device_is_tpm_enabled(const OtSPIDeviceState *s)
{
    return (bool)FIELD_EX32(s->spi_regs[R_TPM_CFG], TPM_CFG, EN);
}

static bool ot_spi_device_is_tpm_mode_crb(const OtSPIDeviceState *s)
{
    return (bool)FIELD_EX32(s->spi_regs[R_TPM_CFG], TPM_CFG, TPM_MODE);
}

static bool ot_spi_device_tpm_disable_hw_regs(const OtSPIDeviceState *s)
{
    return (bool)FIELD_EX32(s->spi_regs[R_TPM_CFG], TPM_CFG, HW_REG_DIS);
}

There may be other locations where the wrong register bank is used, I have not dug further.

[AlexJones0]

I forget to check the different register groups when reviewing, sorry..

We unfortunately don't have too many tests (only 1?) in the Earlgrey regressions right now that are using SPI outside of bootstrapping (hence it not being caught). I think the major regression only occurs when we enter TPM mode because of ot_spi_device_is_tpm_enabled, which will happen when the UPLOAD_CMDFIFO_NOT_EMPTY interrupt is enabled with the current buggy behaviour. It is possible that none of the tests we do have enable this interrupt, and therefore bypass the issue.

[AlexJones0]

Grepping for all usages of spi_regs[...] these seem to be the only three occurrences, so it should be a simple fix.

[rivos-eblot]

I forget to check the different register groups when reviewing, sorry..

No worries, I did not do it either :)

We unfortunately don't have too many tests in the Earlgrey regressions right now that are using SPI (hence it not being caught).

Ok. One of ours triggered at startup: as soon as a regular flash packet has been received, the TPM code got activated, then the TPM FSM asserted with a wrong count:

scripts/opentitan/pyot.py -c pyot.hjson -vvv -d -R -F 'spidev*' -t 'ot_spi_dev*' -t='-ot_spi_device_buf*' -t='-ot_spi_device_io*' 
   DEBUG pyot.vcp.serial0               SPI-DEVICE-flash test
   DEBUG pyot.vcp.serial0               Wait to enable JEDEC
   ERROR pyot.qemu                      ERROR:../hw/opentitan/ot_spi_device.c:2127:ot_spi_device_tpm_addr_state: assertion failed: (size == 3u)
    INFO pyot.qemu                      Bail out! ERROR:../hw/opentitan/ot_spi_device.c:2127:ot_spi_device_tpm_addr_state: assertion failed: (size == 3u)
CRITICAL pyot                           Abnormal QEMU termination: -6 for "spidev-flash-test"

ot_spi_device_reset soc: enter
ot_spi_device_release soc
ot_spi_device_release soc
ot_spi_device_chr_handle_packet soc: 0x1 bytes, eot: 0, rx: msb, tx: msb, mode:ok
ot_spi_device_bus_change_state soc: @2040: SPI_BUS_FLASH [1]
ot_spi_device_flash_ignored_command soc: unmanaged cmd 0x9f
ot_spi_device_flash_unknown_command soc: 0x9f
ot_spi_device_flash_change_state soc: @1482: SPI_FLASH_IDLE[0] -> SPI_FLASH_ERROR[8]
ot_spi_device_bus_change_state soc: @1483: SPI_BUS_DISCARD [3]
ot_spi_device_bus_change_state soc: @2304: SPI_BUS_IDLE [0]
ot_spi_device_chr_handle_packet soc: 0x1 bytes, eot: 0, rx: msb, tx: msb, mode:ok
ot_spi_device_bus_change_state soc: @2040: SPI_BUS_FLASH [1]
ot_spi_device_bus_change_state soc: @2304: SPI_BUS_IDLE [0]
ot_spi_device_chr_handle_packet soc: 0x2 bytes, eot: 1, rx: msb, tx: msb, mode:ok
ot_spi_device_bus_change_state soc: @2040: SPI_BUS_FLASH [1]
ot_spi_device_release soc
ot_spi_device_bus_change_state soc: @877: SPI_BUS_IDLE [0]
ot_spi_device_flash_change_state soc: @922: SPI_FLASH_ERROR[8] -> SPI_FLASH_IDLE[0]
ot_spi_device_chr_handle_packet soc: 0x1 bytes, eot: 0, rx: msb, tx: msb, mode:ok
ot_spi_device_bus_change_state soc: @2034: SPI_BUS_TPM [2]
ot_spi_device_bus_change_state soc: @2304: SPI_BUS_IDLE [0]
ot_spi_device_chr_handle_packet soc: 0x1 bytes, eot: 0, rx: msb, tx: msb, mode:ok
ot_spi_device_bus_change_state soc: @2034: SPI_BUS_TPM [2]

[rivos-eblot]

I think I'll need to run the upcomfing spi_device changes with our tests as well.
Something missing is some kind of fuzzying on the SPI device char dev.