> + PageTableBase = AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64; > + > + // > + // ConvertMemoryPageAttributes might update mPageTablePool. It's safer to > + // remember original one in advance. > + // > + HeadPool = mPageTablePool; > + Pool = HeadPool; > + do { > + Address = (EFI_PHYSICAL_ADDRESS)(UINTN)Pool & > PAGE_TABLE_POOL_ALIGN_MASK;
1. When is the Pool not aligned on 128KB boundary? If it's guaranteed, can we remove the "& PAGE_TABLE_POOL_ALIGN_MASK"? > + PoolSize = Pool->Offset + EFI_PAGES_TO_SIZE (Pool->FreePages); > + > + ConvertMemoryPageAttributes (PageTableBase, m5LevelPagingNeeded, > Address, PoolSize, EFI_MEMORY_RO, TRUE, > NULL, NULL); 2. Can you please explain in comments that above call is to make the entire pool including header, used-memory, free-memory as read-only? 3. It's better to use LinkedList library APIs from BaseLib. The comments apply to the first patch as well. But I am fine if you decide not to do it in this patch. > + { > + if (sizeof (UINTN) == sizeof (UINT64)) { > + // > + // Restriction on access to non-SMRAM memory and heap guard could not > be enabled at the same time. > + // > + ASSERT ( > + !(IsRestrictedMemoryAccess () && > + (PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) != 0) > + ); > + > + // > + // Restriction on access to non-SMRAM memory and SMM profile could not > be enabled at the same time. > + // > + ASSERT (!(IsRestrictedMemoryAccess () && FeaturePcdGet > (PcdCpuSmmProfileEnable))); > + } 4. I don't think we still need the above two assertions. But let's not clean up the code in your patch. @Wang, Jian J -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#97553): https://edk2.groups.io/g/devel/message/97553 Mute This Topic: https://groups.io/mt/95703349/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-