Re: [edk2-devel] [edk2-platforms][PATCH v1 4/5] MinPlatformPkg/TestPointCheckLib: Improve adjacent region checking

["Nate DeSimone" <nathaniel.l.desimone@intel.com>]

Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael
> Kubacki
> Sent: Thursday, August 5, 2021 7:57 AM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>
> Subject: [edk2-devel] [edk2-platforms][PATCH v1 4/5]
> MinPlatformPkg/TestPointCheckLib: Improve adjacent region checking
> 
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3521
> 
> The current logic depends on a particular order in which the descriptors for
> three or more regions are placed in the array to perform proper adjacency
> checking. When three or more regions are all adjacent, but neighboring
> descriptors are not adjacent, the logic can improperly report a failure. Adjust
> the logic so that all descriptors are checked for adjacency.
> 
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Eric Dong <eric.dong@intel.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> ---
> 
> Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckS
> mmInfo.c | 56 ++++++++++----------
>  1 file changed, 29 insertions(+), 27 deletions(-)
> 
> diff --git
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheck
> SmmInfo.c
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheck
> SmmInfo.c
> index c493750a27e6..f15f76eab574 100644
> ---
> a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheck
> SmmInfo.c
> +++
> b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh
> +++ eckSmmInfo.c
> @@ -59,34 +59,36 @@ CheckSmramDescriptor (
>    )
>  {
>    UINTN   Index;
> -  UINT64  Base;
> +  UINTN   Index2;
>    UINT64  Length;
> +  BOOLEAN AdjacentRegion;
> 
> -  Base = 0;
>    Length = 0;
>    for (Index = 0; Index < NumberOfSmmReservedRegions; Index++) {
> -    if (Base == 0) {
> -      Base   = Descriptor[Index].PhysicalStart;
> -      Length = Descriptor[Index].PhysicalSize;
> +    AdjacentRegion = FALSE;
> +    for (Index2 = 0; Index2 < NumberOfSmmReservedRegions; Index2++) {
> +      if ((NumberOfSmmReservedRegions == 1)
> +          || (Descriptor[Index].PhysicalStart + Descriptor[Index].PhysicalSize ==
> Descriptor[Index2].PhysicalStart)
> +          || (Descriptor[Index2].PhysicalStart + Descriptor[Index2].PhysicalSize
> == Descriptor[Index].PhysicalStart)) {
> +        AdjacentRegion = TRUE;
> +        break;
> +      }
> +    }
> +
> +    if (AdjacentRegion == TRUE) {
> +      Length += Descriptor[Index].PhysicalSize;
>      } else {
> -      if (Base + Length == Descriptor[Index].PhysicalStart) {
> -        Length = Length + Descriptor[Index].PhysicalSize;
> -      } else if (Descriptor[Index].PhysicalStart + Descriptor[Index].PhysicalSize
> == Base) {
> -        Base = Descriptor[Index].PhysicalStart;
> -        Length = Length + Descriptor[Index].PhysicalSize;
> -      } else {
> -        DEBUG ((DEBUG_ERROR, "Smram is not adjacent\n"));
> -        TestPointLibAppendErrorString (
> -          PLATFORM_TEST_POINT_ROLE_PLATFORM_IBV,
> -          NULL,
> -
> TEST_POINT_BYTE7_DXE_SMM_READY_TO_LOCK_SMRAM_ALIGNED_ERRO
> R_CODE \
> -            TEST_POINT_DXE_SMM_READY_TO_LOCK
> -
> TEST_POINT_BYTE7_DXE_SMM_READY_TO_LOCK_SMRAM_ALIGNED_ERRO
> R_STRING
> -          );
> -        return EFI_INVALID_PARAMETER;
> -      }
> +      DEBUG ((DEBUG_ERROR, "Smram is not adjacent\n"));
> +      TestPointLibAppendErrorString (
> +        PLATFORM_TEST_POINT_ROLE_PLATFORM_IBV,
> +        NULL,
> +
> TEST_POINT_BYTE7_DXE_SMM_READY_TO_LOCK_SMRAM_ALIGNED_ERRO
> R_CODE \
> +          TEST_POINT_DXE_SMM_READY_TO_LOCK
> +
> TEST_POINT_BYTE7_DXE_SMM_READY_TO_LOCK_SMRAM_ALIGNED_ERRO
> R_STRING
> +        );
> +      return EFI_INVALID_PARAMETER;
>      }
> -  }
> +  }
> 
>    if (Length != GetPowerOfTwo64 (Length)) {
>      DEBUG ((DEBUG_ERROR, "Smram is not aligned\n")); @@ -94,7 +96,7 @@
> CheckSmramDescriptor (
>        PLATFORM_TEST_POINT_ROLE_PLATFORM_IBV,
>        NULL,
> 
> TEST_POINT_BYTE7_DXE_SMM_READY_TO_LOCK_SMRAM_ALIGNED_ERRO
> R_CODE \
> -        TEST_POINT_DXE_SMM_READY_TO_LOCK
> +        TEST_POINT_DXE_SMM_READY_TO_LOCK
> 
> TEST_POINT_BYTE7_DXE_SMM_READY_TO_LOCK_SMRAM_ALIGNED_ERRO
> R_STRING
>        );
>      return EFI_INVALID_PARAMETER;
> @@ -111,14 +113,14 @@ TestPointCheckSmmInfo (
>    EFI_SMM_ACCESS2_PROTOCOL *SmmAccess;
>    UINTN                    Size;
>    EFI_SMRAM_DESCRIPTOR     *SmramRanges;
> -
> +
>    DEBUG ((DEBUG_INFO, "==== TestPointCheckSmmInfo - Enter\n"));
> -
> +
>    Status = gBS->LocateProtocol (&gEfiSmmAccess2ProtocolGuid, NULL, (VOID
> **)&SmmAccess);
>    if (EFI_ERROR (Status)) {
>      goto Done ;
>    }
> -
> +
>    Size = 0;
>    Status = SmmAccess->GetCapabilities (SmmAccess, &Size, NULL);
>    ASSERT (Status == EFI_BUFFER_TOO_SMALL); @@ -128,7 +130,7 @@
> TestPointCheckSmmInfo (
> 
>    Status = SmmAccess->GetCapabilities (SmmAccess, &Size, SmramRanges);
>    ASSERT_EFI_ERROR (Status);
> -
> +
>    DEBUG ((DEBUG_INFO, "SMRAM Info\n"));
>    DumpSmramDescriptor (Size / sizeof (EFI_SMRAM_DESCRIPTOR),
> SmramRanges);
> 
> --
> 2.28.0.windows.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#78714): https://edk2.groups.io/g/devel/message/78714
> Mute This Topic: https://groups.io/mt/84686308/1767664
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [nathaniel.l.desimone@intel.com]
> -=-=-=-=-=-=
>