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] [day] [month] [year] [list]
Date:   Mon, 13 Nov 2023 10:19:38 +0800
From:   Xingyu Wu <>
To:     Daniel Lezcano <>,
        Samuel Holland <>
CC:     Thomas Gleixner <>,
        Emil Renner Berthing <>,
        Christophe JAILLET <>,
        <>, <>,
        Rob Herring <>,
        "Krzysztof Kozlowski" <>,
        Paul Walmsley <>,
        Palmer Dabbelt <>,
        Albert Ou <>,
        Philipp Zabel <>,
        Walker Chen <>,
        Samin Guo <>,
        <>, Conor Dooley <>
Subject: Re: [PATCH v7 2/3] clocksource: Add JH7110 timer driver

On 2023/11/11 2:02, Daniel Lezcano wrote:
> Hi Samuel,
> On 10/11/2023 18:40, Samuel Holland wrote:
>> On 2023-11-08 11:51 PM, Xingyu Wu wrote:
>>> On 2023/11/8 17:10, Daniel Lezcano wrote:
>>>> On 08/11/2023 04:45, Xingyu Wu wrote:
>>>>> On 2023/11/2 22:29, Daniel Lezcano wrote:
>>>> [ ... ]
>>>>> Thanks. The riscv-timer has a clocksource with a higher rating but a
>>>>> clockevent with lower rating[1] than jh7110-timer. I tested the
>>>>> jh7110-timer as clockevent and flagged as one shot, which could do some
>>>>> of the works instead of riscv-timer. And the current_clockevent changed
>>>>> to jh7110-timer.
>>>>> Because the jh7110-timer works as clocksource with lower rating and only
>>>>> will be used as global timer at CPU idle time. Is it necessary to be
>>>>> registered as clocksource? If not, should it just be registered as
>>>>> clockevent?
>>>> Yes, you can register the clockevent without the clocksource.
>>>> You mentioned the JH7110 has a better rating than the CPU architected
>>>> timers. The rating is there to "choose" the best timer, so it is up to the
>>>> author of the driver check against which timers it compares on the
>>>> platform.
>>>> Usually, CPU timers are the best.
>>>> It is surprising the timer-riscv has a so low rating. You may double check
>>>> if jh7110 is really better. If it is the case, then implementing a
>>>> clockevent per cpu would make more sense, otherwise one clockevent as a
>>>> global timer is enough.
>> The timer-riscv clockevent has a low rating because it requires a call to
>> firmware to set the timer, as well as a trap to firmware to handle the
>> interrupt, which both add overhead. Implementations which support the Sstc
>> extension[1] do not require firmware assistance to implement the clockevent, so
>> in that case we register the clockevent with a higher rating.
>> [1]:
> Thanks for the pointer and the clarification.
>>>> Unused clocksource, clockevents should be stopped in case the firmware let
>>>> them in a undetermined state.
>>> The interrupts of jh7110-timer each channel are global interrupts like
>>> SPI(Shared Peripheral Interrupt) not PPI (Private Peripheral Interrupt). They
>>> are up to PLIC to select which core to respond to. So it is hard to implement
>>> a clockevent per cpu core. I tested this with request_percpu_irq() and it
>>> failed.
>> You cannot use request_percpu_irq(), but the driver should be able to set the
>> affinity of each IRQ to a separate CPU.
> Absolutely. And given the bad rating of the local timers, it may be worth to implement this driver in a per CPU (affinity set) basis.
> At the first glance, the arm_global_timer can be used as an example.
> Note in this case, you may want to double check what does with an idle state with a local timer stop flag and this timer which is always on.

Hi Daniel and Samuel,

Thanks for your pointers. I will check it. If it works, I will send the new version of this patch.

Best regards,
Xingyu Wu

Powered by blists - more mailing lists