Re: [Qemu-devel] [PATCH for-2.12 2/2] i386/hyperv: error out if features requested but unsupported

[Eduardo Habkost <ehabkost@redhat.com>]

On Fri, Mar 23, 2018 at 03:58:08PM +0300, Roman Kagan wrote:
> In order to guarantee compatibility on migration, QEMU should have
> complete control over the features it announces to the guest via CPUID.
> 
> However, for a number of Hyper-V-related cpu properties, if the
> corresponding feature is not supported by the underlying KVM, the
> propery is silently ignored and the feature is not announced to the
> guest.
> 
> Refuse to start with an error instead.
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>

I wonder if we should make these just warnings on -stable, and
make them fatal errors only on 2.12.  I wouldn't want to make
existing running VMs not runnable on a stable update.


> ---
>  target/i386/kvm.c | 25 +++++++++++++++++++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index fb20ff18c2..c9c359241c 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -658,17 +658,34 @@ static int hyperv_handle_properties(CPUState *cs)
>          env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_FREQUENCY_MSRS;
>          env->features[FEAT_HYPERV_EDX] |= HV_FREQUENCY_MSRS_AVAILABLE;
>      }
> -    if (cpu->hyperv_crash && has_msr_hv_crash) {
> +    if (cpu->hyperv_crash) {
> +        if (!has_msr_hv_crash) {
> +            fprintf(stderr,
> +                    "Hyper-V crash MSRs are not supported by kernel\n");

I would mention the corresponding "hv-..." -cpu option
explicitly, for clarity.

> +            return -ENOSYS;
> +        }
>          env->features[FEAT_HYPERV_EDX] |= HV_GUEST_CRASH_MSR_AVAILABLE;
>      }
>      env->features[FEAT_HYPERV_EDX] |= HV_CPU_DYNAMIC_PARTITIONING_AVAILABLE;
> -    if (cpu->hyperv_reset && has_msr_hv_reset) {
> +    if (cpu->hyperv_reset) {
> +       if (!has_msr_hv_reset) {
> +            fprintf(stderr, "Hyper-V reset MSR is not supported by kernel\n");
> +            return -ENOSYS;
> +        }
>          env->features[FEAT_HYPERV_EAX] |= HV_RESET_AVAILABLE;
>      }
> -    if (cpu->hyperv_vpindex && has_msr_hv_vpindex) {
> +    if (cpu->hyperv_vpindex) {
> +        if (!has_msr_hv_vpindex) {
> +            fprintf(stderr, "Hyper-V VP_INDEX is not supported by kernel\n");
> +            return -ENOSYS;
> +        }
>          env->features[FEAT_HYPERV_EAX] |= HV_VP_INDEX_AVAILABLE;
>      }
> -    if (cpu->hyperv_runtime && has_msr_hv_runtime) {
> +    if (cpu->hyperv_runtime) {
> +        if (!has_msr_hv_runtime) {
> +            fprintf(stderr, "Hyper-V VP_INDEX is not supported by kernel\n");
> +            return -ENOSYS;
> +        }
>          env->features[FEAT_HYPERV_EAX] |= HV_VP_RUNTIME_AVAILABLE;
>      }
>      if (cpu->hyperv_synic) {
> -- 
> 2.14.3
> 

-- 
Eduardo