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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 2 Apr 2019 13:27:00 -0700
From:   Sean Christopherson <sean.j.christopherson@...el.com>
To:     Yang Weijiang <weijiang.yang@...el.com>
Cc:     pbonzini@...hat.com, kvm@...r.kernel.org, mst@...hat.com,
        rkrcmar@...hat.com, jmattson@...gle.com,
        linux-kernel@...r.kernel.org, yu-cheng.yu@...el.com,
        Zhang Yi Z <yi.z.zhang@...ux.intel.com>
Subject: Re: [RFC PATCH v4 3/8] KVM:CPUID: Fix xsaves area size calculation
 for CPUID.(EAX=0xD,ECX=1).

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@...ux.intel.com>
> Signed-off-by: Yang Weijiang <weijiang.yang@...el.com>
> ---
>  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
>