linux-kernel - Re: [PATCH 5/7] RISC-V: arch/riscv/lib

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <mhng-077efa0f-772e-4953-b673-41472cf49341@palmer-si-x1c4>
Date:   Wed, 24 May 2017 18:59:09 -0700 (PDT)
From:   Palmer Dabbelt <palmer@...belt.com>
To:     Arnd Bergmann <arnd@...db.de>
CC:     albert@...ive.com
Subject:     Re: [PATCH 5/7] RISC-V: arch/riscv/lib

On Tue, 23 May 2017 04:19:42 PDT (-0700), Arnd Bergmann wrote:
> On Tue, May 23, 2017 at 2:41 AM, Palmer Dabbelt <palmer@...belt.com> wrote:
>> diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
>> new file mode 100644
>> index 000000000000..f644e582f4b8
>> --- /dev/null
>> +++ b/arch/riscv/lib/Makefile
>
>> +
>> +void __delay(unsigned long cycles)
>> +{
>> +       u64 t0 = get_cycles();
>> +
>> +       while ((unsigned long)(get_cycles() - t0) < cycles)
>> +               cpu_relax();
>> +}
>> +
>> +void udelay(unsigned long usecs)
>> +{
>> +       u64 ucycles = (u64)usecs * timebase;
>> +       do_div(ucycles, 1000000U);
>> +       __delay((unsigned long)ucycles);
>> +}
>> +EXPORT_SYMBOL(udelay);
>> +
>> +void ndelay(unsigned long nsecs)
>> +{
>> +       u64 ncycles = (u64)nsecs * timebase;
>> +       do_div(ncycles, 1000000000U);
>> +       __delay((unsigned long)ncycles);
>> +}
>
> I'd be slightly worried about a global 'timebase' identifier that
> might conflict with a variable in some random driver.

Makes sense.  I've renamed it to riscv_timebase

  https://github.com/riscv/riscv-linux/commit/ed7d769e2c14e8809c3c125e0bba2978cb6fd37b

> Also, it would be good to replace the multiply+div64
> with a single multiplication here, see how x86 and arm do it
> (for the tsc/__timer_delay case).

Makes sense.  I think this should do it

  https://github.com/riscv/riscv-linux/commit/d397332f6ebff42f3ecb385e9cf3284fdeda6776

but I'm finding this hard to test as this only works for 2ms sleeps.  It seems
at least in the right ballpark

  [    0.048000] before 1000x usleep 1000
  [    1.060000] before 1000x nsleep 1000000
  [    2.072000] done

>       Arnd

Thanks for the feedback.  I'll incorporate this along with all the other
feedback into a v2.