Re: [Qemu-devel] [PATCH v2 1/3] KVM: Add new -cpu best

[Alexander Graf]

On 02.07.2012, at 16:25, Avi Kivity wrote:

> On 06/26/2012 07:39 PM, Alexander Graf wrote:
>> During discussions on whether to make -cpu host the default in SLE, I found
>> myself disagreeing to the thought, because it potentially opens a big can
>> of worms for potential bugs. But if I already am so opposed to it for SLE, 
>> how
>> can it possibly be reasonable to default to -cpu host in upstream QEMU? And
>> what would a sane default look like?
>> 
>> So I had this idea of looping through all available CPU definitions. We can
>> pretty well tell if our host is able to execute any of them by checking the
>> respective flags and seeing if our host has all features the CPU definition
>> requires. With that, we can create a -cpu type that would fall back to the
>> "best known CPU definition" that our host can fulfill. On my Phenom II
>> system for example, that would be -cpu phenom.
>> 
>> With this approach we can test and verify that CPU types actually work at
>> any random user setup, because we can always verify that all the -cpu types
>> we ship actually work. And we only default to some clever mechanism that
>> chooses from one of these.
>> 
>> 
>> +/* Are all guest feature bits present on the host? */
>> +static bool cpu_x86_feature_subset(uint32_t host, uint32_t guest)
>> +{
>> +    int i;
>> +
>> +    for (i = 0; i < 32; i++) {
>> +        uint32_t mask = 1 << i;
>> +        if ((guest & mask) && !(host & mask)) {
>> +            return false;
>> +        }
>> +    }
>> +
>> +    return true;
> 
>    return !(guest & ~host);

I guess it helps to think :).

> 
> 
>> +}
> 
> 
> 
>> +
>> +
>> +
>> +static void cpu_x86_fill_best(x86_def_t *x86_cpu_def)
>> +{
>> +    x86_def_t *def;
>> +
>> +    x86_cpu_def->family = 0;
>> +    x86_cpu_def->model = 0;
>> +    for (def = x86_defs; def; def = def->next) {
>> +        if (cpu_x86_fits_host(def) && cpu_x86_fits_higher(def, 
>> x86_cpu_def)) {
>> +            memcpy(x86_cpu_def, def, sizeof(*def));
>> +        }
>      *x86_cpu_def = *def;
>> +    }
>> +
>> +    if (!x86_cpu_def->family && !x86_cpu_def->model) {
>> +        fprintf(stderr, "No fitting CPU model found!\n");
>> +        exit(1);
>> +    }
>> +}
>> +
>> static int unavailable_host_feature(struct model_features_t *f, uint32_t 
>> mask)
>> {
>>     int i;
>> @@ -878,6 +957,8 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
>> const char *cpu_model)
>>             break;
>>     if (kvm_enabled() && name && strcmp(name, "host") == 0) {
>>         cpu_x86_fill_host(x86_cpu_def);
>> +    } else if (kvm_enabled() && name && strcmp(name, "best") == 0) {
>> +        cpu_x86_fill_best(x86_cpu_def);
>>     } else if (!def) {
>>         goto error;
>>     } else {
>> 
> 
> Should we copy the cache size etc. from the host?

I don't think so. We should rather make sure we always have cpu descriptions 
available close to what people out there actually use.


Alex