[PATCH v3 0/3] Add KVM support for Intel local MCE

[PATCH v3 0/3] Add KVM support for Intel local MCE

[Haozhong Zhang]

Changes in v3:
 * Make guest MSR_IA32_FEATURE_CONTROL always available (Paolo
   Bonzini) and remove the nested vmx check in the 'get' case in patch
   1 (Borislav Petkov).
 * Always mark the locked bit of MSR_IA32_FEATURE_CONTROL
   valid. (Paolo Bonzini)
 * Remove the now unnecessary macros to set
   msr_ia32_feature_control_valid_bits. (Paolo Bonzini)
 * Remove the unnecessary check of MCG_LMCE_P in v2
   vmx_mcg_ext_ctl_msr_present() and inline the remaining part. (Paolo
   Bonzini)

Changes in v2:
 * v1 Patch 1 becomes v2 Patch 3.
 * Fix COB chain in Patch 3. (Boris Petkov)
 * (Patch 1) Move msr_ia32_feature_control from nested_vmx to
   vcpu_vmx, because it does not depend only on nested after this
   patch series. (Radim Krčmář)
 * (Patch 2) Add a valid bitmask for MSR_IA32_FEATURE_CONTROL to allow
   checking individual bits of MSR_IA32_FEATURE_CONTROL according to
   enabled features. (Radim Krčmář)
 * Move the common check in handling MSR_IA32_MCG_EXT_CTL to function
   vmx_mcg_ext_ctl_msr_present. (Radim Krčmář)

Changes in v1:
 * Change macro KVM_MCE_CAP_SUPPORTED to variable kvm_mce_cap_supported.
 * Include LMCE capability in kvm_mce_cap_supported only on Intel CPU,
   i.e. LMCE can be enabled only on Intel CPU.
 * Check if LMCE is enabled in guest MSR_IA32_FEATURE_CONTROL when
   handling guest access to MSR_IA32_MCG_EXT_CTL.

This patch series along with the corresponding QEMU patch series (sent
via another email with title "[PATCH v5 0/4] Add QEMU support for
Intel local MCE") enables Intel local MCE feature for guest. This KVM
patch handles guest access to LMCE-related MSR (MSR_IA32_MCG_EXT_CTL
and MSR_IA32_FEATURE_CONTROL).

Ashok Raj (1):
  KVM: VMX: enable guest access to LMCE related MSRs

Haozhong Zhang (2):
  KVM: VMX: move msr_ia32_feature_control to vcpu_vmx
  KVM: VMX: validate individual bits of guest MSR_IA32_FEATURE_CONTROL

 arch/x86/include/asm/kvm_host.h |  5 +++
 arch/x86/kvm/vmx.c              | 67 ++++++++++++++++++++++++++++++++++++-----
 arch/x86/kvm/x86.c              | 15 +++++----
 3 files changed, 73 insertions(+), 14 deletions(-)

-- 
2.9.0

[PATCH v3 1/3] KVM: VMX: move msr_ia32_feature_control to vcpu_vmx

[Haozhong Zhang]

msr_ia32_feature_control will be used for LMCE and not depend only on
nested anymore, so move it from struct nested_vmx to struct vcpu_vmx.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
 arch/x86/kvm/vmx.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index e185649..ad66978 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -428,7 +428,6 @@ struct nested_vmx {
 	struct pi_desc *pi_desc;
 	bool pi_pending;
 	u16 posted_intr_nv;
-	u64 msr_ia32_feature_control;
 
 	struct hrtimer preemption_timer;
 	bool preemption_timer_expired;
@@ -612,6 +611,8 @@ struct vcpu_vmx {
 	bool guest_pkru_valid;
 	u32 guest_pkru;
 	u32 host_pkru;
+
+	u64 msr_ia32_feature_control;
 };
 
 enum segment_cache_field {
@@ -2970,9 +2971,7 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		msr_info->data = vmcs_read64(GUEST_BNDCFGS);
 		break;
 	case MSR_IA32_FEATURE_CONTROL:
-		if (!nested_vmx_allowed(vcpu))
-			return 1;
-		msr_info->data = to_vmx(vcpu)->nested.msr_ia32_feature_control;
+		msr_info->data = to_vmx(vcpu)->msr_ia32_feature_control;
 		break;
 	case MSR_IA32_VMX_BASIC ... MSR_IA32_VMX_VMFUNC:
 		if (!nested_vmx_allowed(vcpu))
@@ -3064,10 +3063,10 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		break;
 	case MSR_IA32_FEATURE_CONTROL:
 		if (!nested_vmx_allowed(vcpu) ||
-		    (to_vmx(vcpu)->nested.msr_ia32_feature_control &
+		    (to_vmx(vcpu)->msr_ia32_feature_control &
 		     FEATURE_CONTROL_LOCKED && !msr_info->host_initiated))
 			return 1;
-		vmx->nested.msr_ia32_feature_control = data;
+		vmx->msr_ia32_feature_control = data;
 		if (msr_info->host_initiated && data == 0)
 			vmx_leave_nested(vcpu);
 		break;
@@ -6939,7 +6938,7 @@ static int handle_vmon(struct kvm_vcpu *vcpu)
 		return 1;
 	}
 
-	if ((vmx->nested.msr_ia32_feature_control & VMXON_NEEDED_FEATURES)
+	if ((vmx->msr_ia32_feature_control & VMXON_NEEDED_FEATURES)
 			!= VMXON_NEEDED_FEATURES) {
 		kvm_inject_gp(vcpu, 0);
 		return 1;
-- 
2.9.0

[PATCH v3 2/3] KVM: VMX: validate individual bits of guest MSR_IA32_FEATURE_CONTROL

[Haozhong Zhang]

KVM currently does not check the value written to guest
MSR_IA32_FEATURE_CONTROL, though bits corresponding to disabled features
may be set. This patch makes KVM to validate individual bits written to
guest MSR_IA32_FEATURE_CONTROL according to enabled features.

Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
 arch/x86/kvm/vmx.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index ad66978..0a3ccb0 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -612,7 +612,13 @@ struct vcpu_vmx {
 	u32 guest_pkru;
 	u32 host_pkru;
 
+	/*
+	 * Only bits masked by msr_ia32_feature_control_valid_bits can be set in
+	 * msr_ia32_feature_control. FEATURE_CONTROL_LOCKED is always included
+	 * in msr_ia32_feature_control_valid_bits.
+	 */
 	u64 msr_ia32_feature_control;
+	u64 msr_ia32_feature_control_valid_bits;
 };
 
 enum segment_cache_field {
@@ -2929,6 +2935,14 @@ static int vmx_get_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 *pdata)
 	return 0;
 }
 
+static inline bool vmx_feature_control_msr_valid(struct kvm_vcpu *vcpu,
+						 uint64_t val)
+{
+	uint64_t valid_bits = to_vmx(vcpu)->msr_ia32_feature_control_valid_bits;
+
+	return !(val & ~valid_bits);
+}
+
 /*
  * Reads an msr value (of 'msr_index') into 'pdata'.
  * Returns 0 on success, non-0 otherwise.
@@ -3062,7 +3076,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		ret = kvm_set_msr_common(vcpu, msr_info);
 		break;
 	case MSR_IA32_FEATURE_CONTROL:
-		if (!nested_vmx_allowed(vcpu) ||
+		if (!vmx_feature_control_msr_valid(vcpu, data) ||
 		    (to_vmx(vcpu)->msr_ia32_feature_control &
 		     FEATURE_CONTROL_LOCKED && !msr_info->host_initiated))
 			return 1;
@@ -9055,6 +9069,8 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id)
 			goto free_vmcs;
 	}
 
+	vmx->msr_ia32_feature_control_valid_bits = FEATURE_CONTROL_LOCKED;
+
 	return &vmx->vcpu;
 
 free_vmcs:
@@ -9202,6 +9218,13 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
 			vmx->nested.nested_vmx_secondary_ctls_high &=
 				~SECONDARY_EXEC_PCOMMIT;
 	}
+
+	if (nested_vmx_allowed(vcpu))
+		to_vmx(vcpu)->msr_ia32_feature_control_valid_bits |=
+			FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX;
+	else
+		to_vmx(vcpu)->msr_ia32_feature_control_valid_bits &=
+			~FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX;
 }
 
 static void vmx_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry)
-- 
2.9.0

[PATCH v3 3/3] KVM: VMX: enable guest access to LMCE related MSRs

[Haozhong Zhang]

From: Ashok Raj <ashok.raj@intel.com>

On Intel platforms, this patch adds LMCE to KVM MCE supported
capabilities and handles guest access to LMCE related MSRs.

Signed-off-by: Ashok Raj <ashok.raj@intel.com>
[Haozhong: macro KVM_MCE_CAP_SUPPORTED => variable kvm_mce_cap_supported
           Only enable LMCE on Intel platform
           Check MSR_IA32_FEATURE_CONTROL when handling guest
             access to MSR_IA32_MCG_EXT_CTL]
Signed-off-by: Haozhong Zhang <haozhong.zhang@intel.com>
---
 arch/x86/include/asm/kvm_host.h |  5 +++++
 arch/x86/kvm/vmx.c              | 29 +++++++++++++++++++++++++++++
 arch/x86/kvm/x86.c              | 15 +++++++++------
 3 files changed, 43 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 360c517..7a628fb 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -598,6 +598,7 @@ struct kvm_vcpu_arch {
 	u64 mcg_cap;
 	u64 mcg_status;
 	u64 mcg_ctl;
+	u64 mcg_ext_ctl;
 	u64 *mce_banks;
 
 	/* Cache MMIO info */
@@ -1008,6 +1009,8 @@ struct kvm_x86_ops {
 
 	int (*set_hv_timer)(struct kvm_vcpu *vcpu, u64 guest_deadline_tsc);
 	void (*cancel_hv_timer)(struct kvm_vcpu *vcpu);
+
+	void (*setup_mce)(struct kvm_vcpu *vcpu);
 };
 
 struct kvm_arch_async_pf {
@@ -1082,6 +1085,8 @@ extern u64  kvm_max_tsc_scaling_ratio;
 /* 1ull << kvm_tsc_scaling_ratio_frac_bits */
 extern u64  kvm_default_tsc_scaling_ratio;
 
+extern u64 kvm_mce_cap_supported;
+
 enum emulation_result {
 	EMULATE_DONE,         /* no further processing */
 	EMULATE_USER_EXIT,    /* kvm_run ready for userspace exit */
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 0a3ccb0..943609f 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2984,6 +2984,13 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 			return 1;
 		msr_info->data = vmcs_read64(GUEST_BNDCFGS);
 		break;
+	case MSR_IA32_MCG_EXT_CTL:
+		if (!msr_info->host_initiated &&
+		    !(to_vmx(vcpu)->msr_ia32_feature_control &
+		      FEATURE_CONTROL_LMCE))
+			return 1;
+		msr_info->data = vcpu->arch.mcg_ext_ctl;
+		break;
 	case MSR_IA32_FEATURE_CONTROL:
 		msr_info->data = to_vmx(vcpu)->msr_ia32_feature_control;
 		break;
@@ -3075,6 +3082,14 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 	case MSR_IA32_TSC_ADJUST:
 		ret = kvm_set_msr_common(vcpu, msr_info);
 		break;
+	case MSR_IA32_MCG_EXT_CTL:
+		if ((!msr_info->host_initiated &&
+		     !(to_vmx(vcpu)->msr_ia32_feature_control &
+		       FEATURE_CONTROL_LMCE)) ||
+		    (data & ~MCG_EXT_CTL_LMCE_EN))
+			return 1;
+		vcpu->arch.mcg_ext_ctl = data;
+		break;
 	case MSR_IA32_FEATURE_CONTROL:
 		if (!vmx_feature_control_msr_valid(vcpu, data) ||
 		    (to_vmx(vcpu)->msr_ia32_feature_control &
@@ -6484,6 +6499,8 @@ static __init int hardware_setup(void)
 
 	kvm_set_posted_intr_wakeup_handler(wakeup_handler);
 
+	kvm_mce_cap_supported |= MCG_LMCE_P;
+
 	return alloc_kvm_area();
 
 out8:
@@ -11109,6 +11126,16 @@ out:
 	return ret;
 }
 
+static void vmx_setup_mce(struct kvm_vcpu *vcpu)
+{
+	if (vcpu->arch.mcg_cap & MCG_LMCE_P)
+		to_vmx(vcpu)->msr_ia32_feature_control_valid_bits |=
+			FEATURE_CONTROL_LMCE;
+	else
+		to_vmx(vcpu)->msr_ia32_feature_control_valid_bits &=
+			~FEATURE_CONTROL_LMCE;
+}
+
 static struct kvm_x86_ops vmx_x86_ops = {
 	.cpu_has_kvm_support = cpu_has_kvm_support,
 	.disabled_by_bios = vmx_disabled_by_bios,
@@ -11238,6 +11265,8 @@ static struct kvm_x86_ops vmx_x86_ops = {
 	.set_hv_timer = vmx_set_hv_timer,
 	.cancel_hv_timer = vmx_cancel_hv_timer,
 #endif
+
+	.setup_mce = vmx_setup_mce,
 };
 
 static int __init vmx_init(void)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 2992196..0a42fc7 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -70,7 +70,8 @@
 
 #define MAX_IO_MSRS 256
 #define KVM_MAX_MCE_BANKS 32
-#define KVM_MCE_CAP_SUPPORTED (MCG_CTL_P | MCG_SER_P)
+u64 __read_mostly kvm_mce_cap_supported = MCG_CTL_P | MCG_SER_P;
+EXPORT_SYMBOL_GPL(kvm_mce_cap_supported);
 
 #define emul_to_vcpu(ctxt) \
 	container_of(ctxt, struct kvm_vcpu, arch.emulate_ctxt)
@@ -984,6 +985,7 @@ static u32 emulated_msrs[] = {
 	MSR_IA32_MISC_ENABLE,
 	MSR_IA32_MCG_STATUS,
 	MSR_IA32_MCG_CTL,
+	MSR_IA32_MCG_EXT_CTL,
 	MSR_IA32_SMBASE,
 };
 
@@ -2685,11 +2687,9 @@ long kvm_arch_dev_ioctl(struct file *filp,
 		break;
 	}
 	case KVM_X86_GET_MCE_CAP_SUPPORTED: {
-		u64 mce_cap;
-
-		mce_cap = KVM_MCE_CAP_SUPPORTED;
 		r = -EFAULT;
-		if (copy_to_user(argp, &mce_cap, sizeof mce_cap))
+		if (copy_to_user(argp, &kvm_mce_cap_supported,
+				 sizeof(kvm_mce_cap_supported)))
 			goto out;
 		r = 0;
 		break;
@@ -2872,7 +2872,7 @@ static int kvm_vcpu_ioctl_x86_setup_mce(struct kvm_vcpu *vcpu,
 	r = -EINVAL;
 	if (!bank_num || bank_num >= KVM_MAX_MCE_BANKS)
 		goto out;
-	if (mcg_cap & ~(KVM_MCE_CAP_SUPPORTED | 0xff | 0xff0000))
+	if (mcg_cap & ~(kvm_mce_cap_supported | 0xff | 0xff0000))
 		goto out;
 	r = 0;
 	vcpu->arch.mcg_cap = mcg_cap;
@@ -2882,6 +2882,9 @@ static int kvm_vcpu_ioctl_x86_setup_mce(struct kvm_vcpu *vcpu,
 	/* Init IA32_MCi_CTL to all 1s */
 	for (bank = 0; bank < bank_num; bank++)
 		vcpu->arch.mce_banks[bank*4] = ~(u64)0;
+
+	if (kvm_x86_ops->setup_mce)
+		kvm_x86_ops->setup_mce(vcpu);
 out:
 	return r;
 }
-- 
2.9.0

Re: [PATCH v3 0/3] Add KVM support for Intel local MCE

[Paolo Bonzini]

On 22/06/2016 08:59, Haozhong Zhang wrote:
> Changes in v3:
>  * Make guest MSR_IA32_FEATURE_CONTROL always available (Paolo
>    Bonzini) and remove the nested vmx check in the 'get' case in patch
>    1 (Borislav Petkov).
>  * Always mark the locked bit of MSR_IA32_FEATURE_CONTROL
>    valid. (Paolo Bonzini)
>  * Remove the now unnecessary macros to set
>    msr_ia32_feature_control_valid_bits. (Paolo Bonzini)
>  * Remove the unnecessary check of MCG_LMCE_P in v2
>    vmx_mcg_ext_ctl_msr_present() and inline the remaining part. (Paolo
>    Bonzini)
> 
> Changes in v2:
>  * v1 Patch 1 becomes v2 Patch 3.
>  * Fix COB chain in Patch 3. (Boris Petkov)
>  * (Patch 1) Move msr_ia32_feature_control from nested_vmx to
>    vcpu_vmx, because it does not depend only on nested after this
>    patch series. (Radim Krčmář)
>  * (Patch 2) Add a valid bitmask for MSR_IA32_FEATURE_CONTROL to allow
>    checking individual bits of MSR_IA32_FEATURE_CONTROL according to
>    enabled features. (Radim Krčmář)
>  * Move the common check in handling MSR_IA32_MCG_EXT_CTL to function
>    vmx_mcg_ext_ctl_msr_present. (Radim Krčmář)
> 
> Changes in v1:
>  * Change macro KVM_MCE_CAP_SUPPORTED to variable kvm_mce_cap_supported.
>  * Include LMCE capability in kvm_mce_cap_supported only on Intel CPU,
>    i.e. LMCE can be enabled only on Intel CPU.
>  * Check if LMCE is enabled in guest MSR_IA32_FEATURE_CONTROL when
>    handling guest access to MSR_IA32_MCG_EXT_CTL.
> 
> This patch series along with the corresponding QEMU patch series (sent
> via another email with title "[PATCH v5 0/4] Add QEMU support for
> Intel local MCE") enables Intel local MCE feature for guest. This KVM
> patch handles guest access to LMCE-related MSR (MSR_IA32_MCG_EXT_CTL
> and MSR_IA32_FEATURE_CONTROL).
> 
> Ashok Raj (1):
>   KVM: VMX: enable guest access to LMCE related MSRs
> 
> Haozhong Zhang (2):
>   KVM: VMX: move msr_ia32_feature_control to vcpu_vmx
>   KVM: VMX: validate individual bits of guest MSR_IA32_FEATURE_CONTROL
> 
>  arch/x86/include/asm/kvm_host.h |  5 +++
>  arch/x86/kvm/vmx.c              | 67 ++++++++++++++++++++++++++++++++++++-----
>  arch/x86/kvm/x86.c              | 15 +++++----
>  3 files changed, 73 insertions(+), 14 deletions(-)
> 

Thanks, these look good and I'm applying them to kvm/queue.

Paolo