Use DMA framework to create DMAble buffers #722

bhargavshah1988 · 2025-01-24T19:04:16Z

Introduces the initial implementation of the DMA framework, providing a centralized mechanism for DMA-capable memory allocation. It establishes the foundation for managing direct memory access (DMA) operations efficiently in a unified manner. Future enhancements will include pin/unpin support and bounce buffer management to optimize memory handling and improve data transfer performance.

…crosoft#706) Adds a new boolean cxl_memory_enabled that gets passed to mu_msvm via config flags and indicates that the CXL root device ACPI0017 should be exposed in the DSDT ACPI table. Co-authored-by: Patrick Payne <patpa@microsoft.com>

mattkur · 2025-01-24T21:41:37Z

Please add more context in your PR description and update the title.

Our implementation of the hypercall supports it, so just leave it on.

Use the `expect_test` crate to generate inline test result values to compare against. This makes the tests more reliable and easier to debug.

The one function provided by tempfile_helpers had the footgun that files would be kept around forever instead of properly cleaned up. It also forced a specific error handling pattern. Remove it, fix the usage of it in pal_async to properly clean up test files, and inline it into the only remaining uses in petri, with better error handling.

openhcl/page_pool_alloc/src/lib.rs

openhcl/underhill_core/src/dma_manager.rs

chris-oo · 2025-01-27T18:12:53Z

openhcl/underhill_core/src/dma_manager.rs

+        }
+    }
+
+    pub fn create_client(&mut self, pci_id: String) -> DmaClient {


I'm not sure we want to clone DmaClients, as it seems reasonable to me that there is at most one client per pci_id. I'd rather here we make the Arc external, ie don't implement clone, but instead return an Arc<DmaClient>

chris-oo · 2025-01-27T18:13:35Z

openhcl/underhill_core/src/dma_manager.rs

+        self.clients.get(pci_id).cloned()
+    }
+
+    pub fn get_dma_buffer_allocator(


This doesn't make sense here - why is this not a method on DmaClient itself?

chris-oo · 2025-01-27T18:17:44Z

openhcl/underhill_core/src/nvme_manager.rs

        save_restore_supported: bool,
        saved_state: Option<NvmeSavedState>,
+        dma_manager: GlobalDmaManager,


Passing this by value raises questions as noted in the Clone derive for GlobalDmaManager.

chris-oo · 2025-01-27T18:19:33Z

openhcl/underhill_core/src/worker.rs

@@ -1863,43 +1872,67 @@ async fn new_underhill_vm(
        crate::inspect_proc::periodic_telemetry_task(driver_source.simple()),
    );

+    let dma_pool = if let Some(shared_vis_pages_pool) = shared_vis_pages_pool.clone() {


We cannot allow cloning the pools, for the reasons stated above. Which means, you'll need to refactor DmaManager to either take ownership of the pool, or have some method it can use to spawn allocators.

You can probably temporarily get away with just taking the pool spawner to allow you to spawn allocators, but you cannot allow multiple instances of the pool around because then usage / allocation is broken (multiple users of the same GPA range)

chris-oo · 2025-01-27T18:22:24Z

vm/devices/net/mana_driver/src/mana.rs

-            .device()
-            .host_allocator()
+
+        let dma_client = gdma.device().get_dma_client().context("Failed to get DMA client from device")?;


We talked about this before, but this get_dma_client should not return an Option, IE the method should not be faillable. Every device will want a dma client.

chris-oo · 2025-01-27T18:23:08Z

vm/devices/net/mana_driver/src/mana.rs

-    pub async fn host_allocator(&self) -> DmaAllocator<T> {
-        self.inner.gdma.lock().await.device().host_allocator()
+    /// Returns an object that can allocate dma memory to be shared with the device.
+    pub async fn get_dma_client(&self) -> anyhow::Result<Arc<dyn DmaClient>>


prefer not using get_<thing> naming in Rust, generally just call it by it's name, so in this case dma_client like how previously it was host_dma_allocator.

vm/devices/user_driver/src/vfio.rs

Adds support for running Hyper-V VMM tests to Petri. Includes functionality to run basic boot test, more configuration options will be added in future PRs to run more specialized tests.

…ft#731) The nested cargo invocation limitation is unfortunate, but unavoidable according to Wesley. At least we can tell people about it. Also refactor all the argument handling here to be more robust.

…oft#732) For our use of PowerShell, we should not need user profile customizations (which are sometimes quite expensive and/or chatty). Pass `-NoProfile` to disable loading of the user profile.

When running the nvme_fuzzer locally I came across an addition_overflow related panic. Computing the queue size for the I/O queues can cause an addition overflow related panic if value read from the mqes register is MAX_U16. The fix restructures how the io queue size is calculated in way that eliminates the possibility of addition_overflow. Also added a check to make sure that mqes_z() does not report an invalid value (< 1) in accordance with the NVMe spec: ![image](https://github.com/user-attachments/assets/9506ff06-9f5a-4a33-8b9e-70794070bdd6)

This fixes vmbus relaying with SNP.

openhcl/underhill_core/src/dma_manager.rs

… dma) (microsoft#729) Revert "Use the Shared Visibilty pool on aarch64. For reasons that we do not yet" This reverts commit a7c59b7.

) This operating mode was originally added so that we could get automated CI test coverage of our APIC emulation without needing to run a full CVM. However, CVM CI tests are now just around the corner. And, as it turns out, we accidentally weren't using this in our current tests at all, and it's now broken. Seeing as we haven't been hit with any APIC bugs in a very long time, and that proper coverage will be coming soon, just remove it. More cleanup is definitely possible here, such as removing support for certain hypercalls that are no longer needed, or moving trait functions off of Backing and onto HardwareIsolatedBacking. That can all come as follow ups.

Add a safe abstraction for temporarily lending on-stack data into thread local storage. Use it in various places across the stack. This fixes a use-after-free in `pal_async`, and it reduces the overhead of TLS in `pal_async` and `underhill_threadpool`.

…crosoft#748) Attempts to mitigate an issue where on non-ephemeral ARM test runners, if the previous job orphaned a running VM, all subsequent jobs would fail. This is accomplished by stopping and then removing the VM both in the destructor and before creation (if one exists).

Add infrastructure to simulate OpenHCL test failures 1. Introduced a OpenHCL command-line argument to define test scenarios. 2. Implemented custom actions based on the specified test scenario string. This enables better debugging and validation of OpenHCL failure scenarios.

Don't remove the deferred action list from TLS until the `ProcessorRunner` is actually dropped. This fixes crashes after a failed servicing operation.

Get rid of the weird sidecar `elements_processed` field and have the hypercall infra code construct the hypercall output directly.

…osoft#740) Start bringing up missing coverage for private pool and NVMe keepalive to VMM test suite. This is not complete end-to-end test yet, but brings necessary infrastructure changes. Update device tree properties to sync with Windows host changes. --------- Co-authored-by: Daniel Prilik <71350465+daprilik@users.noreply.github.com>

Refactoring work so that the startvp hypercall is not handled by OpenHCL for non-CVMs. Will help to keep future CVM-only work isolated to the CVM backings. Tested: - SNP VMs boot - x86 and aarch64 TVMs boot - TDX VMs boot

chris-oo · 2025-01-31T21:35:08Z

openhcl/underhill_core/src/dma_manager.rs

@@ -0,0 +1,88 @@
+// Copyright (c) Microsoft Corporation.


module comments please

chris-oo · 2025-01-31T21:35:19Z

vm/devices/user_driver/src/lib.rs

@@ -68,3 +71,9 @@ pub trait HostDmaAllocator: Send + Sync {
    /// Attach to a previously allocated memory block with contiguous PFNs.
    fn attach_dma_buffer(&self, len: usize, base_pfn: u64) -> anyhow::Result<MemoryBlock>;
 }
+
+pub trait DmaClient: Send + Sync {


please document this trait

`HvError` is supposed to represent a non-success status code, but it's also used for a possibly-successful status code in some places. Fix this by adding a new `HvStatus` type and making `HvError` a wrapper around `NonZeroU16`. This also gives us a nice niche optimization, so that `HvResult<(), HvError>` is equivalent to `HvStatus`.

I noticed "hvlite" was in a few user-facing strings where we it should not be. Fix the places where this is not a breaking change: in openvmm, flowey, and petri.

Get the physical address width via device tree to determine the alias map on systems that don't reliably report the physical address width architecturally (ARM).

…h1988/openvmm into user/bhsh/dmaPlumbing

bhargavshah1988 and others added 14 commits November 12, 2024 13:10

Merge branch 'main' of https://github.com/bhargavshah1988/openvmm

57c366b

Merge branch 'main' of https://github.com/bhargavshah1988/openvmm

2f4ad18

Merge branch 'main' of https://github.com/bhargavshah1988/openvmm

a9e19ee

Merge branch 'main' of https://github.com/bhargavshah1988/openvmm

9882a22

Merge branch 'main' of https://github.com/bhargavshah1988/openvmm

05d1751

First cut

9806bf8

VfioBuffer plumbing

b4595af

Call DMA API to allocate BB

a6c318a

Updated the nvme to use Dma client

3ed8345

Cleanup

7aad388

network plubming

ebececf

VM starts with disk

7057efc

Clenaup

5e2b704

bhargavshah1988 changed the title ~~User/bhsh/dma plumbing~~ Use DMA framework to create DMAble buffers Jan 24, 2025

smalis-msft and others added 3 commits January 27, 2025 09:40

Always support extended ranges for TLB flushing (microsoft#721)

816e8b8

Our implementation of the hypercall supports it, so just leave it on.

mesh_protobuf,inpsect: use expect_test in tests (microsoft#727)

4c9942a

Use the `expect_test` crate to generate inline test result values to compare against. This makes the tests more reliable and easier to debug.

chris-oo reviewed Jan 27, 2025

View reviewed changes

tjones60 and others added 7 commits January 27, 2025 13:06

Petri: Hyper-V support (microsoft#644)

517d07a

Adds support for running Hyper-V VMM tests to Petri. Includes functionality to run basic boot test, more configuration options will be added in future PRs to run more specialized tests.

Create spawner

ee505b7

format fix

cf4dfb4

xtask: Add support for running fuzzers on a stable toolchain (microso…

7b77c2f

…ft#731) The nested cargo invocation limitation is unfortunate, but unavoidable according to Wesley. At least we can tell people about it. Also refactor all the argument handling here to be more robust.

petri, diag_client: pass -NoProfile to powershell invocations (micros…

af7659b

…oft#732) For our use of PowerShell, we should not need user profile customizations (which are sometimes quite expensive and/or chatty). Pass `-NoProfile` to disable loading of the user profile.

virt_mshv_vtl/snp: fix untrusted sint readiness (microsoft#725)

1c74c1e

This fixes vmbus relaying with SNP.

chris-oo reviewed Jan 28, 2025

View reviewed changes

openhcl/underhill_core/src/dma_manager.rs Outdated Show resolved Hide resolved

openhcl/underhill_core/src/dma_manager.rs Outdated Show resolved Hide resolved

openhcl/underhill_core/src/dma_manager.rs Outdated Show resolved Hide resolved

bhargavshah1988 and others added 2 commits January 28, 2025 14:34

user parking_lot mutex

a21480d

openhcl: revert microsoft#539 (use shared visibility pool for aarch64…

cdf1bc2

… dma) (microsoft#729) Revert "Use the Shared Visibilty pool on aarch64. For reasons that we do not yet" This reverts commit a7c59b7.

smalis-msft and others added 9 commits January 30, 2025 21:30

hcl: fix multiple calls to flush_deferred_actions (microsoft#739)

a4c8d79

Don't remove the deferred action list from TLS until the `ProcessorRunner` is actually dropped. This fixes crashes after a failed servicing operation.

hv1_hypercall: simplify hypercall output handling (microsoft#736)

81e97dd

Get rid of the weird sidecar `elements_processed` field and have the hypercall infra code construct the hypercall output directly.

virt_mshv_vtl: Scope startvp handling to CVMs (microsoft#726)

88225af

Refactoring work so that the startvp hypercall is not handled by OpenHCL for non-CVMs. Will help to keep future CVM-only work isolated to the CVM backings. Tested: - SNP VMs boot - x86 and aarch64 TVMs boot - TDX VMs boot

Do no return error with dma_client

e98eb3c

chris-oo reviewed Jan 31, 2025

View reviewed changes

jstarks and others added 20 commits January 31, 2025 14:12

Rename hvlite to openvmm in more user-facing strings (microsoft#757)

b812e33

I noticed "hvlite" was in a few user-facing strings where we it should not be. Fix the places where this is not a breaking change: in openvmm, flowey, and petri.

openhcl_boot: Get alias map from device tree (microsoft#733)

495daf6

Get the physical address width via device tree to determine the alias map on systems that don't reliably report the physical address width architecturally (ARM).

First cut

8871a24

VfioBuffer plumbing

7c558f8

Call DMA API to allocate BB

a0062c9

Updated the nvme to use Dma client

e8fdac1

Cleanup

28bcb12

network plubming

b448156

VM starts with disk

332b0d6

Clenaup

db03555

Create spawner

48de600

format fix

c9643e9

user parking_lot mutex

1c008be

Add plumbing for emulated devices

df479c0

Must get DMA client

fbe274f

Do no return error with dma_client

71973e2

Add comments

0b26c3b

Merge branch 'user/bhsh/dmaPlumbing' of https://github.com/bhargavsha…

8e4e9dd

…h1988/openvmm into user/bhsh/dmaPlumbing

Remove comment

772a058

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use DMA framework to create DMAble buffers #722

Use DMA framework to create DMAble buffers #722

bhargavshah1988 commented Jan 24, 2025 •

edited

Loading

mattkur commented Jan 24, 2025

chris-oo Jan 27, 2025

chris-oo Jan 27, 2025

chris-oo Jan 27, 2025

chris-oo Jan 27, 2025

chris-oo Jan 27, 2025

chris-oo Jan 27, 2025

chris-oo Jan 27, 2025

chris-oo Jan 31, 2025

chris-oo Jan 31, 2025

Use DMA framework to create DMAble buffers #722

Are you sure you want to change the base?

Use DMA framework to create DMAble buffers #722

Conversation

bhargavshah1988 commented Jan 24, 2025 • edited Loading

mattkur commented Jan 24, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bhargavshah1988 commented Jan 24, 2025 •

edited

Loading