Re: [Qemu-devel] [PATCH 2/2] Expose tsc deadline timer cpuid to guest

[Liu, Jinsong]

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.
> 
> Andreas, do you expect CPU devices to be ready for qemu 1.1? We would
> need them to pass a feature exclusion mask from machine.compat_props
> to 
> the (x86) CPU init code.

cpu devices is just another format of current cpu_model. It helps nothing to 
our problem.
Again, the point is, by what method the feature exclusion mask can be 
generated, if user not give hint manually?

Thanks,
Jinsong

> 
> Well, given that introducing some intermediate solution for this would
> be complex and hacky and that there is a way to configure tsc_deadline
> for old machines away, though only an explicit one, I could live with
> postponing the feature mask after the CPU device conversion. But the
> last word will have the maintainers.
> 
> Jan