linux-kernel - Re: [PATCH 1/6] x86: KVM: Enable CMPccXADD CPUID and expose it to guest

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y1AUhlwWjIkKfZHA@google.com>
Date:   Wed, 19 Oct 2022 15:15:18 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Jiaxi Chen <jiaxi.chen@...ux.intel.com>
Cc:     kvm@...r.kernel.org, tglx@...utronix.de, mingo@...hat.com,
        bp@...en8.de, dave.hansen@...ux.intel.com, x86@...nel.org,
        hpa@...or.com, pbonzini@...hat.com, ndesaulniers@...gle.com,
        alexandre.belloni@...tlin.com, peterz@...radead.org,
        jpoimboe@...nel.org, chang.seok.bae@...el.com,
        pawan.kumar.gupta@...ux.intel.com, babu.moger@....com,
        jmattson@...gle.com, sandipan.das@....com, tony.luck@...el.com,
        sathyanarayanan.kuppuswamy@...ux.intel.com, fenghua.yu@...el.com,
        keescook@...omium.org, jane.malalane@...rix.com, nathan@...nel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/6] x86: KVM: Enable CMPccXADD CPUID and expose it to
 guest

For all the shortlogs, "expose it to guest" is technically wrong.  Adding
recognition in kvm/cpuid.c advertises KVM support to host userspace.  Whether or
not a feature is exposed to the guest is up to the userspace VMM.

On Wed, Oct 19, 2022, Jiaxi Chen wrote:
> CMPccXADD is a new set of instructions in the latest Intel platform Sierra
> Forest. It includes a semaphore operation that can compare and add the

In general, avoid pronouns in changelogs, it's not clear what "it" refers to here.

And for all of these changelogs, please explicitly state that there are no VMX
controls for these instructions, assuming that's actually true.  From a KVM
perspective, that's far more interesting than the details of the instruction(s).

> operands if condition is met, which can improve database performance.
> 
> The bit definition:
> CPUID.(EAX=7,ECX=1):EAX[bit 7]
> 
> This patch enables this CPUID in the kernel feature bits and expose it to
> guest OS.

Same thing here, KVM doesn't decide whether or not to expose the feature to the
guest.

> Signed-off-by: Jiaxi Chen <jiaxi.chen@...ux.intel.com>
> ---
>  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 ef4775c6db01..445626cb5779 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -308,6 +308,7 @@
>  /* Intel-defined CPU features, CPUID level 0x00000007:1 (EAX), word 12 */
>  #define X86_FEATURE_AVX_VNNI		(12*32+ 4) /* AVX VNNI instructions */
>  #define X86_FEATURE_AVX512_BF16		(12*32+ 5) /* AVX512 BFLOAT16 instructions */
> +#define X86_FEATURE_CMPCCXADD           (12*32+ 7) /* CMPccXADD instructions */

Boris,

What do you think about moving CPUID_7_1_EAX to be a KVM-only leaf too?  AFAICT,
KVM passthrough is the only reason the existing features are defined.