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 C65D2211799D6 for ; Tue, 16 Oct 2018 02:48:38 -0700 (PDT) Received: by usa-sjc-mx-foss1.foss.arm.com (Postfix, from userid 105) id 4539135B9; Tue, 16 Oct 2018 03:03:34 -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 B255335AC; Mon, 15 Oct 2018 05:57:50 -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 26A693F71E; Mon, 15 Oct 2018 05:57:30 -0700 (PDT) From: Supreeth Venkatesh To: Eric Jin , edk2-devel@lists.01.org Cc: Jiaxin Wu References: <20181013155111.560-1-eric.jin@intel.com> <61ec5985-249f-472a-89be-e143882fb015@arm.com> Message-ID: <7b1b738c-50c7-a17c-23b3-ad800a923a0a@arm.com> Date: Mon, 15 Oct 2018 13:57:28 +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: <61ec5985-249f-472a-89be-e143882fb015@arm.com> X-Content-Filtered-By: Mailman/MimeDel 2.1.29 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:39 -0000 Content-Language: en-US Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit FYI On 10/15/2018 03:08 AM, Supreeth Venkatesh wrote: > > > 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 >> // >