On Thu, May 20, 2021 at 2:47 PM Peter Xu <peterx@xxxxxxxxxx> wrote:
>
> On Thu, May 20, 2021 at 09:26:54PM +0000, David Matlack wrote:
> > vm_get_max_gfn() casts vm->max_gfn from a uint64_t to an unsigned int,
> > which causes the upper 32-bits of the max_gfn to get truncated.
> >
> > Nobody noticed until now likely because vm_get_max_gfn() is only used
> > as a mechanism to create a memslot in an unused region of the guest
> > physical address space (the top), and the top of the 32-bit physical
> > address space was always good enough.
>
> s/top/bottom/?
I guess it depends on your reference point :). The existing comments
under tools/testing/selftests/kvm use the convention that "top" ==
high addresses.
>
> Looks right.. thanks for fixing it!
>
> >
> > This fix reveals a bug in memslot_modification_stress_test which was
> > trying to create a dummy memslot past the end of guest physical memory.
> > Fix that by moving the dummy memslot lower.
>
> Would it be better to split the different fixes?
I'm fine either way. I figured the net delta was small enough and the
fixes tightly coupled so sending as one patch made the most sense. Is
there value in splitting this up?
>
> >
> > Fixes: 52200d0d944e ("KVM: selftests: Remove duplicate guest mode handling")
> > Signed-off-by: David Matlack <dmatlack@xxxxxxxxxx>
> > ---
> > tools/testing/selftests/kvm/include/kvm_util.h | 2 +-
> > tools/testing/selftests/kvm/lib/kvm_util.c | 2 +-
> > .../testing/selftests/kvm/lib/perf_test_util.c | 2 +-
> > .../kvm/memslot_modification_stress_test.c | 18 +++++++++++-------
> > 4 files changed, 14 insertions(+), 10 deletions(-)
> >
> > diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> > index 84982eb02b29..5d9b35d09251 100644
> > --- a/tools/testing/selftests/kvm/include/kvm_util.h
> > +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> > @@ -303,7 +303,7 @@ 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);
> > +uint64_t vm_get_max_gfn(struct kvm_vm *vm);
> > int vm_get_fd(struct kvm_vm *vm);
> >
> > unsigned int vm_calc_num_guest_pages(enum vm_guest_mode mode, size_t size);
> > diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> > index 1af1009254c4..aeffbb1e7c7d 100644
> > --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> > +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> > @@ -2058,7 +2058,7 @@ unsigned int vm_get_page_shift(struct kvm_vm *vm)
> > return vm->page_shift;
> > }
> >
> > -unsigned int vm_get_max_gfn(struct kvm_vm *vm)
> > +uint64_t vm_get_max_gfn(struct kvm_vm *vm)
> > {
> > return vm->max_gfn;
> > }
> > diff --git a/tools/testing/selftests/kvm/lib/perf_test_util.c b/tools/testing/selftests/kvm/lib/perf_test_util.c
> > index 81490b9b4e32..ed4424ed26d6 100644
> > --- a/tools/testing/selftests/kvm/lib/perf_test_util.c
> > +++ b/tools/testing/selftests/kvm/lib/perf_test_util.c
> > @@ -80,7 +80,7 @@ struct kvm_vm *perf_test_create_vm(enum vm_guest_mode mode, int vcpus,
> > */
> > TEST_ASSERT(guest_num_pages < vm_get_max_gfn(vm),
> > "Requested more guest memory than address space allows.\n"
> > - " guest pages: %lx max gfn: %x vcpus: %d wss: %lx]\n",
> > + " guest pages: %lx max gfn: %lx vcpus: %d wss: %lx]\n",
>
> If to fix it, maybe start to use PRIu64 (and include inttypes.h)?
Will do.
Thanks!
>
> Thanks,
>
> --
> Peter Xu
>
