devices: pit: avoid storing raw file descriptor

The Worker struct is storing a Descriptor obtained from a timer for the
sole purpose of adding it to a WaitContext when the worker thread starts
running. Instead of doing this, create the WaitContext and pass it as
part of the worker data, so all ioctls involving that descriptor are
performed directly on the timer, guaranteeing that the descriptor is
valid.

BUG=233968702
TEST=Linux VM starts and boots with `--split-irqchip` specified.

Change-Id: Ie5da1e9866742108f52c526100e936d292167ab7
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3670105
Reviewed-by: Michael Hoyle <mikehoyle@google.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
This commit is contained in:
Alexandre Courbot 2022-05-26 13:08:18 +09:00 committed by Chromeos LUCI
parent 756c0bea9b
commit ed66850d92

View file

@ -8,9 +8,7 @@ use std::sync::Arc;
use std::thread; use std::thread;
use std::time::Duration; use std::time::Duration;
use base::{ use base::{error, warn, Error as SysError, Event, PollToken, WaitContext};
error, warn, AsRawDescriptor, Descriptor, Error as SysError, Event, PollToken, WaitContext,
};
use bit_field::BitField1; use bit_field::BitField1;
use bit_field::*; use bit_field::*;
use hypervisor::{PitChannelState, PitRWMode, PitRWState, PitState}; use hypervisor::{PitChannelState, PitRWMode, PitRWState, PitState};
@ -148,6 +146,14 @@ const NANOS_PER_SEC: u64 = 1_000_000_000;
const MAX_TIMER_FREQ: u32 = 65536; const MAX_TIMER_FREQ: u32 = 65536;
#[derive(PollToken)]
enum Token {
// The timer expired.
TimerExpire,
// The parent thread requested an exit.
Kill,
}
#[sorted] #[sorted]
#[derive(Error, Debug)] #[derive(Error, Debug)]
pub enum PitError { pub enum PitError {
@ -287,16 +293,21 @@ impl Pit {
} }
fn start(&mut self) -> PitResult<()> { fn start(&mut self) -> PitResult<()> {
let wait_ctx: WaitContext<Token> = WaitContext::build_with(&[
(&self.counters[0].lock().timer, Token::TimerExpire),
(&self.kill_evt, Token::Kill),
])
.map_err(PitError::CreateWaitContext)?;
let mut worker = Worker { let mut worker = Worker {
pit_counter: self.counters[0].clone(), pit_counter: self.counters[0].clone(),
fd: Descriptor(self.counters[0].lock().timer.as_raw_descriptor()), wait_ctx,
}; };
let evt = self.kill_evt.try_clone().map_err(PitError::CloneEvent)?;
self.worker_thread = Some( self.worker_thread = Some(
thread::Builder::new() thread::Builder::new()
.name("pit counter worker".to_string()) .name("pit counter worker".to_string())
.spawn(move || worker.run(evt)) .spawn(move || worker.run())
.map_err(PitError::SpawnThread)?, .map_err(PitError::SpawnThread)?,
); );
@ -878,25 +889,13 @@ impl PitCounter {
struct Worker { struct Worker {
pit_counter: Arc<Mutex<PitCounter>>, pit_counter: Arc<Mutex<PitCounter>>,
fd: Descriptor, wait_ctx: WaitContext<Token>,
} }
impl Worker { impl Worker {
fn run(&mut self, kill_evt: Event) -> PitResult<()> { fn run(&mut self) -> PitResult<()> {
#[derive(PollToken)]
enum Token {
// The timer expired.
TimerExpire,
// The parent thread requested an exit.
Kill,
}
let wait_ctx: WaitContext<Token> =
WaitContext::build_with(&[(&self.fd, Token::TimerExpire), (&kill_evt, Token::Kill)])
.map_err(PitError::CreateWaitContext)?;
loop { loop {
let events = wait_ctx.wait().map_err(PitError::WaitError)?; let events = self.wait_ctx.wait().map_err(PitError::WaitError)?;
for event in events.iter().filter(|e| e.is_readable) { for event in events.iter().filter(|e| e.is_readable) {
match event.token { match event.token {
Token::TimerExpire => { Token::TimerExpire => {