[Qemu-devel] [PATCH] Qemu: Fix eax for cpuid leaf 0x40000000

[Qemu-devel] [PATCH] Qemu: Fix eax for cpuid leaf 0x40000000

[Jidong Xiao]

Hi,

Since Linux kernel 3.5, KVM has set eax to KVM_CPUID_FEATURES, for
leaf 0x40000000, see this:

https://github.com/torvalds/linux/commit/57c22e5f35aa4b9b2fe11f73f3e62bbf9ef36190

But qemu still tries to set it to 0. It would be better to make qemu
and kvm consistent. This patch just fixes this issue.

Signed-off-by: Jidong Xiao <jidong.xiao@gmail.com>
---
diff --git a/qemu-2.0.0/target-i386/kvm.c.orig b/qemu-2.0.0/target-i386/kvm.c
index 4389959..b8b282d 100644
--- a/qemu-2.0.0/target-i386/kvm.c.orig
+++ b/qemu-2.0.0/target-i386/kvm.c
@@ -530,7 +530,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
     memcpy(signature, "KVMKVMKVM\0\0\0", 12);
     c = &cpuid_data.entries[cpuid_i++];
     c->function = KVM_CPUID_SIGNATURE | kvm_base;
-    c->eax = 0;
+   c->eax = KVM_CPUID_FEATURES;
     c->ebx = signature[0];
     c->ecx = signature[1];
     c->edx = signature[2];

Re: [Qemu-devel] [PATCH] Qemu: Fix eax for cpuid leaf 0x40000000

[Paolo Bonzini]

Il 04/06/2014 03:10, Jidong Xiao ha scritto:
> diff --git a/qemu-2.0.0/target-i386/kvm.c.orig b/qemu-2.0.0/target-i386/kvm.c
> index 4389959..b8b282d 100644
> --- a/qemu-2.0.0/target-i386/kvm.c.orig
> +++ b/qemu-2.0.0/target-i386/kvm.c
> @@ -530,7 +530,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
>      memcpy(signature, "KVMKVMKVM\0\0\0", 12);
>      c = &cpuid_data.entries[cpuid_i++];
>      c->function = KVM_CPUID_SIGNATURE | kvm_base;
> -    c->eax = 0;
> +   c->eax = KVM_CPUID_FEATURES;
>      c->ebx = signature[0];
>      c->ecx = signature[1];
>      c->edx = signature[2];

This should actually be "KVM_CPUID_FEATURES | kvm_base", in case Hyper-V 
leaves are available too.  But it is a good catch!

Paolo

Re: [Qemu-devel] [PATCH] Qemu: Fix eax for cpuid leaf 0x40000000

[Jidong Xiao]

On Wed, Jun 4, 2014 at 3:09 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 04/06/2014 03:10, Jidong Xiao ha scritto:
>
>> diff --git a/qemu-2.0.0/target-i386/kvm.c.orig
>> b/qemu-2.0.0/target-i386/kvm.c
>> index 4389959..b8b282d 100644
>> --- a/qemu-2.0.0/target-i386/kvm.c.orig
>> +++ b/qemu-2.0.0/target-i386/kvm.c
>> @@ -530,7 +530,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
>>      memcpy(signature, "KVMKVMKVM\0\0\0", 12);
>>      c = &cpuid_data.entries[cpuid_i++];
>>      c->function = KVM_CPUID_SIGNATURE | kvm_base;
>> -    c->eax = 0;
>> +   c->eax = KVM_CPUID_FEATURES;
>>      c->ebx = signature[0];
>>      c->ecx = signature[1];
>>      c->edx = signature[2];
>
>
> This should actually be "KVM_CPUID_FEATURES | kvm_base", in case Hyper-V
> leaves are available too.  But it is a good catch!
>
> Paolo
>
Thanks Paolo. I have just added that and resend the patch as following:

-Jidong

===
Signed-off-by: Jidong Xiao <jidong.xiao@gmail.com>

---
diff --git a/qemu-2.0.0/target-i386/kvm.c.orig b/qemu-2.0.0/target-i386/kvm.c
index 4389959..fe49a75 100644
--- a/qemu-2.0.0/target-i386/kvm.c.orig
+++ b/qemu-2.0.0/target-i386/kvm.c
@@ -530,7 +530,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
     memcpy(signature, "KVMKVMKVM\0\0\0", 12);
     c = &cpuid_data.entries[cpuid_i++];
     c->function = KVM_CPUID_SIGNATURE | kvm_base;
-    c->eax = 0;
+   c->eax = KVM_CPUID_FEATURES | kvm_base;
     c->ebx = signature[0];
     c->ecx = signature[1];
     c->edx = signature[2];

Re: [Qemu-devel] [PATCH] Qemu: Fix eax for cpuid leaf 0x40000000

[Bandan Das]

Jidong Xiao <jidong.xiao@gmail.com> writes:

> On Wed, Jun 4, 2014 at 3:09 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Il 04/06/2014 03:10, Jidong Xiao ha scritto:
>>
>>> diff --git a/qemu-2.0.0/target-i386/kvm.c.orig
>>> b/qemu-2.0.0/target-i386/kvm.c
>>> index 4389959..b8b282d 100644
>>> --- a/qemu-2.0.0/target-i386/kvm.c.orig
>>> +++ b/qemu-2.0.0/target-i386/kvm.c
>>> @@ -530,7 +530,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
>>>      memcpy(signature, "KVMKVMKVM\0\0\0", 12);
>>>      c = &cpuid_data.entries[cpuid_i++];
>>>      c->function = KVM_CPUID_SIGNATURE | kvm_base;
>>> -    c->eax = 0;
>>> +   c->eax = KVM_CPUID_FEATURES;
>>>      c->ebx = signature[0];
>>>      c->ecx = signature[1];
>>>      c->edx = signature[2];
>>
>>
>> This should actually be "KVM_CPUID_FEATURES | kvm_base", in case Hyper-V
>> leaves are available too.  But it is a good catch!
>>
>> Paolo
>>
> Thanks Paolo. I have just added that and resend the patch as following:
>
> -Jidong
>
> ===
> Signed-off-by: Jidong Xiao <jidong.xiao@gmail.com>

Did you mean to send this as the new version of your patch ?
Please send the revised version in a separate email and add 
"[PATCH v2]" to the subject.

> ---
> diff --git a/qemu-2.0.0/target-i386/kvm.c.orig b/qemu-2.0.0/target-i386/kvm.c
> index 4389959..fe49a75 100644
> --- a/qemu-2.0.0/target-i386/kvm.c.orig
> +++ b/qemu-2.0.0/target-i386/kvm.c
> @@ -530,7 +530,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
>      memcpy(signature, "KVMKVMKVM\0\0\0", 12);
>      c = &cpuid_data.entries[cpuid_i++];
>      c->function = KVM_CPUID_SIGNATURE | kvm_base;
> -    c->eax = 0;
> +   c->eax = KVM_CPUID_FEATURES | kvm_base;

Looks like this is not four spaces

>      c->ebx = signature[0];
>      c->ecx = signature[1];
>      c->edx = signature[2];

Re: [Qemu-devel] [PATCH] Qemu: Fix eax for cpuid leaf 0x40000000

[Jidong Xiao]

On Wed, Jun 4, 2014 at 3:08 PM, Bandan Das <bsd@redhat.com> wrote:
> Jidong Xiao <jidong.xiao@gmail.com> writes:
>
>> On Wed, Jun 4, 2014 at 3:09 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> Il 04/06/2014 03:10, Jidong Xiao ha scritto:
>>>
>>>> diff --git a/qemu-2.0.0/target-i386/kvm.c.orig
>>>> b/qemu-2.0.0/target-i386/kvm.c
>>>> index 4389959..b8b282d 100644
>>>> --- a/qemu-2.0.0/target-i386/kvm.c.orig
>>>> +++ b/qemu-2.0.0/target-i386/kvm.c
>>>> @@ -530,7 +530,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
>>>>      memcpy(signature, "KVMKVMKVM\0\0\0", 12);
>>>>      c = &cpuid_data.entries[cpuid_i++];
>>>>      c->function = KVM_CPUID_SIGNATURE | kvm_base;
>>>> -    c->eax = 0;
>>>> +   c->eax = KVM_CPUID_FEATURES;
>>>>      c->ebx = signature[0];
>>>>      c->ecx = signature[1];
>>>>      c->edx = signature[2];
>>>
>>>
>>> This should actually be "KVM_CPUID_FEATURES | kvm_base", in case Hyper-V
>>> leaves are available too.  But it is a good catch!
>>>
>>> Paolo
>>>
>> Thanks Paolo. I have just added that and resend the patch as following:
>>
>> -Jidong
>>
>> ===
>> Signed-off-by: Jidong Xiao <jidong.xiao@gmail.com>
>
> Did you mean to send this as the new version of your patch ?
> Please send the revised version in a separate email and add
> "[PATCH v2]" to the subject.
>
Hi, Bandan,

Thanks for the advice. I think Paolo has incorporated this patch into
his patch sets, and he sent the revised version in a separate email
earlier today with the subject "[PULL 11/11] kvm: Fix eax for cpuid
leaf 0x40000000".

-Jidong

>> ---
>> diff --git a/qemu-2.0.0/target-i386/kvm.c.orig b/qemu-2.0.0/target-i386/kvm.c
>> index 4389959..fe49a75 100644
>> --- a/qemu-2.0.0/target-i386/kvm.c.orig
>> +++ b/qemu-2.0.0/target-i386/kvm.c
>> @@ -530,7 +530,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
>>      memcpy(signature, "KVMKVMKVM\0\0\0", 12);
>>      c = &cpuid_data.entries[cpuid_i++];
>>      c->function = KVM_CPUID_SIGNATURE | kvm_base;
>> -    c->eax = 0;
>> +   c->eax = KVM_CPUID_FEATURES | kvm_base;
>
> Looks like this is not four spaces
>
>>      c->ebx = signature[0];
>>      c->ecx = signature[1];
>>      c->edx = signature[2];

Re: [Qemu-devel] [PATCH] Qemu: Fix eax for cpuid leaf 0x40000000

[Bandan Das]

Jidong Xiao <jidong.xiao@gmail.com> writes:
..
>>
> Hi, Bandan,
>
> Thanks for the advice. I think Paolo has incorporated this patch into
> his patch sets, and he sent the revised version in a separate email
> earlier today with the subject "[PULL 11/11] kvm: Fix eax for cpuid
> leaf 0x40000000".

Oops! Sorry, must have missed it. Thanks for the heads-up.

> -Jidong
>
>>> ---
>>> diff --git a/qemu-2.0.0/target-i386/kvm.c.orig b/qemu-2.0.0/target-i386/kvm.c
>>> index 4389959..fe49a75 100644
>>> --- a/qemu-2.0.0/target-i386/kvm.c.orig
>>> +++ b/qemu-2.0.0/target-i386/kvm.c
>>> @@ -530,7 +530,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
>>>      memcpy(signature, "KVMKVMKVM\0\0\0", 12);
>>>      c = &cpuid_data.entries[cpuid_i++];
>>>      c->function = KVM_CPUID_SIGNATURE | kvm_base;
>>> -    c->eax = 0;
>>> +   c->eax = KVM_CPUID_FEATURES | kvm_base;
>>
>> Looks like this is not four spaces
>>
>>>      c->ebx = signature[0];
>>>      c->ecx = signature[1];
>>>      c->edx = signature[2];

Re: [Qemu-devel] [PATCH] Qemu: Fix eax for cpuid leaf 0x40000000

[Eduardo Habkost]

On Wed, Jun 04, 2014 at 03:17:56AM -0400, Jidong Xiao wrote:
> On Wed, Jun 4, 2014 at 3:09 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > Il 04/06/2014 03:10, Jidong Xiao ha scritto:
> >
> >> diff --git a/qemu-2.0.0/target-i386/kvm.c.orig
> >> b/qemu-2.0.0/target-i386/kvm.c
> >> index 4389959..b8b282d 100644
> >> --- a/qemu-2.0.0/target-i386/kvm.c.orig
> >> +++ b/qemu-2.0.0/target-i386/kvm.c
> >> @@ -530,7 +530,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
> >>      memcpy(signature, "KVMKVMKVM\0\0\0", 12);
> >>      c = &cpuid_data.entries[cpuid_i++];
> >>      c->function = KVM_CPUID_SIGNATURE | kvm_base;
> >> -    c->eax = 0;
> >> +   c->eax = KVM_CPUID_FEATURES;
> >>      c->ebx = signature[0];
> >>      c->ecx = signature[1];
> >>      c->edx = signature[2];
> >
> >
> > This should actually be "KVM_CPUID_FEATURES | kvm_base", in case Hyper-V
> > leaves are available too.  But it is a good catch!
> >
> > Paolo
> >
> Thanks Paolo. I have just added that and resend the patch as following:
> 
> -Jidong
> 
> ===
> Signed-off-by: Jidong Xiao <jidong.xiao@gmail.com>
> 
> ---
> diff --git a/qemu-2.0.0/target-i386/kvm.c.orig b/qemu-2.0.0/target-i386/kvm.c
> index 4389959..fe49a75 100644
> --- a/qemu-2.0.0/target-i386/kvm.c.orig
> +++ b/qemu-2.0.0/target-i386/kvm.c
> @@ -530,7 +530,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
>      memcpy(signature, "KVMKVMKVM\0\0\0", 12);
>      c = &cpuid_data.entries[cpuid_i++];
>      c->function = KVM_CPUID_SIGNATURE | kvm_base;
> -    c->eax = 0;
> +   c->eax = KVM_CPUID_FEATURES | kvm_base;

This makes the CPUID data change under the guest's feet during
live-migration.

Adding compat code to ensure older machine-types keep the old behavior
is necessary, but in this specific case it is mostly harmless because
0x0 is documented as being equivalent to 0x40000001.

(But I don't know how guests are supposed to behave when they see
CPUID[KVM_CPUID_SIGNATURE_NEXT].EAX==0.)

-- 
Eduardo

Re: [Qemu-devel] [PATCH] Qemu: Fix eax for cpuid leaf 0x40000000

[Paolo Bonzini]

Il 12/08/2014 20:55, Eduardo Habkost ha scritto:
> This makes the CPUID data change under the guest's feet during
> live-migration.
> 
> Adding compat code to ensure older machine-types keep the old behavior
> is necessary, but in this specific case it is mostly harmless because
> 0x0 is documented as being equivalent to 0x40000001.
> 
> (But I don't know how guests are supposed to behave when they see
> CPUID[KVM_CPUID_SIGNATURE_NEXT].EAX==0.)

The only obvious thing to do would be to treat it as 0x40000101.

Paolo

Re: [Qemu-devel] [PATCH] Qemu: Fix eax for cpuid leaf 0x40000000

[Eduardo Habkost]

On Tue, Aug 12, 2014 at 09:12:00PM +0200, Paolo Bonzini wrote:
> Il 12/08/2014 20:55, Eduardo Habkost ha scritto:
> > This makes the CPUID data change under the guest's feet during
> > live-migration.
> > 
> > Adding compat code to ensure older machine-types keep the old behavior
> > is necessary, but in this specific case it is mostly harmless because
> > 0x0 is documented as being equivalent to 0x40000001.
> > 
> > (But I don't know how guests are supposed to behave when they see
> > CPUID[KVM_CPUID_SIGNATURE_NEXT].EAX==0.)
> 
> The only obvious thing to do would be to treat it as 0x40000101.

I just want to be sure the guests really do that. If we know guests
won't do anything different with the CPUID change, I won't mind having
no compat code for this.

-- 
Eduardo

Re: [Qemu-devel] [PATCH] Qemu: Fix eax for cpuid leaf 0x40000000

[Paolo Bonzini]

Il 12/08/2014 21:29, Eduardo Habkost ha scritto:
> On Tue, Aug 12, 2014 at 09:12:00PM +0200, Paolo Bonzini wrote:
>> Il 12/08/2014 20:55, Eduardo Habkost ha scritto:
>>> This makes the CPUID data change under the guest's feet during
>>> live-migration.
>>>
>>> Adding compat code to ensure older machine-types keep the old behavior
>>> is necessary, but in this specific case it is mostly harmless because
>>> 0x0 is documented as being equivalent to 0x40000001.
>>>
>>> (But I don't know how guests are supposed to behave when they see
>>> CPUID[KVM_CPUID_SIGNATURE_NEXT].EAX==0.)
>>
>> The only obvious thing to do would be to treat it as 0x40000101.
> 
> I just want to be sure the guests really do that. If we know guests
> won't do anything different with the CPUID change, I won't mind having
> no compat code for this.
> 

Considering that only two leaves are defined for KVM, and both are
mandatory I don't think current guests have any reason to look at
CPUID[KVM_CPUID_SIGNATURE | kvm_base].EAX at all.

Paolo