| Message ID | 20241003234337.273364-5-seanjc@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: selftests: AVX support + fixes | expand |
Sean Christopherson <seanjc@google.com> writes: > Rework the CR4/CPUID sync test to clear CR4.OSXSAVE, do CPUID, and restore > CR4.OSXSAVE in assembly, so that there is zero chance of AVX instructions > being executed while CR4.OSXSAVE is disabled. This will allow enabling > CR4.OSXSAVE by default for selftests vCPUs as a general means of playing > nice with AVX instructions. > > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > .../kvm/x86_64/cr4_cpuid_sync_test.c | 46 +++++++++++++------ > 1 file changed, 32 insertions(+), 14 deletions(-) > > diff --git a/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c b/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c > index 624dc725e14d..da818afb7031 100644 > --- a/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c > +++ b/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c > @@ -19,15 +19,14 @@ > #include "kvm_util.h" > #include "processor.h" > > -static inline bool cr4_cpuid_is_sync(void) > -{ > - uint64_t cr4 = get_cr4(); > - > - return (this_cpu_has(X86_FEATURE_OSXSAVE) == !!(cr4 & X86_CR4_OSXSAVE)); > -} > +#define MAGIC_HYPERCALL_PORT 0x80 > > static void guest_code(void) > { > + u32 regs[4] = { > + [KVM_CPUID_EAX] = X86_FEATURE_OSXSAVE.function, > + [KVM_CPUID_ECX] = X86_FEATURE_OSXSAVE.index, > + }; > uint64_t cr4; > > /* turn on CR4.OSXSAVE */ > @@ -36,13 +35,29 @@ static void guest_code(void) > set_cr4(cr4); > > /* verify CR4.OSXSAVE == CPUID.OSXSAVE */ > - GUEST_ASSERT(cr4_cpuid_is_sync()); > + GUEST_ASSERT(this_cpu_has(X86_FEATURE_OSXSAVE)); > > - /* notify hypervisor to change CR4 */ > - GUEST_SYNC(0); > + /* > + * Notify hypervisor to clear CR4.0SXSAVE, do CPUID and save output, > + * and then restore CR4. Do this all in assembly to ensure no AVX > + * instructions are executed while OSXSAVE=0. > + */ > + asm volatile ( > + "out %%al, $" __stringify(MAGIC_HYPERCALL_PORT) "\n\t" > + "cpuid\n\t" > + "mov %%rdi, %%cr4\n\t" > + : "+a" (regs[KVM_CPUID_EAX]), > + "=b" (regs[KVM_CPUID_EBX]), > + "+c" (regs[KVM_CPUID_ECX]), > + "=d" (regs[KVM_CPUID_EDX]) > + : "D" (get_cr4()) > + ); > > - /* check again */ > - GUEST_ASSERT(cr4_cpuid_is_sync()); > + /* Verify KVM cleared OSXSAVE in CPUID when it was cleared in CR4. */ > + GUEST_ASSERT(!(regs[X86_FEATURE_OSXSAVE.reg] & BIT(X86_FEATURE_OSXSAVE.bit))); > + > + /* Verify restoring CR4 also restored OSXSAVE in CPUID. */ > + GUEST_ASSERT(this_cpu_has(X86_FEATURE_OSXSAVE)); > > GUEST_DONE(); > } > @@ -62,13 +77,16 @@ int main(int argc, char *argv[]) > vcpu_run(vcpu); > TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO); > > - switch (get_ucall(vcpu, &uc)) { > - case UCALL_SYNC: > + if (vcpu->run->io.port == MAGIC_HYPERCALL_PORT && > + vcpu->run->io.direction == KVM_EXIT_IO_OUT) { > /* emulate hypervisor clearing CR4.OSXSAVE */ > vcpu_sregs_get(vcpu, &sregs); > sregs.cr4 &= ~X86_CR4_OSXSAVE; > vcpu_sregs_set(vcpu, &sregs); > - break; > + continue; > + } > + > + switch (get_ucall(vcpu, &uc)) { > case UCALL_ABORT: > REPORT_GUEST_ASSERT(uc); > break; Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
diff --git a/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c b/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c index 624dc725e14d..da818afb7031 100644 --- a/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c +++ b/tools/testing/selftests/kvm/x86_64/cr4_cpuid_sync_test.c @@ -19,15 +19,14 @@ #include "kvm_util.h" #include "processor.h" -static inline bool cr4_cpuid_is_sync(void) -{ - uint64_t cr4 = get_cr4(); - - return (this_cpu_has(X86_FEATURE_OSXSAVE) == !!(cr4 & X86_CR4_OSXSAVE)); -} +#define MAGIC_HYPERCALL_PORT 0x80 static void guest_code(void) { + u32 regs[4] = { + [KVM_CPUID_EAX] = X86_FEATURE_OSXSAVE.function, + [KVM_CPUID_ECX] = X86_FEATURE_OSXSAVE.index, + }; uint64_t cr4; /* turn on CR4.OSXSAVE */ @@ -36,13 +35,29 @@ static void guest_code(void) set_cr4(cr4); /* verify CR4.OSXSAVE == CPUID.OSXSAVE */ - GUEST_ASSERT(cr4_cpuid_is_sync()); + GUEST_ASSERT(this_cpu_has(X86_FEATURE_OSXSAVE)); - /* notify hypervisor to change CR4 */ - GUEST_SYNC(0); + /* + * Notify hypervisor to clear CR4.0SXSAVE, do CPUID and save output, + * and then restore CR4. Do this all in assembly to ensure no AVX + * instructions are executed while OSXSAVE=0. + */ + asm volatile ( + "out %%al, $" __stringify(MAGIC_HYPERCALL_PORT) "\n\t" + "cpuid\n\t" + "mov %%rdi, %%cr4\n\t" + : "+a" (regs[KVM_CPUID_EAX]), + "=b" (regs[KVM_CPUID_EBX]), + "+c" (regs[KVM_CPUID_ECX]), + "=d" (regs[KVM_CPUID_EDX]) + : "D" (get_cr4()) + ); - /* check again */ - GUEST_ASSERT(cr4_cpuid_is_sync()); + /* Verify KVM cleared OSXSAVE in CPUID when it was cleared in CR4. */ + GUEST_ASSERT(!(regs[X86_FEATURE_OSXSAVE.reg] & BIT(X86_FEATURE_OSXSAVE.bit))); + + /* Verify restoring CR4 also restored OSXSAVE in CPUID. */ + GUEST_ASSERT(this_cpu_has(X86_FEATURE_OSXSAVE)); GUEST_DONE(); } @@ -62,13 +77,16 @@ int main(int argc, char *argv[]) vcpu_run(vcpu); TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO); - switch (get_ucall(vcpu, &uc)) { - case UCALL_SYNC: + if (vcpu->run->io.port == MAGIC_HYPERCALL_PORT && + vcpu->run->io.direction == KVM_EXIT_IO_OUT) { /* emulate hypervisor clearing CR4.OSXSAVE */ vcpu_sregs_get(vcpu, &sregs); sregs.cr4 &= ~X86_CR4_OSXSAVE; vcpu_sregs_set(vcpu, &sregs); - break; + continue; + } + + switch (get_ucall(vcpu, &uc)) { case UCALL_ABORT: REPORT_GUEST_ASSERT(uc); break;
Rework the CR4/CPUID sync test to clear CR4.OSXSAVE, do CPUID, and restore CR4.OSXSAVE in assembly, so that there is zero chance of AVX instructions being executed while CR4.OSXSAVE is disabled. This will allow enabling CR4.OSXSAVE by default for selftests vCPUs as a general means of playing nice with AVX instructions. Signed-off-by: Sean Christopherson <seanjc@google.com> --- .../kvm/x86_64/cr4_cpuid_sync_test.c | 46 +++++++++++++------ 1 file changed, 32 insertions(+), 14 deletions(-)