Supreeth,

Thank for the rewording suggestion. 
Will commit the patch with accurate message.

Best Regards
Eric

-----Original Message-----
From: Supreeth Venkatesh <supreeth.venkat...@arm.com> 
Sent: Tuesday, March 19, 2019 3:45 AM
To: Jin, Eric <eric....@intel.com>; edk2-devel@lists.01.org
Subject: Re: [edk2-test][Patch 1/1] uefi-sct/SctPkg: Size in EraseBlocks() is 
in bytes

On Mon, 2019-03-11 at 13:29 +0800, Eric Jin wrote:
> The erase size used in current test is blocks number,
Please reword. Block number or number of blocks?

> and should be corrected to the size in bytes to be erased. According 
> to UEFI spec, it must be a multiple of the physical block size of the 
> device, even the EraseLengthGranularity blocks for some device.
Please reword. 

Suggestion:
Currently, EFI_ERASE_BLOCK_PROTOCOL test to validate erase functionality of a 
device incorrectly used "BlockNumber" which does not reflect the actual size in 
bytes to be erased.
This patch corrects this by calculating size in bytes to be erased using 
"EraseLengthGranularity" and "BlockSize".
or Something similar.

With that
Reviewed-by: Supreeth Venkatesh <supreeth.venkat...@arm.com>

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Jin <eric....@intel.com>
> ---
>  uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBl
> ockBBTestConformance.c | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBl
> ockBBTestConformance.c b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBl
> ockBBTestConformance.c
> index df057b26667f..a6156b988f44 100644
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBl
> ockBBTestConformance.c
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBl
> ockBBTestConformance.c
> @@ -1,7 +1,7 @@
>  /** @file
>  
>    Copyright 2017 Unified EFI, Inc.<BR>
> -  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2017 - 2019, 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);
>      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);
>        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

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to