On Mon, Nov 06, 2023 at 05:26:01PM +0000, Andre Przywara wrote:
On Sat, 4 Nov 2023 19:45:06 +0000 Simon Glass sjg@chromium.org wrote:
Hi,
On Sat, 4 Nov 2023 at 17:13, Andre Przywara andre.przywara@arm.com wrote:
On Fri, 3 Nov 2023 13:38:58 -0600 Simon Glass sjg@chromium.org wrote:
Hi Simon,
Hi Heinrich,
On Wed, 1 Nov 2023 at 14:20, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 11/1/23 19:05, Andre Przywara wrote:
On Tue, 31 Oct 2023 14:55:50 +0200 Heinrich Schuchardt heinrich.schuchardt@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.przywara@arm.c...
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
Why do we get this? If the device is not there, the bind() function can return -ENODEV
I see this in U-Boot:
U_BOOT_DRVINFO(cpu_arm_rndr) = {
We should not use this.
Agreed.
Use the devicetree.
No, this is definitely not something for the DT, at least not on ARM. It's perfectly discoverable via the architected CPU ID registers. Similar to PCI and USB devices, which we don't probe via the DT as well.
It's arguably not proper "driver" material per se, as I've argued before, but it's the simplest solution and fits in nicely otherwise.
I was wondering if it might be something for UCLASS_CPU, something like a "CPU feature bus": to let devices register on one on the many CPU features (instead of compatible strings), then only bind() those drivers it the respective bit is set.
Does that make sense? Would that be doable without boiling the ocean? As I don't know if we see many users apart from this.
I have seen this so many times, where people want to avoid putting things in the DT and then are surprised that everything is difficult, broken and confusing. Why not just follow the rules? It is not just about whether we can avoid it, etc. It is about how devices fit together cohesively in the system, and how U-Boot operates.
A devicetree is only for peripherals *that cannot be located by probing*. Which are traditionally most peripherals in non-server Arm SoCs. While I do love the DT, the best DT node is the one you don't need.
In general, yes, this. And we keep banging against this too. If we can figure it out at run time, without needing device tree, we should be doing that, not adding a device tree node/property.
A device tree check is not our only run-time "does this exist" check.
But as Heinrich also said: those instructions are not peripherals, they are part of an instruction set extensions, the same story as with x86's RDRAND instruction. We don't have those in ACPI or so as well, because CPUID has you covered. The same on ARM, ID_AA64ISAR0_EL1 is readable on every chip (outside of EL0), and tells you whether you have the RNDR register or not. IIUC RISC-V is slightly different here, since not all ISA extensions are covered by CSRs, hence some of them indeed listed in the DT.
So a proper solution(TM) would be to split this up in architectural *instructions* and proper TRNG *devices*, maybe wrapping this up in some function that tests both. This is roughly what the kernel does, somewhat abstracted by the concept of "entropy sources", which could be TRNG devices, CPU instructions, interrupt jitter or even "instruction execution jitter"[1], with the latter two definitely not being devices really at all.
But I don't know if U-Boot wants to go through the hassle of this whole framework, as we tend to implement things much easier. But a simple get_cpu_random() function, implemented per architecture, and with some kind of success flag, should be easy enough to do. Then either the users (UEFI?) explicitly call this before trying UCLASS_RNG, or we wrap this for every RNG user.
In specifics, yes, some sort of split like this sounds good and we may or may not also want to have some option for talking with op-tee in the case where that, rather than instructions are how we do it. And a similar approach for timers with a possible need for something slightly more complex for bootstage usage?