On 6/4/2020 6:48 PM, Michael Walle wrote:
> +static int deinstantiate_rng(u8 sec_idx, int state_handle_mask)
> +{
> + u32 *desc;
> + int sh_idx, ret = 0;
> + int desc_size = sizeof(u32) * 3;
As you mentioned, descriptor size should be sizeof(u32) * 2.
> +
> + desc = memalign(ARCH_DMA_MINALIGN, desc_size);
> + if (!desc) {
> + debug("cannot allocate RNG init descriptor memory\n");
> + return -ENOMEM;
> + }
> +
> + for (sh_idx = 0; sh_idx < RNG4_MAX_HANDLES; sh_idx++) {
> + /*
> + * If the corresponding bit is set, then it means the state
> + * handle was initialized by us, and thus it needs to be
> + * deinitialized as well
> + */
> +
> + if (state_handle_mask & RDSTA_IF(sh_idx)) {
> + /*
> + * Create the descriptor for deinstantating this state
> + * handle.
> + */
> + inline_cnstr_jobdesc_rng_deinstantiation(desc, sh_idx);
> + flush_dcache_range((unsigned long)desc,
> + (unsigned long)desc + desc_size);
Shouldn't this be roundup(desc_size, ARCH_DMA_MINALIGN) instead of desc_size?
> @@ -466,9 +511,18 @@ static int instantiate_rng(u8 sec_idx, int gen_sk)
> * If the corresponding bit is set, this state handle
> * was initialized by somebody else, so it's left alone.
> */
> - rdsta_val = sec_in32(&rng->rdsta) & RNG_STATE_HANDLE_MASK;
> - if (rdsta_val & (1 << sh_idx))
> - continue;
> + rdsta_val = sec_in32(&rng->rdsta);
> + if (rdsta_val & (RDSTA_IF(sh_idx) | RDSTA_PR(sh_idx))) {
Adding RDSTA_PR(sh_idx) to the mask is not needed,
PR bit is meaningless without IF bit set.
> + if (rdsta_val & RDSTA_PR(sh_idx))
> + continue;
Could combine the condition here with the outer if condition:
if (rdsta_val & RDSTA_IF(sh_idx) && !(rdsta_val &
RDSTA_PR(sh_idx))) {
> +
> + printf("SEC%u: RNG4 SH%d was instantiated w/o
> prediction resistance. Tearing it down\n",
> + sec_idx, sh_idx);
> +
> + ret = deinstantiate_rng(sec_idx, RDSTA_IF(sh_idx));
> + if (ret)
> + break;
> + }
Horia
