From 03c2660f7556957262c41032064716414572833c Mon Sep 17 00:00:00 2001
From: J-Alves <joao.alves@arm.com>
Date: Wed, 27 Nov 2024 15:57:08 +0000
Subject: [PATCH] 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 | 2 +-
tools/tlc/tests/test_transfer_list.py | 61 +++++++++++++++++++--------
tools/tlc/tlc/tl.py | 52 +++++++++++++----------
3 files changed, 75 insertions(+), 40 deletions(-)
diff --git a/tools/tlc/tests/conftest.py b/tools/tlc/tests/conftest.py
index 155746a8b..93e44a9f5 100644
--- a/tools/tlc/tests/conftest.py
+++ b/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
diff --git a/tools/tlc/tests/test_transfer_list.py b/tools/tlc/tests/test_transfer_list.py
index e00280b95..54d93ec94 100644
--- a/tools/tlc/tests/test_transfer_list.py
+++ b/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):
diff --git a/tools/tlc/tlc/tl.py b/tools/tlc/tlc/tl.py
index b7eb48615..326b8575f 100644
--- a/tools/tlc/tlc/tl.py
+++ b/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))