Re: [Qemu-devel] [PATCH V2 1/3] target-i386: add pkeys support for cpuid handling

[Paolo Bonzini]

On 16/11/2015 08:52, Huaitong Han wrote:
> This patch adds pkeys support for cpuid handling.
> 
> Signed-off-by: Huaitong Han <address@hidden>
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 4d1b085..2ff73ee 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -264,6 +264,17 @@ static const char *cpuid_7_0_ebx_feature_name[] = {
>      NULL, NULL, "avx512pf", "avx512er", "avx512cd", NULL, NULL, NULL,
>  };
>  
> +static const char *cpuid_7_0_ecx_feature_name[] = {
> +    NULL, NULL, "pku", "ospke",

These strings are at index 2 and 3, while PKU and OSPKE are respectively
bit 3 and 4 in CPUID[EAX=7,ECX=0].ECX.

Otherwise okay.  The other two patches are fine as well.

Paolo

> +    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,
> +};
> +
>  static const char *cpuid_apm_edx_feature_name[] = {
>      NULL, NULL, NULL, NULL,
>      NULL, NULL, NULL, NULL,
> @@ -351,6 +362,7 @@ static const char *cpuid_6_feature_name[] = {
>            CPUID_7_0_EBX_FSGSBASE, CPUID_7_0_EBX_HLE, CPUID_7_0_EBX_AVX2,
>            CPUID_7_0_EBX_ERMS, CPUID_7_0_EBX_INVPCID, CPUID_7_0_EBX_RTM,
>            CPUID_7_0_EBX_RDSEED */
> +#define TCG_7_0_ECX_FEATURES 0
>  #define TCG_APM_FEATURES 0
>  #define TCG_6_EAX_FEATURES CPUID_6_EAX_ARAT
>  
> @@ -408,6 +420,13 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] 
> = {
>          .cpuid_reg = R_EBX,
>          .tcg_features = TCG_7_0_EBX_FEATURES,
>      },
> +    [FEAT_7_0_ECX] = {
> +        .feat_names = cpuid_7_0_ecx_feature_name,
> +        .cpuid_eax = 7,
> +        .cpuid_needs_ecx = true, .cpuid_ecx = 0,
> +        .cpuid_reg = R_ECX,
> +        .tcg_features = TCG_7_0_ECX_FEATURES,
> +    },
>      [FEAT_8000_0007_EDX] = {
>          .feat_names = cpuid_apm_edx_feature_name,
>          .cpuid_eax = 0x80000007,
> @@ -2401,7 +2420,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
> uint32_t count,
>          if (count == 0) {
>              *eax = 0; /* Maximum ECX value for sub-leaves */
>              *ebx = env->features[FEAT_7_0_EBX]; /* Feature flags */
> -            *ecx = 0; /* Reserved */
> +            *ecx = env->features[FEAT_7_0_ECX]; /* Feature flags */
>              *edx = 0; /* Reserved */
>          } else {
>              *eax = 0;
> diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> index ead2832..c2e7501 100644
> --- a/target-i386/cpu.h
> +++ b/target-i386/cpu.h
> @@ -408,6 +408,7 @@ typedef enum FeatureWord {
>      FEAT_1_EDX,         /* CPUID[1].EDX */
>      FEAT_1_ECX,         /* CPUID[1].ECX */
>      FEAT_7_0_EBX,       /* CPUID[EAX=7,ECX=0].EBX */
> +    FEAT_7_0_ECX,       /* CPUID[EAX=7,ECX=0].ECX */
>      FEAT_8000_0001_EDX, /* CPUID[8000_0001].EDX */
>      FEAT_8000_0001_ECX, /* CPUID[8000_0001].ECX */
>      FEAT_8000_0007_EDX, /* CPUID[8000_0007].EDX */
> @@ -576,6 +577,9 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
>  #define CPUID_7_0_EBX_AVX512ER (1U << 27) /* AVX-512 Exponential and 
> Reciprocal */
>  #define CPUID_7_0_EBX_AVX512CD (1U << 28) /* AVX-512 Conflict Detection */
>  
> +#define CPUID_7_0_ECX_PKU      (1U << 3)
> +#define CPUID_7_0_ECX_OSPKE    (1U << 4)
> +
>  #define CPUID_XSAVE_XSAVEOPT   (1U << 0)
>  #define CPUID_XSAVE_XSAVEC     (1U << 1)
>  #define CPUID_XSAVE_XGETBV1    (1U << 2)
>