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
next prev 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: linkBe 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.