From: Wanpeng Li <[email protected]>
Introduce kvm_get_msr_feature() to handle the msrs which are supported
by different vendors and sharing the same emulation logic.
Cc: Paolo Bonzini <[email protected]>
Cc: Radim Krčmář <[email protected]>
Cc: Liran Alon <[email protected]>
Cc: Nadav Amit <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Tom Lendacky <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
---
arch/x86/kvm/x86.c | 18 +++++++++++++++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index d4985a9..efc8554 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1062,13 +1062,25 @@ static u32 msr_based_features[] = {
static unsigned int num_msr_based_features;
+static int kvm_get_msr_feature(struct kvm_msr_entry *msr)
+{
+ switch (msr->index) {
+ default:
+ if (kvm_x86_ops->get_msr_feature(msr))
+ return 1;
+ }
+ return 0;
+}
+
static int do_get_msr_feature(struct kvm_vcpu *vcpu, unsigned index, u64 *data)
{
struct kvm_msr_entry msr;
+ int r;
msr.index = index;
- if (kvm_x86_ops->get_msr_feature(&msr))
- return 1;
+ r = kvm_get_msr_feature(&msr);
+ if (r)
+ return r;
*data = msr.data;
@@ -4538,7 +4550,7 @@ static void kvm_init_msr_list(void)
struct kvm_msr_entry msr;
msr.index = msr_based_features[i];
- if (kvm_x86_ops->get_msr_feature(&msr))
+ if (kvm_get_msr_feature(&msr))
continue;
if (j < i)
--
2.7.4
From: Wanpeng Li <[email protected]>
Linux (among the others) has checks to make sure that certain features
aren't enabled on a certain family/model/stepping if the microcode version
isn't greater than or equal to a known good version.
By exposing the real microcode version, we're preventing buggy guests that
don't check that they are running virtualized (i.e., they should trust the
hypervisor) from disabling features that are effectively not buggy.
Suggested-by: Filippo Sironi <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Cc: Radim Krčmář <[email protected]>
Cc: Liran Alon <[email protected]>
Cc: Nadav Amit <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Tom Lendacky <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
---
v4 -> v5:
* microcode_version be u64 and initialized suitable, remote hte shifts
v3 -> v4:
* add the shifts back
v2 -> v3:
* remove the shifts
* add the MSR_IA32_UCODE_REV version to the "feature MSRs"
v1 -> v2:
* add MSR_IA32_UCODE_REV to emulated_msrs
arch/x86/include/asm/kvm_host.h | 1 +
arch/x86/kvm/svm.c | 4 +---
arch/x86/kvm/vmx.c | 1 +
arch/x86/kvm/x86.c | 10 ++++++++--
4 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 938d453..df6720f 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -507,6 +507,7 @@ struct kvm_vcpu_arch {
u64 smi_count;
bool tpr_access_reporting;
u64 ia32_xss;
+ u64 microcode_version;
/*
* Paging state of the vcpu
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index f874798..312f33f 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1907,6 +1907,7 @@ static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
u32 dummy;
u32 eax = 1;
+ vcpu->arch.microcode_version = 0x01000065;
svm->spec_ctrl = 0;
if (!init_event) {
@@ -3962,9 +3963,6 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
msr_info->data = svm->spec_ctrl;
break;
- case MSR_IA32_UCODE_REV:
- msr_info->data = 0x01000065;
- break;
case MSR_F15H_IC_CFG: {
int family, model;
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 9968906..2cdbea7 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -5781,6 +5781,7 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
vmx->rmode.vm86_active = 0;
vmx->spec_ctrl = 0;
+ vcpu->arch.microcode_version = 0x100000000ULL;
vmx->vcpu.arch.regs[VCPU_REGS_RDX] = get_rdx_init_val();
kvm_set_cr8(vcpu, 0);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index efc8554..52a09df 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1065,6 +1065,9 @@ static unsigned int num_msr_based_features;
static int kvm_get_msr_feature(struct kvm_msr_entry *msr)
{
switch (msr->index) {
+ case MSR_IA32_UCODE_REV:
+ rdmsrl(msr->index, msr->data);
+ break;
default:
if (kvm_x86_ops->get_msr_feature(msr))
return 1;
@@ -2260,7 +2263,6 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
switch (msr) {
case MSR_AMD64_NB_CFG:
- case MSR_IA32_UCODE_REV:
case MSR_IA32_UCODE_WRITE:
case MSR_VM_HSAVE_PA:
case MSR_AMD64_PATCH_LOADER:
@@ -2268,6 +2270,10 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
case MSR_AMD64_DC_CFG:
break;
+ case MSR_IA32_UCODE_REV:
+ if (msr_info->host_initiated)
+ vcpu->arch.microcode_version = data;
+ break;
case MSR_EFER:
return set_efer(vcpu, data);
case MSR_K7_HWCR:
@@ -2563,7 +2569,7 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
msr_info->data = 0;
break;
case MSR_IA32_UCODE_REV:
- msr_info->data = 0x100000000ULL;
+ msr_info->data = vcpu->arch.microcode_version;
break;
case MSR_MTRRcap:
case 0x200 ... 0x2ff:
--
2.7.4
On 2/27/2018 9:34 PM, Wanpeng Li wrote:
> From: Wanpeng Li <[email protected]>
>
> Linux (among the others) has checks to make sure that certain features
> aren't enabled on a certain family/model/stepping if the microcode version
> isn't greater than or equal to a known good version.
>
> By exposing the real microcode version, we're preventing buggy guests that
> don't check that they are running virtualized (i.e., they should trust the
> hypervisor) from disabling features that are effectively not buggy.
>
> Suggested-by: Filippo Sironi <[email protected]>
> Cc: Paolo Bonzini <[email protected]>
> Cc: Radim Krčmář <[email protected]>
> Cc: Liran Alon <[email protected]>
> Cc: Nadav Amit <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Tom Lendacky <[email protected]>
> Signed-off-by: Wanpeng Li <[email protected]>
> ---
> v4 -> v5:
> * microcode_version be u64 and initialized suitable, remote hte shifts
> v3 -> v4:
> * add the shifts back
> v2 -> v3:
> * remove the shifts
> * add the MSR_IA32_UCODE_REV version to the "feature MSRs"
I think you lost this part from the patch series. I don't see where you
add MSR_IA32_UCODE_REV to the msr_based_features array.
Thanks,
Tom
> v1 -> v2:
> * add MSR_IA32_UCODE_REV to emulated_msrs
>
> arch/x86/include/asm/kvm_host.h | 1 +
> arch/x86/kvm/svm.c | 4 +---
> arch/x86/kvm/vmx.c | 1 +
> arch/x86/kvm/x86.c | 10 ++++++++--
> 4 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 938d453..df6720f 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -507,6 +507,7 @@ struct kvm_vcpu_arch {
> u64 smi_count;
> bool tpr_access_reporting;
> u64 ia32_xss;
> + u64 microcode_version;
>
> /*
> * Paging state of the vcpu
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index f874798..312f33f 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -1907,6 +1907,7 @@ static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
> u32 dummy;
> u32 eax = 1;
>
> + vcpu->arch.microcode_version = 0x01000065;
> svm->spec_ctrl = 0;
>
> if (!init_event) {
> @@ -3962,9 +3963,6 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>
> msr_info->data = svm->spec_ctrl;
> break;
> - case MSR_IA32_UCODE_REV:
> - msr_info->data = 0x01000065;
> - break;
> case MSR_F15H_IC_CFG: {
>
> int family, model;
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 9968906..2cdbea7 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -5781,6 +5781,7 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
> vmx->rmode.vm86_active = 0;
> vmx->spec_ctrl = 0;
>
> + vcpu->arch.microcode_version = 0x100000000ULL;
> vmx->vcpu.arch.regs[VCPU_REGS_RDX] = get_rdx_init_val();
> kvm_set_cr8(vcpu, 0);
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index efc8554..52a09df 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1065,6 +1065,9 @@ static unsigned int num_msr_based_features;
> static int kvm_get_msr_feature(struct kvm_msr_entry *msr)
> {
> switch (msr->index) {
> + case MSR_IA32_UCODE_REV:
> + rdmsrl(msr->index, msr->data);
> + break;
> default:
> if (kvm_x86_ops->get_msr_feature(msr))
> return 1;
> @@ -2260,7 +2263,6 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
>
> switch (msr) {
> case MSR_AMD64_NB_CFG:
> - case MSR_IA32_UCODE_REV:
> case MSR_IA32_UCODE_WRITE:
> case MSR_VM_HSAVE_PA:
> case MSR_AMD64_PATCH_LOADER:
> @@ -2268,6 +2270,10 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> case MSR_AMD64_DC_CFG:
> break;
>
> + case MSR_IA32_UCODE_REV:
> + if (msr_info->host_initiated)
> + vcpu->arch.microcode_version = data;
> + break;
> case MSR_EFER:
> return set_efer(vcpu, data);
> case MSR_K7_HWCR:
> @@ -2563,7 +2569,7 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> msr_info->data = 0;
> break;
> case MSR_IA32_UCODE_REV:
> - msr_info->data = 0x100000000ULL;
> + msr_info->data = vcpu->arch.microcode_version;
> break;
> case MSR_MTRRcap:
> case 0x200 ... 0x2ff:
>
2018-02-28 13:55 GMT+08:00 Tom Lendacky <[email protected]>:
> On 2/27/2018 9:34 PM, Wanpeng Li wrote:
>> From: Wanpeng Li <[email protected]>
>>
>> Linux (among the others) has checks to make sure that certain features
>> aren't enabled on a certain family/model/stepping if the microcode version
>> isn't greater than or equal to a known good version.
>>
>> By exposing the real microcode version, we're preventing buggy guests that
>> don't check that they are running virtualized (i.e., they should trust the
>> hypervisor) from disabling features that are effectively not buggy.
>>
>> Suggested-by: Filippo Sironi <[email protected]>
>> Cc: Paolo Bonzini <[email protected]>
>> Cc: Radim Krčmář <[email protected]>
>> Cc: Liran Alon <[email protected]>
>> Cc: Nadav Amit <[email protected]>
>> Cc: Borislav Petkov <[email protected]>
>> Cc: Tom Lendacky <[email protected]>
>> Signed-off-by: Wanpeng Li <[email protected]>
>> ---
>> v4 -> v5:
>> * microcode_version be u64 and initialized suitable, remote hte shifts
>> v3 -> v4:
>> * add the shifts back
>> v2 -> v3:
>> * remove the shifts
>> * add the MSR_IA32_UCODE_REV version to the "feature MSRs"
>
> I think you lost this part from the patch series. I don't see where you
> add MSR_IA32_UCODE_REV to the msr_based_features array.
I sent out a wrong version, will send again.
Regards,
Wanpeng Li