Received-SPF: pass (google.com: domain of linux-kernel+bounces-44606-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240130083007.1876787-1-kirill.shutemov@linux.intel.com>
In-Reply-To: <20240130083007.1876787-1-kirill.shutemov@linux.intel.com>
From: "Jason A. Donenfeld" <Jason@zx2c4.com>
Date: Tue, 30 Jan 2024 13:29:10 +0100
Message-ID: <CAHmME9pOt=uEmuBzBpgUHw9DqAD2FZTZ3v53AOZbQ3Cd2p97xQ@mail.gmail.com>
Subject: Re: [PATCH 1/2] x86/random: Retry on RDSEED failure
To: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, 
	Dave Hansen <dave.hansen@linux.intel.com>, "H. Peter Anvin" <hpa@zytor.com>, x86@kernel.org, 
	"Theodore Ts'o" <tytso@mit.edu>, 
	Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>, 
	Elena Reshetova <elena.reshetova@intel.com>, Jun Nakajima <jun.nakajima@intel.com>, 
	Tom Lendacky <thomas.lendacky@amd.com>, "Kalra, Ashish" <ashish.kalra@amd.com>, 
	Sean Christopherson <seanjc@google.com>, linux-coco@lists.linux.dev, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Kirill,

I've been following the other discussion closely thinking about the
matter, but I suppose I'll jump in here directly on this patch, if
this is the approach the discussion is congealing around.

A comment below:

On Tue, Jan 30, 2024 at 9:30=E2=80=AFAM Kirill A. Shutemov
<kirill.shutemov@linux.intel.com> wrote:
>  static inline bool __must_check rdseed_long(unsigned long *v)
>  {
> +       unsigned int retry =3D RDRAND_RETRY_LOOPS;
>         bool ok;
> -       asm volatile("rdseed %[out]"
> -                    CC_SET(c)
> -                    : CC_OUT(c) (ok), [out] "=3Dr" (*v));
> -       return ok;
> +
> +       do {
> +               asm volatile("rdseed %[out]"
> +                            CC_SET(c)
> +                            : CC_OUT(c) (ok), [out] "=3Dr" (*v));
> +
> +               if (ok)
> +                       return true;
> +       } while (--retry);
> +
> +       return false;
>  }

So, my understanding of RDRAND vs RDSEED -- deliberately leaving out
any cryptographic discussion here -- is roughly that RDRAND will
expand the seed material for longer, while RDSEED will mostly always
try to sample more bits from the environment. AES is fast, while
sampling is slow, so RDRAND gives better performance and is less
likely to fail, whereas RDSEED always has to wait on the hardware to
collect some bits, so is more likely to fail.

For that reason, most of the usage of RDRAND and RDSEED inside of
random.c is something to the tune of `if (!rdseed(out)) rdrand(out);`,
first trying RDSEED but falling back to RDRAND if it's busy. That
still seems to me like a reasonable approach, which this patch would
partly undermine (in concert with the next patch, which I'll comment
on in a follow up email there).

So maybe this patch #1 (of 2) can be dropped?

Jason