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

[Clément Léger]

On 29/11/2023 17:15, Ben Dooks wrote:
> On 29/11/2023 16:03, Clément Léger wrote:
>> From: Samuel Ortiz <sameo@xxxxxxxxxxxx>
>>
>> From: Samuel Ortiz <sameo@xxxxxxxxxxxx>
>>
>> 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>
>> Signed-off-by: Clément Léger <cleger@xxxxxxxxxxxx>
>>
>> ---
>>
>> This series depends on "riscv: report more ISA extensions through
>> hwprobe" series [1].
>>
>> Link:
>> https://lore.kernel.org/lkml/20231114141256.126749-1-cleger@xxxxxxxxxxxx/ [1]
>> ---
>>   arch/riscv/include/asm/archrandom.h | 69 +++++++++++++++++++++++++++++
>>   arch/riscv/include/asm/csr.h        |  9 ++++
>>   2 files changed, 78 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..795837ccb583
>> --- /dev/null
>> +++ b/arch/riscv/include/asm/archrandom.h
>> @@ -0,0 +1,69 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * Kernel interface for the RISCV arch_random_* functions
>> + *
>> + * Copyright (c) 2023 by Rivos Inc.
>> + *
>> + */
>> +
>> +#ifndef ASM_RISCV_ARCHRANDOM_H
>> +#define ASM_RISCV_ARCHRANDOM_H
>> +
>> +#include <asm/csr.h>
>> +
>> +#define SEED_RETRY_LOOPS 100
>> +
>> +static inline bool __must_check csr_seed_long(unsigned long *v)
>> +{
>> +    unsigned int retry = SEED_RETRY_LOOPS, valid_seeds = 0;
>> +    const int needed_seeds = sizeof(long) / sizeof(u16);
>> +    u16 *entropy = (u16 *)v;
>> +
>> +    do {
>> +        /*
>> +         * The SEED CSR must be accessed with a read-write instruction.
>> +         */
>> +        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("archrandom: Unrecoverable error\n");
>> +            return false;
>> +
>> +        case SEED_OPST_BIST:
>> +        case SEED_OPST_WAIT:
>> +        default:
>> +            continue;
> 
> is it worth adding a cpu_relax() here?

Yeah, that clearly makes sense !

> 
>> +        }
>> +    } 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)
>> +{
>> +    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))
>> +        return 1;
> 
> I'm assuming the code will retry if max_longs > 1 ?

Yes the caller will call it until gathering enough data (see
drivers/char/random.c).

Thanks,

Clément

> 
> 
> 
>> +    return 0;
>> +}
>> +
>> +#endif /* ASM_RISCV_ARCHRANDOM_H */
>> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
>> index 306a19a5509c..510014051f5d 100644
>> --- a/arch/riscv/include/asm/csr.h
>> +++ b/arch/riscv/include/asm/csr.h
>> @@ -411,6 +411,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
>