linux-kernel - Re: [PATCH v2 1/2] x86/cpufeatures: Combine word 11 and 12 into new scattered features word 11

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: <20190619212445.GA234387@romley-ivt3.sc.intel.com>
Date:   Wed, 19 Jun 2019 14:24:45 -0700
From:   Fenghua Yu <fenghua.yu@...el.com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, H Peter Anvin <hpa@...or.com>,
        Christopherson Sean J <sean.j.christopherson@...el.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Radim Krcmar <rkrcmar@...hat.com>,
        Ravi V Shankar <ravi.v.shankar@...el.com>,
        linux-kernel <linux-kernel@...r.kernel.org>, x86 <x86@...nel.org>
Subject: Re: [PATCH v2 1/2] x86/cpufeatures: Combine word 11 and 12 into new
 scattered features word 11

On Wed, Jun 19, 2019 at 07:36:28PM +0200, Borislav Petkov wrote:
> On Mon, Jun 17, 2019 at 11:00:15AM -0700, Fenghua Yu wrote:
> > @@ -832,33 +857,6 @@ void get_cpu_cap(struct cpuinfo_x86 *c)
> >  		c->x86_capability[CPUID_D_1_EAX] = eax;
> > -	/* Additional Intel-defined flags: level 0x0000000F */
> > -	if (c->cpuid_level >= 0x0000000F) {
> What I meant with having a separate patch doing the carve out is to have
> a single patch doing *only* code movement - no changes, no nothing. So
> that it is clear what happens. Intermixing code movement and changes is
> a bad idea and hard to review.
> 
> IOW, I did this:
> 
> ---
> From cef4f58a3da0465bbff33b2d669cc600b775f3ba Mon Sep 17 00:00:00 2001
> From: Borislav Petkov <bp@...e.de>
> Date: Wed, 19 Jun 2019 17:24:34 +0200
> Subject: [PATCH] x86/cpufeatures: Carve out CQM features retrieval
> 
> ... into a separate function for better readability. Split out from a
> patch from Fenghua Yu <fenghua.yu@...el.com> to keep the mechanical,
> sole code movement separate for easy review.
> 
> No functional changes.
> 
> Signed-off-by: Borislav Petkov <bp@...e.de>
> Cc: Fenghua Yu <fenghua.yu@...el.com>
> Cc: x86@...nel.org
> ---
>  arch/x86/kernel/cpu/common.c | 60 ++++++++++++++++++++----------------
>  1 file changed, 33 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 2c57fffebf9b..fe6ed9696467 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -801,6 +801,38 @@ static void init_speculation_control(struct cpuinfo_x86 *c)
>  	}
>  }
>  
> +static void init_cqm(struct cpuinfo_x86 *c)
> +{
> +	u32 eax, ebx, ecx, edx;
> 
> This way you have *pure* code movement only.
> 
> And then your second patch turns into this, which shows *exactly* what
> has been changed in init_cqm().

Yes, the added patch makes this patch set more clear and readable.

> 
> Please have a look and send me only the now third patch with corrected
> commit message.
> 
> From e33527b8cde8bef84cdc90651d1a1c7a9a5234d7 Mon Sep 17 00:00:00 2001
> From: Fenghua Yu <fenghua.yu@...el.com>
> Date: Wed, 19 Jun 2019 18:51:09 +0200
> Subject: [PATCH] x86/cpufeatures: Combine word 11 and 12 into a new
>  scattered features word
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> It's a waste for the four X86_FEATURE_CQM_* feature bits to occupy two
> whole feature bits words. To better utilize feature words, re-define
> word 11 to host scattered features and move the four X86_FEATURE_CQM_*
> features into Linux defined word 11. More scattered features can be
> added in word 11 in the future.

I checked and tested the updated patch set (now three patches). They
look much better than v2.

I will send you the now third patch with corrected commit message
in the other email thread.

Thanks.

-Fenghua