Sean Christopherson <seanjc@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
> .../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@xxxxxxxxxx>
--
Vitaly
