* [edk2-test][Patch 1/1] uefi-sct/SctPkg: Size in EraseBlocks() is in bytes
@ 2019-03-11 5:29 Eric Jin
2019-03-18 19:44 ` Supreeth Venkatesh
0 siblings, 1 reply; 3+ messages in thread
From: Eric Jin @ 2019-03-11 5:29 UTC (permalink / raw)
To: edk2-devel
The erase size used in current test is blocks number,
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.
Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Jin <eric.jin@intel.com>
---
uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlockBBTestConformance.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 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 df057b26667f..a6156b988f44 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 - 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
--
2.20.0.windows.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [edk2-test][Patch 1/1] uefi-sct/SctPkg: Size in EraseBlocks() is in bytes
2019-03-11 5:29 [edk2-test][Patch 1/1] uefi-sct/SctPkg: Size in EraseBlocks() is in bytes Eric Jin
@ 2019-03-18 19:44 ` Supreeth Venkatesh
2019-03-22 7:16 ` Jin, Eric
0 siblings, 1 reply; 3+ messages in thread
From: Supreeth Venkatesh @ 2019-03-18 19:44 UTC (permalink / raw)
To: Eric Jin, edk2-devel
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.venkatesh@arm.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Jin <eric.jin@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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [edk2-test][Patch 1/1] uefi-sct/SctPkg: Size in EraseBlocks() is in bytes
2019-03-18 19:44 ` Supreeth Venkatesh
@ 2019-03-22 7:16 ` Jin, Eric
0 siblings, 0 replies; 3+ messages in thread
From: Jin, Eric @ 2019-03-22 7:16 UTC (permalink / raw)
To: Supreeth Venkatesh, edk2-devel@lists.01.org
Supreeth,
Thank for the rewording suggestion.
Will commit the patch with accurate message.
Best Regards
Eric
-----Original Message-----
From: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
Sent: Tuesday, March 19, 2019 3:45 AM
To: Jin, Eric <eric.jin@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.venkatesh@arm.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Jin <eric.jin@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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-03-22 7:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-11 5:29 [edk2-test][Patch 1/1] uefi-sct/SctPkg: Size in EraseBlocks() is in bytes Eric Jin
2019-03-18 19:44 ` Supreeth Venkatesh
2019-03-22 7:16 ` Jin, Eric
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox