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

["Ard Biesheuvel" <ard.biesheuvel@arm.com>]

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