[PATCH 1/1] rng: detect RISC-V Zkr RNG device in bind method
The existence of devices should be checked in the bind method and not in the probe method. Adjust the RISC-V Zkr RNG driver accordingly.
Use ENOENT (and not ENODEV) to signal that the device is not available.
Fixes: ceec977ba1a9 ("rng: Provide a RNG based on the RISC-V Zkr ISA extension") Reported-by: Andre Przywara andre.przywara@arm.com Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- drivers/rng/riscv_zkr_rng.c | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-)
diff --git a/drivers/rng/riscv_zkr_rng.c b/drivers/rng/riscv_zkr_rng.c index 8c9e111e2e..48a5251988 100644 --- a/drivers/rng/riscv_zkr_rng.c +++ b/drivers/rng/riscv_zkr_rng.c @@ -55,7 +55,7 @@ static int riscv_zkr_read(struct udevice *dev, void *data, size_t len) } break; case DEAD: - return -ENODEV; + return -ENOENT; } }
@@ -63,16 +63,16 @@ static int riscv_zkr_read(struct udevice *dev, void *data, size_t len) }
/** - * riscv_zkr_probe() - check if the seed register is available + * riscv_zkr_bind() - check if the seed register is available * - * If the SBI software has not set mseccfg.sseed=1 or the Zkr - * extension is not available this probe function will result - * in an exception. Currently we cannot recover from this. + * If the SBI software has not set mseccfg.sseed=1 or the Zkr extension is not + * available, reading the seed register will result in an exception from which + * this function safely resumes. * * @dev: RNG device * Return: 0 if successfully probed */ -static int riscv_zkr_probe(struct udevice *dev) +static int riscv_zkr_bind(struct udevice *dev) { struct resume_data resume; int ret; @@ -87,7 +87,24 @@ static int riscv_zkr_probe(struct udevice *dev) val = read_seed(); set_resume(NULL); if (ret) - return -ENODEV; + return -ENOENT; + + return 0; +} + +/** + * riscv_zkr_probe() - check if entropy is available + * + * The bind method already checked that the seed register can be read without + * excpetiong. Here we wait for the self test to finish and entropy becoming + * available. + * + * @dev: RNG device + * Return: 0 if successfully probed + */ +static int riscv_zkr_probe(struct udevice *dev) +{ + u32 val;
do { val = read_seed(); @@ -95,7 +112,7 @@ static int riscv_zkr_probe(struct udevice *dev) } while (val == BIST || val == WAIT);
if (val == DEAD) - return -ENODEV; + return -ENOENT;
return 0; } @@ -108,6 +125,7 @@ U_BOOT_DRIVER(riscv_zkr) = { .name = DRIVER_NAME, .id = UCLASS_RNG, .ops = &riscv_zkr_ops, + .bind = riscv_zkr_bind, .probe = riscv_zkr_probe, };
Hi Heinrich,
On Sat, 4 Nov 2023 at 06:51, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
The existence of devices should be checked in the bind method and not in the probe method. Adjust the RISC-V Zkr RNG driver accordingly.
Use ENOENT (and not ENODEV) to signal that the device is not available.
Fixes: ceec977ba1a9 ("rng: Provide a RNG based on the RISC-V Zkr ISA extension") Reported-by: Andre Przywara andre.przywara@arm.com Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
drivers/rng/riscv_zkr_rng.c | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-)
This device should be in the DT, so you have some control over which RNG is used.
Regards, Simon
On 11/4/23 21:42, Simon Glass wrote:
Hi Heinrich,
On Sat, 4 Nov 2023 at 06:51, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
The existence of devices should be checked in the bind method and not in the probe method. Adjust the RISC-V Zkr RNG driver accordingly.
Use ENOENT (and not ENODEV) to signal that the device is not available.
Fixes: ceec977ba1a9 ("rng: Provide a RNG based on the RISC-V Zkr ISA extension") Reported-by: Andre Przywara andre.przywara@arm.com Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
drivers/rng/riscv_zkr_rng.c | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-)
This device should be in the DT, so you have some control over which RNG is used.
The device-tree provided by QEMU does not contain such a device as Zkr is an ISA extension and not a device. This device-tree is not under the control of the U-Boot project.
Best regards
Heinrich
Hi Heinrich,
On Sun, 5 Nov 2023 at 03:47, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 11/4/23 21:42, Simon Glass wrote:
Hi Heinrich,
On Sat, 4 Nov 2023 at 06:51, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
The existence of devices should be checked in the bind method and not in the probe method. Adjust the RISC-V Zkr RNG driver accordingly.
Use ENOENT (and not ENODEV) to signal that the device is not available.
Fixes: ceec977ba1a9 ("rng: Provide a RNG based on the RISC-V Zkr ISA extension") Reported-by: Andre Przywara andre.przywara@arm.com Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
drivers/rng/riscv_zkr_rng.c | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-)
This device should be in the DT, so you have some control over which RNG is used.
The device-tree provided by QEMU does not contain such a device as Zkr is an ISA extension and not a device. This device-tree is not under the control of the U-Boot project.
Why do you bring up QEMU? I would expect that it follows the norma dt bindings, so it should not be any different from real hardware?
Anyway, you could add it. It is just a binding. I believe RISC-V has all sorts of isa options which result in different machine features.
What are you going to do when you want to choose between the ISA RNG and a TPM one?
Regards, Simon
On 11/5/23 18:25, Simon Glass wrote:
Hi Heinrich,
On Sun, 5 Nov 2023 at 03:47, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 11/4/23 21:42, Simon Glass wrote:
Hi Heinrich,
On Sat, 4 Nov 2023 at 06:51, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
The existence of devices should be checked in the bind method and not in the probe method. Adjust the RISC-V Zkr RNG driver accordingly.
Use ENOENT (and not ENODEV) to signal that the device is not available.
Fixes: ceec977ba1a9 ("rng: Provide a RNG based on the RISC-V Zkr ISA extension") Reported-by: Andre Przywara andre.przywara@arm.com Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
drivers/rng/riscv_zkr_rng.c | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-)
This device should be in the DT, so you have some control over which RNG is used.
The device-tree provided by QEMU does not contain such a device as Zkr is an ISA extension and not a device. This device-tree is not under the control of the U-Boot project.
Why do you bring up QEMU? I would expect that it follows the norma dt bindings, so it should not be any different from real hardware?
Yes Simon, QEMU follows the normal bindings. If you want to change these, please, contribute to the RISC-V working groups or to the Linux kernel project.
Anyway, you could add it. It is just a binding. I believe RISC-V has all sorts of isa options which result in different machine features.
What are you going to do when you want to choose between the ISA RNG and a TPM one?
First of all there are different configurations switches for both drivers. But of course you can enable both. They will be different U-Boot devices. The rng command has a parameter to choose a RNG device by device number. This is not different to having two USB drives.
Best regards
Heinrich
Hi Heinrich,
On Sun, 5 Nov 2023 at 14:54, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 11/5/23 18:25, Simon Glass wrote:
Hi Heinrich,
On Sun, 5 Nov 2023 at 03:47, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
On 11/4/23 21:42, Simon Glass wrote:
Hi Heinrich,
On Sat, 4 Nov 2023 at 06:51, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
The existence of devices should be checked in the bind method and not in the probe method. Adjust the RISC-V Zkr RNG driver accordingly.
Use ENOENT (and not ENODEV) to signal that the device is not available.
Fixes: ceec977ba1a9 ("rng: Provide a RNG based on the RISC-V Zkr ISA extension") Reported-by: Andre Przywara andre.przywara@arm.com Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
drivers/rng/riscv_zkr_rng.c | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-)
This device should be in the DT, so you have some control over which RNG is used.
The device-tree provided by QEMU does not contain such a device as Zkr is an ISA extension and not a device. This device-tree is not under the control of the U-Boot project.
Why do you bring up QEMU? I would expect that it follows the norma dt bindings, so it should not be any different from real hardware?
Yes Simon, QEMU follows the normal bindings. If you want to change these, please, contribute to the RISC-V working groups or to the Linux kernel project.
Rick, can you help with this? I am thinking of something like this as a top-level node:
rng { compatible = "riscv,zkr"; };
Anyway, you could add it. It is just a binding. I believe RISC-V has all sorts of isa options which result in different machine features.
What are you going to do when you want to choose between the ISA RNG and a TPM one?
First of all there are different configurations switches for both drivers. But of course you can enable both. They will be different U-Boot devices. The rng command has a parameter to choose a RNG device by device number. This is not different to having two USB drives.
Did you see the other discussion about this? Without a propert driver, there is no way to choose which RNG device is used.
Also the comment is very clear:
/** * struct driver_info - Information required to instantiate a device * * NOTE: Avoid using this except in extreme circumstances, where device tree * is not feasible (e.g. serial driver in SPL where <8KB of SRAM is * available). U-Boot's driver model uses device tree for configuration.
Do we need a build-time warning so people don't forget this?
Regards, Simon
participants (2)
-
Heinrich Schuchardt -
Simon Glass