Add Hyper-V IDE controller support to chipset resources by moor-coding · Pull Request #3353 · microsoft/openvmm

moor-coding · 2026-04-22T14:07:43Z

Migrates the Hyper-V IDE controller from direct construction in base_chipset_devices_and_manifest! to the resource-handle + resolver pattern.

IDE disks flow through pci_chipset_devices as a LegacyPciChipsetDeviceHandle with unresolved IdeDeviceConfig, removing ide_disks from Config/Manifest and eliminating workers' direct dependency on IDE/SCSI resolution
with_ide capability flag replaces with_hyperv_ide manifest boolean for PCI legacy interrupt registration

Depends on #3344

github-actions · 2026-04-22T14:08:05Z

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

This PR migrates the Hyper-V Gen1 IDE controller from direct construction in the base chipset builder to the repository’s resource-handle + resolver pattern, and threads IDE disks through pci_chipset_devices / VMBus resources instead of Config/Manifest fields.

Changes:

Add HyperVIdeDeviceHandle to chipset_resources and introduce an ide resolver (HyperVIdeResolver) registered in resource registries.
Add a storvsp IDE accelerator resource (StorvspIdeDeviceHandle) and resolver (StorvspIdeResolver) and plumb accelerator handles from config builders.
Extend VFIO support with sparse-mmap capability parsing and direct BAR sub-region GPA mappings; add PKCS#7 authenticated-variable crypto fixtures + Windows backend.

Reviewed changes

Copilot reviewed 40 out of 43 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
vmm_core/vmotherboard/src/base_chipset.rs	Removes direct Hyper-V IDE construction/deps and adds `VmChipsetCapabilities::with_ide`.
vmm_core/vmotherboard/Cargo.toml	Drops `ide` dependency and removes `encryption` feature forwarding.
vmm_core/vm_manifest_builder/src/lib.rs	Initializes `with_ide` capability and removes `with_hyperv_ide` manifest flag usage.
vm/vmcore/memory_range/src/lib.rs	Adds `MemoryRange::bounding_aligned` helper used for alignment-aware range ops.
vm/devices/user_driver/vfio_sys/src/lib.rs	Adds host page size helper + sparse mmap cap parsing and `region_mmap_areas()`.
vm/devices/user_driver/vfio_sys/Cargo.toml	Adds `memory_range` + `headervec` deps for sparse mmap parsing.
vm/devices/storage/storvsp_resources/src/lib.rs	Introduces `StorvspIdeDeviceHandle` resource.
vm/devices/storage/storvsp/src/resolver.rs	Adds `StorvspIdeResolver` to resolve IDE accelerator handles via storvsp.
vm/devices/storage/ide/src/resolver.rs	New `HyperVIdeResolver` that builds the IDE controller from `HyperVIdeDeviceHandle`.
vm/devices/storage/ide/src/lib.rs	Exposes the new `resolver` module.
vm/devices/storage/ide/Cargo.toml	Adds deps required for IDE resource resolution (`vm_resource`, `chipset_resources`, etc.).
vm/devices/pci/vfio_assigned_device/src/resolver.rs	Requires `shared_mem_mapper` to construct VFIO assigned devices.
vm/devices/pci/vfio_assigned_device/src/lib.rs	Adds direct BAR sub-region guest mappings and MSI-X region exclusion.
vm/devices/pci/vfio_assigned_device/Cargo.toml	Adds `memory_range` dependency.
vm/devices/firmware/firmware_uefi/src/service/nvram/spec_services/test_data/auth_var_pkcs7.der	Adds PKCS#7 detached signature fixture (binary).
vm/devices/firmware/firmware_uefi/src/service/nvram/spec_services/test_data/auth_var_cert.der	Adds X.509 cert fixture (binary).
vm/devices/firmware/firmware_uefi/src/service/nvram/spec_services/test_data/README.md	Documents fixture purpose and regeneration steps.
vm/devices/firmware/firmware_uefi/src/service/nvram/spec_services/mod.rs	Switches auth-var crypto gating to OS-based cfg and updates error plumbing.
vm/devices/firmware/firmware_uefi/src/service/nvram/spec_services/auth_var_crypto.rs	Switches cfg gating and adds unit tests for PKCS#7 auth-var verification.
vm/devices/firmware/firmware_uefi/fuzz/Cargo.toml	Removes now-deleted `auth-var-verify-crypto` feature usage for fuzzing.
vm/devices/firmware/firmware_uefi/Cargo.toml	Makes `crypto`/`der` non-optional and removes `auth-var-verify-crypto` feature.
vm/devices/chipset_resources/src/lib.rs	Adds `chipset_resources::ide` module with `HyperVIdeDeviceHandle` + BDF constant.
vm/devices/chipset_resources/Cargo.toml	Adds `ide_resources` dependency for IDE resource config payloads.
support/crypto/src/pkcs7/win.rs	Adds Windows CryptoAPI backend for PKCS#7 verification.
support/crypto/src/pkcs7/ossl.rs	Updates OpenSSL backend verify semantics (consume `self`) and improves docs/comments.
support/crypto/src/pkcs7/mod.rs	Wires Windows backend and expands/clarifies `uefi_mode` verification semantics.
support/crypto/src/lib.rs	Enables pkcs7 on Windows/Linux and tweaks Windows backend error wording.
support/crypto/Cargo.toml	Removes unused `tracing` dep after error-logging changes.
petri/src/vm/openvmm/construct.rs	Routes IDE disks via PCI chipset device handles and adds storvsp IDE accelerator handles.
openvmm/openvmm_resources/src/lib.rs	Registers the new IDE and storvsp IDE resolvers.
openvmm/openvmm_resources/Cargo.toml	Adds `ide` dependency so the resolver is available.
openvmm/openvmm_entry/src/storage_builder.rs	Builds IDE controller handle + storvsp IDE accelerator handles from CLI configuration.
openvmm/openvmm_entry/Cargo.toml	Drops `encryption` feature forwarding (feature no longer exists).
openvmm/openvmm_core/src/worker/dispatch.rs	Removes direct IDE disk resolution and uses `chipset_capabilities.with_ide` for IRQ registration.
openvmm/openvmm_core/Cargo.toml	Drops IDE/scsi dependencies that were only needed for direct IDE resolution.
openvmm/openvmm/Cargo.toml	Drops `encryption` feature forwarding.
openhcl/underhill_core/src/worker.rs	Moves IDE + IDE-accelerator construction to resource handles/resolvers.
openhcl/underhill_core/src/dispatch/mod.rs	Removes stored `_ide_accel_devices` field (accelerators now go through standard VMBus device list).
openhcl/underhill_core/Cargo.toml	Drops direct `ide`/`storvsp` deps and adds `chipset_resources` for IDE handle construction.
openhcl/openvmm_hcl_resources/src/lib.rs	Registers IDE resolver and storvsp IDE resolver for OpenVMM-HCL.
openhcl/openvmm_hcl_resources/Cargo.toml	Adds `ide` dependency so the resolver is available.
flowey/flowey_lib_hvlite/src/_jobs/check_clippy.rs	Updates comment wording around crypto cross-compilation constraints.
Cargo.lock	Updates lockfile for new deps and removed features/deps.

Copilot · 2026-04-22T14:17:26Z

+            // Use the kernel's returned argsz rather than our pre-computed
+            // value, in case it differs.
+            let actual_tail = buf.head.argsz as usize - size_of::<vfio_region_info>();
+            // SAFETY: The kernel initialized the tail bytes via the ioctl.
+            unsafe { buf.set_tail_len(actual_tail.min(tail_len)) };
+            if let Some(areas) = parse_sparse_mmap_caps(&buf) {


actual_tail is computed with buf.head.argsz as usize - size_of::<vfio_region_info>(). If the kernel ever returns an argsz smaller than the header size, this underflows to a huge usize, and set_tail_len(actual_tail.min(tail_len)) would mark uninitialized bytes as initialized (UB) before parsing the capability chain. Please use checked_sub/saturating_sub and also clamp buf.head.argsz to buf_size before calling set_tail_len.

Copilot · 2026-04-22T14:17:26Z

+# PKCS#7 auth-var test fixtures
+
+Fixtures consumed by unit tests in `auth_var_crypto.rs` to exercise the PKCS#7
+signed-data verification path against both the OpenSSL (unix) and WinCrypt
+(windows) backends.
+
+`auth_var_cert.der` is a self-signed RSA-2048 X.509 certificate (DER).
+`auth_var_pkcs7.der` is a detached PKCS#7 signature (DER) covering the byte
+concatenation:


These fixtures are .der binary files; without a .gitattributes entry marking *.der as -text/binary, git's text=auto can apply EOL normalization and corrupt them. Please add a .gitattributes rule for *.der (or at least for this directory) so the committed bytes remain stable across platforms.

Copilot · 2026-04-22T14:17:27Z

-        let mut ide_drives = [[None, None], [None, None]];
-        let mut storvsp_ide_disks = Vec::new();
-        if cfg.chipset.with_hyperv_ide {
+        if cfg.chipset_capabilities.with_ide {


with_ide is used here to register legacy IRQs 14/15, but the Hyper-V IDE controller itself is only added when IDE disks are configured (e.g. openvmm_entry::StorageBuilder / petri / underhill push HyperVIdeDeviceHandle only when the disks list is non-empty). This can leave the IRQs registered even when no IDE controller exists, and it also changes prior Gen1 behavior where the IDE controller was constructed even with no attached disks. Consider deriving this condition from the actual presence of the hyperv-ide PCI device handle, or always creating the IDE controller handle for Gen1 with an empty disks list to preserve behavior.

Suggested change

if cfg.chipset_capabilities.with_ide {

let has_hyperv_ide = cfg

.pci_devices

.iter()

.any(|device| device.kind() == "hyperv-ide");

if has_hyperv_ide {

Copilot · 2026-04-22T14:17:27Z

+        if !ide_disks.is_empty() {
+            use chipset_resources::LEGACY_CHIPSET_PCI_BUS_NAME;
+            use chipset_resources::ide::HYPERV_IDE_BDF;
+            use chipset_resources::ide::HyperVIdeDeviceHandle;
+            use vmotherboard::LegacyPciChipsetDeviceHandle;
+
+            config
+                .pci_chipset_devices
+                .push(LegacyPciChipsetDeviceHandle {
+                    name: "hyperv-ide".to_string(),
+                    resource: HyperVIdeDeviceHandle { disks: ide_disks }.into_resource(),
+                    pci_bus_name: LEGACY_CHIPSET_PCI_BUS_NAME.to_string(),
+                    bdf: HYPERV_IDE_BDF,
+                });
+        }


This only adds the Hyper-V IDE controller handle when there are IDE disks. Previously (Gen1) the IDE controller was created whenever the chipset had Hyper-V IDE enabled, even if no drives were attached (empty drive arrays). If the expectation is that Gen1 always exposes an IDE controller, consider always pushing HyperVIdeDeviceHandle { disks: ide_disks } (even when empty) and letting the resolver attach zero drives, or ensure chipset_capabilities.with_ide is only set/used when the controller handle is present.

Suggested change

if !ide_disks.is_empty() {

use chipset_resources::LEGACY_CHIPSET_PCI_BUS_NAME;

use chipset_resources::ide::HYPERV_IDE_BDF;

use chipset_resources::ide::HyperVIdeDeviceHandle;

use vmotherboard::LegacyPciChipsetDeviceHandle;

config

.pci_chipset_devices

.push(LegacyPciChipsetDeviceHandle {

name: "hyperv-ide".to_string(),

resource: HyperVIdeDeviceHandle { disks: ide_disks }.into_resource(),

pci_bus_name: LEGACY_CHIPSET_PCI_BUS_NAME.to_string(),

bdf: HYPERV_IDE_BDF,

});

}

use chipset_resources::LEGACY_CHIPSET_PCI_BUS_NAME;

use chipset_resources::ide::HYPERV_IDE_BDF;

use chipset_resources::ide::HyperVIdeDeviceHandle;

use vmotherboard::LegacyPciChipsetDeviceHandle;

config

.pci_chipset_devices

.push(LegacyPciChipsetDeviceHandle {

name: "hyperv-ide".to_string(),

resource: HyperVIdeDeviceHandle { disks: ide_disks }.into_resource(),

pci_bus_name: LEGACY_CHIPSET_PCI_BUS_NAME.to_string(),

bdf: HYPERV_IDE_BDF,

});

Copilot · 2026-04-22T14:17:27Z

+    #[cfg(any(windows, target_os = "linux"))]
    async fn authenticate_var(
        &mut self,
-        // NOTE: Due to a compiler limitation with async fn, 'static bound was removed here
-        // https://github.com/rust-lang/rust/issues/63033#issuecomment-521234696
        (key_var_name, key_var_vendor): (Guid, &Ucs2LeSlice),
        auth_var: ParsedAuthVar<'_>,
    ) -> Result<(), (EfiStatus, Option<NvramError>)> {


Within this crypto-enabled authenticate_var implementation, the error handling path for key_var_error() currently panics when the stored signature list is malformed/corrupt. Since signature-list contents come from NVRAM (guest-influenced), this panic can crash the VMM across a trust boundary; please handle it as a validation failure (e.g. return SECURITY_VIOLATION with context) instead of panicking.

Copilot · 2026-04-22T14:17:28Z

+                // safe for cloning.
+                let is_hard_disk = matches!(disk_cfg.guest_media, GuestMedia::Disk { .. });
+                let serialized = mesh::resource::SerializedMessage::from_message(disk_cfg);
+                assert!(


Avoid panicking on potentially untrusted VTL2 settings input: this assert!(serialized.resources.is_empty(), ...) can crash OpenHCL if the settings ever include OS resources. Prefer validating and returning an error (e.g. anyhow::ensure! + bail!) so malformed input is handled gracefully across the trust boundary.

Suggested change

assert!(

anyhow::ensure!(

Copilot · 2026-04-22T14:17:28Z

+        // Add the IDE device handle if there are any IDE disks.
+        if !ide_disks.is_empty() {
+            use chipset_resources::LEGACY_CHIPSET_PCI_BUS_NAME;
+            use chipset_resources::ide::HYPERV_IDE_BDF;
+            use chipset_resources::ide::HyperVIdeDeviceHandle;
+            use vmotherboard::LegacyPciChipsetDeviceHandle;
+
+            pci_chipset_devices.push(LegacyPciChipsetDeviceHandle {
+                name: "hyperv-ide".to_string(),
+                resource: HyperVIdeDeviceHandle { disks: ide_disks }.into_resource(),
+                pci_bus_name: LEGACY_CHIPSET_PCI_BUS_NAME.to_string(),
+                bdf: HYPERV_IDE_BDF,
+            });
+        }


This only adds the HyperVIdeDeviceHandle when IDE disks exist. Prior Gen1 behavior constructed the IDE controller even with no attached disks, so tests that boot Gen1 without IDE devices may now lose the IDE controller entirely. Consider always adding the IDE controller handle for Gen1 (with an empty disks list) if the chipset is expected to expose IDE regardless of attached drives, or otherwise ensure all consumers treat with_ide as 'requested+present' rather than 'capable'.

Copilot · 2026-04-22T14:17:28Z

+    // Push the IDE device handle for the chipset builder resolver.
+    let mut pci_chipset_devices = pci_chipset_devices;
+    if !ide_handle_disks.is_empty() {
+        use chipset_resources::LEGACY_CHIPSET_PCI_BUS_NAME;
+        use chipset_resources::ide::HYPERV_IDE_BDF;
+        use chipset_resources::ide::HyperVIdeDeviceHandle;
+        use vmotherboard::LegacyPciChipsetDeviceHandle;
+
+        pci_chipset_devices.push(LegacyPciChipsetDeviceHandle {
+            name: "hyperv-ide".to_string(),
+            resource: HyperVIdeDeviceHandle {
+                disks: ide_handle_disks,
+            }
+            .into_resource(),
+            pci_bus_name: LEGACY_CHIPSET_PCI_BUS_NAME.to_string(),
+            bdf: HYPERV_IDE_BDF,
+        });
+    }


This only adds the Hyper-V IDE controller handle when ide_handle_disks is non-empty. Previously, Gen1/PCAT would construct the IDE controller whenever enabled, even with no attached drives. If the Gen1 chipset is expected to always expose IDE, consider pushing HyperVIdeDeviceHandle { disks: Vec::new() } as well so the controller exists even when empty, or adjust any with_ide/interrupt-registration logic to reflect actual presence of the controller.

Copilot · 2026-04-22T14:17:29Z

+/// Returns the host page size.
+pub fn host_page_size() -> u64 {
+    // SAFETY: sysconf(_SC_PAGESIZE) is always safe and always succeeds on Linux.
+    unsafe { libc::sysconf(libc::_SC_PAGESIZE) as u64 }
+}


host_page_size() casts sysconf(_SC_PAGESIZE) directly to u64. sysconf can return -1 on error, which would become a huge u64 and can later trigger panics/overflows when used as an alignment. Please check for <= 0 and return an error (or at least anyhow::ensure!(page_size > 0) like map_dma does) instead of assuming success.

- Introduce StorvspIdeDeviceHandle in storvsp_resources with a stable resource ID, mapping IDE channel/device addressing to SCSI path/target. - Add StorvspIdeResolver in storvsp that resolves the handle into a StorageDevice::build_ide() call via the standard VMBus device resolution path. - Remove direct storvsp::StorageDevice::build_ide() construction from dispatch.rs and worker.rs; IDE accelerator channels are now resolved as VMBus device handles alongside other vmbus_devices. - Remove storvsp crate dependency from underhill_core (no longer directly referenced).

- Validate IDE channel_id/device_id (0..=1) in StorvspIdeResolver - Replace assert! with anyhow::ensure! for untrusted VTL2 config in Underhill - Add missing .await on disk_open() in storage_builder

- IDE disks flow through pci_chipset_devices as a LegacyPciChipsetDeviceHandle with unresolved IdeDeviceConfig, removing ide_disks from Config/Manifest and eliminating workers' direct dependency on IDE/SCSI resolution - with_ide capability flag replaces with_hyperv_ide manifest boolean for PCI legacy interrupt registration

Copilot AI review requested due to automatic review settings April 22, 2026 14:07

github-actions Bot added the unsafe Related to unsafe code label Apr 22, 2026

Copilot started reviewing on behalf of moor-coding April 22, 2026 14:08 View session

Copilot AI reviewed Apr 22, 2026

View reviewed changes

moor-coding added 3 commits April 22, 2026 16:49

Address PR review feedback for storvsp IDE resourcify

73d2848

- Validate IDE channel_id/device_id (0..=1) in StorvspIdeResolver - Replace assert! with anyhow::ensure! for untrusted VTL2 config in Underhill - Add missing .await on disk_open() in storage_builder

moor-coding force-pushed the hyperv-ide-resourcify branch from 79ccd16 to 97fbde5 Compare April 22, 2026 17:26

github-actions Bot removed the unsafe Related to unsafe code label Apr 22, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Hyper-V IDE controller support to chipset resources#3353

Add Hyper-V IDE controller support to chipset resources#3353
moor-coding wants to merge 3 commits intomicrosoft:mainfrom
moor-coding:hyperv-ide-resourcify

moor-coding commented Apr 22, 2026

Uh oh!

github-actions Bot commented Apr 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 22, 2026

Uh oh!

Copilot AI Apr 22, 2026

Uh oh!

Copilot AI Apr 22, 2026

Uh oh!

Copilot AI Apr 22, 2026

Uh oh!

Copilot AI Apr 22, 2026

Uh oh!

Copilot AI Apr 22, 2026

Uh oh!

Copilot AI Apr 22, 2026

Uh oh!

Copilot AI Apr 22, 2026

Uh oh!

Copilot AI Apr 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-        if cfg.chipset_capabilities.with_ide {
+        let has_hyperv_ide = cfg
+            .pci_devices
+            .iter()
+            .any(|device| device.kind() == "hyperv-ide");
+        if has_hyperv_ide {

Conversation

moor-coding commented Apr 22, 2026

Uh oh!

github-actions Bot commented Apr 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants