Heyi: Thanks for your catch issue. It only happens when the default alignment is different. Now, only Arm Arch64 uses the different alignments for the different memory types. So, you meet with this issue on QEMU AARCH64 UEFI. I agree your fix. Reviewed-by: Liming Gao <liming....@intel.com>
Btw: if you need my help to commit this change, I would like to do it. Thanks Liming -----Original Message----- From: Heyi Guo [mailto:heyi....@linaro.org] Sent: Monday, June 08, 2015 9:26 AM To: Tian, Feng Cc: edk2-devel@lists.sourceforge.net; Ilias Biris Subject: Re: [edk2] [PATCH] MdeModulePkg: Fix bug of AllocatePages for large alignment Hi Tian, Do you have any comments on this patch? Thanks! On 06/05/2015 09:57 AM, Heyi Guo wrote: > Hi Ard, > > I got this error when running SCT against QEMU AARCH64 UEFI. I think > it will only happen for aarch64 runtime services allocation, because > only this type of memory requires more than 4 pages alignment, which > will trigger the bug. If we use the same alignment (e.g. 64KB) for > both boot and runtime memory, it just avoid the bug indirectly, as > each descriptor would be kept naturally aligned with 64KB. > > Thanks. > > On 06/05/2015 12:08 AM, Ard Biesheuvel wrote: >> On 4 June 2015 at 17:55, Heyi Guo <heyi....@linaro.org> wrote: >>> DescEnd will be clipped for alignment in CoreFindFreePagesI, and it >>> may fall below DescStart, when alignment is more than 16KB >>> (included) and both DescStart and original DescEnd fall into a >>> single range of such alignment. This results in a huge size >>> (Negative number in unsigned type) for this descriptor, fulfilling >>> the allocation requirement but failing to run ConvertPages; at last >>> it causes >> at least >> >>> occasional failure of AllocatePages. >>> >>> A simple comparison is added to ensure we would never get a negative >>> number. >>> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Heyi Guo <heyi....@linaro.org> >> Acked-by: Ard Biesheuvel <ard.biesheu...@linaro.org> >> >> So is this only happening because we use different granularities for >> boottime and runtime allocations on AArch64? >> >> >>> --- >>> MdeModulePkg/Core/Dxe/Mem/Page.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c >>> b/MdeModulePkg/Core/Dxe/Mem/Page.c >>> index a92c865..f2efaf1 100644 >>> --- a/MdeModulePkg/Core/Dxe/Mem/Page.c >>> +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c >>> @@ -1051,6 +1051,11 @@ CoreFindFreePagesI ( >>> >>> DescEnd = ((DescEnd + 1) & (~(Alignment - 1))) - 1; >>> >>> + // Skip if DescEnd is less than DescStart after alignment clipping >>> + if (DescEnd < DescStart) { >>> + continue; >>> + } >>> + >>> // >>> // Compute the number of bytes we can used from this >>> // descriptor, and see it's enough to satisfy the request >>> -- >>> 2.1.4 >>> > ------------------------------------------------------------------------------ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel ------------------------------------------------------------------------------ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
