Sean Christopherson <sean.j.christopherson@xxxxxxxxx> 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@xxxxxxxxxx>
> Suggested-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
--
Vitaly
