linux-kernel - Re: [RFC PATCH v2 6/7] KVM: selftests: Adapt vm_userspace_mem_region

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANgfPd9EF6Yc_SAR9sRY7oXMx3um+6phb71yNL=AsHDV4+tCRA@mail.gmail.com>
Date:   Thu, 25 Feb 2021 15:44:28 -0800
From:   Ben Gardon <bgardon@...gle.com>
To:     Yanan Wang <wangyanan55@...wei.com>
Cc:     kvm <kvm@...r.kernel.org>, linux-kselftest@...r.kernel.org,
        LKML <linux-kernel@...r.kernel.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Sean Christopherson <seanjc@...gle.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...hat.com>,
        Ingo Molnar <mingo@...nel.org>,
        Andrew Jones <drjones@...hat.com>,
        Peter Xu <peterx@...hat.com>, Marc Zyngier <maz@...nel.org>,
        wanghaibin.wang@...wei.com, yuzenghui@...wei.com
Subject: Re: [RFC PATCH v2 6/7] KVM: selftests: Adapt vm_userspace_mem_region_add
 to new helpers

On Wed, Feb 24, 2021 at 10:03 PM Yanan Wang <wangyanan55@...wei.com> wrote:
>
> With VM_MEM_SRC_ANONYMOUS_THP specified in vm_userspace_mem_region_add(),
> we have to get the transparent hugepage size for HVA alignment. With the
> new helpers, we can use get_backing_src_pagesz() to check whether THP is
> configured and then get the exact configured hugepage size.
>
> As different architectures may have different THP page sizes configured,
> this can get the accurate THP page sizes on any platform.
>
> Signed-off-by: Yanan Wang <wangyanan55@...wei.com>
> ---
>  tools/testing/selftests/kvm/lib/kvm_util.c | 27 +++++++---------------
>  1 file changed, 8 insertions(+), 19 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index b91c8e3a7ee1..0105fbfed036 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -18,7 +18,6 @@
>  #include <unistd.h>
>  #include <linux/kernel.h>
>
> -#define KVM_UTIL_PGS_PER_HUGEPG 512
>  #define KVM_UTIL_MIN_PFN       2
>
>  /* Aligns x up to the next multiple of size. Size must be a power of 2. */
> @@ -686,7 +685,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
>  {
>         int ret;
>         struct userspace_mem_region *region;
> -       size_t huge_page_size = KVM_UTIL_PGS_PER_HUGEPG * vm->page_size;
> +       size_t backing_src_pagesz = get_backing_src_pagesz(src_type);
>         size_t alignment;
>
>         TEST_ASSERT(vm_adjust_num_guest_pages(vm->mode, npages) == npages,
> @@ -748,7 +747,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
>  #endif
>
>         if (src_type == VM_MEM_SRC_ANONYMOUS_THP)
> -               alignment = max(huge_page_size, alignment);
> +               alignment = max(backing_src_pagesz, alignment);
>
>         /* Add enough memory to align up if necessary */
>         if (alignment > 1)
> @@ -767,22 +766,12 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
>         region->host_mem = align(region->mmap_start, alignment);
>
>         /* As needed perform madvise */
> -       if (src_type == VM_MEM_SRC_ANONYMOUS || src_type == VM_MEM_SRC_ANONYMOUS_THP) {
> -               struct stat statbuf;
> -
> -               ret = stat("/sys/kernel/mm/transparent_hugepage", &statbuf);
> -               TEST_ASSERT(ret == 0 || (ret == -1 && errno == ENOENT),
> -                           "stat /sys/kernel/mm/transparent_hugepage");
> -
> -               TEST_ASSERT(ret == 0 || src_type != VM_MEM_SRC_ANONYMOUS_THP,
> -                           "VM_MEM_SRC_ANONYMOUS_THP requires THP to be configured in the host kernel");
> -
> -               if (ret == 0) {
> -                       ret = madvise(region->host_mem, npages * vm->page_size,
> -                                     src_type == VM_MEM_SRC_ANONYMOUS ? MADV_NOHUGEPAGE : MADV_HUGEPAGE);
> -                       TEST_ASSERT(ret == 0, "madvise failed, addr: %p length: 0x%lx src_type: %x",
> -                                   region->host_mem, npages * vm->page_size, src_type);
> -               }
> +       if (src_type <= VM_MEM_SRC_ANONYMOUS_THP && thp_configured()) {

This check relies on an unstated property of the backing src type
enums where VM_MEM_SRC_ANONYMOUS and VM_MEM_SRC_ANONYMOUS_THP are
declared first.
It would probably be more readable for folks if the check was explicit:
if ((src_type == VM_MEM_SRC_ANONYMOUS || src_type ==
VM_MEM_SRC_ANONYMOUS_THP) && thp_configured()) {


> +               ret = madvise(region->host_mem, npages * vm->page_size,
> +                             src_type == VM_MEM_SRC_ANONYMOUS ? MADV_NOHUGEPAGE : MADV_HUGEPAGE);
> +               TEST_ASSERT(ret == 0, "madvise failed, addr: %p length: 0x%lx src_type: %s",
> +                           region->host_mem, npages * vm->page_size,
> +                           vm_mem_backing_src_alias(src_type)->name);
>         }
>
>         region->unused_phy_pages = sparsebit_alloc();
> --
> 2.19.1
>