On 06/07/2026 19:17, Ackerley Tng wrote:
Suzuki K Poulose <[email protected]> writes:


[...snip...]

+static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start,
+                                    size_t nr_pages, uint64_t attrs)
+{
+       struct address_space *mapping = inode->i_mapping;
+       struct gmem_inode *gi = GMEM_I(inode);
+       pgoff_t end = start + nr_pages;
+       struct maple_tree *mt;
+       struct ma_state mas;
+       int r;
+
+       mt = &gi->attributes;
+
+       filemap_invalidate_lock(mapping);
+
+       mas_init(&mas, mt, start);
+       r = kvm_gmem_mas_preallocate(&mas, attrs, start, nr_pages);
+       if (r)
+               goto out;
+
+       /*
+        * From this point on guest_memfd has performed necessary
+        * checks and can proceed to do guest-breaking changes.
+        */
+
+       kvm_gmem_invalidate_start(inode, start, end);

I added support for Arm CCA KVM patches with the inplace conversion and
I am hitting the following issue.

1. I am supporting INIT_SHARED + MMAP flags.
2. VMM creates the Gmem_fd with both the flags above.
3. Uses the shared gmem-mmap to load the initial payloads (kernel, dtb).
4. At the VM finalization time, Populate the loaded regions one by one
     by
      a) copying the images to a temparory buffer - Since CCA can't really
         load the contents in-place.

Sounds good :). I see that you blocked this in the kernel by returning
-EOPNOTSUPP if (!src_page) [0].

We could do the copy in kernel with src_page == dst_page, but that would
affect the batching of Granule delegation (and at which point we might
need a temparory buffer in the kernel as big as the vma_pagesize)


      b) Set the "region" to Private in the gmem_fd (via
SET_MEMORY_ATTRIBUTES2)
      c) Invoke CCA backend to populate the private memory via
         ioctl(KVM_ARM_RMI_POPULATE,..) [0]


This flow sounds right.

[0]
https://lore.kernel.org/all/[email protected]/


5. Additionally, VMM can mark the entire RAM to be private before the VM
     starts running, again via SET_MEMORY_ATTRIBUTES2. On CCA, this
action is measured and doesn't require the Host to "commit" memory to
the VM.
Instead the host can lazily donate memory on a fault.


For both TDX and SNP, the host can also lazily donate memory,
guest_memfd supports this.

But step (5) triggers the invalidation of both private and shared
mappings of the gmem area, from the kvm_gmem_invalidate_start()
above.

This is because, the entire DRAM now has, some portions PRIVATE (the
loaded regions) and the rest are SHARED (from the Gmem_fd creation).
   Thus, kvm_gmem_get_invalidate_filter(Dram_start, Dram_end) causes the
invalidation of both "PRIVATE" and "SHARED" regions, which results
in the destruction of the already loaded data and things go south.


This destruction will happen for TDX as well. I think we managed to get
around this because we didn't apply conversion on the already-private
ranges.

IIUC on SNP, zapping pages in the stage 2 page tables doesn't destroy
the data, so that's probably why it has been fine for SNP.

Additionally, the Guest at boot, will try to mark the entire DRAM
as Private (RIPAS_RAM in CCA), which would trigger this anyways.

Suzuki



When we know that the kvm_gmem_invalidate_xx is triggered by a
conversion, we don't need to invalidate the existing pages that
are in the requested state. i.e., the following patch on top of
this series does the trick for me :


diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index a97fcac34a0e..62e0427a49f4 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c
@@ -250,16 +250,23 @@ static void __kvm_gmem_invalidate_start(struct
gmem_file *f, pgoff_t start,
                  KVM_MMU_UNLOCK(kvm);
   }

+static void kvm_gmem_invalidate_start_filter(struct inode *inode,
pgoff_t start,
+                                            pgoff_t end,
+                                            enum kvm_gfn_range_filter
attr_filter)
+{
+       struct gmem_file *f;
+
+       kvm_gmem_for_each_file(f, inode)
+               __kvm_gmem_invalidate_start(f, start, end, attr_filter);
+}
+
   static void kvm_gmem_invalidate_start(struct inode *inode, pgoff_t start,
                                        pgoff_t end)
   {
          enum kvm_gfn_range_filter attr_filter;
-       struct gmem_file *f;
-
          attr_filter = kvm_gmem_get_invalidate_filter(inode, start, end);

-       kvm_gmem_for_each_file(f, inode)
-               __kvm_gmem_invalidate_start(f, start, end, attr_filter);
+       kvm_gmem_invalidate_start_filter(inode, start, end, attr_filter);
   }

   static void __kvm_gmem_invalidate_end(struct gmem_file *f, pgoff_t start,
@@ -724,9 +731,14 @@ static int __kvm_gmem_set_attributes(struct inode
*inode, pgoff_t start,
          /*
           * From this point on guest_memfd has performed necessary
           * checks and can proceed to do guest-breaking changes.
+        * Also, we don't have to invalidate the regions that
+        * may already be in the requested state. Hence, we could
+        * explicitly filter the invalidations to the opposite
+        * state.
           */

-       kvm_gmem_invalidate_start(inode, start, end);
+       kvm_gmem_invalidate_start_filter(inode, start, end,
+                                       to_private ? KVM_FILTER_SHARED :
KVM_FILTER_PRIVATE);


I think this makes sense. Thanks for catching this.

          if (!to_private)
                  kvm_gmem_invalidate(inode, start, end);


Thoughts ?

Suzuki



[...snip...]



Reply via email to