1. Patchew
  2. EDK2
  3. View series

[edk2-devel] [edk2-platforms PATCH] Silicon/Marvell/RealTimeClockLib: make EpochSeconds, WakeupSeconds UINTN

Laszlo Ersek posted 1 patch 3 years, 11 months ago
Failed in applying to current master (apply log)
Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
[edk2-devel] [edk2-platforms PATCH] Silicon/Marvell/RealTimeClockLib: make EpochSeconds, WakeupSeconds UINTN
Posted by Laszlo Ersek 3 years, 11 months ago 
We're going to change EfiTimeToEpoch() in edk2's TimeBaseLib to propagate
its internal UINTN calculation to the caller without an internal UINT32
truncation.

Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib drives 32-bit-only
hardware, so catch any number of seconds since the epoch, from
EfiTimeToEpoch(), that doesn't fit in 32 bits.

Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Marcin Wojtas <mw@semihalf.com>
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    Build-tested only, with
    
      build \
        -a AARCH64 \
        -b NOOPT \
        -p Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc \
        -t GCC5 \
        -m EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf

 Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
index 1974e0144cd8..a811fd368eca 100644
--- a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
+++ b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
@@ -100,33 +100,36 @@ EFIAPI
 LibSetTime (
   IN EFI_TIME                *Time
   )
 {
   EFI_STATUS  Status = EFI_SUCCESS;
-  UINT32      EpochSeconds;
+  UINTN       EpochSeconds;
 
   // Check the input parameters are within the range specified by UEFI
   if (!IsTimeValid (Time)) {
     return EFI_INVALID_PARAMETER;
   }
 
   // Convert time to raw seconds
   EpochSeconds = EfiTimeToEpoch (Time);
+  if (EpochSeconds > MAX_UINT32) {
+    return EFI_INVALID_PARAMETER;
+  }
 
   // Issue delayed write to time register
-  RtcDelayedWrite (RTC_TIME_REG, EpochSeconds);
+  RtcDelayedWrite (RTC_TIME_REG, (UINT32)EpochSeconds);
 
   return Status;
 }
 
 /**
   Returns the current wakeup alarm clock setting.
 
   @param  Enabled               Indicates if the alarm is currently enabled or disabled.
   @param  Pending               Indicates if the alarm signal is pending and requires acknowledgement.
   @param  Time                  The current alarm setting.
 
   @retval EFI_SUCCESS           The alarm settings were returned.
   @retval EFI_INVALID_PARAMETER Any parameter is NULL.
   @retval EFI_DEVICE_ERROR      The wakeup time could not be retrieved due to a hardware error.
 
 **/
@@ -172,32 +175,35 @@ EFIAPI
 LibSetWakeupTime (
   IN BOOLEAN      Enabled,
   OUT EFI_TIME    *Time
   )
 {
-  UINT32      WakeupSeconds;
+  UINTN       WakeupSeconds;
 
   // Convert time to raw seconds
   WakeupSeconds = EfiTimeToEpoch (Time);
+  if (WakeupSeconds > MAX_UINT32) {
+    return EFI_INVALID_PARAMETER;
+  }
 
   // Issue delayed write to alarm register
-  RtcDelayedWrite (RTC_ALARM_2_REG, WakeupSeconds);
+  RtcDelayedWrite (RTC_ALARM_2_REG, (UINT32)WakeupSeconds);
 
   if (Enabled) {
     MmioWrite32 (mArmadaRtcBase + RTC_IRQ_2_CONFIG_REG, RTC_IRQ_ALARM_EN);
   } else {
     MmioWrite32 (mArmadaRtcBase + RTC_IRQ_2_CONFIG_REG, 0);
   }
 
   return EFI_SUCCESS;
 }
 
 /**
   This is the declaration of an EFI image entry point. This can be the entry point to an application
   written to this specification, an EFI boot service driver, or an EFI runtime driver.
 
   @param  ImageHandle           Handle that identifies the loaded image.
   @param  SystemTable           System Table for this image.
 
   @retval EFI_SUCCESS           The operation completed successfully.
 
 **/
-- 
2.19.1.3.g30247aa5d201



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69312): https://edk2.groups.io/g/devel/message/69312
Mute This Topic: https://groups.io/mt/79123470/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-platforms PATCH] Silicon/Marvell/RealTimeClockLib: make EpochSeconds, WakeupSeconds UINTN
Posted by Ard Biesheuvel 3 years, 11 months ago 
On 12/21/20 12:24 PM, Laszlo Ersek wrote:
> We're going to change EfiTimeToEpoch() in edk2's TimeBaseLib to propagate
> its internal UINTN calculation to the caller without an internal UINT32
> truncation.
> 
> Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib drives 32-bit-only
> hardware, so catch any number of seconds since the epoch, from
> EfiTimeToEpoch(), that doesn't fit in 32 bits.
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Cc: Leif Lindholm <leif@nuviainc.com>
> Cc: Marcin Wojtas <mw@semihalf.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>

Acked-by: Ard Biesheuvel <ard.biesheuvel@arm.com>

Feel free to push this directly (no CI+label dance required)


> ---
> 
> Notes:
>     Build-tested only, with
>     
>       build \
>         -a AARCH64 \
>         -b NOOPT \
>         -p Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc \
>         -t GCC5 \
>         -m EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
> 
>  Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
> index 1974e0144cd8..a811fd368eca 100644
> --- a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
> +++ b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
> @@ -100,33 +100,36 @@ EFIAPI
>  LibSetTime (
>    IN EFI_TIME                *Time
>    )
>  {
>    EFI_STATUS  Status = EFI_SUCCESS;
> -  UINT32      EpochSeconds;
> +  UINTN       EpochSeconds;
>  
>    // Check the input parameters are within the range specified by UEFI
>    if (!IsTimeValid (Time)) {
>      return EFI_INVALID_PARAMETER;
>    }
>  
>    // Convert time to raw seconds
>    EpochSeconds = EfiTimeToEpoch (Time);
> +  if (EpochSeconds > MAX_UINT32) {
> +    return EFI_INVALID_PARAMETER;
> +  }
>  
>    // Issue delayed write to time register
> -  RtcDelayedWrite (RTC_TIME_REG, EpochSeconds);
> +  RtcDelayedWrite (RTC_TIME_REG, (UINT32)EpochSeconds);
>  
>    return Status;
>  }
>  
>  /**
>    Returns the current wakeup alarm clock setting.
>  
>    @param  Enabled               Indicates if the alarm is currently enabled or disabled.
>    @param  Pending               Indicates if the alarm signal is pending and requires acknowledgement.
>    @param  Time                  The current alarm setting.
>  
>    @retval EFI_SUCCESS           The alarm settings were returned.
>    @retval EFI_INVALID_PARAMETER Any parameter is NULL.
>    @retval EFI_DEVICE_ERROR      The wakeup time could not be retrieved due to a hardware error.
>  
>  **/
> @@ -172,32 +175,35 @@ EFIAPI
>  LibSetWakeupTime (
>    IN BOOLEAN      Enabled,
>    OUT EFI_TIME    *Time
>    )
>  {
> -  UINT32      WakeupSeconds;
> +  UINTN       WakeupSeconds;
>  
>    // Convert time to raw seconds
>    WakeupSeconds = EfiTimeToEpoch (Time);
> +  if (WakeupSeconds > MAX_UINT32) {
> +    return EFI_INVALID_PARAMETER;
> +  }
>  
>    // Issue delayed write to alarm register
> -  RtcDelayedWrite (RTC_ALARM_2_REG, WakeupSeconds);
> +  RtcDelayedWrite (RTC_ALARM_2_REG, (UINT32)WakeupSeconds);
>  
>    if (Enabled) {
>      MmioWrite32 (mArmadaRtcBase + RTC_IRQ_2_CONFIG_REG, RTC_IRQ_ALARM_EN);
>    } else {
>      MmioWrite32 (mArmadaRtcBase + RTC_IRQ_2_CONFIG_REG, 0);
>    }
>  
>    return EFI_SUCCESS;
>  }
>  
>  /**
>    This is the declaration of an EFI image entry point. This can be the entry point to an application
>    written to this specification, an EFI boot service driver, or an EFI runtime driver.
>  
>    @param  ImageHandle           Handle that identifies the loaded image.
>    @param  SystemTable           System Table for this image.
>  
>    @retval EFI_SUCCESS           The operation completed successfully.
>  
>  **/
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69318): https://edk2.groups.io/g/devel/message/69318
Mute This Topic: https://groups.io/mt/79123470/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2-platforms PATCH] Silicon/Marvell/RealTimeClockLib: make EpochSeconds, WakeupSeconds UINTN
Posted by Laszlo Ersek 3 years, 11 months ago 
On 12/21/20 13:16, Ard Biesheuvel wrote:
> On 12/21/20 12:24 PM, Laszlo Ersek wrote:
>> We're going to change EfiTimeToEpoch() in edk2's TimeBaseLib to propagate
>> its internal UINTN calculation to the caller without an internal UINT32
>> truncation.
>>
>> Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib drives 32-bit-only
>> hardware, so catch any number of seconds since the epoch, from
>> EfiTimeToEpoch(), that doesn't fit in 32 bits.
>>
>> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
>> Cc: Leif Lindholm <leif@nuviainc.com>
>> Cc: Marcin Wojtas <mw@semihalf.com>
>> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> 
> Acked-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
> 
> Feel free to push this directly (no CI+label dance required)

Commit fbdfe8c4100d.

Thank you, Ard!
Laszlo

>> ---
>>
>> Notes:
>>     Build-tested only, with
>>     
>>       build \
>>         -a AARCH64 \
>>         -b NOOPT \
>>         -p Platform/SolidRun/Armada80x0McBin/Armada80x0McBin.dsc \
>>         -t GCC5 \
>>         -m EmbeddedPkg/RealTimeClockRuntimeDxe/RealTimeClockRuntimeDxe.inf
>>
>>  Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c | 14 ++++++++++----
>>  1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
>> index 1974e0144cd8..a811fd368eca 100644
>> --- a/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
>> +++ b/Silicon/Marvell/Armada7k8k/Library/RealTimeClockLib/RealTimeClockLib.c
>> @@ -100,33 +100,36 @@ EFIAPI
>>  LibSetTime (
>>    IN EFI_TIME                *Time
>>    )
>>  {
>>    EFI_STATUS  Status = EFI_SUCCESS;
>> -  UINT32      EpochSeconds;
>> +  UINTN       EpochSeconds;
>>  
>>    // Check the input parameters are within the range specified by UEFI
>>    if (!IsTimeValid (Time)) {
>>      return EFI_INVALID_PARAMETER;
>>    }
>>  
>>    // Convert time to raw seconds
>>    EpochSeconds = EfiTimeToEpoch (Time);
>> +  if (EpochSeconds > MAX_UINT32) {
>> +    return EFI_INVALID_PARAMETER;
>> +  }
>>  
>>    // Issue delayed write to time register
>> -  RtcDelayedWrite (RTC_TIME_REG, EpochSeconds);
>> +  RtcDelayedWrite (RTC_TIME_REG, (UINT32)EpochSeconds);
>>  
>>    return Status;
>>  }
>>  
>>  /**
>>    Returns the current wakeup alarm clock setting.
>>  
>>    @param  Enabled               Indicates if the alarm is currently enabled or disabled.
>>    @param  Pending               Indicates if the alarm signal is pending and requires acknowledgement.
>>    @param  Time                  The current alarm setting.
>>  
>>    @retval EFI_SUCCESS           The alarm settings were returned.
>>    @retval EFI_INVALID_PARAMETER Any parameter is NULL.
>>    @retval EFI_DEVICE_ERROR      The wakeup time could not be retrieved due to a hardware error.
>>  
>>  **/
>> @@ -172,32 +175,35 @@ EFIAPI
>>  LibSetWakeupTime (
>>    IN BOOLEAN      Enabled,
>>    OUT EFI_TIME    *Time
>>    )
>>  {
>> -  UINT32      WakeupSeconds;
>> +  UINTN       WakeupSeconds;
>>  
>>    // Convert time to raw seconds
>>    WakeupSeconds = EfiTimeToEpoch (Time);
>> +  if (WakeupSeconds > MAX_UINT32) {
>> +    return EFI_INVALID_PARAMETER;
>> +  }
>>  
>>    // Issue delayed write to alarm register
>> -  RtcDelayedWrite (RTC_ALARM_2_REG, WakeupSeconds);
>> +  RtcDelayedWrite (RTC_ALARM_2_REG, (UINT32)WakeupSeconds);
>>  
>>    if (Enabled) {
>>      MmioWrite32 (mArmadaRtcBase + RTC_IRQ_2_CONFIG_REG, RTC_IRQ_ALARM_EN);
>>    } else {
>>      MmioWrite32 (mArmadaRtcBase + RTC_IRQ_2_CONFIG_REG, 0);
>>    }
>>  
>>    return EFI_SUCCESS;
>>  }
>>  
>>  /**
>>    This is the declaration of an EFI image entry point. This can be the entry point to an application
>>    written to this specification, an EFI boot service driver, or an EFI runtime driver.
>>  
>>    @param  ImageHandle           Handle that identifies the loaded image.
>>    @param  SystemTable           System Table for this image.
>>  
>>    @retval EFI_SUCCESS           The operation completed successfully.
>>  
>>  **/
>>
> 
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69325): https://edk2.groups.io/g/devel/message/69325
Mute This Topic: https://groups.io/mt/79123470/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.