On Wed, Jan 29, 2020 at 03:46:39PM -0800, Sean Christopherson wrote:
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index d06fb54c9c0d..ca766c460318 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -409,7 +409,6 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
> unsigned f_gbpages = 0;
> unsigned f_lm = 0;
> #endif
> - unsigned f_intel_pt = kvm_x86_ops->pt_supported() ? F(INTEL_PT) : 0;
>
> /* cpuid 1.edx */
> const u32 kvm_cpuid_1_edx_x86_features =
> @@ -648,22 +647,8 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
> break;
> }
> /* Intel PT */
> - case 0x14: {
> - int t, times = entry->eax;
> -
> - if (!f_intel_pt) {
> - entry->eax = entry->ebx = entry->ecx = entry->edx = 0;
> - break;
> - }
> -
> - for (t = 1; t <= times; ++t) {
> - if (*nent >= maxnent)
> - goto out;
> - do_host_cpuid(&entry[t], function, t);
> - ++*nent;
> - }
> + case 0x14:
> break;
> - }
> case KVM_CPUID_SIGNATURE: {
> static const char signature[12] = "KVMKVMKVM\0\0";
> const u32 *sigptr = (const u32 *)signature;
> @@ -778,6 +763,21 @@ static inline int __do_cpuid_func(struct kvm_cpuid_entry2 *entry, u32 function,
>
> kvm_x86_ops->set_supported_cpuid(entry);
>
> + /*
> + * Add feature-dependent sub-leafs after ->set_supported_cpuid() to
> + * properly handle the feature being disabled by SVM/VMX.
> + */
> + if (function == 0x14) {
> + int t, times = entry->eax;
> +
> + for (t = 1; t <= times; ++t) {
> + if (*nent >= maxnent)
> + goto out;
> + do_host_cpuid(&entry[t], function, t);
> + ++*nent;
> + }
> + }
> +
> r = 0;
I belatedly thought of an alternative that I think I like better. Instead
of adding the sub-leafs in common code, introduce a new kvm_x86_ops hook to
add vendor specific sub-leafs, e.g.:
kvm_x86_ops->set_supported_cpuid(entry);
r = kvm_x86_ops->add_cpuid_sub_leafs(entry, nent, maxent,
do_host_cpuid);
That gets Intel PT (and SGX if/when it gets merged) sub-leafs out of the
common x86 code without polluting ->set_supported_cpuid with the extra
params and return value. The other hiccup is that SGX will want access to
cpuid_mask(), but I don't see an issue with moving that to cpuid.h.
