RE: [PATCH] i386: pass CLZERO to guests with EPYC CPU model on AMD ZEN platform

["Moger, Babu" <Babu.Moger@amd.com>]

[AMD Official Use Only - Internal Distribution Only]

> -----Original Message-----
> From: Eduardo Habkost <ehabkost@redhat.com>
> Sent: Wednesday, February 5, 2020 4:38 PM
> To: Ani Sinha <ani.sinha@nutanix.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>; rth@twiddle.net; qemu-
> devel@nongnu.org; Singh, Brijesh <brijesh.singh@amd.com>; Moger, Babu
> <Babu.Moger@amd.com>
> Subject: Re: [PATCH] i386: pass CLZERO to guests with EPYC CPU model on
> AMD ZEN platform
> 
> Hi,
> 
> Sorry for the delayed reply.  I was away from work for the whole
> month of January.
> 
> On Mon, Jan 20, 2020 at 10:56:43AM +0000, Ani Sinha wrote:
> > Sorry Eduardo, it took a little while for me to get to this thread again.
> >
> > > On Dec 18, 2019, at 8:41 PM, Eduardo Habkost <ehabkost@redhat.com>
> wrote:
> > >
> > > On Wed, Dec 18, 2019 at 12:53:45PM +0100, Paolo Bonzini wrote:
> > >> On 18/12/19 10:05, Ani Sinha wrote:
> > >>> CLZERO CPUID should be passed on to the guests that use EPYC or
> EPYC-IBPB CPU
> > >>> model when the AMD ZEN based host supports it. This change makes it
> recognize
> > >>> this CPUID for guests which use EPYC or EPYC-IBPB CPU model.
> > >
> > > Can you clarify what's the intended use case here?  Why the
> > > "if host supports it" conditional?
> >
> > Looking at
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fww
> w.amd.com%2Fsystem%2Ffiles%2FTechDocs%2F24594.pdf&amp;data=02%7
> C01%7Cbabu.moger%40amd.com%7C4d00819020cb4892d50608d7aa8c016b%
> 7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637165391335697450
> &amp;sdata=3%2FFGZeeZoz387eeayAbjQJj9qGhemgw0xC0byr%2F0YJk%3D
> &amp;reserved=0 , it says :
> >
> > "The CLZERO instruction is supported if the feature flag CPUID
> Fn8000_0008_EBX[CLZERO] is set.”
> >
> > This I interpreted to mean that not all AMD Zen architectures
> > supports it. So when the host does support it, this CPUID
> > should be passed on to the guest as well.
> 
> This is not a supported use case of named CPU models.  Named CPU
> models should expose the same guest ABI on all hosts.  This means
> CPUID should be the same on all hosts if using the same CPU
> model (and same machine type).
> 
> If you need features to be automatically enabled/disabled
> depending on host capabilities, I advise you to use "-cpu host"
> or libvirt's mode=host-model.
> 
> >
> >
> > >
> > > If you need host-dependent CPU configuration, "-cpu host" (or the
> > > libvirt "host-model" mode) is the most appropriate solution.
> >
> > Yes that is an option but we are going to use EPYC-IBPB model for now.
> >
> >
> > >
> > >>>
> > >>> Signed-off-by: Ani Sinha <ani.sinha@nutanix.com>
> > >>> ---
> > >>> target/i386/cpu.c | 2 ++
> > >>> 1 file changed, 2 insertions(+)
> > >>>
> > >>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > >>> index 69f518a..55f0691 100644
> > >>> --- a/target/i386/cpu.c
> > >>> +++ b/target/i386/cpu.c
> > >>> @@ -3813,6 +3813,8 @@ static X86CPUDefinition builtin_x86_defs[] = {
> > >>>             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_CLZERO,
> > >>>         .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 |
> > >>>
> > >>
> > >> This needs to be done only for newer machine type (or is it CPU model
> > >> versions now? need Eduardo to respond).
> > >
> > > If we want to add it, it has to be done as a new CPU model version.
> >
> > I see what you mean.
> >
> > >
> > > But I don't know yet if we want to add it.  Do all EPYC CPUs have
> > > CLZERO available?  If not, it's probably not advisable to add it
> > > to EPYC (even if it's just on EPYC-v3).
> >
> > Ok so I think we need to get this clarified from AMD if all
> > their EPYC platforms supports this CPUID or not. Is there any
> > contact point within AMD where we can get this information?
> 
> I'm CCing Brijesh Singh and Babu Moger, who works on the EPYC CPU
> model recently.

Ani, I am already working on it.

Eduardo,  I am still waiting for your feedback on this series.
https://lore.kernel.org/qemu-devel/abd39b75-0a12-5198-5815-dd51a3d5c901@amd.com/

I have added all the missing feature bits for EPYC models(as EPYC-v3) and also added EPYC-Rome model.

> 
> >
> > For our use case, I just verified that even without this patch,
> > if we pass CLZERO through libvirt CPU definition xml, like "
> > <feature policy='require' name='clzero’/>”, it gets passed on
> > to the guest. So this patch is not super critical for us.
> 
> Note that this is done automatically by libvirt if using
> mode=host-model.
> 
> --
> Eduardo