[Qemu-devel] [PATCH for-2.12 0/2] i386/hyperv: fully control Hyper-V features in CPUID

[Qemu-devel] [PATCH for-2.12 0/2] i386/hyperv: fully control Hyper-V features in CPUID

[Roman Kagan]

In order to guarantee compatibility on migration, QEMU should have
complete control over the features it announces to the guest via CPUID.

However, a number of Hyper-V-related features happen to depend on the
support in the underlying KVM, with no regard to QEMU configuration.

Make QEMU regain control over what Hyper-V features it announces to the
guest.

Note: the patches are also being proposed for stable-2.11, even though
one of them introduces a new cpu property.  This is done to minimize the
number of published QEMU releases where the behavior of the features is
unpredictable, with potentially fatal consequences for the guest.

Note #2: there are other problems in the surrounding code, like ugly
error reporting or inconsistent population of MSRs.  I think this can be
put off to post-2.12.

Roman Kagan (2):
  i386/hyperv: add hv-frequencies cpu property
  i386/hyperv: error out if features requested but unsupported

Cc: qemu-stable@nongnu.org

 target/i386/cpu.h |  1 +
 target/i386/cpu.c |  1 +
 target/i386/kvm.c | 37 +++++++++++++++++++++++++++++--------
 3 files changed, 31 insertions(+), 8 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH for-2.12 1/2] i386/hyperv: add hv-frequencies cpu property

[Roman Kagan]

In order to guarantee compatibility on migration, QEMU should have
complete control over the features it announces to the guest via CPUID.

However, the declared availability of Hyper-V frequency MSRs
(HV_X64_MSR_TSC_FREQUENCY and HV_X64_MSR_APIC_FREQUENCY) depends solely
on the support for them in the underlying KVM.

Introduce "hv-frequencies" cpu property (off by default) which gives
QEMU full control over whether these MSRs are announced.

While at this, drop the redundant check of the cpu tsc frequency, and
decouple this feature from hv-time.

Cc: qemu-stable@nongnu.org
Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
Note: this patch introduces a new cpu property, which is not what we
normally do in stable branches.  However, this appears to be the minimal
effort/churn approach to reduce the number of published QEMU releases
where the behavior of the feature is unpredictable, with potentially
fatal consequences for the guest.

 target/i386/cpu.h |  1 +
 target/i386/cpu.c |  1 +
 target/i386/kvm.c | 12 ++++++++----
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 78db1b833a..1b219fafc4 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1296,6 +1296,7 @@ struct X86CPU {
     bool hyperv_runtime;
     bool hyperv_synic;
     bool hyperv_stimer;
+    bool hyperv_frequencies;
     bool check_cpuid;
     bool enforce_cpuid;
     bool expose_kvm;
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 555ae79d29..1a6b082b6f 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4761,6 +4761,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-frequencies", X86CPU, hyperv_frequencies, false),
     DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
     DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
     DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index d23fff12f5..fb20ff18c2 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -648,11 +648,15 @@ static int hyperv_handle_properties(CPUState *cs)
         env->features[FEAT_HYPERV_EAX] |= HV_HYPERCALL_AVAILABLE;
         env->features[FEAT_HYPERV_EAX] |= HV_TIME_REF_COUNT_AVAILABLE;
         env->features[FEAT_HYPERV_EAX] |= HV_REFERENCE_TSC_AVAILABLE;
-
-        if (has_msr_hv_frequencies && tsc_is_stable_and_known(env)) {
-            env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_FREQUENCY_MSRS;
-            env->features[FEAT_HYPERV_EDX] |= HV_FREQUENCY_MSRS_AVAILABLE;
+    }
+    if (cpu->hyperv_frequencies) {
+        if (!has_msr_hv_frequencies) {
+            fprintf(stderr,
+                    "Hyper-V frequency MSRs are not supported by kernel\n");
+            return -ENOSYS;
         }
+        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) {
         env->features[FEAT_HYPERV_EDX] |= HV_GUEST_CRASH_MSR_AVAILABLE;
-- 
2.14.3

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

[Roman Kagan]

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>
---
 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");
+            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

Re: [Qemu-devel] [PATCH for-2.12 0/2] i386/hyperv: fully control Hyper-V features in CPUID

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180323125808.4479-1-rkagan@virtuozzo.com
Subject: [Qemu-devel] [PATCH for-2.12 0/2] i386/hyperv: fully control Hyper-V features in CPUID

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
2efbb18fe2 i386/hyperv: error out if features requested but unsupported
31e7467f4e i386/hyperv: add hv-frequencies cpu property

=== OUTPUT BEGIN ===
Checking PATCH 1/2: i386/hyperv: add hv-frequencies cpu property...
Checking PATCH 2/2: i386/hyperv: error out if features requested but unsupported...
ERROR: suspect code indent for conditional statements (4, 7)
#39: FILE: target/i386/kvm.c:670:
+    if (cpu->hyperv_reset) {
+       if (!has_msr_hv_reset) {

total: 1 errors, 0 warnings, 38 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

Re: [Qemu-devel] [PATCH for-2.12 1/2] i386/hyperv: add hv-frequencies cpu property

[Eduardo Habkost]

On Fri, Mar 23, 2018 at 03:58:07PM +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, the declared availability of Hyper-V frequency MSRs
> (HV_X64_MSR_TSC_FREQUENCY and HV_X64_MSR_APIC_FREQUENCY) depends solely
> on the support for them in the underlying KVM.

So this problem was introduced fairly recently (in v2.11).  This
makes the decision to break compatibility of (some[1]) existing
configurations that didn't specify "hv-frequency" a bit easier.

> Introduce "hv-frequencies" cpu property (off by default) which gives
> QEMU full control over whether these MSRs are announced.
> 

So, as we have two possible results when running QEMU-2.11, we
need to make a guess and choose which half of our users will be
affected:

a) People running "-machine pc-2.11 -cpu ...,+hv-time" on Linux 4.14+
   (including commit 72c139bacfa3), that have hv-frequencies
   enabled automatically.
b) People running "-machine pc-2.11 -cpu ...,+hv-time" on Linux
   4.13 and older (without commit 72c139bacfa3), that have
   hv-frequencies disabled.

If we set hv-frequencies=off by default on pc-2.11 (this patch),
we will inconvenience group (a).  The consequence for them is
having hv-frequencies disabled suddenly on CPUID after updating
QEMU.  The MSRs will still be available to the guest, however (so
the guest won't crash), and they can add hv-frequencies=on to
their configuration manually.

If we set hv-frequencies=on by default on pc-2.11, we will
inconvenience group (b).  The consequence for them is having the
VM not being runnable anymore until they change the machine-type
or add hv-frequencies=off to their configuration.

So it looks like this patch is the safest solution, but I will
get back to the "[PATCH v3 2/2] i386/kvm: lower requirements for
Hyper-V frequency MSRs exposure" thread to be sure we are not
missing anything.


> While at this, drop the redundant check of the cpu tsc frequency, and
> decouple this feature from hv-time.
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
> ---
> Note: this patch introduces a new cpu property, which is not what we
> normally do in stable branches.  However, this appears to be the minimal
> effort/churn approach to reduce the number of published QEMU releases
> where the behavior of the feature is unpredictable, with potentially
> fatal consequences for the guest.
> 
>  target/i386/cpu.h |  1 +
>  target/i386/cpu.c |  1 +
>  target/i386/kvm.c | 12 ++++++++----
>  3 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index 78db1b833a..1b219fafc4 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1296,6 +1296,7 @@ struct X86CPU {
>      bool hyperv_runtime;
>      bool hyperv_synic;
>      bool hyperv_stimer;
> +    bool hyperv_frequencies;
>      bool check_cpuid;
>      bool enforce_cpuid;
>      bool expose_kvm;
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 555ae79d29..1a6b082b6f 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -4761,6 +4761,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-frequencies", X86CPU, hyperv_frequencies, false),
>      DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
>      DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
>      DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index d23fff12f5..fb20ff18c2 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -648,11 +648,15 @@ static int hyperv_handle_properties(CPUState *cs)
>          env->features[FEAT_HYPERV_EAX] |= HV_HYPERCALL_AVAILABLE;
>          env->features[FEAT_HYPERV_EAX] |= HV_TIME_REF_COUNT_AVAILABLE;
>          env->features[FEAT_HYPERV_EAX] |= HV_REFERENCE_TSC_AVAILABLE;
> -
> -        if (has_msr_hv_frequencies && tsc_is_stable_and_known(env)) {
> -            env->features[FEAT_HYPERV_EAX] |= HV_ACCESS_FREQUENCY_MSRS;
> -            env->features[FEAT_HYPERV_EDX] |= HV_FREQUENCY_MSRS_AVAILABLE;
> +    }
> +    if (cpu->hyperv_frequencies) {
> +        if (!has_msr_hv_frequencies) {
> +            fprintf(stderr,
> +                    "Hyper-V frequency MSRs are not supported by kernel\n");
> +            return -ENOSYS;
>          }
> +        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) {
>          env->features[FEAT_HYPERV_EDX] |= HV_GUEST_CRASH_MSR_AVAILABLE;
> -- 
> 2.14.3
> 

-- 
Eduardo

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

[Eduardo Habkost]

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

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

[Roman Kagan]

On Fri, Mar 23, 2018 at 04:56:21PM -0300, Eduardo Habkost wrote:
> 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.

OK let's follow your approach and consider who would suffer more:

a) users who started a VM on a newer kernel and then migrated to an
   older one, and got a guest crash on an unsupported MSR (I'm not even
   sure they'd be able to see the warnings)

b) users who had a VM configuration with features which didn't actually
   work (but that wasn't apparently a problem for them), and suddenly
   couldn't start their VM after QEMU upgrade (the problem is not only
   cold-restarting per se, but also live migration to the upgraded
   version: dunno if the management layer would allow to adjust the VM
   config to migrate successfully).

I don't have a strong opinion on this, will follow whatever direction
you advise.


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

This sounds like a good idea, but I wonder if it should better be done
uniformly for all hv_* options, together with transitioning to
error_report(), and whether we want to do this at this point in the
release cycle.

Thanks,
Roman.

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

[Eduardo Habkost]

On Mon, Mar 26, 2018 at 06:06:16PM +0300, Roman Kagan wrote:
> On Fri, Mar 23, 2018 at 04:56:21PM -0300, Eduardo Habkost wrote:
> > 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.
> 
> OK let's follow your approach and consider who would suffer more:
> 
> a) users who started a VM on a newer kernel and then migrated to an
>    older one, and got a guest crash on an unsupported MSR (I'm not even
>    sure they'd be able to see the warnings)
> 
> b) users who had a VM configuration with features which didn't actually
>    work (but that wasn't apparently a problem for them), and suddenly
>    couldn't start their VM after QEMU upgrade (the problem is not only
>    cold-restarting per se, but also live migration to the upgraded
>    version: dunno if the management layer would allow to adjust the VM
>    config to migrate successfully).
> 
> I don't have a strong opinion on this, will follow whatever direction
> you advise.

(a) is an existing bug (and rare, it seems: I didn't see it being
reported anywhere).  (b) would be a regression in a -stable
update.  I'd prefer to be conservative on -stable.

> 
> 
> > >  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.
> 
> This sounds like a good idea, but I wonder if it should better be done
> uniformly for all hv_* options, together with transitioning to
> error_report(), and whether we want to do this at this point in the
> release cycle.

I don't think it will be a problem if we mention the property
names in the new messages, but not try to reword the existing
ones.

-- 
Eduardo