From 18134c7283f1478cceac73b900f5bdc195f12320 Mon Sep 17 00:00:00 2001 From: Frederick Mayle Date: Tue, 26 Mar 2024 10:46:33 -0700 Subject: [PATCH] devices: vhost-user: forbid packed virtqueues temporarily VHOST_USER_SET_VRING_BASE and VHOST_USER_GET_VRING_BASE have different request/response formats if packed queues have been negotiated and we don't support that yet. Forbid it for now in the vhost-user frontend and backend to keep people from tripping on it. Bug: 331466964 Change-Id: I7a064ab0d4677cec465baa444c308c84d87e00b2 Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5397650 Commit-Queue: Frederick Mayle Reviewed-by: Daniel Verkamp --- devices/src/virtio/vhost_user_frontend/mod.rs | 13 ++++++++++++- third_party/vmm_vhost/src/backend_client.rs | 2 ++ third_party/vmm_vhost/src/backend_server.rs | 18 ++++++++++++++++-- third_party/vmm_vhost/src/message.rs | 2 ++ 4 files changed, 32 insertions(+), 3 deletions(-) diff --git a/devices/src/virtio/vhost_user_frontend/mod.rs b/devices/src/virtio/vhost_user_frontend/mod.rs index 8fadb961d2..37f7dd58ac 100644 --- a/devices/src/virtio/vhost_user_frontend/mod.rs +++ b/devices/src/virtio/vhost_user_frontend/mod.rs @@ -122,10 +122,21 @@ impl VhostUserFrontend { connection: vmm_vhost::SystemStream, device_type: DeviceType, max_queue_size: Option, - base_features: u64, + mut base_features: u64, cfg: Option<&[u8]>, pci_address: Option, ) -> Result { + // Don't allow packed queues even if requested. We don't handle them properly yet at the + // protocol layer. + // TODO: b/331466964 - Remove once packed queue support is added to BackendClient. + if base_features & (1 << virtio_sys::virtio_config::VIRTIO_F_RING_PACKED) != 0 { + base_features &= !(1 << virtio_sys::virtio_config::VIRTIO_F_RING_PACKED); + base::warn!( + "VIRTIO_F_RING_PACKED requested, but not yet supported by vhost-user frontend. \ + Automatically disabled." + ); + } + #[cfg(windows)] let backend_pid = connection.target_pid(); diff --git a/third_party/vmm_vhost/src/backend_client.rs b/third_party/vmm_vhost/src/backend_client.rs index 5755095fd6..b1bbfa609b 100644 --- a/third_party/vmm_vhost/src/backend_client.rs +++ b/third_party/vmm_vhost/src/backend_client.rs @@ -188,6 +188,7 @@ impl BackendClient { } /// Set the first index to look for available descriptors. + // TODO: b/331466964 - Arguments and message format are wrong for packed queues. pub fn set_vring_base(&self, queue_index: usize, base: u16) -> Result<()> { let val = VhostUserVringState::new(queue_index as u32, base.into()); let hdr = self.send_request_with_body(FrontendReq::SET_VRING_BASE, &val, None)?; @@ -195,6 +196,7 @@ impl BackendClient { } /// Get the available vring base offset. + // TODO: b/331466964 - Return type is wrong for packed queues. pub fn get_vring_base(&self, queue_index: usize) -> Result { let req = VhostUserVringState::new(queue_index as u32, 0); let hdr = self.send_request_with_body(FrontendReq::GET_VRING_BASE, &req, None)?; diff --git a/third_party/vmm_vhost/src/backend_server.rs b/third_party/vmm_vhost/src/backend_server.rs index acdb99c0ef..29e4acc49d 100644 --- a/third_party/vmm_vhost/src/backend_server.rs +++ b/third_party/vmm_vhost/src/backend_server.rs @@ -41,7 +41,9 @@ pub trait Backend { available: u64, log: u64, ) -> Result<()>; + // TODO: b/331466964 - Argument type is wrong for packed queues. fn set_vring_base(&mut self, index: u32, base: u32) -> Result<()>; + // TODO: b/331466964 - Return type is wrong for packed queues. fn get_vring_base(&mut self, index: u32) -> Result; fn set_vring_kick(&mut self, index: u8, fd: Option) -> Result<()>; fn set_vring_call(&mut self, index: u8, fd: Option) -> Result<()>; @@ -377,14 +379,26 @@ impl BackendServer { } Ok(FrontendReq::GET_FEATURES) => { self.check_request_size(&hdr, size, 0)?; - let features = self.backend.get_features()?; + let mut features = self.backend.get_features()?; + + // Don't advertise packed queues even if the device does. We don't handle them + // properly yet at the protocol layer. + // TODO: b/331466964 - Remove once support is added. + features &= !(1 << VIRTIO_F_RING_PACKED); + let msg = VhostUserU64::new(features); self.send_reply_message(&hdr, &msg)?; self.virtio_features = features; self.update_reply_ack_flag(); } Ok(FrontendReq::SET_FEATURES) => { - let msg = self.extract_request_body::(&hdr, size, &buf)?; + let mut msg = self.extract_request_body::(&hdr, size, &buf)?; + + // Don't allow packed queues even if the device does. We don't handle them + // properly yet at the protocol layer. + // TODO: b/331466964 - Remove once support is added. + msg.value &= !(1 << VIRTIO_F_RING_PACKED); + let res = self.backend.set_features(msg.value); self.acked_virtio_features = msg.value; self.update_reply_ack_flag(); diff --git a/third_party/vmm_vhost/src/message.rs b/third_party/vmm_vhost/src/message.rs index c62a4b7c2b..7f50c08131 100644 --- a/third_party/vmm_vhost/src/message.rs +++ b/third_party/vmm_vhost/src/message.rs @@ -394,6 +394,8 @@ impl VhostUserMsgValidator for VhostUserMsgHeader { } } +pub const VIRTIO_F_RING_PACKED: u32 = 34; + /// Virtio feature flag for the vhost-user protocol features. pub const VHOST_USER_F_PROTOCOL_FEATURES: u32 = 30;