mbox series

[0/4] Extend time to wait for UIP for some callers

Message ID 20231117063220.65093-1-mario.limonciello@amd.com
Headers show
Series Extend time to wait for UIP for some callers | expand

Message

Mario Limonciello Nov. 17, 2023, 6:32 a.m. UTC 
A number of users have reported their system will have a failure reading
the RTC around s2idle entry or exit.

This failure manifests as UIP clear taking longer than 10ms. Affected users
have reported that after this happens the clock jumps forward to 2077, which
is presumably from epoch + century bit.

Users who used a debugging patch provided by Mateusz Jończyk demonstrated
that this has taken upwards of 480ms in some cases.

This series adjusts the UIP timeout to be configurable by the caller and
changes some callers which aren't called in an interrupt context to allow
longer timeouts.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=217626
Link: https://community.frame.work/t/responded-fw13-amd-fedora-39-system-clock-advances-50-years-during-overnight-suspend
Mario Limonciello (4):
  rtc: mc146818-lib: Adjust failure return code for mc146818_get_time()
  rtc: Adjust failure return code for cmos_set_alarm()
  rtc: Add support for configuring the UIP timeout for RTC reads
  rtc: Extend timeout for waiting for UIP to clear to 1s

 arch/alpha/kernel/rtc.c        |  2 +-
 arch/x86/kernel/hpet.c         |  2 +-
 arch/x86/kernel/rtc.c          |  2 +-
 drivers/base/power/trace.c     |  2 +-
 drivers/rtc/rtc-cmos.c         |  8 ++++----
 drivers/rtc/rtc-mc146818-lib.c | 35 ++++++++++++++++++++++++----------
 include/linux/mc146818rtc.h    |  3 ++-
 7 files changed, 35 insertions(+), 19 deletions(-)

Comments

Mateusz Jończyk Nov. 18, 2023, 7:06 p.m. UTC | #1 
Hello,

W dniu 17.11.2023 o 07:32, Mario Limonciello pisze:
> The UIP timeout is hardcoded to 10ms for all RTC reads, but in some
> contexts this might not be enough time. Add a timeout parameter to
> mc146818_get_time() and mc146818_get_time_callback().
> Make all callers use 10ms to ensure no functional changes.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Fixes: ec5895c0f2d8 ("rtc: mc146818-lib: extract mc146818_avoid_UIP")

If you would like to Cc: stable this patch,

commit d2a632a8a117 ("rtc: mc146818-lib: reduce RTC_UIP polling period")

is a prerequisite.

> ---
>  arch/alpha/kernel/rtc.c        |  2 +-
>  arch/x86/kernel/hpet.c         |  2 +-
>  arch/x86/kernel/rtc.c          |  2 +-
>  drivers/base/power/trace.c     |  2 +-
>  drivers/rtc/rtc-cmos.c         |  6 +++---
>  drivers/rtc/rtc-mc146818-lib.c | 31 +++++++++++++++++++++++--------
>  include/linux/mc146818rtc.h    |  3 ++-
>  7 files changed, 32 insertions(+), 16 deletions(-)
>
[snip]
> --- a/drivers/rtc/rtc-mc146818-lib.c
> +++ b/drivers/rtc/rtc-mc146818-lib.c
> @@ -8,26 +8,29 @@
>  #include <linux/acpi.h>
>  #endif
>  
> +#define UIP_RECHECK_DELAY		100	/* usec */
> +
>  /*
>   * Execute a function while the UIP (Update-in-progress) bit of the RTC is
> - * unset.
> + * unset. The timeout is configurable by the caller in ms.
>   *
>   * Warning: callback may be executed more then once.
>   */
>  bool mc146818_avoid_UIP(void (*callback)(unsigned char seconds, void *param),
> +			int timeout,
>  			void *param)
>  {
>  	int i;
>  	unsigned long flags;
>  	unsigned char seconds;
>  
> -	for (i = 0; i < 100; i++) {
> +	for (i = 0; i < USEC_PER_MSEC / UIP_RECHECK_DELAY * timeout; i++) {
>  		spin_lock_irqsave(&rtc_lock, flags);
>  
>  		/*
>  		 * Check whether there is an update in progress during which the
>  		 * readout is unspecified. The maximum update time is ~2ms. Poll
> -		 * every 100 usec for completion.
> +		 * for completion.
>  		 *
>  		 * Store the second value before checking UIP so a long lasting
>  		 * NMI which happens to hit after the UIP check cannot make
> @@ -37,7 +40,7 @@ bool mc146818_avoid_UIP(void (*callback)(unsigned char seconds, void *param),
>  
>  		if (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) {
>  			spin_unlock_irqrestore(&rtc_lock, flags);
> -			udelay(100);
> +			udelay(UIP_RECHECK_DELAY);
>  			continue;
>  		}
>  
> @@ -56,7 +59,7 @@ bool mc146818_avoid_UIP(void (*callback)(unsigned char seconds, void *param),
>  		 */
>  		if (CMOS_READ(RTC_FREQ_SELECT) & RTC_UIP) {
>  			spin_unlock_irqrestore(&rtc_lock, flags);
> -			udelay(100);
> +			udelay(UIP_RECHECK_DELAY);
>  			continue;
>  		}
>  

I think that when reading the RTC is not finished in 100ms or so
(irrespective of the timeout parameter), the code should log
a warning / an error message.

Greetings,

Mateusz