Re: [PATCH RFC 22/22] i386: expand Hyper-V features early

[Eduardo Habkost <ehabkost@redhat.com>]

On Fri, Sep 04, 2020 at 04:54:31PM +0200, Vitaly Kuznetsov wrote:
> To make Hyper-V features appear in e.g. QMP query-cpu-model-expansion we
> need to expand and set the corresponding CPUID leaves early. Modify
> x86_cpu_get_supported_feature_word() to call newly intoduced Hyper-V
> specific kvm_hv_get_supported_cpuid() instead of
> kvm_arch_get_supported_cpuid(). We can't use kvm_arch_get_supported_cpuid()
> as Hyper-V specific CPUID leaves intersect with KVM's.
> 
> Note, early expansion will only happen when KVM supports system wide
> KVM_GET_SUPPORTED_HV_CPUID ioctl (KVM_CAP_SYS_HYPERV_CPUID).
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
[...]
> +uint32_t kvm_hv_get_supported_cpuid(X86CPU *cpu, enum FeatureWord w)
> +{
> +    CPUState *cs = CPU(cpu);
> +    CPUX86State *env = &cpu->env;
> +    Error *local_err = NULL;
> +
> +    hyperv_expand_features(cs, &local_err);

This makes a function that sounds like it doesn't have any side
effect ("get supported cpuid") have an unintended side effect
(hyperv_expand_features() will change all CPUID data inside the
cpu object).

What about making it more similar to
kvm_arch_get_supported_cpuid(), and be just a wrapper to
get_supported_hv_cpuid()?

I would also make sure get_supported_hv_cpuid() doesn't get
CPUState as argument, just to be sure it will never touch the CPU
object state.

> +
> +    if (local_err) {
> +            error_report_err(local_err);
> +    }
> +
> +    return env->features[w];
> +}
> +
>  static Error *hv_passthrough_mig_blocker;
>  static Error *hv_no_nonarch_cs_mig_blocker;
>  
> diff --git a/target/i386/kvm_i386.h b/target/i386/kvm_i386.h
> index 064b8798a26c..2e7da4f39668 100644
> --- a/target/i386/kvm_i386.h
> +++ b/target/i386/kvm_i386.h
> @@ -48,4 +48,11 @@ bool kvm_has_waitpkg(void);
>  
>  bool kvm_hv_vpindex_settable(void);
>  
> +static inline bool hyperv_feature_word(enum FeatureWord w)
> +{
> +    return w >= FEAT_HYPERV_EAX && w <= FEAT_HV_NESTED_EDX;
> +}
> +
> +uint32_t kvm_hv_get_supported_cpuid(X86CPU *cpu, enum FeatureWord w);
> +
>  #endif
> -- 
> 2.25.4
> 

-- 
Eduardo