* [PATCH v4 1/5] KVM: x86: Introduce kvm_check_cpuid()
2020-07-09 4:34 [PATCH v4 0/5] Refactor handling flow of KVM_SET_CPUID* Xiaoyao Li
@ 2020-07-09 4:34 ` Xiaoyao Li
2020-07-09 4:34 ` [PATCH v4 2/5] KVM: x86: Extract kvm_update_cpuid_runtime() from kvm_update_cpuid() Xiaoyao Li
` (4 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Xiaoyao Li @ 2020-07-09 4:34 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson
Cc: Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
linux-kernel, Xiaoyao Li
Use kvm_check_cpuid() to validate if userspace provides legal cpuid
settings and call it before KVM take any action to update CPUID or
update vcpu states based on given CPUID settings.
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
arch/x86/kvm/cpuid.c | 55 ++++++++++++++++++++++++++++----------------
arch/x86/kvm/cpuid.h | 2 +-
2 files changed, 36 insertions(+), 21 deletions(-)
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index eebd66f86abe..1a053022a961 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -54,7 +54,26 @@ static u32 xstate_required_size(u64 xstate_bv, bool compacted)
#define F feature_bit
-int kvm_update_cpuid(struct kvm_vcpu *vcpu)
+static int kvm_check_cpuid(struct kvm_vcpu *vcpu)
+{
+ struct kvm_cpuid_entry2 *best;
+
+ /*
+ * The existing code assumes virtual address is 48-bit or 57-bit in the
+ * canonical address checks; exit if it is ever changed.
+ */
+ best = kvm_find_cpuid_entry(vcpu, 0x80000008, 0);
+ if (best) {
+ int vaddr_bits = (best->eax & 0xff00) >> 8;
+
+ if (vaddr_bits != 48 && vaddr_bits != 57 && vaddr_bits != 0)
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+void kvm_update_cpuid(struct kvm_vcpu *vcpu)
{
struct kvm_cpuid_entry2 *best;
struct kvm_lapic *apic = vcpu->arch.apic;
@@ -98,18 +117,6 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu)
cpuid_entry_has(best, X86_FEATURE_XSAVEC)))
best->ebx = xstate_required_size(vcpu->arch.xcr0, true);
- /*
- * The existing code assumes virtual address is 48-bit or 57-bit in the
- * canonical address checks; exit if it is ever changed.
- */
- best = kvm_find_cpuid_entry(vcpu, 0x80000008, 0);
- if (best) {
- int vaddr_bits = (best->eax & 0xff00) >> 8;
-
- if (vaddr_bits != 48 && vaddr_bits != 57 && vaddr_bits != 0)
- return -EINVAL;
- }
-
best = kvm_find_cpuid_entry(vcpu, KVM_CPUID_FEATURES, 0);
if (kvm_hlt_in_guest(vcpu->kvm) && best &&
(best->eax & (1 << KVM_FEATURE_PV_UNHALT)))
@@ -131,7 +138,6 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu)
kvm_pmu_refresh(vcpu);
vcpu->arch.cr4_guest_rsvd_bits =
__cr4_reserved_bits(guest_cpuid_has, vcpu);
- return 0;
}
static int is_efer_nx(void)
@@ -206,11 +212,16 @@ int kvm_vcpu_ioctl_set_cpuid(struct kvm_vcpu *vcpu,
vcpu->arch.cpuid_entries[i].padding[2] = 0;
}
vcpu->arch.cpuid_nent = cpuid->nent;
+ r = kvm_check_cpuid(vcpu);
+ if (r) {
+ vcpu->arch.cpuid_nent = 0;
+ kvfree(cpuid_entries);
+ goto out;
+ }
+
cpuid_fix_nx_cap(vcpu);
kvm_x86_ops.cpuid_update(vcpu);
- r = kvm_update_cpuid(vcpu);
- if (r)
- vcpu->arch.cpuid_nent = 0;
+ kvm_update_cpuid(vcpu);
kvfree(cpuid_entries);
out:
@@ -231,10 +242,14 @@ int kvm_vcpu_ioctl_set_cpuid2(struct kvm_vcpu *vcpu,
cpuid->nent * sizeof(struct kvm_cpuid_entry2)))
goto out;
vcpu->arch.cpuid_nent = cpuid->nent;
- kvm_x86_ops.cpuid_update(vcpu);
- r = kvm_update_cpuid(vcpu);
- if (r)
+ r = kvm_check_cpuid(vcpu);
+ if (r) {
vcpu->arch.cpuid_nent = 0;
+ goto out;
+ }
+
+ kvm_x86_ops.cpuid_update(vcpu);
+ kvm_update_cpuid(vcpu);
out:
return r;
}
diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
index 05434cd9342f..f136de1debad 100644
--- a/arch/x86/kvm/cpuid.h
+++ b/arch/x86/kvm/cpuid.h
@@ -9,7 +9,7 @@
extern u32 kvm_cpu_caps[NCAPINTS] __read_mostly;
void kvm_set_cpu_caps(void);
-int kvm_update_cpuid(struct kvm_vcpu *vcpu);
+void kvm_update_cpuid(struct kvm_vcpu *vcpu);
struct kvm_cpuid_entry2 *kvm_find_cpuid_entry(struct kvm_vcpu *vcpu,
u32 function, u32 index);
int kvm_dev_ioctl_get_cpuid(struct kvm_cpuid2 *cpuid,
--
2.18.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v4 2/5] KVM: x86: Extract kvm_update_cpuid_runtime() from kvm_update_cpuid()
2020-07-09 4:34 [PATCH v4 0/5] Refactor handling flow of KVM_SET_CPUID* Xiaoyao Li
2020-07-09 4:34 ` [PATCH v4 1/5] KVM: x86: Introduce kvm_check_cpuid() Xiaoyao Li
@ 2020-07-09 4:34 ` Xiaoyao Li
2020-07-09 10:55 ` Paolo Bonzini
2020-07-09 4:34 ` [PATCH v4 3/5] KVM: x86: Rename kvm_update_cpuid() to kvm_vcpu_after_set_cpuid() Xiaoyao Li
` (3 subsequent siblings)
5 siblings, 1 reply; 8+ messages in thread
From: Xiaoyao Li @ 2020-07-09 4:34 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson
Cc: Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
linux-kernel, Xiaoyao Li
Beside called in kvm_vcpu_ioctl_set_cpuid*(), kvm_update_cpuid() is also
called 5 places else in x86.c and 1 place else in lapic.c. All those 6
places only need the part of updating guest CPUIDs (OSXSAVE, OSPKE, APIC,
KVM_FEATURE_PV_UNHALT, ...) based on the runtime vcpu state, so extract
them as a separate kvm_update_cpuid_runtime().
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
arch/x86/kvm/cpuid.c | 44 +++++++++++++++++++++++++++-----------------
arch/x86/kvm/cpuid.h | 2 +-
arch/x86/kvm/lapic.c | 2 +-
arch/x86/kvm/x86.c | 10 +++++-----
4 files changed, 34 insertions(+), 24 deletions(-)
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 1a053022a961..0ed3b343c44e 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -73,10 +73,9 @@ static int kvm_check_cpuid(struct kvm_vcpu *vcpu)
return 0;
}
-void kvm_update_cpuid(struct kvm_vcpu *vcpu)
+void kvm_update_cpuid_runtime(struct kvm_vcpu *vcpu)
{
struct kvm_cpuid_entry2 *best;
- struct kvm_lapic *apic = vcpu->arch.apic;
best = kvm_find_cpuid_entry(vcpu, 1, 0);
if (best) {
@@ -89,28 +88,14 @@ void kvm_update_cpuid(struct kvm_vcpu *vcpu)
vcpu->arch.apic_base & MSR_IA32_APICBASE_ENABLE);
}
- if (best && apic) {
- if (cpuid_entry_has(best, X86_FEATURE_TSC_DEADLINE_TIMER))
- apic->lapic_timer.timer_mode_mask = 3 << 17;
- else
- apic->lapic_timer.timer_mode_mask = 1 << 17;
-
- kvm_apic_set_version(vcpu);
- }
-
best = kvm_find_cpuid_entry(vcpu, 7, 0);
if (best && boot_cpu_has(X86_FEATURE_PKU) && best->function == 0x7)
cpuid_entry_change(best, X86_FEATURE_OSPKE,
kvm_read_cr4_bits(vcpu, X86_CR4_PKE));
best = kvm_find_cpuid_entry(vcpu, 0xD, 0);
- if (!best) {
- vcpu->arch.guest_supported_xcr0 = 0;
- } else {
- vcpu->arch.guest_supported_xcr0 =
- (best->eax | ((u64)best->edx << 32)) & supported_xcr0;
+ if (best)
best->ebx = xstate_required_size(vcpu->arch.xcr0, false);
- }
best = kvm_find_cpuid_entry(vcpu, 0xD, 1);
if (best && (cpuid_entry_has(best, X86_FEATURE_XSAVES) ||
@@ -129,6 +114,29 @@ void kvm_update_cpuid(struct kvm_vcpu *vcpu)
vcpu->arch.ia32_misc_enable_msr &
MSR_IA32_MISC_ENABLE_MWAIT);
}
+}
+
+static void kvm_update_cpuid(struct kvm_vcpu *vcpu)
+{
+ struct kvm_lapic *apic = vcpu->arch.apic;
+ struct kvm_cpuid_entry2 *best;
+
+ best = kvm_find_cpuid_entry(vcpu, 1, 0);
+ if (best && apic) {
+ if (cpuid_entry_has(best, X86_FEATURE_TSC_DEADLINE_TIMER))
+ apic->lapic_timer.timer_mode_mask = 3 << 17;
+ else
+ apic->lapic_timer.timer_mode_mask = 1 << 17;
+
+ kvm_apic_set_version(vcpu);
+ }
+
+ best = kvm_find_cpuid_entry(vcpu, 0xD, 0);
+ if (!best)
+ vcpu->arch.guest_supported_xcr0 = 0;
+ else
+ vcpu->arch.guest_supported_xcr0 =
+ (best->eax | ((u64)best->edx << 32)) & supported_xcr0;
/* Note, maxphyaddr must be updated before tdp_level. */
vcpu->arch.maxphyaddr = cpuid_query_maxphyaddr(vcpu);
@@ -221,6 +229,7 @@ int kvm_vcpu_ioctl_set_cpuid(struct kvm_vcpu *vcpu,
cpuid_fix_nx_cap(vcpu);
kvm_x86_ops.cpuid_update(vcpu);
+ kvm_update_cpuid_runtime(vcpu);
kvm_update_cpuid(vcpu);
kvfree(cpuid_entries);
@@ -249,6 +258,7 @@ int kvm_vcpu_ioctl_set_cpuid2(struct kvm_vcpu *vcpu,
}
kvm_x86_ops.cpuid_update(vcpu);
+ kvm_update_cpuid_runtime(vcpu);
kvm_update_cpuid(vcpu);
out:
return r;
diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
index f136de1debad..3a923ae15f2f 100644
--- a/arch/x86/kvm/cpuid.h
+++ b/arch/x86/kvm/cpuid.h
@@ -9,7 +9,7 @@
extern u32 kvm_cpu_caps[NCAPINTS] __read_mostly;
void kvm_set_cpu_caps(void);
-void kvm_update_cpuid(struct kvm_vcpu *vcpu);
+void kvm_update_cpuid_runtime(struct kvm_vcpu *vcpu);
struct kvm_cpuid_entry2 *kvm_find_cpuid_entry(struct kvm_vcpu *vcpu,
u32 function, u32 index);
int kvm_dev_ioctl_get_cpuid(struct kvm_cpuid2 *cpuid,
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index e5dbb7ebae78..47801a44cfa6 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -2230,7 +2230,7 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value)
vcpu->arch.apic_base = value;
if ((old_value ^ value) & MSR_IA32_APICBASE_ENABLE)
- kvm_update_cpuid(vcpu);
+ kvm_update_cpuid_runtime(vcpu);
if (!apic)
return;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 7b987bccf0c8..e27d3db7e43f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -940,7 +940,7 @@ static int __kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
vcpu->arch.xcr0 = xcr0;
if ((xcr0 ^ old_xcr0) & XFEATURE_MASK_EXTEND)
- kvm_update_cpuid(vcpu);
+ kvm_update_cpuid_runtime(vcpu);
return 0;
}
@@ -1004,7 +1004,7 @@ int kvm_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
kvm_mmu_reset_context(vcpu);
if ((cr4 ^ old_cr4) & (X86_CR4_OSXSAVE | X86_CR4_PKE))
- kvm_update_cpuid(vcpu);
+ kvm_update_cpuid_runtime(vcpu);
return 0;
}
@@ -2916,7 +2916,7 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
if (!guest_cpuid_has(vcpu, X86_FEATURE_XMM3))
return 1;
vcpu->arch.ia32_misc_enable_msr = data;
- kvm_update_cpuid(vcpu);
+ kvm_update_cpuid_runtime(vcpu);
} else {
vcpu->arch.ia32_misc_enable_msr = data;
}
@@ -8170,7 +8170,7 @@ static void enter_smm(struct kvm_vcpu *vcpu)
kvm_x86_ops.set_efer(vcpu, 0);
#endif
- kvm_update_cpuid(vcpu);
+ kvm_update_cpuid_runtime(vcpu);
kvm_mmu_reset_context(vcpu);
}
@@ -9192,7 +9192,7 @@ static int __set_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
(X86_CR4_OSXSAVE | X86_CR4_PKE));
kvm_x86_ops.set_cr4(vcpu, sregs->cr4);
if (cpuid_update_needed)
- kvm_update_cpuid(vcpu);
+ kvm_update_cpuid_runtime(vcpu);
idx = srcu_read_lock(&vcpu->kvm->srcu);
if (is_pae_paging(vcpu)) {
--
2.18.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v4 2/5] KVM: x86: Extract kvm_update_cpuid_runtime() from kvm_update_cpuid()
2020-07-09 4:34 ` [PATCH v4 2/5] KVM: x86: Extract kvm_update_cpuid_runtime() from kvm_update_cpuid() Xiaoyao Li
@ 2020-07-09 10:55 ` Paolo Bonzini
0 siblings, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2020-07-09 10:55 UTC (permalink / raw)
To: Xiaoyao Li, Sean Christopherson
Cc: Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
linux-kernel
On 09/07/20 06:34, Xiaoyao Li wrote:
> Beside called in kvm_vcpu_ioctl_set_cpuid*(), kvm_update_cpuid() is also
> called 5 places else in x86.c and 1 place else in lapic.c. All those 6
> places only need the part of updating guest CPUIDs (OSXSAVE, OSPKE, APIC,
> KVM_FEATURE_PV_UNHALT, ...) based on the runtime vcpu state, so extract
> them as a separate kvm_update_cpuid_runtime().
I'm not sure KVM_FEATURE_PV_UNHALT counts as one of these, but I guess
it's not a big deal.
Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v4 3/5] KVM: x86: Rename kvm_update_cpuid() to kvm_vcpu_after_set_cpuid()
2020-07-09 4:34 [PATCH v4 0/5] Refactor handling flow of KVM_SET_CPUID* Xiaoyao Li
2020-07-09 4:34 ` [PATCH v4 1/5] KVM: x86: Introduce kvm_check_cpuid() Xiaoyao Li
2020-07-09 4:34 ` [PATCH v4 2/5] KVM: x86: Extract kvm_update_cpuid_runtime() from kvm_update_cpuid() Xiaoyao Li
@ 2020-07-09 4:34 ` Xiaoyao Li
2020-07-09 4:34 ` [PATCH v4 4/5] KVM: x86: Rename cpuid_update() callback to vcpu_after_set_cpuid() Xiaoyao Li
` (2 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Xiaoyao Li @ 2020-07-09 4:34 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson
Cc: Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
linux-kernel, Xiaoyao Li
Now there is no updating CPUID bits behavior in kvm_update_cpuid(),
rename it to kvm_vcpu_after_set_cpuid().
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
arch/x86/kvm/cpuid.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 0ed3b343c44e..b602c0c9078e 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -116,7 +116,7 @@ void kvm_update_cpuid_runtime(struct kvm_vcpu *vcpu)
}
}
-static void kvm_update_cpuid(struct kvm_vcpu *vcpu)
+static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
{
struct kvm_lapic *apic = vcpu->arch.apic;
struct kvm_cpuid_entry2 *best;
@@ -230,7 +230,7 @@ int kvm_vcpu_ioctl_set_cpuid(struct kvm_vcpu *vcpu,
cpuid_fix_nx_cap(vcpu);
kvm_x86_ops.cpuid_update(vcpu);
kvm_update_cpuid_runtime(vcpu);
- kvm_update_cpuid(vcpu);
+ kvm_vcpu_after_set_cpuid(vcpu);
kvfree(cpuid_entries);
out:
@@ -259,7 +259,7 @@ int kvm_vcpu_ioctl_set_cpuid2(struct kvm_vcpu *vcpu,
kvm_x86_ops.cpuid_update(vcpu);
kvm_update_cpuid_runtime(vcpu);
- kvm_update_cpuid(vcpu);
+ kvm_vcpu_after_set_cpuid(vcpu);
out:
return r;
}
--
2.18.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v4 4/5] KVM: x86: Rename cpuid_update() callback to vcpu_after_set_cpuid()
2020-07-09 4:34 [PATCH v4 0/5] Refactor handling flow of KVM_SET_CPUID* Xiaoyao Li
` (2 preceding siblings ...)
2020-07-09 4:34 ` [PATCH v4 3/5] KVM: x86: Rename kvm_update_cpuid() to kvm_vcpu_after_set_cpuid() Xiaoyao Li
@ 2020-07-09 4:34 ` Xiaoyao Li
2020-07-09 4:34 ` [PATCH v4 5/5] KVM: x86: Move kvm_x86_ops.vcpu_after_set_cpuid() into kvm_vcpu_after_set_cpuid() Xiaoyao Li
2020-07-09 11:08 ` [PATCH v4 0/5] Refactor handling flow of KVM_SET_CPUID* Paolo Bonzini
5 siblings, 0 replies; 8+ messages in thread
From: Xiaoyao Li @ 2020-07-09 4:34 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson
Cc: Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
linux-kernel, Xiaoyao Li
The name of callback cpuid_update() is misleading that it's not about
updating CPUID settings of vcpu but updating the configurations of vcpu
based on the CPUIDs. So rename it to vcpu_after_set_cpuid().
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
arch/x86/include/asm/kvm_host.h | 2 +-
arch/x86/kvm/cpuid.c | 4 ++--
arch/x86/kvm/svm/svm.c | 4 ++--
arch/x86/kvm/vmx/nested.c | 3 ++-
arch/x86/kvm/vmx/vmx.c | 4 ++--
5 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 16461a674406..3d7d818a282c 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1052,7 +1052,7 @@ struct kvm_x86_ops {
void (*hardware_unsetup)(void);
bool (*cpu_has_accelerated_tpr)(void);
bool (*has_emulated_msr)(u32 index);
- void (*cpuid_update)(struct kvm_vcpu *vcpu);
+ void (*vcpu_after_set_cpuid)(struct kvm_vcpu *vcpu);
unsigned int vm_size;
int (*vm_init)(struct kvm *kvm);
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index b602c0c9078e..832a24c1334e 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -228,7 +228,7 @@ int kvm_vcpu_ioctl_set_cpuid(struct kvm_vcpu *vcpu,
}
cpuid_fix_nx_cap(vcpu);
- kvm_x86_ops.cpuid_update(vcpu);
+ kvm_x86_ops.vcpu_after_set_cpuid(vcpu);
kvm_update_cpuid_runtime(vcpu);
kvm_vcpu_after_set_cpuid(vcpu);
@@ -257,7 +257,7 @@ int kvm_vcpu_ioctl_set_cpuid2(struct kvm_vcpu *vcpu,
goto out;
}
- kvm_x86_ops.cpuid_update(vcpu);
+ kvm_x86_ops.vcpu_after_set_cpuid(vcpu);
kvm_update_cpuid_runtime(vcpu);
kvm_vcpu_after_set_cpuid(vcpu);
out:
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 6d49afa3063f..535ad311ad02 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3595,7 +3595,7 @@ static u64 svm_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
return 0;
}
-static void svm_cpuid_update(struct kvm_vcpu *vcpu)
+static void svm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
{
struct vcpu_svm *svm = to_svm(vcpu);
@@ -4095,7 +4095,7 @@ static struct kvm_x86_ops svm_x86_ops __initdata = {
.get_exit_info = svm_get_exit_info,
- .cpuid_update = svm_cpuid_update,
+ .vcpu_after_set_cpuid = svm_vcpu_after_set_cpuid,
.has_wbinvd_exit = svm_has_wbinvd_exit,
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 7693d41a2446..e4080ab2df21 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -6354,7 +6354,8 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
/*
* secondary cpu-based controls. Do not include those that
- * depend on CPUID bits, they are added later by vmx_cpuid_update.
+ * depend on CPUID bits, they are added later by
+ * vmx_vcpu_after_set_cpuid.
*/
if (msrs->procbased_ctls_high & CPU_BASED_ACTIVATE_SECONDARY_CONTROLS)
rdmsr(MSR_IA32_VMX_PROCBASED_CTLS2,
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 0cdc78f2f2f4..2b41d987b101 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7282,7 +7282,7 @@ static void update_intel_pt_cfg(struct kvm_vcpu *vcpu)
vmx->pt_desc.ctl_bitmask &= ~(0xfULL << (32 + i * 4));
}
-static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
+static void vmx_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
{
struct vcpu_vmx *vmx = to_vmx(vcpu);
@@ -7940,7 +7940,7 @@ static struct kvm_x86_ops vmx_x86_ops __initdata = {
.get_exit_info = vmx_get_exit_info,
- .cpuid_update = vmx_cpuid_update,
+ .vcpu_after_set_cpuid = vmx_vcpu_after_set_cpuid,
.has_wbinvd_exit = cpu_has_vmx_wbinvd_exit,
--
2.18.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v4 5/5] KVM: x86: Move kvm_x86_ops.vcpu_after_set_cpuid() into kvm_vcpu_after_set_cpuid()
2020-07-09 4:34 [PATCH v4 0/5] Refactor handling flow of KVM_SET_CPUID* Xiaoyao Li
` (3 preceding siblings ...)
2020-07-09 4:34 ` [PATCH v4 4/5] KVM: x86: Rename cpuid_update() callback to vcpu_after_set_cpuid() Xiaoyao Li
@ 2020-07-09 4:34 ` Xiaoyao Li
2020-07-09 11:08 ` [PATCH v4 0/5] Refactor handling flow of KVM_SET_CPUID* Paolo Bonzini
5 siblings, 0 replies; 8+ messages in thread
From: Xiaoyao Li @ 2020-07-09 4:34 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson
Cc: Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
linux-kernel, Xiaoyao Li
kvm_x86_ops.vcpu_after_set_cpuid() is used to update vmx/svm specific
vcpu settings based on updated CPUID settings. So it's supposed to be
called after CPUIDs are updated, i.e., kvm_update_cpuid_runtime().
Currently, kvm_update_cpuid_runtime() only updates CPUID bits of OSXSAVE,
APIC, OSPKE, MWAIT, KVM_FEATURE_PV_UNHALT and CPUID(0xD,0).ebx and
CPUID(0xD, 1).ebx. None of them is consumed by vmx/svm's
update_vcpu_after_set_cpuid(). So there is no dependency between them.
Move kvm_x86_ops.vcpu_after_set_cpuid() into kvm_vcpu_after_set_cpuid() is
obviously more reasonable.
Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
---
arch/x86/kvm/cpuid.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 832a24c1334e..edbed4f522f2 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -121,6 +121,8 @@ static void kvm_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
struct kvm_lapic *apic = vcpu->arch.apic;
struct kvm_cpuid_entry2 *best;
+ kvm_x86_ops.vcpu_after_set_cpuid(vcpu);
+
best = kvm_find_cpuid_entry(vcpu, 1, 0);
if (best && apic) {
if (cpuid_entry_has(best, X86_FEATURE_TSC_DEADLINE_TIMER))
@@ -228,7 +230,6 @@ int kvm_vcpu_ioctl_set_cpuid(struct kvm_vcpu *vcpu,
}
cpuid_fix_nx_cap(vcpu);
- kvm_x86_ops.vcpu_after_set_cpuid(vcpu);
kvm_update_cpuid_runtime(vcpu);
kvm_vcpu_after_set_cpuid(vcpu);
@@ -257,7 +258,6 @@ int kvm_vcpu_ioctl_set_cpuid2(struct kvm_vcpu *vcpu,
goto out;
}
- kvm_x86_ops.vcpu_after_set_cpuid(vcpu);
kvm_update_cpuid_runtime(vcpu);
kvm_vcpu_after_set_cpuid(vcpu);
out:
--
2.18.4
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v4 0/5] Refactor handling flow of KVM_SET_CPUID*
2020-07-09 4:34 [PATCH v4 0/5] Refactor handling flow of KVM_SET_CPUID* Xiaoyao Li
` (4 preceding siblings ...)
2020-07-09 4:34 ` [PATCH v4 5/5] KVM: x86: Move kvm_x86_ops.vcpu_after_set_cpuid() into kvm_vcpu_after_set_cpuid() Xiaoyao Li
@ 2020-07-09 11:08 ` Paolo Bonzini
5 siblings, 0 replies; 8+ messages in thread
From: Paolo Bonzini @ 2020-07-09 11:08 UTC (permalink / raw)
To: Xiaoyao Li, Sean Christopherson
Cc: Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
linux-kernel
On 09/07/20 06:34, Xiaoyao Li wrote:
> 4 Patches of v3 has been queued into kvm/queue branch. This v4 contains
> the rest to refactor the flow of KVM_SET_CPUID* as:
>
> 1. cpuid check: check if userspace provides legal CPUID settings;
>
> 2. cpuid update: Update userspace provided CPUID settings. It currently
> only contains kvm_update_cpuid_runtime, which updates special CPUID
> bits based on the vcpu state, e.g., OSXSAVE, OSPKE. In the future, we
> can re-introduce kvm_update_cpuid() if KVM needs to force on/off some
> bits.
>
> 3. update vcpu states: Update vcpu states/settings based on the final updated
> CPUID settings.
>
> v4:
> - remove 4 queued patches
> - rebased to kvm/queue: c16ced9cc67a "x86/kvm/vmx: Use native read/write_cr2()"
> - fix one bug in v3 to call kvfree(cpuid_entries) in kvm_vcpu_ioctl_set_cpuid()
> - rename "update_vcpu_model" to "vcpu_after_set_cpuid" [Paolo]
> - Add a new patch to extrace kvm_update_cpuid_runtime()
>
> v3:
> https://lkml.kernel.org/r/20200708065054.19713-1-xiaoyao.li@intel.com
> - Add a note in KVM api doc to state the previous CPUID configuration
> is not reliable if current KVM_SET_CPUID* fails [Jim]
> - Adjust Patch 2 to reduce code churn [Sean]
> - Commit message refine to add more justification [Sean]
> - Add a new patch 7
>
> v2:
> https://lkml.kernel.org/r/20200623115816.24132-1-xiaoyao.li@intel.com
> - rebase to kvm/queue: a037ff353ba6 ("Merge branch 'kvm-master' into HEAD")
> - change the name of kvm_update_state_based_on_cpuid() to
> kvm_update_vcpu_model() [Sean]
> - Add patch 5 to rename kvm_x86_ops.cpuid_date() to
> kvm_x86_ops.update_vcpu_model()
>
> v1:
> https://lkml.kernel.org/r/20200529085545.29242-1-xiaoyao.li@intel.com
>
> Xiaoyao Li (5):
> KVM: x86: Introduce kvm_check_cpuid()
> KVM: x86: Extract kvm_update_cpuid_runtime() from kvm_update_cpuid()
> KVM: x86: Rename kvm_update_cpuid() to kvm_vcpu_after_set_cpuid()
> KVM: x86: Rename cpuid_update() callback to vcpu_after_set_cpuid()
> KVM: x86: Move kvm_x86_ops.vcpu_after_set_cpuid() into
> kvm_vcpu_after_set_cpuid()
>
> arch/x86/include/asm/kvm_host.h | 2 +-
> arch/x86/kvm/cpuid.c | 99 +++++++++++++++++++++------------
> arch/x86/kvm/cpuid.h | 2 +-
> arch/x86/kvm/lapic.c | 2 +-
> arch/x86/kvm/svm/svm.c | 4 +-
> arch/x86/kvm/vmx/nested.c | 3 +-
> arch/x86/kvm/vmx/vmx.c | 4 +-
> arch/x86/kvm/x86.c | 10 ++--
> 8 files changed, 76 insertions(+), 50 deletions(-)
>
Queued, thanks.
Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread