linux-kernel - Re: [PATCH 23/27] x86/fpu: Defer FPU state load until return to userspace

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 12 Apr 2019 19:29:31 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Cc:     linux-kernel@...r.kernel.org, x86@...nel.org,
        Andy Lutomirski <luto@...nel.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Radim Krčmář <rkrcmar@...hat.com>,
        kvm@...r.kernel.org, "Jason A. Donenfeld" <Jason@...c4.com>,
        Rik van Riel <riel@...riel.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>
Subject: Re: [PATCH 23/27] x86/fpu: Defer FPU state load until return to
 userspace

On Fri, Apr 12, 2019 at 07:19:55PM +0200, Sebastian Andrzej Siewior wrote:
> remove x86_fpu_activate_state.

Ok, zapping it as part of this patch.

> We could also rip all trcepoints out, rethink the situation and add new
> ones based on current code.

Well, since this changes FPU regs handling considerably, I think the
only correct step would be to adjust the tracepoints. BUT(!) we should
not forget we're not supposed to break luserspace.

> - on schedule out, we may need to save registers (depending on
>   TIF_NEED_FPU_LOAD) which is new. Before the series we always did.

That makes sense.

> - on schedule in do nothing but set that TIF bit. This is probably
>   boring.

Yah.

> - on return to userland we should load the registers but can avoid it if
>   we assume that they are valid for the current task
>   (fpregs_state_valid())

That is interesting info.

> - in kernel_fpu_begin() we may trash the task's FPU state (by saving its
>   registers or by resetting fpu_fpregs_owner_ctx).

Do we care?

You mean, in case you have workloads which might involve a lot of
in-kernel FPU use which would punish task context switches?

> Those might be interesting.
> 
> Currently we have:
>   "x86/fpu: %p load: %d xfeatures: %llx xcomp_bv: %llx"
> 
> and we have to find out what happens based on where that TP was
> recorded. Also I'm not sure if the recorded xfeatures change over time.
> I think they do not…

Good question.

> you mean trace_x86_fpu_activate_state and trace_x86_fpu_regs_activated?
> They were added in d1898b733619b ("x86/fpu: Add tracepoints to dump FPU
> state at key points") and we wouldn't have any otherwise.

I guess it made sense then...

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.