Re: [Qemu-devel] [PATCH 2/2] x86: Fix Opteron xlevels

[Eduardo Habkost]

On Wed, Apr 22, 2015 at 12:38:11AM +0200, Alexander Graf wrote:
> The AMD Opteron family has different xlevel levels depending on the
> generation. I looked up Gen1, Gen2 and Gen3 hardware and adapted the
> levels according to real silicon.
> 
> The reason this came up is that there is a sanity check in KVM making
> sure that SVM is only used when xlevel is high enough. Using real
> hardware levels, they now are.
> 

As noted in the reply to the previous patch: if you increase xlevel you are now
reporting new CPUID leaves as available. I still don't see any reason to start
reporting new CPUID leaves as available if they are returning useless data
(that doesn't match real hardware).

For reference, here are the new CPUID leaves you are introducing (and will
return all-zeroes):

For Opteron_G[123]:
* CPUID Fn8000_0009 Reserved
* CPUID Fn8000_000A SVM Revision and Feature Identification (supported by QEMU)
* CPUID Fn8000_00[18:0B] Reserved

For Opteron_G3:
* CPUID Fn8000_0019_EAX TLB 1GB Page Identifiers
* CPUID Fn8000_0019_EBX TLB 1GB Page Identifiers
* CPUID Fn8000_0019_E[D,C]X Reserved
* CPUID Fn8000_001A_EAX Performance Optimization Identifiers
* CPUID Fn8000_001A_E[D,C,B]X Reserved

The reserved bits look safe, but we can't be 100% sure unless we confirm that
real hardware is returning zeroes on all the reserved bits. If real hardware
return some data, we don't know what exactly that data means (and what
all-zeroes would mean) until AMD documents it.

For L1 and L2 1GB page TLB info, all-zeroes means "TLB disabled".

The Performance Optimization Identifier leaf will now be available, and will
return 0 on the following bits:

* "MOVU: MOVU SSE (multimedia) instructions are more efficient and should be
  preferred to SSE (multimedia) MOVL/MOVH. MOVUPS is more efficient than
  MOVLPS/MOVHPS. MOVUPD is more efficient than MOVLPD/MOVHP";
* "FP128: 128-bit SSE (multimedia) instructions are executed with full-width
  internal operations and pipelines rather than decomposing them into internal
  64-bit suboperations. This may impact how soft- ware performs instruction
  selection and scheduling."

There's a simple way to avoid exposing additional useless information to
guests: just return 0x8000000A on xlevel. 0x8000000A is the CPUID leaf you need
for SVM, the others are unrelated to SVM.

> Reported-by: Bernhard M. Wiedemann <address@hidden>
> Signed-off-by: Alexander Graf <address@hidden>
> 
> ---
> 
> v1 -> v2:
> 
>   - add compat handling for machine types < 2.4
> ---
>  include/hw/i386/pc.h | 13 +++++++++++++
>  target-i386/cpu.c    |  6 +++---
>  2 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 05ac5f9..c3cfe18 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -296,6 +296,19 @@ int e820_get_num_entries(void);
>  bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>  
>  #define PC_COMPAT_2_3 \
> +        {\
> +            .driver   = "Opteron_G1-" TYPE_X86_CPU,\
> +            .property = "xlevel",\
> +            .value    = stringify(0x80000008),\
> +        },{\
> +            .driver   = "Opteron_G2-" TYPE_X86_CPU,\
> +            .property = "xlevel",\
> +            .value    = stringify(0x80000008),\
> +        },{\
> +            .driver   = "Opteron_G3-" TYPE_X86_CPU,\
> +            .property = "xlevel",\
> +            .value    = stringify(0x80000008),\
> +        }
>  
>  #define PC_COMPAT_2_0 \
>          PC_COMPAT_2_3, \
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 03b33cf..d1b1b8c 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1234,7 +1234,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>              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,
> -        .xlevel = 0x80000008,
> +        .xlevel = 0x80000018,
>          .model_id = "AMD Opteron 240 (Gen 1 Class Opteron)",
>      },
>      {
> @@ -1262,7 +1262,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>              CPUID_EXT2_DE | CPUID_EXT2_FPU,
>          .features[FEAT_8000_0001_ECX] =
>              CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM,
> -        .xlevel = 0x80000008,
> +        .xlevel = 0x80000018,
>          .model_id = "AMD Opteron 22xx (Gen 2 Class Opteron)",
>      },
>      {
> @@ -1292,7 +1292,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
>          .features[FEAT_8000_0001_ECX] =
>              CPUID_EXT3_MISALIGNSSE | CPUID_EXT3_SSE4A |
>              CPUID_EXT3_ABM | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM,
> -        .xlevel = 0x80000008,
> +        .xlevel = 0x8000001A,
>          .model_id = "AMD Opteron 23xx (Gen 3 Class Opteron)",
>      },
>      {
> -- 
> 1.7.12.4
> 

-- 
Eduardo