All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marc Zyngier <maz@kernel.org>
To: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Cc: Mark Rutland <mark.rutland@arm.com>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Shier <pshier@google.com>,
	Raghavendra Rao Ananta <rananta@google.com>,
	Ricardo Koller <ricarkol@google.com>,
	Oliver Upton <oupton@google.com>, Will Deacon <will@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	kernel-team@android.com
Subject: [PATCH v3 06/17] clocksource/arm_arch_timer: Fix MMIO base address vs callback ordering issue
Date: Sun, 10 Oct 2021 12:42:55 +0100	[thread overview]
Message-ID: <20211010114306.2910453-7-maz@kernel.org> (raw)
In-Reply-To: <20211010114306.2910453-1-maz@kernel.org>

The MMIO timer base address gets published after we have registered
the callbacks and the interrupt handler, which is... a bit dangerous.

Fix this by moving the base address publication to the point where
we register the timer, and expose a pointer to the timer structure
itself rather than a naked value.

Reviewed-by: Oliver Upton <oupton@google.com>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/clocksource/arm_arch_timer.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 8afe8c814eba..bede10f67f9a 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -54,13 +54,13 @@
 
 static unsigned arch_timers_present __initdata;
 
-static void __iomem *arch_counter_base __ro_after_init;
-
 struct arch_timer {
 	void __iomem *base;
 	struct clock_event_device evt;
 };
 
+static struct arch_timer *arch_timer_mem __ro_after_init;
+
 #define to_arch_timer(e) container_of(e, struct arch_timer, evt)
 
 static u32 arch_timer_rate __ro_after_init;
@@ -973,9 +973,9 @@ static u64 arch_counter_get_cntvct_mem(void)
 	u32 vct_lo, vct_hi, tmp_hi;
 
 	do {
-		vct_hi = readl_relaxed(arch_counter_base + CNTVCT_HI);
-		vct_lo = readl_relaxed(arch_counter_base + CNTVCT_LO);
-		tmp_hi = readl_relaxed(arch_counter_base + CNTVCT_HI);
+		vct_hi = readl_relaxed(arch_timer_mem->base + CNTVCT_HI);
+		vct_lo = readl_relaxed(arch_timer_mem->base + CNTVCT_LO);
+		tmp_hi = readl_relaxed(arch_timer_mem->base + CNTVCT_HI);
 	} while (vct_hi != tmp_hi);
 
 	return ((u64) vct_hi << 32) | vct_lo;
@@ -1166,25 +1166,25 @@ static int __init arch_timer_mem_register(void __iomem *base, unsigned int irq)
 {
 	int ret;
 	irq_handler_t func;
-	struct arch_timer *t;
 
-	t = kzalloc(sizeof(*t), GFP_KERNEL);
-	if (!t)
+	arch_timer_mem = kzalloc(sizeof(*arch_timer_mem), GFP_KERNEL);
+	if (!arch_timer_mem)
 		return -ENOMEM;
 
-	t->base = base;
-	t->evt.irq = irq;
-	__arch_timer_setup(ARCH_TIMER_TYPE_MEM, &t->evt);
+	arch_timer_mem->base = base;
+	arch_timer_mem->evt.irq = irq;
+	__arch_timer_setup(ARCH_TIMER_TYPE_MEM, &arch_timer_mem->evt);
 
 	if (arch_timer_mem_use_virtual)
 		func = arch_timer_handler_virt_mem;
 	else
 		func = arch_timer_handler_phys_mem;
 
-	ret = request_irq(irq, func, IRQF_TIMER, "arch_mem_timer", &t->evt);
+	ret = request_irq(irq, func, IRQF_TIMER, "arch_mem_timer", &arch_timer_mem->evt);
 	if (ret) {
 		pr_err("Failed to request mem timer irq\n");
-		kfree(t);
+		kfree(arch_timer_mem);
+		arch_timer_mem = NULL;
 	}
 
 	return ret;
@@ -1442,7 +1442,6 @@ arch_timer_mem_frame_register(struct arch_timer_mem_frame *frame)
 		return ret;
 	}
 
-	arch_counter_base = base;
 	arch_timers_present |= ARCH_TIMER_TYPE_MEM;
 
 	return 0;
-- 
2.30.2


WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org>
To: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Cc: Mark Rutland <mark.rutland@arm.com>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Shier <pshier@google.com>,
	Raghavendra Rao Ananta <rananta@google.com>,
	Ricardo Koller <ricarkol@google.com>,
	Oliver Upton <oupton@google.com>, Will Deacon <will@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	kernel-team@android.com
Subject: [PATCH v3 06/17] clocksource/arm_arch_timer: Fix MMIO base address vs callback ordering issue
Date: Sun, 10 Oct 2021 12:42:55 +0100	[thread overview]
Message-ID: <20211010114306.2910453-7-maz@kernel.org> (raw)
In-Reply-To: <20211010114306.2910453-1-maz@kernel.org>

The MMIO timer base address gets published after we have registered
the callbacks and the interrupt handler, which is... a bit dangerous.

Fix this by moving the base address publication to the point where
we register the timer, and expose a pointer to the timer structure
itself rather than a naked value.

Reviewed-by: Oliver Upton <oupton@google.com>
Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/clocksource/arm_arch_timer.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 8afe8c814eba..bede10f67f9a 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -54,13 +54,13 @@
 
 static unsigned arch_timers_present __initdata;
 
-static void __iomem *arch_counter_base __ro_after_init;
-
 struct arch_timer {
 	void __iomem *base;
 	struct clock_event_device evt;
 };
 
+static struct arch_timer *arch_timer_mem __ro_after_init;
+
 #define to_arch_timer(e) container_of(e, struct arch_timer, evt)
 
 static u32 arch_timer_rate __ro_after_init;
@@ -973,9 +973,9 @@ static u64 arch_counter_get_cntvct_mem(void)
 	u32 vct_lo, vct_hi, tmp_hi;
 
 	do {
-		vct_hi = readl_relaxed(arch_counter_base + CNTVCT_HI);
-		vct_lo = readl_relaxed(arch_counter_base + CNTVCT_LO);
-		tmp_hi = readl_relaxed(arch_counter_base + CNTVCT_HI);
+		vct_hi = readl_relaxed(arch_timer_mem->base + CNTVCT_HI);
+		vct_lo = readl_relaxed(arch_timer_mem->base + CNTVCT_LO);
+		tmp_hi = readl_relaxed(arch_timer_mem->base + CNTVCT_HI);
 	} while (vct_hi != tmp_hi);
 
 	return ((u64) vct_hi << 32) | vct_lo;
@@ -1166,25 +1166,25 @@ static int __init arch_timer_mem_register(void __iomem *base, unsigned int irq)
 {
 	int ret;
 	irq_handler_t func;
-	struct arch_timer *t;
 
-	t = kzalloc(sizeof(*t), GFP_KERNEL);
-	if (!t)
+	arch_timer_mem = kzalloc(sizeof(*arch_timer_mem), GFP_KERNEL);
+	if (!arch_timer_mem)
 		return -ENOMEM;
 
-	t->base = base;
-	t->evt.irq = irq;
-	__arch_timer_setup(ARCH_TIMER_TYPE_MEM, &t->evt);
+	arch_timer_mem->base = base;
+	arch_timer_mem->evt.irq = irq;
+	__arch_timer_setup(ARCH_TIMER_TYPE_MEM, &arch_timer_mem->evt);
 
 	if (arch_timer_mem_use_virtual)
 		func = arch_timer_handler_virt_mem;
 	else
 		func = arch_timer_handler_phys_mem;
 
-	ret = request_irq(irq, func, IRQF_TIMER, "arch_mem_timer", &t->evt);
+	ret = request_irq(irq, func, IRQF_TIMER, "arch_mem_timer", &arch_timer_mem->evt);
 	if (ret) {
 		pr_err("Failed to request mem timer irq\n");
-		kfree(t);
+		kfree(arch_timer_mem);
+		arch_timer_mem = NULL;
 	}
 
 	return ret;
@@ -1442,7 +1442,6 @@ arch_timer_mem_frame_register(struct arch_timer_mem_frame *frame)
 		return ret;
 	}
 
-	arch_counter_base = base;
 	arch_timers_present |= ARCH_TIMER_TYPE_MEM;
 
 	return 0;
-- 
2.30.2


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

  parent reply	other threads:[~2021-10-10 11:43 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-10 11:42 [PATCH v3 00/17] clocksource/arm_arch_timer: Add basic ARMv8.6 support Marc Zyngier
2021-10-10 11:42 ` Marc Zyngier
2021-10-10 11:42 ` [PATCH v3 01/17] clocksource/arm_arch_timer: Add build-time guards for unhandled register accesses Marc Zyngier
2021-10-10 11:42   ` Marc Zyngier
2021-10-10 11:42 ` [PATCH v3 02/17] clocksource/arm_arch_timer: Drop CNT*_TVAL read accessors Marc Zyngier
2021-10-10 11:42   ` Marc Zyngier
2021-10-10 11:42 ` [PATCH v3 03/17] clocksource/arm_arch_timer: Extend write side of timer register accessors to u64 Marc Zyngier
2021-10-10 11:42   ` Marc Zyngier
2021-10-10 11:42 ` [PATCH v3 04/17] clocksource/arm_arch_timer: Move system register timer programming over to CVAL Marc Zyngier
2021-10-10 11:42   ` Marc Zyngier
2021-10-10 11:42 ` [PATCH v3 05/17] clocksource/arm_arch_timer: Move drop _tval from erratum function names Marc Zyngier
2021-10-10 11:42   ` Marc Zyngier
2021-10-10 11:42 ` Marc Zyngier [this message]
2021-10-10 11:42   ` [PATCH v3 06/17] clocksource/arm_arch_timer: Fix MMIO base address vs callback ordering issue Marc Zyngier
2021-10-10 11:42 ` [PATCH v3 07/17] clocksource/arm_arch_timer: Move MMIO timer programming over to CVAL Marc Zyngier
2021-10-10 11:42   ` Marc Zyngier
2021-10-10 11:42 ` [PATCH v3 08/17] clocksource/arm_arch_timer: Advertise 56bit timer to the core code Marc Zyngier
2021-10-10 11:42   ` Marc Zyngier
2021-10-10 11:42 ` [PATCH v3 09/17] clocksource/arm_arch_timer: Work around broken CVAL implementations Marc Zyngier
2021-10-10 11:42   ` Marc Zyngier
2021-10-10 11:42 ` [PATCH v3 10/17] clocksource/arm_arch_timer: Remove any trace of the TVAL programming interface Marc Zyngier
2021-10-10 11:42   ` Marc Zyngier
2021-10-10 11:43 ` [PATCH v3 11/17] clocksource/arm_arch_timer: Drop unnecessary ISB on CVAL programming Marc Zyngier
2021-10-10 11:43   ` Marc Zyngier
2021-10-10 11:43 ` [PATCH v3 12/17] clocksource/arm_arch_timer: Fix masking for high freq counters Marc Zyngier
2021-10-10 11:43   ` Marc Zyngier
2021-10-10 11:43 ` [PATCH v3 13/17] clocksource/arch_arm_timer: Move workaround synchronisation around Marc Zyngier
2021-10-10 11:43   ` Marc Zyngier
2021-10-10 11:43 ` [PATCH v3 14/17] arm64: Add a capability for FEAT_ECV Marc Zyngier
2021-10-10 11:43   ` Marc Zyngier
2021-10-11 10:56   ` Will Deacon
2021-10-11 10:56     ` Will Deacon
2021-10-11 10:57     ` Will Deacon
2021-10-11 10:57       ` Will Deacon
2021-10-10 11:43 ` [PATCH v3 15/17] arm64: Add CNT{P,V}CTSS_EL0 alternatives to cnt{p,v}ct_el0 Marc Zyngier
2021-10-10 11:43   ` [PATCH v3 15/17] arm64: Add CNT{P, V}CTSS_EL0 alternatives to cnt{p, v}ct_el0 Marc Zyngier
2021-10-11 10:33   ` [PATCH v3 15/17] arm64: Add CNT{P,V}CTSS_EL0 alternatives to cnt{p,v}ct_el0 Will Deacon
2021-10-11 10:33     ` Will Deacon
2021-10-10 11:43 ` [PATCH v3 16/17] arm64: Add handling of CNTVCTSS traps Marc Zyngier
2021-10-10 11:43   ` Marc Zyngier
2021-10-11 10:54   ` Will Deacon
2021-10-11 10:54     ` Will Deacon
2021-10-10 11:43 ` [PATCH v3 17/17] arm64: Add HWCAP for self-synchronising virtual counter Marc Zyngier
2021-10-10 11:43   ` Marc Zyngier
2021-10-11 11:00   ` Will Deacon
2021-10-11 11:00     ` Will Deacon
2021-10-11 11:02 ` [PATCH v3 00/17] clocksource/arm_arch_timer: Add basic ARMv8.6 support Will Deacon
2021-10-11 11:02   ` Will Deacon
2021-10-11 13:39   ` Marc Zyngier
2021-10-11 13:39     ` Marc Zyngier
2021-10-16 21:59     ` Daniel Lezcano
2021-10-16 21:59       ` Daniel Lezcano
2021-10-17  9:57       ` Marc Zyngier
2021-10-17  9:57         ` Marc Zyngier
2021-10-18  7:51         ` Daniel Lezcano
2021-10-18  7:51           ` Daniel Lezcano
2021-10-19  8:00           ` Marc Zyngier
2021-10-19  8:00             ` Marc Zyngier
2021-10-19  8:04             ` Daniel Lezcano
2021-10-19  8:04               ` Daniel Lezcano
2021-10-19 11:09           ` Will Deacon
2021-10-19 11:09             ` Will Deacon
2021-10-19 11:10             ` Daniel Lezcano
2021-10-19 11:10               ` Daniel Lezcano

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=20211010114306.2910453-7-maz@kernel.org \
    --to=maz@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=kernel-team@android.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=oupton@google.com \
    --cc=pshier@google.com \
    --cc=rananta@google.com \
    --cc=ricarkol@google.com \
    --cc=tglx@linutronix.de \
    --cc=will@kernel.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.