linux-kernel - Re: [PATCH v6 1/6] ARM: make register_current_timer

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: <aSCmvsOc13Z1TS6y@google.com>
Date: Fri, 21 Nov 2025 17:51:58 +0000
From: William McVicker <willmcvicker@...gle.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will@...nel.org>,
	Daniel Lezcano <daniel.lezcano@...aro.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Krzysztof Kozlowski <krzk@...nel.org>,
	Alim Akhtar <alim.akhtar@...sung.com>,
	Donghoon Yu <hoony.yu@...sung.com>,
	Hosung Kim <hosung0.kim@...sung.com>, Rob Herring <robh@...nel.org>,
	John Stultz <jstultz@...gle.com>,
	Youngmin Nam <youngmin.nam@...sung.com>,
	Peter Griffin <peter.griffin@...aro.org>,
	Tudor Ambarus <tudor.ambarus@...aro.org>,
	André Draszik <andre.draszik@...aro.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Marek Szyprowski <m.szyprowski@...sung.com>,
	linux-samsung-soc@...r.kernel.org, kernel-team@...roid.com,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 1/6] ARM: make register_current_timer_delay()
 accessible after init

Hi Daniel and Russell,

On 11/21/2025, Russell King (Oracle) wrote:
> On Thu, Nov 20, 2025 at 06:42:29PM +0000, Will McVicker wrote:
> > The function register_current_timer_delay() is called from the
> > exynos_mct clocksource driver at probe time. In the event that the
> > exynos_mct driver is probed deferred or the platform manually unbinds
> > and rebinds the driver we need this function available. So drop the
> > __init tag.
> 
> First question. Why.

Sorry for not explaining this very well. Let me fill in the gaps.

> 
> Second, have you analysed the code to check that you _can_ call this
> after init time?
> 
> Let's look at the code:
> 
>         if (!delay_calibrated && (!delay_res || (res < delay_res))) {
>         } else {
>                 pr_info("Ignoring duplicate/late registration of read_current_ti
> mer delay\n");
>         }
> 
> So, if delay_calibrated is set, then this will fail. When is that set?
> It's set by calibrate_delay_is_known() and calibration_delay_done().
> When are these called? Basically after calibrate_delay() has finished.
> When is calibrate_delay() called? It's called by start_kernel(), while
> the init sections are still present.
> 
> So, calling this _after_ the init sections has been freed will result
> in the above pr_info() printed and this function doing *nothing*.
> So it's utterly pointless to call if the init sections have been freed.
> 
> Please find a different solution.

Sorry for wasting your time digging into this! You're right we shouldn't (and
don't) call register_current_timer_delay() after the init sections are freed.
This change was made purely to address the section mismatch compile-time
errors. The Exynos MCT driver cannot be compiled as a module for ARM 32-bit
devices; however, since ARM64 and ARM devices both call the function
exynos4_clocksource_init() and ARM64 configurations can set the MCT driver as
a module, we hit a section mismatch error due to calling an __init tagged
function -- register_current_timer_delay() -- from a non-init tagged function
-- exynos4_clocksource_init(). If you inspect the code, you will see that
register_current_timer_delay() is compiled out of the driver for ARM64. To
avoid hacking up the MCT driver with more `#if defined(CONFIG_ARM)` to handle
this section mismatch, I decided it was cleaner to drop the __init tag.

I'd be happy to re-factor the MCT driver to split up the ARM and ARM64 parts of
exynos4_clocksource_init() to avoid the section mismatch altogher in order to
keep the __init tag for register_current_timer_delay(). Given the comments
here, I think that makes sense.

I hope that explains it. Let me know if I overlooked anything.

Thanks,
Will