* [PATCH 0/2] x86: xstate CPUID guest output adjustments @ 2016-06-01 14:57 Jan Beulich 2016-06-01 15:05 ` [PATCH 1/2] x86: flush high xstate CPUID sub-leaves to zero Jan Beulich 2016-06-01 15:06 ` [PATCH 2/2] x86/HVM: don't calculate XSTATE area sizes in software Jan Beulich 0 siblings, 2 replies; 9+ messages in thread From: Jan Beulich @ 2016-06-01 14:57 UTC (permalink / raw) To: xen-devel; +Cc: Andrew Cooper, Wei Liu 1: flush high xstate CPUID sub-leaves to zero 2: HVM: don't calculate XSTATE area sizes in software Patch 1 is certainly meant for 4.7. Patch 2 would imo also be nice (as replacing the odd software calculation by whatever the hardware returns can only improve overall output), but I wouldn't insist. Signed-off-by: Jan Beulich <jbeulich@suse.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] x86: flush high xstate CPUID sub-leaves to zero 2016-06-01 14:57 [PATCH 0/2] x86: xstate CPUID guest output adjustments Jan Beulich @ 2016-06-01 15:05 ` Jan Beulich 2016-06-01 15:30 ` Andrew Cooper 2016-06-01 15:45 ` Wei Liu 2016-06-01 15:06 ` [PATCH 2/2] x86/HVM: don't calculate XSTATE area sizes in software Jan Beulich 1 sibling, 2 replies; 9+ messages in thread From: Jan Beulich @ 2016-06-01 15:05 UTC (permalink / raw) To: xen-devel; +Cc: Andrew Cooper, Wei Liu [-- Attachment #1: Type: text/plain, Size: 2387 bytes --] In line with other recent changes, these should be fully white listed, requiring us to zero them until the obtain a meaning we support. Without XSAVE support, all xstate sub-leaves should be zero. Also move away from checking host XSAVE support - we really ought to consider the guest flag for that purpose. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -3433,7 +3433,13 @@ void hvm_cpuid(unsigned int input, unsig *edx = v->vcpu_id * 2; break; - case 0xd: + case XSTATE_CPUID: + hvm_cpuid(1, NULL, NULL, &_ecx, NULL); + if ( !(_ecx & cpufeat_mask(X86_FEATURE_XSAVE)) || count >= 63 ) + { + *eax = *ebx = *ecx = *edx = 0; + break; + } /* EBX value of main leaf 0 depends on enabled xsave features */ if ( count == 0 && v->arch.xcr0 ) { --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -928,6 +928,8 @@ void pv_cpuid(struct cpu_user_regs *regs switch ( leaf ) { + uint32_t tmp; + case 0x00000001: c &= pv_featureset[FEATURESET_1c]; d &= pv_featureset[FEATURESET_1d]; @@ -1085,14 +1087,19 @@ void pv_cpuid(struct cpu_user_regs *regs break; case XSTATE_CPUID: - if ( !cpu_has_xsave ) + if ( !((!is_control_domain(currd) && !is_hardware_domain(currd) + ? ({ + uint32_t ecx; + + domain_cpuid(currd, 1, 0, &tmp, &tmp, &ecx, &tmp); + ecx & pv_featureset[FEATURESET_1c]; + }) + : cpuid_ecx(1)) & cpufeat_mask(X86_FEATURE_XSAVE)) || + subleaf >= 63 ) goto unsupported; switch ( subleaf ) { case 0: - { - uint32_t tmp; - /* * Always read CPUID.0xD[ECX=0].EBX from hardware, rather than * domain policy. It varies with enabled xstate, and the correct @@ -1101,7 +1108,6 @@ void pv_cpuid(struct cpu_user_regs *regs if ( !is_control_domain(currd) && !is_hardware_domain(currd) ) cpuid_count(leaf, subleaf, &tmp, &b, &tmp, &tmp); break; - } case 1: a &= pv_featureset[FEATURESET_Da1]; [-- Attachment #2: x86-xstate-CPUID-high-leaves.patch --] [-- Type: text/plain, Size: 2432 bytes --] x86: flush high xstate CPUID sub-leaves to zero In line with other recent changes, these should be fully white listed, requiring us to zero them until the obtain a meaning we support. Without XSAVE support, all xstate sub-leaves should be zero. Also move away from checking host XSAVE support - we really ought to consider the guest flag for that purpose. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -3433,7 +3433,13 @@ void hvm_cpuid(unsigned int input, unsig *edx = v->vcpu_id * 2; break; - case 0xd: + case XSTATE_CPUID: + hvm_cpuid(1, NULL, NULL, &_ecx, NULL); + if ( !(_ecx & cpufeat_mask(X86_FEATURE_XSAVE)) || count >= 63 ) + { + *eax = *ebx = *ecx = *edx = 0; + break; + } /* EBX value of main leaf 0 depends on enabled xsave features */ if ( count == 0 && v->arch.xcr0 ) { --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -928,6 +928,8 @@ void pv_cpuid(struct cpu_user_regs *regs switch ( leaf ) { + uint32_t tmp; + case 0x00000001: c &= pv_featureset[FEATURESET_1c]; d &= pv_featureset[FEATURESET_1d]; @@ -1085,14 +1087,19 @@ void pv_cpuid(struct cpu_user_regs *regs break; case XSTATE_CPUID: - if ( !cpu_has_xsave ) + if ( !((!is_control_domain(currd) && !is_hardware_domain(currd) + ? ({ + uint32_t ecx; + + domain_cpuid(currd, 1, 0, &tmp, &tmp, &ecx, &tmp); + ecx & pv_featureset[FEATURESET_1c]; + }) + : cpuid_ecx(1)) & cpufeat_mask(X86_FEATURE_XSAVE)) || + subleaf >= 63 ) goto unsupported; switch ( subleaf ) { case 0: - { - uint32_t tmp; - /* * Always read CPUID.0xD[ECX=0].EBX from hardware, rather than * domain policy. It varies with enabled xstate, and the correct @@ -1101,7 +1108,6 @@ void pv_cpuid(struct cpu_user_regs *regs if ( !is_control_domain(currd) && !is_hardware_domain(currd) ) cpuid_count(leaf, subleaf, &tmp, &b, &tmp, &tmp); break; - } case 1: a &= pv_featureset[FEATURESET_Da1]; [-- Attachment #3: Type: text/plain, Size: 126 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] x86: flush high xstate CPUID sub-leaves to zero 2016-06-01 15:05 ` [PATCH 1/2] x86: flush high xstate CPUID sub-leaves to zero Jan Beulich @ 2016-06-01 15:30 ` Andrew Cooper 2016-06-01 15:45 ` Wei Liu 1 sibling, 0 replies; 9+ messages in thread From: Andrew Cooper @ 2016-06-01 15:30 UTC (permalink / raw) To: Jan Beulich, xen-devel; +Cc: Wei Liu On 01/06/16 16:05, Jan Beulich wrote: > In line with other recent changes, these should be fully white listed, > requiring us to zero them until the obtain a meaning we support. > > Without XSAVE support, all xstate sub-leaves should be zero. > > Also move away from checking host XSAVE support - we really ought to > consider the guest flag for that purpose. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, with one suggestion > > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -3433,7 +3433,13 @@ void hvm_cpuid(unsigned int input, unsig > *edx = v->vcpu_id * 2; > break; > > - case 0xd: > + case XSTATE_CPUID: > + hvm_cpuid(1, NULL, NULL, &_ecx, NULL); > + if ( !(_ecx & cpufeat_mask(X86_FEATURE_XSAVE)) || count >= 63 ) > + { > + *eax = *ebx = *ecx = *edx = 0; > + break; > + } > /* EBX value of main leaf 0 depends on enabled xsave features */ > if ( count == 0 && v->arch.xcr0 ) > { > --- a/xen/arch/x86/traps.c > +++ b/xen/arch/x86/traps.c > @@ -928,6 +928,8 @@ void pv_cpuid(struct cpu_user_regs *regs > > switch ( leaf ) > { > + uint32_t tmp; > + > case 0x00000001: > c &= pv_featureset[FEATURESET_1c]; > d &= pv_featureset[FEATURESET_1d]; > @@ -1085,14 +1087,19 @@ void pv_cpuid(struct cpu_user_regs *regs > break; > > case XSTATE_CPUID: > - if ( !cpu_has_xsave ) > + if ( !((!is_control_domain(currd) && !is_hardware_domain(currd) I would recommend extra brackets on this line, to avoid the possible mis-interpretation of !is_control_domain(currd) && (!is_hardware_domain(currd) ? ... > + ? ({ > + uint32_t ecx; > + > + domain_cpuid(currd, 1, 0, &tmp, &tmp, &ecx, &tmp); > + ecx & pv_featureset[FEATURESET_1c]; > + }) > + : cpuid_ecx(1)) & cpufeat_mask(X86_FEATURE_XSAVE)) || > + subleaf >= 63 ) This is rather nasty code. I am glad that my longterm plans involve removing it all. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] x86: flush high xstate CPUID sub-leaves to zero 2016-06-01 15:05 ` [PATCH 1/2] x86: flush high xstate CPUID sub-leaves to zero Jan Beulich 2016-06-01 15:30 ` Andrew Cooper @ 2016-06-01 15:45 ` Wei Liu 1 sibling, 0 replies; 9+ messages in thread From: Wei Liu @ 2016-06-01 15:45 UTC (permalink / raw) To: Jan Beulich; +Cc: xen-devel, Wei Liu, Andrew Cooper On Wed, Jun 01, 2016 at 09:05:56AM -0600, Jan Beulich wrote: > In line with other recent changes, these should be fully white listed, > requiring us to zero them until the obtain a meaning we support. > "they obtain ..." > Without XSAVE support, all xstate sub-leaves should be zero. > > Also move away from checking host XSAVE support - we really ought to > consider the guest flag for that purpose. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > Release-acked-by: Wei Liu <wei.liu2@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] x86/HVM: don't calculate XSTATE area sizes in software 2016-06-01 14:57 [PATCH 0/2] x86: xstate CPUID guest output adjustments Jan Beulich 2016-06-01 15:05 ` [PATCH 1/2] x86: flush high xstate CPUID sub-leaves to zero Jan Beulich @ 2016-06-01 15:06 ` Jan Beulich 2016-06-01 15:35 ` Andrew Cooper 1 sibling, 1 reply; 9+ messages in thread From: Jan Beulich @ 2016-06-01 15:06 UTC (permalink / raw) To: xen-devel; +Cc: Andrew Cooper, Wei Liu [-- Attachment #1: Type: text/plain, Size: 2606 bytes --] Use hardware output instead, brining HVM behavior in line with PV one in this regard. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -3362,7 +3362,7 @@ void hvm_cpuid(unsigned int input, unsig switch ( input ) { - unsigned int sub_leaf, _eax, _ebx, _ecx, _edx; + unsigned int _ecx, _edx; case 0x1: /* Fix up VLAPIC details. */ @@ -3440,42 +3440,24 @@ void hvm_cpuid(unsigned int input, unsig *eax = *ebx = *ecx = *edx = 0; break; } - /* EBX value of main leaf 0 depends on enabled xsave features */ - if ( count == 0 && v->arch.xcr0 ) - { - /* reset EBX to default value first */ - *ebx = XSTATE_AREA_MIN_SIZE; - for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ ) - { - if ( !(v->arch.xcr0 & (1ULL << sub_leaf)) ) - continue; - domain_cpuid(d, input, sub_leaf, &_eax, &_ebx, &_ecx, - &_edx); - if ( (_eax + _ebx) > *ebx ) - *ebx = _eax + _ebx; - } - } - - if ( count == 1 ) + switch ( count ) { + case 1: *eax &= hvm_featureset[FEATURESET_Da1]; - - if ( *eax & cpufeat_mask(X86_FEATURE_XSAVES) ) + if ( !(*eax & cpufeat_mask(X86_FEATURE_XSAVES)) ) { - uint64_t xfeatures = v->arch.xcr0 | v->arch.hvm_vcpu.msr_xss; - - *ebx = XSTATE_AREA_MIN_SIZE; - if ( xfeatures & ~XSTATE_FP_SSE ) - for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ ) - if ( xfeatures & (1ULL << sub_leaf) ) - { - if ( test_bit(sub_leaf, &xstate_align) ) - *ebx = ROUNDUP(*ebx, 64); - *ebx += xstate_sizes[sub_leaf]; - } - } - else *ebx = *ecx = *edx = 0; + break; + } + /* fall through */ + case 0: + /* + * Always read CPUID.0xD[ECX=0/1].EBX from hardware, rather than + * domain policy. It varies with enabled xstate, and the correct + * xcr0/xss are in context. + */ + cpuid_count(input, count, &dummy, ebx, &dummy, &dummy); + break; } break; [-- Attachment #2: x86-HVM-xstate-CPUID-simplify.patch --] [-- Type: text/plain, Size: 2658 bytes --] x86/HVM: don't calculate XSTATE area sizes in software Use hardware output instead, brining HVM behavior in line with PV one in this regard. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -3362,7 +3362,7 @@ void hvm_cpuid(unsigned int input, unsig switch ( input ) { - unsigned int sub_leaf, _eax, _ebx, _ecx, _edx; + unsigned int _ecx, _edx; case 0x1: /* Fix up VLAPIC details. */ @@ -3440,42 +3440,24 @@ void hvm_cpuid(unsigned int input, unsig *eax = *ebx = *ecx = *edx = 0; break; } - /* EBX value of main leaf 0 depends on enabled xsave features */ - if ( count == 0 && v->arch.xcr0 ) - { - /* reset EBX to default value first */ - *ebx = XSTATE_AREA_MIN_SIZE; - for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ ) - { - if ( !(v->arch.xcr0 & (1ULL << sub_leaf)) ) - continue; - domain_cpuid(d, input, sub_leaf, &_eax, &_ebx, &_ecx, - &_edx); - if ( (_eax + _ebx) > *ebx ) - *ebx = _eax + _ebx; - } - } - - if ( count == 1 ) + switch ( count ) { + case 1: *eax &= hvm_featureset[FEATURESET_Da1]; - - if ( *eax & cpufeat_mask(X86_FEATURE_XSAVES) ) + if ( !(*eax & cpufeat_mask(X86_FEATURE_XSAVES)) ) { - uint64_t xfeatures = v->arch.xcr0 | v->arch.hvm_vcpu.msr_xss; - - *ebx = XSTATE_AREA_MIN_SIZE; - if ( xfeatures & ~XSTATE_FP_SSE ) - for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ ) - if ( xfeatures & (1ULL << sub_leaf) ) - { - if ( test_bit(sub_leaf, &xstate_align) ) - *ebx = ROUNDUP(*ebx, 64); - *ebx += xstate_sizes[sub_leaf]; - } - } - else *ebx = *ecx = *edx = 0; + break; + } + /* fall through */ + case 0: + /* + * Always read CPUID.0xD[ECX=0/1].EBX from hardware, rather than + * domain policy. It varies with enabled xstate, and the correct + * xcr0/xss are in context. + */ + cpuid_count(input, count, &dummy, ebx, &dummy, &dummy); + break; } break; [-- Attachment #3: Type: text/plain, Size: 126 bytes --] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] x86/HVM: don't calculate XSTATE area sizes in software 2016-06-01 15:06 ` [PATCH 2/2] x86/HVM: don't calculate XSTATE area sizes in software Jan Beulich @ 2016-06-01 15:35 ` Andrew Cooper 2016-06-01 15:47 ` Wei Liu 2016-06-01 15:50 ` Jan Beulich 0 siblings, 2 replies; 9+ messages in thread From: Andrew Cooper @ 2016-06-01 15:35 UTC (permalink / raw) To: Jan Beulich, xen-devel; +Cc: Wei Liu On 01/06/16 16:06, Jan Beulich wrote: > @@ -3440,42 +3440,24 @@ void hvm_cpuid(unsigned int input, unsig > *eax = *ebx = *ecx = *edx = 0; > break; > } > - /* EBX value of main leaf 0 depends on enabled xsave features */ > - if ( count == 0 && v->arch.xcr0 ) > - { > - /* reset EBX to default value first */ > - *ebx = XSTATE_AREA_MIN_SIZE; > - for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ ) > - { > - if ( !(v->arch.xcr0 & (1ULL << sub_leaf)) ) > - continue; > - domain_cpuid(d, input, sub_leaf, &_eax, &_ebx, &_ecx, > - &_edx); > - if ( (_eax + _ebx) > *ebx ) > - *ebx = _eax + _ebx; > - } > - } > - > - if ( count == 1 ) > + switch ( count ) > { > + case 1: > *eax &= hvm_featureset[FEATURESET_Da1]; > - > - if ( *eax & cpufeat_mask(X86_FEATURE_XSAVES) ) > + if ( !(*eax & cpufeat_mask(X86_FEATURE_XSAVES)) ) > { > - uint64_t xfeatures = v->arch.xcr0 | v->arch.hvm_vcpu.msr_xss; > - > - *ebx = XSTATE_AREA_MIN_SIZE; > - if ( xfeatures & ~XSTATE_FP_SSE ) > - for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ ) > - if ( xfeatures & (1ULL << sub_leaf) ) > - { > - if ( test_bit(sub_leaf, &xstate_align) ) > - *ebx = ROUNDUP(*ebx, 64); > - *ebx += xstate_sizes[sub_leaf]; > - } > - } > - else > *ebx = *ecx = *edx = 0; > + break; > + } > + /* fall through */ > + case 0: > + /* > + * Always read CPUID.0xD[ECX=0/1].EBX from hardware, rather than > + * domain policy. It varies with enabled xstate, and the correct > + * xcr0/xss are in context. > + */ > + cpuid_count(input, count, &dummy, ebx, &dummy, &dummy); > + break; It would be helpful for my PKU bugfix if you could avoid collapsing this into a fallthough, as the fallthough would need to be undone. Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] x86/HVM: don't calculate XSTATE area sizes in software 2016-06-01 15:35 ` Andrew Cooper @ 2016-06-01 15:47 ` Wei Liu 2016-06-01 15:50 ` Jan Beulich 1 sibling, 0 replies; 9+ messages in thread From: Wei Liu @ 2016-06-01 15:47 UTC (permalink / raw) To: Andrew Cooper; +Cc: xen-devel, Wei Liu, Jan Beulich On Wed, Jun 01, 2016 at 04:35:44PM +0100, Andrew Cooper wrote: > On 01/06/16 16:06, Jan Beulich wrote: > > @@ -3440,42 +3440,24 @@ void hvm_cpuid(unsigned int input, unsig > > *eax = *ebx = *ecx = *edx = 0; > > break; > > } > > - /* EBX value of main leaf 0 depends on enabled xsave features */ > > - if ( count == 0 && v->arch.xcr0 ) > > - { > > - /* reset EBX to default value first */ > > - *ebx = XSTATE_AREA_MIN_SIZE; > > - for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ ) > > - { > > - if ( !(v->arch.xcr0 & (1ULL << sub_leaf)) ) > > - continue; > > - domain_cpuid(d, input, sub_leaf, &_eax, &_ebx, &_ecx, > > - &_edx); > > - if ( (_eax + _ebx) > *ebx ) > > - *ebx = _eax + _ebx; > > - } > > - } > > - > > - if ( count == 1 ) > > + switch ( count ) > > { > > + case 1: > > *eax &= hvm_featureset[FEATURESET_Da1]; > > - > > - if ( *eax & cpufeat_mask(X86_FEATURE_XSAVES) ) > > + if ( !(*eax & cpufeat_mask(X86_FEATURE_XSAVES)) ) > > { > > - uint64_t xfeatures = v->arch.xcr0 | v->arch.hvm_vcpu.msr_xss; > > - > > - *ebx = XSTATE_AREA_MIN_SIZE; > > - if ( xfeatures & ~XSTATE_FP_SSE ) > > - for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ ) > > - if ( xfeatures & (1ULL << sub_leaf) ) > > - { > > - if ( test_bit(sub_leaf, &xstate_align) ) > > - *ebx = ROUNDUP(*ebx, 64); > > - *ebx += xstate_sizes[sub_leaf]; > > - } > > - } > > - else > > *ebx = *ecx = *edx = 0; > > + break; > > + } > > + /* fall through */ > > + case 0: > > + /* > > + * Always read CPUID.0xD[ECX=0/1].EBX from hardware, rather than > > + * domain policy. It varies with enabled xstate, and the correct > > + * xcr0/xss are in context. > > + */ > > + cpuid_count(input, count, &dummy, ebx, &dummy, &dummy); > > + break; > > It would be helpful for my PKU bugfix if you could avoid collapsing this > into a fallthough, as the fallthough would need to be undone. > Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> > Release-acked-by: Wei Liu <wei.liu2@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] x86/HVM: don't calculate XSTATE area sizes in software 2016-06-01 15:35 ` Andrew Cooper 2016-06-01 15:47 ` Wei Liu @ 2016-06-01 15:50 ` Jan Beulich 2016-06-01 15:57 ` Wei Liu 1 sibling, 1 reply; 9+ messages in thread From: Jan Beulich @ 2016-06-01 15:50 UTC (permalink / raw) To: Andrew Cooper; +Cc: xen-devel, Wei Liu >>> On 01.06.16 at 17:35, <andrew.cooper3@citrix.com> wrote: > On 01/06/16 16:06, Jan Beulich wrote: >> @@ -3440,42 +3440,24 @@ void hvm_cpuid(unsigned int input, unsig >> *eax = *ebx = *ecx = *edx = 0; >> break; >> } >> - /* EBX value of main leaf 0 depends on enabled xsave features */ >> - if ( count == 0 && v->arch.xcr0 ) >> - { >> - /* reset EBX to default value first */ >> - *ebx = XSTATE_AREA_MIN_SIZE; >> - for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ ) >> - { >> - if ( !(v->arch.xcr0 & (1ULL << sub_leaf)) ) >> - continue; >> - domain_cpuid(d, input, sub_leaf, &_eax, &_ebx, &_ecx, >> - &_edx); >> - if ( (_eax + _ebx) > *ebx ) >> - *ebx = _eax + _ebx; >> - } >> - } >> - >> - if ( count == 1 ) >> + switch ( count ) >> { >> + case 1: >> *eax &= hvm_featureset[FEATURESET_Da1]; >> - >> - if ( *eax & cpufeat_mask(X86_FEATURE_XSAVES) ) >> + if ( !(*eax & cpufeat_mask(X86_FEATURE_XSAVES)) ) >> { >> - uint64_t xfeatures = v->arch.xcr0 | v->arch.hvm_vcpu.msr_xss; >> - >> - *ebx = XSTATE_AREA_MIN_SIZE; >> - if ( xfeatures & ~XSTATE_FP_SSE ) >> - for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ ) >> - if ( xfeatures & (1ULL << sub_leaf) ) >> - { >> - if ( test_bit(sub_leaf, &xstate_align) ) >> - *ebx = ROUNDUP(*ebx, 64); >> - *ebx += xstate_sizes[sub_leaf]; >> - } >> - } >> - else >> *ebx = *ecx = *edx = 0; >> + break; >> + } >> + /* fall through */ >> + case 0: >> + /* >> + * Always read CPUID.0xD[ECX=0/1].EBX from hardware, rather than >> + * domain policy. It varies with enabled xstate, and the correct >> + * xcr0/xss are in context. >> + */ >> + cpuid_count(input, count, &dummy, ebx, &dummy, &dummy); >> + break; > > It would be helpful for my PKU bugfix if you could avoid collapsing this > into a fallthough, as the fallthough would need to be undone. > Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Converting this back is easy to do, but I'll nevertheless wait for Wei's opinion re 4.7 inclusion, as otherwise I'll eventually need to rebase on top of yours anyway. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] x86/HVM: don't calculate XSTATE area sizes in software 2016-06-01 15:50 ` Jan Beulich @ 2016-06-01 15:57 ` Wei Liu 0 siblings, 0 replies; 9+ messages in thread From: Wei Liu @ 2016-06-01 15:57 UTC (permalink / raw) To: Jan Beulich; +Cc: Andrew Cooper, Wei Liu, xen-devel On Wed, Jun 01, 2016 at 09:50:10AM -0600, Jan Beulich wrote: > >>> On 01.06.16 at 17:35, <andrew.cooper3@citrix.com> wrote: > > On 01/06/16 16:06, Jan Beulich wrote: > >> @@ -3440,42 +3440,24 @@ void hvm_cpuid(unsigned int input, unsig > >> *eax = *ebx = *ecx = *edx = 0; > >> break; > >> } > >> - /* EBX value of main leaf 0 depends on enabled xsave features */ > >> - if ( count == 0 && v->arch.xcr0 ) > >> - { > >> - /* reset EBX to default value first */ > >> - *ebx = XSTATE_AREA_MIN_SIZE; > >> - for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ ) > >> - { > >> - if ( !(v->arch.xcr0 & (1ULL << sub_leaf)) ) > >> - continue; > >> - domain_cpuid(d, input, sub_leaf, &_eax, &_ebx, &_ecx, > >> - &_edx); > >> - if ( (_eax + _ebx) > *ebx ) > >> - *ebx = _eax + _ebx; > >> - } > >> - } > >> - > >> - if ( count == 1 ) > >> + switch ( count ) > >> { > >> + case 1: > >> *eax &= hvm_featureset[FEATURESET_Da1]; > >> - > >> - if ( *eax & cpufeat_mask(X86_FEATURE_XSAVES) ) > >> + if ( !(*eax & cpufeat_mask(X86_FEATURE_XSAVES)) ) > >> { > >> - uint64_t xfeatures = v->arch.xcr0 | v->arch.hvm_vcpu.msr_xss; > >> - > >> - *ebx = XSTATE_AREA_MIN_SIZE; > >> - if ( xfeatures & ~XSTATE_FP_SSE ) > >> - for ( sub_leaf = 2; sub_leaf < 63; sub_leaf++ ) > >> - if ( xfeatures & (1ULL << sub_leaf) ) > >> - { > >> - if ( test_bit(sub_leaf, &xstate_align) ) > >> - *ebx = ROUNDUP(*ebx, 64); > >> - *ebx += xstate_sizes[sub_leaf]; > >> - } > >> - } > >> - else > >> *ebx = *ecx = *edx = 0; > >> + break; > >> + } > >> + /* fall through */ > >> + case 0: > >> + /* > >> + * Always read CPUID.0xD[ECX=0/1].EBX from hardware, rather than > >> + * domain policy. It varies with enabled xstate, and the correct > >> + * xcr0/xss are in context. > >> + */ > >> + cpuid_count(input, count, &dummy, ebx, &dummy, &dummy); > >> + break; > > > > It would be helpful for my PKU bugfix if you could avoid collapsing this > > into a fallthough, as the fallthough would need to be undone. > > Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> > > Converting this back is easy to do, but I'll nevertheless wait for > Wei's opinion re 4.7 inclusion, as otherwise I'll eventually need to > rebase on top of yours anyway. > I think this is fine for 4.7. And I will leave it to you two to coordinate the rest. Wei. > Jan > _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-06-01 15:57 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-06-01 14:57 [PATCH 0/2] x86: xstate CPUID guest output adjustments Jan Beulich 2016-06-01 15:05 ` [PATCH 1/2] x86: flush high xstate CPUID sub-leaves to zero Jan Beulich 2016-06-01 15:30 ` Andrew Cooper 2016-06-01 15:45 ` Wei Liu 2016-06-01 15:06 ` [PATCH 2/2] x86/HVM: don't calculate XSTATE area sizes in software Jan Beulich 2016-06-01 15:35 ` Andrew Cooper 2016-06-01 15:47 ` Wei Liu 2016-06-01 15:50 ` Jan Beulich 2016-06-01 15:57 ` Wei Liu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).