From ae895cbe613a5b193f3953c90c075306bfa4d30b Mon Sep 17 00:00:00 2001 From: Michal Luczaj Date: Fri, 28 Jul 2023 02:12:58 +0200 Subject: [PATCH] KVM: selftests: Extend x86's sync_regs_test to check for CR4 races Attempt to modify vcpu->run->s.regs _after_ the sanity checks performed by KVM_CAP_SYNC_REGS's arch/x86/kvm/x86.c:sync_regs(). This can lead to some nonsensical vCPU states accompanied by kernel splats, e.g. disabling PAE while long mode is enabled makes KVM all kinds of confused: WARNING: CPU: 0 PID: 1142 at arch/x86/kvm/mmu/paging_tmpl.h:358 paging32_walk_addr_generic+0x431/0x8f0 [kvm] arch/x86/kvm/mmu/paging_tmpl.h: KVM_BUG_ON(is_long_mode(vcpu) && !is_pae(vcpu), vcpu->kvm) Signed-off-by: Michal Luczaj Link: https://lore.kernel.org/r/20230728001606.2275586-3-mhal@rbox.co [sean: see link] Signed-off-by: Sean Christopherson --- .../testing/selftests/kvm/x86_64/sync_regs_test.c | 72 ++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/tools/testing/selftests/kvm/x86_64/sync_regs_test.c b/tools/testing/selftests/kvm/x86_64/sync_regs_test.c index 2da89fdc2471..13ac3ae8e704 100644 --- a/tools/testing/selftests/kvm/x86_64/sync_regs_test.c +++ b/tools/testing/selftests/kvm/x86_64/sync_regs_test.c @@ -15,6 +15,7 @@ #include #include #include +#include #include "test_util.h" #include "kvm_util.h" @@ -80,6 +81,75 @@ static void compare_vcpu_events(struct kvm_vcpu_events *left, #define TEST_SYNC_FIELDS (KVM_SYNC_X86_REGS|KVM_SYNC_X86_SREGS|KVM_SYNC_X86_EVENTS) #define INVALID_SYNC_FIELD 0x80000000 +/* + * Toggle CR4.PAE while KVM is processing SREGS, EFER.LME=1 with CR4.PAE=0 is + * illegal, and KVM's MMU heavily relies on vCPU state being valid. + */ +static noinline void *race_sregs_cr4(void *arg) +{ + struct kvm_run *run = (struct kvm_run *)arg; + __u64 *cr4 = &run->s.regs.sregs.cr4; + __u64 pae_enabled = *cr4; + __u64 pae_disabled = *cr4 & ~X86_CR4_PAE; + + for (;;) { + WRITE_ONCE(run->kvm_dirty_regs, KVM_SYNC_X86_SREGS); + WRITE_ONCE(*cr4, pae_enabled); + asm volatile(".rept 512\n\t" + "nop\n\t" + ".endr"); + WRITE_ONCE(*cr4, pae_disabled); + + pthread_testcancel(); + } + + return NULL; +} + +static void race_sync_regs(void *racer) +{ + const time_t TIMEOUT = 2; /* seconds, roughly */ + struct kvm_translation tr; + struct kvm_vcpu *vcpu; + struct kvm_run *run; + struct kvm_vm *vm; + pthread_t thread; + time_t t; + + vm = vm_create_with_one_vcpu(&vcpu, guest_code); + run = vcpu->run; + + run->kvm_valid_regs = KVM_SYNC_X86_SREGS; + vcpu_run(vcpu); + run->kvm_valid_regs = 0; + + /* + * Selftests run 64-bit guests by default, both EFER.LME and CR4.PAE + * should already be set in guest state. + */ + TEST_ASSERT((run->s.regs.sregs.cr4 & X86_CR4_PAE) && + (run->s.regs.sregs.efer & EFER_LME), + "vCPU should be in long mode, CR4.PAE=%d, EFER.LME=%d", + !!(run->s.regs.sregs.cr4 & X86_CR4_PAE), + !!(run->s.regs.sregs.efer & EFER_LME)); + + ASSERT_EQ(pthread_create(&thread, NULL, racer, (void *)run), 0); + + for (t = time(NULL) + TIMEOUT; time(NULL) < t;) { + __vcpu_run(vcpu); + + if (racer == race_sregs_cr4) { + tr = (struct kvm_translation) { .linear_address = 0 }; + __vcpu_ioctl(vcpu, KVM_TRANSLATE, &tr); + } + } + + ASSERT_EQ(pthread_cancel(thread), 0); + ASSERT_EQ(pthread_join(thread, NULL), 0); + + kvm_vm_free(vm); +} + int main(int argc, char *argv[]) { struct kvm_vcpu *vcpu; @@ -218,5 +288,7 @@ int main(int argc, char *argv[]) kvm_vm_free(vm); + race_sync_regs(race_sregs_cr4); + return 0; } -- 2.11.0