Remove deallocation changes in page table commit phase.
The first, preparation, phase in a page table update ensures the page
table has tables allocated to the granularity required for the mapping.
The commit phase can then change the address space without the
possibility of failure and, hence, the potential of a partial update.
Previously, the commit phase would merge empty tables into an absent
entry and free the page. To avoid issues with higher level operations
when the preparation and commit phases details are exposed,
deallocations is removed from the commit phase and an explicit
defragmentation request should be used as needed.
Change-Id: I08b429c6faddf226f66dff1b91f0d217d3a190ca
diff --git a/src/mm.c b/src/mm.c
index 94fe6df..d3855d6 100644
--- a/src/mm.c
+++ b/src/mm.c
@@ -348,22 +348,6 @@
}
/**
- * Returns whether all entries in this table are absent.
- */
-static bool mm_page_table_is_empty(struct mm_page_table *table, uint8_t level)
-{
- uint64_t i;
-
- for (i = 0; i < MM_PTE_PER_PAGE; ++i) {
- if (arch_mm_pte_is_present(table->entries[i], level)) {
- return false;
- }
- }
-
- return true;
-}
-
-/**
* Updates the page table at the given level to map the given address range to a
* physical range using the provided (architecture-specific) attributes. Or if
* MM_FLAG_UNMAP is set, unmap the given range instead.
@@ -432,19 +416,6 @@
flags, ppool)) {
return false;
}
-
- /*
- * If the subtable is now empty, replace it with an
- * absent entry at this level. We never need to do
- * break-before-makes here because we are assigning
- * an absent value.
- */
- if (commit && unmap &&
- mm_page_table_is_empty(nt, level - 1)) {
- pte_t v = *pte;
- *pte = arch_mm_absent_pte(level);
- mm_free_page_pte(v, level, ppool);
- }
}
begin = mm_start_of_next_block(begin, entry_size);
diff --git a/src/mm_test.cc b/src/mm_test.cc
index e16ad91..ae13df7 100644
--- a/src/mm_test.cc
+++ b/src/mm_test.cc
@@ -36,6 +36,7 @@
using ::testing::Contains;
using ::testing::Each;
using ::testing::Eq;
+using ::testing::Not;
using ::testing::SizeIs;
using ::testing::Truly;
@@ -261,7 +262,7 @@
TOP_LEVEL - 2)),
Eq(pa_addr(map_begin)));
- /* Checl only the first page of the second table is mapped. */
+ /* Check only the first page of the second table is mapped. */
auto table1_l2 = tables[1];
EXPECT_THAT(table1_l2.subspan(1), Each(arch_mm_absent_pte(TOP_LEVEL)));
ASSERT_TRUE(arch_mm_pte_is_table(table1_l2[0], TOP_LEVEL));
@@ -545,14 +546,41 @@
const paddr_t map_begin = pa_init(0x160'0000'0000 + PAGE_SIZE);
const paddr_t map_end = pa_add(map_begin, PAGE_SIZE);
struct mm_ptable ptable;
+
ASSERT_TRUE(mm_vm_init(&ptable, &ppool));
ASSERT_TRUE(mm_vm_identity_map(&ptable, map_begin, map_end, mode,
nullptr, &ppool));
ASSERT_TRUE(mm_vm_unmap(&ptable, pa_add(map_begin, 93),
pa_add(map_begin, 99), &ppool));
- EXPECT_THAT(
- get_ptable(ptable),
- AllOf(SizeIs(4), Each(Each(arch_mm_absent_pte(TOP_LEVEL)))));
+
+ auto tables = get_ptable(ptable);
+ constexpr auto l3_index = 2;
+
+ /* Check all other top level entries are empty... */
+ EXPECT_THAT(std::span(tables).first(l3_index),
+ Each(Each(arch_mm_absent_pte(TOP_LEVEL))));
+ EXPECT_THAT(std::span(tables).subspan(l3_index + 1),
+ Each(Each(arch_mm_absent_pte(TOP_LEVEL))));
+
+ /* Except the mapped page which is absent. */
+ auto table_l2 = tables[l3_index];
+ constexpr auto l2_index = 384;
+ EXPECT_THAT(table_l2.first(l2_index),
+ Each(arch_mm_absent_pte(TOP_LEVEL)));
+ ASSERT_TRUE(arch_mm_pte_is_table(table_l2[l2_index], TOP_LEVEL));
+ EXPECT_THAT(table_l2.subspan(l2_index + 1),
+ Each(arch_mm_absent_pte(TOP_LEVEL)));
+
+ auto table_l1 = get_table(
+ arch_mm_table_from_pte(table_l2[l2_index], TOP_LEVEL));
+ ASSERT_TRUE(arch_mm_pte_is_table(table_l1.first(1)[0], TOP_LEVEL - 1));
+ EXPECT_THAT(table_l1.subspan(1),
+ Each(arch_mm_absent_pte(TOP_LEVEL - 1)));
+
+ auto table_l0 = get_table(
+ arch_mm_table_from_pte(table_l1.first(1)[0], TOP_LEVEL - 1));
+ EXPECT_THAT(table_l0, Each(arch_mm_absent_pte(TOP_LEVEL - 2)));
+
mm_vm_fini(&ptable, &ppool);
}
@@ -565,13 +593,49 @@
const paddr_t map_begin = pa_init(0x180'0000'0000 - PAGE_SIZE);
const paddr_t map_end = pa_add(map_begin, 2 * PAGE_SIZE);
struct mm_ptable ptable;
+
ASSERT_TRUE(mm_vm_init(&ptable, &ppool));
ASSERT_TRUE(mm_vm_identity_map(&ptable, map_begin, map_end, mode,
nullptr, &ppool));
ASSERT_TRUE(mm_vm_unmap(&ptable, map_begin, map_end, &ppool));
- EXPECT_THAT(
- get_ptable(ptable),
- AllOf(SizeIs(4), Each(Each(arch_mm_absent_pte(TOP_LEVEL)))));
+
+ auto tables = get_ptable(ptable);
+
+ /* Check the untouched tables are empty. */
+ EXPECT_THAT(std::span(tables).first(2),
+ Each(Each(arch_mm_absent_pte(TOP_LEVEL))));
+
+ /* Check the last page is explicity marked as absent. */
+ auto table2_l2 = tables[2];
+ EXPECT_THAT(table2_l2.first(table2_l2.size() - 1),
+ Each(arch_mm_absent_pte(TOP_LEVEL)));
+ ASSERT_TRUE(arch_mm_pte_is_table(table2_l2.last(1)[0], TOP_LEVEL));
+
+ auto table2_l1 = get_table(
+ arch_mm_table_from_pte(table2_l2.last(1)[0], TOP_LEVEL));
+ EXPECT_THAT(table2_l1.first(table2_l1.size() - 1),
+ Each(arch_mm_absent_pte(TOP_LEVEL - 1)));
+ ASSERT_TRUE(arch_mm_pte_is_table(table2_l1.last(1)[0], TOP_LEVEL - 1));
+
+ auto table2_l0 = get_table(
+ arch_mm_table_from_pte(table2_l1.last(1)[0], TOP_LEVEL - 1));
+ EXPECT_THAT(table2_l0, Each(arch_mm_absent_pte(TOP_LEVEL - 2)));
+
+ /* Check the first page is explicitly marked as absent. */
+ auto table3_l2 = tables[3];
+ ASSERT_TRUE(arch_mm_pte_is_table(table3_l2.first(1)[0], TOP_LEVEL));
+ EXPECT_THAT(table3_l2.subspan(1), Each(arch_mm_absent_pte(TOP_LEVEL)));
+
+ auto table3_l1 = get_table(
+ arch_mm_table_from_pte(table3_l2.first(1)[0], TOP_LEVEL));
+ ASSERT_TRUE(arch_mm_pte_is_table(table3_l1.first(1)[0], TOP_LEVEL - 1));
+ EXPECT_THAT(table3_l1.subspan(1),
+ Each(arch_mm_absent_pte(TOP_LEVEL - 1)));
+
+ auto table3_l0 = get_table(
+ arch_mm_table_from_pte(table3_l1.first(1)[0], TOP_LEVEL - 1));
+ EXPECT_THAT(table3_l0, Each(arch_mm_absent_pte(TOP_LEVEL - 2)));
+
mm_vm_fini(&ptable, &ppool);
}
@@ -632,9 +696,29 @@
nullptr, &ppool));
ASSERT_TRUE(mm_vm_unmap(&ptable, pa_add(page_begin, 100),
pa_add(page_begin, 50), &ppool));
- EXPECT_THAT(
- get_ptable(ptable),
- AllOf(SizeIs(4), Each(Each(arch_mm_absent_pte(TOP_LEVEL)))));
+
+ auto tables = get_ptable(ptable);
+ constexpr auto l3_index = 3;
+
+ /* Check all other top level entries are empty... */
+ EXPECT_THAT(std::span(tables).first(l3_index),
+ Each(Each(arch_mm_absent_pte(TOP_LEVEL))));
+
+ /* Except the mapped page which is absent. */
+ auto table_l2 = tables[l3_index];
+ ASSERT_TRUE(arch_mm_pte_is_table(table_l2.first(1)[0], TOP_LEVEL));
+ EXPECT_THAT(table_l2.subspan(1), Each(arch_mm_absent_pte(TOP_LEVEL)));
+
+ auto table_l1 = get_table(
+ arch_mm_table_from_pte(table_l2.first(1)[0], TOP_LEVEL));
+ ASSERT_TRUE(arch_mm_pte_is_table(table_l1.first(1)[0], TOP_LEVEL - 1));
+ EXPECT_THAT(table_l1.subspan(1),
+ Each(arch_mm_absent_pte(TOP_LEVEL - 1)));
+
+ auto table_l0 = get_table(
+ arch_mm_table_from_pte(table_l1.first(1)[0], TOP_LEVEL - 1));
+ EXPECT_THAT(table_l0, Each(arch_mm_absent_pte(TOP_LEVEL - 2)));
+
mm_vm_fini(&ptable, &ppool);
}
@@ -680,9 +764,9 @@
&ppool));
ASSERT_TRUE(mm_vm_unmap(&ptable, l0_begin, l0_end, &ppool));
ASSERT_TRUE(mm_vm_unmap(&ptable, l1_begin, l1_end, &ppool));
- EXPECT_THAT(
- get_ptable(ptable),
- AllOf(SizeIs(4), Each(Each(arch_mm_absent_pte(TOP_LEVEL)))));
+ EXPECT_THAT(get_ptable(ptable),
+ AllOf(SizeIs(4),
+ Not(Each(Each(arch_mm_absent_pte(TOP_LEVEL))))));
mm_vm_fini(&ptable, &ppool);
}
