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.
Signed-off-by: Samuel Ortiz <[email protected]>
---
arch/riscv/include/asm/archrandom.h | 66 +++++++++++++++++++++++++++++
arch/riscv/include/asm/csr.h | 9 ++++
2 files changed, 75 insertions(+)
create mode 100644 arch/riscv/include/asm/archrandom.h
diff --git a/arch/riscv/include/asm/archrandom.h b/arch/riscv/include/asm/archrandom.h
new file mode 100644
index 000000000000..3d01aab2800a
--- /dev/null
+++ b/arch/riscv/include/asm/archrandom.h
@@ -0,0 +1,66 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Kernel interface for the RISCV arch_random_* functions
+ *
+ * Copyright (c) 2022 by Rivos Inc.
+ *
+ */
+
+#ifndef ASM_RISCV_ARCHRANDOM_H
+#define ASM_RISCV_ARCHRANDOM_H
+
+#include <asm/csr.h>
+
+#define PR_PREFIX "Zkr Extension: "
+#define SEED_RETRY_LOOPS 10
+
+static inline bool __must_check csr_seed_long(unsigned long *v)
+{
+ unsigned int retry = SEED_RETRY_LOOPS;
+ unsigned int needed_seeds = sizeof(unsigned long) / 2, valid_seeds = 0;
+ u16 *entropy = (u16 *)v;
+
+ do {
+ /*
+ * The SEED CSR (0x015) must be accessed with a read-write
+ * instruction. Moreover, implementations must ignore the write
+ * value, its purpose is to signal polling for new seed.
+ */
+ unsigned long csr_seed = csr_swap(CSR_SEED, 0);
+
+ switch (csr_seed & SEED_OPST_MASK) {
+ case SEED_OPST_ES16:
+ entropy[valid_seeds++] = csr_seed & SEED_ENTROPY_MASK;
+ if (valid_seeds == needed_seeds)
+ return true;
+ break;
+
+ case SEED_OPST_DEAD:
+ pr_err_once(PR_PREFIX "Unrecoverable error\n");
+ return false;
+
+ case SEED_OPST_BIST:
+ pr_info(PR_PREFIX "On going Built-in Self Test\n");
+ fallthrough;
+
+ case SEED_OPST_WAIT:
+ default:
+ continue;
+ }
+
+ } while (--retry);
+
+ return false;
+}
+
+static inline size_t __must_check arch_get_random_longs(unsigned long *v, size_t max_longs)
+{
+ return 0;
+}
+
+static inline size_t __must_check arch_get_random_seed_longs(unsigned long *v, size_t max_longs)
+{
+ return max_longs && riscv_isa_extension_available(NULL, ZKR) && csr_seed_long(v) ? 1 : 0;
+}
+
+#endif /* ASM_RISCV_ARCHRANDOM_H */
diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
index b98b3b6c9da2..7d0ca9082c66 100644
--- a/arch/riscv/include/asm/csr.h
+++ b/arch/riscv/include/asm/csr.h
@@ -389,6 +389,15 @@
#define CSR_VTYPE 0xc21
#define CSR_VLENB 0xc22
+/* Scalar Crypto Extension - Entropy */
+#define CSR_SEED 0x015
+#define SEED_OPST_MASK _AC(0xC0000000, UL)
+#define SEED_OPST_BIST _AC(0x00000000, UL)
+#define SEED_OPST_WAIT _AC(0x40000000, UL)
+#define SEED_OPST_ES16 _AC(0x80000000, UL)
+#define SEED_OPST_DEAD _AC(0xC0000000, UL)
+#define SEED_ENTROPY_MASK _AC(0xFFFF, UL)
+
#ifdef CONFIG_RISCV_M_MODE
# define CSR_STATUS CSR_MSTATUS
# define CSR_IE CSR_MIE
--
2.41.0
Hey Samuel,
On Tue, Jun 27, 2023 at 04:37:44PM +0200, Samuel Ortiz wrote:
> 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.
>
> Signed-off-by: Samuel Ortiz <[email protected]>
> ---
> arch/riscv/include/asm/archrandom.h | 66 +++++++++++++++++++++++++++++
> arch/riscv/include/asm/csr.h | 9 ++++
> 2 files changed, 75 insertions(+)
> create mode 100644 arch/riscv/include/asm/archrandom.h
>
> diff --git a/arch/riscv/include/asm/archrandom.h b/arch/riscv/include/asm/archrandom.h
> new file mode 100644
> index 000000000000..3d01aab2800a
> --- /dev/null
> +++ b/arch/riscv/include/asm/archrandom.h
> @@ -0,0 +1,66 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Kernel interface for the RISCV arch_random_* functions
> + *
> + * Copyright (c) 2022 by Rivos Inc.
> + *
> + */
> +
> +#ifndef ASM_RISCV_ARCHRANDOM_H
> +#define ASM_RISCV_ARCHRANDOM_H
> +
> +#include <asm/csr.h>
> +
> +#define PR_PREFIX "Zkr Extension: "
Does pr_fmt() not work for you?
Also, "Zkr Extension" doesn't really seem super helpful to a punter if
they saw it in a log. Why not s/Zkr Extension/archrandom/, or similar?
> +#define SEED_RETRY_LOOPS 10
> +
> +static inline bool __must_check csr_seed_long(unsigned long *v)
> +{
> + unsigned int retry = SEED_RETRY_LOOPS;
> + unsigned int needed_seeds = sizeof(unsigned long) / 2, valid_seeds = 0;
> + u16 *entropy = (u16 *)v;
> +
> + do {
> + /*
> + * The SEED CSR (0x015) must be accessed with a read-write
> + * instruction. Moreover, implementations must ignore the write
> + * value, its purpose is to signal polling for new seed.
> + */
What relevance does the second half of this comment have to the kernel?
> + unsigned long csr_seed = csr_swap(CSR_SEED, 0);
> +
> + switch (csr_seed & SEED_OPST_MASK) {
> + case SEED_OPST_ES16:
> + entropy[valid_seeds++] = csr_seed & SEED_ENTROPY_MASK;
> + if (valid_seeds == needed_seeds)
> + return true;
> + break;
> +
> + case SEED_OPST_DEAD:
> + pr_err_once(PR_PREFIX "Unrecoverable error\n");
> + return false;
> +
> + case SEED_OPST_BIST:
> + pr_info(PR_PREFIX "On going Built-in Self Test\n");
tiny nit, "On-going"? My OCD is bother by the capitalisation otherwise.
> + fallthrough;
> +
> + case SEED_OPST_WAIT:
> + default:
> + continue;
> + }
> +
> + } while (--retry);
> +
> + return false;
> +}
> +
> +static inline size_t __must_check arch_get_random_longs(unsigned long *v, size_t max_longs)
> +{
> + return 0;
> +}
> +
> +static inline size_t __must_check arch_get_random_seed_longs(unsigned long *v, size_t max_longs)
> +{
> + return max_longs && riscv_isa_extension_available(NULL, ZKR) && csr_seed_long(v) ? 1 : 0;
Could you please write this in a more readable way, even if that is
going to be a lot more verbose? I know you copied it from x86, but
that's not really an excuse ;)
Also, is there a reason that you opted not to use the alternative-backed
riscv_has_extension_[un]likely() here? The ones backed by alternatives
are preferred if you are not checking on a per-cpu, or once-off basis.
Cheers,
Conor.
> +}
> +
> +#endif /* ASM_RISCV_ARCHRANDOM_H */
> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> index b98b3b6c9da2..7d0ca9082c66 100644
> --- a/arch/riscv/include/asm/csr.h
> +++ b/arch/riscv/include/asm/csr.h
> @@ -389,6 +389,15 @@
> #define CSR_VTYPE 0xc21
> #define CSR_VLENB 0xc22
>
> +/* Scalar Crypto Extension - Entropy */
> +#define CSR_SEED 0x015
> +#define SEED_OPST_MASK _AC(0xC0000000, UL)
> +#define SEED_OPST_BIST _AC(0x00000000, UL)
> +#define SEED_OPST_WAIT _AC(0x40000000, UL)
> +#define SEED_OPST_ES16 _AC(0x80000000, UL)
> +#define SEED_OPST_DEAD _AC(0xC0000000, UL)
> +#define SEED_ENTROPY_MASK _AC(0xFFFF, UL)
> +
> #ifdef CONFIG_RISCV_M_MODE
> # define CSR_STATUS CSR_MSTATUS
> # define CSR_IE CSR_MIE
> --
> 2.41.0
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv
On Tue, Jun 27, 2023, at 10:37 AM, Samuel Ortiz wrote:
> 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.
>
> Signed-off-by: Samuel Ortiz <[email protected]>
> ---
> arch/riscv/include/asm/archrandom.h | 66 +++++++++++++++++++++++++++++
> arch/riscv/include/asm/csr.h | 9 ++++
> 2 files changed, 75 insertions(+)
> create mode 100644 arch/riscv/include/asm/archrandom.h
>
> diff --git a/arch/riscv/include/asm/archrandom.h
> b/arch/riscv/include/asm/archrandom.h
> new file mode 100644
> index 000000000000..3d01aab2800a
> --- /dev/null
> +++ b/arch/riscv/include/asm/archrandom.h
> @@ -0,0 +1,66 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Kernel interface for the RISCV arch_random_* functions
> + *
> + * Copyright (c) 2022 by Rivos Inc.
> + *
> + */
> +
> +#ifndef ASM_RISCV_ARCHRANDOM_H
> +#define ASM_RISCV_ARCHRANDOM_H
> +
> +#include <asm/csr.h>
> +
> +#define PR_PREFIX "Zkr Extension: "
> +#define SEED_RETRY_LOOPS 10
> +
> +static inline bool __must_check csr_seed_long(unsigned long *v)
> +{
> + unsigned int retry = SEED_RETRY_LOOPS;
> + unsigned int needed_seeds = sizeof(unsigned long) / 2, valid_seeds =
> 0;
> + u16 *entropy = (u16 *)v;
> +
> + do {
> + /*
> + * The SEED CSR (0x015) must be accessed with a read-write
> + * instruction. Moreover, implementations must ignore the write
> + * value, its purpose is to signal polling for new seed.
> + */
> + unsigned long csr_seed = csr_swap(CSR_SEED, 0);
> +
> + switch (csr_seed & SEED_OPST_MASK) {
> + case SEED_OPST_ES16:
> + entropy[valid_seeds++] = csr_seed & SEED_ENTROPY_MASK;
> + if (valid_seeds == needed_seeds)
> + return true;
> + break;
> +
> + case SEED_OPST_DEAD:
> + pr_err_once(PR_PREFIX "Unrecoverable error\n");
> + return false;
> +
> + case SEED_OPST_BIST:
> + pr_info(PR_PREFIX "On going Built-in Self Test\n");
> + fallthrough;
> +
> + case SEED_OPST_WAIT:
> + default:
> + continue;
> + }
> +
> + } while (--retry);
> +
> + return false;
> +}
The Entropy Source specification is annoyingly vague about expected retry
counts, only saying that "Without a polling-style mechanism, the entropy
source could hang for thousands of cycles under some circumstances."
Likewise no constraint is placed on the maximum runtime of a BIST or the
maximum number of times SEED_OPST_BIST is repeatedly returned (only that
it be returned at least once if the BIST starts and finishes between seed
reads).
With that, the limit of 10 reads seems suspiciously small. Is there a
specific justification or is it known to work on some hardware?
-s
> +
> +static inline size_t __must_check arch_get_random_longs(unsigned long
> *v, size_t max_longs)
> +{
> + return 0;
> +}
> +
> +static inline size_t __must_check arch_get_random_seed_longs(unsigned
> long *v, size_t max_longs)
> +{
> + return max_longs && riscv_isa_extension_available(NULL, ZKR) &&
> csr_seed_long(v) ? 1 : 0;
> +}
> +
> +#endif /* ASM_RISCV_ARCHRANDOM_H */
> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> index b98b3b6c9da2..7d0ca9082c66 100644
> --- a/arch/riscv/include/asm/csr.h
> +++ b/arch/riscv/include/asm/csr.h
> @@ -389,6 +389,15 @@
> #define CSR_VTYPE 0xc21
> #define CSR_VLENB 0xc22
>
> +/* Scalar Crypto Extension - Entropy */
> +#define CSR_SEED 0x015
> +#define SEED_OPST_MASK _AC(0xC0000000, UL)
> +#define SEED_OPST_BIST _AC(0x00000000, UL)
> +#define SEED_OPST_WAIT _AC(0x40000000, UL)
> +#define SEED_OPST_ES16 _AC(0x80000000, UL)
> +#define SEED_OPST_DEAD _AC(0xC0000000, UL)
> +#define SEED_ENTROPY_MASK _AC(0xFFFF, UL)
> +
> #ifdef CONFIG_RISCV_M_MODE
> # define CSR_STATUS CSR_MSTATUS
> # define CSR_IE CSR_MIE
> --
> 2.41.0
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv
Hi Conor,
On Tue, Jun 27, 2023 at 08:09:08PM +0100, Conor Dooley wrote:
> On Tue, Jun 27, 2023 at 04:37:44PM +0200, Samuel Ortiz wrote:
> > 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.
> >
> > Signed-off-by: Samuel Ortiz <[email protected]>
> > ---
> > arch/riscv/include/asm/archrandom.h | 66 +++++++++++++++++++++++++++++
> > arch/riscv/include/asm/csr.h | 9 ++++
> > 2 files changed, 75 insertions(+)
> > create mode 100644 arch/riscv/include/asm/archrandom.h
> >
> > diff --git a/arch/riscv/include/asm/archrandom.h b/arch/riscv/include/asm/archrandom.h
> > new file mode 100644
> > index 000000000000..3d01aab2800a
> > --- /dev/null
> > +++ b/arch/riscv/include/asm/archrandom.h
> > @@ -0,0 +1,66 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Kernel interface for the RISCV arch_random_* functions
> > + *
> > + * Copyright (c) 2022 by Rivos Inc.
> > + *
> > + */
> > +
> > +#ifndef ASM_RISCV_ARCHRANDOM_H
> > +#define ASM_RISCV_ARCHRANDOM_H
> > +
> > +#include <asm/csr.h>
> > +
> > +#define PR_PREFIX "Zkr Extension: "
>
> Does pr_fmt() not work for you?
It would, but since this is a header that e.g. gets included in
random.h, I would have to ifndef it first.
My v2 is less verbose and gets rid of this prefix anyways.
> Also, "Zkr Extension" doesn't really seem super helpful to a punter if
> they saw it in a log. Why not s/Zkr Extension/archrandom/, or similar?
>
> > +#define SEED_RETRY_LOOPS 10
> > +
> > +static inline bool __must_check csr_seed_long(unsigned long *v)
> > +{
> > + unsigned int retry = SEED_RETRY_LOOPS;
> > + unsigned int needed_seeds = sizeof(unsigned long) / 2, valid_seeds = 0;
> > + u16 *entropy = (u16 *)v;
> > +
> > + do {
> > + /*
> > + * The SEED CSR (0x015) must be accessed with a read-write
> > + * instruction. Moreover, implementations must ignore the write
> > + * value, its purpose is to signal polling for new seed.
> > + */
>
> What relevance does the second half of this comment have to the kernel?
Not much, I will remove it with v2.
> > + unsigned long csr_seed = csr_swap(CSR_SEED, 0);
> > +
> > + switch (csr_seed & SEED_OPST_MASK) {
> > + case SEED_OPST_ES16:
> > + entropy[valid_seeds++] = csr_seed & SEED_ENTROPY_MASK;
> > + if (valid_seeds == needed_seeds)
> > + return true;
> > + break;
> > +
> > + case SEED_OPST_DEAD:
> > + pr_err_once(PR_PREFIX "Unrecoverable error\n");
> > + return false;
> > +
> > + case SEED_OPST_BIST:
> > + pr_info(PR_PREFIX "On going Built-in Self Test\n");
>
> tiny nit, "On-going"? My OCD is bother by the capitalisation otherwise.
I removed that one with v2, as it may be a little too chatty.
> > + fallthrough;
> > +
> > + case SEED_OPST_WAIT:
> > + default:
> > + continue;
> > + }
> > +
> > + } while (--retry);
> > +
> > + return false;
> > +}
> > +
> > +static inline size_t __must_check arch_get_random_longs(unsigned long *v, size_t max_longs)
> > +{
> > + return 0;
> > +}
> > +
> > +static inline size_t __must_check arch_get_random_seed_longs(unsigned long *v, size_t max_longs)
> > +{
> > + return max_longs && riscv_isa_extension_available(NULL, ZKR) && csr_seed_long(v) ? 1 : 0;
>
> Could you please write this in a more readable way, even if that is
> going to be a lot more verbose? I know you copied it from x86, but
> that's not really an excuse ;)
Agreed :)
Fixed as well with v2.
> Also, is there a reason that you opted not to use the alternative-backed
> riscv_has_extension_[un]likely() here?
I did not know about them, I switched to it with v2 (Made it a likely
case).
Cheers,
Samuel.