From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wanpeng Li Subject: Re: [PATCH 5/9] KVM: cpuid: set CPUID(EAX=0xd,ECX=1).EBX correctly Date: Fri, 5 Dec 2014 08:40:24 +0800 Message-ID: <20141205004024.GB6967@kernel> References: <1417708634-24333-1-git-send-email-pbonzini@redhat.com> <1417708634-24333-6-git-send-email-pbonzini@redhat.com> Reply-To: Wanpeng Li Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, rkrcmar@redhat.com To: Paolo Bonzini Return-path: Content-Disposition: inline In-Reply-To: <1417708634-24333-6-git-send-email-pbonzini@redhat.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: kvm.vger.kernel.org Hi Paolo, On Thu, Dec 04, 2014 at 04:57:10PM +0100, Paolo Bonzini wrote: >This is the size of the XSAVES area. This starts providing guest supp= ort >for XSAVES (with no support yet for supervisor states, i.e. XSS =3D=3D= 0 >always in guests for now). > >Wanpeng Li suggested testing XSAVEC as well as XSAVES, since in practi= ce >no real processor exists that only has one of them, and there is no >other way for userspace programs to compute the area of the XSAVEC >save area. CPUID(EAX=3D0xd,ECX=3D1).EBX provides an upper bound. > >Suggested-by: Radim Kr=C4=8Dm=C3=A1=C5=99 >Reviewed-by: Radim Kr=C4=8Dm=C3=A1=C5=99 >Signed-off-by: Wanpeng Li >Signed-off-by: Paolo Bonzini >--- > arch/x86/kvm/cpuid.c | 22 ++++++++++++++++------ > 1 file changed, 16 insertions(+), 6 deletions(-) > >diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c >index e24df01ab118..2f7bc2de9915 100644 >--- a/arch/x86/kvm/cpuid.c >+++ b/arch/x86/kvm/cpuid.c >@@ -23,7 +23,7 @@ > #include "mmu.h" > #include "trace.h" >=20 >-static u32 xstate_required_size(u64 xstate_bv) >+static u32 xstate_required_size(u64 xstate_bv, bool compacted) > { > int feature_bit =3D 0; > u32 ret =3D XSAVE_HDR_SIZE + XSAVE_HDR_OFFSET; >@@ -31,9 +31,10 @@ static u32 xstate_required_size(u64 xstate_bv) > xstate_bv &=3D XSTATE_EXTEND_MASK; > while (xstate_bv) { > if (xstate_bv & 0x1) { >- u32 eax, ebx, ecx, edx; >+ u32 eax, ebx, ecx, edx, offset; > cpuid_count(0xD, feature_bit, &eax, &ebx, &ecx, &edx); >- ret =3D max(ret, eax + ebx); >+ offset =3D compacted ? ret : ebx; >+ ret =3D max(ret, offset + eax); > } >=20 > xstate_bv >>=3D 1; >@@ -87,9 +88,13 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu) > (best->eax | ((u64)best->edx << 32)) & > kvm_supported_xcr0(); > vcpu->arch.guest_xstate_size =3D best->ebx =3D >- xstate_required_size(vcpu->arch.xcr0); >+ xstate_required_size(vcpu->arch.xcr0, false); > } >=20 >+ best =3D kvm_find_cpuid_entry(vcpu, 0xD, 1); >+ if (best && (best->eax & (F(XSAVES)|F(XSAVEC)))) >+ best->ebx =3D xstate_required_size(vcpu->arch.xcr0, true); >+ > /* > * The existing code assumes virtual address is 48-bit in the canoni= cal > * address checks; exit if it is ever changed. >@@ -470,9 +475,14 @@ static inline int __do_cpuid_ent(struct kvm_cpuid= _entry2 *entry, u32 function, > goto out; >=20 > do_cpuid_1_ent(&entry[i], function, idx); >- if (idx =3D=3D 1) When if (idx =3D=3D 1) is added in this patchset? I suspect that you mi= ss=20 to add your patch "kvm: x86: mask out XSAVES" in this patchset. Regards, Wanpeng Li=20 >+ if (idx =3D=3D 1) { > entry[i].eax &=3D kvm_supported_word10_x86_features; >- else if (entry[i].eax =3D=3D 0 || !(supported & mask)) >+ entry[i].ebx =3D 0; >+ if (entry[i].eax & (F(XSAVES)|F(XSAVEC))) >+ entry[i].ebx =3D >+ xstate_required_size(supported, >+ true); >+ } else if (entry[i].eax =3D=3D 0 || !(supported & mask)) > continue; > entry[i].flags |=3D > KVM_CPUID_FLAG_SIGNIFCANT_INDEX; >--=20 >1.8.3.1 >