feat(manifest): check interrupt IDs are unique
Add a checks to prevent two endpoints from declaring the same
interrupt ID resource for a device peripheral.
Also add macros used to access the bitmask that tracks
interrupts.
Signed-off-by: Daniel Boulby <daniel.boulby@arm.com>
Change-Id: I5074e8d4209588a6c8f62ee06e0eb05b20cac387
diff --git a/src/api.c b/src/api.c
index ca39086..788db42 100644
--- a/src/api.c
+++ b/src/api.c
@@ -607,9 +607,8 @@
struct vcpu **next)
{
struct vcpu *target_vcpu = target_locked.vcpu;
- uint32_t intid_index = intid / INTERRUPT_REGISTER_BITS;
- uint32_t intid_shift = intid % INTERRUPT_REGISTER_BITS;
- uint32_t intid_mask = 1U << intid_shift;
+ uint32_t intid_index = INTID_INDEX(intid);
+ uint32_t intid_mask = INTID_MASK(1U, intid);
int64_t ret = 0;
/*
@@ -625,7 +624,7 @@
/* Increment the count. */
if ((target_vcpu->interrupts.interrupt_type[intid_index] &
- intid_mask) == (INTERRUPT_TYPE_IRQ << intid_shift)) {
+ intid_mask) == INTID_MASK(INTERRUPT_TYPE_IRQ, intid)) {
vcpu_irq_count_increment(target_locked);
} else {
vcpu_fiq_count_increment(target_locked);
@@ -1818,9 +1817,8 @@
enum interrupt_type type, struct vcpu *current)
{
struct vcpu_locked current_locked;
- uint32_t intid_index = intid / INTERRUPT_REGISTER_BITS;
- uint32_t intid_shift = intid % INTERRUPT_REGISTER_BITS;
- uint32_t intid_mask = 1U << intid_shift;
+ uint32_t intid_index = INTID_INDEX(intid);
+ uint32_t intid_mask = INTID_MASK(1U, intid);
if (intid >= HF_NUM_INTIDS) {
return -1;
@@ -1845,7 +1843,7 @@
intid_mask) {
if ((current->interrupts.interrupt_type[intid_index] &
intid_mask) ==
- (INTERRUPT_TYPE_IRQ << intid_shift)) {
+ INTID_MASK(INTERRUPT_TYPE_IRQ, intid)) {
vcpu_irq_count_increment(current_locked);
} else {
vcpu_fiq_count_increment(current_locked);
@@ -1862,7 +1860,7 @@
intid_mask) {
if ((current->interrupts.interrupt_type[intid_index] &
intid_mask) ==
- (INTERRUPT_TYPE_IRQ << intid_shift)) {
+ INTID_MASK(INTERRUPT_TYPE_IRQ, intid)) {
vcpu_irq_count_decrement(current_locked);
} else {
vcpu_fiq_count_decrement(current_locked);
diff --git a/src/init.c b/src/init.c
index 170c716..0a5e643 100644
--- a/src/init.c
+++ b/src/init.c
@@ -167,6 +167,9 @@
panic("Unable to load VMs.");
}
+ /* Now manifest parsing has completed free the resourses used. */
+ manifest_deinit(&ppool);
+
if (!boot_flow_update(mm_stage1_locked, &manifest, &update, &cpio,
&ppool)) {
panic("Unable to update boot flow.");
diff --git a/src/manifest.c b/src/manifest.c
index d8350fc..cd70e11 100644
--- a/src/manifest.c
+++ b/src/manifest.c
@@ -38,6 +38,42 @@
(HF_OTHER_WORLD_ID < HF_VM_ID_BASE),
"TrustZone VM ID clashes with normal VM range.");
+/**
+ * A struct to keep track of fields that are allocated by partitions
+ * in the manifest.
+ */
+struct allocated_fields {
+ uint32_t intids[HF_NUM_INTIDS / INTERRUPT_REGISTER_BITS];
+};
+
+/**
+ * Ensure the allocated_fields struct will fit in the entry allocated from
+ * the mpool.
+ */
+static_assert(sizeof(struct allocated_fields) < MM_PPOOL_ENTRY_SIZE,
+ "More space required for the allocated_fields struct.");
+static struct allocated_fields *allocated_fields;
+
+/**
+ * Allocates memory for the allocated fields struct in the given memory
+ * pool.
+ * Returns true if the memory is successfully allocated.
+ */
+static bool manifest_allocated_fields_init(struct mpool *ppool)
+{
+ allocated_fields = (struct allocated_fields *)mpool_alloc(ppool);
+ return allocated_fields != NULL;
+}
+
+/**
+ * Frees the memory used for the allocated field struct in the given
+ * memory pool.
+ */
+static void manifest_allocated_fields_deinit(struct mpool *ppool)
+{
+ mpool_free(ppool, allocated_fields);
+}
+
static inline size_t count_digits(ffa_vm_id_t vm_id)
{
size_t digits = 0;
@@ -450,6 +486,7 @@
struct uint32list_iter list;
uint16_t i = 0;
uint32_t j = 0;
+ uint32_t *allocated_intids = allocated_fields->intids;
dlog_verbose(" Partition Device Regions\n");
@@ -509,8 +546,25 @@
j = 0;
while (uint32list_has_next(&list) &&
j < PARTITION_MAX_INTERRUPTS_PER_DEVICE) {
+ uint32_t intid;
+ uint32_t intid_index;
+ uint32_t intid_mask;
+
TRY(uint32list_get_next(
&list, &dev_regions[i].interrupts[j].id));
+ intid = dev_regions[i].interrupts[j].id;
+ intid_index = INTID_INDEX(intid);
+ intid_mask = INTID_MASK(1U, intid);
+
+ dlog_verbose(" ID = %u\n", intid);
+
+ if ((allocated_intids[intid_index] & intid_mask) !=
+ 0U) {
+ return MANIFEST_ERROR_INTERRUPT_ID_REPEATED;
+ }
+
+ allocated_intids[intid_index] |= intid_mask;
+
if (uint32list_has_next(&list)) {
TRY(uint32list_get_next(&list,
&dev_regions[i]
@@ -520,8 +574,7 @@
return MANIFEST_ERROR_MALFORMED_INTEGER_LIST;
}
- dlog_verbose(" ID = %u, attributes = %u\n",
- dev_regions[i].interrupts[j].id,
+ dlog_verbose(" attributes = %u\n",
dev_regions[i].interrupts[j].attributes);
j++;
}
@@ -900,6 +953,12 @@
memset_s(manifest, sizeof(*manifest), 0, sizeof(*manifest));
+ /* Allocate space in the ppool for tracking the allocated fields. */
+ if (!manifest_allocated_fields_init(ppool)) {
+ panic("Unable to allocated space for allocated fields "
+ "struct.\n");
+ }
+
if (!fdt_init_from_memiter(&fdt, manifest_fdt)) {
return MANIFEST_ERROR_FILE_SIZE; /* TODO */
}
@@ -971,6 +1030,12 @@
return MANIFEST_SUCCESS;
}
+/* Free resources used when parsing the manifest. */
+void manifest_deinit(struct mpool *ppool)
+{
+ manifest_allocated_fields_deinit(ppool);
+}
+
const char *manifest_strerror(enum manifest_return_code ret_code)
{
switch (ret_code) {
@@ -1017,6 +1082,8 @@
return "Memory permission should be RO, RW or RX";
case MANIFEST_ERROR_ARGUMENTS_LIST_EMPTY:
return "Arguments-list node should have at least one argument";
+ case MANIFEST_ERROR_INTERRUPT_ID_REPEATED:
+ return "Interrupt ID already assigned to another endpoint";
}
panic("Unexpected manifest return code.");
diff --git a/src/manifest_test.cc b/src/manifest_test.cc
index beaf883..6d4fda8 100644
--- a/src/manifest_test.cc
+++ b/src/manifest_test.cc
@@ -27,6 +27,10 @@
using ::testing::IsEmpty;
using ::testing::NotNull;
+using struct_manifest = struct manifest;
+
+constexpr size_t TEST_HEAP_SIZE = PAGE_SIZE * 32;
+
template <typename T>
void exec(const char *program, const char *args[], const T &stdin,
std::vector<char> *stdout)
@@ -308,29 +312,102 @@
std::stringstream dts_;
};
-static enum manifest_return_code manifest_from_vec(struct manifest *m,
- const std::vector<char> &vec)
+class manifest : public ::testing::Test
{
- struct memiter it;
+ void SetUp() override
+ {
+ test_heap = std::make_unique<uint8_t[]>(TEST_HEAP_SIZE);
+ mpool_init(&ppool, MM_PPOOL_ENTRY_SIZE);
+ mpool_add_chunk(&ppool, test_heap.get(), TEST_HEAP_SIZE);
+ }
+
+ std::unique_ptr<uint8_t[]> test_heap;
+
+ protected:
struct mpool ppool;
- struct mm_stage1_locked mm_stage1_locked;
- memiter_init(&it, vec.data(), vec.size());
- return manifest_init(mm_stage1_locked, m, &it, &ppool);
-}
+ public:
+ /**
+ * Class for programatically building a Partition package.
+ */
+ class Partition_package
+ {
+ public:
+ __attribute__((aligned(PAGE_SIZE))) struct sp_pkg_header spkg;
+ __attribute__((
+ aligned(PAGE_SIZE))) char manifest_dtb[PAGE_SIZE] = {};
+ __attribute__((aligned(PAGE_SIZE))) char img[PAGE_SIZE] = {};
-TEST(manifest, no_hypervisor_node)
+ Partition_package(const std::vector<char> &vec)
+ {
+ // Initialise header field
+ spkg.magic = SP_PKG_HEADER_MAGIC;
+ spkg.version = SP_PKG_HEADER_VERSION_2;
+ spkg.pm_offset = PAGE_SIZE;
+ spkg.pm_size = vec.size();
+ spkg.img_offset = 2 * PAGE_SIZE;
+ spkg.img_size = ARRAY_SIZE(img);
+
+ // Copy dtb into package
+ std::copy(vec.begin(), vec.end(), manifest_dtb);
+ }
+ };
+
+ enum manifest_return_code manifest_from_vec(
+ struct_manifest *m, const std::vector<char> &vec)
+ {
+ struct memiter it;
+ struct mm_stage1_locked mm_stage1_locked;
+ enum manifest_return_code ret;
+
+ memiter_init(&it, vec.data(), vec.size());
+ ret = manifest_init(mm_stage1_locked, m, &it, &ppool);
+
+ manifest_deinit(&ppool);
+ return ret;
+ }
+
+ enum manifest_return_code ffa_manifest_from_vec(
+ struct_manifest *m, const std::vector<char> &vec)
+ {
+ struct memiter it;
+ struct mm_stage1_locked mm_stage1_locked;
+ enum manifest_return_code ret;
+
+ Partition_package spkg(vec);
+
+ /* clang-format off */
+ std::vector<char> core_dtb = ManifestDtBuilder()
+ .StartChild("hypervisor")
+ .Compatible()
+ .StartChild("vm1")
+ .DebugName("primary_vm")
+ .FfaPartition()
+ .LoadAddress((uint64_t)&spkg)
+ .EndChild()
+ .EndChild()
+ .Build();
+ /* clang-format on */
+ memiter_init(&it, core_dtb.data(), core_dtb.size());
+ ret = manifest_init(mm_stage1_locked, m, &it, &ppool);
+
+ manifest_deinit(&ppool);
+ return ret;
+ }
+};
+
+TEST_F(manifest, no_hypervisor_node)
{
- struct manifest m;
+ struct_manifest m;
std::vector<char> dtb = ManifestDtBuilder().Build();
ASSERT_EQ(manifest_from_vec(&m, dtb),
MANIFEST_ERROR_NO_HYPERVISOR_FDT_NODE);
}
-TEST(manifest, no_compatible_property)
+TEST_F(manifest, no_compatible_property)
{
- struct manifest m;
+ struct_manifest m;
/* clang-format off */
std::vector<char> dtb = ManifestDtBuilder()
@@ -342,9 +419,9 @@
ASSERT_EQ(manifest_from_vec(&m, dtb), MANIFEST_ERROR_NOT_COMPATIBLE);
}
-TEST(manifest, not_compatible)
+TEST_F(manifest, not_compatible)
{
- struct manifest m;
+ struct_manifest m;
/* clang-format off */
std::vector<char> dtb = ManifestDtBuilder()
@@ -357,9 +434,9 @@
ASSERT_EQ(manifest_from_vec(&m, dtb), MANIFEST_ERROR_NOT_COMPATIBLE);
}
-TEST(manifest, compatible_one_of_many)
+TEST_F(manifest, compatible_one_of_many)
{
- struct manifest m;
+ struct_manifest m;
/* clang-format off */
std::vector<char> dtb = ManifestDtBuilder()
@@ -375,9 +452,9 @@
ASSERT_EQ(manifest_from_vec(&m, dtb), MANIFEST_SUCCESS);
}
-TEST(manifest, no_vm_nodes)
+TEST_F(manifest, no_vm_nodes)
{
- struct manifest m;
+ struct_manifest m;
/* clang-format off */
std::vector<char> dtb = ManifestDtBuilder()
@@ -409,9 +486,9 @@
/* clang-format on */
}
-TEST(manifest, long_string)
+TEST_F(manifest, long_string)
{
- struct manifest m;
+ struct_manifest m;
std::vector<char> dtb_last_valid = gen_long_string_dtb(true);
std::vector<char> dtb_first_invalid = gen_long_string_dtb(false);
@@ -420,9 +497,9 @@
MANIFEST_ERROR_STRING_TOO_LONG);
}
-TEST(manifest, reserved_vm_id)
+TEST_F(manifest, reserved_vm_id)
{
- struct manifest m;
+ struct_manifest m;
/* clang-format off */
std::vector<char> dtb = ManifestDtBuilder()
@@ -464,9 +541,9 @@
/* clang-format on */
}
-TEST(manifest, vcpu_count_limit)
+TEST_F(manifest, vcpu_count_limit)
{
- struct manifest m;
+ struct_manifest m;
std::vector<char> dtb_last_valid = gen_vcpu_count_limit_dtb(UINT16_MAX);
std::vector<char> dtb_first_invalid =
gen_vcpu_count_limit_dtb(UINT16_MAX + 1);
@@ -479,9 +556,9 @@
MANIFEST_ERROR_INTEGER_OVERFLOW);
}
-TEST(manifest, no_ramdisk_primary)
+TEST_F(manifest, no_ramdisk_primary)
{
- struct manifest m;
+ struct_manifest m;
/* clang-format off */
std::vector<char> dtb = ManifestDtBuilder()
@@ -500,9 +577,9 @@
ASSERT_STREQ(string_data(&m.vm[0].primary.ramdisk_filename), "");
}
-TEST(manifest, no_boot_address_primary)
+TEST_F(manifest, no_boot_address_primary)
{
- struct manifest m;
+ struct_manifest m;
/* clang-format off */
std::vector<char> dtb = ManifestDtBuilder()
@@ -521,9 +598,9 @@
ASSERT_EQ(m.vm[0].primary.boot_address, MANIFEST_INVALID_ADDRESS);
}
-TEST(manifest, boot_address_primary)
+TEST_F(manifest, boot_address_primary)
{
- struct manifest m;
+ struct_manifest m;
const uint64_t addr = UINT64_C(0x12345678ABCDEFEF);
/* clang-format off */
@@ -560,9 +637,9 @@
/* clang-format on */
}
-TEST(manifest, malformed_booleans)
+TEST_F(manifest, malformed_booleans)
{
- struct manifest m;
+ struct_manifest m;
std::vector<char> dtb_false = gen_malformed_boolean_dtb("\"false\"");
std::vector<char> dtb_true = gen_malformed_boolean_dtb("\"true\"");
@@ -579,9 +656,9 @@
MANIFEST_ERROR_MALFORMED_BOOLEAN);
}
-TEST(manifest, valid)
+TEST_F(manifest, valid)
{
- struct manifest m;
+ struct_manifest m;
struct manifest_vm *vm;
/* clang-format off */
@@ -646,59 +723,9 @@
ASSERT_FALSE(vm->smc_whitelist.permissive);
}
-/**
- * Class for programatically building a Partition package.
- */
-class Partition_package
+TEST_F(manifest, ffa_not_compatible)
{
- public:
- __attribute__((aligned(PAGE_SIZE))) struct sp_pkg_header spkg;
- __attribute__((aligned(PAGE_SIZE))) char manifest_dtb[PAGE_SIZE] = {};
- __attribute__((aligned(PAGE_SIZE))) char img[PAGE_SIZE] = {};
-
- Partition_package(const std::vector<char> &vec)
- {
- // Initialise header field
- spkg.magic = SP_PKG_HEADER_MAGIC;
- spkg.version = SP_PKG_HEADER_VERSION_2;
- spkg.pm_offset = PAGE_SIZE;
- spkg.pm_size = vec.size();
- spkg.img_offset = 2 * PAGE_SIZE;
- spkg.img_size = ARRAY_SIZE(img);
-
- // Copy dtb into package
- std::copy(vec.begin(), vec.end(), manifest_dtb);
- }
-};
-
-static enum manifest_return_code ffa_manifest_from_vec(
- struct manifest *m, const std::vector<char> &vec)
-{
- struct memiter it;
- struct mpool ppool;
- struct mm_stage1_locked mm_stage1_locked;
-
- Partition_package spkg(vec);
-
- /* clang-format off */
- std::vector<char> core_dtb = ManifestDtBuilder()
- .StartChild("hypervisor")
- .Compatible()
- .StartChild("vm1")
- .DebugName("primary_vm")
- .FfaPartition()
- .LoadAddress((uint64_t)&spkg)
- .EndChild()
- .EndChild()
- .Build();
- /* clang-format on */
- memiter_init(&it, core_dtb.data(), core_dtb.size());
- return manifest_init(mm_stage1_locked, m, &it, &ppool);
-}
-
-TEST(manifest, ffa_not_compatible)
-{
- struct manifest m;
+ struct_manifest m;
/* clang-format off */
std::vector<char> dtb = ManifestDtBuilder()
@@ -718,9 +745,9 @@
MANIFEST_ERROR_NOT_COMPATIBLE);
}
-TEST(manifest, ffa_missing_property)
+TEST_F(manifest, ffa_missing_property)
{
- struct manifest m;
+ struct_manifest m;
/* clang-format off */
std::vector<char> dtb = ManifestDtBuilder()
@@ -733,13 +760,13 @@
MANIFEST_ERROR_PROPERTY_NOT_FOUND);
}
-TEST(manifest, ffa_validate_sanity_check)
+TEST_F(manifest, ffa_validate_sanity_check)
{
/*
* TODO: write test excluding all optional fields of the manifest, in
* accordance with specification.
*/
- struct manifest m;
+ struct_manifest m;
/* Incompatible version */
/* clang-format off */
@@ -832,9 +859,9 @@
MANIFEST_ERROR_NOT_COMPATIBLE);
}
-TEST(manifest, ffa_validate_rxtx_info)
+TEST_F(manifest, ffa_validate_rxtx_info)
{
- struct manifest m;
+ struct_manifest m;
/* Not Compatible */
/* clang-format off */
@@ -861,9 +888,9 @@
MANIFEST_ERROR_PROPERTY_NOT_FOUND);
}
-TEST(manifest, ffa_validate_mem_regions)
+TEST_F(manifest, ffa_validate_mem_regions)
{
- struct manifest m;
+ struct_manifest m;
/* Not Compatible */
/* clang-format off */
@@ -936,9 +963,9 @@
MANIFEST_ERROR_RXTX_SIZE_MISMATCH);
}
-TEST(manifest, ffa_validate_dev_regions)
+TEST_F(manifest, ffa_validate_dev_regions)
{
- struct manifest m;
+ struct_manifest m;
/* Not Compatible */
/* clang-format off */
@@ -1000,9 +1027,10 @@
ASSERT_EQ(ffa_manifest_from_vec(&m, dtb),
MANIFEST_ERROR_MALFORMED_INTEGER_LIST);
}
-TEST(manifest, ffa_invalid_memory_region_attributes)
+
+TEST_F(manifest, ffa_invalid_memory_region_attributes)
{
- struct manifest m;
+ struct_manifest m;
/* clang-format off */
std::vector<char> dtb = ManifestDtBuilder()
@@ -1042,9 +1070,9 @@
MANIFEST_ERROR_INVALID_MEM_PERM);
}
-TEST(manifest, ffa_invalid_device_region_attributes)
+TEST_F(manifest, ffa_invalid_device_region_attributes)
{
- struct manifest m;
+ struct_manifest m;
/* clang-format off */
std::vector<char> dtb = ManifestDtBuilder()
@@ -1096,9 +1124,9 @@
MANIFEST_ERROR_INVALID_MEM_PERM);
}
-TEST(manifest, ffa_valid)
+TEST_F(manifest, ffa_valid)
{
- struct manifest m;
+ struct_manifest m;
/* clang-format off */
std::vector<char> dtb = ManifestDtBuilder()