Re: [RFC PATCH v4 3/8] KVM:CPUID: Fix xsaves area size calculation for CPUID.(EAX=0xD,ECX=1).

[Sean Christopherson]

On Mon, Mar 18, 2019 at 11:03:46PM +0800, Yang Weijiang wrote:
> According to latest Software Development Manual vol.2/3.2,

Most readers of this code are already familiar with the SDM acronym,
and if they're not, "Intel SDM" provides great search results.  No
need to define it here.

Avoid referencing specific sections, they tend to change.

> for CPUID.(EAX=0xD,ECX=1), it should report xsaves area size

When quoting the SDM, it's usually best to avoid "should" and simply
state what the SDM says.  In other words, the SDM doesn't hedge on
what the processor should (not) do, it documents the exact behavior.

E.g.:

According to the SDM, Vol 2,  CPUID(EAX=0xD,ECX=1) reports the XSAVE are
size containing all states enabled by XCR0|IA32_MSR_XSS.
 
> containing all states enabled  by XCR0|IA32_MSR_XSS.
> 
> Signed-off-by: Zhang Yi Z <yi.z.zhang@xxxxxxxxxxxxxxx>
> Signed-off-by: Yang Weijiang <weijiang.yang@xxxxxxxxx>
> ---
>  arch/x86/kvm/cpuid.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 53abd6019c68..29d6a5cdc746 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -126,7 +126,8 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu)
>  
>  	best = kvm_find_cpuid_entry(vcpu, 0xD, 1);
>  	if (best && (best->eax & (F(XSAVES) | F(XSAVEC))))
> -		best->ebx = xstate_required_size(vcpu->arch.xcr0, true);
> +		best->ebx = xstate_required_size(vcpu->arch.xcr0 |
> +			    kvm_supported_xss(), true);
>  
>  	/*
>  	 * The existing code assumes virtual address is 48-bit or 57-bit in the
> -- 
> 2.17.1
>