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

[Tom Rini]

On Wed, Nov 08, 2023 at 07:25:22AM -0800, Heinrich Schuchardt wrote:
> On 11/8/23 06:37, Tom Rini wrote:
> > On Wed, Nov 08, 2023 at 06:23:37AM -0800, Heinrich Schuchardt wrote:
> > > On 11/7/23 16:34, Tom Rini wrote:
> > > > On Wed, Nov 08, 2023 at 12:29:03AM +0000, Conor Dooley wrote:
> > > > > On Tue, Nov 07, 2023 at 06:23:05PM -0500, Tom Rini wrote:
> > > > [snip]
> > > > > > Thanks. Setting aside Simon's follow-up, this is what I was looking for.
> > > > > > We might have to wait for Heinrich to return from the conference to have
> > > > > > time to look at how to utilize the above and see what we can do from
> > > > > > there.
> > > > > 
> > > > > I did read that, but I don't think most of it is relevant to the binding
> > > > > itself. His five things were:
> > > > > | - U-Boot models hardware (and other things) as devices in driver model [1]
> > > > > 
> > > > > This I think should be satisfied. The Zkr CSR is a property of the CPU,
> > > > > and shouldn't have its own DT node IMO. Is it problematic for U-Boot to
> > > > > populate multiple devices for its driver model based on one DT node?
> > > 
> > > Devices in U-Boot are bound on the basis of a compatible string. All RISC-V
> > > CPU nodes have a compatible string 'riscv' but that does not provide any
> > > information about the existence of the Zkr extension. That information is in
> > > the 'riscv,isa-extensions' property of the cpu nodes (see
> > > Documentation/devicetree/bindings/riscv/cpus.yaml).
> > > 
> > > > > I know in Linux that I can create devices using something like
> > > > > platform_device_register(), does U-Boot have a similar facility?
> > > 
> > > This is what the U_BOOT_DRVINFO() macro in my driver does and which Simon
> > > discourages.
> > 
> > My current thoughts are that in this case we could use U_BOOT_DRVINFO()
> > like today and then have riscv_zkr_probe() be what checks the
> > riscv,isa-extensions property for an appropriate match? This would mean
> > we don't need any new nodes/compatibles/etc, and possibly not need any
> > bootph- properties added either? I assume we don't need the RNG so early
> > as for that to be an issue.
> > 
> 
> The presence of the Zkr extension in the device-tree 'riscv,isa-extensions'
> property does not indicate if the machine mode firmware has enabled access
> in supervisor mode via the mseccfg.sseed flag. This is why my driver tries
> to read the seed register and checks if an exception occurs.

This I think is a question for Cody or someone else in the RISC-V
community? If just checking the property isn't sufficient, what is? Or,
what's the best / most reliable way? I don't know and I'll let the
RISC-V community sort that out instead of making incorrect assumptions
myself.

> Additionally checking the device-tree would increase code size. Is it really
> needed?

I mean, it depends? My biggest issue right now is that in-tree I don't
see any RISC-V targets that select any of the RISCV_ISA options so I
can't evaluate what platforms grow by how much where. We shouldn't be
talking about kilobytes of change, so no, I don't think somehow checking
the device tree for this is out of bounds. But it comes back to what I
just asked above, first. We need the correct and expected way to check
for this feature to be known, then we decide how to handle that.

> We may have multiple RNG drivers enabled on the same device, e.g. TPM, Zkr,
> virtio-rng. In several code locations we try to use the first RNG device
> (not the first successfully probed RNG device). This is why
> 
> [PATCH 1/1] rng: detect RISC-V Zkr RNG device in bind method
> https://lore.kernel.org/u-boot/20231104065107.23623-1-heinrich.schuchardt@canonical.com/
> 
> moves the detection from probe() to bind(). The patch further corrects the
> return code if the RNG device is not available.

Sounds like we have some other general issues to sort out too then.
Filing an issue on
https://source.denx.de/u-boot/custodians/u-boot-dm/-/issues/ would be
good so it doesn't get forgotten.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20231108/ea5cdae5/attachment.sig>