Re: [Qemu-devel] [PATCH 2/2] i386/cpu/hyperv: support over 64 vcpus for windows guests

[Eduardo Habkost]

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/reference/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 <address@hidden>
> ---
>  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.

> +        .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.)


>      DEFINE_PROP_END_OF_LIST()
>  };
>  
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 525d35d..f8b455a 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1282,6 +1282,8 @@ struct X86CPU {
>      int32_t socket_id;
>      int32_t core_id;
>      int32_t thread_id;
> +
> +    bool hv_cpuid_limits_eax;
>  };
>  
>  static inline X86CPU *x86_env_get_cpu(CPUX86State *env)
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 6db7783..cf6ef96 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -751,7 +751,23 @@ int kvm_arch_init_vcpu(CPUState *cs)
>  
>          c = &cpuid_data.entries[cpuid_i++];
>          c->function = HYPERV_CPUID_IMPLEMENT_LIMITS;
> -        c->eax = 0x40;
> +
> +        if (!cpu->hv_cpuid_limits_eax) {
> +            /*
> +             * 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.

That was a direct quote from a document, so I recommend citing
the specific document you quoted.  e.g.:

    /*
     * From "Requirements for Implementing the Microsoft
     * Hypervisor Interface":
     * 
https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/reference/tlfs
     *
     * "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/reference/tlfs

IMO a long line is preferable to a broken URL.

> +             */
> +            c->eax = -1;
> +        } else {
> +            c->eax = 0x40;
> +        }
>          c->ebx = 0x40;
>  
>          kvm_base = KVM_CPUID_SIGNATURE_NEXT;
> -- 
> 1.8.3.1
> 
> 

-- 
Eduardo