Re: [Patch V2 10/14] UefiCpuPkg/CpuPageTableLib: Modify RandomTest to check IsModified

["duntan" <dun.tan@intel.com>]

Thanks for the comments. I'll split this patch and remove the unneeded 'if' condition check.

Thanks,
Dun
-----Original Message-----
From: Ni, Ray <ray.ni@intel.com> 
Sent: Wednesday, March 15, 2023 2:09 PM
To: Tan, Dun <dun.tan@intel.com>; devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Liu, Zhiguang <zhiguang.liu@intel.com>
Subject: RE: [Patch V2 10/14] UefiCpuPkg/CpuPageTableLib: Modify RandomTest to check IsModified

> 
>    GenerateSingleRandomMapEntry (MaxAddress, MapEntrys);
>    Status = PageTableParse (*PageTable, PagingMode, NULL, &MapCount);
> 
> +  if (MapCount != 0) {
> +    //
> +    // Allocate memory for Map
> +    // Note the memory is only used in this one Single MapEntry Test
> +    //
> +    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 (MapEntrys->Maps[MapsIndex].Length > 0) {
>      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);
> -
>        if (Map[MapCount - 1].LinearAddress + Map[MapCount - 1].Length 
> <
> MapEntrys->Maps[MapsIndex].LinearAddress + MapEntrys-
> >Maps[MapsIndex].Length) {
>          IsNotPresent = TRUE;

1. can you split the Parse() API call into a standalone patch?

> +    if (MapCount2 == 0) {
> +      UT_ASSERT_EQUAL (IsModified, FALSE);
2. no need to treat "MapCount == 0" as special. CompareMem() should be able to accept 0-length bytes.

> +    } else if (CompareMem (Map, Map2, MapCount2 * sizeof
> (IA32_MAP_ENTRY)) != 0) {
> +      UT_ASSERT_EQUAL (IsModified, TRUE);
> +    } else {
> +      UT_ASSERT_EQUAL (IsModified, FALSE);
> +    }
>    }