vmm_core: decouple chipset MMIO ranges from MemoryLayout by jstarks · Pull Request #3550 · microsoft/openvmm

jstarks · 2026-05-21T22:18:45Z

The chipset MMIO ranges (low below 4 GiB, high above RAM) are a logical concept of the chipset configuration, but they were previously read by positional indexing into MemoryLayout::mmio(). This was fragile: it required exactly two MMIO entries, scattered assertions and error paths enforced that invariant, and the coupling prevented evolving the memory layout's MMIO accounting independently of the chipset's view of MMIO.

Introduce a ChipsetMmioRanges struct that carries the chipset low, high, and VTL2 MMIO ranges explicitly, and thread it through the loader, ACPI builder, DSDT construction, MTRR computation, firmware_pcat, and IGVM/UEFI/Linux boot paths. This removes all the mmio().len() >= 2 assertions and error variants, since the ranges are now provided directly rather than reverse-engineered from the memory layout.

As a drive-by, the VTL2 framebuffer GPA allocation moves into the memory layout resolver so it participates in the same non-overlapping placement as other post-MMIO ranges.

smalis-msft · 2026-05-21T22:20:38Z

MTRRs mentioned

oh no, here we go again

Copilot

Pull request overview

This PR refactors how chipset MMIO ranges are represented and consumed across OpenVMM/OpenHCL boot flows by introducing an explicit ChipsetMmioRanges concept instead of relying on positional indexing into MemoryLayout::mmio(). It also moves VTL2 framebuffer GPA placement into the memory layout resolver so it is allocated alongside other post-MMIO ranges.

Changes:

Introduce/propagate explicit chipset MMIO range plumbing (low/high/VTL2) through loaders, ACPI/DSDT generation, firmware config, and MTRR computation.
Remove/relax prior “exactly two MMIO gaps” assertions and error paths by using resolved chipset MMIO ranges directly.
Allocate the VTL2 framebuffer GPA via the memory layout resolver (PostMmio placement) instead of ad-hoc scanning.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
vmm_core/src/acpi_builder.rs	Simplifies the DSDT callback interface to no longer pass `MemoryLayout`.
vm/loader/src/common.rs	Updates x86 variable MTRR computation to take explicit low/high chipset MMIO ranges.
vm/devices/firmware/firmware_pcat/src/lib.rs	PCAT config now carries explicit chipset low/high MMIO ranges; runtime reads use those ranges.
vm/devices/firmware/firmware_pcat/Cargo.toml	Promotes `memory_range` from dev-dependency to dependency to support new config fields.
vm/acpi/src/dsdt.rs	Uses `MemoryRange::len()` when generating QWORD memory resources.
openvmm/openvmm_core/src/worker/vm_loaders/uefi.rs	UEFI loader consumes `ChipsetMmioRanges` for UEFI config MMIO ranges.
openvmm/openvmm_core/src/worker/vm_loaders/linux.rs	ARM64 DT build path consumes explicit chipset low/high MMIO ranges for VMBus ranges.
openvmm/openvmm_core/src/worker/vm_loaders/igvm.rs	Consolidates chipset MMIO parameters into a single `ChipsetMmioRanges` struct for IGVM load paths.
openvmm/openvmm_core/src/worker/memory_layout.rs	Introduces `ChipsetMmioRanges` in the resolver output and adds PostMmio allocation for VTL2 framebuffer GPA.
openvmm/openvmm_core/src/worker/dispatch.rs	Threads resolved chipset MMIO ranges through firmware loaders and DSDT construction; switches MTRR computation callsite to new signature.
openhcl/underhill_core/src/worker.rs	Constructs chipset MMIO ranges from boot-info MMIO list and threads them into firmware + MTRR setup and other consumers.
openhcl/underhill_core/src/loader/mod.rs	Threads chipset MMIO ranges into ACPI DSDT build and UEFI config blob generation.

smalis-msft · 2026-05-22T00:08:17Z

+            // No high MMIO gap — RAM above the low gap is contiguous.
+            // Cover to the 8TB boundary (or GPA space limit) using the
+            // same split as the >64GB path below.
+            result.push(X86Register::MtrrPhysMask2(mtrr_mask(


Changing this code at all makes me nervous given that we no longer have all the extra debug checks compiled into the kernel for the MTRR test...

github-actions · 2026-05-22T00:10:31Z

1 Petri tests failed (0 unstable)

The chipset MMIO ranges (low below 4 GiB, high above RAM) are a logical concept of the chipset configuration, but they were previously read by positional indexing into MemoryLayout::mmio(). This was fragile: it required exactly two MMIO entries, scattered assertions and error paths enforced that invariant, and the coupling prevented evolving the memory layout's MMIO accounting independently of the chipset's view of MMIO. Introduce a ChipsetMmioRanges struct that carries the chipset low, high, and VTL2 MMIO ranges explicitly, and thread it through the loader, ACPI builder, DSDT construction, MTRR computation, firmware_pcat, and IGVM/UEFI/Linux boot paths. This removes all the mmio().len() >= 2 assertions and error variants, since the ranges are now provided directly rather than reverse-engineered from the memory layout. As a drive-by, the VTL2 framebuffer GPA allocation moves into the memory layout resolver so it participates in the same non-overlapping placement as other post-MMIO ranges, and build_acpi_tables's closure no longer receives a &MemoryLayout it didn't need.

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

+        vmod_crs.add_resource(&QwordMemory::new(low.start(), low.len()));
+        vmod_crs.add_resource(&QwordMemory::new(high.start(), high.len()));


-        crs.add_resource(&QwordMemory::new(low.start(), low.end() - low.start()));
-        crs.add_resource(&QwordMemory::new(high.start(), high.end() - high.start()));
+        crs.add_resource(&QwordMemory::new(low.start(), low.len()));
+        crs.add_resource(&QwordMemory::new(high.start(), high.len()));


        result.push(X86Register::MtrrPhysBase1(pcat_mtrr_size | WRITEBACK));
        result.push(X86Register::MtrrPhysMask1(mtrr_mask(
            gpa_space_size,
-            mmio_gap_low.start() - 1,
+            chipset_low_mmio.start() - 1,
        )));
    }


+    // VTL2 framebuffer: a page-aligned PostMmio allocation so the GPA does
+    // not overlap RAM or any MMIO range.
+    let mut vtl2_framebuffer_range = MemoryRange::EMPTY;
+    if input.vtl2_framebuffer_size != 0 {
+        builder.request(
+            "vtl2-framebuffer",
+            &mut vtl2_framebuffer_range,
+            input.vtl2_framebuffer_size,
+            PAGE_SIZE,
+            Placement::PostMmio,
+        );
+    }
+


Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

-            return Err(PcatBiosDeviceInitError::IncorrectMmioHoles(
-                config.mem_layout.mmio().len(),
-            ));
+        if config.chipset_low_mmio.is_empty() || config.chipset_high_mmio.is_empty() {


    if memory.end_of_ram() > pcat_mtrr_size {
        result.push(X86Register::MtrrPhysBase1(pcat_mtrr_size | WRITEBACK));
        result.push(X86Register::MtrrPhysMask1(mtrr_mask(
            gpa_space_size,
-            mmio_gap_low.start() - 1,
+            chipset_low_mmio.start() - 1,
        )));


+    // Construct chipset MMIO ranges from the positional convention in the
+    // device tree: [0] = low (below 4 GiB), [1] = high (above RAM).
+    let mut chipset_mmio = ChipsetMmioRanges {
+        low: boot_info
+            .vtl0_mmio
+            .first()
+            .copied()
+            .unwrap_or(MemoryRange::EMPTY),
+        high: boot_info
+            .vtl0_mmio
+            .get(1)
+            .copied()
+            .unwrap_or(MemoryRange::EMPTY),
+    };


github-actions · 2026-05-23T21:45:38Z

2 Petri tests failed (1 unstable)

github-actions · 2026-05-24T03:40:33Z

1 Petri tests failed (1 unstable)

Copilot AI review requested due to automatic review settings May 21, 2026 22:18

Copilot started reviewing on behalf of jstarks May 21, 2026 22:18 View session

jstarks marked this pull request as ready for review May 21, 2026 22:19

jstarks requested a review from a team as a code owner May 21, 2026 22:19

Copilot AI reviewed May 21, 2026

View reviewed changes

Comment thread vm/loader/src/common.rs Outdated

Comment thread vm/devices/firmware/firmware_pcat/src/lib.rs

Comment thread openvmm/openvmm_core/src/worker/dispatch.rs Outdated

smalis-msft approved these changes May 21, 2026

View reviewed changes

smalis-msft reviewed May 22, 2026

View reviewed changes

smalis-msft approved these changes May 22, 2026

View reviewed changes

jstarks added 2 commits May 23, 2026 12:43

tweaks

928797d

Copilot AI review requested due to automatic review settings May 23, 2026 19:46

jstarks force-pushed the no_mmio branch from 2574d92 to 928797d Compare May 23, 2026 19:46

Copilot started reviewing on behalf of jstarks May 23, 2026 19:46 View session

Copilot AI reviewed May 23, 2026

View reviewed changes

feedback

505c089

jstarks requested a review from Copilot May 23, 2026 20:43

Copilot started reviewing on behalf of jstarks May 23, 2026 20:43 View session

Copilot AI reviewed May 23, 2026

View reviewed changes

cleanup

6da9cdb

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

vmm_core: decouple chipset MMIO ranges from MemoryLayout#3550

vmm_core: decouple chipset MMIO ranges from MemoryLayout#3550
jstarks wants to merge 4 commits into
microsoft:mainfrom
jstarks:no_mmio

jstarks commented May 21, 2026

Uh oh!

smalis-msft commented May 21, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

smalis-msft May 22, 2026

Uh oh!

github-actions Bot commented May 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

github-actions Bot commented May 23, 2026

Uh oh!

github-actions Bot commented May 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		vmod_crs.add_resource(&QwordMemory::new(low.start(), low.len()));
		vmod_crs.add_resource(&QwordMemory::new(high.start(), high.len()));

Conversation

jstarks commented May 21, 2026

Uh oh!

smalis-msft commented May 21, 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!

Uh oh!

Uh oh!

smalis-msft May 22, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

github-actions Bot commented May 23, 2026

Uh oh!

github-actions Bot commented May 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

smalis-msft commented May 21, 2026 •

edited

Loading