Skip to content

Conversation

@will-j-wright
Copy link
Contributor

We decided adding more kernel mode code wasn't the way to go, so this change delegates fetching the NUMA configuration to user-mode VMBus. Instead of calling a proxy IOCTL on the task start, we now accept a NUMA node map from user-mode VMBus in the proxy builder, which is passed down into the ProxyTask.

@will-j-wright will-j-wright requested a review from a team as a code owner December 12, 2025 01:54
Copilot AI review requested due to automatic review settings December 12, 2025 01:54
@will-j-wright will-j-wright requested review from a team as code owners December 12, 2025 01:54
@github-actions github-actions bot added the unsafe Related to unsafe code label Dec 12, 2025
@github-actions
Copy link

⚠️ Unsafe Code Detected

This PR modifies files containing unsafe Rust code. Extra scrutiny is required during review.

For more on why we check whole files, instead of just diffs, check out the Rustonomicon

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors how NUMA node configuration is obtained by moving from a kernel-mode IOCTL approach to accepting the configuration from user-mode VMBus. Instead of the proxy integration calling into the kernel driver to fetch NUMA mappings at runtime, the configuration is now provided upfront through the builder pattern and passed into the ProxyTask.

Key Changes

  • The ProxyIntegrationBuilder now accepts an optional VP-to-physical-node map via a new vp_to_physical_node_map() method
  • The VpToPhysicalNodeMap struct changed from wrapping Vec<u8> to Option<Vec<u16>>, with values now being u16 instead of u8
  • Removed the kernel-mode IOCTL (IOCTL_VMBUS_PROXY_GET_NUMA_MAP) and associated get_numa_node_map() method from the VmbusProxy implementation

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
vm/devices/vmbus/vmbus_server/src/proxyintegration.rs Added builder method for NUMA map; changed VpToPhysicalNodeMap to use Option<Vec>; removed IOCTL-based map fetching from proxy_thread; updated field naming from numa_node_map to vp_to_physical_node_map for consistency
vm/devices/vmbus/vmbus_proxy/src/proxyioctl.rs Removed IOCTL constant IOCTL_VMBUS_PROXY_GET_NUMA_MAP and associated struct VMBUS_PROXY_GET_NUMA_MAP_OUTPUT
vm/devices/vmbus/vmbus_proxy/src/lib.rs Removed get_numa_node_map() method and associated imports (headervec, ERROR_MORE_DATA)
vm/devices/vmbus/vmbus_proxy/Cargo.toml Removed headervec dependency which is no longer needed
Cargo.lock Reflects removal of headervec dependency from vmbus_proxy

@github-actions
Copy link

@github-actions
Copy link

Copy link
Member

@SvenGroot SvenGroot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment

vtl2_hvsock_response_send,
Arc::clone(&proxy),
VpToPhysicalNodeMap(vp_to_physical_node_map),
VpToPhysicalNodeMap(vp_to_physical_node_map.unwrap_or_default()),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would propagate this change back, there's no need to use an Option in the builder or anywhere.

@will-j-wright will-j-wright merged commit 1d297a0 into microsoft:main Dec 18, 2025
56 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

unsafe Related to unsafe code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants