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. 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 > >> > > >