Re: [Qemu-devel] [PATCH] target-i386: Remove AMD feature flag aliases from Opteron models

[Igor Mammedov <imammedo@redhat.com>]

On Fri, 13 Jan 2017 17:00:57 -0200
Eduardo Habkost <ehabkost@redhat.com> wrote:

> When CPU vendor is set to AMD, the AMD feature alias bits on
> CPUID[0x80000001].EDX are already automatically copied from CPUID[1].EDX
> on x86_cpu_realizefn(). When CPU vendor is Intel, those bits are
> reserved and should be zero. On either case, those bits shouldn't be set
> in the CPU model table.
> 
> Commit 726a8ff68677d8d5fba17eb0ffb85076bfb598dc removed those
> bits from most CPU models, but the Opteron_* entries still have
> them. Remove the alias bits from Opteron_* too.
> 
> Add an assert() to x86_register_cpudef_type() to ensure we don't
> make the same mistake again.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  target/i386/cpu.c | 46 ++++++++++++----------------------------------
>  1 file changed, 12 insertions(+), 34 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index a149c8dc42..af726a8fd0 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -1339,12 +1339,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>          .features[FEAT_1_ECX] =
>              CPUID_EXT_SSE3,
>          .features[FEAT_8000_0001_EDX] =
> -            CPUID_EXT2_LM | 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,
> +            CPUID_EXT2_LM | CPUID_EXT2_NX | CPUID_EXT2_SYSCALL,
>          .xlevel = 0x80000008,
>          .model_id = "AMD Opteron 240 (Gen 1 Class Opteron)",
>      },
> @@ -1365,13 +1360,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>              CPUID_EXT_CX16 | CPUID_EXT_SSE3,
>          /* Missing: CPUID_EXT2_RDTSCP */
>          .features[FEAT_8000_0001_EDX] =
> -            CPUID_EXT2_LM | 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,
> +            CPUID_EXT2_LM | CPUID_EXT2_NX | CPUID_EXT2_SYSCALL,
>          .features[FEAT_8000_0001_ECX] =
>              CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM,
>          .xlevel = 0x80000008,
> @@ -1395,13 +1384,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>              CPUID_EXT_SSE3,
>          /* Missing: CPUID_EXT2_RDTSCP */
>          .features[FEAT_8000_0001_EDX] =
> -            CPUID_EXT2_LM | 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,
> +            CPUID_EXT2_LM | CPUID_EXT2_NX | CPUID_EXT2_SYSCALL,
>          .features[FEAT_8000_0001_ECX] =
>              CPUID_EXT3_MISALIGNSSE | CPUID_EXT3_SSE4A |
>              CPUID_EXT3_ABM | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM,
> @@ -1428,13 +1411,8 @@ static X86CPUDefinition builtin_x86_defs[] = {
>              CPUID_EXT_SSE3,
>          /* Missing: CPUID_EXT2_RDTSCP */
>          .features[FEAT_8000_0001_EDX] =
> -            CPUID_EXT2_LM |
> -            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,
> +            CPUID_EXT2_LM | CPUID_EXT2_PDPE1GB | CPUID_EXT2_NX |
> +            CPUID_EXT2_SYSCALL,
>          .features[FEAT_8000_0001_ECX] =
>              CPUID_EXT3_FMA4 | CPUID_EXT3_XOP |
>              CPUID_EXT3_3DNOWPREFETCH | CPUID_EXT3_MISALIGNSSE |
> @@ -1464,13 +1442,8 @@ static X86CPUDefinition builtin_x86_defs[] = {
>              CPUID_EXT_SSSE3 | CPUID_EXT_PCLMULQDQ | CPUID_EXT_SSE3,
>          /* Missing: CPUID_EXT2_RDTSCP */
>          .features[FEAT_8000_0001_EDX] =
> -            CPUID_EXT2_LM |
> -            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,
> +            CPUID_EXT2_LM | CPUID_EXT2_PDPE1GB | CPUID_EXT2_NX |
> +            CPUID_EXT2_SYSCALL,
>          .features[FEAT_8000_0001_ECX] =
>              CPUID_EXT3_TBM | CPUID_EXT3_FMA4 | CPUID_EXT3_XOP |
>              CPUID_EXT3_3DNOWPREFETCH | CPUID_EXT3_MISALIGNSSE |
> @@ -2395,6 +2368,11 @@ static void x86_register_cpudef_type(X86CPUDefinition *def)
>          .class_data = def,
>      };
>  
> +    /* AMD aliases are handled at runtime based on CPUID vendor, so
> +     * they shouldn't be set on the CPU model table.
> +     */
> +    assert(!(def->features[FEAT_8000_0001_EDX] & CPUID_EXT2_AMD_ALIASES));
> +
>      type_register(&ti);
>      g_free(typename);
>  }