* [PATCH 1/1] EmbeddedPkg/VirtualRealTimeClockLib : Fix SetTime issues
@ 2021-07-23 9:04 Sunny Wang
2021-07-23 16:43 ` Pete Batard
0 siblings, 1 reply; 3+ messages in thread
From: Sunny Wang @ 2021-07-23 9:04 UTC (permalink / raw)
To: devel
Cc: Sunny Wang, Samer El-Haj-Mahmoud, Sami Mujawar, Jeremy Linton,
Ard Biesheuvel, Pete Batard, Leif Lindholm, Sunny Wang
This patch fixes two issues below:
1. SCT SetTime_Func failures.
- https://github.com/pftf/RPi4/issues/164
2. Using shell time and date commands to set time can't work.
The problem is that gRT->SetTime always returns EFI_INVALID_PARAMETER
error status.
The root cause is that LibSetTime() sets RtcEpochSeconds variable with
inconsistent attributes. One is without EFI_VARIABLE_NON_VOLATILE,
the other one is with EFI_VARIABLE_NON_VOLATILE. That caused that the
variable driver returns EFI_INVALID_PARAMETER. Per UEFI spec, if a
preexisting variable is rewritten with different attributes,
SetVariable() shall not modify the variable and shall return
EFI_INVALID_PARAMETER.
Therefore, the solution is to add EFI_VARIABLE_NON_VOLATILE attribute
to the first EfiSetVariable() call to make two calls consistent.
By the way, this patch also fix a minor issue with a debug message.
Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Jeremy Linton <jeremy.linton@arm.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Pete Batard <pete@akeo.ie>
Cc: Leif Lindholm <leif@nuviainc.com>
Signed-off-by: Sunny Wang <sunny.wang@arm.com>
---
.../VirtualRealTimeClockLib/VirtualRealTimeClockLib.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
index de6fbb40e6..c10c91bc75 100644
--- a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
+++ b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
@@ -4,7 +4,7 @@
*
* Coypright (c) 2019, Pete Batard <pete@akeo.ie>
* Copyright (c) 2018, Andrei Warkentin <andrey.warkentin@gmail.com>
- * Copyright (c) 2011-2014, ARM Ltd. All rights reserved.
+ * Copyright (c) 2011-2021, ARM Ltd. All rights reserved.
* Copyright (c) 2008-2010, Apple Inc. All rights reserved.
* Copyright (c) Microsoft Corporation. All rights reserved.
*
@@ -96,7 +96,7 @@ LibGetTime (
EfiSetVariable (
(CHAR16 *)mEpochVariableName,
&gEfiCallerIdGuid,
- EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
+ EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
sizeof (EpochSeconds),
&EpochSeconds
);
@@ -324,7 +324,7 @@ LibSetTime (
DEBUG ((
DEBUG_ERROR,
"LibSetTime: Failed to save %s variable to non-volatile storage, Status = %r\n",
- mDaylightVariableName,
+ mEpochVariableName,
Status
));
return Status;
--
2.31.0.windows.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] EmbeddedPkg/VirtualRealTimeClockLib : Fix SetTime issues
2021-07-23 9:04 [PATCH 1/1] EmbeddedPkg/VirtualRealTimeClockLib : Fix SetTime issues Sunny Wang
@ 2021-07-23 16:43 ` Pete Batard
2021-07-29 10:15 ` Ard Biesheuvel
0 siblings, 1 reply; 3+ messages in thread
From: Pete Batard @ 2021-07-23 16:43 UTC (permalink / raw)
To: Sunny Wang, devel
Cc: Samer El-Haj-Mahmoud, Sami Mujawar, Jeremy Linton, Ard Biesheuvel,
Leif Lindholm
Hi Sunny,
Good catch for both these issues. Thanks for fixing them.
With this:
On 2021.07.23 10:04, Sunny Wang wrote:
> This patch fixes two issues below:
> 1. SCT SetTime_Func failures.
> - https://github.com/pftf/RPi4/issues/164
> 2. Using shell time and date commands to set time can't work.
>
> The problem is that gRT->SetTime always returns EFI_INVALID_PARAMETER
> error status.
>
> The root cause is that LibSetTime() sets RtcEpochSeconds variable with
> inconsistent attributes. One is without EFI_VARIABLE_NON_VOLATILE,
> the other one is with EFI_VARIABLE_NON_VOLATILE. That caused that the
> variable driver returns EFI_INVALID_PARAMETER. Per UEFI spec, if a
> preexisting variable is rewritten with different attributes,
> SetVariable() shall not modify the variable and shall return
> EFI_INVALID_PARAMETER.
>
> Therefore, the solution is to add EFI_VARIABLE_NON_VOLATILE attribute
> to the first EfiSetVariable() call to make two calls consistent.
>
> By the way, this patch also fix a minor issue with a debug message.
>
> Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Jeremy Linton <jeremy.linton@arm.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Pete Batard <pete@akeo.ie>
> Cc: Leif Lindholm <leif@nuviainc.com>
>
> Signed-off-by: Sunny Wang <sunny.wang@arm.com>
> ---
> .../VirtualRealTimeClockLib/VirtualRealTimeClockLib.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
> index de6fbb40e6..c10c91bc75 100644
> --- a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
> +++ b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
> @@ -4,7 +4,7 @@
> *
> * Coypright (c) 2019, Pete Batard <pete@akeo.ie>
> * Copyright (c) 2018, Andrei Warkentin <andrey.warkentin@gmail.com>
> - * Copyright (c) 2011-2014, ARM Ltd. All rights reserved.
> + * Copyright (c) 2011-2021, ARM Ltd. All rights reserved.
> * Copyright (c) 2008-2010, Apple Inc. All rights reserved.
> * Copyright (c) Microsoft Corporation. All rights reserved.
> *
> @@ -96,7 +96,7 @@ LibGetTime (
> EfiSetVariable (
> (CHAR16 *)mEpochVariableName,
> &gEfiCallerIdGuid,
> - EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
> + EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
> sizeof (EpochSeconds),
> &EpochSeconds
> );
> @@ -324,7 +324,7 @@ LibSetTime (
> DEBUG ((
> DEBUG_ERROR,
> "LibSetTime: Failed to save %s variable to non-volatile storage, Status = %r\n",
> - mDaylightVariableName,
> + mEpochVariableName,
> Status
> ));
> return Status;
>
Reviewed-by: Pete Batard <pete@akeo.ie>
Tested-by: Pete Batard <pete@akeo.ie>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] EmbeddedPkg/VirtualRealTimeClockLib : Fix SetTime issues
2021-07-23 16:43 ` Pete Batard
@ 2021-07-29 10:15 ` Ard Biesheuvel
0 siblings, 0 replies; 3+ messages in thread
From: Ard Biesheuvel @ 2021-07-29 10:15 UTC (permalink / raw)
To: Pete Batard
Cc: Sunny Wang, edk2-devel-groups-io, Samer El-Haj-Mahmoud,
Sami Mujawar, Jeremy Linton, Ard Biesheuvel, Leif Lindholm
On Fri, 23 Jul 2021 at 18:43, Pete Batard <pete@akeo.ie> wrote:
>
> Hi Sunny,
>
> Good catch for both these issues. Thanks for fixing them.
>
> With this:
>
> On 2021.07.23 10:04, Sunny Wang wrote:
> > This patch fixes two issues below:
> > 1. SCT SetTime_Func failures.
> > - https://github.com/pftf/RPi4/issues/164
> > 2. Using shell time and date commands to set time can't work.
> >
> > The problem is that gRT->SetTime always returns EFI_INVALID_PARAMETER
> > error status.
> >
> > The root cause is that LibSetTime() sets RtcEpochSeconds variable with
> > inconsistent attributes. One is without EFI_VARIABLE_NON_VOLATILE,
> > the other one is with EFI_VARIABLE_NON_VOLATILE. That caused that the
> > variable driver returns EFI_INVALID_PARAMETER. Per UEFI spec, if a
> > preexisting variable is rewritten with different attributes,
> > SetVariable() shall not modify the variable and shall return
> > EFI_INVALID_PARAMETER.
> >
> > Therefore, the solution is to add EFI_VARIABLE_NON_VOLATILE attribute
> > to the first EfiSetVariable() call to make two calls consistent.
> >
> > By the way, this patch also fix a minor issue with a debug message.
> >
> > Cc: Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>
> > Cc: Sami Mujawar <sami.mujawar@arm.com>
> > Cc: Jeremy Linton <jeremy.linton@arm.com>
> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > Cc: Pete Batard <pete@akeo.ie>
> > Cc: Leif Lindholm <leif@nuviainc.com>
> >
> > Signed-off-by: Sunny Wang <sunny.wang@arm.com>
> > ---
> > .../VirtualRealTimeClockLib/VirtualRealTimeClockLib.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
> > index de6fbb40e6..c10c91bc75 100644
> > --- a/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
> > +++ b/EmbeddedPkg/Library/VirtualRealTimeClockLib/VirtualRealTimeClockLib.c
> > @@ -4,7 +4,7 @@
> > *
> > * Coypright (c) 2019, Pete Batard <pete@akeo.ie>
> > * Copyright (c) 2018, Andrei Warkentin <andrey.warkentin@gmail.com>
> > - * Copyright (c) 2011-2014, ARM Ltd. All rights reserved.
> > + * Copyright (c) 2011-2021, ARM Ltd. All rights reserved.
> > * Copyright (c) 2008-2010, Apple Inc. All rights reserved.
> > * Copyright (c) Microsoft Corporation. All rights reserved.
> > *
> > @@ -96,7 +96,7 @@ LibGetTime (
> > EfiSetVariable (
> > (CHAR16 *)mEpochVariableName,
> > &gEfiCallerIdGuid,
> > - EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
> > + EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS,
> > sizeof (EpochSeconds),
> > &EpochSeconds
> > );
> > @@ -324,7 +324,7 @@ LibSetTime (
> > DEBUG ((
> > DEBUG_ERROR,
> > "LibSetTime: Failed to save %s variable to non-volatile storage, Status = %r\n",
> > - mDaylightVariableName,
> > + mEpochVariableName,
> > Status
> > ));
> > return Status;
> >
>
> Reviewed-by: Pete Batard <pete@akeo.ie>
> Tested-by: Pete Batard <pete@akeo.ie>
Merged, thanks,
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-07-29 10:15 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-07-23 9:04 [PATCH 1/1] EmbeddedPkg/VirtualRealTimeClockLib : Fix SetTime issues Sunny Wang
2021-07-23 16:43 ` Pete Batard
2021-07-29 10:15 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox