On 6 September 2015 at 12:52, Ard Biesheuvel <[email protected]> wrote:
> On 6 September 2015 at 10:15, Heyi Guo <[email protected]> wrote:
>> 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 <[email protected]>
>> Cc: Leif Lindholm <[email protected]>
>> Cc: Ard Biesheuvel <[email protected]>
>> ---
>> 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) {
>
> Please use '1UL' instead of '(UINT64) 1'
Wouldn't 1ULL be a better choice?
To still work if someone was to copy code across to a 32-bit arch.
/
Leif
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
