aarch64: make GICv2m SPI count configurable - #4374
Conversation
Extend --gic-msi with v2m,spi_count=N and pass the value through to the existing GicMsiConfig field. This lets device-heavy guests reserve a larger MSI SPI pool without changing the global default or deterministic layout for existing configurations. Keep auto, its, and bare v2m backward compatible and reject zero, duplicate, malformed, or unknown options.
There was a problem hiding this comment.
🟡 Changes recommended
The new CLI behavior should include a clap-level Options::try_parse_from test (aarch64-gated) and the unknown-option error path should report the offending key for actionable diagnostics.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR extends the aarch64 CLI surface to allow configuring the GICv2m MSI SPI pool size via --gic-msi v2m,spi_count=N, plumbing the parsed value through to openvmm_defs::config::GicMsiConfig::V2m { spi_count } while preserving existing defaults and modes.
Changes:
- Add
v2m,spi_count=Nparsing to--gic-msi(rejecting malformed/duplicate/unknown options). - Refactor
openvmm_entryto convertGicMsiCliintoGicMsiConfigviainto_config(). - Add unit coverage for the new
GicMsiClistring parsing.
File summaries
| File | Description |
|---|---|
| openvmm/openvmm_entry/src/lib.rs | Switch to opt.gic_msi.into_config() when building the aarch64 topology config. |
| openvmm/openvmm_entry/src/cli_args.rs | Extend --gic-msi syntax to support v2m,spi_count=N, implement parsing, and add a unit test. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| for option in options.split(',') { | ||
| let count = option | ||
| .strip_prefix("spi_count=") | ||
| .context("expected v2m,spi_count=N")? | ||
| .parse::<u32>() | ||
| .context("spi_count must be a positive integer")?; |
| assert!(GicMsiCli::from_str("v2m,spi_count=0").is_err()); | ||
| assert!(GicMsiCli::from_str("v2m,spi_count=abc").is_err()); | ||
| assert!(GicMsiCli::from_str("v2m,spi_count=64,spi_count=512").is_err()); | ||
| assert!(GicMsiCli::from_str("v2m,unknown=512").is_err()); | ||
| assert!(GicMsiCli::from_str("other").is_err()); |
The GIC MSI option and its production call site are ARM64-only. Gate the CLI type, implementations, and parser test with the same guest architecture cfg so x64 clippy and documentation builds do not report dead code.
|
Fixed the failed x64 clippy/docs checks in 9b0c326. |
There was a problem hiding this comment.
🟢 Approval recommended
The change is localized to CLI parsing/config mapping, preserves existing behavior paths, and includes targeted unit tests for the new parsing logic.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
The additional failure is unrelated to this ARM64-only CLI change: two x64 AMD-SNP TPM guests failed with Hyper-V unsupported-operation triple faults. The same x64 suite passes on sibling ARM64 PRs. GitHub does not permit fork authors to rerun upstream jobs, so 2c4b315 is an empty, tree-identical commit to trigger a clean replacement run. |
Summary
Allow ARM64 command-line users to choose the GICv2m MSI SPI pool size with
--gic-msi v2m,spi_count=N.Changes
auto,its, and barev2m.GicMsiConfigfield.This is an independent follow-up found while validating a device-heavy ARM64 guest for #4344.
Validation
cargo test -p openvmm_entry --lib(88 passed)OPENVMM_GUEST_TARGET=aarch64 cargo check -p openvmm_entrycargo xtask fmt --only-diffed