[v7,11/17] openrisc: account for 0 starting value in random_get_entropy()

As a sanity check, this series makes sure that during early boot, the
cycle counter isn't returning all zeros. However, OpenRISC's TTCR timer
can be rather slow and starts out as zero during stages of early boot.
We know it works, however. So just always add 1 to random_get_entropy()
so that it doesn't trigger these checks.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Jonas Bonn <jonas@southpole.se>
Cc: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
Acked-by: Stafford Horne <shorne@gmail.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
Changes v6->v7:
- Add 1 to cycle counter to account for functional but slow-to-begin
  counter on QEMU.

 arch/openrisc/include/asm/timex.h | 3 +++
 1 file changed, 3 insertions(+)

Hi Jason,

On Fri, Apr 29, 2022 at 02:16:48AM +0200, Jason A. Donenfeld wrote:
> As a sanity check, this series makes sure that during early boot, the
> cycle counter isn't returning all zeros. However, OpenRISC's TTCR timer
> can be rather slow and starts out as zero during stages of early boot.
> We know it works, however. So just always add 1 to random_get_entropy()
> so that it doesn't trigger these checks.

Just one nit, you might want to qualify that this is related to simulators/qemu:
 * "However, in simulators OpenRISC's TTCR timer can be rather slow..."

> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Jonas Bonn <jonas@southpole.se>
> Cc: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
> Acked-by: Stafford Horne <shorne@gmail.com>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
> Changes v6->v7:
> - Add 1 to cycle counter to account for functional but slow-to-begin
>   counter on QEMU.
> 
>  arch/openrisc/include/asm/timex.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/openrisc/include/asm/timex.h b/arch/openrisc/include/asm/timex.h
> index d52b4e536e3f..a78a5807c927 100644
> --- a/arch/openrisc/include/asm/timex.h
> +++ b/arch/openrisc/include/asm/timex.h
> @@ -23,6 +23,9 @@ static inline cycles_t get_cycles(void)
>  {
>  	return mfspr(SPR_TTCR);
>  }
> +#define get_cycles get_cycles
> +
> +#define random_get_entropy() ((unsigned long)get_cycles() + 1)
>  
>  /* This isn't really used any more */
>  #define CLOCK_TICK_RATE 1000

Thanks,

-Stafford

Hi Stafford,

On Sun, May 01, 2022 at 07:11:37AM +0900, Stafford Horne wrote:
 
> I was thinking about this, the reason the tick timer is returing 0 is because
> the timer is not started.  It's getting initialized right after the random
> number generator.
> 
> A patch like this helps to startup the timer during intial startup, but I am not
> sure its the best thing:
> 
> diff --git a/arch/openrisc/kernel/head.S b/arch/openrisc/kernel/head.S
> index 15f1b38dfe03..a9b3b5614e13 100644
> --- a/arch/openrisc/kernel/head.S
> +++ b/arch/openrisc/kernel/head.S
> @@ -521,6 +521,9 @@ _start:
>         l.ori   r3,r0,0x1
>         l.mtspr r0,r3,SPR_SR
>  
> +       l.movhi r3,hi(SPR_TTMR_CR)
> +       l.mtspr r0,r3,SPR_TTMR
> +
>         CLEAR_GPR(r1)
>         CLEAR_GPR(r2)
>         CLEAR_GPR(r3)

Yea, great, I was thinking about doing it in assembly earlier in boot
too, but didn't know how you'd feel about that. I like this better.

The reason I think this is a good approach is that it means the cycle
counter includes some information about how long startup takes from the
earliest stages -- which could involve probing various devices or
strange things. So enabling the timer in head.S seems good to me.

> But I wonder:
>  - Why don't any other architectures have similar issues.
>  - Is there any more correct place to do an early timer kick off.

I think most other archs (like, say, x86) have their cycle counter
enabled by default at boot time. I was surprised to see that the or1k
risc cycle counter comes disabled by default actually.

I'll send a v9 incorporating your suggested assembly change.

Jason

On Sun, May 01, 2022 at 12:34:02AM +0200, Jason A. Donenfeld wrote:
> Hi Stafford,
> 
> On Sun, May 01, 2022 at 07:11:37AM +0900, Stafford Horne wrote:
>  
> > I was thinking about this, the reason the tick timer is returing 0 is because
> > the timer is not started.  It's getting initialized right after the random
> > number generator.
> > 
> > A patch like this helps to startup the timer during intial startup, but I am not
> > sure its the best thing:
> > 
> > diff --git a/arch/openrisc/kernel/head.S b/arch/openrisc/kernel/head.S
> > index 15f1b38dfe03..a9b3b5614e13 100644
> > --- a/arch/openrisc/kernel/head.S
> > +++ b/arch/openrisc/kernel/head.S
> > @@ -521,6 +521,9 @@ _start:
> >         l.ori   r3,r0,0x1
> >         l.mtspr r0,r3,SPR_SR
> >  
> > +       l.movhi r3,hi(SPR_TTMR_CR)
> > +       l.mtspr r0,r3,SPR_TTMR
> > +
> >         CLEAR_GPR(r1)
> >         CLEAR_GPR(r2)
> >         CLEAR_GPR(r3)
> 
> Yea, great, I was thinking about doing it in assembly earlier in boot
> too, but didn't know how you'd feel about that. I like this better.
> 
> The reason I think this is a good approach is that it means the cycle
> counter includes some information about how long startup takes from the
> earliest stages -- which could involve probing various devices or
> strange things. So enabling the timer in head.S seems good to me.
> 
> > But I wonder:
> >  - Why don't any other architectures have similar issues.
> >  - Is there any more correct place to do an early timer kick off.
> 
> I think most other archs (like, say, x86) have their cycle counter
> enabled by default at boot time. I was surprised to see that the or1k
> risc cycle counter comes disabled by default actually.
> 
> I'll send a v9 incorporating your suggested assembly change.

Thanks!

-Stafford