* [PATCH] Check return status on calls to GetTime()
@ 2020-07-31 17:19 Grant Likely
2020-08-01 13:54 ` Heinrich Schuchardt
2020-08-04 11:26 ` Samer El-Haj-Mahmoud
0 siblings, 2 replies; 8+ messages in thread
From: Grant Likely @ 2020-07-31 17:19 UTC (permalink / raw)
To: devel
Cc: nd, Grant Likely, G Edhaya Chandran, Heinrich Schuchardt,
Samer El-Haj-Mahmoud
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/SimpleNetworkENTSTestCase.c b/uefi-sct/SctPkg/TestCase/RIVL/Protocol/SimpleNetwork/SimpleNetworkENTSTestCase.c
index 9c8d2a70..5579be7e 100644
--- a/uefi-sct/SctPkg/TestCase/RIVL/Protocol/SimpleNetwork/SimpleNetworkENTSTestCase.c
+++ b/uefi-sct/SctPkg/TestCase/RIVL/Protocol/SimpleNetwork/SimpleNetworkENTSTestCase.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/BlackBoxTest/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/DriverBindingBBTestFunction.c b/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/DriverBinding/BlackBoxTest/DriverBindingBBTestFunction.c
index bf675feb..4ab52dcd 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/DriverBinding/BlackBoxTest/DriverBindingBBTestFunction.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/DriverBinding/BlackBoxTest/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/StandardTest.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/Eas.c b/uefi-sct/SctPkg/TestInfrastructure/SCT/Framework/ENTS/EasDispatcher/Core/Eas.c
index 28f5ed4a..60b1c4dc 100644
--- a/uefi-sct/SctPkg/TestInfrastructure/SCT/Framework/ENTS/EasDispatcher/Core/Eas.c
+++ b/uefi-sct/SctPkg/TestInfrastructure/SCT/Framework/ENTS/EasDispatcher/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/EasCmdDisp.c b/uefi-sct/SctPkg/TestInfrastructure/SCT/Framework/ENTS/EasDispatcher/Exec/EasCmdDisp.c
index 1ff6d569..cb6f08cf 100644
--- a/uefi-sct/SctPkg/TestInfrastructure/SCT/Framework/ENTS/EasDispatcher/Exec/EasCmdDisp.c
+++ b/uefi-sct/SctPkg/TestInfrastructure/SCT/Framework/ENTS/EasDispatcher/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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] Check return status on calls to GetTime()
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
1 sibling, 1 reply; 8+ messages in thread
From: Heinrich Schuchardt @ 2020-08-01 13:54 UTC (permalink / raw)
To: Grant Likely, devel; +Cc: nd, G Edhaya Chandran, Samer El-Haj-Mahmoud, Eric Jin
On 7/31/20 7:19 PM, Grant Likely wrote:
> 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/SimpleNetworkENTSTestCase.c b/uefi-sct/SctPkg/TestCase/RIVL/Protocol/SimpleNetwork/SimpleNetworkENTSTestCase.c
> index 9c8d2a70..5579be7e 100644
> --- a/uefi-sct/SctPkg/TestCase/RIVL/Protocol/SimpleNetwork/SimpleNetworkENTSTestCase.c
> +++ b/uefi-sct/SctPkg/TestCase/RIVL/Protocol/SimpleNetwork/SimpleNetworkENTSTestCase.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/BlackBoxTest/MiscBootServicesBBTestFunction.c
> @@ -27,6 +27,8 @@ Abstract:
> #include "SctLib.h"
I tried to apply your patch with 'git am --keep-cr'. This chunk has a
problem.
While all other lines in the file have CR LF line endings this very line
has only LF in EDK2 master. But your patch has CR LF for the line.
Did you use Windows and not Linux to create the patch?
I could apply your patch after removing the extraneous CR.
With your patch applied if GetTime() returns EFI_UNSUPPORTED, the test
log is not garbled anymore:
SetVariable_Conf:
Test Finished: 01/01/70 12:00a
Elapsed Time: 00 Days 00:00:00
If GetTime() is available:
SetVariable_Conf:
Test Finished: 08/01/20 01:52p
Elapsed Time: 00 Days 00:00:04
Except for the line-endings:
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> #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/DriverBindingBBTestFunction.c b/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/DriverBinding/BlackBoxTest/DriverBindingBBTestFunction.c
> index bf675feb..4ab52dcd 100644
> --- a/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/DriverBinding/BlackBoxTest/DriverBindingBBTestFunction.c
> +++ b/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/DriverBinding/BlackBoxTest/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/StandardTest.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/Eas.c b/uefi-sct/SctPkg/TestInfrastructure/SCT/Framework/ENTS/EasDispatcher/Core/Eas.c
> index 28f5ed4a..60b1c4dc 100644
> --- a/uefi-sct/SctPkg/TestInfrastructure/SCT/Framework/ENTS/EasDispatcher/Core/Eas.c
> +++ b/uefi-sct/SctPkg/TestInfrastructure/SCT/Framework/ENTS/EasDispatcher/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/EasCmdDisp.c b/uefi-sct/SctPkg/TestInfrastructure/SCT/Framework/ENTS/EasDispatcher/Exec/EasCmdDisp.c
> index 1ff6d569..cb6f08cf 100644
> --- a/uefi-sct/SctPkg/TestInfrastructure/SCT/Framework/ENTS/EasDispatcher/Exec/EasCmdDisp.c
> +++ b/uefi-sct/SctPkg/TestInfrastructure/SCT/Framework/ENTS/EasDispatcher/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) {
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Check return status on calls to GetTime()
2020-08-01 13:54 ` Heinrich Schuchardt
@ 2020-08-04 15:46 ` Grant Likely
0 siblings, 0 replies; 8+ messages in thread
From: Grant Likely @ 2020-08-04 15:46 UTC (permalink / raw)
To: Heinrich Schuchardt, devel
Cc: nd, G Edhaya Chandran, Samer El-Haj-Mahmoud, Eric Jin
On 01/08/2020 14:54, Heinrich Schuchardt wrote:
> On 7/31/20 7:19 PM, Grant Likely wrote:
[...]>> 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/BlackBoxTest/MiscBootServicesBBTestFunction.c
>> @@ -27,6 +27,8 @@ Abstract:
>> #include "SctLib.h"
>
> I tried to apply your patch with 'git am --keep-cr'. This chunk has a
> problem.
>
> While all other lines in the file have CR LF line endings this very line
> has only LF in EDK2 master. But your patch has CR LF for the line.
>
> Did you use Windows and not Linux to create the patch?
Hahaha! No, I did not use Windows. From what I can tell, that file
already had inconsisten line endings. Most of the file is crlf, but the
"#include "SctLib.h" line is lf only. This confuses vim which interprets
it as a unix file and shows the ^M on every line, which in turn allowed
me to add additional lines with unix line endings.
> I could apply your patch after removing the extraneous CR.
>
> With your patch applied if GetTime() returns EFI_UNSUPPORTED, the test
> log is not garbled anymore:
>
> SetVariable_Conf:
> Test Finished: 01/01/70 12:00a
> Elapsed Time: 00 Days 00:00:00
>
> If GetTime() is available:
>
> SetVariable_Conf:
> Test Finished: 08/01/20 01:52p
> Elapsed Time: 00 Days 00:00:04
>
> Except for the line-endings:
>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Ace! Thanks.
g.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Check return status on calls to GetTime()
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 11:26 ` Samer El-Haj-Mahmoud
2020-08-06 13:31 ` Samer El-Haj-Mahmoud
1 sibling, 1 reply; 8+ messages in thread
From: Samer El-Haj-Mahmoud @ 2020-08-04 11:26 UTC (permalink / raw)
To: Grant Likely, devel@edk2.groups.io
Cc: nd, G Edhaya Chandran, Heinrich Schuchardt, Samer El-Haj-Mahmoud
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Check return status on calls to GetTime()
2020-08-04 11:26 ` Samer El-Haj-Mahmoud
@ 2020-08-06 13:31 ` Samer El-Haj-Mahmoud
2020-08-06 14:08 ` Heinrich Schuchardt
0 siblings, 1 reply; 8+ messages in thread
From: Samer El-Haj-Mahmoud @ 2020-08-06 13:31 UTC (permalink / raw)
To: Grant Likely, devel@edk2.groups.io
Cc: nd, G Edhaya Chandran, Heinrich Schuchardt, Samer El-Haj-Mahmoud
One issue here is that UEFI 2.8 spec language around allowing runtime services (like GetTime() ) to return EFI_UNSUPPORTED is a bit vague:
""
EFI_UNSUPPORTED
This call is not supported by this platform at the time the call is made. The platform should describe this runtime service as unsupported at runtime via an EFI_RT_PROPERTIES_TABLE configuration table.
""
The "at the time the call is made" language may be interpreted as the service is allowed to return EFI_UNSUPPORTED anytime (before or after ExitBootServices() ). But the next sentence (about declaring the unsupported at runtime) make it sounds like this is restricted to only being at runtime. While the use case makes sense, we may need some clarification in the UEFI spec language to make this clear. Otherwise, callers (like SCT) may assume the function is always supported.
> -----Original Message-----
> From: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
> Sent: Tuesday, August 4, 2020 7:27 AM
> To: Grant Likely <Grant.Likely@arm.com>; 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()
>
> 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/BlackBoxTes
> > t /MiscBootServicesBBTestFunction.c b/uefi-
> > sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTes
> > t
> > /MiscBootServicesBBTestFunction.c
> > index 1d231d8c..3a530282 100644
> > --- a/uefi-
> > sct/SctPkg/TestCase/UEFI/EFI/BootServices/MiscBootServices/BlackBoxTes
> > t
> > /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/Drive
> > rB
> > indingBBTestFunction.c b/uefi-
> > sct/SctPkg/TestCase/UEFI/IHV/Protocol/DriverBinding/BlackBoxTest/Drive
> > rB
> > indingBBTestFunction.c
> > index bf675feb..4ab52dcd 100644
> > --- a/uefi-
> > sct/SctPkg/TestCase/UEFI/IHV/Protocol/DriverBinding/BlackBoxTest/Drive
> > rB
> > indingBBTestFunction.c
> > +++ b/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/DriverBinding/BlackBo
> > +++ xT
> > +++ 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/Stan
> > +++ da
> > +++ 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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Check return status on calls to GetTime()
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
0 siblings, 1 reply; 8+ messages in thread
From: Heinrich Schuchardt @ 2020-08-06 14:08 UTC (permalink / raw)
To: Samer El-Haj-Mahmoud, Grant Likely, devel@edk2.groups.io
Cc: nd, G Edhaya Chandran
On 06.08.20 15:31, Samer El-Haj-Mahmoud wrote:
> One issue here is that UEFI 2.8 spec language around allowing runtime services (like GetTime() ) to return EFI_UNSUPPORTED is a bit vague:
>
> ""
> EFI_UNSUPPORTED
> This call is not supported by this platform at the time the call is made. The platform should describe this runtime service as unsupported at runtime via an EFI_RT_PROPERTIES_TABLE configuration table.
> ""
>
> The "at the time the call is made" language may be interpreted as the service is allowed to return EFI_UNSUPPORTED anytime (before or after ExitBootServices() ). But the next sentence (about declaring the unsupported at runtime) make it sounds like this is restricted to only being at runtime. While the use case makes sense, we may need some clarification in the UEFI spec language to make this clear. Otherwise, callers (like SCT) may assume the function is always supported.
Some systems do not have a real time clock. So these system have to
return EFI_UNSUPPORTED before as well as after ExitBootServices().
Due to possible conflicts with the access by the operating system many
systems with RTC will opt not to implement GetTime() at runtime but may
implement GetTime() before ExitBootServices(). This is what we do in U-Boot.
But of cause it is allowable to provide the service at all times if
feasible.
An implementation supplying GetTime() only after ExitBootServices()
would not make much sense to me but is not explicitly forbidden by the spec.
A clearer wording for the spec might be:
"If the service is available at runtime or not, has to be marked in the
EFI_RT_PROPROPERTIES_TABLE."
Best regards
Heinrich
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-08-11 20:00 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox