linux-kernel - Re: [PATCH] clocksource/drivers/arm_arch_timer: Add workaround for MediaTek MMIO timer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f3f01c18-a2cc-9329-d186-421f5170e08a@collabora.com>
Date:   Mon, 17 Apr 2023 12:01:15 +0200
From:   AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>
To:     walter.chang@...iatek.com,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>,
        Jonathan Corbet <corbet@....net>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Mark Rutland <mark.rutland@....com>,
        Marc Zyngier <maz@...nel.org>,
        Matthias Brugger <matthias.bgg@...il.com>
Cc:     wsd_upstream@...iatek.com, stanley.chu@...iatek.com,
        Chun-hung.Wu@...iatek.com, Freddy.Hsin@...iatek.com,
        linux-arm-kernel@...ts.infradead.org, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-mediatek@...ts.infradead.org
Subject: Re: [PATCH] clocksource/drivers/arm_arch_timer: Add workaround for
 MediaTek MMIO timer

Il 17/04/23 11:06, walter.chang@...iatek.com ha scritto:
> From: Walter Chang <walter.chang@...iatek.com>
> 
> The MT69XX series SoCs have the incomplete implementation issue in the
> mmio timer. Specifically, the hardware only implements the TVAL
> functionality, but not the CVAL functionality. This hardware limitation
> will cause set_next_event_mem() fail to set the actual expiration time
> when writing a value to the CVAL. On these platforms, the mmio timer's
> internal expiration time will still be judged as 0 (the value of TVAL),
> resulting in the mmio timer not functioning as intended.
> 
> The workaround is to use TVAL in addition to CVAL for these affected
> platforms.
> 
> Signed-off-by: Walter Chang <walter.chang@...iatek.com>
> ---
>   Documentation/arm64/silicon-errata.rst |  4 ++++
>   drivers/clocksource/Kconfig            |  9 ++++++++
>   drivers/clocksource/arm_arch_timer.c   | 29 ++++++++++++++++++++++++++
>   3 files changed, 42 insertions(+)
> 
> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
> index ec5f889d7681..ca1893713a4c 100644
> --- a/Documentation/arm64/silicon-errata.rst
> +++ b/Documentation/arm64/silicon-errata.rst
> @@ -209,3 +209,7 @@ stable kernels.
>   +----------------+-----------------+-----------------+-----------------------------+
>   | Fujitsu        | A64FX           | E#010001        | FUJITSU_ERRATUM_010001      |
>   +----------------+-----------------+-----------------+-----------------------------+
> +
> ++----------------+-----------------+-----------------+-----------------------------+
> +| MediaTek       | MT69XX series   | #690001         | MEDIATEK_ERRATUM_690001     |
> ++----------------+-----------------+-----------------+-----------------------------+
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 5fc8f0e7fb38..475356b8dbdc 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -368,6 +368,15 @@ config SUN50I_ERRATUM_UNKNOWN1
>   	  the Allwinner A64 SoC. The workaround will only be active if the
>   	  allwinner,erratum-unknown1 property is found in the timer node.
>   
> +config MEDIATEK_ERRATUM_690001
> +	bool "Workaround for MediaTek MT69XX erratum 690001"
> +	depends on ARM_ARCH_TIMER && ARM64
> +	help
> +	  This option enables a workaround for incomplete implementation
> +	  in the MMIO timer on the MediaTek MT69XX SoCs. The workaround
> +	  will only be active if mediatek,erratum-690001 property is
> +	  found in the timer node.
> +
>   config ARM_GLOBAL_TIMER
>   	bool "Support for the ARM global timer" if COMPILE_TEST
>   	select TIMER_OF if OF
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index e09d4427f604..920570d57fc0 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -389,6 +389,10 @@ static u64 notrace sun50i_a64_read_cntvct_el0(void)
>   }
>   #endif
>   
> +#ifdef CONFIG_MEDIATEK_ERRATUM_690001
> +static bool arch_timer_mem_sne_use_tval __ro_after_init;

What about reusing part of the CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND logic and
adding something like CONFIG_ARM_ARCH_TIMER_MEM_WORKAROUND?

That would make you able to reuse the already existing infrastructure to parse
the device-tree and possibly more.

Regards,
Angelo

> +#endif
> +
>   #ifdef CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND
>   DEFINE_PER_CPU(const struct arch_timer_erratum_workaround *, timer_unstable_counter_workaround);
>   EXPORT_SYMBOL_GPL(timer_unstable_counter_workaround);
> @@ -783,6 +787,19 @@ static __always_inline void set_next_event_mem(const int access, unsigned long e
>   		cnt = arch_counter_get_cnt_mem(timer, CNTPCT_LO);
>   
>   	arch_timer_reg_write(access, ARCH_TIMER_REG_CVAL, evt + cnt, clk);
> +#ifdef CONFIG_MEDIATEK_ERRATUM_690001
> +	if (arch_timer_mem_sne_use_tval) {
> +		/* Due to the incomplete implementation of mmio timer on
> +		 * specific MediaTek platforms, CVAL has not been implemented.
> +		 * Therefore, the workaround is to use TVAL in addition to
> +		 * CVAL.
> +		 */
> +		if (access == ARCH_TIMER_MEM_VIRT_ACCESS)
> +			writel_relaxed(evt, timer->base + 0x38);
> +		else
> +			writel_relaxed(evt, timer->base + 0x28);
> +	}
> +#endif
>   	arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk);
>   }
>   
> @@ -878,7 +895,16 @@ static void __arch_timer_setup(unsigned type,
>   				arch_timer_set_next_event_phys_mem;
>   		}
>   
> +#ifdef CONFIG_MEDIATEK_ERRATUM_690001
> +		if (arch_timer_mem_sne_use_tval) {
> +			pr_info("Enabling mediatek,erratum-690001 for mmio timer\n");
> +			max_delta = CLOCKSOURCE_MASK(31);
> +		} else {
> +			max_delta = CLOCKSOURCE_MASK(56);
> +		}
> +#else
>   		max_delta = CLOCKSOURCE_MASK(56);
> +#endif
>   	}
>   
>   	clk->set_state_shutdown(clk);
> @@ -1591,6 +1617,9 @@ static int __init arch_timer_mem_of_init(struct device_node *np)
>   		frame->valid = true;
>   	}
>   
> +#ifdef CONFIG_MEDIATEK_ERRATUM_690001
> +	arch_timer_mem_sne_use_tval = of_property_read_bool(np, "mediatek,erratum-690001");
> +#endif
>   	frame = arch_timer_mem_find_best_frame(timer_mem);
>   	if (!frame) {
>   		pr_err("Unable to find a suitable frame in timer @ %pa\n",