Skip to content

Conversation

@mattkur
Copy link
Contributor

@mattkur mattkur commented Oct 16, 2025

Tests were setting this flag, but weren't actually testing their device DMA
with shared visibility pool. The shared visibility pool is only used when
the VM is isolated.

Bring this back in one of two ways:

  1. Add isolated tests (and get the right behavior), or
  2. Change the devices to force into using the shared pool.

Sorta fixes #2185 (the real issue there is that we weren't testing what we thought we were ... :()

@mattkur mattkur requested a review from a team as a code owner October 16, 2025 16:40
Copilot AI review requested due to automatic review settings October 16, 2025 16:40
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes the command line configuration option for enabling the shared visibility pool in OpenHCL, along with associated test cases that were using this option. The shared visibility pool is only used when the VM is isolated, so the removed configuration option and tests were not actually testing the intended behavior since they weren't running in an isolated environment.

Key changes:

  • Removed the OPENHCL_ENABLE_SHARED_VISIBILITY_POOL environment variable and related configuration logic
  • Deleted test cases that were incorrectly attempting to test shared pool behavior in non-isolated environments
  • Simplified the shared pool memory allocation logic to only depend on isolation type

Reviewed Changes

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

Show a summary per file
File Description
vmm_tests/vmm_tests/tests/tests/x86_64/openhcl_uefi.rs Removed nvme_relay_shared_pool test function that was incorrectly testing shared pool behavior
vmm_tests/vmm_tests/tests/tests/x86_64/openhcl_linux_direct.rs Removed command line options from MANA NIC tests that were not actually enabling shared pool behavior
openhcl/underhill_core/src/worker.rs Removed enable_shared_visibility_pool field from UnderhillEnvCfg and simplified memory allocation logic
openhcl/underhill_core/src/options.rs Removed enable_shared_visibility_pool option and related environment variable parsing
openhcl/underhill_core/src/lib.rs Removed assignment of the removed configuration field

@chris-oo
Copy link
Member

Right, now that we have isolated VM tests onboarded, this isn't as useful. We still need to onboard some device tests but those should come soon, i hope.

@mattkur mattkur linked an issue Oct 16, 2025 that may be closed by this pull request
@mattkur mattkur requested review from a team and amy-microsoft October 16, 2025 16:44
@chris-oo chris-oo changed the title underhill_core: remove cmdline to configure shared vis pool + tests openhcl: remove cmdline to configure shared vis pool + tests Oct 16, 2025
@mattkur mattkur enabled auto-merge (squash) October 16, 2025 18:00
@mattkur mattkur merged commit 19460df into microsoft:main Oct 16, 2025
50 checks passed
@mattkur mattkur deleted the delete-shmem-test branch October 16, 2025 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

openhcl: dma: shared pool returns pfn list in reverse order

4 participants