Received-SPF: pass (google.com: best guess record for domain of linux-crypto-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 14 Nov 2019 14:25:12 +0000
From:   Mark Rutland <mark.rutland@arm.com>
To:     richard.henderson@linaro.org
Cc:     linux-arm-kernel@lists.infradead.org, ard.biesheuvel@linaro.org,
        linux-crypto@vger.kernel.org
Subject: Re: [PATCH v7] arm64: Implement archrandom.h for ARMv8.5-RNG
Message-ID: <20191114142512.GC37865@lakrids.cambridge.arm.com>
References: <20191114113932.26186-1-richard.henderson@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20191114113932.26186-1-richard.henderson@linaro.org>
User-Agent: Mutt/1.11.1+11 (2f07cb52) (2018-12-01)
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk

On Thu, Nov 14, 2019 at 12:39:32PM +0100, richard.henderson@linaro.org wrote:
> +bool arch_get_random_seed_long(unsigned long *v)
> +{
> +	bool ok;
> +
> +	if (static_branch_likely(&arm64_const_caps_ready)) {
> +		if (__cpus_have_const_cap(ARM64_HAS_RNG))
> +			return arm64_rndr(v);
> +		return false;
> +	}
> +
> +	/*
> +	 * Before const_caps_ready, check the current cpu.
> +	 * This will generally be the boot cpu for rand_initialize().
> +	 */
> +	preempt_disable_notrace();
> +	ok = this_cpu_has_cap(ARM64_HAS_RNG) && arm64_rndr(v);
> +	preempt_enable_notrace();
> +
> +	return ok;
> +}

As I asked previously, please separate the common case and the boot-cpu
init-time case into separate functions.

The runtime function should just check the RNG cap before using the
instruction, without any preemption check or explicit check of
arm64_const_caps_ready. i.e.

static bool arm64_rndr(unsigned long *v)
{
	bool ok;

	if (!cpus_have_const_cap(ARM64_HAS_RNG))
		return false;

	/*
	 * Reads of RNDR set PSTATE.NZCV to 0b0000 on success,
	 * and set PSTATE.NZCV to 0b0100 otherwise.
	 */
	asm volatile(
		__mrs_s("%0", SYS_RNDR_EL0) "\n"
	"       cset %w1, ne\n"
	: "=r" (*v), "=r" (ok)
	:
	: "cc");

	return ok;
}

Any boot-time seeding should be in a separate function that external
callers cannot invoke at runtime. Either have an arch function that the
common random code calls at init time on the boot CPU, or have some
arch_add_foo_entropy() function that the arm64 code can call somewhere
around setup_arch().

Thanks,
Mark.