Re: [PATCH 6/7] target/i386: Add new CPU model EmeraldRapids

[Daniel P . Berrangé]

On Tue, Jun 27, 2023 at 07:25:21PM +0800, Xiaoyao Li wrote:
> On 6/27/2023 4:49 PM, Igor Mammedov wrote:
> > On Tue, 27 Jun 2023 13:54:23 +0800
> > Xiaoyao Li <xiaoyao.li@intel.com> wrote:
> > 
> > > On 6/26/2023 8:56 PM, Igor Mammedov wrote:
> > > > On Fri, 16 Jun 2023 11:23:10 +0800
> > > > Tao Su<tao1.su@linux.intel.com>  wrote:
> > > > > From: Qian Wen<qian.wen@intel.com>
> > > > > 
> > > > > Emerald Rapids (EMR) is the next generation of Xeon server processor
> > > > > after Sapphire Rapids (SPR).
> > > > > 
> > > > > Currently, regarding the feature set that can be exposed to guest, 
> > > > > there
> > > > > isn't any one new comparing with SPR cpu model, except that EMR has a
> > > > > different model number.
> > > > > 
> > > > > Though it's practicable to define EMR as an alias of a new version of
> > > > > SPR by only updating the model number and model name, it loses the
> > > > > flexibility when new version of EMR cpu model are needed for adding 
> > > > > new
> > > > > features (that hasn't virtalized/supported by KVM yet).
> > > > Which begs a question, why do we need EMR model (or alias) at all
> > > > if it's the same as SPR at the moment.
> > > > 
> > > > Make new features supported 1st and only then introduce a new CPU model.
> > > 
> > > Even if no new feature (that can be virtualized and exposed to guest) in
> > > EMR compared to SPR in the end, I think it still makes sense to provide
> > > a dedicated EMR CPU model in QEMU. Because
> > > 1) User will know EMR, Intel's next generation of Xeon after SRP, is
> > > supported by QEMU, via -cpu ?/ -cpu help;
> > 
> > I don't see any benefits in misleading user by showing EMR model which is
> > nothing else but SPR one.
> > On negative side you would increase maintenance burden by introducing
> > extra versions of CPU model later. Which by itself is abusing versioning,
> > mainly used for fixing CPU bugs, by using it for adding new features.
> > 
> > > 2) It's convenient for user to create an EMR VM. People may not care
> > > that much what the difference between "-cpu SapphireRapids" with "-cpu
> > > EmeraldRapids", while they do want to create an VM which shows the CPU
> > > is EmeraldRapids.
> > > 
> > My guess would be is that you need guest to show EMR for developing EMR
> > features/guest bringup, in that case do it in your fork, and once
> > support is actually ready publish completed patches for it.
> 
> No. I meant for CSPs who want to provide an EMR virtual machine to their
> customers, or lab admin provides an EMR (virtual) machine to its user.
> 
> Without a dedicated EmeraldRapids cpu model provided by QEMU, they need to
> use something like
> 
>   -cpu SapphireRapids,model=207,model-id="Intel Xeon Processor
> (EmeraldRapids)"
>
> It's likely that no difference in supported features between SPR cpu model
> and EMR cpu model in the end. If so, will QEMU choose to provide a dedicated
> CPU model for EMR? or just document somewhere to QEMU users that "if you
> want to create an virtual machine with EMR cpu model, please go with SPR cpu
> model while changing it's model number to EMR's 207 and changing model-id to
> tell EmeraldRapids" ?

I think QEMU's answer would be to not bother trying todo this at all,
just expose '-cpu SapphireRapids', because there's no functional benefit
to overriding the model ID, when all the CPUID features are identical.

Those who have the guest to see the *perfect* functional match of the
host still have '-cpu host' available.

The named CPU models are for the case where we want a rough approximation
for a CPU generation available, to easy migration across mixed CPU clusters.
Given the intent is to have a rough approximation, there's no compelling
reason to add an exact EmeraldRapids named CPU.

> > To exaggerate you reasoning further, we should create CPU models for
> > all future planned Intel/AMD CPU as a one of currently existing in
> > QEMU right now and then sometime in future implement features that
> > actually make those models what they should be.
> 
> No, it's not the purpose. In fact, we're not adding an temporary EMR cpu
> model while planing to complement it in the future. Instead, we are adding
> an official EMR cpu model. The fact is, in terms of the features that are
> virtualizable and can be exposed to guest, there is no difference between
> SPR and EMR.
> 
> This comes to a basic question：Will QEMU provide a EMR cpu model even if no
> difference to SPR cpu model except the model number?

Historically we have generally only added new CPU models if there was
a feature difference. We've skipped adding many of the Intel models
that didn't add bring new features, on the basis that there is no
compelling functional need to have them.

With 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 :|