feat(memory share): validate sp device mem ranges
Validate the device memory specified in a partitions manifest
belongs to the ranges given in the SPMC manifest. This patch
also introduces the ability to define ns device memory ranges
in the SPMC which are needed for later enablement of NWd to SWd
device memory sharing.
Tests made to fail in manifest_test.cc are fixed in this patch
and an additional test is added to check the validation works
as expected. In addition the test checking that normal memory
regions and device memory regions cannot overlap has been removed
as these memory regions need to be allocated in the regions defined
in the SPMC so this would fail with MANIFEST_ERROR_MEM_REGION_INVALID
and is checked by the test added in this patch.
Signed-off-by: Daniel Boulby <daniel.boulby@arm.com>
Change-Id: If93418614f9b58e61e625839ed689048e3c75a03
Signed-off-by: J-Alves <joao.alves@arm.com>
diff --git a/src/arch/aarch64/hypervisor/other_world.c b/src/arch/aarch64/hypervisor/other_world.c
index 7b14b0b..1468cf9 100644
--- a/src/arch/aarch64/hypervisor/other_world.c
+++ b/src/arch/aarch64/hypervisor/other_world.c
@@ -55,10 +55,27 @@
params->ns_mem_ranges[i].end,
MM_MODE_R | MM_MODE_W | MM_MODE_X | MM_MODE_NS,
ppool, NULL)) {
- dlog_error("%s", err_msg);
+ dlog_error("Normal Memory: %s", err_msg);
goto out;
}
}
+
+ /*
+ * Map NS device mem ranges to "Other world VM" Stage-2 PTs to allow
+ * for memory sharing operations from NWd to SWd.
+ */
+ for (i = 0; i < params->ns_device_mem_ranges_count; i++) {
+ if (!vm_identity_map(
+ other_world_vm_locked,
+ params->ns_device_mem_ranges[i].begin,
+ params->ns_device_mem_ranges[i].end,
+ MM_MODE_R | MM_MODE_W | MM_MODE_D | MM_MODE_NS,
+ ppool, NULL)) {
+ dlog_error("Device Memory: %s", err_msg);
+ goto out;
+ }
+ }
+
/*
* Force the hypervisor's version to be same as ours.
* FF-A version at hypervisor's initialization is not getting to the
diff --git a/src/boot_flow/common.c b/src/boot_flow/common.c
index 3e93c33..0e8d5cd 100644
--- a/src/boot_flow/common.c
+++ b/src/boot_flow/common.c
@@ -19,6 +19,7 @@
struct string memory = STRING_INIT("memory");
struct string ns_memory = STRING_INIT("ns-memory");
struct string device_memory = STRING_INIT("device-memory");
+ struct string ns_device_memory = STRING_INIT("ns-device-memory");
p->mem_ranges_count = 0;
p->kernel_arg = plat_boot_flow_get_kernel_arg();
@@ -28,11 +29,15 @@
fdt_find_cpus(fdt, p->cpu_ids, &p->cpu_count) &&
fdt_find_memory_ranges(fdt, &memory, p->mem_ranges,
&p->mem_ranges_count, MAX_MEM_RANGES) &&
+ fdt_find_memory_ranges(fdt, &ns_memory, p->ns_mem_ranges,
+ &p->ns_mem_ranges_count,
+ MAX_MEM_RANGES) &&
fdt_find_memory_ranges(fdt, &device_memory, p->device_mem_ranges,
&p->device_mem_ranges_count,
MAX_DEVICE_MEM_RANGES) &&
- fdt_find_memory_ranges(fdt, &ns_memory, p->ns_mem_ranges,
- &p->ns_mem_ranges_count, MAX_MEM_RANGES);
+ fdt_find_memory_ranges(
+ fdt, &ns_device_memory, p->ns_device_mem_ranges,
+ &p->ns_device_mem_ranges_count, MAX_DEVICE_MEM_RANGES);
}
/**
diff --git a/src/manifest.c b/src/manifest.c
index 1bc794b..92afa7b 100644
--- a/src/manifest.c
+++ b/src/manifest.c
@@ -516,15 +516,33 @@
{
bool is_secure_region =
(attributes & MANIFEST_REGION_ATTR_SECURITY) == 0U;
- const struct mem_range *ranges_from_manifest =
- is_secure_region ? params->mem_ranges : params->ns_mem_ranges;
- size_t ranges_count = is_secure_region ? params->mem_ranges_count
- : params->ns_mem_ranges_count;
- bool within_ranges = is_memory_region_within_ranges(
+ bool is_device_region =
+ (attributes & MANIFEST_REGION_ATTR_MEMORY_TYPE_DEVICE) != 0U;
+ const struct mem_range *ranges_from_manifest;
+ size_t ranges_count;
+ bool within_ranges;
+ enum manifest_return_code error_return;
+
+ if (!is_device_region) {
+ ranges_from_manifest = is_secure_region ? params->mem_ranges
+ : params->ns_mem_ranges;
+ ranges_count = is_secure_region ? params->mem_ranges_count
+ : params->ns_mem_ranges_count;
+ error_return = MANIFEST_ERROR_MEM_REGION_INVALID;
+ } else {
+ ranges_from_manifest = is_secure_region
+ ? params->device_mem_ranges
+ : params->ns_device_mem_ranges;
+ ranges_count = is_secure_region
+ ? params->device_mem_ranges_count
+ : params->ns_device_mem_ranges_count;
+ error_return = MANIFEST_ERROR_DEVICE_MEM_REGION_INVALID;
+ }
+
+ within_ranges = is_memory_region_within_ranges(
base_address, page_count, ranges_from_manifest, ranges_count);
- return within_ranges ? MANIFEST_SUCCESS
- : MANIFEST_ERROR_MEM_REGION_INVALID;
+ return within_ranges ? MANIFEST_SUCCESS : error_return;
}
/*
@@ -789,7 +807,8 @@
static enum manifest_return_code parse_ffa_device_region_node(
struct fdt_node *dev_node, struct device_region *dev_regions,
- uint16_t *count, uint8_t *dma_device_count)
+ uint16_t *count, uint8_t *dma_device_count,
+ const struct boot_params *boot_params)
{
struct uint32list_iter list;
uint16_t i = 0;
@@ -832,6 +851,10 @@
TRY(read_uint32(dev_node, "attributes",
&dev_regions[i].attributes));
+ /* Set the memory type attribute to device. */
+ dev_regions[i].attributes =
+ dev_regions[i].attributes |
+ MANIFEST_REGION_ATTR_MEMORY_TYPE_DEVICE;
/*
* Check RWX permission attributes.
@@ -848,13 +871,17 @@
return MANIFEST_ERROR_INVALID_MEM_PERM;
}
- /* Filer device region attributes. */
+ /* Filter device region attributes. */
dev_regions[i].attributes = dev_regions[i].attributes &
MANIFEST_REGION_ALL_ATTR_MASK;
dlog_verbose(" Attributes: %#x\n",
dev_regions[i].attributes);
+ TRY(check_partition_memory_is_valid(
+ dev_regions[i].base_address, dev_regions[i].page_count,
+ dev_regions[i].attributes, boot_params));
+
TRY(read_optional_uint32list(dev_node, "interrupts", &list));
dlog_verbose(" Interrupt List:\n");
j = 0;
@@ -1402,7 +1429,7 @@
TRY(parse_ffa_device_region_node(
&ffa_node, vm->partition.dev_regions,
&vm->partition.dev_region_count,
- &vm->partition.dma_device_count));
+ &vm->partition.dma_device_count, boot_params));
}
dlog_verbose(" Total %u device regions found\n",
vm->partition.dev_region_count);
@@ -1694,6 +1721,8 @@
"regions";
case MANIFEST_ERROR_MEM_REGION_INVALID:
return "Invalid memory region range";
+ case MANIFEST_ERROR_DEVICE_MEM_REGION_INVALID:
+ return "Invalid device memory region range";
case MANIFEST_ERROR_INVALID_BOOT_ORDER:
return "Boot order should be a unique value less than "
"default largest value";
diff --git a/src/manifest_test.cc b/src/manifest_test.cc
index 6d07c41..c6c919f 100644
--- a/src/manifest_test.cc
+++ b/src/manifest_test.cc
@@ -405,6 +405,18 @@
pa_init((uintpaddr_t)0x7000000);
params->ns_mem_ranges[0].end = pa_init((uintpaddr_t)0x8ffffff);
params->ns_mem_ranges_count = 1;
+
+ params->ns_device_mem_ranges[0].begin =
+ pa_init((uintpaddr_t)0x20000000);
+ params->ns_device_mem_ranges[0].end =
+ pa_init((uintpaddr_t)0x24000000);
+ params->ns_device_mem_ranges_count = 1;
+
+ params->device_mem_ranges[0].begin =
+ pa_init((uintpaddr_t)0x24000000);
+ params->device_mem_ranges[0].end =
+ pa_init((uintpaddr_t)0x28000000);
+ params->device_mem_ranges_count = 1;
}
enum manifest_return_code manifest_from_vec(
@@ -1469,7 +1481,7 @@
.Compatible({ "arm,ffa-manifest-device-regions" })
.StartChild("test-device")
.Description("test-device")
- .Property("base-address", "<0x7200000>")
+ .Property("base-address", "<0x24000000>")
.Property("pages-count", "<16>")
.Property("attributes", "<3>")
.Property("smmu-id", "<1>")
@@ -1491,14 +1503,14 @@
.Compatible({ "arm,ffa-manifest-device-regions" })
.StartChild("test-device-0")
.Description("test-device-0")
- .Property("base-address", "<0x7200000>")
+ .Property("base-address", "<0x24000000>")
.Property("pages-count", "<16>")
.Property("attributes", "<3>")
.Property("interrupts", "<2 3>")
.EndChild()
.StartChild("test-device-1")
.Description("test-device-1")
- .Property("base-address", "<0x8200000>")
+ .Property("base-address", "<0x25000000>")
.Property("pages-count", "<16>")
.Property("attributes", "<3>")
.Property("interrupts", "<1 3>, <2 5> ")
@@ -1526,17 +1538,17 @@
.Compatible({"arm,ffa-manifest-device-regions"})
.StartChild("test-device-0")
.Description("test-device-0")
- .Property("base-address", "<0x7200000>")
+ .Property("base-address", "<0x24000000>")
.Property("pages-count", "<16>")
.Property("attributes", "<3>")
.EndChild()
.StartChild("test-device-1")
.Description("test-device-1")
- .Property("base-address", "<0x7200000>")
+ .Property("base-address", "<0x24000000>")
.Property("pages-count", "<16>")
.Property("attributes", "<3>")
.EndChild()
- .EndChild()
+ .EndChild()
.Build();
/* clang-format on */
@@ -1545,26 +1557,17 @@
manifest_dealloc();
/*
- * Check memory region node and device region node address space
- * cannot overlap.
+ * Device regions cannot be defined outside of the regions specified in
+ * the spmc.
*/
/* clang-format off */
dtb = ManifestDtBuilder()
.FfaValidManifest()
- .StartChild("memory-regions")
- .Compatible({"arm,ffa-manifest-memory-regions" })
- .StartChild("test-memory")
- .Description("test-memory")
- .Property("base-address", "<0x7100000>")
- .Property("pages-count", "<16>")
- .Property("attributes", "<3>")
- .EndChild()
- .EndChild()
.StartChild("device-regions")
.Compatible({"arm,ffa-manifest-device-regions"})
.StartChild("test-device-0")
.Description("test-device-0")
- .Property("base-address", "<0x7100000>")
+ .Property("base-address", "<0x50000000>")
.Property("pages-count", "<16>")
.Property("attributes", "<3>")
.EndChild()
@@ -1572,7 +1575,28 @@
.Build();
/* clang-format on */
ASSERT_EQ(ffa_manifest_from_vec(&m, dtb),
- MANIFEST_ERROR_MEM_REGION_OVERLAP);
+ MANIFEST_ERROR_DEVICE_MEM_REGION_INVALID);
+ manifest_dealloc();
+
+ /*
+ * Memory defined as NS in SPMC manifest given Secure attribute should
+ * fail.
+ */
+ /* clang-format off */
+ dtb = ManifestDtBuilder()
+ .FfaValidManifest()
+ .StartChild("device-regions")
+ .Compatible({"arm,ffa-manifest-device-regions"})
+ .StartChild("test-device-0")
+ .Description("test-device-0")
+ .Property("base-address", "<0x20000000>")
+ .Property("pages-count", "<16>")
+ .Property("attributes", "<3>")
+ .EndChild()
+ .EndChild()
+ .Build();
+ ASSERT_EQ(ffa_manifest_from_vec(&m, dtb),
+ MANIFEST_ERROR_DEVICE_MEM_REGION_INVALID);
}
TEST_F(manifest, ffa_invalid_memory_region_attributes)
@@ -1717,7 +1741,7 @@
.Compatible({ "arm,ffa-manifest-device-regions" })
.StartChild("test-device")
.Description("test-device")
- .Property("base-address", "<0x7400000>")
+ .Property("base-address", "<0x24000000>")
.Property("pages-count", "<16>")
.Property("attributes", "<3>")
.Property("smmu-id", "<1>")
@@ -1726,12 +1750,13 @@
.EndChild()
.StartChild("test-device-ns")
.Description("test-device")
- .Property("base-address", "<0x7500000>")
+ .Property("base-address", "<0x20000000>")
.Property("pages-count", "<1>")
.Property("attributes", "<0x9>")
.EndChild()
.EndChild()
.Build();
+
/* clang-format on */
ASSERT_EQ(ffa_manifest_from_vec(&m, dtb), MANIFEST_SUCCESS);
@@ -1761,9 +1786,9 @@
ASSERT_EQ(vm->partition.rxtx.tx_buffer->page_count, 1);
ASSERT_EQ(vm->partition.rxtx.tx_buffer->attributes, 3);
- ASSERT_EQ(vm->partition.dev_regions[0].base_address, 0x7400000);
+ ASSERT_EQ(vm->partition.dev_regions[0].base_address, 0x24000000);
ASSERT_EQ(vm->partition.dev_regions[0].page_count, 16);
- ASSERT_EQ(vm->partition.dev_regions[0].attributes, 3);
+ ASSERT_EQ(vm->partition.dev_regions[0].attributes, (16 | 3));
ASSERT_EQ(vm->partition.dev_regions[0].smmu_id, 1);
ASSERT_EQ(vm->partition.dev_regions[0].stream_ids[0], 0);
ASSERT_EQ(vm->partition.dev_regions[0].stream_ids[1], 1);
@@ -1771,7 +1796,8 @@
ASSERT_EQ(vm->partition.dev_regions[0].interrupts[0].attributes, 3);
ASSERT_EQ(vm->partition.dev_regions[0].interrupts[1].id, 4);
ASSERT_EQ(vm->partition.dev_regions[0].interrupts[1].attributes, 5);
- ASSERT_EQ(vm->partition.dev_regions[1].attributes, (8 | 1));
+ ASSERT_EQ(vm->partition.dev_regions[1].base_address, 0x20000000);
+ ASSERT_EQ(vm->partition.dev_regions[1].attributes, (16 | 8 | 1));
}
TEST_F(manifest, ffa_valid_interrupt_target_manifest)
@@ -1786,7 +1812,7 @@
.Compatible({ "arm,ffa-manifest-device-regions" })
.StartChild("test-device")
.Description("test-device")
- .Property("base-address", "<0x7400000>")
+ .Property("base-address", "<0x24000000>")
.Property("pages-count", "<16>")
.Property("attributes", "<3>")
.Property("smmu-id", "<1>")
@@ -1802,9 +1828,9 @@
vm = &m->vm[0];
- ASSERT_EQ(vm->partition.dev_regions[0].base_address, 0x7400000);
+ ASSERT_EQ(vm->partition.dev_regions[0].base_address, 0x24000000);
ASSERT_EQ(vm->partition.dev_regions[0].page_count, 16);
- ASSERT_EQ(vm->partition.dev_regions[0].attributes, 3);
+ ASSERT_EQ(vm->partition.dev_regions[0].attributes, (16 | 3));
ASSERT_EQ(vm->partition.dev_regions[0].smmu_id, 1);
ASSERT_EQ(vm->partition.dev_regions[0].stream_ids[0], 0);
ASSERT_EQ(vm->partition.dev_regions[0].stream_ids[1], 1);
@@ -1831,7 +1857,7 @@
.Compatible({ "arm,ffa-manifest-device-regions" })
.StartChild("test-device")
.Description("test-device")
- .Property("base-address", "<0x7400000>")
+ .Property("base-address", "<0x24000000>")
.Property("pages-count", "<16>")
.Property("attributes", "<3>")
.Property("smmu-id", "<1>")
@@ -2014,7 +2040,7 @@
.Compatible({ "arm,ffa-manifest-device-regions" })
.StartChild("test-device-0")
.Description("test-device-0")
- .Property("base-address", "<0x7200000>")
+ .Property("base-address", "<0x24000000>")
.Property("pages-count", "<16>")
.Property("attributes", "<3>")
.EndChild()
@@ -2036,7 +2062,7 @@
.Compatible({ "arm,ffa-manifest-device-regions" })
.StartChild("test-device-0")
.Description("test-device-1")
- .Property("base-address", "<0x7200000>")
+ .Property("base-address", "<0x24000000>")
.Property("pages-count", "<16>")
.Property("attributes", "<3>")
.EndChild()
@@ -2091,7 +2117,7 @@
.Compatible({ "arm,ffa-manifest-device-regions" })
.StartChild("test-device-0")
.Description("test-device-0")
- .Property("base-address", "<0x7200000>")
+ .Property("base-address", "<0x24000000>")
.Property("pages-count", "<16>")
.Property("attributes", "<3>")
.EndChild()
@@ -2113,7 +2139,7 @@
.Compatible({ "arm,ffa-manifest-device-regions" })
.StartChild("test-device-0")
.Description("test-device-1")
- .Property("base-address", "<0x7300000>")
+ .Property("base-address", "<0x25000000>")
.Property("pages-count", "<16>")
.Property("attributes", "<3>")
.EndChild()