Re: [PATCH] arch/mips/kvm/emulate.c: Don't waste /dev/random emulating TLBWR

[Paul Burton <paul.burton@mips.com>]

Hi George,

On Thu, Mar 21, 2019 at 06:04:24AM +0000, George Spelvin wrote:
> KVM_MIPS_GUEST_TLB_SIZE is 64, so we only need one random byte,
> not 4.
> 
> A more complex question is whether we need crypto-grade random
> numbers at all.  If safe, we could use prandom_u32().  If not,
> we could seed a private PRNG and use prandom_u32_state().
> 
> Or could we just use asm("mfc0 %0, Random" : "=r" (index))?
> 
> Signed-off-by: George Spelvin <lkml@sdf.org>
> ---
> I ran across this whie doing some other cleanups, and thought
> I'd pass it on.
> 
> get_random_bytes() is quite an expensive function call.
> Is it needed at all?

Thanks for the patch. I expect we should be fine with:

  index = prandom_u32_max(KVM_MIPS_GUEST_TLB_SIZE);

We certainly don't need crypto-grade randomness here. Using the cp0
Random register would be an option for configurations prior to MIPSr6,
where the Random register was deprecated & we shouldn't rely on its
presence. So we could do:

  if (MIPS_ISA_REV < 6)
    index = read_c0_random() % KVM_MIPS_GUEST_TLB_SIZE;
  else
    index = prandom_u32_max(KVM_MIPS_GUEST_TLB_SIZE);

Though whether that micro-optimization is worth the extra code is
questionable.

Thanks,
    Paul

>  arch/mips/kvm/emulate.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/mips/kvm/emulate.c b/arch/mips/kvm/emulate.c
> index ec9ed23bca7f..a689f3db3094 100644
> --- a/arch/mips/kvm/emulate.c
> +++ b/arch/mips/kvm/emulate.c
> @@ -1139,8 +1139,9 @@ enum emulation_result kvm_mips_emul_tlbwr(struct kvm_vcpu *vcpu)
>  	struct mips_coproc *cop0 = vcpu->arch.cop0;
>  	struct kvm_mips_tlb *tlb = NULL;
>  	unsigned long pc = vcpu->arch.pc;
> -	int index;
> +	unsigned char index;
>  
> +	/* Do we need this quality of random numbers?  Would prandom_u32 do? */
>  	get_random_bytes(&index, sizeof(index));
>  	index &= (KVM_MIPS_GUEST_TLB_SIZE - 1);
>  
> -- 
> 2.20.1
>