From: "Samer El-Haj-Mahmoud" <samer.el-haj-mahmoud@arm.com>
To: Grant Likely <Grant.Likely@arm.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: nd <nd@arm.com>, G Edhaya Chandran <Edhaya.Chandran@arm.com>,
Heinrich Schuchardt <xypron.glpk@gmx.de>,
Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Subject: Re: [PATCH] Check return status on calls to GetTime()
Date: Tue, 4 Aug 2020 11:26:52 +0000 [thread overview]
Message-ID: <DB7PR08MB3260D04C1441E62C250A5C8C904A0@DB7PR08MB3260.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <20200731171949.15748-1-grant.likely@arm.com>
Reviewed-by: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
> -----Original Message-----
> From: Grant Likely <Grant.Likely@arm.com>
> Sent: Friday, July 31, 2020 1:20 PM
> To: devel@edk2.groups.io
> Cc: nd <nd@arm.com>; Grant Likely <Grant.Likely@arm.com>; G Edhaya
> Chandran <Edhaya.Chandran@arm.com>; Heinrich Schuchardt
> <xypron.glpk@gmx.de>; Samer El-Haj-Mahmoud <Samer.El-Haj-
> Mahmoud@arm.com>
> Subject: [PATCH] Check return status on calls to GetTime()
>
> Not all platforms implement GetTime(), but the SCT just assumes calls to
> GetTime will be successful. If GetTime() doesn't return EFI_SUCCESS, then
> the EFI_TIME value will be uninitialized data.
>
> Fix by checking the GetTime() return code. If it doesn't return EFI_SUCCESS,
> then use the traditional 1/1/1970 epoch so that the test report at least looks
> sane, but it is obvious that we don't have a valid timestamp.
>
> Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=2870
>
> Cc: G Edhaya Chandran <Edhaya.Chandran@arm.com>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
> Signed-off-by: Grant Likely <grant.likely@arm.com>
> ---
> .../SimpleNetwork/SimpleNetworkENTSTestCase.c | 26 +++++++++++++----
> --
> .../MiscBootServicesBBTestFunction.c | 8 ++++--
> .../DriverBindingBBTestFunction.c | 5 +++-
> .../SCT/Drivers/StandardTest/StandardTest.c | 11 +++++---
> .../Framework/ENTS/EasDispatcher/Core/Eas.c | 9 +++++--
> .../ENTS/EasDispatcher/Exec/EasCmdDisp.c | 20 +++++++++-----
> 6 files changed, 57 insertions(+), 22 deletions(-)
>
> diff --git a/uefi-
> sct/SctPkg/TestCase/RIVL/Protocol/SimpleNetwork/SimpleNetworkENTSTes
> tCase.c b/uefi-
> sct/SctPkg/TestCase/RIVL/Protocol/SimpleNetwork/SimpleNetworkENTSTes
> tCase.c
> index 9c8d2a70..5579be7e 100644
> --- a/uefi-
> sct/SctPkg/TestCase/RIVL/Protocol/SimpleNetwork/SimpleNetworkENTSTes
> tCase.c
> +++ b/uefi-
> sct/SctPkg/TestCase/RIVL/Protocol/SimpleNetwork/SimpleNetwork
> +++ ENTSTestCase.c
> @@ -24,6 +24,8 @@ Abstract:
>
> #include "SimpleNetworkENTSTestCase.h"
>
> +static EFI_TIME Epoch = { .Year = 1970, .Month = 1, .Day = 1 };
> +
> //
> // SimpleNetwork.Start
> //
> @@ -928,7 +930,8 @@ Returns:
> Status = EFI_SUCCESS;
> tBS->Stall (5000);
>
> - tRT->GetTime (&BeginTime, NULL);
> + if (tRT->GetTime (&BeginTime, NULL) != EFI_SUCCESS)
> + BeginTime = Epoch;
> for (Index = 0; Index < 1;) {
> Status = SimpleNetwork->Transmit (
> SimpleNetwork, @@ -964,7 +967,8 @@ Returns:
> }
> }
>
> - tRT->GetTime (&BeginTime, NULL);
> + if (tRT->GetTime (&BeginTime, NULL) != EFI_SUCCESS)
> + BeginTime = Epoch;
>
> for (Index = 1; Index < TransmitPattern1Number;) {
> Status = SimpleNetwork->Transmit (
> @@ -1002,7 +1006,8 @@ Returns:
> }
>
> End:
> - tRT->GetTime (&EndTime, NULL);
> + if (tRT->GetTime (&EndTime, NULL) != EFI_SUCCESS)
> + EndTime = Epoch;
>
> *TransmitPattern1Status = Status;
>
> @@ -1125,7 +1130,8 @@ Returns:
> Status = EFI_SUCCESS;
> tBS->Stall (5000);
>
> - tRT->GetTime (&BeginTime, NULL);
> + if (tRT->GetTime (&BeginTime, NULL) != EFI_SUCCESS)
> + BeginTime = Epoch;
> for (Index = 0; Index < 1;) {
> Status = SimpleNetwork->Transmit (
> SimpleNetwork, @@ -1161,7 +1167,8 @@ Returns:
> }
> }
>
> - tRT->GetTime (&BeginTime, NULL);
> + if (tRT->GetTime (&BeginTime, NULL) != EFI_SUCCESS)
> + BeginTime = Epoch;
>
> for (Index = 1; Index < TransmitPattern2Number;) {
> Status = SimpleNetwork->Transmit (
> @@ -1199,7 +1206,8 @@ Returns:
> }
>
> End:
> - tRT->GetTime (&EndTime, NULL);
> + if (tRT->GetTime (&EndTime, NULL) != EFI_SUCCESS)
> + EndTime = Epoch;
>
> *TransmitPattern1Status = Status;
>
> @@ -1326,7 +1334,8 @@ Returns:
> }
> }
>
> - tRT->GetTime (&BeginTime, NULL);
> + if (tRT->GetTime (&BeginTime, NULL) != EFI_SUCCESS)
> + BeginTime = Epoch;
>
> for (Index = 1; Index < ReceivePattern1Number;) {
> *ReceivePattern1BufferSize = BufferSizeOrg; @@ -1346,7 +1355,8 @@
> Returns:
> }
> }
>
> - tRT->GetTime (&EndTime, NULL);
> + if (tRT->GetTime (&EndTime, NULL) != EFI_SUCCESS)
> + EndTime = Epoch;
>
> *ReceivePattern1Status = Status;
>
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTest
> /MiscBootServicesBBTestFunction.c b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTest
> /MiscBootServicesBBTestFunction.c
> index 1d231d8c..3a530282 100644
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTest
> /MiscBootServicesBBTestFunction.c
> +++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/Bl
> +++ ackBoxTest/MiscBootServicesBBTestFunction.c
> @@ -27,6 +27,8 @@ Abstract:
> #include "SctLib.h"
> #include "MiscBootServicesBBTestMain.h"
>
> +static EFI_TIME Epoch = { .Year = 1970, .Month = 1, .Day = 1 };
> +
> /**
> * Entrypoint for gtBS->SetWatchdogTimer() Interface Test.
> * @param This a pointer of EFI_BB_TEST_PROTOCOL.
> @@ -821,13 +823,15 @@ BBTestStallInterfaceTest (
> //
> // 4.2.2.1 Stall must succeed.
> //
> - gtRT->GetTime (&StartTime, NULL);
> + if (gtRT->GetTime (&StartTime, NULL) != EFI_SUCCESS)
> + StartTime = Epoch;
> OldTpl = gtBS->RaiseTPL (TplArray[Index]);
> Status = gtBS->Stall (
> 10000000
> );
> gtBS->RestoreTPL (OldTpl);
> - gtRT->GetTime (&EndTime, NULL);
> + if (gtRT->GetTime (&EndTime, NULL) != EFI_SUCCESS)
> + EndTime = Epoch;
> if (Status == EFI_SUCCESS) {
> AssertionType = EFI_TEST_ASSERTION_PASSED;
> } else {
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/IHV/Protocol/DriverBinding/BlackBoxTest/DriverB
> indingBBTestFunction.c b/uefi-
> sct/SctPkg/TestCase/UEFI/IHV/Protocol/DriverBinding/BlackBoxTest/DriverB
> indingBBTestFunction.c
> index bf675feb..4ab52dcd 100644
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/IHV/Protocol/DriverBinding/BlackBoxTest/DriverB
> indingBBTestFunction.c
> +++ b/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/DriverBinding/BlackBoxT
> +++ est/DriverBindingBBTestFunction.c
> @@ -36,6 +36,8 @@ static const UINTN MonthLengths[2][12] = {
> { 31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 } };
>
> +static EFI_TIME Epoch = { .Year = 1970, .Month = 1, .Day = 1 };
> +
> #define MINS_PER_HOUR 60
> #define HOURS_PER_DAY 24
> #define SECS_PER_MIN 60
> @@ -1052,7 +1054,8 @@ EndLogging (
> WriteLogFile (Private, DashLine, SYSTEMLOG);
> WriteLogFile (Private, DashLine, CASELOG);
>
> - gtRT->GetTime (&CurrentTime, NULL);
> + if (gtRT->GetTime (&CurrentTime, NULL) != EFI_SUCCESS)
> + CurrentTime = Epoch;
> DBSPrint (Buffer, EFI_MAX_PRINT_BUFFER, L"Test Finished: %t\n",
> &CurrentTime);
>
> WriteLogFile (Private, Buffer, SYSTEMLOG); diff --git a/uefi-
> sct/SctPkg/TestInfrastructure/SCT/Drivers/StandardTest/StandardTest.c
> b/uefi-
> sct/SctPkg/TestInfrastructure/SCT/Drivers/StandardTest/StandardTest.c
> index 84025457..836f072a 100644
> --- a/uefi-
> sct/SctPkg/TestInfrastructure/SCT/Drivers/StandardTest/StandardTest.c
> +++ b/uefi-sct/SctPkg/TestInfrastructure/SCT/Drivers/StandardTest/Standa
> +++ rdTest.c
> @@ -30,6 +30,8 @@ Abstract:
> #include "StandardTest.h"
> #include <Library/EntsLib.h>
>
> +static EFI_TIME Epoch = { .Year = 1970, .Month = 1, .Day = 1 };
> +
> //
> // Prototypes
> //
> @@ -1081,7 +1083,8 @@ Returns:
> StslWriteLogFile (Private, Buffer);
>
> CurrentTime = &Private->StartTime;
> - tRT->GetTime (CurrentTime, NULL);
> + if (tRT->GetTime (CurrentTime, NULL) != EFI_SUCCESS)
> + *CurrentTime = Epoch;
>
> } else {
> StslWriteLogFile (Private, DashLine); @@ -1118,7 +1121,8 @@ Returns:
>
> StslWriteLogFileName (Private);
> CurrentTime = &Private->StartTime;
> - tRT->GetTime (CurrentTime, NULL);
> + if (tRT->GetTime (CurrentTime, NULL) != EFI_SUCCESS)
> + *CurrentTime = Epoch;
> SctSPrint (Buffer, EFI_MAX_PRINT_BUFFER, L"Test Started: %t\n",
> CurrentTime);
> StslWriteLogFile (Private, Buffer);
>
> @@ -1238,7 +1242,8 @@ Returns:
>
> StslWriteLogFileName (Private);
>
> - tRT->GetTime (&CurrentTime, NULL);
> + if (tRT->GetTime (&CurrentTime, NULL) != EFI_SUCCESS)
> + CurrentTime = Epoch;
>
> SecondsElapsed = SecondsElapsedFromBaseYear (
> Private->StartTime.Year, diff --git a/uefi-
> sct/SctPkg/TestInfrastructure/SCT/Framework/ENTS/EasDispatcher/Core/Ea
> s.c b/uefi-
> sct/SctPkg/TestInfrastructure/SCT/Framework/ENTS/EasDispatcher/Core/Ea
> s.c
> index 28f5ed4a..60b1c4dc 100644
> --- a/uefi-
> sct/SctPkg/TestInfrastructure/SCT/Framework/ENTS/EasDispatcher/Core/Ea
> s.c
> +++ b/uefi-
> sct/SctPkg/TestInfrastructure/SCT/Framework/ENTS/EasDispatche
> +++ r/Core/Eas.c
> @@ -23,9 +23,12 @@ Abstract:
> --*/
>
>
> +#include "Sct.h"
> #include "Sct.h"
> #include EFI_TEST_PROTOCOL_DEFINITION (EntsMonitorProtocol)
>
> +static EFI_TIME Epoch = { .Year = 1970, .Month = 1, .Day = 1 };
> +
> STATIC
> EFI_STATUS
> AgentTestMain (
> @@ -310,7 +313,8 @@ DelaySctAgentCmdPost (
> }
> SctAgentCmdDelayedPost->CmdReturn = CmdReturn;
> SctAgentCmdDelayedPost->Cmd.ComdResult = CmdResult;
> - tRT->GetTime (&SctAgentCmdDelayedPost->StartTime, NULL);
> + if (tRT->GetTime (&SctAgentCmdDelayedPost->StartTime, NULL) !=
> EFI_SUCCESS)
> + SctAgentCmdDelayedPost->StartTime = Epoch;
>
> return Status;
> }
> @@ -327,7 +331,8 @@ PostSctAgentDelayedCmd (
> return EFI_SUCCESS;
> }
>
> - tRT->GetTime (&SctAgentCmdDelayedPost->EndTime, NULL);
> + if (tRT->GetTime (&SctAgentCmdDelayedPost->EndTime, NULL) !=
> EFI_SUCCESS)
> + SctAgentCmdDelayedPost->EndTime = Epoch;
>
> Status = RecordMessage (
> &SctAgentCmdDelayedPost->Cmd.ComdRuntimeInfo,
> diff --git a/uefi-
> sct/SctPkg/TestInfrastructure/SCT/Framework/ENTS/EasDispatcher/Exec/Ea
> sCmdDisp.c b/uefi-
> sct/SctPkg/TestInfrastructure/SCT/Framework/ENTS/EasDispatcher/Exec/Ea
> sCmdDisp.c
> index 1ff6d569..cb6f08cf 100644
> --- a/uefi-
> sct/SctPkg/TestInfrastructure/SCT/Framework/ENTS/EasDispatcher/Exec/Ea
> sCmdDisp.c
> +++ b/uefi-
> sct/SctPkg/TestInfrastructure/SCT/Framework/ENTS/EasDispatche
> +++ r/Exec/EasCmdDisp.c
> @@ -50,6 +50,8 @@ Abstract:
>
> EFI_CPU_ARCH_PROTOCOL *Cpu = NULL;
>
> +static EFI_TIME Epoch = { .Year = 1970, .Month = 1, .Day = 1 };
> +
> //
> // Local Function Definition
> //
> @@ -132,9 +134,11 @@ Returns:
> //
> // Perform EFTP operation.
> //
> - tRT->GetTime (&StartTime, NULL);
> + if (tRT->GetTime (&StartTime, NULL) != EFI_SUCCESS)
> + StartTime = Epoch;
> Status = EftpDispatchFileTransferComd (FileCmdType);
> - tRT->GetTime (&EndTime, NULL);
> + if (tRT->GetTime (&EndTime, NULL) != EFI_SUCCESS)
> + EndTime = Epoch;
>
> if (Status == EFI_OUT_OF_RESOURCES) {
> return EFI_OUT_OF_RESOURCES;
> @@ -365,9 +369,11 @@ Returns:
> //
> // Execute Shell Command
> //
> - tRT->GetTime (&StartTime, NULL);
> + if (tRT->GetTime (&StartTime, NULL) != EFI_SUCCESS)
> + StartTime = Epoch;
> Status = SctShellExecute (&mImageHandle, (gEasFT->Cmd)->ComdArg,
> FALSE, NULL, NULL);;
> - tRT->GetTime (&EndTime, NULL);
> + if (tRT->GetTime (&EndTime, NULL) != EFI_SUCCESS)
> + EndTime = Epoch;
> EFI_ENTS_DEBUG ((EFI_ENTS_D_TRACE, L"dispatch:(%s)", (gEasFT->Cmd)-
> >ComdArg));
> SctPrint (L"dispatch:(%s) - %r\n", (gEasFT->Cmd)->ComdArg, Status);
> if (Status == EFI_OUT_OF_RESOURCES) { @@ -1483,9 +1489,11 @@
> Returns:
> //
> // Resume SCT execution by executing "sct -c" in sct passive mode.
> //
> - tRT->GetTime (&StartTime, NULL);
> + if (tRT->GetTime (&StartTime, NULL) != EFI_SUCCESS)
> + StartTime = Epoch;
> Status = SctShellExecute (&mImageHandle, (gEasFT->Cmd)->ComdArg,
> FALSE, NULL, NULL);;
> - tRT->GetTime (&EndTime, NULL);
> + if (tRT->GetTime (&EndTime, NULL) != EFI_SUCCESS)
> + EndTime = Epoch;
> EFI_ENTS_DEBUG ((EFI_ENTS_D_TRACE, L"dispatch:(%s)", (gEasFT->Cmd)-
> >ComdArg));
> SctPrint (L"dispatch:(%s) - %r\n", (gEasFT->Cmd)->ComdArg, Status);
> if (Status == EFI_OUT_OF_RESOURCES) {
> --
> 2.20.1
next prev parent reply other threads:[~2020-08-04 11:27 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-31 17:19 [PATCH] Check return status on calls to GetTime() Grant Likely
2020-08-01 13:54 ` Heinrich Schuchardt
2020-08-04 15:46 ` Grant Likely
2020-08-04 11:26 ` Samer El-Haj-Mahmoud [this message]
2020-08-06 13:31 ` Samer El-Haj-Mahmoud
2020-08-06 14:08 ` Heinrich Schuchardt
2020-08-11 19:50 ` [edk2-devel] " G Edhaya Chandran
2020-08-11 20:00 ` G Edhaya Chandran
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DB7PR08MB3260D04C1441E62C250A5C8C904A0@DB7PR08MB3260.eurprd08.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox