hash_tester should read the correct number of response bytes

Both versions of hash_tester optimistically assumed a READ_OK or
WRITE_OK response, and tried to read the expected number of bytes.
But ERROR and UNKNOWN responses are shorter, which could lead to an
overrun of available data from i2c. With commit a768072, this is no
longer fatal (i2c will zero-pad the response), but it's good form to do
the right thing when possible.

2 files changed, 114 insertions, 51 deletions

diff --git a/src/sw/hash_tester.c b/src/sw/hash_tester.c
index d02db41..f202a03 100644
--- a/src/sw/hash_tester.c
+++ b/src/sw/hash_tester.c
@@ -63,15 +63,16 @@
 #define EOC       0xaa
 #define READ_CMD  0x10
 #define WRITE_CMD 0x11
+#define RESET_CMD 0x01
 
 /* response codes */
 #define SOR       0xaa
 #define EOR       0x55
-#define UNKNOWN   0xfe
-#define ERROR     0xfd
 #define READ_OK   0x7f
 #define WRITE_OK  0x7e
 #define RESET_OK  0x7d
+#define UNKNOWN   0xfe
+#define ERROR     0xfd
 
 /* addresses and codes common to all hash cores */
 #define ADDR_NAME0              0x00
@@ -253,26 +254,16 @@ int i2c_write(uint8_t *buf, int len)
     return 0;
 }
 
-int i2c_read(uint8_t *buf, int len)
+int i2c_read(uint8_t *b)
 {
-    int i;
-
-    if (debug)
-	printf("read  [");
-
-    for (i = 0; i < len; ++i) {
-	if (read(i2cfd, &buf[i], 1) != 1) {
-	    fprintf(stderr, "i2c read failed on byte %d: ", i);
-	    perror("");
-	    return 1;
-	}
-	if (debug)
-	    printf(" %02x", buf[i]);
+    /* read() on the i2c device only returns one byte at a time,
+     * and tc_get_resp() needs to parse the response one byte at a time
+     */
+    if (read(i2cfd, b, 1) != 1) {
+	perror("i2c read failed");
+	return 1;
     }
 
-    if (debug)
-	printf(" ]\n");
-
     return 0;
 }
 
@@ -307,12 +298,58 @@ int tc_send_read_cmd(uint8_t addr0, uint8_t addr1)
     return i2c_write(buf, sizeof(buf));
 }
 
-int tc_get_resp(uint8_t *expected, int len)
+int tc_get_resp(uint8_t *buf, int len)
+{
+    int i;
+
+    for (i = 0; i < len; ++i) {
+	if (i2c_read(&buf[i]) != 0)
+	    return 1;
+	if ((i == 0) && (buf[i] != SOR)) {
+	    /* we've gotten out of sync, and there's probably nothing we can do */
+	    fprintf(stderr, "response byte 0: expected 0x%02x (SOR), got 0x%02x\n",
+		    SOR, buf[0]);
+	    return 1;
+	}
+	else if (i == 1) {	/* response code */
+	    switch (buf[i]) {
+	    case READ_OK:
+		len = 9;
+		break;
+	    case WRITE_OK:
+		len = 5;
+		break;
+	    case RESET_OK:
+		len = 3;
+		break;
+	    case ERROR:
+	    case UNKNOWN:
+		len = 4;
+		break;
+	    default:
+		/* we've gotten out of sync, and there's probably nothing we can do */
+		fprintf(stderr, "unknown response code 0x%02x\n", buf[i]);
+		return 1;
+	    }
+	}
+    }
+
+    if (debug) {
+	printf("read  [");
+	for (i = 0; i < len; ++i)
+	    printf(" %02x", buf[i]);
+	printf(" ]\n");
+    }
+
+    return 0;
+}
+
+int tc_get_expected(uint8_t *expected, int len)
 {
     uint8_t buf[9];
     int i;
 
-    if (i2c_read(buf, len) != 0)
+    if (tc_get_resp(buf, sizeof(buf)) != 0)
 	return 1;
 
     for (i = 0; i < len; ++i) {
@@ -336,7 +373,7 @@ int tc_get_write_resp(uint8_t addr0, uint8_t addr1)
     expected[3] = addr1;
     expected[4] = EOR;
 
-    return tc_get_resp(expected, sizeof(expected));
+    return tc_get_expected(expected, sizeof(expected));
 }
 
 int tc_get_read_resp(uint8_t addr0, uint8_t addr1, uint32_t data)
@@ -353,7 +390,7 @@ int tc_get_read_resp(uint8_t addr0, uint8_t addr1, uint32_t data)
     expected[7] = data & 0xff;
     expected[8] = EOR;
 
-    return tc_get_resp(expected, sizeof(expected));
+    return tc_get_expected(expected, sizeof(expected));
 }
 
 int tc_write(uint8_t addr0, uint8_t addr1, uint32_t data)
@@ -385,7 +422,9 @@ int tc_wait(uint8_t addr0, uint8_t status)
     do {
 	if (tc_send_read_cmd(addr0, ADDR_STATUS) != 0)
 	    return 1;
-	if (i2c_read(buf, 9) != 0)
+	if (tc_get_resp(buf, 9) != 0)
+	    return 1;
+	if (buf[1] != READ_OK)
 	    return 1;
     } while ((buf[7] & status) != status);
 
@@ -869,7 +908,7 @@ int main(int argc, char *argv[])
     int addr = I2C_addr;
     int i, j, opt;
 
-    while ((opt = getopt(argc, argv, "di:a:")) != -1) {
+    while ((opt = getopt(argc, argv, "h?di:a:")) != -1) {
 	switch (opt) {
 	case 'h':
 	case '?':
diff --git a/src/sw/hash_tester.py b/src/sw/hash_tester.py
index e2b3777..6a5ff0b 100755
--- a/src/sw/hash_tester.py
+++ b/src/sw/hash_tester.py
@@ -158,9 +158,6 @@ SHA512_DOUBLE_DIGEST     = [ 0x8e959b75, 0xdae313da, 0x8cf4f728, 0x14fc143f,
                              0x501d289e, 0x4900f7e4, 0x331b99de, 0xc4b5433a,
                              0xc7d329ee, 0xb6dd2654, 0x5e96e55b, 0x874be909 ]
 
-def hexlist(list):
-    return "[ " + ' '.join('%02x' % b for b in list) + " ]"
-
 #----------------------------------------------------------------
 # I2C class
 #----------------------------------------------------------------
@@ -172,6 +169,9 @@ I2C_addr = 0x0f
 # from /usr/include/linux/i2c-dev.h
 I2C_SLAVE = 0x0703
 
+def hexlist(list):
+    return "[ " + ' '.join('%02x' % b for b in list) + " ]"
+
 class I2C:
     # file handle for the i2c device
     file = None
@@ -202,15 +202,11 @@ class I2C:
             print "write %s" % hexlist(buf)
         self.file.write(bytearray(buf))
 
-    # read 5 or 9 response bytes to a buffer
-    def read(self, len):
-        buf = []
-        # read() on the i2c device will only return 1 byte at a time
-        for i in range(len):
-            buf.append(ord(self.file.read(1)))
-        if DEBUG:
-            print "read  %s" % hexlist(buf)
-        return buf
+    # read one response byte from the i2c device
+    def read(self):
+        # read() on the i2c device will only return one byte at a time,
+        # and tc.get_resp() needs to parse the response one byte at a time
+        return ord(self.file.read(1))
 
 #----------------------------------------------------------------
 # test-case class
@@ -221,12 +217,16 @@ SOC                   = 0x55
 EOC                   = 0xaa
 READ_CMD              = 0x10
 WRITE_CMD             = 0x11
+RESET_CMD             = 0x01
 
 # response codes
 SOR                   = 0xaa
 EOR                   = 0x55
 READ_OK               = 0x7f
 WRITE_OK              = 0x7e
+RESET_OK              = 0x7d
+UNKNOWN               = 0xfe
+ERROR                 = 0xfd
 
 class TcError(Exception):
     pass
@@ -248,22 +248,46 @@ class tc:
         buf = [SOC, READ_CMD, self.addr0, self.addr1, EOC]
         self.i2c.write(buf)
 
-    def get_resp(self, expected):
-        buf = self.i2c.read(len(expected))
+    def get_resp(self):
+        buf = []
+        len = 2
+        i = 0
+        while i < len:
+            b = self.i2c.read()
+            if ((i == 0) and (b != SOR)):
+                # we've gotten out of sync, and there's probably nothing we can do
+                print "response byte 0: expected 0x%02x (SOR), got 0x%02x" % (SOR, b)
+                raise TcError()
+            elif (i == 1):        # response code
+                try:
+                    # anonymous dictionary of message lengths
+                    len = {READ_OK:9, WRITE_OK:5, RESET_OK:3, ERROR:4, UNKNOWN:4}[b]
+                except KeyError:  # unknown response code
+                    # we've gotten out of sync, and there's probably nothing we can do
+                    print "unknown response code 0x%02x" % b
+                    raise TcError()
+            buf.append(b)
+            i += 1
+        if DEBUG:
+            print "read  %s" % hexlist(buf)
+        return buf
+
+    def get_expected(self, expected):
+        buf = self.get_resp()
         if (buf != expected):
             print "expected %s,\nreceived %s" % (hexlist(expected), hexlist(buf))
             raise TcError()
 
     def get_write_resp(self):
         expected = [SOR, WRITE_OK, self.addr0, self.addr1, EOR]
-        self.get_resp(expected)
+        self.get_expected(expected)
 
     def get_read_resp(self, data):
         expected = [SOR, READ_OK, self.addr0, self.addr1]
         for s in (24, 16, 8, 0):
             expected.append(data >> s & 0xff)
         expected.append(EOR)
-        self.get_resp(expected)
+        self.get_expected(expected)
 
     def write(self, data):
         self.send_write_cmd(data)
@@ -289,7 +313,7 @@ def tc_wait(i2c, addr0, status):
     t = tc(i2c, addr0, ADDR_STATUS)
     while 1:
         t.send_read_cmd()
-        buf = t.i2c.read(9)
+        buf = t.get_resp()
         if ((buf[7] & status) == status):
             break
 
@@ -328,9 +352,9 @@ def sha1_wait_valid(i2c):
 def TC1(i2c):
     print "TC1: Reading name, type and version words from SHA-1 core."
 
-    sha1_read(i2c, ADDR_NAME0, 0x73686131)	# "sha1"
-    sha1_read(i2c, ADDR_NAME1, 0x20202020)	# "    "
-    sha1_read(i2c, ADDR_VERSION, 0x302e3530)	# "0.50"
+    sha1_read(i2c, ADDR_NAME0,   0x73686131)    # "sha1"
+    sha1_read(i2c, ADDR_NAME1,   0x20202020)    # "    "
+    sha1_read(i2c, ADDR_VERSION, 0x302e3530)    # "0.50"
 
 # TC2: SHA-1 Single block message test as specified by NIST.
 def TC2(i2c):
@@ -413,9 +437,9 @@ def sha256_wait_valid(i2c):
 def TC4(i2c):
     print "TC4: Reading name, type and version words from SHA-256 core."
 
-    sha256_read(i2c, ADDR_NAME0, 0x73686132)	# "sha2"
-    sha256_read(i2c, ADDR_NAME1, 0x2d323536)	# "-256"
-    sha256_read(i2c, ADDR_VERSION, 0x302e3830)	# "0.80"
+    sha256_read(i2c, ADDR_NAME0,   0x73686132)  # "sha2"
+    sha256_read(i2c, ADDR_NAME1,   0x2d323536)  # "-256"
+    sha256_read(i2c, ADDR_VERSION, 0x302e3830)  # "0.80"
 
 # TC5: SHA-256 Single block message test as specified by NIST.
 def TC5(i2c):
@@ -539,9 +563,9 @@ def sha512_wait_valid(i2c):
 def TC8(i2c):
     print "TC8: Reading name, type and version words from SHA-512 core."
 
-    sha512_read(i2c, ADDR_NAME0, 0x73686132)	# "sha2"
-    sha512_read(i2c, ADDR_NAME1, 0x2d353132)	# "-512"
-    sha512_read(i2c, ADDR_VERSION, 0x302e3830)	# "0.80"
+    sha512_read(i2c, ADDR_NAME0,   0x73686132)  # "sha2"
+    sha512_read(i2c, ADDR_NAME1,   0x2d353132)  # "-512"
+    sha512_read(i2c, ADDR_VERSION, 0x302e3830)  # "0.80"
 
 # TC9: SHA-512 Single block message test as specified by NIST.
 # We do this for all modes.