On 11/1/23 13:49, Andre Przywara wrote:
On Wed, 1 Nov 2023 13:16:24 -0400
Sean Anderson <sean...@gmail.com> wrote:

Hi Sean,

On 11/1/23 13:05, Andre Przywara wrote:
On Tue, 31 Oct 2023 14:55:50 +0200
Heinrich Schuchardt <heinrich.schucha...@canonical.com> wrote:

Hi Heinrich,
The Zkr ISA extension (ratified Nov 2021) introduced the seed CSR. It
provides an interface to a physical entropy source.

A RNG driver based on the seed CSR is provided. It depends on
mseccfg.sseed being set in the SBI firmware.

As you might have seen, I added a similar driver for the respective Arm
functionality:
https://lore.kernel.org/u-boot/20230830113230.3925868-1-andre.przyw...@arm.com/

And I see that you seem to use the same mechanism to probe and init the
driver: U_BOOT_DRVINFO and fail in probe() if the feature is not
implemented.
One downside of this approach is that the driver is always loaded (and
visible in the DM tree), even with the feature not being available.
That doesn't seem too much of a problem on the first glance, but it
occupies a device number, and any subsequent other DM_RNG devices
(like virtio-rng) typically get higher device numbers. So without
the feature, but with virtio-rng, I get:
VExpress64# rng 0
No RNG device
VExpress64# rng 1
00000000: f3 88 b6 d4 24 da 49 ca 49 f7 9e 66 5f 12 07 b2  ....$.I.I..f_...
....

Now the EFI code always picks RNG device 0, which means we don't get
entropy in this case.

Do you have any idea how to solve this?
Maybe EFI tries to probe further - but that sounds arbitrary.
Or we find another way for probing the device, maybe via some artificial
CPU feature "bus"? There is UCLASS_CPU, but that doesn't look helpful?

If anyone has any idea, I'd be grateful.

Wouldn't the right way be to detect the hardware in bind()?

Yes, that's what I thought as well and tried, but the problem is that for
those "fixed drivers" (the ones using U_BOOT_DRVINFO) returning a
failure in bind() is fatal to the boot sequence:
============
Model: FVP Base
DRAM:  2 GiB (effective 4 GiB)
No match for driver 'arm-rndr'
initcall failed at call 00000000fef3d744 (err=-19)
### ERROR ### Please RESET the board ###

That is what a proper "CPU bus" would probably solve, as I agree that
failing bind() should be the proper solution.

Hm, so maybe this should go in riscv_cpu_bind?

--Sean

Cheers,
Andre

--Sean

Cheers,
Andre
If the seed CSR readable, is not determinable by S-mode without risking
an exception. For safe driver probing allow to resume via a longjmp
after an exception.

As the driver depends on mseccfg.sseed=1 we should wait with merging the
driver until a decision has been taken in the RISC-V PRS TG on prescribing
this.

Setting mseccfg.sseed=1 is queued for OpenSBI [1]. This has been discussed
in the RISC-V Boot & Runtime Services TG. Standardization has to be pursued
via the upcoming platform specification.

A bug fix for QEMU relating to the Zkr extension is available in [2].

A similar Linux driver has been proposed in [3].

[1] lib: sbi: Configure seed bits when MSECCFG is readable
      
https://patchwork.ozlabs.org/project/opensbi/patch/20230712083254.1585244-1-sa...@rivosinc.com/
[2] [PATCH v2 1/1] target/riscv: correct csr_ops[CSR_MSECCFG]
      
https://lore.kernel.org/qemu-devel/20231030102105.19501-1-heinrich.schucha...@canonical.com/
[3] [PATCH v4 4/4] RISC-V: Implement archrandom when Zkr is available
      
https://lore.kernel.org/linux-riscv/20230712084134.1648008-5-sa...@rivosinc.com/

v3:
        Add API documentation.
v2:
        Catch exception if mseccfg.sseed=0.

Heinrich Schuchardt (2):
    riscv: allow resume after exception
    rng: Provide a RNG based on the RISC-V Zkr ISA extension

   arch/riscv/lib/interrupts.c |  13 ++++
   doc/api/index.rst           |   1 +
   drivers/rng/Kconfig         |   8 +++
   drivers/rng/Makefile        |   1 +
   drivers/rng/riscv_zkr_rng.c | 116 ++++++++++++++++++++++++++++++++++++
   include/interrupt.h         |  45 ++++++++++++++
   6 files changed, 184 insertions(+)
   create mode 100644 drivers/rng/riscv_zkr_rng.c
   create mode 100644 include/interrupt.h



Reply via email to