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

[Vitaly Kuznetsov <vkuznets@redhat.com>]

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