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

[Ani Sinha <ani.sinha@nutanix.com>]

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://www.amd.com/system/files/TechDocs/24594.pdf , 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.


> 
> 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?

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.

Thanks,
Ani


> 
> -- 
> Eduardo