* [PATCH 2/2] uefi-sct/SctPkg: Correct BBTestEraseBlocks behavior (EFI_BLOCK_IO_PROTOCOL)
@ 2020-11-02 9:59 Chen, ArvinX
2020-11-11 20:56 ` [edk2-devel] " Samer El-Haj-Mahmoud
0 siblings, 1 reply; 7+ messages in thread
From: Chen, ArvinX @ 2020-11-02 9:59 UTC (permalink / raw)
To: devel; +Cc: G Edhaya Chandran, Eric Jin
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3022
The storage device erase behavior may have two possibilities:
1.Write all data into "0"
2.Write all data into "1"
but now tool behavior can only check case 1 (Write all data into "0"),
so we need add the other case into SCT tool to correct the test behavior.
Cc: G Edhaya Chandran <Edhaya.Chandran@arm.com>
Cc: Eric Jin <eric.jin@intel.com>
Signed-off-by: ArvinX Chen <arvinx.chen@intel.com>
---
.../BlackBoxTest/EraseBlockBBTestFunction.c | 55 +++++++++++++++----
1 file changed, 43 insertions(+), 12 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 cbf43e1d..dbbb70c6 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
@@ -71,6 +71,7 @@ BBTestEraseBlocksFunctionTest (
UINT64 Index;
UINTN Index1;
UINTN Remainder;
+ UINT64 EraseCounter;
EFI_ERASE_BLOCK_TOKEN Token;
EFI_BLOCK_IO2_TOKEN BlockIo2Token;
@@ -223,26 +224,41 @@ BBTestEraseBlocksFunctionTest (
// 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) {
+ for (Index1 = 0, EraseCounter = 0; Index1 < BlockSize; Index1++) {
+ if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
IsZero1 = FALSE;
break;
+ } else if (Buffer2[Index1] == 0x00) {
+ EraseCounter++;
}
}
+ if (EraseCounter!=0 && EraseCounter!=BlockSize) {
+ IsZero1 = FALSE;
+ }
- for (Index1 = BlockSize; Index1 < BufferSize - BlockSize; Index1++) {
- if (Buffer2[Index1] != 0) {
+ for (Index1 = BlockSize, EraseCounter = 0; Index1 < BufferSize - BlockSize; Index1++) {
+ if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
IsZero2 = FALSE;
break;
+ } else if (Buffer2[Index1] == 0x00) {
+ EraseCounter++;
}
}
+ if (EraseCounter!=0 && EraseCounter!=(BufferSize - (BlockSize*2))) {
+ IsZero2 = FALSE;
+ }
- for (Index1 = BufferSize - BlockSize; Index1 < BufferSize; Index1++) {
- if (Buffer2[Index1] != 0) {
+ for (Index1 = BufferSize - BlockSize, EraseCounter = 0; Index1 < BufferSize; Index1++) {
+ if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
IsZero3 = FALSE;
break;
+ } else if (Buffer2[Index1] == 0x00) {
+ EraseCounter++;
}
}
+ if (EraseCounter!=0 && EraseCounter!=BlockSize) {
+ IsZero3 = FALSE;
+ }
if ((EraseStatus == EFI_SUCCESS) && (IsZero1 == FALSE) && (IsZero2 == TRUE) && ((IsZero3 == FALSE)))
AssertionType = EFI_TEST_ASSERTION_PASSED;
@@ -492,26 +508,41 @@ BlockIo2:
// 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) {
+ for (Index1 = 0, EraseCounter = 0; Index1 < BlockSize; Index1++) {
+ if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
IsZero1 = FALSE;
break;
+ } else if (Buffer2[Index1] == 0x00) {
+ EraseCounter++;
}
}
+ if (EraseCounter!=0 && EraseCounter!=BlockSize) {
+ IsZero1 = FALSE;
+ }
- for (Index1 = BlockSize; Index1 < BufferSize - BlockSize; Index1++) {
- if (Buffer2[Index1] != 0) {
+ for (Index1 = BlockSize, EraseCounter = 0; Index1 < BufferSize - BlockSize; Index1++) {
+ if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
IsZero2 = FALSE;
break;
+ } else if (Buffer2[Index1] == 0x00) {
+ EraseCounter++;
}
}
+ if (EraseCounter!=0 && EraseCounter!=(BufferSize - (BlockSize*2))) {
+ IsZero2 = FALSE;
+ }
- for (Index1 = BufferSize - BlockSize; Index1 < BufferSize; Index1++) {
- if (Buffer2[Index1] != 0) {
+ for (Index1 = BufferSize - BlockSize, EraseCounter = 0; Index1 < BufferSize; Index1++) {
+ if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
IsZero3 = FALSE;
break;
+ } else if (Buffer2[Index1] == 0x00) {
+ EraseCounter++;
}
}
+ if (EraseCounter!=0 && EraseCounter!=BlockSize) {
+ IsZero3 = FALSE;
+ }
if ((EraseStatus == EFI_SUCCESS) && (IsZero1 == FALSE) && (IsZero2 == TRUE) && ((IsZero3 == FALSE)))
AssertionType = EFI_TEST_ASSERTION_PASSED;
--
2.26.2.windows.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH 2/2] uefi-sct/SctPkg: Correct BBTestEraseBlocks behavior (EFI_BLOCK_IO_PROTOCOL)
2020-11-02 9:59 [PATCH 2/2] uefi-sct/SctPkg: Correct BBTestEraseBlocks behavior (EFI_BLOCK_IO_PROTOCOL) Chen, ArvinX
@ 2020-11-11 20:56 ` Samer El-Haj-Mahmoud
2020-11-17 5:51 ` 回覆: " Chen, ArvinX
2020-12-07 9:49 ` G Edhaya Chandran
0 siblings, 2 replies; 7+ messages in thread
From: Samer El-Haj-Mahmoud @ 2020-11-11 20:56 UTC (permalink / raw)
To: devel@edk2.groups.io, arvinx.chen@intel.com
Cc: G Edhaya Chandran, Eric Jin, Samer El-Haj-Mahmoud,
gaoliming@byosoft.com.cn
Hi Chen,
The UEFI Specification 2.8ErrataB (page 575) states that " It is the intention of the EraseBlocks() operation to be at least as performant as writing zeroes to each of the specified LBA locations while ensuring the equivalent security."
So while not explicitly saying that Erase should "erase to 0", it implies that at least is the intention. Do we know that erasing to "1" is a valid behavior?
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chen,
> ArvinX via groups.io
> Sent: Monday, November 2, 2020 5:00 AM
> To: devel@edk2.groups.io
> Cc: G Edhaya Chandran <Edhaya.Chandran@arm.com>; Eric Jin
> <eric.jin@intel.com>
> Subject: [edk2-devel] [PATCH 2/2] uefi-sct/SctPkg: Correct
> BBTestEraseBlocks behavior (EFI_BLOCK_IO_PROTOCOL)
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3022
>
>
>
> The storage device erase behavior may have two possibilities:
>
> 1.Write all data into "0"
>
> 2.Write all data into "1"
>
> but now tool behavior can only check case 1 (Write all data into "0"),
>
> so we need add the other case into SCT tool to correct the test behavior.
>
>
>
> Cc: G Edhaya Chandran <Edhaya.Chandran@arm.com>
>
> Cc: Eric Jin <eric.jin@intel.com>
>
> Signed-off-by: ArvinX Chen <arvinx.chen@intel.com>
>
> ---
>
> .../BlackBoxTest/EraseBlockBBTestFunction.c | 55 +++++++++++++++----
>
> 1 file changed, 43 insertions(+), 12 deletions(-)
>
>
>
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlock
> BBTestFunction.c b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlock
> BBTestFunction.c
>
> index cbf43e1d..dbbb70c6 100644
>
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlock
> BBTestFunction.c
>
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlock
> BBTestFunction.c
>
> @@ -71,6 +71,7 @@ BBTestEraseBlocksFunctionTest (
>
> UINT64 Index;
>
> UINTN Index1;
>
> UINTN Remainder;
>
> + UINT64 EraseCounter;
>
>
>
> EFI_ERASE_BLOCK_TOKEN Token;
>
> EFI_BLOCK_IO2_TOKEN BlockIo2Token;
>
> @@ -223,26 +224,41 @@ BBTestEraseBlocksFunctionTest (
>
> // 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) {
>
> + for (Index1 = 0, EraseCounter = 0; Index1 < BlockSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero1 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=BlockSize) {
>
> + IsZero1 = FALSE;
>
> + }
>
>
>
> - for (Index1 = BlockSize; Index1 < BufferSize - BlockSize; Index1++) {
>
> - if (Buffer2[Index1] != 0) {
>
> + for (Index1 = BlockSize, EraseCounter = 0; Index1 < BufferSize -
> BlockSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero2 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=(BufferSize - (BlockSize*2))) {
>
> + IsZero2 = FALSE;
>
> + }
>
>
>
> - for (Index1 = BufferSize - BlockSize; Index1 < BufferSize; Index1++) {
>
> - if (Buffer2[Index1] != 0) {
>
> + for (Index1 = BufferSize - BlockSize, EraseCounter = 0; Index1 <
> BufferSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero3 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=BlockSize) {
>
> + IsZero3 = FALSE;
>
> + }
>
>
>
> if ((EraseStatus == EFI_SUCCESS) && (IsZero1 == FALSE) && (IsZero2 ==
> TRUE) && ((IsZero3 == FALSE)))
>
> AssertionType = EFI_TEST_ASSERTION_PASSED;
>
> @@ -492,26 +508,41 @@ BlockIo2:
>
> // 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) {
>
> + for (Index1 = 0, EraseCounter = 0; Index1 < BlockSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero1 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=BlockSize) {
>
> + IsZero1 = FALSE;
>
> + }
>
>
>
> - for (Index1 = BlockSize; Index1 < BufferSize - BlockSize; Index1++) {
>
> - if (Buffer2[Index1] != 0) {
>
> + for (Index1 = BlockSize, EraseCounter = 0; Index1 < BufferSize -
> BlockSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero2 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=(BufferSize - (BlockSize*2))) {
>
> + IsZero2 = FALSE;
>
> + }
>
>
>
> - for (Index1 = BufferSize - BlockSize; Index1 < BufferSize; Index1++) {
>
> - if (Buffer2[Index1] != 0) {
>
> + for (Index1 = BufferSize - BlockSize, EraseCounter = 0; Index1 <
> BufferSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero3 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=BlockSize) {
>
> + IsZero3 = FALSE;
>
> + }
>
>
>
> if ((EraseStatus == EFI_SUCCESS) && (IsZero1 == FALSE) && (IsZero2 ==
> TRUE) && ((IsZero3 == FALSE)))
>
> AssertionType = EFI_TEST_ASSERTION_PASSED;
>
> --
>
> 2.26.2.windows.1
>
>
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#66857): https://edk2.groups.io/g/devel/message/66857
> Mute This Topic: https://groups.io/mt/77977762/1945644
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [samer.el-haj-
> mahmoud@arm.com]
> -=-=-=-=-=-=
>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply [flat|nested] 7+ messages in thread
* 回覆: [edk2-devel] [PATCH 2/2] uefi-sct/SctPkg: Correct BBTestEraseBlocks behavior (EFI_BLOCK_IO_PROTOCOL)
2020-11-11 20:56 ` [edk2-devel] " Samer El-Haj-Mahmoud
@ 2020-11-17 5:51 ` Chen, ArvinX
2020-11-17 13:53 ` Samer El-Haj-Mahmoud
2020-12-07 9:49 ` G Edhaya Chandran
1 sibling, 1 reply; 7+ messages in thread
From: Chen, ArvinX @ 2020-11-17 5:51 UTC (permalink / raw)
To: Samer El-Haj-Mahmoud, devel@edk2.groups.io
Cc: G Edhaya Chandran, Jin, Eric, gaoliming@byosoft.com.cn,
Chu, Maggie
[-- Attachment #1: Type: text/plain, Size: 8769 bytes --]
HI Samer,
Sorry for the slow reply, In EMMC's case, it allowed storage firmware erase to "1" to be a valid behavior (please reference the spec of emmc JESD84-B51/6.6.9), so once verification team use kind of this device, the test case will always failed. To avoid this problem, I think this change is required.
Thanks!!
Arvin
________________________________
寄件者: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
寄件日期: 2020年11月12日 上午 04:56
收件者: devel@edk2.groups.io <devel@edk2.groups.io>; Chen, ArvinX <arvinx.chen@intel.com>
副本: G Edhaya Chandran <Edhaya.Chandran@arm.com>; Jin, Eric <eric.jin@intel.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; gaoliming@byosoft.com.cn <gaoliming@byosoft.com.cn>
主旨: RE: [edk2-devel] [PATCH 2/2] uefi-sct/SctPkg: Correct BBTestEraseBlocks behavior (EFI_BLOCK_IO_PROTOCOL)
Hi Chen,
The UEFI Specification 2.8ErrataB (page 575) states that " It is the intention of the EraseBlocks() operation to be at least as performant as writing zeroes to each of the specified LBA locations while ensuring the equivalent security."
So while not explicitly saying that Erase should "erase to 0", it implies that at least is the intention. Do we know that erasing to "1" is a valid behavior?
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chen,
> ArvinX via groups.io
> Sent: Monday, November 2, 2020 5:00 AM
> To: devel@edk2.groups.io
> Cc: G Edhaya Chandran <Edhaya.Chandran@arm.com>; Eric Jin
> <eric.jin@intel.com>
> Subject: [edk2-devel] [PATCH 2/2] uefi-sct/SctPkg: Correct
> BBTestEraseBlocks behavior (EFI_BLOCK_IO_PROTOCOL)
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3022
>
>
>
> The storage device erase behavior may have two possibilities:
>
> 1.Write all data into "0"
>
> 2.Write all data into "1"
>
> but now tool behavior can only check case 1 (Write all data into "0"),
>
> so we need add the other case into SCT tool to correct the test behavior.
>
>
>
> Cc: G Edhaya Chandran <Edhaya.Chandran@arm.com>
>
> Cc: Eric Jin <eric.jin@intel.com>
>
> Signed-off-by: ArvinX Chen <arvinx.chen@intel.com>
>
> ---
>
> .../BlackBoxTest/EraseBlockBBTestFunction.c | 55 +++++++++++++++----
>
> 1 file changed, 43 insertions(+), 12 deletions(-)
>
>
>
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlock
> BBTestFunction.c b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlock
> BBTestFunction.c
>
> index cbf43e1d..dbbb70c6 100644
>
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlock
> BBTestFunction.c
>
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlock
> BBTestFunction.c
>
> @@ -71,6 +71,7 @@ BBTestEraseBlocksFunctionTest (
>
> UINT64 Index;
>
> UINTN Index1;
>
> UINTN Remainder;
>
> + UINT64 EraseCounter;
>
>
>
> EFI_ERASE_BLOCK_TOKEN Token;
>
> EFI_BLOCK_IO2_TOKEN BlockIo2Token;
>
> @@ -223,26 +224,41 @@ BBTestEraseBlocksFunctionTest (
>
> // 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) {
>
> + for (Index1 = 0, EraseCounter = 0; Index1 < BlockSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero1 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=BlockSize) {
>
> + IsZero1 = FALSE;
>
> + }
>
>
>
> - for (Index1 = BlockSize; Index1 < BufferSize - BlockSize; Index1++) {
>
> - if (Buffer2[Index1] != 0) {
>
> + for (Index1 = BlockSize, EraseCounter = 0; Index1 < BufferSize -
> BlockSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero2 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=(BufferSize - (BlockSize*2))) {
>
> + IsZero2 = FALSE;
>
> + }
>
>
>
> - for (Index1 = BufferSize - BlockSize; Index1 < BufferSize; Index1++) {
>
> - if (Buffer2[Index1] != 0) {
>
> + for (Index1 = BufferSize - BlockSize, EraseCounter = 0; Index1 <
> BufferSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero3 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=BlockSize) {
>
> + IsZero3 = FALSE;
>
> + }
>
>
>
> if ((EraseStatus == EFI_SUCCESS) && (IsZero1 == FALSE) && (IsZero2 ==
> TRUE) && ((IsZero3 == FALSE)))
>
> AssertionType = EFI_TEST_ASSERTION_PASSED;
>
> @@ -492,26 +508,41 @@ BlockIo2:
>
> // 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) {
>
> + for (Index1 = 0, EraseCounter = 0; Index1 < BlockSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero1 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=BlockSize) {
>
> + IsZero1 = FALSE;
>
> + }
>
>
>
> - for (Index1 = BlockSize; Index1 < BufferSize - BlockSize; Index1++) {
>
> - if (Buffer2[Index1] != 0) {
>
> + for (Index1 = BlockSize, EraseCounter = 0; Index1 < BufferSize -
> BlockSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero2 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=(BufferSize - (BlockSize*2))) {
>
> + IsZero2 = FALSE;
>
> + }
>
>
>
> - for (Index1 = BufferSize - BlockSize; Index1 < BufferSize; Index1++) {
>
> - if (Buffer2[Index1] != 0) {
>
> + for (Index1 = BufferSize - BlockSize, EraseCounter = 0; Index1 <
> BufferSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero3 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=BlockSize) {
>
> + IsZero3 = FALSE;
>
> + }
>
>
>
> if ((EraseStatus == EFI_SUCCESS) && (IsZero1 == FALSE) && (IsZero2 ==
> TRUE) && ((IsZero3 == FALSE)))
>
> AssertionType = EFI_TEST_ASSERTION_PASSED;
>
> --
>
> 2.26.2.windows.1
>
>
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#66857): https://edk2.groups.io/g/devel/message/66857
> Mute This Topic: https://groups.io/mt/77977762/1945644
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [samer.el-haj-
> mahmoud@arm.com]
> -=-=-=-=-=-=
>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
[-- Attachment #2: Type: text/html, Size: 18874 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH 2/2] uefi-sct/SctPkg: Correct BBTestEraseBlocks behavior (EFI_BLOCK_IO_PROTOCOL)
2020-11-17 5:51 ` 回覆: " Chen, ArvinX
@ 2020-11-17 13:53 ` Samer El-Haj-Mahmoud
2020-11-24 5:47 ` 回覆: " Chen, ArvinX
0 siblings, 1 reply; 7+ messages in thread
From: Samer El-Haj-Mahmoud @ 2020-11-17 13:53 UTC (permalink / raw)
To: devel@edk2.groups.io, arvinx.chen@intel.com
Cc: G Edhaya Chandran, Jin, Eric, gaoliming@byosoft.com.cn,
Chu, Maggie, Samer El-Haj-Mahmoud
[-- Attachment #1: Type: text/plain, Size: 10349 bytes --]
Thanks for the explanation.
Reviewed-By: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com<mailto:Samer.El-Haj-Mahmoud@arm.com>>
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chen, ArvinX via groups.io
Sent: Tuesday, November 17, 2020 12:51 AM
To: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; devel@edk2.groups.io
Cc: G Edhaya Chandran <Edhaya.Chandran@arm.com>; Jin, Eric <eric.jin@intel.com>; gaoliming@byosoft.com.cn; Chu, Maggie <maggie.chu@intel.com>
Subject: 回覆: [edk2-devel] [PATCH 2/2] uefi-sct/SctPkg: Correct BBTestEraseBlocks behavior (EFI_BLOCK_IO_PROTOCOL)
HI Samer,
Sorry for the slow reply, In EMMC's case, it allowed storage firmware erase to "1" to be a valid behavior (please reference the spec of emmc JESD84-B51/6.6.9), so once verification team use kind of this device, the test case will always failed. To avoid this problem, I think this change is required.
Thanks!!
Arvin
________________________________
寄件者: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com<mailto:Samer.El-Haj-Mahmoud@arm.com>>
寄件日期: 2020年11月12日 上午 04:56
收件者: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Chen, ArvinX <arvinx.chen@intel.com<mailto:arvinx.chen@intel.com>>
副本: G Edhaya Chandran <Edhaya.Chandran@arm.com<mailto:Edhaya.Chandran@arm.com>>; Jin, Eric <eric.jin@intel.com<mailto:eric.jin@intel.com>>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com<mailto:Samer.El-Haj-Mahmoud@arm.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn> <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
主旨: RE: [edk2-devel] [PATCH 2/2] uefi-sct/SctPkg: Correct BBTestEraseBlocks behavior (EFI_BLOCK_IO_PROTOCOL)
Hi Chen,
The UEFI Specification 2.8ErrataB (page 575) states that " It is the intention of the EraseBlocks() operation to be at least as performant as writing zeroes to each of the specified LBA locations while ensuring the equivalent security."
So while not explicitly saying that Erase should "erase to 0", it implies that at least is the intention. Do we know that erasing to "1" is a valid behavior?
> -----Original Message-----
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Chen,
> ArvinX via groups.io
> Sent: Monday, November 2, 2020 5:00 AM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Cc: G Edhaya Chandran <Edhaya.Chandran@arm.com<mailto:Edhaya.Chandran@arm.com>>; Eric Jin
> <eric.jin@intel.com<mailto:eric.jin@intel.com>>
> Subject: [edk2-devel] [PATCH 2/2] uefi-sct/SctPkg: Correct
> BBTestEraseBlocks behavior (EFI_BLOCK_IO_PROTOCOL)
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3022
>
>
>
> The storage device erase behavior may have two possibilities:
>
> 1.Write all data into "0"
>
> 2.Write all data into "1"
>
> but now tool behavior can only check case 1 (Write all data into "0"),
>
> so we need add the other case into SCT tool to correct the test behavior.
>
>
>
> Cc: G Edhaya Chandran <Edhaya.Chandran@arm.com<mailto:Edhaya.Chandran@arm.com>>
>
> Cc: Eric Jin <eric.jin@intel.com<mailto:eric.jin@intel.com>>
>
> Signed-off-by: ArvinX Chen <arvinx.chen@intel.com<mailto:arvinx.chen@intel.com>>
>
> ---
>
> .../BlackBoxTest/EraseBlockBBTestFunction.c | 55 +++++++++++++++----
>
> 1 file changed, 43 insertions(+), 12 deletions(-)
>
>
>
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlock
> BBTestFunction.c b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlock
> BBTestFunction.c
>
> index cbf43e1d..dbbb70c6 100644
>
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlock
> BBTestFunction.c
>
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlock
> BBTestFunction.c
>
> @@ -71,6 +71,7 @@ BBTestEraseBlocksFunctionTest (
>
> UINT64 Index;
>
> UINTN Index1;
>
> UINTN Remainder;
>
> + UINT64 EraseCounter;
>
>
>
> EFI_ERASE_BLOCK_TOKEN Token;
>
> EFI_BLOCK_IO2_TOKEN BlockIo2Token;
>
> @@ -223,26 +224,41 @@ BBTestEraseBlocksFunctionTest (
>
> // 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) {
>
> + for (Index1 = 0, EraseCounter = 0; Index1 < BlockSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero1 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=BlockSize) {
>
> + IsZero1 = FALSE;
>
> + }
>
>
>
> - for (Index1 = BlockSize; Index1 < BufferSize - BlockSize; Index1++) {
>
> - if (Buffer2[Index1] != 0) {
>
> + for (Index1 = BlockSize, EraseCounter = 0; Index1 < BufferSize -
> BlockSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero2 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=(BufferSize - (BlockSize*2))) {
>
> + IsZero2 = FALSE;
>
> + }
>
>
>
> - for (Index1 = BufferSize - BlockSize; Index1 < BufferSize; Index1++) {
>
> - if (Buffer2[Index1] != 0) {
>
> + for (Index1 = BufferSize - BlockSize, EraseCounter = 0; Index1 <
> BufferSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero3 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=BlockSize) {
>
> + IsZero3 = FALSE;
>
> + }
>
>
>
> if ((EraseStatus == EFI_SUCCESS) && (IsZero1 == FALSE) && (IsZero2 ==
> TRUE) && ((IsZero3 == FALSE)))
>
> AssertionType = EFI_TEST_ASSERTION_PASSED;
>
> @@ -492,26 +508,41 @@ BlockIo2:
>
> // 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) {
>
> + for (Index1 = 0, EraseCounter = 0; Index1 < BlockSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero1 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=BlockSize) {
>
> + IsZero1 = FALSE;
>
> + }
>
>
>
> - for (Index1 = BlockSize; Index1 < BufferSize - BlockSize; Index1++) {
>
> - if (Buffer2[Index1] != 0) {
>
> + for (Index1 = BlockSize, EraseCounter = 0; Index1 < BufferSize -
> BlockSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero2 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=(BufferSize - (BlockSize*2))) {
>
> + IsZero2 = FALSE;
>
> + }
>
>
>
> - for (Index1 = BufferSize - BlockSize; Index1 < BufferSize; Index1++) {
>
> - if (Buffer2[Index1] != 0) {
>
> + for (Index1 = BufferSize - BlockSize, EraseCounter = 0; Index1 <
> BufferSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero3 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=BlockSize) {
>
> + IsZero3 = FALSE;
>
> + }
>
>
>
> if ((EraseStatus == EFI_SUCCESS) && (IsZero1 == FALSE) && (IsZero2 ==
> TRUE) && ((IsZero3 == FALSE)))
>
> AssertionType = EFI_TEST_ASSERTION_PASSED;
>
> --
>
> 2.26.2.windows.1
>
>
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#66857): https://edk2.groups.io/g/devel/message/66857
> Mute This Topic: https://groups.io/mt/77977762/1945644
> Group Owner: devel+owner@edk2.groups.io<mailto:devel+owner@edk2.groups.io>
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [samer.el-haj-
> mahmoud@arm.com<mailto:mahmoud@arm.com>]
> -=-=-=-=-=-=
>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
[-- Attachment #2: Type: text/html, Size: 25416 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* 回覆: [edk2-devel] [PATCH 2/2] uefi-sct/SctPkg: Correct BBTestEraseBlocks behavior (EFI_BLOCK_IO_PROTOCOL)
2020-11-17 13:53 ` Samer El-Haj-Mahmoud
@ 2020-11-24 5:47 ` Chen, ArvinX
2020-11-24 5:49 ` G Edhaya Chandran
0 siblings, 1 reply; 7+ messages in thread
From: Chen, ArvinX @ 2020-11-24 5:47 UTC (permalink / raw)
To: devel@edk2.groups.io, samer.el-haj-mahmoud@arm.com
Cc: G Edhaya Chandran, Jin, Eric, gaoliming@byosoft.com.cn,
Chu, Maggie
[-- Attachment #1: Type: text/plain, Size: 11042 bytes --]
Hi all,
If this patch is OK to you guys~ maybe we can submit it into master ^^
Thanks!
Arvin
________________________________
寄件者: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
寄件日期: 2020年11月17日 下午 09:53
收件者: devel@edk2.groups.io <devel@edk2.groups.io>; Chen, ArvinX <arvinx.chen@intel.com>
副本: G Edhaya Chandran <Edhaya.Chandran@arm.com>; Jin, Eric <eric.jin@intel.com>; gaoliming@byosoft.com.cn <gaoliming@byosoft.com.cn>; Chu, Maggie <maggie.chu@intel.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
主旨: Re: [edk2-devel] [PATCH 2/2] uefi-sct/SctPkg: Correct BBTestEraseBlocks behavior (EFI_BLOCK_IO_PROTOCOL)
Thanks for the explanation.
Reviewed-By: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com<mailto:Samer.El-Haj-Mahmoud@arm.com>>
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chen, ArvinX via groups.io
Sent: Tuesday, November 17, 2020 12:51 AM
To: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; devel@edk2.groups.io
Cc: G Edhaya Chandran <Edhaya.Chandran@arm.com>; Jin, Eric <eric.jin@intel.com>; gaoliming@byosoft.com.cn; Chu, Maggie <maggie.chu@intel.com>
Subject: 回覆: [edk2-devel] [PATCH 2/2] uefi-sct/SctPkg: Correct BBTestEraseBlocks behavior (EFI_BLOCK_IO_PROTOCOL)
HI Samer,
Sorry for the slow reply, In EMMC's case, it allowed storage firmware erase to "1" to be a valid behavior (please reference the spec of emmc JESD84-B51/6.6.9), so once verification team use kind of this device, the test case will always failed. To avoid this problem, I think this change is required.
Thanks!!
Arvin
________________________________
寄件者: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com<mailto:Samer.El-Haj-Mahmoud@arm.com>>
寄件日期: 2020年11月12日 上午 04:56
收件者: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Chen, ArvinX <arvinx.chen@intel.com<mailto:arvinx.chen@intel.com>>
副本: G Edhaya Chandran <Edhaya.Chandran@arm.com<mailto:Edhaya.Chandran@arm.com>>; Jin, Eric <eric.jin@intel.com<mailto:eric.jin@intel.com>>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com<mailto:Samer.El-Haj-Mahmoud@arm.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn> <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
主旨: RE: [edk2-devel] [PATCH 2/2] uefi-sct/SctPkg: Correct BBTestEraseBlocks behavior (EFI_BLOCK_IO_PROTOCOL)
Hi Chen,
The UEFI Specification 2.8ErrataB (page 575) states that " It is the intention of the EraseBlocks() operation to be at least as performant as writing zeroes to each of the specified LBA locations while ensuring the equivalent security."
So while not explicitly saying that Erase should "erase to 0", it implies that at least is the intention. Do we know that erasing to "1" is a valid behavior?
> -----Original Message-----
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Chen,
> ArvinX via groups.io
> Sent: Monday, November 2, 2020 5:00 AM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Cc: G Edhaya Chandran <Edhaya.Chandran@arm.com<mailto:Edhaya.Chandran@arm.com>>; Eric Jin
> <eric.jin@intel.com<mailto:eric.jin@intel.com>>
> Subject: [edk2-devel] [PATCH 2/2] uefi-sct/SctPkg: Correct
> BBTestEraseBlocks behavior (EFI_BLOCK_IO_PROTOCOL)
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3022
>
>
>
> The storage device erase behavior may have two possibilities:
>
> 1.Write all data into "0"
>
> 2.Write all data into "1"
>
> but now tool behavior can only check case 1 (Write all data into "0"),
>
> so we need add the other case into SCT tool to correct the test behavior.
>
>
>
> Cc: G Edhaya Chandran <Edhaya.Chandran@arm.com<mailto:Edhaya.Chandran@arm.com>>
>
> Cc: Eric Jin <eric.jin@intel.com<mailto:eric.jin@intel.com>>
>
> Signed-off-by: ArvinX Chen <arvinx.chen@intel.com<mailto:arvinx.chen@intel.com>>
>
> ---
>
> .../BlackBoxTest/EraseBlockBBTestFunction.c | 55 +++++++++++++++----
>
> 1 file changed, 43 insertions(+), 12 deletions(-)
>
>
>
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlock
> BBTestFunction.c b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlock
> BBTestFunction.c
>
> index cbf43e1d..dbbb70c6 100644
>
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlock
> BBTestFunction.c
>
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlock
> BBTestFunction.c
>
> @@ -71,6 +71,7 @@ BBTestEraseBlocksFunctionTest (
>
> UINT64 Index;
>
> UINTN Index1;
>
> UINTN Remainder;
>
> + UINT64 EraseCounter;
>
>
>
> EFI_ERASE_BLOCK_TOKEN Token;
>
> EFI_BLOCK_IO2_TOKEN BlockIo2Token;
>
> @@ -223,26 +224,41 @@ BBTestEraseBlocksFunctionTest (
>
> // 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) {
>
> + for (Index1 = 0, EraseCounter = 0; Index1 < BlockSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero1 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=BlockSize) {
>
> + IsZero1 = FALSE;
>
> + }
>
>
>
> - for (Index1 = BlockSize; Index1 < BufferSize - BlockSize; Index1++) {
>
> - if (Buffer2[Index1] != 0) {
>
> + for (Index1 = BlockSize, EraseCounter = 0; Index1 < BufferSize -
> BlockSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero2 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=(BufferSize - (BlockSize*2))) {
>
> + IsZero2 = FALSE;
>
> + }
>
>
>
> - for (Index1 = BufferSize - BlockSize; Index1 < BufferSize; Index1++) {
>
> - if (Buffer2[Index1] != 0) {
>
> + for (Index1 = BufferSize - BlockSize, EraseCounter = 0; Index1 <
> BufferSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero3 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=BlockSize) {
>
> + IsZero3 = FALSE;
>
> + }
>
>
>
> if ((EraseStatus == EFI_SUCCESS) && (IsZero1 == FALSE) && (IsZero2 ==
> TRUE) && ((IsZero3 == FALSE)))
>
> AssertionType = EFI_TEST_ASSERTION_PASSED;
>
> @@ -492,26 +508,41 @@ BlockIo2:
>
> // 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) {
>
> + for (Index1 = 0, EraseCounter = 0; Index1 < BlockSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero1 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=BlockSize) {
>
> + IsZero1 = FALSE;
>
> + }
>
>
>
> - for (Index1 = BlockSize; Index1 < BufferSize - BlockSize; Index1++) {
>
> - if (Buffer2[Index1] != 0) {
>
> + for (Index1 = BlockSize, EraseCounter = 0; Index1 < BufferSize -
> BlockSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero2 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=(BufferSize - (BlockSize*2))) {
>
> + IsZero2 = FALSE;
>
> + }
>
>
>
> - for (Index1 = BufferSize - BlockSize; Index1 < BufferSize; Index1++) {
>
> - if (Buffer2[Index1] != 0) {
>
> + for (Index1 = BufferSize - BlockSize, EraseCounter = 0; Index1 <
> BufferSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero3 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=BlockSize) {
>
> + IsZero3 = FALSE;
>
> + }
>
>
>
> if ((EraseStatus == EFI_SUCCESS) && (IsZero1 == FALSE) && (IsZero2 ==
> TRUE) && ((IsZero3 == FALSE)))
>
> AssertionType = EFI_TEST_ASSERTION_PASSED;
>
> --
>
> 2.26.2.windows.1
>
>
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#66857): https://edk2.groups.io/g/devel/message/66857
> Mute This Topic: https://groups.io/mt/77977762/1945644
> Group Owner: devel+owner@edk2.groups.io<mailto:devel+owner@edk2.groups.io>
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [samer.el-haj-
> mahmoud@arm.com<mailto:mahmoud@arm.com>]
> -=-=-=-=-=-=
>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
[-- Attachment #2: Type: text/html, Size: 26333 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH 2/2] uefi-sct/SctPkg: Correct BBTestEraseBlocks behavior (EFI_BLOCK_IO_PROTOCOL)
2020-11-24 5:47 ` 回覆: " Chen, ArvinX
@ 2020-11-24 5:49 ` G Edhaya Chandran
0 siblings, 0 replies; 7+ messages in thread
From: G Edhaya Chandran @ 2020-11-24 5:49 UTC (permalink / raw)
To: Chen, ArvinX, devel@edk2.groups.io, Samer El-Haj-Mahmoud
Cc: Jin, Eric, gaoliming@byosoft.com.cn, Chu, Maggie
[-- Attachment #1: Type: text/plain, Size: 12632 bytes --]
Hi Arvin,
This patch is scheduled to be submitted in master today.
With Warm Regards,
Edhay
From: Chen, ArvinX <arvinx.chen@intel.com>
Sent: 24 November 2020 11:17
To: devel@edk2.groups.io; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Cc: G Edhaya Chandran <Edhaya.Chandran@arm.com>; Jin, Eric <eric.jin@intel.com>; gaoliming@byosoft.com.cn; Chu, Maggie <maggie.chu@intel.com>
Subject: 回覆: [edk2-devel] [PATCH 2/2] uefi-sct/SctPkg: Correct BBTestEraseBlocks behavior (EFI_BLOCK_IO_PROTOCOL)
Hi all,
If this patch is OK to you guys~ maybe we can submit it into master ^^
Thanks!
Arvin
________________________________
寄件者: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> 代表 Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com<mailto:samer.el-haj-mahmoud@arm.com>>
寄件日期: 2020年11月17日 下午 09:53
收件者: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Chen, ArvinX <arvinx.chen@intel.com<mailto:arvinx.chen@intel.com>>
副本: G Edhaya Chandran <Edhaya.Chandran@arm.com<mailto:Edhaya.Chandran@arm.com>>; Jin, Eric <eric.jin@intel.com<mailto:eric.jin@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn> <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Chu, Maggie <maggie.chu@intel.com<mailto:maggie.chu@intel.com>>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com<mailto:Samer.El-Haj-Mahmoud@arm.com>>
主旨: Re: [edk2-devel] [PATCH 2/2] uefi-sct/SctPkg: Correct BBTestEraseBlocks behavior (EFI_BLOCK_IO_PROTOCOL)
Thanks for the explanation.
Reviewed-By: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com<mailto:Samer.El-Haj-Mahmoud@arm.com>>
From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Chen, ArvinX via groups.io
Sent: Tuesday, November 17, 2020 12:51 AM
To: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com<mailto:Samer.El-Haj-Mahmoud@arm.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: G Edhaya Chandran <Edhaya.Chandran@arm.com<mailto:Edhaya.Chandran@arm.com>>; Jin, Eric <eric.jin@intel.com<mailto:eric.jin@intel.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>; Chu, Maggie <maggie.chu@intel.com<mailto:maggie.chu@intel.com>>
Subject: 回覆: [edk2-devel] [PATCH 2/2] uefi-sct/SctPkg: Correct BBTestEraseBlocks behavior (EFI_BLOCK_IO_PROTOCOL)
HI Samer,
Sorry for the slow reply, In EMMC's case, it allowed storage firmware erase to "1" to be a valid behavior (please reference the spec of emmc JESD84-B51/6.6.9), so once verification team use kind of this device, the test case will always failed. To avoid this problem, I think this change is required.
Thanks!!
Arvin
________________________________
寄件者: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com<mailto:Samer.El-Haj-Mahmoud@arm.com>>
寄件日期: 2020年11月12日 上午 04:56
收件者: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Chen, ArvinX <arvinx.chen@intel.com<mailto:arvinx.chen@intel.com>>
副本: G Edhaya Chandran <Edhaya.Chandran@arm.com<mailto:Edhaya.Chandran@arm.com>>; Jin, Eric <eric.jin@intel.com<mailto:eric.jin@intel.com>>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com<mailto:Samer.El-Haj-Mahmoud@arm.com>>; gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn> <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>
主旨: RE: [edk2-devel] [PATCH 2/2] uefi-sct/SctPkg: Correct BBTestEraseBlocks behavior (EFI_BLOCK_IO_PROTOCOL)
Hi Chen,
The UEFI Specification 2.8ErrataB (page 575) states that " It is the intention of the EraseBlocks() operation to be at least as performant as writing zeroes to each of the specified LBA locations while ensuring the equivalent security."
So while not explicitly saying that Erase should "erase to 0", it implies that at least is the intention. Do we know that erasing to "1" is a valid behavior?
> -----Original Message-----
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Chen,
> ArvinX via groups.io
> Sent: Monday, November 2, 2020 5:00 AM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Cc: G Edhaya Chandran <Edhaya.Chandran@arm.com<mailto:Edhaya.Chandran@arm.com>>; Eric Jin
> <eric.jin@intel.com<mailto:eric.jin@intel.com>>
> Subject: [edk2-devel] [PATCH 2/2] uefi-sct/SctPkg: Correct
> BBTestEraseBlocks behavior (EFI_BLOCK_IO_PROTOCOL)
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3022
>
>
>
> The storage device erase behavior may have two possibilities:
>
> 1.Write all data into "0"
>
> 2.Write all data into "1"
>
> but now tool behavior can only check case 1 (Write all data into "0"),
>
> so we need add the other case into SCT tool to correct the test behavior.
>
>
>
> Cc: G Edhaya Chandran <Edhaya.Chandran@arm.com<mailto:Edhaya.Chandran@arm.com>>
>
> Cc: Eric Jin <eric.jin@intel.com<mailto:eric.jin@intel.com>>
>
> Signed-off-by: ArvinX Chen <arvinx.chen@intel.com<mailto:arvinx.chen@intel.com>>
>
> ---
>
> .../BlackBoxTest/EraseBlockBBTestFunction.c | 55 +++++++++++++++----
>
> 1 file changed, 43 insertions(+), 12 deletions(-)
>
>
>
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlock
> BBTestFunction.c b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlock
> BBTestFunction.c
>
> index cbf43e1d..dbbb70c6 100644
>
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlock
> BBTestFunction.c
>
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/EraseBlock/BlackBoxTest/EraseBlock
> BBTestFunction.c
>
> @@ -71,6 +71,7 @@ BBTestEraseBlocksFunctionTest (
>
> UINT64 Index;
>
> UINTN Index1;
>
> UINTN Remainder;
>
> + UINT64 EraseCounter;
>
>
>
> EFI_ERASE_BLOCK_TOKEN Token;
>
> EFI_BLOCK_IO2_TOKEN BlockIo2Token;
>
> @@ -223,26 +224,41 @@ BBTestEraseBlocksFunctionTest (
>
> // 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) {
>
> + for (Index1 = 0, EraseCounter = 0; Index1 < BlockSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero1 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=BlockSize) {
>
> + IsZero1 = FALSE;
>
> + }
>
>
>
> - for (Index1 = BlockSize; Index1 < BufferSize - BlockSize; Index1++) {
>
> - if (Buffer2[Index1] != 0) {
>
> + for (Index1 = BlockSize, EraseCounter = 0; Index1 < BufferSize -
> BlockSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero2 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=(BufferSize - (BlockSize*2))) {
>
> + IsZero2 = FALSE;
>
> + }
>
>
>
> - for (Index1 = BufferSize - BlockSize; Index1 < BufferSize; Index1++) {
>
> - if (Buffer2[Index1] != 0) {
>
> + for (Index1 = BufferSize - BlockSize, EraseCounter = 0; Index1 <
> BufferSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero3 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=BlockSize) {
>
> + IsZero3 = FALSE;
>
> + }
>
>
>
> if ((EraseStatus == EFI_SUCCESS) && (IsZero1 == FALSE) && (IsZero2 ==
> TRUE) && ((IsZero3 == FALSE)))
>
> AssertionType = EFI_TEST_ASSERTION_PASSED;
>
> @@ -492,26 +508,41 @@ BlockIo2:
>
> // 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) {
>
> + for (Index1 = 0, EraseCounter = 0; Index1 < BlockSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero1 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=BlockSize) {
>
> + IsZero1 = FALSE;
>
> + }
>
>
>
> - for (Index1 = BlockSize; Index1 < BufferSize - BlockSize; Index1++) {
>
> - if (Buffer2[Index1] != 0) {
>
> + for (Index1 = BlockSize, EraseCounter = 0; Index1 < BufferSize -
> BlockSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero2 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=(BufferSize - (BlockSize*2))) {
>
> + IsZero2 = FALSE;
>
> + }
>
>
>
> - for (Index1 = BufferSize - BlockSize; Index1 < BufferSize; Index1++) {
>
> - if (Buffer2[Index1] != 0) {
>
> + for (Index1 = BufferSize - BlockSize, EraseCounter = 0; Index1 <
> BufferSize; Index1++) {
>
> + if (Buffer2[Index1] != 0x00 && Buffer2[Index1] != 0xFF) {
>
> IsZero3 = FALSE;
>
> break;
>
> + } else if (Buffer2[Index1] == 0x00) {
>
> + EraseCounter++;
>
> }
>
> }
>
> + if (EraseCounter!=0 && EraseCounter!=BlockSize) {
>
> + IsZero3 = FALSE;
>
> + }
>
>
>
> if ((EraseStatus == EFI_SUCCESS) && (IsZero1 == FALSE) && (IsZero2 ==
> TRUE) && ((IsZero3 == FALSE)))
>
> AssertionType = EFI_TEST_ASSERTION_PASSED;
>
> --
>
> 2.26.2.windows.1
>
>
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#66857): https://edk2.groups.io/g/devel/message/66857
> Mute This Topic: https://groups.io/mt/77977762/1945644
> Group Owner: devel+owner@edk2.groups.io<mailto:devel+owner@edk2.groups.io>
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [samer.el-haj-
> mahmoud@arm.com<mailto:mahmoud@arm.com>]
> -=-=-=-=-=-=
>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
[-- Attachment #2: Type: text/html, Size: 33479 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH 2/2] uefi-sct/SctPkg: Correct BBTestEraseBlocks behavior (EFI_BLOCK_IO_PROTOCOL)
2020-11-11 20:56 ` [edk2-devel] " Samer El-Haj-Mahmoud
2020-11-17 5:51 ` 回覆: " Chen, ArvinX
@ 2020-12-07 9:49 ` G Edhaya Chandran
1 sibling, 0 replies; 7+ messages in thread
From: G Edhaya Chandran @ 2020-12-07 9:49 UTC (permalink / raw)
To: Samer El-Haj-Mahmoud, devel
[-- Attachment #1: Type: text/plain, Size: 173 bytes --]
Reviewed-by: G Edhaya Chandran<edhaya.chandran@arm.com>
Upstreamed by commit-id: https://github.com/tianocore/edk2-test/commit/cf7b43c130816f50aca1f10871cb9cf29e386223
[-- Attachment #2: Type: text/html, Size: 191 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-12-07 9:49 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-11-02 9:59 [PATCH 2/2] uefi-sct/SctPkg: Correct BBTestEraseBlocks behavior (EFI_BLOCK_IO_PROTOCOL) Chen, ArvinX
2020-11-11 20:56 ` [edk2-devel] " Samer El-Haj-Mahmoud
2020-11-17 5:51 ` 回覆: " Chen, ArvinX
2020-11-17 13:53 ` Samer El-Haj-Mahmoud
2020-11-24 5:47 ` 回覆: " Chen, ArvinX
2020-11-24 5:49 ` G Edhaya Chandran
2020-12-07 9:49 ` G Edhaya Chandran
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox