> -----Original Message----- > From: Eduardo Habkost [mailto:ehabk...@redhat.com] > Sent: Sunday, September 10, 2017 4:46 AM > To: Gonglei (Arei) > Cc: qemu-devel@nongnu.org; m...@redhat.com; pbonz...@redhat.com; > r...@twiddle.net; mtosa...@redhat.com; vroze...@redhat.com; > Huangweidong (C) > Subject: Re: [PATCH 2/2] i386/cpu/hyperv: support over 64 vcpus for windows > guests > > On Thu, Sep 07, 2017 at 01:05:33AM +0000, Gonglei (Arei) wrote: > > > > > > > -----Original Message----- > > > From: Eduardo Habkost [mailto:ehabk...@redhat.com] > > > Sent: Tuesday, September 05, 2017 9:17 PM > > > To: Gonglei (Arei) > > > Cc: qemu-devel@nongnu.org; m...@redhat.com; pbonz...@redhat.com; > > > r...@twiddle.net; mtosa...@redhat.com; vroze...@redhat.com; > > > Huangweidong (C) > > > Subject: Re: [PATCH 2/2] i386/cpu/hyperv: support over 64 vcpus for > windows > > > guests > > > > > > On Tue, Sep 05, 2017 at 05:30:05PM +0800, Gonglei wrote: > > > > Starting with Windows Server 2012 and Windows 8, if > > > > CPUID.40000005.EAX contains a value of -1, Windows assumes specific > > > > limit to the number of VPs. In this case, Windows Server 2012 > > > > guest VMs may use more than 64 VPs, up to the maximum supported > > > > number of processors applicable to the specific Windows > > > > version being used. > > > > > > > > > > > > https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/referenc > > > e/tlfs > > > > > > > > For compatibility, Let's introduce a new property for X86CPU, > > > > named "hv-cpuid-limits-eax" as Paolo's suggestion, and set it > > > > to "on" before machine 2.10. > > > > > > > > Signed-off-by: Gonglei <arei.gong...@huawei.com> > > > > --- > > > > include/hw/i386/pc.h | 5 +++++ > > > > target/i386/cpu.c | 1 + > > > > target/i386/cpu.h | 2 ++ > > > > target/i386/kvm.c | 18 +++++++++++++++++- > > > > 4 files changed, 25 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > > > > index 8226904..db32e58 100644 > > > > --- a/include/hw/i386/pc.h > > > > +++ b/include/hw/i386/pc.h > > > > @@ -371,6 +371,11 @@ bool e820_get_entry(int, uint32_t, uint64_t *, > > > uint64_t *); > > > > > > > > #define PC_COMPAT_2_10 \ > > > > HW_COMPAT_2_10 \ > > > > + {\ > > > > + .driver = TYPE_X86_CPU,\ > > > > + .property = "hv_cpuid_limits_eax",\ > > > > > > The property name is hv-cpuid-limits-eax. > > > > > Make sense to me. > > > > > > + .value = "on",\ > > > > + },\ > > > > > > > > #define PC_COMPAT_2_9 \ > > > > HW_COMPAT_2_9 \ > > > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > > > > index 69676e1..0d47bdd 100644 > > > > --- a/target/i386/cpu.c > > > > +++ b/target/i386/cpu.c > > > > @@ -4145,6 +4145,7 @@ static Property x86_cpu_properties[] = { > > > > false), > > > > DEFINE_PROP_BOOL("vmware-cpuid-freq", X86CPU, > > > vmware_cpuid_freq, true), > > > > DEFINE_PROP_BOOL("tcg-cpuid", X86CPU, expose_tcg, true), > > > > + DEFINE_PROP_BOOL("hv-cpuid-limits-eax", X86CPU, > > > hv_cpuid_limits_eax, false), > > > > > > The property name "hv-cpuid-limits-eax" doesn't say anything > > > about what it does exactly when set to true. > > > > > > What about just making it int32? e.g.: > > > > > > DEFINE_PROP_INT32("x-hv-max-vps", X86CPU, hv_max_vps, -1) > > > [...] > > > {\ > > > .driver = TYPE_X86_CPU,\ > > > .property = "x-hv-max-vps",\ > > > .value = "0x40",\ > > > },\ > > > [...] > > > c->function = HYPERV_CPUID_IMPLEMENT_LIMITS; > > > c->eax = cpu->hv_max_vps; > > > > > > > > > (The "x-" prefix indicates that the property is not supposed to > > > be a stable user interface.) > > > > > I thought about this as well. > > but actually we can't assure that users set the x-hv-max-vps an invalid > > value if > > we use this method. Do we really need to expose eax directly? > > We don't really need to expose eax directly and I'm not against a > boolean property, but I think an integer property with the actual > CPUID value makes the compat code simpler and the purpose of the > entry at PC_COMPAT_* more obvious. > > Properties prefixed with "x-" are for internal QEMU usage or > debugging, if users want to fiddle with it, they do it at their > own risk. I don't see a problem with that. > Ok, it's fair. Will do it as your suggestion.
Thanks, -Gonglei