On 10/23/18 16:53, Jian J Wang wrote: >> v2 changes: >> a. Update IsHeapGuardEnabled() calling because its prototype change > > Two changes are fixed up: > a. Prototype change of function IsHeapGuardEnabled()
This kind of separation, between the patches, is wrong then. If someone bisects the edk2 git history, and checks out the edk2 tree at patch #4 in this series, they will get a build failure. > b. More memory map descriptors are introduced by new feature. > MergeMemoryMap() is updated to merge freed-pages into adjacent memory > descriptor to reduce the overall descriptor number. In such cases, the usual way to structure the patch series is as follows: - First the patch is added that makes the dependent / consumer code more generic, so that it can later cope with the new feature. Right after this "prep" patch, the new code paths in the consumer code are not exercised in practice. Importantly however, there is neither a compilation failure, nor a functionality error. It's just that the new additions are not active yet; they work as NOPs. - Second, the patch with the new feature is added; it basically "snaps in place", and activates the code paths that were prepared earlier. In large patch sets, it's not uncommon to see 5+ "prep" patches, each equipping a separate aspect (or consumer site) for the new feature, gradually. And then the final feature patch is plugged in. Thanks Laszlo > > Cc: Star Zeng <[email protected]> > Cc: Michael D Kinney <[email protected]> > Cc: Jiewen Yao <[email protected]> > Cc: Ruiyu Ni <[email protected]> > Cc: Laszlo Ersek <[email protected]> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jian J Wang <[email protected]> > --- > MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 2 +- > MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 16 +++++++--------- > 2 files changed, 8 insertions(+), 10 deletions(-) > > diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c > b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c > index fa8f8fe91a..6298b67db1 100644 > --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c > +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c > @@ -1250,7 +1250,7 @@ ApplyMemoryProtectionPolicy ( > // Don't overwrite Guard pages, which should be the first and/or last page, > // if any. > // > - if (IsHeapGuardEnabled ()) { > + if (IsHeapGuardEnabled (GUARD_HEAP_TYPE_PAGE|GUARD_HEAP_TYPE_POOL)) { > if (IsGuardPage (Memory)) { > Memory += EFI_PAGE_SIZE; > Length -= EFI_PAGE_SIZE; > diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c > b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c > index 05eb4f422b..f6595c90ed 100644 > --- a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c > +++ b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c > @@ -32,6 +32,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER > EXPRESS OR IMPLIED. > #include <Guid/PropertiesTable.h> > > #include "DxeMain.h" > +#include "HeapGuard.h" > > #define PREVIOUS_MEMORY_DESCRIPTOR(MemoryDescriptor, Size) \ > ((EFI_MEMORY_DESCRIPTOR *)((UINT8 *)(MemoryDescriptor) - (Size))) > @@ -205,16 +206,13 @@ MergeMemoryMap ( > NextMemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry, > DescriptorSize); > > do { > - MemoryBlockLength = (UINT64) (EfiPagesToSize > (MemoryMapEntry->NumberOfPages)); > + MergeGuardPages (NewMemoryMapEntry, NextMemoryMapEntry->PhysicalStart); > + MemoryBlockLength = (UINT64) (EfiPagesToSize > (NewMemoryMapEntry->NumberOfPages)); > if (((UINTN)NextMemoryMapEntry < (UINTN)MemoryMapEnd) && > - (MemoryMapEntry->Type == NextMemoryMapEntry->Type) && > - (MemoryMapEntry->Attribute == NextMemoryMapEntry->Attribute) && > - ((MemoryMapEntry->PhysicalStart + MemoryBlockLength) == > NextMemoryMapEntry->PhysicalStart)) { > - MemoryMapEntry->NumberOfPages += NextMemoryMapEntry->NumberOfPages; > - if (NewMemoryMapEntry != MemoryMapEntry) { > - NewMemoryMapEntry->NumberOfPages += > NextMemoryMapEntry->NumberOfPages; > - } > - > + (NewMemoryMapEntry->Type == NextMemoryMapEntry->Type) && > + (NewMemoryMapEntry->Attribute == NextMemoryMapEntry->Attribute) && > + ((NewMemoryMapEntry->PhysicalStart + MemoryBlockLength) == > NextMemoryMapEntry->PhysicalStart)) { > + NewMemoryMapEntry->NumberOfPages += > NextMemoryMapEntry->NumberOfPages; > NextMemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (NextMemoryMapEntry, > DescriptorSize); > continue; > } else { > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
