[PATCH] i386/kvm: add NoNonArchitecturalCoreSharing Hyper-V enlightenment

[PATCH] i386/kvm: add NoNonArchitecturalCoreSharing Hyper-V enlightenment

[Vitaly Kuznetsov]

Hyper-V TLFS specifies this enlightenment as:
"NoNonArchitecturalCoreSharing - Indicates that a virtual processor will never
share a physical core with another virtual processor, except for virtual
processors that are reported as sibling SMT threads. This can be used as an
optimization to avoid the performance overhead of STIBP".

However, STIBP is not the only implication. It was found that Hyper-V on
KVM doesn't pass MD_CLEAR bit to its guests if it doesn't see
NoNonArchitecturalCoreSharing bit.

KVM reports NoNonArchitecturalCoreSharing in KVM_GET_SUPPORTED_HV_CPUID to
indicate that SMT on the host is impossible (not supported of forcefully
disabled).

Implement NoNonArchitecturalCoreSharing support in QEMU as tristate:
'off' - the feature is disabled (default)
'on' - the feature is enabled. This is only safe if vCPUS are properly
 pinned and correct topology is exposed. As CPU pinning is done outside
 of QEMU the enablement decision will be made on a higher level.
'auto' - copy KVM setting. As during live migration SMT settings on the
source and destination host may differ this requires us to add a migration
blocker.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
---
 docs/hyperv.txt            | 13 +++++++++++++
 target/i386/cpu.c          |  2 ++
 target/i386/cpu.h          |  2 ++
 target/i386/hyperv-proto.h |  1 +
 target/i386/kvm.c          | 26 ++++++++++++++++++++++++++
 5 files changed, 44 insertions(+)

diff --git a/docs/hyperv.txt b/docs/hyperv.txt
index 8fdf25c8291c..6518b716a958 100644
--- a/docs/hyperv.txt
+++ b/docs/hyperv.txt
@@ -184,6 +184,19 @@ enabled.
 
 Requires: hv-vpindex, hv-synic, hv-time, hv-stimer
 
+3.17. hv-no-nonarch-coresharing=on/off/auto
+===========================================
+This enlightenment tells guest OS that virtual processors will never share a
+physical core unless they are reported as sibling SMT threads. This information
+is required by Windows and Hyper-V guests to properly mitigate SMT related CPU
+vulnerabilities.
+When the option is set to 'auto' QEMU will enable the feature only when KVM
+reports that non-architectural coresharing is impossible, this means that
+hyper-threading is not supported or completely disabled on the host. This
+setting also prevents migration as SMT settings on the destination may differ.
+When the option is set to 'on' QEMU will always enable the feature, regardless
+of host setup. To keep guests secure, this can only be used in conjunction with
+exposing correct vCPU topology and vCPU pinning.
 
 4. Development features
 ========================
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 44f1bbdcac76..4086c0a16767 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6156,6 +6156,8 @@ static Property x86_cpu_properties[] = {
                       HYPERV_FEAT_IPI, 0),
     DEFINE_PROP_BIT64("hv-stimer-direct", X86CPU, hyperv_features,
                       HYPERV_FEAT_STIMER_DIRECT, 0),
+    DEFINE_PROP_ON_OFF_AUTO("hv-no-nonarch-coresharing", X86CPU,
+                            hyperv_no_nonarch_cs, ON_OFF_AUTO_OFF),
     DEFINE_PROP_BOOL("hv-passthrough", X86CPU, hyperv_passthrough, false),
 
     DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index eaa5395aa539..9f47c1e2a52d 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -24,6 +24,7 @@
 #include "cpu-qom.h"
 #include "hyperv-proto.h"
 #include "exec/cpu-defs.h"
+#include "qapi/qapi-types-common.h"
 
 /* The x86 has a strong memory model with some store-after-load re-ordering */
 #define TCG_GUEST_DEFAULT_MO      (TCG_MO_ALL & ~TCG_MO_ST_LD)
@@ -1563,6 +1564,7 @@ struct X86CPU {
     bool hyperv_synic_kvm_only;
     uint64_t hyperv_features;
     bool hyperv_passthrough;
+    OnOffAuto hyperv_no_nonarch_cs;
 
     bool check_cpuid;
     bool enforce_cpuid;
diff --git a/target/i386/hyperv-proto.h b/target/i386/hyperv-proto.h
index cffac10b45dc..056a305be38c 100644
--- a/target/i386/hyperv-proto.h
+++ b/target/i386/hyperv-proto.h
@@ -63,6 +63,7 @@
 #define HV_CLUSTER_IPI_RECOMMENDED          (1u << 10)
 #define HV_EX_PROCESSOR_MASKS_RECOMMENDED   (1u << 11)
 #define HV_ENLIGHTENED_VMCS_RECOMMENDED     (1u << 14)
+#define HV_NO_NONARCH_CORESHARING           (1u << 18)
 
 /*
  * Basic virtualized MSRs
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 11b9c854b543..ef606e51babe 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -1208,6 +1208,16 @@ static int hyperv_handle_properties(CPUState *cs,
         }
     }
 
+    if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_ON) {
+        env->features[FEAT_HV_RECOMM_EAX] |= HV_NO_NONARCH_CORESHARING;
+    } else if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_AUTO) {
+        c = cpuid_find_entry(cpuid, HV_CPUID_ENLIGHTMENT_INFO, 0);
+        if (c) {
+            env->features[FEAT_HV_RECOMM_EAX] |=
+                c->eax & HV_NO_NONARCH_CORESHARING;
+        }
+    }
+
     /* Features */
     r = hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_RELAXED);
     r |= hv_cpuid_check_and_set(cs, cpuid, HYPERV_FEAT_VAPIC);
@@ -1321,6 +1331,7 @@ free:
 }
 
 static Error *hv_passthrough_mig_blocker;
+static Error *hv_no_nonarch_cs_mig_blocker;
 
 static int hyperv_init_vcpu(X86CPU *cpu)
 {
@@ -1340,6 +1351,21 @@ static int hyperv_init_vcpu(X86CPU *cpu)
         }
     }
 
+    if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_AUTO &&
+        hv_no_nonarch_cs_mig_blocker == NULL) {
+        error_setg(&hv_no_nonarch_cs_mig_blocker,
+                   "'hv-no-nonarch-coresharing=auto' CPU flag prevents migration"
+                   " use explicit 'hv-no-nonarch-coresharing=on' instead (but"
+                   " make sure SMT is disabled and/or that vCPUs are properly"
+                   " pinned)");
+        ret = migrate_add_blocker(hv_no_nonarch_cs_mig_blocker, &local_err);
+        if (local_err) {
+            error_report_err(local_err);
+            error_free(hv_no_nonarch_cs_mig_blocker);
+            return ret;
+        }
+    }
+
     if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX) && !hv_vpindex_settable) {
         /*
          * the kernel doesn't support setting vp_index; assert that its value
-- 
2.20.1

Re: [PATCH] i386/kvm: add NoNonArchitecturalCoreSharing Hyper-V enlightenment

[Paolo Bonzini]

On 18/10/19 18:39, Vitaly Kuznetsov wrote:
> Hyper-V TLFS specifies this enlightenment as:
> "NoNonArchitecturalCoreSharing - Indicates that a virtual processor will never
> share a physical core with another virtual processor, except for virtual
> processors that are reported as sibling SMT threads. This can be used as an
> optimization to avoid the performance overhead of STIBP".
> 
> However, STIBP is not the only implication. It was found that Hyper-V on
> KVM doesn't pass MD_CLEAR bit to its guests if it doesn't see
> NoNonArchitecturalCoreSharing bit.
> 
> KVM reports NoNonArchitecturalCoreSharing in KVM_GET_SUPPORTED_HV_CPUID to
> indicate that SMT on the host is impossible (not supported of forcefully
> disabled).
> 
> Implement NoNonArchitecturalCoreSharing support in QEMU as tristate:
> 'off' - the feature is disabled (default)
> 'on' - the feature is enabled. This is only safe if vCPUS are properly
>  pinned and correct topology is exposed. As CPU pinning is done outside
>  of QEMU the enablement decision will be made on a higher level.
> 'auto' - copy KVM setting. As during live migration SMT settings on the
> source and destination host may differ this requires us to add a migration
> blocker.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---
>  docs/hyperv.txt            | 13 +++++++++++++
>  target/i386/cpu.c          |  2 ++
>  target/i386/cpu.h          |  2 ++
>  target/i386/hyperv-proto.h |  1 +
>  target/i386/kvm.c          | 26 ++++++++++++++++++++++++++
>  5 files changed, 44 insertions(+)
> 
> diff --git a/docs/hyperv.txt b/docs/hyperv.txt
> index 8fdf25c8291c..6518b716a958 100644
> --- a/docs/hyperv.txt
> +++ b/docs/hyperv.txt
> @@ -184,6 +184,19 @@ enabled.
>  
>  Requires: hv-vpindex, hv-synic, hv-time, hv-stimer
>  
> +3.17. hv-no-nonarch-coresharing=on/off/auto
> +===========================================
> +This enlightenment tells guest OS that virtual processors will never share a
> +physical core unless they are reported as sibling SMT threads. This information
> +is required by Windows and Hyper-V guests to properly mitigate SMT related CPU
> +vulnerabilities.
> +When the option is set to 'auto' QEMU will enable the feature only when KVM
> +reports that non-architectural coresharing is impossible, this means that
> +hyper-threading is not supported or completely disabled on the host. This
> +setting also prevents migration as SMT settings on the destination may differ.
> +When the option is set to 'on' QEMU will always enable the feature, regardless
> +of host setup. To keep guests secure, this can only be used in conjunction with
> +exposing correct vCPU topology and vCPU pinning.
>  
>  4. Development features
>  ========================
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 44f1bbdcac76..4086c0a16767 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -6156,6 +6156,8 @@ static Property x86_cpu_properties[] = {
>                        HYPERV_FEAT_IPI, 0),
>      DEFINE_PROP_BIT64("hv-stimer-direct", X86CPU, hyperv_features,
>                        HYPERV_FEAT_STIMER_DIRECT, 0),
> +    DEFINE_PROP_ON_OFF_AUTO("hv-no-nonarch-coresharing", X86CPU,
> +                            hyperv_no_nonarch_cs, ON_OFF_AUTO_OFF),
>      DEFINE_PROP_BOOL("hv-passthrough", X86CPU, hyperv_passthrough, false),
>  
>      DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index eaa5395aa539..9f47c1e2a52d 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -24,6 +24,7 @@
>  #include "cpu-qom.h"
>  #include "hyperv-proto.h"
>  #include "exec/cpu-defs.h"
> +#include "qapi/qapi-types-common.h"
>  
>  /* The x86 has a strong memory model with some store-after-load re-ordering */
>  #define TCG_GUEST_DEFAULT_MO      (TCG_MO_ALL & ~TCG_MO_ST_LD)
> @@ -1563,6 +1564,7 @@ struct X86CPU {
>      bool hyperv_synic_kvm_only;
>      uint64_t hyperv_features;
>      bool hyperv_passthrough;
> +    OnOffAuto hyperv_no_nonarch_cs;
>  
>      bool check_cpuid;
>      bool enforce_cpuid;
> diff --git a/target/i386/hyperv-proto.h b/target/i386/hyperv-proto.h
> index cffac10b45dc..056a305be38c 100644
> --- a/target/i386/hyperv-proto.h
> +++ b/target/i386/hyperv-proto.h
> @@ -63,6 +63,7 @@
>  #define HV_CLUSTER_IPI_RECOMMENDED          (1u << 10)
>  #define HV_EX_PROCESSOR_MASKS_RECOMMENDED   (1u << 11)
>  #define HV_ENLIGHTENED_VMCS_RECOMMENDED     (1u << 14)
> +#define HV_NO_NONARCH_CORESHARING           (1u << 18)
>  
>  /*
>   * Basic virtualized MSRs
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 11b9c854b543..ef606e51babe 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -1208,6 +1208,16 @@ static int hyperv_handle_properties(CPUState *cs,
>          }
>      }
>  
> +    if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_ON) {
> +        env->features[FEAT_HV_RECOMM_EAX] |= HV_NO_NONARCH_CORESHARING;
> +    } else if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_AUTO) {

Do you want to make auto the default if "-cpu host,migratable=off"?  It
can be done on top so I started queueing this patch.

Paolo

Re: [PATCH] i386/kvm: add NoNonArchitecturalCoreSharing Hyper-V enlightenment

[Vitaly Kuznetsov]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 18/10/19 18:39, Vitaly Kuznetsov wrote:
>> Hyper-V TLFS specifies this enlightenment as:
>> "NoNonArchitecturalCoreSharing - Indicates that a virtual processor will never
>> share a physical core with another virtual processor, except for virtual
>> processors that are reported as sibling SMT threads. This can be used as an
>> optimization to avoid the performance overhead of STIBP".
>> 
>> However, STIBP is not the only implication. It was found that Hyper-V on
>> KVM doesn't pass MD_CLEAR bit to its guests if it doesn't see
>> NoNonArchitecturalCoreSharing bit.
>> 
>> KVM reports NoNonArchitecturalCoreSharing in KVM_GET_SUPPORTED_HV_CPUID to
>> indicate that SMT on the host is impossible (not supported of forcefully
>> disabled).
>> 
>> Implement NoNonArchitecturalCoreSharing support in QEMU as tristate:
>> 'off' - the feature is disabled (default)
>> 'on' - the feature is enabled. This is only safe if vCPUS are properly
>>  pinned and correct topology is exposed. As CPU pinning is done outside
>>  of QEMU the enablement decision will be made on a higher level.
>> 'auto' - copy KVM setting. As during live migration SMT settings on the
>> source and destination host may differ this requires us to add a migration
>> blocker.
>> 
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> ---
>>  docs/hyperv.txt            | 13 +++++++++++++
>>  target/i386/cpu.c          |  2 ++
>>  target/i386/cpu.h          |  2 ++
>>  target/i386/hyperv-proto.h |  1 +
>>  target/i386/kvm.c          | 26 ++++++++++++++++++++++++++
>>  5 files changed, 44 insertions(+)
>> 
>> diff --git a/docs/hyperv.txt b/docs/hyperv.txt
>> index 8fdf25c8291c..6518b716a958 100644
>> --- a/docs/hyperv.txt
>> +++ b/docs/hyperv.txt
>> @@ -184,6 +184,19 @@ enabled.
>>  
>>  Requires: hv-vpindex, hv-synic, hv-time, hv-stimer
>>  
>> +3.17. hv-no-nonarch-coresharing=on/off/auto
>> +===========================================
>> +This enlightenment tells guest OS that virtual processors will never share a
>> +physical core unless they are reported as sibling SMT threads. This information
>> +is required by Windows and Hyper-V guests to properly mitigate SMT related CPU
>> +vulnerabilities.
>> +When the option is set to 'auto' QEMU will enable the feature only when KVM
>> +reports that non-architectural coresharing is impossible, this means that
>> +hyper-threading is not supported or completely disabled on the host. This
>> +setting also prevents migration as SMT settings on the destination may differ.
>> +When the option is set to 'on' QEMU will always enable the feature, regardless
>> +of host setup. To keep guests secure, this can only be used in conjunction with
>> +exposing correct vCPU topology and vCPU pinning.
>>  
>>  4. Development features
>>  ========================
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index 44f1bbdcac76..4086c0a16767 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -6156,6 +6156,8 @@ static Property x86_cpu_properties[] = {
>>                        HYPERV_FEAT_IPI, 0),
>>      DEFINE_PROP_BIT64("hv-stimer-direct", X86CPU, hyperv_features,
>>                        HYPERV_FEAT_STIMER_DIRECT, 0),
>> +    DEFINE_PROP_ON_OFF_AUTO("hv-no-nonarch-coresharing", X86CPU,
>> +                            hyperv_no_nonarch_cs, ON_OFF_AUTO_OFF),
>>      DEFINE_PROP_BOOL("hv-passthrough", X86CPU, hyperv_passthrough, false),
>>  
>>      DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
>> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
>> index eaa5395aa539..9f47c1e2a52d 100644
>> --- a/target/i386/cpu.h
>> +++ b/target/i386/cpu.h
>> @@ -24,6 +24,7 @@
>>  #include "cpu-qom.h"
>>  #include "hyperv-proto.h"
>>  #include "exec/cpu-defs.h"
>> +#include "qapi/qapi-types-common.h"
>>  
>>  /* The x86 has a strong memory model with some store-after-load re-ordering */
>>  #define TCG_GUEST_DEFAULT_MO      (TCG_MO_ALL & ~TCG_MO_ST_LD)
>> @@ -1563,6 +1564,7 @@ struct X86CPU {
>>      bool hyperv_synic_kvm_only;
>>      uint64_t hyperv_features;
>>      bool hyperv_passthrough;
>> +    OnOffAuto hyperv_no_nonarch_cs;
>>  
>>      bool check_cpuid;
>>      bool enforce_cpuid;
>> diff --git a/target/i386/hyperv-proto.h b/target/i386/hyperv-proto.h
>> index cffac10b45dc..056a305be38c 100644
>> --- a/target/i386/hyperv-proto.h
>> +++ b/target/i386/hyperv-proto.h
>> @@ -63,6 +63,7 @@
>>  #define HV_CLUSTER_IPI_RECOMMENDED          (1u << 10)
>>  #define HV_EX_PROCESSOR_MASKS_RECOMMENDED   (1u << 11)
>>  #define HV_ENLIGHTENED_VMCS_RECOMMENDED     (1u << 14)
>> +#define HV_NO_NONARCH_CORESHARING           (1u << 18)
>>  
>>  /*
>>   * Basic virtualized MSRs
>> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
>> index 11b9c854b543..ef606e51babe 100644
>> --- a/target/i386/kvm.c
>> +++ b/target/i386/kvm.c
>> @@ -1208,6 +1208,16 @@ static int hyperv_handle_properties(CPUState *cs,
>>          }
>>      }
>>  
>> +    if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_ON) {
>> +        env->features[FEAT_HV_RECOMM_EAX] |= HV_NO_NONARCH_CORESHARING;
>> +    } else if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_AUTO) {
>
> Do you want to make auto the default if "-cpu host,migratable=off"?  It
> can be done on top so I started queueing this patch.

Hm, one thing is that CPUID 0x40000004 doesn't exist if no Hyper-V
enlightenments are passed so we'll probably have to modify your idea to
"-cpu host,migratable=off,+any-hyperv-enlightenment" but then the
question is how conservative are we, like if QEMU command line doesn't
change can new CPUID flags appear or not? And we'll probably need a way
to explicitly disable HV_NO_NONARCH_CORESHARING if needed.

-- 
Vitaly

Re: [PATCH] i386/kvm: add NoNonArchitecturalCoreSharing Hyper-V enlightenment

[Paolo Bonzini]

On 21/10/19 16:09, Vitaly Kuznetsov wrote:
>>> +    if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_ON) {
>>> +        env->features[FEAT_HV_RECOMM_EAX] |= HV_NO_NONARCH_CORESHARING;
>>> +    } else if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_AUTO) {
>> Do you want to make auto the default if "-cpu host,migratable=off"?  It
>> can be done on top so I started queueing this patch.
> Hm, one thing is that CPUID 0x40000004 doesn't exist if no Hyper-V
> enlightenments are passed so we'll probably have to modify your idea to
> "-cpu host,migratable=off,+any-hyperv-enlightenment" but then the
> question is how conservative are we, like if QEMU command line doesn't
> change can new CPUID flags appear or not? And we'll probably need a way
> to explicitly disable HV_NO_NONARCH_CORESHARING if needed.

I would defer to Eduardo on whether "migratable=off" would allow adding
new CPUID flags.  The follow-up question however is whether we would
benefit from a "+hyperv" option that enables all known Hyper-V
enlightenment for a given machine type.

Paolo

Re: [PATCH] i386/kvm: add NoNonArchitecturalCoreSharing Hyper-V enlightenment

[Eduardo Habkost]

On Mon, Oct 21, 2019 at 06:26:14PM +0200, Paolo Bonzini wrote:
> On 21/10/19 16:09, Vitaly Kuznetsov wrote:
> >>> +    if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_ON) {
> >>> +        env->features[FEAT_HV_RECOMM_EAX] |= HV_NO_NONARCH_CORESHARING;
> >>> +    } else if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_AUTO) {
> >> Do you want to make auto the default if "-cpu host,migratable=off"?  It
> >> can be done on top so I started queueing this patch.
> > Hm, one thing is that CPUID 0x40000004 doesn't exist if no Hyper-V
> > enlightenments are passed so we'll probably have to modify your idea to
> > "-cpu host,migratable=off,+any-hyperv-enlightenment" but then the
> > question is how conservative are we, like if QEMU command line doesn't
> > change can new CPUID flags appear or not? And we'll probably need a way
> > to explicitly disable HV_NO_NONARCH_CORESHARING if needed.
> 
> I would defer to Eduardo on whether "migratable=off" would allow adding
> new CPUID flags.  The follow-up question however is whether we would
> benefit from a "+hyperv" option that enables all known Hyper-V
> enlightenment for a given machine type.

I'm not sure what "adding new CPUID flags" means exactly, but on
both cases, the answer is yes:

If you mean having new flags appear with the same QEMU command
line, this is 100% OK with "-cpu host".  Doubly so with
"migratable=off".  "-cpu host" doesn't guarantee a stable guest
ABI, and migratable=off doesn't guarantee the ability to live
migrate.

If you just mean the ability to write "-cpu
host,migratable=off,+some-extra-flag", that's OK too.

I would try to make "-cpu host,migratable=off" enable all
features out of the box (because users probably expect that).
But we you have a compelling reason to not enable the hyperv
flags by default (do we?), it's OK to require something like
"-cpu host,...,+hyperv".

-- 
Eduardo

Re: [PATCH] i386/kvm: add NoNonArchitecturalCoreSharing Hyper-V enlightenment

[Vitaly Kuznetsov]

Eduardo Habkost <ehabkost@redhat.com> writes:

> On Mon, Oct 21, 2019 at 06:26:14PM +0200, Paolo Bonzini wrote:
>> On 21/10/19 16:09, Vitaly Kuznetsov wrote:
>> >>> +    if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_ON) {
>> >>> +        env->features[FEAT_HV_RECOMM_EAX] |= HV_NO_NONARCH_CORESHARING;
>> >>> +    } else if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_AUTO) {
>> >> Do you want to make auto the default if "-cpu host,migratable=off"?  It
>> >> can be done on top so I started queueing this patch.
>> > Hm, one thing is that CPUID 0x40000004 doesn't exist if no Hyper-V
>> > enlightenments are passed so we'll probably have to modify your idea to
>> > "-cpu host,migratable=off,+any-hyperv-enlightenment" but then the
>> > question is how conservative are we, like if QEMU command line doesn't
>> > change can new CPUID flags appear or not? And we'll probably need a way
>> > to explicitly disable HV_NO_NONARCH_CORESHARING if needed.
>> 
>> I would defer to Eduardo on whether "migratable=off" would allow adding
>> new CPUID flags.  The follow-up question however is whether we would
>> benefit from a "+hyperv" option that enables all known Hyper-V
>> enlightenment for a given machine type.
>
> I'm not sure what "adding new CPUID flags" means exactly, but on
> both cases, the answer is yes:
>
> If you mean having new flags appear with the same QEMU command
> line, this is 100% OK with "-cpu host".  Doubly so with
> "migratable=off".  "-cpu host" doesn't guarantee a stable guest
> ABI, and migratable=off doesn't guarantee the ability to live
> migrate.
>
> If you just mean the ability to write "-cpu
> host,migratable=off,+some-extra-flag", that's OK too.
>
> I would try to make "-cpu host,migratable=off" enable all
> features out of the box (because users probably expect that).
> But we you have a compelling reason to not enable the hyperv
> flags by default (do we?), it's OK to require something like
> "-cpu host,...,+hyperv".

I'm not sure if the reason is compelling enough but I remember some
Linux tools were only looking at the first hypervisor signature and
reporting that we're now running on Hyper-V. Also, more features you
enable larger the atack surface...

Actually, we already '-cpu host,hv_passthrough' option which implies
'migratable=off', not sure if another one is needed.

-- 
Vitaly

Re: [PATCH] i386/kvm: add NoNonArchitecturalCoreSharing Hyper-V enlightenment

[Eduardo Habkost]

On Wed, Oct 23, 2019 at 01:16:38PM +0200, Vitaly Kuznetsov wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > On Mon, Oct 21, 2019 at 06:26:14PM +0200, Paolo Bonzini wrote:
> >> On 21/10/19 16:09, Vitaly Kuznetsov wrote:
> >> >>> +    if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_ON) {
> >> >>> +        env->features[FEAT_HV_RECOMM_EAX] |= HV_NO_NONARCH_CORESHARING;
> >> >>> +    } else if (cpu->hyperv_no_nonarch_cs == ON_OFF_AUTO_AUTO) {
> >> >> Do you want to make auto the default if "-cpu host,migratable=off"?  It
> >> >> can be done on top so I started queueing this patch.
> >> > Hm, one thing is that CPUID 0x40000004 doesn't exist if no Hyper-V
> >> > enlightenments are passed so we'll probably have to modify your idea to
> >> > "-cpu host,migratable=off,+any-hyperv-enlightenment" but then the
> >> > question is how conservative are we, like if QEMU command line doesn't
> >> > change can new CPUID flags appear or not? And we'll probably need a way
> >> > to explicitly disable HV_NO_NONARCH_CORESHARING if needed.
> >> 
> >> I would defer to Eduardo on whether "migratable=off" would allow adding
> >> new CPUID flags.  The follow-up question however is whether we would
> >> benefit from a "+hyperv" option that enables all known Hyper-V
> >> enlightenment for a given machine type.
> >
> > I'm not sure what "adding new CPUID flags" means exactly, but on
> > both cases, the answer is yes:
> >
> > If you mean having new flags appear with the same QEMU command
> > line, this is 100% OK with "-cpu host".  Doubly so with
> > "migratable=off".  "-cpu host" doesn't guarantee a stable guest
> > ABI, and migratable=off doesn't guarantee the ability to live
> > migrate.
> >
> > If you just mean the ability to write "-cpu
> > host,migratable=off,+some-extra-flag", that's OK too.
> >
> > I would try to make "-cpu host,migratable=off" enable all
> > features out of the box (because users probably expect that).
> > But we you have a compelling reason to not enable the hyperv
> > flags by default (do we?), it's OK to require something like
> > "-cpu host,...,+hyperv".
> 
> I'm not sure if the reason is compelling enough but I remember some
> Linux tools were only looking at the first hypervisor signature and
> reporting that we're now running on Hyper-V. Also, more features you
> enable larger the atack surface...
> 
> Actually, we already '-cpu host,hv_passthrough' option which implies
> 'migratable=off', not sure if another one is needed.

So, if I understood correctly, Paolo's "+hyperv" suggestion above
is already implemented by "hv_passthrough"?  Sounds good enough
to me.

-- 
Eduardo