diff mbox

[v2,1/2] x86/hvm: allow guest to use clflushopt and clwb

Message ID 1451476139-22148-2-git-send-email-haozhong.zhang@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Haozhong Zhang Dec. 30, 2015, 11:48 a.m. UTC
Pass CPU features CLFLUSHOPT and CLWB into HVM domain so that those two
instructions can be used by guest.

The specification of above two instructions can be found in
https://software.intel.com/sites/default/files/managed/0d/53/319433-022.pdf

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
 tools/libxc/xc_cpufeature.h      |  3 ++-
 tools/libxc/xc_cpuid_x86.c       |  4 +++-
 xen/arch/x86/hvm/hvm.c           | 33 +++++++++++++++++++++------------
 xen/include/asm-x86/cpufeature.h |  5 +++++
 4 files changed, 31 insertions(+), 14 deletions(-)

Comments

Andrew Cooper Dec. 30, 2015, 11:53 a.m. UTC | #1
On 30/12/2015 11:48, Haozhong Zhang wrote:
> Pass CPU features CLFLUSHOPT and CLWB into HVM domain so that those two
> instructions can be used by guest.
>
> The specification of above two instructions can be found in
> https://software.intel.com/sites/default/files/managed/0d/53/319433-022.pdf
>
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tian, Kevin Jan. 5, 2016, 7:02 a.m. UTC | #2
> From: Zhang, Haozhong
> Sent: Wednesday, December 30, 2015 7:49 PM
> 
> Pass CPU features CLFLUSHOPT and CLWB into HVM domain so that those two
> instructions can be used by guest.
> 
> The specification of above two instructions can be found in
> https://software.intel.com/sites/default/files/managed/0d/53/319433-022.pdf
> 
> Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Jan Beulich Jan. 7, 2016, 1:49 p.m. UTC | #3
>>> On 30.12.15 at 12:48, <haozhong.zhang@intel.com> wrote:
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -4583,21 +4583,30 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
>              *edx &= ~cpufeat_mask(X86_FEATURE_PSE36);
>          break;
>      case 0x7:
> -        if ( (count == 0) && !cpu_has_smep )
> -            *ebx &= ~cpufeat_mask(X86_FEATURE_SMEP);
> +        if ( count == 0 )
> +        {
> +            if ( !cpu_has_smep )
> +                *ebx &= ~cpufeat_mask(X86_FEATURE_SMEP);
> +
> +            if ( !cpu_has_smap )
> +                *ebx &= ~cpufeat_mask(X86_FEATURE_SMAP);
> +
> +            /* Don't expose MPX to hvm when VMX support is not available */
> +            if ( !(vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) ||
> +                 !(vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS) )
> +                *ebx &= ~cpufeat_mask(X86_FEATURE_MPX);
>  
> -        if ( (count == 0) && !cpu_has_smap )
> -            *ebx &= ~cpufeat_mask(X86_FEATURE_SMAP);
> +            /* Don't expose INVPCID to non-hap hvm. */
> +            if ( !hap_enabled(d) )
> +                *ebx &= ~cpufeat_mask(X86_FEATURE_INVPCID);
>  
> -        /* Don't expose MPX to hvm when VMX support is not available */
> -        if ( (count == 0) &&
> -             (!(vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) ||
> -              !(vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS)) )
> -            *ebx &= ~cpufeat_mask(X86_FEATURE_MPX);
> +            if ( !cpu_has_clflushopt )
> +                *ebx &= ~cpufeat_mask(X86_FEATURE_CLFLUSHOPT);
> +
> +            if ( !cpu_has_clwb )
> +                *ebx &= ~cpufeat_mask(X86_FEATURE_CLWB);

I don't think we need this: Other than other things adjusted here,
there's nothing disabling these two features when found available,
and there are no extra conditions to consider. Otherwise, if we
were to follow this route, quite a bit of code would need to be
added to other case statements in this function. But that's all (I
think) going to be taken care of by Andrew's CPUID leveling series.

Jan
Andrew Cooper Jan. 7, 2016, 2:01 p.m. UTC | #4
On 07/01/16 13:49, Jan Beulich wrote:
>>>> On 30.12.15 at 12:48, <haozhong.zhang@intel.com> wrote:
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -4583,21 +4583,30 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
>>              *edx &= ~cpufeat_mask(X86_FEATURE_PSE36);
>>          break;
>>      case 0x7:
>> -        if ( (count == 0) && !cpu_has_smep )
>> -            *ebx &= ~cpufeat_mask(X86_FEATURE_SMEP);
>> +        if ( count == 0 )
>> +        {
>> +            if ( !cpu_has_smep )
>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_SMEP);
>> +
>> +            if ( !cpu_has_smap )
>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_SMAP);
>> +
>> +            /* Don't expose MPX to hvm when VMX support is not available */
>> +            if ( !(vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) ||
>> +                 !(vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS) )
>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_MPX);
>>  
>> -        if ( (count == 0) && !cpu_has_smap )
>> -            *ebx &= ~cpufeat_mask(X86_FEATURE_SMAP);
>> +            /* Don't expose INVPCID to non-hap hvm. */
>> +            if ( !hap_enabled(d) )
>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_INVPCID);
>>  
>> -        /* Don't expose MPX to hvm when VMX support is not available */
>> -        if ( (count == 0) &&
>> -             (!(vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) ||
>> -              !(vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS)) )
>> -            *ebx &= ~cpufeat_mask(X86_FEATURE_MPX);
>> +            if ( !cpu_has_clflushopt )
>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_CLFLUSHOPT);
>> +
>> +            if ( !cpu_has_clwb )
>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_CLWB);
> I don't think we need this: Other than other things adjusted here,
> there's nothing disabling these two features when found available,
> and there are no extra conditions to consider. Otherwise, if we
> were to follow this route, quite a bit of code would need to be
> added to other case statements in this function. But that's all (I
> think) going to be taken care of by Andrew's CPUID leveling series.

My series does take care of all of this.

I would prefer that these two changes get taken as soon as they are
ready, so I can rebase.

~Andrew
Jan Beulich Jan. 7, 2016, 2:34 p.m. UTC | #5
>>> On 07.01.16 at 15:01, <andrew.cooper3@citrix.com> wrote:
> On 07/01/16 13:49, Jan Beulich wrote:
>>>>> On 30.12.15 at 12:48, <haozhong.zhang@intel.com> wrote:
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -4583,21 +4583,30 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, 
> unsigned int *ebx,
>>>              *edx &= ~cpufeat_mask(X86_FEATURE_PSE36);
>>>          break;
>>>      case 0x7:
>>> -        if ( (count == 0) && !cpu_has_smep )
>>> -            *ebx &= ~cpufeat_mask(X86_FEATURE_SMEP);
>>> +        if ( count == 0 )
>>> +        {
>>> +            if ( !cpu_has_smep )
>>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_SMEP);
>>> +
>>> +            if ( !cpu_has_smap )
>>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_SMAP);
>>> +
>>> +            /* Don't expose MPX to hvm when VMX support is not available */
>>> +            if ( !(vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) ||
>>> +                 !(vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS) )
>>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_MPX);
>>>  
>>> -        if ( (count == 0) && !cpu_has_smap )
>>> -            *ebx &= ~cpufeat_mask(X86_FEATURE_SMAP);
>>> +            /* Don't expose INVPCID to non-hap hvm. */
>>> +            if ( !hap_enabled(d) )
>>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_INVPCID);
>>>  
>>> -        /* Don't expose MPX to hvm when VMX support is not available */
>>> -        if ( (count == 0) &&
>>> -             (!(vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) ||
>>> -              !(vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS)) )
>>> -            *ebx &= ~cpufeat_mask(X86_FEATURE_MPX);
>>> +            if ( !cpu_has_clflushopt )
>>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_CLFLUSHOPT);
>>> +
>>> +            if ( !cpu_has_clwb )
>>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_CLWB);
>> I don't think we need this: Other than other things adjusted here,
>> there's nothing disabling these two features when found available,
>> and there are no extra conditions to consider. Otherwise, if we
>> were to follow this route, quite a bit of code would need to be
>> added to other case statements in this function. But that's all (I
>> think) going to be taken care of by Andrew's CPUID leveling series.
> 
> My series does take care of all of this.
> 
> I would prefer that these two changes get taken as soon as they are
> ready, so I can rebase.

If we don't need the change quoted above, your rebase will actually
be easier to do.

Jan
Haozhong Zhang Jan. 8, 2016, 8:27 a.m. UTC | #6
On 01/07/16 07:34, Jan Beulich wrote:
> >>> On 07.01.16 at 15:01, <andrew.cooper3@citrix.com> wrote:
> > On 07/01/16 13:49, Jan Beulich wrote:
> >>>>> On 30.12.15 at 12:48, <haozhong.zhang@intel.com> wrote:
> >>> --- a/xen/arch/x86/hvm/hvm.c
> >>> +++ b/xen/arch/x86/hvm/hvm.c
> >>> @@ -4583,21 +4583,30 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, 
> > unsigned int *ebx,
> >>>              *edx &= ~cpufeat_mask(X86_FEATURE_PSE36);
> >>>          break;
> >>>      case 0x7:
> >>> -        if ( (count == 0) && !cpu_has_smep )
> >>> -            *ebx &= ~cpufeat_mask(X86_FEATURE_SMEP);
> >>> +        if ( count == 0 )
> >>> +        {
> >>> +            if ( !cpu_has_smep )
> >>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_SMEP);
> >>> +
> >>> +            if ( !cpu_has_smap )
> >>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_SMAP);
> >>> +
> >>> +            /* Don't expose MPX to hvm when VMX support is not available */
> >>> +            if ( !(vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) ||
> >>> +                 !(vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS) )
> >>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_MPX);
> >>>  
> >>> -        if ( (count == 0) && !cpu_has_smap )
> >>> -            *ebx &= ~cpufeat_mask(X86_FEATURE_SMAP);
> >>> +            /* Don't expose INVPCID to non-hap hvm. */
> >>> +            if ( !hap_enabled(d) )
> >>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_INVPCID);
> >>>  
> >>> -        /* Don't expose MPX to hvm when VMX support is not available */
> >>> -        if ( (count == 0) &&
> >>> -             (!(vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) ||
> >>> -              !(vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS)) )
> >>> -            *ebx &= ~cpufeat_mask(X86_FEATURE_MPX);
> >>> +            if ( !cpu_has_clflushopt )
> >>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_CLFLUSHOPT);
> >>> +
> >>> +            if ( !cpu_has_clwb )
> >>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_CLWB);
> >> I don't think we need this: Other than other things adjusted here,
> >> there's nothing disabling these two features when found available,
> >> and there are no extra conditions to consider. Otherwise, if we
> >> were to follow this route, quite a bit of code would need to be
> >> added to other case statements in this function. But that's all (I
> >> think) going to be taken care of by Andrew's CPUID leveling series.
> > 
> > My series does take care of all of this.
> > 
> > I would prefer that these two changes get taken as soon as they are
> > ready, so I can rebase.
> 
> If we don't need the change quoted above, your rebase will actually
> be easier to do.
> 
> Jan
>
I'll remove changes in hvm_cpuid() in the next version. Changes in
xen/include/asm-x86/cpufeature.h will be removed as well, because
there will be no use of them.

Haozhong
Jan Beulich Jan. 8, 2016, 8:39 a.m. UTC | #7
>>> On 08.01.16 at 09:27, <haozhong.zhang@intel.com> wrote:
> On 01/07/16 07:34, Jan Beulich wrote:
>> >>> On 07.01.16 at 15:01, <andrew.cooper3@citrix.com> wrote:
>> > On 07/01/16 13:49, Jan Beulich wrote:
>> >>>>> On 30.12.15 at 12:48, <haozhong.zhang@intel.com> wrote:
>> >>> --- a/xen/arch/x86/hvm/hvm.c
>> >>> +++ b/xen/arch/x86/hvm/hvm.c
>> >>> @@ -4583,21 +4583,30 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, 
>> > unsigned int *ebx,
>> >>>              *edx &= ~cpufeat_mask(X86_FEATURE_PSE36);
>> >>>          break;
>> >>>      case 0x7:
>> >>> -        if ( (count == 0) && !cpu_has_smep )
>> >>> -            *ebx &= ~cpufeat_mask(X86_FEATURE_SMEP);
>> >>> +        if ( count == 0 )
>> >>> +        {
>> >>> +            if ( !cpu_has_smep )
>> >>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_SMEP);
>> >>> +
>> >>> +            if ( !cpu_has_smap )
>> >>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_SMAP);
>> >>> +
>> >>> +            /* Don't expose MPX to hvm when VMX support is not available */
>> >>> +            if ( !(vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) ||
>> >>> +                 !(vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS) )
>> >>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_MPX);
>> >>>  
>> >>> -        if ( (count == 0) && !cpu_has_smap )
>> >>> -            *ebx &= ~cpufeat_mask(X86_FEATURE_SMAP);
>> >>> +            /* Don't expose INVPCID to non-hap hvm. */
>> >>> +            if ( !hap_enabled(d) )
>> >>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_INVPCID);
>> >>>  
>> >>> -        /* Don't expose MPX to hvm when VMX support is not available */
>> >>> -        if ( (count == 0) &&
>> >>> -             (!(vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) ||
>> >>> -              !(vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS)) )
>> >>> -            *ebx &= ~cpufeat_mask(X86_FEATURE_MPX);
>> >>> +            if ( !cpu_has_clflushopt )
>> >>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_CLFLUSHOPT);
>> >>> +
>> >>> +            if ( !cpu_has_clwb )
>> >>> +                *ebx &= ~cpufeat_mask(X86_FEATURE_CLWB);
>> >> I don't think we need this: Other than other things adjusted here,
>> >> there's nothing disabling these two features when found available,
>> >> and there are no extra conditions to consider. Otherwise, if we
>> >> were to follow this route, quite a bit of code would need to be
>> >> added to other case statements in this function. But that's all (I
>> >> think) going to be taken care of by Andrew's CPUID leveling series.
>> > 
>> > My series does take care of all of this.
>> > 
>> > I would prefer that these two changes get taken as soon as they are
>> > ready, so I can rebase.
>> 
>> If we don't need the change quoted above, your rebase will actually
>> be easier to do.
>>
> I'll remove changes in hvm_cpuid() in the next version. Changes in
> xen/include/asm-x86/cpufeature.h will be removed as well, because
> there will be no use of them.

Indeed - this should become a tools only patch.

Jan
diff mbox

Patch

diff --git a/tools/libxc/xc_cpufeature.h b/tools/libxc/xc_cpufeature.h
index c3ddc80..5288ac6 100644
--- a/tools/libxc/xc_cpufeature.h
+++ b/tools/libxc/xc_cpufeature.h
@@ -140,6 +140,7 @@ 
 #define X86_FEATURE_RDSEED      18 /* RDSEED instruction */
 #define X86_FEATURE_ADX         19 /* ADCX, ADOX instructions */
 #define X86_FEATURE_SMAP        20 /* Supervisor Mode Access Protection */
-
+#define X86_FEATURE_CLFLUSHOPT  23 /* CLFLUSHOPT instruction */
+#define X86_FEATURE_CLWB        24 /* CLWB instruction */
 
 #endif /* __LIBXC_CPUFEATURE_H */
diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index 8882c01..fecfd6c 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -426,7 +426,9 @@  static void xc_cpuid_hvm_policy(xc_interface *xch,
                         bitmaskof(X86_FEATURE_RDSEED)  |
                         bitmaskof(X86_FEATURE_ADX)  |
                         bitmaskof(X86_FEATURE_SMAP) |
-                        bitmaskof(X86_FEATURE_FSGSBASE));
+                        bitmaskof(X86_FEATURE_FSGSBASE) |
+                        bitmaskof(X86_FEATURE_CLWB) |
+                        bitmaskof(X86_FEATURE_CLFLUSHOPT));
         } else
             regs[1] = 0;
         regs[0] = regs[2] = regs[3] = 0;
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 21470ec..9099188 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4583,21 +4583,30 @@  void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
             *edx &= ~cpufeat_mask(X86_FEATURE_PSE36);
         break;
     case 0x7:
-        if ( (count == 0) && !cpu_has_smep )
-            *ebx &= ~cpufeat_mask(X86_FEATURE_SMEP);
+        if ( count == 0 )
+        {
+            if ( !cpu_has_smep )
+                *ebx &= ~cpufeat_mask(X86_FEATURE_SMEP);
+
+            if ( !cpu_has_smap )
+                *ebx &= ~cpufeat_mask(X86_FEATURE_SMAP);
+
+            /* Don't expose MPX to hvm when VMX support is not available */
+            if ( !(vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) ||
+                 !(vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS) )
+                *ebx &= ~cpufeat_mask(X86_FEATURE_MPX);
 
-        if ( (count == 0) && !cpu_has_smap )
-            *ebx &= ~cpufeat_mask(X86_FEATURE_SMAP);
+            /* Don't expose INVPCID to non-hap hvm. */
+            if ( !hap_enabled(d) )
+                *ebx &= ~cpufeat_mask(X86_FEATURE_INVPCID);
 
-        /* Don't expose MPX to hvm when VMX support is not available */
-        if ( (count == 0) &&
-             (!(vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) ||
-              !(vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS)) )
-            *ebx &= ~cpufeat_mask(X86_FEATURE_MPX);
+            if ( !cpu_has_clflushopt )
+                *ebx &= ~cpufeat_mask(X86_FEATURE_CLFLUSHOPT);
+
+            if ( !cpu_has_clwb )
+                *ebx &= ~cpufeat_mask(X86_FEATURE_CLWB);
+        }
 
-        /* Don't expose INVPCID to non-hap hvm. */
-        if ( (count == 0) && !hap_enabled(d) )
-            *ebx &= ~cpufeat_mask(X86_FEATURE_INVPCID);
         break;
     case 0xb:
         /* Fix the x2APIC identifier. */
diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h
index ef96514..5818228 100644
--- a/xen/include/asm-x86/cpufeature.h
+++ b/xen/include/asm-x86/cpufeature.h
@@ -162,6 +162,8 @@ 
 #define X86_FEATURE_RDSEED	(7*32+18) /* RDSEED instruction */
 #define X86_FEATURE_ADX		(7*32+19) /* ADCX, ADOX instructions */
 #define X86_FEATURE_SMAP	(7*32+20) /* Supervisor Mode Access Prevention */
+#define X86_FEATURE_CLFLUSHOPT	(7*32+23) /* CLFLUSHOPT instruction */
+#define X86_FEATURE_CLWB	(7*32+24) /* CLWB instruction */
 
 /* Intel-defined CPU features, CPUID level 0x00000007:0 (ecx), word 8 */
 #define X86_FEATURE_PKU	(8*32+ 3) /* Protection Keys for Userspace */
@@ -234,6 +236,9 @@ 
 #define cpu_has_xgetbv1		boot_cpu_has(X86_FEATURE_XGETBV1)
 #define cpu_has_xsaves		boot_cpu_has(X86_FEATURE_XSAVES)
 
+#define cpu_has_clflushopt  boot_cpu_has(X86_FEATURE_CLFLUSHOPT)
+#define cpu_has_clwb        boot_cpu_has(X86_FEATURE_CLWB)
+
 enum _cache_type {
     CACHE_TYPE_NULL = 0,
     CACHE_TYPE_DATA = 1,