Skip to content

Conversation

hargar19
Copy link
Collaborator

@hargar19 hargar19 commented May 21, 2025

The connection-id determines which hypervisor communication channel the guest should use to talk to the VMBus host. Get this value from device tree instead of kernel boot arguments.

Below data shows already existing property under the vmbus device tree node with the compatible id "microsoft,message-connection-id":

cd /sys/firmware/devicetree/base/bus/vmbus

/sys/firmware/devicetree/base/bus/vmbus # hexdump microsoft,message-connection-i
d
0000000 8000 7400
0000004

connection-id value from dmesg logs after the code change:
0.630151] hv_vmbus: VMBus message connection ID: 8388724

The hexdump shows: 00 80 00 74
0x00800074 = 8388724 (decimal)

The connection-id determines which hypervisor
communication channel the guest should use to
talk to the VMBus host.

Signed-off-by: Hardik Garg <[email protected]>
@hargar19 hargar19 requested a review from Copilot May 21, 2025 02:57
Copy link

@Copilot 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 updates the VMBus driver to source the connection ID from the device tree rather than using a module parameter, ensuring consistency with device configuration.

  • Updated vmbus_drv.c to read the connection ID property from the device tree.
  • Revised vmbus_connection handling in connection.c by removing the module parameter and adding default logic.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
drivers/hv/vmbus_drv.c Added code to read and log the connection ID from the device tree.
drivers/hv/connection.c Removed outdated module parameter and updated default connection ID assignment logic.

@hargar19 hargar19 marked this pull request as ready for review May 21, 2025 15:49
@hargar19 hargar19 requested review from dcui and saurabh-sengar May 21, 2025 15:50
@hargar19
Copy link
Collaborator Author

@hargar19 hargar19 changed the title get connection-id from device tree (DO NOT MERGE: WILL UPSTREAM THIS CHANGE FIRST) get connection-id from device tree May 28, 2025
@romank-msft
Copy link
Contributor

@hargar19 we should merge this. Without it, this commit microsoft/openvmm#1501 breaks guests with VMBus relay due to

    // If we're isolated we can't trust the host-provided cmdline
    if can_trust_host {
        let old_cmdline = &partition_info.cmdline;

        // HACK: See if we should set the vmbus connection id via kernel
        // commandline. It may already be set, and we don't want to set it again.
        //
        // This code will be removed when the kernel supports setting connection id
        // via device tree.
        if !old_cmdline.contains("hv_vmbus.message_connection_id=") {
            write!(
                cmdline,
                "hv_vmbus.message_connection_id=0x{:x} ",
                partition_info.vmbus_vtl2.connection_id
            )?;
        }

        // Prepend the computed parameters to the original command line.
        cmdline.write_str(old_cmdline)?;
    }
``` in the bootshim.

@hargar19
Copy link
Collaborator Author

hargar19 commented Jul 1, 2025

@romank-msft I have sent this patch upstream https://lore.kernel.org/all/SN6PR02MB4157F2C0674B85C7206E6047D47CA@SN6PR02MB4157.namprd02.prod.outlook.com/
Once the patch is accepted, we will upgrade the HCL kernel to include it. This PR will be discarded.

stunes-ms added a commit to microsoft/openvmm that referenced this pull request Jul 2, 2025
Release backport of PR #1501 and PR #1627.

If the debug bit is set in the VM's TDX attributes, the host can be
trusted. This change gets the TD report in the boot shim and checks the
debug bit. If it's set, parse the dynamic command line to allow
enabling, e.g., confidential debugging.

Note that microsoft/OHCL-Linux-Kernel#79 will
need to be resolved before hardware debug can be disabled in the
manifest.
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.

3 participants