On Mon, May 24, 2021 at 02:13:09PM +0200, Vitaly Kuznetsov wrote:
[...]
> >> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> >> index a42263b24fca..d5551c4ab5cf 100644
> >> --- a/target/i386/kvm/kvm.c
> >> +++ b/target/i386/kvm/kvm.c
> >> @@ -1216,13 +1216,22 @@ static uint32_t hv_build_cpuid_leaf(CPUState *cs,
> >> uint32_t func, int reg)
> >> * of 'hv_passthrough' mode and fills the environment with all supported
> >> * Hyper-V features.
> >> */
> >> -static void hyperv_expand_features(CPUState *cs, Error **errp)
> >> +void kvm_hyperv_expand_features(X86CPU *cpu, Error **errp)
> >> {
> >> - X86CPU *cpu = X86_CPU(cs);
> >> + CPUState *cs = CPU(cpu);
> >>
> >> if (!hyperv_enabled(cpu))
> >> return;
> >>
> >> + /*
> >> + * When kvm_hyperv_expand_features is called at CPU feature expansion
> >> + * time per-CPU kvm_state is not available yet so we can only proceed
> >> + * when KVM_CAP_SYS_HYPERV_CPUID is supported.
> >> + */
> >> + if (!cs->kvm_state &&
> >> + !kvm_check_extension(kvm_state, KVM_CAP_SYS_HYPERV_CPUID))
> >> + return;
> >> +
> >> if (cpu->hyperv_passthrough) {
> >> cpu->hyperv_vendor_id[0] =
> >> hv_cpuid_get_host(cs, HV_CPUID_VENDOR_AND_MAX_FUNCTIONS,
> >> R_EBX);
> >> @@ -1556,7 +1565,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
> >> env->apic_bus_freq = KVM_APIC_BUS_FREQUENCY;
> >>
> >> /* Paravirtualization CPUIDs */
> >> - hyperv_expand_features(cs, &local_err);
> >> + kvm_hyperv_expand_features(cpu, &local_err);
> >
> > Do we still need to call the function again here?
> >
> > If the first expansion isn't expanding everything, I'm afraid
> > this second call will hide bugs in query-cpu-model-expansion.
> >
>
> The first expansion will do nothing if KVM_CAP_SYS_HYPERV_CPUID is not
> supported, calling it here allows us to proceed. The series makes
> 'query-cpu-model-expansion' output correct only with
> KVM_CAP_SYS_HYPERV_CPUID, without it we don't seem to be able to do much
> (unless we decide to create a 'scratch' CPU or something like that).
Oh, I see. I suggest adding a comment explaining that.
Developers might be tempted to delete it and not notice it breaks
under older kernels.
--
Eduardo
