kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] kvm: x86: Improve emulation of CPUID leaves 0BH and 1FH
@ 2019-09-26  0:04 Jim Mattson
  2019-09-26  0:04 ` [PATCH 2/2] kvm: x86: Use AMD CPUID semantics for AMD vCPUs Jim Mattson
  0 siblings, 1 reply; 5+ messages in thread
From: Jim Mattson @ 2019-09-26  0:04 UTC (permalink / raw)
  To: kvm
  Cc: Jim Mattson, Marc Orr, Peter Shier, Jacob Xu,
	Sean Christopherson, Paolo Bonzini

For these CPUID leaves, the EDX output is not dependent on the ECX
input (i.e. the SIGNIFCANT_INDEX flag doesn't apply to
EDX). Furthermore, the low byte of the ECX output is always identical
to the low byte of the ECX input. KVM does not produce the correct ECX
and EDX outputs for any undefined subleaves beyond the first.

Special-case these CPUID leaves in kvm_cpuid, so that the ECX and EDX
outputs are properly generated for all undefined subleaves.

Fixes: 0771671749b59a ("KVM: Enhance guest cpuid management")
Fixes: a87f2d3a6eadab ("KVM: x86: Add Intel CPUID.1F cpuid emulation support")
Signed-off-by: Jim Mattson <jmattson@google.com>
Reviewed-by: Marc Orr <marcorr@google.com>
Reviewed-by: Peter Shier <pshier@google.com>
Reviewed-by: Jacob Xu <jacobhxu@google.com>
Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/cpuid.c | 83 +++++++++++++++++++++++++-------------------
 1 file changed, 47 insertions(+), 36 deletions(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index dd5985eb61b4c..35e2f930a4b79 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -963,53 +963,64 @@ 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.
+ * If the basic or extended CPUID leaf requested is higher than the
+ * maximum supported basic or extended leaf, respectively, then it is
+ * out of range.
  */
-static struct kvm_cpuid_entry2* check_cpuid_limit(struct kvm_vcpu *vcpu,
-                                                  u32 function, u32 index)
+static bool cpuid_function_in_range(struct kvm_vcpu *vcpu, u32 function)
 {
-	struct kvm_cpuid_entry2 *maxlevel;
-
-	maxlevel = kvm_find_cpuid_entry(vcpu, function & 0x80000000, 0);
-	if (!maxlevel || maxlevel->eax >= function)
-		return NULL;
-	if (function & 0x80000000) {
-		maxlevel = kvm_find_cpuid_entry(vcpu, 0, 0);
-		if (!maxlevel)
-			return NULL;
-	}
-	return kvm_find_cpuid_entry(vcpu, maxlevel->eax, index);
+	struct kvm_cpuid_entry2 *max;
+
+	max = kvm_find_cpuid_entry(vcpu, function & 0x80000000, 0);
+	return max && function <= max->eax;
 }
 
 bool kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx,
 	       u32 *ecx, u32 *edx, bool check_limit)
 {
 	u32 function = *eax, index = *ecx;
-	struct kvm_cpuid_entry2 *best;
-	bool entry_found = true;
-
-	best = kvm_find_cpuid_entry(vcpu, function, index);
-
-	if (!best) {
-		entry_found = false;
-		if (!check_limit)
-			goto out;
+	struct kvm_cpuid_entry2 *entry;
+	struct kvm_cpuid_entry2 *max;
+	bool found;
 
-		best = check_cpuid_limit(vcpu, function, index);
+	entry = kvm_find_cpuid_entry(vcpu, function, index);
+	found = entry;
+	/*
+	 * Intel CPUID semantics treats any query for an out-of-range
+	 * leaf as if the highest basic leaf (i.e. CPUID.0H:EAX) were
+	 * requested.
+	 */
+	if (!entry && check_limit && !cpuid_function_in_range(vcpu, function)) {
+		max = kvm_find_cpuid_entry(vcpu, 0, 0);
+		if (max) {
+			function = max->eax;
+			entry = kvm_find_cpuid_entry(vcpu, function, index);
+		}
 	}
-
-out:
-	if (best) {
-		*eax = best->eax;
-		*ebx = best->ebx;
-		*ecx = best->ecx;
-		*edx = best->edx;
-	} else
+	if (entry) {
+		*eax = entry->eax;
+		*ebx = entry->ebx;
+		*ecx = entry->ecx;
+		*edx = entry->edx;
+	} else {
 		*eax = *ebx = *ecx = *edx = 0;
-	trace_kvm_cpuid(function, *eax, *ebx, *ecx, *edx, entry_found);
-	return entry_found;
+		/*
+		 * When leaf 0BH or 1FH is defined, CL is pass-through
+		 * and EDX is always the x2APIC ID, even for undefined
+		 * subleaves. Index 1 will exist iff the leaf is
+		 * implemented, so we pass through CL iff leaf 1
+		 * exists. EDX can be copied from any existing index.
+		 */
+		if (function == 0xb || function == 0x1f) {
+			entry = kvm_find_cpuid_entry(vcpu, function, 1);
+			if (entry) {
+				*ecx = index & 0xff;
+				*edx = entry->edx;
+			}
+		}
+	}
+	trace_kvm_cpuid(function, *eax, *ebx, *ecx, *edx, found);
+	return found;
 }
 EXPORT_SYMBOL_GPL(kvm_cpuid);
 
-- 
2.23.0.351.gc4317032e6-goog


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

* [PATCH 2/2] kvm: x86: Use AMD CPUID semantics for AMD vCPUs
  2019-09-26  0:04 [PATCH 1/2] kvm: x86: Improve emulation of CPUID leaves 0BH and 1FH Jim Mattson
@ 2019-09-26  0:04 ` Jim Mattson
  2019-09-26  2:30   ` Xiaoyao Li
  2019-09-26 10:31   ` Paolo Bonzini
  0 siblings, 2 replies; 5+ messages in thread
From: Jim Mattson @ 2019-09-26  0:04 UTC (permalink / raw)
  To: kvm
  Cc: Jim Mattson, Marc Orr, Peter Shier, Jacob Xu,
	Sean Christopherson, Paolo Bonzini

When the guest CPUID information represents an AMD vCPU, return all
zeroes for queries of undefined CPUID leaves, whether or not they are
in range.

Signed-off-by: Jim Mattson <jmattson@google.com>
Fixes: bd22f5cfcfe8f6 ("KVM: move and fix substitue search for missing CPUID entries")
Reviewed-by: Marc Orr <marcorr@google.com>
Reviewed-by: Peter Shier <pshier@google.com>
Reviewed-by: Jacob Xu <jacobhxu@google.com>
Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Cc: Paolo Bonzini <pbonzini@redhat.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 35e2f930a4b79..0377d2820a7aa 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -988,9 +988,11 @@ bool kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx,
 	/*
 	 * Intel CPUID semantics treats any query for an out-of-range
 	 * leaf as if the highest basic leaf (i.e. CPUID.0H:EAX) were
-	 * requested.
+	 * requested. AMD CPUID semantics returns all zeroes for any
+	 * undefined leaf, whether or not the leaf is in range.
 	 */
-	if (!entry && check_limit && !cpuid_function_in_range(vcpu, function)) {
+	if (!entry && check_limit && !guest_cpuid_is_amd(vcpu) &&
+	    !cpuid_function_in_range(vcpu, function)) {
 		max = kvm_find_cpuid_entry(vcpu, 0, 0);
 		if (max) {
 			function = max->eax;
-- 
2.23.0.351.gc4317032e6-goog


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

* Re: [PATCH 2/2] kvm: x86: Use AMD CPUID semantics for AMD vCPUs
  2019-09-26  0:04 ` [PATCH 2/2] kvm: x86: Use AMD CPUID semantics for AMD vCPUs Jim Mattson
@ 2019-09-26  2:30   ` Xiaoyao Li
  2019-09-26 19:38     ` Jim Mattson
  2019-09-26 10:31   ` Paolo Bonzini
  1 sibling, 1 reply; 5+ messages in thread
From: Xiaoyao Li @ 2019-09-26  2:30 UTC (permalink / raw)
  To: Jim Mattson, kvm
  Cc: Marc Orr, Peter Shier, Jacob Xu, Sean Christopherson, Paolo Bonzini

On 9/26/2019 8:04 AM, Jim Mattson wrote:
> When the guest CPUID information represents an AMD vCPU, return all
> zeroes for queries of undefined CPUID leaves, whether or not they are
> in range.
> 
> Signed-off-by: Jim Mattson <jmattson@google.com>
> Fixes: bd22f5cfcfe8f6 ("KVM: move and fix substitue search for missing CPUID entries")
> Reviewed-by: Marc Orr <marcorr@google.com>
> Reviewed-by: Peter Shier <pshier@google.com>
> Reviewed-by: Jacob Xu <jacobhxu@google.com>
> Cc: Sean Christopherson <sean.j.christopherson@intel.com>
> Cc: Paolo Bonzini <pbonzini@redhat.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 35e2f930a4b79..0377d2820a7aa 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -988,9 +988,11 @@ bool kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx,
>   	/*
>   	 * Intel CPUID semantics treats any query for an out-of-range
>   	 * leaf as if the highest basic leaf (i.e. CPUID.0H:EAX) were
> -	 * requested.
> +	 * requested. AMD CPUID semantics returns all zeroes for any
> +	 * undefined leaf, whether or not the leaf is in range.
>   	 */
> -	if (!entry && check_limit && !cpuid_function_in_range(vcpu, function)) {
> +	if (!entry && check_limit && !guest_cpuid_is_amd(vcpu) &&
> +	    !cpuid_function_in_range(vcpu, function)) {

IIUC, the parameter check_limit is to indicate whether return highest 
basic leaf when out-of-range. Here you just makes check_limit meaningless.

Maybe we can do like this to use check_limit reasonably:

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 0377d2820a7a..e6a61f3f6c0c 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -1035,7 +1035,8 @@ int kvm_emulate_cpuid(struct kvm_vcpu *vcpu)

         eax = kvm_rax_read(vcpu);
         ecx = kvm_rcx_read(vcpu);
-       kvm_cpuid(vcpu, &eax, &ebx, &ecx, &edx, true);
+       kvm_cpuid(vcpu, &eax, &ebx, &ecx, &edx,
+                       guest_cpuid_is_amd(vcpu) ? false: true);
         kvm_rax_write(vcpu, eax);
         kvm_rbx_write(vcpu, ebx);
         kvm_rcx_write(vcpu, ecx);

>   		max = kvm_find_cpuid_entry(vcpu, 0, 0);
>   		if (max) {
>   			function = max->eax;
> 

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

* Re: [PATCH 2/2] kvm: x86: Use AMD CPUID semantics for AMD vCPUs
  2019-09-26  0:04 ` [PATCH 2/2] kvm: x86: Use AMD CPUID semantics for AMD vCPUs Jim Mattson
  2019-09-26  2:30   ` Xiaoyao Li
@ 2019-09-26 10:31   ` Paolo Bonzini
  1 sibling, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2019-09-26 10:31 UTC (permalink / raw)
  To: Jim Mattson, kvm; +Cc: Marc Orr, Peter Shier, Jacob Xu, Sean Christopherson

On 26/09/19 02:04, Jim Mattson wrote:
> When the guest CPUID information represents an AMD vCPU, return all
> zeroes for queries of undefined CPUID leaves, whether or not they are
> in range.
> 
> Signed-off-by: Jim Mattson <jmattson@google.com>
> Fixes: bd22f5cfcfe8f6 ("KVM: move and fix substitue search for missing CPUID entries")
> Reviewed-by: Marc Orr <marcorr@google.com>
> Reviewed-by: Peter Shier <pshier@google.com>
> Reviewed-by: Jacob Xu <jacobhxu@google.com>
> Cc: Sean Christopherson <sean.j.christopherson@intel.com>
> Cc: Paolo Bonzini <pbonzini@redhat.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 35e2f930a4b79..0377d2820a7aa 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -988,9 +988,11 @@ bool kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx,
>  	/*
>  	 * Intel CPUID semantics treats any query for an out-of-range
>  	 * leaf as if the highest basic leaf (i.e. CPUID.0H:EAX) were
> -	 * requested.
> +	 * requested. AMD CPUID semantics returns all zeroes for any
> +	 * undefined leaf, whether or not the leaf is in range.
>  	 */
> -	if (!entry && check_limit && !cpuid_function_in_range(vcpu, function)) {
> +	if (!entry && check_limit && !guest_cpuid_is_amd(vcpu) &&
> +	    !cpuid_function_in_range(vcpu, function)) {
>  		max = kvm_find_cpuid_entry(vcpu, 0, 0);
>  		if (max) {
>  			function = max->eax;
> 

Queued both, thanks.

Paolo


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

* Re: [PATCH 2/2] kvm: x86: Use AMD CPUID semantics for AMD vCPUs
  2019-09-26  2:30   ` Xiaoyao Li
@ 2019-09-26 19:38     ` Jim Mattson
  0 siblings, 0 replies; 5+ messages in thread
From: Jim Mattson @ 2019-09-26 19:38 UTC (permalink / raw)
  To: Xiaoyao Li
  Cc: kvm list, Marc Orr, Peter Shier, Jacob Xu, Sean Christopherson,
	Paolo Bonzini

On Wed, Sep 25, 2019 at 7:30 PM Xiaoyao Li <xiaoyao.li@intel.com> wrote:
>
> On 9/26/2019 8:04 AM, Jim Mattson wrote:
> > When the guest CPUID information represents an AMD vCPU, return all
> > zeroes for queries of undefined CPUID leaves, whether or not they are
> > in range.
> >
> > Signed-off-by: Jim Mattson <jmattson@google.com>
> > Fixes: bd22f5cfcfe8f6 ("KVM: move and fix substitue search for missing CPUID entries")
> > Reviewed-by: Marc Orr <marcorr@google.com>
> > Reviewed-by: Peter Shier <pshier@google.com>
> > Reviewed-by: Jacob Xu <jacobhxu@google.com>
> > Cc: Sean Christopherson <sean.j.christopherson@intel.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.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 35e2f930a4b79..0377d2820a7aa 100644
> > --- a/arch/x86/kvm/cpuid.c
> > +++ b/arch/x86/kvm/cpuid.c
> > @@ -988,9 +988,11 @@ bool kvm_cpuid(struct kvm_vcpu *vcpu, u32 *eax, u32 *ebx,
> >       /*
> >        * Intel CPUID semantics treats any query for an out-of-range
> >        * leaf as if the highest basic leaf (i.e. CPUID.0H:EAX) were
> > -      * requested.
> > +      * requested. AMD CPUID semantics returns all zeroes for any
> > +      * undefined leaf, whether or not the leaf is in range.
> >        */
> > -     if (!entry && check_limit && !cpuid_function_in_range(vcpu, function)) {
> > +     if (!entry && check_limit && !guest_cpuid_is_amd(vcpu) &&
> > +         !cpuid_function_in_range(vcpu, function)) {
>
> IIUC, the parameter check_limit is to indicate whether return highest
> basic leaf when out-of-range. Here you just makes check_limit meaningless.

That's right. For AMD CPUID semantics, there is no need for check_limit.

> Maybe we can do like this to use check_limit reasonably:
>
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 0377d2820a7a..e6a61f3f6c0c 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -1035,7 +1035,8 @@ int kvm_emulate_cpuid(struct kvm_vcpu *vcpu)
>
>          eax = kvm_rax_read(vcpu);
>          ecx = kvm_rcx_read(vcpu);
> -       kvm_cpuid(vcpu, &eax, &ebx, &ecx, &edx, true);
> +       kvm_cpuid(vcpu, &eax, &ebx, &ecx, &edx,
> +                       guest_cpuid_is_amd(vcpu) ? false: true);
>          kvm_rax_write(vcpu, eax);
>          kvm_rbx_write(vcpu, ebx);
>          kvm_rcx_write(vcpu, ecx);
>
> >               max = kvm_find_cpuid_entry(vcpu, 0, 0);
> >               if (max) {
> >                       function = max->eax;

Since over-limit CPUID queries should be rare, it seems unfortunate to
pay the cost of guest_cpuid_is_amd() for every emulated CPUID
instruction.

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

end of thread, other threads:[~2019-09-26 19:38 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-26  0:04 [PATCH 1/2] kvm: x86: Improve emulation of CPUID leaves 0BH and 1FH Jim Mattson
2019-09-26  0:04 ` [PATCH 2/2] kvm: x86: Use AMD CPUID semantics for AMD vCPUs Jim Mattson
2019-09-26  2:30   ` Xiaoyao Li
2019-09-26 19:38     ` Jim Mattson
2019-09-26 10:31   ` Paolo Bonzini

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).