On Wed, Nov 1, 2023 at 11:35 PM Sean Christopherson [off-list ref] wrote:

On Wed, Nov 01, 2023, Paolo Bonzini wrote:

quoted

On 11/1/23 17:36, Sean Christopherson wrote:

quoted

quoted

quoted

"Allow" isn't perfect, e.g. I would much prefer a straight KVM_GUEST_MEMFD_USE_HUGEPAGES
or KVM_GUEST_MEMFD_HUGEPAGES flag, but I wanted the name to convey that KVM doesn't
(yet) guarantee hugepages.  I.e. KVM_GUEST_MEMFD_ALLOW_HUGEPAGE is stronger than
a hint, but weaker than a requirement.  And if/when KVM supports a dedicated memory
pool of some kind, then we can add KVM_GUEST_MEMFD_REQUIRE_HUGEPAGE.

I think that the current patch is fine, but I will adjust it to always
allow the flag, and to make the size check even if !CONFIG_TRANSPARENT_HUGEPAGE.
If hugepages are not guaranteed, and (theoretically) you could have no
hugepage at all in the result, it's okay to get this result even if THP is not
available in the kernel.

Can you post a fixup patch?  It's not clear to me exactly what behavior you intend
to end up with.

Sure, just this:

diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
index 7d1a33c2ad42..34fd070e03d9 100644
--- a/virt/kvm/guest_memfd.c
+++ b/virt/kvm/guest_memfd.c

@@ -430,10 +430,7 @@ int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args)
 {
      loff_t size = args->size;
      u64 flags = args->flags;
-     u64 valid_flags = 0;
-
-     if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE))
-             valid_flags |= KVM_GUEST_MEMFD_ALLOW_HUGEPAGE;
+     u64 valid_flags = KVM_GUEST_MEMFD_ALLOW_HUGEPAGE;
      if (flags & ~valid_flags)
              return -EINVAL;

@@ -441,11 +438,9 @@ int kvm_gmem_create(struct kvm *kvm, struct kvm_create_guest_memfd *args)
      if (size < 0 || !PAGE_ALIGNED(size))
              return -EINVAL;
-#ifdef CONFIG_TRANSPARENT_HUGEPAGE
      if ((flags & KVM_GUEST_MEMFD_ALLOW_HUGEPAGE) &&
          !IS_ALIGNED(size, HPAGE_PMD_SIZE))
              return -EINVAL;
-#endif

That won't work, HPAGE_PMD_SIZE is valid only for CONFIG_TRANSPARENT_HUGEPAGE=y.

#else /* CONFIG_TRANSPARENT_HUGEPAGE */
#define HPAGE_PMD_SHIFT ({ BUILD_BUG(); 0; })
#define HPAGE_PMD_MASK ({ BUILD_BUG(); 0; })
#define HPAGE_PMD_SIZE ({ BUILD_BUG(); 0; })

Would have caught it when actually testing it, I guess. :) It has to
be PMD_SIZE, possibly with

#ifdef CONFIG_TRANSPARENT_HUGEPAGE
BUILD_BUG_ON(HPAGE_PMD_SIZE != PMD_SIZE);
#endif

for extra safety.

Paolo


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel