Re: [PATCH v2 02/12] KVM: arm64: guest_memfd: Check mmap support when preparing memslot

[Suzuki K Poulose <suzuki.poulose@arm.com>]

On 12/11/2026 10:43, Steven Price wrote:
> On 11/11/2026 15:21, Suzuki K Poulose wrote:
>> On 11/11/2026 14:57, Steven Price wrote:
>>> Check the guest_memfd backing file when deciding whether an arm64
>>> memslot has mappable memory rather than checking whether the slot is
>>> gmem only. A public helper function is added to be able to reuse the
>>> static kvm_gmem_supports_mmap() function.
>>>
>>
>> Please could you explain why we need this ? Why not rely on
>> kvm_memslot_is_gmem_only(), which already tells
>> kvm_gmem_supports_mmap() ? With inplace conversion support gmem supports
>> mmap.
> 
> Unless I'm missing something it's still possible to create a guest_memfd
> that doesn't support mmap() by not passing the GUEST_MEMFD_FLAG_MMAP
> flag. So kvm_memslot_is_gmem_only() doesn't actually guarantee that it's
> a guest_memfd that supports mmap().

I see, while the KVM decides that, with inplace conversion shared pages
come from gmem, we do allow creation of gmem without MMAP set, implying
we would fail the mmap request from VMM, which is pretty pointless. But
should the KVM fail this when VMM is shooting on its foot ? Only makes
sense if there are no untrusted device DMA for the CoCo Guest.

Am I missing something ?

Cheers
Suzuki




> 
>>
>>> Signed-off-by: Steven Price <steven.price@arm.com>
>>>
>>> ---
>>> Changes since v1:
>>> - New patch in v2. Check the guest_memfd backing file rather than the
>>>     memslot flag when deciding whether arm64 can map the backing memory.
>>> ---
>>>    arch/arm64/kvm/mmu.c     | 7 ++++---
>>>    include/linux/kvm_host.h | 1 +
>>>    virt/kvm/guest_memfd.c   | 6 ++++++
>>>    3 files changed, 11 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
>>> index 1db10d8d6db7..a146f6cde3d0 100644
>>> --- a/arch/arm64/kvm/mmu.c
>>> +++ b/arch/arm64/kvm/mmu.c
>>> @@ -2839,10 +2839,11 @@ int kvm_arch_prepare_memory_region(struct kvm
>>> *kvm,
>>>            return -EFAULT;
>>>          /*
>>> -     * Only support guest_memfd backed memslots with mappable memory,
>>> since
>>> -     * there aren't any CoCo VMs that support only private memory on
>>> arm64.
>>> +     * Protected VMs currently require guest_memfd backed memslots with
>>> +     * mappable memory to support shared mappings.
>>>         */
>>> -    if (kvm_slot_has_gmem(new) && !kvm_memslot_is_gmem_only(new))
>>> +    if (kvm_arch_has_private_mem(kvm) && kvm_slot_has_gmem(new) &&
>>> +        !kvm_gmem_slot_supports_mmap(new))
> 
> Sashiko has identified I've screwed up this condition though.
> kvm_arch_has_private_mem(kvm) shouldn't be in there as that allows a
> non-private VM to bind a non-mmappable gmem.
> 
>>>            return -EINVAL;
>>>          /* guest_memfd is incompatible with MTE. */
>>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>>> index e7fc94e2910a..f65728a713ec 100644
>>> --- a/include/linux/kvm_host.h
>>> +++ b/include/linux/kvm_host.h
>>> @@ -2614,6 +2614,7 @@ static inline bool kvm_is_private_gfn(struct kvm
>>> *kvm, gfn_t gfn)
>>>      #ifdef CONFIG_KVM_GUEST_MEMFD
>>>    bool kvm_gmem_is_private_gfn(struct kvm *kvm, gfn_t gfn);
>>> +bool kvm_gmem_slot_supports_mmap(const struct kvm_memory_slot *slot);
>>>    bool kvm_arch_supports_gmem_init_shared(struct kvm *kvm);
>>>      static inline u64 kvm_gmem_get_supported_flags(struct kvm *kvm)
>>> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
>>> index 8c97a69b16dc..3dd8d227bebb 100644
>>> --- a/virt/kvm/guest_memfd.c
>>> +++ b/virt/kvm/guest_memfd.c
>>> @@ -442,6 +442,12 @@ static bool kvm_gmem_supports_mmap(struct inode
>>> *inode)
>>>        return GMEM_I(inode)->flags & GUEST_MEMFD_FLAG_MMAP;
>>>    }
>>>    +bool kvm_gmem_slot_supports_mmap(const struct kvm_memory_slot *slot)
>>> +{
>>> +    /* The guest_memfd file is pinned while preparing a new memslot. */
>>
>> We should gate this on kvm_slot_has_gmem() and how do we ensure that the
>> call comes after it is prepared ?
> 
> I'm not sure what you mean by "gate this on kvm_slot_has_gmem()" - yes
> this function only makes sense on a gmem slot - the user above checks
> that first. I'm not sure what value the function should call if you call
> it on a non-gmem slot?
> 
> Thanks,
> Steve
> 
>>> +    return kvm_gmem_supports_mmap(file_inode(slot->gmem.file));
>>> +}
>>> +
>>>    static vm_fault_t kvm_gmem_fault_user_mapping(struct vm_fault *vmf)
>>>    {
>>>        struct inode *inode = file_inode(vmf->vma->vm_file);
>>
>>
>> Cheers
>> Suzuki
>