From 2a989f5eaf3674dfd07a7483bbf8be5cd839c14c Mon Sep 17 00:00:00 2001 From: Will Wright Date: Mon, 1 Dec 2025 22:58:36 +0000 Subject: [PATCH 1/6] Revert "[VMBus] Add NUMA support in proxyintegration (#2411)" This reverts commit fd7139d91450e8dcb167cacdcf245ee30931fde2. --- Cargo.lock | 1 - vm/devices/vmbus/vmbus_proxy/Cargo.toml | 1 - vm/devices/vmbus/vmbus_proxy/src/lib.rs | 56 ------------------- .../vmbus/vmbus_proxy/src/proxyioctl.rs | 7 --- .../vmbus_server/src/proxyintegration.rs | 49 +++------------- 5 files changed, 9 insertions(+), 105 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0c38e924e6..3bd3c5cb93 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8991,7 +8991,6 @@ dependencies = [ "futures", "guestmem", "guid", - "headervec", "mesh", "pal", "pal_async", diff --git a/vm/devices/vmbus/vmbus_proxy/Cargo.toml b/vm/devices/vmbus/vmbus_proxy/Cargo.toml index 5495a98bbb..d40ed03fba 100644 --- a/vm/devices/vmbus/vmbus_proxy/Cargo.toml +++ b/vm/devices/vmbus/vmbus_proxy/Cargo.toml @@ -9,7 +9,6 @@ rust-version.workspace = true [target.'cfg(windows)'.dependencies] guestmem.workspace = true guid.workspace = true -headervec.workspace = true vmbus_core.workspace = true mesh.workspace = true pal.workspace = true diff --git a/vm/devices/vmbus/vmbus_proxy/src/lib.rs b/vm/devices/vmbus/vmbus_proxy/src/lib.rs index ef23a72c28..ca8eabe91e 100644 --- a/vm/devices/vmbus/vmbus_proxy/src/lib.rs +++ b/vm/devices/vmbus/vmbus_proxy/src/lib.rs @@ -10,7 +10,6 @@ use futures::poll; use guestmem::GuestMemory; use guid::Guid; -use headervec::HeaderVec; use mesh::CancelContext; use mesh::MeshPayload; use pal::windows::ObjectAttributes; @@ -30,7 +29,6 @@ use vmbusioctl::VMBUS_SERVER_OPEN_CHANNEL_OUTPUT_PARAMETERS; use widestring::Utf16Str; use widestring::utf16str; use windows::Wdk::Storage::FileSystem::NtOpenFile; -use windows::Win32::Foundation::ERROR_MORE_DATA; use windows::Win32::Foundation::ERROR_OPERATION_ABORTED; use windows::Win32::Foundation::HANDLE; use windows::Win32::Foundation::NTSTATUS; @@ -457,60 +455,6 @@ impl VmbusProxy { Ok(()) } - pub fn get_numa_node_map(&self) -> Result> { - unsafe { - // This is a synchronous operation, so don't use the async IO infrastructure. - let mut output = - HeaderVec::::with_capacity( - zeroed::(), - 8, - ); - let mut bytes = 0; - if let Err(e) = DeviceIoControl( - HANDLE(self.file.get().as_raw_handle()), - proxyioctl::IOCTL_VMBUS_PROXY_GET_NUMA_MAP, - None, - 0, - Some(output.as_mut_ptr().cast()), - output.total_byte_capacity() as u32, - Some(&mut bytes), - None, - ) { - if e.code() == ERROR_MORE_DATA.into() { - // The buffer was too small, resize and try again. The proxy returns the required buffer size - // in VpCount on overflow, so use that. - assert!( - bytes as usize >= size_of::() - ); - - let required_len = output.head.VpCount as usize; - output.reserve_tail(required_len - output.tail_capacity()); - DeviceIoControl( - HANDLE(self.file.get().as_raw_handle()), - proxyioctl::IOCTL_VMBUS_PROXY_GET_NUMA_MAP, - None, - 0, - Some(output.as_mut_ptr().cast()), - output.total_byte_capacity() as u32, - Some(&mut bytes), - None, - )?; - } else { - return Err(e); - } - } - - assert!( - bytes as usize - >= size_of::() - + output.head.VpCount as usize - ); - - output.set_tail_len(output.head.VpCount as usize); - Ok(output.tail.to_vec()) - } - } - /// Adds GPADL ioctl data to a buffer. fn add_gpadl( buffer: &mut Vec, diff --git a/vm/devices/vmbus/vmbus_proxy/src/proxyioctl.rs b/vm/devices/vmbus/vmbus_proxy/src/proxyioctl.rs index 1edf583b7d..a1a7ece08f 100644 --- a/vm/devices/vmbus/vmbus_proxy/src/proxyioctl.rs +++ b/vm/devices/vmbus/vmbus_proxy/src/proxyioctl.rs @@ -49,7 +49,6 @@ pub const IOCTL_VMBUS_PROXY_TL_CONNECT_REQUEST: u32 = VMBUS_PROXY_IOCTL(0xc); pub const IOCTL_VMBUS_PROXY_RESTORE_CHANNEL: u32 = VMBUS_PROXY_IOCTL(0xd); pub const IOCTL_VMBUS_PROXY_REVOKE_UNCLAIMED_CHANNELS: u32 = VMBUS_PROXY_IOCTL(0xe); pub const IOCTL_VMBUS_PROXY_RESTORE_SET_INTERRUPT: u32 = VMBUS_PROXY_IOCTL(0xf); -pub const IOCTL_VMBUS_PROXY_GET_NUMA_MAP: u32 = VMBUS_PROXY_IOCTL(0x14); #[repr(C)] #[derive(Copy, Clone, zerocopy::IntoBytes)] @@ -207,9 +206,3 @@ pub struct VMBUS_PROXY_TL_CONNECT_REQUEST_INPUT { pub Vtl: u8, pub Padding: [u8; 3], } - -#[repr(C)] -#[derive(Copy, Clone)] -pub struct VMBUS_PROXY_GET_NUMA_MAP_OUTPUT { - pub VpCount: u32, -} diff --git a/vm/devices/vmbus/vmbus_server/src/proxyintegration.rs b/vm/devices/vmbus/vmbus_server/src/proxyintegration.rs index c973e88e9e..7ae83dab2a 100644 --- a/vm/devices/vmbus/vmbus_server/src/proxyintegration.rs +++ b/vm/devices/vmbus/vmbus_server/src/proxyintegration.rs @@ -64,7 +64,6 @@ use vmbus_proxy::ProxyAction; use vmbus_proxy::VmbusProxy; use vmbus_proxy::vmbusioctl::VMBUS_SERVER_OPEN_CHANNEL_OUTPUT_PARAMETERS; use vmcore::interrupt::Interrupt; -use windows::Win32::Foundation::ERROR_INVALID_FUNCTION; use windows::Win32::Foundation::ERROR_NOT_FOUND; use windows::Win32::Foundation::ERROR_OPERATION_ABORTED; use zerocopy::IntoBytes; @@ -242,18 +241,6 @@ impl SavedStatePair { } } -struct VpToPhysicalNodeMap(Vec); - -impl VpToPhysicalNodeMap { - fn new(nodes: Vec) -> Self { - Self(nodes) - } - - fn get_numa_node(&self, vp_index: u32) -> u16 { - self.0.get(vp_index as usize).copied().unwrap_or(0).into() - } -} - struct ProxyTask { channels: Arc>>, gpadls: Arc>>>, @@ -263,7 +250,6 @@ struct ProxyTask { hvsock_response_send: Option>, vtl2_hvsock_response_send: Option>, saved_states: Arc>, - numa_node_map: VpToPhysicalNodeMap, } impl ProxyTask { @@ -273,7 +259,6 @@ impl ProxyTask { hvsock_response_send: Option>, vtl2_hvsock_response_send: Option>, proxy: Arc, - numa_node_map: VpToPhysicalNodeMap, ) -> Self { Self { channels: Arc::new(Mutex::new(HashMap::new())), @@ -287,7 +272,6 @@ impl ProxyTask { saved_state: None, vtl2_saved_state: None, })), - numa_node_map, } } @@ -342,9 +326,7 @@ impl ProxyTask { &VMBUS_SERVER_OPEN_CHANNEL_OUTPUT_PARAMETERS { RingBufferGpadlHandle: open_request.open_data.ring_gpadl_id.0, DownstreamRingBufferPageOffset: open_request.open_data.ring_offset, - NodeNumber: self - .numa_node_map - .get_numa_node(open_request.open_data.target_vp), + NodeNumber: 0, // BUGBUG: NUMA Padding: 0, }, maybe_wrapped.event(), @@ -967,15 +949,14 @@ impl ProxyTask { }) }); - let open_params = - channel - .open_request() - .map(|request| VMBUS_SERVER_OPEN_CHANNEL_OUTPUT_PARAMETERS { - RingBufferGpadlHandle: request.ring_buffer_gpadl_id.0, - DownstreamRingBufferPageOffset: request.downstream_ring_buffer_page_offset, - NodeNumber: self.numa_node_map.get_numa_node(request.target_vp), - Padding: 0, - }); + let open_params = channel.open_request().map(|request| { + VMBUS_SERVER_OPEN_CHANNEL_OUTPUT_PARAMETERS { + RingBufferGpadlHandle: request.ring_buffer_gpadl_id.0, + DownstreamRingBufferPageOffset: request.downstream_ring_buffer_page_offset, + NodeNumber: 0, // BUGBUG: NUMA + Padding: 0, + } + }); let proxy_id = self .proxy @@ -1143,24 +1124,12 @@ async fn proxy_thread( let (send, recv) = mesh::channel(); let proxy = Arc::new(proxy); - let numa_node_map = VpToPhysicalNodeMap::new(proxy.get_numa_node_map().unwrap_or_else(|err| { - if err.code() == ERROR_INVALID_FUNCTION.into() { - tracing::info!("proxy does not support NUMA node map ioctl"); - } else { - tracing::warn!( - error = &err as &dyn std::error::Error, - "failed to get NUMA node map from proxy" - ); - } - Vec::new() - })); let task = Arc::new(ProxyTask::new( server.control, vtl2_control, hvsock_response_send, vtl2_hvsock_response_send, Arc::clone(&proxy), - numa_node_map, )); let offers = task.run_proxy_actions(send, flush_recv, await_flush); let requests = task.run_server_requests( From 14daf7c183076d1b640f50bea7ba9ddb45119d5d Mon Sep 17 00:00:00 2001 From: Will Wright Date: Thu, 11 Dec 2025 15:10:37 -0800 Subject: [PATCH 2/6] Implement user-mode NUMA node map --- .../vmbus_server/src/proxyintegration.rs | 50 +++++++++++++++---- 1 file changed, 41 insertions(+), 9 deletions(-) diff --git a/vm/devices/vmbus/vmbus_server/src/proxyintegration.rs b/vm/devices/vmbus/vmbus_server/src/proxyintegration.rs index 7ae83dab2a..4105ca7940 100644 --- a/vm/devices/vmbus/vmbus_server/src/proxyintegration.rs +++ b/vm/devices/vmbus/vmbus_server/src/proxyintegration.rs @@ -110,6 +110,7 @@ pub struct ProxyIntegrationBuilder<'a, T: SpawnDriver + Clone> { vtl2_server: Option, mem: Option<&'a GuestMemory>, require_flush_before_start: bool, + vp_to_physical_node_map: Option>, } impl<'a, T: SpawnDriver + Clone> ProxyIntegrationBuilder<'a, T> { @@ -131,6 +132,12 @@ impl<'a, T: SpawnDriver + Clone> ProxyIntegrationBuilder<'a, T> { self } + /// Adds a NUMA node map to be passed to the proxy driver. + pub fn vp_to_physical_node_map(mut self, map: Option>) -> Self { + self.vp_to_physical_node_map = map; + self + } + /// Builds and starts the `ProxyIntegration`. pub async fn build(self) -> io::Result { let (cancel_ctx, cancel) = CancelContext::new().with_cancel(); @@ -150,6 +157,7 @@ impl<'a, T: SpawnDriver + Clone> ProxyIntegrationBuilder<'a, T> { self.vtl2_server, flush_recv, self.require_flush_before_start, + self.vp_to_physical_node_map, ), ); @@ -183,6 +191,7 @@ impl ProxyIntegration { vtl2_server: None, mem: None, require_flush_before_start: false, + vp_to_physical_node_map: None, } } @@ -241,6 +250,19 @@ impl SavedStatePair { } } +struct VpToPhysicalNodeMap(Option>); + +impl VpToPhysicalNodeMap { + fn get_numa_node(&self, vp_index: u32) -> u16 { + self.0 + .as_ref() + .map_or(0, |nodes| { + nodes.get(vp_index as usize).copied().unwrap_or(0) + }) + .into() + } +} + struct ProxyTask { channels: Arc>>, gpadls: Arc>>>, @@ -250,6 +272,7 @@ struct ProxyTask { hvsock_response_send: Option>, vtl2_hvsock_response_send: Option>, saved_states: Arc>, + vp_to_physical_node_map: VpToPhysicalNodeMap, } impl ProxyTask { @@ -259,6 +282,7 @@ impl ProxyTask { hvsock_response_send: Option>, vtl2_hvsock_response_send: Option>, proxy: Arc, + vp_to_physical_node_map: VpToPhysicalNodeMap, ) -> Self { Self { channels: Arc::new(Mutex::new(HashMap::new())), @@ -272,6 +296,7 @@ impl ProxyTask { saved_state: None, vtl2_saved_state: None, })), + vp_to_physical_node_map, } } @@ -326,7 +351,9 @@ impl ProxyTask { &VMBUS_SERVER_OPEN_CHANNEL_OUTPUT_PARAMETERS { RingBufferGpadlHandle: open_request.open_data.ring_gpadl_id.0, DownstreamRingBufferPageOffset: open_request.open_data.ring_offset, - NodeNumber: 0, // BUGBUG: NUMA + NodeNumber: self + .vp_to_physical_node_map + .get_numa_node(open_request.open_data.target_vp), Padding: 0, }, maybe_wrapped.event(), @@ -949,14 +976,17 @@ impl ProxyTask { }) }); - let open_params = channel.open_request().map(|request| { - VMBUS_SERVER_OPEN_CHANNEL_OUTPUT_PARAMETERS { - RingBufferGpadlHandle: request.ring_buffer_gpadl_id.0, - DownstreamRingBufferPageOffset: request.downstream_ring_buffer_page_offset, - NodeNumber: 0, // BUGBUG: NUMA - Padding: 0, - } - }); + let open_params = + channel + .open_request() + .map(|request| VMBUS_SERVER_OPEN_CHANNEL_OUTPUT_PARAMETERS { + RingBufferGpadlHandle: request.ring_buffer_gpadl_id.0, + DownstreamRingBufferPageOffset: request.downstream_ring_buffer_page_offset, + NodeNumber: self + .vp_to_physical_node_map + .get_numa_node(request.target_vp), + Padding: 0, + }); let proxy_id = self .proxy @@ -1097,6 +1127,7 @@ async fn proxy_thread( vtl2_server: Option, flush_recv: mesh::Receiver>, await_flush: bool, + vp_to_physical_node_map: Option>, ) { // Separate the hvsocket relay channels. let (hvsock_request_recv, hvsock_response_send) = server @@ -1130,6 +1161,7 @@ async fn proxy_thread( hvsock_response_send, vtl2_hvsock_response_send, Arc::clone(&proxy), + VpToPhysicalNodeMap(vp_to_physical_node_map), )); let offers = task.run_proxy_actions(send, flush_recv, await_flush); let requests = task.run_server_requests( From 8bb9464fa3bcd356d0d9b74042027cf964e56bac Mon Sep 17 00:00:00 2001 From: Will Wright Date: Fri, 12 Dec 2025 13:02:54 -0800 Subject: [PATCH 3/6] Copilot review --- .../vmbus/vmbus_server/src/proxyintegration.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/vm/devices/vmbus/vmbus_server/src/proxyintegration.rs b/vm/devices/vmbus/vmbus_server/src/proxyintegration.rs index 4105ca7940..feb65373f3 100644 --- a/vm/devices/vmbus/vmbus_server/src/proxyintegration.rs +++ b/vm/devices/vmbus/vmbus_server/src/proxyintegration.rs @@ -132,7 +132,8 @@ impl<'a, T: SpawnDriver + Clone> ProxyIntegrationBuilder<'a, T> { self } - /// Adds a NUMA node map to be passed to the proxy driver. + /// Adds a NUMA node map to be passed to the proxy driver. This map is of the format + /// VP -> Physical NUMA Node. For example, `map[0]` is the physical NUMA node for VP 0. pub fn vp_to_physical_node_map(mut self, map: Option>) -> Self { self.vp_to_physical_node_map = map; self @@ -254,12 +255,9 @@ struct VpToPhysicalNodeMap(Option>); impl VpToPhysicalNodeMap { fn get_numa_node(&self, vp_index: u32) -> u16 { - self.0 - .as_ref() - .map_or(0, |nodes| { - nodes.get(vp_index as usize).copied().unwrap_or(0) - }) - .into() + self.0.as_ref().map_or(0, |nodes| { + nodes.get(vp_index as usize).copied().unwrap_or(0) + }) } } From 30058ed27e6833b0e980d9419d3da42fae3a8497 Mon Sep 17 00:00:00 2001 From: Will Wright Date: Tue, 16 Dec 2025 13:17:54 -0800 Subject: [PATCH 4/6] Don't use option for builder map --- vm/devices/vmbus/vmbus_server/src/proxyintegration.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vm/devices/vmbus/vmbus_server/src/proxyintegration.rs b/vm/devices/vmbus/vmbus_server/src/proxyintegration.rs index feb65373f3..7e306af0f5 100644 --- a/vm/devices/vmbus/vmbus_server/src/proxyintegration.rs +++ b/vm/devices/vmbus/vmbus_server/src/proxyintegration.rs @@ -134,8 +134,8 @@ impl<'a, T: SpawnDriver + Clone> ProxyIntegrationBuilder<'a, T> { /// Adds a NUMA node map to be passed to the proxy driver. This map is of the format /// VP -> Physical NUMA Node. For example, `map[0]` is the physical NUMA node for VP 0. - pub fn vp_to_physical_node_map(mut self, map: Option>) -> Self { - self.vp_to_physical_node_map = map; + pub fn vp_to_physical_node_map(mut self, map: Vec) -> Self { + self.vp_to_physical_node_map = Some(map); self } From ffbfacf3a3b42688371798368ed2a40bd7a6ef54 Mon Sep 17 00:00:00 2001 From: Will Wright Date: Wed, 17 Dec 2025 14:54:37 -0800 Subject: [PATCH 5/6] Feedback --- vm/devices/vmbus/vmbus_server/src/proxyintegration.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/vm/devices/vmbus/vmbus_server/src/proxyintegration.rs b/vm/devices/vmbus/vmbus_server/src/proxyintegration.rs index 7e306af0f5..2b4c02ab90 100644 --- a/vm/devices/vmbus/vmbus_server/src/proxyintegration.rs +++ b/vm/devices/vmbus/vmbus_server/src/proxyintegration.rs @@ -251,13 +251,11 @@ impl SavedStatePair { } } -struct VpToPhysicalNodeMap(Option>); +struct VpToPhysicalNodeMap(Vec); impl VpToPhysicalNodeMap { fn get_numa_node(&self, vp_index: u32) -> u16 { - self.0.as_ref().map_or(0, |nodes| { - nodes.get(vp_index as usize).copied().unwrap_or(0) - }) + self.0.get(vp_index as usize).copied().unwrap_or(0) } } @@ -1159,7 +1157,7 @@ async fn proxy_thread( hvsock_response_send, vtl2_hvsock_response_send, Arc::clone(&proxy), - VpToPhysicalNodeMap(vp_to_physical_node_map), + VpToPhysicalNodeMap(vp_to_physical_node_map.unwrap_or_default()), )); let offers = task.run_proxy_actions(send, flush_recv, await_flush); let requests = task.run_server_requests( From cebbbb66af7f8191eccbe774abead1084573522d Mon Sep 17 00:00:00 2001 From: Will Wright Date: Wed, 17 Dec 2025 15:07:17 -0800 Subject: [PATCH 6/6] Remove all usage of Option for the map --- vm/devices/vmbus/vmbus_server/src/proxyintegration.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/vm/devices/vmbus/vmbus_server/src/proxyintegration.rs b/vm/devices/vmbus/vmbus_server/src/proxyintegration.rs index 2b4c02ab90..e7e8eb8fc9 100644 --- a/vm/devices/vmbus/vmbus_server/src/proxyintegration.rs +++ b/vm/devices/vmbus/vmbus_server/src/proxyintegration.rs @@ -110,7 +110,7 @@ pub struct ProxyIntegrationBuilder<'a, T: SpawnDriver + Clone> { vtl2_server: Option, mem: Option<&'a GuestMemory>, require_flush_before_start: bool, - vp_to_physical_node_map: Option>, + vp_to_physical_node_map: Vec, } impl<'a, T: SpawnDriver + Clone> ProxyIntegrationBuilder<'a, T> { @@ -135,7 +135,7 @@ impl<'a, T: SpawnDriver + Clone> ProxyIntegrationBuilder<'a, T> { /// Adds a NUMA node map to be passed to the proxy driver. This map is of the format /// VP -> Physical NUMA Node. For example, `map[0]` is the physical NUMA node for VP 0. pub fn vp_to_physical_node_map(mut self, map: Vec) -> Self { - self.vp_to_physical_node_map = Some(map); + self.vp_to_physical_node_map = map; self } @@ -192,7 +192,7 @@ impl ProxyIntegration { vtl2_server: None, mem: None, require_flush_before_start: false, - vp_to_physical_node_map: None, + vp_to_physical_node_map: vec![], } } @@ -1123,7 +1123,7 @@ async fn proxy_thread( vtl2_server: Option, flush_recv: mesh::Receiver>, await_flush: bool, - vp_to_physical_node_map: Option>, + vp_to_physical_node_map: Vec, ) { // Separate the hvsocket relay channels. let (hvsock_request_recv, hvsock_response_send) = server @@ -1157,7 +1157,7 @@ async fn proxy_thread( hvsock_response_send, vtl2_hvsock_response_send, Arc::clone(&proxy), - VpToPhysicalNodeMap(vp_to_physical_node_map.unwrap_or_default()), + VpToPhysicalNodeMap(vp_to_physical_node_map), )); let offers = task.run_proxy_actions(send, flush_recv, await_flush); let requests = task.run_server_requests(