[PATCH v11 0/9] Introduce support for guest CET feature

[PATCH v11 0/9] Introduce support for guest CET feature

[Yang Weijiang]

Control-flow Enforcement Technology (CET) provides protection against
Return/Jump-Oriented Programming (ROP/JOP) attack. It includes two
sub-features: Shadow Stack (SHSTK) and Indirect Branch Tracking (IBT).

KVM needs to update to enable guest CET feature.
This patchset implements CET related CPUID/XSAVES enumeration, MSRs
and vmentry/vmexit configuration etc.so that guest kernel can setup CET
runtime infrastructure based on them. Some CET MSRs and related feature
flags used reference the definitions in kernel patchset.

CET kernel patches are here:
https://lkml.org/lkml/2020/2/5/593
https://lkml.org/lkml/2020/2/5/604

v10 -> v11
- Fixed a guest vmentry failure issue when guest reboots.
- Used vm_xxx_control_{set, clear}bit() to avoid side effect, it'll
  clear cached data instead of pure VMCS field bits.
- Added vcpu->arch.guest_supported_xss dedidated for guest runtime mask,
  this avoids supported_xss overwritten issue caused by an old qemu.
- Separated vmentry/vmexit state setting with CR0/CR4 dependency check
  to make the patch more clear.
- Added CET VMCS states in dump_vmcs() for debugging purpose.
- Other refactor based on testing.
- This patch serial is built on top of below branch and CET kernel patches
  for seeking xsaves support:
  https://git.kernel.org/pub/scm/virt/kvm/kvm.git/log/?h=cpu-caps

v9 -> v10
- Refactored code per Sean's review feedback.
- Added CET support for nested VM.
- Removed fix-patch for CPUID(0xd,N) enumeration as this part is done
  by Paolo and Sean.
- This new patchset is based on Paolo's queued cpu_caps branch.
- Modified patch per XSAVES related change.
- Consolidated KVM unit-test patch with KVM patches.

v8 -> v9:
- Refactored msr-check functions per Sean's feedback.
- Fixed a few issues per Sean's suggestion.
- Rebased patch to kernel-v5.4.
- Moved CET CPUID feature bits and CR4.CET to last patch.

v7 -> v8:
- Addressed Jim and Sean's feedback on: 1) CPUID(0xD,i) enumeration. 2)
  sanity check when configure guest CET. 3) function improvement.
- Added more sanity check functions.
- Set host vmexit default status so that guest won't leak CET status to
  host when vmexit.
- Added CR0.WP vs. CR4.CET mutual constrains.

v6 -> v7:
- Rebased patch to kernel v5.3
- Sean suggested to change CPUID(0xd, n) enumeration code as alined with
  existing one, and I think it's better to make the fix as an independent patch 
  since XSS MSR are being used widely on X86 platforms.
- Check more host and guest status before configure guest CET
  per Sean's feedback.
- Add error-check before guest accesses CET MSRs per Sean's feedback.
- Other minor fixes suggested by Sean.

v5 -> v6:
- Rebase patch to kernel v5.2.
- Move CPUID(0xD, n>=1) helper to a seperate patch.
- Merge xsave size fix with other patch.
- Other minor fixes per community feedback.

v4 -> v5:
- Rebase patch to kernel v5.1.
- Wrap CPUID(0xD, n>=1) code to a helper function.
- Pass through MSR_IA32_PL1_SSP and MSR_IA32_PL2_SSP to Guest.
- Add Co-developed-by expression in patch description.
- Refine patch description.

v3 -> v4:
- Add Sean's patch for loading Guest fpu state before access XSAVES
  managed CET MSRs.
- Melt down CET bits setting into CPUID configuration patch.
- Add VMX interface to query Host XSS.
- Check Host and Guest XSS support bits before set Guest XSS.
- Make Guest SHSTK and IBT feature enabling independent.
- Do not report CET support to Guest when Host CET feature is Disabled.

v2 -> v3:
- Modified patches to make Guest CET independent to Host enabling.
- Added patch 8 to add user space access for Guest CET MSR access.
- Modified code comments and patch description to reflect changes.

v1 -> v2:
- Re-ordered patch sequence, combined one patch.
- Added more description for CET related VMCS fields.
- Added Host CET capability check while enabling Guest CET loading bit.
- Added Host CET capability check while reporting Guest CPUID(EAX=7, EXC=0).
- Modified code in reporting Guest CPUID(EAX=D,ECX>=1), make it clearer.
- Added Host and Guest XSS mask check while setting bits for Guest XSS.



Sean Christopherson (1):
  KVM: X86: Load guest fpu state when access MSRs managed by XSAVES

Yang Weijiang (8):
  KVM: VMX: Introduce CET VMX fields and flags
  KVM: VMX: Set guest CET MSRs per KVM and host configuration
  KVM: VMX: Set host/guest CET states for vmexit/vmentry
  KVM: VMX: Check CET dependencies on CR settings
  KVM: X86: Refresh CPUID once guest XSS MSR changes
  KVM: X86: Add userspace access interface for CET MSRs
  KVM: VMX: Enable CET support for nested VM
  KVM: X86: Set CET feature bits for CPUID enumeration

 arch/x86/include/asm/kvm_host.h |   4 +-
 arch/x86/include/asm/vmx.h      |   8 ++
 arch/x86/include/uapi/asm/kvm.h |   1 +
 arch/x86/kvm/cpuid.c            |  25 +++-
 arch/x86/kvm/vmx/capabilities.h |  10 ++
 arch/x86/kvm/vmx/nested.c       |  41 +++++-
 arch/x86/kvm/vmx/vmcs12.c       |   6 +
 arch/x86/kvm/vmx/vmcs12.h       |  14 +-
 arch/x86/kvm/vmx/vmx.c          | 232 +++++++++++++++++++++++++++++++-
 arch/x86/kvm/x86.c              |  46 ++++++-
 arch/x86/kvm/x86.h              |   2 +-
 11 files changed, 376 insertions(+), 13 deletions(-)

-- 
2.17.2

[PATCH v11 1/9] KVM: VMX: Introduce CET VMX fields and flags

[Yang Weijiang]

CET(Control-flow Enforcement Technology) is a CPU feature
used to prevent Return/Jump-Oriented Programming(ROP/JOP)
attacks. It provides the following sub-features to defend
against ROP/JOP style control-flow subversion attacks:

Shadow Stack (SHSTK):
  A second stack for program which is used exclusively for
  control transfer operations.

Indirect Branch Tracking (IBT):
  Code branching protection to defend against jump/call oriented
  programming.

Several new CET MSRs are defined in kernel to support CET:
  MSR_IA32_{U,S}_CET: Controls the CET settings for user
                      mode and kernel mode respectively.

  MSR_IA32_PL{0,1,2,3}_SSP: Stores shadow stack pointers for
                            CPL-0,1,2,3 protection respectively.

  MSR_IA32_INT_SSP_TAB: Stores base address of shadow stack
                        pointer table.

Two XSAVES state bits are introduced for CET:
  IA32_XSS:[bit 11]: Control saving/restoring user mode CET states
  IA32_XSS:[bit 12]: Control saving/restoring kernel mode CET states.

Six VMCS fields are introduced for CET:
  {HOST,GUEST}_S_CET: Stores CET settings for kernel mode.
  {HOST,GUEST}_SSP: Stores shadow stack pointer of current task/thread.
  {HOST,GUEST}_INTR_SSP_TABLE: Stores base address of shadow stack pointer
                               table.

If VM_EXIT_LOAD_HOST_CET_STATE = 1, the host CET states are restored
from below VMCS fields at VM-Exit:
  HOST_S_CET
  HOST_SSP
  HOST_INTR_SSP_TABLE

If VM_ENTRY_LOAD_GUEST_CET_STATE = 1, the guest CET states are loaded
from below VMCS fields at VM-Entry:
  GUEST_S_CET
  GUEST_SSP
  GUEST_INTR_SSP_TABLE

Co-developed-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
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 ++++++++
 arch/x86/include/uapi/asm/kvm.h | 1 +
 arch/x86/kvm/x86.c              | 4 ++++
 arch/x86/kvm/x86.h              | 2 +-
 4 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index 5e090d1f03f8..e938bc6c37aa 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -94,6 +94,7 @@
 #define VM_EXIT_CLEAR_BNDCFGS                   0x00800000
 #define VM_EXIT_PT_CONCEAL_PIP			0x01000000
 #define VM_EXIT_CLEAR_IA32_RTIT_CTL		0x02000000
+#define VM_EXIT_LOAD_HOST_CET_STATE             0x10000000
 
 #define VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR	0x00036dff
 
@@ -107,6 +108,7 @@
 #define VM_ENTRY_LOAD_BNDCFGS                   0x00010000
 #define VM_ENTRY_PT_CONCEAL_PIP			0x00020000
 #define VM_ENTRY_LOAD_IA32_RTIT_CTL		0x00040000
+#define VM_ENTRY_LOAD_GUEST_CET_STATE           0x00100000
 
 #define VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR	0x000011ff
 
@@ -328,6 +330,9 @@ enum vmcs_field {
 	GUEST_PENDING_DBG_EXCEPTIONS    = 0x00006822,
 	GUEST_SYSENTER_ESP              = 0x00006824,
 	GUEST_SYSENTER_EIP              = 0x00006826,
+	GUEST_S_CET                     = 0x00006828,
+	GUEST_SSP                       = 0x0000682a,
+	GUEST_INTR_SSP_TABLE            = 0x0000682c,
 	HOST_CR0                        = 0x00006c00,
 	HOST_CR3                        = 0x00006c02,
 	HOST_CR4                        = 0x00006c04,
@@ -340,6 +345,9 @@ enum vmcs_field {
 	HOST_IA32_SYSENTER_EIP          = 0x00006c12,
 	HOST_RSP                        = 0x00006c14,
 	HOST_RIP                        = 0x00006c16,
+	HOST_S_CET                      = 0x00006c18,
+	HOST_SSP                        = 0x00006c1a,
+	HOST_INTR_SSP_TABLE             = 0x00006c1c
 };
 
 /*
diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 3f3f780c8c65..78e5c4266270 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -31,6 +31,7 @@
 #define MC_VECTOR 18
 #define XM_VECTOR 19
 #define VE_VECTOR 20
+#define CP_VECTOR 21
 
 /* Select x86 specific features in <linux/kvm.h> */
 #define __KVM_HAVE_PIT
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 40c6768942ae..830afe5038d1 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -186,6 +186,9 @@ static struct kvm_shared_msrs __percpu *shared_msrs;
 				| XFEATURE_MASK_BNDCSR | XFEATURE_MASK_AVX512 \
 				| XFEATURE_MASK_PKRU)
 
+#define KVM_SUPPORTED_XSS	(XFEATURE_MASK_CET_USER | \
+				 XFEATURE_MASK_CET_KERNEL)
+
 u64 __read_mostly host_efer;
 EXPORT_SYMBOL_GPL(host_efer);
 
@@ -402,6 +405,7 @@ static int exception_class(int vector)
 	case NP_VECTOR:
 	case SS_VECTOR:
 	case GP_VECTOR:
+	case CP_VECTOR:
 		return EXCPT_CONTRIBUTORY;
 	default:
 		break;
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index c1954e216b41..8f0baa6fa72f 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -115,7 +115,7 @@ static inline bool x86_exception_has_error_code(unsigned int vector)
 {
 	static u32 exception_has_error_code = BIT(DF_VECTOR) | BIT(TS_VECTOR) |
 			BIT(NP_VECTOR) | BIT(SS_VECTOR) | BIT(GP_VECTOR) |
-			BIT(PF_VECTOR) | BIT(AC_VECTOR);
+			BIT(PF_VECTOR) | BIT(AC_VECTOR) | BIT(CP_VECTOR);
 
 	return (1U << vector) & exception_has_error_code;
 }
-- 
2.17.2

[PATCH v11 2/9] KVM: VMX: Set guest CET MSRs per KVM and host configuration

[Yang Weijiang]

CET MSRs pass through guest directly to enhance performance.
CET runtime control settings are stored in MSR_IA32_{U,S}_CET,
Shadow Stack Pointer(SSP) are stored in MSR_IA32_PL{0,1,2,3}_SSP,
SSP table base address is stored in MSR_IA32_INT_SSP_TAB,
these MSRs are defined in kernel and re-used here.

MSR_IA32_U_CET and MSR_IA32_PL3_SSP are used for user-mode protection,
the MSR contents are switched between threads during scheduling,
it makes sense to pass through them so that the guest kernel can
use xsaves/xrstors to operate them efficiently. Other MSRs are used
for non-user mode protection. See SDM for detailed info.

The difference between CET VMCS fields and CET MSRs is that,the former
are used during VMEnter/VMExit, whereas the latter are used for CET
state storage between task/thread scheduling.

Co-developed-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
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/vmx.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 860e5f4a9f7b..1aca468d9a10 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -3033,6 +3033,13 @@ void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3)
 		vmcs_writel(GUEST_CR3, guest_cr3);
 }
 
+static bool is_cet_mode_allowed(struct kvm_vcpu *vcpu, u32 mode_mask)
+{
+	return ((supported_xss & mode_mask) &&
+		(guest_cpuid_has(vcpu, X86_FEATURE_SHSTK) ||
+		guest_cpuid_has(vcpu, X86_FEATURE_IBT)));
+}
+
 int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
@@ -7064,6 +7071,35 @@ static void update_intel_pt_cfg(struct kvm_vcpu *vcpu)
 		vmx->pt_desc.ctl_bitmask &= ~(0xfULL << (32 + i * 4));
 }
 
+static void vmx_update_intercept_for_cet_msr(struct kvm_vcpu *vcpu)
+{
+	struct vcpu_vmx *vmx = to_vmx(vcpu);
+	unsigned long *msr_bitmap = vmx->vmcs01.msr_bitmap;
+	bool flag;
+
+	flag = !is_cet_mode_allowed(vcpu, XFEATURE_MASK_CET_USER);
+	/*
+	 * U_CET is required for USER CET, and U_CET, PL3_SPP are bound as
+	 * one component and controlled by IA32_XSS[bit 11].
+	 */
+	vmx_set_intercept_for_msr(msr_bitmap, MSR_IA32_U_CET, MSR_TYPE_RW, flag);
+	vmx_set_intercept_for_msr(msr_bitmap, MSR_IA32_PL3_SSP, MSR_TYPE_RW, flag);
+
+	flag = !is_cet_mode_allowed(vcpu, XFEATURE_MASK_CET_KERNEL);
+	/*
+	 * S_CET is required for KERNEL CET, and PL0_SSP ... PL2_SSP are
+	 * bound as one component and controlled by IA32_XSS[bit 12].
+	 */
+	vmx_set_intercept_for_msr(msr_bitmap, MSR_IA32_S_CET, MSR_TYPE_RW, flag);
+	vmx_set_intercept_for_msr(msr_bitmap, MSR_IA32_PL0_SSP, MSR_TYPE_RW, flag);
+	vmx_set_intercept_for_msr(msr_bitmap, MSR_IA32_PL1_SSP, MSR_TYPE_RW, flag);
+	vmx_set_intercept_for_msr(msr_bitmap, MSR_IA32_PL2_SSP, MSR_TYPE_RW, flag);
+
+	flag |= !guest_cpuid_has(vcpu, X86_FEATURE_SHSTK);
+	/* SSP_TAB is only available for KERNEL SHSTK.*/
+	vmx_set_intercept_for_msr(msr_bitmap, MSR_IA32_INT_SSP_TAB, MSR_TYPE_RW, flag);
+}
+
 static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
@@ -7102,6 +7138,10 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
 			vmx_set_guest_msr(vmx, msr, enabled ? 0 : TSX_CTRL_RTM_DISABLE);
 		}
 	}
+
+	if (guest_cpuid_has(vcpu, X86_FEATURE_SHSTK) ||
+	    guest_cpuid_has(vcpu, X86_FEATURE_IBT))
+		vmx_update_intercept_for_cet_msr(vcpu);
 }
 
 static __init void vmx_set_cpu_caps(void)
-- 
2.17.2

[PATCH v11 3/9] KVM: VMX: Set host/guest CET states for vmexit/vmentry

[Yang Weijiang]

"Load {guest,host} CET state" bit controls whether guest/host
CET states will be loaded at VM entry/exit.
Set default host kernel CET states to 0s in VMCS to avoid guest
CET states leakage. When CR4.CET is cleared due to guest mode
change, make guest CET states invalid in VMCS, this can happen,
e.g., guest reboot.

Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/kvm/vmx/capabilities.h | 10 ++++++
 arch/x86/kvm/vmx/vmx.c          | 56 +++++++++++++++++++++++++++++++--
 2 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h
index 8903475f751e..565340352260 100644
--- a/arch/x86/kvm/vmx/capabilities.h
+++ b/arch/x86/kvm/vmx/capabilities.h
@@ -107,6 +107,16 @@ static inline bool cpu_has_vmx_mpx(void)
 		(vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_BNDCFGS);
 }
 
+static inline bool cpu_has_cet_guest_load_ctrl(void)
+{
+	return (vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_GUEST_CET_STATE);
+}
+
+static inline bool cpu_has_cet_host_load_ctrl(void)
+{
+	return (vmcs_config.vmexit_ctrl & VM_EXIT_LOAD_HOST_CET_STATE);
+}
+
 static inline bool cpu_has_vmx_tpr_shadow(void)
 {
 	return vmcs_config.cpu_based_exec_ctrl & CPU_BASED_TPR_SHADOW;
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 1aca468d9a10..bd7cd175fd81 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -44,6 +44,7 @@
 #include <asm/spec-ctrl.h>
 #include <asm/virtext.h>
 #include <asm/vmx.h>
+#include <asm/cet.h>
 
 #include "capabilities.h"
 #include "cpuid.h"
@@ -2456,7 +2457,8 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf,
 	      VM_EXIT_LOAD_IA32_EFER |
 	      VM_EXIT_CLEAR_BNDCFGS |
 	      VM_EXIT_PT_CONCEAL_PIP |
-	      VM_EXIT_CLEAR_IA32_RTIT_CTL;
+	      VM_EXIT_CLEAR_IA32_RTIT_CTL |
+	      VM_EXIT_LOAD_HOST_CET_STATE;
 	if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_EXIT_CTLS,
 				&_vmexit_control) < 0)
 		return -EIO;
@@ -2480,7 +2482,8 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf,
 	      VM_ENTRY_LOAD_IA32_EFER |
 	      VM_ENTRY_LOAD_BNDCFGS |
 	      VM_ENTRY_PT_CONCEAL_PIP |
-	      VM_ENTRY_LOAD_IA32_RTIT_CTL;
+	      VM_ENTRY_LOAD_IA32_RTIT_CTL |
+	      VM_ENTRY_LOAD_GUEST_CET_STATE;
 	if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_ENTRY_CTLS,
 				&_vmentry_control) < 0)
 		return -EIO;
@@ -3040,6 +3043,12 @@ static bool is_cet_mode_allowed(struct kvm_vcpu *vcpu, u32 mode_mask)
 		guest_cpuid_has(vcpu, X86_FEATURE_IBT)));
 }
 
+static bool is_cet_supported(struct kvm_vcpu *vcpu)
+{
+	return is_cet_mode_allowed(vcpu, XFEATURE_MASK_CET_USER |
+				   XFEATURE_MASK_CET_KERNEL);
+}
+
 int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
@@ -3110,6 +3119,12 @@ int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
 			hw_cr4 &= ~(X86_CR4_SMEP | X86_CR4_SMAP | X86_CR4_PKE);
 	}
 
+	if (!(hw_cr4 & X86_CR4_CET) && is_cet_supported(vcpu)) {
+		vmcs_writel(GUEST_SSP, 0);
+		vmcs_writel(GUEST_S_CET, 0);
+		vmcs_writel(GUEST_INTR_SSP_TABLE, 0);
+	}
+
 	vmcs_writel(CR4_READ_SHADOW, cr4);
 	vmcs_writel(GUEST_CR4, hw_cr4);
 	return 0;
@@ -3939,6 +3954,12 @@ void vmx_set_constant_host_state(struct vcpu_vmx *vmx)
 
 	if (cpu_has_load_ia32_efer())
 		vmcs_write64(HOST_IA32_EFER, host_efer);
+
+	if (cpu_has_cet_host_load_ctrl()) {
+		vmcs_writel(HOST_S_CET, 0);
+		vmcs_writel(HOST_INTR_SSP_TABLE, 0);
+		vmcs_writel(HOST_SSP, 0);
+	}
 }
 
 void set_cr4_guest_host_mask(struct vcpu_vmx *vmx)
@@ -5749,6 +5770,13 @@ void dump_vmcs(void)
 		pr_err("InterruptStatus = %04x\n",
 		       vmcs_read16(GUEST_INTR_STATUS));
 
+	if (vmentry_ctl & VM_ENTRY_LOAD_GUEST_CET_STATE) {
+		pr_err("S_CET = 0x%016lx\n", vmcs_readl(GUEST_S_CET));
+		pr_err("SSP = 0x%016lx\n", vmcs_readl(GUEST_SSP));
+		pr_err("SSP TABLE = 0x%016lx\n",
+		       vmcs_readl(GUEST_INTR_SSP_TABLE));
+	}
+
 	pr_err("*** Host State ***\n");
 	pr_err("RIP = 0x%016lx  RSP = 0x%016lx\n",
 	       vmcs_readl(HOST_RIP), vmcs_readl(HOST_RSP));
@@ -5831,6 +5859,13 @@ void dump_vmcs(void)
 	if (secondary_exec_control & SECONDARY_EXEC_ENABLE_VPID)
 		pr_err("Virtual processor ID = 0x%04x\n",
 		       vmcs_read16(VIRTUAL_PROCESSOR_ID));
+	if (vmexit_ctl & VM_EXIT_LOAD_HOST_CET_STATE) {
+		pr_err("S_CET = 0x%016lx\n", vmcs_readl(HOST_S_CET));
+		pr_err("SSP = 0x%016lx\n", vmcs_readl(HOST_SSP));
+		pr_err("SSP TABLE = 0x%016lx\n",
+		       vmcs_readl(HOST_INTR_SSP_TABLE));
+	}
+
 }
 
 /*
@@ -7140,8 +7175,23 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
 	}
 
 	if (guest_cpuid_has(vcpu, X86_FEATURE_SHSTK) ||
-	    guest_cpuid_has(vcpu, X86_FEATURE_IBT))
+	    guest_cpuid_has(vcpu, X86_FEATURE_IBT)) {
 		vmx_update_intercept_for_cet_msr(vcpu);
+
+		if (cpu_has_cet_guest_load_ctrl() && is_cet_supported(vcpu))
+			vm_entry_controls_setbit(to_vmx(vcpu),
+						 VM_ENTRY_LOAD_GUEST_CET_STATE);
+		else
+			vm_entry_controls_clearbit(to_vmx(vcpu),
+						   VM_ENTRY_LOAD_GUEST_CET_STATE);
+
+		if (cpu_has_cet_host_load_ctrl() && is_cet_supported(vcpu))
+			vm_exit_controls_setbit(to_vmx(vcpu),
+						VM_EXIT_LOAD_HOST_CET_STATE);
+		else
+			vm_exit_controls_clearbit(to_vmx(vcpu),
+						  VM_EXIT_LOAD_HOST_CET_STATE);
+	}
 }
 
 static __init void vmx_set_cpu_caps(void)
-- 
2.17.2

[PATCH v11 4/9] KVM: VMX: Check CET dependencies on CR settings

[Yang Weijiang]

CR4.CET is master control bit for CET function.
There're mutual constrains between CR0.WP and CR4.CET, so need
to check the dependent bit while changing the control registers.

Note:
1)The processor does not allow CR4.CET to be set if CR0.WP = 0,
  similarly, it does not allow CR0.WP to be cleared while
  CR4.CET = 1. In either case, KVM would inject #GP to guest.

2)SHSTK and IBT features share one control MSR:
  MSR_IA32_{U,S}_CET, which means it's difficult to hide
  one feature from another in the case of SHSTK != IBT,
  after discussed in community, it's agreed to allow guest
  control two features independently as it won't introduce
  security hole.

Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/kvm/vmx/vmx.c | 4 ++++
 arch/x86/kvm/x86.c     | 3 +++
 2 files changed, 7 insertions(+)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index bd7cd175fd81..87f101750746 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -3089,6 +3089,10 @@ int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
 			return 1;
 	}
 
+	if ((cr4 & X86_CR4_CET) && (!is_cet_supported(vcpu) ||
+	    !(kvm_read_cr0(vcpu) & X86_CR0_WP)))
+		return 1;
+
 	if (vmx->nested.vmxon && !nested_cr4_valid(vcpu, cr4))
 		return 1;
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 830afe5038d1..90acdbbb8a5a 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -804,6 +804,9 @@ int kvm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0)
 	if (!(cr0 & X86_CR0_PG) && kvm_read_cr4_bits(vcpu, X86_CR4_PCIDE))
 		return 1;
 
+	if (!(cr0 & X86_CR0_WP) && kvm_read_cr4_bits(vcpu, X86_CR4_CET))
+		return 1;
+
 	kvm_x86_ops->set_cr0(vcpu, cr0);
 
 	if ((cr0 ^ old_cr0) & X86_CR0_PG) {
-- 
2.17.2

[PATCH v11 5/9] KVM: X86: Refresh CPUID once guest XSS MSR changes

[Yang Weijiang]

CPUID(0xd, 1) reports the current required storage size of
XCR0 | XSS, when guest updates the XSS, it's necessary to update
the CPUID leaf, otherwise guest will fetch old state size, and
results to some WARN traces during guest running.

Co-developed-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
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 |  1 +
 arch/x86/kvm/cpuid.c            | 21 ++++++++++++++++++---
 arch/x86/kvm/x86.c              |  9 +++++++--
 3 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 24c90ea5ddbd..2c944ad99692 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -650,6 +650,7 @@ struct kvm_vcpu_arch {
 
 	u64 xcr0;
 	u64 guest_supported_xcr0;
+	u64 guest_supported_xss;
 	u32 guest_xstate_size;
 
 	struct kvm_pio_request pio;
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 78d461be2102..25e9a11291b3 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -95,9 +95,24 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu)
 	}
 
 	best = kvm_find_cpuid_entry(vcpu, 0xD, 1);
-	if (best && (cpuid_entry_has(best, X86_FEATURE_XSAVES) ||
-		     cpuid_entry_has(best, X86_FEATURE_XSAVEC)))
-		best->ebx = xstate_required_size(vcpu->arch.xcr0, true);
+	if (best) {
+		if (best->eax & (F(XSAVES) | F(XSAVEC))) {
+			u64 xstate = vcpu->arch.xcr0 | vcpu->arch.ia32_xss;
+
+			best->ebx = xstate_required_size(xstate, true);
+		}
+
+		if (best->eax & F(XSAVES)) {
+			vcpu->arch.guest_supported_xss =
+			(best->ecx | ((u64)best->edx << 32)) & supported_xss;
+		} else {
+			best->ecx = 0;
+			best->edx = 0;
+			vcpu->arch.guest_supported_xss = 0;
+		}
+	} else {
+		vcpu->arch.guest_supported_xss = 0;
+	}
 
 	/*
 	 * The existing code assumes virtual address is 48-bit or 57-bit in the
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 90acdbbb8a5a..51ecb496d47d 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2836,9 +2836,12 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		 * IA32_XSS[bit 8]. Guests have to use RDMSR/WRMSR rather than
 		 * XSAVES/XRSTORS to save/restore PT MSRs.
 		 */
-		if (data & ~supported_xss)
+		if (data & ~vcpu->arch.guest_supported_xss)
 			return 1;
-		vcpu->arch.ia32_xss = data;
+		if (vcpu->arch.ia32_xss != data) {
+			vcpu->arch.ia32_xss = data;
+			kvm_update_cpuid(vcpu);
+		}
 		break;
 	case MSR_SMI_COUNT:
 		if (!msr_info->host_initiated)
@@ -9635,6 +9638,8 @@ int kvm_arch_hardware_setup(void)
 
 	if (!kvm_cpu_cap_has(X86_FEATURE_XSAVES))
 		supported_xss = 0;
+	else
+		supported_xss = host_xss & KVM_SUPPORTED_XSS;
 
 	cr4_reserved_bits = kvm_host_cr4_reserved_bits(&boot_cpu_data);
 
-- 
2.17.2

[PATCH v11 6/9] KVM: X86: Load guest fpu state when access MSRs managed by XSAVES

[Yang Weijiang]

From: Sean Christopherson <sean.j.christopherson@intel.com>

A handful of CET MSRs are not context switched through "traditional"
methods, e.g. VMCS or manual switching, but rather are passed through
to the guest and are saved and restored by XSAVES/XRSTORS, i.e. in the
guest's FPU state.

Load the guest's FPU state if userspace is accessing MSRs whose values
are managed by XSAVES so that the MSR helper, e.g. vmx_{get,set}_msr(),
can simply do {RD,WR}MSR to access the guest's value.

Note that guest_cpuid_has() is not queried as host userspace is allowed
to access MSRs that have not been exposed to the guest, e.g. it might do
KVM_SET_MSRS prior to KVM_SET_CPUID2.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Co-developed-by: Yang Weijiang <weijiang.yang@intel.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/kvm/x86.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 51ecb496d47d..9654d779bdab 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -109,6 +109,8 @@ static void enter_smm(struct kvm_vcpu *vcpu);
 static void __kvm_set_rflags(struct kvm_vcpu *vcpu, unsigned long rflags);
 static void store_regs(struct kvm_vcpu *vcpu);
 static int sync_regs(struct kvm_vcpu *vcpu);
+static void kvm_load_guest_fpu(struct kvm_vcpu *vcpu);
+static void kvm_put_guest_fpu(struct kvm_vcpu *vcpu);
 
 struct kvm_x86_ops *kvm_x86_ops __read_mostly;
 EXPORT_SYMBOL_GPL(kvm_x86_ops);
@@ -3249,6 +3251,12 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 }
 EXPORT_SYMBOL_GPL(kvm_get_msr_common);
 
+static bool is_xsaves_msr(u32 index)
+{
+	return index == MSR_IA32_U_CET ||
+	       (index >= MSR_IA32_PL0_SSP && index <= MSR_IA32_PL3_SSP);
+}
+
 /*
  * Read or write a bunch of msrs. All parameters are kernel addresses.
  *
@@ -3259,11 +3267,20 @@ static int __msr_io(struct kvm_vcpu *vcpu, struct kvm_msrs *msrs,
 		    int (*do_msr)(struct kvm_vcpu *vcpu,
 				  unsigned index, u64 *data))
 {
+	bool fpu_loaded = false;
 	int i;
 
-	for (i = 0; i < msrs->nmsrs; ++i)
+	for (i = 0; i < msrs->nmsrs; ++i) {
+		if (vcpu && !fpu_loaded && supported_xss &&
+		    is_xsaves_msr(entries[i].index)) {
+			kvm_load_guest_fpu(vcpu);
+			fpu_loaded = true;
+		}
 		if (do_msr(vcpu, entries[i].index, &entries[i].data))
 			break;
+	}
+	if (fpu_loaded)
+		kvm_put_guest_fpu(vcpu);
 
 	return i;
 }
-- 
2.17.2

[PATCH v11 7/9] KVM: X86: Add userspace access interface for CET MSRs

[Yang Weijiang]

There're two different places storing Guest CET states, states
managed with XSAVES/XRSTORS, as restored/saved
in previous patch, can be read/write directly from/to the MSRs.
For those stored in VMCS fields, they're access via vmcs_read/
vmcs_write.

To correctly read/write the CET MSRs, it's necessary to check
whether the kernel FPU context switch happened and reload guest
FPU context if needed.

Suggested-by: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/kvm/vmx/vmx.c | 133 +++++++++++++++++++++++++++++++++++++++++
 arch/x86/kvm/x86.c     |  11 ++++
 2 files changed, 144 insertions(+)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 87f101750746..a3d01014b9e7 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -1809,6 +1809,91 @@ static int vmx_get_msr_feature(struct kvm_msr_entry *msr)
 	}
 }
 
+static void vmx_get_xsave_msr(struct msr_data *msr_info)
+{
+	local_irq_disable();
+	if (test_thread_flag(TIF_NEED_FPU_LOAD))
+		switch_fpu_return();
+	rdmsrl(msr_info->index, msr_info->data);
+	local_irq_enable();
+}
+
+static void vmx_set_xsave_msr(struct msr_data *msr_info)
+{
+	local_irq_disable();
+	if (test_thread_flag(TIF_NEED_FPU_LOAD))
+		switch_fpu_return();
+	wrmsrl(msr_info->index, msr_info->data);
+	local_irq_enable();
+}
+
+#define CET_MSR_RSVD_BITS_1  GENMASK(1, 0)
+#define CET_MSR_RSVD_BITS_2  GENMASK(9, 6)
+
+static bool cet_check_msr_write(struct kvm_vcpu *vcpu,
+				struct msr_data *msr,
+				u64 mask)
+{
+	u64 data = msr->data;
+	u32 high_word = data >> 32;
+
+	if (data & mask)
+		return false;
+
+	if (!is_64_bit_mode(vcpu) && high_word)
+		return false;
+
+	return true;
+}
+
+static bool cet_check_ssp_msr_access(struct kvm_vcpu *vcpu,
+				     struct msr_data *msr)
+{
+	u32 index = msr->index;
+
+	if (!boot_cpu_has(X86_FEATURE_SHSTK))
+		return false;
+
+	if (!msr->host_initiated &&
+	    !guest_cpuid_has(vcpu, X86_FEATURE_SHSTK))
+		return false;
+
+	if (index == MSR_IA32_INT_SSP_TAB)
+		return true;
+
+	if (index == MSR_IA32_PL3_SSP) {
+		if (!(supported_xss & XFEATURE_MASK_CET_USER))
+			return false;
+	} else if (!(supported_xss & XFEATURE_MASK_CET_KERNEL)) {
+		return false;
+	}
+
+	return true;
+}
+
+static bool cet_check_ctl_msr_access(struct kvm_vcpu *vcpu,
+				     struct msr_data *msr)
+{
+	u32 index = msr->index;
+
+	if (!boot_cpu_has(X86_FEATURE_SHSTK) &&
+	    !boot_cpu_has(X86_FEATURE_IBT))
+		return false;
+
+	if (!msr->host_initiated &&
+	    !guest_cpuid_has(vcpu, X86_FEATURE_SHSTK) &&
+	    !guest_cpuid_has(vcpu, X86_FEATURE_IBT))
+		return false;
+
+	if (index == MSR_IA32_U_CET) {
+		if (!(supported_xss & XFEATURE_MASK_CET_USER))
+			return false;
+	} else if (!(supported_xss & XFEATURE_MASK_CET_KERNEL)) {
+		return false;
+	}
+
+	return true;
+}
 /*
  * Reads an msr value (of 'msr_index') into 'pdata'.
  * Returns 0 on success, non-0 otherwise.
@@ -1941,6 +2026,26 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		else
 			msr_info->data = vmx->pt_desc.guest.addr_a[index / 2];
 		break;
+	case MSR_IA32_S_CET:
+		if (!cet_check_ctl_msr_access(vcpu, msr_info))
+			return 1;
+		msr_info->data = vmcs_readl(GUEST_S_CET);
+		break;
+	case MSR_IA32_INT_SSP_TAB:
+		if (!cet_check_ssp_msr_access(vcpu, msr_info))
+			return 1;
+		msr_info->data = vmcs_readl(GUEST_INTR_SSP_TABLE);
+		break;
+	case MSR_IA32_U_CET:
+		if (!cet_check_ctl_msr_access(vcpu, msr_info))
+			return 1;
+		vmx_get_xsave_msr(msr_info);
+		break;
+	case MSR_IA32_PL0_SSP ... MSR_IA32_PL3_SSP:
+		if (!cet_check_ssp_msr_access(vcpu, msr_info))
+			return 1;
+		vmx_get_xsave_msr(msr_info);
+		break;
 	case MSR_TSC_AUX:
 		if (!msr_info->host_initiated &&
 		    !guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP))
@@ -2197,6 +2302,34 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		else
 			vmx->pt_desc.guest.addr_a[index / 2] = data;
 		break;
+	case MSR_IA32_S_CET:
+		if (!cet_check_ctl_msr_access(vcpu, msr_info))
+			return 1;
+		if (!cet_check_msr_write(vcpu, msr_info, CET_MSR_RSVD_BITS_2))
+			return 1;
+		vmcs_writel(GUEST_S_CET, data);
+		break;
+	case MSR_IA32_INT_SSP_TAB:
+		if (!cet_check_ctl_msr_access(vcpu, msr_info))
+			return 1;
+		if (!is_64_bit_mode(vcpu))
+			return 1;
+		vmcs_writel(GUEST_INTR_SSP_TABLE, data);
+		break;
+	case MSR_IA32_U_CET:
+		if (!cet_check_ctl_msr_access(vcpu, msr_info))
+			return 1;
+		if (!cet_check_msr_write(vcpu, msr_info, CET_MSR_RSVD_BITS_2))
+			return 1;
+		vmx_set_xsave_msr(msr_info);
+		break;
+	case MSR_IA32_PL0_SSP ... MSR_IA32_PL3_SSP:
+		if (!cet_check_ssp_msr_access(vcpu, msr_info))
+			return 1;
+		if (!cet_check_msr_write(vcpu, msr_info, CET_MSR_RSVD_BITS_1))
+			return 1;
+		vmx_set_xsave_msr(msr_info);
+		break;
 	case MSR_TSC_AUX:
 		if (!msr_info->host_initiated &&
 		    !guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP))
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 9654d779bdab..9e89ee6a09e1 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1229,6 +1229,10 @@ static const u32 msrs_to_save_all[] = {
 	MSR_ARCH_PERFMON_EVENTSEL0 + 12, MSR_ARCH_PERFMON_EVENTSEL0 + 13,
 	MSR_ARCH_PERFMON_EVENTSEL0 + 14, MSR_ARCH_PERFMON_EVENTSEL0 + 15,
 	MSR_ARCH_PERFMON_EVENTSEL0 + 16, MSR_ARCH_PERFMON_EVENTSEL0 + 17,
+
+	MSR_IA32_XSS, MSR_IA32_U_CET, MSR_IA32_S_CET,
+	MSR_IA32_PL0_SSP, MSR_IA32_PL1_SSP, MSR_IA32_PL2_SSP,
+	MSR_IA32_PL3_SSP, MSR_IA32_INT_SSP_TAB,
 };
 
 static u32 msrs_to_save[ARRAY_SIZE(msrs_to_save_all)];
@@ -1504,6 +1508,13 @@ static int __kvm_set_msr(struct kvm_vcpu *vcpu, u32 index, u64 data,
 		 * invokes 64-bit SYSENTER.
 		 */
 		data = get_canonical(data, vcpu_virt_addr_bits(vcpu));
+		break;
+	case MSR_IA32_PL0_SSP ... MSR_IA32_PL3_SSP:
+	case MSR_IA32_U_CET:
+	case MSR_IA32_S_CET:
+	case MSR_IA32_INT_SSP_TAB:
+		if (is_noncanonical_address(data, vcpu))
+			return 1;
 	}
 
 	msr.data = data;
-- 
2.17.2

[PATCH v11 8/9] KVM: VMX: Enable CET support for nested VM

[Yang Weijiang]

CET MSRs pass through guests for performance consideration.
Configure the MSRs to match L0/L1 settings so that nested VM
is able to run with CET.

Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/kvm/vmx/nested.c | 41 +++++++++++++++++++++++++++++++++++++--
 arch/x86/kvm/vmx/vmcs12.c |  6 ++++++
 arch/x86/kvm/vmx/vmcs12.h | 14 ++++++++++++-
 arch/x86/kvm/vmx/vmx.c    |  1 +
 4 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index e47eb7c0fbae..a71ef33de55f 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -627,6 +627,41 @@ static inline bool nested_vmx_prepare_msr_bitmap(struct kvm_vcpu *vcpu,
 	nested_vmx_disable_intercept_for_msr(msr_bitmap_l1, msr_bitmap_l0,
 					     MSR_KERNEL_GS_BASE, MSR_TYPE_RW);
 
+	/* Pass CET MSRs to nested VM if L0 and L1 are set to pass-through. */
+	if (!msr_write_intercepted_l01(vcpu, MSR_IA32_U_CET))
+		nested_vmx_disable_intercept_for_msr(
+					msr_bitmap_l1, msr_bitmap_l0,
+					MSR_IA32_U_CET, MSR_TYPE_RW);
+
+	if (!msr_write_intercepted_l01(vcpu, MSR_IA32_PL3_SSP))
+		nested_vmx_disable_intercept_for_msr(
+					msr_bitmap_l1, msr_bitmap_l0,
+					MSR_IA32_PL3_SSP, MSR_TYPE_RW);
+
+	if (!msr_write_intercepted_l01(vcpu, MSR_IA32_S_CET))
+		nested_vmx_disable_intercept_for_msr(
+					msr_bitmap_l1, msr_bitmap_l0,
+					MSR_IA32_S_CET, MSR_TYPE_RW);
+
+	if (!msr_write_intercepted_l01(vcpu, MSR_IA32_PL0_SSP))
+		nested_vmx_disable_intercept_for_msr(
+					msr_bitmap_l1, msr_bitmap_l0,
+					MSR_IA32_PL0_SSP, MSR_TYPE_RW);
+
+	if (!msr_write_intercepted_l01(vcpu, MSR_IA32_PL1_SSP))
+		nested_vmx_disable_intercept_for_msr(
+					msr_bitmap_l1, msr_bitmap_l0,
+					MSR_IA32_PL1_SSP, MSR_TYPE_RW);
+
+	if (!msr_write_intercepted_l01(vcpu, MSR_IA32_PL2_SSP))
+		nested_vmx_disable_intercept_for_msr(
+					msr_bitmap_l1, msr_bitmap_l0,
+					MSR_IA32_PL2_SSP, MSR_TYPE_RW);
+
+	if (!msr_write_intercepted_l01(vcpu, MSR_IA32_INT_SSP_TAB))
+		nested_vmx_disable_intercept_for_msr(
+					msr_bitmap_l1, msr_bitmap_l0,
+					MSR_IA32_INT_SSP_TAB, MSR_TYPE_RW);
 	/*
 	 * Checking the L0->L1 bitmap is trying to verify two things:
 	 *
@@ -6040,7 +6075,8 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
 	msrs->exit_ctls_high |=
 		VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR |
 		VM_EXIT_LOAD_IA32_EFER | VM_EXIT_SAVE_IA32_EFER |
-		VM_EXIT_SAVE_VMX_PREEMPTION_TIMER | VM_EXIT_ACK_INTR_ON_EXIT;
+		VM_EXIT_SAVE_VMX_PREEMPTION_TIMER | VM_EXIT_ACK_INTR_ON_EXIT |
+		VM_EXIT_LOAD_HOST_CET_STATE;
 
 	/* We support free control of debug control saving. */
 	msrs->exit_ctls_low &= ~VM_EXIT_SAVE_DEBUG_CONTROLS;
@@ -6057,7 +6093,8 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
 #endif
 		VM_ENTRY_LOAD_IA32_PAT;
 	msrs->entry_ctls_high |=
-		(VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR | VM_ENTRY_LOAD_IA32_EFER);
+		(VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR | VM_ENTRY_LOAD_IA32_EFER |
+		 VM_ENTRY_LOAD_GUEST_CET_STATE);
 
 	/* We support free control of debug control loading. */
 	msrs->entry_ctls_low &= ~VM_ENTRY_LOAD_DEBUG_CONTROLS;
diff --git a/arch/x86/kvm/vmx/vmcs12.c b/arch/x86/kvm/vmx/vmcs12.c
index 53dfb401316d..82b82bebeee0 100644
--- a/arch/x86/kvm/vmx/vmcs12.c
+++ b/arch/x86/kvm/vmx/vmcs12.c
@@ -141,6 +141,9 @@ const unsigned short vmcs_field_to_offset_table[] = {
 	FIELD(GUEST_PENDING_DBG_EXCEPTIONS, guest_pending_dbg_exceptions),
 	FIELD(GUEST_SYSENTER_ESP, guest_sysenter_esp),
 	FIELD(GUEST_SYSENTER_EIP, guest_sysenter_eip),
+	FIELD(GUEST_S_CET, guest_s_cet),
+	FIELD(GUEST_SSP, guest_ssp),
+	FIELD(GUEST_INTR_SSP_TABLE, guest_ssp_tbl),
 	FIELD(HOST_CR0, host_cr0),
 	FIELD(HOST_CR3, host_cr3),
 	FIELD(HOST_CR4, host_cr4),
@@ -153,5 +156,8 @@ const unsigned short vmcs_field_to_offset_table[] = {
 	FIELD(HOST_IA32_SYSENTER_EIP, host_ia32_sysenter_eip),
 	FIELD(HOST_RSP, host_rsp),
 	FIELD(HOST_RIP, host_rip),
+	FIELD(HOST_S_CET, host_s_cet),
+	FIELD(HOST_SSP, host_ssp),
+	FIELD(HOST_INTR_SSP_TABLE, host_ssp_tbl),
 };
 const unsigned int nr_vmcs12_fields = ARRAY_SIZE(vmcs_field_to_offset_table);
diff --git a/arch/x86/kvm/vmx/vmcs12.h b/arch/x86/kvm/vmx/vmcs12.h
index d0c6df373f67..62b7be68f05c 100644
--- a/arch/x86/kvm/vmx/vmcs12.h
+++ b/arch/x86/kvm/vmx/vmcs12.h
@@ -118,7 +118,13 @@ struct __packed vmcs12 {
 	natural_width host_ia32_sysenter_eip;
 	natural_width host_rsp;
 	natural_width host_rip;
-	natural_width paddingl[8]; /* room for future expansion */
+	natural_width host_s_cet;
+	natural_width host_ssp;
+	natural_width host_ssp_tbl;
+	natural_width guest_s_cet;
+	natural_width guest_ssp;
+	natural_width guest_ssp_tbl;
+	natural_width paddingl[2]; /* room for future expansion */
 	u32 pin_based_vm_exec_control;
 	u32 cpu_based_vm_exec_control;
 	u32 exception_bitmap;
@@ -301,6 +307,12 @@ static inline void vmx_check_vmcs12_offsets(void)
 	CHECK_OFFSET(host_ia32_sysenter_eip, 656);
 	CHECK_OFFSET(host_rsp, 664);
 	CHECK_OFFSET(host_rip, 672);
+	CHECK_OFFSET(host_s_cet, 680);
+	CHECK_OFFSET(host_ssp, 688);
+	CHECK_OFFSET(host_ssp_tbl, 696);
+	CHECK_OFFSET(guest_s_cet, 704);
+	CHECK_OFFSET(guest_ssp, 712);
+	CHECK_OFFSET(guest_ssp_tbl, 720);
 	CHECK_OFFSET(pin_based_vm_exec_control, 744);
 	CHECK_OFFSET(cpu_based_vm_exec_control, 748);
 	CHECK_OFFSET(exception_bitmap, 752);
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index a3d01014b9e7..c2e950d378bd 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7153,6 +7153,7 @@ static void nested_vmx_cr_fixed1_bits_update(struct kvm_vcpu *vcpu)
 	cr4_fixed1_update(X86_CR4_PKE,        ecx, feature_bit(PKU));
 	cr4_fixed1_update(X86_CR4_UMIP,       ecx, feature_bit(UMIP));
 	cr4_fixed1_update(X86_CR4_LA57,       ecx, feature_bit(LA57));
+	cr4_fixed1_update(X86_CR4_CET,	      ecx, feature_bit(SHSTK));
 
 #undef cr4_fixed1_update
 }
-- 
2.17.2

[PATCH v11 9/9] KVM: X86: Set CET feature bits for CPUID enumeration

[Yang Weijiang]

Set the feature bits so that CET capabilities can be seen
in guest via CPUID enumeration. Add CR4.CET bit support
in order to allow guest set CET master control bit(CR4.CET).

Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/include/asm/kvm_host.h | 3 ++-
 arch/x86/kvm/cpuid.c            | 4 ++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 2c944ad99692..5109c43c6981 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -95,7 +95,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)
 
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 25e9a11291b3..26ab959df92f 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -366,6 +366,10 @@ void kvm_set_cpu_caps(void)
 		kvm_cpu_cap_set(X86_FEATURE_INTEL_STIBP);
 	if (boot_cpu_has(X86_FEATURE_AMD_SSBD))
 		kvm_cpu_cap_set(X86_FEATURE_SPEC_CTRL_SSBD);
+	if (boot_cpu_has(X86_FEATURE_IBT))
+		kvm_cpu_cap_set(X86_FEATURE_IBT);
+	if (boot_cpu_has(X86_FEATURE_SHSTK))
+		kvm_cpu_cap_set(X86_FEATURE_SHSTK);
 
 	kvm_cpu_cap_mask(CPUID_7_1_EAX,
 		F(AVX512_BF16)
-- 
2.17.2

[kvm-unit-tests PATCH] x86: Add tests for user-mode CET

[Yang Weijiang]

This unit-test is intended to test user-mode CET support of KVM,
it's tested on Intel new platform. Two CET features: Shadow Stack
Protection(SHSTK) and Indirect-Branch Tracking(IBT) are enclosed.

In SHSTK test, if the function return-address in normal stack is
tampered with a value not equal to the one on shadow-stack, #CP
(Control Protection Exception)will generated on function returning.
This feature is supported by processor itself, no compiler/link
option is required.

However, to enabled IBT, we need to add -fcf-protection=full in
compiler options, this makes the compiler insert endbr64 at the
very beginning of each jmp/call target given the binary is for
x86_64.

To get PASS results, the following conditions must be met:
1) The processor is powered with CET feature.
2) The kernel is patched with the latest CET kernel patches.
3) The KVM and QEMU are patched with the latest CET patches.
4) Use CET-enabled gcc to compile the test app.

Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 lib/x86/desc.c      |   2 +
 lib/x86/msr.h       |   2 +
 lib/x86/processor.h |   3 +
 x86/Makefile.common |   3 +-
 x86/Makefile.x86_64 |   3 +-
 x86/cet.c           | 212 ++++++++++++++++++++++++++++++++++++++++++++
 x86/unittests.cfg   |   6 ++
 7 files changed, 229 insertions(+), 2 deletions(-)
 create mode 100644 x86/cet.c

diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index 451f504..983d4d8 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -179,6 +179,7 @@ EX(mf, 16);
 EX_E(ac, 17);
 EX(mc, 18);
 EX(xm, 19);
+EX_E(cp, 21);
 
 asm (".pushsection .text \n\t"
      "__handle_exception: \n\t"
@@ -224,6 +225,7 @@ static void *idt_handlers[32] = {
 	[17] = &ac_fault,
 	[18] = &mc_fault,
 	[19] = &xm_fault,
+	[21] = &cp_fault,
 };
 
 void setup_idt(void)
diff --git a/lib/x86/msr.h b/lib/x86/msr.h
index 8dca964..98489e0 100644
--- a/lib/x86/msr.h
+++ b/lib/x86/msr.h
@@ -208,6 +208,8 @@
 #define MSR_IA32_EBL_CR_POWERON		0x0000002a
 #define MSR_IA32_FEATURE_CONTROL        0x0000003a
 #define MSR_IA32_TSC_ADJUST		0x0000003b
+#define MSR_IA32_U_CET                  0x000006a0
+#define MSR_IA32_PL3_SSP                0x000006a7
 
 #define FEATURE_CONTROL_LOCKED				(1<<0)
 #define FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX	(1<<1)
diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index 03fdf64..5763d62 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -44,6 +44,7 @@
 #define X86_CR4_SMEP   0x00100000
 #define X86_CR4_SMAP   0x00200000
 #define X86_CR4_PKE    0x00400000
+#define X86_CR4_CET    0x00800000
 
 #define X86_EFLAGS_CF    0x00000001
 #define X86_EFLAGS_FIXED 0x00000002
@@ -149,8 +150,10 @@ static inline u8 cpuid_maxphyaddr(void)
 #define	X86_FEATURE_PKU			(CPUID(0x7, 0, ECX, 3))
 #define	X86_FEATURE_LA57		(CPUID(0x7, 0, ECX, 16))
 #define	X86_FEATURE_RDPID		(CPUID(0x7, 0, ECX, 22))
+#define	X86_FEATURE_SHSTK		(CPUID(0x7, 0, ECX, 7))
 #define	X86_FEATURE_SPEC_CTRL		(CPUID(0x7, 0, EDX, 26))
 #define	X86_FEATURE_ARCH_CAPABILITIES	(CPUID(0x7, 0, EDX, 29))
+#define	X86_FEATURE_IBT			(CPUID(0x7, 0, EDX, 20))
 #define	X86_FEATURE_NX			(CPUID(0x80000001, 0, EDX, 20))
 #define	X86_FEATURE_RDPRU		(CPUID(0x80000008, 0, EBX, 4))
 
diff --git a/x86/Makefile.common b/x86/Makefile.common
index ab67ca0..c5b4d2c 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -58,7 +58,8 @@ tests-common = $(TEST_DIR)/vmexit.flat $(TEST_DIR)/tsc.flat \
                $(TEST_DIR)/init.flat $(TEST_DIR)/smap.flat \
                $(TEST_DIR)/hyperv_synic.flat $(TEST_DIR)/hyperv_stimer.flat \
                $(TEST_DIR)/hyperv_connections.flat \
-               $(TEST_DIR)/umip.flat $(TEST_DIR)/tsx-ctrl.flat
+               $(TEST_DIR)/umip.flat $(TEST_DIR)/tsx-ctrl.flat \
+               $(TEST_DIR)/cet.flat
 
 test_cases: $(tests-common) $(tests)
 
diff --git a/x86/Makefile.x86_64 b/x86/Makefile.x86_64
index 010102b..43d9706 100644
--- a/x86/Makefile.x86_64
+++ b/x86/Makefile.x86_64
@@ -1,7 +1,7 @@
 cstart.o = $(TEST_DIR)/cstart64.o
 bits = 64
 ldarch = elf64-x86-64
-COMMON_CFLAGS += -mno-red-zone -mno-sse -mno-sse2
+COMMON_CFLAGS += -mno-red-zone -mno-sse -mno-sse2 -fcf-protection=full
 
 cflatobjs += lib/x86/setjmp64.o
 cflatobjs += lib/x86/intel-iommu.o
@@ -20,6 +20,7 @@ tests += $(TEST_DIR)/tscdeadline_latency.flat
 tests += $(TEST_DIR)/intel-iommu.flat
 tests += $(TEST_DIR)/vmware_backdoors.flat
 tests += $(TEST_DIR)/rdpru.flat
+tests += $(TEST_DIR)/cet.flat
 
 include $(SRCDIR)/$(TEST_DIR)/Makefile.common
 
diff --git a/x86/cet.c b/x86/cet.c
new file mode 100644
index 0000000..f0045ed
--- /dev/null
+++ b/x86/cet.c
@@ -0,0 +1,212 @@
+
+#include "libcflat.h"
+#include "x86/desc.h"
+#include "x86/processor.h"
+#include "x86/vm.h"
+#include "x86/msr.h"
+#include "vmalloc.h"
+#include "alloc_page.h"
+#include "fault_test.h"
+
+#define TEST_ARG(a, b, c, m, sf) {.usermode = m, \
+	.func =  (test_fault_func) NULL, \
+	.fault_vector = 21, .should_fault = sf, .arg = {a, b, c, 0}, \
+	.callback = NULL}
+
+#define CET_TEST(name, a, b, c, m, sf) FAULT_TEST(name, \
+		 TEST_ARG(a, b, c, m, sf))
+static unsigned long rbx, rsi, rdi, rsp, rbp, r8, r9,
+		     r10, r11, r12, r13, r14, r15;
+
+struct fault_test cet_tests[] = {
+	CET_TEST("CET SHSTK", 0, 0, 0, true, true),
+	CET_TEST("CET IBT", 0, 0, 0, true, true),
+	{ NULL },
+};
+
+struct fault_test *arg_user;
+static unsigned long expected_rip;
+static int cp_count;
+
+static u64 cet_shstk_func(u64 arg1, u64 arg2, u64 arg3, u64 arg4)
+{
+	unsigned long *ret_addr, *ssp;
+
+	asm volatile ("rdsspq %0" : "=r"(ssp));
+	asm("movq %%rbp,%0" : "=r"(ret_addr));
+	printf("The return-address in shadow-stack = 0x%lx, in normal stack = 0x%lx\n",
+	       *ssp, *(ret_addr + 1));
+
+	/*
+	 * In below line, it modifies the return address, it'll trigger #CP
+	 * while function is returning. The error-code is 0x1, meaning it's
+	 * caused by a near RET instruction, and the execution is terminated
+	 * when HW detects the violation.
+	 */
+	printf("Try to temper the return-address, this causes #CP on returning...\n");
+	*(ret_addr + 1) = 0xdeaddead;
+
+	return 0;
+}
+
+static u64 cet_ibt_func(u64 arg1, u64 arg2, u64 arg3, u64 arg4)
+{
+	/*
+	 * In below assembly code, the first instruction at lable 2 is not
+	 * endbr64, it'll trigger #CP with error code 0x3, and the execution
+	 * is terminated when HW detects the violation.
+	 */
+	printf("No endbr64 instruction at jmp target, this triggers #CP...\n");
+	asm volatile ("movq $2, %rcx\n"
+		      "dec %rcx\n"
+		      "leaq 2f, %rax\n"
+		      "jmp %rax \n"
+		      "2:\n"
+		      "dec %rcx\n");
+	return 0;
+}
+
+void do_cp_tss(unsigned long error_code);
+void do_cp_tss(unsigned long error_code)
+{
+	cp_count++;
+	printf("In #CP exception handler, error_code = 0x%lx\n", error_code);
+	asm("jmp %0" :: "m"(expected_rip));
+}
+
+extern void cp_tss(void);
+asm ("cp_tss: \t\n"
+     "push %rax; push %rcx; push %rdx; push %rsi; push %rdi\n"
+     "push %r8; push %r9; push %r10; push %r11\n"
+     "mov 9*8(%rsp),%rdi\n"
+     "call do_cp_tss \t\n"
+     "pop %r11;pop %r10; pop %r9; pop %r8\n"
+     "pop %rdi; pop %rsi; pop %rdx; pop %rcx; pop %rax\n"
+     "add $8, %rsp\t\n"
+     "iretq \t\n"
+     "jmp cp_tss");
+
+#define SAVE_REGS() \
+	asm ("movq %%rbx, %0\t\n"  \
+	     "movq %%rsi, %1\t\n"  \
+	     "movq %%rdi, %2\t\n"  \
+	     "movq %%rsp, %3\t\n"  \
+	     "movq %%rbp, %4\t\n"  \
+	     "movq %%r8, %5\t\n"   \
+	     "movq %%r9, %6\t\n"   \
+	     "movq %%r10, %7\t\n"  \
+	     "movq %%r11, %8\t\n"  \
+	     "movq %%r12, %9\t\n"  \
+	     "movq %%r13, %10\t\n" \
+	     "movq %%r14, %11\t\n" \
+	     "movq %%r15, %12\t\n" :: \
+	     "m"(rbx), "m"(rsi), "m"(rdi), "m"(rsp), "m"(rbp), \
+	     "m"(r8), "m"(r9), "m"(r10),  "m"(r11), "m"(r12),  \
+	     "m"(r13), "m"(r14), "m"(r15));
+
+#define RESTOR_REGS() \
+	asm ("movq %0, %%rbx\t\n"  \
+	     "movq %1, %%rsi\t\n"  \
+	     "movq %2, %%rdi\t\n"  \
+	     "movq %3, %%rsp\t\n"  \
+	     "movq %4, %%rbp\t\n"  \
+	     "movq %5, %%r8\t\n"   \
+	     "movq %6, %%r9\t\n"   \
+	     "movq %7, %%r10\t\n"  \
+	     "movq %8, %%r11\t\n"  \
+	     "movq %9, %%r12\t\n"  \
+	     "movq %10, %%r13\t\n" \
+	     "movq %11, %%r14\t\n" \
+	     "movq %12, %%r15\t\n" ::\
+	     "m"(rbx), "m"(rsi), "m"(rdi), "m"(rsp), "m"(rbp), \
+	     "m"(r8), "m"(r9), "m"(r10), "m"(r11), "m"(r12),   \
+	     "m"(r13), "m"(r14), "m"(r15));
+
+#define RUN_TEST() \
+	do {		\
+		SAVE_REGS();    \
+		asm volatile ("movq %0, %%rdi\t\n"        \
+			      "pushq %%rax\t\n"           \
+			      "leaq 1f(%%rip), %%rax\t\n" \
+			      "movq %%rax, %1\t\n"        \
+			      "popq %%rax\t\n"            \
+			      "call test_run\t\n"         \
+			      "1:" :: "r"(arg_user), "m"(expected_rip) : "rax", "rdi"); \
+		RESTOR_REGS(); \
+	} while (0)
+
+#define ENABLE_SHSTK_BIT 0x1
+#define ENABLE_IBT_BIT   0x4
+
+int main(int ac, char **av)
+{
+	char *shstk_virt;
+	unsigned long shstk_phys;
+	unsigned long *ptep;
+	pteval_t pte = 0;
+
+	cp_count = 0;
+	if (!this_cpu_has(X86_FEATURE_SHSTK)) {
+		printf("SHSTK not enabled\n");
+		return report_summary();
+	}
+
+	if (!this_cpu_has(X86_FEATURE_IBT)) {
+		printf("IBT not enabled\n");
+		return report_summary();
+	}
+
+	setup_vm();
+	setup_idt();
+	setup_alt_stack();
+	set_intr_alt_stack(21, cp_tss);
+
+	/* Allocate one page for shadow-stack. */
+	shstk_virt = alloc_vpage();
+	shstk_phys = (unsigned long)virt_to_phys(alloc_page());
+
+	/* Install the new page. */
+	pte = shstk_phys | PT_PRESENT_MASK | PT_WRITABLE_MASK | PT_USER_MASK;
+	install_pte(current_page_table(), 1, shstk_virt, pte, 0);
+	memset(shstk_virt, 0x0, PAGE_SIZE);
+
+	/* Mark it as shadow-stack page. */
+	ptep = get_pte_level(current_page_table(), shstk_virt, 1);
+	*ptep &= ~PT_WRITABLE_MASK;
+	*ptep |= PT_DIRTY_MASK;
+
+	/* Flush the paging cache. */
+	invlpg((void *)shstk_phys);
+
+	/* Enable shadow-stack protection */
+	wrmsr(MSR_IA32_U_CET, ENABLE_SHSTK_BIT);
+
+	/* Store shadow-stack pointer. */
+	wrmsr(MSR_IA32_PL3_SSP, (u64)(shstk_virt + 0x1000));
+
+	/* Enable CET master control bit in CR4. */
+	write_cr4(read_cr4() | X86_CR4_CET);
+
+	/* Do user-mode shadow-stack protection test.*/
+	cet_tests[0].arg.func = cet_shstk_func;
+	arg_user = &cet_tests[0];
+
+	RUN_TEST();
+	report(cp_count == 1, "Completed shadow-stack protection test successfully.");
+	cp_count = 0;
+
+	/* Do user-mode indirect-branch-tracking test.*/
+	cet_tests[1].arg.func = cet_ibt_func;
+	arg_user = &cet_tests[1];
+
+	/* Enable indirect-branch tracking */
+	wrmsr(MSR_IA32_U_CET, ENABLE_IBT_BIT);
+
+	RUN_TEST();
+	report(cp_count == 1, "Completed Indirect-branch tracking test successfully.");
+
+	write_cr4(read_cr4() & ~X86_CR4_CET);
+	wrmsr(MSR_IA32_U_CET, 0);
+
+	return report_summary();
+}
diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index f2401eb..87d412f 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -346,3 +346,9 @@ extra_params = -M q35,kernel-irqchip=split -device intel-iommu,intremap=on,eim=o
 file = tsx-ctrl.flat
 extra_params = -cpu host
 groups = tsx-ctrl
+
+[intel_cet]
+file = cet.flat
+arch = x86_64
+smp = 2
+extra_params = -enable-kvm -m 2048 -cpu host
-- 
2.17.2

Re: [PATCH v11 9/9] KVM: X86: Set CET feature bits for CPUID enumeration

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4506 bytes --]

Hi Yang,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on kvm/linux-next]
[also build test ERROR on next-20200326]
[cannot apply to vhost/linux-next tip/auto-latest linus/master linux/master v5.6-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Yang-Weijiang/Introduce-support-for-guest-CET-feature/20200327-040801
base:   https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next
config: x86_64-randconfig-g001-20200327 (attached as .config)
compiler: gcc-7 (Debian 7.5.0-5) 7.5.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/linux/kvm_host.h:36:0,
                    from arch/x86/kvm/emulate.c:21:
   arch/x86/kvm/emulate.c: In function 'check_cr_write':
>> arch/x86/include/asm/kvm_host.h:99:8: error: 'X86_CR4_CET' undeclared (first use in this function); did you mean 'X86_CR4_PCE'?
         | X86_CR4_CET))
           ^
   arch/x86/kvm/emulate.c:4206:3: note: in expansion of macro 'CR4_RESERVED_BITS'
      CR4_RESERVED_BITS,
      ^~~~~~~~~~~~~~~~~
   arch/x86/include/asm/kvm_host.h:99:8: note: each undeclared identifier is reported only once for each function it appears in
         | X86_CR4_CET))
           ^
   arch/x86/kvm/emulate.c:4206:3: note: in expansion of macro 'CR4_RESERVED_BITS'
      CR4_RESERVED_BITS,
      ^~~~~~~~~~~~~~~~~

vim +99 arch/x86/include/asm/kvm_host.h

    51	
    52	#define KVM_DIRTY_LOG_MANUAL_CAPS   (KVM_DIRTY_LOG_MANUAL_PROTECT_ENABLE | \
    53						KVM_DIRTY_LOG_INITIALLY_SET)
    54	
    55	/* x86-specific vcpu->requests bit members */
    56	#define KVM_REQ_MIGRATE_TIMER		KVM_ARCH_REQ(0)
    57	#define KVM_REQ_REPORT_TPR_ACCESS	KVM_ARCH_REQ(1)
    58	#define KVM_REQ_TRIPLE_FAULT		KVM_ARCH_REQ(2)
    59	#define KVM_REQ_MMU_SYNC		KVM_ARCH_REQ(3)
    60	#define KVM_REQ_CLOCK_UPDATE		KVM_ARCH_REQ(4)
    61	#define KVM_REQ_LOAD_MMU_PGD		KVM_ARCH_REQ(5)
    62	#define KVM_REQ_EVENT			KVM_ARCH_REQ(6)
    63	#define KVM_REQ_APF_HALT		KVM_ARCH_REQ(7)
    64	#define KVM_REQ_STEAL_UPDATE		KVM_ARCH_REQ(8)
    65	#define KVM_REQ_NMI			KVM_ARCH_REQ(9)
    66	#define KVM_REQ_PMU			KVM_ARCH_REQ(10)
    67	#define KVM_REQ_PMI			KVM_ARCH_REQ(11)
    68	#define KVM_REQ_SMI			KVM_ARCH_REQ(12)
    69	#define KVM_REQ_MASTERCLOCK_UPDATE	KVM_ARCH_REQ(13)
    70	#define KVM_REQ_MCLOCK_INPROGRESS \
    71		KVM_ARCH_REQ_FLAGS(14, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
    72	#define KVM_REQ_SCAN_IOAPIC \
    73		KVM_ARCH_REQ_FLAGS(15, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
    74	#define KVM_REQ_GLOBAL_CLOCK_UPDATE	KVM_ARCH_REQ(16)
    75	#define KVM_REQ_APIC_PAGE_RELOAD \
    76		KVM_ARCH_REQ_FLAGS(17, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
    77	#define KVM_REQ_HV_CRASH		KVM_ARCH_REQ(18)
    78	#define KVM_REQ_IOAPIC_EOI_EXIT		KVM_ARCH_REQ(19)
    79	#define KVM_REQ_HV_RESET		KVM_ARCH_REQ(20)
    80	#define KVM_REQ_HV_EXIT			KVM_ARCH_REQ(21)
    81	#define KVM_REQ_HV_STIMER		KVM_ARCH_REQ(22)
    82	#define KVM_REQ_LOAD_EOI_EXITMAP	KVM_ARCH_REQ(23)
    83	#define KVM_REQ_GET_VMCS12_PAGES	KVM_ARCH_REQ(24)
    84	#define KVM_REQ_APICV_UPDATE \
    85		KVM_ARCH_REQ_FLAGS(25, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
    86	
    87	#define CR0_RESERVED_BITS                                               \
    88		(~(unsigned long)(X86_CR0_PE | X86_CR0_MP | X86_CR0_EM | X86_CR0_TS \
    89				  | X86_CR0_ET | X86_CR0_NE | X86_CR0_WP | X86_CR0_AM \
    90				  | X86_CR0_NW | X86_CR0_CD | X86_CR0_PG))
    91	
    92	#define CR4_RESERVED_BITS                                               \
    93		(~(unsigned long)(X86_CR4_VME | X86_CR4_PVI | X86_CR4_TSD | X86_CR4_DE\
    94				  | X86_CR4_PSE | X86_CR4_PAE | X86_CR4_MCE     \
    95				  | X86_CR4_PGE | X86_CR4_PCE | X86_CR4_OSFXSR | X86_CR4_PCIDE \
    96				  | X86_CR4_OSXSAVE | X86_CR4_SMEP | X86_CR4_FSGSBASE \
    97				  | X86_CR4_OSXMMEXCPT | X86_CR4_LA57 | X86_CR4_VMXE \
    98				  | X86_CR4_SMAP | X86_CR4_PKE | X86_CR4_UMIP \
  > 99				  | X86_CR4_CET))
   100	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 39637 bytes --]

Re: [PATCH v11 7/9] KVM: X86: Add userspace access interface for CET MSRs

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 8989 bytes --]

Hi Yang,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on kvm/linux-next]
[also build test ERROR on next-20200327]
[cannot apply to vhost/linux-next tip/auto-latest linus/master linux/master v5.6-rc7]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Yang-Weijiang/Introduce-support-for-guest-CET-feature/20200327-040801
base:   https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next
config: x86_64-randconfig-g001-20200327 (attached as .config)
compiler: gcc-7 (Debian 7.5.0-5) 7.5.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   arch/x86/kvm/x86.c: In function 'kvm_set_cr0':
   arch/x86/kvm/x86.c:809:53: error: 'X86_CR4_CET' undeclared (first use in this function); did you mean 'X86_CR4_DE'?
     if (!(cr0 & X86_CR0_WP) && kvm_read_cr4_bits(vcpu, X86_CR4_CET))
                                                        ^~~~~~~~~~~
                                                        X86_CR4_DE
   arch/x86/kvm/x86.c:809:53: note: each undeclared identifier is reported only once for each function it appears in
   arch/x86/kvm/x86.c: At top level:
   arch/x86/kvm/x86.c:1233:16: error: 'MSR_IA32_U_CET' undeclared here (not in a function); did you mean 'MSR_IA32_TSC'?
     MSR_IA32_XSS, MSR_IA32_U_CET, MSR_IA32_S_CET,
                   ^~~~~~~~~~~~~~
                   MSR_IA32_TSC
   arch/x86/kvm/x86.c:1233:32: error: 'MSR_IA32_S_CET' undeclared here (not in a function); did you mean 'MSR_IA32_U_CET'?
     MSR_IA32_XSS, MSR_IA32_U_CET, MSR_IA32_S_CET,
                                   ^~~~~~~~~~~~~~
                                   MSR_IA32_U_CET
   arch/x86/kvm/x86.c:1234:2: error: 'MSR_IA32_PL0_SSP' undeclared here (not in a function); did you mean 'MSR_IA32_MCG_ESP'?
     MSR_IA32_PL0_SSP, MSR_IA32_PL1_SSP, MSR_IA32_PL2_SSP,
     ^~~~~~~~~~~~~~~~
     MSR_IA32_MCG_ESP
   arch/x86/kvm/x86.c:1234:20: error: 'MSR_IA32_PL1_SSP' undeclared here (not in a function); did you mean 'MSR_IA32_PL0_SSP'?
     MSR_IA32_PL0_SSP, MSR_IA32_PL1_SSP, MSR_IA32_PL2_SSP,
                       ^~~~~~~~~~~~~~~~
                       MSR_IA32_PL0_SSP
   arch/x86/kvm/x86.c:1234:38: error: 'MSR_IA32_PL2_SSP' undeclared here (not in a function); did you mean 'MSR_IA32_PL1_SSP'?
     MSR_IA32_PL0_SSP, MSR_IA32_PL1_SSP, MSR_IA32_PL2_SSP,
                                         ^~~~~~~~~~~~~~~~
                                         MSR_IA32_PL1_SSP
   arch/x86/kvm/x86.c:1235:2: error: 'MSR_IA32_PL3_SSP' undeclared here (not in a function); did you mean 'MSR_IA32_PL2_SSP'?
     MSR_IA32_PL3_SSP, MSR_IA32_INT_SSP_TAB,
     ^~~~~~~~~~~~~~~~
     MSR_IA32_PL2_SSP
>> arch/x86/kvm/x86.c:1235:20: error: 'MSR_IA32_INT_SSP_TAB' undeclared here (not in a function); did you mean 'MSR_IA32_PL3_SSP'?
     MSR_IA32_PL3_SSP, MSR_IA32_INT_SSP_TAB,
                       ^~~~~~~~~~~~~~~~~~~~
                       MSR_IA32_PL3_SSP
   arch/x86/kvm/x86.c: In function 'is_xsaves_msr':
   arch/x86/kvm/x86.c:3267:15: error: comparison between pointer and integer [-Werror]
     return index == MSR_IA32_U_CET ||
                  ^~
   arch/x86/kvm/x86.c:3268:16: error: comparison between pointer and integer [-Werror]
            (index >= MSR_IA32_PL0_SSP && index <= MSR_IA32_PL3_SSP);
                   ^~
   arch/x86/kvm/x86.c:3268:45: error: comparison between pointer and integer [-Werror]
            (index >= MSR_IA32_PL0_SSP && index <= MSR_IA32_PL3_SSP);
                                                ^~
   arch/x86/kvm/x86.c: In function 'kvm_arch_hardware_setup':
   arch/x86/kvm/x86.c:191:28: error: 'XFEATURE_MASK_CET_USER' undeclared (first use in this function); did you mean 'XFEATURE_MASK_BNDCSR'?
    #define KVM_SUPPORTED_XSS (XFEATURE_MASK_CET_USER | \
                               ^
   arch/x86/kvm/x86.c:9678:30: note: in expansion of macro 'KVM_SUPPORTED_XSS'
      supported_xss = host_xss & KVM_SUPPORTED_XSS;
                                 ^~~~~~~~~~~~~~~~~
   arch/x86/kvm/x86.c:192:6: error: 'XFEATURE_MASK_CET_KERNEL' undeclared (first use in this function); did you mean 'XFEATURE_MASK_CET_USER'?
         XFEATURE_MASK_CET_KERNEL)
         ^
   arch/x86/kvm/x86.c:9678:30: note: in expansion of macro 'KVM_SUPPORTED_XSS'
      supported_xss = host_xss & KVM_SUPPORTED_XSS;
                                 ^~~~~~~~~~~~~~~~~
   arch/x86/kvm/x86.c:191:51: error: invalid operands to binary | (have 'const u32 * {aka const unsigned int *}' and 'const u32 * {aka const unsigned int *}')
    #define KVM_SUPPORTED_XSS (XFEATURE_MASK_CET_USER | \
                               ~                      ^
   arch/x86/kvm/x86.c:9678:30: note: in expansion of macro 'KVM_SUPPORTED_XSS'
      supported_xss = host_xss & KVM_SUPPORTED_XSS;
                                 ^~~~~~~~~~~~~~~~~
   arch/x86/kvm/x86.c:9678:28: error: invalid operands to binary & (have 'u64 {aka long long unsigned int}' and 'const u32 * {aka const unsigned int *}')
      supported_xss = host_xss & KVM_SUPPORTED_XSS;
                               ^
   arch/x86/kvm/x86.c:9678:17: error: assignment makes integer from pointer without a cast [-Werror=int-conversion]
      supported_xss = host_xss & KVM_SUPPORTED_XSS;
                    ^
   cc1: all warnings being treated as errors

vim +1235 arch/x86/kvm/x86.c

  1180	
  1181	/*
  1182	 * List of msr numbers which we expose to userspace through KVM_GET_MSRS
  1183	 * and KVM_SET_MSRS, and KVM_GET_MSR_INDEX_LIST.
  1184	 *
  1185	 * The three MSR lists(msrs_to_save, emulated_msrs, msr_based_features)
  1186	 * extract the supported MSRs from the related const lists.
  1187	 * msrs_to_save is selected from the msrs_to_save_all to reflect the
  1188	 * capabilities of the host cpu. This capabilities test skips MSRs that are
  1189	 * kvm-specific. Those are put in emulated_msrs_all; filtering of emulated_msrs
  1190	 * may depend on host virtualization features rather than host cpu features.
  1191	 */
  1192	
  1193	static const u32 msrs_to_save_all[] = {
  1194		MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
  1195		MSR_STAR,
  1196	#ifdef CONFIG_X86_64
  1197		MSR_CSTAR, MSR_KERNEL_GS_BASE, MSR_SYSCALL_MASK, MSR_LSTAR,
  1198	#endif
  1199		MSR_IA32_TSC, MSR_IA32_CR_PAT, MSR_VM_HSAVE_PA,
  1200		MSR_IA32_FEAT_CTL, MSR_IA32_BNDCFGS, MSR_TSC_AUX,
  1201		MSR_IA32_SPEC_CTRL,
  1202		MSR_IA32_RTIT_CTL, MSR_IA32_RTIT_STATUS, MSR_IA32_RTIT_CR3_MATCH,
  1203		MSR_IA32_RTIT_OUTPUT_BASE, MSR_IA32_RTIT_OUTPUT_MASK,
  1204		MSR_IA32_RTIT_ADDR0_A, MSR_IA32_RTIT_ADDR0_B,
  1205		MSR_IA32_RTIT_ADDR1_A, MSR_IA32_RTIT_ADDR1_B,
  1206		MSR_IA32_RTIT_ADDR2_A, MSR_IA32_RTIT_ADDR2_B,
  1207		MSR_IA32_RTIT_ADDR3_A, MSR_IA32_RTIT_ADDR3_B,
  1208		MSR_IA32_UMWAIT_CONTROL,
  1209	
  1210		MSR_ARCH_PERFMON_FIXED_CTR0, MSR_ARCH_PERFMON_FIXED_CTR1,
  1211		MSR_ARCH_PERFMON_FIXED_CTR0 + 2, MSR_ARCH_PERFMON_FIXED_CTR0 + 3,
  1212		MSR_CORE_PERF_FIXED_CTR_CTRL, MSR_CORE_PERF_GLOBAL_STATUS,
  1213		MSR_CORE_PERF_GLOBAL_CTRL, MSR_CORE_PERF_GLOBAL_OVF_CTRL,
  1214		MSR_ARCH_PERFMON_PERFCTR0, MSR_ARCH_PERFMON_PERFCTR1,
  1215		MSR_ARCH_PERFMON_PERFCTR0 + 2, MSR_ARCH_PERFMON_PERFCTR0 + 3,
  1216		MSR_ARCH_PERFMON_PERFCTR0 + 4, MSR_ARCH_PERFMON_PERFCTR0 + 5,
  1217		MSR_ARCH_PERFMON_PERFCTR0 + 6, MSR_ARCH_PERFMON_PERFCTR0 + 7,
  1218		MSR_ARCH_PERFMON_PERFCTR0 + 8, MSR_ARCH_PERFMON_PERFCTR0 + 9,
  1219		MSR_ARCH_PERFMON_PERFCTR0 + 10, MSR_ARCH_PERFMON_PERFCTR0 + 11,
  1220		MSR_ARCH_PERFMON_PERFCTR0 + 12, MSR_ARCH_PERFMON_PERFCTR0 + 13,
  1221		MSR_ARCH_PERFMON_PERFCTR0 + 14, MSR_ARCH_PERFMON_PERFCTR0 + 15,
  1222		MSR_ARCH_PERFMON_PERFCTR0 + 16, MSR_ARCH_PERFMON_PERFCTR0 + 17,
  1223		MSR_ARCH_PERFMON_EVENTSEL0, MSR_ARCH_PERFMON_EVENTSEL1,
  1224		MSR_ARCH_PERFMON_EVENTSEL0 + 2, MSR_ARCH_PERFMON_EVENTSEL0 + 3,
  1225		MSR_ARCH_PERFMON_EVENTSEL0 + 4, MSR_ARCH_PERFMON_EVENTSEL0 + 5,
  1226		MSR_ARCH_PERFMON_EVENTSEL0 + 6, MSR_ARCH_PERFMON_EVENTSEL0 + 7,
  1227		MSR_ARCH_PERFMON_EVENTSEL0 + 8, MSR_ARCH_PERFMON_EVENTSEL0 + 9,
  1228		MSR_ARCH_PERFMON_EVENTSEL0 + 10, MSR_ARCH_PERFMON_EVENTSEL0 + 11,
  1229		MSR_ARCH_PERFMON_EVENTSEL0 + 12, MSR_ARCH_PERFMON_EVENTSEL0 + 13,
  1230		MSR_ARCH_PERFMON_EVENTSEL0 + 14, MSR_ARCH_PERFMON_EVENTSEL0 + 15,
  1231		MSR_ARCH_PERFMON_EVENTSEL0 + 16, MSR_ARCH_PERFMON_EVENTSEL0 + 17,
  1232	
  1233		MSR_IA32_XSS, MSR_IA32_U_CET, MSR_IA32_S_CET,
  1234		MSR_IA32_PL0_SSP, MSR_IA32_PL1_SSP, MSR_IA32_PL2_SSP,
> 1235		MSR_IA32_PL3_SSP, MSR_IA32_INT_SSP_TAB,
  1236	};
  1237	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 39637 bytes --]

Re: [PATCH v11 3/9] KVM: VMX: Set host/guest CET states for vmexit/vmentry

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 1821 bytes --]

Hi Yang,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on kvm/linux-next]
[also build test ERROR on next-20200331]
[cannot apply to vhost/linux-next tip/auto-latest linus/master linux/master v5.6]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Yang-Weijiang/Introduce-support-for-guest-CET-feature/20200327-040801
base:   https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-6) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> arch/x86/kvm/vmx/vmx.c:47:10: fatal error: asm/cet.h: No such file or directory
    #include <asm/cet.h>
             ^~~~~~~~~~~
   compilation terminated.

vim +47 arch/x86/kvm/vmx/vmx.c

    30	
    31	#include <asm/apic.h>
    32	#include <asm/asm.h>
    33	#include <asm/cpu.h>
    34	#include <asm/debugreg.h>
    35	#include <asm/desc.h>
    36	#include <asm/fpu/internal.h>
    37	#include <asm/io.h>
    38	#include <asm/irq_remapping.h>
    39	#include <asm/kexec.h>
    40	#include <asm/perf_event.h>
    41	#include <asm/mce.h>
    42	#include <asm/mmu_context.h>
    43	#include <asm/mshyperv.h>
    44	#include <asm/spec-ctrl.h>
    45	#include <asm/virtext.h>
    46	#include <asm/vmx.h>
  > 47	#include <asm/cet.h>
    48	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 71686 bytes --]

Re: [PATCH v11 5/9] KVM: X86: Refresh CPUID once guest XSS MSR changes

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3444 bytes --]

Hi Yang,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on kvm/linux-next]
[also build test ERROR on next-20200331]
[cannot apply to vhost/linux-next tip/auto-latest linus/master linux/master v5.6]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Yang-Weijiang/Introduce-support-for-guest-CET-feature/20200327-040801
base:   https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-6) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   arch/x86/kvm/x86.c: In function 'kvm_set_cr0':
   arch/x86/kvm/x86.c:807:53: error: 'X86_CR4_CET' undeclared (first use in this function); did you mean 'X86_CR4_DE'?
     if (!(cr0 & X86_CR0_WP) && kvm_read_cr4_bits(vcpu, X86_CR4_CET))
                                                        ^~~~~~~~~~~
                                                        X86_CR4_DE
   arch/x86/kvm/x86.c:807:53: note: each undeclared identifier is reported only once for each function it appears in
   arch/x86/kvm/x86.c: In function 'kvm_arch_hardware_setup':
>> arch/x86/kvm/x86.c:189:28: error: 'XFEATURE_MASK_CET_USER' undeclared (first use in this function); did you mean 'XFEATURE_MASK_BNDCSR'?
    #define KVM_SUPPORTED_XSS (XFEATURE_MASK_CET_USER | \
                               ^
   arch/x86/kvm/x86.c:9650:30: note: in expansion of macro 'KVM_SUPPORTED_XSS'
      supported_xss = host_xss & KVM_SUPPORTED_XSS;
                                 ^~~~~~~~~~~~~~~~~
>> arch/x86/kvm/x86.c:190:6: error: 'XFEATURE_MASK_CET_KERNEL' undeclared (first use in this function); did you mean 'XFEATURE_MASK_CET_USER'?
         XFEATURE_MASK_CET_KERNEL)
         ^
   arch/x86/kvm/x86.c:9650:30: note: in expansion of macro 'KVM_SUPPORTED_XSS'
      supported_xss = host_xss & KVM_SUPPORTED_XSS;
                                 ^~~~~~~~~~~~~~~~~

vim +189 arch/x86/kvm/x86.c

18863bdd60f895 Avi Kivity          2009-09-07  183  
cfc481810c903a Sean Christopherson 2020-03-02  184  #define KVM_SUPPORTED_XCR0     (XFEATURE_MASK_FP | XFEATURE_MASK_SSE \
cfc481810c903a Sean Christopherson 2020-03-02  185  				| XFEATURE_MASK_YMM | XFEATURE_MASK_BNDREGS \
cfc481810c903a Sean Christopherson 2020-03-02  186  				| XFEATURE_MASK_BNDCSR | XFEATURE_MASK_AVX512 \
cfc481810c903a Sean Christopherson 2020-03-02  187  				| XFEATURE_MASK_PKRU)
cfc481810c903a Sean Christopherson 2020-03-02  188  
aae536c20cb181 Yang Weijiang       2020-03-26 @189  #define KVM_SUPPORTED_XSS	(XFEATURE_MASK_CET_USER | \
aae536c20cb181 Yang Weijiang       2020-03-26 @190  				 XFEATURE_MASK_CET_KERNEL)
aae536c20cb181 Yang Weijiang       2020-03-26  191  

:::::: The code at line 189 was first introduced by commit
:::::: aae536c20cb18180995aae27b30973f60e148ee7 KVM: VMX: Introduce CET VMX fields and flags

:::::: TO: Yang Weijiang <weijiang.yang@intel.com>
:::::: CC: 0day robot <lkp@intel.com>

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 71686 bytes --]

Re: [PATCH v11 7/9] KVM: X86: Add userspace access interface for CET MSRs

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 8969 bytes --]

Hi Yang,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on kvm/linux-next]
[also build test ERROR on next-20200331]
[cannot apply to vhost/linux-next tip/auto-latest linus/master linux/master v5.6]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Yang-Weijiang/Introduce-support-for-guest-CET-feature/20200327-040801
base:   https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-6) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   arch/x86/kvm/x86.c: In function 'kvm_set_cr0':
   arch/x86/kvm/x86.c:809:53: error: 'X86_CR4_CET' undeclared (first use in this function); did you mean 'X86_CR4_DE'?
     if (!(cr0 & X86_CR0_WP) && kvm_read_cr4_bits(vcpu, X86_CR4_CET))
                                                        ^~~~~~~~~~~
                                                        X86_CR4_DE
   arch/x86/kvm/x86.c:809:53: note: each undeclared identifier is reported only once for each function it appears in
   arch/x86/kvm/x86.c: At top level:
>> arch/x86/kvm/x86.c:1233:16: error: 'MSR_IA32_U_CET' undeclared here (not in a function); did you mean 'MSR_IA32_TSC'?
     MSR_IA32_XSS, MSR_IA32_U_CET, MSR_IA32_S_CET,
                   ^~~~~~~~~~~~~~
                   MSR_IA32_TSC
>> arch/x86/kvm/x86.c:1233:32: error: 'MSR_IA32_S_CET' undeclared here (not in a function); did you mean 'MSR_IA32_U_CET'?
     MSR_IA32_XSS, MSR_IA32_U_CET, MSR_IA32_S_CET,
                                   ^~~~~~~~~~~~~~
                                   MSR_IA32_U_CET
>> arch/x86/kvm/x86.c:1234:2: error: 'MSR_IA32_PL0_SSP' undeclared here (not in a function); did you mean 'MSR_IA32_MCG_ESP'?
     MSR_IA32_PL0_SSP, MSR_IA32_PL1_SSP, MSR_IA32_PL2_SSP,
     ^~~~~~~~~~~~~~~~
     MSR_IA32_MCG_ESP
>> arch/x86/kvm/x86.c:1234:20: error: 'MSR_IA32_PL1_SSP' undeclared here (not in a function); did you mean 'MSR_IA32_PL0_SSP'?
     MSR_IA32_PL0_SSP, MSR_IA32_PL1_SSP, MSR_IA32_PL2_SSP,
                       ^~~~~~~~~~~~~~~~
                       MSR_IA32_PL0_SSP
>> arch/x86/kvm/x86.c:1234:38: error: 'MSR_IA32_PL2_SSP' undeclared here (not in a function); did you mean 'MSR_IA32_PL1_SSP'?
     MSR_IA32_PL0_SSP, MSR_IA32_PL1_SSP, MSR_IA32_PL2_SSP,
                                         ^~~~~~~~~~~~~~~~
                                         MSR_IA32_PL1_SSP
>> arch/x86/kvm/x86.c:1235:2: error: 'MSR_IA32_PL3_SSP' undeclared here (not in a function); did you mean 'MSR_IA32_PL2_SSP'?
     MSR_IA32_PL3_SSP, MSR_IA32_INT_SSP_TAB,
     ^~~~~~~~~~~~~~~~
     MSR_IA32_PL2_SSP
   arch/x86/kvm/x86.c:1235:20: error: 'MSR_IA32_INT_SSP_TAB' undeclared here (not in a function); did you mean 'MSR_IA32_PL3_SSP'?
     MSR_IA32_PL3_SSP, MSR_IA32_INT_SSP_TAB,
                       ^~~~~~~~~~~~~~~~~~~~
                       MSR_IA32_PL3_SSP
   arch/x86/kvm/x86.c: In function 'is_xsaves_msr':
   arch/x86/kvm/x86.c:3267:15: error: comparison between pointer and integer [-Werror]
     return index == MSR_IA32_U_CET ||
                  ^~
   arch/x86/kvm/x86.c:3268:16: error: comparison between pointer and integer [-Werror]
            (index >= MSR_IA32_PL0_SSP && index <= MSR_IA32_PL3_SSP);
                   ^~
   arch/x86/kvm/x86.c:3268:45: error: comparison between pointer and integer [-Werror]
            (index >= MSR_IA32_PL0_SSP && index <= MSR_IA32_PL3_SSP);
                                                ^~
   arch/x86/kvm/x86.c: In function 'kvm_arch_hardware_setup':
   arch/x86/kvm/x86.c:191:28: error: 'XFEATURE_MASK_CET_USER' undeclared (first use in this function); did you mean 'XFEATURE_MASK_BNDCSR'?
    #define KVM_SUPPORTED_XSS (XFEATURE_MASK_CET_USER | \
                               ^
   arch/x86/kvm/x86.c:9678:30: note: in expansion of macro 'KVM_SUPPORTED_XSS'
      supported_xss = host_xss & KVM_SUPPORTED_XSS;
                                 ^~~~~~~~~~~~~~~~~
   arch/x86/kvm/x86.c:192:6: error: 'XFEATURE_MASK_CET_KERNEL' undeclared (first use in this function); did you mean 'XFEATURE_MASK_CET_USER'?
         XFEATURE_MASK_CET_KERNEL)
         ^
   arch/x86/kvm/x86.c:9678:30: note: in expansion of macro 'KVM_SUPPORTED_XSS'
      supported_xss = host_xss & KVM_SUPPORTED_XSS;
                                 ^~~~~~~~~~~~~~~~~
>> arch/x86/kvm/x86.c:191:51: error: invalid operands to binary | (have 'const u32 * {aka const unsigned int *}' and 'const u32 * {aka const unsigned int *}')
    #define KVM_SUPPORTED_XSS (XFEATURE_MASK_CET_USER | \
                               ~                      ^
   arch/x86/kvm/x86.c:9678:30: note: in expansion of macro 'KVM_SUPPORTED_XSS'
      supported_xss = host_xss & KVM_SUPPORTED_XSS;
                                 ^~~~~~~~~~~~~~~~~
>> arch/x86/kvm/x86.c:9678:28: error: invalid operands to binary & (have 'u64 {aka long long unsigned int}' and 'const u32 * {aka const unsigned int *}')
      supported_xss = host_xss & KVM_SUPPORTED_XSS;
                               ^
   arch/x86/kvm/x86.c:9678:17: error: assignment makes integer from pointer without a cast [-Werror=int-conversion]
      supported_xss = host_xss & KVM_SUPPORTED_XSS;
                    ^
   cc1: all warnings being treated as errors

vim +1233 arch/x86/kvm/x86.c

  1180	
  1181	/*
  1182	 * List of msr numbers which we expose to userspace through KVM_GET_MSRS
  1183	 * and KVM_SET_MSRS, and KVM_GET_MSR_INDEX_LIST.
  1184	 *
  1185	 * The three MSR lists(msrs_to_save, emulated_msrs, msr_based_features)
  1186	 * extract the supported MSRs from the related const lists.
  1187	 * msrs_to_save is selected from the msrs_to_save_all to reflect the
  1188	 * capabilities of the host cpu. This capabilities test skips MSRs that are
  1189	 * kvm-specific. Those are put in emulated_msrs_all; filtering of emulated_msrs
  1190	 * may depend on host virtualization features rather than host cpu features.
  1191	 */
  1192	
  1193	static const u32 msrs_to_save_all[] = {
  1194		MSR_IA32_SYSENTER_CS, MSR_IA32_SYSENTER_ESP, MSR_IA32_SYSENTER_EIP,
  1195		MSR_STAR,
  1196	#ifdef CONFIG_X86_64
  1197		MSR_CSTAR, MSR_KERNEL_GS_BASE, MSR_SYSCALL_MASK, MSR_LSTAR,
  1198	#endif
  1199		MSR_IA32_TSC, MSR_IA32_CR_PAT, MSR_VM_HSAVE_PA,
  1200		MSR_IA32_FEAT_CTL, MSR_IA32_BNDCFGS, MSR_TSC_AUX,
  1201		MSR_IA32_SPEC_CTRL,
  1202		MSR_IA32_RTIT_CTL, MSR_IA32_RTIT_STATUS, MSR_IA32_RTIT_CR3_MATCH,
  1203		MSR_IA32_RTIT_OUTPUT_BASE, MSR_IA32_RTIT_OUTPUT_MASK,
  1204		MSR_IA32_RTIT_ADDR0_A, MSR_IA32_RTIT_ADDR0_B,
  1205		MSR_IA32_RTIT_ADDR1_A, MSR_IA32_RTIT_ADDR1_B,
  1206		MSR_IA32_RTIT_ADDR2_A, MSR_IA32_RTIT_ADDR2_B,
  1207		MSR_IA32_RTIT_ADDR3_A, MSR_IA32_RTIT_ADDR3_B,
  1208		MSR_IA32_UMWAIT_CONTROL,
  1209	
  1210		MSR_ARCH_PERFMON_FIXED_CTR0, MSR_ARCH_PERFMON_FIXED_CTR1,
  1211		MSR_ARCH_PERFMON_FIXED_CTR0 + 2, MSR_ARCH_PERFMON_FIXED_CTR0 + 3,
  1212		MSR_CORE_PERF_FIXED_CTR_CTRL, MSR_CORE_PERF_GLOBAL_STATUS,
  1213		MSR_CORE_PERF_GLOBAL_CTRL, MSR_CORE_PERF_GLOBAL_OVF_CTRL,
  1214		MSR_ARCH_PERFMON_PERFCTR0, MSR_ARCH_PERFMON_PERFCTR1,
  1215		MSR_ARCH_PERFMON_PERFCTR0 + 2, MSR_ARCH_PERFMON_PERFCTR0 + 3,
  1216		MSR_ARCH_PERFMON_PERFCTR0 + 4, MSR_ARCH_PERFMON_PERFCTR0 + 5,
  1217		MSR_ARCH_PERFMON_PERFCTR0 + 6, MSR_ARCH_PERFMON_PERFCTR0 + 7,
  1218		MSR_ARCH_PERFMON_PERFCTR0 + 8, MSR_ARCH_PERFMON_PERFCTR0 + 9,
  1219		MSR_ARCH_PERFMON_PERFCTR0 + 10, MSR_ARCH_PERFMON_PERFCTR0 + 11,
  1220		MSR_ARCH_PERFMON_PERFCTR0 + 12, MSR_ARCH_PERFMON_PERFCTR0 + 13,
  1221		MSR_ARCH_PERFMON_PERFCTR0 + 14, MSR_ARCH_PERFMON_PERFCTR0 + 15,
  1222		MSR_ARCH_PERFMON_PERFCTR0 + 16, MSR_ARCH_PERFMON_PERFCTR0 + 17,
  1223		MSR_ARCH_PERFMON_EVENTSEL0, MSR_ARCH_PERFMON_EVENTSEL1,
  1224		MSR_ARCH_PERFMON_EVENTSEL0 + 2, MSR_ARCH_PERFMON_EVENTSEL0 + 3,
  1225		MSR_ARCH_PERFMON_EVENTSEL0 + 4, MSR_ARCH_PERFMON_EVENTSEL0 + 5,
  1226		MSR_ARCH_PERFMON_EVENTSEL0 + 6, MSR_ARCH_PERFMON_EVENTSEL0 + 7,
  1227		MSR_ARCH_PERFMON_EVENTSEL0 + 8, MSR_ARCH_PERFMON_EVENTSEL0 + 9,
  1228		MSR_ARCH_PERFMON_EVENTSEL0 + 10, MSR_ARCH_PERFMON_EVENTSEL0 + 11,
  1229		MSR_ARCH_PERFMON_EVENTSEL0 + 12, MSR_ARCH_PERFMON_EVENTSEL0 + 13,
  1230		MSR_ARCH_PERFMON_EVENTSEL0 + 14, MSR_ARCH_PERFMON_EVENTSEL0 + 15,
  1231		MSR_ARCH_PERFMON_EVENTSEL0 + 16, MSR_ARCH_PERFMON_EVENTSEL0 + 17,
  1232	
> 1233		MSR_IA32_XSS, MSR_IA32_U_CET, MSR_IA32_S_CET,
> 1234		MSR_IA32_PL0_SSP, MSR_IA32_PL1_SSP, MSR_IA32_PL2_SSP,
> 1235		MSR_IA32_PL3_SSP, MSR_IA32_INT_SSP_TAB,
  1236	};
  1237	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 71686 bytes --]

Re: [PATCH v11 8/9] KVM: VMX: Enable CET support for nested VM

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 9029 bytes --]

Hi Yang,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on kvm/linux-next]
[also build test ERROR on next-20200331]
[cannot apply to vhost/linux-next tip/auto-latest linus/master linux/master v5.6]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Yang-Weijiang/Introduce-support-for-guest-CET-feature/20200327-040801
base:   https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.4.0-6) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   arch/x86/kvm/vmx/nested.c: In function 'nested_vmx_prepare_msr_bitmap':
>> arch/x86/kvm/vmx/nested.c:630:39: error: 'MSR_IA32_U_CET' undeclared (first use in this function); did you mean 'MSR_IA32_TSC'?
     if (!msr_write_intercepted_l01(vcpu, MSR_IA32_U_CET))
                                          ^~~~~~~~~~~~~~
                                          MSR_IA32_TSC
   arch/x86/kvm/vmx/nested.c:630:39: note: each undeclared identifier is reported only once for each function it appears in
>> arch/x86/kvm/vmx/nested.c:635:39: error: 'MSR_IA32_PL3_SSP' undeclared (first use in this function); did you mean 'MSR_IA32_MCG_ESP'?
     if (!msr_write_intercepted_l01(vcpu, MSR_IA32_PL3_SSP))
                                          ^~~~~~~~~~~~~~~~
                                          MSR_IA32_MCG_ESP
>> arch/x86/kvm/vmx/nested.c:640:39: error: 'MSR_IA32_S_CET' undeclared (first use in this function); did you mean 'MSR_IA32_U_CET'?
     if (!msr_write_intercepted_l01(vcpu, MSR_IA32_S_CET))
                                          ^~~~~~~~~~~~~~
                                          MSR_IA32_U_CET
>> arch/x86/kvm/vmx/nested.c:645:39: error: 'MSR_IA32_PL0_SSP' undeclared (first use in this function); did you mean 'MSR_IA32_PL3_SSP'?
     if (!msr_write_intercepted_l01(vcpu, MSR_IA32_PL0_SSP))
                                          ^~~~~~~~~~~~~~~~
                                          MSR_IA32_PL3_SSP
>> arch/x86/kvm/vmx/nested.c:650:39: error: 'MSR_IA32_PL1_SSP' undeclared (first use in this function); did you mean 'MSR_IA32_PL0_SSP'?
     if (!msr_write_intercepted_l01(vcpu, MSR_IA32_PL1_SSP))
                                          ^~~~~~~~~~~~~~~~
                                          MSR_IA32_PL0_SSP
>> arch/x86/kvm/vmx/nested.c:655:39: error: 'MSR_IA32_PL2_SSP' undeclared (first use in this function); did you mean 'MSR_IA32_PL1_SSP'?
     if (!msr_write_intercepted_l01(vcpu, MSR_IA32_PL2_SSP))
                                          ^~~~~~~~~~~~~~~~
                                          MSR_IA32_PL1_SSP
>> arch/x86/kvm/vmx/nested.c:660:39: error: 'MSR_IA32_INT_SSP_TAB' undeclared (first use in this function); did you mean 'MSR_IA32_PL2_SSP'?
     if (!msr_write_intercepted_l01(vcpu, MSR_IA32_INT_SSP_TAB))
                                          ^~~~~~~~~~~~~~~~~~~~
                                          MSR_IA32_PL2_SSP

vim +630 arch/x86/kvm/vmx/nested.c

   557	
   558	/*
   559	 * Merge L0's and L1's MSR bitmap, return false to indicate that
   560	 * we do not use the hardware.
   561	 */
   562	static inline bool nested_vmx_prepare_msr_bitmap(struct kvm_vcpu *vcpu,
   563							 struct vmcs12 *vmcs12)
   564	{
   565		int msr;
   566		unsigned long *msr_bitmap_l1;
   567		unsigned long *msr_bitmap_l0 = to_vmx(vcpu)->nested.vmcs02.msr_bitmap;
   568		struct kvm_host_map *map = &to_vmx(vcpu)->nested.msr_bitmap_map;
   569	
   570		/* Nothing to do if the MSR bitmap is not in use.  */
   571		if (!cpu_has_vmx_msr_bitmap() ||
   572		    !nested_cpu_has(vmcs12, CPU_BASED_USE_MSR_BITMAPS))
   573			return false;
   574	
   575		if (kvm_vcpu_map(vcpu, gpa_to_gfn(vmcs12->msr_bitmap), map))
   576			return false;
   577	
   578		msr_bitmap_l1 = (unsigned long *)map->hva;
   579	
   580		/*
   581		 * To keep the control flow simple, pay eight 8-byte writes (sixteen
   582		 * 4-byte writes on 32-bit systems) up front to enable intercepts for
   583		 * the x2APIC MSR range and selectively disable them below.
   584		 */
   585		enable_x2apic_msr_intercepts(msr_bitmap_l0);
   586	
   587		if (nested_cpu_has_virt_x2apic_mode(vmcs12)) {
   588			if (nested_cpu_has_apic_reg_virt(vmcs12)) {
   589				/*
   590				 * L0 need not intercept reads for MSRs between 0x800
   591				 * and 0x8ff, it just lets the processor take the value
   592				 * from the virtual-APIC page; take those 256 bits
   593				 * directly from the L1 bitmap.
   594				 */
   595				for (msr = 0x800; msr <= 0x8ff; msr += BITS_PER_LONG) {
   596					unsigned word = msr / BITS_PER_LONG;
   597	
   598					msr_bitmap_l0[word] = msr_bitmap_l1[word];
   599				}
   600			}
   601	
   602			nested_vmx_disable_intercept_for_msr(
   603				msr_bitmap_l1, msr_bitmap_l0,
   604				X2APIC_MSR(APIC_TASKPRI),
   605				MSR_TYPE_R | MSR_TYPE_W);
   606	
   607			if (nested_cpu_has_vid(vmcs12)) {
   608				nested_vmx_disable_intercept_for_msr(
   609					msr_bitmap_l1, msr_bitmap_l0,
   610					X2APIC_MSR(APIC_EOI),
   611					MSR_TYPE_W);
   612				nested_vmx_disable_intercept_for_msr(
   613					msr_bitmap_l1, msr_bitmap_l0,
   614					X2APIC_MSR(APIC_SELF_IPI),
   615					MSR_TYPE_W);
   616			}
   617		}
   618	
   619		/* KVM unconditionally exposes the FS/GS base MSRs to L1. */
   620		nested_vmx_disable_intercept_for_msr(msr_bitmap_l1, msr_bitmap_l0,
   621						     MSR_FS_BASE, MSR_TYPE_RW);
   622	
   623		nested_vmx_disable_intercept_for_msr(msr_bitmap_l1, msr_bitmap_l0,
   624						     MSR_GS_BASE, MSR_TYPE_RW);
   625	
   626		nested_vmx_disable_intercept_for_msr(msr_bitmap_l1, msr_bitmap_l0,
   627						     MSR_KERNEL_GS_BASE, MSR_TYPE_RW);
   628	
   629		/* Pass CET MSRs to nested VM if L0 and L1 are set to pass-through. */
 > 630		if (!msr_write_intercepted_l01(vcpu, MSR_IA32_U_CET))
   631			nested_vmx_disable_intercept_for_msr(
   632						msr_bitmap_l1, msr_bitmap_l0,
   633						MSR_IA32_U_CET, MSR_TYPE_RW);
   634	
 > 635		if (!msr_write_intercepted_l01(vcpu, MSR_IA32_PL3_SSP))
   636			nested_vmx_disable_intercept_for_msr(
   637						msr_bitmap_l1, msr_bitmap_l0,
   638						MSR_IA32_PL3_SSP, MSR_TYPE_RW);
   639	
 > 640		if (!msr_write_intercepted_l01(vcpu, MSR_IA32_S_CET))
   641			nested_vmx_disable_intercept_for_msr(
   642						msr_bitmap_l1, msr_bitmap_l0,
   643						MSR_IA32_S_CET, MSR_TYPE_RW);
   644	
 > 645		if (!msr_write_intercepted_l01(vcpu, MSR_IA32_PL0_SSP))
   646			nested_vmx_disable_intercept_for_msr(
   647						msr_bitmap_l1, msr_bitmap_l0,
   648						MSR_IA32_PL0_SSP, MSR_TYPE_RW);
   649	
 > 650		if (!msr_write_intercepted_l01(vcpu, MSR_IA32_PL1_SSP))
   651			nested_vmx_disable_intercept_for_msr(
   652						msr_bitmap_l1, msr_bitmap_l0,
   653						MSR_IA32_PL1_SSP, MSR_TYPE_RW);
   654	
 > 655		if (!msr_write_intercepted_l01(vcpu, MSR_IA32_PL2_SSP))
   656			nested_vmx_disable_intercept_for_msr(
   657						msr_bitmap_l1, msr_bitmap_l0,
   658						MSR_IA32_PL2_SSP, MSR_TYPE_RW);
   659	
 > 660		if (!msr_write_intercepted_l01(vcpu, MSR_IA32_INT_SSP_TAB))
   661			nested_vmx_disable_intercept_for_msr(
   662						msr_bitmap_l1, msr_bitmap_l0,
   663						MSR_IA32_INT_SSP_TAB, MSR_TYPE_RW);
   664		/*
   665		 * Checking the L0->L1 bitmap is trying to verify two things:
   666		 *
   667		 * 1. L0 gave a permission to L1 to actually passthrough the MSR. This
   668		 *    ensures that we do not accidentally generate an L02 MSR bitmap
   669		 *    from the L12 MSR bitmap that is too permissive.
   670		 * 2. That L1 or L2s have actually used the MSR. This avoids
   671		 *    unnecessarily merging of the bitmap if the MSR is unused. This
   672		 *    works properly because we only update the L01 MSR bitmap lazily.
   673		 *    So even if L0 should pass L1 these MSRs, the L01 bitmap is only
   674		 *    updated to reflect this when L1 (or its L2s) actually write to
   675		 *    the MSR.
   676		 */
   677		if (!msr_write_intercepted_l01(vcpu, MSR_IA32_SPEC_CTRL))
   678			nested_vmx_disable_intercept_for_msr(
   679						msr_bitmap_l1, msr_bitmap_l0,
   680						MSR_IA32_SPEC_CTRL,
   681						MSR_TYPE_R | MSR_TYPE_W);
   682	
   683		if (!msr_write_intercepted_l01(vcpu, MSR_IA32_PRED_CMD))
   684			nested_vmx_disable_intercept_for_msr(
   685						msr_bitmap_l1, msr_bitmap_l0,
   686						MSR_IA32_PRED_CMD,
   687						MSR_TYPE_W);
   688	
   689		kvm_vcpu_unmap(vcpu, &to_vmx(vcpu)->nested.msr_bitmap_map, false);
   690	
   691		return true;
   692	}
   693	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 71686 bytes --]

Re: [PATCH v11 0/9] Introduce support for guest CET feature

[Sean Christopherson]

On Thu, Mar 26, 2020 at 04:18:37PM +0800, Yang Weijiang wrote:
> Control-flow Enforcement Technology (CET) provides protection against
> Return/Jump-Oriented Programming (ROP/JOP) attack. It includes two
> sub-features: Shadow Stack (SHSTK) and Indirect Branch Tracking (IBT).
> 
> KVM needs to update to enable guest CET feature.
> This patchset implements CET related CPUID/XSAVES enumeration, MSRs
> and vmentry/vmexit configuration etc.so that guest kernel can setup CET
> runtime infrastructure based on them. Some CET MSRs and related feature
> flags used reference the definitions in kernel patchset.
> 
> CET kernel patches are here:
> https://lkml.org/lkml/2020/2/5/593
> https://lkml.org/lkml/2020/2/5/604

lkml.org is pretty worthless for this sort of thing, and lkml.kernel.org
is the preferred link method in general.  The syntax is

  https://lkml.kernel.org/r/<Message-ID>

e.g.

  https://lkml.kernel.org/r/20200205181935.3712-1-yu-cheng.yu@intel.com

Note, that will redirect to lore.kernel.org, but the above format is
preferred because it isn't dependent on binning the thread to a specific
mailing list.

Anyways, kernel.org provides a link to download the entire thread in mbox
format, which allows reviewers to get the prerequisite series without much
fuss.

  https://lore.kernel.org/linux-api/20200205181935.3712-1-yu-cheng.yu@intel.com/t.mbox.gz

Re: [PATCH v11 0/9] Introduce support for guest CET feature

[Sean Christopherson]

On Thu, Mar 26, 2020 at 04:18:37PM +0800, Yang Weijiang wrote:
> Control-flow Enforcement Technology (CET) provides protection against
> Return/Jump-Oriented Programming (ROP/JOP) attack. It includes two
> sub-features: Shadow Stack (SHSTK) and Indirect Branch Tracking (IBT).
> 
> KVM needs to update to enable guest CET feature.
> This patchset implements CET related CPUID/XSAVES enumeration, MSRs
> and vmentry/vmexit configuration etc.so that guest kernel can setup CET
> runtime infrastructure based on them. Some CET MSRs and related feature
> flags used reference the definitions in kernel patchset.
> 
> CET kernel patches are here:
> https://lkml.org/lkml/2020/2/5/593
> https://lkml.org/lkml/2020/2/5/604

...

> - This patch serial is built on top of below branch and CET kernel patches
>   for seeking xsaves support:
>   https://git.kernel.org/pub/scm/virt/kvm/kvm.git/log/?h=cpu-caps

Can you provide the full code in a branch/tag somewhere?  The CET patches
are in turn dependent on XSAVES enabling[*], and those don't apply cleanly
on the cpu-caps branch.

It might make sense to also rebase to kvm/queue?  Though that's not a
requirement by any means, e.g. don't bother if the CET patches are going to
be respun soon.

https://lkml.kernel.org/r/20200328164307.17497-1-yu-cheng.yu@intel.com

Re: [PATCH v11 1/9] KVM: VMX: Introduce CET VMX fields and flags

[Sean Christopherson]

On Thu, Mar 26, 2020 at 04:18:38PM +0800, Yang Weijiang wrote:
> CET(Control-flow Enforcement Technology) is a CPU feature
> used to prevent Return/Jump-Oriented Programming(ROP/JOP)
> attacks. It provides the following sub-features to defend
> against ROP/JOP style control-flow subversion attacks:

Changelogs should wrap at 75 characters.  Wrapping slightly earlier is ok,
but wrapping at ~60 chars is too narrow.

> Shadow Stack (SHSTK):
>   A second stack for program which is used exclusively for
>   control transfer operations.
> 
> Indirect Branch Tracking (IBT):
>   Code branching protection to defend against jump/call oriented
>   programming.
> 
> Several new CET MSRs are defined in kernel to support CET:
>   MSR_IA32_{U,S}_CET: Controls the CET settings for user
>                       mode and kernel mode respectively.
> 
>   MSR_IA32_PL{0,1,2,3}_SSP: Stores shadow stack pointers for
>                             CPL-0,1,2,3 protection respectively.
> 
>   MSR_IA32_INT_SSP_TAB: Stores base address of shadow stack
>                         pointer table.
> 
> Two XSAVES state bits are introduced for CET:
>   IA32_XSS:[bit 11]: Control saving/restoring user mode CET states
>   IA32_XSS:[bit 12]: Control saving/restoring kernel mode CET states.
> 
> Six VMCS fields are introduced for CET:
>   {HOST,GUEST}_S_CET: Stores CET settings for kernel mode.
>   {HOST,GUEST}_SSP: Stores shadow stack pointer of current task/thread.
>   {HOST,GUEST}_INTR_SSP_TABLE: Stores base address of shadow stack pointer
>                                table.
> 
> If VM_EXIT_LOAD_HOST_CET_STATE = 1, the host CET states are restored
> from below VMCS fields at VM-Exit:
>   HOST_S_CET
>   HOST_SSP
>   HOST_INTR_SSP_TABLE
> 
> If VM_ENTRY_LOAD_GUEST_CET_STATE = 1, the guest CET states are loaded
> from below VMCS fields at VM-Entry:
>   GUEST_S_CET
>   GUEST_SSP
>   GUEST_INTR_SSP_TABLE
> 
> Co-developed-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
> 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 ++++++++
>  arch/x86/include/uapi/asm/kvm.h | 1 +
>  arch/x86/kvm/x86.c              | 4 ++++
>  arch/x86/kvm/x86.h              | 2 +-
>  4 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
> index 5e090d1f03f8..e938bc6c37aa 100644
> --- a/arch/x86/include/asm/vmx.h
> +++ b/arch/x86/include/asm/vmx.h
> @@ -94,6 +94,7 @@
>  #define VM_EXIT_CLEAR_BNDCFGS                   0x00800000
>  #define VM_EXIT_PT_CONCEAL_PIP			0x01000000
>  #define VM_EXIT_CLEAR_IA32_RTIT_CTL		0x02000000
> +#define VM_EXIT_LOAD_HOST_CET_STATE             0x10000000
>  
>  #define VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR	0x00036dff
>  
> @@ -107,6 +108,7 @@
>  #define VM_ENTRY_LOAD_BNDCFGS                   0x00010000
>  #define VM_ENTRY_PT_CONCEAL_PIP			0x00020000
>  #define VM_ENTRY_LOAD_IA32_RTIT_CTL		0x00040000
> +#define VM_ENTRY_LOAD_GUEST_CET_STATE           0x00100000
>  
>  #define VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR	0x000011ff
>  
> @@ -328,6 +330,9 @@ enum vmcs_field {
>  	GUEST_PENDING_DBG_EXCEPTIONS    = 0x00006822,
>  	GUEST_SYSENTER_ESP              = 0x00006824,
>  	GUEST_SYSENTER_EIP              = 0x00006826,
> +	GUEST_S_CET                     = 0x00006828,
> +	GUEST_SSP                       = 0x0000682a,
> +	GUEST_INTR_SSP_TABLE            = 0x0000682c,
>  	HOST_CR0                        = 0x00006c00,
>  	HOST_CR3                        = 0x00006c02,
>  	HOST_CR4                        = 0x00006c04,
> @@ -340,6 +345,9 @@ enum vmcs_field {
>  	HOST_IA32_SYSENTER_EIP          = 0x00006c12,
>  	HOST_RSP                        = 0x00006c14,
>  	HOST_RIP                        = 0x00006c16,
> +	HOST_S_CET                      = 0x00006c18,
> +	HOST_SSP                        = 0x00006c1a,
> +	HOST_INTR_SSP_TABLE             = 0x00006c1c
>  };
>  
>  /*
> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> index 3f3f780c8c65..78e5c4266270 100644
> --- a/arch/x86/include/uapi/asm/kvm.h
> +++ b/arch/x86/include/uapi/asm/kvm.h
> @@ -31,6 +31,7 @@
>  #define MC_VECTOR 18
>  #define XM_VECTOR 19
>  #define VE_VECTOR 20
> +#define CP_VECTOR 21
>  
>  /* Select x86 specific features in <linux/kvm.h> */
>  #define __KVM_HAVE_PIT
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 40c6768942ae..830afe5038d1 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -186,6 +186,9 @@ static struct kvm_shared_msrs __percpu *shared_msrs;
>  				| XFEATURE_MASK_BNDCSR | XFEATURE_MASK_AVX512 \
>  				| XFEATURE_MASK_PKRU)
>  
> +#define KVM_SUPPORTED_XSS	(XFEATURE_MASK_CET_USER | \
> +				 XFEATURE_MASK_CET_KERNEL)

This belongs in a later patch, KVM obviously doesn't support XSS.

> +
>  u64 __read_mostly host_efer;
>  EXPORT_SYMBOL_GPL(host_efer);
>  
> @@ -402,6 +405,7 @@ static int exception_class(int vector)
>  	case NP_VECTOR:
>  	case SS_VECTOR:
>  	case GP_VECTOR:
> +	case CP_VECTOR:
>  		return EXCPT_CONTRIBUTORY;
>  	default:
>  		break;
> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> index c1954e216b41..8f0baa6fa72f 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -115,7 +115,7 @@ static inline bool x86_exception_has_error_code(unsigned int vector)
>  {
>  	static u32 exception_has_error_code = BIT(DF_VECTOR) | BIT(TS_VECTOR) |
>  			BIT(NP_VECTOR) | BIT(SS_VECTOR) | BIT(GP_VECTOR) |
> -			BIT(PF_VECTOR) | BIT(AC_VECTOR);
> +			BIT(PF_VECTOR) | BIT(AC_VECTOR) | BIT(CP_VECTOR);
>  
>  	return (1U << vector) & exception_has_error_code;

Maybe it's gratuitous, but I feel like the #CP logic should be in a patch
of its own, e.g. the changelog doesn't mention anything about #CP.

>  }
> -- 
> 2.17.2
> 

Re: [PATCH v11 2/9] KVM: VMX: Set guest CET MSRs per KVM and host configuration

[Sean Christopherson]

On Thu, Mar 26, 2020 at 04:18:39PM +0800, Yang Weijiang wrote:
> CET MSRs pass through guest directly to enhance performance.
> CET runtime control settings are stored in MSR_IA32_{U,S}_CET,
> Shadow Stack Pointer(SSP) are stored in MSR_IA32_PL{0,1,2,3}_SSP,
> SSP table base address is stored in MSR_IA32_INT_SSP_TAB,
> these MSRs are defined in kernel and re-used here.
> 
> MSR_IA32_U_CET and MSR_IA32_PL3_SSP are used for user-mode protection,
> the MSR contents are switched between threads during scheduling,
> it makes sense to pass through them so that the guest kernel can
> use xsaves/xrstors to operate them efficiently. Other MSRs are used
> for non-user mode protection. See SDM for detailed info.
> 
> The difference between CET VMCS fields and CET MSRs is that,the former
> are used during VMEnter/VMExit, whereas the latter are used for CET
> state storage between task/thread scheduling.
> 
> Co-developed-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
> 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/vmx.c | 40 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 860e5f4a9f7b..1aca468d9a10 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -3033,6 +3033,13 @@ void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3)
>  		vmcs_writel(GUEST_CR3, guest_cr3);
>  }
>  
> +static bool is_cet_mode_allowed(struct kvm_vcpu *vcpu, u32 mode_mask)

CET itself isn't a mode.  And since this ends up being an inner helper for
is_cet_supported(), I think __is_cet_supported() would be the way to go.

Even @mode_mask is a bit confusing without the context of it being kernel
vs. user.  The callers are very readable, e.g. I'd much prefer passing the
mask as opposed to doing 'bool kernel'.  Maybe s/mode_mask/cet_mask?  That
doesn't exactly make things super clear, but at least the reader knows the
mask is for CET features.

> +{
> +	return ((supported_xss & mode_mask) &&
> +		(guest_cpuid_has(vcpu, X86_FEATURE_SHSTK) ||
> +		guest_cpuid_has(vcpu, X86_FEATURE_IBT)));
> +}
> +
>  int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
>  {
>  	struct vcpu_vmx *vmx = to_vmx(vcpu);
> @@ -7064,6 +7071,35 @@ static void update_intel_pt_cfg(struct kvm_vcpu *vcpu)
>  		vmx->pt_desc.ctl_bitmask &= ~(0xfULL << (32 + i * 4));
>  }
>  
> +static void vmx_update_intercept_for_cet_msr(struct kvm_vcpu *vcpu)
> +{
> +	struct vcpu_vmx *vmx = to_vmx(vcpu);
> +	unsigned long *msr_bitmap = vmx->vmcs01.msr_bitmap;
> +	bool flag;

Maybe s/flag/incpt or something to make it more obvious that the bool is
true if we want to intercept?  vmx_set_intercept_for_msr()s's @value isn't
any better :-/.

> +
> +	flag = !is_cet_mode_allowed(vcpu, XFEATURE_MASK_CET_USER);
> +	/*
> +	 * U_CET is required for USER CET, and U_CET, PL3_SPP are bound as
> +	 * one component and controlled by IA32_XSS[bit 11].
> +	 */
> +	vmx_set_intercept_for_msr(msr_bitmap, MSR_IA32_U_CET, MSR_TYPE_RW, flag);
> +	vmx_set_intercept_for_msr(msr_bitmap, MSR_IA32_PL3_SSP, MSR_TYPE_RW, flag);
> +
> +	flag = !is_cet_mode_allowed(vcpu, XFEATURE_MASK_CET_KERNEL);
> +	/*
> +	 * S_CET is required for KERNEL CET, and PL0_SSP ... PL2_SSP are
> +	 * bound as one component and controlled by IA32_XSS[bit 12].
> +	 */
> +	vmx_set_intercept_for_msr(msr_bitmap, MSR_IA32_S_CET, MSR_TYPE_RW, flag);
> +	vmx_set_intercept_for_msr(msr_bitmap, MSR_IA32_PL0_SSP, MSR_TYPE_RW, flag);
> +	vmx_set_intercept_for_msr(msr_bitmap, MSR_IA32_PL1_SSP, MSR_TYPE_RW, flag);
> +	vmx_set_intercept_for_msr(msr_bitmap, MSR_IA32_PL2_SSP, MSR_TYPE_RW, flag);
> +
> +	flag |= !guest_cpuid_has(vcpu, X86_FEATURE_SHSTK);
> +	/* SSP_TAB is only available for KERNEL SHSTK.*/
> +	vmx_set_intercept_for_msr(msr_bitmap, MSR_IA32_INT_SSP_TAB, MSR_TYPE_RW, flag);
> +}
> +
>  static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
>  {
>  	struct vcpu_vmx *vmx = to_vmx(vcpu);
> @@ -7102,6 +7138,10 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
>  			vmx_set_guest_msr(vmx, msr, enabled ? 0 : TSX_CTRL_RTM_DISABLE);
>  		}
>  	}
> +
> +	if (guest_cpuid_has(vcpu, X86_FEATURE_SHSTK) ||
> +	    guest_cpuid_has(vcpu, X86_FEATURE_IBT))
> +		vmx_update_intercept_for_cet_msr(vcpu);

This is wrong, it will miss the case where userspace double configures CPUID
and goes from CET=1 to CET=0.  This should instead be:

	if (supported_xss & (XFEATURE_MASK_CET_KERNEL | XFEATURE_MASK_CET_USER))
		vmx_update_intercept_for_cet_msr(vcpu);

>  }
>  
>  static __init void vmx_set_cpu_caps(void)
> -- 
> 2.17.2
> 

Re: [PATCH v11 1/9] KVM: VMX: Introduce CET VMX fields and flags

[Sean Christopherson]

On Thu, Mar 26, 2020 at 04:18:38PM +0800, Yang Weijiang wrote:
> If VM_EXIT_LOAD_HOST_CET_STATE = 1, the host CET states are restored
> from below VMCS fields at VM-Exit:
>   HOST_S_CET
>   HOST_SSP
>   HOST_INTR_SSP_TABLE
> 
> If VM_ENTRY_LOAD_GUEST_CET_STATE = 1, the guest CET states are loaded
> from below VMCS fields at VM-Entry:
>   GUEST_S_CET
>   GUEST_SSP
>   GUEST_INTR_SSP_TABLE
> 
> Co-developed-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
> Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> ---

...

> diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
> index 5e090d1f03f8..e938bc6c37aa 100644
> --- a/arch/x86/include/asm/vmx.h
> +++ b/arch/x86/include/asm/vmx.h
> @@ -94,6 +94,7 @@
>  #define VM_EXIT_CLEAR_BNDCFGS                   0x00800000
>  #define VM_EXIT_PT_CONCEAL_PIP			0x01000000
>  #define VM_EXIT_CLEAR_IA32_RTIT_CTL		0x02000000
> +#define VM_EXIT_LOAD_HOST_CET_STATE             0x10000000
>  
>  #define VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR	0x00036dff
>  
> @@ -107,6 +108,7 @@
>  #define VM_ENTRY_LOAD_BNDCFGS                   0x00010000
>  #define VM_ENTRY_PT_CONCEAL_PIP			0x00020000
>  #define VM_ENTRY_LOAD_IA32_RTIT_CTL		0x00040000
> +#define VM_ENTRY_LOAD_GUEST_CET_STATE           0x00100000

I think it probably make senses to drop HOST/GUEST from the controls,
i.e. VM_{ENTER,EXIT}_LOAD_CET_STATE.  The SDM doesn't qualify them with
guest vs. host, nor does KVM qualify any of the other entry/exit controls
that are effective guest vs. host.

Re: [PATCH v11 9/9] KVM: X86: Set CET feature bits for CPUID enumeration

[Sean Christopherson]

On Thu, Mar 26, 2020 at 04:18:46PM +0800, Yang Weijiang wrote:
> Set the feature bits so that CET capabilities can be seen
> in guest via CPUID enumeration. Add CR4.CET bit support
> in order to allow guest set CET master control bit(CR4.CET).
> 
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> ---
>  arch/x86/include/asm/kvm_host.h | 3 ++-
>  arch/x86/kvm/cpuid.c            | 4 ++++
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 2c944ad99692..5109c43c6981 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -95,7 +95,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)
>  
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 25e9a11291b3..26ab959df92f 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -366,6 +366,10 @@ void kvm_set_cpu_caps(void)
>  		kvm_cpu_cap_set(X86_FEATURE_INTEL_STIBP);
>  	if (boot_cpu_has(X86_FEATURE_AMD_SSBD))
>  		kvm_cpu_cap_set(X86_FEATURE_SPEC_CTRL_SSBD);
> +	if (boot_cpu_has(X86_FEATURE_IBT))
> +		kvm_cpu_cap_set(X86_FEATURE_IBT);
> +	if (boot_cpu_has(X86_FEATURE_SHSTK))
> +		kvm_cpu_cap_set(X86_FEATURE_SHSTK);

This is the wrong way to advertise bits, the correct method is to declare
the flag in the appriorate kvm_cpu_cap_mask() call.  The manually handling
is only needed when the feature bit diverges from kernel support, either
because KVM allow a feature based purely on hardware support, e.g. LA57, or
when emulating a feature based on a different similar feature, e.g. the
STIBP/SSBD flags above.

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 6828be99b908..6262438f9527 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -329,7 +329,8 @@ void kvm_set_cpu_caps(void)
                F(AVX512VBMI) | F(LA57) | 0 /*PKU*/ | 0 /*OSPKE*/ | F(RDPID) |
                F(AVX512_VPOPCNTDQ) | F(UMIP) | F(AVX512_VBMI2) | F(GFNI) |
                F(VAES) | F(VPCLMULQDQ) | F(AVX512_VNNI) | F(AVX512_BITALG) |
-               F(CLDEMOTE) | F(MOVDIRI) | F(MOVDIR64B) | 0 /*WAITPKG*/
+               F(CLDEMOTE) | F(MOVDIRI) | F(MOVDIR64B) | 0 /*WAITPKG*/ |
+               F(SHSTK)
        );
        /* Set LA57 based on hardware capability. */
        if (cpuid_ecx(7) & F(LA57))
@@ -338,7 +339,7 @@ void kvm_set_cpu_caps(void)
        kvm_cpu_cap_mask(CPUID_7_EDX,
                F(AVX512_4VNNIW) | F(AVX512_4FMAPS) | F(SPEC_CTRL) |
                F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES) | F(INTEL_STIBP) |
-               F(MD_CLEAR) | F(AVX512_VP2INTERSECT) | F(FSRM)
+               F(MD_CLEAR) | F(AVX512_VP2INTERSECT) | F(FSRM) | F(IBT)
        );

>  
>  	kvm_cpu_cap_mask(CPUID_7_1_EAX,
>  		F(AVX512_BF16)
> -- 
> 2.17.2
> 

Re: [PATCH v11 9/9] KVM: X86: Set CET feature bits for CPUID enumeration

[Sean Christopherson]

For the shortlog, use something like

  KVM: x86: Enable CET virtualization and advertise CET to userspace

It took me a while to find the patch that actually allowed setting CR4.CET
because I was only scanning the shortlogs :-)

Re: [PATCH v11 3/9] KVM: VMX: Set host/guest CET states for vmexit/vmentry

[Sean Christopherson]

On Thu, Mar 26, 2020 at 04:18:40PM +0800, Yang Weijiang wrote:
> "Load {guest,host} CET state" bit controls whether guest/host
> CET states will be loaded at VM entry/exit.
> Set default host kernel CET states to 0s in VMCS to avoid guest
> CET states leakage. When CR4.CET is cleared due to guest mode
> change, make guest CET states invalid in VMCS, this can happen,
> e.g., guest reboot.
> 
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> ---
>  arch/x86/kvm/vmx/capabilities.h | 10 ++++++
>  arch/x86/kvm/vmx/vmx.c          | 56 +++++++++++++++++++++++++++++++--
>  2 files changed, 63 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h
> index 8903475f751e..565340352260 100644
> --- a/arch/x86/kvm/vmx/capabilities.h
> +++ b/arch/x86/kvm/vmx/capabilities.h
> @@ -107,6 +107,16 @@ static inline bool cpu_has_vmx_mpx(void)
>  		(vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_BNDCFGS);
>  }
>  
> +static inline bool cpu_has_cet_guest_load_ctrl(void)
> +{
> +	return (vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_GUEST_CET_STATE);
> +}
> +
> +static inline bool cpu_has_cet_host_load_ctrl(void)
> +{
> +	return (vmcs_config.vmexit_ctrl & VM_EXIT_LOAD_HOST_CET_STATE);
> +}

We should bundle these together, same as we do for PERF_GLOBAL_CTRL.  Not
just for code clarity, but also for functionality, e.g. if KVM ends up on a
frankenstein CPU with VM_ENTRY_LOAD_CET_STATE and !VM_EXIT_LOAD_CET_STATE
then KVM will end up running with guest state.  This is also an argument
for not qualifying the control names with GUEST vs. HOST.

> +
>  static inline bool cpu_has_vmx_tpr_shadow(void)
>  {
>  	return vmcs_config.cpu_based_exec_ctrl & CPU_BASED_TPR_SHADOW;
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 1aca468d9a10..bd7cd175fd81 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -44,6 +44,7 @@
>  #include <asm/spec-ctrl.h>
>  #include <asm/virtext.h>
>  #include <asm/vmx.h>
> +#include <asm/cet.h>
>  
>  #include "capabilities.h"
>  #include "cpuid.h"
> @@ -2456,7 +2457,8 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf,
>  	      VM_EXIT_LOAD_IA32_EFER |
>  	      VM_EXIT_CLEAR_BNDCFGS |
>  	      VM_EXIT_PT_CONCEAL_PIP |
> -	      VM_EXIT_CLEAR_IA32_RTIT_CTL;
> +	      VM_EXIT_CLEAR_IA32_RTIT_CTL |
> +	      VM_EXIT_LOAD_HOST_CET_STATE;
>  	if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_EXIT_CTLS,
>  				&_vmexit_control) < 0)
>  		return -EIO;
> @@ -2480,7 +2482,8 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf,
>  	      VM_ENTRY_LOAD_IA32_EFER |
>  	      VM_ENTRY_LOAD_BNDCFGS |
>  	      VM_ENTRY_PT_CONCEAL_PIP |
> -	      VM_ENTRY_LOAD_IA32_RTIT_CTL;
> +	      VM_ENTRY_LOAD_IA32_RTIT_CTL |
> +	      VM_ENTRY_LOAD_GUEST_CET_STATE;
>  	if (adjust_vmx_controls(min, opt, MSR_IA32_VMX_ENTRY_CTLS,
>  				&_vmentry_control) < 0)
>  		return -EIO;
> @@ -3040,6 +3043,12 @@ static bool is_cet_mode_allowed(struct kvm_vcpu *vcpu, u32 mode_mask)
>  		guest_cpuid_has(vcpu, X86_FEATURE_IBT)));
>  }
>  
> +static bool is_cet_supported(struct kvm_vcpu *vcpu)
> +{
> +	return is_cet_mode_allowed(vcpu, XFEATURE_MASK_CET_USER |
> +				   XFEATURE_MASK_CET_KERNEL);
> +}
> +
>  int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
>  {
>  	struct vcpu_vmx *vmx = to_vmx(vcpu);
> @@ -3110,6 +3119,12 @@ int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
>  			hw_cr4 &= ~(X86_CR4_SMEP | X86_CR4_SMAP | X86_CR4_PKE);
>  	}
>  
> +	if (!(hw_cr4 & X86_CR4_CET) && is_cet_supported(vcpu)) {
> +		vmcs_writel(GUEST_SSP, 0);
> +		vmcs_writel(GUEST_S_CET, 0);
> +		vmcs_writel(GUEST_INTR_SSP_TABLE, 0);

Can't we simply toggle the VM_{ENTRY,EXIT}_LOAD controls?  If CR4.CET=0 in
the guest, then presumably keeping host state loaded is ok?  I.e. won't
leak host information to the guest.

> +	}
> +
>  	vmcs_writel(CR4_READ_SHADOW, cr4);
>  	vmcs_writel(GUEST_CR4, hw_cr4);
>  	return 0;
> @@ -3939,6 +3954,12 @@ void vmx_set_constant_host_state(struct vcpu_vmx *vmx)
>  
>  	if (cpu_has_load_ia32_efer())
>  		vmcs_write64(HOST_IA32_EFER, host_efer);
> +
> +	if (cpu_has_cet_host_load_ctrl()) {
> +		vmcs_writel(HOST_S_CET, 0);
> +		vmcs_writel(HOST_INTR_SSP_TABLE, 0);
> +		vmcs_writel(HOST_SSP, 0);

This is unnecessary, the VMCS is zeroed on allocation.

And IIUC, this is only correct because the main shadow stack enabling only
adds user support.  Assuming that's correct, (a) it absolutely needs to be
called out in the changelog and (b) KVM needs a WARN in hardware_setup() to
guard against kernel shadow stack support being added without updating KVM,
e.g. something this (not sure the MSRs are correct):

	if (boot_cpu_has(X86_FEATURE_IBT) || boot_cpu_has(X86_FEATURE_SHSTK)) {
		rdmsrl(MSR_IA32_S_CET, cet_msr);
		WARN_ONCE(cet_msr, "KVM: S_CET in host will be lost");

	}
	if (boot_cpu_has(X86_FEATURE_SHSTK)) {
		rdmsrl(MSR_IA32_PL0_SSP, cet_msr);
		WARN_ONCE(cet_msr, "KVM: PL0_SPP in host will be lost");
	}

> +	}
>  }
>  
>  void set_cr4_guest_host_mask(struct vcpu_vmx *vmx)
> @@ -5749,6 +5770,13 @@ void dump_vmcs(void)
>  		pr_err("InterruptStatus = %04x\n",
>  		       vmcs_read16(GUEST_INTR_STATUS));
>  
> +	if (vmentry_ctl & VM_ENTRY_LOAD_GUEST_CET_STATE) {
> +		pr_err("S_CET = 0x%016lx\n", vmcs_readl(GUEST_S_CET));
> +		pr_err("SSP = 0x%016lx\n", vmcs_readl(GUEST_SSP));
> +		pr_err("SSP TABLE = 0x%016lx\n",
> +		       vmcs_readl(GUEST_INTR_SSP_TABLE));
> +	}
> +
>  	pr_err("*** Host State ***\n");
>  	pr_err("RIP = 0x%016lx  RSP = 0x%016lx\n",
>  	       vmcs_readl(HOST_RIP), vmcs_readl(HOST_RSP));
> @@ -5831,6 +5859,13 @@ void dump_vmcs(void)
>  	if (secondary_exec_control & SECONDARY_EXEC_ENABLE_VPID)
>  		pr_err("Virtual processor ID = 0x%04x\n",
>  		       vmcs_read16(VIRTUAL_PROCESSOR_ID));
> +	if (vmexit_ctl & VM_EXIT_LOAD_HOST_CET_STATE) {
> +		pr_err("S_CET = 0x%016lx\n", vmcs_readl(HOST_S_CET));
> +		pr_err("SSP = 0x%016lx\n", vmcs_readl(HOST_SSP));
> +		pr_err("SSP TABLE = 0x%016lx\n",
> +		       vmcs_readl(HOST_INTR_SSP_TABLE));
> +	}
> +
>  }
>  
>  /*
> @@ -7140,8 +7175,23 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
>  	}
>  
>  	if (guest_cpuid_has(vcpu, X86_FEATURE_SHSTK) ||
> -	    guest_cpuid_has(vcpu, X86_FEATURE_IBT))
> +	    guest_cpuid_has(vcpu, X86_FEATURE_IBT)) {
>  		vmx_update_intercept_for_cet_msr(vcpu);
> +
> +		if (cpu_has_cet_guest_load_ctrl() && is_cet_supported(vcpu))
> +			vm_entry_controls_setbit(to_vmx(vcpu),
> +						 VM_ENTRY_LOAD_GUEST_CET_STATE);
> +		else
> +			vm_entry_controls_clearbit(to_vmx(vcpu),
> +						   VM_ENTRY_LOAD_GUEST_CET_STATE);
> +
> +		if (cpu_has_cet_host_load_ctrl() && is_cet_supported(vcpu))
> +			vm_exit_controls_setbit(to_vmx(vcpu),
> +						VM_EXIT_LOAD_HOST_CET_STATE);
> +		else
> +			vm_exit_controls_clearbit(to_vmx(vcpu),
> +						  VM_EXIT_LOAD_HOST_CET_STATE);

As above, I think this can be done in vmx_set_cr4().

> +	}
>  }
>  
>  static __init void vmx_set_cpu_caps(void)
> -- 
> 2.17.2
> 

Re: [PATCH v11 4/9] KVM: VMX: Check CET dependencies on CR settings

[Sean Christopherson]

On Thu, Mar 26, 2020 at 04:18:41PM +0800, Yang Weijiang wrote:
> CR4.CET is master control bit for CET function.
> There're mutual constrains between CR0.WP and CR4.CET, so need
> to check the dependent bit while changing the control registers.
> 
> Note:
> 1)The processor does not allow CR4.CET to be set if CR0.WP = 0,
>   similarly, it does not allow CR0.WP to be cleared while
>   CR4.CET = 1. In either case, KVM would inject #GP to guest.

Nit: the CET vs. WP dependency and #GP belongs in the "main" part of the
changelog, as it's the crux of the patch.  Item (2) below is more along
the lines of "note" material.

> 
> 2)SHSTK and IBT features share one control MSR:
>   MSR_IA32_{U,S}_CET, which means it's difficult to hide
>   one feature from another in the case of SHSTK != IBT,
>   after discussed in community, it's agreed to allow guest
>   control two features independently as it won't introduce
>   security hole.
> 
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> ---
>  arch/x86/kvm/vmx/vmx.c | 4 ++++
>  arch/x86/kvm/x86.c     | 3 +++
>  2 files changed, 7 insertions(+)
> 
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index bd7cd175fd81..87f101750746 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -3089,6 +3089,10 @@ int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
>  			return 1;
>  	}
>  
> +	if ((cr4 & X86_CR4_CET) && (!is_cet_supported(vcpu) ||
> +	    !(kvm_read_cr0(vcpu) & X86_CR0_WP)))
> +		return 1;
> +
>  	if (vmx->nested.vmxon && !nested_cr4_valid(vcpu, cr4))
>  		return 1;
>  
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 830afe5038d1..90acdbbb8a5a 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -804,6 +804,9 @@ int kvm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0)
>  	if (!(cr0 & X86_CR0_PG) && kvm_read_cr4_bits(vcpu, X86_CR4_PCIDE))
>  		return 1;
>  
> +	if (!(cr0 & X86_CR0_WP) && kvm_read_cr4_bits(vcpu, X86_CR4_CET))
> +		return 1;
> +
>  	kvm_x86_ops->set_cr0(vcpu, cr0);
>  
>  	if ((cr0 ^ old_cr0) & X86_CR0_PG) {
> -- 
> 2.17.2
> 

Re: [PATCH v11 5/9] KVM: X86: Refresh CPUID once guest XSS MSR changes

[Sean Christopherson]

On Thu, Mar 26, 2020 at 04:18:42PM +0800, Yang Weijiang wrote:
> CPUID(0xd, 1) reports the current required storage size of
> XCR0 | XSS, when guest updates the XSS, it's necessary to update
> the CPUID leaf, otherwise guest will fetch old state size, and
> results to some WARN traces during guest running.
> 
> Co-developed-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
> 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 |  1 +
>  arch/x86/kvm/cpuid.c            | 21 ++++++++++++++++++---
>  arch/x86/kvm/x86.c              |  9 +++++++--
>  3 files changed, 26 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 24c90ea5ddbd..2c944ad99692 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -650,6 +650,7 @@ struct kvm_vcpu_arch {
>  
>  	u64 xcr0;
>  	u64 guest_supported_xcr0;
> +	u64 guest_supported_xss;
>  	u32 guest_xstate_size;
>  
>  	struct kvm_pio_request pio;
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 78d461be2102..25e9a11291b3 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -95,9 +95,24 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu)
>  	}
>  
>  	best = kvm_find_cpuid_entry(vcpu, 0xD, 1);
> -	if (best && (cpuid_entry_has(best, X86_FEATURE_XSAVES) ||
> -		     cpuid_entry_has(best, X86_FEATURE_XSAVEC)))
> -		best->ebx = xstate_required_size(vcpu->arch.xcr0, true);
> +	if (best) {
> +		if (best->eax & (F(XSAVES) | F(XSAVEC))) {

Please use cpuid_entry_has() to preserve the automagic register lookup and
compile-time assertions that are provided.  E.g. I don't know off the top
of my whether %eax is the correct register, and I don't want to know :-).

> +			u64 xstate = vcpu->arch.xcr0 | vcpu->arch.ia32_xss;
> +
> +			best->ebx = xstate_required_size(xstate, true);
> +		}
> +
> +		if (best->eax & F(XSAVES)) {

Same thing here.

> +			vcpu->arch.guest_supported_xss =
> +			(best->ecx | ((u64)best->edx << 32)) & supported_xss;

The indentation is funky, I'm guessing you're trying to squeak in less than
80 chars.  Maybe this?

		if (!cpuid_entry_has(best, X86_FEATURE_XSAVES)) {
			best->ecx = 0;
			best->edx = 0;
		}

		 vcpu->arch.guest_supported_xss =
			(((u64)best->edx << 32) | best->ecx) & supported_xss;

Nit: my preference is to have the high half first, x86 is little endian
(the xcr0 code is "wrong" :-D).  For me, this also makes it more obvious
that the effective size is a u64.

> +		} else {
> +			best->ecx = 0;
> +			best->edx = 0;
> +			vcpu->arch.guest_supported_xss = 0;
> +		}
> +	} else {
> +		vcpu->arch.guest_supported_xss = 0;
> +	}
>  
>  	/*
>  	 * The existing code assumes virtual address is 48-bit or 57-bit in the
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 90acdbbb8a5a..51ecb496d47d 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2836,9 +2836,12 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  		 * IA32_XSS[bit 8]. Guests have to use RDMSR/WRMSR rather than
>  		 * XSAVES/XRSTORS to save/restore PT MSRs.
>  		 */
> -		if (data & ~supported_xss)
> +		if (data & ~vcpu->arch.guest_supported_xss)
>  			return 1;
> -		vcpu->arch.ia32_xss = data;
> +		if (vcpu->arch.ia32_xss != data) {
> +			vcpu->arch.ia32_xss = data;
> +			kvm_update_cpuid(vcpu);
> +		}
>  		break;
>  	case MSR_SMI_COUNT:
>  		if (!msr_info->host_initiated)
> @@ -9635,6 +9638,8 @@ int kvm_arch_hardware_setup(void)
>  
>  	if (!kvm_cpu_cap_has(X86_FEATURE_XSAVES))
>  		supported_xss = 0;
> +	else
> +		supported_xss = host_xss & KVM_SUPPORTED_XSS;

Silly nit: I'd prefer to invert the check, e.g.

	if (kvm_cpu_cap_has(X86_FEATURE_XSAVES))
		supported_xss = host_xss & KVM_SUPPORTED_XSS;
	else
		supported_xss = 0;

>  
>  	cr4_reserved_bits = kvm_host_cr4_reserved_bits(&boot_cpu_data);
>  
> -- 
> 2.17.2
> 

Re: [PATCH v11 7/9] KVM: X86: Add userspace access interface for CET MSRs

[Sean Christopherson]

On Thu, Mar 26, 2020 at 04:18:44PM +0800, Yang Weijiang wrote:
> +#define CET_MSR_RSVD_BITS_1  GENMASK(1, 0)
> +#define CET_MSR_RSVD_BITS_2  GENMASK(9, 6)
> +
> +static bool cet_check_msr_write(struct kvm_vcpu *vcpu,

s/cet_check_msr_write/is_cet_msr_valid

Otherwise the polarity of the return value isn't obvious.

> +				struct msr_data *msr,

Unnecessary newline.

> +				u64 mask)

s/mask/rsvd_bits

> +{
> +	u64 data = msr->data;
> +	u32 high_word = data >> 32;
> +
> +	if (data & mask)
> +		return false;
> +
> +	if (!is_64_bit_mode(vcpu) && high_word)
> +		return false;

As I called out before, this is wrong.  AFAIK, the CPU never depends on
WRMSR to prevent loading bits 63:32, software can simply do WRMSR and then
transition back to 32-bit mode.  Yes, the shadow stack itself is 32 bits,
but the internal value is still 64 bits.  This is backed up by the CALL
pseudocode:

  IF ShadowStackEnabled(CPL)
    IF (EFER.LMA and DEST(CodeSegmentSelector).L) = 0
      (* If target is legacy or compatibility mode then the SSP must be in low 4GB *)
      IF (SSP & 0xFFFFFFFF00000000 != 0)
        THEN #GP(0); FI;
  FI;

as well as RDSSP:

  IF CPL = 3
    IF CR4.CET & IA32_U_CET.SH_STK_EN
      IF (operand size is 64 bit)
        THEN
          Dest ← SSP;
        ELSE
          Dest ← SSP[31:0];
      FI;
    FI;
  ELSE

> +
> +	return true;
> +}
> +
> +static bool cet_check_ssp_msr_access(struct kvm_vcpu *vcpu,
> +				     struct msr_data *msr)

Similar to above, the polarity of the return isn't obvious.  Maybe
is_cet_ssp_msr_accessible()?

I'd prefer to pass in @index, passing the full @msr makes it look like
this helper might also check msr->data.

> +{
> +	u32 index = msr->index;
> +
> +	if (!boot_cpu_has(X86_FEATURE_SHSTK))
> +		return false;
> +
> +	if (!msr->host_initiated &&
> +	    !guest_cpuid_has(vcpu, X86_FEATURE_SHSTK))
> +		return false;
> +
> +	if (index == MSR_IA32_INT_SSP_TAB)
> +		return true;
> +
> +	if (index == MSR_IA32_PL3_SSP) {
> +		if (!(supported_xss & XFEATURE_MASK_CET_USER))
> +			return false;
> +	} else if (!(supported_xss & XFEATURE_MASK_CET_KERNEL)) {
> +		return false;
> +	}

	if (index == MSR_IA32_PL3_SSP)
		return supported_xss & XFEATURE_MASK_CET_USER;

	/* MSR_IA32_PL[0-2]_SSP */
	return supported_xss & XFEATURE_MASK_CET_KERNEL;
> +
> +	return true;
> +}
> +
> +static bool cet_check_ctl_msr_access(struct kvm_vcpu *vcpu,

is_cet_ctl_msr_accessible?

> +				     struct msr_data *msr)
> +{
> +	u32 index = msr->index;
> +
> +	if (!boot_cpu_has(X86_FEATURE_SHSTK) &&
> +	    !boot_cpu_has(X86_FEATURE_IBT))
> +		return false;
> +
> +	if (!msr->host_initiated &&
> +	    !guest_cpuid_has(vcpu, X86_FEATURE_SHSTK) &&
> +	    !guest_cpuid_has(vcpu, X86_FEATURE_IBT))
> +		return false;
> +
> +	if (index == MSR_IA32_U_CET) {
> +		if (!(supported_xss & XFEATURE_MASK_CET_USER))
> +			return false;
> +	} else if (!(supported_xss & XFEATURE_MASK_CET_KERNEL)) {
> +		return false;
> +	}

Same as above:

	if (index == MSR_IA32_U_CET)
		return supported_xss & XFEATURE_MASK_CET_USER;

	return supported_xss & XFEATURE_MASK_CET_KERNEL;
> +
> +	return true;
> +}
>  /*
>   * Reads an msr value (of 'msr_index') into 'pdata'.
>   * Returns 0 on success, non-0 otherwise.
> @@ -1941,6 +2026,26 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  		else
>  			msr_info->data = vmx->pt_desc.guest.addr_a[index / 2];
>  		break;
> +	case MSR_IA32_S_CET:
> +		if (!cet_check_ctl_msr_access(vcpu, msr_info))
> +			return 1;
> +		msr_info->data = vmcs_readl(GUEST_S_CET);
> +		break;
> +	case MSR_IA32_INT_SSP_TAB:
> +		if (!cet_check_ssp_msr_access(vcpu, msr_info))
> +			return 1;
> +		msr_info->data = vmcs_readl(GUEST_INTR_SSP_TABLE);
> +		break;
> +	case MSR_IA32_U_CET:
> +		if (!cet_check_ctl_msr_access(vcpu, msr_info))
> +			return 1;
> +		vmx_get_xsave_msr(msr_info);
> +		break;
> +	case MSR_IA32_PL0_SSP ... MSR_IA32_PL3_SSP:
> +		if (!cet_check_ssp_msr_access(vcpu, msr_info))
> +			return 1;
> +		vmx_get_xsave_msr(msr_info);
> +		break;
>  	case MSR_TSC_AUX:
>  		if (!msr_info->host_initiated &&
>  		    !guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP))
> @@ -2197,6 +2302,34 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>  		else
>  			vmx->pt_desc.guest.addr_a[index / 2] = data;
>  		break;
> +	case MSR_IA32_S_CET:
> +		if (!cet_check_ctl_msr_access(vcpu, msr_info))
> +			return 1;
> +		if (!cet_check_msr_write(vcpu, msr_info, CET_MSR_RSVD_BITS_2))
> +			return 1;
> +		vmcs_writel(GUEST_S_CET, data);
> +		break;
> +	case MSR_IA32_INT_SSP_TAB:
> +		if (!cet_check_ctl_msr_access(vcpu, msr_info))
> +			return 1;
> +		if (!is_64_bit_mode(vcpu))

This is wrong, the SDM explicitly calls out the !64 case:

  IA32_INTERRUPT_SSP_TABLE_ADDR (64 bits; 32 bits on processors that do not
  support Intel 64 architecture).

> +			return 1;
> +		vmcs_writel(GUEST_INTR_SSP_TABLE, data);
> +		break;
> +	case MSR_IA32_U_CET:
> +		if (!cet_check_ctl_msr_access(vcpu, msr_info))
> +			return 1;
> +		if (!cet_check_msr_write(vcpu, msr_info, CET_MSR_RSVD_BITS_2))
> +			return 1;
> +		vmx_set_xsave_msr(msr_info);
> +		break;
> +	case MSR_IA32_PL0_SSP ... MSR_IA32_PL3_SSP:
> +		if (!cet_check_ssp_msr_access(vcpu, msr_info))
> +			return 1;
> +		if (!cet_check_msr_write(vcpu, msr_info, CET_MSR_RSVD_BITS_1))
> +			return 1;
> +		vmx_set_xsave_msr(msr_info);
> +		break;
>  	case MSR_TSC_AUX:
>  		if (!msr_info->host_initiated &&
>  		    !guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP))
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 9654d779bdab..9e89ee6a09e1 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1229,6 +1229,10 @@ static const u32 msrs_to_save_all[] = {
>  	MSR_ARCH_PERFMON_EVENTSEL0 + 12, MSR_ARCH_PERFMON_EVENTSEL0 + 13,
>  	MSR_ARCH_PERFMON_EVENTSEL0 + 14, MSR_ARCH_PERFMON_EVENTSEL0 + 15,
>  	MSR_ARCH_PERFMON_EVENTSEL0 + 16, MSR_ARCH_PERFMON_EVENTSEL0 + 17,
> +
> +	MSR_IA32_XSS, MSR_IA32_U_CET, MSR_IA32_S_CET,
> +	MSR_IA32_PL0_SSP, MSR_IA32_PL1_SSP, MSR_IA32_PL2_SSP,
> +	MSR_IA32_PL3_SSP, MSR_IA32_INT_SSP_TAB,
>  };
>  
>  static u32 msrs_to_save[ARRAY_SIZE(msrs_to_save_all)];
> @@ -1504,6 +1508,13 @@ static int __kvm_set_msr(struct kvm_vcpu *vcpu, u32 index, u64 data,
>  		 * invokes 64-bit SYSENTER.
>  		 */
>  		data = get_canonical(data, vcpu_virt_addr_bits(vcpu));
> +		break;
> +	case MSR_IA32_PL0_SSP ... MSR_IA32_PL3_SSP:
> +	case MSR_IA32_U_CET:
> +	case MSR_IA32_S_CET:
> +	case MSR_IA32_INT_SSP_TAB:
> +		if (is_noncanonical_address(data, vcpu))

IMO the canonical check belongs in cet_check_msr_write().  The above checks
are for MSRs that are common to VMX and SVM, i.e. the common check saves
having to duplicate the logic.  If SVM picks up CET support, then they'll
presumably want to share all of the checks, not just the canonical piece.

> +			return 1;
>  	}
>  
>  	msr.data = data;
> -- 
> 2.17.2
> 

Re: [PATCH v11 8/9] KVM: VMX: Enable CET support for nested VM

[Sean Christopherson]

On Thu, Mar 26, 2020 at 04:18:45PM +0800, Yang Weijiang wrote:
> CET MSRs pass through guests for performance consideration.
> Configure the MSRs to match L0/L1 settings so that nested VM
> is able to run with CET.
> 
> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> ---
>  arch/x86/kvm/vmx/nested.c | 41 +++++++++++++++++++++++++++++++++++++--
>  arch/x86/kvm/vmx/vmcs12.c |  6 ++++++
>  arch/x86/kvm/vmx/vmcs12.h | 14 ++++++++++++-
>  arch/x86/kvm/vmx/vmx.c    |  1 +
>  4 files changed, 59 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index e47eb7c0fbae..a71ef33de55f 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -627,6 +627,41 @@ static inline bool nested_vmx_prepare_msr_bitmap(struct kvm_vcpu *vcpu,
>  	nested_vmx_disable_intercept_for_msr(msr_bitmap_l1, msr_bitmap_l0,
>  					     MSR_KERNEL_GS_BASE, MSR_TYPE_RW);
>  
> +	/* Pass CET MSRs to nested VM if L0 and L1 are set to pass-through. */
> +	if (!msr_write_intercepted_l01(vcpu, MSR_IA32_U_CET))
> +		nested_vmx_disable_intercept_for_msr(
> +					msr_bitmap_l1, msr_bitmap_l0,
> +					MSR_IA32_U_CET, MSR_TYPE_RW);
> +
> +	if (!msr_write_intercepted_l01(vcpu, MSR_IA32_PL3_SSP))
> +		nested_vmx_disable_intercept_for_msr(
> +					msr_bitmap_l1, msr_bitmap_l0,
> +					MSR_IA32_PL3_SSP, MSR_TYPE_RW);
> +
> +	if (!msr_write_intercepted_l01(vcpu, MSR_IA32_S_CET))
> +		nested_vmx_disable_intercept_for_msr(
> +					msr_bitmap_l1, msr_bitmap_l0,
> +					MSR_IA32_S_CET, MSR_TYPE_RW);
> +
> +	if (!msr_write_intercepted_l01(vcpu, MSR_IA32_PL0_SSP))
> +		nested_vmx_disable_intercept_for_msr(
> +					msr_bitmap_l1, msr_bitmap_l0,
> +					MSR_IA32_PL0_SSP, MSR_TYPE_RW);
> +
> +	if (!msr_write_intercepted_l01(vcpu, MSR_IA32_PL1_SSP))
> +		nested_vmx_disable_intercept_for_msr(
> +					msr_bitmap_l1, msr_bitmap_l0,
> +					MSR_IA32_PL1_SSP, MSR_TYPE_RW);
> +
> +	if (!msr_write_intercepted_l01(vcpu, MSR_IA32_PL2_SSP))
> +		nested_vmx_disable_intercept_for_msr(
> +					msr_bitmap_l1, msr_bitmap_l0,
> +					MSR_IA32_PL2_SSP, MSR_TYPE_RW);
> +
> +	if (!msr_write_intercepted_l01(vcpu, MSR_IA32_INT_SSP_TAB))
> +		nested_vmx_disable_intercept_for_msr(
> +					msr_bitmap_l1, msr_bitmap_l0,
> +					MSR_IA32_INT_SSP_TAB, MSR_TYPE_RW);

That's a lot of copy-paste.  Maybe add a helper to do the conditional l01
check and subsequent call to nested_vmx_disable_intercept_for_msr()?  It's
still a lot of boilerplate, but it's at least a little better.  Not sure
what a good name would be.

	nested_vmx_update_intercept_for_msr(vcpu, MSR_IA32_U_CET,
					    msr_bitmap_l1, msr_bitmap_l0,
					    MSR_TYPE_RW);


>  	/*
>  	 * Checking the L0->L1 bitmap is trying to verify two things:
>  	 *
> @@ -6040,7 +6075,8 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
>  	msrs->exit_ctls_high |=
>  		VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR |
>  		VM_EXIT_LOAD_IA32_EFER | VM_EXIT_SAVE_IA32_EFER |
> -		VM_EXIT_SAVE_VMX_PREEMPTION_TIMER | VM_EXIT_ACK_INTR_ON_EXIT;
> +		VM_EXIT_SAVE_VMX_PREEMPTION_TIMER | VM_EXIT_ACK_INTR_ON_EXIT |
> +		VM_EXIT_LOAD_HOST_CET_STATE;
>  
>  	/* We support free control of debug control saving. */
>  	msrs->exit_ctls_low &= ~VM_EXIT_SAVE_DEBUG_CONTROLS;
> @@ -6057,7 +6093,8 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
>  #endif
>  		VM_ENTRY_LOAD_IA32_PAT;
>  	msrs->entry_ctls_high |=
> -		(VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR | VM_ENTRY_LOAD_IA32_EFER);
> +		(VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR | VM_ENTRY_LOAD_IA32_EFER |
> +		 VM_ENTRY_LOAD_GUEST_CET_STATE);

This is wrong, the OR path is only for emulated stuff, I'm guessing you're
not planning on emulating CET :-)

And I think this needs to be conditional based on supported_xss?
 
>  	/* We support free control of debug control loading. */
>  	msrs->entry_ctls_low &= ~VM_ENTRY_LOAD_DEBUG_CONTROLS;
> diff --git a/arch/x86/kvm/vmx/vmcs12.c b/arch/x86/kvm/vmx/vmcs12.c
> index 53dfb401316d..82b82bebeee0 100644
> --- a/arch/x86/kvm/vmx/vmcs12.c
> +++ b/arch/x86/kvm/vmx/vmcs12.c
> @@ -141,6 +141,9 @@ const unsigned short vmcs_field_to_offset_table[] = {
>  	FIELD(GUEST_PENDING_DBG_EXCEPTIONS, guest_pending_dbg_exceptions),
>  	FIELD(GUEST_SYSENTER_ESP, guest_sysenter_esp),
>  	FIELD(GUEST_SYSENTER_EIP, guest_sysenter_eip),
> +	FIELD(GUEST_S_CET, guest_s_cet),
> +	FIELD(GUEST_SSP, guest_ssp),
> +	FIELD(GUEST_INTR_SSP_TABLE, guest_ssp_tbl),
>  	FIELD(HOST_CR0, host_cr0),
>  	FIELD(HOST_CR3, host_cr3),
>  	FIELD(HOST_CR4, host_cr4),
> @@ -153,5 +156,8 @@ const unsigned short vmcs_field_to_offset_table[] = {
>  	FIELD(HOST_IA32_SYSENTER_EIP, host_ia32_sysenter_eip),
>  	FIELD(HOST_RSP, host_rsp),
>  	FIELD(HOST_RIP, host_rip),
> +	FIELD(HOST_S_CET, host_s_cet),
> +	FIELD(HOST_SSP, host_ssp),
> +	FIELD(HOST_INTR_SSP_TABLE, host_ssp_tbl),
>  };
>  const unsigned int nr_vmcs12_fields = ARRAY_SIZE(vmcs_field_to_offset_table);
> diff --git a/arch/x86/kvm/vmx/vmcs12.h b/arch/x86/kvm/vmx/vmcs12.h
> index d0c6df373f67..62b7be68f05c 100644
> --- a/arch/x86/kvm/vmx/vmcs12.h
> +++ b/arch/x86/kvm/vmx/vmcs12.h
> @@ -118,7 +118,13 @@ struct __packed vmcs12 {
>  	natural_width host_ia32_sysenter_eip;
>  	natural_width host_rsp;
>  	natural_width host_rip;
> -	natural_width paddingl[8]; /* room for future expansion */
> +	natural_width host_s_cet;
> +	natural_width host_ssp;
> +	natural_width host_ssp_tbl;
> +	natural_width guest_s_cet;
> +	natural_width guest_ssp;
> +	natural_width guest_ssp_tbl;
> +	natural_width paddingl[2]; /* room for future expansion */

Tangetial topic, it'd be helpful if FIELD and FIELD64 had compile-time
assertions similar to vmcs_read*() to verify the size of the vmcs12 field
is correct.  In other words, I don't feel like reviewing all of these :-).

>  	u32 pin_based_vm_exec_control;
>  	u32 cpu_based_vm_exec_control;
>  	u32 exception_bitmap;
> @@ -301,6 +307,12 @@ static inline void vmx_check_vmcs12_offsets(void)
>  	CHECK_OFFSET(host_ia32_sysenter_eip, 656);
>  	CHECK_OFFSET(host_rsp, 664);
>  	CHECK_OFFSET(host_rip, 672);
> +	CHECK_OFFSET(host_s_cet, 680);
> +	CHECK_OFFSET(host_ssp, 688);
> +	CHECK_OFFSET(host_ssp_tbl, 696);
> +	CHECK_OFFSET(guest_s_cet, 704);
> +	CHECK_OFFSET(guest_ssp, 712);
> +	CHECK_OFFSET(guest_ssp_tbl, 720);
>  	CHECK_OFFSET(pin_based_vm_exec_control, 744);
>  	CHECK_OFFSET(cpu_based_vm_exec_control, 748);
>  	CHECK_OFFSET(exception_bitmap, 752);
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index a3d01014b9e7..c2e950d378bd 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -7153,6 +7153,7 @@ static void nested_vmx_cr_fixed1_bits_update(struct kvm_vcpu *vcpu)
>  	cr4_fixed1_update(X86_CR4_PKE,        ecx, feature_bit(PKU));
>  	cr4_fixed1_update(X86_CR4_UMIP,       ecx, feature_bit(UMIP));
>  	cr4_fixed1_update(X86_CR4_LA57,       ecx, feature_bit(LA57));
> +	cr4_fixed1_update(X86_CR4_CET,	      ecx, feature_bit(SHSTK));
>  
>  #undef cr4_fixed1_update
>  }
> -- 
> 2.17.2
> 

Re: [PATCH v11 0/9] Introduce support for guest CET feature

[Yang Weijiang]

On Thu, Apr 23, 2020 at 08:51:09AM -0700, Sean Christopherson wrote:
> On Thu, Mar 26, 2020 at 04:18:37PM +0800, Yang Weijiang wrote:
> > Control-flow Enforcement Technology (CET) provides protection against
> > Return/Jump-Oriented Programming (ROP/JOP) attack. It includes two
> > sub-features: Shadow Stack (SHSTK) and Indirect Branch Tracking (IBT).
> > 
> > KVM needs to update to enable guest CET feature.
> > This patchset implements CET related CPUID/XSAVES enumeration, MSRs
> > and vmentry/vmexit configuration etc.so that guest kernel can setup CET
> > runtime infrastructure based on them. Some CET MSRs and related feature
> > flags used reference the definitions in kernel patchset.
> > 
> > CET kernel patches are here:
> > https://lkml.org/lkml/2020/2/5/593
> > https://lkml.org/lkml/2020/2/5/604
> 
> lkml.org is pretty worthless for this sort of thing, and lkml.kernel.org
> is the preferred link method in general.  The syntax is
> 
>   https://lkml.kernel.org/r/<Message-ID>
> 
> e.g.
> 
>   https://lkml.kernel.org/r/20200205181935.3712-1-yu-cheng.yu@intel.com
> 
> Note, that will redirect to lore.kernel.org, but the above format is
> preferred because it isn't dependent on binning the thread to a specific
> mailing list.
> 
> Anyways, kernel.org provides a link to download the entire thread in mbox
> format, which allows reviewers to get the prerequisite series without much
> fuss.
> 
>   https://lore.kernel.org/linux-api/20200205181935.3712-1-yu-cheng.yu@intel.com/t.mbox.gz
Thanks Sean for the detailed review!

I completely omitted such kind of consideration, will change it from
next version.

Re: [PATCH v11 0/9] Introduce support for guest CET feature

[Yang Weijiang]

On Thu, Apr 23, 2020 at 09:03:14AM -0700, Sean Christopherson wrote:
> On Thu, Mar 26, 2020 at 04:18:37PM +0800, Yang Weijiang wrote:
> > Control-flow Enforcement Technology (CET) provides protection against
> > Return/Jump-Oriented Programming (ROP/JOP) attack. It includes two
> > sub-features: Shadow Stack (SHSTK) and Indirect Branch Tracking (IBT).
> > 
> > KVM needs to update to enable guest CET feature.
> > This patchset implements CET related CPUID/XSAVES enumeration, MSRs
> > and vmentry/vmexit configuration etc.so that guest kernel can setup CET
> > runtime infrastructure based on them. Some CET MSRs and related feature
> > flags used reference the definitions in kernel patchset.
> > 
> > CET kernel patches are here:
> > https://lkml.org/lkml/2020/2/5/593
> > https://lkml.org/lkml/2020/2/5/604
> 
> ...
> 
> > - This patch serial is built on top of below branch and CET kernel patches
> >   for seeking xsaves support:
> >   https://git.kernel.org/pub/scm/virt/kvm/kvm.git/log/?h=cpu-caps
> 
> Can you provide the full code in a branch/tag somewhere?  The CET patches
> are in turn dependent on XSAVES enabling[*], and those don't apply cleanly
> on the cpu-caps branch.
> 
> It might make sense to also rebase to kvm/queue?  Though that's not a
> requirement by any means, e.g. don't bother if the CET patches are going to
> be respun soon.
>
I'll rebase the patches to 5.7-rc2, so things will be clear then.

> https://lkml.kernel.org/r/20200328164307.17497-1-yu-cheng.yu@intel.com

Re: [PATCH v11 1/9] KVM: VMX: Introduce CET VMX fields and flags

[Yang Weijiang]

On Thu, Apr 23, 2020 at 09:07:48AM -0700, Sean Christopherson wrote:
> On Thu, Mar 26, 2020 at 04:18:38PM +0800, Yang Weijiang wrote:
> > CET(Control-flow Enforcement Technology) is a CPU feature
> > used to prevent Return/Jump-Oriented Programming(ROP/JOP)
> > attacks. It provides the following sub-features to defend
> > against ROP/JOP style control-flow subversion attacks:
> 
> Changelogs should wrap at 75 characters.  Wrapping slightly earlier is ok,
> but wrapping at ~60 chars is too narrow.
>
Got it, thank you!

> > Shadow Stack (SHSTK):
> >   A second stack for program which is used exclusively for
> >   control transfer operations.
> > 

> >  /* Select x86 specific features in <linux/kvm.h> */
> >  #define __KVM_HAVE_PIT
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 40c6768942ae..830afe5038d1 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -186,6 +186,9 @@ static struct kvm_shared_msrs __percpu *shared_msrs;
> >  				| XFEATURE_MASK_BNDCSR | XFEATURE_MASK_AVX512 \
> >  				| XFEATURE_MASK_PKRU)
> >  
> > +#define KVM_SUPPORTED_XSS	(XFEATURE_MASK_CET_USER | \
> > +				 XFEATURE_MASK_CET_KERNEL)
> 
> This belongs in a later patch, KVM obviously doesn't support XSS.
>
OK, will change it.

> > +
> >  u64 __read_mostly host_efer;
> >  EXPORT_SYMBOL_GPL(host_efer);
> >  
> > @@ -402,6 +405,7 @@ static int exception_class(int vector)
> >  	case NP_VECTOR:
> >  	case SS_VECTOR:
> >  	case GP_VECTOR:
> > +	case CP_VECTOR:
> >  		return EXCPT_CONTRIBUTORY;
> >  	default:
> >  		break;
> > diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> > index c1954e216b41..8f0baa6fa72f 100644
> > --- a/arch/x86/kvm/x86.h
> > +++ b/arch/x86/kvm/x86.h
> > @@ -115,7 +115,7 @@ static inline bool x86_exception_has_error_code(unsigned int vector)
> >  {
> >  	static u32 exception_has_error_code = BIT(DF_VECTOR) | BIT(TS_VECTOR) |
> >  			BIT(NP_VECTOR) | BIT(SS_VECTOR) | BIT(GP_VECTOR) |
> > -			BIT(PF_VECTOR) | BIT(AC_VECTOR);
> > +			BIT(PF_VECTOR) | BIT(AC_VECTOR) | BIT(CP_VECTOR);
> >  
> >  	return (1U << vector) & exception_has_error_code;
> 
> Maybe it's gratuitous, but I feel like the #CP logic should be in a patch
> of its own, e.g. the changelog doesn't mention anything about #CP.
> 
My fault, will find a proper place to hold it. 

> >  }
> > -- 
> > 2.17.2
> > 

Re: [PATCH v11 1/9] KVM: VMX: Introduce CET VMX fields and flags

[Yang Weijiang]

On Thu, Apr 23, 2020 at 09:39:48AM -0700, Sean Christopherson wrote:
> On Thu, Mar 26, 2020 at 04:18:38PM +0800, Yang Weijiang wrote:
> > If VM_EXIT_LOAD_HOST_CET_STATE = 1, the host CET states are restored
> > from below VMCS fields at VM-Exit:
> >   HOST_S_CET
> >   HOST_SSP
> >   HOST_INTR_SSP_TABLE
> > 
> > If VM_ENTRY_LOAD_GUEST_CET_STATE = 1, the guest CET states are loaded
> > from below VMCS fields at VM-Entry:
> >   GUEST_S_CET
> >   GUEST_SSP
> >   GUEST_INTR_SSP_TABLE
> > 
> > Co-developed-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
> > Signed-off-by: Zhang Yi Z <yi.z.zhang@linux.intel.com>
> > Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> > ---
> 
> ...
> 
> > diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
> > index 5e090d1f03f8..e938bc6c37aa 100644
> > --- a/arch/x86/include/asm/vmx.h
> > +++ b/arch/x86/include/asm/vmx.h
> > @@ -94,6 +94,7 @@
> >  #define VM_EXIT_CLEAR_BNDCFGS                   0x00800000
> >  #define VM_EXIT_PT_CONCEAL_PIP			0x01000000
> >  #define VM_EXIT_CLEAR_IA32_RTIT_CTL		0x02000000
> > +#define VM_EXIT_LOAD_HOST_CET_STATE             0x10000000
> >  
> >  #define VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR	0x00036dff
> >  
> > @@ -107,6 +108,7 @@
> >  #define VM_ENTRY_LOAD_BNDCFGS                   0x00010000
> >  #define VM_ENTRY_PT_CONCEAL_PIP			0x00020000
> >  #define VM_ENTRY_LOAD_IA32_RTIT_CTL		0x00040000
> > +#define VM_ENTRY_LOAD_GUEST_CET_STATE           0x00100000
> 
> I think it probably make senses to drop HOST/GUEST from the controls,
> i.e. VM_{ENTER,EXIT}_LOAD_CET_STATE.  The SDM doesn't qualify them with
> guest vs. host, nor does KVM qualify any of the other entry/exit controls
> that are effective guest vs. host.
Sure, will fix them, thanks.

Re: [PATCH v11 2/9] KVM: VMX: Set guest CET MSRs per KVM and host configuration

[Yang Weijiang]

On Thu, Apr 23, 2020 at 09:27:49AM -0700, Sean Christopherson wrote:
> On Thu, Mar 26, 2020 at 04:18:39PM +0800, Yang Weijiang wrote:
> > @@ -3033,6 +3033,13 @@ void vmx_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3)
> >  		vmcs_writel(GUEST_CR3, guest_cr3);
> >  }
> >  
> > +static bool is_cet_mode_allowed(struct kvm_vcpu *vcpu, u32 mode_mask)
> 
> CET itself isn't a mode.  And since this ends up being an inner helper for
> is_cet_supported(), I think __is_cet_supported() would be the way to go.
> 
> Even @mode_mask is a bit confusing without the context of it being kernel
> vs. user.  The callers are very readable, e.g. I'd much prefer passing the
> mask as opposed to doing 'bool kernel'.  Maybe s/mode_mask/cet_mask?  That
> doesn't exactly make things super clear, but at least the reader knows the
> mask is for CET features.
Make sense, will change it.

> 
> > +{
> > +	return ((supported_xss & mode_mask) &&
> > +		(guest_cpuid_has(vcpu, X86_FEATURE_SHSTK) ||
> > +		guest_cpuid_has(vcpu, X86_FEATURE_IBT)));
> > +}
> > +
> >  int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
> >  {
> >  	struct vcpu_vmx *vmx = to_vmx(vcpu);
> > @@ -7064,6 +7071,35 @@ static void update_intel_pt_cfg(struct kvm_vcpu *vcpu)
> >  		vmx->pt_desc.ctl_bitmask &= ~(0xfULL << (32 + i * 4));
> >  }
> >  
> > +static void vmx_update_intercept_for_cet_msr(struct kvm_vcpu *vcpu)
> > +{
> > +	struct vcpu_vmx *vmx = to_vmx(vcpu);
> > +	unsigned long *msr_bitmap = vmx->vmcs01.msr_bitmap;
> > +	bool flag;
> 
> Maybe s/flag/incpt or something to make it more obvious that the bool is
> true if we want to intercept?  vmx_set_intercept_for_msr()s's @value isn't
> any better :-/.
I prefer using incpt now ;-) 
> > +
> > +	flag = !is_cet_mode_allowed(vcpu, XFEATURE_MASK_CET_USER);
> > +	/*
> > +	 * U_CET is required for USER CET, and U_CET, PL3_SPP are bound as
> > +	 * one component and controlled by IA32_XSS[bit 11].
> > +	 */
> > +	vmx_set_intercept_for_msr(msr_bitmap, MSR_IA32_U_CET, MSR_TYPE_RW, flag);
> > +	vmx_set_intercept_for_msr(msr_bitmap, MSR_IA32_PL3_SSP, MSR_TYPE_RW, flag);
> > +
> > +	flag = !is_cet_mode_allowed(vcpu, XFEATURE_MASK_CET_KERNEL);
> > +	/*
> > +	 * S_CET is required for KERNEL CET, and PL0_SSP ... PL2_SSP are
> > +	 * bound as one component and controlled by IA32_XSS[bit 12].
> > +	 */
> > +	vmx_set_intercept_for_msr(msr_bitmap, MSR_IA32_S_CET, MSR_TYPE_RW, flag);
> > +	vmx_set_intercept_for_msr(msr_bitmap, MSR_IA32_PL0_SSP, MSR_TYPE_RW, flag);
> > +	vmx_set_intercept_for_msr(msr_bitmap, MSR_IA32_PL1_SSP, MSR_TYPE_RW, flag);
> > +	vmx_set_intercept_for_msr(msr_bitmap, MSR_IA32_PL2_SSP, MSR_TYPE_RW, flag);
> > +
> > +	flag |= !guest_cpuid_has(vcpu, X86_FEATURE_SHSTK);
> > +	/* SSP_TAB is only available for KERNEL SHSTK.*/
> > +	vmx_set_intercept_for_msr(msr_bitmap, MSR_IA32_INT_SSP_TAB, MSR_TYPE_RW, flag);
> > +}
> > +
> >  static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
> >  {
> >  	struct vcpu_vmx *vmx = to_vmx(vcpu);
> > @@ -7102,6 +7138,10 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
> >  			vmx_set_guest_msr(vmx, msr, enabled ? 0 : TSX_CTRL_RTM_DISABLE);
> >  		}
> >  	}
> > +
> > +	if (guest_cpuid_has(vcpu, X86_FEATURE_SHSTK) ||
> > +	    guest_cpuid_has(vcpu, X86_FEATURE_IBT))
> > +		vmx_update_intercept_for_cet_msr(vcpu);
> 
> This is wrong, it will miss the case where userspace double configures CPUID
> and goes from CET=1 to CET=0.  This should instead be:
> 
> 	if (supported_xss & (XFEATURE_MASK_CET_KERNEL | XFEATURE_MASK_CET_USER))
> 		vmx_update_intercept_for_cet_msr(vcpu);
> 
> >  }
Here CET=1/0, did you mean the CET bit in XSS or CR4.CET? If it's the
former, then it's OK for me.

> >  
> >  static __init void vmx_set_cpu_caps(void)
> > -- 
> > 2.17.2
> > 

Re: [PATCH v11 9/9] KVM: X86: Set CET feature bits for CPUID enumeration

[Yang Weijiang]

On Thu, Apr 23, 2020 at 09:56:31AM -0700, Sean Christopherson wrote:
> On Thu, Mar 26, 2020 at 04:18:46PM +0800, Yang Weijiang wrote:
> > Set the feature bits so that CET capabilities can be seen
> > in guest via CPUID enumeration. Add CR4.CET bit support
> > in order to allow guest set CET master control bit(CR4.CET).
> > 
> > Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> > ---
> >  arch/x86/include/asm/kvm_host.h | 3 ++-
> >  arch/x86/kvm/cpuid.c            | 4 ++++
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index 2c944ad99692..5109c43c6981 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -95,7 +95,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)
> >  
> > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> > index 25e9a11291b3..26ab959df92f 100644
> > --- a/arch/x86/kvm/cpuid.c
> > +++ b/arch/x86/kvm/cpuid.c
> > @@ -366,6 +366,10 @@ void kvm_set_cpu_caps(void)
> >  		kvm_cpu_cap_set(X86_FEATURE_INTEL_STIBP);
> >  	if (boot_cpu_has(X86_FEATURE_AMD_SSBD))
> >  		kvm_cpu_cap_set(X86_FEATURE_SPEC_CTRL_SSBD);
> > +	if (boot_cpu_has(X86_FEATURE_IBT))
> > +		kvm_cpu_cap_set(X86_FEATURE_IBT);
> > +	if (boot_cpu_has(X86_FEATURE_SHSTK))
> > +		kvm_cpu_cap_set(X86_FEATURE_SHSTK);
> 
> This is the wrong way to advertise bits, the correct method is to declare
> the flag in the appriorate kvm_cpu_cap_mask() call.  The manually handling
> is only needed when the feature bit diverges from kernel support, either
> because KVM allow a feature based purely on hardware support, e.g. LA57, or
> when emulating a feature based on a different similar feature, e.g. the
> STIBP/SSBD flags above.
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 6828be99b908..6262438f9527 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -329,7 +329,8 @@ void kvm_set_cpu_caps(void)
>                 F(AVX512VBMI) | F(LA57) | 0 /*PKU*/ | 0 /*OSPKE*/ | F(RDPID) |
>                 F(AVX512_VPOPCNTDQ) | F(UMIP) | F(AVX512_VBMI2) | F(GFNI) |
>                 F(VAES) | F(VPCLMULQDQ) | F(AVX512_VNNI) | F(AVX512_BITALG) |
> -               F(CLDEMOTE) | F(MOVDIRI) | F(MOVDIR64B) | 0 /*WAITPKG*/
> +               F(CLDEMOTE) | F(MOVDIRI) | F(MOVDIR64B) | 0 /*WAITPKG*/ |
> +               F(SHSTK)
>         );
>         /* Set LA57 based on hardware capability. */
>         if (cpuid_ecx(7) & F(LA57))
> @@ -338,7 +339,7 @@ void kvm_set_cpu_caps(void)
>         kvm_cpu_cap_mask(CPUID_7_EDX,
>                 F(AVX512_4VNNIW) | F(AVX512_4FMAPS) | F(SPEC_CTRL) |
>                 F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES) | F(INTEL_STIBP) |
> -               F(MD_CLEAR) | F(AVX512_VP2INTERSECT) | F(FSRM)
> +               F(MD_CLEAR) | F(AVX512_VP2INTERSECT) | F(FSRM) | F(IBT)
>         );
>
Aah, thanks a lot for the explanation, will fix them.
> >  
> >  	kvm_cpu_cap_mask(CPUID_7_1_EAX,
> >  		F(AVX512_BF16)
> > -- 
> > 2.17.2
> > 

Re: [PATCH v11 9/9] KVM: X86: Set CET feature bits for CPUID enumeration

[Yang Weijiang]

On Thu, Apr 23, 2020 at 09:58:12AM -0700, Sean Christopherson wrote:
> For the shortlog, use something like
> 
>   KVM: x86: Enable CET virtualization and advertise CET to userspace
> 
> It took me a while to find the patch that actually allowed setting CR4.CET
> because I was only scanning the shortlogs :-)
I always feel it's hard for me to get a good shortlog, sorry for
the inconvenience :-(

Re: [PATCH v11 3/9] KVM: VMX: Set host/guest CET states for vmexit/vmentry

[Yang Weijiang]

On Thu, Apr 23, 2020 at 10:17:41AM -0700, Sean Christopherson wrote:
> On Thu, Mar 26, 2020 at 04:18:40PM +0800, Yang Weijiang wrote:
> > "Load {guest,host} CET state" bit controls whether guest/host
> > CET states will be loaded at VM entry/exit.
> > Set default host kernel CET states to 0s in VMCS to avoid guest
> > CET states leakage. When CR4.CET is cleared due to guest mode
> > change, make guest CET states invalid in VMCS, this can happen,
> > e.g., guest reboot.
> > 
> > Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> > ---
> >  arch/x86/kvm/vmx/capabilities.h | 10 ++++++
> >  arch/x86/kvm/vmx/vmx.c          | 56 +++++++++++++++++++++++++++++++--
> >  2 files changed, 63 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h
> > index 8903475f751e..565340352260 100644
> > --- a/arch/x86/kvm/vmx/capabilities.h
> > +++ b/arch/x86/kvm/vmx/capabilities.h
> > @@ -107,6 +107,16 @@ static inline bool cpu_has_vmx_mpx(void)
> >  		(vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_BNDCFGS);
> >  }
> >  
> > +static inline bool cpu_has_cet_guest_load_ctrl(void)
> > +{
> > +	return (vmcs_config.vmentry_ctrl & VM_ENTRY_LOAD_GUEST_CET_STATE);
> > +}
> > +
> > +static inline bool cpu_has_cet_host_load_ctrl(void)
> > +{
> > +	return (vmcs_config.vmexit_ctrl & VM_EXIT_LOAD_HOST_CET_STATE);
> > +}
> 
> We should bundle these together, same as we do for PERF_GLOBAL_CTRL.  Not
> just for code clarity, but also for functionality, e.g. if KVM ends up on a
> frankenstein CPU with VM_ENTRY_LOAD_CET_STATE and !VM_EXIT_LOAD_CET_STATE
> then KVM will end up running with guest state.  This is also an argument
> for not qualifying the control names with GUEST vs. HOST.
Cannot agree with you more! Thank you!
Will fix them.

> > +
> >  static inline bool cpu_has_vmx_tpr_shadow(void)
> >  {
> >  int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
> >  {
> >  	struct vcpu_vmx *vmx = to_vmx(vcpu);
> > @@ -3110,6 +3119,12 @@ int vmx_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
> >  			hw_cr4 &= ~(X86_CR4_SMEP | X86_CR4_SMAP | X86_CR4_PKE);
> >  	}
> >  
> > +	if (!(hw_cr4 & X86_CR4_CET) && is_cet_supported(vcpu)) {
> > +		vmcs_writel(GUEST_SSP, 0);
> > +		vmcs_writel(GUEST_S_CET, 0);
> > +		vmcs_writel(GUEST_INTR_SSP_TABLE, 0);
> 
> Can't we simply toggle the VM_{ENTRY,EXIT}_LOAD controls?  If CR4.CET=0 in
> the guest, then presumably keeping host state loaded is ok?  I.e. won't
> leak host information to the guest.
>
Yes, it's doable, let me change it.

> > +	}
> > +
> >  	vmcs_writel(CR4_READ_SHADOW, cr4);
> >  	vmcs_writel(GUEST_CR4, hw_cr4);
> >  	return 0;
> > @@ -3939,6 +3954,12 @@ void vmx_set_constant_host_state(struct vcpu_vmx *vmx)
> >  
> >  	if (cpu_has_load_ia32_efer())
> >  		vmcs_write64(HOST_IA32_EFER, host_efer);
> > +
> > +	if (cpu_has_cet_host_load_ctrl()) {
> > +		vmcs_writel(HOST_S_CET, 0);
> > +		vmcs_writel(HOST_INTR_SSP_TABLE, 0);
> > +		vmcs_writel(HOST_SSP, 0);
> 
> This is unnecessary, the VMCS is zeroed on allocation.
> 
> And IIUC, this is only correct because the main shadow stack enabling only
> adds user support.  Assuming that's correct, (a) it absolutely needs to be
> called out in the changelog and (b) KVM needs a WARN in hardware_setup() to
> guard against kernel shadow stack support being added without updating KVM,
> e.g. something this (not sure the MSRs are correct):
> 
> 	if (boot_cpu_has(X86_FEATURE_IBT) || boot_cpu_has(X86_FEATURE_SHSTK)) {
> 		rdmsrl(MSR_IA32_S_CET, cet_msr);
> 		WARN_ONCE(cet_msr, "KVM: S_CET in host will be lost");
> 
> 	}
> 	if (boot_cpu_has(X86_FEATURE_SHSTK)) {
> 		rdmsrl(MSR_IA32_PL0_SSP, cet_msr);
> 		WARN_ONCE(cet_msr, "KVM: PL0_SPP in host will be lost");
> 	}
> 
Yes, just wanted to make host CET stuffs invalid before kernel mode
CET is available. OK, will follow your advice, thanks!

> > +	}
> >  }

> >  /*
> > @@ -7140,8 +7175,23 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
> >  	}
> >  
> >  	if (guest_cpuid_has(vcpu, X86_FEATURE_SHSTK) ||
> > -	    guest_cpuid_has(vcpu, X86_FEATURE_IBT))
> > +	    guest_cpuid_has(vcpu, X86_FEATURE_IBT)) {
> >  		vmx_update_intercept_for_cet_msr(vcpu);
> > +
> > +		if (cpu_has_cet_guest_load_ctrl() && is_cet_supported(vcpu))
> > +			vm_entry_controls_setbit(to_vmx(vcpu),
> > +						 VM_ENTRY_LOAD_GUEST_CET_STATE);
> > +		else
> > +			vm_entry_controls_clearbit(to_vmx(vcpu),
> > +						   VM_ENTRY_LOAD_GUEST_CET_STATE);
> > +
> > +		if (cpu_has_cet_host_load_ctrl() && is_cet_supported(vcpu))
> > +			vm_exit_controls_setbit(to_vmx(vcpu),
> > +						VM_EXIT_LOAD_HOST_CET_STATE);
> > +		else
> > +			vm_exit_controls_clearbit(to_vmx(vcpu),
> > +						  VM_EXIT_LOAD_HOST_CET_STATE);
> 
> As above, I think this can be done in vmx_set_cr4().
>
Hmm, it's in vmx_set_cr4() in early versions, OK, will move them back.

> > +	}
> >  }
> >  
> >  static __init void vmx_set_cpu_caps(void)
> > -- 
> > 2.17.2
> > 

Re: [PATCH v11 4/9] KVM: VMX: Check CET dependencies on CR settings

[Yang Weijiang]

On Thu, Apr 23, 2020 at 10:20:32AM -0700, Sean Christopherson wrote:
> On Thu, Mar 26, 2020 at 04:18:41PM +0800, Yang Weijiang wrote:
> > CR4.CET is master control bit for CET function.
> > There're mutual constrains between CR0.WP and CR4.CET, so need
> > to check the dependent bit while changing the control registers.
> > 
> > Note:
> > 1)The processor does not allow CR4.CET to be set if CR0.WP = 0,
> >   similarly, it does not allow CR0.WP to be cleared while
> >   CR4.CET = 1. In either case, KVM would inject #GP to guest.
> 
> Nit: the CET vs. WP dependency and #GP belongs in the "main" part of the
> changelog, as it's the crux of the patch.  Item (2) below is more along
> the lines of "note" material.
>
OK, will change it, thank you!
> > 
> > 2)SHSTK and IBT features share one control MSR:
> >   MSR_IA32_{U,S}_CET, which means it's difficult to hide
> >   one feature from another in the case of SHSTK != IBT,
> >   after discussed in community, it's agreed to allow guest
> >   control two features independently as it won't introduce
> >   security hole.
> > 
 > 

Re: [PATCH v11 5/9] KVM: X86: Refresh CPUID once guest XSS MSR changes

[Yang Weijiang]

On Thu, Apr 23, 2020 at 10:34:50AM -0700, Sean Christopherson wrote:
> On Thu, Mar 26, 2020 at 04:18:42PM +0800, Yang Weijiang wrote:
> > CPUID(0xd, 1) reports the current required storage size of
 
> >  	struct kvm_pio_request pio;
> > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> > index 78d461be2102..25e9a11291b3 100644
> > --- a/arch/x86/kvm/cpuid.c
> > +++ b/arch/x86/kvm/cpuid.c
> > @@ -95,9 +95,24 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu)
> >  	}
> >  
> >  	best = kvm_find_cpuid_entry(vcpu, 0xD, 1);
> > -	if (best && (cpuid_entry_has(best, X86_FEATURE_XSAVES) ||
> > -		     cpuid_entry_has(best, X86_FEATURE_XSAVEC)))
> > -		best->ebx = xstate_required_size(vcpu->arch.xcr0, true);
> > +	if (best) {
> > +		if (best->eax & (F(XSAVES) | F(XSAVEC))) {
> 
> Please use cpuid_entry_has() to preserve the automagic register lookup and
> compile-time assertions that are provided.  E.g. I don't know off the top
> of my whether %eax is the correct register, and I don't want to know :-).
>
Got it, will fix it.

> > +			u64 xstate = vcpu->arch.xcr0 | vcpu->arch.ia32_xss;
> > +
> > +			best->ebx = xstate_required_size(xstate, true);
> > +		}
> > +
> > +		if (best->eax & F(XSAVES)) {
> 
> Same thing here.
> 
> > +			vcpu->arch.guest_supported_xss =
> > +			(best->ecx | ((u64)best->edx << 32)) & supported_xss;
> 
> The indentation is funky, I'm guessing you're trying to squeak in less than
> 80 chars.  Maybe this?
> 
> 		if (!cpuid_entry_has(best, X86_FEATURE_XSAVES)) {
> 			best->ecx = 0;
> 			best->edx = 0;
> 		}
> 
> 		 vcpu->arch.guest_supported_xss =
> 			(((u64)best->edx << 32) | best->ecx) & supported_xss;
> 
> Nit: my preference is to have the high half first, x86 is little endian
> (the xcr0 code is "wrong" :-D).  For me, this also makes it more obvious
> that the effective size is a u64.
>
Good suggestion, will fixed it together the xcr0 part!

> > +		} else {
> > +			best->ecx = 0;
> > +			best->edx = 0;
> > +			vcpu->arch.guest_supported_xss = 0;
> > +		}
> > +	} else {
> > +		vcpu->arch.guest_supported_xss = 0;
> > +	}
> >  
> >  	/*
> >  	 * The existing code assumes virtual address is 48-bit or 57-bit in the
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 90acdbbb8a5a..51ecb496d47d 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -2836,9 +2836,12 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >  		 * IA32_XSS[bit 8]. Guests have to use RDMSR/WRMSR rather than
> >  		 * XSAVES/XRSTORS to save/restore PT MSRs.
> >  		 */
> > -		if (data & ~supported_xss)
> > +		if (data & ~vcpu->arch.guest_supported_xss)
> >  			return 1;
> > -		vcpu->arch.ia32_xss = data;
> > +		if (vcpu->arch.ia32_xss != data) {
> > +			vcpu->arch.ia32_xss = data;
> > +			kvm_update_cpuid(vcpu);
> > +		}
> >  		break;
> >  	case MSR_SMI_COUNT:
> >  		if (!msr_info->host_initiated)
> > @@ -9635,6 +9638,8 @@ int kvm_arch_hardware_setup(void)
> >  
> >  	if (!kvm_cpu_cap_has(X86_FEATURE_XSAVES))
> >  		supported_xss = 0;
> > +	else
> > +		supported_xss = host_xss & KVM_SUPPORTED_XSS;
> 
> Silly nit: I'd prefer to invert the check, e.g.
> 
> 	if (kvm_cpu_cap_has(X86_FEATURE_XSAVES))
> 		supported_xss = host_xss & KVM_SUPPORTED_XSS;
> 	else
> 		supported_xss = 0;
Fair enough!
> 
> >  
> >  	cr4_reserved_bits = kvm_host_cr4_reserved_bits(&boot_cpu_data);
> >  
> > -- 
> > 2.17.2
> > 

Re: [PATCH v11 3/9] KVM: VMX: Set host/guest CET states for vmexit/vmentry

[Sean Christopherson]

On Fri, Apr 24, 2020 at 10:35:10PM +0800, Yang Weijiang wrote:
> On Thu, Apr 23, 2020 at 10:17:41AM -0700, Sean Christopherson wrote:
> > On Thu, Mar 26, 2020 at 04:18:40PM +0800, Yang Weijiang wrote:
> > > @@ -7140,8 +7175,23 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
> > >  	}
> > >  
> > >  	if (guest_cpuid_has(vcpu, X86_FEATURE_SHSTK) ||
> > > -	    guest_cpuid_has(vcpu, X86_FEATURE_IBT))
> > > +	    guest_cpuid_has(vcpu, X86_FEATURE_IBT)) {
> > >  		vmx_update_intercept_for_cet_msr(vcpu);
> > > +
> > > +		if (cpu_has_cet_guest_load_ctrl() && is_cet_supported(vcpu))
> > > +			vm_entry_controls_setbit(to_vmx(vcpu),
> > > +						 VM_ENTRY_LOAD_GUEST_CET_STATE);
> > > +		else
> > > +			vm_entry_controls_clearbit(to_vmx(vcpu),
> > > +						   VM_ENTRY_LOAD_GUEST_CET_STATE);
> > > +
> > > +		if (cpu_has_cet_host_load_ctrl() && is_cet_supported(vcpu))
> > > +			vm_exit_controls_setbit(to_vmx(vcpu),
> > > +						VM_EXIT_LOAD_HOST_CET_STATE);
> > > +		else
> > > +			vm_exit_controls_clearbit(to_vmx(vcpu),
> > > +						  VM_EXIT_LOAD_HOST_CET_STATE);
> > 
> > As above, I think this can be done in vmx_set_cr4().
> >
> Hmm, it's in vmx_set_cr4() in early versions, OK, will move them back.

Did I advise you to move them out of vmx_set_cr4()?  It's entirely possible
I forgot some detail since the last time I reviewed this series.

Re: [PATCH v11 2/9] KVM: VMX: Set guest CET MSRs per KVM and host configuration

[Sean Christopherson]

On Fri, Apr 24, 2020 at 10:07:51PM +0800, Yang Weijiang wrote:
> On Thu, Apr 23, 2020 at 09:27:49AM -0700, Sean Christopherson wrote:
> > On Thu, Mar 26, 2020 at 04:18:39PM +0800, Yang Weijiang wrote:
> > > @@ -7102,6 +7138,10 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
> > >  			vmx_set_guest_msr(vmx, msr, enabled ? 0 : TSX_CTRL_RTM_DISABLE);
> > >  		}
> > >  	}
> > > +
> > > +	if (guest_cpuid_has(vcpu, X86_FEATURE_SHSTK) ||
> > > +	    guest_cpuid_has(vcpu, X86_FEATURE_IBT))
> > > +		vmx_update_intercept_for_cet_msr(vcpu);
> > 
> > This is wrong, it will miss the case where userspace double configures CPUID
> > and goes from CET=1 to CET=0.  This should instead be:
> > 
> > 	if (supported_xss & (XFEATURE_MASK_CET_KERNEL | XFEATURE_MASK_CET_USER))
> > 		vmx_update_intercept_for_cet_msr(vcpu);
> > 
> > >  }
> Here CET=1/0, did you mean the CET bit in XSS or CR4.CET? If it's the
> former, then it's OK for me.

The former, i.e. update the CET MSRs if KVM supports CET virtualization and
the guest's CPUID configuration is changing.

Re: [PATCH v11 7/9] KVM: X86: Add userspace access interface for CET MSRs

[Yang Weijiang]

On Thu, Apr 23, 2020 at 11:14:06AM -0700, Sean Christopherson wrote:
> On Thu, Mar 26, 2020 at 04:18:44PM +0800, Yang Weijiang wrote:
> > +#define CET_MSR_RSVD_BITS_1  GENMASK(1, 0)
> > +#define CET_MSR_RSVD_BITS_2  GENMASK(9, 6)
> > +
> > +static bool cet_check_msr_write(struct kvm_vcpu *vcpu,
> 
> s/cet_check_msr_write/is_cet_msr_valid
> 
> Otherwise the polarity of the return value isn't obvious.
> 
> > +				struct msr_data *msr,
> 
> Unnecessary newline.
> 
> > +				u64 mask)
> 
> s/mask/rsvd_bits
>
Sure, will change them, thank you!

> > +{
> > +	u64 data = msr->data;
> > +	u32 high_word = data >> 32;
> > +
> > +	if (data & mask)
> > +		return false;
> > +
> > +	if (!is_64_bit_mode(vcpu) && high_word)
> > +		return false;
> 
> As I called out before, this is wrong.  AFAIK, the CPU never depends on
> WRMSR to prevent loading bits 63:32, software can simply do WRMSR and then
> transition back to 32-bit mode.  Yes, the shadow stack itself is 32 bits,
> but the internal value is still 64 bits.  This is backed up by the CALL
> pseudocode:
>
So I'll remove this invalid check, thanks for the comments!

>   IF ShadowStackEnabled(CPL)
>     IF (EFER.LMA and DEST(CodeSegmentSelector).L) = 0
>       (* If target is legacy or compatibility mode then the SSP must be in low 4GB *)
>       IF (SSP & 0xFFFFFFFF00000000 != 0)
>         THEN #GP(0); FI;
>   FI;
> 
> as well as RDSSP:
> 
>   IF CPL = 3
>     IF CR4.CET & IA32_U_CET.SH_STK_EN
>       IF (operand size is 64 bit)
>         THEN
>           Dest ← SSP;
>         ELSE
>           Dest ← SSP[31:0];
>       FI;
>     FI;
>   ELSE
> 
> > +
> > +	return true;
> > +}
> > +
> > +static bool cet_check_ssp_msr_access(struct kvm_vcpu *vcpu,
> > +				     struct msr_data *msr)
> 
> Similar to above, the polarity of the return isn't obvious.  Maybe
> is_cet_ssp_msr_accessible()?
> 
> I'd prefer to pass in @index, passing the full @msr makes it look like
> this helper might also check msr->data.
>
Sure, will follow it.

> > +{
> > +	u32 index = msr->index;
> > +
> > +	if (!boot_cpu_has(X86_FEATURE_SHSTK))
> > +		return false;
> > +
> > +	if (!msr->host_initiated &&
> > +	    !guest_cpuid_has(vcpu, X86_FEATURE_SHSTK))
> > +		return false;
> > +
> > +	if (index == MSR_IA32_INT_SSP_TAB)
> > +		return true;
> > +
> > +	if (index == MSR_IA32_PL3_SSP) {
> > +		if (!(supported_xss & XFEATURE_MASK_CET_USER))
> > +			return false;
> > +	} else if (!(supported_xss & XFEATURE_MASK_CET_KERNEL)) {
> > +		return false;
> > +	}
> 
> 	if (index == MSR_IA32_PL3_SSP)
> 		return supported_xss & XFEATURE_MASK_CET_USER;
> 
> 	/* MSR_IA32_PL[0-2]_SSP */
> 	return supported_xss & XFEATURE_MASK_CET_KERNEL;
Nice! ;-))

> > +
> > +	return true;
> > +}
> > +
> > +static bool cet_check_ctl_msr_access(struct kvm_vcpu *vcpu,
> 
> is_cet_ctl_msr_accessible?
> 
OK.

> > +				     struct msr_data *msr)
> > +{
> > +	u32 index = msr->index;
> > +
> > +	if (!boot_cpu_has(X86_FEATURE_SHSTK) &&
> > +	    !boot_cpu_has(X86_FEATURE_IBT))
> > +		return false;
> > +
> > +	if (!msr->host_initiated &&
> > +	    !guest_cpuid_has(vcpu, X86_FEATURE_SHSTK) &&
> > +	    !guest_cpuid_has(vcpu, X86_FEATURE_IBT))
> > +		return false;
> > +
> > +	if (index == MSR_IA32_U_CET) {
> > +		if (!(supported_xss & XFEATURE_MASK_CET_USER))
> > +			return false;
> > +	} else if (!(supported_xss & XFEATURE_MASK_CET_KERNEL)) {
> > +		return false;
> > +	}
> 
> Same as above:
> 
> 	if (index == MSR_IA32_U_CET)
> 		return supported_xss & XFEATURE_MASK_CET_USER;
> 
> 	return supported_xss & XFEATURE_MASK_CET_KERNEL;
Got it!

> > +
> > +	return true;
> > +}
> >  /*
> >   * Reads an msr value (of 'msr_index') into 'pdata'.
> >   * Returns 0 on success, non-0 otherwise.
> > @@ -1941,6 +2026,26 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >  		else
> >  			msr_info->data = vmx->pt_desc.guest.addr_a[index / 2];
> >  		break;
> > +	case MSR_IA32_S_CET:
> > +		if (!cet_check_ctl_msr_access(vcpu, msr_info))
> > +			return 1;
> > +		msr_info->data = vmcs_readl(GUEST_S_CET);
> > +		break;
> > +	case MSR_IA32_INT_SSP_TAB:
> > +		if (!cet_check_ssp_msr_access(vcpu, msr_info))
> > +			return 1;
> > +		msr_info->data = vmcs_readl(GUEST_INTR_SSP_TABLE);
> > +		break;
> > +	case MSR_IA32_U_CET:
> > +		if (!cet_check_ctl_msr_access(vcpu, msr_info))
> > +			return 1;
> > +		vmx_get_xsave_msr(msr_info);
> > +		break;
> > +	case MSR_IA32_PL0_SSP ... MSR_IA32_PL3_SSP:
> > +		if (!cet_check_ssp_msr_access(vcpu, msr_info))
> > +			return 1;
> > +		vmx_get_xsave_msr(msr_info);
> > +		break;
> >  	case MSR_TSC_AUX:
> >  		if (!msr_info->host_initiated &&
> >  		    !guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP))
> > @@ -2197,6 +2302,34 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> >  		else
> >  			vmx->pt_desc.guest.addr_a[index / 2] = data;
> >  		break;
> > +	case MSR_IA32_S_CET:
> > +		if (!cet_check_ctl_msr_access(vcpu, msr_info))
> > +			return 1;
> > +		if (!cet_check_msr_write(vcpu, msr_info, CET_MSR_RSVD_BITS_2))
> > +			return 1;
> > +		vmcs_writel(GUEST_S_CET, data);
> > +		break;
> > +	case MSR_IA32_INT_SSP_TAB:
> > +		if (!cet_check_ctl_msr_access(vcpu, msr_info))
> > +			return 1;
> > +		if (!is_64_bit_mode(vcpu))
> 
> This is wrong, the SDM explicitly calls out the !64 case:
> 
>   IA32_INTERRUPT_SSP_TABLE_ADDR (64 bits; 32 bits on processors that do not
>   support Intel 64 architecture).
So the check is also unnecessary as it's natual size?
> 
> > +			return 1;
> > +		vmcs_writel(GUEST_INTR_SSP_TABLE, data);
> > +		break;
> > +	case MSR_IA32_U_CET:
> > +		if (!cet_check_ctl_msr_access(vcpu, msr_info))
> > +			return 1;
> > +		if (!cet_check_msr_write(vcpu, msr_info, CET_MSR_RSVD_BITS_2))
> > +			return 1;
> > +		vmx_set_xsave_msr(msr_info);
> > +		break;
> > +	case MSR_IA32_PL0_SSP ... MSR_IA32_PL3_SSP:
> > +		if (!cet_check_ssp_msr_access(vcpu, msr_info))
> > +			return 1;
> > +		if (!cet_check_msr_write(vcpu, msr_info, CET_MSR_RSVD_BITS_1))
> > +			return 1;
> > +		vmx_set_xsave_msr(msr_info);
> > +		break;
> >  	case MSR_TSC_AUX:
> >  		if (!msr_info->host_initiated &&
> >  		    !guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP))
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 9654d779bdab..9e89ee6a09e1 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -1229,6 +1229,10 @@ static const u32 msrs_to_save_all[] = {
> >  	MSR_ARCH_PERFMON_EVENTSEL0 + 12, MSR_ARCH_PERFMON_EVENTSEL0 + 13,
> >  	MSR_ARCH_PERFMON_EVENTSEL0 + 14, MSR_ARCH_PERFMON_EVENTSEL0 + 15,
> >  	MSR_ARCH_PERFMON_EVENTSEL0 + 16, MSR_ARCH_PERFMON_EVENTSEL0 + 17,
> > +
> > +	MSR_IA32_XSS, MSR_IA32_U_CET, MSR_IA32_S_CET,
> > +	MSR_IA32_PL0_SSP, MSR_IA32_PL1_SSP, MSR_IA32_PL2_SSP,
> > +	MSR_IA32_PL3_SSP, MSR_IA32_INT_SSP_TAB,
> >  };
> >  
> >  static u32 msrs_to_save[ARRAY_SIZE(msrs_to_save_all)];
> > @@ -1504,6 +1508,13 @@ static int __kvm_set_msr(struct kvm_vcpu *vcpu, u32 index, u64 data,
> >  		 * invokes 64-bit SYSENTER.
> >  		 */
> >  		data = get_canonical(data, vcpu_virt_addr_bits(vcpu));
> > +		break;
> > +	case MSR_IA32_PL0_SSP ... MSR_IA32_PL3_SSP:
> > +	case MSR_IA32_U_CET:
> > +	case MSR_IA32_S_CET:
> > +	case MSR_IA32_INT_SSP_TAB:
> > +		if (is_noncanonical_address(data, vcpu))
> 
> IMO the canonical check belongs in cet_check_msr_write().  The above checks
> are for MSRs that are common to VMX and SVM, i.e. the common check saves
> having to duplicate the logic.  If SVM picks up CET support, then they'll
> presumably want to share all of the checks, not just the canonical piece.
OK, I'll move them back.
> 
> > +			return 1;
> >  	}
> >  
> >  	msr.data = data;
> > -- 
> > 2.17.2
> > 

Re: [PATCH v11 7/9] KVM: X86: Add userspace access interface for CET MSRs

[Sean Christopherson]

On Fri, Apr 24, 2020 at 11:02:46PM +0800, Yang Weijiang wrote:
> On Thu, Apr 23, 2020 at 11:14:06AM -0700, Sean Christopherson wrote:
> > > +	case MSR_IA32_INT_SSP_TAB:
> > > +		if (!cet_check_ctl_msr_access(vcpu, msr_info))
> > > +			return 1;
> > > +		if (!is_64_bit_mode(vcpu))
> > 
> > This is wrong, the SDM explicitly calls out the !64 case:
> > 
> >   IA32_INTERRUPT_SSP_TABLE_ADDR (64 bits; 32 bits on processors that do not
> >   support Intel 64 architecture).
> So the check is also unnecessary as it's natual size?

It still needs a canonical check.

Note, KVM diverges from the SDM for canonical checks in that it performs
canonical checks even when the virtual CPU doesn't support 64-bit and/or
the host kernel is a 32-bit kernel.  This is intentional because the
underlying hardware will still enforce the checks, i.e. KVM needs to make
the physical CPU happy, and the number of people running KVM on hardware
without 64-bit support can probably be counted on one hand.

Re: [PATCH v11 8/9] KVM: VMX: Enable CET support for nested VM

[Yang Weijiang]

On Thu, Apr 23, 2020 at 11:29:06AM -0700, Sean Christopherson wrote:
> On Thu, Mar 26, 2020 at 04:18:45PM +0800, Yang Weijiang wrote:
> > CET MSRs pass through guests for performance consideration.
> > Configure the MSRs to match L0/L1 settings so that nested VM
> > is able to run with CET.
> > 
> > Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
> > ---
> >  arch/x86/kvm/vmx/nested.c | 41 +++++++++++++++++++++++++++++++++++++--
> >  arch/x86/kvm/vmx/vmcs12.c |  6 ++++++
> >  arch/x86/kvm/vmx/vmcs12.h | 14 ++++++++++++-
> >  arch/x86/kvm/vmx/vmx.c    |  1 +
> >  4 files changed, 59 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> > index e47eb7c0fbae..a71ef33de55f 100644
> > --- a/arch/x86/kvm/vmx/nested.c
> > +++ b/arch/x86/kvm/vmx/nested.c
> > @@ -627,6 +627,41 @@ static inline bool nested_vmx_prepare_msr_bitmap(struct kvm_vcpu *vcpu,
> >  	nested_vmx_disable_intercept_for_msr(msr_bitmap_l1, msr_bitmap_l0,
> >  					     MSR_KERNEL_GS_BASE, MSR_TYPE_RW);
> >  
> > +	/* Pass CET MSRs to nested VM if L0 and L1 are set to pass-through. */
> > +	if (!msr_write_intercepted_l01(vcpu, MSR_IA32_U_CET))
> > +		nested_vmx_disable_intercept_for_msr(
> > +					msr_bitmap_l1, msr_bitmap_l0,
> > +					MSR_IA32_U_CET, MSR_TYPE_RW);
> > +
> > +	if (!msr_write_intercepted_l01(vcpu, MSR_IA32_PL3_SSP))
> > +		nested_vmx_disable_intercept_for_msr(
> > +					msr_bitmap_l1, msr_bitmap_l0,
> > +					MSR_IA32_PL3_SSP, MSR_TYPE_RW);
> > +
> > +	if (!msr_write_intercepted_l01(vcpu, MSR_IA32_S_CET))
> > +		nested_vmx_disable_intercept_for_msr(
> > +					msr_bitmap_l1, msr_bitmap_l0,
> > +					MSR_IA32_S_CET, MSR_TYPE_RW);
> > +
> > +	if (!msr_write_intercepted_l01(vcpu, MSR_IA32_PL0_SSP))
> > +		nested_vmx_disable_intercept_for_msr(
> > +					msr_bitmap_l1, msr_bitmap_l0,
> > +					MSR_IA32_PL0_SSP, MSR_TYPE_RW);
> > +
> > +	if (!msr_write_intercepted_l01(vcpu, MSR_IA32_PL1_SSP))
> > +		nested_vmx_disable_intercept_for_msr(
> > +					msr_bitmap_l1, msr_bitmap_l0,
> > +					MSR_IA32_PL1_SSP, MSR_TYPE_RW);
> > +
> > +	if (!msr_write_intercepted_l01(vcpu, MSR_IA32_PL2_SSP))
> > +		nested_vmx_disable_intercept_for_msr(
> > +					msr_bitmap_l1, msr_bitmap_l0,
> > +					MSR_IA32_PL2_SSP, MSR_TYPE_RW);
> > +
> > +	if (!msr_write_intercepted_l01(vcpu, MSR_IA32_INT_SSP_TAB))
> > +		nested_vmx_disable_intercept_for_msr(
> > +					msr_bitmap_l1, msr_bitmap_l0,
> > +					MSR_IA32_INT_SSP_TAB, MSR_TYPE_RW);
> 
> That's a lot of copy-paste.  Maybe add a helper to do the conditional l01
> check and subsequent call to nested_vmx_disable_intercept_for_msr()?  It's
> still a lot of boilerplate, but it's at least a little better.  Not sure
> what a good name would be.
> 
> 	nested_vmx_update_intercept_for_msr(vcpu, MSR_IA32_U_CET,
> 					    msr_bitmap_l1, msr_bitmap_l0,
> 					    MSR_TYPE_RW);
>
OK, I'll figure out how to make it nicer :-)

> 
> >  	/*
> >  	 * Checking the L0->L1 bitmap is trying to verify two things:
> >  	 *
> > @@ -6040,7 +6075,8 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
> >  	msrs->exit_ctls_high |=
> >  		VM_EXIT_ALWAYSON_WITHOUT_TRUE_MSR |
> >  		VM_EXIT_LOAD_IA32_EFER | VM_EXIT_SAVE_IA32_EFER |
> > -		VM_EXIT_SAVE_VMX_PREEMPTION_TIMER | VM_EXIT_ACK_INTR_ON_EXIT;
> > +		VM_EXIT_SAVE_VMX_PREEMPTION_TIMER | VM_EXIT_ACK_INTR_ON_EXIT |
> > +		VM_EXIT_LOAD_HOST_CET_STATE;
> >  
> >  	/* We support free control of debug control saving. */
> >  	msrs->exit_ctls_low &= ~VM_EXIT_SAVE_DEBUG_CONTROLS;
> > @@ -6057,7 +6093,8 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
> >  #endif
> >  		VM_ENTRY_LOAD_IA32_PAT;
> >  	msrs->entry_ctls_high |=
> > -		(VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR | VM_ENTRY_LOAD_IA32_EFER);
> > +		(VM_ENTRY_ALWAYSON_WITHOUT_TRUE_MSR | VM_ENTRY_LOAD_IA32_EFER |
> > +		 VM_ENTRY_LOAD_GUEST_CET_STATE);
> 
> This is wrong, the OR path is only for emulated stuff, I'm guessing you're
> not planning on emulating CET :-)
When I tested nested VM, if it's not "ORed" here, nested VM entry will
fail, so what's your suggestion on this?
> 
> And I think this needs to be conditional based on supported_xss?
Yes, a check is necessary.

>  
> >  	/* We support free control of debug control loading. */
> >  	msrs->entry_ctls_low &= ~VM_ENTRY_LOAD_DEBUG_CONTROLS;
> > diff --git a/arch/x86/kvm/vmx/vmcs12.c b/arch/x86/kvm/vmx/vmcs12.c
> > index 53dfb401316d..82b82bebeee0 100644
> > --- a/arch/x86/kvm/vmx/vmcs12.c
> > +++ b/arch/x86/kvm/vmx/vmcs12.c
> > @@ -141,6 +141,9 @@ const unsigned short vmcs_field_to_offset_table[] = {
> >  	FIELD(GUEST_PENDING_DBG_EXCEPTIONS, guest_pending_dbg_exceptions),
> >  	FIELD(GUEST_SYSENTER_ESP, guest_sysenter_esp),
> >  	FIELD(GUEST_SYSENTER_EIP, guest_sysenter_eip),
> > +	FIELD(GUEST_S_CET, guest_s_cet),
> > +	FIELD(GUEST_SSP, guest_ssp),
> > +	FIELD(GUEST_INTR_SSP_TABLE, guest_ssp_tbl),
> >  	FIELD(HOST_CR0, host_cr0),
> >  	FIELD(HOST_CR3, host_cr3),
> >  	FIELD(HOST_CR4, host_cr4),
> > @@ -153,5 +156,8 @@ const unsigned short vmcs_field_to_offset_table[] = {
> >  	FIELD(HOST_IA32_SYSENTER_EIP, host_ia32_sysenter_eip),
> >  	FIELD(HOST_RSP, host_rsp),
> >  	FIELD(HOST_RIP, host_rip),
> > +	FIELD(HOST_S_CET, host_s_cet),
> > +	FIELD(HOST_SSP, host_ssp),
> > +	FIELD(HOST_INTR_SSP_TABLE, host_ssp_tbl),
> >  };
> >  const unsigned int nr_vmcs12_fields = ARRAY_SIZE(vmcs_field_to_offset_table);
> > diff --git a/arch/x86/kvm/vmx/vmcs12.h b/arch/x86/kvm/vmx/vmcs12.h
> > index d0c6df373f67..62b7be68f05c 100644
> > --- a/arch/x86/kvm/vmx/vmcs12.h
> > +++ b/arch/x86/kvm/vmx/vmcs12.h
> > @@ -118,7 +118,13 @@ struct __packed vmcs12 {
> >  	natural_width host_ia32_sysenter_eip;
> >  	natural_width host_rsp;
> >  	natural_width host_rip;
> > -	natural_width paddingl[8]; /* room for future expansion */
> > +	natural_width host_s_cet;
> > +	natural_width host_ssp;
> > +	natural_width host_ssp_tbl;
> > +	natural_width guest_s_cet;
> > +	natural_width guest_ssp;
> > +	natural_width guest_ssp_tbl;
> > +	natural_width paddingl[2]; /* room for future expansion */
> 
> Tangetial topic, it'd be helpful if FIELD and FIELD64 had compile-time
> assertions similar to vmcs_read*() to verify the size of the vmcs12 field
> is correct.  In other words, I don't feel like reviewing all of these :-).
>
OK, let me figure out how to polish them...

> >  	u32 pin_based_vm_exec_control;
> >  	u32 cpu_based_vm_exec_control;
> >  	u32 exception_bitmap;
> > @@ -301,6 +307,12 @@ static inline void vmx_check_vmcs12_offsets(void)
> >  	CHECK_OFFSET(host_ia32_sysenter_eip, 656);
> >  	CHECK_OFFSET(host_rsp, 664);
> >  	CHECK_OFFSET(host_rip, 672);
> > +	CHECK_OFFSET(host_s_cet, 680);
> > +	CHECK_OFFSET(host_ssp, 688);
> > +	CHECK_OFFSET(host_ssp_tbl, 696);
> > +	CHECK_OFFSET(guest_s_cet, 704);
> > +	CHECK_OFFSET(guest_ssp, 712);
> > +	CHECK_OFFSET(guest_ssp_tbl, 720);
> >  	CHECK_OFFSET(pin_based_vm_exec_control, 744);
> >  	CHECK_OFFSET(cpu_based_vm_exec_control, 748);
> >  	CHECK_OFFSET(exception_bitmap, 752);
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index a3d01014b9e7..c2e950d378bd 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -7153,6 +7153,7 @@ static void nested_vmx_cr_fixed1_bits_update(struct kvm_vcpu *vcpu)
> >  	cr4_fixed1_update(X86_CR4_PKE,        ecx, feature_bit(PKU));
> >  	cr4_fixed1_update(X86_CR4_UMIP,       ecx, feature_bit(UMIP));
> >  	cr4_fixed1_update(X86_CR4_LA57,       ecx, feature_bit(LA57));
> > +	cr4_fixed1_update(X86_CR4_CET,	      ecx, feature_bit(SHSTK));
> >  
> >  #undef cr4_fixed1_update
> >  }
> > -- 
> > 2.17.2
> > 

Re: [PATCH v11 2/9] KVM: VMX: Set guest CET MSRs per KVM and host configuration

[Yang Weijiang]

On Fri, Apr 24, 2020 at 07:55:29AM -0700, Sean Christopherson wrote:
> On Fri, Apr 24, 2020 at 10:07:51PM +0800, Yang Weijiang wrote:
> > On Thu, Apr 23, 2020 at 09:27:49AM -0700, Sean Christopherson wrote:
> > > On Thu, Mar 26, 2020 at 04:18:39PM +0800, Yang Weijiang wrote:
> > > > @@ -7102,6 +7138,10 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
> > > >  			vmx_set_guest_msr(vmx, msr, enabled ? 0 : TSX_CTRL_RTM_DISABLE);
> > > >  		}
> > > >  	}
> > > > +
> > > > +	if (guest_cpuid_has(vcpu, X86_FEATURE_SHSTK) ||
> > > > +	    guest_cpuid_has(vcpu, X86_FEATURE_IBT))
> > > > +		vmx_update_intercept_for_cet_msr(vcpu);
> > > 
> > > This is wrong, it will miss the case where userspace double configures CPUID
> > > and goes from CET=1 to CET=0.  This should instead be:
> > > 
> > > 	if (supported_xss & (XFEATURE_MASK_CET_KERNEL | XFEATURE_MASK_CET_USER))
> > > 		vmx_update_intercept_for_cet_msr(vcpu);
> > > 
> > > >  }
> > Here CET=1/0, did you mean the CET bit in XSS or CR4.CET? If it's the
> > former, then it's OK for me.
> 
> The former, i.e. update the CET MSRs if KVM supports CET virtualization and
> the guest's CPUID configuration is changing.
Yep, this case should be taken into account, thank you for pointing it out!

Re: [PATCH v11 3/9] KVM: VMX: Set host/guest CET states for vmexit/vmentry

[Yang Weijiang]

On Fri, Apr 24, 2020 at 07:49:41AM -0700, Sean Christopherson wrote:
> On Fri, Apr 24, 2020 at 10:35:10PM +0800, Yang Weijiang wrote:
> > On Thu, Apr 23, 2020 at 10:17:41AM -0700, Sean Christopherson wrote:
> > > On Thu, Mar 26, 2020 at 04:18:40PM +0800, Yang Weijiang wrote:
> > > > @@ -7140,8 +7175,23 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
> > > >  	}
> > > >  
> > > >  	if (guest_cpuid_has(vcpu, X86_FEATURE_SHSTK) ||
> > > > -	    guest_cpuid_has(vcpu, X86_FEATURE_IBT))
> > > > +	    guest_cpuid_has(vcpu, X86_FEATURE_IBT)) {
> > > >  		vmx_update_intercept_for_cet_msr(vcpu);
> > > > +
> > > > +		if (cpu_has_cet_guest_load_ctrl() && is_cet_supported(vcpu))
> > > > +			vm_entry_controls_setbit(to_vmx(vcpu),
> > > > +						 VM_ENTRY_LOAD_GUEST_CET_STATE);
> > > > +		else
> > > > +			vm_entry_controls_clearbit(to_vmx(vcpu),
> > > > +						   VM_ENTRY_LOAD_GUEST_CET_STATE);
> > > > +
> > > > +		if (cpu_has_cet_host_load_ctrl() && is_cet_supported(vcpu))
> > > > +			vm_exit_controls_setbit(to_vmx(vcpu),
> > > > +						VM_EXIT_LOAD_HOST_CET_STATE);
> > > > +		else
> > > > +			vm_exit_controls_clearbit(to_vmx(vcpu),
> > > > +						  VM_EXIT_LOAD_HOST_CET_STATE);
> > > 
> > > As above, I think this can be done in vmx_set_cr4().
> > >
> > Hmm, it's in vmx_set_cr4() in early versions, OK, will move them back.
> 
> Did I advise you to move them out of vmx_set_cr4()?  It's entirely possible
> I forgot some detail since the last time I reviewed this series.
Things are always changing, I'm willing to change any part of the patch
before it's landed :-).

Re: [PATCH v11 7/9] KVM: X86: Add userspace access interface for CET MSRs

[Yang Weijiang]

On Fri, Apr 24, 2020 at 08:10:49AM -0700, Sean Christopherson wrote:
> On Fri, Apr 24, 2020 at 11:02:46PM +0800, Yang Weijiang wrote:
> > On Thu, Apr 23, 2020 at 11:14:06AM -0700, Sean Christopherson wrote:
> > > > +	case MSR_IA32_INT_SSP_TAB:
> > > > +		if (!cet_check_ctl_msr_access(vcpu, msr_info))
> > > > +			return 1;
> > > > +		if (!is_64_bit_mode(vcpu))
> > > 
> > > This is wrong, the SDM explicitly calls out the !64 case:
> > > 
> > >   IA32_INTERRUPT_SSP_TABLE_ADDR (64 bits; 32 bits on processors that do not
> > >   support Intel 64 architecture).
> > So the check is also unnecessary as it's natual size?
> 
> It still needs a canonical check.
> 
> Note, KVM diverges from the SDM for canonical checks in that it performs
> canonical checks even when the virtual CPU doesn't support 64-bit and/or
> the host kernel is a 32-bit kernel.  This is intentional because the
> underlying hardware will still enforce the checks, i.e. KVM needs to make
> the physical CPU happy, and the number of people running KVM on hardware
> without 64-bit support can probably be counted on one hand.
Got it, thank you!

Re: [PATCH v11 5/9] KVM: X86: Refresh CPUID once guest XSS MSR changes

[Paolo Bonzini]

On 23/04/20 19:34, Sean Christopherson wrote:
>>  	if (!kvm_cpu_cap_has(X86_FEATURE_XSAVES))
>>  		supported_xss = 0;
>> +	else
>> +		supported_xss = host_xss & KVM_SUPPORTED_XSS;
> Silly nit: I'd prefer to invert the check, e.g.
> 
> 	if (kvm_cpu_cap_has(X86_FEATURE_XSAVES))
> 		supported_xss = host_xss & KVM_SUPPORTED_XSS;
> 	else
> 		supported_xss = 0;
> 

Also a nit: Linux coding style should be

	supported_xss = 0;
	if (kvm_cpu_cap_has(X86_FEATURE_XSAVES))
		supported_xss = host_xss & KVM_SUPPORTED_XSS;

Paolo

Re: [PATCH v11 2/9] KVM: VMX: Set guest CET MSRs per KVM and host configuration

[Paolo Bonzini]

On 23/04/20 18:27, Sean Christopherson wrote:
>>  
>> +static bool is_cet_mode_allowed(struct kvm_vcpu *vcpu, u32 mode_mask)
> CET itself isn't a mode.  And since this ends up being an inner helper for
> is_cet_supported(), I think __is_cet_supported() would be the way to go.
> 
> Even @mode_mask is a bit confusing without the context of it being kernel
> vs. user.  The callers are very readable, e.g. I'd much prefer passing the
> mask as opposed to doing 'bool kernel'.  Maybe s/mode_mask/cet_mask?  That
> doesn't exactly make things super clear, but at least the reader knows the
> mask is for CET features.

What about is_cet_state_supported and xss_states?

Paolo

>> +{
>> +	return ((supported_xss & mode_mask) &&
>> +		(guest_cpuid_has(vcpu, X86_FEATURE_SHSTK) ||
>> +		guest_cpuid_has(vcpu, X86_FEATURE_IBT)));
>> +}

Re: [PATCH v11 7/9] KVM: X86: Add userspace access interface for CET MSRs

[Paolo Bonzini]

On 26/03/20 09:18, Yang Weijiang wrote:
> There're two different places storing Guest CET states, states
> managed with XSAVES/XRSTORS, as restored/saved
> in previous patch, can be read/write directly from/to the MSRs.
> For those stored in VMCS fields, they're access via vmcs_read/
> vmcs_write.
> 
> To correctly read/write the CET MSRs, it's necessary to check
> whether the kernel FPU context switch happened and reload guest
> FPU context if needed.

I have one question here, it may be just a misunderstanding.

As I understand it, the PLx_SSP MSRs are only used when the current
privilege level changes; the processor has a hidden SSP register for the
current privilege level, and the SSP can be accessed via VMCS only.

These patches do not allow saving/restoring this hidden register.
However, this should be necessary in order to migrate the virtual
machine.  The simplest way to plumb this is through a KVM-specific MSR
in arch/x86/include/uapi/asm/kvm_para.h.  This MSR should only be
accessible to userspace, i.e. only if msr_info->host_initiated.

Testing CET in the state-test selftest is a bit hard because you have to
set up S_CET and the shadow stack, but it would be great to have a
separate test similar to tools/testing/selftests/x86_64/smm_test.  It's
not an absolute requirement for merging, but if you can put it on your
todo list it would be better.

Thanks,

Paolo

Re: [PATCH v11 5/9] KVM: X86: Refresh CPUID once guest XSS MSR changes

[Yang Weijiang]

On Sat, Apr 25, 2020 at 03:19:24PM +0200, Paolo Bonzini wrote:
> On 23/04/20 19:34, Sean Christopherson wrote:
> >>  	if (!kvm_cpu_cap_has(X86_FEATURE_XSAVES))
> >>  		supported_xss = 0;
> >> +	else
> >> +		supported_xss = host_xss & KVM_SUPPORTED_XSS;
> > Silly nit: I'd prefer to invert the check, e.g.
> > 
> > 	if (kvm_cpu_cap_has(X86_FEATURE_XSAVES))
> > 		supported_xss = host_xss & KVM_SUPPORTED_XSS;
> > 	else
> > 		supported_xss = 0;
> > 
> 
> Also a nit: Linux coding style should be
> 
> 	supported_xss = 0;
> 	if (kvm_cpu_cap_has(X86_FEATURE_XSAVES))
> 		supported_xss = host_xss & KVM_SUPPORTED_XSS;
> 
> Paolo
Ah, I should follow the coding style, thank you Paolo!

Re: [PATCH v11 7/9] KVM: X86: Add userspace access interface for CET MSRs

[Yang Weijiang]

On Sat, Apr 25, 2020 at 05:31:59PM +0200, Paolo Bonzini wrote:
> On 26/03/20 09:18, Yang Weijiang wrote:
> > There're two different places storing Guest CET states, states
> > managed with XSAVES/XRSTORS, as restored/saved
> > in previous patch, can be read/write directly from/to the MSRs.
> > For those stored in VMCS fields, they're access via vmcs_read/
> > vmcs_write.
> > 
> > To correctly read/write the CET MSRs, it's necessary to check
> > whether the kernel FPU context switch happened and reload guest
> > FPU context if needed.
> 
> I have one question here, it may be just a misunderstanding.
> 
> As I understand it, the PLx_SSP MSRs are only used when the current
> privilege level changes; the processor has a hidden SSP register for the
> current privilege level, and the SSP can be accessed via VMCS only.
>
> These patches do not allow saving/restoring this hidden register.
> However, this should be necessary in order to migrate the virtual
> machine.  The simplest way to plumb this is through a KVM-specific MSR
> in arch/x86/include/uapi/asm/kvm_para.h.  This MSR should only be
> accessible to userspace, i.e. only if msr_info->host_initiated.
Thanks for raising the issue!
I checked SDM again, yes, it's neccessary to save the current SSP for
migration case, I'll follow your advice to add it as custom MSR.
 
> Testing CET in the state-test selftest is a bit hard because you have to
> set up S_CET and the shadow stack, but it would be great to have a
> separate test similar to tools/testing/selftests/x86_64/smm_test.  It's
> not an absolute requirement for merging, but if you can put it on your
> todo list it would be better.
> 
What's the purpose of the selftest? Is it just for Shadow Stack SSP
state transitions in various cases? e.g., L0 SSP<--->L3 SSP,
L0 SSP1<--->L0 SSP2? We now have the KVM unit-test for CET functionalities,
i.e., Shadow Stack and Indirect Branch Tracking for user-mode, I can put the
state test app into the todo list as current patchset is mainly for user-mode
protection, the supervisor-mode CET protection is the next step.

> Thanks,
> 
> Paolo

Re: [PATCH v11 2/9] KVM: VMX: Set guest CET MSRs per KVM and host configuration

[Yang Weijiang]

On Sat, Apr 25, 2020 at 03:26:30PM +0200, Paolo Bonzini wrote:
> On 23/04/20 18:27, Sean Christopherson wrote:
> >>  
> >> +static bool is_cet_mode_allowed(struct kvm_vcpu *vcpu, u32 mode_mask)
> > CET itself isn't a mode.  And since this ends up being an inner helper for
> > is_cet_supported(), I think __is_cet_supported() would be the way to go.
> > 
> > Even @mode_mask is a bit confusing without the context of it being kernel
> > vs. user.  The callers are very readable, e.g. I'd much prefer passing the
> > mask as opposed to doing 'bool kernel'.  Maybe s/mode_mask/cet_mask?  That
> > doesn't exactly make things super clear, but at least the reader knows the
> > mask is for CET features.
> 
> What about is_cet_state_supported and xss_states?
>
It's good for me, I'll change them accordingly, thank you for review!

> Paolo
> 
> >> +{
> >> +	return ((supported_xss & mode_mask) &&
> >> +		(guest_cpuid_has(vcpu, X86_FEATURE_SHSTK) ||
> >> +		guest_cpuid_has(vcpu, X86_FEATURE_IBT)));
> >> +}

Re: [PATCH v11 7/9] KVM: X86: Add userspace access interface for CET MSRs

[Paolo Bonzini]

On 26/04/20 17:23, Yang Weijiang wrote:
> What's the purpose of the selftest? Is it just for Shadow Stack SSP
> state transitions in various cases? e.g., L0 SSP<--->L3 SSP,
> L0 SSP1<--->L0 SSP2?

No, it checks that the whole state can be extracted and restored from a
running VM.  For example, it would have caught immediately that the
current SSP could not be saved and restored.

> We now have the KVM unit-test for CET functionalities,
> i.e., Shadow Stack and Indirect Branch Tracking for user-mode, I can put the
> state test app into the todo list as current patchset is mainly for user-mode
> protection, the supervisor-mode CET protection is the next step.

What are the limitations?  Or are you referring to the unit test?

Paolo

Re: [PATCH v11 3/9] KVM: VMX: Set host/guest CET states for vmexit/vmentry

[Sean Christopherson]

On Sat, Apr 25, 2020 at 05:20:21PM +0800, Yang Weijiang wrote:
> On Fri, Apr 24, 2020 at 07:49:41AM -0700, Sean Christopherson wrote:
> > On Fri, Apr 24, 2020 at 10:35:10PM +0800, Yang Weijiang wrote:
> > > On Thu, Apr 23, 2020 at 10:17:41AM -0700, Sean Christopherson wrote:
> > > > On Thu, Mar 26, 2020 at 04:18:40PM +0800, Yang Weijiang wrote:
> > > > > @@ -7140,8 +7175,23 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
> > > > >  	}
> > > > >  
> > > > >  	if (guest_cpuid_has(vcpu, X86_FEATURE_SHSTK) ||
> > > > > -	    guest_cpuid_has(vcpu, X86_FEATURE_IBT))
> > > > > +	    guest_cpuid_has(vcpu, X86_FEATURE_IBT)) {
> > > > >  		vmx_update_intercept_for_cet_msr(vcpu);
> > > > > +
> > > > > +		if (cpu_has_cet_guest_load_ctrl() && is_cet_supported(vcpu))
> > > > > +			vm_entry_controls_setbit(to_vmx(vcpu),
> > > > > +						 VM_ENTRY_LOAD_GUEST_CET_STATE);
> > > > > +		else
> > > > > +			vm_entry_controls_clearbit(to_vmx(vcpu),
> > > > > +						   VM_ENTRY_LOAD_GUEST_CET_STATE);
> > > > > +
> > > > > +		if (cpu_has_cet_host_load_ctrl() && is_cet_supported(vcpu))
> > > > > +			vm_exit_controls_setbit(to_vmx(vcpu),
> > > > > +						VM_EXIT_LOAD_HOST_CET_STATE);
> > > > > +		else
> > > > > +			vm_exit_controls_clearbit(to_vmx(vcpu),
> > > > > +						  VM_EXIT_LOAD_HOST_CET_STATE);
> > > > 
> > > > As above, I think this can be done in vmx_set_cr4().
> > > >
> > > Hmm, it's in vmx_set_cr4() in early versions, OK, will move them back.
> > 
> > Did I advise you to move them out of vmx_set_cr4()?  It's entirely possible
> > I forgot some detail since the last time I reviewed this series.
> Things are always changing, I'm willing to change any part of the patch
> before it's landed :-).

I'm worried that there was a reason for requesting the logic to be moved
out vmx_set_cr4() that I've since forgotten.  I'll see if I can dredge up
the old mail.

Re: [PATCH v11 3/9] KVM: VMX: Set host/guest CET states for vmexit/vmentry

[Sean Christopherson]

On Mon, Apr 27, 2020 at 10:04:26AM -0700, Sean Christopherson wrote:
> On Sat, Apr 25, 2020 at 05:20:21PM +0800, Yang Weijiang wrote:
> > On Fri, Apr 24, 2020 at 07:49:41AM -0700, Sean Christopherson wrote:
> > > On Fri, Apr 24, 2020 at 10:35:10PM +0800, Yang Weijiang wrote:
> > > > On Thu, Apr 23, 2020 at 10:17:41AM -0700, Sean Christopherson wrote:
> > > > > On Thu, Mar 26, 2020 at 04:18:40PM +0800, Yang Weijiang wrote:
> > > > > > @@ -7140,8 +7175,23 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
> > > > > >  	}
> > > > > >  
> > > > > >  	if (guest_cpuid_has(vcpu, X86_FEATURE_SHSTK) ||
> > > > > > -	    guest_cpuid_has(vcpu, X86_FEATURE_IBT))
> > > > > > +	    guest_cpuid_has(vcpu, X86_FEATURE_IBT)) {
> > > > > >  		vmx_update_intercept_for_cet_msr(vcpu);
> > > > > > +
> > > > > > +		if (cpu_has_cet_guest_load_ctrl() && is_cet_supported(vcpu))
> > > > > > +			vm_entry_controls_setbit(to_vmx(vcpu),
> > > > > > +						 VM_ENTRY_LOAD_GUEST_CET_STATE);
> > > > > > +		else
> > > > > > +			vm_entry_controls_clearbit(to_vmx(vcpu),
> > > > > > +						   VM_ENTRY_LOAD_GUEST_CET_STATE);
> > > > > > +
> > > > > > +		if (cpu_has_cet_host_load_ctrl() && is_cet_supported(vcpu))
> > > > > > +			vm_exit_controls_setbit(to_vmx(vcpu),
> > > > > > +						VM_EXIT_LOAD_HOST_CET_STATE);
> > > > > > +		else
> > > > > > +			vm_exit_controls_clearbit(to_vmx(vcpu),
> > > > > > +						  VM_EXIT_LOAD_HOST_CET_STATE);
> > > > > 
> > > > > As above, I think this can be done in vmx_set_cr4().
> > > > >
> > > > Hmm, it's in vmx_set_cr4() in early versions, OK, will move them back.
> > > 
> > > Did I advise you to move them out of vmx_set_cr4()?  It's entirely possible
> > > I forgot some detail since the last time I reviewed this series.
> > Things are always changing, I'm willing to change any part of the patch
> > before it's landed :-).
> 
> I'm worried that there was a reason for requesting the logic to be moved
> out vmx_set_cr4() that I've since forgotten.  I'll see if I can dredge up
> the old mail.

Aha.  v1-v7 had this in cr4.  In v7, you stated that you would move the
toggling to VM-Enter, and in v8 you did just that[2].  In v9 I questioned
why the bits were being toggled in vmx_vcpu_run() and advised moving the
code to vmx_cpuid_update()[3], obviously forgetting that earlier versions
did the toggling in vmx_set_cr4().

AFAICT, I was only reacting to the immediate patch when I advised moving
the code to vmx_cpuid_update(), i.e. the recommendation to move the code
to vmx_set_cr4() doesn't contradict any previous feedback and thus doesn't
reintroduce a known bug.

[1] https://patchwork.kernel.org/patch/11163639/#22931561
[2] https://patchwork.kernel.org/patch/11222763/
[3] https://patchwork.kernel.org/patch/11310823/

Re: [PATCH v11 7/9] KVM: X86: Add userspace access interface for CET MSRs

[Yang Weijiang]

On Mon, Apr 27, 2020 at 04:04:28PM +0200, Paolo Bonzini wrote:
> On 26/04/20 17:23, Yang Weijiang wrote:
> > What's the purpose of the selftest? Is it just for Shadow Stack SSP
> > state transitions in various cases? e.g., L0 SSP<--->L3 SSP,
> > L0 SSP1<--->L0 SSP2?
> 
> No, it checks that the whole state can be extracted and restored from a
> running VM.  For example, it would have caught immediately that the
> current SSP could not be saved and restored.
> 
> > We now have the KVM unit-test for CET functionalities,
> > i.e., Shadow Stack and Indirect Branch Tracking for user-mode, I can put the
> > state test app into the todo list as current patchset is mainly for user-mode
> > protection, the supervisor-mode CET protection is the next step.
> 
> What are the limitations?  Or are you referring to the unit test?
>
I'm referring to the unit test, I enabled basic CET function test to verify
if SHSTK/IBT is supported with current platform and KVM, but didn't cover
what you mentioned above. OK, I put the state
self-test to my todo list. Thank you for the reminder.

> Paolo