| Message ID | 20190827131015.21691-5-peterx@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: selftests: Introduce VM_MODE_PXXV48_4K | expand |
On Tue, Aug 27, 2019 at 09:10:15PM +0800, Peter Xu wrote: > Remove the duplication code in run_test() of dirty_log_test because > after some reordering of functions now we can directly use the outcome > of vm_create(). > > Meanwhile, with the new VM_MODE_PXXV48_4K, we can safely revert > b442324b58 too where we stick the x86_64 PA width to 39 bits for > dirty_log_test. > > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > tools/testing/selftests/kvm/dirty_log_test.c | 52 ++----------------- > .../testing/selftests/kvm/include/kvm_util.h | 4 ++ > tools/testing/selftests/kvm/lib/kvm_util.c | 17 ++++++ > 3 files changed, 26 insertions(+), 47 deletions(-) > > diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c > index b2e07a3173b2..73f679bbf082 100644 > --- a/tools/testing/selftests/kvm/dirty_log_test.c > +++ b/tools/testing/selftests/kvm/dirty_log_test.c > @@ -268,10 +268,8 @@ static struct kvm_vm *create_vm(enum vm_guest_mode mode, uint32_t vcpuid, > static void run_test(enum vm_guest_mode mode, unsigned long iterations, > unsigned long interval, uint64_t phys_offset) > { > - unsigned int guest_pa_bits, guest_page_shift; > pthread_t vcpu_thread; > struct kvm_vm *vm; > - uint64_t max_gfn; > unsigned long *bmap; > > /* > @@ -286,54 +284,13 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations, > 2ul << (DIRTY_MEM_BITS - PAGE_SHIFT_4K), > guest_code); > > - switch (mode) { > - case VM_MODE_P52V48_4K: > - case VM_MODE_PXXV48_4K: > - guest_pa_bits = 52; > - guest_page_shift = 12; > - break; > - case VM_MODE_P52V48_64K: > - guest_pa_bits = 52; > - guest_page_shift = 16; > - break; > - case VM_MODE_P48V48_4K: > - guest_pa_bits = 48; > - guest_page_shift = 12; > - break; > - case VM_MODE_P48V48_64K: > - guest_pa_bits = 48; > - guest_page_shift = 16; > - break; > - case VM_MODE_P40V48_4K: > - guest_pa_bits = 40; > - guest_page_shift = 12; > - break; > - case VM_MODE_P40V48_64K: > - guest_pa_bits = 40; > - guest_page_shift = 16; > - break; > - default: > - TEST_ASSERT(false, "Unknown guest mode, mode: 0x%x", mode); > - } > - > - DEBUG("Testing guest mode: %s\n", vm_guest_mode_string(mode)); > - > -#ifdef __x86_64__ > - /* > - * FIXME > - * The x86_64 kvm selftests framework currently only supports a > - * single PML4 which restricts the number of physical address > - * bits we can change to 39. > - */ > - guest_pa_bits = 39; > -#endif > - max_gfn = (1ul << (guest_pa_bits - guest_page_shift)) - 1; > - guest_page_size = (1ul << guest_page_shift); > + guest_page_size = vm_get_page_size(vm); > /* > * A little more than 1G of guest page sized pages. Cover the > * case where the size is not aligned to 64 pages. > */ > - guest_num_pages = (1ul << (DIRTY_MEM_BITS - guest_page_shift)) + 16; > + guest_num_pages = (1ul << (DIRTY_MEM_BITS - > + vm_get_page_shift(vm))) + 16; > #ifdef __s390x__ > /* Round up to multiple of 1M (segment size) */ > guest_num_pages = (guest_num_pages + 0xff) & ~0xffUL; > @@ -343,7 +300,8 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations, > !!((guest_num_pages * guest_page_size) % host_page_size); > > if (!phys_offset) { > - guest_test_phys_mem = (max_gfn - guest_num_pages) * guest_page_size; > + guest_test_phys_mem = (vm_get_max_gfn(vm) - > + guest_num_pages) * guest_page_size; > guest_test_phys_mem &= ~(host_page_size - 1); > } else { > guest_test_phys_mem = phys_offset; > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h > index 1c700c6b31b5..0d65fc676182 100644 > --- a/tools/testing/selftests/kvm/include/kvm_util.h > +++ b/tools/testing/selftests/kvm/include/kvm_util.h > @@ -155,6 +155,10 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code); > > bool vm_is_unrestricted_guest(struct kvm_vm *vm); > > +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); > + > 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/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c > index 8c6f872a8793..cf39643ff2c7 100644 > --- a/tools/testing/selftests/kvm/lib/kvm_util.c > +++ b/tools/testing/selftests/kvm/lib/kvm_util.c > @@ -137,6 +137,8 @@ struct kvm_vm *_vm_create(enum vm_guest_mode mode, uint64_t phy_pages, > { > struct kvm_vm *vm; > > + DEBUG("Testing guest mode: %s\n", vm_guest_mode_string(mode)); > + > vm = calloc(1, sizeof(*vm)); > TEST_ASSERT(vm != NULL, "Insufficient Memory"); > > @@ -1662,3 +1664,18 @@ bool vm_is_unrestricted_guest(struct kvm_vm *vm) > > return val == 'Y'; > } > + > +unsigned int vm_get_page_size(struct kvm_vm *vm) > +{ > + return vm->page_size; > +} > + > +unsigned int vm_get_page_shift(struct kvm_vm *vm) > +{ > + return vm->page_shift; > +} We could get by with just one of the above two, but whatever > + > +unsigned int vm_get_max_gfn(struct kvm_vm *vm) > +{ > + return vm->max_gfn; > +} > -- > 2.21.0 > Reviewed-by: Andrew Jones <drjones@redhat.com>
On Wed, Aug 28, 2019 at 01:46:13PM +0200, Andrew Jones wrote: [...] > > +unsigned int vm_get_page_size(struct kvm_vm *vm) > > +{ > > + return vm->page_size; > > +} > > + > > +unsigned int vm_get_page_shift(struct kvm_vm *vm) > > +{ > > + return vm->page_shift; > > +} > > We could get by with just one of the above two, but whatever Right... and imho if we export kvm_vm struct we don't even any helpers. :) But I didn't touch that. > > + > > +unsigned int vm_get_max_gfn(struct kvm_vm *vm) > > +{ > > + return vm->max_gfn; > > +} > > -- > > 2.21.0 > > > > Reviewed-by: Andrew Jones <drjones@redhat.com> Thanks!
On Thu, Aug 29, 2019 at 10:09:35AM +0800, Peter Xu wrote: > On Wed, Aug 28, 2019 at 01:46:13PM +0200, Andrew Jones wrote: > > [...] > > > > +unsigned int vm_get_page_size(struct kvm_vm *vm) > > > +{ > > > + return vm->page_size; > > > +} > > > + > > > +unsigned int vm_get_page_shift(struct kvm_vm *vm) > > > +{ > > > + return vm->page_shift; > > > +} > > > > We could get by with just one of the above two, but whatever > > Right... and imho if we export kvm_vm struct we don't even any > helpers. :) But I didn't touch that. yeah, I'm starting to wonder if there's much value in keeping the vm and vcpu structures private. I've already had a couple cases where I wanted to write a quick+dirty test that needed the vcpu_fd, so I cheated and included the internal header to get to it. Thanks, drew > > > > + > > > +unsigned int vm_get_max_gfn(struct kvm_vm *vm) > > > +{ > > > + return vm->max_gfn; > > > +} > > > -- > > > 2.21.0 > > > > > > > Reviewed-by: Andrew Jones <drjones@redhat.com> > > Thanks! > > -- > Peter Xu
diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c index b2e07a3173b2..73f679bbf082 100644 --- a/tools/testing/selftests/kvm/dirty_log_test.c +++ b/tools/testing/selftests/kvm/dirty_log_test.c @@ -268,10 +268,8 @@ static struct kvm_vm *create_vm(enum vm_guest_mode mode, uint32_t vcpuid, static void run_test(enum vm_guest_mode mode, unsigned long iterations, unsigned long interval, uint64_t phys_offset) { - unsigned int guest_pa_bits, guest_page_shift; pthread_t vcpu_thread; struct kvm_vm *vm; - uint64_t max_gfn; unsigned long *bmap; /* @@ -286,54 +284,13 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations, 2ul << (DIRTY_MEM_BITS - PAGE_SHIFT_4K), guest_code); - switch (mode) { - case VM_MODE_P52V48_4K: - case VM_MODE_PXXV48_4K: - guest_pa_bits = 52; - guest_page_shift = 12; - break; - case VM_MODE_P52V48_64K: - guest_pa_bits = 52; - guest_page_shift = 16; - break; - case VM_MODE_P48V48_4K: - guest_pa_bits = 48; - guest_page_shift = 12; - break; - case VM_MODE_P48V48_64K: - guest_pa_bits = 48; - guest_page_shift = 16; - break; - case VM_MODE_P40V48_4K: - guest_pa_bits = 40; - guest_page_shift = 12; - break; - case VM_MODE_P40V48_64K: - guest_pa_bits = 40; - guest_page_shift = 16; - break; - default: - TEST_ASSERT(false, "Unknown guest mode, mode: 0x%x", mode); - } - - DEBUG("Testing guest mode: %s\n", vm_guest_mode_string(mode)); - -#ifdef __x86_64__ - /* - * FIXME - * The x86_64 kvm selftests framework currently only supports a - * single PML4 which restricts the number of physical address - * bits we can change to 39. - */ - guest_pa_bits = 39; -#endif - max_gfn = (1ul << (guest_pa_bits - guest_page_shift)) - 1; - guest_page_size = (1ul << guest_page_shift); + guest_page_size = vm_get_page_size(vm); /* * A little more than 1G of guest page sized pages. Cover the * case where the size is not aligned to 64 pages. */ - guest_num_pages = (1ul << (DIRTY_MEM_BITS - guest_page_shift)) + 16; + guest_num_pages = (1ul << (DIRTY_MEM_BITS - + vm_get_page_shift(vm))) + 16; #ifdef __s390x__ /* Round up to multiple of 1M (segment size) */ guest_num_pages = (guest_num_pages + 0xff) & ~0xffUL; @@ -343,7 +300,8 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations, !!((guest_num_pages * guest_page_size) % host_page_size); if (!phys_offset) { - guest_test_phys_mem = (max_gfn - guest_num_pages) * guest_page_size; + guest_test_phys_mem = (vm_get_max_gfn(vm) - + guest_num_pages) * guest_page_size; guest_test_phys_mem &= ~(host_page_size - 1); } else { guest_test_phys_mem = phys_offset; diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h index 1c700c6b31b5..0d65fc676182 100644 --- a/tools/testing/selftests/kvm/include/kvm_util.h +++ b/tools/testing/selftests/kvm/include/kvm_util.h @@ -155,6 +155,10 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code); bool vm_is_unrestricted_guest(struct kvm_vm *vm); +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); + 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/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c index 8c6f872a8793..cf39643ff2c7 100644 --- a/tools/testing/selftests/kvm/lib/kvm_util.c +++ b/tools/testing/selftests/kvm/lib/kvm_util.c @@ -137,6 +137,8 @@ struct kvm_vm *_vm_create(enum vm_guest_mode mode, uint64_t phy_pages, { struct kvm_vm *vm; + DEBUG("Testing guest mode: %s\n", vm_guest_mode_string(mode)); + vm = calloc(1, sizeof(*vm)); TEST_ASSERT(vm != NULL, "Insufficient Memory"); @@ -1662,3 +1664,18 @@ bool vm_is_unrestricted_guest(struct kvm_vm *vm) return val == 'Y'; } + +unsigned int vm_get_page_size(struct kvm_vm *vm) +{ + return vm->page_size; +} + +unsigned int vm_get_page_shift(struct kvm_vm *vm) +{ + return vm->page_shift; +} + +unsigned int vm_get_max_gfn(struct kvm_vm *vm) +{ + return vm->max_gfn; +}
Remove the duplication code in run_test() of dirty_log_test because after some reordering of functions now we can directly use the outcome of vm_create(). Meanwhile, with the new VM_MODE_PXXV48_4K, we can safely revert b442324b58 too where we stick the x86_64 PA width to 39 bits for dirty_log_test. Signed-off-by: Peter Xu <peterx@redhat.com> --- tools/testing/selftests/kvm/dirty_log_test.c | 52 ++----------------- .../testing/selftests/kvm/include/kvm_util.h | 4 ++ tools/testing/selftests/kvm/lib/kvm_util.c | 17 ++++++ 3 files changed, 26 insertions(+), 47 deletions(-)