diff --git a/devices/src/virtio/pvclock.rs b/devices/src/virtio/pvclock.rs index 9b191a79d5..7ae6dc35b9 100644 --- a/devices/src/virtio/pvclock.rs +++ b/devices/src/virtio/pvclock.rs @@ -209,6 +209,7 @@ pub struct PvClock { /// by the PvClockWorker thread but read by PvClock from the mmio bus in the main thread. total_suspend_ns: Arc, features: u64, + acked_features: u64, worker_thread: Option>, /// If the device is sleeping, a [PvClockWorkerSnapshot] that can re-create the worker /// will be stored here. (We can't just store the worker itself as it contains an object @@ -223,6 +224,7 @@ struct PvClockSnapshot { paused_worker: Option, total_suspend_ns: u64, features: u64, + acked_features: u64, } impl PvClock { @@ -235,6 +237,7 @@ impl PvClock { | 1 << VIRTIO_PVCLOCK_F_TSC_STABLE | 1 << VIRTIO_PVCLOCK_F_INJECT_SLEEP | 1 << VIRTIO_PVCLOCK_F_CLOCKSOURCE_RATING, + acked_features: 0, worker_thread: None, paused_worker: None, } @@ -704,11 +707,12 @@ impl VirtioDevice for PvClock { self.features } - // TODO(b/237300012): `self.features` should not be mutated here. Also need - // to check if `value` is subset of `self.features`. - // Example: https://source.chromium.org/chromium/chromiumos/platform/crosvm/+/main:devices/src/virtio/balloon.rs;l=690-693;drc=d2ca9e04c7e477c09a8e14eff392b2d90f52e295 - fn ack_features(&mut self, value: u64) { - self.features &= value; + fn ack_features(&mut self, mut value: u64) { + if value & !self.features != 0 { + warn!("virtio-pvclock got unknown feature ack {:x}", value); + value &= self.features; + } + self.acked_features |= value; } fn read_config(&self, offset: u64, data: &mut [u8]) { @@ -780,6 +784,7 @@ impl VirtioDevice for PvClock { fn virtio_snapshot(&self) -> anyhow::Result { serde_json::to_value(PvClockSnapshot { features: self.features, + acked_features: self.acked_features, total_suspend_ns: self.total_suspend_ns.load(Ordering::SeqCst), tsc_frequency: self.tsc_frequency, paused_worker: self.paused_worker.clone(), @@ -796,6 +801,7 @@ impl VirtioDevice for PvClock { snap.features, ); } + self.acked_features = snap.acked_features; self.total_suspend_ns .store(snap.total_suspend_ns, Ordering::SeqCst); self.paused_worker = snap.paused_worker;