Re: [PATCH v4] KVM: VMX: Fix enable VPID even if INVVPID is not exposed in vmx capability

[David Hildenbrand]

On 23.03.2017 03:39, Wanpeng Li wrote:
> From: Wanpeng Li 
> 
> This can be reproduced by running L2 on L1, and disable VPID on L0 
> if w/o commit "KVM: nVMX: Fix nested VPID vmx exec control", the L2 
> crash as below:
> 
> KVM: entry failed, hardware error 0x7
> EAX= EBX= ECX= EDX=000306c3
> ESI= EDI= EBP= ESP=
> EIP=fff0 EFL=0002 [---] CPL=0 II=0 A20=1 SMM=0 HLT=0
> ES =   9300
> CS =f000   9b00
> SS =   9300
> DS =   9300
> FS =   9300
> GS =   9300
> LDT=   8200
> TR =   8b00
> GDT=  
> IDT=  
> CR0=6010 CR2= CR3= CR4=
> DR0= DR1= DR2= 
> DR3= 
> DR6=0ff0 DR7=0400
> EFER=
> 
> Reference SDM 30.3 INVVPID:
>  
> Protected Mode Exceptions
> #UD 
>   - If not in VMX operation.
>   - If the logical processor does not support VPIDs 
> (IA32_VMX_PROCBASED_CTLS2[37]=0).
>   - If the logical processor supports VPIDs (IA32_VMX_PROCBASED_CTLS2[37]=1) 
> but does 
> not support the INVVPID instruction (IA32_VMX_EPT_VPID_CAP[32]=0).
> 
> So we should check both VPID enable bit in vmx exec control and INVVPID 
> support bit 
> in vmx capability MSRs to enable VPID. This patch adds the guarantee to not 
> enable VPID
> if INVVPID is not exposed in vmx capability MSRs.

You should change the subject and this description to also take the
single/global capability into account.

> 
> Reviewed-by: David Hildenbrand 
> Cc: Jim Mattson 
> Cc: Paolo Bonzini 
> Cc: Radim Krčmář 
> Signed-off-by: Wanpeng Li 
> ---
>  arch/x86/kvm/vmx.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 8795a70..8925c76 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -1239,6 +1239,11 @@ static inline bool cpu_has_vmx_invvpid_global(void)
>   return vmx_capability.vpid & VMX_VPID_EXTENT_GLOBAL_CONTEXT_BIT;
>  }
>  
> +static inline bool cpu_has_vmx_invvpid(void)
> +{
> + return vmx_capability.vpid & VMX_VPID_INVVPID_BIT;
> +}
> +
>  static inline bool cpu_has_vmx_ept(void)
>  {
>   return vmcs_config.cpu_based_2nd_exec_ctrl &
> @@ -6518,8 +6523,10 @@ static __init int hardware_setup(void)
>   if (boot_cpu_has(X86_FEATURE_NX))
>   kvm_enable_efer_bits(EFER_NX);
>  
> - if (!cpu_has_vmx_vpid())
> + if (!cpu_has_vmx_vpid() || !cpu_has_vmx_invvpid() ||
> + !cpu_has_vmx_invvpid_single() || !cpu_has_vmx_invvpid_global())

According to Jim

"KVM's vpid_sync_context() assumes that at least one of
{VMX_VPID_EXTENT_SINGLE_CONTEXT, VMX_VPID_EXTENT_ALL_CONTEXT} ..."

So we should only require one of these two, not both.

if (!cpu_has_vmx_vpid() || !cpu_has_vmx_invvpid() ||
!(cpu_has_vmx_invvpid_single() || cpu_has_vmx_invvpid_global()))

>   enable_vpid = 0;
> +
>   if (!cpu_has_vmx_shadow_vmcs())
>   enable_shadow_vmcs = 0;
>   if (enable_shadow_vmcs)
> 


-- 

Thanks,

David

Re: [PATCH v4] KVM: VMX: Fix enable VPID even if INVVPID is not exposed in vmx capability

[David Hildenbrand]

On 23.03.2017 03:39, Wanpeng Li wrote:
> From: Wanpeng Li 
> 
> This can be reproduced by running L2 on L1, and disable VPID on L0 
> if w/o commit "KVM: nVMX: Fix nested VPID vmx exec control", the L2 
> crash as below:
> 
> KVM: entry failed, hardware error 0x7
> EAX= EBX= ECX= EDX=000306c3
> ESI= EDI= EBP= ESP=
> EIP=fff0 EFL=0002 [---] CPL=0 II=0 A20=1 SMM=0 HLT=0
> ES =   9300
> CS =f000   9b00
> SS =   9300
> DS =   9300
> FS =   9300
> GS =   9300
> LDT=   8200
> TR =   8b00
> GDT=  
> IDT=  
> CR0=6010 CR2= CR3= CR4=
> DR0= DR1= DR2= 
> DR3= 
> DR6=0ff0 DR7=0400
> EFER=
> 
> Reference SDM 30.3 INVVPID:
>  
> Protected Mode Exceptions
> #UD 
>   - If not in VMX operation.
>   - If the logical processor does not support VPIDs 
> (IA32_VMX_PROCBASED_CTLS2[37]=0).
>   - If the logical processor supports VPIDs (IA32_VMX_PROCBASED_CTLS2[37]=1) 
> but does 
> not support the INVVPID instruction (IA32_VMX_EPT_VPID_CAP[32]=0).
> 
> So we should check both VPID enable bit in vmx exec control and INVVPID 
> support bit 
> in vmx capability MSRs to enable VPID. This patch adds the guarantee to not 
> enable VPID
> if INVVPID is not exposed in vmx capability MSRs.

You should change the subject and this description to also take the
single/global capability into account.

> 
> Reviewed-by: David Hildenbrand 
> Cc: Jim Mattson 
> Cc: Paolo Bonzini 
> Cc: Radim Krčmář 
> Signed-off-by: Wanpeng Li 
> ---
>  arch/x86/kvm/vmx.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 8795a70..8925c76 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -1239,6 +1239,11 @@ static inline bool cpu_has_vmx_invvpid_global(void)
>   return vmx_capability.vpid & VMX_VPID_EXTENT_GLOBAL_CONTEXT_BIT;
>  }
>  
> +static inline bool cpu_has_vmx_invvpid(void)
> +{
> + return vmx_capability.vpid & VMX_VPID_INVVPID_BIT;
> +}
> +
>  static inline bool cpu_has_vmx_ept(void)
>  {
>   return vmcs_config.cpu_based_2nd_exec_ctrl &
> @@ -6518,8 +6523,10 @@ static __init int hardware_setup(void)
>   if (boot_cpu_has(X86_FEATURE_NX))
>   kvm_enable_efer_bits(EFER_NX);
>  
> - if (!cpu_has_vmx_vpid())
> + if (!cpu_has_vmx_vpid() || !cpu_has_vmx_invvpid() ||
> + !cpu_has_vmx_invvpid_single() || !cpu_has_vmx_invvpid_global())

According to Jim

"KVM's vpid_sync_context() assumes that at least one of
{VMX_VPID_EXTENT_SINGLE_CONTEXT, VMX_VPID_EXTENT_ALL_CONTEXT} ..."

So we should only require one of these two, not both.

if (!cpu_has_vmx_vpid() || !cpu_has_vmx_invvpid() ||
!(cpu_has_vmx_invvpid_single() || cpu_has_vmx_invvpid_global()))

>   enable_vpid = 0;
> +
>   if (!cpu_has_vmx_shadow_vmcs())
>   enable_shadow_vmcs = 0;
>   if (enable_shadow_vmcs)
> 


-- 

Thanks,

David

[PATCH v4] KVM: VMX: Fix enable VPID even if INVVPID is not exposed in vmx capability

[Wanpeng Li]

From: Wanpeng Li 

This can be reproduced by running L2 on L1, and disable VPID on L0 
if w/o commit "KVM: nVMX: Fix nested VPID vmx exec control", the L2 
crash as below:

KVM: entry failed, hardware error 0x7
EAX= EBX= ECX= EDX=000306c3
ESI= EDI= EBP= ESP=
EIP=fff0 EFL=0002 [---] CPL=0 II=0 A20=1 SMM=0 HLT=0
ES =   9300
CS =f000   9b00
SS =   9300
DS =   9300
FS =   9300
GS =   9300
LDT=   8200
TR =   8b00
GDT=  
IDT=  
CR0=6010 CR2= CR3= CR4=
DR0= DR1= DR2= 
DR3= 
DR6=0ff0 DR7=0400
EFER=

Reference SDM 30.3 INVVPID:
 
Protected Mode Exceptions
#UD 
  - If not in VMX operation.
  - If the logical processor does not support VPIDs 
(IA32_VMX_PROCBASED_CTLS2[37]=0).
  - If the logical processor supports VPIDs (IA32_VMX_PROCBASED_CTLS2[37]=1) 
but does 
not support the INVVPID instruction (IA32_VMX_EPT_VPID_CAP[32]=0).

So we should check both VPID enable bit in vmx exec control and INVVPID support 
bit 
in vmx capability MSRs to enable VPID. This patch adds the guarantee to not 
enable VPID
if INVVPID is not exposed in vmx capability MSRs.

Reviewed-by: David Hildenbrand 
Cc: Jim Mattson 
Cc: Paolo Bonzini 
Cc: Radim Krčmář 
Signed-off-by: Wanpeng Li 
---
 arch/x86/kvm/vmx.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 8795a70..8925c76 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -1239,6 +1239,11 @@ static inline bool cpu_has_vmx_invvpid_global(void)
return vmx_capability.vpid & VMX_VPID_EXTENT_GLOBAL_CONTEXT_BIT;
 }
 
+static inline bool cpu_has_vmx_invvpid(void)
+{
+   return vmx_capability.vpid & VMX_VPID_INVVPID_BIT;
+}
+
 static inline bool cpu_has_vmx_ept(void)
 {
return vmcs_config.cpu_based_2nd_exec_ctrl &
@@ -6518,8 +6523,10 @@ static __init int hardware_setup(void)
if (boot_cpu_has(X86_FEATURE_NX))
kvm_enable_efer_bits(EFER_NX);
 
-   if (!cpu_has_vmx_vpid())
+   if (!cpu_has_vmx_vpid() || !cpu_has_vmx_invvpid() ||
+   !cpu_has_vmx_invvpid_single() || !cpu_has_vmx_invvpid_global())
enable_vpid = 0;
+
if (!cpu_has_vmx_shadow_vmcs())
enable_shadow_vmcs = 0;
if (enable_shadow_vmcs)
-- 
2.7.4

[PATCH v4] KVM: VMX: Fix enable VPID even if INVVPID is not exposed in vmx capability

[Wanpeng Li]

From: Wanpeng Li 

This can be reproduced by running L2 on L1, and disable VPID on L0 
if w/o commit "KVM: nVMX: Fix nested VPID vmx exec control", the L2 
crash as below:

KVM: entry failed, hardware error 0x7
EAX= EBX= ECX= EDX=000306c3
ESI= EDI= EBP= ESP=
EIP=fff0 EFL=0002 [---] CPL=0 II=0 A20=1 SMM=0 HLT=0
ES =   9300
CS =f000   9b00
SS =   9300
DS =   9300
FS =   9300
GS =   9300
LDT=   8200
TR =   8b00
GDT=  
IDT=  
CR0=6010 CR2= CR3= CR4=
DR0= DR1= DR2= 
DR3= 
DR6=0ff0 DR7=0400
EFER=

Reference SDM 30.3 INVVPID:
 
Protected Mode Exceptions
#UD 
  - If not in VMX operation.
  - If the logical processor does not support VPIDs 
(IA32_VMX_PROCBASED_CTLS2[37]=0).
  - If the logical processor supports VPIDs (IA32_VMX_PROCBASED_CTLS2[37]=1) 
but does 
not support the INVVPID instruction (IA32_VMX_EPT_VPID_CAP[32]=0).

So we should check both VPID enable bit in vmx exec control and INVVPID support 
bit 
in vmx capability MSRs to enable VPID. This patch adds the guarantee to not 
enable VPID
if INVVPID is not exposed in vmx capability MSRs.

Reviewed-by: David Hildenbrand 
Cc: Jim Mattson 
Cc: Paolo Bonzini 
Cc: Radim Krčmář 
Signed-off-by: Wanpeng Li 
---
 arch/x86/kvm/vmx.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 8795a70..8925c76 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -1239,6 +1239,11 @@ static inline bool cpu_has_vmx_invvpid_global(void)
return vmx_capability.vpid & VMX_VPID_EXTENT_GLOBAL_CONTEXT_BIT;
 }
 
+static inline bool cpu_has_vmx_invvpid(void)
+{
+   return vmx_capability.vpid & VMX_VPID_INVVPID_BIT;
+}
+
 static inline bool cpu_has_vmx_ept(void)
 {
return vmcs_config.cpu_based_2nd_exec_ctrl &
@@ -6518,8 +6523,10 @@ static __init int hardware_setup(void)
if (boot_cpu_has(X86_FEATURE_NX))
kvm_enable_efer_bits(EFER_NX);
 
-   if (!cpu_has_vmx_vpid())
+   if (!cpu_has_vmx_vpid() || !cpu_has_vmx_invvpid() ||
+   !cpu_has_vmx_invvpid_single() || !cpu_has_vmx_invvpid_global())
enable_vpid = 0;
+
if (!cpu_has_vmx_shadow_vmcs())
enable_shadow_vmcs = 0;
if (enable_shadow_vmcs)
-- 
2.7.4