| Message ID | 20200302235709.27467-49-sean.j.christopherson@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: Introduce KVM cpu caps | expand |
Sean Christopherson <sean.j.christopherson@intel.com> writes: > Remove the code for handling stateful CPUID 0x2 and mark the associated > flags as deprecated. WARN if host CPUID 0x2.0.AL > 1, i.e. if by some > miracle a host with stateful CPUID 0x2 is encountered. > > No known CPU exists that supports hardware accelerated virtualization > _and_ a stateful CPUID 0x2. Barring an extremely contrived nested > virtualization scenario, stateful CPUID support is dead code. > > Suggested-by: Vitaly Kuznetsov <vkuznets@redhat.com> > Suggested-by: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > --- > Documentation/virt/kvm/api.rst | 22 ++-------- > arch/x86/kvm/cpuid.c | 73 ++++++---------------------------- > 2 files changed, 17 insertions(+), 78 deletions(-) > > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst > index ebd383fba939..c38cd9f88237 100644 > --- a/Documentation/virt/kvm/api.rst > +++ b/Documentation/virt/kvm/api.rst > @@ -1574,8 +1574,8 @@ This ioctl would set vcpu's xcr to the value userspace specified. > }; > > #define KVM_CPUID_FLAG_SIGNIFCANT_INDEX BIT(0) > - #define KVM_CPUID_FLAG_STATEFUL_FUNC BIT(1) > - #define KVM_CPUID_FLAG_STATE_READ_NEXT BIT(2) > + #define KVM_CPUID_FLAG_STATEFUL_FUNC BIT(1) /* deprecated */ > + #define KVM_CPUID_FLAG_STATE_READ_NEXT BIT(2) /* deprecated */ > > struct kvm_cpuid_entry2 { > __u32 function; > @@ -1626,13 +1626,6 @@ emulate them efficiently. The fields in each entry are defined as follows: > > KVM_CPUID_FLAG_SIGNIFCANT_INDEX: > if the index field is valid > - KVM_CPUID_FLAG_STATEFUL_FUNC: > - if cpuid for this function returns different values for successive > - invocations; there will be several entries with the same function, > - all with this flag set > - KVM_CPUID_FLAG_STATE_READ_NEXT: > - for KVM_CPUID_FLAG_STATEFUL_FUNC entries, set if this entry is > - the first entry to be read by a cpu > > eax, ebx, ecx, edx: > the values returned by the cpuid instruction for > @@ -3347,8 +3340,8 @@ The member 'flags' is used for passing flags from userspace. > :: > > #define KVM_CPUID_FLAG_SIGNIFCANT_INDEX BIT(0) > - #define KVM_CPUID_FLAG_STATEFUL_FUNC BIT(1) > - #define KVM_CPUID_FLAG_STATE_READ_NEXT BIT(2) > + #define KVM_CPUID_FLAG_STATEFUL_FUNC BIT(1) /* deprecated */ > + #define KVM_CPUID_FLAG_STATE_READ_NEXT BIT(2) /* deprecated */ > > struct kvm_cpuid_entry2 { > __u32 function; > @@ -3394,13 +3387,6 @@ The fields in each entry are defined as follows: > > KVM_CPUID_FLAG_SIGNIFCANT_INDEX: > if the index field is valid > - KVM_CPUID_FLAG_STATEFUL_FUNC: > - if cpuid for this function returns different values for successive > - invocations; there will be several entries with the same function, > - all with this flag set > - KVM_CPUID_FLAG_STATE_READ_NEXT: > - for KVM_CPUID_FLAG_STATEFUL_FUNC entries, set if this entry is > - the first entry to be read by a cpu > > eax, ebx, ecx, edx: > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index b5dce17c070f..49527dbcc90c 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -495,25 +495,16 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function) > * time, with the least-significant byte in EAX enumerating the > * number of times software should do CPUID(2, 0). > * > - * Modern CPUs (quite likely every CPU KVM has *ever* run on) > - * are less idiotic. Intel's SDM states that EAX & 0xff "will > - * always return 01H. Software should ignore this value and not > + * Modern CPUs, i.e. every CPU KVM has *ever* run on are less > + * idiotic. Intel's SDM states that EAX & 0xff "will always > + * return 01H. Software should ignore this value and not > * interpret it as an informational descriptor", while AMD's > * APM states that CPUID(2) is reserved. > + * > + * WARN if a frankenstein CPU that supports virtualization and > + * a stateful CPUID.0x2 is encountered. > */ > - max_idx = entry->eax & 0xff; > - if (likely(max_idx <= 1)) > - break; > - > - entry->flags |= KVM_CPUID_FLAG_STATEFUL_FUNC; > - entry->flags |= KVM_CPUID_FLAG_STATE_READ_NEXT; > - > - for (i = 1; i < max_idx; ++i) { > - entry = do_host_cpuid(array, function, 0); > - if (!entry) > - goto out; > - entry->flags |= KVM_CPUID_FLAG_STATEFUL_FUNC; > - } > + WARN_ON_ONCE((entry->eax & 0xff) > 1); > break; > /* functions 4 and 0x8000001d have additional index. */ > case 4: > @@ -894,58 +885,20 @@ int kvm_dev_ioctl_get_cpuid(struct kvm_cpuid2 *cpuid, > return r; > } > > -static int move_to_next_stateful_cpuid_entry(struct kvm_vcpu *vcpu, int i) > -{ > - struct kvm_cpuid_entry2 *e = &vcpu->arch.cpuid_entries[i]; > - struct kvm_cpuid_entry2 *ej; > - int j = i; > - int nent = vcpu->arch.cpuid_nent; > - > - e->flags &= ~KVM_CPUID_FLAG_STATE_READ_NEXT; > - /* when no next entry is found, the current entry[i] is reselected */ > - do { > - j = (j + 1) % nent; > - ej = &vcpu->arch.cpuid_entries[j]; > - } while (ej->function != e->function); > - > - ej->flags |= KVM_CPUID_FLAG_STATE_READ_NEXT; > - > - return j; > -} > - > -/* find an entry with matching function, matching index (if needed), and that > - * should be read next (if it's stateful) */ > -static int is_matching_cpuid_entry(struct kvm_cpuid_entry2 *e, > - u32 function, u32 index) > -{ > - if (e->function != function) > - return 0; > - if ((e->flags & KVM_CPUID_FLAG_SIGNIFCANT_INDEX) && e->index != index) > - return 0; > - if (unlikely(e->flags & KVM_CPUID_FLAG_STATEFUL_FUNC) && > - !(e->flags & KVM_CPUID_FLAG_STATE_READ_NEXT)) > - return 0; > - return 1; > -} > - > struct kvm_cpuid_entry2 *kvm_find_cpuid_entry(struct kvm_vcpu *vcpu, > u32 function, u32 index) > { > + struct kvm_cpuid_entry2 *e; > int i; > - struct kvm_cpuid_entry2 *best = NULL; > > for (i = 0; i < vcpu->arch.cpuid_nent; ++i) { > - struct kvm_cpuid_entry2 *e; > - > e = &vcpu->arch.cpuid_entries[i]; > - if (is_matching_cpuid_entry(e, function, index)) { > - if (unlikely(e->flags & KVM_CPUID_FLAG_STATEFUL_FUNC)) > - move_to_next_stateful_cpuid_entry(vcpu, i); > - best = e; > - break; > - } > + > + if (e->function == function && (e->index == index || > + !(e->flags & KVM_CPUID_FLAG_SIGNIFCANT_INDEX))) > + return e; > } > - return best; > + return NULL; > } > EXPORT_SYMBOL_GPL(kvm_find_cpuid_entry); Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
On Mon, Mar 2, 2020 at 3:57 PM Sean Christopherson <sean.j.christopherson@intel.com> wrote: > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index b5dce17c070f..49527dbcc90c 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -495,25 +495,16 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function) > * time, with the least-significant byte in EAX enumerating the > * number of times software should do CPUID(2, 0). > * > - * Modern CPUs (quite likely every CPU KVM has *ever* run on) > - * are less idiotic. Intel's SDM states that EAX & 0xff "will > - * always return 01H. Software should ignore this value and not > + * Modern CPUs, i.e. every CPU KVM has *ever* run on are less Nit: missing comma after "run on." Reviewed-by: Jim Mattson <jmattson@google.com>
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst index ebd383fba939..c38cd9f88237 100644 --- a/Documentation/virt/kvm/api.rst +++ b/Documentation/virt/kvm/api.rst @@ -1574,8 +1574,8 @@ This ioctl would set vcpu's xcr to the value userspace specified. }; #define KVM_CPUID_FLAG_SIGNIFCANT_INDEX BIT(0) - #define KVM_CPUID_FLAG_STATEFUL_FUNC BIT(1) - #define KVM_CPUID_FLAG_STATE_READ_NEXT BIT(2) + #define KVM_CPUID_FLAG_STATEFUL_FUNC BIT(1) /* deprecated */ + #define KVM_CPUID_FLAG_STATE_READ_NEXT BIT(2) /* deprecated */ struct kvm_cpuid_entry2 { __u32 function; @@ -1626,13 +1626,6 @@ emulate them efficiently. The fields in each entry are defined as follows: KVM_CPUID_FLAG_SIGNIFCANT_INDEX: if the index field is valid - KVM_CPUID_FLAG_STATEFUL_FUNC: - if cpuid for this function returns different values for successive - invocations; there will be several entries with the same function, - all with this flag set - KVM_CPUID_FLAG_STATE_READ_NEXT: - for KVM_CPUID_FLAG_STATEFUL_FUNC entries, set if this entry is - the first entry to be read by a cpu eax, ebx, ecx, edx: the values returned by the cpuid instruction for @@ -3347,8 +3340,8 @@ The member 'flags' is used for passing flags from userspace. :: #define KVM_CPUID_FLAG_SIGNIFCANT_INDEX BIT(0) - #define KVM_CPUID_FLAG_STATEFUL_FUNC BIT(1) - #define KVM_CPUID_FLAG_STATE_READ_NEXT BIT(2) + #define KVM_CPUID_FLAG_STATEFUL_FUNC BIT(1) /* deprecated */ + #define KVM_CPUID_FLAG_STATE_READ_NEXT BIT(2) /* deprecated */ struct kvm_cpuid_entry2 { __u32 function; @@ -3394,13 +3387,6 @@ The fields in each entry are defined as follows: KVM_CPUID_FLAG_SIGNIFCANT_INDEX: if the index field is valid - KVM_CPUID_FLAG_STATEFUL_FUNC: - if cpuid for this function returns different values for successive - invocations; there will be several entries with the same function, - all with this flag set - KVM_CPUID_FLAG_STATE_READ_NEXT: - for KVM_CPUID_FLAG_STATEFUL_FUNC entries, set if this entry is - the first entry to be read by a cpu eax, ebx, ecx, edx: diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index b5dce17c070f..49527dbcc90c 100644 --- a/arch/x86/kvm/cpuid.c +++ b/arch/x86/kvm/cpuid.c @@ -495,25 +495,16 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function) * time, with the least-significant byte in EAX enumerating the * number of times software should do CPUID(2, 0). * - * Modern CPUs (quite likely every CPU KVM has *ever* run on) - * are less idiotic. Intel's SDM states that EAX & 0xff "will - * always return 01H. Software should ignore this value and not + * Modern CPUs, i.e. every CPU KVM has *ever* run on are less + * idiotic. Intel's SDM states that EAX & 0xff "will always + * return 01H. Software should ignore this value and not * interpret it as an informational descriptor", while AMD's * APM states that CPUID(2) is reserved. + * + * WARN if a frankenstein CPU that supports virtualization and + * a stateful CPUID.0x2 is encountered. */ - max_idx = entry->eax & 0xff; - if (likely(max_idx <= 1)) - break; - - entry->flags |= KVM_CPUID_FLAG_STATEFUL_FUNC; - entry->flags |= KVM_CPUID_FLAG_STATE_READ_NEXT; - - for (i = 1; i < max_idx; ++i) { - entry = do_host_cpuid(array, function, 0); - if (!entry) - goto out; - entry->flags |= KVM_CPUID_FLAG_STATEFUL_FUNC; - } + WARN_ON_ONCE((entry->eax & 0xff) > 1); break; /* functions 4 and 0x8000001d have additional index. */ case 4: @@ -894,58 +885,20 @@ int kvm_dev_ioctl_get_cpuid(struct kvm_cpuid2 *cpuid, return r; } -static int move_to_next_stateful_cpuid_entry(struct kvm_vcpu *vcpu, int i) -{ - struct kvm_cpuid_entry2 *e = &vcpu->arch.cpuid_entries[i]; - struct kvm_cpuid_entry2 *ej; - int j = i; - int nent = vcpu->arch.cpuid_nent; - - e->flags &= ~KVM_CPUID_FLAG_STATE_READ_NEXT; - /* when no next entry is found, the current entry[i] is reselected */ - do { - j = (j + 1) % nent; - ej = &vcpu->arch.cpuid_entries[j]; - } while (ej->function != e->function); - - ej->flags |= KVM_CPUID_FLAG_STATE_READ_NEXT; - - return j; -} - -/* find an entry with matching function, matching index (if needed), and that - * should be read next (if it's stateful) */ -static int is_matching_cpuid_entry(struct kvm_cpuid_entry2 *e, - u32 function, u32 index) -{ - if (e->function != function) - return 0; - if ((e->flags & KVM_CPUID_FLAG_SIGNIFCANT_INDEX) && e->index != index) - return 0; - if (unlikely(e->flags & KVM_CPUID_FLAG_STATEFUL_FUNC) && - !(e->flags & KVM_CPUID_FLAG_STATE_READ_NEXT)) - return 0; - return 1; -} - struct kvm_cpuid_entry2 *kvm_find_cpuid_entry(struct kvm_vcpu *vcpu, u32 function, u32 index) { + struct kvm_cpuid_entry2 *e; int i; - struct kvm_cpuid_entry2 *best = NULL; for (i = 0; i < vcpu->arch.cpuid_nent; ++i) { - struct kvm_cpuid_entry2 *e; - e = &vcpu->arch.cpuid_entries[i]; - if (is_matching_cpuid_entry(e, function, index)) { - if (unlikely(e->flags & KVM_CPUID_FLAG_STATEFUL_FUNC)) - move_to_next_stateful_cpuid_entry(vcpu, i); - best = e; - break; - } + + if (e->function == function && (e->index == index || + !(e->flags & KVM_CPUID_FLAG_SIGNIFCANT_INDEX))) + return e; } - return best; + return NULL; } EXPORT_SYMBOL_GPL(kvm_find_cpuid_entry);
Remove the code for handling stateful CPUID 0x2 and mark the associated flags as deprecated. WARN if host CPUID 0x2.0.AL > 1, i.e. if by some miracle a host with stateful CPUID 0x2 is encountered. No known CPU exists that supports hardware accelerated virtualization _and_ a stateful CPUID 0x2. Barring an extremely contrived nested virtualization scenario, stateful CPUID support is dead code. Suggested-by: Vitaly Kuznetsov <vkuznets@redhat.com> Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> --- Documentation/virt/kvm/api.rst | 22 ++-------- arch/x86/kvm/cpuid.c | 73 ++++++---------------------------- 2 files changed, 17 insertions(+), 78 deletions(-)