x86/time: prefer CMOS over EFI_GET_TIME

Commit Message

Roger Pau Monne March 15, 2024, 11:42 a.m. UTC

EFI_GET_TIME doesn't seem to be very reliable:

----[ Xen-4.19-unstable  x86_64  debug=y  Tainted:   C    ]----
CPU:    0
RIP:    e008:[<0000000062ccfa70>] 0000000062ccfa70
[...]
Xen call trace:
   [<0000000062ccfa70>] R 0000000062ccfa70
   [<00000000732e9a3f>] S 00000000732e9a3f
   [<ffff82d04034f34f>] F arch/x86/time.c#get_cmos_time+0x1b3/0x26e
   [<ffff82d04045926f>] F init_xen_time+0x28/0xa4
   [<ffff82d040454bc4>] F __start_xen+0x1ee7/0x2578
   [<ffff82d040203334>] F __high_start+0x94/0xa0

Pagetable walk from 0000000062ccfa70:
 L4[0x000] = 000000207ef1c063 ffffffffffffffff
 L3[0x001] = 000000005d6c0063 ffffffffffffffff
 L2[0x116] = 8000000062c001e3 ffffffffffffffff (PSE)

****************************************
Panic on CPU 0:
FATAL PAGE FAULT
[error_code=0011]
Faulting linear address: 0000000062ccfa70
****************************************

Swap the preference to default to CMOS first, and EFI later, in an attempt to
use EFI_GET_TIME as a last resort option only.  Note that Linux for example
doesn't allow calling the get_time method, and instead provides a dummy handler
that unconditionally returns EFI_UNSUPPORTED on x86-64.

The error checks are moved to the end of the function, in order to have them
grouped together.

Signed-off-by: Roger Pau Monné <[e-mail]>
---
 xen/arch/x86/time.c | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

Patch

diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 60870047894b..fcce5528e1f0 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1176,26 +1176,17 @@  static void __get_cmos_time(struct rtc_time *rtc)
 
 static unsigned long get_cmos_time(void)
 {
-    unsigned long res, flags;
+    unsigned long flags;
     struct rtc_time rtc;
     unsigned int seconds = 60;
     static bool __read_mostly cmos_rtc_probe;
     boolean_param("cmos-rtc-probe", cmos_rtc_probe);
 
-    if ( efi_enabled(EFI_RS) )
-    {
-        res = efi_get_time();
-        if ( res )
-            return res;
-    }
-
     if ( likely(!(acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_CMOS_RTC)) )
         cmos_rtc_probe = false;
-    else if ( system_state < SYS_STATE_smp_boot && !cmos_rtc_probe )
-        panic("System with no CMOS RTC advertised must be booted from EFI"
-              " (or with command line option \"cmos-rtc-probe\")\n");
 
-    for ( ; ; )
+    for ( ; !(acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_CMOS_RTC) ||
+            cmos_rtc_probe; )
     {
         s_time_t start, t1, t2;
 
@@ -1223,7 +1214,8 @@  static unsigned long get_cmos_time(void)
              rtc.sec >= 60 || rtc.min >= 60 || rtc.hour >= 24 ||
              !rtc.day || rtc.day > 31 ||
              !rtc.mon || rtc.mon > 12 )
-            break;
+            return mktime(rtc.year, rtc.mon, rtc.day, rtc.hour, rtc.min,
+                          rtc.sec);
 
         if ( seconds < 60 )
         {
@@ -1231,6 +1223,8 @@  static unsigned long get_cmos_time(void)
             {
                 cmos_rtc_probe = false;
                 acpi_gbl_FADT.boot_flags &= ~ACPI_FADT_NO_CMOS_RTC;
+                return mktime(rtc.year, rtc.mon, rtc.day, rtc.hour, rtc.min,
+                              rtc.sec);
             }
             break;
         }
@@ -1240,10 +1234,22 @@  static unsigned long get_cmos_time(void)
         seconds = rtc.sec;
     }
 
-    if ( unlikely(cmos_rtc_probe) )
-        panic("No CMOS RTC found - system must be booted from EFI\n");
+    if ( efi_enabled(EFI_RS) )
+    {
+        unsigned long res = efi_get_time();
+
+        if ( res )
+            return res;
+
+        panic("Broken EFI_GET_TIME %s\n",
+              !cmos_rtc_probe ? "try booting with \"cmos-rtc-probe\""
+                              : "and no CMOS RTC found");
+    }
+    if ( !cmos_rtc_probe )
+        panic("System with no CMOS RTC advertised must be booted from EFI"
+              " (or with command line option \"cmos-rtc-probe\")\n");
 
-    return mktime(rtc.year, rtc.mon, rtc.day, rtc.hour, rtc.min, rtc.sec);
+    panic("No CMOS RTC found - system must be booted from EFI\n");
 }
 
 static unsigned int __ro_after_init cmos_alias_mask;