Re: [Qemu-devel] [PATCH 1/7] target-i386: Eliminate CONFIG_KVM #ifdefs

[Paolo Bonzini]

Il 27/11/2013 20:34, Eduardo Habkost ha scritto:
> The compiler is capable of eliminating the KVM-specific function calls
> as long as the calling function has an assert(kvm_enabled()) line, so we
> don't need to wrap all KVM-specific inside #ifdefs.
> 
> Signed-off-by: Eduardo Habkost <address@hidden>
> ---
>  target-i386/cpu.c | 13 ++-----------
>  1 file changed, 2 insertions(+), 11 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index d747e04..b525592 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -373,7 +373,6 @@ void disable_kvm_pv_eoi(void)
>  void host_cpuid(uint32_t function, uint32_t count,
>                  uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx)
>  {
> -#if defined(CONFIG_KVM)
>      uint32_t vec[4];
>  
>  #ifdef __x86_64__
> @@ -401,7 +400,6 @@ void host_cpuid(uint32_t function, uint32_t count,
>          *ecx = vec[2];
>      if (edx)
>          *edx = vec[3];
> -#endif
>  }

NACK for this one.  It includes an "asm", it is plausible that it fails
to parse before dead-code elimination.  Rather, change the #else to an
#elif, and add a new #else with an abort inside.

Otherwise looks good.

Paolo

>  #define iswhite(c) ((c) && ((c) <= ' ' || '~' < (c)))
> @@ -1118,7 +1116,6 @@ void x86_cpu_compat_set_features(const char *cpu_model, 
> FeatureWord w,
>      }
>  }
>  
> -#ifdef CONFIG_KVM
>  static int cpu_x86_fill_model_id(char *str)
>  {
>      uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
> @@ -1133,7 +1130,6 @@ static int cpu_x86_fill_model_id(char *str)
>      }
>      return 0;
>  }
> -#endif
>  
>  /* Fill a x86_def_t struct with information about the host CPU, and
>   * the CPU features supported by the host hardware + host kernel
> @@ -1142,7 +1138,6 @@ static int cpu_x86_fill_model_id(char *str)
>   */
>  static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
>  {
> -#ifdef CONFIG_KVM
>      KVMState *s = kvm_state;
>      uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0;
>  
> @@ -1172,8 +1167,6 @@ static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def)
>              kvm_arch_get_supported_cpuid(s, wi->cpuid_eax, wi->cpuid_ecx,
>                                           wi->cpuid_reg);
>      }
> -
> -#endif /* CONFIG_KVM */
>  }
>  
>  static int unavailable_host_feature(FeatureWordInfo *f, uint32_t mask)
> @@ -1798,13 +1791,14 @@ CpuDefinitionInfoList 
> *arch_query_cpu_definitions(Error **errp)
>      return cpu_list;
>  }
>  
> -#ifdef CONFIG_KVM
>  static void filter_features_for_kvm(X86CPU *cpu)
>  {
>      CPUX86State *env = &cpu->env;
>      KVMState *s = kvm_state;
>      FeatureWord w;
>  
> +    assert(kvm_enabled());
> +
>      for (w = 0; w < FEATURE_WORDS; w++) {
>          FeatureWordInfo *wi = &feature_word_info[w];
>          uint32_t host_feat = kvm_arch_get_supported_cpuid(s, wi->cpuid_eax,
> @@ -1815,7 +1809,6 @@ static void filter_features_for_kvm(X86CPU *cpu)
>          cpu->filtered_features[w] = requested_features & ~env->features[w];
>      }
>  }
> -#endif
>  
>  static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
>  {
> @@ -2525,9 +2518,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
> **errp)
>                         "Host's CPU doesn't support requested features");
>              goto out;
>          }
> -#ifdef CONFIG_KVM
>          filter_features_for_kvm(cpu);
> -#endif
>      }
>  
>  #ifndef CONFIG_USER_ONLY
>