refactor(mm): replace max level with root level
The name `mm_max_level()` is misleading, since it is not actually the
maximum value that the level can be (since the root level is the "max"
level plus 1).
The use of `mm_max_level()` in `mm.c` was also inconsistent: sometimes
they call `mm_max_level() + 1` to calculate the root level, sometimes
they just pass `mm_max_level()` to recursive subcalls to process the
next level down. This made it hard to know when a plus or minus one was
needed and so ended up introducing off-by-one errors. Now, you always
start at `mm_root_level()` and pass `root_level - 1` to recursive
subcalls.
Also add explanatory comments explaining the level-related arithmetic.
Change-Id: I7b701d6d2c908db03c2853a008565126890e7959
Signed-off-by: Karl Meakin <karl.meakin@arm.com>
diff --git a/src/arch/aarch64/hftest/mm.c b/src/arch/aarch64/hftest/mm.c
index 483dc4d..ac6edd3 100644
--- a/src/arch/aarch64/hftest/mm.c
+++ b/src/arch/aarch64/hftest/mm.c
@@ -58,7 +58,7 @@
* Limit PA bits to HFTEST_S1_PA_BITS. Using the pa_bits reported by
* arch_mm_get_pa_range requires an increase in page pool size.
*/
- arch_mm_stage1_max_level_set(HFTEST_S1_PA_BITS);
+ arch_mm_stage1_root_level_set(HFTEST_S1_PA_BITS);
/*
* Preserve initial values of the system registers in case we want to
diff --git a/src/arch/aarch64/mm.c b/src/arch/aarch64/mm.c
index 49ae545..5c13f6a 100644
--- a/src/arch/aarch64/mm.c
+++ b/src/arch/aarch64/mm.c
@@ -137,8 +137,8 @@
uintreg_t vstcr_el2;
} arch_mm_config;
-static mm_level_t mm_s1_max_level;
-static mm_level_t mm_s2_max_level;
+static mm_level_t mm_s1_root_level;
+static mm_level_t mm_s2_root_level;
static uint8_t mm_s2_root_table_count;
/**
@@ -677,27 +677,27 @@
return mode;
}
-void arch_mm_stage1_max_level_set(uint32_t pa_bits)
+void arch_mm_stage1_root_level_set(uint32_t pa_bits)
{
/* Maximum supported PA range in bits is 48 */
CHECK(pa_bits <= 48);
if (pa_bits >= 40) {
- mm_s1_max_level = 3;
+ mm_s1_root_level = 4;
} else {
- /* Setting to 2 covers physical memory upto 512GB */
- mm_s1_max_level = 2;
+ /* Setting to 3 covers physical memory upto 512GB */
+ mm_s1_root_level = 3;
}
}
-mm_level_t arch_mm_stage1_max_level(void)
+mm_level_t arch_mm_stage1_root_level(void)
{
- return mm_s1_max_level;
+ return mm_s1_root_level;
}
-mm_level_t arch_mm_stage2_max_level(void)
+mm_level_t arch_mm_stage2_root_level(void)
{
- return mm_s2_max_level;
+ return mm_s2_root_level;
}
uint8_t arch_mm_stage1_root_table_count(void)
@@ -780,22 +780,18 @@
* of bits. The value is chosen to give the shallowest tree by making
* use of concatenated translation tables.
*
- * - 0 => start at level 1
- * - 1 => start at level 2
- * - 2 => start at level 3
*/
if (pa_bits >= 44) {
- sl0 = 2;
- mm_s2_max_level = 3;
+ mm_s2_root_level = 4;
} else if (pa_bits >= 35) {
- sl0 = 1;
- mm_s2_max_level = 2;
+ mm_s2_root_level = 3;
} else {
- sl0 = 0;
- mm_s2_max_level = 1;
+ mm_s2_root_level = 2;
}
- arch_mm_stage1_max_level_set(pa_bits);
+ sl0 = mm_s2_root_level - 2;
+
+ arch_mm_stage1_root_level_set(pa_bits);
/*
* Since the shallowest possible tree is used, the maximum number of
@@ -811,11 +807,11 @@
dlog_info(
"Stage 2 has %d page table levels with %d pages at the root.\n",
- mm_s2_max_level + 1, mm_s2_root_table_count);
+ mm_s2_root_level + 1, mm_s2_root_table_count);
dlog_info(
"Stage 1 has %d page table levels with %d pages at the root.\n",
- mm_s1_max_level + 1, arch_mm_stage1_root_table_count());
+ mm_s1_root_level + 1, arch_mm_stage1_root_table_count());
/*
* If the PE implements S-EL2 then VTCR_EL2.NSA/NSW bits are significant
diff --git a/src/arch/fake/mm.c b/src/arch/fake/mm.c
index ea1f6ac..870e085 100644
--- a/src/arch/fake/mm.c
+++ b/src/arch/fake/mm.c
@@ -131,20 +131,20 @@
/* There's no modelling of the cache. */
}
-void arch_mm_stage1_max_level_set(uint32_t pa_bits)
+void arch_mm_stage1_root_level_set(uint32_t pa_bits)
{
- /* Not required to set this value as its hardcoded to 2 */
+ /* Not required to set this value as it's hardcoded to 3 */
(void)pa_bits;
}
-mm_level_t arch_mm_stage1_max_level(void)
+mm_level_t arch_mm_stage1_root_level(void)
{
- return 2;
+ return 3;
}
-mm_level_t arch_mm_stage2_max_level(void)
+mm_level_t arch_mm_stage2_root_level(void)
{
- return 2;
+ return 3;
}
uint8_t arch_mm_stage1_root_table_count(void)
diff --git a/src/ipi_test.cc b/src/ipi_test.cc
index 91cc229..59672ee 100644
--- a/src/ipi_test.cc
+++ b/src/ipi_test.cc
@@ -35,7 +35,6 @@
*/
constexpr size_t TEST_HEAP_SIZE = PAGE_SIZE * 64;
-const mm_level_t TOP_LEVEL = arch_mm_stage2_max_level();
class ipi : public ::testing::Test
{
protected:
diff --git a/src/mm.c b/src/mm.c
index 9b2c006..078b3f0 100644
--- a/src/mm.c
+++ b/src/mm.c
@@ -79,19 +79,27 @@
/**
* Calculates the size of the address space represented by a page table entry at
- * the given level.
+ * the given level. See also Arm ARM, table D8-15
+ * - `level == 4`: 256 TiB (1 << 48)
+ * - `level == 3`: 512 GiB (1 << 39)
+ * - `level == 2`: 1 GiB (1 << 30)
+ * - `level == 1`: 2 MiB (1 << 21)
+ * - `level == 0`: 4 KiB (1 << 12)
*/
static size_t mm_entry_size(mm_level_t level)
{
+ assert(level <= 4);
return UINT64_C(1) << (PAGE_BITS + level * PAGE_LEVEL_BITS);
}
/**
- * Gets the address of the start of the next block of the given level.
+ * Get the start address of the range mapped by the next block of the given
+ * level.
*/
static ptable_addr_t mm_start_of_next_block(ptable_addr_t addr,
mm_level_t level)
{
+ assert(level <= 4);
return align_up(addr + 1, mm_entry_size(level));
}
@@ -108,7 +116,12 @@
/**
* For a given address, calculates the index at which its entry is stored in a
- * table at the given level.
+ * table at the given level. See also Arm ARM, table D8-14
+ * - `level == 4`: bits[51:48]
+ * - `level == 3`: bits[47:39]
+ * - `level == 2`: bits[38:30]
+ * - `level == 1`: bits[29:21]
+ * - `level == 0`: bits[20:12]
*/
static size_t mm_index(ptable_addr_t addr, mm_level_t level)
{
@@ -131,12 +144,12 @@
}
/**
- * Returns the maximum level in the page table given the flags.
+ * Returns the root level in the page table given the flags.
*/
-static mm_level_t mm_max_level(struct mm_flags flags)
+static mm_level_t mm_root_level(struct mm_flags flags)
{
- return flags.stage1 ? arch_mm_stage1_max_level()
- : arch_mm_stage2_max_level();
+ return flags.stage1 ? arch_mm_stage1_root_level()
+ : arch_mm_stage2_root_level();
}
/**
@@ -193,8 +206,7 @@
*/
ptable_addr_t mm_ptable_addr_space_end(struct mm_flags flags)
{
- return mm_root_table_count(flags) *
- mm_entry_size(mm_max_level(flags) + 1);
+ return mm_root_table_count(flags) * mm_entry_size(mm_root_level(flags));
}
/**
@@ -205,6 +217,7 @@
{
struct mm_page_table *tables;
uint8_t root_table_count = mm_root_table_count(flags);
+ mm_level_t root_level = mm_root_level(flags);
tables = mm_alloc_page_tables(root_table_count, ppool);
if (tables == NULL) {
@@ -214,7 +227,7 @@
for (size_t i = 0; i < root_table_count; i++) {
for (size_t j = 0; j < MM_PTE_PER_PAGE; j++) {
tables[i].entries[j] =
- arch_mm_absent_pte(mm_max_level(flags));
+ arch_mm_absent_pte(root_level - 1);
}
}
@@ -234,12 +247,13 @@
struct mm_flags flags, struct mpool *ppool)
{
struct mm_page_table *tables = mm_page_table_from_pa(ptable->root);
- mm_level_t level = mm_max_level(flags);
+ mm_level_t root_level = mm_root_level(flags);
uint8_t root_table_count = mm_root_table_count(flags);
for (size_t i = 0; i < root_table_count; ++i) {
for (size_t j = 0; j < MM_PTE_PER_PAGE; ++j) {
- mm_free_page_pte(tables[i].entries[j], level, ppool);
+ mm_free_page_pte(tables[i].entries[j], root_level - 1,
+ ppool);
}
}
@@ -432,7 +446,7 @@
paddr_t pa_end, mm_attr_t attrs,
struct mm_flags flags, struct mpool *ppool)
{
- mm_level_t root_level = mm_max_level(flags) + 1;
+ mm_level_t root_level = mm_root_level(flags);
ptable_addr_t ptable_end = mm_ptable_addr_space_end(flags);
ptable_addr_t end = mm_round_up_to_page(pa_addr(pa_end));
ptable_addr_t begin = mm_round_down_to_page(pa_addr(pa_begin));
@@ -440,10 +454,11 @@
ptable->root)[mm_index(begin, root_level)];
/*
- * Assert condition to communicate the API constraint of mm_max_level(),
- * that isn't encoded in the types, to the static analyzer.
+ * Assert condition to communicate the API constraint of
+ * mm_root_level(), that isn't encoded in the types, to the static
+ * analyzer.
*/
- assert(root_level >= 2);
+ assert(root_level >= 3);
/* Cap end to stay within the bounds of the page table. */
if (end > ptable_end) {
@@ -636,7 +651,7 @@
struct mm_flags flags)
{
struct mm_page_table *root_tables = mm_page_table_from_pa(ptable->root);
- mm_level_t root_level = mm_max_level(flags) + 1;
+ mm_level_t root_level = mm_root_level(flags);
uint8_t root_table_count = mm_root_table_count(flags);
uint32_t indent = 0;
@@ -792,7 +807,7 @@
bool non_secure, struct mpool *ppool)
{
struct mm_page_table *tables = mm_page_table_from_pa(ptable->root);
- mm_level_t level = mm_max_level(flags);
+ mm_level_t root_level = mm_root_level(flags);
uint8_t root_table_count = mm_root_table_count(flags);
ptable_addr_t block_addr = 0;
@@ -802,10 +817,11 @@
*/
for (size_t i = 0; i < root_table_count; ++i) {
for (size_t j = 0; j < MM_PTE_PER_PAGE; ++j) {
- mm_ptable_defrag_entry(ptable, block_addr,
- &(tables[i].entries[j]), level,
- flags, non_secure, ppool);
- block_addr = mm_start_of_next_block(block_addr, level);
+ mm_ptable_defrag_entry(
+ ptable, block_addr, &(tables[i].entries[j]),
+ root_level - 1, flags, non_secure, ppool);
+ block_addr = mm_start_of_next_block(block_addr,
+ root_level - 1);
}
}
@@ -877,8 +893,7 @@
ptable_addr_t end, mm_attr_t *attrs,
struct mm_flags flags)
{
- mm_level_t max_level = mm_max_level(flags);
- mm_level_t root_level = max_level + 1;
+ mm_level_t root_level = mm_root_level(flags);
ptable_addr_t ptable_end = mm_ptable_addr_space_end(flags);
struct mm_page_table *table;
bool got_attrs = false;
@@ -894,8 +909,9 @@
table = &mm_page_table_from_pa(
ptable->root)[mm_index(begin, root_level)];
while (begin < end) {
- if (!mm_ptable_get_attrs_level(table, begin, end, max_level,
- got_attrs, attrs)) {
+ if (!mm_ptable_get_attrs_level(table, begin, end,
+ root_level - 1, got_attrs,
+ attrs)) {
return false;
}
diff --git a/src/mm_test.cc b/src/mm_test.cc
index 5e4b31b..8b0ea75 100644
--- a/src/mm_test.cc
+++ b/src/mm_test.cc
@@ -37,7 +37,7 @@
using ::mm_test::get_ptable;
constexpr size_t TEST_HEAP_SIZE = PAGE_SIZE * 16;
-const mm_level_t TOP_LEVEL = arch_mm_stage2_max_level();
+const mm_level_t TOP_LEVEL = arch_mm_stage2_root_level() - 1;
const paddr_t VM_MEM_END = pa_init(0x200'0000'0000);
/**
diff --git a/src/vm_test.cc b/src/vm_test.cc
index 08dd3ca..97f5b74 100644
--- a/src/vm_test.cc
+++ b/src/vm_test.cc
@@ -39,7 +39,7 @@
using struct_vm_locked = struct vm_locked;
constexpr size_t TEST_HEAP_SIZE = PAGE_SIZE * 64;
-const mm_level_t TOP_LEVEL = arch_mm_stage2_max_level();
+const mm_level_t TOP_LEVEL = arch_mm_stage2_root_level() - 1;
class vm : public ::testing::Test
{