[PATCH] clocksource/drivers/arm_arch_timer: Initialize evtstrm after finalizing cpucaps

[PATCH] clocksource/drivers/arm_arch_timer: Initialize evtstrm after finalizing cpucaps

[Mark Rutland]

We attempt to initialize each CPU's arch_timer event stream in
arch_timer_evtstrm_enable(), which we call from the
arch_timer_starting_cpu() cpu hotplug callback which is registered early
in boot. As this is registered before we initialize the system cpucaps,
the test for ARM64_HAS_ECV will always be false for CPUs present at boot
time, and will only be taken into account for CPUs onlined late
(including those which are hotplugged out and in again).

Due to this, CPUs present and boot time may not use the intended divider
and scale factor to generate the event stream, and may differ from other
CPUs.

Correct this by only initializing the event stream after cpucaps have been
finalized, registering a separate CPU hotplug callback for the event stream
configuration. Since the caps must be finalized by this point, use
spus_have_final_cap() to verify this.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/clocksource/arm_arch_timer.c | 31 +++++++++++++++++++++++-----
 include/linux/cpuhotplug.h           |  1 +
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index e733a2a1927a4..8f7eae19c6fac 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -910,7 +910,7 @@ static void arch_timer_evtstrm_enable(unsigned int divider)
 
 #ifdef CONFIG_ARM64
 	/* ECV is likely to require a large divider. Use the EVNTIS flag. */
-	if (cpus_have_const_cap(ARM64_HAS_ECV) && divider > 15) {
+	if (cpus_have_final_cap(ARM64_HAS_ECV) && divider > 15) {
 		cntkctl |= ARCH_TIMER_EVT_INTERVAL_SCALE;
 		divider -= 8;
 	}
@@ -948,6 +948,30 @@ static void arch_timer_configure_evtstream(void)
 	arch_timer_evtstrm_enable(max(0, lsb));
 }
 
+static int arch_timer_evtstrm_starting_cpu(unsigned int cpu)
+{
+	arch_timer_configure_evtstream();
+	return 0;
+}
+
+static int arch_timer_evtstrm_dying_cpu(unsigned int cpu)
+{
+	cpumask_clear_cpu(smp_processor_id(), &evtstrm_available);
+	return 0;
+}
+
+static int __init arch_timer_evtstrm_register(void)
+{
+	if (!arch_timer_evt || !evtstrm_enable)
+		return 0;
+
+	return cpuhp_setup_state(CPUHP_AP_ARM_ARCH_TIMER_EVTSTRM_STARTING,
+				 "clockevents/arm/arch_timer_evtstrm:starting",
+				 arch_timer_evtstrm_starting_cpu,
+				 arch_timer_evtstrm_dying_cpu);
+}
+core_initcall(arch_timer_evtstrm_register);
+
 static void arch_counter_set_user_access(void)
 {
 	u32 cntkctl = arch_timer_get_cntkctl();
@@ -1009,8 +1033,6 @@ static int arch_timer_starting_cpu(unsigned int cpu)
 	}
 
 	arch_counter_set_user_access();
-	if (evtstrm_enable)
-		arch_timer_configure_evtstream();
 
 	return 0;
 }
@@ -1157,8 +1179,6 @@ static int arch_timer_dying_cpu(unsigned int cpu)
 {
 	struct clock_event_device *clk = this_cpu_ptr(arch_timer_evt);
 
-	cpumask_clear_cpu(smp_processor_id(), &evtstrm_available);
-
 	arch_timer_stop(clk);
 	return 0;
 }
@@ -1272,6 +1292,7 @@ static int __init arch_timer_register(void)
 
 out_free:
 	free_percpu(arch_timer_evt);
+	arch_timer_evt = NULL;
 out:
 	return err;
 }
diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
index 25b6e6e6ba6bc..6726ba52361f1 100644
--- a/include/linux/cpuhotplug.h
+++ b/include/linux/cpuhotplug.h
@@ -173,6 +173,7 @@ enum cpuhp_state {
 	CPUHP_AP_ARM_L2X0_STARTING,
 	CPUHP_AP_EXYNOS4_MCT_TIMER_STARTING,
 	CPUHP_AP_ARM_ARCH_TIMER_STARTING,
+	CPUHP_AP_ARM_ARCH_TIMER_EVTSTRM_STARTING,
 	CPUHP_AP_ARM_GLOBAL_TIMER_STARTING,
 	CPUHP_AP_JCORE_TIMER_STARTING,
 	CPUHP_AP_ARM_TWD_STARTING,
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] clocksource/drivers/arm_arch_timer: Initialize evtstrm after finalizing cpucaps

[Thomas Gleixner]

On Thu, Sep 07 2023 at 14:34, Mark Rutland wrote:

> We attempt to initialize each CPU's arch_timer event stream in
> arch_timer_evtstrm_enable(), which we call from the
> arch_timer_starting_cpu() cpu hotplug callback which is registered early
> in boot. As this is registered before we initialize the system cpucaps,
> the test for ARM64_HAS_ECV will always be false for CPUs present at boot
> time, and will only be taken into account for CPUs onlined late
> (including those which are hotplugged out and in again).
>
> Due to this, CPUs present and boot time may not use the intended divider
> and scale factor to generate the event stream, and may differ from other
> CPUs.
>
> Correct this by only initializing the event stream after cpucaps have been
> finalized, registering a separate CPU hotplug callback for the event stream
> configuration. Since the caps must be finalized by this point, use
> spus_have_final_cap() to verify this.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>

Acked-by: Thomas Gleixner <tglx@linutronix.de>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] clocksource/drivers/arm_arch_timer: Initialize evtstrm after finalizing cpucaps

[Marc Zyngier]

On Thu, 07 Sep 2023 14:34:10 +0100,
Mark Rutland <mark.rutland@arm.com> wrote:
> 
> We attempt to initialize each CPU's arch_timer event stream in
> arch_timer_evtstrm_enable(), which we call from the
> arch_timer_starting_cpu() cpu hotplug callback which is registered early
> in boot. As this is registered before we initialize the system cpucaps,
> the test for ARM64_HAS_ECV will always be false for CPUs present at boot
> time, and will only be taken into account for CPUs onlined late
> (including those which are hotplugged out and in again).
> 
> Due to this, CPUs present and boot time may not use the intended divider
> and scale factor to generate the event stream, and may differ from other
> CPUs.
> 
> Correct this by only initializing the event stream after cpucaps have been
> finalized, registering a separate CPU hotplug callback for the event stream
> configuration. Since the caps must be finalized by this point, use
> spus_have_final_cap() to verify this.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>

Acked-by: Marc Zyngier <maz@kernel.org>

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] clocksource/drivers/arm_arch_timer: Initialize evtstrm after finalizing cpucaps

[Daniel Lezcano]

On 07/09/2023 15:34, Mark Rutland wrote:
> We attempt to initialize each CPU's arch_timer event stream in
> arch_timer_evtstrm_enable(), which we call from the
> arch_timer_starting_cpu() cpu hotplug callback which is registered early
> in boot. As this is registered before we initialize the system cpucaps,
> the test for ARM64_HAS_ECV will always be false for CPUs present at boot
> time, and will only be taken into account for CPUs onlined late
> (including those which are hotplugged out and in again).
> 
> Due to this, CPUs present and boot time may not use the intended divider
> and scale factor to generate the event stream, and may differ from other
> CPUs.
> 
> Correct this by only initializing the event stream after cpucaps have been
> finalized, registering a separate CPU hotplug callback for the event stream
> configuration. Since the caps must be finalized by this point, use
> spus_have_final_cap() to verify this.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> ---

Applied thanks


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] clocksource/drivers/arm_arch_timer: Initialize evtstrm after finalizing cpucaps

[Mark Rutland]

Hi Daniel,

On Wed, Oct 11, 2023 at 10:30:39AM +0200, Daniel Lezcano wrote:
> On 07/09/2023 15:34, Mark Rutland wrote:
> > We attempt to initialize each CPU's arch_timer event stream in
> > arch_timer_evtstrm_enable(), which we call from the
> > arch_timer_starting_cpu() cpu hotplug callback which is registered early
> > in boot. As this is registered before we initialize the system cpucaps,
> > the test for ARM64_HAS_ECV will always be false for CPUs present at boot
> > time, and will only be taken into account for CPUs onlined late
> > (including those which are hotplugged out and in again).
> > 
> > Due to this, CPUs present and boot time may not use the intended divider
> > and scale factor to generate the event stream, and may differ from other
> > CPUs.
> > 
> > Correct this by only initializing the event stream after cpucaps have been
> > finalized, registering a separate CPU hotplug callback for the event stream
> > configuration. Since the caps must be finalized by this point, use
> > spus_have_final_cap() to verify this.
> > 
> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
> > Cc: Marc Zyngier <maz@kernel.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > ---
> 
> Applied thanks

This got folded into a larger series that we were hoping to take through the arm64 tree:

  https://lore.kernel.org/linux-arm-kernel/20231010103139.3113421-1-mark.rutland@arm.com/
  https://lore.kernel.org/linux-arm-kernel/20231010103139.3113421-2-mark.rutland@arm.com/

I think that won't conflict, since all that's changed is the commit text, but
it might be worth dropping this patch for now to avoid the risk of a conflict.

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] clocksource/drivers/arm_arch_timer: Initialize evtstrm after finalizing cpucaps

[Daniel Lezcano]

On 11/10/2023 11:36, Mark Rutland wrote:
> Hi Daniel,
> 
> On Wed, Oct 11, 2023 at 10:30:39AM +0200, Daniel Lezcano wrote:
>> On 07/09/2023 15:34, Mark Rutland wrote:
>>> We attempt to initialize each CPU's arch_timer event stream in
>>> arch_timer_evtstrm_enable(), which we call from the
>>> arch_timer_starting_cpu() cpu hotplug callback which is registered early
>>> in boot. As this is registered before we initialize the system cpucaps,
>>> the test for ARM64_HAS_ECV will always be false for CPUs present at boot
>>> time, and will only be taken into account for CPUs onlined late
>>> (including those which are hotplugged out and in again).
>>>
>>> Due to this, CPUs present and boot time may not use the intended divider
>>> and scale factor to generate the event stream, and may differ from other
>>> CPUs.
>>>
>>> Correct this by only initializing the event stream after cpucaps have been
>>> finalized, registering a separate CPU hotplug callback for the event stream
>>> configuration. Since the caps must be finalized by this point, use
>>> spus_have_final_cap() to verify this.
>>>
>>> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
>>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>>> Cc: Marc Zyngier <maz@kernel.org>
>>> Cc: Thomas Gleixner <tglx@linutronix.de>
>>> ---
>>
>> Applied thanks
> 
> This got folded into a larger series that we were hoping to take through the arm64 tree:
> 
>    https://lore.kernel.org/linux-arm-kernel/20231010103139.3113421-1-mark.rutland@arm.com/
>    https://lore.kernel.org/linux-arm-kernel/20231010103139.3113421-2-mark.rutland@arm.com/
> 
> I think that won't conflict, since all that's changed is the commit text, but
> it might be worth dropping this patch for now to avoid the risk of a conflict.

Sure, thanks for letting me know. I was suspecting that was the case :)

I've dropped it

-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] clocksource/drivers/arm_arch_timer: Initialize evtstrm after finalizing cpucaps

[Mark Rutland]

On Wed, Oct 11, 2023 at 12:06:20PM +0200, Daniel Lezcano wrote:
> On 11/10/2023 11:36, Mark Rutland wrote:
> > On Wed, Oct 11, 2023 at 10:30:39AM +0200, Daniel Lezcano wrote:
> > > Applied thanks
> > 
> > This got folded into a larger series that we were hoping to take through the arm64 tree:
> > 
> >    https://lore.kernel.org/linux-arm-kernel/20231010103139.3113421-1-mark.rutland@arm.com/
> >    https://lore.kernel.org/linux-arm-kernel/20231010103139.3113421-2-mark.rutland@arm.com/
> > 
> > I think that won't conflict, since all that's changed is the commit text, but
> > it might be worth dropping this patch for now to avoid the risk of a conflict.
> 
> Sure, thanks for letting me know. I was suspecting that was the case :)
> 
> I've dropped it

Thanks, and sorry for the hassle!

Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel