On Tue, Jan 28, 2020 at 1:34 AM Andrew Jones <drjones@xxxxxxxxxx> wrote:
>
> Guests and hosts don't have to have the same page size. This means
> calculations are necessary when selecting the number of guest pages
> to allocate in order to ensure the number is compatible with the
> host. Provide utilities to help with those calculations.
>
> Signed-off-by: Andrew Jones <drjones@xxxxxxxxxx>
Reviewed-by: Ben Gardon <bgardon@xxxxxxxxxx>
> ---
> tools/testing/selftests/kvm/dirty_log_test.c | 10 ++++----
> .../testing/selftests/kvm/include/kvm_util.h | 3 +++
> .../testing/selftests/kvm/include/test_util.h | 2 ++
> tools/testing/selftests/kvm/lib/kvm_util.c | 24 +++++++++++++++++++
> 4 files changed, 33 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
> index 5614222a6628..2383c55a1a1a 100644
> --- a/tools/testing/selftests/kvm/dirty_log_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_test.c
> @@ -178,12 +178,11 @@ static void *vcpu_worker(void *data)
> return NULL;
> }
>
> -static void vm_dirty_log_verify(unsigned long *bmap)
> +static void vm_dirty_log_verify(struct kvm_vm *vm, unsigned long *bmap)
> {
> + uint64_t step = vm_num_host_pages(vm, 1);
> uint64_t page;
> uint64_t *value_ptr;
> - uint64_t step = host_page_size >= guest_page_size ? 1 :
> - guest_page_size / host_page_size;
>
> for (page = 0; page < host_num_pages; page += step) {
> value_ptr = host_test_mem + page * host_page_size;
> @@ -295,8 +294,7 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
> guest_num_pages = (guest_num_pages + 0xff) & ~0xffUL;
> #endif
> host_page_size = getpagesize();
> - host_num_pages = (guest_num_pages * guest_page_size) / host_page_size +
> - !!((guest_num_pages * guest_page_size) % host_page_size);
> + host_num_pages = vm_num_host_pages(vm, guest_num_pages);
>
> if (!phys_offset) {
> guest_test_phys_mem = (vm_get_max_gfn(vm) -
> @@ -369,7 +367,7 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
> kvm_vm_clear_dirty_log(vm, TEST_MEM_SLOT_INDEX, bmap, 0,
> host_num_pages);
> #endif
> - vm_dirty_log_verify(bmap);
> + vm_dirty_log_verify(vm, bmap);
> iteration++;
> sync_global_to_guest(vm, iteration);
> }
> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> index 29cccaf96baf..0d05ade3022c 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -158,6 +158,9 @@ unsigned int vm_get_page_size(struct kvm_vm *vm);
> unsigned int vm_get_page_shift(struct kvm_vm *vm);
> unsigned int vm_get_max_gfn(struct kvm_vm *vm);
>
> +unsigned int vm_num_host_pages(struct kvm_vm *vm, unsigned int num_guest_pages);
> +unsigned int vm_num_guest_pages(struct kvm_vm *vm, unsigned int num_host_pages);
> +
> struct kvm_userspace_memory_region *
> kvm_userspace_memory_region_find(struct kvm_vm *vm, uint64_t start,
> uint64_t end);
> diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
> index a41db6fb7e24..25c27739e085 100644
> --- a/tools/testing/selftests/kvm/include/test_util.h
> +++ b/tools/testing/selftests/kvm/include/test_util.h
> @@ -19,6 +19,8 @@
> #include <fcntl.h>
> #include "kselftest.h"
>
> +#define getpageshift() (__builtin_ffs(getpagesize()) - 1)
> +
> ssize_t test_write(int fd, const void *buf, size_t count);
> ssize_t test_read(int fd, void *buf, size_t count);
> int test_seq_read(const char *path, char **bufp, size_t *sizep);
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index 41cf45416060..d9bca2f1cc95 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -1667,3 +1667,27 @@ unsigned int vm_get_max_gfn(struct kvm_vm *vm)
> {
> return vm->max_gfn;
> }
> +
> +static unsigned int vm_calc_num_pages(unsigned int num_pages,
> + unsigned int page_shift,
> + unsigned int new_page_shift)
> +{
> + unsigned int n = 1 << (new_page_shift - page_shift);
> +
> + if (page_shift >= new_page_shift)
> + return num_pages * (1 << (page_shift - new_page_shift));
> +
> + return num_pages / n + !!(num_pages % n);
> +}
> +
> +unsigned int vm_num_host_pages(struct kvm_vm *vm, unsigned int num_guest_pages)
> +{
> + return vm_calc_num_pages(num_guest_pages, vm_get_page_shift(vm),
> + getpageshift());
> +}
> +
> +unsigned int vm_num_guest_pages(struct kvm_vm *vm, unsigned int num_host_pages)
> +{
> + return vm_calc_num_pages(num_host_pages, getpageshift(),
> + vm_get_page_shift(vm));
> +}
This function appears to be unused. I don't have any opposition to
adding it since it is simple, unlikely to bitrot, and seems like a
useful utility.
> --
> 2.21.1
>
