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