Correct aarch64 PTE attribute handling.
Not all the non-address bits are attribute bits so the mask needs to
exclude those that are not. This was causing mapping to miss the case
when then entry is already in place as the attributes appeared
different.
Other cases continued to work by luck as the extra included bits did not
interfere with behaviour.
Change-Id: I8384e952cd964e245097d72909999ce6599c3736
diff --git a/src/arch/aarch64/mm.c b/src/arch/aarch64/mm.c
index 568bfb3..1745122 100644
--- a/src/arch/aarch64/mm.c
+++ b/src/arch/aarch64/mm.c
@@ -86,13 +86,12 @@
/* clang-format on */
-/*
- * This mask actually includes everything other than the address bits: not just
- * the attributes but also some ignored bits, reserved bits, and the entry type
- * bits which distinguish between absent, table, block or page entries.
- */
-#define PTE_ATTR_MASK \
- (((UINT64_C(1) << PAGE_BITS) - 1) | ~((UINT64_C(1) << 48) - 1))
+/** Mask for the address bits of the pte. */
+#define PTE_ADDR_MASK \
+ (((UINT64_C(1) << 48) - 1) & ~((UINT64_C(1) << PAGE_BITS) - 1))
+
+/** Mask for the attribute bits of the pte. */
+#define PTE_ATTR_MASK (~(PTE_ADDR_MASK | UINT64_C(0x3)))
static uint8_t mm_s2_max_level;
static uint8_t mm_s2_root_table_count;
@@ -172,18 +171,18 @@
(pte & 0x3) == (level == 0 ? 0x3 : 0x1);
}
-uint64_t arch_aarch64_mm_clear_pte_attrs(pte_t pte)
+static uint64_t pte_addr(pte_t pte)
{
- return pte & ~PTE_ATTR_MASK;
+ return pte & PTE_ADDR_MASK;
}
/**
- * Clears the given physical address, i.e., sets the ignored bits (from a page
- * table perspective) to zero.
+ * Clears the given physical address, i.e., clears the bits of the address that
+ * are not used in the pte.
*/
paddr_t arch_mm_clear_pa(paddr_t pa)
{
- return pa_init(arch_aarch64_mm_clear_pte_attrs(pa_addr(pa)));
+ return pa_init(pte_addr(pa_addr(pa)));
}
/**
@@ -192,7 +191,7 @@
*/
paddr_t arch_mm_block_from_pte(pte_t pte)
{
- return pa_init(arch_aarch64_mm_clear_pte_attrs(pte));
+ return pa_init(pte_addr(pte));
}
/**
@@ -201,7 +200,7 @@
*/
paddr_t arch_mm_table_from_pte(pte_t pte)
{
- return pa_init(arch_aarch64_mm_clear_pte_attrs(pte));
+ return pa_init(pte_addr(pte));
}
/**
diff --git a/test/arch/mm_test.c b/test/arch/mm_test.c
index 4af0c7a..3498086 100644
--- a/test/arch/mm_test.c
+++ b/test/arch/mm_test.c
@@ -84,3 +84,37 @@
}
EXPAND_LEVEL_TESTS
#undef LEVEL_TEST
+
+/**
+ * The address and attributes of a block must be preserved when encoding and
+ * decoding.
+ */
+#define LEVEL_TEST(lvl) \
+ TEST(arch_mm, block_addr_and_attrs_preserved_level##lvl) \
+ { \
+ uint8_t level = lvl; \
+ paddr_t addr; \
+ uint64_t attrs; \
+ pte_t block_pte; \
+ \
+ /* Test doesn't apply if a block is not allowed. */ \
+ if (!arch_mm_is_block_allowed(level)) { \
+ return; \
+ } \
+ \
+ addr = pa_init(0); \
+ attrs = arch_mm_mode_to_attrs(0); \
+ block_pte = arch_mm_block_pte(level, addr, attrs); \
+ EXPECT_EQ(arch_mm_pte_attrs(block_pte), attrs); \
+ EXPECT_EQ(pa_addr(arch_mm_block_from_pte(block_pte)), \
+ pa_addr(addr)); \
+ \
+ addr = pa_init(PAGE_SIZE * 500); \
+ attrs = arch_mm_mode_to_attrs(MM_MODE_R | MM_MODE_W); \
+ block_pte = arch_mm_block_pte(level, addr, attrs); \
+ EXPECT_EQ(arch_mm_pte_attrs(block_pte), attrs); \
+ EXPECT_EQ(pa_addr(arch_mm_block_from_pte(block_pte)), \
+ pa_addr(addr)); \
+ }
+EXPAND_LEVEL_TESTS
+#undef LEVEL_TEST
