Dan Kenigsberg wrote:
> On Mon, Jan 14, 2008 at 02:49:31PM +0100, Alexander Graf wrote:
>   
>> Hi,
>>
>> Currently CPUID function 4 is broken. This function's values rely on the
>> value of ECX.
>> To solve the issue cleanly, there is already a new API for cpuid
>> settings, which is not used yet.
>> Using the current interface, the function 4 can be easily passed
>> through, by giving multiple function 4 outputs and increasing the
>> index-identifier on the fly. This does not break compatibility.
>>
>> This fix is really important for Mac OS X, as it requires cache
>> information. Please also see my previous patches for Mac OS X (or rather
>> core duo target) compatibility.
>>
>> Regards,
>>
>> Alex
>>     
>
>   
>> diff --git a/kernel/x86.c b/kernel/x86.c
>> index b55c177..73312e9 100644
>> --- a/kernel/x86.c
>> +++ b/kernel/x86.c
>> @@ -783,7 +783,7 @@ static int kvm_vcpu_ioctl_set_cpuid(struct kvm_vcpu 
>> *vcpu,
>>                                  struct kvm_cpuid *cpuid,
>>                                  struct kvm_cpuid_entry __user *entries)
>>  {
>> -    int r, i;
>> +    int r, i, n = 0;
>>      struct kvm_cpuid_entry *cpuid_entries;
>>  
>>      r = -E2BIG;
>> @@ -803,8 +803,17 @@ static int kvm_vcpu_ioctl_set_cpuid(struct kvm_vcpu 
>> *vcpu,
>>              vcpu->arch.cpuid_entries[i].ebx = cpuid_entries[i].ebx;
>>              vcpu->arch.cpuid_entries[i].ecx = cpuid_entries[i].ecx;
>>              vcpu->arch.cpuid_entries[i].edx = cpuid_entries[i].edx;
>> -            vcpu->arch.cpuid_entries[i].index = 0;
>> -            vcpu->arch.cpuid_entries[i].flags = 0;
>> +                switch(vcpu->arch.cpuid_entries[i].function) {
>> +                    case 4:
>> +                        vcpu->arch.cpuid_entries[i].index = n;
>> +                        vcpu->arch.cpuid_entries[i].flags = 
>> KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
>> +                        n++;
>> +                        break;
>> +                    default:
>> +                        vcpu->arch.cpuid_entries[i].index = 0;
>> +                        vcpu->arch.cpuid_entries[i].flags = 0;
>> +                        break;
>> +                }
>>     
>
> I will not mention the whitespace damage here :-). Instead, I'd ask you
>   
Oh well, after having been into qemu source, I just got used to use
spaces instead of tabs ;-).

> to review, comment, and even try, the patch that I posted here not long
> ago, exposing all safe host cpuid functions to guests.
>   
Sure.
Basically your patch targets at a completely different use case than
mine though. You want to expose the host features on the virtual CPU,
whereas my goal is to have a virtual Core Duo/Solo CPU, even if your
host CPU is actually an SVM capable one.

So my CoreDuo CPU definition still fails to populate a proper CPUID
function 4. With the -cpu host option, Linux works (as it's bright
enough to know that some values are just plain wrong), but Darwin
crashes. I am not exactly sure why it is, but I guess it's due to the
function 4 values exposing a 2-core CPU, which kvm simply doesn't emulate.

So far I still like the approach of the cpuid2 API, so it would be great
if you could take the patch one step further and have qemu defined CPUs
work properly as well.

I do not know too much about the kvm code style, so I can't really say
anything regarding that.

Regards,

Alex

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
kvm-devel mailing list
kvm-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-devel

Reply via email to