All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ben Gardon <bgardon@google.com>
To: David Matlack <dmatlack@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	kvm@vger.kernel.org, Sean Christopherson <seanjc@google.com>,
	Andrew Jones <drjones@redhat.com>,
	Jim Mattson <jmattson@google.com>,
	Yanan Wang <wangyanan55@huawei.com>, Peter Xu <peterx@redhat.com>,
	Aaron Lewis <aaronlewis@google.com>
Subject: Re: [PATCH v2 02/12] KVM: selftests: Expose align() helpers to tests
Date: Thu, 11 Nov 2021 09:43:11 -0800	[thread overview]
Message-ID: <CANgfPd82cPXjsAgLVMhmgjpSpiomWvYdn+x_CVFoc-=wAT0fPQ@mail.gmail.com> (raw)
In-Reply-To: <20211111000310.1435032-3-dmatlack@google.com>

On Wed, Nov 10, 2021 at 4:03 PM David Matlack <dmatlack@google.com> wrote:
>
> From: Sean Christopherson <seanjc@google.com>
>
> Refactor align() to work with non-pointers and split into separate
> helpers for aligning up vs. down. Add align_ptr_up() for use with
> pointers. Expose all helpers so that they can be used by tests and/or
> other utilities.  The align_down() helper in particular will be used to
> ensure gpa alignment for hugepages.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> [Added sepearate up/down helpers and replaced open-coded alignment

Nit: separate

>  bit math throughout the KVM selftests.]
> Signed-off-by: David Matlack <dmatlack@google.com>

Reviewed-by: Ben Gardon <bgardon@google.com>

This is wonderful.

> ---
>  tools/testing/selftests/kvm/dirty_log_test.c  |  6 ++---
>  .../testing/selftests/kvm/include/test_util.h | 25 +++++++++++++++++++
>  .../selftests/kvm/kvm_page_table_test.c       |  2 +-
>  tools/testing/selftests/kvm/lib/elf.c         |  3 +--
>  tools/testing/selftests/kvm/lib/kvm_util.c    | 13 ++--------
>  .../selftests/kvm/lib/perf_test_util.c        |  4 +--
>  6 files changed, 34 insertions(+), 19 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
> index 792c60e1b17d..3fcd89e195c7 100644
> --- a/tools/testing/selftests/kvm/dirty_log_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_test.c
> @@ -115,7 +115,7 @@ static void guest_code(void)
>                         addr = guest_test_virt_mem;
>                         addr += (READ_ONCE(random_array[i]) % guest_num_pages)
>                                 * guest_page_size;
> -                       addr &= ~(host_page_size - 1);
> +                       addr = align_down(addr, host_page_size);
>                         *(uint64_t *)addr = READ_ONCE(iteration);
>                 }
>
> @@ -737,14 +737,14 @@ static void run_test(enum vm_guest_mode mode, void *arg)
>         if (!p->phys_offset) {
>                 guest_test_phys_mem = (vm_get_max_gfn(vm) -
>                                        guest_num_pages) * guest_page_size;
> -               guest_test_phys_mem &= ~(host_page_size - 1);
> +               guest_test_phys_mem = align_down(guest_test_phys_mem, host_page_size);
>         } else {
>                 guest_test_phys_mem = p->phys_offset;
>         }
>
>  #ifdef __s390x__
>         /* Align to 1M (segment size) */
> -       guest_test_phys_mem &= ~((1 << 20) - 1);
> +       guest_test_phys_mem = align_down(guest_test_phys_mem, 1 << 20);
>  #endif
>
>         pr_info("guest physical test memory offset: 0x%lx\n", guest_test_phys_mem);
> diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
> index f8fddc84c0d3..78c06310cc0e 100644
> --- a/tools/testing/selftests/kvm/include/test_util.h
> +++ b/tools/testing/selftests/kvm/include/test_util.h
> @@ -117,4 +117,29 @@ static inline bool backing_src_is_shared(enum vm_mem_backing_src_type t)
>         return vm_mem_backing_src_alias(t)->flag & MAP_SHARED;
>  }
>
> +/* Aligns x up to the next multiple of size. Size must be a power of 2. */
> +static inline uint64_t align_up(uint64_t x, uint64_t size)
> +{
> +       uint64_t mask = size - 1;
> +
> +       TEST_ASSERT(size != 0 && !(size & (size - 1)),
> +                   "size not a power of 2: %lu", size);
> +       return ((x + mask) & ~mask);
> +}
> +
> +static inline uint64_t align_down(uint64_t x, uint64_t size)
> +{
> +       uint64_t x_aligned_up = align_up(x, size);
> +
> +       if (x == x_aligned_up)
> +               return x;
> +       else
> +               return x_aligned_up - size;
> +}
> +
> +static inline void *align_ptr_up(void *x, size_t size)
> +{
> +       return (void *)align_up((unsigned long)x, size);
> +}
> +
>  #endif /* SELFTEST_KVM_TEST_UTIL_H */
> diff --git a/tools/testing/selftests/kvm/kvm_page_table_test.c b/tools/testing/selftests/kvm/kvm_page_table_test.c
> index 36407cb0ec85..3836322add00 100644
> --- a/tools/testing/selftests/kvm/kvm_page_table_test.c
> +++ b/tools/testing/selftests/kvm/kvm_page_table_test.c
> @@ -280,7 +280,7 @@ static struct kvm_vm *pre_init_before_test(enum vm_guest_mode mode, void *arg)
>  #ifdef __s390x__
>         alignment = max(0x100000, alignment);
>  #endif
> -       guest_test_phys_mem &= ~(alignment - 1);
> +       guest_test_phys_mem = align_down(guest_test_virt_mem, alignment);
>
>         /* Set up the shared data structure test_args */
>         test_args.vm = vm;
> diff --git a/tools/testing/selftests/kvm/lib/elf.c b/tools/testing/selftests/kvm/lib/elf.c
> index eac44f5d0db0..13e8e3dcf984 100644
> --- a/tools/testing/selftests/kvm/lib/elf.c
> +++ b/tools/testing/selftests/kvm/lib/elf.c
> @@ -157,8 +157,7 @@ void kvm_vm_elf_load(struct kvm_vm *vm, const char *filename)
>                         "memsize of 0,\n"
>                         "  phdr index: %u p_memsz: 0x%" PRIx64,
>                         n1, (uint64_t) phdr.p_memsz);
> -               vm_vaddr_t seg_vstart = phdr.p_vaddr;
> -               seg_vstart &= ~(vm_vaddr_t)(vm->page_size - 1);
> +               vm_vaddr_t seg_vstart = align_down(phdr.p_vaddr, vm->page_size);
>                 vm_vaddr_t seg_vend = phdr.p_vaddr + phdr.p_memsz - 1;
>                 seg_vend |= vm->page_size - 1;
>                 size_t seg_size = seg_vend - seg_vstart + 1;
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index b624c24290dd..63375118d48f 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -22,15 +22,6 @@
>
>  static int vcpu_mmap_sz(void);
>
> -/* Aligns x up to the next multiple of size. Size must be a power of 2. */
> -static void *align(void *x, size_t size)
> -{
> -       size_t mask = size - 1;
> -       TEST_ASSERT(size != 0 && !(size & (size - 1)),
> -                   "size not a power of 2: %lu", size);
> -       return (void *) (((size_t) x + mask) & ~mask);
> -}
> -
>  /*
>   * Open KVM_DEV_PATH if available, otherwise exit the entire program.
>   *
> @@ -911,7 +902,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
>                     region->mmap_start, errno);
>
>         /* Align host address */
> -       region->host_mem = align(region->mmap_start, alignment);
> +       region->host_mem = align_ptr_up(region->mmap_start, alignment);
>
>         /* As needed perform madvise */
>         if ((src_type == VM_MEM_SRC_ANONYMOUS ||
> @@ -954,7 +945,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
>                             "mmap of alias failed, errno: %i", errno);
>
>                 /* Align host alias address */
> -               region->host_alias = align(region->mmap_alias, alignment);
> +               region->host_alias = align_ptr_up(region->mmap_alias, alignment);
>         }
>  }
>
> diff --git a/tools/testing/selftests/kvm/lib/perf_test_util.c b/tools/testing/selftests/kvm/lib/perf_test_util.c
> index 0ef80dbdc116..6b8d5020dc54 100644
> --- a/tools/testing/selftests/kvm/lib/perf_test_util.c
> +++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
> @@ -92,10 +92,10 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int vcpus,
>
>         guest_test_phys_mem = (vm_get_max_gfn(vm) - guest_num_pages) *
>                               perf_test_args.guest_page_size;
> -       guest_test_phys_mem &= ~(perf_test_args.host_page_size - 1);
> +       guest_test_phys_mem = align_down(guest_test_phys_mem, perf_test_args.host_page_size);
>  #ifdef __s390x__
>         /* Align to 1M (segment size) */
> -       guest_test_phys_mem &= ~((1 << 20) - 1);
> +       guest_test_phys_mem = align_down(guest_test_phys_mem, 1 << 20);
>  #endif
>         pr_info("guest physical test memory offset: 0x%lx\n", guest_test_phys_mem);
>
> --
> 2.34.0.rc1.387.gb447b232ab-goog
>

  reply	other threads:[~2021-11-11 17:43 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-11  0:02 [PATCH v2 00/12] KVM: selftests: Hugepage fixes and cleanups David Matlack
2021-11-11  0:02 ` [PATCH v2 01/12] KVM: selftests: Explicitly state indicies for vm_guest_mode_params array David Matlack
2021-11-11  0:03 ` [PATCH v2 02/12] KVM: selftests: Expose align() helpers to tests David Matlack
2021-11-11 17:43   ` Ben Gardon [this message]
2021-11-11  0:03 ` [PATCH v2 03/12] KVM: selftests: Assert mmap HVA is aligned when using HugeTLB David Matlack
2021-11-11  0:03 ` [PATCH v2 04/12] KVM: selftests: Require GPA to be aligned when backed by hugepages David Matlack
2021-11-11 17:49   ` Ben Gardon
2021-11-11 19:21     ` Sean Christopherson
2021-11-11  0:03 ` [PATCH v2 05/12] KVM: selftests: Use shorthand local var to access struct perf_tests_args David Matlack
2021-11-11  0:03 ` [PATCH v2 06/12] KVM: selftests: Capture per-vCPU GPA in perf_test_vcpu_args David Matlack
2021-11-11  0:03 ` [PATCH v2 07/12] KVM: selftests: Use perf util's per-vCPU GPA/pages in demand paging test David Matlack
2021-11-11  0:03 ` [PATCH v2 08/12] KVM: selftests: Move per-VM GPA into perf_test_args David Matlack
2021-11-11  0:03 ` [PATCH v2 09/12] KVM: selftests: Remove perf_test_args.host_page_size David Matlack
2021-11-11  0:03 ` [PATCH v2 10/12] KVM: selftests: Create VM with adjusted number of guest pages for perf tests David Matlack
2021-11-11  0:03 ` [PATCH v2 11/12] KVM: selftests: Fill per-vCPU struct during "perf_test" VM creation David Matlack
2021-11-11 17:53   ` Ben Gardon
2021-11-11  0:03 ` [PATCH v2 12/12] KVM: selftests: Sync perf_test_args to guest during " David Matlack
2021-11-11 17:55   ` Ben Gardon
2021-11-16 11:12 ` [PATCH v2 00/12] KVM: selftests: Hugepage fixes and cleanups Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CANgfPd82cPXjsAgLVMhmgjpSpiomWvYdn+x_CVFoc-=wAT0fPQ@mail.gmail.com' \
    --to=bgardon@google.com \
    --cc=aaronlewis@google.com \
    --cc=dmatlack@google.com \
    --cc=drjones@redhat.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=peterx@redhat.com \
    --cc=seanjc@google.com \
    --cc=wangyanan55@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.