public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-platforms PATCH] Silicon/Marvell/RealTimeClockLib: make EpochSeconds, WakeupSeconds UINTN
@ 2020-12-21 11:24 Laszlo Ersek
  2020-12-21 12:16 ` Ard Biesheuvel
  0 siblings, 1 reply; 3+ messages in thread
From: Laszlo Ersek @ 2020-12-21 11:24 UTC (permalink / raw)
  To: edk2-devel-groups-io
  Cc: Ard Biesheuvel, Leif Lindholm, Marcin Wojtas,
	Philippe Mathieu-Daudé

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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [edk2-platforms PATCH] Silicon/Marvell/RealTimeClockLib: make EpochSeconds, WakeupSeconds UINTN
  2020-12-21 11:24 [edk2-platforms PATCH] Silicon/Marvell/RealTimeClockLib: make EpochSeconds, WakeupSeconds UINTN Laszlo Ersek
@ 2020-12-21 12:16 ` Ard Biesheuvel
  2020-12-21 13:21   ` [edk2-devel] " Laszlo Ersek
  0 siblings, 1 reply; 3+ messages in thread
From: Ard Biesheuvel @ 2020-12-21 12:16 UTC (permalink / raw)
  To: Laszlo Ersek, edk2-devel-groups-io
  Cc: Leif Lindholm, Marcin Wojtas, Philippe Mathieu-Daudé

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.
>  
>  **/
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [edk2-devel] [edk2-platforms PATCH] Silicon/Marvell/RealTimeClockLib: make EpochSeconds, WakeupSeconds UINTN
  2020-12-21 12:16 ` Ard Biesheuvel
@ 2020-12-21 13:21   ` Laszlo Ersek
  0 siblings, 0 replies; 3+ messages in thread
From: Laszlo Ersek @ 2020-12-21 13:21 UTC (permalink / raw)
  To: devel, ard.biesheuvel
  Cc: Leif Lindholm, Marcin Wojtas, Philippe Mathieu-Daudé

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.
>>  
>>  **/
>>
> 
> 
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-12-21 13:22 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-12-21 11:24 [edk2-platforms PATCH] Silicon/Marvell/RealTimeClockLib: make EpochSeconds, WakeupSeconds UINTN Laszlo Ersek
2020-12-21 12:16 ` Ard Biesheuvel
2020-12-21 13:21   ` [edk2-devel] " Laszlo Ersek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox