linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Valentin Schneider <valentin.schneider@arm.com>
To: Ionela Voinescu <ionela.voinescu@arm.com>,
	catalin.marinas@arm.com, will@kernel.org, mark.rutland@arm.com,
	maz@kernel.org, suzuki.poulose@arm.com, sudeep.holla@arm.com,
	dietmar.eggemann@arm.com
Cc: peterz@infradead.org, mingo@redhat.com, ggherdovich@suse.cz,
	vincent.guittot@linaro.org, linux-arm-kernel@lists.infradead.org,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 6/6] arm64: use activity monitors for frequency invariance
Date: Wed, 29 Jan 2020 23:39:11 +0000	[thread overview]
Message-ID: <3ed9af08-82ef-e30c-b1ec-3a1dac0d2091@arm.com> (raw)
In-Reply-To: <96fdead6-9896-5695-6744-413300d424f5@arm.com>

On 29/01/2020 17:13, Valentin Schneider wrote:
> I had a brief look at the Arm ARM, for the arch timer it says it is
> "typically in the range 1-50MHz", but then also gives an example with 20KHz
> in a low-power mode.
> 
> If we take say 5GHz max CPU frequency, our lower bound for the arch timer
> (with that SCHED_CAPACITY_SCALE² trick) is about ~4.768KHz. It's not *too*
> far from that 20KHz, but I'm not sure we would actually be executing stuff
> in that low-power mode.
> 

I mixed up a few things in there; that low-power mode is supposed to do
higher increments, so it would emulate a similar frequency as the non-low-power
mode. Thus the actual frequency matters less than what is reported in CNTFRQ
(though we hope to get the behaviour we're told we should see), so we should
be quite safe from that ~5KHz value. Still, to make it obvious, I don't think
something like this would hurt:

---
diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 9a5464c625b45..a72832093575a 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -885,6 +885,17 @@ static int arch_timer_starting_cpu(unsigned int cpu)
 	return 0;
 }
 
+static int validate_timer_rate(void)
+{
+	if (!arch_timer_rate)
+		return 1;
+
+	/* Arch timer frequency < 1MHz is shady */
+	WARN_ON(arch_timer_rate < 1000000);
+
+	return 0;
+}
+
 /*
  * For historical reasons, when probing with DT we use whichever (non-zero)
  * rate was probed first, and don't verify that others match. If the first node
@@ -900,7 +911,7 @@ static void arch_timer_of_configure_rate(u32 rate, struct device_node *np)
 		arch_timer_rate = rate;
 
 	/* Check the timer frequency. */
-	if (arch_timer_rate == 0)
+	if (validate_timer_rate())
 		pr_warn("frequency not available\n");
 }
 
@@ -1594,7 +1605,7 @@ static int __init arch_timer_acpi_init(struct acpi_table_header *table)
 	 * CNTFRQ value. This *must* be correct.
 	 */
 	arch_timer_rate = arch_timer_get_cntfrq();
-	if (!arch_timer_rate) {
+	if (validate_timer_rate()) {
 		pr_err(FW_BUG "frequency not available.\n");
 		return -EINVAL;
 	}
---

> Long story short, we're probably fine, but it would nice to shove some of
> the above into comments (especially the SCHED_CAPACITY_SCALE² trick)
> 

  parent reply	other threads:[~2020-01-29 23:39 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-18 18:26 [PATCH v2 0/6] arm64: ARMv8.4 Activity Monitors support Ionela Voinescu
2019-12-18 18:26 ` [PATCH v2 1/6] arm64: add support for the AMU extension v1 Ionela Voinescu
2020-01-23 17:04   ` Valentin Schneider
2020-01-23 18:32     ` Ionela Voinescu
2020-01-24 12:00       ` Valentin Schneider
2020-01-28 11:00         ` Ionela Voinescu
2020-01-28 16:34   ` Suzuki Kuruppassery Poulose
2020-01-29 16:42     ` Ionela Voinescu
2019-12-18 18:26 ` [PATCH v2 2/6] arm64: trap to EL1 accesses to AMU counters from EL0 Ionela Voinescu
2020-01-23 17:04   ` Valentin Schneider
2020-01-23 17:34     ` Ionela Voinescu
2019-12-18 18:26 ` [PATCH v2 3/6] arm64/kvm: disable access to AMU registers from kvm guests Ionela Voinescu
2020-01-27 15:33   ` Valentin Schneider
2020-01-28 15:48     ` Ionela Voinescu
2020-01-28 17:26     ` Suzuki Kuruppassery Poulose
2020-01-28 17:37       ` Valentin Schneider
2020-01-28 17:52         ` Ionela Voinescu
2019-12-18 18:26 ` [PATCH v2 4/6] Documentation: arm64: document support for the AMU extension Ionela Voinescu
2020-01-27 16:47   ` Valentin Schneider
2020-01-28 16:53     ` Ionela Voinescu
2020-01-28 18:36       ` Valentin Schneider
2020-01-30 15:04   ` Suzuki Kuruppassery Poulose
2020-01-30 16:45     ` Ionela Voinescu
2020-01-30 18:26       ` Suzuki K Poulose
2020-01-31  9:54         ` Ionela Voinescu
2019-12-18 18:26 ` [PATCH v2 5/6] TEMP: sched: add interface for counter-based frequency invariance Ionela Voinescu
2020-01-29 19:37   ` Peter Zijlstra
2020-01-30 15:33     ` Ionela Voinescu
2019-12-18 18:26 ` [PATCH v2 6/6] arm64: use activity monitors for " Ionela Voinescu
2020-01-23 11:49   ` Lukasz Luba
2020-01-23 17:07     ` Ionela Voinescu
2020-01-24  1:19       ` Lukasz Luba
2020-01-24 13:12         ` Ionela Voinescu
2020-01-24 15:17           ` Lukasz Luba
2020-01-28 17:36             ` Ionela Voinescu
2020-01-29 17:13   ` Valentin Schneider
2020-01-29 17:52     ` Ionela Voinescu
2020-01-29 23:39     ` Valentin Schneider [this message]
2020-01-30 15:49       ` Ionela Voinescu
2020-01-30 16:11         ` Valentin Schneider

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3ed9af08-82ef-e30c-b1ec-3a1dac0d2091@arm.com \
    --to=valentin.schneider@arm.com \
    --cc=catalin.marinas@arm.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=ggherdovich@suse.cz \
    --cc=ionela.voinescu@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=maz@kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=sudeep.holla@arm.com \
    --cc=suzuki.poulose@arm.com \
    --cc=vincent.guittot@linaro.org \
    --cc=will@kernel.org \
    --subject='Re: [PATCH v2 6/6] arm64: use activity monitors for frequency invariance' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).