Re: [PATCH v3] KVM: x86: Add Intel CPUID.1F cpuid emulation support

[Sean Christopherson <sean.j.christopherson@intel.com>]

On Fri, Apr 26, 2019 at 11:44:46PM +0800, Like Xu wrote:
> Add support to expose Intel V2 Extended Topology Enumeration Leaf for
> some new systems with multiple software-visible die within each package.
> 
> When CPUID executes with EAX set to 1FH, the processor returns information
> about extended topology enumeration data. Software must detect the presence
> of CPUID leaf 1FH by verifying (a) the highest leaf index supported by
> CPUID is >= 1FH, and (b) CPUID.1FH:EBX[15:0] reports a non-zero value.

Nit: When quoting the SDM, it's helpful to explicitly say so, otherwise
readers may assume you're just stating your take on things.

Reviewed-by: Sean Christopherson <sean.j.christopherson@intel.com>

> 
> Co-developed-by: Xiaoyao Li <xiaoyao.li@linux.intel.com>
> Signed-off-by: Xiaoyao Li <xiaoyao.li@linux.intel.com>
> Signed-off-by: Like Xu <like.xu@linux.intel.com>
> ---
> 
> ==changelog==
> v3:
> - Redefine commit message and comment
> 
> v2:
> - Apply cpuid.1f check rule on Intel SDM page 3-222 Vol.2A
> - Add comment to handle 0x1f anf 0xb in common code
> - Reduce check time in a descending-break style
> 
> v1: https://lkml.org/lkml/2019/4/22/28
> 
>  arch/x86/kvm/cpuid.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index fd39516..176a67a 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -425,6 +425,11 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>  
>  	switch (function) {
>  	case 0:
> +		/* Check if the cpuid leaf 0x1f is actually implemented */
> +		if (entry->eax >= 0x1f && (cpuid_ebx(0x1f) & 0x0000ffff)) {
> +			entry->eax = 0x1f;
> +			break;
> +		}
>  		entry->eax = min(entry->eax, (u32)(f_intel_pt ? 0x14 : 0xd));
>  		break;
>  	case 1:
> @@ -544,7 +549,11 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>  		entry->edx = edx.full;
>  		break;
>  	}
> -	/* function 0xb has additional index. */
> +	/*
> +	 * Per Intel's SDM, 0x1f is a superset of 0xb, thus they can be handled
> +	 * by common code.
> +	 */
> +	case 0x1f:
>  	case 0xb: {
>  		int i, level_type;
>  
> -- 
> 1.8.3.1
>