linux-kernel - Re: [PATCH 2/2] clocksource/drivers/nxp-timer: Add the System Timer Module for the s32g platform

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: <e9216aaf-f53e-4256-bb73-489c2261d4c5@kernel.org>
Date: Tue, 25 Mar 2025 19:42:03 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Daniel Lezcano <daniel.lezcano@...aro.org>, tglx@...utronix.de
Cc: linux-kernel@...r.kernel.org, Thomas Fossati <thomas.fossati@...aro.org>,
 Larisa Grigore <Larisa.Grigore@....com>,
 Ghennadi Procopciuc <ghennadi.procopciuc@....com>,
 Maxime Coquelin <mcoquelin.stm32@...il.com>,
 Alexandre Torgue <alexandre.torgue@...s.st.com>,
 "moderated list:ARM/STM32 ARCHITECTURE"
 <linux-stm32@...md-mailman.stormreply.com>,
 "moderated list:ARM/STM32 ARCHITECTURE"
 <linux-arm-kernel@...ts.infradead.org>, dl-S32 <S32@....com>
Subject: Re: [PATCH 2/2] clocksource/drivers/nxp-timer: Add the System Timer
 Module for the s32g platform

On 25/03/2025 19:38, Daniel Lezcano wrote:
> On 25/03/2025 13:30, Krzysztof Kozlowski wrote:
>> On 25/03/2025 13:23, Daniel Lezcano wrote:
> 
> [ ... ]
> 
>>>>> +	if (!stm_instances->clocksource && (stm_instances->features & STM_CLKSRC)) {
>>>>> +
>>>>> +		/*
>>>>> +		 * First probed STM will be a clocksource
>>>>> +		 */
>>>>> +		ret = nxp_stm_clocksource_init(dev, name, base, clk);
>>>>> +		if (ret)
>>>>> +			return ret;
>>>>> +		stm_instances->clocksource++;
>>>>
>>>> That's racy. Devices can be brought async, ideally. This should be
>>>> rather idr or probably entire structure protected with a mutex.
>>>
>>> Mmh, interesting. I never had to think about this problem before
>>>
>>> Do you know at what moment the probing is parallelized ?
>>
>> You don't have PROBE_PREFER_ASYNCHRONOUS, so currently this will be
>> still sync, but I don't think we want it to be that way forever. I think
>> new drivers should not rely on implicit sync, because converting it
>> later to async will be difficult. It's easier to design it now or even
>> choose async explicitly (after testing).
> 
> I gave a try and sometimes I reach the warnings below. I suspect the 
> underlying code in the time framework is not yet ready for that.
> 
> Even if it could be a good candidate for parallelizing the boot, this 
> driver should stay sync ATM. Except if someone has the willing to dig 
> into the core framework to find out the race when switching the 
> clockevent. I think a thread is setting a timer while we are switching 
> the driver.
> 
> IMO, this core framework is too sensitive for this kind of change now.
> 
> Alternatively, I can put anyway the lock which is harmless for the sync 
> code but making the driver race free. The async flag can be put later.


Yes, that's what I meant, although indeed good point that clocksource is
way too early to be async. In that case this part is up to you, maybe my
suggestion was not correct.


Best regards,
Krzysztof