Xen devel - Sep 2013 - [PATCH 0/3] Nested VMX: fix bugs during reading VMX MSRs.

From: Yang Zhang <yang.z.zhang@Intel.com>

The following patches fix the issues which existing in current vmx msr reading
logic.

Yang Zhang (3):
  Nested VMX: Check VMX capability before read VMX related MSRs.
  Nested VMX: Clear bit 31 of IA32_VMX_BASIC MSR
  Nested VMX: Fix IA32_VMX_CR4_FIXED1 msr emulation

 xen/arch/x86/hvm/vmx/vvmx.c |   54 ++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 51 insertions(+), 3 deletions(-)

From: Yang Zhang <yang.z.zhang@Intel.com>

VMX MSRs only available when the CPU support the VMX feature. In addition,
VMX_TRUE* MSRs only available when bitt 55 of VMX_BASIC MSR is set.

Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
---
 xen/arch/x86/hvm/vmx/vvmx.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index cecc72f..2e0b7f7 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1820,6 +1820,24 @@ int nvmx_msr_read_intercept(unsigned int msr, u64
*msr_content)
     if ( !nestedhvm_enabled(v->domain) )
         return 0;
 
+    /*
+     * VMX capablitys MSRs available only when guest
+     * support VMX.
+     */
+    hvm_cpuid(0x1, &eax, &ebx, &ecx, &edx);
+    if ( !(ecx & cpufeat_mask(X86_FEATURE_VMXE)) ) 
+        return 0;
+
+    /* 
+     * Those MSRs available only when bit 55 of
+     * MSR_IA32_VMX_BASIC is set.
+     */
+    rdmsrl(MSR_IA32_VMX_BASIC, data);
+    if ( msr >= MSR_IA32_VMX_TRUE_PINBASED_CTLS &&
+            msr <= MSR_IA32_VMX_TRUE_ENTRY_CTLS &&
+            !(data & VMX_BASIC_DEFAULT1_ZERO) )
+        return 0;
+
     rdmsrl(msr, host_data);
 
     /*
-- 
1.7.1

From: Yang Zhang <yang.z.zhang@Intel.com>

The bit 31 of revision_id will set to 1 if vmcs shadowing enabled. And
according intel SDM, the bit 31 of IA32_VMX_BASIC MSR is always 0. So we
cannot set low 32 bit of IA32_VMX_BASIC to revision_id directly. Must clear
the bit 31 to 0.

Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
---
 xen/arch/x86/hvm/vmx/vvmx.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 2e0b7f7..8571002 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1846,7 +1846,7 @@ int nvmx_msr_read_intercept(unsigned int msr, u64
*msr_content)
     switch (msr) {
     case MSR_IA32_VMX_BASIC:
         data = (host_data & (~0ul << 32)) |
-               ((v->arch.hvm_vmx.vmcs)->vmcs_revision_id);
+               ((v->arch.hvm_vmx.vmcs)->vmcs_revision_id & ~(1ul
<< 31));
         break;
     case MSR_IA32_VMX_PINBASED_CTLS:
     case MSR_IA32_VMX_TRUE_PINBASED_CTLS:
-- 
1.7.1

From: Yang Zhang <yang.z.zhang@Intel.com>

Currently, it use hardcode value for IA32_VMX_CR4_FIXED1. This is wrong.
We should check guest''s cpuid to know which bits are writeable in CR4
by guest
and allow the guest to set the corresponding bit only when guest has the
feature.

Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
---
 xen/arch/x86/hvm/vmx/vvmx.c |   34 ++++++++++++++++++++++++++++++++--
 1 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index 8571002..8e53beb 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1815,6 +1815,7 @@ int nvmx_msr_read_intercept(unsigned int msr, u64
*msr_content)
 {
     struct vcpu *v = current;
     u64 data = 0, host_data = 0;
+    unsigned int eax, ebx, ecx, edx;
     int r = 1;
 
     if ( !nestedhvm_enabled(v->domain) )
@@ -1943,8 +1944,37 @@ int nvmx_msr_read_intercept(unsigned int msr, u64
*msr_content)
         data = X86_CR4_VMXE;
         break;
     case MSR_IA32_VMX_CR4_FIXED1:
-        /* allow 0-settings except SMXE */
-        data = 0x267ff & ~X86_CR4_SMXE;
+        data |= (edx & cpufeat_mask(X86_FEATURE_VME) ?
+                                       (X86_CR4_VME | X86_CR4_PVI) : 0) |
+                (edx & cpufeat_mask(X86_FEATURE_TSC) ? X86_CR4_TSD : 0) |
+                (edx & cpufeat_mask(X86_FEATURE_DE)  ? X86_CR4_DE  : 0) |
+                (edx & cpufeat_mask(X86_FEATURE_PSE) ? X86_CR4_PSE : 0) |
+                (edx & cpufeat_mask(X86_FEATURE_PAE) ? X86_CR4_PAE : 0) |
+                (edx & cpufeat_mask(X86_FEATURE_MCE) ? X86_CR4_MCE : 0) |
+                (edx & cpufeat_mask(X86_FEATURE_PGE) ? X86_CR4_PGE : 0) |
+                (edx & cpufeat_mask(X86_FEATURE_FXSR) ? X86_CR4_OSFXSR : 0)
|
+                (edx & cpufeat_mask(X86_FEATURE_XMM) ? X86_CR4_OSXMMEXCPT :
0) |
+                (ecx & cpufeat_mask(X86_FEATURE_VMXE) ? X86_CR4_VMXE : 0) |
+                (ecx & cpufeat_mask(X86_FEATURE_SMXE) ? X86_CR4_SMXE : 0) |
+                (ecx & cpufeat_mask(X86_FEATURE_PCID) ? X86_CR4_PCIDE : 0)
|
+                (ecx & cpufeat_mask(X86_FEATURE_XSAVE) ? X86_CR4_OSXSAVE :
0);
+
+        hvm_cpuid(0x0, &eax, &ebx, &ecx, &edx);
+        if ( eax >= 0xa )
+        {
+            unsigned int temp_eax;
+
+            hvm_cpuid(0xa, &temp_eax, &ebx, &ecx, &edx);
+            /* Check whether guest has the perf monitor feature. */
+            if ( (temp_eax & 0xff) && (temp_eax & 0xff00) )
+                data |= X86_CR4_PCE;
+        } else if ( eax >= 0x7 )
+        {
+            hvm_cpuid(0x7, &eax, &ebx, &ecx, &edx);
+            data |= (ebx & cpufeat_mask(X86_FEATURE_SMEP) ? X86_CR4_SMEP :
0) |
+                    (ebx & cpufeat_mask(X86_FEATURE_FSGSBASE) ?
+                                              X86_CR4_FSGSBASE : 0);
+        }
         break;
     case MSR_IA32_VMX_MISC:
         /* Do not support CR3-target feature now */
-- 
1.7.1

>>> On 05.09.13 at 04:57, Yang Zhang <yang.z.zhang@intel.com>
wrote:
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -1820,6 +1820,24 @@ int nvmx_msr_read_intercept(unsigned int msr, u64 
> *msr_content)
>      if ( !nestedhvm_enabled(v->domain) )
>          return 0;
>  
> +    /*
> +     * VMX capablitys MSRs available only when guest
> +     * support VMX.
> +     */
> +    hvm_cpuid(0x1, &eax, &ebx, &ecx, &edx);
> +    if ( !(ecx & cpufeat_mask(X86_FEATURE_VMXE)) ) 
> +        return 0;
> +
> +    /* 
> +     * Those MSRs available only when bit 55 of
> +     * MSR_IA32_VMX_BASIC is set.
> +     */
> +    rdmsrl(MSR_IA32_VMX_BASIC, data);
> +    if ( msr >= MSR_IA32_VMX_TRUE_PINBASED_CTLS &&
> +            msr <= MSR_IA32_VMX_TRUE_ENTRY_CTLS &&
> +            !(data & VMX_BASIC_DEFAULT1_ZERO) )
Indentation.

Jan
> +        return 0;
> +
>      rdmsrl(msr, host_data);
>  
>      /*

>>> On 05.09.13 at 04:57, Yang Zhang <yang.z.zhang@intel.com>
wrote:
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -1815,6 +1815,7 @@ int nvmx_msr_read_intercept(unsigned int msr, u64
*msr_content)
>  {
>      struct vcpu *v = current;
>      u64 data = 0, host_data = 0;
> +    unsigned int eax, ebx, ecx, edx;
>      int r = 1;
>  
>      if ( !nestedhvm_enabled(v->domain) )
> @@ -1943,8 +1944,37 @@ int nvmx_msr_read_intercept(unsigned int msr, u64
*msr_content)
>          data = X86_CR4_VMXE;
>          break;
>      case MSR_IA32_VMX_CR4_FIXED1:
> -        /* allow 0-settings except SMXE */
> -        data = 0x267ff & ~X86_CR4_SMXE;
> +        data |= (edx & cpufeat_mask(X86_FEATURE_VME) ?
Did you perhaps send a stale patch? I can''t see how this would
even have compiled: edx is uninitialized at this point afaict.
> +                                       (X86_CR4_VME | X86_CR4_PVI) : 0) |
> +                (edx & cpufeat_mask(X86_FEATURE_TSC) ? X86_CR4_TSD :
0) |
> +                (edx & cpufeat_mask(X86_FEATURE_DE)  ? X86_CR4_DE  :
0) |
> +                (edx & cpufeat_mask(X86_FEATURE_PSE) ? X86_CR4_PSE :
0) |
> +                (edx & cpufeat_mask(X86_FEATURE_PAE) ? X86_CR4_PAE :
0) |
> +                (edx & cpufeat_mask(X86_FEATURE_MCE) ? X86_CR4_MCE :
0) |
> +                (edx & cpufeat_mask(X86_FEATURE_PGE) ? X86_CR4_PGE :
0) |
> +                (edx & cpufeat_mask(X86_FEATURE_FXSR) ? X86_CR4_OSFXSR
: 0) |
> +                (edx & cpufeat_mask(X86_FEATURE_XMM) ?
X86_CR4_OSXMMEXCPT : 0) |
> +                (ecx & cpufeat_mask(X86_FEATURE_VMXE) ? X86_CR4_VMXE :
0) |
> +                (ecx & cpufeat_mask(X86_FEATURE_SMXE) ? X86_CR4_SMXE :
0) |
> +                (ecx & cpufeat_mask(X86_FEATURE_PCID) ? X86_CR4_PCIDE
: 0) |
> +                (ecx & cpufeat_mask(X86_FEATURE_XSAVE) ?
X86_CR4_OSXSAVE : 0);
I think this would be more legible if you used a series of "if() data
|=".
Or at least suitably pad the lines so the similar parts align nicely.
> +
> +        hvm_cpuid(0x0, &eax, &ebx, &ecx, &edx);
> +        if ( eax >= 0xa )
> +        {
> +            unsigned int temp_eax;
Why is this needed? You don''t need eax anymore below.
> +
> +            hvm_cpuid(0xa, &temp_eax, &ebx, &ecx, &edx);
> +            /* Check whether guest has the perf monitor feature. */
> +            if ( (temp_eax & 0xff) && (temp_eax & 0xff00)
)
> +                data |= X86_CR4_PCE;
> +        } else if ( eax >= 0x7 )
Coding style. Also, is this really "else if"? If not, _that_ would
explain the (apparent; can be avoided nevertheless) need for
temp_eax above...
> +        {
> +            hvm_cpuid(0x7, &eax, &ebx, &ecx, &edx);
> +            data |= (ebx & cpufeat_mask(X86_FEATURE_SMEP) ?
X86_CR4_SMEP : 0) |
> +                    (ebx & cpufeat_mask(X86_FEATURE_FSGSBASE) ?
> +                                              X86_CR4_FSGSBASE : 0);
Same as above. I''d also like to see SMAP added here right away,
even if for now hvm_cpuid() [hopefully] always returns the
respective bit clear.

Jan

Jan Beulich wrote on 2013-09-05:
>>>> On 05.09.13 at 04:57, Yang Zhang <yang.z.zhang@intel.com>
wrote:
>> --- a/xen/arch/x86/hvm/vmx/vvmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
>> @@ -1815,6 +1815,7 @@ int nvmx_msr_read_intercept(unsigned int msr,
>> u64 *msr_content)  {
>>      struct vcpu *v = current; u64 data = 0, host_data = 0; +   
>>      unsigned int eax, ebx, ecx, edx; int r = 1;
>>      
>>      if ( !nestedhvm_enabled(v->domain) ) @@ -1943,8 +1944,37 @@ int
>> nvmx_msr_read_intercept(unsigned int msr, u64 *msr_content)
>>          data = X86_CR4_VMXE;
>>          break;
>>      case MSR_IA32_VMX_CR4_FIXED1:
>> -        /* allow 0-settings except SMXE */
>> -        data = 0x267ff & ~X86_CR4_SMXE;
>> +        data |= (edx & cpufeat_mask(X86_FEATURE_VME) ?
> 
> Did you perhaps send a stale patch? I can''t see how this would
even have
> compiled: edx is uninitialized at this point afaict.
It is already filled. See the first patch.
> 
>> +                                       (X86_CR4_VME | X86_CR4_PVI) :
>> 0) | +                (edx & cpufeat_mask(X86_FEATURE_TSC) ?
>> X86_CR4_TSD : 0) | +                (edx &
cpufeat_mask(X86_FEATURE_DE)
>>  ? X86_CR4_DE  : 0) | +                (edx &
>> cpufeat_mask(X86_FEATURE_PSE) ? X86_CR4_PSE : 0) | +               
>> (edx & cpufeat_mask(X86_FEATURE_PAE) ? X86_CR4_PAE : 0) | +
>>    (edx & cpufeat_mask(X86_FEATURE_MCE) ? X86_CR4_MCE : 0) | +
>>       (edx & cpufeat_mask(X86_FEATURE_PGE) ? X86_CR4_PGE : 0) | +
>>          (edx & cpufeat_mask(X86_FEATURE_FXSR) ? X86_CR4_OSFXSR :
0) |
>> +                (edx & cpufeat_mask(X86_FEATURE_XMM) ?
>> X86_CR4_OSXMMEXCPT : 0) | +                (ecx &
>> cpufeat_mask(X86_FEATURE_VMXE) ? X86_CR4_VMXE : 0) | +               
>> (ecx & cpufeat_mask(X86_FEATURE_SMXE) ? X86_CR4_SMXE : 0) | +
>>      (ecx & cpufeat_mask(X86_FEATURE_PCID) ? X86_CR4_PCIDE : 0) | +
>>            (ecx & cpufeat_mask(X86_FEATURE_XSAVE) ? +
X86_CR4_OSXSAVE :
>> 0);
> 
> I think this would be more legible if you used a series of "if() data
|=".
> Or at least suitably pad the lines so the similar parts align nicely.
if() should be better. Since the line will exceed the 80 characters if using
pad.
> 
>> +
>> +        hvm_cpuid(0x0, &eax, &ebx, &ecx, &edx);
>> +        if ( eax >= 0xa )
>> +        {
>> +            unsigned int temp_eax;
> 
> Why is this needed? You don''t need eax anymore below.
Good point!
> 
>> +
>> +            hvm_cpuid(0xa, &temp_eax, &ebx, &ecx,
&edx);
>> +            /* Check whether guest has the perf monitor feature. */
>> +            if ( (temp_eax & 0xff) && (temp_eax &
0xff00) )
>> +                data |= X86_CR4_PCE;
>> +        } else if ( eax >= 0x7 )
> 
> Coding style. Also, is this really "else if"? If not, _that_
would
> explain the (apparent; can be avoided nevertheless) need for temp_eax
above...
The different coding style between upstream Linux and Xen really defeats me. :(
> 
>> +        { +            hvm_cpuid(0x7, &eax, &ebx, &ecx,
&edx); +
>>      data |= (ebx & cpufeat_mask(X86_FEATURE_SMEP) ? X86_CR4_SMEP :
0)
>> | +                    (ebx & cpufeat_mask(X86_FEATURE_FSGSBASE) ?
+
> X86_CR4_FSGSBASE : 0);
> 
> Same as above. I''d also like to see SMAP added here right away,
even
> if for now
> hvm_cpuid() [hopefully] always returns the respective bit clear.
Sure.
> 
> Jan

Best regards,
Yang

>>> On 05.09.13 at 11:05, "Zhang, Yang Z"
<yang.z.zhang@intel.com> wrote:
> Jan Beulich wrote on 2013-09-05:
>>>>> On 05.09.13 at 04:57, Yang Zhang
<yang.z.zhang@intel.com> wrote:
>>>      case MSR_IA32_VMX_CR4_FIXED1:
>>> -        /* allow 0-settings except SMXE */
>>> -        data = 0x267ff & ~X86_CR4_SMXE;
>>> +        data |= (edx & cpufeat_mask(X86_FEATURE_VME) ?
>> 
>> Did you perhaps send a stale patch? I can''t see how this would
even have
>> compiled: edx is uninitialized at this point afaict.
> 
> It is already filled. See the first patch.
Then the first patch fails to declare all four variables, and hence
won''t build. It just caught my eye that you declare the variables
here and don''t initialize them between declaration and use.

Jan

On 05/09/13 03:57, Yang Zhang wrote:
> From: Yang Zhang <yang.z.zhang@Intel.com>
>
> VMX MSRs only available when the CPU support the VMX feature. In addition,
> VMX_TRUE* MSRs only available when bitt 55 of VMX_BASIC MSR is set.
As you already have to respin the patch, "bit" rather than
"bitt"

~Andrew
>
> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> ---
>  xen/arch/x86/hvm/vmx/vvmx.c |   18 ++++++++++++++++++
>  1 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> index cecc72f..2e0b7f7 100644
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -1820,6 +1820,24 @@ int nvmx_msr_read_intercept(unsigned int msr, u64
*msr_content)
>      if ( !nestedhvm_enabled(v->domain) )
>          return 0;
>  
> +    /*
> +     * VMX capablitys MSRs available only when guest
> +     * support VMX.
> +     */
> +    hvm_cpuid(0x1, &eax, &ebx, &ecx, &edx);
> +    if ( !(ecx & cpufeat_mask(X86_FEATURE_VMXE)) ) 
> +        return 0;
> +
> +    /* 
> +     * Those MSRs available only when bit 55 of
> +     * MSR_IA32_VMX_BASIC is set.
> +     */
> +    rdmsrl(MSR_IA32_VMX_BASIC, data);
> +    if ( msr >= MSR_IA32_VMX_TRUE_PINBASED_CTLS &&
> +            msr <= MSR_IA32_VMX_TRUE_ENTRY_CTLS &&
> +            !(data & VMX_BASIC_DEFAULT1_ZERO) )
> +        return 0;
> +
>      rdmsrl(msr, host_data);
>  
>      /*

On 05/09/13 03:57, Yang Zhang wrote:
> From: Yang Zhang <yang.z.zhang@Intel.com>
>
> The bit 31 of revision_id will set to 1 if vmcs shadowing enabled. And
> according intel SDM, the bit 31 of IA32_VMX_BASIC MSR is always 0. So we
> cannot set low 32 bit of IA32_VMX_BASIC to revision_id directly. Must clear
> the bit 31 to 0.
>
> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
> ---
>  xen/arch/x86/hvm/vmx/vvmx.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
> index 2e0b7f7..8571002 100644
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -1846,7 +1846,7 @@ int nvmx_msr_read_intercept(unsigned int msr, u64
*msr_content)
>      switch (msr) {
>      case MSR_IA32_VMX_BASIC:
>          data = (host_data & (~0ul << 32)) |
> -               ((v->arch.hvm_vmx.vmcs)->vmcs_revision_id);
> +               ((v->arch.hvm_vmx.vmcs)->vmcs_revision_id & ~(1ul
<< 31));
What are the chances of vmcs_revision_id extending beyond 32 bits?

The SDM states that the bottom 31 bits of IA32_VMX_BASIC shall be the
bottom 31 bits of the revision id, so

(v->arch.hvm_vmx.vmcs->vmcs_revision_id & 0x7fffffff);

would seem more obvious.  Also, the brackets were superfluous.

~Andrew
>          break;
>      case MSR_IA32_VMX_PINBASED_CTLS:
>      case MSR_IA32_VMX_TRUE_PINBASED_CTLS:

Andrew Cooper wrote on 2013-09-05:
> On 05/09/13 03:57, Yang Zhang wrote:
>> From: Yang Zhang <yang.z.zhang@Intel.com>
>> 
>> The bit 31 of revision_id will set to 1 if vmcs shadowing enabled.
>> And according intel SDM, the bit 31 of IA32_VMX_BASIC MSR is always
>> 0. So we cannot set low 32 bit of IA32_VMX_BASIC to revision_id
directly.
>> Must clear the bit 31 to 0.
>> 
>> Signed-off-by: Yang Zhang <yang.z.zhang@Intel.com>
>> ---
>>  xen/arch/x86/hvm/vmx/vvmx.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>> diff --git a/xen/arch/x86/hvm/vmx/vvmx.c
>> b/xen/arch/x86/hvm/vmx/vvmx.c index 2e0b7f7..8571002 100644
>> --- a/xen/arch/x86/hvm/vmx/vvmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
>> @@ -1846,7 +1846,7 @@ int nvmx_msr_read_intercept(unsigned int msr,
> u64 *msr_content)
>>      switch (msr) {
>>      case MSR_IA32_VMX_BASIC:
>>          data = (host_data & (~0ul << 32)) |
>> -               ((v->arch.hvm_vmx.vmcs)->vmcs_revision_id);
>> +               ((v->arch.hvm_vmx.vmcs)->vmcs_revision_id &
~(1ul <<
>> + 31));
> 
> What are the chances of vmcs_revision_id extending beyond 32 bits?
> 
> The SDM states that the bottom 31 bits of IA32_VMX_BASIC shall be the
> bottom 31 bits of the revision id, so
> 
> (v->arch.hvm_vmx.vmcs->vmcs_revision_id & 0x7fffffff);
> 
> would seem more obvious.  Also, the brackets were superfluous.
Right!
> 
> ~Andrew
> 
>>          break;
>>      case MSR_IA32_VMX_PINBASED_CTLS:
>>      case MSR_IA32_VMX_TRUE_PINBASED_CTLS:

Best regards,
Yang

Jan Beulich wrote on 2013-09-05:
>>>> On 05.09.13 at 11:05, "Zhang, Yang Z"
<yang.z.zhang@intel.com> wrote:
>> Jan Beulich wrote on 2013-09-05:
>>>>>> On 05.09.13 at 04:57, Yang Zhang
<yang.z.zhang@intel.com> wrote:
>>>>      case MSR_IA32_VMX_CR4_FIXED1:
>>>> -        /* allow 0-settings except SMXE */
>>>> -        data = 0x267ff & ~X86_CR4_SMXE;
>>>> +        data |= (edx & cpufeat_mask(X86_FEATURE_VME) ?
>>> 
>>> Did you perhaps send a stale patch? I can''t see how this
would even
>>> have
>>> compiled: edx is uninitialized at this point afaict.
>> 
>> It is already filled. See the first patch.
> 
> Then the first patch fails to declare all four variables, and hence
> won''t build. It just caught my eye that you declare the variables
here
> and don''t initialize them between declaration and use.
You are right. I adjust the order of patches but forget tweaking the code
accordingly.

Best regards,
Yang