[PATCH] kvm: fix cpuid eax

[PATCH] kvm: fix cpuid eax

[Michael S. Tsirkin]

cpuid eax should return the max leaf so that
guests can find out the valid range.
This matches Xen et al.

Tested using -cpu host.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 arch/x86/kvm/cpuid.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 7d00d2d..bda4877 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -397,7 +397,7 @@ static int do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 	case KVM_CPUID_SIGNATURE: {
 		char signature[12] = "KVMKVMKVM\0\0";
 		u32 *sigptr = (u32 *)signature;
-		entry->eax = 0;
+		entry->eax = KVM_CPUID_FEATURES;
 		entry->ebx = sigptr[0];
 		entry->ecx = sigptr[1];
 		entry->edx = sigptr[2];
-- 
MST

Re: [PATCH] kvm: fix cpuid eax

[H. Peter Anvin]

On 04/30/2012 07:39 AM, Michael S. Tsirkin wrote:
> cpuid eax should return the max leaf so that
> guests can find out the valid range.
> This matches Xen et al.

Not to mention it's what all the "real" CPU leaf chunks do.

Acked-by: H. Peter Anvin <hpa@zytor.com>

Re: [PATCH] kvm: fix cpuid eax

[Anthony Liguori]

On 04/30/2012 09:39 AM, Michael S. Tsirkin wrote:
> cpuid eax should return the max leaf so that
> guests can find out the valid range.
> This matches Xen et al.

What KVM does here predates Xen and Hyper-V.

This is an ABI breaker.

Regards,

Anthony Liguori

>
> Tested using -cpu host.
>
> Signed-off-by: Michael S. Tsirkin<mst@redhat.com>
> ---
>   arch/x86/kvm/cpuid.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 7d00d2d..bda4877 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -397,7 +397,7 @@ static int do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>   	case KVM_CPUID_SIGNATURE: {
>   		char signature[12] = "KVMKVMKVM\0\0";
>   		u32 *sigptr = (u32 *)signature;
> -		entry->eax = 0;
> +		entry->eax = KVM_CPUID_FEATURES;
>   		entry->ebx = sigptr[0];
>   		entry->ecx = sigptr[1];
>   		entry->edx = sigptr[2];

Re: [PATCH] kvm: fix cpuid eax

[H. Peter Anvin]

On 04/30/2012 01:37 PM, Anthony Liguori wrote:
> On 04/30/2012 09:39 AM, Michael S. Tsirkin wrote:
>> cpuid eax should return the max leaf so that
>> guests can find out the valid range.
>> This matches Xen et al.
> 
> What KVM does here predates Xen and Hyper-V.
> 
> This is an ABI breaker.
> 

What Intel and AMD have done here predates KVM...

What exactly relies on a zero here?

	-hpa

Re: [PATCH] kvm: fix cpuid eax

[Michael S. Tsirkin]

On Mon, Apr 30, 2012 at 03:37:14PM -0500, Anthony Liguori wrote:
> On 04/30/2012 09:39 AM, Michael S. Tsirkin wrote:
> >cpuid eax should return the max leaf so that
> >guests can find out the valid range.
> >This matches Xen et al.
> 
> What KVM does here predates Xen and Hyper-V.
> 
> This is an ABI breaker.

It is? I tested an old guest and qemu and both work fine.
What breaks?

> Regards,
> 
> Anthony Liguori
> 
> >
> >Tested using -cpu host.
> >
> >Signed-off-by: Michael S. Tsirkin<mst@redhat.com>
> >---
> >  arch/x86/kvm/cpuid.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> >diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> >index 7d00d2d..bda4877 100644
> >--- a/arch/x86/kvm/cpuid.c
> >+++ b/arch/x86/kvm/cpuid.c
> >@@ -397,7 +397,7 @@ static int do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
> >  	case KVM_CPUID_SIGNATURE: {
> >  		char signature[12] = "KVMKVMKVM\0\0";
> >  		u32 *sigptr = (u32 *)signature;
> >-		entry->eax = 0;
> >+		entry->eax = KVM_CPUID_FEATURES;
> >  		entry->ebx = sigptr[0];
> >  		entry->ecx = sigptr[1];
> >  		entry->edx = sigptr[2];

Re: [PATCH] kvm: fix cpuid eax

[Gleb Natapov]

On Mon, Apr 30, 2012 at 05:39:03PM +0300, Michael S. Tsirkin wrote:
> cpuid eax should return the max leaf so that
> guests can find out the valid range.
> This matches Xen et al.
> 
> Tested using -cpu host.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Gleb Natapov <gleb@redhat.com>

> ---
>  arch/x86/kvm/cpuid.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 7d00d2d..bda4877 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -397,7 +397,7 @@ static int do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>  	case KVM_CPUID_SIGNATURE: {
>  		char signature[12] = "KVMKVMKVM\0\0";
>  		u32 *sigptr = (u32 *)signature;
> -		entry->eax = 0;
> +		entry->eax = KVM_CPUID_FEATURES;
>  		entry->ebx = sigptr[0];
>  		entry->ecx = sigptr[1];
>  		entry->edx = sigptr[2];
> -- 
> MST
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

--
			Gleb.

Re: [PATCH] kvm: fix cpuid eax

[Avi Kivity]

On 04/30/2012 11:37 PM, Anthony Liguori wrote:
> On 04/30/2012 09:39 AM, Michael S. Tsirkin wrote:
>> cpuid eax should return the max leaf so that
>> guests can find out the valid range.
>> This matches Xen et al.
>
> What KVM does here predates Xen and Hyper-V.
>
> This is an ABI breaker.

First, I don't think we ever documented eax, so nothing can rely on it.
Second, this isn't guest visible, it's just part of
KVM_GET_SUPPORTED_CPUID (used by qemu's -cpu host, but doesn't affect
qemu stable cpus).

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] kvm: fix cpuid eax

[Avi Kivity]

On 04/30/2012 05:39 PM, Michael S. Tsirkin wrote:
> cpuid eax should return the max leaf so that
> guests can find out the valid range.
> This matches Xen et al.
>
> Tested using -cpu host.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  arch/x86/kvm/cpuid.c |    2 +-

Documentation/virtual/kvm/cpuid.c |  ++++

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] kvm: fix cpuid eax

[Avi Kivity]

On 05/01/2012 03:53 PM, Avi Kivity wrote:
> On 04/30/2012 11:37 PM, Anthony Liguori wrote:
> > On 04/30/2012 09:39 AM, Michael S. Tsirkin wrote:
> >> cpuid eax should return the max leaf so that
> >> guests can find out the valid range.
> >> This matches Xen et al.
> >
> > What KVM does here predates Xen and Hyper-V.
> >
> > This is an ABI breaker.
>
> First, I don't think we ever documented eax, so nothing can rely on it.
>

Actually, we did document it.

However, I think it's safe to change.  The documentation in this case
should say "reserved, returns zero in this implementation", rather than 0.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH] kvm: fix cpuid eax

[Ingo Molnar]

* Avi Kivity <avi@redhat.com> wrote:

> On 05/01/2012 03:53 PM, Avi Kivity wrote:
> > On 04/30/2012 11:37 PM, Anthony Liguori wrote:
> > > On 04/30/2012 09:39 AM, Michael S. Tsirkin wrote:
> > >> cpuid eax should return the max leaf so that
> > >> guests can find out the valid range.
> > >> This matches Xen et al.
> > >
> > > What KVM does here predates Xen and Hyper-V.
> > >
> > > This is an ABI breaker.
> >
> > First, I don't think we ever documented eax, so nothing can rely on it.
> >
> 
> Actually, we did document it.
> 
> However, I think it's safe to change.  The documentation in 
> this case should say "reserved, returns zero in this 
> implementation", rather than 0.

Just a side note: whether and how it got documented is 
immaterial, what matters is whether any application binary that 
is already out there breaks. (The 'B' in ABI.)

Whether it was fully documented or it was entirly undocumented, 
the code obfuscated, encrypted and all this information got 
locked away in a safe at a military base protected by nuclear 
missiles is immaterial: if someone accidentally stumbled upon it 
and an app learned to rely on the ABI detail then tough luck.

Fortunately, this does not appear to have happened, so as a 
result it's *not* an Application Binary Interface (it's only a 
Binary Interface) and can thus be changed for the better.

Thanks,

	Ingo