From: "Ni, Ray" <ray.ni@intel.com>
To: "Tan, Dun" <dun.tan@intel.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Dong, Eric" <eric.dong@intel.com>,
"Kumar, Rahul R" <rahul.r.kumar@intel.com>,
Gerd Hoffmann <kraxel@redhat.com>
Subject: Re: [Patch V5 14/22] UefiCpuPkg/CpuPageTableLib: Add OUTPUT IsModified parameter.
Date: Fri, 24 Mar 2023 08:08:11 +0000 [thread overview]
Message-ID: <MN6PR11MB824440AE0A52BE2332EEBBEB8C849@MN6PR11MB8244.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20230324060020.940-15-dun.tan@intel.com>
Reviewed-by: Ray Ni <ray.ni@intel.com>
> -----Original Message-----
> From: Tan, Dun <dun.tan@intel.com>
> Sent: Friday, March 24, 2023 2:00 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar,
> Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [Patch V5 14/22] UefiCpuPkg/CpuPageTableLib: Add OUTPUT
> IsModified parameter.
>
> Add OUTPUT IsModified parameter in PageTableMap() to indicate
> if page table has been modified. With this parameter, caller
> can know if need to call FlushTlb when the page table is in CR3.
>
> Signed-off-by: Dun Tan <dun.tan@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Tested-by: Gerd Hoffmann <kraxel@redhat.com>
> Acked-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> UefiCpuPkg/Include/Library/CpuPageTableLib.h | 4 +++-
> UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c | 46
> +++++++++++++++++++++++++++++++++++++++++-----
>
> UefiCpuPkg/Library/CpuPageTableLib/UnitTest/CpuPageTableLibUnitTestHo
> st.c | 72 ++++++++++++++++++++++++++++++++++++--------------------------
> ----------
> UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c | 6
> ++++--
> UefiCpuPkg/Library/MpInitLib/X64/CreatePageTable.c | 6 ++++-
> -
> 5 files changed, 88 insertions(+), 46 deletions(-)
>
> diff --git a/UefiCpuPkg/Include/Library/CpuPageTableLib.h
> b/UefiCpuPkg/Include/Library/CpuPageTableLib.h
> index 4ef4a8b6af..352b6df6c6 100644
> --- a/UefiCpuPkg/Include/Library/CpuPageTableLib.h
> +++ b/UefiCpuPkg/Include/Library/CpuPageTableLib.h
> @@ -74,6 +74,7 @@ typedef enum {
> Page table entries that map the linear address range are
> reset to 0 before set to the new attribute
> when a new physical base address is set.
> @param[in] Mask The mask used for attribute. The corresponding
> field in Attribute is ignored if that in Mask is 0.
> + @param[out] IsModified TRUE means page table is modified. FALSE
> means page table is not modified.
>
> @retval RETURN_UNSUPPORTED PagingMode is not supported.
> @retval RETURN_INVALID_PARAMETER PageTable, BufferSize, Attribute
> or Mask is NULL.
> @@ -97,7 +98,8 @@ PageTableMap (
> IN UINT64 LinearAddress,
> IN UINT64 Length,
> IN IA32_MAP_ATTRIBUTE *Attribute,
> - IN IA32_MAP_ATTRIBUTE *Mask
> + IN IA32_MAP_ATTRIBUTE *Mask,
> + OUT BOOLEAN *IsModified OPTIONAL
> );
>
> typedef struct {
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> index c0b41472ce..885f1601fc 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c
> @@ -274,6 +274,7 @@ IsAttributesAndMaskValidForNonPresentEntry (
> Page table entries that map the linear address range are
> reset to 0 before set to the new attribute
> when a new physical base address is set.
> @param[in] Mask The mask used for attribute. The corresponding
> field in Attribute is ignored if that in Mask is 0.
> + @param[out] IsModified TRUE means page table is modified. FALSE
> means page table is not modified.
>
> @retval RETURN_INVALID_PARAMETER For non-present range, Mask-
> >Bits.Present is 0 but some other attributes are provided.
> @retval RETURN_INVALID_PARAMETER For non-present range, Mask-
> >Bits.Present is 1, Attribute->Bits.Present is 1 but some other attributes are
> not provided.
> @@ -292,7 +293,8 @@ PageTableLibMapInLevel (
> IN UINT64 Length,
> IN UINT64 Offset,
> IN IA32_MAP_ATTRIBUTE *Attribute,
> - IN IA32_MAP_ATTRIBUTE *Mask
> + IN IA32_MAP_ATTRIBUTE *Mask,
> + OUT BOOLEAN *IsModified
> )
> {
> RETURN_STATUS Status;
> @@ -318,6 +320,8 @@ PageTableLibMapInLevel (
> IA32_MAP_ATTRIBUTE LocalParentAttribute;
> UINT64 PhysicalAddrInEntry;
> UINT64 PhysicalAddrInAttr;
> + IA32_PAGING_ENTRY OriginalParentPagingEntry;
> + IA32_PAGING_ENTRY OriginalCurrentPagingEntry;
>
> ASSERT (Level != 0);
> ASSERT ((Attribute != NULL) && (Mask != NULL));
> @@ -333,6 +337,8 @@ PageTableLibMapInLevel (
> LocalParentAttribute.Uint64 = ParentAttribute->Uint64;
> ParentAttribute = &LocalParentAttribute;
>
> + OriginalParentPagingEntry.Uint64 = ParentPagingEntry->Uint64;
> +
> //
> // RegionLength: 256T (1 << 48) 512G (1 << 39), 1G (1 << 30), 2M (1 << 21) or
> 4K (1 << 12).
> //
> @@ -568,7 +574,15 @@ PageTableLibMapInLevel (
> ASSERT (CreateNew || (Mask->Bits.Nx == 0) || (Attribute->Bits.Nx ==
> 1));
> }
>
> + //
> + // Check if any leaf PagingEntry is modified.
> + //
> + OriginalCurrentPagingEntry.Uint64 = CurrentPagingEntry->Uint64;
> PageTableLibSetPle (Level, CurrentPagingEntry, Offset, Attribute,
> &CurrentMask);
> +
> + if (OriginalCurrentPagingEntry.Uint64 != CurrentPagingEntry->Uint64) {
> + *IsModified = TRUE;
> + }
> }
> } else {
> //
> @@ -591,7 +605,8 @@ PageTableLibMapInLevel (
> Length,
> Offset,
> Attribute,
> - Mask
> + Mask,
> + IsModified
> );
> if (RETURN_ERROR (Status)) {
> return Status;
> @@ -603,6 +618,14 @@ PageTableLibMapInLevel (
> Index++;
> }
>
> + //
> + // Check if ParentPagingEntry entry is modified here is enough. Except the
> changes happen in leaf PagingEntry during
> + // the while loop, if there is any other change happens in page table, the
> ParentPagingEntry must has been modified.
> + //
> + if (OriginalParentPagingEntry.Uint64 != ParentPagingEntry->Uint64) {
> + *IsModified = TRUE;
> + }
> +
> return RETURN_SUCCESS;
> }
>
> @@ -623,6 +646,7 @@ PageTableLibMapInLevel (
> Page table entries that map the linear address range are
> reset to 0 before set to the new attribute
> when a new physical base address is set.
> @param[in] Mask The mask used for attribute. The corresponding
> field in Attribute is ignored if that in Mask is 0.
> + @param[out] IsModified TRUE means page table is modified. FALSE
> means page table is not modified.
>
> @retval RETURN_UNSUPPORTED PagingMode is not supported.
> @retval RETURN_INVALID_PARAMETER PageTable, BufferSize, Attribute
> or Mask is NULL.
> @@ -646,7 +670,8 @@ PageTableMap (
> IN UINT64 LinearAddress,
> IN UINT64 Length,
> IN IA32_MAP_ATTRIBUTE *Attribute,
> - IN IA32_MAP_ATTRIBUTE *Mask
> + IN IA32_MAP_ATTRIBUTE *Mask,
> + OUT BOOLEAN *IsModified OPTIONAL
> )
> {
> RETURN_STATUS Status;
> @@ -656,6 +681,7 @@ PageTableMap (
> IA32_PAGE_LEVEL MaxLevel;
> IA32_PAGE_LEVEL MaxLeafLevel;
> IA32_MAP_ATTRIBUTE ParentAttribute;
> + BOOLEAN LocalIsModified;
>
> if (Length == 0) {
> return RETURN_SUCCESS;
> @@ -718,6 +744,12 @@ PageTableMap (
> TopPagingEntry.Pce.Nx = 0;
> }
>
> + if (IsModified == NULL) {
> + IsModified = &LocalIsModified;
> + }
> +
> + *IsModified = FALSE;
> +
> ParentAttribute.Uint64 = 0;
> ParentAttribute.Bits.PageTableBaseAddress = 1;
> ParentAttribute.Bits.Present = 1;
> @@ -741,8 +773,10 @@ PageTableMap (
> Length,
> 0,
> Attribute,
> - Mask
> + Mask,
> + IsModified
> );
> + ASSERT (*IsModified == FALSE);
> if (RETURN_ERROR (Status)) {
> return Status;
> }
> @@ -773,8 +807,10 @@ PageTableMap (
> Length,
> 0,
> Attribute,
> - Mask
> + Mask,
> + IsModified
> );
> +
> if (!RETURN_ERROR (Status)) {
> *PageTable = (UINTN)(TopPagingEntry.Uintn &
> IA32_PE_BASE_ADDRESS_MASK_40);
> }
> diff --git
> a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/CpuPageTableLibUnitTest
> Host.c
> b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/CpuPageTableLibUnitTest
> Host.c
> index c682d4ea04..759da09271 100644
> ---
> a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/CpuPageTableLibUnitTest
> Host.c
> +++
> b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/CpuPageTableLibUnitTest
> Host.c
> @@ -51,26 +51,26 @@ TestCaseForParameter (
> //
> // If the input linear address is not 4K align, it should return invalid
> parameter
> //
> - UT_ASSERT_EQUAL (PageTableMap (&PageTable, PagingMode, &Buffer,
> &PageTableBufferSize, 1, SIZE_4KB, &MapAttribute, &MapMask),
> RETURN_INVALID_PARAMETER);
> + UT_ASSERT_EQUAL (PageTableMap (&PageTable, PagingMode, &Buffer,
> &PageTableBufferSize, 1, SIZE_4KB, &MapAttribute, &MapMask, NULL),
> RETURN_INVALID_PARAMETER);
>
> //
> // If the input PageTableBufferSize is not 4K align, it should return invalid
> parameter
> //
> PageTableBufferSize = 10;
> - UT_ASSERT_EQUAL (PageTableMap (&PageTable, PagingMode, &Buffer,
> &PageTableBufferSize, 0, SIZE_4KB, &MapAttribute, &MapMask),
> RETURN_INVALID_PARAMETER);
> + UT_ASSERT_EQUAL (PageTableMap (&PageTable, PagingMode, &Buffer,
> &PageTableBufferSize, 0, SIZE_4KB, &MapAttribute, &MapMask, NULL),
> RETURN_INVALID_PARAMETER);
>
> //
> // If the input PagingMode is Paging32bit, it should return invalid parameter
> //
> PageTableBufferSize = 0;
> PagingMode = Paging32bit;
> - UT_ASSERT_EQUAL (PageTableMap (&PageTable, PagingMode, &Buffer,
> &PageTableBufferSize, 1, SIZE_4KB, &MapAttribute, &MapMask),
> RETURN_UNSUPPORTED);
> + UT_ASSERT_EQUAL (PageTableMap (&PageTable, PagingMode, &Buffer,
> &PageTableBufferSize, 1, SIZE_4KB, &MapAttribute, &MapMask, NULL),
> RETURN_UNSUPPORTED);
>
> //
> // If the input MapMask is NULL, it should return invalid parameter
> //
> PagingMode = Paging5Level1GB;
> - UT_ASSERT_EQUAL (PageTableMap (&PageTable, PagingMode, &Buffer,
> &PageTableBufferSize, 1, SIZE_4KB, &MapAttribute, NULL),
> RETURN_INVALID_PARAMETER);
> + UT_ASSERT_EQUAL (PageTableMap (&PageTable, PagingMode, &Buffer,
> &PageTableBufferSize, 1, SIZE_4KB, &MapAttribute, NULL, NULL),
> RETURN_INVALID_PARAMETER);
>
> return UNIT_TEST_PASSED;
> }
> @@ -119,10 +119,10 @@ TestCaseWhichNoNeedExtraSize (
> //
> // Create page table to cover [0, 10M], it should have 5 PTE
> //
> - Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, 0, (UINT64)SIZE_2MB * 5, &MapAttribute,
> &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, 0, (UINT64)SIZE_2MB * 5, &MapAttribute,
> &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_BUFFER_TOO_SMALL);
> Buffer = AllocatePages (EFI_SIZE_TO_PAGES (PageTableBufferSize));
> - Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, 0, (UINT64)SIZE_2MB * 5, &MapAttribute,
> &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, 0, (UINT64)SIZE_2MB * 5, &MapAttribute,
> &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_SUCCESS);
> TestStatus = IsPageTableValid (PageTable, PagingMode);
> if (TestStatus != UNIT_TEST_PASSED) {
> @@ -134,7 +134,7 @@ TestCaseWhichNoNeedExtraSize (
> // We assume the fucntion doesn't need to change page table, return
> success and output BufferSize is 0
> //
> Buffer = NULL;
> - Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, 0, (UINT64)SIZE_4KB, &MapAttribute, &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, 0, (UINT64)SIZE_4KB, &MapAttribute, &MapMask,
> NULL);
> UT_ASSERT_EQUAL (PageTableBufferSize, 0);
> UT_ASSERT_EQUAL (Status, RETURN_SUCCESS);
> TestStatus = IsPageTableValid (PageTable, PagingMode);
> @@ -148,7 +148,7 @@ TestCaseWhichNoNeedExtraSize (
> //
> MapMask.Bits.Nx = 0;
> PageTableBufferSize = 0;
> - Status = PageTableMap (&PageTable, PagingMode, NULL,
> &PageTableBufferSize, 0, (UINT64)SIZE_4KB, &MapAttribute, &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, NULL,
> &PageTableBufferSize, 0, (UINT64)SIZE_4KB, &MapAttribute, &MapMask,
> NULL);
> UT_ASSERT_EQUAL (Status, RETURN_SUCCESS);
> UT_ASSERT_EQUAL (PageTableBufferSize, 0);
> TestStatus = IsPageTableValid (PageTable, PagingMode);
> @@ -164,7 +164,7 @@ TestCaseWhichNoNeedExtraSize (
> MapAttribute.Bits.Accessed = 1;
> MapMask.Bits.Accessed = 1;
> PageTableBufferSize = 0;
> - Status = PageTableMap (&PageTable, PagingMode, NULL,
> &PageTableBufferSize, (UINT64)SIZE_2MB, (UINT64)SIZE_2MB,
> &MapAttribute, &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, NULL,
> &PageTableBufferSize, (UINT64)SIZE_2MB, (UINT64)SIZE_2MB,
> &MapAttribute, &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_SUCCESS);
> UT_ASSERT_EQUAL (PageTableBufferSize, 0);
> TestStatus = IsPageTableValid (PageTable, PagingMode);
> @@ -217,10 +217,10 @@ TestCase1Gmapto4K (
> MapAttribute.Bits.Present = 1;
> MapMask.Bits.Present = 1;
> MapMask.Uint64 = MAX_UINT64;
> - Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, (UINT64)0, (UINT64)SIZE_1GB, &MapAttribute,
> &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, (UINT64)0, (UINT64)SIZE_1GB, &MapAttribute,
> &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_BUFFER_TOO_SMALL);
> Buffer = AllocatePages (EFI_SIZE_TO_PAGES (PageTableBufferSize));
> - Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, (UINT64)0, (UINT64)SIZE_1GB, &MapAttribute,
> &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, (UINT64)0, (UINT64)SIZE_1GB, &MapAttribute,
> &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_SUCCESS);
>
> //
> @@ -281,11 +281,11 @@ TestCaseManualChangeReadWrite (
> //
> // Create Page table to cover [0,2G], with ReadWrite = 1
> //
> - Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, 0, SIZE_2GB, &MapAttribute, &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, 0, SIZE_2GB, &MapAttribute, &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_BUFFER_TOO_SMALL);
> BackupPageTableBufferSize = PageTableBufferSize;
> Buffer = AllocatePages (EFI_SIZE_TO_PAGES
> (PageTableBufferSize));
> - Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, 0, SIZE_2GB, &MapAttribute, &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, 0, SIZE_2GB, &MapAttribute, &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_SUCCESS);
> IsPageTableValid (PageTable, PagingMode);
>
> @@ -331,7 +331,7 @@ TestCaseManualChangeReadWrite (
> // Call library to change ReadWrite to 0 for [0,2M]
> //
> MapAttribute.Bits.ReadWrite = 0;
> - Status = PageTableMap (&PageTable, PagingMode, NULL,
> &PageTableBufferSize, 0, SIZE_2MB, &MapAttribute, &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, NULL,
> &PageTableBufferSize, 0, SIZE_2MB, &MapAttribute, &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_SUCCESS);
> IsPageTableValid (PageTable, PagingMode);
> MapCount = 0;
> @@ -360,7 +360,7 @@ TestCaseManualChangeReadWrite (
> //
> MapAttribute.Bits.ReadWrite = 1;
> PageTableBufferSize = 0;
> - Status = PageTableMap (&PageTable, PagingMode, NULL,
> &PageTableBufferSize, 0, SIZE_2MB, &MapAttribute, &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, NULL,
> &PageTableBufferSize, 0, SIZE_2MB, &MapAttribute, &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_SUCCESS);
> IsPageTableValid (PageTable, PagingMode);
> MapCount = 0;
> @@ -434,10 +434,10 @@ TestCaseManualSizeNotMatch (
> //
> // Create Page table to cover [2M-4K, 4M], with ReadWrite = 1
> //
> - Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, SIZE_2MB - SIZE_4KB, SIZE_4KB + SIZE_2MB,
> &MapAttribute, &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, SIZE_2MB - SIZE_4KB, SIZE_4KB + SIZE_2MB,
> &MapAttribute, &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_BUFFER_TOO_SMALL);
> Buffer = AllocatePages (EFI_SIZE_TO_PAGES (PageTableBufferSize));
> - Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, SIZE_2MB - SIZE_4KB, SIZE_4KB + SIZE_2MB,
> &MapAttribute, &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, SIZE_2MB - SIZE_4KB, SIZE_4KB + SIZE_2MB,
> &MapAttribute, &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_SUCCESS);
> IsPageTableValid (PageTable, PagingMode);
>
> @@ -493,7 +493,7 @@ TestCaseManualSizeNotMatch (
> MapAttribute.Bits.ReadWrite = 1;
> PageTableBufferSize = 0;
> MapAttribute.Bits.PageTableBaseAddress = (SIZE_2MB - SIZE_4KB) >> 12;
> - Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, SIZE_2MB - SIZE_4KB, SIZE_4KB * 2, &MapAttribute,
> &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, SIZE_2MB - SIZE_4KB, SIZE_4KB * 2, &MapAttribute,
> &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_SUCCESS);
> return UNIT_TEST_PASSED;
> }
> @@ -540,10 +540,10 @@ TestCaseManualNotMergeEntry (
> //
> // Create Page table to cover [0,4M], and [4M, 1G] is not present
> //
> - Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, (UINT64)0, (UINT64)SIZE_2MB * 2, &MapAttribute,
> &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, (UINT64)0, (UINT64)SIZE_2MB * 2, &MapAttribute,
> &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_BUFFER_TOO_SMALL);
> Buffer = AllocatePages (EFI_SIZE_TO_PAGES (PageTableBufferSize));
> - Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, (UINT64)0, (UINT64)SIZE_2MB * 2, &MapAttribute,
> &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, (UINT64)0, (UINT64)SIZE_2MB * 2, &MapAttribute,
> &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_SUCCESS);
> TestStatus = IsPageTableValid (PageTable, PagingMode);
> if (TestStatus != UNIT_TEST_PASSED) {
> @@ -555,7 +555,7 @@ TestCaseManualNotMergeEntry (
> // It looks like the chioce is not bad, but sometime, we need to keep some
> small entry
> //
> PageTableBufferSize = 0;
> - Status = PageTableMap (&PageTable, PagingMode, NULL,
> &PageTableBufferSize, (UINT64)0, (UINT64)SIZE_1GB, &MapAttribute,
> &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, NULL,
> &PageTableBufferSize, (UINT64)0, (UINT64)SIZE_1GB, &MapAttribute,
> &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_SUCCESS);
> TestStatus = IsPageTableValid (PageTable, PagingMode);
> if (TestStatus != UNIT_TEST_PASSED) {
> @@ -564,7 +564,7 @@ TestCaseManualNotMergeEntry (
>
> MapAttribute.Bits.Accessed = 1;
> PageTableBufferSize = 0;
> - Status = PageTableMap (&PageTable, PagingMode, NULL,
> &PageTableBufferSize, (UINT64)0, (UINT64)SIZE_2MB, &MapAttribute,
> &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, NULL,
> &PageTableBufferSize, (UINT64)0, (UINT64)SIZE_2MB, &MapAttribute,
> &MapMask, NULL);
> //
> // If it didn't use a big 1G entry to cover whole range, only change [0,2M]
> for some attribute won't need extra memory
> //
> @@ -619,10 +619,10 @@ TestCaseManualChangeNx (
> //
> // Create Page table to cover [0,2G], with Nx = 0
> //
> - Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, (UINT64)0, (UINT64)SIZE_1GB * 2, &MapAttribute,
> &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, (UINT64)0, (UINT64)SIZE_1GB * 2, &MapAttribute,
> &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_BUFFER_TOO_SMALL);
> Buffer = AllocatePages (EFI_SIZE_TO_PAGES (PageTableBufferSize));
> - Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, (UINT64)0, (UINT64)SIZE_1GB * 2, &MapAttribute,
> &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, (UINT64)0, (UINT64)SIZE_1GB * 2, &MapAttribute,
> &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_SUCCESS);
> TestStatus = IsPageTableValid (PageTable, PagingMode);
> if (TestStatus != UNIT_TEST_PASSED) {
> @@ -666,7 +666,7 @@ TestCaseManualChangeNx (
> //
> // Call library to change Nx to 0 for [0,1G]
> //
> - Status = PageTableMap (&PageTable, PagingMode, NULL,
> &PageTableBufferSize, (UINT64)0, (UINT64)SIZE_1GB, &MapAttribute,
> &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, NULL,
> &PageTableBufferSize, (UINT64)0, (UINT64)SIZE_1GB, &MapAttribute,
> &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_SUCCESS);
> TestStatus = IsPageTableValid (PageTable, PagingMode);
> if (TestStatus != UNIT_TEST_PASSED) {
> @@ -741,30 +741,30 @@ TestCaseToCheckMapMaskAndAttr (
> //
> // Create Page table to cover [0, 2G]. All fields of MapMask should be set.
> //
> - Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, 0, SIZE_2GB, &MapAttribute, &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, 0, SIZE_2GB, &MapAttribute, &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_INVALID_PARAMETER);
> MapMask.Uint64 = MAX_UINT64;
> - Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, 0, SIZE_2GB, &MapAttribute, &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, 0, SIZE_2GB, &MapAttribute, &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_BUFFER_TOO_SMALL);
> Buffer = AllocatePages (EFI_SIZE_TO_PAGES (PageTableBufferSize));
> - Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, 0, SIZE_2GB, &MapAttribute, &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, 0, SIZE_2GB, &MapAttribute, &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_SUCCESS);
>
> //
> // Update Page table to set [2G - 8K, 2G] from present to non-present. All
> fields of MapMask except present should not be set.
> //
> PageTableBufferSize = 0;
> - MapAttribute.Uint64 = SIZE_2GB - SIZE_8KB;
> + MapAttribute.Uint64 = 0;
> MapMask.Uint64 = 0;
> MapMask.Bits.Present = 1;
> MapMask.Bits.ReadWrite = 1;
> - Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, SIZE_2GB - SIZE_8KB, SIZE_8KB, &MapAttribute,
> &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, SIZE_2GB - SIZE_8KB, SIZE_8KB, &MapAttribute,
> &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_INVALID_PARAMETER);
> MapMask.Bits.ReadWrite = 0;
> - Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, SIZE_2GB - SIZE_8KB, SIZE_8KB, &MapAttribute,
> &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, SIZE_2GB - SIZE_8KB, SIZE_8KB, &MapAttribute,
> &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_BUFFER_TOO_SMALL);
> Buffer = AllocatePages (EFI_SIZE_TO_PAGES (PageTableBufferSize));
> - Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, 0, SIZE_2GB, &MapAttribute, &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, SIZE_2GB - SIZE_8KB, SIZE_8KB, &MapAttribute,
> &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_SUCCESS);
>
> //
> @@ -774,11 +774,11 @@ TestCaseToCheckMapMaskAndAttr (
> MapAttribute.Uint64 = 0;
> MapMask.Uint64 = 0;
> MapMask.Bits.Present = 1;
> - Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, SIZE_2GB - SIZE_8KB, SIZE_8KB, &MapAttribute,
> &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, SIZE_2GB - SIZE_8KB, SIZE_8KB, &MapAttribute,
> &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_SUCCESS);
> MapAttribute.Bits.ReadWrite = 1;
> MapMask.Bits.ReadWrite = 1;
> - Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, SIZE_2GB - SIZE_8KB, SIZE_8KB, &MapAttribute,
> &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, SIZE_2GB - SIZE_8KB, SIZE_8KB, &MapAttribute,
> &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_INVALID_PARAMETER);
>
> //
> @@ -791,10 +791,10 @@ TestCaseToCheckMapMaskAndAttr (
> MapMask.Uint64 = 0;
> MapMask.Bits.ReadWrite = 1;
> MapMask.Bits.Present = 1;
> - Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, SIZE_2GB - SIZE_8KB, SIZE_8KB, &MapAttribute,
> &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, SIZE_2GB - SIZE_8KB, SIZE_8KB, &MapAttribute,
> &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_INVALID_PARAMETER);
> MapMask.Uint64 = MAX_UINT64;
> - Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, SIZE_2GB - SIZE_8KB, SIZE_8KB, &MapAttribute,
> &MapMask);
> + Status = PageTableMap (&PageTable, PagingMode, Buffer,
> &PageTableBufferSize, SIZE_2GB - SIZE_8KB, SIZE_8KB, &MapAttribute,
> &MapMask, NULL);
> UT_ASSERT_EQUAL (Status, RETURN_SUCCESS);
>
> MapCount = 0;
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c
> b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c
> index 121cc4f2b2..e603dba269 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c
> @@ -699,7 +699,8 @@ SingleMapEntryTest (
> LastMapEntry->LinearAddress,
> LastMapEntry->Length,
> &LastMapEntry->Attribute,
> - &LastMapEntry->Mask
> + &LastMapEntry->Mask,
> + NULL
> );
>
> Attribute = &LastMapEntry->Attribute;
> @@ -759,7 +760,8 @@ SingleMapEntryTest (
> LastMapEntry->LinearAddress,
> LastMapEntry->Length,
> &LastMapEntry->Attribute,
> - &LastMapEntry->Mask
> + &LastMapEntry->Mask,
> + NULL
> );
> }
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/X64/CreatePageTable.c
> b/UefiCpuPkg/Library/MpInitLib/X64/CreatePageTable.c
> index f20068152b..da8729e752 100644
> --- a/UefiCpuPkg/Library/MpInitLib/X64/CreatePageTable.c
> +++ b/UefiCpuPkg/Library/MpInitLib/X64/CreatePageTable.c
> @@ -57,7 +57,8 @@ CreatePageTable (
> Address,
> Length,
> &MapAttribute,
> - &MapMask
> + &MapMask,
> + NULL
> );
> ASSERT (Status == EFI_BUFFER_TOO_SMALL);
> DEBUG ((DEBUG_INFO, "AP Page Table Buffer Size = %x\n",
> PageTableBufferSize));
> @@ -72,7 +73,8 @@ CreatePageTable (
> Address,
> Length,
> &MapAttribute,
> - &MapMask
> + &MapMask,
> + NULL
> );
> ASSERT_EFI_ERROR (Status);
> return PageTable;
> --
> 2.31.1.windows.1
next prev parent reply other threads:[~2023-03-24 8:08 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-24 5:59 [Patch V5 00/22] Fix issues in CpuPageTableLib duntan
2023-03-24 5:59 ` [Patch V5 01/22] UefiCpuPkg/CpuPageTableLib: Remove unneeded 'if' condition duntan
2023-03-24 6:00 ` [Patch V5 02/22] UefiCpuPkg/CpuPageTableLib: Add check for input Length duntan
2023-03-24 6:00 ` [Patch V5 03/22] UefiCpuPkg/CpuPageTableLib:Initialize some LocalVariable at beginning duntan
2023-03-24 6:00 ` [Patch V5 04/22] UefiCpuPkg/CpuPageTableLib: Fix the non-1:1 mapping issue duntan
2023-03-24 6:00 ` [Patch V5 05/22] UefiCpuPkg/CpuPageTableLib:Clear PageSize bit(Bit7) for non-leaf duntan
2023-03-24 6:00 ` [Patch V5 06/22] UefiCpuPkg/CpuPageTableLib: Fix issue when splitting leaf entry duntan
2023-03-24 6:00 ` [Patch V5 07/22] UefiCpuPkg/MpInitLib: Add code to initialize MapMask duntan
2023-03-24 6:00 ` [Patch V5 08/22] UefiCpuPkg/CpuPageTableLib:Add check for Mask and Attr duntan
2023-03-24 8:06 ` Ni, Ray
2023-03-24 6:00 ` [Patch V5 09/22] UefiCpuPkg/CpuPageTableLib: Add manual test to check " duntan
2023-03-24 8:06 ` Ni, Ray
2023-03-24 6:00 ` [Patch V5 10/22] UefiCpuPkg/CpuPageTableLib:Modify RandomBoolean() in RandomTest duntan
2023-03-24 6:00 ` [Patch V5 11/22] UefiCpuPkg/CpuPageTableLib: Add LastMapEntry pointer duntan
2023-03-24 8:07 ` Ni, Ray
2023-03-24 6:00 ` [Patch V5 12/22] UefiCpuPkg/CpuPageTableLib:Modify RandomTest to check Mask/Attr duntan
2023-03-24 8:07 ` Ni, Ray
2023-03-24 6:00 ` [Patch V5 13/22] UefiCpuPkg/CpuPageTableLib: Enable non-1:1 mapping in random test duntan
2023-03-24 6:00 ` [Patch V5 14/22] UefiCpuPkg/CpuPageTableLib: Add OUTPUT IsModified parameter duntan
2023-03-24 8:08 ` Ni, Ray [this message]
2023-03-24 6:00 ` [Patch V5 15/22] UefiCpuPkg/CpuPageTableLib: Modify RandomTest to check IsModified duntan
2023-03-24 6:00 ` [Patch V5 16/22] UefiCpuPkg: Fix IA32 build failure in CpuPageTableLib.inf duntan
2023-03-24 6:00 ` [Patch V5 17/22] UefiCpuPkg: Modify UnitTest code since tested API is changed duntan
2023-03-24 6:00 ` [Patch V5 18/22] UefiCpuPkg/CpuPageTableLib: Add check for page table creation duntan
2023-03-24 6:00 ` [Patch V5 19/22] UefiCpuPkg: Combine branch for non-present and leaf ParentEntry duntan
2023-03-24 8:09 ` Ni, Ray
2023-03-24 6:00 ` [Patch V5 20/22] UefiCpuPkg/CpuPageTableLib: Enable PAE paging duntan
2023-03-24 8:10 ` Ni, Ray
2023-03-24 6:00 ` [Patch V5 21/22] UefiCpuPkg/CpuPageTableLib: Add RandomTest for " duntan
2023-03-24 6:00 ` [Patch V5 22/22] UefiCpuPkg/CpuPageTableLib: Reduce the number of random tests duntan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=MN6PR11MB824440AE0A52BE2332EEBBEB8C849@MN6PR11MB8244.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox