Re: [Qemu-devel] [PATCH 03/17] target-i386: move kvm_check_features_against_host() check to realize time

[Eduardo Habkost]

On Fri, Jan 11, 2013 at 03:10:17AM +0100, Igor Mammedov wrote:
> kvm_check_features_against_host() should be called when features can't be
> changed and when features are convereted to properties it would be possible to
> change them until realize time, so correct way is to call
> kvm_check_features_against_host() in x86_cpu_realize()
> 
> Signed-off-by: Igor Mammedov <address@hidden>

Reviewed-by: Eduardo Habkost <address@hidden>

> ---
>  v3:
>    - call kvm_check_features_against_host() outside of #ifdef CONFIG_KVM
>      to avoid adding ifdefs in it and around unavailable_host_feature()
>    - rebased on top of "disable kvm_mmu + -cpu "enforce" fixes (v3)"
>  v2:
>    - squash in ifdef-ing kvm specific functions into this patch
>  v3:
>    - fit commit message into 80 column limit, no change to content
> ---
>  target-i386/cpu.c |   28 +++++++++++++++-------------
>  1 files changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index ea68bc0..c4ff761 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1022,27 +1022,28 @@ static int unavailable_host_feature(FeatureWordInfo 
> *f, uint32_t mask)
>   *
>   * This function may be called only if KVM is enabled.
>   */
> -static int kvm_check_features_against_host(x86_def_t *guest_def)
> +static int kvm_check_features_against_host(X86CPU *cpu)
>  {
> +    CPUX86State *env = &cpu->env;
>      x86_def_t host_def;
>      uint32_t mask;
>      int rv, i;
>      struct model_features_t ft[] = {
> -        {&guest_def->features, &host_def.features,
> +        {&env->cpuid_features, &host_def.features,
>              FEAT_1_EDX },
> -        {&guest_def->ext_features, &host_def.ext_features,
> +        {&env->cpuid_ext_features, &host_def.ext_features,
>              FEAT_1_ECX },
> -        {&guest_def->ext2_features, &host_def.ext2_features,
> +        {&env->cpuid_ext2_features, &host_def.ext2_features,
>              FEAT_8000_0001_EDX },
> -        {&guest_def->ext3_features, &host_def.ext3_features,
> +        {&env->cpuid_ext3_features, &host_def.ext3_features,
>              FEAT_8000_0001_ECX },
> -        {&guest_def->ext4_features, &host_def.ext4_features,
> +        {&env->cpuid_ext4_features, &host_def.ext4_features,
>              FEAT_C000_0001_EDX },
> -        {&guest_def->cpuid_7_0_ebx_features, 
> &host_def.cpuid_7_0_ebx_features,
> +        {&env->cpuid_7_0_ebx_features, &host_def.cpuid_7_0_ebx_features,
>              FEAT_7_0_EBX },
> -        {&guest_def->svm_features, &host_def.svm_features,
> +        {&env->cpuid_svm_features, &host_def.svm_features,
>              FEAT_SVM },
> -        {&guest_def->kvm_features, &host_def.kvm_features,
> +        {&env->cpuid_kvm_features, &host_def.kvm_features,
>              FEAT_KVM },
>      };
>  
> @@ -1471,10 +1472,6 @@ static int cpu_x86_parse_featurestr(x86_def_t 
> *x86_cpu_def, char *features)
>      x86_cpu_def->kvm_features &= ~minus_features[FEAT_KVM];
>      x86_cpu_def->svm_features &= ~minus_features[FEAT_SVM];
>      x86_cpu_def->cpuid_7_0_ebx_features &= ~minus_features[FEAT_7_0_EBX];
> -    if (check_cpuid && kvm_enabled()) {
> -        if (kvm_check_features_against_host(x86_cpu_def) && enforce_cpuid)
> -            goto error;
> -    }
>      return 0;
>  
>  error:
> @@ -2177,6 +2174,11 @@ void x86_cpu_realize(Object *obj, Error **errp)
>  #ifdef CONFIG_KVM
>          filter_features_for_kvm(cpu);
>  #endif
> +        if (check_cpuid && kvm_check_features_against_host(cpu)
> +            && enforce_cpuid) {
> +            error_setg(errp, "Host's CPU doesn't support requested 
> features");
> +            return;
> +        }
>      }
>  
>  #ifndef CONFIG_USER_ONLY
> -- 
> 1.7.1
> 

-- 
Eduardo