-
Notifications
You must be signed in to change notification settings - Fork 7.6k
drivers: entropy: stm32: make get_entropy_isr
re-entrant
#91593
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -749,45 +749,90 @@ static int entropy_stm32_rng_get_entropy_isr(const struct device *dev, | |
|
||
if (len) { | ||
/** | ||
* On TRNG without interrupt line, we cannot allow reentrancy, | ||
* so we have to suspend all interrupts. Otherwise, only suspend | ||
* it until we have established ourselves as owner of the TRNG | ||
* to prevent race with a higher priority interrupt handler. | ||
* Suspend interrupts to check whether RNG is active or not. | ||
* If active, use the RNG and return without disabling it. | ||
* Otherwise, enable the RNG, fill the caller's buffer and | ||
* disable the RNG before returning. | ||
* | ||
* NOTE: insufficient on SMP (but there's no such STM32 yet) | ||
*/ | ||
unsigned int key = irq_lock(); | ||
bool rng_already_acquired = false; | ||
bool rng_owned_by_us; | ||
bool release_rng_after_read; | ||
unsigned int key; | ||
enum clock_control_status rng_clk_status; | ||
|
||
key = irq_lock(); | ||
rng_clk_status = clock_control_get_status( | ||
entropy_stm32_rng_data.clock, | ||
&(entropy_stm32_rng_config.pclken[0])); | ||
|
||
/** | ||
* z_stm32_hsem_is_owned() evaluates to false on single-CPU SoCs, | ||
* but we always own RNG on these SoCs since there's only one CPU. | ||
*/ | ||
if (IS_ENABLED(CONFIG_SOC_SERIES_STM32WBX) | ||
|| IS_ENABLED(CONFIG_SOC_SERIES_STM32MP1X) | ||
|| IS_ENABLED(CONFIG_STM32H7_DUAL_CORE)) { | ||
rng_owned_by_us = z_stm32_hsem_is_owned(CFG_HW_RNG_SEMID); | ||
} else { | ||
rng_owned_by_us = true; | ||
} | ||
|
||
if (rng_clk_status == CLOCK_CONTROL_STATUS_ON | ||
&& ll_rng_is_enabled(entropy_stm32_rng_data.rng) | ||
&& rng_owned_by_us) { | ||
/* RNG already in use by this CPU. | ||
* Preempted user will release once we return control | ||
* and it generates the entropy it wanted. | ||
*/ | ||
release_rng_after_read = false; | ||
} else { | ||
/* RNG not in use - release it once we are done */ | ||
release_rng_after_read = true; | ||
} | ||
|
||
#if !IRQLESS_TRNG | ||
int irq_enabled = irq_is_enabled(IRQN); | ||
/** | ||
* Disable RNG interrupt at NVIC level to ensure RNG ISR cannot preempt us. | ||
* The ISR might disable the RNG while we're using it, and it would steal | ||
* entropy that our caller wants anyways, so let's make sure it will not | ||
* get a chance to run while we're generating entropy. | ||
*/ | ||
int rng_irq_enabled = irq_is_enabled(IRQN); | ||
|
||
rng_already_acquired = (irq_enabled != 0); | ||
irq_disable(IRQN); | ||
irq_unlock(key); | ||
#endif /* !IRQLESS_TRNG */ | ||
|
||
/* Do not release if IRQ is enabled. RNG will be released in ISR | ||
* when the pools are full. On TRNG without interrupt line, the | ||
* default value of false ensures TRNG is always released. | ||
*/ | ||
if (z_stm32_hsem_is_owned(CFG_HW_RNG_SEMID)) { | ||
rng_already_acquired = true; | ||
} | ||
acquire_rng(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it ok to acquire the RNG (possibly polling on a HSEM) with interrupts locked? |
||
|
||
/** | ||
* RNG is enabled so interrupts can be resumed: if a re-entrant | ||
* call to `get_entropy_isr` is made while we are in this section, | ||
* the re-entrant call will see that the RNG is enabled and owned, | ||
* and `release_rng_after_read` will be false for that call, making | ||
* sure it doesn't disable RNG before returning control to us. | ||
*/ | ||
irq_unlock(key); | ||
|
||
cnt = generate_from_isr(buf, len); | ||
|
||
/* Restore the state of the RNG lock and IRQ */ | ||
if (!rng_already_acquired) { | ||
release_rng(); | ||
} | ||
/** | ||
* Disable the RNG (and re-enable interrupt at NVIC level if applicable) | ||
* with interrupts suspended to ensure function is re-entrant. | ||
*/ | ||
key = irq_lock(); | ||
|
||
#if IRQLESS_TRNG | ||
/* Exit critical section */ | ||
irq_unlock(key); | ||
#else | ||
if (irq_enabled) { | ||
#if !IRQLESS_TRNG | ||
if (rng_irq_enabled) { | ||
irq_enable(IRQN); | ||
} | ||
#endif /* !IRQLESS_TRNG */ | ||
|
||
if (release_rng_after_read) { | ||
release_rng(); | ||
} | ||
|
||
irq_unlock(key); | ||
} | ||
|
||
return cnt; | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -24,6 +24,20 @@ | |||||
#endif /* IRQLESS_TRNG */ | ||||||
|
||||||
/* Cross-series LL compatibility wrappers */ | ||||||
static inline uint32_t ll_rng_is_enabled(RNG_TypeDef *RNGx) | ||||||
{ | ||||||
#if defined(CONFIG_SOC_STM32WB09XX) | ||||||
/** | ||||||
* LL_RNG_IsEnabled() is missing from STM32CubeWB0 v1.0.0 | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpciking also:
Suggested change
|
||||||
* but implemented in v1.1.0 - this wrapper can be removed | ||||||
* when the STM32CubeWB0 is updated in hal_stm32. | ||||||
*/ | ||||||
return (READ_BIT(RNGx->CR, RNG_CR_DISABLE) == LL_RNG_CR_DISABLE_0); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpicking: mimic expect LL implementation and prevent bool/uint32_t implicit cast? return READ_BIT(RNGx->CR, RNG_CR_DISABLE) != (RNG_CR_DISABLE)) ? 1UL : 0UL; |
||||||
#else | ||||||
return LL_RNG_IsEnabled(RNGx); | ||||||
#endif | ||||||
} | ||||||
|
||||||
static inline void ll_rng_enable_it(RNG_TypeDef *RNGx) | ||||||
{ | ||||||
/* Silence "unused" warning on IRQ-less hardware*/ | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
entropy_stm32.c
is the sole caller ofz_stm32_hsem_is_owned()
. For consistency I think this SoC helper function should return true if there is no other possible owner, that is when not WBX, MPx or H7 dual core.