[PATCH v3 0/2] rng: Provide a RNG based on the RISC-V Zkr ISA extension

[Simon Glass]

Hi Tom,

On Mon, 6 Nov 2023 at 13:46, Tom Rini <trini at konsulko.com> wrote:
>
> On Mon, Nov 06, 2023 at 01:38:39PM -0700, Simon Glass wrote:
> > Hi Andre,
> >
> > On Mon, 6 Nov 2023 at 10:26, Andre Przywara <andre.przywara at arm.com> wrote:
> > >
> > > On Sat, 4 Nov 2023 19:45:06 +0000
> > > Simon Glass <sjg at chromium.org> wrote:
> > >
> > > Hi,
> > >
> > > > On Sat, 4 Nov 2023 at 17:13, Andre Przywara <andre.przywara at arm.com> wrote:
> > > > >
> > > > > On Fri, 3 Nov 2023 13:38:58 -0600
> > > > > Simon Glass <sjg at chromium.org> wrote:
> > > > >
> > > > > Hi Simon,
> > > > >
> > > > > > Hi Heinrich,
> > > > > >
> > > > > > On Wed, 1 Nov 2023 at 14:20, Heinrich Schuchardt
> > > > > > <heinrich.schuchardt at 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 at 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.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
> > > > > >
> > > > > > 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*.
> >
> > I have to stop you there. It absolutely is not limited to that.
>
> It is limited to that if we're going to keep using the device trees that
> Linux uses. Full stop. There's not really wiggle room there either.

That is really the problem, I agree.

But I would be happy with a u-boot.dtsi file to resolve this, while we
wait. I believe a binding makes sense in this case.

Regards,
Simon