linux-kernel - Re: [Patch v3 08/13] x86/speculation: Rename SSBD update functions

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:   Thu, 18 Oct 2018 15:37:12 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Tim Chen <tim.c.chen@...ux.intel.com>
cc:     Jiri Kosina <jikos@...nel.org>,
        Tom Lendacky <thomas.lendacky@....com>,
        Ingo Molnar <mingo@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        David Woodhouse <dwmw@...zon.co.uk>,
        Andi Kleen <ak@...ux.intel.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Casey Schaufler <casey.schaufler@...el.com>,
        Asit Mallick <asit.k.mallick@...el.com>,
        Arjan van de Ven <arjan@...ux.intel.com>,
        Jon Masters <jcm@...hat.com>, linux-kernel@...r.kernel.org,
        x86@...nel.org
Subject: Re: [Patch v3 08/13] x86/speculation: Rename SSBD update functions

On Wed, 17 Oct 2018, Tim Chen wrote:

> This is a clean up patch that doesn't change any

This is not a cleanup. It's a preparatory patch and again you want to
explain first the context and then what you are doing.

> functionality.  We rename intel_set_ssb_state to
> spec_ctrl_update_msr, speculative_store_bypass_update to
> speculation_ctrl_update and speculative_store_bypass_update_current to
> speculation_ctrl_update_current.

There is no point in listing all what the patch is doing. We all can see
that from the patch itself. And again 'We'? We are doing nothing. Please
write your changelogs in imperative mode, like Documentation/process
requires.

> This prepares us to update of other bits in the SPEC_CTRL MSR
> dynamically.

Suggestion:

  The speculative store bypass related functions are controlling the SSBD
  bit in SPEC_CTRL_MSR depending on a TIF flag. 

  For more fine grained control of STIBP this control needs to be extended,
  so the SSB naming convention does not apply anymore.

  Rename the functions to a more generic name.

See?

Looks good otherwise.

Thanks,

	tglx