linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/8] This patch-set is to enable kvm Guest OS CET support.
@ 2018-12-26  8:15 Yang Weijiang
  2018-12-26  8:15 ` [PATCH v1 1/8] kvm:vmx Introduce CET related VMCS field definitions Yang Weijiang
                   ` (7 more replies)
  0 siblings, 8 replies; 21+ messages in thread
From: Yang Weijiang @ 2018-12-26  8:15 UTC (permalink / raw)
  To: pbonzini, rkrcmar, linux-kernel, kvm, mst, yu-cheng.yu,
	yi.z.zhang, hjl.tools
  Cc: Yang Weijiang

Control-flow Enforcement Technology (CET) provides protection against
return/jump-oriented programming (ROP) attacks. To make kvm Guest OS
own the capability, this patch-set is required. It enables CET related
CPUID report, xsaves/xrstors, vmx entry configuration etc for Guest OS.

PATCH 1/3/4: Enable CET support in VMCS.
PATCH 2    : Define CR4.CET master enable bit.
PATCH 5    : Enable xsave components for CET in XSS.
PATCH 6/7/8: Report CET feature support in CPUID.
 
Yang Weijiang (8):
  kvm:vmx  Introduce CET related VMCS field definitions.
  kvm:  Define CR4.CET[bit 23] (master enable bit) for guest OS.
  kvm:vmx  Enable loading CET state bit while guest CR4.CET is being
    set.
  kvm:vmx  Pass through host CET related MSRs to Guest.
  kvm:x86  Enable MSR_IA32_XSS bit 11 and 12 for CET xsaves/xrstors.
  kvm:cpuid  Add CPUID support for CET xsaves component query.
  kvm:cpuid  Fix xsaves area size calculation for CPUID.(EAX=0xD,ECX=1).
  kvm:cpuid  Report CET SHSTK and IBT support in CPUID.(EAX=0x7,ECX=0).

 arch/x86/include/asm/kvm_host.h |  3 ++-
 arch/x86/include/asm/vmx.h      |  8 +++++++
 arch/x86/kvm/cpuid.c            | 23 +++++++++++++-------
 arch/x86/kvm/vmx.c              | 37 ++++++++++++++++++++++++++++++---
 4 files changed, 60 insertions(+), 11 deletions(-)

-- 
2.17.1


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

* [PATCH v1 1/8] kvm:vmx  Introduce CET related VMCS field definitions.
  2018-12-26  8:15 [PATCH v1 0/8] This patch-set is to enable kvm Guest OS CET support Yang Weijiang
@ 2018-12-26  8:15 ` Yang Weijiang
  2019-01-02 18:09   ` Sean Christopherson
  2018-12-26  8:15 ` [PATCH v1 2/8] kvm: Define CR4.CET[bit 23] (master enable bit) for guest OS Yang Weijiang
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Yang Weijiang @ 2018-12-26  8:15 UTC (permalink / raw)
  To: pbonzini, rkrcmar, linux-kernel, kvm, mst, yu-cheng.yu,
	yi.z.zhang, hjl.tools
  Cc: Yang Weijiang, Zhang Yi Z

VMX relies on these fields to save/restore CET states for
guest and host. They are added here as VMCS placeholders
for the function.

Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/include/asm/vmx.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index ade0f153947d..db745a1d49ae 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -98,6 +98,7 @@
 #define VM_EXIT_LOAD_IA32_EFER                  0x00200000
 #define VM_EXIT_SAVE_VMX_PREEMPTION_TIMER       0x00400000
 #define VM_EXIT_CLEAR_BNDCFGS                   0x00800000
+#define VM_EXIT_LOAD_HOST_CET_STATE             0x10000000
 
 #define VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR	0x00036dff
 
@@ -109,6 +110,7 @@
 #define VM_ENTRY_LOAD_IA32_PAT			0x00004000
 #define VM_ENTRY_LOAD_IA32_EFER                 0x00008000
 #define VM_ENTRY_LOAD_BNDCFGS                   0x00010000
+#define VM_ENTRY_LOAD_GUEST_CET_STATE           0x00100000
 
 #define VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR	0x000011ff
 
@@ -325,6 +327,9 @@ enum vmcs_field {
 	GUEST_PENDING_DBG_EXCEPTIONS    = 0x00006822,
 	GUEST_SYSENTER_ESP              = 0x00006824,
 	GUEST_SYSENTER_EIP              = 0x00006826,
+	GUEST_IA32_S_CET                = 0x00006828,
+	GUEST_SSP                       = 0x0000682a,
+	GUEST_INT_SSP_TABL              = 0x0000682c,
 	HOST_CR0                        = 0x00006c00,
 	HOST_CR3                        = 0x00006c02,
 	HOST_CR4                        = 0x00006c04,
@@ -337,6 +342,9 @@ enum vmcs_field {
 	HOST_IA32_SYSENTER_EIP          = 0x00006c12,
 	HOST_RSP                        = 0x00006c14,
 	HOST_RIP                        = 0x00006c16,
+	HOST_IA32_S_CET                 = 0x00006c18,
+	HOST_SSP                        = 0x00006c1a,
+	HOST_INT_SSP_TABL               = 0x00006c1c
 };
 
 /*
-- 
2.17.1


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

* [PATCH v1 2/8] kvm:  Define CR4.CET[bit 23] (master enable bit) for guest OS.
  2018-12-26  8:15 [PATCH v1 0/8] This patch-set is to enable kvm Guest OS CET support Yang Weijiang
  2018-12-26  8:15 ` [PATCH v1 1/8] kvm:vmx Introduce CET related VMCS field definitions Yang Weijiang
@ 2018-12-26  8:15 ` Yang Weijiang
  2018-12-26  8:15 ` [PATCH v1 3/8] kvm:vmx Enable loading CET state bit while guest CR4.CET is being set Yang Weijiang
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 21+ messages in thread
From: Yang Weijiang @ 2018-12-26  8:15 UTC (permalink / raw)
  To: pbonzini, rkrcmar, linux-kernel, kvm, mst, yu-cheng.yu,
	yi.z.zhang, hjl.tools
  Cc: Yang Weijiang, Zhang Yi Z

This bit controls CET feature availability in guest OS.

Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/include/asm/kvm_host.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 55e51ff7e421..df002936088f 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -90,7 +90,8 @@
 			  | 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_LA57 | X86_CR4_VMXE \
-			  | X86_CR4_SMAP | X86_CR4_PKE | X86_CR4_UMIP))
+			  | X86_CR4_SMAP | X86_CR4_PKE | X86_CR4_UMIP \
+			  | X86_CR4_CET))
 
 #define CR8_RESERVED_BITS (~(unsigned long)X86_CR8_TPR)
 
-- 
2.17.1


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

* [PATCH v1 3/8] kvm:vmx  Enable loading CET state bit while guest CR4.CET is being set.
  2018-12-26  8:15 [PATCH v1 0/8] This patch-set is to enable kvm Guest OS CET support Yang Weijiang
  2018-12-26  8:15 ` [PATCH v1 1/8] kvm:vmx Introduce CET related VMCS field definitions Yang Weijiang
  2018-12-26  8:15 ` [PATCH v1 2/8] kvm: Define CR4.CET[bit 23] (master enable bit) for guest OS Yang Weijiang
@ 2018-12-26  8:15 ` Yang Weijiang
  2018-12-26  8:52   ` Liran Alon
  2018-12-26  8:15 ` [PATCH v1 4/8] kvm:vmx Pass through host CET related MSRs to Guest Yang Weijiang
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Yang Weijiang @ 2018-12-26  8:15 UTC (permalink / raw)
  To: pbonzini, rkrcmar, linux-kernel, kvm, mst, yu-cheng.yu,
	yi.z.zhang, hjl.tools
  Cc: Yang Weijiang, Zhang Yi Z

This bit controls whether guest CET states will be loaded on guest entry.

Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/kvm/vmx.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 7bbb8b26e901..25fa6bd2fb95 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -1045,6 +1045,8 @@ struct vcpu_vmx {
 
 	bool req_immediate_exit;
 
+	bool vcpu_cet_on;
+
 	/* Support for PML */
 #define PML_ENTITY_NUM		512
 	struct page *pml_pg;
@@ -5409,6 +5411,23 @@ static int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
 			return 1;
 	}
 
+	/*
+	 * When CET.CR4 is being set, it means we're enabling CET for
+	 * the guest, then enable loading CET state bit in entry control.
+	 * Otherwise, clear loading CET bit to disable guest CET.
+	 */
+	if (cr4 & X86_CR4_CET) {
+		if (!to_vmx(vcpu)->vcpu_cet_on) {
+			vmcs_set_bits(VM_ENTRY_CONTROLS,
+				      VM_ENTRY_LOAD_GUEST_CET_STATE);
+			to_vmx(vcpu)->vcpu_cet_on = 1;
+		}
+	} else if (to_vmx(vcpu)->vcpu_cet_on) {
+		vmcs_clear_bits(VM_ENTRY_CONTROLS,
+				VM_ENTRY_LOAD_GUEST_CET_STATE);
+		to_vmx(vcpu)->vcpu_cet_on = 0;
+	}
+
 	if (to_vmx(vcpu)->nested.vmxon && !nested_cr4_valid(vcpu, cr4))
 		return 1;
 
-- 
2.17.1


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

* [PATCH v1 4/8] kvm:vmx  Pass through host CET related MSRs to Guest.
  2018-12-26  8:15 [PATCH v1 0/8] This patch-set is to enable kvm Guest OS CET support Yang Weijiang
                   ` (2 preceding siblings ...)
  2018-12-26  8:15 ` [PATCH v1 3/8] kvm:vmx Enable loading CET state bit while guest CR4.CET is being set Yang Weijiang
@ 2018-12-26  8:15 ` Yang Weijiang
  2019-01-02 18:18   ` Sean Christopherson
  2019-01-02 19:12   ` Jim Mattson
  2018-12-26  8:15 ` [PATCH v1 5/8] kvm:x86 Enable MSR_IA32_XSS bit 11 and 12 for CET xsaves/xrstors Yang Weijiang
                   ` (3 subsequent siblings)
  7 siblings, 2 replies; 21+ messages in thread
From: Yang Weijiang @ 2018-12-26  8:15 UTC (permalink / raw)
  To: pbonzini, rkrcmar, linux-kernel, kvm, mst, yu-cheng.yu,
	yi.z.zhang, hjl.tools
  Cc: Yang Weijiang, Zhang Yi Z

During Guest OS execution, it accesses these MSRs to
configure CET runtime settings.

Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/kvm/vmx.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 25fa6bd2fb95..fa2db6248404 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -11550,6 +11550,13 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id)
 	vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_SYSENTER_CS, MSR_TYPE_RW);
 	vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_SYSENTER_ESP, MSR_TYPE_RW);
 	vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_SYSENTER_EIP, MSR_TYPE_RW);
+
+	vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_U_CET, MSR_TYPE_RW);
+	vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_S_CET, MSR_TYPE_RW);
+	vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_INT_SSP_TAB, MSR_TYPE_RW);
+	vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_PL0_SSP, MSR_TYPE_RW);
+	vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_PL3_SSP, MSR_TYPE_RW);
+
 	vmx->msr_bitmap_mode = 0;
 
 	vmx->loaded_vmcs = &vmx->vmcs01;
-- 
2.17.1


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

* [PATCH v1 5/8] kvm:x86  Enable MSR_IA32_XSS bit 11 and 12 for CET xsaves/xrstors.
  2018-12-26  8:15 [PATCH v1 0/8] This patch-set is to enable kvm Guest OS CET support Yang Weijiang
                   ` (3 preceding siblings ...)
  2018-12-26  8:15 ` [PATCH v1 4/8] kvm:vmx Pass through host CET related MSRs to Guest Yang Weijiang
@ 2018-12-26  8:15 ` Yang Weijiang
  2019-01-02 18:24   ` Sean Christopherson
  2018-12-26  8:15 ` [PATCH v1 6/8] kvm:cpuid Add CPUID support for CET xsaves component query Yang Weijiang
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 21+ messages in thread
From: Yang Weijiang @ 2018-12-26  8:15 UTC (permalink / raw)
  To: pbonzini, rkrcmar, linux-kernel, kvm, mst, yu-cheng.yu,
	yi.z.zhang, hjl.tools
  Cc: Yang Weijiang, Zhang Yi Z

For kvm Guest OS, right now, only bit 11(user mode CET) and bit 12
(supervisor CET) are supported in XSS MSR, if other bits are being set,
the write to XSS will be skipped.

Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/kvm/vmx.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index fa2db6248404..5739ab393b90 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -47,6 +47,7 @@
 #include <asm/virtext.h>
 #include <asm/mce.h>
 #include <asm/fpu/internal.h>
+#include <asm/fpu/types.h>
 #include <asm/perf_event.h>
 #include <asm/debugreg.h>
 #include <asm/kexec.h>
@@ -4323,12 +4324,16 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 	case MSR_IA32_XSS:
 		if (!vmx_xsaves_supported())
 			return 1;
+
 		/*
-		 * The only supported bit as of Skylake is bit 8, but
-		 * it is not supported on KVM.
+		 * Right now, only support XSS_CET_U[bit 11] and
+		 * XSS_CET_S[bit 12] in MSR_IA32_XSS.
 		 */
-		if (data != 0)
+
+		if (data & ~(XFEATURE_MASK_SHSTK_USER
+			     | XFEATURE_MASK_SHSTK_KERNEL))
 			return 1;
+
 		vcpu->arch.ia32_xss = data;
 		if (vcpu->arch.ia32_xss != host_xss)
 			add_atomic_switch_msr(vmx, MSR_IA32_XSS,
-- 
2.17.1


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

* [PATCH v1 6/8] kvm:cpuid  Add CPUID support for CET xsaves component query.
  2018-12-26  8:15 [PATCH v1 0/8] This patch-set is to enable kvm Guest OS CET support Yang Weijiang
                   ` (4 preceding siblings ...)
  2018-12-26  8:15 ` [PATCH v1 5/8] kvm:x86 Enable MSR_IA32_XSS bit 11 and 12 for CET xsaves/xrstors Yang Weijiang
@ 2018-12-26  8:15 ` Yang Weijiang
  2019-01-02 18:49   ` Sean Christopherson
  2018-12-26  8:15 ` [PATCH v1 7/8] kvm:cpuid Fix xsaves area size calculation for CPUID.(EAX=0xD,ECX=1) Yang Weijiang
  2018-12-26  8:15 ` [PATCH v1 8/8] kvm:cpuid Report CET SHSTK and IBT support in CPUID.(EAX=0x7,ECX=0) Yang Weijiang
  7 siblings, 1 reply; 21+ messages in thread
From: Yang Weijiang @ 2018-12-26  8:15 UTC (permalink / raw)
  To: pbonzini, rkrcmar, linux-kernel, kvm, mst, yu-cheng.yu,
	yi.z.zhang, hjl.tools
  Cc: Yang Weijiang, Zhang Yi Z

CET xsaves component size is queried through CPUID.(EAX=0xD, ECX=11)
and CPUID.(EAX=0xD, ECX=12).

Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/kvm/cpuid.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 7bcfa61375c0..5bac31e58955 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -565,6 +565,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 	case 0xd: {
 		int idx, i;
 		u64 supported = kvm_supported_xcr0();
+		u64 sv_supported;
 
 		entry->eax &= supported;
 		entry->ebx = xstate_required_size(supported, false);
@@ -584,18 +585,23 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 				entry[i].eax &= kvm_cpuid_D_1_eax_x86_features;
 				cpuid_mask(&entry[i].eax, CPUID_D_1_EAX);
 				entry[i].ebx = 0;
+				sv_supported = entry[i].ecx +
+					((u64)entry[i].edx << 32);
 				if (entry[i].eax & (F(XSAVES)|F(XSAVEC)))
 					entry[i].ebx =
 						xstate_required_size(supported,
 								     true);
-			} else {
+			} else if (!(entry[i].ecx & 1)) {
 				if (entry[i].eax == 0 || !(supported & mask))
 					continue;
-				if (WARN_ON_ONCE(entry[i].ecx & 1))
+				entry[i].ecx = 0;
+				entry[i].edx = 0;
+			} else {
+				if (entry[i].eax == 0 || !(sv_supported & mask))
 					continue;
+				entry[i].ecx = 1;
+				entry[i].edx = 0;
 			}
-			entry[i].ecx = 0;
-			entry[i].edx = 0;
 			entry[i].flags |=
 			       KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
 			++*nent;
-- 
2.17.1


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

* [PATCH v1 7/8] kvm:cpuid  Fix xsaves area size calculation for CPUID.(EAX=0xD,ECX=1).
  2018-12-26  8:15 [PATCH v1 0/8] This patch-set is to enable kvm Guest OS CET support Yang Weijiang
                   ` (5 preceding siblings ...)
  2018-12-26  8:15 ` [PATCH v1 6/8] kvm:cpuid Add CPUID support for CET xsaves component query Yang Weijiang
@ 2018-12-26  8:15 ` Yang Weijiang
  2019-01-02 18:54   ` Sean Christopherson
  2018-12-26  8:15 ` [PATCH v1 8/8] kvm:cpuid Report CET SHSTK and IBT support in CPUID.(EAX=0x7,ECX=0) Yang Weijiang
  7 siblings, 1 reply; 21+ messages in thread
From: Yang Weijiang @ 2018-12-26  8:15 UTC (permalink / raw)
  To: pbonzini, rkrcmar, linux-kernel, kvm, mst, yu-cheng.yu,
	yi.z.zhang, hjl.tools
  Cc: Yang Weijiang, Zhang Yi Z

According to latest Software Development Manual vol.2/3.2,
for CPUID.(EAX=0xD,ECX=1), it should report xsaves area size
containing all states enabled  by XCR0|IA32_MSR_XSS.

Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/kvm/cpuid.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 5bac31e58955..55c282f71f93 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -121,7 +121,8 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu)
 
 	best = kvm_find_cpuid_entry(vcpu, 0xD, 1);
 	if (best && (best->eax & (F(XSAVES) | F(XSAVEC))))
-		best->ebx = xstate_required_size(vcpu->arch.xcr0, true);
+		best->ebx = xstate_required_size(vcpu->arch.xcr0 |
+		best->ecx | ((u64)best->edx << 32), true);
 
 	/*
 	 * The existing code assumes virtual address is 48-bit or 57-bit in the
-- 
2.17.1


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

* [PATCH v1 8/8] kvm:cpuid  Report CET SHSTK and IBT support in CPUID.(EAX=0x7,ECX=0).
  2018-12-26  8:15 [PATCH v1 0/8] This patch-set is to enable kvm Guest OS CET support Yang Weijiang
                   ` (6 preceding siblings ...)
  2018-12-26  8:15 ` [PATCH v1 7/8] kvm:cpuid Fix xsaves area size calculation for CPUID.(EAX=0xD,ECX=1) Yang Weijiang
@ 2018-12-26  8:15 ` Yang Weijiang
  2019-01-02 19:00   ` Sean Christopherson
  2019-01-07 16:03   ` Paolo Bonzini
  7 siblings, 2 replies; 21+ messages in thread
From: Yang Weijiang @ 2018-12-26  8:15 UTC (permalink / raw)
  To: pbonzini, rkrcmar, linux-kernel, kvm, mst, yu-cheng.yu,
	yi.z.zhang, hjl.tools
  Cc: Yang Weijiang, Zhang Yi Z

Guest OS can query CET SHSTK and IBT support by CPUID.(EAX=0x7,ECX=0),
in return, ECX[bit 7] corresponds to SHSTK feature, and EDX[bit 20]
corresponds to IBT feature.

Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/kvm/cpuid.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 55c282f71f93..d7b60b3c4326 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -67,6 +67,8 @@ u64 kvm_supported_xcr0(void)
 
 #define F(x) bit(X86_FEATURE_##x)
 
+#define CET_IBT_BIT bit(20)
+
 /* For scattered features from cpufeatures.h; we currently expose none */
 #define KF(x) bit(KVM_CPUID_BIT_##x)
 
@@ -407,12 +409,12 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 		F(AVX512VBMI) | F(LA57) | F(PKU) | 0 /*OSPKE*/ |
 		F(AVX512_VPOPCNTDQ) | F(UMIP) | F(AVX512_VBMI2) | F(GFNI) |
 		F(VAES) | F(VPCLMULQDQ) | F(AVX512_VNNI) | F(AVX512_BITALG) |
-		F(CLDEMOTE);
+		F(CLDEMOTE) | F(SHSTK);
 
 	/* cpuid 7.0.edx*/
 	const u32 kvm_cpuid_7_0_edx_x86_features =
 		F(AVX512_4VNNIW) | F(AVX512_4FMAPS) | F(SPEC_CTRL) |
-		F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES);
+		F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES) | CET_IBT_BIT;
 
 	/* all calls to cpuid_count() should be made on the same cpu */
 	get_cpu();
-- 
2.17.1


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

* Re: [PATCH v1 3/8] kvm:vmx  Enable loading CET state bit while guest CR4.CET is being set.
  2018-12-26  8:15 ` [PATCH v1 3/8] kvm:vmx Enable loading CET state bit while guest CR4.CET is being set Yang Weijiang
@ 2018-12-26  8:52   ` Liran Alon
  2018-12-27  6:07     ` Yang,Weijiang
  0 siblings, 1 reply; 21+ messages in thread
From: Liran Alon @ 2018-12-26  8:52 UTC (permalink / raw)
  To: Yang Weijiang
  Cc: Paolo Bonzini, rkrcmar, linux-kernel, kvm, mst, yu-cheng.yu,
	yi.z.zhang, hjl.tools, Zhang Yi Z



> On 26 Dec 2018, at 10:15, Yang Weijiang <weijiang.yang@intel.com> wrote:
> 
> This bit controls whether guest CET states will be loaded on guest entry.
> 
> Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> ---
> arch/x86/kvm/vmx.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 7bbb8b26e901..25fa6bd2fb95 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -1045,6 +1045,8 @@ struct vcpu_vmx {
> 
> 	bool req_immediate_exit;
> 
> +	bool vcpu_cet_on;
> +
> 	/* Support for PML */
> #define PML_ENTITY_NUM		512
> 	struct page *pml_pg;
> @@ -5409,6 +5411,23 @@ static int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
> 			return 1;
> 	}
> 
> +	/*
> +	 * When CET.CR4 is being set, it means we're enabling CET for

You probably meant to write here CR4.CET.

> +	 * the guest, then enable loading CET state bit in entry control.
> +	 * Otherwise, clear loading CET bit to disable guest CET.
> +	 */
> +	if (cr4 & X86_CR4_CET) {
> +		if (!to_vmx(vcpu)->vcpu_cet_on) {
> +			vmcs_set_bits(VM_ENTRY_CONTROLS,
> +				      VM_ENTRY_LOAD_GUEST_CET_STATE);
> +			to_vmx(vcpu)->vcpu_cet_on = 1;
> +		}
> +	} else if (to_vmx(vcpu)->vcpu_cet_on) {
> +		vmcs_clear_bits(VM_ENTRY_CONTROLS,
> +				VM_ENTRY_LOAD_GUEST_CET_STATE);
> +		to_vmx(vcpu)->vcpu_cet_on = 0;
> +	}
> +
> 	if (to_vmx(vcpu)->nested.vmxon && !nested_cr4_valid(vcpu, cr4))
> 		return 1;
> 
> -- 
> 2.17.1
> 

I haven’t seen a patch in the series that modifies kvm_set_cr4() to verify CR4.CET is not set when CET is not reported as supported by CPUID.
I think that is missing from the series.

-Liran



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

* Re: [PATCH v1 3/8] kvm:vmx  Enable loading CET state bit while guest CR4.CET is being set.
  2018-12-26  8:52   ` Liran Alon
@ 2018-12-27  6:07     ` Yang,Weijiang
  0 siblings, 0 replies; 21+ messages in thread
From: Yang,Weijiang @ 2018-12-27  6:07 UTC (permalink / raw)
  To: Liran Alon
  Cc: Paolo Bonzini, rkrcmar, linux-kernel, kvm, mst, Yu, Yu-cheng,
	Zhang, Yi Z, hjl.tools, Zhang Yi Z

On Wed, Dec 26, 2018 at 04:52:38PM +0800, Liran Alon wrote:

Thanks Liran for pointing out the issues!
I'll fix them in next version.
> 
> 
> > On 26 Dec 2018, at 10:15, Yang Weijiang <weijiang.yang@intel.com> wrote:
> > 
> > This bit controls whether guest CET states will be loaded on guest entry.
> > 
> > Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
> > Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> > ---
> > arch/x86/kvm/vmx.c | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> > 
> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > index 7bbb8b26e901..25fa6bd2fb95 100644
> > --- a/arch/x86/kvm/vmx.c
> > +++ b/arch/x86/kvm/vmx.c
> > @@ -1045,6 +1045,8 @@ struct vcpu_vmx {
> > 
> > 	bool req_immediate_exit;
> > 
> > +	bool vcpu_cet_on;
> > +
> > 	/* Support for PML */
> > #define PML_ENTITY_NUM		512
> > 	struct page *pml_pg;
> > @@ -5409,6 +5411,23 @@ static int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
> > 			return 1;
> > 	}
> > 
> > +	/*
> > +	 * When CET.CR4 is being set, it means we're enabling CET for
> 
> You probably meant to write here CR4.CET.
> 
> > +	 * the guest, then enable loading CET state bit in entry control.
> > +	 * Otherwise, clear loading CET bit to disable guest CET.
> > +	 */
> > +	if (cr4 & X86_CR4_CET) {
> > +		if (!to_vmx(vcpu)->vcpu_cet_on) {
> > +			vmcs_set_bits(VM_ENTRY_CONTROLS,
> > +				      VM_ENTRY_LOAD_GUEST_CET_STATE);
> > +			to_vmx(vcpu)->vcpu_cet_on = 1;
> > +		}
> > +	} else if (to_vmx(vcpu)->vcpu_cet_on) {
> > +		vmcs_clear_bits(VM_ENTRY_CONTROLS,
> > +				VM_ENTRY_LOAD_GUEST_CET_STATE);
> > +		to_vmx(vcpu)->vcpu_cet_on = 0;
> > +	}
> > +
> > 	if (to_vmx(vcpu)->nested.vmxon && !nested_cr4_valid(vcpu, cr4))
> > 		return 1;
> > 
> > -- 
> > 2.17.1
> > 
> 
> I haven’t seen a patch in the series that modifies kvm_set_cr4() to verify CR4.CET is not set when CET is not reported as supported by CPUID.
> I think that is missing from the series.
> 
> -Liran
> 
> 

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

* Re: [PATCH v1 1/8] kvm:vmx  Introduce CET related VMCS field definitions.
  2018-12-26  8:15 ` [PATCH v1 1/8] kvm:vmx Introduce CET related VMCS field definitions Yang Weijiang
@ 2019-01-02 18:09   ` Sean Christopherson
  0 siblings, 0 replies; 21+ messages in thread
From: Sean Christopherson @ 2019-01-02 18:09 UTC (permalink / raw)
  To: Yang Weijiang
  Cc: pbonzini, rkrcmar, linux-kernel, kvm, mst, yu-cheng.yu,
	yi.z.zhang, hjl.tools, Zhang Yi Z

On Wed, Dec 26, 2018 at 04:15:25PM +0800, Yang Weijiang wrote:
> VMX relies on these fields to save/restore CET states for
> guest and host. They are added here as VMCS placeholders
> for the function.

The changelog needs a lot more detail on what is saved where and when,
e.g. hardware unconditionally saves the guest MSRs to the VMCS fields
on exit, hence there is no VM_EXIT_SAVE_GUEST_CET_STATE control bit.

It's also worth calling out that the other CET MSRs are saved/loaded
via XSAVES/XRSTORS.

> 
> Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> ---
>  arch/x86/include/asm/vmx.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
> index ade0f153947d..db745a1d49ae 100644
> --- a/arch/x86/include/asm/vmx.h
> +++ b/arch/x86/include/asm/vmx.h
> @@ -98,6 +98,7 @@
>  #define VM_EXIT_LOAD_IA32_EFER                  0x00200000
>  #define VM_EXIT_SAVE_VMX_PREEMPTION_TIMER       0x00400000
>  #define VM_EXIT_CLEAR_BNDCFGS                   0x00800000
> +#define VM_EXIT_LOAD_HOST_CET_STATE             0x10000000
>  
>  #define VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR	0x00036dff
>  
> @@ -109,6 +110,7 @@
>  #define VM_ENTRY_LOAD_IA32_PAT			0x00004000
>  #define VM_ENTRY_LOAD_IA32_EFER                 0x00008000
>  #define VM_ENTRY_LOAD_BNDCFGS                   0x00010000
> +#define VM_ENTRY_LOAD_GUEST_CET_STATE           0x00100000
>  
>  #define VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR	0x000011ff
>  
> @@ -325,6 +327,9 @@ enum vmcs_field {
>  	GUEST_PENDING_DBG_EXCEPTIONS    = 0x00006822,
>  	GUEST_SYSENTER_ESP              = 0x00006824,
>  	GUEST_SYSENTER_EIP              = 0x00006826,
> +	GUEST_IA32_S_CET                = 0x00006828,
> +	GUEST_SSP                       = 0x0000682a,
> +	GUEST_INT_SSP_TABL              = 0x0000682c,

Dropping a single letter from "TABLE" seems pointless, "INTERRUPT" is
usually "INTR", and the docs I've seen list the name as
IA32_INTERRUPT_SSP_TABLE_ADDR.  So maybe?

    {GUEST,HOST}_INTR_SSP_TABLE_ADDR


Also, the SSP and INTERRUPT_SSP_TABL_ADDR fields are 64-bit fields, we
we should define the _HIGH variations.


>  	HOST_CR0                        = 0x00006c00,
>  	HOST_CR3                        = 0x00006c02,
>  	HOST_CR4                        = 0x00006c04,
> @@ -337,6 +342,9 @@ enum vmcs_field {
>  	HOST_IA32_SYSENTER_EIP          = 0x00006c12,
>  	HOST_RSP                        = 0x00006c14,
>  	HOST_RIP                        = 0x00006c16,
> +	HOST_IA32_S_CET                 = 0x00006c18,
> +	HOST_SSP                        = 0x00006c1a,
> +	HOST_INT_SSP_TABL               = 0x00006c1c
>  };
>  
>  /*
> -- 
> 2.17.1
> 

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

* Re: [PATCH v1 4/8] kvm:vmx  Pass through host CET related MSRs to Guest.
  2018-12-26  8:15 ` [PATCH v1 4/8] kvm:vmx Pass through host CET related MSRs to Guest Yang Weijiang
@ 2019-01-02 18:18   ` Sean Christopherson
  2019-01-02 19:12   ` Jim Mattson
  1 sibling, 0 replies; 21+ messages in thread
From: Sean Christopherson @ 2019-01-02 18:18 UTC (permalink / raw)
  To: Yang Weijiang
  Cc: pbonzini, rkrcmar, linux-kernel, kvm, mst, yu-cheng.yu,
	yi.z.zhang, hjl.tools, Zhang Yi Z

On Wed, Dec 26, 2018 at 04:15:28PM +0800, Yang Weijiang wrote:
> During Guest OS execution, it accesses these MSRs to
> configure CET runtime settings.

The changelog needs to explain why it's ok to expose these MSRS to the
guest, e.g. call out which are saved/loaded through the VMCS and which
are saved/loaded via XSAVES/XRSTORS.  And maybe expand "runtime settings"
to elaborate on when most guest kernels will access the MSRs, i.e. to
justify exposing them to the guest.

It should also clarify why IA32_PL1_SSP and IA32_PL2_SSP are NOT exposed
to the guest.

> 
> Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> ---
>  arch/x86/kvm/vmx.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 25fa6bd2fb95..fa2db6248404 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -11550,6 +11550,13 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id)
>  	vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_SYSENTER_CS, MSR_TYPE_RW);
>  	vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_SYSENTER_ESP, MSR_TYPE_RW);
>  	vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_SYSENTER_EIP, MSR_TYPE_RW);
> +
> +	vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_U_CET, MSR_TYPE_RW);
> +	vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_S_CET, MSR_TYPE_RW);
> +	vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_INT_SSP_TAB, MSR_TYPE_RW);
> +	vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_PL0_SSP, MSR_TYPE_RW);
> +	vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_PL3_SSP, MSR_TYPE_RW);
> +
>  	vmx->msr_bitmap_mode = 0;
>  
>  	vmx->loaded_vmcs = &vmx->vmcs01;
> -- 
> 2.17.1
> 

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

* Re: [PATCH v1 5/8] kvm:x86  Enable MSR_IA32_XSS bit 11 and 12 for CET xsaves/xrstors.
  2018-12-26  8:15 ` [PATCH v1 5/8] kvm:x86 Enable MSR_IA32_XSS bit 11 and 12 for CET xsaves/xrstors Yang Weijiang
@ 2019-01-02 18:24   ` Sean Christopherson
  2019-01-02 19:19     ` Jim Mattson
  0 siblings, 1 reply; 21+ messages in thread
From: Sean Christopherson @ 2019-01-02 18:24 UTC (permalink / raw)
  To: Yang Weijiang
  Cc: pbonzini, rkrcmar, linux-kernel, kvm, mst, yu-cheng.yu,
	yi.z.zhang, hjl.tools, Zhang Yi Z

On Wed, Dec 26, 2018 at 04:15:29PM +0800, Yang Weijiang wrote:
> For kvm Guest OS, right now, only bit 11(user mode CET) and bit 12
> (supervisor CET) are supported in XSS MSR, if other bits are being set,
> the write to XSS will be skipped.
> 
> Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> ---
>  arch/x86/kvm/vmx.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index fa2db6248404..5739ab393b90 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -47,6 +47,7 @@
>  #include <asm/virtext.h>
>  #include <asm/mce.h>
>  #include <asm/fpu/internal.h>
> +#include <asm/fpu/types.h>
>  #include <asm/perf_event.h>
>  #include <asm/debugreg.h>
>  #include <asm/kexec.h>
> @@ -4323,12 +4324,16 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  	case MSR_IA32_XSS:
>  		if (!vmx_xsaves_supported())
>  			return 1;
> +
>  		/*
> -		 * The only supported bit as of Skylake is bit 8, but
> -		 * it is not supported on KVM.
> +		 * Right now, only support XSS_CET_U[bit 11] and
> +		 * XSS_CET_S[bit 12] in MSR_IA32_XSS.
>  		 */
> -		if (data != 0)
> +
> +		if (data & ~(XFEATURE_MASK_SHSTK_USER
> +			     | XFEATURE_MASK_SHSTK_KERNEL))

New lines are usually after the operator, e.g.:

	if (data & ~(XFEATURE_MASK_SHSTK_USER |
		     XFEATURE_MASK_SHSTK_KERNEL))

And doesn't this flow need to check that the bits are actually supported?

>  			return 1;
> +
>  		vcpu->arch.ia32_xss = data;
>  		if (vcpu->arch.ia32_xss != host_xss)
>  			add_atomic_switch_msr(vmx, MSR_IA32_XSS,
> -- 
> 2.17.1
> 

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

* Re: [PATCH v1 6/8] kvm:cpuid  Add CPUID support for CET xsaves component query.
  2018-12-26  8:15 ` [PATCH v1 6/8] kvm:cpuid Add CPUID support for CET xsaves component query Yang Weijiang
@ 2019-01-02 18:49   ` Sean Christopherson
  0 siblings, 0 replies; 21+ messages in thread
From: Sean Christopherson @ 2019-01-02 18:49 UTC (permalink / raw)
  To: Yang Weijiang
  Cc: pbonzini, rkrcmar, linux-kernel, kvm, mst, yu-cheng.yu,
	yi.z.zhang, hjl.tools, Zhang Yi Z

On Wed, Dec 26, 2018 at 04:15:30PM +0800, Yang Weijiang wrote:
> CET xsaves component size is queried through CPUID.(EAX=0xD, ECX=11)
> and CPUID.(EAX=0xD, ECX=12).
> 
> Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> ---
>  arch/x86/kvm/cpuid.c | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 7bcfa61375c0..5bac31e58955 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -565,6 +565,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>  	case 0xd: {
>  		int idx, i;
>  		u64 supported = kvm_supported_xcr0();
> +		u64 sv_supported;

What about u_supported and s_supported?  sv_supported doesn't make me
think "supervisor"e.

>  		entry->eax &= supported;
>  		entry->ebx = xstate_required_size(supported, false);
> @@ -584,18 +585,23 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>  				entry[i].eax &= kvm_cpuid_D_1_eax_x86_features;
>  				cpuid_mask(&entry[i].eax, CPUID_D_1_EAX);
>  				entry[i].ebx = 0;
> +				sv_supported = entry[i].ecx +
> +					((u64)entry[i].edx << 32);

Use '|' instead of '+' to smush ECX and EDX together, as is it looks
like the code is calculating a size or something.

>  				if (entry[i].eax & (F(XSAVES)|F(XSAVEC)))
>  					entry[i].ebx =
>  						xstate_required_size(supported,
>  								     true);
> -			} else {
> +			} else if (!(entry[i].ecx & 1)) {

Now that we're actually consuming bit 0, it'd be nice to formally define
it as referring to supervisor state.

What about styling the code like this?  Might make it more obvious that
the logic for user vs. supervisor is identical, i.e. ECX bit 0 only
determines which mask is applied.

			if (idx == 1) {
				...
			} else {
				supported = (entry[i].ecx & 1) ? s_supported :
								 u_supported;
				if (entry[i].eax == 0 || !(supported & mask))
					continue;
				entry[i].ecx &= 1;
				entry[i].edx = 0;
			}


>  				if (entry[i].eax == 0 || !(supported & mask))
>  					continue;
> -				if (WARN_ON_ONCE(entry[i].ecx & 1))
> +				entry[i].ecx = 0;
> +				entry[i].edx = 0;
> +			} else {
> +				if (entry[i].eax == 0 || !(sv_supported & mask))
>  					continue;
> +				entry[i].ecx = 1;
> +				entry[i].edx = 0;
>  			}
> -			entry[i].ecx = 0;
> -			entry[i].edx = 0;

Removing this entirely isn't correct for CPUID.0xD.0x1, KVM should still
zero out the bits it doesn't handle, e.g. KVM will signal a fault if the
guest attempts to set any bits in IA32_XSS other than the CET bits.

>  			entry[i].flags |=
>  			       KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
>  			++*nent;
> -- 
> 2.17.1
> 

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

* Re: [PATCH v1 7/8] kvm:cpuid  Fix xsaves area size calculation for CPUID.(EAX=0xD,ECX=1).
  2018-12-26  8:15 ` [PATCH v1 7/8] kvm:cpuid Fix xsaves area size calculation for CPUID.(EAX=0xD,ECX=1) Yang Weijiang
@ 2019-01-02 18:54   ` Sean Christopherson
  0 siblings, 0 replies; 21+ messages in thread
From: Sean Christopherson @ 2019-01-02 18:54 UTC (permalink / raw)
  To: Yang Weijiang
  Cc: pbonzini, rkrcmar, linux-kernel, kvm, mst, yu-cheng.yu,
	yi.z.zhang, hjl.tools, Zhang Yi Z

On Wed, Dec 26, 2018 at 04:15:31PM +0800, Yang Weijiang wrote:
> According to latest Software Development Manual vol.2/3.2,
> for CPUID.(EAX=0xD,ECX=1), it should report xsaves area size
> containing all states enabled  by XCR0|IA32_MSR_XSS.
> 
> Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> ---
>  arch/x86/kvm/cpuid.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 5bac31e58955..55c282f71f93 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -121,7 +121,8 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu)
>  
>  	best = kvm_find_cpuid_entry(vcpu, 0xD, 1);
>  	if (best && (best->eax & (F(XSAVES) | F(XSAVEC))))
> -		best->ebx = xstate_required_size(vcpu->arch.xcr0, true);
> +		best->ebx = xstate_required_size(vcpu->arch.xcr0 |
> +		best->ecx | ((u64)best->edx << 32), true);

Passing the CPUID bits is wrong, it needs to be the guest's IA32_MSR_XSS.
The CPUID bits enumerate what is legal to enable, not what features are
actually enabled by software.

>  
>  	/*
>  	 * The existing code assumes virtual address is 48-bit or 57-bit in the
> -- 
> 2.17.1
> 

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

* Re: [PATCH v1 8/8] kvm:cpuid  Report CET SHSTK and IBT support in CPUID.(EAX=0x7,ECX=0).
  2018-12-26  8:15 ` [PATCH v1 8/8] kvm:cpuid Report CET SHSTK and IBT support in CPUID.(EAX=0x7,ECX=0) Yang Weijiang
@ 2019-01-02 19:00   ` Sean Christopherson
  2019-01-07 16:03   ` Paolo Bonzini
  1 sibling, 0 replies; 21+ messages in thread
From: Sean Christopherson @ 2019-01-02 19:00 UTC (permalink / raw)
  To: Yang Weijiang
  Cc: pbonzini, rkrcmar, linux-kernel, kvm, mst, yu-cheng.yu,
	yi.z.zhang, hjl.tools, Zhang Yi Z

On Wed, Dec 26, 2018 at 04:15:32PM +0800, Yang Weijiang wrote:
> Guest OS can query CET SHSTK and IBT support by CPUID.(EAX=0x7,ECX=0),
> in return, ECX[bit 7] corresponds to SHSTK feature, and EDX[bit 20]
> corresponds to IBT feature.
> 
> Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> ---
>  arch/x86/kvm/cpuid.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 55c282f71f93..d7b60b3c4326 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -67,6 +67,8 @@ u64 kvm_supported_xcr0(void)
>  
>  #define F(x) bit(X86_FEATURE_##x)
>  
> +#define CET_IBT_BIT bit(20)

Properly define X86_FEATURE_CET_IBT if it's not being defined by some
other series, even if KVM is the only immediate user of the bit.  Linux
reserves all of word 18 for CPUID_7_0_EDX, i.e. defining the bit won't
bloat some random scattered word.

> +
>  /* For scattered features from cpufeatures.h; we currently expose none */
>  #define KF(x) bit(KVM_CPUID_BIT_##x)
>  
> @@ -407,12 +409,12 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>  		F(AVX512VBMI) | F(LA57) | F(PKU) | 0 /*OSPKE*/ |
>  		F(AVX512_VPOPCNTDQ) | F(UMIP) | F(AVX512_VBMI2) | F(GFNI) |
>  		F(VAES) | F(VPCLMULQDQ) | F(AVX512_VNNI) | F(AVX512_BITALG) |
> -		F(CLDEMOTE);
> +		F(CLDEMOTE) | F(SHSTK);
>  
>  	/* cpuid 7.0.edx*/
>  	const u32 kvm_cpuid_7_0_edx_x86_features =
>  		F(AVX512_4VNNIW) | F(AVX512_4FMAPS) | F(SPEC_CTRL) |
> -		F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES);
> +		F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES) | CET_IBT_BIT;
>  
>  	/* all calls to cpuid_count() should be made on the same cpu */
>  	get_cpu();
> -- 
> 2.17.1
> 

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

* Re: [PATCH v1 4/8] kvm:vmx Pass through host CET related MSRs to Guest.
  2018-12-26  8:15 ` [PATCH v1 4/8] kvm:vmx Pass through host CET related MSRs to Guest Yang Weijiang
  2019-01-02 18:18   ` Sean Christopherson
@ 2019-01-02 19:12   ` Jim Mattson
  1 sibling, 0 replies; 21+ messages in thread
From: Jim Mattson @ 2019-01-02 19:12 UTC (permalink / raw)
  To: Yang Weijiang
  Cc: Paolo Bonzini, Radim Krčmář,
	LKML, kvm list, Michael S. Tsirkin, yu-cheng.yu, yi.z.zhang,
	hjl.tools, Zhang Yi Z

On Wed, Dec 26, 2018 at 12:13 AM Yang Weijiang <weijiang.yang@intel.com> wrote:
>
> During Guest OS execution, it accesses these MSRs to
> configure CET runtime settings.
>
> Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> ---
>  arch/x86/kvm/vmx.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 25fa6bd2fb95..fa2db6248404 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -11550,6 +11550,13 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id)
>         vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_SYSENTER_CS, MSR_TYPE_RW);
>         vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_SYSENTER_ESP, MSR_TYPE_RW);
>         vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_SYSENTER_EIP, MSR_TYPE_RW);
> +
> +       vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_U_CET, MSR_TYPE_RW);
> +       vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_S_CET, MSR_TYPE_RW);
> +       vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_INT_SSP_TAB, MSR_TYPE_RW);
> +       vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_PL0_SSP, MSR_TYPE_RW);
> +       vmx_disable_intercept_for_msr(msr_bitmap, MSR_IA32_PL3_SSP, MSR_TYPE_RW);
> +

Shouldn't this be conditional on the guest having CET support
enumerated in CPUID?

>         vmx->msr_bitmap_mode = 0;
>
>         vmx->loaded_vmcs = &vmx->vmcs01;
> --
> 2.17.1
>

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

* Re: [PATCH v1 5/8] kvm:x86 Enable MSR_IA32_XSS bit 11 and 12 for CET xsaves/xrstors.
  2019-01-02 18:24   ` Sean Christopherson
@ 2019-01-02 19:19     ` Jim Mattson
  2019-01-06 21:17       ` Yang Weijiang
  0 siblings, 1 reply; 21+ messages in thread
From: Jim Mattson @ 2019-01-02 19:19 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Yang Weijiang, Paolo Bonzini, Radim Krčmář,
	LKML, kvm list, Michael S. Tsirkin, yu-cheng.yu, yi.z.zhang,
	hjl.tools, Zhang Yi Z

On Wed, Jan 2, 2019 at 10:24 AM Sean Christopherson
<sean.j.christopherson@intel.com> wrote:
>
> On Wed, Dec 26, 2018 at 04:15:29PM +0800, Yang Weijiang wrote:
> > For kvm Guest OS, right now, only bit 11(user mode CET) and bit 12
> > (supervisor CET) are supported in XSS MSR, if other bits are being set,
> > the write to XSS will be skipped.
> >
> > Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
> > Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> > ---
> >  arch/x86/kvm/vmx.c | 11 ++++++++---
> >  1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > index fa2db6248404..5739ab393b90 100644
> > --- a/arch/x86/kvm/vmx.c
> > +++ b/arch/x86/kvm/vmx.c
> > @@ -47,6 +47,7 @@
> >  #include <asm/virtext.h>
> >  #include <asm/mce.h>
> >  #include <asm/fpu/internal.h>
> > +#include <asm/fpu/types.h>
> >  #include <asm/perf_event.h>
> >  #include <asm/debugreg.h>
> >  #include <asm/kexec.h>
> > @@ -4323,12 +4324,16 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >       case MSR_IA32_XSS:
> >               if (!vmx_xsaves_supported())
> >                       return 1;
> > +
> >               /*
> > -              * The only supported bit as of Skylake is bit 8, but
> > -              * it is not supported on KVM.
> > +              * Right now, only support XSS_CET_U[bit 11] and
> > +              * XSS_CET_S[bit 12] in MSR_IA32_XSS.
> >                */
> > -             if (data != 0)
> > +
> > +             if (data & ~(XFEATURE_MASK_SHSTK_USER
> > +                          | XFEATURE_MASK_SHSTK_KERNEL))
>
> New lines are usually after the operator, e.g.:
>
>         if (data & ~(XFEATURE_MASK_SHSTK_USER |
>                      XFEATURE_MASK_SHSTK_KERNEL))
>
> And doesn't this flow need to check that the bits are actually supported?
Supported on the host and in the guest.

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

* Re: [PATCH v1 5/8] kvm:x86 Enable MSR_IA32_XSS bit 11 and 12 for CET xsaves/xrstors.
  2019-01-02 19:19     ` Jim Mattson
@ 2019-01-06 21:17       ` Yang Weijiang
  0 siblings, 0 replies; 21+ messages in thread
From: Yang Weijiang @ 2019-01-06 21:17 UTC (permalink / raw)
  To: Jim Mattson
  Cc: Sean Christopherson, Paolo Bonzini, Radim Krčmář,
	LKML, kvm list, Michael S. Tsirkin, yu-cheng.yu, yi.z.zhang,
	hjl.tools, Zhang Yi Z

On Wed, Jan 02, 2019 at 11:19:34AM -0800, Jim Mattson wrote:
Thanks Sean and Jim for your valuable comments, I'll consolidate them in
next release!

> On Wed, Jan 2, 2019 at 10:24 AM Sean Christopherson
> <sean.j.christopherson@intel.com> wrote:
> >
> > On Wed, Dec 26, 2018 at 04:15:29PM +0800, Yang Weijiang wrote:
> > > For kvm Guest OS, right now, only bit 11(user mode CET) and bit 12
> > > (supervisor CET) are supported in XSS MSR, if other bits are being set,
> > > the write to XSS will be skipped.
> > >
> > > Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
> > > Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> > > ---
> > >  arch/x86/kvm/vmx.c | 11 ++++++++---
> > >  1 file changed, 8 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > > index fa2db6248404..5739ab393b90 100644
> > > --- a/arch/x86/kvm/vmx.c
> > > +++ b/arch/x86/kvm/vmx.c
> > > @@ -47,6 +47,7 @@
> > >  #include <asm/virtext.h>
> > >  #include <asm/mce.h>
> > >  #include <asm/fpu/internal.h>
> > > +#include <asm/fpu/types.h>
> > >  #include <asm/perf_event.h>
> > >  #include <asm/debugreg.h>
> > >  #include <asm/kexec.h>
> > > @@ -4323,12 +4324,16 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> > >       case MSR_IA32_XSS:
> > >               if (!vmx_xsaves_supported())
> > >                       return 1;
> > > +
> > >               /*
> > > -              * The only supported bit as of Skylake is bit 8, but
> > > -              * it is not supported on KVM.
> > > +              * Right now, only support XSS_CET_U[bit 11] and
> > > +              * XSS_CET_S[bit 12] in MSR_IA32_XSS.
> > >                */
> > > -             if (data != 0)
> > > +
> > > +             if (data & ~(XFEATURE_MASK_SHSTK_USER
> > > +                          | XFEATURE_MASK_SHSTK_KERNEL))
> >
> > New lines are usually after the operator, e.g.:
> >
> >         if (data & ~(XFEATURE_MASK_SHSTK_USER |
> >                      XFEATURE_MASK_SHSTK_KERNEL))
> >
> > And doesn't this flow need to check that the bits are actually supported?
> Supported on the host and in the guest.

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

* Re: [PATCH v1 8/8] kvm:cpuid Report CET SHSTK and IBT support in CPUID.(EAX=0x7,ECX=0).
  2018-12-26  8:15 ` [PATCH v1 8/8] kvm:cpuid Report CET SHSTK and IBT support in CPUID.(EAX=0x7,ECX=0) Yang Weijiang
  2019-01-02 19:00   ` Sean Christopherson
@ 2019-01-07 16:03   ` Paolo Bonzini
  1 sibling, 0 replies; 21+ messages in thread
From: Paolo Bonzini @ 2019-01-07 16:03 UTC (permalink / raw)
  To: Yang Weijiang, rkrcmar, linux-kernel, kvm, mst, yu-cheng.yu,
	yi.z.zhang, hjl.tools
  Cc: Zhang Yi Z

On 26/12/18 09:15, Yang Weijiang wrote:
> Guest OS can query CET SHSTK and IBT support by CPUID.(EAX=0x7,ECX=0),
> in return, ECX[bit 7] corresponds to SHSTK feature, and EDX[bit 20]
> corresponds to IBT feature.
> 
> Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> ---
>  arch/x86/kvm/cpuid.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 55c282f71f93..d7b60b3c4326 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -67,6 +67,8 @@ u64 kvm_supported_xcr0(void)
>  
>  #define F(x) bit(X86_FEATURE_##x)
>  
> +#define CET_IBT_BIT bit(20)
> +
>  /* For scattered features from cpufeatures.h; we currently expose none */
>  #define KF(x) bit(KVM_CPUID_BIT_##x)
>  
> @@ -407,12 +409,12 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>  		F(AVX512VBMI) | F(LA57) | F(PKU) | 0 /*OSPKE*/ |
>  		F(AVX512_VPOPCNTDQ) | F(UMIP) | F(AVX512_VBMI2) | F(GFNI) |
>  		F(VAES) | F(VPCLMULQDQ) | F(AVX512_VNNI) | F(AVX512_BITALG) |
> -		F(CLDEMOTE);
> +		F(CLDEMOTE) | F(SHSTK);
>  
>  	/* cpuid 7.0.edx*/
>  	const u32 kvm_cpuid_7_0_edx_x86_features =
>  		F(AVX512_4VNNIW) | F(AVX512_4FMAPS) | F(SPEC_CTRL) |
> -		F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES);
> +		F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES) | CET_IBT_BIT;
>  
>  	/* all calls to cpuid_count() should be made on the same cpu */
>  	get_cpu();
> 

This might change soon, but at least for now KVM's XSAVES/XRSTORS usage
depends on Linux's.  Therefore, this should only be enabled if Linux has
set bits 11 and 12 in MSR_IA32_XSS.

Paolo

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

end of thread, other threads:[~2019-01-07 16:04 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-26  8:15 [PATCH v1 0/8] This patch-set is to enable kvm Guest OS CET support Yang Weijiang
2018-12-26  8:15 ` [PATCH v1 1/8] kvm:vmx Introduce CET related VMCS field definitions Yang Weijiang
2019-01-02 18:09   ` Sean Christopherson
2018-12-26  8:15 ` [PATCH v1 2/8] kvm: Define CR4.CET[bit 23] (master enable bit) for guest OS Yang Weijiang
2018-12-26  8:15 ` [PATCH v1 3/8] kvm:vmx Enable loading CET state bit while guest CR4.CET is being set Yang Weijiang
2018-12-26  8:52   ` Liran Alon
2018-12-27  6:07     ` Yang,Weijiang
2018-12-26  8:15 ` [PATCH v1 4/8] kvm:vmx Pass through host CET related MSRs to Guest Yang Weijiang
2019-01-02 18:18   ` Sean Christopherson
2019-01-02 19:12   ` Jim Mattson
2018-12-26  8:15 ` [PATCH v1 5/8] kvm:x86 Enable MSR_IA32_XSS bit 11 and 12 for CET xsaves/xrstors Yang Weijiang
2019-01-02 18:24   ` Sean Christopherson
2019-01-02 19:19     ` Jim Mattson
2019-01-06 21:17       ` Yang Weijiang
2018-12-26  8:15 ` [PATCH v1 6/8] kvm:cpuid Add CPUID support for CET xsaves component query Yang Weijiang
2019-01-02 18:49   ` Sean Christopherson
2018-12-26  8:15 ` [PATCH v1 7/8] kvm:cpuid Fix xsaves area size calculation for CPUID.(EAX=0xD,ECX=1) Yang Weijiang
2019-01-02 18:54   ` Sean Christopherson
2018-12-26  8:15 ` [PATCH v1 8/8] kvm:cpuid Report CET SHSTK and IBT support in CPUID.(EAX=0x7,ECX=0) Yang Weijiang
2019-01-02 19:00   ` Sean Christopherson
2019-01-07 16:03   ` 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).