Re: [Qemu-devel] [PATCH 1/1] hyperv: cpu hotplug fix with HyperV enabled

[Igor Mammedov]

On Fri, 12 Feb 2016 14:27:25 +0300
"Denis V. Lunev" <address@hidden> wrote:

> On 02/12/2016 02:13 PM, Andreas Färber wrote:
> > Am 12.02.2016 um 12:08 schrieb Denis V. Lunev:  
> >> On 02/12/2016 02:00 PM, Andreas Färber wrote:  
> >>> Am 11.02.2016 um 21:19 schrieb Denis V. Lunev:  
> >>>> From: "Alexey V. Kostyushko" <address@hidden>
> >>>>
> >>>> With Hyper-V enabled CPU hotplug stops working. The CPU appears in
> >>>> device
> >>>> manager on Windows but does not appear in peformance monitor and control
> >>>> panel.
> >>>>
> >>>> The root of the problem is the following. Windows checks
> >>>> HV_X64_CPU_DYNAMIC_PARTITIONING_AVAILABLE bit in CPUID. The presence of
> >>>> this bit is enough to cure the situation.
> >>>>
> >>>> Add option 'hv-cpuhotplug' to control this behavior.
> >>>>
> >>>> Signed-off-by: Alexey V. Kostyushko <address@hidden>
> >>>> Signed-off-by: Denis V. Lunev <address@hidden>
> >>>> CC: Paolo Bonzini <address@hidden>
> >>>> CC: Richard Henderson <address@hidden>
> >>>> CC: Eduardo Habkost <address@hidden>
> >>>> CC: "Andreas Färber" <address@hidden>
> >>>> ---
> >>>>    target-i386/cpu-qom.h | 1 +
> >>>>    target-i386/cpu.c     | 1 +
> >>>>    target-i386/kvm.c     | 6 +++++-
> >>>>    3 files changed, 7 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
> >>>> index 5f9d960..4aec616 100644
> >>>> --- a/target-i386/cpu-qom.h
> >>>> +++ b/target-i386/cpu-qom.h
> >>>> @@ -96,6 +96,7 @@ typedef struct X86CPU {
> >>>>        bool hyperv_runtime;
> >>>>        bool hyperv_synic;
> >>>>        bool hyperv_stimer;
> >>>> +    bool hyperv_cpuhotplug;
> >>>>        bool check_cpuid;
> >>>>        bool enforce_cpuid;
> >>>>        bool expose_kvm;
> >>>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> >>>> index b255644..32c38ae 100644
> >>>> --- a/target-i386/cpu.c
> >>>> +++ b/target-i386/cpu.c
> >>>> @@ -3172,6 +3172,7 @@ static Property x86_cpu_properties[] = {
> >>>>        DEFINE_PROP_BOOL("hv-runtime", X86CPU, hyperv_runtime, false),
> >>>>        DEFINE_PROP_BOOL("hv-synic", X86CPU, hyperv_synic, false),
> >>>>        DEFINE_PROP_BOOL("hv-stimer", X86CPU, hyperv_stimer, false),
> >>>> +    DEFINE_PROP_BOOL("hv-cpuhotplug", X86CPU, hyperv_cpuhotplug,
> >>>> false),  
> >>> Is "cpuhotplug" some fixed HyperV name? Otherwise we generally use a
> >>> dashes convention for QOM properties, i.e. "hv-cpu-hotplug".
> >>>
> >>> Regards,
> >>> Andreas  
> >> This name is for libvirt. We can take one one. This is not a problem.
> >>
> >> Roman Kagan has proposed verbally a bit different approach.
> >> He suggests not to introduce the option but
> >> check here that HyperV is enabled (any single option is enough
> >> to face the problem) and CPU hotplug is enabled and automatically
> >> enable this bit.
> >>
> >> Paolo, Andreas, is there any opinion on this?  
> > That implicit proposal sounds even more appealing to me, yes.
> >
> > You could still additionally do a dynamic property though, in case you
> > want to inspect or toggle it at runtime (no clue when Windows reads it).  
> no. this will not work. I should setup CPUID at the beginning
> (before boot) and keep it persistent. Thus either option
> coming from libvirt or this bit would be automatically
> calculated by QEMU.
> 
> I'll check that we could create read-only property to export
> the value.
> 
> OK. I'll redo this tomorrow if nobody will explicitly say 'no'
> today :)
+to Anreas suggestion and readonly prop

there is not need to create a field for it in CPU structure,
just checking for present hyperv flags should be sufficient.


> 
> Den
>