Add vmservice processor topology overrides

[Justin (jterry75)]

Adds support to the VmService ttrpc interface when creating the config to override the processor topology arch specific defaults.

[Copilot]

Pull request overview

This PR extends the OpenVMM VmService (ttrpc) configuration surface to allow clients to override architecture-specific processor topology defaults when creating a VM, with initial support focused on aarch64 GIC/PMU/MSI topology knobs.

Changes:

• Adds new arch-specific topology override messages/oneofs under ProcessorConfig in the vmservice ttrpc protobuf.
• Implements aarch64 override parsing in openvmm_entry and wires the parsed arch topology into ProcessorTopologyConfig.arch.

Reviewed changes

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

File	Description
openvmm/openvmm_ttrpc_vmservice/src/vmservice.proto	Adds new arch-specific processor topology override schema (aarch64 GIC/PMU/MSI; x86 placeholder).
openvmm/openvmm_entry/src/ttrpc/mod.rs	Parses aarch64 topology override proto fields into openvmm_defs config and passes them into VM creation config.

Suppressed comments (1)

openvmm/openvmm_ttrpc_vmservice/src/vmservice.proto:158

• Typo in the aarch64 MSI controller override oneof name: gic_mis_config reads like mis rather than msi. This will leak into generated types (e.g., GicMisConfig) and is hard to change later, so it’s worth renaming to gic_msi_config now (keeping the same field numbers).

    // If left unset, automatically select the best available MSI controller:
    // - `GicMsiItsConfig` when the hypervisor supports it,
    // - otherwise `GicMsiV2mConfig`.
    oneof gic_mis_config {
        // Force GICv3 ITS for MSI delivery via LPIs.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

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

Suppressed comments (3)

openvmm/openvmm_entry/src/ttrpc/mod.rs:138

• Aarch64TopologyConfig uses GicConfig::{V2,V3}(None) to mean “use defaults for that version’s addresses”. When the request selects gic_v2/gic_v3 but doesn’t provide any optional base overrides, this code currently forces Some(Gic*Config { ...defaults... }), which loses that intent and bakes defaults into the ttrpc layer unnecessarily.

    let gic_config = match cfg.gic_config {
        Some(ProtoGicConfig::GicV2(v2)) => Some(GicConfig::V2(Some(GicV2Config {
            gic_distributor_base: v2
                .gic_distributor_base
                .unwrap_or(openvmm_defs::config::DEFAULT_GIC_DISTRIBUTOR_BASE),
            cpu_interface_base: v2
                .cpu_interface_base
                .unwrap_or(openvmm_defs::config::DEFAULT_GIC_REDISTRIBUTORS_BASE),
        }))),

openvmm/openvmm_ttrpc_vmservice/src/vmservice.proto:157

• Typo in the public proto API: gic_mis_config reads like “mis” but all related types/use-sites are “MSI” (GicMsi*, GicMsiConfig). Since this is a new field name, it’s better to correct it now (keeping field numbers the same) to avoid a permanent generated-binding typo.

    oneof gic_mis_config {

openvmm/openvmm_entry/src/ttrpc/mod.rs:128

• This uses the generated GicMisConfig/gic_mis_config identifiers, which appear to come from a typo in the proto (mis vs msi). If the proto is corrected to gic_msi_config, update the generated enum name and field access here to keep the Rust API consistent with GicMsiConfig.

This issue also appears on line 130 of the same file.

    use vmservice::processor_aarch64_config::GicConfig as ProtoGicConfig;
    use vmservice::processor_aarch64_config::GicMisConfig as ProtoGicMsiConfig;
    use vmservice::processor_aarch64_config::PmuGsivConfig as ProtoPmuGsivConfig;

[Copilot]

Pull request overview

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

Suppressed comments (4)

openvmm/openvmm_entry/src/ttrpc/mod.rs:184

• On aarch64 builds, mapping a request’s arch_config = x86 into ArchTopologyConfig::X86 will later fail in topology building with a generic "invalid architecture config" error. It’s safer to reject or ignore the mismatched config here (ideally with a clear warning/error).

        Some(ProtoArchConfig::X86(_)) => Some(ArchTopologyConfig::X86(Default::default())),

openvmm/openvmm_entry/src/ttrpc/mod.rs:197

• On non-aarch64 builds this currently ignores all arch_config overrides silently. At minimum, it should accept the matching x86 variant (even if it’s currently empty) and warn when an aarch64 variant is provided, to avoid surprising no-op behavior for clients.

fn parse_arch_topology_overrides(
    _processor_cfg: Option<&vmservice::ProcessorConfig>,
) -> Option<openvmm_defs::config::ArchTopologyConfig> {
    None
}

openvmm/openvmm_ttrpc_vmservice/src/vmservice.proto:157

• The oneof name gic_mis_config looks like a typo (the surrounding comments and message names use MSI). Since this is part of the public proto API and will be reflected in generated bindings, it’s worth renaming to gic_msi_config now (field numbers are unchanged).

    oneof gic_mis_config {

openvmm/openvmm_entry/src/ttrpc/mod.rs:162

• This field name mirrors the proto gic_mis_config typo; after renaming the proto oneof to gic_msi_config, this access will also need to switch to cfg.gic_msi_config to match the regenerated bindings.

This issue also appears in the following locations of the same file:

• line 184
• line 193

    let gic_msi = match cfg.gic_mis_config {

[github-actions]

4 Petri tests failed (0 unstable)

[Copilot]

Pull request overview

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

Suppressed comments (1)

openvmm/openvmm_ttrpc_vmservice/src/vmservice.proto:148

• Using a bool disabled inside a oneof creates an invalid-but-representable state (disabled = false) which then has to be rejected server-side (as the Rust parser already does). For a public proto API, consider making this a presence-only option (e.g., replace the bool with an empty message like message PmuDisabled {} / PmuGsivDisabled {} in the oneof, or use an enum) so clients can't accidentally send a nonsensical value.

    oneof pmu_gsiv_config {
        // Use the specified GSIV value for the PMU.
        uint32 gsiv_value = 3;
        // Disable the PMU.
        bool disabled = 4;
    }

[Copilot]

Pull request overview

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

Suppressed comments (2)

openvmm/openvmm_entry/src/ttrpc/mod.rs:134

• parse_aarch64_topology_overrides eagerly materializes default GIC base addresses even when the proto override message leaves all address fields unset. This duplicates the defaulting logic and can drift if defaults change; it also prevents using the None inner config path that already means “use defaults for this GIC version”. Consider emitting GicConfig::{V2,V3}(None) when the corresponding proto message contains no address overrides, and only filling defaults when a partial override is provided.

    let gic_config = match cfg.gic_config {
        Some(pc::GicConfig::GicV2(v2)) => Some(dc::GicConfig::V2(Some(dc::GicV2Config {
            gic_distributor_base: v2
                .gic_distributor_base
                .unwrap_or(dc::DEFAULT_GIC_DISTRIBUTOR_BASE),
            cpu_interface_base: v2
                .cpu_interface_base
                .unwrap_or(dc::DEFAULT_GIC_REDISTRIBUTORS_BASE),
        }))),
        Some(pc::GicConfig::GicV3(v3)) => Some(dc::GicConfig::V3(Some(dc::GicV3Config {
            gic_distributor_base: v3
                .gic_distributor_base
                .unwrap_or(dc::DEFAULT_GIC_DISTRIBUTOR_BASE),
            gic_redistributors_base: v3
                .gic_redistributors_base
                .unwrap_or(dc::DEFAULT_GIC_REDISTRIBUTORS_BASE),
        }))),
        None => None,
    };

openvmm/openvmm_entry/src/ttrpc/mod.rs:955

• New processor topology override parsing is added here (and then threaded into ProcessorTopologyConfig.arch), but there are no unit tests exercising the new parsing paths (e.g., aarch64: GIC v2/v3 selection, PMU GSIV enable/disable, MSI controller selection). Since this file already has a mod tests with parsing-focused coverage, adding targeted tests would help prevent regressions and ensure the ttrpc surface matches openvmm_defs::config semantics.

        let arch = parse_arch_topology_overrides(req_config.processor_config.as_ref())?;

[Copilot]

Pull request overview

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

Suppressed comments (3)

openvmm/openvmm_entry/src/ttrpc/mod.rs:140

• Same move-out-of-borrow issue for pmu_gsiv_config: cfg.pmu_gsiv_config is a non-Copy oneof field, so matching on it by value from &cfg won’t compile. Use as_ref() and dereference the scalar as needed.

    let pmu_gsiv = match cfg.pmu_gsiv_config {
        Some(pc::PmuGsivConfig::GsivValue(gsiv)) => dc::PmuGsivConfig::Gsiv(gsiv),
        Some(pc::PmuGsivConfig::Disabled(_)) => dc::PmuGsivConfig::Disabled,
        None => dc::PmuGsivConfig::Platform,
    };

openvmm/openvmm_entry/src/ttrpc/mod.rs:146

• Same move-out-of-borrow issue for gic_msi_config: matching on cfg.gic_msi_config by value moves the oneof out of a borrowed protobuf struct. Match on cfg.gic_msi_config.as_ref() instead.

    let gic_msi = match cfg.gic_msi_config {
        Some(pc::GicMsiConfig::MsiIts(_)) => dc::GicMsiConfig::Its,
        Some(pc::GicMsiConfig::MsiV2m(v2m)) => dc::GicMsiConfig::V2m {
            spi_count: v2m.spi_count,
        },

openvmm/openvmm_entry/src/ttrpc/mod.rs:178

• parse_arch_topology_overrides matches on Option<&ArchConfig>, so aarch64 is already a reference; passing &aarch64 creates an unnecessary &&T (and can trigger clippy’s needless_borrow). Pass aarch64 directly.

        Some(ProtoArchConfig::Aarch64(aarch64)) => {
            Ok(Some(parse_aarch64_topology_overrides(&aarch64)?))
        }

[github-actions]

33 Petri tests failed (4 unstable)

[Copilot]

Pull request overview

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

Suppressed comments (2)

openvmm/openvmm_ttrpc_vmservice/src/vmservice.proto:121

• This blank line contains trailing whitespace, which may fail formatting checks and makes diffs noisier. Prefer an empty line with no spaces.
  openvmm/openvmm_entry/src/ttrpc/mod.rs:134
• The gic_config parser always builds GicConfig::{V2,V3}(Some(...)) with defaulted addresses, even when the request didn't set any address override fields. That defeats the GicConfig contract where a None inner config means "use defaults" and makes it harder to distinguish "force version" from "override addresses" in future logic/inspection output. Consider emitting GicConfig::V2(None)/V3(None) when all address fields are unset, and only constructing the inner config when at least one address override is present.

    let gic_config = match &cfg.gic_config {
        Some(pc::GicConfig::GicV2(v2)) => Some(dc::GicConfig::V2(Some(dc::GicV2Config {
            gic_distributor_base: v2
                .gic_distributor_base
                .unwrap_or(dc::DEFAULT_GIC_DISTRIBUTOR_BASE),

[github-actions]

42 Petri tests failed (4 unstable)

[Copilot]

Pull request overview

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

Suppressed comments (2)

openvmm/openvmm_entry/src/ttrpc/mod.rs:132

• GicConfig::{V2,V3}(None) is documented as “use defaults for that version's addresses” (openvmm_defs::config::GicConfig). Here, selecting gic_v2/gic_v3 always materializes a Some(GicV{2,3}Config{...}) with default constants when the proto address fields are unset, which effectively turns “no address overrides” into “explicitly set to current defaults” (freezing defaults if the config is persisted/serialized). Consider preserving None for the inner config unless at least one address override field is present.

        Some(pc::GicConfig::GicV2(v2)) => Some(dc::GicConfig::V2(Some(dc::GicV2Config {
            gic_distributor_base: v2
                .gic_distributor_base
                .unwrap_or(dc::DEFAULT_GIC_DISTRIBUTOR_BASE),
            cpu_interface_base: v2

openvmm/openvmm_entry/src/ttrpc/mod.rs:160

• New parsing logic for ProcessorConfig.arch_config / aarch64 topology overrides is introduced here, but the existing unit tests in this module only cover VFIO BAR parsing. Adding unit tests for parse_arch_topology_overrides (and, on aarch64 builds, parse_aarch64_topology_overrides) would help lock down the proto->config mappings for gic_config/pmu_gsiv_config/gic_msi_config.

fn parse_arch_topology_overrides(
    processor_cfg: Option<&vmservice::ProcessorConfig>,
) -> Result<Option<ArchTopologyConfig>, anyhow::Error> {
    use vmservice::processor_config::ArchConfig as ProtoArchConfig;

[github-actions]

11 Petri tests failed (0 unstable)