* [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
* [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 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 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 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
* 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).