Reviewed-by: Ray Ni <ray...@intel.com>
> -----Original Message-----
> From: Tan, Dun <dun....@intel.com>
> Sent: Friday, March 24, 2023 2:00 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.d...@intel.com>; Ni, Ray <ray...@intel.com>; Kumar,
> Rahul R <rahul.r.ku...@intel.com>; Gerd Hoffmann <kra...@redhat.com>;
> Liu, Zhiguang <zhiguang....@intel.com>
> Subject: [Patch V5 12/22] UefiCpuPkg/CpuPageTableLib:Modify RandomTest
> to check Mask/Attr
>
> Modify RandomTest to check invalid input. When creating new page
> table or updating exsiting page table:
> 1.If set [LinearAddress, LinearAddress+Length] to non-present, all
> other attributes should not be provided.
> 2.If [LinearAddress, LinearAddress+Length] contain non-present range,
> the Returnstatus of PageTableMap() should be InvalidParameter when:
> 2.1Some of attributes are not provided when mapping non-present range
> to present.
> 2.2Set any other attribute without setting the non-present range to
> Present.
>
> Signed-off-by: Dun Tan <dun....@intel.com>
> Cc: Eric Dong <eric.d...@intel.com>
> Cc: Ray Ni <ray...@intel.com>
> Cc: Rahul Kumar <rahul1.ku...@intel.com>
> Tested-by: Gerd Hoffmann <kra...@redhat.com>
> Acked-by: Gerd Hoffmann <kra...@redhat.com>
> Cc: Zhiguang Liu <zhiguang....@intel.com>
> ---
> UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c | 153
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++++++++++-------------------------
> UefiCpuPkg/Library/CpuPageTableLib/UnitTest/TestHelper.c | 6 +++++-
> 2 files changed, 133 insertions(+), 26 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c
> b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c
> index 612fddcee0..121cc4f2b2 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/RandomTest.c
> @@ -273,6 +273,27 @@ ValidateAndRandomeModifyPageTable (
> return Status;
> }
>
> +/**
> + Remove the last MAP_ENTRY in MapEntrys.
> +
> + @param MapEntrys Pointer to MapEntrys buffer
> +**/
> +VOID
> +RemoveLastMapEntry (
> + IN OUT MAP_ENTRYS *MapEntrys
> + )
> +{
> + UINTN MapsIndex;
> +
> + if (MapEntrys->Count == 0) {
> + return;
> + }
> +
> + MapsIndex = MapEntrys->Count - 1;
> + ZeroMem (&(MapEntrys->Maps[MapsIndex]), sizeof (MAP_ENTRY));
> + MapEntrys->Count = MapsIndex;
> +}
> +
> /**
> Generate single random map entry.
> The map entry can be the input of function PageTableMap
> @@ -327,7 +348,16 @@ GenerateSingleRandomMapEntry (
> MapEntrys->Maps[MapsIndex].Mask.Uint64 = MapEntrys-
> >Maps[Random32 (0, (UINT32)MapsIndex-1)].Mask.Uint64;
> } else {
> MapEntrys->Maps[MapsIndex].Attribute.Uint64 = Random64 (0,
> MAX_UINT64) & mSupportedBit.Uint64;
> - MapEntrys->Maps[MapsIndex].Mask.Uint64 = Random64 (0,
> MAX_UINT64) & mSupportedBit.Uint64;
> + if (RandomBoolean (5)) {
> + //
> + // The probability to get random Mask should be small since all bits
> of a
> random number
> + // have a high probability of containing 0, which may be a invalid
> input.
> + //
> + MapEntrys->Maps[MapsIndex].Mask.Uint64 = Random64 (0,
> MAX_UINT64) & mSupportedBit.Uint64;
> + } else {
> + MapEntrys->Maps[MapsIndex].Mask.Uint64 = MAX_UINT64;
> + }
> +
> if (MapEntrys->Maps[MapsIndex].Mask.Bits.ProtectionKey != 0) {
> MapEntrys->Maps[MapsIndex].Mask.Bits.ProtectionKey = 0xF;
> }
> @@ -337,15 +367,7 @@ GenerateSingleRandomMapEntry (
> MapEntrys->Maps[MapsIndex].Attribute.Bits.PageTableBaseAddress =
> MapEntrys->Maps[MapsIndex].LinearAddress >> 12;
> MapEntrys->Maps[MapsIndex].Mask.Bits.PageTableBaseAddress =
> 0xFFFFFFFFFF;
> } else {
> - //
> - // Todo: If the mask bit for base address is zero, when dump the
> pagetable, every entry mapping to physical address zeor.
> - // This means the map count will be a large number, and impossible
> to
> finish in proper time.
> - // Need to avoid such case when remove the Random option
> ONLY_ONE_ONE_MAPPING
> - //
> MapEntrys->Maps[MapsIndex].Attribute.Bits.PageTableBaseAddress =
> (Random64 (0, (((UINT64)1)<<52) - 1) & AlignedTable[Random32 (0,
> ARRAY_SIZE (AlignedTable) -1)])>> 12;
> - if (RandomBoolean (50)) {
> - MapEntrys->Maps[MapsIndex].Mask.Bits.PageTableBaseAddress = 0;
> - }
> }
>
> MapEntrys->Count += 1;
> @@ -608,25 +630,65 @@ SingleMapEntryTest (
> IN UINTN InitMapCount
> )
> {
> - UINTN MapsIndex;
> - RETURN_STATUS Status;
> - UINTN PageTableBufferSize;
> - VOID *Buffer;
> - IA32_MAP_ENTRY *Map;
> - UINTN MapCount;
> - UINTN Index;
> - UINTN KeyPointCount;
> - UINTN NewKeyPointCount;
> - UINT64 *KeyPointBuffer;
> - UINTN Level;
> - UINT64 Value;
> - UNIT_TEST_STATUS TestStatus;
> - MAP_ENTRY *LastMapEntry;
> -
> - MapsIndex = MapEntrys->Count;
> + UINTN MapsIndex;
> + RETURN_STATUS Status;
> + UINTN PageTableBufferSize;
> + VOID *Buffer;
> + IA32_MAP_ENTRY *Map;
> + UINTN MapCount;
> + UINTN Index;
> + UINTN KeyPointCount;
> + UINTN NewKeyPointCount;
> + UINT64 *KeyPointBuffer;
> + UINTN Level;
> + UINT64 Value;
> + UNIT_TEST_STATUS TestStatus;
> + MAP_ENTRY *LastMapEntry;
> + IA32_MAP_ATTRIBUTE *Mask;
> + IA32_MAP_ATTRIBUTE *Attribute;
> + UINT64 LastNotPresentRegionStart;
> + BOOLEAN IsNotPresent;
> +
> + MapsIndex = MapEntrys->Count;
> + MapCount = 0;
> + LastNotPresentRegionStart = 0;
> + IsNotPresent = FALSE;
>
> GenerateSingleRandomMapEntry (MaxAddress, MapEntrys);
> LastMapEntry = &MapEntrys->Maps[MapsIndex];
> + Status = PageTableParse (*PageTable, PagingMode, NULL, &MapCount);
> +
> + if (MapCount != 0) {
> + UT_ASSERT_EQUAL (Status, RETURN_BUFFER_TOO_SMALL);
> + Map = AllocatePages (EFI_SIZE_TO_PAGES (MapCount * sizeof
> (IA32_MAP_ENTRY)));
> + ASSERT (Map != NULL);
> + Status = PageTableParse (*PageTable, PagingMode, Map, &MapCount);
> + }
> +
> + //
> + // Check if the generated MapEntrys->Maps[MapsIndex] contains not-
> present range.
> + //
> + if (LastMapEntry->Length > 0) {
> + for (Index = 0; Index < MapCount; Index++) {
> + if ((LastNotPresentRegionStart < Map[Index].LinearAddress) &&
> + (LastMapEntry->LinearAddress < Map[Index].LinearAddress) &&
> (LastMapEntry->LinearAddress + LastMapEntry->Length >
> LastNotPresentRegionStart))
> + {
> + //
> + // MapEntrys->Maps[MapsIndex] contains not-present range in
> exsiting page table.
> + //
> + break;
> + }
> +
> + LastNotPresentRegionStart = Map[Index].LinearAddress +
> Map[Index].Length;
> + }
> +
> + //
> + // Either LastMapEntry overlaps with the not-present region in the very
> end
> + // Or it overlaps with one in the middle
> + if (LastNotPresentRegionStart < LastMapEntry->LinearAddress +
> LastMapEntry->Length) {
> + IsNotPresent = TRUE;
> + }
> + }
>
> PageTableBufferSize = 0;
> Status = PageTableMap (
> @@ -639,6 +701,47 @@ SingleMapEntryTest (
> &LastMapEntry->Attribute,
> &LastMapEntry->Mask
> );
> +
> + Attribute = &LastMapEntry->Attribute;
> + Mask = &LastMapEntry->Mask;
> + //
> + // If set [LinearAddress, LinearAddress+Attribute] to not preset, all
> + // other attributes should not be provided.
> + //
> + if ((LastMapEntry->Length > 0) && (Attribute->Bits.Present == 0) &&
> (Mask->Bits.Present == 1) && (Mask->Uint64 > 1)) {
> + RemoveLastMapEntry (MapEntrys);
> + UT_ASSERT_EQUAL (Status, RETURN_INVALID_PARAMETER);
> + return UNIT_TEST_PASSED;
> + }
> +
> + //
> + // Return Status for non-present range also should be InvalidParameter
> when:
> + // 1. Some of attributes are not provided when mapping non-present
> range to present.
> + // 2. Set any other attribute without setting the non-present range to
> Present.
> + //
> + if (IsNotPresent) {
> + if ((Mask->Bits.Present == 1) && (Attribute->Bits.Present == 1)) {
> + //
> + // Creating new page table or remapping non-present range to present.
> + //
> + if ((Mask->Bits.ReadWrite == 0) || (Mask->Bits.UserSupervisor == 0) ||
> (Mask->Bits.WriteThrough == 0) || (Mask->Bits.CacheDisabled == 0) ||
> + (Mask->Bits.Accessed == 0) || (Mask->Bits.Dirty == 0) || (Mask-
> >Bits.Pat == 0) || (Mask->Bits.Global == 0) ||
> + (Mask->Bits.PageTableBaseAddress == 0) || (Mask-
> >Bits.ProtectionKey == 0) || (Mask->Bits.Nx == 0))
> + {
> + RemoveLastMapEntry (MapEntrys);
> + UT_ASSERT_EQUAL (Status, RETURN_INVALID_PARAMETER);
> + return UNIT_TEST_PASSED;
> + }
> + } else if ((Mask->Bits.Present == 0) && (Mask->Uint64 > 1)) {
> + //
> + // Only change other attributes for non-present range is not permitted.
> + //
> + RemoveLastMapEntry (MapEntrys);
> + UT_ASSERT_EQUAL (Status, RETURN_INVALID_PARAMETER);
> + return UNIT_TEST_PASSED;
> + }
> + }
> +
> if (PageTableBufferSize != 0) {
> UT_ASSERT_EQUAL (Status, RETURN_BUFFER_TOO_SMALL);
>
> diff --git a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/TestHelper.c
> b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/TestHelper.c
> index 5bd70c0f65..10fdee2f94 100644
> --- a/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/TestHelper.c
> +++ b/UefiCpuPkg/Library/CpuPageTableLib/UnitTest/TestHelper.c
> @@ -1,7 +1,7 @@
> /** @file
> helper file for Unit tests of the CpuPageTableLib instance of the
> CpuPageTableLib class
>
> - Copyright (c) 2022, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2022 - 2023, Intel Corporation. All rights reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -171,6 +171,10 @@ IsPageTableValid (
> UNIT_TEST_STATUS Status;
> IA32_PAGING_ENTRY *PagingEntry;
>
> + if (PageTable == 0) {
> + return UNIT_TEST_PASSED;
> + }
> +
> if ((PagingMode == Paging32bit) || (PagingMode == PagingPae) ||
> (PagingMode >= PagingModeMax)) {
> //
> // 32bit paging is never supported.
> --
> 2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#101754): https://edk2.groups.io/g/devel/message/101754
Mute This Topic: https://groups.io/mt/97818234/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
