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

[Heinrich Schuchardt]

On 11/8/23 08:44, Tom Rini wrote:
> 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.

Hardware with the Zkr extension is yet to hit the market. Please, run 
upstream QEMU (commit 2f32dcabc2f0 or later) with -cpu rv64,zkr=on to 
see the device-tree entry. A patch for OpenSBI to enable mseccfg.sseed 
is pending.

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

Here is the issue:

Missing function to find first successfully probed device for a uclass
https://source.denx.de/u-boot/custodians/u-boot-dm/-/issues/8

Best regards

Heinrich