There is a hidden bug for below code:

(1 << BaseAddressAlignment) & *BlockEntrySize

>From disassembly code, we can the literal number 1 will be treated as
INT32 by compiler by default, and we'll get 0xFFFFFFFF80000000 when
BaseAddressAlignment is equal to 31. So we will always get 31 when
alignment is larger than 31.

    if ((1 << BaseAddressAlignment) & *BlockEntrySize) {
5224: f9404be0  ldr x0, [sp,#144]
5228: 2a0003e1  mov w1, w0
522c: 52800020  mov w0, #0x1                    // #1
5230: 1ac12000  lsl w0, w0, w1
5234: 93407c01  sxtw  x1, w0

The bug can be replayed on QEMU AARCH64; by adding some debug print,
we can see lots of level 1 tables created (for block of 1GB) even
when the region is large enough to use 512GB block size.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Heyi Guo <heyi....@linaro.org>
Cc: Leif Lindholm <leif.lindh...@linaro.org>
Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
---
 ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c 
b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
index e7b095c..b039ae1 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
@@ -295,14 +295,16 @@ GetBlockEntryListFromAddress (
   if (RegionStart == 0) {
     // Identify the highest possible alignment for the Region Size
     for (BaseAddressAlignment = 0; BaseAddressAlignment < 64; 
BaseAddressAlignment++) {
-      if ((1 << BaseAddressAlignment) & *BlockEntrySize) {
+      // Type casting is needed as literal number will be treated as INT32 by
+      // compiler
+      if (((UINT64) 1 << BaseAddressAlignment) & *BlockEntrySize) {
         break;
       }
     }
   } else {
     // Identify the highest possible alignment for the Base Address
     for (BaseAddressAlignment = 0; BaseAddressAlignment < 64; 
BaseAddressAlignment++) {
-      if ((1 << BaseAddressAlignment) & RegionStart) {
+      if (((UINT64) 1 << BaseAddressAlignment) & RegionStart) {
         break;
       }
     }
-- 
2.5.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to