linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/4] KVM: Emulate UMIP (or almost do so)
@ 2016-07-12 19:20 Paolo Bonzini
  2016-07-12 19:20 ` [RFC PATCH 1/4] x86: add UMIP feature and CR4 bit Paolo Bonzini
                   ` (5 more replies)
  0 siblings, 6 replies; 32+ messages in thread
From: Paolo Bonzini @ 2016-07-12 19:20 UTC (permalink / raw)
  To: linux-kernel, kvm

UMIP (User-Mode Instruction Prevention) is a feature of future
Intel processors (Cannonlake?) that blocks SLDT, SGDT, STR, SIDT
and SMSW from user-mode processes.

The idea here is to use virtualization intercepts to emulate UMIP; it
slows down the instructions when they're executed in ring 0, but they
are really never executed in practice.  On AMD systems it's possible
to emulate it entirely; instead on Intel systems it's *almost* possible
to emulate it, because SMSW doesn't cause a vmexit, and hence SMSW will
not fault.

This patch series provides the infrastructure and implements it on
Intel.  I tested it through kvm-unit-tests.

Still I think the idea is interesting, even if it's buggy for current
Intel processors.  Any opinions?

Paolo

Paolo Bonzini (4):
  x86: add UMIP feature and CR4 bit
  KVM: x86: emulate sldt and str
  KVM: x86: add support for emulating UMIP
  KVM: vmx: add support for emulating UMIP

 arch/x86/include/asm/cpufeatures.h          |  1 +
 arch/x86/include/asm/kvm_host.h             |  3 ++-
 arch/x86/include/asm/vmx.h                  |  1 +
 arch/x86/include/uapi/asm/processor-flags.h |  2 ++
 arch/x86/include/uapi/asm/vmx.h             |  4 +++
 arch/x86/kvm/cpuid.c                        |  5 +++-
 arch/x86/kvm/cpuid.h                        |  8 ++++++
 arch/x86/kvm/emulate.c                      | 40 ++++++++++++++++++++++++-----
 arch/x86/kvm/svm.c                          |  6 +++++
 arch/x86/kvm/vmx.c                          | 40 ++++++++++++++++++++++++++++-
 arch/x86/kvm/x86.c                          |  3 +++
 11 files changed, 104 insertions(+), 9 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [RFC PATCH 1/4] x86: add UMIP feature and CR4 bit
  2016-07-12 19:20 [RFC PATCH 0/4] KVM: Emulate UMIP (or almost do so) Paolo Bonzini
@ 2016-07-12 19:20 ` Paolo Bonzini
  2016-07-12 19:20 ` [RFC PATCH 2/4] KVM: x86: emulate sldt and str Paolo Bonzini
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2016-07-12 19:20 UTC (permalink / raw)
  To: linux-kernel, kvm

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/include/asm/cpufeatures.h          | 1 +
 arch/x86/include/uapi/asm/processor-flags.h | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 4a413485f9eb..33703c83b647 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -279,6 +279,7 @@
 #define X86_FEATURE_AVIC	(15*32+13) /* Virtual Interrupt Controller */
 
 /* Intel-defined CPU features, CPUID level 0x00000007:0 (ecx), word 16 */
+#define X86_FEATURE_UMIP	(16*32+ 2) /* User-Mode Instruction Prevention */
 #define X86_FEATURE_PKU		(16*32+ 3) /* Protection Keys for Userspace */
 #define X86_FEATURE_OSPKE	(16*32+ 4) /* OS Protection Keys Enable */
 
diff --git a/arch/x86/include/uapi/asm/processor-flags.h b/arch/x86/include/uapi/asm/processor-flags.h
index 567de50a4c2a..5ef9d6f459c9 100644
--- a/arch/x86/include/uapi/asm/processor-flags.h
+++ b/arch/x86/include/uapi/asm/processor-flags.h
@@ -104,6 +104,8 @@
 #define X86_CR4_OSFXSR		_BITUL(X86_CR4_OSFXSR_BIT)
 #define X86_CR4_OSXMMEXCPT_BIT	10 /* enable unmasked SSE exceptions */
 #define X86_CR4_OSXMMEXCPT	_BITUL(X86_CR4_OSXMMEXCPT_BIT)
+#define X86_CR4_UMIP_BIT	11 /* enable user-mode instruction prevention */
+#define X86_CR4_UMIP		_BITUL(X86_CR4_UMIP_BIT)
 #define X86_CR4_VMXE_BIT	13 /* enable VMX virtualization */
 #define X86_CR4_VMXE		_BITUL(X86_CR4_VMXE_BIT)
 #define X86_CR4_SMXE_BIT	14 /* enable safer mode (TXT) */
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 2/4] KVM: x86: emulate sldt and str
  2016-07-12 19:20 [RFC PATCH 0/4] KVM: Emulate UMIP (or almost do so) Paolo Bonzini
  2016-07-12 19:20 ` [RFC PATCH 1/4] x86: add UMIP feature and CR4 bit Paolo Bonzini
@ 2016-07-12 19:20 ` Paolo Bonzini
  2016-07-12 19:20 ` [RFC PATCH 3/4] KVM: x86: add support for emulating UMIP Paolo Bonzini
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2016-07-12 19:20 UTC (permalink / raw)
  To: linux-kernel, kvm

These will be used to emulate UMIP through VMX descriptor-table
vmexits.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/emulate.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index a2f24af3c999..3e09c7461ba8 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3577,15 +3577,20 @@ static int em_rdmsr(struct x86_emulate_ctxt *ctxt)
 	return X86EMUL_CONTINUE;
 }
 
+static int em_store_sreg(struct x86_emulate_ctxt *ctxt, int segment)
+{
+	ctxt->dst.val = get_segment_selector(ctxt, segment);
+	if (ctxt->dst.bytes == 4 && ctxt->dst.type == OP_MEM)
+		ctxt->dst.bytes = 2;
+	return X86EMUL_CONTINUE;
+}
+
 static int em_mov_rm_sreg(struct x86_emulate_ctxt *ctxt)
 {
 	if (ctxt->modrm_reg > VCPU_SREG_GS)
 		return emulate_ud(ctxt);
 
-	ctxt->dst.val = get_segment_selector(ctxt, ctxt->modrm_reg);
-	if (ctxt->dst.bytes == 4 && ctxt->dst.type == OP_MEM)
-		ctxt->dst.bytes = 2;
-	return X86EMUL_CONTINUE;
+	return em_store_sreg(ctxt, ctxt->modrm_reg);
 }
 
 static int em_mov_sreg_rm(struct x86_emulate_ctxt *ctxt)
@@ -3603,6 +3608,11 @@ static int em_mov_sreg_rm(struct x86_emulate_ctxt *ctxt)
 	return load_segment_descriptor(ctxt, sel, ctxt->modrm_reg);
 }
 
+static int em_sldt(struct x86_emulate_ctxt *ctxt)
+{
+	return em_store_sreg(ctxt, VCPU_SREG_LDTR);
+}
+
 static int em_lldt(struct x86_emulate_ctxt *ctxt)
 {
 	u16 sel = ctxt->src.val;
@@ -3612,6 +3622,11 @@ static int em_lldt(struct x86_emulate_ctxt *ctxt)
 	return load_segment_descriptor(ctxt, sel, VCPU_SREG_LDTR);
 }
 
+static int em_str(struct x86_emulate_ctxt *ctxt)
+{
+	return em_store_sreg(ctxt, VCPU_SREG_TR);
+}
+
 static int em_ltr(struct x86_emulate_ctxt *ctxt)
 {
 	u16 sel = ctxt->src.val;
@@ -4161,8 +4176,8 @@ static const struct opcode group5[] = {
 };
 
 static const struct opcode group6[] = {
-	DI(Prot | DstMem,	sldt),
-	DI(Prot | DstMem,	str),
+	II(Prot | DstMem,	   em_sldt, sldt),
+	II(Prot | DstMem,	   em_str, str),
 	II(Prot | Priv | SrcMem16, em_lldt, lldt),
 	II(Prot | Priv | SrcMem16, em_ltr, ltr),
 	N, N, N, N,
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 3/4] KVM: x86: add support for emulating UMIP
  2016-07-12 19:20 [RFC PATCH 0/4] KVM: Emulate UMIP (or almost do so) Paolo Bonzini
  2016-07-12 19:20 ` [RFC PATCH 1/4] x86: add UMIP feature and CR4 bit Paolo Bonzini
  2016-07-12 19:20 ` [RFC PATCH 2/4] KVM: x86: emulate sldt and str Paolo Bonzini
@ 2016-07-12 19:20 ` Paolo Bonzini
  2016-07-13 20:03   ` Radim Krčmář
  2016-07-12 19:20 ` [RFC PATCH 4/4] KVM: vmx: " Paolo Bonzini
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 32+ messages in thread
From: Paolo Bonzini @ 2016-07-12 19:20 UTC (permalink / raw)
  To: linux-kernel, kvm

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/include/asm/kvm_host.h |  3 ++-
 arch/x86/kvm/cpuid.c            |  5 ++++-
 arch/x86/kvm/cpuid.h            |  8 ++++++++
 arch/x86/kvm/emulate.c          | 13 +++++++++++++
 arch/x86/kvm/svm.c              |  6 ++++++
 arch/x86/kvm/vmx.c              |  6 ++++++
 arch/x86/kvm/x86.c              |  3 +++
 7 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index d8204817a5fe..9e1a1db65c7b 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -86,7 +86,7 @@
 			  | X86_CR4_PGE | X86_CR4_PCE | X86_CR4_OSFXSR | X86_CR4_PCIDE \
 			  | X86_CR4_OSXSAVE | X86_CR4_SMEP | X86_CR4_FSGSBASE \
 			  | X86_CR4_OSXMMEXCPT | X86_CR4_VMXE | X86_CR4_SMAP \
-			  | X86_CR4_PKE))
+			  | X86_CR4_PKE | X86_CR4_UMIP))
 
 #define CR8_RESERVED_BITS (~(unsigned long)X86_CR8_TPR)
 
@@ -971,6 +971,7 @@ struct kvm_x86_ops {
 	void (*handle_external_intr)(struct kvm_vcpu *vcpu);
 	bool (*mpx_supported)(void);
 	bool (*xsaves_supported)(void);
+	bool (*umip_emulated)(void);
 
 	int (*check_nested_events)(struct kvm_vcpu *vcpu, bool external_intr);
 
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 7597b42a8a88..ac80e74056c4 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -315,6 +315,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 	unsigned f_invpcid = kvm_x86_ops->invpcid_supported() ? F(INVPCID) : 0;
 	unsigned f_mpx = kvm_mpx_supported() ? F(MPX) : 0;
 	unsigned f_xsaves = kvm_x86_ops->xsaves_supported() ? F(XSAVES) : 0;
+	unsigned f_umip = kvm_x86_ops->umip_emulated() ? F(UMIP) : 0;
 
 	/* cpuid 1.edx */
 	const u32 kvm_cpuid_1_edx_x86_features =
@@ -373,7 +374,8 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 		F(XSAVEOPT) | F(XSAVEC) | F(XGETBV1) | f_xsaves;
 
 	/* cpuid 7.0.ecx*/
-	const u32 kvm_cpuid_7_0_ecx_x86_features = F(PKU) | 0 /*OSPKE*/;
+	const u32 kvm_cpuid_7_0_ecx_x86_features = F(PKU) | 0 /*OSPKE*/ |
+		F(UMIP);
 
 	/* all calls to cpuid_count() should be made on the same cpu */
 	get_cpu();
@@ -454,6 +456,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 			entry->ebx |= F(TSC_ADJUST);
 			entry->ecx &= kvm_cpuid_7_0_ecx_x86_features;
 			cpuid_mask(&entry->ecx, CPUID_7_ECX);
+			entry->ecx |= f_umip;
 			/* PKU is not yet implemented for shadow paging. */
 			if (!tdp_enabled)
 				entry->ecx &= ~F(PKU);
diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
index e17a74b1d852..422de9db1785 100644
--- a/arch/x86/kvm/cpuid.h
+++ b/arch/x86/kvm/cpuid.h
@@ -88,6 +88,14 @@ static inline bool guest_cpuid_has_pku(struct kvm_vcpu *vcpu)
 	return best && (best->ecx & bit(X86_FEATURE_PKU));
 }
 
+static inline bool guest_cpuid_has_umip(struct kvm_vcpu *vcpu)
+{
+	struct kvm_cpuid_entry2 *best;
+
+	best = kvm_find_cpuid_entry(vcpu, 7, 0);
+	return best && (best->ecx & bit(X86_FEATURE_UMIP));
+}
+
 static inline bool guest_cpuid_has_longmode(struct kvm_vcpu *vcpu)
 {
 	struct kvm_cpuid_entry2 *best;
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 3e09c7461ba8..d2f568a0fc46 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -3579,6 +3579,11 @@ static int em_rdmsr(struct x86_emulate_ctxt *ctxt)
 
 static int em_store_sreg(struct x86_emulate_ctxt *ctxt, int segment)
 {
+	if (segment > VCPU_SREG_GS &&
+	    (ctxt->ops->get_cr(ctxt, 4) & X86_CR4_UMIP) &&
+	    ctxt->ops->cpl(ctxt) > 0)
+		return emulate_gp(ctxt, 0);
+
 	ctxt->dst.val = get_segment_selector(ctxt, segment);
 	if (ctxt->dst.bytes == 4 && ctxt->dst.type == OP_MEM)
 		ctxt->dst.bytes = 2;
@@ -3679,6 +3684,10 @@ static int emulate_store_desc_ptr(struct x86_emulate_ctxt *ctxt,
 {
 	struct desc_ptr desc_ptr;
 
+	if ((ctxt->ops->get_cr(ctxt, 4) & X86_CR4_UMIP) &&
+	    ctxt->ops->cpl(ctxt) > 0)
+		return emulate_gp(ctxt, 0);
+
 	if (ctxt->mode == X86EMUL_MODE_PROT64)
 		ctxt->op_bytes = 8;
 	get(ctxt, &desc_ptr);
@@ -3738,6 +3747,10 @@ static int em_lidt(struct x86_emulate_ctxt *ctxt)
 
 static int em_smsw(struct x86_emulate_ctxt *ctxt)
 {
+	if ((ctxt->ops->get_cr(ctxt, 4) & X86_CR4_UMIP) &&
+	    ctxt->ops->cpl(ctxt) > 0)
+		return emulate_gp(ctxt, 0);
+
 	if (ctxt->dst.type == OP_MEM)
 		ctxt->dst.bytes = 2;
 	ctxt->dst.val = ctxt->ops->get_cr(ctxt, 0);
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 5bfdbbf1ce79..227258ddab24 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -4736,6 +4736,11 @@ static bool svm_xsaves_supported(void)
 	return false;
 }
 
+static bool svm_umip_emulated(void)
+{
+	return false;
+}
+
 static bool svm_has_wbinvd_exit(void)
 {
 	return true;
@@ -5054,6 +5059,7 @@ static struct kvm_x86_ops svm_x86_ops = {
 	.invpcid_supported = svm_invpcid_supported,
 	.mpx_supported = svm_mpx_supported,
 	.xsaves_supported = svm_xsaves_supported,
+	.umip_emulated = svm_umip_emulated,
 
 	.set_supported_cpuid = svm_set_supported_cpuid,
 
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index a9e35b3f3ea6..b133fc3d6a7d 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -8595,6 +8595,11 @@ static bool vmx_xsaves_supported(void)
 		SECONDARY_EXEC_XSAVES;
 }
 
+static bool vmx_umip_emulated(void)
+{
+	return false;
+}
+
 static void vmx_recover_nmi_blocking(struct vcpu_vmx *vmx)
 {
 	u32 exit_intr_info;
@@ -11239,6 +11244,7 @@ static struct kvm_x86_ops vmx_x86_ops = {
 	.handle_external_intr = vmx_handle_external_intr,
 	.mpx_supported = vmx_mpx_supported,
 	.xsaves_supported = vmx_xsaves_supported,
+	.umip_emulated = vmx_umip_emulated,
 
 	.check_nested_events = vmx_check_nested_events,
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 548f546f92db..85c7e8135342 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -746,6 +746,9 @@ int kvm_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
 	if (!guest_cpuid_has_pku(vcpu) && (cr4 & X86_CR4_PKE))
 		return 1;
 
+	if (!guest_cpuid_has_umip(vcpu) && (cr4 & X86_CR4_UMIP))
+		return 1;
+
 	if (is_long_mode(vcpu)) {
 		if (!(cr4 & X86_CR4_PAE))
 			return 1;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [RFC PATCH 4/4] KVM: vmx: add support for emulating UMIP
  2016-07-12 19:20 [RFC PATCH 0/4] KVM: Emulate UMIP (or almost do so) Paolo Bonzini
                   ` (2 preceding siblings ...)
  2016-07-12 19:20 ` [RFC PATCH 3/4] KVM: x86: add support for emulating UMIP Paolo Bonzini
@ 2016-07-12 19:20 ` Paolo Bonzini
  2016-07-13  9:21   ` Yang Zhang
  2016-07-13 20:30   ` Radim Krčmář
  2016-07-13  8:29 ` [RFC PATCH 0/4] KVM: Emulate UMIP (or almost do so) Yang Zhang
  2016-12-13  4:03 ` Li, Liang Z
  5 siblings, 2 replies; 32+ messages in thread
From: Paolo Bonzini @ 2016-07-12 19:20 UTC (permalink / raw)
  To: linux-kernel, kvm

UMIP (User-Mode Instruction Prevention) is a feature of future
Intel processors (Cannonlake?) that blocks SLDT, SGDT, STR, SIDT
and SMSW from user-mode processes.

On Intel systems it's *almost* possible to emulate it; it slows
down the instructions when they're executed in ring 0, but they
are really never executed in practice.  The catch is that SMSW
doesn't cause a vmexit, and hence SMSW will not fault.

When UMIP is enabled but not supported by the host, descriptor table
exits are enabled, and the emulator takes care of injecting a #GP when
any of SLDT, SGDT, STR, SIDT are encountered.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/include/asm/vmx.h      |  1 +
 arch/x86/include/uapi/asm/vmx.h |  4 ++++
 arch/x86/kvm/vmx.c              | 36 ++++++++++++++++++++++++++++++++++--
 3 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index 14c63c7e8337..8b95f0ec0190 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -60,6 +60,7 @@
  */
 #define SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES 0x00000001
 #define SECONDARY_EXEC_ENABLE_EPT               0x00000002
+#define SECONDARY_EXEC_DESC			0x00000004
 #define SECONDARY_EXEC_RDTSCP			0x00000008
 #define SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE   0x00000010
 #define SECONDARY_EXEC_ENABLE_VPID              0x00000020
diff --git a/arch/x86/include/uapi/asm/vmx.h b/arch/x86/include/uapi/asm/vmx.h
index 5b15d94a33f8..2f9a69b9559c 100644
--- a/arch/x86/include/uapi/asm/vmx.h
+++ b/arch/x86/include/uapi/asm/vmx.h
@@ -65,6 +65,8 @@
 #define EXIT_REASON_TPR_BELOW_THRESHOLD 43
 #define EXIT_REASON_APIC_ACCESS         44
 #define EXIT_REASON_EOI_INDUCED         45
+#define EXIT_REASON_GDTR_IDTR           46
+#define EXIT_REASON_LDTR_TR             47
 #define EXIT_REASON_EPT_VIOLATION       48
 #define EXIT_REASON_EPT_MISCONFIG       49
 #define EXIT_REASON_INVEPT              50
@@ -114,6 +116,8 @@
 	{ EXIT_REASON_MCE_DURING_VMENTRY,    "MCE_DURING_VMENTRY" }, \
 	{ EXIT_REASON_TPR_BELOW_THRESHOLD,   "TPR_BELOW_THRESHOLD" }, \
 	{ EXIT_REASON_APIC_ACCESS,           "APIC_ACCESS" }, \
+	{ EXIT_REASON_GDTR_IDTR,	     "GDTR_IDTR" }, \
+	{ EXIT_REASON_LDTR_TR,		     "LDTR_TR" }, \
 	{ EXIT_REASON_EPT_VIOLATION,         "EPT_VIOLATION" }, \
 	{ EXIT_REASON_EPT_MISCONFIG,         "EPT_MISCONFIG" }, \
 	{ EXIT_REASON_INVEPT,                "INVEPT" }, \
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index b133fc3d6a7d..0706e363c5e3 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -3342,6 +3342,7 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
 			SECONDARY_EXEC_ENABLE_EPT |
 			SECONDARY_EXEC_UNRESTRICTED_GUEST |
 			SECONDARY_EXEC_PAUSE_LOOP_EXITING |
+			SECONDARY_EXEC_DESC |
 			SECONDARY_EXEC_RDTSCP |
 			SECONDARY_EXEC_ENABLE_INVPCID |
 			SECONDARY_EXEC_APIC_REGISTER_VIRT |
@@ -3967,6 +3968,14 @@ static int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
 		(to_vmx(vcpu)->rmode.vm86_active ?
 		 KVM_RMODE_VM_CR4_ALWAYS_ON : KVM_PMODE_VM_CR4_ALWAYS_ON);
 
+	if ((cr4 & X86_CR4_UMIP) && !boot_cpu_has(X86_FEATURE_UMIP)) {
+		vmcs_set_bits(SECONDARY_VM_EXEC_CONTROL,
+			      SECONDARY_EXEC_DESC);
+		hw_cr4 &= ~X86_CR4_UMIP;
+	} else
+		vmcs_clear_bits(SECONDARY_VM_EXEC_CONTROL,
+				SECONDARY_EXEC_DESC);
+
 	if (cr4 & X86_CR4_VMXE) {
 		/*
 		 * To use VMXON (and later other VMX instructions), a guest
@@ -4913,6 +4922,11 @@ static u32 vmx_exec_control(struct vcpu_vmx *vmx)
 static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx)
 {
 	u32 exec_control = vmcs_config.cpu_based_2nd_exec_ctrl;
+
+	/* SECONDARY_EXEC_DESC is enabled/disabled on writes to CR4.UMIP,
+	 * in vmx_set_cr4.  */
+	exec_control &= ~SECONDARY_EXEC_DESC;
+
 	if (!cpu_need_virtualize_apic_accesses(&vmx->vcpu))
 		exec_control &= ~SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
 	if (vmx->vpid == 0)
@@ -5649,6 +5663,12 @@ static void handle_clts(struct kvm_vcpu *vcpu)
 		vmx_set_cr0(vcpu, kvm_read_cr0_bits(vcpu, ~X86_CR0_TS));
 }
 
+static int handle_desc(struct kvm_vcpu *vcpu)
+{
+	WARN_ON(!(vcpu->arch.cr4 & X86_CR4_UMIP));
+	return emulate_instruction(vcpu, 0) == EMULATE_DONE;
+}
+
 static int handle_cr(struct kvm_vcpu *vcpu)
 {
 	unsigned long exit_qualification, val;
@@ -7705,6 +7731,8 @@ static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = {
 	[EXIT_REASON_XSETBV]                  = handle_xsetbv,
 	[EXIT_REASON_TASK_SWITCH]             = handle_task_switch,
 	[EXIT_REASON_MCE_DURING_VMENTRY]      = handle_machine_check,
+	[EXIT_REASON_GDTR_IDTR]		      = handle_desc,
+	[EXIT_REASON_LDTR_TR]		      = handle_desc,
 	[EXIT_REASON_EPT_VIOLATION]	      = handle_ept_violation,
 	[EXIT_REASON_EPT_MISCONFIG]           = handle_ept_misconfig,
 	[EXIT_REASON_PAUSE_INSTRUCTION]       = handle_pause,
@@ -7972,6 +8000,8 @@ static bool nested_vmx_exit_handled(struct kvm_vcpu *vcpu)
 		return nested_cpu_has(vmcs12, CPU_BASED_MOV_DR_EXITING);
 	case EXIT_REASON_IO_INSTRUCTION:
 		return nested_vmx_exit_handled_io(vcpu, vmcs12);
+	case EXIT_REASON_GDTR_IDTR: case EXIT_REASON_LDTR_TR:
+		return nested_cpu_has2(vmcs12, SECONDARY_EXEC_DESC);
 	case EXIT_REASON_MSR_READ:
 	case EXIT_REASON_MSR_WRITE:
 		return nested_vmx_exit_handled_msr(vcpu, vmcs12, exit_reason);
@@ -8597,7 +8627,8 @@ static bool vmx_xsaves_supported(void)
 
 static bool vmx_umip_emulated(void)
 {
-	return false;
+	return vmcs_config.cpu_based_2nd_exec_ctrl &
+		SECONDARY_EXEC_DESC;
 }
 
 static void vmx_recover_nmi_blocking(struct vcpu_vmx *vmx)
@@ -9173,7 +9204,8 @@ static void vmcs_set_secondary_exec_control(u32 new_ctl)
 	u32 mask =
 		SECONDARY_EXEC_SHADOW_VMCS |
 		SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE |
-		SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
+		SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
+		SECONDARY_EXEC_DESC;
 
 	u32 cur_ctl = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 0/4] KVM: Emulate UMIP (or almost do so)
  2016-07-12 19:20 [RFC PATCH 0/4] KVM: Emulate UMIP (or almost do so) Paolo Bonzini
                   ` (3 preceding siblings ...)
  2016-07-12 19:20 ` [RFC PATCH 4/4] KVM: vmx: " Paolo Bonzini
@ 2016-07-13  8:29 ` Yang Zhang
  2016-07-13  9:34   ` Paolo Bonzini
  2016-12-13  4:03 ` Li, Liang Z
  5 siblings, 1 reply; 32+ messages in thread
From: Yang Zhang @ 2016-07-13  8:29 UTC (permalink / raw)
  To: Paolo Bonzini, linux-kernel, kvm

On 2016/7/13 3:20, Paolo Bonzini wrote:
> UMIP (User-Mode Instruction Prevention) is a feature of future
> Intel processors (Cannonlake?) that blocks SLDT, SGDT, STR, SIDT

I remember there is no Cannonlake any more. It should be Icelake. :)

> and SMSW from user-mode processes.

Do you know the background of this feature? For security or other purpose?

>
> The idea here is to use virtualization intercepts to emulate UMIP; it
> slows down the instructions when they're executed in ring 0, but they
> are really never executed in practice.  On AMD systems it's possible
> to emulate it entirely; instead on Intel systems it's *almost* possible
> to emulate it, because SMSW doesn't cause a vmexit, and hence SMSW will
> not fault.
>
> This patch series provides the infrastructure and implements it on
> Intel.  I tested it through kvm-unit-tests.
>
> Still I think the idea is interesting, even if it's buggy for current
> Intel processors.  Any opinions?
>
> Paolo
>
> Paolo Bonzini (4):
>   x86: add UMIP feature and CR4 bit
>   KVM: x86: emulate sldt and str
>   KVM: x86: add support for emulating UMIP
>   KVM: vmx: add support for emulating UMIP
>
>  arch/x86/include/asm/cpufeatures.h          |  1 +
>  arch/x86/include/asm/kvm_host.h             |  3 ++-
>  arch/x86/include/asm/vmx.h                  |  1 +
>  arch/x86/include/uapi/asm/processor-flags.h |  2 ++
>  arch/x86/include/uapi/asm/vmx.h             |  4 +++
>  arch/x86/kvm/cpuid.c                        |  5 +++-
>  arch/x86/kvm/cpuid.h                        |  8 ++++++
>  arch/x86/kvm/emulate.c                      | 40 ++++++++++++++++++++++++-----
>  arch/x86/kvm/svm.c                          |  6 +++++
>  arch/x86/kvm/vmx.c                          | 40 ++++++++++++++++++++++++++++-
>  arch/x86/kvm/x86.c                          |  3 +++
>  11 files changed, 104 insertions(+), 9 deletions(-)
>


-- 
Yang
Alibaba Cloud Computing

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 4/4] KVM: vmx: add support for emulating UMIP
  2016-07-12 19:20 ` [RFC PATCH 4/4] KVM: vmx: " Paolo Bonzini
@ 2016-07-13  9:21   ` Yang Zhang
  2016-07-13  9:35     ` Paolo Bonzini
  2016-07-13  9:35     ` Paolo Bonzini
  2016-07-13 20:30   ` Radim Krčmář
  1 sibling, 2 replies; 32+ messages in thread
From: Yang Zhang @ 2016-07-13  9:21 UTC (permalink / raw)
  To: Paolo Bonzini, linux-kernel, kvm

On 2016/7/13 3:20, Paolo Bonzini wrote:
> UMIP (User-Mode Instruction Prevention) is a feature of future
> Intel processors (Cannonlake?) that blocks SLDT, SGDT, STR, SIDT
> and SMSW from user-mode processes.
>
> On Intel systems it's *almost* possible to emulate it; it slows
> down the instructions when they're executed in ring 0, but they
> are really never executed in practice.  The catch is that SMSW
> doesn't cause a vmexit, and hence SMSW will not fault.
>
> When UMIP is enabled but not supported by the host, descriptor table
> exits are enabled, and the emulator takes care of injecting a #GP when
> any of SLDT, SGDT, STR, SIDT are encountered.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/include/asm/vmx.h      |  1 +
>  arch/x86/include/uapi/asm/vmx.h |  4 ++++
>  arch/x86/kvm/vmx.c              | 36 ++++++++++++++++++++++++++++++++++--
>  3 files changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
> index 14c63c7e8337..8b95f0ec0190 100644
> --- a/arch/x86/include/asm/vmx.h
> +++ b/arch/x86/include/asm/vmx.h
> @@ -60,6 +60,7 @@
>   */
>  #define SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES 0x00000001
>  #define SECONDARY_EXEC_ENABLE_EPT               0x00000002
> +#define SECONDARY_EXEC_DESC			0x00000004
>  #define SECONDARY_EXEC_RDTSCP			0x00000008
>  #define SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE   0x00000010
>  #define SECONDARY_EXEC_ENABLE_VPID              0x00000020
> diff --git a/arch/x86/include/uapi/asm/vmx.h b/arch/x86/include/uapi/asm/vmx.h
> index 5b15d94a33f8..2f9a69b9559c 100644
> --- a/arch/x86/include/uapi/asm/vmx.h
> +++ b/arch/x86/include/uapi/asm/vmx.h
> @@ -65,6 +65,8 @@
>  #define EXIT_REASON_TPR_BELOW_THRESHOLD 43
>  #define EXIT_REASON_APIC_ACCESS         44
>  #define EXIT_REASON_EOI_INDUCED         45
> +#define EXIT_REASON_GDTR_IDTR           46
> +#define EXIT_REASON_LDTR_TR             47
>  #define EXIT_REASON_EPT_VIOLATION       48
>  #define EXIT_REASON_EPT_MISCONFIG       49
>  #define EXIT_REASON_INVEPT              50
> @@ -114,6 +116,8 @@
>  	{ EXIT_REASON_MCE_DURING_VMENTRY,    "MCE_DURING_VMENTRY" }, \
>  	{ EXIT_REASON_TPR_BELOW_THRESHOLD,   "TPR_BELOW_THRESHOLD" }, \
>  	{ EXIT_REASON_APIC_ACCESS,           "APIC_ACCESS" }, \
> +	{ EXIT_REASON_GDTR_IDTR,	     "GDTR_IDTR" }, \
> +	{ EXIT_REASON_LDTR_TR,		     "LDTR_TR" }, \
>  	{ EXIT_REASON_EPT_VIOLATION,         "EPT_VIOLATION" }, \
>  	{ EXIT_REASON_EPT_MISCONFIG,         "EPT_MISCONFIG" }, \
>  	{ EXIT_REASON_INVEPT,                "INVEPT" }, \
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index b133fc3d6a7d..0706e363c5e3 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -3342,6 +3342,7 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf)
>  			SECONDARY_EXEC_ENABLE_EPT |
>  			SECONDARY_EXEC_UNRESTRICTED_GUEST |
>  			SECONDARY_EXEC_PAUSE_LOOP_EXITING |
> +			SECONDARY_EXEC_DESC |
>  			SECONDARY_EXEC_RDTSCP |
>  			SECONDARY_EXEC_ENABLE_INVPCID |
>  			SECONDARY_EXEC_APIC_REGISTER_VIRT |
> @@ -3967,6 +3968,14 @@ static int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
>  		(to_vmx(vcpu)->rmode.vm86_active ?
>  		 KVM_RMODE_VM_CR4_ALWAYS_ON : KVM_PMODE_VM_CR4_ALWAYS_ON);
>
> +	if ((cr4 & X86_CR4_UMIP) && !boot_cpu_has(X86_FEATURE_UMIP)) {
> +		vmcs_set_bits(SECONDARY_VM_EXEC_CONTROL,
> +			      SECONDARY_EXEC_DESC);
> +		hw_cr4 &= ~X86_CR4_UMIP;
> +	} else
> +		vmcs_clear_bits(SECONDARY_VM_EXEC_CONTROL,
> +				SECONDARY_EXEC_DESC);
> +

Since the faults based on privilege level have priority over VM exits. 
So we don't need to enable/disable SECONDARY_EXEC_DESC dynamically. 
Instead, we can set it unconditionally.

>  	if (cr4 & X86_CR4_VMXE) {
>  		/*
>  		 * To use VMXON (and later other VMX instructions), a guest
> @@ -4913,6 +4922,11 @@ static u32 vmx_exec_control(struct vcpu_vmx *vmx)
>  static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx)
>  {
>  	u32 exec_control = vmcs_config.cpu_based_2nd_exec_ctrl;
> +
> +	/* SECONDARY_EXEC_DESC is enabled/disabled on writes to CR4.UMIP,
> +	 * in vmx_set_cr4.  */
> +	exec_control &= ~SECONDARY_EXEC_DESC;
> +
>  	if (!cpu_need_virtualize_apic_accesses(&vmx->vcpu))
>  		exec_control &= ~SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
>  	if (vmx->vpid == 0)
> @@ -5649,6 +5663,12 @@ static void handle_clts(struct kvm_vcpu *vcpu)
>  		vmx_set_cr0(vcpu, kvm_read_cr0_bits(vcpu, ~X86_CR0_TS));
>  }
>
> +static int handle_desc(struct kvm_vcpu *vcpu)
> +{
> +	WARN_ON(!(vcpu->arch.cr4 & X86_CR4_UMIP));

I think WARN_ON is too heavy since a malicious guest may trigger it always.

> +	return emulate_instruction(vcpu, 0) == EMULATE_DONE;
> +}
> +
>  static int handle_cr(struct kvm_vcpu *vcpu)
>  {
>  	unsigned long exit_qualification, val;
> @@ -7705,6 +7731,8 @@ static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = {
>  	[EXIT_REASON_XSETBV]                  = handle_xsetbv,
>  	[EXIT_REASON_TASK_SWITCH]             = handle_task_switch,
>  	[EXIT_REASON_MCE_DURING_VMENTRY]      = handle_machine_check,
> +	[EXIT_REASON_GDTR_IDTR]		      = handle_desc,
> +	[EXIT_REASON_LDTR_TR]		      = handle_desc,
>  	[EXIT_REASON_EPT_VIOLATION]	      = handle_ept_violation,
>  	[EXIT_REASON_EPT_MISCONFIG]           = handle_ept_misconfig,
>  	[EXIT_REASON_PAUSE_INSTRUCTION]       = handle_pause,
> @@ -7972,6 +8000,8 @@ static bool nested_vmx_exit_handled(struct kvm_vcpu *vcpu)
>  		return nested_cpu_has(vmcs12, CPU_BASED_MOV_DR_EXITING);
>  	case EXIT_REASON_IO_INSTRUCTION:
>  		return nested_vmx_exit_handled_io(vcpu, vmcs12);
> +	case EXIT_REASON_GDTR_IDTR: case EXIT_REASON_LDTR_TR:
> +		return nested_cpu_has2(vmcs12, SECONDARY_EXEC_DESC);
>  	case EXIT_REASON_MSR_READ:
>  	case EXIT_REASON_MSR_WRITE:
>  		return nested_vmx_exit_handled_msr(vcpu, vmcs12, exit_reason);
> @@ -8597,7 +8627,8 @@ static bool vmx_xsaves_supported(void)
>
>  static bool vmx_umip_emulated(void)
>  {
> -	return false;
> +	return vmcs_config.cpu_based_2nd_exec_ctrl &
> +		SECONDARY_EXEC_DESC;
>  }
>
>  static void vmx_recover_nmi_blocking(struct vcpu_vmx *vmx)
> @@ -9173,7 +9204,8 @@ static void vmcs_set_secondary_exec_control(u32 new_ctl)
>  	u32 mask =
>  		SECONDARY_EXEC_SHADOW_VMCS |
>  		SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE |
> -		SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
> +		SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
> +		SECONDARY_EXEC_DESC;
>
>  	u32 cur_ctl = vmcs_read32(SECONDARY_VM_EXEC_CONTROL);
>
>


-- 
Yang
Alibaba Cloud Computing

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 0/4] KVM: Emulate UMIP (or almost do so)
  2016-07-13  8:29 ` [RFC PATCH 0/4] KVM: Emulate UMIP (or almost do so) Yang Zhang
@ 2016-07-13  9:34   ` Paolo Bonzini
  0 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2016-07-13  9:34 UTC (permalink / raw)
  To: Yang Zhang, linux-kernel, kvm



On 13/07/2016 10:29, Yang Zhang wrote:
> On 2016/7/13 3:20, Paolo Bonzini wrote:
>> UMIP (User-Mode Instruction Prevention) is a feature of future
>> Intel processors (Cannonlake?) that blocks SLDT, SGDT, STR, SIDT
> 
> I remember there is no Cannonlake any more. It should be Icelake. :)
> 
>> and SMSW from user-mode processes.
> 
> Do you know the background of this feature? For security or other purpose?

Yes, it's for security.  SGDT and SIDT in particular can leak
kernel-mode addresses to userspace, and can be used to defeat kernel ASLR.

SLDT, STR and SMSW aren't as bad because SLDT and STR only leak
selectors, while SMSW only leaks CR0.TS in practice.

Paolo

>>
>> The idea here is to use virtualization intercepts to emulate UMIP; it
>> slows down the instructions when they're executed in ring 0, but they
>> are really never executed in practice.  On AMD systems it's possible
>> to emulate it entirely; instead on Intel systems it's *almost* possible
>> to emulate it, because SMSW doesn't cause a vmexit, and hence SMSW will
>> not fault.
>>
>> This patch series provides the infrastructure and implements it on
>> Intel.  I tested it through kvm-unit-tests.
>>
>> Still I think the idea is interesting, even if it's buggy for current
>> Intel processors.  Any opinions?
>>
>> Paolo
>>
>> Paolo Bonzini (4):
>>   x86: add UMIP feature and CR4 bit
>>   KVM: x86: emulate sldt and str
>>   KVM: x86: add support for emulating UMIP
>>   KVM: vmx: add support for emulating UMIP
>>
>>  arch/x86/include/asm/cpufeatures.h          |  1 +
>>  arch/x86/include/asm/kvm_host.h             |  3 ++-
>>  arch/x86/include/asm/vmx.h                  |  1 +
>>  arch/x86/include/uapi/asm/processor-flags.h |  2 ++
>>  arch/x86/include/uapi/asm/vmx.h             |  4 +++
>>  arch/x86/kvm/cpuid.c                        |  5 +++-
>>  arch/x86/kvm/cpuid.h                        |  8 ++++++
>>  arch/x86/kvm/emulate.c                      | 40
>> ++++++++++++++++++++++++-----
>>  arch/x86/kvm/svm.c                          |  6 +++++
>>  arch/x86/kvm/vmx.c                          | 40
>> ++++++++++++++++++++++++++++-
>>  arch/x86/kvm/x86.c                          |  3 +++
>>  11 files changed, 104 insertions(+), 9 deletions(-)
>>
> 
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 4/4] KVM: vmx: add support for emulating UMIP
  2016-07-13  9:21   ` Yang Zhang
@ 2016-07-13  9:35     ` Paolo Bonzini
  2016-07-13  9:54       ` Yang Zhang
  2016-07-13  9:35     ` Paolo Bonzini
  1 sibling, 1 reply; 32+ messages in thread
From: Paolo Bonzini @ 2016-07-13  9:35 UTC (permalink / raw)
  To: Yang Zhang, linux-kernel, kvm



On 13/07/2016 11:21, Yang Zhang wrote:
>>
>> +    if ((cr4 & X86_CR4_UMIP) && !boot_cpu_has(X86_FEATURE_UMIP)) {
>> +        vmcs_set_bits(SECONDARY_VM_EXEC_CONTROL,
>> +                  SECONDARY_EXEC_DESC);
>> +        hw_cr4 &= ~X86_CR4_UMIP;
>> +    } else
>> +        vmcs_clear_bits(SECONDARY_VM_EXEC_CONTROL,
>> +                SECONDARY_EXEC_DESC);
>> +
> 
> Since the faults based on privilege level have priority over VM exits.
> So we don't need to enable/disable SECONDARY_EXEC_DESC dynamically.
> Instead, we can set it unconditionally.

I'm setting it dynamically because it slows down LGDT, LLDT, LIDT and LTR.

Paolo

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 4/4] KVM: vmx: add support for emulating UMIP
  2016-07-13  9:21   ` Yang Zhang
  2016-07-13  9:35     ` Paolo Bonzini
@ 2016-07-13  9:35     ` Paolo Bonzini
  2016-07-13 10:02       ` Yang Zhang
  1 sibling, 1 reply; 32+ messages in thread
From: Paolo Bonzini @ 2016-07-13  9:35 UTC (permalink / raw)
  To: Yang Zhang, linux-kernel, kvm



On 13/07/2016 11:21, Yang Zhang wrote:
>>
>> +static int handle_desc(struct kvm_vcpu *vcpu)
>> +{
>> +    WARN_ON(!(vcpu->arch.cr4 & X86_CR4_UMIP));
> 
> I think WARN_ON is too heavy since a malicious guest may trigger it always.

I missed this---how so?  Setting the bit is under "if ((cr4 &
X86_CR4_UMIP) && !boot_cpu_has(X86_FEATURE_UMIP))".

Paolo

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 4/4] KVM: vmx: add support for emulating UMIP
  2016-07-13  9:35     ` Paolo Bonzini
@ 2016-07-13  9:54       ` Yang Zhang
  0 siblings, 0 replies; 32+ messages in thread
From: Yang Zhang @ 2016-07-13  9:54 UTC (permalink / raw)
  To: Paolo Bonzini, linux-kernel, kvm

On 2016/7/13 17:35, Paolo Bonzini wrote:
>
>
> On 13/07/2016 11:21, Yang Zhang wrote:
>>>
>>> +    if ((cr4 & X86_CR4_UMIP) && !boot_cpu_has(X86_FEATURE_UMIP)) {
>>> +        vmcs_set_bits(SECONDARY_VM_EXEC_CONTROL,
>>> +                  SECONDARY_EXEC_DESC);
>>> +        hw_cr4 &= ~X86_CR4_UMIP;
>>> +    } else
>>> +        vmcs_clear_bits(SECONDARY_VM_EXEC_CONTROL,
>>> +                SECONDARY_EXEC_DESC);
>>> +
>>
>> Since the faults based on privilege level have priority over VM exits.
>> So we don't need to enable/disable SECONDARY_EXEC_DESC dynamically.
>> Instead, we can set it unconditionally.
>
> I'm setting it dynamically because it slows down LGDT, LLDT, LIDT and LTR.

You are right. And SGDT, SIDT, SLDT, SMSW, STR also will be intercepted 
even in CPL 0 if we don't disable it.

-- 
Yang
Alibaba Cloud Computing

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 4/4] KVM: vmx: add support for emulating UMIP
  2016-07-13  9:35     ` Paolo Bonzini
@ 2016-07-13 10:02       ` Yang Zhang
  2016-07-13 10:21         ` Paolo Bonzini
  0 siblings, 1 reply; 32+ messages in thread
From: Yang Zhang @ 2016-07-13 10:02 UTC (permalink / raw)
  To: Paolo Bonzini, linux-kernel, kvm

On 2016/7/13 17:35, Paolo Bonzini wrote:
>
>
> On 13/07/2016 11:21, Yang Zhang wrote:
>>>
>>> +static int handle_desc(struct kvm_vcpu *vcpu)
>>> +{
>>> +    WARN_ON(!(vcpu->arch.cr4 & X86_CR4_UMIP));
>>
>> I think WARN_ON is too heavy since a malicious guest may trigger it always.
>
> I missed this---how so?  Setting the bit is under "if ((cr4 &
> X86_CR4_UMIP) && !boot_cpu_has(X86_FEATURE_UMIP))".

Sorry, I consider it under my previous suggestion(setting it 
unconditionally). :(

-- 
Yang
Alibaba Cloud Computing

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 4/4] KVM: vmx: add support for emulating UMIP
  2016-07-13 10:02       ` Yang Zhang
@ 2016-07-13 10:21         ` Paolo Bonzini
  0 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2016-07-13 10:21 UTC (permalink / raw)
  To: Yang Zhang, linux-kernel, kvm



On 13/07/2016 12:02, Yang Zhang wrote:
> On 2016/7/13 17:35, Paolo Bonzini wrote:
>>
>>
>> On 13/07/2016 11:21, Yang Zhang wrote:
>>>>
>>>> +static int handle_desc(struct kvm_vcpu *vcpu)
>>>> +{
>>>> +    WARN_ON(!(vcpu->arch.cr4 & X86_CR4_UMIP));
>>>
>>> I think WARN_ON is too heavy since a malicious guest may trigger it
>>> always.
>>
>> I missed this---how so?  Setting the bit is under "if ((cr4 &
>> X86_CR4_UMIP) && !boot_cpu_has(X86_FEATURE_UMIP))".
> 
> Sorry, I consider it under my previous suggestion(setting it
> unconditionally). :(

No problem, thanks for your interest!

Paolo

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 3/4] KVM: x86: add support for emulating UMIP
  2016-07-12 19:20 ` [RFC PATCH 3/4] KVM: x86: add support for emulating UMIP Paolo Bonzini
@ 2016-07-13 20:03   ` Radim Krčmář
  2016-07-14  7:00     ` Paolo Bonzini
  0 siblings, 1 reply; 32+ messages in thread
From: Radim Krčmář @ 2016-07-13 20:03 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm

2016-07-12 21:20+0200, Paolo Bonzini:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> @@ -3738,6 +3747,10 @@ static int em_lidt(struct x86_emulate_ctxt *ctxt)
>  
>  static int em_smsw(struct x86_emulate_ctxt *ctxt)
>  {
> +	if ((ctxt->ops->get_cr(ctxt, 4) & X86_CR4_UMIP) &&
> +	    ctxt->ops->cpl(ctxt) > 0)

UMIP should #GP(0) in virtual-8086 mode too (for SMSW, SIDT, and SGDT),
but cpl() returns 0 in vm86.

> +		return emulate_gp(ctxt, 0);

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 4/4] KVM: vmx: add support for emulating UMIP
  2016-07-12 19:20 ` [RFC PATCH 4/4] KVM: vmx: " Paolo Bonzini
  2016-07-13  9:21   ` Yang Zhang
@ 2016-07-13 20:30   ` Radim Krčmář
  2016-07-14  8:09     ` Paolo Bonzini
  1 sibling, 1 reply; 32+ messages in thread
From: Radim Krčmář @ 2016-07-13 20:30 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm

2016-07-12 21:20+0200, Paolo Bonzini:
> UMIP (User-Mode Instruction Prevention) is a feature of future
> Intel processors (Cannonlake?) that blocks SLDT, SGDT, STR, SIDT
> and SMSW from user-mode processes.
> 
> On Intel systems it's *almost* possible to emulate it; it slows
> down the instructions when they're executed in ring 0, but they
> are really never executed in practice.  The catch is that SMSW
> doesn't cause a vmexit, and hence SMSW will not fault.
> 
> When UMIP is enabled but not supported by the host, descriptor table
> exits are enabled, and the emulator takes care of injecting a #GP when
> any of SLDT, SGDT, STR, SIDT are encountered.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> @@ -3967,6 +3968,14 @@ static int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
>  		(to_vmx(vcpu)->rmode.vm86_active ?
>  		 KVM_RMODE_VM_CR4_ALWAYS_ON : KVM_PMODE_VM_CR4_ALWAYS_ON);
>  
> +	if ((cr4 & X86_CR4_UMIP) && !boot_cpu_has(X86_FEATURE_UMIP)) {
> +		vmcs_set_bits(SECONDARY_VM_EXEC_CONTROL,
> +			      SECONDARY_EXEC_DESC);

If UMIP support is not exposed in CPUID, we ought to #GP(0), because it
is a write to reserved bits.  It could also mean that the vm control is
not supported.

> +		hw_cr4 &= ~X86_CR4_UMIP;
> +	} else
> +		vmcs_clear_bits(SECONDARY_VM_EXEC_CONTROL,
> +				SECONDARY_EXEC_DESC);

I think we don't have to do anything when the CPU supports UMIP,

 if (!boot_cpu_has(X86_FEATURE_UMIP) {
   if ((cr4 & X86_CR4_UMIP)) { ... } else ...
 }

And we could then return true in vmx_umip_emulated() when
boot_cpu_has(X86_FEATURE_UMIP).
(Just for self-documentation, because occurrence of X86_FEATURE_UMIP is
 most likely a subset of SECONDARY_EXEC_DESC.)

> @@ -8597,7 +8627,8 @@ static bool vmx_xsaves_supported(void)
>  
>  static bool vmx_umip_emulated(void)
>  {
> -	return false;
> +	return vmcs_config.cpu_based_2nd_exec_ctrl &
> +		SECONDARY_EXEC_DESC;
>  }

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 3/4] KVM: x86: add support for emulating UMIP
  2016-07-13 20:03   ` Radim Krčmář
@ 2016-07-14  7:00     ` Paolo Bonzini
  2016-07-14 12:24       ` Radim Krčmář
  0 siblings, 1 reply; 32+ messages in thread
From: Paolo Bonzini @ 2016-07-14  7:00 UTC (permalink / raw)
  To: Radim Krčmář; +Cc: linux-kernel, kvm



On 13/07/2016 22:03, Radim Krčmář wrote:
> 2016-07-12 21:20+0200, Paolo Bonzini:
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>> @@ -3738,6 +3747,10 @@ static int em_lidt(struct x86_emulate_ctxt *ctxt)
>>  
>>  static int em_smsw(struct x86_emulate_ctxt *ctxt)
>>  {
>> +	if ((ctxt->ops->get_cr(ctxt, 4) & X86_CR4_UMIP) &&
>> +	    ctxt->ops->cpl(ctxt) > 0)
> 
> UMIP should #GP(0) in virtual-8086 mode too (for SMSW, SIDT, and SGDT),
> but cpl() returns 0 in vm86.

No, it returns 3.

vmx->rmode.vm86_active means "there is no unrestricted guest support,
and we're using VM86 mode to emulate real mode".  The special case in
vmx_get_cpl is needed exactly because real mode CPL is 0 but VM86 has
CPL 3.  In fact fix_rmode_seg sets DPL=3 for all segments, and CPL=SS.DPL.

Paolo

>> +		return emulate_gp(ctxt, 0);
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 4/4] KVM: vmx: add support for emulating UMIP
  2016-07-13 20:30   ` Radim Krčmář
@ 2016-07-14  8:09     ` Paolo Bonzini
  2016-07-14 12:36       ` Radim Krčmář
  2016-07-31  2:32       ` Wanpeng Li
  0 siblings, 2 replies; 32+ messages in thread
From: Paolo Bonzini @ 2016-07-14  8:09 UTC (permalink / raw)
  To: Radim Krčmář; +Cc: linux-kernel, kvm



On 13/07/2016 22:30, Radim Krčmář wrote:
> 2016-07-12 21:20+0200, Paolo Bonzini:
>> UMIP (User-Mode Instruction Prevention) is a feature of future
>> Intel processors (Cannonlake?) that blocks SLDT, SGDT, STR, SIDT
>> and SMSW from user-mode processes.
>>
>> On Intel systems it's *almost* possible to emulate it; it slows
>> down the instructions when they're executed in ring 0, but they
>> are really never executed in practice.  The catch is that SMSW
>> doesn't cause a vmexit, and hence SMSW will not fault.
>>
>> When UMIP is enabled but not supported by the host, descriptor table
>> exits are enabled, and the emulator takes care of injecting a #GP when
>> any of SLDT, SGDT, STR, SIDT are encountered.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> @@ -3967,6 +3968,14 @@ static int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
>>  		(to_vmx(vcpu)->rmode.vm86_active ?
>>  		 KVM_RMODE_VM_CR4_ALWAYS_ON : KVM_PMODE_VM_CR4_ALWAYS_ON);
>>  
>> +	if ((cr4 & X86_CR4_UMIP) && !boot_cpu_has(X86_FEATURE_UMIP)) {
>> +		vmcs_set_bits(SECONDARY_VM_EXEC_CONTROL,
>> +			      SECONDARY_EXEC_DESC);
> 
> If UMIP support is not exposed in CPUID, we ought to #GP(0), because it
> is a write to reserved bits.  It could also mean that the vm control is
> not supported.

Yes, this is done in kvm_set_cr4:

        if (cr4 & CR4_RESERVED_BITS)
                return 1;
	...
        if (!guest_cpuid_has_umip(vcpu) && (cr4 & X86_CR4_UMIP))
                return 1;

>> +		hw_cr4 &= ~X86_CR4_UMIP;
>> +	} else
>> +		vmcs_clear_bits(SECONDARY_VM_EXEC_CONTROL,
>> +				SECONDARY_EXEC_DESC);
> 
> I think we don't have to do anything when the CPU supports UMIP,
> 
>  if (!boot_cpu_has(X86_FEATURE_UMIP) {
>    if ((cr4 & X86_CR4_UMIP)) { ... } else ...
>  }

Right.

> And we could then return true in vmx_umip_emulated() when
> boot_cpu_has(X86_FEATURE_UMIP).
> (Just for self-documentation, because occurrence of X86_FEATURE_UMIP is
>  most likely a subset of SECONDARY_EXEC_DESC.)

This is not necessary because this is how KVM computes
CPUID[EAX=7,EBX=0].ECX:

        unsigned f_umip = kvm_x86_ops->umip_emulated() ? F(UMIP) : 0;
	...
        const u32 kvm_cpuid_7_0_ecx_x86_features = F(PKU) | F(UMIP);
	...
	// Mask userspace-provided value against supported features
	entry->ecx &= kvm_cpuid_7_0_ecx_x86_features;
	// Mask userspace-provided value against host features
	cpuid_mask(&entry->ecx, CPUID_7_ECX);
	// Finally add emulated features
	entry->ecx |= f_umip;

Paolo

>> @@ -8597,7 +8627,8 @@ static bool vmx_xsaves_supported(void)
>>  
>>  static bool vmx_umip_emulated(void)
>>  {
>> -	return false;
>> +	return vmcs_config.cpu_based_2nd_exec_ctrl &
>> +		SECONDARY_EXEC_DESC;
>>  }

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 3/4] KVM: x86: add support for emulating UMIP
  2016-07-14  7:00     ` Paolo Bonzini
@ 2016-07-14 12:24       ` Radim Krčmář
  0 siblings, 0 replies; 32+ messages in thread
From: Radim Krčmář @ 2016-07-14 12:24 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm

2016-07-14 09:00+0200, Paolo Bonzini:
> On 13/07/2016 22:03, Radim Krčmář wrote:
>> 2016-07-12 21:20+0200, Paolo Bonzini:
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> ---
>>> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>>> @@ -3738,6 +3747,10 @@ static int em_lidt(struct x86_emulate_ctxt *ctxt)
>>>  
>>>  static int em_smsw(struct x86_emulate_ctxt *ctxt)
>>>  {
>>> +	if ((ctxt->ops->get_cr(ctxt, 4) & X86_CR4_UMIP) &&
>>> +	    ctxt->ops->cpl(ctxt) > 0)
>> 
>> UMIP should #GP(0) in virtual-8086 mode too (for SMSW, SIDT, and SGDT),
>> but cpl() returns 0 in vm86.
> 
> No, it returns 3.
> 
> vmx->rmode.vm86_active means "there is no unrestricted guest support,
> and we're using VM86 mode to emulate real mode".  The special case in
> vmx_get_cpl is needed exactly because real mode CPL is 0 but VM86 has
> CPL 3.  In fact fix_rmode_seg sets DPL=3 for all segments, and CPL=SS.DPL.

Right, thanks.

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 4/4] KVM: vmx: add support for emulating UMIP
  2016-07-14  8:09     ` Paolo Bonzini
@ 2016-07-14 12:36       ` Radim Krčmář
  2016-07-14 12:54         ` Paolo Bonzini
  2016-07-31  2:32       ` Wanpeng Li
  1 sibling, 1 reply; 32+ messages in thread
From: Radim Krčmář @ 2016-07-14 12:36 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: linux-kernel, kvm

2016-07-14 10:09+0200, Paolo Bonzini:
> On 13/07/2016 22:30, Radim Krčmář wrote:
>> 2016-07-12 21:20+0200, Paolo Bonzini:
>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>> @@ -3967,6 +3968,14 @@ static int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
>>>  		(to_vmx(vcpu)->rmode.vm86_active ?
>>>  		 KVM_RMODE_VM_CR4_ALWAYS_ON : KVM_PMODE_VM_CR4_ALWAYS_ON);
>>>  
>>> +	if ((cr4 & X86_CR4_UMIP) && !boot_cpu_has(X86_FEATURE_UMIP)) {
>>> +		vmcs_set_bits(SECONDARY_VM_EXEC_CONTROL,
>>> +			      SECONDARY_EXEC_DESC);
>> 
>> If UMIP support is not exposed in CPUID, we ought to #GP(0), because it
>> is a write to reserved bits.  It could also mean that the vm control is
>> not supported.
> 
> Yes, this is done in kvm_set_cr4:
> 
>         if (cr4 & CR4_RESERVED_BITS)
>                 return 1;
> 	...
>         if (!guest_cpuid_has_umip(vcpu) && (cr4 & X86_CR4_UMIP))
>                 return 1;

Missed that,

>> And we could then return true in vmx_umip_emulated() when
>> boot_cpu_has(X86_FEATURE_UMIP).
>> (Just for self-documentation, because occurrence of X86_FEATURE_UMIP is
>>  most likely a subset of SECONDARY_EXEC_DESC.)
> 
> This is not necessary because this is how KVM computes
> CPUID[EAX=7,EBX=0].ECX:
> 
>         unsigned f_umip = kvm_x86_ops->umip_emulated() ? F(UMIP) : 0;
> 	...
>         const u32 kvm_cpuid_7_0_ecx_x86_features = F(PKU) | F(UMIP);

and that too, I'm really sorry for the review.

> 	...
> 	// Mask userspace-provided value against supported features
> 	entry->ecx &= kvm_cpuid_7_0_ecx_x86_features;
> 	// Mask userspace-provided value against host features
> 	cpuid_mask(&entry->ecx, CPUID_7_ECX);
> 	// Finally add emulated features
> 	entry->ecx |= f_umip;

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 4/4] KVM: vmx: add support for emulating UMIP
  2016-07-14 12:36       ` Radim Krčmář
@ 2016-07-14 12:54         ` Paolo Bonzini
  0 siblings, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2016-07-14 12:54 UTC (permalink / raw)
  To: Radim Krčmář; +Cc: linux-kernel, kvm



On 14/07/2016 14:36, Radim Krčmář wrote:
>>> And we could then return true in vmx_umip_emulated() when
>>> boot_cpu_has(X86_FEATURE_UMIP).
>>> (Just for self-documentation, because occurrence of X86_FEATURE_UMIP is
>>>  most likely a subset of SECONDARY_EXEC_DESC.)
>>
>> This is not necessary because this is how KVM computes
>> CPUID[EAX=7,EBX=0].ECX:
>>
>>         unsigned f_umip = kvm_x86_ops->umip_emulated() ? F(UMIP) : 0;
>> 	...
>>         const u32 kvm_cpuid_7_0_ecx_x86_features = F(PKU) | F(UMIP);
> 
> and that too, I'm really sorry for the review.

At least in the latter case, that says something about the code quality too.

Paolo

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 4/4] KVM: vmx: add support for emulating UMIP
  2016-07-14  8:09     ` Paolo Bonzini
  2016-07-14 12:36       ` Radim Krčmář
@ 2016-07-31  2:32       ` Wanpeng Li
  2016-08-01 15:01         ` Paolo Bonzini
  1 sibling, 1 reply; 32+ messages in thread
From: Wanpeng Li @ 2016-07-31  2:32 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Radim Krčmář, linux-kernel, kvm

2016-07-14 16:09 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
[...]
>
> This is not necessary because this is how KVM computes
> CPUID[EAX=7,EBX=0].ECX:
>
>         unsigned f_umip = kvm_x86_ops->umip_emulated() ? F(UMIP) : 0;
>         ...
>         const u32 kvm_cpuid_7_0_ecx_x86_features = F(PKU) | F(UMIP);
>         ...
>         // Mask userspace-provided value against supported features
>         entry->ecx &= kvm_cpuid_7_0_ecx_x86_features;
>         // Mask userspace-provided value against host features
>         cpuid_mask(&entry->ecx, CPUID_7_ECX);
>         // Finally add emulated features
>         entry->ecx |= f_umip;

I think you mean:

- entry->ecx  ->  userspace-provided value
- kvm_cpuid_7_0_ecx_x86_features  ->  supported features
- CPUID_7_ECX  ->  host features

However, entry->ecx is returned by cpuid instruction
(do_cpuid_1_ent()), so why it is a userspace-provided value?

Regards,
Wanpeng Li

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 4/4] KVM: vmx: add support for emulating UMIP
  2016-07-31  2:32       ` Wanpeng Li
@ 2016-08-01 15:01         ` Paolo Bonzini
  2016-08-02  1:05           ` Wanpeng Li
  0 siblings, 1 reply; 32+ messages in thread
From: Paolo Bonzini @ 2016-08-01 15:01 UTC (permalink / raw)
  To: Wanpeng Li; +Cc: Radim Krčmář, linux-kernel, kvm



On 31/07/2016 04:32, Wanpeng Li wrote:
> 2016-07-14 16:09 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
> [...]
>>
>> This is not necessary because this is how KVM computes
>> CPUID[EAX=7,EBX=0].ECX:
>>
>>         unsigned f_umip = kvm_x86_ops->umip_emulated() ? F(UMIP) : 0;
>>         ...
>>         const u32 kvm_cpuid_7_0_ecx_x86_features = F(PKU) | F(UMIP);
>>         ...
>>         // Mask userspace-provided value against supported features
>>         entry->ecx &= kvm_cpuid_7_0_ecx_x86_features;
>>         // Mask userspace-provided value against host features
>>         cpuid_mask(&entry->ecx, CPUID_7_ECX);
>>         // Finally add emulated features
>>         entry->ecx |= f_umip;
> 
> I think you mean:
> 
> - entry->ecx  ->  userspace-provided value
> - kvm_cpuid_7_0_ecx_x86_features  ->  supported features
> - CPUID_7_ECX  ->  host features
> 
> However, entry->ecx is returned by cpuid instruction
> (do_cpuid_1_ent()), so why it is a userspace-provided value?

You're right, it's this:

         // Mask host processor value against supported features
         entry->ecx &= kvm_cpuid_7_0_ecx_x86_features;
         // Mask host processor value further, e.g. to drop
	 // features that the host kernel has blacklisted.
         cpuid_mask(&entry->ecx, CPUID_7_ECX);
         // Finally add emulated features
         entry->ecx |= f_umip;

The idea is the same. :)

On the other hand, it is true that in many cases of the "switch
(function)" the call to do_cpuid_1_ent is unnecessary, and instead of
cpuid_mask you could just access boot_cpu_data.x86_capability[wordnum].

Paolo

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 4/4] KVM: vmx: add support for emulating UMIP
  2016-08-01 15:01         ` Paolo Bonzini
@ 2016-08-02  1:05           ` Wanpeng Li
  0 siblings, 0 replies; 32+ messages in thread
From: Wanpeng Li @ 2016-08-02  1:05 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Radim Krčmář, linux-kernel, kvm

2016-08-01 23:01 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
>
>
> On 31/07/2016 04:32, Wanpeng Li wrote:
>> 2016-07-14 16:09 GMT+08:00 Paolo Bonzini <pbonzini@redhat.com>:
>> [...]
>>>
>>> This is not necessary because this is how KVM computes
>>> CPUID[EAX=7,EBX=0].ECX:
>>>
>>>         unsigned f_umip = kvm_x86_ops->umip_emulated() ? F(UMIP) : 0;
>>>         ...
>>>         const u32 kvm_cpuid_7_0_ecx_x86_features = F(PKU) | F(UMIP);
>>>         ...
>>>         // Mask userspace-provided value against supported features
>>>         entry->ecx &= kvm_cpuid_7_0_ecx_x86_features;
>>>         // Mask userspace-provided value against host features
>>>         cpuid_mask(&entry->ecx, CPUID_7_ECX);
>>>         // Finally add emulated features
>>>         entry->ecx |= f_umip;
>>
>> I think you mean:
>>
>> - entry->ecx  ->  userspace-provided value
>> - kvm_cpuid_7_0_ecx_x86_features  ->  supported features
>> - CPUID_7_ECX  ->  host features
>>
>> However, entry->ecx is returned by cpuid instruction
>> (do_cpuid_1_ent()), so why it is a userspace-provided value?
>
> You're right, it's this:
>
>          // Mask host processor value against supported features
>          entry->ecx &= kvm_cpuid_7_0_ecx_x86_features;
>          // Mask host processor value further, e.g. to drop
>          // features that the host kernel has blacklisted.
>          cpuid_mask(&entry->ecx, CPUID_7_ECX);
>          // Finally add emulated features
>          entry->ecx |= f_umip;
>

Cool, more clear this time. :)

> The idea is the same. :)
>
> On the other hand, it is true that in many cases of the "switch
> (function)" the call to do_cpuid_1_ent is unnecessary, and instead of
> cpuid_mask you could just access boot_cpu_data.x86_capability[wordnum].

Agreed.

Regards,
Wanpeng Li

^ permalink raw reply	[flat|nested] 32+ messages in thread

* RE: [RFC PATCH 0/4] KVM: Emulate UMIP (or almost do so)
  2016-07-12 19:20 [RFC PATCH 0/4] KVM: Emulate UMIP (or almost do so) Paolo Bonzini
                   ` (4 preceding siblings ...)
  2016-07-13  8:29 ` [RFC PATCH 0/4] KVM: Emulate UMIP (or almost do so) Yang Zhang
@ 2016-12-13  4:03 ` Li, Liang Z
  2016-12-13 11:03   ` Paolo Bonzini
  5 siblings, 1 reply; 32+ messages in thread
From: Li, Liang Z @ 2016-12-13  4:03 UTC (permalink / raw)
  To: Paolo Bonzini, linux-kernel, kvm

> UMIP (User-Mode Instruction Prevention) is a feature of future Intel
> processors (Cannonlake?) that blocks SLDT, SGDT, STR, SIDT and SMSW from
> user-mode processes.
> 
> The idea here is to use virtualization intercepts to emulate UMIP; it slows
> down the instructions when they're executed in ring 0, but they are really
> never executed in practice.  On AMD systems it's possible to emulate it
> entirely; instead on Intel systems it's *almost* possible to emulate it,
> because SMSW doesn't cause a vmexit, and hence SMSW will not fault.
> 
> This patch series provides the infrastructure and implements it on Intel.  I
> tested it through kvm-unit-tests.
> 
> Still I think the idea is interesting, even if it's buggy for current Intel
> processors.  Any opinions?

Hi Paolo,

We intended to enable UMIP for KVM and found you had already worked on it. 
Do you have any plan for the following patch set? It's there anything else you expect
us help to do?

Thanks!
Liang

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 0/4] KVM: Emulate UMIP (or almost do so)
  2016-12-13  4:03 ` Li, Liang Z
@ 2016-12-13 11:03   ` Paolo Bonzini
  2017-03-01  9:04     ` Yu Zhang
  0 siblings, 1 reply; 32+ messages in thread
From: Paolo Bonzini @ 2016-12-13 11:03 UTC (permalink / raw)
  To: Li, Liang Z, linux-kernel, kvm



On 13/12/2016 05:03, Li, Liang Z wrote:
> Hi Paolo,
> 
> We intended to enable UMIP for KVM and found you had already worked on it. 
> Do you have any plan for the following patch set? It's there anything else you expect
> us help to do?

Yes, I plan to resend these patches for 4.11.

Paolo

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 0/4] KVM: Emulate UMIP (or almost do so)
  2016-12-13 11:03   ` Paolo Bonzini
@ 2017-03-01  9:04     ` Yu Zhang
  2017-03-06 21:07       ` Paolo Bonzini
  2017-03-10  8:02       ` Yu Zhang
  0 siblings, 2 replies; 32+ messages in thread
From: Yu Zhang @ 2017-03-01  9:04 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qian.ouyang, linux-kernel, kvm



On 12/13/2016 7:03 PM, Paolo Bonzini wrote:
>
> On 13/12/2016 05:03, Li, Liang Z wrote:
>> Hi Paolo,
>>
>> We intended to enable UMIP for KVM and found you had already worked on it.
>> Do you have any plan for the following patch set? It's there anything else you expect
>> us help to do?
> Yes, I plan to resend these patches for 4.11.

Hi Paolo,

   Previously we saw your RFC patches of UMIP sent out, and we would 
like to try some unit test in Intel. I found a patch written by you in 
https://patchwork.kernel.org/patch/9225929/, guess this is for the kvm 
unit test(though I failed to git apply it directly).
   And I wonder, when will it be integrated to kvm unit test repo?
   Besides, is this all the test for UMIP unit test? I.e. do we need to 
construct a scenario in the test to trigger vm exit and let hypervisor 
inject a GP fault? - I did not see this scenario in this patch. Or any 
other suggestions? :-)

Thanks
Yu

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 0/4] KVM: Emulate UMIP (or almost do so)
  2017-03-01  9:04     ` Yu Zhang
@ 2017-03-06 21:07       ` Paolo Bonzini
  2017-03-10  8:02       ` Yu Zhang
  1 sibling, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-03-06 21:07 UTC (permalink / raw)
  To: Yu Zhang; +Cc: qian ouyang, linux-kernel, kvm



----- Original Message -----
> From: "Yu Zhang" <yu.c.zhang@linux.intel.com>
> To: "Paolo Bonzini" <pbonzini@redhat.com>
> Cc: "qian ouyang" <qian.ouyang@intel.com>, linux-kernel@vger.kernel.org, kvm@vger.kernel.org
> Sent: Wednesday, March 1, 2017 10:04:17 AM
> Subject: Re: [RFC PATCH 0/4] KVM: Emulate UMIP (or almost do so)
> 
> 
> 
> On 12/13/2016 7:03 PM, Paolo Bonzini wrote:
> >
> > On 13/12/2016 05:03, Li, Liang Z wrote:
> >> Hi Paolo,
> >>
> >> We intended to enable UMIP for KVM and found you had already worked on it.
> >> Do you have any plan for the following patch set? It's there anything else
> >> you expect
> >> us help to do?
> > Yes, I plan to resend these patches for 4.11.
> 
> Hi Paolo,
> 
>    Previously we saw your RFC patches of UMIP sent out, and we would
> like to try some unit test in Intel. I found a patch written by you in
> https://patchwork.kernel.org/patch/9225929/, guess this is for the kvm
> unit test(though I failed to git apply it directly).
>    And I wonder, when will it be integrated to kvm unit test repo?

Probably together with the patches for UMIP support and emulation.

>    Besides, is this all the test for UMIP unit test? I.e. do we need to
> construct a scenario in the test to trigger vm exit and let hypervisor
> inject a GP fault? - I did not see this scenario in this patch. Or any
> other suggestions? :-)

The unit test only tests that UMIP works as advertised; if the host processor
does not support UMIP, it will test the hypervisor emulation path.  Whenever
UMIP support is added to KVM, we will probably add a module parameter so that
we can force UMIP emulation path even on processor that have native support.

Paolo

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 0/4] KVM: Emulate UMIP (or almost do so)
  2017-03-01  9:04     ` Yu Zhang
  2017-03-06 21:07       ` Paolo Bonzini
@ 2017-03-10  8:02       ` Yu Zhang
  2017-03-10  8:36         ` Paolo Bonzini
  1 sibling, 1 reply; 32+ messages in thread
From: Yu Zhang @ 2017-03-10  8:02 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qian.ouyang, linux-kernel, kvm

On 3/1/2017 5:04 PM, Yu Zhang wrote:
>
>
> On 12/13/2016 7:03 PM, Paolo Bonzini wrote:
>>
>> On 13/12/2016 05:03, Li, Liang Z wrote:
>>> Hi Paolo,
>>>
>>> We intended to enable UMIP for KVM and found you had already worked 
>>> on it.
>>> Do you have any plan for the following patch set? It's there 
>>> anything else you expect
>>> us help to do?
>> Yes, I plan to resend these patches for 4.11.
>
> Hi Paolo,
>
>   Previously we saw your RFC patches of UMIP sent out, and we would 
> like to try some unit test in Intel. I found a patch written by you in 
> https://patchwork.kernel.org/patch/9225929/, guess this is for the kvm 
> unit test(though I failed to git apply it directly).
>   And I wonder, when will it be integrated to kvm unit test repo?
>   Besides, is this all the test for UMIP unit test? I.e. do we need to 
> construct a scenario in the test to trigger vm exit and let hypervisor 
> inject a GP fault? - I did not see this scenario in this patch. Or any 
> other suggestions? :-)
>

Hi Paolo, any suggestions?
Sorry for the disturb. :)

Yu
> Thanks
> Yu
>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 0/4] KVM: Emulate UMIP (or almost do so)
  2017-03-10  8:02       ` Yu Zhang
@ 2017-03-10  8:36         ` Paolo Bonzini
  2017-03-10  9:31           ` Yu Zhang
  0 siblings, 1 reply; 32+ messages in thread
From: Paolo Bonzini @ 2017-03-10  8:36 UTC (permalink / raw)
  To: Yu Zhang; +Cc: qian.ouyang, linux-kernel, kvm



On 10/03/2017 09:02, Yu Zhang wrote:
>>   Besides, is this all the test for UMIP unit test? I.e. do we need to
>> construct a scenario in the test to trigger vm exit and let hypervisor
>> inject a GP fault? - I did not see this scenario in this patch. Or any
>> other suggestions? :-)
> 
> Hi Paolo, any suggestions?
> Sorry for the disturb. :)

Hi, you get the scenario where a vmexit is triggered by the hypervisor
if you run the unit test on a machine that lacks UMIP support.

We can also add a module parameter to force emulation, so that it will
be possible to test UMIP emulation on newer processors too.

Paolo

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 0/4] KVM: Emulate UMIP (or almost do so)
  2017-03-10  8:36         ` Paolo Bonzini
@ 2017-03-10  9:31           ` Yu Zhang
  2017-03-10 11:28             ` Yu Zhang
  2017-03-10 15:00             ` Paolo Bonzini
  0 siblings, 2 replies; 32+ messages in thread
From: Yu Zhang @ 2017-03-10  9:31 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qian.ouyang, linux-kernel, kvm



On 3/10/2017 4:36 PM, Paolo Bonzini wrote:
>
> On 10/03/2017 09:02, Yu Zhang wrote:
>>>    Besides, is this all the test for UMIP unit test? I.e. do we need to
>>> construct a scenario in the test to trigger vm exit and let hypervisor
>>> inject a GP fault? - I did not see this scenario in this patch. Or any
>>> other suggestions? :-)
>> Hi Paolo, any suggestions?
>> Sorry for the disturb. :)
> Hi, you get the scenario where a vmexit is triggered by the hypervisor
> if you run the unit test on a machine that lacks UMIP support.
>
> We can also add a module parameter to force emulation, so that it will
> be possible to test UMIP emulation on newer processors too.

Thanks for your reply, Paolo. :-)

Well, my previous understanding is that there might be a situation on a 
machine with UMIP
feature:
1> when an APP in VM runs instructions such as sgdt addrA,
2> and the addrA may cause anVM exit(e.g. ept violation),
3> next, the emulator in hypervisor need to inject a GP fault to the VM.
Is this situation possible?
This is the case I'd like to test, yet do not know to construct the 
scenario.

But as to the scenario you described, I do not quit understand.
I mean, on a host which do not support UMIP, although hypervisor may 
intercept cpuid and
provide an emulated cr4 to guest, how does it guarantee those 
instructions in VM will cause
a VM exit?

Yu
> Paolo
>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 0/4] KVM: Emulate UMIP (or almost do so)
  2017-03-10  9:31           ` Yu Zhang
@ 2017-03-10 11:28             ` Yu Zhang
  2017-03-10 15:00             ` Paolo Bonzini
  1 sibling, 0 replies; 32+ messages in thread
From: Yu Zhang @ 2017-03-10 11:28 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qian.ouyang, linux-kernel, kvm



On 3/10/2017 5:31 PM, Yu Zhang wrote:
>
>
> On 3/10/2017 4:36 PM, Paolo Bonzini wrote:
>>
>> On 10/03/2017 09:02, Yu Zhang wrote:
>>>>    Besides, is this all the test for UMIP unit test? I.e. do we 
>>>> need to
>>>> construct a scenario in the test to trigger vm exit and let hypervisor
>>>> inject a GP fault? - I did not see this scenario in this patch. Or any
>>>> other suggestions? :-)
>>> Hi Paolo, any suggestions?
>>> Sorry for the disturb. :)
>> Hi, you get the scenario where a vmexit is triggered by the hypervisor
>> if you run the unit test on a machine that lacks UMIP support.
>>
>> We can also add a module parameter to force emulation, so that it will
>> be possible to test UMIP emulation on newer processors too.
>
> Thanks for your reply, Paolo. :-)
>
> Well, my previous understanding is that there might be a situation on 
> a machine with UMIP
> feature:
> 1> when an APP in VM runs instructions such as sgdt addrA,
> 2> and the addrA may cause anVM exit(e.g. ept violation),
> 3> next, the emulator in hypervisor need to inject a GP fault to the VM.
> Is this situation possible?
> This is the case I'd like to test, yet do not know to construct the 
> scenario.
>

Sorry, Paolo. I may have misunderstanding on this.

In intel SDM chapt 25, it says "Certain exceptions have priority over VM 
exits. These include
invalid-opcode exceptions, faults based on privilege level...".
So in above case, it is GP fault in VM that should happen, instead of VM 
exit, right?

> But as to the scenario you described, I do not quit understand.
> I mean, on a host which do not support UMIP, although hypervisor may 
> intercept cpuid and
> provide an emulated cr4 to guest, how does it guarantee those 
> instructions in VM will cause
> a VM exit?

For this scenario, vm exit should be caused if "descriptor-table 
exiting” VM-execution control is 1,
then hypervisor should have opportunity to do the force emulation.

Is my new understanding correct?

Another question is, what if host do have the UMIP feature, and the 
"descriptor-table exiting” VM-execution
control is 1? Will a GP fault in VM happen, or a VM exit?

Thanks
Yu

> Yu
>> Paolo
>>
>
>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [RFC PATCH 0/4] KVM: Emulate UMIP (or almost do so)
  2017-03-10  9:31           ` Yu Zhang
  2017-03-10 11:28             ` Yu Zhang
@ 2017-03-10 15:00             ` Paolo Bonzini
  1 sibling, 0 replies; 32+ messages in thread
From: Paolo Bonzini @ 2017-03-10 15:00 UTC (permalink / raw)
  To: Yu Zhang; +Cc: qian.ouyang, linux-kernel, kvm

On 10/03/2017 10:31, Yu Zhang wrote:
>> We can also add a module parameter to force emulation, so that it will
>> be possible to test UMIP emulation on newer processors too.
> 
> Thanks for your reply, Paolo. :-)
> 
> Well, my previous understanding is that there might be a situation on a
> machine with UMIP
> feature:
> 1> when an APP in VM runs instructions such as sgdt addrA,
> 2> and the addrA may cause anVM exit(e.g. ept violation),
> 3> next, the emulator in hypervisor need to inject a GP fault to the VM.
> Is this situation possible?

No, the guest will execute the instruction again after the vmexit.

> But as to the scenario you described, I do not quit understand.
> I mean, on a host which do not support UMIP, although hypervisor may
> intercept cpuid and
> provide an emulated cr4 to guest, how does it guarantee those
> instructions in VM will cause a VM exit?

All instructions except SMSW can be trapped using descriptor table vmexits.

Paolo

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2017-03-10 15:00 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-12 19:20 [RFC PATCH 0/4] KVM: Emulate UMIP (or almost do so) Paolo Bonzini
2016-07-12 19:20 ` [RFC PATCH 1/4] x86: add UMIP feature and CR4 bit Paolo Bonzini
2016-07-12 19:20 ` [RFC PATCH 2/4] KVM: x86: emulate sldt and str Paolo Bonzini
2016-07-12 19:20 ` [RFC PATCH 3/4] KVM: x86: add support for emulating UMIP Paolo Bonzini
2016-07-13 20:03   ` Radim Krčmář
2016-07-14  7:00     ` Paolo Bonzini
2016-07-14 12:24       ` Radim Krčmář
2016-07-12 19:20 ` [RFC PATCH 4/4] KVM: vmx: " Paolo Bonzini
2016-07-13  9:21   ` Yang Zhang
2016-07-13  9:35     ` Paolo Bonzini
2016-07-13  9:54       ` Yang Zhang
2016-07-13  9:35     ` Paolo Bonzini
2016-07-13 10:02       ` Yang Zhang
2016-07-13 10:21         ` Paolo Bonzini
2016-07-13 20:30   ` Radim Krčmář
2016-07-14  8:09     ` Paolo Bonzini
2016-07-14 12:36       ` Radim Krčmář
2016-07-14 12:54         ` Paolo Bonzini
2016-07-31  2:32       ` Wanpeng Li
2016-08-01 15:01         ` Paolo Bonzini
2016-08-02  1:05           ` Wanpeng Li
2016-07-13  8:29 ` [RFC PATCH 0/4] KVM: Emulate UMIP (or almost do so) Yang Zhang
2016-07-13  9:34   ` Paolo Bonzini
2016-12-13  4:03 ` Li, Liang Z
2016-12-13 11:03   ` Paolo Bonzini
2017-03-01  9:04     ` Yu Zhang
2017-03-06 21:07       ` Paolo Bonzini
2017-03-10  8:02       ` Yu Zhang
2017-03-10  8:36         ` Paolo Bonzini
2017-03-10  9:31           ` Yu Zhang
2017-03-10 11:28             ` Yu Zhang
2017-03-10 15:00             ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).