Re: [PATCH v2] KVM: selftests: Fix 32-bit truncation of vm_get_max_gfn()

[Christian Borntraeger <borntraeger@de.ibm.com>]

On 08.06.21 10:39, Christian Borntraeger wrote:
> 
> 
> On 21.05.21 19:38, 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.
>>
>> 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.
>>
>> Fixes: 52200d0d944e ("KVM: selftests: Remove duplicate guest mode handling")
>> Reviewed-by: Venkatesh Srinivas <venkateshs@chromium.org>
>> Signed-off-by: David Matlack <dmatlack@google.com>
> 
> As a heads up:
> I have not yet looked into this, but this broke demand_paging_test and kvm_page_table_test
> on s390:
> 
> not ok 4 selftests: kvm: demand_paging_test # exit=254
> # selftests: kvm: dirty_log_test
> # ==== Test Assertion Failure ====
> #   lib/kvm_util.c:900: ret == 0
> #   pid=245410 tid=245410 errno=22 - Invalid argument
> #      1    0x0000000001005457: vm_userspace_mem_region_add at kvm_util.c:900
> #      2    0x0000000001002cbf: run_test at dirty_log_test.c:757
> #      3     (inlined by) run_test at dirty_log_test.c:702
> #      4    0x000000000100c055: for_each_guest_mode at guest_modes.c:37
> #      5    0x00000000010022b5: main at dirty_log_test.c:929 (discriminator 3)
> #      6    0x000003ff96fabdb3: ?? ??:0
> #      7    0x000000000100241d: .annobin_lto.hot at crt1.o:?
> #   KVM_SET_USER_MEMORY_REGION IOCTL failed,
> #   rc: -1 errno: 22
> #   slot: 1 flags: 0x1
> #   guest_phys_addr: 0xfffffbfe00000 size: 0x40100000

Ah. We do have a limit of 128TB for guest physical memory. The patch now made this
apparent as we no longer cut the upper bits off.