Eduardo Habkost wrote:
> On Fri, Mar 09, 2012 at 09:52:29PM +0100, Jan Kiszka wrote:
>> On 2012-03-09 20:09, Liu, Jinsong wrote:
>>> Jan Kiszka wrote:
>>>> On 2012-03-09 19:27, Liu, Jinsong wrote:
>>>>> Jan Kiszka wrote:
>>>>>> On 2012-03-06 08:49, Liu, Jinsong wrote:
>>>>>>> Jan,
>>>>>>>
>>>>>>> Any comments? I feel some confused about your point 'disable
>>>>>>> cpuid feature for older machine types by default': are you
>>>>>>> planning a common approach for this common issue, or, you just
>>>>>>> ask me a specific solution for the tsc deadline timer case?
>>>>>>
>>>>>> I think a generic solution for this can be as simple as passing a
>>>>>> feature exclusion mask to cpu_init. You could simple append a
>>>>>> string of "-feature1,-feature2" to the cpu model that is
>>>>>> specified on creation. And that string could be defined in the
>>>>>> compat machine descriptions. Does this make sense?
>>>>>>
>>>>>
>>>>> Jan, to prevent misunderstanding, I elaborate my understanding of
>>>>> your points below (if any misunderstanding please point out to
>>>>> me): ===================== Your target is, to migrate from A(old
>>>>> qemu) to B(new qemu) by
>>>>> 1. at A: qemu-version-A [-cpu whatever] // currently the
>>>>> default machine type is pc-A
>>>>> 2. at B: qemu-version-B -machine pc-A [-cpu whatever] -feature1
>>>>> -feature2
>>>>>
>>>>> B run new qemu-version-B (w/ new features 'feature1' and
>>>>> 'feature2'), but when B runs w/ compat '-machine pc-A', vm should
>>>>> not see 'feature1' and 'feature2', so commandline append string to
>>>>> cpu model '-cpu whatever -feature1 -feature2' to hidden new
>>>>> feature1 and feature2 to vm, hence vm can see same cpuid features
>>>>> (at B) as those at A (which means, no feature1, no feature2)
>>>>> =====================
>>>>>
>>>>> If my understanding of your thoughts is right, I think currently
>>>>> qemu has satisfied your target, code refer to
>>>>> pc_cpus_init(cpu_model) ...... cpu_init(cpu_model)
>>>>> --> cpu_x86_register(*env, cpu_model)
>>>>> --> cpu_x86_find_by_name(*def, cpu_model) // parse '+/-
>>>>>
>>>>> features', generate feature masks plus_features... // and
>>>>> minus_features...(this is feature exclusion masks you want)
>>>>> I think your point 'define in the compat machine description' is
>>>>> unnecessary.
>>>>
>>>> The user would have to specify the new feature as exclusions
>>>> *manually* on the command line if -machine pc-A doesn't inject them
>>>> *automatically*. So it is necessary to enhance qemu in this regard.
>>>>
>>>
>>> ... You suggest 'append a string of "-feature1,-feature2" to the
>>> cpu model that is specified on creation' at your last email. Could
>>> you tell me other way user exclude features? I only know qemu
>>> command line :-(
>>
>> I was thinking of something like
>>
>> diff --git a/hw/boards.h b/hw/boards.h
>> index 667177d..2bae071 100644
>> --- a/hw/boards.h
>> +++ b/hw/boards.h
>> @@ -28,6 +28,7 @@ typedef struct QEMUMachine {
>> int is_default;
>> const char *default_machine_opts;
>> GlobalProperty *compat_props;
>> + const char *compat_cpu_features;
>> struct QEMUMachine *next;
>> } QEMUMachine;
>>
>> diff --git a/hw/pc.c b/hw/pc.c
>> index bb9867b..4d11559 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -949,8 +949,9 @@ static CPUState *pc_new_cpu(const char
>> *cpu_model) return env; }
>>
>> -void pc_cpus_init(const char *cpu_model)
>> +void pc_cpus_init(const char *cpu_model, const char
>> *append_features) { + char *model_and_features;
>> int i;
>>
>> /* init CPUs */
>> @@ -961,10 +962,13 @@ void pc_cpus_init(const char *cpu_model)
>> cpu_model = "qemu32";
>> #endif
>> }
>> + model_and_features = g_strconcat(cpu_model, ",",
>> append_features, NULL);
>>
>> for(i = 0; i < smp_cpus; i++) {
>> - pc_new_cpu(cpu_model);
>> + pc_new_cpu(model_and_features);
>> }
>> +
>> + g_free(model_and_features);
>> }
>>
>> void pc_memory_init(MemoryRegion *system_memory,
>>
>>
>> However, getting machine.compat_cpu_features to pc_cpus_init is
>> rather
>> ugly. And we will have CPU devices with real properties soon. Then
>> the compat feature string could be passed that way, without changing
>> any
>> machine init function.
>
> What if one cpudef had the wrong flags set but another cpudef was
> correct, and we had to fix it on Qemu 1.1 for only one model? What if
> the user _really_ wanted to edit the config file to add or remove a
> given flag?
>
> I think the best approach would be:
> - Having versioned CPU model names;
> - Specifying per-machine-type aliases.
>
> See also the "[libvirt] Modern CPU models cannot be used with libvirt"
> for related discussion.
>
> The config file would look like:
>
> [cpudef]
> name = "Westmere-1.0"
> features = "[...]" # no tsc-deadline
> [...]
>
> [cpudef]
> name = "Westmere-1.1"
> # so we don't have to copy everything from Westmere-1.0 manually:
> base_cpudef = "Westemere-1.0"
> # we could simply copy & extend:
> features = "[...] tsc-deadline"
> # or, even better, if we had a "append" mechanism. e.g.:
> #features_append = "tsc-deadline"
>
>
> Then, on the machine-type table:
>
> - Machine-types "pc-1.0" and older would have:
> .cpudef_aliases = {
> {"Westmere", "Westmere-1.0"},
> [...]
> }
>
> - Machine-type "pc-1.1" would have:
> .cpudef_aliases = {
> {"Westmere", "Westmere-1.1"},
> [...]
> }
Yes, it's reasonable in this way (or something like this). Considering currently qemu1.1 is changing cpu_device/per-machine-type, we'd better wait until it done and then update tsc deadline timer accordingly.
Thanks,
Jinsong--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
