[PATCH 0/9] KVM: x86: enable cflushopt/clwb/pcommit and simplify code

[PATCH 0/9] KVM: x86: enable cflushopt/clwb/pcommit and simplify code

[Xiao Guangrong]

This pachset enables clfushopt, clwb and pcommit instructions for guest which
are used by NVDIMM.

The specification locates at:
https://software.intel.com/sites/default/files/managed/0d/53/319433-022.pdf

Patch 1 fixes a uninitialized value used in KVM MMU code,
patch 2 and patch 3 enable these three instructions for guest
patch 4 ~ patch 9 simplify current VMX code
 
Xiao Guangrong (9):
  KVM: MMU: fix use uninitialized value
  KVM: x86: allow guest to use cflushopt anc clwb
  KVM: x86: add pcommit support
  KVM: VMX: drop rdtscp_enabled check in prepare_vmcs02()
  KVM: VMX: simplify rdtscp handling in vmx_cpuid_update()
  KVM: VMX: simplify invpcid handling in vmx_cpuid_update()
  KVM: VMX: unify SECONDARY_VM_EXEC_CONTROL update
  KVM: VMX: introduce set_clear_2nd_exec_ctrl()
  KVM: VMX: drop rdtscp_enabled field

 arch/x86/include/asm/vmx.h      |   2 +-
 arch/x86/include/uapi/asm/vmx.h |   4 +-
 arch/x86/kvm/cpuid.c            |   2 +-
 arch/x86/kvm/cpuid.h            |  16 ++++++
 arch/x86/kvm/mmu.c              |   2 +-
 arch/x86/kvm/vmx.c              | 116 ++++++++++++++++++++--------------------
 6 files changed, 80 insertions(+), 62 deletions(-)

-- 
2.4.3

[PATCH 1/9] KVM: MMU: fix use uninitialized value

[Xiao Guangrong]

GCC (gcc version 5.1.1 20150618 (Red Hat 5.1.1-4) (GCC)) complains
of this warning:

arch/x86/kvm//mmu.c:3332:9: warning: ‘leaf’ may be used uninitialized in this function [-Wmaybe-uninitialized]
   while (root >= leaf) {
         ^
arch/x86/kvm//mmu.c:3304:12: note: ‘leaf’ was declared here
  int root, leaf;

It's true as shadow_walk_init() may stop the loop

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/kvm/mmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 70c375f..a8a5b8d 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -3301,7 +3301,7 @@ walk_shadow_page_get_mmio_spte(struct kvm_vcpu *vcpu, u64 addr, u64 *sptep)
 {
 	struct kvm_shadow_walk_iterator iterator;
 	u64 sptes[PT64_ROOT_LEVEL], spte = 0ull;
-	int root, leaf;
+	int root, leaf = PT64_ROOT_LEVEL;
 	bool reserved = false;
 
 	if (!VALID_PAGE(vcpu->arch.mmu.root_hpa))
-- 
2.4.3

[PATCH 2/9] KVM: x86: allow guest to use cflushopt anc clwb

[Xiao Guangrong]

Pass its CPU feature to guest to enable them in guest

These are needed by nvdimm drivers

The specification locates at:
https://software.intel.com/sites/default/files/managed/0d/53/319433-022.pdf

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/kvm/cpuid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 2fbea25..962fc7d 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -348,7 +348,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 		F(FSGSBASE) | F(BMI1) | F(HLE) | F(AVX2) | F(SMEP) |
 		F(BMI2) | F(ERMS) | f_invpcid | F(RTM) | f_mpx | F(RDSEED) |
 		F(ADX) | F(SMAP) | F(AVX512F) | F(AVX512PF) | F(AVX512ER) |
-		F(AVX512CD);
+		F(AVX512CD) | F(CLFLUSHOPT) | F(CLWB);
 
 	/* cpuid 0xD.1.eax */
 	const u32 kvm_supported_word10_x86_features =
-- 
2.4.3

[PATCH 3/9] KVM: x86: add pcommit support

[Xiao Guangrong]

Pass PCOMMIT CPU feature to guest to enable PCOMMIT instruction

Currently we do not catch pcommit instruction for L1 guest and
allow L1 to catch this instruction for L2

The specification locates at:
https://software.intel.com/sites/default/files/managed/0d/53/319433-022.pdf

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/include/asm/vmx.h      |  2 +-
 arch/x86/include/uapi/asm/vmx.h |  4 +++-
 arch/x86/kvm/cpuid.c            |  2 +-
 arch/x86/kvm/cpuid.h            |  8 ++++++++
 arch/x86/kvm/vmx.c              | 29 ++++++++++++++++++++++++-----
 5 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index 9299ae5..e2ad08a 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -72,7 +72,7 @@
 #define SECONDARY_EXEC_SHADOW_VMCS              0x00004000
 #define SECONDARY_EXEC_ENABLE_PML               0x00020000
 #define SECONDARY_EXEC_XSAVES			0x00100000
-
+#define SECONDARY_EXEC_PCOMMIT			0x00200000
 
 #define PIN_BASED_EXT_INTR_MASK                 0x00000001
 #define PIN_BASED_NMI_EXITING                   0x00000008
diff --git a/arch/x86/include/uapi/asm/vmx.h b/arch/x86/include/uapi/asm/vmx.h
index 37fee27..5b15d94 100644
--- a/arch/x86/include/uapi/asm/vmx.h
+++ b/arch/x86/include/uapi/asm/vmx.h
@@ -78,6 +78,7 @@
 #define EXIT_REASON_PML_FULL            62
 #define EXIT_REASON_XSAVES              63
 #define EXIT_REASON_XRSTORS             64
+#define EXIT_REASON_PCOMMIT             65
 
 #define VMX_EXIT_REASONS \
 	{ EXIT_REASON_EXCEPTION_NMI,         "EXCEPTION_NMI" }, \
@@ -126,7 +127,8 @@
 	{ EXIT_REASON_INVVPID,               "INVVPID" }, \
 	{ EXIT_REASON_INVPCID,               "INVPCID" }, \
 	{ EXIT_REASON_XSAVES,                "XSAVES" }, \
-	{ EXIT_REASON_XRSTORS,               "XRSTORS" }
+	{ EXIT_REASON_XRSTORS,               "XRSTORS" }, \
+	{ EXIT_REASON_PCOMMIT,               "PCOMMIT" }
 
 #define VMX_ABORT_SAVE_GUEST_MSR_FAIL        1
 #define VMX_ABORT_LOAD_HOST_MSR_FAIL         4
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 962fc7d..faeb0b3 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -348,7 +348,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 		F(FSGSBASE) | F(BMI1) | F(HLE) | F(AVX2) | F(SMEP) |
 		F(BMI2) | F(ERMS) | f_invpcid | F(RTM) | f_mpx | F(RDSEED) |
 		F(ADX) | F(SMAP) | F(AVX512F) | F(AVX512PF) | F(AVX512ER) |
-		F(AVX512CD) | F(CLFLUSHOPT) | F(CLWB);
+		F(AVX512CD) | F(CLFLUSHOPT) | F(CLWB) | F(PCOMMIT);
 
 	/* cpuid 0xD.1.eax */
 	const u32 kvm_supported_word10_x86_features =
diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
index dd05b9c..aed7bfe 100644
--- a/arch/x86/kvm/cpuid.h
+++ b/arch/x86/kvm/cpuid.h
@@ -133,4 +133,12 @@ static inline bool guest_cpuid_has_mpx(struct kvm_vcpu *vcpu)
 	best = kvm_find_cpuid_entry(vcpu, 7, 0);
 	return best && (best->ebx & bit(X86_FEATURE_MPX));
 }
+
+static inline bool guest_cpuid_has_pcommit(struct kvm_vcpu *vcpu)
+{
+	struct kvm_cpuid_entry2 *best;
+
+	best = kvm_find_cpuid_entry(vcpu, 7, 0);
+	return best && (best->ebx & bit(X86_FEATURE_PCOMMIT));
+}
 #endif
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 4cf25b9..b526c61 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2474,7 +2474,8 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
 		SECONDARY_EXEC_APIC_REGISTER_VIRT |
 		SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
 		SECONDARY_EXEC_WBINVD_EXITING |
-		SECONDARY_EXEC_XSAVES;
+		SECONDARY_EXEC_XSAVES |
+		SECONDARY_EXEC_PCOMMIT;
 
 	if (enable_ept) {
 		/* nested EPT: emulate EPT also to L1 */
@@ -3015,7 +3016,8 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
 			SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
 			SECONDARY_EXEC_SHADOW_VMCS |
 			SECONDARY_EXEC_XSAVES |
-			SECONDARY_EXEC_ENABLE_PML;
+			SECONDARY_EXEC_ENABLE_PML |
+			SECONDARY_EXEC_PCOMMIT;
 		if (adjust_vmx_controls(min2, opt2,
 					MSR_IA32_VMX_PROCBASED_CTLS2,
 					&_cpu_based_2nd_exec_control) < 0)
@@ -4570,6 +4572,9 @@ static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx)
 	/* PML is enabled/disabled in creating/destorying vcpu */
 	exec_control &= ~SECONDARY_EXEC_ENABLE_PML;
 
+	/* Currently, we allow L1 guest to directly run pcommit instruction. */
+	exec_control &= ~SECONDARY_EXEC_PCOMMIT;
+
 	return exec_control;
 }
 
@@ -4613,10 +4618,9 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx)
 
 	vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, vmx_exec_control(vmx));
 
-	if (cpu_has_secondary_exec_ctrls()) {
+	if (cpu_has_secondary_exec_ctrls())
 		vmcs_write32(SECONDARY_VM_EXEC_CONTROL,
 				vmx_secondary_exec_control(vmx));
-	}
 
 	if (vmx_cpu_uses_apicv(&vmx->vcpu)) {
 		vmcs_write64(EOI_EXIT_BITMAP0, 0);
@@ -7208,6 +7212,13 @@ static int handle_pml_full(struct kvm_vcpu *vcpu)
 	return 1;
 }
 
+static int handle_pcommit(struct kvm_vcpu *vcpu)
+{
+	/* we never catch pcommit instruct for L1 guest. */
+	BUG();
+	return 1;
+}
+
 /*
  * The exit handlers return 1 if the exit was handled fully and guest execution
  * may resume.  Otherwise they set the kvm_run parameter to indicate what needs
@@ -7258,6 +7269,7 @@ static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = {
 	[EXIT_REASON_XSAVES]                  = handle_xsaves,
 	[EXIT_REASON_XRSTORS]                 = handle_xrstors,
 	[EXIT_REASON_PML_FULL]		      = handle_pml_full,
+	[EXIT_REASON_PCOMMIT]                 = handle_pcommit,
 };
 
 static const int kvm_vmx_max_exit_handlers =
@@ -7559,6 +7571,8 @@ static bool nested_vmx_exit_handled(struct kvm_vcpu *vcpu)
 		 * the XSS exit bitmap in vmcs12.
 		 */
 		return nested_cpu_has2(vmcs12, SECONDARY_EXEC_XSAVES);
+	case EXIT_REASON_PCOMMIT:
+		return nested_cpu_has2(vmcs12, SECONDARY_EXEC_PCOMMIT);
 	default:
 		return true;
 	}
@@ -8688,6 +8702,10 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
 		if (best)
 			best->ebx &= ~bit(X86_FEATURE_INVPCID);
 	}
+
+	if (!guest_cpuid_has_pcommit(vcpu) && nested)
+		vmx->nested.nested_vmx_secondary_ctls_high &=
+			~SECONDARY_EXEC_PCOMMIT;
 }
 
 static void vmx_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry)
@@ -9301,7 +9319,8 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
 		exec_control &= ~(SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
 				  SECONDARY_EXEC_RDTSCP |
 				  SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
-				  SECONDARY_EXEC_APIC_REGISTER_VIRT);
+				  SECONDARY_EXEC_APIC_REGISTER_VIRT |
+				  SECONDARY_EXEC_PCOMMIT);
 		if (nested_cpu_has(vmcs12,
 				CPU_BASED_ACTIVATE_SECONDARY_CONTROLS))
 			exec_control |= vmcs12->secondary_vm_exec_control;
-- 
2.4.3

[PATCH 4/9] KVM: VMX: drop rdtscp_enabled check in prepare_vmcs02()

[Xiao Guangrong]

SECONDARY_EXEC_RDTSCP set for L2 guest comes from vmcs12

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/kvm/vmx.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index b526c61..f7a721e 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -9313,8 +9313,7 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
 
 	if (cpu_has_secondary_exec_ctrls()) {
 		exec_control = vmx_secondary_exec_control(vmx);
-		if (!vmx->rdtscp_enabled)
-			exec_control &= ~SECONDARY_EXEC_RDTSCP;
+
 		/* Take the following fields only from vmcs12 */
 		exec_control &= ~(SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
 				  SECONDARY_EXEC_RDTSCP |
-- 
2.4.3

[PATCH 5/9] KVM: VMX: simplify rdtscp handling in vmx_cpuid_update()

[Xiao Guangrong]

if vmx_rdtscp_supported() is true SECONDARY_EXEC_RDTSCP must
have already been set in current vmcs by
vmx_secondary_exec_control()

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/kvm/vmx.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index f7a721e..99f638e 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -8668,16 +8668,15 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
 	vmx->rdtscp_enabled = false;
 	if (vmx_rdtscp_supported()) {
 		exec_control = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
-		if (exec_control & SECONDARY_EXEC_RDTSCP) {
-			best = kvm_find_cpuid_entry(vcpu, 0x80000001, 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);
-			}
+		best = kvm_find_cpuid_entry(vcpu, 0x80000001, 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);
 		}
+
 		if (nested && !vmx->rdtscp_enabled)
 			vmx->nested.nested_vmx_secondary_ctls_high &=
 				~SECONDARY_EXEC_RDTSCP;
-- 
2.4.3

[PATCH 6/9] KVM: VMX: simplify invpcid handling in vmx_cpuid_update()

[Xiao Guangrong]

If vmx_invpcid_supported() is true, second execution control
filed must be supported and SECONDARY_EXEC_ENABLE_INVPCID
must have already been set in current vmcs by
vmx_secondary_exec_control()

If vmx_invpcid_supported() is false, no need to clear
SECONDARY_EXEC_ENABLE_INVPCID

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/kvm/vmx.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 99f638e..0d68140 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -8684,20 +8684,13 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
 
 	/* Exposing INVPCID only when PCID is exposed */
 	best = kvm_find_cpuid_entry(vcpu, 0x7, 0);
-	if (vmx_invpcid_supported() &&
-	    best && (best->ebx & bit(X86_FEATURE_INVPCID)) &&
-	    guest_cpuid_has_pcid(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);
-	} else {
-		if (cpu_has_secondary_exec_ctrls()) {
-			exec_control = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
-			exec_control &= ~SECONDARY_EXEC_ENABLE_INVPCID;
-			vmcs_write32(SECONDARY_VM_EXEC_CONTROL,
-				     exec_control);
-		}
+		exec_control &= ~SECONDARY_EXEC_ENABLE_INVPCID;
+		vmcs_write32(SECONDARY_VM_EXEC_CONTROL, exec_control);
+
 		if (best)
 			best->ebx &= ~bit(X86_FEATURE_INVPCID);
 	}
-- 
2.4.3

[PATCH 7/9] KVM: VMX: unify SECONDARY_VM_EXEC_CONTROL update

[Xiao Guangrong]

Unify the update in vmx_cpuid_update()

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 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 0d68140..4f238b7 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -8663,19 +8663,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, 0x80000001, 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 &=
@@ -8687,14 +8683,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

[PATCH 8/9] KVM: VMX: introduce set_clear_2nd_exec_ctrl()

[Xiao Guangrong]

It's used to clean up the code

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/kvm/vmx.c | 42 +++++++++++++++++++-----------------------
 1 file changed, 19 insertions(+), 23 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 4f238b7..58f7b89 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -4589,6 +4589,18 @@ static void ept_set_mmio_spte_mask(void)
 	kvm_mmu_set_mmio_spte_mask((0x3ull << 62) | 0x6ull);
 }
 
+static void set_clear_2nd_exec_ctrl(u32 ctrls, bool set)
+{
+	u32 exec_ctrl = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
+
+	if (set)
+		exec_ctrl |= ctrls;
+	else
+		exec_ctrl &= ~ctrls;
+
+	vmcs_write32(SECONDARY_VM_EXEC_CONTROL, exec_ctrl);
+}
+
 #define VMX_XSS_EXIT_BITMAP 0
 /*
  * Sets up the vmcs for emulated real mode.
@@ -6632,7 +6644,6 @@ static int nested_vmx_check_permission(struct kvm_vcpu *vcpu)
 
 static inline void nested_release_vmcs12(struct vcpu_vmx *vmx)
 {
-	u32 exec_control;
 	if (vmx->nested.current_vmptr == -1ull)
 		return;
 
@@ -6645,9 +6656,7 @@ static inline void nested_release_vmcs12(struct vcpu_vmx *vmx)
 		   they were modified */
 		copy_shadow_to_vmcs12(vmx);
 		vmx->nested.sync_shadow_vmcs = false;
-		exec_control = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
-		exec_control &= ~SECONDARY_EXEC_SHADOW_VMCS;
-		vmcs_write32(SECONDARY_VM_EXEC_CONTROL, exec_control);
+		set_clear_2nd_exec_ctrl(SECONDARY_EXEC_SHADOW_VMCS, false);
 		vmcs_write64(VMCS_LINK_POINTER, -1ull);
 	}
 	vmx->nested.posted_intr_nv = -1;
@@ -7043,7 +7052,6 @@ static int handle_vmptrld(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
 	gpa_t vmptr;
-	u32 exec_control;
 
 	if (!nested_vmx_check_permission(vcpu))
 		return 1;
@@ -7075,9 +7083,8 @@ static int handle_vmptrld(struct kvm_vcpu *vcpu)
 		vmx->nested.current_vmcs12 = new_vmcs12;
 		vmx->nested.current_vmcs12_page = page;
 		if (enable_shadow_vmcs) {
-			exec_control = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
-			exec_control |= SECONDARY_EXEC_SHADOW_VMCS;
-			vmcs_write32(SECONDARY_VM_EXEC_CONTROL, exec_control);
+			set_clear_2nd_exec_ctrl(SECONDARY_EXEC_SHADOW_VMCS,
+						true);
 			vmcs_write64(VMCS_LINK_POINTER,
 				     __pa(vmx->nested.current_shadow_vmcs));
 			vmx->nested.sync_shadow_vmcs = true;
@@ -7587,7 +7594,6 @@ static void vmx_get_exit_info(struct kvm_vcpu *vcpu, u64 *info1, u64 *info2)
 static int vmx_enable_pml(struct vcpu_vmx *vmx)
 {
 	struct page *pml_pg;
-	u32 exec_control;
 
 	pml_pg = alloc_page(GFP_KERNEL | __GFP_ZERO);
 	if (!pml_pg)
@@ -7598,24 +7604,18 @@ static int vmx_enable_pml(struct vcpu_vmx *vmx)
 	vmcs_write64(PML_ADDRESS, page_to_phys(vmx->pml_pg));
 	vmcs_write16(GUEST_PML_INDEX, PML_ENTITY_NUM - 1);
 
-	exec_control = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
-	exec_control |= SECONDARY_EXEC_ENABLE_PML;
-	vmcs_write32(SECONDARY_VM_EXEC_CONTROL, exec_control);
+	set_clear_2nd_exec_ctrl(SECONDARY_EXEC_ENABLE_PML, true);
 
 	return 0;
 }
 
 static void vmx_disable_pml(struct vcpu_vmx *vmx)
 {
-	u32 exec_control;
-
 	ASSERT(vmx->pml_pg);
 	__free_page(vmx->pml_pg);
 	vmx->pml_pg = NULL;
 
-	exec_control = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
-	exec_control &= ~SECONDARY_EXEC_ENABLE_PML;
-	vmcs_write32(SECONDARY_VM_EXEC_CONTROL, exec_control);
+	set_clear_2nd_exec_ctrl(SECONDARY_EXEC_ENABLE_PML, false);
 }
 
 static void vmx_flush_pml_buffer(struct kvm_vcpu *vcpu)
@@ -8689,12 +8689,8 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
 			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 (clear_exe_ctrl)
+		set_clear_2nd_exec_ctrl(clear_exe_ctrl, false);
 
 	if (!guest_cpuid_has_pcommit(vcpu) && nested)
 		vmx->nested.nested_vmx_secondary_ctls_high &=
-- 
2.4.3

[PATCH 9/9] KVM: VMX: drop rdtscp_enabled field

[Xiao Guangrong]

Check cpuid bit instead of it

Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
---
 arch/x86/kvm/cpuid.h |  8 ++++++++
 arch/x86/kvm/vmx.c   | 19 ++++++-------------
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
index aed7bfe..d434ee9 100644
--- a/arch/x86/kvm/cpuid.h
+++ b/arch/x86/kvm/cpuid.h
@@ -141,4 +141,12 @@ static inline bool guest_cpuid_has_pcommit(struct kvm_vcpu *vcpu)
 	best = kvm_find_cpuid_entry(vcpu, 7, 0);
 	return best && (best->ebx & bit(X86_FEATURE_PCOMMIT));
 }
+
+static inline bool guest_cpuid_has_rdtscp(struct kvm_vcpu *vcpu)
+{
+	struct kvm_cpuid_entry2 *best;
+
+	best = kvm_find_cpuid_entry(vcpu, 0x80000001, 0);
+	return best && (best->edx & bit(X86_FEATURE_RDTSCP));
+}
 #endif
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 58f7b89..d50fb48 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -532,8 +532,6 @@ struct vcpu_vmx {
 	s64 vnmi_blocked_time;
 	u32 exit_reason;
 
-	bool rdtscp_enabled;
-
 	/* Posted interrupt descriptor */
 	struct pi_desc pi_desc;
 
@@ -2207,7 +2205,7 @@ static void setup_msrs(struct vcpu_vmx *vmx)
 		if (index >= 0)
 			move_msr_up(vmx, index, save_nmsrs++);
 		index = __find_msr_index(vmx, MSR_TSC_AUX);
-		if (index >= 0 && vmx->rdtscp_enabled)
+		if (index >= 0 && guest_cpuid_has_rdtscp(&vmx->vcpu))
 			move_msr_up(vmx, index, save_nmsrs++);
 		/*
 		 * MSR_STAR is only needed on long mode guests, and only
@@ -2674,7 +2672,7 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		msr_info->data = vcpu->arch.ia32_xss;
 		break;
 	case MSR_TSC_AUX:
-		if (!to_vmx(vcpu)->rdtscp_enabled)
+		if (!guest_cpuid_has_rdtscp(vcpu))
 			return 1;
 		/* Otherwise falls through */
 	default:
@@ -2780,7 +2778,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 			clear_atomic_switch_msr(vmx, MSR_IA32_XSS);
 		break;
 	case MSR_TSC_AUX:
-		if (!vmx->rdtscp_enabled)
+		if (!guest_cpuid_has_rdtscp(vcpu))
 			return 1;
 		/* Check reserved bit, higher 32 bits should be zero */
 		if ((data >> 32) != 0)
@@ -8665,15 +8663,10 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
 	u32 clear_exe_ctrl = 0;
 
-	vmx->rdtscp_enabled = false;
-	if (vmx_rdtscp_supported()) {
-		best = kvm_find_cpuid_entry(vcpu, 0x80000001, 0);
-		if (best && (best->edx & bit(X86_FEATURE_RDTSCP)))
-			vmx->rdtscp_enabled = true;
-		else
-			clear_exe_ctrl |= SECONDARY_EXEC_RDTSCP;
+	if (vmx_rdtscp_supported() && !guest_cpuid_has_rdtscp(vcpu)) {
+		clear_exe_ctrl |= SECONDARY_EXEC_RDTSCP;
 
-		if (nested && !vmx->rdtscp_enabled)
+		if (nested)
 			vmx->nested.nested_vmx_secondary_ctls_high &=
 				~SECONDARY_EXEC_RDTSCP;
 	}
-- 
2.4.3

Re: [PATCH 3/9] KVM: x86: add pcommit support

[Paolo Bonzini]

On 21/08/2015 06:50, Xiao Guangrong wrote:
> Pass PCOMMIT CPU feature to guest to enable PCOMMIT instruction
> 
> Currently we do not catch pcommit instruction for L1 guest and
> allow L1 to catch this instruction for L2
> 
> The specification locates at:
> https://software.intel.com/sites/default/files/managed/0d/53/319433-022.pdf
> 
> Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com>
> ---
>  arch/x86/include/asm/vmx.h      |  2 +-
>  arch/x86/include/uapi/asm/vmx.h |  4 +++-
>  arch/x86/kvm/cpuid.c            |  2 +-
>  arch/x86/kvm/cpuid.h            |  8 ++++++++
>  arch/x86/kvm/vmx.c              | 29 ++++++++++++++++++++++++-----
>  5 files changed, 37 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
> index 9299ae5..e2ad08a 100644
> --- a/arch/x86/include/asm/vmx.h
> +++ b/arch/x86/include/asm/vmx.h
> @@ -72,7 +72,7 @@
>  #define SECONDARY_EXEC_SHADOW_VMCS              0x00004000
>  #define SECONDARY_EXEC_ENABLE_PML               0x00020000
>  #define SECONDARY_EXEC_XSAVES			0x00100000
> -
> +#define SECONDARY_EXEC_PCOMMIT			0x00200000
>  
>  #define PIN_BASED_EXT_INTR_MASK                 0x00000001
>  #define PIN_BASED_NMI_EXITING                   0x00000008
> diff --git a/arch/x86/include/uapi/asm/vmx.h b/arch/x86/include/uapi/asm/vmx.h
> index 37fee27..5b15d94 100644
> --- a/arch/x86/include/uapi/asm/vmx.h
> +++ b/arch/x86/include/uapi/asm/vmx.h
> @@ -78,6 +78,7 @@
>  #define EXIT_REASON_PML_FULL            62
>  #define EXIT_REASON_XSAVES              63
>  #define EXIT_REASON_XRSTORS             64
> +#define EXIT_REASON_PCOMMIT             65
>  
>  #define VMX_EXIT_REASONS \
>  	{ EXIT_REASON_EXCEPTION_NMI,         "EXCEPTION_NMI" }, \
> @@ -126,7 +127,8 @@
>  	{ EXIT_REASON_INVVPID,               "INVVPID" }, \
>  	{ EXIT_REASON_INVPCID,               "INVPCID" }, \
>  	{ EXIT_REASON_XSAVES,                "XSAVES" }, \
> -	{ EXIT_REASON_XRSTORS,               "XRSTORS" }
> +	{ EXIT_REASON_XRSTORS,               "XRSTORS" }, \
> +	{ EXIT_REASON_PCOMMIT,               "PCOMMIT" }
>  
>  #define VMX_ABORT_SAVE_GUEST_MSR_FAIL        1
>  #define VMX_ABORT_LOAD_HOST_MSR_FAIL         4
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 962fc7d..faeb0b3 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -348,7 +348,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>  		F(FSGSBASE) | F(BMI1) | F(HLE) | F(AVX2) | F(SMEP) |
>  		F(BMI2) | F(ERMS) | f_invpcid | F(RTM) | f_mpx | F(RDSEED) |
>  		F(ADX) | F(SMAP) | F(AVX512F) | F(AVX512PF) | F(AVX512ER) |
> -		F(AVX512CD) | F(CLFLUSHOPT) | F(CLWB);
> +		F(AVX512CD) | F(CLFLUSHOPT) | F(CLWB) | F(PCOMMIT);
>  
>  	/* cpuid 0xD.1.eax */
>  	const u32 kvm_supported_word10_x86_features =
> diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
> index dd05b9c..aed7bfe 100644
> --- a/arch/x86/kvm/cpuid.h
> +++ b/arch/x86/kvm/cpuid.h
> @@ -133,4 +133,12 @@ static inline bool guest_cpuid_has_mpx(struct kvm_vcpu *vcpu)
>  	best = kvm_find_cpuid_entry(vcpu, 7, 0);
>  	return best && (best->ebx & bit(X86_FEATURE_MPX));
>  }
> +
> +static inline bool guest_cpuid_has_pcommit(struct kvm_vcpu *vcpu)
> +{
> +	struct kvm_cpuid_entry2 *best;
> +
> +	best = kvm_find_cpuid_entry(vcpu, 7, 0);
> +	return best && (best->ebx & bit(X86_FEATURE_PCOMMIT));
> +}
>  #endif
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 4cf25b9..b526c61 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -2474,7 +2474,8 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx)
>  		SECONDARY_EXEC_APIC_REGISTER_VIRT |
>  		SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
>  		SECONDARY_EXEC_WBINVD_EXITING |
> -		SECONDARY_EXEC_XSAVES;
> +		SECONDARY_EXEC_XSAVES |
> +		SECONDARY_EXEC_PCOMMIT;
>  
>  	if (enable_ept) {
>  		/* nested EPT: emulate EPT also to L1 */
> @@ -3015,7 +3016,8 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
>  			SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
>  			SECONDARY_EXEC_SHADOW_VMCS |
>  			SECONDARY_EXEC_XSAVES |
> -			SECONDARY_EXEC_ENABLE_PML;
> +			SECONDARY_EXEC_ENABLE_PML |
> +			SECONDARY_EXEC_PCOMMIT;
>  		if (adjust_vmx_controls(min2, opt2,
>  					MSR_IA32_VMX_PROCBASED_CTLS2,
>  					&_cpu_based_2nd_exec_control) < 0)
> @@ -4570,6 +4572,9 @@ static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx)
>  	/* PML is enabled/disabled in creating/destorying vcpu */
>  	exec_control &= ~SECONDARY_EXEC_ENABLE_PML;
>  
> +	/* Currently, we allow L1 guest to directly run pcommit instruction. */
> +	exec_control &= ~SECONDARY_EXEC_PCOMMIT;
> +
>  	return exec_control;
>  }
>  
> @@ -4613,10 +4618,9 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx)
>  
>  	vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, vmx_exec_control(vmx));
>  
> -	if (cpu_has_secondary_exec_ctrls()) {
> +	if (cpu_has_secondary_exec_ctrls())
>  		vmcs_write32(SECONDARY_VM_EXEC_CONTROL,
>  				vmx_secondary_exec_control(vmx));
> -	}
>  
>  	if (vmx_cpu_uses_apicv(&vmx->vcpu)) {
>  		vmcs_write64(EOI_EXIT_BITMAP0, 0);
> @@ -7208,6 +7212,13 @@ static int handle_pml_full(struct kvm_vcpu *vcpu)
>  	return 1;
>  }
>  
> +static int handle_pcommit(struct kvm_vcpu *vcpu)
> +{
> +	/* we never catch pcommit instruct for L1 guest. */
> +	BUG();

Please WARN instead.

> +	return 1;
> +}
> +
>  /*
>   * The exit handlers return 1 if the exit was handled fully and guest execution
>   * may resume.  Otherwise they set the kvm_run parameter to indicate what needs
> @@ -7258,6 +7269,7 @@ static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = {
>  	[EXIT_REASON_XSAVES]                  = handle_xsaves,
>  	[EXIT_REASON_XRSTORS]                 = handle_xrstors,
>  	[EXIT_REASON_PML_FULL]		      = handle_pml_full,
> +	[EXIT_REASON_PCOMMIT]                 = handle_pcommit,
>  };
>  
>  static const int kvm_vmx_max_exit_handlers =
> @@ -7559,6 +7571,8 @@ static bool nested_vmx_exit_handled(struct kvm_vcpu *vcpu)
>  		 * the XSS exit bitmap in vmcs12.
>  		 */
>  		return nested_cpu_has2(vmcs12, SECONDARY_EXEC_XSAVES);
> +	case EXIT_REASON_PCOMMIT:
> +		return nested_cpu_has2(vmcs12, SECONDARY_EXEC_PCOMMIT);
>  	default:
>  		return true;
>  	}
> @@ -8688,6 +8702,10 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
>  		if (best)
>  			best->ebx &= ~bit(X86_FEATURE_INVPCID);
>  	}
> +
> +	if (!guest_cpuid_has_pcommit(vcpu) && nested)
> +		vmx->nested.nested_vmx_secondary_ctls_high &=
> +			~SECONDARY_EXEC_PCOMMIT;

Why is this needed?

Paolo

>  }
>  
>  static void vmx_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry)
> @@ -9301,7 +9319,8 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12)
>  		exec_control &= ~(SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
>  				  SECONDARY_EXEC_RDTSCP |
>  				  SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
> -				  SECONDARY_EXEC_APIC_REGISTER_VIRT);
> +				  SECONDARY_EXEC_APIC_REGISTER_VIRT |
> +				  SECONDARY_EXEC_PCOMMIT);
>  		if (nested_cpu_has(vmcs12,
>  				CPU_BASED_ACTIVATE_SECONDARY_CONTROLS))
>  			exec_control |= vmcs12->secondary_vm_exec_control;
> 

Re: [PATCH 8/9] KVM: VMX: introduce set_clear_2nd_exec_ctrl()

[Paolo Bonzini]

On 21/08/2015 06:50, Xiao Guangrong wrote:
>  
> +static void set_clear_2nd_exec_ctrl(u32 ctrls, bool set)
> +{
> +	u32 exec_ctrl = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
> +
> +	if (set)
> +		exec_ctrl |= ctrls;
> +	else
> +		exec_ctrl &= ~ctrls;
> +
> +	vmcs_write32(SECONDARY_VM_EXEC_CONTROL, exec_ctrl);
> +}

The second argument is always true.  Do you have any plans for it?

Should we instead add functions like vmcs_or32 and vmcs_clear32?

Paolo

Re: [PATCH 6/9] KVM: VMX: simplify invpcid handling in vmx_cpuid_update()

[Paolo Bonzini]

On 21/08/2015 06:50, Xiao Guangrong wrote:
> +	if (vmx_invpcid_supported() && (!best ||

Please start the "(" subexpression on a new line.

Paolo

> +	    !(best->ebx & bit(X86_FEATURE_INVPCID)) ||
> +	    !guest_cpuid_has_pcid(vcpu))) {

Re: [PATCH 3/9] KVM: x86: add pcommit support

[Xiao Guangrong]

On 09/07/2015 07:18 PM, Paolo Bonzini wrote:

>>
>> +static int handle_pcommit(struct kvm_vcpu *vcpu)
>> +{
>> +	/* we never catch pcommit instruct for L1 guest. */
>> +	BUG();
>
> Please WARN instead.
>

Okay.

>> +	return 1;
>> +}
>> +
>>   /*
>>    * The exit handlers return 1 if the exit was handled fully and guest execution
>>    * may resume.  Otherwise they set the kvm_run parameter to indicate what needs
>> @@ -7258,6 +7269,7 @@ static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = {
>>   	[EXIT_REASON_XSAVES]                  = handle_xsaves,
>>   	[EXIT_REASON_XRSTORS]                 = handle_xrstors,
>>   	[EXIT_REASON_PML_FULL]		      = handle_pml_full,
>> +	[EXIT_REASON_PCOMMIT]                 = handle_pcommit,
>>   };
>>
>>   static const int kvm_vmx_max_exit_handlers =
>> @@ -7559,6 +7571,8 @@ static bool nested_vmx_exit_handled(struct kvm_vcpu *vcpu)
>>   		 * the XSS exit bitmap in vmcs12.
>>   		 */
>>   		return nested_cpu_has2(vmcs12, SECONDARY_EXEC_XSAVES);
>> +	case EXIT_REASON_PCOMMIT:
>> +		return nested_cpu_has2(vmcs12, SECONDARY_EXEC_PCOMMIT);
>>   	default:
>>   		return true;
>>   	}
>> @@ -8688,6 +8702,10 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
>>   		if (best)
>>   			best->ebx &= ~bit(X86_FEATURE_INVPCID);
>>   	}
>> +
>> +	if (!guest_cpuid_has_pcommit(vcpu) && nested)
>> +		vmx->nested.nested_vmx_secondary_ctls_high &=
>> +			~SECONDARY_EXEC_PCOMMIT;
>
> Why is this needed?
>

If pcommit is not allowed in L1 guest, L1 is not allowed to intercept pcommit
for L2.

BTW, the spec saied:

| IA32_VMX_PROCBASED_CTLS2[53] (which enumerates support for the 1-setting of “PCOMMIT exiting”) is
| always the same as CPUID.07H:EBX.PCOMMIT[bit 22]. Thus, software can set “PCOMMIT exiting” to 1
| if and only if the PCOMMIT instruction is enumerated via CPUID

Re: [PATCH 6/9] KVM: VMX: simplify invpcid handling in vmx_cpuid_update()

[Xiao Guangrong]

On 09/07/2015 07:28 PM, Paolo Bonzini wrote:
>
>
> On 21/08/2015 06:50, Xiao Guangrong wrote:
>> +	if (vmx_invpcid_supported() && (!best ||
>
> Please start the "(" subexpression on a new line.
>

Okay, will fix.

Re: [PATCH 8/9] KVM: VMX: introduce set_clear_2nd_exec_ctrl()

[Xiao Guangrong]

On 09/07/2015 07:27 PM, Paolo Bonzini wrote:
>
>
> On 21/08/2015 06:50, Xiao Guangrong wrote:
>>
>> +static void set_clear_2nd_exec_ctrl(u32 ctrls, bool set)
>> +{
>> +	u32 exec_ctrl = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
>> +
>> +	if (set)
>> +		exec_ctrl |= ctrls;
>> +	else
>> +		exec_ctrl &= ~ctrls;
>> +
>> +	vmcs_write32(SECONDARY_VM_EXEC_CONTROL, exec_ctrl);
>> +}
>
> The second argument is always true.

No...

There are 3 places calling this function with set=false：
nested_release_vmcs12(), vmx_disable_pml() and
vmx_cpuid_update()


> Do you have any plans for it?
>
> Should we instead add functions like vmcs_or32 and vmcs_clear32?
>

Sounds good to me, will do it.

Re: [PATCH 8/9] KVM: VMX: introduce set_clear_2nd_exec_ctrl()

[Paolo Bonzini]

On 08/09/2015 16:24, Xiao Guangrong wrote:
>>
>> The second argument is always true.
> 
> No...
> 
> There are 3 places calling this function with set=false：
> nested_release_vmcs12(), vmx_disable_pml() and
> vmx_cpuid_update()

You're right.  It's always constant---I don't know why I wrote it's
always true, and then suggested vmcs_clear32...

Paolo

Re: [PATCH 3/9] KVM: x86: add pcommit support

[Paolo Bonzini]

On 08/09/2015 16:17, Xiao Guangrong wrote:
> 
> BTW, the spec saied:
> 
> | IA32_VMX_PROCBASED_CTLS2[53] (which enumerates support for the
> 1-setting of “PCOMMIT exiting”) is
> | always the same as CPUID.07H:EBX.PCOMMIT[bit 22]. Thus, software can
> set “PCOMMIT exiting” to 1
> | if and only if the PCOMMIT instruction is enumerated via CPUID

Thanks.  Can you add it to the commit message ("allow L1 to catch this
instruction for L2 if, as required by the spec, L1 can enumerate the
PCOMMIT instruction via CPUID").

Paolo