From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH v3] x86/cpuid: expose AVX512_4VNNIW and AVX512_4FMAPS features to kvm guest Date: Fri, 4 Nov 2016 15:53:02 +0100 Message-ID: References: <1478243239-15586-1-git-send-email-he.chen@linux.intel.com> <20161104105235.qbqvmsv4ikajvjtu@pd.tnic> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, x86@kernel.org, =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Thomas Gleixner , Ingo Molnar , "H . Peter Anvin" , Luwei Kang , Piotr Luc To: Borislav Petkov , He Chen Return-path: Received: from mx1.redhat.com ([209.132.183.28]:56012 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934112AbcKDOxJ (ORCPT ); Fri, 4 Nov 2016 10:53:09 -0400 In-Reply-To: <20161104105235.qbqvmsv4ikajvjtu@pd.tnic> Sender: kvm-owner@vger.kernel.org List-ID: On 04/11/2016 11:52, Borislav Petkov wrote: > Please CC me on your future submissions, thanks. > > On Fri, Nov 04, 2016 at 03:07:19PM +0800, He Chen wrote: >> The spec can be found in Intel Software Developer Manual or in >> Instruction Set Extensions Programming Reference. > > This commit message is completely useless. Write commit messages in > the way as if you're explaining to another person *why* this change is > needed and that other person doesn't have an idea what you're doing. > >> Signed-off-by: Luwei Kang >> Signed-off-by: He Chen > > This SOB chain means what exactly? > >> --- >> Changes in v3: >> * add a helper in scattered.c to get scattered leaf. > > The modification to scattered et al without the kvm use should be a > separate patch. With no usage? Sounds like a good use of Acked-by. :) >> +u32 get_scattered_cpuid_leaf(unsigned int level, unsigned int sub_leaf, >> + enum cpuid_regs_idx reg) > > Align arguments on the opening brace. They are aligned. Paolo >> +{ >> + u32 cpuid_val = 0; >> + const struct cpuid_bit *cb; > > > Please sort function local variables declaration in a reverse christmas > tree order: > > longest_variable_name; > shorter_var_name; > even_shorter; > i; > >> + >> + for (cb = cpuid_bits; cb->feature; cb++) { >> + >> + if (level > cb->level) >> + continue; >> + >> + if (level < cb->level) >> + break; >> + >> + if (reg == cb->reg && sub_leaf == cb->sub_leaf) { >> + if (cpu_has(&boot_cpu_data, cb->feature)) >> + cpuid_val |= BIT(cb->bit); >> + } >> + } >> + >> + return cpuid_val; >> +} >