cmos: Move alarm state into separate struct

Consolidate alarm related state into a single struct, so that it can be
shared between the Cmos and worker thread with a single Arc<Mutex<>>.
This change also gets rid of the ability to skip having an alarm, which
was only used by tests.

BUG=None
TEST=CQ

Change-Id: Id8dd25195758acc5e576ac7db7544c29f8b41a47
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5512663
Commit-Queue: David Stevens <stevensd@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>

1 files changed, 81 insertions, 95 deletions

diff --git a/devices/src/cmos.rs b/devices/src/cmos.rs
index f0aeaf865..510363b38 100644
--- a/devices/src/cmos.rs
+++ b/devices/src/cmos.rs
@@ -74,45 +74,19 @@ const RTC_REG_D_VRT: u8 = 0x80; // RAM and time valid
 
 pub type CmosNowFn = fn() -> DateTime<Utc>;
 
-/// A CMOS/RTC device commonly seen on x86 I/O port 0x70/0x71.
-#[derive(Serialize)]
-pub struct Cmos {
-    index: u8,
-    #[serde(serialize_with = "serialize_arr")]
-    data: [u8; DATA_LEN],
-    #[serde(skip_serializing)] // skip serializing time function.
-    now_fn: CmosNowFn,
-    #[serde(skip_serializing)] // skip serializing the timer
-    alarm: Arc<Mutex<Timer>>,
-    alarm_time: Option<DateTime<Utc>>,
-    #[serde(skip_serializing)] // skip serializing the alarm function
-    alarm_fn: Option<AlarmFn>,
-    #[serde(skip_serializing)] // skip serializing the worker thread
-    worker: Option<WorkerThread<AlarmFn>>,
-    #[serde(skip_serializing)] // skip serializing the armed time
-    armed_time: Option<Arc<Mutex<Instant>>>,
-}
-
-struct AlarmFn {
-    irq: IrqEdgeEvent,
+// Alarm state shared between Cmos and the alarm worker thread.
+struct AlarmState {
+    alarm: Timer,
     vm_control: Tube,
-    armed_time: Arc<Mutex<Instant>>,
+    irq: IrqEdgeEvent,
+    armed_time: Instant,
 }
 
-impl AlarmFn {
-    fn new(irq: IrqEdgeEvent, vm_control: Tube) -> Self {
-        Self {
-            irq,
-            vm_control,
-            // Not actually armed, but simpler than wrapping with an Option.
-            armed_time: Arc::new(Mutex::new(Instant::now())),
-        }
-    }
-
-    fn fire(&self) -> anyhow::Result<()> {
+impl AlarmState {
+    fn trigger_rtc_interrupt(&self) -> anyhow::Result<()> {
         self.irq.trigger().context("failed to trigger irq")?;
 
-        let elapsed = self.armed_time.lock().elapsed().as_millis();
+        let elapsed = self.armed_time.elapsed().as_millis();
         log_metric(
             MetricEventType::RtcWakeup,
             elapsed.try_into().unwrap_or(i64::MAX),
@@ -130,6 +104,26 @@ impl AlarmFn {
     }
 }
 
+/// A CMOS/RTC device commonly seen on x86 I/O port 0x70/0x71.
+#[derive(Serialize)]
+pub struct Cmos {
+    index: u8,
+    #[serde(serialize_with = "serialize_arr")]
+    data: [u8; DATA_LEN],
+    #[serde(skip_serializing)] // skip serializing time function.
+    now_fn: CmosNowFn,
+    // alarm_time is re-loaded from data on deserialization, so there's
+    // no need to explicitly serialize it.
+    #[serde(skip_serializing)]
+    alarm_time: Option<DateTime<Utc>>,
+    // alarm_state fields are either constant across snapshotting or
+    // reloaded from |data| on restore, so no need to serialize.
+    #[serde(skip_serializing)]
+    alarm_state: Arc<Mutex<AlarmState>>,
+    #[serde(skip_serializing)] // skip serializing the worker thread
+    worker: Option<WorkerThread<()>>,
+}
+
 impl Cmos {
     /// Constructs a CMOS/RTC device with initial data.
     /// `mem_below_4g` is the size of memory in bytes below the 32-bit gap.
@@ -142,20 +136,6 @@ impl Cmos {
         vm_control: Tube,
         irq: IrqEdgeEvent,
     ) -> anyhow::Result<Cmos> {
-        Self::new_inner(
-            mem_below_4g,
-            mem_above_4g,
-            now_fn,
-            Some(AlarmFn::new(irq, vm_control)),
-        )
-    }
-
-    fn new_inner(
-        mem_below_4g: u64,
-        mem_above_4g: u64,
-        now_fn: CmosNowFn,
-        alarm_fn: Option<AlarmFn>,
-    ) -> anyhow::Result<Cmos> {
         let mut data = [0u8; DATA_LEN];
 
         data[0x0B] = RTC_REG_B_24_HOUR_MODE; // Status Register B: 24-hour mode
@@ -174,31 +154,32 @@ impl Cmos {
         data[0x5c] = (high_mem >> 8) as u8;
         data[0x5d] = (high_mem >> 16) as u8;
 
-        let armed_time = alarm_fn.as_ref().map(|a| a.armed_time.clone());
         Ok(Cmos {
             index: 0,
             data,
             now_fn,
-            alarm: Arc::new(Mutex::new(Timer::new().context("cmos timer")?)),
             alarm_time: None,
-            alarm_fn,
+            alarm_state: Arc::new(Mutex::new(AlarmState {
+                alarm: Timer::new().context("cmos timer")?,
+                irq,
+                vm_control,
+                // Not actually armed, but simpler than wrapping with an Option.
+                armed_time: Instant::now(),
+            })),
             worker: None,
-            armed_time,
         })
     }
 
-    fn spawn_worker(&mut self) {
-        let alarm = self.alarm.clone();
-        let alarm_fn = self.alarm_fn.take().expect("no alarm function");
+    fn spawn_worker(&mut self, alarm_state: Arc<Mutex<AlarmState>>) {
         self.worker = Some(WorkerThread::start("CMOS_alarm", move |kill_evt| {
-            if let Err(e) = run_cmos_worker(alarm, kill_evt, &alarm_fn) {
+            if let Err(e) = run_cmos_worker(alarm_state, kill_evt) {
                 error!("Failed to spawn worker {:?}", e);
             }
-            alarm_fn
         }));
     }
 
     fn set_alarm(&mut self) {
+        let mut state = self.alarm_state.lock();
         if self.data[RTC_REG_B as usize] & RTC_REG_B_ALARM_ENABLE != 0 {
             let now = (self.now_fn)();
             let target = alarm_from_registers(now.year(), &self.data).and_then(|this_year| {
@@ -221,55 +202,57 @@ impl Cmos {
             if let Some(target) = target {
                 if Some(target) != self.alarm_time {
                     self.alarm_time = Some(target);
-
-                    if self.alarm_fn.is_some() {
-                        self.spawn_worker();
-                    }
-                    if let Some(armed_time) = self.armed_time.as_ref() {
-                        *armed_time.lock() = Instant::now();
-                    }
+                    state.armed_time = Instant::now();
 
                     let duration = target
                         .signed_duration_since(now)
                         .to_std()
                         .unwrap_or(Duration::new(0, 0));
-                    if let Err(e) = self.alarm.lock().reset_oneshot(duration) {
+                    if let Err(e) = state.alarm.reset_oneshot(duration) {
                         error!("Failed to set alarm {:?}", e);
                     }
                 }
             }
         } else if self.alarm_time.take().is_some() {
-            if let Err(e) = self.alarm.lock().clear() {
+            if let Err(e) = state.alarm.clear() {
                 error!("Failed to clear alarm {:?}", e);
             }
         }
+
+        let needs_worker = self.alarm_time.is_some();
+        drop(state);
+
+        if needs_worker && self.worker.is_none() {
+            self.spawn_worker(self.alarm_state.clone());
+        }
     }
 }
 
-fn run_cmos_worker(
-    alarm: Arc<Mutex<Timer>>,
-    kill_evt: Event,
-    alarm_fn: &AlarmFn,
-) -> anyhow::Result<()> {
+fn run_cmos_worker(alarm_state: Arc<Mutex<AlarmState>>, kill_evt: Event) -> anyhow::Result<()> {
     #[derive(EventToken)]
     enum Token {
         Alarm,
         Kill,
     }
 
-    let wait_ctx: WaitContext<Token> =
-        WaitContext::build_with(&[(&*alarm.lock(), Token::Alarm), (&kill_evt, Token::Kill)])
-            .context("worker context failed")?;
+    let wait_ctx: WaitContext<Token> = WaitContext::build_with(&[
+        (&alarm_state.lock().alarm, Token::Alarm),
+        (&kill_evt, Token::Kill),
+    ])
+    .context("worker context failed")?;
 
     loop {
         let events = wait_ctx.wait().context("wait failed")?;
+        let mut state = alarm_state.lock();
         for event in events.iter().filter(|e| e.is_readable) {
             match event.token {
                 Token::Alarm => {
-                    if alarm.lock().mark_waited().context("timer ack failed")? {
+                    if state.alarm.mark_waited().context("timer ack failed")? {
                         continue;
                     }
-                    alarm_fn.fire()?;
+                    if let Err(e) = state.trigger_rtc_interrupt() {
+                        error!("Failed to send rtc {:?}", e);
+                    }
                 }
                 Token::Kill => return Ok(()),
             }
@@ -427,15 +410,13 @@ impl Suspendable for Cmos {
 
     fn sleep(&mut self) -> anyhow::Result<()> {
         if let Some(worker) = self.worker.take() {
-            self.alarm_fn = Some(worker.stop());
+            worker.stop();
         }
         Ok(())
     }
 
     fn wake(&mut self) -> anyhow::Result<()> {
-        if self.alarm_time.is_some() {
-            self.spawn_worker();
-        }
+        self.spawn_worker(self.alarm_state.clone());
         Ok(())
     }
 }
@@ -518,9 +499,14 @@ mod tests {
         timestamp_to_datetime(1483228800)
     }
 
+    fn new_cmos_for_test(now_fn: CmosNowFn) -> Cmos {
+        let irq = IrqEdgeEvent::new().unwrap();
+        Cmos::new(1024, 0, now_fn, Tube::pair().unwrap().0, irq).unwrap()
+    }
+
     #[test]
     fn cmos_write_index() {
-        let mut cmos = Cmos::new_inner(1024, 0, test_now_party_like_its_1999, None).unwrap();
+        let mut cmos = new_cmos_for_test(test_now_party_like_its_1999);
         // Write index.
         cmos.write(
             BusAccessInfo {
@@ -535,7 +521,7 @@ mod tests {
 
     #[test]
     fn cmos_write_data() {
-        let mut cmos = Cmos::new_inner(1024, 0, test_now_party_like_its_1999, None).unwrap();
+        let mut cmos = new_cmos_for_test(test_now_party_like_its_1999);
         // Write data 0x01 at index 0x41.
         cmos.write(
             BusAccessInfo {
@@ -575,7 +561,7 @@ mod tests {
 
     #[test]
     fn cmos_date_time_1999() {
-        let mut cmos = Cmos::new_inner(1024, 0, test_now_party_like_its_1999, None).unwrap();
+        let mut cmos = new_cmos_for_test(test_now_party_like_its_1999);
         assert_eq!(read_reg(&mut cmos, 0x00), 0x59); // seconds
         assert_eq!(read_reg(&mut cmos, 0x02), 0x59); // minutes
         assert_eq!(read_reg(&mut cmos, 0x04), 0x23); // hours
@@ -588,7 +574,7 @@ mod tests {
 
     #[test]
     fn cmos_date_time_2000() {
-        let mut cmos = Cmos::new_inner(1024, 0, test_now_y2k_compliant, None).unwrap();
+        let mut cmos = new_cmos_for_test(test_now_y2k_compliant);
         assert_eq!(read_reg(&mut cmos, 0x00), 0x00); // seconds
         assert_eq!(read_reg(&mut cmos, 0x02), 0x00); // minutes
         assert_eq!(read_reg(&mut cmos, 0x04), 0x00); // hours
@@ -601,7 +587,7 @@ mod tests {
 
     #[test]
     fn cmos_date_time_before_leap_second() {
-        let mut cmos = Cmos::new_inner(1024, 0, test_now_2016_before_leap_second, None).unwrap();
+        let mut cmos = new_cmos_for_test(test_now_2016_before_leap_second);
         assert_eq!(read_reg(&mut cmos, 0x00), 0x59); // seconds
         assert_eq!(read_reg(&mut cmos, 0x02), 0x59); // minutes
         assert_eq!(read_reg(&mut cmos, 0x04), 0x23); // hours
@@ -614,7 +600,7 @@ mod tests {
 
     #[test]
     fn cmos_date_time_after_leap_second() {
-        let mut cmos = Cmos::new_inner(1024, 0, test_now_2017_after_leap_second, None).unwrap();
+        let mut cmos = new_cmos_for_test(test_now_2017_after_leap_second);
         assert_eq!(read_reg(&mut cmos, 0x00), 0x00); // seconds
         assert_eq!(read_reg(&mut cmos, 0x02), 0x00); // minutes
         assert_eq!(read_reg(&mut cmos, 0x04), 0x00); // hours
@@ -629,7 +615,7 @@ mod tests {
     fn cmos_alarm() {
         // 2000-01-02T03:04:05+00:00
         let now_fn = || timestamp_to_datetime(946782245);
-        let mut cmos = Cmos::new_inner(1024, 0, now_fn, None).unwrap();
+        let mut cmos = new_cmos_for_test(now_fn);
 
         // A date later this year
         write_reg(&mut cmos, 0x01, 0x06); // seconds
@@ -671,14 +657,14 @@ mod tests {
 
     #[test]
     fn cmos_reg_d() {
-        let mut cmos = Cmos::new_inner(1024, 0, test_now_party_like_its_1999, None).unwrap();
+        let mut cmos = new_cmos_for_test(test_now_party_like_its_1999);
         assert_eq!(read_reg(&mut cmos, 0x0d), 0x80) // RAM and time are valid
     }
 
     #[test]
     fn cmos_snapshot_restore() -> anyhow::Result<()> {
         // time function doesn't matter in this case.
-        let mut cmos = Cmos::new_inner(1024, 0, test_now_party_like_its_1999, None).unwrap();
+        let mut cmos = new_cmos_for_test(test_now_party_like_its_1999);
 
         let info_index = BusAccessInfo {
             offset: 0,
@@ -715,9 +701,9 @@ mod tests {
     #[test]
     fn cmos_sleep_wake() {
         // 2000-01-02T03:04:05+00:00
+        let irq = IrqEdgeEvent::new().unwrap();
         let now_fn = || timestamp_to_datetime(946782245);
-        let alarm_fn = AlarmFn::new(IrqEdgeEvent::new().unwrap(), Tube::pair().unwrap().0);
-        let mut cmos = Cmos::new_inner(1024, 0, now_fn, Some(alarm_fn)).unwrap();
+        let mut cmos = Cmos::new(1024, 0, now_fn, Tube::pair().unwrap().0, irq).unwrap();
 
         // A date later this year
         write_reg(&mut cmos, 0x01, 0x06); // seconds
@@ -739,22 +725,22 @@ mod tests {
 
     suspendable_tests!(
         cmos1999,
-        Cmos::new_inner(1024, 0, test_now_party_like_its_1999, None).unwrap(),
+        new_cmos_for_test(test_now_party_like_its_1999),
         modify_device
     );
     suspendable_tests!(
         cmos2k,
-        Cmos::new_inner(1024, 0, test_now_y2k_compliant, None).unwrap(),
+        new_cmos_for_test(test_now_y2k_compliant),
         modify_device
     );
     suspendable_tests!(
         cmos2016,
-        Cmos::new_inner(1024, 0, test_now_2016_before_leap_second, None).unwrap(),
+        new_cmos_for_test(test_now_2016_before_leap_second),
         modify_device
     );
     suspendable_tests!(
         cmos2017,
-        Cmos::new_inner(1024, 0, test_now_2017_after_leap_second, None).unwrap(),
+        new_cmos_for_test(test_now_2017_after_leap_second),
         modify_device
     );
 }