[Sival, spi_dev] Tpm mode

hw/top_earlgrey/dv/chip_sim_cfg.hjson

@@ -694,7 +694,7 @@
     {
       name: chip_sw_spi_device_tpm
       uvm_test_seq: chip_sw_spi_device_tpm_vseq
-      sw_images: ["//sw/device/tests/sim_dv:spi_device_tpm_tx_rx_test:1:new_rules"]
+      sw_images: ["//sw/device/tests:spi_device_tpm_tx_rx_test:1:new_rules"]
       en_run_modes: ["sw_test_mode_test_rom"]
     }
     {

hw/top_earlgrey/dv/env/seq_lib/chip_sw_spi_device_tpm_vseq.sv

@@ -49,7 +49,7 @@ class chip_sw_spi_device_tpm_vseq extends chip_sw_base_vseq;
     cfg.m_spi_host_agent_cfg.csid = 1;
 
     // enable spi agent interface to begin
-    `DV_WAIT(cfg.sw_logger_vif.printed_log == "Begin TPM Test",
+    `DV_WAIT(cfg.sw_logger_vif.printed_log == "SYNC: Begin TPM Test",
              "Timedout waiting for spi host c configuration.")
 
     for (int i = 0; i < 10; i++) begin
@@ -63,6 +63,8 @@ class chip_sw_spi_device_tpm_vseq extends chip_sw_base_vseq;
       end
       tpm_txn (.wr(1), .addr(addr), .data_q(data_q), .len(data_q.size()), .rdata_q(rdata_q));
 
+      `DV_WAIT(cfg.sw_logger_vif.printed_log == "SYNC: Waiting Read",
+             "Timedout waiting for read config.")
       // Read transaction
       tpm_txn (.wr(0), .addr(addr), .len(data_q.size()), .rdata_q(rdata_q));
       foreach (rdata_q[i]) begin

sw/device/lib/dif/dif_spi_device.c

@@ -1608,9 +1608,7 @@ dif_result_t dif_spi_device_tpm_write_data(dif_spi_device_handle_t *spi,
   uint8_t offset = length & 0x3;  // lower two bits of length
   uint32_t rdfifo_wdata;
 
-  if (result != kDifOk) {
-    return result;
-  }
+  DIF_RETURN_IF_ERROR(result);
 
   // TODO: Ensure the received length is greater than FIFO SIZE
   if (DIF_SPI_DEVICE_TPM_FIFO_DEPTH * sizeof(uint32_t) < length) {
@@ -1639,10 +1637,7 @@ dif_result_t dif_spi_device_tpm_read_data(dif_spi_device_handle_t *spi,
     return kDifBadArg;
   }
   dif_spi_device_tpm_data_status_t status;
-  dif_result_t result = dif_spi_device_tpm_get_data_status(spi, &status);
-  if (result != kDifOk) {
-    return result;
-  }
+  DIF_RETURN_IF_ERROR(dif_spi_device_tpm_get_data_status(spi, &status));
   if (status.write_fifo_occupancy < length) {
     return kDifOutOfRange;
   }

sw/device/tests/BUILD

@@ -2938,6 +2938,50 @@ opentitan_test(
     ],
 )
 
+opentitan_test(
+    name = "spi_device_tpm_tx_rx_test",
+    srcs = ["spi_device_tpm_tx_rx_test.c"],
+    cw310 = new_cw310_params(
+        # This test requires the spi full duplex on the hyperdebug board.
+        tags = [
+            "broken",
+            "manual",
+        ],
+        test_cmd = """
+            --bitstream="{bitstream}"
+            --bootstrap="{firmware}"
+            "{firmware:elf}"
+        """,
+        test_harness = "//sw/host/tests/chip/spi_device_tpm_test",
+    ),
+    exec_env = {
+        "//hw/top_earlgrey:fpga_cw310_sival": None,
+        "//hw/top_earlgrey:silicon_owner_sival_rom_ext": "silicon_owner",
+        "//hw/top_earlgrey:sim_dv": None,
+    },
+    silicon_owner = silicon_params(
+        test_cmd = """
+            --bootstrap="{firmware}"
+            "{firmware:elf}"
+        """,
+        test_harness = "//sw/host/tests/chip/spi_device_tpm_test",
+    ),
+    deps = [
+        "//hw/top_earlgrey/sw/autogen:top_earlgrey",
+        "//sw/device/lib/arch:device",
+        "//sw/device/lib/base:mmio",
+        "//sw/device/lib/dif:pinmux",
+        "//sw/device/lib/dif:rv_plic",
+        "//sw/device/lib/dif:spi_device",
+        "//sw/device/lib/runtime:hart",
+        "//sw/device/lib/runtime:irq",
+        "//sw/device/lib/runtime:log",
+        "//sw/device/lib/testing:isr_testutils",
+        "//sw/device/lib/testing/test_framework:ottf_main",
+        "//sw/device/lib/testing/test_framework:status",
+    ],
+)
+
 cc_library(
     name = "spi_host_flash_test_impl",
     srcs = ["spi_host_flash_test_impl.c"],

sw/device/tests/sim_dv/BUILD

@@ -671,26 +671,6 @@ opentitan_test(
     ],
 )
 
-opentitan_test(
-    name = "spi_device_tpm_tx_rx_test",
-    srcs = ["spi_device_tpm_tx_rx_test.c"],
-    exec_env = {"//hw/top_earlgrey:sim_dv": None},
-    deps = [
-        "//hw/top_earlgrey/sw/autogen:top_earlgrey",
-        "//sw/device/lib/arch:device",
-        "//sw/device/lib/base:mmio",
-        "//sw/device/lib/dif:pinmux",
-        "//sw/device/lib/dif:rv_plic",
-        "//sw/device/lib/dif:spi_device",
-        "//sw/device/lib/runtime:hart",
-        "//sw/device/lib/runtime:irq",
-        "//sw/device/lib/runtime:log",
-        "//sw/device/lib/testing:isr_testutils",
-        "//sw/device/lib/testing/test_framework:ottf_main",
-        "//sw/device/lib/testing/test_framework:status",
-    ],
-)
-
 opentitan_test(
     name = "spi_host_tx_rx_test",
     srcs = ["spi_host_tx_rx_test.c"],

sw/device/tests/sival/BUILD

@@ -139,6 +139,7 @@ test_suite(
         "//sw/device/tests:sensor_ctrl_alert_test",
         "//sw/device/tests:sensor_ctrl_wakeup_test",
         "//sw/device/tests:sleep_pwm_pulses_test",
+        "//sw/device/tests:spi_device_tpm_tx_rx_test",
         "//sw/device/tests:sram_ctrl_memset_test",
         "//sw/device/tests:sram_ctrl_sleep_sram_ret_contents_no_scramble_test",
         "//sw/device/tests:sram_ctrl_subword_access_test",

sw/device/tests/sim_dv/spi_device_tpm_tx_rx_test.c → sw/device/tests/spi_device_tpm_tx_rx_test.c

@@ -28,13 +28,16 @@ static dif_rv_plic_t plic;
 typedef enum {
   kTpmWriteCommand = 0x0,
   kTpmReadCommand = 0x80,
+  kTpmCommandMask = 0xbf
 } tpm_cmd_t;
 
-const static uint8_t kIterations = 10;
-const static uint8_t kTpmCommandRwMask = 0x80;
-const static uint8_t kTpmCommandSizeMask = 0x3f;
+enum {
+  kIterations = 10,
+  kTpmCommandRwMask = 0x80,
+  kTpmCommandSizeMask = 0x3f,
+};
 
-const static dif_spi_device_tpm_config_t tpm_config = {
+const static dif_spi_device_tpm_config_t kTpmConfig = {
     .interface = kDifSpiDeviceTpmInterfaceCrb,
     .disable_return_by_hardware = false,
     .disable_address_prefix_check = false,
@@ -45,7 +48,7 @@ static volatile bool header_interrupt_received = false;
 static void en_plic_irqs(dif_rv_plic_t *plic) {
   // Enable functional interrupts as well as error interrupts to make sure
   // everything is behaving as expected.
-  top_earlgrey_plic_irq_id_t plic_irqs[] = {
+  const top_earlgrey_plic_irq_id_t kIrqs[] = {
       kTopEarlgreyPlicIrqIdSpiDeviceGenericRxFull,
       kTopEarlgreyPlicIrqIdSpiDeviceGenericRxWatermark,
       kTopEarlgreyPlicIrqIdSpiDeviceGenericTxWatermark,
@@ -59,13 +62,13 @@ static void en_plic_irqs(dif_rv_plic_t *plic) {
       kTopEarlgreyPlicIrqIdSpiDeviceReadbufFlip,
       kTopEarlgreyPlicIrqIdSpiDeviceTpmHeaderNotEmpty};
 
-  for (uint32_t i = 0; i < ARRAYSIZE(plic_irqs); ++i) {
+  for (uint32_t i = 0; i < ARRAYSIZE(kIrqs); ++i) {
     CHECK_DIF_OK(dif_rv_plic_irq_set_enabled(
-        plic, plic_irqs[i], kTopEarlgreyPlicTargetIbex0, kDifToggleEnabled));
+        plic, kIrqs[i], kTopEarlgreyPlicTargetIbex0, kDifToggleEnabled));
 
     // Assign a default priority
-    CHECK_DIF_OK(dif_rv_plic_irq_set_priority(plic, plic_irqs[i],
-                                              kDifRvPlicMaxPriority));
+    CHECK_DIF_OK(
+        dif_rv_plic_irq_set_priority(plic, kIrqs[i], kDifRvPlicMaxPriority));
   }
 
   // Enable the external IRQ at Ibex.
@@ -74,22 +77,21 @@ static void en_plic_irqs(dif_rv_plic_t *plic) {
 }
 
 static void en_spi_device_irqs(dif_spi_device_t *spi_device) {
-  dif_spi_device_irq_t spi_device_irqs[] = {
-      kDifSpiDeviceIrqGenericRxFull,
-      kDifSpiDeviceIrqGenericRxWatermark,
-      kDifSpiDeviceIrqGenericTxWatermark,
-      kDifSpiDeviceIrqGenericRxError,
-      kDifSpiDeviceIrqGenericRxOverflow,
-      kDifSpiDeviceIrqGenericTxUnderflow,
-      kDifSpiDeviceIrqUploadCmdfifoNotEmpty,
-      kDifSpiDeviceIrqUploadPayloadNotEmpty,
-      kDifSpiDeviceIrqUploadPayloadOverflow,
-      kDifSpiDeviceIrqReadbufWatermark,
-      kDifSpiDeviceIrqReadbufFlip,
-      kDifSpiDeviceIrqTpmHeaderNotEmpty};
-
-  for (uint32_t i = 0; i <= ARRAYSIZE(spi_device_irqs); ++i) {
-    CHECK_DIF_OK(dif_spi_device_irq_set_enabled(spi_device, spi_device_irqs[i],
+  const dif_spi_device_irq_t kIrqs[] = {kDifSpiDeviceIrqGenericRxFull,
+                                        kDifSpiDeviceIrqGenericRxWatermark,
+                                        kDifSpiDeviceIrqGenericTxWatermark,
+                                        kDifSpiDeviceIrqGenericRxError,
+                                        kDifSpiDeviceIrqGenericRxOverflow,
+                                        kDifSpiDeviceIrqGenericTxUnderflow,
+                                        kDifSpiDeviceIrqUploadCmdfifoNotEmpty,
+                                        kDifSpiDeviceIrqUploadPayloadNotEmpty,
+                                        kDifSpiDeviceIrqUploadPayloadOverflow,
+                                        kDifSpiDeviceIrqReadbufWatermark,
+                                        kDifSpiDeviceIrqReadbufFlip,
+                                        kDifSpiDeviceIrqTpmHeaderNotEmpty};
+
+  for (uint32_t i = 0; i <= ARRAYSIZE(kIrqs); ++i) {
+    CHECK_DIF_OK(dif_spi_device_irq_set_enabled(spi_device, kIrqs[i],
                                                 kDifToggleEnabled));
   }
 }
@@ -135,12 +137,16 @@ static void ack_spi_tpm_header_irq(dif_spi_device_handle_t *spi_device) {
 }
 
 // This routine is needed to make sure that an interrupt does not sneak in
-// and jump excution away between the boolean check and the actual invocation
+// and jump execution away between the boolean check and the actual invocation
 // of wait_for_interrupt.
 static void atomic_wait_for_interrupt(void) {
-  irq_global_ctrl(false);
-  if (!header_interrupt_received) {
+  while (true) {
+    irq_global_ctrl(false);
+    if (header_interrupt_received) {
+      break;
+    }
     wait_for_interrupt();
+    irq_global_ctrl(true);
   }
   irq_global_ctrl(true);
 }
@@ -162,37 +168,39 @@ bool test_main(void) {
       &pinmux, kTopEarlgreyPinmuxPeripheralInSpiDeviceTpmCsb,
       kTopEarlgreyPinmuxInselIoa7));
 
-  dif_pinmux_pad_attr_t out_attr;
-  dif_pinmux_pad_attr_t in_attr = {
-      .slew_rate = 0,
-      .drive_strength = 0,
-      .flags = kDifPinmuxPadAttrPullResistorEnable |
-               kDifPinmuxPadAttrPullResistorUp};
-
-  CHECK_DIF_OK(dif_pinmux_pad_write_attrs(&pinmux, kTopEarlgreyMuxedPadsIoa7,
-                                          kDifPinmuxPadKindMio, in_attr,
-                                          &out_attr));
+  if (kDeviceType == kDeviceSimDV) {
+    dif_pinmux_pad_attr_t out_attr;
+    dif_pinmux_pad_attr_t in_attr = {
+        .slew_rate = 0,
+        .drive_strength = 0,
+        .flags = kDifPinmuxPadAttrPullResistorEnable |
+                 kDifPinmuxPadAttrPullResistorUp};
+
+    CHECK_DIF_OK(dif_pinmux_pad_write_attrs(&pinmux, kTopEarlgreyMuxedPadsIoa7,
+                                            kDifPinmuxPadKindMio, in_attr,
+                                            &out_attr));
+  }
 
   CHECK_DIF_OK(
-      dif_spi_device_tpm_configure(&spi_device, kDifToggleEnabled, tpm_config));
+      dif_spi_device_tpm_configure(&spi_device, kDifToggleEnabled, kTpmConfig));
 
   // enable interrupts
   en_plic_irqs(&plic);
   en_spi_device_irqs(&spi_device.dev);
 
   // Sync message with testbench to begin.
-  LOG_INFO("Begin TPM Test");
+  LOG_INFO("SYNC: Begin TPM Test");
 
   for (uint32_t i = 0; i < kIterations; i++) {
     LOG_INFO("Iteration %d", i);
 
     // Wait for write interrupt.
     atomic_wait_for_interrupt();
 
-    // Check what comamnd we have received. Store it as expected variables
+    // Check what command we have received. Store it as expected variables
     // and compare when the read command is issued.
-    uint8_t write_command;
-    uint32_t write_addr;
+    uint8_t write_command = 0;
+    uint32_t write_addr = 0;
     CHECK_DIF_OK(dif_spi_device_tpm_get_command(&spi_device, &write_command,
                                                 &write_addr));
     CHECK((write_command & kTpmCommandRwMask) == kTpmWriteCommand,
@@ -202,7 +210,7 @@ bool test_main(void) {
     uint32_t num_bytes = (write_command & kTpmCommandSizeMask) + 1;
     LOG_INFO("Expecting %d bytes from tpm write", num_bytes);
 
-    uint8_t buf[64];
+    uint8_t buf[64] = {0};
     dif_result_t status = kDifOutOfRange;
     while (status == kDifOutOfRange) {
       status = dif_spi_device_tpm_read_data(&spi_device, num_bytes, buf);
@@ -212,18 +220,21 @@ bool test_main(void) {
     // Finished processing the write command
     ack_spi_tpm_header_irq(&spi_device);
 
+    LOG_INFO("SYNC: Waiting Read");
+    // Send the written data right back out for reads.
     // Wait for read interrupt.
     atomic_wait_for_interrupt();
     // Send the written data right back out for reads.
     CHECK_DIF_OK(dif_spi_device_tpm_write_data(&spi_device, num_bytes, buf));
 
-    uint8_t read_command;
-    uint32_t read_addr;
+    uint8_t read_command = 0;
+    uint32_t read_addr = 0;
     CHECK_DIF_OK(
         dif_spi_device_tpm_get_command(&spi_device, &read_command, &read_addr));
     ack_spi_tpm_header_irq(&spi_device);
 
     // Make sure the received command matches expectation
+    read_command &= kTpmCommandMask;
     LOG_INFO("Expected 0x%x, received 0x%x",
              (kTpmReadCommand | (num_bytes - 1)), read_command);
     CHECK((kTpmReadCommand | (num_bytes - 1)) == read_command,

sw/host/tests/chip/spi_device_tpm_test/BUILD

@@ -0,0 +1,24 @@
+# Copyright lowRISC contributors.
+# Licensed under the Apache License, Version 2.0, see LICENSE for details.
+# SPDX-License-Identifier: Apache-2.0
+
+load("@rules_rust//rust:defs.bzl", "rust_binary")
+
+package(default_visibility = ["//visibility:public"])
+
+rust_binary(
+    name = "spi_device_tpm_test",
+    srcs = [
+        "src/main.rs",
+    ],
+    deps = [
+        "//sw/host/opentitanlib",
+        "//third_party/rust/crates:anyhow",
+        "//third_party/rust/crates:clap",
+        "//third_party/rust/crates:humantime",
+        "//third_party/rust/crates:log",
+        "//third_party/rust/crates:rand",
+        "//third_party/rust/crates:regex",
+        "@crate_index//:object",
+    ],
+)