Skip to content

vmbus_server: don't use monitor info if interrupts are disabled#3579

Open
SvenGroot wants to merge 2 commits into
microsoft:mainfrom
SvenGroot:vmbus/mnf_interrupt_disabled_fix
Open

vmbus_server: don't use monitor info if interrupts are disabled#3579
SvenGroot wants to merge 2 commits into
microsoft:mainfrom
SvenGroot:vmbus/mnf_interrupt_disabled_fix

Conversation

@SvenGroot
Copy link
Copy Markdown
Member

This change fixes an issue where channels with MNF enabled were supplied with information about the MNF usage when opened, even when the channel has interrupts disabled. This was unnecessary, as the MNF support set up for the channel would not be used anyway, but it would also cause the check in ServerTaskInner::set_monitor_page to fail during the handling of a ModifyConnection message because it believes a channel is using MNF when it really isn't.

This affects situations when the server is used to host a VM using OpenHCL with the VMBus relay, and a guest which is using synthetic kdnet. KD opens the networking channel (which is often configured to use MNF) as a reserved channel, but the OpenHCL relay forwards these to the host as a regular, non-reserved open. Reserved channels are explicitly excluded from using MNF, but because the channel is not reserved on the host, that didn't apply here. When the relay uses ModifyChannel to change connection parameters as the VTL0 guest reconnects, this would fail causing guest boot failure.

Because reserved channels don't use interrupts, the relay still forwards them with their interrupt target VP set to VP_INDEX_DISABLE_INTERRUPT. This change therefore removes the monitor info from the channel's OpenParams if interrupts are disabled, which fixes this problem.

Copilot AI review requested due to automatic review settings May 27, 2026 21:37
@SvenGroot SvenGroot requested review from a team as code owners May 27, 2026 21:37
Copy link
Copy Markdown
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

Fixes a bug where MNF-enabled channels opened with interrupts disabled (target VP = VP_INDEX_DISABLE_INTERRUPT) still carried monitor info in their OpenParams, causing later ModifyConnection handling to incorrectly reject monitor page changes. This breaks synthetic kdnet scenarios through the OpenHCL VMBus relay, where reserved-on-guest channels are forwarded as non-reserved opens with disabled interrupts.

Changes:

  • In OpenParams::from_request, suppress monitor_info whenever request.target_vp is None (interrupts disabled).
  • Add test_mnf_channel_open_with_interrupts_disabled covering the new behavior.

Reviewed changes

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

File Description
vm/devices/vmbus/vmbus_server/src/channels.rs Gate OpenParams.monitor_info on request.target_vp.is_some()
vm/devices/vmbus/vmbus_server/src/channels/tests.rs New test verifying MNF channel opened with interrupts disabled has no monitor info

event_flag,
monitor_info,
// Only include monitor info if the request has interrupts enabled.
monitor_info: request.target_vp.and(monitor_info),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice, this is a much simpler change

@github-actions
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants