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 <address@hidden>
> ---
>  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