Re: [PATCH v3 4/4] RISC-V: Implement archrandom when Zkr is available

[Samuel Ortiz]

Hi Heiko,

On Sun, Jul 09, 2023 at 04:06:16PM +0200, Heiko Stuebner wrote:
> Am Sonntag, 9. Juli 2023, 13:55:46 CEST schrieb Samuel Ortiz:
> > The Zkr extension is ratified and provides 16 bits of entropy seed when
> > reading the SEED CSR.
> > 
> > We can implement arch_get_random_seed_longs() by doing multiple csrrw to
> > that CSR and filling an unsigned long with valid entropy bits.
> > 
> > Acked-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
> > Signed-off-by: Samuel Ortiz <sameo@xxxxxxxxxxxx>
> > ---
> 
> > +static inline size_t __must_check arch_get_random_seed_longs(unsigned long *v, size_t max_longs)
> > +{
> > +	if (!max_longs)
> > +		return 0;
> > +
> > +	/*
> > +	 * If Zkr is supported and csr_seed_long succeeds, we return one long
> > +	 * worth of entropy.
> > +	 */
> > +	if (riscv_has_extension_likely(RISCV_ISA_EXT_ZKR) && csr_seed_long(v))
> 
> While this whole thing looks really nice, I don't think you can only
> check the ZKR existence though.
> 
> To access the seed csr from supervisor-mode, it looks like the SSEED
> bit in the mseccfg register also needs to be set by firmware.
> And in the kernel we will likely need to check this setting somehow
> before enabling access.

We can't check it as msseccfg is an M-mode only CSR. While reviewing v2
of this patchset, Stephen suggested to either document the SSEED
requirement with the dt-bindings documentation, use the SBI FWFEATURE
extension to ask firmware to set mseecfg properly, or trap seed access
and feed the caller with a virtual entropy source.
I'd like to go with the second proposed approach (FWFEATURE) but that
requires the corresponding pending patch to be merged first. So for now,
I will only document the SSEED requirement when passing the Zkr
extension, so that we at least have a contract definition for firmwares
that enable Zkr through DT. When they do, they're required to at least
set SSEED in MSSECFG.

I have a couple of pending patches ([1],[2]) related to that, so that an
OpenSBI+qemu+linux combination works as expected when enabling Zkr. I am
going to submit them upstream as well.

Cheers,
Samuel.

[1] https://github.com/qemu/qemu/commit/2a146057099ada946bf4a9c2e355a5a290c23c80
[2] https://github.com/riscv-software-src/opensbi/pull/315