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.venkat...@arm.com>
> Signed-off-by: Eric Jin <eric....@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.venkat...@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) {
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#46311): https://edk2.groups.io/g/devel/message/46311
Mute This Topic: https://groups.io/mt/32998144/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
