public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
To: Eric Jin <eric.jin@intel.com>, edk2-devel@lists.01.org
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Subject: Re: [PATCH] uefi-sct/SctPkg:Enhance the EraseBlock Test
Date: Mon, 15 Oct 2018 03:43:06 +0100	[thread overview]
Message-ID: <a0a379ac-b249-a5f0-1aa2-d50eb86e34ae@arm.com> (raw)
In-Reply-To: <20181014162639.12728-1-eric.jin@intel.com>



On 10/14/2018 05:26 PM, Eric Jin wrote:
> The EraseSize in the EraseBlocks conf test should be bytes.
> Cover the case that the size of the data to erase is a
> multiple of the 'EraseLengthGranularity' value of an Erase Block
> Protocol instance. And check whether the data on adjacent blocks
> are mistakenly erased.
>
> Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Jin <eric.jin@intel.com>
> ---
>   .../EraseBlockBBTestConformance.c             |  25 +-
>   .../BlackBoxTest/EraseBlockBBTestFunction.c   | 600 ++++++++++++++++--
>   .../BlackBoxTest/EraseBlockBBTestMain.h       |  16 +-
>   .../Protocol/EraseBlock/BlackBoxTest/Guid.c   |   4 +-
>   .../Protocol/EraseBlock/BlackBoxTest/Guid.h   |   7 +
>   5 files changed, 589 insertions(+), 63 deletions(-)
>
> diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestConformance.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestConformance.c
> index df057b26..7e848239 100644
> --- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestConformance.c
> +++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestConformance.c
> @@ -1,7 +1,7 @@
>   /** @file
>   
>     Copyright 2017 Unified EFI, Inc.<BR>
> -  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2017 - 2018, Intel Corporation. All rights reserved.<BR>
>   
>     This program and the accompanying materials
>     are licensed and made available under the terms and conditions of the BSD License
> @@ -51,8 +51,8 @@ BBTestEraseBlocksConformanceTest (
>     UINT32                                BlockSize;
>     UINT32                                IoAlign;
>     EFI_LBA                               LastBlock;
> -
> -  UINT32                                BlockNumber;
> +  UINT32                                EraseLengthGranularity;
> +  UINTN                                 EraseSize;
>   
>     EFI_ERASE_BLOCK_TOKEN                 Token;
>   
> @@ -121,10 +121,11 @@ BBTestEraseBlocksConformanceTest (
>     IoAlign           = Media->IoAlign;
>     LastBlock         = Media->LastBlock;
>   
> -  BlockNumber       = (UINT32) MINIMUM(LastBlock, MAX_NUMBER_OF_READ_BLOCK_BUFFER);
> +  EraseLengthGranularity = EraseBlock->EraseLengthGranularity;
> +  EraseSize              = (UINTN)SctMultU64x32 (EraseLengthGranularity, BlockSize);
>   
>     if (MediaPresent == FALSE) {
> -    Status = EraseBlock->EraseBlocks(EraseBlock, MediaId, 0, &Token, BlockNumber);
> +    Status = EraseBlock->EraseBlocks(EraseBlock, MediaId, 0, &Token, EraseSize);
>       if (Status == EFI_NO_MEDIA)
>         AssertionType = EFI_TEST_ASSERTION_PASSED;
>       else
> @@ -141,7 +142,7 @@ BBTestEraseBlocksConformanceTest (
>                        Status
>                        );
>   
> -    Status = EraseBlock->EraseBlocks(EraseBlock, MediaId, LastBlock + 1, &Token, BlockNumber);
> +    Status = EraseBlock->EraseBlocks(EraseBlock, MediaId, LastBlock + 1, &Token, EraseSize);
>       if (Status == EFI_NO_MEDIA)
>         AssertionType = EFI_TEST_ASSERTION_PASSED;
>       else
> @@ -158,7 +159,7 @@ BBTestEraseBlocksConformanceTest (
>                        Status
>                        );
>   
> -    Status = EraseBlock->EraseBlocks(EraseBlock, MediaId, LastBlock - 10, &Token, BlockNumber + 1);
> +    Status = EraseBlock->EraseBlocks(EraseBlock, MediaId, LastBlock - 10, &Token, EraseSize + 1);
Why -10? Magic Number.
>       if (Status == EFI_NO_MEDIA)
>         AssertionType = EFI_TEST_ASSERTION_PASSED;
>       else
> @@ -177,7 +178,7 @@ BBTestEraseBlocksConformanceTest (
>    
>     } else {
>       if (ReadOnly == TRUE) {
> -      Status = EraseBlock->EraseBlocks(EraseBlock, MediaId, 0, &Token, BlockNumber);
> +      Status = EraseBlock->EraseBlocks(EraseBlock, MediaId, 0, &Token, EraseSize);
>         if (Status == EFI_WRITE_PROTECTED)
>           AssertionType = EFI_TEST_ASSERTION_PASSED;
>         else
> @@ -195,7 +196,7 @@ BBTestEraseBlocksConformanceTest (
>                        );
>   
>       } else {
> -      Status = EraseBlock->EraseBlocks(EraseBlock, MediaId + 1, 0, &Token, BlockNumber);
> +      Status = EraseBlock->EraseBlocks(EraseBlock, MediaId + 1, 0, &Token, EraseSize);
>         if (Status == EFI_MEDIA_CHANGED)
>           AssertionType = EFI_TEST_ASSERTION_PASSED;
>         else
> @@ -212,7 +213,7 @@ BBTestEraseBlocksConformanceTest (
>                        Status
>                        );
>   
> -      Status = EraseBlock->EraseBlocks(EraseBlock, MediaId + 1, LastBlock + 1, &Token, BlockNumber);
> +      Status = EraseBlock->EraseBlocks(EraseBlock, MediaId + 1, LastBlock + 1, &Token, EraseSize);
>         if (Status == EFI_MEDIA_CHANGED)
>           AssertionType = EFI_TEST_ASSERTION_PASSED;
>         else
> @@ -229,7 +230,7 @@ BBTestEraseBlocksConformanceTest (
>                        Status
>                        );
>   
> -      Status = EraseBlock->EraseBlocks(EraseBlock, MediaId + 1, LastBlock - 10, &Token, BlockNumber + 1);
> +      Status = EraseBlock->EraseBlocks(EraseBlock, MediaId + 1, LastBlock - 10, &Token, EraseSize + 1);
Magic Number 10.
>         if (Status == EFI_MEDIA_CHANGED)
>           AssertionType = EFI_TEST_ASSERTION_PASSED;
>         else
> @@ -246,7 +247,7 @@ BBTestEraseBlocksConformanceTest (
>                        Status
>                        );
>   
> -      Status = EraseBlock->EraseBlocks(EraseBlock, MediaId, LastBlock + 1, &Token, BlockNumber);
> +      Status = EraseBlock->EraseBlocks(EraseBlock, MediaId, LastBlock + 1, &Token, EraseSize);
>         if (Status == EFI_INVALID_PARAMETER)
>           AssertionType = EFI_TEST_ASSERTION_PASSED;
>         else
> diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestFunction.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestFunction.c
> index ea081625..7124b525 100644
> --- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestFunction.c
> +++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestFunction.c
> @@ -37,12 +37,55 @@ BBTestEraseBlocksFunctionTest (
>     )
>   {
>     EFI_STANDARD_TEST_LIBRARY_PROTOCOL    *StandardLib;
> +  EFI_STATUS                            Status;
> +  EFI_ERASE_BLOCK_PROTOCOL              *EraseBlock;
> +
> +  //
> +  // init
> +  //
> +  EraseBlock = (EFI_ERASE_BLOCK_PROTOCOL*)ClientInterface;
> +
> +  //
> +  // Get the Standard Library Interface
> +  //
> +  Status = gtBS->HandleProtocol (
> +                   SupportHandle,
> +                   &gEfiStandardTestLibraryGuid,
> +                   (VOID **) &StandardLib
> +                   );
> +
> +  if ( EFI_ERROR(Status) ) {
> +    StandardLib->RecordAssertion (
> +                   StandardLib,
> +                   EFI_TEST_ASSERTION_FAILED,
> +                   gTestGenericFailureGuid,
> +                   L"BS.HandleProtocol - Handle standard test library",
> +                   L"%a:%d:Status - %r\n",
> +                   __FILE__,
> +                   (UINTN)__LINE__,
> +                   Status
> +                   );
> +    return Status;
> +  }
> +
> +  BBTestEraseBlocksFunctionTestCheckPoint1 (StandardLib, EraseBlock);
> +
> +  BBTestEraseBlocksFunctionTestCheckPoint2 (StandardLib, EraseBlock);
> +
> +  return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +BBTestEraseBlocksFunctionTestCheckPoint1 (
> +  IN EFI_STANDARD_TEST_LIBRARY_PROTOCOL    *StandardLib,
> +  IN EFI_ERASE_BLOCK_PROTOCOL              *EraseBlock
> +  )
> +{
>     EFI_STATUS                            Status;
>     EFI_STATUS                            EraseStatus;
>     EFI_STATUS                            ReadStatus;
>     EFI_STATUS                            WriteStatus;
>     EFI_STATUS                            FlushStatus;
> -  EFI_ERASE_BLOCK_PROTOCOL              *EraseBlock;
>     EFI_BLOCK_IO_PROTOCOL                 *BlockIo;
>     EFI_BLOCK_IO2_PROTOCOL                *BlockIo2;
>     EFI_TEST_ASSERTION                    AssertionType;
> @@ -64,9 +107,9 @@ BBTestEraseBlocksFunctionTest (
>     UINT8                                 *Buffer1 = NULL;
>     UINT8                                 *Buffer2 = NULL;
>     BOOLEAN                               IsZero  = TRUE;
> -  BOOLEAN                               IsZero1 = TRUE;
> -  BOOLEAN                               IsZero2 = TRUE;
> -  BOOLEAN                               IsZero3 = TRUE;
> +  BOOLEAN                               IsZero1;
> +  BOOLEAN                               IsZero2;
> +  BOOLEAN                               IsZero3;
>   
>     UINT64                                Index;
>     UINTN                                 Index1;
> @@ -75,30 +118,6 @@ BBTestEraseBlocksFunctionTest (
>     EFI_ERASE_BLOCK_TOKEN                 Token;
>     EFI_BLOCK_IO2_TOKEN                   BlockIo2Token;
>   
> -  EraseBlock = (EFI_ERASE_BLOCK_PROTOCOL*)ClientInterface;
> -
> -  //
> -  // Get the Standard Library Interface
> -  //
> -  Status = gtBS->HandleProtocol (
> -                   SupportHandle,
> -                   &gEfiStandardTestLibraryGuid,
> -                   (VOID **) &StandardLib
> -                   );
> -
> -  if ( EFI_ERROR(Status) ) {
> -    StandardLib->RecordAssertion (
> -                   StandardLib,
> -                   EFI_TEST_ASSERTION_FAILED,
> -                   gTestGenericFailureGuid,
> -                   L"BS.HandleProtocol - Handle standard test library",
> -                   L"%a:%d:Status - %r\n",
> -                   __FILE__,
> -                   (UINTN)__LINE__,
> -                   Status
> -                   );
> -    return Status;
> -  }
>   
>     EraseLengthGranularity = EraseBlock->EraseLengthGranularity;
>   
> @@ -223,9 +242,13 @@ BBTestEraseBlocksFunctionTest (
>           // Read the data with 0, the first/last block should not be erased
>           ReadStatus = BlockIo->ReadBlocks (BlockIo, MediaId, Lba, BufferSize, (VOID*)Buffer2);
>           if (ReadStatus == EFI_SUCCESS) {
> +          IsZero1 = FALSE;
> +          IsZero2 = TRUE;
> +          IsZero3 = FALSE;
> +        	
>             for (Index1 = 0; Index1 < BlockSize; Index1++) {
> -            if (Buffer2[Index1] != 0) {
> -              IsZero1 = FALSE;
> +            if (Buffer2[Index1] != 1) {
> +              IsZero1 = TRUE;
>                 break;
>               }
>             }
> @@ -238,8 +261,8 @@ BBTestEraseBlocksFunctionTest (
>             }
>   
>             for (Index1 = BufferSize - BlockSize; Index1 < BufferSize; Index1++) {
> -            if (Buffer2[Index1] != 0) {
> -              IsZero3 = FALSE;
> +            if (Buffer2[Index1] != 1) {
> +              IsZero3 = TRUE;
>                 break;
>               }
>             }
> @@ -254,7 +277,7 @@ BBTestEraseBlocksFunctionTest (
>                            StandardLib,
>                            AssertionType,
>                            gEraseBlockBBTestFunctionAssertionGuid003,
> -                         L"EraseBlocks - EraseBlocks for testing, the first/last block should not be erased",
> +                         L"EraseBlocks - EraseBlocks for testing with BlockIo, the first/last block should not be erased",
>                            L"%a:%d:EraseBlocks Status - %r, IsZero1 - %d, IsZero2 - %d, IsZero3 - %d",
>                            __FILE__,
>                            (UINTN)__LINE__,
> @@ -293,7 +316,7 @@ BBTestEraseBlocksFunctionTest (
>                          StandardLib,
>                          AssertionType,
>                          gEraseBlockBBTestFunctionAssertionGuid001,
> -                       L"EraseBlocks - EraseBlocks for testing",
> +                       L"EraseBlocks - EraseBlocks for testing with BlockIo",
>                          L"%a:%d:EraseBlocks Status - %r, IsZero - %d",
>                          __FILE__,
>                          (UINTN)__LINE__,
> @@ -333,9 +356,6 @@ BBTestEraseBlocksFunctionTest (
>   BlockIo2:
>   
>     IsZero  = TRUE;
> -  IsZero1 = TRUE;
> -  IsZero2 = TRUE;
> -  IsZero3 = TRUE;
>   
>     Status = LocateBlockIo2FromEraseBlock(EraseBlock, &BlockIo2, StandardLib);
>     if (Status == EFI_SUCCESS) {
> @@ -492,9 +512,13 @@ BlockIo2:
>           // Read the data with 0, the first/last block should not be erased
>           ReadStatus = BlockIo2->ReadBlocksEx (BlockIo2, MediaId, Lba, &BlockIo2Token, BufferSize, (VOID*)Buffer2);
>           if (ReadStatus == EFI_SUCCESS) {
> +          IsZero1 = FALSE;
> +          IsZero2 = TRUE;
> +          IsZero3 = FALSE;
> +        	
>             for (Index1 = 0; Index1 < BlockSize; Index1++) {
> -            if (Buffer2[Index1] != 0) {
> -              IsZero1 = FALSE;
> +            if (Buffer2[Index1] != 1) {
> +              IsZero1 = TRUE;
>                 break;
>               }
>             }
> @@ -507,13 +531,13 @@ BlockIo2:
>             }
>   
>             for (Index1 = BufferSize - BlockSize; Index1 < BufferSize; Index1++) {
> -            if (Buffer2[Index1] != 0) {
> -              IsZero3 = FALSE;
> +            if (Buffer2[Index1] != 1) {
> +              IsZero3 = TRUE;
>                 break;
>               }
>             }
>   
> -          if ((EraseStatus == EFI_SUCCESS) && (IsZero1 == FALSE) && (IsZero2 == TRUE) && ((IsZero3 == FALSE)))
> +          if ((EraseStatus == EFI_SUCCESS) && (IsZero1 == FALSE) && (IsZero2 == TRUE) && (IsZero3 == FALSE) && (EnterEvent == 1))
Define Macro for 1.
>          	    AssertionType = EFI_TEST_ASSERTION_PASSED;
>             else
>               AssertionType = EFI_TEST_ASSERTION_FAILED;
> @@ -523,12 +547,12 @@ BlockIo2:
>                          StandardLib,
>                          AssertionType,
>                          gEraseBlockBBTestFunctionAssertionGuid004,
> -                       L"EraseBlocks - EraseBlocks for testing, the first/last block should not be erased",
> -                       L"%a:%d:EraseBlocks Status - %r, IsZero1 - %d, IsZero2 - %d, IsZero3 - %d",
> +                       L"EraseBlocks - EraseBlocks for testing with BlockIo2, the first/last block should not be erased",
> +                       L"%a:%d:EraseBlocks Status - %r, IsZero1 - %d, IsZero2 - %d, IsZero3 - %d, EnterEvent - %d",
>                          __FILE__,
>                          (UINTN)__LINE__,
>                          EraseStatus,
> -                       IsZero1, IsZero2, IsZero3
> +                       IsZero1, IsZero2, IsZero3, EnterEvent
>                          );
>   
>           }
> @@ -589,8 +613,8 @@ BlockIo2:
>           StandardLib->RecordAssertion (
>                          StandardLib,
>                          AssertionType,
> -                       gEraseBlockBBTestFunctionAssertionGuid004,
> -                       L"EraseBlocks - EraseBlocks for testing",
> +                       gEraseBlockBBTestFunctionAssertionGuid002,
> +                       L"EraseBlocks - EraseBlocks for testing with BlockIo2",
>                          L"%a:%d:EraseBlocks Status - %r, IsZero - %d, EnterEvent - %d",
>                          __FILE__,
>                          (UINTN)__LINE__,
> @@ -635,3 +659,485 @@ End:
>      return EFI_SUCCESS;
>   }
>   
> +EFI_STATUS
> +BBTestEraseBlocksFunctionTestCheckPoint2 (
> +  IN EFI_STANDARD_TEST_LIBRARY_PROTOCOL    *StandardLib,
> +  IN EFI_ERASE_BLOCK_PROTOCOL              *EraseBlock
> +  )
> +{
> +  EFI_STATUS                            Status;
> +  EFI_STATUS                            EraseStatus;
> +  EFI_STATUS                            ReadStatus;
> +  EFI_STATUS                            WriteStatus;
> +  EFI_STATUS                            FlushStatus;
> +  EFI_BLOCK_IO_PROTOCOL                 *BlockIo;
> +  EFI_BLOCK_IO2_PROTOCOL                *BlockIo2;
> +  EFI_TEST_ASSERTION                    AssertionType;
> +  UINT32                                EraseLengthGranularity;
> +
> +  EFI_BLOCK_IO_MEDIA                    *Media;
> +  UINT32                                MediaId;
> +  BOOLEAN                               RemovableMedia;
> +  BOOLEAN                               MediaPresent;
> +  BOOLEAN                               LogicalPartition;
> +  BOOLEAN                               ReadOnly;
> +  BOOLEAN                               WriteCaching;
> +  UINT32                                BlockSize;
> +  UINT32                                IoAlign;
> +  EFI_LBA                               LastBlock;
> +  EFI_LBA                               Lba;
> +
> +  UINTN                                 BufferSize;
> +  UINT8                                 *Buffer1 = NULL;
> +  UINT8                                 *Buffer2 = NULL;
> +  BOOLEAN                               IsZero1;
> +  BOOLEAN                               IsZero2;
> +  BOOLEAN                               IsZero3;
> +
> +  UINTN                                 Index;
> +
> +  EFI_ERASE_BLOCK_TOKEN                 Token;
> +  EFI_BLOCK_IO2_TOKEN                   BlockIo2Token;
> +
> +
> +  EraseLengthGranularity = EraseBlock->EraseLengthGranularity;
> +
> +  Status = LocateBlockIoFromEraseBlock(EraseBlock, &BlockIo, StandardLib);
> +  if (Status == EFI_SUCCESS) {
> +    Media = BlockIo->Media;
> +
> +    MediaId           = Media->MediaId;
> +    RemovableMedia    = Media->RemovableMedia;
> +    MediaPresent      = Media->MediaPresent;
> +    LogicalPartition  = Media->LogicalPartition;
> +    ReadOnly          = Media->ReadOnly;
> +    WriteCaching      = Media->WriteCaching;
> +    BlockSize         = Media->BlockSize;
> +    IoAlign           = Media->IoAlign;
> +    LastBlock         = Media->LastBlock;
> +
> +    BufferSize        = (UINTN)SctMultU64x32 (4 * EraseLengthGranularity, BlockSize);
4, 5 = magic numbers.
> +
> +    if ( 5 * EraseLengthGranularity > LastBlock ) {
> +      StandardLib->RecordMessage (
> +                     StandardLib,
> +                     EFI_VERBOSE_LEVEL_DEFAULT,
> +                     L"\r\nThe space on the device is limited, and the LastBlock is: 0x%lx",
> +                     LastBlock
> +                     );
> +
> +      goto BlockIo2;
> +    }
> +
> +    //
> +    // allocate aligned buffer
> +    //
> +    Buffer1 = AllocateAlignedPool(
> +                EfiBootServicesData,
> +                BufferSize,
> +                IoAlign
> +                );
> +    if (Buffer1 == NULL) {
> +      StandardLib->RecordAssertion (
> +                     StandardLib,
> +                     EFI_TEST_ASSERTION_FAILED,
> +                     gTestGenericFailureGuid,
> +                     L"AllocateAlignedPool - Allocate aligned buffer for testing",
> +                     L"%a:%d:Status - %r",
> +                     __FILE__,
> +                     (UINTN)__LINE__,
> +                     EFI_OUT_OF_RESOURCES
> +                     );
> +      return EFI_OUT_OF_RESOURCES;
> +    }
> +
> +    Buffer2 = AllocateAlignedPool(
> +                EfiBootServicesData,
> +                BufferSize,
> +                IoAlign
> +                );
> +    if (Buffer2 == NULL) {
> +      StandardLib->RecordAssertion (
> +                     StandardLib,
> +                     EFI_TEST_ASSERTION_FAILED,
> +                     gTestGenericFailureGuid,
> +                     L"AllocateAlignedPool - Allocate aligned buffer for testing",
> +                     L"%a:%d:Status - %r",
> +                     __FILE__,
> +                     (UINTN)__LINE__,
> +                     EFI_OUT_OF_RESOURCES
> +                     );
> +      FreeAlignedPool(Buffer1);
> +      return EFI_OUT_OF_RESOURCES;
> +    }
> +
> +    if ((MediaPresent == TRUE) && (ReadOnly == FALSE)) {
> +
> +      Lba = EraseLengthGranularity;
> +
> +      //
> +      // Read the data at first with ReadBlocks
> +      //
> +      Status = BlockIo->ReadBlocks (BlockIo, MediaId, Lba, BufferSize, (VOID*)Buffer1);
> +      if (EFI_ERROR (Status)) {
> +        StandardLib->RecordAssertion (
> +                       StandardLib,
> +                       EFI_TEST_ASSERTION_FAILED,
> +                       gTestGenericFailureGuid,
> +                       L"ReadBlocks - ReadBlocks for testing fail",
> +                       L"%a:%d:Status - %r",
> +                       __FILE__,
> +                       (UINTN)__LINE__,
> +                       Status
> +                       );
> +
> +        FreeAlignedPool(Buffer1);
> +        FreeAlignedPool(Buffer2);
> +        goto BlockIo2;
> +      }
> +
> +      //
> +      // Write 1
> +      //
> +      for (Index = 0; Index < BufferSize; Index++) {
> +      	 Buffer2[Index] = 1;
> +      }
> +
> +      Status = BlockIo->WriteBlocks (BlockIo, MediaId, Lba, BufferSize, (VOID*)Buffer2);
> +      if (EFI_ERROR (Status)) {
> +        StandardLib->RecordAssertion (
> +                       StandardLib,
> +                       EFI_TEST_ASSERTION_FAILED,
> +                       gTestGenericFailureGuid,
> +                       L"WriteBlocks - WriteBlocks for testing fail",
> +                       L"%a:%d:Status - %r",
> +                       __FILE__,
> +                       (UINTN)__LINE__,
> +                       Status
> +                       );
> +
> +        FreeAlignedPool(Buffer1);
> +        FreeAlignedPool(Buffer2);
> +        goto BlockIo2;
> +      }
> +
> +      // Erase Blocks with non-EraseLengthGranularity blocks
> +      Token.Event = NULL;
> +      Token.TransactionStatus = EFI_NOT_READY;
> +
> +      EraseStatus = EraseBlock->EraseBlocks (EraseBlock, MediaId, 2 * EraseLengthGranularity, &Token, 2 * EraseLengthGranularity * BlockSize);
Why by a factor of 2?
> +
> +      // Read the data with 0, the first/last EraseLengthGranularity should not be erased
> +      ReadStatus = BlockIo->ReadBlocks (BlockIo, MediaId, Lba, BufferSize, (VOID*)Buffer2);
> +      if (ReadStatus == EFI_SUCCESS) {
> +        IsZero1 = FALSE;
> +        IsZero2 = TRUE;
> +        IsZero3 = FALSE;
> +      	
> +        for (Index = 0; Index < EraseLengthGranularity * BlockSize; Index++) {
> +          if (Buffer2[Index] != 1) {
> +            IsZero1 = TRUE;
> +            break;
> +          }
> +        }
> +
> +        for (Index = EraseLengthGranularity * BlockSize; Index < 3 * EraseLengthGranularity * BlockSize; Index++) {
> +          if (Buffer2[Index] != 0) {
> +            IsZero2 = FALSE;
> +            break;
> +          }
> +        }
3 - Magic Number
> +
> +        for (Index = 3 * EraseLengthGranularity * BlockSize; Index < 4 * EraseLengthGranularity * BlockSize; Index++) {
> +          if (Buffer2[Index] != 1) {
> +            IsZero3 = TRUE;
> +            break;
> +          }
> +        }
4 - Magic Number
Perhaps 2 "for" loops will be useful here.
> +
> +        if ((EraseStatus == EFI_SUCCESS) && (IsZero1 == FALSE) && (IsZero2 == TRUE) && ((IsZero3 == FALSE)))
> +          AssertionType = EFI_TEST_ASSERTION_PASSED;
> +        else
> +          AssertionType = EFI_TEST_ASSERTION_FAILED;
> +
> +        StandardLib->RecordAssertion (
> +                         StandardLib,
> +                         AssertionType,
> +                         gEraseBlockBBTestFunctionAssertionGuid005,
> +                         L"EraseBlocks - EraseBlocks for testing with BlockIo, the first/last EraseLengthGranularity should not be erased",
> +                         L"%a:%d:EraseBlocks Status - %r, IsZero1 - %d, IsZero2 - %d, IsZero3 - %d",
> +                         __FILE__,
> +                         (UINTN)__LINE__,
> +                         EraseStatus,
> +                         IsZero1, IsZero2, IsZero3
> +                         );
> +
> +      }
> +
> +      //
> +      // Restore the data with WriteBlocks and FlushBlocks
> +      //
> +      WriteStatus = BlockIo->WriteBlocks (BlockIo, MediaId, Lba, BufferSize, (VOID*)Buffer1);
> +      FlushStatus = EFI_SUCCESS;
> +      if (WriteCaching == TRUE)
> +        FlushStatus = BlockIo->FlushBlocks(BlockIo);
> +
> +      if ((WriteStatus != EFI_SUCCESS) || (FlushStatus != EFI_SUCCESS))
> +        StandardLib->RecordAssertion (
> +                       StandardLib,
> +                       EFI_TEST_ASSERTION_FAILED,
> +                       gTestGenericFailureGuid,
> +                       L"WriteBlocks/FlushBlocks - Restore for testing fail",
> +                       L"%a:%d: WriteStatus - %r, FlushStatus - %r",
> +                       __FILE__,
> +                       (UINTN)__LINE__,
> +                       WriteStatus,
> +                       FlushStatus
> +                       );
> +
> +      FreeAlignedPool(Buffer1);
> +      FreeAlignedPool(Buffer2);
> +    }
> +  }
> +
> +BlockIo2:
> +
> +  Status = LocateBlockIo2FromEraseBlock(EraseBlock, &BlockIo2, StandardLib);
> +  if (Status == EFI_SUCCESS) {
> +    Media = BlockIo2->Media;
> +
> +    MediaId           = Media->MediaId;
> +    RemovableMedia    = Media->RemovableMedia;
> +    MediaPresent      = Media->MediaPresent;
> +    LogicalPartition  = Media->LogicalPartition;
> +    ReadOnly          = Media->ReadOnly;
> +    WriteCaching      = Media->WriteCaching;
> +    BlockSize         = Media->BlockSize;
> +    IoAlign           = Media->IoAlign;
> +    LastBlock         = Media->LastBlock;
> +
> +    BufferSize        = (UINTN)SctMultU64x32 (4 * EraseLengthGranularity, BlockSize);
Why 4
> +
> +    if ( 5 * EraseLengthGranularity > LastBlock ) {
> +      StandardLib->RecordMessage (
> +                     StandardLib,
> +                     EFI_VERBOSE_LEVEL_DEFAULT,
> +                     L"\r\nThe space on the device is limited, and the LastBlock is: 0x%lx",
> +                     LastBlock
> +                     );
> +
> +      return EFI_SUCCESS;
> +    }
Why 5
> +
> +    //
> +    // allocate aligned buffer
> +    //
> +    Buffer1 = AllocateAlignedPool(
> +                EfiBootServicesData,
> +                BufferSize,
> +                IoAlign
> +                );
> +    if (Buffer1 == NULL) {
> +      StandardLib->RecordAssertion (
> +                     StandardLib,
> +                     EFI_TEST_ASSERTION_FAILED,
> +                     gTestGenericFailureGuid,
> +                     L"AllocateAlignedPool - Allocate aligned buffer for testing",
> +                     L"%a:%d:Status - %r",
> +                     __FILE__,
> +                     (UINTN)__LINE__,
> +                     EFI_OUT_OF_RESOURCES
> +                     );
> +      return EFI_OUT_OF_RESOURCES;
> +    }
> +
> +    Buffer2 = AllocateAlignedPool(
> +                EfiBootServicesData,
> +                BufferSize,
> +                IoAlign
> +                );
> +    if (Buffer2 == NULL) {
> +      StandardLib->RecordAssertion (
> +                     StandardLib,
> +                     EFI_TEST_ASSERTION_FAILED,
> +                     gTestGenericFailureGuid,
> +                     L"AllocateAlignedPool - Allocate aligned buffer for testing",
> +                     L"%a:%d:Status - %r",
> +                     __FILE__,
> +                     (UINTN)__LINE__,
> +                     EFI_OUT_OF_RESOURCES
> +                     );
> +      FreeAlignedPool(Buffer1);
> +      return EFI_OUT_OF_RESOURCES;
> +    }
> +
> +    if ((MediaPresent == TRUE) && (ReadOnly == FALSE)) {
> +      BlockIo2Token.Event = NULL;
> +      BlockIo2Token.TransactionStatus = EFI_NOT_READY;
> +
> +      Lba = EraseLengthGranularity;
> +
> +      //
> +      // Read the data at first with ReadBlocks
> +      //
> +      Status = BlockIo2->ReadBlocksEx (BlockIo2, MediaId, Lba, &BlockIo2Token, BufferSize, (VOID*)Buffer1);
> +      if (EFI_ERROR (Status)) {
> +        StandardLib->RecordAssertion (
> +                       StandardLib,
> +                       EFI_TEST_ASSERTION_FAILED,
> +                       gTestGenericFailureGuid,
> +                       L"ReadBlocks - ReadBlocks for testing fail",
> +                       L"%a:%d:Status - %r",
> +                       __FILE__,
> +                       (UINTN)__LINE__,
> +                       Status
> +                       );
> +
> +        FreeAlignedPool(Buffer1);
> +        FreeAlignedPool(Buffer2);
> +        goto End;
> +      }
> +
> +      //
> +      // Write 1
> +      //
> +
> +      for (Index = 0; Index < BufferSize; Index++) {
> +        Buffer2[Index] = 1;
> +      }
> +
> +      Status = BlockIo2->WriteBlocksEx (BlockIo2, MediaId, Lba, &BlockIo2Token, BufferSize, (VOID*)Buffer2);
> +      if (EFI_ERROR (Status)) {
> +        StandardLib->RecordAssertion (
> +                       StandardLib,
> +                       EFI_TEST_ASSERTION_FAILED,
> +                       gTestGenericFailureGuid,
> +                       L"WriteBlocks - WriteBlocks for testing fail",
> +                       L"%a:%d:Status - %r",
> +                       __FILE__,
> +                       (UINTN)__LINE__,
> +                       Status
> +                       );
> +
> +        FreeAlignedPool(Buffer1);
> +        FreeAlignedPool(Buffer2);
> +        goto End;
> +      }
> +
> +      //
> +      // Erase Blocks with non EraseLengthGranularity blocks
> +      //
> +      Token.Event             = NULL;
> +      Token.TransactionStatus = EFI_NOT_READY;
> +
> +      EnterEvent = 0;
> +
> +      Status = gtBS->CreateEvent (
> +                   EVT_NOTIFY_SIGNAL,
> +                   TPL_CALLBACK,
> +                   (EFI_EVENT_NOTIFY) NotifyFunction,
> +                   NULL,
> +                   &Token.Event
> +                   );
> +
> +      if (EFI_ERROR (Status)) {
> +        StandardLib->RecordAssertion (
> +                       StandardLib,
> +                       EFI_TEST_ASSERTION_FAILED,
> +                       gTestGenericFailureGuid,
> +                       L"CreateEvent - CreateEvent for testing fail",
> +                       L"%a:%d:Status - %r",
> +                       __FILE__,
> +                       (UINTN)__LINE__,
> +                       Status
> +                       );
> +        FreeAlignedPool(Buffer1);
> +        FreeAlignedPool(Buffer2);
> +        goto End;
> +      }
> +
> +      EraseStatus = EraseBlock->EraseBlocks (EraseBlock, MediaId, 2 * EraseLengthGranularity, &Token, 2 * EraseLengthGranularity * BlockSize);
> +      while(Token.TransactionStatus == EFI_NOT_READY);
> +
> +      // Read the data with 0, the first/last EraseLengthGranularity should not be erased
> +      ReadStatus = BlockIo2->ReadBlocksEx (BlockIo2, MediaId, Lba, &BlockIo2Token, BufferSize, (VOID*)Buffer2);
> +      if (ReadStatus == EFI_SUCCESS) {
> +        IsZero1 = FALSE;
> +        IsZero2 = TRUE;
> +        IsZero3 = FALSE;
> +
> +        for (Index = 0; Index < EraseLengthGranularity * BlockSize; Index++) {
> +          if (Buffer2[Index] != 1) {
> +            IsZero1 = TRUE;
> +            break;
> +          }
> +        }
> +
> +        for (Index = EraseLengthGranularity * BlockSize; Index < 3 * EraseLengthGranularity * BlockSize; Index++) {
> +          if (Buffer2[Index] != 0) {
> +            IsZero2 = FALSE;
> +            break;
> +          }
> +        }
> +
> +        for (Index = 3 * EraseLengthGranularity * BlockSize; Index < 4 * EraseLengthGranularity * BlockSize; Index++) {
> +          if (Buffer2[Index] != 1) {
> +            IsZero3 = TRUE;
> +            break;
> +          }
> +        }
Magic numbers in above block.
> +
> +        if ((EraseStatus == EFI_SUCCESS) && (IsZero1 == FALSE) && (IsZero2 == TRUE) && (IsZero3 == FALSE) && (EnterEvent == 1))
> +       	  AssertionType = EFI_TEST_ASSERTION_PASSED;
> +        else
> +          AssertionType = EFI_TEST_ASSERTION_FAILED;
> +
> +        StandardLib->RecordAssertion (
> +                       StandardLib,
> +                       AssertionType,
> +                       gEraseBlockBBTestFunctionAssertionGuid006,
> +                       L"EraseBlocks - EraseBlocks for testing with BlockIo2, the first/last EraseLengthGranularity should not be erased",
> +                       L"%a:%d:EraseBlocks Status - %r, IsZero1 - %d, IsZero2 - %d, IsZero3 - %d, EnterEvent - %d",
> +                       __FILE__,
> +                       (UINTN)__LINE__,
> +                       EraseStatus,
> +                       IsZero1, IsZero2, IsZero3, EnterEvent
> +                       );
> +
> +      }
> +
> +
> +      //
> +      // Restore the data with WriteBlocks and FlushBlocks
> +      //
> +      WriteStatus = BlockIo2->WriteBlocksEx (BlockIo2, MediaId, Lba, &BlockIo2Token, BufferSize, (VOID*)Buffer1);
> +      FlushStatus = EFI_SUCCESS;
> +      if (WriteCaching == TRUE)
> +        FlushStatus = BlockIo2->FlushBlocksEx (BlockIo2, &BlockIo2Token);
> +
> +      if ((WriteStatus != EFI_SUCCESS) || (FlushStatus != EFI_SUCCESS))
> +        StandardLib->RecordAssertion (
> +                       StandardLib,
> +                       EFI_TEST_ASSERTION_FAILED,
> +                       gTestGenericFailureGuid,
> +                       L"WriteBlocks/FlushBlocks - Restore for testing fail",
> +                       L"%a:%d: WriteStatus - %r, FlushStatus - %r",
> +                       __FILE__,
> +                       (UINTN)__LINE__,
> +                       WriteStatus,
> +                       FlushStatus
> +                       );
> +
> +      FreeAlignedPool(Buffer1);
> +      FreeAlignedPool(Buffer2);
> +    }
> +
> +End:
> +    if (Token.Event != NULL)
> +      gtBS->CloseEvent(Token.Event);
> +
> +  }
> +
> +   return EFI_SUCCESS;
> +}
> +
> diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestMain.h b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestMain.h
> index 4b569d32..280cf070 100644
> --- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestMain.h
> +++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestMain.h
> @@ -1,7 +1,7 @@
>   /** @file
>   
>     Copyright 2017 Unified EFI, Inc.<BR>
> -  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2017 - 2018, Intel Corporation. All rights reserved.<BR>
>   
>     This program and the accompanying materials
>     are licensed and made available under the terms and conditions of the BSD License
> @@ -58,14 +58,12 @@ Abstract:
>   //
>   //
>   EFI_STATUS
> -EFIAPI
>   InitializeEraseBlockBBTest (
>     IN EFI_HANDLE                   ImageHandle,
>     IN EFI_SYSTEM_TABLE             *SystemTable
>     );
>     
>   EFI_STATUS
> -EFIAPI
>   UnloadEraseBlockBBTest (
>     IN EFI_HANDLE                   ImageHandle
>     );
> @@ -90,6 +88,18 @@ BBTestEraseBlocksFunctionTest (
>     IN EFI_HANDLE                 SupportHandle
>     );
>   
> +EFI_STATUS
> +BBTestEraseBlocksFunctionTestCheckPoint1 (
> +  IN EFI_STANDARD_TEST_LIBRARY_PROTOCOL    *StandardLib,
> +  IN EFI_ERASE_BLOCK_PROTOCOL              *EraseBlock
> +  );
> +
> +EFI_STATUS
> +BBTestEraseBlocksFunctionTestCheckPoint2 (
> +  IN EFI_STANDARD_TEST_LIBRARY_PROTOCOL    *StandardLib,
> +  IN EFI_ERASE_BLOCK_PROTOCOL              *EraseBlock
> +  );
> +
>   EFI_STATUS
>   LocateBlockIoFromEraseBlock (
>     IN EFI_ERASE_BLOCK_PROTOCOL              *EraseBlock,
> diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/Guid.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/Guid.c
> index cfc30707..eb210e3b 100644
> --- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/Guid.c
> +++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/Guid.c
> @@ -40,4 +40,6 @@ EFI_GUID gEraseBlockBBTestConformanceAssertionGuid009 = EFI_TEST_ERASEBLOCKBBTES
>   EFI_GUID gEraseBlockBBTestFunctionAssertionGuid001 = EFI_TEST_ERASEBLOCKBBTESTFUNCTION_ASSERTION_001_GUID;
>   EFI_GUID gEraseBlockBBTestFunctionAssertionGuid002 = EFI_TEST_ERASEBLOCKBBTESTFUNCTION_ASSERTION_002_GUID;
>   EFI_GUID gEraseBlockBBTestFunctionAssertionGuid003 = EFI_TEST_ERASEBLOCKBBTESTFUNCTION_ASSERTION_003_GUID;
> -EFI_GUID gEraseBlockBBTestFunctionAssertionGuid004 = EFI_TEST_ERASEBLOCKBBTESTFUNCTION_ASSERTION_004_GUID;
> \ No newline at end of file
> +EFI_GUID gEraseBlockBBTestFunctionAssertionGuid004 = EFI_TEST_ERASEBLOCKBBTESTFUNCTION_ASSERTION_004_GUID;
> +EFI_GUID gEraseBlockBBTestFunctionAssertionGuid005 = EFI_TEST_ERASEBLOCKBBTESTFUNCTION_ASSERTION_005_GUID;
> +EFI_GUID gEraseBlockBBTestFunctionAssertionGuid006 = EFI_TEST_ERASEBLOCKBBTESTFUNCTION_ASSERTION_006_GUID;
> \ No newline at end of file
> diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/Guid.h b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/Guid.h
> index 34aa88c1..61f634f4 100644
> --- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/Guid.h
> +++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/Guid.h
> @@ -79,4 +79,11 @@ extern EFI_GUID gEraseBlockBBTestFunctionAssertionGuid003;
>   { 0x402a045e, 0xa67b, 0x4a3e, { 0x89, 0x9e, 0x2d, 0xe4, 0x71, 0x75, 0x6e, 0x16 } }
>   extern EFI_GUID gEraseBlockBBTestFunctionAssertionGuid004;
>   
> +#define EFI_TEST_ERASEBLOCKBBTESTFUNCTION_ASSERTION_005_GUID \
> +{ 0xcea3e3a5, 0xaa6f, 0x47fe, { 0xaa, 0x2b, 0xe0, 0x35, 0x4c, 0x63, 0x69, 0xd6 } }
> +extern EFI_GUID gEraseBlockBBTestFunctionAssertionGuid005;
> +
> +#define EFI_TEST_ERASEBLOCKBBTESTFUNCTION_ASSERTION_006_GUID \
> +{ 0x125ad006, 0x2886, 0x4022, { 0xb8, 0x2c, 0x60, 0x2f, 0x14, 0xad, 0xfa, 0x28 } }
> +extern EFI_GUID gEraseBlockBBTestFunctionAssertionGuid006;
>   



  reply	other threads:[~2018-10-16  9:48 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-14 16:26 [PATCH] uefi-sct/SctPkg:Enhance the EraseBlock Test Eric Jin
2018-10-15  2:43 ` Supreeth Venkatesh [this message]
2018-10-15 13:09   ` Supreeth Venkatesh

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=a0a379ac-b249-a5f0-1aa2-d50eb86e34ae@arm.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