[PATCH v4 3/6] x86/time: split CMOS read and probe logic into function

Roger Pau Monne posted 6 patches 3 months ago
There is a newer version of this series
[PATCH v4 3/6] x86/time: split CMOS read and probe logic into function
Posted by Roger Pau Monne 3 months ago
The current logic to probe for the CMOS RTC is open-coded in get_cmos_time(),
move it to a separate function that both serves the purpose of testing for the
CMOS RTC existence and returning its value.

The goal is to be able to split the probing and the reading logic into separate
helpers, and putting the current logic in a separate function helps simplifying
further changes.

A transient *rtc_p variable is introduced as a parameter to the function, that
will be removed by further changes.  Also note that due to the code movement,
now cmos_rtc_probe will only get cleared on a second call to get_cmos_time(),
as the newly introduced cmos_probe() function doesn't modify the variable
anymore.

No functional change intended.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v2:
 - New in this version.
---
 xen/arch/x86/time.c | 59 +++++++++++++++++++++++++++------------------
 1 file changed, 35 insertions(+), 24 deletions(-)

diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index 571e23431ccd..1e3c13fc7360 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1291,45 +1291,32 @@ static bool __get_cmos_time(struct rtc_time *rtc)
     return t1 <= SECONDS(1) && t2 < MILLISECS(3);
 }
 
-static unsigned long get_cmos_time(void)
+static bool cmos_probe(struct rtc_time *rtc_p, bool cmos_rtc_probe)
 {
-    unsigned long res;
-    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 ( ; ; )
     {
-        bool success = __get_cmos_time(&rtc);
+        bool success = __get_cmos_time(rtc_p);
+        struct rtc_time rtc = *rtc_p;
 
-        if ( likely(!cmos_rtc_probe) || !success ||
+        if ( likely(!cmos_rtc_probe) )
+            return true;
+
+        if ( !success ||
              rtc.sec >= 60 || rtc.min >= 60 || rtc.hour >= 24 ||
              !rtc.day || rtc.day > 31 ||
              !rtc.mon || rtc.mon > 12 )
-            break;
+            return false;
 
         if ( seconds < 60 )
         {
             if ( rtc.sec != seconds )
             {
-                cmos_rtc_probe = false;
                 acpi_gbl_FADT.boot_flags &= ~ACPI_FADT_NO_CMOS_RTC;
+                return true;
             }
-            break;
+            return false;
         }
 
         process_pending_softirqs();
@@ -1337,7 +1324,31 @@ static unsigned long get_cmos_time(void)
         seconds = rtc.sec;
     }
 
-    if ( unlikely(cmos_rtc_probe) )
+    ASSERT_UNREACHABLE();
+    return false;
+}
+
+static unsigned long get_cmos_time(void)
+{
+    unsigned long res;
+    struct rtc_time rtc;
+    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");
+
+    if ( !cmos_probe(&rtc, cmos_rtc_probe) )
         panic("No CMOS RTC found - system must be booted from EFI\n");
 
     return mktime(rtc.year, rtc.mon, rtc.day, rtc.hour, rtc.min, rtc.sec);
-- 
2.46.0


Re: [PATCH v4 3/6] x86/time: split CMOS read and probe logic into function
Posted by Jan Beulich 2 months, 4 weeks ago
On 04.09.2024 17:31, Roger Pau Monne wrote:
> The current logic to probe for the CMOS RTC is open-coded in get_cmos_time(),
> move it to a separate function that both serves the purpose of testing for the
> CMOS RTC existence and returning its value.
> 
> The goal is to be able to split the probing and the reading logic into separate
> helpers, and putting the current logic in a separate function helps simplifying
> further changes.
> 
> A transient *rtc_p variable is introduced as a parameter to the function, that
> will be removed by further changes.  Also note that due to the code movement,
> now cmos_rtc_probe will only get cleared on a second call to get_cmos_time(),
> as the newly introduced cmos_probe() function doesn't modify the variable
> anymore.
> 
> No functional change intended.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>