Re: [PATCH 1/3] use kernel-provided para_features instead of statically coming up with new capabilities

[Avi Kivity <avi@redhat.com>]

On 03/18/2011 12:42 AM, Glauber Costa wrote:
> According to Avi's comments over my last submission, I decided to take a
> different, and more correct direction - we hope.
>
> This patch is now using the features provided by KVM_GET_SUPPORTED_CPUID directly to
> mask out features from guest-visible cpuid.
>
> The old get_para_features() mechanism is kept for older kernels that do not implement it.
>
>
>
> +#ifdef CONFIG_KVM_PARA
> +struct kvm_para_features {
> +    int cap;
> +    int feature;
> +} para_features[] = {
> +    { KVM_CAP_CLOCKSOURCE, KVM_FEATURE_CLOCKSOURCE },
> +    { KVM_CAP_NOP_IO_DELAY, KVM_FEATURE_NOP_IO_DELAY },
> +    { KVM_CAP_PV_MMU, KVM_FEATURE_MMU_OP },
> +#ifdef KVM_CAP_ASYNC_PF
> +    { KVM_CAP_ASYNC_PF, KVM_FEATURE_ASYNC_PF },
> +#endif

Shouldn't the others get the same #ifdef treatment?

Yes, we depend on a kernels of a certain age, but let's not add more 
dependencies.

> +    { -1, -1 }
> +};

Since you use ARRAY_SIZE() later, don't need the guard here.

> +
> +static int get_para_features(CPUState *env)
> +{
> +    int i, features = 0;
> +
> +    for (i = 0; i<  ARRAY_SIZE(para_features) - 1; i++) {

So you can drop the - 1.

> +        if (kvm_check_extension(env->kvm_state, para_features[i].cap)) {
> +            features |= (1<<  para_features[i].feature);
> +        }
> +    }
> +
> +    return features;
> +}
> +#endif
> +
> +
>   uint32_t kvm_arch_get_supported_cpuid(CPUState *env, uint32_t function,
>                                         uint32_t index, int reg)
>   {
>
>
> -#ifdef CONFIG_KVM_PARA
> -struct kvm_para_features {
> -    int cap;
> -    int feature;
> -} para_features[] = {
> -    { KVM_CAP_CLOCKSOURCE, KVM_FEATURE_CLOCKSOURCE },
> -    { KVM_CAP_NOP_IO_DELAY, KVM_FEATURE_NOP_IO_DELAY },
> -    { KVM_CAP_PV_MMU, KVM_FEATURE_MMU_OP },
> -#ifdef KVM_CAP_ASYNC_PF
> -    { KVM_CAP_ASYNC_PF, KVM_FEATURE_ASYNC_PF },
> -#endif
> -    { -1, -1 }
> -};
> -
> -static int get_para_features(CPUState *env)
> -{
> -    int i, features = 0;
> -
> -    for (i = 0; i<  ARRAY_SIZE(para_features) - 1; i++) {
> -        if (kvm_check_extension(env->kvm_state, para_features[i].cap)) {
> -            features |= (1<<  para_features[i].feature);
> -        }
> -    }
> -#ifdef KVM_CAP_ASYNC_PF
> -    has_msr_async_pf_en = features&  (1<<  KVM_FEATURE_ASYNC_PF);
> -#endif
> -    return features;
> -}
> -#endif
> -

Oh.  The whole thing was copied wholesale.  Nevermind.

-- 
error compiling committee.c: too many arguments to function