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 v2 06/16] clocksource/arm_arch_timer: Fix MMIO base address vs callback ordering issue
Date: Wed, 22 Sep 2021 22:19:31 +0100 [thread overview]
Message-ID: <20210922211941.2756270-7-maz@kernel.org> (raw)
In-Reply-To: <20210922211941.2756270-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
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 v2 06/16] clocksource/arm_arch_timer: Fix MMIO base address vs callback ordering issue
Date: Wed, 22 Sep 2021 22:19:31 +0100 [thread overview]
Message-ID: <20210922211941.2756270-7-maz@kernel.org> (raw)
In-Reply-To: <20210922211941.2756270-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
next prev parent reply other threads:[~2021-09-22 21:22 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-22 21:19 [PATCH v2 00/16] clocksource/arm_arch_timer: Add basic ARMv8.6 support Marc Zyngier
2021-09-22 21:19 ` Marc Zyngier
2021-09-22 21:19 ` [PATCH v2 01/16] clocksource/arm_arch_timer: Add build-time guards for unhandled register accesses Marc Zyngier
2021-09-22 21:19 ` Marc Zyngier
2021-09-22 21:19 ` [PATCH v2 02/16] clocksource/arm_arch_timer: Drop CNT*_TVAL read accessors Marc Zyngier
2021-09-22 21:19 ` Marc Zyngier
2021-09-22 21:19 ` [PATCH v2 03/16] clocksource/arm_arch_timer: Extend write side of timer register accessors to u64 Marc Zyngier
2021-09-22 21:19 ` Marc Zyngier
2021-09-22 21:19 ` [PATCH v2 04/16] clocksource/arm_arch_timer: Move system register timer programming over to CVAL Marc Zyngier
2021-09-22 21:19 ` Marc Zyngier
2021-09-22 21:19 ` [PATCH v2 05/16] clocksource/arm_arch_timer: Move drop _tval from erratum function names Marc Zyngier
2021-09-22 21:19 ` Marc Zyngier
2021-09-22 21:19 ` Marc Zyngier [this message]
2021-09-22 21:19 ` [PATCH v2 06/16] clocksource/arm_arch_timer: Fix MMIO base address vs callback ordering issue Marc Zyngier
2021-09-22 21:19 ` [PATCH v2 07/16] clocksource/arm_arch_timer: Move MMIO timer programming over to CVAL Marc Zyngier
2021-09-22 21:19 ` Marc Zyngier
2021-09-22 21:19 ` [PATCH v2 08/16] clocksource/arm_arch_timer: Advertise 56bit timer to the core code Marc Zyngier
2021-09-22 21:19 ` Marc Zyngier
2021-09-22 21:19 ` [PATCH v2 09/16] clocksource/arm_arch_timer: Work around broken CVAL implementations Marc Zyngier
2021-09-22 21:19 ` Marc Zyngier
2021-09-22 21:19 ` [PATCH v2 10/16] clocksource/arm_arch_timer: Remove any trace of the TVAL programming interface Marc Zyngier
2021-09-22 21:19 ` Marc Zyngier
2021-09-22 21:19 ` [PATCH v2 11/16] clocksource/arm_arch_timer: Drop unnecessary ISB on CVAL programming Marc Zyngier
2021-09-22 21:19 ` Marc Zyngier
2021-09-22 21:19 ` [PATCH v2 12/16] clocksource/arm_arch_timer: Fix masking for high freq counters Marc Zyngier
2021-09-22 21:19 ` Marc Zyngier
2021-09-22 21:19 ` [PATCH v2 13/16] clocksource/arch_arm_timer: Move workaround synchronisation around Marc Zyngier
2021-09-22 21:19 ` Marc Zyngier
2021-09-22 21:19 ` [PATCH v2 14/16] arm64: Add a capability for FEAT_ECV Marc Zyngier
2021-09-22 21:19 ` Marc Zyngier
2021-09-29 16:03 ` Will Deacon
2021-09-29 16:03 ` Will Deacon
2021-09-30 7:42 ` Marc Zyngier
2021-09-30 7:42 ` Marc Zyngier
2021-09-30 8:30 ` Will Deacon
2021-09-30 8:30 ` Will Deacon
2021-09-22 21:19 ` [PATCH v2 15/16] arm64: Add CNT{P, V}CTSS_EL0 alternatives to cnt{p, v}ct_el0 Marc Zyngier
2021-09-22 21:19 ` [PATCH v2 15/16] arm64: Add CNT{P,V}CTSS_EL0 alternatives to cnt{p,v}ct_el0 Marc Zyngier
2021-09-22 21:19 ` [PATCH v2 16/16] arm64: Add handling of CNTVCTSS traps Marc Zyngier
2021-09-22 21:19 ` Marc Zyngier
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=20210922211941.2756270-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.