Repository: chipsalliance/caliptra-sw
Pull request #1063 (2023-11-10 22:26:31)

Fix flaky test_mailbox_soc_to_uc() on fpga_realtime model
With fpga_realtime model, there seems to be a delay between when caliptra writes a uart character and when it becomes visible to the application CPU in the test-bench's FIFO. Tests that are asserting output should wait for the data to be available rather than assume it is already there.


Patch:
diff --git a/hw-model/src/lib.rs b/hw-model/src/lib.rs
index 27b3133f..3371e3dc 100644
--- a/hw-model/src/lib.rs
+++ b/hw-model/src/lib.rs
@@ -560,7 +560,7 @@ pub trait HwModel {
         }
     }
 
-    /// Execute until the output ends with `expected_output`
+    /// Execute until the output buffer starts with `expected_output`
     fn step_until_output(&mut self, expected_output: &str) -> Result<(), Box<dyn Error>> {
         self.step_until(|m| m.output().peek().len() >= expected_output.len());
         if &self.output().peek()[..expected_output.len()] != expected_output {
@@ -574,6 +574,14 @@ pub trait HwModel {
         Ok(())
     }
 
+    /// Execute until the output buffer starts with `expected_output`, and remove it
+    /// from the output buffer.
+    fn step_until_output_and_take(&mut self, expected_output: &str) -> Result<(), Box<dyn Error>> {
+        self.step_until_output(expected_output)?;
+        self.output().take(expected_output.len());
+        Ok(())
+    }
+
     // Execute (at least) until the output provided substr is written to the
     // output. Additional data may be present in the output after the provided
     // substr, which often happens with the fpga_realtime hardware model.


Test patch:
diff --git a/drivers/tests/drivers_integration_tests/main.rs b/drivers/tests/drivers_integration_tests/main.rs
index ae79d9c7..bff35a7a 100644
--- a/drivers/tests/drivers_integration_tests/main.rs
+++ b/drivers/tests/drivers_integration_tests/main.rs
@@ -362,13 +362,12 @@ fn test_mailbox_soc_to_uc() {
         // successful before the firmware as a chance to look at the buffer (!?),
         // so give the firmware a chance to print it out.
         model
-            .step_until_output(
+            .step_until_output_and_take(
                 "cmd: 0x50000000\n\
                  dlen: 8\n\
                  buf: [67452301, efcdab89, 00000000, 00000000]\n",
             )
             .unwrap();
-        model.output().take(usize::MAX);
         assert_eq!(resp, None);
 
         // Try again, but with a non-multiple-of-4 size
@@ -376,25 +375,23 @@ fn test_mailbox_soc_to_uc() {
             .mailbox_execute(0x5000_0000, &[0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd])
             .unwrap();
         model
-            .step_until_output(
+            .step_until_output_and_take(
                 "cmd: 0x50000000\n\
                  dlen: 7\n\
                  buf: [67452301, 00cdab89, 00000000, 00000000]\n",
             )
             .unwrap();
-        model.output().take(usize::MAX);
         assert_eq!(resp, None);
 
         // Try again, but with no data in the FIFO
         let resp = model.mailbox_execute(0x5000_0000, &[]).unwrap();
         model
-            .step_until_output(
+            .step_until_output_and_take(
                 "cmd: 0x50000000\n\
                  dlen: 0\n\
                  buf: [00000000, 00000000, 00000000, 00000000]\n",
             )
             .unwrap();
-        model.output().take(usize::MAX);
         assert_eq!(resp, None);
 
         // Try again, but with a non-multiple-of-4 dest buffer (0x5000_0001)
@@ -402,13 +399,12 @@ fn test_mailbox_soc_to_uc() {
             .mailbox_execute(0x5000_0001, &[0x01, 0x23, 0x45, 0x67, 0x89])
             .unwrap();
         model
-            .step_until_output(
+            .step_until_output_and_take(
                 "cmd: 0x50000001\n\
                  dlen: 5\n\
                  buf: [01, 23, 45, 67, 89]\n",
             )
             .unwrap();
-        model.output().take(usize::MAX);
         assert_eq!(resp, None);
 
         // Try again, but with one more byte than will fit in the dest buffer
@@ -416,13 +412,12 @@ fn test_mailbox_soc_to_uc() {
             .mailbox_execute(0x5000_0001, &[0x01, 0x23, 0x45, 0x67, 0x89, 0xab])
             .unwrap();
         model
-            .step_until_output(
+            .step_until_output_and_take(
                 "cmd: 0x50000001\n\
                  dlen: 6\n\
                  buf: [01, 23, 45, 67, 89]\n",
             )
             .unwrap();
-        model.output().take(usize::MAX);
         assert_eq!(resp, None);
 
         // Try again, but with 4 more bytes than will fit in the dest buffer
@@ -432,14 +427,14 @@ fn test_mailbox_soc_to_uc() {
                 &[0x01, 0x23, 0x45, 0x67, 0x89, 0xab, 0xcd, 0xef, 0x11],
             )
             .unwrap();
+
         model
-            .step_until_output(
+            .step_until_output_and_take(
                 "cmd: 0x50000001\n\
                  dlen: 9\n\
                  buf: [01, 23, 45, 67, 89]\n",
             )
             .unwrap();
-        model.output().take(usize::MAX);
         assert_eq!(resp, None);
     }
 
@@ -454,13 +449,14 @@ fn test_mailbox_soc_to_uc() {
                 ],
             )
             .unwrap();
-        assert_eq!(
-            model.output().take(usize::MAX),
-            "cmd: 0x60000000\n\
-             dlen: 16\n\
-             buf: [67452301, efcdab89]\n\
-             buf: [33221100, 77665544]\n"
-        );
+        model
+            .step_until_output_and_take(
+                "cmd: 0x60000000\n\
+                 dlen: 16\n\
+                 buf: [67452301, efcdab89]\n\
+                 buf: [33221100, 77665544]\n",
+            )
+            .unwrap();
         assert_eq!(resp, None);
 
         // Try again, but with a non-multiple-of-4 size
@@ -472,13 +468,14 @@ fn test_mailbox_soc_to_uc() {
                 ],
             )
             .unwrap();
-        assert_eq!(
-            model.output().take(usize::MAX),
-            "cmd: 0x60000000\n\
-             dlen: 13\n\
-             buf: [67452301, efcdab89]\n\
-             buf: [33221100, 00000044]\n"
-        );
+        model
+            .step_until_output_and_take(
+                "cmd: 0x60000000\n\
+                 dlen: 13\n\
+                 buf: [67452301, efcdab89]\n\
+                 buf: [33221100, 00000044]\n",
+            )
+            .unwrap();
         assert_eq!(resp, None);
 
         // Try again, but where the buffer is larger than the last chunk
@@ -490,22 +487,25 @@ fn test_mailbox_soc_to_uc() {
                 ],
             )
             .unwrap();
-        assert_eq!(
-            model.output().take(usize::MAX),
-            "cmd: 0x60000000\n\
-             dlen: 12\n\
-             buf: [67452301, efcdab89]\n\
-             buf: [33221100, 00000000]\n"
-        );
+        model
+            .step_until_output_and_take(
+                "cmd: 0x60000000\n\
+                 dlen: 12\n\
+                 buf: [67452301, efcdab89]\n\
+                 buf: [33221100, 00000000]\n",
+            )
+            .unwrap();
         assert_eq!(resp, None);
 
         // Try again, but with no data in the FIFO
         let resp = model.mailbox_execute(0x6000_0000, &[]).unwrap();
-        assert_eq!(
-            model.output().take(usize::MAX),
-            "cmd: 0x60000000\n\
-             dlen: 0\n"
-        );
+
+        model
+            .step_until_output_and_take(
+                "cmd: 0x60000000\n\
+                 dlen: 0\n",
+            )
+            .unwrap();
         assert_eq!(resp, None);
     }
 
@@ -514,19 +514,24 @@ fn test_mailbox_soc_to_uc() {
         let resp = model
             .mailbox_execute(0x7000_0000, &[0x88, 0x99, 0xaa, 0xbb])
             .unwrap();
-        assert_eq!(model.output().take(usize::MAX), "cmd: 0x70000000\n");
+
+        model
+            .step_until_output_and_take("cmd: 0x70000000\n")
+            .unwrap();
         assert_eq!(resp, None);
 
         // Make sure the next command doesn't see the FIFO from the previous command
         let resp = model
             .mailbox_execute(0x6000_0000, &[0x07, 0x06, 0x05, 0x04, 0x03])
             .unwrap();
-        assert_eq!(
-            model.output().take(usize::MAX),
-            "cmd: 0x60000000\n\
-             dlen: 5\n\
-             buf: [04050607, 00000003]\n"
-        );
+
+        model
+            .step_until_output_and_take(
+                "cmd: 0x60000000\n\
+                 dlen: 5\n\
+                 buf: [04050607, 00000003]\n",
+            )
+            .unwrap();
         assert_eq!(resp, None);
     }
 
@@ -536,18 +541,22 @@ fn test_mailbox_soc_to_uc() {
             model.mailbox_execute(0x8000_0000, &[0x88, 0x99, 0xaa, 0xbb]),
             Err(ModelError::MailboxCmdFailed(0))
         );
-        assert_eq!(model.output().take(usize::MAX), "cmd: 0x80000000\n");
+
+        model
+            .step_until_output_and_take("cmd: 0x80000000\n")
+            .unwrap();
 
         // Make sure the next command doesn't see the FIFO from the previous command
         let resp = model
             .mailbox_execute(0x6000_0000, &[0x07, 0x06, 0x05, 0x04, 0x03])
             .unwrap();
-        assert_eq!(
-            model.output().take(usize::MAX),
-            "cmd: 0x60000000\n\
-             dlen: 5\n\
-             buf: [04050607, 00000003]\n"
-        );
+        model
+            .step_until_output_and_take(
+                "cmd: 0x60000000\n\
+                 dlen: 5\n\
+                 buf: [04050607, 00000003]\n",
+            )
+            .unwrap();
         assert_eq!(resp, None);
     }
 
@@ -559,19 +568,22 @@ fn test_mailbox_soc_to_uc() {
                 &[0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08],
             )
             .unwrap();
-        assert_eq!(
-            model.output().take(usize::MAX),
-            "cmd: 0x90000000\n\
-             dlen: 8\n\
-             buf: [08070605]\n"
-        );
+        model
+            .step_until_output_and_take(
+                "cmd: 0x90000000\n\
+                 dlen: 8\n\
+                 buf: [08070605]\n",
+            )
+            .unwrap();
         assert_eq!(resp, None);
     }
 
     // Test 4 byte response with no request data
     {
         let resp = model.mailbox_execute(0xA000_0000, &[]).unwrap().unwrap();
-        assert_eq!(model.output().take(usize::MAX), "cmd: 0xa0000000\n");
+        model
+            .step_until_output_and_take("cmd: 0xa0000000\n")
+            .unwrap();
         assert_eq!(resp, [0x12, 0x34, 0x56, 0x78]);
     }
 
@@ -581,28 +593,38 @@ fn test_mailbox_soc_to_uc() {
             .mailbox_execute(0xB000_0000, &[0x0f, 0x0e, 0x0d, 0x0c, 0x0b, 0xa])
             .unwrap()
             .unwrap();
-        assert_eq!(
-            model.output().take(usize::MAX),
-            "cmd: 0xb0000000\n\
-             dlen: 6\n\
-             buf: [0c0d0e0f, 00000a0b]\n"
-        );
+        model
+            .step_until_output_and_take(
+                "cmd: 0xb0000000\n\
+                 dlen: 6\n\
+                 buf: [0c0d0e0f, 00000a0b]\n",
+            )
+            .unwrap();
         assert_eq!(resp, [0x98, 0x76]);
     }
 
     // Test 9 byte reponse
     {
         let resp = model.mailbox_execute(0xC000_0000, &[]).unwrap().unwrap();
-        assert_eq!(model.output().take(usize::MAX), "cmd: 0xc0000000\n");
+        model
+            .step_until_output_and_take("cmd: 0xc0000000\n")
+            .unwrap();
         assert_eq!(resp, [0x0A, 0x0B, 0x0C, 0x0D, 0x05, 0x04, 0x03, 0x02, 0x01]);
     }
 
     // Test reponse with 0 bytes (still calls copy_response)
     {
         let resp = model.mailbox_execute(0xD000_0000, &[]).unwrap().unwrap();
-        assert_eq!(model.output().take(usize::MAX), "cmd: 0xd0000000\n");
+        model
+            .step_until_output_and_take("cmd: 0xd0000000\n")
+            .unwrap();
         assert_eq!(resp, []);
     }
+    // Ensure there isn't any unexpected output
+    for _i in 0..100000 {
+        model.step();
+    }
+    assert_eq!(model.output().take(usize::MAX), "");
 }
 
 #[test]