From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.linutronix.de (193.142.43.55:993) by crypto-ml.lab.linutronix.de with IMAP4-SSL for ; 28 Feb 2020 17:38:52 -0000 Received: from mga06.intel.com ([134.134.136.31]) by Galois.linutronix.de with esmtps (TLS1.2:DHE_RSA_AES_256_CBC_SHA256:256) (Exim 4.80) (envelope-from ) id 1j7jb8-0003iR-IG for speck@linutronix.de; Fri, 28 Feb 2020 18:38:51 +0100 Received: from localhost (mtg-dev.jf.intel.com [10.54.74.10]) by smtp.ostc.intel.com (Postfix) with ESMTP id EFC266361 for ; Fri, 28 Feb 2020 17:38:44 +0000 (UTC) Date: Fri, 28 Feb 2020 09:38:45 -0800 From: mark gross Subject: [MODERATED] Re: Additional sampling fun Message-ID: <20200228173845.GA2466@mtg-dev.jf.intel.com> Reply-To: mgross@linux.intel.com References: <20200220081420.GA3328448@kroah.com> <20200228162140.GB24511@zn.tnic> <20200228163447.GA3241225@kroah.com> MIME-Version: 1.0 In-Reply-To: <20200228163447.GA3241225@kroah.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit To: speck@linutronix.de List-ID: On Fri, Feb 28, 2020 at 05:34:47PM +0100, speck for Greg KH wrote: > On Fri, Feb 28, 2020 at 05:21:40PM +0100, speck for Borislav Petkov wrote: > > On Thu, Feb 20, 2020 at 09:14:20AM +0100, speck for Greg KH wrote: > > > Then we need to stop using RDRAND internally for our "give me a random > > > number api" which has spread to more and more parts of the kernel. > > > > > > Here's a patch that does so: > > > https://lore.kernel.org/lkml/20200216161836.1976-1-Jason@zx2c4.com/ > > > which I'm going to advise get merged now and backported to the stable > > > branches. > > Note, the above patch (well the v2 version) is now merged and should > show up in the next -rc1 release. > > > So one of our guys - Nicolai Stange - was looking at this > > wrt backporting it to trees and there's another problem in > > add_interrupt_randomness() which could potentially turn out > > to be nasty. > > > > We asked him to write it up for speck@ (he's not subscribed) so that we > > can discuss it here first. Here is the deal in his own words: > > > > "In the context of the get_random_long() patch posted at [1], I noticed > > that there's also a RDSEED insn issued from the interrupt path, which > > perhaps might have undesired effects performance-wise. > > > > More specifically, add_interrupt_randomness() would issue one RDSEED > > either once a second or every 64 interrupts, whichever comes first: > > > > void add_interrupt_randomness(int irq, int irq_flags) > > { > > fast_mix(fast_pool); /* increments fast_pool->count */ > > ... > > if ((fast_pool->count < 64) && > > !time_after(now, fast_pool->last + HZ)) > > return; > > ... > > fast_pool->last = now; > > if (arch_get_random_seed_long(&seed)) {} > > ... > > } > > > > So while this certainly won't matter much on average, I'm still > > wondering whether or not this RDSEED could potentially cause IRQ > > latency spikes relevant e.g. to -RT and/or under high IRQ load? > > > > FWIW, the commit introducing the arch_get_random_seed_long() invocation > > to add_interrupt_randomness() was commit 83664a6928a4 ("random: Use > > arch_get_random_seed*() at init time and once a second"). > > > > I can only guess, but I think the motivation for mixing > > arch_get_random_seed_long() from the interrupt path was probably to > > sync that with the IRQ rate. That is, to make sure that the entropy > > mixed from RDSEED doesn't dominate the interrupt entropy source. > > > > [1] https://lkml.kernel.org/r/20200216161836.1976-1-Jason@zx2c4.com > > " > > Ugh. I think we need to drag Jason into this as well, but really, > talking about that can be done on the mailing list as there's nothing > wrong with trying to get that slow code out of the irq path today, > right? > > thanks, > > greg k-h FWIW unless someone is abusing rdrand/rdseed I don't think the impact of the mitigation will be measurable. Running multiple instances of spanking rdrand in a loop will show nonlinear impacts due to bus lock contention but, I don't think there is any contention issues with once/64IRS's or once a second. you are looking at approximately O(100cycles) vrs O(1000cycles) every second or every 64th interrupt. I don't think you'll be able to measure the impact of that. (unless you force lock contention on the HW bus lock) --mark