Re: [PATCH v2 6/8] KVM: VMX: unify SECONDARY_VM_EXEC_CONTROL update

[Paolo Bonzini]

On 09/09/2015 08:05, Xiao Guangrong wrote:
> Unify the update in vmx_cpuid_update()
> 
> Signed-off-by: Xiao Guangrong 

What if we instead start fresh from vmx_secondary_exec_control, like this:

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 4504cbc1b287..98c4d3f1653a 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -8660,11 +8660,26 @@ static int vmx_get_lpage_level(void)
return PT_PDPE_LEVEL;
 }
 
+static void vmcs_set_secondary_exec_control(u32 new_ctl)
+{
+   /*
+* These bits in the secondary execution controls field
+* are dynamic, the others are mostly based on the hypervisor
+* architecture and the guest's CPUID.  Do not touch the
+* dynamic bits.
+*/
+   u32 mask = SECONDARY_EXEC_ENABLE_PML | SECONDARY_EXEC_SHADOW_VMCS;
+   u32 cur_ctl = vmcs_read32(SECONDARY_EXEC_CONTROL);
+
+   vmcs_write32(SECONDARY_EXEC_CONTROL,
+(new_ctl & ~mask) | (cur_ctl & mask));
+}
+
 static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
 {
struct kvm_cpuid_entry2 *best;
struct vcpu_vmx *vmx = to_vmx(vcpu);
-   u32 clear_exe_ctrl = 0;
+   u32 secondary_exec_ctl = vmx_secondary_exec_control(vmx);
 
vmx->rdtscp_enabled = false;
if (vmx_rdtscp_supported()) {
@@ -8672,7 +8681,7 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
if (best && (best->edx & bit(X86_FEATURE_RDTSCP)))
vmx->rdtscp_enabled = true;
else
-   clear_exe_ctrl |= SECONDARY_EXEC_RDTSCP;
+   secondary_exec_ctl &= ~SECONDARY_EXEC_RDTSCP;
 
if (nested) {
if (vmx->rdtscp_enabled)
@@ -8689,18 +8698,13 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
if (vmx_invpcid_supported() &&
(!best || !(best->ebx & bit(X86_FEATURE_INVPCID)) ||
!guest_cpuid_has_pcid(vcpu))) {
-   clear_exe_ctrl |= SECONDARY_EXEC_ENABLE_INVPCID;
+   secondary_exec_ctl &= ~SECONDARY_EXEC_ENABLE_INVPCID;
 
if (best)
best->ebx &= ~bit(X86_FEATURE_INVPCID);
}
 
-   if (clear_exe_ctrl) {
-   u32 exec_ctl = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
-
-   exec_ctl &= ~clear_exe_ctrl;
-   vmcs_write32(SECONDARY_VM_EXEC_CONTROL, exec_ctl);
-   }
+   vmcs_set_secondary_exec_control(secondary_exec_ctl);
 
if (static_cpu_has(X86_FEATURE_PCOMMIT) && nested) {
if (guest_cpuid_has_pcommit(vcpu))

> ---
>  arch/x86/kvm/vmx.c | 21 +++--
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 97e3340..5a074d0 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -8673,19 +8673,15 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
>  {
>   struct kvm_cpuid_entry2 *best;
>   struct vcpu_vmx *vmx = to_vmx(vcpu);
> - u32 exec_control;
> + u32 clear_exe_ctrl = 0;
>  
>   vmx->rdtscp_enabled = false;
>   if (vmx_rdtscp_supported()) {
> - exec_control = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
>   best = kvm_find_cpuid_entry(vcpu, 0x8001, 0);
>   if (best && (best->edx & bit(X86_FEATURE_RDTSCP)))
>   vmx->rdtscp_enabled = true;
> - else {
> - exec_control &= ~SECONDARY_EXEC_RDTSCP;
> - vmcs_write32(SECONDARY_VM_EXEC_CONTROL,
> - exec_control);
> - }
> + else
> + clear_exe_ctrl |= SECONDARY_EXEC_RDTSCP;
>  
>   if (nested && !vmx->rdtscp_enabled)
>   vmx->nested.nested_vmx_secondary_ctls_high &=
> @@ -8697,14 +8693,19 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
>   if (vmx_invpcid_supported() &&
>   (!best || !(best->ebx & bit(X86_FEATURE_INVPCID)) ||
>   !guest_cpuid_has_pcid(vcpu))) {
> - exec_control = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
> - exec_control &= ~SECONDARY_EXEC_ENABLE_INVPCID;
> - vmcs_write32(SECONDARY_VM_EXEC_CONTROL, exec_control);
> + clear_exe_ctrl |= SECONDARY_EXEC_ENABLE_INVPCID;
>  
>   if (best)
>   best->ebx &= ~bit(X86_FEATURE_INVPCID);
>   }
>  
> + if (clear_exe_ctrl) {
> + u32 exec_ctl = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
> +
> + exec_ctl &= ~clear_exe_ctrl;
> + vmcs_write32(SECONDARY_VM_EXEC_CONTROL, exec_ctl);
> + }
> +
>   if (!guest_cpuid_has_pcommit(vcpu) && nested)
>   vmx->nested.nested_vmx_secondary_ctls_high &=
>   ~SECONDARY_EXEC_PCOMMIT;
> 
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to 

[PATCH v2 6/8] KVM: VMX: unify SECONDARY_VM_EXEC_CONTROL update

[Xiao Guangrong]

Unify the update in vmx_cpuid_update()

Signed-off-by: Xiao Guangrong 
---
 arch/x86/kvm/vmx.c | 21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 97e3340..5a074d0 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -8673,19 +8673,15 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
 {
struct kvm_cpuid_entry2 *best;
struct vcpu_vmx *vmx = to_vmx(vcpu);
-   u32 exec_control;
+   u32 clear_exe_ctrl = 0;
 
vmx->rdtscp_enabled = false;
if (vmx_rdtscp_supported()) {
-   exec_control = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
best = kvm_find_cpuid_entry(vcpu, 0x8001, 0);
if (best && (best->edx & bit(X86_FEATURE_RDTSCP)))
vmx->rdtscp_enabled = true;
-   else {
-   exec_control &= ~SECONDARY_EXEC_RDTSCP;
-   vmcs_write32(SECONDARY_VM_EXEC_CONTROL,
-   exec_control);
-   }
+   else
+   clear_exe_ctrl |= SECONDARY_EXEC_RDTSCP;
 
if (nested && !vmx->rdtscp_enabled)
vmx->nested.nested_vmx_secondary_ctls_high &=
@@ -8697,14 +8693,19 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
if (vmx_invpcid_supported() &&
(!best || !(best->ebx & bit(X86_FEATURE_INVPCID)) ||
!guest_cpuid_has_pcid(vcpu))) {
-   exec_control = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
-   exec_control &= ~SECONDARY_EXEC_ENABLE_INVPCID;
-   vmcs_write32(SECONDARY_VM_EXEC_CONTROL, exec_control);
+   clear_exe_ctrl |= SECONDARY_EXEC_ENABLE_INVPCID;
 
if (best)
best->ebx &= ~bit(X86_FEATURE_INVPCID);
}
 
+   if (clear_exe_ctrl) {
+   u32 exec_ctl = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
+
+   exec_ctl &= ~clear_exe_ctrl;
+   vmcs_write32(SECONDARY_VM_EXEC_CONTROL, exec_ctl);
+   }
+
if (!guest_cpuid_has_pcommit(vcpu) && nested)
vmx->nested.nested_vmx_secondary_ctls_high &=
~SECONDARY_EXEC_PCOMMIT;
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html