drivers: qcom: Add QRNG and secure watchdog drivers for Bobcat family#15
drivers: qcom: Add QRNG and secure watchdog drivers for Bobcat family#15thariaruchamy-bit wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Have shared initial review comments. Please check it out and fix them @thariaruchamy-bit
@ldts / @b49020, Please help to review the patches posted here when you guys have some time and share your feedback as well, would be really helpful.
aab5ca7 to
288414e
Compare
|
@thariaruchamy-bit how is QRNG driver different from the PRNG driver? TBH, I don't see any difference. Please rather reuse the existing driver. |
288414e to
0fbb25c
Compare
@b49020, QRNG is a separate H/W IP thats different from PRNG H/W IP. Discussed previously with @ldts and we have certain features in QRNG like "KATS" (Known answer tests) which we can potentially add in future and hence we thought it'd be better to keep it as a separate driver based on the discussion? Would you rather suggest to keep common driver now and split it later whenever we add those QRNG specific features? Or can we keep it this way now and add features into PRNG/QRNG later? (as required?) Also, if we have to use the same driver, would it be an option to rename the prng/prng.c to rng/rng.c and keep the macro names generic so that each platform can define the QRNG/PRNG EE addr as applicable? Keeping the driver/config as PRNG and using it in a platform which has QRNG IP (not PRNG IP) would be confusing? Note: If we did PRNG/QRNG init completely in OP-TEE, then the sequences would be different, here we are re-using the init done by bootloaders and in its current state, it seems like there are no major diffs. |
Discussed with Sumit. The suggestion is to rename the driver as qcom-rng.c and then, we can create an abstraction layer based on platform support. (i.e. qrng or prng base addresses alone can come from target specific headers that we already have). That way we can have a single driver for now. We can create more abstractions/layers later if we add QRNG/PRNG specific stuff. |
|
@zelvam95 @b49020 the abstraction is a bit silly TBH (the driver is basically a single function that could be in a header file); the IPs are different, they drive different use cases (one offers crypto key generation capabilties, the other one doesnt even thought it is being used in that way because we have no other choice). Forcing a shared abstraction for a function is really unnecessary...best is keep them as separate compilation units and let them evolve separately. |
987d9cc to
e714a98
Compare
|
@ldts isn't code reuse the standard way how upstream drivers are written? These are exactly the same APIs used to fetch random numbers. Surely, they can differ on entropy estimates or certification which can be referenced. Look for the Qcom RNG driver in the kernel: |
|
It's all about maintenance and keeping code in good shape for all the targets. |
|
@b49020 100% in favor of reuse in general terms. But this is a completely different IP block which as we go will probably cause us to use conditional compilation macros (maybe will need a udelay in the future, surely we will add the missing functionality and so on...and all for the sake of a loop that is simple and generic is my point (that is what I meant by silly, as in just a while loop)....IMO we should keep them separate (QRNG and PRNG even though they are initially sharing a while loop). |
|
@ldts it's the same QRNG and PRNG in the Linux kernel too and they share the same |
|
doesnt linux tries to ship one binary for many boards? op-tee does not therefore the split (which incidentally makes maintainance easier since a bug would not propagate). |
e714a98 to
e05bd0d
Compare
We need OP-TEE be more target independent with the migration towards DT.
Having the common driver is better to catch any corner cases as it will be tested across targets. |
Thanks Sumit/Jorge; For now, will go ahead with common driver and maybe in future if we have more use-cases added specific to PRNG / QRNG IP or if we in case need to do the init itself in OP-TEE for either of them, etc., we can probably think of separating them at that point (if/as required). |
e05bd0d to
19ab8e2
Compare
19ab8e2 to
a4079e2
Compare
3ea68f0 to
87ed93f
Compare
7929f55 to
4b793e5
Compare
- Rename prng.c to qcom-rng.c for platform-agnostic naming - Move driver source inclusion to parent qcom/sub.mk with CFG_QCOM_RNG flag - Simplify driver by using generic RNG naming conventions Change-Id: I5b262d5038e10aca7f6b4e8ee93847006b1b0a9f Signed-off-by: Harikrishna <hart@qti.qualcomm.com>
- Update from CFG_QCOM_PRNG to unified CFG_QCOM_RNG - Configure RNG_REG_BASE for PRNG variant - Enable CFG_HWRNG_PTA configuration for ipq52xx - When HWRNG_PTA is enabled: - Disable CFG_WITH_SOFTWARE_PRNG to prevent fallback - Force enable CFG_QCOM_RNG to use hardware QRNG driver - Configure HWRNG quality to 1024 bits entropy - Set HWRNG rate to 0 (unlimited) Signed-off-by: Harikrishna <hart@qti.qualcomm.com>
QCOM platforms manage secure watchdog via driver_init() without framework registration. This is sufficient as QCOM currently does not require HLOS control over the secure watchdog. The implementation maintains separation between secure and non-secure world watchdog management. Key implementation details: - Maps APSS_WDT_TMR2_BASE into secure I/O memory - Configures bark timeout (6s) and bite timeout (22s) using a 32 KHz watchdog clock, converting milliseconds to hardware ticks - Registers qcom_sec_wdog_bark_handler() for SEC_WDOG_BARK_INT_ID via interrupt_create_handler() - Bark handler pets the watchdog by writing to WDOG_RESET register Signed-off-by: Harikrishna <hart@qti.qualcomm.com>
4b793e5 to
1778362
Compare
- CFG_QCOM_SEC_WDOG enabled in bobcat/arch.mk for all Bobcat targets - Per-target hardware addresses (WDT_TMR_BASE, WDT_BARK_INT_ID, WDT_RESET_REG_OFFSET) defined in target-specific target_config.h files: - ipq96xx/ipq54xx: base=0x0F411000, bark_int=0x36 - ipq52xx: base=0x0B117000, bark_int=0x23 Signed-off-by: Harikrishna <hart@qti.qualcomm.com>
1778362 to
c33815d
Compare
|
With above 2 comments addressed, you can post the PR to upstream OPTEE as well @thariaruchamy-bit. We can get further review comments from Sumit/Jorge/Upstream maintainers as part of the upstream review |
|
@zelvam95 Sure, have posted the changes in upstream optee master branch |
Implement hardware drivers for random number generation and watchdog
functionality across the Qualcomm Bobcat (IPQ) platform family.
QRNG Driver (ipq52xx):
proper VA mapping
Secure Watchdog Driver (Bobcat Family):
with millisecond-to-hardware tick conversion
via interrupt_create_handler()
Platform Configuration:
APSS_WDT_TMR2_BASE = 0x0F411000
bark_int = 0x36
APSS_WDT_TMR2_BASE = 0x0B117000
bark_int = 0x23
Signed-off-by: Harikrishna hart@qti.qualcomm.com