netvsp: gdma driver report git commit by erfrimod · Pull Request #2963 · microsoft/openvmm

erfrimod · 2026-03-12T23:00:43Z

Soc needs a signal for determining what features are available. During triage, soc logs will provide more information to determine which version of the Gdma driver and OpenHCL was running.

OSVersionString1 set to "OpenHCL"
OSVersionString2 set to git commit
OPENHCL_VERSION split into 4 values and supplied to
- OSVersionMajor
- OSVersionMinor
- OSVersionBuild
- OSVersionPlatform

erfrimod · 2026-03-12T23:02:18Z

Tested on a lab machine. socmana log shows OSVersionString2 set to the git commit cb7c8c705ecec62637a8cef51f068093f7284e38

Built binaries with OPENHCL_VERSION="1.2.3.4" cargo xflowey build-igvm x64

[260313 20:03:42.587626-000,003;2205] [gdma v=f1e024 h=0451] [INFO] [vf=3-2 vtl=2 cl=1 act=0xca930003 msg=0x1 err=0x0 ms=0 vm=ErikUhVm] VerifyVFVersion(292): VFId:2 VFVersion:0x0 OSType:0x60 OSVersionMajor:0x1 OSVersionMinor:0x2 OSVersionBuild:0x3 OSVersionPlatform:0x4 ProtocolMin:0x1 ProtocolMax:0x1 Flags1:0x14068 Flags2:0x0 Flags3:0x0 Flags4:0x0 LinuxManaCapabilityEnforcement:0x0
[260313 20:03:42.587632-000,002;2206] [gdma v=f1e024 h=0451] [INFO] [vf=3-2 vtl=2 cl=1 act=0xca930003 msg=0x1 err=0x0 ms=0 vm=ErikUhVm] VerifyVFVersion(302): VFId:2 VFVersion:0x0 OSType:0x60 OSVersionString1:OpenHCL OSVersionString2:cb7c8c705ecec62637a8cef51f068093f7284e38 OSVersionString3: OSVersionString4: LinuxManaCapabilityEnforcement:0x0

commit cb7c8c705ecec62637a8cef51f068093f7284e38 (HEAD -> erfrimod/mana-driver-report-commit, origin/erfrimod/mana-driver-report-commit)
Author: erfrimod <erfrimod@microsoft.com>
Date:   Fri Mar 13 12:43:23 2026 -0700

Copilot

Pull request overview

This PR adds build-time Git revision information to the MANA GDMA VF driver verification request so that the SoC/HWC side can log which driver build is running during triage.

Changes:

Populate GdmaVerifyVerReq OS/build identity strings (os_ver_str1/os_ver_str2) with a driver name and BUILD_GIT_SHA.
Add a build.rs for mana_driver to emit BUILD_GIT_SHA/BUILD_GIT_BRANCH via build_rs_git_info.
Log the received driver name/commit on the GDMA HWC side when handling GDMA_VERIFY_VF_DRIVER_VERSION.

Reviewed changes

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

Show a summary per file

File	Description
vm/devices/net/mana_driver/src/gdma_driver.rs	Fill verify-version request fields with driver name and Git SHA to report to the SoC.
vm/devices/net/mana_driver/build.rs	Emit Git metadata at build time for use via `option_env!`.
vm/devices/net/mana_driver/Cargo.toml	Add `build_rs_git_info` as a build dependency.
vm/devices/net/gdma/src/hwc.rs	Parse and log the driver name/commit from the verify-version request.
Cargo.lock	Record the new build dependency in the lockfile.

vm/devices/net/gdma/src/hwc.rs

github-actions · 2026-03-13T02:20:24Z

At least one Petri test failed.

sunilmut · 2026-03-13T17:32:18Z

vm/devices/net/mana_driver/src/gdma_driver.rs

    #[tracing::instrument(skip(self), level = "debug", err)]
    pub async fn verify_vf_driver_version(&mut self) -> anyhow::Result<()> {
+        let mut req = GdmaVerifyVerReq {
+            protocol_ver_min: 1,


Lets see if we can plumb in the OPENHCL_VERSION value into protocol_ver_min/max

Copilot

Pull request overview

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

sunilmut · 2026-03-13T20:14:59Z

vm/devices/net/mana_driver/src/gdma_driver.rs


+/// Parse an OPENHCL_VERSION string of the form "major.minor.build.platform"
+/// into four u32 components. Missing or unparseable components default to 0.
+fn parse_openhcl_version(version: &str) -> (u32, u32, u32, u32) {


This belongs better in the build_rs_git_info crate

sunilmut · 2026-03-13T20:20:28Z

vm/devices/net/mana_driver/src/gdma_driver.rs

+    let mut parts = version.splitn(4, '.');
+    let major = parts.next().and_then(|s| s.parse().ok()).unwrap_or(0);
+    let minor = parts.next().and_then(|s| s.parse().ok()).unwrap_or(0);
+    let build = parts.next().and_then(|s| s.parse().ok()).unwrap_or(0);


@justus-camp-microsoft or @maheeraeron - Do you know in the openhcl_version="1.5.10492.0", what do the last two (i.e. 10492.0 in the above example) fields for?

github-actions · 2026-03-13T20:38:21Z

⚠️ 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

Copilot

Pull request overview

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

sunilmut · 2026-03-13T20:55:58Z

vm/devices/net/mana_driver/src/gdma_driver.rs

+        };
+
+        // Identify the driver and build to the SOC
+        // str1 = OS/driver name, str2 = build identity.


Let's also move this to build_info crate. Rest all looks good to me.

sunilmut · 2026-03-13T20:57:29Z

vm/devices/net/gdma/src/hwc.rs

+                    .ok()
+                    .and_then(|c| c.to_str().ok())
+                    .unwrap_or("<invalid>");
+                tracing::debug!(drv_name, drv_commit, "vf driver version");


We can log the other fields here as well and move this to info level as this is good debug info.

sunilmut · 2026-03-13T21:50:07Z

vm/devices/net/mana_driver/src/gdma_driver.rs

+        // Identify the driver and build to the SOC
+        // str1 = "OpenHCL", str2 = build identity.
+        let name = build_info::PRODUCT_NAME.as_bytes();
+        req.os_ver_str1[..name.len()].copy_from_slice(name);


This also needs a min length check like below

Copilot

Pull request overview

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

sunilmut · 2026-03-13T22:31:55Z

vm/devices/net/mana_driver/Cargo.toml

 rust-version.workspace = true

 [dependencies]
+build_info.workspace = true


@justus-camp-microsoft @Brian-Perkins - Will this be a problem for the reproducible build stuff?

I wouldn't think so, we already depend on build_info in other important places

sunilmut · 2026-03-13T22:17:47Z

vm/devices/net/mana_driver/src/gdma_driver.rs

+        req.os_ver_str1[..len].copy_from_slice(&name.as_bytes()[..len]);
+
+        let revision = build_info::get().scm_revision();
+        let len = revision.len().min(req.os_ver_str2.len() - 1);


Generally, for these you want to use saturating_sub, but its fine for now

smalis-msft · 2026-03-16T14:57:27Z

openhcl/build_info/src/lib.rs

+
+// Parse the OPENHCL_VERSION string of the form "major.minor.build.platform"
+// into four u32 components. Missing or unparseable components default to 0.
+pub fn parse_openhcl_version() -> (u32, u32, u32, u32) {


I don't think we should be doing this at runtime, we should do it at build time instead (either providing the ints in env vars or using const fn). That way we can also validate at build time that things match the expected format.

smalis-msft

Convert parsing to a build-time operation, not runtime

sunilmut · 2026-03-16T16:17:52Z

Convert parsing to a build-time operation, not runtime

Is it possible to prevent runtime inclusion of the crate so that this can be prevented in future?

smalis-msft · 2026-03-16T16:19:38Z

I'm not sure what you mean by that. We have to include the crate to call the getters at runtime. I'm just opposed to doing any significant logic at runtime around this data, since it all should be available at build time.

Copilot

Pull request overview

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

vm/devices/net/mana_driver/Cargo.toml

 [dependencies]
+build_info.workspace = true


smalis-msft · 2026-03-18T18:04:27Z

openhcl/build_info/src/lib.rs

+
+static OPENHCL_VERSION: OpenHclVersion = OpenHclVersion::new();
+
+pub fn openhcl_version() -> &'static OpenHclVersion {


nit: I'd get rid of the function and just make the static pub

smalis-msft · 2026-03-18T18:05:24Z

openhcl/build_info/src/lib.rs

+        }
+    }
+
+    pub fn product_name(&self) -> &'static str {


Make all of these getters const too

smalis-msft · 2026-03-18T18:06:30Z

openhcl/build_info/src/lib.rs

+}
+
+impl OpenHclVersion {
+    const VERSION: (u32, u32, u32, u32) = const_parse_version(BuildInfo::new().openhcl_version);


This shouldn't be a separate const, it should get called in new

smalis-msft · 2026-03-18T18:07:33Z

openhcl/build_info/src/lib.rs

+
+// Parse `bytes[start..end]` as a u32. Returns 0 if the segment is empty
+// or contains any non-digit character.
+const fn const_parse_u32_segment(bytes: &[u8], start: usize, end: usize) -> u32 {


i'm surprised std doesn't provide a const way to do this, but this is fine if it doesn't.

I looked a little harder and found u32::from_str_radix!. Doing things in const is hard. :P

smalis-msft · 2026-03-18T20:40:50Z

openhcl/build_info/src/lib.rs

+
+impl OpenHclVersion {
+    pub const fn new() -> Self {
+        let (major, minor, build, platform) = const_parse_version(BuildInfo::new().openhcl_version);


nit: You can just have this reutrn [u32;4] and then have let [major, minor, build, platform] = ... instead of going through a tuple.

smalis-msft · 2026-03-18T20:41:12Z

openhcl/build_info/src/lib.rs

+    }
+    match u32::from_str_radix(s, 10) {
+        Ok(v) => v,
+        Err(_) => 0,


can't we panic in a const context?

Copilot

Pull request overview

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

github-actions · 2026-03-18T23:34:08Z

At least one Petri test failed.

erfrimod requested review from a team as code owners March 12, 2026 23:00

Copilot AI review requested due to automatic review settings March 12, 2026 23:00

Copilot started reviewing on behalf of erfrimod March 12, 2026 23:01 View session

Copilot AI reviewed Mar 12, 2026

View reviewed changes

vm/devices/net/gdma/src/hwc.rs Outdated Show resolved Hide resolved

sunilmut requested changes Mar 13, 2026

View reviewed changes

erfrimod added 2 commits March 13, 2026 11:20

Rebase

f950ef5

adding opehhcl_version

cb7c8c7

Copilot AI review requested due to automatic review settings March 13, 2026 19:43

erfrimod force-pushed the erfrimod/mana-driver-report-commit branch from efd6945 to cb7c8c7 Compare March 13, 2026 19:43

Copilot started reviewing on behalf of erfrimod March 13, 2026 19:44 View session

Copilot AI reviewed Mar 13, 2026

View reviewed changes

sunilmut requested changes Mar 13, 2026

View reviewed changes

Move fn parse_openhcl_version to build_info

81d4f49

github-actions bot added the unsafe Related to unsafe code label Mar 13, 2026

make comment for parse_openhcl_version two slashes instead of three

fa36f5a

Copilot AI review requested due to automatic review settings March 13, 2026 20:42

Copilot started reviewing on behalf of erfrimod March 13, 2026 20:43 View session

Copilot AI reviewed Mar 13, 2026

View reviewed changes

sunilmut requested changes Mar 13, 2026

View reviewed changes

pr feedback

fd7a2de

sunilmut requested changes Mar 13, 2026

View reviewed changes

check the length of product name

92d259a

Copilot AI review requested due to automatic review settings March 13, 2026 22:09

Copilot started reviewing on behalf of erfrimod March 13, 2026 22:09 View session

Copilot AI reviewed Mar 13, 2026

View reviewed changes

sunilmut reviewed Mar 13, 2026

View reviewed changes

sunilmut previously approved these changes Mar 13, 2026

View reviewed changes

saturating sub

be242f7

erfrimod dismissed sunilmut’s stale review via be242f7 March 13, 2026 22:26

sunilmut previously approved these changes Mar 13, 2026

View reviewed changes

smalis-msft reviewed Mar 16, 2026

View reviewed changes

smalis-msft requested changes Mar 16, 2026

View reviewed changes

perform version parsing at build time. add unit tests for logic

888c6f4

Copilot AI review requested due to automatic review settings March 16, 2026 19:29

erfrimod dismissed sunilmut’s stale review via 888c6f4 March 16, 2026 19:29

Copilot started reviewing on behalf of erfrimod March 16, 2026 19:30 View session

Copilot AI reviewed Mar 16, 2026

View reviewed changes

vm/devices/net/mana_driver/Cargo.toml

Comment on lines 9 to +10

[dependencies]

build_info.workspace = true

smalis-msft reviewed Mar 18, 2026

View reviewed changes

openhcl/build_info/src/lib.rs Outdated

}

}

pub fn product_name(&self) -> &'static str {

Copy link

Contributor

smalis-msft Mar 18, 2026

Choose a reason for hiding this comment

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

Make all of these getters const too

smalis-msft reviewed Mar 18, 2026

View reviewed changes

pr feedback

ecf314f

smalis-msft reviewed Mar 18, 2026

View reviewed changes

smalis-msft previously approved these changes Mar 18, 2026

View reviewed changes

enforce version string format at compile

0642e95

Copilot AI review requested due to automatic review settings March 18, 2026 21:34

erfrimod dismissed smalis-msft’s stale review via 0642e95 March 18, 2026 21:34

Copilot AI reviewed Mar 18, 2026

View reviewed changes


		static OPENHCL_VERSION: OpenHclVersion = OpenHclVersion::new();

		pub fn openhcl_version() -> &'static OpenHclVersion {

Conversation

erfrimod commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

erfrimod commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

github-actions bot commented Mar 13, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Mar 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smalis-msft left a comment

Choose a reason for hiding this comment

Uh oh!

sunilmut commented Mar 16, 2026

Uh oh!

smalis-msft commented Mar 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

github-actions bot commented Mar 18, 2026

Uh oh!

erfrimod commented Mar 12, 2026 •

edited

Loading

erfrimod commented Mar 12, 2026 •

edited

Loading