From 4775bc63f880001ee4fbd6456b12ab04674149e3 Mon Sep 17 00:00:00 2001 From: Marc Zyngier Date: Sun, 17 Oct 2021 13:42:09 +0100 Subject: clocksource/arm_arch_timer: Add build-time guards for unhandled register accesses As we are about to change the registers that are used by the driver, start by adding build-time checks to ensure that we always handle all registers and access modes. Suggested-by: Mark Rutland Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20211017124225.3018098-2-maz@kernel.org Signed-off-by: Daniel Lezcano --- drivers/clocksource/arm_arch_timer.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'drivers/clocksource') diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index be6d741d404c..3b7d46d9db73 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -112,6 +112,8 @@ void arch_timer_reg_write(int access, enum arch_timer_reg reg, u32 val, case ARCH_TIMER_REG_TVAL: writel_relaxed(val, timer->base + CNTP_TVAL); break; + default: + BUILD_BUG(); } } else if (access == ARCH_TIMER_MEM_VIRT_ACCESS) { struct arch_timer *timer = to_arch_timer(clk); @@ -122,6 +124,8 @@ void arch_timer_reg_write(int access, enum arch_timer_reg reg, u32 val, case ARCH_TIMER_REG_TVAL: writel_relaxed(val, timer->base + CNTV_TVAL); break; + default: + BUILD_BUG(); } } else { arch_timer_reg_write_cp15(access, reg, val); @@ -143,6 +147,8 @@ u32 arch_timer_reg_read(int access, enum arch_timer_reg reg, case ARCH_TIMER_REG_TVAL: val = readl_relaxed(timer->base + CNTP_TVAL); break; + default: + BUILD_BUG(); } } else if (access == ARCH_TIMER_MEM_VIRT_ACCESS) { struct arch_timer *timer = to_arch_timer(clk); @@ -153,6 +159,8 @@ u32 arch_timer_reg_read(int access, enum arch_timer_reg reg, case ARCH_TIMER_REG_TVAL: val = readl_relaxed(timer->base + CNTV_TVAL); break; + default: + BUILD_BUG(); } } else { val = arch_timer_reg_read_cp15(access, reg); -- cgit From d72689988d67d56aebf7afb7f609373ea6b548db Mon Sep 17 00:00:00 2001 From: Marc Zyngier Date: Sun, 17 Oct 2021 13:42:10 +0100 Subject: clocksource/drivers/arm_arch_timer: Drop CNT*_TVAL read accessors The arch timer driver never reads the various TVAL registers, only writes to them. It is thus pointless to provide accessors for them and to implement errata workarounds. Drop these read-side accessors, and add a couple of BUG() statements for the time being. These statements will be removed further down the line. Reviewed-by: Oliver Upton Reviewed-by: Mark Rutland Tested-by: Mark Rutland Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20211017124225.3018098-3-maz@kernel.org Signed-off-by: Daniel Lezcano --- drivers/clocksource/arm_arch_timer.c | 44 ------------------------------------ 1 file changed, 44 deletions(-) (limited to 'drivers/clocksource') diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index 3b7d46d9db73..67bdc7288f59 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -144,9 +144,6 @@ u32 arch_timer_reg_read(int access, enum arch_timer_reg reg, case ARCH_TIMER_REG_CTRL: val = readl_relaxed(timer->base + CNTP_CTL); break; - case ARCH_TIMER_REG_TVAL: - val = readl_relaxed(timer->base + CNTP_TVAL); - break; default: BUILD_BUG(); } @@ -156,9 +153,6 @@ u32 arch_timer_reg_read(int access, enum arch_timer_reg reg, case ARCH_TIMER_REG_CTRL: val = readl_relaxed(timer->base + CNTV_CTL); break; - case ARCH_TIMER_REG_TVAL: - val = readl_relaxed(timer->base + CNTV_TVAL); - break; default: BUILD_BUG(); } @@ -247,16 +241,6 @@ struct ate_acpi_oem_info { _new; \ }) -static u32 notrace fsl_a008585_read_cntp_tval_el0(void) -{ - return __fsl_a008585_read_reg(cntp_tval_el0); -} - -static u32 notrace fsl_a008585_read_cntv_tval_el0(void) -{ - return __fsl_a008585_read_reg(cntv_tval_el0); -} - static u64 notrace fsl_a008585_read_cntpct_el0(void) { return __fsl_a008585_read_reg(cntpct_el0); @@ -293,16 +277,6 @@ static u64 notrace fsl_a008585_read_cntvct_el0(void) _new; \ }) -static u32 notrace hisi_161010101_read_cntp_tval_el0(void) -{ - return __hisi_161010101_read_reg(cntp_tval_el0); -} - -static u32 notrace hisi_161010101_read_cntv_tval_el0(void) -{ - return __hisi_161010101_read_reg(cntv_tval_el0); -} - static u64 notrace hisi_161010101_read_cntpct_el0(void) { return __hisi_161010101_read_reg(cntpct_el0); @@ -387,16 +361,6 @@ static u64 notrace sun50i_a64_read_cntvct_el0(void) { return __sun50i_a64_read_reg(cntvct_el0); } - -static u32 notrace sun50i_a64_read_cntp_tval_el0(void) -{ - return read_sysreg(cntp_cval_el0) - sun50i_a64_read_cntpct_el0(); -} - -static u32 notrace sun50i_a64_read_cntv_tval_el0(void) -{ - return read_sysreg(cntv_cval_el0) - sun50i_a64_read_cntvct_el0(); -} #endif #ifdef CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND @@ -446,8 +410,6 @@ static const struct arch_timer_erratum_workaround ool_workarounds[] = { .match_type = ate_match_dt, .id = "fsl,erratum-a008585", .desc = "Freescale erratum a005858", - .read_cntp_tval_el0 = fsl_a008585_read_cntp_tval_el0, - .read_cntv_tval_el0 = fsl_a008585_read_cntv_tval_el0, .read_cntpct_el0 = fsl_a008585_read_cntpct_el0, .read_cntvct_el0 = fsl_a008585_read_cntvct_el0, .set_next_event_phys = erratum_set_next_event_tval_phys, @@ -459,8 +421,6 @@ static const struct arch_timer_erratum_workaround ool_workarounds[] = { .match_type = ate_match_dt, .id = "hisilicon,erratum-161010101", .desc = "HiSilicon erratum 161010101", - .read_cntp_tval_el0 = hisi_161010101_read_cntp_tval_el0, - .read_cntv_tval_el0 = hisi_161010101_read_cntv_tval_el0, .read_cntpct_el0 = hisi_161010101_read_cntpct_el0, .read_cntvct_el0 = hisi_161010101_read_cntvct_el0, .set_next_event_phys = erratum_set_next_event_tval_phys, @@ -470,8 +430,6 @@ static const struct arch_timer_erratum_workaround ool_workarounds[] = { .match_type = ate_match_acpi_oem_info, .id = hisi_161010101_oem_info, .desc = "HiSilicon erratum 161010101", - .read_cntp_tval_el0 = hisi_161010101_read_cntp_tval_el0, - .read_cntv_tval_el0 = hisi_161010101_read_cntv_tval_el0, .read_cntpct_el0 = hisi_161010101_read_cntpct_el0, .read_cntvct_el0 = hisi_161010101_read_cntvct_el0, .set_next_event_phys = erratum_set_next_event_tval_phys, @@ -492,8 +450,6 @@ static const struct arch_timer_erratum_workaround ool_workarounds[] = { .match_type = ate_match_dt, .id = "allwinner,erratum-unknown1", .desc = "Allwinner erratum UNKNOWN1", - .read_cntp_tval_el0 = sun50i_a64_read_cntp_tval_el0, - .read_cntv_tval_el0 = sun50i_a64_read_cntv_tval_el0, .read_cntpct_el0 = sun50i_a64_read_cntpct_el0, .read_cntvct_el0 = sun50i_a64_read_cntvct_el0, .set_next_event_phys = erratum_set_next_event_tval_phys, -- cgit From 1e8d929231cf7b397101c5e6aaaa3d9bc9832f10 Mon Sep 17 00:00:00 2001 From: Marc Zyngier Date: Sun, 17 Oct 2021 13:42:11 +0100 Subject: clocksource/drivers/arm_arch_timer: Extend write side of timer register accessors to u64 The various accessors for the timer sysreg and MMIO registers are currently hardwired to 32bit. However, we are about to introduce the use of the CVAL registers, which require a 64bit access. Upgrade the write side of the accessors to take a 64bit value (the read side is left untouched as we don't plan to ever read back any of these registers). No functional change expected. Reviewed-by: Oliver Upton Reviewed-by: Mark Rutland Tested-by: Mark Rutland Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20211017124225.3018098-4-maz@kernel.org Signed-off-by: Daniel Lezcano --- drivers/clocksource/arm_arch_timer.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers/clocksource') diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index 67bdc7288f59..a49bcefaa370 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -100,17 +100,17 @@ early_param("clocksource.arm_arch_timer.evtstrm", early_evtstrm_cfg); */ static __always_inline -void arch_timer_reg_write(int access, enum arch_timer_reg reg, u32 val, +void arch_timer_reg_write(int access, enum arch_timer_reg reg, u64 val, struct clock_event_device *clk) { if (access == ARCH_TIMER_MEM_PHYS_ACCESS) { struct arch_timer *timer = to_arch_timer(clk); switch (reg) { case ARCH_TIMER_REG_CTRL: - writel_relaxed(val, timer->base + CNTP_CTL); + writel_relaxed((u32)val, timer->base + CNTP_CTL); break; case ARCH_TIMER_REG_TVAL: - writel_relaxed(val, timer->base + CNTP_TVAL); + writel_relaxed((u32)val, timer->base + CNTP_TVAL); break; default: BUILD_BUG(); @@ -119,10 +119,10 @@ void arch_timer_reg_write(int access, enum arch_timer_reg reg, u32 val, struct arch_timer *timer = to_arch_timer(clk); switch (reg) { case ARCH_TIMER_REG_CTRL: - writel_relaxed(val, timer->base + CNTV_CTL); + writel_relaxed((u32)val, timer->base + CNTV_CTL); break; case ARCH_TIMER_REG_TVAL: - writel_relaxed(val, timer->base + CNTV_TVAL); + writel_relaxed((u32)val, timer->base + CNTV_TVAL); break; default: BUILD_BUG(); -- cgit From a38b71b0833eb2fabd2b1fa37d665c0a88b8b7e4 Mon Sep 17 00:00:00 2001 From: Marc Zyngier Date: Sun, 17 Oct 2021 13:42:12 +0100 Subject: clocksource/drivers/arm_arch_timer: Move system register timer programming over to CVAL In order to cope better with high frequency counters, move the programming of the timers from the countdown timer (TVAL) over to the comparator (CVAL). The programming model is slightly different, as we now need to read the current counter value to have an absolute deadline instead of a relative one. There is a small overhead to this change, which we will address in the following patches. Reviewed-by: Oliver Upton Reviewed-by: Mark Rutland Tested-by: Mark Rutland Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20211017124225.3018098-5-maz@kernel.org Signed-off-by: Daniel Lezcano --- drivers/clocksource/arm_arch_timer.c | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) (limited to 'drivers/clocksource') diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index a49bcefaa370..322165468edf 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -691,10 +691,18 @@ static __always_inline void set_next_event(const int access, unsigned long evt, struct clock_event_device *clk) { unsigned long ctrl; + u64 cnt; + ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL, clk); ctrl |= ARCH_TIMER_CTRL_ENABLE; ctrl &= ~ARCH_TIMER_CTRL_IT_MASK; - arch_timer_reg_write(access, ARCH_TIMER_REG_TVAL, evt, clk); + + if (access == ARCH_TIMER_PHYS_ACCESS) + cnt = __arch_counter_get_cntpct(); + else + cnt = __arch_counter_get_cntvct(); + + arch_timer_reg_write(access, ARCH_TIMER_REG_CVAL, evt + cnt, clk); arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk); } @@ -712,17 +720,29 @@ static int arch_timer_set_next_event_phys(unsigned long evt, return 0; } +static __always_inline void set_next_event_mem(const int access, unsigned long evt, + struct clock_event_device *clk) +{ + unsigned long ctrl; + ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL, clk); + ctrl |= ARCH_TIMER_CTRL_ENABLE; + ctrl &= ~ARCH_TIMER_CTRL_IT_MASK; + + arch_timer_reg_write(access, ARCH_TIMER_REG_TVAL, evt, clk); + arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk); +} + static int arch_timer_set_next_event_virt_mem(unsigned long evt, struct clock_event_device *clk) { - set_next_event(ARCH_TIMER_MEM_VIRT_ACCESS, evt, clk); + set_next_event_mem(ARCH_TIMER_MEM_VIRT_ACCESS, evt, clk); return 0; } static int arch_timer_set_next_event_phys_mem(unsigned long evt, struct clock_event_device *clk) { - set_next_event(ARCH_TIMER_MEM_PHYS_ACCESS, evt, clk); + set_next_event_mem(ARCH_TIMER_MEM_PHYS_ACCESS, evt, clk); return 0; } -- cgit From ac9ef4f24cb2313fb047f2097396204b033799b8 Mon Sep 17 00:00:00 2001 From: Marc Zyngier Date: Sun, 17 Oct 2021 13:42:13 +0100 Subject: clocksource/drivers/arm_arch_timer: Move drop _tval from erratum function names The '_tval' name in the erratum handling function names doesn't make much sense anymore (and they were using CVAL the first place). Drop the _tval tag. Reviewed-by: Oliver Upton Reviewed-by: Mark Rutland Tested-by: Mark Rutland Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20211017124225.3018098-6-maz@kernel.org Signed-off-by: Daniel Lezcano --- drivers/clocksource/arm_arch_timer.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) (limited to 'drivers/clocksource') diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index 322165468edf..8afe8c814eba 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -369,7 +369,7 @@ EXPORT_SYMBOL_GPL(timer_unstable_counter_workaround); static atomic_t timer_unstable_counter_workaround_in_use = ATOMIC_INIT(0); -static void erratum_set_next_event_tval_generic(const int access, unsigned long evt, +static void erratum_set_next_event_generic(const int access, unsigned long evt, struct clock_event_device *clk) { unsigned long ctrl; @@ -390,17 +390,17 @@ static void erratum_set_next_event_tval_generic(const int access, unsigned long arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk); } -static __maybe_unused int erratum_set_next_event_tval_virt(unsigned long evt, +static __maybe_unused int erratum_set_next_event_virt(unsigned long evt, struct clock_event_device *clk) { - erratum_set_next_event_tval_generic(ARCH_TIMER_VIRT_ACCESS, evt, clk); + erratum_set_next_event_generic(ARCH_TIMER_VIRT_ACCESS, evt, clk); return 0; } -static __maybe_unused int erratum_set_next_event_tval_phys(unsigned long evt, +static __maybe_unused int erratum_set_next_event_phys(unsigned long evt, struct clock_event_device *clk) { - erratum_set_next_event_tval_generic(ARCH_TIMER_PHYS_ACCESS, evt, clk); + erratum_set_next_event_generic(ARCH_TIMER_PHYS_ACCESS, evt, clk); return 0; } @@ -412,8 +412,8 @@ static const struct arch_timer_erratum_workaround ool_workarounds[] = { .desc = "Freescale erratum a005858", .read_cntpct_el0 = fsl_a008585_read_cntpct_el0, .read_cntvct_el0 = fsl_a008585_read_cntvct_el0, - .set_next_event_phys = erratum_set_next_event_tval_phys, - .set_next_event_virt = erratum_set_next_event_tval_virt, + .set_next_event_phys = erratum_set_next_event_phys, + .set_next_event_virt = erratum_set_next_event_virt, }, #endif #ifdef CONFIG_HISILICON_ERRATUM_161010101 @@ -423,8 +423,8 @@ static const struct arch_timer_erratum_workaround ool_workarounds[] = { .desc = "HiSilicon erratum 161010101", .read_cntpct_el0 = hisi_161010101_read_cntpct_el0, .read_cntvct_el0 = hisi_161010101_read_cntvct_el0, - .set_next_event_phys = erratum_set_next_event_tval_phys, - .set_next_event_virt = erratum_set_next_event_tval_virt, + .set_next_event_phys = erratum_set_next_event_phys, + .set_next_event_virt = erratum_set_next_event_virt, }, { .match_type = ate_match_acpi_oem_info, @@ -432,8 +432,8 @@ static const struct arch_timer_erratum_workaround ool_workarounds[] = { .desc = "HiSilicon erratum 161010101", .read_cntpct_el0 = hisi_161010101_read_cntpct_el0, .read_cntvct_el0 = hisi_161010101_read_cntvct_el0, - .set_next_event_phys = erratum_set_next_event_tval_phys, - .set_next_event_virt = erratum_set_next_event_tval_virt, + .set_next_event_phys = erratum_set_next_event_phys, + .set_next_event_virt = erratum_set_next_event_virt, }, #endif #ifdef CONFIG_ARM64_ERRATUM_858921 @@ -452,8 +452,8 @@ static const struct arch_timer_erratum_workaround ool_workarounds[] = { .desc = "Allwinner erratum UNKNOWN1", .read_cntpct_el0 = sun50i_a64_read_cntpct_el0, .read_cntvct_el0 = sun50i_a64_read_cntvct_el0, - .set_next_event_phys = erratum_set_next_event_tval_phys, - .set_next_event_virt = erratum_set_next_event_tval_virt, + .set_next_event_phys = erratum_set_next_event_phys, + .set_next_event_virt = erratum_set_next_event_virt, }, #endif #ifdef CONFIG_ARM64_ERRATUM_1418040 -- cgit From 72f47a3f0ea4cda4ca5d90c0d6043f697b9b0647 Mon Sep 17 00:00:00 2001 From: Marc Zyngier Date: Sun, 17 Oct 2021 13:42:14 +0100 Subject: clocksource/drivers/arm_arch_timer: Fix MMIO base address vs callback ordering issue 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 Reviewed-by: Mark Rutland Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20211017124225.3018098-7-maz@kernel.org Signed-off-by: Daniel Lezcano --- drivers/clocksource/arm_arch_timer.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) (limited to 'drivers/clocksource') 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; -- cgit From 8b82c4f883a7b22660464c0232fbdb7a6deb3061 Mon Sep 17 00:00:00 2001 From: Marc Zyngier Date: Sun, 17 Oct 2021 13:42:15 +0100 Subject: clocksource/drivers/arm_arch_timer: Move MMIO timer programming over to CVAL Similarily to the sysreg-based timer, move the MMIO over to using the CVAL registers instead of TVAL. Note that there is no warranty that the 64bit MMIO access will be atomic, but the timer is always disabled at the point where we program CVAL. Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20211017124225.3018098-8-maz@kernel.org Signed-off-by: Daniel Lezcano --- drivers/clocksource/arm_arch_timer.c | 50 +++++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 12 deletions(-) (limited to 'drivers/clocksource') diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index bede10f67f9a..f4db3a65bc79 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -44,11 +44,13 @@ #define CNTACR_RWVT BIT(4) #define CNTACR_RWPT BIT(5) -#define CNTVCT_LO 0x08 -#define CNTVCT_HI 0x0c +#define CNTVCT_LO 0x00 +#define CNTPCT_LO 0x08 #define CNTFRQ 0x10 +#define CNTP_CVAL_LO 0x20 #define CNTP_TVAL 0x28 #define CNTP_CTL 0x2c +#define CNTV_CVAL_LO 0x30 #define CNTV_TVAL 0x38 #define CNTV_CTL 0x3c @@ -112,6 +114,13 @@ void arch_timer_reg_write(int access, enum arch_timer_reg reg, u64 val, case ARCH_TIMER_REG_TVAL: writel_relaxed((u32)val, timer->base + CNTP_TVAL); break; + case ARCH_TIMER_REG_CVAL: + /* + * Not guaranteed to be atomic, so the timer + * must be disabled at this point. + */ + writeq_relaxed(val, timer->base + CNTP_CVAL_LO); + break; default: BUILD_BUG(); } @@ -124,6 +133,10 @@ void arch_timer_reg_write(int access, enum arch_timer_reg reg, u64 val, case ARCH_TIMER_REG_TVAL: writel_relaxed((u32)val, timer->base + CNTV_TVAL); break; + case ARCH_TIMER_REG_CVAL: + /* Same restriction as above */ + writeq_relaxed(val, timer->base + CNTV_CVAL_LO); + break; default: BUILD_BUG(); } @@ -720,15 +733,36 @@ static int arch_timer_set_next_event_phys(unsigned long evt, return 0; } +static u64 arch_counter_get_cnt_mem(struct arch_timer *t, int offset_lo) +{ + u32 cnt_lo, cnt_hi, tmp_hi; + + do { + cnt_hi = readl_relaxed(t->base + offset_lo + 4); + cnt_lo = readl_relaxed(t->base + offset_lo); + tmp_hi = readl_relaxed(t->base + offset_lo + 4); + } while (cnt_hi != tmp_hi); + + return ((u64) cnt_hi << 32) | cnt_lo; +} + static __always_inline void set_next_event_mem(const int access, unsigned long evt, struct clock_event_device *clk) { + struct arch_timer *timer = to_arch_timer(clk); unsigned long ctrl; + u64 cnt; + ctrl = arch_timer_reg_read(access, ARCH_TIMER_REG_CTRL, clk); ctrl |= ARCH_TIMER_CTRL_ENABLE; ctrl &= ~ARCH_TIMER_CTRL_IT_MASK; - arch_timer_reg_write(access, ARCH_TIMER_REG_TVAL, evt, clk); + if (access == ARCH_TIMER_MEM_VIRT_ACCESS) + cnt = arch_counter_get_cnt_mem(timer, CNTVCT_LO); + else + cnt = arch_counter_get_cnt_mem(timer, CNTPCT_LO); + + arch_timer_reg_write(access, ARCH_TIMER_REG_CVAL, evt + cnt, clk); arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk); } @@ -970,15 +1004,7 @@ bool arch_timer_evtstrm_available(void) static u64 arch_counter_get_cntvct_mem(void) { - u32 vct_lo, vct_hi, tmp_hi; - - do { - 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; + return arch_counter_get_cnt_mem(arch_timer_mem, CNTVCT_LO); } static struct arch_timer_kvm_info arch_timer_kvm_info; -- cgit From 30aa08da35e07a51a81d489517a3f6fb5164b09c Mon Sep 17 00:00:00 2001 From: Marc Zyngier Date: Sun, 17 Oct 2021 13:42:16 +0100 Subject: clocksource/drivers/arm_arch_timer: Advertise 56bit timer to the core code Proudly tell the code code that we have a timer able to handle 56 bits deltas. Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20211017124225.3018098-9-maz@kernel.org Signed-off-by: Daniel Lezcano --- drivers/clocksource/arm_arch_timer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/clocksource') diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index f4db3a65bc79..36e091412151 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -834,7 +834,7 @@ static void __arch_timer_setup(unsigned type, clk->set_state_shutdown(clk); - clockevents_config_and_register(clk, arch_timer_rate, 0xf, 0x7fffffff); + clockevents_config_and_register(clk, arch_timer_rate, 0xf, CLOCKSOURCE_MASK(56)); } static void arch_timer_evtstrm_enable(int divider) -- cgit From 012f188504528b8cb32f441ac3bd9ea2eba39c9e Mon Sep 17 00:00:00 2001 From: Marc Zyngier Date: Sun, 17 Oct 2021 13:42:17 +0100 Subject: clocksource/drivers/arm_arch_timer: Work around broken CVAL implementations The Applied Micro XGene-1 SoC has a busted implementation of the CVAL register: it looks like it is based on TVAL instead of the other way around. The net effect of this implementation blunder is that the maximum deadline you can program in the timer is 32bit wide. Use a MIDR check to notice the broken CPU, and reduce the width of the timer to 32bit. Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20211017124225.3018098-10-maz@kernel.org Signed-off-by: Daniel Lezcano --- drivers/clocksource/arm_arch_timer.c | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) (limited to 'drivers/clocksource') diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index 36e091412151..ef3f83865dcd 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -780,9 +780,32 @@ static int arch_timer_set_next_event_phys_mem(unsigned long evt, return 0; } +static u64 __arch_timer_check_delta(void) +{ +#ifdef CONFIG_ARM64 + const struct midr_range broken_cval_midrs[] = { + /* + * XGene-1 implements CVAL in terms of TVAL, meaning + * that the maximum timer range is 32bit. Shame on them. + */ + MIDR_ALL_VERSIONS(MIDR_CPU_MODEL(ARM_CPU_IMP_APM, + APM_CPU_PART_POTENZA)), + {}, + }; + + if (is_midr_in_range_list(read_cpuid_id(), broken_cval_midrs)) { + pr_warn_once("Broken CNTx_CVAL_EL1, limiting width to 32bits"); + return CLOCKSOURCE_MASK(32); + } +#endif + return CLOCKSOURCE_MASK(56); +} + static void __arch_timer_setup(unsigned type, struct clock_event_device *clk) { + u64 max_delta; + clk->features = CLOCK_EVT_FEAT_ONESHOT; if (type == ARCH_TIMER_TYPE_CP15) { @@ -814,6 +837,7 @@ static void __arch_timer_setup(unsigned type, } clk->set_next_event = sne; + max_delta = __arch_timer_check_delta(); } else { clk->features |= CLOCK_EVT_FEAT_DYNIRQ; clk->name = "arch_mem_timer"; @@ -830,11 +854,13 @@ static void __arch_timer_setup(unsigned type, clk->set_next_event = arch_timer_set_next_event_phys_mem; } + + max_delta = CLOCKSOURCE_MASK(56); } clk->set_state_shutdown(clk); - clockevents_config_and_register(clk, arch_timer_rate, 0xf, CLOCKSOURCE_MASK(56)); + clockevents_config_and_register(clk, arch_timer_rate, 0xf, max_delta); } static void arch_timer_evtstrm_enable(int divider) -- cgit From 41f8d02a6a558f80775bf61fe6312a14eeabbca0 Mon Sep 17 00:00:00 2001 From: Marc Zyngier Date: Sun, 17 Oct 2021 13:42:18 +0100 Subject: clocksource/drivers/arm_arch_timer: Remove any trace of the TVAL programming interface TVAL usage is now long gone, get rid of the leftovers. Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20211017124225.3018098-11-maz@kernel.org Signed-off-by: Daniel Lezcano --- drivers/clocksource/arm_arch_timer.c | 8 -------- 1 file changed, 8 deletions(-) (limited to 'drivers/clocksource') diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index ef3f83865dcd..6e20bc12dc35 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -48,10 +48,8 @@ #define CNTPCT_LO 0x08 #define CNTFRQ 0x10 #define CNTP_CVAL_LO 0x20 -#define CNTP_TVAL 0x28 #define CNTP_CTL 0x2c #define CNTV_CVAL_LO 0x30 -#define CNTV_TVAL 0x38 #define CNTV_CTL 0x3c static unsigned arch_timers_present __initdata; @@ -111,9 +109,6 @@ void arch_timer_reg_write(int access, enum arch_timer_reg reg, u64 val, case ARCH_TIMER_REG_CTRL: writel_relaxed((u32)val, timer->base + CNTP_CTL); break; - case ARCH_TIMER_REG_TVAL: - writel_relaxed((u32)val, timer->base + CNTP_TVAL); - break; case ARCH_TIMER_REG_CVAL: /* * Not guaranteed to be atomic, so the timer @@ -130,9 +125,6 @@ void arch_timer_reg_write(int access, enum arch_timer_reg reg, u64 val, case ARCH_TIMER_REG_CTRL: writel_relaxed((u32)val, timer->base + CNTV_CTL); break; - case ARCH_TIMER_REG_TVAL: - writel_relaxed((u32)val, timer->base + CNTV_TVAL); - break; case ARCH_TIMER_REG_CVAL: /* Same restriction as above */ writeq_relaxed(val, timer->base + CNTV_CVAL_LO); -- cgit From c1153d52c4140424a5e31a5916fca3edd91fe13a Mon Sep 17 00:00:00 2001 From: Oliver Upton Date: Sun, 17 Oct 2021 13:42:20 +0100 Subject: clocksource/drivers/arm_arch_timer: Fix masking for high freq counters Unfortunately, the architecture provides no means to determine the bit width of the system counter. However, we do know the following from the specification: - the system counter is at least 56 bits wide - Roll-over time of not less than 40 years To date, the arch timer driver has depended on the first property, assuming any system counter to be 56 bits wide and masking off the rest. However, combining a narrow clocksource mask with a high frequency counter could result in prematurely wrapping the system counter by a significant margin. For example, a 56 bit wide, 1GHz system counter would wrap in a mere 2.28 years! This is a problem for two reasons: v8.6+ implementations are required to provide a 64 bit, 1GHz system counter. Furthermore, before v8.6, implementers may select a counter frequency of their choosing. Fix the issue by deriving a valid clock mask based on the second property from above. Set the floor at 56 bits, since we know no system counter is narrower than that. [maz: fixed width computation not to lose the last bit, added max delta generation for the timer] Suggested-by: Marc Zyngier Signed-off-by: Oliver Upton Reviewed-by: Linus Walleij Signed-off-by: Marc Zyngier Link: https://lore.kernel.org/r/20210807191428.3488948-1-oupton@google.com Link: https://lore.kernel.org/r/20211017124225.3018098-13-maz@kernel.org Signed-off-by: Daniel Lezcano --- drivers/clocksource/arm_arch_timer.c | 34 +++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) (limited to 'drivers/clocksource') diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c index 6e20bc12dc35..9a04eacc4412 100644 --- a/drivers/clocksource/arm_arch_timer.c +++ b/drivers/clocksource/arm_arch_timer.c @@ -52,6 +52,12 @@ #define CNTV_CVAL_LO 0x30 #define CNTV_CTL 0x3c +/* + * The minimum amount of time a generic counter is guaranteed to not roll over + * (40 years) + */ +#define MIN_ROLLOVER_SECS (40ULL * 365 * 24 * 3600) + static unsigned arch_timers_present __initdata; struct arch_timer { @@ -95,6 +101,22 @@ static int __init early_evtstrm_cfg(char *buf) } early_param("clocksource.arm_arch_timer.evtstrm", early_evtstrm_cfg); +/* + * Makes an educated guess at a valid counter width based on the Generic Timer + * specification. Of note: + * 1) the system counter is at least 56 bits wide + * 2) a roll-over time of not less than 40 years + * + * See 'ARM DDI 0487G.a D11.1.2 ("The system counter")' for more details. + */ +static int arch_counter_get_width(void) +{ + u64 min_cycles = MIN_ROLLOVER_SECS * arch_timer_rate; + + /* guarantee the returned width is within the valid range */ + return clamp_val(ilog2(min_cycles - 1) + 1, 56, 64); +} + /* * Architected system timer support. */ @@ -212,13 +234,11 @@ static struct clocksource clocksource_counter = { .id = CSID_ARM_ARCH_COUNTER, .rating = 400, .read = arch_counter_read, - .mask = CLOCKSOURCE_MASK(56), .flags = CLOCK_SOURCE_IS_CONTINUOUS, }; static struct cyclecounter cyclecounter __ro_after_init = { .read = arch_counter_read_cc, - .mask = CLOCKSOURCE_MASK(56), }; struct ate_acpi_oem_info { @@ -790,7 +810,7 @@ static u64 __arch_timer_check_delta(void) return CLOCKSOURCE_MASK(32); } #endif - return CLOCKSOURCE_MASK(56); + return CLOCKSOURCE_MASK(arch_counter_get_width()); } static void __arch_timer_setup(unsigned type, @@ -1035,6 +1055,7 @@ struct arch_timer_kvm_info *arch_timer_get_kvm_info(void) static void __init arch_counter_register(unsigned type) { u64 start_count; + int width; /* Register the CP15 based counter if we have one */ if (type & ARCH_TIMER_TYPE_CP15) { @@ -1059,6 +1080,10 @@ static void __init arch_counter_register(unsigned type) arch_timer_read_counter = arch_counter_get_cntvct_mem; } + width = arch_counter_get_width(); + clocksource_counter.mask = CLOCKSOURCE_MASK(width); + cyclecounter.mask = CLOCKSOURCE_MASK(width); + if (!arch_counter_suspend_stop) clocksource_counter.flags |= CLOCK_SOURCE_SUSPEND_NONSTOP; start_count = arch_timer_read_counter(); @@ -1068,8 +1093,7 @@ static void __init arch_counter_register(unsigned type) timecounter_init(&arch_timer_kvm_info.timecounter, &cyclecounter, start_count); - /* 56 bits minimum, so we assume worst case rollover */ - sched_clock_register(arch_timer_read_counter, 56, arch_timer_rate); + sched_clock_register(arch_timer_read_counter, width, arch_timer_rate); } static void arch_timer_stop(struct clock_event_device *clk) -- cgit