| Message ID | 20200128093443.25414-1-drjones@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] kvm: selftests: Introduce num-pages conversion utilities | expand |
On Tue, Jan 28, 2020 at 1:34 AM Andrew Jones <drjones@redhat.com> 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@redhat.com> Reviewed-by: Ben Gardon <bgardon@google.com> > --- > 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 >
On Tue, Jan 28, 2020 at 09:37:21AM -0800, Ben Gardon wrote: > On Tue, Jan 28, 2020 at 1:34 AM Andrew Jones <drjones@redhat.com> 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@redhat.com> > Reviewed-by: Ben Gardon <bgardon@google.com> > > --- > > 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. > Yeah, I was thinking it might be useful for tests to calculate the number of guest pages from a given memory size num_host_pages = DIV_ROUND_UP(memory_size, getpagesize()); num_guest_pages = vm_num_guest_pages(vm, num_host_pages); But now I see we need a v3 of this patch. When calculating the number of guest pages from host pages we should round down, not up. Also, I should have used DIV_ROUND_UP() which is defined in tools/include/linux/kernel.h. I see it's even defined in lib/kvm_util_internal.h too, but we can delete it from there and just use linux/kernel.h. I'll do the deleting as a separate cleanup patch though. Thanks, drew
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)); +}
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@redhat.com> --- 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(-)