Re: [PATCH 25/26] KVM: x86: Handle main Intel PT CPUID leaf in vendor code

[Sean Christopherson]

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.