Re: cpu_rnd_messybits() for arm64

2020-06-05 Thread Theo de Raadt
> It's a 64-bit counter, which we reduce to 32 bits.  Since there is
> progressively less entropy in the higher bits of a counter than in
> the lower bits, it intuitively makes sense not just to do hi^lo,
> but to bit-reverse one half in order to extract maximal entropy,
> and on aarch64 bit reversal is a simple instruction.

That doesn't matter at all.  First of all, this is +='d to multiple
rounds in the ring which never clean.  Then a crc is run over it ^= over
old contents.  Then at some timeout it does a sha256 and ^= it again,
leaving that in a buffer, which a different timeout merges into the
chacha state, yes you guess right some more ^=.

But if it makes you feel better :)



Re: cpu_rnd_messybits() for arm64

2020-06-05 Thread Theo de Raadt
ok deraadt

Christian Weisgerber  wrote:

> Mark Kettenis:
> 
> > > Here is a cpu_rnd_messybits() implementation for arm64.
> > > It reads the virtual counter and xors it with a bit-reversed copy
> > > of itself.
> > > 
> > > The virtual counter is used by the only timecounter implementation
> > > used on arm64, so I assume it is generally available.
> > > 
> > > It's a 64-bit counter, which we reduce to 32 bits.  Since there is
> > > progressively less entropy in the higher bits of a counter than in
> > > the lower bits, it intuitively makes sense not just to do hi^lo,
> > > but to bit-reverse one half in order to extract maximal entropy,
> > > and on aarch64 bit reversal is a simple instruction.
> > 
> > Can you make that uint64_t?
> > Can you use %0 and %1 instead of %x0 and %x1?
> 
> Right.
> 
> Index: arch/arm64/arm64/machdep.c
> ===
> RCS file: /cvs/src/sys/arch/arm64/arm64/machdep.c,v
> retrieving revision 1.52
> diff -u -p -r1.52 machdep.c
> --- arch/arm64/arm64/machdep.c31 May 2020 06:23:57 -  1.52
> +++ arch/arm64/arm64/machdep.c5 Jun 2020 20:00:20 -
> @@ -1247,12 +1247,3 @@ dumpregs(struct trapframe *frame)
>   printf("pc: 0x%016lx\n", frame->tf_elr);
>   printf("spsr: 0x%016lx\n", frame->tf_spsr);
>  }
> -
> -unsigned int
> -cpu_rnd_messybits(void)
> -{
> - struct timespec ts;
> -
> - nanotime();
> - return (ts.tv_nsec ^ (ts.tv_sec << 20));
> -}
> Index: arch/arm64/include/cpu.h
> ===
> RCS file: /cvs/src/sys/arch/arm64/include/cpu.h,v
> retrieving revision 1.17
> diff -u -p -r1.17 cpu.h
> --- arch/arm64/include/cpu.h  31 May 2020 06:23:57 -  1.17
> +++ arch/arm64/include/cpu.h  5 Jun 2020 22:13:07 -
> @@ -183,7 +183,16 @@ void cpu_boot_secondary_processors(void)
>  
>  #define curpcb   curcpu()->ci_curpcb
>  
> -unsigned int cpu_rnd_messybits(void);
> +static inline unsigned int
> +cpu_rnd_messybits(void)
> +{
> + uint64_t val, rval;
> +
> + __asm volatile("mrs %0, CNTVCT_EL0; rbit %1, %0;"
> + : "=r" (val), "=r" (rval));
> +
> + return (val ^ rval);
> +}
>  
>  /*
>   * Scheduling glue
> -- 
> Christian "naddy" Weisgerber  na...@mips.inka.de
> 



Re: cpu_rnd_messybits() for arm64

2020-06-05 Thread Mark Kettenis
> Date: Sat, 6 Jun 2020 00:30:51 +0200
> From: Christian Weisgerber 
> Cc: tech@openbsd.org
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> 
> Mark Kettenis:
> 
> > > Here is a cpu_rnd_messybits() implementation for arm64.
> > > It reads the virtual counter and xors it with a bit-reversed copy
> > > of itself.
> > > 
> > > The virtual counter is used by the only timecounter implementation
> > > used on arm64, so I assume it is generally available.
> > > 
> > > It's a 64-bit counter, which we reduce to 32 bits.  Since there is
> > > progressively less entropy in the higher bits of a counter than in
> > > the lower bits, it intuitively makes sense not just to do hi^lo,
> > > but to bit-reverse one half in order to extract maximal entropy,
> > > and on aarch64 bit reversal is a simple instruction.
> > 
> > Can you make that uint64_t?
> > Can you use %0 and %1 instead of %x0 and %x1?
> 
> Right.

ok kettenis@

> Index: arch/arm64/arm64/machdep.c
> ===
> RCS file: /cvs/src/sys/arch/arm64/arm64/machdep.c,v
> retrieving revision 1.52
> diff -u -p -r1.52 machdep.c
> --- arch/arm64/arm64/machdep.c31 May 2020 06:23:57 -  1.52
> +++ arch/arm64/arm64/machdep.c5 Jun 2020 20:00:20 -
> @@ -1247,12 +1247,3 @@ dumpregs(struct trapframe *frame)
>   printf("pc: 0x%016lx\n", frame->tf_elr);
>   printf("spsr: 0x%016lx\n", frame->tf_spsr);
>  }
> -
> -unsigned int
> -cpu_rnd_messybits(void)
> -{
> - struct timespec ts;
> -
> - nanotime();
> - return (ts.tv_nsec ^ (ts.tv_sec << 20));
> -}
> Index: arch/arm64/include/cpu.h
> ===
> RCS file: /cvs/src/sys/arch/arm64/include/cpu.h,v
> retrieving revision 1.17
> diff -u -p -r1.17 cpu.h
> --- arch/arm64/include/cpu.h  31 May 2020 06:23:57 -  1.17
> +++ arch/arm64/include/cpu.h  5 Jun 2020 22:13:07 -
> @@ -183,7 +183,16 @@ void cpu_boot_secondary_processors(void)
>  
>  #define curpcb   curcpu()->ci_curpcb
>  
> -unsigned int cpu_rnd_messybits(void);
> +static inline unsigned int
> +cpu_rnd_messybits(void)
> +{
> + uint64_t val, rval;
> +
> + __asm volatile("mrs %0, CNTVCT_EL0; rbit %1, %0;"
> + : "=r" (val), "=r" (rval));
> +
> + return (val ^ rval);
> +}
>  
>  /*
>   * Scheduling glue
> -- 
> Christian "naddy" Weisgerber  na...@mips.inka.de
> 



Re: cpu_rnd_messybits() for arm64

2020-06-05 Thread Christian Weisgerber
Mark Kettenis:

> > Here is a cpu_rnd_messybits() implementation for arm64.
> > It reads the virtual counter and xors it with a bit-reversed copy
> > of itself.
> > 
> > The virtual counter is used by the only timecounter implementation
> > used on arm64, so I assume it is generally available.
> > 
> > It's a 64-bit counter, which we reduce to 32 bits.  Since there is
> > progressively less entropy in the higher bits of a counter than in
> > the lower bits, it intuitively makes sense not just to do hi^lo,
> > but to bit-reverse one half in order to extract maximal entropy,
> > and on aarch64 bit reversal is a simple instruction.
> 
> Can you make that uint64_t?
> Can you use %0 and %1 instead of %x0 and %x1?

Right.

Index: arch/arm64/arm64/machdep.c
===
RCS file: /cvs/src/sys/arch/arm64/arm64/machdep.c,v
retrieving revision 1.52
diff -u -p -r1.52 machdep.c
--- arch/arm64/arm64/machdep.c  31 May 2020 06:23:57 -  1.52
+++ arch/arm64/arm64/machdep.c  5 Jun 2020 20:00:20 -
@@ -1247,12 +1247,3 @@ dumpregs(struct trapframe *frame)
printf("pc: 0x%016lx\n", frame->tf_elr);
printf("spsr: 0x%016lx\n", frame->tf_spsr);
 }
-
-unsigned int
-cpu_rnd_messybits(void)
-{
-   struct timespec ts;
-
-   nanotime();
-   return (ts.tv_nsec ^ (ts.tv_sec << 20));
-}
Index: arch/arm64/include/cpu.h
===
RCS file: /cvs/src/sys/arch/arm64/include/cpu.h,v
retrieving revision 1.17
diff -u -p -r1.17 cpu.h
--- arch/arm64/include/cpu.h31 May 2020 06:23:57 -  1.17
+++ arch/arm64/include/cpu.h5 Jun 2020 22:13:07 -
@@ -183,7 +183,16 @@ void cpu_boot_secondary_processors(void)
 
 #define curpcb curcpu()->ci_curpcb
 
-unsigned int   cpu_rnd_messybits(void);
+static inline unsigned int
+cpu_rnd_messybits(void)
+{
+   uint64_t val, rval;
+
+   __asm volatile("mrs %0, CNTVCT_EL0; rbit %1, %0;"
+   : "=r" (val), "=r" (rval));
+
+   return (val ^ rval);
+}
 
 /*
  * Scheduling glue
-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



Re: cpu_rnd_messybits() for arm64

2020-06-05 Thread Mark Kettenis
> Date: Fri, 5 Jun 2020 23:27:13 +0200
> From: Christian Weisgerber 
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> 
> Here is a cpu_rnd_messybits() implementation for arm64.
> It reads the virtual counter and xors it with a bit-reversed copy
> of itself.
> 
> The virtual counter is used by the only timecounter implementation
> used on arm64, so I assume it is generally available.
> 
> It's a 64-bit counter, which we reduce to 32 bits.  Since there is
> progressively less entropy in the higher bits of a counter than in
> the lower bits, it intuitively makes sense not just to do hi^lo,
> but to bit-reverse one half in order to extract maximal entropy,
> and on aarch64 bit reversal is a simple instruction.
> 
> This patch I have tested!
> 
> ok?
> 
> Index: arch/arm64/arm64/machdep.c
> ===
> RCS file: /cvs/src/sys/arch/arm64/arm64/machdep.c,v
> retrieving revision 1.52
> diff -u -p -r1.52 machdep.c
> --- arch/arm64/arm64/machdep.c31 May 2020 06:23:57 -  1.52
> +++ arch/arm64/arm64/machdep.c5 Jun 2020 20:00:20 -
> @@ -1247,12 +1247,3 @@ dumpregs(struct trapframe *frame)
>   printf("pc: 0x%016lx\n", frame->tf_elr);
>   printf("spsr: 0x%016lx\n", frame->tf_spsr);
>  }
> -
> -unsigned int
> -cpu_rnd_messybits(void)
> -{
> - struct timespec ts;
> -
> - nanotime();
> - return (ts.tv_nsec ^ (ts.tv_sec << 20));
> -}
> Index: arch/arm64/include/cpu.h
> ===
> RCS file: /cvs/src/sys/arch/arm64/include/cpu.h,v
> retrieving revision 1.17
> diff -u -p -r1.17 cpu.h
> --- arch/arm64/include/cpu.h  31 May 2020 06:23:57 -  1.17
> +++ arch/arm64/include/cpu.h  5 Jun 2020 20:32:01 -
> @@ -183,7 +183,16 @@ void cpu_boot_secondary_processors(void)
>  
>  #define curpcb   curcpu()->ci_curpcb
>  
> -unsigned int cpu_rnd_messybits(void);
> +static inline unsigned int
> +cpu_rnd_messybits(void)
> +{
> + u_int64_t val, rval;

Can you make that uint64_t?

> +
> + __asm volatile("mrs %x0, CNTVCT_EL0; rbit %x1, %x0;"
> + : "=r" (val), "=r" (rval));

Can you use %0 and %1 instead of %x0 and %x1?

> +
> + return (val ^ rval);
> +}
>  
>  /*
>   * Scheduling glue
> -- 
> Christian "naddy" Weisgerber  na...@mips.inka.de
> 
>