KVM Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/2] KVM: CPUID: CPUID emulation flow adjustment and one minor refinement when updating maxphyaddr  
@ 2019-09-10  8:24 Xiaoyao Li
  2019-09-10  8:24 ` [PATCH 1/2] KVM: CPUID: Check limit first when emulating CPUID instruction Xiaoyao Li
  2019-09-10  8:24 ` [PATCH 2/2] KVM: CPUID: Put maxphyaddr updating together with virtual address width checking Xiaoyao Li
  0 siblings, 2 replies; 4+ messages in thread
From: Xiaoyao Li @ 2019-09-10  8:24 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář
  Cc: Xiaoyao Li, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li,
	Jim Mattson, Joerg Roedel, kvm

Patch 1 adjusts the execution flow of CPUID instruction emulation, which
checks the leaf number first per CPUID specification.

Patch 2 moves physical address width updating to where we check the virtual
address width in function kvm_update_cpuid() since they two use the same
cpuid leaf, which makes it more reasonable and no functional change.  

Xiaoyao Li (2):
  KVM: CPUID: Check limit first when emulating CPUID instruction
  KVM: CPUID: Put maxphyaddr updating together with virtual address
    width checking

 arch/x86/kvm/cpuid.c | 56 +++++++++++++++++++++++++-------------------
 1 file changed, 32 insertions(+), 24 deletions(-)

-- 
2.19.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] KVM: CPUID: Check limit first when emulating CPUID instruction
  2019-09-10  8:24 [PATCH 0/2] KVM: CPUID: CPUID emulation flow adjustment and one minor refinement when updating maxphyaddr Xiaoyao Li
@ 2019-09-10  8:24 ` Xiaoyao Li
  2019-09-10  9:52   ` Xiaoyao Li
  2019-09-10  8:24 ` [PATCH 2/2] KVM: CPUID: Put maxphyaddr updating together with virtual address width checking Xiaoyao Li
  1 sibling, 1 reply; 4+ messages in thread
From: Xiaoyao Li @ 2019-09-10  8:24 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář
  Cc: Xiaoyao Li, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li,
	Jim Mattson, Joerg Roedel, kvm

When limit checking is required, it should be executed first, which is
consistent with the CPUID specification.

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

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 22c2720cd948..866546b4d834 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -952,23 +952,33 @@ struct kvm_cpuid_entry2 *kvm_find_cpuid_entry(struct kvm_vcpu *vcpu,
 EXPORT_SYMBOL_GPL(kvm_find_cpuid_entry);
 
 /*
- * If no match is found, check whether we exceed the vCPU's limit
- * and return the content of the highest valid _standard_ leaf instead.
- * This is to satisfy the CPUID specification.
+ * Based on CPUID specification, if leaf number exceeds the vCPU's limit,
+ * it should return the content of the highest valid _standard_ leaf instead.
+ * Note: *found is set true only means the queried leaf number doesn't exceed
+ * the maximum leaf number of basic or extented leaf.
  */
-static struct kvm_cpuid_entry2* check_cpuid_limit(struct kvm_vcpu *vcpu,
-                                                  u32 function, u32 index)
+static struct kvm_cpuid_entry2* cpuid_check_limit(struct kvm_vcpu *vcpu,
+                                                  u32 function, u32 index,
+						  bool *found)
 {
 	struct kvm_cpuid_entry2 *maxlevel;
 
 	maxlevel = kvm_find_cpuid_entry(vcpu, function & 0x80000000, 0);
-	if (!maxlevel || maxlevel->eax >= function)
+	if (!maxlevel)
 		return NULL;
-	if (function & 0x80000000) {
-		maxlevel = kvm_find_cpuid_entry(vcpu, 0, 0);
-		if (!maxlevel)
-			return NULL;
+
+	if (maxlevel->eax >= function) {
+		if (found)
+			*found = true;
+		return kvm_find_cpuid_entry(vcpu, function, index);
 	}
+
+	if (function & 0x80000000)
+		maxlevel = kvm_find_cpuid_entry(vcpu, 0, 0);
+
+	if (!maxlevel)
+		return NULL;
+
 	return kvm_find_cpuid_entry(vcpu, maxlevel->eax, index);
 }
 
@@ -977,26 +987,22 @@ bool kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx,
 {
 	u32 function = *eax, index = *ecx;
 	struct kvm_cpuid_entry2 *best;
-	bool entry_found = true;
+	bool entry_found = false;
 
-	best = kvm_find_cpuid_entry(vcpu, function, index);
+	if (check_limit)
+		best = cpuid_check_limit(vcpu, function, index, &entry_found);
+	else
+		best = kvm_find_cpuid_entry(vcpu, function, index);
 
-	if (!best) {
-		entry_found = false;
-		if (!check_limit)
-			goto out;
-
-		best = check_cpuid_limit(vcpu, function, index);
-	}
-
-out:
 	if (best) {
 		*eax = best->eax;
 		*ebx = best->ebx;
 		*ecx = best->ecx;
 		*edx = best->edx;
-	} else
+	} else {
+		entry_found = false;
 		*eax = *ebx = *ecx = *edx = 0;
+	}
 	trace_kvm_cpuid(function, *eax, *ebx, *ecx, *edx, entry_found);
 	return entry_found;
 }
-- 
2.19.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/2] KVM: CPUID: Put maxphyaddr updating together with virtual address width checking
  2019-09-10  8:24 [PATCH 0/2] KVM: CPUID: CPUID emulation flow adjustment and one minor refinement when updating maxphyaddr Xiaoyao Li
  2019-09-10  8:24 ` [PATCH 1/2] KVM: CPUID: Check limit first when emulating CPUID instruction Xiaoyao Li
@ 2019-09-10  8:24 ` Xiaoyao Li
  1 sibling, 0 replies; 4+ messages in thread
From: Xiaoyao Li @ 2019-09-10  8:24 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář
  Cc: Xiaoyao Li, Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li,
	Jim Mattson, Joerg Roedel, kvm

Since both of maxphyaddr updating and virtual address width checking
need to query the cpuid leaf 0x80000008. We can put them together.

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

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 866546b4d834..5e7039d421ac 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -118,6 +118,7 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu)
 		best->ebx = xstate_required_size(vcpu->arch.xcr0, true);
 
 	/*
+	 * Update physical address width and check virtual address width.
 	 * The existing code assumes virtual address is 48-bit or 57-bit in the
 	 * canonical address checks; exit if it is ever changed.
 	 */
@@ -127,7 +128,10 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu)
 
 		if (vaddr_bits != 48 && vaddr_bits != 57 && vaddr_bits != 0)
 			return -EINVAL;
+
+		vcpu->arch.maxphyaddr = best->eax & 0xff;
 	}
+	vcpu->arch.maxphyaddr = 36;
 
 	best = kvm_find_cpuid_entry(vcpu, KVM_CPUID_FEATURES, 0);
 	if (kvm_hlt_in_guest(vcpu->kvm) && best &&
@@ -144,8 +148,6 @@ int kvm_update_cpuid(struct kvm_vcpu *vcpu)
 		}
 	}
 
-	/* Update physical-address width */
-	vcpu->arch.maxphyaddr = cpuid_query_maxphyaddr(vcpu);
 	kvm_mmu_reset_context(vcpu);
 
 	kvm_pmu_refresh(vcpu);
-- 
2.19.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/2] KVM: CPUID: Check limit first when emulating CPUID instruction
  2019-09-10  8:24 ` [PATCH 1/2] KVM: CPUID: Check limit first when emulating CPUID instruction Xiaoyao Li
@ 2019-09-10  9:52   ` Xiaoyao Li
  0 siblings, 0 replies; 4+ messages in thread
From: Xiaoyao Li @ 2019-09-10  9:52 UTC (permalink / raw)
  To: Paolo Bonzini, Radim Krčmář
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm

On Tue, 2019-09-10 at 16:24 +0800, Xiaoyao Li wrote:
> When limit checking is required, it should be executed first, which is
> consistent with the CPUID specification.
> 
> Signed-off-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
>  arch/x86/kvm/cpuid.c | 50 +++++++++++++++++++++++++-------------------
>  1 file changed, 28 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 22c2720cd948..866546b4d834 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -952,23 +952,33 @@ struct kvm_cpuid_entry2 *kvm_find_cpuid_entry(struct
> kvm_vcpu *vcpu,
>  EXPORT_SYMBOL_GPL(kvm_find_cpuid_entry);
>  
>  /*
> - * If no match is found, check whether we exceed the vCPU's limit
> - * and return the content of the highest valid _standard_ leaf instead.
> - * This is to satisfy the CPUID specification.
> + * Based on CPUID specification, if leaf number exceeds the vCPU's limit,
> + * it should return the content of the highest valid _standard_ leaf instead.
> + * Note: *found is set true only means the queried leaf number doesn't exceed
> + * the maximum leaf number of basic or extented leaf.
>   */
> -static struct kvm_cpuid_entry2* check_cpuid_limit(struct kvm_vcpu *vcpu,
> -                                                  u32 function, u32 index)
> +static struct kvm_cpuid_entry2* cpuid_check_limit(struct kvm_vcpu *vcpu,
> +                                                  u32 function, u32 index,
> +						  bool *found)
>  {
>  	struct kvm_cpuid_entry2 *maxlevel;
>  
>  	maxlevel = kvm_find_cpuid_entry(vcpu, function & 0x80000000, 0);
> -	if (!maxlevel || maxlevel->eax >= function)
> +	if (!maxlevel)
>  		return NULL;
> -	if (function & 0x80000000) {
> -		maxlevel = kvm_find_cpuid_entry(vcpu, 0, 0);
> -		if (!maxlevel)
> -			return NULL;
> +
> +	if (maxlevel->eax >= function) {
> +		if (found)
> +			*found = true;
> +		return kvm_find_cpuid_entry(vcpu, function, index);
>  	}
> +
> +	if (function & 0x80000000)
> +		maxlevel = kvm_find_cpuid_entry(vcpu, 0, 0);
> +
> +	if (!maxlevel)
> +		return NULL;
> +
>  	return kvm_find_cpuid_entry(vcpu, maxlevel->eax, index);
>  }
>  
> @@ -977,26 +987,22 @@ bool kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32
> *ebx,
>  {
>  	u32 function = *eax, index = *ecx;
>  	struct kvm_cpuid_entry2 *best;
> -	bool entry_found = true;
> +	bool entry_found = false;
>  
> -	best = kvm_find_cpuid_entry(vcpu, function, index);
> +	if (check_limit)
> +		best = cpuid_check_limit(vcpu, function, index, &entry_found);
> +	else
> +		best = kvm_find_cpuid_entry(vcpu, function, index);
>  
> -	if (!best) {
> -		entry_found = false;
> -		if (!check_limit)
> -			goto out;
> -
> -		best = check_cpuid_limit(vcpu, function, index);
> -	}
> -
> -out:
>  	if (best) {
>  		*eax = best->eax;
>  		*ebx = best->ebx;
>  		*ecx = best->ecx;
>  		*edx = best->edx;
> -	} else
> +	} else {
> +		entry_found = false;
>  		*eax = *ebx = *ecx = *edx = 0;
> +	}
>  	trace_kvm_cpuid(function, *eax, *ebx, *ecx, *edx, entry_found);
>  	return entry_found;
>  }

Just realise the entry_found is not set true if found in no limit checking case.
Will send v2. 


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-10  8:24 [PATCH 0/2] KVM: CPUID: CPUID emulation flow adjustment and one minor refinement when updating maxphyaddr Xiaoyao Li
2019-09-10  8:24 ` [PATCH 1/2] KVM: CPUID: Check limit first when emulating CPUID instruction Xiaoyao Li
2019-09-10  9:52   ` Xiaoyao Li
2019-09-10  8:24 ` [PATCH 2/2] KVM: CPUID: Put maxphyaddr updating together with virtual address width checking Xiaoyao Li

KVM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvm/0 kvm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvm kvm/ https://lore.kernel.org/kvm \
		kvm@vger.kernel.org kvm@archiver.kernel.org
	public-inbox-index kvm


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.kvm


AGPL code for this site: git clone https://public-inbox.org/ public-inbox