diff --git a/Cargo.lock b/Cargo.lock index 3c0549b4..585e768c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -296,6 +296,12 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "intmap" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ee87fd093563344074bacf24faa0bb0227fb6969fb223e922db798516de924d6" + [[package]] name = "io-lifetimes" version = "1.0.3" @@ -333,18 +339,19 @@ checksum = "fc7fcc620a3bff7cdd7a365be3376c97191aeaccc2a603e600951e452615bf89" [[package]] name = "libgpiod" version = "0.1.0" -source = "git+https://github.com/vireshk/libgpiod#52f16effabd5c6838d00f4d4ad054d5304e0d5d6" +source = "git+https://github.com/vireshk/libgpiod?branch=vhost-gpio#d7b36c56170331699fd5e4e18918af81333aa64a" dependencies = [ + "errno", + "intmap", "libc", "libgpiod-sys", "thiserror", - "vmm-sys-util", ] [[package]] name = "libgpiod-sys" version = "0.1.0" -source = "git+https://github.com/vireshk/libgpiod#52f16effabd5c6838d00f4d4ad054d5304e0d5d6" +source = "git+https://github.com/vireshk/libgpiod?branch=vhost-gpio#d7b36c56170331699fd5e4e18918af81333aa64a" dependencies = [ "cc", ] diff --git a/coverage_config_x86_64.json b/coverage_config_x86_64.json index 9cf6dcc8..5bc39d89 100644 --- a/coverage_config_x86_64.json +++ b/coverage_config_x86_64.json @@ -1,5 +1,5 @@ { - "coverage_score": 69.6, + "coverage_score": 68.9, "exclude_path": "", "crate_features": "" } diff --git a/crates/gpio/Cargo.toml b/crates/gpio/Cargo.toml index c4024555..cc0f9eaa 100644 --- a/crates/gpio/Cargo.toml +++ b/crates/gpio/Cargo.toml @@ -25,7 +25,7 @@ vm-memory = "0.10" vmm-sys-util = "0.11" [target.'cfg(target_env = "gnu")'.dependencies] -libgpiod = { git = "https://github.com/vireshk/libgpiod" } +libgpiod = { git = "https://github.com/vireshk/libgpiod", branch = "vhost-gpio" } [dev-dependencies] virtio-queue = { version = "0.7", features = ["test-utils"] } diff --git a/crates/gpio/src/gpio.rs b/crates/gpio/src/gpio.rs index 1f7a9912..c8db25f0 100644 --- a/crates/gpio/src/gpio.rs +++ b/crates/gpio/src/gpio.rs @@ -9,10 +9,7 @@ use log::error; use std::sync::{Arc, RwLock}; use std::time::Duration; -use libgpiod::{ - Chip, Direction, Edge, EdgeEventBuffer, Error as LibGpiodError, LineConfig, LineRequest, - RequestConfig, -}; +use libgpiod::{chip, line, request, Error as LibGpiodError}; use thiserror::Error as ThisError; use vm_memory::{ByteValued, Le16, Le32}; @@ -31,8 +28,6 @@ pub(crate) enum Error { GpioMessageInvalid(u16), #[error("Gpiod operation failed {0:?}")] GpiodFailed(LibGpiodError), - #[error("Gpio irq operation timed out")] - GpioIrqOpTimedOut, #[error("Gpio irq type invalid {0}")] GpioIrqTypeInvalid(u16), #[error("Gpio irq type not supported {0}")] @@ -102,25 +97,25 @@ pub(crate) trait GpioDevice: Send + Sync + 'static { where Self: Sized; - fn get_num_gpios(&self) -> Result; - fn get_gpio_name(&self, gpio: u16) -> Result; - fn get_direction(&self, gpio: u16) -> Result; + fn num_gpios(&self) -> Result; + fn gpio_name(&self, gpio: u16) -> Result; + fn direction(&self, gpio: u16) -> Result; fn set_direction(&self, gpio: u16, dir: u8, value: u32) -> Result<()>; - fn get_value(&self, gpio: u16) -> Result; + fn value(&self, gpio: u16) -> Result; fn set_value(&self, gpio: u16, value: u32) -> Result<()>; fn set_irq_type(&self, gpio: u16, value: u16) -> Result<()>; - fn wait_for_interrupt(&self, gpio: u16) -> Result<()>; + fn wait_for_interrupt(&self, gpio: u16) -> Result; } pub(crate) struct PhysLineState { // See wait_for_interrupt() for explanation of Arc. - request: Option>, - buffer: Option, + request: Option>, + buffer: Option, } pub(crate) struct PhysDevice { - chip: Chip, + chip: chip::Chip, ngpio: u16, state: Vec>, } @@ -138,8 +133,8 @@ impl GpioDevice for PhysDevice { Self: Sized, { let path = format!("/dev/gpiochip{}", device); - let chip = Chip::open(&path).map_err(Error::GpiodFailed)?; - let ngpio = chip.get_num_lines() as u16; + let chip = chip::Chip::open(&path).map_err(Error::GpiodFailed)?; + let ngpio = chip.info().map_err(Error::GpiodFailed)?.num_lines() as u16; // Can't set a vector to all None easily let mut state: Vec> = Vec::new(); @@ -153,36 +148,34 @@ impl GpioDevice for PhysDevice { Ok(PhysDevice { chip, ngpio, state }) } - fn get_num_gpios(&self) -> Result { + fn num_gpios(&self) -> Result { Ok(self.ngpio) } - fn get_gpio_name(&self, gpio: u16) -> Result { + fn gpio_name(&self, gpio: u16) -> Result { let line_info = self .chip .line_info(gpio.into()) .map_err(Error::GpiodFailed)?; - Ok(line_info.get_name().unwrap_or("").to_string()) + Ok(line_info.name().unwrap_or("").to_string()) } - fn get_direction(&self, gpio: u16) -> Result { + fn direction(&self, gpio: u16) -> Result { let line_info = self .chip .line_info(gpio.into()) .map_err(Error::GpiodFailed)?; - Ok( - match line_info.get_direction().map_err(Error::GpiodFailed)? { - Direction::AsIs => VIRTIO_GPIO_DIRECTION_NONE, - Direction::Input => VIRTIO_GPIO_DIRECTION_IN, - Direction::Output => VIRTIO_GPIO_DIRECTION_OUT, - }, - ) + Ok(match line_info.direction().map_err(Error::GpiodFailed)? { + line::Direction::AsIs => VIRTIO_GPIO_DIRECTION_NONE, + line::Direction::Input => VIRTIO_GPIO_DIRECTION_IN, + line::Direction::Output => VIRTIO_GPIO_DIRECTION_OUT, + }) } fn set_direction(&self, gpio: u16, dir: u8, value: u32) -> Result<()> { - let mut config = LineConfig::new().map_err(Error::GpiodFailed)?; + let mut lsettings = line::Settings::new().map_err(Error::GpiodFailed)?; let state = &mut self.state[gpio as usize].write().unwrap(); match dir { @@ -192,29 +185,41 @@ impl GpioDevice for PhysDevice { } VIRTIO_GPIO_DIRECTION_IN => { - config.set_direction_override(Direction::Input, gpio as u32); + lsettings + .set_direction(line::Direction::Input) + .map_err(Error::GpiodFailed)?; } VIRTIO_GPIO_DIRECTION_OUT => { - config.set_direction_default(Direction::Output); - config.set_output_value_override(gpio as u32, value); + let value = line::Value::new(value as i32).map_err(Error::GpiodFailed)?; + lsettings + .set_direction(line::Direction::Output) + .map_err(Error::GpiodFailed)? + .set_output_value(value) + .map_err(Error::GpiodFailed)?; } _ => return Err(Error::GpioDirectionInvalid(value)), }; + let lconfig = line::Config::new().map_err(Error::GpiodFailed)?; + lconfig + .add_line_settings(&[gpio as u32], lsettings) + .map_err(Error::GpiodFailed)?; + if let Some(request) = &state.request { request - .reconfigure_lines(&config) + .reconfigure_lines(&lconfig) .map_err(Error::GpiodFailed)?; } else { - let rconfig = RequestConfig::new().map_err(Error::GpiodFailed)?; + let rconfig = request::Config::new().map_err(Error::GpiodFailed)?; - rconfig.set_consumer("vhu-gpio"); - rconfig.set_offsets(&[gpio as u32]); + rconfig + .set_consumer("vhu-gpio") + .map_err(Error::GpiodFailed)?; state.request = Some(Arc::new( self.chip - .request_lines(&rconfig, &config) + .request_lines(&rconfig, &lconfig) .map_err(Error::GpiodFailed)?, )); } @@ -222,11 +227,11 @@ impl GpioDevice for PhysDevice { Ok(()) } - fn get_value(&self, gpio: u16) -> Result { + fn value(&self, gpio: u16) -> Result { let state = &self.state[gpio as usize].read().unwrap(); if let Some(request) = &state.request { - Ok(request.get_value(gpio as u32).map_err(Error::GpiodFailed)? as u8) + Ok(request.value(gpio as u32).map_err(Error::GpiodFailed)? as u8) } else { Err(Error::GpioDirectionInvalid( VIRTIO_GPIO_DIRECTION_NONE as u32, @@ -240,8 +245,9 @@ impl GpioDevice for PhysDevice { // Direction change can follow value change, don't fail here for invalid // direction. if let Some(request) = &state.request { + let value = line::Value::new(value as i32).map_err(Error::GpiodFailed)?; request - .set_value(gpio as u32, value as i32) + .set_value(gpio as u32, value) .map_err(Error::GpiodFailed)?; } @@ -250,12 +256,11 @@ impl GpioDevice for PhysDevice { fn set_irq_type(&self, gpio: u16, value: u16) -> Result<()> { let state = &mut self.state[gpio as usize].write().unwrap(); - let mut config = LineConfig::new().map_err(Error::GpiodFailed)?; - match value as u16 { - VIRTIO_GPIO_IRQ_TYPE_EDGE_RISING => config.set_edge_detection_default(Edge::Rising), - VIRTIO_GPIO_IRQ_TYPE_EDGE_FALLING => config.set_edge_detection_default(Edge::Falling), - VIRTIO_GPIO_IRQ_TYPE_EDGE_BOTH => config.set_edge_detection_default(Edge::Both), + let edge = match value as u16 { + VIRTIO_GPIO_IRQ_TYPE_EDGE_RISING => line::Edge::Rising, + VIRTIO_GPIO_IRQ_TYPE_EDGE_FALLING => line::Edge::Falling, + VIRTIO_GPIO_IRQ_TYPE_EDGE_BOTH => line::Edge::Both, // Drop the buffer. VIRTIO_GPIO_IRQ_TYPE_NONE => { @@ -267,24 +272,35 @@ impl GpioDevice for PhysDevice { _ => return Err(Error::GpioIrqTypeNotSupported(value)), }; - // Allocate the buffer and configure the line for interrupt. if state.request.is_none() { - Err(Error::GpioLineRequestNotConfigured) - } else { - // The GPIO Virtio specification allows a single interrupt event for each - // `wait_for_interrupt()` message. And for that we need a single `EdgeEventBuffer`. - state.buffer = Some(EdgeEventBuffer::new(1).map_err(Error::GpiodFailed)?); - - state - .request - .as_ref() - .unwrap() - .reconfigure_lines(&config) - .map_err(Error::GpiodFailed) + return Err(Error::GpioLineRequestNotConfigured); } + + let mut lsettings = line::Settings::new().map_err(Error::GpiodFailed)?; + lsettings + .set_edge_detection(Some(edge)) + .map_err(Error::GpiodFailed)?; + + let lconfig = line::Config::new().map_err(Error::GpiodFailed)?; + lconfig + .add_line_settings(&[gpio as u32], lsettings) + .map_err(Error::GpiodFailed)?; + + // Allocate the buffer and configure the line for interrupt. + // + // The GPIO Virtio specification allows a single interrupt event for each + // `wait_for_interrupt()` message. And for that we need a single `request::Buffer`. + state.buffer = Some(request::Buffer::new(1).map_err(Error::GpiodFailed)?); + + state + .request + .as_ref() + .unwrap() + .reconfigure_lines(&lconfig) + .map_err(Error::GpiodFailed) } - fn wait_for_interrupt(&self, gpio: u16) -> Result<()> { + fn wait_for_interrupt(&self, gpio: u16) -> Result { // While waiting here for the interrupt to occur, it is possible that we receive another // request from the guest to disable the interrupt instead, via a call to `set_irq_type()`. // @@ -321,19 +337,21 @@ impl GpioDevice for PhysDevice { }; // Wait for the interrupt for a second. - match request.wait_edge_event(Duration::new(1, 0)) { - Err(LibGpiodError::OperationTimedOut) => return Err(Error::GpioIrqOpTimedOut), - x => x.map_err(Error::GpiodFailed)?, + if !request + .wait_edge_event(Some(Duration::new(1, 0))) + .map_err(Error::GpiodFailed)? + { + return Ok(false); } // The interrupt has already occurred, we can lock now just fine. - let state = &self.state[gpio as usize].write().unwrap(); - if let Some(buffer) = &state.buffer { + let state = &mut self.state[gpio as usize].write().unwrap(); + if let Some(buffer) = &mut state.buffer { request - .read_edge_event(buffer, 1) + .read_edge_events(buffer) .map_err(Error::GpiodFailed)?; - Ok(()) + Ok(true) } else { Err(Error::GpioLineRequestNotConfigured) } @@ -358,7 +376,7 @@ pub(crate) struct GpioController { impl GpioController { // Creates a new controller corresponding to `device`. pub(crate) fn new(device: D) -> Result> { - let ngpio = device.get_num_gpios()?; + let ngpio = device.num_gpios()?; // The gpio's name can be of any length, we are just trying to allocate something // reasonable to start with, we can always extend it later. @@ -366,14 +384,14 @@ impl GpioController { let mut state = Vec::with_capacity(ngpio as usize); for offset in 0..ngpio { - let name = device.get_gpio_name(offset)?; + let name = device.gpio_name(offset)?; // Create line names gpio_names.push_str(&name); gpio_names.push('\0'); state.push(RwLock::new(GpioState { - dir: device.get_direction(offset)?, + dir: device.direction(offset)?, val: None, irq_type: VIRTIO_GPIO_IRQ_TYPE_NONE, })); @@ -391,12 +409,12 @@ impl GpioController { }) } - pub(crate) fn get_num_gpios(&self) -> u16 { - self.device.get_num_gpios().unwrap() + pub(crate) fn num_gpios(&self) -> u16 { + self.device.num_gpios().unwrap() } - fn get_direction(&self, gpio: u16) -> Result { - self.device.get_direction(gpio) + fn direction(&self, gpio: u16) -> Result { + self.device.direction(gpio) } fn set_direction(&self, gpio: u16, dir: u32) -> Result<()> { @@ -422,8 +440,8 @@ impl GpioController { Ok(()) } - fn get_value(&self, gpio: u16) -> Result { - self.device.get_value(gpio) + fn value(&self, gpio: u16) -> Result { + self.device.value(gpio) } fn set_value(&self, gpio: u16, value: u32) -> Result<()> { @@ -436,7 +454,7 @@ impl GpioController { Ok(()) } - pub(crate) fn get_irq_type(&self, gpio: u16) -> u16 { + pub(crate) fn irq_type(&self, gpio: u16) -> u16 { self.state[gpio as usize].read().unwrap().irq_type } @@ -469,27 +487,28 @@ impl GpioController { pub(crate) fn wait_for_interrupt(&self, gpio: u16) -> Result<()> { loop { - match self.device.wait_for_interrupt(gpio) { - Err(Error::GpioIrqOpTimedOut) => continue, - Ok(_) => return Ok(()), - x => x?, + if !self.device.wait_for_interrupt(gpio)? { + continue; } + + // Event found + return Ok(()); } } - pub(crate) fn get_config(&self) -> &VirtioGpioConfig { + pub(crate) fn config(&self) -> &VirtioGpioConfig { &self.config } pub(crate) fn operation(&self, rtype: u16, gpio: u16, value: u32) -> Result> { Ok(match rtype { VIRTIO_GPIO_MSG_GET_LINE_NAMES => self.gpio_names.as_bytes().to_vec(), - VIRTIO_GPIO_MSG_GET_DIRECTION => vec![self.get_direction(gpio)?], + VIRTIO_GPIO_MSG_GET_DIRECTION => vec![self.direction(gpio)?], VIRTIO_GPIO_MSG_SET_DIRECTION => { self.set_direction(gpio, value)?; vec![0] } - VIRTIO_GPIO_MSG_GET_VALUE => vec![self.get_value(gpio)?], + VIRTIO_GPIO_MSG_GET_VALUE => vec![self.value(gpio)?], VIRTIO_GPIO_MSG_SET_VALUE => { self.set_value(gpio, value)?; vec![0] @@ -515,14 +534,14 @@ pub(crate) mod tests { ngpio: u16, pub(crate) gpio_names: Vec, state: RwLock>, - get_num_gpios_result: Result, - get_gpio_name_result: Result, - get_direction_result: Result, + num_gpios_result: Result, + gpio_name_result: Result, + direction_result: Result, set_direction_result: Result<()>, - get_value_result: Result, + value_result: Result, set_value_result: Result<()>, set_irq_type_result: Result<()>, - pub(crate) wait_for_irq_result: Result<()>, + pub(crate) wait_for_irq_result: Result, } impl DummyDevice { @@ -538,14 +557,14 @@ pub(crate) mod tests { }; ngpio.into() ]), - get_num_gpios_result: Ok(0), - get_gpio_name_result: Ok("".to_string()), - get_direction_result: Ok(0), + num_gpios_result: Ok(0), + gpio_name_result: Ok("".to_string()), + direction_result: Ok(0), set_direction_result: Ok(()), - get_value_result: Ok(0), + value_result: Ok(0), set_value_result: Ok(()), set_irq_type_result: Ok(()), - wait_for_irq_result: Ok(()), + wait_for_irq_result: Ok(true), } } } @@ -558,27 +577,27 @@ pub(crate) mod tests { Ok(DummyDevice::new(8)) } - fn get_num_gpios(&self) -> Result { - if self.get_num_gpios_result.is_err() { - return self.get_num_gpios_result; + fn num_gpios(&self) -> Result { + if self.num_gpios_result.is_err() { + return self.num_gpios_result; } Ok(self.ngpio) } - fn get_gpio_name(&self, gpio: u16) -> Result { + fn gpio_name(&self, gpio: u16) -> Result { assert!((gpio as usize) < self.gpio_names.len()); - if self.get_gpio_name_result.is_err() { - return self.get_gpio_name_result.clone(); + if self.gpio_name_result.is_err() { + return self.gpio_name_result.clone(); } Ok(self.gpio_names[gpio as usize].clone()) } - fn get_direction(&self, gpio: u16) -> Result { - if self.get_direction_result.is_err() { - return self.get_direction_result; + fn direction(&self, gpio: u16) -> Result { + if self.direction_result.is_err() { + return self.direction_result; } Ok(self.state.read().unwrap()[gpio as usize].dir) @@ -601,9 +620,9 @@ pub(crate) mod tests { Ok(()) } - fn get_value(&self, gpio: u16) -> Result { - if self.get_value_result.is_err() { - return self.get_value_result; + fn value(&self, gpio: u16) -> Result { + if self.value_result.is_err() { + return self.value_result; } if let Some(val) = self.state.read().unwrap()[gpio as usize].val { @@ -630,12 +649,12 @@ pub(crate) mod tests { Ok(()) } - fn wait_for_interrupt(&self, _gpio: u16) -> Result<()> { + fn wait_for_interrupt(&self, _gpio: u16) -> Result { if self.wait_for_irq_result.is_err() { return self.wait_for_irq_result; } - Ok(()) + Ok(true) } } @@ -662,7 +681,7 @@ pub(crate) mod tests { let controller = GpioController::new(device).unwrap(); assert_eq!( - *controller.get_config(), + *controller.config(), VirtioGpioConfig { ngpio: From::from(NGPIO), padding: From::from(0), @@ -676,7 +695,7 @@ pub(crate) mod tests { name.push('\0'); } - assert_eq!(controller.get_num_gpios(), NGPIO); + assert_eq!(controller.num_gpios(), NGPIO); assert_eq!( controller @@ -703,7 +722,7 @@ pub(crate) mod tests { ); // No initial irq type - assert_eq!(controller.get_irq_type(gpio), VIRTIO_GPIO_IRQ_TYPE_NONE); + assert_eq!(controller.irq_type(gpio), VIRTIO_GPIO_IRQ_TYPE_NONE); } } @@ -779,10 +798,7 @@ pub(crate) mod tests { .unwrap(); // Verify interrupt type - assert_eq!( - controller.get_irq_type(gpio), - VIRTIO_GPIO_IRQ_TYPE_EDGE_RISING, - ); + assert_eq!(controller.irq_type(gpio), VIRTIO_GPIO_IRQ_TYPE_EDGE_RISING); // Set irq type none controller @@ -794,7 +810,7 @@ pub(crate) mod tests { .unwrap(); // Verify interrupt type - assert_eq!(controller.get_irq_type(gpio), VIRTIO_GPIO_IRQ_TYPE_NONE); + assert_eq!(controller.irq_type(gpio), VIRTIO_GPIO_IRQ_TYPE_NONE); // Set irq type falling controller @@ -806,10 +822,7 @@ pub(crate) mod tests { .unwrap(); // Verify interrupt type - assert_eq!( - controller.get_irq_type(gpio), - VIRTIO_GPIO_IRQ_TYPE_EDGE_FALLING, - ); + assert_eq!(controller.irq_type(gpio), VIRTIO_GPIO_IRQ_TYPE_EDGE_FALLING); // Set irq type none controller @@ -821,7 +834,7 @@ pub(crate) mod tests { .unwrap(); // Verify interrupt type - assert_eq!(controller.get_irq_type(gpio), VIRTIO_GPIO_IRQ_TYPE_NONE); + assert_eq!(controller.irq_type(gpio), VIRTIO_GPIO_IRQ_TYPE_NONE); // Set irq type both controller @@ -833,10 +846,7 @@ pub(crate) mod tests { .unwrap(); // Verify interrupt type - assert_eq!( - controller.get_irq_type(gpio), - VIRTIO_GPIO_IRQ_TYPE_EDGE_BOTH, - ); + assert_eq!(controller.irq_type(gpio), VIRTIO_GPIO_IRQ_TYPE_EDGE_BOTH); } } @@ -846,19 +856,19 @@ pub(crate) mod tests { // Get num lines failure let error = Error::GpioOperationFailed("get-num-lines"); let mut device = DummyDevice::new(NGPIO); - device.get_num_gpios_result = Err(error); + device.num_gpios_result = Err(error); assert_eq!(GpioController::new(device).unwrap_err(), error); // Get line name failure let error = Error::GpioOperationFailed("get-line-name"); let mut device = DummyDevice::new(NGPIO); - device.get_gpio_name_result = Err(error); + device.gpio_name_result = Err(error); assert_eq!(GpioController::new(device).unwrap_err(), error); // Get direction failure let error = Error::GpioOperationFailed("get-direction"); let mut device = DummyDevice::new(NGPIO); - device.get_direction_result = Err(error); + device.direction_result = Err(error); assert_eq!(GpioController::new(device).unwrap_err(), error); } diff --git a/crates/gpio/src/vhu_gpio.rs b/crates/gpio/src/vhu_gpio.rs index ea259dff..a12d1f5b 100644 --- a/crates/gpio/src/vhu_gpio.rs +++ b/crates/gpio/src/vhu_gpio.rs @@ -165,7 +165,7 @@ impl VhostUserGpioBackend { pub(crate) fn new(controller: GpioController) -> Result { // Can't set a vector to all None easily let mut handles: Vec>> = Vec::new(); - handles.resize_with(controller.get_num_gpios() as usize, || None); + handles.resize_with(controller.num_gpios() as usize, || None); Ok(VhostUserGpioBackend { controller: Arc::new(controller), @@ -289,7 +289,7 @@ impl VhostUserGpioBackend { // Interrupt should be enabled before sending buffer and no other buffer // should have been received earlier for this GPIO pin. - if controller.get_irq_type(gpio) == VIRTIO_GPIO_IRQ_TYPE_NONE || handle.is_some() { + if controller.irq_type(gpio) == VIRTIO_GPIO_IRQ_TYPE_NONE || handle.is_some() { send_event_response(vring, desc_chain, addr, VIRTIO_GPIO_IRQ_STATUS_INVALID); return; } @@ -412,7 +412,7 @@ impl VhostUserBackendMut unsafe { from_raw_parts( self.controller - .get_config() + .config() .as_slice() .as_ptr() .offset(offset as isize) as *const _ as *const _,