Hi!
On Mon, 2025-04-14 at 12:51 +0100, David Hildenbrand wrote:
>
[...]
> On top of that, I was wondering if we could look into doing something like
> the following. It would also allow for pulling pages out of gmem for
> existing SW-protected VMs once they enable shared memory for GMEM IIUC.
>
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 08eebd24a0e18..6f878cab0f466 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -4495,11 +4495,6 @@ static int kvm_mmu_faultin_pfn_gmem(struct kvm_vcpu *vcpu,
> {
> int max_order, r;
>
> - if (!kvm_slot_has_gmem(fault->slot)) {
> - kvm_mmu_prepare_memory_fault_exit(vcpu, fault);
> - return -EFAULT;
> - }
> -
> r = kvm_gmem_get_pfn(vcpu->kvm, fault->slot, fault->gfn, &fault->pfn,
> &fault->refcounted_page, &max_order);
> if (r) {
> @@ -4518,8 +4513,19 @@ static int __kvm_mmu_faultin_pfn(struct kvm_vcpu *vcpu,
> struct kvm_page_fault *fault)
> {
> unsigned int foll = fault->write ? FOLL_WRITE : 0;
> + bool use_gmem = false;
> +
> + if (fault->is_private) {
> + if (!kvm_slot_has_gmem(fault->slot)) {
> + kvm_mmu_prepare_memory_fault_exit(vcpu, fault);
> + return -EFAULT;
> + }
> + use_gmem = true;
> + } else if (kvm_slot_has_gmem_with_shared(fault->slot)) {
> + use_gmem = true;
> + }
>
> - if (fault->is_private)
> + if (use_gmem)
> return kvm_mmu_faultin_pfn_gmem(vcpu, fault);
>
> foll |= FOLL_NOWAIT;
>
>
> That is, we'd not claim that things are private when they are not, but instead
> teach the code about shared memory coming from gmem.
>
> There might be some more missing, just throwing it out there if I am completely off.
I think I arrived at the need for this as well while experimenting with
building a Firecracker version that works with my direct map removal
patches.
With this patch series, on ARM, as soon as a memslot has a guest_memfd
associated with it, all guest faults go through kvm_gmem_get_pfn, but on
x86, they go through slot->userspace_addr by default, as
CONFIG_KVM_SW_PROTECTED_VM selects CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES.
There's no real difference between these if slot->userspace_addr can be
GUP'd, but if its a VMA of a guest_memfd without direct map entries,
faulting through slot->userspace_addr wont work. So on x86 Firecracker
has to formally set the memory attributes to private, while on ARM it
doesn't [1], which is a bit awkward.
David, I couldn't find an implementation of
kvm_slot_has_gmem_with_shared() in the branch you shared, but would it
be something like "slot->userspace_addr points to a gmem VMA,
particularly to a VMA of the gmem that's associated with this memslot,
mapped at the same offset"?
Best,
Patrick
[1]: https://github.com/firecracker-microvm/firecracker/blob/feature/secret-hiding/src/vmm/src/builder.rs#L268
> --
> Cheers,
>
> David / dhildenb
>
