Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 10 Dec 2020 17:03:44 -0800
From:   Sean Christopherson <seanjc@google.com>
To:     Kyung Min Park <kyung.min.park@intel.com>
Cc:     x86@kernel.org, linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, hpa@zytor.com,
        pbonzini@redhat.com, sean.j.christopherson@intel.com,
        jmattson@google.com, joro@8bytes.org, vkuznets@redhat.com,
        wanpengli@tencent.com, cathy.zhang@intel.com
Subject: Re: [PATCH 2/2] x86: Expose AVX512_FP16 for supported CPUID
Message-ID: <X9LFcPchzKA4cTMF@google.com>
References: <20201208033441.28207-1-kyung.min.park@intel.com>
 <20201208033441.28207-3-kyung.min.park@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20201208033441.28207-3-kyung.min.park@intel.com>
Precedence: bulk

Shortlog should use "KVM: x86: ...", and probably s/for/in.  It currently reads
like the kernel is exposing the flag to KVM for KVM's supported CPUID, e.g.:

  KVM: x86: Expose AVX512_FP16 in supported CPUID

On Mon, Dec 07, 2020, Kyung Min Park wrote:
> From: Cathy Zhang <cathy.zhang@intel.com>
> 
> AVX512_FP16 is supported by Intel processors, like Sapphire Rapids.
> It could gain better performance for it's faster compared to FP32
> while meets the precision or magnitude requirement. It's availability
> is indicated by CPUID.(EAX=7,ECX=0):EDX[bit 23].
> 
> Expose it in KVM supported CPUID, then guest could make use of it.

For new features like this that don't require additional KVM enabling, it would
be nice to explicitly state as much in the changelog, along with a brief
explanation of why additional KVM enabling is not necessary.  It doesn't have to
be much, just something to help people that aren't already familiar with FP16
understand what this patch actually exposes to the guest.  E.g. I assume there
are new instructions that are available with FP16?

> Signed-off-by: Cathy Zhang <cathy.zhang@intel.com>
> Signed-off-by: Kyung Min Park <kyung.min.park@intel.com>
> Acked-by: Dave Hansen <dave.hansen@intel.com>
> Reviewed-by: Tony Luck <tony.luck@intel.com>
> ---
>  arch/x86/kvm/cpuid.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index e83bfe2daf82..d7707cfc9401 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -416,7 +416,7 @@ void kvm_set_cpu_caps(void)
>  		F(AVX512_4VNNIW) | F(AVX512_4FMAPS) | F(SPEC_CTRL) |
>  		F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES) | F(INTEL_STIBP) |
>  		F(MD_CLEAR) | F(AVX512_VP2INTERSECT) | F(FSRM) |
> -		F(SERIALIZE) | F(TSXLDTRK)
> +		F(SERIALIZE) | F(TSXLDTRK) | F(AVX512_FP16)
>  	);
>  
>  	/* TSC_ADJUST and ARCH_CAPABILITIES are emulated in software. */
> -- 
> 2.17.1
>