All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jason A. Donenfeld" <Jason@zx2c4.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Borislav Petkov <bp@alien8.de>,
	LKML <linux-kernel@vger.kernel.org>,
	x86@kernel.org, Filipe Manana <fdmanana@suse.com>,
	linux-crypto@vger.kernel.org
Subject: Re: [patch 3/3] x86/fpu: Make FPU protection more robust
Date: Thu, 5 May 2022 01:52:32 +0200	[thread overview]
Message-ID: <YnMRwPFfvB0RlBow@zx2c4.com> (raw)
In-Reply-To: <87wnf1huwj.ffs@tglx>

Hi Thomas,

On Wed, May 04, 2022 at 11:04:12PM +0200, Thomas Gleixner wrote:

> > The second stance seems easier and more conservative from a certain
> > perspective -- we don't need to change anything -- so I'm more inclined
> > toward it.
> 
> That's not conservative, that's lazy and lame. Staying with the status
> quo and piling more stuff on top because we can is just increasing
> technical debt. Works for a while by some definition of works.

I actually find this minorly upsetting :(. Considering the amount of
technical debt I've been tirelessly cleaning up over the last 5 months,
"lazy" certainly can't be correct here. None of this has anything to do
with laziness, but rather how the entropy collection logic works out.
It'd be lazy to just wave my hands around and say, "oh well it's handled
in add_interrupt_randomness() anyway, so let's torch the other thing,"
without having taken the time to do the analysis to see if that's
actually true or not. One way or another, it _will_ require real
analysis. Which obviously I'm volunteering to do here (it's an
interesting question to me) and have already started poking around with
it.

> > And given that you've fixed the bug now, it sounds like that's fine
> > with you too. But if you're thinking about it differently in fact, let
> > me know.
> 
> That still does not address my observation that using the FPU for this
> mixing, which is handling a couple of bytes per invocation, is not
> really benefitial.
> 
> Which in turn bears the question, why we have to maintain an asymmetric
> FPU protection mechanism in order to support hard interrupt FPU usage
> for no or questionable benefit.
> 
> The current implementation, courtesy to hard interrupt support, has the
> following downside:
> 
>   Any FPU usage in task context where soft interrupts are enabled will
>   prevent FPU usage in soft interrupt processing when the interrupt hits
>   into the FPU usage region. That means the softirq processing has to
>   fall back to the generic implementations.
> 
> Sure, the protection could be context dependent, but that's generally
> frowned upon. If we go there, then there has to be a really convincing
> technical argument.

That's curious about the softirq; I hadn't realized that. Indeed it
sounds like the technical burden of supporting it may not be worth it.
From the perspective of high-speed crypto, I know two areas pretty well:
pacrypt/padata and wireguard's queueing. Both of these run in workqueues
pinned to a CPU with queue_work_on(). In some cases, I believe locks may
be held during various crypto operations though. Also, I suspect some
paths of xfrm and mac80211 may process during softirq. Anyway, none of
those cases is hardirq. I haven't looked at dmcrypt, but I'd be
surprised if that was doing anything in hardirqs either.

So if truly the only user of this is random.c as of 5.18 (is it? I'm
assuming from a not very thorough survey...), and if the performance
boost doesn't even exist, then yeah, I think it'd make sense to just get
rid of it, and have kernel_fpu_usable() return false in those cases.

I'll run some benchmarks on a little bit more hardware in representative
cases and see.

Jason

  reply	other threads:[~2022-05-05  0:01 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-01 19:31 [patch 0/3] x86/fpu: Prevent FPU state corruption Thomas Gleixner
2022-05-01 19:31 ` [patch 1/3] " Thomas Gleixner
2022-05-02 13:16   ` Borislav Petkov
2022-05-05  0:42   ` [tip: x86/urgent] " tip-bot2 for Thomas Gleixner
2022-05-01 19:31 ` [patch 2/3] x86/fpu: Rename irq_fpu_usable() Thomas Gleixner
2022-05-02 13:57   ` Borislav Petkov
2022-05-01 19:31 ` [patch 3/3] x86/fpu: Make FPU protection more robust Thomas Gleixner
2022-05-02 14:35   ` Borislav Petkov
2022-05-02 15:58     ` Thomas Gleixner
2022-05-03  9:06       ` Peter Zijlstra
2022-05-04 15:36         ` Thomas Gleixner
2022-05-04 15:55           ` Jason A. Donenfeld
2022-05-04 16:45             ` Thomas Gleixner
2022-05-04 19:05               ` Jason A. Donenfeld
2022-05-04 21:04                 ` Thomas Gleixner
2022-05-04 23:52                   ` Jason A. Donenfeld [this message]
2022-05-05  0:55                     ` Thomas Gleixner
2022-05-05  1:11                       ` Jason A. Donenfeld
2022-05-05  1:21                         ` Thomas Gleixner
2022-05-05 11:02                           ` Jason A. Donenfeld
2022-05-05 11:34                             ` David Laight
2022-05-05 11:35                               ` Jason A. Donenfeld
2022-05-05 11:53                                 ` David Laight
2022-05-06 22:34                               ` Jason A. Donenfeld
2022-05-07 13:50                                 ` David Laight
2022-05-05 13:48                             ` Jason A. Donenfeld
2022-05-06 22:15                 ` Jason A. Donenfeld
2022-05-03  9:03   ` Peter Zijlstra
2022-05-02 10:02 ` [patch 0/3] x86/fpu: Prevent FPU state corruption Filipe Manana
2022-05-02 12:22   ` Borislav Petkov
2022-05-04 15:40 ` Jason A. Donenfeld
2022-05-04 18:05   ` Thomas Gleixner
2022-05-18  1:02   ` Jason A. Donenfeld
2022-05-18 11:14     ` Jason A. Donenfeld
2022-05-18 11:18       ` Jason A. Donenfeld
2022-05-18 13:09     ` Thomas Gleixner
2022-05-18 14:08       ` Jason A. Donenfeld
2022-05-25 20:36         ` Jason A. Donenfeld

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YnMRwPFfvB0RlBow@zx2c4.com \
    --to=jason@zx2c4.com \
    --cc=bp@alien8.de \
    --cc=fdmanana@suse.com \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.