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

[Eduardo Habkost]

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