Messages in this thread Patch in this message | | | Date | Tue, 12 Dec 2023 11:39:38 +0100 | Subject | Re: [PATCH] KVM: selftests: fix supported_flags for aarch64 | From | Paolo Bonzini <> |
| |
On 12/9/23 03:29, Sean Christopherson wrote: > On Fri, Dec 08, 2023, Paolo Bonzini wrote: >> KVM/Arm supports readonly memslots; fix the calculation of >> supported_flags in set_memory_region_test.c, otherwise the >> test fails. > > You got beat by a few hours, and by a better solution ;-) > > https://lore.kernel.org/all/20231208033505.2930064-1-shahuang@redhat.com
Better but also wrong---and my patch has the debatable merit of more clearly exposing the wrongness. Testing individual architectures is bad, but testing __KVM_HAVE_READONLY_MEM makes the test fail when running a new test on an old kernel.
This scenario of course will fail when the test detects a bug, but readonly memory is just new functionality (think of the case where RISC-V starts defining __KVM_HAVE_READONLY_MEM in the future). For new functionality, the right thing to do is one of 1) skip the whole test 2) skip the individual test case 3) code the test to adapt to the old kernel. The third choice is rarely possible, but this is one of the cases in which it _is_ possible.
So, the only good way to do this is to get _all_ supported_flags from KVM_CHECK_EXTENSION(KVM_CAP_USER_MEMORY2). We can change the value returned by KVM_CHECK_EXTENSION because KVM_CAP_USER_MEMORY2 has not been included in any released kernel. Calling KVM_CHECK_EXTENSION subsumes
supported_flags |= KVM_MEM_READONLY; if (kvm_check_cap(KVM_CAP_MEMORY_ATTRIBUTES) & KVM_MEMORY_ATTRIBUTE_PRIVATE) supported_flags |= KVM_MEM_GUEST_MEMFD;
and v2_only_flags would be defined as
const uint32_t v2_only_flags = ~(KVM_MEM_LOG_DIRTY_PAGES | KVM_MEM_READONLY);
(not guaranteed to work in the future, but good enough since new KVM_MEM_* flags are a very rare occurrence). Then, the test checks that the supported flags are consistent with the value returned by KVM_CHECK_EXTENSION.
Shaoqin, would you give it a shot?
Thanks,
Paolo
>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> --- >> tools/testing/selftests/kvm/set_memory_region_test.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/tools/testing/selftests/kvm/set_memory_region_test.c b/tools/testing/selftests/kvm/set_memory_region_test.c >> index 6637a0845acf..dfd1d1e22da3 100644 >> --- a/tools/testing/selftests/kvm/set_memory_region_test.c >> +++ b/tools/testing/selftests/kvm/set_memory_region_test.c >> @@ -333,9 +333,11 @@ static void test_invalid_memory_region_flags(void) >> struct kvm_vm *vm; >> int r, i; >> >> -#ifdef __x86_64__ >> +#if defined __aarch64__ || defined __x86_64__ >> supported_flags |= KVM_MEM_READONLY; >> +#endif >> >> +#ifdef __x86_64__ >> if (kvm_check_cap(KVM_CAP_VM_TYPES) & BIT(KVM_X86_SW_PROTECTED_VM)) >> vm = vm_create_barebones_protected_vm(); >> else >> -- >> 2.39.1 >>
| |