Thanks, Ray. I will update patch to add the commit message you mentioned. Regards, Sunny Wang
-----Original Message----- From: Ni, Ruiyu [mailto:[email protected]] Sent: Wednesday, June 22, 2016 5:51 PM To: Wang, Sunny (HPS SW) <[email protected]>; [email protected] Cc: El-Haj-Mahmoud, Samer <[email protected]> Subject: RE: [PATCH] MdeModulePkg/UefiBootManagerLib: Fix for wrong data updated into MemoryTypeInformation variable Importance: High Sunny, I agree with this patch. After all the patch is an improvement though it cannot fix all the bugs. Please mention in the commit message that: But there is a corner case even when the memory bins don't include the RAM disk memory, the memory used by all other modules exceeds RamDiskSizeInPages. Reviewed-by: Ruiyu Ni <[email protected]> I will think about how to fix that corner case. Regards, Ray >-----Original Message----- >From: Sunny Wang [mailto:[email protected]] >Sent: Monday, June 20, 2016 4:03 PM >To: [email protected] >Cc: [email protected]; Ni, Ruiyu <[email protected]>; Sunny Wang ><[email protected]> >Subject: [PATCH] MdeModulePkg/UefiBootManagerLib: Fix for wrong data >updated into MemoryTypeInformation variable > >After booting a large-size ISO RAM disk (HTTP boot option pointing to a >ISO file) and rebooting the system, system will possibly run into the >following ASSERT because the BDS core code doesn't consider the case >that Memory page management >(Page.c) would possibly skip updating current memory usage statistics >(CurrentMemoryTypeInformation) if system allocates a memory buffer with >a large number of pages. For this case, the problematic BDS code block >in >BmSetMemoryTypeInformationVariable() function will get a >CurrentMemoryTypeInformation[Index1].NumberOfPages value which is not >updated with large-size ISO RAM disk usage by Memory page management >(it keeps original value without RAM disk size. For example, 0x9000) >and gets RamDiskSizeInPages (ISO RAM disk size) in a big value (For example, >0xC0000). Then, the result will become a very big value (0xFFF49000) to be >updated into MemoryTypeInformation variable to cause the ASSERT in next boot. > >ASSERT [DxeCore] u:\MdeModulePkg\Core\Dxe\Gcd\Gcd.c(2273): Length >= >MinimalMemorySizeNeeded > >For fixing this issue, we need to add a check to the problematic BDS >code block for checking that NumberOfPages is bigger than >RamDiskSizeInPages to prevent BDS core code updating wrong data (very big >value) to MemoryTypeInformation variable in this case. > >Contributed-under: TianoCore Contribution Agreement 1.0 >Signed-off-by: Sunny Wang <[email protected]> >--- > MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > >diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c >b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c >index 29c1bfa..93502fe 100644 >--- a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c >+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c >@@ -2,6 +2,7 @@ > Misc library functions. > > Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.<BR> >+(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR> > This program and the accompanying materials are licensed and made >available under the terms and conditions of the BSD License which >accompanies this distribution. The full text of the license may be >found at @@ -231,7 +232,8 @@ BmSetMemoryTypeInformationVariable ( > // > // Do not count the reserved memory occupied by RAM Disk. > // >- if (CurrentMemoryTypeInformation[Index1].Type == EfiReservedMemoryType) { >+ if ((CurrentMemoryTypeInformation[Index1].Type == EfiReservedMemoryType) >&& >+ (CurrentMemoryTypeInformation[Index1].NumberOfPages > >+ ((UINT32) RamDiskSizeInPages))) { > CurrentMemoryTypeInformation[Index1].NumberOfPages -= (UINT32) > RamDiskSizeInPages; > } > >-- >2.5.0.windows.1 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
