[Qemu-devel] [PATCH v3] i386: Add new Hygon 'Dhyana' CPU model

Pu Wen posted 1 patch 5 years ago
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test checkpatch passed
Test asan passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1555416373-28690-1-git-send-email-puwen@hygon.cn
Maintainers: Richard Henderson <rth@twiddle.net>, Eduardo Habkost <ehabkost@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
target/i386/cpu.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
target/i386/cpu.h |  2 ++
2 files changed, 52 insertions(+)
[Qemu-devel] [PATCH v3] i386: Add new Hygon 'Dhyana' CPU model
Posted by Pu Wen 5 years ago
Add a new base CPU model called 'Dhyana' to model processors from Hygon
Dhyana(family 18h), which derived from AMD EPYC(family 17h).

The following features bits have been removed compare to AMD EPYC:
aes, pclmulqdq, sha_ni

The Hygon Dhyana support to KVM in Linux is already accepted upstream[1].
So add Hygon Dhyana support to Qemu is necessary to create Hygon's own
CPU model.

Reference:
[1] https://git.kernel.org/tip/fec98069fb72fb656304a3e52265e0c2fc9adf87

Signed-off-by: Pu Wen <puwen@hygon.cn>
---
v2->v3:
  - Remove the Dhyana machine types in the back-compat property tables
    since there is no Dhyana CPU model in older QEMU releases.

v1->v2:
  - Remove CPU model 'Dhyana' and rename the CPU model 'Dhyana-IBPB' to
    'Dhyana' because Dhyana CPUs already have the IBPB feature.

 target/i386/cpu.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 target/i386/cpu.h |  2 ++
 2 files changed, 52 insertions(+)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index d6bb57d..d314c5f 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -2934,6 +2934,56 @@ static X86CPUDefinition builtin_x86_defs[] = {
         .model_id = "AMD EPYC Processor (with IBPB)",
         .cache_info = &epyc_cache_info,
     },
+    {
+        .name = "Dhyana",
+        .level = 0xd,
+        .vendor = CPUID_VENDOR_HYGON,
+        .family = 24,
+        .model = 0,
+        .stepping = 1,
+        .features[FEAT_1_EDX] =
+            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_VME | CPUID_FP87,
+        .features[FEAT_1_ECX] =
+            CPUID_EXT_RDRAND | CPUID_EXT_F16C | CPUID_EXT_AVX |
+            CPUID_EXT_XSAVE | CPUID_EXT_POPCNT |
+            CPUID_EXT_MOVBE | CPUID_EXT_SSE42 | CPUID_EXT_SSE41 |
+            CPUID_EXT_CX16 | CPUID_EXT_FMA | CPUID_EXT_SSSE3 |
+            CPUID_EXT_MONITOR | 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 |
+            CPUID_EXT3_TOPOEXT,
+        .features[FEAT_8000_0008_EBX] =
+            CPUID_8000_0008_EBX_IBPB,
+        .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,
+        /*
+         * 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,
+        .features[FEAT_SVM] =
+            CPUID_SVM_NPT | CPUID_SVM_NRIPSAVE,
+        .xlevel = 0x8000001E,
+        .model_id = "Hygon Dhyana Processor",
+        .cache_info = &epyc_cache_info,
+    },
 };
 
 typedef struct PropValue {
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 83fb522..553dbe7 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -726,6 +726,8 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
 
 #define CPUID_VENDOR_VIA   "CentaurHauls"
 
+#define CPUID_VENDOR_HYGON    "HygonGenuine"
+
 #define CPUID_MWAIT_IBE     (1U << 1) /* Interrupts can exit capability */
 #define CPUID_MWAIT_EMX     (1U << 0) /* enumeration supported */
 
-- 
2.7.4


Re: [Qemu-devel] [PATCH v3] i386: Add new Hygon 'Dhyana' CPU model
Posted by Daniel P. Berrangé 5 years ago
On Tue, Apr 16, 2019 at 08:06:13PM +0800, Pu Wen wrote:
> Add a new base CPU model called 'Dhyana' to model processors from Hygon
> Dhyana(family 18h), which derived from AMD EPYC(family 17h).
> 
> The following features bits have been removed compare to AMD EPYC:
> aes, pclmulqdq, sha_ni
> 
> The Hygon Dhyana support to KVM in Linux is already accepted upstream[1].
> So add Hygon Dhyana support to Qemu is necessary to create Hygon's own
> CPU model.
> 
> Reference:
> [1] https://git.kernel.org/tip/fec98069fb72fb656304a3e52265e0c2fc9adf87
> 
> Signed-off-by: Pu Wen <puwen@hygon.cn>
> ---
> v2->v3:
>   - Remove the Dhyana machine types in the back-compat property tables
>     since there is no Dhyana CPU model in older QEMU releases.
> 
> v1->v2:
>   - Remove CPU model 'Dhyana' and rename the CPU model 'Dhyana-IBPB' to
>     'Dhyana' because Dhyana CPUs already have the IBPB feature.
> 
>  target/i386/cpu.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  target/i386/cpu.h |  2 ++
>  2 files changed, 52 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v3] i386: Add new Hygon 'Dhyana' CPU model
Posted by Pavel Hrdina 5 years ago
On Tue, Apr 16, 2019 at 08:06:13PM +0800, Pu Wen wrote:
> Add a new base CPU model called 'Dhyana' to model processors from Hygon
> Dhyana(family 18h), which derived from AMD EPYC(family 17h).
> 
> The following features bits have been removed compare to AMD EPYC:
> aes, pclmulqdq, sha_ni
> 
> The Hygon Dhyana support to KVM in Linux is already accepted upstream[1].
> So add Hygon Dhyana support to Qemu is necessary to create Hygon's own
> CPU model.

I have once question that we will have to solve for EPYC CPUs as well.
The name should not be based on the Product name or Model name as that
usually doesn't change with introduction of new microarchitecture.

With EPYC we made a mistake to name the CPU like that, luckily with
Intel we already use the microarchitecture name, so the EPYC CPU should
have been named ZEN-Server and for Ryzen CPUs there should be ZEN-Client
if there is any difference or otherwise we can simply use ZEN.

The issue here is what happens once the ZEN2 microarchitecture is out
wihch introduces new features and we will have to come up with a CPU
name.

Obviously we cannot change/remove the EPYC models so the question is
what is the difference between the AMD EPYC CPU and this new Dhyana CPU
if they are both based on the ZEN microarchitecture?

In addition is there any way how we can introduce ZEN-Server &
ZEN-Client or simply ZEN, if there is no difference, as an alias or a
new model next to the EPYC?

Pavel
Re: [Qemu-devel] [PATCH v3] i386: Add new Hygon 'Dhyana' CPU model
Posted by Eduardo Habkost 5 years ago
On Tue, Apr 16, 2019 at 04:17:00PM +0200, Pavel Hrdina wrote:
> On Tue, Apr 16, 2019 at 08:06:13PM +0800, Pu Wen wrote:
> > Add a new base CPU model called 'Dhyana' to model processors from Hygon
> > Dhyana(family 18h), which derived from AMD EPYC(family 17h).
> > 
> > The following features bits have been removed compare to AMD EPYC:
> > aes, pclmulqdq, sha_ni
> > 
> > The Hygon Dhyana support to KVM in Linux is already accepted upstream[1].
> > So add Hygon Dhyana support to Qemu is necessary to create Hygon's own
> > CPU model.
> 
> I have once question that we will have to solve for EPYC CPUs as well.
> The name should not be based on the Product name or Model name as that
> usually doesn't change with introduction of new microarchitecture.
> 
> With EPYC we made a mistake to name the CPU like that, luckily with
> Intel we already use the microarchitecture name, so the EPYC CPU should
> have been named ZEN-Server and for Ryzen CPUs there should be ZEN-Client
> if there is any difference or otherwise we can simply use ZEN.
> 
> The issue here is what happens once the ZEN2 microarchitecture is out
> wihch introduces new features and we will have to come up with a CPU
> name.
> 
> Obviously we cannot change/remove the EPYC models so the question is
> what is the difference between the AMD EPYC CPU and this new Dhyana CPU
> if they are both based on the ZEN microarchitecture?
> 
> In addition is there any way how we can introduce ZEN-Server &
> ZEN-Client or simply ZEN, if there is no difference, as an alias or a
> new model next to the EPYC?

Currently there's no CPU model alias or inheritance system in x86
(yet), but feel free to submit a CPU model that's a copy of EPYC,
including the justification above.

When we implement CPU model versioning, we can make "EPYC" be
translated to "ZEN" by the CPU model version resolution API.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3] i386: Add new Hygon 'Dhyana' CPU model
Posted by Pu Wen 5 years ago
On 2019/4/16 22:17, Pavel Hrdina wrote:
> On Tue, Apr 16, 2019 at 08:06:13PM +0800, Pu Wen wrote:
>> Add a new base CPU model called 'Dhyana' to model processors from Hygon
>> Dhyana(family 18h), which derived from AMD EPYC(family 17h).
>>
>> The following features bits have been removed compare to AMD EPYC:
>> aes, pclmulqdq, sha_ni
>>
>> The Hygon Dhyana support to KVM in Linux is already accepted upstream[1].
>> So add Hygon Dhyana support to Qemu is necessary to create Hygon's own
>> CPU model.
> 
> I have once question that we will have to solve for EPYC CPUs as well.
> The name should not be based on the Product name or Model name as that
> usually doesn't change with introduction of new microarchitecture.
> 
> With EPYC we made a mistake to name the CPU like that, luckily with
> Intel we already use the microarchitecture name, so the EPYC CPU should
> have been named ZEN-Server and for Ryzen CPUs there should be ZEN-Client
> if there is any difference or otherwise we can simply use ZEN.
> 
> The issue here is what happens once the ZEN2 microarchitecture is out
> wihch introduces new features and we will have to come up with a CPU
> name.
> 
> Obviously we cannot change/remove the EPYC models so the question is
> what is the difference between the AMD EPYC CPU and this new Dhyana CPU
> if they are both based on the ZEN microarchitecture?

Right now there's no much difference between Dhyana and EPYC from the 
software's view. Dhyana removed the instructions aes, pclmulqdq, sha_ni 
compared to EPYC, but will have it's own implementation such as for aes 
in future CPU models. Hygon also will implement something different from 
AMD in the future.

> In addition is there any way how we can introduce ZEN-Server &
> ZEN-Client or simply ZEN, if there is no difference, as an alias or a
> new model next to the EPYC?

Also as Eduardo mentioned that there's no CPU model alias or inheritance 
system in x86, so I think it's worthwhile to keep a separate CPU model 
for Hygon.

-- 
Regards,
Pu Wen

Re: [Qemu-devel] [PATCH v3] i386: Add new Hygon 'Dhyana' CPU model
Posted by Pavel Hrdina 5 years ago
On Wed, Apr 17, 2019 at 10:53:04PM +0800, Pu Wen wrote:
> On 2019/4/16 22:17, Pavel Hrdina wrote:
> > On Tue, Apr 16, 2019 at 08:06:13PM +0800, Pu Wen wrote:
> > > Add a new base CPU model called 'Dhyana' to model processors from Hygon
> > > Dhyana(family 18h), which derived from AMD EPYC(family 17h).
> > > 
> > > The following features bits have been removed compare to AMD EPYC:
> > > aes, pclmulqdq, sha_ni
> > > 
> > > The Hygon Dhyana support to KVM in Linux is already accepted upstream[1].
> > > So add Hygon Dhyana support to Qemu is necessary to create Hygon's own
> > > CPU model.
> > 
> > I have once question that we will have to solve for EPYC CPUs as well.
> > The name should not be based on the Product name or Model name as that
> > usually doesn't change with introduction of new microarchitecture.
> > 
> > With EPYC we made a mistake to name the CPU like that, luckily with
> > Intel we already use the microarchitecture name, so the EPYC CPU should
> > have been named ZEN-Server and for Ryzen CPUs there should be ZEN-Client
> > if there is any difference or otherwise we can simply use ZEN.
> > 
> > The issue here is what happens once the ZEN2 microarchitecture is out
> > wihch introduces new features and we will have to come up with a CPU
> > name.
> > 
> > Obviously we cannot change/remove the EPYC models so the question is
> > what is the difference between the AMD EPYC CPU and this new Dhyana CPU
> > if they are both based on the ZEN microarchitecture?
> 
> Right now there's no much difference between Dhyana and EPYC from the
> software's view. Dhyana removed the instructions aes, pclmulqdq, sha_ni
> compared to EPYC, but will have it's own implementation such as for aes in
> future CPU models. Hygon also will implement something different from AMD in
> the future.
> 
> > In addition is there any way how we can introduce ZEN-Server &
> > ZEN-Client or simply ZEN, if there is no difference, as an alias or a
> > new model next to the EPYC?
> 
> Also as Eduardo mentioned that there's no CPU model alias or inheritance
> system in x86, so I think it's worthwhile to keep a separate CPU model for
> Hygon.

So what happens once Zen2 is out and there are new Dhyana CPUs based on
the Zen2 microarchitecture with some new features, what CPU models we
will introduce, EPYC-G2 and Dhyana-G2, but that will not correspond to
the CPU model anymore.

My idea was that we should probably introduce CPU model Zen-Server which
could cover both EPYC and Dhyana as they are both based on the Zen
microarchitecture.  The fact that Dhyana doesn't support all the
features is not an issue as QEMU will not use them if they are not
available on the host.

Another possibility is to introduce Zen-Server (or AMD-Zen-Server) and
Hygon-Zen-Server with different set of features.

In the future when new microarchitecture is introduced we can simply use
the exact name of the microarchitecture and we don't have to follow the
road as we did with AMD Opteron CPUs where we have different suffixes.

The whole point is that we should name the CPUs the same way for all
vendors and stop introducing new CPU names that will create more
confusion.

Pavel
Re: [Qemu-devel] [PATCH v3] i386: Add new Hygon 'Dhyana' CPU model
Posted by Daniel P. Berrangé 5 years ago
On Wed, Apr 17, 2019 at 09:26:10PM +0200, Pavel Hrdina wrote:
> On Wed, Apr 17, 2019 at 10:53:04PM +0800, Pu Wen wrote:
> > On 2019/4/16 22:17, Pavel Hrdina wrote:
> > > On Tue, Apr 16, 2019 at 08:06:13PM +0800, Pu Wen wrote:
> > > > Add a new base CPU model called 'Dhyana' to model processors from Hygon
> > > > Dhyana(family 18h), which derived from AMD EPYC(family 17h).
> > > > 
> > > > The following features bits have been removed compare to AMD EPYC:
> > > > aes, pclmulqdq, sha_ni
> > > > 
> > > > The Hygon Dhyana support to KVM in Linux is already accepted upstream[1].
> > > > So add Hygon Dhyana support to Qemu is necessary to create Hygon's own
> > > > CPU model.
> > > 
> > > I have once question that we will have to solve for EPYC CPUs as well.
> > > The name should not be based on the Product name or Model name as that
> > > usually doesn't change with introduction of new microarchitecture.
> > > 
> > > With EPYC we made a mistake to name the CPU like that, luckily with
> > > Intel we already use the microarchitecture name, so the EPYC CPU should
> > > have been named ZEN-Server and for Ryzen CPUs there should be ZEN-Client
> > > if there is any difference or otherwise we can simply use ZEN.
> > > 
> > > The issue here is what happens once the ZEN2 microarchitecture is out
> > > wihch introduces new features and we will have to come up with a CPU
> > > name.
> > > 
> > > Obviously we cannot change/remove the EPYC models so the question is
> > > what is the difference between the AMD EPYC CPU and this new Dhyana CPU
> > > if they are both based on the ZEN microarchitecture?
> > 
> > Right now there's no much difference between Dhyana and EPYC from the
> > software's view. Dhyana removed the instructions aes, pclmulqdq, sha_ni
> > compared to EPYC, but will have it's own implementation such as for aes in
> > future CPU models. Hygon also will implement something different from AMD in
> > the future.
> > 
> > > In addition is there any way how we can introduce ZEN-Server &
> > > ZEN-Client or simply ZEN, if there is no difference, as an alias or a
> > > new model next to the EPYC?
> > 
> > Also as Eduardo mentioned that there's no CPU model alias or inheritance
> > system in x86, so I think it's worthwhile to keep a separate CPU model for
> > Hygon.
> 
> So what happens once Zen2 is out and there are new Dhyana CPUs based on
> the Zen2 microarchitecture with some new features, what CPU models we
> will introduce, EPYC-G2 and Dhyana-G2, but that will not correspond to
> the CPU model anymore.
> 
> My idea was that we should probably introduce CPU model Zen-Server which
> could cover both EPYC and Dhyana as they are both based on the Zen
> microarchitecture.  The fact that Dhyana doesn't support all the
> features is not an issue as QEMU will not use them if they are not
> available on the host.

I don't think this is a good idea. AFAICT, Dhyana and EPYC should not be
thought of as the same microarchitecture. Dhyana is a fork of the Zen
microarchitecture as illustrated by the dropping of a number of CPU
features. The Dhyana SEV patches show that it has had other significant
changes, which are likely to require extra work in QEMU to support too.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v3] i386: Add new Hygon 'Dhyana' CPU model
Posted by Pavel Hrdina 5 years ago
On Thu, Apr 18, 2019 at 09:48:25AM +0100, Daniel P. Berrangé wrote:
> On Wed, Apr 17, 2019 at 09:26:10PM +0200, Pavel Hrdina wrote:
> > On Wed, Apr 17, 2019 at 10:53:04PM +0800, Pu Wen wrote:
> > > On 2019/4/16 22:17, Pavel Hrdina wrote:
> > > > On Tue, Apr 16, 2019 at 08:06:13PM +0800, Pu Wen wrote:
> > > > > Add a new base CPU model called 'Dhyana' to model processors from Hygon
> > > > > Dhyana(family 18h), which derived from AMD EPYC(family 17h).
> > > > > 
> > > > > The following features bits have been removed compare to AMD EPYC:
> > > > > aes, pclmulqdq, sha_ni
> > > > > 
> > > > > The Hygon Dhyana support to KVM in Linux is already accepted upstream[1].
> > > > > So add Hygon Dhyana support to Qemu is necessary to create Hygon's own
> > > > > CPU model.
> > > > 
> > > > I have once question that we will have to solve for EPYC CPUs as well.
> > > > The name should not be based on the Product name or Model name as that
> > > > usually doesn't change with introduction of new microarchitecture.
> > > > 
> > > > With EPYC we made a mistake to name the CPU like that, luckily with
> > > > Intel we already use the microarchitecture name, so the EPYC CPU should
> > > > have been named ZEN-Server and for Ryzen CPUs there should be ZEN-Client
> > > > if there is any difference or otherwise we can simply use ZEN.
> > > > 
> > > > The issue here is what happens once the ZEN2 microarchitecture is out
> > > > wihch introduces new features and we will have to come up with a CPU
> > > > name.
> > > > 
> > > > Obviously we cannot change/remove the EPYC models so the question is
> > > > what is the difference between the AMD EPYC CPU and this new Dhyana CPU
> > > > if they are both based on the ZEN microarchitecture?
> > > 
> > > Right now there's no much difference between Dhyana and EPYC from the
> > > software's view. Dhyana removed the instructions aes, pclmulqdq, sha_ni
> > > compared to EPYC, but will have it's own implementation such as for aes in
> > > future CPU models. Hygon also will implement something different from AMD in
> > > the future.
> > > 
> > > > In addition is there any way how we can introduce ZEN-Server &
> > > > ZEN-Client or simply ZEN, if there is no difference, as an alias or a
> > > > new model next to the EPYC?
> > > 
> > > Also as Eduardo mentioned that there's no CPU model alias or inheritance
> > > system in x86, so I think it's worthwhile to keep a separate CPU model for
> > > Hygon.
> > 
> > So what happens once Zen2 is out and there are new Dhyana CPUs based on
> > the Zen2 microarchitecture with some new features, what CPU models we
> > will introduce, EPYC-G2 and Dhyana-G2, but that will not correspond to
> > the CPU model anymore.
> > 
> > My idea was that we should probably introduce CPU model Zen-Server which
> > could cover both EPYC and Dhyana as they are both based on the Zen
> > microarchitecture.  The fact that Dhyana doesn't support all the
> > features is not an issue as QEMU will not use them if they are not
> > available on the host.
> 
> I don't think this is a good idea. AFAICT, Dhyana and EPYC should not be
> thought of as the same microarchitecture. Dhyana is a fork of the Zen
> microarchitecture as illustrated by the dropping of a number of CPU
> features. The Dhyana SEV patches show that it has had other significant
> changes, which are likely to require extra work in QEMU to support too.

Fair enough, still the point stands that we should use the name of the
microarchitecture.

Pavel
Re: [Qemu-devel] [PATCH v3] i386: Add new Hygon 'Dhyana' CPU model
Posted by Eduardo Habkost 5 years ago
On Thu, Apr 18, 2019 at 11:59:35AM +0200, Pavel Hrdina wrote:
> On Thu, Apr 18, 2019 at 09:48:25AM +0100, Daniel P. Berrangé wrote:
> > On Wed, Apr 17, 2019 at 09:26:10PM +0200, Pavel Hrdina wrote:
> > > On Wed, Apr 17, 2019 at 10:53:04PM +0800, Pu Wen wrote:
> > > > On 2019/4/16 22:17, Pavel Hrdina wrote:
> > > > > On Tue, Apr 16, 2019 at 08:06:13PM +0800, Pu Wen wrote:
> > > > > > Add a new base CPU model called 'Dhyana' to model processors from Hygon
> > > > > > Dhyana(family 18h), which derived from AMD EPYC(family 17h).
> > > > > > 
> > > > > > The following features bits have been removed compare to AMD EPYC:
> > > > > > aes, pclmulqdq, sha_ni
> > > > > > 
> > > > > > The Hygon Dhyana support to KVM in Linux is already accepted upstream[1].
> > > > > > So add Hygon Dhyana support to Qemu is necessary to create Hygon's own
> > > > > > CPU model.
> > > > > 
> > > > > I have once question that we will have to solve for EPYC CPUs as well.
> > > > > The name should not be based on the Product name or Model name as that
> > > > > usually doesn't change with introduction of new microarchitecture.
> > > > > 
> > > > > With EPYC we made a mistake to name the CPU like that, luckily with
> > > > > Intel we already use the microarchitecture name, so the EPYC CPU should
> > > > > have been named ZEN-Server and for Ryzen CPUs there should be ZEN-Client
> > > > > if there is any difference or otherwise we can simply use ZEN.
> > > > > 
> > > > > The issue here is what happens once the ZEN2 microarchitecture is out
> > > > > wihch introduces new features and we will have to come up with a CPU
> > > > > name.
> > > > > 
> > > > > Obviously we cannot change/remove the EPYC models so the question is
> > > > > what is the difference between the AMD EPYC CPU and this new Dhyana CPU
> > > > > if they are both based on the ZEN microarchitecture?
> > > > 
> > > > Right now there's no much difference between Dhyana and EPYC from the
> > > > software's view. Dhyana removed the instructions aes, pclmulqdq, sha_ni
> > > > compared to EPYC, but will have it's own implementation such as for aes in
> > > > future CPU models. Hygon also will implement something different from AMD in
> > > > the future.
> > > > 
> > > > > In addition is there any way how we can introduce ZEN-Server &
> > > > > ZEN-Client or simply ZEN, if there is no difference, as an alias or a
> > > > > new model next to the EPYC?
> > > > 
> > > > Also as Eduardo mentioned that there's no CPU model alias or inheritance
> > > > system in x86, so I think it's worthwhile to keep a separate CPU model for
> > > > Hygon.
> > > 
> > > So what happens once Zen2 is out and there are new Dhyana CPUs based on
> > > the Zen2 microarchitecture with some new features, what CPU models we
> > > will introduce, EPYC-G2 and Dhyana-G2, but that will not correspond to
> > > the CPU model anymore.
> > > 
> > > My idea was that we should probably introduce CPU model Zen-Server which
> > > could cover both EPYC and Dhyana as they are both based on the Zen
> > > microarchitecture.  The fact that Dhyana doesn't support all the
> > > features is not an issue as QEMU will not use them if they are not
> > > available on the host.

It is an issue.  The fact that features are missing has huge
importance for most virtualization management software that
supports live migration.

Not having the "enforce" flag enabled by default was a mistake,
but it's hard to fix because we don't want to break backwards
compatibility.

> > 
> > I don't think this is a good idea. AFAICT, Dhyana and EPYC should not be
> > thought of as the same microarchitecture. Dhyana is a fork of the Zen
> > microarchitecture as illustrated by the dropping of a number of CPU
> > features. The Dhyana SEV patches show that it has had other significant
> > changes, which are likely to require extra work in QEMU to support too.
> 
> Fair enough, still the point stands that we should use the name of the
> microarchitecture.

Note that using "microarchitecture[-variant]" is likely to be
necessary if there's variation inside the same microarchitecture.
We absolutely need to use different names for different sets of
CPU features if we want to support them out of the box.

Note that we can't (and don't need to) enumerate all possible
combinations of features that exist in the world.  I let CPU
vendors who submit patches decide which combinations of features
are important to be supported out of the box.

Dhyana adds to the complexity, because of the new CPU vendor ID.
Different vendor IDs require different CPU models because the CPU
model contains family/model/stepping/model_id values that make
sense only in conjunction with the vendor ID.  Should we call it
Zen-Dhyana?  Dhyana_G1?  I think I'll just let the Hygon
developers decide.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3] i386: Add new Hygon 'Dhyana' CPU model
Posted by Pavel Hrdina 5 years ago
On Thu, Apr 18, 2019 at 11:05:20AM -0300, Eduardo Habkost wrote:
> On Thu, Apr 18, 2019 at 11:59:35AM +0200, Pavel Hrdina wrote:
> > On Thu, Apr 18, 2019 at 09:48:25AM +0100, Daniel P. Berrangé wrote:
> > > On Wed, Apr 17, 2019 at 09:26:10PM +0200, Pavel Hrdina wrote:
> > > > On Wed, Apr 17, 2019 at 10:53:04PM +0800, Pu Wen wrote:
> > > > > On 2019/4/16 22:17, Pavel Hrdina wrote:
> > > > > > On Tue, Apr 16, 2019 at 08:06:13PM +0800, Pu Wen wrote:
> > > > > > > Add a new base CPU model called 'Dhyana' to model processors from Hygon
> > > > > > > Dhyana(family 18h), which derived from AMD EPYC(family 17h).
> > > > > > > 
> > > > > > > The following features bits have been removed compare to AMD EPYC:
> > > > > > > aes, pclmulqdq, sha_ni
> > > > > > > 
> > > > > > > The Hygon Dhyana support to KVM in Linux is already accepted upstream[1].
> > > > > > > So add Hygon Dhyana support to Qemu is necessary to create Hygon's own
> > > > > > > CPU model.
> > > > > > 
> > > > > > I have once question that we will have to solve for EPYC CPUs as well.
> > > > > > The name should not be based on the Product name or Model name as that
> > > > > > usually doesn't change with introduction of new microarchitecture.
> > > > > > 
> > > > > > With EPYC we made a mistake to name the CPU like that, luckily with
> > > > > > Intel we already use the microarchitecture name, so the EPYC CPU should
> > > > > > have been named ZEN-Server and for Ryzen CPUs there should be ZEN-Client
> > > > > > if there is any difference or otherwise we can simply use ZEN.
> > > > > > 
> > > > > > The issue here is what happens once the ZEN2 microarchitecture is out
> > > > > > wihch introduces new features and we will have to come up with a CPU
> > > > > > name.
> > > > > > 
> > > > > > Obviously we cannot change/remove the EPYC models so the question is
> > > > > > what is the difference between the AMD EPYC CPU and this new Dhyana CPU
> > > > > > if they are both based on the ZEN microarchitecture?
> > > > > 
> > > > > Right now there's no much difference between Dhyana and EPYC from the
> > > > > software's view. Dhyana removed the instructions aes, pclmulqdq, sha_ni
> > > > > compared to EPYC, but will have it's own implementation such as for aes in
> > > > > future CPU models. Hygon also will implement something different from AMD in
> > > > > the future.
> > > > > 
> > > > > > In addition is there any way how we can introduce ZEN-Server &
> > > > > > ZEN-Client or simply ZEN, if there is no difference, as an alias or a
> > > > > > new model next to the EPYC?
> > > > > 
> > > > > Also as Eduardo mentioned that there's no CPU model alias or inheritance
> > > > > system in x86, so I think it's worthwhile to keep a separate CPU model for
> > > > > Hygon.
> > > > 
> > > > So what happens once Zen2 is out and there are new Dhyana CPUs based on
> > > > the Zen2 microarchitecture with some new features, what CPU models we
> > > > will introduce, EPYC-G2 and Dhyana-G2, but that will not correspond to
> > > > the CPU model anymore.
> > > > 
> > > > My idea was that we should probably introduce CPU model Zen-Server which
> > > > could cover both EPYC and Dhyana as they are both based on the Zen
> > > > microarchitecture.  The fact that Dhyana doesn't support all the
> > > > features is not an issue as QEMU will not use them if they are not
> > > > available on the host.
> 
> It is an issue.  The fact that features are missing has huge
> importance for most virtualization management software that
> supports live migration.
> 
> Not having the "enforce" flag enabled by default was a mistake,
> but it's hard to fix because we don't want to break backwards
> compatibility.
> 
> > > 
> > > I don't think this is a good idea. AFAICT, Dhyana and EPYC should not be
> > > thought of as the same microarchitecture. Dhyana is a fork of the Zen
> > > microarchitecture as illustrated by the dropping of a number of CPU
> > > features. The Dhyana SEV patches show that it has had other significant
> > > changes, which are likely to require extra work in QEMU to support too.
> > 
> > Fair enough, still the point stands that we should use the name of the
> > microarchitecture.
> 
> Note that using "microarchitecture[-variant]" is likely to be
> necessary if there's variation inside the same microarchitecture.
> We absolutely need to use different names for different sets of
> CPU features if we want to support them out of the box.
> 
> Note that we can't (and don't need to) enumerate all possible
> combinations of features that exist in the world.  I let CPU
> vendors who submit patches decide which combinations of features
> are important to be supported out of the box.
> 
> Dhyana adds to the complexity, because of the new CPU vendor ID.
> Different vendor IDs require different CPU models because the CPU
> model contains family/model/stepping/model_id values that make
> sense only in conjunction with the vendor ID.  Should we call it
> Zen-Dhyana?  Dhyana_G1?  I think I'll just let the Hygon
> developers decide.

Just for the reference, this discussion happened last year and AMD
developer Brijesh agreed [1] that the microarchitecture name would be
better, one of the reasons I've objected again.

Pavel

[1] <https://lists.gnu.org/archive/html/qemu-devel/2018-07/msg03902.html>
Re: [Qemu-devel] [PATCH v3] i386: Add new Hygon 'Dhyana' CPU model
Posted by Eduardo Habkost 5 years ago
On Tue, Apr 16, 2019 at 08:06:13PM +0800, Pu Wen wrote:
> Add a new base CPU model called 'Dhyana' to model processors from Hygon
> Dhyana(family 18h), which derived from AMD EPYC(family 17h).
> 
> The following features bits have been removed compare to AMD EPYC:
> aes, pclmulqdq, sha_ni
> 
> The Hygon Dhyana support to KVM in Linux is already accepted upstream[1].
> So add Hygon Dhyana support to Qemu is necessary to create Hygon's own
> CPU model.
> 
> Reference:
> [1] https://git.kernel.org/tip/fec98069fb72fb656304a3e52265e0c2fc9adf87
> 
> Signed-off-by: Pu Wen <puwen@hygon.cn>

Queued for 4.1, thanks!

-- 
Eduardo

Re: [Qemu-devel] [PATCH v3] i386: Add new Hygon 'Dhyana' CPU model
Posted by Pu Wen 5 years ago
On 2019/4/17 2:03, Eduardo Habkost wrote:
> On Tue, Apr 16, 2019 at 08:06:13PM +0800, Pu Wen wrote:
>> Add a new base CPU model called 'Dhyana' to model processors from Hygon
>> Dhyana(family 18h), which derived from AMD EPYC(family 17h).
>>
>> The following features bits have been removed compare to AMD EPYC:
>> aes, pclmulqdq, sha_ni
>>
>> The Hygon Dhyana support to KVM in Linux is already accepted upstream[1].
>> So add Hygon Dhyana support to Qemu is necessary to create Hygon's own
>> CPU model.
>>
>> Reference:
>> [1] https://git.kernel.org/tip/fec98069fb72fb656304a3e52265e0c2fc9adf87
>>
>> Signed-off-by: Pu Wen <puwen@hygon.cn>
> 
> Queued for 4.1, thanks!

Thanks a lot!

-- 
Regards,
Pu Wen