openvmm_entry/ttrpc: expose PCIe NUMA and ACS config#4009
Draft
jstarks wants to merge 6 commits into
Draft
Conversation
GB200-style device topologies need the management interface to describe PCIe devices as ACPI Generic Initiators and control the ACS capabilities advertised by individual ports. Extend the VM service schema and topology conversion so these settings reach the canonical OpenVMM configuration. Use pipette in the ttrpc integration test to verify the resulting SRAT Generic Initiator entry and ACS capability directly from inside the guest.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the OpenVMM ttrpc VM service schema and its openvmm_entry conversion layer to support (1) PCIe NUMA “Generic Initiator” descriptions and (2) per-port ACS capability advertisement, and adds a VMM integration test that validates both behaviors from inside the guest using pipette.
Changes:
- Extend
vmservice.protowithPcieGenericInitiatorandPciePort.acs_capabilities_supported. - Plumb the new schema fields through
openvmm_entrytopology conversion into the canonicalopenvmm_defs::config. - Enhance the ttrpc VMM test to inject pipette into the initrd and validate SRAT GI affinity + ACS capability in-guest.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| vmm_tests/vmm_tests/tests/tests/ttrpc.rs | Injects pipette into the guest initrd, enables hvsocket for one VM instance, configures ACS + GI, and validates SRAT/PCIe config from inside the guest. |
| vmm_tests/vmm_tests/Cargo.toml | Adds initrd_cpio dependency for initrd injection in the test. |
| openvmm/openvmm_ttrpc_vmservice/src/vmservice.proto | Adds schema support for PCIe Generic Initiators and per-port ACS capability configuration. |
| openvmm/openvmm_entry/src/ttrpc/mod.rs | Threads new PCIe schema fields into openvmm_defs::config via a flattened BuiltPcieTopology. |
| Cargo.lock | Locks the new initrd_cpio dependency. |
Comments suppressed due to low confidence (2)
vmm_tests/vmm_tests/tests/tests/ttrpc.rs:866
- This
unwrap()can panic if the slice length is wrong; prefer propagating a contextual error so failures are reported cleanly (especially when debugging guest PCI config parsing).
config
.get(capability_offset..capability_offset + 4)
.context("invalid PCIe extended capability offset")?
.try_into()
.unwrap(),
vmm_tests/vmm_tests/tests/tests/ttrpc.rs:885
- This
unwrap()can panic on unexpected config layout. Usingcontext(...)?here will keep the test failure in the anyhow error path with a clearer message.
config
.get(acs_offset + 4..acs_offset + 6)
.context("parent port has no ACS capability register")?
.try_into()
.unwrap(),
Comment on lines
+826
to
+832
| let proximity_domain = | ||
| u32::from_le_bytes(srat[offset + 4..offset + 8].try_into().unwrap()); | ||
| let entry_segment = | ||
| u16::from_le_bytes(srat[offset + 8..offset + 10].try_into().unwrap()); | ||
| let entry_bus = srat[offset + 10]; | ||
| let entry_devfn = srat[offset + 11]; | ||
| let flags = u32::from_le_bytes(srat[offset + 24..offset + 28].try_into().unwrap()); |
| message PcieTopologyConfig { | ||
| // The PCIe root complexes in the VM. | ||
| repeated PcieRootComplex root_complexes = 1; | ||
| // Devices that act as initiators for device-only NUMA nodes. |
Comment on lines
+62
to
+65
| let pipette_initrd = initrd_cpio::inject_into_initrd(&initrd, "pipette", &pipette, 0o100755) | ||
| .context("failed to inject pipette into test initrd")?; | ||
| let mut pipette_initrd_file = tempfile::NamedTempFile::new()?; | ||
| pipette_initrd_file.write_all(&pipette_initrd)?; |
Comment on lines
+535
to
+547
| if let Some(listener) = pipette_listener { | ||
| let mut listener = PolledSocket::new(&driver, listener)?; | ||
| let (conn, _) = listener.accept().await?; | ||
| let conn = PolledSocket::new(&driver, conn)?; | ||
| let agent = pipette_client::PipetteClient::new( | ||
| &driver, | ||
| conn, | ||
| params.logger.output_dir(), | ||
| ) | ||
| .await?; | ||
| validate_pcie_config(&agent).await?; | ||
| agent.power_off().await?; | ||
| } |
jstarks
force-pushed
the
ttrpc-generic-initiator-acs
branch
from
July 24, 2026 11:11
d806e1c to
09f26e6
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
vmm_tests/vmm_tests/tests/tests/ttrpc.rs:539
listener.accept().awaithas no timeout, so if the guest fails to start pipette (or HvSocket wiring breaks) this test can hang indefinitely in CI. Add an explicit deadline around the accept and fail with a clear error message on timeout.
if let Some(listener) = pipette_listener {
let mut listener = PolledSocket::new(&driver, listener)?;
let (conn, _) = listener.accept().await?;
let conn = PolledSocket::new(&driver, conn)?;
let agent = pipette_client::PipetteClient::new(
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
vmm_tests/vmm_tests/tests/tests/ttrpc.rs:66
pipette_initrd_fileis created withNamedTempFile::new(), which places it in the global temp directory rather than undertempdir. This contradicts the comment above that all temporary files for the test live under a single temp directory, and also makes it harder to correlate artifacts when debugging failures.
Create the file inside tempdir with NamedTempFile::new_in(tempdir.path()) (or update the comment if the intent is to allow temp files elsewhere).
let pipette_initrd = initrd_cpio::inject_into_initrd(&initrd, "pipette", &pipette, 0o100755)
.context("failed to inject pipette into test initrd")?;
let mut pipette_initrd_file = tempfile::NamedTempFile::new()?;
pipette_initrd_file.write_all(&pipette_initrd)?;
| // graph to the log so VM state is captured for | ||
| // post-mortem diagnosis even when no client is | ||
| // attached to inspect it. | ||
| self.dump_inspect_graph().await; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GB200-style device topologies need the management interface to describe PCIe devices as ACPI Generic Initiators and control the ACS capabilities advertised by individual ports. Extend the VM service schema and topology conversion so these settings reach the canonical OpenVMM configuration.
Use pipette in the ttrpc integration test to verify the resulting SRAT Generic Initiator entry and ACS capability directly from inside the guest.