Re: [PATCH v2 6/6] target/i386: Add new CPU model GraniteRapids

[Igor Mammedov]

On Tue, 30 Jan 2024 21:34:36 +0800
Tao Su <tao1.su@linux.intel.com> wrote:

> On Tue, Jan 30, 2024 at 11:14:59AM +0100, Igor Mammedov wrote:
> > On Thu,  6 Jul 2023 13:49:49 +0800
> > Tao Su <tao1.su@linux.intel.com> wrote:
> >   
> > > The GraniteRapids CPU model mainly adds the following new features
> > > based on SapphireRapids:
> > > - PREFETCHITI CPUID.(EAX=7,ECX=1):EDX[bit 14]
> > > - AMX-FP16 CPUID.(EAX=7,ECX=1):EAX[bit 21]  
> > 
> > it seems the list/definition is not complete, see
> > https://lore.kernel.org/kvm/20221125125845.1182922-1-jiaxi.chen@linux.intel.com/
> > and those feature bits were merged into QEMU earlier 
> > (a9ce107fd0f..d1a11115143)
> > 
> > were they omited intentionaly?
> >   
> 
> No, Jiaxi’s patch series includes new feature bits of both Granite Rapids(GNR)
> and Sierra Forest(SRF).
> 
> GNR contains:
>       PREFETCHITI CPUID.(EAX=7,ECX=1):EDX[bit 14]
>       AMX-FP16 CPUID.(EAX=7,ECX=1):EAX[bit 21]
> 
> SRF contains:
>       CMPccXADD CPUID.(EAX=7,ECX=1):EAX[bit 7]
>       AVX-IFMA CPUID.(EAX=7,ECX=1):EAX[bit 23]
>       AVX-VNNI-INT8 CPUID.(EAX=7,ECX=1):EDX[bit 4]
>       AVX-NE-CONVERT CPUID.(EAX=7,ECX=1):EDX[bit 5]

> What new platforms support the new features can be found in Table 1-2 of 
> ISE[1].
> And the SRF CPU model we submitted[2] contains the four feature bits 
> supported above.
Thanks,

for future patches: 
this kind of info should be part of commit message incl.
spec/doc title/revision it's coming from with a specific
chapter/table also mentioned. This way whoever reads it
later won't have to ask or spend time for searching where
it comes from.

And maybe also have a comment close to new code,
aka like we do for ACPI patches.
 
> [1] https://cdrdv2.intel.com/v1/dl/getContent/671368
> [2] 
> https://lore.kernel.org/all/20231206131923.1192066-1-tao1.su@linux.intel.com/
> 
> Thanks,
> Tao
>