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

Avi Kivity
2011-03-24T10:27:35Z

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