lkml.org 
[lkml]   [2018]   [Nov]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [patch 20/24] x86/speculation: Split out TIF update

    * Thomas Gleixner <tglx@linutronix.de> wrote:

    > The update of the TIF_SSBD flag and the conditional speculation control MSR
    > update is done in the ssb_prctl_set() function directly. The upcoming prctl
    > support for controlling indirect branch speculation via STIBP needs the
    > same mechanism.
    >
    > Split the code out and make it reusable.
    >
    > Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
    > ---
    > arch/x86/kernel/cpu/bugs.c | 31 +++++++++++++++++++------------
    > 1 file changed, 19 insertions(+), 12 deletions(-)
    >
    > --- a/arch/x86/kernel/cpu/bugs.c
    > +++ b/arch/x86/kernel/cpu/bugs.c
    > @@ -703,10 +703,25 @@ static void ssb_select_mitigation(void)
    > #undef pr_fmt
    > #define pr_fmt(fmt) "Speculation prctl: " fmt
    >
    > -static int ssb_prctl_set(struct task_struct *task, unsigned long ctrl)
    > +static void task_update_spec_tif(struct task_struct *tsk, int tifbit, bool on)
    > {
    > bool update;
    >
    > + if (on)
    > + update = !test_and_set_tsk_thread_flag(tsk, tifbit);
    > + else
    > + update = test_and_clear_tsk_thread_flag(tsk, tifbit);
    > +
    > + /*
    > + * If being set on non-current task, delay setting the CPU
    > + * mitigation until it is scheduled next.
    > + */
    > + if (tsk == current && update)
    > + speculation_ctrl_update_current();

    Had to read this twice, because the comment and the code are both correct
    but deal with the inverse case. This might have helped:

    /*
    * Immediately update the speculation MSRs on the current task,
    * but for non-current tasks delay setting the CPU mitigation
    * until it is scheduled next.
    */
    if (tsk == current && update)
    speculation_ctrl_update_current();

    But can the target task ever be non-current here? I don't think so: the
    two callers are prctl and seccomp, and both are passing in the current
    task pointer.

    If so then it would be nice to rename all these task variable names from
    'task' to 'curr' or such, to make this property apparent.

    Then we can also remove the condition and the comment, and update
    unconditionally, and maybe add:

    WARN_ON_ONCE(curr != current);

    ... or so?

    Thanks,

    Ingo

    \
     
     \ /
      Last update: 2018-11-22 08:44    [W:4.019 / U:0.068 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site