linux-kernel - Re: [PATCH] clocksource/drivers/timer-riscv: Drop extra CSR write

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-3a5fcdd0-0bf4-45d3-aa7d-fffddd852288@palmer-ri-x1c9a>
Date: Tue, 16 Apr 2024 18:49:51 -0700 (PDT)
From: Palmer Dabbelt <palmer@...belt.com>
To: apatel@...tanamicro.com
CC: samuel.holland@...ive.com, daniel.lezcano@...aro.org, tglx@...utronix.de,
  aou@...s.berkeley.edu, Paul Walmsley <paul.walmsley@...ive.com>, linux-kernel@...r.kernel.org,
  linux-riscv@...ts.infradead.org
Subject:     Re: [PATCH] clocksource/drivers/timer-riscv: Drop extra CSR write

On Wed, 13 Mar 2024 09:56:34 PDT (-0700), apatel@...tanamicro.com wrote:
> On Wed, Mar 13, 2024 at 1:03 AM Samuel Holland
> <samuel.holland@...ive.com> wrote:
>>
>> On riscv32, the time comparator value is split across two CSRs. We write
>> both when stopping the timer, but realistically the time is just as
>> unlikely to reach 0xffffffff00000000 as 0xffffffffffffffff, so there is
>> no need to write the low CSR.
>
> Even though unlikely, there is still a theoretical possibility of
> counter reaching value 0xffffffff00000000.

I guess that depends on the timebase frequency, but if my math is right 
then we've got some timers that will overflow a 32-bit counter in 10 
minutes -- take that with a grain of salt as they're all 64-bit systems 
(we don't have any 32-bit DTs upstream?), but it still seems plausible 
for 32-bit overflows to happen here on real systems.

> The good thing about value 0xffffffffffffffff is that the counter will
> immediately wrap around after reaching it.

I'm not sure how that's good?  Luckily we've got ~100,000 years to 
figure out, even on these systems with pretty fast timers ;)

> Regards,
> Anup
>
>
>>
>> Signed-off-by: Samuel Holland <samuel.holland@...ive.com>
>> ---
>>
>>  drivers/clocksource/timer-riscv.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clocksource/timer-riscv.c b/drivers/clocksource/timer-riscv.c
>> index e66dcbd66566..eaaf01f3c34b 100644
>> --- a/drivers/clocksource/timer-riscv.c
>> +++ b/drivers/clocksource/timer-riscv.c
>> @@ -35,9 +35,10 @@ static bool riscv_timer_cannot_wake_cpu;
>>  static void riscv_clock_event_stop(void)
>>  {
>>         if (static_branch_likely(&riscv_sstc_available)) {
>> -               csr_write(CSR_STIMECMP, ULONG_MAX);
>>                 if (IS_ENABLED(CONFIG_32BIT))
>>                         csr_write(CSR_STIMECMPH, ULONG_MAX);
>> +               else
>> +                       csr_write(CSR_STIMECMP, ULONG_MAX);
>>         } else {
>>                 sbi_set_timer(U64_MAX);
>>         }
>> --
>> 2.43.1
>>
>>