Re: [PATCH] x86: Add CPUID KVM support for new instruction WBNOINVD

[Eduardo Habkost]

CCing qemu-devel.

On Tue, Sep 24, 2019 at 01:30:04PM -0700, Jim Mattson wrote:
> On Wed, Dec 19, 2018 at 1:02 PM Paolo Bonzini <address@hidden> wrote:
> >
> > On 19/12/18 18:39, Jim Mattson wrote:
> > > Is this an instruction that kvm has to be able to emulate before it
> > > can enumerate its existence?
> >
> > It doesn't have any operands, so no.
> >
> > Paolo
> >
> > > On Wed, Dec 19, 2018 at 5:51 AM Robert Hoo <address@hidden> wrote:
> > >>
> > >> Signed-off-by: Robert Hoo <address@hidden>
> > >> ---
> > >>  arch/x86/include/asm/cpufeatures.h | 1 +
> > >>  arch/x86/kvm/cpuid.c               | 2 +-
> > >>  2 files changed, 2 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/arch/x86/include/asm/cpufeatures.h 
> > >> b/arch/x86/include/asm/cpufeatures.h
> > >> index 28c4a50..932b19f 100644
> > >> --- a/arch/x86/include/asm/cpufeatures.h
> > >> +++ b/arch/x86/include/asm/cpufeatures.h
> > >> @@ -280,6 +280,7 @@
> > >>  /* AMD-defined CPU features, CPUID level 0x80000008 (EBX), word 13 */
> > >>  #define X86_FEATURE_CLZERO             (13*32+ 0) /* CLZERO instruction 
> > >> */
> > >>  #define X86_FEATURE_IRPERF             (13*32+ 1) /* Instructions 
> > >> Retired Count */
> > >> +#define X86_FEATURE_WBNOINVD           (13*32+ 9) /* Writeback and 
> > >> Don't invalid cache */
> > >>  #define X86_FEATURE_XSAVEERPTR         (13*32+ 2) /* Always 
> > >> save/restore FP error pointers */
> > >>  #define X86_FEATURE_AMD_IBPB           (13*32+12) /* "" Indirect Branch 
> > >> Prediction Barrier */
> > >>  #define X86_FEATURE_AMD_IBRS           (13*32+14) /* "" Indirect Branch 
> > >> Restricted Speculation */
> > >> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> > >> index cc6dd65..763e115 100644
> > >> --- a/arch/x86/kvm/cpuid.c
> > >> +++ b/arch/x86/kvm/cpuid.c
> > >> @@ -380,7 +380,7 @@ static inline int __do_cpuid_ent(struct 
> > >> kvm_cpuid_entry2 *entry, u32 function,
> > >>
> > >>         /* cpuid 0x80000008.ebx */
> > >>         const u32 kvm_cpuid_8000_0008_ebx_x86_features =
> > >> -               F(AMD_IBPB) | F(AMD_IBRS) | F(AMD_SSBD) | F(VIRT_SSBD) |
> > >> +               F(WBNOINVD) | F(AMD_IBPB) | F(AMD_IBRS) | F(AMD_SSBD) | 
> > >> F(VIRT_SSBD) |
> > >>                 F(AMD_SSB_NO) | F(AMD_STIBP);
> > >>
> > >>         /* cpuid 0xC0000001.edx */
> > >> --
> > >> 1.8.3.1
> > >>
> 
> What is the point of enumerating support for WBNOINVD if kvm is going
> to implement it as WBINVD?

I expect GET_SUPPORTED_CPUID to return WBNOINVD, because it
indicates to userspace what is supported by KVM.  Are there any
expectations that GET_SUPPORTED_CPUID will also dictate what is
enabled by default in some cases?

In either case, your question applies to QEMU: why do we want
WBNOINVD to be enabled by "-cpu host" by default and be part of
QEMU's Icelake-* CPU model definitions?

-- 
Eduardo