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 16:35, Suzuki K Poulose wrote:
> 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.

See Documentation/virt/kvm/api.rst, under section : KVM_CREATE_GUEST_MEMFD


"When the KVM MMU performs a PFN lookup to service a guest fault, the 
fault will
always be consumed from guest_memfd, regardless of whether it is a 
shared or a
private fault (unless in-place conversion is disabled and the backing
guest_memfd does not have the GUEST_MEMFD_FLAG_MMAP flag set)."


Cheers
Suzuki


> 
> 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
>>
>