[Qemu-devel] [PATCH] target-i386/cpu: Add new EYPC CPU model

Brijesh Singh posted 1 patch 6 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170814155217.4898-1-brijesh.singh@amd.com
Test FreeBSD passed
Test checkpatch passed
Test docker passed
Test s390x passed
target/i386/cpu.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 44 insertions(+)
[Qemu-devel] [PATCH] target-i386/cpu: Add new EYPC CPU model
Posted by Brijesh Singh 6 years, 7 months ago
Add a new base CPU model called 'EPYC' to model processors from AMD EPYC
family (which includes EPYC 76xx,75xx,74xx,73xx and 72xx).

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
 target/i386/cpu.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index ddc45ab..ed1708b 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1522,6 +1522,50 @@ static X86CPUDefinition builtin_x86_defs[] = {
         .xlevel = 0x8000001A,
         .model_id = "AMD Opteron 63xx class CPU",
     },
+    {
+        .name = "EPYC",
+        .level = 0xd,
+        .vendor = CPUID_VENDOR_AMD,
+        .family = 23,
+        .model = 1,
+        .stepping = 2,
+        .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_RDRAND | CPUID_EXT_F16C | CPUID_EXT_AVX |
+            CPUID_EXT_XSAVE | CPUID_EXT_MOVBE | 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_FFXSR | CPUID_EXT2_MMXEXT | CPUID_EXT2_NX |
+            CPUID_EXT2_SYSCALL,
+        .features[FEAT_8000_0001_ECX] =
+            CPUID_EXT3_OSVW | CPUID_EXT3_3DNOWPREFETCH |
+            CPUID_EXT3_MISALIGNSSE | CPUID_EXT3_SSE4A | CPUID_EXT3_ABM |
+            CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM,
+        .features[FEAT_7_0_EBX] =
+            CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_AVX2 |
+            CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_RDSEED |
+            CPUID_7_0_EBX_ADX | CPUID_7_0_EBX_SMAP | CPUID_7_0_EBX_CLFLUSHOPT |
+            CPUID_7_0_EBX_SHA_NI,
+        /* Missing: XSAVES (not supported by some Linux versions,
+         * including v4.1 to v4.12).
+         * KVM doesn't yet expose any XSAVES state save component.
+         */
+        .features[FEAT_XSAVE] =
+            CPUID_XSAVE_XSAVEOPT | CPUID_XSAVE_XSAVEC |
+            CPUID_XSAVE_XGETBV1,
+        .features[FEAT_6_EAX] =
+            CPUID_6_EAX_ARAT,
+        .xlevel = 0x8000001F,
+        .model_id = "AMD EYPC Processor",
+    },
 };
 
 typedef struct PropValue {
-- 
2.9.4


Re: [Qemu-devel] [PATCH] target-i386/cpu: Add new EYPC CPU model
Posted by Eduardo Habkost 6 years, 7 months ago
Hi,

Thanks for the patch.

On Mon, Aug 14, 2017 at 10:52:17AM -0500, Brijesh Singh wrote:
> Add a new base CPU model called 'EPYC' to model processors from AMD EPYC
> family (which includes EPYC 76xx,75xx,74xx,73xx and 72xx).

I suggest enumerating in the commit message which features were
added to the CPU model in comparison to Opteron_G5.

> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
>  target/i386/cpu.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index ddc45ab..ed1708b 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -1522,6 +1522,50 @@ static X86CPUDefinition builtin_x86_defs[] = {
>          .xlevel = 0x8000001A,
>          .model_id = "AMD Opteron 63xx class CPU",
>      },
> +    {
> +        .name = "EPYC",
> +        .level = 0xd,
> +        .vendor = CPUID_VENDOR_AMD,
> +        .family = 23,
> +        .model = 1,
> +        .stepping = 2,
[...]
> +        /* Missing: XSAVES (not supported by some Linux versions,
> +         * including v4.1 to v4.12).
> +         * KVM doesn't yet expose any XSAVES state save component.
> +         */

Do you know which supervisor state components are available in
EPYC CPUs?  Do you have a pointer to public AMD documentation
about XSAVES?


> +        .features[FEAT_XSAVE] =
> +            CPUID_XSAVE_XSAVEOPT | CPUID_XSAVE_XSAVEC |
> +            CPUID_XSAVE_XGETBV1,
> +        .features[FEAT_6_EAX] =
> +            CPUID_6_EAX_ARAT,
> +        .xlevel = 0x8000001F,

All CPUID leaves from 0x8000000B to 0x8000001F return all-zeroes
today.  If we set xlevel to 0x8000001F before we actually
implement those CPUID leaves, we will be forced to add extra
machine-type compat code when we finally implement them.

I suggest setting it to 0x8000000A, and increasing it only after
we actually implement the new CPUID leaves.


> +        .model_id = "AMD EYPC Processor",
> +    },
>  };
>  
>  typedef struct PropValue {
> -- 
> 2.9.4
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH] target-i386/cpu: Add new EYPC CPU model
Posted by Brijesh Singh 6 years, 7 months ago
Hi Eduardo,


On 08/15/2017 06:35 AM, Eduardo Habkost wrote:
> Hi,
> 
> Thanks for the patch.
> 
> On Mon, Aug 14, 2017 at 10:52:17AM -0500, Brijesh Singh wrote:
>> Add a new base CPU model called 'EPYC' to model processors from AMD EPYC
>> family (which includes EPYC 76xx,75xx,74xx,73xx and 72xx).
> 
> I suggest enumerating in the commit message which features were
> added to the CPU model in comparison to Opteron_G5.
> 

Will do.

[snip]

>> +        /* Missing: XSAVES (not supported by some Linux versions,
>> +         * including v4.1 to v4.12).
>> +         * KVM doesn't yet expose any XSAVES state save component.
>> +         */
> 
> Do you know which supervisor state components are available in
> EPYC CPUs?  Do you have a pointer to public AMD documentation
> about XSAVES?
> 

The available state components that may be saved by XSAVES are solely
determined by XCR0, and are therefore the same set of components that
can be used in XSAVE.

AMD APM vol4 [1] provides some information about XSAVES

[1] http://support.amd.com/TechDocs/26568.pdf#search=XSAVES

Note:
Looking at the document, I see that it says state elements saved are
determined by logical OR of XCRO with the IA32_XSS MSR. But the IA32_XSS MSR
definition is missing from the document and I have flagged it, it should be
updated with correct definition. But in the meantime, CPU team has confirmed
that the EPYC does not support any bits in IA32_XSS MSR.

> 
>> +        .features[FEAT_XSAVE] =
>> +            CPUID_XSAVE_XSAVEOPT | CPUID_XSAVE_XSAVEC |
>> +            CPUID_XSAVE_XGETBV1,
>> +        .features[FEAT_6_EAX] =
>> +            CPUID_6_EAX_ARAT,
>> +        .xlevel = 0x8000001F,
> 
> All CPUID leaves from 0x8000000B to 0x8000001F return all-zeroes
> today.  If we set xlevel to 0x8000001F before we actually
> implement those CPUID leaves, we will be forced to add extra
> machine-type compat code when we finally implement them.
> 
> I suggest setting it to 0x8000000A, and increasing it only after
> we actually implement the new CPUID leaves.
> 
> 

Sure, I will limit xlevel to 0x8000_000A.

-Brijesh