Skip to content

Fix VMGenID's HID to align with upstream Linux kernel#5760

Merged
zulinx86 merged 2 commits into
firecracker-microvm:mainfrom
zulinx86:sysgenid_test
Mar 23, 2026
Merged

Fix VMGenID's HID to align with upstream Linux kernel#5760
zulinx86 merged 2 commits into
firecracker-microvm:mainfrom
zulinx86:sysgenid_test

Conversation

@zulinx86

@zulinx86 zulinx86 commented Mar 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Changes & Reason

fix(vmgenid): Align HID with upstream

Currently, Firecracker sets "FCVMGID" as HID (Hardware ID) and
"VM_Gen_Counter" as CID (Compatbile ID).  Linux kernel [1] specifies
VMGENCTR and VM_GEN_COUNTER as ACPI IDs to bind the driver to the
VMGenID device.

If the VMGenID driver is implemented as ACPI driver, only HID is checked
to match the ACPI IDs.  On the other hand, if implemented as platform
driver, Linux kernel checks whether HID or CID matches the ACPI IDs.
Linux kernel 6.10 [2] re-implemented it from ACPI driver to platform
driver in order to support devcie tree.  As a result, prior to Linux
kernel 6.10, the driver isn't bound correctly.

We didn't see any issue due to HID mismatch, because we backported the
above kernel patches to our 6.1 guest kernel [3].  VMGenID itself is
only supported since upstream Linux kernel 5.18+ [4] and we don't test
VMGenID on our 5.10 guest kernel.

[1]: https://elixir.bootlin.com/linux/v6.19.7/source/drivers/virt/vmgenid.c#L162-L163
[2]: https://github.com/torvalds/linux/commit/e07606713a908eee8099883c8c830ebe33aaf748
[3]: https://github.com/firecracker-microvm/firecracker/blob/81236d82b1640cfa41f825f50be5585c758e165b/docs/snapshotting/random-for-clones.md?plain=1#L132-L139
[4]: https://github.com/torvalds/linux/commit/af6b54e2b5baa54c844573b6d49cc91157bcdd7e

In addition, add integration tests for SysGenID.

License Acceptance

By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following Developer
Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md.

PR Checklist

  • I have read and understand CONTRIBUTING.md.
  • I have run tools/devtool checkbuild --all to verify that the PR passes
    build checks on all supported architectures.
  • I have run tools/devtool checkstyle to verify that the PR passes the
    automated style checks.
  • I have described what is done in these changes, why they are needed, and
    how they are solving the problem in a clear and encompassing way.
  • [na] I have updated any relevant documentation (both in code and in the docs)
    in the PR.
  • [na] I have mentioned all user-facing changes in CHANGELOG.md.
  • [na] If a specific issue led to this PR, this PR closes the issue.
  • [na] When making API changes, I have followed the
    Runbook for Firecracker API changes.
  • I have tested all new and changed functionalities in unit tests and/or
    integration tests.
  • [na] I have linked an issue to every new TODO.
  • This functionality cannot be added in rust-vmm.

@codecov

codecov Bot commented Mar 13, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.02%. Comparing base (40c67b2) to head (e185e6b).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #5760   +/-   ##
=======================================
  Coverage   83.02%   83.02%           
=======================================
  Files         276      276           
  Lines       29340    29340           
=======================================
  Hits        24359    24359           
  Misses       4981     4981
Flag Coverage Δ
5.10-m5n.metal 83.30% <100.00%> (ø)
5.10-m6a.metal 82.62% <100.00%> (ø)
5.10-m6g.metal 80.02% <0.00%> (ø)
5.10-m6i.metal 83.30% <100.00%> (ø)
5.10-m7a.metal-48xl 82.62% <100.00%> (ø)
5.10-m7g.metal 80.02% <0.00%> (ø)
5.10-m7i.metal-24xl 83.27% <100.00%> (-0.01%) ⬇️
5.10-m7i.metal-48xl 83.27% <100.00%> (-0.01%) ⬇️
5.10-m8g.metal-24xl 80.02% <0.00%> (-0.01%) ⬇️
5.10-m8g.metal-48xl 80.02% <0.00%> (ø)
5.10-m8i.metal-48xl 83.27% <100.00%> (-0.01%) ⬇️
5.10-m8i.metal-96xl 83.27% <100.00%> (-0.01%) ⬇️
6.1-m5n.metal 83.33% <100.00%> (ø)
6.1-m6a.metal 82.65% <100.00%> (ø)
6.1-m6g.metal 80.01% <0.00%> (-0.01%) ⬇️
6.1-m6i.metal 83.33% <100.00%> (+<0.01%) ⬆️
6.1-m7a.metal-48xl 82.64% <100.00%> (ø)
6.1-m7g.metal 80.02% <0.00%> (-0.01%) ⬇️
6.1-m7i.metal-24xl 83.33% <100.00%> (-0.01%) ⬇️
6.1-m7i.metal-48xl 83.34% <100.00%> (-0.01%) ⬇️
6.1-m8g.metal-24xl 80.01% <0.00%> (-0.01%) ⬇️
6.1-m8g.metal-48xl 80.02% <0.00%> (ø)
6.1-m8i.metal-48xl 83.35% <100.00%> (+<0.01%) ⬆️
6.1-m8i.metal-96xl 83.34% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@zulinx86 zulinx86 enabled auto-merge (rebase) March 16, 2026 08:18
@zulinx86 zulinx86 added Status: Awaiting author Indicates that an issue or pull request requires author action Status: Awaiting review Indicates that a pull request is ready to be reviewed and removed Status: Awaiting author Indicates that an issue or pull request requires author action labels Mar 16, 2026
@zulinx86 zulinx86 force-pushed the sysgenid_test branch 2 times, most recently from 780a797 to b57d216 Compare March 16, 2026 08:54
JackThomson2
JackThomson2 reviewed Mar 16, 2026
View reviewed changes
Comment thread tests/host_tools/sysgenid.c Outdated
Manciukic
Manciukic previously approved these changes Mar 16, 2026
View reviewed changes
JackThomson2
JackThomson2 previously approved these changes Mar 16, 2026
View reviewed changes
zulinx86 added 2 commits March 20, 2026 09:11
@zulinx86
fix(vmgenid): Align HID with upstream
e8819ab 
Currently, Firecracker sets "FCVMGID" as HID (Hardware ID) and
"VM_Gen_Counter" as CID (Compatbile ID).  Linux kernel [1] specifies
"VMGENCTR" and "VM_GEN_COUNTER" as ACPI IDs to bind the driver to the
VMGenID device.

If the VMGenID driver is implemented as platform driver, Linux kernel
checks whether either HID or CID matches the ACPI IDs.  On the other
hand, if implemented as ACPI driver, only HID is checked for the match.
Linux kernel 6.10 [2] re-implemented it from ACPI driver to platform
driver in order to support devcie tree.  As a result, prior to Linux
kernel 6.10, the driver isn't bound correctly.

We didn't see any issue due to HID mismatch, because we backported the
above kernel patches to our 6.1 guest kernel [3].  VMGenID itself is
only supported since upstream Linux kernel 5.18+ [4] and we don't test
VMGenID on our 5.10 guest kernel.

Note that Amazon Linux-provided microVM kernel 5.10 [5][6] actually
implements VMGenID driver but it is a downstream implementation.  It
is never used by customers (instead SysGenID is used) and it specifies
yet another set of ACPI IDs ("VMGENID" and "QEMUVMGID").  So, more
precisely, that is why we don't test VMGenID on our 5.10 guest kernel.

[1]: https://elixir.bootlin.com/linux/v6.19.7/source/drivers/virt/vmgenid.c#L162-L163
[2]: torvalds/linux@e076067
[3]: https://github.com/firecracker-microvm/firecracker/blob/81236d82b1640cfa41f825f50be5585c758e165b/docs/snapshotting/random-for-clones.md?plain=1#L132-L139
[4]: torvalds/linux@af6b54e
[5]: https://github.com/amazonlinux/linux/blob/microvm-kernel-5.10.245-268.975.amzn2/drivers/virt/vmgenid.c#L121-L122
[6]: amazonlinux/linux@c9b81dc

Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
@zulinx86
test(sysgenid): Add integration tests for SysGenID
e185e6b 
Although Firecracker doesn't do anything for SysGenID, it is used for
the snapsafety issue by guest userspace in Amazon Linux-provided microVM
kernels [1][2].  Let's test them to detect functional regression that
might be introduced in microVM kernels.

[1]: https://github.com/amazonlinux/linux/blob/microvm-kernel-5.10.245-268.975.amzn2/drivers/misc/sysgenid.c
[2]: https://github.com/amazonlinux/linux/blob/microvm-kernel-6.1.164-23.303.amzn2023/drivers/misc/sysgenid.c
Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
@zulinx86 zulinx86 dismissed stale reviews from JackThomson2 and Manciukic via e185e6b March 20, 2026 09:12
@zulinx86 zulinx86 requested a review from Manciukic March 20, 2026 10:54
@zulinx86 zulinx86 merged commit 80ab750 into firecracker-microvm:main Mar 23, 2026
7 checks passed
@zulinx86 zulinx86 deleted the sysgenid_test branch March 23, 2026 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Manciukic Manciukic Manciukic approved these changes

@JackThomson2 JackThomson2 JackThomson2 approved these changes

@kalyazin kalyazin Awaiting requested review from kalyazin

@pb8o pb8o Awaiting requested review from pb8o

Assignees

No one assigned

Labels

Status: Awaiting review Indicates that a pull request is ready to be reviewed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@zulinx86 @Manciukic @JackThomson2