virtio-rng: Register /dev/urandom driver if CONFIG_DEV_URANDOM=y

[tmedicci]

Summary

• virtio-rng: Register /dev/urandom driver if CONFIG_DEV_URANDOM=y

Virtio RNG support (CONFIG_DRIVERS_VIRTIO_RNG=y) selects CONFIG_ARCH_HAVE_RNG. On the other hand, if CONFIG_DEV_URANDOM=y, it defaults to CONFIG_DEV_URANDOM_ARCH if CONFIG_ARCH_HAVE_RNG=y. DEV_URANDOM_ARCH definition states that the implementation of the /dev/urandom should be provided by the architecture-specific logic, including the function devurandom_register(). In this case, the /dev/urandom may refer to the same driver as /dev/random that is provided by the Virtio RNG driver.

Impact

Enables setting CONFIG_DEV_URANDOM=y for configs that use the Virtio RNG driver, based on the same driver of /dev/random.

Testing

Internal CI testing + rv-virt:netnsh:

How to reproduce the test

Building rv-virt:netnsh with CONFIG_DEV_URANDOM=y before this PR fails:

$ make -j distclean && ./tools/configure.sh rv-virt:netnsh && kconfig-tweak -e DEV_URANDOM && make olddefconfig && make -j$(nproc)
(...)
CPP:  /home/tiago/Documents/work/espressif/projects/nuttx/nuttxspace_alfa/nuttx/boards/risc-v/qemu-rv/rv-virt/scripts/ld.script-> /home/tiago/Documents/work/espressif/projects/nuttx/nuttxspace_alfa/nuttx/boards/risc-v/qemu-rv/rv-virt/scLD: nuttx
riscv-none-elf-ld: warning: /home/tiago/Documents/work/espressif/projects/nuttx/nuttxspace_alfa/nuttx/nuttx has a LOAD segment with RWX permissions
riscv-none-elf-ld: /home/tiago/Documents/work/espressif/projects/nuttx/nuttxspace_alfa/nuttx/staging/libdrivers.a(drivers_initialize.o): in function `drivers_initialize':
/home/tiago/Documents/work/espressif/projects/nuttx/nuttxspace_alfa/nuttx/drivers/drivers_initialize.c:124:(.text.drivers_initialize+0xe): undefined reference to `devurandom_register'
Memory region         Used Size  Region Size  %age Used
make[1]: *** [Makefile:191: nuttx] Error 1
make: *** [tools/Unix.mk:551: nuttx] Error 2

After this PR, the build finish successfully and the /dev/urandom is properly registered:

$ qemu-system-riscv32 -semihosting -M virt,aclint=on -cpu rv32 -smp 8 \
  -global virtio-mmio.force-legacy=false \
  -device virtio-serial-device,bus=virtio-mmio-bus.0 \
  -chardev socket,telnet=on,host=127.0.0.1,port=3450,server=on,wait=off,id=foo \
  -device virtconsole,chardev=foo \
  -device virtio-rng-device,bus=virtio-mmio-bus.1 \
  -netdev user,id=u1,hostfwd=tcp:127.0.0.1:10023-10.0.2.15:23,hostfwd=tcp:127.0.0.1:15001-10.0.2.15:5001 \
  -device virtio-net-device,netdev=u1,bus=virtio-mmio-bus.2 \
  -drive file=./mydisk-1gb.img,if=none,format=raw,id=hd \
  -device virtio-blk-device,bus=virtio-mmio-bus.3,drive=hd \
  -bios none -kernel ./nuttx -nographic
telnetd [4:100]

NuttShell (NSH) NuttX-10.4.0
nsh> ls /dev/
/dev:
 console
 null
 random
 telnet
 ttyS0
 ttyV0
 urandom
 virtblk0
 zero

[nuttxpr]

[Experimental Bot, please feedback here]

This PR appears to meet the NuttX requirements.

Summary Meets Requirements?

YES. The summary clearly explains the why, what, and how of the change. It links the motivating factor (virtio-rng enabling CONFIG_ARCH_HAVE_RNG) to the consequence (requiring devurandom_register() if CONFIG_DEV_URANDOM is also enabled). It concisely describes the solution (registering /dev/urandom using the virtio-rng driver).

Impact Meets Requirements?

MOSTLY. While the high-level impact is clear, the individual impact sections could be more explicit. Here's a suggested improvement:

• Is new feature added? Is existing feature changed?: No new feature, enabling an existing feature (CONFIG_DEV_URANDOM) in combination with virtio-rng.
• Impact on user: YES. Users can now enable CONFIG_DEV_URANDOM when using virtio-rng, providing access to /dev/urandom.
• Impact on build: NO. The build process itself doesn't change, but successful builds are now possible in configurations where they previously failed.
• Impact on hardware: NO. No changes to hardware support.
• Impact on documentation: YES. Documentation should be updated to reflect that CONFIG_DEV_URANDOM is supported with virtio-rng. (It's not clear from the PR description if this documentation update is included).
• Impact on security: Potentially YES. Enabling /dev/urandom impacts the system's entropy source. This should be explicitly addressed, even if the impact is considered minor because it's using the same underlying driver as /dev/random.
• Impact on compatibility: NO. No backward compatibility issues. Forward and interoperability impacts are not mentioned and should be considered.
• Anything else to consider?: None explicitly mentioned.

Testing Meets Requirements?

YES. The testing section clearly explains how to reproduce the issue before the change and demonstrates the successful outcome after the change. The inclusion of the specific commands, build output snippets, and the Qemu command line is helpful. Mentioning "Internal CI testing" is good, but ideally, a link to the CI run would be beneficial.

Overall Conclusion

The PR mostly meets the requirements but could benefit from minor improvements in the Impact section to be fully compliant. Specifically, being more explicit about the individual impact categories and addressing documentation, security, and compatibility more thoroughly. Adding a link to the CI results would also be a good practice.