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 48E0F21A07093 for ; Mon, 18 Mar 2019 12:44:46 -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 6EB3D1650; Mon, 18 Mar 2019 12:44:46 -0700 (PDT) Received: from supven01-thinkstation-p720.austin.arm.com (supven01-thinkstation-p720.austin.arm.com [10.118.28.44]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 3E36F3F59C; Mon, 18 Mar 2019 12:44:46 -0700 (PDT) Message-ID: <365c74dea999e06ebfd663b8ac47ad51c5f41ae7.camel@arm.com> From: Supreeth Venkatesh To: Eric Jin , edk2-devel@lists.01.org Date: Mon, 18 Mar 2019 14:44:45 -0500 In-Reply-To: <20190311052947.3960-1-eric.jin@intel.com> References: <20190311052947.3960-1-eric.jin@intel.com> X-Mailer: Evolution 3.28.5-0ubuntu0.18.04.1 Mime-Version: 1.0 Subject: Re: [edk2-test][Patch 1/1] uefi-sct/SctPkg: Size in EraseBlocks() is in bytes 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: Mon, 18 Mar 2019 19:44:47 -0000 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit 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 > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eric Jin > --- > 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.
> - Copyright (c) 2017, Intel Corporation. All rights reserved.
> + Copyright (c) 2017 - 2019, Intel Corporation. All rights > reserved.
> > 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