Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 5 Jul 2022 15:42:21 +0200
From:   "Jason A. Donenfeld" <Jason@zx2c4.com>
To:     Holger Dengler <dengler@linux.ibm.com>
Cc:     Harald Freudenberger <freude@linux.ibm.com>,
        Heiko Carstens <hca@linux.ibm.com>,
        Vasily Gorbik <gor@linux.ibm.com>,
        Alexander Gordeev <agordeev@linux.ibm.com>,
        Christian Borntraeger <borntraeger@linux.ibm.com>,
        Juergen Christ <jchrist@linux.ibm.com>,
        linux-crypto@vger.kernel.org, linux-s390@vger.kernel.org
Subject: Re: [PATCH v1 1/1] s390/arch_random: Buffer true random data
Message-ID: <YsQ/vZSkzWPLwIte@zx2c4.com>
References: <20220705112712.4433-1-dengler@linux.ibm.com>
 <20220705112712.4433-2-dengler@linux.ibm.com>
 <YsQ6OOrOWPhdynoM@zx2c4.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <YsQ6OOrOWPhdynoM@zx2c4.com>
Precedence: bulk

Hey again,

On Tue, Jul 5, 2022 at 3:18 PM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> On Tue, Jul 05, 2022 at 01:27:12PM +0200, Holger Dengler wrote:
> > The trng instruction is very expensive and has a constant runtime for
> > getting 0 to 32 bytes of (conditioned) true random data. Calling trng for

Thinking about this a bit more, though, makes me think that maybe the
API itself is suboptimal.

First, we never use arch_get_random_int() anywhere in the kernel, so in
theory it could be removed. Next, we could change the signature of it to
be something like:

  static inline size_t arch_get_random_words_seed(unsigned long *buf, size_t num_words);
  static inline size_t arch_get_random_words(unsigned long *buf, size_t num_words);

Since it's a static inline, when words==1, the code would be constant
folded to the same thing it is now on x86. And when it's larger, we'd
get more bang for our buck on s390.

An unfortunate complication is that the uses in random.c currently
follow the pattern of:

  if (!arch_get_random_seed_long(&v) || !arch_get_random_long(&v))
      v = random_get_entropy();

And in that way things cascade down nicely per word, depending on
current bus activity. The cascade would get a little bit uglier with
what I suggested above. So we'd need to figure out something there to
make that not hideous or result in awful codegen or something. This is,
no doubt, a snag.

Anyway, If you want to work on a tree-wide cleanup of this, I'd be happy
to consider something like that in the random.git tree for 5.20.
But, alternatively, maybe you think none of the above is really worth it
just to get more per call from the TRNG, and so emphatic "meh" on what
I've described. I can see this perspective well too.

Jason