Given how the meaning of the attribute bits for page table entry types
is slightly awkward, and changes between levels, add some helpers to
abstract from this.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
---
 ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 40 +++++++++++++++++---
 1 file changed, 35 insertions(+), 5 deletions(-)

diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index d78918cf7ba8..0680ba36d907 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -162,6 +162,36 @@ FreePageTablesRecursive (
   FreePages (TranslationTable, 1);
 }
 
+STATIC
+BOOLEAN
+IsBlockEntry (
+  IN  UINT64  Entry,
+  IN  UINTN   Level
+  )
+{
+  if (Level == 3) {
+    return (Entry & TT_TYPE_MASK) == TT_TYPE_BLOCK_ENTRY_LEVEL3;
+  }
+  return (Entry & TT_TYPE_MASK) == TT_TYPE_BLOCK_ENTRY;
+}
+
+STATIC
+BOOLEAN
+IsTableEntry (
+  IN  UINT64  Entry,
+  IN  UINTN   Level
+  )
+{
+  if (Level == 3) {
+    //
+    // TT_TYPE_TABLE_ENTRY aliases TT_TYPE_BLOCK_ENTRY_LEVEL3
+    // so we need to take the level into account as well.
+    //
+    return FALSE;
+  }
+  return (Entry & TT_TYPE_MASK) == TT_TYPE_TABLE_ENTRY;
+}
+
 STATIC
 EFI_STATUS
 UpdateRegionMappingRecursive (
@@ -203,7 +233,7 @@ UpdateRegionMappingRecursive (
     if (Level == 0 || ((RegionStart | BlockEnd) & BlockMask) != 0) {
       ASSERT (Level < 3);
 
-      if ((*Entry & TT_TYPE_MASK) != TT_TYPE_TABLE_ENTRY) {
+      if (!IsTableEntry (*Entry, Level)) {
         //
         // No table entry exists yet, so we need to allocate a page table
         // for the next level.
@@ -221,7 +251,7 @@ UpdateRegionMappingRecursive (
           InvalidateDataCacheRange (TranslationTable, EFI_PAGE_SIZE);
         }
 
-        if ((*Entry & TT_TYPE_MASK) == TT_TYPE_BLOCK_ENTRY) {
+        if (IsBlockEntry (*Entry, Level)) {
           //
           // We are splitting an existing block entry, so we have to populate
           // the new table with the attributes of the block entry it replaces.
@@ -252,7 +282,7 @@ UpdateRegionMappingRecursive (
                  AttributeSetMask, AttributeClearMask, TranslationTable,
                  Level + 1);
       if (EFI_ERROR (Status)) {
-        if ((*Entry & TT_TYPE_MASK) != TT_TYPE_TABLE_ENTRY) {
+        if (!IsTableEntry (*Entry, Level)) {
           //
           // We are creating a new table entry, so on failure, we can free all
           // allocations we made recursively, given that the whole subhierarchy
@@ -265,10 +295,10 @@ UpdateRegionMappingRecursive (
         return Status;
       }
 
-      if ((*Entry & TT_TYPE_MASK) != TT_TYPE_TABLE_ENTRY) {
+      if (!IsTableEntry (*Entry, Level)) {
         EntryValue = (UINTN)TranslationTable | TT_TYPE_TABLE_ENTRY;
         ReplaceTableEntry (Entry, EntryValue, RegionStart,
-                           (*Entry & TT_TYPE_MASK) == TT_TYPE_BLOCK_ENTRY);
+          IsBlockEntry (*Entry, Level));
       }
     } else {
       EntryValue = (*Entry & AttributeClearMask) | AttributeSetMask;
-- 
2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#56303): https://edk2.groups.io/g/devel/message/56303
Mute This Topic: https://groups.io/mt/72543077/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Reviewed-by: Ashish Singhal <ashishsingha@nvidia.com>
Tested-by: Ashish Singhal <ashishsingha@nvidia.com>

________________________________
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Sent: Wednesday, March 25, 2020 9:29 AM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Laszlo Ersek <lersek@redhat.com>; Leif Lindholm <leif@nuviainc.com>; Ashish Singhal <ashishsingha@nvidia.com>
Subject: [PATCH v3 2/3] ArmPkg/ArmMmuLib AARCH64: use helpers to determine table entry types

External email: Use caution opening links or attachments


Given how the meaning of the attribute bits for page table entry types
is slightly awkward, and changes between levels, add some helpers to
abstract from this.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
---
 ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 40 +++++++++++++++++---
 1 file changed, 35 insertions(+), 5 deletions(-)

diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
index d78918cf7ba8..0680ba36d907 100644
--- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
@@ -162,6 +162,36 @@ FreePageTablesRecursive (
   FreePages (TranslationTable, 1);
 }

+STATIC
+BOOLEAN
+IsBlockEntry (
+  IN  UINT64  Entry,
+  IN  UINTN   Level
+  )
+{
+  if (Level == 3) {
+    return (Entry & TT_TYPE_MASK) == TT_TYPE_BLOCK_ENTRY_LEVEL3;
+  }
+  return (Entry & TT_TYPE_MASK) == TT_TYPE_BLOCK_ENTRY;
+}
+
+STATIC
+BOOLEAN
+IsTableEntry (
+  IN  UINT64  Entry,
+  IN  UINTN   Level
+  )
+{
+  if (Level == 3) {
+    //
+    // TT_TYPE_TABLE_ENTRY aliases TT_TYPE_BLOCK_ENTRY_LEVEL3
+    // so we need to take the level into account as well.
+    //
+    return FALSE;
+  }
+  return (Entry & TT_TYPE_MASK) == TT_TYPE_TABLE_ENTRY;
+}
+
 STATIC
 EFI_STATUS
 UpdateRegionMappingRecursive (
@@ -203,7 +233,7 @@ UpdateRegionMappingRecursive (
     if (Level == 0 || ((RegionStart | BlockEnd) & BlockMask) != 0) {
       ASSERT (Level < 3);

-      if ((*Entry & TT_TYPE_MASK) != TT_TYPE_TABLE_ENTRY) {
+      if (!IsTableEntry (*Entry, Level)) {
         //
         // No table entry exists yet, so we need to allocate a page table
         // for the next level.
@@ -221,7 +251,7 @@ UpdateRegionMappingRecursive (
           InvalidateDataCacheRange (TranslationTable, EFI_PAGE_SIZE);
         }

-        if ((*Entry & TT_TYPE_MASK) == TT_TYPE_BLOCK_ENTRY) {
+        if (IsBlockEntry (*Entry, Level)) {
           //
           // We are splitting an existing block entry, so we have to populate
           // the new table with the attributes of the block entry it replaces.
@@ -252,7 +282,7 @@ UpdateRegionMappingRecursive (
                  AttributeSetMask, AttributeClearMask, TranslationTable,
                  Level + 1);
       if (EFI_ERROR (Status)) {
-        if ((*Entry & TT_TYPE_MASK) != TT_TYPE_TABLE_ENTRY) {
+        if (!IsTableEntry (*Entry, Level)) {
           //
           // We are creating a new table entry, so on failure, we can free all
           // allocations we made recursively, given that the whole subhierarchy
@@ -265,10 +295,10 @@ UpdateRegionMappingRecursive (
         return Status;
       }

-      if ((*Entry & TT_TYPE_MASK) != TT_TYPE_TABLE_ENTRY) {
+      if (!IsTableEntry (*Entry, Level)) {
         EntryValue = (UINTN)TranslationTable | TT_TYPE_TABLE_ENTRY;
         ReplaceTableEntry (Entry, EntryValue, RegionStart,
-                           (*Entry & TT_TYPE_MASK) == TT_TYPE_BLOCK_ENTRY);
+          IsBlockEntry (*Entry, Level));
       }
     } else {
       EntryValue = (*Entry & AttributeClearMask) | AttributeSetMask;
--
2.17.1


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#56305): https://edk2.groups.io/g/devel/message/56305
Mute This Topic: https://groups.io/mt/72543077/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-