linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] Refactor handling flow of SET_CPUID*
@ 2020-05-29  8:55 Xiaoyao Li
  2020-05-29  8:55 ` [PATCH 1/6] KVM: X86: Reset vcpu->arch.cpuid_nent to 0 if SET_CPUID fails Xiaoyao Li
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Xiaoyao Li @ 2020-05-29  8:55 UTC (permalink / raw)
  To: Paolo Bonzini, Sean Christopherson, kvm
  Cc: Vitaly Kuznetsov, Jim Mattson, linux-kernel, Xiaoyao Li

This serial is the extended version of
https://lkml.kernel.org/r/20200528151927.14346-1-xiaoyao.li@intel.com

First two patches are bug fixing, and the other aim to refactor the flow
of SET_CPUID* as:
1. cpuid check: check if userspace provides legal CPUID settings;

2. cpuid update: Update some special CPUID bits based on current vcpu
                 state, e.g., OSXSAVE, OSPKE, ...

3. update KVM state: Update KVM states based on the final CPUID
                     settings. 

Xiaoyao Li (6):
  KVM: X86: Reset vcpu->arch.cpuid_nent to 0 if SET_CPUID fails
  KVM: X86: Go on updating other CPUID leaves when leaf 1 is absent
  KVM: X86: Introduce kvm_check_cpuid()
  KVM: X86: Split kvm_update_cpuid()
  KVM: X86: Move kvm_x86_ops.cpuid_update() into
    kvm_update_state_based_on_cpuid()
  KVM: X86: Move kvm_apic_set_version() to
    kvm_update_state_based_on_cpuid()

 arch/x86/kvm/cpuid.c | 107 +++++++++++++++++++++++++++----------------
 arch/x86/kvm/cpuid.h |   3 +-
 arch/x86/kvm/x86.c   |   1 +
 3 files changed, 70 insertions(+), 41 deletions(-)

-- 
2.18.2


^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH 1/6] KVM: X86: Reset vcpu->arch.cpuid_nent to 0 if SET_CPUID fails
  2020-05-29  8:55 [PATCH 0/6] Refactor handling flow of SET_CPUID* Xiaoyao Li
@ 2020-05-29  8:55 ` Xiaoyao Li
  2020-05-29  8:55 ` [PATCH 2/6] KVM: X86: Go on updating other CPUID leaves when leaf 1 is absent Xiaoyao Li
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Xiaoyao Li @ 2020-05-29  8:55 UTC (permalink / raw)
  To: Paolo Bonzini, Sean Christopherson, kvm
  Cc: Vitaly Kuznetsov, Jim Mattson, linux-kernel, Xiaoyao Li

It needs to invalidate CPUID configruations if usersapce provides
illegal input.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 arch/x86/kvm/cpuid.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index cd708b0b460a..2f1a9650b7f2 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -210,6 +210,8 @@ int kvm_vcpu_ioctl_set_cpuid(struct kvm_vcpu *vcpu,
 	kvm_apic_set_version(vcpu);
 	kvm_x86_ops.cpuid_update(vcpu);
 	r = kvm_update_cpuid(vcpu);
+	if (r)
+		vcpu->arch.cpuid_nent = 0;
 
 out:
 	vfree(cpuid_entries);
@@ -233,6 +235,8 @@ int kvm_vcpu_ioctl_set_cpuid2(struct kvm_vcpu *vcpu,
 	kvm_apic_set_version(vcpu);
 	kvm_x86_ops.cpuid_update(vcpu);
 	r = kvm_update_cpuid(vcpu);
+	if (r)
+		vcpu->arch.cpuid_nent = 0;
 out:
 	return r;
 }
-- 
2.18.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH 2/6] KVM: X86: Go on updating other CPUID leaves when leaf 1 is absent
  2020-05-29  8:55 [PATCH 0/6] Refactor handling flow of SET_CPUID* Xiaoyao Li
  2020-05-29  8:55 ` [PATCH 1/6] KVM: X86: Reset vcpu->arch.cpuid_nent to 0 if SET_CPUID fails Xiaoyao Li
@ 2020-05-29  8:55 ` Xiaoyao Li
  2020-05-29  8:55 ` [PATCH 3/6] KVM: X86: Introduce kvm_check_cpuid() Xiaoyao Li
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Xiaoyao Li @ 2020-05-29  8:55 UTC (permalink / raw)
  To: Paolo Bonzini, Sean Christopherson, kvm
  Cc: Vitaly Kuznetsov, Jim Mattson, linux-kernel, Xiaoyao Li

As handling of bits other leaf 1 added over time, kvm_update_cpuid()
should not return directly if leaf 1 is absent, but should go on
updateing other CPUID leaves.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 arch/x86/kvm/cpuid.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 2f1a9650b7f2..795bbaf37110 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -60,22 +60,21 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu)
 	struct kvm_lapic *apic = vcpu->arch.apic;
 
 	best = kvm_find_cpuid_entry(vcpu, 1, 0);
-	if (!best)
-		return 0;
-
-	/* Update OSXSAVE bit */
-	if (boot_cpu_has(X86_FEATURE_XSAVE) && best->function == 0x1)
-		cpuid_entry_change(best, X86_FEATURE_OSXSAVE,
+	if (best) {
+		/* Update OSXSAVE bit */
+		if (boot_cpu_has(X86_FEATURE_XSAVE))
+			cpuid_entry_change(best, X86_FEATURE_OSXSAVE,
 				   kvm_read_cr4_bits(vcpu, X86_CR4_OSXSAVE));
 
-	cpuid_entry_change(best, X86_FEATURE_APIC,
+		cpuid_entry_change(best, X86_FEATURE_APIC,
 			   vcpu->arch.apic_base & MSR_IA32_APICBASE_ENABLE);
 
-	if (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;
+		if (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;
+		}
 	}
 
 	best = kvm_find_cpuid_entry(vcpu, 7, 0);
-- 
2.18.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH 3/6] KVM: X86: Introduce kvm_check_cpuid()
  2020-05-29  8:55 [PATCH 0/6] Refactor handling flow of SET_CPUID* Xiaoyao Li
  2020-05-29  8:55 ` [PATCH 1/6] KVM: X86: Reset vcpu->arch.cpuid_nent to 0 if SET_CPUID fails Xiaoyao Li
  2020-05-29  8:55 ` [PATCH 2/6] KVM: X86: Go on updating other CPUID leaves when leaf 1 is absent Xiaoyao Li
@ 2020-05-29  8:55 ` Xiaoyao Li
  2020-05-29  8:55 ` [PATCH 4/6] KVM: X86: Split kvm_update_cpuid() Xiaoyao Li
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Xiaoyao Li @ 2020-05-29  8:55 UTC (permalink / raw)
  To: Paolo Bonzini, Sean Christopherson, kvm
  Cc: Vitaly Kuznetsov, Jim Mattson, linux-kernel, Xiaoyao Li

Use kvm_check_cpuid() to validate if userspace provides legal cpuid
settings and call it before KVM updates CPUID.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
Is the check of virutal address width really necessary?
KVM doesn't check other bits at all. I guess the policy is that KVM allows
illegal CPUID settings as long as it doesn't break host kernel. Userspace
takes the consequences itself if it sets bogus CPUID settings that breaks
its guest.
But why vaddr_bits is special? It seems illegal vaddr_bits won't break host
kernel.
---
 arch/x86/kvm/cpuid.c | 54 ++++++++++++++++++++++++++++----------------
 arch/x86/kvm/cpuid.h |  2 +-
 2 files changed, 35 insertions(+), 21 deletions(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 795bbaf37110..c8cb373056f1 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;
@@ -96,18 +115,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)))
@@ -127,7 +134,6 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu)
 	kvm_mmu_reset_context(vcpu);
 
 	kvm_pmu_refresh(vcpu);
-	return 0;
 }
 
 static int is_efer_nx(void)
@@ -205,12 +211,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;
+		goto out;
+	}
+
 	cpuid_fix_nx_cap(vcpu);
 	kvm_apic_set_version(vcpu);
 	kvm_x86_ops.cpuid_update(vcpu);
-	r = kvm_update_cpuid(vcpu);
-	if (r)
-		vcpu->arch.cpuid_nent = 0;
+	kvm_update_cpuid(vcpu);
 
 out:
 	vfree(cpuid_entries);
@@ -231,11 +241,15 @@ 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;
+	r = kvm_check_cpuid(vcpu);
+	if (r) {
+		vcpu->arch.cpuid_nent = 0;
+		goto out;
+	}
+
 	kvm_apic_set_version(vcpu);
 	kvm_x86_ops.cpuid_update(vcpu);
-	r = kvm_update_cpuid(vcpu);
-	if (r)
-		vcpu->arch.cpuid_nent = 0;
+	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.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH 4/6] KVM: X86: Split kvm_update_cpuid()
  2020-05-29  8:55 [PATCH 0/6] Refactor handling flow of SET_CPUID* Xiaoyao Li
                   ` (2 preceding siblings ...)
  2020-05-29  8:55 ` [PATCH 3/6] KVM: X86: Introduce kvm_check_cpuid() Xiaoyao Li
@ 2020-05-29  8:55 ` Xiaoyao Li
  2020-06-03  1:10   ` Sean Christopherson
  2020-05-29  8:55 ` [PATCH 5/6] KVM: X86: Move kvm_x86_ops.cpuid_update() into kvm_update_state_based_on_cpuid() Xiaoyao Li
  2020-05-29  8:55 ` [PATCH 6/6] KVM: X86: Move kvm_apic_set_version() to kvm_update_state_based_on_cpuid() Xiaoyao Li
  5 siblings, 1 reply; 9+ messages in thread
From: Xiaoyao Li @ 2020-05-29  8:55 UTC (permalink / raw)
  To: Paolo Bonzini, Sean Christopherson, kvm
  Cc: Vitaly Kuznetsov, Jim Mattson, linux-kernel, Xiaoyao Li

Split the part of updating KVM states from kvm_update_cpuid(), and put
it into a new kvm_update_state_based_on_cpuid(). So it's clear that
kvm_update_cpuid() is to update guest CPUID settings, while
kvm_update_state_based_on_cpuid() is to update KVM states based on the
updated CPUID settings.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
 arch/x86/kvm/cpuid.c | 38 ++++++++++++++++++++++++--------------
 arch/x86/kvm/cpuid.h |  1 +
 arch/x86/kvm/x86.c   |  1 +
 3 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index c8cb373056f1..a4a2072f5253 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -76,7 +76,6 @@ static int kvm_check_cpuid(struct kvm_vcpu *vcpu)
 void kvm_update_cpuid(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) {
@@ -87,13 +86,6 @@ void kvm_update_cpuid(struct kvm_vcpu *vcpu)
 
 		cpuid_entry_change(best, X86_FEATURE_APIC,
 			   vcpu->arch.apic_base & MSR_IA32_APICBASE_ENABLE);
-
-		if (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;
-		}
 	}
 
 	best = kvm_find_cpuid_entry(vcpu, 7, 0);
@@ -102,13 +94,8 @@ void kvm_update_cpuid(struct kvm_vcpu *vcpu)
 				   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) ||
@@ -127,6 +114,27 @@ void kvm_update_cpuid(struct kvm_vcpu *vcpu)
 					   vcpu->arch.ia32_misc_enable_msr &
 					   MSR_IA32_MISC_ENABLE_MWAIT);
 	}
+}
+
+void kvm_update_state_based_on_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;
+	}
+
+	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,
 	kvm_apic_set_version(vcpu);
 	kvm_x86_ops.cpuid_update(vcpu);
 	kvm_update_cpuid(vcpu);
+	kvm_update_state_based_on_cpuid(vcpu);
 
 out:
 	vfree(cpuid_entries);
@@ -250,6 +259,7 @@ int kvm_vcpu_ioctl_set_cpuid2(struct kvm_vcpu *vcpu,
 	kvm_apic_set_version(vcpu);
 	kvm_x86_ops.cpuid_update(vcpu);
 	kvm_update_cpuid(vcpu);
+	kvm_update_state_based_on_cpuid(vcpu);
 out:
 	return r;
 }
diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h
index f136de1debad..0ccf9ec4bf55 100644
--- a/arch/x86/kvm/cpuid.h
+++ b/arch/x86/kvm/cpuid.h
@@ -10,6 +10,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_state_based_on_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,
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index dc993b79ae2f..fcb85432d30b 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8100,6 +8100,7 @@ static void enter_smm(struct kvm_vcpu *vcpu)
 #endif
 
 	kvm_update_cpuid(vcpu);
+	kvm_update_state_based_on_cpuid(vcpu);
 	kvm_mmu_reset_context(vcpu);
 }
 
-- 
2.18.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH 5/6] KVM: X86: Move kvm_x86_ops.cpuid_update() into kvm_update_state_based_on_cpuid()
  2020-05-29  8:55 [PATCH 0/6] Refactor handling flow of SET_CPUID* Xiaoyao Li
                   ` (3 preceding siblings ...)
  2020-05-29  8:55 ` [PATCH 4/6] KVM: X86: Split kvm_update_cpuid() Xiaoyao Li
@ 2020-05-29  8:55 ` Xiaoyao Li
  2020-05-29  8:55 ` [PATCH 6/6] KVM: X86: Move kvm_apic_set_version() to kvm_update_state_based_on_cpuid() Xiaoyao Li
  5 siblings, 0 replies; 9+ messages in thread
From: Xiaoyao Li @ 2020-05-29  8:55 UTC (permalink / raw)
  To: Paolo Bonzini, Sean Christopherson, kvm
  Cc: Vitaly Kuznetsov, Jim Mattson, linux-kernel, Xiaoyao Li

kvm_x86_ops.cpuid_update() is used to update vmx/svm settings based on
updated CPUID settings. So it's supposed to be called after CPUIDs are
fully updated, i.e., kvm_update_cpuid(), not in the middle stage.

Put it in kvm_update_state_based_on_cpuid() to make it clear that it's
to update vmx/svm specific states based on CPUID.

Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
---
Should we rename kvm_x86_ops.cpuid_update to something like
kvm_x86_ops.update_state_based_on_cpuid?

cpuid_update is really confusing especially when kvm_x86_ops.update_cpuid()
is needed someday.
---
 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 a4a2072f5253..5d4da8970940 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -136,6 +136,8 @@ void kvm_update_state_based_on_cpuid(struct kvm_vcpu *vcpu)
 		vcpu->arch.guest_supported_xcr0 =
 			(best->eax | ((u64)best->edx << 32)) & supported_xcr0;
 
+	kvm_x86_ops.cpuid_update(vcpu);
+
 	/* Note, maxphyaddr must be updated before tdp_level. */
 	vcpu->arch.maxphyaddr = cpuid_query_maxphyaddr(vcpu);
 	vcpu->arch.tdp_level = kvm_x86_ops.get_tdp_level(vcpu);
@@ -227,7 +229,6 @@ int kvm_vcpu_ioctl_set_cpuid(struct kvm_vcpu *vcpu,
 
 	cpuid_fix_nx_cap(vcpu);
 	kvm_apic_set_version(vcpu);
-	kvm_x86_ops.cpuid_update(vcpu);
 	kvm_update_cpuid(vcpu);
 	kvm_update_state_based_on_cpuid(vcpu);
 
@@ -257,7 +258,6 @@ int kvm_vcpu_ioctl_set_cpuid2(struct kvm_vcpu *vcpu,
 	}
 
 	kvm_apic_set_version(vcpu);
-	kvm_x86_ops.cpuid_update(vcpu);
 	kvm_update_cpuid(vcpu);
 	kvm_update_state_based_on_cpuid(vcpu);
 out:
-- 
2.18.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH 6/6] KVM: X86: Move kvm_apic_set_version() to kvm_update_state_based_on_cpuid()
  2020-05-29  8:55 [PATCH 0/6] Refactor handling flow of SET_CPUID* Xiaoyao Li
                   ` (4 preceding siblings ...)
  2020-05-29  8:55 ` [PATCH 5/6] KVM: X86: Move kvm_x86_ops.cpuid_update() into kvm_update_state_based_on_cpuid() Xiaoyao Li
@ 2020-05-29  8:55 ` Xiaoyao Li
  5 siblings, 0 replies; 9+ messages in thread
From: Xiaoyao Li @ 2020-05-29  8:55 UTC (permalink / raw)
  To: Paolo Bonzini, Sean Christopherson, kvm
  Cc: Vitaly Kuznetsov, Jim Mattson, linux-kernel, Xiaoyao Li

Obviously, kvm_apic_set_version() fits well in
kvm_update_state_based_on_cpuid().

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 5d4da8970940..eb60098aca29 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -127,6 +127,8 @@ void kvm_update_state_based_on_cpuid(struct kvm_vcpu *vcpu)
 			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);
@@ -228,7 +230,6 @@ int kvm_vcpu_ioctl_set_cpuid(struct kvm_vcpu *vcpu,
 	}
 
 	cpuid_fix_nx_cap(vcpu);
-	kvm_apic_set_version(vcpu);
 	kvm_update_cpuid(vcpu);
 	kvm_update_state_based_on_cpuid(vcpu);
 
@@ -257,7 +258,6 @@ int kvm_vcpu_ioctl_set_cpuid2(struct kvm_vcpu *vcpu,
 		goto out;
 	}
 
-	kvm_apic_set_version(vcpu);
 	kvm_update_cpuid(vcpu);
 	kvm_update_state_based_on_cpuid(vcpu);
 out:
-- 
2.18.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [PATCH 4/6] KVM: X86: Split kvm_update_cpuid()
  2020-05-29  8:55 ` [PATCH 4/6] KVM: X86: Split kvm_update_cpuid() Xiaoyao Li
@ 2020-06-03  1:10   ` Sean Christopherson
  2020-06-03  7:44     ` Xiaoyao Li
  0 siblings, 1 reply; 9+ messages in thread
From: Sean Christopherson @ 2020-06-03  1:10 UTC (permalink / raw)
  To: Xiaoyao Li
  Cc: Paolo Bonzini, kvm, Vitaly Kuznetsov, Jim Mattson, linux-kernel

On Fri, May 29, 2020 at 04:55:43PM +0800, Xiaoyao Li wrote:
> Split the part of updating KVM states from kvm_update_cpuid(), and put
> it into a new kvm_update_state_based_on_cpuid(). So it's clear that
> kvm_update_cpuid() is to update guest CPUID settings, while
> kvm_update_state_based_on_cpuid() is to update KVM states based on the
> updated CPUID settings.

What about kvm_update_vcpu_model()?  "state" isn't necessarily correct
either.

^ permalink raw reply	[flat|nested] 9+ messages in thread
* Re: [PATCH 4/6] KVM: X86: Split kvm_update_cpuid()
  2020-06-03  1:10   ` Sean Christopherson
@ 2020-06-03  7:44     ` Xiaoyao Li
  0 siblings, 0 replies; 9+ messages in thread
From: Xiaoyao Li @ 2020-06-03  7:44 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Paolo Bonzini, kvm, Vitaly Kuznetsov, Jim Mattson, linux-kernel

On 6/3/2020 9:10 AM, Sean Christopherson wrote:
> On Fri, May 29, 2020 at 04:55:43PM +0800, Xiaoyao Li wrote:
>> Split the part of updating KVM states from kvm_update_cpuid(), and put
>> it into a new kvm_update_state_based_on_cpuid(). So it's clear that
>> kvm_update_cpuid() is to update guest CPUID settings, while
>> kvm_update_state_based_on_cpuid() is to update KVM states based on the
>> updated CPUID settings.
> 
> What about kvm_update_vcpu_model()?  "state" isn't necessarily correct
> either.
> 

yeah, it's better.

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-06-03  7:44 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-29  8:55 [PATCH 0/6] Refactor handling flow of SET_CPUID* Xiaoyao Li
2020-05-29  8:55 ` [PATCH 1/6] KVM: X86: Reset vcpu->arch.cpuid_nent to 0 if SET_CPUID fails Xiaoyao Li
2020-05-29  8:55 ` [PATCH 2/6] KVM: X86: Go on updating other CPUID leaves when leaf 1 is absent Xiaoyao Li
2020-05-29  8:55 ` [PATCH 3/6] KVM: X86: Introduce kvm_check_cpuid() Xiaoyao Li
2020-05-29  8:55 ` [PATCH 4/6] KVM: X86: Split kvm_update_cpuid() Xiaoyao Li
2020-06-03  1:10   ` Sean Christopherson
2020-06-03  7:44     ` Xiaoyao Li
2020-05-29  8:55 ` [PATCH 5/6] KVM: X86: Move kvm_x86_ops.cpuid_update() into kvm_update_state_based_on_cpuid() Xiaoyao Li
2020-05-29  8:55 ` [PATCH 6/6] KVM: X86: Move kvm_apic_set_version() to kvm_update_state_based_on_cpuid() Xiaoyao Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).