fix(tlc): add void entries to align data

Add void entries to ensure proper alignment of data in the TL,
addressing runtime errors caused by previously unaccounted padding bytes
between TE's.

Change-Id: Id2acee8f4df0dcc52eedc4372b962a51acb9d8ce
Signed-off-by: J-Alves <joao.alves@arm.com>
Co-authored-by:: Harrison Mutai <harrison.mutai@arm.com>

tools/tlc/tests/conftest.py

@@ -69,7 +69,7 @@ def tmpyamlconfig_blob_file(tmpdir, tmpfdt, non_empty_tag_id):
 def tlcrunner(tmptlstr):
     runner = CliRunner()
     with runner.isolated_filesystem():
-        runner.invoke(cli, ["create", tmptlstr])
+        runner.invoke(cli, ["create", "--size", 0x1F000, tmptlstr])
     return runner
 
 

tools/tlc/tests/test_transfer_list.py

@@ -9,6 +9,7 @@
 """Contains unit tests for the types TransferEntry and TransferList."""
 
 import math
+from random import randint
 
 import pytest
 
@@ -49,15 +50,39 @@ def test_make_transfer_list(size, csum):
         assert tl.checksum == csum
 
 
-@pytest.mark.parametrize(("tag_id", "data"), test_entries)
-def test_add_transfer_entry(tag_id, data):
+def test_add_transfer_entry(random_entries):
     tl = TransferList(0x1000)
-    te = TransferEntry(tag_id, len(data), data)
-
-    tl.add_transfer_entry(tag_id, data)
 
+    # Add a single entry and check it's in the list of entries
+    te = tl.add_transfer_entry(1, bytes(100))
     assert te in tl.entries
-    assert tl.size == TransferList.hdr_size + te.size
+    assert tl.size % 8 == 0
+
+    # Add a range of tag id's
+    for id, data in random_entries(50, 1):
+        te = tl.add_transfer_entry(id, data)
+        assert te in tl.entries
+        assert tl.size % 8 == 0
+
+
+@pytest.mark.parametrize("align", [4, 6, 12, 13])
+def test_add_transfer_entry_with_align(align, random_entries, random_entry):
+    tl = TransferList(0xF00000)
+    id, data = random_entry(4)
+
+    tl.add_transfer_entry(id, data)
+
+    # Add an entry with a larger alignment requirement
+    _, data = random_entry(4)
+    te = tl.add_transfer_entry(1, data, data_align=align)
+    assert (te.offset + te.hdr_size) % (1 << align) == 0
+    assert tl.alignment == align
+
+    # Add some more entries and ensure the alignment is preserved
+    for id, data in random_entries(5, 0x200):
+        te = tl.add_transfer_entry(id, data, data_align=align)
+        assert (te.offset + te.hdr_size) % (1 << align) == 0
+        assert tl.alignment == align
 
 
 @pytest.mark.parametrize(
@@ -155,6 +180,10 @@ def test_write_multiple_tes_to_file(tmpdir, random_entries):
             data_size = int.from_bytes(f.read(4), "little")
             assert f.read(data_size) == data
 
+        # padding is added to align TE's, make sure padding is added to the size of
+        # the TL by checking we don't overflow.
+        assert f.tell() <= tl.size
+
 
 def test_read_empty_transfer_list_from_file(tmpdir):
     test_file = tmpdir.join("test_tl_blob.bin")
@@ -204,19 +233,17 @@ def test_read_multiple_transfer_list_from_file(tmpdir):
     assert tl.sum_of_bytes() == 0
 
 
-@pytest.mark.parametrize("tag", [tag for tag, _ in test_entries])
-def test_remove_tag_from_file(tag):
-    tl = TransferList(0x1000)
+def test_remove_tag(random_entry):
+    """Adds a transfer entry and remove it, size == transfer list header."""
+    tl = TransferList(0x100)
+    id, data = random_entry(tl.total_size // 2)
 
-    for tag_id, data in test_entries:
-        tl.add_transfer_entry(tag_id, data)
+    te = tl.add_transfer_entry(id, data)
+    assert te in tl.entries
 
-    removed_entries = list(filter(lambda te: te.id == tag, tl.entries))
-    original_size = tl.size
-    tl.remove_tag(tag)
-
-    assert not any(tag == te.id for te in tl.entries)
-    assert tl.size == original_size - sum(map(lambda te: te.size, removed_entries))
+    tl.remove_tag(id)
+    assert not tl.get_entry(id) and te not in tl.entries
+    assert tl.size == tl.hdr_size
 
 
 def test_get_fdt_offset(tmpdir):

tools/tlc/tlc/tl.py

@@ -83,6 +83,7 @@ class TransferList:
     hdr_size = 0x18
     signature = 0x4A0FB10B
     version = 1
+    granule = 8
 
     def __init__(
         self, max_size: int = hdr_size, flags: int = TRANSFER_LIST_ENABLE_CHECKSUM
@@ -138,16 +139,15 @@ class TransferList:
                     # the 3-byte wide ID as a 4-byte uint, shift out this padding
                     # once we have the id.
                     te_base = f.tell()
-                    (id, hdr_size, data_size) = struct.unpack(
+                    (id, _, data_size) = struct.unpack(
                         TransferEntry.encoding[0] + "I" + TransferEntry.encoding[1:],
                         b"\x00" + f.read(TransferEntry.hdr_size),
                     )
 
                     id >>= 8
-
                     te = tl.add_transfer_entry(id, f.read(data_size))
                     te.offset = te_base
-                    f.seek(align(te_base + hdr_size + data_size, 2**tl.alignment))
+                    f.seek(align(f.tell(), tl.granule))
 
         return tl
 
@@ -215,18 +215,35 @@ class TransferList:
 
         return te.offset + te.hdr_size
 
-    def add_transfer_entry(self, tag_id: int, data: bytes) -> TransferEntry:
+    def add_transfer_entry(
+        self, tag_id: int, data: bytes, data_align: int = 0
+    ) -> TransferEntry:
         """Appends a TransferEntry into the internal list of TE's."""
+        data_offset = TransferEntry.hdr_size + self.size
+        data_align = self.alignment if not data_align else data_align
+
+        aligned_data_offset = align(data_offset, 1 << data_align)
+
+        if tag_id != 0 and data_offset != aligned_data_offset:
+            void_len = aligned_data_offset - data_offset - TransferEntry.hdr_size
+            self.add_transfer_entry(0, bytes(void_len))
+
+        assert align(self.size, self.granule)
+
         if not (self.total_size >= self.size + TransferEntry.hdr_size + len(data)):
             raise MemoryError(
                 f"TL size has exceeded the maximum allocation {self.total_size}."
             )
-        else:
-            te = TransferEntry(tag_id, len(data), data)
-            self.entries.append(te)
-            self.size += te.size
-            self.update_checksum()
-            return te
+
+        te = TransferEntry(tag_id, len(data), data, offset=self.size)
+        self.entries.append(te)
+
+        self.size += align(te.size, self.granule)
+        if data_align > self.alignment:
+            self.alignment = data_align
+
+        self.update_checksum()
+        return te
 
     def add_transfer_entry_from_struct_format(
         self, tag_id: int, struct_format: str, *args: Any
@@ -340,20 +357,11 @@ class TransferList:
             f.write(self.header_to_bytes())
             for te in self.entries:
                 assert f.tell() + te.hdr_size + te.data_size < self.total_size
-                te_base = f.tell()
+
                 f.write(te.header_to_bytes())
                 f.write(te.data)
-                # Ensure the next TE has the correct alignment
-                f.write(
-                    bytes(
-                        (
-                            align(
-                                te_base + te.hdr_size + te.data_size, 2**self.alignment
-                            )
-                            - f.tell()
-                        )
-                    )
-                )
+                # Ensure the next TE is at an 8-byte aligned address
+                f.write(bytes((align(f.tell(), self.granule) - f.tell())))
 
     def remove_tag(self, tag: int) -> None:
         self.entries = list(filter(lambda te: te.id != tag, self.entries))