On 30/10/2025 21:37, Sean Christopherson wrote:
On Fri, Oct 24, 2025, Nikita Kalyazin wrote:


On 23/10/2025 17:07, Sean Christopherson wrote:
On Mon, Oct 20, 2025, Nikita Kalyazin wrote:
From: Nikita Kalyazin <[email protected]>

+ Vishal and Ackerley


write syscall populates guest_memfd with user-supplied data in a generic
way, ie no vendor-specific preparation is performed.  If the request is
not page-aligned, the remaining bytes are initialised to 0.

write is only supported for non-CoCo setups where guest memory is not
hardware-encrypted.

Please include all of the "why".  The code mostly communicates the "what", but
it doesn't capture why write() support is at all interesting, nor does it 
explain
why read() isn't supported.

Hi Sean,

Thanks for the review.

Do you think including the explanation from the cover letter would be
sufficient?

It's pretty close.  A few more details would be helpful, e.g. to explain that 
VMMs
may use write() to populate the initial image

Ack.


Shall I additionally say that read() isn't supported because there is no use
case for it as of now or would it be obvious?

Hmm, I think if you want to exclude read() support, the changelog should 
explicitly
state why.  E.g. "there's no use case" is quite different from "deliberately
don't support read() for security reasons".

Ack.


Signed-off-by: Nikitia Kalyazin <[email protected]>
---
   virt/kvm/guest_memfd.c | 48 ++++++++++++++++++++++++++++++++++++++++++

There's a notable lack of uAPI and Documentation chanegs.  I.e. this needs a
GUEST_MEMFD_FLAG_xxx along with proper documentation.

Would the following be ok in the doc?

When the capability KVM_CAP_GUEST_MEMFD_WRITE is supported, the 'flags'

No capability is necessary, see d2042d8f96dd ("KVM: Rework 
KVM_CAP_GUEST_MEMFD_MMAP
into KVM_CAP_GUEST_MEMFD_FLAGS").

Thanks, I didn't realise that kvm/next was behind kvm/master.


field
supports GUEST_MEMFD_FLAG_WRITE. Setting this flag on guest_memfd creation
enables write() syscall operations to populate guest_memfd memory from host
userspace.

When a write() operation is performed on a guest_memfd file descriptor with
the
GUEST_MEMFD_FLAG_WRITE set, the syscall will populate the guest memory with
user-supplied data in a generic way, without any vendor-specific
preparation.
The write operation is only supported for non-CoCo (Confidential Computing)
setups where guest memory is not hardware-encrypted.

The restriction should be that guest memory must be SHARED, i.e. not PRIVATE.
Strictly speaking, guest memory can be encrypted, e.g. with SME and TME (I think
TME is still a thing?), but with a shared key and thus accessible from the host.

Even if that weren't the case, we want to support this for CoCo VMs.

To clarify, should it depend on GUEST_MEMFD_FLAG_INIT_SHARED for now?

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 5bd76cf394fa..5fbf65f49586 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -736,7 +736,7 @@ static inline u64 kvm_gmem_get_supported_flags(struct kvm *kvm)
        u64 flags = GUEST_MEMFD_FLAG_MMAP;

        if (!kvm || kvm_arch_supports_gmem_init_shared(kvm))
-               flags |= GUEST_MEMFD_FLAG_INIT_SHARED;
+ flags |= GUEST_MEMFD_FLAG_INIT_SHARED | GUEST_MEMFD_FLAG_WRITE;

        return flags;
 }


If the write request is not page-aligned, any remaining bytes within the page
are initialized to zero.

Why?  (Honest question, e.g. is that standard file semantics?)

The clause was originally suggested by James in v5 [1]. The behaviour shouldn't be deviating from the standard semantics though, so I will omit it. Moreover, when looking at the shmem implementation, I realised that I hadn't handled the case of clearing bytes _before_ written bytes properly. I will fix it in the next version.

[1] https://lore.kernel.org/kvm/CADrL8HUObfEd80sr783dB3dPWGSX7H5=0hcp9ojil6d_sp+...@mail.gmail.com/


And while it's definitely it's a-ok to land .write() in advance of the direct 
map
changes, we do need to at least map out how we want the two to interact, e.g. so
that we don't end up with constraints that are impossible to satisfy.


write() shall not attempt to access a page that is not in the direct map,
which I believe can be achieved via kvm_kmem_gmem_write_begin() consulting
the KVM_GMEM_FOLIO_NO_DIRECT_MAP in folio->private (introduced in [1]).

Do you think we should mention it in the commit message in some way? What
particular constraint are you cautious about?

I want to be cautious with respect to the ABI/uAPI.  Patrick's series also adds
a flag, and guest_memfd doesn't currently provide a way to toggle flags after 
the
file is created.  That begs the question of how GUEST_MEMFD_FLAG_NO_DIRECT_MAP
will co-exist with GUEST_MEMFD_FLAG_WRITE.  Presumably the goal is to use 
write()
to initialize memory, and _then_ nuke the direct map.

I want line of sight to understanding the exact semantics/flows.  E.g. will KVM
require userspace to clear GUEST_MEMFD_FLAG_WRITE before allowing
NO_DIRECT_MAP?  Or will the write() simply fail?  How will the sequencing be
achieved?

No, I don't think we can clear the GUEST_MEMFD_FLAG_WRITE as we expect faults and writes to different pages to be arriving interspersed: some pages will be populated by write() proactively, some will be allocated by faults in the user mapping on demand. Both write() and the fault handler, if they need to allocate a page, will be writing content to it and "sealing" by removing it from the direct map. If write() faces an already "sealed" page, it will fail (with EEXIST [1]).


+     struct inode *inode = file_inode(file);
+     pgoff_t index = pos >> PAGE_SHIFT;
+     struct folio *folio;
+
+     if (!kvm_gmem_supports_mmap(inode))

Checking for MMAP is neither sufficient nor strictly necessary.  MMAP doesn't
imply SHARED, and it's not clear to me that mmap() support should be in any way
tied to WRITE support.

As in my reply to the comment about doc, I plan to introduce
KVM_CAP_GUEST_MEMFD_WRITE and GUEST_MEMFD_FLAG_WRITE.  The
kvm_arch_supports_gmem_write() will be a weak symbol and relying on
!kvm_arch_has_private_mem() on x86, similar to
kvm_arch_supports_gmem_mmap().  Does it look right?

No.  As above, write() should be allowed iff memory is SHARED.  Relevant commits
that are now in Linus' tree:

   44c6cb9fe9888b371e31165b2854bd0f4e2787d4 KVM: guest_memfd: Allow mmap() on 
guest_memfd for x86 VMs with private memory
   9aef71c892a55e004419923ba7129abe3e58d9f1 KVM: Explicitly mark 
KVM_GUEST_MEMFD as depending on KVM_GENERIC_MMU_NOTIFIER
   5d3341d684be80892d8f6f9812f90f9274b81177 KVM: guest_memfd: Invalidate SHARED 
GPAs if gmem supports INIT_SHARED

Ack.


Reply via email to