| Message ID | 20240517173926.965351-5-seanjc@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: CPUID overhaul, fixes, and caching | expand |
On Fri, 2024-05-17 at 10:38 -0700, Sean Christopherson wrote: > Rework x86's set sregs test to verify that KVM enforces CPUID vs. CR4 > features even if userspace hasn't explicitly set guest CPUID. KVM used to > allow userspace to set any KVM-supported CR4 value prior to KVM_SET_CPUID2, > and the test verified that behavior. > > However, the testcase was written purely to verify KVM's existing behavior, > i.e. was NOT written to match the needs of real world VMMs. > > Opportunistically verify that KVM continues to reject unsupported features > after KVM_SET_CPUID2 (using KVM_GET_SUPPORTED_CPUID). > > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > .../selftests/kvm/x86_64/set_sregs_test.c | 53 +++++++++++-------- > 1 file changed, 30 insertions(+), 23 deletions(-) > > diff --git a/tools/testing/selftests/kvm/x86_64/set_sregs_test.c b/tools/testing/selftests/kvm/x86_64/set_sregs_test.c > index c021c0795a96..96fd690d479a 100644 > --- a/tools/testing/selftests/kvm/x86_64/set_sregs_test.c > +++ b/tools/testing/selftests/kvm/x86_64/set_sregs_test.c > @@ -41,13 +41,15 @@ do { \ > TEST_ASSERT(!memcmp(&new, &orig, sizeof(new)), "KVM modified sregs"); \ > } while (0) > > +#define KVM_ALWAYS_ALLOWED_CR4 (X86_CR4_VME | X86_CR4_PVI | X86_CR4_TSD | \ > + X86_CR4_DE | X86_CR4_PSE | X86_CR4_PAE | \ > + X86_CR4_MCE | X86_CR4_PGE | X86_CR4_PCE | \ > + X86_CR4_OSFXSR | X86_CR4_OSXMMEXCPT) > + > static uint64_t calc_supported_cr4_feature_bits(void) > { > - uint64_t cr4; > + uint64_t cr4 = KVM_ALWAYS_ALLOWED_CR4; > > - cr4 = X86_CR4_VME | X86_CR4_PVI | X86_CR4_TSD | X86_CR4_DE | > - X86_CR4_PSE | X86_CR4_PAE | X86_CR4_MCE | X86_CR4_PGE | > - X86_CR4_PCE | X86_CR4_OSFXSR | X86_CR4_OSXMMEXCPT; > if (kvm_cpu_has(X86_FEATURE_UMIP)) > cr4 |= X86_CR4_UMIP; > if (kvm_cpu_has(X86_FEATURE_LA57)) > @@ -72,28 +74,14 @@ static uint64_t calc_supported_cr4_feature_bits(void) > return cr4; > } > > -int main(int argc, char *argv[]) > +static void test_cr_bits(struct kvm_vcpu *vcpu, uint64_t cr4) > { > struct kvm_sregs sregs; > - struct kvm_vcpu *vcpu; > - struct kvm_vm *vm; > - uint64_t cr4; > int rc, i; > > - /* > - * Create a dummy VM, specifically to avoid doing KVM_SET_CPUID2, and > - * use it to verify all supported CR4 bits can be set prior to defining > - * the vCPU model, i.e. without doing KVM_SET_CPUID2. > - */ > - vm = vm_create_barebones(); > - vcpu = __vm_vcpu_add(vm, 0); > - > vcpu_sregs_get(vcpu, &sregs); > - > - sregs.cr0 = 0; > - sregs.cr4 |= calc_supported_cr4_feature_bits(); > - cr4 = sregs.cr4; > - > + sregs.cr0 &= ~(X86_CR0_CD | X86_CR0_NW); > + sregs.cr4 |= cr4; > rc = _vcpu_sregs_set(vcpu, &sregs); > TEST_ASSERT(!rc, "Failed to set supported CR4 bits (0x%lx)", cr4); > > @@ -101,7 +89,6 @@ int main(int argc, char *argv[]) > TEST_ASSERT(sregs.cr4 == cr4, "sregs.CR4 (0x%llx) != CR4 (0x%lx)", > sregs.cr4, cr4); > > - /* Verify all unsupported features are rejected by KVM. */ > TEST_INVALID_CR_BIT(vcpu, cr4, sregs, X86_CR4_UMIP); > TEST_INVALID_CR_BIT(vcpu, cr4, sregs, X86_CR4_LA57); > TEST_INVALID_CR_BIT(vcpu, cr4, sregs, X86_CR4_VMXE); > @@ -119,10 +106,28 @@ int main(int argc, char *argv[]) > /* NW without CD is illegal, as is PG without PE. */ > TEST_INVALID_CR_BIT(vcpu, cr0, sregs, X86_CR0_NW); > TEST_INVALID_CR_BIT(vcpu, cr0, sregs, X86_CR0_PG); > +} > > +int main(int argc, char *argv[]) > +{ > + struct kvm_sregs sregs; > + struct kvm_vcpu *vcpu; > + struct kvm_vm *vm; > + int rc; > + > + /* > + * Create a dummy VM, specifically to avoid doing KVM_SET_CPUID2, and > + * use it to verify KVM enforces guest CPUID even if *userspace* never > + * sets CPUID. > + */ > + vm = vm_create_barebones(); > + vcpu = __vm_vcpu_add(vm, 0); > + test_cr_bits(vcpu, KVM_ALWAYS_ALLOWED_CR4); > kvm_vm_free(vm); > > - /* Create a "real" VM and verify APIC_BASE can be set. */ > + /* Create a "real" VM with a fully populated guest CPUID and verify > + * APIC_BASE and all supported CR4 can be set. > + */ > vm = vm_create_with_one_vcpu(&vcpu, NULL); > > vcpu_sregs_get(vcpu, &sregs); > @@ -135,6 +140,8 @@ int main(int argc, char *argv[]) > TEST_ASSERT(!rc, "Couldn't set IA32_APIC_BASE to %llx (valid)", > sregs.apic_base); > > + test_cr_bits(vcpu, calc_supported_cr4_feature_bits()); > + > kvm_vm_free(vm); > > return 0; Makes sense. Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com> Best regards, Maxim Levitsky
diff --git a/tools/testing/selftests/kvm/x86_64/set_sregs_test.c b/tools/testing/selftests/kvm/x86_64/set_sregs_test.c index c021c0795a96..96fd690d479a 100644 --- a/tools/testing/selftests/kvm/x86_64/set_sregs_test.c +++ b/tools/testing/selftests/kvm/x86_64/set_sregs_test.c @@ -41,13 +41,15 @@ do { \ TEST_ASSERT(!memcmp(&new, &orig, sizeof(new)), "KVM modified sregs"); \ } while (0) +#define KVM_ALWAYS_ALLOWED_CR4 (X86_CR4_VME | X86_CR4_PVI | X86_CR4_TSD | \ + X86_CR4_DE | X86_CR4_PSE | X86_CR4_PAE | \ + X86_CR4_MCE | X86_CR4_PGE | X86_CR4_PCE | \ + X86_CR4_OSFXSR | X86_CR4_OSXMMEXCPT) + static uint64_t calc_supported_cr4_feature_bits(void) { - uint64_t cr4; + uint64_t cr4 = KVM_ALWAYS_ALLOWED_CR4; - cr4 = X86_CR4_VME | X86_CR4_PVI | X86_CR4_TSD | X86_CR4_DE | - X86_CR4_PSE | X86_CR4_PAE | X86_CR4_MCE | X86_CR4_PGE | - X86_CR4_PCE | X86_CR4_OSFXSR | X86_CR4_OSXMMEXCPT; if (kvm_cpu_has(X86_FEATURE_UMIP)) cr4 |= X86_CR4_UMIP; if (kvm_cpu_has(X86_FEATURE_LA57)) @@ -72,28 +74,14 @@ static uint64_t calc_supported_cr4_feature_bits(void) return cr4; } -int main(int argc, char *argv[]) +static void test_cr_bits(struct kvm_vcpu *vcpu, uint64_t cr4) { struct kvm_sregs sregs; - struct kvm_vcpu *vcpu; - struct kvm_vm *vm; - uint64_t cr4; int rc, i; - /* - * Create a dummy VM, specifically to avoid doing KVM_SET_CPUID2, and - * use it to verify all supported CR4 bits can be set prior to defining - * the vCPU model, i.e. without doing KVM_SET_CPUID2. - */ - vm = vm_create_barebones(); - vcpu = __vm_vcpu_add(vm, 0); - vcpu_sregs_get(vcpu, &sregs); - - sregs.cr0 = 0; - sregs.cr4 |= calc_supported_cr4_feature_bits(); - cr4 = sregs.cr4; - + sregs.cr0 &= ~(X86_CR0_CD | X86_CR0_NW); + sregs.cr4 |= cr4; rc = _vcpu_sregs_set(vcpu, &sregs); TEST_ASSERT(!rc, "Failed to set supported CR4 bits (0x%lx)", cr4); @@ -101,7 +89,6 @@ int main(int argc, char *argv[]) TEST_ASSERT(sregs.cr4 == cr4, "sregs.CR4 (0x%llx) != CR4 (0x%lx)", sregs.cr4, cr4); - /* Verify all unsupported features are rejected by KVM. */ TEST_INVALID_CR_BIT(vcpu, cr4, sregs, X86_CR4_UMIP); TEST_INVALID_CR_BIT(vcpu, cr4, sregs, X86_CR4_LA57); TEST_INVALID_CR_BIT(vcpu, cr4, sregs, X86_CR4_VMXE); @@ -119,10 +106,28 @@ int main(int argc, char *argv[]) /* NW without CD is illegal, as is PG without PE. */ TEST_INVALID_CR_BIT(vcpu, cr0, sregs, X86_CR0_NW); TEST_INVALID_CR_BIT(vcpu, cr0, sregs, X86_CR0_PG); +} +int main(int argc, char *argv[]) +{ + struct kvm_sregs sregs; + struct kvm_vcpu *vcpu; + struct kvm_vm *vm; + int rc; + + /* + * Create a dummy VM, specifically to avoid doing KVM_SET_CPUID2, and + * use it to verify KVM enforces guest CPUID even if *userspace* never + * sets CPUID. + */ + vm = vm_create_barebones(); + vcpu = __vm_vcpu_add(vm, 0); + test_cr_bits(vcpu, KVM_ALWAYS_ALLOWED_CR4); kvm_vm_free(vm); - /* Create a "real" VM and verify APIC_BASE can be set. */ + /* Create a "real" VM with a fully populated guest CPUID and verify + * APIC_BASE and all supported CR4 can be set. + */ vm = vm_create_with_one_vcpu(&vcpu, NULL); vcpu_sregs_get(vcpu, &sregs); @@ -135,6 +140,8 @@ int main(int argc, char *argv[]) TEST_ASSERT(!rc, "Couldn't set IA32_APIC_BASE to %llx (valid)", sregs.apic_base); + test_cr_bits(vcpu, calc_supported_cr4_feature_bits()); + kvm_vm_free(vm); return 0;
Rework x86's set sregs test to verify that KVM enforces CPUID vs. CR4 features even if userspace hasn't explicitly set guest CPUID. KVM used to allow userspace to set any KVM-supported CR4 value prior to KVM_SET_CPUID2, and the test verified that behavior. However, the testcase was written purely to verify KVM's existing behavior, i.e. was NOT written to match the needs of real world VMMs. Opportunistically verify that KVM continues to reject unsupported features after KVM_SET_CPUID2 (using KVM_GET_SUPPORTED_CPUID). Signed-off-by: Sean Christopherson <seanjc@google.com> --- .../selftests/kvm/x86_64/set_sregs_test.c | 53 +++++++++++-------- 1 file changed, 30 insertions(+), 23 deletions(-)