On 22/01/2026 16:34, Ackerley Tng wrote:
Nikita Kalyazin <[email protected]> writes:
Was preparing the reply but couldn't get to it before the
meeting. Here's what was also discussed at the guest_memfd biweekly on
2026-01-22:
[...snip...]
@@ -423,6 +464,12 @@ static vm_fault_t kvm_gmem_fault_user_mapping(struct
vm_fault *vmf)
kvm_gmem_mark_prepared(folio);
}
+ err = kvm_gmem_folio_zap_direct_map(folio);
Perhaps the check for gmem_flags & GUEST_MEMFD_FLAG_NO_DIRECT_MAP should
be done here before making the call to kvm_gmem_folio_zap_direct_map()
to make it more obvious that zapping is conditional.
Makes sense to me.
Perhaps also add a check for kvm_arch_gmem_supports_no_direct_map() so
this call can be completely removed by the compiler if it wasn't
compiled in.
But if it is compiled in, we will be paying the cost of the call on
every page fault? Eg on arm64, it will call the following:
bool can_set_direct_map(void)
{
...
return rodata_full || debug_pagealloc_enabled() ||
arm64_kfence_can_set_direct_map() || is_realm_world();
}
You're right that this could end up paying the cost on every page
fault. Please ignore this request!
The kvm_gmem_folio_no_direct_map() check should probably remain in
kvm_gmem_folio_zap_direct_map() since that's a "if already zapped, don't
zap again" check.
+ if (err) {
+ ret = vmf_error(err);
+ goto out_folio;
+ }
+
vmf->page = folio_file_page(folio, vmf->pgoff);
out_folio:
@@ -533,6 +580,8 @@ static void kvm_gmem_free_folio(struct folio *folio)
kvm_pfn_t pfn = page_to_pfn(page);
int order = folio_order(folio);
+ kvm_gmem_folio_restore_direct_map(folio);
+
I can't decide if the kvm_gmem_folio_no_direct_map(folio) should be in
the caller or within kvm_gmem_folio_restore_direct_map(), since this
time it's a folio-specific property being checked.
I'm tempted to keep it similar to the kvm_gmem_folio_zap_direct_map()
case. How does the fact it's a folio-speicific property change your
reasoning?
This is good too:
if (kvm_gmem_folio_no_direct_map(folio))
kvm_gmem_folio_restore_direct_map(folio)
It turns out we can't do that because folio->mapping is gone by the time
filemap_free_folio() is called so we can't inspect the flags. Are you
ok with only having this check when zapping (but not when restoring)?
Do you think we should add a comment saying it's conditional here?
Perhaps also add a check for kvm_arch_gmem_supports_no_direct_map() so
this call can be completely removed by the compiler if it wasn't
compiled in. IIUC whether the check is added in the caller or within
kvm_gmem_folio_restore_direct_map() the call can still be elided.
Same concern as the above about kvm_gmem_folio_zap_direct_map(), ie the
performance of the case where kvm_arch_gmem_supports_no_direct_map() exists.
Please ignore this request!
kvm_arch_gmem_invalidate(pfn, pfn + (1ul << order));
}
@@ -596,6 +645,9 @@ static int __kvm_gmem_create(struct kvm *kvm, loff_t size,
u64 flags)
/* Unmovable mappings are supposed to be marked unevictable as well. */
WARN_ON_ONCE(!mapping_unevictable(inode->i_mapping));
+ if (flags & GUEST_MEMFD_FLAG_NO_DIRECT_MAP)
+ mapping_set_no_direct_map(inode->i_mapping);
+
GMEM_I(inode)->flags = flags;
file = alloc_file_pseudo(inode, kvm_gmem_mnt, name, O_RDWR,
&kvm_gmem_fops);
@@ -807,6 +859,8 @@ int kvm_gmem_get_pfn(struct kvm *kvm, struct
kvm_memory_slot *slot,
if (!is_prepared)
r = kvm_gmem_prepare_folio(kvm, slot, gfn, folio);
+ kvm_gmem_folio_zap_direct_map(folio);
+
Is there a reason why errors are not handled when faulting private memory?
No, I can't see a reason. Will add a check, thanks.
folio_unlock(folio);
if (!r)
--
2.50.1
