* [edk2-test][Patch V2 1/1] uefi-sct/SctPkg: Eliminate 2nd execution of ExitBootServices Test
@ 2019-08-23 5:46 Eric Jin
2019-08-23 16:51 ` Supreeth Venkatesh
0 siblings, 1 reply; 3+ messages in thread
From: Eric Jin @ 2019-08-23 5:46 UTC (permalink / raw)
To: devel; +Cc: Supreeth Venkatesh
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2098
In the ExitBootServices() test, after ExitBootServices() call, all
boot services are forbidden. The original design is to save the return
status value of ExitBootServices() in variable using variable service
and reset, but this needs one additional execution of the test to
retrieve the value from variable and this design was not straightforward
from end user perspective.
This patch enhances the test by leveraging RecoveryLib to restore
execution after reset automatically, thus requiring only one execution.
Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
Signed-off-by: Eric Jin <eric.jin@intel.com>
---
uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTest.inf | 3 ++-
uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTest.h | 9 ++++++++-
uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTestConformance.c | 121 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------
3 files changed, 115 insertions(+), 18 deletions(-)
diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTest.inf b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTest.inf
index 49ad79915934..3de43a20e8a4 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTest.inf
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTest.inf
@@ -1,7 +1,7 @@
## @file
#
# Copyright 2006 - 2012 Unified EFI, Inc.<BR>
-# Copyright (c) 2010 - 2012, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2010 - 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
@@ -53,4 +53,5 @@
[Protocols]
gEfiTestProfileLibraryGuid
+ gEfiTestRecoveryLibraryGuid
gBlackBoxEfiHIIPackageListProtocolGuid
diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTest.h b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTest.h
index b1c35fee7435..008584577ed1 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTest.h
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTest.h
@@ -1,7 +1,7 @@
/** @file
Copyright 2006 - 2017 Unified EFI, Inc.<BR>
- Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2010 - 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
@@ -35,6 +35,13 @@ Abstract:
#include EFI_PROTOCOL_DEFINITION (LoadFile)
#include EFI_TEST_PROTOCOL_DEFINITION (TestProfileLibrary)
+#include EFI_TEST_PROTOCOL_DEFINITION (TestRecoveryLibrary)
+
+typedef struct _RESET_DATA {
+ UINTN Step;
+ UINTN TplIndex;
+ UINT32 RepeatTimes;
+} RESET_DATA;
#if (EFI_SPECIFICATION_VERSION >= 0x0002000A)
diff --git a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTestConformance.c b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTestConformance.c
index 0a26d46847da..6ce1d2d72669 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTestConformance.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/ImageBBTestConformance.c
@@ -1,7 +1,7 @@
/** @file
Copyright 2006 - 2016 Unified EFI, Inc.<BR>
- Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2010 - 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
@@ -30,7 +30,8 @@ Abstract:
#define TEST_VENDOR1_GUID \
{ 0xF6FAB04F, 0xACAF, 0x4af3, { 0xB9, 0xFA, 0xDC, 0xF9, 0x7F, 0xB4, 0x42, 0x6F } }
-#define MAX_BUFFER_SIZE 10
+#define STATUS_BUFFER_SIZE 8
+#define RECOVER_BUFFER_SIZE 1024
EFI_GUID gTestVendor1Guid = TEST_VENDOR1_GUID;
@@ -778,19 +779,23 @@ BBTestExitBootServicesConsistencyTest (
)
{
EFI_STANDARD_TEST_LIBRARY_PROTOCOL *StandardLib;
+ EFI_TEST_RECOVERY_LIBRARY_PROTOCOL *RecoveryLib;
EFI_STATUS Status;
EFI_TEST_ASSERTION AssertionType;
UINTN MapKey;
-
+ UINTN Size;
UINTN Numbers;
UINTN DataSize;
- UINT8 Data[MAX_BUFFER_SIZE];
+ RESET_DATA *ResetData;
+ UINT8 Data[STATUS_BUFFER_SIZE];
+ UINT8 Buffer[RECOVER_BUFFER_SIZE];
EFI_STATUS ReturnStatus;
//
// Init
//
StandardLib = NULL;
+ RecoveryLib = NULL;
//
// Get the Standard Library Interface
@@ -803,6 +808,14 @@ BBTestExitBootServicesConsistencyTest (
return Status;
}
+ Status = gtBS->HandleProtocol (
+ SupportHandle,
+ &gEfiTestRecoveryLibraryGuid,
+ (VOID **) &RecoveryLib);
+ if (EFI_ERROR(Status)) {
+ return Status;
+ }
+
Status = ImageTestCheckForCleanEnvironment (&Numbers);
if (EFI_ERROR(Status)) {
StandardLib->RecordAssertion (
@@ -819,25 +832,101 @@ BBTestExitBootServicesConsistencyTest (
return Status;
}
- DataSize = MAX_BUFFER_SIZE;
- Status = gtRT->GetVariable (
- L"ExitBootServicesTestVariable", // VariableName
- &gTestVendor1Guid, // VendorGuid
- NULL, // Attributes
- &DataSize, // DataSize
- &ReturnStatus // Data
- );
+ //
+ // Read reset record
+ //
+ Status = RecoveryLib->ReadResetRecord (
+ RecoveryLib,
+ &Size,
+ Buffer
+ );
+ ResetData = (RESET_DATA *)Buffer;
- if (Status == EFI_SUCCESS) {
- goto CheckResult;
+ //
+ // The workflow to check the recovery data
+ //
+ if (EFI_SUCCESS == Status) {
+ //
+ // To check the recovery data size
+ //
+ if (Size == sizeof(RESET_DATA)){
+ //
+ // To Check the recovery data
+ //
+ if (ResetData->Step == 1) {
+ //
+ // Step 1: find the valid recovery data, to retrive presaved status from variable
+ //
+ DataSize = STATUS_BUFFER_SIZE;
+ Status = gtRT->GetVariable (
+ L"ExitBootServicesTestVariable", // VariableName
+ &gTestVendor1Guid, // VendorGuid
+ NULL, // Attributes
+ &DataSize, // DataSize
+ &ReturnStatus // Data
+ );
+
+ if (EFI_ERROR(Status)) {
+ StandardLib->RecordAssertion (
+ StandardLib,
+ EFI_TEST_ASSERTION_FAILED,
+ gTestGenericFailureGuid,
+ L"GetVariable - Can't get the test variable - ExitBootServicesTestVariable",
+ L"%a:%d:Status - %r",
+ __FILE__,
+ (UINTN)__LINE__,
+ Status
+ );
+ return Status;
+ }
+ goto CheckResult;
+ } else {
+ //
+ // It is invalid recovery data
+ //
+ return EFI_LOAD_ERROR;
+ }
+ } else {
+ //
+ // The size of recovery data is invalid
+ //
+ return EFI_LOAD_ERROR;
+ }
}
+ //
+ // Step 0: No recovery data is found.
+ //
+
//
// Print out some information to avoid the user thought it is an error
//
- SctPrint (L"System will cold reset after 2 second. please run this test again...");
+ SctPrint (L"System will cold reset after 2 second and test will be resumed after reboot.");
gtBS->Stall (2000000);
+
+ ResetData->Step = 1;
+ ResetData->TplIndex = 0;
+ Status = RecoveryLib->WriteResetRecord (
+ RecoveryLib,
+ sizeof (RESET_DATA),
+ (UINT8*)ResetData
+ );
+ if (EFI_ERROR(Status)) {
+ StandardLib->RecordAssertion (
+ StandardLib,
+ EFI_TEST_ASSERTION_FAILED,
+ gTestGenericFailureGuid,
+ L"TestRecoveryLib - WriteResetRecord",
+ L"%a:%d:Status - %r",
+ __FILE__,
+ (UINTN)__LINE__,
+ Status
+ );
+ return Status;
+ }
+
+
//
// Checkpoint 1:
// 3.5.2.1 ExitBootServices should not succeed with an invalid MapKey
@@ -885,7 +974,7 @@ BBTestExitBootServicesConsistencyTest (
//reset system
gtRT->ResetSystem (EfiResetCold, EFI_SUCCESS, 0, NULL);
- // get var to get the status
+
CheckResult:
if (ReturnStatus == EFI_INVALID_PARAMETER) {
--
2.20.0.windows.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [edk2-test][Patch V2 1/1] uefi-sct/SctPkg: Eliminate 2nd execution of ExitBootServices Test
2019-08-23 5:46 [edk2-test][Patch V2 1/1] uefi-sct/SctPkg: Eliminate 2nd execution of ExitBootServices Test Eric Jin
@ 2019-08-23 16:51 ` Supreeth Venkatesh
2019-08-26 9:24 ` Eric Jin
0 siblings, 1 reply; 3+ messages in thread
From: Supreeth Venkatesh @ 2019-08-23 16:51 UTC (permalink / raw)
To: Eric Jin, devel@edk2.groups.io
On Fri, 2019-08-23 at 00:46 -0500, Eric Jin wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2098
>
> In the ExitBootServices() test, after ExitBootServices() call, all
> boot services are forbidden. The original design is to save the
> return
> status value of ExitBootServices() in variable using variable service
> and reset, but this needs one additional execution of the test to
> retrieve the value from variable and this design was not
> straightforward
> from end user perspective.
> This patch enhances the test by leveraging RecoveryLib to restore
> execution after reset automatically, thus requiring only one
> execution.
>
> Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> Signed-off-by: Eric Jin <eric.jin@intel.com>
Please re-order the statements like below to make it more clear since
ResetData is a just a pointer to Buffer Array which is statically
allocated to 1024 and ResetData is not being really initialized to
Reset Record after reading reset record, but to Buffer Array.
1. ResetData = (RESET_DATA *)Buffer;
2. RecoveryLib->ReadResetRecord(RecoveryLib, Size, Buffer);
while commiting it.
With that,
Reviewed-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> ---
> uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTest.inf | 3 ++-
> uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTest.h | 9 ++++++++-
> uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTestConformance.c | 121
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++++++++++++++++++++++++++++++++++----------------
> 3 files changed, 115 insertions(+), 18 deletions(-)
>
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTest.inf b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTest.inf
> index 49ad79915934..3de43a20e8a4 100644
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTest.inf
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTest.inf
> @@ -1,7 +1,7 @@
> ## @file
> #
> # Copyright 2006 - 2012 Unified EFI, Inc.<BR>
> -# Copyright (c) 2010 - 2012, Intel Corporation. All rights
> reserved.<BR>
> +# Copyright (c) 2010 - 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
> @@ -53,4 +53,5 @@
>
> [Protocols]
> gEfiTestProfileLibraryGuid
> + gEfiTestRecoveryLibraryGuid
> gBlackBoxEfiHIIPackageListProtocolGuid
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTest.h b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTest.h
> index b1c35fee7435..008584577ed1 100644
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTest.h
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTest.h
> @@ -1,7 +1,7 @@
> /** @file
>
> Copyright 2006 - 2017 Unified EFI, Inc.<BR>
> - Copyright (c) 2010 - 2017, Intel Corporation. All rights
> reserved.<BR>
> + Copyright (c) 2010 - 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
> @@ -35,6 +35,13 @@ Abstract:
> #include EFI_PROTOCOL_DEFINITION (LoadFile)
>
> #include EFI_TEST_PROTOCOL_DEFINITION (TestProfileLibrary)
> +#include EFI_TEST_PROTOCOL_DEFINITION (TestRecoveryLibrary)
> +
> +typedef struct _RESET_DATA {
> + UINTN Step;
> + UINTN TplIndex;
> + UINT32 RepeatTimes;
> +} RESET_DATA;
>
> #if (EFI_SPECIFICATION_VERSION >= 0x0002000A)
>
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTestConformance.c b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTestConformance.c
> index 0a26d46847da..6ce1d2d72669 100644
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTestConformance.c
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTestConformance.c
> @@ -1,7 +1,7 @@
> /** @file
>
> Copyright 2006 - 2016 Unified EFI, Inc.<BR>
> - Copyright (c) 2010 - 2016, Intel Corporation. All rights
> reserved.<BR>
> + Copyright (c) 2010 - 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
> @@ -30,7 +30,8 @@ Abstract:
> #define TEST_VENDOR1_GUID \
> { 0xF6FAB04F, 0xACAF, 0x4af3, { 0xB9, 0xFA, 0xDC, 0xF9, 0x7F,
> 0xB4, 0x42, 0x6F } }
>
> -#define MAX_BUFFER_SIZE 10
> +#define STATUS_BUFFER_SIZE 8
> +#define RECOVER_BUFFER_SIZE 1024
>
> EFI_GUID gTestVendor1Guid = TEST_VENDOR1_GUID;
>
> @@ -778,19 +779,23 @@ BBTestExitBootServicesConsistencyTest (
> )
> {
> EFI_STANDARD_TEST_LIBRARY_PROTOCOL *StandardLib;
> + EFI_TEST_RECOVERY_LIBRARY_PROTOCOL *RecoveryLib;
> EFI_STATUS Status;
> EFI_TEST_ASSERTION AssertionType;
> UINTN MapKey;
> -
> + UINTN Size;
> UINTN Numbers;
> UINTN DataSize;
> - UINT8 Data[MAX_BUFFER_SIZE];
> + RESET_DATA *ResetData;
> + UINT8 Data[STATUS_BUFFER_SIZE];
> + UINT8 Buffer[RECOVER_BUFFER_SIZE];
> EFI_STATUS ReturnStatus;
>
> //
> // Init
> //
> StandardLib = NULL;
> + RecoveryLib = NULL;
>
> //
> // Get the Standard Library Interface
> @@ -803,6 +808,14 @@ BBTestExitBootServicesConsistencyTest (
> return Status;
> }
>
> + Status = gtBS->HandleProtocol (
> + SupportHandle,
> + &gEfiTestRecoveryLibraryGuid,
> + (VOID **) &RecoveryLib);
> + if (EFI_ERROR(Status)) {
> + return Status;
> + }
> +
> Status = ImageTestCheckForCleanEnvironment (&Numbers);
> if (EFI_ERROR(Status)) {
> StandardLib->RecordAssertion (
> @@ -819,25 +832,101 @@ BBTestExitBootServicesConsistencyTest (
> return Status;
> }
>
> - DataSize = MAX_BUFFER_SIZE;
> - Status = gtRT->GetVariable (
> - L"ExitBootServicesTestVariable", //
> VariableName
> - &gTestVendor1Guid, //
> VendorGuid
> - NULL, //
> Attributes
> - &DataSize, //
> DataSize
> - &ReturnStatus //
> Data
> - );
> + //
> + // Read reset record
> + //
> + Status = RecoveryLib->ReadResetRecord (
> + RecoveryLib,
> + &Size,
> + Buffer
> + );
> + ResetData = (RESET_DATA *)Buffer;
>
> - if (Status == EFI_SUCCESS) {
> - goto CheckResult;
> + //
> + // The workflow to check the recovery data
> + //
> + if (EFI_SUCCESS == Status) {
> + //
> + // To check the recovery data size
> + //
> + if (Size == sizeof(RESET_DATA)){
> + //
> + // To Check the recovery data
> + //
> + if (ResetData->Step == 1) {
> + //
> + // Step 1: find the valid recovery data, to retrive presaved
> status from variable
> + //
> + DataSize = STATUS_BUFFER_SIZE;
> + Status = gtRT->GetVariable (
> + L"ExitBootServicesTestVariable", //
> VariableName
> + &gTestVendor1Guid, //
> VendorGuid
> + NULL, //
> Attributes
> + &DataSize, //
> DataSize
> + &ReturnStatus // Data
> + );
> +
> + if (EFI_ERROR(Status)) {
> + StandardLib->RecordAssertion (
> + StandardLib,
> + EFI_TEST_ASSERTION_FAILED,
> + gTestGenericFailureGuid,
> + L"GetVariable - Can't get the test variable -
> ExitBootServicesTestVariable",
> + L"%a:%d:Status - %r",
> + __FILE__,
> + (UINTN)__LINE__,
> + Status
> + );
> + return Status;
> + }
> + goto CheckResult;
> + } else {
> + //
> + // It is invalid recovery data
> + //
> + return EFI_LOAD_ERROR;
> + }
> + } else {
> + //
> + // The size of recovery data is invalid
> + //
> + return EFI_LOAD_ERROR;
> + }
> }
>
> + //
> + // Step 0: No recovery data is found.
> + //
> +
> //
> // Print out some information to avoid the user thought it is an
> error
> //
> - SctPrint (L"System will cold reset after 2 second. please run this
> test again...");
> + SctPrint (L"System will cold reset after 2 second and test will be
> resumed after reboot.");
> gtBS->Stall (2000000);
>
> +
> + ResetData->Step = 1;
> + ResetData->TplIndex = 0;
> + Status = RecoveryLib->WriteResetRecord (
> + RecoveryLib,
> + sizeof (RESET_DATA),
> + (UINT8*)ResetData
> + );
> + if (EFI_ERROR(Status)) {
> + StandardLib->RecordAssertion (
> + StandardLib,
> + EFI_TEST_ASSERTION_FAILED,
> + gTestGenericFailureGuid,
> + L"TestRecoveryLib - WriteResetRecord",
> + L"%a:%d:Status - %r",
> + __FILE__,
> + (UINTN)__LINE__,
> + Status
> + );
> + return Status;
> + }
> +
> +
> //
> // Checkpoint 1:
> // 3.5.2.1 ExitBootServices should not succeed with an invalid
> MapKey
> @@ -885,7 +974,7 @@ BBTestExitBootServicesConsistencyTest (
> //reset system
> gtRT->ResetSystem (EfiResetCold, EFI_SUCCESS, 0, NULL);
>
> - // get var to get the status
> +
> CheckResult:
>
> if (ReturnStatus == EFI_INVALID_PARAMETER) {
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [edk2-test][Patch V2 1/1] uefi-sct/SctPkg: Eliminate 2nd execution of ExitBootServices Test
2019-08-23 16:51 ` Supreeth Venkatesh
@ 2019-08-26 9:24 ` Eric Jin
0 siblings, 0 replies; 3+ messages in thread
From: Eric Jin @ 2019-08-26 9:24 UTC (permalink / raw)
To: Supreeth Venkatesh, devel@edk2.groups.io
Pushed 4d3395a9a54d575850fa12ca04326c797b7e1fe9
Re-order the statements as Supreeth's comment.
Best Regards
Eric
-----Original Message-----
From: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
Sent: Saturday, August 24, 2019 12:52 AM
To: Jin, Eric <eric.jin@intel.com>; devel@edk2.groups.io
Subject: Re: [edk2-test][Patch V2 1/1] uefi-sct/SctPkg: Eliminate 2nd execution of ExitBootServices Test
On Fri, 2019-08-23 at 00:46 -0500, Eric Jin wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2098
>
> In the ExitBootServices() test, after ExitBootServices() call, all
> boot services are forbidden. The original design is to save the return
> status value of ExitBootServices() in variable using variable service
> and reset, but this needs one additional execution of the test to
> retrieve the value from variable and this design was not
> straightforward from end user perspective.
> This patch enhances the test by leveraging RecoveryLib to restore
> execution after reset automatically, thus requiring only one
> execution.
>
> Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> Signed-off-by: Eric Jin <eric.jin@intel.com>
Please re-order the statements like below to make it more clear since ResetData is a just a pointer to Buffer Array which is statically allocated to 1024 and ResetData is not being really initialized to Reset Record after reading reset record, but to Buffer Array.
1. ResetData = (RESET_DATA *)Buffer;
2. RecoveryLib->ReadResetRecord(RecoveryLib, Size, Buffer); while commiting it.
With that,
Reviewed-by: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> ---
> uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTest.inf | 3 ++-
> uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTest.h | 9 ++++++++-
> uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTestConformance.c | 121
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++++++++++++++++++++++++++++++++++----------------
> 3 files changed, 115 insertions(+), 18 deletions(-)
>
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTest.inf b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTest.inf
> index 49ad79915934..3de43a20e8a4 100644
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTest.inf
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTest.inf
> @@ -1,7 +1,7 @@
> ## @file
> #
> # Copyright 2006 - 2012 Unified EFI, Inc.<BR> -# Copyright (c) 2010
> - 2012, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2010 - 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 @@
> -53,4 +53,5 @@
>
> [Protocols]
> gEfiTestProfileLibraryGuid
> + gEfiTestRecoveryLibraryGuid
> gBlackBoxEfiHIIPackageListProtocolGuid
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTest.h b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTest.h
> index b1c35fee7435..008584577ed1 100644
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTest.h
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTest.h
> @@ -1,7 +1,7 @@
> /** @file
>
> Copyright 2006 - 2017 Unified EFI, Inc.<BR>
> - Copyright (c) 2010 - 2017, Intel Corporation. All rights
> reserved.<BR>
> + Copyright (c) 2010 - 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 @@ -35,6 +35,13 @@ Abstract:
> #include EFI_PROTOCOL_DEFINITION (LoadFile)
>
> #include EFI_TEST_PROTOCOL_DEFINITION (TestProfileLibrary)
> +#include EFI_TEST_PROTOCOL_DEFINITION (TestRecoveryLibrary)
> +
> +typedef struct _RESET_DATA {
> + UINTN Step;
> + UINTN TplIndex;
> + UINT32 RepeatTimes;
> +} RESET_DATA;
>
> #if (EFI_SPECIFICATION_VERSION >= 0x0002000A)
>
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTestConformance.c b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTestConformance.c
> index 0a26d46847da..6ce1d2d72669 100644
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTestConformance.c
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/ImageServices/BlackBoxTest/
> ImageBBTestConformance.c
> @@ -1,7 +1,7 @@
> /** @file
>
> Copyright 2006 - 2016 Unified EFI, Inc.<BR>
> - Copyright (c) 2010 - 2016, Intel Corporation. All rights
> reserved.<BR>
> + Copyright (c) 2010 - 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 @@ -30,7 +30,8 @@ Abstract:
> #define TEST_VENDOR1_GUID \
> { 0xF6FAB04F, 0xACAF, 0x4af3, { 0xB9, 0xFA, 0xDC, 0xF9, 0x7F, 0xB4,
> 0x42, 0x6F } }
>
> -#define MAX_BUFFER_SIZE 10
> +#define STATUS_BUFFER_SIZE 8
> +#define RECOVER_BUFFER_SIZE 1024
>
> EFI_GUID gTestVendor1Guid = TEST_VENDOR1_GUID;
>
> @@ -778,19 +779,23 @@ BBTestExitBootServicesConsistencyTest (
> )
> {
> EFI_STANDARD_TEST_LIBRARY_PROTOCOL *StandardLib;
> + EFI_TEST_RECOVERY_LIBRARY_PROTOCOL *RecoveryLib;
> EFI_STATUS Status;
> EFI_TEST_ASSERTION AssertionType;
> UINTN MapKey;
> -
> + UINTN Size;
> UINTN Numbers;
> UINTN DataSize;
> - UINT8 Data[MAX_BUFFER_SIZE];
> + RESET_DATA *ResetData;
> + UINT8 Data[STATUS_BUFFER_SIZE];
> + UINT8 Buffer[RECOVER_BUFFER_SIZE];
> EFI_STATUS ReturnStatus;
>
> //
> // Init
> //
> StandardLib = NULL;
> + RecoveryLib = NULL;
>
> //
> // Get the Standard Library Interface @@ -803,6 +808,14 @@
> BBTestExitBootServicesConsistencyTest (
> return Status;
> }
>
> + Status = gtBS->HandleProtocol (
> + SupportHandle,
> + &gEfiTestRecoveryLibraryGuid,
> + (VOID **) &RecoveryLib); if (EFI_ERROR(Status)) {
> + return Status;
> + }
> +
> Status = ImageTestCheckForCleanEnvironment (&Numbers);
> if (EFI_ERROR(Status)) {
> StandardLib->RecordAssertion (
> @@ -819,25 +832,101 @@ BBTestExitBootServicesConsistencyTest (
> return Status;
> }
>
> - DataSize = MAX_BUFFER_SIZE;
> - Status = gtRT->GetVariable (
> - L"ExitBootServicesTestVariable", //
> VariableName
> - &gTestVendor1Guid, //
> VendorGuid
> - NULL, //
> Attributes
> - &DataSize, //
> DataSize
> - &ReturnStatus //
> Data
> - );
> + //
> + // Read reset record
> + //
> + Status = RecoveryLib->ReadResetRecord (
> + RecoveryLib,
> + &Size,
> + Buffer
> + );
> + ResetData = (RESET_DATA *)Buffer;
>
> - if (Status == EFI_SUCCESS) {
> - goto CheckResult;
> + //
> + // The workflow to check the recovery data // if (EFI_SUCCESS ==
> + Status) {
> + //
> + // To check the recovery data size
> + //
> + if (Size == sizeof(RESET_DATA)){
> + //
> + // To Check the recovery data
> + //
> + if (ResetData->Step == 1) {
> + //
> + // Step 1: find the valid recovery data, to retrive presaved
> status from variable
> + //
> + DataSize = STATUS_BUFFER_SIZE;
> + Status = gtRT->GetVariable (
> + L"ExitBootServicesTestVariable", //
> VariableName
> + &gTestVendor1Guid, //
> VendorGuid
> + NULL, //
> Attributes
> + &DataSize, //
> DataSize
> + &ReturnStatus // Data
> + );
> +
> + if (EFI_ERROR(Status)) {
> + StandardLib->RecordAssertion (
> + StandardLib,
> + EFI_TEST_ASSERTION_FAILED,
> + gTestGenericFailureGuid,
> + L"GetVariable - Can't get the test variable -
> ExitBootServicesTestVariable",
> + L"%a:%d:Status - %r",
> + __FILE__,
> + (UINTN)__LINE__,
> + Status
> + );
> + return Status;
> + }
> + goto CheckResult;
> + } else {
> + //
> + // It is invalid recovery data
> + //
> + return EFI_LOAD_ERROR;
> + }
> + } else {
> + //
> + // The size of recovery data is invalid
> + //
> + return EFI_LOAD_ERROR;
> + }
> }
>
> + //
> + // Step 0: No recovery data is found.
> + //
> +
> //
> // Print out some information to avoid the user thought it is an
> error
> //
> - SctPrint (L"System will cold reset after 2 second. please run this
> test again...");
> + SctPrint (L"System will cold reset after 2 second and test will be
> resumed after reboot.");
> gtBS->Stall (2000000);
>
> +
> + ResetData->Step = 1;
> + ResetData->TplIndex = 0;
> + Status = RecoveryLib->WriteResetRecord (
> + RecoveryLib,
> + sizeof (RESET_DATA),
> + (UINT8*)ResetData
> + );
> + if (EFI_ERROR(Status)) {
> + StandardLib->RecordAssertion (
> + StandardLib,
> + EFI_TEST_ASSERTION_FAILED,
> + gTestGenericFailureGuid,
> + L"TestRecoveryLib - WriteResetRecord",
> + L"%a:%d:Status - %r",
> + __FILE__,
> + (UINTN)__LINE__,
> + Status
> + );
> + return Status;
> + }
> +
> +
> //
> // Checkpoint 1:
> // 3.5.2.1 ExitBootServices should not succeed with an invalid
> MapKey @@ -885,7 +974,7 @@ BBTestExitBootServicesConsistencyTest (
> //reset system
> gtRT->ResetSystem (EfiResetCold, EFI_SUCCESS, 0, NULL);
>
> - // get var to get the status
> +
> CheckResult:
>
> if (ReturnStatus == EFI_INVALID_PARAMETER) {
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-08-26 9:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-08-23 5:46 [edk2-test][Patch V2 1/1] uefi-sct/SctPkg: Eliminate 2nd execution of ExitBootServices Test Eric Jin
2019-08-23 16:51 ` Supreeth Venkatesh
2019-08-26 9:24 ` Eric Jin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox