Skip to content

vfio_assigned_device: allow fixed BAR addresses#4001

Open
jstarks wants to merge 5 commits into
microsoft:mainfrom
jstarks:vfio-explicit-bar-address
Open

vfio_assigned_device: allow fixed BAR addresses#4001
jstarks wants to merge 5 commits into
microsoft:mainfrom
jstarks:vfio-explicit-bar-address

Conversation

@jstarks

@jstarks jstarks commented Jul 22, 2026

Copy link
Copy Markdown
Member

The nvgrace-gpu VFIO variant driver synthesizes a coherent-memory BAR whose physical address is not reported through the PCI sysfs resource table. OpenVMM therefore cannot discover that BAR's host address, even though it must be placed at the same guest address for peer-to-peer DMA on platforms where transactions bypass IOMMU translation.

Allow each VFIO BAR to use either the host-reported address or an explicit fixed address. Carry this policy through the CLI, management service, and VFIO resource boundary, and validate fixed addresses against the discovered BAR layout before programming them.

This replaces the opaque barN=pt CLI syntax with barN=host; the old spelling is no longer accepted. The management schema gains equivalent structured support for host-derived and fixed BAR addresses.

@jstarks
jstarks requested a review from a team as a code owner July 22, 2026 14:05
Copilot AI review requested due to automatic review settings July 22, 2026 14:05
@github-actions github-actions Bot added the Guide label Jul 22, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Enables VFIO-assigned devices to pre-program PCI BARs using either host-reported physical BAR addresses or explicit fixed physical addresses, propagating this configuration through the CLI, ttrpc management schema, and VFIO resource handles. This supports devices (e.g. nvgrace GPU variant drivers) whose coherent-memory BAR physical address is not discoverable via sysfs resource.

Changes:

  • Replace per-BAR passthrough booleans (barN=pt) with a structured per-BAR address policy (GuestAssigned / HostAssigned / Fixed(u64)), and update CLI syntax to barN=host or barN=0x<addr>.
  • Extend the ttrpc management API schema with VfioBarAddress entries and parse them into the resource handle config.
  • Validate and apply configured BAR addresses against discovered BAR layout (including address/size alignment), and update Guide documentation.

Reviewed changes

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

Show a summary per file
File Description
vm/devices/pci/vfio_assigned_device/src/resolver.rs Threads the renamed BAR configuration field through resolver paths.
vm/devices/pci/vfio_assigned_device/src/lib.rs Implements BarAddressConfig-driven BAR pre-programming + validation and adds unit tests.
vm/devices/pci/vfio_assigned_device_resources/src/lib.rs Introduces BarAddressConfig and updates VFIO resource handle structs to carry it.
openvmm/openvmm_ttrpc_vmservice/src/vmservice.proto Adds VfioBarAddress and bar_addresses to the management API schema.
openvmm/openvmm_entry/src/ttrpc/mod.rs Parses proto BAR address entries into BarAddressConfig for resource construction.
openvmm/openvmm_entry/src/lib.rs Plumbs CLI-parsed BAR config into VFIO resource handles.
openvmm/openvmm_entry/src/cli_args.rs Updates --vfio parsing to accept barN=host and barN=0x<addr> and adds tests.
Guide/src/user_guide/openvmm/vfio.md Documents updated --vfio BAR pinning syntax and the fixed-address use case.
Guide/src/reference/openvmm/management/cli.md Updates CLI reference example from bar0=pt to bar0=host.

Comment thread vm/devices/pci/vfio_assigned_device/src/lib.rs
Comment thread vm/devices/pci/vfio_assigned_device_resources/src/lib.rs
Copilot AI review requested due to automatic review settings July 22, 2026 14:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comments suppressed due to low confidence (1)

vm/devices/pci/vfio_assigned_device/src/lib.rs:838

  • apply_bar_addresses indexes bar_masks[i + 1] and later writes bars[i + 1] whenever is_64bit is true, but the function itself doesn't guard i + 1 < 6. Today the caller rejects a 64-bit BAR at index 5, but keeping the bounds check local avoids a potential panic if this helper is reused or if the pre-validation changes.
        let is_64bit = cfg_space::BarEncodingBits::from(bar_flags[i]).type_64_bit();
        if !is_64bit && address > u32::MAX as u64 {
            anyhow::bail!("BAR {i} is 32-bit but address {address:#x} exceeds 4 GB");
        }
        let address_mask = if is_64bit {

Comment thread vm/devices/pci/vfio_assigned_device/src/lib.rs Outdated
jstarks added 3 commits July 22, 2026 16:25
The nvgrace-gpu VFIO variant driver synthesizes a coherent-memory BAR whose
physical address is not reported through the PCI sysfs resource table. OpenVMM
therefore cannot discover that BAR's host address, even though it must be placed
at the same guest address for peer-to-peer DMA on platforms where transactions
bypass IOMMU translation.

Allow each VFIO BAR to use either the host-reported address or an explicit fixed
address. Carry this policy through the CLI, management service, and VFIO
resource boundary, and validate fixed addresses against the discovered BAR
layout before programming them.

This replaces the opaque `barN=pt` CLI syntax with `barN=host`; the old spelling
is no longer accepted. The management schema gains equivalent structured
support for host-derived and fixed BAR addresses.
Copilot AI review requested due to automatic review settings July 22, 2026 16:26
@jstarks
jstarks force-pushed the vfio-explicit-bar-address branch from 4a530af to 61e40a1 Compare July 22, 2026 16:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Comments suppressed due to low confidence (1)

vm/devices/pci/vfio_assigned_device/src/lib.rs:846

  • apply_bar_addresses indexes bar_masks[i + 1] / writes bars[i + 1] for 64-bit BARs without first ensuring i + 1 < 6. If the device reports a (malformed) 64-bit BAR at index 5 and the user pins it, this will panic with an out-of-bounds access. Please reject 64-bit BARs that start at BAR5 before using i + 1.
        let is_64bit = cfg_space::BarEncodingBits::from(bar_flags[i]).type_64_bit();
        if !is_64bit && address > u32::MAX as u64 {
            anyhow::bail!("BAR {i} is 32-bit but address {address:#x} exceeds 4 GB");
        }
        let address_mask = if is_64bit {
            (bar_masks[i + 1] as u64) << 32 | (bar_masks[i] as u64 & !0xf)
        } else {

Comment thread vm/devices/pci/vfio_assigned_device/src/lib.rs Outdated
Comment thread vm/devices/pci/vfio_assigned_device/src/lib.rs Outdated
Copilot AI review requested due to automatic review settings July 23, 2026 05:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Copilot AI review requested due to automatic review settings July 23, 2026 08:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

@github-actions

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants