linux-kernel - Re: [PATCH 3/4] x86/cpuid: move filter_cpuid

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]

Date:   Wed, 22 Jun 2022 08:07:46 -0700
From:   Dave Hansen <dave.hansen@...el.com>
To:     Maxim Levitsky <mlevitsk@...hat.com>, linux-kernel@...r.kernel.org
Cc:     "Chang S. Bae" <chang.seok.bae@...el.com>,
        Jiri Olsa <jolsa@...nel.org>, linux-perf-users@...r.kernel.org,
        Peter Zijlstra <peterz@...radead.org>,
        "H. Peter Anvin" <hpa@...or.com>,
        "David S. Miller" <davem@...emloft.net>,
        Borislav Petkov <bp@...en8.de>,
        Kees Cook <keescook@...omium.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Tim Chen <tim.c.chen@...ux.intel.com>,
        Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>,
        Jane Malalane <jane.malalane@...rix.com>,
        Tony Luck <tony.luck@...el.com>,
        Ingo Molnar <mingo@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        "open list:CRYPTO API" <linux-crypto@...r.kernel.org>,
        Paolo Bonzini <pbonzini@...hat.com>
Subject: Re: [PATCH 3/4] x86/cpuid: move filter_cpuid_features to cpuid-deps.c

On 6/22/22 07:48, Maxim Levitsky wrote:
> No functional change intended.

It would be really nice to at least write a "why" sentence.  You wrote
the "what" (move code), but I have no idea why you are moving it.

>  arch/x86/kernel/cpu/common.c      | 46 -----------------------------
>  arch/x86/kernel/cpu/cpuid-deps.c  | 48 +++++++++++++++++++++++++++++++

That looks a wee bit odd for a code move.  Where did the 2 lines go?

> diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
> index ea34cc31b0474f..3eb5fe0d654e63 100644
> --- a/arch/x86/include/asm/cpufeature.h
> +++ b/arch/x86/include/asm/cpufeature.h
> @@ -147,6 +147,7 @@ extern const char * const x86_bug_flags[NBUGINTS*32];
>  
>  extern void setup_clear_cpu_cap(unsigned int bit);
>  extern void clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int bit);
> +extern void filter_cpuid_features(struct cpuinfo_x86 *c, bool warn);
>  
>  #define setup_force_cpu_cap(bit) do { \
>  	set_cpu_cap(&boot_cpu_data, bit);	\
> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 4730b0a58f24a5..4cc79971d2d847 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -620,52 +620,6 @@ __noendbr void cet_disable(void)
>  		wrmsrl(MSR_IA32_S_CET, 0);
>  }
>  
> -/*
...
> -}
>  
>  /*
>   * Naming convention should be: <Name> [(<Codename>)]

One, by leaving extra whitespace.

> diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c
> index d6777d07ba3302..bcb091d02a754b 100644
> --- a/arch/x86/kernel/cpu/cpuid-deps.c
> +++ b/arch/x86/kernel/cpu/cpuid-deps.c
> @@ -131,3 +131,51 @@ void setup_clear_cpu_cap(unsigned int feature)
>  {
>  	clear_cpu_cap(&boot_cpu_data, feature);
>  }
> +
> +
> +/*

Two, by adding extra whitespace.