From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=217.140.101.70; helo=foss.arm.com; envelope-from=supreeth.venkatesh@arm.com; receiver=edk2-devel@lists.01.org Received: from foss.arm.com (usa-sjc-mx-foss1.foss.arm.com [217.140.101.70]) by ml01.01.org (Postfix) with ESMTP id 859D4211799D8 for ; Tue, 16 Oct 2018 02:48:37 -0700 (PDT) Received: by usa-sjc-mx-foss1.foss.arm.com (Postfix, from userid 105) id A46223628; Tue, 16 Oct 2018 03:28:31 -0700 (PDT) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 89E4B32F9; Sun, 14 Oct 2018 18:44:23 -0700 (PDT) Received: from [10.6.43.238] (bc-c3-3-14.eu.iaas.arm.com [10.6.43.238]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id AD0F93F5B1; Sun, 14 Oct 2018 18:44:02 -0700 (PDT) To: Eric Jin , edk2-devel@lists.01.org Cc: Jiaxin Wu References: <20181013155111.560-1-eric.jin@intel.com> From: Supreeth Venkatesh Message-ID: <997b7ed5-ccaa-984a-adf1-6be67d0961e8@arm.com> Date: Mon, 15 Oct 2018 02:44:01 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20181013155111.560-1-eric.jin@intel.com> Subject: Re: [PATCH] uefi-sct/SctPkg:The original design for the 'EraseLengthGranularity' test need consider this case X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Oct 2018 09:48:38 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Commit Message less than 80 cols please. On 10/13/2018 04:51 PM, Eric Jin wrote: > For the SD device, no same as the eMMC, the 'EraseLengthGranularity' is 1. > > Cc: Supreeth Venkatesh > Cc: Jiaxin Wu > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eric Jin > --- > .../BlackBoxTest/EraseBlockBBTestFunction.c | 154 +++++++++--------- > 1 file changed, 78 insertions(+), 76 deletions(-) > > 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 bc16a473..ea081625 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 > @@ -214,54 +214,56 @@ BBTestEraseBlocksFunctionTest ( > } > > // Erase Blocks with non-EraseLengthGranularity blocks > - Token.Event = NULL; > - Token.TransactionStatus = EFI_NOT_READY; > - EraseStatus = EraseBlock->EraseBlocks (EraseBlock, MediaId, Lba+1, &Token, BufferSize - 2 * BlockSize); > - > - // 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) { > - for (Index1 = 0; Index1 < BlockSize; Index1++) { > - if (Buffer2[Index1] != 0) { > - IsZero1 = FALSE; > - break; > + if (BufferSize > 2 * BlockSize) { Magic Number 2. > + Token.Event = NULL; > + Token.TransactionStatus = EFI_NOT_READY; > + > + EraseStatus = EraseBlock->EraseBlocks (EraseBlock, MediaId, Lba+1, &Token, BufferSize - 2 * BlockSize); Magic number 2. > + > + // 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) { > + for (Index1 = 0; Index1 < BlockSize; Index1++) { > + if (Buffer2[Index1] != 0) { > + IsZero1 = FALSE; > + break; > + } > } > - } > > - for (Index1 = BlockSize; Index1 < BufferSize - BlockSize; Index1++) { > - if (Buffer2[Index1] != 0) { > - IsZero2 = FALSE; > - break; > + for (Index1 = BlockSize; Index1 < BufferSize - BlockSize; Index1++) { > + if (Buffer2[Index1] != 0) { > + IsZero2 = FALSE; > + break; > + } > } > - } > > - for (Index1 = BufferSize - BlockSize; Index1 < BufferSize; Index1++) { > - if (Buffer2[Index1] != 0) { > - IsZero3 = FALSE; > - break; > + for (Index1 = BufferSize - BlockSize; Index1 < BufferSize; Index1++) { > + if (Buffer2[Index1] != 0) { > + IsZero3 = FALSE; > + break; > + } > } > - } > > - if ((EraseStatus == EFI_SUCCESS) && (IsZero1 == FALSE) && (IsZero2 == TRUE) && ((IsZero3 == FALSE))) > - AssertionType = EFI_TEST_ASSERTION_PASSED; > - else > - AssertionType = EFI_TEST_ASSERTION_FAILED; > + if ((EraseStatus == EFI_SUCCESS) && (IsZero1 == FALSE) && (IsZero2 == TRUE) && ((IsZero3 == FALSE))) Less than 80 cols. > + AssertionType = EFI_TEST_ASSERTION_PASSED; > + else > + AssertionType = EFI_TEST_ASSERTION_FAILED; > > > - StandardLib->RecordAssertion ( > - StandardLib, > - AssertionType, > - gEraseBlockBBTestFunctionAssertionGuid003, > - 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", > - __FILE__, > - (UINTN)__LINE__, > - Status, > - IsZero1, IsZero2, IsZero3 > - ); > + StandardLib->RecordAssertion ( > + StandardLib, > + AssertionType, > + gEraseBlockBBTestFunctionAssertionGuid003, > + 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", > + __FILE__, > + (UINTN)__LINE__, > + EraseStatus, > + IsZero1, IsZero2, IsZero3 > + ); > > + } > } > - > // > // Erase Blocks with the EraseLengthGranularity blocks > // > @@ -453,13 +455,13 @@ BlockIo2: > // > // Erase Blocks with non EraseLengthGranularity blocks > // > + if (BufferSize > 2 * BlockSize) { Magic Number 2. > + Token.Event = NULL; > + Token.TransactionStatus = EFI_NOT_READY; > > - Token.Event = NULL; > - Token.TransactionStatus = EFI_NOT_READY; > - > - EnterEvent = 0; > + EnterEvent = 0; > > - Status = gtBS->CreateEvent ( > + Status = gtBS->CreateEvent ( > EVT_NOTIFY_SIGNAL, > TPL_CALLBACK, > (EFI_EVENT_NOTIFY) NotifyFunction, > @@ -467,8 +469,8 @@ BlockIo2: > &Token.Event > ); > > - if (EFI_ERROR (Status)) { > - StandardLib->RecordAssertion ( > + if (EFI_ERROR (Status)) { > + StandardLib->RecordAssertion ( > StandardLib, > EFI_TEST_ASSERTION_FAILED, > gTestGenericFailureGuid, > @@ -478,46 +480,46 @@ BlockIo2: > (UINTN)__LINE__, > Status > ); > - FreeAlignedPool(Buffer1); > - FreeAlignedPool(Buffer2); > - goto End; > - } > + FreeAlignedPool(Buffer1); > + FreeAlignedPool(Buffer2); > + goto End; > + } > > - EraseStatus = EraseBlock->EraseBlocks (EraseBlock, MediaId, Lba+1, &Token, BufferSize - 2 * BlockSize); > + EraseStatus = EraseBlock->EraseBlocks (EraseBlock, MediaId, Lba+1, &Token, BufferSize - 2 * BlockSize); Magic Number 2. > > - while(Token.TransactionStatus == EFI_NOT_READY); > + while(Token.TransactionStatus == EFI_NOT_READY); > > - // 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) { > - for (Index1 = 0; Index1 < BlockSize; Index1++) { > - if (Buffer2[Index1] != 0) { > - IsZero1 = FALSE; > - break; > + // 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) { > + for (Index1 = 0; Index1 < BlockSize; Index1++) { > + if (Buffer2[Index1] != 0) { > + IsZero1 = FALSE; > + break; > + } > } > - } > > - for (Index1 = BlockSize; Index1 < BufferSize - BlockSize; Index1++) { > - if (Buffer2[Index1] != 0) { > - IsZero2 = FALSE; > - break; > + for (Index1 = BlockSize; Index1 < BufferSize - BlockSize; Index1++) { > + if (Buffer2[Index1] != 0) { > + IsZero2 = FALSE; > + break; > + } > } > - } > > - for (Index1 = BufferSize - BlockSize; Index1 < BufferSize; Index1++) { > - if (Buffer2[Index1] != 0) { > - IsZero3 = FALSE; > - break; > + for (Index1 = BufferSize - BlockSize; Index1 < BufferSize; Index1++) { > + if (Buffer2[Index1] != 0) { > + IsZero3 = FALSE; > + break; > + } > } > - } > > - if ((EraseStatus == EFI_SUCCESS) && (IsZero1 == FALSE) && (IsZero2 == TRUE) && ((IsZero3 == FALSE))) > - AssertionType = EFI_TEST_ASSERTION_PASSED; > - else > - AssertionType = EFI_TEST_ASSERTION_FAILED; > + if ((EraseStatus == EFI_SUCCESS) && (IsZero1 == FALSE) && (IsZero2 == TRUE) && ((IsZero3 == FALSE))) less than 80 cols. > + AssertionType = EFI_TEST_ASSERTION_PASSED; > + else > + AssertionType = EFI_TEST_ASSERTION_FAILED; > > > - StandardLib->RecordAssertion ( > + StandardLib->RecordAssertion ( > StandardLib, > AssertionType, > gEraseBlockBBTestFunctionAssertionGuid004, > @@ -525,12 +527,12 @@ BlockIo2: > L"%a:%d:EraseBlocks Status - %r, IsZero1 - %d, IsZero2 - %d, IsZero3 - %d", > __FILE__, > (UINTN)__LINE__, > - Status, > + EraseStatus, > IsZero1, IsZero2, IsZero3 > ); > > + } > } > - > // > // Erase Blocks with the EraseLengthGranularity blocks > //