Re: [PATCH] KVM: SVM: fix trashing of MSR_TSC_AUX

[Paolo Bonzini <pbonzini@redhat.com>]

> On Thu, Jul 07, 2016 at 03:16:21PM +0200, Paolo Bonzini wrote:
> > Eduardo is the one to answer, but usually we add features to QEMU
> > before the processors are released (typically as soon as KVM supports
> > them).  So with a new enough QEMU this in theory should not be
> > necessary.
> > 
> > Adding a new feature that's not in a CPU model and that's not
> > associated to new state is really trivial:
> 
> Cool.
> 
> Btw, how about something like this?
> 
> Specifically, I'd like to test RAS features on the new upcoming AMD
> Zen CPU and I've defined one from the stuff we know so far from kernel
> patches.

It looks good from skimming it---but again this isn't quite my
territory.

Paolo

> The "filter=off" thing I've added in case I want to disable
> x86_cpu_filter_features() but it works just fine without it when I boot
> with -cpu Zen. So I can remove it too.
> 
> Would something like that be acceptable?
> 
> We can continue improving on this as features become known and even
> implement some functionality in qemu/kvm as time allows.
> 
> ---
> From: Borislav Petkov <bp@suse.de>
> Date: Tue, 5 Jul 2016 16:12:18 +0200
> Subject: [PATCH] Zen emu: first working version
> 
> Boot with "-c Zen,filter=off" to disable CPUID bits filtering.
> 
> Signed-off-by: Borislav Petkov <bp@suse.de>
> ---
>  target-i386/cpu.c | 60
>  ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  target-i386/cpu.h |  7 +++++++
>  2 files changed, 66 insertions(+), 1 deletion(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 3bd3cfc3ad16..cc9c97457387 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -307,6 +307,17 @@ static const char *cpuid_6_feature_name[] = {
>      NULL, NULL, NULL, NULL,
>  };
>  
> +static const char *smca_feature_name[] = {
> +    "overflow_recov", "succor", NULL, "smca",
> +    NULL, NULL, NULL, NULL,
> +    NULL, NULL, NULL, NULL,
> +    NULL, NULL, NULL, NULL,
> +    NULL, NULL, NULL, NULL,
> +    NULL, NULL, NULL, NULL,
> +    NULL, NULL, NULL, NULL,
> +    NULL, NULL, NULL, NULL,
> +};
> +
>  #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
>  #define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \
>            CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_MMX | CPUID_APIC)
> @@ -449,6 +460,11 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS]
> = {
>          .cpuid_eax = 6, .cpuid_reg = R_EAX,
>          .tcg_features = TCG_6_EAX_FEATURES,
>      },
> +    [FEAT_8000_0007_EBX] = {
> +	.feat_names = smca_feature_name,
> +	.cpuid_eax = 0x80000007,
> +	.cpuid_reg = R_EBX,
> +    },
>  };
>  
>  typedef struct X86RegisterInfo32 {
> @@ -1449,6 +1465,44 @@ static X86CPUDefinition builtin_x86_defs[] = {
>          .xlevel = 0x8000001A,
>          .model_id = "AMD Opteron 63xx class CPU",
>      },
> +    {
> +        .name = "Zen",
> +        .level = 0xd,
> +        .vendor = CPUID_VENDOR_AMD,
> +        .family = 23,
> +        .model = 0,
> +        .stepping = 0,
> +        .features[FEAT_1_EDX] =
> +            CPUID_VME | CPUID_SSE2 | CPUID_SSE | CPUID_FXSR | CPUID_MMX |
> +            CPUID_CLFLUSH | CPUID_PSE36 | CPUID_PAT | CPUID_CMOV | CPUID_MCA
> |
> +            CPUID_PGE | CPUID_MTRR | CPUID_SEP | CPUID_APIC | CPUID_CX8 |
> +            CPUID_MCE | CPUID_PAE | CPUID_MSR | CPUID_TSC | CPUID_PSE |
> +            CPUID_DE | CPUID_FP87,
> +        .features[FEAT_1_ECX] =
> +            CPUID_EXT_F16C | CPUID_EXT_AVX | CPUID_EXT_XSAVE |
> +            CPUID_EXT_AES | CPUID_EXT_POPCNT | CPUID_EXT_SSE42 |
> +            CPUID_EXT_SSE41 | CPUID_EXT_CX16 | CPUID_EXT_FMA |
> +            CPUID_EXT_SSSE3 | CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3,
> +        .features[FEAT_8000_0001_EDX] =
> +            CPUID_EXT2_LM | CPUID_EXT2_RDTSCP |
> +            CPUID_EXT2_PDPE1GB | CPUID_EXT2_FXSR | CPUID_EXT2_MMX |
> +            CPUID_EXT2_NX | CPUID_EXT2_PSE36 | CPUID_EXT2_PAT |
> +            CPUID_EXT2_CMOV | CPUID_EXT2_MCA | CPUID_EXT2_PGE |
> +            CPUID_EXT2_MTRR | CPUID_EXT2_SYSCALL | CPUID_EXT2_APIC |
> +            CPUID_EXT2_CX8 | CPUID_EXT2_MCE | CPUID_EXT2_PAE |
> CPUID_EXT2_MSR |
> +            CPUID_EXT2_TSC | CPUID_EXT2_PSE | CPUID_EXT2_DE |
> CPUID_EXT2_FPU,
> +        .features[FEAT_8000_0001_ECX] =
> +            CPUID_EXT3_TBM | CPUID_EXT3_FMA4 | CPUID_EXT3_XOP |
> +            CPUID_EXT3_3DNOWPREFETCH | CPUID_EXT3_MISALIGNSSE |
> +            CPUID_EXT3_SSE4A | CPUID_EXT3_ABM | CPUID_EXT3_SVM |
> +            CPUID_EXT3_LAHF_LM,
> +        /* no xsaveopt! */
> +        .features[FEAT_8000_0007_EBX] =
> +            CPUID_OVERFLOW_RECOV | CPUID_SUCCOR | CPUID_SMCA,
> +        .xlevel = 0x8000001A,
> +        .model_id = "AMD Zen CPU",
> +    },
> +
>  };
>  
>  typedef struct PropValue {
> @@ -2118,6 +2172,9 @@ static int x86_cpu_filter_features(X86CPU *cpu)
>      FeatureWord w;
>      int rv = 0;
>  
> +    if (!cpu->filter_cpuid)
> +        return 0;
> +
>      for (w = 0; w < FEATURE_WORDS; w++) {
>          uint32_t host_feat =
>              x86_cpu_get_supported_feature_word(w, cpu->migratable);
> @@ -2596,7 +2653,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index,
> uint32_t count,
>          break;
>      case 0x80000007:
>          *eax = 0;
> -        *ebx = 0;
> +        *ebx = env->features[FEAT_8000_0007_EBX];
>          *ecx = 0;
>          *edx = env->features[FEAT_8000_0007_EDX];
>          break;
> @@ -3256,6 +3313,7 @@ static Property x86_cpu_properties[] = {
>      DEFINE_PROP_BOOL("hv-stimer", X86CPU, hyperv_stimer, false),
>      DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
>      DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
> +    DEFINE_PROP_BOOL("filter", X86CPU, filter_cpuid, false),
>      DEFINE_PROP_BOOL("kvm", X86CPU, expose_kvm, true),
>      DEFINE_PROP_UINT32("level", X86CPU, env.cpuid_level, 0),
>      DEFINE_PROP_UINT32("xlevel", X86CPU, env.cpuid_xlevel, 0),
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index 474b0b937d71..258c1b261cd2 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -443,6 +443,7 @@ typedef enum FeatureWord {
>      FEAT_SVM,           /* CPUID[8000_000A].EDX */
>      FEAT_XSAVE,         /* CPUID[EAX=0xd,ECX=1].EAX */
>      FEAT_6_EAX,         /* CPUID[6].EAX */
> +    FEAT_8000_0007_EBX, /* CPUID[8000_0007].EBX */
>      FEATURE_WORDS,
>  } FeatureWord;
>  
> @@ -620,6 +621,11 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
>  #define CPUID_APM_INVTSC       (1U << 8)
>  
>  #define CPUID_VENDOR_SZ      12
> +/* CPUID[0x80000007].EBX flags: */
> +#define CPUID_OVERFLOW_RECOV   (1U << 0) /* MCA overflow recovery support */
> +#define CPUID_SUCCOR	       (1U << 1) /* Uncorrectable error containment and
> recovery */
> +#define CPUID_SMCA	       (1U << 3) /* Scalable MCA */
> +
>  
>  #define CPUID_VENDOR_INTEL_1 0x756e6547 /* "Genu" */
>  #define CPUID_VENDOR_INTEL_2 0x49656e69 /* "ineI" */
> @@ -1160,6 +1166,7 @@ struct X86CPU {
>      bool hyperv_stimer;
>      bool check_cpuid;
>      bool enforce_cpuid;
> +    bool filter_cpuid;
>      bool expose_kvm;
>      bool migratable;
>      bool host_features;
> --
> 2.7.3
> 
> --
> Regards/Gruss,
>     Boris.
> 
> ECO tip #101: Trim your mails when you reply.
>