[libvirt] [PATCH 3/3] qemu_process: add check for hyperv features

[Maxim Nestratov]

29.03.2016 16:31, Pavel Hrdina пишет:
> Commit 7068b56c introduced several hyperv features.  Not all hyperv
> features are supported by old enough kernels and we shouldn't allow to
> start a guest if kernel doesn't support any of the hyperv feature.
>
> There is one exception, for backward compatibility we cannot error out
> if one of the RELAXED, VAPIC or SPINLOCKS isn't supported, for the same
> reason we ignore invtsc, to not break restoring saved domains with older
> libvirt.
>
> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> ---
>   src/cpu/cpu_x86.c       |  8 ++++++++
>   src/cpu/cpu_x86_data.h  |  8 ++++++++
>   src/qemu/qemu_process.c | 31 +++++++++++++++++++++++++++++++
>   3 files changed, 47 insertions(+)
>
> diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
> index 90949f6..b7f1690 100644
> --- a/src/cpu/cpu_x86.c
> +++ b/src/cpu/cpu_x86.c
> @@ -75,6 +75,14 @@ static const struct x86_kvm_feature x86_kvm_features[] =
>       {VIR_CPU_x86_KVM_PV_UNHALT,    { .function = 0x40000001, .eax = 0x00000080 }},
>       {VIR_CPU_x86_KVM_CLOCKSOURCE_STABLE_BIT,
>                                      { .function = 0x40000001, .eax = 0x01000000 }},
> +    {VIR_CPU_x86_KVM_HV_RUNTIME,   { .function = 0x40000003, .eax = 0x00000001 }},
> +    {VIR_CPU_x86_KVM_HV_SYNIC,     { .function = 0x40000003, .eax = 0x00000004 }},
> +    {VIR_CPU_x86_KVM_HV_STIMER,    { .function = 0x40000003, .eax = 0x00000008 }},
> +    {VIR_CPU_x86_KVM_HV_RELAXED,   { .function = 0x40000003, .eax = 0x00000020 }},
> +    {VIR_CPU_x86_KVM_HV_SPINLOCK,  { .function = 0x40000003, .eax = 0x00000022 }},
> +    {VIR_CPU_x86_KVM_HV_VAPIC,     { .function = 0x40000003, .eax = 0x00000030 }},
> +    {VIR_CPU_x86_KVM_HV_VPINDEX,   { .function = 0x40000003, .eax = 0x00000040 }},
> +    {VIR_CPU_x86_KVM_HV_RESET,     { .function = 0x40000003, .eax = 0x00000080 }},
>   };
>   
>   struct x86_model {
> diff --git a/src/cpu/cpu_x86_data.h b/src/cpu/cpu_x86_data.h
> index 88dccf6..777cc8d 100644
> --- a/src/cpu/cpu_x86_data.h
> +++ b/src/cpu/cpu_x86_data.h
> @@ -48,6 +48,14 @@ struct _virCPUx86CPUID {
>   # define VIR_CPU_x86_KVM_PV_EOI       "__kvm_pv_eoi"
>   # define VIR_CPU_x86_KVM_PV_UNHALT    "__kvm_pv_unhalt"
>   # define VIR_CPU_x86_KVM_CLOCKSOURCE_STABLE_BIT "__kvm_clocksource_stable"
> +# define VIR_CPU_x86_KVM_HV_RUNTIME   "__kvm_hv_runtime"
> +# define VIR_CPU_x86_KVM_HV_SYNIC     "__kvm_hv_synic"
> +# define VIR_CPU_x86_KVM_HV_STIMER    "__kvm_hv_stimer"
> +# define VIR_CPU_x86_KVM_HV_RELAXED   "__kvm_hv_relaxed"
> +# define VIR_CPU_x86_KVM_HV_SPINLOCK  "__kvm_hv_spinlock"
> +# define VIR_CPU_x86_KVM_HV_VAPIC     "__kvm_hv_vapic"
> +# define VIR_CPU_x86_KVM_HV_VPINDEX   "__kvm_hv_vpindex"
> +# define VIR_CPU_x86_KVM_HV_RESET     "__kvm_hv_reset"
>   
>   
>   typedef struct _virCPUx86Data virCPUx86Data;
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 9334a75..07b9df2 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -3928,6 +3928,37 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver,
>               }
>           }
>   
> +        for (i = 0; i < VIR_DOMAIN_HYPERV_LAST; i++) {
> +            if (def->hyperv_features[i] == VIR_TRISTATE_SWITCH_ON) {
> +                char *cpuFeature;
> +                if (virAsprintf(&cpuFeature, "__kvm_hv_%s",
> +                                virDomainHypervTypeToString(i)) < 0)
> +                    goto cleanup;
> +                if (!cpuHasFeature(guestcpu, cpuFeature)) {
> +                    switch ((virDomainHyperv) i) {
> +                    case VIR_DOMAIN_HYPERV_RELAXED:
> +                    case VIR_DOMAIN_HYPERV_VAPIC:
> +                    case VIR_DOMAIN_HYPERV_SPINLOCKS:
> +                        VIR_WARN("host doesn't support hyperv '%s' feature",
> +                                 virDomainHypervTypeToString(i));
> +                        break;
> +                    case VIR_DOMAIN_HYPERV_VPINDEX:
> +                    case VIR_DOMAIN_HYPERV_RUNTIME:
> +                    case VIR_DOMAIN_HYPERV_SYNIC:
> +                    case VIR_DOMAIN_HYPERV_STIMER:
> +                    case VIR_DOMAIN_HYPERV_RESET:
> +                    case VIR_DOMAIN_HYPERV_VENDOR_ID:
> +                        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                                       _("host doesn't support hyperv '%s' feature"),
> +                                       virDomainHypervTypeToString(i));
> +                        goto cleanup;
> +                        break;
> +                    case VIR_DOMAIN_HYPERV_LAST:
> +                        break;
> +                    }
> +                }
> +            }
> +        }
>   
>           if (def->cpu && def->cpu->mode != VIR_CPU_MODE_HOST_PASSTHROUGH) {
>               for (i = 0; i < def->cpu->nfeatures; i++) {

Hmm, qemu already checks them and simply ignores most of them and 
doesn't prevent guest from starting in case they are not supported and 
optional. In case they are reqired it fails. Why should we check them 
here? At least we should follow the logic qemu has.

Extract from target-i386/kvm.c does the following:

         if (cpu->hyperv_relaxed_timing) {
             c->eax |= HV_X64_MSR_HYPERCALL_AVAILABLE;
         }
         if (cpu->hyperv_vapic) {
             c->eax |= HV_X64_MSR_HYPERCALL_AVAILABLE;
             c->eax |= HV_X64_MSR_APIC_ACCESS_AVAILABLE;
             has_msr_hv_vapic = true;
         }
...
         if (cpu->hyperv_crash && has_msr_hv_crash) {
             c->edx |= HV_X64_GUEST_CRASH_MSR_AVAILABLE;
         }
         if (cpu->hyperv_reset && has_msr_hv_reset) {
             c->eax |= HV_X64_MSR_RESET_AVAILABLE;
         }
         if (cpu->hyperv_vpindex && has_msr_hv_vpindex) {
             c->eax |= HV_X64_MSR_VP_INDEX_AVAILABLE;
         }
         if (cpu->hyperv_runtime && has_msr_hv_runtime) {
             c->eax |= HV_X64_MSR_VP_RUNTIME_AVAILABLE;
         }

thus, features  hv_vapic, hv_relaxed, hv_reset, hv_runtime, hv_crash, 
hv_vpindex are ignored if not supported. Only absence of hv_synic, 
hv_stimer leads to error.

Maxim